githubnext/chopinPublic

Let's compose rich plans together

Open the document research composer from /research in Chat #369

MergedMaggieAppletonmerged 5 commits intomainfromrefs/pull/369/head
  1. MA

    ## Why

    Typing `/research` in the Chat composer posted it as a literal message (audit finding 9). Jev then analysed that stray message. The docs and the document editor both call it "/research", so people reasonably try it in Chat.

    ## What changed

    - **Command menu:** typing `/` at the start of the Chat composer opens a command menu. It reuses the document `/` menu's surface and row classes, now exported from `@chopin/editor` as `MENU_SURFACE`, `MENU_ROW`, `MENU_ROW_ON` and `MENU_ROW_OFF`. Arrow keys move, Enter or Tab chooses, and Escape closes it while keeping the draft and focus. The only command is **Research**; no other command makes sense in Chat today. It shows only while the draft is just `/query`, so `/usr/bin` doesn't open it. - **Research opens in the document:** choosing Research, or sending `/research <brief>`, switches to the document (on phone too). It uses the empty last paragraph, or appends one, and opens the existing research composer there with the brief filled in and focused. The Chat draft is cleared. From there, everything is the existing document flow: the same `ResearchDraftStore`, request creation, and placement. - **Editor API:** `@chopin/editor` exports a host-owned `ResearchLauncher` (`available()`, `open(brief)`), passed to `PlanEditor` as `researchLauncher`. A new `ResearchLaunchRegistration` component in the toolbar attaches it to the mounted editor's draft store and binding. It doesn't touch `toolbar/research.tsx`, so it stays compatible with #355. - **When it can't open**, the Chat draft and its `#` references stay put. The draft is cleared only after the document composer holds the brief, which happens after the document has laid out. Each cause has its own message: - A brief over 4,096 characters is refused in Chat before switching: "Briefs can be up to 4,096 characters". - A research draft is already open: "A research request is already being written in the document", with **Go to it**, which focuses that draft. - Read-only: "This document is read-only". - Disconnected: "Reconnecting… try again in a moment", with **Retry**. - **Caret:** it lands at the end of a prefilled brief. - **`#` references:** they carry into the brief as their visible titles (`#Pricing`). The composer is plain text, so they become text, not links. - **`@chopin /research …`:** this, and `/research` in Chopin mode, are the same command and are never sent to the Planner. - **Child documents:** research isn't offered there. The menu doesn't appear, a `/research` draft is held instead of sent, and the notice says "Research isn't available in this document".

    ## Screenshots

    **Before**: `/research` + Enter posts a literal message

    ![Before, desktop](https://github.com/user-attachments/assets/70e8bab2-8220-46ee-8662-b7c9b0cfbb45)

    **After**: `/` opens the command menu

    ![After, menu](https://github.com/user-attachments/assets/3ec314de-f196-45b6-b42f-1c4f704efb62)

    **After**: a typed brief…

    ![After, typed](https://github.com/user-attachments/assets/f76224dc-01aa-4eb2-81ae-edcda625122c)

    **…opens the document's research composer with the brief, after the last paragraph**

    ![After, composer](https://github.com/user-attachments/assets/69d76649-f4b5-4e63-807c-e767d2cc1231)

    **After**: a draft is already open, or the brief is too long

    ![After, drafting](https://github.com/user-attachments/assets/36e419e9-5c48-4c50-94ac-64f79bd49e59) ![After, too long](https://github.com/user-attachments/assets/af58ce26-6eca-485a-8c38-acafda85d168)

    **Phone**: before / after

    ![Before, phone](https://github.com/user-attachments/assets/b733eeeb-8b18-4609-8c9f-f3ef6a200fe1) ![After, phone](https://github.com/user-attachments/assets/9ae3e62c-cd47-4917-a6f1-808b87a62b1b) ![After, phone drafting](https://github.com/user-attachments/assets/8cc666ee-64f1-48dc-9148-c5288e758a96)

    ## Testing

    - `bun test --timeout 60000 packages/editor apps/web`: 1007 pass, 0 fail. New tests: `packages/editor/src/research-launcher.test.ts` covers attach, detach, stale detach, unavailable, the editor's reason, and the 4,096-character limit. `apps/web/src/chat/commands.test.ts` covers `@chopin /research` and brief extraction. - `bun run types`: pass. `bun run build`: pass, initial JS 246661 B. - `bun run ci`: pass on the rebased integration; the design-contract review below is complete. - E2E `e2e/chat-commands.e2e.ts` covers: - the menu, Escape, and no menu for `/usr/bin` - choosing Research: an empty, focused composer - `/research <brief>`: prefilled, reusing the empty last paragraph, caret at the end - `@chopin /research` while a draft is open: notice, draft kept, **Go to it** focuses the open draft - a 4,097-character brief refused in Chat - no `chat:send` frame sent

    I ran the same flow with a standalone Playwright script against a fake-GitHub server at 1440×900 and 390×844: it passed, with 0 frames sent. - **#355 overlap:** #355 is still open. The launcher centres the target paragraph with `scrollIntoView({ block: "center" })` so the current composer placement has room. Once #355 lands, I'll rebase, check whether its reveal scroll makes this redundant, remove it if so, and re-check the geometry.

    ## Design-contract review (completed)

    `bun run ci` reports:

    - `packages/editor/src/plan-editor.tsx`: pinned in `scripts/design-contract/exceptions/dynamic-editor.json` ("reviewed dynamic owner changed"). New `sourceHash`: `83e854244e4f0b86d116002456f6eeb52520661d559c06142f1058bced06bb76`. This is safe: the only change is passing the `researchLauncher` prop through to the widget plugin, with no class logic. - `apps/web/src/chat/command-picker.tsx`: `MENU_SURFACE`, `MENU_ROW`, `MENU_ROW_ON` and `MENU_ROW_OFF` are flagged as dynamic classes that need a reviewed exception. They are the exported `/` menu constants from `packages/editor/src/toolbar/surface.ts`, all static string literals. Importing them instead of copying the strings is what makes the checker flag them.

    The integrated branch renews these exact exceptions after checking the current source. Final rebased head `117cb39a` passed hosted validation, E2E, and container checks.

    🤖 Generated with [Claude Code](https://claude.com/claude-code)

  2. MaggieAppletonhead-ref-force-pushed ae4f725fd150last month on GitHub

  3. MaggieAppletonrenamed Point /research in Chat to the document instead of posting it to Open the document research composer from /research in Chatlast month on GitHub

  4. MA
    Author

    **Review:** Changes needed

    Verified in a worktree build against a fake-GitHub server (AGENT on, research on) at 1440x900 and 390x844. Menu, `/usr/bin` (no menu), Enter/Tab, choosing Research, `/research <brief>`, phone tab switch, no `chat:send` frames, and a second `/research` while a draft is open (keeps the Chat draft, shows the Retry notice, still one composer) all behave as described. Menu surface and rows match the document `/` menu. Two real bugs and a few smaller points.

    **Must fix** 1. Long briefs fail with a generic error. `research-launch.tsx:46` calls `drafts.change(brief)`, which bypasses the composer textarea's `maxLength={4096}` (`widgets/research.tsx:162`). A pasted 6000-character `/research ...` opened with all 6000 characters; pressing send gave "Research could not be started." with no hint why. Truncate to 4096 (or refuse and keep the Chat draft with a clear message) in `open()`. 2. The caret lands at the start of the prefilled brief. After `/research short brief` + Enter, typing "ZZ" gave `ZZshort brief`. Put the caret at the end (set `selectionStart/End` after the composer focuses, or have the composer do it when `question` is prefilled). This matters most for long briefs, where the textarea shows the start and the user can't see where they are typing.

    **Should fix** 3. `room-workspace.tsx:529-536` returns `true` and clears the Chat draft, then calls `open()` in a `requestAnimationFrame` whose result is ignored. If it fails there (editor remounted by an epoch reset or document switch inside that frame, or a draft opened by another path in between), the brief is lost silently. Either do the open before reporting success when the editor is laid out, or on a failed rAF restore the Chat draft/show the same "couldn't open" notice. 4. The failure copy is the same for every cause (existing draft, read-only, disconnected, epoch reset). "Research couldn't open in the document." with Retry is fine for transient causes, but for an already-open draft Retry can never work; consider "Finish or dismiss the open research draft first." Same for read-only writers. 5. `/research` handling drops composer references: `launchResearch` calls `reviseComposerDraft(current, text, [])` and the brief is built from raw text. A `#reference` in the brief is lost. Also `@chopin /research ...` or Chopin mode (`effectiveMode`) with `/research` is not recognised by `draftCommand` and posts as a message. Decide whether that is intended; if so, one line in the PR.

    **Overlap with #355 (research composer)** - No shared lines with `toolbar/research.tsx`, but both touch `plan-editor.tsx` (different hunks, so a textual rebase is clean; the design-contract hash for that file will need renewing after whichever lands second). - `research-launch.tsx:41-44` centres the paragraph with `scrollIntoView({ block: "center" })` and then relies on the old composer placement. #355 adds flip-above/pinned placement and its own reveal scroll; the two scrolls could fight. Re-check after #355 lands (the e2e here only asserts focus and value, not geometry). - #355 renames "Place Research" to "Place research"; no strings from this PR collide.

    **Checks / hygiene** - `bun run ci` fails only on design-contract items already listed in the PR body (4 `MENU_*` dynamic classes in `command-picker.tsx`, `plan-editor.tsx` hash). Acceptable per the audit rule. - Tests: `commands.test.ts` covers the pure logic; no test for launcher attach/detach, the unavailable path, or long briefs. A unit test for `ResearchLauncher` (attach/detach identity, stale detach is a no-op) and one for brief clamping would cover items 1 and 3. - Child/read-only documents: `onResearch` is undefined when `profile.research` is false, so the menu is hidden and `/research` is held with the notice; archived/read-only replace the composer via `notice`. Code path looks right (I could not drive a child document on the fake server).

  5. MA
    Author

    Thanks. All review points are addressed in 5b19ca8d:

    1. **Long briefs** are refused in Chat before switching views. `ResearchLauncher.check()` and `open()` return `too-long` above `MAX_RESEARCH_BRIEF` (4,096, matching the composer's `maxLength`). Chat shows "Briefs can be up to 4,096 characters" and keeps the draft. 2. **Caret:** it lands at the end of the prefilled brief. A one-shot `focusin` sets the selection to the end when the composer focuses. The e2e test types `!` and checks it is appended. 3. **The draft is no longer cleared early.** `onResearch` returns a `Promise<ResearchLaunchResult>`: the host checks first, switches view, waits a frame, then reports `open()`'s result. Chat clears its draft only on `ok`, and only if the draft is unchanged. Otherwise it keeps the draft and its references and shows the cause. 4. **Messages by cause:** - an open draft: "A research request is already being written in the document", with **Go to it** (`launcher.reveal()` focuses that draft) - read-only: "This document is read-only" - disconnected: "Reconnecting… try again in a moment", with Retry - editor unavailable: "Research couldn't open in the document", with Retry 5. **References and `@chopin`:** - `#` references carry into the brief as their visible titles, since the composer is plain text. - `@chopin /research …` and `/research` in Chopin mode are recognised as the command and are never sent. 6. **#355 is still open.** I kept `scrollIntoView({ block: "center" })` for now. Once #355 lands I'll rebase, check its reveal scroll against this one, drop mine if it's redundant, and re-check the geometry. 7. **Tests:** `research-launcher.test.ts` covers attach, detach, stale detach, unavailable, the editor's reason, and the long-brief limit. The e2e test now also covers the open-draft and too-long cases.

    Checks: `bun test packages/editor apps/web` passes (1007, 0 fail) and `bun run types` passes. `bun run ci` still fails only on the design-contract items listed in the PR body, including the same `plan-editor.tsx` hash `83e8542…6bb76`.

  6. MaggieAppletonhead-ref-force-pushed 117cb39a9029last month on GitHub

  7. MaggieAppletonmerged e6ed8fbf1331last month on GitHub

  8. MaggieAppletonclosedlast month on GitHub

  9. MaggieAppletonhead-ref-deletedlast month on GitHub