-
Notifications
You must be signed in to change notification settings - Fork 0
fix: remove hardcoded 48-character limit from text inputs #31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: coderabbit_full_base_fix_remove_hardcoded_48-character_limit_from_text_inputs_pr5
Are you sure you want to change the base?
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.