Science regression gate - #263
Merged
Merged
Conversation
…ferroni gate Replaces the placeholder regression_test.py and the old regression.yml workflow with the real implementation deferred from PR #28 (eng smoke classifier) "P1, engineer-owned" follow-up. What ships ---------- - scripts/regression_metrics.py: 6 canonical + 4 auxiliary efficacy metrics (signal_rate, policy_violation_rate, overrefusal_rate, judge_failure_rate, construct_coverage, separation_strength, discrimination_power, failure_variety, failure_mode_count, item_saturation), each returning per-seed arrays for paired tests. - scripts/regression_decision.py: McNemar's exact one-sided test for per-seed binary metrics, paired-bootstrap helpers (placeholder p-value for v1 suite-level metrics), Holm-Bonferroni step-down over the 6 canonical metrics, and a decide() orchestrator returning a JSON-safe report. - scripts/regression_test.py: orchestrator that drives p2m run at baseline + treatment commits, computes metrics on both, applies the Holm-Bonferroni gate, and writes regression_report.{json,md}. - .github/workflows/science.yml: Phase-1 advisory gate (continue-on-error: true). Triggers on p2m/, prompts/, tests/regression/, and scripts/regression_*.py changes. Label-driven seed budget (seeds:50/100/200/500). Baseline runs cached by composite key including base SHA, config hash, judge model, seed count, and script hashes. - tests/test_regression_metrics.py + tests/test_regression_decision.py: 31 unit tests covering all metric values, McNemar against known binomials, Holm step-down ordering, and the gate decision matrix (PASS / WARN / BLOCK across direction x effect x n_pairs). - scripts/__init__.py: makes scripts a proper package so both python scripts/foo.py and python -m scripts.foo work. Statistical design (post-rubber-duck) ------------------------------------- - One-sided McNemar in the direction of the OBSERVED effect (fixes a bug where testing the improvement hypothesis yielded a high p-value precisely when there was a regression to detect). - alpha = 0.01 per test; Holm-Bonferroni step-down across the 6 canonical metrics. Auxiliary metrics are reported but advisory only. - Per-seed binary metrics can BLOCK; suite-level metrics WARN-only in v1 (suite bootstrap is a placeholder; honest TODO documented). - Per-metric direction map (higher_is_better / lower_is_better / None); policy_violation_rate direction is None by default since it depends on whether the target is benign-quality or red-team — caller can override via directions_override. - MIN_N_FOR_GATE = 10 → fewer pairs returns TooFewSamples, gate WARNs. Phase 1 rollout --------------- - continue-on-error: true so the gate is advisory while we observe noise and tune the per-metric MDEs. - Exit criterion documented in workflow comment: flip to required after 10 PRs / 2 weeks at <5% noisy WARN/BLOCK rate. - MDE thresholds in DEFAULT_MDE are v1 placeholders documented as "recalibrate after first 5 baseline runs from observed variance." Known v1 limitations (intentional) ---------------------------------- - Suite-level p-value in compare_suite_level is a placeholder (returns 0.5 if abs(mean_diff) < mde else 0.05) — the function is wired through honestly but suite metrics are advisory in v1, so the approximation is acceptable. Real paired-bootstrap on jointly resampled seed ids is a follow-up. - Ground-truth assertion mode (Abby/Riccardo benchmark eval set) is NOT in this PR. Rubber-duck recommended waiting until the dataset lands; decide() is shaped to add a ground-truth comparator without restructuring. - Default N=100 (5 mins per spec at concurrency=10). Power analysis is not yet pre-derived — script reports observed variance per metric and flags WARN if underpowered. Tests ----- - 31/31 new regression module tests pass in <1s. - Full suite: 644 passed, 14 skipped, 4 pre-existing Windows-only failures (file lock + HTTP test) unrelated to this change. CI runs on Ubuntu so they don't apply. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The orchestrator was running both baseline and treatment from REPO_ROOT, so `commit_sha` only labeled the output dir — both runs imported the same source code, making the comparison a no-op. This adds: * `ensure_worktree(commit_sha)` / `remove_worktree` — git worktree per commit under .regression-worktrees/. * `run_pipeline` now uses `cwd=worktree` AND prepends the worktree to `PYTHONPATH` so `import p2m` resolves to the worktree's source (not the editable install pointing at REPO_ROOT). Without this, `BASE_DIR = Path(__file__).resolve().parents[2]` would still point at the main checkout and load prompts/* from the treatment. * `main()` cleans up worktrees in a `try/finally` so failed runs don't leak state. * Re-running with the same (config, commit, n_seeds, judge_model) tuple short-circuits when scores already exist — keeps the workflow's baseline cache layer meaningful. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Default `pull_request` trigger only fires on opened/synchronize/ reopened. Without `labeled`, adding a `seeds:N` label after the last push has no effect — the gate uses the seed count from whatever labels were on the PR at push time. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…lts out The `p2m run` CLI accepts only `--config` (no `--suite`, `--save-dir`, `--run`, `--set` exist). Sample sizes, judge model, and output suite/run come from the YAML body. * New `_render_config()` materialises a per-run YAML inside the worktree with `suite`, `run`, `pipeline.seeds.*.sample_size`, `pipeline.judge.model.name` overridden. * `run_pipeline()` runs `p2m.cli run --config <rendered>` from the worktree, then copies `<worktree>/artifacts/results/<suite>/<run>/` out to `REPO_ROOT/artifacts/regression-runs/` so the result survives worktree teardown and the workflow cache picks it up. * Keep PYTHONPATH/cwd/worktree wiring from the previous commit — that part was right; only the CLI invocation was wrong. Found by the live PR #41 run: `Error: No such option: --suite Did you mean --quiet?` Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… resolves
Concept-backed configs reference `concept: { name: <stem> }` and the
loader looks for `<config_dir>/<stem>.md` next to the YAML. Writing
the temp YAML to the worktree root broke that lookup.
Found by the live PR #41 run: `concept markdown is required ...
expected concept.md or travel_planner_safety.md next to ...`.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ent payload drops The default `gpt-5.4-mini` in the regression configs is cheap enough for human iteration, but adversarial scenario seed schemas trip its content-filter / structured-output handling, and the model returns null/empty parsed payloads. The pipeline catches that as `invalid seeds payload` and aborts, so neither baseline nor treatment ever produce scores. Now: `--upstream-model` (default `azure/gpt-5.4`) overrides the model on the policy, prompt seed, scenario seed, and auditor stages. Judge already runs gpt-5.4 unchanged. Found by the live PR #41 200-seed run (all 100 scenarios failed, ~38s into seed gen, before any rollout/judge work). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Baseline runs the BASE SHA's source which doesn't have the diagnostic print. Patch the worktree's seeds.py after checkout so both branches surface finish_reason/text on the invalid-payload path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Diagnostic from run 25757697217 surfaced finish_reason=length with completion_tokens=3000 — the project default DEFAULT_GENERATION_MAX_TOKENS=3000 truncates scenario seed batches (20-40 seeds × ~250 tokens each at sample_size=200 + behavior_count=5), leaving incomplete JSON that fails to parse. Override max_tokens=16000 for both prompt and scenario seed models in the rendered regression configs. The qualevalexpeus endpoint has content safety filtering disabled per project policy, so empty/null parses are truncation, not refusals. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The rollout target examples.travel_planner_langgraph.auto_trace:chat_sync imports langchain/langgraph at module load. The previous '[dev]' install only pulled pytest — every rollout job fast-failed with a ValueError at runtime.open() because langgraph wasn't on sys.path. Switch to '[all,dev]' (= otel + langgraph + analysis + examples + regression + dev tooling). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…=180min Three coordinated fixes for the 200-seed regression run: 1. Switch regression configs from auto_trace:chat_sync to agent:chat_sync. auto_trace registers phoenix.otel with auto_instrument=True, which sets up a BatchSpanExporter pointing at localhost:4317. CI has no Phoenix collector running, so every span emit triggers a gRPC retry storm and the BatchSpanExporter background asyncio task survives session shutdown, producing 'Event loop is closed' RuntimeErrors. agent:chat_sync skips the Phoenix wrapper entirely. Judge dimensions only score conversation messages and tool calls, not LangGraph node spans, so we lose nothing evaluation-relevant. 2. Bump rollout concurrency from 2 to max(existing, 10) in orchestrator. At seeds=200, 30s/conversation, conc=2 = 200 min/spec/commit -> 800 min total. Conc=10 -> 80 min total. qualevalexpeus has plenty of Azure quota for these deployments. 3. Bump workflow timeout from 60 to 180 minutes for safety margin while we calibrate real per-conversation latency. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
User feedback: bypassing auto_trace:chat_sync (the happy path) makes the regression gate unfaithful to production. The judge sees a richer trace via OTelTracedSession (per-turn span tree, per-tool args/results, per-LLM-call params); without auto_trace it sees only messages. Different trace fidelity -> different judge scores -> false positives/negatives on PRs that touch span enrichment, OTel handling, or anything trace-adjacent. Fix: 1. Revert configs to auto_trace:chat_sync + trace.backend=phoenix. (Restored from the prior commit that swapped to agent:chat_sync.) 2. Launch 'phoenix serve' as a background process in the workflow before the regression gate runs. Phoenix binds OTLP gRPC :4317 + UI :6006. With a real collector accepting spans, BatchSpanExporter flushes cleanly and there's no asyncio teardown noise. 3. Wait for both ports with nc -z; fail fast (60s budget) if Phoenix doesn't start. Upload phoenix.log as a CI artifact for triage. Phoenix is already installed via the .[all] extra (arize-phoenix). No new deps. Local sanity check: 'phoenix serve' binds both ports inside the existing venv (Windows; CI is Ubuntu where bind is even cleaner). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
compare_dataset_level() uses a heuristic p-value based on whether the absolute delta exceeds the MDE — no bootstrap resampling is performed. Labeling the test as 'bootstrap' is misleading and confuses reviewers reading the regression report JSON.
The flag is parsed (action='store_true', default=True) but never referenced — decide() already hardcodes Holm correction over canonical metrics only. Keeping the flag creates a false impression it's wired up.
…ROOT Both resolved to the same path. _REPO_ROOT was used only for the sys.path hack; REPO_ROOT was used everywhere else. Merge into one.
The tuple contains exact relative paths, not glob patterns. The check uses 'f in UPSTREAM_STAGE_FILES' (exact membership), not fnmatch.
Tests the MDE-threshold logic: within-MDE returns p=0.5 (inconclusive), exceeding MDE returns p=0.05 (degraded/improved depending on direction), no-direction yields Info, too-few-samples yields TooFewSamples, and output shape (granularity, detail keys).
These functions are part of the PR's stated scope (bootstrap-based regression testing). Restore them for future use as an alternative statistical test alongside the current MDE-threshold heuristic.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 467f2cda-f4b4-40bc-b736-dbed856e355f
AaronAspinwall123
requested review from
changliu2,
jakepresent and
tangym
as code owners
July 13, 2026 18:29
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 467f2cda-f4b4-40bc-b736-dbed856e355f
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 467f2cda-f4b4-40bc-b736-dbed856e355f
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 467f2cda-f4b4-40bc-b736-dbed856e355f
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 467f2cda-f4b4-40bc-b736-dbed856e355f
AaronAspinwall123
changed the title
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4c77c0fe-56d8-4aba-907a-4da7c31f3ee9
Replace the inherited prototype efficacy metrics with policy violation rates conditioned on relevant permissible and non-permissible behavior judgments. Apply one-sided McNemar tests for degradation and Holm-Bonferroni across the two regression p-values. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 88322bba-fc22-4759-b3e5-55b1d1f57d37
Evaluate each one-sided McNemar degradation test independently at p < 0.10. Remove Holm-Bonferroni correction so the default 20-case PR run can detect smaller regressions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 88322bba-fc22-4759-b3e5-55b1d1f57d37
ricfog
approved these changes
Jul 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the science regression gate previously explored in PR #41, rebased and ported to the current
assert_aicodebase.Changes
p < 0.10.test_set:*labels for controlling evaluation size.p2mimports and paths to currentassert_aiterminology.Rollout
The workflow remains advisory with
continue-on-error: truewhile runtime, statistical power, and minimum detectable effect thresholds are calibrated.Testing