[erikdarlingdata]

Closes #2495.

The convention, and whether I agree with option 2

I agree with option 2, and I'd go further: option 1 is not just wider, it's the wrong shape for this surface.

hours_back is not a window — it is half a window plus an implicit end = now. Option 2 doesn't add a knob so much as it makes the existing knob honest: hours_back is the SPAN, as_of is where the span ends. Every read keeps exactly one time concept it already had, plus one anchor.

Adding from/to would have given ~100 reads three time parameters with a validity rule between them (to > from, and what does hours_back mean when both are set?). That is three states to document, three to test, and one — from/to/hours_back all supplied — that has no defensible answer. And it buys almost nothing: the real question is always "N hours around a known time", which as_of answers exactly.

Option 3 I reject on the issue's own argument, which the tests now make concrete: widening hours_back returns a different aggregate, not a superset.

So: every read that already takes a window takes as_of. hours_back keeps its meaning. Reads that take no window don't get one.

What changed

Shared — McpHelpers.ResolveAsOf parses the anchor, McpHelpers.ValidateWindow(hours_back, as_of, out end) replaces the bare ValidateHoursBack at every windowed read, and McpHelpers.AsOfDescription is the single description string both SKUs' [Description] attributes point at, so the two servers cannot drift into describing the same parameter differently.

Reads that gained as_of — 57 on Darling, 50 on Lite (the difference is the 8 Darling-only PostgreSQL reads, plus the known get_blocking ↔ get_blocked_process_reports naming pair and get_cpu_scheduler_pressure, which is windowed on Lite and unwindowed on Darling — a divergence that predates this):

The full list

get_active_queries, get_alert_history, get_blocked_process_reports (Lite) / get_blocking (Darling), get_blocked_process_xml, get_blocking_stats, get_blocking_trend, get_health_parser_significant_waits, get_procedure_duration_trend, get_query_store_duration_trend, get_query_store_regressions, get_collection_log, get_cpu_scheduler_pressure (Lite), get_cpu_utilization, get_current_waits_trend, get_database_config_changes, get_deadlock_detail, get_deadlock_trend, get_deadlocks, get_default_trace_events, get_file_io_trend, get_health_parser_cpu_tasks, get_health_parser_io_issues, get_health_parser_memory_broker, get_health_parser_memory_conditions, get_health_parser_memory_node_oom, get_health_parser_scheduler_issues, get_health_parser_severe_errors, get_health_parser_system_health, get_latch_stats, get_long_query_completions, get_memory_grants, get_memory_pressure_events, get_memory_trend, get_perfmon_trend, get_pg_autovacuum_health, get_pg_blocking, get_pg_io_stats, get_pg_replication_slots, get_pg_top_queries, get_pg_wait_stats, get_pg_wraparound_risk, get_pg_xmin_horizon, get_plan_cache_bloat, get_plan_corrections, get_query_duration_trend, get_query_store_top, get_query_trend, get_resource_semaphore, get_server_config_changes, get_spinlock_stats, get_tempdb_trend, get_top_procedures_by_cpu, get_top_queries_by_cpu, get_trace_flag_changes, get_wait_stats, get_wait_trend, get_wait_types, get_waiting_tasks

Reads left alone, and why. Two rules, both stated in the instructions and both pinned by a test that names the exclusions explicitly, so removing one from the list has to be deliberate:

1. No window, nothing to anchor. get_memory_stats, get_file_io_stats, get_memory_clerks, get_index_usage, get_object_locking, get_table_index_sizes, get_database_sizes, get_server_properties, get_session_stats, get_running_jobs, get_collection_health, get_server_summary, get_ag_health, get_perfmon_stats, get_cpu_scheduler_pressure (Darling) and the configuration reads all return the newest row. Giving them as_of would mean turning a latest-row read into a windowed one, which is a different feature and a different answer.

2. Half-windowed reads. get_pvs_stats and get_fleet_overview return a latest-snapshot measurement and a windowed one in the same payload — PVS's current size next to a trend; the fleet cards' current CPU/memory/threads next to windowed blocking and deadlock counts. Anchoring only the windowed half returns a result whose two halves describe different instants, which is worse than not offering the parameter. get_plan_corrections looks like this case and isn't: its unwindowed component is per-database FORCE_LAST_GOOD_PLAN configuration, not a measurement, in the same class as the CPU count get_top_queries_by_cpu reads for its attribution denominator.

3. The analysis family — analyze_server, get_analysis_facts, compare_analysis, get_analysis_findings — computes its window inside the analysis engine from a bare hours_back, so anchoring it is an analysis-engine change on a shared seam, not a read-surface one; analyze_server also persists what it finds. Deferred rather than dropped: filed as The analysis family cannot be anchored at a past window, and it is the one an incident starts with #2506, including the persistence decision that needs making rather than coding.

Web surface. PAsOf() on all 57 Darling descriptors and as_of: AsOf(c) in all 57 dispatch lambdas. Two details worth review:

• the dispatch argument is named, not positional — several tools have trailing optional parameters the dispatch omits (dedup_key on the blocking reads, group_by on get_top_queries_by_cpu), and a positional append would have bound the anchor to the wrong one;
• the anchor is passed through to the tool unvalidated, so the parse and the refusal message live in one place and the bare string lands on the existing 400 mapping.

The wire key is as_of on both surfaces — no alias, so ServerPageTabsTests' canonical-time-key rule is untouched.

Three reads that bounded only their lower edge now bound both: get_cpu_utilization, get_tempdb_trend, get_alert_history (and their Lite twins). For alert history this is a correctness fix independent of the anchor — its row cap is applied by the database, so trimming after the read would have spent the whole LIMIT on rows newer than the anchor and returned an empty window that looks exactly like a quiet one. The two SQL constants renumber; their pins moved with them.

For an unanchored call the closed edge is now, which is what every other windowed read on the surface already used — so this makes those three consistent rather than special. The only rows it can exclude are ones written between the tool computing now and the query running, which is the semantics the rest of the surface has always had.

Lite plumbing. LocalDataService.GetTimeRange and GetTimeRangeServerLocal take an optional asOfUtc, threaded through the 48 windowed methods the MCP reads call. Deliberately not routed through the existing fromDate/toDate pair: those are server-local (converted back to UTC inside the helper) while the anchor is UTC, so reusing them would silently shift every window by the monitored server's offset. The two helpers keep their own time bases and both take the anchor in UTC.

Backward compatibility

• ValidateWindow(h, null, out _) is asserted equal to ValidateHoursBack(h) across {-1, 0, 1, 24, 168, 169}, so a caller that sends no anchor gets the same validation and the same message.
• The anchor defaults to DateTime.UtcNow, i.e. every existing call site computes the identical window.
• Live and DuckDB tests both assert the default-anchor read still returns the recent window and not the old one.
• No tool was added or removed, so the instructions' tool census is unchanged and still arithmetically true.
• as_of is appended LAST on every signature — the convention group_by (Per-query CPU attribution unusable on a plan-churning instance: reads see 18 execs where Datadog sees 43% of the box; literal fragmentation defeats top-N #2235) and dedup_key ([FEATURE] Let the MCP incident readers filter by the alert fingerprint (Dedup Key) #2159) already set — so no shipped parameter moved.

One consequence worth naming rather than burying: for the four tools that carry a Darling-only trailing optional (get_top_queries_by_cpu, get_blocking, get_deadlocks, get_deadlock_detail), Lite's parameter list is no longer a prefix of Darling's — Darling reads …, min_dop, group_by, as_of while Lite reads …, min_dop, as_of. Three pins asserted that prefix, and I widened them to what the prefix was standing in for: every Lite name present in Lite's relative order, and every Darling-only extra optional. That is the guarantee those pins were written for ("a client written against Lite's contract still calls Darling correctly") and it still fails on a drop, a reorder, or a required Darling-only addition. It is not positional, and does not need to be: MCP invokes by name, and the only C# call sites — the /api/read dispatch — do not pass Darling's extra optionals at all. The alternative was relocating two already-shipped parameters to preserve a position no caller can observe.

Refusal semantics

Following ValidateTop — report, don't substitute.

The parser is an ISO-8601 allowlist (TryParseExact), not a general date parse. That was a review catch and a good one: DateTime.TryParse under the invariant culture also accepts 01/02/2026 as M/d/yyyy, so a caller who meant 1 February would have got a window around 2 January silently — the same defect as the fall-back-to-now this parameter exists to remove, one step removed. A space in place of the T is refused for the same reason: the message names the four forms that work, which is more useful than a fifth guess.

input	behaviour
absent / blank	window ends at now (unchanged)
2026-08-19T05:00:00Z, ...T05:00:00, ...+02:00, 2026-08-19	all resolve to one UTC instant; the offset-less form is read as UTC, never as the service host's local time
unparseable	refused, naming the value
in the future beyond 5 minutes	refused — a stored read cannot cover uncollected data
in the future within 5 minutes	accepted; that is the client-clock allowance, not a grace period for asking about the future
older than anything retained	accepted, returns the read's own empty / unavailable

That last row is the one I'd most like a second opinion on. A floor would have to guess at retention, which is per-deployment, per-server and per-collector, and would refuse legitimate reads on a long-retention store. The honest answer is the miss vocabulary, and it is unambiguous because the caller chose the window. The span stays bounded by ValidateHoursBack; only the anchor is free.

Tests

The pin that matters most, added after review found eight tools that advertised as_of, validated it, refused a bad one correctly — and then computed their window from DateTime.UtcNow anyway. All eight came from resolving a rebase in dev's favour; all eight compiled and passed a green 5,600-test suite. That is this issue's own defect one level up: the caller gets NOW labelled as their window, and the validation succeeding is what makes them believe it. So EveryToolThatTakesTheAnchor_ActuallyUsesIt asserts, over the shipped source of BOTH SKUs, that an anchored tool names no DateTime.UtcNow at all, uses the instant it resolved, and (Lite) passes asOfUtc to every LocalDataService read that accepts one — that set read off source, not transcribed, with a floor assertion so the scan cannot pass by parsing nothing.

Worth recording how close that pin came to being useless. Its first form asserted "does not ASSIGN DateTime.UtcNow". I reintroduced the get_file_io_trend bug to watch it go red and it stayed green — the body still contained the word windowEnd from its own out var. A check that green-lights the defect it was written for converts an open question into confidence, so it is an absolute now, and I reintroduced one bug of each form and watched both fail before restoring.

Darling.Tests/AsOfWindowAnchorTests.cs

• ungated: the resolver's contract (four spellings → one instant, refusals, no lower bound, span-before-anchor ordering, and the ValidateHoursBack equivalence above);
• ungated: MCP ↔ catalog agreement in both directions, plus the convention pin — a windowed read with no anchor and not on the named exclusion list fails;
• [Collection("live-postgres")], DARLING_TEST_PG, cleanup through LiveStoreCleanup.RunAsync: the test that matters — wait_stats seeded 30 hours back, plus a recent row with a different wait type. The default read returns the recent type and not the incident; anchored at the incident the same read returns the incident and not the recent one; a widened 48-hour window returns both, with a different top-N, which is the issue's argument as an assertion rather than a claim. Then get_collection_log (where the gap was noticed) anchored and unanchored, both refusals, and the ancient-anchor unavailable.

Lite.Tests/AsOfWindowAnchorTests.cs — the same shape against a real DuckDB through the real tool methods, seeded under the derived server id (RemoteCollectorService.GetDeterministicHashCode(GetServerNameForStorage(server))), covering both window families: get_wait_stats for the UTC helper and get_cpu_utilization for the server-local one. That second one sets UTC+5:30 rather than reading the ambient offset — review caught that at offset 0 (which is what any normal machine gives) a correct conversion and no conversion are indistinguishable, so it would have passed vacuously for exactly the bug its summary names. It joins the existing server-time-helper collection because that static is process-wide.

What I could not verify

• The test suites do not run on macOS (net10.0-windows). Everything below builds clean, including a full solution build; CI is the arbiter for execution. I did verify the resolver's arithmetic locally by running the shipped McpHelpers from a throwaway net10.0 console harness — 18 assertions, all passing — which covers the parse/refusal/window logic but not the plumbing, and the plumbing is where this kind of change breaks.
• No live-Postgres or live-DuckDB run from here. The live assertions are written but have only ever executed in CI.
• The empty result does not echo the resolved window. A caller who sent an anchor knows what they asked for, so empty is unambiguous — but a shared payload field would make it self-describing. That would touch ~100 serialization sites and change every response shape, so I left it; happy to file it if you want it.
• Nine tools have no instructions table row at all (the eight PostgreSQL reads and get_plan_corrections) — pre-existing, not introduced here. They are covered by the new section's rule ("every tool below that takes hours_back also takes as_of") rather than by a row.
• Three reads validate the anchor but not the span — get_collection_log, get_current_waits_trend, get_blocking_stats call ResolveAsOf rather than ValidateWindow. Raised in review and deliberate: those three have never capped hours_back (they Math.Abs() it, no MaxHoursBack on the path), so the "free" swap would impose the 168-hour ceiling every other read carries and take reach away from the read this issue was reported against. The asymmetry is now commented at all six sites so the next reader meets the reasoning before the one-line cleanup.
• get_cpu_scheduler_pressure still diverges between SKUs — windowed on Lite, unwindowed on Darling. I anchored the Lite one because it has a window; I did not add a window to Darling's, which would be a separate behaviour change.
• Rebased twice onto a moving dev, which added five windowed reads while this was open: get_blocking_stats (Serve blocking severity, and share the deadlock aggregation (#2484) #2497), then get_query_store_regressions, get_procedure_duration_trend, get_query_store_duration_trend and get_health_parser_significant_waits (Four more reads that could not say which nothing they had found (#2485) #2505). All five took the anchor under the same rule as everything else. Two needed a decision rather than a substitution, and are worth a look in the diff: get_query_store_regressions threads the anchor into the private helper that explains an empty result (a window the probe computed for itself would be a different window than the one it is explaining), and Lite's get_blocking_trend already pinned one instant for its two reads — that instant is now the anchor, so both reads move together.

🤖 Generated with Claude Code

[claude]

Minor: DateTime.TryParse (vs. TryParseExact) is more lenient than the documented "ISO-8601" contract. With CultureInfo.InvariantCulture, ambiguous non-ISO strings like "08/18/2026" or "01/02/2026" parse silently as M/d/yyyy instead of being refused.

That's the same failure mode ResolveAsOf's own doc comment says this parameter exists to prevent — the caller gets an answer to a subtly different question than they asked, with no error (just one level removed from the "silently answers as now" case that is refused). It's also untested: the AnUnusableAnchor_IsRefused_NotSilentlyTreatedAsNow theory only covers "last tuesday", "2026-13-45", and "4 hours ago", not an ambiguous-but-parseable date.

Worth considering DateTime.TryParseExact against an explicit ISO-8601 pattern set (or DateTimeStyles.RoundtripKind + a format allowlist) to match the documented/tested contract exactly. Low severity — not a crash risk, and probably rare in practice for an LLM-driven caller — but worth a decision either way.

[erikdarlingdata]

Good catch, and the reasoning is right — fixed in de4ae1f.

The parser is now TryParseExact against an explicit ISO-8601 allowlist rather than a general TryParse:

"yyyy-MM-dd",
"yyyy-MM-ddTHH:mmK",
"yyyy-MM-ddTHH:mm:ssK",
"yyyy-MM-ddTHH:mm:ss.FFFFFFFK",

K covers the empty offset, Z, and ±HH:mm / ±HHmm, so one format serves all three documented shapes and the allowlist stays short enough to read.

On the severity: I think it is a bit higher than "minor", for the reason you gave — 01/02/2026 is not garbage that produces an error, it is a plausible value that produces a wrong window silently, which is the exact defect as_of exists to remove, one step removed. A refusal that names the working forms is strictly more useful than a guess.

Also deliberately NOT accepted now: a space in place of the T (2026-08-18 14:30:00). It is unambiguous, but the contract is "the documented forms", and the refusal message lists all four, so a caller who sends the space form is one round-trip from correct rather than one round-trip from a subtly different answer.

The theory gained the ambiguous cases you named plus the sibling shapes — 08/18/2026, 01/02/2026, 2026/08/18, 18 Aug 2026, 2026-08-18 14:30:00 — and the accepted-spellings theory gained T14:30Z, .000Z and a whitespace-padded value, so the allowlist is pinned from both directions. Lite's suite got the 01/02/2026 case too, since the parser is shared.

Verified locally against the shipped McpHelpers (the xUnit suites are net10.0-windows and cannot run on macOS): 29 assertions covering every accepted form, every refusal above, the round-trip "o" form both SKUs' tests emit, and the ISO basic-format offset +0200, which the allowlist accepts on purpose — it is unambiguous.

[claude]

Pre-existing Lite/Darling parity gap, surfaced (not introduced) by this PR: UpperEdge deliberately leaves the diff's upper bound at DateTime.MaxValue when no as_of is given, and the doc comment above explains why — "these snapshots are stamped by the collector, so a capture whose clock ran a little ahead of the service's would drop out of a window it belongs in."

Lite's equivalent (LocalDataService.GetServerConfigChangesAsync/GetDatabaseConfigChangesAsync/GetTraceFlagChangesAsync in Lite/Services/LocalDataService.ConfigChanges.cs) calls the shared GetTimeRange(hoursBack, fromDate, toDate, asOfUtc), whose no-anchor case returns endTime = DateTime.UtcNow — not an open upper edge — and passes that straight into ConfigChangeDiff.Diff*Changes(snapshots, startTime, endTime). So on Lite, a config-snapshot capture whose clock ran slightly ahead of the service's is exactly the case this comment says Darling avoids, but Lite doesn't.

This divergence predates this PR (Lite's GetTimeRange returned DateTime.UtcNow as the upper bound before as_of existed too), so it's not a regression here — but this PR is the first place the reasoning against it gets written down, and it applies equally to Lite. Might be worth a follow-up issue if it's not already tracked.

[erikdarlingdata]

You are right that the two SKUs diverge here, and right that this PR is where it first got written down — but the justification I wrote for it is wrong, which is the more useful half of this finding. Corrected in de4ae1f.

capture_time is stamped DateTime.UtcNow by the collector, in the same process that later reads it (Lite's own header comment on LocalDataService.ConfigChanges.cs says so, and Darling's collector does the same). So the clock-skew story I told cannot happen on either SKU: a snapshot can never carry a timestamp after the reading call's own now. I asserted a protection the code does not provide, which is exactly the kind of doc claim that outlives the person who wrote it.

With the skew argument gone, the divergence is real but unobservable: DateTime.MaxValue and now cannot select different rows, because no row can sit between them. So there is nothing to fix and nothing worth filing — an issue that says "two reads use different upper bounds that always return the same set" would be noise, and unifying them inside this PR would mean changing two shipped reads' behaviour in a change whose whole contract is that an unanchored read does not change.

What each SKU keeps is its own prior behaviour, which is the compatibility rule applied consistently rather than a rule applied twice differently. The comment now says that, and says why the two differ, so the next reader gets the real answer instead of mine.

[claude]

Review summary

Reviewed the full diff (81 files) against CONTRIBUTING.md's conventions, correctness, Lite/Darling parity, and security. This is pure C# (no .sql touched), so the T-SQL style rules don't apply here.

Overall: solid, careful, well-tested implementation. I traced the as_of anchor end-to-end on both SKUs — validation (McpHelpers.ValidateWindow/ResolveAsOf), the ~97 tool signatures, the reader/service layers, the web dispatch, and both new test suites — plus ran four independent deep-dive passes (Darling SQL/reader plumbing, Lite DuckDB plumbing, Lite/Darling parity, and McpHelpers/tests/style). Specifically verified and found correct:

• SQL injection: every new time bound is passed through parameterized queries (NpgsqlParameter/DuckDBParameter) on both backends — no string concatenation of as_of or any derived value into SQL text anywhere.
• The three named lower-bound-only fixes (get_cpu_utilization, get_tempdb_trend, get_alert_history) now correctly bind both edges on both SKUs, with parameter ordinals correctly renumbered.
• UTC vs. server-local handling: LocalDataService.GetTimeRangeServerLocal converts the UTC anchor to the server's local offset before use — it doesn't compare a UTC anchor against server-local timestamps directly, which would have silently shifted every window by the monitored server's offset.
• Web dispatch: the new as_of argument is passed as a named argument (as_of: AsOf(c)) everywhere, not positional — so it can't accidentally bind to a trailing optional like dedup_key or group_by.
• Backward compatibility: as_of is appended last on every signature, ValidateWindow(h, null, out _) is asserted equal to the old ValidateHoursBack(h), and the anchor defaults to DateTime.UtcNow.
• Parity of the as_of rollout itself: all tools called out in the PR description carry as_of symmetrically on both SKUs, off one shared McpHelpers.AsOfDescription constant (verified as a real shared reference, not two hand-typed copies) — no new naming/behavior drift introduced.
• Tests: both new AsOfWindowAnchorTests suites are genuine regression tests (seed before/after an incident window, assert content — not just row counts — differs between anchored/unanchored/widened reads), not smoke tests. The live-Postgres test skips gracefully without DARLING_TEST_PG and cleans up via LiveStoreCleanup.RunAsync in a finally.

Two minor findings posted inline, both low severity:

1. McpHelpers.ResolveAsOf's use of DateTime.TryParse is more lenient than the documented "ISO-8601" contract — ambiguous non-ISO strings could parse silently instead of being refused, which cuts against the parameter's own "refuse, don't substitute" design goal.
2. A pre-existing Lite/Darling divergence in how config-change diffs (get_server_config_changes etc.) bound their upper edge — surfaced (not introduced) by this PR's own new code comment on the Darling side, which doesn't apply to Lite's equivalent path.

No missing-index DMV suggestions, no security issues in input handling, and no performance regressions spotted — the added parameter is a pure pass-through with no new hot-path work.

[claude]

Minor, pre-existing gap this PR had a natural chance to close while it was already touching every one of these signatures: GetCollectionLog, GetCurrentWaitsTrend (line ~1155), and GetBlockingStats (line ~1224) call McpHelpers.ResolveAsOf(as_of, out var windowEnd) directly instead of McpHelpers.ValidateWindow(hours_back, as_of, out var windowEnd). That means hours_back is never validated here (no negative/MaxHoursBack check) — it only gets Math.Abs()'d — unlike the ~50 sibling reads that now route through ValidateWindow. Same pattern on the Lite side (Lite/Mcp/McpHealthTools.cs lines 281, 359, 422).

This isn't a regression — these three methods never validated hours_back before this PR either — so it's not blocking. But since the PR's whole premise is "every windowed read validates its window the same way," these three are now the only reads left where as_of is refused/validated but hours_back isn't, which is a small, visible inconsistency in the new convention. Worth a follow-up to swap these onto ValidateWindow for free, since the return value is unused either way when there's no error.

[erikdarlingdata]

Right that it is an inconsistency, and right that it is pre-existing — but the swap is not free, and that is worth pinning down rather than deferring.

Those three reads have never capped hours_back. They Math.Abs() it and window on the result, with no MaxHoursBack check anywhere on the path (Darling: DarlingMcpDataTools lines 1079 / 1161 / 1230 straight into the reader; Lite: Math.Abs into GetRecentCollectionLogAsync → GetTimeRange, no clamp). So get_collection_log(hours_back: 400) works today and returns 400 hours.

Routing them through ValidateWindow would refuse that, because ValidateHoursBack caps at 168. That is a functional regression on the read this issue was reported against, whose entire premise is looking further back than the default — inside a change whose contract is that an unanchored call does not move. Consistency of the validator is not worth buying with reach on the one tool that needs it.

So: the anchor is validated because it is new and has no prior behaviour to preserve; the span keeps exactly the behaviour callers have. That asymmetry is the point rather than an oversight, and I have written it at all six sites in 525ac10 so the next reader meets the reasoning before the one-line cleanup rather than after it.

Not filing a follow-up, because the conclusion is "leave it": the issue would say "make three reads worse for symmetry". What WOULD be worth filing, if anyone wants it, is the opposite direction — lifting the 168-hour ceiling off the reads that do carry it, now that as_of makes a narrow window at an arbitrary point in history the natural way to ask a historical question. That is a real design question about MaxHoursBack, not a cleanup, and it belongs to whoever decides what the store promises to retain.

[claude]

Review summary

This is a large but mechanically consistent PR (as_of anchor added to ~50 Darling reads / ~46 Lite reads plus the shared web dispatch), and it holds up well under review. I checked the shared resolver (McpHelpers.ResolveAsOf/ValidateWindow/AsOfDescription), the Darling web dispatch (DarlingWebEndpoints.cs), a representative cross-section of Mcp tool files on both SKUs (blocking, wait, query, CPU, health, session, config-history, PG family), the Lite LocalDataService time-range plumbing, and both McpInstructions.cs files, then had a second independent pass sweep every remaining touched Mcp/LocalDataService file for the same checklist. No T-SQL was touched by this PR (it's all C#), so the T-SQL style conventions don't apply here.

Correctness

• The ISO-8601 allowlist parser (TryParseExact with 4 explicit formats) correctly rejects ambiguous slash-delimited dates instead of silently misparsing them (01/02/2026 would otherwise resolve to the wrong date under DateTime.TryParse) — good catch already baked in.
• AssumeUniversal | AdjustToUniversal correctly reads an offset-less timestamp as UTC rather than the service host's local time, which is the right call for a UTC-only store read by remote agents.
• The 5-minute future-tolerance window is a sensible clock-skew allowance, not a loophole for asking about uncollected data.
• Every place I checked that reassigns var now = windowEnd (or end = windowEnd) uses that single value consistently for both the start and end of the query window — no half-anchored windows.
• SQL parameter renumbering (Postgres $1,$2,... and DuckDB positional params) is correct everywhere a new window-end bind parameter was inserted ahead of existing trailing parameters (e.g. LocalDataService.WaitingTasks.cs's BuildDbInClause shifting from $3 to $4).
• All as_of values are parsed to DateTime before ever reaching SQL and are otherwise passed through fully parameterized queries (Npgsql $n / DuckDB $n) — no injection surface. The web dispatch's AsOf(context) passes the raw string through unvalidated by design (the tool owns the refusal message), which is safe since it's parsed by the same strict allowlist before use.

Parity

• as_of is appended as the last parameter in every signature I checked, on both SKUs, preserving the documented "Lite parameter list is a prefix of Darling's" invariant for the four tools with Darling-only trailing optionals.
• The exclusion list (analyze_server, get_analysis_facts, get_analysis_findings, compare_analysis, get_pvs_stats, get_fleet_overview, get_store_metrics, and all latest-snapshot-only reads) is correctly excluded from as_of in the dispatch/catalog, and the documented Darling-only divergence on get_cpu_scheduler_pressure (windowed on Lite, not on Darling) is real and intentional, not an oversight.
• Both McpInstructions.cs files document the same set of tools with as_of that the code actually implements it for.

One minor, non-blocking finding (left as an inline comment): get_collection_log, get_current_waits_trend, and get_blocking_stats — on both Darling and Lite — call ResolveAsOf directly instead of the new ValidateWindow, so hours_back still isn't validated on these three reads (pre-existing gap, not a regression, but this PR had a natural opportunity to fold them in since it touches all three signatures anyway).

No security, performance, or parity-drift issues found beyond the note above.

[claude]

windowStart/windowEnd (already-correct UTC instants derived from the anchor) are passed positionally into GetBlockingTrendAsync's fromDate/toDate parameters. But GetTimeRange (LocalDataService.cs:117-123) treats fromDate/toDate as server-local whenever both are supplied, and subtracts ServerTimeHelper.UtcOffsetMinutes from them to convert back to UTC — even though these values are already UTC. The net effect: for any monitored server that isn't at UTC+0, the actual query window is shifted by the server's UTC offset, silently, in both the anchored and default (no as_of) case.

This PR added a proper asOfUtc parameter to GetBlockingTrendAsync (LocalDataService.Blocking.cs:668) specifically so callers route the anchor through GetTimeRange's UTC-safe branch instead of the server-local fromDate/toDate branch — the comment right above this call (lines 252-255) even explains the "one instant for both reads" discipline the anchor is meant to preserve. But the call site here never adopted it; it still uses the pre-existing fromDate/toDate positional pattern, so asOfUtc is dead code from this path and the anchor doesn't reach the query correctly.

This looks pre-existing (the double-offset conversion predates as_of), but it directly undermines this PR's own stated goal for this tool and is worth fixing alongside the dropped-anchor bugs — swap to dataService.GetBlockingTrendAsync(resolved.ServerId, hours_back, asOfUtc: windowEnd) the way ~48 other call sites in this PR do.

[erikdarlingdata]

All eight are real, all eight are fixed in 25d7279, and this is the most useful review pass on the PR — thank you.

Every one came from resolving the last rebase in dev's favour and then re-applying the convention with a script that checked the parameter was present rather than that the anchor arrived. They compiled, and they passed a suite that had just gone green on 5,600 tests. Exactly the seam-not-logic shape: the code on each side of the seam was individually correct.

On McpBlockingTools.cs:259 specifically — you are right that it is worse than a dropped anchor. windowStart/windowEnd were already UTC and GetTimeRange treats fromDate/toDate as server-local, subtracting ServerTimeHelper.UtcOffsetMinutes, so the window was shifted by the monitored server's offset in the unanchored case too. Both that read and its capture count now thread asOfUtc; the capture-count helpers (GetBlockingCaptureCountsAsync / GetDeadlockCaptureCountsAsync / the shared GetCaptureCountsAsync) learned the anchor so the "one instant for BOTH reads" discipline its comment describes survives — one value, both reads, UTC-safe branch.

The category, not the eight instances. Eight at once is not eight mistakes, so there is now a pin: an anchored tool must name no DateTime.UtcNow at all, must use the instant it resolved, and — on Lite — must pass asOfUtc to every LocalDataService read that accepts one (that set is read off the shipped source, not transcribed, and the scan asserts it found 40+ so it cannot pass by parsing nothing).

Worth recording how close that pin came to being useless. The first version asserted "does not ASSIGN DateTime.UtcNow", which is what my own detector had used. I reintroduced the get_file_io_trend bug to watch it go red and it stayed green — the body still contained the word windowEnd from its own out var, so the reaches-check was satisfied by the validation line alone. A coverage check that green-lights the defect it was written for is worse than none, because it converts an open question into confidence. It is an absolute now, and I reintroduced one bug of each form (recomputed-now, and dropped-on-the-call) and watched both fail before restoring.

On Lite.Tests/AsOfWindowAnchorTests.cs:203 — correct, and it is the same vacuity in miniature: at offset 0 a correct conversion and no conversion are indistinguishable, so the test could not fail for the reason its own comment gives. Taking that one now.

[claude]

as_of is validated into windowEnd (line 71) but never passed to GetFileIoLatencyTrendAsync — the overload already accepts asOfUtc (LocalDataService.FileIo.cs:71). get_file_io_trend silently ignores its as_of parameter and always answers as of now.

[claude]

Same class of bug as McpIoTools.cs:74 — windowEnd is computed but never passed to GetMemoryTrendAsync (which supports asOfUtc per LocalDataService.Memory.cs:70). get_memory_trend silently ignores as_of.

[claude]

Same class of bug — windowEnd is validated but never passed to GetQueryDurationTrendAsync (which supports asOfUtc per LocalDataService.QueryStats.cs:1111). get_query_duration_trend silently ignores as_of.

[claude]

windowEnd is validated but never passed to GetDistinctWaitTypesAsync here — get_wait_types silently ignores as_of. Notably the same method is called correctly with asOfUtc: windowEnd at line ~131 in this file's GetWaitTrend fallback, so the anchor-aware call pattern was known and used elsewhere in this file — just missed here.

[claude]

Minor test-coverage gap: this reads whatever ServerTimeHelper.UtcOffsetMinutes currently is (a process-wide mutable static) rather than pinning a known non-zero value itself. SystemEventsReaderTests.cs is the only other place in this collection that sets it (to 330, then restores the saved value afterward), so absent a specific ordering guarantee this test most likely runs with offset 0 — at which point the server-local conversion this test's own doc-comment says it's meant to catch ("a bug here would shift the window by the monitored server's offset and still look plausible") can't actually be exercised: a correct implementation and one that skips the offset conversion entirely would produce identical results at offset 0. Consider setting a known non-zero offset (save/restore, same pattern as SystemEventsReaderTests) so this test can't pass vacuously.

[claude]

windowEnd resolved here is never used — a few lines below, var now = DateTime.UtcNow; recomputes "now" from scratch instead of var now = windowEnd;, so get_blocking_trend silently ignores as_of and always answers as of the real current time. Same bug in GetDeadlockTrend just below (line 397 / var now = DateTime.UtcNow; under it). Every other windowed tool touched by this PR does var now = windowEnd; after ValidateWindow; these two are the outliers.

This is exactly the failure mode as_of exists to prevent — a caller anchoring at a past incident gets a silent, unlabeled "now" answer, and the validation succeeding gives false confidence the anchor was honored (an unparseable/future as_of is still correctly rejected inside ValidateWindow itself — it's only the successful resolution that gets thrown away).

The Lite twin has the identical bug in GetDeadlockTrend (Lite/Mcp/McpBlockingTools.cs:311, var dlWindowEnd = DateTime.UtcNow;), so this looks like a copy-paste omission that landed on both SKUs.

[claude]

Same bug as GetBlockingTrend above (line 347): windowEnd from ValidateWindow here is discarded a few lines down in favor of DateTime.UtcNow, so get_deadlock_trend silently ignores as_of.

[claude]

windowEnd resolved here is discarded a few lines down (var dlWindowEnd = DateTime.UtcNow;), so get_deadlock_trend silently ignores as_of and always answers as of real now. This is the Lite twin of the identical bug in Darling/.../DarlingMcpBlockingTools.cs GetDeadlockTrend. The fix right above it in this file's GetBlockingTrend (var windowEnd = anchorEnd;) shows the intended pattern — it just wasn't applied here.

[claude]

Reviewed the as_of anchor plumbing across the shared McpHelpers, both SKUs' MCP tool/reader layers, the Darling web catalog/dispatch, and both new test suites (used parallel review passes, then verified every claim directly against the checked-out source before posting).

PerformanceMonitor.Common/Mcp/McpHelpers.cs (the shared core both SKUs depend on) holds up well under adversarial scrutiny: the future-skew boundary is exact integer-tick arithmetic, the date-only form is forced to UTC midnight (AssumeUniversal/AdjustToUniversal, never host-local), the ambiguous-format allowlist genuinely can't match a space-for-T substitution, ValidateWindow(h, null, …) is provably equivalent to the old ValidateHoursBack(h), null/empty/whitespace as_of are all treated as "omitted," and there's no mutable shared state to race on.

DarlingWebEndpoints.cs / DarlingMcpInstructions.cs: catalog descriptors and dispatch wiring match 1:1 (57/57), every dispatch call uses the anchor as a named argument (verified — no positional binding risk for the tools with trailing optionals like dedup_key/group_by), and the instructions doc's exclusion list matches reality.

Real bugs found — the anchor is silently dropped or miscomputed for 8 tools across both SKUs, all confirmed by reading the actual source (not diff-only):

• get_blocking_trend and get_deadlock_trend on Darling (DarlingMcpBlockingTools.cs:347/397) validate and resolve as_of into windowEnd, then throw it away — var now = DateTime.UtcNow; a few lines down recomputes real "now" instead of using the resolved anchor.
• get_deadlock_trend on Lite (McpBlockingTools.cs:307) has the identical bug — looks like the same copy-paste omission landed on both SKUs.
• get_blocking_trend on Lite (McpBlockingTools.cs:259) does use the anchor, but routes it through GetBlockingTrendAsync's legacy fromDate/toDate positional parameters instead of the new asOfUtc parameter this PR added to that method. GetTimeRange treats supplied fromDate/toDate as server-local and subtracts UtcOffsetMinutes to convert to UTC — but the values passed in are already UTC, so the window is silently shifted by the monitored server's UTC offset for both anchored and default calls. Looks pre-existing (the positional-passing pattern predates as_of), but it defeats the anchor for this tool and is exactly the UTC/local mixup this PR's own design notes for LocalDataService.GetTimeRange/GetTimeRangeServerLocal say was deliberately avoided elsewhere.
• get_file_io_trend, get_memory_trend, get_query_duration_trend, get_wait_types on Lite (McpIoTools.cs:74, McpMemoryTools.cs:66, McpQueryTools.cs:411, McpWaitTools.cs:77) all validate as_of into windowEnd and then never pass it to the underlying LocalDataService call, even though every one of those methods already accepts asOfUtc. McpWaitTools.cs even calls the same method correctly with asOfUtc: windowEnd a few lines later in GetWaitTrend's fallback, so the right pattern was known — just missed on these four.

Left individual inline comments on each. Given the PR's whole premise is "an anchor that's silently not honored is worse than no anchor" (the parser refuses ambiguous input for exactly this reason), these 8 are worth fixing before merge — they're not edge cases, they're the tool's main path.

Also flagged a minor test-coverage gap inline on Lite.Tests/AsOfWindowAnchorTests.cs:203 — TheAnchorAlsoMoves_AServerLocalWindow reads the ambient ServerTimeHelper.UtcOffsetMinutes rather than pinning a known non-zero value, so it likely runs at offset 0 in CI, where it can't actually distinguish a correct server-local conversion from a broken one.

Everything else checked out: no SQL injection (Darling's PostgreSQL queries are consistently parameterized via AddTimestamp/AddWindow, and no DuckDB query concatenates the anchor into SQL text), no window-math sign errors anywhere else, the three deliberate ResolveAsOf-instead-of-ValidateWindow exceptions (get_collection_log, get_current_waits_trend, get_blocking_stats) are correctly implemented and commented, as_of is consistently the last parameter with matching [Description] text via the shared AsOfDescription constant on both SKUs, and the new AsOfWindowAnchorTests.cs suites (live-Postgres and DuckDB) genuinely assert window content (top-N/wait-type membership), not just counts, with solid cleanup discipline.