Skip to content

Conversation

@tomerqodo
Copy link

@tomerqodo tomerqodo commented Jan 29, 2026

Benchmark PR from agentic-review-benchmarks#5

Summary by CodeRabbit

  • Bug Fixes

    • Removed automatic character limit enforcement on text and paragraph input fields; maximum length is now only applied when explicitly configured.
  • Tests

    • Added test coverage validating maxLength behavior for inputs with and without configured limits.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 29, 2026

Walkthrough

Remove 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

Cohort / File(s) Summary
Configuration Components - Max Length Removal
web/app/components/app/configuration/config-var/config-modal/index.tsx, web/app/components/app/configuration/config-var/index.tsx, web/app/components/app/configuration/debug/chat-user-input.tsx
Removed DEFAULT_VALUE_MAX_LEN import and fallback logic; components now rely on explicit max_length without defaults. Paragraph type in config-modal no longer auto-sets max_length.
Prompt Value & Text Generation Components
web/app/components/app/configuration/prompt-value-panel/index.tsx, web/app/components/share/text-generation/index.tsx, web/app/components/share/text-generation/run-once/index.tsx
Removed DEFAULT_VALUE_MAX_LEN fallback; prompt-value-panel changes number input type from "number" to "text" and relies on direct max_length assignment. Text generation components use max_length without defaults.
RAG Pipeline Input Field Editor
web/app/components/rag-pipeline/components/panel/input-field/editor/form/hooks.ts
Removed DEFAULT_VALUE_MAX_LEN import and paragraph type max_length assignment logic; only multiFiles type now receives maxLength via file upload settings.
Configuration Templates
web/config/index.ts
Removed max_length property from VAR_ITEM_TEMPLATE, VAR_ITEM_TEMPLATE_IN_WORKFLOW, and VAR_ITEM_TEMPLATE_IN_PIPELINE exported templates.
Utility Functions
web/utils/var.ts
Modified getNewVarInWorkflow to preserve max_length from template instead of explicitly excluding it for non-textInput branches.
Test Updates
web/app/components/rag-pipeline/components/panel/input-field/editor/form/index.spec.tsx, web/app/components/share/text-generation/run-once/index.spec.tsx
Removed test validating paragraph type maxLength assignment; added new test group verifying maxLength behavior is conditional on max_length prop presence.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 No more defaults in sight, we hop so free,
Max length now explicit, as it ought to be!
From config to components, we've trimmed the fat,
Each input stands tall—no fallback safety mat.
The rabbit approves! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is a minimal reference to a benchmark PR with no substantive information about the changes, objectives, motivation, or impact. Provide a detailed summary of the changes, explain the motivation for removing the hardcoded limit, include before/after screenshots if applicable, and verify all required checklist items are addressed.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: removing a hardcoded 48-character limit from text inputs across multiple components.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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 the onChange handler or use inputMode="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

📥 Commits

Reviewing files that changed from the base of the PR and between cfbbcc3 and fc5f36e.

📒 Files selected for processing (11)
  • web/app/components/app/configuration/config-var/config-modal/index.tsx
  • web/app/components/app/configuration/config-var/index.tsx
  • web/app/components/app/configuration/debug/chat-user-input.tsx
  • web/app/components/app/configuration/prompt-value-panel/index.tsx
  • web/app/components/rag-pipeline/components/panel/input-field/editor/form/hooks.ts
  • web/app/components/rag-pipeline/components/panel/input-field/editor/form/index.spec.tsx
  • web/app/components/share/text-generation/index.tsx
  • web/app/components/share/text-generation/run-once/index.spec.tsx
  • web/app/components/share/text-generation/run-once/index.tsx
  • web/config/index.ts
  • web/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_length matches 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 })
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

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.

Suggested change
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants