fix(#325): floor meter at provider anchor minus tokens reclaimed since it - #328
fix(#325): floor meter at provider anchor minus tokens reclaimed since it#328ranxianglei wants to merge 1 commit into
Conversation
…e it After a successful compress, usageAnchorPredatesCompression() skipped the provider-usage floor entirely, so tokenCount fell back to the raw sent-view estimate — which structurally omits tool-definition schemas, ref-tag overhead, and the injected nudge (a fixed ~70–80K for this host). The meter read false-idle; on the next turn a fresh provider usage re-engaged the floor at the full real prompt size (~84–95% of limit) and fired a spurious EMERGENCY, which the model answered by compressing again → loop. Fix: when the anchor predates a successful compress, floor at (lastProviderUsage − netReclaimedSinceAnchor) instead of skipping. netReclaimed sums max(0, compressedTokens − summaryTokens) over active blocks whose creating compress toolResult lands after the last valid usage anchor. The provider's real number already counts all fixed overhead accurately, so subtracting only the genuinely-freed tokens yields an exact post-compression size without needing to measure tool schemas (unmeasurable/unstable in the extension). New work added after the compress still grows tokenCount via sentTokens as before. No regression on the non-stale path (floor unchanged); a stale-anchor compress that reclaimed little still correctly trips the nudge.
[bot] 🏷 Reviewing now — pulling the PR diff, re-verifying the root-cause chain independently, and running the full suite locally before giving a verdict. Will report back shortly. |
Reviewed end-to-end — the fix logic is correct (root cause independently verified, floor math proven in both quadrants), but the PR cannot land as-is: it is What I verified
Design verdictApproving the approach over the alternative: subtracting genuinely-freed tokens from the provider's real number is exact and needs no magic constant; "add the fixed component back to the estimate" would require measuring tool schemas that are unmeasurable/unstable inside the extension. Right call. Blocker: dirty merge vs masterBranch point is I resolved it locally and validated: full suite 633 / 630 pass / 0 fail / 3 skip, typecheck + build green. Recipe for the rebase:
Minor finding (report-only, no separate issue per policy)
Recommendation: rebase onto current master with the recipe above (happy to share the exact hunks if useful). Fix code itself: LGTM. |
Fixes #325.
Problem
After a successful compress, ACP's own token accounting dropped ~70–80K below the provider-reported prompt size, so the nudge meter flipped to
idle. On the next turn the host-floor anchor (#257) snappedtokenCountback up to the real prompt size (~84–95% of the effective limit) and re-injected an EMERGENCY "compress now" nudge. The model compressed again → loop. The agent stopped working every few turns just to compress.Root cause
src/index.tssetstokenCount = estimateTokens(coreMessages, coveredIds, imageTokens) + systemPromptTokens, then floors it at the provider-reported prompt size — unlessusageAnchorPredatesCompression()is true (a successful compress landed after the last valid assistant usage), in which case the floor was skipped entirely.estimateTokens(src/tokens.ts) sums message text only. It structurally omits tool-definition schemas, ref-tag overhead, and the injected nudge — a fixed ~70–80K on this host. So the raw sent-view is always that far below the real request.tokenCountto the provider number — which is why pre-compression estimates "matched the provider exactly."tokenCountback to the undercounting raw estimate → falseidle. Next turn, fresh provider usage re-engaged the floor at full real size → spurious EMERGENCY.The #289 view-recount inherited the same blind spot because it compares against the post-processTurn view.
Fix
When the anchor predates a successful compress, floor at
(lastProviderUsage − netReclaimedSinceAnchor)instead of skipping.netReclaimedSinceAnchor= Σ over active blocks whose creatingcompresstoolResult lands after the last valid usage anchor ofmax(0, compressedTokens − summaryTokens).The provider's real number already counts all fixed overhead (tool schemas, etc.) accurately; subtracting only the genuinely-freed tokens yields an exact post-compression size with no need to measure tool schemas — which are unmeasurable/unstable inside the extension (no API enumerates Pi's full tool set). This is why the issue's suggested direction ("add the fixed component to the estimate") was not taken: it would need an unstable magic constant.
New work added after the compress still grows
tokenCountthroughsentTokensas before; the non-stale path is unchanged; a stale-anchor compress that reclaimed little still correctly trips the nudge.Tests
tests/floor-stale.test.ts: unit tests forcompressionAnchorStaleness(fresh / stale / pre-anchor exclusion / inactive+unattributable skip / negative-savings clamp / multi-block sum).tests/sent-view-arbitration.test.ts: rewrote the stale test to seed a real reclaiming block (no false emergency), plus a guard test asserting a tiny-reclaim + near-limit context still fires.Notes