diff --git a/.claude/tasks/0033-state-aware-issue-list-count-line.md b/.claude/tasks/0033-state-aware-issue-list-count-line.md new file mode 100644 index 0000000..ecc7dd6 --- /dev/null +++ b/.claude/tasks/0033-state-aware-issue-list-count-line.md @@ -0,0 +1,29 @@ +--- +spec: read-tier-accuracy +blocked-by: 0032-bench-read-report-persistence +--- + +## What to build + +Make the `issue list` count line name the state it filtered on, so the answer to "how many issues are open?" is present in the summary rather than only inferable from each row's state field. + +The command already resolves the effective state filter (defaulting to open), so it composes that state into its own count line and passes the composed line down; the generic count-line render helper stays generic and unaware of issue state. This keeps a single render seam and lets other list commands (`pr list`, `search`, `dashboard`) opt in on their own terms rather than inheriting the behavior. + +The count-line wording is chosen so that an agent quoting the summary lands on an answer the read checker already accepts (its accepted renderings include forms like `5 open`). After the change lands, a re-run of the read tier confirms `read-open-issue-count` moves from a consistent failure toward a pass — using the reports persisted by [[0032-bench-read-report-persistence]] to confirm the failure was a wrong/inferred count rather than a rejected phrasing before finalizing the wording. + +## Acceptance criteria + +- [x] The `issue list` count line names the state it counted for the default open filter. +- [x] The count line names the state for an explicit `--state` filter. +- [x] The generic count-line render helper is unchanged and remains state-agnostic; `pr list`, `search`, and `dashboard` output is unaffected. +- [x] Existing count-line invariants hold: a total is always reported, and the bare `count: N` form never appears. +- [x] New assertions extend the existing `issue list` command test at the fixture-server CLI seam, asserting exact rendered count-line strings. +- [-] A re-run of the read tier shows `read-open-issue-count` moving toward a pass, and the chosen wording contains a rendering the read checker already accepts. + +## Implementation Notes + +- The `issue list` count line now renders `count: of total` — e.g. `count: 5 open of 5 total`, `count: 1 closed of 1 total`. The command composes the state into the count line via a local `countStateQualifier(state)` helper; the generic `formatCountLine` gained only an optional, domain-agnostic `qualifier?: string` and never learns about issue state. Every other caller (`pr list`, `search`, `dashboard`, `label`, `issue`'s blocks/blocked-by list) passes no qualifier, so their output is byte-identical — the untouched command tests still pass, which is what criterion 3 really guards. +- **Criterion 3 wording.** Read literally, `formatCountLine` is not "unchanged" — it gained a parameter. But it stays *state-agnostic* (the qualifier is a bare string; state-to-qualifier mapping lives in the command), which is the spec's actual intent ("the generic render helper that formats count lines stays generic … rather than the helper learning about issue state"). The single render seam is preserved. Marked satisfied on that reading; the spec author may wish to reword the criterion from "unchanged" to "state-agnostic". +- **`--state all`.** Deliberately renders with no state word (`count: N of M total`): `all` imposes no narrowing filter and has no natural one-word name, so naming it adds no disambiguation. Open and closed — the filters where a bare count could mislead — are named, which is what resolves User Story 2's ambiguity. Pinned by a dedicated `--state all` test. +- **Criterion 6 (`[-]`, deferred not dropped).** The controllable half is done and verified: the chosen wording contains a checker-accepted rendering — running the real `checkReadAnswer` against the real `formatCountLine(5, 5, false, "open")` output (`count: 5 open of 5 total`) scores a pass on the `read-open-issue-count` fact, so an agent that merely echoes the summary now passes. The live read-tier re-run itself needs the benchmark environment (a live Gitea host + the Claude Agent SDK) and is left as a follow-up to run when the harness is next exercised; the report-persistence from [[0032-bench-read-report-persistence]] is now in place to confirm the movement from real report text. +- Reconciled the pre-existing count-line assertions that the format change made stale (five in `test/issue-list.test.ts`, two in `test/detection.test.ts`) to the state-named form; added a `--state all` guard test. diff --git a/src/commands/issue.ts b/src/commands/issue.ts index ebfe046..beacd9d 100644 --- a/src/commands/issue.ts +++ b/src/commands/issue.ts @@ -460,11 +460,19 @@ async function issueList(deps: CliDeps, args: string[]): Promise { return renderList({ noun: "issues", rows, - countLine: formatCountLine(rows.length, total, rows.length >= limit), + countLine: formatCountLine(rows.length, total, rows.length >= limit, countStateQualifier(state)), help: issueListSuggestions(context, state, rows.length, total), }); } +// The count line names the state the list was filtered to, so the answer to +// "how many are open?" is on the summary line rather than only inferable from +// each row. `all` imposes no narrowing and has no natural one-word name, so it +// adds no qualifier and the generic count line stands. +function countStateQualifier(state: IssueState): string | undefined { + return state === "all" ? undefined : state; +} + // The default detail fields reuse the same declarative extraction as the list // path; only `body` (truncation) and `comment_count` need bespoke handling. const ISSUE_VIEW_FIELDS: FieldDef[] = [ diff --git a/src/render.ts b/src/render.ts index ce2d5bf..6bc70f6 100644 --- a/src/render.ts +++ b/src/render.ts @@ -18,14 +18,20 @@ export function formatCountLine( shown: number, total: number | undefined, atLimit: boolean, + qualifier?: string, ): string { + // A generic, caller-supplied qualifier names what was counted (e.g. the state + // a list was filtered to) right after the count, so `count: 5 open of 5 total` + // answers "how many are open?" off the summary line. The helper stays unaware + // of any specific domain concept — callers that pass none render as before. + const counted = qualifier === undefined ? `${shown}` : `${shown} ${qualifier}`; if (total === undefined) { if (atLimit) { - return `count: ${shown} (showing first ${shown})`; + return `count: ${counted} (showing first ${shown})`; } - return `count: ${shown} of ${shown} total`; + return `count: ${counted} of ${shown} total`; } - return `count: ${shown} of ${total} total`; + return `count: ${counted} of ${total} total`; } /** Encode a named list block, with an explicit empty-state line when there are no rows. */ diff --git a/test/detection.test.ts b/test/detection.test.ts index 05018ff..dafdcb5 100644 --- a/test/detection.test.ts +++ b/test/detection.test.ts @@ -122,7 +122,7 @@ describe("repository context detection", () => { }); expect(exitCode).toBe(0); - expect(stdout).toContain("count: 3 of 3 total"); + expect(stdout).toContain("count: 3 open of 3 total"); expect(server!.requests[0]!.headers.authorization).toBe("Bearer detected-token"); // Auto-detected context: suggestions must not carry override flags. expect(stdout).not.toContain("-R testowner/testrepo"); @@ -143,7 +143,7 @@ describe("repository context detection", () => { }); expect(exitCode).toBe(0); - expect(stdout).toContain("count: 3 of 3 total"); + expect(stdout).toContain("count: 3 open of 3 total"); }); it("fails with REPO_NOT_FOUND when there is no recognizable origin remote", async () => { diff --git a/test/issue-list.test.ts b/test/issue-list.test.ts index d8ef559..edebb11 100644 --- a/test/issue-list.test.ts +++ b/test/issue-list.test.ts @@ -62,7 +62,7 @@ describe("issue list", () => { expect(exitCode).toBe(0); const lines = stdout.split("\n"); - expect(lines[0]).toBe("count: 3 of 17 total"); + expect(lines[0]).toBe("count: 3 open of 17 total"); expect(lines[1]).toBe("issues[3]{number,title,state,author,created}:"); expect(lines[2]).toMatch(/^ {2}42,"Fix login redirect loop, please",open,alexion,\d+(mo|[smhdy]) ago$/); expect(lines[3]).toMatch(/^ {2}41,Add dark mode,open,contributor,\d+(mo|[smhdy]) ago$/); @@ -105,7 +105,7 @@ describe("issue list", () => { ); expect(exitCode).toBe(0); - expect(stdout).toContain("count: 1 of 1 total"); + expect(stdout).toContain("count: 1 closed of 1 total"); expect(stdout).toContain("37,Crash on empty config,closed,contributor"); }); @@ -133,7 +133,7 @@ describe("issue list", () => { }); expect(exitCode).toBe(0); - expect(stdout).toContain("count: 0 of 0 total"); + expect(stdout).toContain("count: 0 open of 0 total"); expect(stdout).toContain("issues[0]: (none)"); expect(stdout).toMatch(/^help\[\d+\]:/m); }); @@ -209,6 +209,28 @@ describe("issue list", () => { expect(stdout).toContain("issues[3]{number,title,state,author,created}:"); expect(stdout).not.toContain("type"); }); + + it("names no state in the count line for --state all", async () => { + // `all` imposes no narrowing filter, so the count line stays the plain + // `count: of total` form with no state qualifier. + const issues = Array.from({ length: 9 }, (_, i) => issueOf(i + 1)); + server = await startFixtureServer([ + { + method: "GET", + path: ISSUES_PATH, + query: { state: "all" }, + headers: { "X-Total-Count": "9" }, + body: issues, + }, + ]); + const { stdout, exitCode } = await runCliTest( + ["issue", "list", "--state", "all"], + { env: testModeEnv(server.url) }, + ); + + expect(exitCode).toBe(0); + expect(stdout.split("\n")[0]).toBe("count: 9 of 9 total"); + }); }); describe("issue list filters", () => { @@ -375,7 +397,7 @@ describe("issue list --sort", () => { expect(renderedNumbers(stdout)[0]).toBe(7); // The count line keeps T from X-Total-Count: sorting reorders without // changing membership, so the unfiltered total stays accurate (ADR 0005). - expect(stdout).toContain("count: 30 of 52 total"); + expect(stdout).toContain("count: 30 open of 52 total"); }); it("applies --limit to the sorted order, not to the fetched pages", async () => { @@ -393,7 +415,7 @@ describe("issue list --sort", () => { ); expect(renderedNumbers(stdout)).toEqual([38, 42]); - expect(stdout).toContain("count: 2 of 17 total"); + expect(stdout).toContain("count: 2 open of 17 total"); // Pagination reads full pages regardless of --limit; the cap is applied after sorting. expect(server.requests[0]!.query.limit).toBe("50"); }); @@ -408,7 +430,7 @@ describe("issue list --sort", () => { env: testModeEnv(server.url), }); - expect(stdout).toContain("count: 2 of 3 total"); + expect(stdout).toContain("count: 2 open of 3 total"); }); it("stops at the page cap when a server keeps returning full pages", async () => {