Repository navigation
Conversation
![opencodejr[bot]](https://avatars.githubusercontent.com/in/5497873?s=60&v=4){kind=small}
There was a problem hiding this comment.
The fix is sound and small. I reproduced #61372 on the base (estimateContext returns 89000 where 21 is expected), and the same assertion passes with this PR. The compaction and session-runner tests pass on both base and PR (279 each), and packages/core type-checks.
Public API: unchanged.
The only open point is design: due's native-compaction lookup and measuredPrompt still use the provider-only anchor, while estimatePrompt now uses a model-scoped one (see the inline comment). Note too that after a model switch within one provider, the size now comes from the chars/4 estimate rather than the sibling model's count. That estimate can run low on dense content, so a switch to a smaller-window model may hit the overflow path before automatic compaction kicks in. That seems to be the intended trade-off from the issue, but maintainers should be aware of it.
Related: #57392
| /** The size of the prompt `context` sends. */ | ||
| export const estimatePrompt = (context: SessionContext.Loaded): Estimate => { | ||
| const anchorIndex = context.messages.findLastIndex((message) => hasMeasuredPrompt(message, context.model.ref)) | ||
| const anchorIndex = context.messages.findLastIndex( |
There was a problem hiding this comment.
This anchor is now model-scoped, but two other places still use the provider-only hasMeasuredPrompt. due's native-compaction guard (around line 428) and measuredPrompt (around line 1178) can now pick a different anchor than this one. One case: model A is measured, then a native compaction happens, then model B (same provider) is measured. If the user switches back to A, the guard sees B's later count and lets due continue. This function then anchors on A's count from before the native compaction, which still includes content the native window replaced, so the estimate can be stale in the other direction. Consider a single helper for "measured by the current model", and decide explicitly which rule each caller needs. The handoff on #61372 also suggests matching variant and agent, as measuredPrompt does, since the agent changes the system prompt and tools and so the prompt size.
There was a problem hiding this comment.
The earlier point is addressed. Every place that reads a measured count now uses one check (same provider, model, variant and agent). So due's native-window guard and the prompt estimate can no longer pick different anchors, and the case of switching back to a model after a native compaction now waits for a matching measurement. I re-ran the #61372 reproduction and it passes, as do the compaction and session-runner tests (279) and the packages/core type check. Public API: unchanged. No new concerns.
b43248b to
5e319e9
Compare
There was a problem hiding this comment.
The earlier point is addressed. After the rebase, the background-summary start (cycleStart) also uses the shared provider/model/variant/agent check, so every place that reads a measured count now agrees. I re-ran the #61372 reproduction: it fails on the base (89000 where 21 is expected) and passes with this PR. The new small-window test fails without the last commit's source change. The compaction and session-runner tests pass (291), and packages/core type-checks. Public API: unchanged. No new concerns.
Issue for this PR
Closes #61372
Type of change
What does this PR do?
Reuse measured usage only for the same provider, model, variant, and agent. The same check covers prompt estimates, native-window eligibility, background-summary anchors, and the first measurement that sets the background-summary lead.
Other request shapes use transcript estimation; dense content can still be underestimated until the provider reports overflow.
How did you verify your code works?
dev: an earlier model's 46k count delays the background start from 34k to 47k.bun run check: lint and all 35 typecheck tasks passed. Corebun run typecheck:tscpassed.AI-assisted implementation and local verification; deterministic test responses, no live provider calls.
Screenshots / recordings
Not applicable to this core logic change.
Checklist
Added by the opencode bot (AI-generated).
Related: #57392