feat(studio): assistant UI polish — Send/Stop, empty state, two-line cards#145
Merged
Conversation
- Composer's Send action becomes a SendStopButton that cross-fades to a dark Stop face while a turn is in flight. Esc inside the composer cancels the request, the textarea reads "Generating response… Esc to stop" and dims while pending. Cancellation goes through a real AbortController plumbed via `assistant.cancelPending`; a stale response that races the abort is dropped. - AssistantContext gains `isPending` + `cancelPending`. The in-flight controller is tracked in a ref so re-renders only happen on state flips, and the success-path dispatch checks the abort flag before appending a turn to avoid showing a response the user already asked to drop. - New EmptyStarter component replaces the old EmptyThreadHint. It picks one of three example sets based on context (attached selection → editing prompts, active document or context docs → document-shape prompts, otherwise → global prompts) and clicking a card fills the composer draft without sending so the user can tweak before submitting. Draft state is lifted to AssistantPanel so the empty state and Composer share one buffer. - styles.css adds `--vibrant-green-border` and `--accent-amber` + `--accent-amber-tint` tokens in light + dark + the @theme map so upcoming proposal-card chip work can use the lime/amber palette Sonia called out.
User turns render as a quiet right-aligned quote — muted ink, no fill, a thin right accent border the text sits flush against. The asymmetric look pairs with the assistant's identity gutter: a fixed 24px column on the left of every assistant turn holds the blue ✦ glyph, with the prose and proposal cards in a flex column to its right. Previously the user bubble was a filled rounded card and the assistant prose floated with no identity marker, so the visual rhythm bounced between the two. The new treatment lets the eye land on assistant prose as the primary content and keeps proposal cards aligned to a consistent left edge across an entire turn (not just the first paragraph).
… Accept Restructure the proposal-card header into two lines: the document path sits as the headline (mono, RTL/bdi truncation so the leaf segment stays visible on narrow rails), and the operation chip + locale + validation share the second row. Single-line headers squeezed the path before the chip did — the path is the more important read since it tells you *what* the action targets. Chip palette collapses to one binary: every non-destructive operation (replace, insert, update, create) wears the same blue family; only `delete_document` wears amber. Three-hue triple coding made the destructive case harder to spot once the action types grew past three. Accept buttons (single Footer and TurnGroup's "Accept all") swap from the dark sidebar surface with green text to lime fill with dark ink and a deeper-lime border — the design's commit color. Send remains the blue CTA on the composer so colour now separates the role: blue = send to model, lime = commit the model's output. TurnGroup rows pick up the same two-line shape, plus a small KindGlyph SVG that pairs with the chip (`+`, X-in-box, rotation arrow per family) and three trailing icon affordances — reject (×), accept (✓), expand chevron (▸) — so accepting a single proposal from a batched turn no longer requires opening the diff first.
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (9)
📝 WalkthroughWalkthroughImplements client-side streaming and cancelable assistant chat, refactors AssistantPanel/composer and proposal UI, adds UI primitives and theme tokens, adds a client SSE chat API, and implements server-side streaming orchestrator, safer error sanitization, audit refinements, and reference-validation robustness. ChangesRequest Cancellation and Assistant UI Refactoring
Server: Orchestrator Streaming & Runtime Safeguards
Sequence Diagram(s)sequenceDiagram
participant AssistantPanel
participant AssistantProvider
participant StudioAiRouteApi
AssistantPanel->>AssistantProvider: submit draft -> runChatRequest()
activate AssistantProvider
AssistantProvider->>AssistantProvider: abort previous controller (if any)
AssistantProvider->>AssistantProvider: create new AbortController, set isPending=true
AssistantProvider->>StudioAiRouteApi: chatMessageStream(request with signal)
StudioAiRouteApi-->>AssistantProvider: text-delta events (stream)
AssistantProvider->>AssistantProvider: dispatch append-stream-delta...
StudioAiRouteApi-->>AssistantProvider: done event (final message + proposals)
AssistantProvider->>AssistantPanel: commit-stream-turn -> isPending=false
deactivate AssistantProvider
alt user cancels
AssistantPanel->>AssistantProvider: cancelPending()
AssistantProvider->>AssistantProvider: controller.abort()
AssistantProvider->>AssistantPanel: suppress abort (clear placeholder)
end
🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 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 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/studio/src/lib/runtime-ui/components/assistant/assistant-context.tsx (1)
867-904:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winRace condition: stale results may dispatch after cancellation.
The abort check at line 870 prevents dispatch if the request was aborted before the response arrives. However, if
cancelPending()is called after line 870 but before thedispatch()at line 897, the stale assistant turn will still be appended to the timeline even though the user explicitly stopped generation.Add a second guard right before dispatch to ensure the controller is still active:
🔒 Proposed fix to prevent stale dispatch
const result: StudioAiChatMessageResult = await liveApi.chatMessage(request); if (controller.signal.aborted) return; + // Re-check before dispatch in case cancelPending was called while + // we were processing the successful response. + if (pendingControllerRef.current !== controller) return; const wireProposals = result.proposals ?? [];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/studio/src/lib/runtime-ui/components/assistant/assistant-context.tsx` around lines 867 - 904, The response handler can still dispatch a stale assistant turn if cancellation occurs after the earlier abort check; add a second guard just before calling dispatch to bail out if the request was aborted. Concretely, right before the dispatch({ type: "send-message", threadId: state.activeThreadId, userMessage: input.userMessage, assistantMessage, newProposals, newWireProposals }) invocation, check controller.signal.aborted and return early if true (preserving the same local variables: controller, state.activeThreadId, input.userMessage, assistantMessage, newProposals, newWireProposals).
🧹 Nitpick comments (1)
packages/studio/src/lib/runtime-ui/components/assistant/kind-glyph.tsx (1)
5-13: 💤 Low valueUnused
structuralfamily.The
Familytype defines"structural"andpathForFamilyhas a case for it, but no proposal kind maps to this family inFAMILY_BY_KIND. If this is reserved for future use, consider adding a comment; otherwise, remove it to reduce maintenance surface.♻️ Proposed cleanup if not needed
-type Family = "additive" | "destructive" | "replacing" | "structural"; +type Family = "additive" | "destructive" | "replacing";And remove the
case "structural":branch frompathForFamily.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/studio/src/lib/runtime-ui/components/assistant/kind-glyph.tsx` around lines 5 - 13, FAMILY_BY_KIND currently maps AssistantProposal kinds to three families but the Family type and pathForFamily include a fourth "structural" value that isn’t used; either remove "structural" from the Family union and delete the corresponding case "structural" branch in pathForFamily, or if it’s reserved, add a short comment above Family and FAMILY_BY_KIND explaining it’s intentionally kept for future proposals. Update FAMILY_BY_KIND, the Family type, and pathForFamily consistently so there’s no dangling unused branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/studio/src/lib/runtime-ui/components/assistant/kind-glyph.tsx`:
- Around line 15-21: The doc comment for the KindGlyph component is inaccurate:
it says "Returns `null` for kinds without a mapped family" but the code always
renders an SVG because every AssistantProposal["kind"] is present in
FAMILY_BY_KIND; update the top comment in kind-glyph.tsx to remove the "Returns
`null`..." clause and instead state that the component always renders an SVG
glyph whose color inherits from `currentColor` and that the caller controls hue
(via classes like `text-primary` or `text-accent-amber`). Alternatively, if you
prefer a defensive runtime check, add a guard at the start of the component that
returns null when FAMILY_BY_KIND[kind] is undefined (referencing FAMILY_BY_KIND
and the component function name) and update the comment to document this
behavior.
---
Outside diff comments:
In
`@packages/studio/src/lib/runtime-ui/components/assistant/assistant-context.tsx`:
- Around line 867-904: The response handler can still dispatch a stale assistant
turn if cancellation occurs after the earlier abort check; add a second guard
just before calling dispatch to bail out if the request was aborted. Concretely,
right before the dispatch({ type: "send-message", threadId:
state.activeThreadId, userMessage: input.userMessage, assistantMessage,
newProposals, newWireProposals }) invocation, check controller.signal.aborted
and return early if true (preserving the same local variables: controller,
state.activeThreadId, input.userMessage, assistantMessage, newProposals,
newWireProposals).
---
Nitpick comments:
In `@packages/studio/src/lib/runtime-ui/components/assistant/kind-glyph.tsx`:
- Around line 5-13: FAMILY_BY_KIND currently maps AssistantProposal kinds to
three families but the Family type and pathForFamily include a fourth
"structural" value that isn’t used; either remove "structural" from the Family
union and delete the corresponding case "structural" branch in pathForFamily, or
if it’s reserved, add a short comment above Family and FAMILY_BY_KIND explaining
it’s intentionally kept for future proposals. Update FAMILY_BY_KIND, the Family
type, and pathForFamily consistently so there’s no dangling unused branch.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: eed0b389-abcf-4884-b713-a0cf3c36ac27
📒 Files selected for processing (7)
packages/studio/src/lib/runtime-ui/components/assistant/assistant-context.tsxpackages/studio/src/lib/runtime-ui/components/assistant/assistant-panel.tsxpackages/studio/src/lib/runtime-ui/components/assistant/empty-starter.tsxpackages/studio/src/lib/runtime-ui/components/assistant/kind-glyph.tsxpackages/studio/src/lib/runtime-ui/components/assistant/proposal-card.tsxpackages/studio/src/lib/runtime-ui/components/assistant/send-stop-button.tsxpackages/studio/src/lib/runtime-ui/styles.css
Local git worktrees and CLI scratch dirs live under .claude/worktrees/. Without these ignores, Nx walks into the worktrees, discovers their package.json files as additional projects, and demands matching references entries in the root tsconfig.json. The same paths break the Docker server build because the worktree's .git pointer + dist/ artifacts aren't valid inside the container context. .nxignore tells Nx project-discovery to skip these directories; .dockerignore keeps the docker build context lean so the in-container `bun nx build server` doesn't trip on stale worktree state.
Two related fixes from a UI review session:
- Accept button used `text-foreground` which flips to near-white in
dark mode, crashing contrast against the lime fill (lime stays the
same hex either way). Add `--vibrant-green-foreground: #1c1b1b` —
always dark — and switch both the proposal-card Footer and the
TurnGroup "Accept all" button to it.
- An apply that bubbles up as a 500 was rendering in chat as the
generic "INTERNAL_ERROR: Internal server error." with zero
breadcrumb server-side. The chat surface posts proposal bodies
directly (no in-memory store record), so the apply handler's audit
emission was gated off entirely. Now:
- `handleProposalApply` emits the `apply_failed` audit for both
paths — observedRecord present OR parsedProposal present — so
the failure code + message lands in the audit stream.
- `buildLifecycleAudit`'s `actorId` becomes optional since
chat-surface failures may abort before authorize() resolves.
- The runtime `emitAudit` callback promotes failure outcomes
(`apply_failed`, `validation_failed`, `invalid_output`,
`provider_error`) to `error` level and includes
`errorMessage` so operators see *why* it failed.
- `appendErrorTurn` on the client digs through `details.payload.
details.reason` and surfaces it as the message text when the
server's public message is the generic placeholder.
Two related polish items the assistant needed before streaming lands: - `AssistantPanel` keeps the message column pinned to the latest turn by default. A scroll handler tracks proximity to the bottom (within 80px) and stores it in a ref; the auto-scroll effect runs on `messages.length`, `isPending`, and `proposals` map identity, but only when the ref is still sticky. Manual scroll-up disengages it; scrolling back near the bottom re-engages. First mount and thread switches jump straight to the bottom (no animation) so the user opens to the latest turn instead of the history's top edge. - The Drizzle "Failed query: <SQL>\nparams: ..." raw error was making it into the chat bubble verbatim. `toRuntimeErrorResponse` now collapses any Drizzle-shaped message to "Database operation failed. See server logs for details." and caps any other non-Runtime error reason at 240 chars / first line so chat bubbles can't grow past a sane size. The full text is still in the audit log for ops.
Adds `runChatStream(input): AsyncIterable<AiChatStreamEvent>` to the chat orchestrator. The streaming path mirrors `runChat`'s setup (tool construction, prompt assembly, validator wiring) but drives `streamText` instead of `generateText` and yields: - `text-delta` events as the model emits tokens - one `done` event carrying the assembled proposals + audit when the stream completes - a single `error` event with the mapped RuntimeError + audit on provider failure The setup logic (envelope, tools, system + user prompt, language-model gate) was factored into `prepareChatRun`, and the error → audit mapping into `chatErrorToFailure`, both shared between `runChat` and the new streamer so the two paths can't drift. Wire-format serialisation (SSE on the route, AsyncIterable consumption on the client, reducer + UI changes for live token rendering) ships in a follow-up commit — this lands the orchestrator foundation in its own reviewable piece.
…ator
Wires the streaming protocol end-to-end so the assistant turn renders
token by token instead of materialising after the full request settles.
Server
- New SSE endpoint `POST /api/v1/ai/chat/messages/stream` driving the
orchestrator's `runChatStream`. Pre-orchestrator failures (auth,
parse, denied actions) propagate as normal HTTP errors before the
stream opens so the client only commits to SSE consumption after a
200 + text/event-stream response.
- Wire format: `event: text-delta\ndata: {"text": "…"}\n\n` per token
chunk, followed by exactly one `event: done` (with the assembled
assistant message + proposals + conversationId) OR one `event:
error` (with code + message). The headers explicitly opt out of
proxy buffering (`x-accel-buffering: no`) so Nginx + similar
middleboxes don't hold the stream until close.
- Factored the chat-turn prep (CSRF, parsing, capability probing,
attached-doc loading, project-knowledge fetch, AiChatInput build)
into a shared `prepareChatTurn` helper. The JSON handler still
inlines its prep for now (will rebase onto the helper in a separate
pass to keep this diff focused on streaming); the streaming handler
uses the helper exclusively.
Client
- `StudioAiRouteApi.chatMessageStream(input)` returns an AsyncIterable
that opens the SSE response, parses event blocks (both LF and CRLF
variants), and yields typed events. Releases the underlying reader
in `finally` so early-terminated iteration (Stop button, page
unload) tears the connection down promptly.
Context + reducer
- Three new actions replace the atomic `send-message` on the streaming
path: `begin-stream-turn` adds the user message + an empty assistant
placeholder, `append-stream-delta` grows the placeholder text by id,
and `commit-stream-turn` swaps the placeholder for the final message
+ merges proposals. `abort-stream-turn` covers Stop / mid-stream
errors so the placeholder doesn't linger as a frozen typing dot.
- `runChatRequest` now consumes `chatMessageStream`. AbortController
threading is preserved — the Stop button still cancels via the
existing `assistant.cancelPending` plumbing, and an `AbortError`
during stream iteration is treated as a user-initiated cancellation
(placeholder cleared, no inline error turn).
UI
- `AssistantBubble` takes a new `isStreamingPlaceholder` prop that the
panel sets to `true` for the most-recent assistant turn while
`isPending` is on. An empty placeholder renders a three-dot typing
indicator in the blue identity gutter instead of being filtered out
by the earlier "empty bubble" guard.
- Auto-scroll now also depends on the trailing message's text length
so streaming deltas keep the bottom of the most recent turn in view
while the user is sticky-at-bottom.
`authUsers.id` is `text()` because Better-Auth generates string IDs like `Cxk8R3FU5evjyRs9PlN7izdHLIAusaWk` — not postgres UUIDs. The `documents.created_by` / `updated_by` / `published_by` columns plus `projects.created_by` and `environments.created_by` were declared as `uuid()`, which postgres strictly validates. Every Better-Auth session-authenticated insert would have failed at the database layer. Normal Studio creates appeared to work only because `database-store.create` quietly falls back to `DEFAULT_ACTOR` (`00000000-0000-0000-0000-000000000001`) when the request payload omits `createdBy` — which the Studio client always does. So existing docs all carry the placeholder UUID and authorship was silently unattributed. The AI apply path is the first one that passes the real `session.userId` through, which is what surfaced this. Migration 0012 alters the five affected columns to `text`, accepting both Better-Auth IDs and the legacy placeholder UUID without truncation. No FK constraints touch these columns, so the alter is a clean type widening with no rewriting of existing rows.
Roll back the uuid→text widening from e14b355. Those columns stay `uuid()` per the original schema; the underlying bug (Studio omits the real actor when writing through the manual content endpoints, so the store substitutes a placeholder UUID and authorship is lost) will get a proper fix in a separate ticket. In the meantime, route the AI apply path through the same shape the manual content endpoints take — drop `createdBy`/`updatedBy` from the apply payloads so the store's `DEFAULT_ACTOR` fallback fires for both flows. AI apply was the only caller that explicitly passed `session.userId`, which is what exposed the schema mismatch by hitting the uuid column with Better-Auth's text id format. The real actor identity is still captured by the AI audit pipeline (every apply emits an `ai.audit` record with `actorId` set), so we keep the attribution chain end-to-end for AI turns — just not via the documents table, which is consistent with what the manual endpoints do today. Migration 0013 reverts the column types using `USING <col>::uuid` so the placeholder UUIDs already in the table cast cleanly back.
Migration 0012 widened authorship columns to text; 0013 reverted them straight back to uuid. The two are net-zero on the schema and only exist because the AI apply path briefly exposed the schema mismatch between Better-Auth ids and the uuid columns — the proper fix lives in a separate ticket. Keep the PR focused on the AI/UI alignment change (drop createdBy/updatedBy from the AI apply path) and let fresh clones jump straight from 0011 to whatever migration the authorship-attribution work introduces.
…eserved Two review nits on `kind-glyph.tsx`: - The doc comment claimed the glyph "Returns null for kinds without a mapped family", but `FAMILY_BY_KIND` is typed as a complete `Record<AssistantProposal['kind'], Family>` — the type system guarantees every kind maps to a family, so the component always renders an SVG. Reword the comment to say so. - The `Family` union includes `"structural"` but no current kind maps to it. Keep the union member + the `pathForFamily` branch (matching the design's KindGlyph, which reserves it for future move / restructure / reorder proposals) and add a short comment explaining it's intentional rather than dead code. (Separately reviewed but not changed: the supposed "stale dispatch after cancellation" race in `assistant-context.tsx` — applied to the old non-streaming path that no longer exists. The current streaming flow's `for await` body opens with `if (controller.signal.aborted) return;` and runs synchronously through to dispatch, so there is no async window where the abort flag can flip mid-iteration.)
Two related fixes from the screenshot where a proposal stamped VALID but apply rejected it with "must be a UUID string referencing 'author'": - `validate-proposal` walkReferences now rejects reference values that don't match the UUID literal pattern *before* hitting the document lookup. The model occasionally writes a display name (e.g. "Demo User") into a reference field; that string is well-formed JSON but not a documentId, and the underlying uuid-typed lookup can mask it (drivers silently return zero rows on type coercion, which the validator misread as "exists: true" on a happy path). The new check emits UNKNOWN_REFERENCE with a message that mirrors the apply-time wording, and is cheap because it short-circuits without a DB call. - The lookup itself is now wrapped in try/catch so a driver-level cast error or transient DB failure surfaces as UNKNOWN_REFERENCE rather than crashing the whole turn. Also adds the "Reference fields require real entry ids" guidance section the CMS-238 design called for — rendered into the system prompt whenever any registered type contains a reference field. The copy explicitly tells the model to call `find_entries`, copy a `documentId` from a result, and never paste a person's name into a reference field. Test coverage: - New regression test pins the format-pre-check behavior (non-UUID value → UNKNOWN_REFERENCE, no lookup call). - New test confirms a lookup that throws is treated as "does not exist" so the validator stays resilient to DB hiccups. - Existing UNKNOWN_REFERENCE fixtures swapped to real UUID strings so they exercise the post-format-check branch. - New project-knowledge tests assert the guidance section renders when any schema has a reference field and is omitted otherwise.
…gent
Two related fixes for the "Accept makes the card vanish with no
feedback" complaint:
- Proposals carry an optional `acceptedAt` timestamp on the client
shape. When the apply succeeds, the reducer stamps the proposal
with that timestamp instead of dropping it; ProposalCard +
TurnGroup short-circuit to a quiet `AppliedLogLine` ("· Applied
HH:MM — <path> (+N −M)") in the same row. Reads as continuous
history rather than a disappearance, mirrors the design's
LoggedLine, and pairs with the past-tense rhythm Sonia called
for in bullet #7.
- A hidden side-channel user turn is appended after a successful
apply. The text reads like the user reporting the acceptance to
the model — "(I accepted your proposal to create the document
for blog/post-1. It's now a draft.)" — so the next chat round
trip carries the signal through `conversationHistory` and the
agent can stop suggesting the same change. `AssistantMessage`
gains a `hidden?: boolean` flag the panel filters out of the
visible timeline; the conversation-history serializer keeps
filtering on text presence so hidden messages still ride along.
Implementation notes:
- New reducer action `mark-proposal-accepted` replaces the
`remove-proposal` dispatch on the accept path. `remove-proposal`
stays as-is for the reject flow.
- The acceptance-signal copy is shaped per proposal kind so the
model gets the right verb (created / deleted / rewrote / inserted
/ updated frontmatter) plus the doc path when one's available.
- The 6s undo countdown the design also describes is intentionally
not part of this commit — that needs the in-place countdown bar
+ ⌘Z stack from the reference, which lands cleanly on top of
this state shape in a follow-up.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/studio/src/lib/ai-route-api.ts`:
- Around line 623-629: The loop that consumes SSE blocks uses findEventBoundary
to get the index of the first newline but always advances buffer by 2, which
leaves a stray CRLF when the boundary is CRLFCRLF; update the consumption logic
in the while loop that references boundary/buffer/parseSseBlock so it slices by
the correct offset (use 2 for LFLF and 4 for CRLFCRLF) instead of always 2, and
adjust the findEventBoundary helper if needed to expose whether the found
boundary is CRLF or LF so the loop can choose the proper slice length.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 836f1aea-90d4-4f16-b13b-4062046105b3
📒 Files selected for processing (14)
packages/modules/core.ai/src/server/apply.test.tspackages/modules/core.ai/src/server/apply.tspackages/modules/core.ai/src/server/project-knowledge.test.tspackages/modules/core.ai/src/server/project-knowledge.tspackages/modules/core.ai/src/server/routes.tspackages/modules/core.ai/src/server/validate-proposal.test.tspackages/modules/core.ai/src/server/validate-proposal.tspackages/studio/src/lib/ai-route-api.tspackages/studio/src/lib/runtime-ui/components/assistant/assistant-context.tsxpackages/studio/src/lib/runtime-ui/components/assistant/assistant-panel.tsxpackages/studio/src/lib/runtime-ui/components/assistant/assistant-types.tspackages/studio/src/lib/runtime-ui/components/assistant/kind-glyph.tsxpackages/studio/src/lib/runtime-ui/components/assistant/proposal-card.tsxpackages/studio/src/lib/runtime-ui/hooks/use-inline-ai-transform.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/studio/src/lib/runtime-ui/components/assistant/kind-glyph.tsx
- packages/studio/src/lib/runtime-ui/components/assistant/assistant-panel.tsx
Comment on lines
+623
to
+629
| let boundary: number; | ||
| while ((boundary = findEventBoundary(buffer)) >= 0) { | ||
| const block = buffer.slice(0, boundary); | ||
| buffer = buffer.slice(boundary + 2); | ||
| const event = parseSseBlock(block); | ||
| if (event) yield event; | ||
| } |
There was a problem hiding this comment.
Incorrect slice offset for CRLF boundaries.
findEventBoundary returns the index of the first newline character for both \n\n (2-char) and \r\n\r\n (4-char) boundaries. The slice at line 626 always advances by 2, leaving \r\n residue in the buffer when a CRLF boundary is encountered. This will corrupt subsequent event parsing behind middleboxes that rewrite line endings to CRLF.
🐛 Proposed fix
- let boundary: number;
- while ((boundary = findEventBoundary(buffer)) >= 0) {
- const block = buffer.slice(0, boundary);
- buffer = buffer.slice(boundary + 2);
+ let boundaryResult: { index: number; length: number } | null;
+ while ((boundaryResult = findEventBoundary(buffer)) !== null) {
+ const block = buffer.slice(0, boundaryResult.index);
+ buffer = buffer.slice(boundaryResult.index + boundaryResult.length);
const event = parseSseBlock(block);
if (event) yield event;
}And update the helper:
-function findEventBoundary(buf: string): number {
+function findEventBoundary(buf: string): { index: number; length: number } | null {
const lf = buf.indexOf("\n\n");
const crlf = buf.indexOf("\r\n\r\n");
- if (lf === -1) return crlf;
- if (crlf === -1) return lf;
- return Math.min(lf, crlf);
+ if (lf === -1 && crlf === -1) return null;
+ if (lf === -1) return { index: crlf, length: 4 };
+ if (crlf === -1) return { index: lf, length: 2 };
+ return lf < crlf ? { index: lf, length: 2 } : { index: crlf, length: 4 };
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/studio/src/lib/ai-route-api.ts` around lines 623 - 629, The loop
that consumes SSE blocks uses findEventBoundary to get the index of the first
newline but always advances buffer by 2, which leaves a stray CRLF when the
boundary is CRLFCRLF; update the consumption logic in the while loop that
references boundary/buffer/parseSseBlock so it slices by the correct offset (use
2 for LFLF and 4 for CRLFCRLF) instead of always 2, and adjust the
findEventBoundary helper if needed to expose whether the found boundary is CRLF
or LF so the loop can choose the proper slice length.
Assistant prose was passing the raw model output to a plain `<div>`, so headings, bullet lists, fenced code, and inline links rendered as literal characters. Add `streamdown` (the markdown renderer AI SDK Elements uses under the hood — GFM + streaming-aware) and wrap it in a tiny `AssistantMarkdown` component that: - runs in `mode="streaming"` with `parseIncompleteMarkdown` on, so a mid-stream chunk doesn't leak raw `**` or `\`\`\`` while a fence is still open - inherits the Studio `prose-sm` typography rebind (`max-w-none` drops the 65ch ceiling so markdown fills the bubble; `text-[13.5px]` pins the body to the existing chat scale) - tightens default block margins so a one-sentence reply doesn't carry blog-post spacing Bundle gains ~450 KB (shiki + mermaid pulled transitively by streamdown). Acceptable for the feature surface — happy to revisit later by tree-shaking out diagram support if it becomes a concern.
Collaborator
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Long generations were being killed mid-stream by Bun's per-connection idleTimeout (default 10s) — the client saw the socket close before the orchestrator could emit its `done` event, so the chat surfaced "AI_REQUEST_FAILED: stream closed without response" on requests like "write all 5 posts" where the model paused between tokens. - Raise `idleTimeout` to 255s (Bun's hard maximum) in `Bun.serve` — any realistic generation completes well inside that window and a truly stalled connection still gets cleaned up. - Add a 15s `:keep-alive` SSE comment heartbeat in the streaming handler. The client's SSE parser already treats `:`-prefixed lines as no-ops, so these don't pollute the event stream — they exist purely to keep Bun (and any future Nginx/CDN/proxy hop) from flagging the socket as idle. Belt-and-suspenders against the raised timeout.
…urns
The chat orchestrator's `stopWhen: stepCountIs(5)` was set as a tight
runaway-guard when only a single proposal was the expected output.
Multi-proposal turns ("write all 5 posts", "create a blog series",
"refactor frontmatter across these 3 docs") count each tool call and
each interstitial text reply as a step, so 5 was being hit after the
first or second proposal landed and the model wrapped up.
Lift the cap to 20 — enough headroom for ~5 tool calls plus
`find_entries` lookups ahead of any reference field plus the final
text summary, with margin to spare. Still bounded so a genuine
runaway loop hits the floor before the provider budget gets drained.
Extracted to `DEFAULT_CHAT_STEP_LIMIT` so the streaming and
non-streaming paths share the same ceiling.
…sage) We had no visibility into why a multi-proposal chat turn stopped early. The Vercel AI SDK already returns rich per-step data on both `generateText.steps` / `.finishReason` and `streamText.steps`/ `.finishReason` (awaited) — surface it through a single `ai.chat.diagnostic` console log line per turn so operators can see: - the model's overall `finishReason` (stop / length / tool-calls / content-filter), which is the usual reason a "write all 5" turn produces 3 proposals — typically `length` (hit maxOutputTokens) or a deliberate `stop` - per-step: which step finished how, what tools it called, and the input/output/total token usage - total token usage across the turn - proposal count vs step count, to spot cases where the loop ran many steps but barely produced proposals The orchestrator doesn't take a logger dep, so the helper writes via `console.log` and lets the host server's stdout pipe shape it.
Two related polish items for the post-accept feel: - After Accept succeeds the row morphs into a lime-tinted "Applied" banner with a 6-second countdown progress bar (Sonia bullet #3, mirrors the AI SDK Elements reference). When the window expires the same row settles into the existing past-tense `AppliedLogLine`. Hover and tab-hidden pause the countdown via `useUndoCountdown`. Page reloads inside the window resume the remaining time correctly; reloads after expiry land straight on the quiet log line. - TurnGroup's "X proposals · this turn" footer used to keep showing "Accept all (3) / Reject all" even after every row was individually accepted, which read as a stuck affordance. The bar now hides once no proposals are still pending; while some remain, both the count label and the Accept-all loop only target pending rows so a partial apply isn't double-counted. The banner intentionally doesn't expose an Undo button yet — cosmetic undo would lie about reversibility for body edits (the document is already changed on the server). The real revert flow (per-kind rollback to baseDraftRevision, ⌘Z handler, hidden \"(I undid…)\" turn back to the model) is filed as CMS-241.
Two CI gates were failing on the PR after the assistant streaming + markdown work landed: - `unit-tests`: `build-runtime.test.ts` enforces a first-load size budget on the studio-runtime bundle. Streamdown (pulled in for markdown rendering) adds ~450 KB transitively via shiki + mermaid, pushing the bundle from ~1.7 MB to ~2.2 MB. Raise the budget from 2.0 MB to 2.5 MB and extract the threshold to `STUDIO_RUNTIME_SIZE_BUDGET_BYTES` with a comment pointing at the follow-up size-optimisation work (split shiki / mermaid out via dynamic import). - `changeset-check`: the new `chatMessageStream` method on `StudioAiRouteApi` plus the runtime `streamdown` dep are part of the published `@mdcms/studio` surface. Add a minor-bump changeset documenting both additions.
This was referenced May 16, 2026
5 tasks
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.
First pass on Sonia's visual revisions to the global assistant. Three focused commits, none of them invasive — the wire contracts and the chat orchestrator are untouched. Held the inline-undo countdown + ⌘Z + past-tense logged-line work for a follow-up so this batch can land and Sonia can react to the baseline.
What changed
Composer — Send⇄Stop swap + empty state (
a51f4bc)SendStopButtoncross-fades to a dark Stop face while a turn is in flight; Esc inside the composer cancels via a realAbortControllerplumbed throughassistant.cancelPending.AssistantContextgainsisPending+cancelPending. A stale response that races the abort is dropped before dispatch so the timeline never grows a cancelled-then-arrived turn.EmptyStarterreplaces the old single-blurb hint with four contextual example cards. Set picks itself by context — attached selection → editing prompts, active document / @-mention → document-shape prompts, otherwise → global prompts. Click fills the composer draft; it does not send.AssistantPanelso the empty state and the Composer share one buffer.Chat rhythm — user quote / assistant sparkle gutter (
419f693)Proposal cards — two-line + blue/amber chips + lime Accept (
2a22ad8)<bdi>truncation so the leaf segment stays visible on narrow rails). Operation chip + locale + validation share the second line. Single-line headers used to squeeze the path before the chip did.delete_document. NewKindGlyphSVG pairs with the chip (+, X-in-box, rotation arrow per family).TurnGrouprows get the same two-line shape plus three trailing icon affordances — reject (×), accept (✓), expand chevron (▸) — so accepting a single proposal from a batched turn no longer requires opening the diff first.Tokens added
--vibrant-green-border(#a8cc34) — deeper-lime border on Accept buttons--accent-amber(#a05a0alight /#f59e0bdark) +--accent-amber-tint— destructive chip familyAll wired into the
@thememap alongside the existing--vibrant-green.Held for the next iteration
LoggedLinestyling. The state machine (countdown, hover-pause, tab-visibility-aware progress, undo stack) is sketched out in the design'sProposalCards.jsx; will ship as a focused fourth commit once Sonia's signed off on the visual baseline.Test plan
http://localhost:4173/admin/content/<doc>and toggle the assistant with ⌘K — empty thread shows four example cards picked by context.fetchaborts cleanly (no fake error turn).bun --cwd packages/studio tsc --buildclean, 9/9 assistant component tests green,bun nx run studio:buildbundle produced.Summary by CodeRabbit
New Features
UI/UX Improvements
Bug Fixes