Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors the application's backend proxying logic by centralizing stream handling into a single, robust utility. The primary goal is to eliminate duplicated code paths for Server-Sent Events (SSE) and standardize how long-lived connections and their associated authentication and error handling are managed across various API routes. This change streamlines the codebase, making it more maintainable and consistent, and sets the stage for further simplification of backend fetch handlers. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and valuable refactoring by unifying the server-sent event (SSE) stream proxy logic into a shared proxyStream helper. This greatly simplifies the route handlers for studio/chat, gen-code, and various research-related endpoints, making them more consistent and easier to maintain. The addition of the passthroughNonStreamResponse option to handle mixed stream/JSON responses is a clever solution that cleans up complex logic in the paperscool/daily route. The new tests for the helper and each migrated route are also well-written and improve test coverage.
I've found one minor issue in the new proxyStream helper related to the fallback content type for non-stream responses, for which I've left a specific comment with a suggested fix. Overall, this is an excellent improvement.
| ) { | ||
| const text = await upstream.text() | ||
| return buildTextResponse(text, upstream, { | ||
| responseContentType: requestOptions.responseContentType ?? "application/json", |
There was a problem hiding this comment.
The expression requestOptions.responseContentType ?? "application/json" is currently equivalent to requestOptions.responseContentType because requestOptions.responseContentType is always defined (it defaults to "text/event-stream").
This can lead to a bug when passthroughNonStreamResponse is true and the upstream API returns a non-stream response (e.g., JSON) without a Content-Type header. In that scenario, the response will be incorrectly served with Content-Type: text/event-stream.
For non-stream passthrough responses, the fallback content type should be application/json to correctly handle this case.
| responseContentType: requestOptions.responseContentType ?? "application/json", | |
| responseContentType: "application/json", |
There was a problem hiding this comment.
Pull request overview
Refactors several Next.js API route handlers that proxy to the backend (including SSE endpoints) to use a shared proxyStream helper, consolidating streaming proxy behavior and adding targeted Vitest coverage.
Changes:
- Added
proxyStreamtoweb/src/app/api/_utils/backend-proxy.ts(SSE dispatcher + optional JSON passthrough). - Updated multiple API routes to use
proxyStreaminstead of bespoke fetch/SSE handling. - Added route-level tests verifying correct
proxyStreaminvocation and error fallback behavior.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| web/src/app/api/studio/chat/route.ts | Switch studio chat POST to shared stream proxy helper. |
| web/src/app/api/studio/chat/route.test.ts | Adds test to confirm shared proxy helper usage. |
| web/src/app/api/research/repro/context/route.ts | Uses shared stream proxy for POST context generation while keeping existing GET proxy. |
| web/src/app/api/research/repro/context/route.test.ts | Adds tests for GET proxy behavior and POST stream proxy usage. |
| web/src/app/api/research/paperscool/daily/route.ts | Replaces custom SSE/JSON fallback proxying with proxyStream options. |
| web/src/app/api/research/paperscool/daily/route.test.ts | Adds test for stream proxy options and onError fallback response. |
| web/src/app/api/research/paperscool/analyze/route.ts | Replaces custom SSE proxying with proxyStream. |
| web/src/app/api/research/paperscool/analyze/route.test.ts | Adds test to confirm shared proxy helper usage. |
| web/src/app/api/gen-code/route.ts | Switch gen-code POST to shared stream proxy helper. |
| web/src/app/api/gen-code/route.test.ts | Adds test to confirm shared proxy helper usage. |
| web/src/app/api/_utils/backend-proxy.ts | Introduces proxyStream and undici dispatcher support for long-lived streams. |
| web/src/app/api/_utils/backend-proxy.test.ts | Adds tests for SSE passthrough and non-stream fallback behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| ) { | ||
| const text = await upstream.text() | ||
| return buildTextResponse(text, upstream, { | ||
| responseContentType: requestOptions.responseContentType ?? "application/json", |
jerry609
force-pushed
the
refactor/pr13-next-json-fallback-proxies
branch
from
2462ff9 to
2310e36
Compare
jerry609
force-pushed
the
refactor/pr14-next-stream-proxies
branch
from
dcba4e9 to
706ab9e
Compare
jerry609
changed the base branch from
refactor/pr13-next-json-fallback-proxies
to
dev
jerry609
force-pushed
the
refactor/pr14-next-stream-proxies
branch
from
706ab9e to
f4648a9
Compare
Vercel Preview
|
|
There was a problem hiding this comment.
Pull request overview
This PR centralizes Server-Sent Events (SSE) proxying for Next.js API routes by introducing a shared proxyStream helper and updating several streaming routes to use it, alongside new contract/tests to prevent regressions.
Changes:
- Add
proxyStream(SSE-aware) to the shared backend proxy utilities, including long-lived dispatcher support and optional JSON passthrough fallback. - Refactor streaming routes (studio chat, gen-code, paperscool analyze/daily, repro context generation) to use
proxyStreaminstead of bespoke fetch/stream plumbing. - Add/extend Vitest coverage for streaming route contracts and the new proxy helper behavior.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| web/src/app/api/studio/chat/route.ts | Switch studio chat SSE route to shared proxyStream helper. |
| web/src/app/api/research/repro/context/route.ts | Route POST (context generation) now proxies via proxyStream; GET remains on existing _base proxy helper. |
| web/src/app/api/research/repro/context/route.test.ts | Adds unit tests ensuring GET stays on _base and POST uses proxyStream with expected options. |
| web/src/app/api/research/paperscool/daily/route.ts | Replace custom dispatcher + dual SSE/JSON handling with proxyStream + passthrough fallback and custom error handler. |
| web/src/app/api/research/paperscool/analyze/route.ts | Replace custom dispatcher-based SSE proxy with proxyStream and custom error handler. |
| web/src/app/api/gen-code/route.ts | Switch gen-code SSE route to shared proxyStream helper. |
| web/src/app/api/_utils/stream-route-contracts.test.ts | Adds contract tests enforcing that key streaming routes call proxyStream with stable options. |
| web/src/app/api/_utils/backend-proxy.ts | Introduces proxyStream, shared SSE dispatcher, and fetch support for undici dispatcher. |
| web/src/app/api/_utils/backend-proxy.test.ts | Adds coverage for SSE passthrough behavior and non-stream fallback behavior in proxyStream. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| ) { | ||
| const text = await upstream.text() | ||
| return buildTextResponse(text, upstream, { | ||
| responseContentType: "application/json", |
What changed
proxyStream()to the shared Next backend proxy helper with long-lived SSE support, optional JSON passthrough, shared auth handling, and no default request timeoutstudio/chat,gen-code,research/paperscool/daily,research/paperscool/analyze, andresearch/repro/contextPOSTpaperscool/dailyJSON fast-path and the explicit502 Upstream API unreachablefallback shape for the research stream routes that already had itWhy
Validation
npx vitest run src/app/api/_utils/backend-proxy.test.ts src/app/api/_utils/auth-headers.test.ts src/app/api/papers/export/route.test.ts src/app/api/studio/chat/route.test.ts src/app/api/gen-code/route.test.ts src/app/api/research/paperscool/daily/route.test.ts src/app/api/research/paperscool/analyze/route.test.ts src/app/api/research/repro/context/route.test.tsnpx eslint src/app/api/_utils/backend-proxy.ts src/app/api/_utils/backend-proxy.test.ts src/app/api/studio/chat/route.ts src/app/api/studio/chat/route.test.ts src/app/api/gen-code/route.ts src/app/api/gen-code/route.test.ts src/app/api/research/paperscool/daily/route.ts src/app/api/research/paperscool/daily/route.test.ts src/app/api/research/paperscool/analyze/route.ts src/app/api/research/paperscool/analyze/route.test.ts src/app/api/research/repro/context/route.ts src/app/api/research/repro/context/route.test.tsnpx tsc --noEmitnpm run buildRemaining cleanup
web/src/app/api/papers/export/route.tsweb/src/app/api/newsletter/unsubscribe/[token]/route.ts