Skip to content

fix: remove hardcoded 48-character limit from text inputs#5

Open
tomerqodo wants to merge 2 commits intocodex_full_base_fix_remove_hardcoded_48-character_limit_from_text_inputs_pr5from
codex_full_head_fix_remove_hardcoded_48-character_limit_from_text_inputs_pr5
Open

fix: remove hardcoded 48-character limit from text inputs#5
tomerqodo wants to merge 2 commits intocodex_full_base_fix_remove_hardcoded_48-character_limit_from_text_inputs_pr5from
codex_full_head_fix_remove_hardcoded_48-character_limit_from_text_inputs_pr5

Conversation

@tomerqodo
Copy link
Copy Markdown

Benchmark PR from agentic-review-benchmarks#5

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fc5f36e4a6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

return false
}
const headerData = data[0]
console.log('Checking batch inputs:', { dataLength: data.length, headerData })
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Remove console logging from batch validation

This adds a console.log in the share text-generation batch validator, which violates the repo’s frontend rule forbidding console statements and can leak user-supplied CSV header data in production. That can trip lint checks and expose sensitive inputs in the browser console; please remove it or gate it behind a debug-only logger.

Useful? React with 👍 / 👎.

Comment on lines 260 to 262
if (varItem.type === 'string') {
const maxLen = varItem.max_length || DEFAULT_VALUE_MAX_LEN
if (item[varIndex].length > maxLen) {
if (item[varIndex].length > varItem.max_length) {
moreThanMaxLengthVarName = varItem.name
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Guard max_length nulls in batch length check

The new comparison uses item[varIndex].length > varItem.max_length without checking for null/undefined. When existing prompt configs have max_length: null (common for “no limit”), JavaScript coerces null to 0, so any non-empty cell fails validation and batch runs are blocked. Please only enforce the check when max_length is a finite number or normalize null/undefined to “no limit.”

Useful? React with 👍 / 👎.

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.

1 participant