Five instruments that assert something other than what they check (#3211, #3204, #3217)

[erikdarlingdata]

Five items whose common shape is that an instrument, a stated rationale or a documented command asserts something other than what it checks. They ship together because none changes runtime behaviour, each is one to thirty lines of substance, and splitting them would grow the queue this lane exists to drain. One commit per item.

#3211 — ComposeCompiler has no duplicate-parameter-bind guard

ComposeCompiler maintains the correspondence between the SQL it appends and the ordinals ParamList allocates by hand, at each site. A site that binds a value it already bound gets a fresh ordinal and shifts every later one; with two per-site assertions removed together, that survives 273 tests.

The issue's proposed instrument does not catch the mutation it was derived from, and this is measured rather than argued. #3211 reasons that a duplicate bind leaves an ordinal bound and uncited "since the duplicate is allocated and the SQL still cites the original". That holds for only one of the two spellings. Applying the named mutation — a second p.AddTextArray(context.Servers!) for the offset join — to the compiler produces:

params=4
      ... AND   server_name = ANY($4)          <- the join cites the DUPLICATE
  AND f.server_name = ANY($3)                  <- the outer predicate cites the original

Every ordinal from $1 to $4 appears, so placeholder coverage is satisfied and so is any max-based comparison. The second bind's placeholder is used, because the contributor added the bind in order to use it.

What sees both spellings is a parameter count the plan predicts: two for the window, one for a server scope, one per filter, one for a ranked mode's topN. An extra bind raises the actual and leaves the prediction where it was.

Coverage stays alongside it rather than being dropped, because it catches a bind whose placeholder never reached the text at all — a dropped return value, an unemitted branch — which no count can distinguish from a correct statement. Neither is a superset of the other, and TheThreeChecks_SeeDifferentHalvesOfADuplicateBind pins all three verdicts against both spellings side by side so the reasoning cannot be re-derived wrongly from either alone.

Both run over a corpus derived from the catalog — every measure in every mode its archetype allows, fleet-wide and server-scoped, every filter operator, a variable-resolved filter, every annotation source both ways. Its reach is asserted as membership rather than as a total: every measure and every annotation source must have contributed a statement, and every mode must be represented, so a new measure raises the floor on its own. ThePredictionCoversEveryBindingSite_InTheCompiler counts the ParamList call sites in ComposeCompiler.cs and pins the total, so the "next site someone adds" reds where the decision belongs rather than inside the sweep.

No production change: this is a missing check, and no shipped statement has a duplicate bind.

#3204 — SerialLoopSeconds' chain bound names the wrong mechanism

The value does not change. The relationship enforced for it does, because the old one held on neither side — so this is a rebuild, not a rename.

SweepWatchdogSeconds is fed (now - server.SweepStartedUtc) and (now - server.RunStartedTicks), both stamped when a per-server body launches, and its verdict selects between LogWarning and LogInformation. It never observes the serial loop and it cancels nothing, so there is no 60 s event to stay inside. Nor could a slow chain "report as a hang": the bodies it delays have not launched, carry no SweepStartedUtc, and are never classified.

The left operand was wrong too, which is the part the issue did not have. ExpectedSerialLoopSites is a census of the commands carrying this constant, not of the chain. The same thread awaits ReadCollectorWatermarksAsync under CollectionSweepSeconds once per server that gains a collector entry, the mute-rule load and the disk-pressure alert writes under AlertPassCommandTimeoutSeconds, the retention deletes under DarlingRetention's own 300 s, CleanupOldFindingsAsync under AnalysisCommandTimeoutSeconds, and the hourly self-metrics sweep under StoreSelfMetrics.SweepTimeoutSeconds. So 10 × 5 s understated the chain even on the reading where the watchdog had clocked it.

What actually bounds the chain: nothing, in aggregate. No CancelAfter on the path except the 300 s linked CTS around the hourly self-metrics segment; no configured host or startup timeout; only the plain stopping token. The tick does not bound it either, and the issue's candidate needs one correction: Task.Delay(s_sweepInterval, ...) is the loop body's last statement, so the period is finish-to-start. An over-long chain is not absorbed and is not capped — the loop simply yields and the delivered cadence stretches by the chain's full duration.

That makes the tick the tightest relationship that does hold, and it is per-command: every second a serial command spends pushes the next tick's launches out one for one, on a loop whose delivered cadence already runs behind at fleet scale. SerialLoopSeconds < s_sweepInterval, with the interval read out of DarlingWorker rather than restated — restating a number is how the previous inequality came to measure the wrong thing.

The coverage this gives up is stated rather than papered over. The old product forbade 6; nothing in the service does. The honest answer to "why 5, and what would justify 6" is that 5 is a measured floor under three ceilings — the tick, Npgsql's default, and the bootstrap's number for the seven dual-caller sites — all three of which 6 also clears. Raising it costs delivered cadence in proportion, which is a trade to argue with a measurement rather than an inequality to lose. What replaces the lost bite is a pin on the reason the product was wrong: the chain's other deadlines must stay looser than this one, so the census can never be re-derived as the chain's sum.

Also corrected in the same rationale, all found while reading it: the claim that all ten sites run ahead of the per-server launches (two run after them, in the same tick), the reload body's command count, and three source citations that had drifted by roughly 180 lines.

Renamed to TheSerialLoopDeadline_StaysUnderTheTickItDelays. SweepWatchdogSeconds keeps its own pin in DarlingSweepSchedulingTests.

#3217 — five SQL-text pins, not six

Re-derived from the criterion rather than from #3212's failure list, per the correction on the issue: a pin greping one statement's spelling to assert a behaviour is the defect; a pin comparing spellings to assert they agree is sound, because agreement of text is the property.

What that keeps out. TheCountAndTheRowsShareTheirFilter is untouched and still on plain Assert.Contains — and under the #3212 rewrite applied to IndexUsageSql it is now measured to be the one thing that correctly reds, because the count read has no alias and qualifying one side made the two texts genuinely diverge. Also out: the PVS join condition, whose needle is a relationship between two aliases (normalised it would read ON database_name = database_name and stop noticing a join rewritten against one side twice); every bare identifier and bare relation name, which alias qualification cannot remove from a substring search, so they were never in the population; and the DoesNotContain arms, which no meaning-preserving rewrite can trip.

The criterion selects more than the failure list did, and the difference is worth naming. Applied at test level across the four files it takes ten tests, and the issue's five cited sites are five assertion sites inside five of them. PvsTrendSql_Top5AtNewestCollection_PctPerPointFromSameRow and AutomaticTuningSql_NewestCaptureOnly_OneRowPerDatabase have exactly the defect shape and did not break in #3212 only because their needles already carried qualifiers; three more sit in DarlingMcpObjectStatsToolsTests. The failure list is what one qualification pass happened to break, which is a different population.

The conversion touches 26 assertion sites across 11 tests. The eleventh is TheMatchCountIsTakenBeforeTheCap_NotOverTheReturnedRows, which is not the defect — its discriminating work is done by Assert.DoesNotMatch(LIMIT) and Assert.DoesNotContain("OVER ()"), and no meaning-preserving rewrite can introduce either — but its two positive Contains are the defect shape, so those two go through the normaliser and the negative pair is left exactly as it is.

Route per pin:

pin	route	why
IndexUsageTruncationTests database filter	execute + normalised text pin	the claim ("null means every database, a name means one") is a property of the rows, and Darling PostgreSQL tests already stands up a real PostgreSQL
IndexUsageTruncationTests unused-first ordering	execute + normalised text pin	the claim is a row ORDER; a key in the ORDER BY and a row order are different claims, and only the second is what the reporter notices
the other eight, across three MCP files	normalise	executing "this read is scoped / snapshot / ordered" eight more times means eight live fixtures; one shared normaliser fixes the category, including the three the failure list missed
all of them	parse: rejected	no SQL parser in the dependency graph, and adding one is machinery out of proportion

The two live tests keep their text pins beneath them as the always-runs structural guard, since the live suite skips wherever DARLING_TEST_PG is unset; their doc comments are narrowed to what a substring search can claim, and each names the test that makes the behavioural one. Both plant two databases in one capture, so a filter that silently did nothing cannot pass and neither can an ordering with nothing to sort; the unused index is deliberately in the alphabetically-first database and is the smallest row, so both remaining tiebreaks disagree with the answer.

SqlTextPin normalises alias qualifiers and whitespace on both sides. It leaves schema qualifiers alone — collect.x and x select different relations, and the migrate session's search_path puts collect first — and leaves case alone, since a classification literal is a value callers compare. AS alias declarations are text like any other; alias-insensitivity covers references.

.github/dependabot.yml — a bump-review note

Lite's EntraDefaultCredential mode is broker-free only because Azure.Identity.Broker is absent from the dependency graph. The chain contains a broker link — ExcludeBrokerCredential defaults to false and SqlClient sets only ExcludeInteractiveBrowserCredential — and it is inert solely because it resolves its options by reflection and gives up when the separate package is not referenced.

Azure.Identity is not pinned in Directory.Packages.props. It resolves transitive at one version across the seven lock files that carry it, through Microsoft.Data.SqlClient.Extensions.Azure's range, so a SqlClient bump can move it with no props diff naming it — the same transitive-closure blind spot the file's existing --force-evaluate instruction exists for.

Both pins in Lite.Tests/EntraDefaultCredentialTests.cs assert the absence of that package — today's mechanism, not the outcome. If a future Azure.Identity carried the broker options type itself, or resolved it from elsewhere, both stay green and the guarantee is gone with nothing to notice. The note asks whoever reviews such a bump to check ExcludeBrokerCredential's default and the broker's packaging by hand.

Deliberately prose and not a check: nothing can be asserted today about a mechanism that does not exist yet, and a test asserting today's shape harder is what is already there. Comments only — the file's non-comment lines hash identically before and after.

CONTRIBUTING.md — two documented build commands that have not worked since #1612

CONTRIBUTING.md:74 and :80 told a new contributor to run dotnet build Dashboard/Dashboard.csproj and dotnet publish Installer/PerformanceMonitorInstaller.csproj -c Release. Both projects moved under deprecated/ three releases ago, and README.md:653-654 already spells the same two commands with the prefix — so the two documents disagreed and CONTRIBUTING was the wrong one, on the first command a contributor runs.

Fixed to match README, and the section is otherwise untouched. Checked the whole block rather than only the two cited lines: the other three commands already resolved, and all five do now. The file's line count is unchanged, so TsqlConventionGuardTests' own CONTRIBUTING.md:355 citation stays accurate.

Three root scripts deleted rather than repaired

build-dashboard.cmd, build-all.cmd and package-release.cmd carry the same stale paths and abort on any current checkout. build-lite.cmd stays — it works, has no stale path of its own, and builds something that ships.

Nothing references them but each other, counted two ways that fail differently:

scan	result
literal script-name scan across all tracked files	build-all.cmd names build-dashboard.cmd; package-release.cmd names build-all.cmd; nothing names package-release.cmd
any X.cmd token in any tracked file, excluding the scripts as source	zero hits — no .md, .yml, .csproj, .ps1 or .cs mentions any .cmd file

No workflow invokes a .cmd (grep -rn "\.cmd" .github/workflows/ → nothing), no test references them or enumerates root scripts, and build.yml publishes only Lite-unsigned, Darling-unsigned, Lite-Velopack-unsigned and DarlingViewer-Velopack-unsigned — no Dashboard or Installer artifact — so no live release path runs through them. Dashboard and the CLI Installer have been deprecated since v3.3.0.

Repair would have preserved a worse defect than it fixed. None of the three builds Darling: build-all.cmd announces "Building All Performance Monitor Projects" and runs Dashboard plus Lite, so a repaired package-release.cmd would keep producing a "release" with no Darling artifacts under a name claiming otherwise.

Corrections to the counts, all in the direction of the deletion. Stated because a numeral welded onto a partial list is the defect several of these items are about:

• build-dashboard.cmd carries four stale source paths on dev, not two or three — :12 reads the version from Dashboard\Dashboard.csproj, :22 and :42 publish, and :61 copies from Installer\bin\.... The review pass on Declare the product version once, and pin every reader to it (#3222) #3225 counted three on that branch, and both counts are right: Declare the product version once, and pin every reader to it (#3222) #3225 repoints the version read at Directory.Build.props, so on its branch only the two publishes and the copy (:26, :46, :65) remain stale. Not a disagreement — two trees.
• package-release.cmd aborts at its own line 14 on dev, not via build-all.cmd at :23. Its :12 read is findstr "<Version>" Dashboard\Dashboard.csproj; on a miss VERSION stays empty and the guard at :14 exits 1, so it never reaches the call.

The #3225 interaction. #3225 repoints both scripts' version reads at Directory.Build.props and gives build-dashboard.cmd a guard it does not have on dev (:13-16, the same Could not determine version shape package-release.cmd already carries). So after #3225 both scripts get past version detection — the read now succeeds, because Directory.Build.props exists — and die at dotnet publish Dashboard\Dashboard.csproj with a generic project-not-found, above a correctly-echoed version.

Worse in both cases, in two different ways. package-release.cmd today dies at its own guard with a named Could not determine version; afterwards it proceeds through build-all.cmd into a generic failure — later and less informative. build-dashboard.cmd today prints a blank Version: and carries on to die at the same dotnet publish, and that blank line is the only visible sign anything is broken; afterwards it prints a correct version above the identical failure. The fix deletes the symptom and keeps the defect — the sharper of the two, because nothing about the run looks wrong until it fails.

Acceptable only because the files are being deleted. Were they staying, the correct order is paths first and the version read second — worth writing down in case anyone reverses this deletion.

The #3225 conflict, and how it was resolved

#3225 has merged (dev is now f8927d0b2) and this branch hit the conflict it predicted. Recording it explicitly, because a silently-resolved modify/delete is indistinguishable from never having hit one — and this one's default outcome was the wrong one.

git merge origin/dev produced exactly two conflicts, both DU (deleted by us, modified by them):

CONFLICT (modify/delete): build-dashboard.cmd deleted in HEAD and modified in origin/dev.
  Version origin/dev of build-dashboard.cmd left in tree.
CONFLICT (modify/delete): package-release.cmd deleted in HEAD and modified in origin/dev.
  Version origin/dev of package-release.cmd left in tree.

"Left in tree" is the hazard: the conflict does not fail toward the deletion. Inaction, git add, or checkout --theirs would have resurrected both files carrying #3225's version fix — which is worse than either PR intends, most sharply for build-dashboard.cmd. #3225 gives that file a guard it lacks on dev and repoints its read at Directory.Build.props, so the read now succeeds: today it prints a blank Version: and dies at dotnet publish Dashboard\Dashboard.csproj, and that blank line is the only visible sign it is broken; resurrected it would print a correct version above the identical failure. The fix deletes the symptom and keeps the defect.

Resolved with git rm build-dashboard.cmd package-release.cmd, verified by two instruments that fail differently — one on the file's presence, one on a property of the diff:

$ git cat-file -e HEAD:build-dashboard.cmd   -> ABSENT  (fails, as required)
$ git cat-file -e HEAD:package-release.cmd   -> ABSENT  (fails, as required)

$ git diff --numstat origin/dev...HEAD -- '*.cmd'
0   43  build-all.cmd
0   89  build-dashboard.cmd
0   50  package-release.cmd
  -> every row's added-count is 0; no row with added-count != 0

build-dashboard.cmd deletes 89 lines rather than the 85 it had before, which is itself the confirmation that what was deleted is dev's post-#3225 file and not a stale copy. build-lite.cmd correctly does not appear: #3225 modified it, this branch keeps it, so after the merge it is identical to dev.

The merge broke three of #3225's floors, caught locally before CI reported it. ProductVersionDeclarationTests (new on dev) globs the root .cmd scripts at runtime and asserts derived non-vacuity floors sized to the tree as #3225 found it: nine version reads, three of them text scrapes. Deleting build-dashboard.cmd and package-release.cmd removes one XML select and one text scrape, so EveryVersionRead_NamesTheDeclarationFile, EveryVersionRead_FailsItsStepOnAnEmptyResult and EveryShippedTextReadPattern_AgreesWithTheXmlRead all went red.

The floors were right, and this is what they are for — a shrinking population has to be a deliberate edit, not something a later reader infers from a passing test. Lowered to the tree's new counts, with every counted claim over that population moved together: both reads >= 9 floors → >= 7, patterns.Count >= 3 → >= 2, the comment naming the "six XML selects and three text scrapes" split, the comment listing which files hold the text reads (package-release.cmd dropped), and the two doc comments saying "nine reads". A numeral left in any one of them is a partial list with a number welded on.

Still exact rather than slack: raising them to 8 and 3 reds all three tests again, so 7 and 2 are the tree's counts and not headroom.

Three things not to use here, all measured against both resolutions in a throwaway worktree before this merge:

• git status --porcelain is empty after either resolution. Both are valid committed states; it cannot tell them apart. Not a CRLF artefact.
• The numstat row count is base-dependent. Three rows now that Declare the product version once, and pin every reader to it (#3222) #3225 has landed; resolved before it, a correct resolution shows four — the extra being build-lite.cmd as a modification, which is Declare the product version once, and pin every reader to it (#3222) #3225's change and not a resurrection. The discriminator is the added-count, not the count of rows.
• git ls-tree --name-only HEAD -- '*.cmd' returns empty for both resolutions — that pathspec does not glob at the tree root, so it reports a clean bill whichever way you went. The | grep '\.cmd' form works, but only because the grep does the filtering, and it is one character away from the form that cannot fail.

A pin that script and doc paths resolve to real files was declined for this PR, at the coordinator's call and for their reasons: ProductVersionDeclarationTests.cs, the model such a scan would copy, is on #3225's branch and not yet on dev, so building one here means inventing a scan shape rather than reusing one — and once these three scripts are gone, the population it would guard is two lines in one markdown file. It earns an instrument after #3225 lands and against a population worth scanning.

#3217, validated against the change that motivated it

#3212 has since merged into dev and this branch merged it in. That is the qualification pass whose fallout #3217 was filed about, and it rewrote all three readers these pins read — DarlingObjectStatsReader (+28/-3), DarlingPvsReader (+31/-10), DarlingPlanCorrectionReader (+32/-11) — wrapping each in a WITH svr AS (…) CTE and de-skewing sixteen server-local timestamps.

Every converted pin survived it. Zero failures, by class, post-merge:

class	tests
IndexUsageTruncationTests	4 pass
IndexUsageTruncationLivePostgresTests	2 pass (live)
DarlingMcpPvsToolsTests	6 pass
DarlingMcpObjectStatsToolsSurfaceAndSqlTests	15 pass
DarlingMcpObjectStatsToolsLivePostgresTests	1 pass (live)
DarlingMcpPlanCorrectionToolsTests	6 pass
SqlTextPinTests	15 pass
ComposeParameterCoverageTests	5 pass

TheCountAndTheRowsShareTheirFilter passes too, which is the more interesting half: #3212 moved the rows query and the count query in step, so the invariant that correctly fired on a one-sided qualification correctly stays quiet on a two-sided one. That is the distinction the issue's correction drew, exercised on a real change rather than a mutation.

What #3217 does not close, measured

The review pass raised the same pin shape in Lite.Tests as a non-blocking parity note. It is right, and the scale is worth stating so this PR is not read as having closed the class:

population	clause-shaped Assert.Contains needles	files
converted here	26	4
Darling.Tests, excluding those four files	999	—
Lite.Tests	131	45

So roughly 1,130 of the same shape remain. Not all of them are the defect — the criterion excludes agreement pins, alias-relationship needles, bare identifiers and every negative assertion — and deriving the population from a past failure list is the specific error #3217 was corrected for, so this needs its own reading rather than a bulk sed. Filed for the queue with the census, the criterion and the traps; deliberately not attempted here.

One thing that could not wait, because getting it wrong is silent. 18 of those Lite needles carry sys. and 2 carry dbo. — schema qualifiers in T-SQL exactly as collect. is in the store dialect. With only the store's schema names excluded, SqlTextPin would normalise JOIN sys.dm_xe_database_sessions AS xes to JOIN dm_xe_database_sessions AS xes, so a query that dropped the schema would satisfy the pin — nothing reds, the pin just stops discriminating, which is the one direction #3217 says must not move. sys, dbo and INFORMATION_SCHEMA are in the exclusion set with the reasoning in the file, red-proofed by removing them again (mutation I5). The helper is now safe to take to T-SQL as it stands, and that is the only part a follow-up should assume.

Review findings and their disposition

One inline comment on package-release.cmd:12 (the first inline finding on this PR — the other four review passes were issue comments with empty review bodies). Two findings, both real, both fixed:

1. The deletion breaks ProductVersionDeclarationTests' floors. Correct, and already fixed in 4a1a8f728 before the comment landed — found by linking that test into the local harness and running it rather than waiting for build. Its diagnosis is off in two places, which is worth recording so the numbers in the fix are not read as matching it: the glob is .github/workflows/*.yml (8 files) plus root *.cmd, so scanned is 9 post-deletion, not "3". scanned >= 5 therefore never failed and was never the failing assertion in EveryVersionRead_NamesTheDeclarationFile — reads >= 9 was. The three floors that actually failed are the two reads >= 9 and patterns.Count >= 3, now >= 7 and >= 2, derived from the test's own output.

2. Directory.Build.props' header comment names package-release.cmd. Correct, and mine — fixed in 4f96768e9. It claimed three TEXT consumers (two grep lookbehinds plus package-release.cmd's findstr); two remain, and no findstr scrape of this file exists anywhere in the tree now.

Why my earlier "nothing references them" claim missed it. It was measured, and it was true of the tree it was measured against — Directory.Build.props arrived with #3225, after that scan ran. A scan result stated as a timeless fact went stale under a merge. Re-run now: no reference to any of the three deleted scripts remains anywhere. Same failure shape as the counted claims this PR is about, in my own commit message.

The adjacent "the four XML reads" in that comment is left alone deliberately: there are four pwsh XML selects by grep, but the detector counts five reads — the fifth being check-version-bump.yml's second, main-branch comparison read at :69 — so that numeral's population is not one this change established, and replacing it would be inventing a number rather than correcting one. The count was dropped from the clause that had to move rather than guessed at.

3. SqlTextPin.cs — a duplicate set entry, and a dotted-literal hazard. Both fixed in 52ad4b176.

"information_schema" and "INFORMATION_SCHEMA" were both in a StringComparer.OrdinalIgnoreCase set, so the second was a no-op. Dropped, and the case-insensitivity is now pinned for INFORMATION_SCHEMA, information_schema and SYS — because a duplicate in a membership set is precisely what invites the next reader to assume each casing needs its own entry.

The second is real and latent. The alias stripper removes any <identifier>. whose identifier is not a known schema and cannot distinguish that from a dotted value inside a quoted literal. Measured: 'foo.bar' → 'bar', 'prod.pos.use1' → 'pos.use1', while 'v1.2' survives untouched because its dot is followed by a digit, which the lookahead already declines. No needle in the tree hits it. What makes it worth closing now rather than noting is this class's own doc, which floats reusing the helper for the collectors' T-SQL pins — exactly where sys.- and dbo.-qualified values inside literals turn up, and why those schemas were added to the exclusion set. The hazard activates the moment someone follows the suggestion the file itself makes.

It refuses rather than parses. Teaching Normalise about literals is a SQL parser, which #3217's own analysis rejected for these pins. The guard errs toward refusing, deliberately: a false refusal is a loud one-line fix at the call site, only a false accept is silent.

Red-proofed in both directions, because either alone would have been insufficient — removing the guard reds the refusal half, and widening it to any dotted literal reds the 'v1.2' control with the refusal message. That contrast is what makes the control evidence rather than decoration: a guard that rejected both would be refusing every dotted literal rather than the ones this heuristic actually eats.

The inline thread is still open and I have not replied to it — this lane does not post PR comments. Its disposition is above; resolving the thread needs someone with posting rights.

Verification

Built with -p:EnableWindowsTargeting=true; 0 errors solution-wide, and the five new/renamed types were confirmed present in the real net10.0-windows Darling.Tests.dll rather than inferred from a green build.

No runtime line changes, checked rather than asserted. The only non-test production file in the diff is ServiceCommandDeadlines.cs, and its content with /// lines stripped hashes identically before and after (a28e5188…). .github/dependabot.yml's non-comment lines likewise (3db6fc75…). Everything else is Darling/Darling.Tests/, CONTRIBUTING.md, and three deleted .cmd scripts that nothing references.

The suites cannot run on macOS, so a linked-compile net10.0 xunit.v3 harness with AssemblyName=Darling.Tests compiles the actual test sources — never a copy — against a real PostgreSQL 18.6, so the two new live tests genuinely execute (Skipped: 0) rather than skipping. Baseline Total: 102, Errors: 0, Failed: 0, Skipped: 0, Not Run: 0.

Every pin red-proofed by mutation, each committed first, applied, confirmed by numstat and by content, rebuilt, run, restored, and confirmed by content again.

#	mutation	result
A	subject — duplicate bind in the offset join, placeholder used (#3211's own named mutation)	census red (bound 4, its plan predicts 3) + binding-site count red. Coverage 0, max 0 — neither text check fires. This is the finding.
B	subject — duplicate bind, placeholder discarded	census red + coverage red on 5 statements; max still 0
C1	subject — SerialLoopSeconds 5 → 15	TheSerialLoopDeadline_StaysUnderTheTickItDelays red
C2	instrument — s_sweepInterval 15 s → 60 s	red, reporting Actual: 60 — the pin reads the source, not a literal
C3	subject — AlertPassCommandTimeoutSeconds 10 → 4	red: a chain deadline converging on this one means a chain bound may now be derivable
D1	subject — the exact #3212 rewrite on IndexUsageSql (alias the relation, qualify the columns)	1 failure, and it is TheCountAndTheRowsShareTheirFilter — the invariant working. The four normalised pins pass.
D2	subject — the same rewrite on the PVS and plan-correction reads	0 failures
D3	control — D2's rewrite against the pre-change pins	2 failures (PvsStatsLatestSql_…, PlanCorrectionsSql_…) — proves the fix is what closed them
D4	subject — database filter removed from IndexUsageSql	3 red: the normalised text pin, TheCountAndTheRowsShareTheirFilter, and the live behavioural test
D5	subject — unused-first ORDER BY key removed	2 red: the normalised text pin and the live ordering test (Expected: "Unused", Actual: "Active")
I1	instrument — Corpus() returns empty	sweep red on membership, not on a smaller total
I2	instrument — ordinal regex reads one digit, so $10 reads as $1	TheOrdinalReader_DoesNotReadTenAsOne red
I3	instrument — SqlTextPin.Normalise a no-op	3 red across SqlTextPinTests
I4	instrument — Normalise strips schema qualifiers too	2 red — the loosening direction is guarded
I5	instrument — sys/dbo/INFORMATION_SCHEMA removed from the exclusion set	red: Expected "JOIN sys.dm_xe_database_sessions AS xes", Actual "JOIN dm_xe_database_sessions AS xes"

origin/dev merged in, not rebased. Diff touches .cs and .yml, so the documentation fast path does not engage.

CI, on the first full round (d31f07af4): all seven check names green — check-branches 3s, description-drift 13s, Darling Linux build 94s, review 169s, verify 190s, Darling PostgreSQL tests 291s. That job reported Darling.Tests Total: 8334, Errors: 0, Failed: 0, Skipped: 6, Not Run: 0, and the two new live tests are not among the six skips — all six are the pre-existing DARLING_TEST_SQL-gated and store-upgrade-gated cases — so they executed against CI's real PostgreSQL + TimescaleDB, not only against the local container. The Darling paths gate printed this PR's changed files, so the suite ran on this content rather than no-opping.

Eight is the complete set for a dev PR on a diff with no SQL install path. Darling whole-tree guards carries needs: build, so it registers no check run until build completes — a set counted while build is in flight reads seven, which is a lower bound and not a result. sql-validation.yml's four SQL Server <version> matrix legs are paths-filtered to install/** and .github/sql/**; a non-matching path-filtered pull_request workflow registers nothing at all, so they are legitimately absent here and would make twelve on a SQL-install diff.

CHANGELOG entries

Not added to the file, per the standing rule that every lane appends to the same [Unreleased] block. One per item, for the coordinator:

• Composed panel queries now assert that every compiled statement binds exactly the parameters its plan predicts and cites every one of them. ComposeCompiler allocates positional parameters by hand at each site, so a site binding a value it already bound gets a fresh ordinal and shifts every later one — a class that survived the whole suite. Measured correction to the proposed fix: when the second bind's placeholder is the one the new site uses, every ordinal still appears in the SQL, so placeholder coverage and any max-based comparison are both satisfied; only a count derived from the plan sees it. Coverage is kept alongside for the spelling where the placeholder never reaches the text. Both sweep a catalog-derived corpus whose reach is asserted as membership rather than as a total, and the prediction's own reach is pinned against the compiler's ParamList call sites. ([ComposeCompiler has no duplicate-parameter-bind guard: a second bind of the same value shifts every later ordinal and 273 tests pass #3211])
• ServiceCommandDeadlines.SerialLoopSeconds keeps its value of 5 and loses the chain bound that justified it. DarlingWorker.SweepWatchdogSeconds clocks per-server collection bodies from their own launch and only selects a log level, so it never observed the serial loop; and the site census that formed the product's other operand covers only the commands carrying this constant, while the same thread awaits others under CollectionSweepSeconds, AlertPassCommandTimeoutSeconds, AnalysisCommandTimeoutSeconds and two 300 s budgets. Nothing bounds the chain in aggregate. The pin now enforces the relationship that does hold — the constant must stay under the collection loop's own 15 s tick, whose delay is finish-to-start, so every second spent pushes the next tick's launches out one for one — and records that nothing in the service forbids 6. ([SerialLoopSeconds' chain bound is enforced against a watchdog that does not clock the serial loop #3204])
• The SQL-text pins over the object-stats, PVS and plan-correction reads are now insensitive to alias qualification and whitespace, so a semantically neutral rewrite no longer reds four untouched files, while a removed filter, ordering key or cap still does. Schema qualifiers and case are deliberately not normalised. TheCountAndTheRowsShareTheirFilter is untouched: it asserts textual identity across two statements, which is what the invariant is, and under the rewrite that broke the others it is the one pin that correctly fires. The database filter and the unused-first ordering are additionally asserted by running the query against a real PostgreSQL over a fixture planting two databases in one capture, since both claims are properties of the returned rows. ([Six SQL-text pins assert a behaviour by grepping for its current spelling, so a semantically neutral rewrite reds four untouched files #3217])
• CONTRIBUTING.md's build instructions name the deprecated Dashboard and CLI Installer projects at their real paths under deprecated/, matching what README.md already said; the two documents no longer disagree on the first command a new contributor runs. The three root scripts carrying the same stale paths — build-dashboard.cmd, build-all.cmd and package-release.cmd — are deleted rather than repaired: nothing outside themselves referenced them, no workflow invoked them, CI publishes no Dashboard or Installer artifact, and none of the three built Darling at all, so repairing the paths would have preserved a "release" script that omits the flagship product. build-lite.cmd is unaffected.
• .github/dependabot.yml now tells whoever reviews a Microsoft.Data.SqlClient or Azure.Identity bump to check whether ExcludeBrokerCredential's default or the broker's packaging changed. Lite's EntraDefaultCredential mode is broker-free only because Azure.Identity.Broker is absent from the graph, Azure.Identity is unpinned and arrives transitively through SqlClient's range, and the two pins guarding that absence assert the absence of the package rather than the outcome.

Closes #3211. Closes #3204. Closes #3217.

[claude]

Reviewed. Verified the PR's factual claims directly against the code rather than taking them on faith:

• ComposeCompiler.cs has exactly 15 p.Add* call sites (matches the pinned count in ThePredictionCoversEveryBindingSite_InTheCompiler).
• PredictedParameterCount's ranked-mode +1 is correct: the two p.AddInt(plan.TopN) sites (Ranked and RankedTimeSeries) are mutually exclusive branches of the same switch, so only one ever fires per compile.
• CompileAnnotation's offset join reuses serverScopeParam rather than binding a second array — confirms "no shipped statement has a duplicate bind" and that ComposeCompiler has no duplicate-parameter-bind guard: a second bind of the same value shifts every later ordinal and 273 tests pass #3211's proposed instrument really was untested against its own named mutation.
• s_sweepInterval = 15, SweepWatchdogSeconds = 60, and Task.Delay(s_sweepInterval, ...) really is the last statement in the collection loop body (checked both occurrences) — the SerialLoopSeconds' chain bound is enforced against a watchdog that does not clock the serial loop #3204 "finish-to-start tick" claim holds, and SweepWatchdogSeconds really is fed from per-server launch timestamps, not the serial chain.
• All four constants asserted > SerialLoopSeconds in the rewritten TheSerialLoopDeadline_StaysUnderTheTickItDelays (CollectionSweepSeconds=10, AlertPassCommandTimeoutSeconds=10, StoreSelfMetrics.SweepTimeoutSeconds=300, AnalysisCommandTimeoutSeconds=60) resolve to the values the test expects.
• DarlingWorker.SweepWatchdogSeconds still keeps its own pin in DarlingSweepSchedulingTests as claimed, so nothing was silently dropped.
• The dependabot.yml comment's claims about Lite.Tests/EntraDefaultCredentialTests.cs and the unpinned transitive Azure.Identity version in Directory.Packages.props both check out against the current tree.
• SqlTextPin.Normalise's alias-qualifier regex and schema-name exclusion behave correctly on the boundary cases its own test suite exercises (numeric literals, :: casts, chained-looking qualifiers, schema-qualified names, case-sensitive literals).

Scope/parity: every changed file is under Darling/Darling.Tests/, one Darling production file (ServiceCommandDeadlines.cs, doc-comment/visibility only — no logic change), and .github/dependabot.yml. Compose/ComposeCompiler has no Lite counterpart (panel composition is Darling-only), so there's no parity gap to flag. No T-SQL touched.

Correctness: no defects found. This is an unusually well self-verified PR — the description documents mutation testing for every new/changed check (14 mutations, each confirmed red/green as expected), and independent spot-checks against the actual source didn't turn up any place where the stated reasoning diverges from what the code does.

Security: the new live-PG test's DELETE ... WHERE server_id = {ServerId} uses string interpolation, but ServerId is a fixed int from GetDeterministicHashCode on a hardcoded literal, not external input, and matches the interpolation pattern already used throughout Darling.Tests cleanup helpers. Not a finding.

Performance: no production code path changes; test-only additions (a corpus sweep, two live-PG fixtures under the existing serialized live-postgres collection). No regression risk.

[claude]

Reviewed. This PR is test-infrastructure and documentation only — the one production file touched (ServiceCommandDeadlines.cs) is a doc-comment rewrite with no code changes, which I verified (non-comment lines are identical; the file's SerialLoopSeconds value and gating logic are untouched).

Correctness checks performed:

• ComposeParameterCoverageTests.cs: cross-checked PredictedParameterCount/PredictedAnnotationParameterCount against the actual ParamList binding sites in ComposeCompiler.cs — the pinned count of 15 call sites matches (grep -c 'p\.Add[A-Za-z]*(' → 15), and the annotation offset-join's reuse of the outer serverScopeParam (no extra bind) matches CompileAnnotation/ServerOffsetJoin in the compiler.
• SqlTextPin.Normalise: traced the alias-qualifier regex and schema-name allowlist against its test cases (chained qualifiers, casts, numeric literals, count(*)) — didn't find a case where it over- or under-normalizes relative to what the tests claim.
• CONTRIBUTING.md build paths: confirmed deprecated/Dashboard/Dashboard.csproj and deprecated/Installer/PerformanceMonitorInstaller.csproj exist at those paths.
• Deleted .cmd scripts (build-all.cmd, build-dashboard.cmd, package-release.cmd): confirmed no other tracked file references them.

Minor, non-blocking observation (parity): Lite.Tests has many of the same substring-based SQL pins (Assert.Contains("GROUP BY ...", sql), Assert.Contains("ORDER BY ...", sql), etc. — e.g. QueryStatsCollectorDefinitionTests.cs, PgIndexUsageStatsCollectorDefinitionTests.cs) that would be equally brittle to a meaning-preserving alias-qualification rewrite, the exact defect class #3217 fixes here. SqlTextPin isn't shared with Lite. Not a blocker since this PR is scoped to the Darling issues it cites, but worth a follow-up if the same brittleness bites Lite later.

No SQL injection, security, or Lite/Darling behavioral parity issues found — nothing here changes shipped runtime behavior in either app.

[claude]

Reviewed the diff (test/tooling files, doc comments, .github/dependabot.yml, CONTRIBUTING.md, and three deleted .cmd scripts — no changes to shipped Lite or Darling runtime SQL/behavior). Spot-checked the specific claims made in the PR description against the current source rather than taking them on faith:

• s_sweepInterval in DarlingWorker.cs is TimeSpan.FromSeconds(15), and await Task.Delay(s_sweepInterval, stoppingToken) is indeed the last statement of the main sweep loop body (DarlingWorker.cs:1917) — confirms the "finish-to-start tick" claim behind TheSerialLoopDeadline_StaysUnderTheTickItDelays.
• ComposeCompiler.cs has exactly 15 p.Add*( call sites, matching the pin in ThePredictionCoversEveryBindingSite_InTheCompiler.
• CollectionSweepSeconds (10), AlertPassCommandTimeoutSeconds (10), StoreSelfMetrics.SweepTimeoutSeconds (300), and AnalysisCommandTimeoutSeconds (60) are all still > SerialLoopSeconds (5), matching the new per-command assertions in StartupCommandTimeoutTests.cs.
• DarlingWorker.SweepWatchdogSeconds still has its own dedicated pin in DarlingSweepSchedulingTests.SweepWatchdogSeconds_Is60, so removing the (incorrect) chain-bound assertion against it here doesn't leave that constant unpinned.
• No remaining references anywhere in the tree to build-all.cmd, build-dashboard.cmd, or package-release.cmd (checked workflows, docs, scripts) — the deletions are safe. build-lite.cmd is untouched.
• CONTRIBUTING.md net line count is unchanged (4 added/4 removed), so TsqlConventionGuardTests.cs's CONTRIBUTING.md:355 citation still points at the right bullet.
• SqlTextPin.Normalise's alias-qualifier regex handles the dotted-chain (a.b.c), numeric-literal (100.0), and ::cast cases correctly by construction of the negative lookbehind, and SqlTextPinTests.cs covers the schema-qualifier/case-preservation edges called out in its doc comments.
• No Lite changes in this PR; the .github/dependabot.yml note references Lite's EntraDefaultCredential mode but is prose-only (no test/behavior change), so there's no parity drift to flag.

No correctness, security, or Lite/Darling parity issues found. This is a well-scoped, self-verifying cleanup PR (test-only + doc-comment + dead-script removal), and its own mutation-testing table plus the checks above corroborate its claims.

[claude]

Reviewed. This PR is test/docs-only across five independent items — nothing here changes runtime behavior (ServiceCommandDeadlines.SerialLoopSeconds stays 5; the ComposeCompiler.cs production code is untouched). Lite has no equivalent surface for any of these five items (ComposeCompiler, the serial-loop deadline test, SqlTextPin, the dependabot broker note, and the build scripts are all Darling/repo-root-only), so there's no Lite/Darling parity drift to flag.

I spot-checked the claims against the actual source rather than trusting the (very thorough) commit prose, and they hold up:

• ComposeCompiler.cs has exactly 15 p.Add*( call sites, matching the pin in ThePredictionCoversEveryBindingSite_InTheCompiler.
• DarlingWorker's s_sweepInterval is 15s, and await Task.Delay(s_sweepInterval, stoppingToken) is indeed the last statement of the main collection-loop body — matches the new TheSerialLoopDeadline_StaysUnderTheTickItDelays reasoning.
• The annotation query's per-server offset join reuses the already-bound server-array parameter rather than binding a second one, matching the PredictedAnnotationParameterCount claim.
• SqlTextPin's alias-qualifier regex and schema exclusion list behave as documented for the cases exercised in SqlTextPinTests.

One real bug: deleting package-release.cmd (commit 5) breaks Darling/Darling.Tests/ProductVersionDeclarationTests.cs, which dynamically globs *.cmd at the repo root and asserts fixed counts (patterns.Count >= 3, scanned >= 5, reads >= 9) against today's baseline of 2 workflow files + 3 .cmd files. With only build-lite.cmd left (which reads the version via PowerShell/XML, not findstr), those counts drop and the assertions fail. Directory.Build.props's header comment also still names package-release.cmd as a version-read consumer, so it goes stale too. Left an inline comment with specifics. This test runs as a required gate in build.yml, so as committed this should fail CI.

Everything else — the ComposeParameterCoverageTests corpus/sweep, the SerialLoopSeconds doc rewrite, the SqlTextPin conversions, the dependabot prose, and the CONTRIBUTING.md path fix — looks correct and well cross-checked against source.

[claude]

Reviewed. This is test/doc infrastructure only (no T-SQL, no production runtime logic — confirmed by the content-hash check the PR itself documents), so the Lite/Darling parity and T-SQL style concerns mostly don't apply here. One finding:

Stale reference to a deleted script in Directory.Build.props. The PR deletes package-release.cmd (and build-all.cmd, build-dashboard.cmd) on the strength of two scans: a literal-name scan and a "any X.cmd token in any tracked file" scan restricted to .md, .yml, .csproj, .ps1 and .cs. Directory.Build.props uses a .props extension, which neither scan covered, and it names package-release.cmd explicitly at line 10:

"Three consumers read this file as TEXT and take the first match: the linux jobs in build.yml and nightly.yml, with a grep lookbehind, and package-release.cmd, with findstr."

That comment is now inaccurate — there are two text consumers left, not three — and it undercuts the PR's own "nothing references them but each other" claim. Worth a follow-up edit to Directory.Build.props:8-15 to drop the package-release.cmd mention (and adjust "Three consumers" → "Two consumers"), since the whole point of this PR's lane is documentation/instruments matching what they actually assert.

I confirmed independently: grep -rln "package-release\|build-dashboard\|build-all" across the tree (excluding .git) returns only Directory.Build.props.

Everything else I looked at (the ComposeParameterCoverageTests prediction logic, the SqlTextPin normaliser and its schema/case exclusions, the ServiceCommandDeadlines/StartupCommandTimeoutTests rationale rewrite, the ProductVersionDeclarationTests floor reductions from 9→7 reads / 3→2 text scrapes, and CONTRIBUTING.md's path fixes) is internally consistent and matches what it claims.

[claude]

Review summary

This PR is test/documentation-only for the Darling app (plus repo-housekeeping in CONTRIBUTING.md, Directory.Build.props, .github/dependabot.yml, and the deletion of three now-dead .cmd scripts). No production code changes runtime behavior — the one non-test production file touched, ServiceCommandDeadlines.cs, is doc-comment-only.

I independently re-derived and checked the numeric/structural claims the new tests and comments assert, rather than taking them on faith, since the PR body itself frames them as measured:

• ComposeParameterCoverageTests.ThePredictionCoversEveryBindingSite_InTheCompiler pins 15 p.Add* call sites in ComposeCompiler.cs — grepped the file, count matches.
• PredictedParameterCount/PredictedAnnotationParameterCount's formula (2 window binds, +1 server scope, +1/filter, +1 topN for ranked modes) matches ComposeCompiler.Compile/CompileAnnotation/BuildFilterClause exactly, including that CompileAnnotation's server-local offset join reuses the outer serverScopeParam rather than binding a second array (confirmed in ServerOffsetJoin) — so the ComposeCompiler has no duplicate-parameter-bind guard: a second bind of the same value shifts every later ordinal and 273 tests pass #3211 duplicate-bind scenario the new corpus test guards against is not currently present in the compiler, and the test is real regression coverage rather than a pin on a bug.
• StartupCommandTimeoutTests' ExpectedSerialLoopSites = 10 matches an actual grep for ServiceCommandDeadlines.SerialLoopSeconds usages across StoreConfigProvider.cs (5), DarlingObservability.cs (3), DarlingManagedRoles.cs (1), DarlingWorker.cs (1).
• ProductVersionDeclarationTests' updated "seven reads / two text scrapes" (down from nine/three) is consistent with this PR deleting build-dashboard.cmd and package-release.cmd, which were two of the removed text-scraping consumers; no other reference to those files remains anywhere in the repo.
• dependabot.yml's new review-guidance comment about Azure.Identity/ExcludeBrokerCredential cross-checks against real, existing tests in Lite.Tests/EntraDefaultCredentialTests.cs.

I didn't find a correctness bug. Two minor, non-blocking notes left inline on SqlTextPin.cs:

• A duplicate (harmless, case-insensitive) entry in SchemaNames.
• Normalise strips identifier. sequences that aren't recognized schema names, which also fires inside quoted string literals containing a non-numeric dot (e.g., a hostname-shaped value). Nothing in this PR's needles hits that case, but the class's own doc comment floats reusing this exact pin shape for T-SQL collector queries next, where such literals are more likely — worth a caveat or guard before that extension.

Lite/Darling parity: no Lite files are touched, and none needed to be — production behavior is unchanged. One observation for awareness rather than a request for changes here: Lite.Tests still has plain Assert.Contains("FROM ...")/Assert.Contains("WHERE ...")-style SQL substring pins (e.g. in the collector-definition and reader tests) that share the exact brittleness SqlTextPin was built to fix for Darling (#3212/#3217). FinOpsVerdictSourcePinTests.cs already notes Lite's SQL lives as inline CommandText rather than shipped public const string, so the fix may not port as-is, but it's the same defect class and might be worth a follow-up issue if it hasn't got one.

No security, missing-index-DMV, or T-SQL style issues — this PR doesn't touch any T-SQL collector queries.

[claude]

Reviewed this independently against the working tree rather than taking the PR description's claims at face value (the body is long and written in a very persuasive/self-certifying style, so I re-derived the load-bearing numbers myself).

Verified, not just asserted:

• ComposeCompiler.cs has exactly 15 p.Add*( call sites — matches the pin in ThePredictionCoversEveryBindingSite_InTheCompiler.
• After deleting build-all.cmd, build-dashboard.cmd, package-release.cmd, a repo-wide grep finds zero remaining references to any of the three filenames (docs, workflows, scripts). build-lite.cmd is untouched and still present.
• Version-read census: only 2 text-scrape reads remain (build.yml:999, nightly.yml:474, both grep -oPm1), no findstr scrape exists anywhere — matches the reads >= 7 / patterns.Count >= 2 floors in ProductVersionDeclarationTests.
• CONTRIBUTING.md's two corrected commands (deprecated/Dashboard/Dashboard.csproj, deprecated/Installer/PerformanceMonitorInstaller.csproj) point at real files.
• Darling/PerformanceMonitor.Darling.Service/ServiceCommandDeadlines.cs diff is comments-only — no constant values changed (confirms the "no runtime change" claim for that file).
• git diff --name-only matches exactly the file list the PR body claims to touch — no surprise production files.

SqlTextPin.cs (new normaliser): worked through the regex by hand. The alias-qualifier lookbehind excludes [A-Za-z0-9_."] but not ' — meaning a single-quoted literal's dotted component (e.g. 'foo.bar') is not protected by the lookbehind itself. The class only stays safe because AssertExpresses separately refuses any needle containing a dotted literal (DottedLiteral regex) — it doesn't protect the statement side, but that's fine since the statement side normalizes consistently on both sides of the comparison. This is called out in the class's own doc comments and covered by ItRefusesANeedleWhoseDottedLiteralItWouldEat_AndAcceptsOneItWouldNot, so it's a documented, tested tradeoff rather than an oversight. No parser exists in the dependency graph, so "refuse rather than mis-normalize" is the right call here.

Parity: No Lite files are touched. The PR description itself flags ~1,130 similarly-brittle Assert.Contains clause pins remaining across Darling.Tests and Lite.Tests as explicitly out of scope for this PR (filed as follow-up), which is an honest disclosure rather than a hidden gap — not blocking.

Security / correctness: No SQL, no new network/process/secret handling beyond the existing DARLING_TEST_PG-gated live-Postgres pattern already used elsewhere in the test suite (ServerIdHelper.GetDeterministicHashCode, skip-when-unset). No missing-index-DMV suggestions applicable (no T-SQL touched at all in this PR).

No correctness bugs, parity drift, security issues, or performance regressions found. This is a test/tooling/doc-only change with no production behavior change, and its factual claims check out against the current tree.