Add auto-saving draft with Save / Save As and Wipe#108
Conversation
|
Note Reviews pausedUse the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds a per-project draft buffer with save, save as, discard, and wipe flows, updates the loader and modal wiring around that draft state, introduces morphology display and editing controls, bundles view options, and expands tests, strings, menus, settings, and supporting mocks. ChangesDraft workflow and morphology UI
Sequence Diagram(s)sequenceDiagram
participant User
participant InterlinearizerLoader
participant ProjectModals
participant Main
participant ProjectStorage
User->>InterlinearizerLoader: Save / Save As / Wipe action
InterlinearizerLoader->>ProjectModals: open saveAs or confirm flow
ProjectModals->>Main: createProject / saveAnalysis / getProject
InterlinearizerLoader->>Main: saveDraft
Main->>ProjectStorage: saveDraft(sourceProjectId, draft)
ProjectModals-->>InterlinearizerLoader: markSynced or loadFromProject
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
alex-rawlings-yyc
left a comment
There was a problem hiding this comment.
@alex-rawlings-yyc resolved 7 discussions.
Reviewable status: 0 of 29 files reviewed, all discussions resolved (waiting on alex-rawlings-yyc).
c99f7ea to
6fa7dfa
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
03ce6af to
58101e8
Compare
Editing the interlinearizer no longer writes to the active project on every
keystroke. Each source project now has an always-present draft
(draft:{sourceProjectId}) that auto-saves every edit to papi storage,
decoupled from the user's saved projects and never shown in the picker, so
work is never lost.
Persistence is now explicit:
- Save writes the draft to the active project
- Save As writes the draft to a new project, or overwrites an existing one
- New starts an empty draft (a project is created only on Save As)
- Open loads a project into the draft as a working copy
- Wipe clears the draft — the current book or the whole thing
Switching projects (New / Open) while the draft is dirty prompts to discard.
The tab title shows a "●" marker while the draft has unsaved changes, toggled
via updateWebViewDefinition (Platform.Bible exposes no native unsaved-tab
indicator).
Implementation:
- DraftProject type + isDraftProject guard; getDraft/saveDraft storage with a
per-source serialization queue; getDraft/saveDraft backend commands.
- useDraftProject hook owns the draft (autosave, dirty tracking, and a
draftVersion that remounts the editor on New/Open/Wipe).
- New SaveAsProjectModal, WipeConfirm, and DiscardDraftConfirm; the New modal
is repurposed to configure a draft rather than create a project.
- removeBookFromAnalysis util backs the per-book wipe.
Alignment links are intentionally not carried in the draft yet (no link-editing
feature exists); Save preserves a target project's existing links.
Deferred UX decisions are recorded in user-questions.md. 988 tests, 100%
coverage, lint clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- markSynced now only clears the dirty flag when the live draft still matches the persisted snapshot, so a gloss edit made during an in-flight Save / Save As no longer clears the unsaved-changes indicator while the project is left stale. The save paths pass the analysis they wrote. - The discard-changes confirmation overlays the active modal instead of replacing it, so canceling returns to the Create dialog with its typed input intact, and confirming Open no longer unmounts and re-fetches the still-open select modal underneath. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
58101e8 to
ad59825
Compare
* Address review feedback: race fix, draft validation, autosave debounce, save guard
- handleConfirmReplace: await openProject so the discard confirm stays up during
the project fetch; setPendingReplace only clears after the modal is dismissed
- getDraft: validate parsed JSON with isDraftProject; resets to empty draft on
shape mismatch instead of propagating an unvalidated object
- autosaveAnalysis: debounce persist (300 ms trailing edge) to prevent unbounded
command queue growth under rapid typing; applyReplacement cancels any pending
debounce before its immediate write, and the sourceProjectId effect cleanup
cancels it on source change
- handleSave: add isSavingRef guard (matching SaveAsProjectModal pattern) so a
rapid double-click cannot fire two saveAnalysis commands
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Restore project creation in the New flow
startNewDraft was refactored to only reset the local draft, but this meant
clicking "Create" in "New Interlinear Project..." never persisted anything.
The project would not appear in "Select Interlinear Project..." because
interlinearizer.createProject was never called.
Restore the original behaviour: startNewDraft now calls createProject, loads
the empty project into the draft via loadFromProject, and sets it as the active
Save target — exactly what the old CreateProjectModal did before the draft model
was introduced. handleConfirmReplace now also awaits startNewDraft so the discard
confirm stays visible for the duration of the network call, consistent with the
openProject path.
Remove the now-unused resetDraft prop from ProjectModals and its call-site in
InterlinearizerLoader.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Cancel debounced autosave in markSynced to prevent stale dirty overwrite
The setTimeout closure in autosaveAnalysis captures `next` ({dirty:true}) at
scheduling time. If markSynced runs before the 300ms timer fires (e.g. a fast
Save round-trip), it persists {dirty:false} first — then the stale timer fires
and persists the captured {dirty:true}, overwriting the clean state in storage.
applyReplacement already cancels the timer before its own persist; apply the
same pattern to markSynced so the {dirty:false} record is always the last write.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Fix tests, coverage, and JSDoc for debounce and project-creation changes
- Update autosaveAnalysis tests to advance fake timers past the 300ms
debounce before asserting saveDraft was called
- Rewrite the two ProjectModals "new flow" tests to assert
interlinearizer.createProject is called instead of the removed
resetDraft prop; remove resetDraft from ModalsOverrides/buildProps
- Fix InterlinearizerLoader autosave test with the same fake-timer pattern
- Add startNewDraft error-path tests (invalid shape, targetProjectId
branch, and throw) to close ProjectModals branch coverage gaps
- Add useDraftProject test verifying applyReplacement cancels a pending
autosave timer so the stale dirty write never fires after a reset
- Add projectStorage getDraft test for the isDraftProject validation-fail
path (returns empty draft + warns)
- Fix /* v8 ignore next */ spans on multi-line guards in
InterlinearizerLoader (isSavingRef) and ProjectModals (isCreatingRef)
- Remove stale @param props.resetDraft JSDoc; document re-entry guard in
handleCreateDraft JSDoc
- Fix PendingReplace.new.config type from undefined NewDraftConfig to
CreateDraftConfig
- jest.config.ts: exclude .claude worktrees from Haste module resolution;
add isolatedModules: true so ts-jest runs from worktree paths
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Tidy
* Fix debounce data loss on unmount and duplicate-create on DiscardDraftConfirm
- Flush the pending debounced autosave in the useEffect cleanup so the last
edit before unmount or source-project change is not silently dropped.
- Add isCreatingRef guard to the 'new' path of handleConfirmReplace, matching
the guard already present in handleCreateDraft, so rapid clicks on the
DiscardDraftConfirm button cannot fire multiple createProject commands.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Remove resetDraft, dead NewDraftConfig type, and related tests
resetDraft is no longer called by any production code — the New flow now
goes through startNewDraft → loadFromProject in ProjectModals. Remove the
hook method, its exported NewDraftConfig type, its unit tests, and the
stale prop reference in the InterlinearizerLoader mock.
Also make handleCreateDraft async so it awaits the isCreatingRef reset,
and tighten two inline comments in useDraftProject to reflect that the
cleanup now flushes rather than silently drops a pending autosave.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Add loading feedback to Create modal and drop redundant submitting refs
- Add `isSubmitting` prop to `CreateProjectModal`; `ProjectModals` threads
`isCreating` state into it so both buttons are disabled during the backend
round-trip, addressing the reviewer's missing-loading-indicator comment
- Drop the `ref + state` dual pattern from `ProjectModals`, `SaveAsProjectModal`,
and `ProjectMetadataModal`; state alone is sufficient because the buttons are
already disabled before a second invocation can reach the guard
- Add a `useDraftProject` test covering the timeout-cancellation branch in
`applyReplacement` to restore 100% coverage
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Disable DiscardDraftConfirm buttons during async project creation
The confirm overlay remained interactive during the backend round-trip when
the 'new' branch of handleConfirmReplace was awaiting startNewDraft. Both
buttons now receive isSubmitting so the user gets clear visual feedback and
cannot interact with the dialog while creation is in flight.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Tweak
* Fix race: disable DiscardDraftConfirm during both open and new-draft flows
isCreating only covered the new-draft branch of handleConfirmReplace,
leaving the open-project branch unguarded. A single isReplacing state now
wraps both paths, so the discard overlay buttons are disabled for the full
duration of the async operation.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
@imnasnainaec reviewed 16 files and all commit messages, and made 4 comments.
Reviewable status: 16 of 61 files reviewed, 3 unresolved discussions (waiting on alex-rawlings-yyc).
a discussion (no related file):
It appears Devin (https://app.devin.ai/review/sillsdev/interlinearizer-extension/pull/108) found another bug (and a handful of ⛏️s).
user-questions.md line 36 at r6 (raw file):
3. **"Wipe book" scope.** "Wipe Current Book" targets the book currently in view. Alternative: present a picker of books that have draft analysis. Current choice: current book only.
An alternative would be to combine the 2 wipe options, the dialog could give wipe all vs wipe selected books options. I'm pondering whether that should even be done now to simplify the menu and give more immediate view of edited books when the decision is being made. (Also easier to have only one option when only 1 book has been modified.)
Code quote:
3. **"Wipe book" scope.** "Wipe Current Book" targets the book currently in view. Alternative: present
a picker of books that have draft analysis. Current choice: current book only.src/__tests__/components/InterlinearizerLoader.test.tsx line 609 at r6 (raw file):
}); expect(capturedInterlinearizerProps?.viewOptions.simplifyPhrases).toBe(false);
⛏️ These strictly-default-prop-capture tests seem like they could be combined into a single test.
Code quote:
expect(capturedInterlinearizerProps?.viewOptions.simplifyPhrases).toBe(false);src/__tests__/types/type-guards.test.ts at r6 (raw file):
The type-guards.ts file is located in src/types/ which is excluded from the coverage requirements, so it would be okay to delete this test file.
|
Previously, imnasnainaec (D. Ror.) wrote…
I'm going to add an issue to add UI that allows users to see books that they've interlinearized. Perhaps they could choose to wipe books from there. We can discuss this further at a later time. I think for now I will add a combined "Wipe..." menu item that opens a dialogue |
alex-rawlings-yyc
left a comment
There was a problem hiding this comment.
@alex-rawlings-yyc resolved 2 discussions.
Reviewable status: 16 of 61 files reviewed, 1 unresolved discussion (waiting on alex-rawlings-yyc and imnasnainaec).
imnasnainaec
left a comment
There was a problem hiding this comment.
@imnasnainaec reviewed 55 files and all commit messages, and resolved 1 discussion.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on alex-rawlings-yyc).
alex-rawlings-yyc
left a comment
There was a problem hiding this comment.
@alex-rawlings-yyc resolved 1 discussion.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on alex-rawlings-yyc).
Editing the interlinearizer no longer writes to the active project on every keystroke. Each source project now has an always-present draft (draft:{sourceProjectId}) that auto-saves every edit to papi storage, decoupled from the user's saved projects and never shown in the picker, so work is never lost.
Persistence is now explicit:
Switching projects (New / Open) while the draft is dirty prompts to discard. The tab title shows a "●" marker while the draft has unsaved changes, toggled via updateWebViewDefinition (Platform.Bible exposes no native unsaved-tab indicator).
Implementation:
Alignment links are intentionally not carried in the draft yet (no link-editing feature exists); Save preserves a target project's existing links.
Deferred UX decisions are recorded in user-questions.md. 988 tests, 100% coverage, lint clean.
This change is
Summary by CodeRabbit
Release Notes
New Features