Conversation
… o4-mini, and display reasoning summaries in the UI above the main response in example
|
Closes #11 |
ianmacartney
left a comment
ianmacartney
left a comment
There was a problem hiding this comment.
overall I like the approach. There's a bit of a race folks will run into where old clients will be getting JSON from new servers. Ideally they could release the frontend first and have it handle either string or JSON (maybe assume string in the json parse failure branch?) but I think this pushes us forward
example/convex/chat.ts
Outdated
| model: "gpt-4.1-mini", | ||
| messages: [ | ||
| // o4-mini works best with the Responses API for reasoning | ||
| const response = await (openai as any).responses.create({ |
There was a problem hiding this comment.
why is the cast here necessary?
src/react/index.ts
Outdated
| const lines = response.body | ||
| .pipeThrough(new TextDecoderStream()) | ||
| .pipeThrough(new TransformStream<string, string>({ | ||
| transform(chunk, controller) { | ||
| buffer += chunk; | ||
| const lines = buffer.split('\n'); | ||
| buffer = lines.pop() || ''; | ||
| for (const line of lines) { | ||
| if (line.trim()) { | ||
| controller.enqueue(line); | ||
| } | ||
| } | ||
| }, | ||
| flush(controller) { | ||
| if (buffer.trim()) { | ||
| controller.enqueue(buffer); | ||
| } | ||
| } | ||
| })) | ||
| .getReader(); |
There was a problem hiding this comment.
Am I understanding correctly we are changing the behavior here to break up streams by line for everyone? Can you say more about this?
There was a problem hiding this comment.
The new protocol sends chunks as newline-delimited JSON {"text":"...","reasoning":"..."}\n to support the reasoning field. I added format detection from the first chunk; If it starts with { we enter json mode and split by newlines and otherwise chunks are passed through directly. This way old servers sending plain text still stream correctly and line splitting happens for new servers
WalkthroughThe changes introduce AI reasoning stream support alongside text output across the entire stack. The OpenAI API integration shifts from chat completions to a responses-based model (o4-mini) with reasoning configuration. A new reasoning field is added to the database schema and flows through client streaming, React hooks, and UI components, with separate accumulation and rendering of reasoning and text deltas. Changes
Sequence DiagramsequenceDiagram
participant User
participant Client as Client Component
participant Stream as Streaming Layer
participant API as OpenAI API<br/>(responses)
participant Server as Server/DB
participant UI as ServerMessage<br/>Component
User->>Client: Request chat with reasoning
Client->>API: Stream request (o4-mini + reasoning config)
activate API
API-->>Stream: Event: reasoning_summary delta
API-->>Stream: Event: output_text delta
deactivate API
activate Stream
Stream->>Stream: Accumulate text delta
Stream->>Stream: Accumulate reasoning delta
Stream-->>Server: Write chunk {text, reasoning?}
Server->>Server: Insert to chunks table
Server-->>Stream: Acknowledge
Stream->>Stream: Normalize & encode JSON chunk
Stream-->>Client: Emit {text, reasoning?}
deactivate Stream
activate Client
Client->>Client: Parse JSON chunk or plain-text
Client->>Client: Aggregate text & reasoning separately
Client-->>UI: Update via useStream hook<br/>{text, reasoning, status}
deactivate Client
activate UI
UI->>UI: Render reasoning block (if present)
UI->>UI: Render text block
UI-->>User: Display reasoning + output
deactivate UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![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 (2)
example/src/components/ServerMessage.tsx (1)
38-41: Consider triggering scroll on reasoning updates too.The
scrollToBottomeffect only depends ontextchanges. If reasoning content arrives before text (which can happen with o4-mini's reasoning-first streaming), users may not see the reasoning appearing until text starts flowing.useEffect(() => { - if (!text) return; + if (!text && !reasoning) return; scrollToBottom(); - }, [text, scrollToBottom]); + }, [text, reasoning, scrollToBottom]);src/react/index.ts (1)
178-179: JSON mode detection may have false positives.The detection
firstChunk.trimStart().startsWith("{")assumes any content starting with{is JSON. If a plain-text stream legitimately starts with{(e.g., code output containing JSON), it would incorrectly enter JSON mode and potentially fail parsing.Given the controlled server-side implementation in this PR always sends valid JSON lines when reasoning is involved, this is likely acceptable. However, consider documenting this assumption or using a more robust detection mechanism if the streaming format needs to be more flexible in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/component/_generated/component.tsis excluded by!**/_generated/**
📒 Files selected for processing (6)
example/convex/chat.ts(1 hunks)example/src/components/ServerMessage.tsx(2 hunks)src/client/index.ts(6 hunks)src/component/lib.ts(4 hunks)src/component/schema.ts(1 hunks)src/react/index.ts(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/component/lib.ts (1)
src/client/index.ts (1)
stream(114-186)
example/src/components/ServerMessage.tsx (1)
src/react/index.ts (1)
useStream(27-128)
src/client/index.ts (1)
src/component/schema.ts (1)
StreamStatus(11-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (15)
src/component/schema.ts (1)
17-21: LGTM!The optional
reasoningfield addition to thechunkstable is well-designed. Usingv.optional(v.string())ensures backward compatibility with existing data while supporting the new reasoning feature.example/src/components/ServerMessage.tsx (2)
20-25: LGTM!Clean destructuring of the new
reasoningfield fromuseStream. The hook signature aligns with the updatedStreamBodytype.
45-53: LGTM!The conditional rendering of the reasoning section is well-implemented with appropriate styling to visually separate it from the main response.
src/component/lib.ts (2)
19-42: LGTM!The
addChunkmutation correctly accepts optional reasoning and persists it to the database. The implementation aligns with the schema definition.
96-125: LGTM!The
getStreamTextquery properly aggregates reasoning from all chunks with appropriate fallback to empty string for chunks without reasoning data. The return type correctly reflects that the aggregated reasoning is always a string (never undefined).src/react/index.ts (4)
66-79: LGTM!State management correctly updated to track both text and reasoning, with proper accumulation in the chunk callback.
185-197: LGTM with a note on error handling.The
processBufferfunction correctly handles newline-delimited JSON with proper buffer management for partial lines. The fallback to{ text: line }on parse failure provides graceful degradation, though it silently swallows malformed JSON which could mask bugs during development.
201-221: LGTM!The JSON mode streaming loop correctly handles buffer accumulation, final flush of remaining content, and error conditions. The
while(true)pattern with explicit returns is a valid approach for stream consumption.
222-237: LGTM!Plain text mode passthrough is straightforward and maintains backward compatibility with servers that don't send JSON-formatted chunks.
example/convex/chat.ts (1)
24-42: The o4-mini model and Responses API are confirmed as real, available features from OpenAI. The code implementation usingopenai.responses.create()with thereasoningparameter (effort and summary fields) is consistent with documented API capabilities. No changes needed.src/client/index.ts (5)
189-202: LGTM!The helper correctly forwards reasoning to the mutation. The
reasoning || undefinedconversion on line 199 ensures empty strings aren't unnecessarily stored in the database, which is appropriate since empty reasoning carries no semantic value.
15-22: Well-designed type hierarchy for reasoning support.The type design properly handles the optionality:
StreamChunkallows optional reasoning for flexibility when appending, whileStreamBodyguarantees a string (defaulting to"") for consumers. This provides a clean API contract.
82-91: LGTM!The nullish coalescing (
reasoning ?? "") correctly handles cases where the backend returnsnullorundefined, ensuringStreamBodyalways has a string value forreasoning.
134-161: Logic for accumulating and flushing reasoning looks correct.The normalization, accumulation, and periodic flushing logic properly handles both string and object chunk formats. Using delimiter detection only on
normalized.textis sensible since reasoning content doesn't need sentence-based database persistence triggers.
144-146: Wire format change: stream now emits JSON lines instead of plain text.Lines 144-146 write JSON-encoded chunks (
{"text":"...","reasoning":"..."}) instead of raw text. This is a breaking change for any clients consuming the readable stream directly. Verify that the React client correctly parses this new JSON line format and confirm it handles the format change appropriately.
| let currentReasoning = ""; | ||
| let currentText = ""; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove unused variables.
currentReasoning and currentText are accumulated but never read. They appear to be leftover from an earlier implementation.
- let currentReasoning = "";
- let currentText = "";
-
// Process the streaming response
for await (const event of response) {🤖 Prompt for AI Agents
In example/convex/chat.ts around lines 44-45, the variables currentReasoning and
currentText are declared and accumulated but never read; remove these unused
variables and any related accumulation code (appends/concatenations) so only
live variables remain—delete the declarations and search for any assignments or
+= operations tied to them and remove those lines as well.
There was a problem hiding this comment.
Pull request overview
This PR adds support for streaming reasoning data alongside text content, enabling AI models that provide reasoning chains (like OpenAI's o-series models) to display their thought process to users. The architecture maintains backward compatibility by accepting both plain string chunks and objects with text/reasoning fields.
Key changes:
- Extended the streaming infrastructure to support optional reasoning field alongside text content
- Updated example to demonstrate reasoning display in the UI, shown above the main response
- Modified client/server streaming protocol to auto-detect JSON vs plain text format
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
src/component/schema.ts |
Added optional reasoning field to chunks table |
src/component/lib.ts |
Extended addChunk mutation and getStreamText query to handle reasoning data |
src/component/_generated/component.ts |
Updated generated TypeScript types to include reasoning field |
src/client/index.ts |
Added StreamChunk union type, modified streaming logic to serialize chunks as JSON, and normalize inputs |
src/react/index.ts |
Implemented format auto-detection (JSON vs text), updated state management to track both text and reasoning |
example/src/components/ServerMessage.tsx |
Added UI rendering for reasoning summary above main response, removed "Thinking..." placeholder |
example/convex/chat.ts |
Migrated from Chat Completions API to hypothetical Responses API with o4-mini model and reasoning support |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return { | ||
| text, | ||
| reasoning: reasoning ?? "", | ||
| status: status as StreamStatus |
There was a problem hiding this comment.
[nitpick] Missing trailing comma. The other properties in this object have trailing commas (lines 87-88), so line 89 should also have one for consistency with the codebase style.
| status: status as StreamStatus | |
| status: status as StreamStatus, |
| reasoning: string; | ||
| status: StreamStatus; | ||
| }; | ||
|
|
There was a problem hiding this comment.
Consider adding a JSDoc comment for the new StreamChunk type to document that it supports both legacy string format and the new object format with optional reasoning field. This would help developers understand the API evolution and migration path.
| /** | |
| * Represents a chunk of streamed data. | |
| * | |
| * Supports both the legacy string format and the new object format. | |
| * - Legacy format: `string` (e.g., "Hello world") | |
| * - Object format: `{ text: string; reasoning?: string }` | |
| * | |
| * The object format allows for optional reasoning metadata to be attached to each chunk. | |
| * This dual format supports API evolution: existing code using strings will continue to work, | |
| * while new code can migrate to the richer object format as needed. | |
| */ |
| } catch { | ||
| onUpdate({ text: line }); |
There was a problem hiding this comment.
The catch block silently swallows JSON parse errors. Consider logging the parse error and the problematic line to aid debugging when malformed JSON is received from the server.
| } catch { | ||
| onUpdate({ text: buffer }); |
There was a problem hiding this comment.
The catch block silently swallows JSON parse errors. Consider logging the parse error and the problematic buffer content to aid debugging when malformed JSON is received from the server.
| </div> | ||
| </div> | ||
| )} | ||
| <Markdown>{text}</Markdown> |
There was a problem hiding this comment.
The fallback text "Thinking..." has been removed, but no replacement is provided when text is empty. This means that during the initial "pending" state before any text has streamed, the UI will show an empty <Markdown> component. Consider adding back a loading indicator or placeholder text to provide feedback to users while the stream is starting.
| <Markdown>{text}</Markdown> | |
| {isCurrentlyStreaming && !text ? ( | |
| <div className="text-gray-400 italic">Thinking...</div> | |
| ) : ( | |
| <Markdown>{text}</Markdown> | |
| )} |
| input: [ | ||
| { | ||
| role: "system", | ||
| content: `You are a helpful assistant that can answer questions and help with tasks. | ||
| Please provide your response in markdown format. | ||
|
|
||
| You are continuing a conversation. The conversation so far is found in the following JSON-formatted value:`, | ||
| }, | ||
| ...history, | ||
| ], |
There was a problem hiding this comment.
The parameter input is used instead of messages. The OpenAI Chat Completions API uses messages as the parameter name for the conversation history. If this is intended to use a different/future API, please document which version of the OpenAI SDK supports this interface.
| } | ||
|
|
||
| const firstChunk = firstRead.value; | ||
| const isJsonMode = firstChunk.trimStart().startsWith("{"); |
There was a problem hiding this comment.
The format detection logic assumes that JSON mode starts with {, but this check is performed on the trimmed first chunk which may not represent the complete first JSON object. If the first chunk is very small (e.g., just {), or if there's whitespace before the JSON, this detection could be unreliable. Consider a more robust detection method, such as checking for a complete JSON object or using a header/flag to indicate the format.
| if (event.type === "response.output_text.delta") { | ||
| currentText += event.delta || ""; | ||
| await append({ | ||
| text: event.delta || "", | ||
| reasoning: "", | ||
| }); | ||
| } |
There was a problem hiding this comment.
The event processing loop doesn't handle other event types that might be emitted by the streaming API. If the API emits events other than response.reasoning_summary_text.delta and response.output_text.delta (such as error events, completion events, or other metadata), they will be silently ignored. Consider adding a default case to handle or log unexpected event types for debugging purposes.
| if (event.type === "response.output_text.delta") { | |
| currentText += event.delta || ""; | |
| await append({ | |
| text: event.delta || "", | |
| reasoning: "", | |
| }); | |
| } | |
| else if (event.type === "response.output_text.delta") { | |
| currentText += event.delta || ""; | |
| await append({ | |
| text: event.delta || "", | |
| reasoning: "", | |
| }); | |
| } | |
| // Handle unexpected event types | |
| else { | |
| console.warn( | |
| `Unhandled event type in streaming response: ${event.type}`, | |
| event | |
| ); | |
| } |
| let currentReasoning = ""; | ||
| let currentText = ""; |
There was a problem hiding this comment.
These variables currentReasoning and currentText are declared but never used. They accumulate values from the event stream but are not referenced anywhere else in the code. Consider removing them if they're not needed, or use them if they serve a purpose (e.g., for logging or validation).
Add optional reasoning field to streams, update example to use OpenAI o4-mini, and display reasoning summaries in the UI above the main response in example
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.