Skip to content

docs(ci): correct the shard-classifier comment to describe the post-#6589 classifier - #10817

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-10713-shard-classifier-comment
Aug 21, 2026
Merged

os-zhuang merged 2 commits into
mainfrom
claude/issue-10713-shard-classifier-comment

Conversation

@claude

@claude claude Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Fixes #10713

The Compute this shard's package set step in .github/workflows/ci.yml justified its
git cat-file -e spelling with three claims about scripts/check-shard-attestation.mjs.
All three describe the classifier as it stood before #6589. This corrects the comment
only — the step's behaviour is untouched and git cat-file -e stays.

The comment was authored in e4737b6ac2 (#6195) and the classifier was rewritten later
the same day in 4e271b2c67 (#6589), so the text has described a classifier that no
longer exists ever since.

The three claims, re-derived from the classifier source

Each row was settled against the code, not against the script's docblock (which is prose
and could itself have gone stale) and not against the card's table.

The comment claimed What the current source does Verdict
classification reads "the job's joined run: text" scanWorkflow builds jobInvokes = (job, flag) => stepsOf(job).some((step) => invokesScript(step?.run, flag)) and classifies with if (!jobInvokes(job, '--verify')) continue; false — applied per STEP
the flag counts "as a bare substring" invokesScript lexes the text into commands, locates the token equal to SCRIPT_BASENAME (or ending / + it), then requires token === flag || token.startsWith(flag + '=') among the tokens after it in that same command false — an exact argument token, adjacent within one command
"including in a comment, since comments are part of run:" shellCommands hits if (ch === '#' && atWordStart), skips to the newline and flushes false — # comments are dropped

Two details worth recording, because neither is visible from the docblock alone:

Why the corrected comment now spells --verify literally

The old comment went out of its way to write "dash-dash-verify" instead of the flag. That
avoidance is no longer necessary, and demonstrating it is the point — so the replacement
names the flag three times, in the very step that already carries the script's basename in
its --emit step. That makes this the load-bearing measurement rather than a vacuous one:

WITH MY EDITED ci.yml on disk:
  joined run: text contains "check-shard-attestation.mjs" : true
  joined run: text contains "--verify"                    : true
  PRE-#6589 co-occurrence rule  => job `test` is a GATE ?  : true
  CURRENT invocation classifier => job `test` is a GATE ?  : false
  still classified an attesting SHARD (--emit)            : true

The pre-#6589 rule quoted there (text.includes(BASENAME) && text.includes('--verify')) is
the one the classifier's own docblock records as the defect #6589 removed. The safety of
this text is pinned upstream by two --self-test assertions — a --verify-in-a-comment
fixture and a trailing-comment fixture.

Evidence

pnpm check:shard-attestation, before and after, verdict lines quoted — the shard job's
classification is unchanged:

BEFORE  exit=0
  ✓ check-shard-attestation --self-test: 92 assertions (dominance experiment + both #6082 counter-examples + the #4928 guard + the #6589 classifier pins).
  ✓ check-shard-attestation: 2 aggregate gate(s) count 3 declared leg(s) across 3 attesting job(s).

AFTER   exit=0
  ✓ check-shard-attestation --self-test: 92 assertions (dominance experiment + both #6082 counter-examples + the #4928 guard + the #6589 classifier pins).
  ✓ check-shard-attestation: 2 aggregate gate(s) count 3 declared leg(s) across 3 attesting job(s).

Gate union re-derived by node scripts/pm/dispatch-gates.mjs (no paths passed — it takes
its own change set) and re-run at final commit db6e53c3bd. Exit codes captured with
cmd > file 2>&1; ec=$?, never through a pipe:

Gate Exit
pnpm check:node-version 0
pnpm check:required-contexts 0
pnpm check:shard-attestation 0
pnpm check:workflow-status-functions 0
node scripts/check-aggregator-roster.mjs 0
node scripts/check-required-contexts.mjs 0
node scripts/check-shard-attestation.mjs 0
pnpm check:nul-bytes 0

dispatch-gates named exactly the four workflow-parsing families the card predicted, plus
the shard/required-contexts duplicates from their other workflows; it added nothing my diff
did not touch. No scripts/** path is touched, so the check:entry-guard /
check-parse-guard.mjs blind spot does not apply here.

No ablation applies. A comment correction has no guard under test — there is no mutation
whose removal could turn anything red, and the counterfactual above is the honest substitute:
it shows the edit would have flipped the classification under the classifier the comment
describes.

Scope

Generated by Claude Code


Generated by Claude Code

…6589 classifier

The "Compute this shard's package set" step explained its `git cat-file -e`
spelling with three claims about check-shard-attestation.mjs, all of which
describe the classifier as it stood BEFORE #6589:

  - "the job's joined `run:` text"  -> classification is `jobInvokes`, applied
    per STEP; the joined text is built only after a job is already classified
    a gate, to extract its --leg tokens.
  - the flag counts "as a bare substring" -> `invokesScript()` tokenises each
    command and requires the flag to be an exact argument token following the
    script's own name IN THE SAME COMMAND.
  - "including in a comment, since comments are part of `run:`" -> the
    `shellCommands()` lexer drops `#` comments outright.

The comment was authored in e4737b6 (#6195); the classifier was rewritten
later the same day in 4e271b2 (#6589), so it has described a classifier
that no longer exists ever since.

The step's behaviour is untouched: `git cat-file -e` stays. Only its stated
reason changes, because the hazard it cites is now closed by construction.

The corrected comment deliberately spells `--verify` literally, which the old
comment avoided. That is the load-bearing check: under the pre-#6589
co-occurrence rule this text would reclassify job `test` as an aggregate gate,
and `pnpm check:shard-attestation` reports the identical verdict before and
after -- "2 aggregate gate(s) count 3 declared leg(s) across 3 attesting
job(s)".

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt
@os-zhuang
os-zhuang marked this pull request as ready for review August 21, 2026 13:18
@os-zhuang
os-zhuang added this pull request to the merge queue Aug 21, 2026
Merged via the queue into main with commit b303673 Aug 21, 2026
31 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-10713-shard-classifier-comment branch August 21, 2026 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci/cd size/s skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[finding] ci.yml's shard-classifier warning describes the pre-#6589 co-occurrence classifier — all three of its stated mechanisms are now false

2 participants