Repository navigation
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new utility file open-sse/utils/stream/errors.ts to normalize and handle upstream stream failure payloads. The feedback highlights two main issues: unused imports (convertOpenAIToResponsesToolCall and uuidv4) that should be removed, and a violation of the repository style guide requiring unit tests to be added for any new production code under open-sse/.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| import { convertOpenAIToResponsesToolCall } from "../handlers/responseTranslator.ts"; | ||
| import { v4 as uuidv4 } from "uuid"; | ||
|
|
||
| import { asRecord } from "./utils.ts"; | ||
| import { JsonRecord, StreamFailurePayload } from "./types.ts"; |
There was a problem hiding this comment.
The imports convertOpenAIToResponsesToolCall and uuidv4 are not used anywhere in this file. They should be removed to keep the codebase clean and avoid unnecessary dependencies.
| import { convertOpenAIToResponsesToolCall } from "../handlers/responseTranslator.ts"; | |
| import { v4 as uuidv4 } from "uuid"; | |
| import { asRecord } from "./utils.ts"; | |
| import { JsonRecord, StreamFailurePayload } from "./types.ts"; | |
| import { asRecord } from "./utils.ts"; | |
| import { JsonRecord, StreamFailurePayload } from "./types.ts"; |
| return looksLikeStreamRateLimit(code, type || "", message) ? 429 : 502; | ||
| } | ||
|
|
||
| export function normalizeStreamFailurePayload(payload: unknown): StreamFailurePayload | null { |
There was a problem hiding this comment.
According to the Repository Style Guide (Rule 9), we must always include tests when changing or adding production code under open-sse/. Please add unit tests for these extracted stream error utility functions (e.g., toStreamFailureStatus, looksLikeStreamRateLimit, and normalizeStreamFailurePayload) inside the tests/unit/ directory.
References
- Always include tests when changing production code (src/, open-sse/, electron/, bin/). (link)
Part of Issue diegosouzapw#3594 modularization. Extracted error handling from the monolithic stream.ts. Exports: - toStreamFailureStatus, looksLikeStreamRateLimit, normalizeStreamFailurePayload
|
Thanks, @oyi77 🙏. These stream extractions are currently additive — the new modules under |
Part of modularization effort (Issue #3594).
Extracts error handling from the monolithic
open-sse/utils/stream.tsinto a dedicated module.Changes:
open-sse/utils/stream/errors.ts(87 lines)toStreamFailureStatus,looksLikeStreamRateLimit,normalizeStreamFailurePayloadTesting: No behavior change. Existing stream tests pass.
Follow-up PRs will extract: types (#3917), utils (#3918), textualToolCalls, sseFormatters, openaiChunks, claudeLifecycle, responsesLifecycle, and streamCore.