Skip to content

fix(efc): close eight EFC controller-surface defects (#459, #460, #461, #463, #464, #465, #466, #467) - #661

Merged
drmoisan merged 17 commits into
epic/quickfiler-bug-family-integrationfrom
bug/efc-controller-surface-defects-464
Aug 28, 2026
Merged

fix(efc): close eight EFC controller-surface defects (#459, #460, #461, #463, #464, #465, #466, #467)#661
drmoisan merged 17 commits into
epic/quickfiler-bug-family-integrationfrom
bug/efc-controller-surface-defects-464

Conversation

@drmoisan

Copy link
Copy Markdown
Owner

fix(efc): close eight EFC controller-surface defects (#459, #460, #461, #463, #464, #465, #466, #467)

Epic child of quickfiler-bug-family. Base is epic/quickfiler-bug-family-integration, not main.

What this changes

Eight defects across the QuickFiler EFC (Email Folder Chooser) controller surface, grouped by root
cause rather than by issue, so one remedy closes several issues where they share a cause.

Issue Defect Remedy class
#459 KbdActions<> contract misuse in EfcItemController DELETION
#460 Cleanup() NRE on a partially-constructed controller, and an undisposed timer GUARD + CORRECTION
#461 Dead conversation-expanded handler bound to a name the publisher never raises DELETION
#463 WebView2 --incognito argument carrying a non-ASCII EN DASH CORRECTION + DELETION
#464 Null-guard gaps and faults escaping async void boundaries GUARD + CORRECTION
#465 Form-controller lifecycle and selection defects GUARD + CORRECTION
#466 Dead code carrying latent NRE traps DELETION
#467 ProcessCmdKey swallowing Alt mnemonics GUARD

Production diff (9 owned paths + 3 wholesale deletions)

166  50   QuickFiler/Controllers/EfcFormController.cs
108 161   QuickFiler/Controllers/EfcItemController.cs
  1   1   QuickFiler/Controllers/QfcItemController.ViewerSetup.cs
 20  13   QuickFiler/Viewers/EfcViewer.cs
  0 510   QuickFiler/Viewers/EfcViewer3.Designer.cs   (deleted)
  0  88   QuickFiler/Viewers/EfcViewer3.cs            (deleted)
  0 120   QuickFiler/Viewers/EfcViewer3.resx          (deleted)
317   0   QuickFiler.Test/Controllers/EfcFormControllerTests.cs
260   0   QuickFiler.Test/Controllers/EfcItemController.CleanupTests.cs
470   0   QuickFiler.Test/Controllers/EfcItemControllerTests.cs
164   0   QuickFiler.Test/Controllers/EfcViewerTests.cs
  3   0   QuickFiler.Test/QuickFiler.Test.csproj

17 commits; 128 files changed overall, of which 116 are documentation and evidence.

Notable remedies

Verification

Full toolchain in the mandated order, /t:Rebuild throughout, 0 loop restarts:

Stage Result
dotnet tool run csharpier check . EXIT 0 — 1549 files, 0 unformatted
Analyzer /t:Rebuild (EnableNETAnalyzers + EnforceCodeStyleInBuild) EXIT 0 — 0 errors, 5 warnings, identical to baseline
Nullable /t:Rebuild (TreatWarningsAsErrors, no /p:Nullable=enable) EXIT 0 — 0 errors
vstest.console.exe QuickFiler.Test /EnableCodeCoverage /InIsolation EXIT 0 — 1169 / 1169 passed, 0 failed

Non-vacuity proven: both MSBuild logs contain 0 lines matching Skipping target "CoreCompile"
against 36 csc.exe invocations. A warm /t:Build would have exited 0 having compiled nothing, so
this is asserted rather than assumed.

Test-count arithmetic: 1099 baseline + 26 merged-sibling results + 44 added by this feature = 1169.

Coverage — reported honestly, with its denominator named

Both figures use the unfiltered whole-run first-party repository-wide denominator over the same 9
assemblies.

Baseline Post-change
line-rate 70.32% (57714/82070) 85.25% (54667/64124)
branch-rate 59.12% 79.19%

The no-regression requirement holds. This is not a claimed 15-point improvement. lines-valid
differs between the two runs by 17,946 against a production diff of roughly 150 lines, so the movement
is dominated by what was measured, not by tests added. The baseline Cobertura was not retained, so its
composition cannot be recovered; that limitation is recorded rather than papered over.

Review outcome

feature-review produced all three audit artifacts and returned 0 Blocking findings, independently
substantiating 74 of 74 acceptance criteria — 72 by direct re-verification, 2 resting on committed
evidence with the non-vacuity proof above.

Six non-blocking findings are recorded, the material ones being: the new-code 90% floor is unmet for 6 of
11 measured members (all COM/dialog-bound); new members in EfcItemController and EfcViewer are
unmeasured under pre-existing class-level [ExcludeFromCodeCoverage]; and seven follow-ups remain to
be promoted.

Boundaries held

This feature owns the Efc* prefix only. Verified against the base:

  • QuickFiler/QuickFiler.csprojzero-line diff (the deleted EfcViewer3 files were orphans, never
    referenced, which is why no project-file edit was needed)
  • QuickFiler.Test/QuickFiler.Test.csproj — 3 added / 0 deleted, all Controllers\Efc*; feature Bug: kbdactions-enumerable-ctor-bypasses-duplicate-guard #444's
    eight QfcCollectionController* entries and Bug: uithread-dispatcher-static-swap-no-restore #493's two QfcItemController.UiThreadDispatcherFixture*
    entries preserved exactly
  • Zero-line diffs on EfcSelectionGuard.cs, BreadcrumbRowBuilder.cs, KeyboardHandler.cs,
    QfcFormViewer.cs, QfcFormKeyHandler.cs, and everything under QuickFiler/Interfaces/
  • QfcItemController.ViewerSetup.cs — exactly 1 added / 1 deleted, the incognito literal alone
  • EfcFormController.cs 1189 lines against a 1193 gate; EfcItemController.cs 1117, down from 1170
  • EfcFormController.cs new WebView2BreadcrumbHost(...) construction byte-identical, as merged
    feature Bug: webview2breadcrumbhost-unmarshalled-sdk-call-and-unsynchronized-state #476 depends on it
  • Zero [ExcludeFromCodeCoverage] attributes added in the .cs-scoped diff

Deliberately not fixed — RC7 residual, promoted instead

EfcSelectionGuard.BannerPrefix is "===" (three =) while both row producers use "====" — a third
arity variant — and a comment near EfcFormController.cs:325 describes a "====" rejection the guard
does not implement.

EfcSelectionGuard.cs belongs to merged sibling #614 and is outside this feature's owned set.
Widening its prefix would have relaxed a filing guard #614 deliberately tightened, making a three-=
row filable, in a file this feature does not own — and it would have passed tests, because
EfcSelectionGuardTests.cs asserts only on four-= banners. RC7 was therefore delivered additively:
IsBannerRow consumes BreadcrumbRowBuilder.BannerPrefix by direct symbol reference, and
ActionOkAsync composes it with EfcSelectionGuard.IsValidFilingSelection so #614's rooted-path
rejection survives intact. Review independently confirmed this was the correct call.

Known non-blocking conditions

  • Fifteen load-driven failures in three QfcItemController.* test files (owned by a concurrently-live
    sibling) appear only under aggregate instrumented load and pass in isolation. Both final runs are
    green: 1169/1169 isolated and 6789/6789 aggregate. Not this feature's defects and not remediated here.
  • One manual check is recorded as MANUAL_CHECK_DEFERRED, not as a pass: confirming Alt+F / Alt+M
    physically open the menus needs a live Outlook host, which the test policy forbids substituting. Five
    passing ClaimsAltChord tests prove EfcViewer no longer claims those chords; that is not the same
    claim, and it is not reported as one.

Issue linkage

This PR targets epic/quickfiler-bug-family-integration, not the default branch. GitHub registers
closing references only for PRs targeting the default branch, so no closing keyword is used here and
these issues will not auto-close on merge. All eight were confirmed OPEN via gh issue view at
authoring time and are closed out through the epic's integration-to-main PR.

Addresses #459, #460, #461, #463, #464, #465, #466, #467.

drmoisan and others added 17 commits August 27, 2026 17:04
…tion base

The approved plan was authored at 49199503 and asserts the branch point
carries neither #484 nor #444. The actual base, 69e8317, carries both
(PR #619 and PR #654), and merged feature #614 has since centralized the
two RC7 banner-classification call sites into EfcSelectionGuard.

The addendum re-reads every locator against the real base, corrects the
five that moved, and fixes the RC7 delivery shape so that #465 D's four
criteria are met additively without relaxing #614's merged filing guard.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T45irz1DRk6bPnxx5ifqFA
…d 444

Both dependencies are merged into the execution base, so their design
decisions bind this feature. The briefing records the three 484 downstream
notes addressed to 464 by name, the KbdActions<> contract 444 declared
'required by features 464 and 489' (including the new enumerable-constructor
duplicate guard that is live on EfcFormController's three seed sites), 444's
explicit statement that CharActions is reached only from the Alt-key
ProcessCmdKey path (which constrains the #467 guard), the established
fault-boundary shapes RC1 and RC3 must mirror rather than reinvent, and the
test conventions to reuse.

Also corrects the base-drift addendum's attribution of MoveFailureNotifier
to #444; it is a #484 member.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Records the full Phase 0 evidence set for the efc-controller-surface-defects
feature: policy and feature-document read proofs, repository state, resolved
MSBuild and vstest paths, SDK and package bootstrap, and the five baseline
measurements (csharpier, analyzer build, nullable build, QuickFiler.Test run,
repository-wide coverage) plus the structural inventories.

Headline baselines: 0 analyzer errors / 5 identifier-less warnings, 0 nullable
errors, QuickFiler.Test 1099/1099 passing with an empty BASELINE_FAILED set,
and repository-wide coverage at line-rate 0.7032 / branch-rate 0.5912.

Three locator discrepancies against the plan are recorded rather than silently
absorbed, all three predicted by the base-drift addendum: EfcFormController.cs
is 1073 lines (not 1084), QfcItemController.ViewerSetup.cs is 499 (not 430),
and the QuickFiler.Test.csproj insertion anchor is at :117 (not :112).

Raw Cobertura XML is deliberately not committed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… B/C)

Deletes seven dead members from QuickFiler/Controllers/EfcItemController.cs,
each backed by an individually verified zero-call-site finding recorded in the
Phase 0 deletion inventory:

- both synchronous ToggleExpansion overloads, in one edit. They were the sole
  writers of the 'B'/'D' CharActions entries and the sole home of the two
  genuine async void lambdas, so this closes #459 B, #459 C and #464 D by
  removal rather than by repair.
- RegisterActions, which misused the KbdActions<> indexer setter (#459 A).
- InitializeWebView(), which carries #463's dead third EN DASH site; the
  literal is removed with its container rather than edited in place.
- the seven-parameter constructor (#466 C).
- the _selectorsCtrls field (#466 B); both SetupThemes call sites now pass an
  explicit null with a comment, which is behaviour-identical because the field
  was declared null and never assigned.

QuickFiler/Controllers/KbdActions.cs and QuickFiler/QuickFiler.csproj are
untouched: the KbdActions<> contract is documented in spec.md §RC4 instead, and
the deleted members carry no project-file entry.

Adds three test files wired through three new Efc* Compile Include entries in
QuickFiler.Test.csproj, a three-line insertion that reorders nothing and
preserves the file's CRLF endings and BOM. Six regression tests: five
type-metadata absence assertions that were red before the deletions and green
after, plus one that pins the surviving asynchronous expansion path.

EfcItemController.cs goes from 1170 to 1054 lines.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
 A/D)

Deletes EfcViewer._formController, EfcViewer.SetController, and the viewer-side
EfcViewer.EditFiltersMenuItem_Click. EfcFormController never calls
SetController, unlike its QuickFiler twin, so _formController is permanently
null and the viewer-side handler would throw the moment it ran. It is
unreachable today only because EfcViewer.Designer.cs never wires
EditFiltersMenuItem.Click; a routine Designer regeneration would arm it.
Removing the three members disarms that trap without changing behaviour.

The Edit Filters command is NOT broken and is not repaired here. It is wired by
EfcFormController.WireEventHandlers at :398 directly to the controller's own
EditFiltersMenuItem_Click at :559, bypassing the viewer. That subscription is
verified byte-identical to its pre-change text, its target is pinned by a named
test that is green both before and after the deletion, and
QuickFiler/Viewers/EfcViewer.Designer.cs shows an empty diff.

Also deletes the three orphaned QuickFiler/Viewers/EfcViewer3.* files. They
carried no Compile Include, EmbeddedResource or DependentUpon entry, so
QuickFiler/QuickFiler.csproj needs no edit and stays untouched, which removes
all contention with the concurrent breadcrumb feature. A repository-wide search
for the token EfcViewer3 now returns zero files.

A full solution build exits 0 after all Phase 1 and Phase 2 deletions, which is
the compiler backstop for the zero-call-site findings: every removed member is
internal or private, so any surviving caller would be a compile error.

EfcViewer.cs goes from 162 to 150 lines.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
EfcItemController.ConversationResolverPropertyChanged guarded on
nameof(_dataModel.ConversationResolver.ConversationInfo.Expanded), which
compiles to the literal "Expanded". ConversationResolver raises exactly four
names -- "ConversationInfo", "ConversationItems", "Df" and "UpdateUI" -- and
never "Expanded", so the subscription fired on every background conversation
load while the handler body never executed.

The remedy is removal, not retargeting. Retargeting the guard to a name the
resolver does raise would run a second SetObjects and Sort in addition to the
existing UpdateUI dispatch, and would re-enter the lazy ConversationInfo getter
that the publisher documents itself as deliberately bypassing -- converting a
dead-but-harmless member into a live performance regression.

Removal costs nothing because the intended behaviour is already delivered by a
different, live route: PopulateConversation assigns SetTopicThread to
ConversationResolver.UpdateUI, and the resolver invokes that delegate on the UI
thread after loading. A named test now pins that route, so a future regression
that removes it fails loudly. Deleting the handler also discharges one genuine
async void member that the later fault-boundary work would otherwise have had
to wrap.

Both fixed-string searches the acceptance criteria name now return zero matches
against a pre-change count of one each. The four sibling event attachments in
WireEventHandlers are untouched.

EfcItemController.cs goes from 1054 to 1034 lines.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…tes (#463)

The additional-browser-arguments literal was "-incognito " with a leading
U+2013 EN DASH rather than two ASCII hyphen-minus characters. Chromium
introduces command-line switches with two ASCII hyphens and passes
CoreWebView2EnvironmentOptions.AdditionalBrowserArguments through verbatim, so
the unrecognised token was discarded silently and the item preview retained
browsing data.

Two live sites, corrected differently because their testability differs:

- EfcItemController: the literal is hoisted to
  internal const string IncognitoArgument, which InitializeWebViewAsync now
  passes. The constant is the assertion target -- the enclosing member needs the
  real WebView2 runtime and cannot execute under the unit-test policy, so a
  direct assertion against the value the program uses is the only instrument.
  Per the plan's defect-preserving rule the constant was introduced first with
  the EN DASH intact, the test was observed red with the TRX bytes identifying
  the actual leading character as E2 80 93 (U+2013), and only then was the
  value corrected.

- QfcItemController.ViewerSetup.cs: corrected in place as exactly one changed
  line, git diff --numstat reporting 1 added and 1 deleted. A hoisted constant
  is not available there because this feature's diff over that sibling-owned
  file is constrained to a single line. Byte comparison confirms the only
  change is E2 80 93 becoming 2D 2D; the trailing space, the indentation and
  the CRLF terminator are byte-identical, and the file is still 499 lines.

The third site, inside the dead InitializeWebView(), was removed with its
container in the earlier dead-code phase rather than edited.

Two locators in the plan were stale and are recorded as such in the evidence:
the QFC literal is at line 61, not 55, and that file is 499 lines, not 430.
Both are the merged-#484 drift the base-drift addendum predicted, and both mean
the two edits are sequential rather than concurrent, so the textual-conflict
risk is lower than the plan rates it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ration' into bug/efc-controller-surface-defects-464
Re-runs the mandated four-step toolchain after the orchestrator merged the
moved base (siblings 476/501, PRs #658/#659/#660) into this branch.

- csharpier check: EXIT 0, 1549 files, zero diffs.
- analyzer gate (/t:Rebuild): EXIT 0, 0 errors / 5 identifier-less warnings,
  matching the Phase 0 baseline; zero CoreCompile skips.
- nullable gate (/t:Rebuild): EXIT 0, 0 errors; zero CoreCompile skips.
- QuickFiler.Test: 1137 executed, 1123 passed, 14 failed. All 14 are
  load-driven timeouts in three QfcItemController.* files outside this
  feature's owned set; all 15 pass in an isolated scoped re-run. Classified
  base-introduced and not remediated here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…posal

Phase 5 of the atomic plan, [P5-T1] through [P5-T27].

Production (EfcFormController.cs, EfcItemController.cs):
- Both Cleanup() methods are now callable on a partially constructed
  controller and idempotent. Every event detach precedes the nulling of the
  field it detaches from (constraint C6 item 1).
- EfcFormController.Cleanup() captures and clears _parentCleanup before
  invoking it, so the parent teardown callback runs exactly once.
- EfcItemController.Cleanup() replaces the unconditional Buttons.ForEach with
  a null-tested iteration, adds _buttons = null, drops the duplicate
  _itemViewer assignment, and disposes the timer before nulling it
  (constraint C6 item 2). The 484 swallow-and-continue shape is not copied.
- The five theme/dark-mode accessors across both controllers test at the call
  site, so the params object[] dependency array is never materialised on the
  torn-down path and the strict dependency check is never reached with a null
  _themes.
- EfcItemController.Subject now reads the cached _itemInfo model like Sender
  and To; all three are null-safe after teardown.
- ApplyReadEmailFormat gains a null-collaborator early return that does not
  log, following the 484 post-teardown timer-callback shape. Its signature is
  unchanged, so the one surviving arming site still binds.

Tests (13 new, MSTest + Moq + FluentAssertions, no Form, no sleep, no wait):
- EfcFormControllerTests.cs: 5 RC1 accessor and lifecycle tests.
- EfcItemController.CleanupTests.cs: 8 RC1/RC2 tests, including the
  deterministic timer test armed with Timeout.Infinite for both due time and
  period and observing disposal through ObjectDisposedException.

Evidence: fail-before 5/5 and 8/8 red (rc1-form-fail.md, rc1-item-fail.md);
pass-after 13/13 green (rc1-pass.md); delivered statement order recorded
against constraint C6 (cleanup-statement-order.md).

Sizes: EfcFormController.cs 1107 (gate 1193), EfcItemController.cs 1100
(gate < 1170), both new/extended test files under 500.

spec.md: 14 acceptance criteria checked off against recorded evidence.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…row trace

Phase 6 of the atomic plan, [P6-T1] through [P6-T21].

#464 B — five async void rims:
- Adds internal Action<string, Exception> BoundaryErrorSink to
  EfcFormController, defaulting to exactly one logger.Error(message,
  exception) call on the pre-existing static logger. It follows 484's
  MoveFailureNotifier seam-and-default shape.
- Extracts each of ButtonCancel_Click, ButtonOK_Click, ButtonRefresh_Click,
  ButtonCreate_Click and ButtonDelete_Click into an internal async Task
  member, leaving a one-line await wrapper, then removes the throw; from each
  catch. At an async void rim the boundary logs and does not rethrow, per the
  in-repo pattern 444 established in KeyboardHandler_KeyDownAsync.
- Fixed-string count of throw; in EfcFormController.cs: 5 -> 0.

#464 C — fire-and-forget PopulateFolderCombobox:
- Wraps the body in a try whose catch logs through the sink and returns,
  mirroring InitializeBreadcrumbHostAsync. The pre-existing null-viewer early
  return stays inside the try and stays first, so the existing assertion is
  not weakened. Both call sites are unchanged.

#464 E — stack-trace-resetting rethrow:
- Extracts ThrowInitializationFailure(System.Exception) as internal static and
  reduces the handler failure branch to a one-line adapter, then replaces the
  plain rethrow with an ExceptionDispatchInfo capture-and-throw so the
  originating frame survives.
- Fixed-string count of throw (e.InitializationException): 1 -> 0.

Tests (9 new results): a five-row [DataTestMethod] over the extracted
boundary members, a default-sink coverage test, the PopulateFolderCombobox
fault test, and the stack-trace preservation test.

Evidence: fail-before 5/5, 1/1 and 1/1 red; pass-after 12/12 green.

Recorded deviations, both in the evidence rather than absorbed:
- [P6-T14] observed 12 results, not the plan's projected 9: vstest's ~
  operator is case-insensitive and also matched three pre-existing
  PopulateFolderComboBox tests in QfcItemController_FolderHandlingTests, a
  class outside this feature's owned set. Those three also pass.
- Plan decision D10's premise that QuickFiler.Test carries no log4net
  reference is FALSE on this base: merged sibling 33bcd21 (#614) added
  QuickFiler.Test.csproj:214-215. D10's instruction still stands and the sink
  is the instrument spec.md:959 names, so no implementation change was made,
  but the stale rationale is recorded in rc3-boundary-structure.md.

Sizes: EfcFormController.cs 1145 (gate 1193), EfcItemController.cs 1117
(gate < 1170), test files 405 and 470.

spec.md: 6 acceptance criteria checked off against recorded evidence.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 7 of the atomic plan, [P7-T1] through [P7-T23].

RC8 (#465 B) — cross-thread control read:
- Adds internal static MatchesForSearchText(Func<string,string[]>, string),
  a pure helper tolerant of a null delegate, null search text and null
  result.
- RefreshSuggestionsAsync now reads _formViewer.SearchText.Text into a local
  as its first statement, on the UI thread; the lambda closes over the local.
  No member access on _formViewer remains inside either Task.Run lambda.
  Measured: the _formViewer-to-first-Task.Run( offset flips from +2 at the
  merge base (:799 against :797) to -2 delivered (:879 against :881).

RC9 (#465 C) — accumulating trash row:
- Adds TrashRowText, an idempotent WithTrashRow, and ApplyDeleteGesture;
  ActionDeleteAsync reduces to the UI-thread await plus one call.
- BindFolderRows becomes presentation-only: the _folderRows write-back and
  the field read are both gone, and it passes its own rows parameter to
  BindBreadcrumbRowsAsync. Retention moves to a new BindSourceFolderRows,
  called from the three source sites; the delete site keeps BindFolderRows.
  _folderRows is now assigned in exactly two method bodies plus its retained
  declaration initializer.

RC7 (#465 D) — banner-row classification, delivered additively:
- Adds IsBannerRow (StartsWith over BreadcrumbRowBuilder.BannerPrefix under
  StringComparison.Ordinal, never Substring, so a short row cannot throw) and
  IsSelectableFolder. IsValidSelection becomes IsSelectableFolder(...), and
  ActionOkAsync's guard composes IsBannerRow with
  EfcSelectionGuard.IsValidFilingSelection so #614's rooted-path rejection
  survives. EfcSelectionGuard.cs and BreadcrumbRowBuilder.cs both show a
  zero-line diff against the base.

Tests: 6 new (MatchesForSearchText, WithTrashRow, ActionDeleteAsync driven
twice through its literal instrument, and three classification tests).
Evidence: fail-before 2/2 and 2-of-3 red; pass-after 7/7 green, including the
pre-existing Issue439 breadcrumb bind-boundary test.

Recorded deviations:
- The plan's [P7-T9] prose describes the third RC7 test as asserting
  IsSelectableFolder == !IsBannerRow. That is unsatisfiable on this base and
  is not what spec.md:977 requires; the test asserts the criterion's actual
  property, identical classification at the two EFC sites. Recorded in
  465d-fail.md.
- Constraint C2's overflow rule triggered at 492 lines. Resolved by prose
  compression, not the [DataTestMethod] consolidation the rule names, because
  every near-identical group in the file is individually required by name. No
  fifth test file and no fourth Compile Include entry. Recorded in
  file-sizes-after-p7.md.
- Residual reported, not fixed: EfcSelectionGuard.BannerPrefix remains a
  third arity variant (three '='), and the EfcFormController.cs:325 comment
  remains inaccurate. Both are outside this feature's owned set.

RC8 has no behavioural fail-before and cannot have one; the exception dossier
fail-before-exception.2026-08-28T01-26.md records why and cites the
structural sign flip.

Sizes: EfcFormController.cs 1189 (gate 1193, true merge base 1073, +116 of a
120-line allowance), EfcItemController.cs 1117 (gate < 1170), test files 485,
470, 260, 104.

spec.md: 8 acceptance criteria checked off against recorded evidence.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 8 of the atomic plan, [P8-T1] through [P8-T12].

EfcViewer.ProcessCmdKey claimed every Alt-modified chord, because its guard
tested only handler != null && keyData.HasFlag(Keys.Alt). Any Alt-plus-key
chord is a WinForms mnemonic and was swallowed before base.ProcessCmdKey
could route it, so Alt+F ("&Filters") and Alt+M ("&Move Options") never
opened their menus.

- Extracts internal static ClaimsAltChord(IQfcKeyboardHandler, Keys) and
  routes ProcessCmdKey through it, first in defect-preserving form.
- Narrows the predicate to claim a chord only when the handler is non-null,
  the key data carries Keys.Alt, and the key-code portion (masked with
  Keys.KeyCode) is Keys.Menu or Keys.None. Every other chord now reaches
  base.ProcessCmdKey.

The claim the keyboard handler actually services is bare Alt:
ToggleKeyboardDialogAsync never inspects the key data. The guard is scoped
to EfcViewer and narrows only what EfcViewer claims, so 444's Alt-mnemonic
ProcessCmdKey -> CharActions route stays reachable; bare Alt still returns
true. KeyboardHandler.cs is owned by #498 and was not edited. The QFC twin
shares the defect and is deliberately not changed.

Tests: five ClaimsAltChord predicate tests in EfcViewerTests.cs, none of
which constructs, shows, or derives from a System.Windows.Forms.Form.

Evidence: fail-before 2 of 5 red, exactly the two mnemonic tests; pass-after
8/8 green including the three Phase 2 #466 tests.

Phase 8 boundary toolchain: csharpier check 0 diffs; analyzers 0 errors /
5 warnings matching baseline with 0 CoreCompile skips; nullable 0 errors with
0 CoreCompile skips; QuickFiler.Test 1169 executed, 1155 passed, 14 failed —
the identical pre-existing base-introduced QfcItemController.* timeout set,
a failed-count delta of 0 and a passed-count delta of exactly +32 matching
the 32 results Phases 5-8 added.

Sizes: EfcViewer.cs 169, EfcViewerTests.cs 164, both well under 500.

spec.md: 6 acceptance criteria checked off against recorded evidence.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Records the nine Phase 9 gates: intent-to-add, changed-file set, sibling
ownership, project files, the ViewerSetup one-line constraint, the #476
WebView2BreadcrumbHost invariant, pre-existing test integrity, and
interface stability. Checks off the nine corresponding spec.md criteria.

Records a deviation: a mandated mid-plan integration merge (2592467)
placed merged siblings #476 and #501 inside BASELINE_SHA..HEAD, so the
scope gates are additionally evaluated against the merged integration tip
38f0978, which isolates this feature's own diff. Both results are
recorded in every affected artifact.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
One consecutive clean pass with zero loop restarts:
- csharpier format over the eight owned files: exit 0, 0 of 8 rewritten
  by SHA-256; csharpier check . exit 0 over 1549 files, 0 unformatted.
- Analyzer rebuild: exit 0, 0 errors / 5 warnings, equal to baseline;
  delivered diagnostic-identifier set empty, a subset of the baseline.
- Nullable rebuild: exit 0, 0 errors; identifier set empty. Both builds
  proved non-vacuous by 0 CoreCompile skips against 36 csc invocations.
- QuickFiler.Test: 1169 executed, 1169 passed, 0 failed; +70 over the
  1099 baseline, reconciling as 26 merged-sibling results plus this
  feature's 44. TRX retained and sanitised.
- Repository-wide coverage 85.25% line / 79.19% branch over 9 assemblies,
  not lower than the 70.32% / 59.12% baseline, recorded with an explicit
  denominator caveat. Raw Cobertura read and deleted, not committed.
- File sizes, test policy and coverage exemptions all within their gates.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…andoff

All 200 plan tasks and all 74 spec.md acceptance criteria are now checked
off against recorded evidence. spec.md differs from its pre-batch state on
exactly 21 lines, every one a pure checkbox flip; no criterion text was
modified.

Recorded rather than concealed:
- Manual check 1 (Alt+F / Alt+M open the two menus in a live Outlook host)
  is MANUAL_CHECK_DEFERRED, not a pass; no Outlook process is running and
  the test policy forbids substituting a live form. Check 2 is verified by
  instrument: the two bytes are code points 0x2d 0x2d.
- The six spec.md follow-up items, plus a seventh discovered on this base,
  are recorded with their reason and target document path but no GitHub
  issue was created: the lifecycle MCP tool is absent from this executor's
  tool set, and writing the intake document would breach the scope gate. A
  duplicate search returned empty for all seven. Handed to the orchestrator.
- The RC7 residual (EfcSelectionGuard.BannerPrefix is a third arity variant,
  and the comment near EfcFormController.cs:325 describes a rejection the
  guard does not implement) is reported and deliberately not fixed; the file
  belongs to merged sibling 614.
- Decision D10's stated premise is false on this base: QuickFiler.Test.csproj
  does carry a log4net reference. The correction is recorded and the false
  premise is not repeated; the BoundaryErrorSink seam is retained on its
  merits.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Policy audit, code review, and feature audit for the efc-controller-surface-defects-464
branch review: 0 blocking findings, 6 non-blocking; 74/74 acceptance criteria substantiated.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@drmoisan
drmoisan merged commit ee7d0ec into epic/quickfiler-bug-family-integration Aug 28, 2026
5 checks passed
@drmoisan
drmoisan deleted the bug/efc-controller-surface-defects-464 branch August 28, 2026 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant