test: pin what the compiler cannot see, not how it is spelled - #418
Merged
Conversation
The suite grew fast, and 674 of its assertions read source text rather
than run code. Some of those are the only tool available - `invoke`
passes a command name as a plain string, "this behaviour has one
implementation" is not expressible in types, and Svelte components
cannot be imported under node. But a large share had drifted down a
layer, pinning private identifiers, parameter names, a statement's
literal punctuation, and in one case a `finally` block matched down to
three tabs of indentation. Those make a local rename a test failure,
which is friction, not safety.
Nothing was deleted for being redundant without first showing what else
catches the same regression, and the result is measured two ways rather
than asserted:
- nine pure private renames, no behaviour change: 5 tests red before,
0 after;
- sixteen injected regressions: 15 of 16 caught before, 16 of 16 after.
The one the old suite missed is a front-matter bypass: the raw
`invoke('render_markdown')` moving out of `renderMarkdownPreview` while
the file still contained exactly one occurrence, within the 400-character
proximity window the assertion allowed. Naming the enclosing function
instead of counting occurrences catches it.
Markers moved to the layer that is actually the contract: an exported
symbol, a cross-language magic string the compiler cannot check
(`return 'plaintext'`, `--highlight-color:`), or the literal shape of a
fixed defect. Three byte-identical copies of a directory walker and a
duplicated DOMPurify allowlist collapse into `scripts/sourceTree.ts`,
which carries the rule in a comment.
Kept deliberately: the prefix-forgery, CRLF and path-case tests, which
reproduce defects that really happened; the two rules pinning a defect's
literal shape; and two brittle files that are the only coverage of a
stale async render and a listener leaking past destroy - flagged rather
than removed, since nothing else catches those.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PathGao
pushed a commit
that referenced
this pull request
Aug 3, 2026
Measured against injected defects, these 14 files detect that a specific line was edited and nothing else. Each one stayed green while a real defect in its own stated subject was live; four of them are the only file that greps the line they pin, so the catch they do provide is the rename detector #418 rules out, not coverage. Four assertions inside them were the exception — they pin something no compiler can see (a Tauri command name, a locale's own dictionary entry, a CSS duration, an `async fn` whose absence only deadlocks on Windows). Those move into surviving files rather than being lost: findCollapsedMatches FOLD_TRANSITION_MS vs the styles.css transition -> foldLayout.test.ts toolbarCustomization two per-locale translation tests (they import and run the dictionary) -> i18nCoverage.test.ts windowsPdfExport invoke() name <-> generate_handler! registration -> macosPdfExport.test.ts tabContextMenuIsolation create_transfer_window must stay `async` -> windowOrganization.test.ts Five files from the same list are kept: each was measured to catch a real defect that nothing else catches. See the pull request for the per-file table. 562 -> 524 tests; 199 assertions removed, 5 tests added back by the salvage. Every mutation the suite caught before it still catches, including the three controls (sanitizer `<style>`, lossy-decode save guard, checkbox toggle line). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PathGao
pushed a commit
that referenced
this pull request
Aug 3, 2026
Measured against injected defects, these 14 files detect that a specific line was edited and nothing else. Each one stayed green while a real defect in its own stated subject was live; four of them are the only file that greps the line they pin, so the catch they do provide is the rename detector #418 rules out, not coverage. Four assertions inside them were the exception — they pin something no compiler can see (a Tauri command name, a locale's own dictionary entry, a CSS duration, an `async fn` whose absence only deadlocks on Windows). Those move into surviving files rather than being lost: findCollapsedMatches FOLD_TRANSITION_MS vs the styles.css transition -> foldLayout.test.ts toolbarCustomization two per-locale translation tests (they import and run the dictionary) -> i18nCoverage.test.ts windowsPdfExport invoke() name <-> generate_handler! registration -> macosPdfExport.test.ts tabContextMenuIsolation create_transfer_window must stay `async` -> windowOrganization.test.ts Five files from the same list are kept: each was measured to catch a real defect that nothing else catches. See the pull request for the per-file table. 562 -> 524 tests; 199 assertions removed, 5 tests added back by the salvage. Every mutation the suite caught before it still catches, including the three controls (sanitizer `<style>`, lossy-decode save guard, checkbox toggle line). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PathGao
added a commit
that referenced
this pull request
Aug 3, 2026
Measured against injected defects, these 14 files detect that a specific line was edited and nothing else. Each one stayed green while a real defect in its own stated subject was live; four of them are the only file that greps the line they pin, so the catch they do provide is the rename detector #418 rules out, not coverage. Four assertions inside them were the exception — they pin something no compiler can see (a Tauri command name, a locale's own dictionary entry, a CSS duration, an `async fn` whose absence only deadlocks on Windows). Those move into surviving files rather than being lost: findCollapsedMatches FOLD_TRANSITION_MS vs the styles.css transition -> foldLayout.test.ts toolbarCustomization two per-locale translation tests (they import and run the dictionary) -> i18nCoverage.test.ts windowsPdfExport invoke() name <-> generate_handler! registration -> macosPdfExport.test.ts tabContextMenuIsolation create_transfer_window must stay `async` -> windowOrganization.test.ts Five files from the same list are kept: each was measured to catch a real defect that nothing else catches. See the pull request for the per-file table. 562 -> 524 tests; 199 assertions removed, 5 tests added back by the salvage. Every mutation the suite caught before it still catches, including the three controls (sanitizer `<style>`, lossy-decode save guard, checkbox toggle line). Co-authored-by: PathGao <gaoyanbo@gaoyanbodeMacBook-Air.local> 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.
No production code changes.
git status src src-tauriis empty.The problem, measured
The suite grew quickly, and 674 of its assertions read source text rather than run code. Some of those have no alternative:
invokepasses a command name as a plain string that no compiler checks, "this behaviour has one implementation" is not expressible in types, and Svelte components cannot be imported under node (.svelte.tsstores throw$state is not definedundertsx— verified).But a large share had drifted a layer too low — pinning private identifiers, parameter names, a statement's literal punctuation, and in one case a
finallyblock matched down to three tabs of indentation. A local rename then fails a test, which is friction rather than safety.The count barely moved: 674 → 668. This is a composition change, not a trim.
Result, measured two ways
function walk(dir)inscripts/The one the old suite missed
invoke('render_markdown')moving out ofrenderMarkdownPreview— bypassing front-matter stripping — while the file still contained exactly one occurrence, ~120 characters after the wrapper's declaration. The old assertion wascount === 1plus a[\s\S]{0,400}proximity window, and it stayed green. Naming the enclosing function of every raw call catches it, with no count and no window.Where the markers moved to
/["']dark["']:["']neutral["']/— the ternary's literal spellingresolveMermaidTheme\(if/elsefunction renderRichContent\(\s*options— the parameter nameRenderRichContentOptionsfunction getLanguage\(— a private 25-line helperreturn 'plaintext'const highlightColorMap— a private binding--highlight-color:styles.cssandFindBar.svelteindexOf('return processMarkdownHtml(html, filePath, collapsedHeaders);')(?:fn|filename)local names +emitted.length === 3[[…]]template must contain#let sanitizedHtml = $derived(sanitizeMarkdownHtml(htmlContent)){@html ident}is that oneThree assertions got stronger, not merely looser. New
scripts/sourceTree.tsholds the ex-duplicate walker, the ex-duplicate allowlist, andenclosingFunctionName/callSiteOffsets— the replacements for proximity windows and verbatim-statement matching. It carries the rule in a comment: a marker may be an exported symbol, a cross-language or library magic string, or the literal shape of a fixed defect — never a private identifier.Kept deliberately
asset.localhost.evil.testprefix forgery, CRLF handling, path case / NFC-NFD. Untouched.getScrollHeight() - …height(Exclude editor bottom padding from split scroll sync #316) andcreateElement('iframe')(refactor(preview): drop the dead YouTube iframe copy from the viewer #388), bothallowed: []. The criterion protects exactly this.previewRenderRevision.test.tsandviewerDisposal.test.tspin private locals and are brittle by the criterion — but they are the only coverage of a stale async render overwriting a newer one, and of listeners leaking past component destroy. The logic lives inside.svelte, and no name-free formulation still pins the check. Per "nothing else catches it → do not delete": kept, and flagged here as known-brittle.updateStoredRecentFiles(×3,discardUnsavedBuffer: true×2). These close over an enumerated list — "these three, shown above, and no others" — which is the legitimate single-implementation shape, unlikeemitted.length === 3, which enumerated nothing.Verification of the verification
Each of the 18 changes lists which mutation proves the replacement still fires; every mutation was reverted with
git checkout. Independently re-run here: a legal rename (npm run check0 errors, proving it compiles) leaves 501/501 green, and breakingmermaid.renderplus planting a duplicate theme ternary inexport.tsturns 2 tests red.Not covered
previewSanitize.test.tsstill records a browser-measured result in a comment.return 'plaintext',--highlight-color:). Strictly better than the private names they replace, but grep-based single-implementation rules are heuristics, not proofs.lossyDecodeSaveGuard.test.tshas the same smells but is being rewritten in fix(tabs): ask the filesystem whether two paths name the same file #416; left alone to avoid a conflict..sveltelogic importable — extractinggetLanguageand friends intosrc/lib/utils/— would be the real fix for a whole tier of these tests. That is production surgery and out of scope here.🤖 Generated with Claude Code