Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/review-drift-budget-arm-labels.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"review": patch
---

Eval reports for identical-arm runs (`--force-arms`: the weekly drift watch and manual wobble controls) no longer read as an A/B. The single-run, multi-repeat, and aggregate renderers retitle themselves "Review wobble control (identical arms)", relabel the Baseline/Candidate columns to Arm A/Arm B, state up front that between-arm deltas are run-to-run wobble, and the aggregate leads with the noise-floor bands (on an identical-arm pool the bands are the product; the per-case table is the raw material). Motivated by the first scheduled drift report (PR #265), whose baseline-vs-candidate framing over one prompt invited reading noise as a result. Also makes a contaminated noise floor a red run instead of a footnote: the drift workflow now fails (after uploading the report and opening the visibility PR) when the aggregate flags case asymmetry, i.e. the pooled samples scored different case sets because the budget skipped the corpus tail. The budget resize itself rides with the corpus-growth stack, which owns the sizing.
14 changes: 14 additions & 0 deletions .github/workflows/review-eval-drift.yml
Original file line number Diff line number Diff line change
Expand Up @@ -121,3 +121,17 @@ jobs:
--head "$BRANCH" \
--title "review: eval drift report $STAMP" \
--body-file out/live-ab-report.md
- name: Fail on a contaminated noise floor

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

question (non-blocking): This red-run gate lands while its trigger is already live and the paired budget bump was pulled out of this PR. The last committed drift report carries caseAsymmetry: true, and this diff keeps MAX_USD at 85 (the 85→120 bump was handed to the corpus-growth stack), so the next scheduled run will likely trip this exit 1 on the same corpus. The step's own remedy ("Raise max_usd and re-dispatch") also isn't reachable on the cron path without that bump. Is the intent for the weekly job to sit red until the corpus-growth stack lands, or should this gate land together with (or after) the budget raise?

# `caseAsymmetry` means the pooled samples scored different case
# sets (a budget-skipped tail), so the noise-floor bands carry
# case-mix variance and the drift run's product is degraded. The
# report body already warns, but a warning inside a footnote lets
# the degradation persist silently until someone reads it; a corpus
# outgrowing the budget should be a red run. Runs after the report
# is uploaded and the visibility PR exists, so nothing is lost.
run: |
if [ -f out/live-ab-report.json ] && \
[ "$(jq '.aggregate.noiseFloor.caseAsymmetry // false' out/live-ab-report.json)" = "true" ]; then
echo "::error::Noise-floor bands are contaminated: pooled samples scored different case sets (budget-skipped tail?). Raise max_usd and re-dispatch." >&2
exit 1
fi
49 changes: 49 additions & 0 deletions workflows/review/eval/aggregate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,55 @@ describe("renderAggregateMarkdown", () => {
"| must-catch recall | 100% | 100% | 100% | 0% | 0% |",
);
});

it("prefixes drop notes with the neutral arm names on identical-arm pools", () => {
// Every other identical-arm test uses caught specs with no drops, so
// dropNote never renders; without this case a regression back to
// base:/cand: would ship a report headed "Arm A/Arm B" whose drop
// notes still read as an A/B.
const raw = rawReport({
baselineRuns: [rawRun("case-1", {caught: ["spec-1"]})],
candidateRuns: [
rawRun("case-1", {
missedDetail: [
{specKey: "spec-1", droppedBy: "provenance"},
],
}),
],
baselineSha: "a".repeat(64),
candidateSha: "a".repeat(64),
});
const markdown = renderAggregateMarkdown(
aggregateSamples(extractSamples("r1", raw)),
);
expect(markdown).toContain("arm B: ");
expect(markdown).not.toContain("cand: ");
});

it("relabels the arms and leads with the noise floor on identical-arm pools", () => {
const raw = rawReport({
baselineRuns: [rawRun("case-1", {caught: ["spec-1"]})],
candidateRuns: [rawRun("case-1", {caught: ["spec-1"]})],
baselineSha: "a".repeat(64),
candidateSha: "a".repeat(64),
});
const markdown = renderAggregateMarkdown(
aggregateSamples(extractSamples("r1", raw)),
);
// One prompt in both arms: a wobble control, not an A/B. The
// baseline/candidate framing would invite reading noise as a result.
expect(markdown).toContain(
"## Review wobble control: repeat aggregation (identical arms)",
);
expect(markdown).toContain("run-to-run wobble, not a prompt effect");
expect(markdown).toContain("| Case / spec | Arm A | 95% CI | Arm B |");
expect(markdown).toContain("| Metric | Arm A | 95% CI | Arm B |");
expect(markdown).not.toContain("Baseline");
// The bands are the identical-arm pool's product; they render first.
expect(markdown.indexOf("### Noise floor")).toBeLessThan(
markdown.indexOf("| Case / spec |"),
);
});
});

describe("rateStat", () => {
Expand Down
114 changes: 71 additions & 43 deletions workflows/review/eval/aggregate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -624,23 +624,44 @@ const dropNote = (spec: SpecAggregate): string => {
};

/**
* The aggregate as a markdown report: a per-case table (spec catch rates and
* verdict agreement, both arms, Wilson intervals), the pooled rows, and the
* noise-floor bands when the pool was an identical-arm control.
* The aggregate as a markdown report: the noise-floor bands first when the
* pool was an identical-arm control (they are that pool's product), then a
* per-case table (spec catch rates and verdict agreement, both arms, Wilson
* intervals) and the pooled rows.
*/
export const renderAggregateMarkdown = (report: AggregateReport): string => {
const {baseline, candidate} = report.arms;
// `noiseFloor` is only computed for identical-arm pools (wobble controls
// and the weekly drift run), so its presence IS the identical-arms
// signal. Those reports relabel the arms: "baseline vs candidate" over
// one prompt invites reading wobble as an A/B result, and the arm split
// is arbitrary there.
const identicalArms = report.noiseFloor !== undefined;
Comment thread
khan-actions-bot marked this conversation as resolved.
const [armALabel, armBLabel] = identicalArms
? ["Arm A", "Arm B"]
: ["Baseline", "Candidate"];
const [armANote, armBNote] = identicalArms
? ["arm A", "arm B"]
: ["base", "cand"];
const lines = [
"## Review live A/B: repeat aggregation",
identicalArms
? "## Review wobble control: repeat aggregation (identical arms)"
: "## Review live A/B: repeat aggregation",
"",
`Pooled ${report.samples} run(s) per arm from: ${report.sources.join(
", ",
)}.`,
`Baseline review.md ${baseline.reviewMdShas
.map((sha) => sha.slice(0, 12))
.join(", ")}; candidate ${candidate.reviewMdShas
.map((sha) => sha.slice(0, 12))
.join(", ")}.`,
identicalArms
? `Every sample ran the same review.md (${baseline.reviewMdShas
.map((sha) => sha.slice(0, 12))
.join(", ")}): the arm split is arbitrary, and every ` +
`between-arm delta below is run-to-run wobble, not a prompt ` +
`effect.`
: `Baseline review.md ${baseline.reviewMdShas
.map((sha) => sha.slice(0, 12))
.join(", ")}; candidate ${candidate.reviewMdShas
.map((sha) => sha.slice(0, 12))
.join(", ")}.`,
"",
];
if (baseline.reviewMdShas.length > 1 || candidate.reviewMdShas.length > 1) {
Expand Down Expand Up @@ -678,8 +699,41 @@ export const renderAggregateMarkdown = (report: AggregateReport): string => {
);
}

// The noise-floor bands lead when present: on an identical-arm pool
// they are the product, and everything below them is the raw material.
if (report.noiseFloor !== undefined) {
lines.push(
"### Noise floor (identical arms: every sample ran the same prompt)",
"",
`Bands across ${report.noiseFloor.armSamples} arm-samples of one review.md; ` +
"any A/B delta inside a band is indistinguishable from " +
"run-to-run wobble. Min/max only widen as samples accumulate; " +
"mean +/- sd is the band to track week to week.",
"",
...(report.noiseFloor.caseAsymmetry
? [
"**WARNING: the samples did not all score the same " +
"case set (budget skips or mixed corpora), so " +
"these bands fold case-mix variance in on top of " +
"run-to-run wobble. Re-run with a budget that " +
"clears the full corpus before trusting them.**",
"",
]
: []),
"| Metric | Min | Mean | Max | SD | Spread |",
"| --- | --- | --- | --- | --- | --- |",
...Object.entries(report.noiseFloor.bands).map(
([metric, band]) =>
`| ${metric} | ${pct(band.min)} | ${pct(band.mean)} | ${pct(
band.max,
)} | ${pct(band.sd)} | ${pct(band.max - band.min)} |`,
),
"",
);
}

lines.push(
"| Case / spec | Baseline | 95% CI | Candidate | 95% CI | Miss classes |",
`| Case / spec | ${armALabel} | 95% CI | ${armBLabel} | 95% CI | Miss classes |`,
"| --- | --- | --- | --- | --- | --- |",
);
const caseIds = [
Expand All @@ -701,8 +755,12 @@ export const renderAggregateMarkdown = (report: AggregateReport): string => {
const b = base?.specs.find((s) => s.specKey === specKey);
const c = cand?.specs.find((s) => s.specKey === specKey);
const notes = [
...(b && dropNote(b) !== "" ? [`base: ${dropNote(b)}`] : []),
...(c && dropNote(c) !== "" ? [`cand: ${dropNote(c)}`] : []),
...(b && dropNote(b) !== ""
? [`${armANote}: ${dropNote(b)}`]
Comment thread
khan-actions-bot marked this conversation as resolved.
: []),
...(c && dropNote(c) !== ""
? [`${armBNote}: ${dropNote(c)}`]
: []),
];
lines.push(
`| ${caseId}:${specKey} | ${b ? statCell(b.caught) : "n/a"} | ${
Expand Down Expand Up @@ -740,7 +798,7 @@ export const renderAggregateMarkdown = (report: AggregateReport): string => {
"",
"### Pooled",
"",
"| Metric | Baseline | 95% CI | Candidate | 95% CI |",
`| Metric | ${armALabel} | 95% CI | ${armBLabel} | 95% CI |`,
"| --- | --- | --- | --- | --- |",
pooledRow("Must-catch recall", (a) => a.pooled.recall),
pooledRow("Verdict agreement", (a) => a.pooled.verdictAgreement),
Expand All @@ -763,36 +821,6 @@ export const renderAggregateMarkdown = (report: AggregateReport): string => {
"",
);

if (report.noiseFloor !== undefined) {
lines.push(
"### Noise floor (identical arms: every sample ran the same prompt)",
"",
`Bands across ${report.noiseFloor.armSamples} arm-samples of one review.md; ` +
"any A/B delta inside a band is indistinguishable from " +
"run-to-run wobble. Min/max only widen as samples accumulate; " +
"mean +/- sd is the band to track week to week.",
"",
...(report.noiseFloor.caseAsymmetry
? [
"**WARNING: the samples did not all score the same " +
"case set (budget skips or mixed corpora), so " +
"these bands fold case-mix variance in on top of " +
"run-to-run wobble. Re-run with a budget that " +
"clears the full corpus before trusting them.**",
"",
]
: []),
"| Metric | Min | Mean | Max | SD | Spread |",
"| --- | --- | --- | --- | --- | --- |",
...Object.entries(report.noiseFloor.bands).map(
([metric, band]) =>
`| ${metric} | ${pct(band.min)} | ${pct(band.mean)} | ${pct(
band.max,
)} | ${pct(band.sd)} | ${pct(band.max - band.min)} |`,
),
"",
);
}
return lines.join("\n");
};

Expand Down
68 changes: 49 additions & 19 deletions workflows/review/eval/live-ab-report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,20 +132,36 @@ export type MultiAbReport = {

export const renderMultiMarkdownReport = (report: MultiAbReport): string => {
const first = report.repeats[0];
// Identical review.md in both arms only happens under `--force-arms`
// (the runner short-circuits otherwise): a wobble control or the weekly
// drift run, not an A/B. Say so up front; a report headed
// "baseline vs candidate" over one prompt invites reading noise as a
// result.
const identicalArms =
first !== undefined &&
first.reviewMdSha.baseline === first.reviewMdSha.candidate;
const lines = [
`## Review live A/B: ${report.repeatCount} repeats`,
identicalArms
? `## Review wobble control: ${report.repeatCount} repeats (identical arms)`
: `## Review live A/B: ${report.repeatCount} repeats`,
"",
...(first !== undefined
? [
`Baseline: \`${
first.baseRef
}\` (review.md ${first.reviewMdSha.baseline.slice(
0,
12,
)}); candidate: working tree (review.md ${first.reviewMdSha.candidate.slice(
0,
12,
)}).`,
identicalArms
? `Both arms ran the same review.md (${first.reviewMdSha.baseline.slice(
0,
12,
)}, base \`${first.baseRef}\`): every between-arm ` +
`delta below is run-to-run wobble, not a prompt effect.`
: `Baseline: \`${
first.baseRef
}\` (review.md ${first.reviewMdSha.baseline.slice(
0,
12,
)}); candidate: working tree (review.md ${first.reviewMdSha.candidate.slice(
0,
12,
)}).`,
"",
]
: []),
Expand Down Expand Up @@ -248,6 +264,12 @@ const NOISE_FLOOR_FOOTER =

export const renderMarkdownReport = (report: AbReport): string => {
const {baseline, candidate} = report.arms;
// See renderMultiMarkdownReport: identical shas imply `--force-arms`.
const identicalArms =
report.reviewMdSha.baseline === report.reviewMdSha.candidate;
const [armALabel, armBLabel] = identicalArms
? ["Arm A", "Arm B"]
: ["Baseline", "Candidate"];
const row = (
label: string,
base: string,
Expand All @@ -268,15 +290,23 @@ export const renderMarkdownReport = (report: AbReport): string => {
);

const lines = [
"## Review live A/B",
identicalArms
? "## Review wobble control (identical arms)"
: "## Review live A/B",
"",
`Baseline: \`${
report.baseRef
}\` (review.md ${report.reviewMdSha.baseline.slice(0, 12)}); ` +
`candidate: working tree (review.md ${report.reviewMdSha.candidate.slice(
0,
12,
)}).`,
identicalArms
? `Both arms ran the same review.md (${report.reviewMdSha.baseline.slice(
0,
12,
)}, base \`${report.baseRef}\`): every between-arm delta ` +
`below is run-to-run wobble, not a prompt effect.`
: `Baseline: \`${
report.baseRef
}\` (review.md ${report.reviewMdSha.baseline.slice(0, 12)}); ` +
`candidate: working tree (review.md ${report.reviewMdSha.candidate.slice(
0,
12,
)}).`,
"",
...(report.provenance !== undefined
? [
Expand All @@ -286,7 +316,7 @@ export const renderMarkdownReport = (report: AbReport): string => {
"",
]
: []),
"| Metric | Baseline | Candidate | Delta |",
`| Metric | ${armALabel} | ${armBLabel} | Delta |`,
"| --- | --- | --- | --- |",
metric("Must-catch recall", (a) => a.metrics.mustCatchRecall.rate),
metric("Verdict agreement", (a) => a.metrics.verdictAgreement.rate),
Expand Down
Loading
Loading