From 94d3455b0445366a3aec5e9271af0165c60e20be Mon Sep 17 00:00:00 2001 From: Lukas Babaliauskas Date: Mon, 12 Oct 2026 20:21:58 +0200 Subject: [PATCH 01/17] feat(insights): facts carry each budget's status and the decision's reason Co-Authored-By: Claude Opus 5.5 (1M context) --- src/evalshift_cli/insights/facts.py | 71 ++++++++++++++++++++++---- tests/unit/insights_factories.py | 27 ++++++++++ tests/unit/test_insight_facts.py | 78 ++++++++++++++++++++++++++++- 3 files changed, 166 insertions(+), 10 deletions(-) diff --git a/src/evalshift_cli/insights/facts.py b/src/evalshift_cli/insights/facts.py index e68fec1..21a2942 100644 --- a/src/evalshift_cli/insights/facts.py +++ b/src/evalshift_cli/insights/facts.py @@ -83,6 +83,14 @@ #: The budgets that are plain counts. _COUNT_BUDGETS: frozenset[str] = frozenset({"max_critical_regressions", "max_invariant_violations"}) +#: Budget status words, rendered beside each budget's limit in the FACTS +#: block. Digit-free plain English — the model copies them into prose — and +#: the only vocabulary the blame validator keys on. +BUDGET_BREACHED = "breached" +BUDGET_WITHIN = "within" +BUDGET_UNMEASURED = "measured nothing" +BUDGET_UNCONFIRMED = "over its limit but not confirmed at this sample size" + @dataclass(frozen=True, slots=True) class ExampleFact: @@ -158,6 +166,17 @@ class Facts: #: naming it as why a run failed would be false. ``blocking_evaluators`` #: keeps every scope, for the prose that lists them rather than blames them. overall_blocking_evaluators: list[str] = field(default_factory=list) + #: One ``(display label, rendered limit, status)`` per gating budget row — + #: the overall rows and the distinct slice rows, in policy order. Handed to + #: the model beside each limit so a FAIL's cause is read off the line, not + #: guessed: a narrative once blamed "Trace-rule violations" on a run where + #: that budget held and only the latency ceiling was breached, because the + #: prompt listed limits without results. + budget_statuses: list[tuple[str, str, str]] = field(default_factory=list) + #: The decision's own explanation of its verdict + #: (``MigrationDecision.reason``), or ``""`` when it carries none. The same + #: sentence the report hero renders, so the two surfaces cannot disagree. + verdict_reason: str = "" def build_facts( @@ -218,10 +237,15 @@ def build_facts( _COVERAGE_BASIS_UNMEASURED if (unmeasured_budgets or unmeasured_evaluators) else "" ) failed_budgets = _unique( - _failed_budget_label(b) + _scoped_budget_label(b) for b in gating_budget_results(decision) if not b.passed and b.conclusive ) + budget_statuses = [ + (_scoped_budget_label(b), _budget_limit(b.name, b.allowed), _budget_status(b)) + for b in gating_budget_results(decision) + ] + verdict_reason = decision.reason or "" rendered: dict[str, str] = { "verdict": decision.verdict.replace("_", " ").upper(), @@ -286,6 +310,8 @@ def build_facts( allowed_numbers=_allowed_numbers( rendered=rendered, budget_limits=budget_limits, + slice_limits=[limit for _, limit, _ in budget_statuses], + verdict_reason=verdict_reason, failure_categories=failure_categories, model_ids=(state.models.source, state.models.target), n_examples=len(examples), @@ -308,6 +334,8 @@ def build_facts( overall_blocking_evaluators=_unique( r.evaluator_name for r in decision.blocking_regressions if r.slice_name == ALL_SLICE ), + budget_statuses=budget_statuses, + verdict_reason=verdict_reason, ) @@ -357,13 +385,13 @@ def _p_value(value: float) -> str: return f"< {P_VALUE_FLOOR}" if value < P_VALUE_FLOOR else f"{value:.4f}" -def _failed_budget_label(budget: BudgetResult) -> str: - """A breached budget's display name, scoped when it is a slice's own row. +def _scoped_budget_label(budget: BudgetResult) -> str: + """A budget's display name, scoped when it is a slice's own row. - A bare "overall regression rate" for a slice breach would send the reader - to the overall row — green in exactly the run this names. Worded so the + A bare "overall regression rate" for a slice row would send the reader to + the overall row — green in exactly the run that names it. Worded so the templated advice reads "the 'security' slice overall regression rate - budget was breached". + budget was breached" and the FACTS block lists the row under that name. """ label = BUDGET_LABELS.get(budget.name, budget.name) if budget.scope == "overall": @@ -379,6 +407,21 @@ def _budget_limit(name: str, allowed: float) -> str: return _rate_pct(allowed) +def _budget_status(budget: BudgetResult) -> str: + """Which of the four things a budget row says, as a word the prose may use. + + ``measured`` first: the ``0/0`` default passes by arithmetic, and calling + it ``within`` is the clean-sweep claim the coverage facts exist to stop. + """ + if not budget.measured: + return BUDGET_UNMEASURED + if budget.passed: + return BUDGET_WITHIN + if budget.conclusive: + return BUDGET_BREACHED + return BUDGET_UNCONFIRMED + + # --------------------------------------------------------------------------- # Helpers # --------------------------------------------------------------------------- @@ -416,6 +459,8 @@ def _allowed_numbers( *, rendered: dict[str, str], budget_limits: dict[str, str], + slice_limits: Sequence[str], + verdict_reason: str, failure_categories: Sequence[tuple[str, int]], model_ids: tuple[str, ...], n_examples: int, @@ -424,14 +469,18 @@ def _allowed_numbers( Beyond the rendered figures themselves this admits each one's bare-numeral form (``+102%`` also admits ``102``), the counts behind the budget table - and failure categories, the numerals inside the model ids — writing - ``gemini-3.1-flash`` must not read as an invented figure — and every - integer up to ``n_examples`` so the model can write "15 of 21". + and failure categories, the slice budgets' limits (rendered only in the + status lines) and the decision reason's own figures, the numerals inside + the model ids — writing ``gemini-3.1-flash`` must not read as an invented + figure — and every integer up to ``n_examples`` so the model can write + "15 of 21". """ allowed: set[str] = {str(index) for index in range(n_examples + 1)} sources: list[str] = [ *rendered.values(), *budget_limits.values(), + *slice_limits, + verdict_reason, *(str(count) for _, count in failure_categories), *model_ids, ] @@ -449,6 +498,10 @@ def _unique(values: Iterable[str]) -> list[str]: __all__ = [ + "BUDGET_BREACHED", + "BUDGET_UNCONFIRMED", + "BUDGET_UNMEASURED", + "BUDGET_WITHIN", "NOT_AVAILABLE", "NOT_COMPARABLE", "NOT_MEASURED", diff --git a/tests/unit/insights_factories.py b/tests/unit/insights_factories.py index 7e2e566..b1f6e3c 100644 --- a/tests/unit/insights_factories.py +++ b/tests/unit/insights_factories.py @@ -256,6 +256,33 @@ def unmeasured_budget(name: str = "max_tool_divergence") -> BudgetResult: ) +#: A severity-only fail's ``MigrationDecision.reason``, as the policy words it. +#: Carries figures: a generation that copies them must not be rejected. +SEVERITY_REASON = ( + "semantic.cosine regressed at critical severity on prompt replay " + "(effect size -2.15, adjusted p 0.0004). A critical or high regression from a " + "blocking evaluator fails the run on its own, whatever the budgets say." +) + + +def converter_decision() -> MigrationDecision: + """r_20261012_converter_d418e5: the latency ceiling breached, every other budget held. + + The run whose narrative blamed "Trace-rule violations" — a budget that + passed at 0 against a limit of 1 — because the prompt listed each budget's + limit without its result. Six held budgets, one breached, one blind. + """ + return decision( + verdict="fail", + budget_results=[ + *budgets()[:5], + BudgetResult(name="max_latency_increase", observed=1.753, allowed=0.30, passed=False), + BudgetResult(name="max_invariant_violations", observed=0.0, allowed=1.0, passed=True), + unmeasured_budget(), + ], + ) + + def unmeasured_comparison(evaluator_name: str) -> ComparisonResult: """An evaluator that was handed pairs and produced no comparable row. diff --git a/tests/unit/test_insight_facts.py b/tests/unit/test_insight_facts.py index e7db20a..e4708d3 100644 --- a/tests/unit/test_insight_facts.py +++ b/tests/unit/test_insight_facts.py @@ -6,6 +6,7 @@ from __future__ import annotations +from dataclasses import replace from typing import Any from evalshift_cli.analysis.policy import ( @@ -17,14 +18,22 @@ from evalshift_cli.analysis.statistics import ComparisonResult from evalshift_cli.config.models import MigrationPolicy, SliceMigrationPolicy from evalshift_cli.evaluators.base import EvalRecord -from evalshift_cli.insights.facts import build_facts +from evalshift_cli.insights.facts import ( + BUDGET_BREACHED, + BUDGET_UNCONFIRMED, + BUDGET_UNMEASURED, + BUDGET_WITHIN, + build_facts, +) from evalshift_cli.insights.templates import fallback_insight from evalshift_cli.reports.economics import PromptEconomics from evalshift_cli.runner.models import Call from tests.unit.insights_factories import ( COST_TARGET, + SEVERITY_REASON, budgets, comparisons, + converter_decision, decision, records, role, @@ -677,3 +686,70 @@ def test_the_critical_count_reads_the_blocking_regressions(sample_run: dict[str, facts = build_facts(**sample_run) assert facts.rendered["critical_regressions"] == "1" assert "1 blocking regression (1 critical)" in fallback_insight(facts).verdict_summary + + +# --------------------------------------------------------------------------- +# Budget status and the decision's reason — what a FAIL's cause is read from +# --------------------------------------------------------------------------- + + +def _status_of(facts_statuses: list[tuple[str, str, str]], label: str) -> tuple[str, str]: + (hit,) = [(limit, status) for name, limit, status in facts_statuses if name == label] + return hit + + +def test_every_gating_budget_carries_a_status(sample_run: dict[str, Any]) -> None: + """The converter run: one breached, one blind, the rest held.""" + sample_run["decision"] = converter_decision() + facts = build_facts(**sample_run) + + assert len(facts.budget_statuses) == 8 + assert _status_of(facts.budget_statuses, "Latency increase") == ("+30%", BUDGET_BREACHED) + assert _status_of(facts.budget_statuses, "Trace-rule violations") == ("1", BUDGET_WITHIN) + assert _status_of(facts.budget_statuses, "Cost increase") == ("+200%", BUDGET_WITHIN) + assert _status_of(facts.budget_statuses, "Tool-selection divergence") == ( + "20%", + BUDGET_UNMEASURED, + ) + assert facts.failed_budgets == ["Latency increase"] + + +def test_an_unconfirmed_breach_has_its_own_status(sample_run: dict[str, Any]) -> None: + over = BudgetResult( + name="max_overall_regression_rate", + observed=0.1, + allowed=0.03, + passed=False, + conclusive=False, + ci_low=0.0, + ci_high=0.3, + ) + sample_run["decision"] = decision(verdict="inconclusive", budget_results=[over, *budgets()[1:]]) + facts = build_facts(**sample_run) + assert _status_of(facts.budget_statuses, "Overall regression rate") == ( + "3%", + BUDGET_UNCONFIRMED, + ) + assert facts.failed_budgets == [] + + +def test_a_slice_budget_is_listed_under_its_scope(sample_run: dict[str, Any]) -> None: + """A slice row is a gating budget too, and its limit is a copyable figure.""" + facts = build_facts(**_slice_budget_run(sample_run, extra=[])) + assert _status_of(facts.budget_statuses, "'security' slice overall regression rate") == ( + "0%", + BUDGET_BREACHED, + ) + assert "0%" in facts.allowed_numbers + + +def test_the_decision_reason_travels_with_the_facts(sample_run: dict[str, Any]) -> None: + sample_run["decision"] = replace(decision(verdict="fail"), reason=SEVERITY_REASON) + facts = build_facts(**sample_run) + assert facts.verdict_reason == SEVERITY_REASON + # Its figures are facts the model may copy. + assert {"-2.15", "2.15", "0.0004"} <= facts.allowed_numbers + + +def test_a_decision_without_a_reason_renders_none(sample_run: dict[str, Any]) -> None: + assert build_facts(**sample_run).verdict_reason == "" From 0eb24ee3ce60e826cb57d254d5a9ca2604935aa4 Mon Sep 17 00:00:00 2001 From: Lukas Babaliauskas Date: Mon, 12 Oct 2026 20:24:56 +0200 Subject: [PATCH 02/17] test(insights): make the slice-limit allow-list test bite Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/unit/test_insight_facts.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/tests/unit/test_insight_facts.py b/tests/unit/test_insight_facts.py index e4708d3..bfa1709 100644 --- a/tests/unit/test_insight_facts.py +++ b/tests/unit/test_insight_facts.py @@ -735,12 +735,20 @@ def test_an_unconfirmed_breach_has_its_own_status(sample_run: dict[str, Any]) -> def test_a_slice_budget_is_listed_under_its_scope(sample_run: dict[str, Any]) -> None: """A slice row is a gating budget too, and its limit is a copyable figure.""" - facts = build_facts(**_slice_budget_run(sample_run, extra=[])) + # A slice limit no overall figure shares, so only the status lines can + # have admitted it to the allow-list. + policy = MigrationPolicy( + max_overall_regression_rate=0.5, + min_equivalence_rate=0.0, + slices={"security": SliceMigrationPolicy(max_overall_regression_rate=0.07)}, + ) + facts = build_facts(**_slice_budget_run(sample_run, extra=[], policy=policy)) assert _status_of(facts.budget_statuses, "'security' slice overall regression rate") == ( - "0%", + "7%", BUDGET_BREACHED, ) - assert "0%" in facts.allowed_numbers + assert "7%" not in facts.budget_limits.values() + assert {"7%", "7"} <= facts.allowed_numbers def test_the_decision_reason_travels_with_the_facts(sample_run: dict[str, Any]) -> None: From f9960883548a7b387fb2cd2190ca75e75e20efd6 Mon Sep 17 00:00:00 2001 From: Lukas Babaliauskas Date: Mon, 12 Oct 2026 20:27:46 +0200 Subject: [PATCH 03/17] fix(insights): tell the narrative which budget failed the run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The prompt listed each budget's limit with no result, so a FAIL on the latency ceiling alone was explained as 'failed to meet the budget for Trace-rule violations' — a budget that held. Each budget line now ends with its status, the breached ones ride on failed_budgets, the decision's reason travels with them, and the instruction allows only a breached budget to be named as the cause. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/evalshift_cli/insights/generator.py | 20 +++++- tests/unit/test_insight_generator.py | 93 ++++++++++++++++++++++++- 2 files changed, 109 insertions(+), 4 deletions(-) diff --git a/src/evalshift_cli/insights/generator.py b/src/evalshift_cli/insights/generator.py index d3d2b66..3a9292a 100644 --- a/src/evalshift_cli/insights/generator.py +++ b/src/evalshift_cli/insights/generator.py @@ -93,6 +93,12 @@ fact is present, never write that all budgets passed, that every constraint or requirement is met, or that the migration is safe — name the blind gates and say their silence is unknown. +- Every "budget" line ends with its status. Only a budget whose status is + "breached" — the same ones listed under "failed_budgets" — may be named as + why the run failed or as something to fix. A budget marked "within" held: + never write that it was breached, exceeded, failed, missed, violated or + must be addressed. When "verdict_reason" is present it is the decision's + own explanation of the verdict, and your verdict summary must agree with it. - Findings describe BEHAVIORAL changes visible in the sampled outputs — what the target does differently from the source. They are not restatements of the statistics; the report already renders those. @@ -139,6 +145,8 @@ def __init__(self, message: str, *, tokens: list[str] | None = None) -> None: "worst_evaluator", "rates_basis", "coverage_basis", + "failed_budgets", + "verdict_reason", "blocking_evaluators", "unmeasured_budgets", "unmeasured_evaluators", @@ -280,11 +288,17 @@ def _facts_block(facts: Facts) -> str: lines.append(f"unmeasured_evaluators: {', '.join(facts.unmeasured_evaluators)}") if facts.coverage_basis: lines.append(f"coverage_basis: {facts.coverage_basis}") + if facts.failed_budgets: + lines.append(f"failed_budgets: {', '.join(facts.failed_budgets)}") + if facts.verdict_reason: + lines.append(f"verdict_reason: {facts.verdict_reason}") # Budgets and categories reach the model under their display names: it is # told to copy tokens verbatim, so whatever appears here is what a reader - # may end up seeing in the narrative. - for name, limit in facts.budget_limits.items(): - lines.append(f'budget "{BUDGET_LABELS.get(name, name)}": {limit}') + # may end up seeing in the narrative. Each budget carries its status, so + # the cause of a FAIL is read off the line rather than guessed from the + # limit — a narrative once blamed a budget that held. + for label, limit, status in facts.budget_statuses: + lines.append(f'budget "{label}": limit {limit}, {status}') for category, count in facts.failure_categories: lines.append(f'failure_category "{category_label(category)}": {count}') body = "\n".join(lines) diff --git a/tests/unit/test_insight_generator.py b/tests/unit/test_insight_generator.py index 30ad1a9..5bfcb10 100644 --- a/tests/unit/test_insight_generator.py +++ b/tests/unit/test_insight_generator.py @@ -8,6 +8,7 @@ from __future__ import annotations import json +from dataclasses import replace from typing import Any, cast import pytest @@ -17,8 +18,10 @@ from evalshift_cli.insights.generator import generate_insight, validate_numbers from evalshift_cli.models.client import ModelClient from tests.unit.insights_factories import ( + SEVERITY_REASON, FakeModelClient, budgets, + converter_decision, decision, unmeasured_budget, unmeasured_comparison, @@ -421,7 +424,7 @@ async def test_the_prompt_names_the_blocking_evaluators_and_failure_categories( # Categories and budgets reach the model already humanised, so the tokens # it can copy into prose are the ones a reader may see. assert 'failure_category "Missing field": 15' in prompt - assert 'budget "Cost increase": +200%' in prompt + assert 'budget "Cost increase": limit +200%, within' in prompt assert "failure_category.missing_field" not in prompt assert "max_cost_increase" not in prompt @@ -516,3 +519,91 @@ async def test_a_fully_measured_run_carries_no_coverage_facts( assert "\nunmeasured_budgets: " not in prompt assert "\nunmeasured_evaluators: " not in prompt assert "\ncoverage_basis: " not in prompt + + +# --------------------------------------------------------------------------- +# A FAIL's cause is a fact, not an inference +# --------------------------------------------------------------------------- + + +async def test_each_budget_line_carries_its_status( + fake_client: FakeModelClient, sample_run: dict[str, Any] +) -> None: + """The converter run: the model is told which budget broke and which held. + + The shipped narrative blamed "Trace-rule violations" because every budget + line read as a bare limit — nothing said which one the run had failed on. + """ + sample_run["decision"] = converter_decision() + fake_client.queue_responses(_payload()) + await _generate(build_facts(**sample_run), fake_client) + + prompt = fake_client.prompts[0] + assert '\nbudget "Latency increase": limit +30%, breached\n' in prompt + assert '\nbudget "Trace-rule violations": limit 1, within\n' in prompt + assert '\nbudget "Tool-selection divergence": limit 20%, measured nothing\n' in prompt + assert "\nfailed_budgets: Latency increase\n" in prompt + + +async def test_a_passing_run_lists_no_failed_budgets_and_no_reason( + fake_client: FakeModelClient, sample_facts: Facts +) -> None: + fake_client.queue_responses(_payload()) + await _generate(sample_facts, fake_client) + + prompt = fake_client.prompts[0] + assert "\nfailed_budgets: " not in prompt + assert "\nverdict_reason: " not in prompt + assert '\nbudget "Cost increase": limit +200%, within\n' in prompt + + +async def test_the_verdict_reason_reaches_the_prompt( + fake_client: FakeModelClient, sample_run: dict[str, Any] +) -> None: + sample_run["decision"] = replace(decision(verdict="fail"), reason=SEVERITY_REASON) + fake_client.queue_responses(_payload()) + await _generate(build_facts(**sample_run), fake_client) + + assert f"\nverdict_reason: {SEVERITY_REASON}\n" in fake_client.prompts[0] + + +async def test_a_copied_reason_figure_is_not_an_invented_number( + fake_client: FakeModelClient, sample_run: dict[str, Any] +) -> None: + """The reason's own figures are facts; copying them must not cost a retry.""" + sample_run["decision"] = replace(decision(verdict="fail"), reason=SEVERITY_REASON) + fake_client.queue_responses( + _payload(verdict_summary="semantic.cosine regressed with effect size -2.15 (p 0.0004).") + ) + insight = await _generate(build_facts(**sample_run), fake_client) + + assert insight.verdict_summary.endswith("(p 0.0004).") + assert fake_client.call_count == 1 + + +async def test_the_instruction_rules_on_budget_status( + fake_client: FakeModelClient, sample_facts: Facts +) -> None: + """A plain-English accusation carries no digit; the rule has to be spelled out.""" + fake_client.queue_responses(_payload()) + await _generate(sample_facts, fake_client) + + instruction = fake_client.prompts[0].split("FACTS (")[0] + assert '"breached"' in instruction + assert '"within"' in instruction + assert "failed_budgets" in instruction + assert "verdict_reason" in instruction + + +async def test_an_echoed_failed_budgets_key_is_rejected_and_retried( + fake_client: FakeModelClient, sample_run: dict[str, Any] +) -> None: + sample_run["decision"] = converter_decision() + fake_client.queue_responses( + _payload(verdict_summary="failed_budgets: Latency increase."), + _payload(verdict_summary="The latency ceiling of +30% was exceeded."), + ) + insight = await _generate(build_facts(**sample_run), fake_client) + + assert insight.verdict_summary == "The latency ceiling of +30% was exceeded." + assert fake_client.call_count == 2 From 69428206ac1ad8e85e7dcba3f9c660124e19bbbd Mon Sep 17 00:00:00 2001 From: Lukas Babaliauskas Date: Mon, 12 Oct 2026 20:31:49 +0200 Subject: [PATCH 04/17] fix(insights): keep config keys out of the verdict reason the narrative copies The decision's reason travels into a FACTS block the model copies verbatim, but the policy words it with evalshift.yaml keys: a trace-rule FAIL quoted migration_policy.max_invariant_violations, a banned token that cost a faithful generation its retry, and a dropped-params FAIL quoted fail_on_dropped_params and drop_params, which would ship to readers. The reason now reaches the facts in plain words, figures intact. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/evalshift_cli/insights/facts.py | 21 ++++++++++++++++- tests/unit/test_insight_facts.py | 36 +++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/src/evalshift_cli/insights/facts.py b/src/evalshift_cli/insights/facts.py index 21a2942..785326c 100644 --- a/src/evalshift_cli/insights/facts.py +++ b/src/evalshift_cli/insights/facts.py @@ -245,7 +245,7 @@ def build_facts( (_scoped_budget_label(b), _budget_limit(b.name, b.allowed), _budget_status(b)) for b in gating_budget_results(decision) ] - verdict_reason = decision.reason or "" + verdict_reason = _plain_reason(decision.reason or "") rendered: dict[str, str] = { "verdict": decision.verdict.replace("_", " ").upper(), @@ -451,6 +451,25 @@ def _regression_samples(examples: Sequence[ExampleFact]) -> list[RegressionSampl ] +def _plain_reason(reason: str) -> str: + """The decision's reason with the config keys it quotes put into words. + + The reason enters a FACTS block the model is told to copy verbatim, and the + policy's own prose quotes ``evalshift.yaml`` keys the narrative may not + show — a banned one costs a faithful generation its retry, an unbanned one + ships to readers as config vocabulary. Figures are left untouched. + """ + for key, label in BUDGET_LABELS.items(): + reason = reason.replace( + f"migration_policy.{key}", f"the {label[:1].lower()}{label[1:]} budget" + ) + reason = reason.replace( + "migration_policy.fail_on_dropped_params", + "the policy's fail-on-dropped-parameters setting", + ) + return reason.replace("(drop_params)", "(parameters dropped)") + + def _bounded(text: str | None) -> str: return (text or "")[:MAX_SAMPLE_TEXT_CHARS] diff --git a/tests/unit/test_insight_facts.py b/tests/unit/test_insight_facts.py index bfa1709..4b1d590 100644 --- a/tests/unit/test_insight_facts.py +++ b/tests/unit/test_insight_facts.py @@ -13,6 +13,8 @@ BlockingRegression, BudgetResult, FailureCategoryCount, + _dropped_params_reason, + _invariant_reason, evaluate_migration_policy, ) from evalshift_cli.analysis.statistics import ComparisonResult @@ -25,6 +27,7 @@ BUDGET_WITHIN, build_facts, ) +from evalshift_cli.insights.generator import banned_identifiers from evalshift_cli.insights.templates import fallback_insight from evalshift_cli.reports.economics import PromptEconomics from evalshift_cli.runner.models import Call @@ -761,3 +764,36 @@ def test_the_decision_reason_travels_with_the_facts(sample_run: dict[str, Any]) def test_a_decision_without_a_reason_renders_none(sample_run: dict[str, Any]) -> None: assert build_facts(**sample_run).verdict_reason == "" + + +def test_a_trace_rule_reason_reaches_the_facts_without_its_config_key( + sample_run: dict[str, Any], +) -> None: + """The policy quotes ``migration_policy.max_invariant_violations``; the facts must not. + + The reason lands in a copy-verbatim block, and that key is a banned + identifier — a faithful copy would be rejected on exactly these runs. + """ + breach = BudgetResult( + name="max_invariant_violations", observed=2.0, allowed=1.0, passed=False, denominator=10 + ) + sample_run["decision"] = replace(decision(verdict="fail"), reason=_invariant_reason([breach])) + facts = build_facts(**sample_run) + + assert "the trace-rule violations budget" in facts.verdict_reason + assert "migration_policy." not in facts.verdict_reason + assert not [t for t in banned_identifiers(facts) if t in facts.verdict_reason] + assert {"2", "10", "1"} <= facts.allowed_numbers + + +def test_a_dropped_params_reason_reaches_the_facts_in_plain_words( + sample_run: dict[str, Any], +) -> None: + """Config vocabulary is not banned, so it would otherwise ship to readers.""" + reason = _dropped_params_reason({"anthropic/claude-sonnet-5-5": ["temperature"]}) + sample_run["decision"] = replace(decision(verdict="fail"), reason=reason) + facts = build_facts(**sample_run) + + assert "migration_policy." not in facts.verdict_reason + assert "drop_params" not in facts.verdict_reason + assert "anthropic/claude-sonnet-5-5 dropped temperature" in facts.verdict_reason From fd0ae7f3096c8120b6a150dea2462b683cb6ca10 Mon Sep 17 00:00:00 2001 From: Lukas Babaliauskas Date: Mon, 12 Oct 2026 20:35:54 +0200 Subject: [PATCH 05/17] fix(insights): reject a narrative that blames a budget that held On a FAIL the verdict summary and recommendation are scanned for a 'within' budget sharing a clause with a non-negated blame word. A hit is rejected like an invented number, the retry names the labels, and two rejections ship the templated prose, which already names the breached budget. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/evalshift_cli/insights/generator.py | 128 +++++++++++++++++- tests/unit/test_insight_generator.py | 167 +++++++++++++++++++++++- 2 files changed, 287 insertions(+), 8 deletions(-) diff --git a/src/evalshift_cli/insights/generator.py b/src/evalshift_cli/insights/generator.py index 3a9292a..d785901 100644 --- a/src/evalshift_cli/insights/generator.py +++ b/src/evalshift_cli/insights/generator.py @@ -6,7 +6,9 @@ means the model calculated something, and in a report that gates merges a derived number is a defect even when it happens to be right. -Two attempts, then :func:`evalshift_cli.insights.templates.fallback_insight`. The +Three checks on the way out — invented figures, echoed identifiers, and on a +FAIL a held budget blamed for the verdict (:func:`validate_blame`). Two +attempts, then :func:`evalshift_cli.insights.templates.fallback_insight`. The prose is never load-bearing enough to fail a run over — every headline figure on the report is rendered from data, and this module only writes the sentences around them. @@ -17,12 +19,13 @@ import json import logging import re +from collections.abc import Sequence from datetime import UTC, datetime from typing import Any, cast from evalshift_cli.analysis.policy import BUDGET_LABELS from evalshift_cli.evaluators.failures import CATEGORY_LABELS, category_label -from evalshift_cli.insights.facts import Facts, RegressionSample +from evalshift_cli.insights.facts import BUDGET_WITHIN, Facts, RegressionSample from evalshift_cli.insights.models import ( INSIGHT_KINDS, MAX_FINDING_DETAIL_CHARS, @@ -71,6 +74,31 @@ #: outermost braces of a fenced or prose-wrapped object. _JSON_OBJECT_RE = re.compile(r"\{.*\}", re.DOTALL) +#: Words that lay a verdict at a budget's door. A budget marked "within" that +#: shares a clause with one of these, un-negated, is a cause the model +#: invented: a shipped narrative said a run "failed to meet the budget for +#: Trace-rule violations" when that budget held and only latency broke. +_BLAME_RE = re.compile( + r"\b(?:breach\w*|exceed\w*|fail\w*|violat\w*|address\w*|miss(?:ed|es|ing)?|" + r"unmet|not met|over (?:the|its)|above (?:the|its)|broke|broken)\b", + re.IGNORECASE, +) + +#: A blame word shortly after one of these is a denial ("was not breached", +#: "no breach of"), not an accusation. Bounded so a "not" three clauses back +#: cannot excuse an accusation. +_NEGATION_RE = re.compile(r"\b(?:not|no|never|without)\b[^.;,]{0,24}$", re.IGNORECASE) + +#: Clause boundaries. A contrast word starts a new claim, so "latency was +#: exceeded while Trace-rule violations stayed within budget" blames only +#: latency. A bare "and" is deliberately not one: "exceeded latency and failed +#: to meet Trace-rule violations" is the shipped sentence, and both halves +#: accuse. +_CLAUSE_RE = re.compile( + r"[.;:!?]\s+|\s(?:while|but|whereas|although|though|however)\s|,\s*(?:and|but)\s", + re.IGNORECASE, +) + _INSTRUCTION = """\ You are writing the explanation section of a model-migration report. The reader is an engineer deciding whether to switch their production model to the target. @@ -128,11 +156,20 @@ class GenerationError(Exception): tokens: Offending tokens — invented figures or echoed internal identifiers — fed back into the retry so the second attempt can correct them. Empty for every other kind of rejection. + misblamed: Display labels of budgets that held but were blamed for + the verdict — fed back the same way, under their own hint. """ - def __init__(self, message: str, *, tokens: list[str] | None = None) -> None: + def __init__( + self, + message: str, + *, + tokens: list[str] | None = None, + misblamed: list[str] | None = None, + ) -> None: super().__init__(message) self.tokens = tokens or [] + self.misblamed = misblamed or [] #: FACTS field names that are not ``rendered`` keys but appear in the prompt @@ -199,6 +236,50 @@ def validate_numbers(text: str, allowed: frozenset[str]) -> list[str]: return [token for token in NUMERIC_TOKEN_RE.findall(text) if not _is_allowed(token, allowed)] +def validate_blame(text: str, within: Sequence[str]) -> list[str]: + """Return the budgets in ``within`` that ``text`` blames for the verdict. + + A budget that held may be mentioned — "Trace-rule violations stayed within + budget" is accurate — but not accused: it may not share a clause with a + blame word unless that word is negated. A plain-English accusation carries + no digit for :func:`validate_numbers` and no identifier for + :func:`validate_identifiers`, so this is the only check that sees it. + + A safety net, not a parser: it catches the shipped phrasing and its close + variants, and a false match costs one retry. Labels are matched + case-insensitively with hyphens and runs of whitespace folded to one space. + + Args: + text: One prose field of a generation. + within: Display labels of the budgets whose status is + :data:`~evalshift_cli.insights.facts.BUDGET_WITHIN`. + + Returns: + The wrongly blamed labels, each once, in ``within`` order. Empty means + valid. + """ + needles = {label: _normalise(label) for label in within} + blamed: set[str] = set() + for clause in (_normalise(c) for c in _CLAUSE_RE.split(text)): + present = [label for label, needle in needles.items() if needle in clause] + if not present: + continue + # Every held label comes out before the scan, not just the one under + # test: "Trace-rule violations" would otherwise read as a violation + # of whichever budget shares its clause. + rest = clause + for needle in needles.values(): + rest = rest.replace(needle, " ") + if any(not _NEGATION_RE.search(rest[: m.start()]) for m in _BLAME_RE.finditer(rest)): + blamed.update(present) + return [label for label in within if label in blamed] + + +def _normalise(text: str) -> str: + """Lower-case with hyphens and whitespace runs folded, for label matching.""" + return re.sub(r"[-\s]+", " ", text.lower()) + + async def generate_insight(facts: Facts, *, model: str, client: ModelClient) -> Insight: """Write the run's narrative, or fall back to deterministic prose. @@ -215,10 +296,11 @@ async def generate_insight(facts: Facts, *, model: str, client: ModelClient) -> :data:`MAX_ATTEMPTS` unusable generations. """ invented: list[str] = [] + misblamed: list[str] = [] for attempt in range(1, MAX_ATTEMPTS + 1): response = await client.complete( model=model, - prompt=build_prompt(facts, invented=invented), + prompt=build_prompt(facts, invented=invented, misblamed=misblamed), temperature=0.0, ) log.debug( @@ -232,6 +314,7 @@ async def generate_insight(facts: Facts, *, model: str, client: ModelClient) -> except GenerationError as exc: log.warning("insights generation attempt %d rejected: %s", attempt, exc) invented = exc.tokens + misblamed = exc.misblamed log.warning( "insights generation produced nothing usable in %d attempts; using templated prose", MAX_ATTEMPTS, @@ -239,7 +322,12 @@ async def generate_insight(facts: Facts, *, model: str, client: ModelClient) -> return fallback_insight(facts) -def build_prompt(facts: Facts, *, invented: list[str] | None = None) -> str: +def build_prompt( + facts: Facts, + *, + invented: list[str] | None = None, + misblamed: list[str] | None = None, +) -> str: """Assemble the instruction, the FACTS block and the regression samples. Args: @@ -248,6 +336,9 @@ def build_prompt(facts: Facts, *, invented: list[str] | None = None) -> str: or echoed internal identifiers. Present only on a retry, where naming them is the whole reason the second call is worth paying for. + misblamed: Budgets that held but a previous attempt blamed for the + verdict. Named separately from ``invented``: the label itself is + allowed in prose, the accusation is not. """ sections = [ _INSTRUCTION.format( @@ -265,6 +356,14 @@ def build_prompt(facts: Facts, *, invented: list[str] | None = None) -> str: + ". Figures must be copied verbatim from FACTS; internal " "identifiers must be rewritten as plain language." ) + if misblamed: + sections.append( + "Your previous answer blamed budgets that held: " + + ", ".join(dict.fromkeys(misblamed)) + + ". Only the budgets under failed_budgets were breached; a budget " + "marked within may be described as within policy or left out, never " + "as a cause of the verdict or as something to address." + ) return "\n\n".join(section for section in sections if section) @@ -347,10 +446,24 @@ def _build_insight(text: str, *, facts: Facts, model: str) -> Insight: for field in (*prose.values(), *(t for f in findings for t in (f.title, f.detail))): offending.extend(validate_numbers(field, facts.allowed_numbers)) offending.extend(t for t in validate_identifiers(field, banned) if t not in offending) - if offending: + # Only a FAIL assigns a cause, and only its verdict summary and + # recommendation do the assigning; the advisory and economics summaries + # describe signals, and findings describe behaviour. + misblamed: list[str] = [] + if facts.verdict == "fail": + held = [label for label, _, status in facts.budget_statuses if status == BUDGET_WITHIN] + for field in (prose["verdict_summary"], prose["recommendation"]): + misblamed.extend(t for t in validate_blame(field, held) if t not in misblamed) + if offending or misblamed: + problems: list[str] = [] + if offending: + problems.append(f"tokens that may not be shown: {', '.join(offending)}") + if misblamed: + problems.append(f"budgets blamed although they held: {', '.join(misblamed)}") raise GenerationError( - f"response contained tokens that may not be shown: {', '.join(offending)}", + "response contained " + "; ".join(problems), tokens=offending, + misblamed=misblamed, ) return Insight( @@ -442,5 +555,6 @@ def _is_allowed(token: str, allowed: frozenset[str]) -> bool: "GenerationError", "build_prompt", "generate_insight", + "validate_blame", "validate_numbers", ] diff --git a/tests/unit/test_insight_generator.py b/tests/unit/test_insight_generator.py index 5bfcb10..47237f7 100644 --- a/tests/unit/test_insight_generator.py +++ b/tests/unit/test_insight_generator.py @@ -15,7 +15,7 @@ from evalshift_cli.analysis.policy import BlockingRegression, FailureCategoryCount from evalshift_cli.insights.facts import Facts, build_facts -from evalshift_cli.insights.generator import generate_insight, validate_numbers +from evalshift_cli.insights.generator import generate_insight, validate_blame, validate_numbers from evalshift_cli.models.client import ModelClient from tests.unit.insights_factories import ( SEVERITY_REASON, @@ -607,3 +607,168 @@ async def test_an_echoed_failed_budgets_key_is_rejected_and_retried( assert insight.verdict_summary == "The latency ceiling of +30% was exceeded." assert fake_client.call_count == 2 + + +# --------------------------------------------------------------------------- +# The blame validator +# --------------------------------------------------------------------------- + +_HELD = ["Trace-rule violations", "Cost increase"] + + +def test_blame_catches_the_shipped_sentence() -> None: + text = ( + "The migration failed because the target model exceeded the latency ceiling " + "of +30% and failed to meet the budget for Trace-rule violations." + ) + assert validate_blame(text, _HELD) == ["Trace-rule violations"] + + +def test_blame_catches_advice_to_address_a_held_budget() -> None: + text = ( + "Investigate the cause of the latency increase and address the Trace-rule " + "violations before re-evaluating." + ) + assert validate_blame(text, _HELD) == ["Trace-rule violations"] + + +def test_a_held_budget_may_be_called_within() -> None: + assert validate_blame("Trace-rule violations stayed within budget.", _HELD) == [] + + +def test_a_contrast_clause_separates_the_blame() -> None: + text = "Latency increase was exceeded while Trace-rule violations stayed within budget." + assert validate_blame(text, _HELD) == [] + + +def test_a_sentence_boundary_separates_the_blame() -> None: + text = "The latency ceiling was exceeded. Trace-rule violations and Cost increase held." + assert validate_blame(text, _HELD) == [] + + +def test_a_negated_blame_is_not_blame() -> None: + assert validate_blame("The Trace-rule violations budget was not breached.", _HELD) == [] + assert validate_blame("No breach of Cost increase was observed.", _HELD) == [] + + +def test_blame_matches_case_and_hyphen_variants() -> None: + assert validate_blame("the run failed on trace rule violations", _HELD) == [ + "Trace-rule violations" + ] + + +def test_a_budget_not_in_the_held_list_may_be_blamed() -> None: + assert validate_blame("Latency increase was breached.", _HELD) == [] + + +def test_every_misblamed_budget_is_returned_once_in_list_order() -> None: + text = ( + "Cost increase was exceeded and Trace-rule violations were violated; Cost increase failed." + ) + assert validate_blame(text, _HELD) == ["Trace-rule violations", "Cost increase"] + + +# --------------------------------------------------------------------------- +# Blame in a generation: retry with the labels named, then the template +# --------------------------------------------------------------------------- + +_SHIPPED_SUMMARY = ( + "The migration failed because the target model exceeded the latency ceiling of " + "+30% and failed to meet the budget for Trace-rule violations." +) + + +async def test_a_fail_that_blames_a_held_budget_is_rejected_and_retried( + fake_client: FakeModelClient, sample_run: dict[str, Any] +) -> None: + sample_run["decision"] = converter_decision() + fake_client.queue_responses( + _payload(verdict_summary=_SHIPPED_SUMMARY), + _payload(verdict_summary="The migration failed because the latency ceiling was exceeded."), + ) + insight = await _generate(build_facts(**sample_run), fake_client) + + assert insight.verdict_summary.endswith("latency ceiling was exceeded.") + assert fake_client.call_count == 2 + assert ( + "Your previous answer blamed budgets that held: Trace-rule violations." + in fake_client.prompts[1] + ) + + +async def test_a_misblaming_recommendation_is_rejected( + fake_client: FakeModelClient, sample_run: dict[str, Any] +) -> None: + sample_run["decision"] = converter_decision() + fake_client.queue_responses( + _payload(recommendation="Address the Trace-rule violations before re-evaluating."), + _payload(recommendation="Bring latency back under the ceiling, then re-run."), + ) + insight = await _generate(build_facts(**sample_run), fake_client) + + assert insight.recommendation == "Bring latency back under the ceiling, then re-run." + assert fake_client.call_count == 2 + + +async def test_two_misblaming_generations_fall_back_to_the_template( + fake_client: FakeModelClient, sample_run: dict[str, Any] +) -> None: + """The template already names the breached budget, so it is the safe floor.""" + sample_run["decision"] = converter_decision() + bad = _payload(recommendation="Address the Trace-rule violations before re-evaluating.") + fake_client.queue_responses(bad, bad) + insight = await _generate(build_facts(**sample_run), fake_client) + + assert insight.model == "none" + assert insight.recommendation.startswith( + "Do not migrate under the configured policy — the latency increase budget was breached." + ) + + +async def test_a_fail_may_mention_a_held_budget_without_blaming_it( + fake_client: FakeModelClient, sample_run: dict[str, Any] +) -> None: + sample_run["decision"] = converter_decision() + fake_client.queue_responses( + _payload( + verdict_summary=( + "The latency ceiling of +30% was exceeded; Trace-rule violations stayed " + "within budget." + ) + ) + ) + await _generate(build_facts(**sample_run), fake_client) + + assert fake_client.call_count == 1 + + +async def test_blame_is_not_checked_outside_a_fail( + fake_client: FakeModelClient, sample_run: dict[str, Any] +) -> None: + """The scan is scoped to a FAIL (Global Constraints): only a FAIL assigns a cause. + + The sentence is false on a pass, but it accuses no budget of failing a + run that did not fail; the scope is pinned here so widening it is a + deliberate change. + """ + fake_client.queue_responses( + _payload(verdict_summary="PASS although the Cost increase budget was exceeded.") + ) + await _generate(build_facts(**sample_run), fake_client) + + assert fake_client.call_count == 1 + + +async def test_an_invented_number_and_a_misblame_are_both_named_on_retry( + fake_client: FakeModelClient, sample_run: dict[str, Any] +) -> None: + sample_run["decision"] = converter_decision() + fake_client.queue_responses( + _payload(verdict_summary=_SHIPPED_SUMMARY, economics_summary="Cost rose +43%."), + _payload(), + ) + await _generate(build_facts(**sample_run), fake_client) + + retry = fake_client.prompts[1] + assert "+43%" in retry + assert "blamed budgets that held: Trace-rule violations" in retry From d0dfdf3c47c2255be6c7470f193a1711749ecc6e Mon Sep 17 00:00:00 2001 From: Lukas Babaliauskas Date: Mon, 12 Oct 2026 20:41:37 +0200 Subject: [PATCH 06/17] fix(insights): mask every budget label before scanning for blame validate_blame took held labels only and matched them by substring, so a breached slice label such as "'checkout' slice cost increase" accused its held overall row, and a breached label carrying a blame word ("Trace-rule violations") accused a held neighbour. Labels from both sides are now located longest first, non-overlapping, and masked before the scan; _build_insight passes the non-held labels as others. ', with', ', however,' and a dash now end a clause too, so a retry phrased the way the hint invites ('..., with Cost increase within policy') is accepted. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/evalshift_cli/insights/generator.py | 67 ++++++++++++++++---- tests/unit/test_insight_generator.py | 82 ++++++++++++++++++++++++- 2 files changed, 136 insertions(+), 13 deletions(-) diff --git a/src/evalshift_cli/insights/generator.py b/src/evalshift_cli/insights/generator.py index d785901..6a04006 100644 --- a/src/evalshift_cli/insights/generator.py +++ b/src/evalshift_cli/insights/generator.py @@ -93,9 +93,12 @@ #: exceeded while Trace-rule violations stayed within budget" blames only #: latency. A bare "and" is deliberately not one: "exceeded latency and failed #: to meet Trace-rule violations" is the shipped sentence, and both halves -#: accuse. +#: accuse; nor is a bare comma. ", with" is, because the retry hint invites +#: "…exceeded its ceiling, with Cost increase within policy", and so are a +#: parenthetical ", however," and a dash. _CLAUSE_RE = re.compile( - r"[.;:!?]\s+|\s(?:while|but|whereas|although|though|however)\s|,\s*(?:and|but)\s", + r"[.;:!?]\s+|\s(?:while|but|whereas|although|though|however)\s|,\s*(?:and|but)\s" + r"|,\s*with\s|,\s*however,?\s|\s*[\u2014\u2013]\s*", re.IGNORECASE, ) @@ -236,7 +239,7 @@ def validate_numbers(text: str, allowed: frozenset[str]) -> list[str]: return [token for token in NUMERIC_TOKEN_RE.findall(text) if not _is_allowed(token, allowed)] -def validate_blame(text: str, within: Sequence[str]) -> list[str]: +def validate_blame(text: str, within: Sequence[str], *, others: Sequence[str] = ()) -> list[str]: """Return the budgets in ``within`` that ``text`` blames for the verdict. A budget that held may be mentioned — "Trace-rule violations stayed within @@ -248,33 +251,70 @@ def validate_blame(text: str, within: Sequence[str]) -> list[str]: A safety net, not a parser: it catches the shipped phrasing and its close variants, and a false match costs one retry. Labels are matched case-insensitively with hyphens and runs of whitespace folded to one space. + Every label — held or not — is located longest first and masked before the + scan: "'checkout' slice cost increase" contains "cost increase", and + "Trace-rule violations" contains a blame word, and neither may accuse a + held budget by containment. Args: text: One prose field of a generation. within: Display labels of the budgets whose status is :data:`~evalshift_cli.insights.facts.BUDGET_WITHIN`. + others: Display labels of the gating budgets that did not hold. Never + reported, only located: a span one of them claims cannot also + match a shorter held label, and its words are not blame words. Returns: The wrongly blamed labels, each once, in ``within`` order. Empty means valid. """ - needles = {label: _normalise(label) for label in within} + held = set(within) + # Longest first, so a position a longer label covers cannot also match a + # shorter one it contains. ``sorted`` is stable: on a tie, held labels win. + needles = sorted( + ((label, _normalise(label)) for label in (*within, *others)), + key=lambda pair: -len(pair[1]), + ) blamed: set[str] = set() for clause in (_normalise(c) for c in _CLAUSE_RE.split(text)): - present = [label for label, needle in needles.items() if needle in clause] + spans, present = _locate_labels(clause, needles) + present &= held if not present: continue - # Every held label comes out before the scan, not just the one under - # test: "Trace-rule violations" would otherwise read as a violation - # of whichever budget shares its clause. - rest = clause - for needle in needles.values(): - rest = rest.replace(needle, " ") + # Every located label comes out before the scan, not just the one + # under test: "Trace-rule violations" would otherwise read as a + # violation of whichever budget shares its clause. + rest, cursor = "", 0 + for start, end in sorted(spans): + rest += clause[cursor:start] + " " + cursor = end + rest += clause[cursor:] if any(not _NEGATION_RE.search(rest[: m.start()]) for m in _BLAME_RE.finditer(rest)): blamed.update(present) return [label for label in within if label in blamed] +def _locate_labels( + clause: str, needles: list[tuple[str, str]] +) -> tuple[list[tuple[int, int]], set[str]]: + """Claim non-overlapping spans of ``clause`` for ``needles``, in the order given. + + Returns: + The claimed ``(start, end)`` spans, and the labels that claimed one. + """ + spans: list[tuple[int, int]] = [] + found: set[str] = set() + for label, needle in needles: + start = clause.find(needle) + while start != -1: + end = start + len(needle) + if all(end <= s or start >= e for s, e in spans): + spans.append((start, end)) + found.add(label) + start = clause.find(needle, start + 1) + return spans, found + + def _normalise(text: str) -> str: """Lower-case with hyphens and whitespace runs folded, for label matching.""" return re.sub(r"[-\s]+", " ", text.lower()) @@ -452,8 +492,11 @@ def _build_insight(text: str, *, facts: Facts, model: str) -> Insight: misblamed: list[str] = [] if facts.verdict == "fail": held = [label for label, _, status in facts.budget_statuses if status == BUDGET_WITHIN] + others = [label for label, _, status in facts.budget_statuses if status != BUDGET_WITHIN] for field in (prose["verdict_summary"], prose["recommendation"]): - misblamed.extend(t for t in validate_blame(field, held) if t not in misblamed) + misblamed.extend( + t for t in validate_blame(field, held, others=others) if t not in misblamed + ) if offending or misblamed: problems: list[str] = [] if offending: diff --git a/tests/unit/test_insight_generator.py b/tests/unit/test_insight_generator.py index 47237f7..371d64a 100644 --- a/tests/unit/test_insight_generator.py +++ b/tests/unit/test_insight_generator.py @@ -13,7 +13,12 @@ import pytest -from evalshift_cli.analysis.policy import BlockingRegression, FailureCategoryCount +from evalshift_cli.analysis.policy import ( + BlockingRegression, + BudgetResult, + FailureCategoryCount, + SliceDecision, +) from evalshift_cli.insights.facts import Facts, build_facts from evalshift_cli.insights.generator import generate_insight, validate_blame, validate_numbers from evalshift_cli.models.client import ModelClient @@ -668,6 +673,46 @@ def test_every_misblamed_budget_is_returned_once_in_list_order() -> None: assert validate_blame(text, _HELD) == ["Trace-rule violations", "Cost increase"] +def test_a_breached_slice_label_does_not_accuse_its_held_overall_row() -> None: + """The slice label contains the overall one; the longer match owns its span.""" + held = ["Cost increase"] + others = ["'checkout' slice cost increase"] + assert ( + validate_blame( + "The 'checkout' slice cost increase budget was breached.", held, others=others + ) + == [] + ) + assert ( + validate_blame( + "The run failed because the 'checkout' slice cost increase exceeded its +10% limit.", + held, + others=others, + ) + == [] + ) + + +def test_a_breached_label_with_a_blame_word_does_not_accuse_its_neighbour() -> None: + """ "violations" is part of a label, not an accusation, once the label is masked.""" + text = "Trace-rule violations rose to 3, with cost increase inside its limit." + assert validate_blame(text, ["Cost increase"], others=["Trace-rule violations"]) == [] + + +def test_the_retry_hints_own_phrasing_is_not_blame() -> None: + """The hint invites "within policy"; a reply phrased that way must pass.""" + text = ( + "The run failed because latency exceeded its ceiling, with Cost increase and " + "Trace-rule violations within policy." + ) + assert validate_blame(text, _HELD) == [] + + +def test_a_dash_or_however_separates_the_blame() -> None: + assert validate_blame("Latency breached its ceiling — Cost increase held.", _HELD) == [] + assert validate_blame("Latency was exceeded, however, Cost increase held.", _HELD) == [] + + # --------------------------------------------------------------------------- # Blame in a generation: retry with the labels named, then the template # --------------------------------------------------------------------------- @@ -772,3 +817,38 @@ async def test_an_invented_number_and_a_misblame_are_both_named_on_retry( retry = fake_client.prompts[1] assert "+43%" in retry assert "blamed budgets that held: Trace-rule violations" in retry + + +async def test_a_slice_budget_fail_may_name_the_breached_slice_budget( + fake_client: FakeModelClient, sample_run: dict[str, Any] +) -> None: + """The overall "Cost increase" held; its 'checkout' slice row broke. + + The slice label contains the held overall one, so a substring match would + accuse the overall row in every run a slice budget failed. + """ + base = decision(verdict="fail") + slice_row = BudgetResult( + name="max_cost_increase", observed=1.02, allowed=0.10, passed=False, scope="checkout" + ) + sample_run["decision"] = replace( + base, + slices={ + "checkout": SliceDecision( + name="checkout", verdict="fail", metrics=base.overall, budget_results=[slice_row] + ) + }, + ) + facts = build_facts(**sample_run) + assert facts.failed_budgets == ["'checkout' slice cost increase"] + fake_client.queue_responses( + _payload( + verdict_summary=( + "The run failed because the 'checkout' slice cost increase exceeded its +10% limit." + ), + recommendation="Bring the 'checkout' slice cost increase under its limit.", + ) + ) + await _generate(facts, fake_client) + + assert fake_client.call_count == 1 From fe96d2d2c7d9c5100324878914eeb2e2605cffe0 Mon Sep 17 00:00:00 2001 From: Lukas Babaliauskas Date: Mon, 12 Oct 2026 20:45:56 +0200 Subject: [PATCH 07/17] fix(insights): fold quotes and pin label masking in the blame scan A slice label reaches the model as "'checkout' slice cost increase" and may come back bare or curly-quoted; the held overall "cost increase" inside it then claimed the span. _normalise now drops straight and curly quotes from labels and prose alike. The neighbour test's sentence split at ', with' and passed without the masking; it now keeps both labels in one clause and asserts the accusation that appears when others is omitted. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/evalshift_cli/insights/generator.py | 13 +++++++++++-- tests/unit/test_insight_generator.py | 20 ++++++++++++++++++-- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/src/evalshift_cli/insights/generator.py b/src/evalshift_cli/insights/generator.py index 6a04006..b726946 100644 --- a/src/evalshift_cli/insights/generator.py +++ b/src/evalshift_cli/insights/generator.py @@ -102,6 +102,10 @@ re.IGNORECASE, ) +#: Straight and curly quotes, folded out of labels and prose alike before the +#: blame scan matches one against the other. +_QUOTE_RE = re.compile(r"[\"'\u2018\u2019\u201c\u201d]") + _INSTRUCTION = """\ You are writing the explanation section of a model-migration report. The reader is an engineer deciding whether to switch their production model to the target. @@ -316,8 +320,13 @@ def _locate_labels( def _normalise(text: str) -> str: - """Lower-case with hyphens and whitespace runs folded, for label matching.""" - return re.sub(r"[-\s]+", " ", text.lower()) + """Lower-case, quotes dropped, hyphens and whitespace runs folded, for label matching. + + Quotes go because a slice label reaches the model as "'checkout' slice + cost increase" and comes back bare or curly-quoted; left in, the held + "cost increase" inside it would claim the span. + """ + return re.sub(r"[-\s]+", " ", _QUOTE_RE.sub("", text.lower())) async def generate_insight(facts: Facts, *, model: str, client: ModelClient) -> Insight: diff --git a/tests/unit/test_insight_generator.py b/tests/unit/test_insight_generator.py index 371d64a..a2f451e 100644 --- a/tests/unit/test_insight_generator.py +++ b/tests/unit/test_insight_generator.py @@ -694,9 +694,25 @@ def test_a_breached_slice_label_does_not_accuse_its_held_overall_row() -> None: def test_a_breached_label_with_a_blame_word_does_not_accuse_its_neighbour() -> None: - """ "violations" is part of a label, not an accusation, once the label is masked.""" - text = "Trace-rule violations rose to 3, with cost increase inside its limit." + """ "violations" is part of a label, not an accusation, once the label is masked. + + The bare "and" keeps both labels in one clause, so the masking alone saves + it — without ``others`` the same sentence accuses "Cost increase". + """ + text = "Trace-rule violations rose to 3 and cost increase stayed inside its limit." assert validate_blame(text, ["Cost increase"], others=["Trace-rule violations"]) == [] + assert validate_blame(text, ["Cost increase"]) == ["Cost increase"] + + +def test_a_slice_label_matches_whatever_quotes_the_model_used() -> None: + """The model may drop the slice's quotes or swap them; the slice still owns its span.""" + others = ["'checkout' slice cost increase"] + for text in ( + "The checkout slice cost increase budget was breached.", + "The \u2018checkout\u2019 slice cost increase budget was breached.", + 'The "checkout" slice cost increase budget was breached.', + ): + assert validate_blame(text, ["Cost increase"], others=others) == [], text def test_the_retry_hints_own_phrasing_is_not_blame() -> None: From e2df6476fe55b6f3465fcc74145edad39fbbc5b2 Mon Sep 17 00:00:00 2001 From: Lukas Babaliauskas Date: Mon, 12 Oct 2026 20:50:08 +0200 Subject: [PATCH 08/17] docs(insights): a FAIL's cause is a fact the narrative is given Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 10 ++++++++++ DOCS.md | 2 ++ docs/faq.md | 5 +++++ llms-full.txt | 28 ++++++++++++++++++++++++++++ 4 files changed, 45 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 77cc6a2..3235b61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 under its verdict) warns that the evaluator checked nothing because no example offered tools. Coverage is written by `evaluate`, so an older run needs `evaluate` re-run to drop the stale coverage; `analyze` alone keeps it, and the `conditional_pass` with it. +- The run-insights narrative no longer guesses why a run failed. The prompt listed each + budget's limit but not its result, so a FAIL on the latency ceiling alone was explained + as "failed to meet the budget for Trace-rule violations" — a budget that held. Every + budget, slice rows included, now enters the prompt with its status, the breached ones + are listed as `failed_budgets`, and the decision's `reason` travels with them in plain + words, without `evalshift.yaml` keys. Only a breached budget may be named as the cause, + and on a FAIL a summary or recommendation that blames a budget that held is rejected + and retried, then replaced by the templated prose. Narratives are cached on the run's + config hash and model, not the CLI version: delete a run's `insights.json` and re-run + `report` to rewrite an existing one. ## [1.4.1] - 2026-10-11 diff --git a/DOCS.md b/DOCS.md index ba95693..c8bfd7a 100644 --- a/DOCS.md +++ b/DOCS.md @@ -781,6 +781,8 @@ CI wiring (on `analyze` and `compare`): **A gate that measured nothing is never counted as a gate that passed.** A budget handed an empty sample — a tool-divergence ceiling on a run that scored no divergence row, a cost ceiling on a run with no priced call — is within its limit by arithmetic: `0/0` is below every ceiling. Counted naively that renders "7 of 7 budgets passed", and a narrative restates it as "all hard constraints are met" while the report body two sections above names the blocking evaluators that scored nothing. So `budgets_passed` counts only budgets that were *measured*, the blind ones are named on their own `unmeasured_budgets` / `unmeasured_evaluators` facts with a digit-free `coverage_basis` saying why, and the instruction forbids claiming a clean sweep, a met constraint or a safe migration while that basis is present. The blind-gate set is the same one `migration_decision.json`'s own `recommendations` name, computed by one shared function, so the two surfaces cannot disagree. This is distinct from the rates rule above and not covered by it: that fires when the *whole run* measured nothing, this fires when only *some* gate did. +**A FAIL's cause is read off the facts, not inferred.** Every gating budget, overall and slice rows alike, enters the prompt with its status beside its limit — `budget "