Skip to content

Commit 7c2af1d

Browse files
committed
feat(eval): add an Improvement recommendations section to the eval report (#2472)
Implements the "Improvement recommendations" section from the parent epic (#2087), per docs/evaluation/eval-framework.md §9.1 -- surfaces findings in the per-cell eval report as concrete, targeted actions rather than a bare statement of which metrics failed. eval/recommendations.py deterministically gathers evidence for six finding types (over-grant, under-grant, sensitivity, invariance, consistency, Scale correctness mistake) from data the suites already record, each EvidenceCase carrying the concrete scenario(s)/cell(s) that produced it. A category with no supporting evidence in a given run is omitted entirely, never printed as a vacuous pass statement. One batched structured-LLM call clusters same-root-cause cases into distinct, case-specific recommendations instead of one templated blurb per failing scenario, with a deterministic, clearly-labeled fallback grouping when the LLM call fails or returns nothing usable -- the section is never silently blank or fabricated. Also hardened end-to-end: a failure anywhere in the recommendations pipeline (including a settings/client-construction error, not just a transport failure) can never prevent the rest of the per-cell report from being written. under_grant_cases also surfaces a pair that is simultaneously granted AND denied (e.g. a coarse scope split by the auditor into one approved and one best-effort-rejected sub-decision) -- previously such a pair landed in incorrectly_denied without ever reaching under_grants, so the finding never became an EvidenceCase. eval/conftest.py wires this into pytest_sessionfinish via _collect_recommendation_evidence/_render_recommendations_section, reusing the existing nodeid-marker dicts to attribute each already-recorded property dict to its scenario and finding type. docs/evaluation/eval-framework.md §9.1 records the two implementation decisions the code cites by number: no numeric degradation threshold for the Scale row (reuses the suite's own zero-tolerance gate), and the Consistency cluster-vs-random classification's deterministic-fraction heuristic. eval/test_recommendations.py -- 24 new unit tests covering each evidence-gathering function, the LLM-draft/fallback path, and the end-to-end build_recommendations assembly, plus one live-LLM test (require_env_or_skip-gated) confirming the real batched call merges same-root-cause cases. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Amitfre15 <amitfrework@gmail.com>
1 parent 6610e78 commit 7c2af1d

5 files changed

Lines changed: 971 additions & 10 deletions

File tree

‎docs/evaluation/eval-framework.md‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,14 +275,32 @@ produced whenever the supporting evidence is present:
275275
| Sensitivity family failures (output did not change when it should have) | Flag which policy-text edit types the PRB is insensitive to (e.g. negation words, exception clauses, restriction words like "only"/"just"); recommend adding those edit patterns to the Robustness corpus and reviewing PRB prompts for those constructs. |
276276
| Invariance family failures (output changed when it shouldn't have) | Flag which surface-form changes destabilize the PRB; recommend prompt hardening or normalization pre-processing. |
277277
| Consistency disagreements | Note whether disagreements cluster on specific scenarios (structural prompt sensitivity) or appear random (temperature/batching noise); recommend `temperature=0` enforcement or a retry-with-majority-vote strategy accordingly. |
278-
| Scale correctness degradation above a threshold | Identify whether degradation is in per-decision scale (large candidate lists) or total-corpus scale; recommend context-window management changes (chunking, summarization) or candidate-list pruning strategies respectively. |
278+
| Scale correctness mistake (over-grant, under-grant, or incorrect denial in a Scale-suite scenario) | Identify whether the mistake is in per-decision scale (large candidate lists) or total-corpus scale; recommend context-window management changes (chunking, summarization) or candidate-list pruning strategies respectively. |
279279

280280
Recommendations that have no supporting evidence in the current run are
281281
omitted (not printed as vacuous "no issues found" items). Each
282282
recommendation references the specific failing scenario(s) or metric
283283
cell(s) that produced it, so the reader can verify the evidence directly
284284
in the same report.
285285

286+
**Implementation decisions** (referenced as "decision 1"/"decision 4" in
287+
`eval/recommendations.py`/`eval/conftest.py`):
288+
289+
- **Decision 1 — no numeric degradation threshold for the Scale row.** The Scale
290+
suite's correctness check reuses the same zero-tolerance gate as the
291+
Correctness suites (any over-grant/under-grant/incorrect denial already
292+
fails the test) — there is no accumulated history within a single run to
293+
compute a "rate" or "degradation" against, so any occurrence of a mistake
294+
IS the finding, with no threshold to cross.
295+
- **Decision 4 — Consistency's cluster-vs-random classification is a
296+
deterministic fraction**, computed once per run from the share of
297+
Consistency scenarios that disagreed: at most half → "clusters on these scenario(s)"
298+
(structural prompt sensitivity); more than half → "appears
299+
random/widespread" (temperature/batching noise). This is a heuristic,
300+
not a statistical test, and reads as degenerate on a very small or
301+
`-k`-filtered run (e.g. a single scored scenario) — acceptable because
302+
the suite's documented full-corpus run always scores all 8 scenarios.
303+
286304
## 10. Framework trust
287305

288306
No mutation-testing validation of the harness itself (e.g. deliberately

‎eval/conftest.py‎

Lines changed: 143 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@
6363
``true_positives`` and ``denied_total`` — not rendered in the Markdown report (the floats and pair
6464
breakdowns already cover a human reader) but read back here by ``_write_trend_log`` to pool a
6565
run's precision/recall/denial_precision into one committed, append-only trend-log row per suite
66-
(``eval/trend_log.py``, spec: ``docs/evaluation/eval-framework.md`` §9) — pooled by summed count
66+
(``eval/dashboard/trend_log.py``, spec: ``docs/evaluation/eval-framework.md`` §9) — pooled by summed count
6767
across every scenario that reached ``score_scenario`` in the run, not averaged per-scenario, so a
6868
run with an uneven pair count per scenario isn't skewed by weighting every scenario equally. A
6969
scenario whose own setup failed never recorded these, so it contributes nothing to the pooled row
@@ -80,7 +80,7 @@
8080
committed trend-log row per family/tier combination -- ``suite="robustness_mechanical_invariance"``/
8181
``"robustness_semantic_invariance"``/``"robustness_mechanical_sensitivity"``/
8282
``"robustness_semantic_sensitivity"`` -- each carrying that row's own precision/recall/
83-
denial_precision (``eval/trend_log.py``'s ``pool_correctness_metrics``, the same pooling the two
83+
denial_precision (``eval/dashboard/trend_log.py``'s ``pool_correctness_metrics``, the same pooling the two
8484
Correctness suites use, so the resulting chart is directly comparable to theirs: two measuring
8585
performance against the *original* inputs, two against *deliberately edited/reworded* inputs) plus
8686
that row's own pass/fail rate (``invariance_rate``/``sensitivity_rate``).
@@ -91,7 +91,7 @@
9191
mismatch detail when ``True``) via ``_render_consistency_block``, on every entry, pass or fail,
9292
unlike every other test in this suite besides the correctness/robustness metrics blocks above -- and
9393
pooled by nodeid substring (``_CONSISTENCY_TEST_MARKERS``) into its own committed trend-log row,
94-
``suite="consistency"``, carrying an ``agreement_rate`` (``eval/trend_log.py``'s
94+
``suite="consistency"``, carrying an ``agreement_rate`` (``eval/dashboard/trend_log.py``'s
9595
``pool_consistency_metrics`` -- its own smaller pooling function, since there's no truth table here
9696
to produce a ``pool_correctness_metrics``-shaped precision/recall).
9797
@@ -107,8 +107,25 @@
107107
pooled by nodeid substring (``_SCALE_TEST_MARKERS``, mapping to ``(suite, "structural" |
108108
"correctness")`` rather than a bare suite name) and merged into one row per dimension/level --
109109
``scale_total_corpus_prb``/``scale_per_decision_prb`` (and their ``_e2e`` counterparts) -- via
110-
``eval/trend_log.py``'s ``pool_scale_metrics`` for the structural half and the existing
110+
``eval/dashboard/trend_log.py``'s ``pool_scale_metrics`` for the structural half and the existing
111111
``pool_correctness_metrics`` for the correctness half.
112+
113+
Below the per-category drill-down, ``pytest_sessionfinish`` appends an **"Improvement
114+
recommendations" section** (spec: ``docs/evaluation/eval-framework.md`` §9.1, #2472), generated
115+
after every run and omitted entirely when it has no supporting evidence. ``_collect_recommendation_
116+
evidence`` makes a dedicated pass over ``_reports`` (deliberately separate from ``_write_trend_
117+
log``'s own loop above, which stays untouched) and attributes every already-recorded property dict
118+
to its scenario (via ``_scenario_name_from_nodeid``) and finding type -- over/under-grants from the
119+
two Correctness suites, sensitivity/invariance from their respective Robustness suites (split by
120+
which boolean prop each ``_ROBUSTNESS_TEST_MARKERS`` entry records), inconsistent scenarios from the
121+
Consistency suite (plus a deterministic cluster-vs-random classification, ``_classify_consistency_
122+
disagreements``, computed from the fraction of inconsistent scenarios -- no LLM needed to count),
123+
and over/under-grant/incorrectly-denied mistakes from the Scale suite's correctness tests. That
124+
evidence is handed to ``eval.recommendations.build_recommendations``, which gathers it into
125+
``EvidenceCase``s and makes ONE batched structured-LLM call to cluster them into distinct
126+
recommendation patterns (falling back to a deterministic, clearly-labeled grouping if the call
127+
fails or returns nothing usable) -- ``_render_recommendations_section`` renders the result as one
128+
``###`` sub-heading per recommendation, with its own evidence lines.
112129
"""
113130

114131
from __future__ import annotations
@@ -122,7 +139,8 @@
122139
import pytest
123140
from dotenv import load_dotenv
124141

125-
from eval.trend_log import append_row, pool_consistency_metrics, pool_correctness_metrics, pool_scale_metrics
142+
from eval.dashboard.trend_log import append_row, pool_consistency_metrics, pool_correctness_metrics, pool_scale_metrics
143+
from eval.recommendations import build_recommendations
126144

127145
HERE = Path(__file__).resolve().parent
128146
REPORTS_DIR = HERE / "reports"
@@ -161,6 +179,17 @@ def _is_eval_suite_nodeid(nodeid: str) -> bool:
161179
return nodeid.split("::", 1)[0].startswith(_EVAL_SUITE_PATH_PREFIX)
162180

163181

182+
def _scenario_name_from_nodeid(nodeid: str) -> str:
183+
"""Extract a parametrized test's scenario name from its trailing ``[...]`` (e.g.
184+
``eval/test_policy_pipeline_eval.py::test_prb_correctness[baseline]`` -> ``"baseline"``) --
185+
used by ``_collect_recommendation_evidence`` to attribute each recorded property dict to the
186+
scenario it came from. Returns the nodeid unchanged for a non-parametrized test (e.g. the Scale
187+
suite's structural/correctness tests, which run once per dimension/level, not per scenario)."""
188+
if nodeid.endswith("]") and "[" in nodeid:
189+
return nodeid[nodeid.rindex("[") + 1 : -1]
190+
return nodeid
191+
192+
164193
def pytest_collection_modifyitems(session: pytest.Session, config: pytest.Config, items: list) -> None:
165194
# `_docstrings` is best-effort under `-n`: this hook does run in every worker (each does its own
166195
# full collection), just never in the controller -- so a controller-authored report's "What it
@@ -263,7 +292,7 @@ def _format_pairs_dict(pairs_by_gate: dict) -> str:
263292
"::test_prb_sensitive_to_semantic_perturbation[",
264293
)
265294

266-
# Nodeid substring -> trend-log suite name (eval/trend_log.py). With the five former ``eval_*``
295+
# Nodeid substring -> trend-log suite name (eval/dashboard/trend_log.py). With the five former ``eval_*``
267296
# markers collapsed to one flat ``eval`` marker, every suite under `eval/` is disambiguated by its
268297
# own test function's nodeid rather than by marker — the same ``::test_prb_correctness[``/
269298
# ``::test_e2e_correctness[`` substrings ``_CORRECTNESS_TEST_MARKERS`` already uses. Deliberately
@@ -318,7 +347,7 @@ def _format_pairs_dict(pairs_by_gate: dict) -> str:
318347
# invariance_rate/sensitivity_rate, which _write_trend_log injects on top of pool_correctness_
319348
# metrics' output separately). Kept as its own dict (not folded into _TREND_LOG_SUITES) since its
320349
# entries are pooled by pool_consistency_metrics, not pool_correctness_metrics -- see
321-
# eval/trend_log.py.
350+
# eval/dashboard/trend_log.py.
322351
_CONSISTENCY_TEST_MARKERS = {
323352
"::test_prb_consistent_across_repeats[": "consistency",
324353
}
@@ -327,7 +356,7 @@ def _format_pairs_dict(pairs_by_gate: dict) -> str:
327356
# docs/evaluation/policy-eval-scale.md). Unlike every dict above, each Scale trend-log row
328357
# (e.g. "scale_total_corpus_prb") is built from *two* separate test functions -- a structural test
329358
# (record_property("structural_pass"/"structural_issue_count"/...), pooled by
330-
# eval.trend_log.pool_scale_metrics) and a correctness test (the familiar precision/recall shape,
359+
# eval.dashboard.trend_log.pool_scale_metrics) and a correctness test (the familiar precision/recall shape,
331360
# pooled by the existing pool_correctness_metrics) -- so this dict's value is a
332361
# (suite, "structural" | "correctness") pair rather than a bare suite name; _write_trend_log pools
333362
# each half separately then merges them into one row. Neither test function is parametrized (each
@@ -504,6 +533,102 @@ def _render_scale_block(lines: list[str], props: dict) -> None:
504533
)
505534

506535

536+
def _classify_consistency_disagreements(entries: list[tuple[str, dict]]) -> str:
537+
"""Decision 4 (``docs/evaluation/eval-framework.md`` §9.1, #2472): a minority of this run's
538+
Consistency scenarios inconsistent (<= 50%) classifies as clustering on those scenario(s)
539+
(structural prompt sensitivity); more than half classifies as appearing random/widespread
540+
(temperature/batching noise). Computed deterministically from the fraction of inconsistent
541+
scenarios -- no LLM needed to count -- then handed to the LLM as part of the Consistency
542+
finding's evidence (``eval.recommendations.consistency_cases``) so its pattern-level writeup
543+
stays consistent with this classification rather than re-deriving its own."""
544+
if not entries:
545+
return "no consistency data"
546+
inconsistent = sum(1 for _scenario, props in entries if props.get("inconsistent"))
547+
if inconsistent == 0:
548+
return "no disagreements"
549+
fraction = inconsistent / len(entries)
550+
return "clusters on these scenario(s)" if fraction <= 0.5 else "appears random/widespread"
551+
552+
553+
def _collect_recommendation_evidence() -> dict:
554+
"""One new pass over ``_reports``, deliberately separate from ``_write_trend_log``'s existing
555+
loop (not merged into it) so the already-shipped trend-log writer stays untouched. Reuses the
556+
existing marker dicts as-is (``_TREND_LOG_SUITES``/``_ROBUSTNESS_TEST_MARKERS``/
557+
``_CONSISTENCY_TEST_MARKERS``/``_SCALE_TEST_MARKERS``) but additionally captures each entry's
558+
scenario name (via ``_scenario_name_from_nodeid``) and splits ``_ROBUSTNESS_TEST_MARKERS`` into
559+
sensitivity vs invariance groups by which boolean prop each marker records (``"sensitive"`` vs
560+
``"invariant"``) -- the input shape ``eval.recommendations.build_recommendations`` expects."""
561+
correctness_entries: list[tuple[str, str, dict]] = []
562+
sensitivity_entries: list[tuple[str, str, dict]] = []
563+
invariance_entries: list[tuple[str, str, dict]] = []
564+
consistency_entries: list[tuple[str, dict]] = []
565+
scale_entries: list[tuple[str, dict]] = []
566+
567+
for nodeid, report in _reports.items():
568+
props = dict(report.user_properties)
569+
for substring, suite in _TREND_LOG_SUITES.items():
570+
if substring in nodeid:
571+
if "over_grants" in props or "under_grants" in props:
572+
correctness_entries.append((suite, _scenario_name_from_nodeid(nodeid), props))
573+
break
574+
for substring, (prop_name, robustness_suite, _rate_key) in _ROBUSTNESS_TEST_MARKERS.items():
575+
if substring in nodeid:
576+
if prop_name in props:
577+
scenario = _scenario_name_from_nodeid(nodeid)
578+
if prop_name == "sensitive":
579+
sensitivity_entries.append((robustness_suite, scenario, props))
580+
else:
581+
invariance_entries.append((robustness_suite, scenario, props))
582+
break
583+
for substring in _CONSISTENCY_TEST_MARKERS:
584+
if substring in nodeid:
585+
if "inconsistent" in props:
586+
consistency_entries.append((_scenario_name_from_nodeid(nodeid), props))
587+
break
588+
for substring, (scale_suite, check_type) in _SCALE_TEST_MARKERS.items():
589+
if substring in nodeid and check_type == "correctness":
590+
if "over_grants" in props or "under_grants" in props or "incorrectly_denied" in props:
591+
scale_entries.append((scale_suite, props))
592+
break
593+
594+
return {
595+
"over_grant": correctness_entries,
596+
"under_grant": correctness_entries,
597+
"sensitivity": sensitivity_entries,
598+
"invariance": invariance_entries,
599+
"consistency": consistency_entries,
600+
"consistency_classification": _classify_consistency_disagreements(consistency_entries),
601+
"scale_mistake": scale_entries,
602+
}
603+
604+
605+
def _render_recommendations_section(lines: list[str], evidence: dict) -> None:
606+
"""Calls ``eval.recommendations.build_recommendations`` and renders the "Improvement
607+
recommendations" section below the drill-down (spec §9.1, #2472) -- **omits the whole section**
608+
when the result is empty (no ``_none_`` fallback here, unlike the six pass/fail category
609+
sections above: the spec is explicit that an empty category must not render as a vacuous pass
610+
statement)."""
611+
recommendations = build_recommendations(
612+
over_grant=evidence["over_grant"],
613+
under_grant=evidence["under_grant"],
614+
sensitivity=evidence["sensitivity"],
615+
invariance=evidence["invariance"],
616+
consistency=evidence["consistency"],
617+
consistency_classification=evidence["consistency_classification"],
618+
scale_mistake=evidence["scale_mistake"],
619+
)
620+
if not recommendations:
621+
return
622+
lines.append("## Improvement recommendations")
623+
lines.append("")
624+
for rec in recommendations:
625+
lines.append(f"### {rec.heading}")
626+
lines.append("")
627+
_render_field(lines, "Recommendation", rec.body)
628+
_render_field(lines, "Evidence", "\n".join(rec.evidence))
629+
lines.append("")
630+
631+
507632
def _render_entry(lines: list[str], nodeid: str, report: pytest.TestReport, category: str) -> None:
508633
"""Per-cell tests (``test_inbound``/``test_outbound``) ``record_property`` a concrete
509634
description + expected/actual boolean + explanation; ``test_prb_correctness`` (correctness-prb)
@@ -717,6 +842,16 @@ def pytest_sessionfinish(session: pytest.Session, exitstatus: int) -> None:
717842
for nodeid, report in entries:
718843
_render_entry(lines, nodeid, report, cat)
719844

845+
try:
846+
_render_recommendations_section(lines, _collect_recommendation_evidence())
847+
except Exception as err:
848+
# The six category sections above have already been appended to `lines` by this point --
849+
# an unexpected failure drafting recommendations (a bug in evidence-gathering, or an LLM/
850+
# settings error _draft_patterns itself didn't already sanitize to []) must not cost the
851+
# rest of this already-computed report, which `report_path.write_text` below still owes
852+
# every run regardless of whether this section can be produced.
853+
print(f"\nImprovement recommendations section failed, omitting it: {err!r}")
854+
720855
REPORTS_DIR.mkdir(parents=True, exist_ok=True)
721856
suffix = now.strftime("%d_%m_%H_%M_%S")
722857
report_path = REPORTS_DIR / f"report_{suffix}.md"

0 commit comments

Comments
 (0)