Skip to content

Conversation

@waleedlatif1
Copy link
Collaborator

Summary

  • allow test value if no real value provided for inputs in deployed executions
    • if there is no real value provided for a field, but it is set in the inputForm, then use it even in deployed executions via API/chat

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 Dec 4, 2025

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

1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
docs Skipped Skipped Dec 4, 2025 7:41pm

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 4, 2025

Greptile Overview

Greptile Summary

Removed the isDeployedExecution check to allow test values from inputFormat to be used in deployed executions when no runtime value is provided.

Key Changes:

  • Removed isDeployedExecution parameter from buildStartBlockOutput function signature and all callsites
  • Modified deriveInputFromFormat to always use field.value as fallback when runtime value is null/undefined, regardless of execution context
  • Updated buildUnifiedStartOutput to accept structuredInput and hasStructured parameters for proper merging of test values with runtime values
  • Added logic to prioritize non-empty runtime values over test values, but preserve test values when runtime values are empty/null/undefined

Behavior Change:
Previously, test values in inputFormat were only used in manual/non-deployed executions. Now they serve as defaults for all execution types (API, chat, deployed) when the corresponding field is not provided at runtime.

Confidence Score: 4/5

  • This PR is safe to merge with low risk after considering the edge case behavior
  • The change is well-structured and the logic is sound for the intended use case. The refactor removes the isDeployedExecution flag cleanly across all files. However, there is a subtle behavior where empty strings from runtime cannot override test values, which may or may not be intentional. The tests pass and the change achieves its stated goal, but this edge case should be verified to match expected behavior.
  • Pay attention to apps/sim/executor/utils/start-block.ts to verify the empty string handling behavior matches requirements

Important Files Changed

File Analysis

Filename Score Overview
apps/sim/executor/execution/executor.ts 5/5 Removed isDeployedExecution parameter from buildStartBlockOutput call - clean refactor
apps/sim/executor/utils/start-block.ts 4/5 Changed logic to always use test values when runtime values are missing; modified buildUnifiedStartOutput to merge structured and workflow inputs with priority handling
apps/sim/executor/utils/start-block.test.ts 5/5 Updated tests to remove isDeployedExecution parameter - tests remain logically equivalent

Sequence Diagram

sequenceDiagram
    participant Executor
    participant buildStartBlockOutput
    participant deriveInputFromFormat
    participant buildUnifiedStartOutput
    
    Executor->>buildStartBlockOutput: resolution, workflowInput
    Note over Executor: Removed isDeployedExecution param
    
    buildStartBlockOutput->>deriveInputFromFormat: inputFormat, workflowInput
    Note over deriveInputFromFormat: For each field in inputFormat
    
    alt fieldValue from workflowInput exists
        deriveInputFromFormat->>deriveInputFromFormat: Use workflowInput value
    else fieldValue is null/undefined
        deriveInputFromFormat->>deriveInputFromFormat: Use field.value (test value)
        Note over deriveInputFromFormat: NEW: Always use test value as fallback
    end
    
    deriveInputFromFormat-->>buildStartBlockOutput: structuredInput, hasStructured
    
    buildStartBlockOutput->>buildUnifiedStartOutput: workflowInput, structuredInput, hasStructured
    Note over buildUnifiedStartOutput: NEW: Receives structuredInput
    
    alt hasStructured is true
        buildUnifiedStartOutput->>buildUnifiedStartOutput: Add all structuredInput fields to output
    end
    
    loop For each workflowInput field
        alt value is not empty
            buildUnifiedStartOutput->>buildUnifiedStartOutput: Overwrite output[key]
        else value is empty AND key not in output
            buildUnifiedStartOutput->>buildUnifiedStartOutput: Set output[key] = empty value
        else value is empty AND key exists in output
            buildUnifiedStartOutput->>buildUnifiedStartOutput: Keep existing output[key]
            Note over buildUnifiedStartOutput: Test value preserved
        end
    end
    
    buildUnifiedStartOutput-->>buildStartBlockOutput: output
    buildStartBlockOutput-->>Executor: blockOutput
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.

3 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

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