Skip to content

Make every conformance runner account for the fixture cases it ran - #742

Merged
jeremy merged 9 commits into
mainfrom
feat/conformance-case-census
Aug 13, 2026
Merged

Make every conformance runner account for the fixture cases it ran#742
jeremy merged 9 commits into
mainfrom
feat/conformance-case-census

Conversation

@jeremy

@jeremy jeremy commented Aug 13, 2026

Copy link
Copy Markdown
Member

What

Every conformance runner now asserts, at the end of its own run:

passed + failed + skipped  ==  cases under conformance/tests, recursively,
                               whose mode != "live"

On mismatch it names the short side and exits non-zero — a silent under-count is
the failure mode this exists to remove, so it is not a warning.

The right side is counted by its own recursive walk and parse, deliberately not the
runner's load path. That independence is the whole point: a check fed by the load path
can only confirm the load path agrees with itself.

mode != "live" rather than mode == "mock" is what makes the divergence arithmetic.
All six runners select with "mock unless told otherwise" ((mode ?? "mock") == "mock"
and its five equivalents), so a mode nobody recognizes is dropped by every runner at
once with nothing printed anywhere.

Why this layer

This is #602is any fixture case executed by nothing? — answered where the answer
lives. #740 originally carried a hand-rolled source-text parser over the six runners'
skip tables. Scored the way AGENTS.md asks:

rules added across four review rounds:  9 commits
call sites covered:  6 runners, 8 tables, 2 tag branches  — unchanged

Twenty-seven findings, nearly all "here is another source spelling the parser does not
recognize"
. Ground truth for "what did this runner skip" is runtime; a source parser
structurally cannot see result.skipped, a derived table, or a case the loader dropped.
That half is parked on conformance-skips-parser-archive.

What it catches

  • a typo'd or otherwise unrecognized mode
  • a fixture that failed to parse, or was never globbed — including one nested below
    conformance/tests/
    , which no runner discovers
  • a case dropped between load and dispatch
  • a whole fixture emptied to [] — the census refuses such a file rather than
    counting it as zero (see the review correction below)
  • any future skip channel that bypasses the counters, because the counters are what it reads

The typo class is not this check's alone: conformance-fixtures-check pins mode to
enum: ["mock", "live"], so a typo in a top-level fixture fails there first. Verified
what that gate structurally cannot see — its glob is not recursive, so a fixture at
conformance/tests/nested/probe.json passes make conformance-fixtures-check (exit 0)
and fails the census
. Nor does that gate run when make conformance-<lang> is invoked
alone.

What it does not catch

The literal all-six case #602 names. When every runner excludes the same case for its
own reason, every runner's census is green — each counted its own skip. Detecting that
needs the six exclusion sets in one place, which needs artifact plumbing across six CI
jobs (no precedent in test.yml, and the macOS-only Swift lane means a Linux run can
never assemble six manifests). #602 stays open for that; today's maximum overlap is
2 of 6 and #596 already narrowed it.

Swift's arm is macOS-only. conformance-swift is ifdef IS_MACOS; on Linux it prints
SKIP and Swift's census never runs. A green Linux make is five-runner coverage, not six.
The Makefile now says so where the ifdef is.

Review corrections

Three rounds with Copilot and Codex produced four distinct defect classes, each now
closed in every runner that had it. The first falsified a claim in the original
description, so it is corrected above.

1. [] truncation was not caught — the claim was false. An emptied fixture is the
one truncation both sides of the census read identically: the runner registers nothing
from that file and the census expected nothing, so both totals fall by the same amount
and no mismatch appears. The census now refuses an emptied fixture instead of
counting it as zero. A file declaring no cases tests nothing, so refusing it costs
nothing — and it closes the same hole in conformance-fixtures-check, where [] is a
schema-valid list of zero items. Self-tests flipped from "accepts as zero" to
"rejects". (6/6 runners)

2. A nested-only tree stepped over the comparison. Runners returned success from
their no test files found branch when the top-level glob was empty — before reaching
the count check. That is exactly the nested-fixture under-count this PR advertises, so
the early exit is gone; the message still prints and the run falls through to fail the
comparison. (5/5 that had one)

3. Walk errors were swallowed. Each language's directory walk had its own silent-drop
behavior — Kotlin's FileTreeWalk skips a directory whose listFiles() fails, Python's
Path.rglob suppresses the scan OSError, Ruby's Dir.glob omits what it cannot
traverse (and Find.find is no better — find.rb rescues Errno::EACCES and moves on),
Swift's try? on per-entry metadata discarded the failure. In every case the subtree
leaves the census, the runner's non-recursive glob never saw it either, and the two sides
agree on a count that omits it: a walk documented as fail-closed, failing open. Go
(filepath.WalkDir) and TypeScript (readdirSync) were already correct. (4/4 that
swallowed)

4. A falsy mode defaulted to mock. Python's (mode or "mock") read "mode": "" as
an absent key; Ruby's (mode || "mock") did the same for an explicit false. Both ran a
value the other runners refuse, while the census counted it as non-live — matching totals
over a mode nothing accepts. Both now default on None/nil only. (2/2 that had it)

Also fixed: Go accepted a top-level null as zero cases (nil slice, no error), and its
Mode string could not tell an absent key from "mode": "", so it alone ran a mode the
other five refuse (now *string).

Every fix was mutation-tested: reverting each one kills exactly its own new test, with no
compile errors, and sources were restored by copy and diffed. The unreadable-subtree
tests branch on euid, since root reads through a 0o000 directory — under root the
assertion becomes "the cases are still counted", so neither environment can pass by
silently dropping the subtree.

Self-tests

The census is green on the real tree by construction, so a live run only proves it can say
yes. Each language gets a suite against a synthetic fixture set (conformance-runner-tests,
plus vitest for TS) proving it can say no: a mode: "moc" case where the runner's own
predicate and the census disagree by one, and the fail-closed paths — an unparseable
fixture, a non-array fixture, an empty walk.

The predicate is shared, not copied: each runner's load filter now calls the same
isMockMode the tests exercise.

Verification

  • make conformance — all six report 198 = 198, green.
  • make conformance-runner-tests — five suites green; TS census tests run under conformance-typescript.
  • Proved it can say no end-to-end, three ways, since the count check lives in main()
    where a unit test cannot reach it. All six runners exit non-zero for: a mode: "moc"
    case (each naming 1 executed by nothing); every fixture moved one directory down; and
    one fixture emptied to []. The fixture tree was restored by copy and diffed
    byte-identical after each probe.
  • Full serial make check on a Linux host under LC_ALL=C.UTF-8 — exit 0.

jeremy added 6 commits August 13, 2026 01:41
#602 asks whether any fixture case is executed by nothing. Ground truth for
that is runtime, so each runner now asserts, at the end of its own run:

  passed + failed + skipped == cases under conformance/tests whose mode != "live"

The right side is counted by its own recursive walk and parse, deliberately
not the runner's load path — a check fed by the load path can only confirm
the load path agrees with itself.

"Not explicitly live" rather than "mock" is what makes the divergence
arithmetic: all six runners select with "mock unless told otherwise", so a
mode nobody recognizes is dropped by every one of them with nothing printed.

case_census.go states what this reaches and what it does not. The all-six
case #602 names is NOT covered — every runner's census stays green when each
excludes the same case for its own reason — so #602 stays open for the
cross-runner manifest that needs artifact plumbing across six CI jobs.

The census is green on the real tree by construction, so a live run only
proves it can say yes. case_census_test.go runs it against a synthetic set
where loadTests and the census disagree by one, and pins the fail-closed
paths: an unparseable fixture, a non-array fixture, and an empty walk.
Same invariant as the Go runner, expressed where vitest can see it. vitest
owns pass/fail/skip accounting and `it.skip` is how this runner spells a
skip, so there are no counters to sum; a dedicated `it()` asserts instead
that every non-live fixture case became a registered test. A case dropped at
load never becomes an `it` at all — and this is the one lane with no
`SKIP: <name>` line, so a dropped case leaves no trace in the output either.

case-census.ts carries the census (its own recursive walk and parse) plus
isMockMode, which loadTestSuites now applies, so the self-tests exercise the
predicate the load path uses rather than a copy of it.
Same invariant and shape as the Go runner: CaseCensus takes its own
recursive walk of conformance/tests before the run loop, the summary reports
both sides, and a mismatch names the short side and exits non-zero. A silent
under-count is the failure mode this exists to remove, so it is not a warning.

CaseCensus.mock_mode? is the run loop's own filter, shared with the
self-tests so the divergence they assert is the real one.
Same invariant as the Go runner. The census lives in ConformanceSupport, the
SDK-free target, for the reason that target exists: one carrying @main cannot
host XCTest cleanly, and a check green on the real fixture tree by
construction can only be proven to say NO against a synthetic one.

TestCase.isMock now calls CaseCensus.isMockMode, so the self-tests exercise
the predicate the run loop applies.

Swift's arm is macOS-only, and the Makefile now says so where the ifdef is:
conformance-swift prints SKIP on Linux, so a green Linux `make` is five-runner
census coverage, not six. CI closes that on macos-15; nothing closes it for a
Linux developer.
Same invariant and shape as the Go runner, with CaseCensus.isMockMode shared
between the run loop's filter and the self-tests.

The census KDoc spells the rule out in prose rather than as a glob: Kotlin
nests block comments, so `tests/` followed by two stars opens one inside the
KDoc and swallows the rest of the file.
Same invariant and shape as the Go runner, with is_mock_mode shared between
the run loop's filter and the self-tests.
Copilot AI balanced review requested due to automatic review settings August 13, 2026 08:48
@github-actions github-actions Bot added kotlin conformance Conformance test suite labels Aug 13, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds independent fixture censuses to all six conformance runners, ensuring non-live cases are accounted for and mismatches fail the run.

Changes:

  • Adds recursive census implementations and runner integration.
  • Shares mock-mode predicates between loaders and census tests.
  • Adds synthetic tests for mismatch and failure paths.

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 18 out of 18 changed files in this pull request and generated 11 comments.

Show a summary per file
File Description
Makefile Documents Swift’s macOS-only census coverage.
kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/CaseCensus.kt Implements Kotlin census logic.
kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/Main.kt Integrates census into Kotlin runner.
kotlin/conformance/src/test/kotlin/com/basecamp/sdk/conformance/CaseCensusTest.kt Tests Kotlin census behavior.
conformance/runner/typescript/case-census.ts Implements TypeScript census logic.
conformance/runner/typescript/case-census.test.ts Tests TypeScript census behavior.
conformance/runner/typescript/runner.test.ts Verifies registered fixture-case count.
conformance/runner/swift/Sources/ConformanceSupport/CaseCensus.swift Implements Swift census support.
conformance/runner/swift/Sources/ConformanceRunner/Runner.swift Integrates census into Swift runner.
conformance/runner/swift/Sources/ConformanceRunner/Fixtures.swift Shares Swift mock-mode selection.
conformance/runner/swift/Tests/ConformanceSupportTests/CaseCensusTests.swift Tests Swift census behavior.
conformance/runner/ruby/runner.rb Implements and integrates Ruby census.
conformance/runner/ruby/case_census_test.rb Tests Ruby census behavior.
conformance/runner/python/runner.py Implements and integrates Python census.
conformance/runner/python/test_case_census.py Tests Python census behavior.
conformance/runner/go/case_census.go Implements Go census logic.
conformance/runner/go/main.go Integrates census into Go runner.
conformance/runner/go/case_census_test.go Tests Go census behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread conformance/runner/go/main.go
Comment thread conformance/runner/python/runner.py
Comment thread conformance/runner/python/runner.py Outdated
Comment thread conformance/runner/ruby/runner.rb
Comment thread conformance/runner/python/runner.py
Comment thread conformance/runner/typescript/case-census.ts
Comment thread conformance/runner/go/case_census.go Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 32bb6a232e

ℹ️ 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".

Comment thread conformance/runner/typescript/case-census.ts
Comment thread conformance/runner/go/case_census.go
Comment thread conformance/runner/python/runner.py Outdated
Two defects both PR bots found independently, one of which falsified a claim
in the PR description.

An emptied fixture was NOT caught. `[]` is the one truncation both sides of
the census read identically: the runner registers nothing from the file and
the census expected nothing, so both totals fall by the same amount and no
mismatch ever appears. Counting it as zero made "a whole fixture truncated to
[]" a guarantee the check could not keep. The census now refuses such a file.
A fixture declaring no cases tests nothing, so refusing it costs nothing —
and it closes the same hole in conformance-fixtures-check, where an empty
array is a schema-valid list of zero items. The self-tests flip from
"accepts as zero" to "rejects".

A nested-only tree stepped over the comparison. Four runners returned success
from their "no test files found" branch when the top-level glob came back
empty — before reaching the count check — which is exactly the nested-fixture
under-count this census advertises. The early exit is gone in all four; the
message still prints and the run falls through to fail the comparison. Swift
already exited non-zero there but printed "No test files found" for a tree
full of them, so it falls through too and prints the diagnosis instead.

Also: Go accepted a top-level `null` as zero cases (it unmarshals to a nil
slice without error) and now rejects it as a non-array root; Go's
`Mode string` could not tell an absent key from `"mode": ""`, so it alone ran
a mode the other five refuse, and is now `*string`; Python's
`(mode or "mock")` defaulted on falsiness with the same effect and now
defaults on None only; Swift's directory enumerator silently skipped
unreadable subtrees, dropping their cases from both sides at once, and now
takes an errorHandler that aborts and reports.

Both new guarantees are proven end-to-end, not just by unit test — the count
check lives in main() where a unit test cannot reach it. With every fixture
moved one directory down, and separately with one fixture emptied to `[]`,
all six runners exit non-zero; the fixture tree was restored by copy and
diffed byte-identical after each.
Copilot AI review requested due to automatic review settings August 13, 2026 09:38
@jeremy

jeremy commented Aug 13, 2026

Copy link
Copy Markdown
Member Author

Addressed the first review round — both bots independently found the same two defects, and the first one falsified a claim in the description, so it is corrected there too.

  • [] truncation was not caught. It is the one truncation both sides of the census read identically, so the totals fall together and no mismatch appears. The census now refuses an emptied fixture rather than counting it as zero.
  • A nested-only tree stepped over the comparison in four runners, which is exactly the under-count this PR advertises. The early exit is gone.
  • Plus: Go accepted a top-level null as zero cases; Go and Python both ran "mode": "" as if the key were absent, where the other four refuse it; Swift's enumerator silently skipped unreadable subtrees.

Both new guarantees are proven end-to-end rather than by unit test, since the count check lives in main() where a unit test cannot reach it: with every fixture moved one directory down, and separately with one fixture emptied to [], all six runners exit 2. The fixture tree was restored by copy and diffed byte-identical after each probe.

make conformance and make conformance-runner-tests green; a full serial make check is running on Linux under LC_ALL=C.UTF-8.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.

Suppressed comments (4)

kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/CaseCensus.kt:94

  • walkTopDown() has no onFail handler, so Kotlin's FileTreeWalk silently skips a descendant whose listFiles() fails. Because the runner also ignores nested fixtures, an unreadable nested subtree disappears from both totals and the census can pass, contrary to this method's fail-closed contract. Propagate traversal failures (and add the same inaccessible-subtree coverage used by Swift).
        val files = testsDir.walkTopDown()
            .filter { it.isFile && it.extension == "json" }
            .sortedBy { it.path }
            .toList()

conformance/runner/python/runner.py:113

  • On the authoritative Python 3.13 conformance lane, Path.rglob() suppresses OSErrors raised while scanning. An unreadable nested directory is therefore omitted from this list; the non-recursive runner omits it too, so both counts can agree while cases are unaccounted for. Use a recursive walk that propagates scan errors and test an inaccessible subtree.
    files = sorted(Path(tests_dir).rglob("*.json"))

conformance/runner/ruby/runner.rb:81

  • Dir.glob silently omits descendants it cannot traverse. Since the runner's top-level glob omits the same nested subtree, its cases vanish from both sides and this supposedly fail-closed census remains green. Enumerate recursively with an API that raises on directory-read errors, convert those errors to CaseCensus::Error, and cover an inaccessible subtree.
    files = Dir.glob(File.join(tests_dir, "**", "*.json")).sort

conformance/runner/swift/Sources/ConformanceSupport/CaseCensus.swift:120

  • The enumerator's errorHandler covers traversal failures, but this try? introduces another silent-drop path: if metadata lookup for a .json entry fails, that fixture is omitted instead of failing the census. For a nested entry the runner omits it as well, so the totals can still agree. Convert the metadata error to CensusError.unreadableTree rather than discarding it.
        for case let url as URL in walker where url.pathExtension == "json" {
            let isRegular = (try? url.resourceValues(forKeys: [.isRegularFileKey]))?.isRegularFile
            if isRegular == true { files.append(url) }
        }

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c45a686c69

ℹ️ 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".

Comment thread conformance/runner/python/runner.py Outdated
Comment thread conformance/runner/ruby/runner.rb Outdated
Round two of one class, found by Copilot in its suppressed comments after
Codex flagged the Swift instance in round one. The round-one fix covered one
runner; these are the call sites it missed.

Each language's directory walk had its own silent-drop behavior:

  Kotlin   FileTreeWalk skips a directory whose listFiles() fails
  Python   Path.rglob suppresses the OSError raised while scanning
  Ruby     Dir.glob omits what it cannot traverse — and Find.find is no
           better, since find.rb rescues Errno::EACCES and moves on
  Swift    a second path past the round-one errorHandler: `try?` on the
           per-entry metadata lookup discarded the failure

All four are the same failure. The subtree leaves the census, the runner's
non-recursive glob never saw it either, and the two sides agree on a count
that omits it — a walk documented as fail-closed, failing open. Go
(filepath.WalkDir, error returned) and TypeScript (readdirSync, throws) were
already correct and are unchanged.

Ruby now walks Dir.children, which raises where both library helpers swallow.

Each new test was shown to FAIL against the un-fixed walk before being kept:
reverting each fix in turn kills exactly `reports an unreadable subtree` in
that language, with no compile errors, and the sources were restored by copy
and diffed. The tests branch on euid because root reads through a 0o000
directory — under root the assertion becomes "the cases are still counted",
so neither environment can pass by silently dropping the subtree.
Copilot AI review requested due to automatic review settings August 13, 2026 09:51
@jeremy

jeremy commented Aug 13, 2026

Copy link
Copy Markdown
Member Author

Second round — addressing Copilot's four suppressed comments, which were all one class: the census's own walk swallowing traversal errors. Codex flagged the Swift instance last round; these are the call sites that fix missed.

runner silent-drop behavior now
Kotlin FileTreeWalk skips a directory whose listFiles() fails .onFail { throw CensusException }
Python Path.rglob suppresses the OSError raised while scanning os.walk(..., onerror=)
Ruby Dir.glob omits what it cannot traverse — and Find.find is no better, since find.rb rescues Errno::EACCES and moves on hand-rolled walk over Dir.children, which raises
Swift a second path past last round's errorHandler: try? on the per-entry metadata lookup try, converted to CensusError.unreadableTree

Go (filepath.WalkDir, error returned) and TypeScript (readdirSync, throws) were already correct and are unchanged.

On whether this is a treadmill. Scoring it the way AGENTS.md asks — the control's rule count grew, but so did the call sites it covers, by four the previous round genuinely missed. That is reach, not accretion, so I fixed it rather than escalating. Recording the tripwire, though: if a third round produces more silent-drop paths in these walks, the instrument question is the one Codex raised in passing — compare against a committed per-file manifest instead of walking at all. I am not doing that now because a nested file absent from the manifest is invisible again, so it trades this failure for a different one.

Each test was shown to fail first. Reverting each fix in turn kills exactly reports an unreadable subtree in that language, with no compile errors; sources restored by copy and diffed. The tests branch on euid, since root reads through a 0o000 directory — under root the assertion becomes "the cases are still counted", so neither environment can pass by silently dropping the subtree.

make conformance and make conformance-runner-tests green at 2b3a34d.

@jeremy

jeremy commented Aug 13, 2026

Copy link
Copy Markdown
Member Author

@codex review

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.

Suppressed comments (1)

conformance/runner/ruby/runner.rb:72

  • || also defaults an explicit JSON false, so { "mode": false } is executed as a mock case. The census counts that value as non-live too, leaving matching totals and silently missing the unrecognized-mode divergence this check is meant to expose; the other runners reject it. Default only nil.
    (mode || "mock") == "mock"

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2b3a34d509

ℹ️ 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".

Comment thread conformance/runner/ruby/runner.rb Outdated
`mode || "mock"` also defaults an explicit JSON `false`, so Ruby alone ran
`{"mode": false}` as a mock case — and since the census counts that value as
non-live, the totals matched over a mode nothing accepts. Verified: Ruby
returned true where Python (fixed last round) and TypeScript both return
false; the typed runners reject a non-string `mode` when they decode it.

Same one-line shape as the Python fix, and the last language carrying it.
The assertion was shown to fail against the old predicate before being kept.
Copilot AI review requested due to automatic review settings August 13, 2026 09:57
@jeremy

jeremy commented Aug 13, 2026

Copy link
Copy Markdown
Member Author

Third round, at 00c0077.

Codex's three unresolved threads (Python rglob, Ruby Dir.glob, Kotlin walkTopDown) were against c45a686 — the head before the traversal fixes — and Copilot had raised the same three in its suppressed comments. All were already fixed in 2b3a34d; replied and resolved.

One genuinely new finding, from both bots independently: Ruby's mode || "mock" also defaulted an explicit JSON false, so Ruby alone ran {"mode": false} while the census counted it as non-live — matching totals over a value nothing accepts. Verified empirically rather than by reading: Ruby returned true where Python (fixed last round) and TypeScript return false. Fixed; Ruby was the last language defaulting on a falsy value.

Class ledger, since this is round three. Four distinct defect classes have come out of review, each now closed across every runner that had it:

  1. [] truncation counted as zero — census refuses it (6/6)
  2. nested-only tree stepping over the count check — early exits removed (5/5 that had one)
  3. walk errors swallowed — propagated (4/4 that swallowed; Go and TS were already correct)
  4. falsy mode defaulted to mock — nil/None-only default (2/2 that had it)

Scoring per AGENTS.md: the rule count grew, and so did covered call sites, every time — reach rather than accretion, which is why I fixed rather than escalated. The tripwire I named last round still stands: if a fourth round produces more silent-drop paths in these walks, the instrument question is a committed per-file manifest instead of walking, and that is a design change for a human to call, not another patch.

Green at 00c0077 locally; make check passed on Linux under LC_ALL=C.UTF-8 at 2b3a34d, and CI was fully green there (30 runs, 0 failures). Re-running both for this head.

@jeremy

jeremy commented Aug 13, 2026

Copy link
Copy Markdown
Member Author

@codex review

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Nice work!

Reviewed commit: 00c0077bc6

ℹ️ 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".

@jeremy
jeremy merged commit ba586ed into main Aug 13, 2026
46 checks passed
@jeremy
jeremy deleted the feat/conformance-case-census branch August 13, 2026 15:30
jeremy added a commit that referenced this pull request Aug 13, 2026
…iting deletion

Codex raised the first as a P1 and Copilot as a suppressed comment; both were
right, and it is the defect class this PR family exists to prevent — prose
claiming coverage CI cannot provide.

SPEC §19 said `make check-fixture-execution` (#602) "is what detects it now".
That gate was the source-text parser split out of this PR to
conformance-skips-parser-archive; the prose describing it stayed behind. There
is no such script and no such target — `make -n check-fixture-execution` exits
"No rule to make target" — so the paragraph promised all-six detection that
does not exist, while #602 is still open.

Replaced with what is actually true: each runner's case census (#742) catches a
case executed by no runner for a MECHANICAL reason, and explicitly does not
catch the deliberate all-six exclusion this section describes, because each
census counts its own skip and stays green. The roster below it is restated
rather than derived, and nothing checks it (#736) — which is why #736 waits for
#602's cross-runner manifest instead of being fixed on its own.

Separately, roster_vacuity's comment claimed the guard "buys no coverage". That
is true only when ONE side is empty. When BOTH are, `missing` and `extra` are
both empty, the comparison is trivially satisfied, and this guard is the only
thing refusing the vacuous pass — a committed self-test case covers exactly
that. The comment as written invited deleting a live guard on the strength of
reasoning that applies to a different case.
jeremy added a commit that referenced this pull request Aug 13, 2026
…them (#740)

* Account for every conformance fixture in both SPEC rosters

SPEC §19's Test Categories table and Appendix D each claim to account for
every fixture under conformance/tests/, and both had drifted. The last
three fixture-adding commits missed the convention in three different
ways: dee221c (#601) added documents_write.json and updated neither
table; b238e5e (#683) added uploads_write.json with four Appendix D
rows and no §19 row; #726 added search.json's §19 row and missed
Appendix D. Two half-applications in opposite directions is not
carelessness — CONTRIBUTING.md tells contributors to add conformance
tests and mentions neither table.

Each cell is derived from the fixture's own description citations, which
is the convention the existing rows follow. documents_write.json cites
"SPEC 18 body compaction" and "SPEC 18 rule 6", and §5's Documents
subsection already back-references it; uploads_write.json cites "SPEC
§18", "SPEC.md §5" and "SPEC §6 step 11", the same four attributions its
Appendix D rows already spell out.

Also moves the search row into its sorted position, where #726 misfiled
it between retry and schedule-entries-write.

Claude-Session: https://claude.ai/code/session_01VyZUi8zkRfhatnBYoS4eyn

* Gate both conformance-fixture rosters against git ls-files

`sync-doc-constants.rb` already does table-completeness checking:
@assertion-types wraps SPEC §19's assertion-type table in a block marker
and set-compares it against conformance/schema.json. The two fixture
rosters are the same shape one level out — a table that claims to
account for every fixture under conformance/tests/, with nothing
checking it — so they become two more block kinds rather than a new
script and a new CI step. `make doc-constants-check` already runs the
gate and its self-test, and is already in check-targets and spec-gates.

The source is `git ls-files conformance/tests/*.json`, not Dir.glob, for
tracked_markdown's reason: an untracked scratch fixture must not fail a
developer's build. Direct children only — git's pathspec `*` matches
across `/`, and a nested fixture is discovered by no runner, so
demanding a roster row for it would be documenting a claim that is not
true. That scope is also how SPEC §23's carve-out is honored:
conformance/oauth/, oauth-token/ and event-feed*/ are documented at
their own section and directory.

The two invariants differ because the artifacts differ. §19's table is a
bijection, so all of it is asserted: one row per fixture, both
directions, and category slug == basename with `_` as `-` (verified
across all 22 rows). Appendix D's rows are curated summaries that
deliberately bundle several cases — uploads_write.json legitimately has
four — so it gets coverage only, and a self-test case pins that
difference by asserting several rows for one fixture still passes.

Both tables also reject a row whose attribution cell is blank, and a
`§N` reference that resolves to no `## §N.` heading — the latter catching
a reference that resolved when written and stopped resolving when a
section was renumbered, which a reviewer of the same PR cannot see. A row
with no section reference at all is still accepted, because rejecting it
needs a carve-out for live-my-surface.json's external-governance
attribution and the carve-out list is the part that grows.

Neither table is writable: --write only ever touched line spans, and a
row here carries an owning-section attribution or a case summary only
the fixture's author can make.

Both checks reject SPEC.md as it stood before the preceding commit:

  SPEC.md:2110-2131: conformance/tests holds 22 tracked fixture(s), the
    table categorises 20; missing: `documents_write.json`,
    `uploads_write.json`.
  SPEC.md:3379-3458: no row maps these tracked fixtures to a primary
    section: `documents_write.json`, `search.json`.

Claude-Session: https://claude.ai/code/session_01VyZUi8zkRfhatnBYoS4eyn

* Keep block bodies under the pin scan, and reject colliding category slugs

Two review findings on the roster gate, both real interactions rather than
Markdown-spelling edge cases.

Block span bodies were dropped from the prose pool along with line spans, but
the two are not alike. The writer rewrites line spans only, and the block
checkers read nothing but the `|` rows, so an ordinary sentence parked inside
a roster or assertion-types block survives both untouched. Excluding the whole
body let "verified against <current pin>" sit there with no marker and no
grant — invisible to check_unmarked_pin and silently stale at the next repin,
which is the exact claim class this gate exists to catch, hidden by the gate's
own span bookkeeping. Only line spans leave the pool now.

The §19 categories table tallied FILES, which catches one fixture on two rows
but not two fixtures deriving one category. `_` and `-` collapse to the same
slug, so `foo_bar.json` and `foo-bar.json` each satisfy the per-row slug rule
while the table stops being the bijection its heading asserts. Now tallied on
the DERIVED slug — a row whose category cell is simply wrong is already
reported and still reaches that tally, so grouping by the declared cell would
both miss real collisions and invent false ones. Appendix D is unaffected: it
has no category column and deliberately allows many rows per fixture.

Both self-test cases were shown to fail against the un-fixed gate first —
reverting each fix in turn leaves exactly its own case reporting "expected
FAILURE, gate exited 0".

* Stop SPEC promising a gate this PR removed, and correct a comment inviting deletion

Codex raised the first as a P1 and Copilot as a suppressed comment; both were
right, and it is the defect class this PR family exists to prevent — prose
claiming coverage CI cannot provide.

SPEC §19 said `make check-fixture-execution` (#602) "is what detects it now".
That gate was the source-text parser split out of this PR to
conformance-skips-parser-archive; the prose describing it stayed behind. There
is no such script and no such target — `make -n check-fixture-execution` exits
"No rule to make target" — so the paragraph promised all-six detection that
does not exist, while #602 is still open.

Replaced with what is actually true: each runner's case census (#742) catches a
case executed by no runner for a MECHANICAL reason, and explicitly does not
catch the deliberate all-six exclusion this section describes, because each
census counts its own skip and stays green. The roster below it is restated
rather than derived, and nothing checks it (#736) — which is why #736 waits for
#602's cross-runner manifest instead of being fixed on its own.

Separately, roster_vacuity's comment claimed the guard "buys no coverage". That
is true only when ONE side is empty. When BOTH are, `missing` and `extra` are
both empty, the comparison is trivially satisfied, and this guard is the only
thing refusing the vacuous pass — a committed self-test case covers exactly
that. The comment as written invited deleting a live guard on the strength of
reasoning that applies to a different case.

* Require exactly three cells per roster row, and correct a self-falsifying claim

Copilot raised the cell count three times across two rounds; taking it, because
it asks for something different from the Markdown-spelling findings declined
alongside it.

Those asked the splitter to UNDERSTAND more Markdown — separator widths,
backslash parity. This asks it to REFUSE what it does not understand, which is
the direction this file already argues for: "a row the parser cannot see is a
row it silently vouches for." And it closes the pipe class as a class rather
than one spelling at a time — however a stray pipe was written, the cell count
is wrong and the row fails loudly instead of being mis-parsed quietly.

The consequence was real, not cosmetic. A raw pipe in an attribution shifts the
real section into a fourth cell and leaves the fragment before it in cells[2],
where non-`§` attributions are legitimately allowed — so the gate validated the
wrong cell and a `§99` in the actual section position was never checked, on a
gate whose whole claim is that it validates every section reference. Both
tables had it; Appendix D's free-form summaries are the likeliest place for
someone to write `supports A | B`.

Self-test cases added for both tables and shown to fail against `< 3` first.

CONTRIBUTING.md separately claimed the checklist item "is the only place this
convention was written down" — false the moment this PR also stated it in SPEC
§19 and the gate. Rephrased as the historical absence it describes.
jeremy added a commit that referenced this pull request Aug 14, 2026
…743)

* Detect a fixture case that every runner excludes (#602)

Each runner's case census (#742) answers "did THIS runner account for every
case". It cannot answer the question #602 actually asks — is any case executed
by NO runner — because a case every runner excludes leaves all six censuses
green: each one counted its own skip. Only a comparison across runners sees it.

Every runner now writes the cases it did not execute, with reasons, to
conformance/manifests/<runner>.json, and scripts/check-fixture-execution.rb
fails when a case appears in all six.

Manifests rather than parsed output because TypeScript prints no `SKIP:` line —
a skip there is `it.skip`, reported in vitest's own format — so a gate scraping
stdout would be blind to exactly one runner, in the silent direction:
TypeScript would contribute an empty exclusion set and no case could reach
all-six. Each manifest also records `executed`, asserted against the census
total by both writer and reader: without it a case a runner silently dropped is
simply absent from its exclusion set, and absent reads identically to "ran
fine".

THE ABSENCE RULE IS THE DESIGN. FULL mode requires all six manifests and fails
if any is missing — a missing manifest must never read as "that runner executed
everything", which is exactly what makes an all-six case invisible. Swift's
runner is macOS-only, so a Linux run produces five and uses PARTIAL mode: an
exclusion shared by every VISIBLE runner is a warning, never a failure, because
five-of-six is not the all-six claim and a warning cannot false-fail. Both
modes fail on zero manifests.

CI resolves it properly rather than living with the partial answer: the six
language jobs each upload their manifest and the existing fan-in job runs FULL
mode over all six. That step is ordered AFTER the results check, because the
job is `if: always()` — a language job that died before its upload would
otherwise be reported as a missing manifest, burying the real cause.

The local target depends on `conformance` rather than trusting whatever
manifests are on disk, which would let it validate last week's exclusion sets.

Maximum overlap today is 2 of 6 (#596 narrowed it), so the gate is green on
arrival and a live run only proves it can say yes.
scripts/test-check-fixture-execution.rb crafts the all-six state and every
absence, integrity and disagreement case; it already found one defect (a
non-object manifest crashed with a backtrace instead of naming the file).
Verified end-to-end too: adding one shared exclusion to all six real manifests
makes the gate fail and name every runner's reason.

SPEC §19 now describes the gate that exists, closing the gap #740's P1 found —
where the prose promised `make check-fixture-execution` after the source-text
parser it referred to had been withdrawn.

* Gate the Ruby and Python manifest uploads to the leg that runs conformance

Both jobs are matrixed and run their conformance suite on ONE version —
`matrix.ruby == '3.3'`, `matrix.python == '3.13'`. The upload steps went in
unconditionally, so on every other leg the conformance step was skipped, no
manifest was written, and `if-no-files-found: error` failed the job. Seven red
checks, all of them this.

Mirroring the condition is the fix rather than relaxing if-no-files-found: a
leg that DOES run conformance and produces no manifest is a real defect, and
that is exactly what the fan-in gate's absence rule depends on being loud.

Go, TypeScript, Kotlin and Swift have no matrix, so their uploads stay
unconditional and each artifact name remains unique.

* Keep --partial from softening a complete manifest set, and wire the target in

Two review findings from Copilot, both correct.

`--partial` describes the INPUT — "this run cannot produce all six" — not a
licence to soften the verdict. When every expected runner reported anyway (a
macOS developer passing the flag out of habit, or a CI step keeping it for
safety), "excluded by all present runners" IS the all-six claim, and the
warning path let the one state this gate exists to reject exit 0. Partial
handling now applies only when a manifest is genuinely absent. The self-test
case for it was shown to fail against the un-narrowed code first.

`check-fixture-execution` was also absent from `check-targets`: the edit that
was supposed to add it matched the first occurrence of its anchor, which was
the .PHONY line, so the target existed and `make check` never ran it. Now in
the list, verified by reading the check-targets line itself rather than
grepping the file.

* Give exclusions a real identity, own a fresh run, and self-test in CI

Four review findings, all real, two of them defects in the gate's core claim.

CASE IDENTITY IS [file, name], NOT name. Codex is right that fixture case
names are not unique: verified, "replace-omission-clears: sparse replace sends
the request verbatim with no GET" appears in THREE fixtures and the
non-idempotent POST retry name in two, while names are unique within a file.
Keyed on name alone, a runner excluding one of those collapsed two entries
into one — its own `executed + excluded` integrity check would then fail
spuriously, and worse, a name excluded by three runners in one file and three
in another would read as excluded by all six. A false failure on cases that
all run. Every runner now records the fixture file, and the gate keys on the
pair; it also rejects the same case excluded twice by one runner, which would
otherwise add up while the comparison saw a single entry.

STALE MANIFESTS. Both bots found this, and it was created by the --partial
narrowing in the previous commit. On Linux `conformance-swift` is a no-op, so
a swift.json left by an earlier macOS run over the same checkout survives
while the other five refresh; the gate then sees six manifests, stops treating
the run as partial, and compares five current exclusion sets against a stale
sixth. Silent-wrong, in a gate whose whole claim is about what the runners
actually did. The target now wipes conformance/manifests and runs the suite
itself, so "six manifests present" means "six runners reported in this run".
The reset is in the recipe, not a prerequisite: prerequisite order is not
guaranteed under `make -j`, and a reset racing the runners would delete the
output it exists to protect. check-targets therefore lists this target INSTEAD
of `conformance`, so the suite still runs exactly once per `make check`.

SELF-TEST IN CI. The fan-in job ran only the live gate, whose inputs are built
to pass; the self-test was reachable only through the Make target the workflow
deliberately bypasses. A regression making the gate accept a missing manifest
or an all-six exclusion would have left CI green — the check-targets-is-not-CI
-coverage shape, in the PR that adds the gate. It now runs as its own step.

New self-test cases: one name in two files is two cases and must NOT fire; the
same name in one file excluded everywhere still must; an exclusion without its
fixture file is rejected; one case excluded twice in a manifest is rejected.

* Reset manifests via an order-only prerequisite, not a sub-make

The previous commit had check-fixture-execution run `$(MAKE) conformance`
itself so it could wipe the manifest directory first. That worked, and cost
something I did not price: with `conformance` no longer listed in
check-targets, `conformance-kotlin` stopped being reachable from
check-targets in the graph check-gradle-serialization walks — so that gate
could no longer see one of the two Gradle invocations it exists to keep off
each other. Its self-test caught it exactly as designed ("conformance-kotlin
edge removed: expected the gate to FAIL, it passed"), which is a mutation
case earning its keep on a change nobody wrote it for.

Restructured so both properties hold. `conformance` is back in check-targets
and check-fixture-execution depends on it, restoring reachability. The reset
is now a phony target that all six language targets take as an ORDER-ONLY
prerequisite (`|`), which is what makes it correct under `make -j`: make
builds a prerequisite to completion before any dependent starts, and builds
the phony target once per invocation, so the reset cannot race the runners it
protects. A plain prerequisite of the aggregate target would not do that —
siblings may run concurrently.

It fires for a single-language run too, deliberately: `make conformance-go`
clears the directory, so it holds only what that invocation produced and the
gate goes partial rather than silently mixing runs across machines.

Verified both directions: a planted stale swift.json is wiped and regenerated,
and `make conformance-go` alone leaves only go.json.

* Let a re-run replace its own manifest artifact

Copilot flagged this on all six uploads. Re-running a job creates a new
ATTEMPT within the same run, and upload-artifact v4+ artifacts are immutable
per run — so the second attempt fails on a name conflict with the first
attempt's manifest, before the fan-in gate can run. Reachable exactly when
someone is re-running to get a red PR green, which is the worst time for the
gate to become unreachable.

`overwrite: true` is also right on the merits, not just as conflict avoidance:
the collecting gate must read THIS attempt's exclusion set. An artifact left
by a previous attempt is the CI-side version of the stale-manifest bug fixed
two commits ago, and the same answer applies — six manifests present must mean
six runners reported in this run.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

conformance Conformance test suite kotlin

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants