From 4cbed21ff3d23fcf43c5d4f9f983cbe254aa98c1 Mon Sep 17 00:00:00 2001 From: alexion Date: Fri, 17 Jul 2026 09:50:38 -0400 Subject: [PATCH] feat: persist agent report on read result records (task 0032) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Retain the agent's final report on the benchmark result record for read tasks, so a failed read is diagnosable directly from the stored record instead of only carrying an opaque `incorrect` tag. The runner resolves the scoring spec once and records `run.finalReport` when the spec is a read; mutation records omit the field entirely. The sample store needs no change — it serializes whatever record it is handed. This is the prerequisite for confirming the read-open-issue-count failure from real report text before the state-aware count-line change (task 0033). --- .claude/spec/read-tier-accuracy.md | 78 +++++++++++++ .../0032-bench-read-report-persistence.md | 26 +++++ bench/result.ts | 9 ++ bench/runner.test.ts | 107 ++++++++++++++++++ bench/runner.ts | 24 +++- bench/store.test.ts | 20 ++++ 6 files changed, 258 insertions(+), 6 deletions(-) create mode 100644 .claude/spec/read-tier-accuracy.md create mode 100644 .claude/tasks/0032-bench-read-report-persistence.md diff --git a/.claude/spec/read-tier-accuracy.md b/.claude/spec/read-tier-accuracy.md new file mode 100644 index 0000000..7f79b71 --- /dev/null +++ b/.claude/spec/read-tier-accuracy.md @@ -0,0 +1,78 @@ +# Read-tier accuracy + +## Problem Statement + +The benchmark ranks gitea-axi first on both cost metrics — lowest cost-equivalent tokens and lowest imputed cost of the four arms — but second on accuracy, 95% against gitea-mcp's 97%. +That entire two-point deficit is a single task: `read-open-issue-count`, which gitea-axi fails on all three trials while gitea-mcp passes one of three. +The read tier is the weakest tier for every arm, and on it gitea-axi spends more turns and more output than gitea-mcp yet scores lower, so agents are working harder to answer count questions and still getting them wrong. + +Two things stand in the way of closing this gap. +First, the `issue list` summary reports `count: N of M total` and never names the state it filtered on, so an agent asking "how many issues are open?" has to infer the answer rather than read it off the summary line. +Second, the benchmark records only tokens and a `failure: "incorrect"` tag for a failed read — never the agent's actual report — so a maintainer cannot tell whether the agent reported a wrong number or reported the right number in wording the checker's phrase list did not accept. +Without the report text the root cause of the 3/3 failure cannot be confirmed, so the product fix would be a guess. + +## Solution + +Make list count summaries self-answering, and make read failures diagnosable. + +For the product, the count line of a filtered list names the state it counted. +When a maintainer or an agent runs `issue list` on the default open filter, the summary states that the count is a count of open issues, so the answer to "how many are open?" is present in the summary rather than only inferable from each row's state field. + +For the harness, every result record carries the agent's final report for read tasks. +A failed read is then diagnosable directly from the stored record: the maintainer can see the exact text the agent submitted and tell a wrong count apart from a right count phrased in words the checker did not list. +This is the prerequisite that turns the `read-open-issue-count` failure from an opaque tag into evidence, and it is the data that decides whether the checker's accepted-phrasing list is later too strict. + +## User Stories + +1. As an agent answering a count question, I want the list summary to state which state it counted, so that I can report "5 open issues" straight from the summary line without inferring it from the rows. +2. As a maintainer reading `issue list` output, I want the count line to say what it counted, so that a bare `count: 5` is never ambiguous about whether those five are open, closed, or all issues. +3. As a maintainer auditing a failed read trial, I want the stored result record to include the agent's final report, so that I can see exactly what the agent said instead of only that it was "incorrect". +4. As a maintainer deciding whether the read checker is too strict, I want the persisted reports across trials, so that I can judge from evidence whether correct answers are being rejected on wording rather than substance. +5. As a maintainer re-running the benchmark after the count-line change, I want `read-open-issue-count` to move from a consistent failure toward a pass, so that gitea-axi's accuracy stops trailing on the one task that accounts for the whole gap. + +## Implementation Decisions + +- Two modules change: the `issue list` command in the product, and the benchmark runner's result-record assembly in the harness. + They serve one goal but are testable at different seams and can land independently, with the harness change first so the product fix can be confirmed against real report text. + +- **State-aware count line (product).** + The list command composes the state into its own count line; the generic render helper that formats count lines stays generic. + The command already resolves the effective state filter (defaulting to open), so it passes that filter descriptor into the summary rather than the helper learning about issue state. + This keeps a single render seam and lets each list command opt in on its own terms; `pr list`, `search`, and `dashboard` are unaffected unless they choose to opt in the same way. + The existing count-line invariants are preserved: a total is always reported, and the bare `count: N` form must never appear. + +- **Report persistence (harness).** + The agent's final report is already in hand at the point the runner scores a read task, so persisting it is threading that value into the record the runner assembles rather than plumbing it up from a new source. + The report is added to the result record for read tasks; the store serializes whatever record it is handed, so it needs no change of its own. + Mutation tasks are scored by diffing repository state and have no agent report to record, so the field is populated for read tasks and absent otherwise. + +- **Ordering.** + Persist the report first and re-run enough of the read tier to capture real reports, then confirm from those reports whether the failure is a wrong count or a rejected phrasing before finalizing the count-line wording. + The count-line wording should be chosen so that an agent quoting the summary lands on an answer the read checker already accepts. + +## Testing Decisions + +- A good test here asserts external behavior: the text a user or agent sees on stdout, and the shape of the record the harness writes — not the internal composition of the count string. + +- **Count line** is tested at the command's behavioral seam — the fixture-server CLI harness that runs the real command against a stubbed Gitea and asserts on rendered stdout. + Prior art is the existing `issue list` command test, which already asserts exact count-line strings such as `count: 3 of 17 total` and guards that the bare `count: N` form never appears. + New assertions extend that file: the count line names the state for the default open filter and for an explicit state filter, and the existing total-and-cap invariants still hold. + +- **Report persistence** is tested at the runner's record-assembly seam, where the existing runner test already drives a cell to a recorded outcome and asserts on the produced record. + A completed read cell yields a record carrying the agent's final report; a mutation cell yields a record without one. + +## Out of Scope + +- Trimming gitea-axi's input and cache-read footprint. + gitea-axi replays the largest context of the efficient arms and wins cost only on the 5×-weighted output component, so reducing its input footprint would harden the cost lead — but it is a separate optimization touching many commands and is not part of closing the accuracy gap. + +- Relaxing the read checker's accepted-phrasing list. + Whether the checker is too strict is a benchmark-validity decision that must be made from the persisted report evidence this spec produces, not pre-judged; changing accepted phrasings before seeing the reports risks tuning the benchmark to the tool rather than fixing the tool. + +- Any change to how mutation tasks are scored, and any change to the cost-equivalent-token metric or its weighting. + +## Further Notes + +The two leaders trade a narrow accuracy edge for a clear cost lead, and this one task is the whole of that edge, so the count-line change is the single highest-leverage move on the accuracy axis. +The report-persistence change also has standing value beyond this task: it makes every future read failure diagnosable rather than opaque, which is the harness's current blind spot and the reason the root cause could not be confirmed from the existing records. +The state-explicit summary is squarely on gitea-axi's central thesis — an agent-ergonomic, low-token interface — because it hands the answer to a count question directly on the summary line instead of forcing the agent into extra turns to derive it. diff --git a/.claude/tasks/0032-bench-read-report-persistence.md b/.claude/tasks/0032-bench-read-report-persistence.md new file mode 100644 index 0000000..6684007 --- /dev/null +++ b/.claude/tasks/0032-bench-read-report-persistence.md @@ -0,0 +1,26 @@ +--- +spec: read-tier-accuracy +--- + +## What to build + +Persist the agent's final report on the benchmark result record for read tasks, so a failed read is diagnosable directly from the stored record instead of only carrying an opaque `incorrect` tag. + +The report is already in hand where the runner scores a read task — threading it into the assembled record is all that is required; the sample store serializes whatever record it is handed and needs no change of its own. + +Mutation tasks are scored by diffing repository state and have no agent report, so the field is populated for read tasks and absent otherwise. + +## Acceptance criteria + +- [x] The result record carries the agent's final report for read tasks. +- [x] The field is absent on records for mutation tasks. +- [x] A completed read cell produces a record whose report is the agent's final report; a mutation cell produces a record without one — asserted at the runner's record-assembly seam. +- [x] The sample store round-trips the report-bearing record without any change to the store itself. +- [x] Existing runner and store tests still pass. + +## Implementation Notes + +- `ResultRecord` gained an optional `report?: string`. The runner resolves the scoring spec once in `runCell` and records `run.finalReport` only when `spec.kind === "read"`, so the field is populated for read tasks and absent otherwise. `makeRecord` conditionally spreads the key (`...(report !== undefined ? { report } : {})`) so a mutation (or hung) record genuinely omits it rather than carrying `report: undefined`; verified by `expect(sample).not.toHaveProperty("report")` after the JSON round-trip. +- `scoreRun` was refactored to take a pre-resolved `ScoringSpec` instead of a `BenchTask`. This is marginally more than "thread the value into the record," but it is the minimal clean way to branch on `spec.kind` in `runCell` without calling `task.scoringSpec(owner)` twice. +- The store needed no change, as the spec predicted: it serializes whatever record it is handed, so the round-trip test passed on first run. +- Two review findings were left as deliberate judgement calls (both non-blocking baseline smells, no documented-standard breach): the `makeRecord` positional parameter list (extended by one param in the module's pre-existing positional style, kept consistent with surrounding code rather than refactored to an options object), and the two `spec.kind` branches a few lines apart in `runCell` and `scoreRun` (they select different things — the report vs. the scorer — and read clearer inline). diff --git a/bench/result.ts b/bench/result.ts index 323678e..6327320 100644 --- a/bench/result.ts +++ b/bench/result.ts @@ -72,6 +72,15 @@ export interface ResultRecord { /** Pass/fail outcome with a failure tag. */ outcome: Outcome; + + /** + * The agent's final report, retained for read tasks so a failed read is + * diagnosable directly from the record — the exact text the agent submitted, + * distinguishing a wrong answer from a right one phrased in words the checker's + * accepted-phrasing list did not match. Absent for mutation tasks, which are + * scored by diffing repository state and have no agent report to record. + */ + report?: string; } /** diff --git a/bench/runner.test.ts b/bench/runner.test.ts index 9f47520..4747354 100644 --- a/bench/runner.test.ts +++ b/bench/runner.test.ts @@ -6,6 +6,7 @@ import type { RepoState } from "./scoring-spec.js"; import type { BenchAccess, RepoCoords } from "./seed.js"; import { groundTruth } from "./seed-plan.js"; import { createSampleStore } from "./store.js"; +import type { BenchTask } from "./task.js"; import { SAMPLE_TASK } from "./task.js"; import type { AgentDriver, BenchHost } from "./runner.js"; import { runCell } from "./runner.js"; @@ -397,4 +398,110 @@ describe("runCell", () => { if (sample === undefined) return; expect(sample.outcome).toEqual({ pass: false, failure: "incorrect" }); }); + + // Behavior: a completed read cell produces a record whose report is the agent's + // final report (benchmark-harness spec, record-assembly seam). A read task is + // scored by matching the agent's final report against required facts; the + // recorded ResultRecord must carry that final report in its `report` field so a + // read is diagnosable from the record alone. Here an inline read task's fact is + // satisfied by the driver's finalReport (it contains "5 open"), so the cell + // completes as a pass; the recorded sample's `report` must equal the exact + // planted finalReport literal — an independent literal, read back through the + // store, not recomputed from runner.ts. + it("records the agent's final report on a completed read cell", async () => { + const READ_TASK: BenchTask = { + id: "count-open-issues", + tier: "read", + intent: "How many issues are open?", + scoringSpec: () => ({ + kind: "read", + facts: [{ description: "open-issue count", anyOf: ["5 open"] }], + }), + }; + + // The independent literal we plant as the driver's final report. It contains + // the fact's rendering ("5 open"), so the read scorer scores it a pass. + const FINAL_REPORT = "There are 5 open issues."; + + const readDriver: AgentDriver = { + async run() { + return { + tokens: { freshInput: 50, cacheCreation: 0, cacheRead: 0, output: 10 }, + turns: 2, + imputedCostUsd: 0.03, + transcript: [{ kind: "mcp", server: "gitea-mcp", tool: "list_repo_issues" }], + finalReport: FINAL_REPORT, + stoppedByTurnCap: false, + }; + }, + }; + + const { host } = createFakeHost(); + const store = createSampleStore(storeRoot); + + const outcome = await runCell({ + arm: "gitea-mcp", + task: READ_TASK, + trial: 1, + access: ACCESS, + host, + driver: readDriver, + store, + bounds: { turnCap: 10, wallClockMs: 60_000 }, + build: { binRoot }, + }); + + expect(outcome.kind).toBe("recorded"); + if (outcome.kind !== "recorded") return; + + const samples = store.read({ arm: "gitea-mcp", taskId: READ_TASK.id }); + expect(samples).toHaveLength(1); + const [sample] = samples; + expect(sample).toBeDefined(); + if (sample === undefined) return; + + // A completed read cell: the checker scored the final report a pass. + expect(sample.outcome).toEqual({ pass: true }); + // The record carries the agent's final report verbatim. + expect(sample.report).toBe(FINAL_REPORT); + }); + + // Behavior: a completed mutation cell produces a record WITHOUT an agent report + // (benchmark-harness spec, record-assembly seam). A mutation task is scored by + // diffing repository state, not by reading the agent's words, so the recorded + // ResultRecord must carry no `report` field at all — absent, not present-but- + // undefined. SAMPLE_TASK is a mutation task; paired with the passing fake host + // and driver it drives a completed mutation PASS. The absence is the independent + // source of truth (the acceptance criterion), read back through the store after + // the JSON round-trip so an absent field is genuinely not a key. + it("records no agent report on a completed mutation cell", async () => { + const { host } = createFakeHost(); + const store = createSampleStore(storeRoot); + + const outcome = await runCell({ + arm: "gitea-mcp", + task: SAMPLE_TASK, + trial: 1, + access: ACCESS, + host, + driver, + store, + bounds: { turnCap: 10, wallClockMs: 60_000 }, + build: { binRoot }, + }); + + expect(outcome.kind).toBe("recorded"); + if (outcome.kind !== "recorded") return; + + const samples = store.read({ arm: "gitea-mcp", taskId: SAMPLE_TASK.id }); + expect(samples).toHaveLength(1); + const [sample] = samples; + expect(sample).toBeDefined(); + if (sample === undefined) return; + + // A completed mutation cell: the state diff scored a pass. + expect(sample.outcome).toEqual({ pass: true }); + // A mutation record carries no agent report — the key is absent entirely. + expect(sample).not.toHaveProperty("report"); + }); }); diff --git a/bench/runner.ts b/bench/runner.ts index afa3096..25caa3f 100644 --- a/bench/runner.ts +++ b/bench/runner.ts @@ -15,7 +15,7 @@ import { buildArm, type ArmDefinition, type BuildArmOptions, type SharedContext import { auditTranscript, type ToolUse } from "./audit.js"; import { score } from "./checker.js"; import type { Arm, Outcome, ResultRecord, TokenComponents } from "./result.js"; -import type { RepoState } from "./scoring-spec.js"; +import type { RepoState, ScoringSpec } from "./scoring-spec.js"; import type { BenchAccess, RepoCoords } from "./seed.js"; import type { SampleStore } from "./store.js"; import type { BenchTask } from "./task.js"; @@ -149,7 +149,10 @@ export async function runCell(input: RunCellInput): Promise { // A hung run produced no completed transcript to audit or score; record it // as a failure with no measured consumption. if (result.kind === "hung") { - return recorded(store, makeRecord(input, NO_TOKENS, 0, 0, durationMs, { pass: false, failure: "hung" }, clock)); + return recorded( + store, + makeRecord(input, NO_TOKENS, 0, 0, durationMs, { pass: false, failure: "hung" }, undefined, clock), + ); } const run = result.run; @@ -161,13 +164,19 @@ export async function runCell(input: RunCellInput): Promise { return { kind: "invalid", leaks: audit.leaks }; } + const spec = task.scoringSpec(coords.owner); const outcome = run.stoppedByTurnCap ? ({ pass: false, failure: "confused" } as const) - : await scoreRun(host, coords, task, run); + : await scoreRun(host, coords, spec, run); + + // Retain the agent's final report for read tasks so a failed read is + // diagnosable directly from the record; mutation tasks are scored by diffing + // repository state and have no agent report to record. + const report = spec.kind === "read" ? run.finalReport : undefined; return recorded( store, - makeRecord(input, run.tokens, run.turns, run.imputedCostUsd, durationMs, outcome, clock), + makeRecord(input, run.tokens, run.turns, run.imputedCostUsd, durationMs, outcome, report, clock), ); } finally { await host.delete(coords); @@ -214,8 +223,7 @@ async function runBounded( * final report against the required facts for a read. A pass is a pass; anything * the checker rejects is an incorrect failure. */ -async function scoreRun(host: BenchHost, coords: RepoCoords, task: BenchTask, run: AgentRun): Promise { - const spec = task.scoringSpec(coords.owner); +async function scoreRun(host: BenchHost, coords: RepoCoords, spec: ScoringSpec, run: AgentRun): Promise { const snapshot = await host.capture(coords); const check = spec.kind === "mutation" @@ -232,6 +240,7 @@ function makeRecord( imputedCostUsd: number, durationMs: number, outcome: Outcome, + report: string | undefined, clock: RunnerClock, ): ResultRecord { return { @@ -245,6 +254,9 @@ function makeRecord( durationMs, imputedCostUsd, outcome, + // Absent for mutation runs and runs with no completed report (hung); JSON + // serialization drops the key when undefined. + ...(report !== undefined ? { report } : {}), }; } diff --git a/bench/store.test.ts b/bench/store.test.ts index d6e6ada..b5b365a 100644 --- a/bench/store.test.ts +++ b/bench/store.test.ts @@ -102,4 +102,24 @@ describe("SampleStore", () => { secondRun.append(second); expect(secondRun.read(cell)).toEqual([first, second]); }); + + // Behavior: the store round-trips a report-bearing record without any change to + // the store itself. A read task's record carries the agent's final report in the + // `report` field; appending it and reading it back must return it equal, `report` + // and all, proving the store persists the field with no store change. The planted + // report string is an independent literal, not anything production code computes. + it("round-trips a record carrying a report field, preserving it unchanged", () => { + const store = createSampleStore(root); + const report = "There are 5 open issues in the repository."; + const record = sample({ tier: "read", report }); + + store.append(record); + + const readBack = store.read({ arm: record.arm, taskId: record.taskId }); + expect(readBack).toEqual([record]); + const [only] = readBack; + expect(only).toBeDefined(); + if (only === undefined) return; + expect(only.report).toBe(report); + }); });