fix: defer wide-event emit for streaming AI responses#367
Open
HugoRCD wants to merge 3 commits into
Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Thank you for following the naming conventions! 🙏 |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR defers wide-event emission for streaming HTTP responses until the response body completes, adds streaming-detection/lifecycle utilities, tracks pending emitted events to allow AI-only late merges, exposes a drain-start marker, updates middleware/adapters to use finishResponse, and adds tests and docs. ChangesStreaming emit deferral for AI SDK metadata
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labelsbug 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.changeset/streaming-ai-emit-defer.md:
- Line 5: Split the dense single-sentence changeset into 2–3 clearer sentences:
first state the main change ("Defer wide-event emit for streaming HTTP responses
... until the response body finishes") as one sentence, then add a second
sentence describing the effect on createAILogger metadata and prevention of
post-emit warnings, and a short third sentence listing the affected integrations
(Next.js withEvlog, SvelteKit, Hono, React Router, oRPC, Nitro/Nuxt) and the
merge behavior for late `ai` fields; preserve all original technical details and
wording but break into natural sentence boundaries for readability.
In `@apps/docs/content/2.learn/2.wide-events.md`:
- Around line 191-192: Update the sentence about "supported framework
integrations" to explicitly list the frameworks that defer wide-event emit so
readers know which integrations behave this way (reference the
createAILogger(log) metadata behavior and the wide-event emit mechanism); if you
prefer to keep it short, add a parenthetical pointer saying "see AI SDK usage
guide for full list" and/or mirror the same framework list used in the AI SDK
usage guide so wording is consistent with that document.
In `@apps/docs/content/3.integrate/frameworks/04.nitro.md`:
- Line 128: The note about post-emit warnings is potentially misleading for AI
SDK streaming — update the sentence referencing post-emit warnings to clarify
scope: explain that Nitro uses useLogger(event) (event-bound scope) so
log.fork() is unavailable, and that evlog defers wide-event emit for AI SDK
streaming so createAILogger(log) metadata remains attached until the response
body finishes; then either remove the blanket "Post-emit warnings still
apply..." clause or change it to a brief conditional statement saying warnings
only occur if code calls set() after the wide event has actually emitted (e.g.,
in non-streaming or background code paths), referencing useLogger(event),
log.fork(), createAILogger(log), evlog, wide-event emit and set() so readers can
locate the related concepts.
In `@packages/evlog/src/logger.ts`:
- Around line 891-894: The pending merge window allows AI data to be merged
after a drain even when deferDrain is false: update the logic around
pendingWideEvent/pendingDrainState so non-deferred loggers are not left with
drainStarted:false after immediate drain. Specifically, in emitWideEvent and
where pendingDrainState is initialized (symbols: pendingWideEvent,
pendingDrainState, emitWideEvent, deferDrain, and the set method that accepts {
ai }), either only create a pendingDrainState entry when deferDrain is true, or
set drainStarted:true immediately when deferDrain is false so subsequent set({
ai }) calls cannot merge into an already-drained event; apply the same rule
wherever pendingDrainState is populated to ensure AI data is not lost.
In `@packages/evlog/src/next/handler.ts`:
- Around line 232-241: The logger context is set to result.status before
streaming completes, so when a streaming response errors the final status
(meta.status) diverges; inside the bindStreamingResponseLifecycle completion
callback (the async meta => { ... } passed to bindStreamingResponseLifecycle)
update the logger context with the final status (use logger.set({ status:
meta.status ?? result.status })) before calling emitRequestEvent and logging
meta.error so the logger reflects the actual final response status; adjust the
block around bindStreamingResponseLifecycle, emitRequestEvent, and logger.set
accordingly.
In `@packages/evlog/src/shared/streamResponse.ts`:
- Around line 1-4: Add a JSDoc block above the exported StreamCompleteMeta
interface that documents the interface as a public API: describe that
StreamCompleteMeta represents the final metadata for a stream completion,
explain the status field as the HTTP-like numeric status code (number) and the
optional error field as an Error object present when the stream failed, and
include any relevant tags (e.g., `@export` or `@public`) per project convention;
place the comment immediately above the StreamCompleteMeta declaration to
satisfy the coding guideline for public APIs.
In `@packages/evlog/test/next/handler.test.ts`:
- Around line 7-24: Extract the duplicated createDeferredStream helper (and its
TextEncoder instance) into a shared helper module (e.g., helpers/stream) that
exports createDeferredStream, implement the safer close wrapper that throws if
close is uninitialized, then remove the local createDeferredStream definitions
in the three tests and import the shared createDeferredStream instead (update
usages in packages/evlog/test/shared/streamResponse.test.ts,
packages/evlog/test/next/handler.test.ts, and
packages/evlog/test/nitro/plugin.test.ts to use the exported function).
In `@packages/evlog/test/nitro/plugin.test.ts`:
- Around line 979-996: The createDeferredStream helper (and its encoder) is
duplicated across tests; extract it into a shared helper module (e.g.,
helpers/stream.ts) that exports createDeferredStream, keeping the same behavior
but replacing the inline close wrapper with a safe check that throws if close is
uninitialized, then update the tests (including the instance in plugin.test.ts)
to import createDeferredStream from the new module and remove the local
definition; reference symbols: createDeferredStream and encoder.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 43fc0668-d010-4d39-b0df-52cdc6ea24c8
⛔ Files ignored due to path filters (1)
packages/evlog/test/toolkit/__snapshots__/api-surface.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (21)
.changeset/streaming-ai-emit-defer.mdapps/docs/content/2.learn/2.wide-events.mdapps/docs/content/3.integrate/frameworks/02.nextjs.mdapps/docs/content/3.integrate/frameworks/04.nitro.mdapps/docs/content/4.use-cases/2.ai-sdk/02.usage.mdpackages/evlog/src/hono/index.tspackages/evlog/src/logger.tspackages/evlog/src/next/handler.tspackages/evlog/src/nitro-v3/plugin.tspackages/evlog/src/nitro/plugin.tspackages/evlog/src/orpc/index.tspackages/evlog/src/react-router/index.tspackages/evlog/src/shared/index.tspackages/evlog/src/shared/middleware.tspackages/evlog/src/shared/streamResponse.tspackages/evlog/src/sveltekit/index.tspackages/evlog/test/core/logger-request-logger.test.tspackages/evlog/test/frameworks/sveltekit.test.tspackages/evlog/test/next/handler.test.tspackages/evlog/test/nitro/plugin.test.tspackages/evlog/test/shared/streamResponse.test.ts
Co-authored-by: Hussain Arslan <m.hussain.arslan@gmail.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/evlog/src/shared/streamResponse.ts (1)
12-17:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd JSDoc comment to exported function.
Per coding guidelines, all public APIs require JSDoc comments. This function is exported but lacks documentation.
📝 Suggested JSDoc
+/** + * Whether a {`@link` Response} carries a non-null body stream. + */ export function isStreamingResponse(response: Response): boolean { return response.body !== null }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/evlog/src/shared/streamResponse.ts` around lines 12 - 17, The exported function isStreamingResponse lacks a JSDoc block; add a JSDoc comment above the isStreamingResponse function describing what it does, the parameter (response: Response) and its meaning, and the boolean return value (true when the Response has a non-null streaming body), following project JSDoc style (brief description, `@param`, `@returns`). Ensure the comment is placed directly above the export so tooling and docs pick it up.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/evlog/src/shared/streamResponse.ts`:
- Around line 12-17: The exported function isStreamingResponse lacks a JSDoc
block; add a JSDoc comment above the isStreamingResponse function describing
what it does, the parameter (response: Response) and its meaning, and the
boolean return value (true when the Response has a non-null streaming body),
following project JSDoc style (brief description, `@param`, `@returns`). Ensure the
comment is placed directly above the export so tooling and docs pick it up.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6ff37a97-c365-4ed4-a8cd-98141231b041
📒 Files selected for processing (11)
.changeset/streaming-ai-emit-defer.mdapps/docs/content/2.learn/2.wide-events.mdapps/docs/content/3.integrate/frameworks/04.nitro.mdpackages/evlog/src/logger.tspackages/evlog/src/next/handler.tspackages/evlog/src/shared/streamResponse.tspackages/evlog/test/core/logger-request-logger.test.tspackages/evlog/test/helpers/stream.tspackages/evlog/test/next/handler.test.tspackages/evlog/test/nitro/plugin.test.tspackages/evlog/test/shared/streamResponse.test.ts
Co-authored-by: Hussain Arslan <m.hussain.arslan@gmail.com>
commit: |
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
log.emit()until streaming response bodies finish (SSE, AI SDK UI streams, chunked bodies) across Next.js, SvelteKit, Hono, React Router, oRPC, and Nitro/Nuxt integrations.log.set({ ai })before enrich/drain to cover narrow race windows.createAILogger(log)API — no Nitro-specific wrapper.Closes #321 #363
Test plan
pnpm run lint && pnpm run test(1390 tests)pnpm --filter evlog exec vitest run test/next/handler.test.ts test/frameworks/sveltekit.test.ts test/nitro/plugin.test.ts test/core/logger-request-logger.test.ts test/shared/streamResponse.test.tsaiand no[evlog] Keys dropped: aiwarningSummary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests