fix(platform): chat active turn, canvas/code-block streaming, and tool prompt cleanup#1614
Merged
Merged
Conversation
Short snippets previously shrank to content width; align the outer wrapper to its parent so code blocks render at a consistent width.
The bubble was sized to content inside a flex justify-start parent, so short responses (including code blocks with short snippets) shrank. Set w-full on the assistant bubble so layouts stay stable regardless of content length. Reverts the no-op w-full on the code block wrapper — a block div already fills its parent; the real constraint was the bubble above it.
Gate hiding of completed file-only replies on whether the latest assistant row still trails a new user message, so a prior turn's file-only reply is not hidden while the next turn is generating.
Runs bunx convex dev --once before the long-running dev daemon so first-run binary download, SQLite bootstrap, migrations, and function push happen outside the daemon's 30s port-bind timeout window.
Follow-up to 650ff92. When `maxAssistant === maxUser`, the hide filter can't tell an intra-turn gap (appendFilePart shares order with prompt) from a cross-turn gap (previous turn settled, new send initiated, user message not yet persisted server-side). Use the client-side `pendingMessage` as the unambiguous cross-turn signal — when set, the prior turn's file-only reply stays visible. The previous `>=` behavior caused the prior turn's image to disappear during the markGenerating→saveMessage window; the list's currentLastKey regressed to the old user message, the optimistic bubble got dropped, and the reactKey adoption made the bottom bubble briefly render the previous user message's text before the new user row arrived.
Rename drainSentenceModeMinChars to sentenceModeMinChars and apply the threshold in both streaming and drain phases. When the buffer grows past ~300 chars while the stream is still open, reveal switches to sentence chunks at streamSentenceModeMinCPS (~400) so a big paragraph catches up a sentence at a time instead of trickling at drain pace. Also raise maxChunkChars to 80 so typical long tokens / URLs fit one tick without mid-token flicker.
Clarify that the PDF tool is for persistent downloadable artifacts, not for demo / comparison / visualization pages the user reads inline. Redirects interactive content to HTML/Mermaid/SVG/Markdown code blocks which the Canvas pane renders automatically.
Strip createAgentConfig of its instruction-wrapping logic (human-input enforcement, disambiguation, inline-output preference, approval-card placement, JSON-output suffix) and inline each rule into the description of the tool it governs. The factory now passes instructions through unchanged and no longer threads outputFormat.
larryro
force-pushed
the
fix/chat-active-turn-and-dev-preflight
branch
from
ae2b8d7 to
cad9c25
Compare
📝 WalkthroughWalkthroughThis PR introduces multiple changes across the chat messaging system, agent tools, and backend infrastructure. Key updates include: exposing streaming state to message components via context and implementing auto-opening behavior for Canvas after message streaming completes; adjusting pending-send and active-turn logic in message processing; refining sentence-mode activation and CPS calculation in the stream buffer; standardizing approval-card messaging across agent tool descriptions by removing positional language; refactoring agent configuration to remove Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/platform/convex/agent_tools/documents/document_write_tool.ts (1)
54-79:⚠️ Potential issue | 🟠 MajorRemove stale
pptxreferences fromdocument_writeinstructions.Lines 57, 67, and 72 still instruct use of
pptx, but this PR removes that tool. This can push the agent toward invalid calls and break save flows.🔧 Proposed fix
-• Save generated files (text, docx, pdf, excel, pptx) to the documents hub +• Save generated files (text, docx, pdf, excel) to the documents hub ... -• Generating files — use text, docx, pdf, excel, or pptx tools first +• Generating files — use text, docx, pdf, or excel tools first ... -1. First generate file(s) using text (generate), docx (generate), pdf (generate), etc. +1. First generate file(s) using text (generate), docx (generate), pdf (generate), excel (generate), etc.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/platform/convex/agent_tools/documents/document_write_tool.ts` around lines 54 - 79, The tool description in document_write_tool.ts still mentions the removed "pptx" tool in the description string (see the USE THIS TOOL TO and WHEN TO USE THIS TOOL sections); update the description to remove all "pptx" references (lines mentioning "pptx" in the USE THIS TOOL TO list, the WHEN TO USE THIS TOOL examples, and any other occurrences) so the text only references current generation tools (text, docx, pdf, excel) and does not instruct agents to call a non-existent "pptx" tool; keep the rest of the guidance and wording unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@services/platform/app/features/chat/components/message-bubble/code-block.tsx`:
- Around line 192-212: The effect currently auto-opens canvas for any
non-streaming message (including historical ones); change it to only open when
the message's streaming state transitions from true to false (i.e., observe the
streaming transition). Add a local ref/flag (e.g., prevIsStreamingRef) and track
messageContext.isStreaming across renders inside the useEffect(s) for
messageContext, and only call extractFirstRenderableBlock /
canvasContext.openCanvas when prevIsStreamingRef.current was true and
messageContext.isStreaming is now false (also update the ref after checks); keep
the existing guards (canvasContext, messageId, resolveCanvasType,
autoOpenedMessages) and reference the same symbols: useEffect,
messageContext.isStreaming, autoOpenedMessages, extractFirstRenderableBlock,
canvasContext.openCanvas.
In `@services/platform/app/features/chat/hooks/use-message-processing.ts`:
- Around line 141-144: The hasPendingSendForThread check can return true when
both pendingMessage.* and threadId are undefined; update the predicate in
use-message-processing so comparisons only happen when threadId is defined —
e.g., ensure threadId !== undefined (or threadId != null) before comparing with
pendingMessage.threadId or pendingMessage.arenaThreadIdB; locate the
hasPendingSendForThread variable and adjust its condition to short-circuit with
a defined threadId check.
---
Outside diff comments:
In `@services/platform/convex/agent_tools/documents/document_write_tool.ts`:
- Around line 54-79: The tool description in document_write_tool.ts still
mentions the removed "pptx" tool in the description string (see the USE THIS
TOOL TO and WHEN TO USE THIS TOOL sections); update the description to remove
all "pptx" references (lines mentioning "pptx" in the USE THIS TOOL TO list, the
WHEN TO USE THIS TOOL examples, and any other occurrences) so the text only
references current generation tools (text, docx, pdf, excel) and does not
instruct agents to call a non-existent "pptx" tool; keep the rest of the
guidance and wording unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 29febcb4-94a0-4266-9990-9119bafa20b7
📒 Files selected for processing (19)
services/platform/app/features/chat/components/message-bubble.tsxservices/platform/app/features/chat/components/message-bubble/code-block.tsxservices/platform/app/features/chat/components/message-bubble/message-content-context.tsxservices/platform/app/features/chat/hooks/use-message-processing.tsservices/platform/app/features/chat/hooks/use-stream-buffer.tsservices/platform/convex/agent_tools/documents/document_write_tool.tsservices/platform/convex/agent_tools/files/pdf_tool.tsservices/platform/convex/agent_tools/files/text_tool.tsservices/platform/convex/agent_tools/human_input/request_human_input_tool.tsservices/platform/convex/agent_tools/integrations/integration_tool.tsservices/platform/convex/agent_tools/workflows/create_workflow_tool.tsservices/platform/convex/agent_tools/workflows/run_workflow_tool.tsservices/platform/convex/agent_tools/workflows/save_workflow_definition_tool.tsservices/platform/convex/agent_tools/workflows/update_workflow_step_tool.tsservices/platform/convex/lib/agent_chat/internal_actions.tsservices/platform/convex/lib/create_agent_config.tsservices/platform/convex/workflow_engine/helpers/nodes/llm/execute_agent_with_tools.tsservices/platform/convex/workflow_engine/helpers/nodes/llm/utils/create_llm_result.tsservices/platform/scripts/dev.ts
💤 Files with no reviewable changes (2)
- services/platform/convex/workflow_engine/helpers/nodes/llm/utils/create_llm_result.ts
- services/platform/convex/lib/agent_chat/internal_actions.ts
Comment on lines
+192
to
+212
| useEffect(() => { | ||
| if (!canvasContext) return; | ||
| if (!messageContext?.messageId) return; | ||
| if (messageContext.isStreaming) return; | ||
| if (resolveCanvasType(lang) === 'code') return; | ||
| if (autoOpenedMessages.has(messageContext.messageId)) return; | ||
| const block = extractFirstRenderableBlock(messageContext.messageContent); | ||
| if (!block) return; | ||
| autoOpenedMessages.add(messageContext.messageId); | ||
| canvasContext.openCanvas( | ||
| block.content, | ||
| block.type, | ||
| block.lang || 'code', | ||
| block.lang, | ||
| { | ||
| messageId: messageContext.messageId, | ||
| messageContent: messageContext.messageContent, | ||
| threadId: messageContext.threadId, | ||
| }, | ||
| ); | ||
| }, [canvasContext, messageContext, lang]); |
There was a problem hiding this comment.
Auto-open can fire for historical/non-streamed messages.
On Line 195, the effect only checks current streaming state. Messages rendered from history can already be non-streaming on first mount, so Lines 198-211 may auto-open Canvas immediately, not “after a stream ends.”
💡 Proposed fix (gate on observed streaming transition)
const autoOpenedMessages = new Set<string>();
+const seenStreamingMessages = new Set<string>();
useEffect(() => {
if (!canvasContext) return;
- if (!messageContext?.messageId) return;
- if (messageContext.isStreaming) return;
- if (resolveCanvasType(lang) === 'code') return;
- if (autoOpenedMessages.has(messageContext.messageId)) return;
- const block = extractFirstRenderableBlock(messageContext.messageContent);
- if (!block) return;
- autoOpenedMessages.add(messageContext.messageId);
+ const messageId = messageContext?.messageId;
+ if (!messageId) return;
+
+ if (messageContext.isStreaming) {
+ seenStreamingMessages.add(messageId);
+ return;
+ }
+
+ // Only auto-open after we've observed this message streaming at least once.
+ if (!seenStreamingMessages.has(messageId)) return;
+ if (resolveCanvasType(lang) === 'code') return;
+ if (autoOpenedMessages.has(messageId)) return;
+
+ const block = extractFirstRenderableBlock(messageContext.messageContent);
+ if (!block) {
+ seenStreamingMessages.delete(messageId);
+ return;
+ }
+
+ autoOpenedMessages.add(messageId);
+ seenStreamingMessages.delete(messageId);
canvasContext.openCanvas(
block.content,
block.type,
block.lang || 'code',
block.lang,
{
- messageId: messageContext.messageId,
+ messageId,
messageContent: messageContext.messageContent,
threadId: messageContext.threadId,
},
);
}, [canvasContext, messageContext, lang]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/features/chat/components/message-bubble/code-block.tsx`
around lines 192 - 212, The effect currently auto-opens canvas for any
non-streaming message (including historical ones); change it to only open when
the message's streaming state transitions from true to false (i.e., observe the
streaming transition). Add a local ref/flag (e.g., prevIsStreamingRef) and track
messageContext.isStreaming across renders inside the useEffect(s) for
messageContext, and only call extractFirstRenderableBlock /
canvasContext.openCanvas when prevIsStreamingRef.current was true and
messageContext.isStreaming is now false (also update the ref after checks); keep
the existing guards (canvasContext, messageId, resolveCanvasType,
autoOpenedMessages) and reference the same symbols: useEffect,
messageContext.isStreaming, autoOpenedMessages, extractFirstRenderableBlock,
canvasContext.openCanvas.
Comment on lines
+141
to
+144
| const hasPendingSendForThread = | ||
| !!pendingMessage && | ||
| (pendingMessage.threadId === threadId || | ||
| pendingMessage.arenaThreadIdB === threadId); |
There was a problem hiding this comment.
Guard pending-send matching when threadId is undefined.
At Line 143, pendingMessage.arenaThreadIdB === threadId becomes true when both are undefined, which can falsely mark hasPendingSendForThread during thread transitions.
Proposed fix
- const hasPendingSendForThread =
- !!pendingMessage &&
- (pendingMessage.threadId === threadId ||
- pendingMessage.arenaThreadIdB === threadId);
+ const hasPendingSendForThread =
+ threadId !== undefined &&
+ !!pendingMessage &&
+ (pendingMessage.threadId === threadId ||
+ pendingMessage.arenaThreadIdB === threadId);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const hasPendingSendForThread = | |
| !!pendingMessage && | |
| (pendingMessage.threadId === threadId || | |
| pendingMessage.arenaThreadIdB === threadId); | |
| const hasPendingSendForThread = | |
| threadId !== undefined && | |
| !!pendingMessage && | |
| (pendingMessage.threadId === threadId || | |
| pendingMessage.arenaThreadIdB === threadId); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/features/chat/hooks/use-message-processing.ts` around
lines 141 - 144, The hasPendingSendForThread check can return true when both
pendingMessage.* and threadId are undefined; update the predicate in
use-message-processing so comparisons only happen when threadId is defined —
e.g., ensure threadId !== undefined (or threadId != null) before comparing with
pendingMessage.threadId or pendingMessage.arenaThreadIdB; locate the
hasPendingSendForThread variable and adjust its condition to short-circuit with
a defined threadId check.
The hook now reads pendingMessage from ChatLayoutProvider (added in the cross-turn gap fix), so unit tests that renderHook without the provider were throwing and failing all 33 cases. Add a module-level mock with a mutable pendingMessage slot reset in beforeEach.
…G retry/watchdog Media classification is now byte-driven via `detectMediaMime`, so `.ts` source files no longer get routed to the transcription pipeline. `saveFileMetadata` re-queues non-terminal rows on reuse and stamps `ragQueuedAt`; a new `expireStaleRagQueue` watchdog times out uploads that never reached RAG so clients stop polling forever.
Swap the hand-rolled regex list for Yelp's detect-secrets plugin pipeline (AWS, Stripe, GitHub, JWT, private keys, entropy + keyword detectors). Adds a small post-filter for bareword placeholders (REDACTED, your-api-key-here, member-access refs like config.apiKey) that the KeywordDetector still flags. Tests now cover false-positive avoidance for the TS-config-ref shape that caused the original regression.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Bundle of chat/canvas streaming fixes plus a tool-config cleanup that landed on this branch:
consistent-returnin the stick-to-bottom effect.dev --oncebefore the daemon starts so the first request isn't cold.pptxtool entirely; removegenerateop fromimagetool; tightenpdftool description to file-only use cases.createAgentConfigof its instruction-wrapping (human-input enforcement, disambiguation, inline-output preference, approval-card placement, JSON suffix) and inline each rule into the description of the tool that owns it; drop the now-unusedoutputFormatplumbing.Test plan
request_human_input, an integration write, and a workflow approval — confirm the rules previously injected bycreateAgentConfigstill take effect via the tool descriptions, and that no message references "above" / "below" for approval cards.tale devcold-start: confirm Convex backend is warm before the daemon accepts requests.pptxtool is gone from the registry andimagetool no longer exposesgenerate.Summary by CodeRabbit
New Features
Documentation
Chores