From 2ad18c2a62f78b08bd2b32d4895cde61da42b29f Mon Sep 17 00:00:00 2001 From: wenshao Date: Sat, 4 Apr 2026 13:23:16 +0800 Subject: [PATCH 01/13] fix(core): prevent followup suggestion input/output from appearing in tool call UI The follow-up suggestion generation was leaking into the conversation UI through three channels: 1. The forked query included tools in its generation config, allowing the model to produce function calls during suggestion generation. Fixed by setting `tools: []` in runForkedQuery's per-request config (kept in createForkedChat for speculation which needs tools). 2. logApiResponse and logApiError recorded suggestion API events to the chatRecordingService, causing them to appear in session JSONL files and the WebUI. Fixed by adding isInternalPromptId() guard that skips chatRecordingService for 'prompt_suggestion' and 'forked_query' IDs. uiTelemetryService.addEvent() is preserved so /stats still tracks suggestion token usage. 3. LoggingContentGenerator logged suggestion requests/responses to the OpenAI logger and telemetry pipeline. Fixed by skipping logApiRequest, buildOpenAIRequestForLogging, and logOpenAIInteraction for internal prompt IDs. _logApiResponse is preserved (for /stats) but its chatRecordingService path is filtered by fix #2. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../loggingContentGenerator.ts | 47 ++++++++++++++++--- packages/core/src/followup/forkedQuery.ts | 6 ++- packages/core/src/telemetry/loggers.ts | 17 ++++++- 3 files changed, 59 insertions(+), 11 deletions(-) diff --git a/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts b/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts index 4f4b00138cf..274644b239b 100644 --- a/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts +++ b/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts @@ -74,6 +74,15 @@ export class LoggingContentGenerator implements ContentGenerator { return this.wrapped; } + /** + * Returns true if the promptId belongs to an internal background operation + * (e.g., suggestion generation, forked queries) whose input/output should + * not be recorded to OpenAI logs or other persistent stores. + */ + private isInternalPromptId(promptId: string): boolean { + return promptId === 'prompt_suggestion' || promptId === 'forked_query'; + } + private logApiRequest( contents: Content[], model: string, @@ -143,8 +152,17 @@ export class LoggingContentGenerator implements ContentGenerator { userPromptId: string, ): Promise { const startTime = Date.now(); - this.logApiRequest(this.toContents(req.contents), req.model, userPromptId); - const openaiRequest = await this.buildOpenAIRequestForLogging(req); + const isInternal = this.isInternalPromptId(userPromptId); + if (!isInternal) { + this.logApiRequest( + this.toContents(req.contents), + req.model, + userPromptId, + ); + } + const openaiRequest = isInternal + ? undefined + : await this.buildOpenAIRequestForLogging(req); try { const response = await this.wrapped.generateContent(req, userPromptId); const durationMs = Date.now() - startTime; @@ -155,12 +173,16 @@ export class LoggingContentGenerator implements ContentGenerator { userPromptId, response.usageMetadata, ); - await this.logOpenAIInteraction(openaiRequest, response); + if (!isInternal) { + await this.logOpenAIInteraction(openaiRequest, response); + } return response; } catch (error) { const durationMs = Date.now() - startTime; this._logApiError('', durationMs, error, req.model, userPromptId); - await this.logOpenAIInteraction(openaiRequest, undefined, error); + if (!isInternal) { + await this.logOpenAIInteraction(openaiRequest, undefined, error); + } throw error; } } @@ -170,8 +192,17 @@ export class LoggingContentGenerator implements ContentGenerator { userPromptId: string, ): Promise> { const startTime = Date.now(); - this.logApiRequest(this.toContents(req.contents), req.model, userPromptId); - const openaiRequest = await this.buildOpenAIRequestForLogging(req); + const isInternal = this.isInternalPromptId(userPromptId); + if (!isInternal) { + this.logApiRequest( + this.toContents(req.contents), + req.model, + userPromptId, + ); + } + const openaiRequest = isInternal + ? undefined + : await this.buildOpenAIRequestForLogging(req); let stream: AsyncGenerator; try { @@ -179,7 +210,9 @@ export class LoggingContentGenerator implements ContentGenerator { } catch (error) { const durationMs = Date.now() - startTime; this._logApiError('', durationMs, error, req.model, userPromptId); - await this.logOpenAIInteraction(openaiRequest, undefined, error); + if (!isInternal) { + await this.logOpenAIInteraction(openaiRequest, undefined, error); + } throw error; } diff --git a/packages/core/src/followup/forkedQuery.ts b/packages/core/src/followup/forkedQuery.ts index 798374c7323..7ab1f884c80 100644 --- a/packages/core/src/followup/forkedQuery.ts +++ b/packages/core/src/followup/forkedQuery.ts @@ -191,8 +191,10 @@ export async function runForkedQuery( const model = options?.model ?? params.model; const chat = createForkedChat(config, params); - // Build per-request config overrides for JSON schema if needed - const requestConfig: GenerateContentConfig = {}; + // Build per-request config overrides for JSON schema if needed. + // Strip tools — forked queries are pure text completion and must never + // produce function calls or appear inside tool-call UI elements. + const requestConfig: GenerateContentConfig = { tools: [] }; if (options?.abortSignal) { requestConfig.abortSignal = options.abortSignal; } diff --git a/packages/core/src/telemetry/loggers.ts b/packages/core/src/telemetry/loggers.ts index b7c18c9d30b..8121dd011de 100644 --- a/packages/core/src/telemetry/loggers.ts +++ b/packages/core/src/telemetry/loggers.ts @@ -121,6 +121,15 @@ function getCommonAttributes(config: Config): LogAttributes { export { getCommonAttributes }; +/** + * Returns true if the prompt_id belongs to an internal background operation + * (suggestion generation, forked queries) whose events should not be + * recorded to the chatRecordingService. + */ +function isInternalPromptId(promptId: string): boolean { + return promptId === 'prompt_suggestion' || promptId === 'forked_query'; +} + export function logStartSession( config: Config, event: StartSessionEvent, @@ -382,7 +391,9 @@ export function logApiError(config: Config, event: ApiErrorEvent): void { 'event.timestamp': new Date().toISOString(), } as UiEvent; uiTelemetryService.addEvent(uiEvent); - config.getChatRecordingService()?.recordUiTelemetryEvent(uiEvent); + if (!isInternalPromptId(event.prompt_id)) { + config.getChatRecordingService()?.recordUiTelemetryEvent(uiEvent); + } QwenLogger.getInstance(config)?.logApiErrorEvent(event); if (!isTelemetrySdkInitialized()) return; @@ -449,7 +460,9 @@ export function logApiResponse(config: Config, event: ApiResponseEvent): void { 'event.timestamp': new Date().toISOString(), } as UiEvent; uiTelemetryService.addEvent(uiEvent); - config.getChatRecordingService()?.recordUiTelemetryEvent(uiEvent); + if (!isInternalPromptId(event.prompt_id)) { + config.getChatRecordingService()?.recordUiTelemetryEvent(uiEvent); + } QwenLogger.getInstance(config)?.logApiResponseEvent(event); if (!isTelemetrySdkInitialized()) return; const attributes: LogAttributes = { From 3c28d03fa79cf358b14ed0332026db12e747480c Mon Sep 17 00:00:00 2001 From: wenshao Date: Sat, 4 Apr 2026 13:37:00 +0800 Subject: [PATCH 02/13] refactor: deduplicate isInternalPromptId into shared export from loggers.ts Address review feedback: extract isInternalPromptId() to a single exported function in telemetry/loggers.ts and import it in LoggingContentGenerator, eliminating the duplicate private method. Also update loggingContentGenerator.test.ts mock to use importOriginal so the real isInternalPromptId is available during tests. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../loggingContentGenerator.test.ts | 15 ++++++++++----- .../loggingContentGenerator.ts | 14 +++----------- packages/core/src/telemetry/loggers.ts | 6 ++++-- 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts b/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts index b39e653572e..8c3486a42f0 100644 --- a/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts +++ b/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts @@ -23,11 +23,16 @@ import { import { OpenAILogger } from '../../utils/openaiLogger.js'; import type OpenAI from 'openai'; -vi.mock('../../telemetry/loggers.js', () => ({ - logApiRequest: vi.fn(), - logApiResponse: vi.fn(), - logApiError: vi.fn(), -})); +vi.mock('../../telemetry/loggers.js', async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + logApiRequest: vi.fn(), + logApiResponse: vi.fn(), + logApiError: vi.fn(), + }; +}); vi.mock('../../utils/openaiLogger.js', () => ({ OpenAILogger: vi.fn().mockImplementation(() => ({ diff --git a/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts b/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts index 274644b239b..a6bc8dafb8b 100644 --- a/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts +++ b/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts @@ -30,6 +30,7 @@ import { logApiError, logApiRequest, logApiResponse, + isInternalPromptId, } from '../../telemetry/loggers.js'; import type { ContentGenerator, @@ -74,15 +75,6 @@ export class LoggingContentGenerator implements ContentGenerator { return this.wrapped; } - /** - * Returns true if the promptId belongs to an internal background operation - * (e.g., suggestion generation, forked queries) whose input/output should - * not be recorded to OpenAI logs or other persistent stores. - */ - private isInternalPromptId(promptId: string): boolean { - return promptId === 'prompt_suggestion' || promptId === 'forked_query'; - } - private logApiRequest( contents: Content[], model: string, @@ -152,7 +144,7 @@ export class LoggingContentGenerator implements ContentGenerator { userPromptId: string, ): Promise { const startTime = Date.now(); - const isInternal = this.isInternalPromptId(userPromptId); + const isInternal = isInternalPromptId(userPromptId); if (!isInternal) { this.logApiRequest( this.toContents(req.contents), @@ -192,7 +184,7 @@ export class LoggingContentGenerator implements ContentGenerator { userPromptId: string, ): Promise> { const startTime = Date.now(); - const isInternal = this.isInternalPromptId(userPromptId); + const isInternal = isInternalPromptId(userPromptId); if (!isInternal) { this.logApiRequest( this.toContents(req.contents), diff --git a/packages/core/src/telemetry/loggers.ts b/packages/core/src/telemetry/loggers.ts index 8121dd011de..7e04b8aa669 100644 --- a/packages/core/src/telemetry/loggers.ts +++ b/packages/core/src/telemetry/loggers.ts @@ -124,9 +124,11 @@ export { getCommonAttributes }; /** * Returns true if the prompt_id belongs to an internal background operation * (suggestion generation, forked queries) whose events should not be - * recorded to the chatRecordingService. + * recorded to the chatRecordingService or OpenAI logs. + * + * Known internal IDs: `'prompt_suggestion'`, `'forked_query'`. */ -function isInternalPromptId(promptId: string): boolean { +export function isInternalPromptId(promptId: string): boolean { return promptId === 'prompt_suggestion' || promptId === 'forked_query'; } From 662a123b43740474c10b59b3742b0f1af7a13fc4 Mon Sep 17 00:00:00 2001 From: wenshao Date: Sat, 4 Apr 2026 13:39:31 +0800 Subject: [PATCH 03/13] refactor: extract isInternalPromptId to shared utils, add tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address maintainer review feedback: 1. Move isInternalPromptId() to packages/core/src/utils/internalPromptIds.ts using a ReadonlySet for the ID registry. Adding new internal prompt IDs only requires changing one file. loggers.ts re-exports for compatibility, loggingContentGenerator.ts imports directly from utils. 2. Extract `tools: []` magic value to a frozen NO_TOOLS constant in forkedQuery.ts. 3. Add unit tests for isInternalPromptId: prompt_suggestion → true, forked_query → true, user_query → false, empty string → false. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../loggingContentGenerator.ts | 2 +- packages/core/src/followup/forkedQuery.ts | 11 ++++--- packages/core/src/telemetry/loggers.ts | 13 ++------ .../core/src/utils/internalPromptIds.test.ts | 31 +++++++++++++++++++ packages/core/src/utils/internalPromptIds.ts | 28 +++++++++++++++++ 5 files changed, 70 insertions(+), 15 deletions(-) create mode 100644 packages/core/src/utils/internalPromptIds.test.ts create mode 100644 packages/core/src/utils/internalPromptIds.ts diff --git a/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts b/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts index a6bc8dafb8b..7126a960303 100644 --- a/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts +++ b/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts @@ -30,8 +30,8 @@ import { logApiError, logApiRequest, logApiResponse, - isInternalPromptId, } from '../../telemetry/loggers.js'; +import { isInternalPromptId } from '../../utils/internalPromptIds.js'; import type { ContentGenerator, ContentGeneratorConfig, diff --git a/packages/core/src/followup/forkedQuery.ts b/packages/core/src/followup/forkedQuery.ts index 7ab1f884c80..80310339d28 100644 --- a/packages/core/src/followup/forkedQuery.ts +++ b/packages/core/src/followup/forkedQuery.ts @@ -21,6 +21,9 @@ import type { import { GeminiChat, StreamEventType } from '../core/geminiChat.js'; import type { Config } from '../config/config.js'; +/** Per-request config that strips tools so the model never produces function calls. */ +const NO_TOOLS: GenerateContentConfig = Object.freeze({ tools: [] }); + /** * Snapshot of the main conversation's cache-critical parameters. * Captured after each successful main turn so forked queries share the same prefix. @@ -191,10 +194,10 @@ export async function runForkedQuery( const model = options?.model ?? params.model; const chat = createForkedChat(config, params); - // Build per-request config overrides for JSON schema if needed. - // Strip tools — forked queries are pure text completion and must never - // produce function calls or appear inside tool-call UI elements. - const requestConfig: GenerateContentConfig = { tools: [] }; + // Build per-request config overrides. + // NO_TOOLS prevents the model from producing function calls — forked + // queries are pure text completion and must not appear in tool-call UI. + const requestConfig: GenerateContentConfig = { ...NO_TOOLS }; if (options?.abortSignal) { requestConfig.abortSignal = options.abortSignal; } diff --git a/packages/core/src/telemetry/loggers.ts b/packages/core/src/telemetry/loggers.ts index 7e04b8aa669..acfcd5d63cd 100644 --- a/packages/core/src/telemetry/loggers.ts +++ b/packages/core/src/telemetry/loggers.ts @@ -8,6 +8,7 @@ import type { LogAttributes, LogRecord } from '@opentelemetry/api-logs'; import { logs } from '@opentelemetry/api-logs'; import { SemanticAttributes } from '@opentelemetry/semantic-conventions'; import type { Config } from '../config/config.js'; +import { isInternalPromptId } from '../utils/internalPromptIds.js'; import { safeJsonStringify } from '../utils/safeJsonStringify.js'; import { EVENT_API_ERROR, @@ -121,16 +122,8 @@ function getCommonAttributes(config: Config): LogAttributes { export { getCommonAttributes }; -/** - * Returns true if the prompt_id belongs to an internal background operation - * (suggestion generation, forked queries) whose events should not be - * recorded to the chatRecordingService or OpenAI logs. - * - * Known internal IDs: `'prompt_suggestion'`, `'forked_query'`. - */ -export function isInternalPromptId(promptId: string): boolean { - return promptId === 'prompt_suggestion' || promptId === 'forked_query'; -} +// Re-export for consumers that import from this module. +export { isInternalPromptId } from '../utils/internalPromptIds.js'; export function logStartSession( config: Config, diff --git a/packages/core/src/utils/internalPromptIds.test.ts b/packages/core/src/utils/internalPromptIds.test.ts new file mode 100644 index 00000000000..357c3805142 --- /dev/null +++ b/packages/core/src/utils/internalPromptIds.test.ts @@ -0,0 +1,31 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect } from 'vitest'; +import { isInternalPromptId } from './internalPromptIds.js'; + +describe('isInternalPromptId', () => { + it('returns true for prompt_suggestion', () => { + expect(isInternalPromptId('prompt_suggestion')).toBe(true); + }); + + it('returns true for forked_query', () => { + expect(isInternalPromptId('forked_query')).toBe(true); + }); + + it('returns false for user_query', () => { + expect(isInternalPromptId('user_query')).toBe(false); + }); + + it('returns false for empty string', () => { + expect(isInternalPromptId('')).toBe(false); + }); + + it('returns false for arbitrary prompt ids', () => { + expect(isInternalPromptId('btw-prompt-id')).toBe(false); + expect(isInternalPromptId('context-prompt-id')).toBe(false); + }); +}); diff --git a/packages/core/src/utils/internalPromptIds.ts b/packages/core/src/utils/internalPromptIds.ts new file mode 100644 index 00000000000..ea6cce57bc1 --- /dev/null +++ b/packages/core/src/utils/internalPromptIds.ts @@ -0,0 +1,28 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + * + * Internal Prompt ID utilities + * + * Centralises the set of prompt IDs used by background operations + * (suggestion generation, forked queries) so that logging, recording, + * and UI layers can consistently recognise and filter them. + */ + +/** Prompt IDs that belong to internal background operations. */ +const INTERNAL_PROMPT_IDS: ReadonlySet = new Set([ + 'prompt_suggestion', + 'forked_query', +]); + +/** + * Returns true if the prompt_id belongs to an internal background operation + * whose events should not be recorded to the chatRecordingService, + * OpenAI logs, or other persistent stores visible in the UI. + * + * Known internal IDs: `'prompt_suggestion'`, `'forked_query'`. + */ +export function isInternalPromptId(promptId: string): boolean { + return INTERNAL_PROMPT_IDS.has(promptId); +} From f502f3a3707785f5b4fd4543a55627847512b022 Mon Sep 17 00:00:00 2001 From: wenshao Date: Sat, 4 Apr 2026 13:45:47 +0800 Subject: [PATCH 04/13] =?UTF-8?q?fix:=20address=20Copilot=20review=20?= =?UTF-8?q?=E2=80=94=20docs,=20stream=20optimization,=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Update forkedQuery.ts module docs to reflect that runForkedQuery overrides tools: [] at the per-request level while createForkedChat retains the full generationConfig for speculation callers. 2. Propagate isInternal into loggingStreamWrapper to skip response collection and consolidation for internal prompts, avoiding unnecessary CPU/memory overhead. 3. Add logApiResponse chatRecordingService filter tests: verify prompt_suggestion/forked_query skip recording while normal IDs still record. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../loggingContentGenerator.ts | 19 ++++++-- packages/core/src/followup/forkedQuery.ts | 8 +++- packages/core/src/telemetry/loggers.test.ts | 48 +++++++++++++++++++ 3 files changed, 68 insertions(+), 7 deletions(-) diff --git a/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts b/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts index 7126a960303..12d5e6a5f59 100644 --- a/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts +++ b/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts @@ -224,12 +224,17 @@ export class LoggingContentGenerator implements ContentGenerator { model: string, openaiRequest?: OpenAI.Chat.ChatCompletionCreateParams, ): AsyncGenerator { + const isInternal = isInternalPromptId(userPromptId); + // For internal prompts we only need the last usage metadata (for /stats); + // skip collecting full responses to avoid unnecessary memory overhead. const responses: GenerateContentResponse[] = []; let lastUsageMetadata: GenerateContentResponseUsageMetadata | undefined; try { for await (const response of stream) { - responses.push(response); + if (!isInternal) { + responses.push(response); + } if (response.usageMetadata) { lastUsageMetadata = response.usageMetadata; } @@ -244,9 +249,11 @@ export class LoggingContentGenerator implements ContentGenerator { userPromptId, lastUsageMetadata, ); - const consolidatedResponse = - this.consolidateGeminiResponsesForLogging(responses); - await this.logOpenAIInteraction(openaiRequest, consolidatedResponse); + if (!isInternal) { + const consolidatedResponse = + this.consolidateGeminiResponsesForLogging(responses); + await this.logOpenAIInteraction(openaiRequest, consolidatedResponse); + } } catch (error) { const durationMs = Date.now() - startTime; this._logApiError( @@ -256,7 +263,9 @@ export class LoggingContentGenerator implements ContentGenerator { responses[0]?.modelVersion || model, userPromptId, ); - await this.logOpenAIInteraction(openaiRequest, undefined, error); + if (!isInternal) { + await this.logOpenAIInteraction(openaiRequest, undefined, error); + } throw error; } } diff --git a/packages/core/src/followup/forkedQuery.ts b/packages/core/src/followup/forkedQuery.ts index 80310339d28..44f5b00b015 100644 --- a/packages/core/src/followup/forkedQuery.ts +++ b/packages/core/src/followup/forkedQuery.ts @@ -6,11 +6,15 @@ * Forked Query Infrastructure * * Enables cache-aware secondary LLM calls that share the main conversation's - * prompt prefix (systemInstruction + tools + history) for cache hits. + * prompt prefix (systemInstruction + history) for cache hits. * * DashScope already enables cache_control via X-DashScope-CacheControl header. * By constructing the forked GeminiChat with identical generationConfig and * history prefix, the fork automatically benefits from prefix caching. + * + * Note: `runForkedQuery` overrides `tools: []` at the per-request level so the + * model cannot produce function calls. `createForkedChat` retains the full + * generationConfig (including tools) for callers like speculation that need them. */ import type { @@ -168,7 +172,7 @@ function extractUsage( /** * Run a forked query using a GeminiChat that shares the main conversation's - * cache prefix. This is a single-turn request (no tool execution loop). + * cache prefix. This is a single-turn, tool-free request (no function calls). * * @param config - App config * @param userMessage - The user message to send (e.g., SUGGESTION_PROMPT) diff --git a/packages/core/src/telemetry/loggers.test.ts b/packages/core/src/telemetry/loggers.test.ts index 288e02f0395..49efed93638 100644 --- a/packages/core/src/telemetry/loggers.test.ts +++ b/packages/core/src/telemetry/loggers.test.ts @@ -359,6 +359,54 @@ describe('loggers', () => { }); }); + describe('logApiResponse skips chatRecordingService for internal prompt IDs', () => { + it.each(['prompt_suggestion', 'forked_query'])( + 'should not record to chatRecordingService when prompt_id is %s', + (promptId) => { + const mockRecordUiTelemetryEvent = vi.fn(); + const configWithRecording = { + getSessionId: () => 'test-session-id', + getUsageStatisticsEnabled: () => false, + getChatRecordingService: () => ({ + recordUiTelemetryEvent: mockRecordUiTelemetryEvent, + }), + } as unknown as Config; + + const event = new ApiResponseEvent( + 'resp-id', + 'test-model', + 50, + promptId, + ); + logApiResponse(configWithRecording, event); + + expect(mockRecordUiTelemetryEvent).not.toHaveBeenCalled(); + expect(mockUiEvent.addEvent).toHaveBeenCalled(); + }, + ); + + it('should record to chatRecordingService for normal prompt IDs', () => { + const mockRecordUiTelemetryEvent = vi.fn(); + const configWithRecording = { + getSessionId: () => 'test-session-id', + getUsageStatisticsEnabled: () => false, + getChatRecordingService: () => ({ + recordUiTelemetryEvent: mockRecordUiTelemetryEvent, + }), + } as unknown as Config; + + const event = new ApiResponseEvent( + 'resp-id', + 'test-model', + 50, + 'user_query', + ); + logApiResponse(configWithRecording, event); + + expect(mockRecordUiTelemetryEvent).toHaveBeenCalled(); + }); + }); + describe('logApiRequest', () => { const mockConfig = { getSessionId: () => 'test-session-id', From 864dc2aa018f2d0238b640ab1fa80618367cd5ac Mon Sep 17 00:00:00 2001 From: wenshao Date: Sat, 4 Apr 2026 13:53:03 +0800 Subject: [PATCH 05/13] fix: deep-freeze NO_TOOLS, add internal prompt guard tests Address Copilot review round 3: 1. Deep-freeze NO_TOOLS.tools array to prevent shared mutable state across forked query calls. 2. Add LoggingContentGenerator tests verifying that internal prompt IDs (prompt_suggestion, forked_query) skip logApiRequest and OpenAI interaction logging while preserving logApiResponse. 3. Add logApiError chatRecordingService filter tests matching the existing logApiResponse coverage. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../loggingContentGenerator.test.ts | 41 ++++++++++++++++ packages/core/src/followup/forkedQuery.ts | 4 +- packages/core/src/telemetry/loggers.test.ts | 49 +++++++++++++++++++ 3 files changed, 93 insertions(+), 1 deletion(-) diff --git a/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts b/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts index 8c3486a42f0..805c34faf2e 100644 --- a/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts +++ b/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts @@ -479,4 +479,45 @@ describe('LoggingContentGenerator', () => { }, ]); }); + + it.each(['prompt_suggestion', 'forked_query'])( + 'skips logApiRequest and OpenAI logging for internal promptId %s (generateContent)', + async (promptId) => { + const mockResponse = { + responseId: 'internal-resp', + modelVersion: 'test-model', + candidates: [{ content: { parts: [{ text: 'suggestion' }] } }], + usageMetadata: { promptTokenCount: 10, candidatesTokenCount: 5 }, + } as unknown as GenerateContentResponse; + + const mockWrapped = { + generateContent: vi.fn().mockResolvedValue(mockResponse), + generateContentStream: vi.fn(), + } as unknown as ContentGenerator; + + const gen = new LoggingContentGenerator(mockWrapped, createConfig(), { + enableOpenAILogging: true, + openAILoggingDir: '/tmp/test-logs', + }); + + const request = { + model: 'test-model', + contents: [{ role: 'user', parts: [{ text: 'test' }] }], + } as unknown as GenerateContentParameters; + + await gen.generateContent(request, promptId); + + // logApiRequest should NOT be called for internal prompts + expect(logApiRequest).not.toHaveBeenCalled(); + // logApiResponse SHOULD be called (for /stats token tracking) + expect(logApiResponse).toHaveBeenCalled(); + // OpenAI logger should NOT be called + const loggerInstance = ( + OpenAILogger as unknown as ReturnType + ).mock.results[0]?.value; + if (loggerInstance) { + expect(loggerInstance.logInteraction).not.toHaveBeenCalled(); + } + }, + ); }); diff --git a/packages/core/src/followup/forkedQuery.ts b/packages/core/src/followup/forkedQuery.ts index 44f5b00b015..c2d5ba99e1f 100644 --- a/packages/core/src/followup/forkedQuery.ts +++ b/packages/core/src/followup/forkedQuery.ts @@ -26,7 +26,9 @@ import { GeminiChat, StreamEventType } from '../core/geminiChat.js'; import type { Config } from '../config/config.js'; /** Per-request config that strips tools so the model never produces function calls. */ -const NO_TOOLS: GenerateContentConfig = Object.freeze({ tools: [] }); +const NO_TOOLS: GenerateContentConfig = Object.freeze({ + tools: Object.freeze([]), +}); /** * Snapshot of the main conversation's cache-critical parameters. diff --git a/packages/core/src/telemetry/loggers.test.ts b/packages/core/src/telemetry/loggers.test.ts index 49efed93638..706ab0d69b1 100644 --- a/packages/core/src/telemetry/loggers.test.ts +++ b/packages/core/src/telemetry/loggers.test.ts @@ -55,6 +55,7 @@ import { logExtensionInstallEvent, logExtensionUninstall, logHookCall, + logApiError, } from './loggers.js'; import * as metrics from './metrics.js'; import { QwenLogger } from './qwen-logger/qwen-logger.js'; @@ -77,6 +78,7 @@ import { ExtensionInstallEvent, ExtensionUninstallEvent, HookCallEvent, + ApiErrorEvent, } from './types.js'; import { FileOperation } from './metrics.js'; import type { @@ -407,6 +409,53 @@ describe('loggers', () => { }); }); + describe('logApiError skips chatRecordingService for internal prompt IDs', () => { + it.each(['prompt_suggestion', 'forked_query'])( + 'should not record to chatRecordingService when prompt_id is %s', + (promptId) => { + const mockRecordUiTelemetryEvent = vi.fn(); + const configWithRecording = { + getSessionId: () => 'test-session-id', + getUsageStatisticsEnabled: () => false, + getChatRecordingService: () => ({ + recordUiTelemetryEvent: mockRecordUiTelemetryEvent, + }), + } as unknown as Config; + + const event = new ApiErrorEvent({ + model: 'test-model', + durationMs: 100, + promptId, + errorMessage: 'test error', + }); + logApiError(configWithRecording, event); + + expect(mockRecordUiTelemetryEvent).not.toHaveBeenCalled(); + }, + ); + + it('should record to chatRecordingService for normal prompt IDs', () => { + const mockRecordUiTelemetryEvent = vi.fn(); + const configWithRecording = { + getSessionId: () => 'test-session-id', + getUsageStatisticsEnabled: () => false, + getChatRecordingService: () => ({ + recordUiTelemetryEvent: mockRecordUiTelemetryEvent, + }), + } as unknown as Config; + + const event = new ApiErrorEvent({ + model: 'test-model', + durationMs: 100, + promptId: 'user_query', + errorMessage: 'test error', + }); + logApiError(configWithRecording, event); + + expect(mockRecordUiTelemetryEvent).toHaveBeenCalled(); + }); + }); + describe('logApiRequest', () => { const mockConfig = { getSessionId: () => 'test-session-id', From 59656d9f619b9556685f0fcb9901552b27dc8027 Mon Sep 17 00:00:00 2001 From: wenshao Date: Sat, 4 Apr 2026 13:58:49 +0800 Subject: [PATCH 06/13] docs: reconcile createForkedChat JSDoc with module header Clarify that createForkedChat retains the full generationConfig (including tools) for speculation callers, while runForkedQuery strips tools at the per-request level via NO_TOOLS. Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/core/src/followup/forkedQuery.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/core/src/followup/forkedQuery.ts b/packages/core/src/followup/forkedQuery.ts index c2d5ba99e1f..56645b85b46 100644 --- a/packages/core/src/followup/forkedQuery.ts +++ b/packages/core/src/followup/forkedQuery.ts @@ -120,9 +120,13 @@ export function clearCacheSafeParams(): void { // --------------------------------------------------------------------------- /** - * Create an isolated GeminiChat that shares the same cache prefix as the main - * conversation. The fork uses identical generationConfig (systemInstruction + - * tools) and history, so DashScope's cache_control mechanism produces cache hits. + * Create an isolated GeminiChat that shares the main conversation's + * generationConfig (including systemInstruction, tools, and history). + * + * The full config is retained so that callers like `runSpeculativeLoop` + * can execute tool calls during speculation. For pure-text callers like + * `runForkedQuery`, tools are stripped at the per-request level via + * `NO_TOOLS` — see {@link runForkedQuery}. * * The fork does NOT have chatRecordingService or telemetryService to avoid * polluting the main session's recordings and token counts. From a248ac44effddb45b7092bf3e73b5982cd5a96cc Mon Sep 17 00:00:00 2001 From: wenshao Date: Sat, 4 Apr 2026 14:32:12 +0800 Subject: [PATCH 07/13] fix: build errors and Copilot round 4 feedback 1. Fix NO_TOOLS type: Object.freeze produces readonly array incompatible with ToolUnion[]. Use Readonly> instead; spread in requestConfig already creates a fresh mutable copy per call. 2. Fix test missing required 'model' field in ContentGeneratorConfig. 3. Track firstResponseId/firstModelVersion in loggingStreamWrapper so _logApiResponse/_logApiError have accurate values even when full response collection is skipped for internal prompts. 4. Strengthen OpenAI logger test assertion: assert OpenAILogger was constructed (not guarded by if), then assert logInteraction was not called. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../loggingContentGenerator.test.ts | 8 ++++---- .../loggingContentGenerator.ts | 18 ++++++++++++++---- packages/core/src/followup/forkedQuery.ts | 4 +--- 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts b/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts index 805c34faf2e..a2d1f5f1240 100644 --- a/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts +++ b/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts @@ -496,6 +496,7 @@ describe('LoggingContentGenerator', () => { } as unknown as ContentGenerator; const gen = new LoggingContentGenerator(mockWrapped, createConfig(), { + model: 'test-model', enableOpenAILogging: true, openAILoggingDir: '/tmp/test-logs', }); @@ -511,13 +512,12 @@ describe('LoggingContentGenerator', () => { expect(logApiRequest).not.toHaveBeenCalled(); // logApiResponse SHOULD be called (for /stats token tracking) expect(logApiResponse).toHaveBeenCalled(); - // OpenAI logger should NOT be called + // OpenAI logger should be constructed, but no interaction should be logged + expect(OpenAILogger).toHaveBeenCalled(); const loggerInstance = ( OpenAILogger as unknown as ReturnType ).mock.results[0]?.value; - if (loggerInstance) { - expect(loggerInstance.logInteraction).not.toHaveBeenCalled(); - } + expect(loggerInstance.logInteraction).not.toHaveBeenCalled(); }, ); }); diff --git a/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts b/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts index 12d5e6a5f59..4464f5a7e2d 100644 --- a/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts +++ b/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts @@ -229,9 +229,19 @@ export class LoggingContentGenerator implements ContentGenerator { // skip collecting full responses to avoid unnecessary memory overhead. const responses: GenerateContentResponse[] = []; + // Track first-seen IDs so _logApiResponse/_logApiError have accurate + // values even when we skip collecting full responses for internal prompts. + let firstResponseId = ''; + let firstModelVersion = ''; let lastUsageMetadata: GenerateContentResponseUsageMetadata | undefined; try { for await (const response of stream) { + if (!firstResponseId && response.responseId) { + firstResponseId = response.responseId; + } + if (!firstModelVersion && response.modelVersion) { + firstModelVersion = response.modelVersion; + } if (!isInternal) { responses.push(response); } @@ -243,9 +253,9 @@ export class LoggingContentGenerator implements ContentGenerator { // Only log successful API response if no error occurred const durationMs = Date.now() - startTime; this._logApiResponse( - responses[0]?.responseId ?? '', + firstResponseId, durationMs, - responses[0]?.modelVersion || model, + firstModelVersion || model, userPromptId, lastUsageMetadata, ); @@ -257,10 +267,10 @@ export class LoggingContentGenerator implements ContentGenerator { } catch (error) { const durationMs = Date.now() - startTime; this._logApiError( - responses[0]?.responseId ?? '', + firstResponseId, durationMs, error, - responses[0]?.modelVersion || model, + firstModelVersion || model, userPromptId, ); if (!isInternal) { diff --git a/packages/core/src/followup/forkedQuery.ts b/packages/core/src/followup/forkedQuery.ts index 56645b85b46..13b7daeeb75 100644 --- a/packages/core/src/followup/forkedQuery.ts +++ b/packages/core/src/followup/forkedQuery.ts @@ -26,9 +26,7 @@ import { GeminiChat, StreamEventType } from '../core/geminiChat.js'; import type { Config } from '../config/config.js'; /** Per-request config that strips tools so the model never produces function calls. */ -const NO_TOOLS: GenerateContentConfig = Object.freeze({ - tools: Object.freeze([]), -}); +const NO_TOOLS: Readonly> = { tools: [] }; /** * Snapshot of the main conversation's cache-critical parameters. From 28b1998d0e75873bc65f66d6580d5dfc4abc2b1e Mon Sep 17 00:00:00 2001 From: wenshao Date: Sat, 4 Apr 2026 14:38:09 +0800 Subject: [PATCH 08/13] fix: remove dead Object.keys check, add streaming internal prompt test 1. Simplify runForkedQuery: requestConfig always has tools:[] from NO_TOOLS spread, so the Object.keys().length > 0 ternary is dead code. Pass requestConfig directly. 2. Add generateContentStream test for internal prompt IDs to match the existing generateContent coverage, ensuring the streaming wrapper also skips logApiRequest and OpenAI interaction logging. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../loggingContentGenerator.test.ts | 46 +++++++++++++++++++ packages/core/src/followup/forkedQuery.ts | 2 +- 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts b/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts index a2d1f5f1240..277a14ca90d 100644 --- a/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts +++ b/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts @@ -520,4 +520,50 @@ describe('LoggingContentGenerator', () => { expect(loggerInstance.logInteraction).not.toHaveBeenCalled(); }, ); + + it.each(['prompt_suggestion', 'forked_query'])( + 'skips logApiRequest and OpenAI logging for internal promptId %s (generateContentStream)', + async (promptId) => { + const mockChunk = { + responseId: 'stream-resp', + modelVersion: 'test-model', + candidates: [{ content: { parts: [{ text: 'suggestion' }] } }], + usageMetadata: { promptTokenCount: 10, candidatesTokenCount: 5 }, + } as unknown as GenerateContentResponse; + + async function* fakeStream() { + yield mockChunk; + } + + const mockWrapped = { + generateContent: vi.fn(), + generateContentStream: vi.fn().mockResolvedValue(fakeStream()), + } as unknown as ContentGenerator; + + const gen = new LoggingContentGenerator(mockWrapped, createConfig(), { + model: 'test-model', + enableOpenAILogging: true, + openAILoggingDir: '/tmp/test-logs', + }); + + const request = { + model: 'test-model', + contents: [{ role: 'user', parts: [{ text: 'test' }] }], + } as unknown as GenerateContentParameters; + + const stream = await gen.generateContentStream(request, promptId); + // Consume the stream + for await (const _chunk of stream) { + // drain + } + + expect(logApiRequest).not.toHaveBeenCalled(); + expect(logApiResponse).toHaveBeenCalled(); + expect(OpenAILogger).toHaveBeenCalled(); + const loggerInstance = ( + OpenAILogger as unknown as ReturnType + ).mock.results[0]?.value; + expect(loggerInstance.logInteraction).not.toHaveBeenCalled(); + }, + ); }); diff --git a/packages/core/src/followup/forkedQuery.ts b/packages/core/src/followup/forkedQuery.ts index 13b7daeeb75..93313987f8e 100644 --- a/packages/core/src/followup/forkedQuery.ts +++ b/packages/core/src/followup/forkedQuery.ts @@ -218,7 +218,7 @@ export async function runForkedQuery( model, { message: [{ text: userMessage }], - config: Object.keys(requestConfig).length > 0 ? requestConfig : undefined, + config: requestConfig, }, 'forked_query', ); From 14c08b5145561a8ab5b7f4b4efb8b059290722a6 Mon Sep 17 00:00:00 2001 From: wenshao Date: Sun, 5 Apr 2026 20:06:01 +0800 Subject: [PATCH 09/13] fix: prevent Enter accept from re-inserting suggestion into buffer When accepting a followup suggestion via Enter, accept() queued buffer.insert(suggestion) in a microtask that executed after handleSubmitAndClear had already cleared the buffer, leaving the suggestion text stuck in the input. Add skipOnAccept option to accept() so the Enter path bypasses the onAccept callback. Also add runForkedQuery unit tests verifying tools: [] is passed in per-request config. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/ui/components/InputPrompt.test.tsx | 4 + .../cli/src/ui/components/InputPrompt.tsx | 6 +- .../src/ui/hooks/useFollowupSuggestions.tsx | 5 +- .../core/src/followup/followupState.test.ts | 31 +++ packages/core/src/followup/followupState.ts | 14 +- .../core/src/followup/forkedQuery.test.ts | 200 +++++++++++++++++- .../webui/src/components/layout/InputForm.tsx | 12 +- .../webui/src/hooks/useFollowupSuggestions.ts | 5 +- 8 files changed, 266 insertions(+), 11 deletions(-) diff --git a/packages/cli/src/ui/components/InputPrompt.test.tsx b/packages/cli/src/ui/components/InputPrompt.test.tsx index cd33395f858..039ff77e975 100644 --- a/packages/cli/src/ui/components/InputPrompt.test.tsx +++ b/packages/cli/src/ui/components/InputPrompt.test.tsx @@ -235,6 +235,10 @@ describe('InputPrompt', () => { await wait(); expect(props.onSubmit).toHaveBeenCalledWith('commit this'); + // Enter path must NOT call buffer.insert — it passes text directly to + // handleSubmitAndClear. Calling insert would re-fill the buffer after + // it was already cleared (the microtask race bug). + expect(mockBuffer.insert).not.toHaveBeenCalled(); unmount(); }); diff --git a/packages/cli/src/ui/components/InputPrompt.tsx b/packages/cli/src/ui/components/InputPrompt.tsx index 56448f85bf3..ec84f9e4432 100644 --- a/packages/cli/src/ui/components/InputPrompt.tsx +++ b/packages/cli/src/ui/components/InputPrompt.tsx @@ -892,7 +892,11 @@ export const InputPrompt: React.FC = ({ followup.state.suggestion ) { const text = followup.state.suggestion; - followup.accept('enter'); + // Skip onAccept (buffer.insert) — we pass the text directly to + // handleSubmitAndClear which clears the buffer synchronously. + // Without skipOnAccept the microtask in accept() would re-insert + // the suggestion into the buffer after it was already cleared. + followup.accept('enter', { skipOnAccept: true }); handleSubmitAndClear(text); return true; } diff --git a/packages/cli/src/ui/hooks/useFollowupSuggestions.tsx b/packages/cli/src/ui/hooks/useFollowupSuggestions.tsx index 81773f0fb89..4ff8e4d6863 100644 --- a/packages/cli/src/ui/hooks/useFollowupSuggestions.tsx +++ b/packages/cli/src/ui/hooks/useFollowupSuggestions.tsx @@ -43,7 +43,10 @@ export interface UseFollowupSuggestionsReturn { /** Set suggestion text (called by parent component) */ setSuggestion: (text: string | null) => void; /** Accept the current suggestion */ - accept: (method?: 'tab' | 'enter' | 'right') => void; + accept: ( + method?: 'tab' | 'enter' | 'right', + options?: { skipOnAccept?: boolean }, + ) => void; /** Dismiss the current suggestion */ dismiss: () => void; /** Clear all state */ diff --git a/packages/core/src/followup/followupState.test.ts b/packages/core/src/followup/followupState.test.ts index 325c967b0fc..331a28db5ea 100644 --- a/packages/core/src/followup/followupState.test.ts +++ b/packages/core/src/followup/followupState.test.ts @@ -294,6 +294,37 @@ describe('createFollowupController', () => { ctrl.cleanup(); }); + it('accept with skipOnAccept skips onAccept callback but still clears state and fires telemetry', async () => { + const onStateChange = vi.fn(); + const onAccept = vi.fn(); + const onOutcome = vi.fn(); + const ctrl = createFollowupController({ + onStateChange, + getOnAccept: () => onAccept, + onOutcome, + }); + + ctrl.setSuggestion('run tests'); + vi.advanceTimersByTime(300); + onStateChange.mockClear(); + + ctrl.accept('enter', { skipOnAccept: true }); + + // State should be cleared + expect(onStateChange).toHaveBeenCalledWith(INITIAL_FOLLOWUP_STATE); + + // Telemetry should still fire + expect(onOutcome).toHaveBeenCalledWith( + expect.objectContaining({ outcome: 'accepted', accept_method: 'enter' }), + ); + + // Flush microtask — onAccept should NOT be called + await Promise.resolve(); + expect(onAccept).not.toHaveBeenCalled(); + + ctrl.cleanup(); + }); + it('setSuggestion replaces a pending suggestion', () => { const onStateChange = vi.fn(); const ctrl = createFollowupController({ onStateChange }); diff --git a/packages/core/src/followup/followupState.ts b/packages/core/src/followup/followupState.ts index 8430e206f81..60e8f717a0a 100644 --- a/packages/core/src/followup/followupState.ts +++ b/packages/core/src/followup/followupState.ts @@ -72,7 +72,10 @@ export interface FollowupControllerActions { /** Set suggestion text (with delayed show). Null clears immediately. */ setSuggestion: (text: string | null) => void; /** Accept the current suggestion and invoke onAccept callback */ - accept: (method?: 'tab' | 'enter' | 'right') => void; + accept: ( + method?: 'tab' | 'enter' | 'right', + options?: { skipOnAccept?: boolean }, + ) => void; /** Dismiss/clear suggestion */ dismiss: () => void; /** Hard-clear all state and timers */ @@ -135,7 +138,10 @@ export function createFollowupController( }, SUGGESTION_DELAY_MS); }; - const accept = (method?: 'tab' | 'enter' | 'right'): void => { + const accept = ( + method?: 'tab' | 'enter' | 'right', + options?: { skipOnAccept?: boolean }, + ): void => { if (accepting) { return; } @@ -170,7 +176,9 @@ export function createFollowupController( queueMicrotask(() => { try { - getOnAccept?.()?.(text); + if (!options?.skipOnAccept) { + getOnAccept?.()?.(text); + } } catch (error: unknown) { // eslint-disable-next-line no-console console.error('[followup] onAccept callback threw:', error); diff --git a/packages/core/src/followup/forkedQuery.test.ts b/packages/core/src/followup/forkedQuery.test.ts index 862d9b9e6f5..a223a308ee8 100644 --- a/packages/core/src/followup/forkedQuery.test.ts +++ b/packages/core/src/followup/forkedQuery.test.ts @@ -4,13 +4,24 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { describe, it, expect, beforeEach } from 'vitest'; +import { describe, it, expect, beforeEach, vi } from 'vitest'; import { saveCacheSafeParams, getCacheSafeParams, clearCacheSafeParams, + runForkedQuery, } from './forkedQuery.js'; import type { GenerateContentConfig } from '@google/genai'; +import type { Config } from '../config/config.js'; +import { GeminiChat, StreamEventType } from '../core/geminiChat.js'; + +vi.mock('../core/geminiChat.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + GeminiChat: vi.fn(), + }; +}); describe('CacheSafeParams', () => { beforeEach(() => { @@ -113,3 +124,190 @@ describe('CacheSafeParams', () => { }); }); }); + +describe('runForkedQuery', () => { + beforeEach(() => { + clearCacheSafeParams(); + vi.mocked(GeminiChat).mockReset(); + }); + + it('passes tools: [] in per-request config so the model cannot produce function calls', async () => { + // Save cache params with real tools to simulate a normal conversation + saveCacheSafeParams( + { + systemInstruction: 'You are helpful', + tools: [ + { + functionDeclarations: [ + { name: 'edit', description: 'Edit a file' }, + { name: 'shell', description: 'Run a command' }, + ], + }, + ], + }, + [{ role: 'user', parts: [{ text: 'hello' }] }], + 'test-model', + ); + + // Track what sendMessageStream receives + let capturedParams: unknown = null; + + const mockSendMessageStream = vi.fn( + (_model: string, params: unknown, _promptId: string) => { + capturedParams = params; + async function* generate() { + yield { + type: StreamEventType.CHUNK, + value: { + candidates: [ + { + content: { + role: 'model', + parts: [{ text: 'commit this' }], + }, + }, + ], + usageMetadata: { + promptTokenCount: 10, + candidatesTokenCount: 5, + totalTokenCount: 15, + }, + }, + }; + } + return Promise.resolve(generate()); + }, + ); + + vi.mocked(GeminiChat).mockImplementation( + () => + ({ + sendMessageStream: mockSendMessageStream, + }) as unknown as GeminiChat, + ); + + const mockConfig = {} as unknown as Config; + + const result = await runForkedQuery(mockConfig, 'suggest something'); + + // Verify GeminiChat was constructed with the full generationConfig + // (including tools) — createForkedChat retains tools for speculation callers + expect(GeminiChat).toHaveBeenCalledOnce(); + const ctorArgs = vi.mocked(GeminiChat).mock.calls[0]; + const chatGenerationConfig = ctorArgs[1] as GenerateContentConfig; + expect(chatGenerationConfig.tools).toEqual([ + { + functionDeclarations: [ + { name: 'edit', description: 'Edit a file' }, + { name: 'shell', description: 'Run a command' }, + ], + }, + ]); + // chatRecordingService and telemetryService must be undefined + // to avoid polluting the main session's recordings + expect(ctorArgs[3]).toBeUndefined(); // chatRecordingService + expect(ctorArgs[4]).toBeUndefined(); // telemetryService + + // Verify sendMessageStream was called + expect(mockSendMessageStream).toHaveBeenCalledOnce(); + expect(capturedParams).not.toBeNull(); + + // KEY ASSERTION: per-request config must have tools: [] to prevent + // the model from producing function calls (Root Cause 1 fix) + const sendParams = capturedParams as { config?: { tools?: unknown } }; + expect(sendParams.config).toBeDefined(); + expect(sendParams.config!.tools).toEqual([]); + + // Verify prompt_id is 'forked_query' and message is passed correctly + expect(mockSendMessageStream).toHaveBeenCalledWith( + 'test-model', + expect.objectContaining({ + message: [{ text: 'suggest something' }], + config: expect.objectContaining({ tools: [] }), + }), + 'forked_query', + ); + + // Verify result is correct + expect(result.text).toBe('commit this'); + expect(result.usage.inputTokens).toBe(10); + expect(result.usage.outputTokens).toBe(5); + }); + + it('preserves tools: [] even when jsonSchema is provided', async () => { + saveCacheSafeParams( + { + tools: [{ functionDeclarations: [{ name: 'edit' }] }], + }, + [], + 'test-model', + ); + + let capturedParams: unknown = null; + + const mockSendMessageStream = vi.fn( + (_model: string, params: unknown, _promptId: string) => { + capturedParams = params; + async function* generate() { + yield { + type: StreamEventType.CHUNK, + value: { + candidates: [ + { + content: { + role: 'model', + parts: [{ text: '{"suggestion":"run tests"}' }], + }, + }, + ], + usageMetadata: { + promptTokenCount: 5, + candidatesTokenCount: 3, + }, + }, + }; + } + return Promise.resolve(generate()); + }, + ); + + vi.mocked(GeminiChat).mockImplementation( + () => + ({ + sendMessageStream: mockSendMessageStream, + }) as unknown as GeminiChat, + ); + + const schema = { + type: 'object', + properties: { suggestion: { type: 'string' } }, + }; + + const result = await runForkedQuery({} as Config, 'suggest', { + jsonSchema: schema, + }); + + const sendParams = capturedParams as { + config?: { + tools?: unknown; + responseMimeType?: string; + responseJsonSchema?: unknown; + }; + }; + // tools: [] must still be present alongside JSON schema options + expect(sendParams.config!.tools).toEqual([]); + expect(sendParams.config!.responseMimeType).toBe('application/json'); + expect(sendParams.config!.responseJsonSchema).toBe(schema); + + // Verify JSON was parsed correctly + expect(result.jsonResult).toEqual({ suggestion: 'run tests' }); + }); + + it('throws when CacheSafeParams are not available', async () => { + const mockConfig = {} as unknown as Config; + + await expect(runForkedQuery(mockConfig, 'test')).rejects.toThrow( + 'CacheSafeParams not available', + ); + }); +}); diff --git a/packages/webui/src/components/layout/InputForm.tsx b/packages/webui/src/components/layout/InputForm.tsx index f34436843ce..9a4ef2fdc27 100644 --- a/packages/webui/src/components/layout/InputForm.tsx +++ b/packages/webui/src/components/layout/InputForm.tsx @@ -143,7 +143,10 @@ export interface InputFormProps { /** Prompt suggestion state */ followupState?: InputFormFollowupState; /** Callback to accept prompt suggestion */ - onAcceptFollowup?: (method?: 'tab' | 'enter' | 'right') => void; + onAcceptFollowup?: ( + method?: 'tab' | 'enter' | 'right', + options?: { skipOnAccept?: boolean }, + ) => void; /** Callback to dismiss prompt suggestion */ onDismissFollowup?: () => void; } @@ -278,9 +281,10 @@ export const InputForm: FC = ({ // Accept and submit prompt suggestion on Enter when input is empty if (hasFollowup && !inputText && followupSuggestion) { e.preventDefault(); - onAcceptFollowup?.('enter'); - // Pass suggestion text explicitly — onInputChange is async (React setState) - // so onSubmit cannot rely on reading inputText from the closure. + // Skip onAccept callback — we pass the text directly to onSubmit. + // Without skipOnAccept the microtask in accept() would re-insert + // the suggestion into the input after it was already cleared. + onAcceptFollowup?.('enter', { skipOnAccept: true }); onSubmit(e, followupSuggestion); return; } diff --git a/packages/webui/src/hooks/useFollowupSuggestions.ts b/packages/webui/src/hooks/useFollowupSuggestions.ts index 60fa84ce3ba..1337a479c4b 100644 --- a/packages/webui/src/hooks/useFollowupSuggestions.ts +++ b/packages/webui/src/hooks/useFollowupSuggestions.ts @@ -49,7 +49,10 @@ export interface UseFollowupSuggestionsReturn { /** Set suggestion text (called by parent component) */ setSuggestion: (text: string | null) => void; /** Accept the current suggestion */ - accept: (method?: 'tab' | 'enter' | 'right') => void; + accept: ( + method?: 'tab' | 'enter' | 'right', + options?: { skipOnAccept?: boolean }, + ) => void; /** Dismiss the current suggestion */ dismiss: () => void; /** Clear all state */ From 8293d7a088c7c0b9ce3dcc6573e904cf7cfd13ce Mon Sep 17 00:00:00 2001 From: wenshao Date: Wed, 8 Apr 2026 09:54:26 +0800 Subject: [PATCH 10/13] fix(core): add speculation to internal IDs, fix logToolCall filtering, improve suggestion prompt - Add 'speculation' to INTERNAL_PROMPT_IDS so speculation API traffic and tool calls are hidden from chat recordings and tool call UI - Add isInternalPromptId check to logToolCall() for consistency with logApiError/logApiResponse - Improve SUGGESTION_PROMPT: prioritize assistant's last few lines and extract actionable text from explicit tips (e.g. "Tip: type X") - Fix garbled unicode in prompt text - Update design docs and user docs to reflect changes - Add test coverage for all new behavior --- .../prompt-suggestion-design.md | 26 +++++++++++ docs/users/features/followup-suggestions.md | 2 +- .../core/src/followup/suggestionGenerator.ts | 9 +++- packages/core/src/telemetry/loggers.test.ts | 44 ++++++++++++++++++- packages/core/src/telemetry/loggers.ts | 4 +- .../core/src/utils/internalPromptIds.test.ts | 4 ++ packages/core/src/utils/internalPromptIds.ts | 3 +- 7 files changed, 86 insertions(+), 6 deletions(-) diff --git a/docs/design/prompt-suggestion/prompt-suggestion-design.md b/docs/design/prompt-suggestion/prompt-suggestion-design.md index 1636db6cfb7..a8d7ef38124 100644 --- a/docs/design/prompt-suggestion/prompt-suggestion-design.md +++ b/docs/design/prompt-suggestion/prompt-suggestion-design.md @@ -64,10 +64,19 @@ A **prompt suggestion** (Next-step Suggestion / NES) is a short prediction (2-12 ``` [SUGGESTION MODE: Suggest what the user might naturally type next.] +FIRST: Read the LAST FEW LINES of the assistant's most recent message — that's where +next-step hints, tips, and actionable suggestions usually appear. Then check the user's +recent messages and original request. + Your job is to predict what THEY would type - not what you think they should do. THE TEST: Would they think "I was just about to type that"? +PRIORITY: If the assistant's last message contains a tip or hint like "Tip: type X to ..." +or "type X to ...", extract X as the suggestion. These are explicit next-step hints. + EXAMPLES: +Assistant says "Tip: type post comments to publish findings" → "post comments" +Assistant says "type /review to start" → "/review" User asked "fix the bug and run tests", bug is fixed → "run the tests" After code written → "try it out" Task complete, obvious follow-up → "commit this" or "push it" @@ -197,6 +206,23 @@ The Tab handler uses `key.name === 'tab'` explicitly (not `ACCEPT_SUGGESTION` ma | `enableSpeculation` | boolean | false | Predictive execution engine | | `fastModel` (top-level) | string | "" | Model for all background tasks (empty = use main model). Set via `/model --fast` | +### Internal Prompt ID Filtering + +Background operations use dedicated prompt IDs (`INTERNAL_PROMPT_IDS` in `utils/internalPromptIds.ts`) to prevent their API traffic and tool calls from appearing in the user-visible UI: + +| Prompt ID | Used by | +| ------------------- | -------------------------- | +| `prompt_suggestion` | Suggestion generation | +| `forked_query` | Cache-aware forked queries | +| `speculation` | Speculation engine | + +**Filtering applied:** + +- `loggingContentGenerator` — skips `logApiRequest` and OpenAI interaction logging for internal IDs +- `logApiResponse` / `logApiError` — skips `chatRecordingService.recordUiTelemetryEvent` +- `logToolCall` — skips `chatRecordingService.recordUiTelemetryEvent` +- `uiTelemetryService.addEvent` — **not filtered** (ensures `/stats` token tracking works) + ### Thinking Mode Thinking/reasoning is explicitly disabled (`thinkingConfig: { includeThoughts: false }`) for all background task paths: diff --git a/docs/users/features/followup-suggestions.md b/docs/users/features/followup-suggestions.md index 3dbf11df59b..6b72a1d7089 100644 --- a/docs/users/features/followup-suggestions.md +++ b/docs/users/features/followup-suggestions.md @@ -12,7 +12,7 @@ After Qwen Code finishes responding, a suggestion appears as dimmed text in the > run the tests ``` -The suggestion is generated by sending the conversation history to the model, which predicts what you would naturally type next. +The suggestion is generated by sending the conversation history to the model, which predicts what you would naturally type next. If the response contains an explicit tip (e.g., `Tip: type post comments to publish findings`), the suggested action is extracted automatically. ## Accepting Suggestions diff --git a/packages/core/src/followup/suggestionGenerator.ts b/packages/core/src/followup/suggestionGenerator.ts index e5b25b7684f..50f872be747 100644 --- a/packages/core/src/followup/suggestionGenerator.ts +++ b/packages/core/src/followup/suggestionGenerator.ts @@ -24,13 +24,20 @@ import { ApiResponseEvent } from '../telemetry/types.js'; */ export const SUGGESTION_PROMPT = `[SUGGESTION MODE: Suggest what the user might naturally type next.] -FIRST: Look at the user's recent messages and original request. +FIRST: Read the LAST FEW LINES of the assistant's most recent message -- that's where +next-step hints, tips, and actionable suggestions usually appear. Then check the user's +recent messages and original request. Your job is to predict what THEY would type - not what you think they should do. THE TEST: Would they think "I was just about to type that"? +PRIORITY: If the assistant's last message contains a tip or hint like "Tip: type X to ..." +or "type X to ...", extract X as the suggestion. These are explicit next-step hints. + EXAMPLES: +Assistant says "Tip: type post comments to publish findings" → "post comments" +Assistant says "type /review to start" → "/review" User asked "fix the bug and run tests", bug is fixed → "run the tests" After code written → "try it out" Model offers options → suggest the one the user would likely pick, based on conversation diff --git a/packages/core/src/telemetry/loggers.test.ts b/packages/core/src/telemetry/loggers.test.ts index 706ab0d69b1..19a3997dc05 100644 --- a/packages/core/src/telemetry/loggers.test.ts +++ b/packages/core/src/telemetry/loggers.test.ts @@ -362,7 +362,7 @@ describe('loggers', () => { }); describe('logApiResponse skips chatRecordingService for internal prompt IDs', () => { - it.each(['prompt_suggestion', 'forked_query'])( + it.each(['prompt_suggestion', 'forked_query', 'speculation'])( 'should not record to chatRecordingService when prompt_id is %s', (promptId) => { const mockRecordUiTelemetryEvent = vi.fn(); @@ -410,7 +410,7 @@ describe('loggers', () => { }); describe('logApiError skips chatRecordingService for internal prompt IDs', () => { - it.each(['prompt_suggestion', 'forked_query'])( + it.each(['prompt_suggestion', 'forked_query', 'speculation'])( 'should not record to chatRecordingService when prompt_id is %s', (promptId) => { const mockRecordUiTelemetryEvent = vi.fn(); @@ -1107,6 +1107,46 @@ describe('loggers', () => { }, }); }); + + it.each(['prompt_suggestion', 'forked_query', 'speculation'])( + 'should not record to chatRecordingService when prompt_id is %s', + (promptId) => { + const mockRecordUiTelemetryEvent = vi.fn(); + const configWithRecording = { + ...mockConfig, + getChatRecordingService: () => ({ + recordUiTelemetryEvent: mockRecordUiTelemetryEvent, + }), + } as unknown as Config; + + const call: CompletedToolCall = { + status: 'success', + request: { + name: 'test-function', + args: {}, + callId: 'test-call-id', + isClientInitiated: true, + prompt_id: promptId, + }, + response: { + callId: 'test-call-id', + responseParts: [{ text: 'ok' }], + resultDisplay: undefined, + error: undefined, + errorType: undefined, + }, + tool: new EditTool(mockConfig), + invocation: {} as AnyToolInvocation, + durationMs: 50, + outcome: ToolConfirmationOutcome.ProceedOnce, + }; + const event = new ToolCallEvent(call); + logToolCall(configWithRecording, event); + + expect(mockRecordUiTelemetryEvent).not.toHaveBeenCalled(); + expect(mockUiEvent.addEvent).toHaveBeenCalled(); + }, + ); }); describe('logMalformedJsonResponse', () => { diff --git a/packages/core/src/telemetry/loggers.ts b/packages/core/src/telemetry/loggers.ts index acfcd5d63cd..0ced0d3206f 100644 --- a/packages/core/src/telemetry/loggers.ts +++ b/packages/core/src/telemetry/loggers.ts @@ -218,7 +218,9 @@ export function logToolCall(config: Config, event: ToolCallEvent): void { 'event.timestamp': new Date().toISOString(), } as UiEvent; uiTelemetryService.addEvent(uiEvent); - config.getChatRecordingService()?.recordUiTelemetryEvent(uiEvent); + if (!isInternalPromptId(event.prompt_id)) { + config.getChatRecordingService()?.recordUiTelemetryEvent(uiEvent); + } QwenLogger.getInstance(config)?.logToolCallEvent(event); if (!isTelemetrySdkInitialized()) return; diff --git a/packages/core/src/utils/internalPromptIds.test.ts b/packages/core/src/utils/internalPromptIds.test.ts index 357c3805142..fbdfcd6ae8f 100644 --- a/packages/core/src/utils/internalPromptIds.test.ts +++ b/packages/core/src/utils/internalPromptIds.test.ts @@ -16,6 +16,10 @@ describe('isInternalPromptId', () => { expect(isInternalPromptId('forked_query')).toBe(true); }); + it('returns true for speculation', () => { + expect(isInternalPromptId('speculation')).toBe(true); + }); + it('returns false for user_query', () => { expect(isInternalPromptId('user_query')).toBe(false); }); diff --git a/packages/core/src/utils/internalPromptIds.ts b/packages/core/src/utils/internalPromptIds.ts index ea6cce57bc1..8a178d7832e 100644 --- a/packages/core/src/utils/internalPromptIds.ts +++ b/packages/core/src/utils/internalPromptIds.ts @@ -14,6 +14,7 @@ const INTERNAL_PROMPT_IDS: ReadonlySet = new Set([ 'prompt_suggestion', 'forked_query', + 'speculation', ]); /** @@ -21,7 +22,7 @@ const INTERNAL_PROMPT_IDS: ReadonlySet = new Set([ * whose events should not be recorded to the chatRecordingService, * OpenAI logs, or other persistent stores visible in the UI. * - * Known internal IDs: `'prompt_suggestion'`, `'forked_query'`. + * Known internal IDs: `'prompt_suggestion'`, `'forked_query'`, `'speculation'`. */ export function isInternalPromptId(promptId: string): boolean { return INTERNAL_PROMPT_IDS.has(promptId); From 4aa3da9a64e5abcae48f4effcb6988b2a95a30d0 Mon Sep 17 00:00:00 2001 From: wenshao Date: Wed, 8 Apr 2026 10:18:11 +0800 Subject: [PATCH 11/13] fix(core): deep-freeze NO_TOOLS, add speculation to loggingContentGenerator tests - Object.freeze NO_TOOLS and its tools array to prevent runtime mutation - Add 'speculation' to loggingContentGenerator internal prompt ID tests for consistency with loggers.test.ts and internalPromptIds.ts --- .../loggingContentGenerator/loggingContentGenerator.test.ts | 4 ++-- packages/core/src/followup/forkedQuery.ts | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts b/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts index 277a14ca90d..624f04f3b0f 100644 --- a/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts +++ b/packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts @@ -480,7 +480,7 @@ describe('LoggingContentGenerator', () => { ]); }); - it.each(['prompt_suggestion', 'forked_query'])( + it.each(['prompt_suggestion', 'forked_query', 'speculation'])( 'skips logApiRequest and OpenAI logging for internal promptId %s (generateContent)', async (promptId) => { const mockResponse = { @@ -521,7 +521,7 @@ describe('LoggingContentGenerator', () => { }, ); - it.each(['prompt_suggestion', 'forked_query'])( + it.each(['prompt_suggestion', 'forked_query', 'speculation'])( 'skips logApiRequest and OpenAI logging for internal promptId %s (generateContentStream)', async (promptId) => { const mockChunk = { diff --git a/packages/core/src/followup/forkedQuery.ts b/packages/core/src/followup/forkedQuery.ts index 93313987f8e..7ccb8e58881 100644 --- a/packages/core/src/followup/forkedQuery.ts +++ b/packages/core/src/followup/forkedQuery.ts @@ -26,7 +26,9 @@ import { GeminiChat, StreamEventType } from '../core/geminiChat.js'; import type { Config } from '../config/config.js'; /** Per-request config that strips tools so the model never produces function calls. */ -const NO_TOOLS: Readonly> = { tools: [] }; +const NO_TOOLS: Readonly> = Object.freeze({ + tools: Object.freeze([]), +}); /** * Snapshot of the main conversation's cache-critical parameters. From 57732ff2e2a8359730dd11f02e3561b29991c18e Mon Sep 17 00:00:00 2001 From: wenshao Date: Wed, 8 Apr 2026 10:42:53 +0800 Subject: [PATCH 12/13] fix(core): fix NO_TOOLS Object.freeze type error Use `as const` with type assertion to satisfy TypeScript while keeping runtime immutability via Object.freeze. --- packages/core/src/followup/forkedQuery.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/core/src/followup/forkedQuery.ts b/packages/core/src/followup/forkedQuery.ts index 7ccb8e58881..0d4dc2186e5 100644 --- a/packages/core/src/followup/forkedQuery.ts +++ b/packages/core/src/followup/forkedQuery.ts @@ -26,9 +26,10 @@ import { GeminiChat, StreamEventType } from '../core/geminiChat.js'; import type { Config } from '../config/config.js'; /** Per-request config that strips tools so the model never produces function calls. */ -const NO_TOOLS: Readonly> = Object.freeze({ - tools: Object.freeze([]), -}); +const NO_TOOLS = Object.freeze({ tools: [] as const }) as Pick< + GenerateContentConfig, + 'tools' +>; /** * Snapshot of the main conversation's cache-critical parameters. From 5980db32b90be155e47e07e1558405eeda8303e2 Mon Sep 17 00:00:00 2001 From: wenshao Date: Wed, 8 Apr 2026 10:44:29 +0800 Subject: [PATCH 13/13] refactor(core): remove unused isInternalPromptId re-export from loggers.ts All consumers import directly from utils/internalPromptIds.js. The re-export was dead code with no importers. --- packages/core/src/telemetry/loggers.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/core/src/telemetry/loggers.ts b/packages/core/src/telemetry/loggers.ts index 0ced0d3206f..6cd70679998 100644 --- a/packages/core/src/telemetry/loggers.ts +++ b/packages/core/src/telemetry/loggers.ts @@ -122,9 +122,6 @@ function getCommonAttributes(config: Config): LogAttributes { export { getCommonAttributes }; -// Re-export for consumers that import from this module. -export { isInternalPromptId } from '../utils/internalPromptIds.js'; - export function logStartSession( config: Config, event: StartSessionEvent,