Skip to content

assistant: remove debug UI and reasoning-token streaming#1488

Open
leonardmq wants to merge 1 commit into
leonard/kil-692-assistant-auto-modefrom
leonard/remove-assistant-debugging
Open

assistant: remove debug UI and reasoning-token streaming#1488
leonardmq wants to merge 1 commit into
leonard/kil-692-assistant-auto-modefrom
leonard/remove-assistant-debugging

Conversation

@leonardmq

Copy link
Copy Markdown
Collaborator

Removes the optional debug UI from the Kiln Assistant and the per-thinking-token reasoning stream, keeping the production experience intact. Stacked on top of #1451 (auto-mode).

What's removed

  • Debug render branch in chat.svelte — the PUBLIC_SHOW_TOOL_CALL_DETAILS env flag, the alternate view that dumped raw tool input/output JSON in <pre> blocks, the (debug mode) page title, and the related dead helpers/collapse consumers. Only the production "simplified" view remains.
  • Reasoning (thinking-token) stream — the reasoning ChatMessagePart type and all reasoning stream handlers in streaming_chat.ts; session restore (session_messages.ts) no longer rebuilds reasoning parts. (The backend now drops reasoning events entirely — see the companion kiln_server PR.)
  • chat_step_group.svelte — dead component (imported nowhere) that only existed for the reasoning-render pattern.
  • The PUBLIC_SHOW_TOOL_CALL_DETAILS entry in .env.example.

What's preserved

  • Output-token streaming (text-delta).
  • The "assistant is working" affordance: streaming cursor + animated icon + ChatStatusSteps activity indicator + kiln-tool-execution-* framing.
  • Tool approval UI and the "Thought" step grouping.

Verification

  • svelte-check: 0 errors.
  • Full vitest suite: 1118 passed.
  • One test updated: session_messages.test.ts now asserts reasoning_content is ignored on restore.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KwfR9HM3b8Z9gs2PvJz5xA

Removes the optional debug UI from the Kiln Assistant (the
PUBLIC_SHOW_TOOL_CALL_DETAILS env flag and its alternate render branch
that dumped raw tool input/output JSON and a "(debug mode)" title),
keeping only the production "simplified" view.

Also drops the per-thinking-token reasoning stream: the reasoning
ChatMessagePart type and its stream handlers are removed, and session
restore no longer reconstructs reasoning parts. The "assistant is
working" affordance (streaming cursor, animated icon, ChatStatusSteps
activity indicator, tool-execution framing) and output-token streaming
are preserved. Deletes the now-dead chat_step_group.svelte (unused).

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

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Removes reasoning/thinking content support from the chat UI. The reasoning variant is stripped from ChatMessagePart, all reasoning state and event handlers are deleted from StreamEventProcessor, chat_step_group.svelte is deleted, and chat.svelte is rewritten with a simplified segment-based rendering model. The PUBLIC_SHOW_TOOL_CALL_DETAILS debug flag is also removed.

Changes

Remove reasoning support from chat UI

Layer / File(s) Summary
Remove reasoning from ChatMessagePart type and stream processor
app/web_ui/src/lib/chat/streaming_chat.ts, app/web_ui/src/lib/chat/session_messages.ts, app/web_ui/src/lib/chat/session_messages.test.ts
Drops the reasoning union member from ChatMessagePart, removes reasoning serialization in toBackendMessage(), strips the reasoning slot from PartSlot, deletes all reasoning state fields (reasoningBlocks, currentReasoningId), removes the reasoning-start/reasoning-delta/reasoning-end event handlers and slot-creation helper, removes the reasoning branch from flushAssistant(), removes reasoning_content mapping from buildAssistantParts, and updates the snapshot hydration test to assert that reasoning_content is ignored.
Delete chat_step_group.svelte and rewrite chat.svelte rendering
app/web_ui/src/routes/(app)/assistant/chat_step_group.svelte, app/web_ui/src/routes/(app)/assistant/chat.svelte
Deletes the entire chat_step_group.svelte component. In chat.svelte, removes the collapsedPartKeys store binding and all reasoning/tool-output collapse helpers, introduces the RenderSegment abstraction with segment-grouping and step-group-loading logic, adds getToolCallId, and rewrites the assistant message template to use segments with ToolApprovalBox/ToolStatusLine for tool parts and a simplified step-group expansion UI.
Remove PUBLIC_SHOW_TOOL_CALL_DETAILS env flag
app/web_ui/.env.example, app/web_ui/src/routes/(app)/assistant/+page.svelte
Removes the PUBLIC_SHOW_TOOL_CALL_DETAILS documentation from .env.example, deletes the $env/dynamic/public import from +page.svelte, and replaces the conditional debug/normal title with a constant "Assistant".

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • Kiln-AI/Kiln#1290: Modifies chat_step_group.svelte to use ToolStatusLine for tool status rendering — the component this PR deletes entirely.

Suggested reviewers

  • scosman

Poem

🐇 Hop, hop, the reasoning's gone away,
No more "thinking" blocks on display!
chat_step_group fades into the night,
Segments now render clean and bright.
The assistant says "I'm just… Assistant!" with glee,
Simple is better — that's plain to see! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarizes the main changes: removal of debug UI and reasoning-token streaming from the assistant.
Description check ✅ Passed The description covers all required sections: 'What does this PR do?' is thoroughly detailed, but related issues link and contributor license agreement confirmation are missing.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch leonard/remove-assistant-debugging

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@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 removes the "reasoning" message parts and the debug mode (previously controlled by the PUBLIC_SHOW_TOOL_CALL_DETAILS environment variable) from the assistant chat UI. This includes deleting reasoning-related event handlers, state, and UI components, leaving only the simplified view for chat messages and tool calls. A review comment identifies a redundant definition of the segments variable in chat.svelte that can be safely removed to avoid unnecessary recalculation.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +635 to +639
{#if message.role === "assistant"}
{@const segments = groupPartsForSimplifiedView(
message.parts ?? [],
)}
{@const lastSegIsText =

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

The segments variable is already defined in the parent scope ({#if message.parts && message.parts.length > 0}) on line 478. Redefining and recalculating it here is redundant and inefficient, especially since Svelte templates re-evaluate these expressions on updates. You can safely remove this duplicate @const definition and reuse the parent scope's segments variable.

                  {#if message.role === "assistant"}
                    {@const lastSegIsText =

@github-actions

Copy link
Copy Markdown

📊 Coverage Report

Overall Coverage: 92%

Diff: origin/leonard/kil-692-assistant-auto-mode...HEAD

No lines with coverage information in this diff.


@coderabbitai coderabbitai 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.

🧹 Nitpick comments (1)
app/web_ui/src/lib/chat/streaming_chat.ts (1)

225-254: Confirm unknown stream events are safely ignored; consider adding explicit test coverage.

Unknown event types are handled safely via the conditional guard at line 258-261 (if (handler) { ... }), so unrecognized events result in no-op behavior rather than throwing. This prevents frontend/backend deploy skew from interrupting the stream. However, no explicit test case currently verifies this behavior for unknown or unhandled event types (e.g., legacy reasoning-* events); adding one would document the intended rollout safety.

🤖 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 `@app/web_ui/src/lib/chat/streaming_chat.ts` around lines 225 - 254, Add
explicit test coverage to verify that unknown or unhandled stream event types
are safely ignored without throwing errors. Create a test case that simulates
sending an unknown event type (for example, a legacy "reasoning-*" event)
through the streaming handler mechanism and confirm that the conditional guard
check (if handler exists before calling it) causes the unknown event to result
in a no-op rather than an error or exception. This documents the intended
rollout safety behavior that prevents frontend/backend deploy skew from
interrupting the stream.
🤖 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.

Nitpick comments:
In `@app/web_ui/src/lib/chat/streaming_chat.ts`:
- Around line 225-254: Add explicit test coverage to verify that unknown or
unhandled stream event types are safely ignored without throwing errors. Create
a test case that simulates sending an unknown event type (for example, a legacy
"reasoning-*" event) through the streaming handler mechanism and confirm that
the conditional guard check (if handler exists before calling it) causes the
unknown event to result in a no-op rather than an error or exception. This
documents the intended rollout safety behavior that prevents frontend/backend
deploy skew from interrupting the stream.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 42b42364-a9f9-41ed-9baa-932a1b578f9e

📥 Commits

Reviewing files that changed from the base of the PR and between 575a41a and a4265ac.

📒 Files selected for processing (7)
  • app/web_ui/.env.example
  • app/web_ui/src/lib/chat/session_messages.test.ts
  • app/web_ui/src/lib/chat/session_messages.ts
  • app/web_ui/src/lib/chat/streaming_chat.ts
  • app/web_ui/src/routes/(app)/assistant/+page.svelte
  • app/web_ui/src/routes/(app)/assistant/chat.svelte
  • app/web_ui/src/routes/(app)/assistant/chat_step_group.svelte
💤 Files with no reviewable changes (3)
  • app/web_ui/src/lib/chat/session_messages.ts
  • app/web_ui/.env.example
  • app/web_ui/src/routes/(app)/assistant/chat_step_group.svelte

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