Server time display reads each connection's own offset (E5)

[erikdarlingdata]

Fixes review finding E5. There is no issue to close.

What changed

The Server mode of the time display used one process-wide number for the server's offset from UTC. TimeDisplayHelper.ServerUtcOffsetMinutes was a static, and every connect wrote to it. With the display set to Server, two sessions on servers in different time zones shifted each other's Query Store times. So did one session that reconnected to another server. The grid, the slicer, and the History times all moved by the wrong server's offset.

The static is gone. Each connect now makes a small holder, ServerUtcOffset, with one Minutes value. The offset query fills that holder. Every document opened on the connection keeps a reference to it. That covers:

• the Query Store grid, its rows, its time slicer, and its wait ribbon
• History and its rows
• the Overview, its slicer, and its wait chart

TimeDisplayHelper.Current stays global, because it is the user's display preference. ConvertForDisplay and FormatForDisplay now take the offset as an argument, with no default. The compiler found every reader. One of them was the slicer's conversion from typed time back to UTC, which read the static in the other direction.

QueryStoreHistoryRow is a Core model, so it cannot see app types. It got a ServerUtcOffset? ServerOffset property, and ServerUtcOffset lives in Core next to TimeDisplayHelper. The History control sets the property on each row after a fetch. A row without a holder reads as UTC in Server mode.

Five constructors take the holder as a required argument: the grid, History, the History window, the Overview, and QueryStoreRow. A creator cannot leave it out. The two places that built a grid now share QuerySessionControl.NewQueryStoreGrid, so the holder is passed in one place.

How long a holder lives

ShowConnectionDialogAsync makes a new holder (BeginServerConnection) when a connect succeeds. It does this before the first await, so a document opened while the offset query runs already has its holder.

The offset query takes its connection string and its holder as arguments. If the user reconnects before the answer arrives, the answer fills the connection that asked. Before, it wrote to shared state.

A reconnect makes a new holder. Documents that are already open keep the old one, because their data came from the old server. A History opened from an old grid after a reconnect gets that grid's holder, not the session's current one. The session already throws the Overview away on a reconnect, so the next Overview gets the new holder.

If the offset arrives after a document has drawn its times, that document shows offset 0 (UTC) while Server is on. Every reader takes the value each time it formats a time, so the document shows the real offset on its next redraw. A redraw happens when the user changes the time display box, fetches again, or resizes the slicer.

I did not add a change event. A change event needs each document to unsubscribe when it closes. Without that, a closed document stays in memory as long as the session. The query starts right after the connect, and a document draws times only after several more round trips, so the gap is small.

If the offset query fails, the holder stays at 0. Before this change, a failed query on a second connect left the previous server's offset in place.

Tests

The new class ServerUtcOffsetPerConnectionTests has 18 cases:

• The conversion with an explicit offset in all three display modes, including a half-hour zone (+330).
• Grid rows, History rows, slicers, ribbons, and the slicer's conversion back to UTC, each on two connections with different offsets. Each shows its own server's time.
• Two sessions on servers in different time zones. Each session's documents keep their own holder.
• A reconnect. A grid opened before it keeps the old holder, and so does a History opened from that grid afterward. A grid and an Overview opened after it get the new holder.
• Every connect makes a new holder.

To check that the tests fail without the fix, I put the old behavior back. The holder's setter wrote one static, and the conversion read that static and ignored its argument. Twelve of the 18 cases failed. Six passed:

• the two Utc cases and the two Local cases, which never used the offset
• the Server case for -300, because an earlier test had left the static at -300
• the test that every connect makes a new holder, which checks identity and not display

I then restored the fix and all 18 passed.

Eight existing test files got the new holder argument in their constructor calls.

Also fixed: the time display box and the wait ribbon

The last commit fixes two small display problems next to this finding.

• The grid's time display box opened on Local whatever TimeDisplayHelper.Current was. If the setting or another grid had chosen Server, a new grid showed Server times under a box that said Local. The box now opens on the mode in effect. Opening a grid does not change the mode.
• Changing the mode in the box redrew the grid's rows and the slicer, but not the wait ribbon. The ribbon kept the old mode's labels and tips until something resized it. It is now redrawn too.

Four new cases cover these, which makes 22 in the class. AGridsTimeDisplayBoxOpensOnTheModeInEffect runs for Local, Utc and Server. RedrawingTheRibbonAfterAModeChangeShowsTheNewMode checks a bar's tip before and after a change. Without the two fixes, 3 of the 4 fail. The Local case passes either way, because the box used to open on Local.

The grid's call to the ribbon redraw has no test of its own, because the ribbon only draws inside a laid-out, expanded wait stats panel. The test calls the same method directly.

Totals

• Full suite on fd5007f: total 1291, failed 0, succeeded 1261, skipped 30. The skipped tests are platform specific (the macOS keychain and non-Windows Entra).
• dotnet build PlanViewer.sln -c Debug reports 0 warnings and 0 errors.

Not done

• No test runs the offset query itself, because it needs a real connection. No test covers the History control copying its holder onto fetched rows either, because that needs a server. The rows are tested with the holder already set, and the History control is tested for which holder it received.
• No redraw when the offset arrives late (see above).
• The mode is still one setting for the whole app. Changing it in one grid changes it for every grid, but the other grids' boxes and times only update when they next redraw. Making the mode per grid, or telling every grid when it changes, is a separate design question.

🤖 Generated with Claude Code

https://claude.ai/code/session_019n3G844aTidqrD6A6iMgza

[claude]

Reviewed the diff. I found nothing blocking.

• Every ConvertForDisplay and FormatForDisplay caller in src now passes an offset. The static ServerUtcOffsetMinutes is gone, and I found no caller still reading it.
• The offset query is a fixed literal with no plan-derived input. The fetch takes its connection string and holder as parameters, which closes the reconnect race.
• There is no version bump and no new NoWarn. I did not build the project or run the tests.
• ServerUtcOffset.Minutes is a plain int written from the async fetch and read from UI code. A stale read is harmless here, and Avalonia's UI-thread await continuation makes it moot in practice.
• A failed offset fetch is swallowed (catch { }) and leaves the holder at 0, so Server mode silently shows UTC. That was already the behavior, so it is fine.
• Test coverage looks proportionate: the new ServerUtcOffsetPerConnectionTests covers the per-connection holder.