Skip to content

feat(prompt-input): collapse /<command> into inline pill#785

Merged
Astro-Han merged 27 commits into
devfrom
claude/i778-slash-pill
May 20, 2026
Merged

feat(prompt-input): collapse /<command> into inline pill#785
Astro-Han merged 27 commits into
devfrom
claude/i778-slash-pill

Conversation

@Astro-Han

@Astro-Han Astro-Han commented May 20, 2026

Copy link
Copy Markdown
Owner

Summary

Collapse /<command> args in the prompt input into the same inline pill (icon + brand-colored name) that PR #768 already renders in the sent bubble. Selecting from the slash popover, typing the command followed by Space, or pasting /<known-name> args into an empty input all materialise the leading /<name> into a data-cmd-mark element while the args remain plain text.

Why

Fixes #778. The sent bubble shows the command name as a brand-colored pill, but until now the same text in the prompt input rendered as raw /<name>. Two surfaces, two visual treatments for the same concept. This PR aligns the editor side to the send-side model: a single optional TextPart.command field carries the metadata, content stays /<name> <args> end-to-end, and metadata.commandInvocation on the wire is unchanged.

Related Issue

Fixes #778

Human Review Status

Pending

Review Focus

The marked-TextPart invariants. There is at most one per Prompt, always at index 0, and content always equals /<name> <args> with a single trailing space after the name. Most of the new code is the helpers (command-text-part.ts, command-prepend.ts, command-space-trigger.ts, command-paste.ts, command-backspace.ts) that enforce these invariants on the way into the store, plus the editor-side primitives (editor-dom.ts, editor-serialize.ts) that treat the pill as a unit of logical length 1 + name.length. The contenteditable round-trip is the most fragile surface and has the largest test coverage.

Other things worth double-checking:

  • ASCII-only case-insensitive matching in command-text-part.ts:16-45. Non-ASCII names fall back to exact byte match so caret math stays length-preserving.
  • Path B trigger uses inputType === "insertText" && data === " ", which is naturally false on paste, Backspace, IME commit, and programmatic mutations. No flag state machine.
  • Backspace ladder in command-backspace.ts: caret inside the prefix region of a marked TextPart degrades the pill back to plain text without dropping attachments.

Risk Notes

  • Path A (popover-select) and Path B (Space-trigger) preserve image attachments by passing imageAttachments() through to the new prompt, so the existing image flow is unchanged. Same for prompt.context.items().
  • E1 mid-prompt variant: when the popover is opened by the global trigger while the editor already has unrelated content, the marked TextPart is prepended and original parts keep both order and reference identity. No clone of incoming FileAttachmentPart / AgentPart / ImagePart.
  • Legacy fallback boundary: the existing text.split(" ")[0] === "/<known-name>" codepath in submit.ts is kept as a safety net; the new path only fires when part.command metadata is present.
  • Deferred follow-ups, none of which affect this PR's user-facing acceptance:
    • Homepage owner-mirror in Path A and Path C (Codex P2, narrow widening of an existing gap on Path A; new but narrow on Path C).
    • E2E snap target for the pill rendering. The unit tests at editor-dom.test.ts and editor-serialize.test.ts cover the DOM contract; a visual diff for the pill itself can be added in a small follow-up.
    • dev:desktop walkthrough of the IME + paste + Backspace paths on macOS/Windows. The unit and integration coverage exercises the same code paths.

How To Verify

bun --cwd packages/app test                  → 1405 pass, 0 fail
bun --cwd packages/app run typecheck         → 0 errors
gh pr diff (visual)                          → 17 commits, 1 P0 + 2 P1 from crosscheck addressed
manual: open slash popover, select brainstorming → pill renders, caret lands after the trailing space
manual: type `/brainstorming ` (with Space)  → buffer materialises into pill, popover closes
manual: Backspace just after pill            → pill collapses back to `/brainstorming ` plain text, attachments survive
manual: paste `/brainstorming hello` into empty input → pill + " hello" args

The 4 pre-existing icon-button test failures on dev (CSS 30x30 vs test expectation 24x24) are unrelated to this PR.

Screenshots or Recordings

(To be attached by reviewer; pill visuals match PR #768 sent-bubble exactly, sharing --brand-primary and --font-weight-body via command-pill.css.)

Checklist

  • Type label — this PR carries exactly one of bug, enhancement, task, documentation. Type labels are author-added; the labeler bot does NOT assign them. Add the label in the GitHub UI, then tick this.
  • Routing labels — this PR carries at least one of app, ui, platform, harness, ci. The labeler bot assigns these on PR open based on changed paths. Confirm the bot's choice (or override if wrong), then tick this.
  • Priority label — this PR carries exactly one of P0, P1, P2, P3. The priority-triage bot suggests one on PR open. Confirm or override, then tick this.
  • Human Review Status above is set to Pending, Approved by @<reviewer>, or Not required: <reason> (default is Pending; "not required" is restricted to bot-authored low-risk PRs).
  • I linked the related issue, or stated in Summary why there is no issue.
  • I described the review focus and any meaningful risks.
  • I replaced the example block in How To Verify with the real verification steps and the key result for each.
  • I did not introduce unrelated refactors, dependencies, generated files, or file changes beyond the stated scope.
  • (conditional) I manually checked visible UI or copy changes when needed, with screenshots or recordings. Leave unticked only if no visible UI or copy changed.
  • (conditional) I considered macOS and Windows impact for platform, packaging, updater, signing, paths, shell, or permissions changes. Leave unticked only if no platform/packaging surface was touched.
  • (conditional) I called out docs, release notes, dependencies, permissions, credentials, deletion behavior, generated content, or local file changes when relevant. Leave unticked only if none of those surfaces was touched.
  • I reviewed the final diff for unrelated changes and suspicious dependency changes.
  • I am targeting dev, and my PR title and commit messages use Conventional Commits in English.

Summary by CodeRabbit

  • New Features
    • Added visual command pills that display slash commands in the prompt editor with branded styling and icons.
    • Implemented copy/paste support for slash commands with proper clipboard handling and text rewriting.
    • Added keyboard interactions for command editing, including backspace handling to remove or modify command content.
    • Command icons now display with appropriate visual resolution and styling.

Review Change Stack

Astro-Han added 9 commits May 20, 2026 18:08
Export a pure `(icon: string) => string` helper from command-icon.tsx.
Refactor `<CommandIcon>` to call it internally so the fallback logic
(REGISTRY[icon] ?? REGISTRY.command) lives in one place. The DOM-side
input pill serializer (upcoming task) will import this without touching
SolidJS reactivity.
Add cmd-mark pill awareness to getNodeLength, setCursorPosition, and setRangeEdge.
A slash-command pill uses data-cmd-mark="true" and data-name=<name>; its logical
offset length is 1 + name.length (slash trigger + name), while textContent is only
name. The cmd-mark branch in getNodeLength is placed before the textContent fallback
so the asymmetry is preserved. Both isPill predicates in setCursorPosition and
setRangeEdge now include the data-cmd-mark check alongside data-type file/agent.

Tests: 3 new cases covering getNodeLength value, branch-order asymmetry, and
setCursorPosition snap behavior; all 1312 tests pass.
- Add createCommandMark() that builds the pill DOM: outer span with
  data-cmd-mark/name/source/icon + contenteditable=false, icon child
  (data-cmd-icon, class=command-icon, innerHTML from resolveCommandIconSvg),
  label child (data-cmd-label, textContent = name, no slash).
- renderPartsToEditor: index-0 TextPart with .command → pill + tail fragment;
  tail = content.slice(1 + name.length) preserving separator + args verbatim.
- parseEditorToParts: before main visit loop, detect leading data-cmd-mark child.
  Walk siblings collecting followingTextRun (text nodes, BR as \n); stop at
  file/agent/cmd-mark boundary. Push marked TextPart content = "/" + name + run.
  Skip consumed nodes by slicing children from startIndex = 1 + consumedSiblings.
- Mid-stream data-cmd-mark (not at index 0) falls through to visit()'s
  recursive-children branch: label textContent surfaced as plain text, no
  command metadata emitted.
- Import command-pill.css via side-effect import.
- Add editor-serialize.test.ts: pill DOM attrs, round-trips for "/cmd ",
  "/cmd args", "/cmd  args" (double-space), self-heal mid-stream pill.
Replace the plain TextPart in the command branch of extractPromptFromParts
with createCommandTextPart so the restored Prompt carries the command field.
The editor can then re-render the pill instead of falling back to raw text.

Plain-prompt and image-attachment branches are unchanged byte-for-byte.
Tests now assert command: { name, source } on all three command-mode cases.
@github-actions github-actions Bot added app Application behavior and product flows ui Design system and user interface labels May 20, 2026
@coderabbitai

coderabbitai Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor

Warning

Rate limit exceeded

@Astro-Han has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 53 minutes and 35 seconds before requesting another review.

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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 8b0a8d85-c4cf-48e2-ba02-ccac6810c65c

📥 Commits

Reviewing files that changed from the base of the PR and between 2372019 and 82e01b4.

📒 Files selected for processing (28)
  • packages/app/src/components/prompt-input.tsx
  • packages/app/src/components/prompt-input/attachments.ts
  • packages/app/src/components/prompt-input/command-backspace.test.ts
  • packages/app/src/components/prompt-input/command-backspace.ts
  • packages/app/src/components/prompt-input/command-copy.test.ts
  • packages/app/src/components/prompt-input/command-copy.ts
  • packages/app/src/components/prompt-input/command-paste.test.ts
  • packages/app/src/components/prompt-input/command-paste.ts
  • packages/app/src/components/prompt-input/command-prepend.test.ts
  • packages/app/src/components/prompt-input/command-prepend.ts
  • packages/app/src/components/prompt-input/command-space-trigger.test.ts
  • packages/app/src/components/prompt-input/command-space-trigger.ts
  • packages/app/src/components/prompt-input/command-text-part.ts
  • packages/app/src/components/prompt-input/editor-dom.test.ts
  • packages/app/src/components/prompt-input/editor-dom.ts
  • packages/app/src/components/prompt-input/editor-imperatives.ts
  • packages/app/src/components/prompt-input/editor-input.ts
  • packages/app/src/components/prompt-input/editor-serialize.test.ts
  • packages/app/src/components/prompt-input/editor-serialize.ts
  • packages/app/src/components/prompt-input/keydown.ts
  • packages/app/src/components/prompt-input/owner-mirror.test.ts
  • packages/app/src/components/prompt-input/owner-mirror.ts
  • packages/app/src/components/prompt-input/path-b-integration.test.ts
  • packages/app/src/components/prompt-input/pinned-draft.test.ts
  • packages/app/src/components/prompt-input/popover-controllers.ts
  • packages/app/src/components/prompt-input/portable-draft.test.ts
  • packages/app/src/components/prompt-input/submit.test.ts
  • packages/app/src/components/prompt-input/submit.ts
📝 Walkthrough

Walkthrough

This PR implements visual collapsing of slash commands into inline pills within the prompt input. Users can trigger pills by typing a command then space, pasting command text into an empty prompt, or selecting from the slash popover. Pills render atomically (icon + brand-name), are deletable as a unit via backspace, and route through dedicated submit paths while maintaining backend compatibility.

Changes

Slash Command Pill Input Feature

Layer / File(s) Summary
Command metadata and type contracts
packages/app/src/context/prompt.tsx, packages/app/src/context/prompt.test.ts
TextPart gains optional command metadata (name, source: CommandSource, icon); isPromptEqual now compares command fields; new isStructurallyEmpty checks prompt emptiness including attachments.
Command text parsing and descriptor building
packages/app/src/components/prompt-input/command-text-part.ts, packages/app/src/components/prompt-input/command-text-part.test.ts
CommandDescriptor interface; createCommandTextPart builds marked text with metadata; tryParseLeadingCommandFromText matches typed names against registry using ASCII-case-insensitive rules; buildSlashRegistry converts sync entries; assertCommandTextPart validates invariants.
Command pill DOM rendering and serialization
packages/app/src/components/prompt-input/editor-serialize.ts, packages/app/src/components/prompt-input/editor-serialize.test.ts
createCommandMark builds non-contenteditable pill spans with icon/name/metadata; renderPartsToEditor renders first command TextPart as pill + tail text; parseEditorToParts reconstructs command metadata from leading pills; isNormalizedEditor permits pill boundaries.
Command entry point conversion (space, paste, prepend)
packages/app/src/components/prompt-input/command-space-trigger.ts, packages/app/src/components/prompt-input/command-paste.ts, packages/app/src/components/prompt-input/command-prepend.ts, plus test files
Path B: tryPathBConversion converts trailing-space typed command into marked part; Path C: tryPathCConversion converts pasted command when prompt is structurally empty; prependCommandMark builds pill for popover selection, replacing vs. prepending based on current state.
Copy and backspace handling for command pills
packages/app/src/components/prompt-input/command-copy.ts, packages/app/src/components/prompt-input/command-backspace.ts, plus test files
rewriteRangeForCommandCopy converts selected pills to /<name> literals; selectionTouchesCommandMark detects pill selection; computeCommandBackspaceResult implements fallback ladder: strip prefix (args remain), remove pill (parts remain), or collapse to empty.
Editor input handler integration
packages/app/src/components/prompt-input/editor-input.ts, packages/app/src/components/prompt-input/attachments.ts
Editor input now accepts optional InputEvent for Path B conversion; new handleCopy intercepts clipboard for command pills; sync dependency wired in; createPromptAttachments passes image/composing/sync context.
Main component and popover wiring
packages/app/src/components/prompt-input.tsx, packages/app/src/components/prompt-input/popover-controllers.ts
Component passes sync and renderEditorWithCursor to editor/popover factories; popover uses prependCommandMark on custom command selection, explicitly re-renders pills; onCopy handler attached.
Submit routing for marked commands
packages/app/src/components/prompt-input/submit.ts, packages/app/src/components/prompt-input/submit.test.ts
Path D: validates marked first-part command prefix; emits client.session.command with arguments sliced from validated prefix; reports invariant breaches on mismatch; falls through to legacy parsing on failure.
Command restoration for history flows
packages/app/src/utils/prompt.ts, packages/app/src/utils/prompt.test.ts
extractPromptFromParts now restores command invocations as marked TextParts via createCommandTextPart instead of plain text, preserving pill representation in Undo/Fork/Edit paths.
Invariant reporting and UI utilities
packages/app/src/components/prompt-input/invariant.ts, packages/ui/src/components/command-icon.tsx, plus test files
reportInvariantBreach centralizes dev/prod error handling (async in dev, one-time toast in prod); resolveCommandIconSvg extracts icon lookup into pure helper.
Command pill styling
packages/app/src/components/prompt-input/command-pill.css
[data-cmd-mark] pill layout: inline-flex, brand-primary color, baseline alignment, non-interactive text selection.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • Astro-Han/pawwork#768: Both PRs modify packages/app/src/utils/prompt.ts—specifically the extractPromptFromParts command-mode restoration logic—to change how command invocation content/metadata is reconstructed.

Suggested labels

enhancement, P2, app, ui

Poem

🐰 A slash command, now a pill so fine,
No longer text, but icon'd sign,
Type or paste, select from view,
The prompt input collapses true!
Backspace deletes the mark with care,
While args remain, still floating there.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 34.48% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'collapse / into inline pill' directly summarizes the main change—rendering slash commands as inline pills in the prompt input, matching the feature objective.
Description check ✅ Passed The PR description is comprehensive, covering Summary, Why, Related Issue, Review Focus, Risk Notes, How To Verify, and most checklist items are addressed.
Linked Issues check ✅ Passed The PR fully implements the objectives from #778: inline command pill rendering via Path A (popover), Path B (Space-trigger), Path C (paste), with invariant enforcement, DOM serialization, editor-primitive updates, and comprehensive test coverage for all insertion/deletion paths.
Out of Scope Changes check ✅ Passed All code changes directly support the command-pill feature: new helpers enforce invariants, editor primitives handle pill DOM/cursor math, serializers handle round-trip, submit paths validate pill metadata, and tests cover these flows. No unrelated refactors or dependency changes.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/i778-slash-pill

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added the P2 Medium priority label May 20, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested priority: P2 (includes user-path files (packages/app/src/components/prompt-input.tsx, packages/app/src/components/prompt-input/attachments.ts, packages/app/src/components/prompt-input/command-backspace.test.ts, packages/app/src/components/prompt-input/command-backspace.ts, packages/app/src/components/prompt-input/command-copy.test.ts, packages/app/src/components/prompt-input/command-copy.ts, packages/app/src/components/prompt-input/command-paste.test.ts, packages/app/src/components/prompt-input/command-paste.ts, packages/app/src/components/prompt-input/command-pill.css, packages/app/src/components/prompt-input/command-prepend.test.ts, packages/app/src/components/prompt-input/command-prepend.ts, packages/app/src/components/prompt-input/command-space-trigger.test.ts, packages/app/src/components/prompt-input/command-space-trigger.ts, packages/app/src/components/prompt-input/command-text-part.test.ts, packages/app/src/components/prompt-input/command-text-part.ts, packages/app/src/components/prompt-input/editor-dom.test.ts, packages/app/src/components/prompt-input/editor-dom.ts, packages/app/src/components/prompt-input/editor-input.ts, packages/app/src/components/prompt-input/editor-serialize.test.ts, packages/app/src/components/prompt-input/editor-serialize.ts, packages/app/src/components/prompt-input/invariant.test.ts, packages/app/src/components/prompt-input/invariant.ts, packages/app/src/components/prompt-input/keydown.ts, packages/app/src/components/prompt-input/popover-controllers.ts, packages/app/src/components/prompt-input/submit.test.ts, packages/app/src/components/prompt-input/submit.ts, packages/app/src/context/prompt.test.ts, packages/app/src/context/prompt.tsx, packages/app/src/utils/prompt.test.ts, packages/app/src/utils/prompt.ts)).

P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.

@Astro-Han Astro-Han added the enhancement New feature or request label May 20, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements a comprehensive 'command pill' system for the prompt input, enabling slash commands to be rendered as interactive UI elements. The changes introduce four primary interaction paths: selection from popovers (Path A), conversion of typed commands upon pressing space (Path B), conversion of command-shaped pastes into pills (Path C), and specialized submission routing for command-marked parts (Path D). Key technical additions include clipboard rewriting to ensure the leading slash is preserved during copy operations, a custom backspace fallback ladder for pill deconstruction, and updated DOM serialization logic to maintain command metadata across state changes. Extensive unit tests accompany the new logic, and I have no feedback to provide as no review comments were included.

Astro-Han added 8 commits May 20, 2026 19:06
…ction

Helper module + tests. The editor-input.ts/prompt-input.tsx wiring landed in the preceding Path B commit since they share the same file.
When the user opens the slash popover by typing `/<query>` and then
selects a command from the popover, the previous code prepended the
marked TextPart without removing the typed slash literal, producing
`[marked, Text("/foo")]` with a leftover slash. Detect the slash-query
shape ([Text("/<query>")]) and replace instead of prepend.

Empty prompt and slash-query state both go through the replace branch;
the E1 branch keeps the prepend-with-original-content semantics for
non-empty unrelated prompts.
getTextLength and isNormalizedEditor both walked the DOM without
recognising the command pill. getCursorPosition feeds cloned ranges
through getTextLength, so caret math after a pill was off by one
(visible label has only `name.length` chars but the logical length
is `1 + name.length` for the implicit slash). isNormalizedEditor
returning false on a normal pill child forced the reconcile pass to
rebuild the entire editor on every keystroke, moving the caret and
breaking IME composition.

Both primitives now treat a `data-cmd-mark="true"` element the same
way getNodeLength already did: as an opaque pill whose logical length
is `1 + dataset.name.length`.
@Astro-Han Astro-Han force-pushed the claude/i778-slash-pill branch from 2372019 to 876d6ff Compare May 20, 2026 11:07
@github-actions

github-actions Bot commented May 20, 2026

Copy link
Copy Markdown

Perf delta summary

Comparator: pass

Profile / Scenario interaction median interaction worst long task max tbt frame gap p95 frame gap max jank count cls status
default / homepage-cold 56 -> 24 (-32) 72 -> 40 (-32) 82 -> 66 (-16) 32 -> 16 (-16) 33.3 -> 16.8 (-16.5) 133.3 -> 116.6 (-16.7) 4 -> 3 (-1) 0 -> 0 (0) pass
default / long-session-input-lag 48 -> 40 (-8) 48 -> 64 (+16) 0 -> 0 (0) 0 -> 0 (0) 16.8 -> 16.7 (-0.1) 16.8 -> 16.7 (-0.1) 0 -> 0 (0) 0 -> 0 (0) pass
default / session-streaming-long 56 -> 48 (-8) 72 -> 80 (+8) 0 -> 0 (0) 0 -> 0 (0) 16.8 -> 16.8 (0) 33.4 -> 33.3 (-0.1) 0 -> 0 (0) 0 -> 0 (0) pass
default / tool-call-expand 16 -> 16 (0) 16 -> 24 (+8) 0 -> 0 (0) 0 -> 0 (0) 16.7 -> 16.7 (0) 16.7 -> 16.7 (0) 0 -> 0 (0) 0 -> 0 (0) pass
default / tool-default-open-heavy-bash 32 -> 32 (0) 32 -> 32 (0) 64 -> 63 (-1) 14 -> 13 (-1) 50 -> 50 (0) 166.6 -> 100 (-66.6) 3 -> 3 (0) 0 -> 0 (0) pass
default / terminal-side-panel-open 56 -> 48 (-8) 56 -> 64 (+8) 0 -> 0 (0) 0 -> 0 (0) 33.4 -> 33.4 (0) 50 -> 33.4 (-16.6) 0 -> 0 (0) 0 -> 0 (0) pass
default / session-scroll-reading 32 -> 32 (0) 40 -> 32 (-8) 0 -> 0 (0) 0 -> 0 (0) 16.7 -> 16.8 (+0.1) 16.7 -> 16.8 (+0.1) 0 -> 0 (0) 0.505 -> 0.505 (0) warn: cls
low-end / session-scroll-reading-long 64 -> 64 (0) 80 -> 72 (-8) 60 -> 74 (+14) 14 -> 34 (+20) 16.8 -> 16.8 (0) 183.3 -> 216.7 (+33.4) 2 -> 3 (+1) 0.011 -> 0.011 (0) pass
low-end / session-timeline-recompute 128 -> 128 (0) 136 -> 144 (+8) 114 -> 113 (-1) 194 -> 187 (-7) 100 -> 100 (0) 183.3 -> 200 (+16.7) 3 -> 3 (0) 0.081 -> 0.081 (0) pass
low-end / concurrent-shimmer-extreme 0 -> 0 (0) 0 -> 0 (0) 0 -> 0 (0) 0 -> 0 (0) 16.8 -> 16.8 (0) 33.3 -> 16.8 (-16.5) 0 -> 0 (0) 0 -> 0 (0) pass

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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/app/src/components/prompt-input/command-copy.ts`:
- Around line 36-38: The current check only tests range.startContainer and
range.endContainer and misses ranges that intersect the editor with both
endpoints outside; update the condition in command-copy.ts to also consider the
range.commonAncestorContainer (or otherwise test intersection) so you return
false only when none of startContainer, endContainer or commonAncestorContainer
are inside the editor—for example, replace the single contains check with a
combined check using editor.contains(range.startContainer) ||
editor.contains(range.endContainer) ||
editor.contains(range.commonAncestorContainer) before returning false.

In `@packages/app/src/components/prompt-input/editor-serialize.ts`:
- Around line 71-75: The normalization currently treats any element with
el.dataset.cmdMark === "true" as normalized; change this so a cmd-mark is only
considered normalized when it is the first child (index 0) of its parent. In the
function containing this check in editor-serialize.ts, replace the unconditional
cmdMark return with a test that confirms el.dataset.cmdMark === "true" AND that
el is the parent's first child (e.g., parentElement?.children[0] === el or
computed index === 0); otherwise continue with normal checks (e.g., allow BR
only).
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: ac496bb7-596f-42a1-a602-a677a5b1d977

📥 Commits

Reviewing files that changed from the base of the PR and between a1b78c9 and 2372019.

📒 Files selected for processing (32)
  • packages/app/src/components/prompt-input.tsx
  • packages/app/src/components/prompt-input/attachments.ts
  • packages/app/src/components/prompt-input/command-backspace.test.ts
  • packages/app/src/components/prompt-input/command-backspace.ts
  • packages/app/src/components/prompt-input/command-copy.test.ts
  • packages/app/src/components/prompt-input/command-copy.ts
  • packages/app/src/components/prompt-input/command-paste.test.ts
  • packages/app/src/components/prompt-input/command-paste.ts
  • packages/app/src/components/prompt-input/command-pill.css
  • packages/app/src/components/prompt-input/command-prepend.test.ts
  • packages/app/src/components/prompt-input/command-prepend.ts
  • packages/app/src/components/prompt-input/command-space-trigger.test.ts
  • packages/app/src/components/prompt-input/command-space-trigger.ts
  • packages/app/src/components/prompt-input/command-text-part.test.ts
  • packages/app/src/components/prompt-input/command-text-part.ts
  • packages/app/src/components/prompt-input/editor-dom.test.ts
  • packages/app/src/components/prompt-input/editor-dom.ts
  • packages/app/src/components/prompt-input/editor-input.ts
  • packages/app/src/components/prompt-input/editor-serialize.test.ts
  • packages/app/src/components/prompt-input/editor-serialize.ts
  • packages/app/src/components/prompt-input/invariant.test.ts
  • packages/app/src/components/prompt-input/invariant.ts
  • packages/app/src/components/prompt-input/keydown.ts
  • packages/app/src/components/prompt-input/popover-controllers.ts
  • packages/app/src/components/prompt-input/submit.test.ts
  • packages/app/src/components/prompt-input/submit.ts
  • packages/app/src/context/prompt.test.ts
  • packages/app/src/context/prompt.tsx
  • packages/app/src/utils/prompt.test.ts
  • packages/app/src/utils/prompt.ts
  • packages/ui/src/components/command-icon.test.ts
  • packages/ui/src/components/command-icon.tsx

Comment thread packages/app/src/components/prompt-input/command-copy.ts
Comment thread packages/app/src/components/prompt-input/editor-serialize.ts Outdated
isNormalizedEditor previously accepted a `data-cmd-mark` element at any
position, but spec invariant requires the marked TextPart to be at
index 0 only. A mid-stream pill should signal that some path breached
the invariant; letting the normalizer report it as normal blocks the
reconcile pass from rebuilding the editor from the canonical Prompt
and prevents self-healing. Tighten the predicate to require
`index === 0` and add coverage for both shapes.
Path B's `mirror.input=true` short-circuited reconcile because the raw
`/<name> ` text node passed isNormalizedEditor(): the prompt store gained
a marked TextPart but the editor kept showing raw slash text, breaking
#778's main acceptance path.

Drop the mirror flag in the Path B branch so reconcile falls through to
the non-mirror branch, which compares store vs DOM via isPromptEqual and
forces renderEditorWithCursor when they diverge.

Same call site also passed no target cursor to renderEditorWithCursor,
which then read currentCursor() off the pre-repaint DOM. For Path C
paste this meant caret 0 on an empty editor even though the store said
"end of /<name> args"; for Backspace-into-pill it kept the stale caret
near the deleted pill. Add an optional cursor parameter and have
reconcile pass prompt.cursor() so store-originated repaints honor the
target position the path computed.

Regression covered by path-b-integration.test.ts: pins the chain from
raw DOM + insertText space through to repainted [data-cmd-mark] DOM.
Astro-Han added 8 commits May 20, 2026 19:56
Re-opening the slash popover while the prompt already started with a
marked TextPart and picking another command used to produce
`[marked_new, marked_old, ...rest]`, violating the spec invariant that
a Prompt has at most one marked TextPart and it lives at index 0.

Detect the leading-marked case in prependCommandMark and replace the
old marked TextPart with the new one instead of prepending alongside,
preserving any tail args/files/agents/images by identity. Treat the
user picking a second command as changing their mind about the
command, not stacking two commands.

Update command-prepend.test.ts to assert the at-most-one invariant
and identity preservation of tail parts.
Switching to a second command via prependCommandMark drops the old
args along with the old marked TextPart. The previous commit already
behaved this way via `current.slice(1)`, but the intent was only
implicit. Add a test that asserts `content === "/new "` after
switching from `/old hello world`, plus a comment explaining the
product reasoning: a pill is an atomic unit, and args carry the old
command's semantics that may not map to the new command (e.g.
/summarize "the diff" → /translate would silently feed mismatched
arguments). This kills any ambiguity about whether args-drop was
incidental or intended.
The previous commit dropped old args on command switch out of an
overcautious "args might not fit the new command" reasoning. That
reasoning does not survive contact with the actual command system:
built-in commands like /review and /init both use \$ARGUMENTS as a
plain string substitution (e.g. `git show \$ARGUMENTS`,
`git diff \$ARGUMENTS...HEAD`), and most plugin commands do the same.
There is no schema enforcing what args mean, so any text the user
kept is still legitimate input.

Silently deleting args the user already typed costs them real work
and is not recoverable by re-selecting the original command. The
user can see the args in the editor; if they no longer fit the new
command, they remove them themselves.

Extract args from the leading marked TextPart's content (everything
after the `/<name> ` prefix) and feed them into createCommandTextPart
for the new command. Tail parts (files, agents, images) still
survive by identity. Test reversed to assert args carry over, plus
an empty-args case covering the trailing-space invariant.
Path A/B/C all wrote the new marked Prompt straight into the prompt
store and returned, bypassing the homepage owner mirror logic at the
end of handleInput. Result: a user who turned a slash query into a
pill on the homepage, then navigated away and came back, lost the
pill — the owner still held the pre-pill raw text and replayed that
on hydration. Same shape as the args-drop bug — the system silently
discarded work the user did.

Move the portable/pinned record() call into its own createEffect on
prompt.current() + prompt.context.items() + imageAttachments(). Every
mutation of the prompt store now flows through the same mirror,
regardless of which path produced it. handleInput's tail-only
duplicate is removed, as is the shouldReset branch's explicit
owner clear — both are subsumed by the effect (portable.record()
treats an empty payload as "clear snapshot", so reset still works).

No loop risk: portable.record() and pinned.recordEdit() both dedupe
by deep-equal payload, so hydration paths that set the prompt to
the owner's own snapshot are a no-op on re-record.

Test added to portable-draft.test.ts: a marked TextPart survives a
record + restore + consumeForHomepage round-trip with its command
field intact, pinning the contract the owner has to honor.
The mirror effect introduced last commit fires at mount with the
prompt store's default empty state. Both owner implementations treat
an empty payload as destructive: portable.record() clears the
snapshot via setSnapshot(null), and pinned.recordEdit() overwrites
the existing slot's content in place. Because the mirror effect is
declared before the hydration effect in createEditorInput, the
mount-time fire ran first and destroyed any pending portable
snapshot or pinned prefill before hydration had a chance to
consume them. Net effect: a homepage A→B navigation lost A's
draft, and a pinned prefill on mount got wiped to empty.

Add {defer: true} to the mirror effect's `on()` so the effect
skips its mount-time invocation and waits for the first real
prompt change. That change is whichever comes first: hydration
replaying a snapshot into the prompt (in which case the resulting
record() call is a deep-equal no-op against the owner's own
snapshot, no destructive overwrite), or the user typing
(legitimate record). A fresh homepage with no owner data simply
stays quiet until the user does something.

Two regression guards added to pin the hazard the defer:true
protects against — empty payload through portable.record()
destroys the snapshot, and empty payload through
pinned.recordEdit() overwrites the prefill in place. Anyone
reorganizing the mirror effect will see exactly what they're
defending against.
prompt.current() is session-keyed by {dir, id} in the prompt binding, so
homepage A -> homepage B navigation flips prompt.current() to the new
session's DEFAULT_PROMPT. defer:true only protects the initial mount fire;
the subsequent scope-swap fire would record an empty payload against the
new directory and destroy the snapshot held for the old one before the
hydration effect can move it.

Track lastSeenDir / lastSeenSessionID in the mirror effect and skip the
first fire after either changes. Hydration then runs and either applies a
pinned prefill or consumes a portable carry; the resulting prompt change
re-wakes the effect with refs updated, and recording proceeds normally
(deep-equal dedupe against the snapshot's own contents is a no-op).
Browsers usually fire an additional input event after compositionend with
inputType="insertCompositionText" and composing=false, which would wake
the mirror effect via handleInput. Not all browsers/input methods do; on
those, the final committed prompt sits in the store but never reaches
portable.record() / pinned.recordEdit(), so a draft committed via IME
alone is lost on next homepage navigation.

Add composing() to the mirror effect's tracked deps. The false->true
transition fires the effect (skipped immediately by the compose guard);
the true->false transition fires the effect with the post-commit prompt
and records it. Scope-change refs are unaffected because dir/sessionID
don't change across a compositionend.
…in tests

The mirror effect is the single source of truth for portable/pinned draft
recording. Pull it into owner-mirror.ts so the decision branches (composing
skip, session-route skip, scope-change skip, pinned vs portable routing)
can be exercised directly without going through createEffect.

Tests drive applyOwnerMirrorTick with synthetic state. They cover:
  - IME compositionend records the committed prompt without relying on a
    post-compose input event
  - session-route ticks return skip-session and leave both owners alone
  - homepage A -> homepage B navigation returns skip-scope and preserves
    the snapshot held for the old directory
  - pinned slot bound to the current dir routes to pinned (portable
    untouched); slot bound elsewhere routes to portable

`{defer: true}` on the wrapping createEffect is a SolidJS framework primitive
and is not exercised here; bun's test runner resolves solid-js to its
server build, where createEffect is a no-op. The pinned-draft.test.ts /
portable-draft.test.ts hazard-pin tests at the owner layer remain the
canonical proof of why defer matters.
@Astro-Han Astro-Han merged commit e1ed770 into dev May 20, 2026
26 of 27 checks passed
@Astro-Han Astro-Han deleted the claude/i778-slash-pill branch May 20, 2026 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app Application behavior and product flows enhancement New feature or request P2 Medium priority ui Design system and user interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Collapse slash command into inline mark inside the prompt input

1 participant