fix(core): apply output language to side queries#4636
Conversation
| }; | ||
| } | ||
|
|
||
| async function getOutputLanguageInstruction( |
There was a problem hiding this comment.
[Suggestion] getOutputLanguageInstruction reads the file on every runSideQuery call. This is a hot path (13+ callers: chatCompressionService, ArenaManager, sessionRecap, toolUseSummary, etc.), and the file content is effectively static within a session (only changed via /language command).
Consider caching the result — either in the Config constructor (read once at startup) or as a module-level cache — to make this a synchronous lookup and avoid repeated disk I/O.
— qwen3.7-max via Qwen Code /review
| 'Follow the user-visible output language preference below for this side query.', | ||
| preference, | ||
| ].join('\n\n'); | ||
| } catch { |
There was a problem hiding this comment.
[Suggestion] The empty catch {} block silently swallows all file read errors without distinguishing between "file doesn't exist" (expected) and "file exists but unreadable" (EACCES, EISDIR, etc.). This makes configuration issues hard to diagnose.
Compare with sessionRecap.ts:85-89 and sessionTitle.ts:159-166 which use debugLogger.warn(...) to log failures. Consider adding a debug-level log here, or checking fs.existsSync first to separate expected missing files from unexpected I/O errors.
— qwen3.7-max via Qwen Code /review
Verification ReportBranch: Test Results
Code Review Notes
Verdict✅ Ready to merge — all test plan items pass, implementation is clean and well-tested. Verified by wenshao |
BZ-D
left a comment
There was a problem hiding this comment.
LGTM. appendSystemInstruction 对各种 systemInstruction 类型(string / Part[] / Content / undefined)的处理都正确,getOutputLanguageInstruction 里的错误处理也合理(读文件失败静默降级)。
Fixes #4494.
Summary
Test plan