Repository navigation
Move Save and Save As into the host - #475
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
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.
These comments were generated by Claude Opus 5.5 (Claude Code) and reviewed by me before posting.
Save and Save As no longer send the analysis over the bus. The WebView settles its draft writes, then one host command copies the stored draft into the interlinear project and replies with the project's summary. saveAnalysis, which carried the whole analysis both ways, is removed. Three commands become two: saveDraftToProject covers Save and Save As onto an existing project, taking the draft's configuration as one object so that overwrite sets the project's languages and target in the same write it used to need a second call for; saveDraftAsProject covers Save As to a new project, creating and filling it in one call where a failed second call used to leave an empty project behind. Each reports its own failure, so a failed Save As to a new project no longer reads as a failed save of an existing one. useDraftProject owns the save end to end through commitDraft: it settles the draft's writes through a DebouncedWriter, which sends any scheduled write, waits for every write in flight, and sends a failed write once more, so the stored draft is the one in memory before the host copies it; it then runs the copy and marks the draft synced only when the copy landed. A save started while another runs does nothing, and a draft replaced while the writes settle is not saved. While a save runs, the toolbar shows it and New, Open, Save, Save As, and Wipe tell the user to wait, so the draft the copy reads is the one the user chose to save. A Save whose project no longer exists says so. The copy reads the stored draft strictly: a damaged shard or envelope fails the save instead of writing an emptied book into the project. A wiped book's shard that cannot be deleted no longer fails the draft write that stored everything else; it stays journaled for a later save to retry, so one stuck deletion cannot fail every Save after it. The perf harness times saveDraftToProject in place of saveAnalysis and records the request size as the bytes sent, so the Phase 1 checkpoint keeps a number to compare against the 74.9 MB baseline. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Save no longer sends the draft again once the copy lands. The host marks the stored draft clean itself, rewriting only its envelope, and only when no draft write reached it after the copy read the draft. The WebView flips its own flag and writes the clean draft only when a write went out during the Save, as an edit undone mid-save does. The perf harness no longer waits for an autosave after each Save. A book of the draft that cannot be read no longer fails every Save. The copy names such books instead of throwing: Save keeps the project's stored analysis and boundaries for them, Save As leaves them out, and either tells the user which books. A draft whose envelope is unreadable still fails, since no book in it can be vouched for, and its first autosave rewrites the envelope. AnalysisRepository: exportAll is lenient again, as 1b had it; exportForCopy reports the books it could not read; markClean records the stored draft as saved. The saveDraftToProject command metadata describes its config object rather than two positional parameters, and commitDraft documents its rejections with @throws. "Saving..." uses the ellipsis character like the other progress strings, a test comment no longer names hook internals, and the gating and flush tests are split per side. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ce03673 to
72fd355
Compare
alex-rawlings-yyc
left a comment
There was a problem hiding this comment.
These comments were generated by Claude Opus 5.5 (Claude Code) and reviewed by me before posting.
A wipe now lets go of a book whose stored draft analysis could not be read. Before, the draft repository held such a book through every save that carried nothing for it, Wipe Book and Wipe All included, so Save kept the project's copy of it and warned on every Save until the user edited inside it. Each draft write now names the books released since the draft was loaded: Wipe Book adds its book, and Wipe All, New and Open release all of them. Releasing a book again is harmless, so every write carries the whole set and a failed write needs no retry of its own; a load starts the set afresh, since holds come from a load and book codes repeat across sources. AnalysisRepository.replaceAll and the saveDraft command take the released books as an optional argument. Save As leaves out the boundaries of a book it could not read along with its analysis, matching what it tells the user. The unreadable books warning names analyses linked to no book in words rather than by their storage name. A test now covers an edit and its undo that are both written before the copy lands, the case only the count of writes sent can catch. It runs on fake timers for its whole body, so each write fires when the test advances the clock and never on real time. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
alex-rawlings-yyc
left a comment
There was a problem hiding this comment.
These comments were generated by Claude Opus 5.5 (Claude Code) and reviewed by me before posting.
Wiping a book whose stored draft analysis could not be read deleted that analysis, so undoing the wipe could not bring the hold back, and the next Save emptied the project's copy of the book without a word. A wipe now changes only the draft: the draft records its wiped books, Wipe Book adding its book and Wipe All, New and Open recording all of them, as part of the content the undo history restores. The host keeps an unreadable book's stored analysis whatever the draft says, stores the wiped books as those it actually holds unread, and a Save empties only the ones the draft has wiped. The release argument on saveDraft and AnalysisRepository.replaceAll is gone. A failed update of the shard journal no longer fails a draft save whose content has landed; it is logged, and the next save retries it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
alex-rawlings-yyc
left a comment
There was a problem hiding this comment.
These comments were generated by Claude Opus 5.5 (Claude Code) and reviewed by me before posting.
Stop saving around a draft book whose stored analysis cannot be read, and refuse instead. Each review round had found another path through the partial save that kept a project's copy of such a book; the case needs a torn write or outside damage to arise, so it is not worth that machinery. Refuse a draft write that carries analysis for an unreadable book, so an edit can never hide the damage, and refuse Save to an existing project while any book is unreadable, so the project is never touched. Save As to a new project still leaves such a book out, so the rest of the draft can be kept. New and Open discard the unreadable analysis, which is how the user recovers; each refusal tells the user which books and how, once until a write succeeds. Drop the draft's wiped books, the copy that kept a project's books, and the clean-draft write after an edit undone during a Save: the draft simply stays marked unsaved until the next Save. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@alex-rawlings-yyc, a change of direction on the unreadable-book handling, prompted by your fourth round. Your finding held, and so did a broader one: with JHN's draft shard damaged, a single gloss in JHN that lands, with no undo at all, makes the next Save replace the project's whole JHN with that one gloss. Looking back over the rounds, 8 of the 9 substantive findings since round one were paths through the same design: saving around a book the draft cannot read, keeping the project's copy, tracking wipes, and so on. Each fix added state the next round found a way around. So 1d now fails fast instead. A draft write carrying analysis for an unreadable book is refused, so an edit can never hide the damage. Save to an existing project is refused while any book is unreadable, so the project is never touched. Save As to a new project leaves the book out, so other work can still be kept. New and Open discard the unreadable analysis, which is how the user recovers, and each refusal tells them so once until a write succeeds. The case needs a torn write or outside damage, so I would rather it be loud and simple than handled. That removes |
alex-rawlings-yyc
left a comment
There was a problem hiding this comment.
These comments were generated by Claude Opus 5.5 (Claude Code) and reviewed by me before posting.
|
@alex-rawlings-yyc Keep
Reverse (refuse, as in
|
alex-rawlings-yyc
left a comment
There was a problem hiding this comment.
@alex-rawlings-yyc reviewed 24 files and all commit messages, and resolved 25 discussions.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on imnasnainaec).
imnasnainaec
left a comment
There was a problem hiding this comment.
@imnasnainaec reviewed 22 files and all commit messages, and resolved 1 discussion.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on jasonleenaylor).
imnasnainaec
left a comment
There was a problem hiding this comment.
@imnasnainaec reviewed 2 files and all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on jasonleenaylor).
alex-rawlings-yyc
left a comment
There was a problem hiding this comment.
@alex-rawlings-yyc reviewed 2 files and all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on jasonleenaylor).
The interface now names the books a load could not read, keeps such a book as stored on save unless told to discard it, settles a save once its content lands, and adds exportForCopy and markClean. The suite checks each of those at the seam, so a later backend owes them too. - ConformanceStore gains hooks to damage one book or the whole draft, fail a save's cleanup after its content lands, and report leftovers. - The DraftRepository runner tolerates warnings only in tests that damage storage or fail cleanup. - DraftRepository tests the suite now covers are dropped; those that check which keys are written or what bytes stay remain. - exportForCopy's doc names the empty draft it returns when none is stored. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Closes #433. Part of Phase 1 (#424) of the data layer plan, step 1d.
Summary
Save and Save As no longer send the analysis over the bus. The WebView settles its draft writes, then one host command copies the stored draft into the interlinear project and replies with the project's summary.
saveAnalysisis removed.Save As to a new project creates and fills the project in one call, so a failure leaves no empty project behind, and reports its failure as a failed project creation. A Save whose project no longer exists says so.
While a save runs, the toolbar shows "Saving…", and New, Open, Save, Save As and Wipe send an info notification and do nothing, so the draft the copy reads is the one the user chose to save.
src/utils/debounced-writer.tsowns the autosave timer, the latest write and its failure. Itswritemust land writes in the order they are sent, which the host's per-source draft queue does.Unreadable draft books
A draft book whose stored analysis cannot be read (a torn write or outside damage) is kept as stored and never written over:
Changes to 1b's repository
@imnasnainaec, for the conformance suite in #469:
replaceAll(draft, options)resolves with the books it left out. A book its load could not read keeps its shard as stored, whatever the draft carries for it, untildiscardUnreadablesays the draft was started over. In 1b, a save carrying analysis for such a book wrote over its shard.replaceAllno longer rejects once the draft has landed: a failed orphan-shard deletion or journal update is logged, and a later save retries it.exportAllresolves with the draft and the books it could not read.AnalysisRepositorygainsexportForCopy, which reads asexportAlldoes but rejects an unreadable envelope, andmarkClean, which rewrites only the envelope.Evidence
TextAnalysisinsaveAnalysis(74.9 MB at the largest perf tier, per the Phase 1 baseline indocs/data-layer-plan/plan.md).After: Save sends two ids, and no draft write follows it. The perf harness times
saveDraftToProjectin place ofsaveAnalysisand still records the request assave-bytes.Merge Danger
Door: two-way
No storage format changes;
markCleanwrites the existing envelope withdirty: false. The one irreversible path is New or Open discarding a draft book's unreadable shard, which only damaged data reaches.Blast Radius: Save
Save, Save As and autosave all change path. Replacing
saveAnalysiswith two commands and addingdiscardUnreadabletosaveDraftchanges the command surface 4a (#453) builds on. The repository interface grows, so #469 needs cases for it.🤖 Generated with Claude Code
This change is