Repository navigation
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
Conversation
…… 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"`).
|
Coordinator review under the review-dark window. The one-token rule change ( |
What does this PR do?
Headline mechanism. Both analysis services (
DarlingAnalysisService, Lite'sAnalysisService) 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 wasfacts.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_serveragainst a server whose collector was up for the whole window, but whose window held nothing any family turns into a fact, answeredstatus: 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 intoanalysis_stateas 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 readsserver_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 readsemptywith coverage 1.0 today only because the skeleton emits onePG_SERVER_MAJOR_VERSIONfact, which is a fact about the registry, not about the window.Why this shape
WindowEmptyMessagemeans "dead collector": bothanalyze_servertools map it tounavailableand append "why they stopped"; the worker writes it as theanalysis_statemarker; 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' existingemptyshape 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.emptypayload always carrieshints.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 namingCoverage.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.COLLECTION_GAPcontext fact whenever the observed fraction is underWindowCoverage.PartialThreshold, sofacts.Count == 0with 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.Coverageis 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.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 composedWindowEmptyMessagestatement — 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.cswhile thelitefilter 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).DarlingMcpTools.cs,McpAnalysisTools.cs): "zero facts in the window is a DEAD-COLLECTOR shape" → an UNOBSERVED window is; and theemptyarm'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_serverstatus vocabulary, theempty/unavailableprose,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 theemptyprose 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
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 throughCollectAndScoreFactsAsync(Empty,IsObserved, not partial) so the arm is known to exercise the zero-facts path and not "facts but no findings" (the existing control). Thenanalyze_server:empty,hints.coverage.observed_fractionin [0.99, 1.0],unobserved/partialfalse, no "NOT an all-clear" / "appears to have stopped" / "get_collection_health" / "PARTIAL";WindowEmptyMessageandInsufficientDataMessagenull. The two existing arms (unobserved, no facts; unobserved with point-in-time facts inAnalysisCoverageTests) are unchanged and green.AnalyzeWindowEmptyLivePostgresTests.AnObservedWindowWithNoFacts_IsAnAllClearAtItsCoverage_NotADeadCollector(gated,DARLING_TEST_PG): the twin, plus aCapturingTestLoggerpin 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".AnalysisWindowEmptyRuleParityTests(ungated): the two pins described above.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
runtimeconfigtrick): Darling — the two live arms and the parity pins against a throwawaytimescale/timescaledb:2.28.1-pg18store migrated to the current ladder, plusAnalysisCoverageLivePostgresTests,DarlingAnalysisPipelineTests,DarlingAnalysisServiceEngineRoutingTests(its stamped-row pass is the Darling unobserved-with-point-in-time arm),PgTargetFactCollectorTests— 78/78;DocCommentHygieneTests77/77 from inside the tree (the newcrefs resolve). Lite —AnalysisWindowEmptyTests3/3,AnalysisCoverageTests11/11. Builds:Darling.TestsandLite.Tests-c Release -p:EnableWindowsTargeting=true, 0 warnings of ours (the one Lite warning,LogTailOverlapThresholdPinTests.cs:95xUnit2000, is pre-existing on dev from #3646). First execution of the Windows-hosted suites is CI.