Skip to content

feat(gateway): rich tool cards in history + thread processing indicator - #2477

Merged
henrypark133 merged 5 commits into
stagingfrom
feat/rich-tool-history-cards
Apr 15, 2026
Merged

henrypark133 merged 5 commits into
stagingfrom
feat/rich-tool-history-cards

Conversation

@henrypark133

Copy link
Copy Markdown
Collaborator

Summary

  • Rich activity cards for most recent turn: When loading history, the last turn's tool calls render as the same expandable .activity-tool-card cards used during live SSE (icons, output preview, error auto-expand). Older turns keep the compact "N tools used" summary to limit DOM size.
  • Thread processing indicator: Background threads with active agent work show a spinner in the sidebar. Tracks via processingThreads Set fed by thinking/tool_started/stream_chunk SSE events; cleared on status: Done and SSE reconnect.
  • 4 Playwright E2E tests: message persistence after reload, rich tool card rendering, expand/collapse behavior, background thread unread badge.

Closes the UX gap where switching threads and coming back showed a flat summary instead of the rich tool cards the user saw live.

Test plan

  • pytest scenarios/test_message_persistence.py -v — 4 tests pass (18.7s)
  • pytest scenarios/test_chat.py scenarios/test_tool_execution.py -v — 8 existing tests pass, zero regressions
  • CI E2E tests pass
  • Manual: HEADED=1 pytest scenarios/test_message_persistence.py -v — visually verify cards match live rendering

🤖 Generated with Claude Code

History rendering:
- Add createActivityGroupFromHistory() to render the most recent turn's
  tool calls as the same .activity-tool-card DOM structure used during
  live SSE (expandable cards with icons, output preview, error details).
  Older turns keep the compact "N tools used" summary to limit DOM size.

Thread processing indicator:
- Track background threads with active agent work via processingThreads
  Set (fed by thinking, tool_started, stream_chunk SSE events for
  non-current threads; cleared on status "Done" and SSE reconnect).
- Render a .thread-processing spinner in the sidebar for threads that
  are actively processing.

E2E tests:
- test_message_persists_across_page_reload: message + response survive
  full page reload
- test_tool_calls_rendered_as_activity_cards_after_reload: echo tool
  renders as rich .activity-tool-card with data-status="success"
- test_tool_calls_expandable_after_reload: summary click expands cards
  container, card header click expands body
- test_background_thread_shows_processing_indicator: background thread
  gets unread badge after completion

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 14, 2026 22:04
@github-actions github-actions Bot added size: M 50-199 changed lines risk: low Changes to docs, tests, or low-risk modules contributor: core 20+ merged PRs labels Apr 14, 2026

Copilot AI left a comment

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.

Pull request overview

Adds richer history rendering in the gateway UI by reusing the live “activity tool card” UI for the most recent turn when loading history, and introduces a sidebar indicator for background threads that are actively processing based on SSE events. Also adds E2E coverage for persistence/reload and (intended) processing indicator behavior.

Changes:

  • Render most recent turn’s tool calls in history as expandable .activity-tool-card UI (older turns keep compact summaries).
  • Track background thread “processing” state from SSE events and show a sidebar spinner for non-active threads.
  • Add new Playwright E2E scenario tests and selectors for tool cards + processing indicator.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File Description
crates/ironclaw_gateway/static/app.js Adds processingThreads tracking + history rendering of rich activity tool cards.
crates/ironclaw_gateway/static/style.css Styles the sidebar processing indicator wrapper/spinner sizing.
tests/e2e/helpers.py Adds selectors for activity cards/history UI and thread processing indicator.
tests/e2e/scenarios/test_message_persistence.py Adds E2E tests for persistence after reload, history tool cards, and background-thread indicator behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread crates/ironclaw_gateway/static/app.js
Comment thread crates/ironclaw_gateway/static/app.js
Comment thread crates/ironclaw_gateway/static/app.js
Comment thread tests/e2e/scenarios/test_message_persistence.py
Comment thread tests/e2e/scenarios/test_message_persistence.py Outdated
- test_processing_indicator_shows_on_thread_switch: verify completed
  turns show no stale "Processing..." when switching back
- test_processing_indicator_shows_for_incomplete_turn: verify the
  thinking indicator appears when switching to a mid-turn thread
  (gracefully skips if agent completes too fast to catch)
- Add activity_thinking/activity_thinking_text selectors to helpers.py

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

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

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.

Code Review

This pull request introduces a background processing indicator for chat threads and enhances history rendering by displaying rich activity cards for the most recent turn. Key changes include the addition of a processingThreads set to track active agent work across threads, UI updates to show spinners in the sidebar, and new E2E tests for persistence and activity visualization. Feedback suggests clearing the processing state when an agent is "Awaiting approval," expanding activity groups by default if errors are present in history, and strengthening E2E tests by explicitly verifying the spinner's visibility.

Comment thread crates/ironclaw_gateway/static/app.js Outdated
Comment thread crates/ironclaw_gateway/static/app.js Outdated
Comment thread tests/e2e/scenarios/test_message_persistence.py
- Clear processingThreads + refresh sidebar on SSE reconnect so stale
  spinners are removed immediately
- Clear processingThreads on "Awaiting approval" status (terminal state
  where agent is blocked on user input, not actively processing)
- Map tool call status from has_result/has_error: running (neither),
  success (has_result), fail (has_error) — shows spinner for in-progress
  tools in history instead of misleading checkmark
- Auto-expand activity group when any tool call has an error
- Add data-thread-id attribute to .thread-item for testability
- Scope processing spinner and unread badge assertions to specific
  thread ID in E2E tests
- Add explicit spinner visibility/removal assertions to background
  thread processing indicator test

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 14, 2026 22:25

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread crates/ironclaw_gateway/static/app.js Outdated
Comment thread tests/e2e/scenarios/test_message_persistence.py Outdated
Comment thread tests/e2e/scenarios/test_message_persistence.py
Comment thread tests/e2e/scenarios/test_message_persistence.py Outdated
henrypark133 and others added 2 commits April 14, 2026 15:43
- Use activity-icon-success/activity-icon-fail CSS classes for history
  tool card icons (matches live card styling with colored ✓/✗)
- Fix _wait_for_completed_turn to check turns[-1] instead of any() to
  avoid early return when earlier turns are already completed
- Rename test_processing_indicator_shows_on_thread_switch to
  test_no_stale_processing_indicator_for_completed_thread to match
  what it actually verifies (no stale indicator, not indicator presence)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Merge staging into feat/rich-tool-history-cards, resolving the
test_message_persistence.py conflict (PR's superset version wins over
the basic staging version from #2475).

Also fix a bug where the processingThreads spinner would persist if a
background thread ended with "Interrupted", "Rejected", or
"Tool call denied." — only "Done" and "Awaiting approval" were clearing
the Set, leaving stale spinners until the next SSE reconnect.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 15, 2026 06:41

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +983 to +987
if (data.message === 'Done' || data.message === 'Awaiting approval'
|| data.message === 'Interrupted' || data.message === 'Rejected'
|| data.message === 'Tool call denied.') {
processingThreads.delete(data.thread_id);
}

Copilot AI Apr 15, 2026

Copy link

Choose a reason for hiding this comment

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

processingThreads is cleared for background threads only when data.message exactly matches hard-coded strings. Server-side status strings are treated case-insensitively and trimmed in multiple places (e.g., terminal detection uses trim() + eq_ignore_ascii_case), so a status like "done" or " Done " would leave a stale processing spinner forever. Normalize the status text in the client (e.g., trim + lowercase) before comparing, and use that for both the background-thread cleanup and the current-thread terminal checks.

Copilot uses AI. Check for mistakes.
Comment on lines 3118 to +3124
if (turn.tool_calls && turn.tool_calls.length > 0) {
addToolCallsSummary(turn.tool_calls);
if (i === lastTurnIndex) {
// Rich activity cards for the most recent turn
container.appendChild(createActivityGroupFromHistory(turn.tool_calls));
} else {
addToolCallsSummary(turn.tool_calls);
}

Copilot AI Apr 15, 2026

Copy link

Choose a reason for hiding this comment

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

loadHistory() renders rich activity tool cards for the last turn unconditionally. If the last turn is still Processing, those cards are built from in-memory history but are not registered in _activeToolCards, so subsequent SSE tool_completed / tool_result events won’t update them (and new live cards may be created in a separate activity group). This can leave stuck “running” spinners or duplicate cards after a reload mid-turn. Consider only using createActivityGroupFromHistory() when the last turn is terminal (e.g., state === 'Completed' or has a response), and fall back to the compact summary for in-progress turns, or explicitly hydrate _activeGroup/_activeToolCards from history so SSE updates apply to the same DOM.

Copilot uses AI. Check for mistakes.
Comment on lines +4384 to +4387
.thread-processing .spinner {
width: 12px;
height: 12px;
border-width: 2px;

Copilot AI Apr 15, 2026

Copy link

Choose a reason for hiding this comment

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

.thread-processing .spinner only sets width/height/border-width, but never sets border style/colors, border-radius, or the spin animation. Unlike the spinners in .activity-tool-icon and .scroll-load-spinner, this will likely render as an invisible/unstyled div, so the sidebar processing indicator won’t actually show a spinner. Consider reusing the same spinner styling (border + border-top-color + border-radius + animation) or introducing a shared .spinner base class that all contexts inherit from.

Suggested change
.thread-processing .spinner {
width: 12px;
height: 12px;
border-width: 2px;
.thread-processing .spinner {
display: inline-block;
width: 12px;
height: 12px;
border-width: 2px;
border-style: solid;
border-color: var(--border);
border-top-color: var(--accent);
border-radius: 50%;
animation: spin 1s linear infinite;

Copilot uses AI. Check for mistakes.

@serrrfirat serrrfirat left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Paranoid Architect Review — APPROVED ✅

v2 Engine Impact: Not related

Clean PR. No significant findings.

  • No XSS — all user-controlled data (tc.name, tc.error, tc.result_preview) flows through .textContent; all innerHTML uses static HTML literals only
  • E2E tests are well-designed with pragmatic timing-race handling
  • Code follows existing patterns (addToolCard, createToolCallsSummaryElement)

Minor notes (non-blocking):

  • Terminal status clearing uses exact string matching ('Done', 'Awaiting approval', etc.) — v2 engine gate events (gate_required) won't clear the spinner for background threads until SSE reconnect. Cosmetic only, backstopped by processingThreads.clear().
  • History card rendering duplicates live-card DOM structure without sharing code — acceptable trade-off, worth noting for future changes.

Ship it.

@henrypark133 henrypark133 mentioned this pull request Apr 21, 2026
theredspoon pushed a commit to theredspoon/ironclaw that referenced this pull request Jun 21, 2026
…or (nearai#2477)

* feat(gateway): rich tool cards in history + thread processing indicator

History rendering:
- Add createActivityGroupFromHistory() to render the most recent turn's
  tool calls as the same .activity-tool-card DOM structure used during
  live SSE (expandable cards with icons, output preview, error details).
  Older turns keep the compact "N tools used" summary to limit DOM size.

Thread processing indicator:
- Track background threads with active agent work via processingThreads
  Set (fed by thinking, tool_started, stream_chunk SSE events for
  non-current threads; cleared on status "Done" and SSE reconnect).
- Render a .thread-processing spinner in the sidebar for threads that
  are actively processing.

E2E tests:
- test_message_persists_across_page_reload: message + response survive
  full page reload
- test_tool_calls_rendered_as_activity_cards_after_reload: echo tool
  renders as rich .activity-tool-card with data-status="success"
- test_tool_calls_expandable_after_reload: summary click expands cards
  container, card header click expands body
- test_background_thread_shows_processing_indicator: background thread
  gets unread badge after completion

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* test(e2e): add processing indicator tests + review fixes

- test_processing_indicator_shows_on_thread_switch: verify completed
  turns show no stale "Processing..." when switching back
- test_processing_indicator_shows_for_incomplete_turn: verify the
  thinking indicator appears when switching to a mid-turn thread
  (gracefully skips if agent completes too fast to catch)
- Add activity_thinking/activity_thinking_text selectors to helpers.py

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(gateway): address PR nearai#2477 review comments

- Clear processingThreads + refresh sidebar on SSE reconnect so stale
  spinners are removed immediately
- Clear processingThreads on "Awaiting approval" status (terminal state
  where agent is blocked on user input, not actively processing)
- Map tool call status from has_result/has_error: running (neither),
  success (has_result), fail (has_error) — shows spinner for in-progress
  tools in history instead of misleading checkmark
- Auto-expand activity group when any tool call has an error
- Add data-thread-id attribute to .thread-item for testability
- Scope processing spinner and unread badge assertions to specific
  thread ID in E2E tests
- Add explicit spinner visibility/removal assertions to background
  thread processing indicator test

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(gateway): address second round of PR nearai#2477 review comments

- Use activity-icon-success/activity-icon-fail CSS classes for history
  tool card icons (matches live card styling with colored ✓/✗)
- Fix _wait_for_completed_turn to check turns[-1] instead of any() to
  avoid early return when earlier turns are already completed
- Rename test_processing_indicator_shows_on_thread_switch to
  test_no_stale_processing_indicator_for_completed_thread to match
  what it actually verifies (no stale indicator, not indicator presence)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: core 20+ merged PRs risk: low Changes to docs, tests, or low-risk modules size: M 50-199 changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants