fix: remove hardcoded 48-character limit from text inputs#31
Conversation
WalkthroughRemove global default max length constant and its usage across components, configuration templates, and tests. Components now rely solely on explicit max_length values without fallback defaults. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
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/app/configuration/prompt-value-panel/index.tsx (1)
138-173: Add numeric validation for the number input field.The number input uses
type="text"without enforcing numeric format, allowing non-numeric characters to be entered and stored. If numeric-only values are required, add validation in theonChangehandler or useinputMode="numeric"combined with input filtering.
🤖 Fix all issues with AI agents
In `@web/app/components/share/text-generation/index.tsx`:
- Line 199: The console.log call that prints "Checking batch inputs:" is leaking
user-provided data; remove it or gate it behind a development-only check (e.g.,
only run when NODE_ENV === 'development' or an isDev flag) or replace it with a
debug-level logger that is disabled in production, ensuring no user data is
emitted in production from the text-generation component where the console.log
currently appears.
📜 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/config/index.ts
- web/app/components/rag-pipeline/components/panel/input-field/editor/form/index.spec.tsx
- web/app/components/app/configuration/config-var/index.tsx
- web/app/components/app/configuration/config-var/config-modal/index.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 (6)
web/app/components/app/configuration/debug/chat-user-input.tsx (1)
84-119: MaxLength now correctly follows per-variable config.Looks good—maxLength is only applied when max_length is explicitly provided.
web/app/components/share/text-generation/index.tsx (2)
29-29: No concerns with this import adjustment.
261-264: Max length validation now aligns with explicit config.Direct use of
max_lengthmatches the new behavior and keeps validation consistent.web/utils/var.ts (1)
33-33: LGTM: template fields are preserved for non-textInput vars.This keeps
max_length(and other fields) intact when needed.web/app/components/share/text-generation/run-once/index.spec.tsx (1)
240-279: Nice coverage for maxLength behavior.These tests capture the new “only when provided” behavior clearly.
web/app/components/share/text-generation/run-once/index.tsx (1)
137-143: MaxLength now correctly tied to per-variable config.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| return false | ||
| } | ||
| const headerData = data[0] | ||
| console.log('Checking batch inputs:', { dataLength: data.length, headerData }) |
There was a problem hiding this comment.
Remove or gate debug logging before release.
This logs user-provided data to the console; it’s better to remove it or guard it for development only.
🧹 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 })📝 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.
| console.log('Checking batch inputs:', { dataLength: data.length, headerData }) | |
| if (process.env.NODE_ENV === 'development') | |
| console.log('Checking batch inputs:', { dataLength: data.length, headerData }) |
🤖 Prompt for AI Agents
In `@web/app/components/share/text-generation/index.tsx` at line 199, The
console.log call that prints "Checking batch inputs:" is leaking user-provided
data; remove it or gate it behind a development-only check (e.g., only run when
NODE_ENV === 'development' or an isDev flag) or replace it with a debug-level
logger that is disabled in production, ensuring no user data is emitted in
production from the text-generation component where the console.log currently
appears.
Benchmark PR from agentic-review-benchmarks#5
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.