refactor(spec): build-docs uses the shared generated-output sink#3145
Merged
Conversation
#3138 extracted the emit()/manageDir()/flush() sink to lib/generated-output.ts and moved build-skill-references + build-react-blocks-contract onto it, but left build-docs.ts carrying the inline original: #3126 was rewriting the same file at the time and the migration would have conflicted. #3126 has landed, so collect the debt. Two copies of a check/write sink is the one duplication this design cannot afford — its whole claim is that --check and write are the same code, and that claim is per copy. A fix to one (say the empty-dir prune #3138 added) silently leaves the other behind, and the failure mode is a gate that passes on output a real run would not produce. Adds wasEmitted() to the shared sink. build-docs is the first caller whose later output depends on its earlier output — each category index links only the pages that got generated — and that question belongs to the sink: it already owns the emitted map, and having the caller keep a second copy would fork the state the sink exists to consolidate. The other two callers are unaffected (new export, no signature change). The guard moves from an inline `managedDirs.size === 0` to flush()'s guard hook, keeping the same meaning: json-schema/ is gitignored, so a fresh checkout that skips gen:schema emits almost nothing and "nothing differs" would read as success. Pure refactor, verified rather than assumed: - regenerating produces byte-identical output — all 258 files, tree stays clean - all five drift classes still fail: stale content, missing page, stale leftover, no-schema guard, plus in-sync staying green - both existing sink callers regress clean, including #3138's reverse test that a hand-written .md in skills/*/references/ keeps the gate green and survives a write Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 102 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
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.
Collects the debt #3138 deliberately left behind.
#3138 extracted the
emit()/manageDir()/flush()sink tolib/generated-output.tsand movedbuild-skill-references+build-react-blocks-contractonto it — but leftbuild-docs.tscarrying the inline original, because #3126 was rewriting the same file at the time and the migration would have conflicted. #3126 has landed (a6058728d), so the conflict is gone.Why this duplication in particular is worth removing
Two copies of a check/write sink is the one duplication this design cannot afford. The sink's entire claim is that
--checkand write are the same code — and that claim holds per copy. A fix to one (say the empty-dir prune #3138 added) silently leaves the other behind, and the failure mode isn't cosmetic: it's a gate that passes on output a real run wouldn't produce. Exactly the false-green the gate exists to prevent.wasEmitted()added to the shared sinkbuild-docsis the first caller whose later output depends on its earlier output — each categoryindex.mdxlinks only the pages that were actually generated (a.zod.tswhose schemas are all unrepresentable in JSON Schema produces no page, and carding it would be a dangling 404).That question belongs to the sink: it already owns the
emittedmap, and having the caller keep a second copy would fork the very state the sink exists to consolidate. Under--checkit also must come from the sink — nothing is written, so the disk still holds the stale tree and reading it would make the check diverge from the run it models.New export, no signature change; the other two callers are untouched.
The guard moves from an inline
managedDirs.size === 0toflush()'sguardhook, same meaning:json-schema/is gitignored, so a fresh checkout that skipsgen:schemaemits almost nothing and "nothing differs" would read as success.Verified, not assumed
Net −58 lines. A refactor of gate machinery is exactly where "it's only a refactor" gets you a gate that no longer bites, so the full red-test suite was re-run rather than skipped:
~ … (out of date)+ … (missing — spec adds it)- … (stale — …)check:skill-refs/check:react-blocksregressionskills/stays clean.mdinskills/*/references/tsc --noEmitclean.🤖 Generated with Claude Code