From 2965847f782dce757aa0fd0b5fd9c82fdd8c0cf6 Mon Sep 17 00:00:00 2001 From: lostmartian Date: Tue, 25 Aug 2026 03:37:24 +0530 Subject: [PATCH] =?UTF-8?q?feat(governance):=20threshold-change=20flagging?= =?UTF-8?q?=20=E2=80=94=20the=20Goodhart=20guard=20(G6/G7)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - governance.py: diff_gate_thresholds() compares effective gate values between baseline config and current config (max_divergence, max_loops, max_cost_delta, max_recovery_ratio) - CLI: --baseline-config PATH on diff; PR format renders [!WARNING] block above the gate table listing every changed threshold; --explain prints the same summary; missing file exits 2 - agentdiff-check action: new 'baseline-config' input, CI one-liner: git show origin/main:agentdiff.toml > /tmp/baseline-agentdiff.toml - Docs: 'Gate governance (Goodhart guard)' section in Configuration guide, README flag reference - 14 new tests (333 total) --- .github/actions/agentdiff-check/action.yml | 7 + README.md | 1 + src/agentdiff/cli.py | 20 ++ src/agentdiff/governance.py | 64 +++++++ src/agentdiff/reporters/pr.py | 31 ++- tests/test_governance.py | 207 +++++++++++++++++++++ website/docs/03-Guides/05-Configuration.md | 25 +++ 7 files changed, 350 insertions(+), 5 deletions(-) create mode 100644 src/agentdiff/governance.py create mode 100644 tests/test_governance.py diff --git a/.github/actions/agentdiff-check/action.yml b/.github/actions/agentdiff-check/action.yml index 011ea8bc..42c683b4 100644 --- a/.github/actions/agentdiff-check/action.yml +++ b/.github/actions/agentdiff-check/action.yml @@ -39,6 +39,10 @@ inputs: description: "GitHub pull request number to post the PR-ready report comment to." required: false default: "" + baseline-config: + description: "Path to the agentdiff.toml the BASELINE was recorded with. When gate values differ from this run's config, the PR report flags the change (Goodhart guard)." + required: false + default: "" github-token: description: "GitHub token used to post the PR comment (e.g. secrets.GITHUB_TOKEN). Required when pr is set." required: false @@ -71,6 +75,9 @@ runs: if [[ -n "${{ inputs.pr }}" ]]; then args+=(--pr "${{ inputs.pr }}") fi + if [[ -n "${{ inputs.baseline-config }}" ]]; then + args+=(--baseline-config "${{ inputs.baseline-config }}") + fi agentdiff "${{ inputs.baseline }}" "${{ inputs.candidate }}" \ --baseline "${{ inputs.baseline }}" \ --fail-on-regression --format markdown "${args[@]}" \ No newline at end of file diff --git a/README.md b/README.md index 123bb0f3..82452c33 100644 --- a/README.md +++ b/README.md @@ -70,6 +70,7 @@ Options: - `--max-cost-delta`: Maximum cost increase percentage allowed. - `--baseline, -b PATH`: Compare against a persistent baseline trace file (see [Baseline workflow](#baseline-workflow)). - `--update-baseline`: Overwrite the persistent baseline with the candidate after a clean diff. +- `--baseline-config PATH`: The `agentdiff.toml` the **baseline** was recorded with. When gate values differ from this run's config, the report flags the change (Goodhart guard — see [Gate governance](https://agentdiff.lostmartian.in/docs/configuration#gate-governance-goodhart-guard)). - `--config PATH`: Load defaults from an `agentdiff.toml` (auto-discovered if not given). #### Config-as-code (`agentdiff.toml`) diff --git a/src/agentdiff/cli.py b/src/agentdiff/cli.py index e175aa13..d8b19c8e 100644 --- a/src/agentdiff/cli.py +++ b/src/agentdiff/cli.py @@ -12,6 +12,7 @@ from agentdiff.engine.comparator import compare from agentdiff.engine.explanations import format_explanations, locate_culprit from agentdiff.engine.tree import render_tree +from agentdiff.governance import diff_gate_thresholds from agentdiff.loader import load_trace from agentdiff.models.step import StepStatus from agentdiff.recorder import record_run, save_trace @@ -180,9 +181,22 @@ def diff( max_drift: float = typer.Option( 0.05, help="Max TDI a clean run may have for staged auto-rotation." ), + baseline_config: str | None = typer.Option( + None, + "--baseline-config", + help="Path to the agentdiff.toml the BASELINE was recorded with. When gate values differ from this run's config, the report flags the change (Goodhart guard).", + ), ): """Compares baseline and candidate agent trajectories.""" cfg = load_config(config) + threshold_changes: list = [] + if baseline_config: + try: + baseline_cfg = load_config(baseline_config) + except FileNotFoundError as e: + typer.echo(f"Baseline config not found: {e}", err=True) + sys.exit(2) + threshold_changes = diff_gate_thresholds(baseline_cfg, cfg) values = _resolve_cli( cfg, adapter=adapter, @@ -286,6 +300,7 @@ def diff( max_loops=max_loops, max_cost_delta=max_cost_delta, max_recovery_ratio=max_recovery_ratio, + threshold_changes=threshold_changes, ) if not output_file: typer.echo(output_content) @@ -299,6 +314,10 @@ def diff( culprit = locate_culprit(report) if culprit: typer.echo("\n" + culprit.render()) + if threshold_changes: + typer.echo("\nGate thresholds changed vs baseline config:") + for change in threshold_changes: + typer.echo(f" ! {change.render()}") # Print the collapsed divergence tree when requested if tree: @@ -312,6 +331,7 @@ def diff( max_loops=max_loops, max_cost_delta=max_cost_delta, max_recovery_ratio=max_recovery_ratio, + threshold_changes=threshold_changes, ) comment = post_pr_comment(body, pr) url = comment.get("html_url") diff --git a/src/agentdiff/governance.py b/src/agentdiff/governance.py new file mode 100644 index 00000000..8fee372d --- /dev/null +++ b/src/agentdiff/governance.py @@ -0,0 +1,64 @@ +"""Gate governance — detect when the gate itself changed (G6/G7). + +Goodhart guard: a threshold tuned until CI goes green stops being a control. +These helpers make threshold changes visible *next to the diff they let +through*, so loosening the gate is as reviewable as the code it guards. +""" + +from __future__ import annotations + +from dataclasses import dataclass + +from agentdiff.config import AgentDiffConfig + +# The gate knobs that decide pass/fail in the CLI diff path, in display order. +_GATED_KEYS: tuple[str, ...] = ( + "max_divergence", + "max_loops", + "max_cost_delta", + "max_recovery_ratio", +) + +# Defaults used when neither config nor flag provides a value (mirrors cli.py). +_EFFECTIVE_DEFAULTS: dict[str, float | int | None] = { + "max_divergence": 0.3, + "max_loops": 0, + "max_cost_delta": 10.0, + "max_recovery_ratio": None, +} + + +@dataclass(frozen=True) +class ThresholdChange: + """One gate knob that differs between the baseline config and this run.""" + + gate: str + old: float | int | None + new: float | int | None + + def render(self) -> str: + return f"{self.gate}: `{self.old}` → `{self.new}`" + + +def effective_gates(cfg: AgentDiffConfig) -> dict[str, float | int | None]: + """Resolves the effective gate values from a config (config or defaults).""" + resolved: dict[str, float | int | None] = {} + for key in _GATED_KEYS: + value = getattr(cfg.cli, key, None) + if value is None: + value = _EFFECTIVE_DEFAULTS[key] + resolved[key] = value + return resolved + + +def diff_gate_thresholds( + baseline_cfg: AgentDiffConfig, candidate_cfg: AgentDiffConfig +) -> list[ThresholdChange]: + """Returns gate knobs that differ between two configs, display-ordered.""" + old_gates = effective_gates(baseline_cfg) + new_gates = effective_gates(candidate_cfg) + return [ + ThresholdChange(gate=key, old=old_gates[key], new=new_gates[key]) + for key in _GATED_KEYS + if old_gates[key] != new_gates[key] + ] diff --git a/src/agentdiff/reporters/pr.py b/src/agentdiff/reporters/pr.py index 14e5707c..cd8b18e7 100644 --- a/src/agentdiff/reporters/pr.py +++ b/src/agentdiff/reporters/pr.py @@ -2,6 +2,7 @@ from agentdiff.engine.explanations import locate_culprit from agentdiff.engine.tree import render_tree +from agentdiff.governance import ThresholdChange from agentdiff.models.report import DiffReport DEFAULT_MAX_DIVERGENCE = 0.3 @@ -14,12 +15,17 @@ def generate_pr_markdown( max_loops: int = 0, max_cost_delta: float = DEFAULT_MAX_COST_DELTA, max_recovery_ratio: float | None = None, + threshold_changes: list[ThresholdChange] | None = None, ) -> str: """Renders a compact, PR-ready markdown comment. Summary status + gate thresholds, the collapsed divergence tree, and the root-cause step — everything a reviewer needs without the full diff. The Recovery Step Ratio row only appears when a threshold is provided. + + When ``threshold_changes`` is non-empty (G6), a warning block renders + above the gate table: the gate itself moved in this PR, so the diff + above was judged against looser/tighter rules than the baseline had. """ status = "⛔ **FAILED**" if not report.passed else "✅ **PASSED**" @@ -28,12 +34,27 @@ def generate_pr_markdown( "", f"**Status:** {status}", "", - "| Gate | Value | Threshold |", - "| :--- | :--- | :--- |", - f"| TDI | `{report.trajectory_divergence_index:.4f}` | ≤ `{max_divergence}` |", - f"| Loops | `{len(report.loops_detected)}` | ≤ `{max_loops}` |", - f"| Cost delta | `{report.cost_delta_percentage:+.2f}%` | ≤ `{max_cost_delta}%` |", ] + + if threshold_changes: + lines.append("> [!WARNING]") + lines.append( + "> **Gate thresholds changed in this PR** — the diff below was judged against this PR's rules, not the baseline's." + ) + lines.append(">") + for change in threshold_changes: + lines.append(f"> - {change.render()}") + lines.append("") + + lines.extend( + [ + "| Gate | Value | Threshold |", + "| :--- | :--- | :--- |", + f"| TDI | `{report.trajectory_divergence_index:.4f}` | ≤ `{max_divergence}` |", + f"| Loops | `{len(report.loops_detected)}` | ≤ `{max_loops}` |", + f"| Cost delta | `{report.cost_delta_percentage:+.2f}%` | ≤ `{max_cost_delta}%` |", + ] + ) if max_recovery_ratio is not None: lines.append( f"| Recovery Step Ratio | `{report.recovery_step_ratio:.2f} " diff --git a/tests/test_governance.py b/tests/test_governance.py new file mode 100644 index 00000000..91a5cb39 --- /dev/null +++ b/tests/test_governance.py @@ -0,0 +1,207 @@ +"""Tests for gate governance (G6/G7): threshold-change detection + PR warning.""" + +import json + +from typer.testing import CliRunner + +from agentdiff.config import AgentDiffConfig, load_config +from agentdiff.governance import ( + ThresholdChange, + diff_gate_thresholds, + effective_gates, +) +from agentdiff.recorder import record_run +from agentdiff.reporters.pr import generate_pr_markdown + +runner = CliRunner() + + +class TestEffectiveGates: + def test_defaults_when_config_empty(self): + gates = effective_gates(AgentDiffConfig()) + assert gates == { + "max_divergence": 0.3, + "max_loops": 0, + "max_cost_delta": 10.0, + "max_recovery_ratio": None, + } + + def test_config_values_win(self, tmp_path): + cfg_file = tmp_path / "agentdiff.toml" + cfg_file.write_text("[cli]\nmax_divergence = 0.5\nmax_loops = 2\n") + gates = effective_gates(load_config(cfg_file)) + assert gates["max_divergence"] == 0.5 + assert gates["max_loops"] == 2 + assert gates["max_cost_delta"] == 10.0 # default + + +class TestDiffGateThresholds: + def test_no_changes_returns_empty(self): + changes = diff_gate_thresholds(AgentDiffConfig(), AgentDiffConfig()) + assert changes == [] + + def test_detects_loosened_threshold(self, tmp_path): + old = tmp_path / "old.toml" + old.write_text("[cli]\nmax_divergence = 0.25\n") + changes = diff_gate_thresholds(load_config(old), AgentDiffConfig()) + assert changes == [ThresholdChange("max_divergence", 0.25, 0.3)] + + def test_detects_tightened_threshold(self, tmp_path): + new = tmp_path / "new.toml" + new.write_text("[cli]\nmax_divergence = 0.4\nmax_cost_delta = 20.0\n") + changes = diff_gate_thresholds(AgentDiffConfig(), load_config(new)) + by_gate = {c.gate: c for c in changes} + assert by_gate["max_divergence"].old == 0.3 + assert by_gate["max_divergence"].new == 0.4 + assert by_gate["max_cost_delta"].old == 10.0 + assert by_gate["max_cost_delta"].new == 20.0 + + def test_render_format(self): + change = ThresholdChange("max_divergence", 0.25, 0.40) + assert change.render() == "max_divergence: `0.25` → `0.4`" + + def test_recovery_ratio_none_to_value_detected(self, tmp_path): + new = tmp_path / "new.toml" + new.write_text("[cli]\nmax_recovery_ratio = 1.5\n") + changes = diff_gate_thresholds(AgentDiffConfig(), load_config(new)) + assert ThresholdChange("max_recovery_ratio", None, 1.5) in changes + + +class TestPRMarkdownWarning: + def _report(self): + return record_run("json:loads", task_input={"s": "{}"}).model_dump() + + def test_no_warning_when_no_changes(self, tmp_path): + base = tmp_path / "base.json" + cand = tmp_path / "cand.json" + base.write_text( + json.dumps(record_run("json:loads", task_input={"s": "{}"}).model_dump()) + ) + cand.write_text( + json.dumps(record_run("json:loads", task_input={"s": "{}"}).model_dump()) + ) + from agentdiff.engine.comparator import compare + from agentdiff.loader import load_trace + + report = compare(load_trace(str(base)), load_trace(str(cand))) + md = generate_pr_markdown(report) + assert "Gate thresholds changed" not in md + + def test_warning_block_renders_changes(self, tmp_path): + base = tmp_path / "base.json" + cand = tmp_path / "cand.json" + base.write_text( + json.dumps(record_run("json:loads", task_input={"s": "{}"}).model_dump()) + ) + cand.write_text( + json.dumps(record_run("json:loads", task_input={"s": "{}"}).model_dump()) + ) + from agentdiff.engine.comparator import compare + from agentdiff.loader import load_trace + + report = compare(load_trace(str(base)), load_trace(str(cand))) + md = generate_pr_markdown( + report, + threshold_changes=[ThresholdChange("max_divergence", 0.25, 0.4)], + ) + assert "[!WARNING]" in md + assert "Gate thresholds changed in this PR" in md + assert "max_divergence: `0.25` → `0.4`" in md + # Warning appears above the gate table + assert md.index("[!WARNING]") < md.index("| Gate |") + + def test_warning_appears_even_when_passing(self, tmp_path): + """A loosened gate that goes green must still be flagged — that's the point.""" + base = tmp_path / "base.json" + cand = tmp_path / "cand.json" + base.write_text( + json.dumps(record_run("json:loads", task_input={"s": "{}"}).model_dump()) + ) + cand.write_text( + json.dumps(record_run("json:loads", task_input={"s": "{}"}).model_dump()) + ) + from agentdiff.engine.comparator import compare + from agentdiff.loader import load_trace + + report = compare(load_trace(str(base)), load_trace(str(cand))) + md = generate_pr_markdown( + report, + threshold_changes=[ThresholdChange("max_divergence", 0.25, 0.6)], + ) + assert "✅ **PASSED**" in md + assert "Gate thresholds changed in this PR" in md + + +class TestCLIBaselineConfig: + def _traces(self, tmp_path): + base = tmp_path / "base.json" + cand = tmp_path / "cand.json" + base.write_text( + json.dumps(record_run("json:loads", task_input={"s": "{}"}).model_dump()) + ) + cand.write_text( + json.dumps(record_run("json:loads", task_input={"s": "{}"}).model_dump()) + ) + return base, cand + + def test_pr_format_includes_warning_with_baseline_config(self, tmp_path): + base, cand = self._traces(tmp_path) + old_cfg = tmp_path / "old.toml" + old_cfg.write_text("[cli]\nmax_divergence = 0.2\n") + result = runner.invoke( + __import__("agentdiff.cli", fromlist=["app"]).app, + [ + "diff", + str(base), + str(cand), + "--format", + "pr", + "--baseline-config", + str(old_cfg), + ], + ) + assert result.exit_code == 0, result.output + assert "Gate thresholds changed in this PR" in result.output + assert "max_divergence: `0.2` → `0.3`" in result.output + + def test_pr_format_no_warning_without_flag(self, tmp_path): + base, cand = self._traces(tmp_path) + result = runner.invoke( + __import__("agentdiff.cli", fromlist=["app"]).app, + ["diff", str(base), str(cand), "--format", "pr"], + ) + assert result.exit_code == 0, result.output + assert "Gate thresholds changed" not in result.output + + def test_explain_prints_threshold_changes(self, tmp_path): + base, cand = self._traces(tmp_path) + old_cfg = tmp_path / "old.toml" + old_cfg.write_text("[cli]\nmax_cost_delta = 5.0\n") + result = runner.invoke( + __import__("agentdiff.cli", fromlist=["app"]).app, + [ + "diff", + str(base), + str(cand), + "--explain", + "--baseline-config", + str(old_cfg), + ], + ) + assert result.exit_code == 0, result.output + assert "Gate thresholds changed vs baseline config:" in result.output + assert "max_cost_delta: `5.0` → `10.0`" in result.output + + def test_missing_baseline_config_file_exits_2(self, tmp_path): + base, cand = self._traces(tmp_path) + result = runner.invoke( + __import__("agentdiff.cli", fromlist=["app"]).app, + [ + "diff", + str(base), + str(cand), + "--baseline-config", + str(tmp_path / "nope.toml"), + ], + ) + assert result.exit_code == 2 diff --git a/website/docs/03-Guides/05-Configuration.md b/website/docs/03-Guides/05-Configuration.md index af6794df..cb68dcfa 100644 --- a/website/docs/03-Guides/05-Configuration.md +++ b/website/docs/03-Guides/05-Configuration.md @@ -41,6 +41,31 @@ overriding them for a one-off run: agentdiff baseline.json candidate.json --max-divergence 0.5 ``` +## Gate governance (Goodhart guard) + +A threshold tuned until CI goes green stops being a control. Pass +`--baseline-config` to compare the gate values the *baseline* was recorded +with against the ones this run uses — any change is flagged right next to +the diff it let through: + +```bash +# In CI: fetch the config the baseline was recorded against, then diff +git show origin/main:agentdiff.toml > /tmp/baseline-agentdiff.toml +agentdiff baseline.json candidate.json --baseline-config /tmp/baseline-agentdiff.toml --format pr +``` + +The PR comment renders a warning block above the gate table: + +```text +> [!WARNING] +> Gate thresholds changed in this PR — the diff below was judged +> against this PR's rules, not the baseline's. +> - max_divergence: `0.25` → `0.4` +``` + +The same summary prints with `--explain`. Loosening a gate is now as +visible in review as the code it guards. + ## From the SDK You can load config programmatically with `load_config()`, which returns an