fix(quickfiler): skip the metrics write when no diagnostic lines survive (#646)

[drmoisan]

fix(quickfiler): skip the metrics write when no diagnostic lines survive (#646)

Summary

• QfcHomeController.WriteMetricsAsync filtered null and whitespace diagnostic lines but then awaited MetricsFileWriter unconditionally, so a session producing no diagnostic content still created a zero-content session-metrics file, or refreshed the last-write timestamp of an existing one.
• Adds a four-line early return between the filter and the writer call, textually equivalent to the guard the EmailFiler path already carries in EfcHomeController.Metrics.cs.
• Adds one 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.
• Production diff is 4 added lines and 0 removed. The MetricsFileWriter delegate signature and the if (!metricsWritten) failure branch, both owned by Bug: fileio2-write-retry-reports-success-on-final-failure #647, are untouched.
• Feature review returned PASS with zero blocking findings; all eight acceptance criteria were re-derived against evidence rather than accepted from checkboxes.

Why

The QuickFiler and EmailFiler metrics writers diverged. 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.

This is a narrow regression rather than a long-standing defect. Before the #442 flush 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 guard was deliberately not applied inside the #442 feature. That work was complete and its toolchain green when the finding was raised in review, and the General Code Change Policy directs opening a new issue rather than widening scope in flight.

What Changed

Core fix

• QuickFiler/Controllers/QfcHomeController.Metrics.cs — early return when the filtered diagnostic-line array is empty, placed immediately after the filter statement and before the CancellationToken.None comment, so that comment stays adjacent to the writer statement it explains.

Tests

• QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs — adds WriteMetricsAsync_WithAllNullOrWhitespaceDiagnostics_DoesNotInvokeWriter. The two pre-existing metrics tests are provably unmodified: the test-file diff contains zero removal lines.

Docs and evidence

• Plan, acceptance-criteria reconciliation, and 24 evidence artifacts covering Phase 0 baseline, fail-before/pass-after regression, and Phase 2 final-QC gates.

Architecture / How It Fits Together

WriteMetricsAsync computes diagnostics through IQfcCollectionController.GetMoveDiagnostics, filters null and whitespace entries, then awaits the injectable MetricsFileWriter seam, which defaults to FileIO2.WriteTextFileAsync. That default opens the target for append, which is why an empty array still had a filesystem effect. The new guard returns before the seam is reached, so no writer implementation is invoked at all rather than being invoked with an empty payload.

Verification

Completed

• dotnet tool run csharpier format . then dotnet tool run csharpier check . — exit 0. The format pass rewrote the new test's chained assertion, so the QC loop was restarted; pass 2 was a fixpoint.
• msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true — exit 0, Build succeeded, 5 warnings, 0 errors.
• msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:TreatWarningsAsErrors=true — exit 0, Build succeeded, zero CS86xx.
• vstest.console.exe QuickFiler.Test\bin\Debug\QuickFiler.Test.dll /EnableCodeCoverage — exit 0, 1285 passed of 1285 (baseline was 1284 of 1284).
• Fail-before run: exit 1, Failed: 1, 346 ms, failing on the test's own assertion message with invoked observed as True.
• Pass-after run: exit 0, Passed: 1. The two pre-existing tests: Passed: 2.

Coverage

New-code coverage on the guard is 3 of 3 measurable lines. The closing brace emits no <line> entry in the report; this was verified structural by comparing against the identically shaped EFC guard in the same report, whose closing brace also emits no entry in long-standing fully-exercised code.

Two qualifications a reviewer should not skip:

• The recorded line-rate of 0.3405 is not a repository-wide or policy coverage figure. Only QuickFiler.Test.dll was run, and the denominator includes eight vendored third-party assemblies and the test assembly itself. The no-regression comparison is still valid, because baseline and final came from the identical invocation, and it moved up (0.3404862683 to 0.3405230596). The first-party subset of the same run moved from 23.4022% to 23.4059%.
• Branch coverage was not measured by this run. The report contained zero condition-coverage occurrences and a root branch-rate="1", so the zero BRANCH counters in the committed projections are faithful rather than a parse failure. The guard's two branch outcomes are covered behaviourally instead: the true path by the new test, the false path by the two pre-existing non-empty tests.

Coverage evidence is committed as package-level JaCoCo projections rather than the raw Cobertura the runner emits, following the precedent in d0955dc4. Each projection reconciles exactly to its source root lines-covered and lines-valid (48426/142226 baseline, 48436/142240 final). evidence/qa-gates/coverage-artifact-substitution.2026-09-01T16-41.md records the conversion so the gate sequence stays auditable.

Recommended

• Re-run the four-command C# toolchain above.
• Confirm the branch diff stays within the three paths AC7 permits.

Backward Compatibility / Migration Notes

No breaking changes. No public API, signature, or file was renamed or removed. The only behavioural change is the absence of a filesystem write in a case that previously wrote nothing meaningful.

Risks and Mitigations

• Risk: a consumer depends on the session-metrics file existing, or on its timestamp advancing, even with no content. Mitigation: the repository contains no reader for that CSV, and the EFC path has behaved this way for longer. Rollback: revert the four-line guard.
• Risk: the guard suppresses a write that should have happened. Mitigation: it triggers only when every diagnostic line is null or whitespace, so no content can be lost. WriteMetricsAsync_InvokesInjectedMetricsFileWriterOnce continues to prove the non-empty path still writes.

Review Guide

1. QuickFiler/Controllers/QfcHomeController.Metrics.cs — the four added lines.
2. QuickFiler.Test/Controllers/QfcHomeControllerMetricsTests.cs — the one added test.
3. evidence/qa-gates/coverage-delta-verification.2026-08-31T20-04.md — the coverage argument and its explicit denominator caveats.
4. evidence/qa-gates/coverage-artifact-substitution.2026-09-01T16-41.md — why the committed coverage evidence is a projection.

The remaining 27 paths are evidence artifacts and can be skimmed.

Follow-ups

None are promoted from this branch, because AC7 restricts the diff to a named path set. They are listed here for consolidation after merge:

• CR-1 (Minor) — GetMoveDiagnostics still has no null-array guard, so a narrower null asymmetry with the EFC reference survives. Not reachable today: it needs an IQfcCollectionController implementation returning null, and the sole production implementation does not.
• CR-2 (Minor) — WriteMoveToCalendar runs before the new guard, so an empty-diagnostics session still creates an Outlook calendar appointment. Reachable on every occurrence of the reported input, but not a regression: that session previously produced the appointment and an empty file. Whether the appointment should also be suppressed is a product decision, and both AC1 and the issue's Expected Behavior are scoped to MetricsFileWriter.
• CR-3 (Minor) — the test file is now 477 lines against the 500-line cap. The next comparable addition breaches it and must split the file first.
• CR-4 (Nit) — the new test leaves TimeProvider at TimeProvider.System. No assertion depends on the clock, and this matches every sibling test.
• CR-5 (Informational) — two evidence artifacts quote superseded intermediate figures; both are reconciled elsewhere in the same evidence set.
• REPO-1 (Informational) — quality-tiers.yml does not exist at the repository root though .claude/rules/quality-tiers.md declares it the source of truth, so tier-dependent gates are unevaluable. CLAUDE.md UT2 also states an 80/90 coverage floor while .claude/rules states a uniform 85/75. Both are pre-existing and repository-wide.

GitHub Auto-close

• Closes Bug: qfc-metrics-flush-writes-empty-session-file #646

The PR-context bundle proposed #442, #647 and #CR-1 alongside #646, and reported the GitHub CLI as unavailable. That report is incorrect: the CLI is present and was queried directly. #442 and #647 are both already CLOSED and are cited in this item's prose only as context; #CR-1 is a code-review finding identifier, not an issue. Only #646 is closed here.