From fbc2ada08b917acc21597f86c44b0d78b9d6a9fc Mon Sep 17 00:00:00 2001
From: tolvitty <oelfkeluca12@proton.me>
Date: Fri, 13 Feb 2026 20:46:56 +0100
Subject: [PATCH] fix: address code review issues in conditional logic

- Add explicit boolean handling for toggle equals/not_equals
- Fix toNum to return NaN instead of 0 for empty values
- Use NaN guards in greater_than/less_than comparisons
- Add cleanupStaleConditions after field reorder/delete
- Clean up empty conditions objects before save

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 src/ui/form-builder.ts        | 24 ++++++++++++++++++++++++
 src/utils/condition-engine.ts | 21 ++++++++++++++++-----
 2 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/src/ui/form-builder.ts b/src/ui/form-builder.ts
index 2607bb5..ead95d7 100644
--- a/src/ui/form-builder.ts
+++ b/src/ui/form-builder.ts
@@ -61,6 +61,20 @@ export class FormBuilderModal extends Modal {
     this.contentEl.empty();
   }
 
+  private cleanupStaleConditions(): void {
+    for (let i = 0; i < this.draft.fields.length; i++) {
+      const field = this.draft.fields[i];
+      if (!field.conditions) continue;
+      const precedingIds = new Set(this.draft.fields.slice(0, i).map((f) => f.id));
+      field.conditions.rules = field.conditions.rules.filter((r) =>
+        precedingIds.has(r.fieldId),
+      );
+      if (field.conditions.rules.length === 0) {
+        field.conditions = undefined;
+      }
+    }
+  }
+
   private pushSnapshot(): void {
     // Truncate any forward history
     this.history = this.history.slice(0, this.historyIndex + 1);
@@ -272,6 +286,12 @@ export class FormBuilderModal extends Modal {
         new Notice('Form name cannot be empty.');
         return;
       }
+      // Clean up empty conditions before saving
+      for (const f of this.draft.fields) {
+        if (f.conditions && f.conditions.rules.length === 0) {
+          f.conditions = undefined;
+        }
+      }
       this.onSave(this.draft);
       this.close();
     });
@@ -317,6 +337,7 @@ export class FormBuilderModal extends Modal {
             this.draft.fields[i],
             this.draft.fields[i - 1],
           ];
+          this.cleanupStaleConditions();
           this.render();
         });
       }
@@ -334,6 +355,7 @@ export class FormBuilderModal extends Modal {
             this.draft.fields[i + 1],
             this.draft.fields[i],
           ];
+          this.cleanupStaleConditions();
           this.render();
         });
       }
@@ -347,6 +369,7 @@ export class FormBuilderModal extends Modal {
       deleteBtn.addEventListener('click', () => {
         this.pushSnapshot();
         this.draft.fields.splice(i, 1);
+        this.cleanupStaleConditions();
         this.render();
       });
 
@@ -398,6 +421,7 @@ export class FormBuilderModal extends Modal {
         if (this.dragSourceIndex < i) targetIndex--;
         this.draft.fields.splice(targetIndex, 0, moved);
         this.dragSourceIndex = null;
+        this.cleanupStaleConditions();
         this.render();
       });
 
diff --git a/src/utils/condition-engine.ts b/src/utils/condition-engine.ts
index 8d607ab..9d9ce5d 100644
--- a/src/utils/condition-engine.ts
+++ b/src/utils/condition-engine.ts
@@ -30,17 +30,27 @@ function evaluateRule(
     case 'is_not_empty':
       return !isEmpty(value);
     case 'equals':
+      if (typeof value === 'boolean') {
+        return value === (rule.value === 'true' || rule.value === true);
+      }
       return String(value ?? '') === String(rule.value ?? '');
     case 'not_equals':
+      if (typeof value === 'boolean') {
+        return value !== (rule.value === 'true' || rule.value === true);
+      }
       return String(value ?? '') !== String(rule.value ?? '');
     case 'contains':
       return checkContains(value, rule.value);
     case 'not_contains':
       return !checkContains(value, rule.value);
-    case 'greater_than':
-      return toNum(value) > toNum(rule.value);
-    case 'less_than':
-      return toNum(value) < toNum(rule.value);
+    case 'greater_than': {
+      const a = toNum(value), b = toNum(rule.value);
+      return !isNaN(a) && !isNaN(b) && a > b;
+    }
+    case 'less_than': {
+      const a = toNum(value), b = toNum(rule.value);
+      return !isNaN(a) && !isNaN(b) && a < b;
+    }
     default:
       return true;
   }
@@ -61,7 +71,8 @@ function checkContains(value: unknown, search: unknown): boolean {
 }
 
 function toNum(value: unknown): number {
-  return parseFloat(String(value ?? '')) || 0;
+  if (value === null || value === undefined || value === '') return NaN;
+  return parseFloat(String(value));
 }
 
 /**