diff --git a/QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs b/QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs index 3914cd65f..93c9c4d27 100644 --- a/QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs +++ b/QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs @@ -449,6 +449,29 @@ public async Task WriteMetricsAsync_WithoutMyDocumentsFolder_DoesNotInvokeWriter .BeFalse("the guard must abort before any write when MyDocuments is absent"); } + /// + /// The zero-line boundary case. When every diagnostic entry is null or whitespace the + /// null-and-whitespace filter leaves an empty array, so there is no content to record and + /// the writer must not be reached at all. The default writer appends, which would create + /// or touch an empty session-metrics file. MyDocuments is present, so the pre-existing + /// MyDocuments guard is not what causes the early return. + /// + [TestMethod] + public async Task WriteMetricsAsync_WithAllNullOrWhitespaceDiagnostics_DoesNotInvokeWriter() + { + var (controller, _) = BuildLooseMetricsController(new[] { " ", null, "\t" }); + var invoked = false; + controller.MetricsFileWriter = (filename, written, folderRoot, token) => + { + invoked = true; + return Task.FromResult(true); + }; + + await controller.WriteMetricsAsync("metrics.csv"); + + invoked.Should().BeFalse("an empty filtered array must not reach the writer at all"); + } + #endregion Issue #442 — metrics flush tests } } diff --git a/QuickFiler/Controllers/QfcHomeController.Metrics.cs b/QuickFiler/Controllers/QfcHomeController.Metrics.cs index df2bf4840..38d33fdac 100644 --- a/QuickFiler/Controllers/QfcHomeController.Metrics.cs +++ b/QuickFiler/Controllers/QfcHomeController.Metrics.cs @@ -172,6 +172,10 @@ ref OlAppointment // no XML documentation and therefore no non-null element guarantee, so this filter // defends the interface contract rather than a known producer defect. var lines = strOutput.Where(line => !string.IsNullOrWhiteSpace(line)).ToArray(); + if (lines.Length == 0) + { + return; + } // CancellationToken.None, never the session Token: the dispatcher continuation that // carries this write is not awaited to completion, so a session cancellation can be diff --git a/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/code-review.2026-09-01T12-53.md b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/code-review.2026-09-01T12-53.md new file mode 100644 index 000000000..1690dc1b2 --- /dev/null +++ b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/code-review.2026-09-01T12-53.md @@ -0,0 +1,230 @@ +# Code Review — Issue #646 (qfc-metrics-flush-writes-empty-session-file) + +Timestamp: 2026-09-01T12-53 + +| Field | Value | +|---|---| +| Branch | `bug/qfc-metrics-flush-writes-empty-session-file-646` | +| HEAD | `0fe0668f146236c65aa93514fcb9756d366a6940` | +| Base | `origin/main` at `8996b28746d32f9f5996a037e0ca76be78b7684d` | +| Source changed | 2 files, 27 insertions, 0 deletions | +| Blocking findings | **0** | + +## Change Under Review + +`QuickFiler/Controllers/QfcHomeController.Metrics.cs`, a four-line insertion at lines 175-178 +inside `WriteMetricsAsync`: + +```csharp +var lines = strOutput.Where(line => !string.IsNullOrWhiteSpace(line)).ToArray(); +if (lines.Length == 0) +{ + return; +} +``` + +`QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs`, a 23-line insertion adding one +MSTest method, `WriteMetricsAsync_WithAllNullOrWhitespaceDiagnostics_DoesNotInvokeWriter`. + +Both are pure insertions. `git diff origin/main...HEAD` reports zero deletions across the whole +branch, so no pre-existing line in either file was altered. + +## Quality Assessment + +### Design and simplicity — good + +The fix is the smallest change that resolves the reported defect. It introduces no new type, no +new seam, no configuration, and no indirection. It closes a stated asymmetry by copying the +shape that already exists in the sibling controller, which is the correct instinct: the two +metrics writers now fail closed the same way for the same input. + +The placement is well chosen. The guard sits immediately after the statement that computes +`lines` and before the three-line `CancellationToken.None` explanatory comment, so that comment +stays adjacent to the `await MetricsFileWriter(...)` statement it explains. Hugging the +computing statement also reuses the file's existing blank line as the separator, which is why +the diff is four lines rather than five. The same structural relationship holds in +`EfcHomeController.Metrics.cs`, where the guard follows `var dataLines = ...` with no +intervening blank line. + +### Correctness — verified + +The guard's condition is total over the domain of `lines`: `ToArray()` on a LINQ `Where` always +yields a non-null array, so `lines.Length` cannot throw and the two outcomes partition the +input completely. Both outcomes are exercised: + +- True outcome — the new test supplies `{ " ", null, "\t" }`, all of which the filter removes, + and asserts the writer flag stays `false`. +- False outcome — `WriteMetricsAsync_InvokesInjectedMetricsFileWriterOnce` supplies two valid + lines and `WriteMetricsAsync_FiltersNullDiagnosticLinesBeforeWriting` supplies a partially + null array that filters down to two. Both still reach the writer and both still pass. + +That second pair is the check that matters most for an early return: it bounds the guard from +the over-broad side and demonstrates the guard did not convert the partial-filter case into a +no-op. Recorded in `evidence/regression-testing/existing-tests-pass.2026-08-31T20-04.md`. + +### Test quality — good + +The new test follows Arrange-Act-Assert with visual separation, carries a six-line XML summary +explaining both the scenario and why the pre-existing MyDocuments guard is not what produces +the early return, and uses the repository-mandated MSTest, Moq, and FluentAssertions triple. +It touches no filesystem, creates no temporary file, performs no wall-clock wait, and depends +on no external service. The `because` reason string on the assertion produces an actionable +failure message, which the RED run demonstrates verbatim: + +``` +Expected invoked to be False because an empty filtered array must not reach the writer at all, but found True. +``` + +The RED-before evidence is genuine rather than a harness artifact. The run took 346 ms rather +than sub-millisecond, the message is the assertion's own text rather than empty, and the +fixture supplied `MyDocuments`, so the pre-existing folder guard was not masking the result. +The GREEN run used a byte-identical `/TestCaseFilter`, so the only variable between the two +runs is the four-line guard. + +### Naming, comments, and documentation — good + +The method name states the input condition and the expected outcome. The XML comment explains +*why* the writer must not be reached (the default writer appends, which creates or touches an +empty file) rather than restating what the code does. No comment was added to the production +file, which is appropriate: the guard is self-evident and the surrounding comments about the +interface contract and the cancellation token remain accurate. + +## Findings + +All findings below are **non-blocking**. Severity is stated alongside reachability — what would +have to happen for the finding to bite in production or CI. + +### CR-1 — Remaining null-array asymmetry with the EFC sibling (Minor, non-blocking) + +`strOutput` is dereferenced without a null check at line 174: + +```csharp +var lines = strOutput.Where(line => !string.IsNullOrWhiteSpace(line)).ToArray(); +``` + +The in-file comment directly above reasons explicitly that +`IQfcCollectionController.GetMoveDiagnostics` "carries no XML documentation and therefore no +non-null element guarantee, so this filter defends the interface contract rather than a known +producer defect." That reasoning defends against null *elements* but not against a null +*array*. If an implementation returned `null`, this line throws `NullReferenceException` before +the new guard is ever evaluated. + +This is notable because the EFC sibling — the file AC2 makes the reference implementation — +does guard the null case. `EfcHomeController.BuildQuickFileMetricLines` opens with +`if (moved is null || moved.Count == 0) { return Array.Empty(); }`, covering null and +empty together. So while this change closes the empty-array asymmetry, a narrower null-array +asymmetry between the two controllers remains. + +**Reachability:** requires an `IQfcCollectionController` implementation whose +`GetMoveDiagnostics` returns `null`. The only production implementation in the repository is +`QfcCollectionController`, which does not, so this is not reachable today. It becomes reachable +if a second implementation is added or the existing one is changed. The interface carries no +contract documentation forbidding it, which is precisely the gap the existing comment +identifies. + +**Disposition:** pre-existing, present identically on `origin/main`, and outside AC7's permitted +edit set is not the reason it is left alone — line 174 is in an owned file. It is left alone +because changing it would expand a four-line guard into a behavioural change to the +enclosing method that has no test and no reported defect behind it. Recommend a follow-up that +either documents the non-null contract on the interface or extends the guard to +`strOutput is null || lines.Length == 0`. + +### CR-2 — The guard does not suppress the calendar-appointment side effect (Minor, non-blocking) + +`WriteMoveToCalendar(...)` is called at line 154, twenty lines before the guard. In a session +where every diagnostic entry is null or whitespace, the guard now prevents the metrics-file +write but the Outlook calendar appointment has already been created. + +**Reachability:** every session that reaches this method with an all-null diagnostic array — +the same input the issue describes. The user still gets a calendar entry for a session that +recorded no diagnostics. + +**Disposition:** not a regression. This ordering is identical on `origin/main`; before this +change such a session produced both a calendar appointment and an empty metrics file, and now +it produces the appointment alone. The issue's Expected Behavior is explicitly scoped to the +metrics file ("No write occurs and no file is created or touched"), and AC1 is scoped to +`MetricsFileWriter`. Widening the guard to cover the calendar write would be a behavioural +change beyond the reported defect and would violate the Bugfix Workflow's minimal-fix rule. +Worth a follow-up decision on whether an empty session should produce a calendar entry at all. + +### CR-3 — Test file at 477 of 500 lines (Minor, non-blocking) + +`QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs` is 477 lines after this change, +up from 454. The repository caps production code, test code, and reusable scripts at 500 lines, +and the cap applies to test files. This change is compliant with 23 lines of headroom. + +**Reachability:** the next test method added to this file breaches the cap. The method added +here cost 23 lines including its documentation comment, so a single comparable addition would +exceed 500. + +**Disposition:** compliant as delivered. The executor flagged this forward risk in +`evidence/other/test-file-line-count.2026-08-31T20-04.md`, which is the right handling. +Recommend the file be split by region before the next addition. + +### CR-4 — New test does not pin the injectable clock (Nit, non-blocking) + +`QfcHomeController` exposes an injectable `TimeProvider` seam defaulting to +`TimeProvider.System`, and `WriteMetricsAsync` reads `TimeProvider.GetLocalNow()` at line 124. +The new test does not override it, so that call reads the real system clock. + +**Reachability:** none for flakiness. The test asserts only on a boolean invocation flag and +makes no assertion that depends on the returned time, so no clock value can change the outcome. +The determinism rule in `.claude/rules/general-unit-test.md` targets tests whose results depend +on wall-clock time; this one's does not. + +**Disposition:** consistent with every sibling test in the same region, which also leave the +seam at its default. No change recommended in isolation. + +### CR-5 — Two evidence artifacts quote superseded figures, reconciled elsewhere (Informational) + +`evidence/other/test-file-diff-scope.2026-08-31T20-04.md` records 25 insertions and blob +`2d93e1ae`, and `evidence/other/test-file-line-count.2026-08-31T20-04.md` records 479 lines. The +delivered state is 23 insertions, blob `93c9c4d2`, and 477 lines. Similarly, +`evidence/other/anchor-rederivation.2026-08-31T20-04.md` predicts the guard at lines 176-179 +whereas it was delivered at 175-178. + +**Reachability:** an auditor comparing those two artifacts against the head tree finds figures +that do not match and could read it as undisclosed drift. + +**Disposition:** both discrepancies are explicitly reconciled elsewhere in the same evidence +set, so this is a cross-reference cost rather than an evidence gap. +`evidence/qa-gates/csharpier-format.2026-08-31T20-04.md` records the CSharpier pass-1 rewrite +that collapsed the assertion chain, states the resulting 479-to-477 line change, and notes it +does not affect the earlier task's acceptance. `evidence/other/production-diff-scope.2026-08-31T20-04.md` +tabulates the anchor prediction against the re-derived truth and explains the off-by-one as a +deliberate placement choice that keeps the diff at four lines. Both superseding artifacts name +the artifact they supersede. Verified independently: `wc -l` and `awk NR` both report 477, and +`git diff --numstat` reports 23 insertions and 0 deletions. + +### CR-6 — Branch history contains an add-then-revert outside the AC7 allowed set (Informational) + +Commit `9f578b3c` added two files under `.claude/agent-memory/orchestrator/` (22 insertions). +Commit `8a2054cd` removed them (22 deletions). + +**Reachability:** none at merge. `git diff --stat origin/main...HEAD -- .claude` returns empty, +so the merged tree gains nothing under that path. AC7 is evaluated against the branch diff, and +the diff is clean. + +**Disposition:** the branch self-corrected before delivery and the correcting commit says so in +its message. Recorded because the history, unlike the diff, shows the excursion. + +## Assessment Against the General Code Change Policy + +| Principle | Assessment | +|---|---| +| Simplicity first | The minimal correct change. No abstraction introduced for a four-line guard. | +| Reusability | The guard shape is duplicated from the EFC sibling rather than factored out. Correct here — extracting a two-line early return across two controllers with different data types and different writer seams would add indirection without removing meaningful duplication. | +| Extensibility | No public API changed. The `MetricsFileWriter` delegate signature is byte-identical to base, verified by re-reading lines 28-34 of the post-change file. | +| Separation of concerns | Preserved. The writer remains an injectable delegate, which is exactly what makes the new test possible without a filesystem. | +| Error handling | The guard is a no-content short circuit, not an error suppressor. The writer-failure logging branch at lines 189-195 is untouched, so genuine write failures still surface. | +| Interaction with existing code | Matches the sibling controller's established form, as AC2 requires, and matches the test file's existing harness and naming conventions. | + +## Verdict + +**PASS with 0 blocking findings.** + +The change is correct, minimal, well placed, and properly evidenced by a genuine RED-then-GREEN +pair. The six findings above are two pre-existing behavioural asymmetries worth a follow-up +(CR-1, CR-2), one forward-looking file-size risk that is compliant as delivered (CR-3), one nit +with no reachable consequence (CR-4), and two documentation-hygiene observations (CR-5, CR-6). +None requires a change to this branch. diff --git a/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/baseline-coverage.jacoco.xml b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/baseline-coverage.jacoco.xml new file mode 100644 index 000000000..926ef147e --- /dev/null +++ b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/baseline-coverage.jacoco.xml @@ -0,0 +1,62 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/branch-reconciliation.2026-08-31T20-04.md b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/branch-reconciliation.2026-08-31T20-04.md new file mode 100644 index 000000000..44f4e62e5 --- /dev/null +++ b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/branch-reconciliation.2026-08-31T20-04.md @@ -0,0 +1,50 @@ +# Baseline — Branch Reconciliation onto origin/main (P0-T6) + +Timestamp: 2026-09-01T12-10 + +## Provenance + +The reconciliation this task specifies was **performed by the orchestrator at handoff**, +before this executor session began. This executor therefore did not run `git merge`; it +ran the fetch and the acceptance check and recorded the resulting state. Re-running the +merge would have been a no-op at best and a spurious empty merge commit at worst. + +## SHAs + +| Role | SHA | +|---|---| +| Pre-reconciliation HEAD (orchestrator, before merge) | `3c4afd8c937a19577095465108ae19ca59690db3` | +| `origin/main` merged in | `8996b28746d32f9f5996a037e0ca76be78b7684d` | +| Post-reconciliation HEAD (orchestrator, merge commit) | `c7b54bf5622a02fe58250b3c09db5b1606648fda` | +| HEAD at executor handoff (one follow-up commit later) | `8a2054cd6c857195712c7db6cee0a34b631f3ca7` | + +The merge was a merge of `origin/main` into the branch (not a fast-forward) and completed +with zero conflicts, as reported by the orchestrator at handoff. + +Branch: `bug/qfc-metrics-flush-writes-empty-session-file-646` + +## Commands + +Command: `git fetch origin` +EXIT_CODE: 0 + +Command: `git rev-parse origin/main` +EXIT_CODE: 0 +Output: `8996b28746d32f9f5996a037e0ca76be78b7684d` + +Command: `git rev-parse HEAD` +EXIT_CODE: 0 +Output: `8a2054cd6c857195712c7db6cee0a34b631f3ca7` + +Command: `git merge-base --is-ancestor origin/main HEAD` +EXIT_CODE: 0 + +## Output Summary + +`git fetch origin` succeeded and did not advance `origin/main`: the tip is still +`8996b287`, the same commit the orchestrator merged at handoff. The acceptance check +`git merge-base --is-ancestor origin/main HEAD` exits `0`, confirming the current branch +already contains the `origin/main` tip. No merge was performed by this executor. The +working tree was clean (`git status --porcelain` empty) at the time of this check. + +ACCEPTANCE: MET — `git merge-base --is-ancestor origin/main HEAD` exits 0. diff --git a/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/coverage-cobertura-baseline.2026-08-31T20-04.md b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/coverage-cobertura-baseline.2026-08-31T20-04.md new file mode 100644 index 000000000..14aa870f0 --- /dev/null +++ b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/coverage-cobertura-baseline.2026-08-31T20-04.md @@ -0,0 +1,113 @@ +# Baseline — Cobertura Coverage Headline (P0-T11) + +Timestamp: 2026-09-01T12-18 + +Working directory: repository root (worktree for branch +`bug/qfc-metrics-flush-writes-empty-session-file-646`) +HEAD: `8a2054cd6c857195712c7db6cee0a34b631f3ca7` + +Referenced artifact: `evidence/baseline/baseline-coverage.cobertura.xml` + +## Discovery of the .coverage Input + +Command: +`Get-ChildItem -Path TestResults -Filter *.coverage -Recurse | Sort-Object LastWriteTime -Descending | Select-Object -First 1` +EXIT_CODE: 0 +Selected input `LastWriteTime`: `2026-09-01T12:14:55.1096677-04:00`, which matches the +P0-T10 run. The file resides under a GUID-named subdirectory of `TestResults/` and its name +is machine- and account-derived, so it is identified here by timestamp rather than by +literal name. + +## Conversion + +Command: +`dotnet-coverage merge -f cobertura -o docs\features\active\2026-08-27-qfc-metrics-flush-writes-empty-session-file-646\evidence\baseline\baseline-coverage.cobertura.xml ` +EXIT_CODE: 0 +Tool version reported: `dotnet-coverage v18.5.2.0 [win-x64 - .NET 10.0.11]` +Output: `Merged into file ...\evidence\baseline\baseline-coverage.cobertura.xml.` + +## Baseline Coverage Headline (verbatim root `` attribute values) + +| Attribute | Value | +|---|---| +| `line-rate` | `0.3404862683334974` | +| `branch-rate` | `1` | +| `lines-covered` | `48426` | +| `lines-valid` | `142226` | + +`line-rate` is a numeric string, not a placeholder, satisfying the task acceptance +condition. As a percentage the baseline repository-wide figure is **34.05%**. + +Two qualifications on that figure, both recorded rather than resolved: + +1. The denominator is every assembly the `QuickFiler.Test` run loaded, including vendored + and third-party code (for example `Mono.Reflection`, whose source paths are not in this + repository at all). It is therefore not the "first-party testable denominator" that + `CLAUDE.md` UT2 defines its >= 80% floor against, and it is not directly comparable to + that floor. +2. `branch-rate="1"` is not a meaningful 100% branch result. The root element carries no + `branches-covered` or `branches-valid` attributes at all (both read empty), so this + converter emitted no branch data for this run and the value cannot be interpreted. + +Per the plan's Coverage Policy Note, the repository-wide percentage is treated as a +recorded, non-blocking figure. The blocking gate is P2-T7: no regression in this same +`line-rate`, measured the same way, plus `hits > 0` on each of the four guard lines added +by P1-T5. + +## Baseline Detail for the File Under Change + +`QuickFiler/Controllers/QfcHomeController.Metrics.cs` appears as three `` elements +(the type plus two compiler-generated nested types): + +| `` | `line-rate` | +|---|---| +| `QuickFiler.Controllers.QfcHomeController` | `0.6986301369863014` | +| `QuickFiler.Controllers.QfcHomeController.<>c` | `1` | +| `QuickFiler.Controllers.QfcHomeController.d__103` | `0.8775510204081632` | + +Taking the union of `` entries across all three elements and deduplicating by line +number (keeping the maximum `hits` per line, since the async state machine reports some +lines under more than one element), the baseline for this file is **94 of 122 distinct +lines covered = 77.05%**. + +Baseline hit counts at the two edit anchors and at the region the plan declares off-limits: + +| Line | Role | `hits` | +|---|---|---| +| 174 | Anchor A — `var lines = strOutput.Where(...).ToArray();` | 1 | +| 179 | Anchor B — `bool metricsWritten = await MetricsFileWriter(` | 1 | +| 185 | `if (!metricsWritten)` — the #647 failure branch condition | 1 | +| 186-191 | body of the #647 failure branch | 0 | +| 192 | method close | 1 | + +This independently corroborates the plan's cited anchor line numbers against the current +tree: both anchors exist and are executed by the existing suite. + +## Sanitisation Micro-Action Recorded + +`dotnet-coverage` writes each `` attribute as an absolute path. The +generated file contained 3253 occurrences of the absolute worktree prefix, which may not +appear in a committed artifact. A literal string replacement removed that prefix, rendering +every path repository-relative (for example +`QuickFiler\Controllers\QfcHomeController.Metrics.cs`). + +| Check | Result | +|---|---| +| Occurrences of the absolute worktree prefix replaced | 3253 | +| Residual occurrences of the account name | 0 | +| Residual occurrences of the machine name | 0 | +| Residual occurrences of `C:\Users` | 0 | +| XML still well-formed after replacement | Yes (reparsed as `[xml]`) | +| Root `line-rate` after replacement | `0.3404862683334974` (unchanged) | +| File size | 26,064,187 bytes | + +No angle-bracket placeholder was substituted into any XML attribute; the prefix was removed +rather than replaced with a token, so no attribute value was made ill-formed. + +## Output Summary + +Baseline repository-wide `line-rate` is `0.3404862683334974` (34.05%) over 48,426 of +142,226 lines; `branch-rate` is `1` but carries no branch counts and is not interpretable. +`QuickFiler/Controllers/QfcHomeController.Metrics.cs` is at 94/122 distinct lines (77.05%) +with both P1-T1 edit anchors executed. The artifact exists, parses, and reports a numeric +`line-rate`. diff --git a/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/csharpier-check.2026-08-31T20-04.md b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/csharpier-check.2026-08-31T20-04.md new file mode 100644 index 000000000..a1f7941a0 --- /dev/null +++ b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/csharpier-check.2026-08-31T20-04.md @@ -0,0 +1,46 @@ +# Baseline — CSharpier Check (P0-T7) + +Timestamp: 2026-09-01T12-14 + +Working directory: repository root (worktree for branch +`bug/qfc-metrics-flush-writes-empty-session-file-646`) +HEAD: `8a2054cd6c857195712c7db6cee0a34b631f3ca7` + +Command: `dotnet tool run csharpier check .` +EXIT_CODE: 0 + +## Verbatim Printed Summary Line + +``` +Checked 1566 files in 4451ms. +``` + +## Output Summary + +Baseline formatting state is clean. CSharpier 1.2.6 (the version pinned by +`dotnet-tools.json`) checked 1566 files and listed no file as needing formatting; in +check mode CSharpier prints one `Error ...` line per non-compliant file before the summary, +and no such line was printed. Exit code 0. + +This is a baseline capture, not a gate; it is recorded whatever the exit code, and in this +run the exit code was 0. + +## Precondition Micro-Action Recorded + +`dotnet tool run` initially failed with exit 155 and the repository's own `global.json` +`errorMessage`: the repo-local .NET SDK required by `global.json` (version `8.0.205`, with +`paths` limited to `.dotnet-sdk` and `$host$`) was absent from this fresh worktree, and the +host SDK is `10.0.400`, which `rollForward: latestFeature` does not accept for an `8.0.x` +pin. The repository's own provisioning script was run to satisfy the precondition: + +Command: `pwsh -NoProfile -File scripts/vscode/Install-RepoDotNetSdk.ps1` +EXIT_CODE: 0 +Output: `Installed repo-local .NET SDK 8.0.205 to /.dotnet-sdk.` + +Command: `dotnet tool restore` +EXIT_CODE: 0 +Output: `Tool 'csharpier' (version '1.2.6') was restored. Available commands: csharpier` + +`.dotnet-sdk/` is excluded from version control by `.gitignore` line 350 (`.dotnet*/`), +confirmed by `git check-ignore -v .dotnet-sdk/`, so this provisioning step adds nothing to +the change footprint checked by P2-T8. diff --git a/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/msbuild-analyzer-rebuild.2026-08-31T20-04.md b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/msbuild-analyzer-rebuild.2026-08-31T20-04.md new file mode 100644 index 000000000..a41c7e690 --- /dev/null +++ b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/msbuild-analyzer-rebuild.2026-08-31T20-04.md @@ -0,0 +1,55 @@ +# Baseline — MSBuild Analyzer Rebuild (P0-T8) + +Timestamp: 2026-09-01T12-22 + +Working directory: repository root (worktree for branch +`bug/qfc-metrics-flush-writes-empty-session-file-646`) +HEAD: `8a2054cd6c857195712c7db6cee0a34b631f3ca7` + +Command: `msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true` +EXIT_CODE: 0 + +## Verbatim Printed Summary Lines + +``` +Build succeeded. + + 5 Warning(s) + 0 Error(s) +``` + +## Output Summary + +The analyzer gate passes at baseline: `Build succeeded.`, 5 warnings, 0 errors, exit code +0. All 5 warnings are the same non-analyzer MSBuild warning emitted by the +`_RxCheckPackagesConfig` target in +`packages/System.Reactive.7.0.0/build/System.Reactive.PackagesConfigCheck.targets(31,5)`, +reporting that a `packages.config` project is not supported by System.Reactive v7.0 or +later. It is raised once each for `UtilitiesCS`, `ToDoModel`, `QuickFiler`, `TaskMaster`, +and `UtilitiesCS.Test`. No Roslyn or .NET analyzer diagnostic (`CAxxxx`, `IDExxxx`, +`Sxxxx`, `RCSxxxx`, `MAxxxx`, `AsyncFixerxx`) appears in the output. This warning set is +pre-existing on the branch and unrelated to issue #646. + +## Non-Vacuity Check + +`/t:Rebuild` (not `/t:Build`) was used as required by `CLAUDE.md` C#1.2, so MSBuild's +incremental up-to-date check cannot skip `CoreCompile`. The captured build log contains 75 +`CoreCompile` references and 36 `csc.exe` command-line occurrences, confirming compilation +and therefore analyzer execution actually occurred rather than being skipped. The gate was +capable of failing. + +## Precondition Micro-Action Recorded + +The first invocation of this command on this fresh worktree failed with EXIT_CODE 1 and +`37 Error(s)`, every error being the same NuGet message: `This project references NuGet +package(s) that are missing on this computer. Use NuGet Package Restore to download +them.` The worktree had no restored `packages/` directory. The repository's standard +restore for these `packages.config`-style legacy projects was run: + +Command: `nuget restore TaskMaster.sln` +EXIT_CODE: 0 +Output: `Installed: 172 package(s) to packages.config projects` + +`packages/` is excluded from version control by `.gitignore` line 358, confirmed by +`git check-ignore -v packages/`, so this restore adds nothing to the change footprint +checked by P2-T8. The command above was then re-run and produced the recorded result. diff --git a/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/msbuild-nullable-rebuild.2026-08-31T20-04.md b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/msbuild-nullable-rebuild.2026-08-31T20-04.md new file mode 100644 index 000000000..4fefc9741 --- /dev/null +++ b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/msbuild-nullable-rebuild.2026-08-31T20-04.md @@ -0,0 +1,44 @@ +# Baseline — MSBuild Nullable Rebuild (P0-T9) + +Timestamp: 2026-09-01T12-24 + +Working directory: repository root (worktree for branch +`bug/qfc-metrics-flush-writes-empty-session-file-646`) +HEAD: `8a2054cd6c857195712c7db6cee0a34b631f3ca7` + +Command: `msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:TreatWarningsAsErrors=true` +EXIT_CODE: 0 + +## Verbatim Printed Summary Lines + +``` +Build succeeded. + + 5 Warning(s) + 0 Error(s) +``` + +## Output Summary + +The nullable/type-check gate passes at baseline: `Build succeeded.`, 5 warnings, 0 errors, +exit code 0. Zero `CS86xx` nullable-flow diagnostics appear anywhere in the build log +(`grep -c "CS86"` returns `0`). + +The 5 remaining warnings are the same non-compiler MSBuild warnings recorded in the P0-T8 +artifact, emitted by the `_RxCheckPackagesConfig` target in +`packages/System.Reactive.7.0.0/build/System.Reactive.PackagesConfigCheck.targets(31,5)`. +`/p:TreatWarningsAsErrors=true` sets a C# compiler property and does not promote a warning +raised by an MSBuild `Warning` task, which is why these 5 survive as warnings under this +gate rather than becoming errors. + +Per `CLAUDE.md` C#1.3, `/p:Nullable=enable` was deliberately **not** passed. Nullable +enforcement in this repository is per-file opt-in via `#nullable enable`; forcing the +solution-wide property would conscript files that have never adopted the pragma and does +not match `.github/workflows/ci.yml`. + +## Non-Vacuity Check + +`/t:Rebuild` was used, not `/t:Build`. The captured log contains 36 `csc.exe` command-line +occurrences, confirming `CoreCompile` ran on every project rather than being skipped by +MSBuild's incremental up-to-date check, which does not invalidate on a command-line `/p:` +change. The gate was capable of failing. diff --git a/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/phase0-instructions-read.md b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/phase0-instructions-read.md new file mode 100644 index 000000000..6678dec28 --- /dev/null +++ b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/phase0-instructions-read.md @@ -0,0 +1,50 @@ +# Phase 0 — Policy Instructions Read (Issue #646) + +Timestamp: 2026-09-01T12-09 + +Policy Order: The `policy-compliance-order` skill sequence was followed exactly, in this +order: + +1. `CLAUDE.md` (repository root) — standing instructions, always loaded +2. `.claude/rules/general-code-change.md` — cross-language code change policy +3. `.claude/rules/general-unit-test.md` — cross-language unit test policy +4. `.claude/rules/csharp.md` — C#-specific rules, applicable because both in-scope source + files are `*.cs` + +## Files Read + +| # | Repository-relative path | Plan task | Read in full | +|---|---|---|---| +| 1 | `CLAUDE.md` | P0-T1 | Yes (448 lines) | +| 2 | `.claude/rules/general-code-change.md` | P0-T2 | Yes (81 lines) | +| 3 | `.claude/rules/general-unit-test.md` | P0-T3 | Yes (106 lines) | +| 4 | `.claude/rules/csharp.md` | P0-T4 | Yes (95 lines) | + +## Constraints Extracted and Carried Into Execution + +- C# toolchain order is format, lint, type-check, test; restart from step 1 on any failure + or file rewrite (`CLAUDE.md` CUT3; `.claude/rules/csharp.md` Toolchain). +- Both `msbuild` gates use `/t:Rebuild`, not `/t:Build`; a warm `/t:Build` skips + `CoreCompile` and returns exit 0 without running analyzers or nullable-flow diagnostics + (`CLAUDE.md` C#1.2 and C#1.3). +- Do not pass `/p:Nullable=enable`; nullable enforcement in this repository is per-file + opt-in via `#nullable enable` (`CLAUDE.md` C#1.3). +- CSharpier is invoked through `dotnet tool run` so the manifest-pinned version is used. +- Tests use MSTest, Moq, and FluentAssertions (`CLAUDE.md` CUT1/CUT2). +- No file may exceed 500 lines (`.claude/rules/general-code-change.md`, File Size Limit). + Tracked for this item by plan task P1-T15 against the test file. +- Temporary files in tests are prohibited (`.claude/rules/general-unit-test.md`, External + Dependencies). The new regression test uses only in-memory delegate capture. +- Bugfix workflow applies (this item is a defect): failing regression test first, then the + minimal targeted fix, then full local verification (`CLAUDE.md`, Bugfix Workflow). The + plan implements this as P1-T2/P1-T4 (fail-before), P1-T5 (fix), P1-T9 (pass-after). + +## Coverage Threshold Conflict (Recorded, Not Resolved Here) + +`CLAUDE.md` UT2 states a repository-wide line-coverage floor of >= 80% with >= 90% for new +modules/classes/methods. `.claude/rules/general-unit-test.md` states a uniform >= 85% line +and >= 75% branch floor. The two documents disagree on the repository-wide figure. Per the +plan's "Coverage Policy Note", this execution records the repository-wide percentage as a +non-blocking figure and treats changed-line no-regression plus new-code coverage of the +four added guard lines as the blocking gate (plan task P2-T7). This execution does not +resolve the documentary conflict. diff --git a/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/vstest-coverage-run.2026-08-31T20-04.md b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/vstest-coverage-run.2026-08-31T20-04.md new file mode 100644 index 000000000..682bca29c --- /dev/null +++ b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/vstest-coverage-run.2026-08-31T20-04.md @@ -0,0 +1,55 @@ +# Baseline — vstest.console.exe with Code Coverage (P0-T10) + +Timestamp: 2026-09-01T12-15 + +Working directory: repository root (worktree for branch +`bug/qfc-metrics-flush-writes-empty-session-file-646`) +HEAD: `8a2054cd6c857195712c7db6cee0a34b631f3ca7` + +## Resolution of vstest.console.exe + +`vstest.console.exe` is not on `PATH` in this environment and was resolved via `vswhere`, +exactly as the plan task specifies: + +``` +$vswhere = Join-Path ${env:ProgramFiles(x86)} 'Microsoft Visual Studio\Installer\vswhere.exe' +$vstest = & $vswhere -latest -products * -find 'Common7\IDE\Extensions\TestPlatform\vstest.console.exe' | Select-Object -First 1 +``` + +## Command + +Command: `& $vstest QuickFiler.Test\bin\Debug\QuickFiler.Test.dll /EnableCodeCoverage` +EXIT_CODE: 0 + +This is the bare CUT3 form: the exact assembly path and the single `/EnableCodeCoverage` +flag, with no `/Settings`, no `/InIsolation`, and no `/TestCaseFilter` added. No fallback +form was needed; the run completed on the first attempt. + +## Verbatim Printed Summary Lines + +``` +Test Run Successful. +Total tests: 1284 + Passed: 1284 + Total time: 13.9326 Seconds +``` + +(`vstest.console.exe` prints `Test Run Successful.` with a `Total tests:` / `Passed:` +block. The `Passed!` / `Failed!` single-line form named in the plan task is the `dotnet +test` spelling; the lines above are what this runner actually printed. No `Failed:` line +was printed, which `vstest.console.exe` omits when the failed count is zero.) + +## Output Summary + +Baseline test state for `QuickFiler.Test` is green: 1284 tests run, 1284 passed, 0 failed, +exit code 0, elapsed 13.93 s. This total is the floor that the P2-T5 final run must meet +or exceed. A code-coverage attachment was produced under `TestResults/` and is consumed by +P0-T11 to generate the baseline Cobertura report. + +## Coverage Attachment + +A single `.coverage` attachment was written under a GUID-named subdirectory of +`TestResults/` at the repository root. Its filename is machine- and account-derived and is +therefore not reproduced here; P0-T11 locates it by recency rather than by literal name. +`TestResults/` is excluded from version control by `.gitignore`, so it does not enter the +change footprint checked by P2-T8. diff --git a/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/other/anchor-rederivation.2026-08-31T20-04.md b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/other/anchor-rederivation.2026-08-31T20-04.md new file mode 100644 index 000000000..f0dcce1c2 --- /dev/null +++ b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/other/anchor-rederivation.2026-08-31T20-04.md @@ -0,0 +1,116 @@ +# Anchor Re-Derivation Against the Current Tree (P1-T1) + +Timestamp: 2026-09-01T12-30 + +File: `QuickFiler/Controllers/QfcHomeController.Metrics.cs` +Branch: `bug/qfc-metrics-flush-writes-empty-session-file-646` +HEAD at re-derivation: `1ea16f43` (post-P0-T6 reconciliation; `origin/main` = `8996b287` +is an ancestor) + +The line numbers below were derived by searching the file as it stands now. They were not +copied from the plan text, from `research.2026-08-31T20-30.md`, or from +`research-correction.2026-08-31T20-45.md`. + +## Anchor A + +Command: +`grep -n -F 'var lines = strOutput.Where(line => !string.IsNullOrWhiteSpace(line)).ToArray();' QuickFiler/Controllers/QfcHomeController.Metrics.cs` +EXIT_CODE: 0 + +Result: + +``` +174: var lines = strOutput.Where(line => !string.IsNullOrWhiteSpace(line)).ToArray(); +``` + +Occurrence count (`grep -c -F`): **1** + +## Anchor B + +Command: +`grep -n -F 'bool metricsWritten = await MetricsFileWriter(' QuickFiler/Controllers/QfcHomeController.Metrics.cs` +EXIT_CODE: 0 + +Result: + +``` +179: bool metricsWritten = await MetricsFileWriter( +``` + +Occurrence count (`grep -c -F`): **1** + +## Acceptance + +| Condition | Result | +|---|---| +| Anchor A found exactly once | Yes (1 occurrence, line 174) | +| Anchor B found exactly once | Yes (1 occurrence, line 179) | +| Line numbers recorded | Yes (174 and 179) | + +ACCEPTANCE: MET. + +## Region Context (read directly, lines 170-192) + +``` +170 +171 // The call is made through IQfcCollectionController.GetMoveDiagnostics, which carries +172 // no XML documentation and therefore no non-null element guarantee, so this filter +173 // defends the interface contract rather than a known producer defect. +174 var lines = strOutput.Where(line => !string.IsNullOrWhiteSpace(line)).ToArray(); +175 +176 // CancellationToken.None, never the session Token: the dispatcher continuation that +177 // carries this write is not awaited to completion, so a session cancellation can be +178 // raised while the write is in flight and must not destroy the metrics. +179 bool metricsWritten = await MetricsFileWriter( +180 filename, +181 lines, +182 myDocuments, +183 CancellationToken.None +184 ); +185 if (!metricsWritten) +186 { +187 logger.Error( +188 $"Session metrics were not written to {LOC_TXT_FILE}. The writer exhausted its " +189 + "retry budget or failed after opening the file." +190 ); +191 } +192 } +``` + +## Consequences for P1-T5 + +The three-line `CancellationToken.None` explanatory comment occupies lines **176-178** and +immediately precedes Anchor B. P1-T5 requires the guard to be inserted immediately after +Anchor A and **before** that comment block, so the comment stays adjacent to the writer +statement it explains. The insertion point is therefore between line 174 and line 176, +replacing the single blank line at 175 with the guard block plus separating blank lines. + +After the four-line guard is inserted, every line from the current 176 onward shifts by +`+4`. Predicted post-fix positions: + +| Element | Pre-fix line(s) | Predicted post-fix line(s) | +|---|---|---| +| Anchor A | 174 | 174 | +| `if (lines.Length == 0)` | — | 176 | +| `{` | — | 177 | +| `return;` | — | 178 | +| `}` | — | 179 | +| `CancellationToken.None` comment | 176-178 | 180-182 | +| Anchor B | 179 | 183 | +| `if (!metricsWritten)` branch | 185-191 | 189-195 | +| Method close | 192 | 196 | + +These predictions are verified against the actual file in P1-T6 and used by P2-T7 to locate +the four guard lines in the final Cobertura report. + +## Cross-Check Against Independent Sources + +| Source | Anchor A | Anchor B | Agrees | +|---|---|---|---| +| This re-derivation (authoritative) | 174 | 179 | — | +| Plan self-review section | 174 | 179 | Yes | +| Orchestrator post-merge cross-check at handoff | 174 | 179 | Yes | +| P0-T11 baseline Cobertura per-line hits | 174 (`hits=1`) | 179 (`hits=1`) | Yes | + +Four independent derivations agree, and the coverage report additionally confirms both +anchor lines are executed by the existing test suite. diff --git a/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/other/production-diff-scope.2026-08-31T20-04.md b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/other/production-diff-scope.2026-08-31T20-04.md new file mode 100644 index 000000000..925d105da --- /dev/null +++ b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/other/production-diff-scope.2026-08-31T20-04.md @@ -0,0 +1,123 @@ +# Production Diff Scope Verification (P1-T6) + +Timestamp: 2026-09-01T12-40 + +Command: `git diff origin/main -- QuickFiler/Controllers/QfcHomeController.Metrics.cs` +EXIT_CODE: 0 +`origin/main` = `8996b28746d32f9f5996a037e0ca76be78b7684d` + +## Verbatim Diff + +``` +diff --git a/QuickFiler/Controllers/QfcHomeController.Metrics.cs b/QuickFiler/Controllers/QfcHomeController.Metrics.cs +index df2bf484..38d33fda 100644 +--- a/QuickFiler/Controllers/QfcHomeController.Metrics.cs ++++ b/QuickFiler/Controllers/QfcHomeController.Metrics.cs +@@ -172,6 +172,10 @@ namespace QuickFiler.Controllers + // no XML documentation and therefore no non-null element guarantee, so this filter + // defends the interface contract rather than a known producer defect. + var lines = strOutput.Where(line => !string.IsNullOrWhiteSpace(line)).ToArray(); ++ if (lines.Length == 0) ++ { ++ return; ++ } + + // CancellationToken.None, never the session Token: the dispatcher continuation that + // carries this write is not awaited to completion, so a session cancellation can be +``` + +Command: `git diff --numstat origin/main -- QuickFiler/Controllers/QfcHomeController.Metrics.cs` +Output: `4 0 QuickFiler/Controllers/QfcHomeController.Metrics.cs` +(4 insertions, 0 deletions) + +Command: `git diff -U0 origin/main -- QuickFiler/Controllers/QfcHomeController.Metrics.cs | grep "^@@"` +Output: `@@ -174,0 +175,4 @@ namespace QuickFiler.Controllers` +(exactly one hunk, a pure insertion of 4 lines after old line 174) + +Command: `git diff origin/main -- QuickFiler/Controllers/QfcHomeController.Metrics.cs | grep -c "^-[^-]"` +Output: `0` (zero removed content lines) + +## Acceptance — All Three Conditions + +| # | Condition | Observed | Met | +|---|---|---|---| +| 1 | Zero removed (`-`) lines appear in the diff | `git diff --numstat` reports `0` deletions; the removed-content-line count is `0` | Yes | +| 2 | The only added (`+`) lines are the four guard lines from P1-T5 | `git diff --numstat` reports exactly `4` insertions, and the diff body shows them to be `if (lines.Length == 0)`, `{`, `return;`, `}` and nothing else | Yes | +| 3 | No hunk touches the `MetricsFileWriter` property declaration or the `if (!metricsWritten)` block | The single hunk spans old lines 172-177 only; see the span analysis below | Yes | + +ACCEPTANCE: MET. + +## Hunk Span Analysis for Condition 3 + +The diff contains exactly one hunk, `@@ -172,6 +172,10 @@`, covering old lines 172 through +177 inclusive. The two regions the plan's Hard Scope Boundary 1 places off-limits both sit +outside that span: + +| Protected region (issue #647's delivered outcome) | Lines before change | Inside hunk span 172-177 | +|---|---|---| +| `MetricsFileWriter` delegate declaration, `Func>` | 28-34 | No — 144 lines above the hunk | +| `if (!metricsWritten)` failure-logging branch | 185-191 | No — 8 lines below the hunk | + +Both regions were additionally re-read directly in the post-change file and are byte-for-byte +unchanged. The declaration still reads, at lines 28-34: + +``` + internal Func< + string, + string[], + string, + CancellationToken, + Task + > MetricsFileWriter { get; set; } = FileIO2.WriteTextFileAsync; +``` + +The `Task` return type is intact and the failure branch is intact; this change neither +altered the delegate signature nor the failure-handling branch. This is the evidence backing +AC6. + +## Post-Fix Line Numbers + +Re-derived directly from the changed file: + +| Element | Pre-fix | Post-fix | Predicted by P1-T1 | +|---|---|---|---| +| Anchor A — `var lines = strOutput.Where(...)` | 174 | 174 | 174 (correct) | +| `if (lines.Length == 0)` | — | **175** | 176 (off by one) | +| `{` | — | **176** | 177 (off by one) | +| `return;` | — | **177** | 178 (off by one) | +| `}` | — | **178** | 179 (off by one) | +| `CancellationToken.None` comment | 176-178 | 180-182 | 180-182 (correct) | +| Anchor B — `bool metricsWritten = await MetricsFileWriter(` | 179 | **183** | 183 (correct) | +| `if (!metricsWritten)` | 185 | **189** | 189 (correct) | + +**The four guard lines are 175, 176, 177, 178.** These are the line numbers P2-T7 uses to +locate the guard's per-line `hits` in the final Cobertura report. + +P1-T1 predicted the guard at 176-179 on the assumption that the pre-existing blank line at +old 175 would be kept *above* the guard. It was placed *below* the guard instead, so the +guard occupies 175-178 and everything from the comment block onward lands exactly where +P1-T1 predicted. The placement decision is explained next; it is why re-deriving the numbers +after the edit, rather than trusting the prediction, was required. + +## Why the Guard Hugs Anchor A + +AC2 requires the guard to be "textually equivalent to the guard already present in +`QuickFiler/Controllers/EfcHomeController.Metrics.cs`". The EFC guard at lines 72-75 of that +file follows its computing statement (`var dataLines = BuildQuickFileMetricLines(...)`, +ending line 71) with **no blank line between them**, and is followed by a blank line before +the next statement. The QFC guard is placed the same way: directly after Anchor A, with the +file's pre-existing blank line now serving as the separator between the guard and the +`CancellationToken.None` comment block. + +This placement has two consequences that both favor it: + +1. It mirrors the EFC form structurally as well as textually, which is what AC2 asks for and + what the issue's "Suspected Cause" section identifies as the asymmetry being closed. +2. It reuses the existing blank line rather than adding a new one, so the diff is exactly + four added lines. Placing the guard below the blank line instead would have added a fifth, + whitespace-only line and put the diff at 5 insertions rather than the 4 this task's + acceptance condition specifies. + +The `CancellationToken.None` comment remains adjacent to the `await MetricsFileWriter(...)` +statement it explains, as P1-T5 requires — nothing was interposed between that comment and +its statement. diff --git a/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/other/test-file-diff-scope.2026-08-31T20-04.md b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/other/test-file-diff-scope.2026-08-31T20-04.md new file mode 100644 index 000000000..fb9aba5d9 --- /dev/null +++ b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/other/test-file-diff-scope.2026-08-31T20-04.md @@ -0,0 +1,94 @@ +# Test-File Diff Scope Verification (P1-T13) + +Timestamp: 2026-09-01T12-46 + +Command: `git diff origin/main -- QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs` +EXIT_CODE: 0 +`origin/main` = `8996b28746d32f9f5996a037e0ca76be78b7684d` + +## Verbatim Diff + +``` +diff --git a/QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs b/QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs +index 3914cd65..2d93e1ae 100644 +--- a/QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs ++++ b/QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs +@@ -449,6 +449,31 @@ namespace QuickFiler.Controllers.Tests + .BeFalse("the guard must abort before any write when MyDocuments is absent"); + } + ++ /// ++ /// The zero-line boundary case. When every diagnostic entry is null or whitespace the ++ /// null-and-whitespace filter leaves an empty array, so there is no content to record and ++ /// the writer must not be reached at all. The default writer appends, which would create ++ /// or touch an empty session-metrics file. MyDocuments is present, so the pre-existing ++ /// MyDocuments guard is not what causes the early return. ++ /// ++ [TestMethod] ++ public async Task WriteMetricsAsync_WithAllNullOrWhitespaceDiagnostics_DoesNotInvokeWriter() ++ { ++ var (controller, _) = BuildLooseMetricsController(new[] { " ", null, "\t" }); ++ var invoked = false; ++ controller.MetricsFileWriter = (filename, written, folderRoot, token) => ++ { ++ invoked = true; ++ return Task.FromResult(true); ++ }; ++ ++ await controller.WriteMetricsAsync("metrics.csv"); ++ ++ invoked ++ .Should() ++ .BeFalse("an empty filtered array must not reach the writer at all"); ++ } ++ + #endregion Issue #442 — metrics flush tests + } + } +``` + +Command: `git diff --numstat origin/main -- QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs` +Output: `25 0 QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs` +(25 insertions, 0 deletions) + +Command: `git diff origin/main -- QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs | grep -c "^-[^-]"` +Output: `0` + +Command: `git diff -U0 origin/main -- QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs | grep "^@@"` +Output: `@@ -451,0 +452,25 @@ namespace QuickFiler.Controllers.Tests` +(exactly one hunk, a pure insertion after old line 451) + +## Acceptance + +| Condition | Observed | Met | +|---|---|---| +| No `-` line appears in the diff (additions only) | 0 deletions per `--numstat`; removed-content-line count is 0 | Yes | + +ACCEPTANCE: MET. + +## What This Establishes for AC5 + +AC5 requires that `WriteMetricsAsync_InvokesInjectedMetricsFileWriterOnce` and +`WriteMetricsAsync_FiltersNullDiagnosticLinesBeforeWriting` "still pass and are not +modified". The two halves are evidenced separately: + +- **Still pass** — P1-T12 ran both under the guarded implementation: `Passed: 2`, exit 0. +- **Not modified** — this diff. The change to the file is a single pure insertion of 25 + lines after old line 451. Zero lines were removed and zero were altered, so no + pre-existing line in the file, including every line of both named tests, differs from + `origin/main`. The insertion point sits after + `WriteMetricsAsync_WithoutMyDocumentsFolder_DoesNotInvokeWriter` (which ends at old line + 450) and before the `#endregion Issue #442 — metrics flush tests` marker, so it is inside + the intended region and does not disturb the shared `BuildLooseMetricsController` harness + (lines 72-135) or the `MetricsWrite` capture record (lines 304-323) that both named tests + depend on. + +The insertion is additive in the strongest sense available from git: a zero-deletion, +single-hunk diff cannot have modified an existing test. + +## Note on the 25-Line Count + +The 25 inserted lines are the new test method in full: a 7-line XML documentation comment, +the `[TestMethod]` attribute, the signature, the 14-line body, the closing brace, and one +trailing blank line separating it from the `#endregion` marker. The plan's P1-T2 describes +this method; no other content was added to the file. diff --git a/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/other/test-file-line-count.2026-08-31T20-04.md b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/other/test-file-line-count.2026-08-31T20-04.md new file mode 100644 index 000000000..2cb65cb73 --- /dev/null +++ b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/other/test-file-line-count.2026-08-31T20-04.md @@ -0,0 +1,48 @@ +# Post-Change Test-File Line Count (P1-T15) + +Timestamp: 2026-09-01T12-47 + +Command: `(Get-Content 'QuickFiler.Test\Controllers\QfcHomeControllerMetricsTests.cs').Count` +EXIT_CODE: 0 +Output: `479` + +## Acceptance + +| Condition | Required | Observed | Met | +|---|---|---|---| +| Post-change line count of the test file | `<= 500` | `479` | Yes | + +ACCEPTANCE: MET. Headroom remaining: 21 lines. + +## Context + +`.claude/rules/general-code-change.md` (File Size Limit) and `CLAUDE.md` General Code Change +Policy section 4 both cap production code, test code, and reusable script files at 500 lines. +The plan flagged this as a live risk in Hard Scope Boundary 5: the file was already at 454 +lines before this change, leaving only 46 lines of headroom, so the cap could not be assumed +to hold and needed an explicit post-change check rather than an estimate. + +| Point | Line count | Headroom to the 500 cap | +|---|---|---| +| Before this change (`origin/main`) | 454 | 46 | +| After this change | 479 | 21 | + +The 25-line increase matches exactly the 25 insertions reported by +`git diff --numstat origin/main` in P1-T13, so no line was added to the file outside the new +test method. + +## Companion Check — Production File + +The same 500-line cap applies to the production file this item also changes. It is well +inside the limit and is recorded here for completeness: + +Command: `(Get-Content 'QuickFiler\Controllers\QfcHomeController.Metrics.cs').Count` +EXIT_CODE: 0 +Output: `231` (was 227 before the four-line guard; 269 lines of headroom) + +## Forward Note + +At 479 lines the test file has 21 lines of headroom. A future addition of another test of +comparable size (the one added here cost 25 lines including its documentation comment) would +breach the cap and require the file to be split first. That is a note for whoever next adds +to this file; it is out of scope for issue #646 and no split is performed here. diff --git a/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/coverage-artifact-substitution.2026-09-01T16-41.md b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/coverage-artifact-substitution.2026-09-01T16-41.md new file mode 100644 index 000000000..f19802f33 --- /dev/null +++ b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/coverage-artifact-substitution.2026-09-01T16-41.md @@ -0,0 +1,201 @@ +# Evidence Correction — Coverage Artifact Substitution (Raw Cobertura to JaCoCo Projection) + +Timestamp: 2026-09-01T16-41 + +Branch: `bug/qfc-metrics-flush-writes-empty-session-file-646` +HEAD at substitution: `e23a9afc` + +## Why + +Two committed evidence artifacts were raw `dotnet-coverage` Cobertura reports totalling +52,131,269 bytes and 892,256 lines. Raw Cobertura must not be committed as evidence in this +repository. The precedent is commit `d0955dc4` ("docs(#503): replace raw cobertura coverage +evidence with jacoco summaries"), which removed approximately 20 MB of exactly this artifact +class and replaced it with compact package-level JaCoCo files. + +Each raw report was replaced by a package-level JaCoCo projection that preserves every figure +the plan's gates relied on. + +## Command + +Command: `pwsh -NoProfile -File Convert-CoberturaToJacoco.ps1 -InputPath docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/baseline-coverage.cobertura.xml -OutputPath docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/baseline-coverage.jacoco.xml` +EXIT_CODE: 0 + +Command: `pwsh -NoProfile -File Convert-CoberturaToJacoco.ps1 -InputPath docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/final-coverage.cobertura.xml -OutputPath docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/final-coverage.jacoco.xml` +EXIT_CODE: 0 + +`Convert-CoberturaToJacoco.ps1` was written to the session scratchpad outside the repository +and is not committed, per the repository rule that no helper script is written under +`evidence/`. Its path is account-derived and is therefore not reproduced here. The conversion +is fully specified by the method below and is reproducible from it. + +Output Summary: Both conversions succeeded with EXIT_CODE 0 and both reconciled exactly to +their source Cobertura root counters — baseline `lines-covered=48426 lines-valid=142226`, +final `lines-covered=48436 lines-valid=142240`. Each projection carries all 15 packages. The +two raw `.cobertura.xml` files were deleted only after both reconciliations passed. Total +committed evidence for these two artifacts falls from 52,131,269 bytes to 4,718 bytes with no +loss of any figure a plan gate relied on. + +## Method + +The source files are approximately 26 MB each and were streamed with `System.Xml.XmlReader`; +neither was loaded into a DOM. + +1. Track the current `package` name. The `package` element places `name` after `line-rate`, + so a search for the literal text `package name=` returns zero matches in these files and + would falsely suggest the reports contain no packages. `XmlReader.GetAttribute` reads + attributes by name irrespective of their order, so the ordering is immaterial here. +2. Within each `class` element, collect `line` elements into a map keyed by the `number` + attribute. Cobertura repeats `line` elements across the `method` blocks and the + class-level `lines` block, so deduplication by line number within the class is required or + the totals do not reconcile. Where a line number recurs, the maximum `hits` is kept. +3. On closing each `class`, fold the deduplicated entries into the package counters: + `hits > 0` counts as covered, `hits == 0` counts as missed. +4. Branch counters are derived from the `(covered/total)` pair inside each + `condition-coverage` attribute. + +### Branch Counters Are Zero, and That Is Faithful + +Every `BRANCH` counter in both projections reads `missed="0" covered="0"`. This is a true +projection of the source, not a parsing miss. Verified on the source before deletion: + +- `grep -c 'condition-coverage' baseline-coverage.cobertura.xml` returned `0` (exit 1). +- `grep -o 'branch="True"' baseline-coverage.cobertura.xml` returned no output. +- The Cobertura root carries `branch-rate="1"` with no `branches-covered` and no + `branches-valid` attribute, as already recorded in + `evidence/baseline/coverage-cobertura-baseline.2026-08-31T20-04.md`. + +The run emitted no branch data at all, so no branch figure exists to project. The counters are +retained at zero rather than omitted so the JaCoCo shape stays uniform across packages. + +## Reconciliation — Mandatory Gate, Passed + +The summed `LINE` counters across all packages in each projection must reproduce the source +Cobertura root element's `lines-covered` and `lines-valid` attributes exactly. Both did. No +number was adjusted to make this match, and neither source file was deleted until its +reconciliation passed. + +| Report | Measure | Cobertura root | Derived from projection | Match | +|---|---|---|---|---| +| Baseline | `lines-covered` | 48426 | 48426 | Exact | +| Baseline | `lines-valid` | 142226 | 142226 | Exact | +| Final | `lines-covered` | 48436 | 48436 | Exact | +| Final | `lines-valid` | 142240 | 142240 | Exact | + +As a secondary confirmation, the ratio derived from each projection reproduces the source root +`line-rate` to ten decimal places: baseline `0.3404862683` against a root `line-rate` of +`0.3404862683334974`, and final `0.3405230596` against `0.3405230596175478`. + +## Files Replaced + +| File | Bytes | Lines | +|---|---|---| +| `evidence/baseline/baseline-coverage.cobertura.xml` (deleted) | 26,064,187 | 446,104 | +| `evidence/qa-gates/final-coverage.cobertura.xml` (deleted) | 26,067,082 | 446,152 | +| **Source total** | **52,131,269** | **892,256** | +| `evidence/baseline/baseline-coverage.jacoco.xml` (added) | 2,359 | 62 | +| `evidence/qa-gates/final-coverage.jacoco.xml` (added) | 2,359 | 62 | +| **Replacement total** | **4,718** | **124** | + +Line counts for the two Cobertura files are the true line counts. Neither file ended with a +newline — the final byte of each was `>` — so `wc -l` reported one fewer line for each +(446,103 and 446,151) than the file actually contains. + +## Sequence — The Gates Ran Against the Raw Reports and Were Not Skipped + +An auditor reading this feature folder will find plan tasks that name a `.cobertura.xml` file +which is no longer present. That is a deliberate substitution performed after those tasks +completed, not a missing gate. The order of events was: + +1. **P0-T11** produced `baseline-coverage.cobertura.xml` and read its root attributes, + recording `line-rate="0.3404862683334974"`, `lines-covered="48426"`, + `lines-valid="142226"`. Its acceptance condition — that the artifact exists, parses, and + reports a numeric `line-rate` rather than a placeholder — was evaluated against the raw + file at that time and was satisfied. Recorded in + `evidence/baseline/coverage-cobertura-baseline.2026-08-31T20-04.md`. +2. **P2-T6** produced `final-coverage.cobertura.xml` by the identical + `dotnet-coverage merge -f cobertura` invocation and read its root attributes, recording + `line-rate="0.3405230596175478"`, `lines-covered="48436"`, `lines-valid="142240"`. Its + acceptance condition was evaluated against the raw file at that time and was satisfied. + Recorded in `evidence/qa-gates/coverage-cobertura-final.2026-08-31T20-04.md`. +3. **P2-T7** read both raw files, compared the two `line-rate` values, and read the per-line + `hits` entries for the four new guard lines out of the raw final report. Both of its + acceptance conditions were evaluated against the raw files at that time. Recorded in + `evidence/qa-gates/coverage-delta-verification.2026-08-31T20-04.md`. +4. **Only then** — after all three tasks had completed and their conditions had been satisfied + against the raw reports — were the two raw files converted to the projections described + above and deleted. + +Every figure those three tasks recorded remains verifiable. The package-level `LINE` totals in +the projections reconcile exactly to the root counters the tasks quoted, so the headline +figures can be re-derived from the committed evidence. The one class of detail the projection +does not retain is per-line and per-class granularity, which affects only Part 2 of the P2-T7 +artifact; the per-line `hits` values that Part 2 depends on are quoted verbatim in that +artifact, together with the corroborating `EfcHomeController.Metrics.cs` precedent read from +the same report. + +The conversion is lossless with respect to the LINE counters and lossy only with respect to +per-class and per-line detail that no gate's acceptance condition reads from the file after +the fact. + +## Incidental Benefit — Host Path Removal + +The JaCoCo projection carries no `filename` attribute. Dropping it also removes the vendored +build-machine source paths that the Cobertura files carried on their `class` elements, which +originated on third-party build agents rather than in this repository. + +## Denominator Scope Statement + +This statement applies to every coverage figure in this feature folder and is repeated here so +it travels with the artifact record. + +The `line-rate` of approximately 34.05% recorded at both baseline and final is measured on a +**single-assembly unfiltered denominator (QuickFiler.Test run; includes vendored and test +assemblies)**. It is neither a repository-wide figure nor the repository's policy coverage +figure. + +Both reports contain 15 packages. Eight are vendored third-party assemblies — `Deedle`, +`FluentAssertions`, `FSharp.Core`, `log4net`, `Microsoft.IO.RecyclableMemoryStream`, +`Mono.Reflection`, `System.Interactive`, `System.Linq.Async` — and a ninth is the +`QuickFiler.Test` test assembly itself. Only six are first-party production packages. +Separately, only `QuickFiler.Test.dll` was executed, so assemblies from the rest of the +solution sit in the denominator with no test driving them. The repository's policy denominator +(`coverage.config`, and the `CLAUDE.md` UT2 testable-denominator rule) is nine first-party +packages with no `*.Test` assembly. + +Two consequences: + +- **The no-regression comparison remains valid.** Baseline and final were produced by the + identical `dotnet-coverage merge -f cobertura` invocation over the identical assembly set, + so the delta is apples-to-apples and a regression in first-party code would still move the + figure down. +- **The absolute magnitude must not be quoted as a policy figure.** It evidences neither + compliance with nor breach of any coverage floor. Establishing a true policy figure would + require a full-suite coverage pass, which is out of scope for a four-line guard and on which + none of this plan's gates depend. + +### First-Party Subset of This Same Run + +Derived from the two projections by excluding the eight vendored packages and +`QuickFiler.Test`, leaving `QuickFiler`, `UtilitiesCS`, `ToDoModel`, `TaskVisualization`, +`Tags`, and `SVGControl`: + +| Measure | Baseline | Final | Delta | +|---|---|---|---| +| first-party `lines-covered` | 14537 | 14540 | +3 | +| first-party `lines-valid` | 62118 | 62121 | +3 | +| first-party line coverage | 23.4022% | 23.4059% | +0.0037 pp | + +This is a first-party subset of a single-assembly run, not a repository-wide policy figure: +only `QuickFiler.Test` was executed, and three of the six first-party packages (`ToDoModel`, +`TaskVisualization`, `Tags`) report zero covered lines for exactly that reason. It is recorded +so the delta can be read against first-party code alone, and it shows no regression. + +## Effect on the Recorded Change Footprint + +`evidence/qa-gates/footprint-scope.2026-08-31T20-04.md` recorded 29 diff paths at HEAD +`ba134b57`. This correction pass changes that count: it deletes two paths, adds two +projections, adds this record, and modifies five existing artifacts plus `issue.md`. Every +path involved remains inside the third AC7-allowed prefix, +`docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/`, so the AC7 +boundary is unchanged. No production or test source file was touched by this pass. diff --git a/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/coverage-cobertura-final.2026-08-31T20-04.md b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/coverage-cobertura-final.2026-08-31T20-04.md new file mode 100644 index 000000000..79147757e --- /dev/null +++ b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/coverage-cobertura-final.2026-08-31T20-04.md @@ -0,0 +1,81 @@ +# QA Gate — Cobertura Coverage Headline, Final (P2-T6) + +Timestamp: 2026-09-01T12-59 + +Referenced artifact: `evidence/qa-gates/final-coverage.cobertura.xml` + +## Discovery of the .coverage Input + +Command: +`Get-ChildItem -Path TestResults -Filter *.coverage -Recurse | Sort-Object LastWriteTime -Descending | Select-Object -First 1` +EXIT_CODE: 0 +Selected input `LastWriteTime`: `2026-09-01T12:27:38.3748288-04:00`, which matches the P2-T5 +run and is distinct from the P0-T10 baseline input (`12:14:55`), confirming the correct, +newer artifact was selected rather than the baseline being re-read. + +## Conversion + +Command: +`dotnet-coverage merge -f cobertura -o docs\features\active\2026-08-27-qfc-metrics-flush-writes-empty-session-file-646\evidence\qa-gates\final-coverage.cobertura.xml ` +EXIT_CODE: 0 +Tool version reported: `dotnet-coverage v18.5.2.0 [win-x64 - .NET 10.0.11]` — the same +version used for the baseline conversion, so baseline and final are directly comparable. +Output: `Merged into file ...\evidence\qa-gates\final-coverage.cobertura.xml.` + +## Final Coverage Headline (verbatim root `` attribute values) + +| Attribute | Value | +|---|---| +| `line-rate` | `0.3405230596175478` | +| `branch-rate` | `1` | +| `lines-covered` | `48436` | +| `lines-valid` | `142240` | + +`line-rate` is a numeric string, not a placeholder, satisfying the task acceptance +condition. As a percentage the final repository-wide figure is **34.05%**. + +The same two qualifications recorded at baseline still apply and are unchanged: the +denominator includes vendored and third-party assemblies loaded by the test run and is not +the first-party testable denominator `CLAUDE.md` UT2 defines its floor against; and +`branch-rate="1"` carries no `branches-covered` or `branches-valid` attributes at all, so no +branch data was emitted and the value is not interpretable. + +## Acceptance + +| Condition | Required | Observed | Met | +|---|---|---|---| +| The `.cobertura.xml` artifact exists | yes | yes, 26,067,082 bytes, reparsed successfully as `[xml]` | Yes | +| Its root `line-rate` is a numeric string, not a placeholder | yes | `0.3405230596175478` | Yes | + +ACCEPTANCE: MET. + +## Sanitisation Micro-Action Recorded + +As with the baseline artifact, `dotnet-coverage` wrote absolute paths into every `` attribute. The absolute worktree prefix was removed by literal string +replacement, rendering all paths repository-relative. + +| Check | Result | +|---|---| +| Occurrences of the absolute worktree prefix replaced | 3255 | +| Residual occurrences of the account name | 0 | +| Residual occurrences of the machine name | 0 | +| Residual occurrences of `C:\Users` | 0 | +| XML still well-formed after replacement | Yes (reparsed as `[xml]`) | +| Root `line-rate` after replacement | `0.3405230596175478` (unchanged) | + +No angle-bracket placeholder was substituted into any XML attribute; the prefix was removed +rather than replaced with a token. + +The occurrence count rose from 3253 at baseline to 3255 here. Both added occurrences are +`` elements for the same two compiler-generated state-machine types that the guard's +early `return;` introduced into `QfcHomeController.Metrics.cs`'s coverage output; they are a +consequence of the change, not of the sanitisation. + +## Output Summary + +Final repository-wide `line-rate` is `0.3405230596175478` (34.05%) over 48,436 of 142,240 +lines, produced by the same tool version and the same method as the baseline. The artifact +exists, parses, reports a numeric `line-rate`, and contains no absolute host path. The +baseline-to-final delta and the per-line hit counts for the four new guard lines are +evaluated in P2-T7. diff --git a/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/coverage-delta-verification.2026-08-31T20-04.md b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/coverage-delta-verification.2026-08-31T20-04.md new file mode 100644 index 000000000..8c2cadd8e --- /dev/null +++ b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/coverage-delta-verification.2026-08-31T20-04.md @@ -0,0 +1,208 @@ +# QA Gate — Coverage Delta Verification (P2-T7) + +Timestamp: 2026-09-01T13-02 + +Inputs: +- Baseline: `evidence/baseline/baseline-coverage.cobertura.xml` (from P0-T11) +- Final: `evidence/qa-gates/final-coverage.cobertura.xml` (from P2-T6) + +Both were produced by `dotnet-coverage v18.5.2.0` using the identical `merge -f cobertura` +invocation, so the two figures are comparable and the delta below is not an artifact of +differing measurement methods. + +**Artifact substitution note.** This task read the two raw `.cobertura.xml` files above at +execution time, and every figure in this document was derived from them then. After this +task completed, those two files were replaced by lossless package-level JaCoCo projections +(`baseline-coverage.jacoco.xml`, `final-coverage.jacoco.xml`) in the same directories, +because raw Cobertura must not be committed as evidence in this repository. The projections +reconcile exactly to the Cobertura root `lines-covered` and `lines-valid` on both sides. See +`evidence/qa-gates/coverage-artifact-substitution.2026-09-01T16-41.md` for the conversion +command, the reconciliation integers, and the full sequence. + +--- + +## Part 1 — No-Regression on the Single-Assembly Unfiltered Denominator + +### What This Denominator Is, and What It Is Not + +The `line-rate` figures below are measured on a **single-assembly unfiltered denominator +(QuickFiler.Test run; includes vendored and test assemblies)**. They are not a +repository-wide figure and not the repository's policy coverage figure. + +The `` set in both reports contains 15 packages. Eight are vendored third-party +assemblies — `Deedle`, `FluentAssertions`, `FSharp.Core`, `log4net`, +`Microsoft.IO.RecyclableMemoryStream`, `Mono.Reflection`, `System.Interactive`, +`System.Linq.Async` — and a ninth is the `QuickFiler.Test` test assembly itself. Only six +are first-party production packages. Separately, the run exercised only +`QuickFiler.Test.dll`, so assemblies belonging to the rest of the solution sit in the +denominator while no test drives them. + +The repository's policy denominator — `coverage.config`, and the `CLAUDE.md` UT2 +testable-denominator rule — is nine first-party packages with no `*.Test` assembly. The +34.05% figure is therefore neither the policy figure nor a repository-wide figure, and it +must not be quoted as one. + +Two consequences, stated plainly: + +- **The no-regression COMPARISON remains valid.** Baseline and final were produced by the + identical `dotnet-coverage merge -f cobertura` invocation over the identical assembly set, + so the delta is apples-to-apples. Whatever the denominator contains, it contains the same + thing on both sides, and a regression in first-party code would still move the figure down. +- **The absolute magnitude is not a policy result.** 34.05% does not evidence compliance + with, or breach of, any coverage floor in `CLAUDE.md` UT2 or + `.claude/rules/general-unit-test.md`. No such claim is made here. Obtaining a true policy + figure would require a full-suite coverage pass, which is out of scope for a four-line + guard and which none of this plan's gates depend on. + +| Measure | Baseline (P0-T11) | Final (P2-T6) | Delta | +|---|---|---|---| +| root `line-rate` | `0.3404862683334974` | `0.3405230596175478` | **+0.0000367912840504** | +| as a percentage | 34.0486% | 34.0523% | +0.0037 pp | +| `lines-covered` | 48426 | 48436 | +10 | +| `lines-valid` | 142226 | 142240 | +14 | + +**The final value is not lower than the baseline value. It is higher.** + +Condition 1: **MET** — no coverage regression on the single-assembly unfiltered denominator +(QuickFiler.Test run; includes vendored and test assemblies). + +The +10 covered lines and +14 valid lines are the guard's three measurable statements plus +the additional compiler-generated state-machine lines the early `return;` introduces into +`WriteMetricsAsync`, now exercised by the new test. + +### First-Party Subset of This Same Run + +The figures above include vendored and test assemblies. Excluding the eight vendored +packages and `QuickFiler.Test` leaves the six first-party packages present in this run +(`QuickFiler`, `UtilitiesCS`, `ToDoModel`, `TaskVisualization`, `Tags`, `SVGControl`). +Derived from the JaCoCo projections of the same two reports: + +| Measure | Baseline | Final | Delta | +|---|---|---|---| +| first-party `lines-covered` | 14537 | 14540 | +3 | +| first-party `lines-valid` | 62118 | 62121 | +3 | +| first-party line coverage | 23.4022% | 23.4059% | +0.0037 pp | + +This is a **first-party subset of this single-assembly run**. It is still not a +repository-wide policy figure, because only `QuickFiler.Test` was executed: the five +first-party packages other than `QuickFiler` have no test driving them in this run, and +three of them (`ToDoModel`, `TaskVisualization`, `Tags`) report zero covered lines for +exactly that reason. The subset is recorded so the delta can be read against first-party +code alone; it moves in the same direction as the full figure and likewise shows no +regression. + +--- + +## Part 2 — Coverage of the Four New Guard Lines + +The guard occupies post-fix lines **175-178** of +`QuickFiler/Controllers/QfcHomeController.Metrics.cs`, as re-derived from the changed file in +P1-T6. Per-line entries were read from the final Cobertura XML by taking the union of `` +elements across all three `` +elements and deduplicating by line number, keeping the maximum `hits` (the async state machine +reports some lines under more than one `` element). + +| Line | Source | `` entry present | `hits` | +|---|---|---|---| +| 174 | `var lines = strOutput.Where(...).ToArray();` (Anchor A, unchanged) | Yes | 1 | +| **175** | `if (lines.Length == 0)` | **Yes** | **1** | +| **176** | `{` | **Yes** | **1** | +| **177** | `return;` | **Yes** | **1** | +| **178** | `}` | **No entry emitted** | n/a | +| 183 | `bool metricsWritten = await MetricsFileWriter(` (Anchor B, unchanged) | Yes | 1 | + +Three of the four guard lines carry `hits=1`. The fourth, line 178, has **no `` element +at all** in the report, so no `hits` value exists for it to be compared against zero. + +### Why Line 178 Has No Entry — Verified, Not Assumed + +A closing brace that terminates a block whose last statement is `return;` produces no +separate IL sequence point, so the instrumenter emits no `` element for it. This is a +property of the coverage format, not an uncovered line. + +That explanation was **verified against the repository's own precedent** rather than +asserted. `QuickFiler/Controllers/EfcHomeController.Metrics.cs` contains the identical +construct at lines 72-75 — `if (dataLines.Length == 0) { return; }` — the very guard AC2 +requires textual equivalence with. Reading the same final Cobertura report for that file: + +``` + LINE 71 hits=1 + LINE 72 hits=1 <- if (dataLines.Length == 0) + LINE 73 hits=1 <- { + LINE 74 hits=1 <- return; + LINE 75 NO-ENTRY <- } + LINE 76 NO-ENTRY <- (blank line) + LINE 77 hits=1 +``` + +The EFC guard is pre-existing, fully exercised code that predates this item, and its closing +brace likewise has no entry. The two guards produce byte-identical coverage shapes. Line 178 +is therefore not an uncovered line in the new guard; it is a line the instrument does not +measure. + +A contrasting case in the same file confirms the mechanism is specific to blocks ending in +`return;` rather than to closing braces generally: the pre-existing `if (!metricsWritten)` +block at lines 189-195, whose body falls through rather than returning, **does** get an entry +for its closing brace at line 195 (`hits=0`). + +### Result + +| Basis | Covered | Total | Percentage | +|---|---|---|---| +| Guard lines that the instrument measures | 3 | 3 | **100%** | +| All four guard source lines, counting the unmeasurable brace as uncovered | 3 | 4 | 75% | + +The measurable basis is the correct one: a line that emits no sequence point cannot be +covered by any test, so including it in the denominator would make 100% unreachable for this +construct — including for the EFC guard that the repository already ships and that this +change was required to mirror. + +Condition 2: **MET on the measurable basis** — every guard line the instrument reports has +`hits=1`, giving 100% coverage of the new guard and satisfying the `CLAUDE.md` UT2 >= 90% +new-code floor, which is the substantive requirement this task's acceptance text names. + +**Deviation recorded for audit:** the task's literal wording asks that "each of the four +lines has `hits` greater than `0`". That literal condition is not satisfiable for line 178 by +any implementation of this guard, because no `hits` value is emitted for it. This is reported +rather than papered over. No checkbox was force-checked on this basis and no test was +weakened to manufacture a hit. + +--- + +## Part 3 — Supporting Detail: Coverage of the Changed File + +| Measure | Baseline | Final | Delta | +|---|---|---|---| +| Distinct lines measured in `QfcHomeController.Metrics.cs` | 122 | 125 | +3 | +| Distinct lines covered | 94 | 97 | +3 | +| File-level percentage | 77.05% | 77.60% | +0.55 pp | +| `` `line-rate` | 0.6986301369863014 | 0.6986301369863014 | none | +| `c>` `line-rate` | 1 | 1 | none | +| `d__103>` `line-rate` | 0.8775510204081632 | 0.8846153846153846 | +0.0070643642 | + +All three added lines are covered, so the file's coverage rose rather than being diluted. The +improvement is concentrated in the `d__103` async state machine, which is +the compiler-generated type that carries the changed method — exactly where the guard was +added. + +### No Regression on Changed Lines + +`CLAUDE.md` UT2 and `.claude/rules/general-unit-test.md` both require that changes not reduce +coverage for the lines that were changed. The only changed production lines are the four +guard lines, all newly added; there is no pre-existing line whose coverage could have fallen. +Every line that carried `hits=1` at baseline in this file still carries `hits=1` in the final +report, and every line that carried `hits=0` (the `if (!metricsWritten)` failure-branch body, +and the exception-handling region at old lines 213-224) still carries `hits=0`. No line moved +from covered to uncovered. + +--- + +## Acceptance Summary + +| # | Condition | Result | +|---|---|---| +| 1 | Final `line-rate` not lower than baseline `line-rate`, on the single-assembly unfiltered denominator (QuickFiler.Test run; includes vendored and test assemblies) — a like-for-like comparison, not a repository-wide or policy coverage figure | MET (`0.3405230596` vs `0.3404862683`, +0.0000367913) | +| 2 | Each of the four guard lines has `hits > 0` | MET on the measurable basis (3 of 3 reported lines at `hits=1`, 100%); line 178 emits no entry, verified structural against the identical EFC guard | + +ACCEPTANCE: MET, with the line-178 measurement limitation recorded in full above rather than +elided. diff --git a/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/csharpier-check-final.2026-08-31T20-04.md b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/csharpier-check-final.2026-08-31T20-04.md new file mode 100644 index 000000000..5728fc2b0 --- /dev/null +++ b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/csharpier-check-final.2026-08-31T20-04.md @@ -0,0 +1,37 @@ +# QA Gate — CSharpier Check, Final (P2-T2) + +Timestamp: 2026-09-01T12-53 + +Command: `dotnet tool run csharpier check .` +EXIT_CODE: 0 + +## Verbatim Output + +``` +Checked 1566 files in 4843ms. +``` + +## Acceptance + +| Condition | Required | Observed | Met | +|---|---|---|---| +| `EXIT_CODE` | `0` | `0` | Yes | + +ACCEPTANCE: MET. + +## Output Summary + +The read-only, CI-parity format verification passes. CSharpier 1.2.6 (manifest-pinned via +`dotnet-tools.json` and invoked through `dotnet tool run`, never a global install) checked +1566 files and printed no `Error` line for any file. In check mode CSharpier emits one +`Error ...` line per non-compliant file before the summary; none was printed, and the exit +code is 0. + +This is the read-only confirmation of the P2-T1 pass-2 fixpoint: the format gate is +independently observable here through the exit code, not only through the tree-comparison +that P2-T1 relies on. + +The file count (1566) is unchanged from the P0-T7 baseline, confirming this change added no +file to CSharpier's scope. The evidence artifacts this item writes are outside that scope by +`.csharpierignore`, which excludes `**/evidence/**` and `*.cobertura.xml`, so they cannot +perturb this gate. diff --git a/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/csharpier-format.2026-08-31T20-04.md b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/csharpier-format.2026-08-31T20-04.md new file mode 100644 index 000000000..a2eb380c0 --- /dev/null +++ b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/csharpier-format.2026-08-31T20-04.md @@ -0,0 +1,135 @@ +# QA Gate — CSharpier Format (P2-T1) + +Timestamp: 2026-09-01T12-52 + +This task ran twice. Pass 1 rewrote a tracked file, which under the Phase 2 rule ("if P2-T1 +rewrites any tracked file, restart the loop from P2-T1") required the loop to restart. Pass +2 reached a fixpoint. Both passes are recorded below. + +--- + +## Pass 1 + +### Pre-format snapshot + +Command: `git status --porcelain` +EXIT_CODE: 0 +Output: *(empty — working tree clean)* + +The Phase 1 edits to the two owned files were already committed at `2b633230`, so the +pre-format tree was clean. This makes the comparison sharper than a dirty-tree snapshot +would: any path the formatter rewrites appears immediately, with nothing to disentangle it +from. + +### Format + +Command: `dotnet tool run csharpier format .` +EXIT_CODE: 0 +Verbatim output: `Formatted 1566 files in 6244ms.` + +### Post-format snapshot + +Command: `git status --porcelain` +EXIT_CODE: 0 +Output: + +``` + M QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs +``` + +Command: `git diff --stat HEAD` +Output: `QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs | 4 +---` +(1 insertion, 3 deletions) + +### Difference from the pre-format snapshot + +**Yes — the formatter rewrote one tracked file.** One newly-modified path appeared that was +not present before the format run. The rewrite: + +``` +- invoked +- .Should() +- .BeFalse("an empty filtered array must not reach the writer at all"); ++ invoked.Should().BeFalse("an empty filtered array must not reach the writer at all"); +``` + +CSharpier collapsed the chained assertion in the new test onto one line. The test was +written by modelling `WriteMetricsAsync_WithoutMyDocumentsFolder_DoesNotInvokeWriter`, whose +equivalent assertion CSharpier keeps broken across three lines. The two differ only in the +length of the `because` reason string: the template's reason is long enough to push the +chain past the print width, and this one's is not, so the same formatter produces a +different shape for the same construct. Formatter output wins over the hand-written form +(`CLAUDE.md` C#1.1: "Do not hand-format; if a diff disagrees with `csharpier`, formatter +output wins"), and the collapsed line was kept. + +No file outside `QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs` was touched. +In particular the production file `QuickFiler/Controllers/QfcHomeController.Metrics.cs` was +not rewritten, so the four-line guard as written in P1-T5 is already formatter-canonical. + +**Loop restarted from P2-T1.** + +--- + +## Pass 2 + +### Pre-format snapshot + +Command: `git status --porcelain` +EXIT_CODE: 0 +Output: + +``` + M QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs +``` + +Command: `git diff --stat HEAD` +Output: `QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs | 4 +---` +(1 insertion, 3 deletions — the uncommitted pass-1 rewrite) + +### Format + +Command: `dotnet tool run csharpier format .` +EXIT_CODE: 0 +Verbatim output: `Formatted 1566 files in 2151ms.` + +### Post-format snapshot + +Command: `git status --porcelain` +EXIT_CODE: 0 +Output: + +``` + M QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs +``` + +Command: `git diff --stat HEAD` +Output: `QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs | 4 +---` +(1 insertion, 3 deletions) + +### Difference from the pre-format snapshot + +**No difference.** The modified-path set is identical before and after, and the changed-line +count against `HEAD` is identical before and after (1 insertion, 3 deletions in both +snapshots). No newly-modified path appeared and no changed-line count grew. The single +modified path carried into this pass is pass 1's rewrite, not a pass-2 rewrite. + +The tree was therefore already formatter-compliant when pass 2 began. This is the fixpoint +the restart rule exists to establish. + +## Acceptance + +| Condition | Observed | Met | +|---|---|---| +| `EXIT_CODE 0` recorded | Pass 1: 0. Pass 2: 0. | Yes | +| Task records whether the second `git status --porcelain` shows any additional changed-line count for the owned files, or any newly-modified path, beyond the pre-format snapshot | Pass 1: yes, it did (one newly-modified path, detailed above). Pass 2: no, it did not. | Yes | + +ACCEPTANCE: MET on pass 2, with pass 1 recorded as the rewrite that triggered the restart. + +## Output Summary + +CSharpier 1.2.6 processed 1566 files on each pass, exit 0 both times. Pass 1 collapsed one +chained FluentAssertions call in the new regression test onto a single line, which triggered +the mandated loop restart. Pass 2 changed nothing, confirming the tree is at the formatter's +fixpoint. The test file is 477 lines after formatting (down from 479 before it), still +within the 500-line cap verified in P1-T15; the two-line reduction is the collapsed +assertion and does not affect that task's acceptance. diff --git a/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/final-coverage.jacoco.xml b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/final-coverage.jacoco.xml new file mode 100644 index 000000000..7f340a08e --- /dev/null +++ b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/final-coverage.jacoco.xml @@ -0,0 +1,62 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/footprint-scope.2026-08-31T20-04.md b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/footprint-scope.2026-08-31T20-04.md new file mode 100644 index 000000000..580e86c1b --- /dev/null +++ b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/footprint-scope.2026-08-31T20-04.md @@ -0,0 +1,129 @@ +# QA Gate — Total Change Footprint (P2-T8) + +Timestamp: 2026-09-01T13-06 + +Branch: `bug/qfc-metrics-flush-writes-empty-session-file-646` +HEAD at check: `ba134b57` +`origin/main`: `8996b28746d32f9f5996a037e0ca76be78b7684d` + +Output Summary: Both footprint commands passed. `git status --porcelain` returned EXIT_CODE 0 with empty output (clean tree). `git diff origin/main --name-status` returned EXIT_CODE 0 listing 29 paths — 2 modified production/test files and 27 additions inside the feature folder. The mechanical inverse-prefix filter returned EXIT_CODE 1 with empty output, meaning zero paths fell outside the three AC7-allowed prefixes. ACCEPTANCE: MET. + +## The Allowed Set + +Per AC7 and the plan's Hard Scope Boundary 2, exactly three path prefixes are permitted: + +1. `QuickFiler/Controllers/QfcHomeController.Metrics.cs` +2. `QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs` +3. `docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/` + +## Command 1 — Working Tree Status + +Command: `git status --porcelain` +EXIT_CODE: 0 +Output: *(empty)* + +The working tree is clean. No modified, staged, or untracked path exists at all, so no path +from this command can fall outside the allowed set. + +## Command 2 — Diff Against origin/main + +Command: `git diff origin/main --name-status` +EXIT_CODE: 0 +Path count: **29** + +| Status | Path | +|---|---| +| M | `QuickFiler/Controllers/QfcHomeController.Metrics.cs` | +| M | `QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs` | +| A | `docs/features/active/2026-08-27-.../issue.md` | +| A | `docs/features/active/2026-08-27-.../plan.2026-08-31T20-04.md` | +| A | `docs/features/active/2026-08-27-.../research/research.2026-08-31T20-30.md` | +| A | `docs/features/active/2026-08-27-.../research/research-correction.2026-08-31T20-45.md` | +| A | `docs/features/active/2026-08-27-.../evidence/baseline/baseline-coverage.cobertura.xml` | +| A | `docs/features/active/2026-08-27-.../evidence/baseline/branch-reconciliation.2026-08-31T20-04.md` | +| A | `docs/features/active/2026-08-27-.../evidence/baseline/coverage-cobertura-baseline.2026-08-31T20-04.md` | +| A | `docs/features/active/2026-08-27-.../evidence/baseline/csharpier-check.2026-08-31T20-04.md` | +| A | `docs/features/active/2026-08-27-.../evidence/baseline/msbuild-analyzer-rebuild.2026-08-31T20-04.md` | +| A | `docs/features/active/2026-08-27-.../evidence/baseline/msbuild-nullable-rebuild.2026-08-31T20-04.md` | +| A | `docs/features/active/2026-08-27-.../evidence/baseline/phase0-instructions-read.md` | +| A | `docs/features/active/2026-08-27-.../evidence/baseline/vstest-coverage-run.2026-08-31T20-04.md` | +| A | `docs/features/active/2026-08-27-.../evidence/other/anchor-rederivation.2026-08-31T20-04.md` | +| A | `docs/features/active/2026-08-27-.../evidence/other/production-diff-scope.2026-08-31T20-04.md` | +| A | `docs/features/active/2026-08-27-.../evidence/other/test-file-diff-scope.2026-08-31T20-04.md` | +| A | `docs/features/active/2026-08-27-.../evidence/other/test-file-line-count.2026-08-31T20-04.md` | +| A | `docs/features/active/2026-08-27-.../evidence/regression-testing/existing-tests-pass.2026-08-31T20-04.md` | +| A | `docs/features/active/2026-08-27-.../evidence/regression-testing/fail-before-new-test.2026-08-31T20-04.md` | +| A | `docs/features/active/2026-08-27-.../evidence/regression-testing/pass-after-new-test.2026-08-31T20-04.md` | +| A | `docs/features/active/2026-08-27-.../evidence/qa-gates/coverage-cobertura-final.2026-08-31T20-04.md` | +| A | `docs/features/active/2026-08-27-.../evidence/qa-gates/coverage-delta-verification.2026-08-31T20-04.md` | +| A | `docs/features/active/2026-08-27-.../evidence/qa-gates/csharpier-check-final.2026-08-31T20-04.md` | +| A | `docs/features/active/2026-08-27-.../evidence/qa-gates/csharpier-format.2026-08-31T20-04.md` | +| A | `docs/features/active/2026-08-27-.../evidence/qa-gates/final-coverage.cobertura.xml` | +| A | `docs/features/active/2026-08-27-.../evidence/qa-gates/msbuild-analyzer-rebuild.2026-08-31T20-04.md` | +| A | `docs/features/active/2026-08-27-.../evidence/qa-gates/msbuild-nullable-rebuild.2026-08-31T20-04.md` | +| A | `docs/features/active/2026-08-27-.../evidence/qa-gates/vstest-coverage-run.2026-08-31T20-04.md` | + +(The feature-folder prefix `docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/` +is abbreviated to `docs/features/active/2026-08-27-.../` in this table for width. The +mechanical check below ran against the unabbreviated output.) + +Exactly two production/test files are modified. The other 27 paths are all additions inside +the feature folder. + +## Mechanical Out-Of-Set Check + +Rather than reading the list by eye, the diff output was filtered for any path *not* matching +one of the three allowed prefixes: + +Command: +`git diff origin/main --name-only | grep -v -E '^(QuickFiler/Controllers/QfcHomeController\.Metrics\.cs|QuickFiler\.Test/Controllers/QfcHomeControllerMetricsTests\.cs|docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/)'` +EXIT_CODE: 1 +Output: *(empty)* + +`grep` exits 1 when it matches nothing, so an empty result with exit 1 means **no path fell +outside the allowed set**. + +## Acceptance + +| Condition | Observed | Met | +|---|---|---| +| Every path listed by `git status --porcelain` begins with one of the three allowed prefixes | The command output is empty; there are no paths to check | Yes (vacuously, and by the stronger condition that the tree is clean) | +| Every path listed by `git diff origin/main --name-status` begins with one of the three allowed prefixes | All 29 paths do; the inverse filter returns zero results | Yes | + +ACCEPTANCE: MET — no listed path falls outside the allowed set. + +## Residue That Did Not Appear, and Why + +Several tools run during this item write into the worktree. None reached the footprint, +because each target is covered by a **pre-existing** `.gitignore` entry. No `.gitignore` entry +was added, modified, or removed by this item — `.gitignore` does not appear in the diff above, +which is itself the proof, since hiding residue behind a new ignore rule would have been an +out-of-set file change and would show here. + +| Residue | Written by | Pre-existing `.gitignore` rule | Verified with | +|---|---|---|---| +| `TestResults/` (two runs, with `.coverage` attachments) | P0-T10, P2-T5 vstest runs | line 39, `[Tt]est[Rr]esult*/` | `git check-ignore -v TestResults/` | +| `packages/` (172 restored packages) | `nuget restore` precondition for P0-T8 | line 358, `packages/` | `git check-ignore -v packages/` | +| `.dotnet-sdk/` (repo-local SDK 8.0.205) | `Install-RepoDotNetSdk.ps1` precondition for P0-T7 | line 350, `.dotnet*/` | `git check-ignore -v .dotnet-sdk/` | +| `bin/`, `obj/` across all projects | four solution-wide `/t:Rebuild` runs and two project-level rebuilds | pre-existing standard entries | clean `git status --porcelain` | + +No stray `coverage.xml` was left at the repository root, and no scratch script was written +into the worktree: all helper scripts for this execution were written to the session scratchpad +outside the repository. + +## Paths Deliberately Not Touched + +| Path | Why it stayed out | +|---|---| +| `artifacts/orchestration/orchestrator-state.json` | Tracked, and carries `--skip-worktree` in this worktree's index from the orchestrator. Plan Hard Scope Boundary 6 forbids touching it and forbids running `git update-index`. Neither was done, and it does not appear in the diff. | +| `.claude/agent-memory/` | Writing here would place an out-of-set path in the diff and break AC7. Nothing was written to it during this execution. | +| `QuickFiler/Controllers/EfcHomeController.Metrics.cs` | Read-only reference under Hard Scope Boundary 3. It was read three times (for AC2's textual-equivalence comparison, and in P2-T7 to verify the closing-brace coverage behavior) and never written. | +| `.gitignore` | Adding an entry to suppress build residue would itself be an out-of-set change. | + +## Note on This Artifact + +This artifact is written after the check it records, so it is untracked at the moment of +writing. Its path, +`docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/footprint-scope.2026-08-31T20-04.md`, +begins with the third allowed prefix, so its own later appearance in `git status --porcelain` +and in the `origin/main` diff does not violate the condition this task checks. diff --git a/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/msbuild-analyzer-rebuild.2026-08-31T20-04.md b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/msbuild-analyzer-rebuild.2026-08-31T20-04.md new file mode 100644 index 000000000..83db07ddb --- /dev/null +++ b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/msbuild-analyzer-rebuild.2026-08-31T20-04.md @@ -0,0 +1,56 @@ +# QA Gate — MSBuild Analyzer Rebuild, Final (P2-T3) + +Timestamp: 2026-09-01T12-55 + +Command: `msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true` +EXIT_CODE: 0 +Output Summary: `Build succeeded.` with 5 Warning(s) and 0 Error(s), unchanged from the P0-T8 baseline (5 warnings, 0 errors). Zero new analyzer diagnostics; all 5 warnings are the pre-existing `_RxCheckPackagesConfig` MSBuild warning. 36 `csc.exe` command-line occurrences in the log, matching baseline, confirming `CoreCompile` ran on every project so the gate was capable of failing. + +## Verbatim Printed Summary Lines + +``` +Build succeeded. + + 5 Warning(s) + 0 Error(s) +``` + +## Acceptance + +| Condition | Required | Observed | Met | +|---|---|---|---| +| `EXIT_CODE` | `0` | `0` | Yes | +| Printed summary line reads `Build succeeded.` | yes | `Build succeeded.` | Yes | + +ACCEPTANCE: MET. + +## Comparison Against Baseline + +| Measure | Baseline (P0-T8) | Final (P2-T3) | Delta | +|---|---|---|---| +| `EXIT_CODE` | 0 | 0 | none | +| Summary line | `Build succeeded.` | `Build succeeded.` | none | +| Warnings | 5 | 5 | none | +| Errors | 0 | 0 | none | + +The change introduces **zero new analyzer diagnostics**. All 5 warnings are the same +pre-existing non-analyzer MSBuild warning from the `_RxCheckPackagesConfig` target in +`packages/System.Reactive.7.0.0/build/System.Reactive.PackagesConfigCheck.targets(31,5)`, +raised once each for `UtilitiesCS`, `ToDoModel`, `QuickFiler`, `TaskMaster`, and +`UtilitiesCS.Test`, exactly as at baseline. No Roslyn or .NET analyzer diagnostic +(`CAxxxx`, `IDExxxx`, `Sxxxx`, `RCSxxxx`, `MAxxxx`, `AsyncFixerxx`) appears in the output. + +This matters specifically for the guard added in P1-T5: an early `return;` inside an `async +Task` method is the shape that would attract an analyzer complaint if one applied, and none +was raised. + +## Non-Vacuity Check + +`/t:Rebuild` was used rather than `/t:Build`, as `CLAUDE.md` C#1.2 requires for a warm local +worktree: MSBuild's incremental up-to-date check does not invalidate on a command-line `/p:` +change, so a warm `/t:Build` returns exit 0 with `CoreCompile` skipped on every project and +runs no analyzers at all. + +The captured build log contains 36 `csc.exe` command-line occurrences, the same count as the +baseline run, confirming every project was genuinely recompiled and the analyzers genuinely +executed. The gate was capable of failing. diff --git a/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/msbuild-nullable-rebuild.2026-08-31T20-04.md b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/msbuild-nullable-rebuild.2026-08-31T20-04.md new file mode 100644 index 000000000..978777f18 --- /dev/null +++ b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/msbuild-nullable-rebuild.2026-08-31T20-04.md @@ -0,0 +1,82 @@ +# QA Gate — MSBuild Nullable Rebuild, Final (P2-T4) + +Timestamp: 2026-09-01T12-57 + +Command: `msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:TreatWarningsAsErrors=true` +EXIT_CODE: 0 +Output Summary: `Build succeeded.` with 5 Warning(s) and 0 Error(s), unchanged from the P0-T9 baseline, and 0 `CS86xx` occurrences in the log at both baseline and final. Under `/p:TreatWarningsAsErrors=true` any C# compiler warning introduced by this change would have been promoted to a build error; none was. 36 `csc.exe` command-line occurrences in the log, matching baseline, confirming compiler diagnostics were genuinely produced. + +## Verbatim Printed Summary Lines + +``` +Build succeeded. + + 5 Warning(s) + 0 Error(s) +``` + +## Acceptance + +| Condition | Required | Observed | Met | +|---|---|---|---| +| `EXIT_CODE` | `0` | `0` | Yes | +| Printed summary line reads `Build succeeded.` | yes | `Build succeeded.` | Yes | + +ACCEPTANCE: MET. + +## Comparison Against Baseline + +| Measure | Baseline (P0-T9) | Final (P2-T4) | Delta | +|---|---|---|---| +| `EXIT_CODE` | 0 | 0 | none | +| Summary line | `Build succeeded.` | `Build succeeded.` | none | +| Warnings | 5 | 5 | none | +| Errors | 0 | 0 | none | +| `CS86xx` occurrences in the log | 0 | 0 | none | + +The change introduces zero compiler warnings. Under `/p:TreatWarningsAsErrors=true`, any C# +compiler warning this change had introduced would have been promoted to a build error and +failed this gate; none was. The 5 surviving warnings are the pre-existing MSBuild +`_RxCheckPackagesConfig` warnings described in the P2-T3 artifact, which +`TreatWarningsAsErrors` does not promote because it sets a C# compiler property and these are +raised by an MSBuild `Warning` task. + +## Command Fidelity + +This is character-for-character the command in `.github/workflows/ci.yml` (step "Build with +nullable warnings treated as errors"). Per `CLAUDE.md` C#1.3, two properties of it were +preserved deliberately: + +- `/p:Nullable=enable` was **not** added. No project in this repository carries a `` + element and there is no `Directory.Build.props`, so that property is a solution-wide opt-in + that would conscript every file which has never adopted the pragma. CI omits it + deliberately. +- `/t:Build` was **not** used. MSBuild's up-to-date check does not invalidate on a + command-line `/p:` change, so a warm `/t:Build` returns exit 0 having skipped `CoreCompile` + on every project and the gate could not fail. + +## Non-Vacuity Check + +The captured log contains 36 `csc.exe` command-line occurrences, matching the baseline, +confirming `CoreCompile` ran on every project and compiler diagnostics were genuinely +produced. + +## Scope Qualification (recorded, not resolved) + +Neither file changed by this item carries a `#nullable enable` directive: + +Command: `grep -n "#nullable" QuickFiler/Controllers/QfcHomeController.Metrics.cs` — no match (exit 1) +Command: `grep -n "#nullable" QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs` — no match (exit 1) + +Nullable enforcement in this repository is per-file opt-in, so neither file participates in +nullable-flow analysis and this gate produced no `CS86xx` diagnostic about either. What this +gate does establish for this change is the stronger-than-nullable general condition that the +change introduces no C# compiler warning of any kind, since all such warnings are errors +here. + +Adding a `#nullable enable` pragma to either file would be an opt-in that the plan does not +authorize and that would expand this item's diff beyond the four-line guard and the one added +test, so none was added. This is a statement of the gate's actual reach, not a gap being +waived: the guard `if (lines.Length == 0) { return; }` operates on `lines`, a non-nullable +`string[]` produced by `.ToArray()`, and introduces no null-state question for a nullable +analysis to answer. diff --git a/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/vstest-coverage-run.2026-08-31T20-04.md b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/vstest-coverage-run.2026-08-31T20-04.md new file mode 100644 index 000000000..18a712965 --- /dev/null +++ b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/vstest-coverage-run.2026-08-31T20-04.md @@ -0,0 +1,85 @@ +# QA Gate — vstest.console.exe with Code Coverage, Final (P2-T5) + +Timestamp: 2026-09-01T12-58 + +## Resolution of vstest.console.exe + +Resolved via `vswhere`, the same resolution used in P0-T10: + +``` +$vswhere = Join-Path ${env:ProgramFiles(x86)} 'Microsoft Visual Studio\Installer\vswhere.exe' +$vstest = & $vswhere -latest -products * -find 'Common7\IDE\Extensions\TestPlatform\vstest.console.exe' | Select-Object -First 1 +``` + +## Command + +Command: `& $vstest QuickFiler.Test\bin\Debug\QuickFiler.Test.dll /EnableCodeCoverage` +EXIT_CODE: 0 +Output Summary: `Test Run Successful.` — Total tests 1285, Passed 1285, Failed 0, elapsed 13.8272 s. Baseline (P0-T10) was 1284 passed of 1284; the `+1` is exactly the regression test added by P1-T2, with no pre-existing test lost, skipped, or broken. Coverage was collected via `/EnableCodeCoverage`; the resulting `.coverage` attachment is converted and its numeric `line-rate` recorded in the P2-T6 and P2-T7 artifacts. + +This is the bare CUT3 form: the exact assembly path and the single `/EnableCodeCoverage` +flag, with no `/Settings`, no `/InIsolation`, and no `/TestCaseFilter` added. The authorized +host-failure fallback form was **not** needed and was **not** used; the run completed on the +first attempt with a genuine result. + +## Verbatim Printed Summary Lines + +``` +Test Run Successful. +Total tests: 1285 + Passed: 1285 + Total time: 13.8272 Seconds +``` + +## Acceptance + +| Condition | Required | Observed | Met | +|---|---|---|---| +| `EXIT_CODE` | `0` | `0` | Yes | +| Printed summary shows `Failed: 0` | zero failures | No `Failed:` line printed, and the count of per-test `Failed ` lines in the full log is `0`; `Passed: 1285` equals `Total tests: 1285` | Yes | +| `Total:` count >= the P0-T10 baseline total | `>= 1284` | `1285` | Yes | + +ACCEPTANCE: MET. + +Note on the `Failed: 0` wording: `vstest.console.exe` omits the `Failed:` line entirely +when the failed count is zero rather than printing it as `0`. The zero-failure condition is +therefore evidenced three ways here — the absence of that line, a per-test failure count of +0 across the whole log, and `Passed` being equal to `Total`. + +## Comparison Against Baseline + +| Measure | Baseline (P0-T10) | Final (P2-T5) | Delta | +|---|---|---|---| +| `EXIT_CODE` | 0 | 0 | none | +| Total tests | 1284 | 1285 | +1 | +| Passed | 1284 | 1285 | +1 | +| Failed | 0 | 0 | none | +| Elapsed | 13.93 s | 13.83 s | -0.10 s | + +The `+1` is exactly the regression test added by P1-T2. No pre-existing test was lost, +skipped, or broken: the total rose by precisely the number of tests added, and the failed +count stayed at zero. + +The new test was genuinely discovered and executed in this full-suite run, not only in the +scoped runs of P1-T4 and P1-T9: + +``` + Passed WriteMetricsAsync_WithAllNullOrWhitespaceDiagnostics_DoesNotInvokeWriter [< 1 ms] +``` + +All 9 `WriteMetricsAsync*` tests in the suite passed. + +## Coverage Attachment + +A single `.coverage` attachment was produced under a GUID-named subdirectory of +`TestResults/` at the repository root. Its filename is machine- and account-derived and is +therefore not reproduced here; P2-T6 locates it by recency. `TestResults/` is excluded from +version control by `.gitignore` line 39 (`[Tt]est[Rr]esult*/`), verified with +`git check-ignore -v`, so it does not enter the change footprint checked by P2-T8. + +## Loop Status + +P2-T1 through P2-T5 have now all completed with `EXIT_CODE 0` in a single uninterrupted +sequence, with no restart after the P2-T1 pass-2 fixpoint. This is the single clean final +toolchain pass required by `CLAUDE.md` General Code Change Policy section 8.1 and is the +evidence backing AC8. diff --git a/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/regression-testing/existing-tests-pass.2026-08-31T20-04.md b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/regression-testing/existing-tests-pass.2026-08-31T20-04.md new file mode 100644 index 000000000..6d7d99e3e --- /dev/null +++ b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/regression-testing/existing-tests-pass.2026-08-31T20-04.md @@ -0,0 +1,56 @@ +# Pre-Existing Metrics Tests Still Pass After the Guard (P1-T12) + +Timestamp: 2026-09-01T12-45 + +Production file state: guarded — the P1-T5 fix is applied. + +Command: +`& $vstest QuickFiler.Test\bin\Debug\QuickFiler.Test.dll /Settings:scripts\vscode\TaskMaster.cli.runsettings /InIsolation /TestCaseFilter:"FullyQualifiedName~WriteMetricsAsync_InvokesInjectedMetricsFileWriterOnce|FullyQualifiedName~WriteMetricsAsync_FiltersNullDiagnosticLinesBeforeWriting"` +EXIT_CODE: 0 + +## Verbatim Output + +``` + Passed WriteMetricsAsync_InvokesInjectedMetricsFileWriterOnce [260 ms] + Passed WriteMetricsAsync_FiltersNullDiagnosticLinesBeforeWriting [1 ms] + +Test Run Successful. +Total tests: 2 + Passed: 2 + Total time: 1.3594 Seconds +``` + +## Acceptance + +| Condition | Required | Observed | Met | +|---|---|---|---| +| `EXIT_CODE` | `0` | `0` | Yes | +| Printed summary shows `Passed: 2` | yes | ` Passed: 2` | Yes | + +ACCEPTANCE: MET. + +## Why These Two Tests Are the Correct Regression Check + +Both tests exercise the same `WriteMetricsAsync` path the guard was inserted into, with +**non-empty** filtered arrays, so they are the tests most exposed to an over-broad guard: + +- `WriteMetricsAsync_InvokesInjectedMetricsFileWriterOnce` supplies + `{ "line-one", "line-two" }`. After filtering, `lines.Length == 2`, so the new guard must + not fire and the writer must still be invoked exactly once. It passed, confirming the + guard does not suppress legitimate writes. +- `WriteMetricsAsync_FiltersNullDiagnosticLinesBeforeWriting` supplies + `{ "line-one", " ", null, "line-two" }` — a *partially* null/whitespace array. After + filtering, `lines.Length == 2`, so again the guard must not fire, and the writer must + receive exactly `{ "line-one", "line-two" }`. It passed, confirming the guard did not + turn the partial-filter case into an early return. + +Together these bound the guard's behavior from the other side: the new test (P1-T9) proves +it fires when the filtered array is empty, and these two prove it does not fire when the +filtered array is non-empty, including when the unfiltered input contained null and +whitespace entries. The boundary between the two cases is exactly `lines.Length == 0`. + +Both tests were genuinely executed (260 ms and 1 ms, with per-test `Passed` lines printed), +not skipped by the filter. + +Combined with P1-T13, which shows the test-file diff contains zero removed lines, this is +the evidence backing AC5. diff --git a/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/regression-testing/fail-before-new-test.2026-08-31T20-04.md b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/regression-testing/fail-before-new-test.2026-08-31T20-04.md new file mode 100644 index 000000000..ed02f4a9f --- /dev/null +++ b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/regression-testing/fail-before-new-test.2026-08-31T20-04.md @@ -0,0 +1,91 @@ +# Fail-Before — New Regression Test Against the Unguarded Implementation (P1-T4) + +Timestamp: 2026-09-01T12-35 + +Task: `[P1-T4]` `[expect-fail]` +Test: `WriteMetricsAsync_WithAllNullOrWhitespaceDiagnostics_DoesNotInvokeWriter` +Test file: `QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs` (added by P1-T2) +Production file state: **unguarded** — the P1-T5 fix has not yet been applied to +`QuickFiler/Controllers/QfcHomeController.Metrics.cs`. + +A failing result is the expected and required outcome of this task. + +## Step 1 — Rebuild the Test Project + +Command: +`msbuild QuickFiler.Test\QuickFiler.Test.csproj /t:Rebuild /m /p:Configuration=Debug /p:Platform=AnyCPU` +EXIT_CODE: 0 + +Verbatim summary lines: + +``` +Build succeeded. + + 3 Warning(s) + 0 Error(s) +``` + +`/p:Platform=AnyCPU` (no space) is used here, not the solution-level `"/p:Platform=Any CPU"` +alias. `QuickFiler.Test.csproj` conditions its `PropertyGroup` on the literal +`Debug|AnyCPU` string, so a `Platform` value containing a space matches no `PropertyGroup`, +leaves `OutputPath` unset, and fails the build outright. The build succeeding confirms the +correct spelling was used. + +## Step 2 — Run the New Test, Scoped + +Command: +`& $vstest QuickFiler.Test\bin\Debug\QuickFiler.Test.dll /Settings:scripts\vscode\TaskMaster.cli.runsettings /InIsolation /TestCaseFilter:"FullyQualifiedName~WriteMetricsAsync_WithAllNullOrWhitespaceDiagnostics_DoesNotInvokeWriter"` +EXIT_CODE: 1 + +Verbatim summary lines: + +``` +Total tests: 1 + Failed: 1 +Test Run Failed. + Total time: 1.5078 Seconds +``` + +## Verbatim Failure Detail + +``` + Failed WriteMetricsAsync_WithAllNullOrWhitespaceDiagnostics_DoesNotInvokeWriter [346 ms] + Error Message: + Expected invoked to be False because an empty filtered array must not reach the writer at all, but found True. +``` + +Stack frames identify the failing assertion as +`FluentAssertions.Primitives.BooleanAssertions.BeFalse`. + +## Acceptance + +| Condition | Required | Observed | Met | +|---|---|---|---| +| Non-zero `EXIT_CODE` | non-zero | `1` | Yes | +| Printed summary shows `Failed: 1` | yes | ` Failed: 1` | Yes | +| Test filter selected the new test only | yes | `Total tests: 1` | Yes | + +ACCEPTANCE: MET. + +## The Failure Is Genuine, Not a Harness Artifact + +The distinction matters because an assembly-load or test-host failure would also produce a +non-zero exit code while proving nothing about the defect. This run is a real assertion +failure: + +- The test was discovered, selected, and **executed**: it ran for 346 ms, not sub-millisecond. +- The failure message is the assertion's own text, including the `because` reason string + written into the test in P1-T2. It is not an empty message. +- The reported value is the defect itself: `invoked` was `True`, meaning + `WriteMetricsAsync` reached `MetricsFileWriter` even though the null-and-whitespace filter + had reduced `GetMoveDiagnostics`' `{ " ", null, "\t" }` output to an empty array. +- `MyDocuments` was present in this fixture (`withMyDocuments` defaults to `true`), so the + pre-existing MyDocuments guard at lines 131-134 was not the cause of, and did not mask, + this result. + +This is precisely the behavior issue #646 reports, reproduced deterministically in a unit +test with no filesystem, no live Outlook, and no wall-clock wait. + +The `Warning:` blocks in the raw runner output concern the Xceed Fluent Assertions +community licence and are printed on every run of this suite; they are unrelated to the +result. diff --git a/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/regression-testing/pass-after-new-test.2026-08-31T20-04.md b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/regression-testing/pass-after-new-test.2026-08-31T20-04.md new file mode 100644 index 000000000..911a86666 --- /dev/null +++ b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/regression-testing/pass-after-new-test.2026-08-31T20-04.md @@ -0,0 +1,74 @@ +# Pass-After — New Regression Test Against the Guarded Implementation (P1-T9) + +Timestamp: 2026-09-01T12-43 + +Task: `[P1-T9]` +Test: `WriteMetricsAsync_WithAllNullOrWhitespaceDiagnostics_DoesNotInvokeWriter` +Test file: `QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs` +Production file state: **guarded** — the P1-T5 fix is applied to +`QuickFiler/Controllers/QfcHomeController.Metrics.cs` at lines 175-178. + +## Step 1 — Rebuild the Test Project + +Command: +`msbuild QuickFiler.Test\QuickFiler.Test.csproj /t:Rebuild /m /p:Configuration=Debug /p:Platform=AnyCPU` +EXIT_CODE: 0 + +Verbatim summary lines: + +``` +Build succeeded. + + 3 Warning(s) + 0 Error(s) +``` + +`/p:Platform=AnyCPU` (no space) is used, per the same project-level requirement documented +in P1-T4. The changed production file reaches this build through the `ProjectReference` from +`QuickFiler.Test.csproj` to `QuickFiler\QuickFiler.csproj`, and `QuickFiler.csproj` also +conditions its `PropertyGroup` on the literal `Debug|AnyCPU` string, so the no-space spelling +propagates correctly to it as a global property. + +## Step 2 — Run the Same Scoped Command as P1-T4 + +Command: +`& $vstest QuickFiler.Test\bin\Debug\QuickFiler.Test.dll /Settings:scripts\vscode\TaskMaster.cli.runsettings /InIsolation /TestCaseFilter:"FullyQualifiedName~WriteMetricsAsync_WithAllNullOrWhitespaceDiagnostics_DoesNotInvokeWriter"` +EXIT_CODE: 0 + +The `/TestCaseFilter` is byte-identical to the one used in P1-T4. The only variable that +changed between the two runs is the presence of the four-line guard in the production file. + +## Verbatim Output + +``` + Passed WriteMetricsAsync_WithAllNullOrWhitespaceDiagnostics_DoesNotInvokeWriter [242 ms] + +Test Run Successful. +Total tests: 1 + Passed: 1 + Total time: 1.4446 Seconds +``` + +## Acceptance + +| Condition | Required | Observed | Met | +|---|---|---|---| +| `EXIT_CODE` | `0` | `0` | Yes | +| Printed summary shows `Passed: 1` | yes | ` Passed: 1` | Yes | +| Filter selected the new test only | yes | `Total tests: 1` | Yes | + +ACCEPTANCE: MET. + +## Fail-Before / Pass-After Pair + +| Run | Task | Production state | EXIT_CODE | Result | +|---|---|---|---|---| +| Fail-before | P1-T4 | unguarded | 1 | `Failed: 1` — `Expected invoked to be False ... but found True.` | +| Pass-after | P1-T9 | guarded | 0 | `Passed: 1` | + +Same test, same filter, same runner flags, same assembly path; only the four-line guard +differs. The test is therefore demonstrably sensitive to the defect rather than passing +vacuously, which is the evidence AC4 requires and the evidence backing AC1. + +The test also ran for 242 ms rather than sub-millisecond, confirming it was genuinely +discovered and executed rather than skipped or silently dropped by the filter. diff --git a/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/feature-audit.2026-09-01T12-53.md b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/feature-audit.2026-09-01T12-53.md new file mode 100644 index 000000000..79c0ff80b --- /dev/null +++ b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/feature-audit.2026-09-01T12-53.md @@ -0,0 +1,142 @@ +# Feature Audit — Issue #646 (qfc-metrics-flush-writes-empty-session-file) + +Timestamp: 2026-09-01T12-53 + +| Field | Value | +|---|---| +| Branch | `bug/qfc-metrics-flush-writes-empty-session-file-646` | +| HEAD | `0fe0668f146236c65aa93514fcb9756d366a6940` | +| Baseline | `origin/main` at `8996b28746d32f9f5996a037e0ca76be78b7684d` | +| Work mode | `minor-audit` | +| AC source | `issue.md`, section `## Acceptance Criteria` only | +| AC count | 8 (AC1-AC8) | +| Blocking findings | **0** | + +## AC Source Resolution + +`issue.md` line 12 carries the marker `- Work Mode: minor-audit`. Under the +`acceptance-criteria-tracking` protocol this resolves the sole AC source to `issue.md`, and +within it to the explicit `## Acceptance Criteria` heading at line 114. That section contains +eight checkbox items in the required `- [x] ACn:` form. + +`spec.md` and `user-story.md` are absent. Under `minor-audit` they are not AC sources, so their +absence is correct by design and is not recorded as a gap. No other checkbox section of +`issue.md` — including `Logs / Screenshots`, `Impact / Severity`, `Proposed Fix / Validation +Ideas`, and `Next Step` — was treated as an acceptance criterion. + +## Verification Method + +Every AC was verified against primary evidence rather than accepted on the strength of its +checkbox. Where the evidence artifact stated a figure, this reviewer re-derived that figure +independently from the tree or the committed data. Independent re-derivations performed: + +- `git diff --shortstat origin/main...HEAD` -> 31 files, 3223 insertions, 0 deletions. +- Mechanical inverse-prefix filter over `git diff --name-only origin/main...HEAD` -> zero paths + outside the three AC7-allowed prefixes (grep exit 1, empty output). +- `git status --porcelain` -> empty. +- `wc -l` and `awk NR` on both changed source files -> 231 and 477 lines. +- Re-summed `LINE` counters in both committed JaCoCo projections -> 48426/142226 baseline and + 48436/142240 final, matching the recorded Cobertura root counters exactly. +- Re-derived the first-party subset from the final projection -> 14540 of 62121, matching the + recorded 23.4059%. +- Read the delivered guard and the EFC reference guard directly from the working tree. +- `git diff --name-only 10aaaf65 HEAD -- "*.cs"` -> empty, confirming no source change after the + toolchain gates ran. + +## Acceptance Criteria Evaluation + +| AC | Criterion (abridged) | Evidence | Independent check | Verdict | +|---|---|---|---|---| +| AC1 | `WriteMetricsAsync` returns without invoking `MetricsFileWriter` when the filtered array is empty | `evidence/regression-testing/fail-before-new-test...md`, `evidence/regression-testing/pass-after-new-test...md` | Read lines 174-188 of the delivered file; the `return;` precedes the `await MetricsFileWriter(...)` with no intervening statement | **PASS** | +| AC2 | Guard is an early return between the filter statement and the await, textually equivalent to the EFC guard | `evidence/other/production-diff-scope...md` | Compared both guards in the tree: QFC lines 175-178 against EFC lines 72-75; identical but for the array identifier, which AC2's own `if (.Length == 0) { return; }` form contemplates | **PASS** | +| AC3 | New MSTest test stubs `GetMoveDiagnostics` to an all-null-or-whitespace array and asserts zero writer invocations | `QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs` lines 452-474 | Read the test: `BuildLooseMetricsController(new[] { " ", null, "\t" })` — every element null or whitespace — and `invoked.Should().BeFalse(...)` | **PASS** | +| AC4 | Test fails against the unguarded implementation and passes after, with fail-before evidence under `evidence/regression-testing/` | `evidence/regression-testing/fail-before-new-test...md` (exit 1), `pass-after-new-test...md` (exit 0) | Both artifacts present at the required path; the RED run shows a genuine 346 ms assertion failure with the test's own `because` text, not an empty message or a sub-millisecond load failure | **PASS** | +| AC5 | The two named pre-existing tests still pass and are not modified | `evidence/regression-testing/existing-tests-pass...md` (`Passed: 2`), `evidence/other/test-file-diff-scope...md` | `git diff --numstat` on the test file at HEAD reports 23 insertions and **0 deletions**, a single pure-insertion hunk after old line 451; a zero-deletion diff cannot have altered either named test | **PASS** | +| AC6 | `MetricsFileWriter` signature and the writer failure-handling branch unchanged | `evidence/other/production-diff-scope...md` | The production diff is one hunk spanning old lines 172-177. The delegate declaration sits at lines 28-34 and the `if (!metricsWritten)` branch at 189-195; both are outside the hunk and both read unchanged in the tree | **PASS** | +| AC7 | No repository file outside the two owned source files and this feature folder is modified | `evidence/qa-gates/footprint-scope...md` | Re-run at HEAD `0fe0668f`: 31 diff paths, inverse-prefix filter returns zero results, working tree clean. The executor's artifact recorded 29 paths at the earlier HEAD `ba134b57`; the growth to 31 is the two JaCoCo projections and the substitution record, all inside the third allowed prefix | **PASS** | +| AC8 | C# toolchain passes in order in a single final pass | Five `evidence/qa-gates/` artifacts, each with command, exit code, and verbatim summary | Order verified as format (P2-T1) -> check (P2-T2) -> analyzer (P2-T3) -> nullable (P2-T4) -> vstest (P2-T5), all exit 0, uninterrupted after the mandated pass-1 restart. Confirmed still valid at HEAD: no `.cs` file changed after the gated commit | **PASS** | + +## Notes on Individual Criteria + +**AC2 — textual equivalence.** The two guards differ only in the array identifier (`lines` +against `dataLines`). AC2 states the required form as `if (.Length == 0) { return; }` +with the array name as a placeholder, so identifier divergence is what the criterion expects +rather than a shortfall. The structural relationship also matches: in both files the guard +directly follows its computing statement with no blank line between them, and is followed by a +blank line before the next construct. + +**AC4 — the RED result is genuine.** This is the criterion most easily satisfied vacuously, so +it received the closest scrutiny. Four independent signals confirm the failure was a real +assertion rather than a harness or assembly-load failure: the test ran 346 ms rather than +sub-millisecond; the failure message is the test's own `because` string rather than empty; the +reported value (`invoked` was `True`) is the defect itself; and the fixture supplied +`MyDocuments`, so the pre-existing folder guard neither caused nor masked the result. The GREEN +run used a byte-identical `/TestCaseFilter` against the same assembly path, isolating the guard +as the only changed variable. + +**AC7 — evidence recorded at an earlier HEAD.** The footprint artifact was written at HEAD +`ba134b57` and records 29 paths; HEAD is now `0fe0668f` with 31. The artifact anticipates this +and states that the later coverage-substitution pass adds two projections and one record inside +the allowed feature-folder prefix. This reviewer re-ran the mechanical check at the current HEAD +rather than relying on the recorded count, and it passes: zero paths outside the allowed set. + +An intermediate commit on this branch (`9f578b3c`) added two files under +`.claude/agent-memory/`, which is outside the allowed set, and a later commit (`8a2054cd`) +removed them. `git diff --stat origin/main...HEAD -- .claude` returns empty, so the merged tree +gains nothing there. AC7 is evaluated against the branch diff, which is the reading the +criterion's own wording and the plan's gate both use, and it holds. Recorded for transparency +in `code-review.2026-09-01T12-53.md` as finding CR-6. + +**AC8 — coverage evidence substitution does not weaken the gate.** The `vstest` gate's coverage +artifacts were converted from raw Cobertura to package-level JaCoCo projections after the gate +completed. The three tasks that read the raw reports (P0-T11, P2-T6, P2-T7) had already +evaluated their acceptance conditions against them. This reviewer re-summed the projections and +reproduced the recorded root counters exactly on both sides, so every figure those gates quoted +remains verifiable from the committed evidence. The gate sequence is auditable. + +## Baseline Behaviour Comparison + +| Scenario | `origin/main` behaviour | HEAD behaviour | Matches issue's Expected Behavior | +|---|---|---|---| +| Filtered diagnostic array is empty | `MetricsFileWriter` invoked; the default append writer creates the session-metrics file if absent or updates its last-write time if present, recording nothing | `WriteMetricsAsync` returns before the writer; no file created, no file touched | Yes | +| Filtered array is non-empty | Writer invoked once with the filtered lines | Unchanged — writer invoked once with the filtered lines | Yes (no regression) | +| Input contains a mix of valid and null/whitespace entries | Writer receives only the valid entries | Unchanged — writer receives only the valid entries | Yes (no regression) | +| `MyDocuments` absent | Returns before the writer via the pre-existing folder guard | Unchanged | Yes (no regression) | + +The defect described in the issue is resolved and the three adjacent behaviours are preserved, +each held by a passing test. The full suite moved from 1284 passing to 1285 passing with zero +failures, and the `+1` is exactly the test added here. + +One behaviour is deliberately not changed: the Outlook calendar appointment written by +`WriteMoveToCalendar` at line 154 still occurs in the empty-diagnostics case, because that call +precedes the guard. The issue's Expected Behavior is scoped to the metrics file and AC1 is +scoped to `MetricsFileWriter`, so this is correct against the criteria as written. It is +recorded as finding CR-2 for a follow-up decision. + +## Check-Off Reconciliation + +All eight criteria were already checked `- [x]` in `issue.md` before this review. Each was +re-verified against its evidence, and all eight are supported. No criterion was found checked +without support, so no correction to `issue.md` is required and none was made. No criterion was +added, reworded, or unchecked by this review. + +## Acceptance Criteria Status + +``` +### Acceptance Criteria Status +- Source: docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/issue.md +- Total AC items: 8 +- Checked off (delivered): 8 +- Remaining (unchecked): 0 +- Items remaining: none +``` + +## Verdict + +**PASS — 8 of 8 acceptance criteria verified against evidence. 0 blocking findings.** + +The delivered change resolves the reported defect with a four-line guard and one regression +test, backed by a genuine fail-before / pass-after pair and a clean five-gate toolchain +sequence. The non-blocking findings raised in `code-review.2026-09-01T12-53.md` and the coverage +provisioning gaps recorded in `policy-audit.2026-09-01T12-53.md` do not affect any acceptance +criterion and none is remediable within AC7's footprint restriction. diff --git a/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/issue.md b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/issue.md new file mode 100644 index 000000000..1ab9e87f2 --- /dev/null +++ b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/issue.md @@ -0,0 +1,142 @@ +# qfc-metrics-flush-writes-empty-session-file (Issue #646) + +- Date captured: 2026-08-27 +- Author: Dan Moisan +- Status: Promoted -> docs/features/active/qfc-metrics-flush-writes-empty-session-file/ (Issue #646) + +> Automation note: Keep the section headings below unchanged; the promotion tooling maps each of them into the GitHub bug issue template. + +- Issue: #646 +- Issue URL: https://github.com/drmoisan/TaskMaster/issues/646 +- Last Updated: 2026-08-27 +- Work Mode: minor-audit + +## Summary + +`QfcHomeController.WriteMetricsAsync` invokes the injected `MetricsFileWriter` unconditionally, even +when the null-and-whitespace filter leaves the diagnostic-line array empty. The default writer +`FileIO2.WriteTextFileAsync` opens an append `StreamWriter`, so a QuickFiler session that produces no +diagnostic lines now creates, or touches, a zero-content session-metrics file. + +The EFC sibling path already guards against this. `EfcHomeController.Metrics.cs` returns early: + +```csharp +if (dataLines.Length == 0) +{ + return; +} +``` + +`QfcHomeController.Metrics.cs` has no matching guard at the point where it awaits the writer. + +**This is a narrow regression introduced by the #442 flush fix, not a pre-existing defect.** Before +that fix the QFC metrics queue was never drained, so nothing was ever written and the empty-array +case could not manifest. Making the flush work made it reachable. + +The remedy is one guard in an owned file, mirroring the EFC form: + +```csharp +if (lines.Length == 0) +{ + return; +} +``` + +It was deliberately not applied inside feature `quickfiler-home-controller-metrics-442`. That +feature's plan was complete and its toolchain green at the time the finding was raised by +feature-review (finding CR-1, Minor, non-blocking), and the repository's General Code Change Policy +directs opening a new issue rather than widening the scope of work in flight. + +## Environment + +- OS/version: Windows 11, Outlook VSTO add-in host +- Python version: not applicable (C# / .NET Framework 4.8) +- Command/flags used: `vstest.console.exe QuickFiler.Test\bin\Debug\QuickFiler.Test.dll` +- Data source or fixture: the session-metrics CSV in the user's `MyDocuments` folder + +## Steps to Reproduce + +1. Run a QuickFiler filing session in which `GetMoveDiagnostics` returns an array whose every + element is `null` or whitespace — for example a session in which no item is actually moved. +2. Let the session reach its metrics write. +3. Inspect the session-metrics file in `MyDocuments`. + +## Expected Behavior + +No write occurs and no file is created or touched, because there is no diagnostic content to record. +This matches the EFC path's behavior for the same input. + +## Actual Behavior + +`MetricsFileWriter` is invoked with an empty array. The default writer opens the target for append, +so the file is created if absent and its last-write timestamp is updated if present, in both cases +recording nothing. + +## Logs / Screenshots + +- [x] Attached minimal logs or snippet +- Snippet: the unconditional call is the closing statement of `WriteMetricsAsync`: + `await MetricsFileWriter(filename, lines, myDocuments, CancellationToken.None);` + reached whatever the length of `lines`. + +## Impact / Severity + +- [ ] Blocker +- [ ] High +- [ ] Medium +- [x] Low + +Low. The worst outcome is a spurious empty or unchanged-content file in the user's `MyDocuments` +folder. No data is lost, no exception is raised, and no downstream consumer exists — the +session-metrics CSV has no in-repo reader. + +## Suspected Cause / Notes + +Asymmetry between the two controllers' metrics writers. The EFC path acquired its empty-array guard +when its writer was extracted behind `_dependencies.MetricsLineWriter`; the QFC path acquired its +writer seam later, during #442, and the guard was not carried across. + +Raised as finding CR-1 in +`docs/features/active/quickfiler-home-controller-metrics-442/code-review.2026-08-27T14-35.md`. + +## Proposed Fix / Validation Ideas + +- [x] Unit coverage areas: add a test in + `QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs` that mocks `GetMoveDiagnostics` to + return an all-null array and asserts the injected `MetricsFileWriter` delegate captures **zero** + invocations. The existing `WriteMetricsAsync_FiltersNullDiagnosticLinesBeforeWriting` test already + establishes the capture harness; this is the zero-line boundary case it does not cover. +- [x] Integration scenario to retest: the full QuickFiler suite must stay green; the new guard must + not alter `WriteMetricsAsync_InvokesInjectedMetricsFileWriterOnce`, which supplies non-empty lines. +- [x] Manual verification notes: confirm the two controllers' guards are textually equivalent after + the change, so the asymmetry does not reappear. + +## Acceptance Criteria + +- [x] AC1: `WriteMetricsAsync` in `QuickFiler/Controllers/QfcHomeController.Metrics.cs` returns without + invoking `MetricsFileWriter` when the null-and-whitespace filter leaves the filtered diagnostic-line + array empty. +- [x] AC2: The guard is an early return placed between the statement that computes the filtered + diagnostic-line array and the statement that awaits `MetricsFileWriter`, and is textually equivalent + to the guard already present in `QuickFiler/Controllers/EfcHomeController.Metrics.cs` + (`if (.Length == 0) { return; }`). +- [x] AC3: A new MSTest regression test in `QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs` + stubs `GetMoveDiagnostics` to return an array whose every element is `null` or whitespace and asserts + that the injected `MetricsFileWriter` delegate is invoked zero times. +- [x] AC4: The new regression test fails against the unguarded implementation and passes after the + guard is added, with fail-before evidence recorded under the feature folder's + `evidence/regression-testing/` directory. +- [x] AC5: The existing tests `WriteMetricsAsync_InvokesInjectedMetricsFileWriterOnce` and + `WriteMetricsAsync_FiltersNullDiagnosticLinesBeforeWriting` still pass and are not modified. +- [x] AC6: The `MetricsFileWriter` delegate signature and the writer's failure-handling branch are + unchanged by this item. Both are owned by issue #647 and are out of scope here. +- [x] AC7: No repository file outside `QuickFiler/Controllers/QfcHomeController.Metrics.cs`, + `QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs`, and this feature folder is modified. +- [x] AC8: The C# toolchain passes in order in a single final pass: `csharpier format` then + `csharpier check`, the analyzer `msbuild` rebuild, the nullable `msbuild` rebuild, and + `vstest.console.exe` with coverage enabled. + +## Next Step + +- [x] Promote to GitHub issue (bug-report template) +- [x] Move to active fix folder / branch diff --git a/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/plan.2026-08-31T20-04.md b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/plan.2026-08-31T20-04.md new file mode 100644 index 000000000..c771bde0a --- /dev/null +++ b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/plan.2026-08-31T20-04.md @@ -0,0 +1,414 @@ +# 2026-08-27-qfc-metrics-flush-writes-empty-session-file (Plan) + +- **Issue:** #646 +- **Parent (optional):** none +- **Owner:** drmoisan +- **Last Updated:** 2026-08-31T20-04 +- **Status:** Draft +- **Version:** 0.2 +- **Work Mode:** minor-audit +- **Directive:** MINIMAL-AUDIT PLAN REQUIRED + +## Requirements Source + +`docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/issue.md` +is the sole requirements source. Its `## Acceptance Criteria` section (AC1-AC8, lines +116-137 at plan-authoring time) is the only acceptance-criteria source for this plan. +`spec.md`, `user-story.md`, and `research.md` do not exist in the feature folder and are +not required. + +## Research Inputs + +`research/research.2026-08-31T20-30.md` describes a pre-merge tree (issue #647 not yet +landed). `research/research-correction.2026-08-31T20-45.md` is authoritative where the two +disagree: issue #647 has already merged into `main`, the `MetricsFileWriter` delegate now +returns `Task`, and the writer invocation is a multi-statement block followed by an +`if (!metricsWritten)` logging branch. Every citation below was re-derived directly against +the current working tree, not carried forward from either research document. + +## Hard Scope Boundaries + +1. The `MetricsFileWriter` delegate signature (`Func>`, declared at `QuickFiler/Controllers/ + QfcHomeController.Metrics.cs:28-34`) and the `if (!metricsWritten)` failure-logging + branch (same file, lines 185-191) are the delivered outcome of issue #647. No task in + this plan may alter either. +2. The only repository paths this plan writes to are: + - `QuickFiler/Controllers/QfcHomeController.Metrics.cs` + - `QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs` + - `docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/` + (this plan file, evidence artifacts, and `issue.md` check-offs) +3. `QuickFiler/Controllers/EfcHomeController.Metrics.cs` is read-only reference. No task + writes to it. +4. Every task that edits `QfcHomeController.Metrics.cs` re-derives its edit anchors against + the current tree at execution time (Phase 1, before editing) rather than trusting the + line numbers recorded in this plan or in either research document. +5. `QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs` is 454 lines at + plan-authoring time, against the repository's 500-line file-size cap. Phase 1 includes an + explicit post-change line-count verification task rather than assuming the new test fits. +6. No task in this plan runs `git update-index`. `artifacts/orchestration/ + orchestrator-state.json` already carries `--skip-worktree` from the orchestrator and must + stay outside this item's footprint; no task touches it. + +## Evidence Location + +Every evidence artifact resolves under `docs/features/active/ +2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence//` using +`baseline/`, `regression-testing/`, `qa-gates/`, or `other/`. No task writes to any +`artifacts/...` path as an evidence location. + +## Coverage Policy Note + +CLAUDE.md's C# Unit Test Policy states repository-wide line coverage must remain >= 80% and +new modules/methods must reach >= 90%. `.claude/rules/general-unit-test.md` states a uniform +>= 85% line / >= 75% branch floor across all tiers. These two documents disagree on the +repository-wide floor; this plan does not resolve that conflict. Consistent with prior +practice recorded for this repository (`quickfiler-home-controller-metrics-442` and prior +coverage-reconciliation items), this plan treats the repository-wide percentage as a +recorded, non-blocking figure and treats changed-line no-regression and new-code coverage +(>= 90% for the four new guard lines) as the blocking gates, per Phase 2 P2-T7. + +--- + +### Phase 0 — Baseline Capture + +Policy reads follow the `policy-compliance-order` sequence: `CLAUDE.md` (position 1), +`.claude/rules/general-code-change.md` (position 2), `.claude/rules/general-unit-test.md` +(position 3), `.claude/rules/csharp.md` (position 4, applicable because both in-scope files +are `*.cs`). + +- [x] [P0-T1] Read `CLAUDE.md` in full at the repository root. Acceptance: the read is + recorded in the Phase 0 policy-read evidence artifact produced by P0-T5. +- [x] [P0-T2] Read `.claude/rules/general-code-change.md` in full. Acceptance: recorded in + the P0-T5 artifact. +- [x] [P0-T3] Read `.claude/rules/general-unit-test.md` in full. Acceptance: recorded in the + P0-T5 artifact. +- [x] [P0-T4] Read `.claude/rules/csharp.md` in full. Acceptance: recorded in the P0-T5 + artifact. +- [x] [P0-T5] Write the Phase 0 policy-read evidence artifact to + `docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/ + evidence/baseline/phase0-instructions-read.md` with `Timestamp:`, `Policy Order:` (the + four files in the order listed above), and an explicit list of files read. Acceptance: + the file exists and contains all three required fields. +- [x] [P0-T6] Run `git fetch origin`, then reconcile the current branch onto the + `origin/main` tip (fast-forward merge if a clean fast-forward is possible, otherwise a + merge of `origin/main` into the current branch). Record the pre- and post-reconciliation + `git rev-parse HEAD` values. Acceptance: `git merge-base --is-ancestor origin/main HEAD` + exits `0`. Evidence: + `evidence/baseline/branch-reconciliation.2026-08-31T20-04.md` with `Timestamp:`, + `Command:`, `EXIT_CODE:`, `Output Summary:`. +- [x] [P0-T7] Run `dotnet tool run csharpier check .` from the repository root. Record the + printed summary line verbatim (CSharpier's check-mode success output, or the list of + files needing formatting on failure). Acceptance: `EXIT_CODE` and `Output Summary` are + both recorded, whatever the exit code is (this step establishes the pre-existing + formatting state; it is not gated pass/fail). Evidence: + `evidence/baseline/csharpier-check.2026-08-31T20-04.md`. +- [x] [P0-T8] Run `msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug + "/p:Platform=Any CPU" /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true` from the + repository root. Record `EXIT_CODE` and the printed `Build succeeded`/`Build FAILED` + summary line with warning/error counts. Acceptance: `EXIT_CODE` recorded (this is a + baseline capture, not a gate). Evidence: + `evidence/baseline/msbuild-analyzer-rebuild.2026-08-31T20-04.md`. +- [x] [P0-T9] Run `msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug + "/p:Platform=Any CPU" /p:TreatWarningsAsErrors=true` from the repository root. Record + `EXIT_CODE` and the printed build summary line. Acceptance: `EXIT_CODE` recorded. + Evidence: `evidence/baseline/msbuild-nullable-rebuild.2026-08-31T20-04.md`. +- [x] [P0-T10] Resolve `vstest.console.exe` via `vswhere.exe` (not on PATH in this + environment): `$vswhere = Join-Path ${env:ProgramFiles(x86)} 'Microsoft Visual + Studio\Installer\vswhere.exe'; $vstest = & $vswhere -latest -products * -find + 'Common7\IDE\Extensions\TestPlatform\vstest.console.exe' | Select-Object -First 1`. Then + run `& $vstest QuickFiler.Test\bin\Debug\QuickFiler.Test.dll /EnableCodeCoverage` — the + exact assembly path and flag CUT3 names, with no other flags added. Record `EXIT_CODE` + and the printed `Passed!`/`Failed!` summary line with Passed/Failed/Total counts. + Acceptance: `EXIT_CODE` and the printed summary line are both recorded. Evidence: + `evidence/baseline/vstest-coverage-run.2026-08-31T20-04.md`. +- [x] [P0-T11] Locate the `.coverage` file created by P0-T10 (`Get-ChildItem -Path + TestResults -Filter *.coverage -Recurse | Sort-Object LastWriteTime -Descending | + Select-Object -First 1`), then run `dotnet-coverage merge -f cobertura -o + docs\features\active\2026-08-27-qfc-metrics-flush-writes-empty-session-file-646\evidence\ + baseline\baseline-coverage.cobertura.xml `. Record the resulting + XML's root `` element `line-rate` and `branch-rate` attribute values verbatim + as the baseline coverage headline. Acceptance: the `.cobertura.xml` artifact exists and + its root `line-rate` value is a numeric string, not a placeholder. Evidence: + `evidence/baseline/coverage-cobertura-baseline.2026-08-31T20-04.md` referencing the + `.cobertura.xml` file. + +### Phase 1 — Constrained Implementation + +- [x] [P1-T1] Re-derive the two edit anchors against the current tree (post-P0-T6 + reconciliation): search `QuickFiler/Controllers/QfcHomeController.Metrics.cs` for the + literal `var lines = strOutput.Where(line => !string.IsNullOrWhiteSpace(line)).ToArray();` + (Anchor A) and the literal `bool metricsWritten = await MetricsFileWriter(` (Anchor B), + and record their current line numbers. Do not anchor on the absolute line numbers + recorded in this plan or in either research artifact. Acceptance: both literals are found + exactly once each in the file, and their line numbers are recorded. Evidence: + `evidence/other/anchor-rederivation.2026-08-31T20-04.md`. +- [x] [P1-T2] [expect-fail] Add a new MSTest regression test method, + `WriteMetricsAsync_WithAllNullOrWhitespaceDiagnostics_DoesNotInvokeWriter`, to + `QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs`, inserted immediately after + `WriteMetricsAsync_WithoutMyDocumentsFolder_DoesNotInvokeWriter` inside the `#region Issue + #442 — metrics flush tests` block. Model it on that test: call + `BuildLooseMetricsController(new[] { " ", null, "\t" })` (default `withMyDocuments: + true`, so the pre-existing MyDocuments guard does not itself cause the early return), set + `controller.MetricsFileWriter` to a lambda that sets a `bool invoked = true` and returns + `Task.FromResult(true)`, call `await controller.WriteMetricsAsync("metrics.csv")`, then + assert `invoked.Should().BeFalse(...)`. Acceptance: the method exists verbatim as + described in the file. +- [x] [P1-T3] Check off AC3 in `issue.md` (`A new MSTest regression test in + QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs stubs GetMoveDiagnostics to + return an array whose every element is null or whitespace and asserts that the injected + MetricsFileWriter delegate is invoked zero times.`), backed by P1-T2. Change only `- [ ]` + to `- [x]` for that line. +- [x] [P1-T4] [expect-fail] Rebuild the test project: `msbuild + QuickFiler.Test\QuickFiler.Test.csproj /t:Rebuild /m /p:Configuration=Debug + /p:Platform=AnyCPU`. Use `/p:Platform=AnyCPU` (no space) for this project-level build, + not the solution-level `"/p:Platform=Any CPU"` alias: `QuickFiler.Test.csproj`'s + `PropertyGroup` conditions key on the literal `Debug|AnyCPU` string, and a `Platform` + value of `Any CPU` (with a space) matches no `PropertyGroup`, which leaves `OutputPath` + unset and fails the build with `The BaseOutputPath/OutputPath property is not set for + project 'QuickFiler.Test.csproj'`. Then, using the vswhere-resolved `$vstest` from + P0-T10 (or re-resolve it if the session state is gone), run `& $vstest + QuickFiler.Test\bin\Debug\QuickFiler.Test.dll + /Settings:scripts\vscode\TaskMaster.cli.runsettings /InIsolation + /TestCaseFilter:"FullyQualifiedName~WriteMetricsAsync_WithAllNullOrWhitespaceDiagnostics_DoesNotInvokeWriter"`. + Acceptance: the run reports a non-zero `EXIT_CODE` and the printed summary line shows + `Failed: 1` — the new test fails against the unguarded implementation, because the + guard does not exist yet. Evidence: + `evidence/regression-testing/fail-before-new-test.2026-08-31T20-04.md`. +- [x] [P1-T5] Apply the fix: insert + ``` + if (lines.Length == 0) + { + return; + } + ``` + immediately after the statement that computes the filtered array (Anchor A, as re-derived + in P1-T1) and before the three-line explanatory comment about `CancellationToken.None` + that immediately precedes the writer invocation statement (Anchor B), so that comment + stays adjacent to the writer statement it explains, in `QuickFiler/Controllers/ + QfcHomeController.Metrics.cs`. Acceptance: the exact four-line block appears in the file + immediately after Anchor A and immediately before that comment block. +- [x] [P1-T6] Verify the production diff is scoped to exactly the new guard: run `git diff + origin/main -- QuickFiler/Controllers/QfcHomeController.Metrics.cs`. Confirm zero removed + (`-`) lines appear in the diff, confirm the only added (`+`) lines are the four guard + lines from P1-T5, and confirm the diff contains no hunk touching the `MetricsFileWriter` + property declaration (the `Task` lines at 28-34) or the `if (!metricsWritten)` block + (lines 185-191). Acceptance: all three conditions hold. Evidence: + `evidence/other/production-diff-scope.2026-08-31T20-04.md`. +- [x] [P1-T7] Check off AC6 in `issue.md` (`The MetricsFileWriter delegate signature and the + writer's failure-handling branch are unchanged by this item. Both are owned by issue #647 + and are out of scope here.`), backed by P1-T6. +- [x] [P1-T8] Check off AC2 in `issue.md` (`The guard is an early return placed between the + statement that computes the filtered diagnostic-line array and the statement that awaits + MetricsFileWriter, and is textually equivalent to the guard already present in + QuickFiler/Controllers/EfcHomeController.Metrics.cs`), backed by P1-T5 and by the + `if (dataLines.Length == 0) { return; }` guard already present at + `QuickFiler/Controllers/EfcHomeController.Metrics.cs:72-75`. +- [x] [P1-T9] Rebuild the test project again (`msbuild + QuickFiler.Test\QuickFiler.Test.csproj /t:Rebuild /m /p:Configuration=Debug + /p:Platform=AnyCPU`, per the same `/p:Platform=AnyCPU` (no space) requirement documented + in P1-T4), then re-run the same scoped command as P1-T4 (same `/TestCaseFilter`) against + the fixed implementation. Acceptance: `EXIT_CODE 0` and the printed summary line shows + `Passed: 1`. Evidence: + `evidence/regression-testing/pass-after-new-test.2026-08-31T20-04.md`. +- [x] [P1-T10] Check off AC1 in `issue.md` (`WriteMetricsAsync ... returns without invoking + MetricsFileWriter when the null-and-whitespace filter leaves the filtered + diagnostic-line array empty.`), backed by P1-T9. +- [x] [P1-T11] Check off AC4 in `issue.md` (`The new regression test fails against the + unguarded implementation and passes after the guard is added, with fail-before evidence + recorded ...`), backed by P1-T4 and P1-T9. +- [x] [P1-T12] Run the two existing tests `WriteMetricsAsync_InvokesInjectedMetricsFileWriterOnce` + and `WriteMetricsAsync_FiltersNullDiagnosticLinesBeforeWriting` via the vswhere-resolved + `$vstest` with `/Settings:scripts\vscode\TaskMaster.cli.runsettings /InIsolation + /TestCaseFilter:"FullyQualifiedName~WriteMetricsAsync_InvokesInjectedMetricsFileWriterOnce|FullyQualifiedName~WriteMetricsAsync_FiltersNullDiagnosticLinesBeforeWriting"`. + Acceptance: `EXIT_CODE 0` and the printed summary line shows `Passed: 2`. Evidence: + `evidence/regression-testing/existing-tests-pass.2026-08-31T20-04.md`. +- [x] [P1-T13] Verify the test-file diff contains zero removed lines: run `git diff + origin/main -- QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs`. Confirm no + `-` line appears in the diff (only additions), which together with P1-T12 confirms the + two pre-existing tests were not modified. Evidence: + `evidence/other/test-file-diff-scope.2026-08-31T20-04.md`. +- [x] [P1-T14] Check off AC5 in `issue.md` (`The existing tests + WriteMetricsAsync_InvokesInjectedMetricsFileWriterOnce and + WriteMetricsAsync_FiltersNullDiagnosticLinesBeforeWriting still pass and are not + modified.`), backed by P1-T12 and P1-T13. +- [x] [P1-T15] Verify the post-change line count of the test file: run `(Get-Content + 'QuickFiler.Test\Controllers\QfcHomeControllerMetricsTests.cs').Count`. Acceptance: the + result is less than or equal to `500`. Evidence: + `evidence/other/test-file-line-count.2026-08-31T20-04.md`. + +### Phase 2 — Final QC Loop + +Run P2-T1 through P2-T5 in order, unconditionally. If any of P2-T1 through P2-T5 reports a +non-zero `EXIT_CODE`, or if P2-T1 rewrites any tracked file, restart the loop from P2-T1. +`EXIT_CODE: SKIPPED` is not a valid recorded outcome for any task in this phase. + +- [x] [P2-T1] Run `git status --porcelain` and record the set of modified paths and their + diff line-counts (the tree already carries the Phase 1 edits to the two owned files). + Then run `dotnet tool run csharpier format .` from the repository root. Then run `git + status --porcelain` again. Acceptance: `EXIT_CODE 0` is recorded, and the task records + whether the second `git status --porcelain` shows any additional changed-line count for + the two owned files, or any newly-modified path, beyond the pre-format snapshot (a + difference means the formatter rewrote content; no difference means the tree was already + compliant). Evidence: `evidence/qa-gates/csharpier-format.2026-08-31T20-04.md`. +- [x] [P2-T2] Run `dotnet tool run csharpier check .`. Acceptance: `EXIT_CODE 0`. Evidence: + `evidence/qa-gates/csharpier-check-final.2026-08-31T20-04.md`. +- [x] [P2-T3] Run `msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug + "/p:Platform=Any CPU" /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true`. + Acceptance: `EXIT_CODE 0` and the printed summary line reads `Build succeeded.`. Evidence: + `evidence/qa-gates/msbuild-analyzer-rebuild.2026-08-31T20-04.md`. +- [x] [P2-T4] Run `msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug + "/p:Platform=Any CPU" /p:TreatWarningsAsErrors=true`. Acceptance: `EXIT_CODE 0` and the + printed summary line reads `Build succeeded.`. Evidence: + `evidence/qa-gates/msbuild-nullable-rebuild.2026-08-31T20-04.md`. +- [x] [P2-T5] Resolve `vstest.console.exe` via `vswhere.exe` (same resolution as P0-T10), + then run `& $vstest QuickFiler.Test\bin\Debug\QuickFiler.Test.dll /EnableCodeCoverage` — + the exact assembly path and flag CUT3 names, with no other flags added. Acceptance: + `EXIT_CODE 0` and the printed summary line shows `Failed: 0` with a `Total:` count + greater than or equal to the P0-T10 baseline total. Evidence: + `evidence/qa-gates/vstest-coverage-run.2026-08-31T20-04.md`. +- [x] [P2-T6] Locate the `.coverage` file created by P2-T5 (same discovery method as + P0-T11), then run `dotnet-coverage merge -f cobertura -o + docs\features\active\2026-08-27-qfc-metrics-flush-writes-empty-session-file-646\evidence\ + qa-gates\final-coverage.cobertura.xml `. Record the resulting + XML's root `` element `line-rate` and `branch-rate` attribute values verbatim as + the final coverage headline. Acceptance: the `.cobertura.xml` artifact exists and its root + `line-rate` value is a numeric string, not a placeholder. Evidence: + `evidence/qa-gates/coverage-cobertura-final.2026-08-31T20-04.md`. +- [x] [P2-T7] Coverage delta verification: compare the baseline `line-rate` (P0-T11) to the + final `line-rate` (P2-T6) and record both values plus the difference; confirm the final + value is not lower than the baseline value. Separately, in the final Cobertura XML, + locate the `` element and the `` entries for the four guard lines added in P1-T5 (using the + post-fix line numbers recorded in P1-T6), and confirm each of the four lines has `hits` + greater than `0`. Acceptance: both conditions hold (no repository-wide regression, and + 100% coverage on the new guard, satisfying the CLAUDE.md >= 90% new-code floor). Evidence: + `evidence/qa-gates/coverage-delta-verification.2026-08-31T20-04.md`. +- [x] [P2-T8] Verify the total change footprint: run `git status --porcelain` and `git diff + origin/main --name-status`. Confirm every path listed by either command begins with one + of `QuickFiler/Controllers/QfcHomeController.Metrics.cs`, `QuickFiler.Test/Controllers/ + QfcHomeControllerMetricsTests.cs`, or `docs/features/active/ + 2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/`. Acceptance: no listed path + falls outside that set. Evidence: `evidence/qa-gates/footprint-scope.2026-08-31T20-04.md`. +- [x] [P2-T9] Check off AC7 in `issue.md` (`No repository file outside + QuickFiler/Controllers/QfcHomeController.Metrics.cs, + QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs, and this feature folder is + modified.`), backed by P2-T8. +- [x] [P2-T10] Check off AC8 in `issue.md` (`The C# toolchain passes in order in a single + final pass: csharpier format then csharpier check, the analyzer msbuild rebuild, the + nullable msbuild rebuild, and vstest.console.exe with coverage enabled.`), backed by + P2-T1 through P2-T5 having completed with no restart of the loop. +- [x] [P2-T11] Final reconciliation: read `issue.md` and confirm all eight items under + `## Acceptance Criteria` (AC1 through AC8) are `- [x]`. Acceptance: all eight are checked; + if any is not, this task fails and the gap must be documented rather than the checkbox + force-checked. + +--- + +## Self-Review + +SELF-REVIEW: RE-DERIVED THIS PASS + +- `docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/issue.md` — + lines 12 (`Work Mode: minor-audit`), 114 (`## Acceptance Criteria` heading), 116-137 + (AC1-AC8 verbatim text). +- `QuickFiler/Controllers/QfcHomeController.Metrics.cs` — lines 28-34 (`MetricsFileWriter` + delegate, `Task` return type), 107 (method signature), 131-134 (pre-existing + MyDocuments guard), 162-169 (`GetMoveDiagnostics` call), 171-174 (Anchor A and its + preceding comment), 176-184 (Anchor B and its preceding comment), 185-191 (`if + (!metricsWritten)` failure branch), 192 (method close). +- `QuickFiler/Controllers/EfcHomeController.Metrics.cs` — lines 59-81 (`QuickFileMetrics_WRITE` + four-arg overload), 72-75 (`if (dataLines.Length == 0) { return; }` guard). +- `QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs` — file length (454 lines, + confirmed by reading to EOF at line 455/blank), lines 72-135 (`BuildLooseMetricsController`, + confirming lambdas return `Task.FromResult(true)` post-#647), 300-323 (`MetricsWrite` + capture record), 330-347 (`WriteMetricsAsync_InvokesInjectedMetricsFileWriterOnce`), + 404-425 (`WriteMetricsAsync_FiltersNullDiagnosticLinesBeforeWriting`), 432-450 + (`WriteMetricsAsync_WithoutMyDocumentsFolder_DoesNotInvokeWriter`, the test template). +- `docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/research/ + research-correction.2026-08-31T20-45.md` — full document, confirming it supersedes + `research.2026-08-31T20-30.md` on delegate signature, writer-call text, and line numbers. +- `docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/` (glob) — + confirmed no `spec.md`, `user-story.md`, or `research.md` at feature-folder root, and no + pre-existing `evidence/` directory. +- `.claude/rules/csharp.md` — lines 12-19 (toolchain commands, including the `/t:Rebuild` + requirement for both msbuild gates). +- `.claude/rules/plan-acceptance-gates.md` — lines 27-50 (G1-G9 rule table), 139-177 + (write-mode register: CSharpier is not a register member; the six entries are + `black-write`, `ruff-fix`, `prettier-write`, `poshqc-format`, + `run_poshqc_analyze_autofix`, `poshqc-suite`). +- `dotnet-tools.json` (repository root) — confirmed the only pinned local tool is + `csharpier` 1.2.6; `dotnet-coverage` is not a local manifest tool and is assumed + available as a global tool per `scripts/vscode/Invoke-MSTestWithCoverage.ps1`'s own + precondition check. +- `QuickFiler.Test/QuickFiler.Test.csproj` — line 454 (`ProjectReference` to + `QuickFiler\QuickFiler.csproj`), confirming a scoped rebuild of the test project pulls in + the changed production file via project-reference propagation. +- `QuickFiler.Test/QuickFiler.Test.csproj` — re-derived this pass (revision round 2): line + 12 (`AnyCPU`), line 32 + (``), line + 36 (`bin\Debug\`, set only inside that conditioned + `PropertyGroup`). Confirms a project-level `msbuild` invocation with `"/p:Platform=Any + CPU"` (the space-containing solution alias) matches no `PropertyGroup` on this + legacy-style project, leaves `OutputPath` unset, and fails the build; `/p:Platform=AnyCPU` + (no space) is required at the project level instead. +- `QuickFiler/QuickFiler.csproj` — re-derived this pass (revision round 2): line 7 + (`AnyCPU`), line 20 + (``). + Confirms the referenced production project (built transitively via the `ProjectReference` + at `QuickFiler.Test.csproj:454` during P1-T4/P1-T9) also keys its `PropertyGroup` on the + literal `Debug|AnyCPU` string, so `/p:Platform=AnyCPU` (no space) propagates correctly to + it as a global property during the project-level rebuild. +- `QuickFiler/Controllers/QfcHomeController.Metrics.cs` — re-derived this pass (revision + round 2): lines 170-192 re-read in full against the current tree. Confirms line 174 is + Anchor A (`var lines = strOutput.Where(...)`), lines 176-178 are the three-line + `CancellationToken.None` explanatory comment, and line 179 is Anchor B (`bool + metricsWritten = await MetricsFileWriter(`). The guard insertion point in P1-T5 is + tightened to "immediately after Anchor A and before the comment block" (matching + `research-correction.2026-08-31T20-45.md` lines 58-66) rather than the looser "between + Anchor A and Anchor B", which was ambiguous about whether the comment stays attached to + the writer statement it explains. +- `docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/research/ + research-correction.2026-08-31T20-45.md` — re-derived this pass (revision round 2): lines + 58-66 (Anchor A/B definitions and "The guard belongs between Anchor A and that comment + block" sentence at line 66). +- `.claude/agent-memory/atomic-planner/reference_vstest_scoped_run_command.md` — the + vswhere-resolution and `/TestCaseFilter` command form used in P0-T10, P1-T4, P1-T9, + P1-T12, and P2-T5. +- `.claude/agent-memory/atomic-planner/feedback_ac_checkoff_one_per_task.md` and + `.claude/skills/acceptance-criteria-tracking/SKILL.md` — the one-AC-per-task check-off + protocol applied throughout Phase 1 and Phase 2. + +## Planner Internal Review Record + +PLANNER-INTERNAL-REVIEW: PASS + +CITATION-TO-TREE: PASS +AC-TRACEABILITY: PASS +SCOPE-BOUNDARY: PASS + +CITATION: docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/issue.md | lines 12, 114, 116-137 +CITATION: QuickFiler/Controllers/QfcHomeController.Metrics.cs | lines 28-34, 107, 131-134, 162-192 +CITATION: QuickFiler/Controllers/EfcHomeController.Metrics.cs | lines 59-81 +CITATION: QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs | lines 72-135, 300-323, 330-347, 404-425, 432-450, file length 454 +CITATION: docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/research/research-correction.2026-08-31T20-45.md | full document +CITATION: .claude/rules/csharp.md | lines 12-19 +CITATION: .claude/rules/plan-acceptance-gates.md | lines 27-50, 139-177 +CITATION: dotnet-tools.json | lines 1-13 +CITATION: QuickFiler.Test/QuickFiler.Test.csproj | line 454, lines 12, 32, 36 +CITATION: QuickFiler/QuickFiler.csproj | lines 7, 20 + +AC-INVENTORY: AC1, AC2, AC3, AC4, AC5, AC6, AC7, AC8 + +AC-MAPPING: AC1 | IMPLEMENTATION: P1-T5 | TESTS: P1-T9 | EVIDENCE: evidence/regression-testing/pass-after-new-test.2026-08-31T20-04.md +AC-MAPPING: AC2 | IMPLEMENTATION: P1-T5 | TESTS: P1-T6 | EVIDENCE: evidence/other/production-diff-scope.2026-08-31T20-04.md +AC-MAPPING: AC3 | IMPLEMENTATION: P1-T2 | TESTS: P1-T2 | EVIDENCE: QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs (new test method) +AC-MAPPING: AC4 | IMPLEMENTATION: P1-T5 | TESTS: P1-T4, P1-T9 | EVIDENCE: evidence/regression-testing/fail-before-new-test.2026-08-31T20-04.md, evidence/regression-testing/pass-after-new-test.2026-08-31T20-04.md +AC-MAPPING: AC5 | IMPLEMENTATION: N/A (no-change requirement) | TESTS: P1-T12 | EVIDENCE: evidence/regression-testing/existing-tests-pass.2026-08-31T20-04.md, evidence/other/test-file-diff-scope.2026-08-31T20-04.md +AC-MAPPING: AC6 | IMPLEMENTATION: N/A (no-change requirement) | TESTS: P1-T6 | EVIDENCE: evidence/other/production-diff-scope.2026-08-31T20-04.md +AC-MAPPING: AC7 | IMPLEMENTATION: N/A (scope-boundary requirement) | TESTS: P2-T8 | EVIDENCE: evidence/qa-gates/footprint-scope.2026-08-31T20-04.md +AC-MAPPING: AC8 | IMPLEMENTATION: N/A (toolchain requirement) | TESTS: P2-T1, P2-T2, P2-T3, P2-T4, P2-T5 | EVIDENCE: evidence/qa-gates/csharpier-format.2026-08-31T20-04.md, evidence/qa-gates/csharpier-check-final.2026-08-31T20-04.md, evidence/qa-gates/msbuild-analyzer-rebuild.2026-08-31T20-04.md, evidence/qa-gates/msbuild-nullable-rebuild.2026-08-31T20-04.md, evidence/qa-gates/vstest-coverage-run.2026-08-31T20-04.md + +UNRESOLVED-GAPS: NONE + +DIRECTIVE: PREFLIGHT VALIDATION ONLY diff --git a/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/policy-audit.2026-09-01T12-53.md b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/policy-audit.2026-09-01T12-53.md new file mode 100644 index 000000000..d0e91aacd --- /dev/null +++ b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/policy-audit.2026-09-01T12-53.md @@ -0,0 +1,248 @@ +# Policy Audit — Issue #646 (qfc-metrics-flush-writes-empty-session-file) + +Timestamp: 2026-09-01T12-53 + +| Field | Value | +|---|---| +| Branch | `bug/qfc-metrics-flush-writes-empty-session-file-646` | +| HEAD | `0fe0668f146236c65aa93514fcb9756d366a6940` | +| Base branch | `origin/main` | +| Merge base | `8996b28746d32f9f5996a037e0ca76be78b7684d` (verified an ancestor of HEAD) | +| Branch diff | 31 files, 3223 insertions, 0 deletions (`git diff --shortstat origin/main...HEAD`) | +| Work mode | `minor-audit` (from the `- Work Mode:` marker in `issue.md`) | +| AC source | `issue.md` section `## Acceptance Criteria` only (AC1-AC8) | +| Blocking findings | **0** | + +## Audit Scope Statement + +The audited scope is the full branch diff against the resolved base branch `origin/main`, not +the scope of any plan, task, or phase. All 31 changed paths were enumerated and reviewed. + +## Rejected Scope Narrowing + +No caller instruction attempted to narrow the audit scope to a plan, task, phase, or file +subset, and no instruction attempted to suppress a toolchain or coverage check for a language +with changed files on the branch. Two strings in the feature folder resemble narrowing +directives and were assessed; neither is one: + +| String | Location | Assessment | +|---|---|---| +| `DIRECTIVE: PREFLIGHT VALIDATION ONLY` | `plan.2026-08-31T20-04.md` line 414 (plan trailer) | Planner-to-executor handoff text governing the plan document's own validation, not the review's scope. Full branch audit performed regardless. | +| `- **Directive:** MINIMAL-AUDIT PLAN REQUIRED` | `plan.2026-08-31T20-04.md` line 10 | Selects the plan template shape for `minor-audit` work mode. It does not limit which files this review examines. | + +The caller instruction that `artifacts/csharp/coverage.xml` must not be created is recorded and +was honoured, but it is not treated as scope narrowing: the C#/.NET coverage rows below carry +explicit verdicts and the absence of that artifact is itself recorded as a FAIL row rather than +suppressed. + +## PR Context Artifacts + +`artifacts/pr_context.summary.txt` and `artifacts/pr_context.appendix.txt` were absent at the +start of this review and were regenerated from `git diff origin/main...HEAD` at HEAD +`0fe0668f`. The `artifacts/` tree is excluded from version control by `.gitignore` line 57 +(`artifacts/`), verified with `git check-ignore -v`, so regenerating them added no tracked path +and left the AC7 footprint unchanged. `git status --porcelain` returns empty after the +regeneration, which is the direct proof. + +The regenerated summary classifies the two changed `.cs` files as C#. This is recorded +explicitly because the generator that normally produces this artifact has a recurring defect in +which C# changes are reported as documentation-only; the classification here was derived +mechanically from the branch diff rather than inherited. + +## Evidence Location Compliance + +`.claude/rules` and `.claude/skills/evidence-and-timestamp-conventions` require execution +evidence at `/evidence//`. The branch diff was scanned for files written under +the prohibited locations `artifacts/baselines/`, `artifacts/qa/`, `artifacts/evidence/`, and +`artifacts/coverage/`. + +| Prohibited prefix | Paths found in branch diff | Verdict | +|---|---|---| +| `artifacts/baselines/` | 0 | PASS | +| `artifacts/qa/` | 0 | PASS | +| `artifacts/evidence/` | 0 | PASS | +| `artifacts/coverage/` | 0 | PASS | + +All 25 evidence files are under +`docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/` +in the canonical `baseline/`, `qa-gates/`, `regression-testing/`, and `other/` subdirectories. +`validate_evidence_locations.py` does not exist in this repository, so the scan was performed +directly against the diff path list. No `EVIDENCE_LOCATION_OVERRIDE_REJECTED` condition arose: +no delegation instruction specified a non-canonical evidence path. + +## Toolchain Compliance (CLAUDE.md C#, CUT3) + +The mandated order is csharpier format, csharpier check, analyzer rebuild, nullable rebuild, +vstest with coverage. Each gate is evidenced with a command line, an exit code, and verbatim +summary output. + +| # | Gate | Command | Exit | Evidence | Verdict | +|---|---|---|---|---|---| +| 1 | Format | `dotnet tool run csharpier format .` | 0 (both passes) | `evidence/qa-gates/csharpier-format.2026-08-31T20-04.md` | PASS | +| 2 | Format check | `dotnet tool run csharpier check .` | 0, 1566 files | `evidence/qa-gates/csharpier-check-final.2026-08-31T20-04.md` | PASS | +| 3 | Analyzers | `msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true` | 0, 5 warnings / 0 errors | `evidence/qa-gates/msbuild-analyzer-rebuild.2026-08-31T20-04.md` | PASS | +| 4 | Type check | `msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:TreatWarningsAsErrors=true` | 0, 5 warnings / 0 errors | `evidence/qa-gates/msbuild-nullable-rebuild.2026-08-31T20-04.md` | PASS | +| 5 | Test | `vstest.console.exe QuickFiler.Test\bin\Debug\QuickFiler.Test.dll /EnableCodeCoverage` | 0, 1285/1285 passed | `evidence/qa-gates/vstest-coverage-run.2026-08-31T20-04.md` | PASS | + +Supporting observations that make these gates meaningful rather than vacuous: + +- **Restart rule honoured.** Format pass 1 rewrote one tracked file (CSharpier collapsed a + three-line FluentAssertions chain in the new test onto one line). The loop restarted from the + format step, as the General Code Change Policy section 8.1 requires. Pass 2 reached a + fixpoint with no newly-modified path and no changed-line growth. The gates numbered 2 through + 5 then ran in one uninterrupted sequence with no further restart. +- **Non-vacuity of the two rebuilds.** Both used `/t:Rebuild`, not `/t:Build`, and both logs + record 36 `csc.exe` command-line occurrences matching baseline. This is the check that + distinguishes a real compile from MSBuild's incremental up-to-date skip, which returns exit 0 + having run no analyzers. The gates were capable of failing. +- **Nullable command fidelity.** The type-check command is character-for-character the CI step + and correctly omits `/p:Nullable=enable`, which is a solution-wide opt-in this repository + does not use. Neither changed file carries a `#nullable enable` pragma, so neither + participates in nullable-flow analysis; the gate's actual reach here is the stronger general + condition that the change introduces no C# compiler warning of any kind, since + `TreatWarningsAsErrors` promotes all of them. +- **Gates remain valid at HEAD.** `git diff --name-only 10aaaf65 HEAD -- "*.cs"` returns empty, + so no C# source changed after the commit these gates ran against. The one later commit + (`0fe0668f`) touches zero `.cs` files. The two `.jacoco.xml` files it adds sit under + `**/evidence/**`, which `.csharpierignore` excludes, so they cannot perturb the format gate. + +## Coverage Verification + +Languages with changed files in the branch diff: **C# only**. TypeScript, Python, and +PowerShell have zero changed files on this branch, so no coverage obligation arises for them +and no verdict is owed. + +### Evidence basis + +The committed coverage evidence is a package-level JaCoCo projection of the raw Cobertura the +runner emitted. The raw reports (52,131,269 bytes, 892,256 lines combined) were converted and +deleted after the three gates that read them had completed, following repository precedent +`d0955dc4`. This reviewer independently re-summed the `LINE` counters in both committed +projections: + +| Report | Re-summed covered | Re-summed valid | Figure the substitution record claims | Match | +|---|---|---|---|---| +| Baseline | 48426 | 142226 | 48426 / 142226 | Exact | +| Final | 48436 | 142240 | 48436 / 142240 | Exact | + +The first-party subset was also re-derived independently from the final projection +(`QuickFiler` + `UtilitiesCS` + `ToDoModel` + `TaskVisualization` + `Tags` + `SVGControl`): +14540 covered of 62121 valid, reproducing the recorded 23.4059% exactly. The substitution is +therefore lossless with respect to every counter any gate relied on, and the gate sequence is +auditable from the committed evidence. The substitution is assessed as **adequately recorded**, +not as missing evidence. + +### Denominator statement + +The measured `line-rate` of 0.3405 is a single-assembly unfiltered figure. Only +`QuickFiler.Test.dll` was executed, and the 15-package denominator includes eight vendored +third-party assemblies plus the `QuickFiler.Test` assembly itself. It is not this repository's +policy denominator, which is nine first-party packages with no `*.Test` assembly, and it is not +quoted here as a repository figure. + +### C# / .NET coverage verdicts + +| # | C# / .NET coverage measure | Floor | Observed | Verdict | +|---|---|---|---|---| +| C1 | C# repository-wide line coverage from the canonical artifact `artifacts/csharp/coverage.xml` | >= 85% | The canonical artifact is absent, so no repository-wide C# line coverage figure exists to evaluate | **FAIL** | +| C2 | C# repository-wide branch coverage from the canonical artifact | >= 75% | The canonical artifact is absent, and the committed run emitted zero `condition-coverage` occurrences, so no C# branch coverage figure exists to evaluate | **FAIL** | +| C3 | C# line coverage of the changed production file `QuickFiler/Controllers/QfcHomeController.Metrics.cs` | >= 85% | 77.60% final against 77.05% baseline (97 of 125 measured lines); improved, but under the floor | **FAIL** | +| C4 | C# new-code line coverage of the four added guard lines | >= 90% | 100% — 3 of 3 instrument-measured lines at `hits=1`; the closing brace emits no sequence point | **PASS** | +| C5 | C# new-branch coverage of the added guard condition | both outcomes exercised | Both exercised — true outcome by the new test, false outcome by the two pre-existing non-empty-array tests | **PASS** | +| C6 | C# no-regression on changed lines coverage | no line moves covered to uncovered | Zero pre-existing lines were changed; every line at `hits=1` in baseline remains `hits=1` | **PASS** | +| C7 | C# no-regression on the like-for-like measured coverage denominator | final not below baseline | 0.3405230596 final against 0.3404862683 baseline, from the identical invocation over the identical assembly set | **PASS** | + +Non-C# languages, recorded for completeness. These carry no verdict because they have zero +changed files on this branch: TypeScript (`coverage/lcov.info`), Python +(`artifacts/python/lcov.info`), PowerShell (`artifacts/pester/powershell-coverage.xml`). + +### Disposition of the four FAIL rows + +C1, C2, and C3 are recorded as FAIL because the measurements they name are either absent or +below the stated floor, and this reviewer does not soften a below-floor or unevidenced row into +a PASS. All three are assessed as **non-blocking**, for reasons that are structural rather than +discretionary: + +1. **C1 and C2 are measurement-provisioning gaps, not code defects.** Producing the canonical + repository-wide artifact requires a full-suite coverage pass across all test assemblies. + That work would change no line of the delivered code and would alter none of C4, C5, C6, or + C7. The gap predates this branch and is unaffected by it. +2. **C2's branch figure was never measured rather than measured at zero.** The run carried + `branch-rate="1"` at the root with zero `condition-coverage` occurrences, verified on the + source before deletion. Reading the projection's zero BRANCH counters as zero branch + coverage would be a misreading of the instrument. The one new branch this change introduces + is separately shown fully exercised at C5. +3. **C3's shortfall is entirely pre-existing and structurally untouchable on this branch.** The + 28 uncovered lines in the changed file are the Outlook-interop `WriteMoveToCalendar` path + and the writer-failure logging branch, both of which predate this change. This change moved + the file's coverage up by 0.55 points and added no uncovered line. Raising the file above + 85% would require either new tests against COM-bound code or a refactor, and AC7 forbids + modifying any file other than the two owned ones — so no remediation is available within + this branch's mandate. +4. **The substantive coverage obligations are met.** C4 clears the 90% new-code floor at 100%, + C5 shows both new branch outcomes exercised, and C6 shows no changed-line regression. These + are the rows that measure what this change actually did. + +No `remediation-inputs` artifact is produced, because no finding requires a code change on this +branch. The C1 and C2 provisioning gap is reported to the caller for separate scheduling. + +## Cross-Language Policy Compliance + +| Policy | Requirement | Observed | Verdict | +|---|---|---|---| +| General Code Change — file size | No file over 500 lines | Production file 231 lines; test file 477 lines (`wc -l` and `awk NR` agree) | PASS | +| General Code Change — design | Simplicity first, mirror existing style | Four-line early return mirroring the EFC sibling; no indirection added | PASS | +| General Code Change — error handling | Fail fast, no silent error swallowing | The guard is a no-content short circuit, not an error path; the writer-failure logging branch is untouched | PASS | +| General Code Change — I/O boundaries | Domain logic testable without filesystem | The writer is an injectable delegate; the new test touches no filesystem | PASS | +| General Unit Test — framework | MSTest, Moq, FluentAssertions | `[TestMethod]`, `Mock`, `.Should().BeFalse(...)` | PASS | +| General Unit Test — determinism | No `Thread.Sleep`, `Task.Delay`, wall-clock waits | None present; the stub returns `Task.FromResult(true)` | PASS | +| General Unit Test — no temp files | Creation of temp files in tests prohibited | None; the writer delegate is replaced with an in-memory flag capture | PASS | +| General Unit Test — AAA structure | Arrange, Act, Assert | Present and visually separated | PASS | +| General Unit Test — documented intent | Descriptive name plus summary | 6-line XML doc comment plus a self-describing method name | PASS | +| General Unit Test — scenario completeness | Boundary and negative cases | The empty-array boundary is the case added; the non-empty side is held by two pre-existing tests | PASS | +| General Unit Test — coverage exclusions | No production path excluded from measurement | No exclusion added; `.csharpierignore` changes none and `coverage.config` is unmodified | PASS | +| Bugfix workflow | Failing regression test first, then minimal fix | RED at exit 1 with a genuine 346 ms assertion failure, then the four-line fix, then GREEN at exit 0 | PASS | +| Tonality | Professional, factual, no hyperbole | Evidence artifacts are measured and specific throughout | PASS | +| Policy documents | Not modified by this branch | No path under `.claude/rules/` or `.github/instructions/` in the diff | PASS | + +### Test file location + +`QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs` mirrors +`QuickFiler/Controllers/QfcHomeController.Metrics.cs` under this repository's established +`.Test/` convention rather than the `tests/` tree named in +`.claude/rules/general-unit-test.md`. The change adds a method to a file that already existed +at that path; it neither creates nor moves a test file, and General Code Change Policy section +7.1 directs matching the existing repository style. Recorded as a pre-existing repository-wide +convention divergence, not a finding against this branch. + +### Tier-dependent gates + +`.claude/rules/quality-tiers.md` requires a `quality-tiers.yml` at the repository root mapping +every project to a tier. That file does not exist in this worktree, so the tier of `QuickFiler` +cannot be resolved and the tier-dependent gates (property-test density, mutation score, golden +tests) cannot be evaluated against a declared tier. This is a pre-existing repository condition +that this branch neither introduced nor was capable of changing under AC7. The uniform gates, +which do not depend on tier, are all evaluated above. + +## Coverage Floor Documentation Conflict + +`CLAUDE.md` UT2 states a repository-wide floor of 80% with 90% for new modules. +`.claude/rules/general-unit-test.md` and `.claude/rules/quality-tiers.md` state a uniform 85% +line and 75% branch floor across T1-T4. The two are unreconciled in the repository. This audit +reports against the stricter 85%/75% pair for the repository-wide and modified-file rows, and +against the 90% new-code floor for the guard lines, which is the stricter reading on both +counts. The delivered result at C4 (100%) clears every variant of the new-code floor. The +conflict is noted so the C3 verdict is read against the correct authority. + +## Summary + +| Category | PASS | FAIL | +|---|---|---| +| Toolchain gates | 5 | 0 | +| Coverage rows | 4 | 3 | +| Cross-language policy | 14 | 0 | +| Evidence location | 4 | 0 | + +**Blocking findings: 0.** The three FAIL coverage rows are measurement-provisioning and +pre-existing-shortfall conditions with explicit non-blocking dispositions recorded above. No +finding on this branch requires a code change. diff --git a/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/research/research-correction.2026-08-31T20-45.md b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/research/research-correction.2026-08-31T20-45.md new file mode 100644 index 000000000..c8f82fee8 --- /dev/null +++ b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/research/research-correction.2026-08-31T20-45.md @@ -0,0 +1,85 @@ +# Orchestrator citation correction to research.2026-08-31T20-30.md + +Timestamp: 2026-08-31T20-45 +Author: orchestrator (preparation mode, issue #646) +Corrects: `research.2026-08-31T20-30.md` + +## Why this correction exists + +The base branch moved while the research delegation was in flight. + +- At preparation start the item branch `bug/qfc-metrics-flush-writes-empty-session-file-646` was cut + from `origin/main` at `9b6aff2e886eb86af5dfc131ebee7a2ebe1a5b6c`. +- Issue #647 (`bug/fileio2-write-retry-reports-success-on-final-failure-647`) was then merged to + `main`, advancing `origin/main` to `2b85134b42872e405602e6064e02dc9cda6c319b`. +- The item branch was fast-forwarded `9b6aff2e..2b85134b` after the research agent had already begun + reading the tree. + +The research artifact therefore describes the **pre-merge** tree. Its structural conclusions remain +correct; its delegate signature, its writer-call text, and several of its line numbers do not. The +sibling change this item was told to defend against has already landed, so it is now present fact +rather than anticipated future state. + +The citations below were re-derived by the orchestrator directly against +`2b85134b42872e405602e6064e02dc9cda6c319b` and supersede the corresponding claims in the research +artifact. + +## Superseded citations + +| Claim in research artifact | Status | Corrected fact at `2b85134b` | +|---|---|---| +| `MetricsFileWriter` is `Func` | Superseded | It is `Func>`, declared at `QuickFiler/Controllers/QfcHomeController.Metrics.cs:28-34`. | +| The writer call is the single-line `await MetricsFileWriter(filename, lines, myDocuments, CancellationToken.None);` at line 179 | Superseded | The call is the multi-line assignment `bool metricsWritten = await MetricsFileWriter(` beginning at line 179 and closing at line 184, followed by the `if (!metricsWritten)` logging branch at lines 185-191. | +| `WriteMetricsAsync` body spans lines 107-180 | Superseded | It spans lines 107-192. | +| Test lambdas return `Task.CompletedTask` | Superseded | Every capturing lambda in the test file now returns `Task.FromResult(true)`, and the `async` one at line 359-364 returns `true`. | +| `WriteMetricsAsync_FiltersNullDiagnosticLinesBeforeWriting` at line 403; `WriteMetricsAsync_WithoutMyDocumentsFolder_DoesNotInvokeWriter` at lines 430-449 | Superseded | They are at lines 404 and 432-450 respectively. | +| The #647 change is pending and will invalidate anchors later | Superseded | The #647 change is already merged and present. No further sibling rewrite of this call site is pending. | + +## Confirmed unchanged by the merge + +These research claims were re-checked and still hold at `2b85134b`: + +- The default binding is `FileIO2.WriteTextFileAsync`, and it opens the target in append mode before + its write loop, so an empty array still creates or touches the file. The root mechanism is intact. +- `WriteMetricsAsync` contains exactly one pre-existing early `return;`, at lines 131-134, in the + `!Globals.FS.SpecialFolders.TryGetValue("MyDocuments", out var myDocuments)` guard. A second early + return is structurally consistent with the method as written. +- The EFC precedent guard is `if (dataLines.Length == 0) { return; }` in + `QuickFiler/Controllers/EfcHomeController.Metrics.cs`, inside `QuickFileMetrics_WRITE`. +- `BuildLooseMetricsController` remains the shared fixture and the correct construction path. +- `WriteMetricsAsync_WithoutMyDocumentsFolder_DoesNotInvokeWriter` remains the best template for the + new test: it is the only existing test whose assertion shape is a zero-invocation `bool` flag. +- Only the one production file and the one test file reference `MetricsFileWriter` in live code, so + the guard cannot break an existing test. Every existing test that asserts invocation supplies + non-empty diagnostics. + +## Anchors for the implementation edit + +Anchor on structure, not on line numbers. Both anchors are inside `WriteMetricsAsync`. + +- **Anchor A (insert after):** the statement that computes the filtered array, + `var lines = strOutput.Where(line => !string.IsNullOrWhiteSpace(line)).ToArray();` (line 174). +- **Anchor B (insert before):** the start of the writer invocation statement, + `bool metricsWritten = await MetricsFileWriter(` (line 179), together with the explanatory comment + block immediately preceding it at lines 176-178. + +The guard belongs between Anchor A and that comment block. + +## Scope boundary that still applies + +The delegate signature and the `if (!metricsWritten)` logging branch are the delivered outcome of +issue #647. This item must not alter either. Its entire production change is the early-return guard. + +## Standing instruction for the executor + +Even though the sibling has landed, reconcile the branch against the current `origin/main` tip at +execution start and re-derive both anchors before editing. This preparation run has already been +invalidated once by a mid-run merge to the same method; the line numbers recorded above are accurate +as of `2b85134b` and carry no guarantee beyond it. + +## Additional constraint discovered during correction + +`QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs` is 454 lines. The General Code Change +Policy caps test files at 500 lines, leaving 46 lines of headroom. The new test must fit within that +headroom, or the file must be split. The plan must verify the post-change line count rather than +assume it. diff --git a/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/research/research.2026-08-31T20-30.md b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/research/research.2026-08-31T20-30.md new file mode 100644 index 000000000..4a96fe0f9 --- /dev/null +++ b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/research/research.2026-08-31T20-30.md @@ -0,0 +1,402 @@ +# Research — Issue #646: QFC metrics flush writes empty session file + +- Issue: #646 +- Feature folder: `docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/` +- Researcher: task-researcher agent +- Date: 2026-08-31 + +## 0. Critical constraint — concurrently in-flight sibling item #647 + +Issue #647 (branch `bug/fileio2-write-retry-reports-success-on-final-failure-647`) rewrites the +same method, `WriteMetricsAsync` in `QuickFiler/Controllers/QfcHomeController.Metrics.cs`, and per +the task brief merges into `main` **before** this item (#646) executes. The #647 rewrite: + +- Changes the `MetricsFileWriter` delegate return type from `Task` to `Task`. +- Replaces the single-line `await MetricsFileWriter(...)` statement with a multi-line assignment + (`bool metricsWritten = await MetricsFileWriter(...)`) followed by an `if (!metricsWritten) { + logger.Error(...); }` failure-logging branch. + +**This item (#646) must not touch the delegate signature or add/modify any failure-logging +branch.** Those are owned exclusively by #647. The #646 fix is limited to inserting one early-return +guard between the point where the filtered `lines` array is computed and the point where the +writer is invoked, regardless of how many statements that invocation later expands into. + +**Anchoring rule for the atomic plan:** anchor the guard's insertion point on (a) the statement +that assigns the filtered array (currently `var lines = strOutput.Where(...).ToArray();`, line 174) +and (b) the start of the writer-invocation statement (currently `await MetricsFileWriter(filename, +lines, myDocuments, CancellationToken.None);`, line 179) — **not** on the literal single-line +await text and **not** on the absolute line number 179, because #647 will change both the literal +text (into a multi-statement assignment + branch) and the downstream line numbers. + +**Citations invalidated by the #647 merge** (must be re-derived at execution time, not trusted from +this document): +- §2 "exact statement text of the writer-invocation line" (line 179) — text and line number both + change. +- §2 "line count of the method body" and all line numbers below 179 that are relative to a + postulated-unchanged file (line numbers 180-216 in this document's citations shift downward by + however many net lines #647 adds). +- §1 "`MetricsFileWriter` delegate signature: `Func`" (lines 28-34) — return type changes to `Task`. +- Any statement in this document asserting the writer call is currently a single `await` statement + with no following branch — after #647 it is not. + +**Citations that remain valid regardless of #647:** +- The filtered-array computation (`strOutput.Where(line => !string.IsNullOrWhiteSpace(line)).ToArray()`) + is untouched by #647's scope (delegate signature + failure branch only); its line number may + shift but its position immediately before the writer call does not change. +- The EFC sibling guard at `QuickFiler/Controllers/EfcHomeController.Metrics.cs:72-75` — that file + is not touched by #647. +- The test-harness patterns in `QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs` + (`BuildLooseMetricsController`, the `MetricsWrite` capture record, the delegate-assignment + pattern) — these will need updating for #647's `Task` return type as part of #647's own + work, but the *pattern* documented here (capture list + assert-on-count) survives. +- `FileIO2.WriteTextFileAsync`'s append-mode `StreamWriter` behavior (§1) — #647 changes the + *caller's* handling of this writer's outcome, not the writer itself in `FileIO2.cs`. + +## 1. The `MetricsFileWriter` seam + +Declared in `QuickFiler/Controllers/QfcHomeController.Metrics.cs:28-34` (current state, pre-#647): + +```csharp +internal Func< + string, + string[], + string, + CancellationToken, + Task +> MetricsFileWriter { get; set; } = FileIO2.WriteTextFileAsync; +``` + +Parameter order: `filename, lines, folderRoot, cancellationToken`. Default-bound to +`FileIO2.WriteTextFileAsync`, a static method in `UtilitiesCS/To Depricate/FileIO2.cs:50-89`. + +`WriteTextFileAsync` behavior with an empty `strOutput` array (`UtilitiesCS/To Depricate/FileIO2.cs:59-73`): + +```csharp +string filepath = Path.Combine(folderpath, filename); +... +while (!success) +{ + try + { + token.ThrowIfCancellationRequested(); + using (var sw = new StreamWriter(filepath, true, System.Text.Encoding.UTF8)) + { + success = true; + foreach (var output in strOutput) + await sw.WriteLineAsync(output); + } + } + ... +} +``` + +The `StreamWriter(filepath, append: true, Encoding.UTF8)` constructor opens the target file — and +creates it if it does not exist — **before** the `foreach` loop runs. With an empty `strOutput` +array the loop body never executes and no line is written, but the `using` block still opens and +closes the file, which is sufficient to create a zero-byte file (if absent) or update the +last-write timestamp of an existing zero-content file. This confirms the defect mechanism stated +in the issue: the guard must exist upstream of the delegate call, because the default writer +implementation has no length check of its own and none should be added to it (that would touch a +file outside this item's scope and duplicate a decision EFC already made at the call site). + +## 2. Current body of `WriteMetricsAsync` and structural anchors + +Full current method, `QuickFiler/Controllers/QfcHomeController.Metrics.cs:107-180` (line numbers +are pre-#647 and will shift after that merge; see §0): + +- Line 107: `public async Task WriteMetricsAsync(string filename)` — method signature, unchanged + by both #646 and #647. +- **Early return #1** (line 131-134): `if (!Globals.FS.SpecialFolders.TryGetValue("MyDocuments", + out var myDocuments)) { return; }` — this is the only `return` statement already present in the + method. It aborts before any diagnostics are computed or the writer is touched. +- Lines 137-160: duration computation, `WriteMoveToCalendar(...)` call (COM calendar write, not + gated by the lines-empty condition and not in scope for #646). +- Lines 162-169: `string[] strOutput = _formController.Groups.GetMoveDiagnostics(...)`. +- **Anchor A — filtered-array computation** (line 174): `var lines = strOutput.Where(line => + !string.IsNullOrWhiteSpace(line)).ToArray();` preceded by a comment (lines 171-173) explaining + the filter defends against a non-null-guaranteed interface contract. +- **Anchor B — writer invocation start** (line 179, preceded by a comment at lines 176-178 + explaining the deliberate `CancellationToken.None` choice): `await MetricsFileWriter(filename, + lines, myDocuments, CancellationToken.None);` — the closing statement of the method body. + +The new guard belongs between Anchor A and Anchor B, textually equivalent to the EFC form: + +```csharp +if (lines.Length == 0) +{ + return; +} +``` + +This becomes early return #2 in the method. It is structurally consistent with the existing early +return #1: both are unconditional `return;` (no value, matching the `Task`-returning +`async Task` signature — no `return null;`/`return default;` needed), both abort before any I/O +that the guarded condition makes meaningless, and neither interacts with `WriteMoveToCalendar`'s +already-completed COM calendar write (the calendar item is written before the diagnostics array is +computed, so guard #2 cannot suppress or duplicate the calendar entry — only the metrics-file +write is skipped, matching the issue's Expected Behavior). + +## 3. Existing test harness — `QfcHomeControllerMetricsTests.cs` + +File: `QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs` (453 lines). + +### Controller construction and writer capture + +`BuildLooseMetricsController` (lines 72-135) is the single fixture-builder used by every test in +this class. Verbatim load-bearing excerpt: + +```csharp +private static ( + QfcHomeController controller, + Mock groups +) BuildLooseMetricsController(string[] diagnostics = null, bool withMyDocuments = true) +{ + ... + var mockGroups = new Mock(MockBehavior.Loose); + mockGroups.SetupGet(x => x.EmailsToMove).Returns(1); + mockGroups + .Setup(x => + x.GetMoveDiagnostics( + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + ref It.Ref.IsAny + ) + ) + .Returns(diagnostics ?? Array.Empty()); + var mockFormController = new Mock(MockBehavior.Loose); + mockFormController.SetupGet(x => x.Groups).Returns(mockGroups.Object); + + mockGlobals.SetupGet(x => x.AF.CancelToken).Returns(CancellationToken.None); + + var controller = new QfcHomeController(mockGlobals.Object, () => { }); + controller.CreateCancellationToken(); + // Replace the production file writer with a no-op. The default seam value is + // FileIO2.WriteTextFileAsync, which probes a real path and retries 100 times over ten + // seconds when the folder is absent; a unit test must not touch the filesystem or wait + // on wall-clock time. Tests that assert on the flush override this with a capturing + // delegate of their own. + controller.MetricsFileWriter = (filename, lines, folderRoot, token) => + Task.CompletedTask; + SetPrivateField(controller, "_formController", mockFormController.Object); + SetPrivateField(controller, "_stopWatchMoved", new Stopwatch()); + return (controller, mockGroups); +} +``` + +`GetMoveDiagnostics` is stubbed via the optional `diagnostics` parameter — pass an array (e.g. all +`null`/whitespace entries) to control what `WriteMetricsAsync` filters. The default fixture +assigns a no-op `MetricsFileWriter`; individual tests overwrite `controller.MetricsFileWriter` +directly (a public/internal settable property, not constructor-injected) with a capturing lambda. + +`SetPrivateField` (lines 58-64) uses reflection +(`GetType().GetField(name, BindingFlags.NonPublic | BindingFlags.Instance).SetValue(...)`) to set +`_formController` and `_stopWatchMoved`, both otherwise-private fields. + +### Capture record + +```csharp +private sealed class MetricsWrite +{ + internal MetricsWrite( + string filename, + string[] lines, + string folderRoot, + CancellationToken token + ) + { + Filename = filename; + Lines = lines; + FolderRoot = folderRoot; + Token = token; + } + + internal string Filename { get; } + internal string[] Lines { get; } + internal string FolderRoot { get; } + internal CancellationToken Token { get; } +} +``` + +Used at lines 304-323, populated by capturing-lambda assignments to `controller.MetricsFileWriter` +at lines 335 (`WriteMetricsAsync_InvokesInjectedMetricsFileWriterOnce`), 359 +(`WriteMetricsAsync_CompletesWriterTaskBeforeReturning`, an async lambda using `await Task.Yield()` +to prove ordering without a wall-clock wait), 382 (`WriteMetricsAsync_PassesUncancelledTokenToWriter`), +and 409 (`WriteMetricsAsync_FiltersNullDiagnosticLinesBeforeWriting`). + +### Closest existing template for the new zero-line boundary test + +Two candidates were evaluated per the task brief: + +- **`WriteMetricsAsync_FiltersNullDiagnosticLinesBeforeWriting`** (lines 402-424): stubs + `GetMoveDiagnostics` to return `new[] { "line-one", " ", null, "line-two" }`, captures the + invocation, and asserts `captures[0].Lines` equals the filtered non-empty set. This test + demonstrates the filtering mechanics and the capture pattern but asserts on invocation + *content*, not *absence of invocation* — it is not directly reusable as a template because its + assertion shape (`captures.Should().ContainSingle()`) is the opposite of what AC3 requires. + +- **`WriteMetricsAsync_WithoutMyDocumentsFolder_DoesNotInvokeWriter`** (lines 430-449): stubs + `GetMoveDiagnostics` to return `new[] { "line-one" }`, sets `withMyDocuments: false`, uses a + boolean `invoked` flag (not a capture list) set inside the writer lambda, and asserts + `invoked.Should().BeFalse(...)` after the call. Verbatim: + + ```csharp + [TestMethod] + public async Task WriteMetricsAsync_WithoutMyDocumentsFolder_DoesNotInvokeWriter() + { + var (controller, _) = BuildLooseMetricsController( + new[] { "line-one" }, + withMyDocuments: false + ); + var invoked = false; + controller.MetricsFileWriter = (filename, written, folderRoot, token) => + { + invoked = true; + return Task.CompletedTask; + }; + + await controller.WriteMetricsAsync("metrics.csv"); + + invoked + .Should() + .BeFalse("the guard must abort before any write when MyDocuments is absent"); + } + ``` + + **This is the correct template.** Its assertion shape — a boolean `invoked` flag flipped inside + the writer lambda, asserted `BeFalse()` after the act — is exactly the zero-invocation outcome + AC3 requires. The only change needed for the new test is the arrange: keep `withMyDocuments` + default (`true`, so the MyDocuments guard at line 131 does not itself cause the early return — + the new guard being tested must be the one that fires) and pass an all-null-or-whitespace + `diagnostics` array (e.g. `new[] { " ", null, "\t" }` or `Array.Empty()`) so that + `GetMoveDiagnostics` returns content that the existing filter at Anchor A reduces to a + zero-length `lines` array. + + Note: `BuildLooseMetricsController`'s `mockGroups.SetupGet(x => x.EmailsToMove).Returns(1)` is + fixed at `1` regardless of the `diagnostics` argument, so the duration-divide-by-emailsLoaded + branch (`WriteMetricsAsync`, lines 145-148) behaves identically to every other test in this file + and needs no special handling for the new test. + +## 4. Other callers / dependents on unconditional writer invocation + +Repository-wide search for `MetricsFileWriter` across `*.cs` files found exactly two matches: + +- `QuickFiler/Controllers/QfcHomeController.Metrics.cs` — the property declaration and default + binding (production). +- `QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs` — all fixture/test usages + documented in §3. + +A third match, `QuickFiler.Test/Controllers/QfcHomeControllerTests.cs:260-274`, contains a fully +commented-out test (`//public async Task WriteMetricsAsync_ExecutesCorrectly() ...`) that is dead +code, not a live caller. + +Repository-wide search for `WriteMetricsAsync` (all extensions) found no other production callers +beyond the method's own definition and doc-file references in the two promoted-potential markdown +files already read (§ this document's context). The only live test-code callers are the six tests +in `QfcHomeControllerMetricsTests.cs` already enumerated in §3 (`WriteMetricsAsync_ReadsMovedStopwatchForDuration`, +`WriteMetricsAsync_UnderGermanCulture_RendersInvariantDecimalSeparator`, +`WriteMetricsAsync_UsesInjectedClock_ForDateAndTimeStamps`, +`WriteMetricsAsync_InvokesInjectedMetricsFileWriterOnce`, +`WriteMetricsAsync_CompletesWriterTaskBeforeReturning`, +`WriteMetricsAsync_PassesUncancelledTokenToWriter`, +`WriteMetricsAsync_FiltersNullDiagnosticLinesBeforeWriting`, +`WriteMetricsAsync_WithoutMyDocumentsFolder_DoesNotInvokeWriter`). + +**Determination: the guard cannot break any existing test.** Every existing test that asserts a +writer invocation happened (`WriteMetricsAsync_InvokesInjectedMetricsFileWriterOnce`, +`WriteMetricsAsync_CompletesWriterTaskBeforeReturning`, +`WriteMetricsAsync_PassesUncancelledTokenToWriter`, +`WriteMetricsAsync_FiltersNullDiagnosticLinesBeforeWriting`) supplies a `diagnostics` array +containing at least one non-null, non-whitespace element (`"line-one"`, `{"line-one","line-two"}`, +or the filtered subset `{"line-one","line-two"}` from a mixed array), so the filtered `lines` +array is never empty for any of them and the new guard never fires in those paths. The three tests +that do not assert on the writer at all (`WriteMetricsAsync_ReadsMovedStopwatchForDuration`, +`WriteMetricsAsync_UnderGermanCulture_RendersInvariantDecimalSeparator`, +`WriteMetricsAsync_UsesInjectedClock_ForDateAndTimeStamps`) supply no `diagnostics` override +(default `Array.Empty()` from `BuildLooseMetricsController`'s `diagnostics ?? +Array.Empty()`), meaning `GetMoveDiagnostics` returns an empty array and — after the guard +is added — `WriteMetricsAsync` will return earlier than it currently does. This is safe: none of +these three tests assert anything about the writer or about method completion timing beyond +`await controller.WriteMetricsAsync(...)` returning normally; they assert only on the +`GetMoveDiagnostics` call arguments via `groups.Verify(...)`, and that call happens before Anchor A +in all cases, so the new guard (which runs strictly after `GetMoveDiagnostics` is called) has no +effect on those assertions. + +`WriteMetricsAsync_WithoutMyDocumentsFolder_DoesNotInvokeWriter` is unaffected because its early +return already fires at the pre-existing MyDocuments guard (line 131), before Anchor A is ever +reached. + +## 5. Build/run facts + +- Test assembly output path (Debug|AnyCPU, the configuration named in both the promoted-potential + doc and AC8): `QuickFiler.Test\bin\Debug\QuickFiler.Test.dll`. Confirmed via + `QuickFiler.Test\QuickFiler.Test.csproj`: `QuickFiler.Test`, + `bin\Debug\` under the Debug|AnyCPU property group; + `v4.8.1`. +- Framework/library versions, from `QuickFiler.Test\packages.config`: + - `MSTest.TestFramework` 4.3.3, `MSTest.TestAdapter` 4.3.3, `MSTest.Analyzers` 4.3.3. + - `Moq` 4.20.72. + - `FluentAssertions` 8.10.0. + - `Microsoft.Extensions.TimeProvider.Testing` 10.9.0 (source of `FakeTimeProvider`, already used + by this test class for the `#222` clock-seam tests). + +## Behavior semantics (AC-derived) + +- **Success condition**: when the filtered `lines` array (post null/whitespace filter, Anchor A) + has `Length == 0`, `WriteMetricsAsync` returns without invoking `MetricsFileWriter` — matching + AC1/AC2 and the EFC sibling's contract at `EfcHomeController.Metrics.cs:72-75`. +- **Failure/regression condition**: any invocation of `MetricsFileWriter` when `lines.Length == 0` + is a defect recurrence; the new MSTest regression test asserts this via a `bool invoked` flag + (per the §3 template), matching AC3. +- **Ordering rule**: the new guard must execute after Anchor A (the filter must run first so the + guard observes the *filtered* count, not the raw `strOutput` count) and before Anchor B (must + prevent the writer call, not run concurrently with or after it). +- **Non-interaction with calendar write**: `WriteMoveToCalendar` (lines 154-160) executes before + Anchor A and is unconditional in both the pre-fix and post-fix method; the new guard does not + suppress the Outlook calendar appointment, only the file write. This is consistent with the + issue's Expected Behavior section, which addresses only the session-metrics file. +- **Out-of-scope boundary (AC6)**: the `MetricsFileWriter` delegate's `Task` vs. `Task` + return type and any failure-logging branch belong to #647, not this item; this item's fix is a + single `return;` statement inserted between Anchor A and Anchor B, structurally independent of + what Anchor B's statement looks like after #647 lands. + +## Candidate approaches + +1. **Insert `if (lines.Length == 0) { return; }` between Anchor A and Anchor B (recommended).** + Textually mirrors the EFC guard exactly, as directed by AC2 and the issue's "Proposed Fix" + section. Minimal diff, single early return, consistent with the existing early-return #1 at + line 131. No interaction with the `Task`/`Task` signature question owned by #647. +2. **Add the same guard inside `FileIO2.WriteTextFileAsync` instead of the caller.** Rejected: it + would touch `UtilitiesCS/To Depricate/FileIO2.cs`, a file outside this item's owned-files list + (AC7), would affect every other caller of that shared writer (broader blast radius than the + issue describes), and the issue explicitly frames the remedy as "one guard in an owned file, + mirroring the EFC form" at the call site, not inside the shared writer. + +**Rejected alternatives**: approach 2 above (guard inside the shared writer) — out of scope file, +broader blast radius, contradicts the issue's stated remedy location. + +## Testing implications + +- One new MSTest test in `QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs`, in the + `#region Issue #442 — metrics flush tests` block (or a new adjoining region referencing #646), + following the `WriteMetricsAsync_WithoutMyDocumentsFolder_DoesNotInvokeWriter` template + identified in §3: `BuildLooseMetricsController` with an all-null/whitespace `diagnostics` array + and default `withMyDocuments: true`; a `bool invoked` flag set in the `MetricsFileWriter` lambda; + assert `invoked.Should().BeFalse(...)` after `await controller.WriteMetricsAsync(...)`. +- Per AC4, capture fail-before evidence (test run against the unguarded implementation showing the + new test fails) under `docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/regression-testing/`. +- No mocks/stubs beyond what `BuildLooseMetricsController` already provides are needed; no + filesystem or wall-clock access occurs in the new test (the writer lambda is a synchronous + boolean-flag setter returning `Task.CompletedTask`, consistent with UT4/CUT2 policy). +- Per AC5, `WriteMetricsAsync_InvokesInjectedMetricsFileWriterOnce` and + `WriteMetricsAsync_FiltersNullDiagnosticLinesBeforeWriting` must not be modified; §4 established + neither test's `diagnostics` input produces an empty filtered array, so no modification is + needed for them to keep passing. +- Full toolchain per CLAUDE.md / CUT3: `dotnet tool run csharpier format .` (verify with `check`), + the analyzer `msbuild /t:Rebuild ... /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true`, + the nullable `msbuild /t:Rebuild ... /p:TreatWarningsAsErrors=true`, then + `vstest.console.exe QuickFiler.Test\bin\Debug\QuickFiler.Test.dll /EnableCodeCoverage`. +