Skip to content
2 changes: 2 additions & 0 deletions .github/scripts/fixtures/serve-ab-session.jsonl
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{"uuid":"10000000-0000-4000-8000-000000000001","parentUuid":null,"sessionId":"00000000-0000-4000-8000-000000000000","timestamp":"2026-01-01T00:00:00.000Z","type":"user","provenance":"real_user","cwd":"/workspace","version":"0.21.11","message":{"role":"user","parts":[{"text":"serve A/B fixture turn"}]}}
{"uuid":"10000000-0000-4000-8000-000000000002","parentUuid":"10000000-0000-4000-8000-000000000001","sessionId":"00000000-0000-4000-8000-000000000000","timestamp":"2026-01-01T00:00:00.000Z","type":"assistant","provenance":"assistant_output","cwd":"/workspace","version":"0.21.11","model":"fixture-model","message":{"role":"model","parts":[{"text":"serve A/B fixture reply"}]},"usageMetadata":{"promptTokenCount":5,"candidatesTokenCount":3,"thoughtsTokenCount":0,"totalTokenCount":8,"cachedContentTokenCount":0}}
54 changes: 43 additions & 11 deletions .github/scripts/serve-ab-diff.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,15 @@ export function maskPath(path, patterns = DEFAULT_VOLATILE) {
return patterns.some((re) => re.test(path));
}

// The completion marker is OWNED by the drive script (the writer) and imported
// here rather than re-declared: two copies drift silently — each suite would
// keep testing against its own — and a drifted reader either flags every
// complete baseline as truncated or stops noticing truncated ones at all.
// Importing is side-effect-free; the drive's CLI body sits behind an
// `import.meta.url` guard.
export { DRIVE_COMPLETE_MARKER } from './serve-ab-drive.mjs';
import { DRIVE_COMPLETE_MARKER } from './serve-ab-drive.mjs';

export function typeOf(v) {
if (v === null) return 'null';
if (Array.isArray(v)) return 'array';
Expand Down Expand Up @@ -165,6 +174,17 @@ export function buildComment(sections, ctx = {}) {
out.push('— _Qwen Code · serve A/B_');
return out.join('\n') + '\n';
}
// Partial run: the base produced SOME captures and then stopped (a canary
// deviation, a daemon crash). The scenarios it never reached have no
// baseline, so they would render as "this PR adds these responses" — the same
// shape a genuinely new scenario produces. Disclose it rather than let the
// reader mistake a truncated baseline for a complete one.
if (ctx.baselineIncomplete) {
out.push(
'⚠️ _The PR-base drive did not finish, so its capture set is partial. Scenarios it never reached appear below as additions rather than as a before/after — treat those tables as unverified._',
);
out.push('');
}
if (ctx.removed?.length) {
out.push(
`⚠️ _Present in the base but absent from this PR: ${ctx.removed
Expand All @@ -187,11 +207,16 @@ export function buildComment(sections, ctx = {}) {
}

/**
* Read a capture dir's `<scenario>.json` files → `{ sections, baselineMissing }`.
* Each section diffs an after-capture against the same-named base file. When the
* base captures are ENTIRELY absent (a failed base build/drive) but head
* captures exist, `baselineMissing` is set so the caller reports "diff skipped"
* rather than misreporting every field as added. This is the function the CI
* Read a capture dir's `<scenario>.json` files →
* `{ sections, baselineMissing, baselineIncomplete, removed }`. Each section
* diffs an after-capture against the same-named base file.
*
* Two degraded baselines are distinguished, because both would otherwise read
* as an ordinary diff. `baselineMissing`: the base produced NO captures (a
* failed base build/drive), so nothing was compared. `baselineIncomplete`: the
* base drive started and stopped part-way, so the scenarios it never reached
* have no baseline and render as pure additions — indistinguishable, on the
* page, from a scenario this PR genuinely adds. This is the function the CI
* `comment` subcommand actually invokes, so it is exported + covered.
*/
export function diffCaptureDirs(beforeDir, afterDir) {
Expand All @@ -205,6 +230,10 @@ export function diffCaptureDirs(beforeDir, afterDir) {
const afterFiles = jsonFiles(afterDir).sort();
const beforeFiles = jsonFiles(beforeDir);
const baselineMissing = afterFiles.length > 0 && beforeFiles.length === 0;
const baselineIncomplete =
!baselineMissing &&
beforeFiles.length > 0 &&
!existsSync(join(beforeDir, DRIVE_COMPLETE_MARKER));
Comment thread
wenshao marked this conversation as resolved.
const afterSet = new Set(afterFiles);
// Scenarios present in the base but gone from the head — a removed or broken
// scenario would otherwise vanish silently and lower the "across N" count,
Expand All @@ -223,20 +252,23 @@ export function diffCaptureDirs(beforeDir, afterDir) {
const before = existsSync(beforePath) ? readJson(beforePath) : {};
return { scenario, changes: diffJson(before, after) };
});
return { sections, baselineMissing, removed };
return { sections, baselineMissing, baselineIncomplete, removed };
}

if (import.meta.url === pathToFileURL(process.argv[1] ?? '').href) {
const [cmd, ...rest] = process.argv.slice(2);
if (cmd === 'comment') {
const [beforeDir, afterDir, shortSha, bodyFile] = rest;
const { sections, baselineMissing, removed } = diffCaptureDirs(
beforeDir,
afterDir,
);
const { sections, baselineMissing, baselineIncomplete, removed } =
diffCaptureDirs(beforeDir, afterDir);
Comment thread
wenshao marked this conversation as resolved.
writeFileSync(
bodyFile,
buildComment(sections, { shortSha, baselineMissing, removed }),
buildComment(sections, {
shortSha,
baselineMissing,
baselineIncomplete,
removed,
}),
);
const total = baselineMissing
? 0
Expand Down
142 changes: 140 additions & 2 deletions .github/scripts/serve-ab-diff.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,15 @@
*/

import assert from 'node:assert/strict';
import { mkdtempSync, writeFileSync } from 'node:fs';
import { execFileSync } from 'node:child_process';
import { mkdtempSync, readFileSync, writeFileSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { dirname, join } from 'node:path';
import { fileURLToPath } from 'node:url';
import test from 'node:test';

import {
DRIVE_COMPLETE_MARKER,
buildComment,
diffCaptureDirs,
diffJson,
Expand Down Expand Up @@ -230,3 +233,138 @@ test('diffCaptureDirs: a base-only (removed) scenario is surfaced, not dropped',
/Present in the base but absent from this PR: `capabilities`/,
);
});

test('diffCaptureDirs: status-only change is a reported diff', () => {
const before = mkdtempSync(join(tmpdir(), 'sa-before-'));
const after = mkdtempSync(join(tmpdir(), 'sa-after-'));
// Same body, different status — invisible before `_status` was captured.
writeFileSync(
join(before, 'restore.json'),
JSON.stringify({ _status: 200, code: undefined }),
);
writeFileSync(join(after, 'restore.json'), JSON.stringify({ _status: 409 }));
const { sections } = diffCaptureDirs(before, after);
assert.deepEqual(sections[0].changes, [
{ path: '_status', kind: 'changed', before: 200, after: 409 },
]);
});

test('diffCaptureDirs: a scenario absent from the base reports as an addition', () => {
const before = mkdtempSync(join(tmpdir(), 'sa-before-'));
const after = mkdtempSync(join(tmpdir(), 'sa-after-'));
// New scenario: no base capture at all — every field reads as added.
writeFileSync(
join(after, 'new-scenario.json'),
JSON.stringify({ _status: 400, code: 'reserved_session_source' }),
);
writeFileSync(join(before, 'health.json'), JSON.stringify({ status: 'ok' }));
writeFileSync(join(after, 'health.json'), JSON.stringify({ status: 'ok' }));
const { sections } = diffCaptureDirs(before, after);
const added = sections.find((s) => s.scenario === 'new-scenario');
assert.ok(added.changes.some((c) => c.path === '_status'));
});

test('diffCaptureDirs: a base that never finished is reported as incomplete', () => {
const before = mkdtempSync(join(tmpdir(), 'sa-before-'));
const after = mkdtempSync(join(tmpdir(), 'sa-after-'));
// The base drive stopped after one scenario; the head captured two. Without
// the completion marker the second reads as "this PR adds this response".
writeFileSync(join(before, 'health.json'), JSON.stringify({ _status: 200 }));
writeFileSync(join(after, 'health.json'), JSON.stringify({ _status: 200 }));
writeFileSync(join(after, 'restore.json'), JSON.stringify({ _status: 409 }));
const partial = diffCaptureDirs(before, after);
assert.equal(partial.baselineIncomplete, true);
assert.equal(partial.baselineMissing, false);
assert.match(
buildComment(partial.sections, {
shortSha: 'x',
baselineIncomplete: partial.baselineIncomplete,
}),
/PR-base drive did not finish/,
);

// With the marker the same dirs are a complete baseline and say nothing.
writeFileSync(join(before, DRIVE_COMPLETE_MARKER), '');
const complete = diffCaptureDirs(before, after);
assert.equal(complete.baselineIncomplete, false);
assert.doesNotMatch(
buildComment(complete.sections, {
shortSha: 'x',
baselineIncomplete: complete.baselineIncomplete,
}),
/PR-base drive did not finish/,
);
});

test('diffCaptureDirs: an empty base stays "missing", not "incomplete"', () => {
const before = mkdtempSync(join(tmpdir(), 'sa-before-'));
const after = mkdtempSync(join(tmpdir(), 'sa-after-'));
writeFileSync(join(after, 'health.json'), JSON.stringify({ _status: 200 }));
const r = diffCaptureDirs(before, after);
assert.equal(r.baselineMissing, true);
assert.equal(r.baselineIncomplete, false);
});

test('the marker is not itself enumerated as a scenario', () => {
const before = mkdtempSync(join(tmpdir(), 'sa-before-'));
const after = mkdtempSync(join(tmpdir(), 'sa-after-'));
for (const d of [before, after]) {
writeFileSync(join(d, 'health.json'), JSON.stringify({ _status: 200 }));
writeFileSync(join(d, DRIVE_COMPLETE_MARKER), '');
}
const { sections } = diffCaptureDirs(before, after);
assert.deepEqual(
sections.map((s) => s.scenario),
['health'],
);
});

// The `comment` subcommand is the ONLY invocation path in CI, and every test
// above builds the buildComment ctx by hand — so the glue between
// diffCaptureDirs and buildComment (destructure → pass-through) is exercised by
// nothing. A dropped or misspelled flag there loses a degraded-baseline warning
// while the whole suite stays green.
const CLI = join(dirname(fileURLToPath(import.meta.url)), 'serve-ab-diff.mjs');
const runComment = (before, after) => {
const bodyFile = join(mkdtempSync(join(tmpdir(), 'sa-body-')), 'body.md');
execFileSync(
process.execPath,
[CLI, 'comment', before, after, 'abc1234', bodyFile],
{
stdio: 'pipe',
},
);
return readFileSync(bodyFile, 'utf8');
};

test('comment CLI: a marker-less baseline carries the truncation warning', () => {
const before = mkdtempSync(join(tmpdir(), 'sa-before-'));
const after = mkdtempSync(join(tmpdir(), 'sa-after-'));
writeFileSync(join(before, 'health.json'), JSON.stringify({ _status: 200 }));
writeFileSync(join(after, 'health.json'), JSON.stringify({ _status: 200 }));
writeFileSync(join(after, 'restore.json'), JSON.stringify({ _status: 409 }));
assert.match(runComment(before, after), /PR-base drive did not finish/);
});

test('comment CLI: a complete baseline carries no degraded-baseline warning', () => {
const before = mkdtempSync(join(tmpdir(), 'sa-before-'));
const after = mkdtempSync(join(tmpdir(), 'sa-after-'));
for (const d of [before, after]) {
writeFileSync(join(d, 'health.json'), JSON.stringify({ _status: 200 }));
writeFileSync(join(d, DRIVE_COMPLETE_MARKER), '');
}
const body = runComment(before, after);
assert.doesNotMatch(body, /PR-base drive did not finish/);
assert.doesNotMatch(body, /could not be built this run/);
assert.match(
body,
/No response changes against the PR base across 1 scenario/,
);
});

test('comment CLI: an empty baseline reports the diff as skipped', () => {
const before = mkdtempSync(join(tmpdir(), 'sa-before-'));
const after = mkdtempSync(join(tmpdir(), 'sa-after-'));
writeFileSync(join(after, 'health.json'), JSON.stringify({ _status: 200 }));
assert.match(runComment(before, after), /could not be built this run/);
});
Loading
Loading