Conversation
…ution (Fixes #2527) Lower DEFAULT_TOKEN_LIMIT from 1,048,576 to 200,000 so unrecognized models get a fail-safe conservative context window instead of a dangerous 1M fallback that silently overruns sub-200K backends (#2270). Add GLM model family to the token-limit catalog: glm-5.x prefix -> 200K, glm-4.x prefix -> 128K, with exact entries for glm-5.2, glm-5.1, glm-5, glm-4.6, glm-4.5, and glm-4 (#2280). Add Gemini prefix entries (gemini-2.5-pro, gemini-2.5-flash, gemini-2.0-flash -> 1M) to prevent regressions from the lowered default for models that previously fell through to DEFAULT_TOKEN_LIMIT. Consolidate the two duplicated context-window resolution paths into a single shared resolveEffectiveContextLimit() function in tokenLimits.ts. Both getTokenLimitForConfiguredContext (agents layer) and ephemerals.contextLimit() (core runtime) now delegate to it, ensuring all consumers agree by construction (#2270 DRY item).
Contributor
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 29 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (23)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
the
maintainer:e2e:ok
Contributor
OpenCodeReview — PR #2565
|
Contributor
LLxprt PR Review – PR #2565Issue AlignmentStrong. The PR resolves #2527 by lowering Side EffectsThe lowered default changes behavior for unrecognized models, but this is the intended fail-safe fix. Integration tests were updated to expect Code QualityGood. Tests and CoverageCoverage impact: increase. New tests cover GLM exact/prefix resolution, Gemini prefix variants, and VerdictReady. The PR is focused, addresses the issue completely, improves code quality through DRY consolidation, and includes meaningful automated tests for the new behavior. |
…sted-conditional)
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-24.x-ubuntu-latest' artifact from the main CI run. |
Collaborator
Author
CI Status Update — E2E and LLxprt Review Failures Are Pre-existing/InfrastructureI have investigated all remaining CI failures on this PR and confirmed they are not caused by the changes in this PR. Here is the evidence: E2E Test Failures (all 3 environments: macOS, Linux sandbox:none, Linux sandbox:docker)The same 3 test files fail identically across all E2E environments:
Root cause: Stream idle timeout from the LLM API ( Evidence this is pre-existing:
Run LLxprt Review FailureThe automated LLxprt PR reviewer failed with: This is the same API timeout issue — the reviewer uses an LLM API call that timed out. This is an infrastructure failure, not a code issue. OCR (OpenCodeReview) FailureOCR failed with: SummaryAll code-related CI checks pass:
The 3 failing checks are all infrastructure/API-availability issues unrelated to this PR's code changes. |
Collaborator
Author
✅ Update: All CI Checks Now PassAfter the API key rotation, I re-ran the previously-failing checks. All CI checks now pass:
This confirms the earlier failures were caused by the exhausted/rate-limited API key (infrastructure), not by any code changes in this PR. The previously-failing E2E tests ( PR #2565 is fully green and ready for review/merge. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Comment on lines
+192
to
+194
| function isPositiveFiniteLimit(value: unknown): value is number { | ||
| return typeof value === 'number' && Number.isFinite(value) && value > 0; | ||
| } |
Contributor
There was a problem hiding this comment.
The DRY consolidation for context-limit precedence is correctly implemented: both
createAgentRuntimeContext.tsandcontextLimitResolver.tsnow delegate to the sharedresolveEffectiveContextLimitfunction instead of maintaining separate inline precedence logic. Tests intokenLimits.test.tscover the new helper comprehensively (user override, provider fallback, model lookup, invalid values like NaN/Infinity/0/negative), and the integration test inchatSession-isolation.integration.test.tswas updated to expect the correct 1M resolution forgemini-2.0-flash-expvia the new prefix entry rather than the old 1M default. No remaining hardcoded references to the previous1_048_576default were found in the searched TypeScript files. One minor readability improvement: consider renaming thevalueparameter inisPositiveFiniteLimittocandidateLimitto better reflect its domain-specific purpose.
acoliver
added a commit
that referenced
this pull request
Jul 15, 2026
* Make folder trust updates live (Fixes #637) * Harden live trust cleanup and review coverage * Resolve live trust review findings * Tighten trust transition race coverage * Surface live trust transition failures * Format merged MCP manager tests * Fix slash command trust event test mock * Harden trust lifecycle review findings * Complete trust lifecycle error handling * Contain live trust transition failures * Keep trust rollback and cleanup consistent * Extract live trust transition lifecycle * Close live trust authorization races * Harden final live trust lifecycle edges * Preserve complete trust failure reporting * Lower DEFAULT_TOKEN_LIMIT to 200K, add GLM catalog, consolidate resolution (Fixes #2527) (#2565) * Lower DEFAULT_TOKEN_LIMIT to 200K, add GLM catalog, consolidate resolution (Fixes #2527) Lower DEFAULT_TOKEN_LIMIT from 1,048,576 to 200,000 so unrecognized models get a fail-safe conservative context window instead of a dangerous 1M fallback that silently overruns sub-200K backends (#2270). Add GLM model family to the token-limit catalog: glm-5.x prefix -> 200K, glm-4.x prefix -> 128K, with exact entries for glm-5.2, glm-5.1, glm-5, glm-4.6, glm-4.5, and glm-4 (#2280). Add Gemini prefix entries (gemini-2.5-pro, gemini-2.5-flash, gemini-2.0-flash -> 1M) to prevent regressions from the lowered default for models that previously fell through to DEFAULT_TOKEN_LIMIT. Consolidate the two duplicated context-window resolution paths into a single shared resolveEffectiveContextLimit() function in tokenLimits.ts. Both getTokenLimitForConfiguredContext (agents layer) and ephemerals.contextLimit() (core runtime) now delegate to it, ensuring all consumers agree by construction (#2270 DRY item). * fix: replace nested ternary with if/else in test mocks (sonarjs/no-nested-conditional) * Close remaining live trust safety gaps * Raise unconfigured subagent turn fallback to 1000 (Fixes #2541) (#2566) * fix(agents): raise unconfigured subagent turn fallback * fix(agents): preserve foreground turn validation * test(agents): keep profile loader observable * Remove inherited Gemini names from neutral APIs (#2537) * Close final live trust lifecycle races * Harden live trust transition boundaries * Finish provider-neutral Gemini cleanup (#2573) (#2574) * Resolve final c90 review findings * Resolve latest a4 review findings * Resolve latest OCR review findings * Keep trust failure timers test-local * Resolve trust and MCP lifecycle review findings * Preserve provider quota errors across retry timeouts (Fixes #2559) (#2568) * Preserve provider quota errors across retry timeouts (Fixes #2559) * Use the public core error contract * Avoid idle-timeout cleanup deadlocks * Harden retry cleanup and request isolation * Strengthen retry contracts and behavioral coverage * Clarify timeout and structured error contracts * Bound load-balancer retries to request lifetimes * Stop cancelled failovers from mutating sessions * Use the public structured error contract * Close remaining retry lifecycle gaps * Expose retry orchestration through the provider API * Preserve failover and observation request boundaries * Preserve classifications at shared retry exhaustion * Harden trust and MCP review edge cases * Enforce the GenAI enclave boundary (Fixes #2352) (#2546) * Enforce the GenAI enclave boundary (Fixes #2352) * Preserve the published GenAI runtime bridge * Close remaining GenAI boundary bypasses * Tighten GenAI guard regression coverage * Harden GenAI guard diagnostics * Clarify GenAI packaging test types * Trim GenAI guard helper surface * Validate every GenAI dependency section * Close GenAI enclave scanner gaps * Keep boundary-test imports statically inspectable * Harden GenAI boundary against indirect bypasses * Resolve GenAI boundary review gaps * Close remaining GenAI boundary bypasses * Scope neutral GeminiContent naming exception * Close final GenAI scanner review gaps * Close provenance-based scanner bypasses * Preserve fake provider compatibility allowlist * Align GenAI guard with provider-neutral cleanup * Harden trust UI async lifecycle handling * Prevent stale trust commit UI updates * Close trust and MCP review races * Complete MCP cleanup after status failures * Reject unresolved directory trust decisions * Refresh genai import inventory
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
1 participant
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.
Summary
This PR addresses issue #2527 — the 0.11.0 follow-up / consolidation tracker for the context-window resolution gaps that produce a 1M fallback for unrecognized models.
Three changes
1. Lower
DEFAULT_TOKEN_LIMITfrom 1,048,576 to 200,000 (#2270)The overwhelmingly common real-world floor is 200K (Claude, many GLM/Gemini variants). Defaulting to 200K instead of 1M makes the unrecognized-model case fail-safe: proactive compression fires conservatively rather than silently overrunning a sub-200K backend.
2. Add GLM model family to the token-limit catalog (#2280)
Added exact entries and prefix-based matching for the GLM (z.ai) family:
glm-5.2,glm-5.1,glm-5and theglm-5prefix → 200Kglm-4.6,glm-4.5,glm-4and theglm-4prefix → 128KAlso added Gemini prefix entries (
gemini-2.5-pro,gemini-2.5-flash,gemini-2.0-flash→ 1M) to prevent regressions from the lowered default — these models previously fell through toDEFAULT_TOKEN_LIMITand would now incorrectly get 200K instead of their actual 1M window.3. Consolidate the two duplicated context-window resolution paths (#2270 DRY item)
Extracted a single
resolveEffectiveContextLimit(model, userContextLimit?, providerContextLimit?)function inpackages/core/src/core/tokenLimits.tsthat implements the three-tier precedence:context-limitoverridegetContextLimit()(e.g. load-balancer pool min)tokenLimit(model)Both consumers now delegate to it:
getTokenLimitForConfiguredContextinpackages/agents/src/core/contextLimitResolver.tsephemerals.contextLimit()inpackages/core/src/runtime/createAgentRuntimeContext.tsPreviously these two paths implemented the same precedence independently — a latent correctness gap where a future regression in one path would make the two diverge, and the
runtimeContext-based consumers would silently fall back to the (now 200K) default.Test changes
DEFAULT_TOKEN_LIMITconstant or the new 200K valueresolveEffectiveContextLimittest suite (precedence, NaN/Infinity rejection, non-positive values, combined-invalid fallback)vi.mockfactories to includeresolveEffectiveContextLimitwith validation matching the real implementation (addresses OCR finding: mock used??which accepts 0/NaN/Infinity, unlike realisPositiveFiniteLimit)MessageStreamOrchestrator.modelinfo.test.tsassertion to checkresolveEffectiveContextLimitcalls (code now uses shared function instead oftokenLimitdirectly)chatSession.contextlimit.test.tsassertions (spy can't intercept same-module internal calls fromresolveEffectiveContextLimittotokenLimit)Pre-existing failures (not caused by this PR)
packages/agents/src/api/__tests__/publicSurface.guard.test.ts— snapshot comparison fails onmaintoo (4 removed exports:AgentActivationPreflightResult,ActivationPreflightToken,AgentSessionStartResult,preflightAgentActivation)packages/core/src/debug/ConfigurationManager.test.ts(2 failures),FileOutput.test.ts(1),policy/shell-safety.test.ts(4) — all pre-existing onmainpackages/mcp/— pre-existing, unrelated to token limitsRelated issues
Fixes #2527