Skip to content

Commit 1730c67

Browse files
committed
fix(webapp): hide queue header charts when there are no queues to chart
The four header chart tiles rendered even in the not-success states (engine-version upgrade, no tasks) and when the list filtered to empty, showing four empty 'No activity' cards above the blank state. Gate them on the same queue set the table shows.
1 parent 8f7a066 commit 1730c67

3 files changed

Lines changed: 274 additions & 73 deletions

File tree

apps/webapp/app/presenters/v3/reports/health/health-data.ts

Lines changed: 48 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,25 @@ function isConcurrencyRejection(error: unknown): boolean {
108108
/** rows + the actual (clip-aware) time window the query service resolved for this run. */
109109
type QueryResult = { rows: Row[]; timeRange: { from: Date; to: Date } };
110110

111-
async function runQuery(
111+
/** Runs one (TRQL) report query. Injectable so tests can drive the loader with canned results. */
112+
export type HealthQueryRunner = (
113+
env: AuthenticatedEnvironment,
114+
query: string,
115+
period: string
116+
) => Promise<QueryResult>;
117+
118+
/**
119+
* The loader's IO boundary (§7 Seam A): ClickHouse via the query service, Redis via the engine.
120+
* Defaults wire the real singletons; `loadHealthInput` accepts an override so its orchestration
121+
* (source selection, snapshot fallback on empty/throw, dlq parse, window math) is testable without
122+
* booting the env-bound query-service client.
123+
*/
124+
export type HealthDeps = {
125+
runQuery: HealthQueryRunner;
126+
lengthOfEnvQueue: (env: AuthenticatedEnvironment) => Promise<number | undefined>;
127+
};
128+
129+
async function executeReportQuery(
112130
env: AuthenticatedEnvironment,
113131
query: string,
114132
period: string
@@ -226,14 +244,21 @@ ORDER BY fails DESC
226244
LIMIT 10`;
227245
}
228246

247+
/** Default IO wiring — the real query-service runner + the engine's env-queue length. */
248+
const defaultHealthDeps: HealthDeps = {
249+
runQuery: executeReportQuery,
250+
lengthOfEnvQueue: (env) => engine.lengthOfEnvQueue(env),
251+
};
252+
229253
/** Run a query that may reference not-yet-available columns; never break the report. */
230254
async function tryQuery(
255+
deps: HealthDeps,
231256
env: AuthenticatedEnvironment,
232257
query: string,
233258
period: string
234259
): Promise<Row[]> {
235260
try {
236-
return (await runQuery(env, query, period)).rows;
261+
return (await deps.runQuery(env, query, period)).rows;
237262
} catch {
238263
return [];
239264
}
@@ -266,7 +291,8 @@ export interface FlowSource {
266291
loadFlow(
267292
env: AuthenticatedEnvironment,
268293
period: string,
269-
ctx: RunsContext
294+
ctx: RunsContext,
295+
deps: HealthDeps
270296
): Promise<FlowData | null>;
271297
}
272298

@@ -276,20 +302,20 @@ export interface FlowSource {
276302
* caller can fall back to the snapshot.
277303
*/
278304
export const QueueMetricsSource: FlowSource = {
279-
async loadFlow(env, period) {
305+
async loadFlow(env, period, _ctx, deps) {
280306
try {
281307
// Redis depth is not a ClickHouse query, so it runs alongside (doesn't count toward the cap).
282-
const pendingNowPromise = engine.lengthOfEnvQueue(env);
308+
const pendingNowPromise = deps.lengthOfEnvQueue(env);
283309

284310
// Bug 1 fix — route all CH queries through the concurrency cap (max 2 in flight) instead
285311
// of firing 4 at once via Promise.all.
286312
const [series, liveScalar, baselineScalar, worstRows, dlqRows] = await mapWithConcurrency(
287313
[
288-
() => runQuery(env, envSeriesQuery(), period).then((r) => r.rows),
289-
() => runQuery(env, envScalarQuery(), period).then((r) => r.rows[0] ?? {}),
290-
() => runQuery(env, envScalarQuery(), BASELINE_PERIOD).then((r) => r.rows[0] ?? {}),
291-
() => tryQuery(env, queueWorstQuery(), period),
292-
() => tryQuery(env, dlqTotalQuery(), period),
314+
() => deps.runQuery(env, envSeriesQuery(), period).then((r) => r.rows),
315+
() => deps.runQuery(env, envScalarQuery(), period).then((r) => r.rows[0] ?? {}),
316+
() => deps.runQuery(env, envScalarQuery(), BASELINE_PERIOD).then((r) => r.rows[0] ?? {}),
317+
() => tryQuery(deps, env, queueWorstQuery(), period),
318+
() => tryQuery(deps, env, dlqTotalQuery(), period),
293319
],
294320
CH_CONCURRENCY,
295321
(task) => task()
@@ -379,8 +405,8 @@ function buildQueueMetricsFlow(
379405
* series is shape-only (`estimated: true`).
380406
*/
381407
export const SnapshotFlowSource: FlowSource = {
382-
async loadFlow(env, _period, ctx) {
383-
const pendingNow = (await engine.lengthOfEnvQueue(env)) ?? 0;
408+
async loadFlow(env, _period, ctx, deps) {
409+
const pendingNow = (await deps.lengthOfEnvQueue(env)) ?? 0;
384410

385411
let backlog = 0;
386412
const proxy = ctx.liveSeries.map((r) => {
@@ -415,15 +441,16 @@ export const SnapshotFlowSource: FlowSource = {
415441
export async function loadHealthInput(
416442
env: AuthenticatedEnvironment,
417443
period: string,
418-
now: Date = new Date()
444+
now: Date = new Date(),
445+
deps: HealthDeps = defaultHealthDeps
419446
): Promise<HealthInput> {
420447
// Bug 1 fix — route the runs-phase CH queries through the concurrency cap (max 2 in flight)
421448
// instead of firing all 3 at once, so we never exceed the query service's per-project limit.
422449
const [liveScalarRes, liveSeriesRes, baselineScalarRes] = await mapWithConcurrency(
423450
[
424-
() => runQuery(env, runsScalarQuery(), period),
425-
() => runQuery(env, runsSeriesQuery(), period),
426-
() => runQuery(env, runsScalarQuery(), BASELINE_PERIOD),
451+
() => deps.runQuery(env, runsScalarQuery(), period),
452+
() => deps.runQuery(env, runsSeriesQuery(), period),
453+
() => deps.runQuery(env, runsScalarQuery(), BASELINE_PERIOD),
427454
],
428455
CH_CONCURRENCY,
429456
(task) => task()
@@ -444,8 +471,8 @@ export async function loadHealthInput(
444471

445472
// Prefer measured queue metrics; fall back to the runs snapshot when unavailable.
446473
const flow =
447-
(await QueueMetricsSource.loadFlow(env, period, ctx)) ??
448-
(await SnapshotFlowSource.loadFlow(env, period, ctx))!;
474+
(await QueueMetricsSource.loadFlow(env, period, ctx, deps)) ??
475+
(await SnapshotFlowSource.loadFlow(env, period, ctx, deps))!;
449476

450477
const failuresSeries = resampleSeries(
451478
ctx.liveSeries.map((r) => failureRate(num(r.failures), num(r.completed)))
@@ -470,7 +497,7 @@ export async function loadHealthInput(
470497
normalRate > 0 &&
471498
rate / normalRate >= HEALTH_THRESHOLDS.failures.warnMult;
472499
const failureBreakdown = failureDegraded
473-
? await loadFailureBreakdown(env, period, num(ctx.liveScalar.failures))
500+
? await loadFailureBreakdown(deps, env, period, num(ctx.liveScalar.failures))
474501
: undefined;
475502

476503
const lastCompletion = parseTimestamp(ctx.liveScalar.last_completion);
@@ -498,12 +525,13 @@ export async function loadHealthInput(
498525
}
499526

500527
async function loadFailureBreakdown(
528+
deps: HealthDeps,
501529
env: AuthenticatedEnvironment,
502530
period: string,
503531
totalFails: number
504532
): Promise<HealthInput["failureBreakdown"]> {
505533
if (totalFails <= 0) return undefined;
506-
const rows = await tryQuery(env, failureBreakdownQuery(), period);
534+
const rows = await tryQuery(deps, env, failureBreakdownQuery(), period);
507535
if (rows.length === 0) return undefined;
508536
const top = rows[0];
509537
return { task: String(top.task ?? "unknown"), share: num(top.fails) / totalFails };

apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues/route.tsx

Lines changed: 58 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -625,59 +625,64 @@ function QueuesWithMetricsView() {
625625

626626
{/* Env saturation, Backlog, Scheduling delay p95, Throttled viz — full-size, synced,
627627
drag-to-zoom line charts (Agent page pattern). Four chart tiles: 2x2 below lg, 4-up
628-
from lg, derived from the tile count. `kind="charts"` bakes the fixed row height. */}
629-
<ChartSyncProvider onZoom={zoomToTimeFilter}>
630-
<MetricsLayout.Grid kind="charts">
631-
{QUEUE_HEADER_TILES.map((tile) => (
632-
<QueueEnvMetricChart
633-
key={tile.id}
634-
tile={tile}
635-
timeRange={timeRange}
636-
queueNames={chartQueueNames}
637-
referenceLines={
638-
tile.id === "saturation"
639-
? [
640-
{
641-
y: 100,
642-
label: `Limit ${environment.concurrencyLimit}`,
643-
labelPlacement: "outside" as const,
644-
},
645-
...(environment.burstFactor > 1
646-
? [
647-
{
648-
y: Math.round(environment.burstFactor * 100),
649-
label: `Burst ${Math.round(
650-
environment.concurrencyLimit * environment.burstFactor
651-
)}`,
652-
labelPlacement: "outside" as const,
653-
},
654-
]
655-
: []),
656-
]
657-
: undefined
658-
}
659-
// Saturation and p95 "step over the line": a per-bucket overlay retraces only
660-
// the over-threshold stretches in warning colour, so under-threshold values stay
661-
// blue. (A gradient split can't do this reliably — an SVG objectBoundingBox
662-
// gradient tracks the line's own bbox, not the y-axis, so a low/flat line reads
663-
// as entirely warning-coloured.)
664-
// All thresholded lines colour warning where they step over the threshold: the
665-
// per-bucket overlay retraces only the over-threshold stretches, so the colour
666-
// change tracks the axis crossing.
667-
warningOverlay={
668-
tile.id === "saturation"
669-
? { threshold: 100 }
670-
: tile.id === "p95"
671-
? { threshold: 60_000 }
672-
: tile.id === "throttled"
673-
? // Integer counts: threshold 0 warns once a bucket has ≥1 throttle.
674-
{ threshold: 0 }
675-
: undefined
676-
}
677-
/>
678-
))}
679-
</MetricsLayout.Grid>
680-
</ChartSyncProvider>
628+
from lg, derived from the tile count. `kind="charts"` bakes the fixed row height.
629+
Only when there are queues to chart: not-success states (engine-version, no tasks) and a
630+
filtered-to-empty list leave chartQueueNames empty, where the tiles would just render
631+
four "No activity" cards above the blank state. */}
632+
{chartQueueNames.length > 0 ? (
633+
<ChartSyncProvider onZoom={zoomToTimeFilter}>
634+
<MetricsLayout.Grid kind="charts">
635+
{QUEUE_HEADER_TILES.map((tile) => (
636+
<QueueEnvMetricChart
637+
key={tile.id}
638+
tile={tile}
639+
timeRange={timeRange}
640+
queueNames={chartQueueNames}
641+
referenceLines={
642+
tile.id === "saturation"
643+
? [
644+
{
645+
y: 100,
646+
label: `Limit ${environment.concurrencyLimit}`,
647+
labelPlacement: "outside" as const,
648+
},
649+
...(environment.burstFactor > 1
650+
? [
651+
{
652+
y: Math.round(environment.burstFactor * 100),
653+
label: `Burst ${Math.round(
654+
environment.concurrencyLimit * environment.burstFactor
655+
)}`,
656+
labelPlacement: "outside" as const,
657+
},
658+
]
659+
: []),
660+
]
661+
: undefined
662+
}
663+
// Saturation and p95 "step over the line": a per-bucket overlay retraces only
664+
// the over-threshold stretches in warning colour, so under-threshold values stay
665+
// blue. (A gradient split can't do this reliably — an SVG objectBoundingBox
666+
// gradient tracks the line's own bbox, not the y-axis, so a low/flat line reads
667+
// as entirely warning-coloured.)
668+
// All thresholded lines colour warning where they step over the threshold: the
669+
// per-bucket overlay retraces only the over-threshold stretches, so the colour
670+
// change tracks the axis crossing.
671+
warningOverlay={
672+
tile.id === "saturation"
673+
? { threshold: 100 }
674+
: tile.id === "p95"
675+
? { threshold: 60_000 }
676+
: tile.id === "throttled"
677+
? // Integer counts: threshold 0 warns once a bucket has ≥1 throttle.
678+
{ threshold: 0 }
679+
: undefined
680+
}
681+
/>
682+
))}
683+
</MetricsLayout.Grid>
684+
</ChartSyncProvider>
685+
) : null}
681686

682687
{success ? (
683688
<MetricsLayout.Content>

0 commit comments

Comments
 (0)