[erikdarlingdata]

The investigation sweep's resolve check now tests only alert log rows after the oldest last alert among the live candidates. The plan still scans the log; what it saves is the per-row work (the Resolves text test and the join) on every older row. About 5x faster on the one store measured.

What was slow

ResolveCandidatesSql runs once a minute, inside the investigation sweep. Its join to config.config_alert_log had only h.alert_time > m.last_alert, a per-row condition. The planner read every log row and tested each row's context text for Resolves.

The change

• Darling (InvestigationRequestStore.ResolveCandidatesSql): the candidate list is a CTE m, and the join adds h.alert_time > (SELECT min(last_alert) FROM m).
• Lite (DuckDbInvestigationStore.ResolveCandidatesSql): the same, over v_config_alert_log (live log plus archive).

Why it drops no candidate

Every resolution row a candidate can use is after that candidate's last alert, so it is after the oldest last alert. The bound removes only rows no candidate could use. It does not depend on how a request got its alerts. This matters for follow-ups: a follow-up opens at now but copies its source's alert rows with their original times (CopyFollowUpAlertsSql), and follow_up_at may be 30 days out. A fixed now - 3 days window dropped a clear older than 3 days for such a request.

Measurement

Read-only, one production store (PostgreSQL 18, about 30,000 alert log rows), EXPLAIN (ANALYZE), run 1 cold and run 2 warm, wall ms:

Form	Run 1 / run 2	Plan on the log
Old (no bound)	76 / 56	seq scan
Fixed $1 - 3 days (earlier head of this PR)	7.4 / 3.7	index scan
Oldest last alert (this head)	13.2 / 11.9	seq scan

The result rows match the old statement exactly: EXCEPT ALL both ways gave 0 and 0. The store held one follow-up request, so that comparison has weak follow-up coverage. The min() bound is a subquery, not a constant, so the planner does not pick the time index here. It is about 5x faster than the old form, but slower than the fixed window. Lite is not measured: DuckDB is not available to measure here. PostgreSQL 16 and 17 are not measured.

Tests

• InvestigationRequestEventsLiveTests.Resolved_CountsAClearOlderThanThreeDays_ForARequestWhoseAlertsAreOlder and LiteInvestigationRequestEventsTests.ARequestWhoseAlertsAreOlderThanThreeDays_IsResolvedByAClearAfterThem age a request's alerts to 10 days and log a clear after them. Revert proof: with the fixed $1 - 3 days bound, the clear is outside the window, the metric reads NULL and the sweep resolves 0, so both fail. Both wait for CI (no local PostgreSQL, Lite is Windows only).
• An earlier line here said Darling.Tests built on macOS with 0 errors. That was wrong: the new live test called PgAlertHistoryStore without its command timeout, so the project did not compile. Fixed in 400ef1f; CI builds and runs it.

Review round

• Follow-up alerts older than the window (REFUTED claim): real. The bound is now the oldest last alert, and the comments and this body are corrected.
• "No behavior a test could fail on" (REFUTED): real. A test per store is added.
• Riskiest lines 1 to 3: fixed by the same change plus the tests and the new comments.
• EXPLAIN on PostgreSQL 16 or 17: not measured. No such store is reachable. Considered, not added.
• Lite measurement: not measured. Both numbers above are PostgreSQL.
• Built-on-macOS claim: now states which projects built.
• Time index claim: corrected. The new form does a seq scan in the measured plan.

CHANGELOG entry

Investigations: the sweep's resolve check tests only alert log rows after the oldest live candidate's last alert, so each run does less work on the 90-day log. The same requests resolve, including follow-ups whose alerts are weeks old.

Review round (changes requested on cb6f517)

• Build break (REFUTED build claim, riskiest line 1): real. InvestigationRequestEventsLiveTests.cs:198 now passes DarlingAlertReadAdapter.AlertPassCommandTimeoutSeconds, like the file's other nine call sites (400ef1f).
• "No longer reads the whole 90-day alert log" (REFUTED): real. The bound is implied by the per-row join condition, so the same rows qualify and the log is still scanned. The opening line and the CHANGELOG entry now say the saving is per-row work, not rows read.

Review round (changes requested on 400ef1f)

• Both code comments still stated the refuted rows-read mechanism (InvestigationRequestStore.cs "stops reading the whole 90-day log", DuckDbInvestigationStore.cs "keeps the join from reading the whole alert log, the archive included"): real. Fixed in 6cd73cb. Both now say the plan still reads the rows and the bound saves the per-row work on every older row. The Lite comment says the saving was measured on the PostgreSQL store only.
• "About 60 ms a minute" was the cold delta (76 - 13.2): real. The pass runs once a minute, so the warm pair applies: the comment now says about 44 ms a pass (56 - 11.9).
• Neither comment said that a live follow-up pulls min(last_alert) back by up to 30 days: real. Both comments now say the saving is smaller while one is open.
• No test covers min swapped to max (inline, marked non-blocking): not added. This is the third review round, and only a High finding lands in a third round. "Why it drops no candidate" above gives the argument for min.
• Reviewer checklist, InitPlan: (SELECT min(last_alert) FROM m) is uncorrelated, so PostgreSQL runs it once as an InitPlan. The measured 13.2 / 11.9 ms against 76 / 56 ms fits that; a per-row subquery would cost more than the old statement, not less. The EXPLAIN on a store with a weeks-old follow-up was not run.
• 6cd73cb changes comments only, in both files. No code, SQL or test changed.

[claude]

A follow-up request links alerts that are not "at most minutes before it opened", so this bound can drop the only resolution candidate.

SweepFollowUpsAsync opens the follow-up with opened_at = now (OpenFollowUpSql, line 262) and then copies the source request's alert rows with their original alert_time (CopyFollowUpAlertsSql, lines 268-273, called at line 1529). follow_up_at may be up to 30 days ahead of the report (InvestigationReportFields.MaxFollowUpDays = 30), and a deploy follow-up (#6009) can open at that latest date too. So a follow-up satisfies r.opened_at > $1 - interval '2 days' and practice_of IS NULL while its max(alert_time) is days or weeks old.

Concretely: source request reported with follow_up_at 10 days out; the metric cleared minutes after its last alert, i.e. ~10 days ago; the follow-up opens today with last_alert ≈ 10 days ago.

• Before: the resolution row (≈10 days old, after last_alert) came back, every linked metric was named, and the first sweep journaled resolved — an event PendingDeliverySql delivers to the agent route.
• After: the row is outside $1 - interval '3 days', the metric comes back NULL, and the follow-up is seen as unresolved until the metric fires and clears again.

That is the same shape as the practice case the comment acknowledges one line below ("A practice request can link an older alert"), except the follow-up path is not excluded by practice_of IS NULL — OpenFollowUpSql never sets practice_of. So "It drops no candidate" and the body's "the result is the same row for row" do not hold here, and the behaviour change is uncovered: InvestigationRequestEventsLiveTests.Resolved_WaitsForEveryLinkedMetric_AndARefireIsNotAClear keeps its alerts and clears minutes apart, and InvestigationFollowUpLiveTests uses a two-hour follow_up_at, so neither fails on it.

Either bound on the thing the join actually keys off — e.g. h.alert_time > least($1, (SELECT min(last_alert) FROM m-equivalent)) - interval '…', or m.last_alert - interval '…' carried from the inner aggregate — or exclude follow-ups from the window argument explicitly and say so in the comment. If auto-resolving a stale follow-up is judged wrong behaviour that this incidentally fixes, that is a deliberate behaviour change and belongs in the body and the CHANGELOG entry, with a test that pins it.

[claude]

Same gap as Darling's twin, and Lite has no practice_of escape hatch at all.

Lite's follow-up sweep copies the source's alert rows with their original alert_time (CopyFollowUpAlertsSql, lines 184-190, called at line 1042) onto a request whose opened_at is now (OpenFollowUpSql, line 173). follow_up_at is validated only as "after now and at most 30 days ahead" (InvestigationReportFields.ValidateFollowUp), so a follow-up opened today can carry a last_alert weeks old. Its metric's resolution row — written minutes after that last alert — then falls outside $1 - INTERVAL 3 DAY, the metric comes back NULL, and SweepResolvedAsync (line 1083) no longer journals resolved for a request it resolved on the first sweep before this change.

The inline comment's "It drops no candidate: a request here opened in the last 2 days, and each alert linked to it opened or joined it" is the claim this breaks — in Lite a linked alert can also have been copied from an earlier request.

LiteInvestigationRequestEventsTests.ARequestIsResolved_OnlyWhenEveryLinkedMetricHasAResolutionRowAfterItsLastAlert_OnItsOwnServer (lines 119-139) places its rows minutes apart and LiteInvestigationFollowUpTests uses a two-hour follow_up_at, so no behaviour test fails on this.

[claude]

Non-blocking, on the mechanism the body names: "the planner could not use the log's time index".

There is no alert_time-leading index on that table. The only one is idx_config_alert_log_time ON config_alert_log(server_id, metric_name, alert_time) (PgMigrations.cs:4969), and ResolveCandidatesSql constrains server_id (equality) and now alert_time (range) while leaving metric_name unconstrained — the metric match happens in C# (ResolvesMetric). With the middle column open, alert_time is not a range boundary on a plain btree scan; it becomes an index condition that saves heap fetches, and only PostgreSQL 18's skip scan turns it into something close to the per-server time range the body describes.

The bundled managed store is 18 (Darling/README.md:1908), so the measured "index scan" plans are consistent with what you ran. But the supported floor for a bring-your-own store is 16 (Darling/README.md:39), and there the win on a wide server_id group will be smaller than 67 ms → 3 ms. Nothing to change in the SQL for it; worth one clause in the comment or the body so the next reader does not take "uses the log's time index" as true on every supported major.

[claude]

Verdict: CHANGES REQUESTED

Claims

• VERIFIED — "the join adds h.alert_time > $1 - interval '3 days'; $1 is the sweep's clock, which the request filter already uses" — InvestigationRequestStore.cs:389 beside the pre-existing r.opened_at > $1 - interval '2 days' at :384; $1 is P(now) from SweepResolvedAsync (:1553).
• VERIFIED — "Lite: the same bound, $1 - INTERVAL 3 DAY" — DuckDbInvestigationStore.cs:1070, bound from the single Command(connection, ResolveCandidatesSql, now) at :1086; $1 reuse in one DuckDB statement has precedent at :264-267.
• VERIFIED — "Lite reads v_config_alert_log, the live log plus its archive" — DuckDbInvestigationStore.cs:1067; the view is "live UNION ALL archive" (Lite/Services/DuckDbAlertHistoryStore.cs:175).
• VERIFIED — "the alert log … keeps 90 days" — purged at DarlingRetentionHorizons.AlertHistoryRetentionDays (90), Darling/PerformanceMonitor.Darling.Viewer/ViewerTimeRangeWindow.cs:174.
• VERIFIED — "The query leaves practice requests out (practice_of IS NULL), as it did before" — InvestigationRequestStore.cs:385, untouched; Lite has no practice requests (DuckDbInvestigationStore.cs:146), so the missing Lite filter is not a parity gap.
• VERIFIED — "The existing resolve tests … alerts and resolution rows are minutes apart, inside the bound" — Darling.Tests/InvestigationRequestEventsLiveTests.cs:154-186 and Lite.Tests/LiteInvestigationRequestEventsTests.cs:119-139 drive a fixed clock in T0 + minutes, so they pass before and after.
• UNVERIFIED — the EXPLAIN (ANALYZE) table, the EXCEPT ALL row counts (148/148, 19/19, 0 differences) and the pg_stat_statements figures — live-store measurements; a read-only tree has no store or planner to reproduce them.
• UNVERIFIED — "largest gap between a real request's opening and a linked alert was 1 minute 18 seconds" and "a practice request can link an older alert (17 hours, measured)" — production-data queries, not derivable from the tree.
• UNVERIFIED — "Built on macOS … 0 errors" — needs a build; Lite targets net10.0-windows (WPF), so which project built where is worth stating.
• UNVERIFIED — "the planner could not use the log's time index" / "after: index scan" — the index is (server_id, metric_name, alert_time) (PgMigrations.cs:4969) with metric_name unconstrained here, so the plan depends on the major (skip scan is PostgreSQL 18; the floor is 16, Darling/README.md:39). Non-blocking note posted separately.
• REFUTED — "each alert linked to such a request is the alert that opened it, or one that joined it inside the grouping window. So its last alert is at most minutes before the request opened." — a follow-up's alert rows are copied from the source with their original alert_time (InvestigationRequestStore.cs:268-273, called at :1529; Lite :184-190, called at :1042) onto a request whose opened_at is now (:262 / :173), and follow_up_at may be 30 days ahead (InvestigationReportFields.MaxFollowUpDays). OpenFollowUpSql sets no practice_of, so that carve-out does not cover it.
• REFUTED — "The result is the same row for row." / "The change has no behavior a test could fail on." — for a follow-up whose source's metrics cleared more than three days ago the resolution row is now outside the bound, the metric reads NULL, and resolved (delivered to the agent route by PendingDeliverySql) is no longer written where it was. A test can fail on exactly that: report with follow_up_at, clear the metric, advance past the follow-up time, sweep, assert the resolved row.

Riskiest lines

1. Darling/PerformanceMonitor.Darling.Storage/InvestigationRequestStore.cs:389 — the bound is anchored on $1 while the join it filters is anchored on m.last_alert, which the follow-up path can place weeks earlier; a dropped candidate silently costs a resolved event — UNCOVERED (Resolved_WaitsForEveryLinkedMetric_AndARefireIsNotAClear is minute-scale, InvestigationFollowUpLiveTests uses a two-hour follow_up_at).
2. Lite/Database/DuckDbInvestigationStore.cs:1070 — same bound over v_config_alert_log, with no practice_of filter to lean on — UNCOVERED (ARequestIsResolved_OnlyWhenEveryLinkedMetricHasAResolutionRowAfterItsLastAlert_OnItsOwnServer is minute-scale; the follow-up tests never sweep resolved).
3. Darling/PerformanceMonitor.Darling.Storage/InvestigationRequestStore.cs:387-389 — the comment's "It drops no candidate" is the invariant the next reader will trust when moving this window, and it omits the follow-up origin — UNCOVERED (a comment is not coverage; the behaviour needs the test above).

Traps near this diff

• "Two copies of one definition pinned EQUAL to each other fail only after BOTH have drifted (#3684). Alias, do not copy." — does not apply: the two constants are per-store dialect twins (config.config_alert_log vs v_config_alert_log), nothing asserts them equal, and both moved in this diff (:389, :1070).
• "Lite and Darling ship in parity (CONTRIBUTING's two-store rule): a fix to one SKU's reader, tool, alert or description without the counterpart's is a finding" — does not apply: identical predicate in both SKUs, same diff; the only asymmetry is pre-existing and correct (DuckDbInvestigationStore.cs:146).
• "A retention-tiered store answers a 7-day question from a 4-day raw tier and labels the missing days quiet unless the read routes by tier and says which tier served it (#3590)" — applies in shape: the new window makes an out-of-window follow-up read as not cleared rather than as unknown, because SweepResolvedAsync reads the NULL context that way (:1070, :389).
• "hours_back, limit, top, days_back are refused when out of range, never clamped (#3699)" — does not apply: no caller-supplied window; $1 is the service clock (:1553).
• "A test that encodes the lie … Ask what defect a test would fail ON (#3699)" — applies to the Tests section: the two cited tests pass because they are minute-scale, so they cover the old behaviour, not the window this PR adds.
• "Ruling (#5833): tests are load-bearing … A trap is answered with a behaviour test that fails on the defect, or with one sentence in the PR body saying why none is needed" — applies: the body's sentence is refuted above, so this needs the test instead.
• "ORDER BY 3 + 4 DESC", "MAX(max_dop)", integer division in a rate column, a percent whose denominator is the page, CASE WHEN interval > 0, truncation inferred from a full page, pg_relation_size — do not apply: the statement adds one timestamp > constant conjunct and projects three unchanged columns (:377-390), with no ordering, arithmetic, share, LIMIT or relation sizing.

Divergence (blind read vs PR body)

The body's "Why it drops no candidate" enumerates how an alert comes to be linked to a live request — opened it, joined it in the grouping window, or (excluded) a practice link — and that list is incomplete in both SKUs. SweepFollowUpsAsync is a fourth origin: it opens a request at now and copies the earlier request's alert rows verbatim, alert_time included, with follow_up_at up to 30 days out. So "its last alert is at most minutes before the request opened" is false for every follow-up, and with it "the result is the same row for row" and "no behavior a test could fail on". The measurement story and the dialect halves match the diff exactly; the disagreement is entirely in the safety argument, and the author came within one line of it — the practice caveat is the same shape, with the one carve-out that happens not to catch follow-ups.

Reviewer checklist

• Run the follow-up case on a live store: report with follow_up_at four or more days out, let the metric clear, advance to the follow-up, sweep, and check whether a resolved journal row appears (it did before this change).
• EXPLAIN (ANALYZE) the new statement on a bring-your-own PostgreSQL 16 or 17 store, not only the bundled 18, with metric_name unconstrained.
• Re-run the EXCEPT ALL comparison on a store holding a follow-up request (follow_up_of IS NOT NULL) whose copied alerts are older than three days.
• Measure the Lite side: both numbers are PostgreSQL, and DuckDB's gain comes from Parquet pruning on v_config_alert_log, not an index.

[claude]

Follow-up pass, range aed1d07..301e48456.

301e48456 is Merge remote-tracking branch 'origin/dev' into HEAD, with parents aed1d07 (the PR's own commit, already reviewed) and 6641d44d2 (dev's tip). There are no new PR-authored commits in this range — every commit in the range (6641d44d2, 0a6121e1c, 3a32c4597, 6eed85912, 9401603a8, a0973aa4f, 4d4298deb, ada395ecd, 4d8e618d1, 3a0433b18, 94bd5f05c, 750301161) is dev content the merge brought in, already landed (and presumably reviewed) via its own PR (#6639, #6644, #6608, #6593, #6606, #6590, #6602, #6610, #6616, #6607, #6613, #6592).

ResolveCandidatesSql, the method this PR actually changes, does not appear anywhere in this diff — confirmed with git diff aed1d07..301e48456 | grep ResolveCandidatesSql (no output). The only touch to a file this PR modified is a cosmetic named-argument fix in InvestigationRequestStore.ForceChecks.cs (unrelated method, brought in by 4d4298deb), which does not interact with the resolve-bound logic the earlier review flagged.

So nothing in this range interacts with the prior review's findings (the follow-up-request REFUTED claims on InvestigationRequestStore.cs:389 / DuckDbInvestigationStore.cs:1070) — those remain unaddressed in the code, just out of scope for this specific range per the merge carve-out. The next pass that touches ResolveCandidatesSql itself should re-surface them if still unfixed.

[claude]

Verdict: LGTM

Claims

No claims to grade in this range. The PR body's claims are about the PR's own commit (aed1d07), already graded in the prior full-pass review; this range contains no new PR-authored commits and the body makes no claims about it.

Riskiest lines

No lines in this range belong to this PR's own change. The range is a merge of origin/dev (301e48456, parents aed1d07 and 6641d44d2) with zero new PR-authored commits; ResolveCandidatesSql does not appear in the diff. The one incidental touch to a file this PR modified — InvestigationRequestStore.ForceChecks.cs (named-argument style fix, brought in by dev commit 4d4298deb) — is a different, unrelated method and does not interact with the resolve-bound logic.

Traps near this diff

None checked. Per the merge carve-out, what the dev merge brought in is not this PR's change, and no new commit interacts with it, so the traps file was not applied to that content (it lands via dev's own PRs: #6639, #6644, #6608, #6593, #6606, #6590, #6602, #6610, #6616, #6607, #6613, #6592).

Divergence (blind read vs PR body)

None — the PR body makes no claims about this range (a dev-sync merge with no new PR-authored commits), so there is nothing to diverge from.

Reviewer checklist

• This PR is still a draft; the prior full-pass review's CHANGES_REQUESTED findings on InvestigationRequestStore.cs:389 and DuckDbInvestigationStore.cs:1070 (follow-up requests falling outside the 3-day bound) are unaddressed in the code — confirm they get fixed, and re-reviewed, before this leaves draft.

Claude posting for Erik Darling. Both REFUTED findings were real. The resolve bound is now the oldest candidate's last alert instead of a fixed 3 days, so follow-ups with old copied alerts still resolve. A test per store was added. Body corrected. Fixed in 4c6ed3c.

[claude]

This does not compile — Darling.Tests cannot build.

PgAlertHistoryStore has exactly one constructor, PgAlertHistoryStore(NpgsqlDataSource postgres, int commandTimeoutSeconds, ILogger? logger = null) (Darling/PerformanceMonitor.Darling.Storage/PgAlertHistoryStore.cs:41), on a sealed, non-partial class with no other declaration in the tree (the only hit for class PgAlertHistoryStore repo-wide). NullLogger.Instance is a NullLogger, which has no conversion to int, so argument 2 fails overload resolution (CS1503) and there is no 2-arg overload to fall back to (CS1729).

Every other call site in this same file passes the timeout explicitly — :159, :234, :294, :318, :348, :383, :422, :571, :882:

Suggested change

	var history = new PgAlertHistoryStore(_dataSource!, NullLogger.Instance);
	var history = new PgAlertHistoryStore(_dataSource!, DarlingAlertReadAdapter.AlertPassCommandTimeoutSeconds, NullLogger.Instance);

This also refutes the body's "Built on macOS: Darling.Tests (with Storage), 0 errors" — that build cannot have included this file. Because the whole test assembly fails to compile, the breakage is not limited to the new fact: the build job (CONTRIBUTING.md:517, which runs Darling.Tests without a live store) goes red for every Darling test, and the new fact itself — gated behind Assert.SkipWhen(DARLING_TEST_PG) at :931 — never runs anywhere until this is fixed.

[claude]

Non-blocking, on the aged value this writes. alertAt is derived from T0, which is DateTimeKind.Utc (:47), and AddWithValue with no NpgsqlDbType infers timestamptz for a Kind.Utc DateTime. config.investigation_request_alerts.alert_time is timestamp (no zone), so PostgreSQL applies the assignment cast and converts through the session TimeZone — on a scratch store whose server default is not UTC, the row lands offset from T0 - 10 days rather than at it.

The fact still passes either way (the clear at alertAt + 1 day stays after the shifted alert, and opened_at is untouched), so this is not a failure — but it is the one write in this file that does not hold naive UTC the way the production path does (PgAlertHistoryStore: "alert_time is stamped naive-UTC at record time", PgAlertHistoryStore.cs:27-28), which makes the aged state slightly different from the follow-up state the doc comment says it reproduces. new NpgsqlParameter("t", NpgsqlDbType.Timestamp) { Value = alertAt }, or DateTime.SpecifyKind(alertAt, DateTimeKind.Unspecified), pins it.

[claude]

Non-blocking, and the safety argument holds: min(last_alert) OVER m <= m.last_alert for every row of m, so this conjunct is logically implied by the h.alert_time > m.last_alert on the line above. It cannot change the row set — which is why the EXCEPT ALL came back 0/0, and why it is a pure planner hint rather than a semantic change. Good.

The thing worth one clause in the comment: the bound is the minimum over the whole candidate set, so a single live request with ancient alert rows sets it for everyone — and the follow-up path this PR exists to protect is exactly that request. CopyFollowUpAlertsSql copies alert_time verbatim (:271) onto a request opened at now, with follow_up_at up to 30 days out (InvestigationReportFields.MaxFollowUpDays). So on a store holding one follow-up whose source cleared three weeks ago, min(last_alert) is three weeks back and the bound admits nearly the whole 90-day log; the ~5x the body measured came from a store the body itself describes as having "weak follow-up coverage". The claim "reads only rows after the oldest last alert among the live candidates" is true and the saving is real on a follow-up-free store, but it is data-dependent in the one direction the design cares about, and nothing here regresses when it degenerates.

Also worth noting against the earlier round's index note: with idx_config_alert_log_time (server_id, metric_name, alert_time) and metric_name unconstrained here, alert_time cannot be a range boundary on a plain btree scan — consistent with the seq scan the body's own table reports for this form.

[claude]

Non-blocking, on the Lite half, which the body says is unmeasured. Two things the next reader should not assume:

1. The comment claims this "keeps the join from reading the whole alert log, the archive included". The gain on v_config_alert_log would have to come from Parquet pruning inside read_parquet (DuckDbInitializer.cs:3392-3402, a UNION ALL BY NAME over read_parquet(...) with a correlated NOT EXISTS against dismissed_archive_alerts). DuckDB pushes filters into a Parquet scan when the predicate is a constant; (SELECT min(last_alert) FROM m) is an uncorrelated scalar subquery, which DuckDB flattens into a join against a single-row aggregate — the comparison then tends to live above the scan, not in it. If that is what happens, the Lite comment's sentence is describing a saving that does not occur, and the two extra scans of m (DuckDB inlines a CTE rather than materialising it by default) are a small net cost. One EXPLAIN ANALYZE on a store with a Parquet archive settles it either way.

2. No test covers this statement against the Parquet shape of the view: every investigation fact runs on a fresh database where v_config_alert_log is the table-only form (DuckDbInitializer.cs:3430), and the archive facts (ArchiveInterruptedRunTests, ArchiveExportReadHoldTests) archive config_alert_log but never sweep resolved. So "the archive included" is the one word in this comment nothing exercises — true before this PR as well, just newly asserted by it.

Both are also why a max()-for-min() slip would be caught on the Darling side (Resolved_WaitsForEveryLinkedMetric_AndARefireIsNotAClear has one metric clearing at T0+3 behind another's T0+5 last alert) but not here: in ARequestIsResolved_Only… both clears (T0+5, T0+7) sit after the later last alert (T0+2), so the Lite facts pass under either aggregate.

[claude]

Verdict: CHANGES REQUESTED

Claims

• VERIFIED — "Darling: the candidate list is a CTE m, and the join adds h.alert_time > (SELECT min(last_alert) FROM m)" — InvestigationRequestStore.cs:378 and :391.
• VERIFIED — "Lite: the same, over v_config_alert_log" — DuckDbInvestigationStore.cs:1078, :1087, :1090; identical predicate, same diff, so the two-store rule is met.
• VERIFIED — "Its join had only h.alert_time > m.last_alert, a per-row condition" — removed hunk at :368-383; strpos(h.context_json, 'Resolves') was and is the only other conjunct (:392).
• VERIFIED — "The bound removes only rows no candidate could use" — min(last_alert) over m is <= m.last_alert for every row of m, so :391 is logically implied by :390 and cannot change the row set in either SKU. A NULL last_alert fails both comparisons alike; an empty m returns nothing either way.
• VERIFIED — "a follow-up opens at now but copies its source's alert rows with their original times (CopyFollowUpAlertsSql), and follow_up_at may be 30 days out" — :271 copies alert_time verbatim, called at :1548; InvestigationReportFields.MaxFollowUpDays = 30 (:38).
• VERIFIED — "A test per store is added … age a request's alerts to 10 days and log a clear after them" — InvestigationRequestEventsLiveTests.cs:193-214, LiteInvestigationRequestEventsTests.cs:118-129: behaviour facts on SweepAsync().Resolved, no source-text pin.
• VERIFIED — "Revert proof: with the fixed $1 - 3 days bound … the sweep resolves 0, so both fail" — alert at T0-10d, clear at T0-9d, $1 = T0+1min: that bound excludes the clear, the LEFT JOIN yields a NULL context, and the All(done => done) gate is false (:1594, DuckDbInvestigationStore.cs:1126).
• VERIFIED — "The min() bound is a subquery, not a constant, so the planner does not pick the time index here" — the schema agrees independently of the plan: the only index is (server_id, metric_name, alert_time) (PgMigrations.cs:5026) and metric_name is unconstrained here (the match is C#, :1608), so alert_time is no btree range boundary.
• VERIFIED — "Lite is Windows only" — Lite.Tests.csproj:3 and Darling.Tests.csproj:4 are both net10.0-windows.
• UNVERIFIED — the EXPLAIN (ANALYZE) table and "one production store (PostgreSQL 18, ~30,000 alert log rows)" — live measurement; needs that store.
• UNVERIFIED — "EXCEPT ALL both ways gave 0 and 0" — expected given the conjunct is implied, but the run is not checkable here.
• UNVERIFIED — "Lite is not measured … PostgreSQL 16 and 17 are not measured" — a statement about what was not done.
• UNVERIFIED — "Both wait for CI" — CI state. The Darling fact is also gated by Assert.SkipWhen(DARLING_TEST_PG) (:930), so the build job (CONTRIBUTING.md:517) never executes it.
• REFUTED — "Built on macOS: Darling.Tests (with Storage), 0 errors." — InvestigationRequestEventsLiveTests.cs:198 calls new PgAlertHistoryStore(_dataSource!, NullLogger.Instance); the sole constructor is (NpgsqlDataSource, int commandTimeoutSeconds, ILogger? = null) on a sealed, non-partial class with no other declaration in the tree (PgAlertHistoryStore.cs:30, :41), and NullLogger has no conversion to int. Darling.Tests cannot compile; the nine other call sites in the file pass the timeout. Inline comment posted with the fix.
• REFUTED — "The resolve check no longer reads the whole 90-day alert log on every run." — the body's own table reports a seq scan on the log for this head, and :391 is implied by :390, so the same rows qualify and the log is still read end to end. What is saved is per-row work (the strpos test, heap visits), not rows read.

Riskiest lines

1. Darling/Darling.Tests/InvestigationRequestEventsLiveTests.cs:198 — a constructor arity that fails overload resolution and takes the whole assembly with it, so the new fact and every pre-existing Darling fact stop running — UNCOVERED (the compiler is what fails on it, and it is failing).
2. Darling/PerformanceMonitor.Darling.Storage/InvestigationRequestStore.cs:391 — the new semantic line; a bound mis-stated as the clock or as max(last_alert) silently costs a resolved row and the delivery riding it (PendingDeliverySql) — covered by Resolved_CountsAClearOlderThanThreeDays_ForARequestWhoseAlertsAreOlder (fails on a clock window) and Resolved_WaitsForEveryLinkedMetric_AndARefireIsNotAClear (fails on max() for min(): High CPU's clear at T0+3 sits behind Memory Pressure's T0+5 last alert) — both only once :198 compiles, and only in the DARLING_TEST_PG job.
3. Lite/Database/DuckDbInvestigationStore.cs:1090 — the same conjunct over a view that is UNION ALL BY NAME over read_parquet(...) plus a correlated NOT EXISTS (DuckDbInitializer.cs:3392-3402); if DuckDB treats an uncorrelated scalar subquery in a LEFT JOIN … ON differently there, the sweep throws or mis-plans every pass — covered by ARequestWhoseAlertsAreOlderThanThreeDays_IsResolvedByAClearAfterThem for the binder question and the clock-window defect in the table-only view shape; UNCOVERED for the Parquet shape (no Lite fact archives config_alert_log then sweeps resolved) and for a min→max slip.

Traps near this diff

• "Lite and Darling ship in parity (CONTRIBUTING's two-store rule): a fix to one SKU's reader, tool, alert or description without the counterpart's is a finding, not a nit" — does not apply: the identical predicate lands in both SKUs in one diff (:391, :1090), each with its own behaviour fact; Lite's missing practice_of filter is pre-existing and correct (DuckDbInvestigationStore.cs:146).
• "Two copies of one definition pinned EQUAL to each other fail only after BOTH have drifted (#3684). Alias, do not copy." — does not apply: per-dialect twins (config.config_alert_log vs v_config_alert_log, interval '2 days' vs INTERVAL 2 DAY), nothing asserts them equal, and both moved here.
• "Ruling (#5833): tests are load-bearing … A trap is answered with a behaviour test that fails on the defect and passes on the fix" — applies and is answered in shape, but half-delivered: the Darling half does not compile (:198).
• "A test that encodes the lie … Ask what defect a test would fail ON." — applies to the Tests section and survives: both facts fail on the clock-anchored window the earlier head shipped. Neither fails on :391 itself, since that conjunct is redundant — which the body does not claim; it calls them revert proof.
• "A retention-tiered store answers a 7-day question from a 4-day raw tier and labels the missing days quiet unless the read routes by tier (#3590)" — no longer applies, where the prior head's fixed window made it apply in shape: the bound now comes from the candidates' own data, and Lite reads live ∪ archive through v_config_alert_log (:1087).
• "A test that reads the clock twice measures the CI scheduler … One anchor owns both ends of an interval (#3676)" — does not apply: T0 is read once into a static readonly (:47) and the new fact derives alertAt, the clear and _now from it alone.
• "hours_back, limit, top, days_back … never clamped" / "ORDER BY 3 + 4 DESC" / integer division in a rate column / a percent whose denominator is the page / truncation inferred from a full page / pg_relation_size — do not apply: one timestamp > conjunct and three unchanged projected columns, no caller window ($1 is the service clock, :1572), no ordering, arithmetic, share or LIMIT.

Divergence (blind read vs PR body)

The safety argument is now correct and stronger than stated: :391 is not merely "drops no candidate", it is implied by the line above, so it is provably result-preserving. But the headline ("no longer reads the whole 90-day log") disagrees with the body's own table three sections later, which reports a seq scan for this head. And the saving is data-dependent in exactly the direction the design protects: min(last_alert) is a global minimum, so one live follow-up carrying weeks-old copied alert rows (:271, MaxFollowUpDays = 30) pulls the bound back near the retention horizon for every candidate; the body concedes weak follow-up coverage for the EXCEPT ALL comparison but not for the 5x.

The sharper divergence is "Built on macOS: Darling.Tests …, 0 errors" against :198, which cannot compile against the only PgAlertHistoryStore constructor in the tree. That also undoes the answer to the previous round: "A test per store is added" is true as source and false as a running fact, and the cost is every Darling test in the build job, not the new fact alone.

Reviewer checklist

• Fix :198 (pass DarlingAlertReadAdapter.AlertPassCommandTimeoutSeconds), build Darling.Tests, then run it with DARLING_TEST_PG set and confirm the new fact passes rather than skips.
• Run Lite.Tests on Windows: the new Lite fact is the only check that DuckDB accepts an uncorrelated scalar subquery in a LEFT JOIN … ON at all, and nothing here could execute it.
• EXPLAIN ANALYZE the Lite statement on a store whose v_config_alert_log has a Parquet archive, and see whether the min() subquery pushes into read_parquet or sits above the scan.
• Re-run the Darling EXPLAIN (ANALYZE) on a store holding a live follow-up with alerts weeks old, to see what the bound is worth when min(last_alert) is old rather than minutes back.

Claude posting for Erik Darling. Both findings fixed: the constructor call in 400ef1f, the read claim in the body.

[claude]

This comment still carries the mechanism your own round-2 note records as refuted. The body says: "'No longer reads the whole 90-day alert log' (REFUTED): real. The bound is implied by the per-row join condition, so the same rows qualify and the log is still scanned. The opening line and the CHANGELOG entry now say the saving is per-row work, not rows read." — but the correction stopped at the body and the CHANGELOG. The shipped comment still says "the join stops reading the whole 90-day log", and your measurement table says the new form's plan on the log is a seq scan (the same plan as the old form; only the fixed-window variant got the index scan). The next person to read line 372 will believe this statement bought a narrower read, and will not know the saving is the strpos test and the join on rows the bound excludes.

Two smaller things in the same parenthesis:

• "about 60 ms a minute" is the cold delta (76 → 13.2). This statement runs once a minute, so the steady-state saving is the warm pair, 56 → 11.9 ≈ 44 ms. A measurement in a comment in this codebase is load-bearing (CONTRIBUTING: "This codebase's comments carry measurements"), so it should be the number the sweep actually sees.
• The bound is min(last_alert) over every live candidate, and a live follow-up can carry a last alert up to MaxFollowUpDays = 30 days old (PerformanceMonitor.Common/Investigations/InvestigationReportFields.cs:38) — the very case the second half of this comment cites. So the bound is not "recent"; it degrades to ~30 days whenever a follow-up is live. Your body is honest that the measured store "held one follow-up request, so that comparison has weak follow-up coverage"; the comment should say the same, since that is what decides how much the bound removes.

Suggested wording for the two lines:

Suggested change

	The bound on h.alert_time is the oldest last alert among the candidates, so the join stops reading the whole 90-day
	log (about 60 ms a minute on a store with 30,000 alert rows). It drops no candidate: every resolution row a candidate
	The bound on h.alert_time is the oldest last alert among the candidates. The plan on the log is still a seq scan
	(measured, PostgreSQL 18, ~30,000 alert rows): what it saves is the per-row work — the Resolves text test and the
	join — on every row below the bound, about 44 ms of a 56 ms warm pass. How much it removes depends on the oldest
	candidate: a live follow-up carries its source's alert times, so with one of those open the bound sits up to
	MaxFollowUpDays (30) back and the saving shrinks. It drops no candidate: every resolution row a candidate

[claude]

Same refuted mechanism as the Darling twin, and here it is stronger than anything measured. "keeps the join from reading the whole alert log, the archive included, every pass" asserts two things the tree cannot support:

1. Rows read. Your round-2 note concedes the bound is implied by h.alert_time > m.last_alert, so the same rows qualify and the engine still scans the log. The saving is the per-row strpos test and the join, not a narrower read.
2. The archive. v_config_alert_log is live UNION ALL read_parquet(...) (Lite/Database/DuckDbInitializer.cs:3394-3405). The bound is an uncorrelated scalar subquery, not a literal — the same reason you give for PostgreSQL not picking the time index — so there is no basis in this tree for saying it keeps the parquet files from being read, and the body says plainly that "Lite is not measured: DuckDB is not available to measure here." Claiming the archive specifically is the one sentence a Lite reader would act on, and it is the one with no measurement behind it.

Suggested change

	The bound on h.alert_time is the oldest last alert among the candidates, which keeps the join from reading the whole
	alert log, the archive included, every pass. It drops no candidate: a resolution row a candidate can use is after that
	The bound on h.alert_time is the oldest last alert among the candidates, which spares the join and the Resolves text
	test on every row below it. It is not measured on DuckDB (the equivalent PostgreSQL statement still seq-scans its log,
	so the saving is the per-row work, not the rows read, and the archive's parquet files are still opened). It drops no
	candidate: a resolution row a candidate can use is after that

[claude]

Non-blocking, and the revert proof you state is right: with $1 - interval '3 days' the clear at T0-9d falls outside the window, context_json reads NULL, and the sweep resolves 0 — this test and its Lite twin both fail. That shape is covered.

The one word the whole statement turns on is still uncovered, though: min. I walked every resolve test in both SKUs against a max(last_alert) bound and none of them fails.

• Here, m holds one row, so min = max.
• Resolved_WaitsForEveryLinkedMetric_AndARefireIsNotAClear: m is High CPU@T0 and Memory Pressure@T0+2m, so max = T0+2m; the High CPU clear is at T0+3m and clears the tighter bound anyway.
• Lite's ARequestIsResolved_OnlyWhenEveryLinkedMetricHasAResolutionRowAfterItsLastAlert_OnItsOwnServer: m is Blocking@T0 and Blocking Wait@T0+2m; both clears (T0+5m, T0+7m) are past max.

Nothing in either suite puts a clear between the oldest and the newest candidate's last alert, which is the only place a too-tight global bound shows up. A max-for-min edit — or anything else that lifts the bound above the oldest candidate — silently stops resolving those requests and the suite stays green. One extra metric in this test would close it: join a second metric at T0+2h (so it is the newest candidate) and log its clear at T0+3h, keeping High CPU's clear at alertAt.AddDays(1), which sits below the newest candidate's last alert. That fails on max and on the fixed window, and passes on min.

[claude]

Verdict: CHANGES REQUESTED

Claims

• VERIFIED — Darling: candidate list is now a CTE m, join adds h.alert_time > (SELECT min(last_alert) FROM m) — InvestigationRequestStore.cs:378,391; projection and order unchanged, so SweepResolvedAsync's three positional reads (:1577-1586) still hold.
• VERIFIED — Lite: the same over v_config_alert_log — DuckDbInvestigationStore.cs:1078,1090; the view is live UNION ALL read_parquet(...) (DuckDbInitializer.cs:3394-3405).
• VERIFIED — "the bound is implied by the per-row join condition" / "drops no candidate" — min(last_alert) over m is <= every row's m.last_alert, so :391/:1090 is entailed by :390/:1089. NULLs agree: an all-NULL m makes both predicates NULL. The timestamp/timestamptz mix (PgMigrations.cs:5011 vs InvestigationTables.cs:125) converts both predicates identically, so the entailment survives it.
• VERIFIED — a follow-up copies its source's alert times and follow_up_at may be 30 days out — InvestigationRequestStore.cs:271 passes alert_time through; MaxFollowUpDays = 30 (InvestigationReportFields.cs:38), refused not clamped (:105).
• VERIFIED — revert proof — request opens T0, alerts aged to T0-10d, clear T0-9d, clock T0+1m: a $1 - 3 days bound excludes the only resolution row, the LEFT JOIN yields NULL, Resolved is 0 (InvestigationRequestEventsLiveTests.cs:199-211, LiteInvestigationRequestEventsTests.cs:123-128).
• VERIFIED — the build break is fixed "like the file's other nine call sites" — ctor is (NpgsqlDataSource, int, ILogger?) (PgAlertHistoryStore.cs:41); the new site (InvestigationRequestEventsLiveTests.cs:198) passes AlertPassCommandTimeoutSeconds, and the file holds ten such sites.
• UNVERIFIED — the measurement table and "EXCEPT ALL both ways gave 0 and 0" — needs the live store. The direction fits the schema: idx_config_alert_log_time is (server_id, metric_name, alert_time) (PgMigrations.cs:5026), so alert_time sits behind an unconstrained metric_name and cannot bound an index range for this join.
• UNVERIFIED — "Lite is not measured", "PostgreSQL 16 and 17 are not measured" — stated absences. Whether either engine accepts a scalar subquery over a multiply-referenced CTE in a LEFT JOIN ON clause is also unverifiable here (no build on this box); a refusal would red every resolve fact rather than pass quietly.
• REFUTED — "The opening line and the CHANGELOG entry now say the saving is per-row work, not rows read", offered as the correction of that refuted claim — the correction stopped at the body. InvestigationRequestStore.cs:372 still ships "the join stops reading the whole 90-day log" and DuckDbInvestigationStore.cs:1073 still ships "keeps the join from reading the whole alert log, the archive included, every pass" — the Lite one claiming a parquet saving on the SKU the body says is unmeasured. Inline comments carry wording.
• REFUTED — "about 60 ms a minute on a store with 30,000 alert rows" (:373) — that is the cold delta (76 - 13.2). Once a minute means the warm pair: 56 - 11.9, about 44 ms.

Riskiest lines

1. Darling/PerformanceMonitor.Darling.Storage/InvestigationRequestStore.cs:391 — the whole change; a bound above the oldest candidate silently stops resolving requests. Covered for the fixed-window shape by Resolved_CountsAClearOlderThanThreeDays_ForARequestWhoseAlertsAreOlder. UNCOVERED for min swapped to max: no fact in either suite puts a clear between the oldest and newest candidate's last alert (walked inline).
2. Lite/Database/DuckDbInvestigationStore.cs:1090 — same predicate, right side spanning live rows and parquet. Covered for the fixed-window shape by ARequestWhoseAlertsAreOlderThanThreeDays_IsResolvedByAClearAfterThem; UNCOVERED for the min-to-max shape.
3. Lite/Database/DuckDbInvestigationStore.cs:1078 and InvestigationRequestStore.cs:378 — the derived table becomes a CTE referenced twice, a materialized optimization fence in PostgreSQL that it was not before. Covered by ARequestIsResolved_OnlyWhenEveryLinkedMetricHasAResolutionRowAfterItsLastAlert_OnItsOwnServer, ARealertAfterTheResolutionRow_KeepsTheRequestLive and Resolved_WaitsForEveryLinkedMetric_AndARefireIsNotAClear, each failing if the candidate set changes or the engine refuses the statement.

Traps near this diff

• "Strip the DateTimeKind before binding a timestamp parameter ... Npgsql infers timestamptz from a Utc or Local kind, PostgreSQL then converts into the session's time zone on the way into a naive column, and the row lands at the wrong hour with no error anywhere" — does not apply. AddWithValue("t", alertAt) (InvestigationRequestEventsLiveTests.cs:204) targets alert_time timestamptz (InvestigationTables.cs:125), not a naive column. The one mixed comparison (naive config_alert_log.alert_time, PgMigrations.cs:5011) is pre-existing and pinned to UTC in production (DarlingStoreConnection.cs:58); the test's one-day gap absorbs any real offset anyway.
• "Lite and Darling ship in parity: a fix to one SKU's reader, tool, alert or description without the counterpart's is a finding, not a nit" — applies, satisfied for the code (both copies get the CTE and the bound, each gets a behaviour fact; Darling's practice_of prefilter is pre-existing and documented at DuckDbInvestigationStore.cs:146). It is the descriptions that diverge: both comments carry the refuted rows-read mechanism and Lite's adds the unmeasured archive claim, so the wording fix is needed in both files.
• "Never answer a trap with a test that reads source text or asserts how code is spelled ... A trap is answered with a behaviour test that fails on the defect" (#5833) — applies, satisfied. ResolveCandidatesSql is internal const and no test reads its text (grep: two definitions, two call sites, one <see cref>); both new facts assert the sweep's Resolved count.
• "hours_back, limit, top, days_back are refused when out of range, never clamped — a clamp is a silent lie about the window (#3699)" — does not apply. No caller window reaches this statement; the only window is the fixed $1 - interval '2 days' on opened_at (:384, :1083), unchanged, and the new bound comes from the data.

Divergence (blind read vs PR body)

One, and it is the verdict. The body's round-2 entry treats "no longer reads the whole 90-day alert log" as a refuted claim it has corrected, but the correction landed only in the body's opening line and the proposed CHANGELOG entry. The two comments this PR adds still state it (InvestigationRequestStore.cs:372, DuckDbInvestigationStore.cs:1073), so the artifact that outlives the PR carries the mechanism your own EXPLAIN contradicts, and the Lite half extends it to a parquet archive on the SKU the body says is unmeasured.

Two smaller gaps the same way, both inline: "about 60 ms a minute" is the cold delta where the once-a-minute pass is the warm one; and neither comment admits what the body does, that min(last_alert) is pulled back up to 30 days by any live follow-up — the very mechanism the comment's second half cites as the reason not to use a clock window — so the saving's size is contingent where the comment reads settled.

Otherwise the accounts agree: the predicate is entailed by the one above it, the change is confined to those two statements, both callers read the same three columns in the same order, and the new facts do fail on the bound the earlier head shipped.

Reviewer checklist

• Run both new facts: Darling.Tests with DARLING_TEST_PG set (it self-skips otherwise, so a green leg without it proves nothing about the Darling half) and Lite.Tests on Windows. Both engines must first accept a scalar subquery over a multiply-referenced CTE in a LEFT JOIN ON clause — a bind-time question no read-only tree answers, and on DuckDB the one wholly unexercised thing here.
• EXPLAIN (ANALYZE) the Darling statement on a store holding a live follow-up whose alerts are genuinely weeks old, so min(last_alert) sits near its 30-day floor; the body concedes the measured store had weak follow-up coverage.
• Confirm the live plan evaluates (SELECT min(last_alert) FROM m) once as an InitPlan, not per outer row — the per-row shape turns the intended saving into a cost.

Claude posting for Erik Darling. Answered in 6cd73cb (both comments corrected) and the body's last Review round section.

[claude]

Verdict: LGTM

Claims

• VERIFIED — Commit only edits the comments above ResolveCandidatesSql in both stores, correcting the claim from "stops reading the whole log" to "the plan still scans the log; the bound saves the per-row work" — confirmed byte-for-byte: git diff 400ef1f5..6cd73cb32 touches only comment lines in Darling/PerformanceMonitor.Darling.Storage/InvestigationRequestStore.cs:372-378 and Lite/Database/DuckDbInvestigationStore.cs (same block); the ResolveCandidatesSql string constants are unchanged in both files.
• VERIFIED — Comment now notes "a live follow-up can pull the oldest last alert back by up to 30 days, so the saving is smaller while one is open" — consistent with the prior commit's (4c6ed3c) change of the bound from a fixed 3-day window to (SELECT min(last_alert) FROM m), which is exactly the mechanism that lets a follow-up's older last_alert widen the scanned range.
• UNVERIFIED — The "about 44 ms a pass... warm" and "the saving was measured on Darling's PostgreSQL store only" figures — these are live measurements (EXPLAIN ANALYZE / benchmark runs) that can't be checked from a read-only tree.

Riskiest lines

1. Darling/PerformanceMonitor.Darling.Storage/InvestigationRequestStore.cs:372-378 — comment-only; no behavior change, so no defect surface. UNCOVERED (not applicable — no code changed)
2. Lite/Database/DuckDbInvestigationStore.cs:1070-1076 — comment-only; same SQL constant unchanged. UNCOVERED (not applicable — no code changed)
3. n/a — no third risky line; the commit touches no code path, only prose.

Traps near this diff

• None of the SQL/collector/test traps in .github/REVIEW-TRAPS.md apply: this commit changes no SQL, no collector, and no test — it corrects the accuracy of a code comment describing an already-reviewed, unchanged query (the dynamic bound landed in commit 4c6ed3c, reviewed separately).

Divergence (blind read vs PR body)

None — the commit message ("per-row work saved, not rows read; 44 ms warm; follow-ups shrink the saving") exactly describes the diff: a documentation correction to two comments, no SQL or logic change in either file.

Reviewer checklist

• Spot-check the "44 ms a pass (warm)" figure against a fresh EXPLAIN (ANALYZE, BUFFERS) on a store with ~30,000 alert rows, since a read-only tree can't confirm a live timing claim.