Skip to content

fix(editor): surface failed timeline saves - #308

Open
arhxam wants to merge 3 commits into
getopenscreen:mainfrom
arhxam:codex/surface-timeline-save-failures
Open

fix(editor): surface failed timeline saves#308
arhxam wants to merge 3 commits into
getopenscreen:mainfrom
arhxam:codex/surface-timeline-save-failures

Conversation

@arhxam

@arhxam arhxam commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Summary

  • route timeline mutation writes through one shared failure boundary
  • show the existing Save failed toast with the native write error while resolving detached mutation promises safely
  • roll optimistic zoom-focus and annotation drags back to their last pre-drag document when the commit fails
  • keep sequential edits usable after a failed write and avoid misleading post-save selection/success updates
  • cover the two metadata-driven saves that previously bypassed the timeline hooks

The issue's detached-call grep now only reports calls whose implementations pass through this resolving boundary; the two direct state.saveDocument results are gone.

Related issue

Fixes #282

Type of change

  • Bug fix
  • Feature
  • Enhancement
  • Documentation
  • Refactor / maintenance
  • Performance
  • Security

Release impact

  • Patch
  • Minor
  • Major / breaking change
  • No release note needed

Desktop impact

  • Windows
  • macOS
  • Linux
  • Installer / packaging
  • Not platform-specific

Screenshots / video

Not included; the visible change is an existing Sonner error toast when a forced save fails.

Testing

  • Started with failing tests proving both hooks rejected a forced save before the fix
  • npx vitest --run src/lib/ai-edition/store/useTimeline.test.ts src/lib/ai-edition/store/useSequentialTimelineOps.test.ts (26 passed)
  • npm run test (1,679 passed, 1 skipped)
  • npx tsc --noEmit
  • npx tsc -p tsconfig.test.json --noEmit
  • npx biome check on all seven changed files
  • npm run docs:check
  • npm run i18n:check
  • npm run build-vite

Authored with Codex assistance and manually reviewed against every acceptance criterion in #282.

Summary by CodeRabbit

  • Bug Fixes

    • Improved timeline saving reliability with clear error notifications when changes cannot be saved.
    • Prevented unsuccessful edits from leaving the timeline in an inconsistent state by restoring the previous state when needed.
    • Failed operations no longer block subsequent timeline actions.
    • Improved handling for zoom focus, annotations, clips, and regions when saving fails.
  • Tests

    • Added coverage for save failures, state restoration, error notifications, and continued operation processing.

@arhxam
arhxam requested a review from EtienneLescot as a code owner August 8, 2026 18:20
@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review skipped

No new commits to review since the last review.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: fb65a69d-e4bf-4286-895a-1d66346c2e4e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Timeline saves now use a shared mutation helper. Save failures show an error toast and return failure results. Sequential operations continue processing. Direct mutations stop dependent state updates and restore live-edit documents when persistence fails.

Changes

Timeline save handling

Layer / File(s) Summary
Shared save mutation contract
src/lib/ai-edition/store/timelineSave.ts, src/lib/ai-edition/store/useSequentialTimelineOps.ts, src/lib/ai-edition/store/useSequentialTimelineOps.test.ts
Centralized save failures into logged error toasts and boolean results. Sequential operations return null on failure while later queued operations continue.
Timeline mutation rollback and guards
src/lib/ai-edition/store/useTimeline.ts, src/lib/ai-edition/store/useTimeline.test.ts
Timeline mutations stop follow-up state changes after failed saves. Live zoom-focus and annotation edits restore their previous documents. Tests cover rollback, unchanged clips, and error toasts.
Editor initialization save integration
src/components/ai-edition/NewEditorShell.tsx
Timeline initialization and probed-duration updates now use the shared save helper.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: etiennelescot

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes satisfy the coding objectives and acceptance criteria for failed timeline saves in [#282].
Out of Scope Changes check ✅ Passed All changes support centralized timeline save failure handling, rollback behavior, metadata coverage, or related regression tests.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title clearly and concisely describes the main change: surfacing failed timeline saves.
Description check ✅ Passed The description includes the required summary, issue, change type, release impact, platform impact, screenshots note, and detailed testing information.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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 `@src/lib/ai-edition/store/useSequentialTimelineOps.test.ts`:
- Around line 148-163: Update the test around the sequential apply calls to
capture the result of the second operation, assert that it is non-null, and
verify that saveDocument receives the second write. Keep the existing
first-result and toast-error assertions, while ensuring the test distinguishes a
genuinely successful second operation from one resolving to null.

In `@src/lib/ai-edition/store/useTimeline.ts`:
- Around line 519-523: Remove the revision increment from both failed-live-edit
rollback setState calls in src/lib/ai-edition/store/useTimeline.ts at lines
519-523 and 631-635. Keep the conditional document match and rollback
document/dirty updates unchanged; these rollback paths must not modify revision.
🪄 Autofix

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: 2fcbfd92-ad30-4fc0-b840-49c1c151e412

📥 Commits

Reviewing files that changed from the base of the PR and between 4e7a85b and f03057b.

📒 Files selected for processing (7)
  • src/components/ai-edition/NewEditorShell.tsx
  • src/components/ai-edition/v4/V4Timeline.tsx
  • src/lib/ai-edition/store/timelineSave.ts
  • src/lib/ai-edition/store/useSequentialTimelineOps.test.ts
  • src/lib/ai-edition/store/useSequentialTimelineOps.ts
  • src/lib/ai-edition/store/useTimeline.test.ts
  • src/lib/ai-edition/store/useTimeline.ts

Comment thread src/lib/ai-edition/store/useSequentialTimelineOps.test.ts
Comment thread src/lib/ai-edition/store/useTimeline.ts
@arhxam

arhxam commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Removed the unrelated V4Timeline auto-zoom success-toast hunk in ffa0cc9, so this PR is now limited to the timeline save-failure boundary and rollback behavior for #282.

@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

@arhxam I will review the current PR scope for the timeline save-failure boundary and rollback behavior in #282.

⚠️ Action not completed

Already reviewed.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: a failed timeline save is invisible — the edit stays on screen, the document is not written

1 participant