Skip to content
Closed
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
136 changes: 136 additions & 0 deletions desktop/src/apps/chat/ContentBlocks.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
import React, { useState, useId } from "react";
import { ChevronDown, ChevronUp } from "lucide-react";
import { renderInline } from "../MessagesApp";

/* ------------------------------------------------------------------ */
/* ContentBlock type */
/* ------------------------------------------------------------------ */

/**
* Structured content block for session-turn messages (taOStalk #1953).
*
* The `kind` field discriminates the block type. Known kinds are
* rendered by dedicated components; unknown kinds fall through to
* the dim "unsupported block" fallback. The index signature keeps
* the type permissive so server-sent blocks with extra fields still
* type-check without forcing every optional field to be listed.
*/
export type ContentBlock = {
kind: string;
text?: string;
collapsed?: boolean;
call_id?: string;
name?: string;
input_preview?: string;
status?: string;
options?: string[];
[key: string]: unknown;
};

/* ------------------------------------------------------------------ */
/* TextBlock — reuses the existing markdown renderer */

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

1. Em dashes in comments 📜 Skill insight ✧ Quality

The new ContentBlocks.tsx file contains em dashes () in comment headers, which the checklist
prohibits in public-facing text (including code comments). This can cause inconsistent typography
and violates the no-em-dash policy.
Agent Prompt
## Issue description
The file introduces em dash characters (`—`) in comment headers, which are disallowed.

## Issue Context
The compliance rule bans em dashes in public-facing text, explicitly including code comments/docstrings.

## Fix Focus Areas
- desktop/src/apps/chat/ContentBlocks.tsx[31-32]
- desktop/src/apps/chat/ContentBlocks.tsx[45-46]
- desktop/src/apps/chat/ContentBlocks.tsx[91-92]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

/* ------------------------------------------------------------------ */

/**
* Renders a `{kind:"text"}` block by delegating to the same
* `renderInline` markdown pipeline used for ordinary chat messages,
* so formatting (bold, lists, code, links) is identical.
*/
export function TextBlock({ text }: { text: string }) {
const id = useId();
return <>{renderInline(text, `text-${id}`)}</>;
}

/* ------------------------------------------------------------------ */
/* ThinkingBlock — collapsed-by-default disclosure, dim styling */
/* ------------------------------------------------------------------ */

/**
* Renders a `{kind:"thinking"}` block as a collapsed-by-default
* disclosure. The toggle is a button with `aria-expanded` and
* `aria-controls` so screen readers announce the expand/collapse
* state. Thinking text is dimmed to match the Store/Images design
* bar (subtle tertiary text on a muted surface).
*/
export function ThinkingBlock({
text,
collapsed = true,
}: {
text: string;
collapsed?: boolean;
}) {
const id = useId();
const [open, setOpen] = useState(() => !collapsed);
const contentId = `thinking-content-${id}`;

return (
<div className="mt-2 first:mt-0">
<button
type="button"
onClick={() => setOpen(!open)}
className="flex items-center gap-1.5 text-[12px] font-medium text-shell-text-tertiary hover:text-shell-text transition-colors"
aria-expanded={open}
aria-controls={contentId}
>
<span>Thinking</span>
{open ? <ChevronUp size={12} /> : <ChevronDown size={12} />}
</button>
{open && (
<div
id={contentId}
className="mt-1.5 text-[13px] text-shell-text-tertiary whitespace-pre-wrap opacity-70"
>
{text}
</div>
)}
</div>
);
}

/* ------------------------------------------------------------------ */
/* UnknownBlock — dim fallback for unrecognised kinds */
/* ------------------------------------------------------------------ */

/**
* Renders any block whose `kind` is not yet handled by a dedicated
* component. Shows a dim "unsupported block" line with the kind name
* so the user can see something was emitted even if the UI doesn't
* know how to render it yet. This is the slice-2 seam.
*/
export function UnknownBlock({ kind }: { kind: string }) {
return (
<div className="text-[12px] text-shell-text-tertiary opacity-70">
Unsupported block: {kind}
</div>
);
}

/* ------------------------------------------------------------------ */
/* Dispatcher */
/* ------------------------------------------------------------------ */

/**
* Maps a single content block to its renderer via a `kind` switch.
* Unknown kinds fall through to `UnknownBlock`. Each returned element
* gets a stable key based on its position so React can reconcile
* block arrays without duplicate-key warnings.
*/
export function renderContentBlock(
block: ContentBlock,
index: number,
): React.ReactElement {
switch (block.kind) {
case "text":
return <TextBlock key={`block-${index}`} text={block.text ?? ""} />;
case "thinking":
return (
<ThinkingBlock
key={`block-${index}`}
text={block.text ?? ""}
collapsed={block.collapsed}
/>
);
default:
return <UnknownBlock key={`block-${index}`} kind={block.kind} />;
}
Comment on lines +55 to +135

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

3. Index keys break state 🐞 Bug ≡ Correctness

renderContentBlock uses index-based keys (block-${index}), but ThinkingBlock has local open/closed
state, so inserting/removing/reordering blocks can cause React to reuse a previous block’s state for
a different block.
Agent Prompt
### Issue description
`renderContentBlock()` keys each block by its array index. Because `ThinkingBlock` stores UI state (`open`) via `useState`, React can incorrectly carry open/closed state across blocks when the block list changes (insertions/removals/reordering), causing the wrong block to appear expanded/collapsed.

### Issue Context
This is a common React reconciliation pitfall: index keys are only safe for strictly static lists that never change membership/order.

### Fix Focus Areas
- desktop/src/apps/chat/ContentBlocks.tsx[55-88]
- desktop/src/apps/chat/ContentBlocks.tsx[118-135]

### Suggested approach
1. Introduce/require a stable identity on `ContentBlock` (e.g., `id`, or reuse an existing stable field like `call_id` if it is guaranteed unique per block).
2. Compute keys from that stable identity (e.g., `key={block.id}` or `key={`${block.kind}:${block.call_id}`}`).
3. Only fall back to index if you can prove the list is immutable in order and membership (otherwise keep index as last-resort fallback only).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

}
Comment on lines +118 to +136

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. Blocks not rendered 🐞 Bug ≡ Correctness

ContentBlocks.tsx introduces renderContentBlock/TextBlock/ThinkingBlock, but the existing chat
message body still renders msg.content via renderContent(), so structured blocks won’t appear
anywhere in the current MessagesApp message view.
Agent Prompt
### Issue description
`ContentBlocks.tsx` defines block renderers and a dispatcher (`renderContentBlock`), but nothing in the current chat message view calls it. The UI still renders message bodies via `renderContent(msg.content)`.

### Issue Context
If structured/session-turn blocks are expected to render in the main chat timeline, the message model and/or rendering path needs to detect and map blocks to `renderContentBlock` instead of always treating the body as a single markdown string.

### Fix Focus Areas
- desktop/src/apps/chat/ContentBlocks.tsx[118-136]
- desktop/src/apps/chat/MessageList.tsx[524-542]

### Suggested approach
1. Decide how blocks are represented on `MessageRow` (e.g., `content_blocks: ContentBlock[]` or embedded in `metadata`).
2. In `MessageList`, render either:
   - `content_blocks.map(renderContentBlock)` when blocks exist, or
   - fallback to `renderContent(msg.content)` when they don’t.
3. Ensure the new field is typed and populated from the server payload.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Loading