Skip to content

improvement(selectors): remove dead semantic fallback code#3454

Merged
icecrasher321 merged 6 commits intostagingfrom
improvement/semantic-fallback
Mar 7, 2026
Merged

improvement(selectors): remove dead semantic fallback code#3454
icecrasher321 merged 6 commits intostagingfrom
improvement/semantic-fallback

Conversation

@icecrasher321
Copy link
Collaborator

@icecrasher321 icecrasher321 commented Mar 7, 2026

Summary

  • Code hygiene for resolveDisplayName

Type of Change

  • Other: Code quality

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 Mar 7, 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 7, 2026 3:27am

Request Review

@cursor
Copy link

cursor bot commented Mar 7, 2026

PR Summary

Medium Risk
Moderate risk because it changes display-label fallback behavior for unresolved subblocks and makes workflow-state loading throw when a workspaceId is missing, which could surface previously-hidden invalid data paths.

Overview
Simplifies semantic fallback generation in resolveValueForDisplay by deriving it directly from the SubBlockConfig (title/id) and removing the large hardcoded ID-to-label map; if a subblock config can’t be found, the value now returns as unresolved instead of applying heuristic fallbacks.

Tightens workflow persistence migrations by requiring a non-optional workspaceId throughout the migration pipeline and explicitly throwing when a workflow has no workspace before running migrations (affecting both deployed-state loading and normalized-table loading).

Written by Cursor Bugbot for commit ddb2d85. Configure here.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 7, 2026

Greptile Summary

This PR removes dead code from resolve-values.ts by eliminating a redundant 50-entry hardcoded patterns map in getSemanticFallback and replacing it with a simpler lookup: subBlockConfig.title ?? subBlockConfig.id. An early-return guard is also added for cases where subBlockConfig cannot be found in the block registry.

Key changes:

  • getSemanticFallback signature narrowed from (subBlockId: string, subBlockConfig?: SubBlockConfig) to (subBlockConfig: SubBlockConfig) — callers must now guarantee the config is present.
  • New early return at line 174–176: when blockConfig or the subBlock ID is missing from the registry, the function now returns resolved: false immediately instead of continuing to attempt credential/workflow resolution. This is a subtle behavior change — e.g., a field with subBlockId === 'credential' on an unregistered block would previously still attempt resolveCredential; it now returns the raw formatted value.
  • One stale comment on line 225 references "pattern matching" after the patterns map was removed — this needs updating to reflect the new semanticFallback logic.

The simplification is well-reasoned; the patterns map was genuinely redundant given that subBlockConfig is reliably available when a block is properly registered. The early return for missing configs is a defensive improvement.

Confidence Score: 4/5

  • Safe to merge — the refactoring removes dead code cleanly. The stale comment is a documentation issue only.
  • The code refactoring is sound and well-executed. The 50-entry patterns map was genuinely redundant, and replacing it with subBlockConfig.title ?? subBlockConfig.id is correct. The early return for missing configs is a defensive improvement. One stale comment needs updating (identified in review), but this is a minor documentation fix, not a functional issue. The behavior change only affects unregistered/misconfigured blocks, which are edge cases. Main consideration: no unit tests exist for this file, but manual testing was performed.
  • Update the stale comment on line 225 of apps/sim/lib/workflows/comparison/resolve-values.ts from "use pattern matching" to reflect the actual semanticFallback logic.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A([resolveValueForDisplay]) --> B{value is string\nand non-empty?}
    B -- No --> C[Return displayLabel=formatValueForDisplay\nresolved=false]
    B -- Yes --> D[getBlock + find subBlockConfig]
    D --> E{subBlockConfig\nfound?}
    E -- No\n[NEW: early return] --> F[Return displayLabel=formatValueForDisplay\nresolved=false]
    E -- Yes --> G[getSemanticFallback\ntitle ?? id]
    G --> H{Credential\nfield?}
    H -- Yes --> I[resolveCredential]
    I --> J{label\nfound?}
    J -- Yes --> K[Return label, resolved=true]
    J -- No --> L[Return semanticFallback, resolved=true]
    H -- No --> M{workflow-selector?}
    M -- Yes --> N[resolveWorkflow]
    N --> O{label\nfound?}
    O -- Yes --> K
    O -- No --> L
    M -- No --> P{mcp-tool-selector?}
    P -- Yes --> Q[extractMcpToolName]
    Q --> K
    P -- No --> R{Hydration-required\ntype?}
    R -- Yes --> S[resolveSelectorValue]
    S --> T{label\nfound?}
    T -- Yes --> K
    T -- No --> L
    R -- No --> U{isUuid or\nSlack ID?}
    U -- Yes --> L
    U -- No --> V{CREDENTIAL_SET\nprefix?}
    V -- Yes --> W[resolveCredential]
    W --> K
    V -- No --> X[Return formatValueForDisplay\nresolved=false]
Loading

Comments Outside Diff (1)

  1. apps/sim/lib/workflows/comparison/resolve-values.ts, line 225-226 (link)

    The comment "use pattern matching" is now stale — the hardcoded patterns map was removed in this PR. The code now uses semanticFallback (derived from subBlockConfig.title ?? subBlockConfig.id) for recognized ID formats.

Last reviewed commit: 31d640e

@icecrasher321 icecrasher321 changed the title improvement(selectors): remove dead semantic fallback code improvement(selectors): remove dead semantic fallback code + applyBlockMigrations application Mar 7, 2026
@icecrasher321
Copy link
Collaborator Author

bugbot run

@icecrasher321 icecrasher321 force-pushed the improvement/semantic-fallback branch from bf083f1 to ddb2d85 Compare March 7, 2026 03:27
@icecrasher321 icecrasher321 changed the title improvement(selectors): remove dead semantic fallback code + applyBlockMigrations application improvement(selectors): remove dead semantic fallback code Mar 7, 2026
@icecrasher321
Copy link
Collaborator Author

bugbot run

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 1d36b80 into staging Mar 7, 2026
12 checks passed
@icecrasher321 icecrasher321 deleted the improvement/semantic-fallback branch March 7, 2026 03:39
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