fix normalization logic

Author: icecrasher321

apps/sim/lib/workflows/comparison/compare.ts

@@ -1,31 +1,18 @@
-import type { BlockState, WorkflowState } from '@/stores/workflows/workflow/types'
-import { SYSTEM_SUBBLOCK_IDS, TRIGGER_RUNTIME_SUBBLOCK_IDS } from '@/triggers/constants'
+import type { WorkflowState } from '@/stores/workflows/workflow/types'
 import {
+  extractBlockFieldsForComparison,
+  extractSubBlockRest,
+  filterSubBlockIds,
   normalizedStringify,
   normalizeEdge,
   normalizeLoop,
   normalizeParallel,
+  normalizeSubBlockValue,
   normalizeValue,
   normalizeVariables,
-  sanitizeInputFormat,
-  sanitizeTools,
   sanitizeVariable,
 } from './normalize'
 
-/** Block with optional diff markers added by copilot */
-type BlockWithDiffMarkers = BlockState & {
-  is_diff?: string
-  field_diffs?: Record<string, unknown>
-}
-
-/** SubBlock with optional diff marker */
-type SubBlockWithDiffMarker = {
-  id: string
-  type: string
-  value: unknown
-  is_diff?: string
-}
-
 /**
  * Compare the current workflow state with the deployed state to detect meaningful changes.
  * Uses generateWorkflowDiffSummary internally to ensure consistent change detection.
@@ -125,51 +112,21 @@ export function generateWorkflowDiffSummary(
   for (const id of currentBlockIds) {
     if (!previousBlockIds.has(id)) continue
 
-    const currentBlock = currentBlocks[id] as BlockWithDiffMarkers
-    const previousBlock = previousBlocks[id] as BlockWithDiffMarkers
+    const currentBlock = currentBlocks[id]
+    const previousBlock = previousBlocks[id]
     const changes: FieldChange[] = []
 
-    // Compare block-level properties (excluding visual-only fields)
-    const {
-      position: _currentPos,
-      subBlocks: currentSubBlocks = {},
-      layout: _currentLayout,
-      height: _currentHeight,
-      outputs: _currentOutputs,
-      is_diff: _currentIsDiff,
-      field_diffs: _currentFieldDiffs,
-      ...currentRest
-    } = currentBlock
-
+    // Use shared helpers for block field extraction (single source of truth)
     const {
-      position: _previousPos,
-      subBlocks: previousSubBlocks = {},
-      layout: _previousLayout,
-      height: _previousHeight,
-      outputs: _previousOutputs,
-      is_diff: _previousIsDiff,
-      field_diffs: _previousFieldDiffs,
-      ...previousRest
-    } = previousBlock
-
-    // Exclude from data object:
-    // - width/height: container dimensions from autolayout
-    // - nodes: subflow node membership (derived/runtime for parallel/loop blocks)
-    // - distribution: parallel distribution (derived/runtime)
+      blockRest: currentRest,
+      normalizedData: currentDataRest,
+      subBlocks: currentSubBlocks,
+    } = extractBlockFieldsForComparison(currentBlock)
     const {
-      width: _cw,
-      height: _ch,
-      nodes: _cn,
-      distribution: _cd,
-      ...currentDataRest
-    } = (currentRest.data || {}) as Record<string, unknown>
-    const {
-      width: _pw,
-      height: _ph,
-      nodes: _pn,
-      distribution: _pd,
-      ...previousDataRest
-    } = (previousRest.data || {}) as Record<string, unknown>
+      blockRest: previousRest,
+      normalizedData: previousDataRest,
+      subBlocks: previousSubBlocks,
+    } = extractBlockFieldsForComparison(previousBlock)
 
     const normalizedCurrentBlock = { ...currentRest, data: currentDataRest, subBlocks: undefined }
     const normalizedPreviousBlock = {
@@ -194,10 +151,11 @@ export function generateWorkflowDiffSummary(
           newValue: currentBlock.enabled,
         })
       }
-      // Check other block properties
+      // Check other block properties (boolean fields)
+      // Use !! to normalize: null/undefined/false are all equivalent (falsy)
       const blockFields = ['horizontalHandles', 'advancedMode', 'triggerMode'] as const
       for (const field of blockFields) {
-        if (currentBlock[field] !== previousBlock[field]) {
+        if (!!currentBlock[field] !== !!previousBlock[field]) {
           changes.push({
             field,
             oldValue: previousBlock[field],
@@ -210,42 +168,27 @@ export function generateWorkflowDiffSummary(
       }
     }
 
-    // Compare subBlocks
-    const allSubBlockIds = [
+    // Compare subBlocks using shared helper for filtering (single source of truth)
+    const allSubBlockIds = filterSubBlockIds([
       ...new Set([...Object.keys(currentSubBlocks), ...Object.keys(previousSubBlocks)]),
-    ]
-      .filter(
-        (subId) =>
-          !TRIGGER_RUNTIME_SUBBLOCK_IDS.includes(subId) && !SYSTEM_SUBBLOCK_IDS.includes(subId)
-      )
-      .sort()
+    ])
 
     for (const subId of allSubBlockIds) {
-      const currentSub = currentSubBlocks[subId]
-      const previousSub = previousSubBlocks[subId]
+      const currentSub = currentSubBlocks[subId] as Record<string, unknown> | undefined
+      const previousSub = previousSubBlocks[subId] as Record<string, unknown> | undefined
 
       if (!currentSub || !previousSub) {
         changes.push({
           field: subId,
-          oldValue: previousSub?.value ?? null,
-          newValue: currentSub?.value ?? null,
+          oldValue: (previousSub as Record<string, unknown> | undefined)?.value ?? null,
+          newValue: (currentSub as Record<string, unknown> | undefined)?.value ?? null,
         })
         continue
       }
 
-      // Compare subBlock values with sanitization
-      let currentValue: unknown = currentSub.value ?? null
-      let previousValue: unknown = previousSub.value ?? null
-
-      if (subId === 'tools' && Array.isArray(currentValue) && Array.isArray(previousValue)) {
-        currentValue = sanitizeTools(currentValue)
-        previousValue = sanitizeTools(previousValue)
-      }
-
-      if (subId === 'inputFormat' && Array.isArray(currentValue) && Array.isArray(previousValue)) {
-        currentValue = sanitizeInputFormat(currentValue)
-        previousValue = sanitizeInputFormat(previousValue)
-      }
+      // Use shared helper for subBlock value normalization (single source of truth)
+      const currentValue = normalizeSubBlockValue(subId, currentSub.value)
+      const previousValue = normalizeSubBlockValue(subId, previousSub.value)
 
       // For string values, compare directly to catch even small text changes
       if (typeof currentValue === 'string' && typeof previousValue === 'string') {
@@ -260,11 +203,9 @@ export function generateWorkflowDiffSummary(
         }
       }
 
-      // Compare subBlock REST properties (type, id, etc. - excluding value and is_diff)
-      const currentSubWithDiff = currentSub as SubBlockWithDiffMarker
-      const previousSubWithDiff = previousSub as SubBlockWithDiffMarker
-      const { value: _cv, is_diff: _cd, ...currentSubRest } = currentSubWithDiff
-      const { value: _pv, is_diff: _pd, ...previousSubRest } = previousSubWithDiff
+      // Use shared helper for subBlock REST extraction (single source of truth)
+      const currentSubRest = extractSubBlockRest(currentSub)
+      const previousSubRest = extractSubBlockRest(previousSub)
 
       if (normalizedStringify(currentSubRest) !== normalizedStringify(previousSubRest)) {
         changes.push({

apps/sim/lib/workflows/comparison/index.ts

@@ -4,12 +4,22 @@ export {
   generateWorkflowDiffSummary,
   hasWorkflowChanged,
 } from './compare'
-export type { NormalizedWorkflowState } from './normalize'
+export type {
+  BlockWithDiffMarkers,
+  NormalizedWorkflowState,
+  SubBlockWithDiffMarker,
+} from './normalize'
 export {
+  EXCLUDED_BLOCK_DATA_FIELDS,
+  extractBlockFieldsForComparison,
+  extractSubBlockRest,
+  filterSubBlockIds,
+  normalizeBlockData,
   normalizedStringify,
   normalizeEdge,
   normalizeLoop,
   normalizeParallel,
+  normalizeSubBlockValue,
   normalizeValue,
   normalizeVariables,
   normalizeWorkflowState,

apps/sim/lib/workflows/comparison/normalize.test.ts

@@ -21,7 +21,11 @@ describe('Workflow Normalization Utilities', () => {
       expect(normalizeValue('hello')).toBe('hello')
       expect(normalizeValue(true)).toBe(true)
       expect(normalizeValue(false)).toBe(false)
-      expect(normalizeValue(null)).toBe(null)
+    })
+
+    it.concurrent('should normalize null and undefined to undefined', () => {
+      // null and undefined are semantically equivalent in our system
+      expect(normalizeValue(null)).toBe(undefined)
       expect(normalizeValue(undefined)).toBe(undefined)
     })
 
@@ -81,7 +85,7 @@ describe('Workflow Normalization Utilities', () => {
       expect(result[0]).toBe(1)
       expect(result[1]).toBe('string')
       expect(Object.keys(result[2] as Record<string, unknown>)).toEqual(['a', 'b'])
-      expect(result[3]).toBe(null)
+      expect(result[3]).toBe(undefined) // null normalized to undefined
       expect(result[4]).toEqual([3, 2, 1]) // Array order preserved
     })
 
@@ -131,7 +135,11 @@ describe('Workflow Normalization Utilities', () => {
       expect(normalizedStringify(42)).toBe('42')
       expect(normalizedStringify('hello')).toBe('"hello"')
       expect(normalizedStringify(true)).toBe('true')
-      expect(normalizedStringify(null)).toBe('null')
+    })
+
+    it.concurrent('should treat null and undefined equivalently', () => {
+      // Both null and undefined normalize to undefined, which JSON.stringify returns as undefined
+      expect(normalizedStringify(null)).toBe(normalizedStringify(undefined))
     })
 
     it.concurrent('should produce different strings for different values', () => {
@@ -143,8 +151,9 @@ describe('Workflow Normalization Utilities', () => {
   })
 
   describe('normalizeLoop', () => {
-    it.concurrent('should return null/undefined as-is', () => {
-      expect(normalizeLoop(null)).toBe(null)
+    it.concurrent('should normalize null/undefined to undefined', () => {
+      // null and undefined are semantically equivalent
+      expect(normalizeLoop(null)).toBe(undefined)
       expect(normalizeLoop(undefined)).toBe(undefined)
     })
 
@@ -246,8 +255,9 @@ describe('Workflow Normalization Utilities', () => {
   })
 
   describe('normalizeParallel', () => {
-    it.concurrent('should return null/undefined as-is', () => {
-      expect(normalizeParallel(null)).toBe(null)
+    it.concurrent('should normalize null/undefined to undefined', () => {
+      // null and undefined are semantically equivalent
+      expect(normalizeParallel(null)).toBe(undefined)
       expect(normalizeParallel(undefined)).toBe(undefined)
     })