[review-mirror] microsoft/PowerToys#49194#29
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>
… 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
- 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
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>
…into dev/mjolley/dev-mjolley-fast-first-paint
There was a problem hiding this comment.
Pull request overview
This PR updates CmdPal’s main-page ranking and rendering pipeline to improve “fast first paint” behavior and make ordering more deterministic and configurable. It introduces a tier-based rank packing model (so strong textual matches can’t be leapfrogged), replaces the old recent-command “bucket” heuristic with time-decayed frecency (persisted across restarts), adds a per-provider “search weight” nudge exposed in Settings, and re-scores slow global fallbacks on the render path so late-arriving dynamic titles fold in safely.
Changes:
- Implement tiered ranking via packed scores (
RankTier+ clamped within-tier score), including alias and fallback-floor behavior, plus a small per-provider within-tier bonus. - Replace recent-command weighting with persisted, time-decayed frecency (recency half-life + log-frequency), including a capped history store and cached lookup index.
- Add unit tests covering fast-first-paint invariants, provider weighting, frecency decay/frequency, history cap/serialization, and alias-substring-only regression; wire a “Search weight” ComboBox into the extension settings UI.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/RecentCommandsTests.cs | Updates/adds tests for new frecency model, history cap/index invalidation, migration behavior, serialization round-trip, and tier invariants. |
| src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/ProviderWeightingTests.cs | New tests validating provider weight bonus behavior, tier-boundary invariants, and provider settings JSON compatibility. |
| src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/FastFirstPaintTests.cs | New guardrail tests validating deterministic first paint and safe late fallback fold-in via render-path re-scoring. |
| src/modules/cmdpal/Microsoft.CmdPal.UI/Strings/en-us/Resources.resw | Adds localized strings and automation name for the new “Search weight” settings card and options. |
| src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/ExtensionPage.xaml | Adds “Search weight” ComboBox bound to ProviderSettingsViewModel.SearchWeightIndex. |
| src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/RecentCommandsManager.cs | Implements persisted frecency model (time decay + log frequency), increases history cap, and adds cached index invalidation. |
| src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ProviderSettingsViewModel.cs | Adds SearchWeightIndex (ComboBox-friendly) to persist provider weight setting. |
| src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ProviderSettings.cs | Introduces ProviderSearchWeight enum and persists ProviderSettings.SearchWeight with Normal default. |
| src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/HistoryItem.cs | Adds LastUsed timestamp to support real time-decay frecency. |
| src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Commands/MainListRanker.cs | New tier-based ranker: tier classification, within-tier scoring, packed score encoding, acronym/word-boundary helper, provider bonus. |
| src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Commands/MainListPage.cs | Switches scoring to tiered packed scores, adds provider weight lookup, and defers global fallback scoring to render path via snapshot + re-score helper. |
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@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/29227197333/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.
| /// item across a tier boundary. This is what keeps ordering predictable ("an exact match | ||
| /// always beats a fuzzy one"). | ||
| /// </summary> | ||
| public enum RankTier |
Review sandbox mirror of microsoft#49194 (branch
dev/mjolley/dev-mjolley-fast-first-paint). Opened by PR-autopilot for Copilot review + local build validation. Not for merge.