A pass whose fact families could not be read says so: analyze_server and get_analysis_facts carry collection_caveats on both SKUs, so a blind pass stops reading as an all-clear (partial #3691)

[erikdarlingdata]

What does this PR do?

A pass whose fact families could not be read now says so, on both SKUs. Every family read in the three fact collectors (Lite's DuckDbFactCollector, Darling's SQL Server PgFactCollector, Darling's PostgreSQL-target PgTargetFactCollector) degrades to "no facts" and logs (#2826) — and until now that was where the knowledge stopped. Each ReportCollectionFailure records the failure on AnalysisContext.CollectionFailures beside its log line, each collector stamps the number of families it runs, both analysis services carry the list and two fact counts out of the pass, and analyze_server / get_analysis_facts render a collection_caveats block plus a caveat sentence — only when a family failed. A clean pass's payload is the same object through the same serializer call, so its bytes are what they were.

PARTIAL for #3691. Checklist line completed: Per-family collector failures are logged, not carried on the context. Pre-existing on both engines (exit-check §3 residue); twin edit, Lite first then Darling.

The lie

Every family read ends in catch (Exception ex) when (!IsExpectedAbandon(...)) { ReportCollectionFailure(ex, context); }. That is the right posture — one unavailable table must not cost a server its other thirty facts — and since #2826 the catch writes a WARNING (timeout) / DEBUG (42P01 / 42703) / ERROR (else) line. But the pass received the same empty list a quiet server produces. A pass in which the write family timed out, or every family failed against a half-migrated store, scored nothing from them and both analyze_server tools rendered the empty all-clear: "No significant findings. All metrics are within normal ranges." An operator (or the agent reading for them) trusted a clean card that was blind. get_analysis_facts' zero-facts arm said "No facts collected. The collector may not have run yet" — the same words for "no data" and for "every read failed". And empty could not distinguish "facts were scored and none fired" from "no fact was emitted" without a count it did not carry.

The change

Shared (PerformanceMonitor.Analysis/AnalysisContext.cs). AnalysisContext.CollectionFailures (list of CollectionFailure(Family, Read, Outcome, Message)), CollectionFamilyCount, RecordCollectionFailure(...). CollectionFailureOutcome is the reporters' three arms plus cancelled: timeout (57014 / wrapped TimeoutException, classified structurally through PgBaselineProvider.IsCommandTimeout as the log arm already is), missing_schema (42P01 / 42703 — the deploy-window skew the collectors log at Debug; still a family that was not read), cancelled (an OperationCanceledException that was NOT the pass's own abandonment — the only kind the when filter lets through), error. CollectionCaveats builds the sentence and the block once so both SKUs say the same thing (the WindowCoverage pattern): Describe → "N of M fact families could not be read (family (outcome), …) — the absence of findings is not evidence: …", ToPayload → { families_failed, families_total, entries: [{family, read, outcome, message}] }, Compose joins it with the coverage caveat (null when both are null), and Attach(payload, …) — the byte rule — returns the caller's own payload object UNTOUCHED when nothing failed, and otherwise serializes it to a JsonObject and appends collection_caveats as the LAST property. families_failed counts DISTINCT families (a family whose two reads both failed is one missing family); entries is one per failed read. CollectionCaveatState(Failures, FamiliesTotal) is what the facts read hands back.

Family label. Derived, never tabled. The SQL Server collectors report from the collect method itself, so the label is the method's: CollectionFailure.FamilyOf("CollectPlanRegressionFactsAsync") → plan_regression, CollectTraceFlagFactsAsync → trace_flag. The PostgreSQL-target collector splits several families across helper reads (vacuum is three methods), so its reporter takes [CallerFilePath] and the label is the partial FILE: PgTargetFactCollector.Write.cs → write, .Vacuum.cs → vacuum; the method name is kept as the entry's read. CallerFilePath is a compile-time constant at each call site, so a content lane's catch names its family by being in its file; either directory separator is honoured.

families_total. CollectionCaveats.CountFamilies(typeof(TCollector)) reflects the collector's non-public instance Collect*Async(AnalysisContext, List<Fact>) methods — 16 on the PostgreSQL target (the census list), 32 on either SQL Server collector (thirty-one families plus the coverage witness) — computed once per process into a static readonly and stamped onto the context BEFORE the coverage witness runs, so a pass that fails at its first read states "1 of 32", not "1 of 0". Not hard-coded anywhere; an unstamped total reads "an unknown number of".

Collectors. Each ReportCollectionFailure calls context.RecordCollectionFailure(...) as its FIRST statement, ahead of the log arms, so no level can skip the record; the log arms and levels are unchanged. Darling's classifier is one internal static PgFactCollector.ClassifyOutcome(Exception) shared with the PostgreSQL-target collector so the two store collectors cannot classify the same exception two ways; Lite's is OperationCanceledException ? Cancelled : Error, per its own doc's argument that no timeout can arrive there.

Services (twins). AnalyzeAsync takes LastCollectionFailures / LastCollectionFamilyCount off the context on the lines after LastWindowCoverage, logs ONE Warning summary when any family failed (a scheduled pass has no payload to carry the block), and after ScoreAll records LastFactCount (facts the scorer saw — collector's plus detector's) and LastFactsScored (of those, how many graded above zero) — before attribution appends its cards. CollectAndScoreFactsAsync returns (Facts, Coverage, Caveats); every return, the catch included, hands back CollectionCaveatState.From(context), so a read that recorded failures before a throw still reports them. The pass body's AnalysisWindowEmptyRuleParityTests slices are untouched: the new lines sit between the coverage stamp and the unobserved branch, and after ScoreAll.

Tools (twins). analyze_server: the unavailable (dead-collector), empty and findings envelopes all go through Attach; the empty message and the unavailable message gain COLLECTION CAVEAT: <sentence> when owed; the findings caveat is Compose(coverageCaveat, collectionCaveat) — null on a clean full-coverage pass exactly as before. The empty envelope additionally carries fact_count and facts_scored (the v1 exit check's ask) — the ONE deliberate byte change to a clean payload, on empty only, disclosed here. get_analysis_facts: the zero-facts miss, the unobserved miss and the data result go through Attach; the miss messages gain the sentence; the data caveat is composed. No tool body spells collection_caveats = as a property — that would serialize a null on every clean pass (McpHelpers.JsonOptions writes nulls). Both SKUs' analyze_server and get_analysis_facts descriptions name the field, its shape, the absent-on-clean rule, and (on analyze_server) the two counts.

Why this shape

Emit-only-when-non-empty via Attach rather than a nullable property. The SQL Server exit checks pin a clean pass byte-for-byte and the serializer writes nulls; a collection_caveats = null property is a byte change on every clean pass for a caveat that was not owed. Attach returning the same reference makes the clean path the OLD path by construction — the tests pin Assert.Same and prefix-identity of the serialized string.

A third tuple element rather than a property on the read. CollectAndScoreFactsAsync's coverage is returned, not parked, because that path has no IsAnalyzing guard and two on-demand callers can overlap; the failures ride the same way for the same reason. One record (CollectionCaveatState) rather than two elements.

File-derived family on the PostgreSQL target. The alternative — wrapping each family call in the root's emission list — would rewrite the exact lines PgTargetFactCollectorTests pins, and the plan says that census must not change. CallerFilePath costs one parameter on the reporter and no line in any partial.

missing_schema as its own outcome. The STEP listed timeout / cancelled / error; the collectors already hold a distinct third arm for 42P01 / 42703 (Debug, "pre-migration skew"), and folding it into error would have the payload call a fault what the log calls expected. Naming it keeps the payload and the log in agreement.

What it does NOT do

• Does not change any log line or level; does not add a catch to the families that deliberately carry none (the coverage witness, the SQL Server wait / blocking / deadlock / server-config reads — those fail the pass loudly, as designed).
• Does not touch InferenceEngine.cs (lane 23's file), FactScorer.cs, FactAdvice.cs, any PgTarget* partial, the PgTargetFactCollectorTests census, or DarlingWorker.cs.
• Does not change a clean pass's findings envelope, unavailable envelope, or any get_analysis_facts payload by a byte. The empty envelope gains exactly fact_count and facts_scored.
• Does not mirror to deprecated/Dashboard — its AnalysisService has the same collect+score shape; listed for the coordinator (frozen twin, bug-fix support only). Its Dashboard.Tests builds clean against the additive AnalysisContext change.
• Does not surface the failures in the Viewers or the scheduled worker's persisted marker (the Warning summary line is what a scheduled pass leaves).

Tests

All new / changed tests compile-verified here; execution noted per item.

• Darling/Darling.Tests/CollectionCaveatsTests.cs (NEW, 10 tests) — FamilyOf / FamilyOfFile derivations on both separators and the deterministic-build /_/ prefix; every PgTargetFactCollector.*.cs file labels a family the collect surface declares (the four name mismatches spelled out); CountFamilies = 16 / 32; ClassifyOutcome on all four arms (57014, TimeoutException, wrapped Npgsql, 42P01, 42703, OperationCanceledException, bare Npgsql, InvalidOperationException); the record lands on the context; the clean-pass rule (Assert.Same on the payload, no sentence, no block, Compose nulls); the failing-pass shape — serialized string is prefix-identical to the clean one with the block appended last, DISTINCT families_failed (vacuum twice = 1), entry fields, sentence text; an unstamped total reads "an unknown number of". Executed here: 10/10.
• Darling/Darling.Tests/CollectionCaveatsParityTests.cs (NEW, 3 tests) — source pins over both SKUs (the AnalysisWindowEmptyRuleParityTests pattern): every reporter records FIRST and once, every collector stamps its total before the witness, file-vs-method labelling per collector; both passes take failures/total on the lines after coverage, log the summary before the unobserved branch, read the two counts between ScoreAll and attribution; both reads return the state on all three returns and no old-shape return remains; both tool files Attach three envelopes per tool, compose the caveat once per tool, never spell collection_caveats = as a property, and both descriptions carry the field. Together with the existing FactCollectorFailureReportingTests (every swallowing catch → a reporter) this is the census: every catch in a collect method reaches the recorder. Executed here: 3/3; red-first on the reporter pin by commenting the record out (below).
• Darling/Darling.Tests/PgTargetCollectionCaveatsLiveTests.cs (NEW, gated live e2e) — a server stamped engine_kind='postgres', 25 h of pg_database_stats, the REAL DarlingMcpTools.AnalyzeServer: the clean pass carries NO collection_caveats (absent, not null) and stamps 16; then pg_autovacuum_stats is renamed away — the one table read by exactly one target family (the STEP's pg_write_stats example is also read by the buffer family and the WAL baseline) — and the pass returns collection_caveats = {1, 16, [{vacuum, ReadAutovacuumBacklogAsync, missing_schema, …}]} as the last property, with "1 of 16 fact families could not be read (vacuum (missing_schema)) — the absence of findings is not evidence" in the message (or caveat), fact_count / facts_scored on empty; get_analysis_facts on the same store carries the block and the caveat. The rename is undone INSIDE the LiveStoreCleanup.RunAsync body ahead of the row deletes (The other ~124 live-test finally blocks still clean up on the body's connection, so a failing test can still report its teardown's error #1902 shape). Executed here against a throwaway timescale/timescaledb:2.28.1-pg18 (Docker context desktop-linux, torn down): green; red-first: with the PostgreSQL-target record line commented out the test fails on the missing block and the store is still restored (table back, rows gone). Also executed green on the same container: AnalyzeWindowEmptyLivePostgresTests, PgTargetFactCollectorTests (incl. e2e), AsOfWindowAnchorLivePostgresTests, DarlingAnalysisServiceEngineRoutingTests, PgTargetMcpSurfaceTests, PgTargetSharedSwitchRoutingTests, AnalysisWindowEmptyRuleParityTests, AnalysisFactsReadRunsDetectorParityTests, FactCollectorFailureReportingTests, RecurrenceLabelStoreReadTests, McpPayloadContractCensusTests, FactCollectorCommandTimeoutTests, AnalysisPassCommandTimeoutTests, AnalysisPassTokenThreadingTests, StoreSqlClockDisciplineTests, DocCommentHygieneTests, LiveCleanupConversionRatchetTests, LivePostgresCollectionHygieneTests, RepoFileAdoptionTests, CommentFilterAdoptionTests, AsOfWindowAnchorTests, and Lite's McpMissMessageParityPinTests — 400/400 in a net10.0 harness run from inside the repo tree.
• Lite.Tests/CollectionCaveatsTests.cs (NEW, 2 tests) — a private DuckDbInitializer on a temp path (the AnalysisDataSpanTests shape, because the fault must not reach the shared fixture) with v_trace_flags dropped: (1) the collector records exactly trace_flag / CollectTraceFlagFactsAsync / error with total 32 while the wait family still reads its benign wait over an observed window; (2) through the real McpAnalysisTools.AnalyzeServer on a 25 h store — the clean empty all-clear carries no collection_caveats (absent) and both counts, then with the view dropped the same empty carries the block (1 of 32, trace_flag, last property) and "All metrics are within normal ranges. COLLECTION CAVEAT: 1 of 32 fact families could not be read (trace_flag (error)) — …" in the message; get_analysis_facts on the same store carries the block and caveat. First executed in CI — AnalysisService lives in the WPF project and cannot load on this Mac. Right by construction: the seeds are AnalysisWindowEmptyTests' verbatim (the 25 h gate + fifteen-minute benign wait the all-clear control proves observed), v_trace_flags is read by exactly one Lite family and that family's catch reports, and the wait fact's key is its wait type.
• Lite.Tests/McpMissMessageParityPinTests.cs (CHANGED) — three shared fragments join SharedMissFragments: the COLLECTION CAVEAT: label and the two description sentences. Executed here: 61+3 green.
• Changed for the third tuple element (var (facts, coverage, _)): Lite.Tests/AnalysisWindowEmptyTests.cs, Lite.Tests/AnomalyDetectorTests.cs (×2), Darling/Darling.Tests/AnalyzeWindowEmptyLivePostgresTests.cs; AnalysisFactsReadRunsDetectorParityTests.ReadEntry names the new signature.

Census pre-checks: AnalysisWindowEmptyRuleParityTests (observed-zero body untouched; new lines sit before the unobserved branch and after ScoreAll), PgTargetFactCollectorTests (census list, root emission strings and [CallerMemberName] string collectMethod all still present), FactCollectorFailureReportingTests (54-site floor unchanged; the SQLSTATE arm regex still hits the reporter first), DocCommentHygieneTests (crefs short-form under a using), RepoFileAdoptionTests (new pins use the non-LF reader with single-line anchors — no roster entry owed), LiveCleanupConversionRatchetTests (the rename undo lives inside LiveStoreCleanup.RunAsync), McpPayloadContractCensusTests (total_facts = facts.Count untouched), AsOfWindowAnchorTests (no clock added), StoreSqlClockDisciplineTests (no SQL added).

Which component(s) does this affect?

• Lite
• Darling
• Lite Tests
• Darling Tests
• SQL collection scripts
• Documentation
• Full Dashboard (deprecated)
• CLI Installer (deprecated)

How was this tested?

Builds (zero new warnings): PerformanceMonitor.Analysis, PerformanceMonitor.Darling.Analysis, PerformanceMonitor.Darling.Service, PerformanceMonitorLite (-p:EnableWindowsTargeting=true), Darling.Tests, Lite.Tests (one pre-existing xUnit2000 warning in LogTailOverlapThresholdPinTests, untouched), deprecated/Dashboard.Tests. Live PostgreSQL execution as above against PG18 / TimescaleDB 2.28.1 in a throwaway container. Lite behavioural tests are CI-first.

Checklist

• I have read the contributing guide
• My code builds with zero warnings (dotnet build -c Debug)
• I have tested my changes against at least one SQL Server version (PostgreSQL-target path executed live; SQL Server path is the Lite fixture, CI-first)
• I have not introduced any hardcoded credentials or server names