Add functional SSE gateway with keep-alive streaming#89
Merged
Conversation
Review Summary by QodoAdd functional SSE gateway with keep-alive streaming
WalkthroughsDescription• Add functional SSE gateway with persistent streaming support • Implement intent detection based on query parameters • Include keep-alive heartbeats and proper SSE headers • Add npm script and Express dependency for easy execution Diagramflowchart LR
A["Express Server"] -->|GET /sse| B["Intent Detection"]
B -->|Query Parameter| C["Tool Mapping"]
C -->|SSE Stream| D["Status Event"]
D -->|300ms delay| E["Result Event"]
E -->|15s intervals| F["Keep-alive Heartbeat"]
F -->|Client Close| G["Cleanup Timers"]
File Changes1. apps/sse-gateway/server.mjs
|
Hướng Dẫn cho Người ReviewGiới thiệu một ứng dụng gateway SSE mới dựa trên Express với endpoint /sse lâu sống, được nối vào các npm script và dependencies, và tài liệu hướng dẫn cách chạy và kiểm thử hành vi streaming. Biểu đồ trình tự cho luồng streaming SSE /ssesequenceDiagram
actor Client
participant ExpressApp
participant SSEHandler
participant detectIntent
Client->>ExpressApp: HTTP GET /sse?q=email
ExpressApp->>SSEHandler: Route /sse handler
SSEHandler->>detectIntent: detectIntent(q)
detectIntent-->>SSEHandler: intent, tools
SSEHandler->>Client: Set SSE headers
SSEHandler->>Client: flushHeaders (if available)
SSEHandler->>Client: event: status\ndata: {status:"processing"}
Note over SSEHandler,Client: After 300 ms
SSEHandler->>Client: event: result\ndata: {query, intent, tools}
loop Every 15 seconds
SSEHandler->>Client: : keep-alive
end
Client-->>ExpressApp: Connection close
ExpressApp->>SSEHandler: req close event
SSEHandler->>SSEHandler: clearTimeout(resultTimer)
SSEHandler->>SSEHandler: clearInterval(heartbeat)
SSEHandler->>Client: end()
Thay Đổi Cấp Độ Tệp
Mẹo và câu lệnhTương tác với Sourcery
Tùy chỉnh Trải Nghiệm của BạnTruy cập dashboard của bạn để:
Nhận Hỗ Trợ
Original review guide in EnglishReviewer's GuideIntroduces a new Express-based SSE gateway application with a persistent /sse endpoint, wires it into npm scripts and dependencies, and documents how to run and test the streaming behaviour. Sequence diagram for the SSE /sse streaming flowsequenceDiagram
actor Client
participant ExpressApp
participant SSEHandler
participant detectIntent
Client->>ExpressApp: HTTP GET /sse?q=email
ExpressApp->>SSEHandler: Route /sse handler
SSEHandler->>detectIntent: detectIntent(q)
detectIntent-->>SSEHandler: intent, tools
SSEHandler->>Client: Set SSE headers
SSEHandler->>Client: flushHeaders (if available)
SSEHandler->>Client: event: status\ndata: {status:"processing"}
Note over SSEHandler,Client: After 300 ms
SSEHandler->>Client: event: result\ndata: {query, intent, tools}
loop Every 15 seconds
SSEHandler->>Client: : keep-alive
end
Client-->>ExpressApp: Connection close
ExpressApp->>SSEHandler: req close event
SSEHandler->>SSEHandler: clearTimeout(resultTimer)
SSEHandler->>SSEHandler: clearInterval(heartbeat)
SSEHandler->>Client: end()
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Code Review by Qodo
|
CI Feedback 🧐(Feedback updated until commit 5177ccd)A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Chào bạn - Mình đã để lại một số phản hồi tổng quan:
- Hãy cân nhắc tách
detectIntentra thành một module riêng hoặc ít nhất là một file riêng để logic ánh xạ intent/tool có thể được dùng lại và mở rộng độc lập với các vấn đề về truyền SSE trongserver.mjs. - Sẽ hữu ích nếu bạn định nghĩa khoảng thời gian heartbeat và độ trễ trả kết quả thành các hằng số được đặt tên (ví dụ:
RESULT_DELAY_MS,HEARTBEAT_INTERVAL_MS) thay vì dùng số literal trực tiếp, để việc tinh chỉnh sau này và suy luận về hành vi thời gian trở nên dễ dàng hơn.
Prompt cho AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider extracting `detectIntent` into its own module or at least a separate file so that intent/tool mapping logic can be reused and extended independently of the SSE transport concerns in `server.mjs`.
- It might be helpful to define the heartbeat interval and result delay as named constants (e.g., `RESULT_DELAY_MS`, `HEARTBEAT_INTERVAL_MS`) instead of inline literals to make future tuning and reasoning about timing behavior easier.Sourcery miễn phí cho open source - nếu bạn thấy các review này hữu ích, hãy cân nhắc chia sẻ chúng ✨
Original comment in English
Hey - I've left some high level feedback:
- Consider extracting
detectIntentinto its own module or at least a separate file so that intent/tool mapping logic can be reused and extended independently of the SSE transport concerns inserver.mjs. - It might be helpful to define the heartbeat interval and result delay as named constants (e.g.,
RESULT_DELAY_MS,HEARTBEAT_INTERVAL_MS) instead of inline literals to make future tuning and reasoning about timing behavior easier.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider extracting `detectIntent` into its own module or at least a separate file so that intent/tool mapping logic can be reused and extended independently of the SSE transport concerns in `server.mjs`.
- It might be helpful to define the heartbeat interval and result delay as named constants (e.g., `RESULT_DELAY_MS`, `HEARTBEAT_INTERVAL_MS`) instead of inline literals to make future tuning and reasoning about timing behavior easier.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
Adds a runnable Express-based Server-Sent Events (SSE) gateway intended for long-lived streaming (keep-alives) to support demos and integration testing.
Changes:
- Added an Express SSE server (
/sse) with status/result events, heartbeat comments, and disconnect cleanup. - Documented how to run and verify streaming behavior via
curl -N. - Updated root
package.jsonwith ansse:gatewayscript and anexpressdependency.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| package.json | Adds express dependency and a script to run the SSE gateway. |
| apps/sse-gateway/server.mjs | Implements an Express server exposing / health JSON and /sse persistent SSE stream. |
| apps/sse-gateway/README.md | Provides run/test instructions for the new gateway. |
Comment on lines
+26
to
+27
| "@playwright/test": "^1.56.1", | ||
| "express": "^4.21.2" |
There was a problem hiding this comment.
express was added to dependencies, but package-lock.json was not updated accordingly. This will leave installs non-reproducible and can cause CI failures due to lockfile drift; please regenerate/commit an updated lockfile that includes express (or remove the lockfile change requirement if the repo intentionally doesn't track it).
Suggested change
| "@playwright/test": "^1.56.1", | |
| "express": "^4.21.2" | |
| "@playwright/test": "^1.56.1" |
| import express from "express"; | ||
|
|
||
| const app = express(); | ||
| const PORT = Number(process.env.PORT) || 5000; |
There was a problem hiding this comment.
const PORT = Number(process.env.PORT) || 5000; treats PORT=0 as falsy and will incorrectly fall back to 5000. Prefer checking for undefined/empty instead of using ||, so an explicit 0 (or other valid numeric values) is honored.
Suggested change
| const PORT = Number(process.env.PORT) || 5000; | |
| const rawPort = process.env.PORT; | |
| const PORT = | |
| rawPort === undefined || rawPort === "" | |
| ? 5000 | |
| : Number.isNaN(Number(rawPort)) | |
| ? 5000 | |
| : Number(rawPort); |
Comment on lines
+6
to
+7
| npm i express | ||
| node apps/sse-gateway/server.mjs |
There was a problem hiding this comment.
The run instructions suggest npm i express and running the server directly, but the PR also adds express to the root package.json and an npm run sse:gateway script. To avoid mismatched dependency state and to match the documented workflow, update this README to use npm install (root) and npm run sse:gateway (optionally mentioning PORT=...).
Suggested change
| npm i express | |
| node apps/sse-gateway/server.mjs | |
| npm install | |
| npm run sse:gateway | |
| # optional: PORT=5000 npm run sse:gateway |
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| "dependencies": { | ||
| "@playwright/test": "^1.56.1" | ||
| "@playwright/test": "^1.56.1", | ||
| "express": "^4.21.2" |
There was a problem hiding this comment.
1. Lockfile out of sync 🐞 Bug ☼ Reliability
express was added to package.json but package-lock.json was not updated, so lockfile-based installs can fail or omit express, causing apps/sse-gateway/server.mjs to crash at startup with a missing module error.
Agent Prompt
## Issue description
`package.json` declares `express`, but `package-lock.json` is not updated accordingly. This makes installs non-reproducible and can break lockfile-based installs, and can lead to runtime failure when starting the SSE gateway.
## Issue Context
The new SSE server imports `express`. The repo has an existing `package-lock.json` whose root dependency list does not include `express`.
## Fix Focus Areas
- package-lock.json[1-75]
- package.json[25-28]
## Suggested fix
1. Run `npm install` (or `npm install --package-lock-only`) at the repo root so the lockfile includes `express`.
2. Commit the updated `package-lock.json` (and ensure it adds the `express` package entries).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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
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.
Motivation
process.env.PORTwith a local fallback.Description
apps/sse-gateway/server.mjsthat reads the port fromprocess.env.PORTand binds to0.0.0.0./sseas a persistent SSE stream with proper headers, optionalres.flushHeaders(), an initialstatusevent, a delayedresultevent, periodic: keep-aliveheartbeats, and cleanup on client disconnect viareq.on("close").apps/sse-gateway/README.mdshowing how to run and verify withcurl -N.package.jsonto declareexpressand add annpm run sse:gatewayscript.Testing
node --check apps/sse-gateway/server.mjssucceeded and reported no syntax errors.npm install --package-lock-onlyfailed in this environment due to an external npm registry403 Forbiddenpolicy, so dependency installation was not completed here.Codex Task
Summary by Sourcery
Thêm một ứng dụng cổng SSE chuyên dụng dựa trên Express, cung cấp luồng server‑sent events sống lâu phục vụ kiểm thử tích hợp và demo.
Tính năng mới:
Cải tiến:
process.env.PORTvới giá trị dự phòng cục bộ cho các môi trường được host.Build:
Tài liệu:
curl.Original summary in English
Summary by Sourcery
Add a dedicated Express-based SSE gateway application that provides a long‑lived server‑sent events stream for integration testing and demos.
New Features:
Enhancements:
Build:
Documentation: