Skip to content

fix(models): carry failure_class through the DB read path - #38

Merged
usjoh merged 1 commit into
mainfrom
fix/db-reader-carries-failure-class
Aug 16, 2026
Merged

fix(models): carry failure_class through the DB read path#38
usjoh merged 1 commit into
mainfrom
fix/db-reader-carries-failure-class

Conversation

@usjoh

@usjoh usjoh commented Aug 16, 2026

Copy link
Copy Markdown
Owner

--attributable was reporting the opposite of the evidence, silently.

The bug

db_attempt_rows never SELECTed the failure_class column, so its row dicts reached the classifier without it. DB rows also carry no notes, so model_log_row_failure_class had no fallback and returned None for every failure. --attributable keeps a failure only when it is classed model — so it rejected all of them.

Measured on this machine's log, before:

total attempts : 371
non-PASS       : 137
--attributable : "dropped 137 row(s)"   ← every failure in the log
result         : all 33 model rows read 100% first-try

A filter advertised as removing failures no model caused was removing the model's failures too, and printed a reassuring count while doing it.

The column was populated correctly in both runs.jsonl and ringer.db the whole time. Only the reader dropped it — which is why no db rebuild ever fixed it, and why the JSONL fallback path was right while the default path was wrong.

After

--attributable : dropped 46   (2 engine-error + 44 unknown)

Matches the JSONL path exactly, and the scoreboard shows a real distribution again — 0%, 20%, 33%, 44%, 57%, 67%, 74%, 75%, 77%, 100% — instead of a uniform 100%.

opus-5's OpenRouter outage rows are classed unknown and are now genuinely excluded, so the 100% it shows over its one completed task finally means what it claims. (That claim was made in #35 on the strength of the broken flag, corrected in #37, and is only now actually true.)

What this does not do

The four ohalloran-demonstrator rows are stored as model and are correctly retained by the now-working filter. That classification is itself wrong — the workers exited cleanly and could not write to a denied path (#36, #37) — but re-classifying them is a separate problem from the reader dropping the column. claude-opus-4.8 and claude-sonnet-5 still read 0% on site-build in both views.

Verification

  • Full suite: 356 pass
  • Prove-fail: with ringer.py reverted and the tests kept, all four fail
  • Tests pin the classes surviving the round trip, that a judged failure is kept while engine-error and unknown are dropped, that the DB and JSONL paths agree on what counts as evidence, and a guard that any attempts column the classifier consumes actually reaches the row dicts

🤖 Generated with Claude Code

`db_attempt_rows` never SELECTed the `failure_class` column, so the row dicts
it returns reached the classifier without it. DB rows also carry no `notes`, so
`model_log_row_failure_class` had no fallback to derive from and returned None
for every failure. `--attributable` keeps a failure only when it is classed
`model`, so it rejected all of them.

The flag therefore reported the opposite of the evidence. On this machine's log
it announced "dropped 137 row(s)" against 371 attempts — exactly the non-PASS
count, every failure in the log — after which all 33 model rows read 100%
first-try. A filter advertised as removing failures no model caused was in fact
removing the model's failures too, and nothing said so.

The column was populated correctly in both runs.jsonl and ringer.db all along;
only the reader dropped it, which is why no `db rebuild` ever fixed it and why
the JSONL fallback path was right while the default path was wrong.

After: 46 dropped, matching the JSONL path exactly (2 engine-error + 44
unknown), and the scoreboard shows a real distribution again instead of a
uniform 100%. opus-5's OpenRouter outage rows are classed `unknown` and are now
correctly excluded, so the 100% it shows over its one completed task finally
means what it claims.

Note what this does NOT do: the four ohalloran-demonstrator rows are stored as
`model` and are correctly RETAINED by the now-working filter. That
classification is itself wrong — the workers exited cleanly and could not write
to a denied path — but re-classifying them is a separate problem from the
reader dropping the column.

Tests pin the classes surviving the round trip, that a judged failure is kept
while engine-error and unknown are dropped, that the DB and JSONL paths agree
on what counts as evidence, and a guard that any attempts column the classifier
consumes actually reaches the row dicts.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
usjoh added a commit that referenced this pull request Aug 16, 2026
This branch was written while db_attempt_rows still dropped the failure_class
column, so it told the reader not to cite an --attributable rate at all. PR #38
fixes the reader: the flag now drops 46 rows (2 engine-error + 44 unknown)
instead of all 137 failures, matching the JSONL path.

Rewrites the warning as history rather than standing advice, and narrows the
"do not cite" to what it should always have been: any attributable figure read
BEFORE that fix, including the 2026-07-28 ones elsewhere in this file.

The opus-5 entry gets the same treatment. Its 100% over one completed task is
now genuine — those two tasks are classed `unknown` and really are excluded —
where before it was an artifact of every failure being dropped.

Also records the lesson that outlives the bug: a filter that reports how much
it removed can still be removing the wrong things, so check the retained
distribution. A uniform 100% across every model is not a result, it is a
symptom.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@usjoh
usjoh merged commit b3d7288 into main Aug 16, 2026
1 of 3 checks passed
@usjoh
usjoh deleted the fix/db-reader-carries-failure-class branch August 16, 2026 13:25
usjoh added a commit that referenced this pull request Aug 16, 2026
docs(model-notes): re-derive every attributable figure taken before PR #38
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