Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded@chhoumann has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 10 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. 📒 Files selected for processing (4)
WalkthroughAdds NFC Unicode normalization to search/indexing and Fuse value extraction; updates suggesters to use normalized comparisons; fixes test singleton reset and indexing awaiting; adds tests verifying decomposed Unicode filename matching. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/gui/suggesters/fileSuggester.ts (1)
275-281: Critical:headingQueryNormalizedis undefined in this scope.The variable
headingQueryNormalizedis defined ingetHeadingSuggestions()(line 129) but used here inrenderSuggestion(). This causes a build failure as confirmed by static analysis.🔎 Proposed fix
case 'heading': { const [fileName, heading] = displayText.split('#'); // Highlight the query in the heading text if possible const headingQuery = this.lastInput.includes('#') ? this.lastInput.split('#')[1] : ''; - if (headingQuery && normalizeForSearch(heading).includes(headingQueryNormalized)) { + if (headingQuery && normalizeForSearch(heading).includes(normalizeForSearch(headingQuery))) { const tempEl = document.createElement('span'); this.renderMatch(tempEl, heading, headingQuery); mainText = tempEl.innerHTML; } else { mainText = heading; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/gui/suggesters/FileIndex.test.ts(3 hunks)src/gui/suggesters/FileIndex.ts(22 hunks)src/gui/suggesters/fileSuggester.ts(5 hunks)src/gui/suggesters/utils.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Source code lives in
src/: core logic underengine/,services/, andutils/; Svelte UI insrc/gui; shared types insrc/types; settings entry insrc/quickAddSettingsTab.ts
Files:
src/gui/suggesters/utils.tssrc/gui/suggesters/FileIndex.tssrc/gui/suggesters/fileSuggester.tssrc/gui/suggesters/FileIndex.test.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Biome enforces tab indentation (width 2), LF endings, and an 80-character line guide; align editor settings
Use camelCase for variables and functions
Prefer type-only imports in TypeScript files
Route logging through theloggerutilities for consistent output
Structure production code so Obsidian dependencies are injected behind interfaces; unit tests target pure logic and swap in adapters ortests/obsidian-stub.ts
Files:
src/gui/suggesters/utils.tssrc/gui/suggesters/FileIndex.tssrc/gui/suggesters/fileSuggester.tssrc/gui/suggesters/FileIndex.test.ts
src/**/*.{ts,tsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for classes and Svelte components
Files:
src/gui/suggesters/utils.tssrc/gui/suggesters/FileIndex.tssrc/gui/suggesters/fileSuggester.tssrc/gui/suggesters/FileIndex.test.ts
🧠 Learnings (1)
📚 Learning: 2025-12-09T21:20:52.425Z
Learnt from: CR
Repo: chhoumann/quickadd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T21:20:52.425Z
Learning: Applies to tests/**/*.{ts,tsx} : Add regression coverage for bug fixes
Applied to files:
src/gui/suggesters/FileIndex.test.ts
🧬 Code graph analysis (2)
src/gui/suggesters/FileIndex.ts (1)
src/gui/suggesters/utils.ts (2)
normalizeForFuse(15-17)normalizeForSearch(11-13)
src/gui/suggesters/fileSuggester.ts (1)
src/gui/suggesters/utils.ts (1)
normalizeForSearch(11-13)
🪛 GitHub Actions: Build With Lint
src/gui/suggesters/FileIndex.ts
[error] 161-161: TS2345: Argument of type '{ threshold: number; keys: { name: string; weight: number; }[]; ignoreLocation: boolean; findAllMatches: boolean; shouldSort: boolean; includeMatches: boolean; getFn: (obj: IndexedFile, path: string | string[]) => unknown; }' is not assignable to parameter of type 'IFuseOptions'.
🪛 GitHub Check: Build With Lint
src/gui/suggesters/fileSuggester.ts
[failure] 275-275:
Cannot find name 'headingQueryNormalized'.
🔇 Additional comments (8)
src/gui/suggesters/utils.ts (1)
11-17: LGTM! Clean normalization utilities.The separation between
normalizeForSearch(NFC + lowercase for direct comparisons) andnormalizeForFuse(NFC only, since Fuse handles case internally) is a good design choice that correctly addresses the NFD/NFC unicode mismatch issue reported in #289.src/gui/suggesters/fileSuggester.ts (2)
129-155: LGTM! Heading filtering correctly normalized.The normalization of
headingQueryand the filtering logic properly handles NFD/NFC unicode differences for heading suggestions.
218-222: LGTM! Attachment filtering correctly normalized.Both the query and the file basename are normalized before comparison, ensuring consistent matching for filenames with diacritics.
src/gui/suggesters/FileIndex.test.ts (2)
8-18: LGTM! Proper singleton reset handling.The updated reset logic correctly accesses
FileIndex.instanceto clear pending timeouts before resetting, preventing test interference.
344-364: Good regression test for the unicode normalization fix.This test directly validates the fix for issue #289 by creating a file with a decomposed (NFD) basename containing an umlaut and verifying that a search with the composed form finds it. Based on learnings, this provides appropriate regression coverage for the bug fix.
src/gui/suggesters/FileIndex.ts (3)
486-487: LGTM! Query normalization properly separates concerns.Using
normalizeForSearchfor direct comparisons andnormalizeForFusefor Fuse.js queries is the correct approach since Fuse handles case sensitivity internally.
500-530: LGTM! Exact match comparisons use consistent normalization.Both basename and alias exact matching correctly apply
normalizeForSearchto both sides of the comparison, ensuring NFD/NFC equivalence.
566-585: Minor: Word boundary detection uses normalized text correctly.The substring matching logic correctly operates on normalized text for the index lookup and character boundary check.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/gui/suggesters/fileSuggester.ts (1)
172-191: Apply normalization to block ID filtering for consistency.Block suggestions use plain
toLowerCase()at line 183, while heading and attachment filtering usenormalizeForSearch(). This inconsistency means block references containing diacritics would still exhibit the Unicode matching issues this PR aims to fix.🔎 Proposed fix to normalize block filtering
private getBlockSuggestions(input: string): SearchResult[] { // Split on the full "#^" sequence to correctly separate file name and block query const [fileName, blockQuery] = input.split('#^'); + const blockQueryNormalized = normalizeForSearch(blockQuery); const fileResults = this.fileIndex.search(fileName, {}, 1); if (fileResults.length === 0) return []; const file = fileResults[0].file; const blockIds = this.fileIndex.getBlockIds(file); return blockIds - .filter(b => blockQuery === '' || b.toLowerCase().includes(blockQuery.toLowerCase())) + .filter(b => blockQuery === '' || normalizeForSearch(b).includes(blockQueryNormalized)) .slice(0, 20) .map(blockId => ({ file, score: 0, matchType: 'block' as const, displayText: `${file.basename}#^${blockId}` })); }
♻️ Duplicate comments (1)
src/gui/suggesters/FileIndex.ts (1)
72-93: Verify that array return type matches Fuse.js expectations.The previous review flagged that
getFuseValuemust returnstring | readonly string[]to match Fuse.js'sgetFnsignature, but the current implementation returnsstring | string[](mutable array). Ensure the return type at lines 72 and 92 includesreadonlyfor array returns to satisfy TypeScript's type checking.Run the following to confirm the types are now compatible:
#!/bin/bash # Check if there are any type errors related to getFuseValue and Fuse configuration # Look for the Fuse.js type definition to understand the expected getFn signature echo "=== Checking Fuse.js type definition ===" rg -A 5 "interface FuseOptionKeyObject" node_modules/fuse.js echo -e "\n=== Checking getFn usage in FileIndex.ts ===" rg -n -A 2 -B 2 "getFn.*getFuseValue" src/gui/suggesters/FileIndex.ts echo -e "\n=== Checking getFuseValue signature ===" ast-grep --pattern 'const getFuseValue = $$$'
🧹 Nitpick comments (1)
src/gui/suggesters/fileSuggester.ts (1)
218-222: LGTM: Attachment filtering correctly normalized.The attachment suggestions properly apply normalization for Unicode-safe matching. The variable name
queryLowerslightly understates the normalization (NFC + lowercase), but the implementation is correct.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/gui/suggesters/FileIndex.ts(22 hunks)src/gui/suggesters/fileSuggester.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Source code lives in
src/: core logic underengine/,services/, andutils/; Svelte UI insrc/gui; shared types insrc/types; settings entry insrc/quickAddSettingsTab.ts
Files:
src/gui/suggesters/fileSuggester.tssrc/gui/suggesters/FileIndex.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Biome enforces tab indentation (width 2), LF endings, and an 80-character line guide; align editor settings
Use camelCase for variables and functions
Prefer type-only imports in TypeScript files
Route logging through theloggerutilities for consistent output
Structure production code so Obsidian dependencies are injected behind interfaces; unit tests target pure logic and swap in adapters ortests/obsidian-stub.ts
Files:
src/gui/suggesters/fileSuggester.tssrc/gui/suggesters/FileIndex.ts
src/**/*.{ts,tsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for classes and Svelte components
Files:
src/gui/suggesters/fileSuggester.tssrc/gui/suggesters/FileIndex.ts
🧬 Code graph analysis (2)
src/gui/suggesters/fileSuggester.ts (1)
src/gui/suggesters/utils.ts (1)
normalizeForSearch(11-13)
src/gui/suggesters/FileIndex.ts (1)
src/gui/suggesters/utils.ts (2)
normalizeForFuse(15-17)normalizeForSearch(11-13)
🔇 Additional comments (6)
src/gui/suggesters/fileSuggester.ts (2)
8-8: LGTM: Clean import of normalization utility.The import is correctly structured and aligns with the normalization strategy throughout this file.
129-129: LGTM: Consistent normalization in heading filtering and rendering.The heading suggestions correctly apply NFC normalization to both the query and heading text for matching, and the rendering path maintains consistency by using the same normalized comparison for highlighting.
Also applies to: 153-155, 275-276
src/gui/suggesters/FileIndex.ts (4)
4-4: LGTM: Normalization utilities imported correctly.The imports include all necessary normalization functions for the Fuse integration and search logic updates.
148-174: LGTM: Fuse configuration properly wired for normalization.The Fuse configuration correctly integrates the custom
getFuseValuefunction to enable NFC normalization during indexing and matching. TheincludeMatchesflag appropriately supports alias detection.
484-655: LGTM: Comprehensive normalization across all search tiers.The search logic correctly applies normalization at every matching tier:
- Exact matches (lines 502, 519)
- Prefix matches (lines 534-536, 551-553)
- Substring matches (lines 570-571)
- Fuzzy matches via Fuse (lines 588, 592)
- Unresolved links (line 630)
The distinction between
normalizeForSearch(NFC + lowercase) for exact matching andnormalizeForFuse(NFC only) for fuzzy matching is appropriate and well-implemented.
657-733: LGTM: Helper methods maintain normalization consistency.The
calculateScore,searchWithHeadings, andsearchFilesmethods all properly apply normalization when computing positions, matching aliases, and filtering headings. The implementation ensures Unicode-safe comparisons throughout the scoring and search pipeline.Also applies to: 735-919
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/gui/suggesters/FileIndex.ts (1)
72-96: Type incompatibility issue from previous review is now fixed.The helper functions now correctly return
string | ReadonlyArray<string>, which matches Fuse.jsgetFnexpectations. The type guard in line 76 properly narrows the array elements to strings.
🧹 Nitpick comments (1)
src/gui/suggesters/FileIndex.ts (1)
688-688: Consider passing pre-normalized query to avoid redundant normalization.The query is already normalized in the caller (
searchmethod at line 489). Re-normalizing here is redundant. For cleaner code, consider passingqueryLoweras an additional parameter tocalculateScore.🔎 Suggested signature change
-private calculateScore(file: IndexedFile, query: string, context: SearchContext, baseScore: number, matchType?: string): number { +private calculateScore(file: IndexedFile, queryLower: string, context: SearchContext, baseScore: number, matchType?: string): number { let score = baseScore; // ... - const queryLower = normalizeForSearch(query);This would require updating all call sites to pass the pre-normalized query.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/gui/suggesters/FileIndex.ts(22 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Source code lives in
src/: core logic underengine/,services/, andutils/; Svelte UI insrc/gui; shared types insrc/types; settings entry insrc/quickAddSettingsTab.ts
Files:
src/gui/suggesters/FileIndex.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,tsx}: Biome enforces tab indentation (width 2), LF endings, and an 80-character line guide; align editor settings
Use camelCase for variables and functions
Prefer type-only imports in TypeScript files
Route logging through theloggerutilities for consistent output
Structure production code so Obsidian dependencies are injected behind interfaces; unit tests target pure logic and swap in adapters ortests/obsidian-stub.ts
Files:
src/gui/suggesters/FileIndex.ts
src/**/*.{ts,tsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for classes and Svelte components
Files:
src/gui/suggesters/FileIndex.ts
🧬 Code graph analysis (1)
src/gui/suggesters/FileIndex.ts (1)
src/gui/suggesters/utils.ts (2)
normalizeForFuse(15-17)normalizeForSearch(11-13)
🔇 Additional comments (3)
src/gui/suggesters/FileIndex.ts (3)
4-4: LGTM!Import correctly brings in the normalization utilities needed to address the Unicode (NFC vs NFD) matching issue.
160-161: LGTM!The Fuse configuration correctly integrates
getFnfor normalized value extraction during indexing/searching, andincludeMatchesenables proper alias hit detection used downstream at line 610.
489-490: LGTM!The search entry point correctly normalizes the query once for both comparison (
queryLower) and Fuse search (fuseQuery), ensuring consistent Unicode normalization (NFC) throughout the search pipeline.
# [2.10.0](2.9.4...2.10.0) (2026-01-28) ### Bug Fixes * add insert-after blank-line mode ([#1056](#1056)) ([231e908](231e908)) * backfill file opening defaults for legacy choices ([bfffd46](bfffd46)) * default update announcements to major ([#1042](#1042)) ([4e5659b](4e5659b)) * handle template overwrite with case-mismatched paths ([#1049](#1049)) ([0140556](0140556)) * harden suggester display items ([5460657](5460657)) * harden suggester filtering on Android ([4560f22](4560f22)), closes [#1078](#1078) * normalize leading slashes in capture/template paths ([#1050](#1050)) ([1c7def1](1c7def1)) * normalize unicode in file suggestions ([#1046](#1046)) ([10c0402](10c0402)) * pin obsidian types to 1.11.4 for SecretStorage API ([ddbf6f6](ddbf6f6)) * prefill macro rename input ([#1043](#1043)) ([06c4a25](06c4a25)) * preserve blank lines for insert after ([#1054](#1054)) ([818c036](818c036)) * preserve variables for VALUE templating ([36d43ba](36d43ba)) * reduce bundle size below sync limit ([1e1a632](1e1a632)) * restore compatibility with Templater 2.18.0 ([716f2d9](716f2d9)), closes [#1085](#1085) [#1086](#1086) ### Features * add back class to choice suggester ([#1047](#1047)) ([4c8cfe9](4c8cfe9)) * add capture selection-as-value controls ([#1055](#1055)) ([250768a](250768a)) * add inline insert-after capture mode ([b2e1ef5](b2e1ef5)) * add macro selection helper ([786b53c](786b53c)), closes [#483](#483) * add native date picker prompt ([2811c5a](2811c5a)) * add per-token multiline VALUE inputs ([98fa7db](98fa7db)), closes [#339](#339) * add versioned documentation with Docusaurus ([03c2d3e](03c2d3e)) * adopt obsidian 1.11 settings APIs ([#1041](#1041)) ([15c4b34](15c4b34)) * default multi placeholder to name ([fcd058f](fcd058f)) * enhance template folder chooser ([f1e2a9f](f1e2a9f)), closes [#1011](#1011) [#1012](#1012) * improve prompt labeling for VALUE/MACRO and multi choices ([78fd184](78fd184)) * persist input prompt drafts on cancel/escape ([#1044](#1044)) ([62a67f4](62a67f4)) * store AI provider keys in SecretStorage ([4559013](4559013)) * support short-form date aliases ([e04a3f6](e04a3f6))
|
🎉 This PR is included in version 2.10.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
1 participant
Summary
Fixes #289.
Testing
Summary by CodeRabbit
Bug Fixes
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.