Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ The TaskMaster worktree pins a repo-local .NET SDK and routes `dotnet` through a
- After install, invoke tools with `export PATH="$(pwd)/.dotnet-sdk:$PATH" DOTNET_ROOT="$(pwd)/.dotnet-sdk"` then `./.dotnet-sdk/dotnet tool run csharpier ...`.
- CSharpier is a local tool pinned at 1.2.6 (`dotnet-tools.json`). v1 uses SUBCOMMANDS: `csharpier check .` and `csharpier format .` (the old `--check`/bare-path v0 syntax is gone). A separate global csharpier (1.3.0) exists at `~/.dotnet/tools` — prefer the manifest version via `dotnet tool run`.

**Nullable gate quirk:** the CLAUDE.md policy command is `msbuild ... -t:Build -p:Nullable=enable -p:TreatWarningsAsErrors=true` and it passes clean (0/0) for first-party. Do NOT substitute `-t:Rebuild`: a forced Rebuild under those flags fails fast (~0.5s) with ~84 errors confined entirely to the two vendored/exempt projects (SVGControl, UtilitiesSwordfish.NET.General — CS8603/CS0649) before first-party even compiles. Those are pre-existing vendored issues outside the analyzer-stack scope, not a real gate failure. Use `-t:Build` as the policy gate; if you run a Rebuild for rigor, re-run a plain `-t:Build` afterward to restore the Debug build state before vstest.
**Nullable gate quirk:** the CLAUDE.md policy command is `msbuild ... -t:Build -p:Nullable=enable -p:TreatWarningsAsErrors=true` and it passes clean (0/0) for first-party ONLY as an up-to-date no-op (nothing recompiles). Do NOT substitute `-t:Rebuild`: a forced whole-solution Rebuild under those flags CLEANS then fails to rebuild the two vendored/exempt projects (SVGControl ~CS8603/CS0649, UtilitiesSwordfish.NET.General ~CS8625/CS8600) — 84 errors — which corrupts the Debug output state so subsequent incremental builds also try (and fail) to rebuild them. Building a single first-party .csproj with `-t:Rebuild` also drags in its vendored dependency (e.g. QuickFiler -> UtilitiesSwordfish) and fails the same way. Also note a standalone `.csproj` build needs `-p:Platform=AnyCPU` (no space); the solution maps `"Any CPU"` -> `AnyCPU` but a bare project build errors "BaseOutputPath/OutputPath not set" with the spaced form.

**Meaningful nullable gate on a touched legacy project (the correct recipe):** the legacy first-party projects (e.g. QuickFiler) are NOT nullable-annotated and produce a large pre-existing error population when genuinely recompiled under the nullable gate (QuickFiler ~540 unique errors; `-m` parallel double-reports so `grep -c` shows ~1080). The baseline "0 errors" only means nothing recompiled. To validate a change without the vendored noise: (1) restore all outputs with a plain analyzer `-t:Build` (non-nullable, rebuilds cleaned vendored projects to 0 errors); (2) `touch` only the changed source files; (3) run the solution `-t:Build` nullable gate — MSBuild recompiles only the touched projects (their sources are newer) and leaves up-to-date vendored/other projects skipped, so their pre-existing nullable debt does not surface. To PROVE a change adds zero new nullable diagnostics, capture a controlled pre-change count: `git stash push -- <changed .cs files>`, restore outputs, touch+nullable-build, count errors; then `git stash pop`. Identical pre/post error counts with every touched-file diagnostic merely line-shifted by the inserted-line count = no-regression proof. After any Rebuild/stash dance, finish with a plain analyzer `-t:Build` to restore Debug outputs before vstest.

**Pre-existing CS0618 in first-party:** UtilitiesCS/TaskMaster use obsolete IAsyncEnumerable `SelectAwait`/`WhereAwait`/`ForEachAwaitAsync` overloads (warning-only; not promoted under the analyzer `-t:Build` step which omits TWAE). An incremental baseline build may not re-emit them while a later full build does — explains baseline-vs-final warning-count deltas without any new diagnostic.

Expand Down
1 change: 1 addition & 0 deletions .claude/agent-memory/task-researcher/MEMORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@
- [qfc-item-controller-227-r2-denial](project_qfc_item_controller_227_r2_denial.md) — Issue #227: maintainer denied blanket exemption boundary (2026-07-01); precedent for per-member barrier analysis before any future coverage exemption
- [feedback-exemption-audit-check-proven-techniques](feedback_exemption_audit_check_proven_techniques.md) — when re-auditing exemption boundaries, grep for already-proven test techniques and check sibling-method consistency before accepting IRREDUCIBLE (issue #227 cycle-2 re-audit, 2026-07-02)
- [qfc227-headless-itemviewer-and-tlpcellsnapshot](project_qfc227_headless_itemviewer_and_tlpcellsnapshot.md) — Issue #227: headless ItemViewer construction confirmed safe (ProgressPane precedent); TlpCellSnapShot retrofit scoped; revised target 24 -> 19 (2026-07-02)
- [qfc-high-confidence-dual-pipeline](project_qfc_high_confidence_dual_pipeline.md) — QFC has two high-confidence filter pipelines (#169 live, #171 dead-in-prod); "subset shown" is batch-without-backfill, not a scoring bug (2026-07-03)
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
name: qfc-high-confidence-dual-pipeline
description: QuickFiler has two independent high-confidence filter implementations (#169 post-hoc removal, live; #171 pre-filter, dead in production) plus a fixed-batch-without-backfill pattern that explains "subset of items on navigation" reports
metadata:
type: project
---

QuickFiler high-confidence mode has two separately-implemented filtering pipelines:
1. Issue #169 post-hoc removal (`QfcFormController.Actions.cs`, `QfcCollectionController.RemoveBelowThresholdAsync`) -- the only path reachable from `QfcHomeController.RunAsync()`/`Run()`. Loads a fixed `ItemsPerIteration`-size batch unconditionally, scores during `LoadSecondaryAsync`, then strips below-threshold groups after the fact, with no backfill from later candidates.
2. Issue #171 pre-filter (`QfcHighConfidencePreFilter.cs`, `QfcHomeController.HighConfidencePreFilterLoader`, `LoadItemsAsync(IList<QfcPreScoredItem>)`) -- scores before building UI, only survivors get controllers. Confirmed (2026-07-03, via repo-wide grep for call sites) to have **zero production callers** -- only exercised in `QfcHomeControllerRunAsyncTests.cs` / `QfcHomeControllerIssue218Tests.cs` / `QfcFormControllerTests.cs`. Fully implemented and unit-tested but never wired into the live startup path.

Separately, subsequent screens draw from `_masterQueue`, gated by a live per-item score check at admission time (`QfcRemainingQueueAdmission.TryQueueAsync`), but `DequeueNextItemGroupAsync` returns however many items are actually queued (`TryTakeFirst(quantity)`, no padding) -- so any screen can legitimately show fewer than `ItemsPerIteration` items even when the >90% filter is working correctly.

**Why this matters**: reports of "QuickFiler high-confidence mode shows only a subset of items" are very likely explained by the fixed-batch-without-backfill pattern above, not a threshold-comparison bug (checked: `>=`/`<` boundaries are consistent and correct across `QfcHighConfidencePreFilter.FilterAsync`, `QfcRemainingQueueAdmission.TryQueueAsync`, and `RemoveBelowThresholdAsync`).

**How to apply**: before assuming a new high-confidence-mode bug report is a scoring defect, check (a) whether it's actually the dormant #171 pre-filter path being expected but not running, and (b) whether the visible count is explained by batch-size-then-filter with no backfill, before looking for score-calculation errors. Full research: `artifacts/research/2026-07-03T00-00-quickfiler-kbdactions-duplicate-key-research.md` (Investigation 2 section).

Related: [[project_qfc227_headless_itemviewer_and_tlpcellsnapshot]] (same QuickFiler controller family, different issue).
172 changes: 172 additions & 0 deletions QuickFiler.Test/Controllers/QfcCollectionControllerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -324,5 +324,177 @@ public void CarrierLoad_SetsPredeterminedFolderOnItemGroup()
.Be(@"\\Archive\Projects\Active");
group.MailItem.Should().BeSameAs(mail);
}

// ---- Navigation-key register/unregister on page swap (Issue #232) ----

/// <summary>
/// Builds an uninitialized controller wired to exercise the navigation-key swap without live
/// WinForms/COM state: a real KbdActions behind a Loose IQfcKeyboardHandler, Loose
/// IEmailMoveMonitor and IQfcFormViewer (L1v0L2L3v_TableLayout returns null), the outgoing
/// _itemGroups page, and _digits pre-set to 1. _digits = 1 is required because
/// GetUninitializedObject bypasses the field initializer; without it the Digits getter would
/// flip _digitRefreshNeeded and RegisterNavigation would enter the WinForms-bound SetVisualDigits path.
/// </summary>
private static QfcCollectionController CreateControllerForSwap(
int outgoingItemCount,
out KbdActions<string, KaStringAsync, Func<string, Task>> kbd
)
{
var controller = (QfcCollectionController)
FormatterServices.GetUninitializedObject(typeof(QfcCollectionController));

var kbdActions = new KbdActions<string, KaStringAsync, Func<string, Task>>();
kbd = kbdActions;
var kbdHandler = new Mock<IQfcKeyboardHandler>(MockBehavior.Loose);
kbdHandler.SetupGet(x => x.StringActionsAsync).Returns(() => kbdActions);

var moveMonitor = new Mock<IEmailMoveMonitor>(MockBehavior.Loose);

var formViewer = new Mock<IQfcFormViewer>(MockBehavior.Loose);
formViewer
.SetupGet(x => x.L1v0L2L3v_TableLayout)
.Returns((System.Windows.Forms.TableLayoutPanel)null);

SetControllerField(controller, "_kbdHandler", kbdHandler.Object);
SetControllerField(controller, "_moveMonitor", moveMonitor.Object);
SetControllerField(controller, "_formViewer", formViewer.Object);
SetControllerField(controller, "_digits", 1);
SetControllerField(controller, "_itemGroups", MakeGroups(outgoingItemCount));

return controller;
}

/// <summary>Builds a list of <c>count</c> minimal item groups, each carrying a mock mail item.</summary>
private static List<QfcItemGroup> MakeGroups(int count)
{
var groups = new List<QfcItemGroup>();
for (int i = 0; i < count; i++)
{
var mail = new Mock<MailItem>(MockBehavior.Loose);
mail.SetupGet(x => x.EntryID).Returns($"entry-{i}");
groups.Add(new QfcItemGroup { MailItem = mail.Object });
}
return groups;
}

private static void SetControllerField(object target, string name, object value) =>
typeof(QfcCollectionController)
.GetField(name, BindingFlags.NonPublic | BindingFlags.Instance)
?.SetValue(target, value);

/// <summary>Seeds one <c>"Collection"</c>-sourced entry for <paramref name="key"/>.</summary>
private static void SeedCollectionKey(
KbdActions<string, KaStringAsync, Func<string, Task>> kbd,
string key
) => kbd.Add("Collection", key, _ => Task.CompletedTask);

/// <summary>Counts the <c>"Collection"</c>-sourced entries registered for <paramref name="key"/>.</summary>
private static int CountCollectionKey(
KbdActions<string, KaStringAsync, Func<string, Task>> kbd,
string key
) => kbd.Count(a => a.SourceId == "Collection" && a.Key == key);

/// <summary>
/// [P1-T1] Reported reproduction (Issue #232). A 1-item outgoing page has its navigation key
/// "1" registered plus an orphaned "2" left behind by an earlier page abandoned through the
/// pre-fix defective swap path. Swapping in a cached 2-item page walks keys "1" and "2", so
/// "2" collides with the orphan. Pre-fix, <c>LoadControlsAndHandlers_01</c> performs no
/// navigation registration at all, so the call does not throw and this expect-fail assertion
/// fails (the collision is not reproduced at this call boundary). Post-fix the swap routes
/// through <c>SwapItemGroups</c>, which now unregisters the outgoing page and re-registers the
/// incoming page; adding key "2" surfaces the documented <see cref="ArgumentException"/>,
/// proving navigation registration now occurs during the swap.
/// </summary>
[TestMethod]
public void LoadControlsAndHandlers_01_ReportedRepro_SwapToOverlappingCachedPage_ThrowsBeforeFix()
{
// Arrange
var controller = CreateControllerForSwap(outgoingItemCount: 1, out var kbd);
SeedCollectionKey(kbd, "1");
SeedCollectionKey(kbd, "2");
var cachedTwoItemPage = MakeGroups(2);

// Act
System.Action act = () =>
controller.LoadControlsAndHandlers_01(null, cachedTwoItemPage);

// Assert
act.Should().Throw<ArgumentException>().WithMessage("*Key 2 SourceId Collection*");
}

/// <summary>
/// [P3-T1] (AC1) A page swap unregisters every outgoing "Collection" key and registers exactly
/// the incoming page's keys.
/// </summary>
[TestMethod]
public void LoadControlsAndHandlers_01_SwapsPage_RemovesOutgoingKeysAndAddsIncomingKeys()
{
// Arrange: 2-item outgoing page with keys "1" and "2" registered.
var controller = CreateControllerForSwap(outgoingItemCount: 2, out var kbd);
SeedCollectionKey(kbd, "1");
SeedCollectionKey(kbd, "2");
var oneItemIncomingPage = MakeGroups(1);

// Act
controller.LoadControlsAndHandlers_01(null, oneItemIncomingPage);

// Assert: no stale outgoing key remains; exactly one incoming key "1".
CountCollectionKey(kbd, "2").Should().Be(0);
CountCollectionKey(kbd, "1").Should().Be(1);
kbd.Count(a => a.SourceId == "Collection").Should().Be(1);
}

/// <summary>
/// [P3-T3] KbdActions.Add throws on a duplicate key; registering the same page twice without an
/// intervening unregister triggers the collision the Phase 2 guard exists to avoid.
/// </summary>
[TestMethod]
public void RegisterNavigation_CalledTwiceWithoutInterveningUnregister_ThrowsArgumentException()
{
// Arrange: 2-item page, no keys registered yet.
var controller = CreateControllerForSwap(outgoingItemCount: 2, out _);

// Act: first registration succeeds; the second re-adds the same keys.
controller.RegisterNavigation();
System.Action secondRegister = () => controller.RegisterNavigation();

// Assert
secondRegister
.Should()
.Throw<ArgumentException>()
.WithMessage("*SourceId Collection*");
}

/// <summary>
/// [P3-T4] (AC3) The guarded zero-item flow (unregister outgoing, drop its item, then swap in a
/// cached page) leaves exactly one "Collection" entry per incoming key and throws nothing,
/// confirming the production guard's effect (skipping the redundant trailing register).
/// </summary>
[TestMethod]
public void SwapItemGroups_ThenSkipGuardedTrailingRegister_LeavesExactlyOneEntryPerIncomingKey()
{
// Arrange: 1-item outgoing page with key "1" registered.
var controller = CreateControllerForSwap(outgoingItemCount: 1, out var kbd);
SeedCollectionKey(kbd, "1");
var twoItemCachedPage = MakeGroups(2);

// Act: unregister the outgoing page, drop its item, then swap in the cached page.
controller.UnregisterNavigation();
GetItemGroups(controller).RemoveAt(0);
System.Action act = () =>
controller.LoadControlsAndHandlers_01(null, twoItemCachedPage);

// Assert: no exception; exactly one entry per incoming key, no duplicates.
act.Should().NotThrow();
CountCollectionKey(kbd, "1").Should().Be(1);
CountCollectionKey(kbd, "2").Should().Be(1);
kbd.Count(a => a.SourceId == "Collection").Should().Be(2);
}

private static List<QfcItemGroup> GetItemGroups(QfcCollectionController controller) =>
(List<QfcItemGroup>)
typeof(QfcCollectionController)
.GetField("_itemGroups", BindingFlags.NonPublic | BindingFlags.Instance)
.GetValue(controller);
}
}
16 changes: 14 additions & 2 deletions QuickFiler/Controllers/QfcCollectionController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,10 @@ public void LoadControlsAndHandlers_01(TableLayoutPanel tlp, List<QfcItemGroup>
);
_formViewer.SuspendLayout();
ActivateQueuedTlp(tlp);
ActivateQueuedItemGroups(itemGroups);
// Route the item-groups swap through SwapItemGroups so the outgoing page's "Collection"
// navigation keys are unregistered and the incoming page's keys are registered as part of
// the swap (Issue #232). Calling ActivateQueuedItemGroups directly left stale keys behind.
SwapItemGroups(itemGroups);
_formViewer.ResumeLayout();
ActiveIndex = -1;
}
Expand Down Expand Up @@ -1200,13 +1203,19 @@ await _itemGroups[ActiveIndex]
await _kbdHandler.ToggleKeyboardDialogAsync();
}

// Guards against double-registration: when the zero-item branch skips to the next page,
// SkipGroupAsync -> LoadControlsAndHandlers_01 -> SwapItemGroups already registers the
// incoming page's navigation keys. Registering again below would re-add the same keys and
// throw ArgumentException from KbdActions.Add (Issue #232).
bool swapAlreadyRegistered = false;
await UiThread.Dispatcher.InvokeAsync(async () =>
{
TlpLayout = tlpState;
ResetPanelHeight();
if (_itemGroups.Count == 0)
{
await ((QfcFormController)_parent).SkipGroupAsync();
swapAlreadyRegistered = true;
//_parent.ActionOkAsync();
}
});
Expand All @@ -1216,7 +1225,10 @@ await UiThread.Dispatcher.InvokeAsync(async () =>
"RemoveSpecificControlGroupAsync: Counter is greater than 1. Race Condition Exists"
);
}
RegisterNavigation();
if (!swapAlreadyRegistered)
{
RegisterNavigation();
}
Interlocked.Decrement(ref removespecificcontrolgroupcounter);
}

Expand Down
4 changes: 4 additions & 0 deletions QuickFiler/Controllers/QfcDatamodel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,10 @@ CancellationToken cancel
var score = await scoringService
.ScoreAsync(mailItem, _globals, cancel)
.ConfigureAwait(false);
logger.Debug(
$"Probability debug [QfcDatamodel.ScoreRemainingQueueMailItemAsync (master-queue admission)] "
+ $"Subject='{mailItem.Subject}' EntryID='{mailItem.EntryID}' Score={score.Score}"
);
return score.Score;
}

Expand Down
Loading
Loading