fix: remove hardcoded 48-character limit from text inputs#18
Conversation
WalkthroughThis pull request removes the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/app/components/share/text-generation/index.tsx (1)
193-200: Remove or guard the debug console.log.
This runs in production clients and can add noise (and potentially expose data in captured logs).🧹 Suggested guard
- console.log('Checking batch inputs:', { dataLength: data.length, headerData }) + if (process.env.NODE_ENV === 'development') + console.log('Checking batch inputs:', { dataLength: data.length, headerData })
🧹 Nitpick comments (2)
web/utils/var.ts (1)
32-40: Limit template field propagation for non-text inputs.
Spreading the entire template risks inheriting text-only fields if the template grows later. Consider explicitly selecting base fields for non-textInput types.♻️ Proposed refactor
-export const getNewVarInWorkflow = (key: string, type = InputVarType.textInput): InputVar => { - const { ...rest } = VAR_ITEM_TEMPLATE_IN_WORKFLOW - if (type !== InputVarType.textInput) { - return { - ...rest, - type, - variable: key, - label: key.slice(0, getMaxVarNameLength(key)), - } - } +export const getNewVarInWorkflow = (key: string, type = InputVarType.textInput): InputVar => { + const { variable, label, required, options } = VAR_ITEM_TEMPLATE_IN_WORKFLOW + const base = { variable, label, required, options } + if (type !== InputVarType.textInput) { + return { + ...base, + type, + variable: key, + label: key.slice(0, getMaxVarNameLength(key)), + } + } return { ...VAR_ITEM_TEMPLATE_IN_WORKFLOW, type, variable: key, label: key.slice(0, getMaxVarNameLength(key)), placeholder: '', default: '', hint: '', } }web/app/components/app/configuration/prompt-value-panel/index.tsx (1)
138-174: Confirm numeric UX after switching to type="text".
If the field is still intended to be numeric-only, consider restoring numeric keyboard and lightweight validation via inputMode/pattern (or verify downstream validation handles non-numeric input).✅ Suggested tweak (if numeric-only)
- {type === 'number' && ( - <Input - type="text" + {type === 'number' && ( + <Input + type="text" + inputMode="numeric" + pattern="\d*" value={inputs[key] ? `${inputs[key]}` : ''} onChange={(e) => { handleInputValueChange(key, e.target.value) }} placeholder={name} autoFocus={index === 0} maxLength={max_length} /> )}
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
web/app/components/app/configuration/config-var/config-modal/index.tsxweb/app/components/app/configuration/config-var/index.tsxweb/app/components/app/configuration/debug/chat-user-input.tsxweb/app/components/app/configuration/prompt-value-panel/index.tsxweb/app/components/rag-pipeline/components/panel/input-field/editor/form/hooks.tsweb/app/components/rag-pipeline/components/panel/input-field/editor/form/index.spec.tsxweb/app/components/share/text-generation/index.tsxweb/app/components/share/text-generation/run-once/index.spec.tsxweb/app/components/share/text-generation/run-once/index.tsxweb/config/index.tsweb/utils/var.ts
💤 Files with no reviewable changes (5)
- web/app/components/app/configuration/config-var/config-modal/index.tsx
- web/config/index.ts
- web/app/components/app/configuration/config-var/index.tsx
- web/app/components/rag-pipeline/components/panel/input-field/editor/form/index.spec.tsx
- web/app/components/rag-pipeline/components/panel/input-field/editor/form/hooks.ts
🧰 Additional context used
🧬 Code graph analysis (2)
web/app/components/share/text-generation/run-once/index.spec.tsx (1)
web/models/debug.ts (1)
PromptConfig(79-82)
web/utils/var.ts (1)
web/config/index.ts (1)
VAR_ITEM_TEMPLATE_IN_WORKFLOW(214-220)
🔇 Additional comments (5)
web/app/components/share/text-generation/run-once/index.tsx (1)
137-143: LGTM — maxLength now respects per-variable configuration.
This aligns with removing the global fallback and lets undefined mean “no limit.”web/app/components/share/text-generation/run-once/index.spec.tsx (1)
239-280: Tests look solid for maxLength behavior.
Good coverage for both undefined and explicit max_length cases.web/app/components/app/configuration/debug/chat-user-input.tsx (1)
84-119: LGTM — maxLength now directly reflects max_length.
Consistent with the new “no fallback” behavior.web/app/components/share/text-generation/index.tsx (2)
28-30: LGTM on the config import cleanup.
No issues with this update.
257-264: LGTM — max_length now directly controls batch length validation.
Behavior is consistent with the removal of the default length fallback.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
1 participant
Benchmark PR from qodo-benchmark#417
Summary by CodeRabbit
Bug Fixes
Changes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.