Extract Filter Spread into a pure, tested Core module#651
Merged
Conversation
Move the back/fore spread expansion and duplicate-suppression history trimming out of the multi-threaded filter into FilterSpread, a pure static module in LogExpert.Core, pinned by table-driven NUnit tests. Two deliberate behavior changes, both test-pinned: - Line 0 is now a valid back-spread context line (was excluded by an off-by-one boundary check). - The history window is unified at 2x99 (the UI knob's maximum spread); the Core filter previously used 2x50, so parallel filtering with spread > 50 could emit duplicate context lines. The Log Window's serial filter, Filter Pipe processing and rollover shifting migrate in follow-up tickets (see tickets.md).
The Log Window's single-threaded filter path now accumulates per-hit results through FilterSpread.Expand/TrimHistory instead of its inline copy of the spread arithmetic. Locking, hit recording and the count capture stay at the call site. Behavior change on the serial path (sanctioned by the spec, pinned by ticket 1's tests): line 0 is now a valid back-spread context line. The history window is unchanged (was already 2x99 here). Adds an equivalence-guard test pinning the shared accumulation recipe (hit -> Expand -> append -> TrimHistory) with a worked example, so the serial and parallel call sites are held to identical output. The private GetAdditionalFilterResults copy remains for the Filter Pipe path (ticket 3); it and SPREAD_MAX are deleted in ticket 5.
ProcessFilterPipes now expands hits and trims the pipe history through FilterSpread instead of the Log Window's private copy. Pipe sequencing (OpenFile / per-line WriteToPipe / CloseFile) is unchanged; the per-line history trim is observably equivalent (history is only mutated here, so it never exceeds the window by more than one entry per add). Behavior change on the pipe path (sanctioned by the spec, pinned by the module tests): line 0 is now a valid back-spread context line, so pipe tabs receive the same context as the filter view. The trim window is unchanged (this path already used 2x99). TrimHistory widens from List<int> to IList<int> because the pipe history is an IList; trimming now removes from the front in a loop, pinned by a Collection<int>-backed test. Pulled forward from ticket 5: the private GetAdditionalFilterResults copy in LogWindow is deleted - both its callers are now migrated, so it was dead code. SPREAD_MAX stays (knobs + ShiftFilterLines, tickets 4-5).
ShiftFilterLines now delegates to two new pure module operations: - ShiftLines: subtract the rollover offset, drop lines that rolled off the start of the virtual file. - RebuildHistory: rebuild the duplicate-suppression history from the tail of the shifted results, capped at SPREAD_MAX (99, not the 2x99 trim window - preserving the original rollover behavior). The call site keeps its lock scope (result-list shift inside the lock, hit-list shift and history rebuild outside, exactly as before) and the GUI refresh. Both operations are TDD-pinned: plain shift, rolled-off lines dropping out, tail rebuild at the 99 cap, shorter-than-window results, and empty input. Note: four pre-existing commented-out lines inside the rewritten method body were removed along with the rewrite (old grid-refresh experiments). FilterPipe.ShiftLineNums is deliberately not migrated - it has different semantics (-1 placeholders preserve index mapping).
…et 5) The Log Window no longer carries any Filter Spread logic or constants: its private SPREAD_MAX is deleted and both spread knobs' maximum now references FilterSpread.SPREAD_MAX, so the knob range and the duplicate-suppression window derive from one value and cannot drift apart again. CONTEXT.md gains a Filtering section defining Filter Spread (Back Spread / Fore Spread), its ownership by the Core module, and the 99 / 2x99 derivation. Note: FilterParams itself does not clamp its values; the 99 cap is enforced at the UI knobs. This completes the Filter Spread extraction epic (tickets.md): all three call sites delegate, the arithmetic exists once in Core, and both sanctioned behavior changes are test-pinned.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
…/LogExperts/LogExpert into feature/filter-spread-extraction
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
LogWindow.cscarried two divergent private copies of the "filter spread" arithmetic (the back/fore context lines included around each filter hit) — one for the serial filter and pipes, one in Core for the parallel filter. Neither had a single test, and they had already drifted apart. This PR consolidates all of it into one pure module,LogExpert.Core.Classes.Filter.FilterSpread, pinned by 18 table-driven NUnit tests.What changed
FilterSpread(Core) — four pure operations, no dependency on controls, callbacks, readers, or locks:Expand— hit → back-spread lines, hit, fore-spread lines (deduplicated, clamped)TrimHistory— duplicate-suppression window (2 × 99)ShiftLines/RebuildHistory— multi-file rollover adjustmentFilter.cs), the Log Window's serial filter, Filter Pipe processing, and rollover shifting. The old copies are deleted; call sites keep their own locking, progress, and GUI refresh.FilterSpread.SPREAD_MAX(99), so they can no longer drift apart.Behavior changes (intentional, both test-pinned)
lineNum - i > 0silently excluded the first line of the file from every hit's leading context.An equivalence-guard test pins the shared per-hit accumulation recipe so the serial and parallel paths are held to identical output from now on.
Deliberately untouched:
FilterPipe.ShiftLineNums(different semantics —-1placeholders preserve index mapping) and the painting/grid code.Testing
LogExpert.Tests/Filter/FilterSpreadTests.cs: expansion edge table (line 0, EOF clamp, 0/0 shortcut, asymmetric/overlapping spreads, hit already in history), trim boundary at 2 × 99, rollover shift/drop/rebuild,IListtrim, and the accumulation equivalence guard.