Skip to content

ci(science): regression-gate workflow + 6 efficacy metrics + Holm-Bonferroni gate - #41

Closed
Chang Liu (changliu2) wants to merge 22 commits into
mainfrom
science-regression-gate
Closed

ci(science): regression-gate workflow + 6 efficacy metrics + Holm-Bonferroni gate#41
Chang Liu (changliu2) wants to merge 22 commits into
mainfrom
science-regression-gate

Conversation

@changliu2

@changliu2 Chang Liu (changliu2) commented May 11, 2026

Copy link
Copy Markdown
Collaborator

Note

Rebased onto main (post-PR #23 terminology). This branch now merges cleanly after the terminology migration and keeps the science gate advisory.

Terminology renames applied in this rebase

  • --seeds / n_seeds / seeds:* labels -> --test_set / test_set_size / test_set:*.
  • p2m/stages/seeds.py references -> p2m/stages/test_set.py worktree diagnostics.
  • Customer-facing prose now uses failure-mode configs, test cases, and dataset-level metrics.

Summary

Replaces the placeholder scripts/regression_test.py and old regression.yml workflow with the real implementation deferred from PR #28's "P1, engineer-owned" follow-up. Ships 6 canonical efficacy metrics + 4 auxiliary, paired statistical tests (McNemar's exact + paired bootstrap helpers), Holm-Bonferroni-corrected gate, and the new science.yml workflow as Phase 1 advisory (continue-on-error: true).

What's in the box

File Role
scripts/regression_metrics.py 10 pure-functional metric calculators returning MetricResult (per-test-case array + value + detail).
scripts/regression_decision.py McNemar one-sided exact test, paired bootstrap helpers, Holm-Bonferroni step-down, decide() orchestrator.
scripts/regression_test.py Replaces 73-line stub. CLI: --baseline, --treatment, --test_set, --judge-model, --enforce. Detects upstream-stage changes via git diff to decide whether to freeze systematize/stratification/test_set.
.github/workflows/science.yml Phase-1 advisory gate. Triggers on p2m/**, prompts/**, tests/regression/**, scripts/regression_*.py. Label-driven test-set size. Baseline runs cached by composite key.
scripts/__init__.py Makes scripts a proper package so both python scripts/foo.py and python -m scripts.foo work.
tests/test_regression_metrics.py + tests/test_regression_decision.py 31 unit tests; full suite stays green.

The 6 canonical metrics + 4 auxiliary

Canonical (Holm-Bonferroni-gated): signal_rate, policy_violation_rate, overrefusal_rate, judge_failure_rate, construct_coverage, separation_strength.

Auxiliary (advisory only): discrimination_power, failure_variety, failure_mode_count, item_saturation.

Each canonical metric declares a direction (higher_is_better / lower_is_better / None). Per-test-case binary metrics can BLOCK; dataset-level metrics WARN-only in v1.

Statistical design (post-rubber-duck)

  • One-sided McNemar in the direction of the OBSERVED effect. Fixes a real bug in the first draft where testing the improvement hypothesis yielded a high p-value precisely when there was a regression to detect. Caught + corrected by the unit-test matrix.
  • alpha = 0.01 per test; Holm step-down across the 6 canonical metrics.
  • policy_violation_rate.direction = None by default — depends on whether the target is benign-quality or adversarial. Caller can override via directions_override.
  • MIN_N_FOR_GATE = 10; fewer pairs returns TooFewSamples, gate WARNs.
  • Default N=100 per config (200 total test cases at default). Override via PR labels or manual dispatch input: test_set:50, test_set:100, test_set:200, test_set:500.

Phase 1 rollout (intentional)

  • continue-on-error: true so the gate is advisory while we observe noise and tune per-metric MDEs.
  • Exit criterion documented in workflow comment: flip to required after 10 PRs / 2 weeks at <5% noisy WARN/BLOCK rate.
  • DEFAULT_MDE thresholds are placeholders — documented as recalibrate after first 5 baseline runs from observed variance.

Non-blocking daily sentinel

  • Adds a daily schedule trigger on main at 0 12 * * * (12:00 UTC) so the workflow acts as a regression sentinel before the US workday.
  • Adds workflow_dispatch with inputs.test_set for manual scheduled-style runs.
  • Scheduled/manual runs compare origin/main from 7 days ago against current origin/main; PR runs keep base-vs-head behavior.
  • continue-on-error: true remains job-level, so PR, scheduled, and manual runs are advisory/non-blocking.

Known v1 limitations (intentional)

  1. Dataset-level p-value is a placeholder in compare_suite_level. 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.
  2. Ground-truth assertion mode is NOT in this PR. Per rubber-duck, waiting for Abby/Riccardo's benchmark eval set. decide() is shaped to add a ground-truth comparator without restructuring.
  3. Power analysis is reported, not pre-derived. Script reports observed variance per metric and flags WARN if the test-set size was insufficient for adequate power at the configured MDE.

Testing

  • uv run pytest tests/test_regression_metrics.py tests/test_regression_decision.py -x -q -> 31/31 passed.
  • Full non-viewer suite: 714 passed, 14 skipped, 58 deselected, 13 subtests passed.

Followups (parked)

  • Recalibrate DEFAULT_MDE after first 5 baseline runs (separate PR).
  • Implement real paired-bootstrap for dataset-level metrics (separate PR).
  • Plug in Abby/Riccardo benchmark eval set as ground-truth assertion mode (separate PR, when dataset lands).
  • Flip continue-on-error: false after the noise-calibration window (separate PR).

References

@changliu2 Chang Liu (changliu2) added the seeds:200 Run science regression with N=200 seeds per spec label May 12, 2026
Chang Liu (changliu2) added a commit that referenced this pull request May 12, 2026
…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>
Chang Liu (changliu2) added a commit that referenced this pull request May 12, 2026
… 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>
Chang Liu (changliu2) added a commit that referenced this pull request May 12, 2026
…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>
Chang Liu (changliu2) and others added 12 commits May 18, 2026 19:21
…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>
Temporary diagnostic — surface finish_reason, status, incomplete_details, text
when scenario seed generation returns a non-dict payload in CI.

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>
Chang Liu (changliu2) and others added 2 commits May 21, 2026 15:28
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@tangym tangym left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixes pushed

Six commits addressing issues found during review:

  1. test="bootstrap"test="mde_threshold"compare_dataset_level() was passing the wrong test name to _classify_effect, so the MDE-threshold logic was never reached.
  2. Removed dead bootstrap functionspaired_bootstrap_ci and bootstrap_delta_pvalue were unreachable after the switch to MDE-threshold.
  3. Removed dead --alpha-canonical-only CLI flag — The argument was parsed but never used anywhere.
  4. Consolidated _REPO_ROOT / REPO_ROOT — Two identically-valued constants; kept REPO_ROOT and removed the private duplicate.
  5. Renamed UPSTREAM_STAGE_GLOBSUPSTREAM_STAGE_FILES — The values are literal filenames, not globs.
  6. Added CompareDatasetLevelTest coverage — Six new unit tests covering the no-change, degraded, improved, and inconclusive paths through compare_dataset_level().

tangym added 8 commits May 22, 2026 20:53
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.
These functions were defined but never called anywhere in the codebase.
The dataset-level comparison uses an MDE threshold heuristic and the
per-test-case comparison uses McNemar's test — neither invokes bootstrap
resampling. Also removes paired_bootstrap_ci from __all__.
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.
The flag documents the Holm-Bonferroni scoping behavior (canonical
metrics only) and leaves a hook for future toggling.

@tangym tangym left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review — 6 fixes applied (updated)

I picked up this branch and applied fixes for the issues flagged in my earlier inline comments. All 21 tests pass.

Fixes applied (kept)

  1. test="bootstrap"test="mde_threshold" in compare_dataset_level() — the function uses an MDE-threshold heuristic, not bootstrap resampling. The label should match the actual method. (regression_decision.py)

  2. Consolidated _REPO_ROOT / REPO_ROOT — there were two definitions of the repo root; unified to a single REPO_ROOT. (regression_test.py)

  3. Renamed UPSTREAM_STAGE_GLOBSUPSTREAM_STAGE_FILES — the constant holds literal file paths, not glob patterns. (regression_test.py)

  4. Added CompareDatasetLevelTest — 6 new unit tests covering the MDE-threshold logic, regression/improvement/inconclusive classification, and edge cases. (test_regression_decision.py)

Previously removed code — restored

  1. paired_bootstrap_ci() and bootstrap_delta_pvalue() — restored. These are part of the PR's stated scope (bootstrap-based regression testing) and provide an alternative statistical test path for when per-test-case deltas become available in dataset-level comparisons.

  2. --alpha-canonical-only CLI flag — restored. Documents the Holm-Bonferroni scoping (canonical metrics only, which is already the hardcoded behavior in decide()) and leaves a hook for future toggling.

Note on wiring: Both restored pieces are not trivially wirable into the current code path — bootstrap functions need per-test-case delta arrays (not available in dataset-level comparisons today), and the canonical-only flag matches the already-hardcoded Holm behavior. Wiring them would be a substantive design change beyond the scope of this review pass.

@AaronAspinwall123

Copy link
Copy Markdown
Collaborator

closing for hygiene, reopen if needed

Aaron Aspinwall (AaronAspinwall123) added a commit that referenced this pull request Jul 29, 2026
* ci(science): regression-gate workflow + 6 efficacy metrics + Holm-Bonferroni 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>

* fix(science): isolate baseline checkout via git worktree + PYTHONPATH

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>

* ci(science): re-trigger gate when PR labels change

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>

* fix(science): drive seed/judge overrides via rendered YAML, copy results 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>

* fix(science): emit rendered YAML next to source so sibling concept md 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>

* fix(science): drive policy/seeds/auditor through gpt-5.4 to avoid silent 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>

* debug(science): inject seeds-payload diagnostic into both worktrees

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>

* fix(science): bump seed-generation max_tokens to 16000

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>

* fix(science): install langgraph + examples extras in CI

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>

* fix(science): drop Phoenix OTLP wrapper, bump concurrency=10, timeout=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>

* fix(science): keep auto_trace path, run real Phoenix collector in CI

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>

* fix(science): align regression metrics with test cases

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: label dataset-level test as 'mde_threshold' not 'bootstrap'

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.

* chore: remove dead --alpha-canonical-only CLI flag

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.

* chore: consolidate duplicate _REPO_ROOT / REPO_ROOT into single REPO_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.

* chore: rename UPSTREAM_STAGE_GLOBS → UPSTREAM_STAGE_FILES

The tuple contains exact relative paths, not glob patterns. The check
uses 'f in UPSTREAM_STAGE_FILES' (exact membership), not fnmatch.

* test: add coverage for compare_dataset_level()

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).

* revert: restore paired_bootstrap_ci and bootstrap_delta_pvalue

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.

* chore(science): port regression gate to current main

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 467f2cda-f4b4-40bc-b736-dbed856e355f

* ci(science): limit PR trigger to regression files

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 467f2cda-f4b4-40bc-b736-dbed856e355f

* ci(science): remove duplicate tier 4 gate

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 467f2cda-f4b4-40bc-b736-dbed856e355f

* fix(science): make regression gate truly paired

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 467f2cda-f4b4-40bc-b736-dbed856e355f

* fix(science): bound PR regression runtime

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 467f2cda-f4b4-40bc-b736-dbed856e355f

* test(viewer): align inference label expectation

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4c77c0fe-56d8-4aba-907a-4da7c31f3ee9

* fix(ci): gate on permissibility-split violations

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

* fix(ci): increase regression gate power

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

---------

Co-authored-by: Chang Liu <changliu2@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Yeming Tang <tangym@users.noreply.github.com>
Copilot-Session: 467f2cda-f4b4-40bc-b736-dbed856e355f
Copilot-Session: 4c77c0fe-56d8-4aba-907a-4da7c31f3ee9
Copilot-Session: 88322bba-fc22-4759-b3e5-55b1d1f57d37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

seeds:200 Run science regression with N=200 seeds per spec

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants