improvement(variables): add error context for duplicate variable names, only check for collision when focus is lost

[waleedlatif1]

Summary

• add error context for duplicate variable names by only check for collision when focus is lost
• removed the (n+1) prefix we add for duplicate variable names, throw error instead

Type of Change

• Bug fix

Testing

Tested manually

Checklist

• Code follows project style guidelines
• Self-reviewed my changes
• Tests added/updated and passing
• No new warnings introduced
• I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Preview	Comments	Updated (UTC)
docs	Skipped			Nov 2, 2025 0:42am

[greptile-apps]

Greptile Overview

Greptile Summary

Improved variable name validation by deferring duplicate checks until blur event instead of on every keystroke, preventing auto-generated (n+1) suffixes that were confusing to users.

Major Changes:

• Added local state management (localNames, nameErrors) in variables.tsx to track temporary edits and validation errors
• Implemented handleVariableNameBlur() to validate duplicate names only when user finishes editing
• Added visual error feedback with red border and error message for duplicate names
• Removed automatic (n+1) suffix generation from store, now throws error instead
• Refactored type configuration into TYPE_CONFIG constant and VARIABLE_TYPES array for better maintainability
• Simplified duplicate dropdown menu items by using array mapping

Issues Found:

• Empty variable names can be saved to the store (missing validation in handleVariableNameBlur)
• Performance: useEffect at line 221 iterates over all variables on every workflowVariables change, which could be optimized

Confidence Score: 3/5

• This PR has one critical logic bug that needs to be fixed before merging
• The implementation is well-structured and achieves the stated goal of improving UX by deferring validation. However, there's a critical bug where empty variable names can be saved to the store, which could break workflows. The performance concern with the useEffect is minor but worth addressing.
• Pay close attention to variables.tsx:110-126 - the empty name validation bug must be fixed

Important Files Changed

File Analysis

Filename	Score	Overview
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/variables/variables.tsx	4/5	Refactored duplicate name validation to use local state with blur-based checking, added error UI feedback. Simplified type configuration using constants.
apps/sim/stores/panel/variables/store.ts	5/5	Removed auto-incrementing (n+1) suffix logic from store, maintaining only the initial duplicate name handling for new variables.

Sequence Diagram

sequenceDiagram
participant User
participant Variables Component
participant LocalState
participant CollaborativeWorkflow
participant VariablesStore
User->>Variables Component: Types in variable name
Variables Component->>LocalState: Update localNames[variableId]
Variables Component->>LocalState: Clear nameErrors[variableId]
User->>Variables Component: Blur input field
Variables Component->>Variables Component: handleVariableNameBlur()
Variables Component->>Variables Component: Check if trimmedName is duplicate
alt Duplicate name found
Variables Component->>LocalState: Set nameErrors[variableId]
Variables Component->>User: Show error UI (red border)
else No duplicate
Variables Component->>CollaborativeWorkflow: collaborativeUpdateVariable(id, 'name', trimmedName)
CollaborativeWorkflow->>VariablesStore: updateVariable(id, {name})
VariablesStore->>VariablesStore: Update variable name
VariablesStore->>VariablesStore: Update references in workflow
Variables Component->>LocalState: clearLocalState(variableId)
Variables Component->>User: Name updated successfully
end
Note over Variables Component,LocalState: useEffect syncs localNames<br/>when variables are deleted

Loading

_{2 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[waleedlatif1]

@greptile

[greptile-apps]

Greptile Overview

Greptile Summary

This PR improves variable name validation by deferring duplicate checking until focus is lost (onBlur), preventing validation on every keystroke. Local state now tracks edited names before they're committed to the store.

Key changes:

• Replaced immediate collaborativeUpdateVariable on onChange with local state (localNames) that buffers changes
• Added handleVariableNameBlur to validate and save on blur, checking for empty names and duplicates
• Removed the automatic "(n)" suffix for duplicate names from store logic
• Added nameErrors state to display validation errors with red styling
• Added Enter key handler to trigger blur/validation

Issues identified:

• Critical: Empty variable names can still be saved to the store when a variable already has a non-empty persisted name. The validation in handleVariableNameBlur returns early if localName === undefined, meaning if user clears the input and never focuses it, the empty string from collaborativeAddVariable call (line 149: name: '') remains in the store
• Performance: The useEffect on lines 229-250 runs on every workflowVariables change and iterates over all variables to sync local state, causing unnecessary re-renders even when nothing changed for most variables

Confidence Score: 2/5

• This PR introduces a critical bug that allows empty variable names to persist in the store
• The validation logic has a flaw where empty names can bypass validation. When handleAddVariable creates a variable with name: '' (line 149), if the input never receives focus, localName remains undefined and the blur handler returns early without setting an error. Additionally, the re-render issue in the sync effect could impact performance in workflows with many variables
• Pay close attention to the validation logic in handleVariableNameBlur and the performance implications of the sync effect on lines 229-250

Important Files Changed

File Analysis

Filename	Score	Overview
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/variables/variables.tsx	2/5	Added variable name validation on blur with duplicate checking and empty name prevention, but logic allows empty names to be saved despite validation attempt

Sequence Diagram

sequenceDiagram
participant User
participant Input
participant Handler
participant Validation
participant Store
User->>Input: Types variable name
Input->>Handler: onChange event
Handler->>Validation: validateName(newName)
Validation-->>Handler: validatedName (strips invalid chars)
Handler->>Handler: setLocalNames(validatedName)
Handler->>Handler: clearError(variableId)
User->>Input: Blur or press Enter
Input->>Handler: onBlur event
Handler->>Handler: Check if localName exists
alt localName is undefined
Handler-->>Input: Return early (no changes)
end
Handler->>Handler: trim localName
alt trimmedName is empty
Handler->>Handler: setNameErrors("Variable name cannot be empty")
Handler-->>Input: Return (error set, no store update)
Note over Store: Empty name NOT saved to store
end
Handler->>Validation: isDuplicateName(variableId, trimmedName)
Validation->>Store: Check workflowVariables
alt Duplicate found
Handler->>Handler: setNameErrors("Two variables cannot have the same name")
Handler-->>Input: Return (error set, no store update)
end
Handler->>Store: collaborativeUpdateVariable(variableId, 'name', trimmedName)
Store-->>Store: Update persisted name
Handler->>Handler: clearLocalState(variableId)
Handler-->>Input: Success

Loading

_{1 file reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}