Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 65 additions & 1 deletion desktop/src/apps/MessagesApp.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,44 @@ export function resolveAuthorDisplayState(
return "removed";
}

interface TextContentBlock {
kind: "text";
text: string;
}

interface ThinkingContentBlock {
kind: "thinking";
text: string;
collapsed?: boolean;
}

interface ToolCallContentBlock {
kind: "tool_call";
call_id: string;
name: string;
input_preview?: string;
status: "running" | "done" | "error";
result_preview?: string;
}

interface StatusContentBlock {
kind: "status";
text: string;
}

/**
* Structured message content for taOStalk session turns.
* Known kinds are handled by dedicated block components (separate cards);
* any unrecognized kind falls through to the unknown-block fallback in
* renderContent, which is the slice-2 seam.
*/
export type ContentBlock =
| TextContentBlock
| ThinkingContentBlock
| ToolCallContentBlock
| StatusContentBlock
| { kind: string; [key: string]: unknown };
Comment on lines +192 to +197

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

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


interface Message {
id: string;
channel_id: string;
Expand All @@ -167,6 +205,7 @@ interface Message {
/** Parent message id when this message is a thread reply. */
thread_id?: string;
content_type?: "text" | "canvas" | string;
content_blocks?: ContentBlock[];
metadata?: {
canvas_id?: string;
canvas_url?: string;
Expand Down Expand Up @@ -217,7 +256,32 @@ export function relativeTime(ts: number | string, nowMs: number = Date.now()): s
return new Date(ms).toLocaleTimeString(undefined, { hour: "2-digit", minute: "2-digit" });
}

export function renderContent(text: string) {
/**
* Dispatch a single content block to its renderer. Known kinds (text,
* thinking, tool_call, status) are dispatched to dedicated block components
* in separate cards; until those land, they fall through to the unknown
* fallback. This is the slice-2 seam: add a case per kind and return the
* block component.
*/
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));
}
Comment on lines +266 to +284

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

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

// Split on fenced code blocks first, then apply inline markdown to non-code segments.
const result: (string | React.ReactElement)[] = [];
const fenceRegex = /```(?:[^\n]*)?\n([\s\S]*?)```/g;
Expand Down
4 changes: 3 additions & 1 deletion desktop/src/apps/chat/MessageList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { ReactionBar } from "./ReactionBar";
import { resolveAgentEmoji } from "@/lib/agent-emoji";
import { startDrag, endDrag } from "@/shell/dnd/dnd-bus";
import { renderContent, dayLabel, relativeTime, toMs, resolveAuthorDisplayState } from "../MessagesApp";
import type { ContentBlock } from "../MessagesApp";
import type { AttachmentRecord } from "@/lib/chat-attachments-api";
import { displayAuthor } from "./format-author";
import type { LiveAgent, ArchivedAgentEntry, Channel } from "./types";
Expand All @@ -41,6 +42,7 @@ export interface MessageRow {
content: string;
thread_id?: string;
content_type?: "text" | "canvas" | string;
content_blocks?: ContentBlock[];
metadata?: {
canvas_id?: string;
canvas_url?: string;
Expand Down Expand Up @@ -538,7 +540,7 @@ export const MessageList = forwardRef<MessageListHandle, MessageListProps>(funct
: "text-shell-text"
}`}
>
{renderContent(msg.content)}
{renderContent(msg.content, msg.content_blocks)}
{msg.state === "pending" && (
<span className="ml-1 text-shell-text-tertiary">
...
Expand Down
41 changes: 41 additions & 0 deletions desktop/src/apps/chat/__tests__/render-helpers.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,47 @@ describe("renderContent", () => {
expect(a?.getAttribute("href")).toBe("https://example.com");
expect(a?.getAttribute("target")).toBe("_blank");
});

it("dispatches to content_blocks when non-empty", () => {
const { container } = render(<div>{renderContent("", [{ kind: "text", text: "hello" }])}</div>);
expect(container.textContent).toContain("unsupported block: text");
});

it("falls through to markdown when content_blocks is empty", () => {
const { container } = render(<div>{renderContent("hello world", [])}</div>);
expect(container.textContent).toContain("hello world");
});

it("renders unknown fallback for unrecognized block kinds", () => {
const { container } = render(<div>{renderContent("", [{ kind: "question", text: "what?" }])}</div>);
expect(container.textContent).toContain("unsupported block: question");
});

it("renders unknown fallback for all known kinds (separate cards)", () => {
const { container } = render(
<div>{renderContent("", [
{ kind: "text", text: "hi" },
{ kind: "thinking", text: "thinking...", collapsed: true },
{ kind: "tool_call", call_id: "c1", name: "Bash", status: "running" as const },
{ kind: "status", text: "done" },
])}</div>,
);
expect(container.textContent).toContain("unsupported block: text");
expect(container.textContent).toContain("unsupported block: thinking");
expect(container.textContent).toContain("unsupported block: tool_call");
expect(container.textContent).toContain("unsupported block: status");
});

it("renders one fallback line per block", () => {
const { container } = render(
<div>{renderContent("", [
{ kind: "text", text: "a" },
{ kind: "status", text: "b" },
])}</div>,
);
const text = container.textContent || "";
expect(text.match(/unsupported block/g)?.length).toBe(2);
});
});

describe("dayLabel", () => {
Expand Down
Loading