Conversation
…egister The AI drawer already drives two surfaces — content fields and the code editor — through one contract: a JSON array of actions, each naming a refKey that resolves to a handle a component registered on mount. Nothing wrote that contract down, so adding Studio as a third surface reads as new integration work rather than as meeting three existing obligations. docs/ai-action-contract.md specifies the wire format, what a surface owes the contract, the refKeys Studio adds, and where preview and undo stop being inherited. It records that the schemas in systemInstructions.ts are unreferenced exports — the live instruction is server-side in the MCP client — which is the argument for typing this before a third surface joins it. docs/studio.md's postMessage lists were missing wrapElementInLink, unwrapElementLink and DYNAMIC_EDIT_REQUEST, which contradicted the command count cited by the new doc. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
QA Review — ✅ PASSNo linked issue found — change-only QA.
Suggested Cypress coverageNo spec is needed for this PR itself since it changes no runtime behavior. For future coverage once Studio's AI wiring lands, |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| Two things a reader should not be surprised by: | ||
|
|
||
| - **The local engine is wider than the contract.** `src/engine/actionTypes.ts` declares `SET_VALUE·CLICK·FOCUS·BLUR·NAVIGATE·CUSTOM` and `handlers.ts` implements all six — `CUSTOM` invokes an arbitrary method on a registered handle. Only `SET_VALUE` and `NAVIGATE` are reachable from the drawer today, but the others would execute if wired. Treat the table above as the contract and the engine as an implementation detail that currently exceeds it. | ||
| - **The host is tolerant about framing.** `AIDrawer.tsx:140` strips markdown fences and `:143` wraps a bare object in an array. Do not rely on either. |
There was a problem hiding this comment.
🟡 Line citations throughout this doc have already drifted from the current source
Checked several against the checked-out tree: AIDrawer.tsx:140/:143 here are actually lines 150/153 now; the route-gate regex cited as :71-74 is at 81-84; stageLayoutSourceUpdate cited as useLayoutReorderState.ts:945 is at 955; createFile/resolvePathPart cited as files.js:406-415/:673-698 are at 420/706. The technical claims themselves check out (skip list, context shapes, handler bug at handlers.ts:8 are all accurate) but the pinpoint line numbers this doc leans on for precision are already stale at merge time. Consider citing function/symbol names instead of line numbers for citations likely to drift, or re-verify line numbers right before merge.
Code Review — ✅ No blockers · 🟡 1 warning(s) — see inline comments |
Negative QANo reproducible edge-case failures found on the surfaces this PR changes. Also checked and working correctly
|
The doc is handed to whoever builds the model side, so it is restructured
around what they receive and what they may return: the request payload
field by field, the two action types, a capability-to-refKey table for
Studio mode, and the rules that misfire silently.
Drops CREATE_FILE. Creating code files is now explicitly out of the
contract, and a capability that needs one answers with SYSTEM_OUTPUT
naming the file the user must create.
Adds the response envelope, which was missing and is the costliest thing
to get wrong: the body must be { "data": [...] }, since the app reads
aiResponse.data and a bare array reads as undefined.
Also documents that refRegistry entries are display strings rather than
parseable JSON, the value/sourceValue rule that protects Parsley
bindings, the eleven datatypes that never register, and which fields are
live today versus specified for Studio.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Code Review — ✅ No blockers |
The drawer sits above Studio's content/layout toggle rather than inside it: full mode is the union of both and the default for a user entitled to both, so one chat changes copy and code and a single response may write a content field and a code file together. mode therefore stops being a capability gate and becomes routing context. writable, which is permissions, is the only thing that narrows what may be written. Replaces the per-mode rule with the routing decision the model actually has to make on a bound element: copy goes to the content field, markup and structure to the view file, appearance to the stylesheet. Notes that a mixed turn saves layout first and stops if that half fails, so it can land the file write without the field write. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Code Review — ✅ No blockers |
writable was a per-refKey permission list the app would have had to
compute and keep in sync. availableModes already exists, is already
derived from usePermission("CODE") and usePermission("UPDATE") at
StudioWrapper.tsx:116-126, and carries the same information in one
field.
mode cannot carry it: a user entitled to both gets
["full","content","layout"] and toggles freely, so mode reports where
they are looking rather than what their role permits. The doc now says
which of the two is the permission signal, because conflating them is
the mistake this section previously made in the other direction.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mode did no work. The routing rule keys off what the user asked to change and off the slot's own isDynamic/sourceValue, never off the toggle position, and the two selection shapes already say what the user is pointing at -- a content selection carries fieldZuid, a layout selection carries codeId and slots. Against that it carries a real risk: a model reading mode "content" will plausibly refuse a code change the user is entitled to make, which is the restriction this document already had to remove once. availableModes remains as the only gate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
| } | ||
| ``` | ||
|
|
||
| A bare array as the response body reads as `undefined` and breaks the drawer — the app reads `aiResponse.data` (`AIDrawer.tsx:138-143`). Two tolerances exist and should not be relied on: a ` ```json ` fence around a string `data` is stripped, and a lone object is wrapped in an array. |
There was a problem hiding this comment.
🟡 Wrong line citation for where aiResponse.data is read
AIDrawer.tsx:138-143 is the scrollIntoView/setResponsesLS effect (useEffect(() => { if (responsesEndRef.current) {...} setResponsesLS(responses); }, [responses])), not where aiResponse.data is parsed. That happens in the next effect, at lines ~144-152 (if (!aiResponse) return; ... typeof aiResponse.data === "string"). Anyone checking this claim against the code will land on the wrong block.
| - **appearance** → the **stylesheet** | ||
|
|
||
| **A turn may mix both.** The app saves layout first, then content, and stops if the layout half fails — so a mixed turn can land its file write and not its field write. Do not split one logical change across both halves unless the change genuinely spans them. | ||
|
|
There was a problem hiding this comment.
🔴 Save-order/failure claim is backwards relative to the existing save pipeline
The only existing "save both content and layout" mechanism, runMergedSave in StudioWrapper.tsx (~line 1074), saves content first, then layout — the opposite order stated here — and its own comment says the two are "attempted INDEPENDENTLY rather than short-circuiting on the first failure," so a layout failure does not prevent the content write from landing (each half has its own try/catch and both always run if pending). If Studio's AI integration reuses this pipeline, this section's failure-mode guidance will lead to the wrong assumption about which half of a mixed turn survives a partial failure.
Code Review — 🔴 1 blocker(s) · 🟡 1 warning(s) — see inline comments |
| } | ||
| ``` | ||
|
|
||
| A bare array as the response body reads as `undefined` and breaks the drawer — the app reads `aiResponse.data` (`AIDrawer.tsx:138-143`). Two tolerances exist and should not be relied on: a ` ```json ` fence around a string `data` is stripped, and a lone object is wrapped in an array. |
There was a problem hiding this comment.
🟡 Line citation doesn't match the code it describes
AIDrawer.tsx:138-143 is the scroll/localStorage useEffect (responsesEndRef.current.scrollIntoView(...), setResponsesLS(responses)). The actual data parsing — fence stripping, JSON.parse, and the Array.isArray(parsed) ? parsed : [parsed] wrap — lives in the next useEffect, at lines 144-155. Fix the citation so future readers land on the right block.
|
|
||
| `SET_VALUE` is the only write. Every capability below is a `SET_VALUE` against a different refKey — there is no separate action for styling, or reordering, or duplicating. | ||
|
|
||
| (The app's internal action enum is wider than this — it also implements click, focus, blur and navigate. Those are not part of this contract and the model should not emit them.) |
There was a problem hiding this comment.
🟡 Internal action enum list is incomplete
src/engine/actionTypes.ts defines six ActionType members, not five — CUSTOM (with its own handler in handlers.ts:36-40) is missing from this list alongside CLICK, FOCUS, BLUR, NAVIGATE. Doesn't change the model-facing contract, but understates what "wider than this" actually includes.
Code Review — ✅ No blockers · 🟡 2 warning(s) — see inline comments |
availableModes carries "full" because it lists the toggle positions a user may select, and full is the union of the other two. As a permission signal that member is derived, so the contract now takes the pair: ["content"], ["layout"], or both. Renamed from availableModes, because with full removed the array no longer holds modes, and keeping the UI word in the name reintroduces the state leak that dropping mode removed. The host builds it from the two booleans it already has rather than from availableModes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A view edit writes Parsley, and the only safe vocabulary for it is the
fields array -- the same one the code surface already sends alongside
filename and code. Without it the model invents field names and
{{this.whatever}} renders empty on the live site.
States the two rules that come with it: a name must appear in fields to
be referenced, and cross-model references are not to be introduced since
the other models are not supplied. Stylesheets and scripts carry no
Parsley, so fields is ignored there.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Code Review — ✅ No blockers |
path was carried for chat-history scoping, which is a host concern -- the host already knows the page it opened and does not need to send it anywhere to key its own storage. If chat moves server-side the field is a chat zuid, not a path. For the model, page identity is already in sources[].filename and in the selection breadcrumb. Adds the rule the section kept failing: every field has to change a decision the model makes. Studio knows a great deal about what the user is looking at and almost none of it belongs on the wire. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
| - **markup, structure, order, or which elements exist** → the **view file** | ||
| - **appearance** → the **stylesheet** | ||
|
|
||
| **A turn may mix both.** The app saves layout first, then content, and stops if the layout half fails — so a mixed turn can land its file write and not its field write. Do not split one logical change across both halves unless the change genuinely spans them. |
There was a problem hiding this comment.
🔴 Save order and fail-stop behavior described here is backwards from the actual implementation
runMergedSave in src/apps/studio/StudioWrapper.tsx:1074-1119 saves content first, then layout (opposite of what's stated here), and each half runs in its own try/catch that accumulates into failedCount rather than short-circuiting — a failed content save does not stop the layout save from running. A model implementer relying on this doc to reason about which half of a mixed turn is safe to assume happened will draw the wrong conclusion in both dimensions (order and failure semantics). Fix: "The app saves content first, then layout; both halves run independently, so a mixed turn can land one half without the other."
"You" was standing for the engineer reading the document, the MCP service, and the model, sometimes in adjacent paragraphs. The damaging case was the third: it hid which rules the reader implements in code and which they have to teach the model in the system instruction they own. Names app, client and model once, retitles the request and response sections by direction, and splits the old rules section into rules the model must follow -- encoded in the instruction, unenforced by us -- and app behaviour that shapes a good response without being a rule. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Code Review — 🔴 1 blocker(s) — see inline comments |
| } | ||
| ``` | ||
|
|
||
| A bare array as the response body reads as `undefined` and breaks the drawer — the app reads `aiResponse.data` (`AIDrawer.tsx:138-143`). Two tolerances exist and should not be relied on: a ` ```json ` fence around a string `data` is stripped, and a lone object is wrapped in an array. |
There was a problem hiding this comment.
🟡 Wrong line citation for the aiResponse.data parsing logic
AIDrawer.tsx:138-143 is a different useEffect (scroll-into-view / setResponsesLS); the actual aiResponse.data read, fence-stripping, and array-wrapping logic is at lines 144-155. Fix the citation so it points at the right effect.
Code Review — ✅ No blockers · 🟡 1 warning(s) — see inline comments |
"Per-element style or class writes. Styling goes to a stylesheet" read as a ban on the markup carrying a class at all. What is excluded is an action that targets one element -- the only mechanism for it is the bridge's live-DOM class commands, which never touch source, so it would appear to work and vanish on save. Writing a class into the view file and the rule into the stylesheet is two ordinary SET_VALUEs and is the intended path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Studio calls useGetWebViewsQuery and nothing else, so stylesheet: and script: refKeys do not exist until the code app's files store is wired in, and the first release carries views only. The document was naming a target that will not be there. sources is now stated as the list of what exists, the way registryKeys already is for fields, and the routing rule splits by blast radius rather than by file type: page-scoped appearance is a style block in the view, instance-wide theming is the stylesheet when one is offered and a SYSTEM_OUTPUT when it is not. That also matches the product default of scoping a change to the page unless the user asks for it globally. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Code Review — ✅ No blockers |
| } | ||
| ``` | ||
|
|
||
| A bare array as the response body reads as `undefined` and breaks the drawer — the app reads `aiResponse.data` (`AIDrawer.tsx:138-143`). Two tolerances exist and should not be relied on: a ` ```json ` fence around a string `data` is stripped, and a lone object is wrapped in an array. |
There was a problem hiding this comment.
🟡 Stale line reference to AIDrawer.tsx:138-143
Those lines are currently the unrelated scroll-into-view useEffect (responsesEndRef/setResponsesLS). The aiResponse.data handling this sentence describes is the next useEffect, roughly lines 144-153. Since this doc is meant to be the schema of record for external client implementers, an anchor pointing at the wrong code will mislead anyone who jumps to it.
Code Review — ✅ No blockers · 🟡 1 warning(s) — see inline comments |
1 participant
Adds
docs/ai-action-contract.md— the spec for supporting AI in Studio, written for whoever implements the MCP client. It covers what the app sends, what the client may return, and what a Studio user can ask for mapped to the target the model writes to.Studio joins the contract the content and code surfaces already use —
SET_VALUEagainst arefKey— with three additions: view, stylesheet and script files as writable targets; aselectiondescribing what is on the canvas; andcapabilitiesas the permission gate. Studio's fields are marked as specification, since nothing sends them yet.Also updates
docs/studio.md's postMessage lists and adds aCLAUDE.mdpointer to the new doc. Docs only — no source changes.🤖 Generated with Claude Code