Repository navigation
[finding] ci.yml's shard-classifier warning describes the pre-#6589 co-occurrence classifier — all three of its stated mechanisms are now false #10713
Description
Noticed while implementing #10057 (PR #10708), which edits the run: block this comment
guards. Not fixed there — out of scope for that card, and this is an observation about
comment accuracy, not a behaviour defect.
What is stale
.github/workflows/ci.yml (the "Compute this shard's package set" step) explains why it
spells git cat-file -e instead of git rev-parse:
check-shard-attestation.mjs classifies a job as an aggregate GATE when the job's joined
run:text contains BOTH the string "check-shard-attestation.mjs" and, as a bare
substring, that script's own dash-dash-verify flag. [...] so spelling that flag anywhere
in this step — including in a comment, since comments are part ofrun:— silently
reclassifies the shard job as a gate.
All three emphasised claims describe the pre-#6589 classifier. The current
scripts/check-shard-attestation.mjs repudiates each one in its own docblock:
| ci.yml says | classifier now does |
|---|---|
"the job's joined run: text" |
jobInvokes is "Per STEP, never over the job's joined text: two steps' texts concatenated are not a command, and treating them as one is exactly #6589" |
| flag counts "as a bare substring" | "classification is by INVOCATION — invokesScript() requires the flag to be an argument of a command that runs this script — and never by two substrings co-occurring somewhere in a job" |
"including in a comment, since comments are part of run:" |
"The classifier therefore also drops shell comments [...] the self-test's dash-dash-verify-in-a-comment fixture is the pin" |
Why it is worth correcting rather than leaving
The comment's instruction ("do not tidy this back") is still good — git cat-file -e is
fine and there is no reason to churn it. What has decayed is its reason, and that is the
part a reader acts on:
- it overstates the hazard, so an agent editing this high-traffic block may contort to avoid
writing a perfectly safe word in a comment (the classifier now pins that exact case); - a reader who checks the claim against the classifier finds it contradicted, which invites
distrust of a guard that is in fact correct — the same failure mode as a stale runbook.
The honest replacement is roughly: the spelling is retained because it was measured under the
pre-#6589 co-occurrence classifier; that classifier has since been made adjacency-based and
comment-dropping, so the hazard is closed by construction and the spelling is now just a
harmless preference.
Suggested scope
Comment-only edit in .github/workflows/ci.yml; no behaviour change, no gate change.
Verified against origin/main at 47cd3ec1f8.
Generated by Claude Code
Generated by Claude Code
Activity
os-zhuang commentedon Aug 21, 2026
Triage (finding concentration round): promoted to
pm:queue· type Task. Comment-only edit in a high-traffic ci.yml block whose stale hazard claim actively distorts how agents edit it, and whose contradiction-on-inspection undermines a guard that is in fact correct. The card's "honest replacement" paragraph is the spec. Same-day churn note: PR #10708 (#10057) edits the guardedrun:block — re-read currentmainbefore editing, and serialize behind that PR if it is still open at dispatch time. S,mode:subagent, sonnet; Clause-②: no.Generated by Claude Code
claude commentedon Aug 21, 2026
{ "issue": 10713, "status": "done", "branch": "claude/issue-10713-shard-classifier-comment", "pr": "https://github.com/objectstack-ai/objectstack/pull/10817", "premise_still_valid": true, "summary": "The card's premise holds and all three of its table rows re-derive correctly against the current scripts/check-shard-attestation.mjs source (not its docblock). Replaced the stale rationale comment on ci.yml's \"Compute this shard's package set\" step with one describing the post-#6589 classifier; the step's behaviour is byte-identical and `git cat-file -e` stays. Dated the decay precisely: the comment was authored in e4737b6ac2 (#6195) and the classifier was rewritten LATER THE SAME DAY in 4e271b2c67 (#6589), so it never described the classifier it shipped beside. The spelling's stated reason HAS changed and the corrected comment says so: the hazard is closed by construction, so either spelling is safe and this one stays only because churning it buys nothing. The replacement deliberately spells `--verify` literally (three times) where the old comment wrote \"dash-dash-verify\" to avoid it -- proven safe first, and that choice is what makes the before/after gate run load-bearing rather than vacuous.", "tests": "ALL exit codes captured as `cmd > file 2>&1; ec=$?`, never through a pipe. CLASSIFICATION UNCHANGED -- `pnpm check:shard-attestation` BEFORE: exit=0, verdict lines 'check-shard-attestation --self-test: 92 assertions (dominance experiment + both #6082 counter-examples + the #4928 guard + the #6589 classifier pins).' and 'check-shard-attestation: 2 aggregate gate(s) count 3 declared leg(s) across 3 attesting job(s).'; AFTER: exit=0, both verdict lines byte-identical; re-run at final commit db6e53c3bd: exit=0, same. COUNTERFACTUAL proving the measurement is not vacuous -- with the edited ci.yml on disk, the pre-#6589 rule (text.includes(BASENAME) && text.includes('--verify')) classifies job `test` as a GATE = true, while the current invocation classifier = false, and it stays an attesting shard (--emit) = true; the edited step spells '--verify' 3 times and invokesScript(step,'--verify') = false. THREE CLAIMS re-derived from code: (1) 'joined run: text' is FALSE -- classification is `jobInvokes = (job,flag) => stepsOf(job).some(step => invokesScript(step?.run, flag))` used at `if (!jobInvokes(job,'--verify')) continue;`, per STEP; runTextOf() still exists but is built only AFTER a job is classified a gate, to extract --leg tokens. (2) 'bare substring' is FALSE -- invokesScript lexes into commands, finds the token === SCRIPT_BASENAME (or endsWith '/'+it), then requires `token === flag || token.startsWith(flag+'=')` AFTER it in the SAME command. (3) 'comments are part of run:' is FALSE -- shellCommands has `if (ch === '#' && atWordStart) { skip to newline; flush(); }`, and flush() sets atWordStart=true so both line-start and trailing comments are dropped; pinned by two --self-test fixtures. GATE UNION re-derived by `node scripts/pm/dispatch-gates.mjs` with NO paths passed (it takes its own change set) and re-run at final commit db6e53c3bd -- exit=0, same 7 families, nothing new: check:node-version 0, check:required-contexts 0, check:shard-attestation 0, check:workflow-status-functions 0, check-aggregator-roster.mjs 0, check-required-contexts.mjs 0, check-shard-attestation.mjs 0; plus check:nul-bytes 0 and a manual control-byte grep (exit 1 = clean). No scripts/** path touched, so the known dispatch-gates blind spot (#10784/#10705, entry-guard + parse-guard) does not apply. NO ABLATION APPLIES: a comment correction has no guard under test -- there is no mutation whose removal could turn anything red, so that slot is genuinely empty rather than filled with something that did not happen; the counterfactual above is the honest substitute. CI at report time: 21 success, 3 skipped, 7 still in_progress (reported at draft-PR time per the dispatch contract, not waited on).", "open_questions": [], "out_of_scope_findings": [ "filed as #10818: check-required-contexts.mjs's `uncommentedYaml` drops only WHOLE-LINE comments (/^\\s*#/), so a trailing or block-scalar `# --verify-required-set` in any workflow would false-positive BOTH its absence assertion and its presence/caller sweep -- latent today, and the trigger is writing a comment warning about that flag, which is exactly what this card just did for the sibling gate. check-shard-attestation.mjs solved the same problem properly with a real lexer one file over." ] }Generated by Claude Code