fix(deep): remove duplicate --deep block - closes #32#81
Merged
Conversation
The --deep post-processing block in codelens.py was implemented twice for the same set of commands (dead-code, query, impact, smell, complexity). Both blocks ran in sequence when --deep was set: - Block 1 (L1024-1059): used HybridEngine(workspace, deep=True) directly - Block 2 (L1070-1121): used create_hybrid_engine() factory + add_confidence_to_result() Each block instantiated a fresh HybridEngine, so the engine was created TWICE per --deep invocation — doubling LSP subprocess calls and potentially double-counting findings in confidence_distribution. Fix: delete Block 1 entirely, keep Block 2 (strictly more capable: also handles complexity, adds confidence distribution). Folded Block 1's 'unsupported command' hint into Block 2's elif branch so users running --deep on unsupported commands still get a clear message. 2 regression tests added to tests/test_hybrid_engine.py::TestDeepSingleInvocation: - test_smell_deep_invokes_create_hybrid_engine_once: patches create_hybrid_engine, calls main() in-process, asserts call_count == 1 - test_deep_unsupported_command_sets_hint: verifies --deep on unsupported command sets deep_analysis_hint instead of crashing Verified: - grep 'Post-processing: --deep' codelens.py returns 1 (was 2) - smell --deep still produces valid JSON output - Both new tests pass
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix for P0 issue #32. The
--deeppost-processing block incodelens.pywas implemented twice for the same set of commands (dead-code,query,impact,smell,complexity). Both blocks ran in sequence when--deepwas set:HybridEngine(workspace, deep=True)directlycreate_hybrid_engine()factory +add_confidence_to_result()Each block instantiated a fresh
HybridEngine, so the engine was created twice per--deepinvocation — doubling LSP subprocess calls and potentially double-counting findings inconfidence_distribution.Fix
complexity, didn't calladd_confidence_to_result)elifbranch, so users running--deepon commands outside the supported list still get a cleardeep_analysis_hintmessageVerification
grep 'Post-processing: --deep' scripts/codelens.pyreturns 1 (was 2)codelens smell /tmp --deep --format jsonstill produces valid JSON outputcodelens symbols foo /tmp --deep --format json(unsupported command) now setsdeep_analysis: false+deep_analysis_hintinstead of silently doing nothingRegression tests
2 tests added to
tests/test_hybrid_engine.py::TestDeepSingleInvocation:test_smell_deep_invokes_create_hybrid_engine_once— patcheshybrid_engine.create_hybrid_enginewith aMagicMock, callsmain()in-process (not subprocess — mocks don't cross process boundaries), assertsmock_create.call_count == 1. This is the direct regression guard for issue [BUG-02] --deep flag runs HybridEngine twice — LSP findings double-counted #32.test_deep_unsupported_command_sets_hint— runscodelens symbols foo --deep(symbols is NOT in the supported list), asserts output containsdeep_analysis: false+deep_analysis_hintmentioning the command name. Verifies the "unsupported command" behavior was preserved from deleted Block 1.Both tests pass.
Files
scripts/codelens.py(Block 1 deleted, elif branch added to Block 2, +37/-37 LOC net but cleaner)tests/test_hybrid_engine.py(+87 LOC, 2 new tests)Related