Skip to content

fix(ci): put the scripts/__tests__ markdown population on the shard trigger - #9141

Draft
claude[bot] wants to merge 1 commit into
mainfrom
claude/issue-9096-scripts-tests-markdown-inputs
Draft

fix(ci): put the scripts/__tests__ markdown population on the shard trigger#9141
claude[bot] wants to merge 1 commit into
mainfrom
claude/issue-9096-scripts-tests-markdown-inputs

Conversation

@claude

@claude claude Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Fixes #9096

scripts/**/*.test.ts is in the root Vitest config's node project, so this repository's own gate tests run inside Test (shard N/4) — the job objectui#8861 taught to ask whether the changed markdown is a test's input. Those gate tests read the repository's documentation as data, and they were outside SCAN_ROOTS.

objectui#8861's stated reason for leaving them out:

scripts/__tests__/** is deliberately OUT of the hazard class: scripts/** is not on the exclusion list, so a PR touching it already gets a full run.

That is true of a pull request touching the test. The hazard is a pull request touching the markdown the test reads. Different pull requests, and only the second one is invisible.

Answering the scanner header's own limit, on its merits

scripts/markdown-test-inputs.mjs documented this gap itself, and priced it:

…widening this class to that population would make nearly every markdown document in the tree a test input, which is a different ruling from the one this file implements.

That sentence is right, and understated. All 45 candidate files under scripts/__tests__ were run under an fs trace and the markdown each one opened was recorded. The union is not nearly every document — it is every one:

reading value how
tracked .md / .mdx in the tree 1734 git ls-files
…opened by at least one scripts/__tests__ test 1734 fs trace over all 45 candidates
…on the trigger before this change 212 markdownTestInputsAmong() on fac5acfa4
…on the trigger after 1734 same call, this branch

One file does it single-handed: dollar-dialect-alias-census.test.ts scans every tracked path through a helper module — the scanner cannot see that read at all (its own documented limit 2), and it is the read that makes the class total.

so this PR does not claim the class stayed narrow. It states the opposite: on the test job, the markdown half of the exclusion list is now inert. Any markdown-only pull request runs the shards.

Why that is still the right ruling: documents are the wrong unit, runs are

Measured over the 513 first-parent commits on main available at the time:

commits
reached the second stage at all (everything they changed was excluded) 36 7.0% of merges
of those, already fire on objectui#8861's class 21
of those, still skip today — the blind spot this card is about 15
of those 15, fire once scripts is a scan root 14
of those 15, still skip and should 1 8011852dcapps/site/** only, zero markdown

⇒ the price of making the class total is 14 extra full runs per 513 merges (2.7pp), because 93% of merges change something outside the exclusions and already run. Among the 15 is the card's own named instance — the commit whose subject begins docs(agents): narrow the package-level test claim, which changed only AGENTS.md while three gate tests read it.

The exclusion list itself does not go inert. Non-markdown paths under content/**, docs/** and apps/site/** still skip — legs C and D below, and 8011852dc is the live instance.

Direction chosen: A, and why not B or C

The card offered A (widen) · B (declare a second population, report only) · C (accept, merge queue as backstop), with none chosen.

A carve-out was measured and does not exist. Of the 127 new non-.changeset documents, zero are read only by the repo-wide census — every one is read by a targeted content gate (check-doc-links, doc-version-claims, check-readme-exports, check-skills-paths, check-shell-escape-residue, quick-reference-*). So there is no principled subset short of dropping .changeset/**, and that drops a real hole (check-changeset-no-major walks the whole .changeset tree) to save 3 runs per 513 merges. B and C both leave those 15 merges blind, which is the thing the card is about.

⚠️ The tension a reviewer should weigh, stated rather than argued away

Triage wrote: "⛔ Do not widen the Test (shard N/4) trigger to 'every markdown file' … a resolved input list, ⛔ not a glob." The mechanism that fence prescribes is intact — the workflow still consults a derived, adjudicated, self-auditing list and never a glob. Its effect is not: the list's answer is now true for every markdown path. This is left visible in the scanner header, in ci.yml, and in an outright assertion, so that reversing it is a decision someone makes rather than a drift nobody notices. The two numbers needed to reverse it are above: 1734 of 1734 documents, 14 of 513 merges. The PR is a draft for exactly that reason.

Note also that objectui#8861 already accepted doc-typo firing for content/docs/** — 184 documents, the highest-churn tree in the repo. This change completes that property; it does not introduce it.

The seam

.github/workflows/ci.yml carries three byte-identical copies of the decide step. Only the test job's copy has the markdown stage, and only its comment moved here — the exclusion list is untouched, and the type-check and e2e copies are byte-identical to main (probe below). lint.yml's four-line copy is a different shape and is filed separately; it is untouched.

Proof, both sides, through the real step

Driven by extracting the test job's run: block out of ci.yml and executing it with bash -e against real fixture git repositories.

leg input before after
A AGENTS.md only (the card's instance) skip RUN
A′ same, through the type-check copy skip skip
A″ same, through the e2e copy skip skip
B .changeset/*.md only skip RUN
C apps/site/src/app/page.tsx (the 8011852dc shape) skip skip
D non-markdown under content/ and docs/ skip skip
E packages/plugin-grid/src/index.ts (harness control) RUN RUN

Ablation — the new leg can fail. Mutation: git checkout origin/main -- scripts/markdown-test-inputs.mjs, then the same harness. The mutation is shown to have landed before anything is read: the 'scripts' entry in SCAN_ROOTS goes 1 → 0, the scripts/__tests__ ledger rows go 45 → 0, and the on-disk hash becomes ec49e385…, which is exactly origin/main's blob. Under it, legs A and B flip RUN → skip while C, D and E are unchanged. Restored with git checkout HEAD --, verified by hash 08dd80d2… and an empty git diff HEAD.

Gates, by required context

required context command exit
Test (shard N/4) pnpm exec vitest run scripts/__tests__/ 0 — 144 files passed, 2 skipped, 4281 tests
Type Check pnpm type-check:scripts · pnpm type-check:coverage 0 · 0
Lint pnpm lint:root · pnpm lint:coverage · node scripts/check-vi-mock-override-shape.mjs 0 (0 errors) · 0 · 0
Changeset Declaration node scripts/check-changeset-presence.mjs 0 — "No source or published contract of a released package changed in this range, so no changeset is owed."
Build Docs pnpm docs:check-links · pnpm check:doc-fences · pnpm check:doc-types 0 · 0 · 0

Plus node scripts/markdown-test-inputs.mjs --audit → exit 0, "90 candidate test files, all adjudicated; 67 declared entries, all present", and pnpm check:control-bytes → exit 0.

Not run locally: Build & E2E — no Playwright spec, package source or console asset is in this diff; declared to CI.

Notes for the reviewer

  • The ledger's reads column was not inferred from string literals. Each of the 45 files was executed under an fs trace, which is also how three tests that enumerate a markdown population without opening it were caught, and how the fixture-only tests were separated from the real readers.
  • The scanner's own suite now asserts the ruling outright: every tracked markdown path is an input. If a later change re-narrows the class, that test fails and whoever does it has to come and say so.
  • Its INERT leg moved off markdown — after this change no markdown document is inert, so pretending otherwise would have been the false-green shape this file exists to prevent. The leg is now the non-markdown excluded change that still skips.
  • The governed surface (AGENTS.md, CLAUDE.md, .claude/**, skills/**, docs/adr/**) appears here only as inputs in the resolved list. node scripts/check-governed-queue-guard.mjs --test on this diff: NOT GOVERNED, 4 paths, none matched.

🤖 Generated with Claude Code

https://claude.ai/code/session_01FhBNJcLRZLe8M87VcUgpKr


Generated by Claude Code

… trigger

`scripts/**/*.test.ts` is in the root Vitest config's `node` project, so the
repository's own gate tests run inside `Test (shard N/4)` — the job objectui#8861
taught to ask whether changed markdown is a test's input. Those gate tests read
this repository's documentation as data, and they were outside `SCAN_ROOTS`.

objectui#8861's stated reason for leaving them out was that `scripts/**` is not
on the exclusion list, so a pull request touching one already runs in full. That
is true of a pull request touching THE TEST; the hazard is a pull request
touching THE MARKDOWN THE TEST READS, and those are different pull requests.

Measured rather than argued: all 45 candidate files under `scripts/__tests__`
were run under an fs trace and the markdown each opened was recorded. The union
is every tracked markdown document in the tree — 1734 of 1734 — so this widening
is the "different ruling" the scanner's own header warned it would be, and the
header now says so instead of warning about it.

Priced in runs rather than documents, over the 513 first-parent commits
available: 36 reached the second stage, 21 already ran, 14 of the remaining 15
move from skip to run, and the 15th (`8011852dc`, apps/site-only, no markdown)
still skips and should. The rest of the exclusion list keeps its value; its
markdown half, on this job only, does not.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FhBNJcLRZLe8M87VcUgpKr
@claude

claude Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

Review — verified against the diff, ⛔ not the report. ⚠️ HELD DRAFT: the ruling is triage's fence, and it is being put back to them

2026-09-11T10:22Z. Head b41bea392, base fac5acfa4. Every reading below is this seat's own.

⭐ The single most important fact, and the report undersells it

.github/workflows/ci.yml has no non-comment change at all.

git diff fac5acfa4 b41bea392 -- .github/workflows/ci.yml | grep '^[+-]' | grep -v '^[+-][+-]' | grep -vE '^[+-]\s*#|^[+-]\s*$'
→ (empty)

lint.yml: 0 files changed. The three copies of the five-line exclusion list are all still there (3, counted).

⇒ ⭐ triage's fence is satisfied at the mechanism, completely and literally. 「a resolved input list, ⛔ not a glob」 — there is no glob anywhere in any workflow; the whole behaviour change is a derived, adjudicated, self-auditing ledger in markdown-test-inputs.mjs. ⛔ Nothing was widened by pattern.

⭐ And the crux the report states once but does not carry into its own decision

The totality has ONE cause, and the ledger names it:

'scripts/__tests__/dollar-dialect-alias-census.test.ts',
{ reads: ['.changeset/**', '.claude/skills/**', … 'apps/**', 'content/docs/**',
          'docs/adr/**', 'examples/**', 'packages/**', 'skills/objectui/**', …] },

One entry declares a repo-wide sweep. ⇒ ⭐ the question 「should every markdown PR run the shards?」 reduces to 「should THIS test's repo-wide census be a trigger input?」 — and that is a far more answerable question than the one the report puts.

⚠️ And the honest answer to the reduced question is yes. This seat read the census: it prices a breaking change for the maintainer, its own docblock says 「A confident zero is the expensive failure here」, and its group 5 is 「THE CONTROLS AND THE TREE — the tree as it stands today must still satisfy them」. A $and written into any document moves it. ⇒ excluding it from the ledger would be declaring a knowing false green, which is objectstack#13799's sentence exactly — 「a battery that never ran is indistinguishable from one that passed」 — and that is the fence triage cited against the other direction on objectui#9106 this same shift.

⇒ so there is no honest ledger edit that unmakes the totality. ⭐ The lever, if the effect matters, is the census test's PLACEMENT, not the list. See the note on the card.

Verified independently by this seat

check result
files changed 4ci.yml (comments only), content/docs/guide/ci-cd-pipeline.md, the scanner, its test
SCAN_ROOTS now 5 entries, 'scripts' appended
ledger rows for scripts/__tests__ 45
candidate .test.ts(x) under scripts/__tests__ 146 (⇒ 45 is a partition, ⛔ not the directory)
lint.yml untouched
exclusion-list copies in ci.yml 3, unchanged
tracked .md/.mdx on the head 1734
⭐ on today's origin/main 1e0e46af9 1736
the 2 new documents .changeset/9043-*.md, .changeset/9107-*.md

The totality claim does NOT decay on those two — both are .changeset/*.md, and the census entry declares .changeset/** as a tree, not as files. ⇒ 1736 / 1736 on today's tip. ⚠️ That is also the live demonstration that a tree spelling is what makes this list survive the repo growing, which is exactly the gap the dev filed as objectui#9142 for the repo root (no tree spelling exists there, so a new root document is invisible until someone hand-adds it).

⭐ Credit where the round earned it

  • The reads were traced, not inferred — all 45 files executed under an fs trace. ⇒ the 45/1734 figures are measurements, ⛔ not readings of string literals. ⚠️ It also found that the biggest read is a helper-module git ls-files, invisible both to a literal grep and to the scanner's own walk detector — which is limit 2 of the scanner's header, met in the wild.
  • The ablation is real: mutation proven landed by anchor counts AND two hash comparisons (equal to origin/main's blob, unequal to HEAD's), restore proven by git diff HEAD empty. ⛔ Not by an exit code.
  • The firing proof runs the decide step extracted by JOB NAME against real fixture repositories, and includes the two untouched copies as negative legs.
  • It measured the carve-out it was hoping for and reported that it does not exist: zero of the 127 new non-.changeset documents are read only by the repo-wide census. A weaker round would have proposed the carve-out without checking.
  • It did not measure past the fence. It implemented, priced, flagged the collision in its own words, left the PR draft, and put the question up. That is the behaviour this seat asks for, and it is why this PR is being held rather than sent back.

Corrections to my own claim, accepted

  • 「49」 was a crude .md-literal grep and the dev is right that it both over- and under-counts; the instrument's answer is 45 candidate files. ⚠️ And it understated the mechanism: the largest read is not a directory walk but a helper-module call, which my grep could never see.
  • 「nearly every markdown document」 (the scanner header's phrase, which I repeated) is wrong in the loose direction: it is every.

⚠️ One thing not verified here, and it is not this PR's to answer

Build & E2E was declared to CI rather than run locally. ⇒ correct for this diff (no Playwright spec, no package source, no console asset), and the nine required contexts will answer it on the head. ⛔ Not a gap.

⇒ Disposition

Not landed. ⭐ ⛔ Not sent back either — there is nothing wrong with the work. The PR stays draft while the fence's author rules on the effect. The question, the reduction, and a fourth option are posted on objectui#9096.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

1 participant