the ungradeable defect was in five surfaces, not three - #16
Merged
Merged
Conversation
suitedata.load_cells divided by the count of gradeable rows without checking it was non-zero, so a cell whose completed trials were all harness faults raised ZeroDivisionError out of the one loader that table, matrix and live all sit on. Reproduced from the validation grid's own three ungradeable rows: FILES=<3 rows from runs/probe.jsonl> make table suitedata.py:188 in load_cells -> ZeroDivisionError During a live battery that is the first scenario whose completed trials are ceiling hits, which is exactly when the viewer is wanted. probe.py already guarded the same case; the shared loader did not. 0.0 alone would have been the other half of the same defect -- 0% reads as floor, and a scenario at floor gets dropped from the fixture. So the cell carries `gradeable`, one `pass_str` formatter renders the absence as an em dash, and `pass_attr`/`pcol` take the cell rather than the number so it is dimmed instead of painted red. The same exclusion applies one level up: arm_rollup averages over scenarios that have a rate, and the Pareto frontier and the cost/ quality scatter drop cells whose quality axis is absent. Refs #8
…ures docs/HANDOFF.md records this defect found in three surfaces and fixed by one shared suitedata.is_ungradeable(). report.py -- the publishable markdown scoreboard -- was not one of them, and still averaged all_pass over every row. Measured on runs/probe.jsonl, cli-cli-13057 (3 of 5 trials killed at the adapter's 2,700 s ceiling): probe.py / analyze.py / suitedata 1.00 make report 0.40 0.40 is inside the registered band [0.25, 0.80] and 1.00 is outside it, so the surface meant for publication disagreed with the go/no-go verdict about whether that scenario discriminates. Overall pass@1 for the grid moves 0.86 -> 0.88. is_ungradeable is imported, not restated. An `ungradeable` column now sits beside every rate it was computed without, because an exclusion nobody can count is indistinguishable from data that never existed, and a cell with no gradeable trial prints an em dash rather than 0.00. Also here, same file: - The document now states its own inputs. analyze.py refuses rows that are not purpose=experiment; this is the working scoreboard so refusing is wrong, but 120 validation rows rendered a complete scoreboard with no trace of what it had read. Header line, blockquote, stderr warning. - st.mean(r['duration_s'] ...) raised KeyError on rows predating the field, alone among the reads in that row. Closes #9, closes #12
suitedata.calibration() excludes adapter-fault trials, and says why: a
trial the harness stopped has a transcript truncated by construction, so
against the proxy's complete record it reads as a ~100% accounting error,
which is not what H4 measures and buries the disagreements that are real.
calibrate.py -- the registered gate, the one that decides whether P2
opens -- iterated every result row. Same two files, runs/probe.jsonl and
runs/probe.proxy.jsonl:
runs aggregate worst
make calibrate 120 19.571% 100.000%
suitedata.calibration 117 9.484% 87.483%
docs/HANDOFF.md reports "19.6% aggregate" as the H4 result. That is the
un-excluded figure. H4 fails either way -- call counts are 5319 adapter
against 5646 proxy after exclusion -- but two surfaces disagreeing by 2x
about a registered quantity is the defect, not the number.
They now agree exactly. The excluded count is printed rather than
dropped, per exclusion criterion 1.
Also: r['arm'] read directly where analyze.load and suitedata.load_rows
both default it, so a row written before the arms work raised KeyError.
Closes #10, closes #13
check_harness_exclusion pointed at analyze.harness_excluded alone. The invariant it was standing in for is repo-wide -- "there is now one is_ungradeable() in suitedata, imported by the others" -- and two surfaces drifted from it without CI noticing, both fixed in the two commits before this one. The new check builds one cell of five trials: two killed by the harness, two passes, one real failure. Excluded, the rate is 67%; scored, it is 40%. Every surface that reports a rate is then run against it -- analyze, suitedata, report, probe, calibrate -- and must land on the same number. The all-faults case is asserted from the other side: no rate at all, an em dash, "harness broke", and no ZeroDivisionError. Each of the five defects was re-introduced and confirmed to fail it: report.pass_rate ignores ungradeable -> 0.40, want 0.667 calibrate compares every row -> 5 of 5, aggregate non-zero probe scores harness faults -> 40% over 5 trials, KEEP pass_str renders an absent rate as 0% -> '0%' suitedata divides by the survivors -> ZeroDivisionError selftest: 26/26. Closes #11
/tmp/adh-ci-steps.sh is guessable and world-writable-adjacent: on a shared box it can be pre-created as a symlink, or replaced between the write and the bash that executes it with the invoking user's privileges. mktemp -d, and the python shim moves into the same directory so one trap cleans up both. The script already used mktemp for the shim, so the pattern was there; this path was missed. make ci-local: all steps passed, including the A2 == A3 byte assertion. Closes #14
Found reviewing PR #5, whose content had already landed via #6. - AGENTS.md said the TUI is vendored from leather. Upstream moved to pane in 2c0f9d9; README.md and CHANGELOG.md already said so, the routing table agents actually read did not. Same correction in HANDOFF §6. - HANDOFF §1 pinned main at b4ae1ca, two re-vendor commits behind. - HANDOFF §4's probe block put "16 ceiling" under the corrected column with no arrow. cli-cli-13057 moved into ceiling, so 16 is the before-value; the run prints 17. - HANDOFF §2 quoted 19.6% for H4. That was calibrate.py reading rows that exclusion criterion 1 drops. It is 9.5% over 117 gradeable trials. - HANDOFF §4 said the ungradeable defect turned up in three surfaces. It was five; the other two are fixed earlier in this branch. Closes #15
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
docs/HANDOFF.md§4 records a defect — a row nothing graded carriesall_pass=False, so averaging over every row scores a harness fault as a modelthat got the task wrong — found in three surfaces and fixed by one shared
suitedata.is_ungradeable().It was in five. The two that were missed are the publishable scoreboard and the
gate that decides whether the experiment can run.
What was wrong
report.py(#9) —make report, onruns/probe.jsonl:cli-cli-13057probe.py/analyze.py/suitedatamake report0.40 is inside the registered band
[0.25, 0.80]; 1.00 is outside it. Gridoverall pass@1 moves 0.86 → 0.88.
calibrate.py(#10) — the H4 gate, same two files as the viewer tab:make calibratesuitedata.calibrationH4 fails either way — call counts are 5,319 adapter against 5,646 proxy — but
the gate read twice the number its own tab showed, and 19.6% is the figure the
handoff plans against.
suitedata.load_cells(#8) —ZeroDivisionErroron a cell with nogradeable trial. Reproduced from the validation grid's own three ungradeable
rows:
FILES=… make tabledies. That is the shared loader behindtable,matrixandlive, so during a live battery the first scenario whosecompleted trials are all ceiling hits takes down the viewer — exactly when it
is wanted.
Why it recurred
check_harness_exclusionpointed atanalyze.harness_excludedalone. Bothmissed surfaces predate the shared definition and both survived the fix meant
to be repo-wide. A definition shared only by convention is not shared.
What stops it
One selftest check (#11) builds a cell of two harness faults, two passes and
one real failure — 67% excluded, 40% scored — and requires
analyze,suitedata,report,probeandcalibrateto land on the same number. Theall-faults case is asserted from the other side: no rate, an em dash, "harness
broke", no crash.
Each defect was re-introduced and confirmed to fail it:
Also here
report.pystates thepurposemix of what it read, in the document and onstderr. 120 validation rows rendered a full scoreboard silently. (report.py renders validation rows as a scoreboard with no purpose marker #12)
report.pyandcalibrate.pyraisedKeyErroron rows written beforeduration_sandarmexisted. (report.py and calibrate.py raise KeyError on rows predating duration_s / arm #13)bench/ci-local.shwrote a shell script to/tmp/adh-ci-steps.shand thenran it;
mktemp -dwith a cleanup trap. (bench/ci-local.sh writes and executes a predictable path in /tmp #14)AGENTS.mdstill said the TUI is vendored from leather; the handoff pinnedmainatb4ae1ca, put16 ceilingunder the corrected column, and quotedthe un-excluded H4 figure. (docs: AGENTS.md and HANDOFF.md carry stale upstream, SHA and probe-count statements #15)
Verification
make check26/26 ·make ci-localandJOB=lintboth clean, including theA2 == A3 byte assertion and the selftest re-run with
pytestbroken.Closes #8, closes #9, closes #10, closes #11, closes #12, closes #13,
closes #14, closes #15