Job History tab speaks display names

[erikdarlingdata]

Closes #2126.

The gap

ghauan runs the fleet by aliases and the Job History tab was the one place that refused to speak them: both the Server filter dropdown and the Server column carried the raw collected server_name, while every other tab resolves the operator's display alias.

The fix

Darling — both Job History readers now resolve names through the servers registry with COALESCE(reg.display_name, <raw name>) — the exact pattern the FinOps inventory reader already uses:

• GetJobHistoryAsync — the base CTE joins servers and emits the alias as server_name, so the Server column, the Server filter combo (which keys on server_id, so filtering is unchanged), the per-column filter popup, and the CSV export all show display names with zero tab-side changes.
• GetAgentStatusAsync — same join; the fleet Agent roll-up now also sorts by display name.

display_name is never empty on the write path (DarlingConfig.DisplayName falls back to the host), and the COALESCE covers pre-existing rows written before the column existed.

Lite (robot review catch — same gap, different mechanism): Lite's display-name concept lives at the config layer (ServerConnection), not in DuckDB — the stored servers.display_name column is unpopulated by design. So the shell passes a server_id → alias snapshot into the tab Overview-style (the same GetDeterministicHashCode(GetServerNameForStorage(...)) identity the Overview refresh already uses), and rows swap in the alias on every refresh. A server no longer in config keeps its raw collected name — job history is a durable record and rows can outlive their registration.

Tests

No SQL-shape pins exist for these readers (checked — only schema-probe sentinels reference job_history), and the Darling change is a projection swap with the join precedent already live in two other readers. The Lite change is display-only row post-processing behind an optional Initialize parameter, defaulting to off.

🤖 Generated with Claude Code

[claude]

Review: Job History tab speaks display names

The Darling-side fix itself looks correct:

• LEFT JOIN servers AS reg (not INNER JOIN) is the right call vs. the FinOps inventory pattern it borrows from — job_history/agent_status rows can outlive a decommissioned server's registry row, and the COALESCE fallback to the raw name preserves that case instead of silently dropping rows.
• servers.server_id is PRIMARY KEY (confirmed in PgMigrations.cs V2), so the join can't fan out and duplicate job-history rows.
• Filtering still keys on server_id (untouched), so the alias swap is display-only, as the PR description claims.
• display_name is populated from darling.json config (DarlingMcpServerAdminTools.cs:287, defaults to host, trimmed) and via DarlingObservability.cs's INSERT INTO servers, so there's no live write path that could leave a blank (non-NULL) display_name that COALESCE wouldn't catch.

Lite/Darling parity gap: this fix isn't mirrored in Lite. Lite/Services/LocalDataService.JobHistory.cs's GetJobHistoryAsync/GetAgentStatusAsync read raw server_name straight from v_job_history/agent_status with no alias resolution, and Lite/Controls/JobHistoryTab.xaml.cs (PopulateServerFilter, grid binding) uses that raw name as-is — the same bug this PR just fixed in Darling. Two wrinkles specific to Lite make this not a copy-paste fix:

1. Lite's servers table does have a display_name column (Lite/Database/Schema.cs:24), but it's dead — nothing writes to it (grep -rn "UPDATE servers" Lite/ only turns up an unrelated sql_engine_edition/sql_major_version update). Joining to it the way Darling does would resolve to NULL every time.
2. Lite's actual display-name concept lives entirely at the config layer (ServerConnection.DisplayNameWithIntent, threaded through FleetServer), not in DuckDB. The Overview tab already does this correctly — GetServerSummaryAsync(serverId, displayName) takes the display name as a caller-supplied parameter rather than reading it from a row. Job History would need the same treatment (pass the config display name in, or resolve server_id → display name at the JobHistoryTab layer) rather than a SQL-side join, since the join target is unpopulated in Lite.

Net: today, Job History is the one tab in Lite that still shows raw collected names instead of the operator's alias — this PR narrows that gap to Lite-only rather than closing it everywhere. Worth a follow-up issue if it's not folded into this PR.

No other correctness, security, or performance issues found in the diff.

[erikdarlingdata]

Review taken — the Lite parity gap is real and is now closed in 4468d50, your way rather than a copy-paste: Lite's display-name concept lives at the config layer (the stored servers.display_name column is unpopulated by design, as you found), so the shell passes a server_id → alias snapshot into the tab Overview-style — the same GetDeterministicHashCode(GetServerNameForStorage(...)) identity the Overview refresh already uses — and rows swap in the alias on every refresh. A server no longer in config keeps its raw collected name, since job history is a durable record and rows can outlive their registration.

Also updated the PR title's scope: this now closes the gap in BOTH SKUs, not just Darling.

(The earlier Darling PostgreSQL tests failure was unrelated to this diff — CompressionJobSelfHeal_DetectionQueryValid tripped on next_start = -infinity compression jobs left in the shared CI fixture by sibling tests; the same suite passed on three other runs in the same window. Rerun kicked.)

[claude]

Once this swap makes row.ServerName a live-editable alias, PopulateServerFilter a few lines below (same file) goes stale in a way it never could before: its rebuild-skip check only compares server IDs:

var newIds = servers.Select(s => s.ServerId.ToString()).ToList();
if (newIds.SequenceEqual(existingIds)) return;

not the display text. Renaming a server's alias in Manage Servers doesn't touch the id set (and RefreshServerList() never calls JobHistoryContent.RefreshJobs() either), so on the next tick where this tab does refresh (the 30s auto-refresh or a tab reselect) the grid rows pick up the new alias here, but the Server-filter ComboBoxItem.Content built earlier is never regenerated — the dropdown keeps showing the old alias indefinitely (until a server is actually added/removed) while the grid shows the new one. It still filters correctly since Tag is the id, but the label is wrong.

Before this PR ServerName was the raw collected server name, effectively invariant for a session, so that early-return was harmless. Now that it's meant to track an operator-editable alias (the point of #2126), the skip-check needs to include the name too, e.g. compare (id, name) pairs instead of just ids.

Same reused-helper shape exists in Darling's JobHistoryTab.xaml.cs::PopulateServerFilter (not touched by this diff, but it now consumes the same COALESCE(reg.display_name, ...)-resolved name from ViewerDataService.GetJobHistoryAsync, so it has the identical staleness gap there too).

[claude]

Reviewed the diff (CHANGELOG.md, Darling/.../ViewerDataService.JobHistory.cs, Lite/Controls/JobHistoryTab.xaml.cs, Lite/MainWindow.xaml.cs).

Correctness

• Left an inline comment on Lite/Controls/JobHistoryTab.xaml.cs: the new alias-swap makes ServerName a live-editable value, but PopulateServerFilter's rebuild-skip check only diffs the server-id set, not the display text. Renaming a server's alias mid-session (Manage Servers → Edit) leaves the Server-filter dropdown showing the stale alias indefinitely (grid rows update fine; the dropdown label doesn't, until a server is actually added/removed). The identically-shaped PopulateServerFilter in Darling's JobHistoryTab.xaml.cs has the same latent gap now that its data source resolves through servers.display_name too — that file isn't part of this diff so I couldn't leave an inline note there, flagging it here instead.

Darling SQL (ViewerDataService.JobHistory.cs)

• The added LEFT JOIN servers AS reg ON reg.server_id = jh.server_id / ... ON reg.server_id = a.server_id is safe against row fan-out — servers.server_id is the table's primary key, so this can't multiply job_history/agent_status rows or skew the window functions (avg_success_duration, last_success_run_utc) that partition over jh.server_id. Good.
• Parameterization is unchanged and still correctly binds $1/$2/$3; no injection surface introduced.

Lite/Darling parity

• The servers registry join (Darling) and the server_id → alias snapshot (Lite) are architecturally different (SQL-side COALESCE vs. C#-side dictionary swap) but that's called out explicitly in the CHANGELOG entry as a deliberate divergence, since Lite's servers.display_name column is unpopulated by design (config-layer aliases). That's a reasonable and well-documented split — no drift concern there beyond the staleness issue above.
• RemoteCollectorService.GetDeterministicHashCode(GetServerNameForStorage(s)) keying in Lite/MainWindow.xaml.cs matches the same pattern used elsewhere in Lite (MainWindow.SidebarTree.cs, MainWindow.AlertEngine.cs), so it's consistent with the existing convention.

No missing-index or other DMV-folklore suggestions, and no security issues spotted (no new user input reaches SQL text directly; everything stays parameterized).

[claude]

Review

Went through the full diff (CHANGELOG.md, ViewerDataService.JobHistory.cs, JobHistoryTab.xaml.cs, MainWindow.xaml.cs) plus the surrounding code for precedent and parity. No bugs found — a few notes on why specific things I checked are fine, not asks:

Darling (SQL)

• COALESCE(reg.display_name, jh.server_name) / COALESCE(reg.display_name, a.server_name) exactly mirrors the existing FinOps inventory precedent (ViewerDataService.FinOps.Inventory.cs:150) and DarlingFleetReader.cs:70. servers.server_id is the table's PRIMARY KEY (PgMigrations.cs:127), so the added LEFT JOIN servers AS reg is cheap and no perf concern.
• servers is created in V2Sql, well before job_history/agent_status (V24+), so the join target always exists by the time these tables are populated.
• Verified the PR body's claim that no SQL-shape pin tests reference GetJobHistoryAsync/GetAgentStatusAsync — only schema-existence sentinels touch job_history (ViewerDataServiceTests.cs:401). Nothing broken.
• The ORDER BY server_name change in GetAgentStatusAsync is inert today (the WPF tab never renders agent_status server names, only status text), but it's harmless and consistent with the stated intent — not flagging it.

Lite (config-layer)

• The server_id → alias identity (GetDeterministicHashCode(GetServerNameForStorage(s))) is the same one used throughout MainWindow.xaml.cs/AlertEngine/Mcp resolvers — not a new hashing scheme, so no new collision risk.
• PopulateServerFilter keys the combo's Tag on ServerId (not name), so the post-swap alias display doesn't affect filtering correctness even if two servers ever shared a display name.
• The name swap runs after the background Task.Run completes, back on the UI-thread continuation — no cross-thread access to _serverManager/DataGrid.
• A server removed from config correctly falls back to the raw stored name (dictionary lookup miss), matching the stated "durable record" behavior.

Parity

• The two apps deliberately use different mechanisms (SQL-side join in Darling vs. C#-side row post-processing in Lite), which is called out explicitly in the PR description and is the correct call given Lite's DuckDB servers.display_name is unpopulated by design. Confirmed both sides actually converge on the same visible behavior (alias when known, raw name otherwise).
• Minor, non-blocking observation: Lite's Job History now shows DisplayNameWithIntent (appends "(Read-Only)"), while Darling's Job History shows plain display_name with no such suffix. This isn't drift introduced by this PR — it matches each app's own pre-existing convention (Lite's Overview tab already uses DisplayNameWithIntent; Darling's FinOps/Servers list already omit the intent suffix) — just noting it in case a future pass wants the two apps' Job History tabs to render identically for read-only replicas.

No correctness, security, or SQL-injection concerns (all filters are parameterized), and no missing-index recommendations to make.