Skip to content

fix(reports): count unscored rows as 0.0 in run.md's Avg Reliability Score - #257

Open
JYunth wants to merge 2 commits into
UiPath:mainfrom
JYunth:fix/run-md-avg-score-counts-errors
Open

JYunth wants to merge 2 commits into
UiPath:mainfrom
JYunth:fix/run-md-avg-score-counts-errors

Conversation

@JYunth

@JYunth JYunth commented Nov 10, 2026

Copy link
Copy Markdown

run.md's "Avg Reliability Score" only averaged rows that had a weighted_score. ERROR and BUILD_FAILED rows have none, so they left both sides of the mean. A run with one SUCCESS (1.0) and one ERROR showed 1.000 in run.md, while experiment.md showed 0.500 for the same run.

run.md now follows the rule VariantAggregate.average_score already uses, and that docs/REPORT_SCHEMA.md describes: an unscored row counts as 0.0, and only NOT_GRADED rows leave the mean. The status lookup reuses _category_of from reports/junit.py, so an unknown status string doesn't break the report.

Not in this PR: suite.json's average_weighted_score has the same filter. It's a serialized field, so I left it out. Happy to do it here or in a follow-up if you want it.

Tests: a parametrized case in test_reports.py. A third row with ERROR gives 0.333 (fails on main), and with NOT_GRADED it gives 0.500. make verify passes.

…Score

run.md averaged only rows that had a weighted_score. An ERROR or
BUILD_FAILED row has none, so it left both sides of the mean, and a run where
a task errored could report a higher average than the same run with that task
failing. VariantAggregate.average_score (experiment.md) already counts those
rows as 0.0 and drops only NOT_GRADED rows, as docs/REPORT_SCHEMA.md
describes. run.md now uses the same rule.

@uipreliga uipreliga left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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

Scope: pr:257 (2 files) axis:1,2,3,4,5,6,7,8 · branch fix/run-md-avg-score-counts-errors (PR #257 by @JYunth → main, OPEN) · 5056cf1 · 2026-11-10T14:04Z · workflow variant

Change class: complex — changes the run.md Avg Reliability Score aggregation formula (which rows count and how unscored rows are valued), i.e. scoring semantics

PR #257 correctly makes the run.md Avg Reliability Score agree with VariantAggregate.average_score for graded runs: unscored rows count as 0.0 and only NOT_GRADED rows leave the mean, and the type safety, security and error handling are clean. But the change now prints a measured-looking "Avg Reliability Score: 0.000" beside "Pass Rate: n/a" on an execute run that has crashed or timed-out rows. It also copies the averaging rule inline, and leaves _compute_suite_rollup in the same file on the opposite rule (0.333 in run.md against 0.500 in suite.json for the same rows). Bottom line: merge after you gate the score line on verdict evidence and add tests that pin the boundary cases.

Summary

Axis Score 🔴 🟠 🟡 🔵 Top Issue
1. Code Quality & Style 9.9 / 10 0 0 0 1 Test name covers only half of what the parametrized cases assert, and the case labels are comments instead of ids
2. Type Safety 10 / 10 0 0 0 0 —
3. Test Health 9.4 / 10 0 0 1 1 No tests for unscored-row cases in the markdown Avg Reliability Score (all-ERROR, all-NOT_GRADED, mixed, failed-category rows)
4. Security 10 / 10 0 0 0 0 —
5. Architecture & Design 9.5 / 10 0 0 1 0 Graded-score averaging rule copied inline in markdown.py from _measured_scores; suite rollup in the same file still uses the opposite rule
6. Error Handling & Resilience 10 / 10 0 0 0 0 —
7. API Surface & Maintainability 9.9 / 10 0 0 0 1 markdown.py imports the private _category_of helper from the junit renderer
8. Evaluation Harness Quality 9 / 10 0 1 0 0 run.md now prints a measured-looking 'Avg Reliability Score: 0.000' for an execute (grading-off) run that has crashed rows, next to 'Pass Rate: n/a'

Overall Score: 9.7 / 10 · Weakest Axis: Evaluation Harness Quality at 9 / 10
Totals: 🔴 0 · 🟠 1 · 🟡 2 · 🔵 3 across 8 axes.

Blockers

  1. [Axis 8] run.md now prints a measured-looking 'Avg Reliability Score: 0.000' for an execute (grading-off) run that has crashed rows, next to 'Pass Rate: n/a' (src/coder_eval/reports/markdown.py:476-487) — The new filter if _category_of(t["status"]) != "ungraded" (line 479) keeps ERROR rows, and t["weighted_score"] if t.get("weighted_score") is not None else 0.0 (line 477) turns each one into 0.0. Under coder-eval execute, a crashed row lands in the error bucket, not in ungraded. RunSummary._nothing_was_measured (models/results.py:1246) exists to stop this case from becoming a real-looking total failure. I reproduced it on the PR worktree with 95 NOT_GRADED rows, 5 ERROR rows with no score, and tasks_measured=0. On main, run.md prints only Pass Rate: n/a — 95 task(s) executed without grading and omits the score line. On the PR, it also prints - **Avg Reliability Score**: 0.000 (line 487). .claude/notes/orchestration.md:945 forbids exactly this: "a zero beside 'Pass Rate: n/a' is indistinguishable from 'scored zero'". The agent output is identical, but the headline metric changes from absent to 0.000, so the Axis 8 scoring-correctness rule applies. run.md is also what action.yml (lines 107 and 243) appends to GITHUB_STEP_SUMMARY. Fix: gate the score on verdict evidence, the same way the pass rate is gated. For example, skip or render n/a when nothing_was_measured(not_graded=summary.tasks_not_graded, measured=summary.tasks_measured) (or summary.pass_rate is None) is true. Add a third parametrized case to test_avg_reliability_score_counts_unscored_rows_as_zero (tests/test_reports.py:463) for NOT_GRADED plus an unscored ERROR row with tasks_measured=0. The PR copies its rule from VariantAggregate.average_score (_measured_scores, orchestration/experiment.py:800), which has the same execute-crash gap. Fix that function in the same change or record the gap. n/a

Non-blocking, but please consider before merge

  1. [Axis 3] No tests for unscored-row cases in the markdown Avg Reliability Score (all-ERROR, all-NOT_GRADED, mixed, failed-category rows) (tests/test_reports.py:293) — The change moved the boundary where scores is empty. Before, a run whose rows all had weighted_score: None left out the Avg Reliability line. Now any non-ungraded unscored row adds a 0.0, so the line renders. No test pins the new output on either side of the boundary. (1) test_generate_markdown_with_null_scores (L271, row L283 _make_task_result("task_error", "ERROR", None, 60.0)) now renders - **Avg Reliability Score**: 0.000 (I ran it against the PR worktree), but it asserts only assert "N/A" in report_md # Should show N/A for null weighted_score (L293). (2) The execute-night shape (1 ERROR + 9 NOT_GRADED, tasks_measured=0) now renders Pass Rate: n/a — 9 task(s) executed without grading next to Avg Reliability Score: 0.000. This is the pairing that .claude/notes/orchestration.md:945-946 warns about ('a zero beside "Pass Rate: n/a" is indistinguishable from "scored zero"'). TestPassRate...test_the_markdown_says_so_rather_than_rendering_zero_percent (L1385) uses task_results=[] (L1374), so it never reaches the score line. (3) No test covers the all-NOT_GRADED case, where the line must stay absent rather than show 0.000. This output matches _mean_graded_score (ERROR counts as 0.0), so it may be intended. Still, it is the branch that turns a gap into a printed score, and nothing exercises it. Add explicit asserts: "Avg Reliability Score**: 0.000" in md for the all-ERROR run, "Avg Reliability Score" not in md for the all-NOT_GRADED run, and a test of the decided behaviour for the ERROR + NOT_GRADED execute night (either assert it or change the code to return n/a).
  2. [Axis 5] Graded-score averaging rule copied inline in markdown.py from _measured_scores; suite rollup in the same file still uses the opposite rule (src/coder_eval/reports/markdown.py:476) — The PR copies the rule into ReportGenerator._summary_section_lines (markdown.py:474-480):
    scores = [t["weighted_score"] if t.get("weighted_score") is not None else 0.0 for t in summary.task_results if _category_of(t["status"]) != "ungraded"]
    The same rule already lives in orchestration/experiment.py:800 def _measured_scores(rows: Sequence[VariantResult] | Sequence[TaskResult]) -> list[float]. That helper skips the row when result.final_status.category == "ungraded" and otherwise appends result.weighted_score if result.weighted_score is not None else 0.0. Its docstring already holds the full rationale, which is why the new code needs a 2-line comment and a Rationale: pointer to explain itself. The PR also leaves a third average of the same weighted_score field in the same file with the old rule. markdown.py:964 (suite_rollup, which feeds suite.json/suite.md average_weighted_score) reads scored = [r.result.weighted_score for r in rows if r.result.weighted_score is not None]. Before this PR, the two averages in markdown.py agreed with each other and only disagreed with experiment.md. Now one run gives two different headline means: for 1 SUCCESS(1.0) + 1 FAILURE(0.0) + 1 ERROR row, run.md shows 0.333 and suite.json shows 0.500. Recommendation: make _measured_scores public, as a status- and score-based function in result_metrics.py (the documented home for result metrics that both core and reports read; CE066 allows reports to import it). It must accept the typed rows and the RunSummary.task_results dict rows, for example via an iterable of (status, weighted_score) pairs. Then call it from markdown.py:476, markdown.py:964 and orchestration/experiment.py:800. The inline comment and the Rationale pointer can then go, and the rule exists in exactly one place. If the suite.json change stays deferred because it is a serialized field, record the known divergence in a notes entry or a tracked issue. Do not leave two contradictory averages in the same file with no marker.

Nits

  1. [Axis 1] Test name covers only half of what the parametrized cases assert, and the case labels are comments instead of ids (tests/test_reports.py:463) — def test_avg_reliability_score_counts_unscored_rows_as_zero(third_status: str, expected_line: str): is parametrized over ERROR (counted as 0.0) and NOT_GRADED (dropped from the mean, line 459: # NOT_GRADED was never measured, so it leaves the mean.). The name states only the first behavior. Rename the test, e.g. test_avg_reliability_score_zeroes_unscored_and_drops_ungraded. Replace the comments at lines 457/459 with pytest.param("ERROR", "...0.333", id="error-counts-as-zero") / pytest.param("NOT_GRADED", "...0.500", id="not-graded-leaves-mean"), so each case names its intent in pytest output instead of in comments.
  2. [Axis 3] No parity test ties run.md's Avg Reliability Score to VariantAggregate.average_score / _measured_scores (tests/test_reports.py:463) — The defect this PR fixes is drift between two surfaces: run.md showed 1.000 and experiment.md showed 0.500 for the same rows. The fix copies the rule inline in src/coder_eval/reports/markdown.py:476-480 (t["weighted_score"] if t.get("weighted_score") is not None else 0.0 ... if _category_of(t["status"]) != "ungraded"), separate from orchestration/experiment.py _measured_scores (L814-819). The new test test_avg_reliability_score_counts_unscored_rows_as_zero checks run.md against hand-computed constants ("Avg Reliability Score**: 0.333"), so the two copies can drift again with no failing test. Add a parity test like test_the_three_published_pass_rates_agree_on_one_set_of_counts (tests/test_ungraded_reporting.py:98). Build one row set (SUCCESS 1.0, FAILURE 0.0, ERROR None, TIMEOUT None, NOT_GRADED None) as both task_results dicts and VariantResults, then assert that the run.md Avg Reliability value equals f"{_mean_graded_score(rows):.3f}". A better fix is to have markdown.py call one shared helper, so the test checks one implementation.
  3. [Axis 7] markdown.py imports the private _category_of helper from the junit renderer (src/coder_eval/reports/markdown.py:30) — markdown.py:30 adds from .junit import _category_of, and markdown.py:479 calls it (if _category_of(t["status"]) != "ungraded"). The function is defined at reports/junit.py:55 as def _category_of(status: str) -> Literal["succeeded", "failed", "error", "ungraded"]:. The underscore says it belongs to the JUnit writer, but it now has a consumer in a second module. Its signature and its 'unknown status -> "error"' fallback are now a contract between two renderers that nothing states, and the markdown renderer depends on the JUnit writer module for a status-classification helper that has nothing to do with JUnit. There is precedent in the repo (orchestrator.py:33 imports _short_failure_reason), so this is a maintainability paper cut, not a break. Fix: promote it to one public, typed helper and import it from both writers. Option A: status_category(status: str) in reports/helpers.py. Option B: a FinalStatus classmethod beside the category property in models/enums.py, which is closer to the SSOT the docstring already names. Keep the explicit-allowlist + unknown->error behaviour (CE018).

What's Missing

Downstream consumers:

  • 🟠 Downstream consumers: the PR changes the run.md mean formula, but it does not apply the 'nothing was measured' gate that already controls the Pass Rate line beside it (_pass_rate_lines / RunSummary._nothing_was_measured). docs/REPORT_SCHEMA.md:285 says average_score is null when nothing was measured. On an execute night with only NOT_GRADED rows plus crashed (ERROR) or unscored TIMEOUT rows, run.md now prints Avg Reliability Score: 0.000 next to Pass Rate: n/a. Gate the score line on the same predicate as the pass rate. (trigger: src/coder_eval/reports/markdown.py) (restates: Axis 8: run.md now prints a measured-looking 'Avg Reliability Score: 0.000' for an execute (grading-off) run that has crashed rows, next to 'Pass Rate: n/a')
  • 🔵 Downstream consumers (docs): the formula of run.md's 'Avg Reliability Score' changed, but docs/ does not define this figure anywhere (no docs page contains 'Reliability'). REPORT_SCHEMA.md documents only VariantAggregate.average_score. Add one line in docs/REPORT_SCHEMA.md or docs/USER_GUIDE.md that says which rows the run.md mean counts. This is the contract the PR body cites. (trigger: src/coder_eval/reports/markdown.py)

Parallel paths:

  • 🟡 Parallel paths: _compute_suite_rollup in the same file (markdown.py:964, which feeds suite.json/suite.md average_weighted_score) still drops unscored rows. One dataset run with SUCCESS(1.0) + FAILURE(0.0) + ERROR now shows 0.333 in run.md and 0.500 in suite.md. The PR body defers this, but the deferral is not recorded in a notes entry, a tracked issue, or a code marker. Record it, or fix it in the same change. Keep rows_measured separate from the mean. (trigger: src/coder_eval/reports/markdown.py) _(restates: Axis 5: Graded-score averaging rule copied inline in markdown.py from measured_scores; suite rollup in the same file still uses the opposite rule)
  • 🟡 Parallel paths: the PR copies the 'ungraded leaves, unscored counts 0.0' rule from _measured_scores (orchestration/experiment.py:800) into markdown.py instead of calling a shared helper in result_metrics.py. The run.md and experiment.md means can drift again, which is the defect this PR fixes. (trigger: src/coder_eval/reports/markdown.py) _(restates: Axis 5: Graded-score averaging rule copied inline in markdown.py from measured_scores; suite rollup in the same file still uses the opposite rule)

Tests:

  • 🟡 Tests: no test pins the new edge of the if scores: branch. All-ERROR now renders 0.000: test_generate_markdown_with_null_scores passes without a change but asserts only 'N/A'. For all-NOT_GRADED the line must stay absent. The ERROR + NOT_GRADED execute-night shape is not tested either. The new parametrized test covers only mixed runs that have graded rows (tasks_measured=2). (trigger: tests/test_reports.py) (restates: Axis 3: No tests for unscored-row cases in the markdown Avg Reliability Score (all-ERROR, all-NOT_GRADED, mixed, failed-category rows))
  • 🔵 Tests: no parity test asserts that run.md's Avg Reliability Score equals _mean_graded_score / VariantAggregate.average_score for the same row set. The new test checks hand-computed constants (0.333 / 0.500), so the two copies of the rule can diverge again and no test will fail. (trigger: tests/test_reports.py) _(restates: Axis 3: No parity test ties run.md's Avg Reliability Score to VariantAggregate.average_score / measured_scores)
  • 🔵 Tests: the new filter sends each row through _category_of. That function maps an unknown or legacy status string to 'error', so an unscored row with an unrecognised status now adds 0.0 to the mean (before the PR it left the mean). No test exercises the unknown-status path or an unscored TIMEOUT row (category 'failed') through generate_markdown. (trigger: tests/test_reports.py)

Nightly pipeline:

  • 🔵 Daily/nightly impact not stated: action.yml (lines 107 and 243) appends run.md to GITHUB_STEP_SUMMARY. After the merge, the Avg Reliability Score for any nightly with ERROR, BUILD_FAILED or unscored TIMEOUT rows steps down. The number is not comparable with earlier summaries, and execute nights with crashes start to show 0.000. run.json (the coder-eval-uipath consumer contract) is unchanged. The PR body says neither, so a reader of the trend sees a drop and does not know that it comes from the formula change. (trigger: src/coder_eval/reports/markdown.py)

Harness & Lint Improvements

Static checks (lint / type):

  • [ce-lint] CE069 weighted-score-mean-via-ssot: a new rule file at tests/lint/rules/ce069_weighted_score_mean_via_ssot.py, wired in tests/lint/runner.py, with CE069 added to [tool.ruff.lint] external in pyproject.toml. The rule fails on any list or generator comprehension in src/coder_eval/ whose element is a weighted_score read (x.weighted_score, t["weighted_score"] or t.get("weighted_score")) and whose filter is either weighted_score is not None or a category == / != "ungraded" test. The only permitted place is one public helper in result_metrics.py, for example measured_scores(pairs: Iterable[tuple[str | FinalStatus, float | None]]) -> list[float]. CE066 allows both orchestration/experiment.py and reports/* to import it. _measured_scores in orchestration/experiment.py moves there. Current hits: reports/markdown.py:476 (the PR's inline copy), reports/markdown.py:964 (_compute_suite_rollup's scored = [... if r.result.weighted_score is not None]), reports/experiment.py:468, reports/html.py:1163 and orchestration/experiment.py:897. The last is best-variant selection, not a mean, so it gets a # noqa: CE069 with a reason. Each of the others is either migrated or allowlisted with a reason (the deferred suite.json field gets an explicit noqa that points to the tracking issue). This puts the divergence the PR leaves in place under a marker. The module docstring must state a blind spot: a loop-and-append mean, like the current _measured_scores body, is not a comprehension. The rule therefore also flags .append(<weighted_score read>) inside a for loop outside result_metrics.py. Prevents: Finding A5 (markdown.py:476), where the 'ungraded leaves, unscored counts 0.0' rule is copied inline from _measured_scores while _compute_suite_rollup at markdown.py:964 in the same file still uses the opposite rule (run.md 0.333 against suite.json 0.500). Also the grouped A1/A7/A8 members at markdown.py:474/476/964. With one implementation, the parity gap in finding A3 (markdown.py has no parity test against VariantAggregate.average_score) disappears, and so does the 2-line comment plus Rationale: pointer that the inline copy needed. It also prevents the original #257 defect: run.md 1.000 against experiment.md 0.500.
  • [ce-lint] CE070 no-cross-module-private-import: a new rule file at tests/lint/rules/ce070_no_cross_module_private_import.py, wired in tests/lint/runner.py and added to external. The rule fails on an ImportFrom inside src/coder_eval/ that binds a single-underscore, non-dunder name (from .junit import _category_of), unless the source module is itself private (_skills.py, so from ._skills import _plugin_skill_dirs is allowed) or the import renames a public name to a private alias (extract_utterance as _extract_utterance, where the source name has no underscore). One existing violation remains on main: orchestrator.py:32 _short_failure_reason. Put it in a GRANDFATHERED set with a reason, not a cap at the current count, as the CLAUDE.md cap-rule convention requires. Ruff's PLC2701 (import-private-name) cannot do this job. It is preview-only, and it skips imports whose top-level package matches the importing module (coder_eval → coder_eval), so it never fires on first-party code. The module docstring must record that boundary. Prevents: Finding A7 (markdown.py:30 from .junit import _category_of), and the grouped A1/A5 members at the same line. It makes the author promote the helper to a public, typed status_category() in reports/helpers.py, or a FinalStatus classmethod next to .category in models/enums.py. Both renderers then import a declared contract (explicit allowlist, unknown → "error", CE018), not a private detail of the JUnit writer.
  • [ce-lint] Pyright tightening: replace RunSummary.task_results: list[dict[str, Any]] (models/results.py:1182) with a TaskRow TypedDict. It declares status: str (or a Literal of FinalStatus values), weighted_score: float | None, duration: float, plus NotRequired[...] for the optional keys (reference_similarity, grading_ms, cost fields). Pydantic validates TypedDict fields, so the on-disk shape does not change. make typecheck then does the following. It types t["status"] and t["weighted_score"] at markdown.py:476-480, so the status-to-category helper can take a typed argument and not a free str. It reports a missing-key access on a NotRequired key as a pyright error, which settles the question of whether old-format rows can lack status (test_generate_markdown_backward_compatible_task_results) at type-check time, not as a runtime KeyError. It also gives the CE069 helper one typed input shape for both the TaskRow dicts and VariantResult rows. Prevents: The free-str status dispatch behind finding A7 (_category_of(status: str) shared between two renderers), and the unchecked t["status"] KeyError risk on legacy rows that the scope note raised for markdown.py:479. It also makes the shared helper that CE069 requires (findings A5/A3) easy to write, because it needs no Any-typed adapter.

Harness improvements (not statically reachable):

  • A cross-surface headline-score parity test in tests/test_ungraded_reporting.py, next to test_the_three_published_pass_rates_agree_on_one_set_of_counts. Build one canonical row set: SUCCESS 1.0, FAILURE 0.0, ERROR None, TIMEOUT None, NOT_GRADED None. Express it as RunSummary.task_results dicts and as VariantResults / dataset suite rows. Render run.md (ReportGenerator.generate_markdown), experiment.md (VariantAggregate.average_score / _mean_graded_score), the suite rollup (_compute_suite_rollup → suite.json average_weighted_score) and the html.py experiment table. Assert that every surface publishes the same mean, formatted to 3 dp. Mark the deferred suite.json field xfail(strict=True) with the issue link, so the divergence the PR records shows up in every pytest run and turns green when it is fixed. Why not static: CE069 can force one helper, but only rendered output from real row objects can show that every published surface calls it and formats it the same way. A surface that computes its own mean in a form the AST rule does not see (a statistics.mean call, or a loop over a different field name) is caught only by comparing rendered values. Prevents: Finding A3 (no parity test ties run.md's Avg Reliability Score to VariantAggregate.average_score), finding A5's run.md 0.333 against suite.json 0.500 divergence, and the original #257 defect (run.md 1.000 against experiment.md 0.500).
  • An 'unmeasured run publishes no number' invariant test, parametrized over every report writer (run.md, experiment.md, html.py, the GITHUB_STEP_SUMMARY text that action.yml appends) and over execute-shaped summaries: all NOT_GRADED; NOT_GRADED plus unscored ERROR; NOT_GRADED plus unscored TIMEOUT; all with tasks_measured=0. For each one, assert that when nothing_was_measured(not_graded=..., measured=...) is true, no numeric score or rate line (Avg Reliability Score, average_score, pass-rate percent) renders next to Pass Rate: n/a. Also add both sides of the empty-set boundary for the run.md score line: an all-ERROR run renders Avg Reliability Score**: 0.000 exactly, and an all-NOT_GRADED run renders no Avg Reliability line. Tighten test_generate_markdown_with_null_scores (tests/test_reports.py:293) from "N/A" in report_md to an exact assertion on the score line. Whether the score must be gated on verdict evidence is a policy decision (the experiment.py SSOT currently also gives 0.0). The fix and _measured_scores must change in the same change, and this test pins the decision. Why not static: Whether a 0.000 is 'measured zero' or 'nothing measured' depends on runtime counts (tasks_measured, tasks_not_graded) and on which lines end up side by side in the rendered text. An AST rule cannot evaluate that pairing. Only rendering real summaries at the gate boundary shows the branch that turns a gap into a printed score. Prevents: Finding A8 (run.md prints Avg Reliability Score: 0.000 beside Pass Rate: n/a for an execute run with crashed or timed-out rows, the pairing .claude/notes/orchestration.md:945 forbids), and finding A3 (no test for the all-ERROR, all-NOT_GRADED and mixed execute-night boundary cases).
  • Add two items to the Axis 3 (Test Health) checklist in the cr-axis / coder-eval-code-review-full prompts. (a) When a change moves the boundary at which an aggregate's input list becomes empty (if scores:), the reviewer requires a test on each side of the new boundary, not only mixed cases. (b) A parametrized test's name must state every behaviour its cases cover, and each case's intent goes in pytest.param(..., id="..."), not in a trailing comment. Ruff's PT006/PT007 check parametrize argument shapes, not ids, and nothing can check that a name matches the behaviour. Why not static: Whether a test name 'covers' its cases, and whether a filter change moved an empty-set boundary, both need semantic judgment about intent. A rule that requires ids= on every parametrize would mostly produce noise on cases that are already self-describing. Prevents: Finding A1 (the test name test_avg_reliability_score_counts_unscored_rows_as_zero states only the ERROR half; the case labels at tests/test_reports.py:457/459 are comments, not ids), and the boundary-coverage part of finding A3.

Top 5 Priority Actions

  1. Gate the Avg Reliability line in src/coder_eval/reports/markdown.py:476-487 on verdict evidence, the same way the pass rate is gated: omit it or render n/a when summary.pass_rate is None or nothing_was_measured(...) is true. This stops identical agent output under coder-eval execute (NOT_GRADED rows plus unscored ERROR/TIMEOUT rows, tasks_measured=0) from changing the run.md and GITHUB_STEP_SUMMARY headline from absent to a 0.000 score, which .claude/notes/orchestration.md:945 forbids. Add that shape as a third parametrized case at tests/test_reports.py:463.
  2. Move the 'ungraded leaves, unscored counts 0.0' rule into one public helper in src/coder_eval/result_metrics.py that takes (status, weighted_score) pairs, call it from markdown.py:476 and orchestration/experiment.py:800 (_measured_scores), and then delete the inline comment and its Rationale pointer.
  3. Resolve or explicitly record the divergence at src/coder_eval/reports/markdown.py:964 (_compute_suite_rollup), which still drops unscored rows from average_weighted_score and so publishes 0.500 in suite.json where run.md shows 0.333. If you change it, keep rows_measured as a separate verdict-evidence count. If you defer it, add a notes entry or tracked issue so the file does not hold two contradictory means with no marker.
  4. Pin the new empty-scores boundary in tests/test_reports.py: assert 'Avg Reliability Score**: 0.000' for an all-ERROR run, tighten test_generate_markdown_with_null_scores (L293, which now only checks 'N/A'), assert that the line is absent for an all-NOT_GRADED run, and add a parity test that compares the run.md value with f"{_mean_graded_score(rows):.3f}" for one mixed row set (SUCCESS, FAILURE, ERROR, TIMEOUT, NOT_GRADED), so the two surfaces cannot drift again.
  5. Make the private from .junit import _category_of at src/coder_eval/reports/markdown.py:30 a public typed status-category helper, for example in reports/helpers.py or beside FinalStatus.category in models/enums.py, and keep its allowlist plus unknown->error fallback. In the same change, rename the test at tests/test_reports.py:463 to cover both behaviours and replace the case comments at L457/L459 with pytest.param ids.

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

…e mean

Review follow-up. On an execute run whose only non-ungraded rows crashed,
run.md printed "Avg Reliability Score: 0.000" next to "Pass Rate: n/a". The
score line is now gated on nothing_was_measured, the same verdict-evidence
check the pass rate uses.

The averaging rule now lives once, as result_metrics.measured_scores, read by
run.md and VariantAggregate.average_score. The status lookup it needs moves
from junit's private _category_of to FinalStatus.category_of, which junit
also uses now.

Tests cover both sides of the empty-score boundary (all-ERROR scores 0.000;
all-NOT_GRADED and the execute-with-a-crash run show no score), unscored
TIMEOUT and unknown-status rows, and a parity check between run.md and the
variant average. The suite.json average still drops errored rows; that gap is
marked in the code and the notes, and is left for a separate change because
it alters a serialized field.

This branch has not been deployed

No deployments
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.

2 participants