refactor: command/query separation — extract CLI wrappers, shared output helper#373
Conversation
…put helper, test-filter module Extract 15 CLI display wrappers (~800 lines) from queries.js into queries-cli.js, reducing it by 26% (3433 → 2544 lines). Add shared outputResult() helper that replaces the 4–6 line JSON/NDJSON dispatch pattern repeated in every CLI wrapper. Extract isTestFile() into test-filter.js, breaking the circular-ish dependency where 17 modules imported it from queries.js. Add QUERY_OPTS CLI preset consolidating 7 repeated options across 14 commands (~98 lines removed). Zero breaking changes — all public API exports preserved via index.js re-exports. 569 tests pass, 0 lint errors. Impact: 34 functions changed, 170 affected
|
Claude finished @carlos-alm's task —— View job PR Review Complete ✅Reviewing PR #373: refactor: command/query separation — extract CLI wrappers, shared output helper Todo List
Phase 1: Root Cause Analysis ✅Problem Being Solved: This refactoring addresses code organization and maintainability in the queries.js file, which was becoming unwieldy at 3433 lines. The PR extracts:
Why This Approach: This is a legitimate separation of concerns - isolating CLI presentation logic from core data query functions. The approach maintains backward compatibility via re-exports and doesn't change function signatures. Risk Assessment: Low risk. This is purely organizational refactoring with maintained API compatibility. Phase 2: Configuration Change Detection ✅No risky configuration changes detected:
The only configuration-related code is the new Phase 3: Test Integrity ✅Tests are appropriately updated without being weakened:
Phase 4: Code Quality Review ✅Excellent refactoring quality: ✅ Strengths:
✅ Code Quality:
✅ No Issues Found:
Final Assessment |
Greptile SummaryThis PR is a well-structured refactor that separates CLI display concerns from pure data-query logic: 15 CLI wrappers are extracted from Two issues require attention before merging:
Confidence Score: 3/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
CLI["cli.js\n(QUERY_OPTS preset)"]
QCLI["queries-cli.js\n15 CLI wrappers"]
Q["queries.js\n*Data() functions"]
RF["result-formatter.js\noutputResult()"]
TF["test-filter.js\nisTestFile()"]
PAG["paginate.js\nprintNdjson()"]
IDX["index.js\nPublic API"]
CLI -->|"imports wrappers"| QCLI
QCLI -->|"calls *Data()"| Q
QCLI -->|"uses"| RF
RF -->|"delegates to"| PAG
Q -->|"re-exports isTestFile"| TF
IDX -->|"re-exports all"| QCLI
IDX -->|"re-exports all"| Q
IDX -->|"re-exports"| RF
IDX -->|"re-exports"| TF
style RF fill:#d4edda,stroke:#28a745
style TF fill:#d4edda,stroke:#28a745
style QCLI fill:#d4edda,stroke:#28a745
style CLI fill:#fff3cd,stroke:#ffc107
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
src/queries-cli.js
Outdated
|
|
||
| export function children(name, customDbPath, opts = {}) { | ||
| const data = childrenData(name, customDbPath, opts); | ||
| if (outputResult(data, null, opts)) return; |
There was a problem hiding this comment.
Silent NDJSON output when null field used
children stores its results under data.results (confirmed from childrenData returning { name, results } via paginateResult), but outputResult(data, null, opts) passes null as the streaming field. When opts.ndjson is true, printNdjson(data, null) accesses data["null"] (JS coerces null to string), which is undefined and not an array, so nothing gets streamed.
While the children CLI command doesn't currently expose --ndjson, the function is part of the public API exported from index.js. Any programmatic caller passing { ndjson: true } will receive empty output instead of streamed results. The field here should be 'results' to be consistent with the NDJSON contract.
| if (outputResult(data, null, opts)) return; | |
| if (outputResult(data, 'results', opts)) return; |
There was a problem hiding this comment.
Fixed: changed \ to \ in the \ function so NDJSON correctly streams the \ array.
src/queries-cli.js
Outdated
|
|
||
| export function moduleMap(customDbPath, limit = 20, opts = {}) { | ||
| const data = moduleMapData(customDbPath, limit, { noTests: opts.noTests }); | ||
| if (outputResult(data, null, opts)) return; |
There was a problem hiding this comment.
NDJSON field is null for a list-returning function
moduleMapData returns an object with a topNodes array. When opts.ndjson is true, outputResult(data, null, opts) calls printNdjson(data, null) which accesses data["null"] → undefined → nothing streamed. The map CLI command doesn't currently expose --ndjson, but as a public API export any programmatic caller using { ndjson: true } will get empty output.
Consider using 'topNodes' as the field for consistency, or explicitly documenting that NDJSON is unsupported for this function:
| if (outputResult(data, null, opts)) return; | |
| if (outputResult(data, 'topNodes', opts)) return; |
There was a problem hiding this comment.
Fixed: changed \ to \ in the \ function so NDJSON correctly streams the \ array.
|
Claude finished @carlos-alm's task —— View job PR Review Complete ✅Reviewing PR #373: refactor: command/query separation — extract CLI wrappers, shared output helper Todo List
Phase 1: Root Cause Analysis ✅Problem Being Solved: This refactoring addresses code organization and maintainability in the queries.js file, which was becoming unwieldy at 3433 lines. The PR extracts:
Why This Approach: This is a legitimate separation of concerns - isolating CLI presentation logic from core data query functions. The approach maintains backward compatibility via re-exports and doesn't change function signatures. Risk Assessment: Low risk. This is purely organizational refactoring with maintained API compatibility. Phase 2: Configuration Change Detection ✅No risky configuration changes detected:
The only configuration-related code is the new Phase 3: Test Integrity ✅Tests are appropriately updated without being weakened:
Phase 4: Code Quality Review ✅Excellent refactoring quality: ✅ Strengths:
✅ Code Quality:
Final Assessment |
Impact: 2 functions changed, 0 affected
|
Claude finished @carlos-alm's task —— View job PR Review Complete ✅Reviewing PR #373: refactor: command/query separation — extract CLI wrappers, shared output helper Todo List
Phase 1: Root Cause Analysis ✅Problem Being Solved: This refactoring addresses code organization and maintainability in the queries.js file, which was becoming unwieldy at 3433 lines. The PR extracts:
Why This Approach: This is a legitimate separation of concerns - isolating CLI presentation logic from core data query functions. The approach maintains backward compatibility via re-exports and doesn't change function signatures. Risk Assessment: Low risk. This is purely organizational refactoring with maintained API compatibility. Phase 2: Configuration Change Detection ✅No risky configuration changes detected:
The only configuration-related code is the new Phase 3: Test Integrity ✅Tests are appropriately updated without being weakened:
Phase 4: Code Quality Review ✅Excellent refactoring quality: ✅ Strengths:
✅ Code Quality:
✅ No Issues Found:
Final Assessment |
|
Claude finished @carlos-alm's task —— View job Claude finished @carlos-alm's task —— View job PR Review Complete ✅Reviewing PR #373: refactor: command/query separation — extract CLI wrappers, shared output helper Todo List
Phase 1: Root Cause Analysis ✅Problem Being Solved: This refactoring addresses code organization and maintainability in the queries.js file, which was becoming unwieldy at 3433 lines. The PR extracts:
Why This Approach: This is a legitimate separation of concerns - isolating CLI presentation logic from core data query functions. The approach maintains backward compatibility via re-exports and doesn't change function signatures. Risk Assessment: Low risk. This is purely organizational refactoring with maintained API compatibility. Phase 2: Configuration Change Detection ✅No risky configuration changes detected:
The only configuration-related code is the new Phase 3: Test Integrity ✅Tests are appropriately updated without being weakened:
Phase 4: Code Quality Review ✅Excellent refactoring quality: ✅ Strengths:
✅ Code Quality:
✅ No Issues Found:
Final Assessment |
|
All review feedback addressed:
|
|
Claude finished @carlos-alm's task —— View job PR Review Complete ✅Reviewing PR #373: refactor: command/query separation — extract CLI wrappers, shared output helper Todo List
Phase 1: Root Cause Analysis ✅Problem Being Solved: This refactoring addresses code organization and maintainability in the queries.js file, which was becoming unwieldy at 3433 lines. The PR extracts:
Why This Approach: This is a legitimate separation of concerns - isolating CLI presentation logic from core data query functions. The approach maintains backward compatibility via re-exports and doesn't change function signatures. Risk Assessment: Low risk. This is purely organizational refactoring with maintained API compatibility. Phase 2: Configuration Change Detection ✅No risky configuration changes detected:
The only configuration-related code is the new Phase 3: Test Integrity ✅Tests are appropriately updated without being weakened:
Phase 4: Code Quality Review ✅Excellent refactoring quality: ✅ Strengths:
✅ Code Quality:
✅ Previous Review Issues Fixed:
✅ No Issues Found:
Final Assessment |
|
Claude finished @carlos-alm's task —— View job PR Review Complete ✅Reviewing PR #373: refactor: command/query separation — extract CLI wrappers, shared output helper Todo List
Phase 1: Root Cause Analysis ✅Problem Being Solved: This refactoring addresses code organization and maintainability in the queries.js file, which was becoming unwieldy at 3433 lines. The PR extracts:
Why This Approach: This is a legitimate separation of concerns - isolating CLI presentation logic from core data query functions. The approach maintains backward compatibility via re-exports and doesn't change function signatures. Risk Assessment: Low risk. This is purely organizational refactoring with maintained API compatibility. Phase 2: Configuration Change Detection ✅No risky configuration changes detected:
The only configuration-related code is the new Phase 3: Test Integrity ✅Tests are appropriately updated without being weakened:
Phase 4: Code Quality Review ✅Excellent refactoring quality: ✅ Strengths:
✅ Code Quality:
✅ Previous Review Issues Fixed:
✅ No Issues Found:
Final Assessment |
1 participant
Summary
queries.js→queries-cli.js, reducing queries.js by 26% (3433 → 2544 lines)outputResult()helper inresult-formatter.js— replaces the 4–6 line JSON/NDJSON dispatch pattern repeated in every CLI wrapperisTestFile()intotest-filter.js— breaks the circular-ish dependency where 17 modules imported it from queries.jsQUERY_OPTSCLI preset — consolidates 7 repeated.option()calls across 14 commands (~98 lines removed)Details
New files
src/result-formatter.jsoutputResult(data, field, opts)— returnstrueif JSON/NDJSON handledsrc/test-filter.jsisTestFile()+TEST_PATTERN— standalone, no deps on queries.jssrc/queries-cli.jsWhat stays unchanged
*Data()functions remain inqueries.js— MCP, batch, and programmatic API untouchedindex.jsre-exports — zero breaking changesBug fix
cfg.jswas callingprintNdjson(data.results)(passing array directly) instead ofprintNdjson(data, 'results')— fixed viaoutputResultTest plan
biome check— 0 errors (1 pre-existing warning)codegraph --help— all commands listed correctlycodegraph impact --help— shows QUERY_OPTS options🤖 Generated with Claude Code