Skip to content

chore: remove dead code and add knip gate to CI check - #1612

Merged
kevincodex1 merged 1 commit into
Twigpine:mainfrom
kevincodex1:chore/dead-code
Jun 14, 2026
Merged

kevincodex1 merged 1 commit into
Twigpine:mainfrom
kevincodex1:chore/dead-code

Conversation

@kevincodex1

@kevincodex1 kevincodex1 commented Jun 13, 2026 •

Copy link
Copy Markdown
Member

Delete 32 unreferenced source files (~4,000 lines) verified dead by import-specifier grep and knip: test-only token utilities, orphaned hooks (useTaskListWatcher, useSkillImprovementSurvey + its component), the removed DevBar and ConfigTool UIs, unregistered bundled skills (stuck, verifyContent), unused analytics sinks, the benchmark command, and stale migrations/helpers.

Remove unused dependencies code-excerpt, stack-utils, and tsx from package.json plus their entries in build stub/external lists.

Add knip with a tuned knip.json (entrypoints, build-time stub targets, subprocess-launched fixtures, and runtime-string-imported SDKs ignored; providerAutoDetect kept intentionally as provider pre-wiring) and wire bun run deadcode into the check script so dead code stays dead.

Summary by CodeRabbit

  • Removals

    • Removed benchmark command functionality
    • Removed skill improvement survey interface
    • Removed configuration management tool
    • Removed unused dependencies
  • Chores

    • Added dead code detection tooling to improve code quality and maintainability

Delete 32 unreferenced source files (~4,000 lines) verified dead by
import-specifier grep and knip: test-only token utilities, orphaned hooks
(useTaskListWatcher, useSkillImprovementSurvey + its component), the
removed DevBar and ConfigTool UIs, unregistered bundled skills (stuck,
verifyContent), unused analytics sinks, the benchmark command, and
stale migrations/helpers.

Remove unused dependencies code-excerpt, stack-utils, and tsx from
package.json plus their entries in build stub/external lists.

Add knip with a tuned knip.json (entrypoints, build-time stub targets,
subprocess-launched fixtures, and runtime-string-imported SDKs ignored;
providerAutoDetect kept intentionally as provider pre-wiring) and wire
`bun run deadcode` into the `check` script so dead code stays dead.

Co-Authored-By: OpenClaude <openclaude@gitlawb.com>
@kevincodex1
kevincodex1 requested a review from jatmn June 13, 2026 08:20
@coderabbitai

coderabbitai Bot commented Jun 13, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 0fbf17c7-300b-4ca5-9d09-7e86ccddd620

📥 Commits

Reviewing files that changed from the base of the PR and between a3a3c36 and a436a64.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (39)
  • knip.json
  • package.json
  • scripts/build.ts
  • scripts/externals.ts
  • src/commands/benchmark.ts
  • src/components/DevBar.tsx
  • src/components/SkillImprovementSurvey.tsx
  • src/constants/keys.ts
  • src/entrypoints/cli.test.ts
  • src/hooks/useMemoryUsage.ts
  • src/hooks/useSkillImprovementSurvey.ts
  • src/hooks/useTaskListWatcher.ts
  • src/migrations/migrateFennecToOpus.ts
  • src/moreright/useMoreRight.tsx
  • src/services/analytics/firstPartyEventLogger.ts
  • src/services/analytics/sinkKillswitch.ts
  • src/services/api/metricsOptOut.ts
  • src/services/api/openaiShim.ts
  • src/skills/bundled/stuck.ts
  • src/skills/bundled/verifyContent.ts
  • src/tools/ConfigTool/ConfigTool.ts
  • src/tools/ConfigTool/UI.tsx
  • src/tools/ConfigTool/constants.ts
  • src/tools/ConfigTool/prompt.ts
  • src/tools/ConfigTool/supportedSettings.ts
  • src/utils/contextPreload.test.ts
  • src/utils/contextPreload.ts
  • src/utils/model/benchmark.test.ts
  • src/utils/model/benchmark.ts
  • src/utils/model/modelCache.test.ts
  • src/utils/model/modelCache.ts
  • src/utils/streamingTokenCounter.test.ts
  • src/utils/streamingTokenCounter.ts
  • src/utils/taggedId.ts
  • src/utils/telemetryAttributes.ts
  • src/utils/thinkingTokenExtractor.test.ts
  • src/utils/thinkingTokenExtractor.ts
  • src/utils/tokenAnalytics.test.ts
  • src/utils/tokenAnalytics.ts
💤 Files with no reviewable changes (35)
  • src/utils/model/benchmark.test.ts
  • src/utils/taggedId.ts
  • src/constants/keys.ts
  • src/hooks/useSkillImprovementSurvey.ts
  • src/components/DevBar.tsx
  • src/services/analytics/sinkKillswitch.ts
  • src/skills/bundled/stuck.ts
  • src/utils/thinkingTokenExtractor.ts
  • scripts/externals.ts
  • src/hooks/useMemoryUsage.ts
  • src/utils/streamingTokenCounter.test.ts
  • src/utils/contextPreload.ts
  • src/tools/ConfigTool/ConfigTool.ts
  • src/tools/ConfigTool/constants.ts
  • src/utils/telemetryAttributes.ts
  • src/services/analytics/firstPartyEventLogger.ts
  • src/utils/model/benchmark.ts
  • src/migrations/migrateFennecToOpus.ts
  • src/tools/ConfigTool/supportedSettings.ts
  • src/utils/tokenAnalytics.ts
  • src/tools/ConfigTool/UI.tsx
  • src/utils/thinkingTokenExtractor.test.ts
  • src/utils/tokenAnalytics.test.ts
  • src/utils/streamingTokenCounter.ts
  • src/skills/bundled/verifyContent.ts
  • src/moreright/useMoreRight.tsx
  • src/utils/model/modelCache.ts
  • src/entrypoints/cli.test.ts
  • src/commands/benchmark.ts
  • src/services/api/metricsOptOut.ts
  • src/tools/ConfigTool/prompt.ts
  • src/utils/model/modelCache.test.ts
  • src/components/SkillImprovementSurvey.tsx
  • src/utils/contextPreload.test.ts
  • src/hooks/useTaskListWatcher.ts
📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (6)
**/*

⚙️ CodeRabbit configuration file

**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.

Files:

  • knip.json
  • src/services/api/openaiShim.ts
  • scripts/build.ts
  • package.json
**

⚙️ CodeRabbit configuration file

**: # Contributing to OpenClaude

Thanks for contributing.

OpenClaude is a fast-moving open-source coding-agent CLI with support for multiple providers, local backends, MCP, and a terminal-first workflow. The best contributions here are focused, well-tested, and easy to review.

Before You Start

  • Search existing issues and discussions before opening a new thread.
  • Check open pull requests for work that overlaps with your contribution. If a PR already exists that addresses the same change, open an issue or discussion first to align on direction — duplicate PRs may be closed without review.
  • Use issues for confirmed bugs and actionable feature work.
  • Use discussions for setup help, ideas, and general community conversation.
  • For larger changes, open an issue first so the scope is clear before implementation.
  • For security reports, follow SECURITY.md.

Pull Requests

Every PR needs a reason. Your PR description must include:

  • what changed and why
  • the user or developer impact
  • the exact checks you ran
  • a linked issue when one exists, using Fixes fix: skip assertMinVersion for third-party providers #123, `Closes `#123, or another clear link
  • screenshots when the PR touches UI, terminal presentation, or the VS Code extension
  • which provider path was tested when the PR changes provider behavior

The PR author is responsible for ensuring their PR is merge-ready. PRs with merge conflicts will not be reviewed or approved until the conflicts are resolved.

Issues are the recommended starting point for anything non-trivial — opening one first helps avoid wasted effort if the change is out of scope or already being worked on. Small fixes, doc corrections, and obvious improvements can stand on their own without a linked issue, as long as the PR description explains the intent.

What Gets Closed Without Review

PRs may be closed without review...

Files:

  • knip.json
  • src/services/api/openaiShim.ts
  • scripts/build.ts
  • package.json
**/*.{ts,tsx,js,jsx,py}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*.{ts,tsx,js,jsx,py}: Follow the existing code style in the touched files
Keep comments useful and concise

Files:

  • src/services/api/openaiShim.ts
  • scripts/build.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Typecheck TypeScript code before submitting (use bun run typecheck)

Files:

  • src/services/api/openaiShim.ts
  • scripts/build.ts
{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}

⚙️ CodeRabbit configuration file

{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.

Files:

  • src/services/api/openaiShim.ts
{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}

⚙️ CodeRabbit configuration file

{bin/**,scripts/**,package.json,src/setup.ts,src/main.tsx,src/entrypoints/**}: Review install, launcher, build, packaging, startup, and entrypoint changes for cross-platform compatibility, tracked-source rewrites, env/config precedence, and release safety. Block on changes that can break Windows/macOS/Linux startup or publish unexpected artifacts.

Files:

  • scripts/build.ts
  • package.json
🔇 Additional comments (5)
src/services/api/openaiShim.ts (1)

657-660: LGTM!

scripts/build.ts (2)

505-505: LGTM!


505-505: Confirm externals consistency for SDK stub removals

scripts/externals.ts contains no references to code-excerpt or stack-utils, so removing them from the SDK missing-module stubbing list in scripts/build.ts is consistent.

knip.json (1)

31-35: Update: the ignored Foundry/Vertex SDKs are loaded at runtime (so the knip ignore is justified)

src/services/api/client.ts builds importRuntimeModule as new Function(... return import(specifier)), and both @anthropic-ai/foundry-sdk and @anthropic-ai/vertex-sdk are await-imported from string specifiers inside CLAUDE_CODE_USE_FOUNDRY / CLAUDE_CODE_USE_VERTEX env gates.

package.json (1)

64-65: Review package.json: knip deadcode wiring + removed dependency usage

The check script wiring is correct: bun run smoke && bun run deadcode && bun run test:full.

  • Removed deps: searches under src/ and scripts/ for import/require usage of code-excerpt and stack-utils returned no matches, and there are no references in scripts/externals.ts. (Repo-wide/non-TS/JS references and lockfile mentions still need checking.)
  • Non-blocking: knip is set to ^6.16.1—consider pinning if you want to avoid unexpected behavior changes from caret upgrades.

📝 Walkthrough

Walkthrough

This PR implements dead code detection via knip, removing approximately 2,500 lines of flagged unused features including analytics stubs, benchmarking tools, configuration management, token analysis utilities, and UI components. The core change adds knip configuration and a deadcode check script to the CI pipeline.

Changes

Cleanup

Layer / File(s) Summary
Dead code analysis setup
knip.json, package.json
Introduces knip configuration with entry/project/ignore scopes and adds deadcode script (knip --include files,dependencies) to the check pipeline via new devDependencies entry.
Direct dependency cleanup
package.json, scripts/build.ts, scripts/externals.ts
Removes unused code-excerpt and stack-utils from dependencies and strips them from CLI/SDK bundle stubbing lists and intentionally-bundled module registry.
Feature-specific removals and test cleanup
src/entrypoints/cli.test.ts, src/skills/bundled/stuck.ts, src/tools/ConfigTool/constants.ts, src/utils/contextPreload.test.ts
Removes memory usage threshold assertions, stuck skill registration export, config tool name constant, and context preload test suite as identified by dead code analysis.
Documentation clarification
src/services/api/openaiShim.ts
Adjusts comment text to clarify that Claude thinking content comes from .thinking while redacted_thinking comes from .data in message conversion logic.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • Gitlawb/openclaude#1228: Updates src/services/api/openaiShim.ts convertMessages() reasoning content extraction and adds test coverage for the handling (whereas this PR only clarifies the comment).
  • Gitlawb/openclaude#1576: Removes isAntEmployee-gated UI and business logic (DevBar, memory tracking gates), overlapping with portions of this cleanup PR's feature removals.

Suggested reviewers

  • jatmn
  • Vasanthdev2004
🚥 Pre-merge checks | ✅ 4 | ❌ 3

❌ Failed checks (3 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning Description is missing key sections from the template: Impact, Testing checklist, and Notes are not provided. Add Impact (user-facing and developer impact), Testing section with checkbox verification, and Notes with any relevant testing details.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Risk Surface Disclosed ⚠️ Warning PR removes permission/config and analytics/metrics modules (ConfigTool, metricsOptOut, firstPartyEventLogger) and changes CI to run knip; PR text doesn’t disclose risk surface or blockers. Add a brief “Risk surface” note in the PR describing impact on auth/provider/permissions/network/CI behavior and explicitly confirm whether any blocker exists (e.g., blocker: none).
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed Title is concise, scoped, and accurately describes the two main changes: dead code removal and knip integration into CI checks.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
No Hidden Policy Change ✅ Passed PR only adds knip.json + a CI deadcode step, cleans unused deps/stub lists, and deletes dead modules. No routing/trust/permission-policy code is added/modified; telemetry files removed were no-op/s...

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@jatmn jatmn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the contribution. I do not see any actionable issues from my review.

@kevincodex1 LGTM

@kevincodex1
kevincodex1 merged commit f4c3be8 into Twigpine:main Jun 14, 2026
4 checks passed
deagwon97 pushed a commit to deagwon97/openclaude that referenced this pull request Jun 15, 2026
Delete 32 unreferenced source files (~4,000 lines) verified dead by
import-specifier grep and knip: test-only token utilities, orphaned hooks
(useTaskListWatcher, useSkillImprovementSurvey + its component), the
removed DevBar and ConfigTool UIs, unregistered bundled skills (stuck,
verifyContent), unused analytics sinks, the benchmark command, and
stale migrations/helpers.

Remove unused dependencies code-excerpt, stack-utils, and tsx from
package.json plus their entries in build stub/external lists.

Add knip with a tuned knip.json (entrypoints, build-time stub targets,
subprocess-launched fixtures, and runtime-string-imported SDKs ignored;
providerAutoDetect kept intentionally as provider pre-wiring) and wire
`bun run deadcode` into the `check` script so dead code stays dead.

Co-authored-by: OpenClaude <openclaude@gitlawb.com>
hotmanxp pushed a commit to hotmanxp/openclaude that referenced this pull request Jun 15, 2026
Delete 32 unreferenced source files (~4,000 lines) verified dead by
import-specifier grep and knip: test-only token utilities, orphaned hooks
(useTaskListWatcher, useSkillImprovementSurvey + its component), the
removed DevBar and ConfigTool UIs, unregistered bundled skills (stuck,
verifyContent), unused analytics sinks, the benchmark command, and
stale migrations/helpers.

Remove unused dependencies code-excerpt, stack-utils, and tsx from
package.json plus their entries in build stub/external lists.

Add knip with a tuned knip.json (entrypoints, build-time stub targets,
subprocess-launched fixtures, and runtime-string-imported SDKs ignored;
providerAutoDetect kept intentionally as provider pre-wiring) and wire
`bun run deadcode` into the `check` script so dead code stays dead.

Co-authored-by: OpenClaude <openclaude@gitlawb.com>
(cherry picked from commit f4c3be8)
hotmanxp added a commit to hotmanxp/openclaude that referenced this pull request Jun 15, 2026
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.

2 participants