Composer Chips Anywhere#703
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughIntroduces a shared composer-trigger detection/replacement module used across the ADE CLI TUI, desktop AgentChatComposer, ChatCommandMenu, and WorkViewArea for mid-sentence ChangesShared composer trigger utility and TUI/desktop integration
iOS typed-trigger composer
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
|
@copilot review but do not make fixes |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
apps/ios/ADE/Views/Work/WorkComposerTypedTriggers.swift (2)
198-260: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUnresolved lane keeps retrying
listWorkspaces()on every@fetch.
resolveWorkspaceIdonly caches a successful lookup (Line 258:if let resolved { cachedWorkspaceId = resolved }). If the lane hasn't synced a matching workspace yet (or the laneId never resolves), every keystroke's debounced fetch re-issueslistWorkspaces()over sync, with no backoff. Consider caching a "not found" sentinel with a short TTL, or resolving once whenlaneIdis set instead of per-query.🤖 Prompt for 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. In `@apps/ios/ADE/Views/Work/WorkComposerTypedTriggers.swift` around lines 198 - 260, The `scheduleFileFetch` flow keeps calling `resolveWorkspaceId` and `sync.listWorkspaces()` on every debounced `@` query when a lane has no resolved workspace yet. Update `resolveWorkspaceId(laneId:sync:)` to cache the unresolved state too, using a short-lived sentinel/TTL or by resolving once when `laneId` changes, so repeated keystrokes don’t re-run workspace discovery. Keep the existing success cache (`cachedWorkspaceId`) but also prevent repeated retries for the same unresolved lane until the cache expires or the lane changes.
32-67: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTie-break branch appears unreachable given the regex constraints.
Both
slashRegexandatRegexanchor with$and their capture groups exclude whitespace. For both to match the same(text, cursor), the character immediately before the "loser" trigger would have to be whitespace, but that position necessarily falls inside the "winner" trigger's whitespace-excluding capture span — a contradiction. In practice, at most one of the two can ever match, socase let (s?, a?):(Line 57‑58) is dead code, and the two "closest wins" tests (testClosestToCursorWinsWhenBothMatch,testSlashWinsWhenItIsClosestinWorkComposerTriggerDetectorTests.swift) exercise "only one matched" rather than an actual tie-break. Given this PR's goal is exact parity with the desktop/TUI regex behavior, worth confirming the desktopcomposerTriggers.tsimplementation doesn't rely on a genuinely reachable dual-match tie-break that iOS is silently missing.import re slash_re = re.compile(r'(?:^|\s)/([^\s/]*)$') at_re = re.compile(r'(?:^|\s)@([^\s@]*)$') import itertools, string # brute-force short strings to look for any text where both match simultaneously alphabet = list(" ab/@") found = False for n in range(1, 7): for combo in itertools.product(alphabet, repeat=n): s = ''.join(combo) if slash_re.search(s) and at_re.search(s): found = True print(repr(s)) break if found: break print("found any dual match:", found)🤖 Prompt for 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. In `@apps/ios/ADE/Views/Work/WorkComposerTypedTriggers.swift` around lines 32 - 67, The dual-match tie-break in WorkComposerTriggerDetector.detect is unreachable because slashRegex and atRegex cannot both match the same prefix, so the `case let (s?, a?)` branch is dead. Remove that branch and simplify the selection logic so the detector returns the single matching WorkComposerTriggerMatch, or align it with the desktop composerTriggers.ts behavior only if that implementation truly depends on a reachable tie-break. Update WorkComposerTriggerDetectorTests to cover the actual single-match behavior rather than asserting a nonexistent tie.
🤖 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 `@apps/desktop/src/main/services/files/fileSearchIndexService.ts`:
- Around line 399-402: The quick-open cache is being invalidated too early in
the file update flow, allowing a query to repopulate stale results before the
async `shouldIgnore(...).then(...)` work finishes. Update
`fileSearchIndexService` so `invalidateQuickOpenCache(index)` runs after
`removePath` or `upsertFile` completes in the `matchingIndexes` update path, and
keep the guard around unbuilt empty indexes intact. Refer to `matchingIndexes`,
`invalidateQuickOpenCache`, `removePath`, and `upsertFile` to place the
invalidation at the end of the async update.
In `@apps/ios/ADE/Views/Work/WorkComposerTypedTriggers.swift`:
- Around line 472-490: The trigger detection in textViewDidChange is missing the
same IME composition guard used in textViewDidChangeSelection, so it can fire on
marked text and show suggestions mid-composition. Update
WorkComposerTypedTriggers’s textViewDidChange flow to call detectTrigger only
when textView.markedTextRange is nil, keeping it consistent with the existing
restyle and selection handling logic.
- Around line 303-357: The custom TextKit setup in WorkComposerTextView is being
created as local objects in makeUIView, so the NSTexStorage and
WorkComposerChipLayoutManager can be deallocated after setup. Move ownership of
that stack into the Coordinator (or another long-lived property) and have
makeUIView wire the UITextView to those persistent instances so the chip layout
remains active for the lifetime of the view.
---
Nitpick comments:
In `@apps/ios/ADE/Views/Work/WorkComposerTypedTriggers.swift`:
- Around line 198-260: The `scheduleFileFetch` flow keeps calling
`resolveWorkspaceId` and `sync.listWorkspaces()` on every debounced `@` query
when a lane has no resolved workspace yet. Update
`resolveWorkspaceId(laneId:sync:)` to cache the unresolved state too, using a
short-lived sentinel/TTL or by resolving once when `laneId` changes, so repeated
keystrokes don’t re-run workspace discovery. Keep the existing success cache
(`cachedWorkspaceId`) but also prevent repeated retries for the same unresolved
lane until the cache expires or the lane changes.
- Around line 32-67: The dual-match tie-break in
WorkComposerTriggerDetector.detect is unreachable because slashRegex and atRegex
cannot both match the same prefix, so the `case let (s?, a?)` branch is dead.
Remove that branch and simplify the selection logic so the detector returns the
single matching WorkComposerTriggerMatch, or align it with the desktop
composerTriggers.ts behavior only if that implementation truly depends on a
reachable tie-break. Update WorkComposerTriggerDetectorTests to cover the actual
single-match behavior rather than asserting a nonexistent tie.
🪄 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
Run ID: e262374d-62ed-4590-88d4-1f0fec8a5cbb
⛔ Files ignored due to path filters (5)
apps/ios/ADE.xcodeproj/project.pbxprojis excluded by!**/*.xcodeproj/project.pbxprojdocs/features/ade-code/README.mdis excluded by!docs/**docs/features/chat/composer-and-ui.mdis excluded by!docs/**docs/features/files-and-editor/file-watcher-and-trust.mdis excluded by!docs/**docs/features/sync-and-multi-device/ios-companion.mdis excluded by!docs/**
📒 Files selected for processing (17)
apps/ade-cli/src/tuiClient/app.tsxapps/desktop/src/main/services/adeActions/registry.test.tsapps/desktop/src/main/services/adeActions/registry.tsapps/desktop/src/main/services/files/fileSearchIndexService.tsapps/desktop/src/main/services/files/fileService.test.tsapps/desktop/src/main/services/files/fileService.tsapps/desktop/src/renderer/components/chat/AgentChatComposer.test.tsxapps/desktop/src/renderer/components/chat/AgentChatComposer.tsxapps/desktop/src/renderer/components/chat/ChatCommandMenu.tsxapps/desktop/src/renderer/components/terminals/WorkViewArea.tsxapps/desktop/src/shared/composerTriggers.test.tsapps/desktop/src/shared/composerTriggers.tsapps/ios/ADE/Views/Work/WorkChatSessionView.swiftapps/ios/ADE/Views/Work/WorkComposerTypedTriggers.swiftapps/ios/ADE/Views/Work/WorkMentionsPickerSheet.swiftapps/ios/ADE/Views/Work/WorkSlashCommandsSheet.swiftapps/ios/ADETests/WorkComposerTriggerDetectorTests.swift
💤 Files with no reviewable changes (2)
- apps/ios/ADE/Views/Work/WorkMentionsPickerSheet.swift
- apps/ios/ADE/Views/Work/WorkSlashCommandsSheet.swift
arul28
force-pushed
the
ade/composer-chips-anywhere-a9d02bd6
branch
from
18f9ad2 to
f2f8c19
Compare
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2f8c19c8c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5341f3de2e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…across desktop, TUI, and iOS - shared/composerTriggers.ts: detectComposerTrigger + replaceComposerTriggerSpan; slash and @ triggers work at any cursor position with word-boundary guards (paths/URLs/fractions/emails never trigger) - Desktop: both composer inputs (rich contenteditable + textarea) and WorkViewArea consume the shared util; selection splices exactly the trigger span; confirmed tokens render as chips (chip nodes in rich mode, backdrop overlay in textarea); IME composition guarded - ChatCommandMenu: 300ms -> 40ms debounce, per-menu query cache with same-frame warm hits, stale-result sequence guard, spinner only when nothing to show - fileSearchIndexService: name index split from content index (no file reads on build; quickOpen never touches content; searchText loads lazily with bounds); per-query quickOpen cache invalidated by watcher events - registry fileSearch: sessionId->laneId cache via getSessionSummary; empty query warms the index (fileService.warmQuickOpenIndex); composer pings on session bind so first @ is instant - TUI: prompt trigger detection is cursor-relative via the same shared module; slash menu works mid-sentence (Enter/Tab complete the token); confirmed tokens render as colored chips - iOS: new typed-trigger detection (UITextView-backed composer), inline suggestion strip above the keyboard, attributed chip pills; dead WorkSlashCommandsSheet/WorkMentionsPickerSheet removed Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… detection, judo cleanups - ChatCommandMenu.selectCurrent returns whether a row was selected; Enter/Tab fall through to send when a mid-sentence /token or @token has no match (desktop composer + WorkViewArea via handleCommandMenuKeyDown) instead of becoming a dead key; empty-menu arrow guards - iOS: WorkComposerSuggestionController clears its cached workspaceId when laneId changes so @ quick-open never searches the previous lane - registry fileSearch: skip the laneId cache for identity (CTO/worker) sessions — resumeSession can migrate their lane; regular sessions stay cached - shared findConfirmedComposerTokens replaces duplicated token scans in the desktop overlay and TUI prompt renderer (+tests) - collapse duplicate legacy slash-select branches; extract evaluatePlainTrigger for the three textarea handlers Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Fold fileSearchIndexService tests into fileService.test.ts (helper has a single parent module; removes the fragment file) - AgentChatComposer.test.tsx +3: mid-sentence slash splice, Enter fall-through on unmatched token (quality A1 regression), mid-sentence @file splice + attachment - iOS: WorkComposerTriggerDetectorTests (14 pure-logic tests; suite green on simulator) - Docs parity: composer-and-ui source map (composerTriggers + ChatCommandMenu rows), ade-code composer section, ios-companion composer section, file-watcher session-lane cache note - CLI/TUI parity verified clean (headless daemon shares the action registry; TUI fall-through matches desktop) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…in iOS TextKit stack; IME-guard trigger detection - fileSearchIndexService.onFileChanged invalidates the quickOpen query cache again after the deferred removePath/upsertFile applies, so a quickOpen in the gap cannot pin a stale result set - WorkComposerTextView retains its manually-built NSTextStorage/NSLayoutManager on the coordinator (UITextView only retains the text container) - textViewDidChange skips detectTrigger while marked text is active, matching the didChangeSelection guard Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…palette query, mouse insert
Sweep the remaining leading-slash assumptions in the TUI palette plumbing:
arrow-key guards key off live slashRows (not prompt.startsWith("/")), the
slash palette renders for any active slash trigger, its query derives from
the trigger span, and mouse-clicking a row splices the trigger span via
insertSlashCommandRow instead of replacing the whole prompt.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
arul28
force-pushed
the
ade/composer-chips-anywhere-a9d02bd6
branch
from
5341f3d to
03ed94c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 03ed94c664
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
ensureBuilt now awaits an in-flight buildingPromise before the early return, and buildWorkspace invalidates the quickOpen query cache on completion (both exit paths), so a query racing a warm build can neither see nor pin partial results. Adds an in-flight-build regression test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Composer chips that work anywhere, instantly
Typing
/command, a skill name, or@fileANYWHERE in the prompt composer (not just at position 0) now surfaces suggestions instantly and converts to an inline chip — across desktop, the ade-code TUI, and iOS.Trigger detection (shared)
apps/desktop/src/shared/composerTriggers.ts:detectComposerTrigger(text, cursorPos)finds an in-progress/command((?:^|\s)\/([^\s/]*)$) or@filetoken ending at the cursor;replaceComposerTriggerSpansplices exactly the trigger span;findConfirmedComposerTokenslocates confirmed chip tokens. Word-boundary guards keep paths (/usr/bin), fractions (3/4), URLs, and emails from triggering.WorkViewAreacontinue composer, and the TUI (direct import). iOS mirrors the regexes byte-identically in Swift.Anywhere insertion + chips
fix @src/foo.ts then run /test). Lone leading commands keep the legacy path (local/clearintercept, argument-hint scaffold).Performance
ChatCommandMenu: 300ms → 40ms debounce, per-menu query cache (cache hits render same-frame, silent revalidation), stale-result sequence guard, spinner only when nothing to show.fileSearchIndexService: name index split from content index — the build walk never reads file contents,quickOpennever waits on them;searchTextloads lazily (bounded, binary-sniffed, cooperative yields). Per-query quickOpen cache invalidated by watcher events.registry.fileSearch: sessionId→laneId cache viagetSessionSummary(identity sessions excluded —resumeSessioncan migrate their lane); empty-query calls warm the index via newfileService.warmQuickOpenIndexand the composer pings on session bind, so the first@is served from a warm index.iOS
WorkComposerTypedTriggers.swift: cursor-relative detector, suggestion controller (files via existingSyncService.quickOpen, curated slash catalog), inline strip above the keyboard, UITextView-backed composer with chip pills. DeadWorkSlashCommandsSheet/WorkMentionsPickerSheetdeleted. Build + 14 detector tests green.Tests
fileService.test.ts; registry laneId-cache tests; 14 iOS detector tests. Desktop/ade-cli typecheck clean; 813 TUI tests green.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
@and/suggestions now work in the middle of a draft, not just at the start.Bug Fixes
Greptile Summary
This PR adds composer chips that work anywhere in the prompt. The main changes are:
/commandand@filetrigger detection.Confidence Score: 5/5
This PR appears safe to merge with low risk.
No verified issues were found in the changed paths. The changes include focused coverage for shared triggers, composer regressions, file indexing, registry lane caching, and iOS detection.
No files require special attention.
What T-Rex did
Important Files Changed
/and@trigger utilities with boundary guards and token confirmation helpers.Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant User participant Composer as Composer UI participant Trigger as detectComposerTrigger participant Menu as ChatCommandMenu / SuggestionStrip participant Registry as chat.fileSearch participant Files as fileService.quickOpen User->>Composer: "Type /command or @file anywhere" Composer->>Trigger: Detect token ending at cursor Trigger-->>Composer: Trigger kind, query, start span alt Slash trigger Composer->>Menu: Show filtered command rows User->>Menu: Select command Menu->>Composer: Replace trigger span with chip else File trigger Composer->>Menu: Request file suggestions Menu->>Registry: fileSearch(sessionId, query) Registry->>Files: quickOpen(workspace lane, query) Files-->>Registry: Path matches Registry-->>Menu: File suggestions User->>Menu: Select file Menu->>Composer: Replace trigger span and attach file end%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant User participant Composer as Composer UI participant Trigger as detectComposerTrigger participant Menu as ChatCommandMenu / SuggestionStrip participant Registry as chat.fileSearch participant Files as fileService.quickOpen User->>Composer: "Type /command or @file anywhere" Composer->>Trigger: Detect token ending at cursor Trigger-->>Composer: Trigger kind, query, start span alt Slash trigger Composer->>Menu: Show filtered command rows User->>Menu: Select command Menu->>Composer: Replace trigger span with chip else File trigger Composer->>Menu: Request file suggestions Menu->>Registry: fileSearch(sessionId, query) Registry->>Files: quickOpen(workspace lane, query) Files-->>Registry: Path matches Registry-->>Menu: File suggestions User->>Menu: Select file Menu->>Composer: Replace trigger span and attach file endComments Outside Diff (1)
apps/desktop/src/main/services/files/fileSearchIndexService.ts, line 413-418 (link)onFileChangedclearsquickOpenCachebefore the asyncshouldIgnore(...).then(upsertFile)mutation finishes. When aquickOpenrequest lands in that window, it recomputes results from the oldindex.filesand stores them in the now-empty cache. After line 418 adds the changed file, the cache is not cleared again, so file search can keep returning stale results until another watcher event.Artifacts
Repro: focused Vitest harness for the quickOpen stale cache race
Repro: verbose failing test output showing stale cached quickOpen result after watcher upsert
Prompt To Fix With AI
Reviews (5): Last reviewed commit: "review: never serve or cache a partially..." | Re-trigger Greptile