-
Notifications
You must be signed in to change notification settings - Fork 3.1k
refactor(review): define each certification-bar atom exactly once #9257
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -797,6 +797,71 @@ describe('budget-gap disclosures — guarded, parsed, never punished', () => { | |
| ]); | ||
| }); | ||
|
|
||
| it('a whole-diff disclosure is silenced only by a compliant gap-free relaunch', () => { | ||
| // `gapsSuperseded`'s whole-diff branch: the superseding record must have | ||
| // OPENED the key's brief and be gap-free itself. Neither conjunct was | ||
| // reached by any test — a revert of the branch shipped green. | ||
| const p = plan3a(); | ||
| const d = promptRecordDir(p); | ||
| mkdirSync(d, { recursive: true }); | ||
| const brief = briefPath(p, 'audit-w'); | ||
| writeFileSync(brief, 'The audit-w brief.'); | ||
|
Comment on lines
+800
to
+808
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] R4-1: The identical ~9-10-line whole-diff auditor seeding block (mkdir the record dir, write the brief via function briefedWholeDiffLaunch(p: string): string {
const d = promptRecordDir(p);
mkdirSync(d, { recursive: true });
const brief = briefPath(p, 'audit-w');
writeFileSync(brief, 'The audit-w brief.');
const prompt =
'You are review agent `audit-w`.\n' +
`read_file(file_path="${brief}")\n` +
wholeDiff();
writeFileSync(join(d, 'audit-w.txt'), prompt);
return prompt;
}(the recovery test prepends its 中文说明[Suggestion] 本 PR 新增的四个测试逐字粘贴了同一段约 9-10 行的 whole-diff 审计员播种块(创建记录目录、经 — qwen3.8-max via Qwen Code /review (v0.21.13) |
||
| const prompt = | ||
| 'You are review agent `audit-w`.\n' + | ||
| `read_file(file_path="${brief}")\n` + | ||
| wholeDiff(); | ||
| writeFileSync(join(d, 'audit-w.txt'), prompt); | ||
| transcript('g1', prompt, { | ||
| calls: 3, | ||
| text: 'Walked the diff.\nBudget gap: the reconnect state machine', | ||
| }); | ||
| // A gap-free relaunch that opened the brief silences the disclosure. | ||
| transcript('g2', prompt, { calls: 3 }); | ||
| expect(coverageFromTranscripts(p, ENV).budgetGaps).toEqual([]); | ||
|
Comment on lines
+819
to
+820
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] R3-3: No test supplies a NON-RETURNED superseding record to Witness (probe): Negative case to pin the conjunct (e.g. in this test): // A gap-free relaunch that DIED mid-flight must NOT silence the disclosure:
transcript('g2', prompt, { calls: 3, text: '' });
expect(coverageFromTranscripts(p, ENV).budgetGaps).toHaveLength(1);
// and/or in the recovery test: transcript('w2', prompt, { calls: 3, text: '' })
// expecting recoveredAgents to stay 1.中文说明[Suggestion] 没有任何测试向 修法:补一个负例钉住该联言项(示例代码见英文部分)。 — qwen3.8-max via Qwen Code /review (v0.21.12) |
||
| }); | ||
|
|
||
| it('a relaunch that never opened the brief cannot silence the disclosure', () => { | ||
| const p = plan3a(); | ||
| const d = promptRecordDir(p); | ||
| mkdirSync(d, { recursive: true }); | ||
| const brief = briefPath(p, 'audit-w'); | ||
| writeFileSync(brief, 'The audit-w brief.'); | ||
| const prompt = | ||
| 'You are review agent `audit-w`.\n' + | ||
| `read_file(file_path="${brief}")\n` + | ||
| wholeDiff(); | ||
| writeFileSync(join(d, 'audit-w.txt'), prompt); | ||
| transcript('g1', prompt, { | ||
| calls: 3, | ||
| text: 'Walked the diff.\nBudget gap: the reconnect state machine', | ||
| }); | ||
| transcript('g2', prompt, { calls: 3, opens: [] }); | ||
| expect(coverageFromTranscripts(p, ENV).budgetGaps).toHaveLength(1); | ||
| }); | ||
|
|
||
| it('a relaunch still disclosing gaps of its own cannot silence anything', () => { | ||
| const p = plan3a(); | ||
| const d = promptRecordDir(p); | ||
| mkdirSync(d, { recursive: true }); | ||
| const brief = briefPath(p, 'audit-w'); | ||
| writeFileSync(brief, 'The audit-w brief.'); | ||
| const prompt = | ||
| 'You are review agent `audit-w`.\n' + | ||
| `read_file(file_path="${brief}")\n` + | ||
| wholeDiff(); | ||
| writeFileSync(join(d, 'audit-w.txt'), prompt); | ||
| transcript('g1', prompt, { | ||
| calls: 3, | ||
| text: 'Walked the diff.\nBudget gap: the reconnect state machine', | ||
| }); | ||
| transcript('g2', prompt, { | ||
| calls: 3, | ||
| text: 'Walked again.\nBudget gap: the remaining call sites', | ||
| }); | ||
| // Two live disclosures, neither silenced by the other. | ||
| expect(coverageFromTranscripts(p, ENV).budgetGaps).toHaveLength(2); | ||
| }); | ||
|
|
||
| it('a disclosure costs no coverage credit — the gate must not punish it', () => { | ||
| // An earlier draft narrowed a disclosing agent's credit to its ranged | ||
| // reads. `rangeOf` records only reads carrying a positive `limit`, so | ||
|
|
@@ -2291,6 +2356,7 @@ describe('coverage — a resumed run credits the prior attempt through the ledge | |
| // The run ledger `fetch-pr` writes: S0 is the interrupted attempt, S1 the | ||
| // resumed continuation this suite's ENV runs as. Entries carry a current | ||
| // atMs, which sits inside the epoch fence of the backdated plan. | ||
| let ledgerNowMs = 0; | ||
| function ledger(planPath: string, ...ids: string[]): void { | ||
| const d = promptRecordDir(planPath); | ||
| mkdirSync(d, { recursive: true }); | ||
|
|
@@ -2299,6 +2365,7 @@ describe('coverage — a resumed run credits the prior attempt through the ledge | |
| // evidence at all. The current attempt is stamped last, since each | ||
| // attempt's window closes when the next one opened. | ||
| const nowMs = Date.now(); | ||
| ledgerNowMs = nowMs; | ||
| ids.forEach((id, i) => | ||
| appendRunSession( | ||
| planPath, | ||
|
|
@@ -2325,6 +2392,10 @@ describe('coverage — a resumed run credits the prior attempt through the ledge | |
| ), | ||
| ); | ||
| rmSync(from, { force: true }); | ||
| if (ledgerNowMs > 0) { | ||
| const at = new Date(ledgerNowMs); | ||
| utimesSync(to, at, at); | ||
| } | ||
| } | ||
|
|
||
| it('passes 3D on work the interrupted attempt completed, and discloses it', () => { | ||
|
|
@@ -2391,19 +2462,26 @@ describe('verificationGaps — a resumed run reads the prior attempt', () => { | |
| function moveToSession(id: string, session: string): void { | ||
| mkdirSync(join(dir, 'subagents', session), { recursive: true }); | ||
| const from = join(dir, 'subagents', 'S1', `agent-${id}.jsonl`); | ||
| const to = join(dir, 'subagents', session, `agent-${id}.jsonl`); | ||
| writeFileSync( | ||
| join(dir, 'subagents', session, `agent-${id}.jsonl`), | ||
| to, | ||
| readFileSync(from, 'utf8').replaceAll( | ||
| '"sessionId":"S1"', | ||
| `"sessionId":"${session}"`, | ||
| ), | ||
| ); | ||
| rmSync(from, { force: true }); | ||
| if (ledgerNowMs > 0) { | ||
| const at = new Date(ledgerNowMs); | ||
| utimesSync(to, at, at); | ||
| } | ||
| } | ||
|
|
||
| /** The ledger `fetch-pr` writes, through the real writers. */ | ||
| let ledgerNowMs = 0; | ||
| function ledger(planPath: string, ...ids: string[]): void { | ||
| const nowMs = Date.now(); | ||
| ledgerNowMs = nowMs; | ||
| ids.forEach((id, i) => | ||
| appendRunSession( | ||
| planPath, | ||
|
|
@@ -2518,6 +2596,7 @@ describe('verificationGaps — a resumed run reads the prior attempt', () => { | |
| }); | ||
|
|
||
| describe('coverage — a stale Uncoverable declaration cannot cap live coverage', () => { | ||
| let ledgerNowMs = 0; | ||
| function ledger(planPath: string, ...ids: string[]): void { | ||
| const d = promptRecordDir(planPath); | ||
| mkdirSync(d, { recursive: true }); | ||
|
|
@@ -2526,6 +2605,7 @@ describe('coverage — a stale Uncoverable declaration cannot cap live coverage' | |
| // evidence at all. The current attempt is stamped last, since each | ||
| // attempt's window closes when the next one opened. | ||
| const nowMs = Date.now(); | ||
| ledgerNowMs = nowMs; | ||
| ids.forEach((id, i) => | ||
| appendRunSession( | ||
| planPath, | ||
|
|
@@ -2551,6 +2631,10 @@ describe('coverage — a stale Uncoverable declaration cannot cap live coverage' | |
| ), | ||
| ); | ||
| rmSync(from, { force: true }); | ||
| if (ledgerNowMs > 0) { | ||
| const at = new Date(ledgerNowMs); | ||
| utimesSync(to, at, at); | ||
| } | ||
| } | ||
|
|
||
| it('a superseded prior-attempt declaration does not delete the chunk it covers', () => { | ||
|
|
@@ -2630,6 +2714,33 @@ describe('coverage — a stale Uncoverable declaration cannot cap live coverage' | |
| expect(r.recoveredAgents).toBe(0); | ||
| }); | ||
|
|
||
| it('a whole-diff recovery is superseded only by a relaunch that opened the brief', () => { | ||
| // `keySatisfied` — the chunk-less arm of the supersession predicates — | ||
| // was reached by no test: its brief requirement could be deleted (or | ||
| // left dangling) with the suite green. The deciding conjunct is the | ||
| // relaunch's brief read, so both arms pin it. | ||
| const p = plan(); | ||
| ledger(p, 'S0', 'S1'); | ||
| const d = promptRecordDir(p); | ||
| mkdirSync(d, { recursive: true }); | ||
| const brief = briefPath(p, 'audit-w'); | ||
| writeFileSync(brief, 'The audit-w brief.'); | ||
| const prompt = | ||
| 'You are review agent `audit-w`.\n' + | ||
| `read_file(file_path="${brief}")\n` + | ||
| wholeDiff(); | ||
| writeFileSync(join(d, 'audit-w.txt'), prompt); | ||
| transcript('w1', prompt, { calls: 3 }); | ||
| moveToSession('w1', 'S0'); | ||
|
wenshao marked this conversation as resolved.
wenshao marked this conversation as resolved.
|
||
| // The current relaunch never opened its brief: no supersession, the | ||
| // prior work still counts as recovered. | ||
| transcript('w2', prompt, { calls: 3, opens: [] }); | ||
| expect(coverageFromTranscripts(p, ENV).recoveredAgents).toBe(1); | ||
| // A compliant relaunch supersedes it. | ||
| transcript('w3', prompt, { calls: 3 }); | ||
| expect(coverageFromTranscripts(p, ENV).recoveredAgents).toBe(0); | ||
| }); | ||
|
|
||
| it('does NOT credit a prior agent whose text is progress, not a return', () => { | ||
| // `finalText` keeps the last non-empty assistant text, and agents narrate | ||
| // between tool calls — so an agent that said "reading the diff now" and | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,176 @@ | ||
| /** | ||
| * @license | ||
| * Copyright 2026 Qwen Team | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| import { describe, expect, it } from 'vitest'; | ||
| import { | ||
| chunkOfKey, | ||
| declaresOwnUncoverable, | ||
| openedBrief, | ||
| readBrief, | ||
| readFindingsPointer, | ||
| } from './certification.js'; | ||
| import { briefPath } from './prompt-record.js'; | ||
| import type { AgentRecord } from './transcripts.js'; | ||
|
|
||
| const PLAN = '/tmp/certification-test/plan.json'; | ||
|
|
||
| function rec(over: Partial<AgentRecord>): AgentRecord { | ||
| return { | ||
| agentId: 'a1', | ||
| agentName: 'agent-a1', | ||
| launchPrompt: '', | ||
| successfulToolCalls: 0, | ||
| diffToolCalls: 0, | ||
| diffReads: [], | ||
| successfulCallArgs: [], | ||
| successfulReadFileArgs: [], | ||
| recordedSession: 's1', | ||
| finalText: '', | ||
| returned: true, | ||
| mtimeMs: 0, | ||
| ...over, | ||
| }; | ||
| } | ||
|
|
||
| describe('chunkOfKey', () => { | ||
| it('parses the bare chunk key', () => { | ||
| expect(chunkOfKey('chunk-13')).toBe(13); | ||
| }); | ||
|
|
||
| it('parses the per-chunk audit key from its --chunk-N segment', () => { | ||
| expect(chunkOfKey('reverse-audit--chunk-13--round-2--0a1b2c')).toBe(13); | ||
| }); | ||
|
|
||
| it('parses a trailing --chunk-N segment', () => { | ||
| expect(chunkOfKey('reverse-audit--chunk-7')).toBe(7); | ||
| }); | ||
|
|
||
| it('assigns no chunk to chunk-free keys', () => { | ||
| expect(chunkOfKey('verify--0a1b2c')).toBeNull(); | ||
| expect(chunkOfKey('reverse-audit--round-2--0a1b2c')).toBeNull(); | ||
| }); | ||
|
|
||
| it('does not mistake a digest containing "chunk" for an assignment', () => { | ||
| // Only the exact `--chunk-<digits>` segment form assigns; an embedded | ||
| // word does not. | ||
| expect(chunkOfKey('verify--chunky')).toBeNull(); | ||
| }); | ||
| }); | ||
|
|
||
| describe('declaresOwnUncoverable', () => { | ||
| const declaring = 'Uncoverable: chunk 3 — binary payload, no readable lines'; | ||
|
|
||
| it('vetoes the record whose own return declares its own chunk', () => { | ||
| expect(declaresOwnUncoverable(rec({ finalText: declaring }), 3)).toBe(true); | ||
| }); | ||
|
wenshao marked this conversation as resolved.
|
||
|
|
||
| it('does not veto a declaration about a DIFFERENT chunk', () => { | ||
| expect(declaresOwnUncoverable(rec({ finalText: declaring }), 4)).toBe( | ||
| false, | ||
| ); | ||
| }); | ||
|
|
||
| it('never vetoes a chunk-less record — a quotation is not a declaration', () => { | ||
| // The whole-diff auditor quoting the evidence it audited. | ||
| expect(declaresOwnUncoverable(rec({ finalText: declaring }), null)).toBe( | ||
| false, | ||
| ); | ||
| }); | ||
|
|
||
| it('vetoes a declaration preceded by prose on an earlier line (pins /m)', () => { | ||
| expect( | ||
| declaresOwnUncoverable( | ||
| rec({ finalText: `Read what I could.\n${declaring}` }), | ||
| 3, | ||
| ), | ||
| ).toBe(true); | ||
| }); | ||
|
|
||
| it('matches a lowercase marker (pins /i)', () => { | ||
| expect( | ||
| declaresOwnUncoverable( | ||
| rec({ finalText: 'uncoverable: chunk 3 — binary payload' }), | ||
| 3, | ||
| ), | ||
| ).toBe(true); | ||
| }); | ||
|
|
||
| it('ignores a declaration indented into prose', () => { | ||
| // The `^` anchor with only whitespace allowed before the marker: a | ||
| // quoted line inside a bullet does not match. | ||
| const quoted = `- the agent said "${declaring}" which I verified`; | ||
| expect(declaresOwnUncoverable(rec({ finalText: quoted }), 3)).toBe(false); | ||
| }); | ||
|
wenshao marked this conversation as resolved.
|
||
| }); | ||
|
|
||
| describe('openedBrief / readBrief', () => { | ||
| const key = 'chunk-2'; | ||
| const needle = JSON.stringify(briefPath(PLAN, key)); | ||
| const arg = `{"absolute_path":${needle}}`; | ||
|
|
||
| it('credits any successful tool whose args name the exact brief path', () => { | ||
| const r = rec({ successfulCallArgs: [arg] }); | ||
| expect(openedBrief(r, PLAN, key)).toBe(true); | ||
| }); | ||
|
wenshao marked this conversation as resolved.
wenshao marked this conversation as resolved.
|
||
|
|
||
| it('does not credit a record that made zero successful tool calls', () => { | ||
| // `[].every(...)` is true: the existential quantifier needs its own | ||
| // negative, like its two sibling atoms already have. | ||
| expect(openedBrief(rec({}), PLAN, key)).toBe(false); | ||
| }); | ||
|
|
||
| it('does not credit a sibling file sharing the prefix', () => { | ||
| const bak = JSON.stringify(`${briefPath(PLAN, key)}.bak`); | ||
| const r = rec({ successfulCallArgs: [`{"absolute_path":${bak}}`] }); | ||
| expect(openedBrief(r, PLAN, key)).toBe(false); | ||
| }); | ||
|
wenshao marked this conversation as resolved.
|
||
|
|
||
| it('does not credit another key’s brief', () => { | ||
| const r = rec({ successfulCallArgs: [arg] }); | ||
| expect(openedBrief(r, PLAN, 'chunk-3')).toBe(false); | ||
| }); | ||
|
|
||
| it('readBrief does not credit a sibling file sharing the prefix', () => { | ||
| const bak = JSON.stringify(`${briefPath(PLAN, key)}.bak`); | ||
| const r = rec({ successfulReadFileArgs: [`{"absolute_path":${bak}}`] }); | ||
| expect(readBrief(r, PLAN, key)).toBe(false); | ||
| }); | ||
|
|
||
| it('readBrief requires a successful read_file, not a mention', () => { | ||
| // A grep whose args contain the path opened nothing. | ||
| const r = rec({ successfulCallArgs: [arg], successfulReadFileArgs: [] }); | ||
|
wenshao marked this conversation as resolved.
|
||
| expect(openedBrief(r, PLAN, key)).toBe(true); | ||
| expect(readBrief(r, PLAN, key)).toBe(false); | ||
| const read = rec({ successfulReadFileArgs: [arg] }); | ||
| expect(readBrief(read, PLAN, key)).toBe(true); | ||
| }); | ||
| }); | ||
|
|
||
| describe('readFindingsPointer', () => { | ||
| const pointer = | ||
| '/tmp/certification-test/plan.json-prompts/round-2.findings.md'; | ||
| const arg = `{"absolute_path":${JSON.stringify(pointer)}}`; | ||
|
|
||
| it('owes nothing when the prompt names no pointer', () => { | ||
| expect(readFindingsPointer(rec({}), null)).toBe(true); | ||
| }); | ||
|
|
||
| it('does not credit a sibling of the pointer', () => { | ||
| const bak = JSON.stringify(`${pointer}.bak`); | ||
| const r = rec({ successfulReadFileArgs: [`{"absolute_path":${bak}}`] }); | ||
| expect(readFindingsPointer(r, pointer)).toBe(false); | ||
| }); | ||
|
|
||
| it('requires a successful read_file of the exact pointer', () => { | ||
| expect( | ||
| readFindingsPointer(rec({ successfulReadFileArgs: [arg] }), pointer), | ||
| ).toBe(true); | ||
| // Named by a listing, read by nothing. | ||
| expect( | ||
| readFindingsPointer(rec({ successfulCallArgs: [arg] }), pointer), | ||
| ).toBe(false); | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Suggestion] R4-1: The identical ~10-line whole-diff auditor seeding block (mkdir the record dir, write the brief via
briefPath(p, 'audit-w'), compose the launch prompt, writeaudit-w.txt) is pasted verbatim into all four tests this diff adds — copies at lines 807-813, 827-833, 846-852 and 2707-2713 — and the first three additionally repeat the identicaltranscript('g1', prompt, …)line. — Failure scenario: the block hard-codes the recorded shape of a whole-diff agent (launch-prompt text, brief path, record filename); any change to that shape — the exact drift class this PR exists to eliminate — must be made four times in lockstep, and because each fixture is self-consistent by construction a stale copy does not fail loudly — it keeps asserting against a shape that no longer models production.Witness: grep enumeration at HEAD — 16
audit-whits = 4 byte-identical seeding blocks; no helper exists, while this file's established convention is extracted fixtures (plan3a,wholeDiff,satisfyRoster).Suggested fix: extract one helper next to
plan3a()/transcript()and call it from all four tests:中文说明
[Suggestion] 本 diff 新增的四个测试原样粘贴了同一段约 10 行的 whole-diff 审计员播种块(mkdir 记录目录、经
briefPath(p, 'audit-w')写 brief、拼装启动 prompt、写audit-w.txt)——副本位于 807-813、827-833、846-852 与 2707-2713 行——且前三个测试还各自重复了同一行transcript('g1', prompt, …)。— 失败场景:该块硬编码了 whole-diff agent 的记录形状(启动 prompt 文本、brief 路径、记录文件名);任何对该形状的调整——正是本 PR 要消灭的漂移类别——都必须四处同步修改,而每个夹具自身构造上自洽,过期的副本不会醒目地失败——它会继续对着一个不再代表生产形状的夹具做断言。证据:HEAD 处 grep 枚举——16 处
audit-w命中 = 4 个逐字节相同的播种块;该文件的既有约定恰是抽取夹具(plan3a、wholeDiff、satisfyRoster),此处却没有 helper。建议修法:在
plan3a()/transcript()旁抽取一个 helper(代码见英文部分),四个测试共用。— qwen3.8-max via Qwen Code /review (v0.21.12)