mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-07-10 01:29:17 +00:00
fix(core,cli): address PR #4168 review batch 5
- R5.1: tighten /context tier comment + TODO. The rawOverhead-based fix doesn't cover `--continue` restores with many history messages (since rawOverhead excludes messagesTokens). UI may still show 'safe' for one render until the first send. Documented inline and added a TODO to plumb chat history into collectContextData for same-source-of-truth as the cheap-gate. - R5.2a: add TODO(finish_reason) at the truncation guard. The `>= cap` heuristic false-positives on legitimate at-cap summaries; the proper signal is finish_reason which runSideQuery doesn't surface today. - R5.2b: split telemetry — new CompressionStatus.COMPRESSION_FAILED_OUTPUT_TRUNCATED enum value. Distinct from EMPTY_SUMMARY so logs/telemetry can tell prompt-quality failures (tune prompt / splitter) from capacity failures (raise cap / shrink splitter input). isCompressionFailureStatus() treats both as failures so the breaker behavior is unchanged. - R5.3: expand consecutiveFailures JSDoc to clarify it tracks "non-force, non-hard-rescue consecutive failures" — hard-rescue resets the counter and force=true skips increments, so the counter is the "regular path" health signal only; reactive overflow is the real safety net for the force-only paths. - R5.4: document the CompressOptions field rename (hasFailedCompressionAttempt: boolean → consecutiveFailures: number) as an SDK breaking change in the design doc with migration guide.
This commit is contained in:
LaZzyMan
parent
81ec0ee7b8
commit
cb3b09edb8
6 changed files with 85 additions and 28 deletions
|
|
@ -136,12 +136,22 @@ export interface ChatCompressionSettings {
|
|||
|
||||
### Breaking change 处理
|
||||
|
||||
启动时 `Config` 加载发现 `chatCompression.contextPercentageThreshold` 存在：
|
||||
**用户面：** 启动时 `Config` 加载发现 `chatCompression.contextPercentageThreshold` 存在：
|
||||
|
||||
- 写入 stderr 一行警告：`"chatCompression.contextPercentageThreshold has been removed and is now controlled by built-in thresholds."`
|
||||
- **不**报错、**不**阻塞启动
|
||||
- 字段值被忽略
|
||||
|
||||
**SDK 面（R5.4）：** `CompressOptions` 的 `hasFailedCompressionAttempt: boolean` 字段重命名为 `consecutiveFailures: number`。两点差异：
|
||||
|
||||
| | 旧字段 | 新字段 |
|
||||
| ---- | ------------------------------ | -------------------------------------------------------------------- |
|
||||
| 名称 | `hasFailedCompressionAttempt` | `consecutiveFailures` |
|
||||
| 类型 | `boolean` | `number` |
|
||||
| 语义 | `true` = 永久禁用 auto-compact | `>= MAX_CONSECUTIVE_FAILURES`（默认 3）= 暂时禁用直到 force 成功重置 |
|
||||
|
||||
仓库内只有 `GeminiChat.tryCompress` 一个内部消费方，所以内部 migration 风险低；但 `@qwen-code/qwen-code-core` 是 published package、`CompressOptions` 在 d.ts 里可见，下游 SDK 直接调 `service.compress({ ..., hasFailedCompressionAttempt: true })` 的代码会拿到 TS 编译错误。**迁移指引：** 把 `true` 改为 `MAX_CONSECUTIVE_FAILURES`（或任意 >= 3 的整数），`false` 改为 `0`。如果调用方维护自己的失败计数，直接传入即可。
|
||||
|
||||
## Token 估算补偿
|
||||
|
||||
qwen-code 的 `lastPromptTokenCount` 来自上一轮 API response 的 `usageMetadata.totalTokenCount`（[geminiChat.ts:1217-1232](packages/core/src/core/geminiChat.ts:1217)）。这导致：
|
||||
|
|
|
|||
|
|
@ -305,11 +305,22 @@ export async function collectContextData(
|
|||
|
||||
// Tier classification: prefer the API-reported total when available.
|
||||
// When no API call has happened yet (first /context, --continue resume,
|
||||
// sub-agent inheritance), classify against the estimated overhead instead
|
||||
// of forcing `safe` — a restored session with 800K of inherited history
|
||||
// should not silently show "safe" just because the API hasn't been hit.
|
||||
// The estimate is a lower bound (excludes message body until first turn)
|
||||
// so the tier may under-classify, but never over-classifies. (R2.2)
|
||||
// sub-agent inheritance), classify against `rawOverhead` so a session
|
||||
// dominated by system prompt / skills / MCP tools doesn't silently show
|
||||
// "safe". (R2.2)
|
||||
//
|
||||
// SCOPE GAP (R5.1): `rawOverhead` excludes `messagesTokens` — the actual
|
||||
// chat history. A `--continue` restore with 100K of historical messages
|
||||
// (but small overhead) will still display "safe" here, even though the
|
||||
// cheap-gate inside chatCompressionService will trigger compression on
|
||||
// the very next send (it uses `estimatePromptTokens(history, ...)` which
|
||||
// walks the real history). This is a UI/runtime divergence — for a
|
||||
// single render — that resolves the moment any send happens.
|
||||
//
|
||||
// TODO: plumb the chat history into collectContextData and use
|
||||
// estimatePromptTokens(history, undefined, 0, imageTokenEstimate) here
|
||||
// for same-source-of-truth as the cheap-gate. Defer because Config
|
||||
// doesn't expose the active chat instance today.
|
||||
const tierTokens = isEstimated ? rawOverhead : apiTotalTokens;
|
||||
|
||||
const breakdown: ContextCategoryBreakdown = {
|
||||
|
|
|
|||
|
|
@ -105,7 +105,8 @@ function isCompressionFailureStatus(status: CompressionStatus): boolean {
|
|||
return (
|
||||
status === CompressionStatus.COMPRESSION_FAILED_INFLATED_TOKEN_COUNT ||
|
||||
status === CompressionStatus.COMPRESSION_FAILED_EMPTY_SUMMARY ||
|
||||
status === CompressionStatus.COMPRESSION_FAILED_TOKEN_COUNT_ERROR
|
||||
status === CompressionStatus.COMPRESSION_FAILED_TOKEN_COUNT_ERROR ||
|
||||
status === CompressionStatus.COMPRESSION_FAILED_OUTPUT_TRUNCATED
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -450,11 +451,27 @@ export class GeminiChat {
|
|||
private lastPromptTokenCount = 0;
|
||||
|
||||
/**
|
||||
* Number of consecutive auto-compaction failures for this chat. The cheap-gate
|
||||
* NOOPs once this reaches MAX_CONSECUTIVE_FAILURES (default 3) until a successful
|
||||
* compress (forced or not) resets it to 0. Replaces the single-shot
|
||||
* hasFailedCompressionAttempt lock that previously disabled auto-compaction
|
||||
* for the rest of the session on any failure.
|
||||
* Number of consecutive auto-compaction failures for this chat. The
|
||||
* cheap-gate NOOPs once this reaches MAX_CONSECUTIVE_FAILURES (default 3)
|
||||
* until a successful compress (forced or not) resets it to 0. Replaces the
|
||||
* single-shot hasFailedCompressionAttempt lock that previously disabled
|
||||
* auto-compaction for the rest of the session on any failure.
|
||||
*
|
||||
* SEMANTICS (R5.3): this counter tracks "non-force, non-hard-rescue
|
||||
* consecutive failures", NOT every failure literally.
|
||||
* - Auto-compaction failures (cheap-gate path): increment by 1.
|
||||
* - Manual `/compress` failures: skipped (`force=true` → `!force`
|
||||
* guard in the failure branch).
|
||||
* - Hard-tier rescue failures: skipped (force=true) AND the counter
|
||||
* is reset to 0 BEFORE the rescue call (sendMessageStream), so
|
||||
* repeated hard-rescue failures never accumulate here. The rationale
|
||||
* is fail-open: hard predicts imminent overflow, so we should keep
|
||||
* trying regardless of recent failures. Reactive overflow is the
|
||||
* real safety net for that path — it bumps the counter by +1 so
|
||||
* N reactive failures will still trip the breaker.
|
||||
*
|
||||
* If you're debugging "why is hard-rescue firing but the counter is 0",
|
||||
* that's by design.
|
||||
*/
|
||||
private consecutiveFailures = 0;
|
||||
|
||||
|
|
|
|||
|
|
@ -171,6 +171,17 @@ export enum CompressionStatus {
|
|||
|
||||
/** The compression was not necessary and no action was taken */
|
||||
NOOP,
|
||||
|
||||
/**
|
||||
* The compression succeeded but the summary output hit
|
||||
* COMPACT_MAX_OUTPUT_TOKENS, suggesting truncation. Distinct from
|
||||
* `EMPTY_SUMMARY` so telemetry can separate prompt-quality failures
|
||||
* (empty / nonsensical summary) from capacity failures (output cap
|
||||
* hit, may need a higher cap or finer-grained splitter).
|
||||
* `isCompressionFailureStatus` treats this as a failure so it counts
|
||||
* toward the per-chat circuit breaker. (R5.2)
|
||||
*/
|
||||
COMPRESSION_FAILED_OUTPUT_TRUNCATED,
|
||||
}
|
||||
|
||||
export interface ChatCompressionInfo {
|
||||
|
|
|
|||
|
|
@ -2116,11 +2116,12 @@ describe('ChatCompressionService.compress sideQuery config', () => {
|
|||
expect(callArg.config?.maxOutputTokens).toBe(20_000);
|
||||
});
|
||||
|
||||
it('returns FAILED_EMPTY_SUMMARY when the summary output hits the COMPACT_MAX_OUTPUT_TOKENS cap (likely truncated)', async () => {
|
||||
it('returns FAILED_OUTPUT_TRUNCATED when the summary output hits the COMPACT_MAX_OUTPUT_TOKENS cap (likely truncated)', async () => {
|
||||
// Mock the side-query to return a non-empty summary that exactly hits the
|
||||
// 20K cap — the guard added in this PR should drop the result and surface
|
||||
// it as a failure so non-force callers tick the consecutive-failure
|
||||
// breaker (review #4168 R1.1: NOOP made the breaker never trip).
|
||||
// 20K cap — the guard should drop the result and surface it as a failure
|
||||
// with a status distinct from EMPTY_SUMMARY so telemetry can separate
|
||||
// prompt-quality failures (empty) from capacity failures (truncated).
|
||||
// (R1.1 made the breaker tick; R5.2 split the status.)
|
||||
vi.spyOn(sideQueryModule, 'runSideQuery').mockResolvedValue({
|
||||
text: '<state_snapshot>truncated...',
|
||||
usage: {
|
||||
|
|
@ -2166,7 +2167,7 @@ describe('ChatCompressionService.compress sideQuery config', () => {
|
|||
});
|
||||
|
||||
expect(result.info.compressionStatus).toBe(
|
||||
CompressionStatus.COMPRESSION_FAILED_EMPTY_SUMMARY,
|
||||
CompressionStatus.COMPRESSION_FAILED_OUTPUT_TRUNCATED,
|
||||
);
|
||||
expect(result.newHistory).toBeNull();
|
||||
expect(warn).toHaveBeenCalledWith(
|
||||
|
|
|
|||
|
|
@ -541,12 +541,17 @@ export class ChatCompressionService {
|
|||
|
||||
// Defensive guard: if the side-query hit COMPACT_MAX_OUTPUT_TOKENS, the
|
||||
// summary is likely truncated mid-content and unsafe to persist. Drop it
|
||||
// and surface it as a failure so the consecutive-failure breaker counts
|
||||
// it — if the model consistently produces max-length summaries we want
|
||||
// to stop trying after MAX_CONSECUTIVE_FAILURES strikes rather than burn
|
||||
// an API call on every send. Reactive overflow still catches the
|
||||
// catastrophic case. See docs/design/auto-compaction-threshold-redesign.md
|
||||
// risk #2.
|
||||
// and surface as a failure so the consecutive-failure breaker counts it —
|
||||
// if the model consistently produces max-length summaries we want to stop
|
||||
// trying after MAX_CONSECUTIVE_FAILURES strikes rather than burn an API
|
||||
// call on every send. Reactive overflow still catches the catastrophic
|
||||
// case. See docs/design/auto-compaction-threshold-redesign.md risk #2.
|
||||
//
|
||||
// TODO(finish_reason): the current `>= cap` check is a heuristic that
|
||||
// false-positives on legitimate summaries that happen to land exactly at
|
||||
// the cap. The proper signal is `finish_reason === 'length'` (OpenAI) /
|
||||
// `MAX_TOKENS` (Gemini), but `runSideQuery` doesn't surface it today.
|
||||
// Plumb it through and tighten this guard when that's available.
|
||||
if (
|
||||
!isSummaryEmpty &&
|
||||
typeof compressionOutputTokenCount === 'number' &&
|
||||
|
|
@ -565,11 +570,13 @@ export class ChatCompressionService {
|
|||
info: {
|
||||
originalTokenCount,
|
||||
newTokenCount: originalTokenCount,
|
||||
// Reuse the empty-summary status: from the persistence layer's
|
||||
// perspective a truncated summary is unusable just like an empty
|
||||
// one. `isCompressionFailureStatus()` returns true for this enum,
|
||||
// so non-force callers will tick the consecutive-failure counter.
|
||||
compressionStatus: CompressionStatus.COMPRESSION_FAILED_EMPTY_SUMMARY,
|
||||
// Distinct from EMPTY_SUMMARY so telemetry / logs can tell a
|
||||
// prompt-quality failure (empty summary → tune prompt / splitter)
|
||||
// apart from a capacity failure (output cap hit → raise cap or
|
||||
// shrink splitter input). isCompressionFailureStatus() treats both
|
||||
// as failures so the persistence behaviour is unchanged. (R5.2)
|
||||
compressionStatus:
|
||||
CompressionStatus.COMPRESSION_FAILED_OUTPUT_TRUNCATED,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue