#2190: count <summary> openings per doc run, not closing-tag pairs

[erikdarlingdata]

Fixes #2190.

DocCommentHygieneTests.NoMemberCarriesTwoStackedSummaryBlocks matched </summary> immediately followed by a reopening, so it only ever saw a stacked pair when the FIRST block was closed. The rule now counts <summary> OPENINGS inside each contiguous run of /// lines: a run documents exactly one member, so two openings means two summaries, closed or not, single-line or spread over many.

Watched red, then green

The widened detection was written and run against the UNFIXED tree first. It failed on exactly the two live instances the issue names, with all 8 self-test cases (at that point) passing. Verbatim, against Darling.Tests.dll rebuilt from source:

[xUnit.net 00:00:10.87]     Darling.Tests.DocCommentHygieneTests.NoMemberCarriesTwoStackedSummaryBlocks [FAIL]
  Failed Darling.Tests.DocCommentHygieneTests.NoMemberCarriesTwoStackedSummaryBlocks [140 ms]
  Error Message:
   Stacked <summary> blocks found. Each is a member carrying two summaries; XML docs take the LAST one, so tooling looks correct and only a human reading the file is misled.

DO NOT just delete the first summary - check first whether it belongs to a DIFFERENT member that an insertion pushed it away from. Seven of the eight found in #1745 were displaced doc blocks whose real member had been left undocumented, and deleting them would have lost the documentation rather than deduplicating it.

Where an insertion split a block, also check whether the member it came from has since been re-documented in place. If it has, the stray text is a stranded HEAD rather than the whole block, and moving it back would create the very duplicate this rule forbids - confirm sentence by sentence that nothing is lost, then delete it (#2190).

Darling\PerformanceMonitor.Darling.Service\DarlingManagedPostgres.cs:2368 (<summary> openings at lines 2368, 2369)
Lite\Services\QueryStoreSliceRepairService.cs:576 (<summary> openings at lines 576, 579)
  Stack Trace:
     at Darling.Tests.DocCommentHygieneTests.NoMemberCarriesTwoStackedSummaryBlocks() in C:\GitHub\worktrees\2190-doccomment-guard\Darling\Darling.Tests\DocCommentHygieneTests.cs:line 94

Failed!  - Failed:     1, Passed:     8, Skipped:     0, Total:     9, Duration: 569 ms - Darling.Tests.dll (net10.0)

The pre-#2190 rule passes on that same tree, which is the defect.

The two live instances

1. DarlingManagedPostgres.cs:2368 - a duplicated opening tag on ApplyProcessEnvironment, two openings and one closing. The stray line is deleted, exactly as the issue describes.

2. Lite/Services/QueryStoreSliceRepairService.cs:576 - fixed by deletion, NOT by moving the block. This deviates from the issue and needs your eye.

The issue says not to delete it, on the basis that FlushExternalFileCacheAsync "is left undocumented". It is not. git blame gives the full mechanism:

• 8829e101 wrote FlushExternalFileCacheAsync's original doc block: the opening tag, the sentence "Drops DuckDB's cached view of every external file, by toggling the cache off and back on.", then two <para> blocks carrying the WHY (the instance-scoped path cache; the off/on toggle being the only eviction that works).
• bb1f48c0 ("move the cache eviction inside the swap helper, correct-by-construction") then inserted PromoteRewrittenFileAsync between that block's opening and its closing tag. It moved the WHY paragraphs up onto the new helper, where they live today, and gave FlushExternalFileCacheAsync a NEW summary in place at what is now lines 629-631: "Evicts DuckDB's cached view of every external file, by toggling the cache off and back on. The WHY is on <see cref="PromoteRewrittenFileAsync"/>, which is the only thing that should ever need this."

So what sits at 576-578 is not a whole displaced block, it is the stranded HEAD of one: an opening tag plus a single sentence. That sentence survives on the correct member at line 630, improved (Evicts for Drops) and paired with a pointer to the WHY. Deleting it loses nothing, and moving it back would either stack a second summary on FlushExternalFileCacheAsync (the exact defect this rule forbids) or duplicate a sentence already there.

I checked this sentence by sentence rather than taking the issue's word for it, and the guard's own failure message now carries the same triage step so the next person hitting a split block checks whether the source member was re-documented before moving anything.

False-positive triage

The widened detection ran across the WHOLE tree, which is what the test does by construction (every *.cs under the repo root, bin/obj excluded). It flags exactly the two instances above and nothing else - no legitimate pattern was caught, so no tightening was needed. Escaped mentions in prose (<summary>, which this very test file is full of) are not openings and never matched. There are no delimited /** */ doc comments anywhere in the tree, so the /// run model has no gap.

Self-test

The detector now pins its own behavior, because #2190 was a blind spot in the DETECTOR rather than in anyone's reading of the tree, and a synthetic case is what exposed it. Five shapes it must catch (closed-then-reopened; a duplicated opening tag; an insertion-split block; a single-line summary followed by a multi-line one, which is the mixed form a closing-tag matcher cannot see at all; and a stacked run reaching EOF, which pins the scan's one-past-the-end step) and four it must leave alone (summary plus <param>/<returns>; summary carrying several <para> blocks; two adjacent members with one summary each; escaped mentions in prose).

Gates

• Full Darling.Tests suite, rebuilt DLLs: Failed: 0, Passed: 4234, Skipped: 236, Total: 4470. The skips are the live-Postgres-gated families.
• DocCommentHygieneTests on the fixed tree: Failed: 0, Passed: 10, Total: 10.
• Build warnings: 0 introduced. Darling.Tests and PerformanceMonitorLite both build with 0 errors; the warnings they do emit (CA2022 in NpgsqlRootCertificateValidationTests.cs, CA1068 in PerformanceMonitor.Common/AbandonableStep.cs, CS0628 in Lite/Services/LocalDataService) are all pre-existing on dev in files outside this diff. I left them alone deliberately - CA1068 is a signature reorder on a primitive both apps call, and none of the three sit in a file this PR touches - and filed them as [BUG] Pre-existing analyzer warnings on dev make a zero-warning build gate unreadable (CA1068, CA2022, CS0628) #2193 rather than leaving them untracked.
• CHANGELOG: [Unreleased] / Fixed with a [#2190] link-ref.

One note for whoever edits CHANGELOG.md next: it carries 6 bare CR bytes as line terminators among its 2674 CRLFs. A normal editor pass silently rewrites them and turns a two-line addition into a 5350-line diff. This entry was inserted byte-precisely with the CR count asserted unchanged, so the diff here is the 2 lines it should be.

Not merging, and leaving #2190 open for you to close.

Generated with Claude Code

[claude]

Review: PR #2192

Scope is narrow and self-contained: Darling/Darling.Tests/DocCommentHygieneTests.cs (detector rewrite + self-test), and two pure doc-comment deletions in DarlingManagedPostgres.cs (Darling) and QueryStoreSliceRepairService.cs (Lite). No .sql, no runtime/functional code, no user input or network/process/file surface touched beyond comments — so most of the usual checklist (T-SQL style, injection, secrets) doesn't apply here.

Correctness of the new detector — traced StackedSummaryRuns/SummaryOpening by hand against all 9 [InlineData] theory cases plus the two real fixed instances:

• Run boundaries (a contiguous block of ///-prefixed lines, terminated by any non-/// line, including the EOF-closes-a-run case) are correct.
• Opening-count-per-run correctly catches the three shapes closing-tag matching couldn't: duplicated opening/unclosed first block, an insertion-split block, and single-line-summary-followed-by-multi-line-summary (no </summary> immediately precedes the reopening in that last case).
• Escaped mentions (<summary>) are correctly excluded since the regex needs literal </> chars, not entities.
• Multi-member and legitimate-tag-sequence (<param>/<returns>/<para>) cases correctly produce zero stacked runs.

Only edge case I found and didn't see covered: a self-closing <summary/> wouldn't match <summary\s*> (needs whitespace-then->, and / isn't whitespace), so a stray <summary/> sitting next to a real block would slip through uncounted. Grepped the repo and this style isn't used anywhere (every summary tag is <summary>...</summary>), so it's not a live gap — just flagging in case that convention ever changes. Not blocking.

The two production doc-comment fixes:

• DarlingManagedPostgres.cs — straightforward duplicate /// <summary> opening line removed from ApplyProcessEnvironment; the block closes correctly afterward.
• QueryStoreSliceRepairService.cs — verified the deleted stray head ("Drops DuckDB's cached view of every external file...") is in fact a subset of FlushExternalFileCacheAsync's own current summary ("Evicts DuckDB's cached view of every external file, by toggling the cache off and back on. The WHY is on PromoteRewrittenFileAsync..."), so no documentation is lost by deleting rather than moving it, per the PR's own stated reasoning.

Lite/Darling parity — the two fixes touch one app each (Darling service, Lite service) for unrelated pre-existing bugs; this isn't a shared-feature change, so no parity drift concern. The detector itself lives in Darling.Tests but scans the whole repo tree (including Lite/), so both apps stay covered by one rule — consistent with the "coverage limit" paragraph already in the class doc.

No correctness, security, or performance issues found. Nothing to flag as blocking.

[erikdarlingdata]

CI green, PR clean and not merged.

mergeable=MERGEABLE  mergeStateStatus=CLEAN  state=OPEN

Darling Linux build:       SUCCESS
Darling PostgreSQL tests:  SUCCESS
build:                     SUCCESS
check-branches:            SUCCESS
review:                    SUCCESS

Local gates on the fixed tree, rebuilt DLLs: full Darling.Tests at Failed: 0, Passed: 4234, Skipped: 236, and DocCommentHygieneTests at 10/10. The watched-red run against the unfixed tree is in the PR body verbatim.

Leaving the merge and #2190's closure to you. The one thing worth your eye before merging is the instance 2 call: I deleted the stranded doc head in QueryStoreSliceRepairService instead of moving it, because bb1f48c0 had already re-documented FlushExternalFileCacheAsync in place - the reasoning is under "The two live instances" above.