Skip to content

Studio chat completion: post-implementation review of correctness, complexity and design #2306

Open

Description

@kantord

TL;DR

Once the chat completion feature (#2160) has landed, do one deliberate technical review of the finished implementation: is it correct, is it as simple as it can be, and is the design sound? Findings go into new issues; this issue is not for fixing things in place.

Blocked by #2160. Don't start before all of its sub-issues are merged.

Why a separate review

#2160 ships in six small PRs (#2161 to #2166), each reviewed on its own. Nobody looks at the whole. The pieces interact in ways no single PR sees, and the largest change swaps the chat message box for a rich-text editor, which touches everything that sends a prompt.

Terms

  • Composer: the chat message box.
  • Chip: an inline @name or /name token in the composer that behaves as one unit but is sent as plain text.
  • BFF: Studio's web server layer between the browser and the daemon.
  • Arrival prompt / seed: text pre-filled into the composer when Studio is opened with ?prompt=..., or from the OS share sheet (see Studio: text shared to Studio is lost when signing in from a signed-out share #2305).

Review checklist

Correctness

  • Plain-text round trip. A prompt with chips is sent as the same plain text the daemon would get from typing it (Studio chat: slash-command and agent-mention completion #2160 acceptance).
  • Existing composer behavior unchanged: drafts, attachments, queued messages, paste, Enter to send, and IME composition (must not send).
  • Arrival prompt still works in the new editor. ?prompt= and share-sheet text still pre-fill the composer, and &send=1 still only offers a one-click confirmation and never sends on its own. A prompt starting with / must stay prefill-only: it must not run a command or auto-pick a menu item. This rule is written down in the prototype (chat-seed.ts#L1-L22); Studio's implementation is chat-seed.ts. None of Studio chat: slash-command and agent-mention completion #2160 to Studio chat completion: help rows, guidance, and user docs #2166 mention it, so nothing else guards it. Related: Studio: text shared to Studio is lost when signing in from a signed-out share #2305.
  • Menu rules. / opens its menu only at the start of a message, only in an existing session; selecting never sends the prompt; an unknown hand-typed command still passes through unchanged.
  • Capability gating. With the slashCommands and agents capabilities off, the composer behaves exactly as before and the help page says why.
  • BFF routes. Anonymous callers can't list agents or commands; unsupported deployments return an explanation, not an empty success; a session's command list reflects its own sources.
  • Accessibility of the menus: keyboard use, roles, and screen-reader announcements.
  • Side-thread composer behaves the same as the main one.

Complexity

  • Is the new editor dependency worth it? The prototype uses TipTap. Check bundle size, upgrade burden, and whether a lighter approach meets the same acceptance. Studio chat completion: composer editor foundation with plain-text chips #2162 records this decision; confirm it still holds.
  • Every abstraction and edge case earns its place. Remove handling that serves no real scenario; collapse layers that only forward calls.
  • Duplicated logic between the main composer and the side-thread composer, and between the @ and / menus.
  • Did it make the big chat files bigger? chat-workspace.tsx was already about 3,400 lines (see Large batch of tiny follow-up fixes #2303, section B).

Design

  • Where the seams are. Editor, suggestion sources, BFF projections, and capability checks should be separable and testable on their own, not tangled into the chat workspace.
  • Deviations from the prototype (stacklok/mecatl-prototypes#39) are deliberate and written down, not accidental.
  • Studio stays behind the BFF, with no daemon credentials in the browser, and only uses the published SDK.

Output

A short written review in a comment here, plus one new issue per real problem found. Anything that needs a design decision or end-to-end change goes under #2302; small fixes go to #2303.

Done when

  • Every box above is checked or has a linked issue.
  • A summary comment says whether the implementation is fine to leave as is.

Related: #2160, #2305.

Fully or partially written by an AI agent.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions