Merge pull request #651 from LogExperts/feature/filter-spread-extraction

Extract Filter Spread into a pure, tested Core module

Author: Hirogen

CONTEXT.md

@@ -86,6 +86,24 @@ bare "session file" when you mean the workspace (that's a **Session**).
 *Avoid*: "control char rendering" (use **Substitution**), "escape" alone
 (ambiguous between the **C escape** style and the general concept).
 
+## Filtering
+
+- **Filter Spread** — Context expansion around a filter hit: **Back Spread**
+  (`FilterParams.SpreadBefore`) lines before and **Fore Spread**
+  (`FilterParams.SpreadBehind`) lines after the hit are included in the
+  filter result, deduplicated against recently emitted lines and clamped
+  to the file's line range (line 0 is a valid context line). Owned by
+  `LogExpert.Core.Classes.Filter.FilterSpread` — the single home of the
+  expansion, history-trim, and rollover-shift rules; the serial filter,
+  the parallel filter, and Filter Pipes all delegate to it. The maximum
+  UI-configurable spread (99) and the duplicate-suppression window
+  (2 × 99) derive from `FilterSpread.SPREAD_MAX`, which bounds the UI
+  spread knobs (`FilterParams` itself does not clamp its values).
+
+*Avoid*: "spread" alone when ambiguous (say **Back Spread** / **Fore
+Spread**), "additional filter results" (the old internal name — use
+**Filter Spread**).
+
 ## Columnizer selection
 
 - **Columnizer** (`ILogLineMemoryColumnizer`) — A plugin that parses a log

src/LogExpert.Core/Classes/Filter/Filter.cs

@@ -11,7 +11,6 @@ internal class Filter
     #region Fields
 
     private const int PROGRESS_BAR_MODULO = 1000;
-    private const int SPREAD_MAX = 50;
     private static readonly Logger _logger = LogManager.GetCurrentClassLogger();
 
     private readonly ColumnizerCallback _callback;
@@ -108,66 +107,13 @@ private int DoFilter (FilterParams filterParams, int startLine, int maxCount, Li
     private void AddFilterLine (int lineNum, FilterParams filterParams, List<int> filterResultLines, List<int> lastFilterLinesList, List<int> filterHitList)
     {
         filterHitList.Add(lineNum);
-        var filterResult = GetAdditionalFilterResults(filterParams, lineNum, lastFilterLinesList);
+        var filterResult = FilterSpread.Expand(lineNum, filterParams.SpreadBefore, filterParams.SpreadBehind, _callback.GetLineCount(), lastFilterLinesList);
 
         filterResultLines.AddRange(filterResult);
 
         lastFilterLinesList.AddRange(filterResult);
 
-        if (lastFilterLinesList.Count > SPREAD_MAX * 2)
-        {
-            lastFilterLinesList.RemoveRange(0, lastFilterLinesList.Count - SPREAD_MAX * 2);
-        }
-    }
-
-    /// <summary>
-    ///  Returns a list with 'additional filter results'. This is the given line number
-    ///  and (if back spread and/or fore spread is enabled) some additional lines.
-    ///  This function doesn't check the filter condition!
-    /// </summary>
-    /// <param name="filterParams"></param>
-    /// <param name="lineNum"></param>
-    /// <param name="checkList"></param>
-    /// <returns></returns>
-    private IList<int> GetAdditionalFilterResults (FilterParams filterParams, int lineNum, IList<int> checkList)
-    {
-        IList<int> resultList = [];
-
-        if (filterParams.SpreadBefore == 0 && filterParams.SpreadBehind == 0)
-        {
-            resultList.Add(lineNum);
-            return resultList;
-        }
-
-        // back spread
-        for (var i = filterParams.SpreadBefore; i > 0; --i)
-        {
-            if (lineNum - i > 0)
-            {
-                if (!resultList.Contains(lineNum - i) && !checkList.Contains(lineNum - i))
-                {
-                    resultList.Add(lineNum - i);
-                }
-            }
-        }
-        // direct filter hit
-        if (!resultList.Contains(lineNum) && !checkList.Contains(lineNum))
-        {
-            resultList.Add(lineNum);
-        }
-        // after spread
-        for (var i = 1; i <= filterParams.SpreadBehind; ++i)
-        {
-            if (lineNum + i < _callback.GetLineCount())
-            {
-                if (!resultList.Contains(lineNum + i) && !checkList.Contains(lineNum + i))
-                {
-                    resultList.Add(lineNum + i);
-                }
-            }
-        }
-
-        return resultList;
+        FilterSpread.TrimHistory(lastFilterLinesList);
     }
 
     #endregion

src/LogExpert.Core/Classes/Filter/FilterSpread.cs

@@ -0,0 +1,102 @@
+namespace LogExpert.Core.Classes.Filter;
+
+/// <summary>
+/// Owner of the Filter Spread rules: expanding a filter hit into its back/fore spread
+/// context lines and trimming the duplicate-suppression history.
+/// Pure — no dependency on controls, callbacks, readers or locks.
+/// </summary>
+public static class FilterSpread
+{
+    /// <summary>
+    /// Maximum configurable spread. The duplicate-suppression history window derives from
+    /// this value so it always covers the largest configurable spread.
+    /// </summary>
+    public const int SPREAD_MAX = 99;
+
+    /// <summary>
+    /// Trims the duplicate-suppression history to the last 2 × <see cref="SPREAD_MAX"/> entries.
+    /// </summary>
+    public static void TrimHistory (IList<int> history)
+    {
+        ArgumentNullException.ThrowIfNull(history);
+
+        while (history.Count > SPREAD_MAX * 2)
+        {
+            history.RemoveAt(0);
+        }
+    }
+
+    /// <summary>
+    /// Shifts collected line numbers by a multi-file rollover offset. Lines that roll off
+    /// the start of the virtual file are dropped.
+    /// </summary>
+    public static List<int> ShiftLines (IEnumerable<int> lines, int offset)
+    {
+        ArgumentNullException.ThrowIfNull(lines);
+
+        return [.. lines
+            .Select(lineNum => lineNum - offset)
+            .Where(line => line >= 0)];
+    }
+
+    /// <summary>
+    /// Rebuilds the duplicate-suppression history after a rollover shift: the last
+    /// <see cref="SPREAD_MAX"/> entries of the shifted result lines (all of them when
+    /// there are fewer). Capping at SPREAD_MAX — not the 2× trim window — preserves the
+    /// original rollover behavior.
+    /// </summary>
+    public static List<int> RebuildHistory (List<int> shiftedResultLines)
+    {
+        ArgumentNullException.ThrowIfNull(shiftedResultLines);
+
+        var count = Math.Min(SPREAD_MAX, shiftedResultLines.Count);
+        return shiftedResultLines.GetRange(shiftedResultLines.Count - count, count);
+    }
+
+    /// <summary>
+    /// Returns the lines a filter hit contributes to the filter result: the hit itself,
+    /// plus (if configured) back-spread and fore-spread context lines. Lines already present
+    /// in <paramref name="alreadyTaken"/> are suppressed. This function does not evaluate
+    /// the filter condition.
+    /// </summary>
+    public static IList<int> Expand (int lineNum, int spreadBefore, int spreadBehind, int lineCount, IList<int> alreadyTaken)
+    {
+        ArgumentNullException.ThrowIfNull(alreadyTaken);
+
+        IList<int> resultList = [];
+
+        if (spreadBefore == 0 && spreadBehind == 0)
+        {
+            resultList.Add(lineNum);
+            return resultList;
+        }
+
+        // back spread (ascending, clamped at line 0 — line 0 is a valid context line)
+        for (var i = spreadBefore; i > 0; --i)
+        {
+            var line = lineNum - i;
+            if (line >= 0 && !resultList.Contains(line) && !alreadyTaken.Contains(line))
+            {
+                resultList.Add(line);
+            }
+        }
+
+        // the hit itself
+        if (!resultList.Contains(lineNum) && !alreadyTaken.Contains(lineNum))
+        {
+            resultList.Add(lineNum);
+        }
+
+        // fore spread (ascending, clamped at the last line)
+        for (var i = 1; i <= spreadBehind; ++i)
+        {
+            var line = lineNum + i;
+            if (line < lineCount && !resultList.Contains(line) && !alreadyTaken.Contains(line))
+            {
+                resultList.Add(line);
+            }
+        }
+
+        return resultList;
+    }
+}

src/LogExpert.Tests/Filter/FilterSpreadTests.cs

@@ -0,0 +1,167 @@
+using System.Collections.ObjectModel;
+
+using LogExpert.Core.Classes.Filter;
+
+using NUnit.Framework;
+
+namespace LogExpert.Tests.Filter;
+
+[TestFixture]
+public class FilterSpreadTests
+{
+    // Row format: hit line, spread before, spread behind, line count, already-taken history, expected lines.
+    // Expected values are worked examples from the spec (docs/specs/filter-spread-extraction.md).
+    [TestCase(5, 0, 0, 100, new int[0], new[] { 5 },
+        TestName = "Expand_NoSpreadConfigured_ReturnsOnlyTheHitLine")]
+    [TestCase(5, 0, 0, 100, new[] { 5 }, new[] { 5 },
+        TestName = "Expand_NoSpreadConfigured_IgnoresHistory_HistoricalQuirkPreserved")]
+    [TestCase(0, 3, 2, 100, new int[0], new[] { 0, 1, 2 },
+        TestName = "Expand_HitAtLineZero_EmitsNoNegativeLines")]
+    [TestCase(3, 5, 0, 100, new int[0], new[] { 0, 1, 2, 3 },
+        TestName = "Expand_BackSpreadReachingPastTopOfFile_ClampsAtLineZeroAndIncludesIt")]
+    [TestCase(97, 0, 5, 100, new int[0], new[] { 97, 98, 99 },
+        TestName = "Expand_ForeSpreadReachingPastEndOfFile_ClampsAtLastLine")]
+    [TestCase(99, 0, 3, 100, new int[0], new[] { 99 },
+        TestName = "Expand_HitAtLastLineWithForeSpread_ReturnsOnlyTheHit")]
+    [TestCase(10, 1, 3, 100, new int[0], new[] { 9, 10, 11, 12, 13 },
+        TestName = "Expand_AsymmetricSpread_ReturnsBackLinesHitThenForeLines")]
+    [TestCase(12, 2, 2, 100, new[] { 8, 9, 10, 11, 12 }, new[] { 13, 14 },
+        TestName = "Expand_OverlappingWithEarlierHit_SuppressesLinesAlreadyTaken")]
+    [TestCase(5, 1, 1, 100, new[] { 5 }, new[] { 4, 6 },
+        TestName = "Expand_HitItselfAlreadyTaken_IsNotEmittedAgain")]
+    public void Expand_Table (int lineNum, int spreadBefore, int spreadBehind, int lineCount, int[] alreadyTaken, int[] expected)
+    {
+        var result = FilterSpread.Expand(lineNum, spreadBefore, spreadBehind, lineCount, alreadyTaken);
+
+        Assert.That(result, Is.EqualTo(expected));
+    }
+
+    [Test]
+    public void TrimHistory_LongerThanWindow_DropsOldestEntriesDownToWindowSize ()
+    {
+        // Window is 2 × 99 (the maximum configurable spread) — unifies the divergent 50/99 constants.
+        var history = new List<int>(Enumerable.Range(0, 250));
+
+        FilterSpread.TrimHistory(history);
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(history, Has.Count.EqualTo(198));
+            Assert.That(history[0], Is.EqualTo(52));
+            Assert.That(history[^1], Is.EqualTo(249));
+        });
+    }
+
+    [Test]
+    public void TrimHistory_WithinWindow_IsUnchanged ()
+    {
+        var history = new List<int>(Enumerable.Range(0, 198));
+
+        FilterSpread.TrimHistory(history);
+
+        Assert.That(history, Has.Count.EqualTo(198));
+    }
+
+    [Test]
+    public void TrimHistory_WorksOnAnyIListImplementation ()
+    {
+        // The Filter Pipe history is an IList<int>, not a List<int> — the trim rule must not care.
+        IList<int> history = new Collection<int>();
+        foreach (var i in Enumerable.Range(0, 200))
+        {
+            history.Add(i);
+        }
+
+        FilterSpread.TrimHistory(history);
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(history, Has.Count.EqualTo(198));
+            Assert.That(history[0], Is.EqualTo(2));
+        });
+    }
+
+    [Test]
+    public void ShiftLines_OnRollover_SubtractsOffsetFromEveryLine ()
+    {
+        var shifted = FilterSpread.ShiftLines([10, 20, 30], offset: 5);
+
+        Assert.That(shifted, Is.EqualTo(new[] { 5, 15, 25 }));
+    }
+
+    [Test]
+    public void ShiftLines_LinesRolledOffTheStart_AreDropped ()
+    {
+        // Offset 15 rolls lines 10 and 14 off the virtual file; 15 shifts to 0 and survives.
+        var shifted = FilterSpread.ShiftLines([10, 14, 15, 40], offset: 15);
+
+        Assert.That(shifted, Is.EqualTo(new[] { 0, 25 }));
+    }
+
+    [Test]
+    public void RebuildHistory_ResultsLongerThanSpreadMax_TakesTheLastSpreadMaxLines ()
+    {
+        // The rollover rebuild caps the history at SPREAD_MAX (99), not the 2×99 trim
+        // window — a preserved quirk of the original rollover code.
+        var results = new List<int>(Enumerable.Range(0, 150));
+
+        var history = FilterSpread.RebuildHistory(results);
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(history, Has.Count.EqualTo(99));
+            Assert.That(history[0], Is.EqualTo(51));
+            Assert.That(history[^1], Is.EqualTo(149));
+        });
+    }
+
+    [Test]
+    public void RebuildHistory_ResultsShorterThanSpreadMax_TakesAllOfThem ()
+    {
+        var history = FilterSpread.RebuildHistory([3, 4, 5]);
+
+        Assert.That(history, Is.EqualTo(new[] { 3, 4, 5 }));
+    }
+
+    [Test]
+    public void RebuildHistory_EmptyResults_ReturnsEmptyHistory ()
+    {
+        var history = FilterSpread.RebuildHistory([]);
+
+        Assert.That(history, Is.Empty);
+    }
+
+    /// <summary>
+    /// Equivalence guard: the single-threaded Log Window filter and the multi-threaded
+    /// Core filter accumulate per-hit results through the same recipe
+    /// (hit → Expand → append to results/history → TrimHistory). This pins that shared
+    /// recipe's output for a hit sequence with overlapping spreads; a call site that
+    /// deviates from the recipe no longer matches this specification. Expected values are
+    /// a worked example: hits 5, 7 and 50 with spread 2/2 in a 100-line file.
+    /// </summary>
+    [Test]
+    public void AccumulationRecipe_OverlappingHitSequence_ProducesPinnedResultHitAndHistoryLists ()
+    {
+        int[] hits = [5, 7, 50];
+        List<int> resultLines = [];
+        List<int> history = [];
+        List<int> hitList = [];
+
+        foreach (var hit in hits)
+        {
+            hitList.Add(hit);
+            var expanded = FilterSpread.Expand(hit, spreadBefore: 2, spreadBehind: 2, lineCount: 100, alreadyTaken: history);
+            resultLines.AddRange(expanded);
+            history.AddRange(expanded);
+            FilterSpread.TrimHistory(history);
+        }
+
+        Assert.Multiple(() =>
+        {
+            // Hit 5 contributes 3..7; hit 7 contributes only 8,9 (5..7 already taken); hit 50 contributes 48..52.
+            Assert.That(resultLines, Is.EqualTo(new[] { 3, 4, 5, 6, 7, 8, 9, 48, 49, 50, 51, 52 }));
+            Assert.That(hitList, Is.EqualTo(new[] { 5, 7, 50 }));
+            Assert.That(history, Is.EqualTo(resultLines), "history below the window size mirrors the result lines");
+        });
+    }
+}