feat(llmConfigs): surface max output tokens as its own list column - #2234
Open
710leo wants to merge 1 commit into
Open
feat(llmConfigs): surface max output tokens as its own list column#2234710leo wants to merge 1 commit into
710leo wants to merge 1 commit into
Conversation
The list only showed context length, which reads as the knob that caps a reply. Add a max output tokens column next to it (blank renders as "auto") and reword both tips so a truncated answer points at max_tokens instead.
📝 WalkthroughWalkthroughChangesLLM token display
Estimated code review effort: 2 (Simple) | ~5 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![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/pages/aiConfig/llmConfigs/locale/en_US.ts`:
- Around line 50-52: Update the context_length_tip translation in
src/pages/aiConfig/llmConfigs/locale/en_US.ts lines 50-52, ja_JP.ts lines 49-51,
ru_RU.ts lines 50-52, zh_CN.ts lines 50-51, and zh_HK.ts lines 49-50 to preserve
the distinction between the total context window and per-response Max Output
Tokens while stating that conversation history can consume the context budget
and reduce the output tokens available to the model.
In `@src/pages/aiConfig/llmConfigs/pages/List.tsx`:
- Around line 161-164: Update the max_tokens column render in List.tsx to check
explicitly for null or undefined rather than using truthiness, so an explicit
value of 0 is formatted with formatContextLength while only the auto/empty state
displays form.max_tokens_auto.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a1f840dc-9dfb-46e4-ad8f-7f6e98ff53c3
📒 Files selected for processing (6)
src/pages/aiConfig/llmConfigs/locale/en_US.tssrc/pages/aiConfig/llmConfigs/locale/ja_JP.tssrc/pages/aiConfig/llmConfigs/locale/ru_RU.tssrc/pages/aiConfig/llmConfigs/locale/zh_CN.tssrc/pages/aiConfig/llmConfigs/locale/zh_HK.tssrc/pages/aiConfig/llmConfigs/pages/List.tsx
Comment on lines
50
to
+52
| context_length: 'Context Length', | ||
| context_length_tip: 'Maximum context window size supported by the model (in tokens)', | ||
| context_length_tip: | ||
| 'Maximum context window supported by the model (in tokens); controls how much conversation history is carried. It does NOT affect the length of a single answer — raise "Max Output Tokens" for that.', |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Qualify the context-length guidance in all locales.
context_length limits the total context budget. Large conversation history can reduce the output tokens available to the model. The current text says context length never affects a single answer, so users may increase max_tokens when the context window is the actual limit. Keep the distinction between the per-response cap and the total context window, but add this caveat in every translation.
src/pages/aiConfig/llmConfigs/locale/en_US.ts#L50-L52: Clarify that context length can limit available output when history consumes the window.src/pages/aiConfig/llmConfigs/locale/ja_JP.ts#L49-L51: Add the same context-window caveat in Japanese.src/pages/aiConfig/llmConfigs/locale/ru_RU.ts#L50-L52: Add the same context-window caveat in Russian.src/pages/aiConfig/llmConfigs/locale/zh_CN.ts#L50-L51: Add the same context-window caveat in Simplified Chinese.src/pages/aiConfig/llmConfigs/locale/zh_HK.ts#L49-L50: Add the same context-window caveat in Traditional Chinese.
📍 Affects 5 files
src/pages/aiConfig/llmConfigs/locale/en_US.ts#L50-L52(this comment)src/pages/aiConfig/llmConfigs/locale/ja_JP.ts#L49-L51src/pages/aiConfig/llmConfigs/locale/ru_RU.ts#L50-L52src/pages/aiConfig/llmConfigs/locale/zh_CN.ts#L50-L51src/pages/aiConfig/llmConfigs/locale/zh_HK.ts#L49-L50
🤖 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/pages/aiConfig/llmConfigs/locale/en_US.ts` around lines 50 - 52, Update
the context_length_tip translation in
src/pages/aiConfig/llmConfigs/locale/en_US.ts lines 50-52, ja_JP.ts lines 49-51,
ru_RU.ts lines 50-52, zh_CN.ts lines 50-51, and zh_HK.ts lines 49-50 to preserve
the distinction between the total context window and per-response Max Output
Tokens while stating that conversation history can consume the context budget
and reduce the output tokens available to the model.
Comment on lines
+161
to
+164
| dataIndex: ['extra_config', 'max_tokens'], | ||
| title: t('form.max_tokens'), | ||
| width: 110, | ||
| render: (val) => (val ? formatContextLength(val).compact : <span className='text-soft'>{t('form.max_tokens_auto')}</span>), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 5 '\bmax_tokens\b|extra_config' --glob '!node_modules/**' --glob '!dist/**' .Repository: n9e/fe
Length of output: 50362
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Inspect the AI config list type/data shape and nearby list rendering.
wc -l src/pages/aiConfig/llmConfigs/pages/List.tsx
sed -n '1,220p' src/pages/aiConfig/llmConfigs/pages/List.tsx
# Find LLM config model/type definitions specifically.
rg -n -C 3 'interface .*LLM|type .*LLM|extra_config|LLM' src/pages/aiConfig/llmConfigs src --glob '!**/locale/**' --glob '!node_modules/**' --glob '!dist/**' | head -n 240Repository: n9e/fe
Length of output: 28383
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Inspect the shared LLM item/form value model relevant to extra_config.max_tokens.
sed -n '1,90p' src/pages/aiConfig/llmConfigs/types.ts
# Inspect the form item declaration and placeholder handling for max_tokens.
sed -n '140,160p' src/pages/aiConfig/llmConfigs/pages/Form.tsx
# Run a minimal read-only probe of the current renderer’s falsey behavior.
python3 - <<'PY'
values = [None, 0, "", [], {}, False, 1, 100]
print("Current renderer truthiness result:")
for val in values:
print(repr(val), "=>", "num" if val else "auto")
PYRepository: n9e/fe
Length of output: 2591
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Inspect the complete form item declaration and any normalized submit/read behavior around llm config fields.
sed -n '1,220p' src/pages/aiConfig/llmConfigs/pages/Form.tsx
rg -n -C 4 'max_tokens|normalize|adjustSubmitValues|extra_config' src/pages/aiConfig/llmConfigs --glob '!**/locale/**' --glob '!node_modules/**' --glob '!dist/**'Repository: n9e/fe
Length of output: 26344
Use an explicit empty-value check for max_tokens.
extra_config.max_tokens is typed as number | undefined. The form allows explicit zero (InputNumber min={0}), so the current (val ? ...) render treats that as form.max_tokens_auto. Use val == null or the documented sentinel when auto is the only empty state.
🤖 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/pages/aiConfig/llmConfigs/pages/List.tsx` around lines 161 - 164, Update
the max_tokens column render in List.tsx to check explicitly for null or
undefined rather than using truthiness, so an explicit value of 0 is formatted
with formatContextLength while only the auto/empty state displays
form.max_tokens_auto.
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.
The LLM config list only showed context length, which reads as the knob that caps a reply — so a truncated answer sends people to the wrong field.
auto(text-soft).max_tokens/context_lengthtips in all five locales so it's clear which one to raise when a reply is cut off.Locale keys added:
form.max_tokens_auto(en_US / zh_CN / zh_HK / ja_JP / ru_RU).Summary by CodeRabbit