Repository navigation
fix: remove next 40 unused local findings (batch 5) - #2457
Conversation
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
OpenAI shim cleanup src/services/api/openaiShim.ts, src/services/api/openaiShim/requestExecutor.ts, src/services/api/openaiShim/requestExecutor*.test.ts |
The shim and request executor remove unused bindings. The request executor tests remove stream-abort helpers and related test utilities. |
Application binding cleanup src/components/*, src/entrypoints/sdk/v2.ts, src/memdir/*, src/screens/REPL.tsx, src/services/api/bootstrap.ts, src/tools/WebSearchTool/providers/exa.ts, src/utils/* |
Application modules remove unused imports and local helpers. The pending-prompt loop still denies each pending prompt. |
Script and test helper cleanup scripts/*, src/memdir/vectorIndex.test.ts, src/services/api/claude.lifecycle.test.ts, src/services/compact/compact.test.ts, src/tools/ExitPlanModeTool/ExitPlanModeV2Tool.test.ts, src/utils/providerValidation.test.ts |
Scripts and tests remove unused imports, a command wrapper, a timeout helper, and a test context helper. |
Priority:
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Refactor
Suggested reviewers: gravirei
Merge Risk: 
dad5b
-
The cleanup has no identified merge-blocking behavior change.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 0.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 survi… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Title check The title "fix: remove next 40 unused local findings (batch 5)" accurately describes the changeset, which removes unused imports, constants, bindings, and uncalled private helpers across 22 files. It … Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Risk Surface Disclosed PASS. The PR description calls out the relevant risk surfaces: SDK pending-permission resolution, provider request context, and streaming/abort behavior. The diff confirms that production changes only… No Hidden Policy Change PASS. The reviewed diff contains unused-import, unused-binding, and dead-helper cleanup. The only permission-related line changes the loop binding from [toolUseId, pending]to[, pending]; it stil…Description check The pull request description follows the required template. It explains the changes and rationale, documents user-facing and maintainer impact, lists validation commands and results, records the sandb… Full details: Title check
-
Explanation
The title "fix: remove next 40 unused local findings (batch 5)" accurately describes the changeset, which removes unused imports, constants, bindings, and uncalled private helpers across 22 files. It follows a conventional prefix and is scoped to the change. It does not name the affected areas, but it is not vague or misleading.
Full details: Risk Surface Disclosed
-
Explanation
PASS. The PR description calls out the relevant risk surfaces: SDK pending-permission resolution, provider request context, and streaming/abort behavior. The diff confirms that production changes only remove unused bindings/imports or an unused private wrapper; the SDK loop still denies every pending prompt, and OpenAI/WebSearch routing behavior is unchanged. The description states no intended behavior change, and the current review reports zero actionable findings, so no blocker is identified.
Full details: No Hidden Policy Change
-
Explanation
PASS. The reviewed diff contains unused-import, unused-binding, and dead-helper cleanup. The only permission-related line changes the loop binding from
[toolUseId, pending]to[, pending]; it still resolves every pending prompt withbehavior: 'deny'and the same message. The request executor still receivesfastPath; only an unused local destructuring is removed. No product, trust-model, routing-default, telemetry/network, or permission-policy decision changes. Affected test registration counts remain unchanged.
Full details: Description check
-
Explanation
The pull request description follows the required template. It explains the changes and rationale, documents user-facing and maintainer impact, lists validation commands and results, records the sandbox limitation and CI status, and includes relevant notes. No critical information is missing.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.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 1 functions across 10 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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Summary
Fix the next 40 bottom findings from js/unused-local-variable, excluding the 160 IDs covered by #2451, #2453, #2454 and #2456. Selection preserves the managed API's default descending order captured before editing.
Remove unused imports, constants, bindings and uncalled private functions. Most deleted lines are unused streaming-test helpers left behind after test extraction, plus their now-unused private types/helpers. Active stall/idle helpers in the integration tests remain. No test registration or active assertion is removed.
Finding IDs: 2904, 2905, 2903, 2901, 2902, 2899, 2900, 2897, 2898, 2896, 2884, 2877, 2868, 2820, 2795, 2794, 2785, 2784, 2766, 2765, 2764, 2763, 2762, 2761, 2718, 2712, 2695, 2694, 2688, 2687, 2662, 2648, 2647, 2646, 2645, 2603, 2586, 2584, 2583, 2582.
Impact
REPL.tsxin fix: remove next 40 unused local findings (batch 4) #2456, which removes a different import. This PR does not include earlier batches' fixes. The open SDK environment PR feat(sdk): let a v2 session carry its own environment #2268 affects different behavior and does not cover this unused loop-key finding.Testing
dad5b8f5ba9142c75e5fde70a512e1661e243692; no prior batch's test results reused.bun install --frozen-lockfile;bun run check:static(ESLint, any budget, smoke build, deadcode);bun run typecheck:fast;bun run typecheck:type-tests(11 files);node bin/openclaude --version;NODE_DISABLE_COMPILE_CACHE=1 node bin/openclaude --version;git diff --check.bun run test ./scripts/openclaude-bin-heap.test.ts ./scripts/provider-launch.test.ts ./src/memdir/autoExtractFacts.test.ts ./src/memdir/vectorIndex.test.ts ./src/services/api/claude.lifecycle.test.ts ./src/services/api/claude.streamWatchdog.test.ts ./src/services/api/openaiShim/providerStreamInterruptionTrace.test.ts ./src/services/api/openaiShim/requestExecutor.integration.test.ts ./src/services/api/openaiShim/requestExecutor.test.ts ./src/services/api/openaiShim/streamControl.test.ts ./src/services/compact/compact.test.ts ./src/tools/ExitPlanModeTool/ExitPlanModeV2Tool.test.ts ./src/tools/WebSearchTool/providers/exa.test.ts ./src/utils/conversationArc.test.ts ./src/utils/knowledgeGraph.test.ts ./src/utils/providerValidation.test.ts ./src/utils/sessionStorage.atomicReplace.test.ts ./tests/sdk/sdk-v2-lifecycle.test.ts— 629 passed across 18 files, 1850 assertions.bun run test:conversation-arc— 1 passed, 9 assertions. Both guarded reports completed.git fetch https://github.com/Twigpine/openclaude.git main:bun run security:pr-scan -- --base FETCH_HEAD --head HEAD.ec6bc3fd5d75dae8c5657d16125a2f7ffd9c9ac1and this candidate inside the sandbox. Unchanged base tests and the complete candidate suite passed outside it; no source/assertion changes were needed.dad5b8f5ba9142c75e5fde70a512e1661e243692, including the required PR/install/VS Code aggregate checks, full unit shards, dedicated feature tests, supported Node launchers and CodeQL; three unrelated jobs skipped as expected. CodeRabbit review approved the same revision with no requested changes.Notes