taOStalk s1: ToolCallBlock + StatusBlock/question renderers (cards 5+6) - #2275
Conversation
|
Warning Review limit reached
Next review available in: 36 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe PR adds public question content types and dedicated ChangesContent block rendering
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
PR Summary by QodotaOStalk: add ToolCallBlock and StatusBlock/question renderers
AI Description
Diagram
High-Level Assessment
Files changed (6)
|
There was a problem hiding this comment.
🧹 Nitpick comments (2)
desktop/src/apps/MessagesApp.tsx (1)
265-286: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMerge the
statusandquestioncases.Both cases render
<StatusBlock>and differ only in the cast type. Combine them with a fallthrough and reuse the exportedStatusLikeBlocktype fromStatusBlock.tsxto avoid duplicating the cast logic.♻️ Proposed refactor
- case "status": - return ( - <StatusBlock block={block as StatusContentBlock} key={`block-${index}`} /> - ); - case "question": - return ( - <StatusBlock block={block as QuestionContentBlock} key={`block-${index}`} /> - ); + case "status": + case "question": + return ( + <StatusBlock + block={block as StatusContentBlock | QuestionContentBlock} + key={`block-${index}`} + /> + );🤖 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 `@desktop/src/apps/MessagesApp.tsx` around lines 265 - 286, Merge the “status” and “question” branches in renderContentBlock into one fallthrough case, and cast the block once using the exported StatusLikeBlock type from StatusBlock.tsx before passing it to StatusBlock. Preserve the existing key and rendering behavior.desktop/src/components/StatusBlock.tsx (1)
13-33: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNeither renderer announces status changes to assistive technology.
StatusBlockandToolCallBlockboth render state transitions (status text, question prompts, tool-call icon changes) as plain markup with no live region. It is unconfirmed whetherMessagesApp's parent message list already wraps updates in a live region.
desktop/src/components/StatusBlock.tsx#L13-L33: addrole="status"andaria-live="polite"to the wrappingdivso status and, in particular, "question" prompts are announced when they appear.desktop/src/components/ToolCallBlock.tsx#L61-L94: addrole="status"andaria-live="polite"aroundStatusIndicator(or its parentCard) so transitions betweenrunning,done, anderrorare announced.Confirm first whether an ancestor component already provides a live region for the message list; if so, this may already be covered and no change is needed.
🤖 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 `@desktop/src/components/StatusBlock.tsx` around lines 13 - 33, Check whether MessagesApp or an ancestor already provides a live region for these message updates; if so, make no changes. Otherwise, add role="status" and aria-live="polite" to the StatusBlock wrapper in desktop/src/components/StatusBlock.tsx:13-33, and apply the same announcement attributes around StatusIndicator or its parent Card in desktop/src/components/ToolCallBlock.tsx:61-94 so status transitions are announced.
🤖 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 `@desktop/src/apps/MessagesApp.tsx`:
- Around line 265-286: Merge the “status” and “question” branches in
renderContentBlock into one fallthrough case, and cast the block once using the
exported StatusLikeBlock type from StatusBlock.tsx before passing it to
StatusBlock. Preserve the existing key and rendering behavior.
In `@desktop/src/components/StatusBlock.tsx`:
- Around line 13-33: Check whether MessagesApp or an ancestor already provides a
live region for these message updates; if so, make no changes. Otherwise, add
role="status" and aria-live="polite" to the StatusBlock wrapper in
desktop/src/components/StatusBlock.tsx:13-33, and apply the same announcement
attributes around StatusIndicator or its parent Card in
desktop/src/components/ToolCallBlock.tsx:61-94 so status transitions are
announced.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c1fceb9e-3f4d-46d9-8060-8d940c45a68c
📒 Files selected for processing (6)
desktop/src/apps/MessagesApp.tsxdesktop/src/apps/chat/__tests__/render-helpers.test.tsxdesktop/src/components/StatusBlock.tsxdesktop/src/components/ToolCallBlock.tsxdesktop/src/components/__tests__/StatusBlock.test.tsxdesktop/src/components/__tests__/ToolCallBlock.test.tsx
Code Review by Qodo
1. Unchecked block payloads
|
| case "tool_call": | ||
| return ( | ||
| <ToolCallBlock block={block as ToolCallContentBlock} key={`block-${index}`} /> | ||
| ); |
There was a problem hiding this comment.
1. Unchecked block payloads 🐞 Bug ☼ Reliability
renderContentBlock dispatches to ToolCallBlock/StatusBlock based only on block.kind and then type-casts, so malformed/legacy blocks with kind:"tool_call"|"status"|"question" can render as empty/undefined UI instead of hitting the explicit unsupported-block fallback. Because chat messages accept/persist arbitrary content_blocks JSON, the UI cannot assume required fields (e.g., status, name, text) or enum values are valid at runtime.
Agent Prompt
### Issue description
`renderContentBlock()` switches on `block.kind` and then casts to `ToolCallContentBlock` / `StatusContentBlock` / `QuestionContentBlock` without runtime validation. Since `content_blocks` is persisted/echoed as arbitrary JSON, malformed blocks with a recognized `kind` can bypass the unsupported fallback and render with missing fields (e.g. blank text, missing status indicator).
### Issue Context
- Dedicated renderers were added for `tool_call`, `status`, and `question`.
- Backend accepts and stores `content_blocks` without schema validation, so shape/enum mismatches can occur (legacy messages, external clients, bugs upstream).
### Fix Focus Areas
- desktop/src/apps/MessagesApp.tsx[271-293]
- desktop/src/components/ToolCallBlock.tsx[15-18]
- desktop/src/components/ToolCallBlock.tsx[61-94]
- tinyagentos/routes/chat.py[367-381]
- tinyagentos/chat/message_store.py[448-473]
### Suggested fix approach
1. Add runtime type guards (or a small zod-style validator if already used in the repo) for each supported block kind:
- `isToolCallBlock(x): x is ToolCallContentBlock` (checks `call_id`, `name`, `status` in {running,done,error})
- `isStatusBlock(x): x is StatusContentBlock` (checks `text` is string)
- `isQuestionBlock(x): x is QuestionContentBlock` (checks `text` is string; `options` optional string[])
2. In `renderContentBlock`, only dispatch to the dedicated component if the guard passes; otherwise render the unsupported fallback (ideally including the kind + a “malformed payload” hint).
3. In `ToolCallBlock.StatusIndicator`, add a `default` branch that renders a neutral indicator (or a small `?`/warning icon) and consider showing the raw status string so unknown statuses are visible/debuggable.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
nemotron-super review VERDICT: PASS Automated first-pass review by the nemotron-super lane. The lead still reviews before merge. |
|
Reviewed at 3e06de6; changelog added by me at 42f5aab. VERIFIED:
SEQUENCING NOTE: this and #2282 both edit MessagesApp.tsx's renderContent dispatch and render-helpers.test.tsx. Both are armed; whichever lands second will likely need a rebase rather than merging clean. That is expected, not a defect - I am watching for the conflict transition and will rebase the loser. dependency-audit red is the fleet-wide cryptography CVE issue (tsk-4kifi6 / dec-26leeh), not this PR, and is not a required check on dev. Auto-merge armed on green. |
| <AlertTriangle size={12} className="text-red-400" /> | ||
| </span> | ||
| ); | ||
| } |
There was a problem hiding this comment.
WARNING: Missing default branch in StatusIndicator switch
If a malformed block reaches this component with an unexpected status value, the switch falls through and the function returns undefined, causing the status indicator to render nothing.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| raise review-request notifications through the store (and therefore through | ||
| SSE and web push) instead of a raw database insert (#2280). | ||
| - Chat renders `tool_call` and `status` content blocks: tool calls as a | ||
| collapsible detail with an ARIA disclosure contract, status (and the |
There was a problem hiding this comment.
SUGGESTION: "collapsible detail" is inaccurate
ToolCallBlock is a static card with truncated content, not a collapsible element. Consider rephrasing to match the actual implementation.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (7 files)
Fix these issues in Kilo Cloud Reviewed by step-3.7-flash · Input: 123.1K · Output: 23.8K · Cached: 684.3K |
CARD TITLE (intent, not commit subject): taOStalk s1: ToolCallBlock + StatusBlock/question renderers (cards 5+6)
Autonomous build of board card tsk-3lkyq2.
Files:
docs/agent-manual/11-files-api.md | 35 --------
docs/agent-manual/index.md | 1 -
docs/design/taosgo-mesh-join-foundation.md | 8 +-
docs/taos-agent-manual.md | 35 --------
tests/test_agent_manual_compiled.py | 2 +-
tinyagentos/taosnet/mesh.py | 3 +-
tinyagentos/taosnet/mesh_credentials.py | 3 +-
14 files changed, 330 insertions(+), 102 deletions(-)
Summary by Gitar
ToolCallBlockcomponent to render tool call content blocks with status indicators and previewsStatusBlockcomponent to render status and question content blocks with reply hintsToolCallBlockandStatusBlockintorenderContentBlockdispatch logicQuestionContentBlocktype definition and updated exportsToolCallBlock,StatusBlock, and updated render helper testsThis will update automatically on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Tests