[sksizer]

Summary

• Copies the validation corpus from the tag @osdlc/markdown-contract@0.3.0 in osdlc/osdlc (commit 6adfa400a8dfda570d61c121e76d29bce58fa788, path packages/ts/markdown-contract/corpus, 310 files) to crates/markdown-contract-engine/tests/corpus/.
• Records the source tag, commit and a checksum in crates/markdown-contract-engine/tests/CORPUS-SOURCE.
• Points fixtures_dir() in tests/corpus.rs at the pinned copy, and updates its doc comment.
• Adds scripts/check-corpus-pin.sh, which fails when the pinned copy no longer matches the recorded checksum (--update rewrites it).
• Adds .github/workflows/rust.yml, which runs the pin check and cargo test -p markdown-contract-engine.
• Sets the engine crate version to 0.3.0 (and its Cargo.lock entry).

Locally, cargo test -p markdown-contract-engine passes: 172 unit tests and the corpus test, with no corpus edits. The packages/core fixtures are untouched.

[sksizer]

Review record
Actor	@sksizer
Agent	Claude
Model	claude-opus-5-5
Effort	unknown
Skill	review-pr@0.5.0
Session	Mac.local:claude-r7p2kq
Host	Mac.local
Started	2026-10-19T13:31:50Z
Repository	sksizer/markdown-contract
Pull request	#271
Review	full
Head	976459a91bf1e1514762f1553f22c1ce4b7b829c
Base	cb16c7868981c6806755a4a8e907ac0d097b5f4a
Mode	auto

Verdict: changes needed before merge. The copy is correct. I checked that tests/corpus/ is byte-identical to packages/ts/markdown-contract/corpus at osdlc 6adfa400 (the @osdlc/markdown-contract@0.3.0 tag), that it holds 310 files, that check-corpus-pin.sh passes, and that CI ran 172 unit tests plus the corpus test. Two problems remain. The CI guard does not check what it says it checks. And the change breaks D-0018 §D3's "one corpus" rule without amending the decision.

Code

• .github/workflows/rust.yml:17 (with scripts/check-corpus-pin.sh:11-24): the pin check compares against a checksum the script writes itself, not against the osdlc tag. Editing the corpus and running --update passes CI. Fix: clone the tag sparsely and diff -r. See the inline comment.
• Missing coverage: none beyond the above. The changed behaviour (where fixtures_dir() points) is covered by the corpus test itself.

Architecture

• crates/markdown-contract-engine/tests/corpus.rs:4: the Rust harness now reads the osdlc copy, while packages/core still runs its own tests/fixtures/validation/ copy. The two already differ in comments and in two .md files. D-0018 §D3 (docs/planning/decisions/D-0018-desktop-app-and-rust-engine.md:42-46, 149-158) still says both harnesses assert against the same files. Fix: amend D-0018 §D3 and keep one copy. See the inline comment.
• Note: the brief asked me to check docs/LIBRARIES.md, docs/PRINCIPLES.md and D-0001 project structure. This repo has neither file, and its D-0001 is the finding model. I judged placement against D-0018 and CONVENTIONS.md instead. A standalone scripts/ check run from a dedicated workflow fits the side-gate pattern in DEVELOPMENT.md:86-93.

Comments

These lines are outside the diff, so they are listed here. This change made each one wrong:

• README.md:134-135: "the shared-corpus harness over packages/core/tests/fixtures/validation goldens". Fix: "over the corpus pinned in crates/markdown-contract-engine/tests/corpus (see tests/CORPUS-SOURCE)".
• crates/markdown-contract-engine/src/lib.rs:3-4: "pinned by the shared corpus harness over packages/core/tests/fixtures/validation". Fix: "pinned by the shared corpus harness (tests/corpus.rs)", and leave the location to corpus.rs's own doc so it lives in one place.
• packages/core/tests/corpus-manifest.test.ts:10-14: "The corpus manifest ... is the entry point the Rust corpus harness walks (D-0018 §D3)". The Rust harness no longer reads this manifest. Fix: drop the Rust clause, or point it at whichever copy is canonical once the Architecture finding is settled.

The new comments themselves (corpus.rs:3-4, the CORPUS-SOURCE header, the script header) are fine. rust.yml:15 is covered by the Code finding.

Simplicity

• scripts/check-corpus-pin.sh:11-16 and CORPUS-SOURCE:9: the checksum and --update are a weaker stand-in for diffing against the source, which is public. The fix for the Code finding removes both. No other findings.

Anything else

• crates/markdown-contract-engine/Cargo.toml:10: the bump to 0.3.0 implies a link to the corpus tag that is not written down anywhere. See the inline comment.
• The failing audit check is unrelated: 48 npm advisories (nuxt, devalue, browserslist, tiptap), and this PR changes no npm dependency.

[sksizer]

Code — the pin check does not check what this step's comment (line 15) and the script's error text (scripts/check-corpus-pin.sh:24) say it checks.

check-corpus-pin.sh compares the corpus against the sha256= line in CORPUS-SOURCE, and --update writes that line from the local copy. So the check proves only that the copy has not changed since someone last ran --update. It never compares against @osdlc/markdown-contract@0.3.0.

Failing sequence: someone edits a golden in tests/corpus/ so a Rust test passes, then runs scripts/check-corpus-pin.sh --update. CI goes green, and CORPUS-SOURCE still says tag=@osdlc/markdown-contract@0.3.0. Bumping tag=/commit= without copying the new files passes the same way.

(I checked by hand that the copy at this head is byte-identical to packages/ts/markdown-contract/corpus at 6adfa400. The finding is about what CI enforces after this PR, not about today's copy.)

Fix: osdlc/osdlc is public, so compare against the source itself and drop the checksum:

src="$(mktemp -d)"
git clone --quiet --depth 1 --branch "$tag" --filter=blob:none --sparse "$repo" "$src"
git -C "$src" sparse-checkout set "$path"
test "$(git -C "$src" rev-parse HEAD)" = "$commit"
diff -r "$src/$path" "$dir"

That also removes sha256= and --update (see Simplicity in the summary). If CI must stay offline, at least reword line 15 and the error message to "unchanged since CORPUS-SOURCE was last updated", so they don't claim a check that never runs.

_{🤖 Claude · claude-opus-5-5 · review-pr@0.5.0}

[sksizer]

Fixed in aa82cc7. The script now sparse-clones the tag recorded in CORPUS-SOURCE, checks that it points at the recorded commit, and runs diff -r against the pinned copy. The checksum and --update are gone, and the comments and error text say what is checked. I checked locally that an edited file fails it.

[sksizer]

Architecture — the Rust harness and the in-repo TS harness no longer read the same corpus, and D-0018 §D3 still says they do.

D-0018 §D3 (docs/planning/decisions/D-0018-desktop-app-and-rust-engine.md:149-158) and the bullet at lines 42-46 say "Both harnesses assert against the same files" and "the Rust corpus harness walks the same directory". After this change:

• packages/core (npm markdown-contract 0.1.0, still the in-repo TS engine) runs packages/core/tests/fixtures/validation/ through tests/harness.ts and tests/corpus-manifest.test.ts.
• The Rust engine runs crates/markdown-contract-engine/tests/corpus/, copied from osdlc.

The two copies already differ: every .contract.yaml has a different provenance comment, and 20a-real-task-closed-without-completion-note.{pass,fail}.md have different URLs. The goldens match today, but nothing keeps them matched. A fixture added or re-pinned in packages/core is no longer checked against the Rust engine. That parity was the purpose of the gate this file's line 1 still names.

Fix (pick one, and say which in D-0018 §D3):

1. If @osdlc/markdown-contract is now the canonical TS engine, amend D-0018 §D3 to say the shared corpus is the one published with it. Then either point packages/core's harness at crates/markdown-contract-engine/tests/corpus/ or retire the packages/core copy, so only one copy is left.
2. If packages/core stays canonical, keep the Rust harness on packages/core/tests/fixtures/validation. Add a check that its manifest, .md files and .expected.json files match the osdlc tag, instead of keeping a second copy.

_{🤖 Claude · claude-opus-5-5 · review-pr@0.5.0}

[sksizer]

Fixed in aa82cc7. Decision: @osdlc/markdown-contract in osdlc is the canonical TS engine and its corpus/ is the canonical corpus. D-0018 §D3 and the parity bullet are amended in place. packages/core is superseded, and its fixtures stay only until it is retired. Its tests are unchanged.

[sksizer]

Anything else — the version bump to 0.3.0 has no recorded reason.

The PR sets the crate to 0.3.0, the same number as the corpus tag, but nothing records that link. CORPUS-SOURCE's update steps (lines 2-4) don't say to bump the crate version with the tag, and nothing reads the crate version (CARGO_PKG_VERSION is unused; the desktop crate depends by path). The next tag update will leave the two numbers apart without anyone noticing.

Fix: if the version means "finding parity with @osdlc/markdown-contract X.Y.Z", say so in this file's header comment and add "set the crate version to the tag's version" to the update steps in CORPUS-SOURCE. If it means nothing, leave it at 0.1.0.

_{🤖 Claude · claude-opus-5-5 · review-pr@0.5.0}

[sksizer]

Fixed in aa82cc7. A comment in Cargo.toml and the update steps in CORPUS-SOURCE now say the crate version follows the corpus tag.

[cloudflare-workers-and-pages]

Deploying markdown-contract-docs with    Cloudflare Pages

Latest commit:	aa82cc7
Status:	✅  Deploy successful!
Preview URL:	https://535c442d.markdown-contract-docs.pages.dev
Branch Preview URL:	https://corpus-pin-0-3-0.markdown-contract-docs.pages.dev

View logs

[sksizer]

Fixed in aa82cc7.

• r1-4, r1-5, r1-6: the stale corpus paths and comments are corrected in README.md, crates/markdown-contract-engine/src/lib.rs and packages/core/tests/corpus-manifest.test.ts (the Rust clause is dropped there).
• r1-2 decision: @osdlc/markdown-contract is the canonical TypeScript engine and its corpus/ is the canonical corpus. D-0018 §D3 now says the Rust harness reads a pinned copy that CI checks against the osdlc tag. packages/core is superseded; its tests/fixtures/validation/ stay until it is retired and are not changed.
• cargo test -p markdown-contract-engine and the pin check pass locally.
• The failing audit check is unrelated: it reports npm advisories, and this PR changes no npm dependency. I left it alone.