Skip to content

Conversation

@icecrasher321
Copy link
Collaborator

@icecrasher321 icecrasher321 commented Dec 23, 2025

Summary

  • Centralize remaining regex check and normalization of names
    lib/, executor/, app/api/ → import from @/executor/constants
    stores/ → import from @/stores/workflows/utils (re-exports normalizeName)
  • Fixed blockNameMapping storing redundant data. Simplified lookup from O(n) loop to O(1) direct access
  • Pass on error of duplicate/empty block name to copilot

Type of Change

  • Other: Code quality

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
Copy link

vercel bot commented Dec 23, 2025

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

1 Skipped Deployment
Project Deployment Review Updated (UTC)
docs Skipped Skipped Dec 23, 2025 9:11pm

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 23, 2025

Greptile Summary

This PR centralizes regex patterns and name normalization utilities into @/executor/constants, replacing scattered implementations across the codebase. The centralization improves maintainability and consistency.

Key improvements:

  • Consolidated regex patterns (PATTERNS.UUID, PATTERNS.ENV_VAR_NAME), normalization (normalizeName), and utility functions (escapeRegExp, sanitizeFileName, isCustomTool, isMcpTool) into executor/constants.ts
  • Optimized blockNameMapping data structure by storing only normalized names (not both original and normalized), changing lookup from O(n) loop to O(1) direct access
  • Added proper validation for empty/whitespace-only block names in both workflow store and copilot operations
  • Enhanced copilot error reporting with new duplicate_block_name skip type to surface naming conflicts

Migration pattern:

  • lib/, executor/, app/api/ → import from @/executor/constants
  • stores/ → import from @/stores/workflows/utils (which re-exports normalizeName)

All changes maintain backward compatibility and improve code quality without altering functionality.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk - it's a well-executed refactoring that improves code quality
  • The changes are purely refactoring/consolidation with no new functionality. All modifications follow consistent patterns: moving utilities to a central location, updating imports, and optimizing data structures. Tests have been updated to match the new validation behavior. The O(n) to O(1) optimization is correct, and the centralized regex patterns reduce maintenance burden.
  • No files require special attention

Important Files Changed

Filename Overview
apps/sim/executor/constants.ts Added centralized regex patterns (UUID, env var), normalization function, and utility functions for string manipulation
apps/sim/executor/utils/block-data.ts Simplified blockNameMapping to store only normalized names (O(1) lookup), removed redundant entries
apps/sim/lib/copilot/tools/server/workflow/edit-workflow.ts Added duplicate/empty block name validation for copilot with proper error reporting via skipped items
apps/sim/stores/workflows/workflow/store.ts Added empty name validation, uses centralized normalizeName for duplicate checking and reference updates
apps/sim/app/api/function/execute/route.ts Optimized blockNameMapping lookup from O(n) to O(1), uses centralized constants and utilities

Sequence Diagram

sequenceDiagram
    participant Client as Client/UI
    participant Store as Workflow Store
    participant Copilot as Copilot Tool
    participant Utils as executor/constants
    participant Executor as Execution Engine
    
    Note over Utils: Centralized utilities:<br/>normalizeName()<br/>PATTERNS (UUID, ENV_VAR)<br/>isCustomTool(), isMcpTool()<br/>escapeRegExp()

    Client->>Store: updateBlockName(id, name)
    Store->>Utils: normalizeName(name)
    Utils-->>Store: normalized name
    Store->>Store: Check if normalized name empty
    alt Empty normalized name
        Store-->>Client: {success: false}
    else Valid name
        Store->>Store: Find duplicate in blocks
        alt Duplicate found
            Store-->>Client: {success: false}
        else No duplicate
            Store->>Store: Update block name
            Store->>Utils: normalizeName() for references
            Store-->>Client: {success: true}
        end
    end

    Copilot->>Copilot: edit-workflow operation
    Copilot->>Utils: normalizeName(params.name)
    Utils-->>Copilot: normalized name
    alt Empty normalized name
        Copilot->>Copilot: logSkippedItem('missing_required_params')
    else Has duplicate
        Copilot->>Copilot: logSkippedItem('duplicate_block_name')
    else Valid
        Copilot->>Copilot: Apply operation
    end

    Client->>Executor: Execute workflow
    Executor->>Executor: collectBlockData()
    loop For each block
        Executor->>Utils: normalizeName(block.name)
        Utils-->>Executor: normalized name
        Executor->>Executor: blockNameMapping[normalized] = blockId
    end
    
    Executor->>Executor: Resolve variable reference
    Executor->>Executor: Direct O(1) lookup in blockNameMapping
    Note over Executor: Before: O(n) loop through mapping<br/>After: O(1) direct access
Loading

@icecrasher321
Copy link
Collaborator Author

@greptile

@icecrasher321 icecrasher321 merged commit bf8fbeb into staging Dec 23, 2025
11 checks passed
@waleedlatif1 waleedlatif1 deleted the improvement/code-qual branch December 24, 2025 01:56
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.

3 participants