[CmdPal] Address review: O(1) frecency lookup, clearer migration comment

- 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>

Author: michaeljolley

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/RecentCommandsManager.cs

@@ -34,14 +34,26 @@ public record RecentCommandsManager : IRecentCommandsManager
     // evicted still-useful history within a single busy session on active machines.
     internal const int MaxHistoryEntries = 500;
 
-    // Legacy history items (persisted before LastUsed existed) deserialize with a default
-    // timestamp. Treat them as used one day ago: recent enough that they still rank, mild
-    // enough that a single fresh use outranks them, and uniform so their relative order falls
-    // back to Uses (frequency) instead of collapsing to all-equal or zero.
+    // Defensive fallback for any history item that carries a default (missing) LastUsed - for
+    // example a value constructed without a timestamp, or state written by a build that predates
+    // the LastUsed field. Such an item is treated as used one day ago: recent enough that it
+    // still ranks, mild enough that a single fresh use outranks it, and uniform so a group of
+    // them falls back to Uses (frequency) ordering instead of collapsing to all-equal or zero.
+    // In practice this rarely fires: earlier builds never actually persisted history (the
+    // internal History property was dropped by the serializer, see [JsonInclude] below), so
+    // upgrading users start from an empty store rather than one full of timestamp-less items.
     internal static readonly TimeSpan LegacyBackdate = TimeSpan.FromDays(1);
 
     private ImmutableList<HistoryItem>? _history = ImmutableList<HistoryItem>.Empty;
 
+    // Cached commandId -> entry lookup over History, rebuilt lazily whenever History is
+    // (re)assigned and never persisted. ScoreTopLevelItem calls GetCommandHistoryWeight for
+    // every candidate item on every keystroke, and History can hold up to MaxHistoryEntries
+    // (500) entries, so a plain linear scan would be O(items x history) per keystroke. The
+    // dictionary keeps each lookup O(1) so the hot path stays cheap as the store grows.
+    [JsonIgnore]
+    private Dictionary<string, HistoryItem>? _index;
+
     // Persisted so recent-command frecency (including the LastUsed timestamps) survives a
     // restart. [JsonInclude] is required because the property is internal; without it the
     // source-generated serializer emits an empty object and history is silently dropped.
@@ -50,7 +62,38 @@ public record RecentCommandsManager : IRecentCommandsManager
     internal ImmutableList<HistoryItem> History
     {
         get => _history ?? ImmutableList<HistoryItem>.Empty;
-        init => _history = value;
+        init
+        {
+            _history = value;
+
+            // Invalidate the cached lookup so it is rebuilt from the new list on next use.
+            // A record 'with' copy carries over the old field, so this reset is what keeps
+            // the index from going stale after History changes.
+            _index = null;
+        }
+    }
+
+    private Dictionary<string, HistoryItem> Index
+    {
+        get
+        {
+            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)
+                {
+                    // History is most-recent-first and command ids are unique in it, but keep
+                    // the first (most recent) occurrence if a duplicate ever slips in so the
+                    // lookup matches the old FirstOrDefault behavior.
+                    map.TryAdd(item.CommandId, item);
+                }
+
+                _index = map;
+            }
+
+            return _index;
+        }
     }
 
     public RecentCommandsManager()
@@ -68,14 +111,13 @@ public int GetCommandHistoryWeight(string commandId)
     /// </summary>
     internal int GetCommandHistoryWeight(string commandId, DateTimeOffset now)
     {
-        var entry = History.FirstOrDefault(item => item.CommandId == commandId);
-        if (entry is null)
+        if (!Index.TryGetValue(commandId, out var entry))
         {
             return 0;
         }
 
-        // Migrate legacy entries (no timestamp) to a mild backdate so they degrade to
-        // Uses-ordering instead of appearing brand-new or invisible.
+        // Migrate items with a default (missing) timestamp to a mild backdate so they degrade
+        // to Uses-ordering instead of appearing brand-new or invisible. See LegacyBackdate.
         var lastUsed = entry.LastUsed == default ? now - LegacyBackdate : entry.LastUsed;
 
         // Clamp age at zero so a slightly-future timestamp (e.g. clock skew) can't amplify.

src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/RecentCommandsTests.cs

@@ -202,6 +202,38 @@ public void ValidateFrequencyWeighting()
         Assert.IsTrue(many <= RecentCommandsManager.MaxWeight, "Weight stays within the documented cap");
     }
 
+    [TestMethod]
+    public void ValidateLookupIndexStaysConsistentAcrossChanges()
+    {
+        // The commandId lookup is backed by a cached index that must be invalidated whenever
+        // history changes (WithHistoryItem returns a new record, and a 'with' copy carries the
+        // old cached field over). Force the index to build on one instance, then mutate, and
+        // confirm each instance reports exactly its own history.
+        var now = new DateTimeOffset(2025, 6, 1, 0, 0, 0, TimeSpan.Zero);
+        var first = new RecentCommandsManager().WithHistoryItem("alpha", now);
+
+        // Read once to build the cached index on 'first'.
+        var alphaFirst = first.GetCommandHistoryWeight("alpha", now);
+        Assert.IsTrue(alphaFirst > 0, "alpha should have weight on the first instance");
+        Assert.AreEqual(0, first.GetCommandHistoryWeight("beta", now), "beta is not in the first instance");
+
+        // Add a use of alpha and a brand-new beta, producing a new instance.
+        var second = first
+            .WithHistoryItem("alpha", now.AddDays(1))
+            .WithHistoryItem("beta", now.AddDays(1));
+
+        // The new instance sees the newer alpha (more recent + higher use) and the new beta.
+        Assert.IsTrue(
+            second.GetCommandHistoryWeight("alpha", now.AddDays(1)) > 0,
+            "alpha should still resolve on the updated instance");
+        Assert.IsTrue(
+            second.GetCommandHistoryWeight("beta", now.AddDays(1)) > 0,
+            "beta should resolve on the updated instance after its index rebuilds");
+
+        // The original instance is unchanged - its index never gained beta.
+        Assert.AreEqual(0, first.GetCommandHistoryWeight("beta", now), "the original instance must not see beta");
+    }
+
     [TestMethod]
     public void ValidateHistoryCap()
     {