Skip to content

Commit bd4d4a1

Browse files
d-csclaude
andcommitted
fix(webapp): short-circuit ClickHouse lookup on mollifier-buffered runs in events endpoint
`api.v1.runs.$runId.events.ts` calls `ApiRetrieveRunPresenter.findRun`, which now returns a buffered fallback. The route then proceeds to `getRunEvents` against ClickHouse with `run.traceId` (empty string for buffered runs) and `run.taskEventStore` (`"taskEvent"` default) — a guaranteed-empty round trip, since the mollifier gate intercepts BEFORE any trace event is written. Devin's analysis on PR #3755 flagged this as a wasted ClickHouse round trip per request hitting the events endpoint with a buffered runId. Add an `isBuffered: boolean` flag to `FoundRun`. The PG branch of `findRun` sets `isBuffered: false` on the spread Prisma row; the buffered branch's `synthesiseFoundRunFromBuffer` sets `isBuffered: true`. The events route short-circuits on the flag and returns `{ events: [] }` with status 200, matching the semantically-correct behaviour without the ClickHouse hop. Gating on the explicit flag (rather than probing surrogates like `traceId === ""`) keeps the contract clear for future callers that need the same "PG-only" gate — they can introspect the same field. Includes a unit test in `mollifierSynthesiseFoundRun.test.ts` pinning `isBuffered: true` on the synth. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 0afa415 commit bd4d4a1

3 files changed

Lines changed: 32 additions & 1 deletion

File tree

apps/webapp/app/presenters/v3/ApiRetrieveRunPresenter.server.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,15 @@ export type FoundRun = CommonRelatedRun & {
9090
parentTaskRun: CommonRelatedRun | null;
9191
rootTaskRun: CommonRelatedRun | null;
9292
childRuns: CommonRelatedRun[];
93+
// True when this run was synthesised from the mollifier buffer rather
94+
// than read from Postgres. Callers that would otherwise query backing
95+
// stores keyed on PG identifiers (e.g. ClickHouse event lookups by
96+
// traceId) can short-circuit to an empty response — buffered runs
97+
// haven't executed and have no events to fetch. Devin's analysis on
98+
// PR #3755 (events endpoint) flagged the pre-fix code as making a
99+
// wasted ClickHouse round-trip when this is set; gate on this flag
100+
// instead.
101+
isBuffered: boolean;
93102
};
94103

95104
export class ApiRetrieveRunPresenter {
@@ -132,7 +141,7 @@ export class ApiRetrieveRunPresenter {
132141
},
133142
});
134143

135-
if (pgRow) return pgRow;
144+
if (pgRow) return { ...pgRow, isBuffered: false };
136145

137146
// Postgres miss → fall back to the mollifier buffer. When the gate
138147
// diverted a trigger, the run lives in Redis until the drainer replays
@@ -668,5 +677,6 @@ export function synthesiseFoundRunFromBuffer(buffered: SyntheticRun): FoundRun {
668677
parentTaskRun: null,
669678
rootTaskRun: null,
670679
childRuns: [],
680+
isBuffered: true,
671681
};
672682
}

apps/webapp/app/routes/api.v1.runs.$runId.events.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,16 @@ export const loader = createLoaderApiRoute(
3838
},
3939
},
4040
async ({ resource: run, authentication }) => {
41+
// Short-circuit for mollifier-buffered runs. The drainer hasn't
42+
// materialised execution events yet (the gate intercepts before
43+
// any trace event is written), so a ClickHouse round-trip is
44+
// guaranteed to come back empty. `findRun` now sets `isBuffered`
45+
// explicitly on its return value — gate on that rather than
46+
// probing surrogate fields like `traceId === ""`.
47+
if (run.isBuffered) {
48+
return json({ events: [] }, { status: 200 });
49+
}
50+
4151
const eventRepository = await getEventRepositoryForStore(
4252
run.taskEventStore,
4353
authentication.environment.organization.id

apps/webapp/test/mollifierSynthesiseFoundRun.test.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,17 @@ describe("synthesiseFoundRunFromBuffer", () => {
6666
expect(found.friendlyId).toBe("run_friendly_1");
6767
});
6868

69+
it("marks the synth as isBuffered=true so callers like the events route can short-circuit ClickHouse lookups", () => {
70+
// The PG path of `findRun` sets `isBuffered: false`; the buffered
71+
// path goes through `synthesiseFoundRunFromBuffer` and must set
72+
// `isBuffered: true` so consumers (e.g. the events endpoint) can
73+
// skip queries that are guaranteed to return empty for buffered
74+
// runs without rewriting them around surrogate signals like
75+
// `traceId === ""`.
76+
const found: FoundRun = synthesiseFoundRunFromBuffer(makeSyntheticRun());
77+
expect(found.isBuffered).toBe(true);
78+
});
79+
6980
it("forwards scheduleId from the snapshot so resolveSchedule can hydrate the schedule field", () => {
7081
// Regression: scheduleId was previously hardcoded to null, dropping the
7182
// schedule metadata for buffered scheduled runs even though the snapshot

0 commit comments

Comments
 (0)