Skip to content

The analysis pass said "collection appears to have stopped or broken … NOT an all-clear" for a window the coverage witness proves the collector observed: the empty-window rule tested facts.Count == 0 before it tested the witness, so an observed window that simply produced no fact wore the dead-collector envelope — unavailable with a pointer at collection health, the Viewer's dead-collector marker. The rule now gates on the witness alone, and an observed window with no facts runs the pass and reads empty at its stated coverage, both SKUs (#3653) - #3687

Merged
erikdarlingdata merged 1 commit into
devfrom
fix/3653-empty-window-wording
Sep 19, 2026

Conversation

@erikdarlingdata

Copy link
Copy Markdown
Owner

What does this PR do?

Headline mechanism. Both analysis services (DarlingAnalysisService, Lite's AnalysisService) decide the dead-collector envelope — WindowEmptyMessage, the text that ends "Collection appears to have stopped or broken for this window, so nothing was measured — this is NOT an all-clear" — on the coverage witness alone: context.ObservedDurationMs <= 0. The rule was facts.Count == 0 || context.ObservedDurationMs <= 0. The first half is the original #3524 test, written when no collector stamped coverage and "no facts" was the only evidence of a dead collector there was. Since #3592 (SQL Server, wait_stats) and #3665 (PostgreSQL target, pg_database_stats) every pass carries an OBSERVED coverage witness, and the two halves stopped agreeing: an observed window over which nothing rose to a fact tripped the first half and wore prose the second half proves false. This is the "Empty-window rule wording, both SKUs" item on #3653 — partial, the issue stays open.

The lie it stops telling

analyze_server against a server whose collector was up for the whole window, but whose window held nothing any family turns into a fact, answered status: unavailable — "collection appears to have stopped or broken … NOT an all-clear … Check get_collection_health to see when collectors last succeeded and why they stopped." The witness had just measured 100% of the window. The scheduled pass wrote the same sentence into analysis_state as the dead-collector marker the Viewer's Recommendations tab renders with its collection-health pointer. On a SQL Server target this is rare but real (the metadata fact reads server_properties, the wait fact needs accrued time, every other family needs a row); for a PostgreSQL target it is the plumbing's stated exit criterion until content lands — #3665 reads empty with coverage 1.0 today only because the skeleton emits one PG_SERVER_MAJOR_VERSION fact, which is a fact about the registry, not about the window.

Why this shape

  • Observed-zero is not a third envelope; it is a pass. Every consumer of WindowEmptyMessage means "dead collector": both analyze_server tools map it to unavailable and append "why they stopped"; the worker writes it as the analysis_state marker; both viewers suffix the collection-health pointer. None of those is true of an observed window, so the message must stay null — not carry different words — and the pass must continue. It does: the anomaly detector reads the store on its own and still gets its say ("the collector emitted no fact" is a statement about the collector's reads, not the detector's), and whatever survives scoring is the answer. A zero-finding pass out of here is the tools' existing empty shape with the coverage block, exactly the envelope PostgreSQL targets enter the analysis pipeline: the pass routes by registry engine_kind inside the service, a pg_database_stats coverage witness replaces the tombstone, and every shared switch gains its one pg_ arm so the content lanes can build in parallel (#3542 v1 plumbing) #3665 named for a PostgreSQL target, and the worker clears the marker as it does for any completed pass. The status vocabulary is untouched.
  • The all-clear says what coverage it rests on. The empty payload always carries hints.coverage (observed_fraction, observed_hours, sample_count, partial, unobserved); the partial-coverage prose is already scoped. For the scheduled pass, which has no payload, the service logs one Information line naming Coverage.Describe() — "the collector observed 100% of this 4h window …; nothing rose to a fact, and an all-clear from this pass rests on that coverage". Information, not Warning: nothing is wrong with the collector, and a Warning here would be the old lie at a lower volume.
  • Partial coverage cannot reach the new branch, by construction. Every collector adds the COLLECTION_GAP context fact whenever the observed fraction is under WindowCoverage.PartialThreshold, so facts.Count == 0 with observed time means coverage at or above the bar. The code comment states this so the next reader does not add a partial arm that can never fire.
  • "Unstamped" is not a third state either. Coverage is null only when collection threw, and that unwinds through the pass's catch, never through this test; both collectors stamp the witness before their first windowed read.
  • Twin edit, and pinned as one. The two services carry the same comment block and the same message statement; a new Darling.Tests pin (AnalysisWindowEmptyRuleParityTests) reads both from source and asserts the rule's shape (witness-alone gate; old spelling gone from CODE, in either operand order; observed-zero branch after the envelope, before the partial caveat and the detector; sets no message, does not return, logs Information naming the coverage) and that the composed WindowEmptyMessage statement — both arms — is byte-identical across SKUs and still says "appears to have stopped" and "NOT an all-clear", because that prose is now true of exactly the case it is emitted for. It lives in Darling.Tests rather than Lite.Tests because Darling's build filter reaches every Lite .cs while the lite filter does not reach the Darling analysis tree (CrossAppGuardCiGateTests, PR CI skips Lite.Tests when only Darling changes, so cross-app parity guards do not gate merges #2839).
  • Two MCP-tool comments corrected, one clause each (DarlingMcpTools.cs, McpAnalysisTools.cs): "zero facts in the window is a DEAD-COLLECTOR shape" → an UNOBSERVED window is; and the empty arm's "Facts WERE collected and scored this time" now says the window WAS observed, whether facts scored or none were emitted. Comment-only; no code in either tool moved.

What it does NOT do

No change to the analyze_server status vocabulary, the empty/unavailable prose, McpHelpers.Status, the worker's marker write, or either Viewer. The unobserved-window envelope's text is byte-identical to before in both arms (point-in-time facts present / bare). No new public surface on the services. It does not make the empty prose distinguish "facts scored, nothing fired" from "no fact emitted" — the tool does not know the fact count, and adding that surface is a bigger decision than this wording item; the coverage block is the same honest number in both cases. It does not address the pre-existing property that per-family collector failures are logged, not carried on the context (a pass with thirty silently-failed families and one surviving fact has always read as a normal pass; this PR does not change that in either direction). No changelog edit (proposed entry in the lane report). No schema.

Tests

  • Lite AnalysisWindowEmptyTests.AnObservedWindowWithNoFacts_IsAnAllClearAtItsCoverage_NotADeadCollector (DuckDB, SharedDuckDbFixture): 30 h of history clears the real 24 h gate; a reading every fifteen minutes across the window with ZERO accrued wait time gives the witness intervals and the wait read nothing. Fact count asserted through CollectAndScoreFactsAsync (Empty, IsObserved, not partial) so the arm is known to exercise the zero-facts path and not "facts but no findings" (the existing control). Then analyze_server: empty, hints.coverage.observed_fraction in [0.99, 1.0], unobserved/partial false, no "NOT an all-clear" / "appears to have stopped" / "get_collection_health" / "PARTIAL"; WindowEmptyMessage and InsufficientDataMessage null. The two existing arms (unobserved, no facts; unobserved with point-in-time facts in AnalysisCoverageTests) are unchanged and green.
  • Darling AnalyzeWindowEmptyLivePostgresTests.AnObservedWindowWithNoFacts_IsAnAllClearAtItsCoverage_NotADeadCollector (gated, DARLING_TEST_PG): the twin, plus a CapturingTestLogger pin on the one log line — Information:, names the server, contains "the collector observed 100% of this 4h window", "an all-clear from this pass rests on that coverage", and never "collection may be down" / "No observed collection in the analysis window".
  • Darling AnalysisWindowEmptyRuleParityTests (ungated): the two pins described above.
  • Red first. Both e2e arms were run against the pre-change service (the service file stashed, harness rebuilt): both fail on Expected: "empty", Actual: "unavailable" — the exact lie — and pass with the change.

Executed on this machine through net10.0 harnesses (the Analysis assemblies are WPF-free; Lite's via the runtimeconfig trick): Darling — the two live arms and the parity pins against a throwaway timescale/timescaledb:2.28.1-pg18 store migrated to the current ladder, plus AnalysisCoverageLivePostgresTests, DarlingAnalysisPipelineTests, DarlingAnalysisServiceEngineRoutingTests (its stamped-row pass is the Darling unobserved-with-point-in-time arm), PgTargetFactCollectorTests — 78/78; DocCommentHygieneTests 77/77 from inside the tree (the new crefs resolve). Lite — AnalysisWindowEmptyTests 3/3, AnalysisCoverageTests 11/11. Builds: Darling.Tests and Lite.Tests -c Release -p:EnableWindowsTargeting=true, 0 warnings of ours (the one Lite warning, LogTailOverlapThresholdPinTests.cs:95 xUnit2000, is pre-existing on dev from #3646). First execution of the Windows-hosted suites is CI.

…… NOT an all-clear" for a window the coverage witness proves the collector observed: the empty-window rule was `facts.Count == 0 || ObservedDurationMs <= 0`, written before any collector stamped coverage, and its first half made an observed window that simply produced no fact wear the dead-collector envelope — `unavailable` with a pointer at collection health on both analyze_server tools, the Viewer's dead-collector marker from the worker. The rule now gates on the witness alone; an observed window with no facts runs the pass and reads `empty` at its stated coverage, both SKUs (#3653)

The dead-collector envelope (WindowEmptyMessage) is set on `ObservedDurationMs <= 0`
alone, both arms byte-identical to before. `facts.Count == 0` with observed time
falls through: WindowEmptyMessage stays null, the anomaly detector still runs, and
a zero-finding pass renders as the tools' existing `empty` shape with the coverage
block (#3665's PG exit criterion). One Information line names the coverage for the
scheduled pass. Partial coverage cannot reach that branch — the collector adds
COLLECTION_GAP whenever coverage is under the bar.

Tests: Lite DuckDB e2e arm (observed, zero facts → `empty`, coverage ≈ 1, no
dead-collector prose; fact count asserted through CollectAndScoreFactsAsync);
Darling live-PG twin with the captured Information line; a Darling.Tests
cross-SKU parity pin over the rule's shape and the composed message. Both
e2e arms proven red on the pre-change service (`Expected: "empty"`,
`Actual: "unavailable"`).
@erikdarlingdata

Copy link
Copy Markdown
Owner Author

Coordinator review under the review-dark window. The one-token rule change (facts.Count == 0 || removed; gate on ObservedDurationMs <= 0 alone) is the spec; the fall-through was the risk and it is proven rather than argued — the live arm ran the whole pass on an empty fact list through scoring, stories, clustering, muting and insert and produced empty at stated coverage. Cross-SKU parity is pinned on both the rule shape and the composed message, in Darling.Tests for the #2839 reason. The reviewer question left open — whether empty should distinguish "no fact emitted" from "facts scored, nothing fired" — is a real one and correctly not taken tonight: the coverage block is the honest number either way, and a fact-count surface on two services and two tools is its own lane. Arming.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant