fix(discovery): persist provider token ceilings and context windows - #952
Conversation
|
Warning Review limit reachedNext included review available in 15 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthrough모델 검색 결과에 Changes모델 한도 검색 및 합의
에이전트 계약, 저장소 및 요청 제한
CLI 보고 및 통합 검증
Estimated code review effort: 4 (Complex) | ~60 minutes Merge Risk: 🟡 Moderate · up to The PR persists provider limits and uses them to constrain outgoing token budgets, but invalid discovered values can retain stale ceilings and gateway context metadata can bypass deployment-consensus checks. These bounded correctness issues may cause incorrect request limits in production, so merge should wait for fixes or explicit owner acceptance. Sequence Diagram(s)sequenceDiagram
participant ModelProvider as Model provider
participant ModelDiscovery as model_discovery
participant AgentPool as agent_pool
participant ModelClient
participant ProviderAPI as Provider API
ModelProvider->>ModelDiscovery: model limit metadata
ModelDiscovery->>ModelDiscovery: validate and merge metadata
ModelDiscovery->>AgentPool: persist ModelAgent limits
AgentPool-->>ModelClient: load max_output_tokens
ModelClient->>ProviderAPI: clamp outgoing token budget
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 44.93% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 69 functions across 12 files. (3 skipped: 2 unsupported, 1 too large.) ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Cross-PR integration contract: routing identity is provider-neutral |
| context_window=context_window, | ||
| max_output_tokens_conflicted=( | ||
| row.get("_max_output_tokens_conflicted") is True | ||
| or any(value is None for value in output_limits) |
There was a problem hiding this comment.
🟡 Missing enrichment clears saved limits
When Models.dev omits a limit, _merge_models_dev_metadata inserts None, which becomes a conflict. A successful refresh then deletes the saved limit.
Prompt for agents
Distinguish absent Models.dev limit metadata from explicitly invalid provider metadata. _merge_models_dev_metadata currently always writes max_output_tokens and context_window, using None when limit.output or limit.context is absent; _parse_openai_compatible now interprets any present invalid value as a conflict, and provider catalog refreshes use that conflict to clear persisted limits. Preserve the conflict signal for explicit invalid values, but omit candidate keys when the upstream source supplied no value. Add coverage for a previously saved limit followed by Models.dev metadata that omits one or both limits, alongside the existing explicit zero/negative invalid-metadata cases.
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
Testing
platform darwin -- Python 3.13.14, pytest-9.0.3, pluggy-1.6.0
rootdir: /Users/seonghobae/Documents/ChatGPT/contextual-orchestrator
configfile: pyproject.toml
plugins: cov-7.1.0, anyio-4.14.1, hypothesis-6.165.10
collected 0 items
============================ no tests ran in 0.37s =============================
platform darwin -- Python 3.13.14, pytest-9.0.3, pluggy-1.6.0
rootdir: /Users/seonghobae/Documents/ChatGPT/contextual-orchestrator
configfile: pyproject.toml
plugins: cov-7.1.0, anyio-4.14.1, hypothesis-6.165.10
collected 4 items
tests/test_api_contract.py .... [100%]
============================== 4 passed in 0.66s ===============================
Closes #927.
Summary by CodeRabbit