-
-
Notifications
You must be signed in to change notification settings - Fork 617
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes pinyin matching for the built-in Chinese restart command phrase 重启 by introducing a small phrase-level override so that queries like chongqi correctly match 重启 (and 重启 Flow Launcher) while preserving existing behavior for unrelated polyphonic words like 重庆.
Changes:
- Added a polyphonic phrase override map in
PinyinAlphabetand applied it during pinyin cache construction. - Refactored
PinyinAlphabetto supportSettingsinjection (improves testability). - Added unit tests covering the
重启override and fuzzy matching forchongqi.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| Flow.Launcher.Infrastructure/PinyinAlphabet.cs | Adds a targeted override for 重启 pinyin output and enables Settings injection for deterministic behavior in tests. |
| Flow.Launcher.Test/PinyinAlphabetTest.cs | Adds unit tests validating translation and fuzzy-match behavior for 重启/chongqi, plus a regression check for 重庆. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 2 files
|
No actionable comments were generated in the recent review. 🎉 i️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesPolyphonic Phrase Pinyin Override
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Flow.Launcher.Infrastructure/PinyinAlphabet.cs (1)
185-188: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winDefensively validate override span writes.
The override loop assumes phrase length and pinyin token count always align. A future bad entry can throw
IndexOutOfRangeExceptionat Line 187. Add a guard before writing the span to keep translation resilient.Suggested hardening
while (index >= 0) { + if (pinyin.Length != phrase.Length || index + pinyin.Length > resultList.Length) + { + index = content.IndexOf(phrase, index + phrase.Length, StringComparison.Ordinal); + continue; + } + for (var i = 0; i < pinyin.Length; i++) { resultList[index + i] = pinyin[i]; }🤖 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 `@Flow.Launcher.Infrastructure/PinyinAlphabet.cs` around lines 185 - 188, The loop at line 185-188 that writes pinyin characters to resultList lacks bounds validation and assumes the phrase length always matches the pinyin token count. Add a guard condition before the for loop that iterates over pinyin to validate that index plus pinyin.Length does not exceed resultList.Length, preventing potential IndexOutOfRangeException when future bad entries are encountered. If the bounds check fails, handle it gracefully by skipping the write or logging a warning to keep the translation resilient.
🤖 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 `@Flow.Launcher.Infrastructure/PinyinAlphabet.cs`:
- Around line 31-33: The PinyinAlphabet constructor does not validate that the
injected settings parameter is null before assigning it to the _settings field.
Add a null guard at the beginning of the PinyinAlphabet constructor that throws
an ArgumentNullException if the settings parameter is null, ensuring the error
fails fast instead of allowing a NullReferenceException to occur later when
_settings is dereferenced.
---
Nitpick comments:
In `@Flow.Launcher.Infrastructure/PinyinAlphabet.cs`:
- Around line 185-188: The loop at line 185-188 that writes pinyin characters to
resultList lacks bounds validation and assumes the phrase length always matches
the pinyin token count. Add a guard condition before the for loop that iterates
over pinyin to validate that index plus pinyin.Length does not exceed
resultList.Length, preventing potential IndexOutOfRangeException when future bad
entries are encountered. If the bounds check fails, handle it gracefully by
skipping the write or logging a warning to keep the translation resilient.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
i️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 19d08c6a-e4e2-492c-adb9-1621554a762a
📒 Files selected for processing (2)
Flow.Launcher.Infrastructure/PinyinAlphabet.csFlow.Launcher.Test/PinyinAlphabetTest.cs
VictoriousRaptor
commented
Jun 25, 2026
Nice workaround. Our 3rd party pinyin nuget is not good while dealing with polyphonic.
Uh oh!
There was an error while loading. Please reload this page.
Summary
重启重启and重启 Flow Launcherto match queries likechongqiScope
This does not try to solve every Chinese polyphonic character case. It fixes the common built-in command phrase from #3955 while keeping the behavior for
重庆asChong Qing.Tests
dotnet test Flow.Launcher.Test\Flow.Launcher.Test.csproj --filter "FullyQualifiedName~PinyinAlphabetTest"dotnet test Flow.Launcher.Test\Flow.Launcher.Test.csproj --no-restore --filter "FullyQualifiedName~PinyinAlphabetTest|FullyQualifiedName~FuzzyMatcherTest"Fixes #3955
Summary by cubic
Fixes pinyin matching for the Chinese restart command by overriding the polyphonic phrase "重启" to map to "Chong Qi". Queries like "chongqi" now match "重启" and "重启 Flow Launcher" without affecting words like "重庆".
Summary of changes
PinyinAlphabet.BuildCacheFromContentnow applies polyphonic overrides and pre-allocates the translation builder; default constructor delegates toPinyinAlphabet(Settings)with a null check.PolyphonicPhraseOverridesfor "重启" → ["Chong","Qi"];ApplyPolyphonicPhraseOverrideswith bounds checks; NUnit tests for translation and fuzzy match of "chongqi".Release Note
Typing "chongqi" now correctly finds the Chinese "重启" (restart) command.
Written for commit f9ac9b7. Summary will update on new commits.