fix(#312): invalidate stale modelContextLimit on catalog miss (#313 + #314 combined) - #315
Merged
Merged
Conversation
Completes the #314 fix with #313's fail-safe as the fallback path. #314 (merged) reconciles state.modelContextLimit from the model-limit catalog when the request's user message names a known model. When the catalog MISSES (fresh instance + failed hydration + never-used model), the stale limit survived and every percentage threshold still ran against the previous model's window — the exact #312 false positive, narrowed to a rare corner but not eliminated. This PR records the identity pair (modelProviderID/modelID) alongside the limit whenever the system hook writes it, and on a catalog miss the messages hook compares the request's model against that identity: - match → keep the limit (no needless blindness when the catalog simply lacks an entry for the CURRENT model) - mismatch→ invalidate (limit = undefined); consumers already tolerate undefined (fresh sessions run with it until the first system.transform), and this same request's system.transform refreshes the pair anyway - legacy persisted states carry no identity → treated as stale (one blind turn per upgraded session, same as #313) The identity pair is persisted/restored together with the limit. Tests: legacy-state invalidation, identity-mismatch invalidation, same-identity keep (emergency still fires at 130%), system-hook identity recording, persistence round-trip. 991/991 pass, tsc clean.
📦 Built Plugin ArtifactBranch: Option A — Install from npm PR tag (recommended)opencode plugin opencode-acp@pr-315 --globalEach push to this PR publishes a new version under the Option B — Install from GitHubopencode plugin "github:ranxianglei/opencode-acp#fix/stale-limit-fallback" --globalOption C — Download artifact
tar xzf opencode-acp-pr315.tgz
cp -r package/dist ~/.cache/opencode/packages/opencode-acp@latest/node_modules/opencode-acp/dist
This comment is automatically updated on each push. |
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Completes the #314 fix (merged) with #313's fail-safe as the fallback path — the "ideal combined fix" from the review of both PRs.
Background
Issue #312: after switching models (e.g. 200K → 1M), a 50% emergency-compression threshold fired at ~26% real usage, because
experimental.chat.messages.transformfires beforesystem.transformin one request, and only the system hook writesstate.modelContextLimit— so the first request after a switch computes every percentage against the previous model's window.${providerID}/${modelID}→ limit catalog (seeded at init from/config/providers, recorded live by the system hook).This PR
Records the identity pair (
modelProviderID/modelID) alongside the limit whenever the system hook writes it, persisted and restored together. On a catalog miss the messages hook now compares the request's model against that identity:limit = undefined) — #313's fail-safe: never run percentage math against the wrong windowundefinedis already the natural fresh-session state (before the firstsystem.transform), so all consumers (nudges, GC, batch-cleanup, adaptive growth) tolerate it by construction. The same request'ssystem.transformrefreshes the pair right after, so the blind window is a single turn at most.Supersedes
tests/model-switch-limits.test.ts).Changes
lib/state/types.ts—SessionState.modelProviderID/modelID(optional, persisted)lib/state/state.ts— init/reset/load keep the pair atomic with the limitlib/state/persistence.ts— serialize/restore the pairlib/hooks.ts— system hook records identity; messages hook: catalog hit → correct + record identity; miss + mismatch → invalidateVerification
tsc --noEmitclean