-
-
Notifications
You must be signed in to change notification settings - Fork 6
Fix streaming response body type checking for Node.js compatibility #292
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix streaming response body type checking for Node.js compatibility #292
Conversation
The 'in' operator fails when response.body is not a plain object (e.g., ReadableStream in Node.js). Added proper type checking before using the 'in' operator to prevent runtime errors. This resolves issues with streaming responses that contain ReadableStream bodies instead of plain objects. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
WalkthroughAdds runtime type guards before using the "in" operator on response.body in src/github_llms.ts, ensuring it’s an object before accessing choices and usage. Defaults to empty content and zero tokens when absent or non-object. Affects final response construction in the non-streaming path; streaming processing unchanged. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Caller
participant GM as githubModel
participant GH as GitHub API
C->>GM: request()
GM->>GH: fetch()
GH-->>GM: response { body }
alt body is object with choices/usage
GM->>GM: Access body.choices / body.usage
GM-->>C: result(message, tokens)
else body missing / non-object
GM->>GM: Fallback to empty message and zero tokens
GM-->>C: result(defaults)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes(no out-of-scope changes identified) Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/github_llms.ts (2)
1154-1161: Bug: deleting “falsy” values drops legitimate zeros (e.g., temperature/top_p = 0)The cleanup removes valid config values of 0. Only strip undefined/null or empty arrays.
- for (const key in body.body) { - if ( - !body.body[key] || - (Array.isArray(body.body[key]) && !body.body[key].length) - ) - delete body.body[key]; - } + for (const [k, v] of Object.entries(body.body)) { + if ( + v === undefined || + v === null || + (Array.isArray(v) && v.length === 0) + ) { + delete (body.body as any)[k]; + } + }
1112-1114: MIME type typo: should be "text/plain"Using "plain/text" will never match and can select the wrong response_format.
- const textMode = - request.output?.format === "text" || - request.output?.contentType === "plain/text"; + const textMode = + request.output?.format === "text" || + request.output?.contentType === "text/plain";
🧹 Nitpick comments (6)
src/github_llms.ts (6)
1205-1210: Good guard; also verify choices is an array with elements to avoid undefined accessStrengthen the condition so we don’t index into a non-array or empty array.
- (typeof response.body === 'object' && response.body && "choices" in response.body) + (typeof response.body === 'object' + && response.body + && "choices" in response.body + && Array.isArray((response.body as any).choices) + && (response.body as any).choices.length > 0) ? fromGithubChoice( - response.body.choices[0], + (response.body as any).choices[0], request.output?.format === "json", ).message : { role: "model", content: [] },
1213-1213: Always return numbers for inputTokensCoalesce undefined to 0 so callers don’t receive undefined.
- (typeof response.body === 'object' && response.body && "usage" in response.body) ? response.body.usage?.prompt_tokens : 0, + (typeof response.body === 'object' && response.body && "usage" in response.body) ? (response.body.usage?.prompt_tokens ?? 0) : 0,
1215-1217: Always return numbers for outputTokens- (typeof response.body === 'object' && response.body && "usage" in response.body) - ? response.body.usage?.completion_tokens - : 0, + (typeof response.body === 'object' && response.body && "usage" in response.body) + ? (response.body.usage?.completion_tokens ?? 0) + : 0,
1219-1219: Always return numbers for totalTokens- (typeof response.body === 'object' && response.body && "usage" in response.body) ? response.body.usage?.total_tokens : 0, + (typeof response.body === 'object' && response.body && "usage" in response.body) ? (response.body.usage?.total_tokens ?? 0) : 0,
355-359: Version string typo: “Instructt”- versions: ["Llama-4-Scout-17B-16E-Instructt"], + versions: ["Llama-4-Scout-17B-16E-Instruct"],
515-517: Model name typo: “24111” → “2411”The ref’s name includes an extra “1”.
- name: "github/Mistral-large-24111", + name: "github/Mistral-large-2411",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/github_llms.ts(1 hunks)
|
🎉 This PR is included in version 1.15.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Fixes #291
Summary
Changes
src/github_llms.tslines 1205, 1213, 1215, 1219(typeof response.body === 'object' && response.body && "choices" in response.body)Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit