Check SPEC §19's Zero-Skip roster against what the runners reported (#736) - #744
Conversation
SPEC section 19's roster promises "every skip a default (mock-mode) conformance run reports, one line per runner x test, verbatim from the runners' skip mechanisms", plus the maintenance rule "a PR that closes a gap deletes exactly its own lines". Both were enforced by nothing. It was already wrong. Kotlin and Swift each exclude "List operation returns first page with Link header" wholesale through their `link-header` tag branch, and the roster described that in prose instead of enumerating it — two of six runners misstated, in a roster long enough that nobody re-derives it by hand. Those lines are added here, which is what makes the set equality hold. The execution manifests (#602) made the ENUMERATION derivable, so check-fixture-execution now compares it for set equality in both directions: a runner skip with no roster line fails, and a roster line for a skip that no longer exists fails. The CLASSIFICATION and reasoning on each line stay judgement and nothing asserts them, which is why the section keeps `[manual]`. On the parser, since #740 declined to keep teaching sync-doc-constants more GFM: a mis-parse there is SILENT — it validates the wrong cell and reports success. This extraction fails LOUD in both directions. A bullet it cannot read is a name missing from the roster set; a name it invents is an extra. Neither produces a passing comparison, so the failure mode is a false alarm the author fixes, never a false green. A self-test case pins that property. The delimiters are deliberately not @-markers: sync-doc-constants owns those and runs in spec-gates, where no conformance run has happened and there are no manifests to compare against. Registering a kind there whose real enforcement lives here would split one check across two gates. Five self-test cases, each shown to fail against a gate with the roster check removed: skip missing from the roster, roster line for a closed gap, a runner with no section, SPEC with no roster block, and a bullet without a quoted name.
There was a problem hiding this comment.
Pull request overview
Adds automated validation that SPEC §19’s Zero-Skip roster matches runner execution manifests.
Changes:
- Parses and validates roster entries in both directions.
- Adds Kotlin and Swift
link-headerexclusions. - Adds negative self-tests for roster drift and malformed input.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
SPEC.md |
Updates and delimits the roster. |
scripts/check-fixture-execution.rb |
Implements roster parsing and comparison. |
scripts/test-check-fixture-execution.rb |
Tests roster validation failures. |
Suppressed comments (1)
scripts/check-fixture-execution.rb:242
- This silently ignores valid Markdown list items that use indentation,
*,+, or an ordered marker. If such an ignored item is a stale roster entry, there is no manifest-side name to create a mismatch, so the gate passes—the exact false-green the parser is intended to prevent. Recognize all list-item forms within this delimited block (or explicitly reject noncanonical ones) before validating the quoted case name, and cover a stale alternate-marker item in the self-test.
next unless line.start_with?("- ")
raise Failure, "SPEC roster bullet before any runner heading: #{line.strip[0, 60]}" if runner.nil?
name = line[/\A-\s+"([^"]+)"/, 1]
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 527faa4724
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ently Three review findings, both bots converging on two classes. Two of them are silent passes, which is the one failure mode this extractor is not allowed to have — the whole argument for reading prose here is that every misreading surfaces as a set mismatch. ROSTER DRIFT WENT UNCHECKED ON LINUX. The roster comparison sat after the partial-mode early return, and the Linux `make check-fixture-execution` path always passes --partial because Swift's manifest is macOS-only. So the normal local path never checked the roster at all: a stale Go or Ruby line passed and only the CI fan-in could catch it. The check now runs before the branch, over whatever manifests exist — "does Ruby's roster line match what Ruby reported" needs Ruby's manifest and nothing else. Partial input relaxes exactly one thing, the all-six overlap verdict, because that is the only claim needing every runner. A SECOND ROSTER BLOCK WAS IGNORED. parse_roster took the first begin and the first end, so a duplicate complete block was never compared and a stale line inside it passed unnoticed. Now exactly one of each delimiter is required, with "no roster at all" kept as its own distinct failure so the message names what happened. A CASE LISTED TWICE UNDER ONE RUNNER WAS INVISIBLE. Array#- removes every matching occurrence, so `actual - stated` and `stated - actual` both came back empty and the duplicate passed — carrying two possibly conflicting classifications for one case, against a section promising one line per runner x test. Duplicate bullets and duplicate runner headings are both rejected now. Four self-test cases, each shown to fail against the un-fixed code first: roster drift caught in partial mode, two roster blocks, one case listed twice, two sections for one runner.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (2)
scripts/check-fixture-execution.rb:264
- This silently ignores valid Markdown bullets that use
*,+, or up to three spaces of indentation. With clean manifests, a stale line such as* "a closed gap"remains visibly in the roster but is omitted fromstated, so the two-way check exits successfully—the false green this parser is intended to prevent. Recognize all unordered-list markers (or reject unsupported ones), and add the stale alternate-marker case to the self-test.
next unless line.start_with?("- ")
scripts/check-fixture-execution.rb:316
- The new ambiguous-name rejection is not exercised. The existing “one name in two files” test splits the two files across disjoint runner sets, so each runner sees the name only once and this branch never runs. Add a case where one runner excludes the same name from
alpha.jsonandbeta.json, and assert themore than one fixturefailure.
dupes = actual.tally.select { |_, n| n > 1 }.keys
unless dupes.empty?
errors << "#{m.runner} excludes #{dupes.first.inspect} in more than one fixture; the SPEC " \
"roster identifies cases by name alone and cannot express that. Add the fixture " \
"to those roster lines and teach this check to read it."
Copilot's suppressed comment, and the third silent-pass hole this round: a
stale roster entry written as an indented `-`, or with `*`, `+`, or `1.`, was
skipped by `start_with?("- ")`. Skipped means absent from the roster set, which
means it contradicts no manifest, which means the gate passes. A false green —
the one outcome this extractor may not produce, and the invariant the PR argues
from.
Three holes of one shape in a single round is evidence about the instrument,
not a queue of three patches, so this is not a third selector. The default is
inverted instead: a line that looks like a list item in ANY form must be the
canonical `- "case name"`, or it is an error. One predicate covers every
spelling, including ones nobody has written yet — the same "refuse what you do
not understand" move that was right for the roster tables' cell count in #740,
rather than teaching the parser one more shape.
Prose is untouched because it is not list-shaped: the roster's headings wrap
onto continuation lines and Python's section is a sentence, and verified that
every list-like line in the real roster is already canonical.
Four self-test cases, one per marker form, all shown to fail against the
permissive skip.
|
Round one at The PR argued from an invariant that was false three times overThis PR justifies adding a prose parser — one PR after #740 declined to grow one — on the claim that every misreading surfaces as a set mismatch, never a passing comparison. Review found three separate ways it produced a silent pass:
Each is fixed. But three holes of one shape in a single round is evidence about the instrument, not a queue of three patches — so #3 is deliberately not a third selector. The default is inverted: a line that is list-shaped in any form must be the canonical Flagging for your call, not acting further: the invariant now holds as far as two independent reviewers and I can see, and the fail-closed default is a structurally different guarantee from the three patches that preceded it. But if you would rather this roster stop being prose altogether — a data file with the SPEC block generated from it, no parser at all — that is a legitimate read of three-strikes and it is your decision, not mine. I have not started it. Also fixed, and worse than it lookedRoster drift went unchecked on Linux entirely. The comparison sat after the partial-mode early return, and the Linux Every fix has a self-test case shown to fail against the un-fixed code first (11 new cases total). |
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Closes #736.
The roster promised something nothing checked
SPEC §19's Zero-Skip roster states its own contract:
and its maintenance rule:
Neither was enforced. It was already wrong when I got here: Kotlin and Swift each exclude
List operation returns first page with Link headerwholesale through theirlink-headertagbranch, and the roster described that in prose instead of enumerating it — two of six runners
misstated, in a roster long enough that nobody re-derives it by hand. Adding those two lines is
what makes the set equality hold.
What this adds
The execution manifests from #602 made the enumeration derivable, so
check-fixture-executionnow compares it for set equality in both directions:The classification and reasoning on each line stay judgement, and nothing asserts them.
That half is why the section keeps its
[manual]tag — this makes the derivable half derived,not the whole thing.
On adding another prose parser
#740 declined to keep teaching
sync-doc-constants.rbmore GFM — separator widths, backslashparity — and I want to be explicit about why this one is different rather than quietly doing
the thing I argued against.
A mis-parse there is silent: it validates the wrong cell and reports success. This
extraction is designed to fail loud in both directions. A bullet it cannot read is a name
missing from the roster set — a mismatch. A name it invents is an extra — also a mismatch.
The intended property is that no reading of a malformed line yields a passing comparison, so
the failure mode is a false alarm the author fixes, never a false green.
Review found three independent breaches of exactly that property, and they are the reason
to read this claim as a designed invariant now enforced, rather than one that held on
arrival: a second roster block was silently ignored (first begin/end won), a case listed twice
under one runner cancelled out of both diffs (
Array#-removes every occurrence), and a staleentry written with a non-canonical list marker (
*,+,1., indented-) was skippedentirely — each a silent pass. The third was not fixed with a third selector: the default is
inverted, so any list-shaped line must be canonical or the gate errors. Self-test cases pin all
of them.
Three strikes on one invariant is also a fair reason to question prose-parsing here at all.
The alternative — moving the roster to a data file and generating the SPEC block from it —
removes the parser rather than hardening it, and is Jeremy's call; it is not started.
The delimiters are deliberately not
@-markers.sync-doc-constantsowns those and runsin
spec-gates, where no conformance run has happened and there are no manifests to compareagainst. Registering a kind there whose real enforcement lives here would split one check
across two gates.
Identity
The roster names a case by name alone, so it cannot express two same-named cases from different
fixtures — and names are not unique across files (#743 found one appearing in three). No
runner excludes such a pair today; if one ever does, the gate says so and asks for file
qualifiers rather than silently comparing an ambiguous set.
Verification
make check-fixture-execution— green; roster matches all six runners.skip missing from the roster, roster line for a closed gap, a runner with no section, SPEC
with no roster block, and a bullet without a quoted name.
and fails; adding a stale TypeScript line fails the other way.
make doc-constants-checkgreen — the new delimiters do not collide with the@-markerconvention.
Summary by cubic
Checks SPEC §19’s Zero-Skip roster against the runners’ manifests and fails on drift. Previously unchecked and prone to silent passes; now set-equality validation runs before
--partialand fails closed on malformed/duplicate entries. SPEC adds delimiters and enumerates Kotlin/Swiftlink-header.- "case name" — …with quoted names; rejects duplicate bullets and duplicate sections; errors if a runner excludes the same-named case from multiple fixtures.<!-- zero-skip-roster:begin -->/<!-- zero-skip-roster:end -->; added Kotlin/Swift lines for thelink-headerskip.*,+,1., indented), and partial-mode coverage.Developer actions
- "Case name" — reason.*,+, ordered, or indented list markers; do not duplicate bullets.Written for commit 4ce947b. Summary will update on new commits.