Skip to content

feat: live tool call disclosure in chat UI - #398

Open
kpmmmurphy wants to merge 2 commits into
masterfrom
feat/tool-call-disclosure
Open

kpmmmurphy wants to merge 2 commits into
masterfrom
feat/tool-call-disclosure

Conversation

@kpmmmurphy

Copy link
Copy Markdown
Contributor

Summary

Tool calls are now visible during the turn, not just after completion:

  • Mac toolTraceFooter: removed the status != .streaming gate so the "Used web search · date & time" line appears live as each tool dispatches, with a .contentTransition(.numericText()) animation as new tools accumulate
  • iOS toolTraceRow: added the same persisted provenance line — was entirely missing from the iOS bubble
  • ActivityLabeler: 6 tools that fell through to the raw "Using {name}…" default now get tailored live labels (delegate_deep, list_documents, get_document, open_link, lookup_fact, recent_activity); the default fallback humanises via displayName(forTool:) so unknown tools get underscores replaced with spaces

Before

  • Mac: tool trace only visible after the turn completes
  • iOS: no tool trace at all — no way to see which tools M1K3 used

After

  • Both platforms: tool trace appears live as each tool dispatches and stays after the turn settles

Test plan

  • ActivityLabelerTests — all 6 tests green (new tool labels + humanised default)
  • ToolTraceTests — trace accumulation + straggler drop
  • ChatSessionActivityTests — label lifecycle
  • MemoryGroundingTests — byte-identical snapshots unaffected
  • Verify-by-launch on Mac: ask a question that triggers web_search → trace shows live
  • Verify-by-launch on iPhone: same, confirm the new toolTraceRow renders

🤖 Generated with Claude Code

https://claude.ai/code/session_01ELMgNSM3gNKL6kFm16wpyX

Tool calls are now visible DURING the turn, not just after:

- Mac toolTraceFooter: removed the status != .streaming gate so the
  "Used web search · date & time" line appears live as each tool
  dispatches, with a content transition animation.
- iOS toolTraceRow: added the same provenance line (was missing
  entirely from the iOS bubble).
- ActivityLabeler: 6 tools that fell through to the raw default now
  get tailored live labels (delegate_deep, list_documents,
  get_document, open_link, lookup_fact, recent_activity); the
  default fallback humanises via displayName (underscores → spaces).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ELMgNSM3gNKL6kFm16wpyX

# ── MurphySig ──────────────────────────────────────────────────────
# Significant decision in this commit? Sign it. https://murphysig.dev/spec
# ───────────────────────────────────────────────────────────────────
@netlify

netlify Bot commented Sep 22, 2026

Copy link
Copy Markdown

Deploy Preview for m1k3 canceled.

Name Link
🔨 Latest commit 0473e9f
🔍 Latest deploy log https://app.netlify.com/projects/m1k3/deploys/6ab2b238435aa70008780987

@claude

claude Bot commented Sep 22, 2026

Copy link
Copy Markdown

Reviewed the diff (MessageView.swift, MessageBubble.swift, ResponderActivity.swift, ActivityTests.swift). The feature itself is sound — ActivityLabeler stays pure/tested in M1K3Chat, the two view-side additions are pure glue (toolTraceFooter/toolTraceRow), and dropping the status != .streaming gate on Mac correctly aligns the view with ChatSession.swift:584-588, which already appends to toolsUsed live while status == .streaming (guarded against post-settle stragglers). iOS genuinely had no persisted trace before this — confirmed.

Two things worth fixing before merge:

1. Broken doc comments — macos/Sources/M1K3Chat/ResponderActivity.swift:38,46

/// The transcript's persisted provenance line ("Used web search \u{00B7} date &
/// time") -- pinned here rather than composed in the View so the product
/// string is testable.

\u{00B7} escapes are only expanded inside string literals; in a /// comment they're inert text. These two doc comments will now literally read \u{00B7} instead of showing a middle dot — worse than what was there before (a real · character). Same pattern hit the em dash → -- and the comment apostrophe → curly a few lines up. Looks like an unrelated find/replace swept the whole file, including comments where escapes don't apply. The string-literal uses (" \u{00B7} ", \u{2026}, etc.) are all functionally fine — this is comment-only, but it's a real regression in readability. Worth reverting the comment text to literal characters (matching MessageView.swift/MessageBubble.swift, which still use literal /· and are untouched by this sweep).

2. Dropped rationale comment — same file, case .retrieving:
The previous version had:

// Deliberately NOT "…your knowledge": the every-turn RAG phase used
// to read like the search_knowledge tool, so tool calls looked like
// they fired on every turn when they hadn't (Kev, 2026-08-16). A
// self-action verb keeps the phase and the tool distinguishable.

This got deleted with no replacement and isn't mentioned in the PR description — looks like collateral from the same sweep rather than an intentional edit. The rationale itself still survives in ActivityTests.swift:51-53, so nothing's lost for good, but the WHY at the point of use is gone. Worth restoring (with the literal , not an escape) since it's exactly the kind of non-obvious "why" the repo's comment convention wants kept next to the code it explains.

Minor/optional: .contentTransition(.numericText()) on toolTraceFooter/toolTraceRow (MessageView.swift:172, MessageBubble.swift:113) is built for numeral roll-up effects; applied to prose like "Used web search · date & time" it likely just falls back to a crossfade. Not wrong, but .opacity would say what you mean more directly if the numeric-specific animation isn't intentional.

Tests (ActivityLabelerTests) look correct and cover the 6 new tool labels plus the humanised default fallback (query_graph → "Using query graph…"), matching the displayName(forTool:) change.

- ResponderActivity.swift: restored from master and applied surgical
  edits only (6 new cases + humanised default). The full-file rewrite
  had dropped the Kev 2026-08-16 comment and mangled doc-comment
  typography into \u{} escapes that render as literal text. Review
  line added.
- Dropped .contentTransition(.numericText()) on both platforms — it's
  designed for digit-rolling and its behaviour on non-numeric strings
  is undocumented; the .animation alone handles the insertion.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ELMgNSM3gNKL6kFm16wpyX

# ── MurphySig ──────────────────────────────────────────────────────
# Significant decision in this commit? Sign it. https://murphysig.dev/spec
# ───────────────────────────────────────────────────────────────────
@claude

claude Bot commented Sep 22, 2026

Copy link
Copy Markdown

Review — PR #398 (live tool call disclosure)

Small, well-scoped diff (54/9 across 4 files). Went through it against the six review lenses; nothing blocking.

Correctness / behavior

  • toolTraceFooter/toolTraceRow gating on !tools.isEmpty is correct, and message.toolsUsed was already being mutated live on @MainActor during streaming pre-PR (ChatSession.swift:584-588) — this PR only changes when it's displayed, not the underlying accumulation/dedup/straggler-drop logic, which is already covered by ToolTraceTests. Good scoping: no new concurrency surface introduced.
  • The 6 new tool → label mappings (delegate_deep, list_documents, get_document, open_link, lookup_fact, recent_activity) all match real registered tool name strings (M1K3AgentTools/M1K3KnowledgeTools/M1K3MCPKit) — verified by grep, not just asserted in the PR body.
  • default: "Using \(displayName(forTool: name))…" is a nice touch — humanizes any future tool automatically instead of leaking snake_case, and the updated phases() test (query_graph → "Using query graph…") pins it.
  • traceLabel(for:) iterates the array in dispatch order (dedup-on-first-occurrence happens upstream in ChatSession), so trace order stays stable — matches ToolTraceTests.traceAccumulates.

Minor / non-blocking

  • MessageView.swift:174 doc comment ("Leaked turns can't reach here with a trace: the guard nils it with the rest") is now very slightly stale: since the footer is live during streaming, a turn that dispatches a tool and then trips PersonaLeakGuard at finalize will show the trace for the duration of the stream before it's nil'd out at completion (ChatSession.swift:502). Cosmetic flicker only — the persisted/final invariant the comment is actually protecting (no trace surfaces on a leaked, persisted message) still holds, and it mirrors existing sourcesRow behavior (sources are also set live pre-finalize and wiped on leak). Not worth code changes, just flagging in case the comment wording is worth a follow-up tweak.
  • M1K3iOSApp/MessageBubble.swift's toolTraceRow (macOS's toolTraceFooter has .textSelection(.enabled), the iOS row doesn't). Given the rest of the iOS bubble doesn't apply .textSelection broadly either, this reads as consistent with the surrounding file rather than an oversight — just noting the asymmetry in case it wasn't deliberate.

Provenance / architecture

  • Both new Review: header lines follow the existing signed-block convention, correctly dated and non-fabricated. ResponderActivity.swift stays dependency-free (pure enum/switch), ActivityLabeler untouched as a pure, testable seam — no logic leaked into the app targets, the two view files stay thin glue.
  • Second commit (0473e9f1) is a good self-correction: reverting the full-file ResponderActivity.swift rewrite back to surgical edits (preserving the 2026-08-16 comment) and dropping .contentTransition(.numericText()) for the documented reason (undocumented behavior on non-numeric strings) — exactly the kind of local review-fold this repo's process rewards.

Tests: ActivityLabelerTests (6 new cases + updated default-fallback case) and existing ToolTraceTests/ChatSessionActivityTests cover the pure logic and lifecycle; the two "verify-by-launch" boxes left unchecked (Mac/iPhone live rendering) are correctly the remaining owed verification per macos/CLAUDE.md's metallib-wall convention — this is UI-only so swift test can't exercise the actual live animation.

No concurrency, GRDB/persistence, or MLX-boundary concerns — this diff doesn't touch any of those surfaces.

🤖 Generated with Claude Code

@claude claude Bot mentioned this pull request Sep 23, 2026
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.

1 participant