Tighten textbox skill fuzzy filtering#5348
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughIndexing now uses a relative-path-based skill searchKey and omits subtitle; nucleo probing returns Swift-ranked probed matches when present or falls back to full-corpus Swift ranking; completion refresh clears or filters stale suggestions on trimmed empty→non-empty transitions; tests and DEBUG accessors added. ChangesMention Search and Index Refresh
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (3 errors, 2 warnings)
✅ Passed checks (13 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Greptile SummaryThis PR tightens skill-mention fuzzy filtering in the textbox by dropping subtitle paths from the search corpus, indexing skills by name plus relative pack path, adding a token pre-filter before the expensive full-corpus fuzzy search, and revising the nucleo probe pipeline to never merge unvalidated rows. Popover behavior is also improved: stale rows are cleared on bare-trigger ↔ typed-query transitions, filtered in-place when narrowing within a non-empty query, and mention refresh is moved into
Confidence Score: 5/5Safe to merge; all changed paths are guarded by new unit and UI tests, and the coordination logic between insertText and didChangeText is correct across both normal and recursive call shapes. The double-refresh coordination (isHandlingDidChangeText / activeInsertTextDepth) correctly handles both the normal AppKit path (super.insertText → didChangeText fires) and the edge case where AppKit skips didChangeText. The empty↔non-empty transition detection is accurate (previousQueryWasEmpty && !queryIsEmpty), and the in-place stale-row filter is safely bounded at 500 entries and runs synchronously on the main actor. The nucleo probe pipeline change is logically sound, and the searchKey relative-path fix directly addresses the ranking problem described in the PR. No pre-existing issues are worsened. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant TextBoxInputTextView
participant Coordinator
participant CompletionController
participant IndexStore
User->>TextBoxInputTextView: insertText(_:replacementRange:)
Note over TextBoxInputTextView: activeInsertTextDepth += 1
TextBoxInputTextView->>TextBoxInputTextView: super.insertText()
TextBoxInputTextView->>TextBoxInputTextView: didChangeText()
Note over TextBoxInputTextView: isHandlingDidChangeText = true
TextBoxInputTextView->>TextBoxInputTextView: flushAutomaticAttachmentFileCleanup()
TextBoxInputTextView->>CompletionController: refreshMentionCompletions()
Note over TextBoxInputTextView: isHandlingDidChangeText = false
TextBoxInputTextView->>Coordinator: textView(_:didChangeText:) [delegate]
Note over Coordinator: isHandlingDidChangeText == true → skip refreshMentionCompletions
Note over TextBoxInputTextView: activeInsertTextDepth -= 1
CompletionController->>CompletionController: refresh(for:rootDirectory:)
alt bare trigger → non-empty query (or trigger/root change)
Note over CompletionController: clear suggestions immediately
else narrowing within non-empty query
CompletionController->>CompletionController: filterVisibleStaleSuggestions(≤500 rows)
Note over CompletionController: suggestionsQuery = nil (marks non-current)
end
CompletionController->>IndexStore: suggestions(for:rootDirectory:) [async]
IndexStore->>IndexStore: TextBoxMentionCandidateIndex.rankedCandidates()
Note over IndexStore: token pre-filter → swiftRankedCandidates(probedCorpus)<br/>full Swift pass only if probe may have missed results
IndexStore-->>CompletionController: [TextBoxMentionSuggestion]
CompletionController->>CompletionController: update suggestions + suggestionsQuery
Reviews (8): Last reviewed commit: "test: harden textbox mention XCUITest so..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| let queryChangedToNonEmpty = previousActiveQuery?.query != query.query && | ||
| !query.query.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty |
There was a problem hiding this comment.
queryChangedToNonEmpty fires on every non-empty keystroke, not just the bare-trigger → first-character transition
The condition checks previousQuery != query.query && !query.query.isEmpty, which is true on every character typed or deleted that leaves a non-empty query — for example, backspacing /iterate-pr to /iterate-p sets it to true and clears the stale rows. The name and the PR description ("clear stale bare-trigger rows when the same trigger changes to a non-empty query") imply the intent is the empty→non-empty edge only, but the implementation acts on any→non-empty. The old test explicitly asserted that editing within the same trigger kept rows visible; that assertion was removed rather than updated, so there is no coverage of the typing-within-a-query path. Every keystroke beyond the first now shows a loading spinner instead of the previous results, which is the flicker the original design was trying to avoid.
| let queryChangedToNonEmpty = previousActiveQuery?.query != query.query && | |
| !query.query.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty | |
| let previousQueryWasEmpty = (previousActiveQuery?.query ?? "") | |
| .trimmingCharacters(in: .whitespacesAndNewlines).isEmpty | |
| let queryChangedToNonEmpty = previousQueryWasEmpty && | |
| !query.query.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty |
There was a problem hiding this comment.
Fixed in 3b699f7 by making the clear condition require the previous query to be empty. Added a regression test that keeps rows visible when a same-trigger query stays nonempty.
— Claude Code
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 4 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4d789aa. Configure here.
There was a problem hiding this comment.
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 `@Sources/TextBoxInput.swift`:
- Line 3658: didChangeText() is redundantly calling refreshMentionCompletions()
while TextBoxInputView.Coordinator.textDidChange(_:) already performs that
refresh on every edit; remove the duplicate refresh call from didChangeText()
(or add a guard to skip it when invoked by the coordinator) so
refreshMentionCompletions() is only invoked once per keystroke, and ensure any
non-coordinator code paths that legitimately need a mention refresh still call
refreshMentionCompletions() explicitly.
In `@Sources/TextBoxMentionCandidateIndex.swift`:
- Around line 98-102: The eager prefilter in TextBoxMentionCandidateIndex (where
you compute preparedQuery via CommandPaletteFuzzyMatcher.preparedQuery and then
build filteredEntries with entries.filter { mentionCandidate($0, matches:
preparedQuery) }) must respect cancellation; modify that path to periodically
call the provided shouldCancel() closure (or Task.isCancelled) while scanning
entries and abort early if it returns true. Replace the single eager
entries.filter call with a cancellable loop or a lazy sequence that checks
shouldCancel() between iterations, returning an empty result immediately when
cancelled so the subsequent CommandPaletteSearchEngine.search can bail out
quickly.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 0dc5ed73-b044-41f7-904f-4c86a803635d
📒 Files selected for processing (3)
Sources/TextBoxInput.swiftSources/TextBoxMentionCandidateIndex.swiftcmuxTests/TextBoxMentionCompletionTests.swift
![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: 7b6a29b434
ℹ️ 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".
| "mention_should_show": mentionCompletionController.debugShouldShowPopover, | ||
| "mention_current": mentionCompletionController.debugHasCurrentSuggestions, | ||
| "mention_titles": mentionCompletionController.debugSuggestionTitles |
There was a problem hiding this comment.
In non-DEBUG builds, TextBoxMentionCompletionController does not define debugShouldShowPopover, debugHasCurrentSuggestions, or debugSuggestionTitles because those accessors are inside #if DEBUG, but debugInteractionState() is compiled unconditionally. Building Release or any non-DEBUG configuration will fail to type-check when it reaches these new dictionary entries; wrap these fields in #if DEBUG or use non-debug accessors.
Useful? React with 👍 / 👎.
lawrencecchen
dismissed
coderabbitai[bot]’s stale review
Stale CodeRabbit review. Actionable comments were addressed in 7b6a29b, and the latest CodeRabbit check passed/skipped on the current head.
lawrencecchen
force-pushed
the
issue-textbox-skill-fuzzy-filter
branch
from
f2b44f2 to
bcf22d6
Compare
lawrencecchen
force-pushed
the
issue-textbox-skill-fuzzy-filter
branch
from
25d689b to
c09de9c
Compare
lawrencecchen
force-pushed
the
issue-textbox-skill-fuzzy-filter
branch
from
c09de9c to
611714a
Compare
1 participant
Summary
Verification
Local xcodebuild tests were not run because this repo forbids local test actions on the user Mac.
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Touches NSTextView text-change/insert paths and mention indexing used on every keystroke; behavior is well covered by new unit and UI tests but regressions could affect input or completion UX.
Overview
Tightens textbox skill mention search and keeps the completion popover in sync while typing.
Skill indexing and ranking no longer treat absolute subtitle paths as searchable text; skills are keyed by name plus relative pack path, and the candidate index prefilters with token matching before ranking so weak path-only fuzzy hits (e.g.
iteratesurfacing unrelated skills) drop out. Nucleo probe results are validated with Swift ranking and only fall back to a full-corpus pass when the probe may have missed matches.The mention controller clears suggestions when crossing bare trigger ↔ typed query or changing trigger/root; while the query narrows on the same trigger it filters visible stale rows in place instead of flashing empty.
TextBoxInputTextViewrefreshes completions fromdidChangeText/ outerinsertText, withisHandlingDidChangeTextso delegate notifications do not double-refresh; debug mention fields and an XCUITest path exercise$+iteratefiltering via the automation socket.Reviewed by Cursor Bugbot for commit 611714a. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Improves textbox skill mentions so exact skill names rank first, weak path-only fuzzy matches are filtered out, and the popover shows only relevant rows while you type. Safer text-change handling avoids duplicate scans, and new debug fields support an end-to-end UI test.
Bug Fixes
nucleorows; after anucleoprobe, return Swift-ranked matches and only run a full Swift pass if the probe likely missed and the limit isn’t filled.didChangeTextand after directinsertText, with guards to avoid duplicate scans and saferinsertText/attachment cleanup ordering; expose mention state (active/query/trigger/loading/current) and titles for automation; add a UI test that types after a bare$trigger and harden socket diagnostics/launch.Refactors
Written for commit 611714a. Summary will update on new commits.
Summary by CodeRabbit
Improvements
Tests