Skip to content

chore(daemon): enable noUnusedLocals + noUnusedParameters - #6130

Open
arseniy-gl wants to merge 2 commits into
nexu-io:mainfrom
arseniy-gl:chore/daemon-no-unused-locals
Open

chore(daemon): enable noUnusedLocals + noUnusedParameters#6130
arseniy-gl wants to merge 2 commits into
nexu-io:mainfrom
arseniy-gl:chore/daemon-no-unused-locals

Conversation

@arseniy-gl

Copy link
Copy Markdown
Contributor

Relates to #6084

Why

The project has no lint coverage for dead code — LLM-assisted development leaves unused imports, variables, and functions behind after refactoring. This is the first slice of the lint rollout plan agreed in #6084: enable noUnusedLocals + noUnusedParameters in the daemon tsconfig and fix all resulting violations.

What users will see

No user-facing changes. This is a code hygiene change — all removals are provably dead code (each identifier had zero references beyond its declaration).

Surface area

  • UI
  • Keyboard shortcut
  • CLI / env var
  • API / contract
  • Extension point
  • i18n keys
  • New top-level dependency
  • Default behavior change
  • None

Bug fix verification

Not a bug fix — no red spec needed.

Validation

What changed

tsconfig: Added noUnusedLocals: true and noUnusedParameters: true to apps/daemon/tsconfig.json.

src/ (24 files): Removed dead imports, unused constants, unused type declarations, and 4 dead function declarations (uniqueSlug, validateProvenance, possibleDsmlArtifactOpenStart/possibleArtifactCloseStart, fetchVelaModelsWithRetry, readLastSummary). Cascade cleanup freed additional orphaned constants in live-artifacts/schema.ts and mcp-routes.ts.

tests/ (14 files): Removed dead imports and one copy-pasted createSession() call in pi-rpc.test.ts that was shadowed by the test's own setup.

Next slices (from #6084 plan)

  • PR 2: @typescript-eslint/no-floating-promises (introduces minimal ESLint config)
  • PR 3: react-hooks/exhaustive-deps
  • PR 4: @typescript-eslint/no-unsafe-assignment
  • Follow-up: noUnusedLocals + noUnusedParameters for apps/web (141 violations) and other packages

…olations

Add the two tsconfig flags to apps/daemon/tsconfig.json and fix all
resulting violations across src/ (41) and tests/ (18):

- Remove dead imports (unused functions, types, constants)
- Remove dead variables and function declarations
- Remove unused destructured properties
- Remove one copy-pasted dead createSession() call in pi-rpc tests

No behavioral changes — all removals are provably dead code
(each identifier had zero references beyond its declaration).

Relates to nexu-io#6084. First slice of the lint rollout plan.
@lefarcen

lefarcen commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Hey @arseniy-gl — thanks for spinning this into the first slice from #6084. Keeping the rollout daemon-scoped and focused on dead-code cleanup is a sensible way to land the stricter check without turning this into a repo-wide lint sweep.

I’m routing this to the next reviewer now.

💡 To drive this PR to merge hands-free, paste this to your AI coding agent (Claude Code / Codex / opencode / Cursor …):
Take over nexu-io/open-design#6130 until it merges — read https://raw.githubusercontent.com/nexu-io/looper/main/skills/pr-takeover/SKILL.md and follow it.

@lefarcen
lefarcen requested a review from nettee July 27, 2026 17:27
@lefarcen lefarcen added size/M PR changes 100-300 lines risk/high High risk: apps/desktop, daemon, auth, migration, workflows, package deps type/chore CI / build / config / tooling labels Jul 27, 2026
@lefarcen lefarcen added the skip-validation Maintainer override: bot will not auto-add needs-validation on this PR. label Jul 27, 2026

@nettee nettee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pass found one blocking issue in the daemon tsconfig rollout. Please address the inline note below.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

Comment thread apps/daemon/tsconfig.json Outdated
"skipLibCheck": true,
"types": ["node", "vitest"]
"types": ["node", "vitest"],
"noUnusedLocals": true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changed config only enables noUnusedLocals, but this PR title/body and the surrounding cleanup are explicitly about rolling out both noUnusedLocals and noUnusedParameters. Because noUnusedParameters is still absent here, the daemon build will continue to accept unused parameters and this slice does not actually enforce half of the intended check going forward. Please add "noUnusedParameters": true in this compilerOptions block and rerun the daemon typecheck/tests so any remaining parameter violations are fixed in the same PR.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

@lefarcen

Copy link
Copy Markdown
Contributor

Hey @arseniy-gl@nettee's blocking review covers the current blocker on this head: noUnusedParameters still isn't enabled in apps/daemon/tsconfig.json, so this slice doesn't yet enforce the full scope described in the PR.

Once that flag is added and the daemon typecheck/tests are rerun cleanly, this should be in good shape for another pass.

@github-actions

Copy link
Copy Markdown
Contributor

@arseniy-gl friendly reminder: this PR appears to be waiting on author action (outstanding trusted reviewer or maintainer feedback, failing CI) and has had no human activity for more than 3 days.

When you have a chance, please reply here or push an update. To keep the queue manageable, PRs with no human activity for more than 5 days may be closed automatically, but they can be reopened when work resumes.

Address review feedback: the initial commit only added noUnusedLocals
but missed noUnusedParameters. This adds the flag and fixes all
resulting violations — prefixing unused params with _ where they are
required by interface/callback signatures.

@nettee nettee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arseniy-gl I rechecked the daemon tsconfig rollout on the current head and verified the missing noUnusedParameters flag is now present alongside the follow-up underscore renames needed to satisfy it. The remaining diff is cleanup-only dead-code removal across daemon source and tests, and I did not find any changed-range correctness or maintainability regressions in this pass. Local pnpm --filter @open-design/daemon typecheck verification was not runnable in the prepared worktree because the checkout does not include installed workspace dependencies (esbuild was missing under packages/contracts), so this approval is based on the live diff review plus the PR's reported validation. Nice cleanup pass and good follow-through on the earlier review feedback.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk/high High risk: apps/desktop, daemon, auth, migration, workflows, package deps size/M PR changes 100-300 lines skip-validation Maintainer override: bot will not auto-add needs-validation on this PR. type/chore CI / build / config / tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants