test(save): cover the branch where the partial-buffer guard refuses - #445
Merged
Conversation
`truncatedBufferGuard.test.ts` exists to prove a partially loaded buffer is never written to disk, but every test in it drove the guard's *success* path. Deleting the verdict check from `toggleTaskCheckbox` — so it edits and saves whatever `ensureFullContent` leaves behind — left all 565 tests green. Six tests, all running the real `documentSession` against the stubbed backend, put a writer in each state where the buffer stays partial: the tab is gone, the buffer already carries edits, the re-read failed. Each asserts the same pair — nothing reached `save_file_content`, and the buffer was left exactly as it was found — across `saveContent`, `saveContentAs`, `toggleTaskCheckbox` and the close dialog's "Save". Eight defects injected on top, eight caught. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
scripts/truncatedBufferGuard.test.tsexists for one claim: a buffer that holds only the first 50KB of a large file is never written back over that file. Every test in it drove the guard's success path — open a partial buffer, complete it, watch the whole document reach disk.Deleting the verdict check from
toggleTaskCheckbox, so it edits and saves whateverensureFullContentleaves behind:565 tests, 565 passing. The file whose entire purpose is that guard had no test for the guard refusing.
What
falseactually meansensureFullContentre-reads the file behind a partial buffer and returns whether the buffer is whole afterwards. Three states producefalse, and they are not variations of one thing:handleDetach/moveTabToWindowhold an id; the tab can close while the call is in flightonErrorThe second state is a trap and not merely a refusal: once a partial buffer is dirty it can never be completed again, so a writer that edits it first has permanently disarmed the recovery path. That is why the tests assert the buffer is left byte-identical, not merely that no write happened.
Every call site checks the result. All six —
toggleTaskCheckboxindocumentSession, andhandleFrontMatterEdit,handleFrontMatterListChange,toggleSplitView,handleDetach,moveTabToWindowinMarkdownViewer.svelte— stop onfalse; the five in the component also raisetoast.partialDocument. No live bug, a coverage gap.saveContentandsaveContentAsdo not consult it. They carry the same refusal as their ownisTruncatedbackstop, which is the last line of defence if an entry point is ever missed, so they are driven into it here too.Tests
Six added to
truncatedBufferGuard.test.ts, all calling into the realdocumentSessionand the realTabManagerover the stubbed Tauri bridge that file already sets up. No new source-text assertions — the existing three at the bottom of the file, for wiringnode --testcannot import from a.sveltefile, are untouched.false, does not half-fill the buffer, keepsisTruncated, and says so oncefalsereturn is the whole signal, and the typing survivescanCloseTabmust stayfalseand the tab must stay openThe close-dialog test needed the harness's
askCloseto be settable; it still answersdiscardby default, so no existing test changed behaviour.Both recent changes to this file are load-bearing here and neither is fought. #438 serialises saves per tab: the truncated refusal returns before
writeExclusively, so it never enters the queue and a later save on the same tab still writes —completing a partial buffer … unblocks saving, already in the file, is what covers that ordering. #439 madeisLossySaveRefusedask the tab as well as the memory: a partial-buffer refusal never toucheslossySaveWarnedTabsand never reads as a lossy refusal, so the auto-save timer stays free to report a real failure on the same tab.Mutation check
Each defect injected into
src/lib/sessions/documentSession.svelte.ts,scripts/truncatedBufferGuard.test.tsrun, the source restored. 17 tests in the file.toggleTaskCheckboxignores the verdict (the audit's defect)saveContentAsdrops itsisTruncatedguardsaveContentdrops itsisTruncatedguardensureFullContenttreats a failed re-read as successensureFullContentreports the failed re-read to nobodyensureFullContentcompletes a dirty partial buffer anywayensureFullContentanswerstruefor a tab that is gonecanCloseTabcloses on a refused save8 injected, 8 caught. Rows 3 and 6 are caught first by tests that were already there — those two are not vacuous, they were simply the only refusals anything reached.
Messages name the behaviour, not the line:
npm test565 → 571 passing,npm run check637 files / 0 errors,npm run buildclean.Not covered
MarkdownViewer.svelte—handleFrontMatterEdit,handleFrontMatterListChange,toggleSplitView,handleDetach,moveTabToWindow. Each checks the verdict and raisestoast.partialDocument, and each is still asserted only as source text, becausenode --testcannot import a.sveltefile (test(scroll-sync): cover the split-view mapping by running it #442's finding: 21 components, no executable coverage). The grep tests proveensureFullContentis mentioned in those bodies, not that the early return fires. Closing this is the same extraction test(scroll-sync): cover the split-view mapping by running it #442 did for the scroll-sync math, and it is a bigger lift here: these handlers touch the DOM and component state, so it is a behaviour-moving change rather than a mechanical one.falsethemselves — so the user does see something. It is still a gap between "the user was told" and "the UI happened to react", and it is why the checkbox tests assert the return value rather than a message.markTabContentUnavailable's tab.isTruncatedalso means "the file could not be read at all", set by the failed-load path with an empty buffer. Every writer refuses it for the same reason, and nothing here distinguishes the two producers of the flag;documentLoadFailure.test.tsowns that side.oneWritePerTabInFlight.test.ts, and the refusals here are all reached with nothing in flight. A refusal arriving while a save for the same tab is draining is a state neither file drives.saveContent/saveContentAsspecifically. If someone moves theisTruncatedcheck intowriteExclusively, these tests still pass — they assert the refusal, not where it lives.🤖 Generated with Claude Code