[CmdPal] Phase 7c: early-frame relevance (withhold weak fuzzy tail on ultra-short queries) - #49249
Conversation
ab0fa5d to
ff39910
Compare
ff39910 to
b575b5a
Compare
|
/azp run |
|
Azure Pipelines: 1 pipeline(s) were filtered out due to trigger conditions. |
b575b5a to
20935d9
Compare
dcd842c to
6a98532
Compare
6a98532 to
0d5413b
Compare
0d5413b to
735d1c1
Compare
…red ranker (#49189) >[!WARNING] > This PR is one in a series of PRs focused on rearchitecting the search/scoring logic of the `MainListPage`. An explanation of the entire search/scoring logic can be found below. > > **This PR should not be merged until PR #49190 is merged into it.** >[!NOTE] > To test the final result, run the branch associated with PR #49249. This stack rebuilds how Command Palette ranks and displays results on its main page. Strong text matches now consistently appear above weaker ones. Usage history and provider preferences can improve ordering between similarly relevant results, but they cannot push a poor match above an obvious one. The stack also makes search feel faster. Results appear without waiting for slower providers, app scoring runs more efficiently, and weak matches are hidden while the user has typed only one or two characters. Automated tests protect the new behavior, while privacy conscious telemetry measures performance and relevance without recording searches. ## Pull requests 1. [#49189](#49189) introduces the new ranking foundation. Results are grouped by match strength, ensuring exact names, prefixes, and acronyms rank above loose fuzzy matches. 2. [#49190](#49190) improves how Command Palette learns from command usage. Recent and frequently used commands receive a sensible boost, and that history now persists across restarts. 3. [#49191](#49191) lets users give each provider a Lower, Normal, or Higher search preference. This preference helps resolve close matches without overriding result relevance. 4. [#49194](#49194) makes the first set of results appear sooner. Commands and apps are shown immediately, while slower fallback results are added when they become available. 5. [#49195](#49195) adds a comprehensive relevance test suite. It verifies that common searches return the expected results and protects ranking quality from future regressions. 6. [#49197](#49197) adds privacy conscious search telemetry. It measures result counts, response time, and which result position was selected without recording search text, result names, paths, or other user content. 7. [#49246](#49246) adds a performance measurement suite. It identifies where search time is spent and provides a reliable way to evaluate performance improvements. 8. [#49247](#49247) delivers the main performance improvement. App results are scored in parallel and expensive work no longer blocks rendering, while the final result order remains unchanged. 9. [#49249](#49249) prevents misleading results from flashing when a search begins. For one or two character searches, weak fuzzy app matches remain hidden until the query is specific enough to produce useful results. > [!WARNING] > These PRs should be merged in LIFO order starting with #49249 with this PR being the last. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
735d1c1 to
c3b6234
Compare
For 1-2 char queries a huge fraction of the app catalog fuzzy-matches into the single Fuzzy tier, where within-tier order is decided almost entirely by frecency, so the most-frecent app can float to rank 1 on a weak mid-word subsequence match - the "wrong when I start typing" flash. Gate the render path only: while the query is ultra-short, show apps at or above the word-boundary tier and withhold the low-confidence fuzzy tail until the query can discriminate. This slices the already-scored, already-sorted app array and never re-scores or reorders, so the settled order of a 3+ char query is byte-identical and the Phase 7b equivalence guarantee is untouched. Applying the same gate to a late async app batch also stabilizes short-query fold-in. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…metry count Fix #2: publish the app query length atomically with _filteredApps under lock(commands) and gate the early-frame render path on that published field instead of the live SearchText. SearchText is advanced on the UI thread ahead of publish, so on a 2-to-3 char transition a throttled refresh could render the stale 2-char app array while the live length read 3, no-op the gate, and briefly expose the fuzzy tail 7c withholds. _filteredAppsQueryLength is reset in ClearResults alongside _filteredApps. Fix #3: count the gated visible apps for the settled-search telemetry when the published length is in the ultra-short gate window, so NoResults / result count reflect what is actually shown rather than the ungated app array. Added GatedVisibleAppCount, kept the 7b post-lock telemetry structure. Tests: added coverage that the gate decides on the supplied published length (not live text) and that the telemetry count equals the gated visible count. EarlyFrameRelevanceTests and 7b ScoringParallelEquivalenceTests both stay green. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The off-lock scoring phase pins every input into immutable locals so a concurrent keystroke cannot mix two generations into one pass. PrecomputeQuery still read the live SearchText field, which a newer keystroke may already have advanced, scoring the pass against a different query than it was launched for. Precompute from the snapshotted newSearch parameter instead. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5459b847-afeb-4163-a803-977759dd92df
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 92905c83-2de6-449c-b4a1-a08003fe2576
c3b6234 to
e1d4522
Compare
Mike Griese (zadjii-msft)
left a comment
There was a problem hiding this comment.
yea code's fine. Just nitpicking over phrasing again. Still feels like the agents are saying more that the right number of words
|
|
||
| // The array is sorted descending by packed score and the tier lives in that score's high bits, | ||
| // so everything at or above a tier forms a contiguous prefix that one forward scan can find. | ||
| internal static int HighConfidenceAppPrefixLength(IReadOnlyList<RoScored<IListItem>> scored, RankTier minTier) |
There was a problem hiding this comment.
<nit>
HighConfidenceAppPrefixLength makes me think that this is string handling. Maybe from the word Prefix. Maybe GetHighConfidenceAppsCount or something like that?
| // Keeps only the letter-relevant apps while the query is short, and returns a longer query's | ||
| // results untouched. It slices a contiguous prefix of the already-scored, already-sorted array, | ||
| // so it can never re-score or reorder anything. | ||
| internal static IList<RoScored<IListItem>>? ApplyShortQueryAppGate( |
There was a problem hiding this comment.
FilterAppsForShortQueries might be better? The word gate really feels like a lock or synchronization primitive, not a filter.
| // Hold back the weak fuzzy tail while the query is still short, so a frecency-floated | ||
| // match can't surface at the top mid-typing. |
There was a problem hiding this comment.
| // Hold back the weak fuzzy tail while the query is still short, so a frecency-floated | |
| // match can't surface at the top mid-typing. | |
| // Remove fuzzy-only app matches for short queries. This prevents | |
| // frecency-boosted (but otherwise weakly matching) apps from appearing | |
| // at the top while typing |
"hold back" seemed weird? I'm not sure mine is better
|
|
||
| private int AppResultLimit => AllAppsCommandProvider.TopLevelResultLimit; | ||
|
|
||
| // ===== Early-frame relevance ===== |
There was a problem hiding this comment.
| // ===== Early-frame relevance ===== |
I don't think that adds any information? "Early-frame" doesn't feel like a real term we've well defined.
| private int AppResultLimit => AllAppsCommandProvider.TopLevelResultLimit; | ||
|
|
||
| // ===== Early-frame relevance ===== | ||
| // Longest query we still hold back the weak fuzzy app tail for, since one letter matches most |
There was a problem hiding this comment.
<nit> something about the phrase "fuzzy tail" just doesn't immediately click for me.
Can we rephrase this comment to something like
// Longest query to filter fuzzy app matches on. This prevents weak (fuzzy) app matches on short queries.| { | ||
| public TestContext TestContext { get; set; } = null!; | ||
|
|
||
| private sealed partial class CatalogItem : IListItem, IPrecomputedListItem |

Note
Last in a series rearchitecting the
MainListPagesearch and ranking logic. The design detail lives in the initial PR #49189.What's going on
Type one or two characters into Command Palette and you'll watch it show you obviously wrong stuff for a beat before the right results land. Everybody's seen it. This is the fix.
Here's the why. A single character query fuzzy matches a huge slice of the app catalog, and all of it lands in one
Fuzzytier. On a one character match the lexical quality is basically zero, soFrecencyScale 1beatsLexicalScale 10times almost nothing. Frecency ends up deciding the order inside that giant tier, and your most used app floats to the top on a weak mid word match. There's a new test that reproduces exactly that: for a query that only matches mid word, every result lands in the Fuzzy tier, and seeding frecency on one app floats it to rank one.Global fallbacks aren't the culprit, which surprised me. They always classify at
FallbackFloorand can never leapfrog commands or apps. The rest of the reshuffle is a late async app batch folding in and floating a weak fuzzy app, same root cause.The plan
In
GetSearchViewItems, when the query is one or two characters, hold back the low confidence app tail and show only apps at tierAcronymWordBoundaryor better. That's word boundary, prefix, exact, and alias. Commands and fallbacks render exactly as they did.The scored arrays are already sorted descending and the tier sits in the packed score's high bits, so the kept set is a contiguous prefix. That makes this one forward scan and an
ArraySegmentview handed to the result factory. Nothing gets re scored, nothing gets reordered, and no ranker constant, tier, or frecency value moves.The same gate runs on a late async app batch, which is what stabilizes the short query fold in.
At three characters or more the gate is a no op that hands back the same array instance, so the settled order is byte for byte what 7b produced.
EarlyFrameRelevanceTestscovers the frecency floats a weak match evidence, the gate withholding the fuzzy tail, the gate keeping confident tiers, three plus character pass through against the same reference, the length boundary, the null and empty and zero length no ops, and the prefix length helper. The 7b equivalence tests stay green and unchanged.Notes for reviewers
There's a more direct fix I didn't ship. Damping frecency or re gating tiers for short queries inside the ranker would attack the root cause instead of hiding it at render time. It would also change the settled order and break the 7b equivalence test, so it isn't something I want to sneak in under a perf PR. Worth a real conversation on its own.
This is a render path gate, and I want to be honest that it's a gate. The ranker still thinks your most frecent app is the best answer for
c. We just stop drawing that opinion until the query says something.