Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/core/src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2675,6 +2675,10 @@ export class Config {
return this.userMemory;
}

getOutputLanguageFilePath(): string | undefined {
return this.outputLanguageFilePath;
}

setUserMemory(newUserMemory: string): void {
this.userMemory = newUserMemory;
}
Expand Down
67 changes: 67 additions & 0 deletions packages/core/src/utils/sideQuery.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { mkdtemp, rm, writeFile } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import path from 'node:path';
import { describe, it, expect, vi, beforeEach } from 'vitest';
import type { BaseLlmClient } from '../core/baseLlmClient.js';
import type { Config } from '../config/config.js';
Expand All @@ -24,6 +27,7 @@ describe('runSideQuery', () => {
getBaseLlmClient: vi.fn().mockReturnValue(mockBaseLlmClient),
getModel: vi.fn().mockReturnValue('main-model'),
getFastModel: vi.fn().mockReturnValue(undefined),
getOutputLanguageFilePath: vi.fn().mockReturnValue(undefined),
} as unknown as Config;
});

Expand Down Expand Up @@ -141,6 +145,41 @@ describe('runSideQuery', () => {
);
});

it('adds the configured output language to JSON side queries', async () => {
const dir = await mkdtemp(path.join(tmpdir(), 'qwen-side-query-'));
try {
const outputLanguagePath = path.join(dir, 'output-language.md');
await writeFile(outputLanguagePath, '请始终用中文回答用户可见文本。');
vi.mocked(mockConfig.getOutputLanguageFilePath).mockReturnValue(
outputLanguagePath,
);
vi.mocked(mockBaseLlmClient.generateJson).mockResolvedValue({
title: '测试标题',
});

await runSideQuery<{ title: string }>(mockConfig, {
purpose: 'session-title',
contents: [{ role: 'user', parts: [{ text: 'title please' }] }],
schema: {
type: 'object',
properties: { title: { type: 'string' } },
required: ['title'],
},
abortSignal: abortController.signal,
systemInstruction: 'Generate a short title.',
});

const callArg = vi.mocked(mockBaseLlmClient.generateJson).mock
.calls[0][0];
expect(callArg.systemInstruction).toContain('Generate a short title.');
expect(callArg.systemInstruction).toContain(
'请始终用中文回答用户可见文本。',
);
} finally {
await rm(dir, { recursive: true, force: true });
}
});

it('throws when the response does not satisfy the schema', async () => {
vi.mocked(mockBaseLlmClient.generateJson).mockResolvedValue({
status: 'ok',
Expand Down Expand Up @@ -354,6 +393,34 @@ describe('runSideQuery', () => {
);
});

it('adds the configured output language to text side queries', async () => {
const dir = await mkdtemp(path.join(tmpdir(), 'qwen-side-query-'));
try {
const outputLanguagePath = path.join(dir, 'output-language.md');
await writeFile(outputLanguagePath, 'Respond in Spanish.');
vi.mocked(mockConfig.getOutputLanguageFilePath).mockReturnValue(
outputLanguagePath,
);
mockTextResult('ok');

await runSideQuery(mockConfig, {
purpose: 'tool-use-summary',
contents: [{ role: 'user', parts: [{ text: 'summarize tool use' }] }],
abortSignal: abortController.signal,
systemInstruction: 'Summarize the tool batch.',
});

const callArg = vi.mocked(mockBaseLlmClient.generateText).mock
.calls[0][0];
expect(callArg.systemInstruction).toContain(
'Summarize the tool batch.',
);
expect(callArg.systemInstruction).toContain('Respond in Spanish.');
} finally {
await rm(dir, { recursive: true, force: true });
}
});

it('omits systemInstruction when caller does not provide one', async () => {
mockTextResult('ok');

Expand Down
53 changes: 51 additions & 2 deletions packages/core/src/utils/sideQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { readFile } from 'node:fs/promises';
import type {
Content,
GenerateContentConfig,
Expand Down Expand Up @@ -123,6 +124,50 @@ function applyThinkingDefault(
};
}

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

config: Config,
): Promise<string | undefined> {
const outputLanguageFilePath = config.getOutputLanguageFilePath?.();
if (!outputLanguageFilePath) return undefined;

try {
const preference = (await readFile(outputLanguageFilePath, 'utf8')).trim();
if (!preference) return undefined;

return [
'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

return undefined;
}
}

function appendSystemInstruction(
systemInstruction: string | Part | Part[] | Content | undefined,
outputLanguageInstruction: string | undefined,
): string | Part | Part[] | Content | undefined {
if (!outputLanguageInstruction) return systemInstruction;
if (systemInstruction === undefined) return outputLanguageInstruction;
if (typeof systemInstruction === 'string') {
return `${systemInstruction}\n\n${outputLanguageInstruction}`;
}
if (Array.isArray(systemInstruction)) {
return [...systemInstruction, { text: outputLanguageInstruction }];
}
if (
typeof systemInstruction === 'object' &&
'parts' in systemInstruction &&
Array.isArray(systemInstruction.parts)
) {
return {
...systemInstruction,
parts: [...systemInstruction.parts, { text: outputLanguageInstruction }],
};
}
return [systemInstruction as Part, { text: outputLanguageInstruction }];
}

function isJsonOptions<TResponse>(
options: SideQueryTextOptions | SideQueryJsonOptions<TResponse>,
): options is SideQueryJsonOptions<TResponse> {
Expand All @@ -147,14 +192,18 @@ export async function runSideQuery<TResponse>(
const model = resolveDefaultModel(config, options.model);
const promptId = options.promptId ?? buildDefaultPromptId(options.purpose);
const requestConfig = applyThinkingDefault(options.config);
const systemInstruction = appendSystemInstruction(
options.systemInstruction,
await getOutputLanguageInstruction(config),
);

if (isJsonOptions(options)) {
const response = (await config.getBaseLlmClient().generateJson({
contents: options.contents,
schema: options.schema,
abortSignal: options.abortSignal,
model,
systemInstruction: options.systemInstruction,
systemInstruction,
promptId,
config: requestConfig,
...(options.maxAttempts !== undefined && {
Expand All @@ -178,7 +227,7 @@ export async function runSideQuery<TResponse>(
const result = await config.getBaseLlmClient().generateText({
contents: options.contents,
model,
systemInstruction: options.systemInstruction,
systemInstruction,
abortSignal: options.abortSignal,
promptId,
config: requestConfig,
Expand Down
Loading