Pair every desktop clock render with its column's frame, in both SKUs (#3207, #3221)

[erikdarlingdata]

Fixes #3207. Fixes #3221.

Twenty-nine desktop render sites across both SKUs, wrong in opposite directions, and each SKU correct exactly where the other is wrong — which is why no parity check ever caught it. A server-local column through ForDisplay / FormatServerTime adds the collected offset a second time and displays four hours early on the fleet's measured −240; a naive-UTC column through FormatServerClock renders raw and displays four hours late. A blanket "add the offset" or "stop adding it" fixes half and breaks half, so every site here is paired against its column's frame taken from #3220's catalog-derived census, not from the column's name and not from the neighbouring grid.

#3221 is an external reporter reproducing eight of these from the outside, on Lite/Services/LocalDataService.QueryStats.cs. That is the whole file's offending set and it lands exactly where the census put it.

The frames, measured on collected production data

Both directions, from the store rather than from the collectors' C#. server_properties.utc_offset_minutes is −240 across the fleet (one self-monitored server at 0), and a stored server-clock value is therefore utc + offset, so recovering UTC subtracts the offset:

probe	rows	servers	skew vs the UTC stamp beside it	reads as
blocked_process_reports: blocked_last_tran_started − event_time, same row	143	7	−240 … −242	server-local
query_stats.last_execution_time − collection_time	2,092 captures	42 of 42	−240 exactly (min = max)	server-local
procedure_stats.last_execution_time − collection_time	2,092 captures	42 of 42	−240 … −241	server-local
cpu_utilization_stats: max(sample_time) − collection_time	2,092 samples	42 of 42	−240 … −241	server-local (#2932, reproduced)
query_store_stats.last_execution_time − collection_time	225 captures	43	−6 … +1, mean −1	naive UTC
query_store_stats.first_execution_time (max) − collection_time	225 captures	43	−7 … +1, mean −1	naive UTC

The first four are the columns #3207 and #3221 say are server-local; the last two are the ones both issues argue are UTC from QueryStoreCollector's ((DateTimeOffset)…).UtcDateTime line. Nobody had measured either claim. The blocked-process row is the sharpest of the six: both values sit in the same row, one is the XE @timestamp and one is an XML attribute, and a blocked process's last transaction started seconds — not four hours — before the report fired, so the −240 is frame skew with no age mixed in.

The arithmetic direction is therefore checked against data, not against the sign in the source. Two of the store columns land in the tests as −240 with a same-instant fixture pair, so a sign error is eight hours out rather than plausible.

Every site, its column's frame, and its renderer

Line numbers are post-fix. Census row says whether #3220's DesktopRenderFrameMismatch inventory carried the site; the seventeen rows it did carry are deleted in this change, and set equality fails in the removing direction, so each deletion is proved by the fix that earned it.

Darling viewer — naive UTC rendered RAW, four hours late

site	column → table	frame	was	now	census row
ViewerDataService.QueryStore.cs:99 FirstExecutionTimeLocal	first_execution_time → query_store_stats	UTC	FormatServerClock	ViewerTimeHelper.ForDisplay	deleted
ViewerDataService.QueryStore.cs:104 LastExecutionTimeLocal	last_execution_time → query_store_stats	UTC	FormatServerClock	ForDisplay	deleted
ViewerDataService.QueryStoreRegressions.cs:68 LastExecutionTimeLocal	last_execution_time → query_store_stats	UTC	FormatServerClock	ForDisplay	deleted
ViewerHistoryRows.cs:237 FirstExecutionTimeLocal	first_execution_time → query_store_stats	UTC	FormatServerClock	ForDisplay	deleted
ViewerHistoryRows.cs:241 LastExecutionTimeLocal	last_execution_time → query_store_stats	UTC	FormatServerClock	ForDisplay	deleted

ViewerHistoryRows.cs:106-107 and :166-167 are the query_stats and procedure_stats history rows in the same file, on same-named columns, and they are correct as they stand — they keep FormatServerClock. That is the whole reason the census keys its inventory on the resolved TABLE and not on the column: a fix at one last_execution_time site in this file and a regression at another would cancel in a bare count.

Darling viewer — server-local sent through ForDisplay, four hours early

site	column → table	frame	was	now	census row
ViewerDataService.RunningJobs.cs:141 StartTimeLocal	start_time → running_jobs	server-local	ForDisplay	FormatServerClock	deleted
ViewerDataService.Deadlock.cs:76 LastTranStartedLocal	deadlock-graph lasttranstarted	server-local	ForDisplay	FormatServerClock	none
ViewerDataService.PlanCorrection.cs:215 ValidSinceLocal	valid_since → plan_correction	server-local	Local(…) → ForDisplay	FormatServerClock	none
ViewerDataService.PlanCorrection.cs:216 LastRefreshLocal	last_refresh → plan_correction	server-local	Local(…) → ForDisplay	FormatServerClock	none
ViewerDataService.PlanCorrection.cs:217 ExecuteActionInitiatedTimeLocal	execute_action_initiated_time → plan_correction	server-local	Local(…) → ForDisplay	FormatServerClock	none
ViewerDataService.PlanCorrection.cs:218 RevertActionInitiatedTimeLocal	revert_action_initiated_time → plan_correction	server-local	Local(…) → ForDisplay	FormatServerClock	none

Lite — server-local sent through FormatServerTime, four hours early

site	column → table	was	now	census row
LocalDataService.Blocking.cs:1002 LastTranStartedLocal	deadlock-graph lasttranstarted	FormatServerTime	FormatServerClock	none
LocalDataService.Blocking.cs:1058-1060 Blocked{LastTranStarted,LastBatchStarted,LastBatchCompleted}Local	blocked_process_reports	FormatServerTime	FormatServerClock	deleted (3 sites)
LocalDataService.Blocking.cs:1120 TranStartTimeLocal	query_snapshots.tran_start_time	FormatServerTime	FormatServerClock	deleted
LocalDataService.PlanCorrection.cs:210-213	plan_correction ×4	FormatServerTime	FormatServerClock	deleted (4 sites)
LocalDataService.QueryStats.cs:1480, 1671 CreationTimeLocal	query_stats.creation_time	FormatServerTime	FormatServerClock	deleted (1 row / 2 sites)
LocalDataService.QueryStats.cs:1602, 1725 CachedTime{Formatted,Local}	procedure_stats.cached_time	FormatServerTime	FormatServerClock	deleted (1 row / 2 sites)
LocalDataService.QueryStats.cs:1479, 1603, 1672, 1726 LastExecutionTimeLocal	procedure_stats + query_stats	FormatServerTime	FormatServerClock	deleted (1 row / 4 sites)
LocalDataService.RunningJobs.cs:164 StartTimeLocal	running_jobs.start_time	DateTime.ToLocalTime()	FormatServerClock	none

DeadlockTimeLocal (deadlocks.deadlock_time), EventTimeLocal (the XE @timestamp on both blocking arms) and every CollectionTimeLocal keep FormatServerTime: they are naive UTC and were already right. Three of the six blocked-process XML stamps — the Blocking* half — are read into properties and never bound, so there is no render site to fix; the census lists the three that are.

Lite had no server-clock renderer, so its columns had nowhere correct to go

ServerTimeHelper.FormatServerTime names its parameter utcTime and adds the collected offset before converting — it is Lite's ForDisplay, not its FormatServerClock, and there was no raw renderer at all. ServerTimeHelper.FormatServerClock is that renderer. It is not a new conversion: ConvertForDisplay's documented input is already the server's clock, so the new method is FormatServerTime with the offset add removed. Exactly one add on the naive-UTC path, none on this one.

It honours the Server / Local / UTC preference, through the existing ConvertForDisplay, and that matters rather than being a nicety. #3221's reproduction is in Local Time mode on a UTC+02 host against a UTC−05 server, and reports the row rendering at 15:00 where 20:00 is correct. A renderer that emitted the server's clock verbatim would fix Server mode and leave the reporter's own repro rendering 13:00 — a different wrong answer. Lite also prints the mode's timezone label in its status bar (ServerTab.Refresh.cs:122), so a raw render under UTC would sit under a label saying UTC.

Darling's same-named ViewerDataService.FormatServerClock now honours the mode too, so the frame fix costs no preference. It did not at first: it emitted the server's own clock in all three modes, so moving running_jobs.start_time onto it fixed the frame and dropped the mode, at a site that had respected the user's choice. The review bot caught that, correctly.

It composes out of ViewerTimeHelper.ConvertToDisplay's existing arms rather than adding any. The naive-UTC twin of a server-clock value is serverLocal - offset, so ConvertServerClockToDisplay is one subtraction in front of the conversion already there, and every arm — the machine-local one especially — is reused rather than re-derived. That was checked as arithmetic before it was written: a server-clock value converted for display equals the same instant's naive-UTC value converted for display, in every mode.

mode	a server-clock 14:00 at offset −240, on a UTC+02 host	before
Server	14:00 — it is the server's clock	14:00
UTC	18:00	14:00, labelled UTC
Local	20:00	14:00

The pair is now symmetric across the SKUs and the doc comments say so: Darling's ConvertToDisplay takes naive UTC so its server-clock conversion subtracts the offset first; Lite's ConvertForDisplay takes the server's clock so its naive-UTC renderer adds it. One offset step between the frames, either way round.

Both modes are pinned, which the old renderer could not support: with no mode input it was untestable on this axis, and that is how a raw render shipped unnoticed. ViewerTimeHelperTests.FormatServerClock_HonoursTheDisplayMode_AtTheFleetOffset asserts the same server-local value rendering 14:00 under Server and 17:45-shaped under UTC at the fleet's measured −240, and ViewerClockRendererArithmeticTests asserts the composition and the non-interchangeability of the two conversions — the latter runs locally, against the shipped ViewerTimeHelper.cs.

Seven doc comments that said "renders raw" now say what the code does.

Lite is untouched. ServerTimeHelper already honours the mode. Its two job-history getters (RunTimeLocal, NextScheduledRunLocal) show run_datetime and next_scheduled_run as-is and are documented as deliberately showing "the time SSMS shows"; whether that should change is a product question and stays one, in the queue.

The comments that made this spread

Four, all corrected with the code, because a comment stating the wrong frame is the mechanism by which one wrong site became several:

• ViewerDataService.QueryStoreRegressions.cs:29 stated the frame outright — "the SQL server's local wall clock in Darling's store, shown RAW … exactly like the sibling Query Store tab" — and appealed to a sibling tab as precedent. The renderer follows the column, never the neighbouring grid; that sentence is what turned one site into three.
• ViewerHistoryRows.cs:19-22 lumped Query Store in with the DMVs: "the DMV / Query Store wall-clock stamps … are the SQL server's own local time". True of query_stats / procedure_stats — whose rows sit in the same file and are correct — and false of query_store_stats. The header now says the three row types do not agree and that the store table decides it.
• ViewerDataService.ItemHistory.cs:33 listed first_execution_time in the same breath as the DMV stamps. Same error, different file.
• ViewerDataService.QuerySnapshots.cs:84-85 justified the correct Darling site by parity: "Mirrors Lite's QuerySnapshotRow.TranStartTimeLocal (raw server-clock FormatServerTime)". FormatServerTime is not raw. The Darling site was right, the Lite site it claimed to mirror was double-skewed, and the sentence asserting they agreed is what kept the divergence invisible.

Two class summaries in ViewerDataService.FinOps.cs (:337, :545) claimed LastExecutionTime localized in read / LastUserAccess localized in read. Both reads take the value verbatim and say so, with the reason and the contrast against the collection_time-derived timestamps in the same port — so the summary asserted a conversion the code deliberately does not perform, on a server-local column, which is the same mechanism as the four above. Both now defer to their read. The coordinator found one; a grep for the phrase finds two, and the second is wrong for the same reason.

Three raw-render getters in Lite/Services/LocalDataService.JobHistory.cs show run_datetime and next_scheduled_run as-is, and only one of the three said so. RunTimeLocal:235 carried the reasoning ("the time SSMS shows"); LastSuccessfulRunLocal:240 — MAX(run_datetime) over the successful step-0 rows, the same column and the same frame — had nothing at either property or class level, and NextScheduledRunLocal:322 had it in the class summary only. Both now state it at the property, where a reader checks. No behaviour changes: whether Lite should honour the display preference on that grid at all is a product question (Lite is single-server and mirroring SSMS is a coherent choice; Darling is fleet-wide and normalises, which is why it de-skews the same two columns in SQL and says so three times), and it is not settled here.

ViewerDataService.RunningJobs.cs's class doc claimed start_time was "the store's naive-UTC start_time"; the collector proves otherwise on the next line of its own query. It now states both frames in the row and what happens if they are crossed.

The census, and the four sites it could not see

#3220's DesktopRenderFrameMismatch label carried 17 rows / 22 sites, pinned at set equality in both directions. All 17 are deleted here, and the ratchet is what makes the deletions evidence rather than assertion: reverting any one fix while leaving its row behind fails, and reverting any one row while leaving the fix behind fails. The label is now empty, which is a result and not an omission — the scan still fails on a new offender, because a non-empty found set against no rows is a failure.

DesktopRenderMismatchSites goes 22 → 0, and AssertMatchesInventory's Assert.NotEmpty(actual) had to go with it: it stood in for "the scan actually looked", and an assertion that blocks the fix it exists to prove is worse than none. Reach is asserted per scan instead. The MCP arm keeps an explicit Assert.NotEmpty(found) because it still has 33 offenders (#3206's, via #3212), with a note saying it goes when that lane empties its label. The render arm's floor on sites judged is now the exact measured baseline of 43 rather than a loose 30, because with no offenders left it is the only thing standing between a crippled matcher and a green census. Calibrated red-first: at 44 with nothing planted the suite fails and reports only 43 render sites were judged.

The four plan_correction sites in the Darling viewer are not in the census, and could not have been. ValidSinceLocal and its three siblings reached ForDisplay through PlanCorrectionRow.Local(…) — a one-hop wrapper — and the render scan keys on the renderer's name. A wrapper is a different name reaching the same renderer, so the sites were invisible to a census that calls itself closed. They are real: the register classifies all four columns server-local from PlanCorrectionCollector's own SQL, sys.dm_db_tuning_recommendations documents them in the instance's clock, and #3212 is de-skewing the same four on the MCP side.

Two changes close that hole rather than fixing the four instances:

• Each of the four now names its renderer at the site, so the existing scan judges them (judged 38 → 43, with Lite's running-job start time the fifth addition).
• RenderWrappers declares the six one-hop wrappers in the two UI trees with the renderer each reaches, pinned at set equality against a derived set, so a new or renamed wrapper fails the build and has to declare its frame. NoRenderWrapper_IsHandedAColumnEveryTableFramesTheOtherWay then judges each wrapper's call sites over census columns — per (file, method), because two different wrappers in the Darling viewer are both named Local over different frames and a name-keyed scan would judge each one's sites against the other's renderer.

That check's bound is deliberate and stated: unanimous disagreement only. Where a column name spans both frames — event_time is five tables and two of them — a wrapper site cannot be judged from the name, and the direct-call scan's resolution machinery does not transfer, because a wrapper is not the site of the read and the file it sits in is not the file whose SQL decides the table. Sixteen event_time sites through the two SystemEvents wrappers are therefore out of reach rather than cleared, and the count of sites examined is floored (16) with the number of wrappers reaching a census column pinned exactly (2) — because with no offenders left, "examined nothing" and "found nothing" are the same result.

Writing that check's both-directions pin found a bug in it. WrapperSignature matches a doc comment quoting a declaration, and this guard's own file is full of prose about these very methods, so a note would have been derived as a wrapper and failed set equality against the real ones. The derivation strips comments first, and the pin asserts the raw pattern does match the quoted form so the strip is visibly load-bearing rather than tidy. No such comment exists today; a planted one turns it red.

The renderer map's completeness, pinned two ways

FormatStoredUtc is ported from #3228 with its doc comment, and it is the right change for a reason different from the one it was asked for. It is the Darling viewer's named UTC renderer, beside FormatServerClock, so the choice between the pair is reviewable at every site rather than spelled out in a ternary — and picking the wrong one of the pair is silent, since both return a plausible timestamp and they differ by the server's whole offset. The five query_store_stats sites take it.

The stated reason for the port was that the inline ternaries left those sites using no registered renderer, so the scan could not see them. That is not the case, and it is measurable. RenderCall allows an optional receiver and a trailing member access, so ViewerTimeHelper.ForDisplay(FirstExecutionTime.Value) matches \bForDisplay\s*\(\s*(?:[A-Za-z_]\w*\.)?FirstExecutionTime\b. Renaming ONE of the five ternaries' renderer to an unregistered name drops judged from 43 to 42 and reds the floor — so all five were judged before the port, and they are judged after it. Registering FormatStoredUtc is what keeps that true, and the sites were never invisible.

The map's completeness had no criterion, which is the class the guard exists to catch. Two checks now cover it from different directions:

• TheOneHopRenderWrappers_AreExactlyTheDeclaredSet derives every ALIAS — a static formatter over a DateTime reaching a registered renderer under a different name — and pins the set at equality against RenderWrappers. Six today. A new one fails the build and has to declare the frame it renders.
• NoUnregisteredRenderCapableMember_IsAppliedToACensusColumn asks nothing about signatures. Every identifier applied to a census timestamp column across the render surface is derived — 72 applications, eight distinct names — and each must be a registered renderer, a declared alias, or declared as not rendering with what it actually is (if (…HasValue), new DateTime(…Year, …Hour, 0, 0), Nullable.Compare). Three further directions are asserted: a registered renderer that reaches no census column is a map typo, a stale exemption that no longer appears is an assertion about nothing, and every exemption carries its reasoning.

The two do not subsume each other, and the mutation matrix shows it: an unregistered instance method handed a census column is invisible to the shape criterion and caught by the second check alone.

The walker rewrite that was backed out, and the defect it found

The completeness derivation was first written on CSharpMemberMap, the repository's own C# member walker, because a regex over a fixed byte window after a signature is exactly what that walker exists to replace. It found 3 of the 6 aliases and reported nothing wrong.

CSharpMemberMap's brace walk closes an expression-bodied member on the { of an is { } property pattern, so Local, Local and Timestamp — whose bodies all begin utc is { } t ? … — were read as 68, 74 and 80 characters and did not contain their own ForDisplay call. ShapeOf returned WholeMember for all three. ViewerHistoryRows.CollectionLocal, identical in every other respect but with no is { }, read whole.

The only reason the shortfall was visible is that the regex derivation had already answered the same question, so there was something to disagree with; written walker-first, it would have pinned a hand list of three and called the map complete. The walker version is backed out, the regex derivation that finds 6 of 6 is kept, and the CSharpMemberMap defect is staged for whoever owns that file rather than fixed from here — it is shared infrastructure with its own pins.

The seven sites with no census row, and how each is covered

The census is derived from CollectorCatalog.All's timestamp columns, so a site it cannot reach needs saying rather than leaving as an unexplained count. Twenty-two of the twenty-nine have rows. The other seven are three distinct kinds:

• Two are read-time XML — ViewerDataService.Deadlock.cs:76 and LocalDataService.Blocking.cs:1002. lasttranstarted is walked out of the stored deadlock_graph_xml by DeadlockGraphProcessParser at READ time, so no CollectorColumn declares it and no derivation from the catalog can see it. Covered by a doc comment in each SKU stating both frames in the row and why the census cannot reach the pair; a guard for read-time XML provenance is a different shape and Guard a server-local column reaching a UTC-assuming consumer, with the census (#3208) #3220 already says so.
• Four are census columns behind a wrapper — ViewerDataService.PlanCorrection.cs:215-218. The columns are in the register; the render SITE was invisible. Now covered twice: the sites name their renderer so the existing scan judges them, and RenderWrappers pins the wrapper set so the next one cannot hide.
• One is a census column with no renderer at all — LocalDataService.RunningJobs.cs:164 was StartTime.ToLocalTime(). running_jobs.start_time is in the register, but DateTime.ToLocalTime() is not one of the three renderers the scan keys on, so the site was outside the judged population. It carried two defects: ToLocalTime() on a Kind=Unspecified value treats it as UTC when it is msdb Agent's own clock, and it rendered in the Lite host machine's zone regardless of the display preference every other timestamp in the app honours. Now FormatServerClock, which fixes both, and the site enters the judged population — which is why the floor moved to 43 rather than 42.

Verified

Two net10.0 xunit.v3 harnesses over the shipped sources, since both suites target net10.0-windows and cannot run on macOS.

• framecheck — ConsumedTimestampFrameDisciplineTests plus StoreSqlClockDisciplineTests, CollectorTimestampFrameTests, DocCommentHygieneTests, CommentFilterAdoptionTests and TsqlConventionGuardTests, compiled with AssemblyName=Darling.Tests. Baseline before this change Total: 113, Failed: 0, Skipped: 0, Not Run: 0 — byte-identical to Guard a server-local column reaching a UTC-assuming consumer, with the census (#3208) #3220's recorded baseline. After: Total: 116, Errors: 0, Failed: 0, Skipped: 0, Not Run: 0 (three new guards), and Total: 117, Failed: 0, Skipped: 0, Not Run: 0 after merging origin/dev in, which brought Weld concatenated literals before scanning store SQL, and make the PostgreSQL parse-check population what the reader files declare #3223's additions to StoreSqlClockDisciplineTests — the sibling guard this one cross-checks its ambiguous-name register against — and left both green.
• clockcheck — the two new arithmetic test files compiled against the SHIPPED ServerTimeHelper.cs, ViewerTimeHelper.cs and TimeDisplayMode.cs, linked rather than referenced (neither production file touches WPF, so the real arithmetic runs; a retyped copy would prove the transcription). Baseline Total: 21, Errors: 0, Failed: 0, Skipped: 0, Not Run: 0.

The two suites cover different halves and neither covers the other's: the census is a SOURCE scan and cannot see inside a renderer's body, while the arithmetic tests know nothing about which site calls which renderer. Both mutation classes below confirm that split rather than assuming it.

The row-getter wiring is not separately tested, deliberately: "this getter calls the wrong renderer" is exactly what the census scan judges, at set equality, over all 43 sites. A per-getter test would be a second, weaker copy of that.

Everything below the frame boundary is CI's: Lite.Tests (which is where ServerClockRendererArithmeticTests ships) and Darling.Tests run on Windows in build.

Red-proof by mutation

Baseline printed above. Forty mutations, each committed first, applied by byte replacement with a uniqueness-checked anchor, verified present by content and not only by git diff --numstat (several are one-line-for-one-line and report an identical 1 1), rebuilt, run, restored, and verified restored by content with the tree confirmed clean after every row.

The fixes, one direction at a time

mutation	outcome
CONTROL: Darling running-job start time back through ForDisplay (4h early)	RED — render census
CONTROL: Darling Query Store first-execution back to raw (4h late)	RED — render census
CONTROL: Darling Query Store history last-execution back to raw	RED — render census
CONTROL: Darling regressions tab back to raw	RED — render census
CONTROL: Lite creation_time back through FormatServerTime (#3221's own column)	RED — render census
CONTROL: Lite blocked-process last_tran_started back through FormatServerTime	RED — render census
CONTROL: Lite plan_correction.valid_since back through FormatServerTime	RED — render census
CONTROL: Lite running-job start time back to DateTime.ToLocalTime()	RED — render census (via the judged floor)
CONTROL: Darling plan_correction.valid_since back behind the Local() wrapper	RED — two guards: the census AND the new wrapper check

Both directions of the defect are represented, in both SKUs, and the last row is the shape that was invisible before this change.

The renderers' arithmetic — which the census structurally cannot see

mutation	framecheck	clockcheck
Lite FormatServerClock adds the offset again (the original defect, inside the new renderer)	green	RED, 8 tests
Lite FormatServerClock subtracts the offset (sign error, eight hours out)	green	RED, 8 tests
Lite FormatServerClock renders raw, ignoring the display mode (Darling's semantics)	green	RED, 3 tests
Lite FormatServerTime stops adding the offset (the naive-UTC path, broken the other way)	green	RED, 8 tests
Darling ViewerTimeHelper's Server-mode conversion subtracts the offset (sign error)	green	RED, 4 tests
INSTRUMENT: the arithmetic harness's own fixture pair is no longer one instant	green	RED, 6 tests

Six rows where framecheck stays green and clockcheck reds. That is the split stated as a measurement: a source scan over renderer names cannot see a renderer's body, and this is what would have shipped a correctly-paired call to a wrong renderer. The third row is the one that matters for #3221 — a raw renderer passes Server mode and fails Local and UTC, which is exactly the half-fix its reproduction would have caught.

The last row mutates the instrument: with the two fixtures no longer one instant, six assertions move, so the same-instant premise is falsifiable rather than decorative.

The instrument

mutation	outcome
drop Lite from the render root list	RED — render census + wrapper set
drop FormatServerClock from the renderer table	RED
drop the optional receiver from RenderCall	RED
invert the render verdict (== for !=)	RED — census + the both-directions verdict control
WrapperSignature accepts only private members	RED — wrapper set + the matcher pins
a renderer counts as a wrapper of itself	RED — wrapper set
widen the wrapper body bound 400 → 6000	RED — wrapper set
point every wrapper offender scan at the wrong file	RED — wrapper check
INSTRUMENT: stop stripping comments before deriving wrappers, nothing planted	GREEN — see below
the same, AND a doc comment quoting a declaration	RED — wrapper set
CONTROL: a wrapper called with a census column from a file declaring no wrapper	RED — the cross-file arm
NEGATIVE CONTROL: plant a CORRECTLY framed render site	GREEN
NEGATIVE CONTROL: the same planted correct site with the judged floor at 44	GREEN
FLOOR CALIBRATION: judged floor at 44 with nothing planted	RED, reporting only 43 render sites were judged

The comment-strip pair is one argument. Removing the strip changes no result today, which is precisely why it needed a mutation that fails without it: planting a doc comment that quotes a declaration turns it red. A bound that only holds for the comments currently in the tree is one note away from not holding, and this guard's own file is full of prose about these very methods.

The three floor rows are one argument. A negative control that survives being crippled was never a control, so the floor was calibrated red first: at 44 with nothing planted the suite fails, which fixes the baseline at 43. Planting a correctly framed site then makes 44 pass — proving the new site entered the judged population — while the offender set stays empty. Green there is evidence the scan discriminates, not evidence it did not look.

The failure this PR's first CI run reported, and what it was right about

build and Darling PostgreSQL tests both failed on head d6da0314e, on the same single test:
LiveCleanupConversionRatchetTests.NoLiveTestCleansUpOnItsOwnBodysConnection, naming ViewerQueriesTests.cs:417. The mode-blindness pin had been written into ViewerQueriesTests.cs with a try/finally restoring the two process-wide ViewerTimeHelper statics. That file also holds a [Collection("live-postgres")] class, so the whole file is in that ratchet's population, and a finally in it must route through LiveStoreCleanup.

The ratchet was right about the file for a reason beyond the one it reports: ViewerQueriesTests is in no collection, and flipping CurrentDisplayMode from there races the three classes that ARE in [Collection("viewer-time-statics")]. The pin now lives in ViewerTimeHelperTests, which owns those statics and is in that collection, and it gained a control — the naive-UTC renderer in the same viewer must MOVE with the mode, so the mode-blind assertion is about this renderer rather than about a preference nothing honours.

Reproduced and red-proofed locally by adding the ratchet class to framecheck (baseline Total: 132, Failed: 0, Skipped: 0, Not Run: 0): putting a non-store finally back into that file turns NoLiveTestCleansUpOnItsOwnBodysConnection red, and removing it turns it green. The ratchet had never been in this lane's harness, which is why the shape reached CI.

Two pins this lane could not exercise locally

Both are the Darling mode-blindness pin, and both are green locally for the same reason: ViewerTimeHelperTests reaches ViewerDataService, hence the WPF Viewer assembly, so neither net10.0 harness can compile it.

• Making Darling's FormatServerClock honour the display mode — closing the gap the note declines — went GREEN on both harnesses.
• Blinding the pin's own control (flipping the mode to the same value twice, so the "it must move" assertion asserts nothing) also went GREEN.

Neither is a hole in the guard; both are the limit of what a net10.0 harness can compile. CI's build is the arbiter for FormatServerClock_IgnoresTheDisplayMode_AtTheFleetOffset, and that is stated rather than reported as covered.

Corrections to #3207, #3221 and #3220

#3207 numbers 9 sites; the census resolves them to 22 plus 7 it cannot reach, and the two counts are not comparable. Several of #3207's entries name multiple lines (its sites 3, 5 and 7 are two, two and three render sites), and its site 9 is a third mechanism rather than a mis-paired renderer.

#3207's site 1 cites ViewerDataService.RunningJobs.cs:117. The ForDisplay(StartTime) call was at :135; #3220 already noted this.

#3207's fix note says sites 3, 4 and 5 need ForDisplay and that Lite's 6, 7, 8 need a raw renderer "which Lite does not have". Correct on the first count. On the second, "raw" is the wrong shape for Lite: a renderer that emits the server's clock verbatim leaves #3221's own reproduction rendering 13:00 where 20:00 is correct, because that repro is in Local Time mode. Lite's ConvertForDisplay already takes the server's clock and honours the mode, so the renderer Lite needed was FormatServerTime minus the offset add, not a raw one. #3207's parenthetical "(render raw, no offset add)" reads as those being the same thing; they differ in two of three display modes.

#3221 lists eight affected columns and gets all eight right, including the four last_execution_time ones, and its row-type attribution (QueryStatsRow, ProcedureStatsRow, QueryStatsHistoryRow, ProcedureStatsHistoryRow) matches the file exactly. It is also right that collection_time in the same rows is genuinely UTC and correctly formatted, which is what makes each history row visibly mixed-frame. Nothing in it needed correcting.

#3221's "Confirmed in v3.6.0 source and current dev" holds for the frame defect. Its note that #2987's shared-offset custom-range fix does not change the per-column source-frame mismatch is also right, and the two are independent: #2987 moved which server's offset a window used, this moves whether a column gets the offset at all.

#3220's body says the census found "Eight render sites #3207 does not list". Twelve of its rows are outside #3207's list, not eight: the four plan_correction stamps and the creation_time / cached_time pairs are annotated as such, and the four LocalDataService.QueryStats.cs last_execution_time sites are named in the same section without being counted into the eight. #3220 states the fact and undercounts it in the same paragraph.

#3220 says two of #3207's sites sit outside the census. Seven sites sit outside it, of three kinds — the two read-time XML ones it names, the four wrapper-hidden plan_correction ones it could not see, and Lite's running-job start time, whose column IS in the register but whose renderer was DateTime.ToLocalTime(). The last two kinds are both matcher reach, not catalog reach, which is why they are closed here rather than declared.

CHANGELOG entry text

Here rather than in CHANGELOG.md: every concurrent lane appends to the same [Unreleased] block, so a per-PR edit turns one clean append into as many conflicting ones as there are lanes landing after it.

Fixed

• Desktop timestamp columns were rendered in the wrong clock frame in both SKUs, in opposite directions, with each SKU correct exactly where the other was wrong. A column holding the monitored server's own wall clock — sys.dm_exec_* creation / cache / last-execution stamps, plan_correction's recommendation and action stamps, the blocked-process report's transaction and batch attributes, query_snapshots.tran_start_time, msdb Agent's job start time and the deadlock graph's lasttranstarted — was sent through the renderer that adds the collected UTC offset, displaying it four hours early on a fleet at UTC-4; and Query Store's first- and last-execution stamps, which the collector normalises to UTC, were rendered raw and displayed four hours late. Each of twenty-nine render sites now takes the renderer matching its column's frame, and Lite gains ServerTimeHelper.FormatServerClock for the frame it had no renderer for, so a server-clock value converts exactly once and still honours the Server / Local / UTC display preference (Desktop clock frames are wrong in opposite directions in both SKUs, and each SKU is right where the other is wrong #3207, [BUG] Lite query and procedure grids render server-local DMV timestamps through the UTC formatter #3221).
• Four doc comments stated a column's clock frame incorrectly, one of them appealing to a sibling tab as precedent and one asserting a cross-SKU parity that did not hold; they are the reason one wrong site became several, and each now states the frame the code uses (Desktop clock frames are wrong in opposite directions in both SKUs, and each SKU is right where the other is wrong #3207).

Added

• The clock-frame census now sees a renderer reached through a one-hop wrapper. Four server-local plan_correction stamps in the Darling viewer were in the wrong frame and invisible to it, because the render scan keys on the renderer's name and a wrapper is a different name reaching the same renderer. The wrapper set is derived and pinned at set equality, so a new one must declare the frame it renders, and no wrapper may be handed a column every table declaring it frames the other way (Desktop clock frames are wrong in opposite directions in both SKUs, and each SKU is right where the other is wrong #3207).

[claude]

Review summary

Reviewed the timestamp-frame fixes across Lite and Darling (ServerTimeHelper/ViewerTimeHelper renderer swaps, plus the new census/wrapper test infrastructure in ConsumedTimestampFrameDisciplineTests.cs).

Correctness of the frame fixes: I traced each renderer swap against its source column (XE @timestamp vs deadlock-graph/report-XML attributes vs sys.dm_exec_* vs Query Store's datetimeoffset-normalized-to-UTC columns) and they check out — each swap moves a column to the renderer matching its actual stored frame, and the arithmetic in the new FormatServerClock/ConvertForDisplay path (Lite) is correct in all three display modes (verified by hand: Server/UTC/Local all round-trip through the offset correctly).

Parity check: Went through every changed site and its counterpart in the other SKU (Deadlock.LastTranStarted, Blocking's three report attributes, QuerySnapshots.TranStartTime, PlanCorrection's four stamps, QueryStats/ProcedureStats creation/cached/last-execution, RunningJobs.StartTime, and the query_store_stats family across QueryStore/QueryStoreRegressions/ViewerHistoryRows). Lite and Darling agree on frame classification everywhere I checked, and where one SKU was already correct pre-PR (e.g. Lite's LocalDataService.QueryStore.cs for first_execution_time/last_execution_time, Darling's ViewerDataService.QueryStats.cs/ProcedureStats.cs), this PR correctly left it alone.

One parity gap worth flagging (left as an inline comment on ViewerDataService.RunningJobs.cs:141): Darling's ViewerDataService.FormatServerClock is mode-blind — it always prints the raw server clock regardless of ViewerTimeHelper.CurrentDisplayMode, because ViewerTimeHelper has no arm that converts an already-server-local value to Local/UTC display (only ForDisplay, which takes naive-UTC input). Lite's ServerTimeHelper.FormatServerClock does honor the Server/Local/UTC toggle via ConvertForDisplay. This asymmetry predates the PR but this PR expands its footprint from 2 properties to 8 (Deadlock, 4× PlanCorrection, RunningJobs, plus the pre-existing QueryStats/ProcedureStats sites) — so in the Darling viewer, toggling to Local/UTC mode will no longer visibly do anything for these columns, while the identical columns in Lite do respond to the toggle. Worth either giving ViewerTimeHelper a server-local conversion arm or tracking this explicitly as a known follow-up.

No other correctness, security, or T-SQL-convention issues found — this PR touches no T-SQL. The new test infrastructure (wrapper census, render-verdict tests, arithmetic tests pinned against one instant in two frames) is unusually thorough and appears self-consistent; I did not attempt to hand-verify every pinned count in ConsumedTimestampFrameDisciplineTests.cs since those are self-checking via CI.

🤖 Generated with Claude Code

[claude]

Reviewed the diff against dev (783a856..HEAD), focusing on correctness, Lite/Darling parity, and security/perf. No T-SQL in this PR, so the T-SQL style section doesn't apply.

Correctness: Spot-checked every renderer swap (FormatServerTime↔FormatServerClock in Lite, ForDisplay↔FormatServerClock in Darling) against the column's actual source (DMV vs. Query Store's DateTimeOffset-normalized columns vs. msdb Agent) and each matches the frame described in the PR body and doc comments. Direction of each fix (early vs. late) is consistent with the described utc + offset relationship.

Lite/Darling parity: The two new arithmetic test files (Lite.Tests/ServerClockRendererArithmeticTests.cs and Darling/Darling.Tests/ViewerClockRendererArithmeticTests.cs) mirror each other appropriately, including correctly not mirroring behavior where the SKUs are legitimately asymmetric — Lite's FormatServerClock honors the Server/Local/UTC display mode via ConvertForDisplay, while Darling's same-named ViewerDataService.FormatServerClock is confirmed mode-blind (verified against the actual implementation at ViewerDataService.QueryStats.cs:601, which is untouched by this PR). That gap is explicitly called out in the PR description as a separate product decision rather than silently papered over, and is pinned by FormatServerClock_ShowsRawServerWallClock_InEveryDisplayMode_EmptyForNull.

Test harness: The RenderWrappers / NoRenderWrapper_IsHandedAColumnEveryTableFramesTheOtherWay additions to ConsumedTimestampFrameDisciplineTests.cs plausibly close the one-hop-wrapper blind spot that hid the four plan_correction sites, and the floor/set-equality assertions (JudgedRenderSites = 43, DesktopRenderMismatchSites = 0) look consistent with the removal of all 22 census rows in this change.

No correctness bugs, parity drift, or security/perf issues found. (Not flagging missing-index DMV usage per review instructions — none present anyway.)

[claude]

Reviewed this against the diff (origin/dev...HEAD), not just the PR description.

Traced every one of the 20 changed production/test files and cross-checked the frame claims against the actual collector source rather than taking the write-up on faith:

• QueryStoreCollector.cs:1568-1569 confirms ((DateTimeOffset)reader.GetValue(3/4)).UtcDateTime for first_execution_time/last_execution_time — so the switch of ViewerDataService.QueryStore.cs, ViewerDataService.QueryStoreRegressions.cs, and the Query Store row in ViewerHistoryRows.cs from FormatServerClock (raw) to ViewerTimeHelper.ForDisplay (naive-UTC conversion) is correct.
• RunningJobsCollector's own DATEDIFF(SECOND, ja.start_execution_date, GETDATE()) (local-vs-local) backs the claim that running_jobs.start_time is server-local, supporting both the Lite (DateTime.ToLocalTime() → FormatServerClock) and Darling (ForDisplay → FormatServerClock) fixes.
• The new ServerTimeHelper.FormatServerClock/FormatServerTime split is arithmetically sound: FormatServerTime still adds the offset (naive-UTC input), FormatServerClock is that minus the add (already-server-clock input), both routed through the same ConvertForDisplay so the Server/Local/UTC preference is honored either way. Lite.Tests/ServerClockRendererArithmeticTests.cs pins this against one instant expressed in both frames rather than two independently-written strings, which is the right way to catch a sign error.
• Verified the nullable/non-nullable DateTime/DateTime? plumbing at every touched call site (BlockedLastTranStarted, LastTranStarted, RunningJobRow.StartTime, the four plan_correction stamps, etc.) — all consistent on both SKUs.
• The Lite/Darling asymmetry that remains (Darling's FormatServerClock is mode-blind by design since ViewerTimeHelper has no server-local arm; Lite's honors the display mode) is explicitly documented in doc comments and pinned by ViewerTimeHelperTests.FormatServerClock_IgnoresTheDisplayMode_AtTheFleetOffset, and the PR body correctly frames it as a deliberate, pre-existing product-decision gap rather than something this PR should be fixing — so I'm not flagging it as parity drift.
• The ViewerQueriesTests.cs mode-blindness pin was moved into ViewerTimeHelperTests (which carries [Collection("viewer-time-statics")]), correctly avoiding the live-postgres-collection finally-cleanup ratchet that broke the first CI run — confirmed the collection attributes are where the PR says they are.
• No .sql/T-SQL files are touched by this PR, so the T-SQL style conventions don't apply here; the C# changes (naming, XML doc comments on public members) are consistent with CONTRIBUTING.md's C# style section.

No correctness, security, or Lite/Darling parity issues found. This is a well-scoped, thoroughly self-verified fix (the arithmetic tests and the census/wrapper set-equality tests give real teeth against regressions in either direction).

[claude]

Reviewed the diff against the actual collector SQL for every touched column, since the PR body's frame claims are the whole basis for the fix:

• running_jobs.start_time ← ja.start_execution_date, no UTC normalization, confirmed server-local by the collector's own DATEDIFF(SECOND, ja.start_execution_date, GETDATE()) (local-vs-local). Matches the fix (FormatServerClock/FormatServerTime→FormatServerClock swap in both SKUs).
• query_store_stats.first_execution_time/last_execution_time ← normalized via ((DateTimeOffset)...).UtcDateTime in QueryStoreCollector, confirmed naive UTC. Matches the Darling fix to FormatStoredUtc/ForDisplay. Lite's equivalent (LocalDataService.QueryStore.cs) already used FormatServerTime and was correctly left untouched — no new parity gap.
• plan_correction's four stamps ← raw from sys.dm_db_tuning_recommendations, no conversion, confirmed server-local. Matches the fix in both ViewerDataService.PlanCorrection.cs and LocalDataService.PlanCorrection.cs.
• query_snapshots.tran_start_time ← MIN(dat.transaction_begin_time) off sys.dm_tran_active_transactions, confirmed server-local. Lite fixed; Darling's equivalent was already correct pre-PR.
• blocked_process_reports' three blocked_last_* attributes ← parsed raw out of the blocked-process XML with no offset applied, confirmed server-local. Lite fixed (FormatServerClock); Darling has no render site for these fields at all (never bound in XAML), which predates this PR and isn't a gap it introduces.
• Deadlock graph lasttranstarted ← same XML-attribute shape, confirmed server-local, fixed symmetrically in both SKUs.

Checked every fixed site's counterpart in the other SKU (the parity requirement) and didn't find a case where one SKU was fixed and its counterpart wasn't — the four/six pairs above were either fixed in both places or were already correct in the untouched one. ServerTimeHelper.FormatServerClock (Lite) and ViewerTimeHelper.ConvertToDisplay/ViewerDataService.FormatServerClock (Darling) both math out correctly against the stated utc + offset frame, and the Darling FormatServerClock mode-blindness is called out and pinned rather than silently left.

No T-SQL in this PR, so the T-SQL style conventions don't apply. No security, secrets, or perf concerns — this is pure desktop rendering logic, no query or I/O changes. Copyright headers present on the two new test files.

Nothing to flag. This is unusually well self-audited (the collector-source cross-check above didn't turn up anything the PR's own frame census got wrong).

[claude]

Reviewed the diff in full (all 22 changed C# files + the census/arithmetic test additions).

Summary: This is a display-layer-only fix (no T-SQL, no schema/storage changes), so the T-SQL style conventions and two-store schema-parity rules in CONTRIBUTING.md don't apply here. I traced every changed render site by hand against the frame each column is documented to hold (sys.dm_exec_* / plan_correction / blocked-process-report / Agent job start / deadlock lasttranstarted = server-local; query_store_stats execution stamps / XE event_time / collection_time = naive UTC), and cross-checked Lite against Darling for every site touched:

• ServerTimeHelper.FormatServerClock (Lite) and ViewerTimeHelper.ForServerClockDisplay / ViewerDataService.FormatServerClock (Darling) are correctly mirrored — one starts from server-local and subtracts the offset, the other starts from server-local and is the identity for ServerTime mode; verified with the fixture arithmetic in ServerClockRendererArithmeticTests/ViewerClockRendererArithmeticTests (NaiveUtc=18:00, ServerClock=14:00, offset -240 — checks out).
• Every Lite fix (LocalDataService.Blocking.cs, .PlanCorrection.cs, .QueryStats.cs, .RunningJobs.cs) has a corresponding Darling fix in the same commit (ViewerDataService.Deadlock.cs, .PlanCorrection.cs, .RunningJobs.cs, ViewerHistoryRows.cs, .QueryStore.cs, .QueryStoreRegressions.cs) and vice versa — I didn't find a site fixed on one side without its counterpart on the other.
• The new FormatStoredUtc (Darling) is correctly used only for the genuinely-naive-UTC query_store_stats columns, kept distinct from FormatServerClock.
• The RunningJobRow.StartTimeLocal fix in Lite (StartTime.ToLocalTime() → FormatServerClock) correctly removes a DateTime.ToLocalTime() call that, on an Unspecified-kind value, was silently treating it as UTC and applying the host's local offset — unrelated to and independent of the server's collected offset or the display-mode preference.
• Test statics (ViewerTimeHelper.CurrentDisplayMode/UtcOffsetMinutes, ServerTimeHelper equivalents) are correctly isolated behind [Collection("viewer-time-statics")] / [Collection("server-time-helper")] in the new test classes, avoiding xUnit parallel-run races.
• The one intentionally-unconverted case I checked — FinOpsTab.xaml's Idle Databases grid binding LastExecutionTime raw via StringFormat (bypassing any renderer) — is called out explicitly in the updated doc comments on IdleDatabaseRow/IndexUsageRow in both SKUs and is pre-existing/unchanged by this PR, not a regression.

I did not find correctness, parity, or security issues in this diff. No inline comments to post. Note CI (build/verify/Darling PostgreSQL tests) was still in progress at review time — worth confirming green before merge given how much of this PR's confidence rests on the new census/arithmetic tests actually compiling and passing.