Skip to content

feat(tui): enrich activity detail context#2298

Open
axobase001 wants to merge 2 commits into
Hmbown:mainfrom
axobase001:feat/1547-activity-detail-context
Open

feat(tui): enrich activity detail context#2298
axobase001 wants to merge 2 commits into
Hmbown:mainfrom
axobase001:feat/1547-activity-detail-context

Conversation

@axobase001
Copy link
Copy Markdown
Contributor

@axobase001 axobase001 commented May 28, 2026

Fixes #1547

Summary:

  • add Activity Detail position and previous/next activity context for Ctrl+O
  • include artifact/retrieve or Alt+V detail handles without dumping raw tool output into the pager
  • add neighboring reasoning chunk previews in the reasoning timeline

Validation:

  • cargo test -p codewhale-tui --bin codewhale-tui activity_detail

Greptile Summary

This PR enriches the Activity Detail pager (Ctrl+O) with position/navigation context and tool artifact handle lines, and adds neighboring reasoning-chunk previews in the reasoning timeline view.

  • activity_detail_text now computes an activity_indices vector and emits an "Activity chunk: X of Y" line plus Previous/Next activity labels by delegating to the new activity_navigation_lines helper; activity_detail_handle_line adds a "Detail handle" line that describes whether the cell is backed by a session artifact, a raw tool-call record, or a generic Alt+V shortcut.
  • reasoning_timeline_text gains "Previous chunk" / "Next chunk" preview lines around the selected chunk, sourcing content from the new thinking_chunk_preview helper.
  • tests.rs gains one new test (activity_detail_includes_tool_handle_and_neighbor_context) and extends two existing tests to cover the new output lines.

Confidence Score: 5/5

Safe to merge — all new display-only helpers are read-only and bounded correctly.

The changes are purely presentational: no state mutation, no new I/O paths, and no changes to data ownership. The indexing arithmetic in both the 0-based activity helpers and the 1-based reasoning-timeline helpers is correct, all array accesses are guarded against out-of-bounds, and the new test covers the artifact-handle and neighbor-navigation paths end-to-end.

No files require special attention.

Important Files Changed

Filename Overview
crates/tui/src/tui/ui.rs Adds activity_indices, activity_navigation_lines, activity_detail_handle_line, and thinking_chunk_preview helpers; updates activity_detail_text and reasoning_timeline_text to emit position/navigation/handle context. Indexing arithmetic is correct and bounds guards are sound.
crates/tui/src/tui/ui/tests.rs New test activity_detail_includes_tool_handle_and_neighbor_context covers artifact-backed handle lines and prev/next navigation; existing tests extended with neighboring-chunk assertions. Coverage is thorough and assertions are precise.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["Ctrl+O pressed"] --> B["open_activity_detail_pager"]
    B --> C["activity_detail_text(app, cell_index, width)"]
    C --> D{Is cell Thinking?}
    D -- Yes --> E["reasoning_timeline_text(app, cell_index)"]
    D -- No --> F["Build sections: turn, activity label, status"]
    E --> G["Compute thinking_indices (all Thinking cells)"]
    G --> H["Emit 'Selected chunk: X of N'"]
    H --> I{position > 1?}
    I -- Yes --> J["thinking_chunk_preview → 'Previous chunk: X-1 of N - preview'"]
    I -- No --> K
    J --> K{position < total?}
    K -- Yes --> L["thinking_chunk_preview → 'Next chunk: X+1 of N - preview'"]
    K -- No --> M["Emit full timeline list"]
    L --> M
    F --> N["activity_indices (all meaningful cells)"]
    N --> O["Emit 'Activity chunk: X of Y'"]
    O --> P["activity_navigation_lines → prev/next labels"]
    P --> Q["activity_detail_handle_line"]
    Q --> R{tool_detail_record?}
    R -- Yes + artifact --> S["'Detail handle: ART_ID (retrieve_tool_result ref=...; Alt+V raw details)'"]
    R -- Yes, no artifact --> T["'Detail handle: tool:CALL_ID (Alt+V raw details)'"]
    R -- No, Tool/SubAgent --> U["'Detail handle: Alt+V details'"]
    R -- No, other --> V["no handle line"]
    S & T & U & V --> W["Emit activity body text"]
Loading

Reviews (2): Last reviewed commit: "fix(tui): refine activity detail review ..." | Re-trigger Greptile

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the TUI activity detail view to display "Activity chunk" instead of "Thinking chunk", introduces navigation context for previous and next activities, and adds a detail handle line for tool and sub-agent cells. It also enhances the reasoning timeline to display previews of neighboring thinking chunks, supported by new unit tests. The reviewer feedback recommends optimizing redundant virtual cell scans by pre-computing activity indices and correcting a misleading "Alt+V raw details" label to "Alt+V details" when raw details are not actually available.

Comment thread crates/tui/src/tui/ui.rs Outdated
Comment on lines +7594 to +7602
if let Some((position, total)) = activity_position(app, cell_index) {
sections.push(format!("Activity chunk: {position} of {total}"));
}

if let Some(handle) = activity_detail_handle_line(app, cell_index, cell) {
sections.push(handle);
}

sections.extend(activity_navigation_lines(app, cell_index));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Redundant Virtual Cell Scans

Both activity_position and activity_navigation_lines perform a full scan of 0..app.virtual_cell_count(), calling app.cell_at_virtual_index(idx) and is_meaningful_activity_cell(cell) for every single cell. Since HISTORY_SOFT_CAP is 5,000, this can result in up to 10,000 virtual cell lookups and checks on a single invocation of activity_detail_text.

We can optimize this by pre-computing the activity_indices once in activity_detail_text and passing them (or the derived position) to the helper functions.

For example:

let activity_indices: Vec<usize> = (0..app.virtual_cell_count())
    .filter(|&idx| {
        app.cell_at_virtual_index(idx)
            .is_some_and(is_meaningful_activity_cell)
    })
    .collect();

if let Some(position) = activity_indices.iter().position(|&idx| idx == cell_index) {
    sections.push(format!(\"Activity chunk: {} of {}\", position + 1, activity_indices.len()));
    ...
    sections.extend(activity_navigation_lines(app, position, &activity_indices));
}

Comment thread crates/tui/src/tui/ui.rs Outdated
Comment on lines +7903 to +7905
HistoryCell::Tool(_) if app.cell_has_detail_target(cell_index) => {
Some("Detail handle: Alt+V raw details".to_string())
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Misleading Detail Handle Label

When app.tool_detail_record_for_cell(cell_index) is None, there are no raw JSON details (input/output) available for the tool. In this case, open_details_pager_for_cell falls back to displaying the standard formatted history cell text.

Labeling this as Alt+V raw details is misleading to the user because no raw details are actually available. It should be labeled as Alt+V details instead.

        HistoryCell::Tool(_) if app.cell_has_detail_target(cell_index) => {
            Some(\"Detail handle: Alt+V details\".to_string())
        }

Comment thread crates/tui/src/tui/ui.rs
Comment thread crates/tui/src/tui/ui.rs Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Upgrade Ctrl+O into an Activity Detail pager with turn and chunk navigation

1 participant