Skip to content

Conversation

@icecrasher321
Copy link
Collaborator

Summary

Codegen standardized for Python vs JS in terms of variable resolution. Main change fixes boolean resolution.

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

vercel bot commented Jan 26, 2026

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

1 Skipped Deployment
Project Deployment Review Updated (UTC)
docs Skipped Skipped Jan 26, 2026 9:33am

Request Review

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 26, 2026

Greptile Overview

Greptile Summary

This PR standardizes variable resolution and code generation across JavaScript and Python function execution contexts. The main change fixes boolean resolution to be more strict (only the string 'true' evaluates to true, while '1' now evaluates to false), and adds language-aware handling of primitives (booleans, null/undefined, numbers) throughout the codegen pipeline.

Key improvements:

  • Boolean parsing now consistently uses only 'true' string as truthy (removed '1' as a truthy value)
  • Function prologue generation now properly handles null, boolean, and number primitives without unnecessary JSON parsing for both JavaScript and Python
  • Block resolver now formats values based on target language (Python: True/False/None, JavaScript: true/false/null/undefined)
  • Isolated VM worker explicitly handles null values when setting context variables
  • Language detection extracted earlier in variable resolution flow for consistent formatting

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The changes are well-structured, focused on fixing edge cases in variable resolution and code generation. All modified tests pass with updated expectations that align with the new behavior. The changes improve type safety and language consistency without introducing breaking changes to the API surface.
  • No files require special attention

Important Files Changed

Filename Overview
apps/sim/lib/workflows/variables/variable-manager.ts Fixed boolean parsing to only accept 'true' string (removed '1' as truthy), ensuring consistent boolean resolution across the codebase
apps/sim/lib/workflows/variables/variable-manager.test.ts Updated test expectations to reflect new boolean parsing behavior where '1' now evaluates to false instead of true
apps/sim/app/api/function/execute/route.ts Improved function prologue generation with proper handling of null, boolean, and number primitives for both JavaScript and Python
apps/sim/executor/variables/resolvers/block.ts Added language-aware formatting for booleans and null/undefined values in function blocks (Python uses True/False/None, JS uses true/false/null/undefined)
apps/sim/executor/variables/resolver.ts Refactored to extract language detection earlier and pass it to block formatter for language-aware value formatting
apps/sim/lib/execution/isolated-vm-worker.cjs Added explicit null handling when setting context variables in isolated VM to prevent issues with null values

Sequence Diagram

sequenceDiagram
    participant User
    participant FunctionExecute as Function Execute API
    participant VariableManager
    participant VariableResolver
    participant BlockResolver
    participant IsolatedVM as Isolated VM Worker

    User->>FunctionExecute: Execute function with variables
    FunctionExecute->>VariableManager: resolveForExecution(value, type)
    VariableManager->>VariableManager: convertToNativeType(value, 'boolean')
    Note over VariableManager: Now only 'true' string → true<br/>('1' → false, changed behavior)
    VariableManager-->>FunctionExecute: Resolved boolean value
    
    FunctionExecute->>FunctionExecute: Build prologue for JS/Python
    Note over FunctionExecute: Handle null: const x = null / x = None<br/>Handle boolean: const x = true / x = True<br/>Handle number: const x = 42 / x = 42
    
    FunctionExecute->>IsolatedVM: Execute code with contextVariables
    IsolatedVM->>IsolatedVM: Set context variables in jail
    Note over IsolatedVM: Explicit null handling:<br/>if (value === null) jail.set(key, null)
    
    alt Variable reference in template
        User->>VariableResolver: resolveTemplate(template, block)
        VariableResolver->>VariableResolver: Extract language from block config
        Note over VariableResolver: language = block.config.params.language
        VariableResolver->>BlockResolver: formatValueForBlock(value, blockType, isInTemplateLiteral, language)
        BlockResolver->>BlockResolver: formatValueForCodeContext(value, isInTemplateLiteral, language)
        Note over BlockResolver: Python: True/False/None<br/>JS: true/false/undefined/null
        BlockResolver-->>VariableResolver: Formatted value
        VariableResolver-->>User: Resolved template
    end
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

No files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@icecrasher321
Copy link
Collaborator Author

@cursor review

@icecrasher321
Copy link
Collaborator Author

@cursor review

@icecrasher321
Copy link
Collaborator Author

@cursor review

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

@icecrasher321 icecrasher321 merged commit c7bd485 into staging Jan 26, 2026
11 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/codegen-func branch January 26, 2026 21:15
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