Sidecar-strip hardening (#442) + fix s0 suite crash from Comments v1 (bd-ddaqjb91) - #443
Merged
Merged
Conversation
…eEdit (#442) Extract the commitSubtreeEdit stripping replacer into a shared stripSourceInfoFields util and extend it to remove the two remaining bare-pool-index sidecar keys: captionS (Table/Figure captions) and citationIdS (Cite citation objects). Subtrees cloned from the untransformed AST carry pool indices that apply_node_edit cannot resolve (the replacement doc has no pool), so any surviving sidecar fails the whole edit with InvalidSourceInfoRef — the same failure PR #441 fixed for targetS on links. The test file now imports the real util instead of re-declaring a copy, and gains coverage for targetS (Link/Image), captionS (Figure), and citationIdS (Cite). Table-internal sidecars (headS/footS/bodiesS/ rowsS/cellsS/bodyS) are left as-is: their pool-carrying leaves are s/a keys the recursive replacer already removes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tests) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… s0 stub (bd-ddaqjb91) The Comments v1 merge (a6fc44b) made CommentBlock the registry's Block wrapper, so every block render now consults resolveSource and passes resolved.sourceNode to sameCommentableKind without checking it exists. The s0-list-item-surfaces harness's resolveSource stub predates the current ResolvedSource shape (no sourceNode, invalid 'Reachable' class, bogus sourceIndex prop), so 18/23 tests crashed with "TypeError: Cannot read properties of undefined (reading 't')". Two fixes, TDD'd: - CommentBlock treats a resolved entry without sourceNode like an unresolvable block (passthrough render), in both the chrome gate and resolveCommittable. New defensive test proves passthrough for the malformed shape and that the guard is not over-broad. - The s0 stub now satisfies ResolvedSource for real (sourceNode: node, reachabilityClass 'Descendable', typed sourceEntry). s0: 23/23 pass. Production was likely unaffected: the real resolveSource in PreviewRoot.tsx always supplies sourceNode. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… (bd-ddaqjb91)
The s0 breakage was invisible because nothing type-checks test files:
the package tsconfigs exclude *.test.* and vitest transforms with
esbuild. Close the gap:
- tsconfig.tests.json in both packages (extends base, noEmit;
noUnusedLocals/noUnusedParameters off — lint-grade, not drift
detection), exposed as `npm run typecheck:tests`.
- cargo xtask verify step 11 runs both typechecks before the suites.
- Fix the pre-existing type errors the new gate surfaced:
- preview-renderer (6 test files): typed vi.fn generics, Mock import,
missing setLocalAst props, a useContext type typo, tuple-typed pool
fixtures, two documented narrow casts.
- preview-runtime: MockSyncClient's interface was missing
applyEditorOperations (the impl had it); typed the handler mocks;
cast once at the _setClientForTesting test seam.
Reintroducing the stale s0 stub now fails the typecheck
(TS2322: '"Reachable"' is not assignable to 'ReachabilityClass').
Also updates the plan doc checkboxes
(claude-notes/plans/2026-07-30-commentblock-defensive-resolvesource.md).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
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
2 participants
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.
Two related pieces of post-Comments-v1 hardening.
1. Strip
captionS/citationIdSincommitSubtreeEdit(closes #442's immediate ask)commitSubtreeEditsends subtrees cloned from the untransformed AST to Rust'sapply_node_edit, whose replacement doc carries no SourceInfo pool — any surviving pool-index sidecar fails the edit withInvalidSourceInfoRef(the bug #441 fixed fortargetSon links). Two bare-index sidecars were still unstripped:captionS(Table/Figure captions)citationIdS(Cite citation objects)The replacer is now a shared
stripSourceInfoFieldsutil that the test file imports (it previously tested a hand-copied duplicate), with new coverage fortargetS,captionS, andcitationIdS. Table-internal sidecars (headS/rowsS/…) are deferred with table editing — tracked in bd-d01m11aw, along with the systemic Rust-side option (tolerant completing-mode reader) and the sidecar schema doc #442 asks for.2. Fix the s0-list-item-surfaces crash on main (bd-ddaqjb91)
ts-packages/preview-renderer's integration suite was failing 18/23 ins0-list-item-surfacessince the Comments v1 merge: CommentBlock became the registry'sBlockwrapper and dereferencesresolved.sourceNodeon every block render, and the s0 harness'sresolveSourcestub predated the currentResolvedSourceshape (nosourceNode, invalid'Reachable'class). Production was likely unaffected — the realresolveSourcealways suppliessourceNode.resolveCommittable. New test proves the passthrough and that the guard isn't over-broad.ResolvedSourcefor real. 23/23 pass.tsconfig.tests.json+npm run typecheck:testsin preview-renderer and preview-runtime, wired intocargo xtask verifystep 11). The gate provably catches the original defect, and already surfaced real drift in preview-runtime (MockSyncClientmissingapplyEditorOperationsfrom its interface) plus type errors in 6 other test files, all fixed here.Plan/diagnosis:
claude-notes/plans/2026-07-30-commentblock-defensive-resolvesource.mdVerification
typecheck:testscleancd hub-client && npm run build:all✓cargo xtask verify --skip-rust-tests✓ (14/14 steps, including the new typecheck legs)🤖 Generated with Claude Code