-
Notifications
You must be signed in to change notification settings - Fork 2.3k
improvement(var-resolution): resolve variables with block name check and consolidate code #1469
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
Conversation
…and consolidate code
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.
Greptile Overview
Summary
This PR consolidates and improves the variable resolution system across the workflow platform. The changes introduce consistent block name normalization using a centralized normalizeBlockName() function, which converts block names to lowercase and removes spaces for reliable matching.
Key improvements include:
-
Enhanced Reference Detection: Updated
extractReferencePrefixes()andisLikelyReferenceSegment()functions with better validation logic to accurately identify variable references while avoiding false positives from comparison operators and HTML-like content -
Consolidated Resolution Logic: The
InputResolverclass has been significantly refactored to use pre-calculated accessible block maps for better performance, with improved handling of loop and parallel block references -
UI Consistency: All input components now use the new
useAccessibleReferencePrefixeshook, ensuring consistent variable reference highlighting and validation across the interface -
Optimized Performance: The serializer now pre-computes accessible block relationships, reducing runtime complexity during variable resolution
-
Improved Error Handling: Better validation and error messages when blocks are not found or not accessible
The changes maintain backward compatibility while significantly improving the reliability and performance of variable resolution throughout the platform. The consolidated approach ensures all components use the same normalization and validation logic, reducing inconsistencies and potential bugs.
Confidence Score: 4/5
- This PR is safe to merge with careful testing of variable resolution functionality
- Score reflects comprehensive refactoring of a critical system component with good code quality and architectural improvements, but requires thorough testing due to the scope of changes affecting variable resolution across the entire platform
- The core resolver (
apps/sim/executor/resolver/resolver.ts) needs the most attention due to its complexity and critical role in variable resolution
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| apps/sim/lib/workflows/references.ts | 5/5 | Enhanced variable reference detection with improved validation and block name normalization for consistent resolution across workflow components |
| apps/sim/executor/resolver/resolver.ts | 4/5 | Major refactoring to consolidate variable resolution logic with improved accessibility checks, block name normalization, and enhanced loop/parallel reference handling |
| apps/sim/serializer/index.ts | 5/5 | Added comprehensive accessible block calculation and improved workflow validation with consistent block name normalization throughout serialization |
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-accessible-reference-prefixes.ts | 5/5 | New React hook that provides consistent accessible reference prefix calculation for UI components using the same logic as the resolver |
| apps/sim/components/ui/formatted-text.tsx | 5/5 | Enhanced text formatting component with improved block name normalization for consistent syntax highlighting of variable references |
| apps/sim/components/ui/tag-dropdown.tsx | 5/5 | Updated dropdown component to use consistent block name normalization for better variable reference suggestions |
Sequence Diagram
sequenceDiagram
participant UI as UI Components
participant Hook as useAccessibleReferencePrefixes
participant Refs as references.ts
participant Resolver as InputResolver
participant Serializer as Serializer
participant BlockCalc as BlockPathCalculator
Note over UI,BlockCalc: Variable Resolution Flow
UI->>Hook: Request accessible prefixes for blockId
Hook->>BlockCalc: findAllPathNodes(edges, blockId)
BlockCalc-->>Hook: ancestorIds[]
Hook->>Hook: Add starter, loop, parallel blocks
Hook->>Refs: normalizeBlockName() for each block
Hook-->>UI: Set<string> accessible prefixes
UI->>UI: User types variable reference <blockname.property>
UI->>Refs: extractReferencePrefixes(inputText)
Refs->>Refs: isLikelyReferenceSegment() validation
Refs->>Refs: normalizeBlockName() on prefix
Refs-->>UI: Array<{raw, prefix}> references
Note over Resolver: During Execution
Resolver->>Resolver: resolveInputs(block, context)
Resolver->>Resolver: processStringValue() for each input
Resolver->>Resolver: resolveVariableReferences() - handle <variable.name>
Resolver->>Resolver: resolveBlockReferences() - handle <block.property>
Resolver->>Resolver: validateBlockReference(blockRef, currentBlockId)
Resolver->>Resolver: getAccessibleBlocks() using pre-calculated map
Resolver->>Refs: normalizeBlockName() for comparison
alt Block reference found and accessible
Resolver->>Resolver: Resolve to actual value
else Block not accessible
Resolver-->>Resolver: Return validation error
end
Note over Serializer: During Serialization
Serializer->>Serializer: serializeWorkflow()
Serializer->>BlockCalc: computeAccessibleBlockIds()
Serializer->>Refs: normalizeBlockName() for consistency
Serializer-->>Resolver: accessibleBlocksMap for optimization
13 files reviewed, no comments
f0c20a9 to
010753d
Compare
Summary
Type of Change
Testing
Manually + Test suite
Checklist