fix(quickfiler): carry the initialised folder predictor to the item controller (#678) - #724
Merged
drmoisan merged 11 commits intoSep 2, 2026
Conversation
…ted child The parallel-add preparation child for issue 678 was terminated by a session rate limit after authoring the acceptance criteria and the research artifact but before committing them. Its work survived only as untracked files in its worktree, so this commit preserves it on the item branch. Contents: - issue.md with an authored Acceptance Criteria section of 23 criteria, required because the promoted body carried none and the work mode is minor-audit. - research/2026-08-31T21-15-quickfiler-carry-folder-predictor-research.md, 837 lines. - plan.2026-08-31T21-12.md, still the unmodified scaffold. Plan authoring did not run. This path is committed so the resumed planner revises it in place under the Plan-Path Continuity Contract rather than creating a sibling. The parent parallel-orchestrator performed only this terminal commit step. It authored no plan content, no acceptance criteria and no research text. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ATYLDoRLKXS5sgAzegW7ZL
Author the minimal-audit atomic plan for carrying the initialised folder predictor from the dequeue-time confidence gate through to the QuickFiler item controller, and drive it to preflight clearance. - 3 phases, 41 tasks (13/13/15), authored against the 23 acceptance criteria in issue.md as the sole requirements source - Planned against the corrected reading in the preparation research: the live producer is the streaming dequeue confidence gate rather than the dormant post-display filter, and both re-scoring legs are threaded - 4 preflight rounds returning 19, 7, 2 and 0 defects - MCP plan validator reports ok:true with no warnings Preparation only. No implementation, no pull request, no merge. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ATYLDoRLKXS5sgAzegW7ZL
…kfiler-carry-folder-predictor-to-item-controller-678
…ontroller (#678) High-confidence mode scored every accepted mail item twice: once by the dequeue-time confidence gate, and again by the item controller after Show(). The gate built and initialised a FolderPredictor, read two scalars off it, and let it fall out of scope; the item controller then built and initialised a second predictor for the same item. Carry the already-initialised IFolderSearchHandler forward from the gate to the item controller on both display legs, and adopt it in place of the second initialisation. Producer and carrier chain: - QfcPreScoredItem gains an IFolderSearchHandler member and an optional third constructor parameter; MailItem and PredeterminedFolder keep their names, types and non-null contracts. - IFolderScoringService.ScoreAsync widens to publish the handler it initialises instead of discarding it. FolderScoringService keeps its [ExcludeFromCodeCoverage] attribute and its justification remark. - The gate's scoreLoader delegate and acceptance projection, and QfcDatamodel.ScoreRemainingQueueMailItemAsync, forward the handler, so it reaches QfcGateBatch.Accepted and QfcDequeueBatch.PreScored. Leg A, the first page: - QfcHomeController.RunAsync in high-confidence-enabled mode reads the outcome-returning dequeue member and selects the IList<QfcPreScoredItem> overload of LoadItemsAsync. Disabled mode is unchanged. - QfcItemGroup carries the handler; QfcCollectionController.EncapsulateItemGroup and the carrier overload of LoadControlsAndHandlers_01Async thread it into the QfcItemController constructor. Leg B, every subsequent page: - IterateQueueAsync forwards batch.PreScored into IQfcQueue.EnqueueAsync, and QfcQueue carries the handler to the rows it constructs, matching carrier to item by EntryID because UnhookDequeuedNodes can replace an element in place. - Adds the QfcQueue.ItemControllerFactory injectable-delegate seam, whose production default reproduces the previous construction expression exactly. No new interface. Adoption: - LoadFolderHandlerAsync adopts a carried handler inside its varList is null branch only, invoking neither _folderPredictorFactory nor FolderPredictor.InitAsync. The FromArrayOrString branches are unchanged. - Cleanup releases the carried reference alongside _folderHandler. Also resolves the raw-versus-projected path mismatch: FolderArray entries are archive-prefix-stripped while the carried PredeterminedFolder is the raw suggestion path, so FolderContains missed every archive-rooted suggestion and the selection fell back to index 1. AssignFolderComboBox now projects the carried value the same way before the probe. Accepted behavioural delta: reusing the scan-time suggestion set freezes conversation-derived (CtfMap) suggestions at scan time rather than re-deriving them at display time, for both legs; the scan-to-display interval is longer for leg B. Bayesian suggestions and the recents list are unaffected because the folder array is still built lazily at display time. Oversized files are not extended: members that had to gain a parameter were relocated in full into new partial parts, so QfcCollectionController.cs, QfcQueue.cs and QfcFormControllerTests.cs are all smaller than at the base ref. Tests: eight new MSTest tests using Moq and FluentAssertions, no temporary files and no live Outlook COM. Enabled-mode RunAsync tests were rewritten onto the carrier overload rather than deleted; the disabled-mode Times.Never assertions and both preFilterInvoked assertions are byte-identical to their base-ref text, so QfcHighConfidencePreFilter.FilterAsync remains dormant.
Commits every evidence artifact produced by the atomic plan for issue #678, plus the plan checklist state and the acceptance-criteria check-off in issue.md. Phase 0 baseline (13 artifacts): policy reads, minor-audit integrity, base-ref anchor with its re-comparison at all three phase boundaries, dotnet tool restore, CSharpier check, analyzer build, nullable build, the MSTest coverage run, the root and per-file coverage figures, the package-level JaCoCo summary, the file-size census, and the carrier construction-site inventory. Phase 1 (11 artifacts): the implementation handoff packet, the compile seam, the red and green runs of the AC16 single-initialisation regression test, the carrier chain, both display legs, the AC9 negative guard, the AC12 path normalisation with its fail-before evidence, the change description, and the AC22 out-of-scope register. Phase 2 (13 artifacts): the five toolchain-gate records, post-change coverage with its JaCoCo summary, the changed-line and per-member coverage delta, the coverage-exclusion attribute invariant, the file-size audit, the scope confinement audit, the clean-pass declaration, and the per-criterion verdict register. Acceptance criteria: 22 of 23 checked off in issue.md. AC20 is left unchecked and recorded as PARTIAL. Three of its four clauses hold - no regression on the changed lines, figures recorded numerically, and no coverage-exclusion attribute added or removed - but its 90 percent per-member clause fails for QfcQueue.EnqueueAsync and QfcQueue.LoadControllersViewersAsync, both COM- and WinForms-bound and both equally uncovered before this change. The reasoning is in evidence/qa-gates/coverage-delta.md. The raw Cobertura reports are deliberately not committed: coverage/ is git-ignored and a full-repository document is too large for permanent history. Both sides are represented by committed package-level JaCoCo summaries whose LINE counters reproduce the recorded lines-covered and lines-valid values.
… inputs and plan Adds the policy-audit, code-review and feature-audit artifacts from the round-1 feature review (0 blocking, 8 non-blocking), plus the remediation inputs and preflight-pending plan for cycle 1. Cycle 1 addresses the findings that are defects introduced by this change: the leg A pre-unhook carrier set, the projection parity divergence, the unobserved cancellation token, and the evidence timestamp values. Refs #678 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ATYLDoRLKXS5sgAzegW7ZL
… rounds Round 1 reported eleven defects, round 2 three more. All fourteen are applied. The substantive ones: a P2-T5 clause asserted a per-test pass list the coverage runner never prints; a base-anchored file-size audit was unpassable against three pre-existing over-cap files; derivations D2, D3 and D6 could return a silently empty result; and the DR3 rationale named the FromArrayOrString route for a logger.Error that the FromField route emits, which P1-T9 would have transcribed into a production comment. Refs #678 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ATYLDoRLKXS5sgAzegW7ZL
…cancel (#678) Remediation cycle 1 for issue #678. Closes R1, R2, R3 and R4 from remediation-inputs.2026-09-01T23-44.md. R1 - leg A displayed the pre-unhook carrier set. QfcDequeueBatch.PreScored is captured before UnhookDequeuedNodes and Items after it, and TryUnhookOrReplace mutates on the UnhookItem throw path: it removes the failed item and inserts a substitute pulled from the master queue. Consuming PreScored directly therefore displayed a still-hooked item and lost a substitute that had already left the queue. Leg A now reconciles against Items at the load boundary through the new QfcPreScoredItem.ReconcileCarriersToItems, which mirrors leg B. The matching body is generalised into QfcPreScoredItem.ResolveCarrier and QfcQueue.ResolveCarriedHandler delegates to it, so exactly one carrier-matching implementation exists in the tree. Reference identity is tried before EntryID. The QfcDatamodel.QueueProcessing doc block no longer claims an unconditional correspondence. R2 - ProjectPredeterminedFolder guarded on an empty archive root where FolderPredictor.ProjectSuggestionPath guards on null globals, so the two diverged for (non-null globals, empty archive root, leading-separator path) and the AC12 mismatch reopened in that state. The guard now tests archiveRootPath is null and the call site emits null only for a null _globals. FolderPredictor.cs is the parity target and is not modified. R3 - the carried-handler adoption branch returned without observing the cancellation token that every pre-change route observed via Task.Run(..., cancel). It now calls cancel.ThrowIfCancellationRequested() as the branch's first statement, which reproduces the pre-change OperationCanceledException without removing the logger.Error the FromField route emitted. R4 - the 13 evidence artifacts under evidence/qa-gates/ declared timestamps running 9 to 81 minutes ahead of their own mtimes. All 17 declarations (12 top-level plus 5 nested) are corrected to the yyyy-MM-ddTHH-mm truncation of each artifact's own pre-edit LastWriteTime. No Command, EXIT_CODE or Output Summary value is altered. Tests: three new regression tests, each recorded red before the fix and green after. No existing passing test is weakened or deleted; the single assertion corrected is the one R2 authorises, whose asserted parity was untrue. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ATYLDoRLKXS5sgAzegW7ZL
… plan Phase 2 final QC loop for issue #678 remediation cycle 1, plus the CSharpier reflow of two files the format pass rewrote. Toolchain, one uninterrupted pass, all exit 0: csharpier format . Formatted 1575 files csharpier check . Checked 1575 files, empty drift set msbuild analyzer build 5 warnings / 0 errors, CoreCompile 57 msbuild nullable build 0 CS86, CoreCompile 72 MSTest with coverage 6949 total, 6949 passed, 0 failed, 0 skipped One loop restart, caused by cosmetic CSharpier reflow of this cycle's own new code on the first format pass. Coverage: repository-wide line 85.40% and branch 79.45%, both marginally above the P0 baseline of 85.40% and 79.44%. Changed-line coverage for this cycle's own lines is 34/34 (100.00%). All seven new or modified non-exempt members are at or above 90%: ResolveCarrier, ReconcileCarriersToItems and ResolveCarriedHandler at 100%, RunAsync and ProjectPredeterminedFolder at 100%, AssignFolderComboBox at 90.62%, LoadFolderHandlerAsync at 94.67%. Invariants asserted: no [ExcludeFromCodeCoverage] attribute added or removed over a 2127-added/620-removed diff; footprint confined to QuickFiler, QuickFiler.Test and the feature folder with 116 of 116 staged paths in prefix; issue.md digest byte-identical to its Phase 0 preimage, so no acceptance-criterion text changed and AC20 remains unchecked; no file over the 500-line cap. P2-T13 records a plan defect: its re-measurement band and its correction instruction form a fixpoint, because correcting an artifact's timestamp rewrites that artifact's mtime. 22 of this cycle's own artifacts had drifted ahead of their write times and were corrected to a real observed clock value; the band clause is reported as unsatisfiable rather than dispositioned into a pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ATYLDoRLKXS5sgAzegW7ZL
Closing reaudit for remediation cycle 1: 0 blocking findings, 7 non-blocking. All four remediation items (R1 leg A carrier reconciliation, R2 projection alignment, R3 cancellation observation, R4 timestamp fidelity) verified in source. AC20 remains PARTIAL and unchecked; the reviewer records it as self-limiting, since covering the two COM-bound QfcQueue members requires either a live window that the unit-test policy prohibits or an exclusion attribute that AC20's own fourth clause prohibits. Refs #678 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ATYLDoRLKXS5sgAzegW7ZL
This was referenced Sep 2, 2026
drmoisan
deleted the
bug/quickfiler-carry-folder-predictor-to-item-controller-678
branch
September 2, 2026 13:31
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
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.
fix(quickfiler): carry the initialised folder predictor to the item controller
Summary
Show(). The folder predictor the dequeue-time confidence gate already initialised is carried forward toQfcItemControllerinstead of being discarded and rebuilt.QfcHomeController.RunAsync) and leg B (every subsequent page, throughIterateQueueAsyncintoQfcQueue).QfcDequeueBatch.Itemsrather than consumed directly fromQfcDequeueBatch.PreScored, so an item whoseEmailMoveMonitorunhook failed is not displayed and a substitute pulled from the master queue is not silently dropped.FolderArrayentries now use the same archive-prefix projection, soFolderContainsmatches for archive-rooted suggestions instead of falling through to an index default.Why
Issue #678 records the consumer-side work needed to remove a redundant per-item scoring pass in High Confidence mode. Preparation research established that carrying only the top-folder string does not remove the second pass:
FolderArray,SuggestionsandFolderRowArrayare all produced from_folderHandler, so the item controller still had to runFolderPredictor.InitAsync(FromField)on every accepted item even when a fully initialised predictor already existed upstream.FolderScoringService.ScoreAsyncbuilt that predictor and returned only(score, topFolder), so the initialised instance never reached the caller.The user-visible effect was wasted work rather than incorrect behaviour: slower folder-combo population and redundant Outlook COM traffic proportional to the number of rows on screen.
What Changed
Producer and carrier chain
QfcPreScoredItemcarries anIFolderSearchHandleralongside its existingMailItemandPredeterminedFoldermembers, which keep their names, types and non-null contracts.IFolderScoringService.ScoreAsyncandFolderScoringServicepublish the handler they initialise instead of discarding it.QfcStreamingDequeueConfidenceGatescoreLoaderdelegate, its acceptance projection, andQfcDatamodel.QueueProcessing.ScoreRemainingQueueMailItemAsync, so it is present onQfcGateBatch.AcceptedandQfcDequeueBatch.PreScored.Consumers
RunAsyncselects the outcome-returning dequeue and theIList<QfcPreScoredItem>overload ofLoadItemsAsync. High-confidence-disabled mode continues to select theIList<MailItem>overload unchanged.IterateQueueAsyncforwardsbatch.PreScoredintoQfcQueue.EnqueueAsync, which carries the handler to theQfcItemControllerinstances it constructs.QfcItemController.LoadFolderHandlerAsyncadopts a carried handler inside itsvarList is nullbranch only. TheFromArrayOrStringbranches of bothLoadFolderHandlerandLoadFolderHandlerAsyncare unchanged, and a carried handler is never adopted on aFromArrayOrStringcall.QfcItemControllercleanup alongside_folderHandler.Correctness work from the post-review remediation cycle
QfcPreScoredItem.ResolveCarrierandReconcileCarriersToItemsmakebatch.Itemsthe leg A spine. Matching is by reference identity first, thenEntryID. An item with no matching carrier receives a carrier with a null handler and is scored normally at display time.ProjectPredeterminedFolderwas aligned toFolderPredictor.ProjectSuggestionPathrather than narrowing the documented claim;UtilitiesCSis unmodified.Task.Run(..., cancel).Tests — new MSTest coverage for the single-initialisation invariant, the negative guard proving a carried handler is ignored when
varListis non-null, archive-rooted projection, the leg A post-unhook reconciliation, and cancellation observation. Existing pinned tests were rewritten to assert the carrier overload rather than deleted or weakened.Architecture / How It Fits Together
ReconcileCarriersToItemsis the leg A join.ItemsandPreScoredare captured on opposite sides ofUnhookDequeuedNodes, which is not read-only: on anUnhookItemthrow,TryUnhookOrReplaceremoves the failed item, takes a replacement from the master queue and re-inserts it. TakingItemsas the spine is what keeps the displayed set equal to the set that survived that pass.Verification
Completed. Full C# toolchain in policy order, final uninterrupted pass:
dotnet tool run csharpier format .Formatted 1575 files in 2042ms.(tree observation before/after identical)dotnet tool run csharpier check .Checked 1575 files in 4937ms./t:Rebuild)/t:Rebuild)CS86diagnosticsTest Run Successful.Total 6949, Passed 6949, Failed 0, Skipped 0Coverage, read directly from the post-processed Cobertura document rather than from a summary tool:
lines-covered55086 /lines-valid64506). Both rose against the same-session baseline.These clear the 80% floor in
CLAUDE.mdand the 85% line / 75% branch floors in.claude/rules/general-unit-test.mdand.claude/rules/quality-tiers.md.Recommended for the reviewer.
Backward Compatibility / Migration Notes
QfcPreScoredItem's two existing members keep their names, types and non-null contracts.QfcHighConfidencePreFilter.FilterAsyncremains dormant andHighConfidencePreFilterLoaderremains uninvoked. TheTimes.Neverassertions that pin the landed decision of issue Feature: quickfiler-high-confidence-dequeue-streaming #233 are preserved verbatim.[ExcludeFromCodeCoverage]attribute is added or removed anywhere in the change; verified at zero added and zero removed lines across the anchored diff.CtfMap) suggestions at scan time rather than re-deriving them at display time, on both legs. The scan-to-display interval is longer on leg B. Bayesian suggestions and the recents list are unaffected, because the folder array is still built lazily at display time.Risks and Mitigations
EntryID-less mock carrier in an existing passing test would be stranded by anEntryID-first matcher. The change only widens matching, never narrows it.QuickFilerandQuickFiler.Testis touched.Review Guide
Suggested order:
QuickFiler/Controllers/QfcHighConfidencePreFilter.cs— the carrier, the scoring seam, andResolveCarrier/ReconcileCarriersToItems.QuickFiler/Controllers/QfcHomeController.csandQfcHomeController.Iteration.cs— overload selection and the two legs.QuickFiler/Controllers/QfcItemController.FolderHandling.cs— adoption, projection, cancellation.QuickFiler/Controllers/QfcQueue.Enqueue.csandQfcCollectionController.CarrierLoad.cs— new partial parts.Mechanical noise to expect:
QfcQueue.Enqueue.csandQfcCollectionController.CarrierLoad.csare new partial parts created to keep files at or under the 500-line limit. Much of their content is relocated, not new.QfcQueue.Enqueue.csshows a per-file coverage drop from 28.00% to 15.29% that is not a regression: the uncovered line count is unchanged at exactly 72, and the ratio moved only because 15 covered lines were removed.Footprint: 16 files under
QuickFiler/, 20 underQuickFiler.Test/, 86 feature-folder documents, and nothing outside those three prefixes.Follow-ups
None of the below is promoted here; they are being collected into a single consolidated issue filed separately.
QfcQueue.EnqueueAsync(0/46) andLoadControllersViewersAsync(0/24). Both were equally uncovered before this change, so this is not a coverage regression — the combinedQfcQueuesurface improved from 41.47% to 44.90%. The criterion is self-limiting on those two members: covering them requires either a live Outlook window, which the unit-test policy prohibits, or an exclusion attribute, which AC20's own fourth clause prohibits.QfcItemController.LoadFolderHandlernever callsInitAsync(live).MailItemHelper.FromMailItemAsynccalls (live).QfcCollectionController.cs2336,QfcFormControllerTests.cs792,QfcQueue.cs505. All three shrank in this change; none crossed the limit because of it.AC11andAC12are in tension as authored; the code implements the only coherent reading.Related context only. Issue #427 tracks the broader duplicate-scoring report and stays open after this merge, because this change delivers only the scoped consumer-side portion of it. Issues #233 and #446 are already in a closed state and appear here only as precedent that this change preserves. None of these three is affected by merging this pull request.
GitHub Auto-close