Skip to content

fix(conversations): show a pending spinner for unresolved tool nodes instead of "(empty)" - #12727

Merged
diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.51from
hartmark:fix-pending-tool-node-placeholder
Sep 10, 2026
Merged

diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.51from
hartmark:fix-pending-tool-node-placeholder

Conversation

@hartmark

@hartmark hartmark commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Summary

  • /dashboard/conversations rendered every tool node with no content yet
    as a bare "(empty)" text bubble, whether it was a genuinely-purged node
    or simply a node still waiting on its display content to resolve from
    the owning call-log artifact (resolveTurnDisplayContent). Live
    conversations showed a confusing flash of "(empty)" bubbles before the
    real tool call/result content populated a moment later.
  • toTurn() now maps role === "tool" && !textPreview to a distinct
    pending NormalizedBlock variant, rendered as a spinner + "resolving…"
    instead. Every other mapping (real empty assistant text, resolved
    tool_use/tool_result, resolved plain text) is unchanged.
  • toTurn()/ConversationTurn moved out of the conversations page (a
    "use client" component that pulls in ChatBubble/MarkdownMessage's
    dependency tree) into a small colocated pure module
    (toTurn.ts), so this mapping stays unit-testable without a
    browser/DOM harness. Net production LOC in page.tsx goes down.

Related Issues

  • Closes #
  • Related to #

Validation

  • Change type: UI
  • Focused tests and category gates from the golden path
  • npm run lint (targeted: touched files clean)
  • Reconciled with the current active release base (release/v3.8.51); focused checks rerun afterward
  • Production-code changes include a new or updated automated test in this PR

Tests Added Or Updated

  • tests/unit/conversations-pending-tool-node.test.ts (new): proves the
    new pending mapping for an unresolved tool node, proves a genuinely
    empty assistant reply still renders as empty text (not pending), proves
    a tool node renders its real content once textPreview lands, and
    proves tool_use/tool_result mapping is unaffected. Verified RED on
    pre-fix toTurn() (falls back to {type:"text", text:"_(empty)_"}"),
    GREEN after.

Coverage Notes

  • Touches src/app/(dashboard)/dashboard/conversations/{page.tsx,toTurn.ts},
    src/app/(dashboard)/dashboard/tools/traffic-inspector/components/chat/MessageContent.tsx,
    and src/mitm/inspector/types.ts (new pending NormalizedBlock
    variant). The new test covers the entire toTurn() mapping surface
    directly; the spinner rendering itself is a small, visually-obvious
    branch in MessageContent.tsx mirroring the existing tool_use/
    tool_result branches.

Reviewer Notes

  • No migration, no feature flag, no protocol change — purely additive UI
    state (pending) plus a data-mapping refactor (moved, not rewritten).
  • typecheck:dashboard was run against this branch before and after the
    change; the ~330 pre-existing errors on this release base are all
    outside the touched files and unaffected by this change (verified via
    git stash/git stash pop diff).

…instead of "(empty)"

conversation_turn_nodes records a turn's identity (role, blockKind) the
moment it's recorded, independent of when its display content resolves
from the owning call-log artifact (resolveTurnDisplayContent). A tool
node for a request still in flight is real -- it just has nothing to
show yet -- but /api/conversations/[id]/tree's own blockKind ?? "text"
fallback can't tell that apart from a permanently-purged node, so
/dashboard/conversations rendered both as a bare "(empty)" bubble that
read as broken rather than in progress.

The frontend CAN tell them apart: role is set at record time and is
always "tool" for a real tool identity node, regardless of content
resolution. Map that combination (role==="tool" && !textPreview) to a
new NormalizedBlock pending variant instead, rendered with a spinner;
every other mapping is unchanged.

Also extracted toTurn()/ConversationTurn out of the conversations page
(a "use client" component that pulls in ChatBubble/MarkdownMessage's
dependency tree) into a small colocated pure module, so this mapping
logic is unit-testable without a browser/DOM harness. Net production
LOC in page.tsx goes down; the new toTurn.ts is the same logic moved,
plus the new pending branch.
@hartmark
hartmark marked this pull request as ready for review September 4, 2026 11:28
hartmark added a commit to hartmark/OmniRoute that referenced this pull request Sep 4, 2026
…er for unresolved tool nodes instead of "(empty)") into dev/omniroute-dev-combined
…s, rename to "loading..."

Live traffic showed the same textPreview resolution lag the earlier fix
covered for tool nodes also hits user/assistant nodes -- they hit the
same lazy resolveTurnDisplayContent pipeline. Drop the role==="tool"
restriction so any node with no textPreview yet (in the plain-text
fallback branch) renders the pending spinner, not just tool. Also
renames the label from "resolving..." to "loading..." per feedback.
hartmark added a commit to hartmark/OmniRoute that referenced this pull request Sep 4, 2026
…er for unresolved tool nodes instead of "(empty)") into dev/omniroute-dev-combined
hartmark added a commit to hartmark/OmniRoute that referenced this pull request Sep 4, 2026
…er for unresolved tool nodes instead of "(empty)") into dev/omniroute-dev-combined
hartmark added a commit to hartmark/OmniRoute that referenced this pull request Sep 5, 2026
…er for unresolved tool nodes instead of "(empty)") into dev/omniroute-dev-combined
hartmark added a commit to hartmark/OmniRoute that referenced this pull request Sep 6, 2026
…er for unresolved tool nodes instead of "(empty)") into dev/omniroute-dev-combined
hartmark added a commit to hartmark/OmniRoute that referenced this pull request Sep 7, 2026
…er for unresolved tool nodes instead of "(empty)") into dev/omniroute-dev-combined
hartmark added a commit to hartmark/OmniRoute that referenced this pull request Sep 7, 2026
…er for unresolved tool nodes instead of "(empty)") into dev/omniroute-dev-combined
hartmark added a commit to hartmark/OmniRoute that referenced this pull request Sep 8, 2026
…er for unresolved tool nodes instead of "(empty)") into dev/omniroute-dev-combined
hartmark added a commit to hartmark/OmniRoute that referenced this pull request Sep 8, 2026
…er for unresolved tool nodes instead of "(empty)") into dev/omniroute-dev-combined
hartmark added a commit to hartmark/OmniRoute that referenced this pull request Sep 9, 2026
…er for unresolved tool nodes instead of "(empty)") into dev/omniroute-dev-combined
@diegosouzapw
diegosouzapw merged commit b516e95 into diegosouzapw:release/v3.8.51 Sep 10, 2026
9 of 16 checks passed
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…instead of "(empty)" (diegosouzapw#12727)

Validado numa worktree combinada com a onda de dashboard/monitoring desta leva sobre `release/v3.8.51`: typecheck:core limpo, check-api-typecheck OK (289), check-file-size OK após rebaseline, 130/131 nos testes focados — a falha restante é asserção de tempo de parede sob carga, verde 6/6 isolada.

"(empty)" para um nó de ferramenta ainda não resolvido é informação errada, não ausência de informação — o usuário lê como "não retornou nada". Spinner de pendente diz a verdade.
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.

2 participants