✨(back) limit output token per message#458
Conversation
|
Warning Review limit reached
Next review available in: 35 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: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
WalkthroughThis PR adds a configurable per-message LLM output token limit. The backend passes the limit to model execution, detects length-based truncation, and propagates annotations to streamed and persisted messages. The frontend renders a localized truncation notice. ChangesOutput Token Limit Feature
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
d8924a2 to
fbabc9b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/backend/chat/tests/clients/pydantic_ai/test_output_token_limit.py`:
- Around line 20-29: The fixture base_settings is forcing
LLM_MAX_OUTPUT_TOKENS_PER_MESSAGE to 8192 for every test, making the "default is
8192" assertions tautological; remove or stop setting
settings.LLM_MAX_OUTPUT_TOKENS_PER_MESSAGE in the base_settings fixture and
instead set that value only in tests that need it (or create a separate fixture
for tests that require a non-default), and update the tests that assert the
default (the "default is 8192" test and the other assertions flagged) to rely on
the real unset default value so they can detect regressions; locate references
to settings.LLM_MAX_OUTPUT_TOKENS_PER_MESSAGE and the pytest fixture
base_settings to implement this change.
In `@src/backend/conversations/settings.py`:
- Around line 681-685: ConversationsSettings defines
LLM_MAX_OUTPUT_TOKENS_PER_MESSAGE but doesn't validate it; add a fail-fast check
in the class's post_setup method that reads
self.LLM_MAX_OUTPUT_TOKENS_PER_MESSAGE and raises a ValueError (or logger +
sys.exit) if the value is <= 0, with a clear message identifying
LLM_MAX_OUTPUT_TOKENS_PER_MESSAGE so startup fails early and prevents invalid
token limits from propagating into generation settings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a3b26a1d-96d2-470e-9953-ddf14d477e3f
📒 Files selected for processing (9)
CHANGELOG.mddocs/env.mdsrc/backend/chat/clients/pydantic_ai.pysrc/backend/chat/tests/clients/pydantic_ai/test_output_token_limit.pysrc/backend/chat/tests/test_ai_agent_service_co2.pysrc/backend/conversations/settings.pysrc/frontend/apps/conversations/src/features/chat/components/MessageItem.tsxsrc/frontend/apps/conversations/src/features/chat/components/TruncatedResponseMessage.tsxsrc/frontend/apps/conversations/src/features/chat/components/__tests__/MessageItem.test.tsx
fbabc9b to
b16bf5c
Compare
|
f911021 to
05d0882
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/backend/chat/tests/clients/pydantic_ai/test_output_token_limit.py (1)
21-29: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThe default value test is still tautological.
The previous review flagged that forcing the setting in the
base_settingsfixture masks the actual default. Instead of removing it from the fixture, the value was simply changed to1000in both the fixture and the test assertion. This means the test is still just verifying that the fixture successfully overrode the value, rather than ensuring the application's actual default is correctly set to8192.Please remove the override from the fixture and assert the true default value.
🐛 Proposed fix
`@pytest.fixture`(autouse=True) def base_settings(settings): """Set up base settings for all tests in this module.""" settings.AI_BASE_URL = "https://api.llm.com/v1/" settings.AI_API_KEY = "test-key" settings.AI_MODEL = "model-123" settings.AI_AGENT_INSTRUCTIONS = "You are a helpful assistant" settings.AI_AGENT_TOOLS = [] - settings.LLM_MAX_OUTPUT_TOKENS_PER_MESSAGE = 1000 # --------------------------------------------------------------------------- # Fixtures # --------------------------------------------------------------------------- ... def test_llm_max_output_tokens_per_message_setting_exists(): """Setting must exist with a positive integer value.""" assert hasattr(django_settings, "LLM_MAX_OUTPUT_TOKENS_PER_MESSAGE") assert isinstance(django_settings.LLM_MAX_OUTPUT_TOKENS_PER_MESSAGE, int) - assert django_settings.LLM_MAX_OUTPUT_TOKENS_PER_MESSAGE == 1000 + assert django_settings.LLM_MAX_OUTPUT_TOKENS_PER_MESSAGE == 8192Also applies to: 80-84
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/chat/tests/clients/pydantic_ai/test_output_token_limit.py` around lines 21 - 29, Remove the LLM_MAX_OUTPUT_TOKENS_PER_MESSAGE assignment from the base_settings fixture, and update the related default-value test to assert the application's actual default of 8192. Keep the fixture overrides for unrelated settings unchanged and ensure the test reads the unmodified setting default.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/backend/chat/tests/clients/pydantic_ai/test_output_token_limit.py`:
- Around line 114-117: Update test_run_agent_passes_max_tokens_model_settings to
explicitly set settings.LLM_MAX_OUTPUT_TOKENS_PER_MESSAGE to 1000 before
invoking the agent, ensuring the test uses the mocked token limit rather than
the default 8192 value.
---
Duplicate comments:
In `@src/backend/chat/tests/clients/pydantic_ai/test_output_token_limit.py`:
- Around line 21-29: Remove the LLM_MAX_OUTPUT_TOKENS_PER_MESSAGE assignment
from the base_settings fixture, and update the related default-value test to
assert the application's actual default of 8192. Keep the fixture overrides for
unrelated settings unchanged and ensure the test reads the unmodified setting
default.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2cd1308d-c0d5-4b3b-a8b6-37978b4e31f8
📒 Files selected for processing (11)
CHANGELOG.mddocs/env.mdsrc/backend/chat/agents/base.pysrc/backend/chat/clients/pydantic_ai.pysrc/backend/chat/tests/agents/test_base_agent.pysrc/backend/chat/tests/clients/pydantic_ai/test_output_token_limit.pysrc/backend/chat/tests/test_ai_agent_service_co2.pysrc/backend/conversations/settings.pysrc/frontend/apps/conversations/src/features/chat/components/MessageItem.tsxsrc/frontend/apps/conversations/src/features/chat/components/TruncatedResponseMessage.tsxsrc/frontend/apps/conversations/src/features/chat/components/__tests__/MessageItem.test.tsx
🚧 Files skipped from review as they are similar to previous changes (7)
- docs/env.md
- src/backend/chat/tests/test_ai_agent_service_co2.py
- src/backend/conversations/settings.py
- src/frontend/apps/conversations/src/features/chat/components/MessageItem.tsx
- src/frontend/apps/conversations/src/features/chat/components/tests/MessageItem.test.tsx
- src/frontend/apps/conversations/src/features/chat/components/TruncatedResponseMessage.tsx
- src/backend/chat/clients/pydantic_ai.py
| @pytest.mark.asyncio | ||
| async def test_run_agent_passes_max_tokens_model_settings(ui_messages): | ||
| """_run_agent must pass | ||
| ModelSettings(max_tokens=LLM_MAX_OUTPUT_TOKENS_PER_MESSAGE) to agent.iter().""" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Explicitly configure the token limit for this test.
Once the global override is removed from base_settings, this test needs to explicitly set settings.LLM_MAX_OUTPUT_TOKENS_PER_MESSAGE = 1000 to ensure it continues to pass with the mocked value instead of falling back to the 8192 default.
🐛 Proposed fix
`@pytest.mark.asyncio`
-async def test_run_agent_passes_max_tokens_model_settings(ui_messages):
+async def test_run_agent_passes_max_tokens_model_settings(settings, ui_messages):
"""_run_agent must pass
ModelSettings(max_tokens=LLM_MAX_OUTPUT_TOKENS_PER_MESSAGE) to agent.iter()."""
+ settings.LLM_MAX_OUTPUT_TOKENS_PER_MESSAGE = 1000
conversation = await sync_to_async(ChatConversationFactory)()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @pytest.mark.asyncio | |
| async def test_run_agent_passes_max_tokens_model_settings(ui_messages): | |
| """_run_agent must pass | |
| ModelSettings(max_tokens=LLM_MAX_OUTPUT_TOKENS_PER_MESSAGE) to agent.iter().""" | |
| `@pytest.mark.asyncio` | |
| async def test_run_agent_passes_max_tokens_model_settings(settings, ui_messages): | |
| """_run_agent must pass | |
| ModelSettings(max_tokens=LLM_MAX_OUTPUT_TOKENS_PER_MESSAGE) to agent.iter().""" | |
| settings.LLM_MAX_OUTPUT_TOKENS_PER_MESSAGE = 1000 |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/backend/chat/tests/clients/pydantic_ai/test_output_token_limit.py` around
lines 114 - 117, Update test_run_agent_passes_max_tokens_model_settings to
explicitly set settings.LLM_MAX_OUTPUT_TOKENS_PER_MESSAGE to 1000 before
invoking the agent, ensuring the test uses the mocked token limit rather than
the default 8192 value.
If limit is reached display a message to the user. It's meant to limit cost of very long messages, so the limit must be high.
05d0882 to
e3466d5
Compare
|
1 participant
Purpose
Adds a configurable output token limit per AI message to control LLM costs on very long responses. When the limit is reached, the backend stops generation and the frontend displays an error message to
the user.
Proposal
Backend
Frontend
Demo
Screen.Recording.2026-07-14.at.09.34.34.mov
Summary by CodeRabbit
Summary by CodeRabbit
Release Notes
New Features
Documentation
LLM_MAX_OUTPUT_TOKENS_PER_MESSAGEfor configuring the per-message maximum output tokens and its default/truncation behavior