From b847bf34742f57d7306173f2838137d7128c6159 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maple=EF=BC=81?= Date: Wed, 15 Jul 2026 15:46:55 +0800 Subject: [PATCH 01/16] fix: determine tool result status via ladder instead of defaulting to ok MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bash/Read results carry no toolUseResult.success field, so every failed command was rendered as "-> ok: Exit code 1" — misleading LLMs consuming injected context. Status now follows ADR-003's ladder: explicit success field, then is_error:true on the tool_result block (previously unparsed), then a conservative sniff list (non-zero "Exit code N", hook error), else ok. Failure excerpts skip noise lines (line-number prefixes, bare exit code lines, hook boilerplate) and widen to 200 runes. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_013UQYeDtvg1CQYvirZ4jWC5 --- ...dr-003-tool-result-status-and-summaries.md | 66 ++++++++++++++++ internal/claudecodec/model.go | 75 +++++++++++++++---- internal/claudecodec/reader_test.go | 66 ++++++++++++++++ internal/session/event.go | 62 ++++++++++++++- internal/session/event_test.go | 57 ++++++++++++++ 5 files changed, 310 insertions(+), 16 deletions(-) create mode 100644 docs/adr-003-tool-result-status-and-summaries.md diff --git a/docs/adr-003-tool-result-status-and-summaries.md b/docs/adr-003-tool-result-status-and-summaries.md new file mode 100644 index 0000000..26e31d2 --- /dev/null +++ b/docs/adr-003-tool-result-status-and-summaries.md @@ -0,0 +1,66 @@ +# ADR-003: Tool Result Status Determination & Error/Diff Summaries + +## Status + +Accepted + +## Context + +cc-session renders every tool result with a status token (`-> ok` / `-> FAILED`). This status is consumed by LLMs reading injected context, so a wrong status is worse than a missing detail: the reader concludes a step succeeded when it actually failed. + +The current determination (`internal/claudecodec/model.go`) reads only `toolUseResult.success` and **defaults to true when the field is absent**. Inspection of real Claude Code JSONL transcripts shows this default is wrong for the majority of tool results: + +- **Bash** results carry `toolUseResult: {stdout, stderr, interrupted, isImage, noOutputExpected}` — there is **no `success` field**. Failed commands (non-zero exit, hook rejections, `no such file or directory`) are therefore rendered as `-> ok: Exit code 1`, a self-contradictory line. +- **Read** results (`{type: "text", file: {...}}`) also carry no `success` field. +- Only a minority of tools (e.g. Agent lifecycle operations) set `success` explicitly. +- The `is_error` flag on the `tool_result` content block is **not parsed by the codec at all** — and even it is unreliable: real transcripts contain `is_error: false` on Bash results whose text says `Exit code 1`. Claude Code reports non-zero exits as normal results with the exit code appended to the text. + +Separately, ADR-002's "Next Phase Optimizations" proposed diff summaries for Edit/Write and smarter Bash failure excerpts. Real transcripts confirm Edit results carry `toolUseResult.structuredPatch` as a list of hunks `{oldStart, oldLines, newStart, newLines, lines}` (where `lines` entries are prefixed `+`/`-`/` `), so a one-line diff summary is computable without an LLM. Write results for new files carry `structuredPatch: []` plus `content`. + +## Decision + +### 1. Status determination ladder + +Determine a tool result's status by the first applicable rule: + +1. `toolUseResult.success` present → use it (explicit signal wins). +2. `tool_result` content block has `is_error: true` → failed. (The codec must start parsing this field.) +3. Content sniffing on the result text, using a **conservative, enumerated** pattern list — currently: + - a line matching `Exit code N` with N ≠ 0 (Bash convention), + - a `... hook error` prefix (PreToolUse/PostToolUse hook rejections). +4. Otherwise → ok. + +Sniffing is deliberately a small allowlist of known-failure signatures, not a heuristic: a false `FAILED` misleads the reader just as badly as a false `ok`. New signatures are added only with a real transcript sample as evidence, and each gets a regression test. + +### 2. Error excerpts + +On failure, the summary keeps the first **meaningful** error line instead of blindly the first line: + +- Skip known noise prefixes when picking the excerpt: `cat -n` style line numbers (`^\s*\d+\t`), hook boilerplate, and the bare `Exit code N` line itself (the code is already reflected in the status; per ADR-002, surface the actual compiler/test error beneath it). +- Failed results get a larger excerpt budget than successful ones (single line, up to ~200 chars) — errors are the content least safe to drop. + +The status token stays `ok` / `FAILED` (unchanged) to avoid churning the existing output contract. + +### 3. Diff summaries for Edit/Write + +Successful Edit/Write results upgrade from a bare `-> ok`: + +- **Edit** with non-empty `structuredPatch`: `-> ok (+A, -D @ L)`, where A/D are `+`/`-` line counts summed across hunks and L is the first hunk's start; append `, H hunks` when H > 1. +- **Write** (new file, empty `structuredPatch`): `-> ok (new file, N lines)` from `content`. +- `structuredPatch` missing or unparsable: fall back to the current `-> ok`. + +### 4. Unknown-tool fallback + +The summarizer's default branch currently renders `[ToolName]` with zero input context. It changes to render the first 2–3 input key/value pairs (each value truncated to ~60 chars), so tools added to Claude Code after this release degrade gracefully instead of silently losing all information. + +## Negative knowledge (do not "simplify" these away) + +- `toolUseResult.success` is **absent** for the highest-frequency tools (Bash, Read). Defaulting the absent case to either value is a guess; the ladder above exists because no single field is authoritative. +- `is_error` can be `false` on genuinely failed Bash commands. It is a failure signal when true, never a success signal when false. +- Content sniffing therefore cannot be removed in favor of "just use the flags" — the flags do not carry the information. + +## Consequences + +- Injected context becomes trustworthy about which steps failed — the primary correctness property of this tool. +- Diff summaries add a few tokens per Edit/Write but answer the most common follow-up question ("what did that session actually change") without `expand`. +- New failure shapes default to `ok` until their signature is added to the sniff list; the audit command remains the mechanism for spotting such gaps. diff --git a/internal/claudecodec/model.go b/internal/claudecodec/model.go index 175a115..d0121ff 100644 --- a/internal/claudecodec/model.go +++ b/internal/claudecodec/model.go @@ -3,6 +3,7 @@ package claudecodec import ( "encoding/json" "fmt" + "regexp" "strings" "github.com/Mapleeeeeeeeeee/cc-session-reader/internal/session" @@ -126,6 +127,11 @@ type rawContentBlock struct { ToolUseID string `json:"tool_use_id"` Input json.RawMessage `json:"input"` Content json.RawMessage `json:"content"` + // IsError is the tool_result content block's failure flag. Per ADR-003 it + // is a one-directional signal: true always means failed, but false does not + // mean success (real transcripts carry is_error:false on Bash results whose + // text says "Exit code 1") — see the status ladder in toToolResult. + IsError bool `json:"is_error"` } func cleanCwdPaths(text string, cwd string) string { @@ -136,40 +142,83 @@ func cleanCwdPaths(text string, cwd string) string { } func (e rawEntry) toToolResult() session.ToolResult { - result := rawToolUseResult{Success: true} + var result rawToolUseResult if len(e.ToolUseResult) > 0 { _ = json.Unmarshal(e.ToolUseResult, &result) } - text, toolUseID := extractToolResultText(e.Message.Blocks) + text, toolUseID, isError := extractToolResultText(e.Message.Blocks) name := result.CommandName if name == "" { name = result.AgentType } + cleanText := cleanCwdPaths(text, e.Cwd) return session.ToolResult{ ToolUseID: toolUseID, - Success: result.Success, - Text: cleanCwdPaths(text, e.Cwd), + Success: determineSuccess(result.Success, isError, cleanText), + Text: cleanText, RawName: name, } } type rawToolUseResult struct { - Success bool `json:"success"` + // Success is a pointer so an absent field (the common case for Bash and + // Read, see ADR-003) is distinguishable from an explicit false — nil means + // "no explicit signal", not "failed". + Success *bool `json:"success"` CommandName string `json:"commandName"` AgentType string `json:"agentType"` } -func extractToolResultText(blocks []rawContentBlock) (string, string) { +// nonZeroExitCodeLine matches a bare Bash "Exit code N" line. Claude Code +// appends this line to Bash results regardless of outcome, so N must be +// checked: "Exit code 0" is a success marker, not a failure signature. +var nonZeroExitCodeLine = regexp.MustCompile(`(?m)^Exit code (\d+)\s*$`) + +// hookErrorSignature matches PreToolUse/PostToolUse hook rejection text, +// which Claude Code renders as ordinary result content ending in +// "... hook error" rather than setting is_error. +var hookErrorSignature = regexp.MustCompile(`hook error`) + +// determineSuccess applies the ADR-003 status ladder: the first applicable +// signal wins. explicitSuccess is nil when toolUseResult.success was absent +// from the transcript line (the common case for Bash/Read). +func determineSuccess(explicitSuccess *bool, isError bool, text string) bool { + if explicitSuccess != nil { + return *explicitSuccess + } + if isError { + return false + } + if hasKnownFailureSignature(text) { + return false + } + return true +} + +// hasKnownFailureSignature sniffs result text for the small, enumerated set +// of known-failure patterns from ADR-003. This is deliberately not a general +// heuristic: a false FAILED misleads the reader as badly as a false ok, so +// new signatures are added only with a real transcript sample as evidence. +func hasKnownFailureSignature(text string) bool { + for _, match := range nonZeroExitCodeLine.FindAllStringSubmatch(text, -1) { + if match[1] != "0" { + return true + } + } + return hookErrorSignature.MatchString(text) +} + +func extractToolResultText(blocks []rawContentBlock) (string, string, bool) { for _, block := range blocks { if block.Type != "tool_result" { continue } if len(block.Content) == 0 { - return "", block.ToolUseID + return "", block.ToolUseID, block.IsError } var s string if err := json.Unmarshal(block.Content, &s); err == nil { - return s, block.ToolUseID + return s, block.ToolUseID, block.IsError } var subBlocks []rawContentBlock if err := json.Unmarshal(block.Content, &subBlocks); err == nil { @@ -179,15 +228,15 @@ func extractToolResultText(blocks []rawContentBlock) (string, string) { parts = append(parts, subBlock.Text) } } - return strings.Join(parts, "\n"), block.ToolUseID + return strings.Join(parts, "\n"), block.ToolUseID, block.IsError } - return string(block.Content), block.ToolUseID + return string(block.Content), block.ToolUseID, block.IsError } - return "", "" + return "", "", false } func extractUserAnswer(blocks []rawContentBlock) string { - text, _ := extractToolResultText(blocks) + text, _, _ := extractToolResultText(blocks) for _, prefix := range userAnswerPrefixes { if strings.HasPrefix(text, prefix) { return text @@ -209,7 +258,7 @@ func (e rawEntry) extractAllText() string { parts = append(parts, marshalNoEscape(block.Input)) } case "tool_result": - text, _ := extractToolResultText([]rawContentBlock{block}) + text, _, _ := extractToolResultText([]rawContentBlock{block}) if text != "" { parts = append(parts, text) } diff --git a/internal/claudecodec/reader_test.go b/internal/claudecodec/reader_test.go index ab87a16..3f2fe0a 100644 --- a/internal/claudecodec/reader_test.go +++ b/internal/claudecodec/reader_test.go @@ -67,6 +67,72 @@ func TestParseLine_ToolResultTextBlockContent(t *testing.T) { } } +// --- ADR-003 decision 1: status determination ladder --- + +// TestParseLine_ToolResult_GivenExplicitSuccessField_ThenItWinsOverSniffedFailureText +// pins ladder rule 1: an explicit toolUseResult.success always wins, even when +// the result text also contains a known-failure signature (here "Exit code 1"). +func TestParseLine_ToolResult_GivenExplicitSuccessField_ThenItWinsOverSniffedFailureText(t *testing.T) { + event := parseLine(t, `{"type":"user","toolUseResult":{"success":true,"commandName":"Bash"},"message":{"role":"user","content":[{"type":"tool_result","tool_use_id":"tool-1","content":"Exit code 1"}]}}`) + if event.Tool == nil || !event.Tool.Success { + t.Fatalf("tool result = %#v, want Success=true (explicit signal wins)", event.Tool) + } +} + +// TestParseLine_ToolResult_GivenBashResultWithoutSuccessField_ThenNoSignalDefaultsOk +// pins that the highest-frequency shape (Bash toolUseResult carries no +// "success" field at all) still resolves to ok when nothing in the ladder +// signals failure. This is the safe default from ADR-003 rule 4. +func TestParseLine_ToolResult_GivenBashResultWithoutSuccessField_ThenNoSignalDefaultsOk(t *testing.T) { + event := parseLine(t, `{"type":"user","toolUseResult":{"stdout":"done","stderr":"","interrupted":false},"message":{"role":"user","content":[{"type":"tool_result","tool_use_id":"tool-1","content":"done\nExit code 0"}]}}`) + if event.Tool == nil || !event.Tool.Success { + t.Fatalf("tool result = %#v, want Success=true", event.Tool) + } +} + +// TestParseLine_ToolResult_GivenNoSuccessFieldAndIsErrorTrue_ThenFailed guards +// the bug described in ADR-003: Bash toolUseResult has no "success" field, so +// the codec used to default Success=true unconditionally, rendering failed +// commands as "-> ok". With is_error:true parsed, it must resolve to FAILED. +func TestParseLine_ToolResult_GivenNoSuccessFieldAndIsErrorTrue_ThenFailed(t *testing.T) { + event := parseLine(t, `{"type":"user","toolUseResult":{"stdout":"","stderr":"no such file or directory"},"message":{"role":"user","content":[{"type":"tool_result","tool_use_id":"tool-1","is_error":true,"content":"no such file or directory"}]}}`) + if event.Tool == nil || event.Tool.Success { + t.Fatalf("tool result = %#v, want Success=false", event.Tool) + } +} + +// TestParseLine_ToolResult_GivenIsErrorFalseButExitCodeNonZero_ThenSniffedAsFailed +// guards the ADR-003 negative knowledge: is_error is a one-directional signal. +// Real transcripts carry is_error:false on Bash results whose text still says +// "Exit code 1" — false must never be trusted as a success signal, so the +// content sniff step must still catch the failure. +func TestParseLine_ToolResult_GivenIsErrorFalseButExitCodeNonZero_ThenSniffedAsFailed(t *testing.T) { + event := parseLine(t, `{"type":"user","toolUseResult":{"stdout":"","stderr":""},"message":{"role":"user","content":[{"type":"tool_result","tool_use_id":"tool-1","is_error":false,"content":"Exit code 1"}]}}`) + if event.Tool == nil || event.Tool.Success { + t.Fatalf("tool result = %#v, want Success=false (sniffed from Exit code 1)", event.Tool) + } +} + +// TestParseLine_ToolResult_GivenExitCodeZero_ThenNotSniffedAsFailed guards +// against over-eager sniffing: "Exit code 0" reports a successful exit and +// must not trigger the failure signature. +func TestParseLine_ToolResult_GivenExitCodeZero_ThenNotSniffedAsFailed(t *testing.T) { + event := parseLine(t, `{"type":"user","toolUseResult":{"stdout":"ok","stderr":""},"message":{"role":"user","content":[{"type":"tool_result","tool_use_id":"tool-1","content":"ok\nExit code 0"}]}}`) + if event.Tool == nil || !event.Tool.Success { + t.Fatalf("tool result = %#v, want Success=true", event.Tool) + } +} + +// TestParseLine_ToolResult_GivenHookErrorText_ThenSniffedAsFailed pins the +// second ADR-003 sniff pattern: a hook rejection is reported as ordinary +// result content (no success/is_error signal) ending in "... hook error". +func TestParseLine_ToolResult_GivenHookErrorText_ThenSniffedAsFailed(t *testing.T) { + event := parseLine(t, `{"type":"user","toolUseResult":{"stdout":"","stderr":""},"message":{"role":"user","content":[{"type":"tool_result","tool_use_id":"tool-1","content":"PreToolUse:Bash [rm -rf /tmp/x] hook error: blocked by policy"}]}}`) + if event.Tool == nil || event.Tool.Success { + t.Fatalf("tool result = %#v, want Success=false (sniffed from hook error)", event.Tool) + } +} + func TestParseLine_UserAnswer(t *testing.T) { event := parseLine(t, `{"type":"user","toolUseResult":{"success":true},"message":{"role":"user","content":[{"type":"tool_result","tool_use_id":"tool-3","content":"User has answered your questions: ship it"}]}}`) if event.User == nil || !event.User.IsAnswer || event.User.Text != "User has answered your questions: ship it" { diff --git a/internal/session/event.go b/internal/session/event.go index 86e876b..2840e7f 100644 --- a/internal/session/event.go +++ b/internal/session/event.go @@ -198,20 +198,76 @@ func (r ToolResult) Status() string { return "FAILED" } +const ( + // successExcerptMaxRunes bounds the one-line summary shown after a + // successful result, where the reader mainly wants a glance, not detail. + successExcerptMaxRunes = 80 + // failureExcerptMaxRunes is wider than the success budget (ADR-003): + // errors are the content least safe to drop from the compact summary. + failureExcerptMaxRunes = 200 +) + func (r ToolResult) Summary() string { if r.Success { switch r.RawName { case ToolRead, ToolWrite, ToolEdit, ToolAgent: return fmt.Sprintf(" -> %s", r.Status()) } + if firstLine := FirstLine(r.Text, successExcerptMaxRunes); firstLine != "" { + return fmt.Sprintf(" -> %s: %s", r.Status(), firstLine) + } + return fmt.Sprintf(" -> %s", r.Status()) } - firstLine := FirstLine(r.Text, 80) - if firstLine != "" { - return fmt.Sprintf(" -> %s: %s", r.Status(), firstLine) + if excerpt := firstMeaningfulErrorLine(r.Text, failureExcerptMaxRunes); excerpt != "" { + return fmt.Sprintf(" -> %s: %s", r.Status(), excerpt) } return fmt.Sprintf(" -> %s", r.Status()) } +// catLineNumberPrefix matches "cat -n" style line-number prefixes (" 12\t") +// that Read/Edit tool output prepends to every line — noise, not error content. +var catLineNumberPrefix = regexp.MustCompile(`^\s*\d+\t`) + +// bareExitCodeLine matches a standalone "Exit code N" line. The code is +// already reflected in the FAILED status token, so per ADR-003 it is skipped +// when picking an excerpt in favor of the actual error line beneath it. +var bareExitCodeLine = regexp.MustCompile(`^Exit code \d+\s*$`) + +// hookErrorBoilerplate matches PreToolUse/PostToolUse hook rejection preamble +// lines ("... hook error"), which name the hook stage rather than the actual +// failure — the reader wants what's beneath it, not the wrapper. +var hookErrorBoilerplate = regexp.MustCompile(`hook error`) + +// isNoiseExcerptLine reports whether line is one of the known-noise shapes +// that should be skipped when picking a failure excerpt (ADR-003 decision 2). +func isNoiseExcerptLine(line string) bool { + return catLineNumberPrefix.MatchString(line) || + bareExitCodeLine.MatchString(line) || + hookErrorBoilerplate.MatchString(line) +} + +// firstMeaningfulErrorLine returns the first non-noise line of text, skipping +// cat -n prefixes, bare "Exit code N" lines, and hook boilerplate so the +// excerpt surfaces the actual error instead of the noise around it. If every +// line is noise, it falls back to the first non-empty line rather than +// dropping the excerpt entirely. +func firstMeaningfulErrorLine(text string, maxRunes int) string { + var firstNonEmpty string + for _, raw := range strings.Split(text, "\n") { + line := strings.TrimSpace(raw) + if line == "" { + continue + } + if firstNonEmpty == "" { + firstNonEmpty = line + } + if !isNoiseExcerptLine(line) { + return Truncate(line, maxRunes) + } + } + return Truncate(firstNonEmpty, maxRunes) +} + type NoiseEvent struct { Text string } diff --git a/internal/session/event_test.go b/internal/session/event_test.go index d497602..a80fa0b 100644 --- a/internal/session/event_test.go +++ b/internal/session/event_test.go @@ -293,6 +293,63 @@ func TestToolResultSummary(t *testing.T) { } } +// --- ADR-003 decision 2: error excerpts skip noise lines --- + +// TestToolResultSummary_GivenFailureTextWithNoiseLines_ThenExcerptSkipsNoiseAndWidensBudget +// guards the bug described in ADR-003: before this fix, a failed result's +// summary blindly took the first line, which was often noise (a cat -n line +// number prefix, the bare "Exit code N" line itself, or hook rejection +// boilerplate) rather than the actual error. It also pins the widened +// single-line budget (~200 chars) failures get versus successes (~80). +func TestToolResultSummary_GivenFailureTextWithNoiseLines_ThenExcerptSkipsNoiseAndWidensBudget(t *testing.T) { + longError := "compiler error: " + strings.Repeat("x", 190) + tests := []struct { + name string + text string + want string + }{ + { + name: "given bare exit code line then skips it for the real error beneath", + text: "Exit code 1\ncompiler error: unexpected token", + want: " -> FAILED: compiler error: unexpected token", + }, + { + name: "given cat -n line number prefix then skips it", + text: " 12\tfunc broken() {\nsyntax error: missing }", + want: " -> FAILED: syntax error: missing }", + }, + { + name: "given hook error boilerplate then skips it for the detail beneath", + text: "PreToolUse:Bash hook error: blocked\nactual reason: policy violation", + want: " -> FAILED: actual reason: policy violation", + }, + { + name: "given only noise lines then falls back to the first noise line", + text: "Exit code 1", + want: " -> FAILED: Exit code 1", + }, + { + name: "given non-noise first line then keeps prior single-line behavior", + text: "bad", + want: " -> FAILED: bad", + }, + { + name: "given long error line then truncates to the 200-char failure budget", + text: longError, + want: " -> FAILED: " + Truncate(longError, failureExcerptMaxRunes), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := ToolResult{Success: false, Text: tt.text} + if got := result.Summary(); got != tt.want { + t.Fatalf("Summary() = %q, want %q", got, tt.want) + } + }) + } +} + func TestCompactTaskNotification_GivenFullNotification_ThenKeepsSummaryAndResult(t *testing.T) { input := ` ad4760fe24f754e27 From fab62d78bf29bd6996b45a1c433d555a25753974 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maple=EF=BC=81?= Date: Wed, 15 Jul 2026 16:04:59 +0800 Subject: [PATCH 02/16] feat: diff summaries for Edit/Write and graceful unknown-tool fallback Successful Edit results now render "-> ok (+A, -D @ L)" computed from toolUseResult.structuredPatch hunks; Write renders "-> ok (new file, N lines)"; missing patch data falls back to the bare status (ADR-003 decisions 3-4). Unknown tools show their first input key/values instead of a bare tag, so new Claude Code tools degrade gracefully. Also fixes the pre-existing root cause that defeated ADR-002's per-tool suppression on real transcripts: tool names live on the assistant tool_use block, not on toolUseResult (commandName/agentType are absent for Bash/Read/Write/Edit), so RawName was always empty. ReadFile now correlates tool_use id -> name and resolves result names through it, covered by pipeline-level regression tests that parse realistic two-entry fixtures instead of hand-filled RawName values. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_013UQYeDtvg1CQYvirZ4jWC5 --- internal/claudecodec/model.go | 88 +++++++++++- internal/claudecodec/reader.go | 26 +++- internal/claudecodec/reader_test.go | 187 +++++++++++++++++++++++++ internal/session/event.go | 46 ++++++ internal/session/event_test.go | 61 ++++++++ internal/summarizer/summarizer.go | 38 ++++- internal/summarizer/summarizer_test.go | 48 ++++++- 7 files changed, 488 insertions(+), 6 deletions(-) diff --git a/internal/claudecodec/model.go b/internal/claudecodec/model.go index d0121ff..a103a4a 100644 --- a/internal/claudecodec/model.go +++ b/internal/claudecodec/model.go @@ -141,7 +141,13 @@ func cleanCwdPaths(text string, cwd string) string { return strings.ReplaceAll(text, cwd, ".") } -func (e rawEntry) toToolResult() session.ToolResult { +// toToolResult builds a ToolResult from the entry's toolUseResult/tool_result +// block. toolNames is the tool_use_id -> tool name map accumulated by the +// caller's sequential read (nil when called from the stateless public +// ParseLine): real transcripts carry no commandName/agentType field on +// Bash/Edit/Write/Read results, so name falls back to the map, which is +// populated from the preceding assistant tool_use block's declared name. +func (e rawEntry) toToolResult(toolNames map[string]string) session.ToolResult { var result rawToolUseResult if len(e.ToolUseResult) > 0 { _ = json.Unmarshal(e.ToolUseResult, &result) @@ -151,12 +157,17 @@ func (e rawEntry) toToolResult() session.ToolResult { if name == "" { name = result.AgentType } + if name == "" { + name = toolNames[toolUseID] + } cleanText := cleanCwdPaths(text, e.Cwd) + success := determineSuccess(result.Success, isError, cleanText) return session.ToolResult{ ToolUseID: toolUseID, - Success: determineSuccess(result.Success, isError, cleanText), + Success: success, Text: cleanText, RawName: name, + DiffStat: diffStatFor(name, success, result), } } @@ -167,6 +178,79 @@ type rawToolUseResult struct { Success *bool `json:"success"` CommandName string `json:"commandName"` AgentType string `json:"agentType"` + + // StructuredPatch and Content feed the ADR-003 decision 3 diff summary. + // Edit carries a non-empty StructuredPatch; Write (new file) carries an + // empty StructuredPatch plus Content holding the file body. + StructuredPatch []rawPatchHunk `json:"structuredPatch"` + Content *string `json:"content"` +} + +// rawPatchHunk is one hunk of toolUseResult.structuredPatch. Lines entries +// are prefixed "+"/"-"/" " (unified-diff convention) per ADR-003. +type rawPatchHunk struct { + OldStart int `json:"oldStart"` + OldLines int `json:"oldLines"` + NewStart int `json:"newStart"` + NewLines int `json:"newLines"` + Lines []string `json:"lines"` +} + +// diffStatFor computes the ADR-003 decision 3 diff summary from a successful +// Edit/Write result. Returns nil when the result failed, the tool isn't +// Edit/Write, or the expected structuredPatch/content shape is missing — +// callers then fall back to the bare "-> ok" status line. +func diffStatFor(name string, success bool, result rawToolUseResult) *session.DiffStat { + if !success { + return nil + } + switch name { + case session.ToolEdit: + if len(result.StructuredPatch) == 0 { + return nil + } + var additions, deletions int + for _, hunk := range result.StructuredPatch { + for _, line := range hunk.Lines { + switch { + case strings.HasPrefix(line, "+"): + additions++ + case strings.HasPrefix(line, "-"): + deletions++ + } + } + } + return &session.DiffStat{ + Additions: additions, + Deletions: deletions, + NewStartLine: result.StructuredPatch[0].NewStart, + HunkCount: len(result.StructuredPatch), + } + case session.ToolWrite: + if len(result.StructuredPatch) != 0 || result.Content == nil { + return nil + } + return &session.DiffStat{ + IsNewFile: true, + NewFileLines: countContentLines(*result.Content), + } + default: + return nil + } +} + +// countContentLines counts the visible lines of Write's new-file content. A +// trailing newline (the common text-file convention) does not count as an +// extra blank line, so "a\nb\n" and "a\nb" both report 2 lines. +func countContentLines(content string) int { + if content == "" { + return 0 + } + lines := strings.Split(content, "\n") + if lines[len(lines)-1] == "" { + lines = lines[:len(lines)-1] + } + return len(lines) } // nonZeroExitCodeLine matches a bare Bash "Exit code N" line. Claude Code diff --git a/internal/claudecodec/reader.go b/internal/claudecodec/reader.go index dc2053b..b99d36f 100644 --- a/internal/claudecodec/reader.go +++ b/internal/claudecodec/reader.go @@ -41,6 +41,10 @@ func ReadFile(path string, handle func(session.Event) error) error { } defer f.Close() + // toolNames accumulates tool_use_id -> tool name across the sequential + // read, scoped to this file. See parseLineWithToolNames for why this + // state can't live inside the stateless public ParseLine. + toolNames := map[string]string{} reader := bufio.NewReader(f) for { line, readErr := reader.ReadBytes('\n') @@ -56,7 +60,7 @@ func ReadFile(path string, handle func(session.Event) error) error { } continue } - event, ok, parseErr := ParseLine(line) + event, ok, parseErr := parseLineWithToolNames(line, toolNames) if parseErr != nil { return parseErr } @@ -82,6 +86,19 @@ func ReadAll(path string) ([]session.Event, error) { } func ParseLine(line []byte) (session.Event, bool, error) { + return parseLineWithToolNames(line, nil) +} + +// parseLineWithToolNames is ParseLine's implementation, extended with the +// tool_use_id -> tool name state ReadFile accumulates across a sequential +// read. Real transcripts carry no commandName/agentType field on +// Bash/Edit/Write/Read toolUseResults — the tool name only exists on the +// preceding assistant tool_use block, correlated by tool_use_id — so a +// tool_result's name/DiffStat can only be resolved with that cross-line +// state. ParseLine keeps its public, stateless single-line contract by +// passing toolNames=nil, under which name resolution falls back to +// commandName/agentType only, same as before this fix. +func parseLineWithToolNames(line []byte, toolNames map[string]string) (session.Event, bool, error) { var raw rawEntry if err := json.Unmarshal(line, &raw); err != nil { return session.Event{}, false, fmt.Errorf("parse transcript line: %w", err) @@ -112,7 +129,7 @@ func ParseLine(line []byte) (session.Event, bool, error) { } if len(raw.ToolUseResult) > 0 { - toolResult := raw.toToolResult() + toolResult := raw.toToolResult(toolNames) event.Kind = session.EventToolResult event.Tool = &toolResult if answer := extractUserAnswer(raw.Message.Blocks); answer != "" { @@ -144,6 +161,11 @@ func ParseLine(line []byte) (session.Event, bool, error) { for i := range assistant.ToolUses { assistant.ToolUses[i].Cwd = raw.Cwd } + if toolNames != nil { + for _, toolUse := range assistant.ToolUses { + toolNames[toolUse.ID] = toolUse.Name + } + } event.Kind = session.EventAssistantMessage event.Assistant = &assistant return event, true, nil diff --git a/internal/claudecodec/reader_test.go b/internal/claudecodec/reader_test.go index 3f2fe0a..8aabf48 100644 --- a/internal/claudecodec/reader_test.go +++ b/internal/claudecodec/reader_test.go @@ -133,6 +133,193 @@ func TestParseLine_ToolResult_GivenHookErrorText_ThenSniffedAsFailed(t *testing. } } +// --- ADR-003 decision 3: diff summaries for Edit/Write --- + +// TestParseLine_ToolResult_GivenEditSingleHunk_ThenDiffStatSumsHunkLines pins +// the codec's structuredPatch parsing: +/- line counts summed from the hunk's +// "lines" entries, and NewStartLine taken from the hunk's newStart. +func TestParseLine_ToolResult_GivenEditSingleHunk_ThenDiffStatSumsHunkLines(t *testing.T) { + line := `{"type":"user","toolUseResult":{"success":true,"commandName":"Edit","structuredPatch":[` + + `{"oldStart":10,"oldLines":3,"newStart":10,"newLines":4,"lines":[" line1","-old line","+new line","+another new"," line2"]}` + + `]},"message":{"role":"user","content":[{"type":"tool_result","tool_use_id":"tool-1","content":"ok"}]}}` + event := parseLine(t, line) + + stat := event.Tool.DiffStat + if stat == nil { + t.Fatal("DiffStat is nil, want populated stat") + } + if stat.IsNewFile { + t.Fatal("IsNewFile = true, want false for Edit") + } + if stat.Additions != 2 || stat.Deletions != 1 || stat.NewStartLine != 10 || stat.HunkCount != 1 { + t.Fatalf("DiffStat = %#v, want Additions=2 Deletions=1 NewStartLine=10 HunkCount=1", stat) + } +} + +// TestParseLine_ToolResult_GivenEditMultipleHunks_ThenDiffStatAggregatesAcrossHunks +// pins multi-hunk aggregation: additions/deletions sum across every hunk, but +// NewStartLine stays pinned to the *first* hunk (per ADR-003), and HunkCount +// reflects the total hunk count. +func TestParseLine_ToolResult_GivenEditMultipleHunks_ThenDiffStatAggregatesAcrossHunks(t *testing.T) { + line := `{"type":"user","toolUseResult":{"success":true,"commandName":"Edit","structuredPatch":[` + + `{"oldStart":5,"oldLines":2,"newStart":5,"newLines":3,"lines":[" a","+b","+c"]},` + + `{"oldStart":40,"oldLines":4,"newStart":41,"newLines":2,"lines":["-d","-e"," f"]}` + + `]},"message":{"role":"user","content":[{"type":"tool_result","tool_use_id":"tool-1","content":"ok"}]}}` + event := parseLine(t, line) + + stat := event.Tool.DiffStat + if stat == nil { + t.Fatal("DiffStat is nil, want populated stat") + } + if stat.Additions != 2 || stat.Deletions != 2 || stat.NewStartLine != 5 || stat.HunkCount != 2 { + t.Fatalf("DiffStat = %#v, want Additions=2 Deletions=2 NewStartLine=5 HunkCount=2", stat) + } +} + +// TestParseLine_ToolResult_GivenWriteNewFile_ThenDiffStatCountsContentLines +// pins Write's new-file shape: empty structuredPatch plus a content field, no +// hunks to parse, so the line count comes from splitting content instead. +func TestParseLine_ToolResult_GivenWriteNewFile_ThenDiffStatCountsContentLines(t *testing.T) { + line := `{"type":"user","toolUseResult":{"success":true,"commandName":"Write","structuredPatch":[],` + + `"content":"package main\n\nfunc main() {}\n"},` + + `"message":{"role":"user","content":[{"type":"tool_result","tool_use_id":"tool-1","content":"ok"}]}}` + event := parseLine(t, line) + + stat := event.Tool.DiffStat + if stat == nil { + t.Fatal("DiffStat is nil, want populated stat") + } + if !stat.IsNewFile { + t.Fatal("IsNewFile = false, want true for Write") + } + if stat.NewFileLines != 3 { + t.Fatalf("NewFileLines = %d, want 3", stat.NewFileLines) + } +} + +// TestParseLine_ToolResult_GivenEditWithoutStructuredPatch_ThenDiffStatNil +// guards the ADR-003 fallback: a successful Edit whose toolUseResult carries +// no structuredPatch (missing/unparsable) must not synthesize a fake diff — +// DiffStat stays nil so Summary() renders the bare "-> ok" it always did. +func TestParseLine_ToolResult_GivenEditWithoutStructuredPatch_ThenDiffStatNil(t *testing.T) { + line := `{"type":"user","toolUseResult":{"success":true,"commandName":"Edit"},` + + `"message":{"role":"user","content":[{"type":"tool_result","tool_use_id":"tool-1","content":"ok"}]}}` + event := parseLine(t, line) + + if event.Tool.DiffStat != nil { + t.Fatalf("DiffStat = %#v, want nil (no structuredPatch in transcript)", event.Tool.DiffStat) + } +} + +// TestParseLine_ToolResult_GivenFailedEdit_ThenDiffStatNil guards the +// constraint that diff summaries never apply to failed results, even when the +// transcript happens to carry a structuredPatch — the existing FAILED excerpt +// path must own the summary, not the diff annotation. +func TestParseLine_ToolResult_GivenFailedEdit_ThenDiffStatNil(t *testing.T) { + line := `{"type":"user","toolUseResult":{"success":false,"commandName":"Edit","structuredPatch":[` + + `{"oldStart":1,"oldLines":1,"newStart":1,"newLines":1,"lines":["-a","+b"]}` + + `]},"message":{"role":"user","content":[{"type":"tool_result","tool_use_id":"tool-1","content":"permission denied"}]}}` + event := parseLine(t, line) + + if event.Tool.DiffStat != nil { + t.Fatalf("DiffStat = %#v, want nil for a failed result", event.Tool.DiffStat) + } +} + +// --- ADR-003 follow-up: tool_result name resolved from the preceding tool_use --- +// +// Real Claude Code transcripts carry no commandName/agentType field on +// Bash/Edit/Write/Read toolUseResults (only "success"-less bodies like +// {filePath, structuredPatch, ...} for Edit, or {type, file} for Read) — the +// tool name only exists on the earlier assistant tool_use block, correlated +// by tool_use_id. A fixture with RawName/commandName hand-filled (as every +// other test in this file does, matching ParseLine's single-line contract) +// cannot catch a regression here: it has to run the real two-entry sequence +// through ReadAll so the tool_use -> tool_result name resolution actually +// executes. + +// TestReadAll_GivenEditToolResultWithoutCommandName_ThenRawNameResolvedFromPrecedingToolUse +// guards the bug where diffStatFor's tool-name gate silently no-opped on +// every real transcript because RawName was always "": with the tool_use's +// name recovered via tool_use_id, RawName must read "Edit" and the +// structuredPatch diff annotation must render. +func TestReadAll_GivenEditToolResultWithoutCommandName_ThenRawNameResolvedFromPrecedingToolUse(t *testing.T) { + path := filepath.Join(t.TempDir(), "session.jsonl") + lines := []string{ + `{"type":"assistant","timestamp":"2026-07-15T00:00:00Z","message":{"role":"assistant","content":[` + + `{"type":"tool_use","id":"toolu_edit1","name":"Edit","input":{"file_path":"/repo/src/lib/app-metadata.ts"}}` + + `]}}`, + `{"type":"user","timestamp":"2026-07-15T00:00:01Z","toolUseResult":{` + + `"filePath":"/repo/src/lib/app-metadata.ts","oldString":"a","newString":"b","originalFile":"a",` + + `"replaceAll":false,"userModified":false,` + + `"structuredPatch":[{"oldStart":10,"oldLines":3,"newStart":10,"newLines":4,` + + `"lines":[" line1","-old line","+new line","+another new"," line2"]}]` + + `},"message":{"role":"user","content":[{"type":"tool_result","tool_use_id":"toolu_edit1",` + + `"content":"The file /repo/src/lib/app-metadata.ts has been updated successfully."}]}}`, + "", + } + if err := os.WriteFile(path, []byte(strings.Join(lines, "\n")), 0o644); err != nil { + t.Fatalf("write fixture: %v", err) + } + + events, err := ReadAll(path) + if err != nil { + t.Fatalf("ReadAll returned error: %v", err) + } + if len(events) != 2 || events[1].Tool == nil { + t.Fatalf("events = %#v, want an assistant event followed by a tool_result event", events) + } + + tool := events[1].Tool + if tool.RawName != session.ToolEdit { + t.Fatalf("RawName = %q, want %q (resolved from preceding tool_use)", tool.RawName, session.ToolEdit) + } + wantSummary := " -> ok (+2, -1 @ L10)" + if got := tool.Summary(); got != wantSummary { + t.Fatalf("Summary() = %q, want %q", got, wantSummary) + } +} + +// TestReadAll_GivenReadToolResultWithoutCommandName_ThenBareOkSuppressionApplies +// guards the companion regression: Summary()'s ADR-002 bare "-> ok" +// suppression for Read switches on RawName, so with RawName stuck at "" on +// real transcripts the tool's boilerplate confirmation text ("has been +// updated successfully...") leaked into the rendered summary instead of +// being suppressed. +func TestReadAll_GivenReadToolResultWithoutCommandName_ThenBareOkSuppressionApplies(t *testing.T) { + path := filepath.Join(t.TempDir(), "session.jsonl") + lines := []string{ + `{"type":"assistant","timestamp":"2026-07-15T00:00:00Z","message":{"role":"assistant","content":[` + + `{"type":"tool_use","id":"toolu_read1","name":"Read","input":{"file_path":"/repo/README.md"}}` + + `]}}`, + `{"type":"user","timestamp":"2026-07-15T00:00:01Z","toolUseResult":{` + + `"type":"text","file":{"content":"# README","numLines":1}` + + `},"message":{"role":"user","content":[{"type":"tool_result","tool_use_id":"toolu_read1",` + + `"content":" 1\t# README"}]}}`, + "", + } + if err := os.WriteFile(path, []byte(strings.Join(lines, "\n")), 0o644); err != nil { + t.Fatalf("write fixture: %v", err) + } + + events, err := ReadAll(path) + if err != nil { + t.Fatalf("ReadAll returned error: %v", err) + } + if len(events) != 2 || events[1].Tool == nil { + t.Fatalf("events = %#v, want an assistant event followed by a tool_result event", events) + } + + tool := events[1].Tool + if tool.RawName != session.ToolRead { + t.Fatalf("RawName = %q, want %q (resolved from preceding tool_use)", tool.RawName, session.ToolRead) + } + wantSummary := " -> ok" + if got := tool.Summary(); got != wantSummary { + t.Fatalf("Summary() = %q, want %q (boilerplate body content must be suppressed)", got, wantSummary) + } +} + func TestParseLine_UserAnswer(t *testing.T) { event := parseLine(t, `{"type":"user","toolUseResult":{"success":true},"message":{"role":"user","content":[{"type":"tool_result","tool_use_id":"tool-3","content":"User has answered your questions: ship it"}]}}`) if event.User == nil || !event.User.IsAnswer || event.User.Text != "User has answered your questions: ship it" { diff --git a/internal/session/event.go b/internal/session/event.go index 2840e7f..ffe7251 100644 --- a/internal/session/event.go +++ b/internal/session/event.go @@ -189,6 +189,31 @@ type ToolResult struct { Success bool Text string RawName string + + // DiffStat carries the parsed structuredPatch summary for a successful + // Edit/Write result (ADR-003 decision 3), computed by the codec from + // toolUseResult. Nil means no diff info is available (wrong tool, or + // structuredPatch/content missing or unparsable) — Summary() then falls + // back to the bare status line it always rendered. + DiffStat *DiffStat +} + +// DiffStat summarizes a successful Edit's structuredPatch hunks or a +// successful Write's new-file content. +type DiffStat struct { + // IsNewFile distinguishes a Write result (new file, line count from + // content) from an Edit result (hunk-based +/- counts). + IsNewFile bool + + // Edit fields: +/- line counts summed across every hunk, NewStartLine + // taken from the first hunk, HunkCount the total number of hunks. + Additions int + Deletions int + NewStartLine int + HunkCount int + + // NewFileLines is Write's new-file line count, derived from content. + NewFileLines int } func (r ToolResult) Status() string { @@ -209,6 +234,9 @@ const ( func (r ToolResult) Summary() string { if r.Success { + if diff := r.diffSummary(); diff != "" { + return diff + } switch r.RawName { case ToolRead, ToolWrite, ToolEdit, ToolAgent: return fmt.Sprintf(" -> %s", r.Status()) @@ -224,6 +252,24 @@ func (r ToolResult) Summary() string { return fmt.Sprintf(" -> %s", r.Status()) } +// diffSummary renders the ADR-003 decision 3 diff/new-file annotation for a +// successful Edit/Write result. Returns "" when DiffStat wasn't computed (the +// codec couldn't find a structuredPatch/content to parse), so Summary() falls +// back to the bare status line. +func (r ToolResult) diffSummary() string { + if r.DiffStat == nil { + return "" + } + if r.DiffStat.IsNewFile { + return fmt.Sprintf(" -> %s (new file, %d lines)", r.Status(), r.DiffStat.NewFileLines) + } + summary := fmt.Sprintf(" -> %s (+%d, -%d @ L%d", r.Status(), r.DiffStat.Additions, r.DiffStat.Deletions, r.DiffStat.NewStartLine) + if r.DiffStat.HunkCount > 1 { + summary += fmt.Sprintf(", %d hunks", r.DiffStat.HunkCount) + } + return summary + ")" +} + // catLineNumberPrefix matches "cat -n" style line-number prefixes (" 12\t") // that Read/Edit tool output prepends to every line — noise, not error content. var catLineNumberPrefix = regexp.MustCompile(`^\s*\d+\t`) diff --git a/internal/session/event_test.go b/internal/session/event_test.go index a80fa0b..2592579 100644 --- a/internal/session/event_test.go +++ b/internal/session/event_test.go @@ -350,6 +350,67 @@ func TestToolResultSummary_GivenFailureTextWithNoiseLines_ThenExcerptSkipsNoiseA } } +// --- ADR-003 decision 3: diff summaries for Edit/Write --- + +// TestToolResultSummary_GivenEditDiffStat_ThenRendersDiffAnnotation pins the +// Edit diff summary format: "+A, -D @ L" for a single hunk, and the +// same with a trailing ", H hunks" once more than one hunk is present. +func TestToolResultSummary_GivenEditDiffStat_ThenRendersDiffAnnotation(t *testing.T) { + tests := []struct { + name string + result ToolResult + want string + }{ + { + name: "given single hunk then omits hunk count", + result: ToolResult{Success: true, RawName: ToolEdit, DiffStat: &DiffStat{ + Additions: 2, Deletions: 1, NewStartLine: 10, HunkCount: 1, + }}, + want: " -> ok (+2, -1 @ L10)", + }, + { + name: "given multiple hunks then appends hunk count", + result: ToolResult{Success: true, RawName: ToolEdit, DiffStat: &DiffStat{ + Additions: 5, Deletions: 3, NewStartLine: 5, HunkCount: 2, + }}, + want: " -> ok (+5, -3 @ L5, 2 hunks)", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.result.Summary(); got != tt.want { + t.Fatalf("Summary() = %q, want %q", got, tt.want) + } + }) + } +} + +// TestToolResultSummary_GivenWriteDiffStatNewFile_ThenRendersLineCount pins +// the Write new-file summary format from ADR-003 decision 3. +func TestToolResultSummary_GivenWriteDiffStatNewFile_ThenRendersLineCount(t *testing.T) { + result := ToolResult{Success: true, RawName: ToolWrite, DiffStat: &DiffStat{ + IsNewFile: true, NewFileLines: 42, + }} + want := " -> ok (new file, 42 lines)" + if got := result.Summary(); got != want { + t.Fatalf("Summary() = %q, want %q", got, want) + } +} + +// TestToolResultSummary_GivenNoDiffStat_ThenFallsBackToBareOk guards the +// ADR-003 decision 3 fallback: when the codec couldn't parse structuredPatch +// (missing or unparsable), DiffStat is nil and Summary() must not panic or +// render a bogus diff annotation — it keeps the plain "-> ok" the tool +// already got before diff summaries existed. +func TestToolResultSummary_GivenNoDiffStat_ThenFallsBackToBareOk(t *testing.T) { + result := ToolResult{Success: true, RawName: ToolEdit, Text: "irrelevant body"} + want := " -> ok" + if got := result.Summary(); got != want { + t.Fatalf("Summary() = %q, want %q", got, want) + } +} + func TestCompactTaskNotification_GivenFullNotification_ThenKeepsSummaryAndResult(t *testing.T) { input := ` ad4760fe24f754e27 diff --git a/internal/summarizer/summarizer.go b/internal/summarizer/summarizer.go index 50ffc67..c849c86 100644 --- a/internal/summarizer/summarizer.go +++ b/internal/summarizer/summarizer.go @@ -4,6 +4,7 @@ package summarizer import ( "fmt" "path/filepath" + "sort" "strings" "github.com/Mapleeeeeeeeeee/cc-session-reader/internal/session" @@ -13,6 +14,14 @@ const ( maxCommandLen = 80 maxSkillLen = 80 maxQuestionLen = 90 + + // unknownToolMaxKeys caps how many input keys are shown for a tool with no + // dedicated summarizer branch (ADR-003 decision 4) — enough to identify + // the call without dumping its whole payload. + unknownToolMaxKeys = 3 + // unknownToolValueMaxLen truncates each shown value so a large payload + // (e.g. a long prompt) can't blow up the one-line fallback summary. + unknownToolValueMaxLen = 60 ) func cleanPath(path string, cwd string) string { @@ -158,6 +167,33 @@ func SummarizeToolUse(name string, inp session.ToolInput, cwd string) string { return fmt.Sprintf("[ToolSearch] %s", query) default: - return fmt.Sprintf("[%s]", name) + return fmt.Sprintf("[%s]%s", name, summarizeUnknownInput(inp)) + } +} + +// summarizeUnknownInput renders the first few input key/value pairs for a +// tool with no dedicated summarizer branch (ADR-003 decision 4), so tools +// added to Claude Code after this release degrade gracefully instead of +// silently losing all input context. Returns "" for an empty input map. +// Keys are sorted alphabetically: map iteration order is not stable in Go, +// and an unstable key order would make the rendered summary flaky. +func summarizeUnknownInput(inp session.ToolInput) string { + if len(inp.Raw) == 0 { + return "" + } + keys := make([]string, 0, len(inp.Raw)) + for key := range inp.Raw { + keys = append(keys, key) + } + sort.Strings(keys) + if len(keys) > unknownToolMaxKeys { + keys = keys[:unknownToolMaxKeys] + } + + pairs := make([]string, 0, len(keys)) + for _, key := range keys { + value := session.Truncate(fmt.Sprintf("%v", inp.Raw[key]), unknownToolValueMaxLen) + pairs = append(pairs, fmt.Sprintf("%s=%s", key, value)) } + return " " + strings.Join(pairs, ", ") } diff --git a/internal/summarizer/summarizer_test.go b/internal/summarizer/summarizer_test.go index d915d6e..739e2d8 100644 --- a/internal/summarizer/summarizer_test.go +++ b/internal/summarizer/summarizer_test.go @@ -193,7 +193,7 @@ func TestSummarizeToolUse_OtherTools(t *testing.T) { {name: "Grep empty pattern", toolName: "Grep", inp: toolInput(nil), want: `[Grep] "?"`}, {name: "Glob", toolName: "Glob", inp: toolInput(map[string]any{"pattern": "**/*.go"}), want: "[Glob] **/*.go"}, {name: "ToolSearch", toolName: "ToolSearch", inp: toolInput(map[string]any{"query": "react docs"}), want: "[ToolSearch] react docs"}, - {name: "Unknown", toolName: "WebSearch", inp: toolInput(nil), want: "[WebSearch]"}, + {name: "Unknown with no input", toolName: "WebSearch", inp: toolInput(nil), want: "[WebSearch]"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -204,3 +204,49 @@ func TestSummarizeToolUse_OtherTools(t *testing.T) { }) } } + +// --- ADR-003 decision 4: unknown-tool fallback shows input key/value pairs --- + +// TestSummarizeToolUse_GivenUnknownToolWithInput_ThenShowsFirstKeysSortedAndTruncated +// guards the bug described in ADR-003: before this fix, a tool with no +// dedicated summarizer branch rendered "[ToolName]" with the input completely +// discarded, so a future Claude Code tool would silently lose all context. +// Map iteration order is not stable in Go, so this also pins that the key +// order in the rendered output is deterministic (alphabetical) rather than +// flaky across runs. +func TestSummarizeToolUse_GivenUnknownToolWithInput_ThenShowsFirstKeysSortedAndTruncated(t *testing.T) { + tests := []struct { + name string + inp session.ToolInput + want string + }{ + { + name: "given input under the key cap then shows every key sorted alphabetically", + inp: toolInput(map[string]any{"query": "cats", "num_results": float64(5)}), + want: "[WebSearch] num_results=5, query=cats", + }, + { + name: "given more keys than the cap then keeps only the first alphabetically", + inp: toolInput(map[string]any{"z_last": "z", "a_first": "a", "m_mid": "m", "b_second": "b"}), + want: "[WebSearch] a_first=a, b_second=b, m_mid=m", + }, + { + name: "given a long value then truncates it to the 60-rune budget", + inp: toolInput(map[string]any{"payload": strings.Repeat("x", 100)}), + want: "[WebSearch] payload=" + strings.Repeat("x", 60), + }, + { + name: "given empty input map then returns bare tag", + inp: toolInput(map[string]any{}), + want: "[WebSearch]", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := SummarizeToolUse("WebSearch", tt.inp, "") + if got != tt.want { + t.Fatalf("got %q, want %q", got, tt.want) + } + }) + } +} From 40f5497d90720989b0d152a645e08c5649f890da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maple=EF=BC=81?= Date: Wed, 15 Jul 2026 16:24:49 +0800 Subject: [PATCH 03/16] fix: emit minimal context header when session metadata is missing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit writeContextHeader silently skipped the header whenever the session-meta JSON was absent — and upstream stopped producing those files, so every recent session's context output started mid-dialogue with no hint of which project or conversation it was. The fallback header now derives session id, project (from event cwd, else transcript directory), and date (first non-empty event timestamp) from the transcript itself. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_013UQYeDtvg1CQYvirZ4jWC5 --- internal/formatter/context.go | 65 +++++++++++++++++++++-- internal/formatter/formatter_test.go | 79 ++++++++++++++++++++++++++++ 2 files changed, 141 insertions(+), 3 deletions(-) diff --git a/internal/formatter/context.go b/internal/formatter/context.go index 2f2f879..1c511c1 100644 --- a/internal/formatter/context.go +++ b/internal/formatter/context.go @@ -19,7 +19,7 @@ func FormatContextWithStore(transcriptPath string, sessionID string, maxLines in } var buf bytes.Buffer - writeContextHeader(sessionID, &buf, store) + writeContextHeader(sessionID, transcriptPath, events, &buf, store) if err := renderContextEvents(events, agentIDs, opts, &buf); err != nil { return err } @@ -94,11 +94,21 @@ func renderContextEvents(events []session.Event, agentIDs map[string]bool, opts return nil } -func writeContextHeader(sessionID string, out io.Writer, store parser.Store) { +// writeContextHeader writes the one-line session header that opens every +// context output. It prefers session_meta (richer: real project path, +// recorded duration) and falls back to a minimal header derived from the +// transcript itself when metadata is unavailable, so a fresh reader always +// gets a session id and project on line 1 instead of raw dialogue. +func writeContextHeader(sessionID string, transcriptPath string, events []session.Event, out io.Writer, store parser.Store) { meta, err := store.LoadSessionMeta(sessionID) - if err != nil || meta == nil { + if err == nil && meta != nil { + writeMetaContextHeader(sessionID, meta, out) return } + writeFallbackContextHeader(sessionID, transcriptPath, events, out) +} + +func writeMetaContextHeader(sessionID string, meta map[string]any, out io.Writer) { projectPath := jsonutil.GetStr(meta, "project_path") project := filepath.Base(projectPath) if project == "" || project == "." { @@ -111,3 +121,52 @@ func writeContextHeader(sessionID string, out io.Writer, store parser.Store) { shortID := session.ShortID(sessionID, 8) fmt.Fprintf(out, "# Session %s | %s | %sm\n\n", shortID, project, duration) } + +// writeFallbackContextHeader writes a minimal header sourced from the +// transcript itself: session id, project (from a tool call's recorded cwd, +// or the transcript's parent directory name as a last resort), and the date +// of the first event. Used when session metadata is missing — notably the +// ~/.claude/usage-data/session-meta/ upstream stopped writing new files as of +// 2026-07-04, so without this fallback every recent session's context output +// opened with raw dialogue and no indication of which project or session it +// belonged to. +func writeFallbackContextHeader(sessionID string, transcriptPath string, events []session.Event, out io.Writer) { + project := projectFromEventCwd(events) + if project == "" { + project = filepath.Base(filepath.Dir(transcriptPath)) + } + if project == "" || project == "." { + project = "?" + } + date := parser.FormatTimestamp(firstEventTimestamp(events)) + shortID := session.ShortID(sessionID, 8) + fmt.Fprintf(out, "# Session %s | %s | %s (no session metadata)\n\n", shortID, project, date) +} + +// firstEventTimestamp returns the first non-empty timestamp among events, in +// order. Leading noise events (mode/permission-mode/bridge-session/...) carry +// no timestamp field, so events[0].Timestamp alone is not reliable. +func firstEventTimestamp(events []session.Event) string { + for _, event := range events { + if event.Timestamp != "" { + return event.Timestamp + } + } + return "" +} + +// projectFromEventCwd returns the directory name of the first recorded tool +// call's working directory, or "" if no event carries one. +func projectFromEventCwd(events []session.Event) string { + for _, event := range events { + if event.Assistant == nil { + continue + } + for _, tool := range event.Assistant.ToolUses { + if tool.Cwd != "" { + return filepath.Base(tool.Cwd) + } + } + } + return "" +} diff --git a/internal/formatter/formatter_test.go b/internal/formatter/formatter_test.go index 4dacd4c..9537aa1 100644 --- a/internal/formatter/formatter_test.go +++ b/internal/formatter/formatter_test.go @@ -37,6 +37,9 @@ func TestFormatRead_WhenTranscriptHasDialogueAndToolUse_ThenWritesReadableTimeli } func TestFormatContext_WhenSessionMetadataExists_ThenWritesCompactContextWithHeader(t *testing.T) { + // Also guards the metadata-missing fallback (see below): when session_meta + // is present it must still win and produce the original full header, not + // the transcript-derived one. transcriptPath, metaDir := writeFormatterFixture(t) var out bytes.Buffer @@ -59,6 +62,82 @@ func TestFormatContext_WhenSessionMetadataExists_ThenWritesCompactContextWithHea } } +// TestFormatContext_WhenSessionMetadataMissing_ThenWritesMinimalHeaderFromTranscript +// guards a regression: session_meta files stopped being written by the +// upstream harness (production incident starting 2026-07-04), so +// writeContextHeader's silent "meta missing -> write nothing" branch left +// every recent session's context output opening directly with dialogue — +// a fresh reader had no idea which session or project it was looking at. +func TestFormatContext_WhenSessionMetadataMissing_ThenWritesMinimalHeaderFromTranscript(t *testing.T) { + root := t.TempDir() + sessionID := "deadbeef-dead-beef-dead-beefdeadbeef" + transcriptPath := filepath.Join(root, sessionID+".jsonl") + transcript := `{"type":"user","timestamp":"2026-07-10T09:30:00Z","message":{"role":"user","content":"hello"}} +{"type":"assistant","timestamp":"2026-07-10T09:30:01Z","cwd":"/Users/dev/my-project","message":{"role":"assistant","content":[{"type":"text","text":"hi"},{"type":"tool_use","name":"Bash","id":"tool-1","input":{"command":"echo ok"}}]}} +` + if err := os.WriteFile(transcriptPath, []byte(transcript), 0o644); err != nil { + t.Fatalf("write transcript: %v", err) + } + + var out bytes.Buffer + store := parser.Store{SessionMetaDir: filepath.Join(root, "no-such-meta-dir")} + if err := FormatContextWithStore(transcriptPath, sessionID, 0, 0, FormatOptions{}, &out, store, claudecodec.Codec{}); err != nil { + t.Fatalf("FormatContext returned error: %v", err) + } + + got := out.String() + shortID := session.ShortID(sessionID, 8) + if !strings.HasPrefix(got, "# Session "+shortID) { + t.Fatalf("fallback header must open the output with the session id\ngot:\n%q", got) + } + if !strings.Contains(got, "my-project") { + t.Fatalf("fallback header missing project derived from a tool call's cwd\ngot:\n%q", got) + } + if !strings.Contains(got, "07-10") { + t.Fatalf("fallback header missing date derived from the first event's timestamp\ngot:\n%q", got) + } +} + +// TestFormatContext_WhenSessionMetadataMissingAndTranscriptHasNoCwd_ThenDerivesProjectFromTranscriptDirectory +// covers the harder fallback case: no assistant tool call ever ran (so no +// event carries a cwd), so the project must come from the transcript's +// parent directory name instead. +// +// The leading "mode" line also guards a bug found during manual acceptance: +// real transcripts open with noise entries (mode/permission-mode/bridge- +// session/...) that carry no "timestamp" field, so reading events[0].Timestamp +// directly produced "??-?? ??:??" even though a real timestamp was a few +// lines down. +func TestFormatContext_WhenSessionMetadataMissingAndTranscriptHasNoCwd_ThenDerivesProjectFromTranscriptDirectory(t *testing.T) { + root := t.TempDir() + projectDir := filepath.Join(root, "projects", "-Users-dev-encoded-project") + if err := os.MkdirAll(projectDir, 0o755); err != nil { + t.Fatalf("create project dir: %v", err) + } + sessionID := "cafefeed-cafe-feed-cafe-feedcafefeed" + transcriptPath := filepath.Join(projectDir, sessionID+".jsonl") + transcript := `{"type":"mode","mode":"normal","sessionId":"` + sessionID + `"} +{"type":"user","timestamp":"2026-07-10T09:30:00Z","message":{"role":"user","content":"hello"}} +` + if err := os.WriteFile(transcriptPath, []byte(transcript), 0o644); err != nil { + t.Fatalf("write transcript: %v", err) + } + + var out bytes.Buffer + store := parser.Store{SessionMetaDir: filepath.Join(root, "no-such-meta-dir")} + if err := FormatContextWithStore(transcriptPath, sessionID, 0, 0, FormatOptions{}, &out, store, claudecodec.Codec{}); err != nil { + t.Fatalf("FormatContext returned error: %v", err) + } + + got := out.String() + if !strings.Contains(got, "-Users-dev-encoded-project") { + t.Fatalf("fallback header missing project derived from the transcript's directory\ngot:\n%q", got) + } + if !strings.Contains(got, "07-10") { + t.Fatalf("fallback header must skip timestamp-less leading noise events and use the first real event's date\ngot:\n%q", got) + } +} + func TestFormatRead_WhenMaxLinesReached_ThenStopsWithTruncationMessage(t *testing.T) { // Guards the new pagination format: offset N is shown in the truncation // message so the user knows what flag to pass to continue reading. From ffd1bee978a6e60590e10160abe928a95e3f6478 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maple=EF=BC=81?= Date: Wed, 15 Jul 2026 16:39:00 +0800 Subject: [PATCH 04/16] fix: compute list duration and preview without session metadata Session-meta JSON files stopped being produced upstream, so every recent session fell back to the header scan, which showed 0m durations and blank previews for inherit-style sessions. Duration now derives from the transcript's first/last timestamps (last one found via a bounded 16KB tail read, never a whole-file load). The preview scan no longer lets noise lines (command tags, skill injections, tool results, interrupt sentinels) consume the candidate budget, and falls back to a [/command args] preview when no genuine prompt exists. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_013UQYeDtvg1CQYvirZ4jWC5 --- internal/claudecodec/classify.go | 2 + internal/claudecodec/reader.go | 160 +++++++++++++++++++++------- internal/claudecodec/reader_test.go | 129 ++++++++++++++++++++++ internal/claudecodec/tail.go | 70 ++++++++++++ internal/claudecodec/tail_test.go | 116 ++++++++++++++++++++ internal/parser/parser_test.go | 31 ++++++ internal/parser/scan.go | 18 +++- internal/session/reader.go | 10 +- 8 files changed, 494 insertions(+), 42 deletions(-) create mode 100644 internal/claudecodec/tail.go create mode 100644 internal/claudecodec/tail_test.go diff --git a/internal/claudecodec/classify.go b/internal/claudecodec/classify.go index 5a8cd4b..9c98828 100644 --- a/internal/claudecodec/classify.go +++ b/internal/claudecodec/classify.go @@ -11,6 +11,8 @@ import ( const ( tagCommandNameOpen = "" tagCommandNameClose = "" + tagCommandArgsOpen = "" + tagCommandArgsClose = "" tagBashInputOpen = "" tagBashInputClose = "" tagLocalStdout = "" diff --git a/internal/claudecodec/reader.go b/internal/claudecodec/reader.go index b99d36f..7bbd1f6 100644 --- a/internal/claudecodec/reader.go +++ b/internal/claudecodec/reader.go @@ -181,19 +181,57 @@ func (Codec) ReadAll(path string) ([]session.Event, error) { return ReadAll(path) } -// commandTagPrefixes are JSONL user message content prefixes that indicate -// tool/command noise rather than real user prompts. -var commandTagPrefixes = []string{ - "", - "", - "", - "", - "", - "", +// headerScanCandidateBudget bounds how many genuine (non-noise) user-authored +// lines ScanHeader inspects before giving up on finding a real prompt. +// Known-noise shapes — command invocations/output, skill injections, tool +// results, empty text — don't consume this budget: a session that opens with +// a long run of harness noise before the first real question must not +// exhaust the budget on noise alone (see classifyCommandUserMessage / +// classifyHarnessUserMessage below for what counts as noise). +const headerScanCandidateBudget = 20 + +// headerScanMaxPhysicalLines hard-caps the raw lines ScanHeader reads, +// independent of headerScanCandidateBudget. Because noise no longer consumes +// the candidate budget, a session with no real user message at all (e.g. a +// fully automated inherit chain) needs this separate bound so the scan still +// terminates instead of reading toward EOF. +const headerScanMaxPhysicalLines = 200 + +// commandPreviewMaxRunes caps the synthesized command-fallback preview (see +// commandFallbackPreview) at the same width as the bang-command marker. +const commandPreviewMaxRunes = 80 + +// requestInterruptedPrefix marks the fixed harness sentinel Claude Code +// inserts as a user-role text block when a tool call is interrupted (seen as +// both "[Request interrupted by user]" and "[Request interrupted by user for +// tool use]" across real transcripts). It is structurally indistinguishable +// from a real question — same shape, same role — but it is never something +// the human typed, so ScanHeader must not pick it as the list preview ahead +// of an actual question or the command-fallback marker. +const requestInterruptedPrefix = "[Request interrupted by user" + +// headerLine is the minimal shape ScanHeader needs from a transcript line. +// Message reuses rawMessage so command/skill-injection classification (and +// its text normalization across string vs. block-array content) stays a +// single source of truth shared with the full parse path in classify.go. +type headerLine struct { + Type string `json:"type"` + Timestamp string `json:"timestamp"` + Message *rawMessage `json:"message"` } -// ScanHeader reads up to 20 lines of a JSONL transcript and returns the first -// timestamp and first real user prompt found. Implements session.HeaderScanner. +// ScanHeader reads a bounded prefix of a JSONL transcript and returns its +// first timestamp, its last timestamp (from a bounded tail read — see +// readLastTimestamp — not a full scan), and the first real user prompt +// found. Implements session.HeaderScanner. +// +// Known-noise user-role lines (command invocations/output, skill injections, +// tool results, empty text, interrupted-request sentinels) are skipped +// without consuming the scan budget. If no real prompt turns up within that +// budget, FirstUserPrompt falls back +// to a one-line preview of the first command invocation seen, e.g. +// "[/cc-session inherit 9cd01951]" — enough to identify the session in an +// inherit chain even when the real question lies far beyond the scan window. func (Codec) ScanHeader(path string) (*session.HeaderInfo, error) { f, err := os.Open(path) if err != nil { @@ -202,23 +240,19 @@ func (Codec) ScanHeader(path string) (*session.HeaderInfo, error) { defer f.Close() info := &session.HeaderInfo{} + var commandFallback string + candidateBudget := headerScanCandidateBudget + scanner := bufio.NewScanner(f) - linesRead := 0 - for scanner.Scan() && linesRead < 20 { - linesRead++ + physicalLines := 0 + for scanner.Scan() && physicalLines < headerScanMaxPhysicalLines { + physicalLines++ line := scanner.Bytes() if len(line) == 0 { continue } - var h struct { - Type string `json:"type"` - Timestamp string `json:"timestamp"` - Message *struct { - Role string `json:"role"` - Content json.RawMessage `json:"content"` - } `json:"message"` - } + var h headerLine if err := json.Unmarshal(line, &h); err != nil { continue } @@ -227,28 +261,82 @@ func (Codec) ScanHeader(path string) (*session.HeaderInfo, error) { info.Timestamp = h.Timestamp } - if info.FirstUserPrompt == "" && h.Message != nil && h.Message.Role == "user" && h.Type == "user" { - var text string - if err := json.Unmarshal(h.Message.Content, &text); err == nil { - if !hasCommandTagPrefix(text) { - info.FirstUserPrompt = text - } - } + if h.Type != "user" || h.Message == nil || h.Message.Role != "user" { + continue } - if info.Timestamp != "" && info.FirstUserPrompt != "" { + text := h.Message.Text() + + if classified := classifyCommandUserMessage(text); classified != nil { + captureCommandFallback(&commandFallback, text) + continue + } + if classified := classifyHarnessUserMessage(text); classified != nil { + captureCommandFallback(&commandFallback, text) + continue + } + trimmed := strings.TrimSpace(text) + if trimmed == "" || strings.HasPrefix(trimmed, requestInterruptedPrefix) { + continue + } + + if info.FirstUserPrompt == "" { + info.FirstUserPrompt = text + } + candidateBudget-- + if candidateBudget <= 0 || (info.Timestamp != "" && info.FirstUserPrompt != "") { break } } + if info.FirstUserPrompt == "" && commandFallback != "" { + info.FirstUserPrompt = commandFallback + } + + if ts, err := readLastTimestamp(f); err == nil && ts != "" { + info.EndTimestamp = ts + } + return info, nil } -func hasCommandTagPrefix(s string) bool { - for _, prefix := range commandTagPrefixes { - if strings.HasPrefix(s, prefix) { - return true - } +// captureCommandFallback records the first command invocation's one-line +// preview into *fallback. It is a no-op once a preview has already been +// captured, and for lines that carry no tag at all (e.g. +// skill injections, which reach this call too since both known-noise +// branches in ScanHeader route through it). +func captureCommandFallback(fallback *string, text string) { + if *fallback != "" { + return + } + if preview := commandFallbackPreview(text); preview != "" { + *fallback = preview + } +} + +// commandFallbackPreview extracts the / pair from +// a command invocation entry and renders it as a compact marker, e.g. +// "[/cc-session inherit 9cd01951]". Returns "" when text carries no +// tag (the caller then leaves the fallback unset). +func commandFallbackPreview(text string) string { + name := strings.TrimSpace(extractBetween(text, tagCommandNameOpen, tagCommandNameClose)) + if name == "" { + return "" + } + marker := name + if args := strings.TrimSpace(extractBetween(text, tagCommandArgsOpen, tagCommandArgsClose)); args != "" { + marker += " " + shortenCommandArgs(args) + } + return "[" + session.Truncate(marker, commandPreviewMaxRunes) + "]" +} + +// shortenCommandArgs truncates each whitespace-separated argument token to +// session.ShortID's 8-rune convention, so a UUID session-ID argument reads as +// its identifying prefix instead of the full 36-character value. +func shortenCommandArgs(args string) string { + fields := strings.Fields(args) + for i, field := range fields { + fields[i] = session.ShortID(field, 8) } - return false + return strings.Join(fields, " ") } diff --git a/internal/claudecodec/reader_test.go b/internal/claudecodec/reader_test.go index 8aabf48..10d186d 100644 --- a/internal/claudecodec/reader_test.go +++ b/internal/claudecodec/reader_test.go @@ -836,6 +836,135 @@ func parseLineWithText(t *testing.T, text string) session.Event { return parseLine(t, line) } +// --- ScanHeader: tail-read duration + noise-resilient prompt discovery --- + +// TestScanHeader_GivenMultiLineTranscript_ThenEndTimestampIsLastLineTimestamp +// pins the new EndTimestamp field: ScanHeader must report the transcript's +// last timestamp (not just its first), since list.go's duration column has +// nothing else to compute from once metadata is unavailable. +func TestScanHeader_GivenMultiLineTranscript_ThenEndTimestampIsLastLineTimestamp(t *testing.T) { + path := filepath.Join(t.TempDir(), "session.jsonl") + lines := []string{ + `{"type":"user","timestamp":"2026-07-15T02:00:00.000Z","message":{"role":"user","content":"start the task"}}`, + `{"type":"assistant","timestamp":"2026-07-15T02:05:00.000Z","message":{"role":"assistant","content":[{"type":"text","text":"done"}]}}`, + "", + } + if err := os.WriteFile(path, []byte(strings.Join(lines, "\n")), 0o644); err != nil { + t.Fatalf("write fixture: %v", err) + } + + info, err := (Codec{}).ScanHeader(path) + if err != nil { + t.Fatalf("ScanHeader returned error: %v", err) + } + if info.Timestamp != "2026-07-15T02:00:00.000Z" { + t.Fatalf("Timestamp = %q, want the first line's timestamp", info.Timestamp) + } + if info.EndTimestamp != "2026-07-15T02:05:00.000Z" { + t.Fatalf("EndTimestamp = %q, want the last line's timestamp", info.EndTimestamp) + } +} + +// inheritChainNoiseLines returns the noise-only opening of a `/cc-session +// inherit` session transcript: a command invocation, a skill injection, and +// twenty tool_result turns — the shape observed in real inherit chains +// (session 2b73acc2 in the architecture note), none of which is a real human +// question. Twenty-two lines exceeds the old fixed 20-raw-line scan window. +func inheritChainNoiseLines() []string { + lines := []string{ + `{"type":"user","timestamp":"2026-07-15T02:00:00.000Z","message":{"role":"user",` + + `"content":"cc-session\n/cc-session\n` + + `inherit 9cd01951-e149-4d83-84e2-a210818d02aa"}}`, + `{"type":"user","timestamp":"2026-07-15T02:00:01.000Z","message":{"role":"user","content":[` + + `{"type":"text","text":"Base directory for this skill: /Users/maple/.claude/skills/cc-session\n\n# Session Reader"}]}}`, + } + for i := 0; i < 20; i++ { + lines = append(lines, fmt.Sprintf( + `{"type":"user","timestamp":"2026-07-15T02:01:%02dZ","toolUseResult":{"success":true},`+ + `"message":{"role":"user","content":[{"type":"tool_result","tool_use_id":"toolu_%d","content":"ok"}]}}`, + i%60, i)) + } + return lines +} + +// TestScanHeader_GivenNoiseHeavyPrefix_ThenNoiseLinesDoNotConsumeScanBudget +// guards the bug where every scanned line — including command tags, skill +// injections, and tool-result-shaped user entries — consumed one slot of the +// old fixed 20-line window. An inherit-style session opens with a command +// invocation, a skill injection, and a run of tool_result turns before the +// real human question; the window used to exhaust before ever reaching it, +// leaving [refs] sessions like 2b73acc2 with a blank list preview. +func TestScanHeader_GivenNoiseHeavyPrefix_ThenNoiseLinesDoNotConsumeScanBudget(t *testing.T) { + lines := inheritChainNoiseLines() + lines = append(lines, + `{"type":"user","timestamp":"2026-07-15T02:10:00.000Z","message":{"role":"user",`+ + `"content":"可以搭配 trigger 一起看 @docs/trigger-modernization-report.md"}}`, + "") + path := filepath.Join(t.TempDir(), "session.jsonl") + if err := os.WriteFile(path, []byte(strings.Join(lines, "\n")), 0o644); err != nil { + t.Fatalf("write fixture: %v", err) + } + + info, err := (Codec{}).ScanHeader(path) + if err != nil { + t.Fatalf("ScanHeader returned error: %v", err) + } + want := "可以搭配 trigger 一起看 @docs/trigger-modernization-report.md" + if info.FirstUserPrompt != want { + t.Fatalf("FirstUserPrompt = %q, want %q (noise-heavy prefix must not exhaust the scan budget)", info.FirstUserPrompt, want) + } +} + +// TestScanHeader_GivenOnlyCommandNoiseAndNoRealQuestion_ThenFallsBackToCommandPreview +// pins the layer-2 fallback: when the real question never appears within the +// scan window at all (the common case for inherit-chain sessions, where the +// real prompt can be dozens of turns later), FirstUserPrompt must still carry +// enough information to identify the session instead of staying blank. +func TestScanHeader_GivenOnlyCommandNoiseAndNoRealQuestion_ThenFallsBackToCommandPreview(t *testing.T) { + lines := append(inheritChainNoiseLines(), "") + path := filepath.Join(t.TempDir(), "session.jsonl") + if err := os.WriteFile(path, []byte(strings.Join(lines, "\n")), 0o644); err != nil { + t.Fatalf("write fixture: %v", err) + } + + info, err := (Codec{}).ScanHeader(path) + if err != nil { + t.Fatalf("ScanHeader returned error: %v", err) + } + want := "[/cc-session inherit 9cd01951]" + if info.FirstUserPrompt != want { + t.Fatalf("FirstUserPrompt = %q, want %q (command-invocation fallback preview)", info.FirstUserPrompt, want) + } +} + +// TestScanHeader_GivenInterruptedToolCallSentinel_ThenTreatsItAsNoiseNotAPrompt +// guards a bug caught during real-transcript acceptance testing of session +// 2b73acc2: Claude Code inserts a fixed "[Request interrupted by user]" +// text block (same role/shape as a real question) whenever a tool call gets +// interrupted. Before this fix that sentinel was accepted as the first +// "genuine" candidate, so the list preview showed the harness's own +// boilerplate instead of falling through to the command-invocation preview. +func TestScanHeader_GivenInterruptedToolCallSentinel_ThenTreatsItAsNoiseNotAPrompt(t *testing.T) { + lines := inheritChainNoiseLines() + lines = append(lines, + `{"type":"user","timestamp":"2026-07-15T02:05:14.315Z","message":{"role":"user","content":[`+ + `{"type":"text","text":"[Request interrupted by user]"}]}}`, + "") + path := filepath.Join(t.TempDir(), "session.jsonl") + if err := os.WriteFile(path, []byte(strings.Join(lines, "\n")), 0o644); err != nil { + t.Fatalf("write fixture: %v", err) + } + + info, err := (Codec{}).ScanHeader(path) + if err != nil { + t.Fatalf("ScanHeader returned error: %v", err) + } + want := "[/cc-session inherit 9cd01951]" + if info.FirstUserPrompt != want { + t.Fatalf("FirstUserPrompt = %q, want %q (interrupted-request sentinel must not be mistaken for a real prompt)", info.FirstUserPrompt, want) + } +} + func parseLine(t *testing.T, line string) session.Event { t.Helper() event, ok, err := ParseLine([]byte(line)) diff --git a/internal/claudecodec/tail.go b/internal/claudecodec/tail.go new file mode 100644 index 0000000..547e398 --- /dev/null +++ b/internal/claudecodec/tail.go @@ -0,0 +1,70 @@ +package claudecodec + +import ( + "bytes" + "encoding/json" + "io" + "os" +) + +// tailReadBytes bounds how many trailing bytes of a transcript are read to +// locate its last timestamp. Transcripts can run to many megabytes, and +// `list` scans every session file on every invocation, so seeking straight +// to a small tail window keeps duration computation O(1) instead of +// O(file size). +const tailReadBytes = 16 * 1024 + +// tailReadOffset returns the byte offset to seek to before reading the tail +// of a file of the given size, so at most tailReadBytes trailing bytes are +// ever read. +func tailReadOffset(size int64) int64 { + if size <= tailReadBytes { + return 0 + } + return size - tailReadBytes +} + +// readLastTimestamp scans backward through the trailing tailReadBytes of f +// for the last JSONL entry carrying a non-empty "timestamp" field, skipping +// entries (bridge-session, last-prompt, ...) that carry none. Returns "" +// with a nil error if the tail window contains no timestamped entry. +func readLastTimestamp(f *os.File) (string, error) { + stat, err := f.Stat() + if err != nil { + return "", err + } + + offset := tailReadOffset(stat.Size()) + if _, err := f.Seek(offset, io.SeekStart); err != nil { + return "", err + } + chunk, err := io.ReadAll(f) + if err != nil { + return "", err + } + + lines := bytes.Split(chunk, []byte("\n")) + if offset > 0 && len(lines) > 0 { + // Seeking into the middle of the file lands mid-record: the first + // fragment is a partial line and must be discarded rather than + // parsed as JSON. + lines = lines[1:] + } + + for i := len(lines) - 1; i >= 0; i-- { + line := bytes.TrimSpace(lines[i]) + if len(line) == 0 { + continue + } + var h struct { + Timestamp string `json:"timestamp"` + } + if err := json.Unmarshal(line, &h); err != nil { + continue + } + if h.Timestamp != "" { + return h.Timestamp, nil + } + } + return "", nil +} diff --git a/internal/claudecodec/tail_test.go b/internal/claudecodec/tail_test.go new file mode 100644 index 0000000..448ea65 --- /dev/null +++ b/internal/claudecodec/tail_test.go @@ -0,0 +1,116 @@ +package claudecodec + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestTailReadOffset_GivenSizeWithinBudget_ThenReturnsZero(t *testing.T) { + if got := tailReadOffset(tailReadBytes); got != 0 { + t.Fatalf("tailReadOffset(%d) = %d, want 0", tailReadBytes, got) + } +} + +func TestTailReadOffset_GivenSizeBeyondBudget_ThenReturnsTrailingWindowStart(t *testing.T) { + size := int64(tailReadBytes*3 + 17) + want := size - tailReadBytes + if got := tailReadOffset(size); got != want { + t.Fatalf("tailReadOffset(%d) = %d, want %d", size, got, want) + } +} + +func TestReadLastTimestamp_GivenSmallFile_ThenReturnsLastLineTimestamp(t *testing.T) { + path := filepath.Join(t.TempDir(), "session.jsonl") + lines := []string{ + `{"type":"user","timestamp":"2026-07-15T02:00:00.000Z","message":{"role":"user","content":"hi"}}`, + `{"type":"assistant","timestamp":"2026-07-15T02:03:00.000Z","message":{"role":"assistant","content":[{"type":"text","text":"hi back"}]}}`, + "", + } + if err := os.WriteFile(path, []byte(strings.Join(lines, "\n")), 0o644); err != nil { + t.Fatalf("write fixture: %v", err) + } + f, err := os.Open(path) + if err != nil { + t.Fatalf("open fixture: %v", err) + } + defer f.Close() + + got, err := readLastTimestamp(f) + if err != nil { + t.Fatalf("readLastTimestamp returned error: %v", err) + } + if got != "2026-07-15T02:03:00.000Z" { + t.Fatalf("readLastTimestamp = %q, want the last line's timestamp", got) + } +} + +// TestReadLastTimestamp_GivenTrailingLinesWithoutTimestamp_ThenSkipsBackToPriorTimestampedLine +// guards the backward-scan step: real transcripts end with bridge-session/ +// last-prompt entries carrying no "timestamp" field at all, so the tail scan +// must walk past them to the nearest timestamped line rather than giving up. +func TestReadLastTimestamp_GivenTrailingLinesWithoutTimestamp_ThenSkipsBackToPriorTimestampedLine(t *testing.T) { + path := filepath.Join(t.TempDir(), "session.jsonl") + lines := []string{ + `{"type":"user","timestamp":"2026-07-15T02:00:00.000Z","message":{"role":"user","content":"hi"}}`, + `{"type":"assistant","timestamp":"2026-07-15T02:03:00.000Z","message":{"role":"assistant","content":[{"type":"text","text":"hi back"}]}}`, + `{"type":"bridge-session","sessionId":"abc"}`, + `{"type":"last-prompt"}`, + "", + } + if err := os.WriteFile(path, []byte(strings.Join(lines, "\n")), 0o644); err != nil { + t.Fatalf("write fixture: %v", err) + } + f, err := os.Open(path) + if err != nil { + t.Fatalf("open fixture: %v", err) + } + defer f.Close() + + got, err := readLastTimestamp(f) + if err != nil { + t.Fatalf("readLastTimestamp returned error: %v", err) + } + if got != "2026-07-15T02:03:00.000Z" { + t.Fatalf("readLastTimestamp = %q, want the last *timestamped* line, skipping trailing entries without one", got) + } +} + +// TestReadLastTimestamp_GivenTimestampOnlyOutsideTailWindow_ThenReturnsEmpty +// is the O(1) regression guard: transcripts can run to many megabytes, and +// `list` scans every session file, so duration computation must not read the +// whole file just to find its last timestamp. Only a decoy line near the +// start of the fixture carries a timestamp; everything inside the true +// trailing tailReadBytes window is timestamp-less noise. A correct +// tail-bounded read never sees the decoy and must return "" — if it did, the +// read walked further back than the documented window, defeating the point +// of bounding duration computation for large transcripts. +func TestReadLastTimestamp_GivenTimestampOnlyOutsideTailWindow_ThenReturnsEmpty(t *testing.T) { + path := filepath.Join(t.TempDir(), "session.jsonl") + + decoyLine := `{"type":"user","timestamp":"2020-01-01T00:00:00.000Z","message":{"role":"user","content":"decoy"}}` + "\n" + noiseLine := `{"type":"noise","note":"no timestamp field here"}` + "\n" + // Enough noise-only bytes after the decoy to push it outside the tail + // window, however large tailReadBytes is configured to be. + noiseTail := strings.Repeat(noiseLine, (tailReadBytes*2)/len(noiseLine)+1) + + content := decoyLine + noiseTail + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatalf("write fixture: %v", err) + } + + f, err := os.Open(path) + if err != nil { + t.Fatalf("open fixture: %v", err) + } + defer f.Close() + + got, err := readLastTimestamp(f) + if err != nil { + t.Fatalf("readLastTimestamp returned error: %v", err) + } + if got != "" { + t.Fatalf("readLastTimestamp = %q, want empty (decoy timestamp lies outside the tail window and must not be found)", got) + } +} diff --git a/internal/parser/parser_test.go b/internal/parser/parser_test.go index 7b8e055..20b9ce8 100644 --- a/internal/parser/parser_test.go +++ b/internal/parser/parser_test.go @@ -533,6 +533,37 @@ func TestScanTranscriptHeaders_GivenCommandMessage_ThenSkipsToNextUserMessage(t } } +// TestScanTranscriptHeaders_GivenNoSessionMeta_ThenComputesDurationFromTranscriptTimestamps +// guards the bug where the JSONL-fallback path never populated +// DurationMinutes at all (it only ever came from session-meta files), so +// `list` rendered "0m" for every session once the upstream meta-file writer +// stopped producing new files. The fixture's first and last timestamps are +// seven minutes apart. +func TestScanTranscriptHeaders_GivenNoSessionMeta_ThenComputesDurationFromTranscriptTimestamps(t *testing.T) { + root := t.TempDir() + projectDir := filepath.Join(root, "projects", "-Users-me-proj") + if err := os.MkdirAll(projectDir, 0o755); err != nil { + t.Fatalf("create project dir: %v", err) + } + sid := "12121212-1212-1212-1212-121212121212" + lines := []string{ + `{"type":"user","timestamp":"2026-07-15T02:00:00.000Z","message":{"role":"user","content":"start"}}`, + `{"type":"assistant","timestamp":"2026-07-15T02:07:00.000Z","message":{"role":"assistant","content":[{"type":"text","text":"done"}]}}`, + "", + } + writeFile(t, filepath.Join(projectDir, sid+".jsonl"), strings.Join(lines, "\n")) + + store := Store{ProjectsDir: filepath.Join(root, "projects"), HeaderScanner: claudecodec.Codec{}} + entries := store.ScanTranscriptHeaders() + + if len(entries) != 1 { + t.Fatalf("ScanTranscriptHeaders returned %d entries, want 1", len(entries)) + } + if entries[0].DurationMinutes != 7 { + t.Fatalf("DurationMinutes = %d, want 7 (computed from first/last transcript timestamps)", entries[0].DurationMinutes) + } +} + func TestScanTranscriptHeaders_GivenDuplicateUUID_ThenDeduplicates(t *testing.T) { root := t.TempDir() projA := filepath.Join(root, "projects", "-Users-me-alpha") diff --git a/internal/parser/scan.go b/internal/parser/scan.go index 314a647..86d15a4 100644 --- a/internal/parser/scan.go +++ b/internal/parser/scan.go @@ -24,11 +24,12 @@ type SessionListEntry struct { } // ScanTranscriptHeaders walks ProjectsDir for .jsonl files and extracts a -// SessionListEntry from the first 20 lines of each file via the Store's -// HeaderScanner. Files that cannot be opened or parsed are silently skipped. -// Sessions with the same UUID in multiple project directories are deduplicated -// (first walk hit wins). Returns nil when ProjectsDir is empty or no -// HeaderScanner is configured. +// SessionListEntry from a bounded prefix of each file via the Store's +// HeaderScanner, with DurationMinutes computed from the scanner's first and +// last transcript timestamps. Files that cannot be opened or parsed are +// silently skipped. Sessions with the same UUID in multiple project +// directories are deduplicated (first walk hit wins). Returns nil when +// ProjectsDir is empty or no HeaderScanner is configured. func (s Store) ScanTranscriptHeaders() []SessionListEntry { if s.ProjectsDir == "" || s.HeaderScanner == nil { return nil @@ -62,6 +63,13 @@ func (s Store) ScanTranscriptHeaders() []SessionListEntry { entry.StartTime = header.Timestamp if t, err := parseISO(header.Timestamp); err == nil { entry.StartTimeParsed = t + if header.EndTimestamp != "" { + if end, err := parseISO(header.EndTimestamp); err == nil { + if elapsed := end.Sub(t); elapsed >= 0 { + entry.DurationMinutes = int(elapsed.Minutes()) + } + } + } } } diff --git a/internal/session/reader.go b/internal/session/reader.go index 495c240..3ade3e4 100644 --- a/internal/session/reader.go +++ b/internal/session/reader.go @@ -8,7 +8,15 @@ type TranscriptReader interface { // HeaderInfo contains minimal session metadata extracted from transcript headers. type HeaderInfo struct { - Timestamp string + // Timestamp is the transcript's first timestamp (session start). + Timestamp string + + // EndTimestamp is the transcript's last timestamp, read from a bounded + // tail scan rather than a full read of the file. Combined with + // Timestamp, callers derive session duration. Empty when the tail scan + // found no timestamped entry. + EndTimestamp string + FirstUserPrompt string } From e52c71412debd142e815fa49478e609fc246cb61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maple=EF=BC=81?= Date: Wed, 15 Jul 2026 16:39:00 +0800 Subject: [PATCH 05/16] feat: record command outcomes in usage log and normalize inject alias MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Usage entries now carry result (ok/error) and a one-line error message, written after the command finishes via a begin/finalize split around the existing exitOnError choke point — failures before target resolution are captured too. Legacy entries without the field read back as unknown, never as errors. The usage query normalizes the pre-rename "inject" command to "inherit" in both filter and display, and local builds resolve their version from runtime/debug.ReadBuildInfo instead of "dev". Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_013UQYeDtvg1CQYvirZ4jWC5 --- cmd/cc-session/commands.go | 79 ++++++----- cmd/cc-session/helpers.go | 1 + cmd/cc-session/main.go | 40 ++++++ cmd/cc-session/main_test.go | 226 ++++++++++++++++++++++++++++--- cmd/cc-session/usage_cmd.go | 81 ++++++++++- internal/tracker/tracker.go | 33 ++++- internal/tracker/tracker_test.go | 82 +++++++++++ 7 files changed, 486 insertions(+), 56 deletions(-) diff --git a/cmd/cc-session/commands.go b/cmd/cc-session/commands.go index fe725b2..518b464 100644 --- a/cmd/cc-session/commands.go +++ b/cmd/cc-session/commands.go @@ -32,6 +32,12 @@ type command struct { // deprecated "inject" alias. The cheat sheet enforces this by panicking // if a row ever references a hidden command's name. hidden bool + // tracksUsage marks commands whose invocations should be recorded to + // usage.jsonl. main's dispatch starts tracking (see beginUsageTracking + // in usage_cmd.go) before run executes, so a failure anywhere inside + // run — including before it resolves a session ID — still produces a + // usage entry. Meta commands ("help", "usage" itself) leave this false. + tracksUsage bool // run executes the command. reader is the concrete claudecodec.Codec, // which satisfies both session.TranscriptReader and session.HeaderScanner // so one signature covers every cmdXxx regardless of which interface it @@ -52,46 +58,53 @@ var commands []command func init() { commands = []command{ { - name: "list", - summary: "列出最近的 session", - argHint: "list", - run: func(args []string, reader claudecodec.Codec) { cmdList(args, reader) }, + name: "list", + summary: "列出最近的 session", + argHint: "list", + tracksUsage: true, + run: func(args []string, reader claudecodec.Codec) { cmdList(args, reader) }, }, { - name: "inherit", - summary: "分頁繼承 session 到 context", - argHint: "inherit ", - run: func(args []string, reader claudecodec.Codec) { cmdInherit(args, reader) }, + name: "inherit", + summary: "分頁繼承 session 到 context", + argHint: "inherit ", + tracksUsage: true, + run: func(args []string, reader claudecodec.Codec) { cmdInherit(args, reader) }, }, { - name: "read", - summary: "完整對話 + tool call 一行摘要", - argHint: "read ", - run: func(args []string, reader claudecodec.Codec) { cmdRead(args, reader) }, + name: "read", + summary: "完整對話 + tool call 一行摘要", + argHint: "read ", + tracksUsage: true, + run: func(args []string, reader claudecodec.Codec) { cmdRead(args, reader) }, }, { - name: "context", - summary: "精簡注入格式（帶 metadata header）", - argHint: "context ", - run: func(args []string, reader claudecodec.Codec) { cmdContext(args, reader) }, + name: "context", + summary: "精簡注入格式（帶 metadata header）", + argHint: "context ", + tracksUsage: true, + run: func(args []string, reader claudecodec.Codec) { cmdContext(args, reader) }, }, { - name: "expand", - summary: "展開特定 tool call 完整內容", - argHint: "expand ", - run: func(args []string, reader claudecodec.Codec) { cmdExpand(args, reader) }, + name: "expand", + summary: "展開特定 tool call 完整內容", + argHint: "expand ", + tracksUsage: true, + run: func(args []string, reader claudecodec.Codec) { cmdExpand(args, reader) }, }, { - name: "stats", - summary: "字元與 token 分佈統計", - argHint: "stats ", - run: func(args []string, reader claudecodec.Codec) { cmdStats(args, reader) }, + name: "stats", + summary: "字元與 token 分佈統計", + argHint: "stats ", + tracksUsage: true, + run: func(args []string, reader claudecodec.Codec) { cmdStats(args, reader) }, }, { - name: "audit", - summary: "檢視被過濾的內容取樣", - argHint: "audit ", - run: func(args []string, reader claudecodec.Codec) { cmdAudit(args, reader) }, + name: "audit", + summary: "檢視被過濾的內容取樣", + argHint: "audit ", + tracksUsage: true, + run: func(args []string, reader claudecodec.Codec) { cmdAudit(args, reader) }, }, { name: "usage", @@ -107,15 +120,17 @@ func init() { run: func(args []string, _ claudecodec.Codec) { cmdHelp(args) }, }, { - name: "benchmark", - summary: "掃描近期 session，計算壓縮率與成本比較", + name: "benchmark", + summary: "掃描近期 session，計算壓縮率與成本比較", + tracksUsage: true, // No argHint: benchmark is a maintainer/analysis command, not part of // the skill's quick-launch surface. run: func(args []string, reader claudecodec.Codec) { cmdBenchmark(args, reader) }, }, { - name: "inject", - hidden: true, + name: "inject", + hidden: true, + tracksUsage: true, run: func(args []string, reader claudecodec.Codec) { fmt.Fprintln(os.Stderr, "cc-session inject 已改名為 cc-session inherit，inject 別名將於未來版本移除，請改用 inherit。") cmdInherit(args, reader) diff --git a/cmd/cc-session/helpers.go b/cmd/cc-session/helpers.go index f33c7d1..f569b69 100644 --- a/cmd/cc-session/helpers.go +++ b/cmd/cc-session/helpers.go @@ -11,6 +11,7 @@ import ( ) func exitOnError(err error) { + finalizeUsageLog(err) if err == nil { return } diff --git a/cmd/cc-session/main.go b/cmd/cc-session/main.go index 9e74d00..0a1c6b3 100644 --- a/cmd/cc-session/main.go +++ b/cmd/cc-session/main.go @@ -6,6 +6,7 @@ package main import ( "fmt" "os" + "runtime/debug" "github.com/Mapleeeeeeeeeee/cc-session-reader/internal/claudecodec" "github.com/Mapleeeeeeeeeee/cc-session-reader/internal/tokens" @@ -14,6 +15,40 @@ import ( var version = "dev" var commit = "none" +// readBuildInfo is the debug.ReadBuildInfo backend used by resolveVersion. It +// is a package-level seam so tests can substitute deterministic build info +// instead of whatever the go test binary itself was built with. +var readBuildInfo = debug.ReadBuildInfo + +// resolveVersion returns version unchanged unless it's still the "dev" +// placeholder, meaning goreleaser's ldflags -X override (see .goreleaser.yaml) +// never ran — a plain `go build`/`go install` of this module. In that case it +// falls back to runtime/debug build info: `go install pkg@version` embeds the +// module version in info.Main.Version, while a source build embeds the VCS +// revision in info.Settings. Returns "dev" unchanged if neither is available. +func resolveVersion(version string) string { + if version != "dev" { + return version + } + info, ok := readBuildInfo() + if !ok { + return version + } + if info.Main.Version != "" && info.Main.Version != "(devel)" { + return info.Main.Version + } + for _, s := range info.Settings { + if s.Key == "vcs.revision" && s.Value != "" { + rev := s.Value + if len(rev) > 12 { + rev = rev[:12] + } + return "dev+" + rev + } + } + return version +} + type countTokensFunc func(string) (int, error) // countTokensFn is the token-counting backend used by runStats. It is a @@ -37,6 +72,8 @@ func main() { os.Exit(1) } + version = resolveVersion(version) + defer waitUsageLog() reader := claudecodec.Codec{} @@ -56,6 +93,9 @@ func main() { printUsage() os.Exit(1) } + if cmd.tracksUsage { + beginUsageTracking(cmd.name) + } cmd.run(os.Args[2:], reader) } } diff --git a/cmd/cc-session/main_test.go b/cmd/cc-session/main_test.go index 452dade..75048e1 100644 --- a/cmd/cc-session/main_test.go +++ b/cmd/cc-session/main_test.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "runtime" + "runtime/debug" "strconv" "strings" "testing" @@ -1442,7 +1443,28 @@ func TestExitOnError_GivenErrHelp_ThenNoStderrOutput(t *testing.T) { } } -func TestLogUsageAsync_GivenNoUsageEnabled_ThenDoesNotWriteToLog(t *testing.T) { +// usageTestCallerSession creates a fake Claude Code session under root so +// DetectCallerSession(cwd) returns non-empty, the precondition finalizeUsageLog +// checks before writing any entry. +func usageTestCallerSession(t *testing.T, root string) { + t.Helper() + cwd, err := os.Getwd() + if err != nil { + t.Fatalf("os.Getwd: %v", err) + } + if resolved, err := filepath.EvalSymlinks(cwd); err == nil { + cwd = resolved + } + projectDir := filepath.Join(root, ".claude", "projects", strings.ReplaceAll(cwd, "/", "-")) + if err := os.MkdirAll(projectDir, 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + if err := os.WriteFile(filepath.Join(projectDir, "fake-session-id.jsonl"), []byte{}, 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } +} + +func TestFinalizeUsageLog_GivenNoUsageEnabled_ThenDoesNotWriteToLog(t *testing.T) { root := t.TempDir() t.Setenv("HOME", root) t.Setenv("USERPROFILE", root) @@ -1451,7 +1473,9 @@ func TestLogUsageAsync_GivenNoUsageEnabled_ThenDoesNotWriteToLog(t *testing.T) { config.Reset() t.Cleanup(config.Reset) + beginUsageTracking("read") logUsageAsync("read", "abc12345") + finalizeUsageLog(nil) waitUsageLog() usagePath := filepath.Join(root, ".claude", "skills", "cc-session", "usage.jsonl") @@ -1460,7 +1484,49 @@ func TestLogUsageAsync_GivenNoUsageEnabled_ThenDoesNotWriteToLog(t *testing.T) { } } -func TestLogUsageAsync_GivenCallerSession_ThenWritesToLog(t *testing.T) { +func TestFinalizeUsageLog_GivenNoCallerSession_ThenDoesNotWriteToLog(t *testing.T) { + root := t.TempDir() + t.Setenv("HOME", root) + t.Setenv("USERPROFILE", root) + t.Setenv("CC_SESSION_NO_USAGE", "") + t.Setenv("ANTHROPIC_API_KEY", "") + config.Reset() + t.Cleanup(config.Reset) + + // No Claude Code project directory created, so DetectCallerSession returns "". + beginUsageTracking("read") + logUsageAsync("read", "abc12345") + finalizeUsageLog(nil) + waitUsageLog() + + usagePath := filepath.Join(root, ".claude", "skills", "cc-session", "usage.jsonl") + if _, err := os.Stat(usagePath); err == nil { + t.Error("usage.jsonl was created despite no caller session") + } +} + +func TestFinalizeUsageLog_GivenNoBeginUsageTracking_ThenDoesNotWriteToLog(t *testing.T) { + // Regression: "help"/"usage" never call beginUsageTracking, so their + // exitOnError call must stay a no-op instead of logging a bogus entry. + root := t.TempDir() + t.Setenv("HOME", root) + t.Setenv("USERPROFILE", root) + t.Setenv("CC_SESSION_NO_USAGE", "") + t.Setenv("ANTHROPIC_API_KEY", "") + config.Reset() + t.Cleanup(config.Reset) + usageTestCallerSession(t, root) + + finalizeUsageLog(nil) + waitUsageLog() + + usagePath := filepath.Join(root, ".claude", "skills", "cc-session", "usage.jsonl") + if _, err := os.Stat(usagePath); err == nil { + t.Error("usage.jsonl was created despite no tracked command") + } +} + +func TestFinalizeUsageLog_GivenSuccessfulCommand_ThenRecordsOkResult(t *testing.T) { // DetectCallerSession uses forward-slash path mapping that only works on // macOS/Linux where Claude Code actually creates session files. if runtime.GOOS == "windows" { @@ -1473,24 +1539,55 @@ func TestLogUsageAsync_GivenCallerSession_ThenWritesToLog(t *testing.T) { t.Setenv("ANTHROPIC_API_KEY", "") config.Reset() t.Cleanup(config.Reset) + usageTestCallerSession(t, root) - // Create a fake Claude Code session so DetectCallerSession returns non-empty. - cwd, err := os.Getwd() + beginUsageTracking("read") + logUsageAsync("read", "abc12345") + finalizeUsageLog(nil) + waitUsageLog() + + usagePath := filepath.Join(root, ".claude", "skills", "cc-session", "usage.jsonl") + data, err := os.ReadFile(usagePath) if err != nil { - t.Fatalf("os.Getwd: %v", err) + t.Fatalf("usage.jsonl not created: %v", err) } - if resolved, err := filepath.EvalSymlinks(cwd); err == nil { - cwd = resolved + line := strings.TrimSpace(string(data)) + if !strings.Contains(line, `"cmd":"read"`) { + t.Errorf("usage.jsonl missing cmd field, got: %s", line) } - projectDir := filepath.Join(root, ".claude", "projects", strings.ReplaceAll(cwd, "/", "-")) - if err := os.MkdirAll(projectDir, 0o755); err != nil { - t.Fatalf("MkdirAll: %v", err) + if !strings.Contains(line, `"target":"abc12345"`) { + t.Errorf("usage.jsonl missing target field, got: %s", line) } - if err := os.WriteFile(filepath.Join(projectDir, "fake-session-id.jsonl"), []byte{}, 0o644); err != nil { - t.Fatalf("WriteFile: %v", err) + if !strings.Contains(line, `"result":"ok"`) { + t.Errorf("usage.jsonl missing result:ok, got: %s", line) + } + if strings.Contains(line, `"error"`) { + t.Errorf("usage.jsonl should omit error field on success, got: %s", line) } +} - logUsageAsync("read", "abc12345") +// Bug this guards against: a command that failed before ever resolving a +// target (e.g. "read" given an unknown session ID) used to log nothing at +// all, so failures were invisible in usage.jsonl. beginUsageTracking starts +// tracking before the command's own logic runs, so this now produces an +// entry with an empty target and result:error. +func TestFinalizeUsageLog_GivenCommandFailedBeforeResolvingTarget_ThenRecordsErrorResultWithEmptyTarget(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Claude Code sessions only exist on macOS/Linux") + } + root := t.TempDir() + t.Setenv("HOME", root) + t.Setenv("USERPROFILE", root) + t.Setenv("CC_SESSION_NO_USAGE", "") + t.Setenv("ANTHROPIC_API_KEY", "") + config.Reset() + t.Cleanup(config.Reset) + usageTestCallerSession(t, root) + + // Simulate main's dispatch starting tracking, then the command failing + // (e.g. resolveSession erroring) before it ever calls logUsageAsync. + beginUsageTracking("read") + finalizeUsageLog(fmt.Errorf("transcript not found: no-such-session-id")) waitUsageLog() usagePath := filepath.Join(root, ".claude", "skills", "cc-session", "usage.jsonl") @@ -1502,12 +1599,21 @@ func TestLogUsageAsync_GivenCallerSession_ThenWritesToLog(t *testing.T) { if !strings.Contains(line, `"cmd":"read"`) { t.Errorf("usage.jsonl missing cmd field, got: %s", line) } - if !strings.Contains(line, `"target":"abc12345"`) { - t.Errorf("usage.jsonl missing target field, got: %s", line) + if !strings.Contains(line, `"target":""`) { + t.Errorf("usage.jsonl should record an empty target (never resolved), got: %s", line) + } + if !strings.Contains(line, `"result":"error"`) { + t.Errorf("usage.jsonl missing result:error, got: %s", line) + } + if !strings.Contains(line, `"error":"transcript not found: no-such-session-id"`) { + t.Errorf("usage.jsonl missing error message, got: %s", line) } } -func TestLogUsageAsync_GivenNoCallerSession_ThenDoesNotWriteToLog(t *testing.T) { +func TestFinalizeUsageLog_GivenMultilineError_ThenTruncatesToFirstLine(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Claude Code sessions only exist on macOS/Linux") + } root := t.TempDir() t.Setenv("HOME", root) t.Setenv("USERPROFILE", root) @@ -1515,13 +1621,95 @@ func TestLogUsageAsync_GivenNoCallerSession_ThenDoesNotWriteToLog(t *testing.T) t.Setenv("ANTHROPIC_API_KEY", "") config.Reset() t.Cleanup(config.Reset) + usageTestCallerSession(t, root) - // No Claude Code project directory created, so DetectCallerSession returns "". - logUsageAsync("read", "abc12345") + beginUsageTracking("stats") + finalizeUsageLog(fmt.Errorf("count filtered tokens: boom\nrequest id: abc")) + waitUsageLog() + + usagePath := filepath.Join(root, ".claude", "skills", "cc-session", "usage.jsonl") + data, err := os.ReadFile(usagePath) + if err != nil { + t.Fatalf("usage.jsonl not created: %v", err) + } + line := strings.TrimSpace(string(data)) + if !strings.Contains(line, `"error":"count filtered tokens: boom"`) { + t.Errorf("usage.jsonl error field should be truncated to the first line, got: %s", line) + } + if strings.Contains(line, "request id") { + t.Errorf("usage.jsonl error field leaked a second line, got: %s", line) + } +} + +func TestFinalizeUsageLog_GivenErrHelp_ThenDoesNotWriteToLog(t *testing.T) { + // A "-h" request is a deliberate UX flow, not a command failure — it + // shouldn't show up as a tracked error. + root := t.TempDir() + t.Setenv("HOME", root) + t.Setenv("USERPROFILE", root) + t.Setenv("CC_SESSION_NO_USAGE", "") + t.Setenv("ANTHROPIC_API_KEY", "") + config.Reset() + t.Cleanup(config.Reset) + usageTestCallerSession(t, root) + + beginUsageTracking("read") + finalizeUsageLog(flag.ErrHelp) waitUsageLog() usagePath := filepath.Join(root, ".claude", "skills", "cc-session", "usage.jsonl") if _, err := os.Stat(usagePath); err == nil { - t.Error("usage.jsonl was created despite no caller session") + t.Error("usage.jsonl was created for a flag.ErrHelp invocation") + } +} + +func TestResolveVersion_GivenLdflagsVersion_ThenReturnsItUnchanged(t *testing.T) { + // ldflags -X main.version=... (goreleaser) always takes priority over + // build-info fallback. + got := resolveVersion("v1.2.3") + if got != "v1.2.3" { + t.Errorf("resolveVersion(%q) = %q, want unchanged", "v1.2.3", got) + } +} + +func TestResolveVersion_GivenDevWithModuleVersion_ThenReturnsModuleVersion(t *testing.T) { + old := readBuildInfo + t.Cleanup(func() { readBuildInfo = old }) + readBuildInfo = func() (*debug.BuildInfo, bool) { + return &debug.BuildInfo{Main: debug.Module{Version: "v0.5.0"}}, true + } + + got := resolveVersion("dev") + if got != "v0.5.0" { + t.Errorf("resolveVersion(\"dev\") = %q, want %q (go install pkg@version)", got, "v0.5.0") + } +} + +func TestResolveVersion_GivenDevWithNoModuleVersionButVCSRevision_ThenReturnsShortRevision(t *testing.T) { + old := readBuildInfo + t.Cleanup(func() { readBuildInfo = old }) + readBuildInfo = func() (*debug.BuildInfo, bool) { + return &debug.BuildInfo{ + Main: debug.Module{Version: "(devel)"}, + Settings: []debug.BuildSetting{ + {Key: "vcs.revision", Value: "95712a4abcdef1234567890"}, + }, + }, true + } + + got := resolveVersion("dev") + if got != "dev+95712a4abcde" { + t.Errorf("resolveVersion(\"dev\") = %q, want short vcs revision fallback", got) + } +} + +func TestResolveVersion_GivenDevWithNoBuildInfo_ThenReturnsDevUnchanged(t *testing.T) { + old := readBuildInfo + t.Cleanup(func() { readBuildInfo = old }) + readBuildInfo = func() (*debug.BuildInfo, bool) { return nil, false } + + got := resolveVersion("dev") + if got != "dev" { + t.Errorf("resolveVersion(\"dev\") = %q, want %q when build info is unavailable", got, "dev") } } diff --git a/cmd/cc-session/usage_cmd.go b/cmd/cc-session/usage_cmd.go index c5c3748..69a06f2 100644 --- a/cmd/cc-session/usage_cmd.go +++ b/cmd/cc-session/usage_cmd.go @@ -1,10 +1,12 @@ package main import ( + "errors" "flag" "fmt" "io" "os" + "strings" "sync" "time" @@ -15,12 +17,64 @@ import ( var usageWG sync.WaitGroup -// logUsageAsync logs a usage entry in a background goroutine. -// Call waitUsageLog before process exit to ensure the write completes. +// pendingUsageCmd and pendingUsageTarget describe the in-progress command +// invocation to be recorded once its outcome is known. There is at most one +// command per process, so package-level state is safe: main's dispatch sets +// pendingUsageCmd before running a tracked command (see beginUsageTracking), +// and the command itself may later refine pendingUsageTarget once it +// resolves a concrete target (see logUsageAsync). finalizeUsageLog consumes +// and clears both when the command returns. +var ( + pendingUsageCmd string + pendingUsageTarget string +) + +// beginUsageTracking marks cmd as the subcommand whose result should be +// recorded to usage.jsonl once it finishes. Called once from main's dispatch +// for every command with tracksUsage set, before its run executes — this way +// a command that fails before it ever resolves a target (e.g. an unknown +// session ID) still produces a usage entry, just with an empty target. +func beginUsageTracking(cmd string) { + pendingUsageCmd = cmd +} + +// logUsageAsync records target as the resolved session for the current +// tracked invocation (see beginUsageTracking). It no longer writes to disk +// itself: the actual entry is written by finalizeUsageLog once the command's +// outcome is known, so a single JSONL line always carries the correct result. func logUsageAsync(cmd string, target string) { + pendingUsageCmd = cmd + pendingUsageTarget = target +} + +// finalizeUsageLog writes the usage entry for the invocation that just +// finished with cmdErr (nil on success), in a background goroutine. It is a +// no-op when no command started tracking this invocation (beginUsageTracking +// was never called — e.g. "help"/"usage" itself), when CC_SESSION_NO_USAGE is +// set, or when cmdErr is flag.ErrHelp (a deliberate -h request, not a +// failure). Call waitUsageLog before process exit to ensure the write +// completes. +func finalizeUsageLog(cmdErr error) { + if pendingUsageCmd == "" { + return + } + cmd, target := pendingUsageCmd, pendingUsageTarget + pendingUsageCmd, pendingUsageTarget = "", "" + if config.Get().NoUsage { return } + if errors.Is(cmdErr, flag.ErrHelp) { + return + } + + result := "ok" + errMsg := "" + if cmdErr != nil { + result = "error" + errMsg = firstLine(cmdErr.Error()) + } + usageWG.Add(1) go func() { defer usageWG.Done() @@ -40,11 +94,22 @@ func logUsageAsync(cmd string, target string) { Caller: caller, Version: version, Commit: commit, + Result: result, + Error: errMsg, } _ = tracker.LogUsage(entry) }() } +// firstLine truncates s to its first line, so a multi-line error message +// doesn't break usage.jsonl's one-entry-per-line format. +func firstLine(s string) string { + if i := strings.IndexByte(s, '\n'); i >= 0 { + return s[:i] + } + return s +} + func waitUsageLog() { usageWG.Wait() } func cmdUsage(args []string) { @@ -91,8 +156,16 @@ func runUsage(args []string, out io.Writer, errOut io.Writer) error { callerShort = "caller:" + session.ShortID(e.Caller, 8) } - fmt.Fprintf(out, "%s %-8s %s %s %s\n", - dateStr, e.Command, target, callerShort, e.Cwd) + // Result is only present on entries recorded after the result field + // was added; older entries render no marker rather than a + // misleading "ok". + resultMarker := "" + if e.Result == "error" { + resultMarker = " [ERR]" + } + + fmt.Fprintf(out, "%s %-8s %s %s %s%s\n", + dateStr, e.Command, target, callerShort, e.Cwd, resultMarker) } return nil } diff --git a/internal/tracker/tracker.go b/internal/tracker/tracker.go index 41b0ef3..10aef9b 100644 --- a/internal/tracker/tracker.go +++ b/internal/tracker/tracker.go @@ -22,6 +22,31 @@ type UsageEntry struct { Caller string `json:"caller"` Version string `json:"version,omitempty"` Commit string `json:"commit,omitempty"` + // Result is "ok" or "error", recorded once the invocation finishes. + // Empty on entries written before this field existed (and on entries + // for commands that don't track a result) — treat as unknown, not as + // a failure. + Result string `json:"result,omitempty"` + // Error holds the first line of the failing command's error message. + // Empty unless Result == "error". + Error string `json:"error,omitempty"` +} + +// commandAliases maps a deprecated subcommand name recorded by older +// binaries to its current name, so usage queries and stats treat historical +// entries the same as new ones. "inject" was renamed to "inherit" in +// 554e57b; see commands.go's hidden "inject" registry entry. +var commandAliases = map[string]string{ + "inject": "inherit", +} + +// canonicalCommand resolves cmd through commandAliases, returning cmd +// unchanged if it has no alias. +func canonicalCommand(cmd string) string { + if canon, ok := commandAliases[cmd]; ok { + return canon + } + return cmd } // DefaultLogPath returns the canonical path for the usage log. @@ -80,7 +105,10 @@ func ReadUsageLog(limit int, cmdFilter string) ([]UsageEntry, error) { // ReadUsageLogFromPath reads and parses the JSONL file at path. // Returns entries in reverse chronological order (most-recent first). // If the file does not exist, returns an empty slice and nil error. -// Blank or unparseable lines are silently skipped. +// Blank or unparseable lines are silently skipped. cmdFilter and each +// entry's Command are both resolved through canonicalCommand first, so a +// deprecated alias (e.g. "inject") matches its current name ("inherit") in +// either direction, and the returned entries always display the current name. func ReadUsageLogFromPath(limit int, cmdFilter string, path string) ([]UsageEntry, error) { f, err := os.Open(path) if os.IsNotExist(err) { @@ -91,6 +119,8 @@ func ReadUsageLogFromPath(limit int, cmdFilter string, path string) ([]UsageEntr } defer f.Close() + cmdFilter = canonicalCommand(cmdFilter) + var entries []UsageEntry scanner := bufio.NewScanner(f) for scanner.Scan() { @@ -102,6 +132,7 @@ func ReadUsageLogFromPath(limit int, cmdFilter string, path string) ([]UsageEntr if err := json.Unmarshal([]byte(line), &e); err != nil { continue } + e.Command = canonicalCommand(e.Command) if cmdFilter != "" && e.Command != cmdFilter { continue } diff --git a/internal/tracker/tracker_test.go b/internal/tracker/tracker_test.go index b50f2dd..4b1aab7 100644 --- a/internal/tracker/tracker_test.go +++ b/internal/tracker/tracker_test.go @@ -222,6 +222,88 @@ func TestDetectCallerSessionWithBase_GivenMissingDir_WhenDetected_ThenReturnsEmp } } +func TestLogUsageToPath_GivenResultAndError_WhenAppended_ThenRoundTrips(t *testing.T) { + path := filepath.Join(t.TempDir(), "usage.jsonl") + entry := UsageEntry{Command: "read", Target: "x", Result: "error", Error: "transcript not found: abc"} + + writeEntry(t, path, entry) + + entries, err := ReadUsageLogFromPath(0, "", path) + if err != nil { + t.Fatalf("ReadUsageLogFromPath returned error: %v", err) + } + if len(entries) != 1 { + t.Fatalf("entry count = %d, want 1", len(entries)) + } + if entries[0].Result != "error" { + t.Errorf("Result = %q, want %q", entries[0].Result, "error") + } + if entries[0].Error != entry.Error { + t.Errorf("Error = %q, want %q", entries[0].Error, entry.Error) + } +} + +// Regression: entries written before the Result field existed have no +// "result" key at all. Without this, an empty Result on unmarshal could be +// mistaken for a known failure instead of "we don't know". +func TestReadUsageLogFromPath_GivenLegacyEntryWithoutResultField_WhenRead_ThenResultIsEmptyNotError(t *testing.T) { + path := filepath.Join(t.TempDir(), "usage.jsonl") + raw := `{"ts":"2026-06-15T10:00:00Z","cmd":"read","target":"legacy","cwd":"/x","caller":"c"}` + "\n" + if err := os.WriteFile(path, []byte(raw), 0o644); err != nil { + t.Fatalf("write legacy entry: %v", err) + } + + entries, err := ReadUsageLogFromPath(0, "", path) + if err != nil { + t.Fatalf("ReadUsageLogFromPath returned error: %v", err) + } + if len(entries) != 1 { + t.Fatalf("entry count = %d, want 1", len(entries)) + } + if entries[0].Result != "" { + t.Errorf("Result = %q, want empty (unknown) for a pre-existing entry", entries[0].Result) + } +} + +// Regression: "inject" was renamed to "inherit" in 554e57b, but binaries built +// before the rename wrote entries with Command == "inject". Without alias +// normalization, "usage -cmd inherit" can't find that history. +func TestReadUsageLogFromPath_GivenLegacyInjectEntries_WhenFilteredByInherit_ThenMatchesAndNormalizesDisplay(t *testing.T) { + path := filepath.Join(t.TempDir(), "usage.jsonl") + writeEntry(t, path, UsageEntry{Command: "inject", Target: "legacy"}) + writeEntry(t, path, UsageEntry{Command: "inherit", Target: "current"}) + writeEntry(t, path, UsageEntry{Command: "read", Target: "unrelated"}) + + entries, err := ReadUsageLogFromPath(0, "inherit", path) + if err != nil { + t.Fatalf("ReadUsageLogFromPath returned error: %v", err) + } + if len(entries) != 2 { + t.Fatalf("entry count = %d, want 2 (legacy inject + current inherit)", len(entries)) + } + for _, e := range entries { + if e.Command != "inherit" { + t.Errorf("Command = %q, want normalized %q", e.Command, "inherit") + } + } +} + +// Regression: filtering by the deprecated alias itself ("-cmd inject") should +// still surface entries recorded under the current name, for anyone who +// still types the old command out of habit. +func TestReadUsageLogFromPath_GivenLegacyInjectFilter_WhenFiltered_ThenAlsoMatchesCurrentInheritEntries(t *testing.T) { + path := filepath.Join(t.TempDir(), "usage.jsonl") + writeEntry(t, path, UsageEntry{Command: "inherit", Target: "current"}) + + entries, err := ReadUsageLogFromPath(0, "inject", path) + if err != nil { + t.Fatalf("ReadUsageLogFromPath returned error: %v", err) + } + if len(entries) != 1 { + t.Fatalf("entry count = %d, want 1 (alias filter should also match current entries)", len(entries)) + } +} + func TestDetectCallerSessionWithBase_GivenMultipleJSONL_WhenDetected_ThenReturnsNewestSession(t *testing.T) { projectsDir := t.TempDir() cwd := "/Users/maple/Desktop" From 3b81d2f6e60237ce0f523b0aa96d3a9040a716ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maple=EF=BC=81?= Date: Wed, 15 Jul 2026 16:39:00 +0800 Subject: [PATCH 06/16] fix: measure stats from the real render pipeline so breakdown reconciles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ComputeStats kept its own copy of the compaction logic, so the KEPT category sum drifted from the Filtered total (some categories counted pre-compression text, join newlines went unaccounted, and formatter-side cc-session collapsing was invisible). Filtered is now measured from the actual read-render output via a content sink threaded through the formatter, categories derive from that same pass, and the residual template bytes (timestamps, role labels, indentation) surface as an explicit "render overhead" row — the breakdown now reconciles to the character. System-reminder/context-usage blocks that the renderer drops are no longer miscounted as kept user text. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_013UQYeDtvg1CQYvirZ4jWC5 --- internal/analyzer/render.go | 1 + internal/analyzer/stats.go | 107 +++++++-------- internal/analyzer/stats_test.go | 129 ++++++++++++++++-- .../formatter/analyzer_integration_test.go | 41 ++++++ internal/formatter/context.go | 2 +- internal/formatter/integration_test.go | 24 ---- internal/formatter/read.go | 36 ++++- internal/formatter/render.go | 21 ++- 8 files changed, 260 insertions(+), 101 deletions(-) create mode 100644 internal/formatter/analyzer_integration_test.go diff --git a/internal/analyzer/render.go b/internal/analyzer/render.go index d933e5e..1e45221 100644 --- a/internal/analyzer/render.go +++ b/internal/analyzer/render.go @@ -43,6 +43,7 @@ func RenderStats(out io.Writer, errOut io.Writer, result StatsResult, opts Rende {"KEPT user answers: ", "user_answers"}, {"KEPT assistant text: ", "assistant_text"}, {"KEPT tool summaries: ", "tool_summaries"}, + {"FMT render overhead: ", "render_overhead"}, {"CUT tool input (raw): ", "tool_input_raw"}, {"CUT tool result (raw):", "tool_result_raw"}, {"CUT system/noise: ", "system_noise"}, diff --git a/internal/analyzer/stats.go b/internal/analyzer/stats.go index a9b0262..b723748 100644 --- a/internal/analyzer/stats.go +++ b/internal/analyzer/stats.go @@ -5,8 +5,8 @@ import ( "strings" "unicode/utf8" + "github.com/Mapleeeeeeeeeee/cc-session-reader/internal/formatter" "github.com/Mapleeeeeeeeeee/cc-session-reader/internal/session" - "github.com/Mapleeeeeeeeeee/cc-session-reader/internal/summarizer" ) // ToolStats tracks per-tool usage metrics accumulated during ComputeStats. @@ -35,8 +35,16 @@ type StatsResult struct { CompactCount int } +// ComputeStats walks events once to accumulate RAW-side accounting (every +// byte the transcript contains, and the CUT categories that never reach the +// injected output at all), then measures the FILTERED side by running the +// same events through formatter.RenderReadEventsWithSink — the exact render +// pass `cc-session inherit` uses to inject context, collapsing included. That +// single render pass both produces FilteredText and reports each unit of +// kept content to the KEPT categories, so the two can never drift apart the +// way two independent implementations previously did. func ComputeStats(events []session.Event) StatsResult { - var rawParts, filteredParts []string + var rawParts []string categories := map[string]int{ "user_text": 0, "user_answers": 0, @@ -46,10 +54,10 @@ func ComputeStats(events []session.Event) StatsResult { "tool_result_raw": 0, "system_noise": 0, "command_noise": 0, + "render_overhead": 0, } perTool := map[string]*ToolStats{} toolUseNames := map[string]string{} - seenSkills := map[string]bool{} var lastContextTokens, totalOutputTokens, apiCallCount, compactCount, userTurnCount int var prevUsage *session.Usage @@ -69,26 +77,16 @@ func ComputeStats(events []session.Event) StatsResult { if event.User == nil { continue } - // Command invocation: the short marker is kept content. Deliberate - // undercount — the marker counts identically toward raw and filtered, - // so an invocation contributes zero reduction here. The original - // invocation wrapper (...) was already - // dropped at parse time and never reaches stats, so its ~100 chars of - // savings are not reflected in the reduction. We accept this: the - // wrapper is tiny, and the bulk command savings (multi-KB stdout) are - // correctly captured via the IsCommandNoise branch below, which counts - // toward raw but not filtered. Surfacing the wrapper would mean - // re-plumbing the dropped text through the parser for a rounding-error - // gain — not worth the coupling. + // Command invocation marker: cheap and identical in both raw and + // filtered streams, so it contributes no reduction here. Its KEPT + // weight is measured below via the render pass, not here. if event.User.CommandMarker != "" { - categories["user_text"] += utf8.RuneCountInString(event.User.CommandMarker) rawParts = append(rawParts, event.User.CommandMarker) - filteredParts = append(filteredParts, event.User.CommandMarker) continue } - // Command output / caveat: machine noise. Count toward raw so the - // reduction reflects what was actually cut, but never toward - // filtered — mirrors how system_noise is handled. + // Command output / caveat: machine noise, fully cut from the + // filtered stream (formatter.renderUserMessage drops it unless + // -verbose-commands is set, which ComputeStats never sets). if event.User.IsCommandNoise { categories["command_noise"] += utf8.RuneCountInString(event.User.Text) rawParts = append(rawParts, event.User.Text) @@ -97,45 +95,25 @@ func ComputeStats(events []session.Event) StatsResult { if strings.TrimSpace(event.User.Text) == "" { continue } - - // Harness-injected subtypes: mirror the formatter's compression. + // Harness-injected system-reminder/context-usage blocks are + // dropped entirely by the render pass (never shown), so they are + // CUT, not KEPT — folded into system_noise since they are the + // same kind of harness boilerplate as EventNoise. if event.User.IsSystemReminder || event.User.IsContextUsage { - categories["user_text"] += utf8.RuneCountInString(event.User.Text) - rawParts = append(rawParts, event.User.Text) - continue - } - if event.User.IsSkillInjection { - categories["user_text"] += utf8.RuneCountInString(event.User.Text) - rawParts = append(rawParts, event.User.Text) - compact := session.CompactSkillInjection(event.User, seenSkills) - filteredParts = append(filteredParts, compact) - continue - } - if event.User.IsTeammateMessage { - categories["user_text"] += utf8.RuneCountInString(event.User.Text) + categories["system_noise"] += utf8.RuneCountInString(event.User.Text) rawParts = append(rawParts, event.User.Text) - if compacted, ok := session.CompactTeammateMessage(event.User.Text); ok { - filteredParts = append(filteredParts, compacted) - } continue } - if event.User.IsCommandInjection { - categories["user_text"] += utf8.RuneCountInString(event.User.Text) + // Skill/teammate/command injections are compacted rather than + // dropped; their KEPT (compacted) size is measured below via the + // render pass. Only the raw side is recorded here. + if event.User.IsSkillInjection || event.User.IsTeammateMessage || event.User.IsCommandInjection { rawParts = append(rawParts, event.User.Text) - if compacted, ok := session.CompactCommandInjection(event.User.Text); ok { - filteredParts = append(filteredParts, compacted) - } continue } userTurnCount++ - categories["user_text"] += utf8.RuneCountInString(event.User.Text) rawParts = append(rawParts, event.User.Text) - if compacted, ok := session.CompactTaskNotification(event.User.Text); ok { - filteredParts = append(filteredParts, compacted) - } else { - filteredParts = append(filteredParts, event.User.Text) - } case session.EventAssistantMessage: if event.Assistant == nil { @@ -148,9 +126,7 @@ func ComputeStats(events []session.Event) StatsResult { prevUsage = u } if strings.TrimSpace(event.Assistant.Text) != "" { - categories["assistant_text"] += utf8.RuneCountInString(event.Assistant.Text) rawParts = append(rawParts, event.Assistant.Text) - filteredParts = append(filteredParts, event.Assistant.Text) } for _, tool := range event.Assistant.ToolUses { rawJSON := tool.Input.MarshalNoEscape() @@ -171,10 +147,6 @@ func ComputeStats(events []session.Event) StatsResult { } ts.CallCount++ ts.InputChars += utf8.RuneCountInString(rawJSON) - - summary := summarizer.SummarizeToolUse(name, tool.Input, tool.Cwd) - categories["tool_summaries"] += utf8.RuneCountInString(summary) - filteredParts = append(filteredParts, summary) } case session.EventToolResult: @@ -182,16 +154,11 @@ func ComputeStats(events []session.Event) StatsResult { continue } if event.User != nil && event.User.IsAnswer { - categories["user_answers"] += utf8.RuneCountInString(event.User.Text) rawParts = append(rawParts, event.Tool.Text) - filteredParts = append(filteredParts, event.User.Text) continue } categories["tool_result_raw"] += utf8.RuneCountInString(event.Tool.Text) rawParts = append(rawParts, event.Tool.Text) - summary := event.Tool.Summary() - categories["tool_summaries"] += utf8.RuneCountInString(summary) - filteredParts = append(filteredParts, summary) toolName := "" if event.Tool.ToolUseID != "" { @@ -213,13 +180,31 @@ func ComputeStats(events []session.Event) StatsResult { } rawText := strings.Join(rawParts, "\n") - filteredText := strings.Join(filteredParts, "\n") + + // FilteredText is the actual `cc-session inherit`-equivalent render + // output (agentIDs empty and FormatOptions zero-value, matching + // inject.RenderFullOutput's defaults), so "Filtered" always means the + // real injected size. The sink tags each kept unit into categories in + // the same pass that renders it. + filteredText, _ := formatter.RenderReadEventsWithSink(events, map[string]bool{}, formatter.FormatOptions{}, func(category, text string) { + categories[category] += utf8.RuneCountInString(text) + }) + filteredChars := utf8.RuneCountInString(filteredText) + + // render_overhead is the render pipeline's own structure layered on top + // of kept content — timestamps, "user:"/"assistant:" labels, tool-line + // indentation, and blank-line separators — none of which belongs to a + // single content category but all of which is real injected bytes. + // Surfacing it explicitly keeps KEPT-category-sum + this line exactly + // equal to FilteredChars, instead of leaving an unexplained gap. + kept := categories["user_text"] + categories["user_answers"] + categories["assistant_text"] + categories["tool_summaries"] + categories["render_overhead"] = filteredChars - kept return StatsResult{ RawText: rawText, FilteredText: filteredText, RawChars: utf8.RuneCountInString(rawText), - FilteredChars: utf8.RuneCountInString(filteredText), + FilteredChars: filteredChars, Categories: categories, PerTool: perTool, LastContextTokens: lastContextTokens, diff --git a/internal/analyzer/stats_test.go b/internal/analyzer/stats_test.go index 3496e23..de316de 100644 --- a/internal/analyzer/stats_test.go +++ b/internal/analyzer/stats_test.go @@ -1,10 +1,12 @@ package analyzer import ( + "fmt" "strings" "testing" "unicode/utf8" + "github.com/Mapleeeeeeeeeee/cc-session-reader/internal/parser" "github.com/Mapleeeeeeeeeee/cc-session-reader/internal/session" ) @@ -73,12 +75,15 @@ func TestComputeStats_SeparatesRawFromFilteredByContent(t *testing.T) { assertCategory(t, result, "user_answers", 0) } -// TestComputeStats_CountsCharsForSingleUserMessage pins the exact char-count -// arithmetic on a case trivial enough to verify by eye: a single ASCII user -// message becomes both the entire raw and filtered stream with no join -// separators and no summarizer formatting involved. +// TestComputeStats_CountsCharsForSingleUserMessage pins RawChars as a pure +// event-content join (no rendering involved) and documents that +// FilteredChars now includes the real read-pipeline template (timestamp + +// "user:" label + blank-line separator) around the message, because +// FilteredText is measured by rendering through the exact pipeline +// `cc-session inherit` uses to inject context (see stats.go) — not a second, +// decoration-free reimplementation. func TestComputeStats_CountsCharsForSingleUserMessage(t *testing.T) { - const message = "hello" // 5 ASCII runes, the whole stream + const message = "hello" // 5 ASCII runes, the whole raw stream events := []session.Event{ { @@ -92,14 +97,21 @@ func TestComputeStats_CountsCharsForSingleUserMessage(t *testing.T) { if result.RawChars != 5 { t.Fatalf("RawChars = %d, want 5", result.RawChars) } - if result.FilteredChars != 5 { - t.Fatalf("FilteredChars = %d, want 5", result.FilteredChars) - } if result.RawText != message { t.Fatalf("RawText = %q, want %q", result.RawText, message) } - if result.FilteredText != message { - t.Fatalf("FilteredText = %q, want %q", result.FilteredText, message) + + wantFiltered := fmt.Sprintf("[%s] user:\n%s\n\n", parser.FormatTimestamp(""), message) + if result.FilteredText != wantFiltered { + t.Fatalf("FilteredText = %q, want %q", result.FilteredText, wantFiltered) + } + if want := utf8.RuneCountInString(wantFiltered); result.FilteredChars != want { + t.Fatalf("FilteredChars = %d, want %d", result.FilteredChars, want) + } + // The message itself is still measured as pure content in user_text; the + // timestamp/label/blank-line template around it lands in render_overhead. + if got := result.Categories["user_text"]; got != len([]rune(message)) { + t.Fatalf("user_text category = %d, want %d", got, len([]rune(message))) } } @@ -165,6 +177,103 @@ func TestComputeStats_CommandMarkerIsKeptContent(t *testing.T) { assertContains(t, "RawText", result.RawText, marker) } +// TestComputeStats_GivenNestedCCSessionCalls_WhenComputed_ThenKeptCategoriesReconcileWithFilteredChars +// is a regression test for two bugs found in a real session (62e7e52a), where +// the KEPT category sum (39,851) did not match FilteredChars (37,130): +// +// 1. ComputeStats measured Filtered from its own pass over events, while the +// real `cc-session read`/`inherit` pipeline (internal/formatter) collapses +// consecutive same-session cc-session calls into one summary line. Stats +// never applied that collapse, so it overcounted tool_summaries relative +// to what actually gets injected. +// 2. Independent of collapse, ComputeStats' own KEPT categories and its own +// FilteredText join were computed from two different views of the same +// event (e.g. system-reminder/context-usage bytes were tallied into the +// "user_text" KEPT bucket even though they are dropped entirely from the +// filtered stream), so the two numbers could drift even without any +// cc-session nesting involved. +// +// This test pins the fix: FilteredChars must equal the real read-pipeline +// output (collapse included), and every KEPT category plus the explicit +// render_overhead line (the render pipeline's timestamps/labels/blank lines, +// which are real injected bytes but not attributable to a single content +// category) must reconcile with FilteredChars exactly. +func TestComputeStats_GivenNestedCCSessionCalls_WhenComputed_ThenKeptCategoriesReconcileWithFilteredChars(t *testing.T) { + events := buildNestedCCSessionEvents() + + result := ComputeStats(events) + + // Regression guard for bug 1: the real pipeline collapses these 4 calls + // into a single "(cc-session#...)" summary line. + if got := strings.Count(result.FilteredText, "cc-session#"); got != 1 { + t.Fatalf("expected exactly one collapsed cc-session marker in FilteredText, got %d\nFilteredText:\n%s", got, result.FilteredText) + } + if strings.Count(result.FilteredText, "[Bash") != 0 { + t.Fatalf("individual cc-session Bash calls must not survive collapsing\nFilteredText:\n%s", result.FilteredText) + } + + // Regression guard for bug 2: every KEPT category plus the explicit + // structural-overhead line must sum to exactly FilteredChars. + kept := result.Categories["user_text"] + result.Categories["user_answers"] + + result.Categories["assistant_text"] + result.Categories["tool_summaries"] + + result.Categories["render_overhead"] + if kept != result.FilteredChars { + t.Fatalf("KEPT categories + render_overhead = %d, want FilteredChars = %d (categories: %+v)", + kept, result.FilteredChars, result.Categories) + } + // render_overhead is pure render-template decoration (timestamps, "user:" + // labels, blank-line separators) layered on top of content that is + // already accounted for elsewhere; it can only add bytes, never remove. + if result.Categories["render_overhead"] < 0 { + t.Fatalf("render_overhead = %d, want >= 0 (structural formatting only adds bytes, never removes)", result.Categories["render_overhead"]) + } +} + +// buildNestedCCSessionEvents builds a fixture with a plain user turn, four +// consecutive "cc-session inherit" tool calls against the same nested session +// id (which formatter.collapseCCSessionTools collapses into one line), and a +// closing assistant reply — mirroring formatter_test.go's makeCCSessionEvents +// pattern so the regression exercises the same collapsing behavior. +func buildNestedCCSessionEvents() []session.Event { + const nestedSessionID = "16d06326-977b-4c82-a38f-9b2358aa80ca" + events := []session.Event{ + {Kind: session.EventUserMessage, User: &session.UserMessage{Text: "please inherit the earlier session"}}, + } + for p := 1; p <= 4; p++ { + toolID := fmt.Sprintf("inherit-tool-%d", p) + events = append(events, + session.Event{ + Kind: session.EventAssistantMessage, + Assistant: &session.AssistantMessage{ + ToolUses: []session.ToolUse{ + { + ID: toolID, + Name: "Bash", + Input: session.ToolInput{Raw: map[string]any{ + "command": fmt.Sprintf("cc-session inherit %s", nestedSessionID), + "description": "Load session", + }}, + }, + }, + }, + }, + session.Event{ + Kind: session.EventToolResult, + Tool: &session.ToolResult{ + ToolUseID: toolID, + Success: true, + Text: fmt.Sprintf("ok: [page %d/4 | lines 1-100 of 400]", p), + }, + }, + ) + } + events = append(events, session.Event{ + Kind: session.EventAssistantMessage, + Assistant: &session.AssistantMessage{Text: "done inheriting"}, + }) + return events +} + func assertCategory(t *testing.T, result StatsResult, key string, want int) { t.Helper() if got := result.Categories[key]; got != want { diff --git a/internal/formatter/analyzer_integration_test.go b/internal/formatter/analyzer_integration_test.go new file mode 100644 index 0000000..adaf468 --- /dev/null +++ b/internal/formatter/analyzer_integration_test.go @@ -0,0 +1,41 @@ +package formatter_test + +import ( + "testing" + + "github.com/Mapleeeeeeeeeee/cc-session-reader/internal/analyzer" + "github.com/Mapleeeeeeeeeee/cc-session-reader/internal/claudecodec" +) + +// This file lives in the external formatter_test package (not formatter) +// specifically because analyzer.ComputeStats now calls into formatter to +// derive its Filtered measurement from the real read pipeline (see +// analyzer/stats.go). formatter's own tests (package formatter) cannot +// import analyzer without an import cycle; formatter_test can, since it is +// a separate package that only depends on both, never the reverse. + +// TestIntegration_FullPipeline_GivenFixture_WhenStatsComputed_ThenCompressionOccurredAndCategoriesPopulated +// verifies that ComputeStats reflects the compression the formatter performs: +// raw text is larger than filtered, and the key categories are non-zero. +func TestIntegration_FullPipeline_GivenFixture_WhenStatsComputed_ThenCompressionOccurredAndCategoriesPopulated(t *testing.T) { + events, err := claudecodec.Codec{}.ReadAll("testdata/integration.jsonl") + if err != nil { + t.Fatalf("read integration fixture: %v", err) + } + + stats := analyzer.ComputeStats(events) + + if stats.RawChars <= stats.FilteredChars { + t.Errorf("expected raw chars > filtered chars (compression happened), got raw=%d filtered=%d", + stats.RawChars, stats.FilteredChars) + } + if stats.Categories["user_text"] == 0 { + t.Errorf("user_text category must be non-zero") + } + if stats.Categories["assistant_text"] == 0 { + t.Errorf("assistant_text category must be non-zero") + } + if stats.Categories["tool_summaries"] == 0 { + t.Errorf("tool_summaries category must be non-zero") + } +} diff --git a/internal/formatter/context.go b/internal/formatter/context.go index 1c511c1..42e2e12 100644 --- a/internal/formatter/context.go +++ b/internal/formatter/context.go @@ -41,7 +41,7 @@ func renderContextEvents(events []session.Event, agentIDs map[string]bool, opts seenSkills := make(map[string]bool) flush := func() { - flushPendingTools(&pendingTools, opts, out) + flushPendingTools(&pendingTools, opts, out, nil) } for _, event := range events { diff --git a/internal/formatter/integration_test.go b/internal/formatter/integration_test.go index 2c79ffd..634a1bf 100644 --- a/internal/formatter/integration_test.go +++ b/internal/formatter/integration_test.go @@ -8,7 +8,6 @@ import ( "strings" "testing" - "github.com/Mapleeeeeeeeeee/cc-session-reader/internal/analyzer" "github.com/Mapleeeeeeeeeee/cc-session-reader/internal/claudecodec" "github.com/Mapleeeeeeeeeee/cc-session-reader/internal/parser" "github.com/Mapleeeeeeeeeee/cc-session-reader/internal/session" @@ -155,29 +154,6 @@ func TestIntegration_FullPipeline_GivenNormalConversation_WhenFormatted_ThenAllT } } -// TestIntegration_FullPipeline_GivenFixture_WhenStatsComputed_ThenCompressionOccurredAndCategoriesPopulated -// verifies that ComputeStats reflects the compression the formatter performs: -// raw text is larger than filtered, and the key categories are non-zero. -func TestIntegration_FullPipeline_GivenFixture_WhenStatsComputed_ThenCompressionOccurredAndCategoriesPopulated(t *testing.T) { - events, _ := readIntegrationFixture(t) - - stats := analyzer.ComputeStats(events) - - if stats.RawChars <= stats.FilteredChars { - t.Errorf("expected raw chars > filtered chars (compression happened), got raw=%d filtered=%d", - stats.RawChars, stats.FilteredChars) - } - if stats.Categories["user_text"] == 0 { - t.Errorf("user_text category must be non-zero") - } - if stats.Categories["assistant_text"] == 0 { - t.Errorf("assistant_text category must be non-zero") - } - if stats.Categories["tool_summaries"] == 0 { - t.Errorf("tool_summaries category must be non-zero") - } -} - // TestIntegration_TranscriptReaderInterface_GivenFixture_WhenReadViaCodecAndDirectly_ThenOutputIdentical // verifies that claudecodec.Codec{}.ReadAll (via TranscriptReader interface) and // claudecodec.ReadAll (direct package call) return identical events for the same fixture. diff --git a/internal/formatter/read.go b/internal/formatter/read.go index 08cb4ea..a4609f4 100644 --- a/internal/formatter/read.go +++ b/internal/formatter/read.go @@ -21,19 +21,34 @@ func FormatRead(transcriptPath string, maxLines int, offset int, opts FormatOpti func FormatReadEvents(events []session.Event, agentIDs map[string]bool, maxLines int, offset int, opts FormatOptions, out io.Writer) error { // Two-pass: format all events into a buffer, then apply offset + maxLines on output lines. var buf bytes.Buffer - if err := renderReadEvents(events, agentIDs, opts, &buf); err != nil { + if err := renderReadEvents(events, agentIDs, opts, &buf, nil); err != nil { return err } return applyPagination(buf.String(), maxLines, offset, out) } +// RenderReadEventsWithSink renders the full read-format output (no pagination — +// the same text `cc-session inherit` injects via inject.RenderFullOutput) while +// reporting every unit of kept content to sink, tagged by category. This lets +// analyzer.ComputeStats derive its KEPT breakdown from the exact same render +// pass that produces the injected text, instead of a second implementation +// that can silently drift from what read/context actually keep (e.g. cc-session +// call collapsing, which only ever happened here). +func RenderReadEventsWithSink(events []session.Event, agentIDs map[string]bool, opts FormatOptions, sink ContentSink) (string, error) { + var buf bytes.Buffer + if err := renderReadEvents(events, agentIDs, opts, &buf, sink); err != nil { + return "", err + } + return buf.String(), nil +} + // renderReadEvents writes the full formatted timeline to out without any line limits. -func renderReadEvents(events []session.Event, agentIDs map[string]bool, opts FormatOptions, out io.Writer) error { +func renderReadEvents(events []session.Event, agentIDs map[string]bool, opts FormatOptions, out io.Writer, sink ContentSink) error { var pendingTools []pendingTool seenSkills := make(map[string]bool) flush := func() { - flushPendingTools(&pendingTools, opts, out) + flushPendingTools(&pendingTools, opts, out, sink) } for _, event := range events { @@ -45,6 +60,9 @@ func renderReadEvents(events []session.Event, agentIDs map[string]bool, opts For } flush() fmt.Fprintf(out, "[%s] user:\n%s\n\n", parser.FormatTimestamp(event.Timestamp), rendered.body) + if sink != nil { + sink(CategoryUserText, rendered.body) + } case session.EventAssistantMessage: if event.Assistant == nil { @@ -61,6 +79,9 @@ func renderReadEvents(events []session.Event, agentIDs map[string]bool, opts For if hasText { flush() fmt.Fprintf(out, "[%s] assistant:\n%s\n", parser.FormatTimestamp(event.Timestamp), event.Assistant.Text) + if sink != nil { + sink(CategoryAssistantText, event.Assistant.Text) + } } for _, tool := range event.Assistant.ToolUses { pendingTools = append(pendingTools, summarizeToolUse(tool)) @@ -70,7 +91,7 @@ func renderReadEvents(events []session.Event, agentIDs map[string]bool, opts For } case session.EventToolResult: - handleToolResultRead(event, agentIDs, &pendingTools, opts, flush, out) + handleToolResultRead(event, agentIDs, &pendingTools, opts, flush, out, sink) } } @@ -85,10 +106,14 @@ func handleToolResultRead( opts FormatOptions, flushFn func(), out io.Writer, + sink ContentSink, ) { if event.User != nil && event.User.IsAnswer { flushFn() fmt.Fprintf(out, "[%s] user (answer):\n%s\n\n", parser.FormatTimestamp(event.Timestamp), event.User.Text) + if sink != nil { + sink(CategoryUserAnswer, event.User.Text) + } return } if event.Tool == nil { @@ -97,6 +122,9 @@ func handleToolResultRead( if agentIDs[event.Tool.ToolUseID] && strings.TrimSpace(event.Tool.Text) != "" { flushFn() fmt.Fprintf(out, "[%s] agent result:\n%s\n\n", parser.FormatTimestamp(event.Timestamp), event.Tool.Text) + if sink != nil { + sink(CategoryToolSummary, event.Tool.Text) + } return } appendToolResult(event.Tool, pendingTools, opts) diff --git a/internal/formatter/render.go b/internal/formatter/render.go index 279bce4..3f3581e 100644 --- a/internal/formatter/render.go +++ b/internal/formatter/render.go @@ -18,6 +18,22 @@ type FormatOptions struct { VerboseCommands bool } +// ContentSink receives each unit of kept content as it is emitted by the read +// render pipeline, tagged with the category it belongs to. analyzer.ComputeStats +// uses this so its KEPT breakdown is derived from the exact same pass that +// produces the injected text (see RenderReadEventsWithSink), instead of a +// second, drift-prone reimplementation of what read/context actually keep. +type ContentSink func(category string, text string) + +// Content categories reported to ContentSink. The values match the keys +// analyzer.StatsResult.Categories uses for its KEPT buckets. +const ( + CategoryUserText = "user_text" + CategoryUserAnswer = "user_answers" + CategoryAssistantText = "assistant_text" + CategoryToolSummary = "tool_summaries" +) + // userRender is the rendered form of a user-message event: the body to print // and whether anything should be printed at all. type userRender struct { @@ -306,13 +322,16 @@ func collapseCCSessionTools(tools []pendingTool) []pendingTool { return result } -func flushPendingTools(pendingTools *[]pendingTool, opts FormatOptions, out io.Writer) { +func flushPendingTools(pendingTools *[]pendingTool, opts FormatOptions, out io.Writer, sink ContentSink) { tools := *pendingTools if !opts.VerboseBash { tools = collapseCCSessionTools(tools) } for _, pt := range tools { fmt.Fprintf(out, " %s\n", pt.summary) + if sink != nil { + sink(CategoryToolSummary, pt.summary) + } } if len(*pendingTools) > 0 { fmt.Fprintln(out) From a5829778f4736cfb986e9b336d83196204b462ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maple=EF=BC=81?= Date: Wed, 15 Jul 2026 17:10:42 +0800 Subject: [PATCH 07/16] refactor: address review findings from /review and /test-review - analyzer references formatter.Category* constants instead of re-typed string literals, so the category-name invariant is compiler-enforced - render pipeline params (agentIDs/opts/out/sink) bundled into a renderContext struct; output verified byte-identical before/after - mutation-guard tests: countContentLines' documented no-trailing-newline and empty-content contract, and the (?m)^Exit code N$ anchor that keeps mid-sentence mentions from sniffing as FAILED - stats test expectation hardcodes "??-?? ??:??" instead of borrowing parser.FormatTimestamp into the expected value Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_013UQYeDtvg1CQYvirZ4jWC5 --- internal/analyzer/render.go | 9 +-- internal/analyzer/stats.go | 20 +++---- internal/analyzer/stats_test.go | 7 ++- internal/claudecodec/reader_test.go | 55 ++++++++++++++++++ .../formatter/analyzer_integration_test.go | 7 ++- internal/formatter/context.go | 3 +- internal/formatter/read.go | 58 ++++++++----------- internal/formatter/render.go | 24 ++++++-- 8 files changed, 124 insertions(+), 59 deletions(-) diff --git a/internal/analyzer/render.go b/internal/analyzer/render.go index 1e45221..4e38201 100644 --- a/internal/analyzer/render.go +++ b/internal/analyzer/render.go @@ -8,6 +8,7 @@ import ( "strconv" "strings" + "github.com/Mapleeeeeeeeeee/cc-session-reader/internal/formatter" "github.com/Mapleeeeeeeeeee/cc-session-reader/internal/skillpath" ) @@ -39,10 +40,10 @@ func RenderStats(out io.Writer, errOut io.Writer, result StatsResult, opts Rende fmt.Fprintln(out, "\n=== Breakdown ===") for _, bl := range []struct{ label, key string }{ - {"KEPT user text: ", "user_text"}, - {"KEPT user answers: ", "user_answers"}, - {"KEPT assistant text: ", "assistant_text"}, - {"KEPT tool summaries: ", "tool_summaries"}, + {"KEPT user text: ", formatter.CategoryUserText}, + {"KEPT user answers: ", formatter.CategoryUserAnswer}, + {"KEPT assistant text: ", formatter.CategoryAssistantText}, + {"KEPT tool summaries: ", formatter.CategoryToolSummary}, {"FMT render overhead: ", "render_overhead"}, {"CUT tool input (raw): ", "tool_input_raw"}, {"CUT tool result (raw):", "tool_result_raw"}, diff --git a/internal/analyzer/stats.go b/internal/analyzer/stats.go index b723748..16c93ef 100644 --- a/internal/analyzer/stats.go +++ b/internal/analyzer/stats.go @@ -46,15 +46,15 @@ type StatsResult struct { func ComputeStats(events []session.Event) StatsResult { var rawParts []string categories := map[string]int{ - "user_text": 0, - "user_answers": 0, - "assistant_text": 0, - "tool_summaries": 0, - "tool_input_raw": 0, - "tool_result_raw": 0, - "system_noise": 0, - "command_noise": 0, - "render_overhead": 0, + formatter.CategoryUserText: 0, + formatter.CategoryUserAnswer: 0, + formatter.CategoryAssistantText: 0, + formatter.CategoryToolSummary: 0, + "tool_input_raw": 0, + "tool_result_raw": 0, + "system_noise": 0, + "command_noise": 0, + "render_overhead": 0, } perTool := map[string]*ToolStats{} toolUseNames := map[string]string{} @@ -197,7 +197,7 @@ func ComputeStats(events []session.Event) StatsResult { // single content category but all of which is real injected bytes. // Surfacing it explicitly keeps KEPT-category-sum + this line exactly // equal to FilteredChars, instead of leaving an unexplained gap. - kept := categories["user_text"] + categories["user_answers"] + categories["assistant_text"] + categories["tool_summaries"] + kept := categories[formatter.CategoryUserText] + categories[formatter.CategoryUserAnswer] + categories[formatter.CategoryAssistantText] + categories[formatter.CategoryToolSummary] categories["render_overhead"] = filteredChars - kept return StatsResult{ diff --git a/internal/analyzer/stats_test.go b/internal/analyzer/stats_test.go index de316de..ed75a9e 100644 --- a/internal/analyzer/stats_test.go +++ b/internal/analyzer/stats_test.go @@ -6,7 +6,6 @@ import ( "testing" "unicode/utf8" - "github.com/Mapleeeeeeeeeee/cc-session-reader/internal/parser" "github.com/Mapleeeeeeeeeee/cc-session-reader/internal/session" ) @@ -101,7 +100,11 @@ func TestComputeStats_CountsCharsForSingleUserMessage(t *testing.T) { t.Fatalf("RawText = %q, want %q", result.RawText, message) } - wantFiltered := fmt.Sprintf("[%s] user:\n%s\n\n", parser.FormatTimestamp(""), message) + // "??-?? ??:??" is the placeholder parser.FormatTimestamp("") returns for a + // missing timestamp; that contract is pinned independently by + // parser.TestFormatTimestamp, so it is hardcoded here rather than + // re-invoking the SUT to build its own expected value. + wantFiltered := fmt.Sprintf("[%s] user:\n%s\n\n", "??-?? ??:??", message) if result.FilteredText != wantFiltered { t.Fatalf("FilteredText = %q, want %q", result.FilteredText, wantFiltered) } diff --git a/internal/claudecodec/reader_test.go b/internal/claudecodec/reader_test.go index 10d186d..a4360f2 100644 --- a/internal/claudecodec/reader_test.go +++ b/internal/claudecodec/reader_test.go @@ -123,6 +123,20 @@ func TestParseLine_ToolResult_GivenExitCodeZero_ThenNotSniffedAsFailed(t *testin } } +// TestParseLine_ToolResult_GivenExitCodeMentionedMidSentence_ThenNotSniffedAsFailed +// guards the nonZeroExitCodeLine regex anchor `(?m)^Exit code (\d+)\s*$`: it +// only matches a bare "Exit code N" line, not the phrase appearing inside +// other text. This protects against a future edit that loosens the anchor +// (e.g. dropping `^`/`$` or switching to a plain substring match), which +// would sniff any result mentioning "Exit code 1" as FAILED regardless of +// context. +func TestParseLine_ToolResult_GivenExitCodeMentionedMidSentence_ThenNotSniffedAsFailed(t *testing.T) { + event := parseLine(t, `{"type":"user","toolUseResult":{"stdout":"","stderr":""},"message":{"role":"user","content":[{"type":"tool_result","tool_use_id":"tool-1","content":"note: Exit code 1 is documented below\nall good"}]}}`) + if event.Tool == nil || !event.Tool.Success { + t.Fatalf("tool result = %#v, want Success=true (mid-sentence mention must not match the bare-line anchor)", event.Tool) + } +} + // TestParseLine_ToolResult_GivenHookErrorText_ThenSniffedAsFailed pins the // second ADR-003 sniff pattern: a hook rejection is reported as ordinary // result content (no success/is_error signal) ending in "... hook error". @@ -197,6 +211,47 @@ func TestParseLine_ToolResult_GivenWriteNewFile_ThenDiffStatCountsContentLines(t } } +// TestParseLine_ToolResult_GivenWriteNewFile_ThenCountContentLinesMatchesDocumentedContract +// pins the countContentLines doc comment (model.go): a trailing newline is the +// common text-file convention and must not count as an extra blank line, while +// truly empty content reports zero lines. The existing diff-stat test above +// only exercises content with a trailing newline, leaving these two boundary +// cases unguarded. +func TestParseLine_ToolResult_GivenWriteNewFile_ThenCountContentLinesMatchesDocumentedContract(t *testing.T) { + tests := []struct { + name string + content string + wantNewFile bool + wantFileLines int + }{ + {name: "no trailing newline counts visible lines", content: "a\nb", wantNewFile: true, wantFileLines: 2}, + {name: "empty content reports zero lines", content: "", wantNewFile: true, wantFileLines: 0}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + contentJSON, err := json.Marshal(tt.content) + if err != nil { + t.Fatalf("marshal content: %v", err) + } + line := `{"type":"user","toolUseResult":{"success":true,"commandName":"Write","structuredPatch":[],` + + `"content":` + string(contentJSON) + `},` + + `"message":{"role":"user","content":[{"type":"tool_result","tool_use_id":"tool-1","content":"ok"}]}}` + event := parseLine(t, line) + + stat := event.Tool.DiffStat + if stat == nil { + t.Fatal("DiffStat is nil, want populated stat") + } + if stat.IsNewFile != tt.wantNewFile { + t.Fatalf("IsNewFile = %v, want %v", stat.IsNewFile, tt.wantNewFile) + } + if stat.NewFileLines != tt.wantFileLines { + t.Fatalf("NewFileLines = %d, want %d", stat.NewFileLines, tt.wantFileLines) + } + }) + } +} + // TestParseLine_ToolResult_GivenEditWithoutStructuredPatch_ThenDiffStatNil // guards the ADR-003 fallback: a successful Edit whose toolUseResult carries // no structuredPatch (missing/unparsable) must not synthesize a fake diff — diff --git a/internal/formatter/analyzer_integration_test.go b/internal/formatter/analyzer_integration_test.go index adaf468..a0e4e4b 100644 --- a/internal/formatter/analyzer_integration_test.go +++ b/internal/formatter/analyzer_integration_test.go @@ -5,6 +5,7 @@ import ( "github.com/Mapleeeeeeeeeee/cc-session-reader/internal/analyzer" "github.com/Mapleeeeeeeeeee/cc-session-reader/internal/claudecodec" + "github.com/Mapleeeeeeeeeee/cc-session-reader/internal/formatter" ) // This file lives in the external formatter_test package (not formatter) @@ -29,13 +30,13 @@ func TestIntegration_FullPipeline_GivenFixture_WhenStatsComputed_ThenCompression t.Errorf("expected raw chars > filtered chars (compression happened), got raw=%d filtered=%d", stats.RawChars, stats.FilteredChars) } - if stats.Categories["user_text"] == 0 { + if stats.Categories[formatter.CategoryUserText] == 0 { t.Errorf("user_text category must be non-zero") } - if stats.Categories["assistant_text"] == 0 { + if stats.Categories[formatter.CategoryAssistantText] == 0 { t.Errorf("assistant_text category must be non-zero") } - if stats.Categories["tool_summaries"] == 0 { + if stats.Categories[formatter.CategoryToolSummary] == 0 { t.Errorf("tool_summaries category must be non-zero") } } diff --git a/internal/formatter/context.go b/internal/formatter/context.go index 42e2e12..befaa0b 100644 --- a/internal/formatter/context.go +++ b/internal/formatter/context.go @@ -39,9 +39,10 @@ func FormatContextEvents(events []session.Event, agentIDs map[string]bool, maxLi func renderContextEvents(events []session.Event, agentIDs map[string]bool, opts FormatOptions, out io.Writer) error { var pendingTools []pendingTool seenSkills := make(map[string]bool) + rc := renderContext{agentIDs: agentIDs, opts: opts, out: out} flush := func() { - flushPendingTools(&pendingTools, opts, out, nil) + flushPendingTools(&pendingTools, rc) } for _, event := range events { diff --git a/internal/formatter/read.go b/internal/formatter/read.go index a4609f4..7711318 100644 --- a/internal/formatter/read.go +++ b/internal/formatter/read.go @@ -21,7 +21,7 @@ func FormatRead(transcriptPath string, maxLines int, offset int, opts FormatOpti func FormatReadEvents(events []session.Event, agentIDs map[string]bool, maxLines int, offset int, opts FormatOptions, out io.Writer) error { // Two-pass: format all events into a buffer, then apply offset + maxLines on output lines. var buf bytes.Buffer - if err := renderReadEvents(events, agentIDs, opts, &buf, nil); err != nil { + if err := renderReadEvents(events, renderContext{agentIDs: agentIDs, opts: opts, out: &buf}); err != nil { return err } return applyPagination(buf.String(), maxLines, offset, out) @@ -36,62 +36,62 @@ func FormatReadEvents(events []session.Event, agentIDs map[string]bool, maxLines // call collapsing, which only ever happened here). func RenderReadEventsWithSink(events []session.Event, agentIDs map[string]bool, opts FormatOptions, sink ContentSink) (string, error) { var buf bytes.Buffer - if err := renderReadEvents(events, agentIDs, opts, &buf, sink); err != nil { + if err := renderReadEvents(events, renderContext{agentIDs: agentIDs, opts: opts, out: &buf, sink: sink}); err != nil { return "", err } return buf.String(), nil } -// renderReadEvents writes the full formatted timeline to out without any line limits. -func renderReadEvents(events []session.Event, agentIDs map[string]bool, opts FormatOptions, out io.Writer, sink ContentSink) error { +// renderReadEvents writes the full formatted timeline to rc.out without any line limits. +func renderReadEvents(events []session.Event, rc renderContext) error { var pendingTools []pendingTool seenSkills := make(map[string]bool) flush := func() { - flushPendingTools(&pendingTools, opts, out, sink) + flushPendingTools(&pendingTools, rc) } for _, event := range events { switch event.Kind { case session.EventUserMessage: - rendered := renderUserMessage(event.User, opts, seenSkills) + rendered := renderUserMessage(event.User, rc.opts, seenSkills) if !rendered.show { continue } flush() - fmt.Fprintf(out, "[%s] user:\n%s\n\n", parser.FormatTimestamp(event.Timestamp), rendered.body) - if sink != nil { - sink(CategoryUserText, rendered.body) + fmt.Fprintf(rc.out, "[%s] user:\n%s\n\n", parser.FormatTimestamp(event.Timestamp), rendered.body) + if rc.sink != nil { + rc.sink(CategoryUserText, rendered.body) } case session.EventAssistantMessage: if event.Assistant == nil { continue } - if opts.VerboseThinking { + if rc.opts.VerboseThinking { for _, thinking := range event.Assistant.Thinking { flush() - fmt.Fprintf(out, "[%s] thinking:\n%s\n\n", parser.FormatTimestamp(event.Timestamp), thinking) + fmt.Fprintf(rc.out, "[%s] thinking:\n%s\n\n", parser.FormatTimestamp(event.Timestamp), thinking) } } hasText := strings.TrimSpace(event.Assistant.Text) != "" hasTools := len(event.Assistant.ToolUses) > 0 if hasText { flush() - fmt.Fprintf(out, "[%s] assistant:\n%s\n", parser.FormatTimestamp(event.Timestamp), event.Assistant.Text) - if sink != nil { - sink(CategoryAssistantText, event.Assistant.Text) + fmt.Fprintf(rc.out, "[%s] assistant:\n%s\n", parser.FormatTimestamp(event.Timestamp), event.Assistant.Text) + if rc.sink != nil { + rc.sink(CategoryAssistantText, event.Assistant.Text) } } for _, tool := range event.Assistant.ToolUses { pendingTools = append(pendingTools, summarizeToolUse(tool)) } if hasText && !hasTools { - fmt.Fprintln(out) + fmt.Fprintln(rc.out) } case session.EventToolResult: - handleToolResultRead(event, agentIDs, &pendingTools, opts, flush, out, sink) + handleToolResultRead(event, &pendingTools, flush, rc) } } @@ -99,33 +99,25 @@ func renderReadEvents(events []session.Event, agentIDs map[string]bool, opts For return nil } -func handleToolResultRead( - event session.Event, - agentIDs map[string]bool, - pendingTools *[]pendingTool, - opts FormatOptions, - flushFn func(), - out io.Writer, - sink ContentSink, -) { +func handleToolResultRead(event session.Event, pendingTools *[]pendingTool, flushFn func(), rc renderContext) { if event.User != nil && event.User.IsAnswer { flushFn() - fmt.Fprintf(out, "[%s] user (answer):\n%s\n\n", parser.FormatTimestamp(event.Timestamp), event.User.Text) - if sink != nil { - sink(CategoryUserAnswer, event.User.Text) + fmt.Fprintf(rc.out, "[%s] user (answer):\n%s\n\n", parser.FormatTimestamp(event.Timestamp), event.User.Text) + if rc.sink != nil { + rc.sink(CategoryUserAnswer, event.User.Text) } return } if event.Tool == nil { return } - if agentIDs[event.Tool.ToolUseID] && strings.TrimSpace(event.Tool.Text) != "" { + if rc.agentIDs[event.Tool.ToolUseID] && strings.TrimSpace(event.Tool.Text) != "" { flushFn() - fmt.Fprintf(out, "[%s] agent result:\n%s\n\n", parser.FormatTimestamp(event.Timestamp), event.Tool.Text) - if sink != nil { - sink(CategoryToolSummary, event.Tool.Text) + fmt.Fprintf(rc.out, "[%s] agent result:\n%s\n\n", parser.FormatTimestamp(event.Timestamp), event.Tool.Text) + if rc.sink != nil { + rc.sink(CategoryToolSummary, event.Tool.Text) } return } - appendToolResult(event.Tool, pendingTools, opts) + appendToolResult(event.Tool, pendingTools, rc.opts) } diff --git a/internal/formatter/render.go b/internal/formatter/render.go index 3f3581e..18516bd 100644 --- a/internal/formatter/render.go +++ b/internal/formatter/render.go @@ -25,6 +25,18 @@ type FormatOptions struct { // second, drift-prone reimplementation of what read/context actually keep. type ContentSink func(category string, text string) +// renderContext bundles the render-invariant values threaded unchanged through +// the read/context render pipelines (renderReadEvents -> handleToolResultRead +// -> flushPendingTools, and renderContextEvents -> flushPendingTools) so +// callers pass them once instead of repeating agentIDs/opts/out/sink as +// positional parameters at every layer. +type renderContext struct { + agentIDs map[string]bool + opts FormatOptions + out io.Writer + sink ContentSink +} + // Content categories reported to ContentSink. The values match the keys // analyzer.StatsResult.Categories uses for its KEPT buckets. const ( @@ -322,19 +334,19 @@ func collapseCCSessionTools(tools []pendingTool) []pendingTool { return result } -func flushPendingTools(pendingTools *[]pendingTool, opts FormatOptions, out io.Writer, sink ContentSink) { +func flushPendingTools(pendingTools *[]pendingTool, rc renderContext) { tools := *pendingTools - if !opts.VerboseBash { + if !rc.opts.VerboseBash { tools = collapseCCSessionTools(tools) } for _, pt := range tools { - fmt.Fprintf(out, " %s\n", pt.summary) - if sink != nil { - sink(CategoryToolSummary, pt.summary) + fmt.Fprintf(rc.out, " %s\n", pt.summary) + if rc.sink != nil { + rc.sink(CategoryToolSummary, pt.summary) } } if len(*pendingTools) > 0 { - fmt.Fprintln(out) + fmt.Fprintln(rc.out) } *pendingTools = (*pendingTools)[:0] } From c26aa472248ac671eca8e6474bb979bef6a2d355 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maple=EF=BC=81?= Date: Thu, 16 Jul 2026 09:57:37 +0800 Subject: [PATCH 08/16] feat: audit semantic histogram and failure-length distribution Bucket CUT content by semantic risk (failure_output first, harness_noise last), report failed-result length stats (count/median/p90/max) to inform the upcoming head+tail error retention threshold, and annotate audit samples with their bucket. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01SaBPDHw1cZDxhehYrqFgGe --- cmd/cc-session/audit.go | 27 ++- cmd/cc-session/e2e_test.go | 2 +- internal/analyzer/audit.go | 222 ++++++++++++++++++++-- internal/analyzer/audit_test.go | 322 ++++++++++++++++++++++++++++++-- 4 files changed, 539 insertions(+), 34 deletions(-) diff --git a/cmd/cc-session/audit.go b/cmd/cc-session/audit.go index 51c02d0..991bc28 100644 --- a/cmd/cc-session/audit.go +++ b/cmd/cc-session/audit.go @@ -39,13 +39,34 @@ func runAudit(args []string, out io.Writer, errOut io.Writer, store parser.Store result := analyzer.ComputeAudit(events) - for _, catName := range []string{"tool_result_cut", "system_noise", "thinking"} { - items := result.Categories[catName] + fmt.Fprintln(out, "=== CUT-content histogram (risk-descending) ===") + for _, bucket := range analyzer.BucketOrder { + items := result.Samples[bucket] + if len(items) == 0 { + continue + } + fmt.Fprintf(out, " %-24s %6s items %12s chars\n", + bucket, analyzer.FormatNumber(len(items)), analyzer.FormatNumber(result.BucketChars[bucket])) + } + + fmt.Fprintln(out, "\n=== Failed tool result length (chars) ===") + if result.Failures.Count == 0 { + fmt.Fprintln(out, " no failed tool results in this session") + } else { + f := result.Failures + fmt.Fprintf(out, " count: %s median: %s p90: %s max: %s\n", + analyzer.FormatNumber(f.Count), analyzer.FormatNumber(f.Median), + analyzer.FormatNumber(f.P90), analyzer.FormatNumber(f.Max)) + } + + fmt.Fprintln(out, "\n=== Samples ===") + for _, bucket := range analyzer.BucketOrder { + items := result.Samples[bucket] if len(items) == 0 { continue } shown := sampleCount(*samples, len(items)) - fmt.Fprintf(out, "=== %s (%d items, showing %d) ===\n", catName, len(items), shown) + fmt.Fprintf(out, "=== %s (%d items, showing %d) ===\n", bucket, len(items), shown) for _, item := range items[:shown] { fmt.Fprintf(out, " %s\n\n", item) } diff --git a/cmd/cc-session/e2e_test.go b/cmd/cc-session/e2e_test.go index b303dff..58a5726 100644 --- a/cmd/cc-session/e2e_test.go +++ b/cmd/cc-session/e2e_test.go @@ -77,7 +77,7 @@ func TestCLI_WhenSessionExists_ThenListReadContextAndAuditWorkEndToEnd(t *testin name: "audit samples cut tool results", args: []string{"audit", sid, "-n", "5"}, want: []string{ - "=== tool_result_cut (1 items, showing 1) ===", + "=== success_output_bash (1 items, showing 1) ===", }, }, { diff --git a/internal/analyzer/audit.go b/internal/analyzer/audit.go index d3614d0..2f954ef 100644 --- a/internal/analyzer/audit.go +++ b/internal/analyzer/audit.go @@ -2,20 +2,90 @@ package analyzer import ( "fmt" + "math" + "sort" "strings" + "unicode/utf8" "github.com/Mapleeeeeeeeeee/cc-session-reader/internal/session" ) +// Bucket names for the CUT-content histogram: every unit of transcript text +// that the default render pipeline (formatter.RenderReadEventsWithSink, the +// same pass ComputeStats measures) never surfaces in the injected output. +// BucketOrder lists them from highest risk (content a reader most likely +// wants back) to lowest risk (safe framework noise), so `cc-session audit` +// can print top-to-bottom in descending order of "worry about this". +const ( + BucketFailureOutput = "failure_output" + BucketSuccessReadFile = "success_output_read_file" + BucketSuccessBash = "success_output_bash" + BucketSuccessAgent = "success_output_agent" + BucketSuccessOther = "success_output_other" + BucketToolInput = "tool_input" + BucketThinking = "thinking" + BucketHarnessNoise = "harness_noise" +) + +// BucketOrder is the canonical risk-descending presentation order for the CUT +// histogram. +var BucketOrder = []string{ + BucketFailureOutput, + BucketSuccessReadFile, + BucketSuccessBash, + BucketSuccessAgent, + BucketSuccessOther, + BucketToolInput, + BucketThinking, + BucketHarnessNoise, +} + +// sampleTruncateRunes bounds each audit sample's displayed length so one huge +// tool result or thinking block can't dominate the printed report. +const sampleTruncateRunes = 300 + +// FailureLengthStats summarizes the raw character length of every failed +// tool result's original text (ungated by whatever the render pipeline +// would keep). It feeds the "would a keep-short-failures-verbatim budget be +// cheap" decision. Count is zero when the session has no failed tool +// results; Median/P90/Max are meaningless in that case. +type FailureLengthStats struct { + Count int + Median int + P90 int + Max int +} + +// AuditResult holds the CUT-content histogram and its samples. type AuditResult struct { - Categories map[string][]string + // BucketChars is the total CUT character count per bucket, keyed by the + // Bucket* constants above. + BucketChars map[string]int + // Samples holds a handful of representative CUT excerpts per bucket, so + // callers can print "here's what actually got dropped" alongside the + // counts. Every sample is already tagged by its map key. + Samples map[string][]string + // Failures is the raw-length distribution of failed tool results. + Failures FailureLengthStats } +// ComputeAudit walks events once and classifies every unit of CUT content +// (text the default render pipeline drops or shrinks) into the risk-ordered +// buckets above, alongside the raw-length distribution of failed tool +// results. func ComputeAudit(events []session.Event) AuditResult { - categories := map[string][]string{ - "tool_result_cut": {}, - "system_noise": {}, - "thinking": {}, + bucketChars := make(map[string]int, len(BucketOrder)) + samples := make(map[string][]string, len(BucketOrder)) + toolUseNames := map[string]string{} + var failureLengths []int + + // addSample records chars CUT characters into bucket's total and appends + // display (a truncated, possibly tool-name-prefixed excerpt) as a sample. + // chars is always measured from the untruncated raw content, never from + // display, so truncation/prefixing never distorts the histogram totals. + addSample := func(bucket string, chars int, display string) { + bucketChars[bucket] += chars + samples[bucket] = append(samples[bucket], display) } for _, event := range events { @@ -24,33 +94,147 @@ func ComputeAudit(events []session.Event) AuditResult { if event.Noise == nil || strings.TrimSpace(event.Noise.Text) == "" { continue } - categories["system_noise"] = append(categories["system_noise"], - fmt.Sprintf("[%s] %s", event.RawType, session.Truncate(event.Noise.Text, 200))) + addSample(BucketHarnessNoise, utf8.RuneCountInString(event.Noise.Text), + fmt.Sprintf("[%s] %s", event.RawType, session.Truncate(event.Noise.Text, sampleTruncateRunes))) - case session.EventToolResult: - if event.Tool == nil || event.User != nil && event.User.IsAnswer { + case session.EventUserMessage: + if event.User == nil || !isHarnessNoiseMessage(event.User) { continue } - if strings.TrimSpace(event.Tool.Text) != "" && len(event.Tool.Text) > 100 { - name := event.Tool.RawName - if name == "" { - name = "?" - } - categories["tool_result_cut"] = append(categories["tool_result_cut"], - fmt.Sprintf("[%s] %s", name, session.Truncate(event.Tool.Text, 300))) + if strings.TrimSpace(event.User.Text) == "" { + continue } + addSample(BucketHarnessNoise, utf8.RuneCountInString(event.User.Text), + session.Truncate(event.User.Text, sampleTruncateRunes)) case session.EventAssistantMessage: if event.Assistant == nil { continue } for _, thinking := range event.Assistant.Thinking { - if strings.TrimSpace(thinking) != "" { - categories["thinking"] = append(categories["thinking"], session.Truncate(thinking, 300)) + if strings.TrimSpace(thinking) == "" { + continue + } + addSample(BucketThinking, utf8.RuneCountInString(thinking), session.Truncate(thinking, sampleTruncateRunes)) + } + for _, tool := range event.Assistant.ToolUses { + name := tool.Name + if name == "" { + name = "?" } + toolUseNames[tool.ID] = name + + rawJSON := tool.Input.MarshalNoEscape() + addSample(BucketToolInput, utf8.RuneCountInString(rawJSON), + fmt.Sprintf("[%s] %s", name, session.Truncate(rawJSON, sampleTruncateRunes))) + } + + case session.EventToolResult: + if event.Tool == nil || (event.User != nil && event.User.IsAnswer) { + continue + } + text := event.Tool.Text + if strings.TrimSpace(text) == "" { + continue } + + if !event.Tool.Success { + failureLengths = append(failureLengths, utf8.RuneCountInString(text)) + } + + // cutChars approximates how much of this result's raw text never + // reaches the reader. result.Summary() is exactly what the render + // pipeline keeps (formatter/render.go calls the same method), so + // raw-minus-summary is the CUT remainder — modulo the few bytes + // Summary() spends on its own " -> ok"/" -> FAILED" prefix, which + // isn't "kept" raw content either. That makes this a slightly + // conservative (never-overcounts-CUT) estimate, not an exact one. + cutChars := utf8.RuneCountInString(text) - utf8.RuneCountInString(event.Tool.Summary()) + if cutChars <= 0 { + continue + } + + toolName := event.Tool.RawName + if toolName == "" { + toolName = toolUseNames[event.Tool.ToolUseID] + } + if toolName == "" { + toolName = "?" + } + + bucket := resultBucket(event.Tool.Success, toolName) + addSample(bucket, cutChars, fmt.Sprintf("[%s] %s", toolName, session.Truncate(text, sampleTruncateRunes))) } } - return AuditResult{Categories: categories} + return AuditResult{ + BucketChars: bucketChars, + Samples: samples, + Failures: computeFailureLengthStats(failureLengths), + } +} + +// isHarnessNoiseMessage reports whether user carries one of the harness- +// injected subtypes that formatter's render pipeline drops entirely or +// compacts to a fraction of its raw size (system-reminder, context-usage, +// command injection, teammate warnings, and machine command output) — the +// "safe to lose" bucket. Skill injections are excluded: they inject real +// SKILL.md instructions the user opted into, not framework boilerplate, so +// lumping them in here would misrepresent them as noise. +func isHarnessNoiseMessage(user *session.UserMessage) bool { + return user.IsSystemReminder || user.IsContextUsage || user.IsCommandInjection || + user.IsTeammateMessage || user.IsCommandNoise +} + +// resultBucket classifies a tool result into its CUT bucket. Failed results +// always land in BucketFailureOutput regardless of tool; successful results +// split by family using the same {Read, Write, Edit, Agent} vs. everything- +// else grouping that session.ToolResult.Summary() already uses to decide +// between a bare "-> ok" and a first-line excerpt (ADR-002). +func resultBucket(success bool, toolName string) string { + if !success { + return BucketFailureOutput + } + switch toolName { + case session.ToolRead, session.ToolWrite, session.ToolEdit: + return BucketSuccessReadFile + case session.ToolBash: + return BucketSuccessBash + case session.ToolAgent: + return BucketSuccessAgent + default: + return BucketSuccessOther + } +} + +// computeFailureLengthStats returns the zero-value FailureLengthStats for an +// empty input (no failures in the session). +func computeFailureLengthStats(lengths []int) FailureLengthStats { + if len(lengths) == 0 { + return FailureLengthStats{} + } + sorted := append([]int(nil), lengths...) + sort.Ints(sorted) + return FailureLengthStats{ + Count: len(sorted), + Median: percentile(sorted, 0.5), + P90: percentile(sorted, 0.9), + Max: sorted[len(sorted)-1], + } +} + +// percentile returns the p-th percentile (0 < p <= 1) of sorted (ascending, +// non-empty) using the nearest-rank method: rank = ceil(p*n), 1-indexed. +// Nearest-rank avoids interpolation complexity that isn't warranted for the +// small failure counts a single session typically has. +func percentile(sorted []int, p float64) int { + n := len(sorted) + idx := int(math.Ceil(p*float64(n))) - 1 + if idx < 0 { + idx = 0 + } + if idx >= n { + idx = n - 1 + } + return sorted[idx] } diff --git a/internal/analyzer/audit_test.go b/internal/analyzer/audit_test.go index c4d7795..48f2f78 100644 --- a/internal/analyzer/audit_test.go +++ b/internal/analyzer/audit_test.go @@ -1,13 +1,21 @@ package analyzer import ( + "fmt" + "os" + "path/filepath" "strings" "testing" "unicode/utf8" + "github.com/Mapleeeeeeeeeee/cc-session-reader/internal/claudecodec" "github.com/Mapleeeeeeeeeee/cc-session-reader/internal/session" ) +// TestComputeAudit_ToolResultCutUsesRuneSafeTruncation guards that a sample +// straddling a multi-byte rune boundary at the truncation cap is truncated on +// a rune boundary, not a byte boundary (which would either panic or split a +// rune in half). func TestComputeAudit_ToolResultCutUsesRuneSafeTruncation(t *testing.T) { text := strings.Repeat("a", 299) + "你" events := []session.Event{ @@ -18,9 +26,9 @@ func TestComputeAudit_ToolResultCutUsesRuneSafeTruncation(t *testing.T) { } result := ComputeAudit(events) - items := result.Categories["tool_result_cut"] + items := result.Samples[BucketSuccessBash] if len(items) != 1 { - t.Fatalf("tool_result_cut count = %d, want 1", len(items)) + t.Fatalf("success_output_bash count = %d, want 1", len(items)) } if !utf8.ValidString(items[0]) { t.Fatalf("audit sample is not valid UTF-8: %q", items[0]) @@ -30,24 +38,316 @@ func TestComputeAudit_ToolResultCutUsesRuneSafeTruncation(t *testing.T) { } } -func TestComputeAudit_CategorizesSystemNoiseAndThinking(t *testing.T) { +// TestComputeAudit_GivenFailedBashResult_ThenBucketedAsFailureOutputWithCutChars +// pins the failure_output bucket: cutChars is raw text length minus the +// literal Summary() the render pipeline keeps (session.ToolResult.Summary()'s +// format is already pinned by TestToolResultSummary in event_test.go, so it is +// hardcoded here as the "kept" reference rather than re-derived from the SUT). +func TestComputeAudit_GivenFailedBashResult_ThenBucketedAsFailureOutputWithCutChars(t *testing.T) { + text := "boom\n" + strings.Repeat("z", 300) + const wantKept = " -> FAILED: boom" // first line "boom", not noise, well under the 200-rune excerpt budget + events := []session.Event{ { - Kind: session.EventNoise, - RawType: "system", - Noise: &session.NoiseEvent{Text: "system details"}, + Kind: session.EventToolResult, + Tool: &session.ToolResult{Success: false, RawName: "Bash", Text: text}, }, + } + + result := ComputeAudit(events) + + wantCut := utf8.RuneCountInString(text) - utf8.RuneCountInString(wantKept) + if got := result.BucketChars[BucketFailureOutput]; got != wantCut { + t.Fatalf("BucketChars[failure_output] = %d, want %d", got, wantCut) + } + items := result.Samples[BucketFailureOutput] + if len(items) != 1 || !strings.HasPrefix(items[0], "[Bash] boom") { + t.Fatalf("Samples[failure_output] = %#v, want one item prefixed \"[Bash] boom\"", items) + } + // A failed result must never leak into a success_output_* bucket. + for _, bucket := range []string{BucketSuccessReadFile, BucketSuccessBash, BucketSuccessAgent, BucketSuccessOther} { + if len(result.Samples[bucket]) != 0 { + t.Fatalf("Samples[%s] = %#v, want empty for a failed result", bucket, result.Samples[bucket]) + } + } +} + +// TestComputeAudit_GivenSuccessfulToolResults_ThenBucketedByToolFamily pins +// the success_output_* family split: Read/Write/Edit/Agent collapse to the +// bare "-> ok" excerpt (ADR-002's suppression, reused via Summary()), so they +// lose more of their raw text than Bash/other tools which keep a first-line +// excerpt. +func TestComputeAudit_GivenSuccessfulToolResults_ThenBucketedByToolFamily(t *testing.T) { + const text = "result line\n" // + filler below + const firstLine = "result line" + filler := strings.Repeat("y", 200) + fullText := text + filler + rawChars := utf8.RuneCountInString(fullText) + + tests := []struct { + name string + toolName string + wantBucket string + wantKept string + }{ + {"Read collapses to bare ok", session.ToolRead, BucketSuccessReadFile, " -> ok"}, + {"Write collapses to bare ok", session.ToolWrite, BucketSuccessReadFile, " -> ok"}, + {"Edit collapses to bare ok", session.ToolEdit, BucketSuccessReadFile, " -> ok"}, + {"Agent collapses to bare ok", session.ToolAgent, BucketSuccessAgent, " -> ok"}, + {"Bash keeps a first-line excerpt", session.ToolBash, BucketSuccessBash, " -> ok: " + firstLine}, + {"Grep (other) keeps a first-line excerpt", "Grep", BucketSuccessOther, " -> ok: " + firstLine}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + events := []session.Event{ + { + Kind: session.EventToolResult, + Tool: &session.ToolResult{Success: true, RawName: tt.toolName, Text: fullText}, + }, + } + + result := ComputeAudit(events) + + wantCut := rawChars - utf8.RuneCountInString(tt.wantKept) + if got := result.BucketChars[tt.wantBucket]; got != wantCut { + t.Fatalf("BucketChars[%s] = %d, want %d", tt.wantBucket, got, wantCut) + } + if len(result.Samples[tt.wantBucket]) != 1 { + t.Fatalf("Samples[%s] = %#v, want exactly one sample", tt.wantBucket, result.Samples[tt.wantBucket]) + } + }) + } +} + +// TestComputeAudit_GivenTrivialToolResult_ThenNotSampled guards the +// cutChars<=0 skip: a result so short that Summary()'s own status wording +// outweighs it carries nothing CUT, so it must not pollute the histogram or +// sample list. +func TestComputeAudit_GivenTrivialToolResult_ThenNotSampled(t *testing.T) { + events := []session.Event{ + { + Kind: session.EventToolResult, + Tool: &session.ToolResult{Success: true, RawName: session.ToolBash, Text: "ok"}, + }, + } + + result := ComputeAudit(events) + + for _, bucket := range BucketOrder { + if got := result.BucketChars[bucket]; got != 0 { + t.Fatalf("BucketChars[%s] = %d, want 0 for a trivial result", bucket, got) + } + if len(result.Samples[bucket]) != 0 { + t.Fatalf("Samples[%s] = %#v, want empty for a trivial result", bucket, result.Samples[bucket]) + } + } +} + +// TestComputeAudit_TracksToolInputCutInFull verifies that a tool_use's raw +// input JSON is entirely CUT (the render pipeline only ever shows a derived +// one-line description, never the JSON itself, so there is no "kept" portion +// to subtract — unlike tool results, which keep a Summary() excerpt). +func TestComputeAudit_TracksToolInputCutInFull(t *testing.T) { + input := session.ToolInput{Raw: map[string]any{"command": "ls -la", "description": "List files"}} + events := []session.Event{ + { + Kind: session.EventAssistantMessage, + Assistant: &session.AssistantMessage{ + ToolUses: []session.ToolUse{{ID: "t1", Name: "Bash", Input: input}}, + }, + }, + } + + result := ComputeAudit(events) + + want := utf8.RuneCountInString(input.MarshalNoEscape()) + if got := result.BucketChars[BucketToolInput]; got != want { + t.Fatalf("BucketChars[tool_input] = %d, want %d", got, want) + } + items := result.Samples[BucketToolInput] + if len(items) != 1 || !strings.HasPrefix(items[0], "[Bash]") { + t.Fatalf("Samples[tool_input] = %#v, want one item prefixed \"[Bash]\"", items) + } +} + +// TestComputeAudit_TracksThinkingCutInFull verifies thinking blocks are +// entirely CUT: the default render pipeline never surfaces them (VerboseThinking +// is off), so the full block length counts, matching ComputeStats' own +// omission of thinking from its raw/filtered accounting. +func TestComputeAudit_TracksThinkingCutInFull(t *testing.T) { + events := []session.Event{ { Kind: session.EventAssistantMessage, - Assistant: &session.AssistantMessage{Thinking: []string{"private reasoning"}}, + Assistant: &session.AssistantMessage{Thinking: []string{"private reasoning about the plan"}}, + }, + } + + result := ComputeAudit(events) + + want := utf8.RuneCountInString("private reasoning about the plan") + if got := result.BucketChars[BucketThinking]; got != want { + t.Fatalf("BucketChars[thinking] = %d, want %d", got, want) + } + if len(result.Samples[BucketThinking]) != 1 { + t.Fatalf("Samples[thinking] = %#v, want one item", result.Samples[BucketThinking]) + } +} + +// TestComputeAudit_BucketsHarnessNoiseSources pins every user-message subtype +// the render pipeline treats as safe-to-drop framework boilerplate (system +// reminders, context-usage tables, command injections, teammate warnings, +// and machine command output) as harness_noise, and confirms skill +// injections are excluded — they carry real user-requested SKILL.md content, +// not framework noise, so lumping them in here would mislabel them. +func TestComputeAudit_BucketsHarnessNoiseSources(t *testing.T) { + tests := []struct { + name string + user *session.UserMessage + want bool // true: classified as harness_noise + }{ + {"system reminder", &session.UserMessage{IsSystemReminder: true, Text: "reminder body"}, true}, + {"context usage table", &session.UserMessage{IsContextUsage: true, Text: "context table"}, true}, + {"command injection", &session.UserMessage{IsCommandInjection: true, Text: "/cc-session read abc"}, true}, + {"teammate warning", &session.UserMessage{IsTeammateMessage: true, Text: "teammate said hi"}, true}, + {"command noise", &session.UserMessage{IsCommandNoise: true, Text: "bash stdout dump"}, true}, + {"skill injection excluded", &session.UserMessage{IsSkillInjection: true, SkillName: "cc-session", Text: "skill body"}, false}, + {"plain user text excluded", &session.UserMessage{Text: "hello there"}, false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + events := []session.Event{{Kind: session.EventUserMessage, User: tt.user}} + result := ComputeAudit(events) + + got := len(result.Samples[BucketHarnessNoise]) == 1 + if got != tt.want { + t.Fatalf("classified as harness_noise = %v, want %v (samples: %#v)", got, tt.want, result.Samples[BucketHarnessNoise]) + } + if tt.want { + want := utf8.RuneCountInString(tt.user.Text) + if got := result.BucketChars[BucketHarnessNoise]; got != want { + t.Fatalf("BucketChars[harness_noise] = %d, want %d", got, want) + } + } + }) + } +} + +// TestComputeAudit_CategorizesSystemNoise verifies EventNoise entries (system, +// file-history-snapshot, etc.) are bucketed as harness_noise in full, matching +// ComputeStats' own "this is machine boilerplate" treatment of the same +// events. +func TestComputeAudit_CategorizesSystemNoise(t *testing.T) { + events := []session.Event{ + { + Kind: session.EventNoise, + RawType: "system", + Noise: &session.NoiseEvent{Text: "system details"}, }, } result := ComputeAudit(events) - if got := len(result.Categories["system_noise"]); got != 1 { - t.Fatalf("system_noise count = %d, want 1", got) + if got := len(result.Samples[BucketHarnessNoise]); got != 1 { + t.Fatalf("Samples[harness_noise] count = %d, want 1", got) } - if got := len(result.Categories["thinking"]); got != 1 { - t.Fatalf("thinking count = %d, want 1", got) + want := utf8.RuneCountInString("system details") + if got := result.BucketChars[BucketHarnessNoise]; got != want { + t.Fatalf("BucketChars[harness_noise] = %d, want %d", got, want) + } +} + +// TestComputeAudit_GivenNoFailures_ThenFailureStatsZero pins the explicit +// "no failures in this session" case for the failure-length distribution: +// zero failed tool results must report a zero-value FailureLengthStats, not +// a division-by-zero panic or garbage percentile. +func TestComputeAudit_GivenNoFailures_ThenFailureStatsZero(t *testing.T) { + events := []session.Event{ + {Kind: session.EventToolResult, Tool: &session.ToolResult{Success: true, RawName: "Bash", Text: "all good"}}, + } + + result := ComputeAudit(events) + + if result.Failures != (FailureLengthStats{}) { + t.Fatalf("Failures = %#v, want zero-value FailureLengthStats", result.Failures) + } +} + +// TestComputeAudit_GivenMultipleFailures_ThenComputesMedianP90Max pins the +// nearest-rank percentile computation against a hand-checkable set of five +// known lengths: median is the classic middle value of an odd-sized sorted +// set, and p90 of 5 elements lands on the last (5th) element under +// nearest-rank (ceil(0.9*5)=5). +func TestComputeAudit_GivenMultipleFailures_ThenComputesMedianP90Max(t *testing.T) { + lengths := []int{40, 10, 100, 30, 20} // sorted: 10, 20, 30, 40, 100 + events := make([]session.Event, 0, len(lengths)) + for i, n := range lengths { + events = append(events, session.Event{ + Kind: session.EventToolResult, + Tool: &session.ToolResult{ + Success: false, + RawName: "Bash", + ToolUseID: fmt.Sprintf("t%d", i), + Text: strings.Repeat("e", n), + }, + }) + } + + result := ComputeAudit(events) + + want := FailureLengthStats{Count: 5, Median: 30, P90: 100, Max: 100} + if result.Failures != want { + t.Fatalf("Failures = %#v, want %#v", result.Failures, want) + } +} + +// TestReadAll_GivenSuccessfulReadResultWithoutCommandName_ThenAuditBucketsByResolvedToolName +// is a regression guard mirroring claudecodec's own +// TestReadAll_GivenReadToolResultWithoutCommandName_ThenBareOkSuppressionApplies: +// real Claude Code transcripts carry no commandName on a Read's toolUseResult +// at all — RawName only resolves via the preceding tool_use's tool_use_id. A +// fixture with RawName hand-filled (as every other test in this file does) +// cannot catch a regression in that resolution path, so this one runs the +// real two-entry JSONL sequence through claudecodec.ReadAll. +func TestReadAll_GivenSuccessfulReadResultWithoutCommandName_ThenAuditBucketsByResolvedToolName(t *testing.T) { + path := filepath.Join(t.TempDir(), "session.jsonl") + // content is embedded directly into hand-built JSON string literals below, + // so newlines must be the literal two-character JSON escape `\n` (raw + // string, not an interpreted "\n") rather than an actual newline byte — + // an unescaped newline inside a JSON string is invalid JSON. + content := strings.Repeat(`line of file content\n`, 20) + lines := []string{ + `{"type":"assistant","timestamp":"2026-07-15T00:00:00Z","message":{"role":"assistant","content":[` + + `{"type":"tool_use","id":"toolu_read1","name":"Read","input":{"file_path":"/repo/README.md"}}` + + `]}}`, + `{"type":"user","timestamp":"2026-07-15T00:00:01Z","toolUseResult":{` + + `"type":"text","file":{"content":"` + content + `","numLines":20}` + + `},"message":{"role":"user","content":[{"type":"tool_result","tool_use_id":"toolu_read1",` + + `"content":" 1\t` + content + `"}]}}`, + "", + } + writeFixture(t, path, lines) + + events, err := claudecodec.ReadAll(path) + if err != nil { + t.Fatalf("ReadAll returned error: %v", err) + } + + result := ComputeAudit(events) + + if got := len(result.Samples[BucketSuccessReadFile]); got != 1 { + t.Fatalf("Samples[success_output_read_file] count = %d, want 1 (RawName must resolve via the preceding tool_use)", got) + } + for _, bucket := range []string{BucketSuccessBash, BucketSuccessAgent, BucketSuccessOther, BucketFailureOutput} { + if got := len(result.Samples[bucket]); got != 0 { + t.Fatalf("Samples[%s] = %d items, want 0 — a resolved Read result must not fall through to \"other\"", bucket, got) + } + } +} + +func writeFixture(t *testing.T, path string, lines []string) { + t.Helper() + if err := os.WriteFile(path, []byte(strings.Join(lines, "\n")), 0o644); err != nil { + t.Fatalf("write fixture: %v", err) } } From b8ec5d32dec18edb1ac350d4ae22700fcabc8760 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maple=EF=BC=81?= Date: Thu, 16 Jul 2026 10:45:48 +0800 Subject: [PATCH 09/16] =?UTF-8?q?docs:=20ADR-004=20failure=20retention=20s?= =?UTF-8?q?trategy=20=E2=80=94=20investigated,=20deferred?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Records the fleet length distribution (379 sessions), the adversarially reviewed two-tier design (threshold ~1,500, global-ranking head+tail+signal truncation, self-describing omission markers, re-derivability budgets), and the measured rejection of project-path rescue (<1% net coverage, half noise). Implementation deferred until the expand-telemetry feedback loop exists; revisit triggers listed. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01SaBPDHw1cZDxhehYrqFgGe --- docs/adr-004-failure-retention-strategy.md | 66 ++++++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 docs/adr-004-failure-retention-strategy.md diff --git a/docs/adr-004-failure-retention-strategy.md b/docs/adr-004-failure-retention-strategy.md new file mode 100644 index 0000000..0b1df7b --- /dev/null +++ b/docs/adr-004-failure-retention-strategy.md @@ -0,0 +1,66 @@ +# ADR-004: Failed Tool Result Retention Strategy + +## Status + +Proposed — implementation deliberately deferred. Current behavior (ADR-003: single meaningful error line, ~200 runes) stays in place. This document records the investigation, fleet data, and adversarial review so a future revision starts from evidence instead of re-deriving it. + +## Context + +ADR-003 fixed failure *detection* (status ladder) and widened the failure excerpt to one meaningful line / 200 runes. The open question was retention: errors are the content least safe to drop, yet multi-line errors (compiler output, tracebacks) lose everything past the first meaningful line. Truncation has a hidden cost — if the excerpt is insufficient to judge the root cause, the reading LLM calls `expand`, paying excerpt + tool round-trip + full text, which is worse than full retention. Telemetry forensics found a real instance: one session burst-called `expand` 19 times in 6.5 minutes after inheriting its own history (self-recovery after context compaction) — when a summary is insufficient, the failure mode is exploratory fumbling, not one clean retrieval. + +### Fleet data (379 sessions, 30 days, 1,194 failures per the ADR-003 ladder) + +Length distribution of failed-result text (chars): + +| p50 | p75 | p90 | p95 | p99 | max | +|-----|-----|-----|-----|-----|-----| +| 143 | 407 | 1,274 | 2,093 | 11,895 | 26,029 | + +- Extremely right-skewed: half of failures fit in 143 chars; the longest 10% (>1,274) hold **68%** of total failure chars (522K/763K). The long tail is compiler dumps and full test-suite output. +- Failures are rare per session: median 1, p90 7; 30% of sessions have none. +- Candidate thresholds for "short errors fully retained": p75=407 → 75% coverage; p90=1,274 → 90%; p95=2,093 → 95%. **~1,500 chars** (91-92% coverage) was the working recommendation. + +Known bias: the ladder is deliberately conservative (unknown shapes → ok), so this distribution describes *detected* failures only; the true distribution can only be wider. Also measured in chars/runes, not tokens. + +### Design conclusions that survived adversarial review + +A six-claim design was adversarially reviewed by a second model (Codex, xhigh); four PARTIAL, two REFUTED. What survived, with corrections applied: + +1. **Two-tier retention**: failures ≤ ~1,500 chars fully retained; longer ones truncated with head + tail + signal lines. Any reasonable threshold strictly beats the status quo (200 runes flat), so "not yet optimal" is not a reason to keep the worst variant — but see Decision for why we still defer. +2. **Global ranking, not stacked layers** (review correction): candidate lines (head, tail, keyword-matched signal lines) compete in one weighted ranking with per-category caps and a budget — not sequential layers where earlier ones exhaust the budget. +3. **Signal lines**: lines matching `Error:|panic:|FAILED|Caused by:|Traceback|assert` — root-cause position is ecosystem-dependent (Python/Java/test-runners: tail; Go/Node/compilers: head), so head+tail+signals covers both. *Unproven*: the head/middle/tail optimal ratio was never measured against annotated samples; ship conservative, iterate via audit. +4. **Self-describing omission markers**: `(... N lines omitted — ; full text: cc-session expand )`. The expand command must be injected by the formatter (Summary() has no session ID) and must be executable as printed (short-ID collision → print a longer ID). The marker must be honest about what was kept vs omitted per category. +5. **Retention budget should follow re-derivability, not success/failure** (review correction to the original claim): Read output is re-derivable (the file is on disk; re-reading yields the *current* version, usually better); Bash stdout — success or failure — is historical evidence (test logs, API responses, transient state) recoverable only via `expand`. Success/failure is a coarse proxy; the real axis is whether the reader can reconstruct the content. +6. **Feedback loop**: `inherit → expand` adjacency in usage.jsonl is a weak proxy for truncation failure. To make it usable, `expand` must log the tool-id argument (currently only the session id is recorded), and logging coverage must be measured first (entries are silently skipped when no caller is detected). + +### Project-path rescue: investigated and rejected + +Proposal: preserve error lines referencing project-owned files (Sentry-style "in-app frames"), on the actionability argument that the reader — a new Claude session in the same project — can act on `src/foo.go:42` (Read it) but cannot re-run the historical failure. + +Fleet measurement (378 transcripts, 1,169 failures) refuted the premise that such lines hide in the middle where head+tail misses them: + +- Path-line positions in long failures are **uniform** (head/middle/tail = 31.5%/35.5%/33.0%) — head+tail alone already captures ~2/3. +- Long failures where path lines exist *only* outside a head+tail window: 5–11 of 94 (0.4–0.9% of all failures). +- Of those rescued middle lines, over half are noise: node_modules stack frames carrying the project prefix, pnpm script banners. Only 1 of 5 sampled was a true signal (`thread 'main' panicked at src/db.rs:401:57:` mid-way through cargo test output). +- Path-line counts have a long tail (p90=27 per long failure) — any rescue layer would need per-result caps. + +Verdict: a dedicated path-matching layer costs a subsystem (project-root resolution, interaction with `cleanCwdPaths` which rewrites cwd to `.`, path-boundary/symlink handling, `ToolResult` has no cwd field) and buys <1% coverage, half noise. A narrowed `path:line` regex in the signal set was considered and rejected as a workaround. If revisited, the value concentrates in "relative path + line number" patterns (panic/traceback style) with node_modules excluded. + +## Decision + +**Defer implementation. Keep ADR-003 behavior unchanged.** + +Rationale: the two-tier design is well-evidenced but its supporting instrumentation is not in place — the feedback loop (expand tool-id logging, coverage measurement) is what turns a one-shot threshold guess into a tunable system, and shipping retention changes without it means flying blind on exactly the metric (truncation-forced expands) the design is meant to minimize. The investigation cost is sunk into this document; the implementation can start from here at any time. + +### Revisit triggers + +- Recurring burst-expand episodes (the 19-in-6.5-min pattern) observed in telemetry after inherit. +- `expand` starts logging tool ids (prerequisite for the feedback loop). +- A concrete user report of a failure whose root cause was invisible in the one-line excerpt. + +### Known limitations to carry into any implementation + +- False-negative bias in the failure statistics (conservative ladder). +- Runes ≠ tokens; threshold should eventually be validated against token counts. +- Content types (compiler diagnostics vs stack traces vs assertions) may deserve distinct strategies; the generic signal regex can keep a wrong line and drop the adjacent key line. +- Fully-retained short errors can expose secrets/env vars/API responses; consider redaction. From 153dd6a555694a1011b0f22e262c265d11030f2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maple=EF=BC=81?= Date: Thu, 16 Jul 2026 10:59:46 +0800 Subject: [PATCH 10/16] test: pin JSONL malformed-input and UTF-8 truncation boundary contracts Truncated/garbage/null-byte lines error explicitly without panic or silent loss; a 1MB single line guards against a future bufio.Scanner 64KB-token regression; Truncate stays valid UTF-8 at emoji-ZWJ and combining-char boundaries (grapheme-cluster integrity documented as a known limitation). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01SaBPDHw1cZDxhehYrqFgGe --- internal/claudecodec/reader_test.go | 185 ++++++++++++++++++++++++++++ internal/session/event_test.go | 93 ++++++++++++++ 2 files changed, 278 insertions(+) diff --git a/internal/claudecodec/reader_test.go b/internal/claudecodec/reader_test.go index a4360f2..025ad27 100644 --- a/internal/claudecodec/reader_test.go +++ b/internal/claudecodec/reader_test.go @@ -402,6 +402,191 @@ func TestParseLine_UnknownEntryWithoutMessageIsSkipped(t *testing.T) { } } +// --- Malformed JSONL robustness --- +// +// These tests pin the reader's actual contract for malformed input, per the +// project's own decision (see reader.go's parseLineWithToolNames): a line +// that fails json.Unmarshal returns an explicit "parse transcript line" +// error and aborts the read, rather than panicking or silently dropping the +// line to produce a truncated-but-successful result. Read/ReadAll callers +// that check the returned error (as every call site does) can never mistake +// a partially-read transcript for a complete one. + +// TestParseLine_GivenTruncatedJSON_ThenReturnsErrorNotPanic guards the exact +// truncation shape a killed/crashed Claude Code process can leave mid-write: +// a line cut off inside a string value. ParseLine must surface this as an +// error rather than panicking, since a panic here would crash every command +// that reads a transcript ending on such a line. +func TestParseLine_GivenTruncatedJSON_ThenReturnsErrorNotPanic(t *testing.T) { + _, ok, err := ParseLine([]byte(`{"type":"user","message":{"role":"user","con`)) + if err == nil { + t.Fatal("ParseLine returned nil error for truncated JSON, want an explicit error") + } + if ok { + t.Fatal("ParseLine returned ok=true for truncated JSON, want ok=false") + } +} + +// TestParseLine_GivenMalformedInputVariants_ThenReturnsErrorWithoutPanic covers +// the other shapes a corrupt or foreign line can take: a null byte (not +// whitespace, so it reaches the JSON parser) and plain non-JSON text. Both +// must fail the same way truncated JSON does. +func TestParseLine_GivenMalformedInputVariants_ThenReturnsErrorWithoutPanic(t *testing.T) { + tests := []struct { + name string + line string + }{ + {name: "null byte line", line: "\x00"}, + {name: "plain non-JSON text", line: "this is not json at all"}, + {name: "truncated at top level", line: `{"type":"user"`}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + event, ok, err := ParseLine([]byte(tt.line)) + if err == nil { + t.Fatalf("ParseLine(%q) returned nil error, want an explicit error", tt.line) + } + if ok { + t.Fatalf("ParseLine(%q) returned ok=true, want ok=false", tt.line) + } + if event != (session.Event{}) { + t.Fatalf("ParseLine(%q) returned non-zero event %#v alongside an error", tt.line, event) + } + }) + } +} + +// TestReadFile_GivenBlankOrWhitespaceOnlyLinesInterleaved_ThenSkippedWithoutLoss +// guards the whitespace-trim skip in ReadFile: blank lines and lines +// containing only spaces/tabs must be skipped silently, and must not shift +// or drop the valid events surrounding them. +func TestReadFile_GivenBlankOrWhitespaceOnlyLinesInterleaved_ThenSkippedWithoutLoss(t *testing.T) { + path := filepath.Join(t.TempDir(), "session.jsonl") + lines := []string{ + `{"type":"user","message":{"role":"user","content":"first"}}`, + "", + " \t ", + `{"type":"user","message":{"role":"user","content":"second"}}`, + "", + } + if err := os.WriteFile(path, []byte(strings.Join(lines, "\n")), 0o644); err != nil { + t.Fatalf("write fixture: %v", err) + } + + var events []session.Event + if err := ReadFile(path, func(event session.Event) error { + events = append(events, event) + return nil + }); err != nil { + t.Fatalf("ReadFile returned error: %v", err) + } + + if len(events) != 2 || events[0].User.Text != "first" || events[1].User.Text != "second" { + t.Fatalf("events = %#v, want exactly [first, second]", events) + } +} + +// TestReadFile_GivenInvalidLineAmongValidLines_ThenErrorSurfacesWithoutSilentlyDroppingPriorEvents +// guards the "silent data loss" failure mode the task brief calls out +// explicitly: a null-byte or non-JSON line anywhere in the file must abort +// the read with an explicit error (never silently skipped as if it were +// blank), while the valid line already handled before it is not retroactively +// lost — the caller sees both the earlier event and the error, never a +// silently truncated success. +func TestReadFile_GivenInvalidLineAmongValidLines_ThenErrorSurfacesWithoutSilentlyDroppingPriorEvents(t *testing.T) { + tests := []struct { + name string + invalidLine string + }{ + {name: "null byte line", invalidLine: "\x00"}, + {name: "non-JSON garbage line", invalidLine: "not json at all"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "session.jsonl") + lines := []string{ + `{"type":"user","message":{"role":"user","content":"before the bad line"}}`, + tt.invalidLine, + `{"type":"user","message":{"role":"user","content":"after the bad line"}}`, + "", + } + if err := os.WriteFile(path, []byte(strings.Join(lines, "\n")), 0o644); err != nil { + t.Fatalf("write fixture: %v", err) + } + + var events []session.Event + err := ReadFile(path, func(event session.Event) error { + events = append(events, event) + return nil + }) + if err == nil { + t.Fatal("ReadFile returned nil error, want an explicit parse error") + } + if !strings.Contains(err.Error(), "parse transcript line") { + t.Fatalf("error = %v, want parse transcript line", err) + } + if len(events) != 1 || events[0].User.Text != "before the bad line" { + t.Fatalf("events before the error = %#v, want exactly the one line handled before the bad line", events) + } + }) + } +} + +// TestReadFile_GivenSingleLineLargerThanDefaultScannerTokenLimit_ThenReadsWithoutError +// guards the classic bufio.Scanner landmine: Scanner's default MaxScanTokenSize +// is 64KB, and Claude Code transcripts routinely carry multi-megabyte single +// lines (large file reads/writes embedded in a tool_result). ReadFile uses +// bufio.Reader.ReadBytes, which has no such cap, but a future refactor to +// bufio.Scanner without an explicit Buffer() call would silently truncate the +// read (Scanner.Scan returns false with bufio.ErrTooLong) on exactly this +// shape. A multi-MB content field pins that ReadFile keeps working past the +// 64KB boundary. +func TestReadFile_GivenSingleLineLargerThanDefaultScannerTokenLimit_ThenReadsWithoutError(t *testing.T) { + const oversizedContentBytes = 1 * 1024 * 1024 // well past bufio.MaxScanTokenSize (64KB) + hugeContent := strings.Repeat("x", oversizedContentBytes) + contentJSON, err := json.Marshal(hugeContent) + if err != nil { + t.Fatalf("marshal huge content: %v", err) + } + path := filepath.Join(t.TempDir(), "session.jsonl") + line := `{"type":"user","message":{"role":"user","content":` + string(contentJSON) + `}}` + if err := os.WriteFile(path, []byte(line+"\n"), 0o644); err != nil { + t.Fatalf("write fixture: %v", err) + } + + events, err := ReadAll(path) + if err != nil { + t.Fatalf("ReadAll returned error on oversized line: %v", err) + } + if len(events) != 1 || events[0].User == nil || len(events[0].User.Text) != oversizedContentBytes { + t.Fatalf("got %d events, want 1 event with %d-byte text (got %d)", + len(events), oversizedContentBytes, len(events[0].User.Text)) + } +} + +// TestReadFile_GivenFileContainingOnlyNewlines_ThenNoEventsNoError guards the +// boundary next to TestReadAll_GivenEmptyFile_ThenReturnsNoEvents: a file +// that is not byte-empty but contains nothing except newline characters must +// still read as zero events with no error, since every line is blank after +// trimming. +func TestReadFile_GivenFileContainingOnlyNewlines_ThenNoEventsNoError(t *testing.T) { + path := filepath.Join(t.TempDir(), "blank-lines.jsonl") + if err := os.WriteFile(path, []byte("\n\n\n"), 0o644); err != nil { + t.Fatalf("write fixture: %v", err) + } + + var events []session.Event + if err := ReadFile(path, func(event session.Event) error { + events = append(events, event) + return nil + }); err != nil { + t.Fatalf("ReadFile returned error: %v", err) + } + if len(events) != 0 { + t.Fatalf("got %d events, want 0: %#v", len(events), events) + } +} + func TestReadFile_WhenLineIsMalformed_ThenReturnsError(t *testing.T) { path := filepath.Join(t.TempDir(), "bad.jsonl") data := strings.Join([]string{ diff --git a/internal/session/event_test.go b/internal/session/event_test.go index 2592579..5d37558 100644 --- a/internal/session/event_test.go +++ b/internal/session/event_test.go @@ -103,6 +103,83 @@ func TestTruncate(t *testing.T) { } } +// --- UTF-8 truncation boundary safety --- +// +// Truncate slices a []rune, so every result is by construction a sequence of +// complete Unicode code points — it can never split a multi-byte code point +// in half. These tests pin that guarantee at the trickiest boundaries: a +// multi-rune emoji ZWJ sequence and a base+combining-mark pair, both of which +// are made of more than one code point per visual character. +// +// Known limitation (not fixed here, by design): Truncate guarantees valid +// UTF-8 output, not grapheme-cluster integrity. Cutting mid-sequence can +// still separate a ZWJ joiner from its neighbour or a combining mark from its +// base character, which may render as an unintended glyph (e.g. a lone +// zero-width-joiner, or an accent detached from its letter). Fixing that +// would require a grapheme-aware segmentation library; the project has no +// such dependency, and the task brief for this test explicitly calls out not +// to introduce one just to guard this boundary. + +// TestTruncate_GivenEmojiZWJSequenceAtBoundary_ThenCutsOnRuneBoundaryAndStaysValidUTF8 +// guards against a regression that reintroduces byte-based slicing (e.g. +// `s[:n]`) instead of rune-based slicing, which would produce invalid UTF-8 +// for any input needing genuine truncation. +func TestTruncate_GivenEmojiZWJSequenceAtBoundary_ThenCutsOnRuneBoundaryAndStaysValidUTF8(t *testing.T) { + // "👨‍👩‍👧" (family emoji) is 5 runes: 👨 ZWJ 👩 ZWJ 👧. + family := "👨‍👩‍👧" + familyRunes := []rune(family) + if len(familyRunes) != 5 { + t.Fatalf("test setup: family emoji has %d runes, want 5", len(familyRunes)) + } + + got := Truncate(family, 2) + if !utf8.ValidString(got) { + t.Fatalf("Truncate(%q, 2) = %q is not valid UTF-8", family, got) + } + want := string(familyRunes[:2]) // 👨 + ZWJ: a broken cluster, but valid UTF-8 runes + if got != want { + t.Fatalf("Truncate(%q, 2) = %q, want %q", family, got, want) + } +} + +// TestTruncate_GivenCombiningCharacterAtBoundary_ThenCutsOnRuneBoundaryAndStaysValidUTF8 +// guards the same rune-boundary contract for a base character followed by a +// combining mark ("e" + COMBINING ACUTE ACCENT U+0301, not the precomposed +// "é"): cutting between them separates the accent from its letter but must +// still produce valid UTF-8, never a truncated multi-byte sequence. +func TestTruncate_GivenCombiningCharacterAtBoundary_ThenCutsOnRuneBoundaryAndStaysValidUTF8(t *testing.T) { + eWithCombiningAccent := "é" + if len([]rune(eWithCombiningAccent)) != 2 { + t.Fatalf("test setup: %q has %d runes, want 2", eWithCombiningAccent, len([]rune(eWithCombiningAccent))) + } + + got := Truncate(eWithCombiningAccent, 1) + if !utf8.ValidString(got) { + t.Fatalf("Truncate(%q, 1) = %q is not valid UTF-8", eWithCombiningAccent, got) + } + if got != "e" { + t.Fatalf("Truncate(%q, 1) = %q, want %q (bare base character, accent dropped)", eWithCombiningAccent, got, "e") + } +} + +// TestTruncate_GivenEmptyString_ThenReturnsEmpty guards the degenerate input: +// no runes to slice, so both the byte fast-path and the rune path must +// return "" without allocating a rune slice or indexing out of range. +func TestTruncate_GivenEmptyString_ThenReturnsEmpty(t *testing.T) { + if got := Truncate("", 10); got != "" { + t.Fatalf("Truncate(\"\", 10) = %q, want empty", got) + } +} + +// TestTruncate_GivenZeroRuneLimit_ThenReturnsEmptyString guards the maxRunes=0 +// boundary: runes[:0] must not panic and must yield "", distinct from the +// byte-length fast path (len(s) <= 0 is only true for an empty string). +func TestTruncate_GivenZeroRuneLimit_ThenReturnsEmptyString(t *testing.T) { + if got := Truncate("hello", 0); got != "" { + t.Fatalf("Truncate(\"hello\", 0) = %q, want empty", got) + } +} + func TestFirstLine(t *testing.T) { tests := []struct { name string @@ -143,6 +220,22 @@ func TestFirstLine(t *testing.T) { maxRunes: 80, want: "", }, + { + // CJK first line cut mid-string: must land on a rune boundary via + // the shared Truncate helper, never a half-character. + name: "given CJK first line over budget then cuts on rune boundary", + s: "甲乙丙丁\nsecond", + maxRunes: 2, + want: "甲乙", + }, + { + // Zero rune limit: the first line exists but the budget allows no + // runes at all, so the result must be "" rather than panicking. + name: "given zero rune limit then returns empty", + s: "hello\nworld", + maxRunes: 0, + want: "", + }, } for _, tt := range tests { From 7d30256f4357c5943f3d546e12a6d9902d6602bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maple=EF=BC=81?= Date: Thu, 16 Jul 2026 11:03:31 +0800 Subject: [PATCH 11/16] feat: record requested tool ids in expand usage telemetry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prerequisite for ADR-004's feedback loop: expand entries now carry tool_ids (all requested short IDs, including unresolved ones — the goal is what the caller wanted to inspect). Legacy entries without the field read back empty; display appends tools: only for expand rows. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01SaBPDHw1cZDxhehYrqFgGe --- cmd/cc-session/expand.go | 2 +- cmd/cc-session/main_test.go | 80 ++++++++++++++++++++++++++++++++ cmd/cc-session/usage_cmd.go | 43 ++++++++++------- internal/tracker/tracker.go | 5 ++ internal/tracker/tracker_test.go | 57 +++++++++++++++++++++++ 5 files changed, 170 insertions(+), 17 deletions(-) diff --git a/cmd/cc-session/expand.go b/cmd/cc-session/expand.go index abd0404..1ec27d1 100644 --- a/cmd/cc-session/expand.go +++ b/cmd/cc-session/expand.go @@ -29,7 +29,7 @@ func runExpand(args []string, out io.Writer, errOut io.Writer, store parser.Stor if resolved.Path == "" { return fmt.Errorf("transcript not found: %s", resolved.ID) } - logUsageAsync("expand", session.ShortID(resolved.ID, 8)) + logUsageAsync("expand", session.ShortID(resolved.ID, 8), requestedIDs...) events, err := reader.ReadAll(resolved.Path) if err != nil { diff --git a/cmd/cc-session/main_test.go b/cmd/cc-session/main_test.go index 75048e1..4674a03 100644 --- a/cmd/cc-session/main_test.go +++ b/cmd/cc-session/main_test.go @@ -19,6 +19,7 @@ import ( "github.com/Mapleeeeeeeeeee/cc-session-reader/internal/config" "github.com/Mapleeeeeeeeeee/cc-session-reader/internal/parser" "github.com/Mapleeeeeeeeeee/cc-session-reader/internal/session" + "github.com/Mapleeeeeeeeeee/cc-session-reader/internal/tracker" ) // testReader is the concrete reader used by all tests. @@ -617,6 +618,61 @@ func TestRunExpand_GivenExistingToolID_WhenExpanded_ThenShowsFullInputAndResult( } } +// expand's usage entry must record every requested tool ID (not just the +// ones that resolved), so usage analysis can tell what a caller wanted to +// inspect even when a lookup missed. +func TestRunExpand_GivenToolIDs_WhenExpanded_ThenUsageLogRecordsRequestedToolIDs(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Claude Code sessions only exist on macOS/Linux") + } + root := t.TempDir() + t.Setenv("HOME", root) + t.Setenv("USERPROFILE", root) + t.Setenv("CC_SESSION_NO_USAGE", "") + t.Setenv("ANTHROPIC_API_KEY", "") + config.Reset() + t.Cleanup(config.Reset) + usageTestCallerSession(t, root) + + sid := "12345678-1234-1234-1234-123456789abc" + projectDir := filepath.Join(root, "fixture-projects", "proj") + metaDir := filepath.Join(root, "fixture-usage-data", "session-meta") + _ = os.MkdirAll(projectDir, 0o755) + _ = os.MkdirAll(metaDir, 0o755) + transcript := strings.Join([]string{ + `{"type":"assistant","timestamp":"2026-05-28T00:00:01Z","message":{"role":"assistant","content":[{"type":"text","text":"hi"},{"type":"tool_use","name":"Bash","id":"toolu_01XYZabcdefgABCDuCVa","input":{"command":"echo hello","description":"Say hello"}}]}}`, + `{"type":"user","timestamp":"2026-05-28T00:00:02Z","toolUseResult":{"success":true,"commandName":"Bash"},"message":{"role":"user","content":[{"type":"tool_result","tool_use_id":"toolu_01XYZabcdefgABCDuCVa","content":"hello"}]}}`, + "", + }, "\n") + _ = os.WriteFile(filepath.Join(projectDir, sid+".jsonl"), []byte(transcript), 0o644) + writeListMeta(t, metaDir, sid, "/tmp/proj", "hello") + + store := parser.Store{ + ProjectsDir: filepath.Join(root, "fixture-projects"), + SessionMetaDir: metaDir, + } + + var stdout, stderr bytes.Buffer + beginUsageTracking("expand") + // "uCVa" resolves; "ZZZZ" doesn't, but must still be recorded as requested. + err := runExpand([]string{sid, "uCVa", "ZZZZ"}, &stdout, &stderr, store, testReader) + if err != nil { + t.Fatalf("runExpand returned error: %v", err) + } + finalizeUsageLog(err) + waitUsageLog() + + usagePath := filepath.Join(root, ".claude", "skills", "cc-session", "usage.jsonl") + data, readErr := os.ReadFile(usagePath) + if readErr != nil { + t.Fatalf("usage.jsonl not created: %v", readErr) + } + line := strings.TrimSpace(string(data)) + if !strings.Contains(line, `"tool_ids":["uCVa","ZZZZ"]`) { + t.Errorf("usage.jsonl missing requested tool_ids, got: %s", line) + } +} + // Regression: short IDs are only the last 4 chars of tool_use_id, so two // distinct tools can share one short ID. The old code keyed a map by short ID // and silently overwrote, so expand would return whichever tool appeared last @@ -1273,6 +1329,30 @@ func TestRunUsage_GivenHelpFlag_ThenReturnsErrHelp(t *testing.T) { } } +func TestRunUsage_GivenExpandEntryWithToolIDs_WhenDisplayed_ThenShowsToolIDs(t *testing.T) { + root := t.TempDir() + t.Setenv("HOME", root) + t.Setenv("USERPROFILE", root) + + if err := tracker.LogUsage(tracker.UsageEntry{ + Timestamp: "2026-06-15T10:00:00Z", + Command: "expand", + Target: "abc12345", + ToolIDs: []string{"Q1hv", "ooQF"}, + }); err != nil { + t.Fatalf("tracker.LogUsage: %v", err) + } + + var stdout, stderr bytes.Buffer + if err := runUsage(nil, &stdout, &stderr); err != nil { + t.Fatalf("runUsage returned error: %v", err) + } + got := stdout.String() + if !strings.Contains(got, "tools:Q1hv,ooQF") { + t.Fatalf("usage output missing tool IDs, got: %s", got) + } +} + func TestRunHelp_GivenHelpFlag_ThenReturnsErrHelp(t *testing.T) { var stdout, stderr bytes.Buffer err := runHelp([]string{"-h"}, &stdout, &stderr) diff --git a/cmd/cc-session/usage_cmd.go b/cmd/cc-session/usage_cmd.go index 69a06f2..1b56db4 100644 --- a/cmd/cc-session/usage_cmd.go +++ b/cmd/cc-session/usage_cmd.go @@ -17,16 +17,18 @@ import ( var usageWG sync.WaitGroup -// pendingUsageCmd and pendingUsageTarget describe the in-progress command -// invocation to be recorded once its outcome is known. There is at most one -// command per process, so package-level state is safe: main's dispatch sets -// pendingUsageCmd before running a tracked command (see beginUsageTracking), -// and the command itself may later refine pendingUsageTarget once it +// pendingUsageCmd, pendingUsageTarget, and pendingUsageToolIDs describe the +// in-progress command invocation to be recorded once its outcome is known. +// There is at most one command per process, so package-level state is safe: +// main's dispatch sets pendingUsageCmd before running a tracked command (see +// beginUsageTracking), and the command itself may later refine +// pendingUsageTarget (and, for "expand", pendingUsageToolIDs) once it // resolves a concrete target (see logUsageAsync). finalizeUsageLog consumes -// and clears both when the command returns. +// and clears all three when the command returns. var ( - pendingUsageCmd string - pendingUsageTarget string + pendingUsageCmd string + pendingUsageTarget string + pendingUsageToolIDs []string ) // beginUsageTracking marks cmd as the subcommand whose result should be @@ -39,12 +41,15 @@ func beginUsageTracking(cmd string) { } // logUsageAsync records target as the resolved session for the current -// tracked invocation (see beginUsageTracking). It no longer writes to disk -// itself: the actual entry is written by finalizeUsageLog once the command's -// outcome is known, so a single JSONL line always carries the correct result. -func logUsageAsync(cmd string, target string) { +// tracked invocation (see beginUsageTracking), along with any requested tool +// IDs (currently only "expand" passes these, e.g. the "Q1hv" in an argument +// like "expand abc123 Q1hv"). It no longer writes to disk itself: the actual +// entry is written by finalizeUsageLog once the command's outcome is known, +// so a single JSONL line always carries the correct result. +func logUsageAsync(cmd string, target string, toolIDs ...string) { pendingUsageCmd = cmd pendingUsageTarget = target + pendingUsageToolIDs = toolIDs } // finalizeUsageLog writes the usage entry for the invocation that just @@ -58,8 +63,8 @@ func finalizeUsageLog(cmdErr error) { if pendingUsageCmd == "" { return } - cmd, target := pendingUsageCmd, pendingUsageTarget - pendingUsageCmd, pendingUsageTarget = "", "" + cmd, target, toolIDs := pendingUsageCmd, pendingUsageTarget, pendingUsageToolIDs + pendingUsageCmd, pendingUsageTarget, pendingUsageToolIDs = "", "", nil if config.Get().NoUsage { return @@ -96,6 +101,7 @@ func finalizeUsageLog(cmdErr error) { Commit: commit, Result: result, Error: errMsg, + ToolIDs: toolIDs, } _ = tracker.LogUsage(entry) }() @@ -164,8 +170,13 @@ func runUsage(args []string, out io.Writer, errOut io.Writer) error { resultMarker = " [ERR]" } - fmt.Fprintf(out, "%s %-8s %s %s %s%s\n", - dateStr, e.Command, target, callerShort, e.Cwd, resultMarker) + toolIDsSuffix := "" + if len(e.ToolIDs) > 0 { + toolIDsSuffix = " tools:" + strings.Join(e.ToolIDs, ",") + } + + fmt.Fprintf(out, "%s %-8s %s %s %s%s%s\n", + dateStr, e.Command, target, callerShort, e.Cwd, resultMarker, toolIDsSuffix) } return nil } diff --git a/internal/tracker/tracker.go b/internal/tracker/tracker.go index 10aef9b..0f12968 100644 --- a/internal/tracker/tracker.go +++ b/internal/tracker/tracker.go @@ -30,6 +30,11 @@ type UsageEntry struct { // Error holds the first line of the failing command's error message. // Empty unless Result == "error". Error string `json:"error,omitempty"` + // ToolIDs holds the short tool IDs requested by an "expand" invocation + // (e.g. the "Q1hv" in [Grep#Q1hv]), so usage analysis can tell what a + // caller wanted to inspect. Empty for every other command, and for + // entries written before this field existed. + ToolIDs []string `json:"tool_ids,omitempty"` } // commandAliases maps a deprecated subcommand name recorded by older diff --git a/internal/tracker/tracker_test.go b/internal/tracker/tracker_test.go index 4b1aab7..60cdf1f 100644 --- a/internal/tracker/tracker_test.go +++ b/internal/tracker/tracker_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "os" "path/filepath" + "reflect" "strings" "testing" "time" @@ -243,6 +244,62 @@ func TestLogUsageToPath_GivenResultAndError_WhenAppended_ThenRoundTrips(t *testi } } +func TestLogUsageToPath_GivenToolIDs_WhenAppended_ThenRoundTrips(t *testing.T) { + path := filepath.Join(t.TempDir(), "usage.jsonl") + entry := UsageEntry{Command: "expand", Target: "abc123", ToolIDs: []string{"Q1hv", "ooQF"}} + + writeEntry(t, path, entry) + + entries, err := ReadUsageLogFromPath(0, "", path) + if err != nil { + t.Fatalf("ReadUsageLogFromPath returned error: %v", err) + } + if len(entries) != 1 { + t.Fatalf("entry count = %d, want 1", len(entries)) + } + if !reflect.DeepEqual(entries[0].ToolIDs, entry.ToolIDs) { + t.Errorf("ToolIDs = %v, want %v", entries[0].ToolIDs, entry.ToolIDs) + } +} + +func TestLogUsageToPath_GivenNoToolIDs_WhenAppended_ThenOmitsToolIDsFromJSON(t *testing.T) { + path := filepath.Join(t.TempDir(), "usage.jsonl") + entry := UsageEntry{Command: "read", Target: "abc123"} + + writeEntry(t, path, entry) + + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read file: %v", err) + } + line := strings.TrimSpace(string(data)) + if strings.Contains(line, "tool_ids") { + t.Errorf("expected tool_ids to be omitted for a command with no tool IDs, got: %s", line) + } +} + +// Regression: entries written before the ToolIDs field existed have no +// "tool_ids" key at all. Without this, unmarshaling a legacy line must still +// succeed and yield a nil/empty slice rather than an error. +func TestReadUsageLogFromPath_GivenLegacyEntryWithoutToolIDsField_WhenRead_ThenToolIDsIsEmpty(t *testing.T) { + path := filepath.Join(t.TempDir(), "usage.jsonl") + raw := `{"ts":"2026-06-15T10:00:00Z","cmd":"expand","target":"legacy","cwd":"/x","caller":"c"}` + "\n" + if err := os.WriteFile(path, []byte(raw), 0o644); err != nil { + t.Fatalf("write legacy entry: %v", err) + } + + entries, err := ReadUsageLogFromPath(0, "", path) + if err != nil { + t.Fatalf("ReadUsageLogFromPath returned error: %v", err) + } + if len(entries) != 1 { + t.Fatalf("entry count = %d, want 1", len(entries)) + } + if len(entries[0].ToolIDs) != 0 { + t.Errorf("ToolIDs = %v, want empty for a pre-existing entry", entries[0].ToolIDs) + } +} + // Regression: entries written before the Result field existed have no // "result" key at all. Without this, an empty Result on unmarshal could be // mistaken for a known failure instead of "we don't know". From 3f671d615efb4ff1cbe07301582059eb7097b3a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maple=EF=BC=81?= Date: Thu, 16 Jul 2026 11:35:51 +0800 Subject: [PATCH 12/16] feat: collapse retry loops and consecutive same-file reads Consecutive failed calls of the same tool whose command shares a prefix collapse to one line with a xN count and the last error (last tool id kept so expand reaches the final state); consecutive successful Reads of the same file collapse to a single xN line. Collapsing never crosses an interleaved event, never hides a distinct failure, and -verbose-bash exempts retry collapsing per the ADR-001 precedent. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01SaBPDHw1cZDxhehYrqFgGe --- internal/formatter/collapse_test.go | 341 ++++++++++++++++++++++++++++ internal/formatter/render.go | 181 ++++++++++++++- internal/summarizer/summarizer.go | 10 +- 3 files changed, 524 insertions(+), 8 deletions(-) create mode 100644 internal/formatter/collapse_test.go diff --git a/internal/formatter/collapse_test.go b/internal/formatter/collapse_test.go new file mode 100644 index 0000000..9418356 --- /dev/null +++ b/internal/formatter/collapse_test.go @@ -0,0 +1,341 @@ +package formatter + +import ( + "bytes" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/Mapleeeeeeeeeee/cc-session-reader/internal/claudecodec" +) + +// bashAttempt describes one Bash tool_use/tool_result pair for the retry-loop +// collapse fixtures below. +type bashAttempt struct { + toolID string + command string + description string + success bool + resultText string +} + +// writeBashAttemptsFixture writes a real transcript (assistant tool_use +// followed by its user tool_result, per attempt) and runs it through the real +// parse pipeline via FormatRead(claudecodec.Codec{}) — never hand-filling +// RawName, per this project's own regression history (claudecodec's RawName +// only resolves through the preceding tool_use's tool_use_id, so a +// hand-built session.ToolResult can't catch a regression there). +func writeBashAttemptsFixture(t *testing.T, attempts []bashAttempt) string { + t.Helper() + + root := t.TempDir() + transcriptPath := filepath.Join(root, formatterFixtureSessionID+".jsonl") + + var b strings.Builder + second := 0 + for _, a := range attempts { + fmt.Fprintf(&b, `{"type":"assistant","timestamp":"2026-05-28T00:00:%02dZ","message":{"role":"assistant","content":[{"type":"tool_use","name":"Bash","id":%q,"input":{"command":%q,"description":%q}}]}}`+"\n", + second, a.toolID, a.command, a.description) + second++ + fmt.Fprintf(&b, `{"type":"user","timestamp":"2026-05-28T00:00:%02dZ","toolUseResult":{"success":%t,"commandName":"Bash"},"message":{"role":"user","content":[{"type":"tool_result","tool_use_id":%q,"content":%q}]}}`+"\n", + second, a.success, a.toolID, a.resultText) + second++ + } + if err := os.WriteFile(transcriptPath, []byte(b.String()), 0o644); err != nil { + t.Fatalf("write transcript: %v", err) + } + return transcriptPath +} + +// writeBashAttemptsWithNarrationFixture is writeBashAttemptsFixture but +// inserts a plain assistant-text message ("narration") between the two +// attempt groups, mirroring the model narrating between retries — this +// flushes the pending tool list, so it must break any retry-loop grouping. +func writeBashAttemptsWithNarrationFixture(t *testing.T, before, after []bashAttempt, narration string) string { + t.Helper() + + root := t.TempDir() + transcriptPath := filepath.Join(root, formatterFixtureSessionID+".jsonl") + + writeAttempt := func(b *strings.Builder, second *int, a bashAttempt) { + fmt.Fprintf(b, `{"type":"assistant","timestamp":"2026-05-28T00:00:%02dZ","message":{"role":"assistant","content":[{"type":"tool_use","name":"Bash","id":%q,"input":{"command":%q,"description":%q}}]}}`+"\n", + *second, a.toolID, a.command, a.description) + *second++ + fmt.Fprintf(b, `{"type":"user","timestamp":"2026-05-28T00:00:%02dZ","toolUseResult":{"success":%t,"commandName":"Bash"},"message":{"role":"user","content":[{"type":"tool_result","tool_use_id":%q,"content":%q}]}}`+"\n", + *second, a.success, a.toolID, a.resultText) + *second++ + } + + var b strings.Builder + second := 0 + for _, a := range before { + writeAttempt(&b, &second, a) + } + fmt.Fprintf(&b, `{"type":"assistant","timestamp":"2026-05-28T00:00:%02dZ","message":{"role":"assistant","content":[{"type":"text","text":%q}]}}`+"\n", + second, narration) + second++ + for _, a := range after { + writeAttempt(&b, &second, a) + } + + if err := os.WriteFile(transcriptPath, []byte(b.String()), 0o644); err != nil { + t.Fatalf("write transcript: %v", err) + } + return transcriptPath +} + +func TestFormatRead_GivenConsecutiveFailedRetriesWithSameCommand_ThenCollapsesIntoFailedCountLine(t *testing.T) { + transcriptPath := writeBashAttemptsFixture(t, []bashAttempt{ + {toolID: "retry-tool-1", command: "npm test", description: "Run tests", success: false, resultText: "Error: 2 tests failed"}, + {toolID: "retry-tool-2", command: "npm test", description: "Run tests", success: false, resultText: "Error: 3 tests failed"}, + {toolID: "retry-tool-3", command: "npm test", description: "Run tests", success: false, resultText: "Error: TypeError: cannot read prop"}, + }) + + var out bytes.Buffer + if err := FormatRead(transcriptPath, 0, 0, FormatOptions{}, &out, claudecodec.Codec{}); err != nil { + t.Fatalf("FormatRead returned error: %v", err) + } + got := out.String() + + want := "[Bash#ol-3] Run tests -> FAILED ×3: Error: TypeError: cannot read prop" + if !strings.Contains(got, want) { + t.Fatalf("expected collapsed retry-loop line\nwant substring: %q\ngot:\n%s", want, got) + } + if strings.Count(got, "[Bash#") != 1 { + t.Fatalf("expected exactly one Bash summary line after collapsing, got:\n%s", got) + } +} + +func TestFormatRead_GivenSingleFailedBashCall_ThenDoesNotShowMultiplier(t *testing.T) { + // A run of exactly one failed attempt must render as a normal FAILED line — + // "×1" would be a meaningless label for something that wasn't retried. + transcriptPath := writeBashAttemptsFixture(t, []bashAttempt{ + {toolID: "single-tool-1", command: "npm test", description: "Run tests", success: false, resultText: "Error: 1 test failed"}, + }) + + var out bytes.Buffer + if err := FormatRead(transcriptPath, 0, 0, FormatOptions{}, &out, claudecodec.Codec{}); err != nil { + t.Fatalf("FormatRead returned error: %v", err) + } + got := out.String() + + want := "[Bash#ol-1] Run tests -> FAILED: Error: 1 test failed" + if !strings.Contains(got, want) { + t.Fatalf("expected uncollapsed single failure line\nwant substring: %q\ngot:\n%s", want, got) + } + if strings.Contains(got, "×") { + t.Fatalf("a single failed call must not show a retry multiplier, got:\n%s", got) + } +} + +func TestFormatRead_GivenRetryLoopInterruptedByAssistantText_ThenDoesNotCollapseAcrossText(t *testing.T) { + // The model narrating between two otherwise-identical failed attempts + // flushes the pending tool list, splitting what would otherwise be a + // 2-attempt retry loop into two independent single attempts — each of + // which stays uncollapsed (their individual failure text might be what + // the narration is referring to). + before := []bashAttempt{ + {toolID: "gap-tool-1", command: "npm test", description: "Run tests", success: false, resultText: "Error: connection refused"}, + } + after := []bashAttempt{ + {toolID: "gap-tool-2", command: "npm test", description: "Run tests", success: false, resultText: "Error: connection refused"}, + } + transcriptPath := writeBashAttemptsWithNarrationFixture(t, before, after, "let me check the server logs") + + var out bytes.Buffer + if err := FormatRead(transcriptPath, 0, 0, FormatOptions{}, &out, claudecodec.Codec{}); err != nil { + t.Fatalf("FormatRead returned error: %v", err) + } + got := out.String() + + if strings.Contains(got, "×") { + t.Fatalf("attempts split by assistant narration must not collapse, got:\n%s", got) + } + if strings.Count(got, "[Bash#") != 2 { + t.Fatalf("expected both attempts to render as separate Bash lines, got:\n%s", got) + } + if !strings.Contains(got, "let me check the server logs") { + t.Fatalf("narration text between attempts must still render, got:\n%s", got) + } +} + +func TestFormatRead_GivenMixedSuccessAndFailureSameCommand_ThenDoesNotCollapse(t *testing.T) { + // A success sandwiched between two failures of the same command is not a + // clean retry loop — collapsing must not jump over the success to merge + // the two failed ends together. + transcriptPath := writeBashAttemptsFixture(t, []bashAttempt{ + {toolID: "mixed-tool-1", command: "npm test", description: "Run tests", success: false, resultText: "Error: flaky failure"}, + {toolID: "mixed-tool-2", command: "npm test", description: "Run tests", success: true, resultText: "All tests passed"}, + {toolID: "mixed-tool-3", command: "npm test", description: "Run tests", success: false, resultText: "Error: different failure"}, + }) + + var out bytes.Buffer + if err := FormatRead(transcriptPath, 0, 0, FormatOptions{}, &out, claudecodec.Codec{}); err != nil { + t.Fatalf("FormatRead returned error: %v", err) + } + got := out.String() + + if strings.Contains(got, "×") { + t.Fatalf("a success interrupting the run must prevent collapsing, got:\n%s", got) + } + if strings.Count(got, "[Bash#") != 3 { + t.Fatalf("expected all three attempts to render individually, got:\n%s", got) + } +} + +func TestFormatRead_GivenVerboseBashAndRetryLoop_ThenSkipsCollapse(t *testing.T) { + // Mirrors the existing -verbose-bash exemption for collapseCCSessionTools: + // full Bash output must be preserved per attempt, not discarded by + // retry-loop collapsing. + transcriptPath := writeBashAttemptsFixture(t, []bashAttempt{ + {toolID: "verbose-tool-1", command: "npm test", description: "Run tests", success: false, resultText: "Error: attempt one detail"}, + {toolID: "verbose-tool-2", command: "npm test", description: "Run tests", success: false, resultText: "Error: attempt two detail"}, + {toolID: "verbose-tool-3", command: "npm test", description: "Run tests", success: false, resultText: "Error: attempt three detail"}, + }) + + var out bytes.Buffer + if err := FormatRead(transcriptPath, 0, 0, FormatOptions{VerboseBash: true}, &out, claudecodec.Codec{}); err != nil { + t.Fatalf("FormatRead returned error: %v", err) + } + got := out.String() + + if strings.Contains(got, "×") { + t.Fatalf("verbose-bash must skip retry-loop collapsing, got:\n%s", got) + } + for _, want := range []string{"Error: attempt one detail", "Error: attempt two detail", "Error: attempt three detail"} { + if !strings.Contains(got, want) { + t.Fatalf("verbose-bash output missing full detail %q, got:\n%s", want, got) + } + } +} + +// readAttempt describes one Read tool_use/tool_result pair for the +// same-file-read collapse fixtures below. +type readAttempt struct { + toolID string + path string + offset int + limit int + hasWindow bool + success bool + resultText string +} + +func writeReadTranscript(t *testing.T, lines []string) string { + t.Helper() + root := t.TempDir() + transcriptPath := filepath.Join(root, formatterFixtureSessionID+".jsonl") + if err := os.WriteFile(transcriptPath, []byte(strings.Join(lines, "\n")+"\n"), 0o644); err != nil { + t.Fatalf("write transcript: %v", err) + } + return transcriptPath +} + +func readAttemptLines(second *int, a readAttempt) []string { + input := fmt.Sprintf("%q", a.path) + if a.hasWindow { + input = fmt.Sprintf(`%q,"offset":%d,"limit":%d`, a.path, a.offset, a.limit) + } + assistantLine := fmt.Sprintf(`{"type":"assistant","timestamp":"2026-05-28T00:00:%02dZ","message":{"role":"assistant","content":[{"type":"tool_use","name":"Read","id":%q,"input":{"file_path":%s}}]}}`, + *second, a.toolID, input) + *second++ + resultLine := fmt.Sprintf(`{"type":"user","timestamp":"2026-05-28T00:00:%02dZ","toolUseResult":{"success":%t,"commandName":"Read"},"message":{"role":"user","content":[{"type":"tool_result","tool_use_id":%q,"content":%q}]}}`, + *second, a.success, a.toolID, a.resultText) + *second++ + return []string{assistantLine, resultLine} +} + +func TestFormatRead_GivenConsecutiveReadsOfSameFile_ThenCollapsesIntoReadCountLine(t *testing.T) { + second := 0 + var lines []string + lines = append(lines, readAttemptLines(&second, readAttempt{toolID: "read-tool-1", path: "/repo/src/main.go", success: true, resultText: "line 1\nline 2"})...) + lines = append(lines, readAttemptLines(&second, readAttempt{toolID: "read-tool-2", path: "/repo/src/main.go", offset: 100, limit: 50, hasWindow: true, success: true, resultText: "line 101\nline 102"})...) + lines = append(lines, readAttemptLines(&second, readAttempt{toolID: "read-tool-3", path: "/repo/src/main.go", offset: 200, limit: 50, hasWindow: true, success: true, resultText: "line 201\nline 202"})...) + transcriptPath := writeReadTranscript(t, lines) + + var out bytes.Buffer + if err := FormatRead(transcriptPath, 0, 0, FormatOptions{}, &out, claudecodec.Codec{}); err != nil { + t.Fatalf("FormatRead returned error: %v", err) + } + got := out.String() + + want := "[Read#ol-3 ×3] src/main.go -> ok" + if !strings.Contains(got, want) { + t.Fatalf("expected collapsed same-file-read line\nwant substring: %q\ngot:\n%s", want, got) + } + if strings.Count(got, "[Read#") != 1 { + t.Fatalf("expected exactly one Read summary line after collapsing, got:\n%s", got) + } +} + +func TestFormatRead_GivenSingleReadCall_ThenDoesNotShowMultiplier(t *testing.T) { + second := 0 + lines := readAttemptLines(&second, readAttempt{toolID: "solo-read-1", path: "/repo/src/main.go", success: true, resultText: "line 1\nline 2"}) + transcriptPath := writeReadTranscript(t, lines) + + var out bytes.Buffer + if err := FormatRead(transcriptPath, 0, 0, FormatOptions{}, &out, claudecodec.Codec{}); err != nil { + t.Fatalf("FormatRead returned error: %v", err) + } + got := out.String() + + if strings.Contains(got, "×") { + t.Fatalf("a single Read call must not show a multiplier, got:\n%s", got) + } + if !strings.Contains(got, "[Read#ad-1]") { + t.Fatalf("expected uncollapsed single Read line, got:\n%s", got) + } +} + +func TestFormatRead_GivenReadsInterruptedByOtherTool_ThenDoesNotCollapse(t *testing.T) { + second := 0 + var lines []string + lines = append(lines, readAttemptLines(&second, readAttempt{toolID: "gap-read-1", path: "/repo/src/main.go", success: true, resultText: "line 1"})...) + lines = append(lines, + fmt.Sprintf(`{"type":"assistant","timestamp":"2026-05-28T00:00:%02dZ","message":{"role":"assistant","content":[{"type":"tool_use","name":"Grep","id":"grep-tool-1","input":{"pattern":"TODO"}}]}}`, second), + ) + second++ + lines = append(lines, + fmt.Sprintf(`{"type":"user","timestamp":"2026-05-28T00:00:%02dZ","toolUseResult":{"success":true,"commandName":"Grep"},"message":{"role":"user","content":[{"type":"tool_result","tool_use_id":"grep-tool-1","content":"no matches"}]}}`, second), + ) + second++ + lines = append(lines, readAttemptLines(&second, readAttempt{toolID: "gap-read-2", path: "/repo/src/main.go", success: true, resultText: "line 2"})...) + transcriptPath := writeReadTranscript(t, lines) + + var out bytes.Buffer + if err := FormatRead(transcriptPath, 0, 0, FormatOptions{}, &out, claudecodec.Codec{}); err != nil { + t.Fatalf("FormatRead returned error: %v", err) + } + got := out.String() + + if strings.Contains(got, "×") { + t.Fatalf("reads split by another tool call must not collapse, got:\n%s", got) + } + if strings.Count(got, "[Read#") != 2 { + t.Fatalf("expected both reads to render as separate lines, got:\n%s", got) + } +} + +func TestFormatRead_GivenReadsWithOneFailure_ThenDoesNotCollapse(t *testing.T) { + second := 0 + var lines []string + lines = append(lines, readAttemptLines(&second, readAttempt{toolID: "flaky-read-1", path: "/repo/src/main.go", success: true, resultText: "line 1"})...) + lines = append(lines, readAttemptLines(&second, readAttempt{toolID: "flaky-read-2", path: "/repo/src/main.go", success: false, resultText: "permission denied"})...) + lines = append(lines, readAttemptLines(&second, readAttempt{toolID: "flaky-read-3", path: "/repo/src/main.go", success: true, resultText: "line 3"})...) + transcriptPath := writeReadTranscript(t, lines) + + var out bytes.Buffer + if err := FormatRead(transcriptPath, 0, 0, FormatOptions{}, &out, claudecodec.Codec{}); err != nil { + t.Fatalf("FormatRead returned error: %v", err) + } + got := out.String() + + if strings.Contains(got, "×") { + t.Fatalf("a failed read in the run must prevent collapsing, got:\n%s", got) + } + if strings.Count(got, "[Read#") != 3 { + t.Fatalf("expected all three reads to render individually, got:\n%s", got) + } +} diff --git a/internal/formatter/render.go b/internal/formatter/render.go index 18516bd..cbfb328 100644 --- a/internal/formatter/render.go +++ b/internal/formatter/render.go @@ -114,6 +114,34 @@ type pendingTool struct { injectSessionID string // non-empty when this is a cc-session inherit/inject/read/context call injectTotalLines int // total lines from the last page marker ccSubcommand string // "inherit", "inject" (legacy), "read", or "context" + + // callLabel is the tagged call summary as summarizeToolUse first produced + // it (e.g. "[Bash#id] description"), kept alongside summary — which + // appendToolResult mutates by appending the result — so collapseRetryLoops + // can rebuild a single collapsed line from the last attempt's own label. + callLabel string + + // retrySignature is the normalized identity collapseRetryLoops keys a + // retry loop on: the first line of the Bash command, or of the raw input + // for any other tool, with whitespace collapsed. Empty when there is + // nothing meaningful to key on, which makes the call ineligible for + // retry-loop collapsing rather than risking a false match against an + // equally-empty, unrelated call. + retrySignature string + + // readFilePath is Read's raw file_path input (grouping key) and + // readDisplayPath its cleaned display form, both used by + // collapseSameFileReads to detect and render consecutive reads of the + // same file. Empty for non-Read tools. + readFilePath string + readDisplayPath string + + // resultReceived and resultSuccess record whether a tool_result has been + // merged into this pendingTool yet and its outcome. Both collapse passes + // need to tell "this call finished and failed/succeeded" apart from + // "this call is still awaiting its result". + resultReceived bool + resultSuccess bool } func loadEvents(transcriptPath string, isVerboseAgents bool, reader session.TranscriptReader) ([]session.Event, map[string]bool, error) { @@ -143,6 +171,8 @@ func appendToolResult(result *session.ToolResult, pendingTools *[]pendingTool, o pt := &(*pendingTools)[i] if pt.toolUseID == result.ToolUseID { applyInjectResult(pt, result) + pt.resultReceived = true + pt.resultSuccess = result.Success if opts.VerboseBash && pt.name == session.ToolBash { pt.summary += formatVerboseBashResult(result) return @@ -155,6 +185,8 @@ func appendToolResult(result *session.ToolResult, pendingTools *[]pendingTool, o if len(*pendingTools) > 0 { last := &(*pendingTools)[len(*pendingTools)-1] applyInjectResult(last, result) + last.resultReceived = true + last.resultSuccess = result.Success if opts.VerboseBash && last.name == session.ToolBash { last.summary += formatVerboseBashResult(result) return @@ -171,8 +203,10 @@ func appendToolResult(result *session.ToolResult, pendingTools *[]pendingTool, o summary = fmt.Sprintf("[%s]%s", name, formatVerboseBashResult(result)) } *pendingTools = append(*pendingTools, pendingTool{ - summary: summary, - name: name, + summary: summary, + name: name, + resultReceived: true, + resultSuccess: result.Success, }) } @@ -207,9 +241,11 @@ func summarizeToolUse(tool session.ToolUse) pendingTool { // "[Agent(general)] desc" becomes "[Agent(general)#ol-1] desc". tagged := injectShortID(summary, shortID) pt := pendingTool{ - toolUseID: tool.ID, - summary: tagged, - name: name, + toolUseID: tool.ID, + summary: tagged, + callLabel: tagged, + name: name, + retrySignature: retrySignature(name, tool.Input), } if name == session.ToolBash { cmd := tool.Input.String("command") @@ -218,9 +254,36 @@ func summarizeToolUse(tool session.ToolUse) pendingTool { pt.ccSubcommand = sub } } + if name == session.ToolRead { + path := tool.Input.String("file_path") + pt.readFilePath = path + if path != "" { + pt.readDisplayPath = summarizer.CleanPath(path, tool.Cwd) + } + } return pt } +// retrySignature returns the normalized identity collapseRetryLoops keys a +// retry loop on: the first line of the Bash command (multi-line scripts +// don't defeat the match), or the first line of the raw input JSON for any +// other tool. Returns "" when there is nothing to key on. +func retrySignature(name string, input session.ToolInput) string { + raw := input.String("command") + if name != session.ToolBash { + raw = input.MarshalNoEscape() + } + firstLine := strings.SplitN(strings.TrimSpace(raw), "\n", 2)[0] + return normalizeWhitespace(firstLine) +} + +// normalizeWhitespace collapses runs of whitespace to single spaces so +// cosmetic formatting differences (extra spaces, tabs) don't defeat the +// retry-loop prefix comparison. +func normalizeWhitespace(s string) string { + return strings.Join(strings.Fields(s), " ") +} + // injectShortID inserts "#id" before the first ']' in summary. // "[Bash] Run tests" -> "[Bash#uCVa] Run tests" // "[Agent(general)] Inspect" -> "[Agent(general)#uCVa] Inspect" @@ -334,11 +397,119 @@ func collapseCCSessionTools(tools []pendingTool) []pendingTool { return result } +// collapseRetryLoops collapses a consecutive run (>=2) of pendingTools that +// share the same tool name, the same (whitespace-normalized, prefix-matched) +// retrySignature, and all FAILED, into a single "FAILED xN" line — a retry +// loop that repeats the same failing call N times otherwise contributes N +// near-identical lines. A run of exactly 1 is left untouched (no "x1" label); +// a success or an unrelated call in between breaks the run so its individual +// failure information is never silently dropped. +func collapseRetryLoops(tools []pendingTool) []pendingTool { + result := make([]pendingTool, 0, len(tools)) + for i := 0; i < len(tools); i++ { + pt := tools[i] + if !isRetryEligible(pt) { + result = append(result, pt) + continue + } + j := i + 1 + for j < len(tools) && isRetryEligible(tools[j]) && + tools[j].name == pt.name && + sameRetryCommand(pt.retrySignature, tools[j].retrySignature) { + j++ + } + count := j - i + if count < 2 { + result = append(result, pt) + continue + } + last := tools[j-1] + last.summary = formatRetryCollapse(last, count) + result = append(result, last) + i = j - 1 + } + return result +} + +// isRetryEligible reports whether pt finished with a failure and carries a +// non-empty retry signature to key on — the precondition for taking part in +// a retry-loop group at all. +func isRetryEligible(pt pendingTool) bool { + return pt.resultReceived && !pt.resultSuccess && pt.retrySignature != "" +} + +// sameRetryCommand reports whether a and b identify the same retried call: +// exact match, or one is a non-empty prefix of the other (covers "only the +// trailing argument differs" retries, e.g. a script rerun with a new seed). +func sameRetryCommand(a, b string) bool { + if a == b { + return true + } + shorter, longer := a, b + if len(longer) < len(shorter) { + shorter, longer = longer, shorter + } + return shorter != "" && strings.HasPrefix(longer, shorter) +} + +// formatRetryCollapse rebuilds the collapsed retry-loop line from the last +// attempt's own call label (tool id + description) plus its failure +// excerpt, annotated with how many attempts failed. last.summary is always +// last.callLabel followed by exactly the tail appendToolResult appended +// (ToolResult.Summary(), which contains the literal "FAILED" once), so +// inserting the count there mirrors the string-surgery injectShortID already +// uses to tag summaries. +func formatRetryCollapse(last pendingTool, count int) string { + tail := strings.TrimPrefix(last.summary, last.callLabel) + tail = strings.Replace(tail, "FAILED", fmt.Sprintf("FAILED ×%d", count), 1) + return last.callLabel + tail +} + +// collapseSameFileReads collapses a consecutive run (>=2) of successful Read +// calls against the same file into a single "[Read#id xN] path -> ok" line. +// offset/limit differing across the run is expected (progressive paging +// through a long file) and does not break the collapse; any failure, a +// different file, or another tool in between does. +func collapseSameFileReads(tools []pendingTool) []pendingTool { + result := make([]pendingTool, 0, len(tools)) + for i := 0; i < len(tools); i++ { + pt := tools[i] + if !isCollapsibleRead(pt) { + result = append(result, pt) + continue + } + j := i + 1 + for j < len(tools) && isCollapsibleRead(tools[j]) && tools[j].readFilePath == pt.readFilePath { + j++ + } + count := j - i + if count < 2 { + result = append(result, pt) + continue + } + last := tools[j-1] + shortID := session.ToolShortID(last.toolUseID) + last.summary = fmt.Sprintf("[Read#%s ×%d] %s -> ok", shortID, count, pt.readDisplayPath) + result = append(result, last) + i = j - 1 + } + return result +} + +// isCollapsibleRead reports whether pt is a successful Read call carrying a +// known file path — the precondition for taking part in a same-file-read +// group at all. +func isCollapsibleRead(pt pendingTool) bool { + return pt.name == session.ToolRead && pt.resultReceived && pt.resultSuccess && pt.readFilePath != "" +} + func flushPendingTools(pendingTools *[]pendingTool, rc renderContext) { tools := *pendingTools if !rc.opts.VerboseBash { tools = collapseCCSessionTools(tools) + tools = collapseRetryLoops(tools) } + tools = collapseSameFileReads(tools) for _, pt := range tools { fmt.Fprintf(rc.out, " %s\n", pt.summary) if rc.sink != nil { diff --git a/internal/summarizer/summarizer.go b/internal/summarizer/summarizer.go index c849c86..6baa262 100644 --- a/internal/summarizer/summarizer.go +++ b/internal/summarizer/summarizer.go @@ -24,7 +24,11 @@ const ( unknownToolValueMaxLen = 60 ) -func cleanPath(path string, cwd string) string { +// CleanPath shortens an absolute file path for display: relative to cwd when +// possible, otherwise the last two path segments. Exported so callers that +// need to display the same short form outside a one-line tool summary (e.g. +// formatter's same-file-Read collapsing) don't have to reimplement it. +func CleanPath(path string, cwd string) string { if path == "" || path == "?" { return path } @@ -56,7 +60,7 @@ func SummarizeToolUse(name string, inp session.ToolInput, cwd string) string { if path == "" { path = "?" } - short := cleanPath(path, cwd) + short := CleanPath(path, cwd) var offset, limit int var hasOffset, hasLimit bool if o, ok := inp.Raw["offset"]; ok { @@ -91,7 +95,7 @@ func SummarizeToolUse(name string, inp session.ToolInput, cwd string) string { if path == "" { path = "?" } - short := cleanPath(path, cwd) + short := CleanPath(path, cwd) return fmt.Sprintf("[%s] %s", name, short) case session.ToolAgent: From 46f4c6b689ae6b685c23936ea840b9fc0f0ab7f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maple=EF=BC=81?= Date: Thu, 16 Jul 2026 11:37:15 +0800 Subject: [PATCH 13/16] docs: ADR-005 collapse rules for retry loops and same-file reads Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01SaBPDHw1cZDxhehYrqFgGe --- .../adr-005-collapse-retry-loops-and-reads.md | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 docs/adr-005-collapse-retry-loops-and-reads.md diff --git a/docs/adr-005-collapse-retry-loops-and-reads.md b/docs/adr-005-collapse-retry-loops-and-reads.md new file mode 100644 index 0000000..da1c8c7 --- /dev/null +++ b/docs/adr-005-collapse-retry-loops-and-reads.md @@ -0,0 +1,43 @@ +# ADR-005: Collapse Retry Loops and Consecutive Same-File Reads + +## Status + +Accepted + +## Context + +Real transcripts contain two high-frequency repetition patterns that each occupy one summary line per call while carrying near-identical information: + +- **Retry loops**: the model retries a failing command with minor variations (a real sample: 11 consecutive failed `git worktree` diagnostics, all producing the same `Path ... does not exist` error). +- **Chunked reads**: the model reads a large file in several offset windows (`Read` of the same path 2–4 times in a row). + +ADR-001 established the precedent for collapsing adjacent redundant lines (consecutive cc-session calls) and its verbose-flag exemption. This ADR extends collapsing to these two patterns. Like all output-format decisions, the collapsed lines are consumed by LLMs reading injected context, so the rules below are a contract. + +## Decision + +### Rule 1 — retry-loop collapse + +Consecutive tool calls collapse into one line when **all** hold: +- same tool name, +- every call FAILED, +- normalized first-line commands are identical or one is a non-empty prefix of the other (tail-argument variations count as the same retry attempt; two commands where neither prefixes the other never merge). + +Rendered as `[Bash#] -> FAILED ×N: `. The **last** call's tool id and error are kept: the final state is what the retry loop converged to, and `expand` on that id reaches it. + +Never collapses across: an interleaved success, a different tool, any non-tool event (assistant text may reference individual results), or when `-verbose-bash` is set (ADR-001 exemption pattern). + +### Rule 2 — same-file Read collapse + +Consecutive successful `Read` calls of the same `file_path` (offsets/limits may differ) collapse to `[Read# ×N] -> ok`. Any interleaved event or failure breaks the group. + +### Invariants + +- **Failure information is never lost to collapsing** — only repetition of the *same* failure is compressed; distinct failures stay on their own lines. +- `×1` never appears; a single call renders unchanged. +- Comparison is deliberately conservative: prefer under-collapsing to mis-collapsing. Widening the identity rule requires a real transcript sample demonstrating the missed pattern, plus a regression test (same evidence bar as ADR-003's sniff list). + +## Consequences + +- The 11-line retry sample renders as 6 lines (distinct commands preserved, three retry groups at ×2/×3/×4). +- `analyzer` stats stay reconciled because KEPT categories are measured from the render sink *after* collapsing (ADR-003 follow-up work); the reconciliation regression test guards this. +- Negative knowledge: collapse identity uses the command's first line, not the full input — multi-line Bash scripts with identical first lines but different bodies will merge if all fail; accepted because the excerpt shown is the last attempt's real error, and expand reaches every collapsed id via the transcript. From cf71ce7907161d00c39013070dfe59c93b576c7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maple=EF=BC=81?= Date: Thu, 16 Jul 2026 11:56:51 +0800 Subject: [PATCH 14/16] fix: collapse only identical errors and require token-boundary multi-token prefix Adversarial verification (Codex) found two violations of ADR-005's invariants: distinct errors under a prefix-matched command merged into one line hiding the earlier error, and bare HasPrefix let 'git add' absorb 'git add-on-something' (and a bare 'git' absorb everything). Collapsing now additionally requires identical failure excerpts, and a prefix match needs a whitespace token boundary plus a multi-token shorter command. ADR-005 revised to match, including the rendered-event boundary semantics and expanded negative knowledge. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01SaBPDHw1cZDxhehYrqFgGe --- .../adr-005-collapse-retry-loops-and-reads.md | 16 ++- internal/formatter/collapse_test.go | 118 +++++++++++++++++- internal/formatter/render.go | 43 +++++-- 3 files changed, 156 insertions(+), 21 deletions(-) diff --git a/docs/adr-005-collapse-retry-loops-and-reads.md b/docs/adr-005-collapse-retry-loops-and-reads.md index da1c8c7..a4d9704 100644 --- a/docs/adr-005-collapse-retry-loops-and-reads.md +++ b/docs/adr-005-collapse-retry-loops-and-reads.md @@ -20,11 +20,12 @@ ADR-001 established the precedent for collapsing adjacent redundant lines (conse Consecutive tool calls collapse into one line when **all** hold: - same tool name, - every call FAILED, -- normalized first-line commands are identical or one is a non-empty prefix of the other (tail-argument variations count as the same retry attempt; two commands where neither prefixes the other never merge). +- normalized first-line commands are identical or one is a prefix of the other **ending on a token boundary and containing more than one token** (the next character after the prefix must be whitespace, and a bare program name never anchors a match — `git add` prefixes `git add --intent-to-add`, but not `git add-on-something`, and a bare `git` does not absorb `git add`; a whitespace boundary alone would wrongly allow the latter), +- **the error excerpts are identical** (same first meaningful error line) — distinct errors never merge, even under the same command. -Rendered as `[Bash#] -> FAILED ×N: `. The **last** call's tool id and error are kept: the final state is what the retry loop converged to, and `expand` on that id reaches it. +Rendered as `[Bash#] -> FAILED ×N: `. The **last** call's tool id is kept: the final state is what the retry loop converged to, and `expand` on that id reaches it. (Since the error-equality condition holds, the shown excerpt is every collapsed call's error, not just the last one's.) -Never collapses across: an interleaved success, a different tool, any non-tool event (assistant text may reference individual results), or when `-verbose-bash` is set (ADR-001 exemption pattern). +Never collapses across: an interleaved success, a different tool, any **rendered** event (assistant text may reference individual results), or when `-verbose-bash` is set (ADR-001 exemption pattern). Non-rendered events (harness noise, system reminders, non-verbose thinking) do not break a group — the reader cannot reference what the output never shows. ### Rule 2 — same-file Read collapse @@ -32,12 +33,15 @@ Consecutive successful `Read` calls of the same `file_path` (offsets/limits may ### Invariants -- **Failure information is never lost to collapsing** — only repetition of the *same* failure is compressed; distinct failures stay on their own lines. +- **Failure information is never lost to collapsing** — enforced by the error-equality condition: only repetition of the *same* failure is compressed; distinct failures stay on their own lines. (The first shipped version lacked this condition and could hide an earlier, different error behind the last one — caught by adversarial review before any release.) - `×1` never appears; a single call renders unchanged. - Comparison is deliberately conservative: prefer under-collapsing to mis-collapsing. Widening the identity rule requires a real transcript sample demonstrating the missed pattern, plus a regression test (same evidence bar as ADR-003's sniff list). ## Consequences -- The 11-line retry sample renders as 6 lines (distinct commands preserved, three retry groups at ×2/×3/×4). +- The 11-line retry sample renders as 6 lines (distinct commands preserved, three retry groups at ×2/×3/×4 — all sharing the same error, so the error-equality condition keeps them collapsed). - `analyzer` stats stay reconciled because KEPT categories are measured from the render sink *after* collapsing (ADR-003 follow-up work); the reconciliation regression test guards this. -- Negative knowledge: collapse identity uses the command's first line, not the full input — multi-line Bash scripts with identical first lines but different bodies will merge if all fail; accepted because the excerpt shown is the last attempt's real error, and expand reaches every collapsed id via the transcript. +- Negative knowledge (accepted, documented limits): + - Collapse identity uses the command's **first line**, not the full input — multi-line Bash scripts with identical first lines and identical error excerpts still merge; acceptable because error equality bounds the information loss and expand reaches every collapsed id via the transcript. + - Whitespace normalization is not shell-aware: `strings.Fields` folds whitespace inside quoted arguments, so commands differing only in quoted internal spacing can share a signature. With the error-equality condition the residual mis-merge risk is negligible; revisit only with a real sample. + - Read grouping keys on the raw `file_path` string, not a canonical path — same relative path under different cwds would merge, and symlink/`..`/absolute-vs-relative spellings of one file won't. Both accepted: cwd changes within one render batch are rare, and under-merging is the preferred failure direction. diff --git a/internal/formatter/collapse_test.go b/internal/formatter/collapse_test.go index 9418356..87ad3bd 100644 --- a/internal/formatter/collapse_test.go +++ b/internal/formatter/collapse_test.go @@ -86,11 +86,11 @@ func writeBashAttemptsWithNarrationFixture(t *testing.T, before, after []bashAtt return transcriptPath } -func TestFormatRead_GivenConsecutiveFailedRetriesWithSameCommand_ThenCollapsesIntoFailedCountLine(t *testing.T) { +func TestFormatRead_GivenConsecutiveFailedRetriesWithSameCommandAndSameError_ThenCollapsesIntoFailedCountLine(t *testing.T) { transcriptPath := writeBashAttemptsFixture(t, []bashAttempt{ - {toolID: "retry-tool-1", command: "npm test", description: "Run tests", success: false, resultText: "Error: 2 tests failed"}, - {toolID: "retry-tool-2", command: "npm test", description: "Run tests", success: false, resultText: "Error: 3 tests failed"}, - {toolID: "retry-tool-3", command: "npm test", description: "Run tests", success: false, resultText: "Error: TypeError: cannot read prop"}, + {toolID: "retry-tool-1", command: "npm test", description: "Run tests", success: false, resultText: "Error: connection refused"}, + {toolID: "retry-tool-2", command: "npm test", description: "Run tests", success: false, resultText: "Error: connection refused"}, + {toolID: "retry-tool-3", command: "npm test", description: "Run tests", success: false, resultText: "Error: connection refused"}, }) var out bytes.Buffer @@ -99,7 +99,7 @@ func TestFormatRead_GivenConsecutiveFailedRetriesWithSameCommand_ThenCollapsesIn } got := out.String() - want := "[Bash#ol-3] Run tests -> FAILED ×3: Error: TypeError: cannot read prop" + want := "[Bash#ol-3] Run tests -> FAILED ×3: Error: connection refused" if !strings.Contains(got, want) { t.Fatalf("expected collapsed retry-loop line\nwant substring: %q\ngot:\n%s", want, got) } @@ -108,6 +108,39 @@ func TestFormatRead_GivenConsecutiveFailedRetriesWithSameCommand_ThenCollapsesIn } } +func TestFormatRead_GivenConsecutiveFailedRetriesWithSameCommandButDifferentErrors_ThenDoesNotCollapse(t *testing.T) { + // Regression guard: collapseRetryLoops used to key only on tool name + + // command signature + failure status, so three attempts of the same + // command failing with three *different* errors collapsed into one + // "FAILED ×3" line carrying only the last attempt's error — silently + // hiding the first two. That violates ADR-005's invariant that failure + // information is never lost, so attempts whose error excerpts differ + // must stay split even when the command matches. + transcriptPath := writeBashAttemptsFixture(t, []bashAttempt{ + {toolID: "retry-tool-1", command: "npm test", description: "Run tests", success: false, resultText: "Error: 2 tests failed"}, + {toolID: "retry-tool-2", command: "npm test", description: "Run tests", success: false, resultText: "Error: 3 tests failed"}, + {toolID: "retry-tool-3", command: "npm test", description: "Run tests", success: false, resultText: "Error: TypeError: cannot read prop"}, + }) + + var out bytes.Buffer + if err := FormatRead(transcriptPath, 0, 0, FormatOptions{}, &out, claudecodec.Codec{}); err != nil { + t.Fatalf("FormatRead returned error: %v", err) + } + got := out.String() + + if strings.Contains(got, "×") { + t.Fatalf("attempts with different error excerpts must not collapse, got:\n%s", got) + } + if strings.Count(got, "[Bash#") != 3 { + t.Fatalf("expected all three attempts to render individually, got:\n%s", got) + } + for _, want := range []string{"Error: 2 tests failed", "Error: 3 tests failed", "Error: TypeError: cannot read prop"} { + if !strings.Contains(got, want) { + t.Fatalf("expected every distinct error to remain visible, missing %q, got:\n%s", want, got) + } + } +} + func TestFormatRead_GivenSingleFailedBashCall_ThenDoesNotShowMultiplier(t *testing.T) { // A run of exactly one failed attempt must render as a normal FAILED line — // "×1" would be a meaningless label for something that wasn't retried. @@ -185,6 +218,81 @@ func TestFormatRead_GivenMixedSuccessAndFailureSameCommand_ThenDoesNotCollapse(t } } +func TestFormatRead_GivenCommandSharesWordPrefixWithoutTokenBoundary_ThenDoesNotCollapse(t *testing.T) { + // Regression guard: sameRetryCommand used a bare strings.HasPrefix, so + // "git add" matched as a prefix of "git add-on-something" even though + // "add" and "add-on-something" are different tokens — a false-positive + // retry match. A prefix only counts when the longer command's next + // character after the shorter one is a token boundary (whitespace). + transcriptPath := writeBashAttemptsFixture(t, []bashAttempt{ + {toolID: "prefix-tool-1", command: "git add", description: "Stage file", success: false, resultText: "Error: pathspec 'a' did not match any files"}, + {toolID: "prefix-tool-2", command: "git add-on-something", description: "Stage file", success: false, resultText: "Error: pathspec 'a' did not match any files"}, + }) + + var out bytes.Buffer + if err := FormatRead(transcriptPath, 0, 0, FormatOptions{}, &out, claudecodec.Codec{}); err != nil { + t.Fatalf("FormatRead returned error: %v", err) + } + got := out.String() + + if strings.Contains(got, "×") { + t.Fatalf("commands sharing a word prefix without a token boundary must not collapse, got:\n%s", got) + } + if strings.Count(got, "[Bash#") != 2 { + t.Fatalf("expected both attempts to render individually, got:\n%s", got) + } +} + +func TestFormatRead_GivenBareCommandFollowedByLongerCommand_ThenDoesNotCollapse(t *testing.T) { + // Regression guard: when the first attempt's command is a bare word + // (e.g. "git"), the old prefix check let it absorb any later command + // starting with that word ("git add", "git commit", ...) into the same + // retry group, even though they are unrelated calls. + transcriptPath := writeBashAttemptsFixture(t, []bashAttempt{ + {toolID: "bare-tool-1", command: "git", description: "Check status", success: false, resultText: "Error: not a git repository"}, + {toolID: "bare-tool-2", command: "git add", description: "Stage file", success: false, resultText: "Error: not a git repository"}, + }) + + var out bytes.Buffer + if err := FormatRead(transcriptPath, 0, 0, FormatOptions{}, &out, claudecodec.Codec{}); err != nil { + t.Fatalf("FormatRead returned error: %v", err) + } + got := out.String() + + if strings.Contains(got, "×") { + t.Fatalf("a bare command must not absorb a longer, unrelated command, got:\n%s", got) + } + if strings.Count(got, "[Bash#") != 2 { + t.Fatalf("expected both attempts to render individually, got:\n%s", got) + } +} + +func TestFormatRead_GivenRetryCommandExtendedWithTrailingArgs_ThenStillCollapses(t *testing.T) { + // A legitimate prefix retry — same base command, an extra trailing + // argument, separated by a token boundary — must keep collapsing. This + // guards against the token-boundary fix above becoming so strict it + // breaks the "only the trailing argument differs" case the prefix match + // was originally added for. + transcriptPath := writeBashAttemptsFixture(t, []bashAttempt{ + {toolID: "extend-tool-1", command: "npm test", description: "Run tests", success: false, resultText: "Error: connection refused"}, + {toolID: "extend-tool-2", command: "npm test -- --seed=42", description: "Run tests", success: false, resultText: "Error: connection refused"}, + }) + + var out bytes.Buffer + if err := FormatRead(transcriptPath, 0, 0, FormatOptions{}, &out, claudecodec.Codec{}); err != nil { + t.Fatalf("FormatRead returned error: %v", err) + } + got := out.String() + + want := "[Bash#ol-2] Run tests -> FAILED ×2: Error: connection refused" + if !strings.Contains(got, want) { + t.Fatalf("expected collapsed retry-loop line\nwant substring: %q\ngot:\n%s", want, got) + } + if strings.Count(got, "[Bash#") != 1 { + t.Fatalf("expected exactly one Bash summary line after collapsing, got:\n%s", got) + } +} + func TestFormatRead_GivenVerboseBashAndRetryLoop_ThenSkipsCollapse(t *testing.T) { // Mirrors the existing -verbose-bash exemption for collapseCCSessionTools: // full Bash output must be preserved per attempt, not discarded by diff --git a/internal/formatter/render.go b/internal/formatter/render.go index cbfb328..d8bc6b7 100644 --- a/internal/formatter/render.go +++ b/internal/formatter/render.go @@ -399,11 +399,12 @@ func collapseCCSessionTools(tools []pendingTool) []pendingTool { // collapseRetryLoops collapses a consecutive run (>=2) of pendingTools that // share the same tool name, the same (whitespace-normalized, prefix-matched) -// retrySignature, and all FAILED, into a single "FAILED xN" line — a retry -// loop that repeats the same failing call N times otherwise contributes N -// near-identical lines. A run of exactly 1 is left untouched (no "x1" label); -// a success or an unrelated call in between breaks the run so its individual -// failure information is never silently dropped. +// retrySignature, the same failure excerpt, and all FAILED, into a single +// "FAILED xN" line — a retry loop that repeats the same failing call N times +// otherwise contributes N near-identical lines. A run of exactly 1 is left +// untouched (no "x1" label); a success, an unrelated call, or an attempt +// whose error excerpt differs breaks the run so its individual failure +// information is never silently dropped (ADR-005). func collapseRetryLoops(tools []pendingTool) []pendingTool { result := make([]pendingTool, 0, len(tools)) for i := 0; i < len(tools); i++ { @@ -415,7 +416,8 @@ func collapseRetryLoops(tools []pendingTool) []pendingTool { j := i + 1 for j < len(tools) && isRetryEligible(tools[j]) && tools[j].name == pt.name && - sameRetryCommand(pt.retrySignature, tools[j].retrySignature) { + sameRetryCommand(pt.retrySignature, tools[j].retrySignature) && + retryFailureExcerpt(tools[j]) == retryFailureExcerpt(pt) { j++ } count := j - i @@ -431,6 +433,17 @@ func collapseRetryLoops(tools []pendingTool) []pendingTool { return result } +// retryFailureExcerpt isolates the part of a failed pendingTool's summary +// that appendToolResult appended after callLabel (the " -> FAILED: " +// tail) so collapseRetryLoops can compare failure content across attempts. +// Two attempts of the same command that fail with different errors must not +// merge into one "FAILED xN" line showing only the last error — that would +// silently drop the earlier, distinct failures (ADR-005: failure information +// is never lost). +func retryFailureExcerpt(pt pendingTool) string { + return strings.TrimPrefix(pt.summary, pt.callLabel) +} + // isRetryEligible reports whether pt finished with a failure and carries a // non-empty retry signature to key on — the precondition for taking part in // a retry-loop group at all. @@ -439,8 +452,15 @@ func isRetryEligible(pt pendingTool) bool { } // sameRetryCommand reports whether a and b identify the same retried call: -// exact match, or one is a non-empty prefix of the other (covers "only the -// trailing argument differs" retries, e.g. a script rerun with a new seed). +// exact match, or one is a non-empty prefix of the other ending exactly at a +// token boundary and already committing to more than a bare program name +// (covers "only the trailing argument differs" retries, e.g. a script rerun +// with a new seed). Both conditions matter: the boundary check keeps "git +// add" from matching "git add-on-thing" — they share a byte prefix but +// "add" and "add-on-thing" are different tokens — and the multi-token +// requirement keeps a bare "git" from absorbing "git add" or "git commit": +// a single bare word is too generic to identify which call is being +// retried, so it can't anchor a prefix match on its own. func sameRetryCommand(a, b string) bool { if a == b { return true @@ -449,7 +469,10 @@ func sameRetryCommand(a, b string) bool { if len(longer) < len(shorter) { shorter, longer = longer, shorter } - return shorter != "" && strings.HasPrefix(longer, shorter) + if shorter == "" || !strings.HasPrefix(longer, shorter) { + return false + } + return longer[len(shorter)] == ' ' && strings.Contains(shorter, " ") } // formatRetryCollapse rebuilds the collapsed retry-loop line from the last @@ -460,7 +483,7 @@ func sameRetryCommand(a, b string) bool { // inserting the count there mirrors the string-surgery injectShortID already // uses to tag summaries. func formatRetryCollapse(last pendingTool, count int) string { - tail := strings.TrimPrefix(last.summary, last.callLabel) + tail := retryFailureExcerpt(last) tail = strings.Replace(tail, "FAILED", fmt.Sprintf("FAILED ×%d", count), 1) return last.callLabel + tail } From f78aede50ae2b82d0b4fa3b0bce8fd607a0b9519 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maple=EF=BC=81?= Date: Thu, 16 Jul 2026 12:42:27 +0800 Subject: [PATCH 15/16] fix: windows-safe project dir names and CRLF-tolerant SKILL.md parsing in tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ProjectDirName (single source of truth, was duplicated across production caller detection and two test helpers) now also normalizes backslashes and drive colons — a no-op on unix paths; the SKILL.md frontmatter test helper trims CR before quote stripping. Both regressions are pinned by OS-independent tests that inject windows-style inputs directly. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01SaBPDHw1cZDxhehYrqFgGe --- cmd/cc-session/main_test.go | 48 ++++++++++++++++++++++++-------- internal/tracker/tracker.go | 17 +++++++++-- internal/tracker/tracker_test.go | 30 ++++++++++++++++++-- 3 files changed, 79 insertions(+), 16 deletions(-) diff --git a/cmd/cc-session/main_test.go b/cmd/cc-session/main_test.go index 4674a03..bea90d5 100644 --- a/cmd/cc-session/main_test.go +++ b/cmd/cc-session/main_test.go @@ -1447,6 +1447,23 @@ func TestCheatSheet_GivenRegistry_ThenCoversExactlyNonHiddenHintedCommands(t *te } } +// argumentHintFromSkillMD extracts the "argument-hint: ..." frontmatter +// value from SKILL.md's raw content, stripping the surrounding quotes. It +// trims a trailing "\r" off each line first, since a Windows checkout +// converts this repo's LF line endings to CRLF and strings.Split(content, +// "\n") would otherwise leave "\r" attached to the line (and thus inside +// the trimmed value). Returns false if no argument-hint line is found. +func argumentHintFromSkillMD(content string) (string, bool) { + const linePrefix = "argument-hint: " + for _, line := range strings.Split(content, "\n") { + line = strings.TrimRight(line, "\r") + if strings.HasPrefix(line, linePrefix) { + return strings.Trim(strings.TrimPrefix(line, linePrefix), `"`), true + } + } + return "", false +} + // Regression guard for finding 4: SKILL.md's argument-hint frontmatter is a // hand-written literal with no compiler-enforced link to buildArgumentHint. // This asserts it matches verbatim, so a registry change that isn't synced @@ -1457,25 +1474,34 @@ func TestSkillMD_GivenArgumentHintFrontmatter_ThenMatchesBuildArgumentHint(t *te t.Fatalf("read SKILL.md: %v", err) } - const linePrefix = "argument-hint: " - var hintLine string - for _, line := range strings.Split(string(data), "\n") { - if strings.HasPrefix(line, linePrefix) { - hintLine = line - break - } - } - if hintLine == "" { + got, ok := argumentHintFromSkillMD(string(data)) + if !ok { t.Fatal("SKILL.md frontmatter has no argument-hint line") } - got := strings.Trim(strings.TrimPrefix(hintLine, linePrefix), `"`) want := buildArgumentHint() if got != want { t.Fatalf("SKILL.md argument-hint = %q, want %q (buildArgumentHint output); sync SKILL.md's frontmatter with the registry", got, want) } } +// Regression: on a Windows CI runner, the checkout's CRLF line endings left +// "\r" attached to each split line, so the closing-quote trim never matched +// (e.g. `"read "` + "\r" stayed as `read "` + "\r" instead of +// `read `). Feeds a CRLF frontmatter line directly so this is verified +// on any OS instead of depending on the checkout's actual line endings. +func TestArgumentHintFromSkillMD_GivenCRLFLineEndings_ThenStripsCarriageReturn(t *testing.T) { + content := "---\r\nargument-hint: \"read \"\r\n---\r\n" + + got, ok := argumentHintFromSkillMD(content) + if !ok { + t.Fatal("argumentHintFromSkillMD ok = false, want true") + } + if got != "read " { + t.Fatalf("argumentHintFromSkillMD = %q, want %q", got, "read ") + } +} + func TestFindCommand_GivenInjectName_ThenReturnsHiddenButDispatchable(t *testing.T) { cmd, ok := findCommand("inject") if !ok { @@ -1535,7 +1561,7 @@ func usageTestCallerSession(t *testing.T, root string) { if resolved, err := filepath.EvalSymlinks(cwd); err == nil { cwd = resolved } - projectDir := filepath.Join(root, ".claude", "projects", strings.ReplaceAll(cwd, "/", "-")) + projectDir := filepath.Join(root, ".claude", "projects", tracker.ProjectDirName(cwd)) if err := os.MkdirAll(projectDir, 0o755); err != nil { t.Fatalf("MkdirAll: %v", err) } diff --git a/internal/tracker/tracker.go b/internal/tracker/tracker.go index 0f12968..622f585 100644 --- a/internal/tracker/tracker.go +++ b/internal/tracker/tracker.go @@ -209,15 +209,26 @@ func DetectCallerSession(cwd string) string { return DetectCallerSessionWithBase(cwd, filepath.Join(home, ".claude", "projects")) } +// ProjectDirName maps an absolute working directory path to the directory +// name Claude Code uses under ~/.claude/projects, by replacing every path +// separator with "-" (e.g. /Users/maple/Desktop -> -Users-maple-Desktop). +// A Windows cwd uses "\" as its separator and may carry a drive letter +// (e.g. "C:"); both "\" and ":" are illegal inside a single path segment, +// so they are normalized the same way as "/" — otherwise the mapped name +// would embed an OS-illegal segment (e.g. "D:") that os.MkdirAll refuses to +// create. This is a no-op for a macOS/Linux cwd, which never contains +// "\" or ":". +func ProjectDirName(cwd string) string { + return strings.NewReplacer("\\", "-", "/", "-", ":", "-").Replace(cwd) +} + // DetectCallerSessionWithBase is the testable variant of DetectCallerSession // that accepts an explicit projectsDir. func DetectCallerSessionWithBase(cwd string, projectsDir string) string { if resolved, err := filepath.EvalSymlinks(cwd); err == nil { cwd = resolved } - // Claude Code maps an absolute path to a project dir by replacing every - // "/" with "-", e.g. /Users/maple/Desktop -> -Users-maple-Desktop. - projectDir := filepath.Join(projectsDir, strings.ReplaceAll(cwd, "/", "-")) + projectDir := filepath.Join(projectsDir, ProjectDirName(cwd)) entries, err := os.ReadDir(projectDir) if err != nil { diff --git a/internal/tracker/tracker_test.go b/internal/tracker/tracker_test.go index 60cdf1f..5de8599 100644 --- a/internal/tracker/tracker_test.go +++ b/internal/tracker/tracker_test.go @@ -365,8 +365,7 @@ func TestDetectCallerSessionWithBase_GivenMultipleJSONL_WhenDetected_ThenReturns projectsDir := t.TempDir() cwd := "/Users/maple/Desktop" - // Claude Code maps the cwd by replacing "/" with "-". - projectDir := filepath.Join(projectsDir, strings.ReplaceAll(cwd, "/", "-")) + projectDir := filepath.Join(projectsDir, ProjectDirName(cwd)) if err := os.MkdirAll(projectDir, 0o755); err != nil { t.Fatalf("create project dir: %v", err) } @@ -394,3 +393,30 @@ func TestDetectCallerSessionWithBase_GivenMultipleJSONL_WhenDetected_ThenReturns t.Errorf("DetectCallerSessionWithBase = %q, want %q", got, "newer-session-uuid") } } + +func TestProjectDirName_GivenUnixStylePath_ThenReplacesSlashWithDash(t *testing.T) { + got := ProjectDirName("/Users/maple/Desktop") + want := "-Users-maple-Desktop" + if got != want { + t.Errorf("ProjectDirName = %q, want %q", got, want) + } +} + +// Regression: a Windows CI runner's cwd (e.g. from os.Getwd() under +// D:\a\repo\repo) uses "\" separators and a drive-letter colon. +// ProjectDirName used to only replace "/", so "\" and ":" survived into the +// mapped name; once joined under ~/.claude/projects, "D:" then exists as a +// mid-path segment, which os.MkdirAll refuses to create on Windows ("The +// filename, directory name, or volume label syntax is incorrect"). Every +// separator and colon must collapse to "-" so the mapped name is always one +// legal path segment, regardless of which OS produced the cwd. +func TestProjectDirName_GivenWindowsStyleCwd_ThenReplacesBackslashAndColon(t *testing.T) { + got := ProjectDirName(`D:\a\claude-code-session-reader\claude-code-session-reader`) + want := "D--a-claude-code-session-reader-claude-code-session-reader" + if got != want { + t.Errorf("ProjectDirName = %q, want %q", got, want) + } + if strings.ContainsAny(got, `/\:`) { + t.Errorf("ProjectDirName = %q, contains a separator or colon that MkdirAll would reject as a mid-path segment", got) + } +} From 31ed55f5b06c920ae6ff696d1691b4f6e9b9c500 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maple=EF=BC=81?= Date: Thu, 16 Jul 2026 14:19:21 +0800 Subject: [PATCH 16/16] chore: gitignore local .claude directory Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01SaBPDHw1cZDxhehYrqFgGe --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 3953482..d706124 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,4 @@ bin/ coverage.out dist/ /cc-session +.claude/