Skip to content

fix(core): apply output language to side queries#4636

Merged
wenshao merged 1 commit into
QwenLM:mainfrom
he-yufeng:fix/side-query-output-language
May 31, 2026
Merged

fix(core): apply output language to side queries#4636
wenshao merged 1 commit into
QwenLM:mainfrom
he-yufeng:fix/side-query-output-language

Conversation

@he-yufeng

Copy link
Copy Markdown
Contributor

Fixes #4494.

Summary

  • append the configured output-language.md preference to runSideQuery system instructions
  • cover both JSON and text side queries so titles, recaps, tool summaries, and suggestions share the same language preference path
  • keep side queries best-effort if the language file is missing or unreadable

Test plan

  • npm exec -- vitest run packages/core/src/utils/sideQuery.test.ts
  • npm run typecheck --workspace=packages/core
  • npm exec -- eslint packages/core/src/utils/sideQuery.ts packages/core/src/utils/sideQuery.test.ts packages/core/src/config/config.ts
  • npm run build --workspace=packages/core
  • git diff --check

};
}

async function getOutputLanguageInstruction(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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 {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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

@wenshao

wenshao commented May 30, 2026

Copy link
Copy Markdown
Collaborator

Verification Report

Branch: fix/side-query-output-language
Environment: macOS Darwin 25.4.0, Node.js local

Test Results

Check Command Result
Unit Tests npm exec -- vitest run packages/core/src/utils/sideQuery.test.ts ✅ 20 tests passed (35ms)
Type Check npm run typecheck --workspace=packages/core ✅ Clean
Lint npm exec -- eslint packages/core/src/utils/sideQuery.ts packages/core/src/utils/sideQuery.test.ts packages/core/src/config/config.ts ✅ No errors
Build npm run build --workspace=packages/core ✅ Success
Whitespace git diff --check ✅ Clean

Code Review Notes

  • getOutputLanguageInstruction() gracefully handles missing/unreadable files with a try-catch returning undefined — good defensive design for best-effort behavior
  • appendSystemInstruction() correctly handles all systemInstruction type variants (string, Part[], Content, undefined)
  • New tests use real temp files (mkdtemp + cleanup) rather than mocking fs, providing stronger coverage
  • The change is additive and non-breaking — existing side queries without a configured language file behave identically

Verdict

✅ Ready to merge — all test plan items pass, implementation is clean and well-tested.


Verified by wenshao

@BZ-D BZ-D left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. appendSystemInstruction 对各种 systemInstruction 类型(string / Part[] / Content / undefined)的处理都正确,getOutputLanguageInstruction 里的错误处理也合理(读文件失败静默降级)。

@wenshao wenshao merged commit 1c48e41 into QwenLM:main May 31, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Side queries ignore the user's configured output language (recap, title, tool-use summary, suggestions)

3 participants