Skip to content

test(toon): vendor the spec 4.4 fixtures and gate on them (stage 1 of #184) - #195

Merged
Oaklight merged 4 commits into
masterfrom
test/toon-spec44-conformance-gate
Oct 13, 2026
Merged

Oaklight merged 4 commits into
masterfrom
test/toon-spec44-conformance-gate

Conversation

@Oaklight

@Oaklight Oaklight commented Oct 13, 2026 •

Copy link
Copy Markdown
Owner

Refs #184 (stage 1 of a staged plan).

Why

toon/ was green only because #183 pinned the toon_format reference. That pin stops the comparison from floating; it does not make the module produce the current TOON format. Measured against the official spec 4.4 conformance fixtures, toon/ passes 350/564 cases. The gap to full conformance is a rewrite across several spec sections, so this PR is deliberately stage 1: it lands the fixtures, a gate that makes the gap visible and monotonic, and the one normative MUST that is cheap and unambiguous. Later stages move section by section against a fixed, shrinking baseline.

Scope (exactly stage 1)

  1. Vendored fixtures. toon/fixtures/spec/{encode,decode}/ are the official conformance fixtures from toon-format/spec, copied verbatim (byte-identical to the copy toon-python runs in its own CI). Pinned at commit d09c3d33e37389345a74bd96981eb96dd9be6c5e (tag v4.4.0), spec version 4.4: 160 encode + 404 decode = 564 cases. toon/fixtures/README.md records the commit, version, counts, and upstream MIT licence so the pin is auditable. The whole set is 196 KB, so it ships in the repo — no fetch script is needed.

  2. A ratchet gate, not a report. toon/test_toon_conformance.py runs all 564 cases in-process with no network and no reference library. toon/conformance_baseline.json records today's failures (214 IDs, passing_total 350). Failures print §section fixture/id name, so a stage can see which cases it moved. --report prints the section table; --update-baseline regenerates the file.

  3. The §9.1 MUST. Spec §9.1 requires encoders to emit key: [] (object field) and [] (root) for an empty array and forbids the legacy key[0]: header. The encoder already used the canonical form for root and plain object-field empty arrays, but the list-item object path still emitted key[0]:, and an empty-string key was treated as no key, so {"": []} encoded as bare []. Both are fixed. This is the only behavior change in the PR and it moves two conformance cases:

    • encode/arrays-objects[5] §10 — [{"data": [], "name": "Ada"}] now emits - data: [] instead of - data[0]:
    • encode/arrays-primitive[3] §9.1 — {"": []} now emits "": [] instead of []

How the ratchet is enforced

The gate asserts four things about the committed baseline:

  • every case not in it passes;
  • every case in it still fails, so a fix that starts passing fails the build until the baseline is shrunk;
  • the run's pass count is at least passing_total;
  • the committed failing set is a subset of the baseline at the branch point, and passing_total does not decrease. The branch point is the merge base of HEAD with origin/master (falling back to a local master when origin/master is absent, since a dev branch may be behind), or the ref tip when shallow history hides the merge base.

That last check is what closes the hole in the first version of this PR: --update-baseline used to regenerate the file from the current run, so injecting a regression and then running that routine command committed a grown baseline and left the gate green. Now a regenerated baseline is rejected against the branch point, and --update-baseline additionally refuses to add IDs to failing unless --allow-growth is passed. Growth therefore takes a deliberate, visible act; a stage that genuinely fixes a section still shrinks the set with the same command.

Ref resolution, and the local-dev case. origin/master is preferred, then a local master. A local master that is behind is fine: the merge base with HEAD is the fork point, which is the correct thing to ratchet against. When neither ref exists (a shallow or single-branch clone, as in the CI test job) git fetch origin +master:refs/remotes/origin/master is run; the destination is named explicitly because a single-branch clone's refspec is limited to its own branch, so a bare git fetch origin master exits 0 without creating origin/master. If that fetch fails the test fails — it never skips, so the gate cannot quietly turn itself off. This is the only part of the gate that touches git; running the fixtures stays offline.

What exactly is compared, and what the first PR after this one sees. The subset check compares the committed baseline against the file at the resolved base commit: git merge-base HEAD <base-ref> when git can compute it, otherwise the tip of that ref. That resolve is to the branch point locally — in CI on a pull_request it is master at CI time:

  • Locally, with full history, the base commit is the merge base, i.e. the fork point: the last master commit the branch was based on.
  • In CI, HEAD is the synthetic merge commit whose first parent is master at CI time. origin/master is fetched when absent, and the merge base resolves to that first parent; if a shallow checkout leaves too little history for the merge-base walk, the fallback is the origin/master tip, which is the same commit.

So the check runs against the branch point locally and against master-at-CI-time in CI; both are commits the branch is meant to build on.

Once #195 lands, master carries this 214-entry baseline. The next toon stage's branch point (locally) and the first parent of its CI merge commit both name a master commit that includes that baseline, so test_no_regression_against_base goes live for it: its committed failing set must be a subset of the 214 entries, and passing_total must not drop. A stage that fixes a section shrinks the set and passes; a stage that regresses and regenerates is rejected. The resolution order already covers this — the branch point carries the merged baseline — so nothing special is needed for the first PR after this one.

If the vendored fixture set is ever swapped, the failing IDs are not comparable across sets, so the subset check steps aside for that one deliberate, visible change. It is not a general escape hatch: with the set unchanged, growth is always rejected.

Version bump: 0.4.0 → 0.5.0 (minor)

The encoder's output changes for a class of inputs. That is a behavioral change to the module's contract, not an internal fix, so a minor bump is the honest one — a patch bump would understate it even though the new output is the spec-correct one. make manifest was run; the version lives in the frontmatter and is set by zerodep bump, not hand-edited.

The module docstring claimed to be a "drop-in replacement for toon_format". It still is not one at the end of stage 1, so the claim is narrowed to state the spec version the module targets and to point at #184 for the remaining work.

Explicitly left for later stages

  • Every other failing section. This PR does not touch the decoder beyond what the gate observes.
  • Issue toon: the list form does not round-trip a nested object value #188 (a list form does not round-trip a nested object value) stays open; it is not folded in.
  • The remaining §9.1 case, encode/arrays-primitive[2] ({"": [1,2,3]} → wants ""[3]: 1,2,3), is a non-empty array with an empty-string key. Fixing it needs the general falsy-key path in _format_header/_enc_object, which also changes §8 output (encode/objects[29]). That is a second section, so it is left for a later stage rather than pulled into this one. The gate keeps it in the baseline.
  • The bench-toon extra pin is unchanged, at your request. The vendored fixtures are 4.4 while the pinned reference is 1.0.0 (fixtures 4.1.2); the gate does not use the reference, so no pin move is required here.

Numbers: the issue vs this branch

issue (#184) branch, before §9.1 branch, after §9.1
encode 115/160 123/160 125/160
decode 226/404 225/404 225/404
total 341/564 348/564 350/564

The fixture set is the same size (160/404), and the section-level agreement is close (§6 = 42 and §5.1 = 12 match exactly), but I could not reproduce the issue's exact totals. The likely reasons are measurement methodology (this gate requires the specific ToonDecodeError for shouldError cases and compares §2 key order) and the module's state when the issue was written. I treat the numbers on this branch as authoritative for this snapshot, because they come from a checked-in harness anyone can re-run.

Section-by-section failures (fail before → after is the §9.1 change)

encode — 125/160

§ pass/total fail before → after
1.2 0/1 1 → 1
2 13/14 1 → 1
7.1 4/8 4 → 4
7.2 27/36 9 → 9
7.3 8/9 1 → 1
8 4/5 1 → 1
9.1 11/12 2 → 1
9.3 16/17 1 → 1
9.4 8/10 2 → 2
9.5 6/14 8 → 8
10 4/9 6 → 5
11 15/16 1 → 1

Sections with no failures: 9.2, 11.1, 12, 15.

decode — 225/404

§ pass/total fail before → after
1.2 0/1 1 → 1
4 25/30 5 → 5
5 5/12 7 → 7
5.1 4/16 12 → 12
5.2 9/19 10 → 10
6 10/52 42 → 42
7.1 11/23 12 → 12
7.4 15/21 6 → 6
8 8/23 15 → 15
9.3 17/20 3 → 3
9.4 7/10 3 → 3
9.5 0/12 12 → 12
10 5/8 3 → 3
11.2 3/4 1 → 1
12 21/32 11 → 11
14.1 8/16 8 → 8
14.2 15/26 11 → 11
14.3 1/5 4 → 4
14.4 10/22 12 → 12
15 5/6 1 → 1

Sections with no failures: 9.1, 9.2, 11, 16.

Test plan

All commands run under conda activate zerodep (Python 3.10.20).

python toon/test_toon_conformance.py --report          # section table, 350/564
python -m pytest toon/test_toon_conformance.py -q      # 6 passed
make test-toon                                         # 138 passed
make lint                                              # All checks passed!
pre-commit run --all-files                             # ruff, ruff format, ty, complexipy: Passed
python -m pytest --ignore-glob="*/test_*_benchmark.py" toon/ -q   # CI-style collection: 138 passed
python toon/test_toon_conformance.py --update-baseline # idempotent, no diff

Ratchet checks, each verified by a temporary mutation that was reverted:

mutation result
passing case added to the committed baseline test_baseline_entries_still_fail fails
failing case dropped from the committed baseline test_no_new_failures fails
passing_total raised above the floor test_pass_count_never_drops fails
regression injected in _join_values, then --update-baseline command exits 1, file unchanged, test_no_new_failures still fails
baseline hand-grown vs the branch point test_no_regression_against_base fails
branch point older than current (normal shrink) test_no_regression_against_base passes
no local origin/master resolver fetches; a failed fetch raises AssertionError instead of skipping

CI coverage. The test job runs pytest --ignore-glob="*/test_*_benchmark.py" -v -rs over testpaths, which includes toon/, so toon/test_toon_conformance.py is collected on every matrix version with no workflow change. There is no marker and no skip; the whole gate is ~0.06 s.

Suggested stage 2, ranked by cases bought

Per-section (encode + decode combined), highest first:

rank section failing where
1 §6 header grammar 42 decode
2 §9.5 keyed tabular 20 8 encode + 12 decode
3 §8 objects 16 1 encode + 15 decode
4 §7.1 key encoding 16 4 encode + 12 decode
5 §14.4 non-strict recoveries 12 decode
6 §5.1 comments 12 decode
7 §12 trimming 11 decode
8 §14.2 header defects 11 decode
9 §5.2 list-item lines 10 decode
10 §7.2 value quoting 9 encode

Grouped by theme: §6 (42) is the biggest single win and is decode-only. §9.5 (20) is the highest-value section that spans both directions, so it pays off twice. §14.x (35) is a coherent block once the decoder is touched, and §5.x (29) plus §6 (42) are one cluster — the comment/blank/header grammar the decoder never implemented.

@Oaklight

Copy link
Copy Markdown
Owner Author

Independent verification of the fixture provenance, since that is the part a logic review would not catch. I cloned toon-format/spec separately, checked out d09c3d33e37389345a74bd96981eb96dd9be6c5e, and compared every vendored file byte for byte:

23 upstream JSON files, 23 vendored, 0 missing, 0 differing
all 23 sha256 hashes match

So the ratchet is gated on the real spec fixtures at the recorded commit, not on a transcription. For the reviewer: the thing worth attacking in this PR is not the fixture set but the ratchet's enforcement — whether the baseline can grow, and whether CI would actually notice if a passing case regressed. Everything else is a consequence of that.

@clementine-oaklight clementine-oaklight Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed the gate, not just the fixtures. Numbers first, because they are the part I could check hardest.

The arithmetic is right, and it corrected mine. The gate's rule on the 158 shouldError decode cases is stricter than mine was: it requires ToonDecodeError specifically, where I had accepted any exception. Re-running this harness against master's module gives 123/160 encode, 225/404 decode, 348/564 — exactly the "before §9.1" row in the body — and 125/160 + 225/404 = 350/564 after, matching the baseline. So the PR's table is correct and #184's decode figure was 9 cases optimistic. I have corrected the issue; the nine cases are filed separately as #198, because raising ValueError where the contract says ToonDecodeError is an API inconsistency that the baseline is currently absorbing.

Fixtures are what they claim. diff -rq against the copy toon-python runs in its own CI is clean for both encode/ and decode/, so "copied verbatim" holds rather than being asserted.

The gate cannot be silently skipped. Two things I tried:

  • Moving toon/fixtures aside fails the build with 3 failed — test_no_new_failures, test_pass_count_never_drops and test_baseline_entries_exist_in_fixtures. A missing fixture set is loud.
  • It runs in CI for real: the ratchet assertions show as PASSED in the 3.11 job of run 38407431752, not just in the Makefile target.

The ratchet can grow, and that is the one change I would ask for. --update-baseline regenerates from the current run, so a regression can be absorbed by committing the regenerated file. Demonstrated: I injected a regression in _join_values (appending a space to the delimiter), the gate failed correctly with test_no_new_failures and test_pass_count_never_drops, and then the documented command —

python toon/test_toon_conformance.py --update-baseline

— took it from 350 to 309 passing, 214 to 255 failing IDs, and the gate went back to 5 passed. Nothing in CI objects. So the ratchet holds against carelessness and not against a commit that means it.

The fix is cheap and keeps the workflow: read the baseline from the merge base (git show origin/master:toon/conformance_baseline.json) and assert the committed failing set is a subset of it, with passing_total non-decreasing. Growth then needs an explicit override rather than a routine command, and a stage that genuinely fixes a section still shrinks it the same way. A weaker alternative is a CI step diffing the baseline against the base branch and failing on added IDs.

Everything else I checked is sound: the fixture totals assertion ties the vendored set to the pinned baseline, the floor and the "must still fail" test are both live, and the §9.1 change moves exactly the two cases the body claims.

@elena-oaklight elena-oaklight Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Attacked the ratchet as asked. It holds against everything accidental, and there is one deliberate path through it.

The gate is genuinely self-contained. It collected and passed with toon_format absent from the interpreter (ModuleNotFoundError on import, 5 passed), so the comparison does not depend on a live reference. Fixtures are 23 files / 196 KB, and toon/fixtures/README.md records the upstream repo, path, commit d09c3d33e37389345a74bd96981eb96dd9be6c5e (tag v4.4.0), spec version, case counts and MIT licence.

A missing gate fails; it does not skip. Moving toon/fixtures/spec/ aside gives 3 failed, 2 passed, not a green skip. That is the failure mode you were worried about, and it is closed.

Baseline attacks are all rejected:

attack result
record a currently-passing case as failing test_baseline_entries_still_fail fails
a baseline entry naming no fixture test_baseline_entries_exist_in_fixtures fails
real regression (revert §9.1), baseline untouched test_pass_count_never_drops fails

But the baseline can still grow, and the floor can be lowered, in one file. test_pass_count_never_drops reads its floor from BASELINE["passing_total"] — the same file the gate protects. So I reverted the §9.1 fix in a scratch copy (real regression: 350 → 348 passing, 214 → 216 failing) and then rewrote only toon/conformance_baseline.json to match the regressed reality. The gate came back 5 passed in 0.03s. The baseline grew by two entries and the floor dropped by two, and nothing objected.

That is the one place the design does not do what its own summary says. It is not accidental — a deliberate edit is visible in the diff — but the diff is the only thing standing there, and the ratchet's promise is that the number cannot move backwards. Cheap hardening: put the floor outside the mutable file, e.g. a MIN_PASSING = 350 constant in test_toon_conformance.py asserted against both BASELINE["passing_total"] and the measured count, and a cap on len(BASELINE["failing"]). Then raising a stage is a deliberate two-file change and a baseline-only edit cannot walk the ratchet back. Your call whether that is worth it; I would not hold the stage for it.

The normative change is real and correctly scoped. Measured, master vs this head:

value master head
[{'a': []}] '[1]:\n - a[0]:' '[1]:\n - a: []'
{'x': [{'a': []}]} 'x[1]:\n - a[0]:' 'x[1]:\n - a: []'
[{'a': [], 'b': 1}] ... - a[0]:\n b: 1 ... - a: []\n b: 1

So the legacy key[0]: header for an empty array in list-item position is gone, which is §9.1.

One extra fix in the same diff that the description does not mention, worth keeping because it is a data-loss bug: _enc_array tested if key, which is false for an empty-string key, so {'': []} encoded as a bare [] and the key disappeared. if key is not None corrects it — master '[]' → head '": []'. That belongs in the PR description or a commit line, since it is a separate user-visible fix riding along.

Counts agree: 350/564 (160 encode + 404 decode), up from 348, with 214 recorded failures. toon is 0.5.0 and the manifest entry carries a derived last_updated. 137 passed across both toon test files at the pin. No other section touched, and #188 stays open.

Approving. The ratchet is the right shape for a staged rewrite, and the floor constant is the only thing I would tighten.

@Oaklight
Oaklight force-pushed the test/toon-spec44-conformance-gate branch 2 times, most recently from f6a7e4e to a37ef87 Compare October 13, 2026 14:04

@clementine-oaklight clementine-oaklight Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Re-checked at a37ef87. All three of my asks are closed, and the attack I used is now the thing that fails.

1. --update-baseline refuses growth. I injected the same regression as before (_join_values appending a space to the delimiter). The gate failed with test_no_new_failures and test_pass_count_never_drops, and the documented command then exited 1 naming the 41 IDs that would be added — and conformance_baseline.json was byte-identical afterwards. That same sequence used to take the baseline from 350 to 309 and go green. Closed at the exact step I opened it.

2. The subset check works, and I checked it isn't merely dormant. It reads the baseline at the branch point via merge-base HEAD origin/master, and returns early when the base has no baseline — correct for this PR, since it is the commit that introduces the file, but it means the check does nothing on this branch. So I simulated the post-merge state: pointed origin/master at a commit carrying the baseline, then added two IDs to the working-tree baseline. test_no_regression_against_base failed and named them. Ref and file restored afterwards. So the mechanism bites from the next commit onward, which is the only place it can.

3. Ref resolution fails rather than switching itself off. In a repo with no master, no origin/master and no remote, the gate fails with "cannot resolve a base ref to ratchet against" instead of skipping. Constructed that repo and confirmed.

One robustness note, minor. The fallback fetch tests fetch.returncode != 0 or not _has_commit("origin/master"), and in a single-branch clone those come apart: git clone --branch <x> --depth 1 configures a refspec limited to that branch, so git fetch --quiet origin master exits 0 and still creates no origin/master. The gate then fails even though the base is reachable. Reproduced. It does not bite this repo's CI — all six conformance tests PASSED in run 38410239724, and the fact that this PR's gate resolves a base at all is the proof that Actions' checkout gives a usable ref. But the failure mode is a red gate where the base is fetchable, which would read as a broken test rather than a missing ref. One line if you want it: git fetch --quiet origin master:refs/remotes/origin/master, or read FETCH_HEAD.

Nit. The case_counts mismatch early-return means a vendored-fixture change turns the subset check off for that commit. Defensible — IDs are not comparable across two fixture sets — but it is the one path where a fixture refresh plus a baseline regeneration could move the floor, so a line in the docstring saying a fixture bump needs a manual re-baseline would close the loop.

Approving. The numbers also line up with my earlier measurement: the guard reports 41 cases for the same regression that previously produced the 350-to-309 walk-back.

@elena-oaklight elena-oaklight Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Re-checked at a37ef87. All three mechanisms do what you say, and there is one thing about the branch-point check worth stating plainly.

The subset check is inert on this branch, and that is correct. _base_baseline() resolves the merge base with origin/master — d8a36bc — and that commit has no baseline, so test_no_regression_against_base returns early. I confirmed the consequence by re-running my earlier attack unchanged: revert the §9.1 fix and rewrite only conformance_baseline.json, and this head still gives 6 passed. So the hardening cannot demonstrate itself in the PR that introduces it, which is what the test's own docstring says and is the right semantics for a gate that needs a predecessor.

It works the moment a predecessor has a baseline. I simulated the post-merge state in a throwaway clone with origin/master pointed at a37ef87, then applied the identical attack — revert §9.1 (350 → 348 passing, 214 → 216 failing) and regenerate only the baseline. The gate now returns 1 failed, 5 passed, failing at the assert not grown on line 301. That is the attack that walked straight through the previous head, and it is closed. Activation is deferred by exactly one commit, not broken.

--update-baseline refuses to grow. Pristine baseline (350) against the regressed tree: exit code 1, the file byte-unchanged, and a message naming both cases that would be added (encode/arrays-objects[5], encode/arrays-primitive[3]). With --allow-growth it writes 348 / 216 as intended, so the deliberate path is explicit and the accidental one is shut.

Ref resolution fails rather than skips. Deleted both origin/master and master and pointed the remote at a nonexistent path: AssertionError: cannot resolve a base ref to ratchet against: neither origin/master nor master exists and git fetch origin master failed (...). No skip, and the message carries the git stderr, so a shallow clone in CI will say so instead of going green.

One residual, and I would not hold the merge for it. The case_counts early return on the fixture set runs before the passing_total >= base assertion, so a PR that re-vendors the fixtures can lower the floor in the same commit — the ratchet comparison is skipped exactly when the case population changes. The exemption is defensible, since comparing IDs across different case populations is meaningless, and the window is that one PR: once it lands, the next PR's branch point carries the new floor and the check is live again. If you want it closed, key the floor to the fixture digest so a new fixture set has to declare its own floor explicitly rather than inherit the exemption.

Everything else holds: 6 gate tests, self-contained (still passes with toon_format absent), fixtures absent still fails rather than skips, and the --allow-growth flag is separate from --update-baseline rather than folded into it.

Approving at a37ef87. The gate is now monotonic by construction instead of by convention, which is the thing I was worried about.

@clementine-oaklight clementine-oaklight Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Final look at 46d5bd58. Both one-liners are in and the refspec fix works in the exact case I reproduced it in.

The refspec fix, A/B in a single-branch shallow clone. Same clone, same base, only the test file swapped:

  • old code (a37ef87): git fetch --quiet origin master exits 0, creates no ref, test_no_regression_against_base fails with "cannot resolve a base ref", and origin/master is still absent afterwards.
  • new code (46d5bd5): six passed, and origin/master exists at a5a4907 afterwards.

So the fetch now lands where the code looks, and the failure mode is gone rather than merely rarer. The comment above it says exactly why, which is the part that keeps it from being undone later.

The fixture-swap note is in the docstring, on both the test and the branch — a fixture bump stands the subset check aside deliberately, so it is a manual re-baseline that gets reviewed, not something --update-baseline produces. That closes the last path I could see where a refresh plus a regeneration moves the floor.

Everything from my earlier pass still holds at this head: --update-baseline exits 1 and leaves the file byte-identical when the run would add IDs, the subset check bites when the base carries a baseline (simulated by pointing origin/master at a commit that has one, then adding two IDs — it failed and named them), and ref resolution fails rather than disabling the gate when nothing is fetchable. Locally six passed; CI is green on all five checks at 46d5bd58 (run 38411877754).

For completeness on my local run: pytest toon/ reports 141 passed and 12 errors, all fixture 'benchmark' not found — pytest-benchmark is not in my venv, and CI installs it via the bench extra, so that is my environment and not the tree.

Approving at 46d5bd58. Elena's approval is at a37ef87, so she needs the same re-check at this head.

Master is green for toon/ only because #183 pinned the toon_format
reference. That pin stops the comparison from floating, but it says
nothing about the format toon/ produces: measured against the official
conformance fixtures the module passes 348/564 cases, and most of the
gap is unimplemented spec sections.

Vendor the fixtures from toon-format/spec v4.4.0 under toon/fixtures/ so
the gate runs offline against a fixed revision, and add
toon/test_toon_conformance.py as a ratchet. toon/conformance_baseline.json
records today's 216 failures; every case outside the baseline must pass,
a baseline case that starts passing fails the build loudly, and the pass
count may not fall below the recorded floor. Failure output names the spec
section and the fixture ID so later stages can work section by section.

The vendored set is 196 KB, so it ships in the repo; no fetch script is
needed. toon/fixtures/README.md pins the upstream commit and licence.

Refs #184
Spec §9.1 requires encoders to emit `key: []` (object-field position) or
`[]` (root position) for an empty array and forbids the legacy `key[0]:`
header. The encoder already used the canonical form for root and plain
object-field empty arrays, but the list-item object path still emitted
`key[0]:`, and an empty-string key was treated as no key, so `{"": []}`
encoded as bare `[]`.

Emit the canonical form in both cases, and add regression tests next to
the existing empty-array tests. This is the one behavior change in this
stage: the conformance baseline drops the two cases it fixes
(encode/arrays-objects[5], encode/arrays-primitive[3]) and the pass count
rises from 348 to 350.

Bump toon to 0.5.0 (minor). The encoder's output changes for a class of
inputs, which is a behavioral change to the module's contract even though
it aligns it with the spec; a patch bump would understate it. Also narrow
the module docstring, which claimed to be a drop-in replacement for
toon_format, to state the spec version it targets and point at #184 for
the remaining conformance work.

Refs #184
… point

--update-baseline regenerated the file from the current run, so injecting
a regression and then running that routine command committed a grown
baseline and left the gate green. The ratchet held against carelessness
but not against a commit that meant it.

The committed baseline now has to satisfy two more properties:

- its `failing` set must be a subset of the baseline at the branch point
  (merge base with origin/master, falling back to a local master), and
  `passing_total` must not decrease. Regenerating the file after a
  regression no longer helps: the regenerated set is rejected against the
  branch point.
- --update-baseline refuses to add IDs to `failing` unless --allow-growth
  is passed, so the routine command can only shrink the set.

The base ref is fetched when no local base ref exists; a failed fetch
fails the test instead of skipping it, so the gate cannot silently turn
itself off. Growth is now a deliberate, visible act.

Refs #184
… path

Two follow-ups from review of the ratchet.

The fallback used a bare `git fetch origin master`. In a single-branch
clone the configured refspec is limited to that branch, so the fetch
exits 0 without creating `origin/master`, and the gate failed with
"cannot resolve a base ref" for a ref that was in fact reachable. Name
the destination explicitly so the fetch either produces the ref or fails
for a real reason.

Also record that a fixture swap stands the subset check aside by design
(the case counts no longer match), which makes a fixture bump a manual,
reviewed re-baseline rather than a routine one.

Refs #184
@Oaklight
Oaklight force-pushed the test/toon-spec44-conformance-gate branch from 46d5bd5 to 0703973 Compare October 13, 2026 14:21

@elena-oaklight elena-oaklight Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Re-checked at 46d5bd5. Confirmed it is a fast-forward from a37ef87 (git merge-base --is-ancestor passes) and the diff is exactly the two changes: the refspec one-liner with its four comment lines, and two docstring/comment notes. Nothing else moved.

The refspec fix is right, and I reproduced both sides in a real shallow single-branch clone. Built with --depth 1 --single-branch (verified: remote.origin.fetch covers only the checked-out branch, .git/shallow present, origin/master and master both absent), at the same commit whose only difference is the fetch line:

pre-fix a37ef87 post-fix 46d5bd5
gate in a single-branch clone 1 failed, 5 passed 6 passed
failure cannot resolve a base ref to ratchet against: neither origin/master nor master exists and git fetch origin master failed (no output) —

And the mechanism is visible directly: in that clone the bare git fetch --quiet origin master exits 0 and refs/remotes/origin/master never appears, so the code took the exit-0 branch and then failed the _has_commit check. Exiting 0 without creating the ref is precisely the shape that makes this dangerous — the gate reported a reachable base ref as missing, which is the loud direction but the wrong one, and it would have turned the ratchet off with a message blaming the environment rather than the code. Naming the destination removes it. The shallow-clone side effect is the documented one: with no common ancestor to compute, the base resolves to the master tip via the fallback, which is the right notion of "branch point" in CI anyway.

The fixture-swap note answers my residual. I had flagged that the case_counts early return runs before the passing_total assertion, so a fixture bump plus a regeneration could move the floor. Making that path explicitly "a manual re-baseline, not a routine one" in the docstring is the documentation half of the fix, and it is the half that matters for a reviewer reading the guard. The path itself is still open by construction, since failing IDs genuinely are not comparable across case populations — but it is now labelled as deliberate, which is what I asked for.

Re-verified the rest of the gate at this head, since a one-line change to the resolver touches every rule that depends on the base:

  • base with a baseline present (origin/master → 46d5bd5), §9.1 reverted, pristine baseline: 2 failed, 4 passed — the regression is caught.
  • --update-baseline on that tree: exit 1, file byte-unchanged.
  • my earlier attack (revert §9.1, regenerate only the baseline, base has one to compare against): 1 failed, 5 passed, failing at assert not grown. Same closure as the previous head.

Counts at this head: toon/test_toon_correctness.py 132 passed, toon/test_toon_conformance.py 6 passed, 138 with the reference at the pin. CI on this run: lint plus 3.10-3.13 green, 7574 passed, 34 skipped, 7 xfailed on 3.12.

Approving at 46d5bd5. Good catch on the single-branch clone — that one only bites in a clone shape neither of us was testing, and the reproduction you built is what makes it reviewable rather than a guess.

@Oaklight
Oaklight merged commit 8afcf3d into master Oct 13, 2026
6 checks passed
@Oaklight
Oaklight deleted the test/toon-spec44-conformance-gate branch October 13, 2026 14:24
@elena-oaklight

Copy link
Copy Markdown
Contributor

Correction for the record, since my review above names 46d5bd5.

The head moved while I was reviewing. You pointed me at 46d5bd5; by the time I submitted, the branch was at 0703973b, rebased onto master so it now carries soup 0.8.0. GitHub recorded this approval against 0703973b, which is the current head — so the approval is not stale, but the SHA in my text is not the SHA it sits on.

The substance is unchanged, and that is checkable rather than asserted: git diff 46d5bd5 0703973b -- toon/ is empty. The entire change under review is byte-identical between the two SHAs. The rebase moved only the base — soup/soup.py, soup/test_soup_correctness.py (both from #194 via master) and the manifest's soup entry and generated stamp. Everything in my review of the refspec fix, the ratchet and the fixture-swap note applies verbatim.

Re-ran at 0703973b to confirm rather than infer: 138 passed (132 correctness + 6 conformance) with the reference at the pin.

CI on 0703973b is now complete and green — lint, 3.10, 3.11, 3.12, 3.13, GitGuardian.

One small description gap, not a blocker: the PR also touches Makefile, adding toon/test_toon_conformance.py to both the test and test-toon targets. Sensible and it is the right place for it, but it is a third file in a PR described as a test-only gate, so it is worth a line in the body when this merges.

No re-approval needed — the approval already sits on 0703973b.

@Oaklight

Copy link
Copy Markdown
Owner Author

Merged as 89c1c53 + 0be70aa + ad8db4a + 8afcf3d. Your correction is right and I checked the same thing by a different route before merging: git diff d8a36bc 46d5bd5 -- . ':!manifest.json' and git diff 883b7cf 0703973b -- . ':!manifest.json' hash identically (ae08e1a7d30dd7c6), so the review content and the merged content are the same bytes. Worth having both checks on the record given the head moved under you.

On the Makefile: fair, and I will take the note forward. The gate's wiring into make test and make test-toon is what makes it run at all — a fixture set and a test file that nothing invokes is the same failure as a gate CI skips — so it belongs in the body rather than being inferred from the diff. Stage 2 onward will say so.

Changelogs are updated in both doc worktrees (9280322 on docs_en, 0cdaab5 on docs_zh), covering the gate, the §9.1 change, both soup fixes and the CI trigger change.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant