[world] Require a runId on listByCorrelationId - #3280
Conversation
A correlation id names a step, hook or wait within its run, not across runs. Under slot event identity each run numbers its own steps, so `step_...001` is the first step of every slot-numbered run and an unscoped lookup answered with one event per such run. The scope is also what keeps the pagination cursor a key: an event id alone cannot tell two runs' rows apart.
🦋 Changeset detectedLatest commit: 7c7792d The changes in this PR will be included in the next version bump. This PR includes changesets to release 20 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
🧪 E2E Test Results✅ All tests passed E2E Test SummarySummary
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
✅ 📋 Other
✅ vercel-multi-region
|
📊 Workflow Benchmarkscommit Backend:
📈 STSO distribution vs main (inline / queue-hop histograms)1020 steps (inline) Cumulative STSO time: main 437759ms → this run 419570ms (Δ -18189ms, -4%) 1020 steps (queue-hop) Cumulative STSO time: main 8262ms → this run 7676ms (Δ -586ms, -7%) ℹ️ Metric definitions & methodologyThe collapsed STSO distribution section above buckets every step gap of the sequential-steps run (not a sampled window), split by whether the step ending the gap ran inline — in the same warm process as the step before it, so the gap is pure framework overhead — or after a queue-hop — the first step of a fresh process, which pays queue dispatch, client reinit and event-log replay. Bars overlay the two runs: Best/P75/P90/P99 deltas compare against the most recent benchmark run on Metrics — TTFS: time to first step body (in-deployment start() → first step body, deployment clocks) · STSO: step-to-step overhead (gap between consecutive step bodies) · WO: workflow overhead (whole-run time outside step bodies, in-deployment anchored) · SL: stream latency (in-deployment write → read propagation, readAt - writtenAt) · SO: stream overhead (end-to-end write+consume time beyond the modelled generation window) Scenarios — step: one trivial no-op step, no stream; no hooks, so the run stays in turbo mode (in-process fast path) · stream: one streaming step; no hooks, so the run stays in turbo mode (in-process fast path) · hook + stream: registers a hook before one step, which exits turbo mode (dispatch path) · 1020 steps: 1020 trivial sequential steps; STSO is measured between consecutive steps in the given step ranges, and WO is the whole-run overhead outside step bodies · stream latency: parallel reader/writer steps on a dedicated stream; SL is the in-deployment write->read propagation (readAt - writtenAt) · stream overhead (text): writer streams 300 variable-length text token deltas paced at 100/s for 3s (a haiku-size LLM's token throughput) while a parallel reader drains the whole stream; SO is the end-to-end write+consume time beyond the 3s generation window (overhead/backpressure) · stream overhead (structured): same workload as stream overhead (text), but each delta is an AI-SDK-style structured object ({ type: 'text-delta', id, text }) instead of a raw string, so the SO gap vs the text scenario is the added serialization cost 🔴 marks a percentile over its target (within target is left unmarked). Targets (p75/p90/p99, ms) — TTFS 200/300/600 · SL 50/60/125 · SO 250/500/1000 All metrics are measured from deployment-side timestamps only. Runs are triggered by an in-deployment route that stamps the anchor ( Cold starts are kept in the numbers on purpose — they are part of real bursty-workload latency. The workbench deployment cold-starts the |
TooTallNate
left a comment
TooTallNate
left a comment
There was a problem hiding this comment.
Reviewed at 7c7792d (base 3 days / 23 commits behind main — merges clean, no semantic overlap with the interim work).
Verified locally:
- Root
pnpm build+pnpm typecheckgreen; world-local 498, world-vercel 334, web 97 all pass. (No docker on this machine for the Postgres container suite — reviewed those tests statically and CI's run is green.) - Callsite sweep is complete: the only remaining
listByCorrelationIdreferences outside the four world implementations and the web callers are the interface declaration and the docs-typecheck ambient stub ((...args: any[])), which tolerates the new field. - Changeset semver is exactly right: major across
@workflow/world/world-local/world-postgres/world-vercelfor a breaking required param, minor on web. External world implementations get the correct signal.
Design points that held up under scrutiny:
- The Postgres cursor argument is real, not decorative: the query orders and resumes by
eventIdalone, and two runs genuinely can hold the same correlation id at the same slot — scoping restores(run_id, id)as the key. The limit-1 paging test past a sibling run is precisely the regression trap. - Local world: the
filePrefix:${runId}-`` scope matches the established file-naming convention used by the other four run-scoped queries in the same file, and the newassertSafeEntityId('runId', …)guard is a nice catch — runId now feeds a path prefix, so it needs the same injection defense correlationId already had. - Vercel analytics: the run-scoped endpoint is an established route family in this client, and I confirmed the backend's run-scoped events listing accepts the same
correlationIdfilter — so the reroute is a strict improvement (server-side scope instead of none). - Vercel runtime read: the client-side filter keeps the backend's
hasMore/cursoruntouched, so a page that filters to empty is still followed — and the web hook's paging loop keys on the cursor, not page contents (do … while (nextCursor)), so it can't terminate early on an all-foreign page. The retainedevent.runId === runIdpost-filter as defense-in-depth for worlds that can't scope is the right belt-and-suspenders, and theMAX_CORRELATION_SEARCH_PAGES+truncatedhandling bounds the hot-key cost until the backend-side scope lands (good that the follow-up is called out honestly). - The
'correlationId' in paramsdiscriminator ingetWorkflowRunEventsis sound against the params union —ListEventsParamsnever carriescorrelationId, and both variants now carryrunId.
Docs updates are accurate (required param documented with the cursor rationale, snippets updated), and the measured two-run repro in the PR description matches what the code change prevents.
CI green (only the standard permission-gated skips). Ship it.
|
No backport to This is a breaking API change ( To override, re-run the Backport to stable workflow manually via |
2 participants
Why
A correlation id identifies a step, hook or wait within its run, not across runs. Nothing enforced that while every id carried a ULID, so the distinction never mattered. It matters as soon as a run numbers its own steps:
step_…001names the first step of every slot-numbered run.Measured on the Local World before this change — two runs, each with one step, one unscoped lookup:
Two consequences:
runId, so nothing wrong is displayed — but it pages against a bounded page cap, so the run it wants can sit past the cap and the search reports nothing found.eventId. Two runs can hold the same correlation id at the same slot, soeventId > cursorskips the sibling row at the equal id. Scoped,(run_id, id)is the primary key, so the cursor is a key again.Nothing in the runtime reads this path — it is an observability read — so this is not a correctness regression in a run. It is a read that gets quietly wrong answers.
What
Breaking.
ListEventsByCorrelationIdParamsandAnalyticsListEventsByCorrelationIdParamsgain a requiredrunId. An unscoped lookup has no answer worth keeping: it means "every run that numbered one the same", which is never the question a caller is asking.AND run_id = $n, which also disambiguates the cursorhasMorestay the backend'sThe observability search passes the run it is already looking at, and keeps its post-filter as defence for a world that cannot scope.
This was previously the last commit of #3247, stacked behind the slot-identity SDK series. It stands alone: the bug it fixes predates slot identity, slot identity is only what makes it reachable.
Tests
Local 498 pass, Postgres 163 pass (real container), world-vercel 334, web 97, docs-typecheck 967. Root
pnpm buildandpnpm typecheckgreen.Docs
docs/content/docs/v5/api-reference/workflow-runtime/world/{storage,analytics}.mdxdocument the requirement. The v5 migration table and the v4→v5 migration skill are updated in #3100.Follow-up
The backend index behind the Vercel runtime read is keyed by correlation id alone, so a slot-mode
step_…001is a hot key across all runs and the scope is applied client-side. Taking a run scope there is a backend-side change, tracked separately.Docs Preview
Base: https://workflow-docs-git-peter-correlation-id-run-scope.vercel.sh (Vercel SSO).
runIddocumented as required, with why the scope is also what keeps the cursor a key; snippet and params table updatedanalytics.events.listByCorrelationIdsnippet passesrunId