Skip to content

fix(web): keep $ skill picker usable inside question answers

MacroscopeApp / Macroscope - UI Consistency failed Aug 27, 2026 in 10m 7s

UI Consistency: 4 issues found

Four concrete findings were posted as inline review comments.

  • apps/web/src/components/chat/ChatComposer.tsx:3495 — plainText={false} is hardcoded at the only real call site, so the entire new plainText mode added to the shared ComposerPromptEditor primitive (mode-keyed LexicalComposer remount, token-plugin gating, three cursor-mapping swaps, restoreFocusOnRemountRef, render-phase ref mutations) is unreachable; the accompanying state comment at lines 1028-1031 also describes behavior that never occurs.
  • apps/web/src/components/chat/ChatComposer.tsx:1757 — the cursorAdjacentToMention trigger suppression was removed only in the pending-answer branch. With chips still rendered in answer mode, a caret parked beside a committed $skill/@file chip re-opens the picker, diverging from the draft branch (line 1776) in the same editor.
  • apps/web/src/components/chat/ChatComposer.tsx:1527-1533 — the new capture-phase window Escape handler calls preventDefault()/stopPropagation() unconditionally while a composer trigger is active, swallowing Escape from popups/dialogs/selects elsewhere in the app; dismissal also belongs with the menu (cf. ComposerStashMenu's onClose) rather than a global listener.
  • apps/web/src/components/ChatView.tsx:6150-6154 — the isPlainInsert guard only matches appends (startsWith), so mid-answer skill insertion still triggers the focus/handleEditorChange ping-pong it is meant to prevent; the typeof snapshot.value === "string" check is dead.

Details

Note

Your check run agent prompt is: .macroscope/check-run-agents/ui-consistency.md
More information about how Check Run Agents work can be found in our Help Center.

Methodology: read the full diff, then inspected head-of-branch sources for ComposerPromptEditor.tsx (plainText plumbing, plugin gating, remount key, focus-restore layout effect), ChatComposer.tsx (plainAnswerMode, draftPromptRef, trigger/menu state, new Escape/pointerdown effect, editor call site), and ChatView.tsx (onChangeActivePendingUserInputCustomAnswer). Cross-checked detectComposerTrigger, isCollapsedCursorAdjacentToInlineToken, skill replacement text ($name with trailing space), ComposerCommandMenu visibility ownership (hand-rolled portal layer, no Base UI open/onOpenChange), ComposerStashMenu's existing self-owned dismissal, and all ComposerPromptEditor consumers (ChatComposer, SettingsFontPreviews) to confirm no call site enables plainText.

No findings were raised on Tailwind/CSS ownership, ScrollArea/virtualized fades, theme variants, or environment routing: the diff does not touch styling, global CSS, or environment-scoped renderer actions.


🔍 Agent activity — 14 tool calls

💭 I'm looking at the diff involving a skill picker in question answers, checking UI consistency. I need to examine ChatComposer.tsx for the new plain answer mode and outside-click handling, ComposerPromptEditor.tsx for plain text remounting behavior, and ChatView.tsx for related state. I'm concerned about the removal of a cursor-adjacent-mention check in setComposerTrigger, since that changes existing behavior, and about the new Esc/outside-click handler possibly duplicating an existing shared mechanism from the earlier @ mentions fix. Let me pull up the actual files to check.

1. git_tools — isInsideComposerFloatingLayer

2. git_tools — plainText

💭 I notice the plainText flag added in ChatComposer.tsx wasn't showing up in my grep results, likely because that file got skipped as binary in the initial search and excluded entirely from the second grep pass. Let me check whether there's actually a NUL byte embedded in the file, and look directly at the relevant sections around lines 3495 and 1520-1640.

3. browse_code — apps/web/src/components/chat/ChatComposer.tsx

4. browse_code — apps/web/src/components/chat/ChatComposer.tsx

💭 I notice plainText={false} is hardcoded at the call site with a comment explaining they want the chip rather than raw text. This means the entire plainText mode machinery added to ComposerPromptEditor—plugin gating, remount keys, cursor mapping swaps, focus restore ref—is dead code with no enabled consumer, while plainAnswerMode in ChatComposer remains actually used for draft parking. That's a notable finding worth flagging: a large new prop/mode contract that's never actually turned on. I also spot a stale comment in ChatComposer claiming the editor renders raw text with menus closed while a question is active—that's no longer true given plainText is false and menus are active. More concerning, the removal of the cursorAdjacentToMention check in onPromptChange changes behavior even in rich mode: previously the trigger menu was suppressed when the cursor sat next to a mention token, but now it can pop open in that situation, which looks like a regression beyond the scope of this fix. I want to check the surrounding code and whether cursorAdjacentToMention is now an unused parameter that would trigger a lint warning.

5. browse_code — apps/web/src/components/chat/ChatComposer.tsx

6. browse_code — apps/web/src/components/chat/ChatComposer.tsx

💭 I'm confirming that the mention-adjacency suppression logic was actually removed, so the composer trigger detection now runs unconditionally instead of being skipped when the cursor is adjacent to a mention. With cursor at the end of expanded $skill-name text, the trigger detector would likely see a $ and reopen the menu with search text that matches nothing — inconsistent with how the draft path suppresses this. I want to verify by checking detectComposerTrigger's behavior on expanded token text, and also confirm whether any other consumer besides ChatComposer passes plainText to ComposerPromptEditor to rule out the dead-code theory.

7. git_tools — ComposerPromptEditor

8. browse_code — In apps/web/src, what does detectComposerTrigger do (signature, how it decides to open the $ skill / @ mention / slash menus), and what is the role of the cursorAdjacentToMention flag emitted by ComposerPromptEditor's onChange in ChatComposer? Show the implementation of detectComposerTrigger and isCollapsedCursorAdjacentToInlineToken.

💭 I'm comparing indentation levels between the removed and retained code branches to figure out which conditional block was actually modified in the diff. So it looks like the pending-question branch's suppression logic got removed — meaning while a question is open and a skill chip gets inserted, the cursor now sits adjacent to the mention and the trigger detector fires again, reopening the menu. That could be exactly the wedge behavior the Esc/outside-click dismissal was meant to patch over, rather than the actual root cause being fixed. I want to check the ChatView's plain-insert guard to see if that's the real source of the bug.

9. git_tools — apps/web/src/components/ChatView.tsx d3c24a1..efd8141

10. git_tools — apps/web/src/components/chat/ChatComposer.tsx d3c24a1..efd8141

💭 I'm looking at whether the new plainText mode in ComposerPromptEditor is actually dead code—it adds substantial machinery like remount keys, plugin gating, and cursor-mapping logic, but the only prop usage passes plainText as false and no consumer enables it. This fits the "smallest durable contract" principle poorly, so I should flag it and suggest either enabling the mode or removing it. I'm also double-checking whether the render-time ref mutations and the focus-restore effect could subtly affect the existing rich-text path even though the feature itself isn't consumed, but the impact looks minor since the mount-time seeding and no-op effects hold up under closer inspection. Also that stale comment is now misleading since plainText={false} still keeps tokens and menus active in answer mode. And removing the mention-adjacency guard in the pending-answer branch creates real divergence: caret near a committed skill/file chip would reopen the menu in the answer path but stay suppressed in the draft path — that's a concrete inconsistency worth flagging, and possibly what the new Esc/outside-click effect is compensating for. So just moving the caret near a chip could pop open the skill list filtered to that name — that's a real UX inconsistency worth flagging. I want to double check the replacement text used for skill selection to confirm this, and also trace how ChatView handles cursorAdjacentToMention on the answer path to see if it's suppressed there.

11. browse_code — apps/web/src/components/ChatView.tsx

12. browse_code — In apps/web/src/components/chat/ChatComposer.tsx, show onSelectComposerItem / how a selected skill item is turned into replacement text passed to applyPromptReplacement (does it append a trailing space? is the inserted text the serialized $skill token form?). Also show resolveActiveComposerTrigger and where composerMenuOpen is computed.

💭 I'm examining the isPlainInsert guard in the ChatView handler, which treats any value that's a strict prefix-extension of the snapshot as normal append typing—but I need to check whether this holds during regular editing, since the editor itself drives the snapshot update before the check runs. I'm tracing through: for typing, snapshot.value already equals the new value so isPlainInsert is false; it's only true when a parent-triggered insertion (like a skill replacement) appends text at the very end. But if the insertion happens mid-text—say the user types elsewhere and inserts a skill reference in the middle—the value won't start with the old snapshot, isPlainInsert stays false, and the ping-pong focus bug this guard was meant to prevent resurfaces. This looks like a narrow, fragile fix rather than a robust solution, and since ChatView.tsx falls within scope, I'm weighing whether this counts as a real UI consistency issue against the "smallest durable contract" principle. I also notice a redundant runtime type check on an already-typed field, which is a minor smell suggesting some uncertainty in the code. And there's a new global Escape/outside-click listener using capture phase with stopPropagation—this seems to duplicate dismissal logic that a popup component (likely handling its own onOpenChange/Escape behavior) should already provide, raising a risk that the global listener could swallow the popup's own Escape handling.

13. browse_code — In apps/web/src/components/chat: how is the composer command menu (ComposerCommandMenu) rendered and how was Escape / outside-click dismissal handled BEFORE this change? Show ComposerCommandMenu's props/implementation (does it use Base UI Popover/Menu with onOpenChange or open/close props?), and show ChatComposer's onComposerCommandKey handling of Escape and any existing pointerdown/keydown listeners for closing the menu (including the #2635 mention fix referenced in a comment). Also show COMPOSER_FLOATING_LAYER_SELECTOR and data-chat-composer-form usage.

💭 I'm noting that this dismissal behavior didn't exist before, and the justification citing a prior fix seems shaky. More importantly, the ownership feels off—ComposerStashMenu handles its own dismissal locally, but this new logic puts it in ChatComposer as a global listener instead of keeping it in the menu component where it belongs. Also, capturing and swallowing Escape unconditionally at the window level is risky since it can steal the key from other layered UI, like a Select popover or dialog opened on top of the composer. For instance, if the skill menu and model picker are both open, pressing Escape would close the composer menu instead of the Select, since the composer trigger stays active and stopPropagation blocks everything else from receiving the event—that's worth flagging as a real interaction bug. I'm also noting that clicks inside the composer form, like on the model picker or attach button, don't close the menu via the pointerdown handler—a minor issue. Now I'm narrowing down to three inline comments: the unreachable plainText mode in ComposerPromptEditor.tsx since only one call site exists and it's always false, yet it introduces unnecessary remount and cursor-mapping complexity. I want to anchor comments at concrete diff lines—one at ChatComposer.tsx around line 3495 where the contradictory plainText={false} flag sits alongside a stale comment, and possibly another at ComposerPromptEditor.tsx near line 1903 covering the dead machinery, though I'm leaning toward consolidating into a single comment at ChatComposer:3495 that covers the dead-mode contract to avoid duplication. I also want to flag ChatComposer.tsx line 1757, where the cursorAdjacentToMention suppression was removed only in the pending-answer branch, so parking the caret next to a committed chip reopens the mention picker in answer mode while the draft branch at line 1776 still suppresses it—an inconsistency that should be fixed by applying the guard uniformly. Then I want to look at lines 1525-1548, where Escape is handled via a capture-phase window listener with stopPropagation, to see if there's a similar contract mismatch worth flagging there too. Since the guideline says not to fail for aesthetic reasons but this is a genuine functional gap, I'll frame it as a suggestion rather than a hard violation. I'm also verifying finding 2's premise by tracing whether chip rendering actually applies in answer mode — confirming that $skill does render as a chip when plainText is false, and that cursor-adjacency logic diverges as I suspected, so the concern holds. Now I'm second-guessing whether removing that behavior was intentional, checking the picker's trailing-space insertion logic in answer mode. Actually I realize with a chip present in the answer, clicking right after it would pop the skill menu with the chip's name as the query — that's worth flagging as a moderate-high confidence issue. Now I need to check that my inline comments map to lines actually present in the diff, since comments must sit on added lines on the head side. I'm going through the list of added line ranges in ChatComposer.tsx to confirm coverage. I'm cross-referencing the added lines in ChatView.tsx and ComposerPromptEditor.tsx as well, and settling on specific line numbers to anchor each comment — like using a range around 1527-1533 for the Escape handler, and confirming line 3495 and lines 6150-6154 are valid targets. Now I'm drafting the actual comment text, starting with a note on ChatComposer line 3495 about plainText being hardcoded to false and why that might be wrong. I'm also flagging a stale comment nearby (lines 1028-1031) that should probably be folded into that same note rather than made separate. For the second comment on line 1757, I'm proposing an inline suggestion adjusting how setComposerTrigger is called, using the cursor-adjacent-to-mention check to conditionally return null instead of running the trigger detection, and I'm making sure the indentation matches the surrounding block. For the third comment about Escape handling, I'm considering scoping the key handler so it only fires when the event target is inside the composer form or its floating layer, but I'm second-guessing this since it might unintentionally prevent Escape from closing the menu when focus is elsewhere. Now I'm moving on to the fourth comment about ChatView's isPlainInsert, then I'll wrap up with a final answer summarizing the review as a failure state before posting it.

14. modify_pr — ## UI Consistency: composer answer-mode review Four findings, all in changed lines. The main one is that the new plainText mode added to the shared ComposerPromptEditor primitive is never enabled by any call site, while several behavioral guards around it were relaxed in the live (rich) path. - apps/web/src/components/chat/ChatComposer.tsx:3495 — plainText={false} makes the entire new editor mode unreachable, and the state comment at 1028-1031 now describes behavior that does not happen. - apps/web/src/components/chat/ChatComposer.tsx:1757 — dropping the cursorAdjacentToMention suppression only in the pending-answer branch makes chip-adjacent caret behavior diverge from the draft branch in the same rich editor. - apps/web/src/components/chat/ChatComposer.tsx:1527-1533 — capture-phase window Escape handler with stopPropagation() swallows Escape app-wide whenever a composer trigger is active. - apps/web/src/components/ChatView.tsx:6150-6154 — the isPlainInsert heuristic only recognizes appends, so mid-text insertion still hits the ping-pong it is meant to prevent.


Agent Credits: 425 credits