feat: rename tracer to recap with loop detection and enhanced viewer#381
feat: rename tracer to recap with loop detection and enhanced viewer#381anandgupta42 merged 15 commits intomainfrom
Conversation
…y, and enhanced viewer - Rename `Tracer` class to `Recap` with backward-compat aliases - Rename CLI command `trace` to `recap` (hidden `trace` alias preserved) - Add loop detection: flags repeated tool calls with same input (3+ in last 10) - Add post-session summary: `narrative`, `topTools`, `loops` in trace output - New Summary tab (default) in HTML viewer with: - Truncated prompt with expand toggle - Files changed with SQL diff previews - Tool-agnostic outcome extraction (dbt, pytest, Airflow, pip, SQL) - Deduped dbt commands with pass/fail status, clickable to waterfall - Smart command grouping (boring ls/cd collapsed, meaningful shown) - Error details with resolution tracking - Cost breakdown in collapsible section - Virality: Share Recap (self-contained HTML download), Copy Summary (markdown), Copy Link, branded footer - Fix XSS: timeline items escaped with `e()` - Fix memory leak: per-session `sessionUserMsgIds` with cleanup on eviction - Fix JS syntax: onclick quote escaping in collapsible section - Bound `toolCallHistory` to prevent unbounded growth (cap at 200) - Summary view wrapped in try-catch for visible error messages - Update all 13 test files for rename + 8 new adversarial viewer tests - Update docs: `tracing.md` → `recap.md`, CLI/TUI references updated Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRename tracing to “Recap” across code, CLI, TUI, docs, and tests; add loop detection, post-session summary (narrative, topTools, loops), extend persisted recap schema, overhaul HTML viewer with a Summary tab and share/copy exports, and preserve backward-compatible aliases. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…l viewer tests - Fix critical bug: Share Recap and Copy Summary buttons referenced variables from Summary IIFE scope — rewrote `buildMarkdownSummary` to be self-contained - Fix `t.text` → `t.result` in narrative (was rendering "undefined") - Fix `sessionUserMsgIds` not cleaned on MAX_RECAPS eviction (memory leak) - Fix zero cost display: show `$0.00` instead of em-dash - Add try-catch error boundary around Summary view rendering - Add 8 adversarial viewer tests: XSS, NaN/Infinity, null metadata, 200+ spans, JS syntax validation, tool-agnostic outcomes, backward compat Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (1)
packages/opencode/test/altimate/tracing-de-attributes.test.ts (1)
19-28: Consider migrating totmpdir()fixture for consistency.This test uses manual
beforeEach/afterEachsetup withos.tmpdir()andfs.rm()for cleanup. The project's coding guidelines recommend using thetmpdirfunction fromfixture/fixture.tswithawait usingsyntax for automatic cleanup.This is pre-existing code not changed in this PR, so it's a low-priority refactor.
♻️ Example refactor pattern
-import os from "os" +import { tmpdir } from "../fixture/fixture" -let tmpDir: string - -beforeEach(async () => { - tmpDir = path.join(os.tmpdir(), `tracing-de-${Date.now()}-${Math.random().toString(36).slice(2)}`) - await fs.mkdir(tmpDir, { recursive: true }) -}) - -afterEach(async () => { - await fs.rm(tmpDir, { recursive: true, force: true }).catch(() => {}) -}) // Then in each test: - const tracer = Recap.withExporters([new FileExporter(tmpDir)]) + await using tmp = await tmpdir() + const tracer = Recap.withExporters([new FileExporter(tmp.path)])Based on learnings: "Use the
tmpdirfunction fromfixture/fixture.tsto create temporary directories for tests with automatic cleanup in test files" and "Always useawait usingsyntax withtmpdir()for automatic cleanup when the variable goes out of scope".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/test/altimate/tracing-de-attributes.test.ts` around lines 19 - 28, Replace the manual beforeEach/afterEach tmpDir setup with the project's tmpdir fixture: remove the tmpDir variable and the beforeEach/afterEach blocks and instead use the tmpdir() fixture via "await using" to create a temp directory for the test scope; reference the tmpdir function from fixture/fixture.ts and update tests that read from the tmpDir variable to use the fixture-provided directory name returned by tmpdir().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/docs/usage/cli.md`:
- Around line 152-153: The heading says “Disable recap” but the example uses the
legacy flag --no-trace; update the docs to explicitly state that --no-trace is
the compatibility flag and that --no-recap is not supported (add a short note
under the example line `altimate run --no-trace "quick question"` clarifying
this). Locate the example and the heading in the CLI usage doc around the
"Disable recap for a single run" section and insert a one-line note referencing
the flag name --no-trace and the absence of --no-recap so readers aren’t
confused.
In `@packages/opencode/src/altimate/observability/tracing.ts`:
- Around line 305-306: loopsDetected currently aggregates loops only by tool
name so a second loop on the same tool with a different inputHash overwrites the
first; change the loop-tracking to key by the tuple (tool, inputHash): update
the loopsDetected type to include inputHash (e.g., { tool: string; inputHash:
string; count: number; firstSeen: number; lastSeen: number }), update any logic
that searches/updates loopsDetected (and the code that pushes into
toolCallHistory) to match both tool and inputHash when finding an existing
entry, and ensure summary.loops reads the inputHash-aware entries (also apply
the same change in the other occurrence around the logic referenced as lines
594-623).
- Around line 875-883: The code unconditionally sets trace.summary.narrative to
a “Completed in …” success message even when endTrace(error) has set
trace.summary.status to "error"; update the endTrace logic so you only compose
and assign the success narrative when trace.summary.status === "success" (or
when no error is present). Locate endTrace and the assignment to
trace.summary.narrative (the block that computes dur, top3, toolsStr,
loopWarning, costStr) and guard that whole assignment behind a status check (or
generate a distinct error narrative when status === "error") so failing traces
do not get a success message.
In `@packages/opencode/src/altimate/observability/viewer.ts`:
- Around line 300-305: The tab elements currently rendered as plain divs inside
the "tabs" container (class "tabs" and children with class "tab" and data-view
values "summary","waterfall","tree","chat","log") are not keyboard accessible;
update the rendering in viewer.ts to either output semantic <button> elements or
add proper ARIA tab semantics (role="tablist" on the container, role="tab" on
each tab), set tabindex for focus, and implement keyboard handlers
(Left/Right/Home/End and Enter/Space activation) to change views using the same
handler that click currently calls (the code that reads data-view to switch
views). Repeat the same change for the similar tab block at lines 392-400 so all
tab groups are keyboard-operable.
- Around line 893-1027: The timelineEvents array is never sorted so events with
a time field can appear out of chronological order; after you finish populating
timelineEvents (before the final timelineEvents.forEach that renders into html)
perform a stable sort of timelineEvents by numeric time (use 0 as the default
when time is missing) so items are rendered chronologically; locate the
timelineEvents variable and the render loop near the end of this block and add
the sort step there (e.g., sort by (a.time||0) - (b.time||0)) to ensure
commands, sql, and error events appear in time order.
- Around line 825-835: The grouping currently uses a truncated key
(o.command.slice(0, 60)) which collapses distinct long commands; change the
dedupe key in the cmdOutcomes aggregation to use the full normalized command
(e.g. use o.command or the normalized form used elsewhere) when building
outcomeMap and when tracking per-command counters (like dbtErrors) so each
command keeps its own status/span/result; only apply slice(0,60) at render time
when producing the display string. Update the same logic in the other
occurrences referenced (the blocks around the 929-941 and 955-970 diffs) to use
the full command as the map key and per-command state, and truncate solely in
the UI/formatting code path.
In `@packages/opencode/src/cli/cmd/trace.ts`:
- Around line 100-101: The footer/help text prints Recap.getTracesDir() without
the configured directory, so update the two UI.println calls that reference
Recap.getTracesDir() (in the recap list command where traces is computed) to
pass the same configured tracing directory you used to load traces (the variable
or option that produced traces) — e.g., call Recap.getTracesDir(tracingDir) or
Recap.getTracesDir(config.tracing.dir) so the printed path matches the directory
actually used.
In `@packages/opencode/src/cli/cmd/tui/worker.ts`:
- Around line 148-165: The branch handling event.type === "message.updated" only
uses info.sessionID to locate or create a recap, so assistant events that
include a parentID but lack sessionID never call enrichFromAssistant; update the
logic in the message.updated handler to also accept info.parentID when resolving
the recap (use sessionRecaps.get(info.sessionID ?? info.parentID) and call
getOrCreateRecap(info.sessionID ?? info.parentID) as needed), ensuring the
subsequent enrichFromAssistant(...) on the resolved recap runs for assistant
messages that provide parentID instead of sessionID.
- Around line 85-90: The current eviction unconditionally finalizes and deletes
the oldest recap when sessionRecaps.size >= MAX_RECAPS, which can evict an
in-flight recap that is still emitting events; change the eviction logic (around
sessionRecaps, sessionUserMsgIds, MAX_RECAPS) to pick a safe candidate: iterate
sessionRecaps.keys() to find the oldest recap whose trace is complete or not
active (use an existing activity flag or expose a method like
isActive/isFinished on the recap object), and only call endTrace() and delete
that key; if no non-active recap exists, skip eviction (do not delete or call
endTrace) and avoid recreating a live recap in getOrCreateRecap(); update
getOrCreateRecap() to respect this and refrain from silently dropping spans when
a live recap was skipped.
In `@packages/opencode/test/altimate/tracing.test.ts`:
- Around line 699-709: The test never exercises the Recap.withExporters(..., {
maxFiles: 2 }) path because it creates new FileExporter instances inside the
loop; replace the loop to use the previously created tracer from
Recap.withExporters (the tracer variable) to startTrace/endTrace four times so
the FileExporter instance managed by Recap.withExporters enforces maxFiles;
specifically, stop creating new FileExporter(tmpDir, 2) inside the loop and call
tracer.startTrace(...) and await tracer.endTrace() for each iteration to trigger
the maxFiles behavior on the exporter returned by Recap.withExporters.
---
Nitpick comments:
In `@packages/opencode/test/altimate/tracing-de-attributes.test.ts`:
- Around line 19-28: Replace the manual beforeEach/afterEach tmpDir setup with
the project's tmpdir fixture: remove the tmpDir variable and the
beforeEach/afterEach blocks and instead use the tmpdir() fixture via "await
using" to create a temp directory for the test scope; reference the tmpdir
function from fixture/fixture.ts and update tests that read from the tmpDir
variable to use the fixture-provided directory name returned by tmpdir().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3ad59b56-d8b0-4c9c-8403-da61e99c2b6b
📒 Files selected for processing (27)
.github/meta/commit.txtREADME.mddocs/docs/configure/recap.mddocs/docs/data-engineering/guides/ci-headless.mddocs/docs/usage/ci-headless.mddocs/docs/usage/cli.mddocs/docs/usage/tui.mddocs/mkdocs.ymlpackages/opencode/src/altimate/observability/tracing.tspackages/opencode/src/altimate/observability/viewer.tspackages/opencode/src/cli/cmd/trace.tspackages/opencode/src/cli/cmd/tui/app.tsxpackages/opencode/src/cli/cmd/tui/component/dialog-trace-list.tsxpackages/opencode/src/cli/cmd/tui/worker.tspackages/opencode/src/index.tspackages/opencode/test/altimate/tracing-adversarial-2.test.tspackages/opencode/test/altimate/tracing-adversarial-final.test.tspackages/opencode/test/altimate/tracing-adversarial-snapshot.test.tspackages/opencode/test/altimate/tracing-adversarial.test.tspackages/opencode/test/altimate/tracing-de-attributes.test.tspackages/opencode/test/altimate/tracing-display-crash.test.tspackages/opencode/test/altimate/tracing-e2e.test.tspackages/opencode/test/altimate/tracing-final-audit.test.tspackages/opencode/test/altimate/tracing-integration.test.tspackages/opencode/test/altimate/tracing-persistence.test.tspackages/opencode/test/altimate/tracing-thorough.test.tspackages/opencode/test/altimate/tracing.test.ts
| <div class="tabs" id="tabs"> | ||
| <div class="tab active" data-view="waterfall">Waterfall</div> | ||
| <div class="tab active" data-view="summary">Summary</div> | ||
| <div class="tab" data-view="waterfall">Waterfall</div> | ||
| <div class="tab" data-view="tree">Tree</div> | ||
| <div class="tab" data-view="chat">Chat</div> | ||
| <div class="tab" data-view="log">Log</div> |
There was a problem hiding this comment.
Make the view tabs keyboard-operable.
These tabs are plain divs driven only by click events, so keyboard and assistive-tech users cannot switch from Summary to Waterfall/Tree/Chat/Log. Please render them as buttons or add proper tab semantics and keyboard handlers.
Also applies to: 392-400
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/opencode/src/altimate/observability/viewer.ts` around lines 300 -
305, The tab elements currently rendered as plain divs inside the "tabs"
container (class "tabs" and children with class "tab" and data-view values
"summary","waterfall","tree","chat","log") are not keyboard accessible; update
the rendering in viewer.ts to either output semantic <button> elements or add
proper ARIA tab semantics (role="tablist" on the container, role="tab" on each
tab), set tabindex for focus, and implement keyboard handlers
(Left/Right/Home/End and Enter/Space activation) to change views using the same
handler that click currently calls (the code that reads data-view to switch
views). Repeat the same change for the similar tab block at lines 392-400 so all
tab groups are keyboard-operable.
| var outcomeMap = {}; | ||
| cmdOutcomes.forEach(function(o) { | ||
| var key = o.command.slice(0, 60); | ||
| if (!outcomeMap[key]) { | ||
| outcomeMap[key] = { command: o.command, result: o.result, status: o.status, spanId: o.spanId, count: 0 }; | ||
| } | ||
| outcomeMap[key].count++; | ||
| if (o.status === 'error') outcomeMap[key].status = 'error'; | ||
| if (o.spanId) outcomeMap[key].spanId = o.spanId; | ||
| // Keep the most informative result text | ||
| if (o.result.length > outcomeMap[key].result.length) outcomeMap[key].result = o.result; |
There was a problem hiding this comment.
Group commands by the full normalized command.
Using slice(0, 60) as the dedupe key collapses different long commands into one outcome row and one Waterfall link, and the shared dbtErrors counter makes every grouped dbt command look like a warning if any dbt invocation failed. Keep per-command state keyed by the full normalized command, then truncate only when rendering.
Also applies to: 929-941, 955-970
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/opencode/src/altimate/observability/viewer.ts` around lines 825 -
835, The grouping currently uses a truncated key (o.command.slice(0, 60)) which
collapses distinct long commands; change the dedupe key in the cmdOutcomes
aggregation to use the full normalized command (e.g. use o.command or the
normalized form used elsewhere) when building outcomeMap and when tracking
per-command counters (like dbtErrors) so each command keeps its own
status/span/result; only apply slice(0,60) at render time when producing the
display string. Update the same logic in the other occurrences referenced (the
blocks around the 929-941 and 955-970 diffs) to use the full command as the map
key and per-command state, and truncate solely in the UI/formatting code path.
| if (sessionRecaps.size >= MAX_RECAPS) { | ||
| const oldest = sessionRecaps.keys().next().value | ||
| if (oldest) { | ||
| sessionTracers.get(oldest)?.endTrace().catch(() => {}) | ||
| sessionTracers.delete(oldest) | ||
| sessionRecaps.get(oldest)?.endTrace().catch(() => {}) | ||
| sessionRecaps.delete(oldest) | ||
| sessionUserMsgIds.delete(oldest) |
There was a problem hiding this comment.
Don't evict an in-flight recap to enforce MAX_RECAPS.
Lines 85-90 finalize and delete the oldest active session as soon as the 51st recap is created. If that session keeps emitting events, getOrCreateRecap() will recreate it from scratch and silently drop the earlier spans.
🩹 Safer fallback
- if (sessionRecaps.size >= MAX_RECAPS) {
- const oldest = sessionRecaps.keys().next().value
- if (oldest) {
- sessionRecaps.get(oldest)?.endTrace().catch(() => {})
- sessionRecaps.delete(oldest)
- sessionUserMsgIds.delete(oldest)
- }
- }
+ if (sessionRecaps.size >= MAX_RECAPS) {
+ Log.Default.warn("recap capacity reached", {
+ maxRecaps: MAX_RECAPS,
+ sessionID,
+ })
+ return null
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/opencode/src/cli/cmd/tui/worker.ts` around lines 85 - 90, The
current eviction unconditionally finalizes and deletes the oldest recap when
sessionRecaps.size >= MAX_RECAPS, which can evict an in-flight recap that is
still emitting events; change the eviction logic (around sessionRecaps,
sessionUserMsgIds, MAX_RECAPS) to pick a safe candidate: iterate
sessionRecaps.keys() to find the oldest recap whose trace is complete or not
active (use an existing activity flag or expose a method like
isActive/isFinished on the recap object), and only call endTrace() and delete
that key; if no non-active recap exists, skip eviction (do not delete or call
endTrace) and avoid recreating a live recap in getOrCreateRecap(); update
getOrCreateRecap() to respect this and refrain from silently dropping spans when
a live recap was skipped.
| describe("Recap.withExporters — options", () => { | ||
| test("maxFiles option is applied to FileExporter", async () => { | ||
| const fileExporter = new FileExporter(tmpDir) | ||
| const tracer = Tracer.withExporters([fileExporter], { maxFiles: 2 }) | ||
| const tracer = Recap.withExporters([fileExporter], { maxFiles: 2 }) | ||
|
|
||
| // Write 4 traces | ||
| for (let i = 0; i < 4; i++) { | ||
| const t = Tracer.withExporters([new FileExporter(tmpDir, 2)]) | ||
| const t = Recap.withExporters([new FileExporter(tmpDir, 2)]) | ||
| t.startTrace(`s-${i}`, { prompt: `test-${i}` }) | ||
| await t.endTrace() | ||
| await new Promise((r) => setTimeout(r, 50)) |
There was a problem hiding this comment.
This test never hits the maxFiles code path under test.
Line 702's tracer is unused, and the loop writes via new FileExporter(tmpDir, 2) directly. The assertion will still pass if Recap.withExporters(..., { maxFiles: 2 }) is ignored entirely.
🩹 Suggested fix
- const fileExporter = new FileExporter(tmpDir)
- const tracer = Recap.withExporters([fileExporter], { maxFiles: 2 })
-
// Write 4 traces
for (let i = 0; i < 4; i++) {
- const t = Recap.withExporters([new FileExporter(tmpDir, 2)])
+ const t = Recap.withExporters([new FileExporter(tmpDir)], { maxFiles: 2 })
t.startTrace(`s-${i}`, { prompt: `test-${i}` })
await t.endTrace()
await new Promise((r) => setTimeout(r, 50))
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| describe("Recap.withExporters — options", () => { | |
| test("maxFiles option is applied to FileExporter", async () => { | |
| const fileExporter = new FileExporter(tmpDir) | |
| const tracer = Tracer.withExporters([fileExporter], { maxFiles: 2 }) | |
| const tracer = Recap.withExporters([fileExporter], { maxFiles: 2 }) | |
| // Write 4 traces | |
| for (let i = 0; i < 4; i++) { | |
| const t = Tracer.withExporters([new FileExporter(tmpDir, 2)]) | |
| const t = Recap.withExporters([new FileExporter(tmpDir, 2)]) | |
| t.startTrace(`s-${i}`, { prompt: `test-${i}` }) | |
| await t.endTrace() | |
| await new Promise((r) => setTimeout(r, 50)) | |
| describe("Recap.withExporters — options", () => { | |
| test("maxFiles option is applied to FileExporter", async () => { | |
| // Write 4 traces | |
| for (let i = 0; i < 4; i++) { | |
| const t = Recap.withExporters([new FileExporter(tmpDir)], { maxFiles: 2 }) | |
| t.startTrace(`s-${i}`, { prompt: `test-${i}` }) | |
| await t.endTrace() | |
| await new Promise((r) => setTimeout(r, 50)) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/opencode/test/altimate/tracing.test.ts` around lines 699 - 709, The
test never exercises the Recap.withExporters(..., { maxFiles: 2 }) path because
it creates new FileExporter instances inside the loop; replace the loop to use
the previously created tracer from Recap.withExporters (the tracer variable) to
startTrace/endTrace four times so the FileExporter instance managed by
Recap.withExporters enforces maxFiles; specifically, stop creating new
FileExporter(tmpDir, 2) inside the loop and call tracer.startTrace(...) and
await tracer.endTrace() for each iteration to trigger the maxFiles behavior on
the exporter returned by Recap.withExporters.
- Track loops by `(tool, inputHash)` not just tool name (#2) - Use "Failed after" narrative for error traces (#3) - Add keyboard accessibility to viewer tabs (role, tabindex, Enter/Space) (#4) - Use full command as dedup key, not `slice(0,60)` (#5) - Sort timeline events by time before rendering (#6) - Pass `tracesDir` to footer text in `listRecaps` (#7) - Increase `MAX_RECAPS` to 100, add eviction warning log (#8) - Resolve assistant `parentID` for recap enrichment (#9) - Remove unused `tracer` variable in test (#10) - Clarify `--no-trace` backward-compat flag in docs (#1) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/opencode/test/altimate/tracing.test.ts (1)
17-26: Consider usingtmpdirfixture for automatic cleanup.Per coding guidelines, test files should use the
tmpdirfunction fromfixture/fixture.tswithawait usingsyntax for automatic cleanup. The current manualbeforeEach/afterEachpattern works but doesn't follow the recommended pattern.Example refactor (optional)
import { tmpdir } from "../fixture/fixture" // In each test: test("example", async () => { await using tmp = await tmpdir() const exporter = new FileExporter(tmp.path) // ... })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/test/altimate/tracing.test.ts` around lines 17 - 26, Tests manually create and clean tmpDir using beforeEach/afterEach; switch to the tmpdir fixture for automatic cleanup by importing tmpdir from "../fixture/fixture" and replacing manual tmpDir setup/teardown with using the fixture in each test (await using tmp = await tmpdir()) and then reference tmp.path when constructing FileExporter or other consumers; remove the tmpDir variable, beforeEach, and afterEach blocks and update tests to use the tmpdir() fixture to ensure automatic cleanup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/opencode/src/altimate/observability/tracing.ts`:
- Around line 869-875: Update the TraceFile.summary.loops schema to include the
optional inputHash property so the mapped output from this.loopsDetected (used
in trace.summary.loops inside the class/method that maps loopsDetected) matches
the declared type; specifically add inputHash?: string to the loops entry type
in TraceFile.summary.loops (or alternatively remove the inputHash from the
mapping in the code that constructs trace.summary.loops if you prefer not to
expose it), ensuring the symbols TraceFile.summary.loops and this.loopsDetected
remain consistent.
---
Nitpick comments:
In `@packages/opencode/test/altimate/tracing.test.ts`:
- Around line 17-26: Tests manually create and clean tmpDir using
beforeEach/afterEach; switch to the tmpdir fixture for automatic cleanup by
importing tmpdir from "../fixture/fixture" and replacing manual tmpDir
setup/teardown with using the fixture in each test (await using tmp = await
tmpdir()) and then reference tmp.path when constructing FileExporter or other
consumers; remove the tmpDir variable, beforeEach, and afterEach blocks and
update tests to use the tmpdir() fixture to ensure automatic cleanup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 09c1df75-b3de-41f4-b996-cb256a8c4759
📒 Files selected for processing (7)
.github/meta/commit.txtdocs/docs/usage/cli.mdpackages/opencode/src/altimate/observability/tracing.tspackages/opencode/src/altimate/observability/viewer.tspackages/opencode/src/cli/cmd/trace.tspackages/opencode/src/cli/cmd/tui/worker.tspackages/opencode/test/altimate/tracing.test.ts
✅ Files skipped from review due to trivial changes (2)
- docs/docs/usage/cli.md
- .github/meta/commit.txt
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/opencode/src/cli/cmd/trace.ts
- Add Summary tab and full-page screenshots to docs - Update viewer section with 5-tab description - Detail what Summary tab shows: files changed, outcomes, timeline, cost - Add screenshot at top of recap.md for quick visual reference Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Move Recap from Configure > Observability to Use (peer to Commands, Skills) - Move Telemetry from Configure > Observability to Reference (internal analytics) - Remove the Observability section entirely Recap is a feature users interact with after sessions, not a config setting. Telemetry is internal product analytics, not user-facing observability. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/docs/configure/recap.md (1)
88-92:⚠️ Potential issue | 🟡 MinorPrefer the canonical disable flag in docs, then mention
--no-traceas alias.This section currently teaches only the backward-compatible flag. Please show the primary recap-era flag first (if supported), and keep
--no-traceas compatibility note to avoid reinforcing legacy usage.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/docs/configure/recap.md` around lines 88 - 92, Replace the current example that uses the legacy alias by showing the canonical recap-era disable flag as the primary example in the command (use the actual canonical flag name in place of <canonical-flag>) and then mention --no-trace as a backward-compatible alias; specifically update the example command that currently reads `altimate-code run --no-trace "quick question"` to use the canonical flag (`altimate-code run <canonical-flag> "quick question"`) and add a short parenthetical or note stating that `--no-trace` is supported as an alias for compatibility.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/docs/configure/recap.md`:
- Line 327: The doc currently uses the legacy API name startTrace() which leaks
old tracer terminology; update the sentence referencing startTrace() to use
neutral wording such as “when recap capture starts” or replace with the new API
name (if available) so the recap docs stay consistent; locate the sentence
containing startTrace() in the recap.md content and change it to the neutral
phrase or new function name wherever it appears.
---
Outside diff comments:
In `@docs/docs/configure/recap.md`:
- Around line 88-92: Replace the current example that uses the legacy alias by
showing the canonical recap-era disable flag as the primary example in the
command (use the actual canonical flag name in place of <canonical-flag>) and
then mention --no-trace as a backward-compatible alias; specifically update the
example command that currently reads `altimate-code run --no-trace "quick
question"` to use the canonical flag (`altimate-code run <canonical-flag> "quick
question"`) and add a short parenthetical or note stating that `--no-trace` is
supported as an alias for compatibility.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 879b954a-1368-4bff-8be8-5d645f64aa59
⛔ Files ignored due to path filters (2)
docs/docs/assets/images/recap/summary-full.pngis excluded by!**/*.pngdocs/docs/assets/images/recap/summary-tab.pngis excluded by!**/*.png
📒 Files selected for processing (2)
docs/docs/configure/recap.mddocs/mkdocs.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/mkdocs.yml
- Collapse Files Changed after 5 entries with "Show all N files" toggle - Rename "GENS" → "LLM Calls" in header cards - Hide Tokens card when cost is $0 (not actionable without cost context) - Hide Cost metric card when $0.00 (wasted space) - Add prominent error summary banner right after header metrics - Improved dbt outcome detection: catch [PASS], [ERROR], N of M, Compilation Error - Outcome detection rate improved from 18% → 33% across 100 real traces - Updated doc screenshots with cleaner samples Tested across 100 real production traces: 0 crashes, 0 JS errors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
$0.00 is a valid cost (Anthropic Max plan). Hiding it implies we don't support cost tracking. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/opencode/src/altimate/observability/viewer.ts (1)
1093-1099: Duplicated resolution detection logic.The resolution detection loop at lines 1093-1099 is nearly identical to lines 1043-1049. Consider extracting a helper function like
findResolutionSpan(errTime)to reduce duplication and ensure consistent behavior if the logic is later improved.♻️ Proposed refactor - extract helper function
// Add near the top of the Summary IIFE function findResolutionSpan(errTime) { for (var ri = 0; ri < nonSession.length; ri++) { var candidate = nonSession[ri]; if ((candidate.startTime || 0) > errTime && candidate.status === 'ok' && candidate.kind === 'tool') { return candidate; } } return null; } // Then use in both places: var resolution = findResolutionSpan(errTime); if (resolution) { errText += ' \\u2192 Resolved with ' + (resolution.name || 'next action'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opencode/src/altimate/observability/viewer.ts` around lines 1093 - 1099, Extract the duplicated resolution-detection loop that scans nonSession for a tool span after errTime into a helper function (e.g., findResolutionSpan(errTime)) and replace both loops with calls to that function; the helper should iterate over nonSession, return the matching candidate when (candidate.startTime||0) > errTime && candidate.status === 'ok' && candidate.kind === 'tool', or null if none, and the callers (the blocks that previously built the "Resolved" html/text) should use the returned candidate to produce the same output.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/opencode/src/altimate/observability/viewer.ts`:
- Around line 413-433: The selector built in gotoSpan when finding the row using
querySelector('.wf-row[data-span-id="' + spanId + '"]') can break for spanId
values with special CSS characters; update the lookup to escape spanId (use
CSS.escape(spanId)) before inserting it into the attribute selector (or
alternatively iterate document.querySelectorAll('.wf-row') and match
row.getAttribute('data-span-id') === spanId) so querySelector never receives a
malformed selector and the row is reliably found.
- Around line 1040-1051: The resolution scan assumes nonSession is
chronological; to fix, make a local sorted copy of nonSession (e.g., sort by
(candidate.startTime||0) then (candidate.endTime||0)) before the loop that
computes errTime and searches for the next successful span, and use that sorted
array in the for-loop that reads candidate, preserving the existing match
criteria (candidate.status === 'ok' && candidate.kind === 'tool') so
timelineEvents.push still uses errSp.startTime for the event time; this ensures
the search for a resolving span is based on temporal order rather than array
insertion order.
---
Nitpick comments:
In `@packages/opencode/src/altimate/observability/viewer.ts`:
- Around line 1093-1099: Extract the duplicated resolution-detection loop that
scans nonSession for a tool span after errTime into a helper function (e.g.,
findResolutionSpan(errTime)) and replace both loops with calls to that function;
the helper should iterate over nonSession, return the matching candidate when
(candidate.startTime||0) > errTime && candidate.status === 'ok' &&
candidate.kind === 'tool', or null if none, and the callers (the blocks that
previously built the "Resolved" html/text) should use the returned candidate to
produce the same output.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: bfef5f7b-a721-4c2e-a4a1-8293500091f3
⛔ Files ignored due to path filters (1)
docs/docs/assets/images/recap/summary-full.pngis excluded by!**/*.png
📒 Files selected for processing (2)
.github/meta/commit.txtpackages/opencode/src/altimate/observability/viewer.ts
✅ Files skipped from review due to trivial changes (1)
- .github/meta/commit.txt
…neage tools 500-trace analysis revealed: - Schema tasks: 0% outcome visibility → 100% - Validation tasks: 0% outcome visibility → 100% - SQL tasks: 55% outcome visibility → 100% Added outcome extraction for: - schema_inspect, lineage_check, altimate_core_validate results - SQL error messages (not just row counts) - Improved empty session display (shows prompt if available) Tested across 500 diverse synthetic traces (SQL, Airflow, Dagster, Python, schema, validation, migration, connectors) + 100 real traces. 0 crashes, 0 JS errors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add `inputHash` to `TraceFile.summary.loops` schema type (#11) - Replace `startTrace()` API name with plain language in docs (#12) - Use `CSS.escape()` for spanId in querySelector to handle special chars (#13) - Sort spans by startTime before searching for error resolution (#14) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/docs/configure/recap.md (1)
285-299:⚠️ Potential issue | 🟡 MinorUpdate the recap JSON example to include new summary fields.
The
summaryexample omitstopTools,loops, andnarrative, even though this page documents Post-Session Summary and the implementation writes those fields. Please add them to keep the file format section accurate.Proposed doc patch
"summary": { "totalTokens": 2150, "totalCost": 0.005, "totalToolCalls": 1, "totalGenerations": 1, "duration": 45200, "status": "completed", + "topTools": [ + { "name": "sql_execute", "count": 1, "totalDuration": 2000 } + ], + "loops": [], + "narrative": "Completed in 45.2s. Made 1 LLM calls using 1 tools (sql_execute). Total cost: $0.0050.", "tokens": { "input": 1500, "output": 300, "reasoning": 100, "cacheRead": 200, "cacheWrite": 50 } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/docs/configure/recap.md` around lines 285 - 299, The example JSON's "summary" object is missing fields that the implementation and docs expect — add "topTools" (array of tool usage objects or names), "loops" (object or array describing loop counts/details), and "narrative" (string or structured narrative) to the "summary" example in the recap.md sample so the Post-Session Summary format matches the implementation; update the "summary" block to include keys "topTools", "loops", and "narrative" with representative example values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@docs/docs/configure/recap.md`:
- Around line 285-299: The example JSON's "summary" object is missing fields
that the implementation and docs expect — add "topTools" (array of tool usage
objects or names), "loops" (object or array describing loop counts/details), and
"narrative" (string or structured narrative) to the "summary" example in the
recap.md sample so the Post-Session Summary format matches the implementation;
update the "summary" block to include keys "topTools", "loops", and "narrative"
with representative example values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 91d9a80f-351b-439c-8c47-7fadcafc06de
📒 Files selected for processing (3)
docs/docs/configure/recap.mdpackages/opencode/src/altimate/observability/tracing.tspackages/opencode/src/altimate/observability/viewer.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/opencode/src/altimate/observability/tracing.ts
- packages/opencode/src/altimate/observability/viewer.ts
- Sort spans once before error resolution loop instead of per-error (perf) - Narrative omits "Made 0 LLM calls" for tool-only sessions (UX) - Updated tests to match new narrative format Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…am-shared files Wrap renamed code (Tracer→Recap, trace→recap) with markers so the Marker Guard CI check passes. The diff-based checker uses -U5 context windows per hunk — markers must be close enough to added lines to appear within each hunk's context. Files fixed: - `trace.ts` — handler body, option descriptions, viewer message, compat alias - `app.tsx` — recapViewerServer return, openRecapInBrowser function - `dialog-trace-list.tsx` — error title, Recaps title, compat alias - `worker.ts` — getOrCreateRecap, part events, session title/finalization - `index.ts` — .command(RecapCommand) registration Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Marker Guard CI was failing — 5 upstream-shared files had custom code (recap rename) without altimate_change markers. Fixed: trace.ts, app.tsx, dialog-trace-list.tsx, worker.ts, index.ts Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Pre-existing type issues from main: mock missing `context`/`rule` fields and readFile return type mismatch. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
What does this PR do?
Renames the "tracer" feature to "recap" — a session observability tool that helps users understand what an agent did, what went right/wrong, and how much it cost. Adds loop detection, post-session summary, tool-agnostic outcome extraction, and a redesigned HTML viewer with virality features.
Key changes:
Tracer→Recapclass (backward-compat aliases preserved)altimate-code recapcommand (hiddentracealias)topToolsandloopsType of change
Issue for this PR
Closes #380
How did you verify your code works?
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Documentation