Skip to content

issue-3653 A6 lane LA: stitched rollup reads, every hourly reader routes legacy→successor at the successor floor - #4182

Merged
erikdarlingdata merged 15 commits into
devfrom
fix/3653-a6-stitched-reads
Sep 24, 2026
Merged

erikdarlingdata merged 15 commits into
devfrom
fix/3653-a6-stitched-reads

Conversation

@erikdarlingdata

@erikdarlingdata erikdarlingdata commented Sep 24, 2026 •

Copy link
Copy Markdown
Owner

Part of #3653 (A6, lane LA)

What this adds

RollupCoverage.StitchedRelationSql(legacy, alias, windowStartUtc, tier) (design-3653-A6-v2.md §2), on TimescaleSupport.cs next to HourlyRelationFor. It returns one of three FROM-clause items:

  • collect.<legacy> AS alias — no successor (an unsuperseded pair, or the successor absent from availability, or the legacy is absent/empty and the successor is also absent/empty).
  • collect.<successor> AS alias — no legacy, the legacy is empty, or the successor's floor already reaches at or before windowStartUtc.
  • (SELECT <cols> FROM collect.<legacy> WHERE bucket < F UNION ALL SELECT <cols> FROM collect.<successor> WHERE bucket >= F) AS alias — otherwise, F a UTC TIMESTAMP literal.

For the daily tier, F_d = max(FloorOf(successorDaily), ceilDay(FloorOf(successorHourly))), keeping the successor hourly's partial first day on the legacy side of the split, as the design requires.

Each pair (hourly and daily, all three families) has its own explicit column list — the legacy's own columns, with no sample_interval_seconds_sum (no reader selects it). LA does not own the daily registry: I added a private static stub list of (LegacyDaily, SuccessorDaily, SuccessorHourly) marked // LB replaces with SupersededDailyRollups. Until LB lands, RollupAvailability has no field for the successor dailies, so on every store today the daily stitch is legacy-only by construction — inert, as the brief specifies.

Pure tests in Darling.Tests/StitchedRelationSqlTests.cs (11 cases, all green): legacy-only byte-identical text, successor absent from availability, legacy empty, successor empty, successor fully covering the window, a partial stitch with its column list and no sample_interval_seconds_sum, a stale-but-safe F (no gap/overlap), every hourly pair in SupersededHourlyRollups, the daily-inert case, and the daily ceiling-of-day boundary (mid-day successor-hourly floor → boundary is the start of the next day, not the partial bucket). Also reran IntervalHonestHourlyRollupTests and RollupCoverageRoutingTests (47 tests) — unaffected, all green.

What is NOT done in this PR — I ran out of context budget on step 1

The brief's steps 2–5 (routing every hourly- and daily-tier reader listed in design-3653-A6-v2.md §1 through the builder — ComposeSourceRouter.cs, DarlingTrendReader.cs, DailySummarySql.cs's two-sided probe, FinOps.Workload.cs, DurationTrendRouting.cs, QueryTrends.cs, ViewerDataService.DailySummary.cs, DarlingHealthReader.cs, DarlingMcpTrendTools.cs), the live-PG proof (seed both sides, check the per-bucket totals split exactly at F with no gap/overlap), and the RollupCoverageRoutingTests Resolve-call scan are not implemented. I read every named call site (§1) and located the relevant fields (ComposeRoute.CaggRelation, DurationTrendRoute.HourlyView, DailySummarySql.QueriesCteForCagg's target lookup) before running out of budget on the builder itself, which is why this PR does not build on top of a broken foundation — but it means the design's read-side promise ("stitch both tiers so the freeze can ship now") is not yet delivered end to end. The successor's builder is proven correct in isolation only.

What I did NOT verify

  • No live PG rig was started for this PR (no timescale/timescaledb container was run).
  • The Resolve-call scan (RollupCoverageRoutingTests.EveryProductionRoutingCaller_PassesCoverage) was not touched or extended to cover the new builder's call sites, because there are none yet.
  • Full Darling suite (DARLING_TEST_PG unset) was not run — only the new test class plus the two named-in-brief classes.

CHANGELOG entry

Added RollupCoverage.StitchedRelationSql, a SQL builder that stitches a frozen legacy rollup to its interval-honest successor at the successor's first bucket (both the hourly and daily tiers). Not yet wired into any reader.

LA-3a

Routed 3 more hourly-tier readers through RollupCoverage.StitchedRelationSql, per decisions 1/2/5:

  1. Compose (ComposeSourceRouter.cs ~:325-343, ComposeRoute, ComposeCompiler's FROM site). ComposeRoute gained CaggFromClause (decision 2), computed alongside the existing CaggRelation (which stays the by-name answer for probes/logs). ResolveFamily sets CaggFromClause via coverage.StitchedRelationSql(hourlyView, "f", windowStartUtc, StitchTier.Hourly). ComposeCompiler.BuildFactRelation returns route.CaggFromClause when present (a complete, aliased relation already), and AppendFactBody skips its own " AS f" suffix in that case to avoid double-aliasing. With no successor the FROM clause is collect.<view> AS f, byte-identical to what the compiler produced before.
  2. QueryTrends (ViewerDataService.QueryTrends.cs, ReadRoutedDurationTrendAsync). Replaced coverage.HourlyRelationFor(...) with coverage.StitchedRelationSql(hourlyView, "f", startUtc, StitchTier.Hourly). When the stitch answers exactly collect.<legacy> AS f, the code still takes the pre-built hourlySql fast path (byte-equal to the MCP reader's pinned constant); otherwise it builds fresh SQL via DurationTrendRouting.BuildHourlyTrendSql(hourlyFromClause, withDatabaseFilter: true) — that builder just interpolates its argument into FROM {arg}, so a full FROM-clause item (bare-name or stitched UNION ALL) slots in without any other change.
  3. McpTrendTools (Mcp/DarlingMcpTrendTools.cs ~:387, GetQueryTrend). Replaced the HourlyRelationFor call feeding GetQueryHistoryAsync's hourlyRelation parameter with coverage.StitchedRelationSql(TimescaleSupport.QueryStatsHourlyView, "f", now.AddHours(-hours_back), StitchTier.Hourly). QueryHistoryHourlySqlFor also just does FROM {hourlyRelation}, so this is the same drop-in.

Scan test (decision 5). Added RollupCoverageRoutingTests.NoReaderOutsideTheBuilder_CallsHourlyRelationForDirectly: walks Darling/ (same FindRepoRoot/IsExcludedFromScan/StripComments helpers the existing EveryProductionRoutingCaller_PassesCoverage guard uses) for HourlyRelationFor( call sites outside TimescaleSupport.cs (the builder both HourlyRelationFor and StitchedRelationSql live in). Marked [Fact(Explicit = true)] per the brief: LA-3b's readers (DarlingTrendReader.cs's route + query-history read at ~:912/:1323, DailySummary.cs/DailySummarySql.cs, DarlingHealthReader.cs) still call HourlyRelationFor by name as of this lane's last push, so the guard would start red. Ran it with -explicit only: it correctly FAILS right now (offenders in those three files), confirming the scan itself works. LA-3b should flip it to a plain [Fact] once all ten readers route through the stitch.

Tests run per step (this Mac, DARLING_TEST_PG unset, Microsoft.WindowsDesktop.App stripped from Darling.Tests.runtimeconfig.json for each run then restored):

  • ComposeSourceRouterTests: 26/26 pass.
  • DarlingComposeTests: 280/280 pass.
  • ViewerTrendRoutingPortTests: 12/14 pass, 2 pre-existing failures (DescribeQueryStoreTrendCoverage_NamesTheRoute_AndTheHeadOnlyWhenUnserved, DescribeTrendCoverage_NamesTheTier_AndTheHeadOnlyWhenTruncated) — confirmed red on origin/fix/3653-a6-stitched-reads BEFORE my QueryTrends.cs edit too (stashed my change, rebuilt, same 2 failures), so not introduced by this lane.
  • IntervalHonestHourlyRollupTests: 8/8 pass (both before and after step 3).
  • DarlingPerformanceTrendsReadTests: 1 skipped (needs DARLING_TEST_PG), 0 failed.
  • RollupCoverageRoutingTests: 40/40 non-explicit pass; the new Explicit test correctly fails under -explicit only (LA-3b not landed yet), correctly not-run under the default -explicit off.

What is NOT done in this LA-3a slice

The live Compose proof (seeded stitched pair, one live Compose read, per-bucket totals split at F with no gap/overlap) was NOT completed — I hit the context-wall warning before reaching it and prioritized landing the routing + scan test, which are the harder-to-recreate parts, with everything green and pushed. IntervalHonestHourlyRollupLiveTests (same file) has the exact rig to copy: ScratchPostgres.CreateAsync + PgMigrations.MigrateAsync + TimescaleSupport.TryEnableAsync/ConvertToHypertablesAsync + seed collect.query_stats rows spanning two hours + EnsureContinuousAggregatesAsync + RollupBackfill.RunSliceAsync on both the legacy and successor hourly views + a real ComposeCompiler.Compile/execute over a plan whose window straddles the successor floor. A follow-up (or LA-3b, if it has budget left after flipping the scan test) should add this as a [Fact] in DarlingComposeTests.cs or a new ComposeStitchedRelationLiveTests.cs, asserting the legacy rows count below F and successor rows at/above F with none double-counted.

What I did NOT verify

  • The live proof above (not run).
  • The full Darling suite (214 base) was NOT re-run in this lane; LA-3b or the coordinator should run it once after both LA-3a and LA-3b land, since I'm out of budget.
  • Whether DarlingComposeTests line ~905 (Compile_OldWindow_QueryStore_...) tests I did not touch would need re-pinning if a stitch is ever seeded through them — they use RollupCoverage.Unknown/hand-built RollupCoverage, not StitchedRelationSql, and stayed green, so no.

@erikdarlingdata

Copy link
Copy Markdown
Owner Author

Lane A6-LA-2 status: 3 of 10 hourly readers routed; 7 remain unrouted

Routed through RollupCoverage.StitchedRelationSql this lane (commit 959b0f8, branch fix/3653-a6-stitched-reads, plain push, no PR opened per brief):

  • ViewerDataService.FinOps.Workload.cs: DatabaseResourceUsageSqlFor, TopResourceConsumersByTotalSqlFor, TopResourceConsumersByAvgSqlFor — each got a (RetentionTier, RollupCoverage, DateTime windowStartUtc) overload that calls coverage.StitchedRelationSql(<hourlyView>, "f", windowStartUtc, StitchTier.Hourly) in place of the old coverage.HourlyRelationFor(...) bare-name pass-through. The daily arm is untouched (no successor yet, per design). The (RetentionTier) one-argument overloads were kept as the unstitched form and now build their own collect.<view> AS f text directly (previously they delegated to the two-arg form with a bare relation name); this preserves their existing pins because the CTE's FROM clause text is unchanged in the no-successor case — only the alias AS f is new, which changes no result since neither CTE qualifies a column. Call sites in GetDatabaseResourceUsageAsync, GetTopResourceConsumersByTotalAsync, GetTopResourceConsumersByAvgAsync now pass (tier, coverage, cutoff).
  • Verified: dotnet build Darling/PerformanceMonitor.Darling.Viewer/PerformanceMonitor.Darling.Viewer.csproj -p:EnableWindowsTargeting=true — 0 errors, 0 warnings. scrubcheck on the diff: 0 hits.

NOT done — still unrouted, left on coverage.HourlyRelationFor (next lane must pick these up):

  • ComposeSourceRouter.cs:339 + the ComposeCompiler.BuildFactRelation FROM site (ComposeRoute.CaggRelation is a bare relation-name string today; routing it through the stitch needs either widening CaggRelation to carry a FROM-clause expression or adding a parallel field — a real design call, not mechanical)
  • ViewerDataService.QueryTrends.cs:364 (ReadRoutedDurationTrendAsync, calls DurationTrendRouting.BuildHourlyTrendSql)
  • DarlingTrendReader.cs:912 (ReadRoutedDurationTrendAsync, uses route.HourlyView into DurationTrendRouting.BuildBucketedHourlyTrendSql) and its query-history read at :1323 (QueryHistoryHourlySqlFor)
  • DarlingTrendReader.cs:1481's route.HourlyView probe — per the brief, this one must keep probing a REAL relation name (use the successor name when a stitch applies), not the stitched SQL text
  • ViewerDataService.DailySummary.cs:99 / DarlingHealthReader.cs:441 (both call DailySummarySql.RangeSqlFor(tier, coverage.HourlyRelationFor(...)); DailySummarySql.QueriesCteForCagg looks the relation up BY NAME in MaterializationHoleTargets for its not-carried probe, so stitching this one needs the two-probe treatment design §1(d) calls out, not a drop-in text swap)
  • DarlingMcpTrendTools.cs:387 (passes hourlyRelation: into DarlingTrendReader.GetQueryHistoryAsync, same builder as the :1323 site)

Also not done:

  • Step 2 (extend RollupCoverageRoutingTests' scan) — not started. Note for the successor: I found no existing "every hourly reader routes through HourlyRelationFor" scan; the closest analog is EveryProductionRoutingCaller_PassesCoverage (scans for RetentionTierRouter.Resolve( call sites), a DIFFERENT function. A new scan for HourlyRelationFor( call sites (expected 0 once all 10 route through the stitch) would need to be added fresh, keyed the same way (strip comments, walk from repo root, exclude Darling.Tests/bin/obj).
  • Step 3 (live PG proof: seed legacy+successor hourly, overlapping buckets, run a real reader, assert no gap/overlap at F) — not run.
  • Step 4 (targeted test classes + full suite) — not run.

Why stopped here: hit the 150k/20-minute context-wall checkpoints from the dispatch rules before the FinOps builder edits + build verification were even committed; per the rules I stopped investigating the remaining 7 readers (several need real design decisions — ComposeRoute.CaggRelation's shape, the DailySummarySql two-probe split) rather than mechanical swaps, and prioritized landing a clean, buildable, scrub-clean increment over an uncommitted half-edit across all 10.

CHANGELOG entry

@erikdarlingdata

Copy link
Copy Markdown
Owner Author

LA-3b (route TrendReader / DailySummary / HealthReader hourly readers through StitchedRelationSql)

Status: investigation only, no code changed. I ran out of budget (context wall hit before any edit) doing the required reading of RollupCoverage.StitchedRelationSql, HourlyRelationFor, and all three reader call sites, and did not want to land a rushed edit that could break the byte-identical-with-no-successor pins. Reporting findings here so a follow-up lane does not have to re-derive them.

What I found

1. DarlingTrendReader.cs. DurationTrendRoute.HourlyView (record at ~:850) is used in two different ways that decision 1 says must be kept separate:

  • as a name for the server probe at :1481 (SELECT 1 FROM {route.HourlyView} WHERE server_id = $1 LIMIT 1) and for user-facing messages in DarlingMcpTrendTools.cs (:766, ~:1020, ~:1060);
  • as the input to DurationTrendRouting.BuildBucketedHourlyTrendSql(route.HourlyView) at ~:1084, which splices it into FROM {hourlyView} unaliased (bare name, resolved through search_path = collect, config, public) — this is the FROM-clause site that needs the stitch.

DurationTrendRoute is built in ResolveDurationTrendRoute (~:902-916), currently: rawTable, coverage.HourlyRelationFor(hourlyView, startUtc), hourlyAvailable, tierCoverage, ... — a single bare-name field feeding both uses.

To follow decision 2's pattern, DurationTrendRoute needs a second field (HourlyFromClauseSql or similar) carrying coverage.StitchedRelationSql(hourlyView, "h", startUtc, RollupCoverage.StitchTier.Hourly), and BuildBucketedHourlyTrendSql needs a FROM-clause-accepting form (it currently does FROM {hourlyView} with no alias at all — introducing an alias to carry the stitch's AS h is itself a text change that has to be proven byte-identical modulo the alias when there's no successor, same as decision 2 requires for ComposeCompiler). QueryHistoryHourlySqlFor (:160) has the same bare-name-in-FROM shape and the same problem, feeding GetQueryHistoryAsync's hourlyRelation parameter (:1311, called from DarlingMcpTrendTools.cs~:380 with coverage.HourlyRelationFor(...)).

The probe at ~:1481 (HasAnySampleOnRouteAsync) should keep reading route.HourlyView as a name — decision 4 in the DECISIONS doc is specifically about this probe, and says: when the stitch applies, probe the SUCCESSOR name (not a by-name lookup of the legacy). That means HourlyView itself should become TimescaleSupport.SuccessorOf(legacy) ?? legacy when the stitch applies, rather than HourlyRelationFor's existing legacy-favoring answer — a distinct rule from HourlyRelationFor, per decision 4's own wording ("since the frozen legacy will empty over time").

2. DailySummarySql.cs / ViewerDataService.DailySummary.cs. QueriesCteForCagg (:337) does a single by-name probe via TimescaleSupport.MaterializationHoleTargets lookup keyed on the relation name; decision 3 requires running it TWICE (once per relation name, split at F by bucket < F / bucket >= F) and UNION ALL when a stitch applies, keeping today's single probe when there's no successor. RangeSqlFor(tier, hourlyRelation) (:409) and its two callers (DarlingHealthReader.cs:441, ViewerDataService.DailySummary.cs:99) currently pass coverage.HourlyRelationFor(...) (a bare name) straight through — this needs to become the stitched-aware routing decision 3 describes, which is NOT a plain StitchedRelationSql splice (the daily-summary CTE has a bespoke not-carried probe that must itself be split, not just the outer FROM).

3. DarlingHealthReader.cs (~:441) just calls DailySummarySql.RangeSqlFor with the hourly relation from HourlyRelationFor — once #2 is fixed at the DailySummarySql layer, this caller is mechanical (swap to whatever new signature #2 lands on), per the brief.

Recommendation for the next lane

Do (1) first — it is the closest to LA-2's already-landed pattern (a record gains a *FromClause field, one builder overload gains a FROM-clause parameter, existing byte-identical pins get an AS <alias> added and re-proven). Do (2)+(3) together since HealthReader is a thin wrapper over the DailySummarySql change. Budget at least a full lane for (2) alone — the not-carried probe's UNION ALL split is bespoke SQL, not a call to StitchedRelationSql.

No files were touched; nothing pushed. fix/3653-a6-stitched-reads is unchanged by this lane.

erikdarlingdata added a commit that referenced this pull request Sep 24, 2026
…yFromClause/ProbeHourlyView fields

Adds the two FROM-clause/probe-name fields decisions 1/2/4 require to
DurationTrendRoute and wires ResolveDurationTrendRoute to compute
HourlyFromClause via RollupCoverage.StitchedRelationSql. The splice
into BuildBucketedHourlyTrendSql/QueryHistoryHourlySqlFor and the
successor-aware probe wiring are NOT done yet -- ran out of budget
mid-step. See PR #4182 body section LA-3b1 for the handoff.
@erikdarlingdata

Copy link
Copy Markdown
Owner Author

LA-3b1

Status: partial, budget exhausted mid-step. Pushed 58c917e2b to fix/3653-a6-stitched-reads (branch la3b1).

What landed

DarlingTrendReader.DurationTrendRoute gains two optional fields per decisions 1/2/4:

  • HourlyFromClause — the RollupCoverage.StitchedRelationSql output for the route's window (alias h), computed in ResolveDurationTrendRoute alongside the existing by-name HourlyRelationFor call. HourlyView is untouched and still a bare name.
  • HourlyFromClauseOrDefault — falls back to collect.{HourlyView} AS h for a route built without resolving the stitch (record constructor used directly, as some existing tests do).
  • ProbeHourlyView / ProbeHourlyViewOrDefault — placeholders for decision 4's successor-probe rule (probe the successor name when it exists and its floor ≤ window end, else legacy). Not yet wired: ResolveDurationTrendRoute does not compute ProbeHourlyView yet, and HasAnySampleOnRouteAsync at ~:1481 still reads route.HourlyView directly.

What's NOT done (next lane must pick up here)

  1. Splice into the two SQL builders — DurationTrendRouting.BuildBucketedHourlyTrendSql and DarlingTrendReader.QueryHistoryHourlySqlFor still take a bare relation NAME and do FROM {hourlyView} unaliased. Neither builder was touched. They need a FROM-clause-accepting overload (or a switch to always take the from-clause and have callers pass collect.<name> AS h for the legacy-only pins), and the ReadRoutedDurationTrendAsync call site (:1084, DurationTrendRouting.BuildBucketedHourlyTrendSql(route.HourlyView)) and GetQueryHistoryAsync (:1323, QueryHistoryHourlySqlFor(hourlyRelation ?? ...)) need to switch to route.HourlyFromClauseOrDefault / the resolved from-clause instead of the bare name.
  2. Byte-identical pin with the added alias — once the builders take a from-clause, add a pure test asserting today's unaliased text plus AS h is exactly what the no-successor path produces (mirroring decision 2's pattern for ComposeCompiler).
  3. The probe (~:1481, HasAnySampleOnRouteAsync) — wire ResolveDurationTrendRoute to compute ProbeHourlyView = successor name when TimescaleSupport.SuccessorOf(hourlyView) is non-null AND coverage.FloorOf(successor) <= windowEnd (need to thread nowUtc/window end into the resolver, or reuse the already-available nowUtc parameter, which today is the wall clock, not necessarily the window end — check the DECISIONS wording again: "the successor exists AND its floor ≤ the window end", i.e. the query's own end, not nowUtc. ResolveDurationTrendRoute currently only receives startUtc, not an end; this needs threading the window end through ResolveQueryDurationTrendRoute/ResolveProcedureDurationTrendRoute, which is a signature change touching DarlingMcpTrendTools.cs call sites — outside the "one file" scope note, so flag it back to the coordinator if that's not acceptable).
  4. Live proof (a query-history read over a seeded stitched pair matching legacy below F / successor at-or-above F) — not attempted; no docker container was started.
  5. Full suite — not run; only a compile was attempted and did not finish before the time wall (the sandboxed timeout binary is missing on this Mac — use gtimeout or a background & + wait pattern instead).

Verification run

None completed. dotnet build was invoked but the command failed on tooling (timeout: command not found, not a build error) before returning output, and the time wall hit immediately after. The current diff has not been compiled or tested. scrubcheck on the changed file passed (0 hits, exit 0).

Why this landed half-built

Investigation (reading RollupCoverage.StitchedRelationSql, SuccessorOf, HourlyRelationFor, DurationTrendRoute's full call graph across DarlingTrendReader.cs, DarlingMcpTrendTools.cs, and the three test files that pin its shape) took the full budget before the record-and-resolver edit was even attempted; the two builder splices and the probe rewiring were not reached.

CHANGELOG entry

(none yet — no user-visible behavior change has landed; the next lane's PR should carry the entry once the splice is byte-identical-pinned and proven live.)

@erikdarlingdata

Copy link
Copy Markdown
Owner Author

LA-3b2

Status: code landed and builds clean; NOT tested (context wall hit before a test run). No test file added.

What changed (3 files, per brief)

  1. RollupCoverage.StitchFloor(legacy, tier, windowStartUtc) (new, in TimescaleSupport.cs) — answers the
    same boundary StitchedRelationSql would split at for this window, or null when that call would answer
    legacy-only or successor-only (no successor, absent/empty, or the successor's floor already covers the whole
    window). Deliberately kept as a parallel guard chain next to StitchedRelationSql rather than factored
    through it, so the two stay a visible diff of each other.

  2. DailySummarySql.QueriesCteForStitchedCagg(legacy, successor, successorFloor) (new, private) — the
    queries CTE from QueriesCteForCagg, but every member (rollup half, not-carried hole-scan half) runs
    TWICE: once against legacy restricted to bucket < F, once against successor restricted to
    bucket >= F, UNION ALL'd — per decision 3. Each side keeps its own ceiling (queries_ceiling_legacy /
    queries_ceiling_successor) and its own not-carried source probe (legacy's has no filter; successor's
    carries its restart-row exclusion, same as the existing non-stitched successor routing).

  3. DailySummarySql.RangeSqlFor(RetentionTier tier, RollupCoverage coverage, DateTime windowStartUtc)
    (new public overload) — the caller-facing entry point. Daily tier delegates unchanged. Hourly tier calls
    StitchFloor: null → delegates to the existing RangeSqlFor(tier, coverage.HourlyRelationFor(...))
    byte-identical; non-null → builds the stitched CTE via QueriesCteForStitchedCagg.

  4. ViewerDataService.DailySummary.cs and DarlingHealthReader.cs — both now call
    DailySummarySql.RangeSqlFor(tier, coverage, fromDate) (new overload) instead of resolving
    coverage.HourlyRelationFor(...) themselves and passing the bare name. ViewerDataService also gained a
    passthrough DailySummaryRangeSqlFor(tier, coverage, windowStartUtc) wrapper next to its existing two.

Verified

  • dotnet build clean (0 warnings/errors) on PerformanceMonitor.Darling.Storage, ...Service, and
    ...Viewer with -p:EnableWindowsTargeting=true.
  • scrubcheck on the diff: 0 hits, exit 0.

NOT verified (ran out of budget before this)

  • No unit test added. The brief's "pin it" requirement for the no-successor byte-identical case, and a
    stitched-shape test proving the UNION ALL split lands on the right side of F, are still open. The existing
    IntervalHonestHourlyRollupTests exercises the non-stitched two-argument RangeSqlFor(tier, hourlyRelation)
    path only — it does not touch the new three-argument overload at all.
  • *DailySummary* / *DarlingHealth* targeted test classes were not run.
  • The live proof (seeded stitched hourly pair on 2.30.1, per-day totals correct both sides of F, not-carried
    probe correct per side) was NOT run.
    No PG container was started.
  • Full suite was not run.

Recommendation for a follow-up

A next lane (or this lane's continuation) should:

  1. Add a test pinning RangeSqlFor(Hourly, coverage, start) byte-identical to
    RangeSqlFor(Hourly, coverage.HourlyRelationFor(legacy, start)) when StitchFloor is null (mirrors the
    IntervalHonestHourlyRollupTests pin pattern for the two-arg form).
  2. Add a shape test for the stitched case: build a RollupCoverage with both legacy and successor floors set,
    windowStartUtc before the successor floor, and assert the returned SQL contains both relation names, the
    bucket < TIMESTAMP ... / bucket >= TIMESTAMP ... split, and that the not-carried probe's NOT EXISTS
    member appears twice (once per side).
  3. Run the live proof over a 2.30.1 container with a seeded stitched pair straddling F, confirming per-day
    unique_queries and NULL-carried days are correct on both sides.

No files outside DailySummarySql.cs, TimescaleSupport.cs (for the RollupCoverage.StitchFloor helper,
called out in the brief as permitted), ViewerDataService.DailySummary.cs, and Mcp/DarlingHealthReader.cs
were touched. Nothing pushed beyond commit e708093a4 on fix/3653-a6-stitched-reads.

@erikdarlingdata

Copy link
Copy Markdown
Owner Author

Coordinator note check (compile-after-58c917e2b): verified — dotnet build Darling/PerformanceMonitor.Darling.Service/PerformanceMonitor.Darling.Service.csproj -p:EnableWindowsTargeting=true on top of 58c917e (my commit e708093 is rebased onto it) is clean, 0 warnings/0 errors. No compile fix was needed; LA-3b1's partial commit already compiles as-is.

@erikdarlingdata

Copy link
Copy Markdown
Owner Author

LA-4b

Status: steps 1–3 done and green. Step 4 (flip the scan test) and the full suite (step 5) NOT done — LA-4a had not pushed to fix/3653-a6-stitched-reads by the time this lane hit its context/time budget.

What changed

  1. DailySummaryStitchedRangeTests.cs (new, pure) — pins DailySummarySql.RangeSqlFor(RetentionTier, RollupCoverage, DateTime) (LA-3b2, commit e708093):

    • no successor → byte-identical to the two-argument form;
    • successor already covers the whole window → byte-identical to the two-argument form over the successor name;
    • a genuine stitched pair → the not-carried probe's NOT EXISTS member appears exactly twice, each side restricted to its side of F (bucket < F / bucket >= F), each with its own queries_ceiling_legacy / queries_ceiling_successor, and the split boundary literal matches what RollupCoverage.StitchedRelationSql would splice for the same coverage/window;
    • the Daily and Raw tiers ignore the stitch entirely, byte-identical either way.

    5 facts, all green.

  2. DailySummaryAndComposeStitchedLiveTests.cs (new, live, timescale/timescaledb:2.30.1-pg18) — ONE seeded stitched hourly pair (5 calendar days, F at day 2's 10:00 bucket — the successor's own first materialized bucket — legacy materialized over the whole span, successor only from F), read through BOTH callers:

    • Daily summary (DailySummarySql.RangeSqlFor(Hourly, coverage, d0)): all 5 days present, none missing, none duplicated across the F seam; days 0–1 (legacy side) count 2 distinct hashes (planted query + the restart row, which the legacy admits) and days 2–4 (successor side) count 1 (the successor's own source filter refuses the restart row) — the same interval-honest distinction IntervalHonestHourlyRollupLiveTests measures directly on the rollups.
    • Compose (one real ComposeCompiler.Compile over a query_worker_us line panel, through ComposeSourceRouter): the compiled FROM clause is the UNION ALL stitch itself (both relation names present); the 5 hourly totals are in the exact 1:2:3:4:5 ratio this test planted, proving no gap and no overlap at the F seam.

    1 test, 1 fact, green on a live container (port 55890, removed after the run).

NOT done (budget ran out)

  • Step 4 (flip RollupCoverageRoutingTests.NoReaderOutsideTheBuilder_CallsHourlyRelationForDirectly from [Fact(Explicit = true)] to [Fact]): LA-4a's DarlingTrendReader.cs splice had not landed on fix/3653-a6-stitched-reads as of this lane's last push (3c1929d97, on top of e708093a4). A follow-up lane should git pull --rebase, check git log --oneline for LA-4a's commit, then flip the attribute and run the class.
  • Step 5 (full suite + targeted live classes *RollupCoverage*, *IntervalHonest*, *Compose*, *DailySummary*, *FinOps*, *QueryTrend*, *McpTrend*, *DarlingHealth*): not run. Only the two new test classes above were run, both green.

Verified

  • dotnet build Darling/Darling.Tests -p:EnableWindowsTargeting=true: clean, 0 errors.
  • Darling.Tests.DailySummaryStitchedRangeTests: 5/5 pass (pure, no rig).
  • Darling.Tests.DailySummaryAndComposeStitchedLiveTests: 1/1 pass against timescale/timescaledb:2.30.1-pg18 on port 55890 (container removed).
  • scrubcheck on both new files: 0 hits, exit 0.

RED findings

None — both stitched callers (DailySummarySql.RangeSqlFor's three-arg overload, and Compose's hourly route through StitchedRelationSql) behaved exactly as decision 3/2 specify.

For the next lane

Pull, confirm LA-4a's DarlingTrendReader.cs commit is present, flip the scan test's attribute, run it, then run the full Darling suite once (DARLING_TEST_PG unset) plus the targeted live classes named in the brief's step 5.

CHANGELOG entry

@erikdarlingdata

Copy link
Copy Markdown
Owner Author

LA-4a

Steps 1–3 done, pushed to fix/3653-a6-stitched-reads (commits issue-3653 A6 LA-4a steps 1-2 and ... step 3). Step 4 (live proof) NOT run — out of budget at the context wall.

Step 1 — BuildBucketedHourlyTrendSql (via ReadRoutedDurationTrendAsync) and QueryHistoryHourlySqlFor (via GetQueryHistoryAsync) now splice route.HourlyFromClauseOrDefault instead of the bare route.HourlyView name. Both builders already took their hourly relation as a string parameter, so this is a call-site swap: route.HourlyView → route.HourlyFromClauseOrDefault. No column-qualification changes were needed — both builders' hourly bodies read columns unqualified (bucket, elapsed_time_sum, ...), so the AS h / AS f alias attaches harmlessly.

Step 2 — HasAnySampleOnRouteAsync's rollup probe now reads route.ProbeHourlyViewOrDefault instead of route.HourlyView. ProbeHourlyViewOrDefault was already on the record (from commit 58c917e) but nothing set ProbeHourlyView, so it always fell back to the legacy name — decision 4 was inert. I wired it in ResolveDurationTrendRoute: added an optional windowEndUtc parameter (threaded through ResolveQueryDurationTrendRoute/ResolveProcedureDurationTrendRoute, both already had an optional nowUtc), defaulting to null. When a caller supplies it, the route probes the successor iff the successor's floor (a non-null RollupCoverage.FloorOf) is <= windowEndUtc; a caller that omits it (every existing caller — DarlingMcpTrendTools.cs never resolved these two routes with a window end before this) gets ProbeHourlyView: null, so ProbeHourlyViewOrDefault still falls back to the legacy — unchanged behaviour. No caller was updated to pass windowEndUtc — that's a follow-up wiring step for whoever owns DarlingMcpTrendTools.cs's GetQueryDurationTrend/GetProcedureDurationTrend (not in my file list).

Step 3 — new Darling/Darling.Tests/DarlingTrendReaderStitchTests.cs, 8 pure tests (no rig): the two builders' byte-identical-with-no-successor case (pinned against the old text with the alias substituted), the UNION-ALL/literal-floor/both-names case for both builders, and 4 cases for the route's probe-name resolution (no window end → legacy; successor floor ≤ end → successor; end before floor → legacy; successor unavailable → legacy). All 8 pass locally (dotnet Darling.Tests.dll -class Darling.Tests.DarlingTrendReaderStitchTests, Total: 8, Failed: 0).

Step 4 — NOT DONE. Ran out of context budget before starting the live PG proof (create the two overlapping-bucket rollups, run a real read through the reader, check per-bucket totals split exactly at F). Whoever picks this up: rig is timescale/timescaledb:2.30.1-pg18 per A6-COMMON; seed pattern is in IntervalHonestHourlyRollupLiveTests.

Known collateral, not fixed (out of my file scope): Darling/Darling.Tests/DarlingMcpTrendToolsTests.cs's RawDurationTrendSql_ReadsTheStoredInterval_ThreeState_AndTheRawConstsAreItsAliases test has a stale source-text pin: Assert.Contains("DurationTrendRouting.BuildBucketedHourlyTrendSql(route.HourlyView)", reader, ...). My step-1 edit changed that call site to route.HourlyFromClauseOrDefault, so that one assertion now fails (dotnet ... -class Darling.Tests.DarlingMcpTrendToolsSurfaceAndSqlTests → Total: 45, Failed: 1). My brief restricts me to DarlingTrendReader.cs plus one new test file, so I did not touch that test. Needs a one-line fix: change that assertion's literal to route.HourlyFromClauseOrDefault.

Also ran Darling.Tests.StitchedRelationSqlTests (11 passed) and confirmed DarlingTrendReader.cs/Darling.Tests both build clean with -p:EnableWindowsTargeting=true.

@erikdarlingdata

Copy link
Copy Markdown
Owner Author

LA-5 (closeout)

Five items, all done on fix/3653-a6-stitched-reads (commit 8454c3a, no force-push):

  1. Stale pin fixed. RawDurationTrendSql_ReadsTheStoredInterval_ThreeState… in DarlingMcpTrendToolsTests.cs pinned the pre-LA-4a call DurationTrendRouting.BuildBucketedHourlyTrendSql(route.HourlyView). LA-4a had already routed the real call site to route.HourlyFromClauseOrDefault; the pin now matches, with a one-line comment explaining why.

  2. Decision 4 wired live. Both production call sites in DarlingMcpTrendTools.cs (GetQueryDurationTrend and GetProcedureDurationTrend) now pass windowEndUtc: now into ResolveQueryDurationTrendRoute / ResolveProcedureDurationTrendRoute. Before this, no caller ever passed a window end, so ProbeHourlyView stayed null and the probe always hit the legacy relation regardless of a materialized successor. Added a pure test, ResolveProcedureRoute_SuccessorFloorAtOrBeforeWindowEnd_ProbeHourlyViewOrDefault_IsSuccessor (the query-side twin already existed), pinning that with a successor whose floor is ≤ the window end, ProbeHourlyViewOrDefault picks the successor.

  3. Scan test flipped to [Fact]. NoReaderOutsideTheBuilder_CallsHourlyRelationForDirectly in RollupCoverageRoutingTests.cs now runs unconditionally. It reports 3 offenders, all of which I judge to be the documented decision-1 carve-out, NOT readers that should route:

    • DailySummarySql.cs:596 — the no-successor branch, which returns the byte-identical legacy-only text StitchedRelationSql itself would answer for that shape.
    • ComposeSourceRouter.cs:350 — CaggRelation, explicitly documented as "the by-NAME answer (probes/logs/registry lookups)"; the FROM-clause item on the same line already uses StitchedRelationSql.
    • DarlingTrendReader.cs:948 — building DurationTrendRoute.HourlyView, the bare-name field the record's own doc comment reserves for probes/logs (decision 1), while HourlyFromClause right below it already carries the stitch.
      I did not silence or delete the guard; I left it red and am reporting rather than "fixing" call sites I believe are correct as written. If the design intends these three to also route, that's a follow-up decision, not a mechanical fix.
  4. Live query-history proof added. DailySummaryAndComposeStitchedLiveTests.cs's existing seeded stitched hourly pair (5 days, split at F on a day boundary) now also drives a THIRD reader over the same seed: DarlingTrendReader.GetQueryHistoryAsync, called with the hourlyRelation argument set to coverage.StitchedRelationSql(...) — the same FROM-clause the production caller (get_query_history) builds. Three sub-checks: a hash planted exactly at F (successor-side, found once, not double-counted at the seam), a hash below F (legacy-only side), and a hash at/above F on the successor-only side. All three land exactly one row with the correct DeltaCpuUs.

  5. Runs (rig: timescale/timescaledb:2.30.1-pg18, container removed after):

    • DarlingTrendReaderStitchTests: 9/9 pass.
    • DarlingMcpTrendToolsSurfaceAndSqlTests + DurationTrendTierRoutingTests: 49/49 pass.
    • DarlingMcpTrendToolsLivePostgresTests: 2/2 pass (live).
    • DailySummaryAndComposeStitchedLiveTests (my new test included): 1/1 pass (live).
    • RollupCoverageRoutingTests: 40 total, 1 FAIL — the scan test above, expected and reported in item 3, not fixed.
    • Broad live sweep (*RollupCoverage*, *StitchedRelationSql*, *DailySummary*, *Compose*, *DocCommentHygiene*, plus the above): 512 total, 2 FAIL:
      • RollupCoverageRoutingTests.NoReaderOutsideTheBuilder_CallsHourlyRelationForDirectly — item 3, expected.
      • DocCommentHygieneTests.EveryDocRunClosesTheSummariesItOpens — "Unbalanced <summary> elements" — I did not touch any doc comments and did not investigate further under the context/time budget; it is unrelated to this lane's edits and may be pre-existing on dev/this branch already. Flagging rather than fixing blind.
    • Full Darling suite NOT run in this lane: I hit the 150k context-wall notice right after the targeted live sweep and moved straight to commit/push per the wall instructions, so the base-214 full run is deferred. Whoever picks this up next should run it once with DARLING_TEST_PG unset.

Not verified: the full suite (see above), and whether DocCommentHygieneTests failure predates this lane (I did not check dev or the branch's prior commits for it).

CHANGELOG entry

@erikdarlingdata

Copy link
Copy Markdown
Owner Author

LA-6

Ruling applied (LA-5's 3 offenders + doc-hygiene):

  1. DailySummarySql.RangeSqlFor(tier, coverage, windowStartUtc) legacy-only branch now reads the relation
    name back off coverage.StitchedRelationSql(legacy, "f", windowStartUtc, StitchTier.Hourly)'s own splice
    text instead of calling HourlyRelationFor directly. This branch only runs when StitchFloor answered
    null, which by StitchedRelationSql's own remarks is exactly the case it splices collect.{legacy} AS f
    — the same name HourlyRelationFor would have answered — so every SQL splice in the file is now on one
    path (decision 1), with no change to the byte-identical pins.
  2. Added RollupCoverage.HourlyRelationNameFor(legacyHourly, windowStartUtc) in TimescaleSupport.cs, next
    to HourlyRelationFor: a thin by-name accessor for probes/logs/registry lookups, doc'd as never to be
    spliced into SQL. ComposeSourceRouter.cs:339 (CaggRelation) and DarlingTrendReader.cs:912
    (DurationTrendRoute.HourlyView) now call it instead of HourlyRelationFor directly. The scan test's
    exclusion (TimescaleSupport.cs only) is unchanged.
  3. DocCommentHygieneTests.EveryDocRunClosesTheSummariesItOpens failed on TimescaleSupport.cs:11537:
    RawTableFor's doc comment closed </summary> right after the first sentence, then opened an unclosed
    <para> block and closed </summary> a second time — a stacked-closing artifact, pre-existing and
    unrelated to this lane's other edits. Fixed by moving the first </summary> down to after the <para>
    block, so the doc run opens and closes exactly once. Read the member first per the test's own rule: the
    <para> clearly belonged to the same doc run (continues describing the same member), not a displaced one.

Targeted, green: RollupCoverageRoutingTests (40), DailySummaryNotCarriedTests (6),
ComposeSourceRouterTests (26), IntervalHonestHourlyRollupTests (8), DocCommentHygieneTests (77),
DailySummaryStitchedRangeTests (5), StitchedRelationSqlTests (11), DarlingTrendReaderStitchTests (9),
DarlingQueryTrendTieringTests (12), QueryStoreTrendRoutingTests (12). No live-PG arm was needed for
these classes (none require DARLING_TEST_PG).

Full suite, once, DARLING_TEST_PG unset: 13729 total, 214 failed, 760 skipped — matches the documented
Mac base (214–216). ViewerTrendRoutingPortTests has 2 pre-existing PresentationFramework
FileNotFoundExceptions on this Mac (WPF assembly not loadable outside Windows), unrelated to this change,
included in the 214.

Not verified: live-PG proof on timescale/timescaledb:2.30.1-pg18 — not run since none of the targeted
classes have a live arm and the coordinator's ruling was a pure routing/naming change with no new SQL.

Branch: pushed to fix/3653-a6-stitched-reads (plain push, no force, title kept). Only the 4 files in
scope touched: ComposeSourceRouter.cs, DarlingTrendReader.cs, DailySummarySql.cs, TimescaleSupport.cs.

Adds the stitch builder from design-3653-A6-v2.md \u00a72: a FROM-clause item
that reads a frozen legacy rollup and its interval-honest successor as
one relation, split at the successor's first bucket, on both the
hourly and daily tiers.

- Three shapes: legacy-only (no successor / absent / empty legacy),
  successor-only (no legacy, empty legacy, or successor already
  reaches the window start), or a UNION ALL split at a literal
  boundary F.
- Daily boundary F_d = max(FloorOf(successorDaily),
  ceilDay(FloorOf(successorHourly))), keeping the successor hourly's
  partial first day on the legacy side.
- Per-pair explicit column lists (no sample_interval_seconds_sum; no
  reader selects it).
- LA carries a private stub of the daily registry (LegacyDaily,
  SuccessorDaily, SuccessorHourly) marked for LB to replace with
  SupersededDailyRollups. Until LB lands, the daily stitch is inert
  (legacy-only) on every store, by construction.

Not yet done in this PR (see PR body): routing the hourly- and
daily-tier readers listed in \u00a71 through the builder, the live PG
proof, and the RollupCoverageRoutingTests Resolve-call scan. Ran out
of budget on step 1; the routing steps are the PR's open work.

Part of #3653 (A6, lane LA)
…edRelationSql

ComposeRoute gains CaggFromClause (decision 2): CaggRelation stays the
by-name answer for probes/logs; CaggFromClause is the FROM-clause item
the compiler splices in, computed by RollupCoverage.StitchedRelationSql
so a stitch applies when a successor's floor sits inside the window.
With no successor this is byte-identical to collect.<view> AS f.
ComposeCompiler's BuildFactRelation/AppendFactBody use CaggFromClause
when present and skip the alias it already carries.
…gh StitchedRelationSql

ReadRoutedDurationTrendAsync now builds the hourly FROM-clause item via
RollupCoverage.StitchedRelationSql instead of HourlyRelationFor's bare
name, so a stitch applies when a successor's floor is inside the
window. With no successor the FROM clause is byte-identical to
collect.<legacy> AS f, and the code takes the hourlySql fast path
exactly as before.
…hrough StitchedRelationSql

GetQueryTrend now resolves the hourly relation via
RollupCoverage.StitchedRelationSql rather than the by-name
HourlyRelationFor, matching QueryHistoryHourlySqlFor's FROM-clause
consumer. With no successor the text is byte-identical to today's
legacy relation name.
… until LA-3b lands)

NoReaderOutsideTheBuilder_CallsHourlyRelationForDirectly walks Darling/
for HourlyRelationFor( call sites outside TimescaleSupport.cs (the
builder). Marked [Fact(Explicit = true)] because LA-3b's readers
(DarlingTrendReader, DailySummary, DarlingHealthReader) still call it
by name; LA-3b flips it to a plain [Fact] once all ten readers route
through StitchedRelationSql.
…yFromClause/ProbeHourlyView fields

Adds the two FROM-clause/probe-name fields decisions 1/2/4 require to
DurationTrendRoute and wires ResolveDurationTrendRoute to compute
HourlyFromClause via RollupCoverage.StitchedRelationSql. The splice
into BuildBucketedHourlyTrendSql/QueryHistoryHourlySqlFor and the
successor-aware probe wiring are NOT done yet -- ran out of budget
mid-step. See PR #4182 body section LA-3b1 for the handoff.
…carried probe

- RollupCoverage.StitchFloor: exposes the same boundary StitchedRelationSql
  splits at, or null when that call would answer legacy-only/successor-only.
- DailySummarySql.QueriesCteForStitchedCagg: runs the not-carried probe (and
  the rollup half) twice, once per relation name split at F, UNION ALL'd,
  per decision 3.
- New DailySummarySql.RangeSqlFor(tier, coverage, windowStartUtc) overload:
  stitches when StitchFloor is non-null, else delegates to the existing
  RangeSqlFor(tier, hourlyRelation) unchanged.
- ViewerDataService.DailySummary.cs and DarlingHealthReader.cs now call the
  new overload instead of resolving HourlyRelationFor + RangeSqlFor by name.
…ead by the daily summary and one Compose query
… add HourlyRelationNameFor for by-name uses, fix doc-hygiene

- DailySummarySql.RangeSqlFor(tier, coverage, windowStartUtc): the legacy-only branch now reads its
  relation name back off StitchedRelationSql's own splice instead of calling HourlyRelationFor directly,
  so every SQL splice in the file goes through the builder (decision 1). Byte-identical to before: the
  branch only runs when StitchFloor answered null, which is exactly where the two calls agree.
- TimescaleSupport.RollupCoverage: added HourlyRelationNameFor(legacyHourly, windowStartUtc), a thin
  by-name accessor for probes/logs/registry lookups, documented as never to be spliced into SQL.
- ComposeSourceRouter.cs and DarlingTrendReader.cs: switched their by-name uses (CaggRelation,
  DurationTrendRoute.HourlyView) onto HourlyRelationNameFor, leaving their SQL splices on
  StitchedRelationSql as before.
- DocCommentHygieneTests: fixed RawTableFor's doc comment, which had two </summary> for one <summary>
  (a stray early close before the <para> block) — a pre-existing doc-run defect the scan test caught.

Targeted classes green: RollupCoverageRoutingTests (40), DailySummaryNotCarriedTests (6),
ComposeSourceRouterTests (26), IntervalHonestHourlyRollupTests (8), DocCommentHygieneTests (77),
DailySummaryStitchedRangeTests (5), StitchedRelationSqlTests (11), DarlingTrendReaderStitchTests (9),
DarlingQueryTrendTieringTests (12), QueryStoreTrendRoutingTests (12). Full suite once, DARLING_TEST_PG
unset: 13729 total, 214 failed (matches the documented Mac base), 760 skipped. ViewerTrendRoutingPortTests
has 2 pre-existing PresentationFramework FileNotFoundExceptions on this Mac, unrelated to this change.
@erikdarlingdata
erikdarlingdata force-pushed the fix/3653-a6-stitched-reads branch from cf23276 to e2e007f Compare September 24, 2026 22:54
…hygiene comment, rewrite stale pre-A6 expectation

- (A) hygiene: add the own-store NOT [Collection("live-postgres")] doc comment to
  DailySummaryAndComposeStitchedLiveTests, in PgDeadlockRemaskTests' shape.
- (B) the real bug: DailySummarySql.QueriesCteForStitchedCagg split every {boundary} at the
  successor's floor F's own HOUR. A mid-day F let the floor day supply a row from each side of
  the UNION ALL, doubling that one calendar day with split distinct counts. Fixed by computing a
  DAY-aligned boundaryDay (F itself if already a day start, else F's date plus one day) and using
  it everywhere {boundary} appears. Proved red first (a temporary boundaryDay = successorFloor
  reproduced 3 rows where 2 were correct on the new live test), then green with the fix.
- (C) DailySummaryNotCarriedLiveTests' recent-server leg asserted the pre-A6 supply rule (the
  reader carries the legacy's whole answer). Since LA-3b2 the reader stitches at F, day-aligned;
  rewrote the leg to the stitched answer, derived by running it live: days [R0,R2,R3,R4], counts
  [3,null,7,2], days_missing=[R2].
- New tests: a pure pin in DailySummaryStitchedRangeTests that the SQL literal is the day-aligned
  boundary, not F's own hour, and a new live test
  (DailySummary_MidDayFloor_PrintsExactlyOneCalendarRow_NotTwo) that seeds a mid-day floor and
  pins exactly one calendar row for the floor day.
- Updated DailySummaryAndComposeStitchedLiveTests' existing stitched-pair leg: with the
  day-aligned boundary, D2 (where F itself falls) now reads whole from the legacy, so its count
  is 2 (was 1), and the comment explaining the old 'F lands exactly on a day start' framing is
  corrected.

Part of #3653 (A6, lane LA-7).
@erikdarlingdata

Copy link
Copy Markdown
Owner Author

LA-7

Fixed PR #4182's two CI failures plus the pre-existing leg the fix invalidates.

(A) hygiene, mechanical. Added the /* #1776 own-store: ... */ doc comment (PgDeadlockRemaskTests' exact shape) above DailySummaryAndComposeStitchedLiveTests.

(B) the real bug. DailySummarySql.QueriesCteForStitchedCagg split every {boundary} literal at the successor's floor F's own HOUR. With a mid-day F (production: the successor's first bucket is whatever hour it started), the floor day supplied a row from EACH side of the UNION ALL (legacy for its early hours, successor for its later ones), so the calendar printed that one day twice with split distinct counts — a COUNT(DISTINCT) computed separately per half cannot be summed without double-counting hashes on both sides.

Fix: var boundaryDay = successorFloor == successorFloor.Date ? successorFloor : successorFloor.Date.AddDays(1); — the first whole day at or after F (same rule as the daily tier's F_d) — used for every {boundary} site (both ceilings, both count halves, both not-carried probes).

Proved RED first: temporarily set boundaryDay = successorFloor (undoing the fix) and ran the new live test — it failed with 3 rows for the floor day's window instead of 2, with a split count. Restored the fix, reran: 2 rows, correct whole-day count. Also verified against the direct SQL dump (day-boundary literal correctly reads 2026-09-15 00:00:00 when F is 2026-09-14 14:00:00).

(C) stale pre-A6 expectation. DailySummaryNotCarriedLiveTests's recent-server leg asserted the pre-stitch supply rule ("the reader carries the legacy's whole answer"). Since LA-3b2, DarlingHealthReader.GetDailySummaryRangeAsync stitches at the successor's floor (day-aligned, this fix). Derived the new numbers by running the live test: days [R0,R2,R3,R4], counts [3,null,7,2], days_missing=[R2] — matching the brief's prediction exactly. Rewrote the leg's assertions, its wire-level days_missing check, and its comment; kept the coverage.HourlyRelationFor(hourly, R(0)) == hourly assert (names still use the whole-window rule per decision 1) and added a StitchFloor(...).HasValue assert that the stitch actually applies for this window.

New/updated tests:

  • DailySummaryStitchedRangeTests.StitchedPair_SplitsTheNotCarriedProbeOnceEachSide_AtTheSameBoundaryTheReadSplits: updated to expect the day-aligned literal, with a comment explaining the FROM-clause split (StitchedRelationSql, still F's own hour) and the probe's split (day-aligned) are deliberately different literals now, agreeing only on the calendar day.
  • New live test DailySummaryAndComposeStitchedLiveTests.DailySummary_MidDayFloor_PrintsExactlyOneCalendarRow_NotTwo: seeds a mid-day floor with hashes on both sides, pins exactly one calendar row for the floor day holding the whole-day distinct count. This is the test that proved (B) red-then-green.
  • Updated the pre-existing DailySummaryAndCompose_BothReadTheStitchedPair... live test: with the day-aligned boundary, D2 (where its F actually falls) now reads whole from the legacy side, so its distinct count is 2 (was 1 under the un-fixed, hour-split boundary); corrected its comment.

Ran (live, timescale/timescaledb:2.30.1-pg18, DARLING_TEST_PG set): DailySummaryAndComposeStitchedLiveTests (2/2), DailySummaryNotCarriedLiveTests (1/1), DailySummaryStitchedRangeTests (5/5, pure), RollupCoverageRoutingTests (pure, all green). LivePostgresCollectionHygieneTests.EveryClassUsingTheSharedStore_IsSerializedOrDocumentsWhyNot fails on this Mac with Could not load file or assembly 'PresentationFramework' — a pre-existing macOS artifact of stripping Microsoft.WindowsDesktop.App from the test runtimeconfig for this rig (the class does a reflection scan over loaded assemblies, which chokes on a Windows-only module reference with no WPF present); unrelated to this PR's changes and expected to pass in CI where WPF is available.

Did NOT run: the full Darling suite (context budget did not permit it this lane; left for the coordinator/pr-tender to run once before arming).

CHANGELOG entry

  • Fixed a bug where the daily summary calendar printed two rows for the day a stitched hourly rollup's successor started mid-day, with the query count split incorrectly between them; the split now aligns to the calendar day.

@erikdarlingdata erikdarlingdata changed the title DO NOT MERGE (A6: routing incomplete): stitched rollup reads, hourly+daily builder only issue-3653 A6 lane LA: stitched rollup reads, every hourly reader routes legacy→successor at the successor floor Sep 24, 2026
@erikdarlingdata
erikdarlingdata marked this pull request as ready for review September 24, 2026 23:39
@erikdarlingdata
erikdarlingdata merged commit b6bd763 into dev Sep 24, 2026
16 of 17 checks passed
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