Let the host declare LocalDataService's write-lock budget instead of every caller inheriting the dispatcher's

[erikdarlingdata]

LocalDataService.OpenWriteConnectionAsync is the one write-lock caller in Lite that passes a timeout, and its own remarks say why: it sits on the path a WPF dispatcher awaits, so it cannot afford the unbounded wait the app's other nineteen write-lock acquisitions take. DuckDbInitializer.s_dbLock is process-wide. Put those two facts in a host with no dispatcher and the timeout is protecting nobody while still deciding outcomes — a store call queues behind every other class in the process and then fails on a budget sized for a user who is not there.

The budget now resolves from a runtime configuration property the host declares in its own project file. The shipped app declares none and resolves LocalDataService.DefaultWriteLockBudget, five seconds, unchanged. Lite.Tests declares 120 seconds, so an acquisition queued behind a neighbour's deliberate hold waits it out instead of expiring inside it.

Why the wait is now bounded rather than merely luckier

Four test methods in three classes take the process-wide lock EXCLUSIVELY and hold it while they assert on timing — AnalysisPassTokenThreadingTests.TheReadLockWaitIsAbandonableWhileAWriterHoldsIt (holder net 30 s), StatusBarSizeReadLockTests.GetUsedDataSizeMb_WhenTheWriteLockIsHeld_GivesUpInsteadOfBlocking (15 s), and DismissReliabilityTests' two WriteLock_* methods (2 s each). Every one of those nets is a safety valve on an event wait, so the hold length is decided by scheduling rather than by the ceiling, and three of the four ceilings sit at or above five seconds. Their file-wise maxima sum to 47 s, which is the most that can queue in front of one acquisition; 120 s clears it by 2.5x. Two pins carry that. EveryDeliberateHoldInThisSuiteIsReadable requires every duration-shaped call site in a holder file to be one the scan can READ, and TheBudgetOutlastsEveryDeliberateHoldInThisSuite derives the sum from the suite's own source on every run rather than restating it. The readability pin is what makes the sum a measurement rather than a shape: a spelling outside the recognised set does not go uncounted, it joins the population and contributes zero, so an unreadable duration fails loudly instead.

Short of infinite on purpose: a genuinely wedged lock — the leaked-reader failure DuckDbInitializer.LockReleaser documents — still fails, with this method's own message, instead of hanging the step.

It does not hide a product regression that the old budget would have caught. A five-second fuse that fires in 1 of 194 runs is a coin flip rather than a detector, and the tests that exist to pin lock-hold behaviour pass their own explicit timeouts — DismissReliabilityTests' 50 ms and 100 ms, DuckDbInitializer.StatusBarReadLockTimeout's 100 ms — none of which this touches. A product path that genuinely started holding the lock for tens of seconds would show up as suite duration, which is measured here to the second.

The two measurements the issue asked for, and how they were taken

A shared collection costs the Lite suite +186 s, roughly doubling the step. Measured on one runner, one commit, back to back, by running the suite twice in the build job — once normally, once with -parallel none — and uploading both TRX files (run 34122782002; the instrumentation is commit 65b4065af, reverted in 9446d754e, so it is not in this diff). Parallel span 231.7 s, fully serialised 449.3 s: +217.6 s, 1.94x, at an effective 2.90-way parallelism. Those two ratios are different quantities and neither is derived from the other: 2.90 is observed concurrency (summed per-test durations over the parallel span), 1.94 is end-to-end speedup. The +186 s below rests on the summed per-class intervals rather than on either ratio. Charging each class the serialised interval since the previous test ended — which bills it for its own fixture schema build — the 75 classes that touch s_dbLock carry 417.9 s of that 449.3 s, 93.0%. Leaving the other 232 classes parallel recovers only ~21 s, so a collection over the set that would actually have to be in it lands at ≈418 s: +186 s, 1.80x, which would put the step's new median above today's p95 (389 s). That +186 s is a LOWER bound on option 1 rather than an estimate of it: the per-class costs come from a run with no contention at all, while a collection would still have 232 classes running alongside it.

The timeout fires in 1 of 194 runs, 0.52%. Across the last 300 build.yml runs (2026-09-05T13:17Z to 2026-09-07T10:14Z, ~45 h) the Run Lite tests step executed 194 times — 63 skipped by the path gate, 41 cancelled by a re-push. Five of the 194 failed, and reading each failure's log: two were genuine pin failures (WatermarkPolicyTests, CrossAppMcpToolInventoryPinTests), two were CrossAppGuardCiGateTests file-in-use IOExceptions, and exactly one was this timeout — run 34091806113, whose 487 s step sits at p97.9 of the 194 step durations against a p50 of 262 s. It is a slow-runner failure, and four runs that were slower still passed.

The choice, against those figures

The collection was not chosen, and cost is the second reason rather than the first. As scoped in the issue it cannot bound the race at all: [Collection] only stops classes in the same collection running concurrently with each other, and three of the four longest holders build their own DuckDbInitializer and are not classes of SharedDuckDbFixture, so a collection over the store-touching classes leaves the 30 s and 15 s holders running alongside it. DatabaseStateWriteLockTests' own remarks already say this — "DisableParallelization only orders collections inside the non-parallel bucket — it cannot stop other classes from contending on a static lock". Widening it to all 75 lock-touching classes is what the +186 s buys, and it stays unenforceable: nothing makes the next store test remember to join.

Against that, the host budget costs no wall clock, applies at the one choke point every caller already goes through so no future test class can forget it, and turns the failure into a bounded wait. It also does what DatabaseStateWriteLockTests records as needed — "that arm needs the timeout injected, i.e. production shape changed for testability. Left visible rather than implied-covered."

The per-database lock was not chosen either, and what s_dbLock protects across instances was established first rather than assumed. The rule on the field says: "ONE lock for the whole process, deliberately: Lite constructs several DuckDbInitializer instances over the same file (MainWindow, DatabaseStateOverridesWindow, DuckDbAlertHistoryStore), and making it per-instance would trade a slow test suite for a real data race." So it protects instances over the SAME FILE, and keying it per path would preserve that in an app that has exactly one App.DatabasePath. It was still refused: its failure mode is two spellings of one path resolving to two locks, which is silent data corruption rather than a loud timeout — the wrong direction for a mistake to fail in — and it would additionally rewrite the #2463 rule and the four-file source pin (DuckDbLockModelTests.TheLockRuleIsWrittenWhereEveryCallerAlreadyLooks) that keeps it findable.

This is a product change, deliberately

Option 1 would have been test-only; this is not. The measurement is what justifies crossing that line: the test-only option is a permanent +186 s on every Lite run, does not cover the holders it needs to, and cannot be enforced — so "cheaper class of change" was not available at any price worth paying. What crossing it costs is a demonstration that the app is unchanged, and that is what the first two pins are for: OnlyTheTestHostDeclaresABudget scans every .csproj, .props and .targets in the repo outside build output and requires that Lite.Tests/Lite.Tests.csproj is the only one naming the key — whole-tree, because a shared Directory.Build.props would reach the shipped app just as effectively as its own project file — and TheAppKeepsTheDispatchersFiveSeconds pins the value a host that declares nothing resolves.

ThisHostResolvedTheBudgetItsProjectFileDeclares is the one that catches a seam that is wired but inert. A property that never reaches AppContext resolves the default silently, which is indistinguishable from the pre-change behaviour and would leave the flake in place with every other pin green; so the expected value is read out of the project file and compared against what the running host resolved, and the budget is additionally required to differ from the default, because matching the default is exactly what a dead seam looks like.

The seam had a third outcome, at both ends

Review caught that the resolver bounded the wrong end, and it bears directly on the thesis above: the argument for this change is that a budget either lets the wait succeed or fails loudly, and a declared value that throws during resolution is neither. NumberStyles.Float admits exponents and .NET parses the invariant Infinity symbol whatever the style, so "1e300" and "Infinity" arrive as positive doubles and overflow TimeSpan.FromSeconds. Out of a static initializer that is a TypeInitializationException on the first store call anywhere in the process — strictly worse than the timeout it replaces. seconds > 0 is mutation-tested and bounds the other end.

Fixed rather than justified, and the fix is not the obvious one. Bounding by TimeSpan's representable range would have been wrong: measured, TimeSpan.MaxValue.TotalSeconds is 9.22e11 — about 29,000 years, not the 9.22e14 an eye-estimate gives — and FromSeconds of it round-trips without complaint. That bound accepts a budget which makes a wedged lock hang for the life of the process, converting the loud failure into a silent one. So the ceiling is one hour, on the ground that a wait outliving the process or the job containing it cannot produce this method's timeout at all, and being short of forever is the whole reason there is a number.

Writing the hostile-input pin then found the same hole at the low end, which neither the review nor the original guard covered: double.Epsilon ("5E-324") parses, is positive, is far under any ceiling, and TimeSpan.FromSeconds rounds it to TimeSpan.Zero — a timeout that gives up without waiting, a budget in name only. So the resolver now checks the TimeSpan it constructed, not the double it parsed, which is the shape that catches both ends by construction rather than by enumerating them.

The framework exception is named as prose rather than as a cref throughout, which DocCommentHygieneTests requires: a cref names a symbol this repository spells, and TypeInitializationException is declared nowhere here. The two mentions that predate this change both spell it <c>.

Two pins, and they are not redundant. TheResolverIsTotalAndAlwaysReturnsAUsableBudget asserts over hostile inputs that resolution never throws and never leaves (TimeSpan.Zero, MaxWriteLockBudget]. TheCeilingIsAcceptedAndAnythingPastItResolvesTheDefault derives both figures from the constant. Setting the ceiling to TimeSpan.MaxValue leaves the totality pin GREEN — nothing throws and everything is in-band once the band is 29,000 years wide — and only the ceiling pin reds. The totality pin alone would have accepted the bound that was wrong.

The derivation claimed to round up. It rounded down, twice.

Review turned the "what legal spelling would this miss" question on the scan I wrote and found two gaps, both in the unsafe direction — which falsifies the justification I gave for the bound, not merely its coverage.

It searched only the top level of Lite.Tests while the build-file scan recursed. Lite.Tests/Helpers/ already exists with .cs files, so a helper there taking the write lock would never be scanned. Verified with a positive control: the same 200 s holder placed under Helpers/ reds the bound with recursion on, and with recursion off is entirely invisible — 31 green, no finding at all.

The worse one is a different failure: a zero that looks like a measurement. The scan read TimeSpan.From* and .Join(<int>) but not a bare-millisecond .Wait(30000). Such a file still matches the write-lock scan, so it joins the population and contributes TimeSpan.Zero through DefaultIfEmpty. Measured against the old parser: gate.Wait(TimeSpan.FromSeconds(30)) scores 30.0 s and gate.Wait(30000) scores 0.0 s — the same thirty-second hold, one visible and one reporting itself as no hold at all. Gap one omits a file; gap two names a population it did not measure, which is worse.

Fixed structurally rather than by documenting the blind spots, because a labelled wrong sum is still a wrong sum. The scan now recurses, reads the bare-millisecond spellings, and reports a duration it cannot read as a FAILURE rather than a zero — so a file with no duration site at all remains a real zero (it holds the lock across straight-line code) and stays distinguishable from a file whose duration was unreadable. That distinction is what was missing, and it is the same move as checking the TimeSpan the resolver constructed instead of the double it parsed: make the derivation total rather than enumerate the ways it can be wrong.

It reads CSharpSourceWalker.StripCommentsAndStrings — the shared walk this project already compiles in, rather than a fifth private copy — so a duration written in a comment cannot raise the bar and string.Join is not mistaken for Thread.Join. That is load-bearing: reading raw text instead reds the readability pin on exactly that string.Join.

The bar itself is unchanged at 47 s, so the 120 s budget and its 2.5x margin stand. What changed is that the figure is now a measurement of a population the scan can actually see.

Two figures worth accounting for before they are read as evidence

Lite.Tests Total 3437 to 3457 is fully accounted and none of it is drift. The issue quotes 3437 from an older dev; at this branch's measurement base (641c4db7f) the suite was already 3441, measured on both passes of run 34122782002. WriteLockBudgetTests adds seven [Fact] plus nine [Theory] cases — sixteen — and 3441 + 16 = 3457 exactly. The two dev merges in this branch brought only Darling/Darling.Tests changes (assertion shapes in eight files, then RepoFileAdoptionTests), which move no Lite count.

The suite did not get twice as fast; the 485 s in the issue is the outlier. Across the 194 executions the step's p50 is 262 s and its p90 is 346 s; the failing run the issue was filed from sits at p97.9. On one runner, back to back, this branch measured 231.9 s parallel and 449.4 s serialised, and the 259.8 s run is the median of the population, not a halving of it. Nothing in this diff changes how the suite is invoked: .github/workflows/build.yml is byte-identical to dev (git diff origin/dev -- .github/workflows/build.yml is empty). The instrumentation commit did alter the invocation — a -trx argument and a second -parallel none pass — and that is precisely why it is not in this diff.

Instrumenting the workflow did not break a guard that reads the workflow. Both passes of the instrumented run reported Failed: 0 with build.yml modified, so the measurement was taken on a green suite and is not a reading off a failing one. CrossAppGuardCiGateTests does read build.yml — in EveryCrossAppSourceRead_IsReachableByTheFilterThatGatesItsSuite, which stayed green throughout — and the arm that failed later reads project files, not workflows.

One self-inflicted red, isolated

The first push of this fix failed CrossAppGuardCiGateTests.TheEvaluatedSet_ContainsEverythingAParseOfTheSameXmlFinds with Set: [] (run 34124608714), and it was this change rather than the class's known parallelism flake — which is a file-in-use IOException in a different method. The empty set at line 878 is parsedCross, the CRUDE XML PARSE side, not the evaluated side. ParsedProjectXmlPaths scans with "(?<dq>[^"]*)"|'(?<sq>[^']*)'|>(?<text>[^<>]*)<, so an apostrophe is an attribute delimiter: the five in the new project-file comment left the pairing unbalanced and swallowed every quoted attribute value after it, including Include="..\Darling\Darling.Tests\CSharpSourceWalker.cs". Established by a single-variable revert — Lite.Tests/Lite.Tests.csproj back to dev with everything else unchanged turns the real test green in 0.7 s in a net10.0 host, and restoring the ItemGroup reds it in 0.3 s — not by re-running. The comment now carries no apostrophe and says why, next to the constraint.

That guard under-reads, and its own doc says it cannot. It is written to be crude in the safe direction — "it over-reads — a path inside a comment ... counts — and it under-reads anything needing evaluation" — but an unbalanced apostrophe makes it under-read arbitrary quoted values, which is the unsafe direction, and it reports the result as a moved blind spot rather than as a parse it could not complete. Not touched here: it is a pre-existing defect in a guard with its own issue lineage (#3063, #3074, #3076, #3082), and folding it in would put a change to the cross-app guard inside a write-lock PR. Left as a finding.

Verification

Lite.Tests is net10.0-windows and cannot run on macOS, so Lite.Tests/WriteLockBudgetTests.cs and Lite.Tests/ParitySource.cs were compiled unmodified into a net10.0 xunit.v3 console host, with the budget members sliced byte-for-byte out of Lite/Services/LocalDataService.cs by an extractor that asserts on the real file's text. 31 tests green. Eighteen mutations, each applied and reverted on its own, each verified to have actually landed before the run — one first attempt was a no-op substitution and was redone rather than counted as a pass, and one --no-build run reported 28 green off a stale binary while the build was failing, which is why the runner rebuilds every time:

mutation	red
DefaultWriteLockBudget 5 s to 10 s	TheAppKeepsTheDispatchersFiveSeconds + the 7 default-resolving OnlyAUsableDeclarationDisplacesTheDefault cases
host declaration deleted	OnlyTheTestHostDeclaresABudget, ThisHostResolvedTheBudgetItsProjectFileDeclares
declared 120 changed to 300, host still resolving 120	ThisHostResolvedTheBudgetItsProjectFileDeclares only
the app's csproj also declares a budget	OnlyTheTestHostDeclaresABudget only
call site back to AcquireWriteLock(timeout: TimeSpan.FromSeconds(5))	TheCallSiteTakesTheBudgetRatherThanALiteral only
resolver drops seconds > 0	the "0" and "-30" cases only
resolver parses in CurrentCulture	TheDeclarationIsReadInTheInvariantCulture only
resolver accepts any object via ToString()	ANonStringDeclarationResolvesTheDefault only
host budget 120 s to 20 s	TheBudgetOutlastsEveryDeliberateHoldInThisSuite only
a new test file holding the lock for 200 s	TheBudgetOutlastsEveryDeliberateHoldInThisSuite only
a 200 s holder under Lite.Tests/Helpers/	TheBudgetOutlasts… — and nothing at all with recursion off
a holder spelling its wait .Wait(200000)	TheBudgetOutlasts… — scored zero under the old scan
a holder whose duration is an opaque identifier	EveryDeliberateHoldInThisSuiteIsReadable, naming the site
raw text instead of StripCommentsAndStrings	EveryDeliberateHoldInThisSuiteIsReadable on a string.Join
upper bound dropped (the review finding)	TheResolverIsTotal..., TheCeilingIsAccepted...
round-to-zero check dropped	TheResolverIsTotal... only
ceiling set to TimeSpan.MaxValue	TheCeilingIsAccepted... only — totality stays green
positivity guard dropped	TheResolverIsTotal... only

The RuntimeHostConfigurationOption to AppContext.GetData path was verified separately: MSBuild writes it into configProperties as a JSON number and the host hands it back as System.String, which is why the resolver parses rather than casts, and an absent key returns null.

The real CrossAppGuardCiGateTests was compiled into the same host for the apostrophe diagnosis above; it and WriteLockBudgetTests are 30 green together on the merged tree. The parse and overflow behaviour above was measured in the same way rather than reasoned about.

The sibling flake is a different mechanism, and this does not fix it

CrossAppGuardCiGateTests.TheEvaluatedRead_FailsLoudlyRatherThanReturningNothing fails with a file-in-use IOException at CrossAppGuardCiGateTests.cs:811 — the finally's Directory.Delete(scratch, recursive: true). Its last arm deliberately runs dotnet msbuild with a 1 ms deadline to prove a hung child fails rather than hangs; RunDotnet then does a best-effort process.Kill(entireProcessTree: true), and the delete races the OS releasing the killed tree's handles on that scratch directory. Same family in the sense that a test manufactures an adverse timing condition and then loses to it under runner load, but the contended resource is a filesystem handle rather than s_dbLock, so nothing here reaches it — and neither would either of the issue's other two options. It fired twice in the 194 runs (34004387846, 34048086326), so it is the more frequent of the two.

Left out

• The [BUG] Lite's database-state deviation read performs four writes under a READ lock, starving writers and racing archival #2208 best-effort skip arm is still uncovered. DatabaseStateWriteLockTests records that it needs the timeout injected; a host-wide property is the wrong seam for it, because forcing a short budget to observe the skip would put every parallel neighbour on that fuse — a smaller copy of this defect. Covering it wants a per-instance override, which is a separate change from this one.
• No pin asserts that a future write-lock caller passes the budget rather than its own literal. TheCallSiteTakesTheBudgetRatherThanALiteral guards the existing call site by text; a second caller with its own TimeSpan.FromSeconds would slip past it.

CHANGELOG entry text

Not applied here — every lane appends to the same [Unreleased] block. Under ### Fixed:

- **A UI-thread timeout was deciding `Lite.Tests` outcomes in a host with no UI thread** ([#3134]) - `LocalDataService.OpenWriteConnectionAsync` is the only one of Lite's twenty write-lock acquisitions that passes a timeout, and its own remarks say why: it is on the path a WPF dispatcher awaits. `DuckDbInitializer.s_dbLock` is process-wide, so in the test host that five seconds could expire inside an unrelated class's deliberate hold - four test methods take the lock exclusively and hold it for up to 30 s while they assert on timing, and three of them are not classes of `SharedDuckDbFixture`, so no collection over the store-touching classes reaches them. **The timeout fired in 1 of 194 `Run Lite tests` executions across 300 `build.yml` runs, on a step at p97.9 of the duration distribution (487 s against a p50 of 262 s).** The budget now resolves from a `RuntimeHostConfigurationOption` the host declares: the shipped app declares none and keeps `DefaultWriteLockBudget`'s five seconds, `Lite.Tests` declares 120 s, and the suite's deliberate holds sum to 47 s - so the acquisition waits them out and still fails loudly on a wedged lock. The resolver is total: absent, unparseable, non-positive, above a one-hour ceiling, or small enough to round to `TimeSpan.Zero` all resolve the default, because `NumberStyles.Float` admits exponents and the invariant `Infinity` symbol, and `TimeSpan.FromSeconds` throws on both - out of a static initializer that is a `TypeInitializationException` on every store call, not one timeout. **The alternative was measured rather than argued: one runner, one commit, back to back, the suite ran 231.7 s parallel and 449.3 s with `-parallel none`, and the 75 classes that touch the lock carry 417.9 s of that 449.3 s, so a collection over them costs ~+186 s (1.80x) on every run to prevent one failure in 194.**

Needs [#3134]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/3134 in the link block.

[claude]

Reviewed. This PR is scoped to .github/workflows/build.yml only (no T-SQL, no Lite/Darling app code), so the T-SQL style conventions and Lite/Darling parity checks don't apply here — it's pure CI instrumentation, explicitly temporary and to be reverted per the PR description.

Two findings posted inline, both on the new steps:

1. The new upload-artifact step uses if: always(), which breaks a documented invariant in this same file. The concurrency comment at the top of build.yml justifies cancelling superseded PR/push runs by stating every upload-artifact step is gated to release or failure() — true of every existing one (signing steps, darling-pg diagnostics). This new step is the first to upload unconditionally. Since cancel-in-progress is true for pull_request/push, a routine re-push while the job is running cancels the in-flight run before the upload happens — silently losing the exact TRX data this instrumentation exists to collect.
2. Minor: the new serialised-run step's if condition omits || github.event_name == 'release', unlike every sibling step in the job (including the parallel-run step right above it), so a release-triggered run would only capture half of the two datasets the issue needs.

No correctness/security/performance issues beyond the above — the rest (trx flag placement, -parallel none, artifact path glob, actions/upload-artifact@v6 pin) is consistent with existing patterns in the workflow.

[claude]

Reviewed the net diff (5 files: Lite.Tests/Lite.Tests.csproj, Lite.Tests/SharedDuckDbFixture.cs, Lite.Tests/WriteLockBudgetTests.cs, Lite/Database/DuckDbInitializer.cs, Lite/Services/LocalDataService.cs). Confirmed the temporary CI measurement instrumentation mentioned in the description is fully reverted — no workflow files remain in the diff.

Correctness — ResolveWriteLockBudget looks solid:

• Absent/unparseable/non-positive/wrong-type all correctly fall back to DefaultWriteLockBudget (5s), which is the safe failure direction (a host silently losing dispatcher protection would be the dangerous one).
• Invariant-culture parsing with NumberStyles.Float correctly rejects comma-decimal locales (verified de-DE case: "1,5" → default, "1.5" → 1.5s).
• Static-readonly resolution at type load, fixed before Main, matches the doc comment's stated rationale (no settable static that a test could hijack process-wide).
• The TheBudgetOutlastsEveryDeliberateHoldInThisSuite derivation is a nice touch — it recomputes the bound from the suite's own literals rather than hardcoding it, so it can't silently drift stale as new long-held-lock tests are added.

Lite/Darling parity — no gap. DuckDbInitializer.s_dbLock is a Lite-only process-wide ReaderWriterLockSlim around the embedded DuckDB file; Darling's PostgreSQL store has no equivalent construct (confirmed via grep — no AcquireWriteLock/s_dbLock analog anywhere under Darling/). This is correctly scoped as Lite-only and doesn't trigger the "state added to one store" parity rule in CONTRIBUTING.md.

Security — none of the usual concerns apply (no SQL, no network/file/process I/O beyond the existing DuckDB path, no secrets). The AppContext.GetData config key is a compile-time constant, not attacker-influenced input.

Style — comments follow the repo's "explain WHY at length" convention, XML doc comments present, internal visibility used appropriately for cross-file-but-same-app fields.

No findings. This is a well-scoped, well-tested fix for a real flaky-test cause (#2374/#3134's shared process-wide lock across parallel test classes).

[claude]

Reviewed the diff (Lite.Tests/Lite.Tests.csproj, Lite.Tests/SharedDuckDbFixture.cs, Lite.Tests/WriteLockBudgetTests.cs, Lite/Database/DuckDbInitializer.cs, Lite/Services/LocalDataService.cs).

Scope check: this is Lite-only infrastructure — DuckDbInitializer.s_dbLock and the WPF-dispatcher timeout concern have no Darling counterpart (Darling writes PostgreSQL via Npgsql, no process-wide ReaderWriterLockSlim, no RuntimeHostConfigurationOption for this key anywhere else in the tree). No Lite/Darling parity drift from this change.

Correctness: left one inline comment on LocalDataService.ResolveWriteLockBudget — a declared budget of "Infinity" (or any finite value ≥ TimeSpan.MaxValue.TotalSeconds) passes the seconds > 0 check but then throws an unhandled OverflowException out of TimeSpan.FromSeconds, which happens inside a static readonly field initializer and would take down the type (and the host) via TypeInitializationException instead of falling back to the default the way every other unusable shape is documented and tested to do. Currently unreachable (only Lite.Tests.csproj declares the key, at "120"), but it's a gap in both the resolver's stated contract and in OnlyAUsableDeclarationDisplacesTheDefault's theory cases.

Everything else checks out:

• The budget resolution logic (invariant-culture parsing, string-only acceptance since MSBuild hands the value back as a string, non-positive/unparseable/wrong-type all falling back to the 5s default) is correct and matches its test coverage.
• Lite.Tests.csproj's new RuntimeHostConfigurationOption is scoped to the test project only and doesn't leak into the shipped app's runtimeconfig.json via the ProjectReference.
• WriteLockBudgetTests is well-targeted: it pins that only the test host declares the key, that the app keeps the 5s default, that the declared value actually reaches AppContext (catching a "wired but inert" seam), and that the test host's budget outlasts the suite's own worst-case deliberate lock holds (derived from source rather than restated, rounding up).
• The doc-comment updates in DuckDbInitializer.cs and SharedDuckDbFixture.cs accurately reflect the new behavior.
• No SQL, no external input, no secrets/network/process surface touched — nothing security-relevant here beyond the crash-on-bad-config edge case noted above.

[claude]

Reviewed the diff (Lite.Tests/Lite.Tests.csproj, Lite.Tests/SharedDuckDbFixture.cs, Lite.Tests/WriteLockBudgetTests.cs, Lite/Database/DuckDbInitializer.cs, Lite/Services/LocalDataService.cs; the workflow instrumentation was added and reverted within the branch, net no-op against dev).

Parity: no concern. Darling has no DuckDB store and no s_dbLock/AcquireWriteLock equivalent — this is a Lite-only locking mechanism, so there's no Darling counterpart to keep in sync.

Production path: OpenWriteConnectionAsync now waits WriteLockBudget instead of a literal TimeSpan.FromSeconds(5). ResolveWriteLockBudget is total (never throws, always returns a value in (TimeSpan.Zero, MaxWriteLockBudget]) and the shipped app declares no RuntimeHostConfigurationOption, so it resolves the unchanged 5-second default — verified no project file besides Lite.Tests/Lite.Tests.csproj declares the key. Edge cases (NaN, Infinity, 1e300, culture-mismatched decimal separators, non-string inputs, double.Epsilon rounding to TimeSpan.Zero) are all handled correctly and covered by TheResolverIsTotalAndAlwaysReturnsAUsableBudget / TheCeilingIsAcceptedAndAnythingPastItResolvesTheDefault. No security or SQL-surface changes.

One finding posted inline on WriteLockBudgetTests.cs: the DeliberateHolds()/Waits() heuristic that TheBudgetOutlastsEveryDeliberateHoldInThisSuite relies on can silently under-count a future long-held write lock (it only scans Lite.Tests' top-level directory — missing the existing Helpers/ subfolder — and its regex set misses bare-millisecond wait idioms like ManualResetEventSlim.Wait(30000)). Worth tightening since the test's whole premise is that it "fails toward too small, never toward fine."

Everything else — the apostrophe fix to keep CrossAppGuardCiGateTests' XML scan from swallowing the linked Compile item, the MaxWriteLockBudget ceiling, and the derived (not restated) 47s/120s margin — checks out against the current source.

[claude]

Reviewed the write-lock-budget change (Lite.Tests.csproj, SharedDuckDbFixture.cs, WriteLockBudgetTests.cs, DuckDbInitializer.cs, LocalDataService.cs).

Correctness — ResolveWriteLockBudget is careful about the failure modes that matter:

• The ceiling check (seconds <= MaxWriteLockBudget.TotalSeconds) happens before TimeSpan.FromSeconds(seconds), so hostile inputs like "1e300" or "Infinity" (both of which double.TryParse with NumberStyles.Float + InvariantCulture happily parses as finite/valid doubles) are rejected before they can overflow TimeSpan.FromSeconds and blow up the static initializer.
• double.Epsilon-style values that parse to a positive double but round to TimeSpan.Zero are caught by the post-conversion budget > TimeSpan.Zero check, so a "budget in name only" can't slip through.
• Culture handling is correct — parsing is pinned to InvariantCulture explicitly, verified against a de-DE thread culture in TheDeclarationIsReadInTheInvariantCulture.
• Static field initialization order (DefaultWriteLockBudget → MaxWriteLockBudget → WriteLockBudget) is safe since all three are declared in that order within the same file/partial-class part.

Lite/Darling parity — no drift here. DuckDbInitializer's process-wide ReaderWriterLockSlim is Lite-only infrastructure (Darling stores to PostgreSQL/TimescaleDB, not DuckDB), so there's no Darling counterpart that needs the same fix.

Security — no user-facing input; the parsed value only ever comes from the host's own compiled-in runtimeconfig.json, and the resolver is fully guarded (never throws, always returns a bounded positive TimeSpan).

Performance — this only widens the timeout for the Lite.Tests host; the shipped app's default (5s) is unchanged and pinned by TheAppKeepsTheDispatchersFiveSeconds.

Left one inline nit on a minor test-robustness gap (DeliberateHolds() not recursing into Lite.Tests subdirectories). Otherwise this is a well-scoped, heavily self-verified fix — no blocking issues found.

[claude]

Reviewed the diff (Lite.Tests + Lite/Database/DuckDbInitializer.cs + Lite/Services/LocalDataService.cs). This is Lite-only test infrastructure plus one config-driven timeout — no Darling-side counterpart is needed since the dispatcher/process-wide-lock scenario this addresses is WPF/DuckDB-specific and Darling has no equivalent.

Correctness

Traced ResolveWriteLockBudget through its edge cases by hand against the TheResolverIsTotalAndAlwaysReturnsAUsableBudget / TheCeilingIsAcceptedAndAnythingPastItResolvesTheDefault pins:

• NaN fails seconds > 0 (NaN comparisons are always false) → falls to default. Correct.
• Infinity / 1e300 pass seconds > 0 but fail seconds <= MaxWriteLockBudget.TotalSeconds → filtered before TimeSpan.FromSeconds is called, so the documented overflow (TypeInitializationException from a static initializer) can't actually happen. Good — the ceiling check order is what makes the resolver total.
• double.Epsilon passes both range checks but rounds to TimeSpan.Zero at tick granularity, caught by the explicit budget > TimeSpan.Zero check. This is the one case that would be easy to get wrong (checking the input double's sign instead of the resulting TimeSpan), and it's handled correctly.
• The ceiling (3600s) round-trips exactly since 3600s converts to an integral tick count.

Math check on the budget-vs-holds bound

Verified the "47s sum, 120s budget" claim in the PR description by grepping actual hold durations: AnalysisPassTokenThreadingTests maxes at 30s, StatusBarSizeReadLockTests at 15s (via the WaitAsync(TimeSpan.FromSeconds(15)) call, not the 10s WriteLockHold constant — the scan correctly takes the file-wide max, not just the named hold), DismissReliabilityTests maxes at 2s (thread.Join(2000), picked up via the bare-milliseconds arm of WaitSitePattern). Sum = 47s, 120s budget = ~2.5x margin. Checks out.

One file the PR description doesn't mention — DatabaseStateWriteLockTests.cs — also has bare AcquireWriteLock() calls and gets picked up by ScanDeliberateHolds, but contributes 0s since it only holds the lock for a synchronous recursion check with no Wait/Sleep/Delay. Doesn't affect the derived bound (the test computes the sum from source, it isn't hand-maintained), just a minor omission in the PR body's "three classes" framing — not a code issue.

AppContext round-trip risk

The one part I couldn't verify statically: the design relies on RuntimeHostConfigurationOption Value=\"120\" reaching AppContext.GetData as the string \"120\" rather than a boxed number, since MSBuild writes numeric-looking RuntimeHostConfigurationOption values as JSON numbers in runtimeconfig.json. If the host ever handed back a non-string here, ResolveWriteLockBudget would silently resolve to the 5s default — exactly the "dead seam" failure mode ThisHostResolvedTheBudgetItsProjectFileDeclares is designed to catch. That test is an end-to-end pin (reads the declared value out of the project file and asserts the running host actually resolved it), so it should fail loudly in CI if this assumption is wrong rather than silently passing — worth confirming the build job (in progress as of this review) is green before merging.

Style / parity

No T-SQL in this diff. C# style (naming, XML doc comments, partial class organization) is consistent with the surrounding files. The apostrophe-avoidance in the new Lite.Tests.csproj comment (to not trip CrossAppGuardCiGateTests's crude text-scan regex, which treats an unbalanced ' as an attribute delimiter) checks out — I confirmed that regex exists and behaves as described, and the new comment block is in fact apostrophe-free.

Nothing blocking found.