fix: correct duplicate gpt-4o-mini key in MODEL_WINDOWS to gpt-4o#1115
fix: correct duplicate gpt-4o-mini key in MODEL_WINDOWS to gpt-4o#1115jnMetaCode wants to merge 1 commit intoMervinPraison:mainfrom
Conversation
MODEL_WINDOWS had two entries for gpt-4o-mini and none for gpt-4o. The first entry was clearly intended to be gpt-4o based on context. This meant gpt-4o users got no context window match, potentially causing suboptimal chunking or fallback behavior. Signed-off-by: JiangNan <1394485448@qq.com>
Review Summary by QodoFix duplicate gpt-4o-mini key in MODEL_WINDOWS
WalkthroughsDescription• Fixed duplicate key in MODEL_WINDOWS dictionary • Changed first gpt-4o-mini entry to gpt-4o • Ensures gpt-4o model gets correct 96000 token window • Prevents fallback behavior for gpt-4o users Diagramflowchart LR
A["MODEL_WINDOWS<br/>duplicate keys"] -- "remove duplicate<br/>gpt-4o-mini" --> B["MODEL_WINDOWS<br/>corrected entries"]
B -- "gpt-4o: 96000" --> C["gpt-4o gets<br/>correct window"]
B -- "gpt-4o-mini: 96000" --> D["gpt-4o-mini<br/>preserved"]
File Changes1. src/praisonai-agents/praisonaiagents/llm/llm.py
|
Code Review by Qodo
1. Provider-prefixed model mismatch
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a configuration error within the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Code Review
This pull request corrects a typo in the MODEL_WINDOWS dictionary where a duplicate gpt-4o-mini key was used instead of gpt-4o. This fix ensures that the gpt-4o model is assigned the correct context window size, resolving the bug described. The change is accurate and addresses the issue effectively. I have no further feedback on the change itself.
As a side note, while reviewing this dictionary, I noticed that some other model context window sizes might be outdated (e.g., claude-3-5-sonnet). It would be beneficial to review all values in MODEL_WINDOWS in a separate effort to ensure they are up-to-date.
Note: Security Review did not run due to the size of the PR.
| "gpt-4o": 96000, # 128,000 actual | ||
| "gpt-4o-mini": 96000, # 128,000 actual |
There was a problem hiding this comment.
1. Provider-prefixed model mismatch 🐞 Bug ✓ Correctness
When using the LiteLLM path (provider/model strings like openai/gpt-4o), MODEL_WINDOWS keys (e.g., gpt-4o) will not match because get_context_size() uses self.model.startswith(model_prefix) without normalizing the provider prefix. This means the newly-added gpt-4o entry can still fall through to the 4000-token default, so the PR may not fix the reported behavior for LiteLLM usage.
Agent Prompt
### Issue description
`LLM.get_context_size()` uses `self.model.startswith(model_prefix)` against `MODEL_WINDOWS` keys that are *not* provider-prefixed. For LiteLLM usage where `self.model` commonly looks like `openai/gpt-4o`, no entries match and the method returns the 4000-token fallback.
### Issue Context
- Agent passes provider/model strings directly into `LLM(model=...)` when the model contains `/`.
- `MODEL_WINDOWS` contains keys like `gpt-4o`, `gpt-4o-mini`, etc.
### Fix Focus Areas
- src/praisonai-agents/praisonaiagents/llm/llm.py[3793-3798]
- src/praisonai-agents/praisonaiagents/llm/llm.py[106-114]
### Suggested implementation notes
- In `get_context_size()`, compute a normalized model id:
- `raw = self.model.lower().strip()`
- `name = raw.split('/', 1)[-1]` (strip provider prefix)
- Do exact match first: `if name in self.MODEL_WINDOWS: return ...`
- Then do prefix match (prefer longest prefix; see next finding) and finally fallback.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/praisonai-agents/praisonaiagents/llm/llm.py`:
- Around line 107-111: The MODEL_WINDOWS lookup and the startswith check in
get_context_size assume bare model ids but callers may pass provider-qualified
ids like "openai/gpt-4o"; update get_context_size to normalize self.model (e.g.,
strip provider prefixes before using MODEL_WINDOWS and before the
self.model.startswith(model_prefix) checks) so entries in MODEL_WINDOWS are
matched for provider-prefixed ids; reference MODEL_WINDOWS, get_context_size and
the self.model.startswith(model_prefix) usage when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 51e814d5-ef0e-4243-86ba-6a205ca1b1ed
📒 Files selected for processing (1)
src/praisonai-agents/praisonaiagents/llm/llm.py
| MODEL_WINDOWS = { | ||
| # OpenAI | ||
| "gpt-4": 6144, # 8,192 actual | ||
| "gpt-4o-mini": 96000, # 128,000 actual | ||
| "gpt-4o": 96000, # 128,000 actual | ||
| "gpt-4o-mini": 96000, # 128,000 actual |
There was a problem hiding this comment.
Normalize provider-qualified model names before relying on this entry.
This fixes bare "gpt-4o", but Line 3795 still matches with self.model.startswith(model_prefix). If callers pass LiteLLM-style ids like openai/gpt-4o or openai/gpt-4o-mini—which this class already acknowledges elsewhere—this table is skipped and get_context_size() falls back to 4000. That leaves the context-window bug in place for provider-prefixed models.
💡 Proposed fix
def get_context_size(self) -> int:
"""Get safe input size limit for this model"""
- for model_prefix, size in self.MODEL_WINDOWS.items():
- if self.model.startswith(model_prefix):
+ normalized_model = self.model.split("/", 1)[-1] if self.model else ""
+ for model_prefix, size in self.MODEL_WINDOWS.items():
+ if normalized_model.startswith(model_prefix):
return size
return 4000 # Safe default🧰 Tools
🪛 Ruff (0.15.4)
[warning] 107-147: Mutable default value for class attribute
(RUF012)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/praisonai-agents/praisonaiagents/llm/llm.py` around lines 107 - 111, The
MODEL_WINDOWS lookup and the startswith check in get_context_size assume bare
model ids but callers may pass provider-qualified ids like "openai/gpt-4o";
update get_context_size to normalize self.model (e.g., strip provider prefixes
before using MODEL_WINDOWS and before the self.model.startswith(model_prefix)
checks) so entries in MODEL_WINDOWS are matched for provider-prefixed ids;
reference MODEL_WINDOWS, get_context_size and the
self.model.startswith(model_prefix) usage when making the change.
Problem
The
MODEL_WINDOWSdictionary inllm.pyhas a duplicate key bug:Python dictionaries silently discard duplicate keys, keeping only the last value. The first entry was clearly intended to be
"gpt-4o"(the full model, not the mini variant), but it was typed as"gpt-4o-mini"by mistake.As a result, there is no context window entry for
gpt-4o, so any agent using that model falls through to the default/fallback window size instead of getting the correct 96,000 token window.Fix
Changed the first entry from
"gpt-4o-mini"to"gpt-4o"so both models are properly represented in the lookup table.How I found it
Noticed it while reviewing the model configuration — the comment structure and surrounding entries make it clear the intent was to list
gpt-4ofollowed bygpt-4o-mini, matching the pattern used for other model families.Summary by CodeRabbit
New Features