[review-mirror] microsoft/PowerToys#49195

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Commands/MainListPage.cs

@@ -45,7 +45,6 @@ public sealed partial class MainListPage : DynamicListPage,
     private readonly AliasManager _aliasManager;
     private readonly ISettingsService _settingsService;
     private readonly IAppStateService _appStateService;
-    private readonly ScoringFunction<IListItem> _scoringFunction;
     private readonly ScoringFunction<IListItem> _fallbackScoringFunction;
     private readonly IFuzzyMatcherProvider _fuzzyMatcherProvider;
 
@@ -63,9 +62,19 @@ public sealed partial class MainListPage : DynamicListPage,
     private RoScored<IListItem>[]? _filteredItems;
     private RoScored<IListItem>[]? _filteredApps;
 
-    // Keep as IEnumerable for deferred execution. Fallback item titles are updated
-    // asynchronously, so scoring must happen lazily when GetItems is called.
-    private IEnumerable<RoScored<IListItem>>? _scoredFallbackItems;
+    // Global/special fallbacks are re-scored lazily on the render path (GetSearchViewItems)
+    // instead of being frozen at keystroke time. Their dynamic titles are resolved
+    // asynchronously off the typing path by the fallback update manager, so deferring the
+    // score lets slow fallback results fold into the already-rendered list with fresh, correct
+    // scores from the same ranker. Fallbacks always classify at the FallbackFloor tier, so this
+    // only reorders them among themselves and can never leapfrog deterministic command/app
+    // results. We snapshot the source list and the precomputed query together so a superseding
+    // keystroke atomically replaces both.
+    private IReadOnlyList<IListItem>? _globalFallbackSources;
+    private FuzzyQuery _globalFallbackQuery;
+
+    // Common fallbacks use rank-based (query-independent) scores, so freezing them is safe;
+    // only their live titles decide whether they render, so they still fold in as they resolve.
     private IEnumerable<RoScored<IListItem>>? _fallbackItems;
 
     private bool _includeApps;
@@ -100,7 +109,6 @@ public MainListPage(
         _appStateService = appStateService;
         _tlcManager = topLevelCommandManager;
         _fuzzyMatcherProvider = fuzzyMatcherProvider;
-        _scoringFunction = (in query, item) => ScoreTopLevelItem(in query, item, _appStateService.State.RecentCommands, _fuzzyMatcherProvider.Current);
         _fallbackScoringFunction = (in _, item) => ScoreFallbackItem(item, _settingsService.Settings.FallbackRanks);
 
         _tlcManager.PropertyChanged += TlcManager_PropertyChanged;
@@ -259,9 +267,15 @@ public override IListItem[] GetItems()
 
     private IListItem[] GetSearchViewItems()
     {
-        var validScoredFallbacks = _scoredFallbackItems?
-            .Where(s => !string.IsNullOrWhiteSpace(s.Item.Title))
-            .ToList();
+        var fallbackScoringTime = DateTimeOffset.UtcNow;
+        var fallbackScoringFunction = CreateTopLevelScoringFunction(fallbackScoringTime);
+
+        // Re-score the global fallbacks against their current titles so that any fallback whose
+        // dynamic title resolved asynchronously (after first paint) folds into the list with a
+        // fresh score from the same ranker. This runs on the render path and is cheap because
+        // there are only a handful of configured global fallbacks. Deterministic command/app
+        // results below do not depend on this, so first paint never waits on the (async) fallbacks.
+        var validScoredFallbacks = ScoreDeferredFallbacks(_globalFallbackSources, _globalFallbackQuery, fallbackScoringFunction);
 
         var validFallbacks = _fallbackItems?
             .Where(s => !string.IsNullOrWhiteSpace(s.Item.Title))
@@ -277,6 +291,42 @@ private IListItem[] GetSearchViewItems()
             AppResultLimit);
     }
 
+    // Scores the current global-fallback snapshot against its query using the supplied ranker,
+    // dropping any whose (possibly still unresolved) title is empty. Extracted and made static
+    // so the fast-first-paint fold-in can be unit tested with a fake slow source: deterministic
+    // command/app results are produced entirely without this method, and re-scoring here always
+    // reflects the latest snapshot, so a superseding keystroke's snapshot replaces any stale one.
+    internal static List<RoScored<IListItem>>? ScoreDeferredFallbacks(
+        IReadOnlyList<IListItem>? sources,
+        in FuzzyQuery query,
+        ScoringFunction<IListItem> scoringFunction)
+    {
+        if (sources is null || sources.Count == 0)
+        {
+            return null;
+        }
+
+        var scored = InternalListHelpers.FilterListWithScores(sources, query, scoringFunction);
+        if (scored.Length == 0)
+        {
+            return null;
+        }
+
+        List<RoScored<IListItem>>? valid = null;
+        foreach (var s in scored)
+        {
+            if (string.IsNullOrWhiteSpace(s.Item.Title))
+            {
+                continue;
+            }
+
+            valid ??= new List<RoScored<IListItem>>(scored.Length);
+            valid.Add(s);
+        }
+
+        return valid;
+    }
+
     private IListItem[] GetDefaultViewItems()
     {
         if (_defaultViewDirty)
@@ -362,7 +412,12 @@ private void ClearResults()
         _filteredItems = null;
         _filteredApps = null;
         _fallbackItems = null;
-        _scoredFallbackItems = null;
+        _globalFallbackSources = null;
+
+        // Reset the paired query too. ScoreDeferredFallbacks already short-circuits on a null
+        // source list, so a stale query here is harmless, but clearing both keeps the snapshot
+        // pair symmetric and avoids a confusing leftover value.
+        _globalFallbackQuery = default;
     }
 
     public override void UpdateSearchText(string oldSearch, string newSearch)
@@ -563,17 +618,23 @@ private void UpdateSearchTextCore(string oldSearch, string newSearch, bool isUse
             }
 
             var searchQuery = _fuzzyMatcherProvider.Current.PrecomputeQuery(SearchText);
+            var scoringTime = DateTimeOffset.UtcNow;
+            var scoringFunction = CreateTopLevelScoringFunction(scoringTime);
 
             // Produce a list of everything that matches the current filter.
-            _filteredItems = InternalListHelpers.FilterListWithScores(newFilteredItems, searchQuery, _scoringFunction);
+            _filteredItems = InternalListHelpers.FilterListWithScores(newFilteredItems, searchQuery, scoringFunction);
 
             if (token.IsCancellationRequested)
             {
                 return;
             }
 
-            IEnumerable<IListItem> newFallbacksForScoring = commands.Where(s => s.IsFallback && configuredGlobalFallbackIds.Contains(s.Id));
-            _scoredFallbackItems = InternalListHelpers.FilterListWithScores(newFallbacksForScoring, searchQuery, _scoringFunction);
+            // Snapshot the global fallbacks and query, but do NOT score them here. Their dynamic
+            // titles are updated asynchronously by the fallback update manager (BeginUpdate, above),
+            // which runs off the typing path. Scoring is deferred to the render path so late fallback
+            // resolutions fold in with fresh scores instead of a value frozen against a stale title.
+            _globalFallbackSources = commands.Where(s => s.IsFallback && configuredGlobalFallbackIds.Contains(s.Id)).ToArray();
+            _globalFallbackQuery = searchQuery;
 
             if (token.IsCancellationRequested)
             {
@@ -590,7 +651,7 @@ private void UpdateSearchTextCore(string oldSearch, string newSearch, bool isUse
             // Produce a list of filtered apps with the appropriate limit
             if (newApps.Any())
             {
-                _filteredApps = InternalListHelpers.FilterListWithScores(newApps, searchQuery, _scoringFunction);
+                _filteredApps = InternalListHelpers.FilterListWithScores(newApps, searchQuery, scoringFunction);
 
                 if (token.IsCancellationRequested)
                 {
@@ -640,7 +701,9 @@ internal static int ScoreTopLevelItem(
         in FuzzyQuery query,
         IListItem topLevelOrAppItem,
         IRecentCommandsManager history,
-        IPrecomputedFuzzyMatcher precomputedFuzzyMatcher)
+        IPrecomputedFuzzyMatcher precomputedFuzzyMatcher,
+        Func<IListItem, ProviderSearchWeight>? providerWeightLookup = null,
+        DateTimeOffset? historyEvaluationTime = null)
     {
         var title = topLevelOrAppItem.Title;
         if (string.IsNullOrWhiteSpace(title))
@@ -678,37 +741,62 @@ internal static int ScoreTopLevelItem(
             ? (precomputedItem.GetTitleTarget(precomputedFuzzyMatcher), precomputedItem.GetSubtitleTarget(precomputedFuzzyMatcher))
             : (precomputedFuzzyMatcher.PrecomputeTarget(title), precomputedFuzzyMatcher.PrecomputeTarget(topLevelOrAppItem.Subtitle));
 
-        // Score components
+        // Score components. Keep the raw matcher scores so "did this signal match at
+        // all" is decided before the historical subtitle penalty (which can push a real
+        // subtitle match below zero).
         var nameScore = precomputedFuzzyMatcher.Score(query, titleTarget);
-        var descriptionScore = (precomputedFuzzyMatcher.Score(query, subtitleTarget) - 4) / 2.0;
-        var extensionScore = extensionDisplayNameTarget is { } extTarget ? precomputedFuzzyMatcher.Score(query, extTarget) / 1.5 : 0;
+        var rawSubtitleScore = precomputedFuzzyMatcher.Score(query, subtitleTarget);
+        var rawExtensionScore = extensionDisplayNameTarget is { } extTarget ? precomputedFuzzyMatcher.Score(query, extTarget) : 0;
 
-        // Take best match from title/description/fallback, then add extension score
-        // Extension adds to max so items matching both title AND extension bubble up
-        var baseScore = Math.Max(Math.Max(nameScore, descriptionScore), isFallback ? 1 : 0);
-        var matchScore = baseScore + extensionScore;
+        var descriptionScore = (rawSubtitleScore - 4) / 2.0;
+        var extensionScore = rawExtensionScore / 1.5;
 
-        // Apply a penalty to fallback items so they rank below direct matches.
-        // Fallbacks that dynamically match queries (like RDP connections) should
-        // appear after apps and direct command matches.
-        if (isFallback && matchScore > 1)
-        {
-            // Reduce fallback scores by 50% to prioritize direct matches
-            matchScore = matchScore * 0.5;
-        }
+        // Lexical quality preserves the previous relative weighting of the signals: best
+        // of title/description (plus the fallback floor), then a smaller extension-name
+        // contribution added on top so items matching both title AND extension bubble up.
+        var lexicalQuality = Math.Max(Math.Max(nameScore, descriptionScore), isFallback ? 1 : 0) + extensionScore;
 
-        // Alias matching: exact match is overwhelming priority, substring match adds a small boost
-        var aliasBoost = isAliasMatch ? 9001 : (isAliasSubstringMatch ? 1 : 0);
-        var totalMatch = matchScore + aliasBoost;
+        var matchedLexically = nameScore > 0 || rawSubtitleScore > 0 || rawExtensionScore > 0;
 
-        // Apply scaling and history boost only if we matched something real
-        var finalScore = totalMatch * 10;
-        if (totalMatch > 0)
+        // The hard tier decides ordering; recent usage and the alias-substring nudge only
+        // reorder items that already share a tier. ClassifyTier returns None precisely when
+        // nothing matched (no lexical, alias, or fallback signal), so this single gate also
+        // filters non-matches - no separate pre-check is needed.
+        var tier = MainListRanker.ClassifyTier(query.Original, title, isFallback, isAliasMatch, isAliasSubstringMatch, matchedLexically);
+        if (tier == RankTier.None)
         {
-            finalScore += history.GetCommandHistoryWeight(id);
+            return 0;
         }
 
-        return (int)finalScore;
+        var recentUsageWeight = historyEvaluationTime is { } now && history is RecentCommandsManager recentCommands
+            ? recentCommands.GetCommandHistoryWeight(id, now)
+            : history.GetCommandHistoryWeight(id);
+        var aliasSubstringBonus = isAliasSubstringMatch && !isAliasMatch ? MainListRanker.AliasSubstringBonus : 0.0;
+
+        // Per-provider weight is a within-tier nudge only. Resolving it here (rather than in
+        // the tier classifier) guarantees it can never promote an item across a tier boundary.
+        var providerWeight = providerWeightLookup?.Invoke(topLevelOrAppItem) ?? ProviderSearchWeight.Normal;
+        var providerBonus = MainListRanker.ProviderBonus(providerWeight);
+
+        var withinTier = MainListRanker.WithinTierScore(
+            lexicalQuality,
+            recentUsageWeight,
+            aliasSubstringBonus,
+            providerBonus: providerBonus);
+
+        return MainListRanker.Pack(tier, withinTier);
+    }
+
+    private ScoringFunction<IListItem> CreateTopLevelScoringFunction(DateTimeOffset historyEvaluationTime)
+    {
+        var recentCommands = _appStateService.State.RecentCommands;
+        return (in FuzzyQuery query, IListItem item) => ScoreTopLevelItem(
+            in query,
+            item,
+            recentCommands,
+            _fuzzyMatcherProvider.Current,
+            ResolveProviderSearchWeight,
+            historyEvaluationTime);
     }
 
     private static int ScoreWhitespaceQuery(string query, string title, string subtitle, bool isFallback)
@@ -761,6 +849,25 @@ private static string IdForTopLevelOrAppItem(IListItem topLevelOrAppItem)
         }
     }
 
+    // Resolves the user-configured per-provider search weight for an item. Top-level commands
+    // carry their own provider id; installed apps all belong to the well-known "AllApps"
+    // provider, so app items are weighted by that provider's setting.
+    private ProviderSearchWeight ResolveProviderSearchWeight(IListItem topLevelOrAppItem)
+    {
+        var providerId = topLevelOrAppItem is TopLevelViewModel topLevel
+            ? topLevel.CommandProviderId
+            : AllAppsCommandProvider.WellKnownId;
+
+        if (string.IsNullOrEmpty(providerId))
+        {
+            return ProviderSearchWeight.Normal;
+        }
+
+        return _settingsService.Settings.ProviderSettings.TryGetValue(providerId, out var providerSettings)
+            ? providerSettings.SearchWeight
+            : ProviderSearchWeight.Normal;
+    }
+
     public void Receive(ClearSearchMessage message) => SearchText = string.Empty;
 
     public void Receive(UpdateFallbackItemsMessage message)