Skip to content

The wait and perfmon baselines stop counting every restart's fabricated zero as a quiet sample: interval-honest successor aggregates replace the legacy pair in place, the provider reads whichever supply covers its window and gates the LAG heuristic on a NULL interval, the legacy pair retires by a self-executing coverage condition, the startup backfill reads coverage off the materialization, and the Daily-routed calendar answers its unmaterialized days from raw (partial #3653: A6 + A10 mechanical half) - #3698

Merged
erikdarlingdata merged 1 commit into
devfrom
fix/3653-baselines-interval
Sep 19, 2026

Conversation

@erikdarlingdata

Copy link
Copy Markdown
Owner

What does this PR do?

The lie: the wait-stats and perfmon anomaly baselines counted every restart's fabricated zero as a quiet sample. collect.wait_stats_baseline and collect.perfmon_baseline summed delta_wait_time_ms >= 0 / delta_cntr_value >= 0 with no interval filter, so a restart collection — every row written as delta 0 with sample_interval_seconds = 0, the collector's own "no delta knowable" verdict — became a total_wait_ms = 0 row that dragged the mean down and the stddev up. The provider's only guard was a magnitude heuristic (LAG > 10000 / > 1000), which catches a restart only when the collection before it was busy, and which also dropped real idle minutes after busy ones. A continuous aggregate's query cannot be altered in place, and a rebuild forfeits the 35-day tier that the 30-day raw horizon cannot refill — #3527 declined exactly that for perfmon_baseline, and V127's rung note named this shape as the follow-up.

Mechanism. Two interval-honest successor aggregates, collect.perfmon_interval_baseline and collect.wait_stats_interval_baseline, with sample_interval_seconds IS DISTINCT FROM 0 baked into the WHERE (Lite's BaselineProvider filter verbatim) and max(sample_interval_seconds) carried per collection. A restart collection produces no row; a pre-column (NULL) collection is kept; a measured collection carries its interval. PgBaselineProvider's three arms (BatchRequests, WaitStats, WaitMsPerSec) read the successors, divide by the stored interval (LAG-derived only where NULL), and apply the magnitude heuristic only where sample_interval_seconds IS NULL — the three-state rule: 0 = unknowable (already gone), NULL = never recorded (heuristic is the only guard), n = measured and authoritative. Lite's twin arms take the same gate; Lite's BatchRequests QUALIFY heuristic is retired outright because perfmon_stats has carried the column since Lite's first schema and its > 0 filter already removes every restart.

Why a replacement in the registry, not the append the issue's dispatch line sketched. BaselineAggregates drives HourlyRefreshPhaseOrder, AggregateCompressionTargets and RetentionPolicies by position and count. Appending two aggregates widens the light band by two minutes: HeaviestRefreshStartMinute 15→17, HeaviestRefreshWindowMinutes 21→19, RefreshSlotWarningSeconds 1,050 s → 950 s against the 896 s recorded ceiling — a permanent 100 s of lost lead time on the one refresh with a measured growth series, plus the re-derivation of every literal and prose pin on that geometry (TimescaleSupportTests, RefreshCeilingProvenancePinTests), bought for a five-day transition. The successors took the legacy pair's positions instead: seven entries, same minutes (:03, :05), same compression hours (14, 15), same policy count. BaselineSupplyTests.Successors_TookTheLegacyPositions_SoThePhaseGridDidNotMove pins it.

The self-executing retirement. The legacy pair is no longer registered anywhere the ensure sweep reads, so it is never re-created; it lives on only on stores that ran a pre-#3653 build, holding up to 35 days the successors do not yet have. TimescaleSupport.SupersededBaselineRelations pairs each legacy name with its successor, and DropRetiredBaselineAggregatesAsync (now with a utcNow overload; the worker's call site is unchanged) judges each on every start: a legacy continuous aggregate drops once the successor's min(bucket) reaches the tier horizon (host clock minus BaselineRetentionSpan, bound as a parameter — never now() in store SQL); a legacy plain view (plain-PostgreSQL stores) drops as soon as its successor exists; an empty successor never releases anything. DROP MATERIALIZED VIEW ... CASCADE takes the legacy's refresh, retention and compression jobs with it. With the collectors' default 30-day raw horizon the successors are backfilled to ~30 days on the first start, the provider switches to them about a day later (PrefersSuccessor: the successor must reach as far back into this server's window as the legacy can contribute — a server registered after the upgrade goes straight to the successor), and the drop fires on the first service start at or after ~day five. Until then the provider reads the legacy text (GetLegacyBaselineQuery, verbatim pre-#3653) for the window the successor does not cover.

Transitional cost, stated: for those ~five days the legacy aggregates' own jobs keep running on the minute and hour their registry position had — the same minute the successors now take. Two sub-3 s share-lock refreshes on the same raw hypertable at one minute is lock adjacency, not the exclusive-lock convoy #3012 recorded, and both clear inside the 4-minute compression guard; removing the legacy policies on sight was rejected because it would leave the legacy relation frozen at its watermark and make every provider read of it union in days of raw wait_stats.

Two defects found on the rig while proving the rollout, both fixed here:

  1. BackfillBaselineAggregatesAsync's coverage gate read min(bucket) through the real-time view. On a WITH NO DATA aggregate the watermark is -infinity, the real-time branch serves every raw row, and the probe reports raw's own oldest bucket — so the gate read "already covers" on the very start that created the aggregate and skipped. One policy refresh later the watermark jumped to the trailing day and everything older was served by neither branch (19 of 20 planted days went invisible on the rig) until the next start's gate saw a real floor. Every PgBaselineProvider times out client-side scanning raw history, and tiered retention will mask it rather than fix it #1757 aggregate rolled out through that two-start delay. The probe now reads the materialization hypertable's floor (BaselineBackfillProbeSql(view, source, materialization), resolved through timescaledb_information.continuous_aggregates); the view form remains for plain views. On the rig: backfill 0 → 1 on the first start, 2,860 rows in the materialization itself.
  2. DailySummarySql's routed queries CTE: both query rollups are materialized-only (no materialized_only = false; TimescaleDB 2.13+ defaults real-time off) and the daily's policy lags a day or two, so a calendar window routed to the Daily tier printed unique_queries = 0 for yesterday and today beside fresh raw wait/CPU numbers — the issue's "unique_queries = 0 beside fresh raw numbers". The routed CTE now takes the rollup up to its last materialized day for the server and raw query_stats for the days after it (raw keeps four days, the lag is at most two), partitioned on the day so no day gets two rows. Proven on the rig: Raw/Hourly/Daily tiers all read 3 and 7 for the two planted days.

What it does NOT do

  • DST (A10's last clause, hour-of-week buckets keyed UTC) — awaits ruling Q6 on Brains-review campaign: deferred structural residue (from #3538 / #3539 / #3540 / #3541) #3653.
  • query_stats_baseline gets no successor, and does not need one: its delta_execution_count > 0 row filter already excludes every unknowable row (interval 0 ⟹ delta 0 by the calculator's invariant), so a restart collection forms no row, and its baseline is a per-collection total, not a rate. Stated in PlanCacheAnomalyDetector.IsRealDeltaRow's new doc paragraph too: the SQL tiers read the collector's stored verdict, the in-memory tier keeps the predicate — two statements of one rule, neither an orphan.
  • The >24 h outage hole (RetentionTierRouter): read honestly and written into its self-disclosure — the hole is the un-materialized pre-outage tail (≤ end_offset + one schedule interval, ~2 h), not the outage's length; nothing repairs it (both backfills measure the floor); the fix that fits without a rung is a startup gap-scan + forced refresh, its own lane because the scan has to be bounded by shape on the largest raw tables and there is no collection-run ledger. Q for the maintainer: approve that lane?
  • NULL-not-0 for a day the rollup skipped below its ceiling in the daily summary — a payload-shape change (model, reader, MCP tool, both calendars) that belongs with the census lane's DailySummaryRetention.StateFor vocabulary.
  • The --backfill-rollups verb is untouched: it enumerates RollupViews only; the baseline tier is backfilled by the startup pass, which this PR makes fire on the first start.
  • PgMigrations.cs's V127 note still says "the follow-up is a new aggregate under a new name" — true; pointing it at this PR is a one-line touch for the next rung-carrying lane.

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: every touched project plus Darling.Tests and Lite.Tests, -c Release -p:EnableWindowsTargeting=true, 0 warnings (the one warning Lite.Tests emits on a clean rebuild is the pre-existing xUnit2000 at LogTailOverlapThresholdPinTests.cs:95 on dev).

Executed here, not only compiled:

  • Darling pure pins via a net10.0 harness (AssemblyName Darling.Tests): BaselineSupplyTests (+6 new: interval filter/column, position pin, supersession disjointness + provider map, SupersededBaselineRelationDropsAt across time, PrefersSuccessor across store states, materialization-probe shape), DarlingAnomalyBaselineTests (+1: the gate present in successor text / absent from legacy text), RetiredBaselineAggregateTests, TimescaleSupportTests (the whole grid), TimescaleContinuousAggregateTests, TimescaleAggregateCompressionTests, RefreshCeilingProvenance/StalenessTests, StoreSqlClockDisciplineTests, TsqlConventionGuardTests, DocCommentHygieneTests, RollupBackfillTests — 374/374.
  • Live, on a throwaway timescale/timescaledb:2.28.1-pg18 migrated to the current ladder (DARLING_TEST_PG): all 22 gated live tests in that set green, including the new Supersession_RestartZeroInLegacyNotSuccessor_ProviderFollowsCoverage_SweepWaitsForTheTier_AgainstDevPostgres — legacy and successor CAGGs side by side over the same planted history (three interval states, two hours, twelve collections each); the restart collection is a total_wait_ms = 0 row in the legacy (24 rows) and absent from the successor (23); the provider reads the legacy while the successor is one hour shallower (12 samples, mean 45,417 — the unseen restart zero inside it) and the successor once it catches up (11 samples, mean 49,545); the rate arm rates the window's first collection off its stored interval; the day-one sweep holds (SuccessorShort), the day-forty sweep drops the legacy and its policy. Plus the re-planted perfmon live test (restart as interval 0 → 11 samples / 90.909).
  • Rig proof through the production entry points, in the worker's order: drop pass → EnsureContinuousAggregatesAsync (20/20, successor policy on :05 fixed) → BackfillBaselineAggregatesAsync (1 materialized; 2,860 rows in _materialized_hypertable_N, interval 600 carried) → verdict day one SuccessorShort, +15 d Drop → EnsureAggregateCompressionAsync 20, EnsureRetentionPoliciesAsync (35 days on the successor) → drop pass at +15 d drops 1, legacy jobs 0, third pass no-ops. And the daily-summary SQL for all three tiers against planted query_stats.
  • Lite in-process on DuckDB (WPF-hosted harness): BaselineProviderTests (+2: measured idle zero kept; wait-stats three-state), RobustBaselineDuckDbTests, AnomalyDetectorTests — 43/43.

Rig torn down. References #3653 (partial: A6 + the mechanical half of A10) — not closed.

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 (store-side change; proven against PostgreSQL 18.4 + TimescaleDB 2.28.1 and DuckDB in-process)
  • I have not introduced any hardcoded credentials or server names

…ed zero as a quiet sample: interval-honest successor aggregates (sample_interval_seconds IS DISTINCT FROM 0 baked in, the measured interval carried) replace the legacy pair in place, the provider reads whichever supply covers its window and gates the LAG > N heuristic on a NULL interval, the legacy pair retires by a self-executing coverage condition, the startup backfill reads coverage off the materialization, and the Daily-routed calendar answers the days its rollup has not reached from raw (partial #3653: A6 + the mechanical half of A10)

Shape, and why not the brief's append: BaselineAggregates drives HourlyRefreshPhaseOrder,
AggregateCompressionTargets and RetentionPolicies by position and count, so appending two
aggregates moves the light band by two minutes (HeaviestRefreshStartMinute 15->17, the watch
line 1,050 s -> 950 s against the 896 s ceiling) for a five-day transition. The successors
took the legacy positions instead; the grid, hours and policy count are unchanged and pinned.

Retirement: SupersededBaselineRelations, judged on every start by
DropRetiredBaselineAggregatesAsync(utcNow) - a legacy CAGG drops once the successor's oldest
bucket reaches the tier horizon (host clock minus BaselineRetentionSpan, bound as a
parameter); a legacy plain view drops as soon as a successor exists. Measured on a PG18 /
TimescaleDB 2.28.1 rig: day one holds, day five drops, cascade removes the legacy's jobs.

Provider: GetBaselineQuery reads the successors with the three-state rule (0 dropped by the
supply, NULL keeps the heuristic, n authoritative); GetLegacyBaselineQuery keeps the pre-#3653
text; PrefersSuccessor picks per compute against this server's own reach. Lite's twin arms
apply the same gate; its BatchRequests QUALIFY heuristic is retired outright (the column has
always been there).

Also, both found on the rig while proving the rollout: BackfillBaselineAggregatesAsync's
coverage probe read min(bucket) through the real-time view, which on a WITH NO DATA aggregate
is raw's oldest (watermark -infinity), so the gate never fired on the start that created the
aggregate - it now reads the materialization hypertable's floor; and the Daily-routed daily
summary's queries CTE takes raw for the days past the rollup's last materialized day, so
unique_queries no longer prints 0 beside fresh wait and CPU numbers.
@erikdarlingdata
erikdarlingdata merged commit c7c973e into dev Sep 19, 2026
7 of 8 checks passed
@erikdarlingdata
erikdarlingdata deleted the fix/3653-baselines-interval branch September 19, 2026 06:25
erikdarlingdata added a commit that referenced this pull request Sep 19, 2026
…red once, and censuses for rules 1, 2, 3, 5 and 6 (partial #3653)

#3540 stated the contract in one sentence and its review numbered ten rules; two
numbers reached the tree (rule 4 — every family persists its interval; rule 7 —
every family seeded) and the other eight stayed a sentence in an issue. This
writes the list once, in Lite.Tests/MeasurementContractCensusTests' summary, with
each rule's bound and status: 4 and 7 keep the numbers their tests gave them, the
six remaining named rules take the free slots in the body's order, and slots 9
and 10 are left empty rather than invented — the review's enumeration is not on
the record.

Pinned tonight, over source and schema, each with a positive control and a
population floor so an empty offender list cannot be a dead regex:

- Rule 1 (readers NULL-not-0 on unknowable): no product SQL divides by
  sample_interval_seconds except through NULLIF(…, 0); every derived
  interval_seconds alias routes the stored value through NULLIF (two
  PgBaselineProvider arms are the named allowance — they read a #3698 successor
  aggregate whose WHERE already dropped the interval-0 collection, and the census
  asserts that predicate on the named source); every division by the alias sits
  under a positive guard; every rate arm that falls back to ELSE 0 is rostered
  shrink-only (four file-I/O trend columns on both SKUs, nine PostgreSQL trend
  rates, two baseline wait arms — each dead text under its own WHERE).
- Rule 2 (rates divide by measured elapsed): zero divisions of a delta by an
  assumed cadence, SQL and C#, both SKUs — and it stays zero.
- Rule 3 (epoch → ClearServer + marker), as a call-site census over #3694's
  ServerEpoch: exactly the two carriers observe; every forget in the product is
  the comparator's or a host remove path's (exact set, both directions); both
  hosts drain the discontinuity account and compose the measurement onto the
  collection_log note; forget precedes measure precedes persist. Residuals
  rostered: the five families scheduled before the carrier (read off both hosts'
  schedule order), and pg_wait_stats, which has no carrier.
- Rule 5 (rollups aggregate interval > 0 only), in the Darling half against the
  real TimescaleSupport constants: the two #3698 successors carry the predicate,
  query_stats_baseline carries the equivalent delta_execution_count > 0, and the
  roster names what does not — the superseded legacy pair and the three hourly
  rollups over query_stats / procedure_stats, whose min() and sample_count read a
  restart's (0, 0) row as a quiet sample.
- Rule 6 (per-second names divided at comparison): every *_per_sec(ond) alias in
  product SQL is a quotient or a window over one; no collector payload column
  carries the name.
- Rule 8 (gauges never delta'd) is not census-able until cntr_type is stored;
  the perfmon payload's lack of that column is pinned so the rung reds this row.

Two files because the lite CI filter does not reach Darling.Storage or
Darling.Analysis and CrossAppGuardCiGateTests fails a Lite guard PR CI cannot
run: the Darling half sweeps those two trees with the same regexes and holds the
continuous-aggregate census. Test-only; no product code. Every fact executed
here (Lite: WPF-hosted harness, 10/10; Darling: mactest harness, 7/7; the whole
Darling.Tests assembly runs with the same 188 platform reds as dev) and
mutation-checked: twelve planted defects each turned exactly the one fact that
guards them red.
erikdarlingdata added a commit that referenced this pull request Sep 19, 2026
…red once, and censuses for rules 1, 2, 3, 5 and 6 (partial #3653)

#3540 stated the contract in one sentence and its review numbered ten rules; two
numbers reached the tree (rule 4 — every family persists its interval; rule 7 —
every family seeded) and the other eight stayed a sentence in an issue. This
writes the list once, in Lite.Tests/MeasurementContractCensusTests' summary, with
each rule's bound and status: 4 and 7 keep the numbers their tests gave them, the
six remaining named rules take the free slots in the body's order, and slots 9
and 10 are left empty rather than invented — the review's enumeration is not on
the record.

Pinned tonight, over source and schema, each with a positive control and a
population floor so an empty offender list cannot be a dead regex:

- Rule 1 (readers NULL-not-0 on unknowable): no product SQL divides by
  sample_interval_seconds except through NULLIF(…, 0); every derived
  interval_seconds alias routes the stored value through NULLIF (two
  PgBaselineProvider arms are the named allowance — they read a #3698 successor
  aggregate whose WHERE already dropped the interval-0 collection, and the census
  asserts that predicate on the named source); every division by the alias sits
  under a positive guard; every rate arm that falls back to ELSE 0 is rostered
  shrink-only (four file-I/O trend columns on both SKUs, nine PostgreSQL trend
  rates, two baseline wait arms — each dead text under its own WHERE).
- Rule 2 (rates divide by measured elapsed): zero divisions of a delta by an
  assumed cadence, SQL and C#, both SKUs — and it stays zero.
- Rule 3 (epoch → ClearServer + marker), as a call-site census over #3694's
  ServerEpoch: exactly the two carriers observe; every forget in the product is
  the comparator's or a host remove path's (exact set, both directions); both
  hosts drain the discontinuity account and compose the measurement onto the
  collection_log note; forget precedes measure precedes persist. Residuals
  rostered: the five families scheduled before the carrier (read off both hosts'
  schedule order), and pg_wait_stats, which has no carrier.
- Rule 5 (rollups aggregate interval > 0 only), in the Darling half against the
  real TimescaleSupport constants: the two #3698 successors carry the predicate,
  query_stats_baseline carries the equivalent delta_execution_count > 0, and the
  roster names what does not — the superseded legacy pair and the three hourly
  rollups over query_stats / procedure_stats, whose min() and sample_count read a
  restart's (0, 0) row as a quiet sample.
- Rule 6 (per-second names divided at comparison): every *_per_sec(ond) alias in
  product SQL is a quotient or a window over one; no collector payload column
  carries the name.
- Rule 8 (gauges never delta'd) is not census-able until cntr_type is stored;
  the perfmon payload's lack of that column is pinned so the rung reds this row.

Two files because the lite CI filter does not reach Darling.Storage or
Darling.Analysis and CrossAppGuardCiGateTests fails a Lite guard PR CI cannot
run: the Darling half sweeps those two trees with the same regexes and holds the
continuous-aggregate census. Test-only; no product code. Every fact executed
here (Lite: WPF-hosted harness, 10/10; Darling: mactest harness, 7/7; the whole
Darling.Tests assembly runs with the same 188 platform reds as dev) and
mutation-checked: twelve planted defects each turned exactly the one fact that
guards them red.
erikdarlingdata added a commit that referenced this pull request Sep 19, 2026
…red once, and censuses for rules 1, 2, 3, 5 and 6 (partial #3653) (#3701)

#3540 stated the contract in one sentence and its review numbered ten rules; two
numbers reached the tree (rule 4 — every family persists its interval; rule 7 —
every family seeded) and the other eight stayed a sentence in an issue. This
writes the list once, in Lite.Tests/MeasurementContractCensusTests' summary, with
each rule's bound and status: 4 and 7 keep the numbers their tests gave them, the
six remaining named rules take the free slots in the body's order, and slots 9
and 10 are left empty rather than invented — the review's enumeration is not on
the record.

Pinned tonight, over source and schema, each with a positive control and a
population floor so an empty offender list cannot be a dead regex:

- Rule 1 (readers NULL-not-0 on unknowable): no product SQL divides by
  sample_interval_seconds except through NULLIF(…, 0); every derived
  interval_seconds alias routes the stored value through NULLIF (two
  PgBaselineProvider arms are the named allowance — they read a #3698 successor
  aggregate whose WHERE already dropped the interval-0 collection, and the census
  asserts that predicate on the named source); every division by the alias sits
  under a positive guard; every rate arm that falls back to ELSE 0 is rostered
  shrink-only (four file-I/O trend columns on both SKUs, nine PostgreSQL trend
  rates, two baseline wait arms — each dead text under its own WHERE).
- Rule 2 (rates divide by measured elapsed): zero divisions of a delta by an
  assumed cadence, SQL and C#, both SKUs — and it stays zero.
- Rule 3 (epoch → ClearServer + marker), as a call-site census over #3694's
  ServerEpoch: exactly the two carriers observe; every forget in the product is
  the comparator's or a host remove path's (exact set, both directions); both
  hosts drain the discontinuity account and compose the measurement onto the
  collection_log note; forget precedes measure precedes persist. Residuals
  rostered: the five families scheduled before the carrier (read off both hosts'
  schedule order), and pg_wait_stats, which has no carrier.
- Rule 5 (rollups aggregate interval > 0 only), in the Darling half against the
  real TimescaleSupport constants: the two #3698 successors carry the predicate,
  query_stats_baseline carries the equivalent delta_execution_count > 0, and the
  roster names what does not — the superseded legacy pair and the three hourly
  rollups over query_stats / procedure_stats, whose min() and sample_count read a
  restart's (0, 0) row as a quiet sample.
- Rule 6 (per-second names divided at comparison): every *_per_sec(ond) alias in
  product SQL is a quotient or a window over one; no collector payload column
  carries the name.
- Rule 8 (gauges never delta'd) is not census-able until cntr_type is stored;
  the perfmon payload's lack of that column is pinned so the rung reds this row.

Two files because the lite CI filter does not reach Darling.Storage or
Darling.Analysis and CrossAppGuardCiGateTests fails a Lite guard PR CI cannot
run: the Darling half sweeps those two trees with the same regexes and holds the
continuous-aggregate census. Test-only; no product code. Every fact executed
here (Lite: WPF-hosted harness, 10/10; Darling: mactest harness, 7/7; the whole
Darling.Tests assembly runs with the same 188 platform reds as dev) and
mutation-checked: twelve planted defects each turned exactly the one fact that
guards them red.
erikdarlingdata added a commit that referenced this pull request Sep 19, 2026
…le, Opus full tier (#3710) (#3718)

* REVIEW-TRAPS.md: the repository's recurring defect shapes, one line each with the PR that bit (#3710)

Seven layers (SQL, collectors and deltas, analysis, alerting and notifications,
storage and MCP payloads, tests and censuses, workflows and CI, repository
mechanics), fifty-odd lines, every one citing the pull request or issue that
verified it: the >= limit truncation inference (#3594/#3679/#3699), ORDER BY
3 + 4 folding to a constant (#3613), MAX(max_dop) across plans (#3648), integer
division in cntr_value_per_second (#3653 Q9), per-collection alert firing
against a shorter cooldown (#3628/#3636), restart zeros in deltas and baselines
(#3595/#3698/#3705/#3708), ScoreAll skipping amplifiers on a zero base and the
non-virtual GetActiveEdges (#3688), the PgTargetFactKeys censuses (#3688),
pg_total_relation_size vs the heap and the two TimescaleDB views that hide
materializations and history (#3610/#3585), the -infinity crash backoff (#3629),
the Slack block budget and the surrogate-pair cut (#3612/#3618/#3625/#3644),
LCK_M_SCH_M folding into LCK so no SCH_M fact exists (#3709), one wait profile
per run (#3709), and the CRLF round-trip. Each citation was read before it was
written down; a wrong citation is worse than none.

The review workflow reads this file before the PR body and must say, per trap
the diff is near, whether it applies here and why.

* Claude review: blind read before the body, callers rule, structured verdict with a ledger, and an Opus/Sonnet tier by changed path (#3710)

The prompt runs in two phases. Phase 1 forbids gh pr view / git log / git show
until the reviewer has read the diff, every changed region in context, the
callers of every changed public symbol (Grep, both SKUs and both test projects)
and .github/REVIEW-TRAPS.md, and written its own account. Phase 2 reads the
body and grades every claim VERIFIED / UNVERIFIED / REFUTED, then reports the
DIVERGENCE between the two accounts. The verdict is a fixed block -- Claims,
Riskiest lines with the pinning test or UNPINNED, Traps near this diff,
Divergence, Reviewer checklist -- ending in one machine-readable ledger line,
posted through gh pr review --body-file - with a quoted heredoc (no Write tool;
the allowlist is unchanged and still read-only).

The #3650 verdict check now fetches the newest verdict in the window by id and
fails the job when the body lacks the "## Verdict:" header or the ledger, or
when the ledger's verdict disagrees with the review's API state (a
"CHANGES REQUESTED" body submitted with --comment is the shape the guard would
wave through). The guard's own arm is untouched and still keys on the state.

A dorny/paths-filter@v4 step (the repo's pin style) routes changes under the
shared libraries, Darling Storage/Analysis/Service, Lite Services/Mcp/Database,
install/ and every .sql file to the full tier -- claude-opus-5[1m], 60 turns --
and everything else to claude-sonnet-5 at 30 turns; one decision step emits
tier/model/max_turns, one review step consumes them (two steps would double the
"review posted" accounting the guard does), and the prompt carries the tier and
model into the ledger. A filter that dies degrades to FULL, not light.

Step name "Claude review" and the ALWAYS POST sentinel are unchanged: both are
read by claude-review-guard.yml.

* Review tier: Alerting and Notifications read at the full tier (#3710)
erikdarlingdata added a commit that referenced this pull request Sep 19, 2026
…ple: interval-honest successors (sample_interval_seconds IS DISTINCT FROM 0 baked in, the measured interval summed) join the registry beside the legacy trio, every hourly-tier reader takes the successor where it reaches as far as the legacy, and the phase grid re-derives itself by its own method (partial #3653, item 16 / Q12)

query_stats_hourly, procedure_stats_hourly and query_stats_db_hourly aggregate the (0, 0) row a
restart writes: count(*) AS sample_count counts it and min(delta_*) reads it as a real floor. A
continuous aggregate cannot be altered, so the fix is #3698's shape - query_stats_interval_hourly,
procedure_stats_interval_hourly, query_stats_db_interval_hourly, identical dimensions and columns,
the collector's verdict in the WHERE, sum(sample_interval_seconds) carried, WITH NO DATA, on the
--backfill-rollups plan through RollupViews.

Where this departs from #3698, and why: the legacy trio STAYS registered. Each has an indefinite
daily tier hierarchical from it (a CAGG's source is fixed at CREATE), so it cannot be dropped
without cascading the daily nor frozen without stopping it. The successors are therefore APPENDED
to HourlyAggregates and the grid re-derives by the documented method: 16 policies, 15 light,
heaviest at :18, window 18 min = 1,080 s, watch line 900 s - 4 s above the 896 s ceiling, the
"18 whole minutes" the ceiling essay names as the floor. The compression band's minute (:35) and
minutes (:36-:59) do not move; the daily aggregate compression band is now FULL at 23 of 23 hours.
Two consequences are pinned rather than found: the 900 s line coincides with #2136's cadence knob,
and the 952 s clean run RefreshCeilingStalenessTests quotes now classifies ApproachingSlot.

Readers: one supply rule - TimescaleSupport.PrefersSuccessor, the rule PgBaselineProvider applied
per server, now shared - applied to a store's floors by RollupCoverage.HourlyRelationFor AFTER the
tier is decided over the legacy pair. Compose, the MCP duration trends and query history, the
daily summary (calendar + get_daily_health) and the FinOps workload readers all route through it;
an absent successor is never named. LogSupersededHourlyRollupCoverageAsync reports the hand-over
on every start. The raw purge is not gated on the successors (the #1661 precedent), stated.

Census: rule 5's roster keeps its five names - the legacy text still exists - and gains the
invariant that every member is superseded by a registered successor carrying the predicate.

Measured on PG 18.4 / TimescaleDB 2.28.1: legacy 4 samples / floor 0, successor 3 / floor 1,000 /
600 s of interval, sums equal; the store's own probe routes by coverage on both store shapes.
erikdarlingdata added a commit that referenced this pull request Sep 19, 2026
…der a floor that said covered: at service start, every continuous aggregate is scanned for bucket ranges its source holds rows for and it never materialized, and each is closed with one refresh over exactly its bounds - not a wider start_offset (partial #3653, item 9 / Q10)

The hole, read honestly at #3698: a refresh policy re-materializes [now - start_offset, now -
end_offset] and nothing else, so when the service is down longer than the hourly window the first
refresh after resume opens past the raw collected between the last pre-outage refresh's window end
and the moment collection stopped - about two hours - and that tail is never materialized. A
materialized_only rollup reads it EMPTY under a floor that says covered; a real-time baseline
aggregate serves it from neither branch. The floor-only probes (startup backfill, --backfill-rollups)
cannot see a hole above the floor. Ruled Q10: a targeted refresh at start, because a wider
start_offset taxes every hourly refresh forever for a shape that happens once per outage.

RepairMaterializationHolesAsync (a new partial, TimescaleSupport.MaterializationHoles.cs): for every
registered aggregate in RollupBackfill.Targets order (raw-sourced before hierarchical, so a daily's
scan sees its hourly's repair) plus the baselines, read the materialized span off the materialization
hypertable, scan from max(floor, source horizon) to the last materialized bucket with two correlated
EXISTS probes per bucket - the materialization by bucket, the source by time range WITH THE
AGGREGATE'S OWN WHERE, so an hour the aggregate rejects (a restart hour on an interval-honest
successor) is not a hole - merge contiguous buckets, cap at one refresh policy window per aggregate
per start (24 hourly / 3 daily) oldest first, and refresh each range. Plain refresh first, forced
only on a measured remainder: the first cut went straight to force on the reasoning that the tail's
rows were never logged as invalidations, and the rig proved that FALSE on 2.28.1 - the engine records
the region a refresh skips past as invalid, and the plain refresh closes the outage tail. The forced
path is kept for the hole with no invalidation behind it (materialization rows lost, a refresh cut
short), which the rig also plants and which the plain refresh demonstrably leaves standing. One
INFORMATION line per hole with bounds, buckets, path and duration; deferred ranges named; failure
isolated per aggregate. Launched by the worker right after the ensure, not awaited (the baseline
backfill's reason), drained at shutdown.

Measured on PG 18.4 / TimescaleDB 2.28.1: the outage sequence planted step for step, the tail read
empty with raw holding it, the scan found exactly [H3, H5) and not the outage hours, one plain
refresh into a COMPRESSED materialization chunk closed it, a second pass found nothing; H1 deleted
from the materialization survived a plain refresh and closed on the forced one, the pass saying so.
erikdarlingdata added a commit that referenced this pull request Sep 19, 2026
…24 h outage left a two-hour hole under a floor that said covered: interval-honest successors beside the legacy trio with the phase grid re-derived by its own method, and a targeted refresh of each materialization hole at service start (partial #3653 — items 16 + 9, Q12 + Q10) (#3731)

* Three hourly rollups counted every restart's fabricated zero as a sample: interval-honest successors (sample_interval_seconds IS DISTINCT FROM 0 baked in, the measured interval summed) join the registry beside the legacy trio, every hourly-tier reader takes the successor where it reaches as far as the legacy, and the phase grid re-derives itself by its own method (partial #3653, item 16 / Q12)

query_stats_hourly, procedure_stats_hourly and query_stats_db_hourly aggregate the (0, 0) row a
restart writes: count(*) AS sample_count counts it and min(delta_*) reads it as a real floor. A
continuous aggregate cannot be altered, so the fix is #3698's shape - query_stats_interval_hourly,
procedure_stats_interval_hourly, query_stats_db_interval_hourly, identical dimensions and columns,
the collector's verdict in the WHERE, sum(sample_interval_seconds) carried, WITH NO DATA, on the
--backfill-rollups plan through RollupViews.

Where this departs from #3698, and why: the legacy trio STAYS registered. Each has an indefinite
daily tier hierarchical from it (a CAGG's source is fixed at CREATE), so it cannot be dropped
without cascading the daily nor frozen without stopping it. The successors are therefore APPENDED
to HourlyAggregates and the grid re-derives by the documented method: 16 policies, 15 light,
heaviest at :18, window 18 min = 1,080 s, watch line 900 s - 4 s above the 896 s ceiling, the
"18 whole minutes" the ceiling essay names as the floor. The compression band's minute (:35) and
minutes (:36-:59) do not move; the daily aggregate compression band is now FULL at 23 of 23 hours.
Two consequences are pinned rather than found: the 900 s line coincides with #2136's cadence knob,
and the 952 s clean run RefreshCeilingStalenessTests quotes now classifies ApproachingSlot.

Readers: one supply rule - TimescaleSupport.PrefersSuccessor, the rule PgBaselineProvider applied
per server, now shared - applied to a store's floors by RollupCoverage.HourlyRelationFor AFTER the
tier is decided over the legacy pair. Compose, the MCP duration trends and query history, the
daily summary (calendar + get_daily_health) and the FinOps workload readers all route through it;
an absent successor is never named. LogSupersededHourlyRollupCoverageAsync reports the hand-over
on every start. The raw purge is not gated on the successors (the #1661 precedent), stated.

Census: rule 5's roster keeps its five names - the legacy text still exists - and gains the
invariant that every member is superseded by a registered successor carrying the predicate.

Measured on PG 18.4 / TimescaleDB 2.28.1: legacy 4 samples / floor 0, successor 3 / floor 1,000 /
600 s of interval, sums equal; the store's own probe routes by coverage on both store shapes.

* A service down longer than the refresh window left a two-hour hole under a floor that said covered: at service start, every continuous aggregate is scanned for bucket ranges its source holds rows for and it never materialized, and each is closed with one refresh over exactly its bounds - not a wider start_offset (partial #3653, item 9 / Q10)

The hole, read honestly at #3698: a refresh policy re-materializes [now - start_offset, now -
end_offset] and nothing else, so when the service is down longer than the hourly window the first
refresh after resume opens past the raw collected between the last pre-outage refresh's window end
and the moment collection stopped - about two hours - and that tail is never materialized. A
materialized_only rollup reads it EMPTY under a floor that says covered; a real-time baseline
aggregate serves it from neither branch. The floor-only probes (startup backfill, --backfill-rollups)
cannot see a hole above the floor. Ruled Q10: a targeted refresh at start, because a wider
start_offset taxes every hourly refresh forever for a shape that happens once per outage.

RepairMaterializationHolesAsync (a new partial, TimescaleSupport.MaterializationHoles.cs): for every
registered aggregate in RollupBackfill.Targets order (raw-sourced before hierarchical, so a daily's
scan sees its hourly's repair) plus the baselines, read the materialized span off the materialization
hypertable, scan from max(floor, source horizon) to the last materialized bucket with two correlated
EXISTS probes per bucket - the materialization by bucket, the source by time range WITH THE
AGGREGATE'S OWN WHERE, so an hour the aggregate rejects (a restart hour on an interval-honest
successor) is not a hole - merge contiguous buckets, cap at one refresh policy window per aggregate
per start (24 hourly / 3 daily) oldest first, and refresh each range. Plain refresh first, forced
only on a measured remainder: the first cut went straight to force on the reasoning that the tail's
rows were never logged as invalidations, and the rig proved that FALSE on 2.28.1 - the engine records
the region a refresh skips past as invalid, and the plain refresh closes the outage tail. The forced
path is kept for the hole with no invalidation behind it (materialization rows lost, a refresh cut
short), which the rig also plants and which the plain refresh demonstrably leaves standing. One
INFORMATION line per hole with bounds, buckets, path and duration; deferred ranges named; failure
isolated per aggregate. Launched by the worker right after the ensure, not awaited (the baseline
backfill's reason), drained at shutdown.

Measured on PG 18.4 / TimescaleDB 2.28.1: the outage sequence planted step for step, the tail read
empty with raw holding it, the scan found exactly [H3, H5) and not the outage hours, one plain
refresh into a COMPRESSED materialization chunk closed it, a second pass found nothing; H1 deleted
from the materialization survived a plain refresh and closed on the forced one, the pass saying so.
erikdarlingdata added a commit that referenced this pull request Sep 22, 2026
…inuous aggregate instead of rereading a week of raw collection_log on every call (#3893) (#3911)

* The fleet collection-health rollup gets an hourly continuous aggregate, created off the minute grid (#3893 arm 2, stage 1)

The seven-day fleet collection-health read decompresses six of its seven days of collection_log on every
call. collect.collection_health_hourly materializes the read's eleven aggregates per (server, collector,
hour). Each one re-aggregates losslessly (COUNT by SUM, SUM by SUM, MAX by MAX), so a later stage can serve
the read from buckets exactly. This stage only builds the object; nothing reads it yet.

It rides the one creation path every aggregate uses, EnsureContinuousAggregatesAsync, and takes no
migration rung, because a cagg CREATE cannot run inside a migration transaction. It is on NONE of the three
registry lists. A HourlyAggregates or BaselineAggregates member would push RefreshSlotWarningSeconds below
the 896 s ceiling. Any membership would also be a 24th compression target in a band that is full at 23. A
new OffGridAggregates list pairs it with its own policy builder, and the sweep's tier flag became a
per-aggregate policy function so it could.

The policy is finish-to-start with no initial_start (the daily tier's precedent): start_offset 3 hours,
end_offset = schedule_interval = 1 hour. The worst-case real-time tail is 1+1+1+1 = 4 h against a one-day
compress_after, and a pin fails if a cadence change lets it reach a quarter of that. The refresh window sits
in uncompressed rows too. The start minute drifts, and the doc says that is acceptable only because the
refresh measures cheap. Retention is an explicit 8-day leaf entry: 7 d + a bucket + the 3 h start span,
rounded up. Grants follow precedent: GRANT SELECT ON ALL TABLES IN SCHEMA collect TO mcp, re-run every start.

Four census pins moved deliberately:
- the ensure-count sum
- the retention relation list (21)
- the leaf walk, whose consumer for this one is the 7-day read, not the 30-day baseline
- the chunk-width change count, scoped to compression targets, which this is not

TimescaleSupportTests (69, incl. the ceiling), TimescaleAggregateCompressionTests (10, 23 targets unchanged),
TimescaleContinuousAggregateTests (40), BaselineSupplyTests, IntervalHonestHourlyRollupTests and the new
CollectionHealthAggregateTests all pass live on TimescaleDB 2.28.1.

* The collection-health aggregate's refresh window reaches back over the whole consumer window, so no run can jump the watermark past an unrefreshed hour (#3893 arm 2, the watermark hole)

The first design refreshed [now-3h, now-1h]. Its first run after install stranded every older hour below the watermark, and any outage past two hours stranded its gap - permanently, since materialized_only = false serves raw only above the watermark. start_offset is now 8 days (7-day consumer + a bucket + margin); steady state stays milliseconds because a refresh re-materializes only invalidated ranges. Two live tests reproduce both holes through the ensure path and a policy run (failing on the narrow policy, passing now), and a pin holds start_offset >= consumer window + a bucket.

* The fleet collection-health read is served from the hourly aggregate plus a raw head slice when the aggregate exists and has no hole, else from the raw scan (#3893 arm 2, the composed reader)

Composed: whole buckets from the first hour boundary at/after the window start UNION ALL FleetCollectionHealthSql restricted to the head hour (derived by one inserted predicate, not restated), re-aggregated COUNT->SUM, SUM->SUM, MAX->MAX, thirteen ordinals unchanged. Guards: to_regclass probe (absent -> raw), then the watermark from the catalog and a GLOBAL distinct-bucket count over [head end, watermark) bounded by the watermark as a parameter (17-24 ms on the fleet rig; any missing hour -> raw). The memo test's CreateCommand anchor moves deliberately.

* The composed collection-health read is proven equal to the raw scan across every CASE arm, and both guards and the late-row staleness are live tests (#3893 arm 2, the live proof)

* The collection-health aggregate's eleven expressions are pinned to the raw read's, and every predicate it freezes says a change means a rebuild (#3893 arm 2, pins and comments)

- Divergence pin CaggCreate_AndRawRead_CarryTheSameElevenAggregates: comments stripped, whitespace
  collapsed, the CREATE's and FleetCollectionHealthSql's eleven aggregates must match in text and order,
  and the head slice must carry exactly one inserted bound. Checked to fail on a one-arm perturbation.
- Frozen-predicate paragraphs beside AbandonedStatus, WholeCycleBudgetNoteSqlPattern,
  AbandonedByNotePredicateSql, AbandonedRunPredicateSql, NamedSkipStatusSqlList and its three statuses:
  changing any of them requires the aggregate to be dropped and rebuilt (#3698's known price).
- The retention summary line now names CollectionHealthRetentionInterval (PayloadDimensionTests #1958).
- OpenStoreAsync enables TimescaleDB through LiveTimescaleProbe (TimescaleEnableConnectionSafetyTests #1922).
- Source check: the only collection_log INSERT in Darling is DarlingObservability.InsertCollectionLogSql
  (writers :271, :403, :520), each stamping DateTime.UtcNow; no UPDATE anywhere; nothing backdates.
- Lite builds clean. The scratch rig file is deleted (never committed).

Full Darling suite on macOS (triage so far): 12,322 tests, 194 failed. Two were this branch's and are
fixed here (the two pins above). The rest are environmental: WPF assemblies missing, DPAPI and Windows
path conventions, SqlClient's ConfigurationManager, a Lite wpftmp csproj race. Not re-run in full after
the fix; the six touched classes pass, 139/139.

* The collection-health aggregate's materialization chunks at one raw chunk from its first refresh, so its 8-day retention holds 8 days, not up to 18 (#3893 arm 2, #3620's over-hold)

TimescaleDB gives a first-level aggregate's materialization ten raw chunks (10 days here), and drop_chunks removes a chunk only once its whole range is past the horizon, so an 8-day retention on 10-day chunks holds 8 to 18 days. That is the over-hold #3620 removed from every registered aggregate. The off-grid aggregate was left out of it, because #3620's width ensure walks the compression targets only.

Adding it there would not have been early enough. #3620's ensure runs in the aggregate-compression step (TimescaleAfterRepairLaunch), a later convergence stage than the creation sweep (Timescale). set_chunk_time_interval governs only chunks created after the call. The policy carries no initial_start, and its first run is the seven-day backfill, which creates the first chunks. So the creation sweep now sets the width itself, between the CREATE (WITH NO DATA, so no chunks exist yet) and the policy. The statement is catalog-gated, so a settled start calls nothing and a start after a failed attempt converges.

Live, through the product path: the width reads 86400 s after the ensure; after a first policy run over 9 days, every materialization chunk spans one day; a second ensure changes nothing. Negative control, with the width call disabled: Expected 86400, Actual 864000. CollectionHealthAggregateTests 13/13, TimescaleAggregateCompressionTests 10/10, TimescaleSupportTests 69/69, plus TimescaleContinuousAggregate, IntervalHonestHourlyRollup, FleetCollectionHealthMemo, RetentionChunkGeometryLive, CompressionClearanceWatch, PayloadDimension, RefreshCeilingProvenancePin, RetentionReevaluation, StoreObjectConvergence and TimescaleEnableConnectionSafety all green.

* The collection-health tests hold up on CI's store shape: the class records why it is not serialized, the compression test converts collection_log as the worker does, and that order is pinned (#3893 arm 2)

Two CI failures on #3911, both in the tests, neither in the product.

LivePostgresCollectionHygieneTests flagged CollectionHealthAggregateTests. The class reaches DARLING_TEST_PG only through ScratchPostgres, so it is own-store, and it now carries the #1776 own-store note saying so.

TimescaleAggregateCompressionTests' end-to-end test counted 23 created aggregates instead of 24. It builds the aggregates without converting collection_log. On a store whose migrations run before CREATE EXTENSION (CI's bundled PostgreSQL), collection_log is still a plain table at that point, so the off-grid aggregate's CREATE over it fails. That was reproduced locally on a template0 database: Expected 24, Actual 23. The local runs had passed only because the timescaledb image puts the extension in template1, so V23 converted collection_log during migration. The test now calls EnsureCollectionLogHypertableAsync, as the worker's "collection_log hypertable" step does before its aggregate ensure. It passes on the template0 shape.

The product's order was already right. It is now pinned in StoreObjectConvergenceTests.TheListOrder_KeepsTheFourMeasuredOrderingConstraints: collection_log becomes a hypertable before the aggregate ensure. The other order would fail the aggregate's CREATE on every fresh store's first start.

* The two remaining live fixtures that build the whole aggregate registry convert collection_log first, as the worker does (#3893 arm 2)

CI's second run on #3911 failed IntervalHonestHourlyRollupLiveTests (24 aggregates expected, 23 ready) and RetentionReevaluationLiveTests (21 retention policies expected, 20; the aggregate's CREATE raised 0A000 "invalid continuous aggregate view"). The cause is the one 2b2e634 fixed in TimescaleAggregateCompressionTests. The fixture migrates, enables TimescaleDB, and ensures the whole registry, but skips the worker's "collection_log hypertable" step. On a store whose migrations ran before CREATE EXTENSION, collection_log is still a plain table, and the off-grid aggregate cannot be created over it.

Local runs had missed it because the timescaledb image installs the extension into template1, so ScratchPostgres databases came with it and V23 converted collection_log during migration. Reproduced by dropping the extension from the container's template1, which is CI's shape. In that shape, every fixture that calls EnsureContinuousAggregatesAsync was run against its own database, and exactly these two failed. Both pass after this change.
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