[JYunth]

Fixes #253

classification_match only caught FileNotFoundError when reading the label file. Any other read error (a glob that matches more than one file, a file that isn't UTF-8, a directory) came back as a plain CriterionResult, which overlay_classification_metrics skips. The row then dropped out of accuracy, P/R/F1 and the confusion matrix. In the #253 repro, 2 of 3 correct rows reported accuracy: 1.0 and passed an accuracy: 0.9 gate.

Those rows are now recorded as (other), with the read error in details, so they stay in the rollup as a miss. FileNotFoundError still maps to (none).

Tests: checker cases for ValueError, UnicodeDecodeError and IsADirectoryError, and a rollup case where the unreadable row brings accuracy to 2/3 and fails the gate. All of them fail on main. make verify passes.

[uipreliga]

Review: coder_eval — pr:256 (4 files) axis:1,2,3,4,5,6,7,8

Scope: pr:256 (4 files) axis:1,2,3,4,5,6,7,8 · branch fix/classification-match-unreadable-label (PR #256 → main) · 0c270d1 · 2026-11-10T19:32Z · workflow variant

Change class: complex — changes error-handling in a scoring criterion so unreadable label files now count as misses in suite accuracy / P/R/F1 rollups

PR #256 correctly keeps unreadable classification_match rows in the suite rollup as 0.0 '(other)' misses, and no finding changes a task's score or final_status for the same agent output; the real risk is that the broad except (OSError, ValueError) now records the read error only in free-text details and not in CriterionResult.error, so harness- or task-caused read failures (PermissionError, EIO, a glob that matches seeded files) are no longer counted in suite.json error_count and look like ordinary agent misses; bottom line: safe to merge after you set error on the result and add two guard tests.

Summary

Axis	Score	🔴	🟠	🟡	🔵	Top Issue
1. Code Quality & Style	9.9 / 10	0	0	0	1	Edited docstring line in ClassificationMatchCriterion is not re-wrapped to match the paragraph
2. Type Safety	10 / 10	0	0	0	0	—
3. Test Health	9.8 / 10	0	0	0	2	New unreadable-file contract is pinned only against a hand-built fake sandbox, not the real Sandbox.get_file_content error types
4. Security	10 / 10	0	0	0	0	—
5. Architecture & Design	10 / 10	0	0	0	0	—
6. Error Handling & Resilience	9.9 / 10	0	0	0	1	New OSError -> gating 0.0 path is invisible to CE039 and carries no agent-fault exemption reason
7. API Surface & Maintainability	10 / 10	0	0	0	0	—
8. Evaluation Harness Quality	9.5 / 10	0	0	1	0	Unreadable label file reported only in free-text details, not CriterionResult.error, so read failures look like an ordinary '(other)' miss and drop out of error_count/error rendering

Overall Score: 9.9 / 10 · Weakest Axis: Evaluation Harness Quality at 9.5 / 10
Totals: 🔴 0 · 🟠 0 · 🟡 1 · 🔵 4 across 8 axes.

Blockers

None.

Non-blocking, but please consider before merge

1. [Axis 8] Unreadable label file reported only in free-text details, not CriterionResult.error, so read failures look like an ordinary '(other)' miss and drop out of error_count/error rendering (src/coder_eval/criteria/classification_match.py:59-61) — The new branch except (OSError, ValueError) as e: / observed = _SENTINEL_OTHER / unreadable = f" (unreadable: {type(e).__name__}: {e})" (lines 59-61) puts the read error only into details (line 70: details = f"observed={observed!r}, expected={expected!r}{unreadable}"). The returned ClassificationCriterionResult (lines 72-79) leaves error=None. Before this PR the same exception went to handle_criterion_errors -> _failed_result (criteria/base.py), which set error="ValueError: ...". The suite rollup counts if cr.error is not None: errors_by_type[...] += 1 (reports/markdown.py:969) into CriterionStats.error_count ("Evaluations that surfaced a checker-level error", models/results.py:833). That count is persisted in suite.json criterion_stats (docs/REPORT_SCHEMA.md:325), which is part of the cross-repo contract that coder-eval-uipath reads. Effect: for identical agent output, error_count for classification_match drops to 0. A task-side or harness-side cause now looks the same as the agent writing an unrecognised label. Examples: a glob that also matches template-seeded files on every row, or a PermissionError / EIO from an OSError that the agent did not cause, such as root-owned docker artifacts re-graded on the host. Scores do not change, because the row was 0.0 before and is 0.0 now, and no schema or container image needs a lockstep rebuild. Only the error signal is lost. Fix: keep the row as a ClassificationCriterionResult, which overlay_classification_metrics still counts as a pair, and also set error=f"{type(e).__name__}: {e}" on it. The row then stays in accuracy/P/R/F1 and still counts in error_count and in the markdown/HTML error badges. As an option, narrow the catch to the agent-attributable classes (UnicodeDecodeError, IsADirectoryError, the multi-match ValueError), so that other OSErrors keep the error label. Also add a rollup test that asserts criterion_stats[...].error_count for the unreadable row.

Nits

1. [Axis 1] Edited docstring line in ClassificationMatchCriterion is not re-wrapped to match the paragraph (src/coder_eval/models/criteria.py:990) — Line 990 is now 103 characters: allowed set or the file cannot be read. Both are recorded as sentinels so the suite rollup can show. The other lines of the same paragraph (986-991) wrap at about 76-80 characters. It is still under ruff's 120 limit, so the tools did not flag it, but it breaks the paragraph's wrap. Re-flow lines 990-991 so they match the lines around them, e.g. allowed set or the file cannot be read. Both are recorded as sentinels / so the suite rollup can show them as real failure classes in the / confusion matrix.
2. [Axis 3] New unreadable-file contract is pinned only against a hand-built fake sandbox, not the real Sandbox.get_file_content error types (tests/test_classification_match.py:156) — The parametrized test at lines 156-173 feeds _FakeSandbox exceptions that are hand-copied from sandbox.py: ValueError("Pattern 'out/*.txt' matches 2 files — refusing to guess which to grade"), UnicodeDecodeError("utf-8", b"\xff\xfe", 0, 1, "invalid start byte"), IsADirectoryError("result.txt"). The rollup test at line 381 does the same: _FakeSandbox({"result.txt": ValueError("Pattern 'result.txt' matches 2 files")}). The docs line this PR changes now promises that a glob matching more than one file, or a non-UTF-8 file, grades as (other). Nothing ties that promise to the real Sandbox.get_file_content (sandbox.py:1400-1406, which raises ValueError on a multi-match and calls read_text(encoding="utf-8")). Suppose the sandbox later changes the multi-match signal, for example to CheckerMisuseError. These tests stay green, but the row then escalates to ERROR or drops out of the rollup again, which is the #253 regression. I checked the PR head against a real tempdir Sandbox and it is correct today: out/*.txt with 2 matches, a b"\xff\xfe" file, and a directory path all give observed='(other)' with score 0.0. Fix: add one real-Sandbox test, using the cheap fixture pattern from tests/test_detached_grading_boundaries.py:955-962 (Sandbox(SandboxConfig(driver="tempdir"), task_id="t"); sandbox.sandbox_dir = work). Write two files that match the glob, one invalid-UTF-8 file and one directory, then assert observed_label == '(other)' for each. Severity is Low, not Medium: the fake already covers each edge case and the behaviour is correct. Only the coupling to the real error contract is missing.
3. [Axis 3] No test pins that the widened except clause still lets escalating errors (CheckerMisuseError / 'Sandbox not set up') propagate (tests/test_classification_match.py:164) — The PR widens the checker's catch to except (OSError, ValueError) as e: (src/coder_eval/criteria/classification_match.py:59). It is safe today only because CheckerMisuseError subclasses RuntimeError (errors/checker_misuse.py:4) and get_file_content raises a bare RuntimeError("Sandbox not set up") (sandbox.py:1395). I confirmed against a real Sandbox that ../outside.txt still raises CheckerMisuseError. The inverse branch (Technique 5) has no test in this file. Suppose a later edit widens the clause to except Exception, which is a natural step for a "count every unreadable row as a miss" fix. An escaped criterion path, which is an author config error that must go to FinalStatus.ERROR, would then grade silently as a 0.0 '(other)' agent miss. The existing escape tests (tests/test_detached_grading_boundaries.py:978,985) call sandbox.resolve_files directly and do not go through this checker's except clause. Fix: next to test_unreadable_file_becomes_other_sentinel, add a case where the fake raises CheckerMisuseError(...) (and optionally RuntimeError("Sandbox not set up")), then assert pytest.raises(CheckerMisuseError) from checker.check(...).
4. [Axis 6] New OSError -> gating 0.0 path is invisible to CE039 and carries no agent-fault exemption reason (src/coder_eval/criteria/classification_match.py:59) — CE039 (tests/lint/rules/ce039_config_error_escalates.py) forbids booking an IO error as a gating score=0.0 unless the code marks it as the agent's fault with # noqa: CE039 and a reason. The rule fires only on a return ...CriterionResult(score=0.0) lexically inside the handler. This PR sets observed = _SENTINEL_OTHER in except (OSError, ValueError) as e: (line 59) and computes score = 1.0 if observed == expected else 0.0 after the try, so the rule cannot see it. The choice is defensible for an agent-written file. But nothing states that decision at the site, and nothing guards a later edit that routes a task-author path through this branch. reference_comparison.py records the same decision explicitly (# CE039 exemption: genuinely the AGENT's failure + # noqa: CE039). Fix: add a one-line note at the handler. A lint candidate is to widen CE039 to a sentinel/score assignment made inside an IO except handler that flows into a returned *CriterionResult.

What's Missing

Parallel paths:

• 🟡 Only one drop-out path is closed. overlay_classification_metrics (criteria/_classification_aggregate.py:43-45) still drops, without a message, every row that is not a ClassificationCriterionResult. Three paths still produce such rows. (a) Any non-escalating exception outside (OSError, ValueError) in classification_match goes through handle_criterion_errors and comes back as a plain CriterionResult. One example is the bare RuntimeError('Sandbox not set up'). (b) The sibling classification producer skill_triggered returns a plain CriterionResult on its turn_records is None branch (skill_triggered.py:110-116). (c) The rollup skips rows that have no result at index i (reports/markdown.py per_rows filter). Each path gives the same #253 symptom: total_pairs is lower than the row count and the accuracy gate is inflated. Issue #253 named the overlay-side alternative, which is to count a non-classification row as a miss or emit total_pairs against rows_measured. That alternative would close the whole class, not only the read-error case. (trigger: src/coder_eval/criteria/classification_match.py)
• 🔵 The class docstring and TASK_DEFINITION_GUIDE now say that an unreadable file becomes '(other)'. The field descriptions on ClassificationMatchCriterion were not updated: allowed_labels says 'File content not in this set is treated as (other)', and path says 'may be a glob matching exactly one file' with no statement of what happens on a multi-match. The same applies to the generated plugins/coder-eval/reference/criteria.md that /coder-eval:task reads. Update the Field descriptions and run make plugin-reference so the plugin surface matches the guide. (trigger: src/coder_eval/models/criteria.py)

Downstream consumers:

• 🔵 Every consumer of CriterionResult.error now gets no signal for an unreadable label file, because the error is only in details. The affected consumers are suite.json criterion_stats[].error_count (markdown.py:969), the markdown and HTML error badges (markdown.py:1036, html.py:407), the evalboard criterion row (evalboard/lib/runs.ts:181 error: criterion.error ?? null) and evaluate output (evaluate_command.py:568). None of these consumers was updated, and no test asserts their new values. (trigger: src/coder_eval/criteria/classification_match.py) (restates: Axis 8: Unreadable label file reported only in free-text details, not CriterionResult.error)

Tests:

• 🔵 test_unreadable_row_counts_as_a_miss asserts only accuracy, total_pairs and agg.passed. It does not assert that the confusion matrix and the per-label stats contain the ('negative', '(other)') cell, and it does not assert the rendered suite-gate verdict in the markdown or in suite.json. That cell is what the docs line promises ('real failure classes in the confusion matrix'). The parametrized checker test also has no PermissionError case. PermissionError is the broadest OSError subclass the new clause catches, and the one most likely to be caused by the harness, not the agent. (trigger: tests/test_classification_match.py)

Nightly pipeline:

• 🔵 The PR body does not say what happens to scheduled runs or to the coder-eval-uipath consumer. For any dataset suite where a row's label file cannot be read, suite.json criterion_aggregates values change: accuracy, P/R/F1, total_pairs and the confusion matrix. suite_thresholds gates that passed before can now fail, which is the intended fix but a visible verdict flip on trend lines. criterion_stats.error_count for classification_match drops to 0 for those rows. evaluate <run_dir> re-grades of older runs also give different suite numbers than the original run did. Add one line to the PR or CHANGELOG that states this blast radius. (trigger: src/coder_eval/criteria/classification_match.py)

Harness & Lint Improvements

Static checks (lint / type):

• [ce-lint] New rule CE069 captured-exception-reaches-error in tests/lint/rules/ce069_captured_exception_reaches_error.py, wired into tests/lint/runner.py next to ConfigErrorEscalates. Scope is coder_eval/criteria/. The rule fires when an except ... as e handler does not raise, references e in an expression, and that text reaches a *CriterionResult(...) only through details= and never through error=. It covers two shapes: (a) the direct shape, a return XCriterionResult(details=f"...{e}") inside the handler with no error= kwarg; (b) a one-hop local, where the handler assigns a name built from e (here unreadable = f" (unreadable: {type(e).__name__}: {e})") and that name is later used in details= but no error= kwarg in the same function gets it. The fix the rule asks for: a local such as read_error: str | None = None, set in the handler and passed as error=read_error. This keeps the row a ClassificationCriterionResult (overlay_classification_metrics filters only on isinstance), so it stays in accuracy/P/R/F1 and is still counted in CriterionStats.error_count (reports/markdown.py:969). An AST probe shows the rule fires on classification_match.py:59-61. For rollout, it also flags uipath_eval.py:96 (details=f"Failed to parse ...: {e}", no error=). That is an existing site of the same class, outside this PR. The rule's module docstring must state its blind spot: dataflow is followed for one local hop only. Prevents: A8/A1/A5/A6/A7 merged finding (classification_match.py:59-61): the read error goes into free-text details only, error stays None, error_count drops to 0, and harness- or task-caused read failures look the same as an agent writing an unrecognised label.
• [ce-lint] Make CE039 (tests/lint/rules/ce039_config_error_escalates.py) wider. It fires today only on a lexical return ...CriterionResult(score=0.0) inside an IO handler. Add a second trigger: an except handler in coder_eval/criteria/ that catches an IO class (OSError / FileNotFoundError / PermissionError / IsADirectoryError, plus UnicodeDecodeError and ValueError when the try body calls sandbox.get_file_content / sandbox.resolve_files) and has no raise and no return anywhere in it. Such a handler always falls through to a later scored result. The rule then requires # noqa: CE039 plus a one-line agent-fault reason on the except line, the same convention reference_comparison.py:93-96 uses. An AST probe over criteria/ flags exactly the two handlers this PR touches (classification_match.py:57 except FileNotFoundError and :59 except (OSError, ValueError)) and nothing else, so the widening costs nothing to roll out. Update the rule's docstring and the .claude/notes/lint-rules.md § CE039 entry, and add a TestCE039 case for the fall-through shape in tests/test_custom_lint.py. Prevents: A6 finding (classification_match.py:59): the new OSError -> sentinel -> score 0.0 path is invisible to CE039 because the score is computed after the try. Nothing at the site records that this is the agent's fault, and nothing stops a later edit that sends a task-author path through the branch.
• [ce-lint] New rule CE070 broad-except-swallows-escalation in tests/lint/rules/, wired into tests/lint/runner.py. Scope is coder_eval/criteria/. The rule fires on an except Exception / except BaseException / except RuntimeError / bare except (or a tuple containing one of them) whose try body calls a sandbox.* method, when no earlier handler in the same try catches _ESCALATING_EXCEPTIONS / CheckerMisuseError / JudgeInfrastructureError and re-raises, and the handler itself does not do a bare raise. Both escalating types subclass RuntimeError (errors/checker_misuse.py:4, errors/judge.py:20). get_file_content raises CheckerMisuseError on a path escape and a bare RuntimeError on 'Sandbox not set up'. So one natural step from (OSError, ValueError) to except Exception, made in the spirit of 'count every unreadable row as a miss', would silently turn an author config error (which belongs in FinalStatus.ERROR) into a 0.0 '(other)' miss. The rule makes that edit fail make lint. The PR's own (OSError, ValueError) passes. For rollout, an AST probe shows existing broad handlers around sandbox reads at reference_comparison.py:102 and uipath_eval.py:96 that will need triage or a # noqa: CE070 reason. Restrict the rule to try bodies that touch sandbox. so the decorator tails in criteria/base.py:126/153, which already re-raise _ESCALATING_EXCEPTIONS first, do not match. Prevents: A3 finding (test_classification_match.py:164): no guard keeps the widened except clause from swallowing CheckerMisuseError / 'Sandbox not set up'. CE070 turns the missing negative test into a mechanical gate.
• [ruff] Set [tool.ruff.lint.pycodestyle] max-doc-length = 100 in pyproject.toml. W is already selected, so W505 (doc-line-too-long) then turns on. Measured on src/ at the PR head: 106 W505 hits at 100, 18 at 110, 539 at 88. The line in this PR is 103 chars (models/criteria.py:990: W505 Doc line too long (103 > 100)), so 110 would not catch it. The cap must sit below the current value, which means a one-time reflow of about 106 docstring/comment lines. Limit: W505 enforces a ceiling, not paragraph-wrap consistency. A line re-flowed to 95 chars inside an 80-char paragraph still passes, and that residue stays a style judgement. Prevents: A1 finding (models/criteria.py:990): the edited docstring line was not re-wrapped and is 103 chars inside a paragraph that wraps at about 76-80.

Harness improvements (not statically reachable):

• Add a shared real-Sandbox contract matrix for file-reading criteria, for example tests/test_criteria_unreadable_inputs.py. Build a real tempdir sandbox once, using the cheap fixture from tests/test_detached_grading_boundaries.py:955-962 (Sandbox(SandboxConfig(driver="tempdir"), task_id="t"); sandbox.sandbox_dir = work), and promote it to a conftest fixture. Parametrize over every criterion that calls sandbox.get_file_content (classification_match, file_check, file_contains, file_matches_regex, json_check, reference_comparison.agent_file, cli_called, uipath_eval) and over the unreadable-input cases: glob with 2 matches, non-UTF-8 bytes (b"\xff\xfe"), path that is a directory, missing file, and ../outside.txt. For each pair, assert the documented outcome: for classification_match, observed_label '(other)', score 0.0 and error set; for an escaping path, pytest.raises(CheckerMisuseError). The current tests feed _FakeSandbox hand-copied exceptions, so a change to the real error signal (for example multi-match moving from ValueError to another type) leaves them green and brings back #253. Why not static: Which exception the real Sandbox.get_file_content raises for a multi-match, a bad encoding or a directory is decided by runtime filesystem state and pathlib behaviour. A static rule cannot prove that a fake's injected exception matches what the real method raises. Prevents: A3 finding (test_classification_match.py:156): the unreadable contract is pinned only against a fake. A3 finding (test_classification_match.py:164): no inverse test that escalating errors still propagate. A8 merged finding: no assertion on error.
• Add a rollup-level regression assertion next to test_unreadable_row_counts_as_a_miss: build the suite rollup over a mix of readable and unreadable rows and assert criterion_stats['classification_match'].error_count equals the number of unreadable rows, and that accuracy/P/R/F1 still include those rows. This pins the cross-repo contract field (suite.json criterion_stats.error_count, docs/REPORT_SCHEMA.md:325, read by coder-eval-uipath), so the nightly consumer's error signal cannot drop to 0 again without a failing test. Why not static: error_count is computed in reports/markdown.py:969 from runtime CriterionResult values. To show that a row reaches it, you have to run the checker and the rollup end to end. CE069 can force error= at the construction site, but only a test can prove the rollup counts it. Prevents: A8 merged finding (classification_match.py:59-61): error_count drops to 0 for unreadable rows, and the PR's tests check only score, observed_label, details and accuracy.
• Remove the sharp edge in sandbox.py: raise a named AmbiguousMatchError(ValueError) for the multi-match case instead of a bare ValueError identified by message, and document it in get_file_content's Raises block. Checkers can then catch the agent-attributable classes by name (FileNotFoundError, AmbiguousMatchError, UnicodeDecodeError, IsADirectoryError) instead of the wide (OSError, ValueError). Other OSErrors (PermissionError and EIO on root-owned docker artifacts re-graded on the host) would then keep the error label rather than being booked as agent misses. With the type in place, CE069/CE070 and the contract matrix above can name the narrow set. Why not static: Which OSError subclasses are the agent's fault and which are the harness's is a semantic attribution decision. A lint rule can enforce a named type once one exists, but it cannot pick the split. That needs a design decision recorded in .claude/notes/contracts.md § What escalates instead of scoring 0.0. Prevents: A8 merged finding (broad catch counts harness-caused PermissionError/EIO as agent misses). A3 finding (fake exceptions are hand-copied from a message string).

Top 5 Priority Actions

1. Set error=f"{type(e).__name__}: {e}" on the ClassificationCriterionResult returned after the except (OSError, ValueError) branch (src/coder_eval/criteria/classification_match.py:59-61, result built at 72-79), so unreadable rows still count in criterion_stats[...].error_count and in the error badges, and stay in accuracy/P/R/F1. Optionally narrow the catch to the errors the agent causes (UnicodeDecodeError, IsADirectoryError, the multi-match ValueError).
2. Add a test next to test_unreadable_file_becomes_other_sentinel (tests/test_classification_match.py:164) where the fake raises CheckerMisuseError (and RuntimeError("Sandbox not set up")), and assert that it propagates. Without this test, a later change to except Exception would silently grade an author config error as a 0.0 agent miss and not as FinalStatus.ERROR.
3. Add one test that uses a real tempdir Sandbox (fixture pattern from tests/test_detached_grading_boundaries.py:955-962), with two files matching the glob, one invalid-UTF-8 file and one directory, and assert observed_label == '(other)' for each. This ties the documented contract to the real Sandbox.get_file_content error types, not only to the hand-built _FakeSandbox (tests/test_classification_match.py:156-173, 381).
4. Add a rollup test that asserts criterion_stats[...].error_count for the unreadable row, together with the existing accuracy assertion, so the suite.json error signal is pinned as part of the #253 fix.
5. Add a one-line CE039 exemption note at the handler (src/coder_eval/criteria/classification_match.py:59) that says why an unreadable agent-written file is the agent's failure, as reference_comparison.py does, and re-wrap the edited docstring paragraph at src/coder_eval/models/criteria.py:990-991 to the same width as the lines around it.

---

Stats: 0 🔴 · 0 🟠 · 1 🟡 · 4 🔵 across 8 axes reviewed.

[JYunth]

Thanks for the thorough review. Pushed 3ade4d2:

• Unreadable rows now set error as well as recording (other), so they stay in accuracy/P/R/F1 and count in criterion_stats.error_count and the error badges again. The rollup test now checks error_count, the negative → (other) confusion cell and the gate.
• Added a real tempdir-Sandbox test (ambiguous glob, non-UTF-8 file, directory), a PermissionError case, and a test that CheckerMisuseError still propagates.
• Added a one-line CE039 note at the handler, re-wrapped the docstring, and updated the allowed_labels description and the generated plugin reference.

I kept the catch at (OSError, ValueError) rather than narrowing it. Now that error is set, a harness-side PermissionError or EIO still shows up in error_count, while a narrower catch would route those rows back through handle_criterion_errors and they'd drop out of the rollup again.

Blast radius: for a suite where some label file can't be read, accuracy/P/R/F1, total_pairs and the confusion matrix now include that row as a miss, so a suite_thresholds gate that passed before can fail, and re-grading an older run with evaluate can give different suite numbers. error_count for those rows is the same as on main.

I left the broader items for follow-ups, since each is bigger than this fix: the overlay-side change that would also cover skill_triggered's turn_records is None branch, the lint rules (CE069/CE070, widening CE039, W505), the cross-criterion Sandbox test matrix, and a named AmbiguousMatchError. Happy to open issues for any of them.