Repository navigation
fix: remove unreachable internal-only branches - #2440
Conversation
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Deep-search effect and helpers src/components/LogSelector.tsx |
The effect clears deep-search state when its dependencies change. The deferred Fuse search and its result-ordering helpers are removed. |
Mock rate-limit checks
| Layer / File(s) | Summary |
|---|---|
Fast-mode mock branch removal src/services/rateLimitMocking.ts |
The fast-mode-specific mock branch and its imports are removed. The optional parameter remains with an underscore-prefixed name; the existing unified-status and overage-status checks remain. |
Context and output token limits
| Layer / File(s) | Summary |
|---|---|
Context-window and output-token resolution src/utils/context.ts |
Capability metadata no longer sets context-window or maximum output-token limits. Resolution continues through the existing paths. |
Clear-context plan acceptance
| Layer / File(s) | Summary |
|---|---|
Plan acceptance message src/components/permissions/ExitPlanModePermissionRequest/ExitPlanModePermissionRequest.tsx |
The verification instruction and its tool-name import are removed. The clear-context implementation message retains the plan, transcript, team, and feedback content. |
Updater callback cleanup
| Layer / File(s) | Summary |
|---|---|
Remove no-op expression src/components/PackageManagerAutoUpdater.tsx |
The standalone `false |
Priority:
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Refactor
Possibly related PRs
- Twigpine/openclaude#2438: Removes the same no-op expression from
PackageManagerAutoUpdater.tsx, along with an unrelated MCP expression.
Merge Risk: 
Minimal · up to eb7a2
-
No behavior regression was established in the reviewed changes; the PR is mergeable after normal checks.
Pre-merge checks: 
6 · 
1
-
Failed checks: 1 warning Passed checks: 6 passed-
Full details: Title check
-
Explanation
The title "fix: remove unreachable internal-only branches" matches the main change: the diff removes unreachable internal-only branches across context.ts, LogSelector.tsx, rateLimitMocking.ts, ExitPlanModePermissionRequest.tsx, and the updater's no-op expression. It is concise, follows a conventional prefix, and is scoped to the actual diff. It is slightly broad because it does not name the affected areas, but it is not vague or misleading.
Full details: Risk Surface Disclosed
-
Explanation
The PR touches permissions and background/update behavior. The description identifies the affected risk surfaces: plan handoff text, rate-limit handling, resume picker, and update behavior. It states that existing behavior remains unchanged, reports focused tests passing, and notes no provider behavior or visible UI changes. This discloses the impact and indicates no blocker.
Full details: No Hidden Policy Change
-
Explanation
PASS — The diff removes dead or external-build no-op paths and does not introduce a product, trust-model, routing-default, telemetry/network, or permission-policy decision. The plan handoff still uses the existing mode and permission-update logic; only the unreachable
undefined === 'true'verification text is removed. The model-capability module returnsundefined, so removing its reads does not change context or output limits; runtime-route, beta, experiment, Ant, and model-name branches remain. The rate-limit mock module returns no-op values, while normal status/overage/Opus handling and the caller remain. The updater body is unchanged apart from deletingfalse || false, and the deep-search removal deletes an unconditional dead branch and its helpers.
Full details: Description check
-
Explanation
The description includes the required Summary, Impact, Testing, and Notes sections. It explains the changes and rationale, documents user and maintainer impact, lists validation commands and results, identifies focused tests and inapplicable checks, and records CI and CodeRabbit status. No required information is materially missing.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 4 functions across 3 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
Summary
Disabled internal features left callers with impossible capability reads, rate-limit headers, verification text and deep-search work. Remove those unreachable branches and the updater's
false || falseexpression, preserving the existing external-build behavior.Part 1/4 of the requested small-category code-quality cleanup. Addresses findings 74, 75, 99–103, 105 and 411 (9 findings) in the quality report. Deep-search helper functions and their sole constant are removed with their unreachable caller.
Impact
Existing model limits, rate-limit handling, plan handoff text, resume picker and update behavior are unchanged. Removes misleading internal-only code without restoring disabled features.
Testing
bun install --frozen-lockfile.bun run check:static,bun run typecheck:fast,bun run typecheck:type-tests,node bin/openclaude --version, andNODE_DISABLE_COMPILE_CACHE=1 node bin/openclaude --version: passed.git diff --checkandbun run security:pr-scan -- --base upstream/main --head HEAD: passed after fetching canonical upstream main.a91f736ef1c257d00dd4ced04f86e21401289f40; no prior PR validation reused.Focused validation:
bun run test src/utils/context.test.ts src/components/LogSelector.resumeBranches.test.ts src/components/permissions/ExitPlanModePermissionRequest/ExitPlanModePermissionRequest.test.ts src/components/permissions/ExitPlanModePermissionRequest/ExitPlanModePermissionRequest.render.test.tsx src/components/AutoUpdater.test.ts src/components/NativeAutoUpdater.test.ts: 109 passed.Notes
No provider behavior or visible UI changes; existing rendered picker/plan tests cover the affected consumers, so no new screenshot is applicable. Related open PR #2430 changes updater services/build stubs, not the removed no-effect component expression.
Reviewed, locally validated and pushed commit:
eb7a22ae51499e1012da4475fd8638a6e83bb4ed.Summary by CodeRabbit