diff --git a/ROADMAP.md b/ROADMAP.md index 2defe16..8d3e787 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -166,14 +166,26 @@ Live validation on the LB stays. This runs before it, not instead of it. gate is skipped and the loop behaves exactly as before — pinned by a test that fails if anything is replayed without a sample. An unreadable sample logs a warning and refines without the gate. -- [ ] **G4** Published model scorecard. (M, P1) +- [x] **G4** Published model scorecard. (M, P1) — **DONE:** `vpcopilot bench --all-configs + [--skip ]` sweeps every `config/agents*.yaml` and writes `benchmarks/RESULTS.md`. First real + run (Nimbus fixture, 2026-07-27, dgx skipped — the local box was unreachable): claude recall 1.00 + / triage 0.89, openai 0.89 / 1.00, gemini 0.67 / 1.00, precision 1.00 and zero noise on all three. `D3` proved a config-only swap on `gpt-4o`. Turn that into a committed table across four providers including a local Ollama model, regenerated by command. - Acceptance: `vpcopilot bench --all-configs` writes `benchmarks/RESULTS.md` with one row per `config/agents*.yaml`, scoring discovery, verify precision and recall, triage accuracy, bonus finds, noise, and wall time; re-running with the same seed and model reproduces the score; `MODELS.md` links the table rather than restating numbers. - - **BUILT 2026-07-27, not yet run.** `scorecard.py` + `vpcopilot bench --all-configs` + - **The run surfaced a column the acceptance did not ask for, and the table needed.** Gemini + scored precision 1.00 and triage 1.00 while emitting `{}` as the spec for **3 of its 6** + policies — structured output validated, the artifact was useless. Recall, precision and triage + all say the routing was right; none of them notice. `policies (unusable)` counts what + `lint_generated_spec` rejects, reusing machinery the pipeline already runs. Without it the + table would have flattered a model that routed correctly and then emitted nothing. + - **A second latent bug it surfaced:** a re-scan into an existing out dir leaves the previous + run's artifacts in `policies/` — 32 files for 9 generated policies — so the count is taken from + the run's own `policies.json` index rather than the directory. + - **BUILT 2026-07-27.** `scorecard.py` + `vpcopilot bench --all-configs` sweep every `config/agents*.yaml` into `out-` and write `benchmarks/RESULTS.md`; a config that fails gets a row carrying its error rather than vanishing from the table (one dead provider must not cost the other three runs). `bench.py` now scores **verify precision** — the diff --git a/benchmarks/RESULTS.md b/benchmarks/RESULTS.md new file mode 100644 index 0000000..ae79e6b --- /dev/null +++ b/benchmarks/RESULTS.md @@ -0,0 +1,31 @@ +# Model scorecard + +Same code, same prompts, same answer key — only `config/agents.yaml` changed. Target: **Nimbus vuln-lab (9 labeled vulns)** · key: `bench/answer_key.yaml` · recorded 2026-07-27. + +Regenerate with: + +```sh +vpcopilot bench --all-configs --key bench/answer_key.yaml +``` + +| config | model | discovery recall | verify precision | triage accuracy | bonus finds | noise | policies (unusable) | dupes dropped | wall time | +|---|---|---|---|---|---|---|---|---|---| +| `claude` | `anthropic/claude-opus-4-8` | 9/9 = 1.00 | 1.00 (10/10) | 8/9 = 0.89 | 1/6 | 0 | 9 (0) | 0 | 195s | +| `gemini` | `gemini/gemini-3.1-pro-preview` | 6/9 = 0.67 | 1.00 (6/6) | 6/6 = 1.00 | 0/6 | 0 | 6 (**3**) | 0 | 394s | +| `openai` | `openai/gpt-4.1` | 8/9 = 0.89 | 1.00 (8/8) | 8/8 = 1.00 | 0/6 | 0 | 7 (0) | 0 | 53s | + +## Reading this table + +- **discovery recall** — of the vulns the answer key labels, how many were found. +- **verify precision** — of everything the run reported, how much was real. Recall and precision move independently: a run can find every labelled vuln and still bury them in false positives. +- **triage accuracy** — of the vulns found, how many were routed to an acceptable control. +- **bonus finds** — real vulns beyond the core key, credited rather than counted as noise. +- **dupes dropped** — duplicate findings for one vuln, collapsed so one vuln yields one band-aid and one code-fix PR. +- **policies (unusable)** — band-aids generated, and how many the deterministic linter rejects before any live round-trip: an empty spec, no DENY rule, or an OpenAPI fragment missing its envelope. Recall, precision and triage can all be perfect while the artifact is unusable, so this column exists to stop the table flattering a model that routed correctly and then emitted nothing. + +## What this table is not + +- **Not reproducible run to run.** Two of these providers accept no seed at all and none guarantee determinism, so scores vary between runs. Treat a small difference as noise; re-run before reading anything into one. The run date above is part of the result. +- **Not a cost comparison.** Token cost is not measured — nothing in the harness counts tokens, and a cost column was judged not worth threading through every agent call. +- **Not a general model ranking.** It is one target, one answer key, one prompt set. It says which model drives *this* pipeline well. +- **Not the full config set.** `dgx` was not run and is absent from the table — unreachable from the machine that produced it, which is a different thing from a model that scored badly. diff --git a/src/vpcopilot/bench.py b/src/vpcopilot/bench.py index 31aae07..7cb5969 100644 --- a/src/vpcopilot/bench.py +++ b/src/vpcopilot/bench.py @@ -30,6 +30,45 @@ } +def _policy_quality(out: Path) -> tuple[int, int]: + """(generated, lint-flagged) over the policies a run actually emitted. + + Beyond the stated G4 acceptance, and added because the first real four-way run proved the table + would otherwise mislead: one model scored precision 1.00 and triage 1.00 while emitting `{}` as + the spec for three of its policies. Recall, precision and triage all say the routing was right; + none of them notice that the artifact is unusable. This reuses `lint_generated_spec`, which the + pipeline already runs — no new judgement, just a count that reaches the scorecard. + + Counted from THIS run's `policies.json` index, not from the files on disk: a re-scan into an + existing out dir leaves the previous run's artifacts in `policies/` (32 files for 9 generated + policies, on the run that surfaced this), so globbing the directory would score a model against + someone else's leftovers.""" + from .apply import artifact_spec, lint_generated_spec + idx = out / "policies.json" + if not idx.exists(): + return 0, 0 + try: + entries = json.loads(idx.read_text()) + except json.JSONDecodeError: + return 0, 0 + total = flagged = 0 + for a in entries: + control, name = a.get("control", ""), a.get("policy_name", "") + f = out / "policies" / f"{control}.{name}.json" + total += 1 + if not f.exists(): + flagged += 1 + continue + try: + spec = artifact_spec(json.loads(f.read_text())) + except json.JSONDecodeError: + flagged += 1 + continue + if lint_generated_spec(control, spec, None): + flagged += 1 + return total, flagged + + def _precision(*, used: int, verified: int) -> float: """Verify precision — the false-positive filter rate the old `BACKLOG.md` item asked for. @@ -118,6 +157,7 @@ def _triage_ok(exp, f) -> bool: metrics = json.loads(mp.read_text()) except json.JSONDecodeError: metrics = {} + pol_total, pol_flagged = _policy_quality(out) score = { "expected": n, "found": found, @@ -130,6 +170,8 @@ def _triage_ok(exp, f) -> bool: "verified": len(verified), "verify_precision": _precision(used=len(used), verified=len(verified)), "duplicates_dropped": (metrics.get("discovery") or {}).get("duplicates_dropped", 0), + "policies": pol_total, + "policies_flagged": pol_flagged, "wall_time_s": (metrics.get("timing_s") or {}).get("total", 0.0), } return {"rows": rows, "score": score, "noise": noise} diff --git a/src/vpcopilot/cli.py b/src/vpcopilot/cli.py index a5d67cb..7b1a0a9 100644 --- a/src/vpcopilot/cli.py +++ b/src/vpcopilot/cli.py @@ -68,14 +68,16 @@ def bench( all_configs: bool = typer.Option(False, "--all-configs", help="score EVERY config/agents*.yaml and write benchmarks/RESULTS.md"), target: str = typer.Option(None, "--target", help="label for the scorecard (default: the repo path)"), + skip: list[str] = typer.Option(None, "--skip", help="config tag to skip, repeatable (e.g. --skip dgx)"), min_confidence: float = typer.Option(0.5, "--min-confidence", help="drop verified findings below this confidence"), concurrency: int = typer.Option(8, "--concurrency", help="parallel workers for discover/verify"), ): """Run the scan and SCORE it against the answer key (discovery, triage, cure).""" if all_configs: from .scorecard import run_all_configs, write_results + skipped = tuple(skip or ()) res = run_all_configs(repo, key=key, min_confidence=min_confidence, concurrency=concurrency, - rescore=rescore, log=lambda m: rprint(f"[dim]{m}[/dim]")) + rescore=rescore, skip=skipped, log=lambda m: rprint(f"[dim]{m}[/dim]")) t = Table(title="model scorecard") for c in ["config", "model", "recall", "precision", "triage", "noise", "wall"]: t.add_column(c) @@ -88,7 +90,7 @@ def bench( t.add_row(tag, r["model"], f"{sc['discovery_recall']:.2f}", f"{sc['verify_precision']:.2f}", f"{sc['triage_accuracy']:.2f}", str(sc["noise"]), f"{sc['wall_time_s']:.0f}s") rprint(t) - path = write_results(res, target=target or repo, key=key) + path = write_results(res, target=target or repo, key=key, skipped=skipped) rprint(f"wrote [bold]{path}[/bold]") return diff --git a/src/vpcopilot/scorecard.py b/src/vpcopilot/scorecard.py index 051e208..6d48336 100644 --- a/src/vpcopilot/scorecard.py +++ b/src/vpcopilot/scorecard.py @@ -29,12 +29,15 @@ f" = {s.get('triage_accuracy', 0):.2f}"), ("bonus", "bonus finds", lambda s: f"{s.get('bonus_found', 0)}/{s.get('bonus_total', 0)}"), ("noise", "noise", lambda s: str(s.get("noise", 0))), + ("policies", "policies (unusable)", lambda s: f"{s.get('policies', 0)}" + + (f" (**{s['policies_flagged']}**)" if s.get("policies_flagged") else " (0)")), ("dupes", "dupes dropped", lambda s: str(s.get("duplicates_dropped", 0))), ("wall", "wall time", lambda s: f"{s.get('wall_time_s', 0):.0f}s"), ] -def build_results(results: dict, *, target: str, key: str, seed: int | None = None) -> str: +def build_results(results: dict, *, target: str, key: str, seed: int | None = None, + skipped: tuple = ()) -> str: """One row per config. A config whose run errored still gets a row carrying the error — a table that silently omits the provider that crashed reads as if it was never tried.""" ts = datetime.now(timezone.utc).strftime("%Y-%m-%d") @@ -80,11 +83,16 @@ def build_results(results: dict, *, target: str, key: str, seed: int | None = No "- **bonus finds** — real vulns beyond the core key, credited rather than counted as noise.", "- **dupes dropped** — duplicate findings for one vuln, collapsed so one vuln yields one " "band-aid and one code-fix PR.", + "- **policies (unusable)** — band-aids generated, and how many the deterministic linter " + "rejects before any live round-trip: an empty spec, no DENY rule, or an OpenAPI fragment " + "missing its envelope. Recall, precision and triage can all be perfect while the artifact " + "is unusable, so this column exists to stop the table flattering a model that routed " + "correctly and then emitted nothing.", "", "## What this table is not", "", - f"- **Not reproducible run to run.** {'A seed of ' + str(seed) + ' was requested, but ' if seed is not None else ''}" - "two of these providers accept no seed at all and none guarantee determinism, so scores " + f"- **Not reproducible run to run.** {'A seed of ' + str(seed) + ' was requested, but t' if seed is not None else 'T'}" + "wo of these providers accept no seed at all and none guarantee determinism, so scores " "vary between runs. Treat a small difference as noise; re-run before reading anything into " "one. The run date above is part of the result.", "- **Not a cost comparison.** Token cost is not measured — nothing in the harness counts " @@ -93,27 +101,38 @@ def build_results(results: dict, *, target: str, key: str, seed: int | None = No "says which model drives *this* pipeline well.", "", ] + if skipped: + notes[-1:] = [ + f"- **Not the full config set.** {', '.join('`' + t + '`' for t in sorted(skipped))} " + "was not run and is absent from the table — unreachable from the machine that produced " + "it, which is a different thing from a model that scored badly.", + "", + ] return "\n".join(head + [header, sep] + rows + notes) def write_results(results: dict, *, target: str, key: str, seed: int | None = None, - dest_dir: str = "benchmarks") -> str: + skipped: tuple = (), dest_dir: str = "benchmarks") -> str: d = Path(dest_dir) d.mkdir(parents=True, exist_ok=True) p = d / "RESULTS.md" - p.write_text(build_results(results, target=target, key=key, seed=seed)) + p.write_text(build_results(results, target=target, key=key, seed=seed, skipped=skipped)) return str(p) def run_all_configs(repo: str, *, key: str = "bench/answer_key.yaml", config_dir: str = "config", out_prefix: str = "out", min_confidence: float = 0.5, concurrency: int = 8, - seed: int | None = None, rescore: bool = False, + seed: int | None = None, rescore: bool = False, skip: tuple = (), log=print) -> dict: """Score every `config/agents*.yaml` against one target, into `out-` per config. A config that fails is recorded and the sweep continues: one dead provider must not cost you the other three runs' worth of tokens. `rescore=True` scores the existing out dirs without - re-scanning — the cheap path for iterating on the table itself.""" + re-scanning — the cheap path for iterating on the table itself. + + `skip` drops configs by tag. A model that is merely UNREACHABLE from this machine — a local + Ollama box behind a tunnel, say — is not a model that failed, and recording it as an error + would put a false red row in a published table.""" from .bench import run_bench from .config import load_config @@ -130,6 +149,9 @@ def run_all_configs(repo: str, *, key: str = "bench/answer_key.yaml", config_dir if tag == "default": # agents.yaml -> name it by its provider, as the console does tag = {"anthropic": "claude", "openai": "openai", "ollama": "dgx", "gemini": "gemini"}.get(model.split("/")[0], model.split("/")[0]) + if tag in skip: + log(f"[{tag}] skipped") + continue out_dir = f"{out_prefix}-{tag}" log(f"[{tag}] {model} -> {out_dir}") try: diff --git a/tests/test_scorecard.py b/tests/test_scorecard.py index a8bf0b3..b501789 100644 --- a/tests/test_scorecard.py +++ b/tests/test_scorecard.py @@ -146,3 +146,71 @@ def test_rescore_does_not_rerun_the_scan(monkeypatch, tmp_path): lambda repo, key, **kw: got.update(kw) or {"score": _score()}) scorecard.run_all_configs("repo", config_dir=str(cfgs), rescore=True, log=lambda m: None) assert got["scan"] is False + + +def test_a_skipped_config_is_omitted_and_declared(monkeypatch, tmp_path): + """A model unreachable from this machine is not a model that scored badly — it must not get a + red row, and the table must say it was left out rather than quietly under-reporting coverage.""" + cfgs = tmp_path / "config" + cfgs.mkdir() + (cfgs / "agents.yaml").write_text("defaults:\n model: anthropic/x\nagents: {}\n") + (cfgs / "agents.dgx.yaml").write_text("defaults:\n model: openai/local\nagents: {}\n") + import vpcopilot.bench as bench + monkeypatch.setattr(bench, "run_bench", lambda repo, key, **kw: {"score": _score()}) + res = scorecard.run_all_configs("repo", config_dir=str(cfgs), skip=("dgx",), log=lambda m: None) + assert set(res) == {"claude"} and "dgx" not in res + md = scorecard.build_results(res, target="t", key="k", skipped=("dgx",)) + assert "`dgx`" in md and "unreachable" in md.lower() + assert not [ln for ln in md.splitlines() if ln.startswith("| `dgx`")] + + +# ---- policy quality: the column the first real run proved the table needed ---- +def test_policy_quality_counts_this_runs_index_not_the_directory(tmp_path): + """A re-scan into an existing out dir leaves the previous run's artifacts in policies/ — 32 + files for 9 generated policies on the run that surfaced this. Globbing would score a model + against someone else's leftovers.""" + from vpcopilot.bench import _policy_quality + out = tmp_path + (out / "policies").mkdir() + (out / "policies.json").write_text(json.dumps([ + {"control": "service_policy", "policy_name": "good"}])) + (out / "policies" / "service_policy.good.json").write_text(json.dumps({"spec": {"rule_list": { + "rules": [{"spec": {"action": "DENY", "path": {"exact_values": ["/x"]}}}]}}})) + (out / "policies" / "service_policy.stale-from-a-previous-run.json").write_text("{}") + assert _policy_quality(out) == (1, 0) + + +def test_an_empty_spec_counts_as_unusable(tmp_path): + """Observed live: one model returned `{}` as the spec for three policies. Structured output + validated — `{}` is a valid dict — while the artifact was useless.""" + from vpcopilot.bench import _policy_quality + out = tmp_path + (out / "policies").mkdir() + (out / "policies.json").write_text(json.dumps([ + {"control": "service_policy", "policy_name": "empty"}, + {"control": "api_schema", "policy_name": "fragment"}])) + (out / "policies" / "service_policy.empty.json").write_text("{}") + (out / "policies" / "api_schema.fragment.json").write_text(json.dumps({"paths": {}})) + assert _policy_quality(out) == (2, 2) + + +def test_a_missing_artifact_counts_as_unusable(tmp_path): + from vpcopilot.bench import _policy_quality + out = tmp_path + (out / "policies").mkdir() + (out / "policies.json").write_text(json.dumps([{"control": "service_policy", "policy_name": "gone"}])) + assert _policy_quality(out) == (1, 1) + + +def test_no_index_is_zero_not_a_crash(tmp_path): + from vpcopilot.bench import _policy_quality + assert _policy_quality(tmp_path) == (0, 0) + + +def test_the_table_marks_unusable_policies(tmp_path): + md = scorecard.build_results({ + "good": {"model": "m", "score": _score(policies=9, policies_flagged=0)}, + "bad": {"model": "m", "score": _score(policies=6, policies_flagged=3)}, + }, target="t", key="k") + assert "9 (0)" in md and "6 (**3**)" in md + assert "unusable" in md.lower()