feat(tui): add workflow navigator and call inspector - #158
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThe workflow view now loads observations and derives statuses, usage, and timing. The TUI now supports hierarchical navigation and call inspection with prompts, responses, activity details, and keyboard focus controls. Tests cover observation mapping, usage aggregation, and inspector rendering. ChangesWorkflow inspection
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Greptile SummaryThe PR expands the workflow TUI with hierarchical run, phase, and call navigation plus a detailed call inspector.
Confidence Score: 2/5This PR should not merge until terminal control sequences are escaped and displayed unphased calls can be selected in the inspector. The new inspector exposes raw provider-controlled content to terminal interpretation, while the navigation model makes valid calls without a phase visible but impossible to inspect. Files Needing Attention: src/workflow-tui.ts
|
| Filename | Overview |
|---|---|
| src/workflow-tui.ts | Adds navigator and inspector rendering, but unphased calls cannot be selected and unescaped call content reaches the terminal. |
| src/workflow-view.ts | Enriches workflow views with observations, phase metadata, timing, and aggregated token usage. |
| src/workflow-tui.test.ts | Covers the phased-call inspector path but does not exercise unphased-call navigation or terminal control characters. |
| src/workflow-view.test.ts | Verifies observation mapping and run-level token aggregation for the enriched view. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
Store[(Workflow store)] --> View[Build workflow project view]
View --> Runs[Run selection]
Runs --> Phases[Phase navigator]
Phases --> Calls[Call navigator]
Calls --> Inspector[Call inspector]
Inspector --> Terminal[Terminal output]
View --> Unphased[Unphased calls]
Unphased -. displayed but not selectable .-> Calls
Reviews (1): Last reviewed commit: "feat(tui): add workflow navigator and ca..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| lines.push(truncate(`${statusGlyph(call.status)} ${target} · ${usageLabel(call.finalUsage ?? call.usage)} · ${durationLabel(call.startedAt, call.completedAt)}`, width)); | ||
| lines.push(truncate(`phase ${call.phase ?? "unphased"} · ${call.isolation}${call.profileName ? ` · profile ${call.profileName}` : ""}${call.providerSessionId ? ` · session ${call.providerSessionId}` : ""}`, width)); | ||
| lines.push(style("\nPrompt", "heading", ansi)); | ||
| lines.push(truncate(call.prompt, width)); |
There was a problem hiding this comment.
When a stored prompt, provider observation, error, or response contains ANSI, CSI, or OSC sequences, the inspector passes that text through truncate or style without escaping it, causing the terminal to interpret commands that can rewrite the display, alter terminal metadata, or create deceptive links.
How this was verified: Each new text source reaches terminal output through helpers that truncate or style strings but do not remove embedded control sequences.
| } else if (run.unphasedCalls.length > 0) { | ||
| lines.push(style("\nOther calls", "heading", ansi)); | ||
| for (const call of run.unphasedCalls) lines.push(truncate(formatCall(call, " "), width)); |
There was a problem hiding this comment.
When an agent call has no phase, the navigator displays it under “Other calls,” but selection and inspector lookup only index run.phases[phaseIndex].calls, causing the visible call to remain unselectable and the inspector to report “no call selected.”
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/workflow-tui.ts (2)
182-196: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThe run list does not scroll, so the selected run can leave the visible window.
Line 184 always slices from index 0.
maxRunRowsis at most 7.moveSelectionclampsrunIndextoproject.runs.length - 1, andrunWorkflowTuiloads up to 50 runs.When the user presses down past the seventh run, the
›marker leaves the rendered list. The list still shows runs 1 to 7. The user cannot see which run is selected, although the pane below switches to the new run.Scroll the window so that it always contains
selection.runIndex.🐛 Proposed fix to keep the selected run visible
const maxRunRows = Math.max(3, Math.min(7, Math.floor(rows / 5))); lines.push(style("Workflows", "heading", ansi)); - for (const [index, run] of project.runs.slice(0, maxRunRows).entries()) { - const marker = index === selection.runIndex ? "›" : " "; + const firstRunRow = Math.min( + Math.max(0, selection.runIndex - Math.floor(maxRunRows / 2)), + Math.max(0, project.runs.length - maxRunRows), + ); + for (const [offset, run] of project.runs.slice(firstRunRow, firstRunRow + maxRunRows).entries()) { + const index = firstRunRow + offset; + const marker = index === selection.runIndex ? "›" : " "; const phase = run.currentPhase ? ` · ${run.currentPhase}` : ""; lines.push( truncate( `${marker} ${statusGlyph(run.status)} ${run.name}${phase} · ${callSummary(run)} · ${durationLabel(run.startedAt, run.completedAt)}`, width, ), ); } - if (project.runs.length > maxRunRows) { - lines.push(style(` +${project.runs.length - maxRunRows} more`, "muted", ansi)); + const hidden = project.runs.length - maxRunRows; + if (hidden > 0) { + lines.push(style(` ${hidden} more`, "muted", ansi)); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/workflow-tui.ts` around lines 182 - 196, Update the run-list rendering around maxRunRows and project.runs.slice so the visible window is offset to include selection.runIndex, keeping the selected run visible when navigating beyond the initial rows. Preserve the existing row limit, marker rendering, and “more” indicator while adjusting the displayed range and remaining-count calculation to match the scrolled window.
58-76: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
rendercan throw from a timer callback and leave the terminal unusable.Line 63 calls
reconcileSelectionon every refresh. WhenrequestedRunIdis set, line 337 callsfindInitialSelection, and line 378 throws if the run is no longer present inproject.runs.The run can disappear while the TUI is open.
loadWorkflowProjectViewapplieslimit: 50, so the requested run drops out of the list once it falls outside the newest 50 runs. A pruned or deleted run record has the same effect.
renderruns fromsetInterval, from theresizehandler, and fromonKeypress. Thetryblock has only afinallythat resetsrendering. An exception from a timer callback is therefore uncaught. The process exits without runningfinish(). Raw mode stays enabled, the alternate screen buffer stays active, the cursor stays hidden, andstore.close()never runs.Keep the throwing lookup on the initial resolution only. Make refresh-time reconciliation non-throwing, and guard
render.🐛 Proposed fix for refresh-time reconciliation
function reconcileSelection( project: WorkflowProjectView, selection: WorkflowTuiSelection, requestedRunId: string | undefined, ): WorkflowTuiSelection { - const runIndex = requestedRunId - ? findInitialSelection(project, requestedRunId) - : Math.min(Math.max(0, selection.runIndex), Math.max(0, project.runs.length - 1)); + const requestedIndex = requestedRunId + ? project.runs.findIndex((run) => run.id === requestedRunId) + : -1; + const runIndex = requestedIndex >= 0 + ? requestedIndex + : Math.min(Math.max(0, selection.runIndex), Math.max(0, project.runs.length - 1));Guard the render loop as well:
try { project = load(); selection = reconcileSelection(project, selection, requestedRunId); process.stdout.write( `\u001b[H\u001b[2J${renderWorkflowTui( project, selection.runIndex, process.stdout.columns || 100, process.stdout.rows || 40, { ansi: true, selection }, )}`, ); + } catch { + // Keep the previous frame; the next refresh retries. } finally { rendering = false; }Also applies to: 336-338
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/workflow-tui.ts` around lines 58 - 76, Update render and the refresh-time selection reconciliation around reconcileSelection and findInitialSelection so a missing requested run is handled non-throwingly after the initial selection resolution, while preserving the existing throwing lookup during initial resolution. Guard the render callback against reconciliation or rendering failures so interval, resize, and keypress-triggered renders cannot escape uncaught; always reset rendering and ensure the existing finish/cleanup path runs when rendering fails.
🧹 Nitpick comments (7)
src/workflow-tui.ts (4)
44-46: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
selectedIndexandoptions.selectionexpress the same state.
renderWorkflowTuitakesselectedIndexpositionally andoptions.selectionoptionally. When both arrive, line 168 ignoresselectedIndex. Both call sites in this file passselection.runIndexandselectiontogether, so they pass the same value twice.Consider deprecating the positional parameter and reading the run index from
options.selectiononly, withcreateSelection(project, 0)as the default. The change touchessrc/workflow-tui.test.ts, which calls the function positionally.Also applies to: 159-169
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/workflow-tui.ts` around lines 44 - 46, Update renderWorkflowTui to remove or deprecate the redundant positional selectedIndex parameter and use options.selection as the single source of the run index, defaulting it with createSelection(project, 0). Adjust both call sites in workflow-tui.ts and the positional invocations in workflow-tui.test.ts so they use the revised API without passing selection state twice.
322-329: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
createSelectionignoresproject, and the two lookup helpers clamp differently.
createSelectionacceptsprojectbut never reads it. Line 168 uses it as the default path when a caller supplies no selection, so an out-of-rangeselectedIndexpasses through unclamped. Line 199 guards the result, so there is no current defect.
selectedPhaseclamps its index.selectedCallindexescalls[callIndex]directly. Both callers handleundefined, so the behaviour is safe, but the asymmetry is easy to misread.Either clamp
runIndexinsidecreateSelectionand use theprojectargument, or drop the parameter. Apply the same clamping rule in both lookup helpers.Also applies to: 383-389
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/workflow-tui.ts` around lines 322 - 329, Update createSelection to use project when normalizing the supplied runIndex, clamping it to the valid run range; retain the existing default-selection behavior at its caller. Apply the same index-clamping rule used by selectedPhase to selectedCall before accessing the call collection, while preserving undefined handling for empty or unavailable data.
122-138: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace the focus ternary chains with named helpers, and use the declared
shiftkey.Lines 123-129 and 132-138 each end with a branch that assigns the current focus to itself. A
nextFocushelper already exists for the forward direction. Two small helpers make the intent explicit and remove the self-assignments.Line 97 also declares
shifton the key type, but no branch reads it. Shift-tab is the conventional reverse-cycle key in a focus ring.♻️ Proposed refactor for focus movement
if (key.name === "tab") { - selection.focus = nextFocus(selection.focus); + selection.focus = key.shift ? previousFocus(selection.focus) : nextFocus(selection.focus); render(); return; }} else if (key.name === "right" || key.name === "l" || key.name === "return") { - selection.focus = selection.focus === "runs" - ? "phases" - : selection.focus === "phases" - ? "calls" - : selection.focus === "calls" - ? "inspector" - : "inspector"; + selection.focus = drillIn(selection.focus); render(); } else if (key.name === "left" || key.name === "h") { - selection.focus = selection.focus === "inspector" - ? "calls" - : selection.focus === "calls" - ? "phases" - : selection.focus === "phases" - ? "runs" - : "runs"; + selection.focus = drillOut(selection.focus); render(); }Add the helpers next to
nextFocus:const FOCUS_ORDER: readonly TuiFocus[] = ["runs", "phases", "calls", "inspector"]; function drillIn(focus: TuiFocus): TuiFocus { return FOCUS_ORDER[Math.min(FOCUS_ORDER.indexOf(focus) + 1, FOCUS_ORDER.length - 1)] ?? focus; } function drillOut(focus: TuiFocus): TuiFocus { return FOCUS_ORDER[Math.max(FOCUS_ORDER.indexOf(focus) - 1, 0)] ?? focus; } function previousFocus(focus: TuiFocus): TuiFocus { const index = FOCUS_ORDER.indexOf(focus); return FOCUS_ORDER[(index - 1 + FOCUS_ORDER.length) % FOCUS_ORDER.length] ?? focus; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/workflow-tui.ts` around lines 122 - 138, Replace the forward and reverse focus ternary chains in the key handler with the named focus helpers alongside nextFocus: use drillIn for right/l/return and drillOut for left/h, preserving the existing boundary behavior without self-assignments. Also handle the declared key.shift state so Shift-Tab invokes previousFocus and cycles focus backward through FOCUS_ORDER.
257-257: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the layout row budgets into named constants.
Line 257 gates run activity on
rows > 18. Line 290 reservesrows - 17for activity entries. Line 299 reservesrows - lines.length - 3for response lines. The constants 18, 17, and 3 encode the height of the surrounding chrome. They will drift as sections are added.Name them, for example
MIN_ROWS_FOR_RUN_ACTIVITYandINSPECTOR_CHROME_ROWS, and derive them from the sections they account for.Also applies to: 290-290, 299-299
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/workflow-tui.ts` at line 257, Replace the hard-coded layout budgets in the run activity and response rendering logic with named constants, including the checks and calculations around run.recentActivity, rows - 17, and rows - lines.length - 3. Define the constants from the surrounding inspector sections they represent, such as MIN_ROWS_FOR_RUN_ACTIVITY and INSPECTOR_CHROME_ROWS, and reuse them consistently.src/workflow-view.ts (1)
197-211: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winElapsed time is derived in two layers with different semantics.
buildWorkflowRunViewstoresdurationMson phases and calls, andsrc/workflow-tui.tsignores those fields and reparses the timestamps. The view value freezes at build time; the renderer value recomputes at render time, so the two drift apart for anything still running. Choose one owner.
src/workflow-view.ts#L197-L211: if the renderer owns elapsed time, removedurationMsfromWorkflowPhaseViewandWorkflowCallViewand delete theelapsedMscalls; if the view owns it, document that the value is a build-time snapshot.src/workflow-tui.ts#L419-L430: if the view owns elapsed time, changedurationLabelto format a supplieddurationMsinstead of parsingstartedAtandcompletedAtagain, and update the call sites inrenderNavigator,renderCallInspector, andformatCall.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/workflow-view.ts` around lines 197 - 211, Make src/workflow-tui.ts lines 419-430 the sole owner of elapsed-time calculation: remove durationMs and the elapsedMs calls from buildWorkflowRunView’s phase construction in src/workflow-view.ts lines 197-211, and remove durationMs from WorkflowPhaseView and WorkflowCallView. Keep durationLabel’s timestamp-based calculation and its callers in renderNavigator, renderCallInspector, and formatCall unchanged.src/workflow-tui.test.ts (1)
80-85: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd navigator-focus and selection-reconciliation cases.
Lines 80-85 cover the inspector only. These changed paths have no test:
renderWorkflowTuiwithfocus: "phases"andfocus: "calls", which drive the›markers inrenderNavigator.- A project with more runs than
maxRunRows, withrunIndexbeyond the visible window.- A call whose
promptorresponseTextcontains newlines, to pin the rendered row count.reconcileSelectionwhen the requested run id is absent from the refreshed project.
reconcileSelectionandmoveSelectionare module-private today. Exporting them, or testing them through a small internal entry point, would cover the refresh path.The tests also build a synthetic
WorkflowProjectView. They do not exerciserunWorkflowTui, the store, or the non-TTY branch, so only a narrower proxy of the user-consumption path is verified.Do you want me to generate these test cases?
As per path instructions: "Verify the actual user-consumption path, including packaged npm/npx usage, real MCP hosts, restart requirements, checkout/worktree modes, supported platforms, tool surfaces, widgets, and rendered artifacts; clearly state when only a narrower proxy was verified."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/workflow-tui.test.ts` around lines 80 - 85, Expand workflow TUI coverage beyond the inspector case: test renderWorkflowTui with phases and calls focus, run-window reconciliation, multiline prompt/response row counts, and reconcileSelection when the requested run is absent. Expose reconcileSelection and moveSelection through the smallest testable internal entry point, or test them via runWorkflowTui’s refresh path. Add coverage through the actual runWorkflowTui consumption path where feasible, and explicitly identify any remaining synthetic-project proxy coverage.Source: Path instructions
src/workflow-view.test.ts (1)
132-133: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtend coverage for the new derivation logic.
Line 133 asserts
view.usageagainst a single call that carries usage. The assertion passes even ifsumUsagereturned the first entry instead of the sum. Add a second call with usage to make the aggregation meaningful.These changed paths have no assertions yet:
phaseStatusinference, including a terminal run whose phase has no calls.- Phase-level
usage,startedAt,completedAt, anddurationMs.- The observation-aware
version, which is the field that signals change to consumers.elapsedMswith an unparsable timestamp.The tests also call
buildWorkflowRunViewdirectly. They do not exerciseloadWorkflowProjectView, so theobservationLimitoption and the per-call observation lookup remain unverified against a real store.Do you want me to generate these test cases?
As per path instructions: "Verify the actual user-consumption path, including packaged npm/npx usage, real MCP hosts, restart requirements, checkout/worktree modes, supported platforms, tool surfaces, widgets, and rendered artifacts; clearly state when only a narrower proxy was verified."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/workflow-view.test.ts` around lines 132 - 133, Extend workflow-view tests to add a second usage-bearing call so sumUsage aggregation is verified, and cover phaseStatus inference including terminal phases without calls, phase-level usage/timestamps/durationMs, observation-derived version, and unparsable timestamps producing the expected elapsedMs behavior. Add coverage through loadWorkflowProjectView rather than only direct buildWorkflowRunView calls, using a real store fixture to verify observationLimit and per-call observation lookup. Keep the tests focused on the actual consumer path; explicitly note any narrower proxy coverage where packaged or host-level usage is not exercised.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/workflow-tui.ts`:
- Around line 229-251: Update the navigator rendering flow around the phase and
call loops to allocate the available row budget before adding content, rather
than relying on fitRows afterward. Reserve space for the selected phase/call
context, activity preview, run activity, and footer, and prioritize rows
containing the selected phase and selected call while truncating unselected
phases/calls within the remaining budget.
- Around line 284-285: Update the rendering flow around the prompt, error,
message, and detail fields to split newline-containing values into separate
entries before truncation and insertion into lines, matching the existing
responseText handling near line 299. Ensure each rendered terminal row is
counted individually by fitRows so the frame remains within the terminal height.
In `@src/workflow-view.ts`:
- Around line 328-341: Update phaseStatus so an inferred "running" phase is
mapped to the terminal run outcome when isTerminalRun(runStatus) is true,
instead of falling through to return inferred. Preserve the existing running
behavior for non-terminal runs and the current precedence for failed, cancelled,
and completed call states.
- Around line 139-153: Update loadWorkflowProjectView so listAgentObservations
is queried only for the run at selection.runIndex when rendering the TUI, while
preserving empty observations for other runs so their WorkflowRunView
construction remains valid. Use the existing selection and buildWorkflowRunView
flow; do not load observations for every listed run unless introducing a
run-scoped or batch equivalent.
- Around line 347-360: Update sumUsage so aggregated totalTokens includes
entries that omit totalTokens: sum inputTokens and outputTokens, then derive the
missing contribution for each such usage before setting result.totalTokens.
Preserve explicitly reported totals while ensuring mixed providers produce a
complete aggregate consumed by usageLabel.
- Around line 218-220: Update the latestObservedAt computation around
latestObservation to derive the maximum createdAt across every call’s
observations rather than using call order, and include the total observation
count in the resulting value so additions within capped windows change
run.version.
---
Outside diff comments:
In `@src/workflow-tui.ts`:
- Around line 182-196: Update the run-list rendering around maxRunRows and
project.runs.slice so the visible window is offset to include
selection.runIndex, keeping the selected run visible when navigating beyond the
initial rows. Preserve the existing row limit, marker rendering, and “more”
indicator while adjusting the displayed range and remaining-count calculation to
match the scrolled window.
- Around line 58-76: Update render and the refresh-time selection reconciliation
around reconcileSelection and findInitialSelection so a missing requested run is
handled non-throwingly after the initial selection resolution, while preserving
the existing throwing lookup during initial resolution. Guard the render
callback against reconciliation or rendering failures so interval, resize, and
keypress-triggered renders cannot escape uncaught; always reset rendering and
ensure the existing finish/cleanup path runs when rendering fails.
---
Nitpick comments:
In `@src/workflow-tui.test.ts`:
- Around line 80-85: Expand workflow TUI coverage beyond the inspector case:
test renderWorkflowTui with phases and calls focus, run-window reconciliation,
multiline prompt/response row counts, and reconcileSelection when the requested
run is absent. Expose reconcileSelection and moveSelection through the smallest
testable internal entry point, or test them via runWorkflowTui’s refresh path.
Add coverage through the actual runWorkflowTui consumption path where feasible,
and explicitly identify any remaining synthetic-project proxy coverage.
In `@src/workflow-tui.ts`:
- Around line 44-46: Update renderWorkflowTui to remove or deprecate the
redundant positional selectedIndex parameter and use options.selection as the
single source of the run index, defaulting it with createSelection(project, 0).
Adjust both call sites in workflow-tui.ts and the positional invocations in
workflow-tui.test.ts so they use the revised API without passing selection state
twice.
- Around line 322-329: Update createSelection to use project when normalizing
the supplied runIndex, clamping it to the valid run range; retain the existing
default-selection behavior at its caller. Apply the same index-clamping rule
used by selectedPhase to selectedCall before accessing the call collection,
while preserving undefined handling for empty or unavailable data.
- Around line 122-138: Replace the forward and reverse focus ternary chains in
the key handler with the named focus helpers alongside nextFocus: use drillIn
for right/l/return and drillOut for left/h, preserving the existing boundary
behavior without self-assignments. Also handle the declared key.shift state so
Shift-Tab invokes previousFocus and cycles focus backward through FOCUS_ORDER.
- Line 257: Replace the hard-coded layout budgets in the run activity and
response rendering logic with named constants, including the checks and
calculations around run.recentActivity, rows - 17, and rows - lines.length - 3.
Define the constants from the surrounding inspector sections they represent,
such as MIN_ROWS_FOR_RUN_ACTIVITY and INSPECTOR_CHROME_ROWS, and reuse them
consistently.
In `@src/workflow-view.test.ts`:
- Around line 132-133: Extend workflow-view tests to add a second usage-bearing
call so sumUsage aggregation is verified, and cover phaseStatus inference
including terminal phases without calls, phase-level
usage/timestamps/durationMs, observation-derived version, and unparsable
timestamps producing the expected elapsedMs behavior. Add coverage through
loadWorkflowProjectView rather than only direct buildWorkflowRunView calls,
using a real store fixture to verify observationLimit and per-call observation
lookup. Keep the tests focused on the actual consumer path; explicitly note any
narrower proxy coverage where packaged or host-level usage is not exercised.
In `@src/workflow-view.ts`:
- Around line 197-211: Make src/workflow-tui.ts lines 419-430 the sole owner of
elapsed-time calculation: remove durationMs and the elapsedMs calls from
buildWorkflowRunView’s phase construction in src/workflow-view.ts lines 197-211,
and remove durationMs from WorkflowPhaseView and WorkflowCallView. Keep
durationLabel’s timestamp-based calculation and its callers in renderNavigator,
renderCallInspector, and formatCall unchanged.
🪄 Autofix
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 Plus
Run ID: f1d04e80-20e6-44e9-a9df-7501ee910457
📒 Files selected for processing (4)
src/workflow-tui.test.tssrc/workflow-tui.tssrc/workflow-view.test.tssrc/workflow-view.ts
| if (run.phases.length === 0) { | ||
| lines.push(style(" No phase markers yet.", "muted", ansi)); | ||
| } else { | ||
| for (const [index, item] of run.phases.entries()) { | ||
| const marker = index === selection.phaseIndex ? "›" : " "; | ||
| lines.push( | ||
| truncate( | ||
| `${marker} ${phaseGlyph(item.status)} ${item.title} · ${item.calls.length} calls · ${usageLabel(item.usage)} · ${durationLabel(item.startedAt, item.completedAt)}`, | ||
| width, | ||
| ), | ||
| ); | ||
| } | ||
| } | ||
| if (run.unphasedCalls.length > 0 && renderedCalls < phaseBudget) { | ||
| lines.push(style("\nOther calls", "heading", ansi)); | ||
| for (const call of run.unphasedCalls) { | ||
| if (renderedCalls >= phaseBudget) break; | ||
| lines.push(truncate(formatCall(call), width)); | ||
| renderedCalls += 1; | ||
|
|
||
| if (phase) { | ||
| lines.push(style(`\n${phase.title} · ${phase.status}`, "heading", ansi)); | ||
| const calls = phase.calls; | ||
| if (calls.length === 0) lines.push(style(" No agent calls in this phase.", "muted", ansi)); | ||
| for (const [index, call] of calls.entries()) { | ||
| const marker = index === selection.callIndex && (selection.focus === "calls" || selection.focus === "phases") ? "›" : " "; | ||
| lines.push(truncate(formatCall(call, marker), width)); | ||
| } | ||
| renderActivityPreview(lines, calls, selection.callIndex, width, ansi); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect fitRows to determine whether it keeps leading or trailing lines.
rg -nP --type=ts -B2 -A15 'function\s+fitRows\s*\(' src/workflow-tui.tsRepository: Waishnav/devspace
Length of output: 733
🏁 Script executed:
#!/bin/bash
# Inspect the workflow TUI rendering boundaries and row-budgeted windowing.
sed -n '1,330p' src/workflow-tui.ts | cat -nRepository: Waishnav/devspace
Length of output: 14114
Budget the navigator phases and calls before rendering them.
fitRows keeps only the leading lines, so unbounded phase/call rendering can push the call list, selected call marker, activity preview, run activity, and footer out of the frame. Keep the selected phase and selected call visible while respecting the remaining row budget instead of slicing after all lines are added.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/workflow-tui.ts` around lines 229 - 251, Update the navigator rendering
flow around the phase and call loops to allocate the available row budget before
adding content, rather than relying on fitRows afterward. Reserve space for the
selected phase/call context, activity preview, run activity, and footer, and
prioritize rows containing the selected phase and selected call while truncating
unselected phases/calls within the remaining budget.
| lines.push(style("\nPrompt", "heading", ansi)); | ||
| lines.push(truncate(call.prompt, width)); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
A multi-line prompt breaks the frame height.
Line 285 pushes call.prompt as a single array entry. truncate compares only value.length against width. It does not split on newlines.
fitRows(lines, rows) counts array entries, not rendered terminal rows. A prompt that contains newlines therefore occupies several rows while it costs one entry in the budget. The frame overflows the terminal height and scrolls. Line 65 redraws with \u001b[H\u001b[2J on the assumption that the frame fits.
Split the prompt the same way that line 299 splits responseText.
🐛 Proposed fix for multi-line prompts
lines.push(style("\nPrompt", "heading", ansi));
- lines.push(truncate(call.prompt, width));
+ for (const line of call.prompt.split(/\r?\n/).slice(0, PROMPT_PREVIEW_ROWS)) {
+ lines.push(truncate(line, width));
+ }Apply the same treatment wherever a stored field can contain newlines, including call.error at line 296 and entry.message and entry.detail at line 293.
📝 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.
| lines.push(style("\nPrompt", "heading", ansi)); | |
| lines.push(truncate(call.prompt, width)); | |
| lines.push(style("\nPrompt", "heading", ansi)); | |
| for (const line of call.prompt.split(/\r?\n/).slice(0, PROMPT_PREVIEW_ROWS)) { | |
| lines.push(truncate(line, width)); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/workflow-tui.ts` around lines 284 - 285, Update the rendering flow around
the prompt, error, message, and detail fields to split newline-containing values
into separate entries before truncation and insertion into lines, matching the
existing responseText handling near line 299. Ensure each rendered terminal row
is counted individually by fitRows so the frame remains within the terminal
height.
| .map((run) => { | ||
| const calls = store.listAgentCalls(run.id); | ||
| const observations = new Map( | ||
| calls.map((call) => [ | ||
| call.callIndex, | ||
| store.listAgentObservations(run.id, call.callIndex, options.observationLimit ?? 100), | ||
| ]), | ||
| ); | ||
| return buildWorkflowRunView( | ||
| run, | ||
| store.listAgentCalls(run.id), | ||
| calls, | ||
| store.listEvents(run.id, options.eventLimit ?? 100), | ||
| ), | ||
| ); | ||
| observations, | ||
| ); | ||
| }); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the store observation APIs and their call sites.
rg -nP --type=ts -C6 '\blistAgentObservations\s*[(:<]'
rg -nP --type=ts -C3 '\bloadWorkflowProjectView\s*\('Repository: Waishnav/devspace
Length of output: 155
🏁 Script executed:
#!/bin/bash
set -u
echo "Tracked files matching workflow-view.ts:"
git ls-files | grep -F 'workflow-view.ts' || true
echo
echo "Tracked files matching workflow:"
git ls-files | grep -E 'workflow|Workflow' | head -200 || true
echo
echo "Search observation/load symbols (case-insensitive):"
rg -n -i -C 4 'listAgentObservations|listAgentCalls|loadWorkflowProjectView|renderWorkflowTui|observationLimit|eventLimit' .Repository: Waishnav/devspace
Length of output: 14412
🏁 Script executed:
#!/bin/bash
set -u
echo "workflow-tui refresh/render sections:"
sed -n '50,85p' src/workflow-tui.ts
sed -n '159,230p' src/workflow-tui.ts
sed -n '1,100p' src/workflow-summary.ts
echo
echo "workflow-view and store observation API sections:"
sed -n '123,165p' src/workflow-view.ts
sed -n '940,990p' src/workflow-store.ts
sed -n '1028,1072p' src/workflow-store.ts
echo
echo "Search for observation-run batch or selection options:"
rg -n 'observation(Run|Selection|Id|RunIds)|listAgentObservat|WorkflowProjectView|buildWorkflowRunView|renderObservations|renderCallInspector' src/workflow-view.ts src/workflow-tui.ts src/workflow-store.tsRepository: Waishnav/devspace
Length of output: 11048
Load TUI observations only for the selected run.
loadWorkflowProjectView reads observations for every call of every listed run per TUI refresh. The TUI passes limit: 50 and observationLimit: 100, so each refresh runs one listAgentObservations query per call across all selected runs. renderWorkflowTui only passes one selected WorkflowRunView to renderNavigator/renderCallInspector, discarding the remaining observation rows. Load observations only for selection.runIndex, or add a run-scoped/batch observation load path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/workflow-view.ts` around lines 139 - 153, Update loadWorkflowProjectView
so listAgentObservations is queried only for the run at selection.runIndex when
rendering the TUI, while preserving empty observations for other runs so their
WorkflowRunView construction remains valid. Use the existing selection and
buildWorkflowRunView flow; do not load observations for every listed run unless
introducing a run-scoped or batch equivalent.
| const latestObservation = callViews | ||
| .flatMap((call) => call.observations) | ||
| .at(-1)?.createdAt; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find consumers that compare or cache on the view version, and the observation query semantics.
rg -nP --type=ts -C4 '\.version\b' -g '!**/*.test.ts'
rg -nP --type=ts -C10 'listAgentObservations'Repository: Waishnav/devspace
Length of output: 155
🏁 Script executed:
#!/bin/bash
set -u
echo "tracked TypeScript files count:"
git ls-files '*.ts' '*.tsx' | wc -l
echo "candidate filenames:"
git ls-files | grep -E '(^|/)workflow-view\.(ts|tsx)$|workflow-view' || true
echo "search all tracked files for WorkflowProjectView, version, listAgentObservations:"
rg -n -C 4 'WorkflowProjectView|\.version\b|latestObservation|maxObservation|listAgentObservations' . || true
echo "file list around workflow-view if present:"
fd -a 'workflow-view\.(ts|tsx)$|workflow-view' . 2>/dev/null | sed 's#^\./##' || trueRepository: Waishnav/devspace
Length of output: 14484
🏁 Script executed:
#!/bin/bash
set -u
echo "workflow-view relevant sections:"
sed -n '130,245p' src/workflow-view.ts
echo "store observation implementation:"
sed -n '935,1015p' src/workflow-store.ts
echo "observe consumers of project.version from imports/usages:"
rg -n -C 3 'WorkflowProjectView|loadWorkflowProjectView|\.version[[:space:]]*:|project\.version|run\.version|version[[:space:]]*=' src --glob '*.ts' || true
echo "workflow-view tests around version:"
sed -n '1,260p' src/workflow-view.test.tsRepository: Waishnav/devspace
Length of output: 15127
🌐 Web query:
CodeRabbit platform support version caching project version WorkflowProjectView
💡 Result:
The specific term "WorkflowProjectView" does not appear in official CodeRabbit documentation, and there is no direct reference to a "platform support version caching project version" feature by that exact name. However, CodeRabbit provides comprehensive functionality related to caching and project workflow management, which likely covers the concepts implied by your query [1][2]. Caching in CodeRabbit: CodeRabbit utilizes a reusable sandbox cache, which is a prepared copy of a repository and its installed dependencies [1]. This system is designed to speed up reviews and other tasks by avoiding redundant rebuilds [1][3]. - Management: Organizations can view, search, and filter these caches through the Review > Caches tool [1][4]. - Control: Administrators with Write access to repository settings can delete a cache to force a fresh rebuild [1][3]. - Configuration: Caching is enabled by default [1]. You can disable it at the organization or repository level via the dashboard, or by setting reviews.disable_cache to true in your.coderabbit.yaml configuration file [1][2]. - Expiration: Caches expire automatically after seven days [1]. Workflow and Project Views: - CodeRabbit's primary interface for inspecting pull requests is the Change Stack, which organizes changes into logical cohorts and layers rather than a flat file list [5]. - CodeRabbit also features dedicated views for Coding Plans within its web app and IDE extensions (such as VS Code), allowing users to browse, create, and refine agent-ready plans [6][7]. If "WorkflowProjectView" is a specific internal component or a custom identifier from a third-party integration (such as a plugin or a specific CI/CD pipeline implementation), it is not a standard part of the public-facing CodeRabbit documentation [8]. It is recommended to check your specific repository's configuration or any custom integrations being used in conjunction with the platform [9][10].
Citations:
- 1: https://docs.coderabbit.ai/reference/caching
- 2: https://docs.coderabbit.ai/reference/configuration
- 3: https://docs.coderabbit.ai/changelog
- 4: https://coderabbit.mintlify.app/changelog
- 5: https://docs.coderabbit.ai/pr-reviews/coderabbit-review
- 6: https://docs.coderabbit.ai/plan/plan-refinement
- 7: https://docs.coderabbit.ai/ide/vscode-plan
- 8: https://docs.coderabbit.ai/
- 9: https://github.com/jeremylongshore/claude-code-plugins-plus-skills/blob/b2e8c5310d937dd95de0dc63c92aabfad3974c92/plugins/saas-packs/coderabbit-pack/skills/coderabbit-core-workflow-a/SKILL.md
- 10: https://docs.coderabbit.ai/guide/complete
Make latestObservedAt sensitive to any new observation.
callViews.flatMap(...).at(-1) returns the last observation in call order, not the newest observation. A new observation for a lower call index can leave run.version unchanged if no call row or event row changed at the same time. Use a latest timestamp across all observations.get(...) arrays, and include an observation count so later capped windows also change the value.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/workflow-view.ts` around lines 218 - 220, Update the latestObservedAt
computation around latestObservation to derive the maximum createdAt across
every call’s observations rather than using call order, and include the total
observation count in the resulting value so additions within capped windows
change run.version.
| function phaseStatus( | ||
| runStatus: WorkflowRunStatus, | ||
| inferred: WorkflowPhaseStatus | undefined, | ||
| calls: WorkflowCallView[], | ||
| ): WorkflowPhaseStatus { | ||
| if (calls.some((call) => call.status === "failed")) return "failed"; | ||
| if (calls.some((call) => call.status === "cancelled")) return "cancelled"; | ||
| if (calls.some((call) => call.status === "running")) return "running"; | ||
| if (inferred === "running" && !isTerminalRun(runStatus)) return "running"; | ||
| if (calls.length > 0 && calls.every((call) => | ||
| call.status === "completed" || call.status === "from_cache" | ||
| )) return "completed"; | ||
| return inferred ?? "pending"; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
A phase can report running after the run reached a terminal state.
Consider a run with status failed and a phase that has a phase_started event but no recorded calls. inferred is "running". Line 336 rejects it because the run is terminal. Line 337 is skipped because calls.length is 0. Line 340 then returns inferred, which is "running".
The navigator then draws the running glyph for a phase of a finished run.
Map the inferred running state to the run outcome when the run is terminal.
🐛 Proposed fix for terminal runs
if (inferred === "running" && !isTerminalRun(runStatus)) return "running";
if (calls.length > 0 && calls.every((call) =>
call.status === "completed" || call.status === "from_cache"
)) return "completed";
+ if (inferred === "running" && isTerminalRun(runStatus)) {
+ return runStatus === "completed" ? "completed" : runStatus;
+ }
return inferred ?? "pending";As per coding guidelines: "Prefer explicit lifecycle and state over hidden autonomy; make tasks, inputs, outputs, failures, and ownership inspectable."
📝 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.
| function phaseStatus( | |
| runStatus: WorkflowRunStatus, | |
| inferred: WorkflowPhaseStatus | undefined, | |
| calls: WorkflowCallView[], | |
| ): WorkflowPhaseStatus { | |
| if (calls.some((call) => call.status === "failed")) return "failed"; | |
| if (calls.some((call) => call.status === "cancelled")) return "cancelled"; | |
| if (calls.some((call) => call.status === "running")) return "running"; | |
| if (inferred === "running" && !isTerminalRun(runStatus)) return "running"; | |
| if (calls.length > 0 && calls.every((call) => | |
| call.status === "completed" || call.status === "from_cache" | |
| )) return "completed"; | |
| return inferred ?? "pending"; | |
| } | |
| function phaseStatus( | |
| runStatus: WorkflowRunStatus, | |
| inferred: WorkflowPhaseStatus | undefined, | |
| calls: WorkflowCallView[], | |
| ): WorkflowPhaseStatus { | |
| if (calls.some((call) => call.status === "failed")) return "failed"; | |
| if (calls.some((call) => call.status === "cancelled")) return "cancelled"; | |
| if (calls.some((call) => call.status === "running")) return "running"; | |
| if (inferred === "running" && !isTerminalRun(runStatus)) return "running"; | |
| if (calls.length > 0 && calls.every((call) => | |
| call.status === "completed" || call.status === "from_cache" | |
| )) return "completed"; | |
| if (inferred === "running" && isTerminalRun(runStatus)) { | |
| return runStatus === "completed" ? "completed" : runStatus; | |
| } | |
| return inferred ?? "pending"; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/workflow-view.ts` around lines 328 - 341, Update phaseStatus so an
inferred "running" phase is mapped to the terminal run outcome when
isTerminalRun(runStatus) is true, instead of falling through to return inferred.
Preserve the existing running behavior for non-terminal runs and the current
precedence for failed, cancelled, and completed call states.
Source: Coding guidelines
| function sumUsage(usages: Array<LocalAgentTokenUsage | undefined>): LocalAgentTokenUsage | undefined { | ||
| const present = usages.filter((usage): usage is LocalAgentTokenUsage => Boolean(usage)); | ||
| if (present.length === 0) return undefined; | ||
| const sum = (key: keyof LocalAgentTokenUsage): number | undefined => { | ||
| const values = present.map((usage) => usage[key]).filter((value): value is number => value !== undefined); | ||
| return values.length ? values.reduce((total, value) => total + value, 0) : undefined; | ||
| }; | ||
| const result: LocalAgentTokenUsage = {}; | ||
| for (const key of ["inputTokens", "outputTokens", "totalTokens", "cacheReadTokens", "cacheWriteTokens"] as const) { | ||
| const value = sum(key); | ||
| if (value !== undefined) result[key] = value; | ||
| } | ||
| return result; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Aggregated totalTokens can undercount across mixed providers.
sumUsage sums each key independently. It sets totalTokens when at least one usage defines it, and it ignores calls that report only inputTokens and outputTokens.
usageLabel in src/workflow-tui.ts prefers totalTokens when it is defined. So a run that mixes a provider reporting totalTokens with a provider reporting only input and output tokens displays a total that covers only the first provider's calls.
Derive totalTokens from the summed input and output tokens when a usage entry omits it.
🐛 Proposed fix for total-token aggregation
const result: LocalAgentTokenUsage = {};
for (const key of ["inputTokens", "outputTokens", "totalTokens", "cacheReadTokens", "cacheWriteTokens"] as const) {
const value = sum(key);
if (value !== undefined) result[key] = value;
}
+ if (present.some((usage) => usage.totalTokens === undefined)) {
+ const derived = present.reduce(
+ (total, usage) =>
+ total + (usage.totalTokens ?? (usage.inputTokens ?? 0) + (usage.outputTokens ?? 0)),
+ 0,
+ );
+ if (derived > 0) result.totalTokens = derived;
+ }
return result;📝 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.
| function sumUsage(usages: Array<LocalAgentTokenUsage | undefined>): LocalAgentTokenUsage | undefined { | |
| const present = usages.filter((usage): usage is LocalAgentTokenUsage => Boolean(usage)); | |
| if (present.length === 0) return undefined; | |
| const sum = (key: keyof LocalAgentTokenUsage): number | undefined => { | |
| const values = present.map((usage) => usage[key]).filter((value): value is number => value !== undefined); | |
| return values.length ? values.reduce((total, value) => total + value, 0) : undefined; | |
| }; | |
| const result: LocalAgentTokenUsage = {}; | |
| for (const key of ["inputTokens", "outputTokens", "totalTokens", "cacheReadTokens", "cacheWriteTokens"] as const) { | |
| const value = sum(key); | |
| if (value !== undefined) result[key] = value; | |
| } | |
| return result; | |
| } | |
| function sumUsage(usages: Array<LocalAgentTokenUsage | undefined>): LocalAgentTokenUsage | undefined { | |
| const present = usages.filter((usage): usage is LocalAgentTokenUsage => Boolean(usage)); | |
| if (present.length === 0) return undefined; | |
| const sum = (key: keyof LocalAgentTokenUsage): number | undefined => { | |
| const values = present.map((usage) => usage[key]).filter((value): value is number => value !== undefined); | |
| return values.length ? values.reduce((total, value) => total + value, 0) : undefined; | |
| }; | |
| const result: LocalAgentTokenUsage = {}; | |
| for (const key of ["inputTokens", "outputTokens", "totalTokens", "cacheReadTokens", "cacheWriteTokens"] as const) { | |
| const value = sum(key); | |
| if (value !== undefined) result[key] = value; | |
| } | |
| if (present.some((usage) => usage.totalTokens === undefined)) { | |
| const derived = present.reduce( | |
| (total, usage) => | |
| total + (usage.totalTokens ?? (usage.inputTokens ?? 0) + (usage.outputTokens ?? 0)), | |
| 0, | |
| ); | |
| if (derived > 0) result.totalTokens = derived; | |
| } | |
| return result; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/workflow-view.ts` around lines 347 - 360, Update sumUsage so aggregated
totalTokens includes entries that omit totalTokens: sum inputTokens and
outputTokens, then derive the missing contribution for each such usage before
setting result.totalTokens. Preserve explicitly reported totals while ensuring
mixed providers produce a complete aggregate consumed by usageLabel.
|
Closing this stack in favor of the alternative TUI implementation. Review found several unresolved behavioral issues in this version, including unphased calls that cannot be inspected, unsafe terminal-control rendering, refresh/state inconsistencies, and phase status that can remain running after terminal failure. We are proceeding with the implementation that handles these lifecycle and safety cases more explicitly. |
1 participant
The workflow view needs to answer which phase and agent are active without repeating the same summary across every screen, while still allowing a user to investigate a specific call. The TUI now exposes a compact workflow/phase/call navigator with keyboard selection and a call inspector showing prompt, provider/session, elapsed time, usage, latest activity, errors, and result. The view model derives phase lifecycle and aggregate usage from the journal. This layer depends on the storage and runtime layers below it.
Summary by CodeRabbit
New Features
Improvements