Skip to content

Commit dc3b502

Browse files
committed
fix(reports): health report review fixes F2-F11
- FlowLoadResult ok/unavailable/failed + isRolloutError classification - availability 'unknown' + flow_unmeasured reason instead of false greens - telemetry none/fresh/lagging/stale; trustworthy requires a positive signal - env-wide queue totals denominator; bucketCoverage with anomaly windows - finishedPerMin drain math; canonical ReportViewModel/ReportPeriod schemas in core - registry tables auth metadata; MCP prompt enum validation + escaping - period grammar without seconds, 90d cap; new presenter/route/prompt tests
1 parent 6e5f0f0 commit dc3b502

24 files changed

Lines changed: 7085 additions & 5331 deletions
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@trigger.dev/core": patch
3+
"trigger.dev": patch
4+
---
5+
6+
Reports can now be fetched as structured data, not just text: ask for the `json` format and you get the numbers and what they mean, typed. Report periods are also stricter — the shortest window is one minute (`30m`, `1h`, `7d`), because reports summarise data by the minute and anything shorter can't be answered honestly.

apps/webapp/app/presenters/v3/reports/ReportPresenter.server.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*/
99

1010
import { type AuthenticatedEnvironment } from "~/services/apiAuth.server";
11-
import { REPORT_REGISTRY } from "./report-registry";
11+
import { REPORT_REGISTRY, type ReportLoader } from "./report-registry";
1212
import { type ReportViewModel } from "./report-view-model";
1313

1414
const DEFAULT_PERIOD = "1h";
@@ -22,6 +22,8 @@ const DEFAULT_PERIOD = "1h";
2222
const inFlight = new Map<string, Promise<ReportViewModel | undefined>>();
2323

2424
export class ReportPresenter {
25+
constructor(private readonly registry: Record<string, ReportLoader<unknown>> = REPORT_REGISTRY) {}
26+
2527
async call({
2628
environment,
2729
key,
@@ -31,8 +33,8 @@ export class ReportPresenter {
3133
key: string;
3234
period?: string;
3335
}): Promise<ReportViewModel | undefined> {
34-
const loader = REPORT_REGISTRY[key];
35-
if (!loader) return undefined;
36+
if (!Object.hasOwn(this.registry, key)) return undefined;
37+
const loader = this.registry[key];
3638

3739
const flightKey = `${key} ${environment.id} ${period}`;
3840
const existing = inFlight.get(flightKey);

apps/webapp/app/presenters/v3/reports/health/flow.ts

Lines changed: 41 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,24 @@ import {
1717
type Severity,
1818
} from "../report-view-model";
1919
import {
20+
bucketCoverage,
2021
HEALTH_THRESHOLDS,
2122
isPendingIncreasing,
23+
isPendingUnknown,
2224
mean,
2325
metricById,
2426
type HealthInput,
2527
} from "./health-core";
2628

2729
export const FLOW_METRIC_IDS = ["start_latency_p95", "pending", "throughput"];
2830

31+
/**
32+
* Flow reason for "we could not measure the backlog". Not a severity and not a cause — the
33+
* verdict is simply unassessable, so nothing actionable may hang off it. Kept distinct from
34+
* "unknown" (the stale-telemetry guard) so the two failure modes stay legible.
35+
*/
36+
export const FLOW_UNMEASURED = "flow_unmeasured";
37+
2938
/** One row of the declarative cause table — everything a cause defines about itself. */
3039
type CauseSpec = {
3140
reason: string;
@@ -44,6 +53,13 @@ export function interpretFlow(metrics: Metric[], input: HealthInput): Finding {
4453
const flowMetrics = FLOW_METRIC_IDS.map((id) => metricById(metrics, id));
4554
const severity = maxSeverity(...flowMetrics.map((m) => m.severity));
4655

56+
// The backlog couldn't be measured: `pending.now` is a placeholder, so neither "healthy" nor a
57+
// cause may be claimed off it. Severity still reflects the metrics we DID measure (start
58+
// latency), but the finding carries no cause, no attribution and no recommendation.
59+
if (isPendingUnknown(input)) {
60+
return { type: "flow", severity, reason: FLOW_UNMEASURED, metricIds: FLOW_METRIC_IDS };
61+
}
62+
4763
if (isOk(severity)) {
4864
return { type: "flow", severity, reason: "healthy", metricIds: FLOW_METRIC_IDS };
4965
}
@@ -52,12 +68,18 @@ export function interpretFlow(metrics: Metric[], input: HealthInput): Finding {
5268
const pendingIncreasing = isPendingIncreasing(input.pending.series);
5369
const latencyElevated = !isOk(metricById(metrics, "start_latency_p95").severity);
5470
// Concurrency causes need real running-capacity evidence — without it runningShare is a
55-
// meaningless 0 and would falsely select dequeue_stall on the snapshot path (#1).
56-
const hasConcurrencyEvidence = ev.envLimit > 0 && ev.runningSeries.length > 0;
71+
// meaningless 0 and would falsely select dequeue_stall on the snapshot path (#1). They also
72+
// need enough of the window to have ARRIVED: the series isn't gap-filled, so a couple of fresh
73+
// buckets would otherwise read as "pinned the whole window".
74+
const coverage = bucketCoverage(input);
75+
const hasConcurrencyEvidence =
76+
ev.envLimit > 0 && ev.runningSeries.length > 0 && coverage.sufficient;
5777
const runningShare = hasConcurrencyEvidence ? mean(ev.runningSeries) / ev.envLimit : 1;
78+
// Pinned share is measured against EXPECTED buckets, not received rows — "2 of 60 expected",
79+
// never "2 of 2 received".
5880
const pinnedShare = hasConcurrencyEvidence
5981
? ev.runningSeries.filter((r) => r >= t.pinnedLevel * ev.envLimit).length /
60-
ev.runningSeries.length
82+
coverage.expectedBuckets
6183
: 0;
6284
const pinned = pinnedShare >= t.pinnedShare;
6385
const hasTriggerBaseline = input.throughput.normalTriggeredPerMin > 0;
@@ -67,8 +89,9 @@ export function interpretFlow(metrics: Metric[], input: HealthInput): Finding {
6789
// No baseline: a multiplier can't be computed, so an absolute rate selects "new volume".
6890
const triggerSurge = !hasTriggerBaseline && input.throughput.triggeredPerMin >= t.surgePerMin;
6991

70-
const donePerMin = input.throughput.donePerMin;
71-
const net = donePerMin - input.throughput.triggeredPerMin;
92+
// Work leaving the queue = FINISHED (all terminal) runs, not completions only.
93+
const finishedPerMin = input.throughput.finishedPerMin;
94+
const net = finishedPerMin - input.throughput.triggeredPerMin;
7295
// Exclusions must be PROVEN, not assumed. "not your code" needs healthy execution; "limits
7396
// aren't the bottleneck" needs no env-pin AND no queue throttling; the workers/spike ones
7497
// state a measured fact (rate) rather than a global "everything's fine" claim.
@@ -112,10 +135,10 @@ export function interpretFlow(metrics: Metric[], input: HealthInput): Finding {
112135
drivingMetricId: "concurrency",
113136
annotationCode: "pinned_minutes",
114137
exclusions: [],
115-
// States a measured fact (runs ARE completing at {rate}/min) — evidence the workers aren't
138+
// States a measured fact (runs ARE finishing at {rate}/min) — evidence the workers aren't
116139
// dead. An observation, not an exclusion: it doesn't claim it's the limit, nor "keeps pace".
117140
observations:
118-
donePerMin > 0 ? [{ code: "not_workers_platform", evidence: { donePerMin } }] : [],
141+
finishedPerMin > 0 ? [{ code: "not_workers_platform", evidence: { finishedPerMin } }] : [],
119142
recommendation: { code: "raise_env_limit", link: "concurrency" },
120143
usesAttribution: true,
121144
};
@@ -177,16 +200,22 @@ function assembleFlowCause(
177200

178201
// Anomaly window from the driving series. env_limit_saturation breaches ABOVE
179202
// (concurrency pinned at the limit); dequeue_stall breaches BELOW (capacity idle).
180-
// NOTE: runningSeries is at native env_metrics resolution (not resampled), so the "(last N
181-
// min)" figure assumes those buckets are uniform and cover the resolved window. env_metrics
182-
// are emitted on a fixed cadence, so that holds; a gappy/partial window could skew the minutes.
203+
// runningSeries is at native env_metrics resolution (not resampled) and is NOT gap-filled, so
204+
// the duration is counted per REAL bucket cadence with gaps breaking the contiguous run —
205+
// otherwise two fresh buckets would read as "the last 60 min". When the source can't report its
206+
// cadence we fall back to the (documented) even-spread assumption.
183207
let aw: Finding["anomalyWindow"];
184208
if (spec.reason === "env_limit_saturation" || spec.reason === "dequeue_stall") {
185209
const below = spec.reason === "dequeue_stall";
186210
const threshold = below
187211
? t.flowCause.stallRunningShare * input.flowEvidence.envLimit
188212
: t.flowCause.pinnedLevel * input.flowEvidence.envLimit;
189-
aw = anomalyWindow(input.flowEvidence.runningSeries, threshold, input.windowMinutes, { below });
213+
const coverage = bucketCoverage(input);
214+
aw = anomalyWindow(input.flowEvidence.runningSeries, threshold, input.windowMinutes, {
215+
below,
216+
bucketMinutes: coverage.known ? coverage.bucketMinutes : undefined,
217+
timestampsMs: input.flowEvidence.runningBucketsMs,
218+
});
190219
}
191220

192221
// Annotation on the driving metric (a fact, not an invented number).
@@ -295,6 +324,7 @@ const CAUSE_READS: Record<string, string> = {
295324

296325
export function buildFlowRead(flow: Finding, executionOk: boolean, livenessFresh: boolean): string {
297326
if (flow.reason === "unknown") return "data_stale"; // stale-guarded — no causal read
327+
if (flow.reason === FLOW_UNMEASURED) return "flow_unmeasured"; // no depth signal — no read
298328
if (isOk(flow.severity)) return "starting_normally";
299329
if (CAUSE_READS[flow.reason]) return CAUSE_READS[flow.reason];
300330
// fallback symptoms (v1 logic)

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

Lines changed: 119 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,30 @@ export type HealthInput = {
1919
* now = live env-level depth; normal = 7d baseline (omitted on the snapshot path, which has
2020
* no real 7d pending baseline — so we never mislabel a live-window average as "7d normal");
2121
* series measured (v2) or estimated (v1).
22+
*
23+
* `availability: "unknown"` = the depth could NOT be measured at all (Redis down with no
24+
* measured fallback, or the measured source failed for an unrecognized reason). `now` is then
25+
* a placeholder, NEVER a confident 0 — flow is reported unassessable instead of healthy.
2226
*/
23-
pending: { now: number; normal?: number; series: number[]; estimated: boolean };
27+
pending: {
28+
now: number;
29+
normal?: number;
30+
series: number[];
31+
estimated: boolean;
32+
availability?: "measured" | "unknown";
33+
};
2434
startLatency: { p95Ms: number; normalP95Ms: number; series: number[] };
25-
throughput: { donePerMin: number; triggeredPerMin: number; normalTriggeredPerMin: number };
35+
/**
36+
* finishedPerMin = ALL terminal runs per minute — the rate work actually LEAVES the queue, so
37+
* it's what net/drain math uses. completedPerMin (successes only) is an execution-side metric
38+
* and would understate the drain rate whenever runs fail/expire/cancel.
39+
*/
40+
throughput: {
41+
finishedPerMin: number;
42+
completedPerMin: number;
43+
triggeredPerMin: number;
44+
normalTriggeredPerMin: number;
45+
};
2646
failures: { rate: number; normalRate: number; series: number[] };
2747
duration: { p95Ms: number; normalP95Ms: number };
2848
/** Age of the freshest telemetry (ms). null = no signal to assess -> freshness unknown. */
@@ -33,6 +53,18 @@ export type HealthInput = {
3353
*/
3454
flowEvidence: {
3555
runningSeries: number[];
56+
/**
57+
* Epoch ms of each `runningSeries` bucket, aligned by index. Empty/absent when the source
58+
* can't say (snapshot path) — then contiguity falls back to index adjacency.
59+
*/
60+
runningBucketsMs?: number[];
61+
/**
62+
* Bucket cadence of `runningSeries` and how many buckets the window SHOULD contain. The rows
63+
* are NOT gap-filled, so this is the only way to tell "pinned for 60 of 60 minutes" from
64+
* "two fresh samples arrived in a 60-minute window". Absent = cadence unknown; the legacy
65+
* gap-free assumption then applies (received buckets spread evenly over the window).
66+
*/
67+
sampling?: { bucketMinutes: number; expectedBuckets: number } | null;
3668
envLimit: number;
3769
throttledShare: number;
3870
worstQueue: { name: string; share: number } | null;
@@ -67,6 +99,12 @@ export const HEALTH_THRESHOLDS = {
6799
// trigger_surge: with NO usable baseline (normal 0), a multiplier is meaningless, so an
68100
// absolute floor picks the "new volume" cause instead of dropping to the v1 fallback.
69101
surgePerMin: 100,
102+
// Minimum share of the window's EXPECTED buckets that must have arrived before a
103+
// concurrency-shaped cause (pin / stall) may be named. Below it the series is too gappy to
104+
// support a cause or a duration, so flow drops to a symptom-level verdict. Metric buckets are
105+
// produced by queue activity rather than a heartbeat, so a sparse window is normal for a quiet
106+
// env — and a saturation/stall claim isn't supportable there anyway.
107+
minCoverage: 0.5,
70108
},
71109
attribution: { minShare: 0.5 }, // name a queue/task/region only when it owns >= half the problem
72110
};
@@ -99,6 +137,55 @@ function multiplierSeverity(
99137
return classifySeverity(value / normal, { warn: warnMult, crit: critMult });
100138
}
101139

140+
/** True when the backlog depth could not be measured — `pending.now` is a placeholder. */
141+
export function isPendingUnknown(input: HealthInput): boolean {
142+
return input.pending.availability === "unknown";
143+
}
144+
145+
export type BucketCoverage = {
146+
/** buckets the window should contain at the source's cadence. */
147+
expectedBuckets: number;
148+
/** buckets that actually arrived. */
149+
receivedBuckets: number;
150+
/** minutes per bucket. */
151+
bucketMinutes: number;
152+
/** received / expected. 1 when the cadence is unknown (legacy gap-free assumption). */
153+
coverage: number;
154+
/** enough of the window arrived to support a cause + a duration. */
155+
sufficient: boolean;
156+
/** true when the source told us its cadence (so gaps are detectable at all). */
157+
known: boolean;
158+
};
159+
160+
/**
161+
* Coverage of the running series: how much of the window actually arrived. Without the source's
162+
* cadence we can only assume the received buckets span the window evenly (the pre-existing
163+
* assumption); with it, a gappy feed is visible and shares are expressed against EXPECTED buckets.
164+
*/
165+
export function bucketCoverage(input: HealthInput): BucketCoverage {
166+
const received = input.flowEvidence.runningSeries.length;
167+
const sampling = input.flowEvidence.sampling;
168+
if (!sampling || sampling.expectedBuckets <= 0) {
169+
return {
170+
expectedBuckets: received,
171+
receivedBuckets: received,
172+
bucketMinutes: received > 0 ? input.windowMinutes / received : 0,
173+
coverage: 1,
174+
sufficient: true,
175+
known: false,
176+
};
177+
}
178+
const coverage = received / sampling.expectedBuckets;
179+
return {
180+
expectedBuckets: sampling.expectedBuckets,
181+
receivedBuckets: received,
182+
bucketMinutes: sampling.bucketMinutes,
183+
coverage,
184+
sufficient: coverage >= HEALTH_THRESHOLDS.flowCause.minCoverage,
185+
known: true,
186+
};
187+
}
188+
102189
/** Look up a metric by id; throws if absent (buildMetrics guarantees the standard set exists). */
103190
export function metricById(metrics: Metric[], id: string): Metric {
104191
const m = metrics.find((x) => x.id === id);
@@ -133,32 +220,44 @@ export function buildMetrics(input: HealthInput): Metric[] {
133220
),
134221
};
135222

223+
// Unmeasurable depth: `now` is a placeholder, so it must not be CLASSIFIED (a placeholder 0
224+
// would read as a confident "no backlog" green). `availability: "unknown"` says so, and the
225+
// flow analyzer turns it into an unassessable verdict.
226+
const pendingUnknown = isPendingUnknown(input);
136227
const pending: Metric = {
137228
id: "pending",
138229
value: input.pending.now,
139230
unit: "count",
231+
availability: pendingUnknown ? "unknown" : "measured",
140232
normal: input.pending.normal,
141-
delta: delta(input.pending.now, input.pending.normal),
233+
delta: pendingUnknown ? undefined : delta(input.pending.now, input.pending.normal),
142234
series: {
143235
points: input.pending.series,
144236
kind: input.pending.estimated ? "estimated" : "measured",
145237
},
146-
severity: multiplierSeverity(
147-
input.pending.now,
148-
input.pending.normal,
149-
t.pending.warnMult,
150-
t.pending.critMult,
151-
t.pending.floor
152-
),
238+
severity: pendingUnknown
239+
? "ok"
240+
: multiplierSeverity(
241+
input.pending.now,
242+
input.pending.normal,
243+
t.pending.warnMult,
244+
t.pending.critMult,
245+
t.pending.floor
246+
),
153247
};
154248

155-
const net = input.throughput.donePerMin - input.throughput.triggeredPerMin;
249+
// Net drain uses FINISHED (all terminal) runs — every terminal run leaves the queue, so
250+
// completions alone would show a permanent deficit on any env with failures.
251+
const net = input.throughput.finishedPerMin - input.throughput.triggeredPerMin;
156252
const throughput: Metric = {
157253
id: "throughput",
158254
value: net,
159255
unit: "perMin",
160256
aggregation: "rate",
161-
breakdown: { done: input.throughput.donePerMin, triggered: input.throughput.triggeredPerMin },
257+
breakdown: {
258+
done: input.throughput.finishedPerMin,
259+
triggered: input.throughput.triggeredPerMin,
260+
},
162261
severity: net < 0 && isPendingIncreasing(input.pending.series) ? "warn" : "ok",
163262
};
164263

@@ -262,10 +361,15 @@ export function buildMetrics(input: HealthInput): Metric[] {
262361
// ---------------------------------------------------------------------------
263362

264363
export function computeDrain(input: HealthInput): { drainMinutes: number; isDrainable: boolean } {
265-
const donePerMin = input.throughput.donePerMin;
266-
const drainMinutes = donePerMin === 0 ? Number.POSITIVE_INFINITY : input.pending.now / donePerMin;
364+
// Drain rate = runs LEAVING the queue (all terminal), not just successful completions.
365+
const finishedPerMin = input.throughput.finishedPerMin;
366+
const drainMinutes =
367+
finishedPerMin === 0 ? Number.POSITIVE_INFINITY : input.pending.now / finishedPerMin;
267368
return {
268369
drainMinutes,
269-
isDrainable: drainMinutes < HEALTH_THRESHOLDS.flowPolicy.drainCritMinutes,
370+
// An unmeasurable depth can't produce an ETA — never offer "do nothing, it drains" off a
371+
// placeholder.
372+
isDrainable:
373+
!isPendingUnknown(input) && drainMinutes < HEALTH_THRESHOLDS.flowPolicy.drainCritMinutes,
270374
};
271375
}

0 commit comments

Comments
 (0)