Lite gets the Query Store backfill off switch (#2167 parity)

[erikdarlingdata]

What

Completes #2167. Darling got the switch as a store column in #2168 (V58); Lite now gets it as a Settings checkbox — "Fill Query Store history gaps in the background", default on, persisted to settings.json as query_store_backfill_enabled.

Two details that aren't obvious

The switch is checked BEFORE the due-time stamp. If it were checked after, a disabled backfill would still consume its own schedule, and flipping it back on would wait out an interval that elapsed while it was off. Checked first, re-enabling runs on the next due tick.

Read live, not captured. The due-check reads App.QueryStoreBackfillEnabled each pass, so the checkbox takes effect without restarting Lite — the same responsiveness Darling gets from its store reload. A captured bool would have quietly required an app bounce, which is the kind of thing nobody notices until they're mid-incident wondering why the switch did nothing.

Transition-logged once, not once per idle tick, seeded true to match the default so a deployment that never touches the switch says nothing about it in the log.

Why the two apps differ in surface

Darling's is a store column because a headless service has no window to click, and its operator may be nowhere near the box. Lite's is a checkbox because it has a settings window and a single local user. Same behavior, appropriate surface each — noted in both places so the asymmetry reads as deliberate.

Testing

Lite builds clean. The behavior is a two-branch gate on a settings bool in a background loop; there's no seam to unit-test that wouldn't just be asserting if (!flag) return; back to itself. The Darling half's store/probe/gate plumbing is pinned in BackfillSwitchTests from #2168.

Closes #2167 (its Darling half shipped in #2168).

[claude]

Nit: this checkbox (and its description) is grouped under "Automated Analysis" ("Periodically run the triage engine and record findings in the Alerts tab..."), but Query Store backfill has nothing to do with the analysis/triage engine — it's a data-collection concern. On the Darling side this same control sits next to "Capture execution plans" in the collection-related group (Darling/PerformanceMonitor.Darling.Viewer/SettingsWindow.xaml:85), which is the more discoverable placement. Consider moving this checkbox (and the matching App.xaml.cs:180 field, which is likewise sandwiched between AnalysisEnabled and AnalysisNotificationsEnabled) out of the analysis grouping so it doesn't read as an analysis-engine toggle.

[claude]

Review

Went through the diff against the Darling implementation from #2168 for parity, and traced the due-check/switch/log logic in CollectionBackgroundService.cs.

Correctness / parity: This holds up. The core logic is sound:

• The switch check happens before the _lastQueryStoreBackfill due-time stamp is touched, so a disabled backfill doesn't consume its own schedule — confirmed this matches the PR description's claim (re-enabling runs on the next due tick because the stale timestamp guarantees the interval check passes immediately).
• App.QueryStoreBackfillEnabled is read live (not captured) each tick, matching Darling's store-reload responsiveness.
• The transition-log-once behavior (_queryStoreBackfillWasEnabled) mirrors Darling's lastEnabled pattern in DarlingWorker.RunQueryStoreBackfillLoopAsync, including the seed-to-true default so a deployment that never touches the switch stays silent.
• Settings load/save round-trip (App.xaml.cs, SettingsWindow.xaml.cs) follows the exact pattern used by every other boolean setting in this file — read with TryGetProperty/GetBoolean, written unconditionally, checkbox bound both ways.
• CHANGELOG entry is appended to the existing [FEATURE] Query Store backfill needs an off switch (global service toggle, hot-applied) #2167 bullet rather than duplicated, consistent with how this file documents Lite/Darling pairs elsewhere.

No SQL in this diff (pure C#/XAML), no injection/secrets/network surface touched, and no missing-index-DMV territory to worry about.

One nit posted inline: the new checkbox and its backing field are grouped under "Automated Analysis" in both the XAML and App.xaml.cs, but Query Store backfill is a collection concern, not an analysis/triage concern — Darling puts the equivalent control next to "Capture execution plans" instead. Non-blocking, just a discoverability point.

Didn't find anything else worth flagging — this is a small, self-contained parity fix and it reads like one.

[erikdarlingdata]

Taken — and the diagnosis of how it happened is exactly right. I modeled the wiring on AnalysisEnabled (App static + settings.json key + checkbox) and inherited its section along with its pattern, so a data-collection control ended up under Automated Analysis, reading as an analysis-engine toggle in the one place nobody would look for it. The App field was wedged between the two analysis flags for the same reason.

Moved to the Collection Control section, beside Pause Collection — the other "should collection do this" affordance — which is also where Darling puts its equivalent, next to plan capture. Field re-homed out of the analysis block too, with a comment recording why, so a future copy-the-pattern edit doesn't drift it back.

[claude]

Minor organizational nit: this property (and its doc comment) landed directly under the /* System tray settings */ header, ahead of MinimizeToTray — the actual first tray setting. QueryStoreBackfillEnabled isn't a tray setting, so as written it reads as grouped under that section.

This is the same class of mistake the PR's second commit deliberately fixed for the checkbox's XAML placement ("Put the backfill checkbox where it belongs (review catch)") — worth moving this property above the /* System tray settings */ comment (or giving it its own section comment) so the C# side gets the same care the XAML side got.

[claude]

Reviewed the diff (App.xaml.cs, CollectionBackgroundService.cs, SettingsWindow.xaml/.xaml.cs, CHANGELOG.md).

Summary: Solid parity implementation. Compared against Darling's V58 backfill switch (DarlingWorker.cs/StoreConfigProvider.cs/DarlingConfig.cs):

• Default-on true, matches Darling's store default.
• Live read via App.QueryStoreBackfillEnabled (not captured) — matches Darling's store-reload seam, takes effect without a restart.
• Switch checked before the due-time stamp update, so a disabled backfill doesn't consume its own schedule and re-enabling fires on the next due tick — same reasoning as Darling's loop, adapted correctly to Lite's IfDue-ladder shape (where Darling has a dedicated loop with its own Task.Delay, Lite reuses the shared ladder, and the early-return-without-updating-the-timestamp is the right way to get equivalent behavior there).
• Transition-only logging (_queryStoreBackfillWasEnabled, seeded true) mirrors Darling's lastEnabled pattern — no per-tick log spam, silent on a deployment that never touches the switch.
• Settings load/save/persist-to-JSON follows the exact same three-touchpoint pattern as AnalysisEnabled (LoadAlertSettings, save handler, settings.json write/read in App.xaml.cs).
• XAML: the <StackPanel> wrapping the Collection Control Border's contents is well-formed (tag counts balance), and the second commit correctly relocated the checkbox out of the Automated Analysis section per the review catch noted in the commit message.

One nit (posted inline on Lite/App.xaml.cs:206): the new QueryStoreBackfillEnabled property landed directly under the /* System tray settings */ comment header, ahead of MinimizeToTray (the actual first tray setting) — so it now reads as grouped under a section it doesn't belong to. Same class of mistake as the one the PR's second commit deliberately fixed for the XAML placement; worth the same fix here.

No correctness, security, or Lite/Darling parity issues found otherwise.

[erikdarlingdata]

Fair hit, and worse than a nit given the context: I fixed the XAML placement, wrote a commit message about not letting copy-the-pattern drift the grouping, and then landed the C# field under /* System tray settings */ — ahead of the actual first tray setting. Same error, same PR, one commit apart.

Now under its own /* Collection settings */ header with the tray header restored to MinimizeToTray. The general lesson I'm taking: when relocating something, read the destination's surroundings rather than anchoring on the nearest textual match — both instances came from pattern-matching an insertion point instead of looking at what was already there.

[erikdarlingdata]

Round-3 build failed, and it was NOT this PR — but it blocked it, so the fix rides along in e22684ff (filed separately as #2177).

Two AnomalyDetectorTests thin-baseline tests fail deterministically for CI runs between 03:39 and 04:00 UTC. My push landed at 03:55. The seed helpers write ~21 minutes of samples starting at whatever time-of-day _analysisStart inherits from DateTime.UtcNow; when that start falls within 21 minutes of midnight the span crosses a date boundary, so each intended 'day' contributes two dates — 4 instead of 2 — which clears the Full tier's 3-distinct-day minimum, makes a deliberately-thin baseline trustworthy, and routes the detector down the z-path. baseline_low_quality then reads 0 where the test requires 1.

Fixed by flooring the seed start to the hour. Deliberately NOT midday-anchoring (the #1972 discipline used elsewhere in the suite) because the Full tier buckets by hour AND day-of-week — the seeds have to keep _analysisStart's hour and weekday to land in the bucket the analysis window reads. Starting at :00 preserves both and makes a 21-minute span unable to leave the hour. Applied to all seven seed helpers, not just the two that failed, since they all share the shape.

Worth noting: a rerun after 04:00 UTC would have passed and hidden this permanently.

[claude]

Reviewed. This is a clean, well-scoped port of the #2167 backfill off-switch to Lite, and it holds up well against Darling for parity:

Parity check — good:

• Setting key query_store_backfill_enabled matches Darling's config_service.query_store_backfill_enabled column name exactly.
• Gate-before-schedule-stamp logic in RunQueryStoreBackfillIfDueAsync (App.xaml.cs / CollectionBackgroundService.cs) correctly avoids consuming the due-interval while disabled, mirroring the intent of Darling's RunQueryStoreBackfillLoopAsync.
• Live read via App.QueryStoreBackfillEnabled (not captured) matches Darling's store-reload seam — checkbox takes effect without an app restart.
• Transition-only logging (_queryStoreBackfillWasEnabled, seeded true) mirrors Darling's lastEnabled pattern, including "seeded true so an untouched default logs nothing."
• Settings load/save/persist touchpoints for the new field (App.xaml.cs declaration, JSON load, SettingsWindow load-into-UI, save-from-UI, save-to-JSON) all present — same 5 touchpoints as the existing AnalysisEnabled flag it was modeled on.
• XAML checkbox placement (Collection Control group, beside Pause Collection) and copy correctly mirror where Darling put its equivalent control, appropriately adapted for the single-window/no-store-column context.

One thing worth a mention: Lite.Tests/AnomalyDetectorTests.cs carries an unrelated fix (SeedDayStart, tagged #2177) for a midnight-boundary flaky-test bug in the baseline seeding helpers. The fix itself looks correct (floors to the hour, preserving the hour/day-of-week bucket the Full baseline tier reads, per BaselineMath.cs), but it's outside this PR's stated scope (#2167/#2168 parity) and isn't mentioned in the PR description. Worth calling out explicitly in the description (or splitting to its own PR) so it doesn't read as accidental scope creep during review.

No correctness, security, or performance issues found in the core change.