Repository navigation
fix(tests,translator): repair post-merge regressions on release/v3.8.2 - #2597
diegosouzapw wants to merge 1 commit into
Conversation
Two corrections to PRs that merged without their review fixes: 1. #2536 deleted the isCacheable export but tests/unit/semantic-cache.test.ts still imported it — the whole unit suite failed to load. Rewrote the block against the current isCacheableForRead / isCacheableForWrite API (caches streaming + non-streaming; requires explicit temperature: 0). 2. #2474's extractSystemRoleMessages only lifted role=system, dropping role=developer (OpenAI Responses' system alias) into the Anthropic message array — the exact 400 the PR set out to prevent. Now lifts both, matching the normal-path extractSystemMessagesToBody. Adds a developer-role test.
|
Kilo Code Review could not run — your account is out of credits. Add credits or switch to a free model to enable reviews on this change. |
|
Closing as redundant: rebasing this branch onto the current |
![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 updates the system role extraction logic to include the "developer" role as a system-equivalent role for OpenAI parity and refactors the semantic cache into distinct read and write variants. The caching logic now requires an explicit temperature of 0 and supports streaming requests. A review comment identifies that the new isSystemRole helper is duplicated elsewhere in the file and should be moved to a shared scope to improve maintainability.
| const isSystemRole = (role: unknown): boolean => | ||
| typeof role === "string" && | ||
| (role.toLowerCase() === "system" || role.toLowerCase() === "developer"); |
There was a problem hiding this comment.
The isSystemRole helper logic is now duplicated with the internal extractSystemMessagesToBody closure found later in this file (lines 2670-2673). While the current implementation correctly fixes the missing developer role regression, consider moving this helper to a module-level scope or a shared utility file to maintain a single source of truth for system-equivalent roles and improve maintainability across the handler.
1 participant
Summary
Two corrections to PRs that merged without their review fixes:
isCacheableexport buttests/unit/semantic-cache.test.tsstill imported it — the whole unit suite failed to load. Rewritten against the currentisCacheableForRead/isCacheableForWriteAPI (caches streaming + non-streaming; requires explicittemperature: 0).extractSystemRoleMessagesonly liftedrole=system, droppingrole=developer(OpenAI Responses' system alias) into the Anthropic message array — the exact 400 the PR set out to prevent. Now lifts both, matching the normal-pathextractSystemMessagesToBody. Adds a developer-role test.Files
open-sse/handlers/chatCore.ts,tests/unit/semantic-cache.test.ts,tests/unit/system-role-extraction.test.ts(3 files, +63/-23).🤖 Generated with Claude Code