[review-mirror] microsoft/PowerToys#49195#28
Open
yeelam-gordon wants to merge 23 commits into
Open
Conversation
Rework the main/root page relevance scoring into a principled, testable multi-signal ranker. Ordering is now decided by a hard tier ladder (alias-exact > exact-title > prefix > acronym/word-boundary > fuzzy > fallback floor); frecency and other within-tier signals only reorder items that already share a tier, so an obvious match can no longer be buried under junk. - Add MainListRanker: RankTier ladder, tier classifier, normalized within-tier score, and a packed sortable int so the existing score-descending sort is unchanged. - Add IRankSignal / IRerankStage seam interfaces so a future on-device semantic or small-LLM reranker can slot in later. No model ships today. - Replace the hand-tuned constants in ScoreTopLevelItem with tier classification plus the within-tier score. Within-tier math mirrors the old balance so shared-tier ordering is preserved; only cross-tier behavior changes. - Update ranking tests to assert the new tier invariant: heavy usage reorders within a tier but never crosses a tier boundary. Host-side only; the extension SDK toolkit and extension-owned pages are untouched. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Replace the list-position frecency heuristic in RecentCommandsManager with a real time-decayed signal: exponential recency decay (3-day half-life) combined with log2(uses) for frequency, clamped to the previous ~0..70 range so the ranker's within-tier math is unchanged and the tier still dominates ordering. - HistoryItem gains a persisted LastUsed timestamp. - Store cap grows from 50 to 500 entries. - Legacy persisted items (no LastUsed) are treated as used one day ago so day-one ordering degrades to Uses-ordering instead of going all-equal. - Add internal WithHistoryItem/GetCommandHistoryWeight overloads that take an explicit 'now' so tests can inject strictly-increasing timestamps. - Mark the internal History property [JsonInclude] so history (and LastUsed) actually round-trips through the source-generated serializer; old empty state still deserializes cleanly. - Rewrite the position-bucket tests as explicit time-decay tests; keep the tier-invariant tests unchanged in spirit. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Add a per-provider SearchWeight (Lower/Normal/Higher, default Normal) that nudges main-page search results within their relevance tier only. The nudge is a small additive within-tier bonus (+/- 5 points, half a point of lexical quality) so it breaks near-ties but can never move an item across a tier boundary. Applies to installed apps too via the well-known AllApps provider. - ProviderSettings: new ProviderSearchWeight enum + SearchWeight property; legacy/missing JSON deserializes to Normal. - MainListRanker: ProviderWeightBonus constant + ProviderBonus() mapping. - MainListPage.ScoreTopLevelItem: resolves each item's provider weight and feeds it into the within-tier score. - ProviderSettingsViewModel + ExtensionPage.xaml: a Lower/Normal/Higher ComboBox per provider. - Tests: within-tier reorder, never crosses a tier boundary, Normal is a no-op, applies to app items, serialization round-trip + legacy default. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…allbacks Defer scoring of global/special fallbacks from keystroke time to the render path (GetSearchViewItems) so slow, asynchronously-resolved fallback titles fold into the already-rendered list with fresh scores from the same ranker, instead of a score frozen against a stale title. Deterministic in-proc results (top-level commands + apps) are unchanged and never wait on the async fallback work, so first paint stays fast. Fallbacks remain pinned at the FallbackFloor tier, so the deferred re-score only reorders them among themselves and can never leapfrog deterministic command/app matches. A superseding keystroke atomically replaces the snapshot (sources + query) and the existing per-keystroke cancellation token drops stale async fallback callbacks. Extract ScoreDeferredFallbacks as an internal static helper and add guardrail unit tests covering deterministic-only first paint, late fallback fold-in with a fresh score, no leapfrog, and supersession by snapshot replacement. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Add RelevanceHarnessTests: a golden-set MSTest harness that drives the real MainListPage.ScoreTopLevelItem scoring path over a representative fixture of apps and top-level commands, sorts exactly as the product does (positive scores, descending), and asserts query -> expected ordering. Covers the tier ladder end to end (exact > prefix > acronym/word-boundary > fuzzy), frecency reordering within a tier only, per-provider Higher nudge breaking near-ties without crossing tiers, and realistic complaint cases (typing c, code, set surfaces the right thing at rank 1). Also add focused per-tier MainListRanker unit tests: ClassifyTier boundaries (including alias-exact and fallback-floor), Pack monotonicity (a higher tier always outranks a lower one regardless of within-tier score), within-tier clamping and ordering, TierOf round-trip, and ProviderBonus sign. Test-only change; no product behavior modified. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
… ranker A top-level item whose alias merely starts with the query, but whose title, subtitle, and extension do not match, was silently filtered out. ClassifyTier did not receive the alias-substring signal, so it fell through to RankTier.None, Pack returned 0, and FilterListWithScores dropped the item. This defeats the purpose of an alias that is intentionally unrelated to the title (e.g. alias "term" on "Windows PowerShell", query "ter"). Pass isAliasSubstringMatch into ClassifyTier and floor such items to at least the Fuzzy tier. It is only a floor: a stronger title relationship (exact/prefix/word-boundary) still wins, and the existing within-tier AliasSubstringBonus continues to nudge the item up among its peers. Adds regression tests covering the pure-alias case, the floor-only behavior, and the no-match case. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…, cleanup Follow-up to review feedback on the tiered ranker: - Drop the speculative IRankSignal / IRerankStage seam interfaces. They had no implementation or consumer anywhere in the code, so they were dead public API. They can be reintroduced by the change that actually needs them. - Make MainListRanker internal. It is only used by MainListPage and the (InternalsVisibleTo) unit tests, so it does not need to be public. RankTier stays public because it is carried across the assembly boundary by a telemetry message consumed in the host UI project. - Use OrdinalIgnoreCase instead of CurrentCultureIgnoreCase in ClassifyTier and MatchesWordBoundaryOrAcronym. This makes tier classification locale-independent (e.g. the Turkish dotted/dotless-I no longer changes prefix/acronym results) and faster on the per-item, per-keystroke path. - Collapse the redundant no-match gate in ScoreTopLevelItem. ClassifyTier already returns None for every non-match, so the separate pre-check is unnecessary. - Fix a stale test comment in ValidateUsageReordersWithinTier. Behavior for ASCII titles is unchanged; all CmdPal unit tests stay green. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…ul' into dev/mjolley/frecency-redesign
…r-weighting Resolve conflicts in MainListRanker.cs (using block): keep the base's internal class visibility, OrdinalIgnoreCase, 6-param ClassifyTier, and interface removals, alongside the provider-weighting ProviderWeightBonus/ProviderBonus additions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…into dev/mjolley/dev-mjolley-fast-first-paint
…st-paint' into dev/mjolley/relevance-harness
…g arg Phase 4 (merged down) added an isAliasSubstringMatch parameter to MainListRanker.ClassifyTier, inserted before matchedLexically. Update the 8 direct ClassifyTier calls in the per-tier tests to pass isAliasSubstringMatch: false, preserving their original intent, and add a focused test that an alias-substring match floors an otherwise-unmatched item to RankTier.Fuzzy. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
- Index history by commandId so GetCommandHistoryWeight is O(1) instead of an O(N) linear scan. ScoreTopLevelItem calls it for every candidate on every keystroke, and the store now holds up to 500 entries, so the previous scan cost scaled with the larger cap. The cached lookup is rebuilt lazily and invalidated whenever History is (re)assigned, and is excluded from JSON. - Reword the LegacyBackdate documentation to match reality: because earlier builds never actually persisted history, the backdate is defensive handling for any timestamp-less item rather than a common upgrade path. - Add a test that locks in index invalidation across WithHistoryItem copies. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…nto dev/mjolley/provider-weighting
…into dev/mjolley/dev-mjolley-fast-first-paint
…st-paint' into dev/mjolley/relevance-harness
Remove the redundant using directive added while resolving the phase-2 merge conflict (CS0105 under TreatWarningsAsErrors). Also document the tier-floor asymmetry of the Lower provider nudge on ProviderBonus. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
- ClearResults now also resets _globalFallbackQuery so the snapshot pair (_globalFallbackSources + _globalFallbackQuery) is cleared symmetrically. Behavior is unchanged since ScoreDeferredFallbacks short-circuits on a null source list; this just avoids a confusing leftover value. - Rename the superseding-query test to ReScoreUsesLatestSnapshot_StaleStrongMatchNotApplied and document that it exercises the render-path re-score at the helper level, while the true atomic field swap is guaranteed by the lock in UpdateSearchTextCore / GetItems. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…st-paint' into dev/mjolley/relevance-harness
…into dev/mjolley/dev-mjolley-fast-first-paint
…st-paint' into dev/mjolley/relevance-harness
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR introduces a more deterministic “tiered” relevance model for CmdPal’s main/root search ranking, adds per-provider search weighting in Settings, and updates recent-command frecency to use real time-decay (with serialization support) plus a suite of unit tests that lock in the new ranking/fallback invariants.
Changes:
- Implement tier-based packing ranker (exact/prefix/word-boundary/fuzzy/fallback) and integrate it into main-page scoring (including alias floor + provider bonus).
- Replace bucketed recent-history weighting with time-decayed frecency (LastUsed + log(Uses)), persist it via source-generated JSON, and add targeted regression tests.
- Defer scoring of global fallbacks to render-time (“fast first paint”) so late-resolving fallback titles fold in safely without leapfrogging deterministic results; add tests.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/RelevanceHarnessTests.cs | Adds a golden-set relevance harness + tier/packing invariants for the new ranker behavior. |
| src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/RecentCommandsTests.cs | Updates/expands tests for time-decayed frecency, history cap, migration, serialization, and tier-boundary behavior. |
| src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/ProviderWeightingTests.cs | Adds unit tests for provider search-weight nudging and serialization behavior. |
| src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/FastFirstPaintTests.cs | Adds tests ensuring first paint is deterministic and fallbacks fold in later at the floor tier. |
| src/modules/cmdpal/Microsoft.CmdPal.UI/Strings/en-us/Resources.resw | Adds localized strings for the new “Search weight” provider setting (incl. automation name). |
| src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/ExtensionPage.xaml | Adds a provider “Search weight” ComboBox wired to the provider settings view model. |
| src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/RecentCommandsManager.cs | Implements time-decayed frecency, cached lookup index, serialization of history, and larger history cap. |
| src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ProviderSettingsViewModel.cs | Exposes SearchWeightIndex for UI binding and persists to provider settings. |
| src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ProviderSettings.cs | Introduces ProviderSearchWeight and adds SearchWeight to ProviderSettings. |
| src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/HistoryItem.cs | Adds LastUsed timestamp for persisted time-decay frecency. |
| src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Commands/MainListRanker.cs | Adds tier ladder + packed score math + word-boundary/acronym detection utilities. |
| src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Commands/MainListPage.cs | Integrates ranker into scoring, adds provider-weight resolution, and defers global-fallback scoring to render-time. |
This comment has been minimized.
This comment has been minimized.
Resolve Copilot clock-read feedback and replace spelling-prone relevance jargon in comments/tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Keep each CmdPal search scoring pass on one recent-command snapshot alongside the shared evaluation timestamp. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| /// item across a tier boundary. This is what keeps ordering predictable ("an exact match | ||
| /// always beats a fuzzy one"). | ||
| /// </summary> | ||
| public enum RankTier |
Comment on lines
+80
to
+84
| if (_index is null) | ||
| { | ||
| // Ordinal to match the string '==' comparison the previous linear scan used. | ||
| var map = new Dictionary<string, HistoryItem>(StringComparer.Ordinal); | ||
| foreach (var item in History) |
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, 👼 SARIF report, or 📝 job summary for details.Unrecognized words (85)These words are not needed and should be removedATRIOX Autorun Dedup Gotchas intput MTND NONELEVATED NOTXORPEN nullability pfo ssf TILLSON Unsubscribes Uptool VISEGRADRELAY WKSGTo accept these unrecognized words as correct and remove the previously acknowledged and now absent words, you could run the following commands... in a clone of the git@github.com:yeelam-gordon/PowerToys.git repository curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/cfb6f7e75bbfc89c71eaa30366d0c166f1bd9c8c/apply.pl' |
perl - 'https://github.com/yeelam-gordon/PowerToys/actions/runs/29227469396/attempts/1' &&
git commit -m 'Update check-spelling metadata'OR To have the bot accept them for you, comment in the PR quoting the following line: Forbidden patterns 🙅 (7)In order to address this, you could change the content to not match the forbidden patterns (comments before forbidden patterns may help explain why they're forbidden), add patterns for acceptable instances, or adjust the forbidden patterns themselves. These forbidden patterns matched content: Articles generally shouldn't be used without a noun and a verb
Should be
|
| ❌ Errors and Notices | Count |
|---|---|
| ℹ️ candidate-pattern | 3 |
| ❌ check-file-path | 4 |
| ❌ forbidden-pattern | 19 |
See ❌ Event descriptions for more information.
If the flagged items are 🤯 false positives
If items relate to a ...
-
binary file (or some other file you wouldn't want to check at all).
Please add a file path to the
excludes.txtfile matching the containing file.File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.
^refers to the file's path from the root of the repository, so^README\.md$would exclude README.md (on whichever branch you're using). -
well-formed pattern.
If you can write a pattern that would match it,
try adding it to thepatterns.txtfile.Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.
Note that patterns can't match multiline strings.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Review sandbox mirror of microsoft#49195 (branch
dev/mjolley/relevance-harness). Opened by PR-autopilot for Copilot review + local build validation. Not for merge.