Conversation
…from OpenAIService
…d streamGenerateContent method
WalkthroughAdds a V4 HTTP streaming endpoint and service wiring, moves streaming logic into a base generative service, forwards an Changes
Sequence DiagramsequenceDiagram
participant Client
participant V4Controller as V4 Controller
participant GenerateService as Generate Service
participant BaseService as Base Service
participant LLMProvider as LLM Provider
Client->>V4Controller: POST /stream-http/llm (content, type, options)
V4Controller->>V4Controller: validateStreamRequest()\nSet Transfer-Encoding: chunked
V4Controller->>GenerateService: streamGenerate(payload)
GenerateService->>BaseService: streamGenerateContent(payload)
rect rgba(100, 150, 200, 0.5)
Note over BaseService,LLMProvider: Main streaming loop
BaseService->>BaseService: generatePromptText(type, content, options)
BaseService->>BaseService: getResponseFormatForGenerationType(type)
BaseService->>LLMProvider: request streaming with response_format
loop For each chunk
LLMProvider-->>BaseService: chunk data
BaseService-->>GenerateService: yield {status, data}
GenerateService-->>V4Controller: yield chunk
V4Controller->>V4Controller: Check drain before write
V4Controller-->>Client: stream chunk
end
end
rect rgba(200, 100, 100, 0.5)
Note over V4Controller,Client: Error handling & cleanup
alt Client disconnect
Client->>V4Controller: close/abort
V4Controller->>V4Controller: cancel stream, cleanup listeners
else LLM error
LLMProvider-->>V4Controller: error
V4Controller->>V4Controller: log and attempt inline error write
end
V4Controller->>V4Controller: remove listeners, end response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/routes/generate/v4/generate.routes.ts`:
- Around line 10-12: The streaming LLM endpoint lacks request throttling—wrap
the authenticated route that registers
generateController.httpStreamGenerateContent with the existing
rateLimitMiddleware (same pattern used for upload routes) to protect resources;
update the router.use/auth order if needed so authMiddleware still runs and
apply rateLimitMiddleware with an appropriate window and max (e.g., short window
+ low max for heavy streaming ops) before calling
generateController.httpStreamGenerateContent to ensure authenticated requests
are rate-limited.
In `@src/services/generative/base/base.abstract.ts`:
- Around line 149-167: The code lowercases type for the prompt lookup (using
lowercase(type) into TYPE_PROMPT_MAPPING) but passes the original type to
getResponseFormatForGenerationType, causing case-sensitive mismatches (e.g.,
'SHORT_SUMMARY' won't match 'short_summary'); update streamGenerateContent so it
reuses the normalized key (the lowercase value stored in key) when calling
getResponseFormatForGenerationType (or alternatively make
getResponseFormatForGenerationType perform its own normalization), ensuring
comparisons like the 'short_summary' branch are done against a lowercased value
so response_format is selected correctly.
In `@src/utils/prompt/constant/prompt.constant.ts`:
- Around line 10-18: TYPE_PROMPT_MAPPING is missing mappings for types declared
in TYPE_PROMPT which causes undefined to be passed into generatePromptText;
either add the missing keys ('multiple_choice', 'true_false', 'fill_bank') to
TYPE_PROMPT_MAPPING mapped to their corresponding enum strings
('MULTIPLE_CHOICE', 'TRUE_FALSE', 'FILL_BANK') so lookups return valid values,
or if those types are unused remove them from the TYPE_PROMPT union; update the
mapping object in prompt.constant.ts (TYPE_PROMPT_MAPPING) and/or the
TYPE_PROMPT type so generatePromptText in base.abstract.ts always receives a
defined prompt type.
🧹 Nitpick comments (1)
src/services/generative/v3/generative.service.ts (1)
53-53: Consider using the imported constant directly.The class field
private readonly TYPE_PROMPT_MAPPING = TYPE_PROMPT_MAPPING;shadows the imported constant without modification. Unless you intend to allow subclass overrides, consider using the imported constant directly to reduce indirection.♻️ Optional refactor
Remove line 53 and update usages at lines 210 and 567 to reference the imported
TYPE_PROMPT_MAPPINGdirectly.
…w BadRequest on invalid type
|
There was a problem hiding this comment.
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 (1)
src/utils/prompt/quiz.prompt.ts (1)
48-48: Potential TypeError: unsafe access tolistType.length.Lines 22-26 defensively handle potentially undefined
listTypeusing optional chaining and fallbacks, but line 48 accesseslistType.lengthdirectly. IflistTypeis undefined/null, this will throw a TypeError.Use the already-computed safe variable
typeCountinstead.🐛 Proposed fix
- - The first question type (${firstType}) should generate ${remaining} questions in total, which includes ${numberOfItem % listType.length} additional question(s) from the remainder. + - The first question type (${firstType}) should generate ${remaining} questions in total, which includes ${numberOfItem % typeCount} additional question(s) from the remainder.
🧹 Nitpick comments (3)
src/services/generative/base/base.abstract.ts (3)
1-1: Unused import:ResponseFormatJSONSchema.
ResponseFormatJSONSchemais imported but never used in the return values ofgetResponseFormatForGenerationType. Consider removing it to keep imports clean.🧹 Suggested fix
-import { ResponseFormatJSONObject, ResponseFormatJSONSchema, ResponseFormatText } from 'openai/resources/shared'; +import { ResponseFormatJSONObject, ResponseFormatText } from 'openai/resources/shared';
168-170: Reconsider using'connected'status for every chunk.Yielding
{ status: 'connected' }for every data chunk is semantically misleading. "Connected" typically indicates an initial connection event, not ongoing data transmission. Consider using a more descriptive status like'data'or'chunk', or emit'connected'only once at the start of the stream.♻️ Suggested approach
public async *streamGenerateContent(payload: GenerateContentRequestInterface) { // ... validation logic ... + yield { status: 'connected' }; + for await (const chunk of this.getLLMProvider().handleProcessStreamContent(prompt, { response_format })) { - yield { status: 'connected', data: chunk }; + yield { status: 'data', data: chunk }; } }
217-228: Simplify return type to match actual returned values.The return type includes
ResponseFormatJSONSchemabut this type is never returned. Consider simplifying:🧹 Suggested fix
protected getResponseFormatForGenerationType( type: string - ): ResponseFormatText | ResponseFormatJSONSchema | ResponseFormatJSONObject | undefined { + ): ResponseFormatText | ResponseFormatJSONObject { switch (type) { case 'short_summary': { return { type: 'text' }; } default: { return { type: 'json_object' }; } } }
1 participant
What changes on:
ISSUE:
Summary by CodeRabbit
New Features
Refactor
Behavior Change
✏️ Tip: You can customize this high-level summary in your review settings.