Skip to content

test(deps): drop the duplicate DEPENDENCIES.md pin check - #386

Merged
oddharsh merged 1 commit into
mainfrom
chore/dedupe-dep-doc-check
Aug 14, 2026
Merged

oddharsh merged 1 commit into
mainfrom
chore/dedupe-dep-doc-check

Conversation

@oddharsh

Copy link
Copy Markdown
Owner

#382 and #384 landed hours apart, from different sessions, asserting the same invariant:

line test from
1882 DEPENDENCIES.md states the versions package.json actually pins #382
2729 docs/DEPENDENCIES.md agrees with the manifests #384

Two tests for one rule means two places to update and a false read on coverage. This removes the weaker of the pair, which is #382's.

Why #384's survives

It extracts the logic into scripts/lib/dependency-docs.mjs, so the checker is exercised directly rather than only through the file it happens to read. It ships a second test that runs the checker against synthetic drift, which automates the controls #382 could only be verified with by hand. And it catches a case #382 explicitly scoped out: a root dependency nobody documented. #382 only ever walked documented claims back to the manifest, never the reverse.

The removed test also carried a hand-maintained DOC_PINS table mapping prose labels to package names. That is exactly the kind of list that goes stale as pins come and go, and #384 derives the same mapping from the manifests instead.

Coverage was measured, not assumed

Running only the surviving test against each drift the removed one existed to catch:

control result
unmodified PASS
doc states a wrong version FAIL
doc claim deleted FAIL
package retired, doc stale FAIL
Pillow pin drifts FAIL
restored PASS

Pillow is the one worth calling out, since www/scripts/requirements.txt is a second manifest and was the likeliest gap. It is covered — dependency-docs.mjs reads it directly.

Then the same drift was re-run through the full suite after the deletion, to confirm nothing depended on the removed test being the one that failed. A bumped pin with an unbumped sentence still reddens the suite.

Verification

275 tests before, 274 after, so exactly one test left and nothing else moved. Lint and typecheck green.

#382 and #384 landed hours apart, from different sessions, asserting the same
invariant. Two tests for one rule means two places to update and a false read
on coverage, so this removes the weaker of the pair.

#384's is better on every axis. It extracts the logic into
scripts/lib/dependency-docs.mjs, so the checker is exercised directly rather
than only through the file it happens to read. It ships a SECOND test that runs
the checker against synthetic drift, which automates the controls #382 could
only be verified with by hand. And it catches a case #382 explicitly scoped
out: a root dependency nobody documented, where #382 only ever walked
documented claims back to the manifest.

The removed test also carried a hand-maintained DOC_PINS table mapping prose
labels to package names, which is exactly the kind of list that goes stale as
pins come and go. #384 derives the same mapping from the manifests.

Coverage was measured rather than assumed, running ONLY the surviving test
against each drift the removed one existed to catch:

  unmodified                  PASS
  doc states a wrong version  FAIL
  doc claim deleted           FAIL
  package retired, doc stale  FAIL
  Pillow pin drifts           FAIL
  restored                    PASS

Pillow is the one worth calling out, since requirements.txt is a second
manifest and was the likeliest gap. It is covered: dependency-docs.mjs reads
it directly.

Then the same drift was re-run through the FULL suite after the deletion, to
confirm nothing depended on the removed test being the one that failed:
275 tests before, 274 after, and a bumped-pin-with-unbumped-sentence still
reddens the suite. Lint and typecheck green.
@github-actions

Copy link
Copy Markdown
Contributor

Wire-size diff vs merge base

94cb4e9 → dfef6de

Brotli q11, the quality the edge serves. Every number here is deterministic:
an unchanged file produces no row. Advisory — this check fails on nothing.

Client assets

No change. 20 files, 73.98 KiB Brotli total.

Pages

No change. 48 files, 496.66 KiB Brotli total.

Worker bundle

No change. 284.68 KiB gzip.

Dictionary deltas (pd/)

No change. 144 deltas, 458.08 KiB total.

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
aadhar-sh 58ff232 Aug 14 2026, 10:00 PM

@oddharsh
oddharsh merged commit 1e2f5d9 into main Aug 14, 2026
8 checks passed
@oddharsh
oddharsh deleted the chore/dedupe-dep-doc-check branch August 14, 2026 22:02
oddharsh added a commit that referenced this pull request Aug 14, 2026
…ck (#387)

#382 and #384 landed two dependency-doc drift checks the same day, hours apart.
#386 was right that one had to go and wrong about what to keep: it touched only
scripts/contract-tests.mjs, so the THREE rules #382 had that the module did not
went out with the test. Two of them were real protections, and main has had
neither since.

  1. RANGE PINS. The module strips a leading caret and compares, so `^1.2.3`
     agrees with a doc claiming `1.2.3` — a range the prose cannot honestly
     state. #382 refused it outright and said to move the package to the
     versionless list with its reason. Restored, and it now says exactly that.
  2. BASELINE SCOPING. #382 scanned only the `## Current baseline` section; the
     module scanned the whole file, so a version mentioned anywhere in the intro
     prose counted as a claim. Restored, including the case #382 guarded with a
     length assertion: indexOf returning -1 makes slice(-1) one character, which
     scans clean and reports a pass over nothing. A renamed heading is now a
     named failure instead.
  3. A DECLARED PACKAGE LOSING ITS SENTENCE. This one the module already covered
     from the other side, since the reverse direction fails any root dependency
     with no claim. It is now also reported by name, because "wrangler is a root
     dependency and the doc does not state its version" is a worse message than
     "the doc no longer states a version for Wrangler".

Nothing from #384 is lost. The reverse direction over the whole manifest stays,
and it is the half a fixed alias table structurally cannot have: #382's table
held seven rows and @noble/post-quantum was not one of them, so that check could
never have found the site's only shipped runtime dependency undocumented.

The floor moves 5 -> 8, one per alias plus Pillow. It sits AT today's count
rather than under it now that every alias is separately required to appear, so
it can only fall by the scanner breaking.

Verified as a matrix rather than asserted: the collapsed check catches all three
rules only #382 had, the one only #384 had, and the one both had. 275/275
contract tests, lint 0, typecheck 0, build 0.

Test policy is injectable (aliases, exemptions, floor, heading), so each negative
case runs against a two-line fixture instead of reconstructing the whole manifest
to keep the other rules quiet. Adding the alias rule broke that isolation for the
older fixtures, which is exactly the failure the injectable policy exists to make
cheap to fix.

Co-authored-by: Aadharsh Pannirselvam <19518661+oddharsh@users.noreply.github.com>
oddharsh added a commit that referenced this pull request Aug 14, 2026
…rules (#389)

#388 documented lens-reader, zenc's Cargo pins, cal and cf-garage, and
deliberately stated no versions for them: the checker read the root
package.json and requirements.txt only, so a number written there would have
been an unchecked copy. This is the follow-up that note named, so those numbers
can now be written.

The root's four loops are extracted into auditManifest() and called once per
manifest rather than reimplemented, which is the trap #386 had just finished
cleaning up after: two checks over one file, the weaker one reading as coverage
that was not there. The root's message strings are preserved byte-for-byte,
because the negative tests assert on them and a refactor that quietly reworded
them would assert less while still passing. All three pre-existing tests pass
unchanged.

Four of the nine sub-manifest dependencies are exempted rather than stated, and
the split is the useful half: cal and cf-garage caret-range everything, and
Cargo reads a bare "0.25" as a caret range. A range is a version the prose
cannot honestly state.

THREE REAL DEFECTS FOUND WHILE BUILDING IT, none by me:

- serde_json arrived in zenc with the histogram bake in #373 and nothing
  documented it. The reverse direction caught it on its first run over that
  manifest, the second time that direction has found a genuine gap.
- The claim pattern required a bare `name 1.2.3`, but this doc writes package
  names as `code`. Every sub-manifest entry read as undocumented while the
  sentences stating them sat in the file. The pattern takes an optional
  backtick now, and removing it again fails both the new test and the live one.
- oxlint caught `\Z` in the Cargo section regex. JavaScript has no \Z, so that
  arm matched a literal "Z" and the table could only be found when another
  section followed it. It passed against the real file purely because
  [profile.release] sits after [dependencies], and would have returned an empty
  set, silently, for a manifest whose dependencies came last. Fixed, and pinned
  by a test for exactly that shape.

Also fixes an ordering bug of my own: the floor was judged before sub-manifest
claims were gathered, so 8 root claims were measured against a floor of 12.
Claims are collected across all five manifests first, since the floor is a
scanner-broke tripwire and has to see the whole scan.

A manifest that cannot be READ is reported rather than audited as {}, because
an empty pin set scans clean and asserts nothing, which is how a moved file
would read as a healthy project forever.

Co-authored-by: Aadharsh Pannirselvam <19518661+oddharsh@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant