From 579e0d2283e9a0d9f2766ca810c24ca22fc13017 Mon Sep 17 00:00:00 2001 From: Dan Moisan Date: Mon, 31 Aug 2026 20:40:20 -0400 Subject: [PATCH 1/6] docs(issue-646): prepare minimal-audit plan for qfc metrics empty-array guard Preparation-only outputs for issue 646. This commit modifies no production or test source. issue.md adds the Acceptance Criteria section (AC1-AC8) that the minor-audit contract requires as the sole acceptance-criteria source; the scaffolded file carried none. plan.2026-08-31T20-04.md is the three-phase minimal-audit plan, validator clean, cleared preflight in two rounds. research/ holds the task-researcher findings plus an orchestrator correction recording that origin/main advanced mid-run and superseding the citations that change invalidated. Planned footprint at execution: one early-return guard in QuickFiler/Controllers/QfcHomeController.Metrics.cs, one regression test in QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs, and this feature folder. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01ATYLDoRLKXS5sgAzegW7ZL --- .../issue.md | 142 ++++++ .../plan.2026-08-31T20-04.md | 414 ++++++++++++++++++ .../research-correction.2026-08-31T20-45.md | 85 ++++ .../research/research.2026-08-31T20-30.md | 402 +++++++++++++++++ 4 files changed, 1043 insertions(+) create mode 100644 docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/issue.md create mode 100644 docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/plan.2026-08-31T20-04.md create mode 100644 docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/research/research-correction.2026-08-31T20-45.md create mode 100644 docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/research/research.2026-08-31T20-30.md 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..270a6ab04 --- /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 + +- [ ] 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. +- [ ] 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; }`). +- [ ] 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. +- [ ] 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. +- [ ] AC5: The existing tests `WriteMetricsAsync_InvokesInjectedMetricsFileWriterOnce` and + `WriteMetricsAsync_FiltersNullDiagnosticLinesBeforeWriting` still pass and are not modified. +- [ ] 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. +- [ ] AC7: No repository file outside `QuickFiler/Controllers/QfcHomeController.Metrics.cs`, + `QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs`, and this feature folder is modified. +- [ ] 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) +- [ ] 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..c91502f99 --- /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`). + +- [ ] [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. +- [ ] [P0-T2] Read `.claude/rules/general-code-change.md` in full. Acceptance: recorded in + the P0-T5 artifact. +- [ ] [P0-T3] Read `.claude/rules/general-unit-test.md` in full. Acceptance: recorded in the + P0-T5 artifact. +- [ ] [P0-T4] Read `.claude/rules/csharp.md` in full. Acceptance: recorded in the P0-T5 + artifact. +- [ ] [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. +- [ ] [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:`. +- [ ] [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`. +- [ ] [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`. +- [ ] [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`. +- [ ] [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`. +- [ ] [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 + +- [ ] [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`. +- [ ] [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. +- [ ] [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. +- [ ] [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`. +- [ ] [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. +- [ ] [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`. +- [ ] [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. +- [ ] [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`. +- [ ] [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`. +- [ ] [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. +- [ ] [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. +- [ ] [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`. +- [ ] [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`. +- [ ] [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. +- [ ] [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. + +- [ ] [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`. +- [ ] [P2-T2] Run `dotnet tool run csharpier check .`. Acceptance: `EXIT_CODE 0`. Evidence: + `evidence/qa-gates/csharpier-check-final.2026-08-31T20-04.md`. +- [ ] [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`. +- [ ] [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`. +- [ ] [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`. +- [ ] [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`. +- [ ] [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`. +- [ ] [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`. +- [ ] [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. +- [ ] [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. +- [ ] [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/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`. + From 9f578b3c89f02f2f61ad3127af1c1c9d2e8ce2b2 Mon Sep 17 00:00:00 2001 From: Dan Moisan Date: Mon, 31 Aug 2026 20:41:14 -0400 Subject: [PATCH 2/6] docs(agent-memory): record mid-run base-move hazard from issue 646 preparation Captures that a parallel sibling described as pending can merge during a preparation run, moving origin/main under a subagent that is already reading the tree, and that git status stays clean while it happens. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01ATYLDoRLKXS5sgAzegW7ZL --- .claude/agent-memory/orchestrator/MEMORY.md | 1 + ...-item-can-merge-during-your-preparation.md | 21 +++++++++++++++++++ 2 files changed, 22 insertions(+) create mode 100644 .claude/agent-memory/orchestrator/sibling-item-can-merge-during-your-preparation.md diff --git a/.claude/agent-memory/orchestrator/MEMORY.md b/.claude/agent-memory/orchestrator/MEMORY.md index b9d642c85..b996cf7ae 100644 --- a/.claude/agent-memory/orchestrator/MEMORY.md +++ b/.claude/agent-memory/orchestrator/MEMORY.md @@ -114,6 +114,7 @@ - [Shared checkpoint: never read-modify-write](shared-checkpoint-read-modify-write-corrupts.md) - a sibling swaps the session-root file - [External actor can merge your child PR mid-run](external-actor-can-merge-your-child-pr-midrun.md) — re-read PR state before the CI gate - [Stale base anchor passes ancestry vacuously](stale-base-anchor-passes-ancestry-vacuously.md) — on a prep resume the pinned base stays an ancestor, so the check passes while diffs bill another issue's work to your plan +- [Sibling item can merge DURING your preparation](sibling-item-can-merge-during-your-preparation.md) — origin/main moves under a running subagent; git status stays clean, so compare HEAD:file vs origin/main:file - [orchestrator-state.json is TRACKED in git](orchestrator-state-json-is-tracked-in-git.md) — .gitignore does not apply; writing your checkpoint pollutes the footprint. Fix with skip-worktree ## Artifact hygiene diff --git a/.claude/agent-memory/orchestrator/sibling-item-can-merge-during-your-preparation.md b/.claude/agent-memory/orchestrator/sibling-item-can-merge-during-your-preparation.md new file mode 100644 index 000000000..f691dec46 --- /dev/null +++ b/.claude/agent-memory/orchestrator/sibling-item-can-merge-during-your-preparation.md @@ -0,0 +1,21 @@ +--- +name: sibling-item-can-merge-during-your-preparation +description: A parallel item's "in-flight sibling that will merge before you execute" can merge DURING your preparation run, silently invalidating a subagent already reading the tree +metadata: + type: feedback +--- + +When a parallel/epic parent hands you an item and warns that a sibling item is in flight and "will merge before this one executes", treat that merge as something that can land **mid-run**, not as a future event you plan around. + +**Why:** On issue #646 (2026-08-31) the parent described sibling #647 as pending and instructed me to author anchors that would survive its future rewrite of the same method. I cut the branch at `origin/main` = `9b6aff2e` and launched `task-researcher`. While the researcher was reading, #647 merged as PR #712 and `origin/main` advanced to `2b85134b`. The researcher's citations therefore described a superseded tree: it recorded the delegate as returning `Task` and the writer call as a single-line `await`, when the merged tree already had `Task` and a multi-line assignment plus a failure-logging branch. + +The failure is quiet in a specific way. `git rev-parse HEAD` and `git rev-parse origin/main` agreed at branch-creation time and `git status` stayed clean, so nothing looked wrong. The divergence only surfaced when I compared `git rev-parse HEAD:` against `git rev-parse origin/main:` and got different blobs. A plain `git status` cannot show this, because the working tree genuinely matches the HEAD you pinned — it is the remote ref that moved underneath you. + +**How to apply:** +- Re-run `git fetch` and re-compare `HEAD` to `origin/main` at every phase boundary of a preparation run, not once at the start. Do it specifically before delegating planning and before committing. +- When a sibling is named as touching your exact call site, check whether it has ALREADY merged before you write the plan: `git merge-base --is-ancestor origin/ origin/main`. On #646 that returned true, which is what exposed the whole problem. Also note `git diff origin/main...origin/` returning empty means merged, not means "no changes". +- A branch with no commits of its own can be re-anchored with `git merge --ff-only origin/main`. Do NOT reach for `git reset --hard`; it is blocked by a dangerous-command hook in this repo. +- You cannot course-correct a running subagent. There is no `SendMessage` tool in the orchestrator's surface, and a fresh `Agent(...)` call starts a SECOND agent rather than redirecting the first (see [[agent-tool-cannot-course-correct-running-subagent]]). The workable recovery is to let it finish, re-derive its load-bearing citations yourself against the corrected tree, and commit a separate correction artifact alongside its output rather than editing its file. That keeps the researcher's artifact honest as-authored and makes the supersession auditable. +- Fold the correction into the planner's prompt explicitly, naming which document wins where the two disagree. The planner will otherwise cite the stale artifact, and preflight will spend a round on it. + +Related: [[stale-base-anchor-passes-ancestry-vacuously]], [[external-actor-can-merge-your-child-pr-midrun]], [[epic-child-stale-local-integration-ref]], [[reconcile-plan-numbers-against-your-own-measurements]]. From 8a2054cd6c857195712c7db6cee0a34b631f3ca7 Mon Sep 17 00:00:00 2001 From: Dan Moisan Date: Tue, 1 Sep 2026 12:08:00 -0400 Subject: [PATCH 3/6] chore(issue-646): restore agent-memory to base for the footprint criterion The preparation run recorded an orchestrator memory entry on this branch. Acceptance criterion AC7 restricts the branch diff to the two owned source files and the feature folder, so the entry is reverted here. Its content is carried in the run report for re-filing from a separate branch after merge. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01ATYLDoRLKXS5sgAzegW7ZL --- .claude/agent-memory/orchestrator/MEMORY.md | 1 - ...-item-can-merge-during-your-preparation.md | 21 ------------------- 2 files changed, 22 deletions(-) delete mode 100644 .claude/agent-memory/orchestrator/sibling-item-can-merge-during-your-preparation.md diff --git a/.claude/agent-memory/orchestrator/MEMORY.md b/.claude/agent-memory/orchestrator/MEMORY.md index 99be3a907..1a7240169 100644 --- a/.claude/agent-memory/orchestrator/MEMORY.md +++ b/.claude/agent-memory/orchestrator/MEMORY.md @@ -116,7 +116,6 @@ - [Shared checkpoint: never read-modify-write](shared-checkpoint-read-modify-write-corrupts.md) - a sibling swaps the session-root file - [External actor can merge your child PR mid-run](external-actor-can-merge-your-child-pr-midrun.md) — re-read PR state before the CI gate - [Stale base anchor passes ancestry vacuously](stale-base-anchor-passes-ancestry-vacuously.md) — on a prep resume the pinned base stays an ancestor, so the check passes while diffs bill another issue's work to your plan -- [Sibling item can merge DURING your preparation](sibling-item-can-merge-during-your-preparation.md) — origin/main moves under a running subagent; git status stays clean, so compare HEAD:file vs origin/main:file - [orchestrator-state.json is TRACKED in git](orchestrator-state-json-is-tracked-in-git.md) — .gitignore does not apply; writing your checkpoint pollutes the footprint. Fix with skip-worktree ## Artifact hygiene diff --git a/.claude/agent-memory/orchestrator/sibling-item-can-merge-during-your-preparation.md b/.claude/agent-memory/orchestrator/sibling-item-can-merge-during-your-preparation.md deleted file mode 100644 index f691dec46..000000000 --- a/.claude/agent-memory/orchestrator/sibling-item-can-merge-during-your-preparation.md +++ /dev/null @@ -1,21 +0,0 @@ ---- -name: sibling-item-can-merge-during-your-preparation -description: A parallel item's "in-flight sibling that will merge before you execute" can merge DURING your preparation run, silently invalidating a subagent already reading the tree -metadata: - type: feedback ---- - -When a parallel/epic parent hands you an item and warns that a sibling item is in flight and "will merge before this one executes", treat that merge as something that can land **mid-run**, not as a future event you plan around. - -**Why:** On issue #646 (2026-08-31) the parent described sibling #647 as pending and instructed me to author anchors that would survive its future rewrite of the same method. I cut the branch at `origin/main` = `9b6aff2e` and launched `task-researcher`. While the researcher was reading, #647 merged as PR #712 and `origin/main` advanced to `2b85134b`. The researcher's citations therefore described a superseded tree: it recorded the delegate as returning `Task` and the writer call as a single-line `await`, when the merged tree already had `Task` and a multi-line assignment plus a failure-logging branch. - -The failure is quiet in a specific way. `git rev-parse HEAD` and `git rev-parse origin/main` agreed at branch-creation time and `git status` stayed clean, so nothing looked wrong. The divergence only surfaced when I compared `git rev-parse HEAD:` against `git rev-parse origin/main:` and got different blobs. A plain `git status` cannot show this, because the working tree genuinely matches the HEAD you pinned — it is the remote ref that moved underneath you. - -**How to apply:** -- Re-run `git fetch` and re-compare `HEAD` to `origin/main` at every phase boundary of a preparation run, not once at the start. Do it specifically before delegating planning and before committing. -- When a sibling is named as touching your exact call site, check whether it has ALREADY merged before you write the plan: `git merge-base --is-ancestor origin/ origin/main`. On #646 that returned true, which is what exposed the whole problem. Also note `git diff origin/main...origin/` returning empty means merged, not means "no changes". -- A branch with no commits of its own can be re-anchored with `git merge --ff-only origin/main`. Do NOT reach for `git reset --hard`; it is blocked by a dangerous-command hook in this repo. -- You cannot course-correct a running subagent. There is no `SendMessage` tool in the orchestrator's surface, and a fresh `Agent(...)` call starts a SECOND agent rather than redirecting the first (see [[agent-tool-cannot-course-correct-running-subagent]]). The workable recovery is to let it finish, re-derive its load-bearing citations yourself against the corrected tree, and commit a separate correction artifact alongside its output rather than editing its file. That keeps the researcher's artifact honest as-authored and makes the supersession auditable. -- Fold the correction into the planner's prompt explicitly, naming which document wins where the two disagree. The planner will otherwise cite the stale artifact, and preflight will spend a round on it. - -Related: [[stale-base-anchor-passes-ancestry-vacuously]], [[external-actor-can-merge-your-child-pr-midrun]], [[epic-child-stale-local-integration-ref]], [[reconcile-plan-numbers-against-your-own-measurements]]. From 10aaaf655db7b359ebafc20c72378d26506aedde Mon Sep 17 00:00:00 2001 From: Dan Moisan Date: Tue, 1 Sep 2026 12:47:28 -0400 Subject: [PATCH 4/6] fix(quickfiler): skip the metrics write when no diagnostic lines survive WriteMetricsAsync filtered null and whitespace diagnostic lines but then awaited MetricsFileWriter unconditionally. The default writer opens the target for append, so a session producing no diagnostic content created a zero-content session-metrics file, or refreshed the last-write timestamp of an existing one. Add the same early return the EmailFiler path already carries in EfcHomeController.Metrics.cs, placed between the filter and the writer call. The MetricsFileWriter delegate signature and the failure-logging branch are owned by issue 647 and are unchanged here. Covered by a new MSTest regression that stubs an all-null-or-whitespace diagnostics array and asserts the injected writer is never invoked. It fails against the unguarded implementation and passes after the guard. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01ATYLDoRLKXS5sgAzegW7ZL --- .../QfcHomeControllerMetricsTests.cs | 23 +++++++++++++++++++ .../Controllers/QfcHomeController.Metrics.cs | 4 ++++ 2 files changed, 27 insertions(+) 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 From 0fe0668f146236c65aa93514fcb9756d366a6940 Mon Sep 17 00:00:00 2001 From: Dan Moisan Date: Tue, 1 Sep 2026 12:47:37 -0400 Subject: [PATCH 5/6] docs(issue-646): record execution evidence and acceptance reconciliation Phase 0 baseline, Phase 1 fail-before/pass-after regression evidence, and Phase 2 final-QC gate artifacts for the metrics empty-array guard, plus the plan checklist and the eight acceptance criteria checked off against them. Coverage evidence is committed as package-level JaCoCo projections rather than the raw Cobertura the runner emits, following the precedent set in d0955dc4. Each projection reconciles exactly to its source root lines-covered and lines-valid, and coverage-artifact-substitution records the conversion so the gate sequence stays auditable. The measured line-rate is labelled as a single-assembly unfiltered denominator. Only QuickFiler.Test ran, and the denominator includes vendored third-party assemblies and the test assembly itself, so the figure is not the repository-wide policy denominator. The no-regression comparison holds because both sides were produced by the identical invocation. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01ATYLDoRLKXS5sgAzegW7ZL --- .../baseline/baseline-coverage.jacoco.xml | 62 ++++++ .../branch-reconciliation.2026-08-31T20-04.md | 50 +++++ ...age-cobertura-baseline.2026-08-31T20-04.md | 113 ++++++++++ .../csharpier-check.2026-08-31T20-04.md | 46 ++++ ...build-analyzer-rebuild.2026-08-31T20-04.md | 55 +++++ ...build-nullable-rebuild.2026-08-31T20-04.md | 44 ++++ .../baseline/phase0-instructions-read.md | 50 +++++ .../vstest-coverage-run.2026-08-31T20-04.md | 55 +++++ .../anchor-rederivation.2026-08-31T20-04.md | 116 ++++++++++ .../production-diff-scope.2026-08-31T20-04.md | 123 +++++++++++ .../test-file-diff-scope.2026-08-31T20-04.md | 94 ++++++++ .../test-file-line-count.2026-08-31T20-04.md | 48 ++++ ...-artifact-substitution.2026-09-01T16-41.md | 201 +++++++++++++++++ ...verage-cobertura-final.2026-08-31T20-04.md | 81 +++++++ ...age-delta-verification.2026-08-31T20-04.md | 208 ++++++++++++++++++ .../csharpier-check-final.2026-08-31T20-04.md | 37 ++++ .../csharpier-format.2026-08-31T20-04.md | 135 ++++++++++++ .../qa-gates/final-coverage.jacoco.xml | 62 ++++++ .../footprint-scope.2026-08-31T20-04.md | 129 +++++++++++ ...build-analyzer-rebuild.2026-08-31T20-04.md | 56 +++++ ...build-nullable-rebuild.2026-08-31T20-04.md | 82 +++++++ .../vstest-coverage-run.2026-08-31T20-04.md | 85 +++++++ .../existing-tests-pass.2026-08-31T20-04.md | 56 +++++ .../fail-before-new-test.2026-08-31T20-04.md | 91 ++++++++ .../pass-after-new-test.2026-08-31T20-04.md | 74 +++++++ .../issue.md | 18 +- .../plan.2026-08-31T20-04.md | 74 +++---- 27 files changed, 2199 insertions(+), 46 deletions(-) create mode 100644 docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/baseline-coverage.jacoco.xml create mode 100644 docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/branch-reconciliation.2026-08-31T20-04.md create mode 100644 docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/coverage-cobertura-baseline.2026-08-31T20-04.md create mode 100644 docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/csharpier-check.2026-08-31T20-04.md create mode 100644 docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/msbuild-analyzer-rebuild.2026-08-31T20-04.md create mode 100644 docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/msbuild-nullable-rebuild.2026-08-31T20-04.md create mode 100644 docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/phase0-instructions-read.md create mode 100644 docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/baseline/vstest-coverage-run.2026-08-31T20-04.md create mode 100644 docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/other/anchor-rederivation.2026-08-31T20-04.md create mode 100644 docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/other/production-diff-scope.2026-08-31T20-04.md create mode 100644 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 create mode 100644 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 create mode 100644 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 create mode 100644 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 create mode 100644 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 create mode 100644 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 create mode 100644 docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/csharpier-format.2026-08-31T20-04.md create mode 100644 docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/final-coverage.jacoco.xml create mode 100644 docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/evidence/qa-gates/footprint-scope.2026-08-31T20-04.md create mode 100644 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 create mode 100644 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 create mode 100644 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 create mode 100644 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 create mode 100644 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 create mode 100644 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 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/issue.md b/docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/issue.md index 270a6ab04..1ab9e87f2 100644 --- 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 @@ -113,30 +113,30 @@ Raised as finding CR-1 in ## Acceptance Criteria -- [ ] AC1: `WriteMetricsAsync` in `QuickFiler/Controllers/QfcHomeController.Metrics.cs` returns without +- [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. -- [ ] AC2: The guard is an early return placed between the statement that computes the filtered +- [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; }`). -- [ ] AC3: A new MSTest regression test in `QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs` +- [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. -- [ ] AC4: The new regression test fails against the unguarded implementation and passes after the +- [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. -- [ ] AC5: The existing tests `WriteMetricsAsync_InvokesInjectedMetricsFileWriterOnce` and +- [x] AC5: The existing tests `WriteMetricsAsync_InvokesInjectedMetricsFileWriterOnce` and `WriteMetricsAsync_FiltersNullDiagnosticLinesBeforeWriting` still pass and are not modified. -- [ ] AC6: The `MetricsFileWriter` delegate signature and the writer's failure-handling branch are +- [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. -- [ ] AC7: No repository file outside `QuickFiler/Controllers/QfcHomeController.Metrics.cs`, +- [x] AC7: No repository file outside `QuickFiler/Controllers/QfcHomeController.Metrics.cs`, `QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs`, and this feature folder is modified. -- [ ] AC8: The C# toolchain passes in order in a single final pass: `csharpier format` then +- [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) -- [ ] Move to active fix folder / branch +- [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 index c91502f99..c771bde0a 100644 --- 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 @@ -77,43 +77,43 @@ Policy reads follow the `policy-compliance-order` sequence: `CLAUDE.md` (positio (position 3), `.claude/rules/csharp.md` (position 4, applicable because both in-scope files are `*.cs`). -- [ ] [P0-T1] Read `CLAUDE.md` in full at the repository root. Acceptance: the read is +- [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. -- [ ] [P0-T2] Read `.claude/rules/general-code-change.md` in full. Acceptance: recorded in +- [x] [P0-T2] Read `.claude/rules/general-code-change.md` in full. Acceptance: recorded in the P0-T5 artifact. -- [ ] [P0-T3] Read `.claude/rules/general-unit-test.md` in full. Acceptance: recorded in the +- [x] [P0-T3] Read `.claude/rules/general-unit-test.md` in full. Acceptance: recorded in the P0-T5 artifact. -- [ ] [P0-T4] Read `.claude/rules/csharp.md` in full. Acceptance: recorded in the P0-T5 +- [x] [P0-T4] Read `.claude/rules/csharp.md` in full. Acceptance: recorded in the P0-T5 artifact. -- [ ] [P0-T5] Write the Phase 0 policy-read evidence artifact to +- [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. -- [ ] [P0-T6] Run `git fetch origin`, then reconcile the current branch onto the +- [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:`. -- [ ] [P0-T7] Run `dotnet tool run csharpier check .` from the repository root. Record the +- [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`. -- [ ] [P0-T8] Run `msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug +- [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`. -- [ ] [P0-T9] Run `msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug +- [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`. -- [ ] [P0-T10] Resolve `vstest.console.exe` via `vswhere.exe` (not on PATH in this +- [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 @@ -122,7 +122,7 @@ are `*.cs`). 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`. -- [ ] [P0-T11] Locate the `.coverage` file created by P0-T10 (`Get-ChildItem -Path +- [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\ @@ -135,7 +135,7 @@ are `*.cs`). ### Phase 1 — Constrained Implementation -- [ ] [P1-T1] Re-derive the two edit anchors against the current tree (post-P0-T6 +- [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), @@ -143,7 +143,7 @@ are `*.cs`). 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`. -- [ ] [P1-T2] [expect-fail] Add a new MSTest regression test method, +- [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 @@ -154,12 +154,12 @@ are `*.cs`). `Task.FromResult(true)`, call `await controller.WriteMetricsAsync("metrics.csv")`, then assert `invoked.Should().BeFalse(...)`. Acceptance: the method exists verbatim as described in the file. -- [ ] [P1-T3] Check off AC3 in `issue.md` (`A new MSTest regression test in +- [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. -- [ ] [P1-T4] [expect-fail] Rebuild the test project: `msbuild +- [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 @@ -175,7 +175,7 @@ are `*.cs`). `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`. -- [ ] [P1-T5] Apply the fix: insert +- [x] [P1-T5] Apply the fix: insert ``` if (lines.Length == 0) { @@ -188,51 +188,51 @@ are `*.cs`). 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. -- [ ] [P1-T6] Verify the production diff is scoped to exactly the new guard: run `git diff +- [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`. -- [ ] [P1-T7] Check off AC6 in `issue.md` (`The MetricsFileWriter delegate signature and the +- [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. -- [ ] [P1-T8] Check off AC2 in `issue.md` (`The guard is an early return placed between the +- [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`. -- [ ] [P1-T9] Rebuild the test project again (`msbuild +- [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`. -- [ ] [P1-T10] Check off AC1 in `issue.md` (`WriteMetricsAsync ... returns without invoking +- [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. -- [ ] [P1-T11] Check off AC4 in `issue.md` (`The new regression test fails against the +- [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. -- [ ] [P1-T12] Run the two existing tests `WriteMetricsAsync_InvokesInjectedMetricsFileWriterOnce` +- [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`. -- [ ] [P1-T13] Verify the test-file diff contains zero removed lines: run `git diff +- [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`. -- [ ] [P1-T14] Check off AC5 in `issue.md` (`The existing tests +- [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. -- [ ] [P1-T15] Verify the post-change line count of the test file: run `(Get-Content +- [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`. @@ -243,7 +243,7 @@ Run P2-T1 through P2-T5 in order, unconditionally. If any of P2-T1 through P2-T5 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. -- [ ] [P2-T1] Run `git status --porcelain` and record the set of modified paths and their +- [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 @@ -251,23 +251,23 @@ non-zero `EXIT_CODE`, or if P2-T1 rewrites any tracked file, restart the loop fr 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`. -- [ ] [P2-T2] Run `dotnet tool run csharpier check .`. Acceptance: `EXIT_CODE 0`. Evidence: +- [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`. -- [ ] [P2-T3] Run `msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug +- [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`. -- [ ] [P2-T4] Run `msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug +- [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`. -- [ ] [P2-T5] Resolve `vstest.console.exe` via `vswhere.exe` (same resolution as P0-T10), +- [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`. -- [ ] [P2-T6] Locate the `.coverage` file created by P2-T5 (same discovery method as +- [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 @@ -275,7 +275,7 @@ non-zero `EXIT_CODE`, or if P2-T1 rewrites any tracked file, restart the loop fr 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`. -- [ ] [P2-T7] Coverage delta verification: compare the baseline `line-rate` (P0-T11) to the +- [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 `= 90% new-code floor). Evidence: `evidence/qa-gates/coverage-delta-verification.2026-08-31T20-04.md`. -- [ ] [P2-T8] Verify the total change footprint: run `git status --porcelain` and `git diff +- [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`. -- [ ] [P2-T9] Check off AC7 in `issue.md` (`No repository file outside +- [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. -- [ ] [P2-T10] Check off AC8 in `issue.md` (`The C# toolchain passes in order in a single +- [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. -- [ ] [P2-T11] Final reconciliation: read `issue.md` and confirm all eight items under +- [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. From 9b16bf67321a656f6ccac10cbde66ce621b83202 Mon Sep 17 00:00:00 2001 From: Dan Moisan Date: Tue, 1 Sep 2026 13:01:03 -0400 Subject: [PATCH 6/6] docs(issue-646): record feature-review audit artifacts Policy audit, code review, and feature audit for the metrics empty-array guard. Verdict is PASS with zero blocking findings and all eight acceptance criteria re-derived against evidence rather than accepted from checkboxes. Six non-blocking findings are recorded with their reachability. Two are worth carrying forward: GetMoveDiagnostics still has no null-array guard, which no production implementation can currently reach, and the calendar appointment is written before the new early return, so an empty-diagnostics session still creates one. The latter is not a regression, since that session previously produced the appointment and an empty file. Three coverage rows are recorded FAIL rather than softened. Two are absent measurement artifacts that predate this branch, and the third is the changed file at 77.60 percent against an 85 percent floor, where the uncovered lines are Outlook-interop paths that AC7 forbids this branch from touching. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01ATYLDoRLKXS5sgAzegW7ZL --- .../code-review.2026-09-01T12-53.md | 230 ++++++++++++++++ .../feature-audit.2026-09-01T12-53.md | 142 ++++++++++ .../policy-audit.2026-09-01T12-53.md | 248 ++++++++++++++++++ 3 files changed, 620 insertions(+) create mode 100644 docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/code-review.2026-09-01T12-53.md create mode 100644 docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/feature-audit.2026-09-01T12-53.md create mode 100644 docs/features/active/2026-08-27-qfc-metrics-flush-writes-empty-session-file-646/policy-audit.2026-09-01T12-53.md 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/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/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.