[AI] Replace payee and category autocomplete filter/sort with fzf fuzy search#7261
[AI] Replace payee and category autocomplete filter/sort with fzf fuzy search#7261RiiD wants to merge 7 commits intoactualbudget:masterfrom
Conversation
actual-github-bot
bot
changed the title
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
👋 Hello contributor! We would love to review your PR! Before we can do that, please make sure:
We do this to reduce the TOIL the core contributor team has to go through for each PR and to allow for speedy reviews and merges. For more information, please see our Contributing Guide. |
✅ Deploy Preview for actualbudget-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for actualbudget-website ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
RiiD
force-pushed
the
feat/fzf-autocomplete
branch
from
a94b595 to
c2d47b6
Compare
📝 WalkthroughWalkthroughAdds Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx (1)
499-533:⚠️ Potential issue | 🟠 MajorApply the same matcher to nearby payees.
This only runs
FzfoverrealSuggestions; nearby payees are still filtered on a separate path and then prepended/highlighted ahead of these results. That leaves one dropdown using two different matching rules, so the default selected row can come from nearby-payee matching instead of the new fuzzy payee search. Nearby payees should go through the same matcher/casing rules before the list is merged.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx` around lines 499 - 533, filterSuggestions currently only runs Fzf on realSuggestions, leaving nearby-payee items filtered by a different path and merged afterwards; change it so nearby payees are passed through the same matcher/casing rules before merging: locate filterSuggestions and instead of directly prepending nearby items, run the same Fzf matcher (using selector: item => item.name ?? '' and limit: 100) against the nearby-payees list (or construct a single combined input to Fzf that includes nearby items flagged) so both sets are matched equivalently, map Fzf results to items, deduplicate by id, reapply the existing 'new' sentinel logic (showNew, exact-match normalization with getNormalisedString) and then return the merged, de-duplicated ordered results so nearby payees cannot bypass the fuzzy matching used for realSuggestions.packages/desktop-client/src/components/autocomplete/CategoryAutocomplete.tsx (1)
258-279:⚠️ Potential issue | 🟠 MajorDon't feed a globally ranked result list into the grouped renderer unchanged.
fzf.find(value)now returns a single relevance-ordered list, butCategoryListlater buckets that list back bygroup.idwhile keeping the original indexes. Once matches interleave groups, lower-ranked rows from the first group render above higher-ranked rows from later groups, and arrow-key highlighting jumps because the visual order no longer matches the suggestion order. Preserve the ranked order while searching, or render group headers inline on first occurrence instead of regrouping.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/desktop-client/src/components/autocomplete/CategoryAutocomplete.tsx` around lines 258 - 279, filterSuggestions currently returns a single relevance-ordered list (from Fzf.find) which is later re-bucketed by CategoryList causing visual/order inconsistencies; fix by preserving ranked order and emitting group header sentinel items so group headers render inline instead of regrouping: update filterSuggestions to iterate the ranked filtered results (result.item), track first-seen group ids, and when encountering a new group insert a synthetic header item (e.g., id like `group-header:<group.id>` or a flag on the item) before that group's first real item (also keep splitItem handling), so CategoryList can render headers inline without reordering the ranked suggestions.
🧹 Nitpick comments (1)
packages/desktop-client/package.json (1)
21-23: KeepfzfindevDependenciesfordesktop-client.
packages/desktop-clientintentionally uses a dev-only dependency manifest, so introducing a separatedependenciesblock makes this package inconsistent with the rest of the app packaging setup.Based on learnings, `packages/desktop-client/package.json` intentionally keeps runtime packages under `devDependencies` only for this bundled app.♻️ Suggested manifest change
- "dependencies": { - "fzf": "^0.5.2" - }, "devDependencies": { @@ "downshift": "9.0.10", + "fzf": "^0.5.2", "hyperformula": "^3.1.1",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/desktop-client/package.json` around lines 21 - 23, The package.json for packages/desktop-client incorrectly adds "fzf" under "dependencies"; move "fzf": "^0.5.2" from the top-level "dependencies" block into the existing "devDependencies" section so the desktop-client stays a dev-only manifest (ensure you remove the now-empty "dependencies" block if nothing else lives there and keep the version string intact).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@packages/desktop-client/src/components/autocomplete/CategoryAutocomplete.tsx`:
- Around line 258-279: filterSuggestions currently returns a single
relevance-ordered list (from Fzf.find) which is later re-bucketed by
CategoryList causing visual/order inconsistencies; fix by preserving ranked
order and emitting group header sentinel items so group headers render inline
instead of regrouping: update filterSuggestions to iterate the ranked filtered
results (result.item), track first-seen group ids, and when encountering a new
group insert a synthetic header item (e.g., id like `group-header:<group.id>` or
a flag on the item) before that group's first real item (also keep splitItem
handling), so CategoryList can render headers inline without reordering the
ranked suggestions.
In `@packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx`:
- Around line 499-533: filterSuggestions currently only runs Fzf on
realSuggestions, leaving nearby-payee items filtered by a different path and
merged afterwards; change it so nearby payees are passed through the same
matcher/casing rules before merging: locate filterSuggestions and instead of
directly prepending nearby items, run the same Fzf matcher (using selector: item
=> item.name ?? '' and limit: 100) against the nearby-payees list (or construct
a single combined input to Fzf that includes nearby items flagged) so both sets
are matched equivalently, map Fzf results to items, deduplicate by id, reapply
the existing 'new' sentinel logic (showNew, exact-match normalization with
getNormalisedString) and then return the merged, de-duplicated ordered results
so nearby payees cannot bypass the fuzzy matching used for realSuggestions.
---
Nitpick comments:
In `@packages/desktop-client/package.json`:
- Around line 21-23: The package.json for packages/desktop-client incorrectly
adds "fzf" under "dependencies"; move "fzf": "^0.5.2" from the top-level
"dependencies" block into the existing "devDependencies" section so the
desktop-client stays a dev-only manifest (ensure you remove the now-empty
"dependencies" block if nothing else lives there and keep the version string
intact).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 46471b83-eb0a-47a6-a3f4-9c52e6bc3ce7
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
packages/desktop-client/package.jsonpackages/desktop-client/src/components/autocomplete/CategoryAutocomplete.tsxpackages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/desktop-client/src/components/autocomplete/CategoryAutocomplete.tsx (1)
268-272: Consider adding a result limit for consistency with PayeeAutocomplete.PayeeAutocomplete uses
limit: 100in its fzf options, but CategoryAutocomplete doesn't set a limit. While category lists are typically smaller than payee lists, adding a consistent limit would align the implementations and prevent potential performance issues if category lists grow.♻️ Suggested fix
const fzf = new Fzf(realSuggestions, { selector: item => item.group ? item.group.name + ' ' + item.name : item.name, + limit: 100, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/desktop-client/src/components/autocomplete/CategoryAutocomplete.tsx` around lines 268 - 272, In CategoryAutocomplete, when constructing the Fzf instance (the variable fzf built from realSuggestions with selector), add a limit option (e.g., limit: 100) to match PayeeAutocomplete and cap results; update the Fzf instantiation in CategoryAutocomplete to include limit: 100 so filtered (the result of fzf.find(value).map(...)) is bounded and consistent with PayeeAutocomplete behavior.packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx (1)
455-463: Inconsistent filtering: nearby payees use substring matching while regular payees use fuzzy search.
filteredNearbyPayeesstill usesdefaultFilterSuggestion(substring matching), whereas the main payee list now uses fzf fuzzy matching. This inconsistency means a query likehdepwould match "Home Depot" in regular payees but not in nearby payees.Consider applying fzf filtering to nearby payees as well for consistent UX.
♻️ Suggested fix
const filteredNearbyPayees = useMemo(() => { if (!nearbyPayeesWithType.length || !rawPayee) { return nearbyPayeesWithType; } - return nearbyPayeesWithType.filter(payee => { - return defaultFilterSuggestion(payee, rawPayee); - }); + const fzf = new Fzf(nearbyPayeesWithType, { + selector: item => item.name ?? '', + }); + return fzf.find(rawPayee).map(result => result.item); }, [nearbyPayeesWithType, rawPayee]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx` around lines 455 - 463, filteredNearbyPayees uses substring matching via defaultFilterSuggestion while the main payee list uses the fzf fuzzy matcher, causing inconsistent results; update the filteredNearbyPayees memo to use the same fuzzy filtering function used by the main payee list (the fzf-based matcher) instead of defaultFilterSuggestion, preserving any existing tie-break/order logic and ensuring rawPayee is passed the same way; reference the symbols filteredNearbyPayees, nearbyPayeesWithType, rawPayee and replace defaultFilterSuggestion with the project’s fzf filter function (the same one used for regular payees) and import it if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@packages/desktop-client/src/components/autocomplete/CategoryAutocomplete.tsx`:
- Around line 268-272: In CategoryAutocomplete, when constructing the Fzf
instance (the variable fzf built from realSuggestions with selector), add a
limit option (e.g., limit: 100) to match PayeeAutocomplete and cap results;
update the Fzf instantiation in CategoryAutocomplete to include limit: 100 so
filtered (the result of fzf.find(value).map(...)) is bounded and consistent with
PayeeAutocomplete behavior.
In `@packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx`:
- Around line 455-463: filteredNearbyPayees uses substring matching via
defaultFilterSuggestion while the main payee list uses the fzf fuzzy matcher,
causing inconsistent results; update the filteredNearbyPayees memo to use the
same fuzzy filtering function used by the main payee list (the fzf-based
matcher) instead of defaultFilterSuggestion, preserving any existing
tie-break/order logic and ensuring rawPayee is passed the same way; reference
the symbols filteredNearbyPayees, nearbyPayeesWithType, rawPayee and replace
defaultFilterSuggestion with the project’s fzf filter function (the same one
used for regular payees) and import it if needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8725ed81-f60d-4f83-9462-e3a7012e8305
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
packages/desktop-client/package.jsonpackages/desktop-client/src/components/autocomplete/CategoryAutocomplete.tsxpackages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsxupcoming-release-notes/7261.md
✅ Files skipped from review due to trivial changes (2)
- packages/desktop-client/package.json
- upcoming-release-notes/7261.md
|
VRT tests ❌ failed. View the test report. To update the VRT screenshots, comment |
|
/update-vrt |
Auto-generated by VRT workflow PR: actualbudget#7261
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx (1)
497-532: Fzf integration looks correct; minor redundancy in condition.The fuzzy search implementation properly separates the 'new' sentinel, applies Fzf filtering with appropriate limits, and handles exact-match removal logic correctly.
One minor observation: at line 517,
value && value !== ''is redundant since a truthy check on a string already excludes empty strings.🔧 Optional: Simplify redundant condition
- const showNew = newItem && value && value !== '' && !focusTransferPayees; + const showNew = newItem && value && !focusTransferPayees;As per coding guidelines: "the React Compiler automatically memoizes component bodies; omit manual
useCallback,useMemo, andReact.memowhen adding or refactoring code" - so the inline function definition withoutuseCallbackis appropriate for this codebase.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx` around lines 497 - 532, The condition that decides whether to show the 'new' sentinel is redundant: in the expression that sets showNew inside filterSuggestions, replace the redundant check "value && value !== ''" with a single truthy check "value" (refer to the showNew assignment that uses newItem, value, and focusTransferPayees) so the logic remains the same but without the needless duplicate empty-string comparison.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx`:
- Around line 497-532: The condition that decides whether to show the 'new'
sentinel is redundant: in the expression that sets showNew inside
filterSuggestions, replace the redundant check "value && value !== ''" with a
single truthy check "value" (refer to the showNew assignment that uses newItem,
value, and focusTransferPayees) so the logic remains the same but without the
needless duplicate empty-string comparison.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 03c68389-2581-48a1-827f-8d5a12f77c68
📒 Files selected for processing (3)
packages/desktop-client/package.jsonpackages/desktop-client/src/components/autocomplete/CategoryAutocomplete.tsxpackages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
✅ Files skipped from review due to trivial changes (1)
- packages/desktop-client/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/desktop-client/src/components/autocomplete/CategoryAutocomplete.tsx
RiiD
changed the title
|
This looks great. I hope it gets approved and merged! |
|
Hey! Thanks for this, it's a good idea. Looking at |
|
There has also just been a change merged to improve searching, have a look at #6972 and see what you think to that solution. It has also introduced some conflicts to your branch. |
|
If a library is stable, does it need to constantly update? https://www.npmjs.com/package/react-fzf exists and was published 5 months ago, but just adds a wrapper around the fzf package for react. A different alternative may be https://www.npmjs.com/package/flashfuzzy, which uses all the cool new things, like Rust and WASM. |
3 participants
Description
This PR replaces the custom substring-based filter and sort logic in
PayeeAutocompleteandCategoryAutocompletewith thefzffuzzy search library.Why is this needed?
When adding transactions manually, finding the right payee or category quickly matters a lot. The existing autocomplete only matches substrings — so if you have many payees starting with "Home" (Home Depot, Home Hardware, Home Insurance...), you have to type almost the full name before the right one surfaces. There's no way to narrow down by word initials or non-contiguous characters.
With fzf, you can type
hdepand immediately getHome Depotas the top result — the search understands word boundaries and ranks results by match quality. This is the kind of fuzzy search users are accustomed to from editors, terminals, and most modern search UIs.This benefits anyone who adds transactions manually and has a large payee list, which is a common workflow for Actual users who prefer hands-on budgeting.
Changes:
food grocmatchesFood › Groceries)Before/After
Related issue(s)
None
Testing
msshould surfaceMS OfficebeforeMostafa SupermarketbeforeMaxims Bakery)MS) and verify only case-matching results appearfood) — verify categories from that group appearfood grocand verifyFood › GroceriesmatchesChecklist
Bundle Stats
View detailed bundle stats
desktop-client
Total
Changeset
node_modules/fzf/dist/fzf.es.jsnode_modules/@tanstack/query-core/build/modern/environmentManager.jsnode_modules/@babel/runtime/helpers/esm/inheritsLoose.jsnode_modules/@babel/runtime/helpers/esm/setPrototypeOf.jsnode_modules/@babel/runtime/helpers/esm/objectWithoutPropertiesLoose.jsnode_modules/@babel/runtime/helpers/esm/extends.jsnode_modules/react-i18next/dist/es/useTranslation.jsnode_modules/@tanstack/query-core/build/modern/removable.jsnode_modules/@tanstack/query-core/build/modern/query.jsnode_modules/@tanstack/query-core/build/modern/onlineManager.jsnode_modules/@tanstack/query-core/build/modern/focusManager.jssrc/build-shims.jsnode_modules/@tanstack/react-query/build/modern/useBaseQuery.jsnode_modules/@tanstack/query-core/build/modern/retryer.jspackage.jsonnode_modules/i18next/dist/esm/i18next.jssrc/util/versions.tssrc/components/transactions/TransactionMenu.tsxnode_modules/@use-gesture/core/dist/use-gesture-core.esm.jsnode_modules/@tanstack/query-core/build/modern/queryObserver.jsnode_modules/downshift/dist/downshift.esm.mjssrc/components/autocomplete/CategoryAutocomplete.tsxView detailed bundle breakdown
Added
No assets were added
Removed
No assets were removed
Bigger
Smaller
Unchanged
loot-core
Total
Changeset
node_modules/i18next/dist/esm/i18next.jsView detailed bundle breakdown
Added
Removed
Bigger
No assets were bigger
Smaller
No assets were smaller
Unchanged
No assets were unchanged
api
Total
Changeset
node_modules/i18next/dist/esm/i18next.jsView detailed bundle breakdown
Added
No assets were added
Removed
No assets were removed
Bigger
Smaller
No assets were smaller
Unchanged
cli
Total
View detailed bundle breakdown
Added
No assets were added
Removed
No assets were removed
Bigger
No assets were bigger
Smaller
No assets were smaller
Unchanged