refactor: replace hand-rolled utilities and dead code with the shared forms - #7016
Closed
waleedlatif1 wants to merge 3 commits into
Closed
refactor: replace hand-rolled utilities and dead code with the shared forms#7016waleedlatif1 wants to merge 3 commits into
waleedlatif1 wants to merge 3 commits into
Conversation
… forms Each of these has a mandated helper or an established accessor in the repo that the site predates or missed. All are behavior-preserving: - `omit()` for the three `Object.fromEntries(Object.entries(x).filter(...))` block-input filters, which also recovers the `Omit<T, K>` typing that `Object.fromEntries` erases to an index signature. - `getErrorMessage()` for the inline `instanceof Error` message ternary. - `getBlock()` for two `getAllBlocks().find((b) => b.type === x)` scans, one of them inside a loop over selected tools. The same file already resolves the same values through `getBlock`. - A memoised `Map` for three `.find()`-by-id scans over the workspace skill list, one of them inside a render `.map()`. - `SELECTOR_SEARCH_STALE` for three copy-pasted `15 * 1000` literals. They are deliberately shorter than `SELECTOR_STALE`, so this is a new named constant rather than a fold into the existing one. - Tailwind classes for the static half of two duplicated anchor styles, keeping only the genuinely dynamic `left`/`top` inline. - Dropped the unused `catch` bindings on three intentional JSON-parse swallows. `panel.tsx`'s run-button gate loses a `TODO`-stubbed `hasValidationErrors = false` and the `isWorkflowBlocked` term built on it. That term was dead twice over: it reduced to `isExecuting`, and the enclosing expression is already guarded by `!isExecuting`.
…s callers `getBlock` normalizes its argument with `type.replace(...)`, so it throws on `undefined` where the `getAllBlocks().find(...)` it replaced returned `undefined` harmlessly. Both call sites can be reached without a type: `tool-input` reads `state.blocks[blockId]?.type`, which is undefined once the block is deleted while the panel is mounted — and `Record` indexing hides that from the compiler, so it would have thrown during render. `agent-handler`'s `tool.type` is optional and the compiler did catch it. Also index the skill lookup in `resolveSkillsLabel`, which runs a `.find()` inside a `.map()` for every block on the canvas — the case the memoised map in `skill-input` addressed for one component while leaving the hot path. `providers/utils.ts` keeps its `getAllBlocks().find(...)`: it takes the registry as an injected dependency precisely so a client-reachable module never imports it, and reaching for `getBlock` there would cross that boundary. The new constant's doc claimed search-backed selectors take a shorter window. Several still sit on `SELECTOR_STALE`, so it now describes the value its three callers share rather than asserting a rule the tree does not follow.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryLow Risk Overview
Reviewed by Cursor Bugbot for commit 15d9368. Configure here. |
Contributor
Greptile SummaryThe PR consolidates hand-rolled utilities around shared helpers and removes dead logic without intending behavioral changes.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/app/workspace/[workspaceId]/home/components/user-input/components/plus-menu-dropdown/plus-menu-dropdown.tsx | Moves fixed, size, and pointer-event declarations to Tailwind while correctly retaining runtime anchor coordinates inline. |
| apps/sim/app/workspace/[workspaceId]/home/components/user-input/components/skills-menu-dropdown/skills-menu-dropdown.tsx | Applies the same behavior-preserving anchor-style consolidation as the plus menu. |
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/tool-input/tool-input.tsx | Replaces repeated block-catalog scans with guarded registry lookups, including the newly added guard for potentially absent tool types. |
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/skill-input/skill-input.tsx | Builds a memoized skill index and uses it consistently for editing, naming, and rendering lookups. |
| apps/sim/executor/handlers/agent/agent-handler.ts | Uses the shared block registry lookup in the agent execution path. |
| apps/sim/hooks/selectors/providers/shared.ts | Introduces the shared selector-search stale-time constant used by the affected providers. |
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/panel.tsx | Removes validation placeholders and a blocked-state term that reduced to existing execution-state behavior. |
Reviews (2): Last reviewed commit: "fix: guard the second registry lookup in..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 15d9368. Configure here.
`selectedTools` validates only `value[0]?.type` and then casts the whole array, so a persisted workflow whose later rows lost their `type` yields `undefined` here — the cast is what makes the compiler believe otherwise. `getBlock` normalizes with `type.replace`, so that throws during render.
Collaborator
Author
|
@greptile-apps the summary is a commit behind — last reviewed |
This was referenced Aug 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Sites that hand-roll something the repo already mandates, plus the dead code around them. Each was verified equivalent before changing, not pattern-matched:
Object.fromEntries(Object.entries(x).filter(...))omit()blocks/blocks/{stt,fireflies,grain}.tserror instanceof Error ? error.message : String(error)getErrorMessage()executor/utils/errors.ts:53getAllBlocks().find((b) => b.type === x)getBlock()tool-input.tsx×2,agent-handler.ts.find()-by-id in a loopMapindexskill-input.tsx×3,display.ts15 * 1000×3SELECTOR_SEARCH_STALE{jira,google,webflow}/selectors.tsstyle{plus,skills}-menu-dropdown.tsxcatch (e) {}catch {}api-handler.ts,executor/utils.ts×2omit()also recovers theOmit<T, K>typing thatObject.fromEntrieserases to a bare index signature.getAllBlocks()is not memoised — each call built a fresh 336-element array before scanning it.getBlockis an O(1) registry index.agent-handler's call ran per tool during execution;tool-input's ran inside a loop over selected tools.panel.tsxloses aTODO-stubbedconst hasValidationErrors = falseand theisWorkflowBlockedterm built on it. That term was dead twice over — it reduced toisExecuting, and the enclosing expression is already guarded by!isExecuting, so the disjunct could only ever be reached asfalse.Two things the review caught
getBlockis not a drop-in for.find(). It normalizes viatype.replace(...), so it throws onundefinedwhere.find()returnedundefinedharmlessly. Both call sites are reachable without a type —tool-inputreadsstate.blocks[blockId]?.type, undefined once the block is deleted while the panel is mounted.Recordindexing hid that from the compiler; it would have thrown during render. Both are now guarded. The compiler did catch theagent-handlerone.providers/utils.ts:683deliberately keeps itsgetAllBlocks().find(...). It takes the registry as an injected dependency so a client-reachable module never imports it. Reaching forgetBlockthere would cross that boundary, so it was left alone.Scope
Behavior-preserving throughout — no user-visible change intended. The
SELECTOR_SEARCH_STALEconstant preserves the three sites' existing 15s window rather than folding them into the 60sSELECTOR_STALE; its doc describes what those three callers share rather than asserting a rule about search, since several other search-backed selectors still sit on the longer window.Testing
executor,blocks,hooks,lib/workflows/subblocks,app/workspacebun run type-checkclean;bun run check:api-validationpassesProvenance
Found by running mattpocock/skills over the codebase. Worth noting what the audit did not find: the repo's mandated-utility rules are already ~95% enforced — zero
@ts-ignore, zeroJSON.parse(JSON.stringify(, and 52 of 60 non-test double-casts already carry their required annotation.