feat(miot-chat): real answer streaming, inline loading, no truncation, markdown#847
Merged
Merged
Conversation
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tion Render terminal turns via Ink <Static> so they flush to native scrollback and escape the viewport-height '…' truncation on long answers. committed/live are derived by a pure per-render filter so they stay disjoint (no ghost line) and <Static> gets a fresh array each render (required for it to emit newly-committed items). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a new ChangesAnswer delta streaming feature
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Model as Streaming model
participant Harness as stream_llm_with_thinking
participant Projector as applyHarnessEvent
participant Transcript as Transcript UI
participant AssistantTurn as AssistantTurn
Model->>Harness: stream text chunk
Harness->>Harness: emit answer.delta(agent, delta, index)
Harness->>Projector: answer.delta event
Projector->>Projector: appendAnswerDelta updates assistant item
Projector->>Transcript: updated transcript items
Transcript->>AssistantTurn: render live item
AssistantTurn-->>Transcript: plain text while streaming
Transcript->>AssistantTurn: render completed item
AssistantTurn-->>Transcript: markdown when complete
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
11 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
miot-harness/src/miot_harness/agents/llm_streaming.py (1)
49-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated
answer.deltaemission logic.The plain-string branch (lines 62-74) and the structured text-block branch (lines 104-116) build an almost identical
HarnessEvent(type="answer.delta", ...)payload. Extracting a small helper would reduce duplication and the risk of the two paths drifting (e.g., one path forgetting to incrementanswer_indexor include a field).♻️ Proposed refactor
+def _emit_answer_delta( + progress: Progress, + run_id: str, + agent_name: str, + delta: str, + index: int, +) -> None: + progress( + HarnessEvent( + run_id=run_id, + type="answer.delta", + message="", + data={"agent": agent_name, "delta": delta, "index": index}, + ) + ) + + async def stream_llm_with_thinking( ... if isinstance(content, str): if content: text_parts.append(content) - progress( - HarnessEvent( - run_id=run_id, - type="answer.delta", - message="", - data={ - "agent": agent_name, - "delta": content, - "index": answer_index, - }, - ) - ) + _emit_answer_delta(progress, run_id, agent_name, content, answer_index) answer_index += 1 continue ... elif btype == "text": delta = block.get("text") or "" if delta: text_parts.append(delta) - progress( - HarnessEvent( - run_id=run_id, - type="answer.delta", - message="", - data={ - "agent": agent_name, - "delta": delta, - "index": answer_index, - }, - ) - ) + _emit_answer_delta(progress, run_id, agent_name, delta, answer_index) answer_index += 1Also applies to: 62-74, 104-116
🤖 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 `@miot-harness/src/miot_harness/agents/llm_streaming.py` at line 49, The `answer.delta` payload construction is duplicated in `llm_streaming.py` across the plain-string branch and the structured text-block branch inside the streaming logic that uses `answer_index`. Extract the shared `HarnessEvent(type="answer.delta", ...)` creation into a small helper and have both branches call it so the fields stay consistent and `answer_index` is incremented in one place.
🤖 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.
Inline comments:
In `@turbo-repo/packages/miot-chat/src/tui/transcript/Transcript.tsx`:
- Around line 61-67: The Transcript component is splitting items into two
independent buckets, which can reorder entries instead of preserving the
original sequence; update Transcript so committed remains the leading prefix and
live remains the trailing tail in the same relative order they appear in
props.items. In Transcript and its render logic, avoid rendering all committed
rows before all live rows; instead, derive the split point from the first live
item and keep the committed prefix intact while appending the live tail
unchanged.
---
Nitpick comments:
In `@miot-harness/src/miot_harness/agents/llm_streaming.py`:
- Line 49: The `answer.delta` payload construction is duplicated in
`llm_streaming.py` across the plain-string branch and the structured text-block
branch inside the streaming logic that uses `answer_index`. Extract the shared
`HarnessEvent(type="answer.delta", ...)` creation into a small helper and have
both branches call it so the fields stay consistent and `answer_index` is
incremented in one place.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 48d8988e-9266-4a8d-9133-9f73ee07d453
📒 Files selected for processing (14)
miot-harness/src/miot_harness/agents/llm_streaming.pymiot-harness/src/miot_harness/runtime/event_types.jsonmiot-harness/src/miot_harness/runtime/events.pymiot-harness/tests/test_events.pymiot-harness/tests/test_llm_streaming.pyturbo-repo/packages/miot-chat/src/tui/__tests__/components/AssistantTurn.test.tsxturbo-repo/packages/miot-chat/src/tui/__tests__/components/TopLine.test.tsxturbo-repo/packages/miot-chat/src/tui/__tests__/components/Transcript.test.tsxturbo-repo/packages/miot-chat/src/tui/__tests__/transcript.project.test.tsturbo-repo/packages/miot-chat/src/tui/chrome/TopLine.tsxturbo-repo/packages/miot-chat/src/tui/transcript/AssistantTurn.tsxturbo-repo/packages/miot-chat/src/tui/transcript/Transcript.tsxturbo-repo/packages/miot-chat/src/tui/transcript/project.tsturbo-repo/packages/miot-harness-client/src/types.ts
Split at the first in-flight item instead of two independent filters, so a completed item (e.g. a finished tool) no longer floats above an earlier still-active chain row. committed stays the leading prefix, live the trailing tail, in original order. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Streaming (answer.delta), markdown rendering, inline loading, and Static-based truncation fix. Backward-compatible; patch release. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Contributor
App preview imageThe latest app preview image for this PR is ready.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Addresses three TUI complaints from the dev team about
@microboxlabs/miot-chat:Each traced to a concrete root cause (diagnosis below), fixed across the harness, the TS client, and the TUI.
Root causes & fixes
answer.completedevent carried only{length}, never the answer text — so the TUI showed nothing until a separate, sometimes-hangingGET /runs/{id}after the stream ended.answer.deltaevents (the answer text the model already streams), mirroringthinking.delta. The TUI accumulates them so the bubble grows token-by-token; the GET is now reconciliation-only.TopLinerendered a spinner pinned to the top of the screen.AssistantTurnspinner.…Transcripthad removed Ink's<Static>and re-painted the whole transcript in the live region every render; content taller than the terminal gets truncated by Ink.<Static>for finished turns so they flush to native scrollback and escape viewport-height truncation.Markdowncomponent existed but was orphaned;AssistantTurnprinted raw text.Markdown(plain text while streaming, formatted on finish).How
<Static>is done safelycommitted(terminal turns) andlive(in-flight tail) are derived by a pure per-render filter onisTerminalItem, so they're always disjoint — no double-paint / ghost line. The filter returns a fresh array each render, which is required for Ink's<Static>to emit newly-committed items (it only re-renders on a newitemsreference — a same-reference mutation is silently dropped; this was caught and fixed during development via a frame probe).Stack touched
miot-harness(Python):answer.deltaliteral + emission instream_llm_with_thinking@microboxlabs/miot-harness-client(TS): literal +AnswerDeltaData@microboxlabs/miot-chat(TS): projector,Transcript,TopLine,AssistantTurnThe event literal stays in lockstep across
events.py,event_types.json, and the TSHARNESS_EVENT_TYPES(parity-tested both sides).Tests
All green, TDD throughout (failing test → fix → pass per task):
test_llm_streaming.py+ event parity + existing streaming suites)miot-harness-client: 24 passedmiot-chat: 408 passed;check-types+lintclean🤖 Generated with Claude Code
Summary by CodeRabbit