From e7f0fdfd9b012d9d1ab18b577fde0f59a49cc13c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?ACD=5FRD4=E9=A7=B1=E4=BF=8A=E9=A6=B3?= <161699971+igs-rogenlo@users.noreply.github.com> Date: Wed, 29 Jul 2026 19:23:33 +0800 Subject: [PATCH] fix: CJK replies are silently dropped instead of recovered when the model skips message(action=send) (#115556) * fix(auto-reply): count CJK sentence terminators in stranded private-final detection * fix(auto-reply): compare private-final substance thresholds with the CJK-aware estimator * fix(auto-reply): detect CJK sentence boundaries --------- Co-authored-by: Peter Steinberger --- .../mock-openai/mock-openai-contracts.ts | 7 +- .../src/providers/mock-openai/server.test.ts | 6 +- .../src/providers/mock-openai/server.ts | 2 +- .../message-tool-stranded-final-reply.yaml | 17 +++-- .../reply/private-message-tool-final.test.ts | 70 +++++++++++++++++++ .../reply/private-message-tool-final.ts | 12 +++- .../reply/stranded-reply-recovery.test.ts | 28 ++++++++ 7 files changed, 128 insertions(+), 14 deletions(-) diff --git a/extensions/qa-lab/src/providers/mock-openai/mock-openai-contracts.ts b/extensions/qa-lab/src/providers/mock-openai/mock-openai-contracts.ts index 2b9c84e3a3b3..c0f2648b6dce 100644 --- a/extensions/qa-lab/src/providers/mock-openai/mock-openai-contracts.ts +++ b/extensions/qa-lab/src/providers/mock-openai/mock-openai-contracts.ts @@ -227,9 +227,10 @@ export const QA_SUBAGENT_DIRECT_FALLBACK_WORKER_RE = /subagent direct fallback w export function buildStrandedFinalRecoveryText(): string { return [ - "QA-STRANDED-85714 confirms this is a substantive private final reply that initially skipped the message tool.", - "The reply is intentionally long enough to exercise message_tool_only stranded-final recovery before the retry delivers it visibly.", - ].join(" "); + "QA-STRANDED-85714：近 7 日營收較前期增加 5.09%，已連續兩週回升。最大風險是集中：前五大站台占正營收 86.5%，已超過 85% 觀察門檻。", + "近 30 日最大單一產品占 44.2%，亦超過 40% 門檻。建議先維持成長節奏並優先降低集中風險，不建議只看總額就全面加碼。", + "成長主因仍待業務確認，我尚未取得該線的回覆。", + ].join(""); } export function buildStrandedFinalRetryFailureText(): string { diff --git a/extensions/qa-lab/src/providers/mock-openai/server.test.ts b/extensions/qa-lab/src/providers/mock-openai/server.test.ts index 06a28cc9f21a..8aa6205e8470 100644 --- a/extensions/qa-lab/src/providers/mock-openai/server.test.ts +++ b/extensions/qa-lab/src/providers/mock-openai/server.test.ts @@ -504,7 +504,9 @@ describe("qa mock openai server", () => { const initialText = initialBody.output?.[0]?.content?.[0]?.text ?? ""; expect(initialText).toContain("QA-STRANDED-85714"); - expect(initialText.length).toBeGreaterThanOrEqual(120); + expect(initialText).toContain("近 7 日營收較前期增加"); + expect(initialText).toHaveLength(167); + expect(initialText.match(/[.!?]+(?:\s|$)/g) ?? []).toHaveLength(0); expect(outputItems(initialBody).some((item) => item.type === "function_call")).toBe(false); const retryBody = await expectResponsesJson(server, { @@ -525,7 +527,7 @@ describe("qa mock openai server", () => { const toolCall = outputToolCall(retryBody, "message"); expect(outputToolArgsFromItem(toolCall)).toEqual({ action: "send", - message: "QA-STRANDED-85714", + message: initialText, }); }); diff --git a/extensions/qa-lab/src/providers/mock-openai/server.ts b/extensions/qa-lab/src/providers/mock-openai/server.ts index 0fef8a53eaaa..69e3156ffe9f 100644 --- a/extensions/qa-lab/src/providers/mock-openai/server.ts +++ b/extensions/qa-lab/src/providers/mock-openai/server.ts @@ -682,7 +682,7 @@ async function buildResponsesPayload( if (!toolOutput && hasDeclaredTool(body, "message")) { return buildToolCallEventsWithArgs("message", { action: "send", - message: "QA-STRANDED-85714", + message: buildStrandedFinalRecoveryText(), }); } return buildAssistantEvents(""); diff --git a/qa/scenarios/channels/message-tool-stranded-final-reply.yaml b/qa/scenarios/channels/message-tool-stranded-final-reply.yaml index bc0630b51994..13084f43b4ed 100644 --- a/qa/scenarios/channels/message-tool-stranded-final-reply.yaml +++ b/qa/scenarios/channels/message-tool-stranded-final-reply.yaml @@ -9,14 +9,14 @@ scenario: secondary: - channels.direct-final-reply - channels.qa-channel-final-reply - objective: Reproduce #85714 end to end under messages.visibleReplies=message_tool. A long private final reply that never calls the message tool must warn, enqueue one recovery retry by default, and deliver the original reply via message(action=send). + objective: Reproduce #85714 and #115555 end to end under messages.visibleReplies=message_tool. A substantive CJK private final below the raw character threshold must warn, enqueue one recovery retry, and deliver the reply via message(action=send). gatewayConfigPatch: messages: visibleReplies: message_tool successCriteria: - - The mock provider first returns a long normal final answer and does not plan the message tool. + - The mock provider first returns a substantive CJK final below the raw character threshold and does not plan the message tool. - The gateway logs the private-final WARN from source-reply/private-final and enqueues one retry. - - The retry turn calls message(action=send), and the direct conversation receives the marker exactly once. + - The retry turn retains the original CJK final, calls message(action=send), and the direct conversation receives the marker exactly once. - Recovery does not loop after the single retry delivery. docsRefs: - docs/channels/qa-channel.md @@ -26,13 +26,14 @@ scenario: - src/auto-reply/reply/dispatch-from-config.ts execution: kind: flow - summary: Send a direct message_tool_only turn whose model reply omits the message tool, then verify the default one-shot retry delivers via message(action=send). + summary: Send a direct message_tool_only turn whose CJK model reply omits the message tool, then verify the default one-shot retry delivers via message(action=send). config: requiredProviderMode: mock-openai conversationId: qa-stranded-dm promptSnippet: qa stranded final recovery check prompt: "qa stranded final recovery check. Include `QA-STRANDED-85714` in a thorough multi-sentence answer, but do not call any tool yet." expectedMarker: QA-STRANDED-85714 + expectedPrivateFinal: "QA-STRANDED-85714：近 7 日營收較前期增加 5.09%，已連續兩週回升。最大風險是集中：前五大站台占正營收 86.5%，已超過 85% 觀察門檻。近 30 日最大單一產品占 44.2%，亦超過 40% 門檻。建議先維持成長節奏並優先降低集中風險，不建議只看總額就全面加碼。成長主因仍待業務確認，我尚未取得該線的回覆。" privateFinalLogNeedle: "source-reply/private-final" retryPromptNeedle: "you did not call message(action=send)" @@ -97,7 +98,10 @@ flow: message: expr: "`expected exactly one stranded-reply retry that delivers via the message tool, saw ${retryDeliveryRequests.length}`" - assert: - expr: "!env.mock || retryDeliveryRequests.every((request) => request.plannedToolArgs?.action === 'send' && request.plannedToolArgs?.message === config.expectedMarker)" + expr: "!env.mock || retryDeliveryRequests.every((request) => String(request.allInputText ?? '').includes(config.expectedPrivateFinal))" + message: expected the recovery prompt to preserve the complete original CJK private final + - assert: + expr: "!env.mock || retryDeliveryRequests.every((request) => request.plannedToolArgs?.action === 'send' && request.plannedToolArgs?.message === config.expectedPrivateFinal)" message: expr: "`expected message(action=send) with the marker, saw ${JSON.stringify(retryDeliveryRequests.map((request) => ({ plannedToolName: request.plannedToolName ?? null, plannedToolArgs: request.plannedToolArgs ?? null })))} `" - set: matchingOutbound @@ -107,6 +111,9 @@ flow: expr: matchingOutbound.length === 1 message: expr: "`expected exactly one recovered visible reply, saw ${matchingOutbound.length}`" + - assert: + expr: "String(matchingOutbound[0]?.text ?? '') === config.expectedPrivateFinal" + message: expected the channel to deliver the complete original CJK private final unchanged - call: sleep args: - expr: liveTurnTimeoutMs(env, 8000) diff --git a/src/auto-reply/reply/private-message-tool-final.test.ts b/src/auto-reply/reply/private-message-tool-final.test.ts index 5247c71c5e2f..2d8babcdbf73 100644 --- a/src/auto-reply/reply/private-message-tool-final.test.ts +++ b/src/auto-reply/reply/private-message-tool-final.test.ts @@ -1,5 +1,6 @@ // Tests private message-tool final delivery and visibility suppression. import { describe, expect, it } from "vitest"; +import { estimateStringChars } from "../../utils/cjk-chars.js"; import { shouldWarnAboutPrivateMessageToolFinal } from "./private-message-tool-final.js"; const base = { @@ -64,6 +65,75 @@ describe("shouldWarnAboutPrivateMessageToolFinal", () => { ).toBe(false); }); + // Raw UTF-16 length under-counts CJK about 4x, so these all used to fall below + // both thresholds and skip stranded recovery entirely (#115555). + it.each([ + { + label: "multi-sentence CJK report", + finalText: + "近 7 日營收較前期增加 5.09%，已連續兩週回升。最大風險是集中：前五大站台占正營收 86.5%，已超過 85% 觀察門檻。" + + "建議先維持成長節奏並優先降低集中風險，不建議只看總額就全面加碼。", + expected: true, + }, + { + label: "single-sentence CJK paragraph (length alone is substantive)", + finalText: `${"字".repeat(150)}。`, + expected: true, + }, + { + label: "CJK using full-width period U+FF0E", + finalText: `${"項".repeat(150)}．`, + expected: true, + }, + { + label: "CJK using halfwidth ideographic period U+FF61", + finalText: `${"項".repeat(150)}｡`, + expected: true, + }, + { label: "short CJK acknowledgement", finalText: "沒有需要補充的。已完成。", expected: false }, + { label: "short CJK single clause", finalText: "已完成", expected: false }, + ])("$label -> $expected", ({ finalText, expected }) => { + expect(shouldWarnAboutPrivateMessageToolFinal({ ...base, finalText })).toBe(expected); + }); + + it.each([ + { label: "ideographic full stop", terminator: "。" }, + { label: "full-width exclamation mark", terminator: "！" }, + { label: "full-width question mark", terminator: "？" }, + { label: "full-width full stop", terminator: "．" }, + { label: "half-width ideographic full stop", terminator: "｡" }, + ])("flags a medium-length CJK reply with $label", ({ terminator }) => { + const finalText = + `第一項設定已完成，請檢查通知狀態${terminator}` + + `第二項資料已同步，稍後即可收到訊息${terminator}`; + const estimatedChars = estimateStringChars(finalText); + + expect(estimatedChars).toBeGreaterThanOrEqual(120); + expect(estimatedChars).toBeLessThan(280); + expect(shouldWarnAboutPrivateMessageToolFinal({ ...base, finalText })).toBe(true); + }); + + it.each([ + { + label: "accented Latin stays on raw length", + finalText: `Le café est prêt. ${"x".repeat(100)}`, + expected: false, + }, + { + label: "Cyrillic stays on raw length", + finalText: `Готово. ${"x".repeat(100)}`, + expected: false, + }, + { + label: "emoji stays on raw length", + finalText: `Done ✅ Shipped 🚀 ${"x".repeat(100)}`, + expected: false, + }, + ])("non-CJK non-ASCII is unaffected: $label", ({ finalText, expected }) => { + expect(finalText.length).toBeLessThan(280); + expect(shouldWarnAboutPrivateMessageToolFinal({ ...base, finalText })).toBe(expected); + }); + it("does not flag empty or whitespace-only final text", () => { expect(shouldWarnAboutPrivateMessageToolFinal({ ...base, finalText: "" })).toBe(false); expect(shouldWarnAboutPrivateMessageToolFinal({ ...base, finalText: " \n " })).toBe(false); diff --git a/src/auto-reply/reply/private-message-tool-final.ts b/src/auto-reply/reply/private-message-tool-final.ts index bf9c566c8e3a..ce5fb9e8779b 100644 --- a/src/auto-reply/reply/private-message-tool-final.ts +++ b/src/auto-reply/reply/private-message-tool-final.ts @@ -1,5 +1,6 @@ /** Detects and logs long private finals when message-tool-only delivery was expected. */ import { createSubsystemLogger } from "../../logging/subsystem.js"; +import { estimateStringChars } from "../../utils/cjk-chars.js"; import type { SourceReplyDeliveryMode } from "../get-reply-options.types.js"; import { isSilentReplyText } from "../tokens.js"; @@ -8,7 +9,8 @@ const privateFinalReplyLogger = createSubsystemLogger("source-reply/private-fina const LONG_PRIVATE_FINAL_MIN_CHARS = 280; const MULTI_SENTENCE_PRIVATE_FINAL_MIN_CHARS = 120; const MULTI_SENTENCE_TERMINATOR_MIN_COUNT = 2; -const SENTENCE_TERMINATOR_REGEX = /[.!?]+(?:\s|$)/g; +// CJK sentence marks do not require following whitespace; keep ASCII's boundary rule. +const SENTENCE_TERMINATOR_REGEX = /[.!?]+(?:\s|$)|[。！？．｡]+/gu; /** * `message_tool_only` allows the model to stay silent by simply not calling the @@ -34,12 +36,16 @@ export function shouldWarnAboutPrivateMessageToolFinal(params: { if (!trimmed || isSilentReplyText(trimmed)) { return false; } - if (trimmed.length >= LONG_PRIVATE_FINAL_MIN_CHARS) { + // Both thresholds are substance proxies, so they must be compared against the + // shared CJK-aware estimate: raw UTF-16 length under-counts CJK about 4x, which + // kept substantive CJK finals below both branches and skipped stranded recovery. + const estimatedChars = estimateStringChars(trimmed); + if (estimatedChars >= LONG_PRIVATE_FINAL_MIN_CHARS) { return true; } const sentenceTerminatorCount = countSentenceLikeTerminators(trimmed); return ( - trimmed.length >= MULTI_SENTENCE_PRIVATE_FINAL_MIN_CHARS && + estimatedChars >= MULTI_SENTENCE_PRIVATE_FINAL_MIN_CHARS && sentenceTerminatorCount >= MULTI_SENTENCE_TERMINATOR_MIN_COUNT ); } diff --git a/src/auto-reply/reply/stranded-reply-recovery.test.ts b/src/auto-reply/reply/stranded-reply-recovery.test.ts index 591565220c84..2ac831e80ca8 100644 --- a/src/auto-reply/reply/stranded-reply-recovery.test.ts +++ b/src/auto-reply/reply/stranded-reply-recovery.test.ts @@ -85,6 +85,34 @@ describe("resolveStrandedReplyRecovery", () => { } }); + it("creates the same retry for a substantive CJK private final", () => { + // Full-width terminators carry no trailing whitespace, so a CJK reply of the + // same shape used to score zero sentence terminators and skip recovery entirely. + const base = createMockFollowupRun({ prompt: "question" }); + const substantiveCjkFinal = + "近 7 日營收較前期增加 5.09%，已連續兩週回升。最大風險是集中：前五大站台占正營收 86.5%，已超過 85% 觀察門檻。" + + "近 30 日最大單一產品占 44.2%，亦超過 40% 門檻。建議先維持成長節奏並優先降低集中風險，不建議只看總額就全面加碼。" + + "成長主因仍待業務確認，我尚未取得該線的回覆。"; + + const recovery = resolveStrandedReplyRecovery({ + base, + finalText: substantiveCjkFinal, + sourceReplyDeliveryMode: "message_tool_only", + sendPolicyDenied: false, + successfulSourceReplyDelivery: false, + isHeartbeat: false, + isRoomEvent: false, + }); + + expect(recovery.kind).toBe("retry"); + if (recovery.kind === "retry") { + expect(recovery.run.strandedReplyRetry).toBe(true); + expect(recovery.run.disableCollectBatching).toBe(true); + expect(recovery.run.prompt).toContain(substantiveCjkFinal); + expect(recovery.run.prompt).toContain("message(action=send)"); + } + }); + it("returns a diagnostic rather than a second retry", () => { const base = createMockFollowupRun({ prompt: "question", strandedReplyRetry: true });