fix(825): close the residual ETL deadline mechanics left by #811

[drmoisan]

fix(825): close the residual ETL deadline mechanics left by #811

Summary

• Threads an optional TimeProvider through GetTableInViewAsync, so the table-acquisition deadline and both of its retry attempts are armed on the caller's clock instead of the system clock.
• Replaces the hard-coded 2000 ms argument in the TimeoutException retry recursion with the caller's timeoutMs, so both attempts are governed by the same caller-visible deadline. This is proven by a red-before / green-after regression test.
• Makes EtlAsync's tuple return type admit the null it can actually return (object[,]? data) and removes the null-forgiving suppression on that return, so the null-snapshot path is expressed in the type system rather than suppressed.
• Deletes two inert integer-repeat TimeoutAfter overloads and the dead EtlAsyncOld method, together with their three test callers. UtilitiesCS/Threading/TimeOutTask.cs drops 45 lines.
• Documents the unmeasured 250 * rowCount ETL budget in place, without changing the expression or its literal, and names the live-Outlook capture route that would produce a real measurement.
• Removes [DoNotParallelize] from OlTableExtensions_Tests.cs only after the production change eliminated the wall-clock hazard that justified it, and adds the missing verified-reason comment to the one class that keeps it.

All 35 acceptance criteria in spec.md are delivered and independently reviewed: 35 PASS, 0 PARTIAL, 0 FAIL, 0 UNVERIFIED, blocking findings 0.

Why

Issue #811 placed the 250 ms per-row ETL deadline and the 1000 ms dataframe-transform deadline under an injectable TimeProvider and added a descriptive guard for the null-snapshot path. Six residual items were deliberately excluded from that fix to keep its blast radius to a bugfix. None of them is a regression introduced by #811; this change closes them as a set.

The central defect was that GetTableInViewAsync acquired its table under a deadline armed on the system clock, so no test could govern it deterministically, and its TimeoutException retry silently substituted a literal 2000 ms for whatever the caller asked for. A caller that requested 750 ms got 750 ms on the first attempt and 2000 ms on the second, with no way to observe the substitution.

The invariant this change establishes

Every wall-clock deadline reachable from DfDeedle.GetEmailDataInViewAsync — including the table-acquisition deadline in GetTableInViewAsync and both of its retry attempts — is now armed on the TimeProvider the caller supplied, is measured against the caller's timeoutMs rather than any literal, and EtlAsync's declared return type admits the null it can return, so no null-forgiving suppression remains anywhere on that path.

What Changed

Core production change (5 files)

File	Change
UtilitiesCS/OutlookObjects/Table/OlTableExtensions.TableAccess.cs	Adds trailing TimeProvider? timeProvider = null; resolves the deadline factory once from timeProvider ?? TimeProvider.System; propagates the provider through both retry recursions; replaces the literal 2000 with timeoutMs.
UtilitiesCS/OutlookObjects/Table/OlTableExtensions.Etl.cs	EtlAsync return type becomes (object[,]? data, ...); the null-forgiving suppression on its return is removed; the budget-rationale comment is added; EtlAsyncOld is deleted.
UtilitiesCS/Threading/TimeOutTask.cs	Both inert integer-repeat TimeoutAfter overloads deleted (−45 lines).
UtilitiesCS/Extensions/DfDeedle.cs	Forwards timeProvider: to GetTableInViewAsync; reads the tuple through its data name so the existing null guard's flow state carries to both consumers.
UtilitiesCS/Extensions/DfDeedle.QfcColumns.cs	Corrects a stale doc-comment reference from TableEtlInvoker to DefaultTableEtl.

The two deleted overloads were inert: their inner TimeoutAfter(int) returns a faulted Task rather than throwing synchronously, so their catch (TimeoutException) could never run and the repeatAttempts parameter never had any effect.

Tests (6 files)

• New UtilitiesCS.Test/OutlookObjects/Table/GetTableInViewAsyncClockTests.cs (+289) — four tests: the retry-literal regression, the injected-clock arming proof, explicit-factory precedence, and the default-path guard proving a null provider still resolves to the system clock.
• OlTableExtensions_Tests.cs — four reflection binding sites gain a TimeProvider entry; the EtlAsyncOld test is deleted; [DoNotParallelize] and the stale class comment are removed.
• DfDeedleEtlTimeoutTests.cs — a third test-owned gate on table acquisition, so each production timer arms inside its own await/re-arm window on the latch-based barrier. No assertion added or removed.
• TimeOutTask_Tests.cs — the two WithRepeatAttempts tests are deleted; a verified-reason comment is added above the retained [DoNotParallelize].
• OlTableExtensionsEtlClockTests.cs — doc comment only.
• UtilitiesCS.Test.csproj — exactly one added Compile Include line, nothing else.

Architecture / How It Fits Together

GetTableInViewAsync previously passed the caller's timeoutSourceFactory straight to TimeOutTask.RunWithTimeout, and RunWithTimeout fell back to new CancellationTokenSource(ms) — a system-clock timer — when that factory was null.

The change inserts one resolution step in the caller. A local resolvedTimeoutSourceFactory prefers an explicitly supplied factory, and otherwise builds one from (timeProvider ?? TimeProvider.System).CreateCancellationTokenSource(TimeSpan.FromMilliseconds(ms)). That resolved local is what reaches RunWithTimeout. Precedence is therefore explicit and testable: an explicit factory still wins, a supplied provider governs when it does not, and a null provider reproduces the previous production behaviour exactly.

DfDeedle.GetEmailDataInViewAsync already threaded its timeProvider into AddQfcColumnsAsync and EtlAsync; it now does the same for the table-acquisition call, completing the pattern.

A deliberate consequence, recorded because it drove a spec amendment: threading the provider inserts the table-acquisition timer as the first arming signal on the ArmingBarrierTimeProvider that one existing test consumes in a fixed order. The barrier's latch drops a signal whenever two timers arm inside one await window, so that test needed a third gate. The alternative — not forwarding the provider — was rejected because it leaves the production deadline on the system clock, which is the defect this item exists to close.

Verification

Completed

Full C# toolchain, run in order, with one loop restart when the formatter reflowed this change's own new code:

Gate	Result
dotnet tool run csharpier format .	exit 0 — ChangedFileCount: 0 on the final pass
dotnet tool run csharpier check .	exit 0 — 1623 files
msbuild analyzers (/t:Rebuild)	exit 0 — 0 warnings, 0 errors
msbuild nullable (/t:Rebuild, TreatWarningsAsErrors)	exit 0 — 0 warnings, 0 errors
Full suite with coverage	exit 0 — 7213 passed, 0 failed, 0 skipped

Gate non-vacuity measured, not assumed: SkippingCoreCompileCount: 0 and CscInvocationsForWriteSetProjects: 4 on both MSBuild logs. /t:Rebuild is used deliberately — MSBuild's up-to-date check does not invalidate on a command-line /p: change, so a warm /t:Build returns exit 0 with CoreCompile skipped and the gate cannot fail.

Red-before / green-after: GetTableInViewAsync_TimeoutRetry_UsesCallerTimeoutMsNotLiteral2000 was captured failing against the pre-change tree (EXIT_CODE: 1, ExpectedExitCode: 1, recording that the second value the factory received was 2000 rather than 750) and passes after.

Coverage:

• Repository line rate 0.856211 → 0.856686; branch rate 79.84%. Both above the 85% / 75% floors.
• UtilitiesCS package LinesValid 43287 → 43246 (−41), LinesCovered 39010 ≥ the 38973 floor.
• Changed-line coverage 20/22 = 0.909091, at or above the 0.90 new-code threshold.
• Deletion accounting reconciles exactly: 55 deleted − 14 added = 41, residual 0.

No threshold, exclusion list or analyzer severity was lowered, weakened or deleted. No production file was added to a coverage exclusion list, and no [ExcludeFromCodeCoverage] attribute was added.

Determinism: no Thread.Sleep, Task.Delay, Stopwatch, DateTime.Now or timing tolerance was added to any test. Deadline behaviour is tested only through FakeTimeProvider and the latch-based ArmingBarrierTimeProvider. The Task.Delay(200) and Task.Delay(50) calls in TimeOutTask_Tests.cs are pre-existing; this change only added a comment naming them as the reason that class keeps [DoNotParallelize].

Recommended

dotnet tool run csharpier check .
msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:EnableNETAnalyzers=true /p:EnforceCodeStyleInBuild=true
msbuild TaskMaster.sln /t:Rebuild /m /p:Configuration=Debug "/p:Platform=Any CPU" /p:TreatWarningsAsErrors=true
pwsh -File scripts/vscode/Invoke-MSTestWithCoverage.ps1 -SearchRoot . -CoverageOutput coverage/verify.cobertura.xml

Backward Compatibility / Migration Notes

• GetTableInViewAsync gains a trailing optional parameter. Existing call sites compile unchanged, and a null provider resolves to TimeProvider.System, so production timing is unchanged.
• EtlAsync's return type changes from (object[,] data, ...) to (object[,]? data, ...). This is a nullability annotation change, not a shape change. Every in-repo consumer is updated; the nullable gate passes at zero warnings, which is the evidence that the annotation propagated correctly.
• Removed public API: the two integer-repeat TimeoutAfter overloads. They were inert (see above) and had no production caller besides EtlAsyncOld, which is also removed. The proxy-returning TimeProvider overloads are retained untouched.
• EtlAsyncOld is removed. It had one production caller, inside the code also deleted.

Risks and Mitigations

Risk	Mitigation
A caller relied on the retry silently using 2000 ms rather than its own smaller timeoutMs	The prior behaviour was undocumented and untestable. The new behaviour is what the parameter's name promises, and the default remains 2000 ms when no value is supplied.
Removing [DoNotParallelize] from OlTableExtensions_Tests.cs reintroduces flakiness	Removed only after the production change eliminated the hazard, in the order spec.md Risks mandates. The justification is the code change, not observed green runs — RunsObserved: 0 is recorded deliberately, and the green full-suite run explicitly declines to be cited as the justification.
TimeProvider.System.CreateCancellationTokenSource behaves differently from new CancellationTokenSource(ms)	Member availability was compile-proven against a real csc invocation (CS1061Count: 0, with both confounders excluded). Behavioural equivalence rests on the default-path guard test rather than on IL inspection of the BCL shim; residual risk is low and is recorded in the code review.
A CancelAfter call is later added to a provider-created source	An in-source comment warns against it: on pre-.NET 8 runtimes, and net481 is one, CancelAfter(TimeSpan) does not terminate the original delay timer.

Review Guide

Suggested order:

1. docs/features/active/2026-09-08-etl-deadline-mechanics-follow-ups-825/plan.2026-09-08T23-51.md — read the adjudicated design conflict section first. It records a measured conflict between two acceptance criteria and the decision that closed it.
2. The five production files, in the table above.
3. GetTableInViewAsyncClockTests.cs — the new test file.
4. feature-audit.2026-09-10T00-45.md — all 35 criteria evaluated row by row.
5. evidence/other/plan-deviations.md — five recorded deviations, each adjudicated.

Diff noise warning. 64 files change, but seven of them are machine artifacts totalling roughly 740,000 lines: two Cobertura XML documents and five MSBuild logs. The reviewable surface is 11 source files plus about 60 small Markdown artifacts. Automated PR-context classification reports "Core logic changes: 0 files" for this branch; that is a misclassification caused by top-N-churn truncation, not an accurate description.

All five committed MSBuild logs were sanitised of absolute host paths before commit, and carry zero occurrences of the host-profile path token.

Follow-ups

Four follow-ups are recorded in evidence/other/ac35-reachability-observation.md and are deliberately deferred until after this epic merges, plus one raised by review:

1. GetTableInViewAsync returns null on timeout behind a non-null contract. Traced and documented here, not fixed. On the ordinary timeout path TimeOutTask.RunWithTimeout absorbs the cancellation, exhausts its single internal retry, and returns default(TResult) without throwing, so neither catch block is entered and the method returns null through return table!. This change does not alter that behaviour.
2. UtilitiesCS/Threading/TimeOutTask.cs remains 966 lines, over the 500-line file cap. This change is a reduction, not a resolution, of that cap violation.
3. Report the outcome to Bug: utilitiescs-test-determinism-780-803-594 #811, whose acceptance criteria are affected, once this reaches main.
4. The unmeasured 250 * rowCount budget still needs a real measurement, collected from LogTableTiming payloads in a live Outlook session.
5. New, raised by review: UtilitiesCS.Test/OutlookObjects/Table/OlTableExtensions_Tests.cs is about 1822 lines and no existing follow-up names it. Recommend the epic add it.

Non-blocking review suggestions not applied here, because they would touch test files after the recorded final clean toolchain pass: add a [Timeout] attribute to the three barrier-awaiting tests so a regression fails rather than hangs, and have the new class comment name the specific reason it keeps [DoNotParallelize].

GitHub Auto-close

None. GitHub CLI validation was unavailable when this context was collected, so no closing keyword is emitted. This pull request also targets the epic integration branch rather than the repository default branch, where GitHub does not honour closing keywords in any case.

Issue #825 should be closed by the epic integration pull request when epic/review-residuals-2026-09-08-integration reaches main. Issues #780, #798 and #811 appear in this branch's context as references only and are not resolved by this change.

🤖 Generated with Claude Code

https://claude.ai/code/session_01URyGon15RvDXcUEo96tvyh