fix(quickfiler): repair three QfcExplorerController latent defects and remove its coverage exclusion (#449)

[drmoisan]

fix(quickfiler): repair three QfcExplorerController latent defects and remove its coverage exclusion (#449)

Summary

• Removes ExplConvView_Cleanup() from IQfcExplorerController and deletes its NotImplementedException implementation, eliminating a runtime trap for the next caller of a public interface member.
• Fixes OpenQFItem so it reuses the constructor-captured _activeExplorer instead of re-resolving ActiveExplorer(), removing both a redundant COM round-trip and an internal-consistency hazard. Carried by a genuine fail-before/pass-after regression test.
• Deletes the dead 139-line #region Email Sorting To Rewrite block, whose six private/internal statics had a provably empty inbound call graph, and removes ten using directives orphaned by the deletions.
• Removes the class-level [ExcludeFromCodeCoverage], putting the file into the coverage denominator for the first time, behind a new injectable modal-dialog seam that makes both OpenQFItem branches testable headlessly.
• Adds 15 deterministic MSTest tests across two new files. QuickFiler package coverage moves 80.9163% to 80.9898% and the previously unmeasured class enters at 87.8261%.

Why

Three latent defects were found by reading during coverage research for issue #435, and none could be fixed there because that work's acceptance criteria forbid behavior changes. Recording them as prose inside a feature folder would have lost them at merge, so they were promoted to issue #449.

The coverage-exclusion removal is the substantive design decision (spec decision D5). The attribute was added on 2026-06-13 in commit a564add0d as part of the ratified COM/VSTO exemption, but it suppressed the whole class, including members that are testable once the one genuinely untestable dependency — a modal WinForms dialog — is placed behind a seam. Narrowing the attribute onto OpenQFItem was considered and rejected: the seam makes both of that method's branches reachable in a headless test, so narrowing would have left testable code unmeasured.

What Changed

Core fix — QuickFiler/Controllers/QfcExplorerController.cs (323 to 182 lines)

• NavigateToOutlookFolder(MailItem) assigns _activeExplorer.CurrentFolder rather than _globals.Ol.App.ActiveExplorer().CurrentFolder.
• The throwing ExplConvView_Cleanup() implementation is deleted.
• The #region Email Sorting To Rewrite block is deleted whole. Two further latent defects inside it — transposed Path.Combine arguments and a write into a null ref string[] — were deleted rather than fixed, since repairing unreachable code produces no observable effect.
• Adds NotInViewDialogInvoker, an internal settable delegate seam defaulting to MessageBox.Show. The dialog's text, buttons, and icon are byte-identical to before; only the invocation route changes.

Contract — QuickFiler/Interfaces/IQfcExplorerController.cs

• void ExplConvView_Cleanup(); removed. The interface has exactly one implementer, so the compiler (CS0535) enforces the paired edit.

Tests — two new files, 592 lines total

• QuickFiler.Test/Controllers/QfcExplorerControllerTests.cs (387 lines) and QfcExplorerController.ConversationViewTests.cs (205 lines). The split exists to keep every file under the 500-line cap; the combined file measured 569.
• QuickFiler.Test/QuickFiler.Test.csproj gains exactly two <Compile Include> entries in the Controllers item group.

Documentation and evidence

• spec.md acceptance criteria AC-1 through AC-16 checked off, 43 evidence artifacts, and the three feature-review audit artifacts.

Architecture / How It Fits Together

QfcExplorerController is an Outlook-Interop-bound controller reached from IFilerHomeController. It captures the active Explorer once in its constructor and was inconsistently mixing that captured reference with fresh ActiveExplorer() resolutions.

The seam follows the repository's existing settable-delegate idiom, demonstrated by QfcHomeController.QfcExplorerControllerLoader. It is declared with the fully-qualified type System.Func<...> rather than a bare Func<...> specifically so it does not resurrect the using System; directive the same change removes — the file already uses fully-qualified log4net.ILog and System.Reflection.MethodBase in the same style.

Tests reach the internal members through the existing [assembly: InternalsVisibleTo("QuickFiler.Test")] in QuickFiler/Properties/AssemblyInfo.cs.

Verification

Completed — full local C# toolchain, single clean pass

Stage	Command	Result
Format	dotnet tool run csharpier check .	EXIT 0, zero files needing formatting
Analyze	msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true	EXIT 0, zero Skipping target "CoreCompile" occurrences
Type-check	msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:TreatWarningsAsErrors=true	EXIT 0
Test	vstest.console.exe ... /EnableCodeCoverage /InIsolation /TestCaseFilter:"TestCategory!=LiveOutlook"	6452 passed, 0 failed, 0 skipped

/t:Rebuild is load-bearing: a warm /t:Build skips CoreCompile on every project and returns exit 0 having run no analyzers, so the gate could not fail. The analyzer evidence therefore asserts a zero count of Skipping target "CoreCompile" rather than a csc.exe count.

Coverage

Metric	Before	After
Repo-wide root line rate	85.3290%	85.3571%
QuickFiler package	80.9163%	80.9898%
QfcExplorerController	absent from report	87.8261% (101/115)
Changed-line coverage	—	100% (3/3)

The baseline value for the class is recorded as absent, not 0%: the class-level attribute suppressed it entirely, so it contributed no <class> element. The post-change figure aggregates four <class> elements, because OpenQFItem is async and the compiler emits its state machine and lambdas as separate elements; reading a single element would report a figure for a fragment of the file.

Determinism: the full suite was run twice consecutively with byte-identical pass sets, and the added tests contain no Thread.Sleep, Task.Delay, MessageBox.Show, temporary-file API, or Form construction.

Fail-before evidence: defect 2 has a real failing-then-passing pair (EXIT 1 with a verbatim Moq.MockException, then EXIT 0). Defects 1 and 3 carry fail-before-exception dossiers, because removing an uncalled member and deleting provably unreachable code present no observable behavior a test could assert both before and after.

CI note: this PR targets the epic integration branch. .github/workflows/ci.yml triggers pull_request only on [main, development], so no workflow fires and zero checks are expected. The absence of checks here is not a failure. The full local toolchain above is the green gate.

Recommended: reviewers wanting to reproduce should run the four commands above in order from a bootstrapped worktree.

Backward Compatibility / Migration Notes

• Breaking (internal): IQfcExplorerController.ExplConvView_Cleanup() is removed. The interface is internal to QuickFiler with exactly one implementer and no compiled caller, so no in-repo caller required updating. The removed member's legacy semantics are preserved verbatim in spec.md under ## Removed contract — legacy semantics for future restoration, so the behavior can be reimplemented deliberately rather than rediscovered.
• QuickFiler/Notes/notes_interfaces.cs declares a duplicate IQfcExplorerController still carrying the removed member. It is not compiled and is deliberately left inconsistent with the compiled contract; it is out of scope here.
• No public API outside QuickFiler changes. The user-visible dialog is unchanged.

Risks and Mitigations

Risk	Mitigation
The dead-region deletion removes code someone still wants	Every external caller of those six identifiers binds to independent copies in SortEmail.cs, EmailFiler.cs, and SortItemsToExistingFolder.cs, which carry their own tests. Verified by a zero-match search whose non-vacuity was checked (12 matches at merge-base, 0 now).
Removing [ExcludeFromCodeCoverage] drags coverage down	Measured, not assumed: the class enters at 87.83%, above the package average, so the package figure rose.
The seam changes dialog behavior	Only the invocation target changed; the four arguments are byte-identical. A gate asserts exactly one MessageBox.Show in the file and that it sits inside the seam's default initialiser.
Rollback	Three self-contained commits; git revert of 03b50117 restores prior behavior including the attribute.

Review Guide

1. QuickFiler/Interfaces/IQfcExplorerController.cs — one deleted line, establishes the contract change.
2. QuickFiler/Controllers/QfcExplorerController.cs — the substantive diff. The 139-line region deletion is mechanical bulk; the meaningful edits are the NavigateToOutlookFolder assignment target, the seam declaration, and the seam routing.
3. QuickFiler.Test/Controllers/*.cs — new tests (net-new, no prior file existed).
4. QuickFiler.Test/QuickFiler.Test.csproj — two added lines; see the shared-surface note below.
5. spec.md and evidence/ — verification trail, safe to skim.

Shared-surface note: QuickFiler.Test.csproj is co-owned with concurrent sibling child #491, which owns the Form1 compile region and the Form1.resx embedded resource. This PR's entire csproj diff is a single hunk at lines 117-123 in the Controllers group, deliberately placed 42 lines clear of the Form1 region to avoid a three-line merge-context collision. The Form1 regions are untouched.

Follow-ups

• Issue Bug: uithread-dispatcher-null-race-progresstrackerasync #584 was filed from this work: UiThread.Dispatcher exposes a null! backing field with no lazy initialization, and ProgressTrackerAsync.InitializeAsync() dereferences it on the next statement, producing a non-deterministic NullReferenceException under full-suite load. One such failure was observed during this change's QC run in UtilitiesCS.Test, a tree this PR does not touch. It was disclosed rather than suppressed — no test was modified, no retry added, no timing tolerance applied. Issue Bug: uithread-dispatcher-null-race-progresstrackerasync #584 also cross-references related issue Bug: uithread-dispatcher-static-swap-no-restore #493.
• Two unused using directives (System.Collections, System.Collections.Generic) remain at the head of QfcExplorerControllerTests.cs, stranded when the test file was split. No gate fires on them (CS8019 is hidden and IDE0005's analyzer is not wired into these non-SDK projects). Left for the next touch of the file rather than triggering a full toolchain re-run that would invalidate the audit artifacts.
• AC-12's "exactly one appended line" and AC-16's project-file figure of 485 read as one line / 485; the delivered state is two lines / 486 because the 500-line cap forced the test-file split. The supersession is pre-authorized by the spec's own split provision and evidenced in evidence/other/test-file-size.2026-08-22T09-16.md.

GitHub Auto-close

• None.

This PR targets epic/quickfiler-suite-determinism-foundation-integration, not the default branch, so a closing keyword would not fire on merge in any case. Issue #449 is referenced, not closed, and should be closed by the epic's final integration-to-main pull request.

• Refs Bug: quickfiler-explorer-controller-latent-defects #449
• Refs Bug: uithread-dispatcher-null-race-progresstrackerasync #584 (filed by this work; not closed by it)