refactor: extract Filter execution into a pure Core module (Filter Engine)#653
Merged
Conversation
IFilterEngine + FilterRun (Completed/Cancelled/Failed, partial lists stand) with two engines held to one contract: SerialFilterEngine (the Log Window loop, ported behind the seam) and ParallelFilterEngine (wrapping FilterStarter chunk-and-merge). FilterAccumulator is the single home of the per-hit spread recipe, shared later by the tail path. Contract pinned by a dual-engine equivalence table: sorted/deduped output byte-identical across engines, params + LineCount snapshotted at entry, chunk-boundary spread overlap, range filters spanning chunks, cancellation and throwing conditions as narrated outcomes (fixes the parallel path crash once call sites migrate). FilterStarter no longer raises the process-wide ThreadPool minimum (deliberate, spec-noted). Nothing in the UI changes yet. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CheckFilterAndHighlight had two near-identical ~40-line loop bodies (filter- tail branch vs no-filter branch) duplicating the whole trigger dispatch: match -> plugins -> GetTriggerActions -> audio -> bookmark -> stop-tail -> LED. One loop now carries the dispatch once, with the filter work gated by a doFilter condition; rollover handling hoisted above. Deliberate micro-change (spec-noted): a null line breaks and still flushes the filter GUI update and dirty LED for work already done - the old filter branch returned early and swallowed that flush. All trigger side effects stay at the call site, keeping the Tail trigger path invariant enforced by construction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
FilterSearch resolves SerialFilterEngine or ParallelFilterEngine from the MultiThreadFilter preference and runs it with a per-run CancellationToken; ESC reaches the run through the existing cancel-handler registry for both engines. The private Filter loop, MultiThreadedFilter, FilterProgressCallback and the FilterFxAction delegate are deleted. The control narrates the FilterRun outcome: durations as before, Failed as a localized status-line error (new resource, en/de/zh-CN) - no MessageBox from the worker, and a throwing condition on the multi-threaded path no longer crashes the app. Also migrated the second, unreviewed serial call site: RestoreFilterTabs now runs each restored Filter Pipe with fresh spread history instead of passing the window-shared _lastFilterLinesList, so a restored tab's content no longer depends on unrelated filter state. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
AddFilterLine reduces to a lock around an adopting FilterAccumulator - the accumulator wraps the window''s canonical result/hit/history lists per call (ClearFilterList/ShiftFilterLines replace the instances, so no long-lived accumulator field), which also makes the full run''s History handover seamless: the run seeds the very lists the tail continues. ProcessFilterPipes adopts each pipe''s history the same way; AddHit now returns the expansion so the pipe can write it out. The per-hit spread recipe (Expand -> append -> TrimHistory) no longer appears anywhere in LogWindow.cs, pinned by a new seeded-continuation test on the accumulator. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ticket 5) The shared mutable cancel flag and its whole supporting cast are gone: _shouldCancel, FilterCancelHandler, FilterStarter._shouldStop and Filter.ShouldCancel. A window-lifetime CTS is cancelled on CloseLogWindow; each job (filter run, Filter Pipes write, pattern statistics; Log Search''s _searchCts joins the model) creates a linked CTS at start and registers it in the per-window cancel-handler registry, which ESC fans out through. Reload and dead-file fire the registry instead of the flag - so they now cancel a running parallel filter too (previously only ESC could), while a dead file keeps the window lifetime alive for respawn. Filter/FilterStarter observe the token directly. SelectLine''s consume-the-flag gate is deleted; its search- epilogue duties (_isSearching reset, status clear) moved to SearchComplete. WriteFilterToTabFinished takes the cancelled outcome as a parameter instead of re-reading shared state. grep _shouldCancel over src/ returns nothing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CONTEXT.md gains Filter Engine, Filter Run and Filter Accumulator as canonical terms, Avoid entries for multi-threaded filter as a concept name and the deleted FilterFx delegate, and the Tail trigger path entry now reflects the unified loop and the by-construction audio invariant. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Comment on lines
+55
to
+59
| catch (Exception ex) | ||
| { | ||
| _logger.Error(ex, "Exception while filtering (parallel engine)"); | ||
| return Snapshot(filterStarter, FilterRunOutcome.Failed, ex); | ||
| } |
Comment on lines
+63
to
+67
| catch (Exception ex) | ||
| { | ||
| _logger.Error(ex, "Exception while filtering (serial engine)"); | ||
| return Snapshot(accumulator, FilterRunOutcome.Failed, ex); | ||
| } |
Ctrl+W during a filter run threw InvalidOperationException from FilterSearch''s continuation: the run''s token is linked to the window lifetime, so closing now cancels the run promptly - and the continuation then called BeginInvoke(CheckForFilterDirty) on a control whose handle was already destroyed. FilterComplete had a disposal guard; the surrounding epilogue did not. Both the filter run''s and the pipe write''s post-run UI epilogues now return early when the window is disposed, disposing, waiting for close, or handleless. Found by the manual smoke (close tab mid-filter). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
After close-mid-filter, the main window kept showing Filtering... and a frozen progress bar: the parent status bar only updates from the current tab''s events, and the cancelled job''s UI epilogue now (correctly) skips a disposed window - so nobody reset it. CloseLogWindow clears the status line and hides the progress bar itself, right after cancelling its jobs, while its events are still wired to the parent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The close-mid-run guard used !IsHandleCreated as a proxy for the window going away - but a session-restored background tab legitimately runs its restore filter before its own handle exists (WinForms marshals Invoke through the parent), so the guard skipped FilterComplete and the filter grid was never populated even though the result lists were full. The discriminator is now _isClosing, which CloseLogWindow sets synchronously before cancelling the window token, so close-cancelled continuations still bail while restore-time runs reach the grid update. Same correction in the pipe-write epilogue guard. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
LogWindow.PatternStatistic() and the whole PatternWindow/TestStatistic feature had no UI entry point - no menu ever called it, so the feature was unreachable dead code. A View/Navigate > Pattern analysis... item (localized en/de/zh-CN, enabled with the other per-file items) now opens it again. Spike wiring to evaluate the long-orphaned feature: if it turns out rotten, the follow-up is deletion; either way ticket 5''s token-based cancellation of TestStatistic/DetectBlock becomes manually testable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
InitPatternWindow created and configured the PatternWindow but its Show() call was commented out - the second half of how the feature went dark. The window is now shown owned by the main form, and a click while it is already open activates the existing instance instead of constructing a hidden new one. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This reverts commit 282880e.
This reverts commit 0118dbd.
…atch wasFollow was read from _guiStateArgs.FollowTail immediately after the enclosing condition required that same property to be true, so it was always true and the inner condition reduced to firstStopTail. Inherited verbatim from both pre-dedupe loop bodies; now that the dispatch exists once, the dead variable goes. firstStopTail stays - it enforces the scroll-once-per-batch rule if follow-tail is re-enabled mid-batch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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 decomposition (after Filter Spread and Log Search): the two filter implementations that had to agree but shared no contract — the single-threaded loop private to
LogWindowand theFilterStarter-backed multi-threaded path — now sit behind one seam inLogExpert.Core, held to byte-identical output by a dual-engine equivalence test table.IFilterEngine—Run(filterParams, callback, cancellationToken, progress)→FilterRun(ResultLines / HitLines / History + Completed | Cancelled | Failed). Two engines:SerialFilterEngine(the loop, moved out of the control) andParallelFilterEngine(wrappingFilterStarter's chunk-and-merge). TheMultiThreadFilterpreference picks; the control only narrates.FilterAccumulator— the single owner of the per-hit spread recipe (hit → Expand → append → TrimHistory), used by the serial engine, the tail filter path, and Filter Pipes. The recipe no longer exists anywhere inLogWindow.cs._shouldCancelflag and its cast (FilterCancelHandler,FilterStarter._shouldStop,Filter.ShouldCancel,SelectLine's consume-the-flag gate) are deleted. A window-lifetime CTS is cancelled on close; each job (filter run, Filter Pipes write, pattern statistics; Log Search's_searchCtsjoins the model) runs on a linked per-job token,fanned out to by ESC and reload via the existing cancel-handler registry.
grep _shouldCanceloversrc/returns nothing.CheckFilterAndHighlight's two duplicated ~40-line trigger-dispatch loop bodies unified into one loop (side effects stay at the call site, preserving the "Audio Alert fires only on the tail path" invariant by construction).User-visible fixes
async void) and no longer pops a worker-threadMessageBoxon the single-threaded path — both narrate a localized "Filter failed: …" on the status line (en/de/zh-CN).Deliberate behaviour changes (spec-noted, test-pinned where reachable)
[0, LineCount-at-entry)— the serial run no longer chases EOF mid-run; appended lines belong to the tail path (pinned).FilterParamsare snapshotted atRunentry — filter-panel edits can't affect a run in flight (pinned; same ruleLogSearcherfollows).ThreadPool.SetMinThreadsis no longer raised process-wide per filter run.Tests
FilterEngineEquivalenceTests(every fixture through both engines — overlapping spreads, chunk-boundary spread overlap, range filters spanning chunks, line 0/EOF clamps, cancellation, throwing conditions, params/LineCount snapshot pins) andFilterAccumulatorTests(recipe worked example + seeded continuation).