[Suggestion] Two issues at this assignment site:

1. Double-counting via totalTokenCount fallback. When promptTokenCount is absent (line 2538), lastPromptTokenCount falls back to totalTokenCount, which already includes candidatesTokenCount per the Gemini API. Adding candidatesTokenCount again here means the next estimatePromptTokens call computes (prompt + candidates) + candidates + userEst — over-counting by exactly candidatesTokenCount. This path is reachable via OpenAI-compatible providers that only report total_tokens and completion_tokens.

Consider guarding so lastCandidatesTokenCount is only set when lastPromptTokenCount came from the authoritative promptTokenCount:

if (usageMetadata.promptTokenCount) {
  this.lastCandidatesTokenCount = usageMetadata.candidatesTokenCount ?? 0;
}

2. Missing thoughtsTokenCount. For thinking models (Gemini 2.5 Pro, etc.), thinking tokens are reported in a separate thoughtsTokenCount field that is NOT included in candidatesTokenCount. Thought parts are appended to history (line ~2686) and count toward the next promptTokenCount, but this field only captures visible output tokens. For a response with 15K thinking tokens and 500 visible output tokens, the estimate under-counts by 15K.

Consider including thinking tokens:

this.lastCandidatesTokenCount =
  (usageMetadata.candidatesTokenCount ?? 0) +
  (usageMetadata.thoughtsTokenCount ?? 0);

(Consider renaming the field to lastOutputTokenCount to match the broader semantics.)

— qwen3.7-max via Qwen Code /review

Fixed in 846ccf4. The usage metadata path now only carries previous-response output tokens when promptTokenCount is present, so the totalTokenCount fallback is not double-counted. It also adds thoughtsTokenCount to the carried output token count for thinking-model responses.

Added regressions for both cases: one where totalTokenCount + candidatesTokenCount must stay below the hard-tier force threshold, and one where promptTokenCount + candidatesTokenCount + thoughtsTokenCount must cross it.

[Suggestion] This line has a hidden side-effect: it zeroes lastCandidatesTokenCount. The JSDoc on setLastPromptTokenCount (lines 1320-1326) only mentions seeding prompt-token counts and doesn't document this reset. A future caller who uses this method to update the prompt count mid-session (not just at seeding time) will silently discard the candidates count from the last response.

Consider updating the JSDoc to mention the side-effect:

* Side-effect: also resets `lastCandidatesTokenCount` to 0, because
* externally-seeded prompt counts come from a different response than
* the one that populated `lastCandidatesTokenCount`.

— qwen3.7-max via Qwen Code /review

Documented in 846ccf4. The JSDoc now explicitly says setLastPromptTokenCount() clears the remembered previous-response output token count because externally seeded prompt counts come from a different chat instance and should not inherit this chat's last response size.

[Suggestion] For OpenAI-compatible providers, candidatesTokenCount already includes reasoning_tokens (the OpenAI converter at openaiContentGenerator/converter.ts sets candidatesTokenCount = completion_tokens, which includes reasoning_tokens, and separately sets thoughtsTokenCount = reasoning_tokens). Adding them here gives completion_tokens + reasoning_tokens — double-counting the reasoning portion.

Native Gemini is unaffected (candidatesTokenCount and thoughtsTokenCount are disjoint). But for OpenAI thinking models (e.g., o1/o3), this inflates lastCandidatesTokenCount by the size of reasoning_tokens (potentially 1K–10K per turn), making the hard-tier estimate more conservative than necessary.

Two possible fixes:

Option A — derive from totalTokenCount (provider-agnostic):

this.lastCandidatesTokenCount =
  usageMetadata.promptTokenCount !== undefined
    ? Math.max(0, (usageMetadata.totalTokenCount ?? 0) - usageMetadata.promptTokenCount)
    : 0;

Option B — fix the OpenAI converter to subtract reasoning from candidates:

candidatesTokenCount: Math.max(0, completionTokens - thinkingTokens),

Option A keeps the fix local; Option B aligns the converter with Gemini semantics (which may benefit other consumers too).

— qwen3.7-max via Qwen Code /review

Handled in 85668cd. The carried previous-response output token count now prefers totalTokenCount - promptTokenCount when totalTokenCount is available, so OpenAI-compatible completion_tokens that already include reasoning_tokens are not counted again via thoughtsTokenCount. Added a regression covering this exact OpenAI-compatible reasoning-token shape.

[Suggestion] lastCandidatesTokenCount uses promptTokenCount !== undefined as its guard, but lastPromptTokenCount (line 2547) uses a truthy check on the ?? result. When promptTokenCount === 0, lastCandidatesTokenCount IS updated but lastPromptTokenCount is NOT — the two counters become desynchronized, and the next estimatePromptTokens mixes a stale prompt count with a fresh candidate count.

Consider moving lastCandidatesTokenCount inside the if (lastPromptTokenCount) block so both counters update atomically:

if (lastPromptTokenCount) {
  this.lastPromptTokenCount = lastPromptTokenCount;
  this.lastCandidatesTokenCount =
    (usageMetadata.candidatesTokenCount ?? 0) +
    (usageMetadata.thoughtsTokenCount ?? 0);
  this.telemetryService?.setLastPromptTokenCount(lastPromptTokenCount);
}

Also: the field name lastCandidatesTokenCount is slightly misleading — it stores candidatesTokenCount + thoughtsTokenCount (all output tokens), not just candidates. Renaming to lastOutputTokenCount would prevent future maintainainers from accidentally double-counting thoughts. A debugLogger.debug at the assignment site would also help with compaction debugging.

— qwen3.7-max via Qwen Code /review

Fixed in 75afe0c. The previous-output counter now updates only inside the same if (lastPromptTokenCount) block as lastPromptTokenCount, so a zero prompt count cannot refresh one counter while leaving the other stale.

[Suggestion] The 0 here is lastCandidatesTokenCount. Consider adding an inline comment explaining why: the compression service lacks access to the chat's per-chat output token count, and the precomputedEffectiveTokens path (which already includes candidates) covers the common sendMessageStream caller.

— qwen3.7-max via Qwen Code /review

Handled in 75afe0c. Added the inline comment explaining that this path has no per-chat output-token state and that the common sendMessageStream caller uses precomputedEffectiveTokens, which already includes previous output tokens.

[Suggestion] Two test gaps:

1. Compression-success reset: the existing test "resets previous response candidate tokens when seeding last prompt tokens externally" covers the setLastPromptTokenCount reset path (line 1332) but not the compression-success reset at line 1375. Add a test that primes lastCandidatesTokenCount, triggers successful compression, and asserts the follow-up's precomputedEffectiveTokens does NOT include pre-compression candidate tokens.

2. Candidates ignored on fallback: no test verifies estimatePromptTokens(history, user, 0, 9999) ignores lastCandidatesTokenCount when lastPromptTokenCount is 0:

it('ignores lastCandidatesTokenCount when lastPromptTokenCount is 0', () => {
  const fullEst = estimateContentTokens([...history, user]);
  expect(estimatePromptTokens(history, user, 0, 9999)).toBe(fullEst);
});

— qwen3.7-max via Qwen Code /review

Added in 75afe0c. New coverage verifies successful compression clears the previous-output token contribution before the next estimate, and estimatePromptTokens(..., 0, lastCandidatesTokenCount) ignores candidate tokens on the first-send fallback path.

[Suggestion] The nested ternary here duplicates the same three-branch output-token derivation as getUsageOutputTokenCountForPromptEstimate() in sessionService.ts (added in this same PR). Both implement: (1) no promptTokenCount → 0, (2) totalTokenCount present → total - prompt, (3) fallback → candidates + thoughts. When a new provider reports tokens differently, a maintainer may update one copy and miss the other, causing resume vs. steady-state to produce different lastCandidatesTokenCount values.

(Export getUsageOutputTokenCountForPromptEstimate from sessionService.ts or move it to tokenEstimation.ts so both call sites share one implementation.)

— qwen3.7-max via Qwen Code /review

Handled in 9a60577. Moved the output-token derivation into shared getUsageOutputTokenCountForPromptEstimate() in tokenEstimation.ts; both steady-state stream handling and resume restoration now use the same helper.

[Suggestion] The stream handler's candidatesTokenCount + thoughtsTokenCount fallback branch (geminiChat.ts:2564-2565 — fires when promptTokenCount is defined but totalTokenCount is undefined) is not exercised by any stream-path test. All three existing tests that provide promptTokenCount also provide totalTokenCount. The equivalent computation in getUsageOutputTokenCountForPromptEstimate IS tested for this scenario ("should restore disjoint candidate and thought output tokens when total is unavailable"), but the stream handler's duplicate is not.

Add a stream-path test modeled on "includes previous response thought tokens" but with totalTokenCount omitted from mock usageMetadata:

mockResolvedValueOnce(
  makeStreamResponse('first', {
    promptTokenCount: 176_000,
    candidatesTokenCount: 1_200,
    thoughtsTokenCount: 300,
    // totalTokenCount intentionally omitted
  }),
)

Then assert that precomputedEffectiveTokens on the follow-up call reflects 176_000 + 1_500 (prompt + candidates + thoughts).

— qwen3.7-max via Qwen Code /review

Added in 9a60577. The stream path now has coverage for usage metadata with promptTokenCount, candidatesTokenCount, and thoughtsTokenCount while totalTokenCount is omitted, and the follow-up hard-tier estimate includes candidates + thoughts.

[Suggestion] setLastCandidatesTokenCount uses Math.max(0, count), which does not guard against NaN — Math.max(0, NaN) returns NaN. If a future caller passes NaN (e.g., from unchecked arithmetic on usage metadata), lastCandidatesTokenCount becomes NaN, propagating through estimatePromptTokens and making effectiveTokens >= hard evaluate to false — silently disabling the hard-tier rescue gate with no observable signal.

— qwen3.7-max via Qwen Code /review

Fixed in 11d064d. The old candidates setter was replaced by seedResumeTokenCounts(), which normalizes non-finite output counts to 0 before storing them.

[Suggestion] The field lastCandidatesTokenCount (and the setter/parameter names throughout) stores candidatesTokenCount + thoughtsTokenCount — the total non-prompt output — not just the Gemini API's candidatesTokenCount. A future maintainer reading the field name will assume it maps 1:1 to the API's candidatesTokenCount and may omit thoughtsTokenCount in new code paths, or double-count thoughts if added separately.

Consider renaming to lastOutputTokenCount across the field, getter/setter, and the estimatePromptTokens parameter to match the semantic (total output tokens from the previous response).

— qwen3.7-max via Qwen Code /review

Fixed in 11d064d. The per-chat counter and estimatePromptTokens parameter are now named lastOutputTokenCount, matching the candidates+thoughts semantics instead of the raw Gemini field name.

[Suggestion] The resume flow depends on a strict call order: setLastPromptTokenCount (which silently zeroes lastCandidatesTokenCount as a side-effect), then setLastCandidatesTokenCount (which restores the value). Nothing at the type or API level enforces this ordering — a future refactor that reorders or wraps these calls will silently zero the candidates count with no compile error or runtime error.

Consider introducing an atomic seeder method to make the coupling explicit:

Keep setLastPromptTokenCount (with its zeroing side-effect) for subagent/post-compression paths where zeroing is correct.

— qwen3.7-max via Qwen Code /review

Fixed in 11d064d. Resume now seeds prompt/output counts through seedResumeTokenCounts(promptTokenCount, outputTokenCount), so the two restored counters are set atomically.

[Suggestion] The || to ?? change here is a behavioral change: when promptTokenCount === 0, the old || would fall through to totalTokenCount (entering the update block), while ?? keeps 0, causing if (lastPromptTokenCount) to be falsy and skipping both counter updates. No test exercises promptTokenCount: 0 to pin the intended behavior. Without a regression test, a future maintainer could revert to || thinking the change was accidental, reintroducing the original over-counting.

Consider adding a test with usageMetadata: { promptTokenCount: 0, totalTokenCount: 5000 } that documents the expected behavior (counters remain unchanged).

— qwen3.7-max via Qwen Code /review

Fixed in 11d064d. Added does not replace token counters when usage reports zero prompt tokens, which pins that promptTokenCount: 0 does not reseed counters from totalTokenCount.

[Suggestion] The candidatesTokenCount + thoughtsTokenCount fallback branch in getUsageOutputTokenCountForPromptEstimate (when promptTokenCount is defined but totalTokenCount is absent) is not tested through the stream handler path. Every test with promptTokenCount also provides totalTokenCount, so the ternary always takes the subtraction branch. The same function IS tested via sessionService.test.ts ("should restore disjoint candidate and thought output tokens when total is unavailable"), but the stream handler's invocation of this branch remains uncovered.

Providers that return promptTokenCount without totalTokenCount rely on this untested path for correct compaction thresholds.

— qwen3.7-max via Qwen Code /review

Covered in 9a60577 by includes disjoint candidate and thought tokens when total token count is unavailable; the stream-path fixture omits totalTokenCount and verifies the next hard-tier estimate includes candidates + thoughts.

[Suggestion] getResumeTokenCounts(conversation) is called twice for the same conversation object during resume: once inside replayUiTelemetryFromConversation() at line 228 (which was refactored in this PR to call getResumeTokenCounts internally), and again here. Both walk messages backward — an O(n) traversal done redundantly.

Consider having replayUiTelemetryFromConversation return the ResumeTokenCounts | undefined it already computed, and consuming the return value here instead of recomputing:

// sessionService.ts
export function replayUiTelemetryFromConversation(
  conversation: ConversationRecord,
): ResumeTokenCounts | undefined {
  const resumeTokenCounts = getResumeTokenCounts(conversation);
  if (resumeTokenCounts !== undefined) {
    uiTelemetryService.setLastPromptTokenCount(resumeTokenCounts.promptTokenCount);
  }
  return resumeTokenCounts;
}

// client.ts
const resumeTokenCounts = replayUiTelemetryFromConversation(
  resumedSessionData.conversation,
);

— qwen3.7-max via Qwen Code /review

Fixed in 4b27a34. replayUiTelemetryFromConversation() now returns the ResumeTokenCounts it computes, and the resume path consumes that return value instead of walking the conversation a second time.

[Suggestion] The fixture has internally inconsistent token accounting: promptTokenCount: 200 + candidatesTokenCount: 60 + thoughtsTokenCount: 20 = 280, but totalTokenCount: 260. Because getUsageOutputTokenCountForPromptEstimate takes the totalTokenCount - promptTokenCount = 60 path when totalTokenCount is defined, it silently drops thoughtsTokenCount: 20 and the assertion toHaveBeenCalledWith(60) passes without verifying that thinking tokens are included.

Fix the fixture so totalTokenCount matches the sum (280), then assert toHaveBeenCalledWith(80) (candidates 60 + thoughts 20):

— qwen3.7-max via Qwen Code /review

Fixed in 4b27a34. The fixture now uses consistent accounting (promptTokenCount: 200, candidatesTokenCount: 60, thoughtsTokenCount: 20, totalTokenCount: 280) and asserts the resumed seed receives output token count 80.

[Suggestion] This disjoint fallback lacks a Math.max(0, ...) wrapper, unlike the sibling totalTokenCount - promptTokenCount branch on line 94. The streaming path in geminiChat.ts:2559-2560 assigns the return value directly to this.lastCandidatesTokenCount (bypassing the Math.max(0, count) guard in setLastCandidatesTokenCount()), so a provider that returns negative candidatesTokenCount or thoughtsTokenCount would propagate an unclamped value.

Wrap for consistency:

— qwen3.7-max via Qwen Code /review

Fixed in 4b27a34. The disjoint candidates+thoughts fallback is clamped with Math.max(0, ...), and tokenEstimation.test.ts now covers negative disjoint output counts.

Bug: estimatePromptTokens 的签名新增了第 4 个参数 lastOutputTokenCount（插在 imageTokenEstimate 之前），但同文件下方 localPromptTokensAfterCompression 的计算（原 L1603-1608）没有同步更新：

const localPromptTokensAfterCompression = shouldForceFromHard
  ? estimatePromptTokens(
      this.lastPromptTokenCount > 0 ? [] : this.getHistoryShallow(true),
      userContent,
      this.lastPromptTokenCount,
      imageTokenEstimate, // ← 现在被当作 lastOutputTokenCount
    )
  : 0;

imageTokenEstimate（通常 1600）被当成 lastOutputTokenCount 传入，而 imageTokenEstimate 则回退到默认值。恰好两个值目前相同所以不会出明显问题，但逻辑上是错的，用户自定义 imageTokenEstimate 时两个参数会同时出错。

建议加上 0：

estimatePromptTokens(
    ...,
    this.lastPromptTokenCount,
    0,
    imageTokenEstimate,
)

I checked the current PR head (4b27a344e) and this appears to be stale against an older shape of the diff.

The localPromptTokensAfterCompression call site no longer exists (rg localPromptTokensAfterCompression returns no matches). The active geminiChat.ts call now passes both parameters in order:

estimatePromptTokens(
  this.lastPromptTokenCount > 0 ? [] : this.getHistoryShallow(true),
  userContent,
  this.lastPromptTokenCount,
  this.lastOutputTokenCount,
  imageTokenEstimate,
)

The compression-service fallback call site also uses the explicit unavailable-output-token value before the image estimate:

estimatePromptTokens(..., 0, slimmingConfig.imageTokenEstimate)

No code change made for this thread unless there is another current call site I missed.

When totalTokenCount is unavailable, this sums candidatesTokenCount + thoughtsTokenCount assuming they are disjoint. Some OpenAI-compatible providers (e.g. certain DeepSeek-compatible endpoints) include thinking/reasoning tokens inside candidatesTokenCount, making them overlap. In that case this double-counts, inflating the prompt estimate by up to the full thoughtsTokenCount.

The totalTokenCount branch correctly avoids this, but this fallback branch does not. Over-estimating could trigger unnecessary hard-rescue compression one send earlier than needed.

Consider guarding against the overlap case, e.g.:

const candidates = usage.candidatesTokenCount ?? 0;
const thoughts = usage.thoughtsTokenCount ?? 0;
// If thoughts look like they are included in candidates, do not add them.
return Math.max(0, Math.max(candidates, candidates + thoughts));

or documenting the known over-count as acceptable conservatism.

Covered in the current branch. When totalTokenCount is unavailable, getUsageOutputTokenCountForPromptEstimate() now treats thoughtsTokenCount as potentially overlapping only when candidatesTokenCount strictly dominates it, so DeepSeek-compatible shapes that include reasoning inside candidates avoid double-counting. Verified with npm run test --workspace=packages/core -- src/services/tokenEstimation.test.ts, Prettier, and ESLint.

[Suggestion] seedResumeTokenCounts (line 1375) correctly guards with Number.isFinite() + Math.max(0, ...), proving awareness of malformed-input risk. This streaming path assigns promptTokenCount ?? totalTokenCount directly with no finite/non-negative guard. A negative value (e.g. promptTokenCount: -1000) passes the truthy check, sets a negative counter, and causes estimatePromptTokens to under-count — silently disabling the compaction gate. getUsageOutputTokenCountForPromptEstimate also lacks a finite guard on its return value. Low probability with production providers but inconsistent with the resume path.

And for the output count:

— qwen3.7-max via Qwen Code /review