Uh oh!
There was an error while loading. Please reload this page.
[core] Bound event pagination against non-progressing pages - #2187
Closed
VaguelySerious wants to merge 4 commits into
Closed
[core] Bound event pagination against non-progressing pages#2187VaguelySerious wants to merge 4 commits into
VaguelySerious wants to merge 4 commits into
Conversation
A backend that keeps returning fresh cursors with empty or fully-overlapping pages while reporting `hasMore: true` would spin the `loadWorkflowRunEvents` loop forever — the existing cursor guards only catch a cursor that repeats a previously requested value, and `appendUniqueEvents` keeps the result flat instead of growing. Treat a non-final page that adds zero new events as a world contract error. This fully closes the non-progression hole without an arbitrary page-count cap. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 0531d82 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 |
Contributor
Contributor
📊 Benchmark Results
workflow with no steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 1 step💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 10 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 25 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 50 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Promise.all with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) Promise.all with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Promise.all with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) Promise.race with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) Promise.race with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Promise.race with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 10 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 25 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 50 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 10 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 25 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 50 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Stream Benchmarks(includes TTFB metrics)workflow with stream💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) stream pipeline with 5 transform steps (1MB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) 10 parallel streams (1MB each)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) fan-out fan-in 10 streams (1MB each)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) SummaryFastest Framework by WorldWinner determined by most benchmark wins
Fastest World by FrameworkWinner determined by most benchmark wins
Column Definitions
Worlds:
|
Contributor
🧪 E2E Test Results✅ All tests passed Summary
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
✅ 📋 Other
|
Uh oh!
There was an error while loading. Please reload this page.
Signed-off-by: Peter Wielander <mittgfu@gmail.com>
pranaygp
reviewed
Jun 8, 2026
| // spin `while (hasMore)` forever while `appendUniqueEvents` keeps the result | ||
| // flat. Treating zero forward progress as a contract violation bounds the | ||
| // loop without an arbitrary page cap. | ||
| if (!addedNewEvents) { |
Contributor
There was a problem hiding this comment.
This tightens the events.list contract for every @workflow/world implementation: a non-final page must now contain at least one event that is new relative to earlier pages in the same load. The shared pagination docs still describe cursor as an opaque token in packages/world/src/shared.ts, so a community world that legitimately replays a fully overlapping page before reaching new rows would now start throwing WORLD_CONTRACT_ERROR after a core upgrade. Please codify that stronger progress guarantee in @workflow/world in the same PR so the new failure mode is part of the public contract, not just a runtime assumption.
MemberAuthor
There was a problem hiding this comment.
AI Review: Note
Good catch. Codified the forward-progress guarantee in the events.list contract in packages/world/src/interfaces.ts (a hasMore: true page must surface at least one event not seen in an earlier page of the same load; a hasMore: false page may legitimately be empty), and cross-referenced it from the shared hasMore field in packages/world/src/shared.ts. So the WORLD_CONTRACT_ERROR failure mode is now part of the public contract rather than just a runtime assumption. Done in fdd53a1.
Document the progress guarantee that core now enforces: a `hasMore: true` page must surface at least one new event. Addresses review feedback on the public-contract gap in @workflow/world. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #2179 (comment) which hardened event list cursor parsing, to further protect it against cursor llops.
These changes specifically protect against a backend that restarts the read at the beginning (it re-issues a cursor we already requested). It does not bound the loop against a backend that keeps returning fresh cursors with empty or fully-overlapping pages while reporting
hasMore: true: no cursor ever repeats,appendUniqueEventskeeps the result flat, andwhile (hasMore)spins forever.The change is to treat a non-final page that adds zero new events as a
WORLD_CONTRACT_ERROR. A legitimate non-final page over an ascending cursor must surface at least one event after the cursor, so this can't false-positive on large runs — and it needs no arbitrary page-count cap (avoids a magic number that would need sizing above the largest legitimate run).