Skip to content

refactor: delete code nothing reaches - #7019

Merged
waleedlatif1 merged 1 commit into
stagingfrom
deslop-deadcode
Aug 24, 2026
Merged

refactor: delete code nothing reaches#7019
waleedlatif1 merged 1 commit into
stagingfrom
deslop-deadcode

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Stacked on #7018#7016#7015. Retarget to staging as those merge.

Why this accumulated

biome.json:101-102 disables noUnusedVariables and noUnusedFunctionParameters. None of this was ever going to be flagged.

How each item was proven dead

A repo-wide grep for the symbol across apps/ and packages/ returning only its own declaration — not "no importer found", but no occurrence at all, which also rules out barrel re-exports, dynamic string keys, and config references. tsc --noEmit then proves each deleted binding was unread, since a binding that is actually read fails to compile.

What went

11 module-scope loggers nothing logs through + the orphaned createLogger import each left behind
execute-platform-context-use-case.ts whole file — no importer, no barrel, neither export named anywhere
routeToolCall and with it ToolRoute, then ToolRouteTarget — a three-step cascade
processPastChat superseded by processPastChatFromDb; carried the file's last boundary-raw-fetch exemption
withMessageId pasted into three server tools, called in none
Write-only locals activeSubagent, resolvedReadPath, workflowPath, and workflow in a destructure
ACCEPTED_AUDIO_TYPES / ACCEPTED_VIDEO_TYPES never wired to an accept attribute the way their live sibling is
Unused catch bindings error-extractors.ts ×2, defaults.ts

activeSubagent is the interesting one: it was assigned twice and read never, because the scoped subagentBySpanId / subagentByParentToolCallId maps replaced it. The comment explaining why the name-match fallback was abandoned is kept — it just no longer names a symbol that does not exist.

One redundant guard

diff-engine.ts drops if (!proposedSubKeys.includes(key)) return true. The !proposedSub check three lines down already covers it: a key absent from the proposed block reads back undefined there, and so does a key present with a nullish value. Same answer on every input, minus an O(n) scan per iteration.

Deliberately NOT included

  • checkChunkWriteAccess looked like a missing-authz bug — its TSDoc says chunk update/delete require write permission, and nothing calls it. It is not a bug: those routes were migrated to knowledgeOperations.updateChunk / deleteChunk, both minimumRole: 'write'. The wider cluster of superseded knowledge authz helpers is worth removing, but it deserves its own PR rather than riding along in a mechanical sweep.
  • normalize.ts:203,295 — biome flags const { isExpanded, ...rest } as unused, but the destructure is the omit. Removing it would silently change the normalized payload and break diff comparison.
  • success === true coercions — TS calls them redundant, but the values come from third-party tool handlers that can return a truthy non-boolean.
  • ~1,787 orphan export keywords under tools/** — dropping the keyword is inert but it is thousands of lines of churn for no behavioral or bundle win.

Testing

  • 8219 tests passing across lib/copilot, lib/workflows, tools, triggers, lib/uploads
  • bun run type-check clean; bun run check:api-validation passes

@vercel

vercel Bot commented Aug 23, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Aug 24, 2026 1:59am

Request Review

@cursor

cursor Bot commented Aug 23, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Mechanical dead-code removal verified by grep and type-check; the only logic tweak is an equivalent guard in workflow diff comparison.

Overview
Removes unused Copilot/workflow/trigger code that nothing imported or read, after noUnusedVariables was disabled in biome.

Deletes the orphan execute-platform-context-use-case module, unused routeToolCall routing types, the superseded processPastChat HTTP fetch path (past chat already goes through processPastChatFromDb), unused withMessageId helpers in three server tools, unused accept-type constants, unused loggers, and write-only locals (activeSubagent, resolvedReadPath, unused destructures).

Also drops a redundant proposedSubKeys.includes check in diff-engine that the following !proposedSub guard already covers. No intended behavior change.

Reviewed by Cursor Bugbot for commit 34528af. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR removes declarations, imports, local variables, and helper code that are no longer referenced, while simplifying one redundant workflow-diff check.

  • Deletes an unused Copilot platform-context adapter and several unused tool-routing exports.
  • Removes unused loggers, server-tool helpers, upload constants, locals, and catch bindings.
  • Removes the superseded API-based past-chat processor.
  • Simplifies subblock comparison in the workflow diff engine without changing behavior for established workflow inputs.

Confidence Score: 5/5

The PR appears safe to merge because the removed code has no remaining repository consumers and no concrete behavioral regression was identified.

The changes are confined to unreachable declarations, unread bindings, unused imports, and a redundant comparison whose remaining check preserves behavior for established workflow inputs.

Important Files Changed

Filename Overview
apps/sim/lib/copilot/application/execute-platform-context-use-case.ts Deletes an unreferenced private application adapter with no remaining importers or re-exports.
apps/sim/lib/copilot/tool-executor/router.ts Removes unused route projection types and helper while retaining every export used by current callers.
apps/sim/lib/copilot/chat/process-contents.ts Removes the superseded API-based past-chat processor; the database-backed implementation remains.
apps/sim/lib/workflows/diff/diff-engine.ts Removes a redundant own-key membership check; no reachable workflow input was found for the only semantic edge case.
apps/sim/lib/workflows/executor/execution-core.ts Drops an unread workflow binding from snapshot destructuring without changing execution behavior.
apps/sim/lib/uploads/utils/validation.ts Removes two unreferenced derived MIME-type arrays while preserving upload validation and accept attributes.
apps/sim/lib/copilot/chat/effective-transcript.ts Removes a write-only subagent variable while retaining scoped lane attribution state.

Reviews (1): Last reviewed commit: "refactor: delete code nothing reaches" | Re-trigger Greptile

@waleedlatif1
waleedlatif1 changed the base branch from deslop-utils to staging August 24, 2026 01:58
`biome.json:101-102` turns off `noUnusedVariables` and
`noUnusedFunctionParameters`, so none of this was ever going to be flagged.
Everything here was confirmed by grepping the symbol across `apps/` and
`packages/` and finding only its own declaration; `tsc --noEmit` then proves
each deleted binding was unread, since a read one fails to compile.

- Eleven module-scope loggers that nothing logs through, with the now-orphaned
  `createLogger` import each left behind.
- `execute-platform-context-use-case.ts` — the whole file. No importer, no
  barrel, and neither export is named anywhere.
- `routeToolCall` and, once it goes, `ToolRoute` and `ToolRouteTarget` with it.
  The catalog accessors around them stay live.
- `processPastChat`, superseded by `processPastChatFromDb`. It carried the last
  `boundary-raw-fetch` exemption in the file.
- `withMessageId`, pasted into three server tools and called in none.
- Write-only locals: `activeSubagent` (assigned twice, read never — the scoped
  maps replaced it), `resolvedReadPath`, `workflowPath`, and `workflow` in an
  execution-core destructure.
- `ACCEPTED_AUDIO_TYPES` / `ACCEPTED_VIDEO_TYPES`, never wired to an accept
  attribute the way their live sibling is.
- Unused `catch` bindings in `error-extractors.ts` and `defaults.ts`.

`diff-engine.ts` drops a `proposedSubKeys.includes(key)` guard that the
`!proposedSub` check three lines down already covers: a key absent from the
proposed block reads back `undefined` there, and so does a key present with a
nullish value. Same answer on every input, without the O(n) scan per iteration.
@waleedlatif1
waleedlatif1 merged commit 297e970 into staging Aug 24, 2026
28 checks passed
@waleedlatif1
waleedlatif1 deleted the deslop-deadcode branch August 24, 2026 02:05
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