Fix pinyin match for Chinese restart command#4544
Conversation
There was a problem hiding this comment.
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.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
📝 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)
✨ 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.
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
ℹ️ 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
|
Nice workaround. Our 3rd party pinyin nuget is not good while dealing with polyphonic. |
Jack251970
left a comment
Jack251970
left a comment
There was a problem hiding this comment.
LGTM! Could you please add more pairs in PolyphonicPhraseOverrides? Currently, we just have one pair in it.
Here is some tools for your reference: https://chatgpt.com/share/6a428475-1ba4-83ea-b48c-ef9ab02c3859.
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 now delegates toPinyinAlphabet(Settings)with a null check.PolyphonicPhraseOverridesfor “重启” → ["Chong","Qi"];ApplyPolyphonicPhraseOverrideswith bounds checks and multi-occurrence handling; NUnit tests for translation and fuzzy match of “chongqi”.Release Note
Typing “chongqi” now correctly finds the Chinese “重启” (restart) command.
Written for commit 172059e. Summary will update on new commits.