Skip to content

fix(blocks): clarify condition ID suffix slicing#3546

Merged
waleedlatif1 merged 1 commit intostagingfrom
waleedlatif1/condition-id-fix
Mar 12, 2026
Merged

fix(blocks): clarify condition ID suffix slicing#3546
waleedlatif1 merged 1 commit intostagingfrom
waleedlatif1/condition-id-fix

Conversation

@waleedlatif1
Copy link
Collaborator

@waleedlatif1 waleedlatif1 commented Mar 12, 2026

Summary

Test plan

  • Existing tests pass (no logic change, same output)
  • Verify condition/router block duplication still remaps IDs correctly

@cursor
Copy link

cursor bot commented Mar 12, 2026

PR Summary

Low Risk
Small, self-contained string-slicing logic for remapping condition/router IDs; main risk is subtle off-by-one errors that could miswire duplicated workflow edges.

Overview
Adds apps/sim/lib/workflows/condition-ids.ts with helpers to remap duplicated condition/router identifiers.

remapConditionBlockIds updates {blockId}-{suffix} IDs in-place within parsed condition arrays, and remapConditionEdgeHandle rewrites condition/router sourceHandle strings (condition-… / router-…) from an old block ID to a new one.

Written by Cursor Bugbot for commit c6a6557. Configure here.

@vercel
Copy link

vercel bot commented Mar 12, 2026

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Mar 12, 2026 8:18pm

Request Review

Use explicit hyphen separator instead of relying on slice offset to
implicitly include the hyphen in the suffix, making the intent clearer.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@waleedlatif1 waleedlatif1 force-pushed the waleedlatif1/condition-id-fix branch from c6a6557 to ecb1c3a Compare March 12, 2026 20:18
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 12, 2026

Greptile Summary

This PR introduces apps/sim/lib/workflows/condition-ids.ts, a new utility module containing two helpers — remapConditionBlockIds and remapConditionEdgeHandle — that re-key condition/router block IDs when a workflow block is duplicated. It is described as a readability improvement over inline logic from PR #3533, replacing the less-obvious oldBlockId.length + 1 slice offset with an explicit prefix variable that self-documents the hyphen being skipped.

Key observations:

  • The logic in both functions is correct. ID formats are handled consistently and there are no off-by-one errors.
  • remapConditionBlockIds uses the cleaner prefix + prefix.length pattern, but remapConditionEdgeHandle still uses the older oldBlockId.length + 1 approach — these two functions are now slightly inconsistent in style despite the PR's stated goal.
  • Neither function is imported anywhere in the current codebase. The file introduces dead code until a consuming PR lands. If this is an intentional stacked change, the PR description could make that clearer.
  • No unit tests exist for the new file, even though the utility logic is straightforward to test in isolation.

Confidence Score: 4/5

  • Safe to merge — no behavioral risk since the new functions are not yet called anywhere; the logic itself is correct.
  • The change is narrowly scoped (a single new file) and the implementation is logically sound. The only deductions are: one function still uses a slightly different idiom than the other (undermining the stated readability goal), and no tests accompany the new utility code.
  • No files require special attention — apps/sim/lib/workflows/condition-ids.ts is new and isolated.

Important Files Changed

Filename Overview
apps/sim/lib/workflows/condition-ids.ts New utility file that extracts condition/router ID remapping into two exported helpers. Logic is correct; minor style inconsistency between the two functions' suffix-extraction approaches, and no callers exist yet in the codebase.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["remapConditionBlockIds(conditions, oldBlockId, newBlockId)"]
    A --> B["Build prefix = oldBlockId + '-'"]
    B --> C{condition.id starts with prefix?}
    C -- Yes --> D["suffix = condition.id.slice(prefix.length)"]
    D --> E["condition.id = newBlockId + '-' + suffix"]
    E --> F["changed = true"]
    C -- No --> G["skip"]
    F & G --> H{more conditions?}
    H -- Yes --> C
    H -- No --> I["return changed"]

    J["remapConditionEdgeHandle(sourceHandle, oldBlockId, newBlockId)"]
    J --> K{starts with CONDITION_PREFIX or ROUTER_PREFIX?}
    K -- No --> L["try next prefix"]
    K -- Yes --> M["innerId = sourceHandle.slice(handlePrefix.length)"]
    M --> N{"innerId starts with oldBlockId + '-'?"}
    N -- No --> L
    N -- Yes --> O["suffix = innerId.slice(oldBlockId.length + 1)"]
    O --> P["return handlePrefix + newBlockId + '-' + suffix"]
    L --> Q{more prefixes?}
    Q -- Yes --> K
    Q -- No --> R["return sourceHandle unchanged"]
Loading

Comments Outside Diff (2)

  1. apps/sim/lib/workflows/condition-ids.ts, line 13-57 (link)

    Exported functions have no callers

    Both remapConditionBlockIds and remapConditionEdgeHandle are exported but are not imported anywhere in the codebase (confirmed via a full repo search). The file itself is brand-new (new file mode in this diff), so no existing code is being refactored—it is dead code until something imports it.

    If this file is intended to land as part of a stacked PR series (e.g., the actual duplication logic lives in an upcoming PR), that's fine, but it might be worth noting in the PR description or adding a comment to clarify intent, so reviewers don't mistake it for orphaned code.

  2. apps/sim/lib/workflows/condition-ids.ts, line 46-54 (link)

    Inconsistent suffix-extraction style between the two functions

    remapConditionBlockIds (line 19–23) introduces an explicit prefix variable that already includes the hyphen, then calls slice(prefix.length) — which is the readability improvement this PR advertises.

    remapConditionEdgeHandle uses the equivalent innerId.slice(oldBlockId.length + 1) without a named innerPrefix variable. The two functions now use slightly different idioms for the same operation. For full consistency, consider applying the same pattern here:

    This makes the intent equally self-documenting and keeps both functions consistent with each other.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Last reviewed commit: ecb1c3a

@waleedlatif1 waleedlatif1 merged commit aa0101c into staging Mar 12, 2026
11 checks passed
@waleedlatif1 waleedlatif1 deleted the waleedlatif1/condition-id-fix branch March 12, 2026 20:26
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.

1 participant