taOStalk s1: content_blocks types + renderContent dispatcher - #2154
Conversation
Add ContentBlock union (text, thinking, tool_call, status, unknown) and content_blocks field to Message/MessageRow interfaces. Add a dispatcher in renderContent() that switches on block.kind when content_blocks is non-empty, falling through to the markdown path otherwise. Ship the unknown-kind fallback (dim unsupported-block line) as the slice-2 seam; known kind cases are stubs for separate cards.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughStructured content block types are added to messages, routed through ChangesStructured message rendering
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant MessageList
participant renderContent
participant renderContentBlock
MessageList->>renderContent: pass message text and content_blocks
renderContent->>renderContentBlock: dispatch each non-empty block
renderContentBlock-->>renderContent: return unsupported block placeholder
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
PR Summary by QodoAdd ContentBlock union and renderContent dispatcher for structured chat turns
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
|
nemotron-super review VERDICT: Pass Automated first-pass review by the nemotron-super lane. The lead still reviews before merge. |
|
nemotron-ultra-orB review VERDICT: Ready with minor concerns — placeholder fallback renders for all known block kinds; catch-all type allows arbitrary keys.
Automated first-pass review by the nemotron-ultra-orB lane. The lead still reviews before merge. |
Code Review by Qodo
1. Blocks hide real content
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| function renderContentBlock(block: ContentBlock, index: number): React.ReactElement { | ||
| switch (block.kind) { | ||
| case "text": | ||
| case "thinking": | ||
| case "tool_call": | ||
| case "status": | ||
| default: | ||
| return ( | ||
| <div key={`block-${index}`} className="text-shell-text-tertiary text-[12px]"> | ||
| unsupported block: {block.kind} | ||
| </div> | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| export function renderContent(text: string, content_blocks?: ContentBlock[]) { | ||
| if (content_blocks && content_blocks.length > 0) { | ||
| return content_blocks.map((block, i) => renderContentBlock(block, i)); | ||
| } |
There was a problem hiding this comment.
1. Blocks hide real content 🐞 Bug ≡ Correctness
renderContent() takes the content_blocks path whenever it is non-empty, but renderContentBlock() currently renders every kind (including "text") as an "unsupported block" placeholder. Any message that arrives with populated content_blocks will therefore display placeholders instead of its actual content.
Agent Prompt
### Issue description
`renderContent()` prioritizes `content_blocks` when present, but the dispatcher currently returns the unsupported-block placeholder for all kinds (even known kinds like `text`). This makes structured messages unreadable.
### Issue Context
- `MessageList` now passes `msg.content_blocks` into `renderContent()`.
- Unit tests added in this PR lock in placeholder output for known kinds.
### Fix Focus Areas
- Implement minimal renderers for known kinds (at least `text` -> render `block.text` through existing markdown/inline pipeline), and reserve the placeholder only for truly unknown kinds.
- Alternatively, gate the `content_blocks` path: if a block kind is not supported yet, fall back to the legacy markdown rendering using the `text` argument.
#### References
- desktop/src/apps/MessagesApp.tsx[266-305]
- desktop/src/apps/chat/MessageList.tsx[526-544]
- desktop/src/apps/chat/__tests__/render-helpers.test.tsx[61-90]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| export type ContentBlock = | ||
| | TextContentBlock | ||
| | ThinkingContentBlock | ||
| | ToolCallContentBlock | ||
| | StatusContentBlock | ||
| | { kind: string; [key: string]: unknown }; |
There was a problem hiding this comment.
2. Catch-all block overlaps kinds 🐞 Bug ⚙ Maintainability
ContentBlock includes a { kind: string; ... } catch-all member that overlaps all known kind
literals, so malformed shapes like { kind: 'text' } can type-check as ContentBlock. This weakens
compile-time guarantees for future per-kind renderers and encourages scattered runtime validation.
Agent Prompt
### Issue description
The current `ContentBlock` union ends with a broad member (`{ kind: string; [key: string]: unknown }`) that can also match known kinds. This undermines the union’s ability to enforce required fields for known blocks.
### Issue Context
The project is in `strict` mode, so preserving discriminated-union validation is valuable for future block-specific renderers.
### Fix Focus Areas
- Replace the overlapping catch-all with an explicit `UnknownContentBlock` that does not overlap known kinds, e.g. `{ kind: 'unknown'; raw_kind: string; raw: Record<string, unknown> }`.
- Convert/validate API data at the boundary into `ContentBlock` (known blocks) or `UnknownContentBlock` (fallback) so renderers can rely on required fields.
#### References
- desktop/src/apps/MessagesApp.tsx[161-198]
- desktop/tsconfig.json[1-20]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
nemotron-ultra-kilo review VERDICT: Acceptable with architectural debt — known content block kinds render as "unsupported block" placeholders per slice-2 seam design, but no TODO/FIXME marks the intentional deferral.
Automated first-pass review by the nemotron-ultra-kilo lane. The lead still reviews before merge. |
|
Merging. Verified against the frozen content-blocks wire contract, not just the green tick: typed blocks (text/thinking/tool_call/status) plus the open |
…2154) Add ContentBlock union (text, thinking, tool_call, status, unknown) and content_blocks field to Message/MessageRow interfaces. Add a dispatcher in renderContent() that switches on block.kind when content_blocks is non-empty, falling through to the markdown path otherwise. Ship the unknown-kind fallback (dim unsupported-block line) as the slice-2 seam; known kind cases are stubs for separate cards.
…2154) Add ContentBlock union (text, thinking, tool_call, status, unknown) and content_blocks field to Message/MessageRow interfaces. Add a dispatcher in renderContent() that switches on block.kind when content_blocks is non-empty, falling through to the markdown path otherwise. Ship the unknown-kind fallback (dim unsupported-block line) as the slice-2 seam; known kind cases are stubs for separate cards.
1 participant
Autonomous build of board card tsk-xocdcd.
Add ContentBlock union (text, thinking, tool_call, status, unknown)
and content_blocks field to Message/MessageRow interfaces. Add a
dispatcher in renderContent() that switches on block.kind when
content_blocks is non-empty, falling through to the markdown path
otherwise. Ship the unknown-kind fallback (dim unsupported-block line)
as the slice-2 seam; known kind cases are stubs for separate cards.
Files:
desktop/src/apps/MessagesApp.tsx | 66 +++++++++++++++++++++-
desktop/src/apps/chat/MessageList.tsx | 4 +-
.../apps/chat/tests/render-helpers.test.tsx | 41 ++++++++++++++
3 files changed, 109 insertions(+), 2 deletions(-)
Summary by CodeRabbit