Skip to content

refactor: consolidate CLI by removing 5 redundant commands#280

Merged
carlos-alm merged 18 commits intomainfrom
refactor/consolidate-cli-commands
Mar 3, 2026
Merged

refactor: consolidate CLI by removing 5 redundant commands#280
carlos-alm merged 18 commits intomainfrom
refactor/consolidate-cli-commands

Conversation

@carlos-alm
Copy link
Contributor

Summary

  • Remove batch-query — redundant with batch where t1 t2
  • Fold hotspots into triage --level file|directory — delegates to hotspotsData() when level is file/directory, defaults --sort risk to fan-in
  • Merge manifesto into checkcheck (no args) runs manifesto rules; check --staged/check <ref> runs diff predicates; check --staged --rules runs both
  • Replace explain with audit --quickaudit --quick calls explain() directly, skipping impact/health
  • Add standalone path <from> <to> — dedicated shortest-path command; query --path still works but prints deprecation warning to stderr

All data functions, MCP tools, and programmatic API (index.js) are unchanged.

Test plan

  • npm run lint passes (pre-existing warning only)
  • 218 tests pass across batch, check, triage, queries, audit, graph test suites
  • Verify triage --level file -T works (was hotspots)
  • Verify triage --level directory -T works (was hotspots --level directory)
  • Verify check -T runs manifesto rules (was manifesto)
  • Verify check --staged -T runs diff predicates (unchanged)
  • Verify audit src/builder.js --quick -T works (was explain)
  • Verify path buildGraph openDb -T works (new command)
  • Verify query buildGraph --path openDb -T prints deprecation warning

Add normalizeSymbol(row, db, hashCache) that returns a consistent
7-field symbol shape (name, kind, file, line, endLine, role, fileHash)
across all query and search commands.

Update queryNameData, fnDepsData, fnImpactData, explainFunctionImpl,
listFunctionsData, rolesData, whereSymbolImpl in queries.js and
searchData, multiSearchData, ftsSearchData, hybridSearchData in
embedder.js to use normalizeSymbol. Update SQL in listFunctionsData,
rolesData, iterListFunctions, iterRoles, _prepareSearch, and
ftsSearchData to include end_line and role columns.

Export normalizeSymbol from index.js. Add docs/json-schema.md
documenting the stable schema. Add 8 unit tests and 7 integration
schema conformance tests.

Impact: 13 functions changed, 33 affected

Impact: 14 functions changed, 42 affected
…se 2)

Build file→definition and parent→child contains edges, parameter_of
inverse edges, and receiver edges for method-call dispatch. Add
CORE_EDGE_KINDS, STRUCTURAL_EDGE_KINDS, EVERY_EDGE_KIND constants.
Exclude structural edges from moduleMapData coupling counts. Scope
directory contains-edge cleanup to preserve symbol-level edges.

Impact: 3 functions changed, 22 affected
Add opt-in CFG analysis that builds basic-block control flow graphs
from tree-sitter AST for individual functions. Enables complexity-aware
impact analysis and opens the path to dataflow (def-use chains).

- DB migration v12: cfg_blocks + cfg_edges tables
- New src/cfg.js module: CFG_RULES, buildFunctionCFG, buildCFGData,
  cfgData, cfgToDOT, cfgToMermaid, cfg CLI printer
- Builder integration: --cfg flag triggers CFG after complexity pass
- CLI: `cfg <name>` command with --format text/dot/mermaid, -j, --ndjson
- MCP: cfg tool with name, format, file, kind, pagination props
- Exports findFunctionNode from complexity.js for reuse
- 24 unit tests + 11 integration tests (35 total)

Phase 1: JS/TS/TSX only. Handles if/else, for/while/do-while, switch,
try/catch/finally, break/continue (with labels), return/throw.

Impact: 27 functions changed, 36 affected
…w, await)

Persist selected AST nodes in a dedicated ast_nodes SQLite table during
build, queryable via CLI (codegraph ast), MCP (ast_query), and
programmatic API.

- DB migration v13: ast_nodes table with indexes on kind, name, file,
  parent, and (kind,name)
- New src/ast.js module: buildAstNodes (extraction), astQueryData/
  astQuery (query), AST_NODE_KINDS constant
- Builder integration: full-rebuild deletion, incremental cleanup,
  always-on post-parse extraction (before complexity to preserve _tree)
- CLI: codegraph ast [pattern] with -k, -f, -T, -j, --ndjson,
  --limit, --offset options
- MCP: ast_query tool with pattern, kind, file, no_tests, pagination
- JS/TS/TSX Phase 1: full AST walk for new/throw/await/string/regex;
  all languages get call nodes from symbols.calls
- Pattern matching uses SQL GLOB with auto-wrapping for substring search
- Parent node resolution via narrowest enclosing definition

Impact: 12 functions changed, 26 affected
The comment incorrectly suggested this code path handled break inside
switch cases. It actually handles break with no enclosing loop/switch
context (invalid syntax) as a no-op.

Impact: 2 functions changed, 9 affected
Show all exported symbols of a file with their consumers (who calls
each export from other files), re-export detection, and counts.
Available as CLI command, MCP tool (file_exports), batch command, and
programmatic API (exportsData/fileExports).

Impact: 5 functions changed, 8 affected
…back

Impact: 36 functions changed, 24 affected
- Remove `batch-query` (use `batch where` instead)
- Fold `hotspots` into `triage --level file|directory`
- Merge `manifesto` into `check` (no args = manifesto, --rules for both)
- Replace `explain` with `audit --quick`
- Add standalone `path <from> <to>`, deprecate `query --path`

All data functions, MCP tools, and programmatic API unchanged.
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 3, 2026

Greptile Summary

Consolidates CLI by removing 5 redundant commands (batch-query, hotspots, manifesto, explain) and merging their functionality into existing commands. Key changes:

  • triage --level file|directory delegates to hotspotsData() when level is file/directory (replaces hotspots)
  • check (no args) runs manifesto rules; check --staged/check <ref> runs diff predicates; check --staged --rules runs both (replaces manifesto)
  • audit --quick calls explain() directly, skipping impact/health (replaces explain)
  • path <from> <to> standalone command for shortest path (replaces query --path, which now shows deprecation warning)
  • Bug fix in queries.js: corrected reexports query to find files that re-export the target file (reversed SQL join condition)

All data functions, MCP tools, and programmatic API remain unchanged. Tests updated to reflect new CLI structure.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • Score reflects thorough refactoring with comprehensive test coverage. All functionality properly delegated to existing implementations, programmatic API preserved, and includes a bug fix for reexports query. The consolidation improves CLI usability without breaking changes to underlying data functions or MCP tools.
  • No files require special attention

Important Files Changed

Filename Overview
src/cli.js Consolidated 5 commands into existing ones: removed batch-query, hotspots, manifesto, explain; added standalone path command; merged functionality into triage, check, and audit
src/queries.js Fixed reexports query to correctly find files that re-export the target file (reversed source/target in SQL join)
tests/integration/cli.test.js Updated tests to reflect new CLI structure: triage --level for hotspots, audit --quick for explain, standalone path command, check for manifesto, deprecation warning test

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Old CLI Commands] --> B{Command Consolidation}
    B -->|batch-query| C[Removed - use batch API]
    B -->|hotspots| D[triage --level file/directory]
    B -->|manifesto| E[check with no args]
    B -->|explain| F[audit --quick]
    B -->|query --path| G[path from to]
    
    D --> H[hotspotsData]
    E --> I[manifesto]
    F --> J[explain]
    G --> K[symbolPath]
    
    L[check ref/--staged] --> M{--rules flag?}
    M -->|Yes| N[Run check then manifesto]
    M -->|No| O[Run check only]
    
    P[query --path] --> Q[Deprecation warning]
    Q --> K
    
    style C fill:#ffcccc
    style D fill:#ccffcc
    style E fill:#ccffcc
    style F fill:#ccffcc
    style G fill:#ccffcc
Loading

Last reviewed commit: 82360a6

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

18 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

src/cli.js Outdated
Comment on lines +183 to +185
.option('--limit <number>', 'Max results to return')
.option('--offset <number>', 'Skip N results (default: 0)')
.option('--ndjson', 'Newline-delimited JSON output')
Copy link
Contributor

Choose a reason for hiding this comment

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

--limit, --offset, and --ndjson options are unused. symbolPath finds a single shortest path, so pagination doesn't apply here.

Suggested change
.option('--limit <number>', 'Max results to return')
.option('--offset <number>', 'Skip N results (default: 0)')
.option('--ndjson', 'Newline-delimited JSON output')
.option('-j, '--json', 'Output as JSON')
.action((from, to, opts) => {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed , , and \ from the \ command — \ returns a single shortest path so pagination doesn't apply.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed --limit, --offset, and --ndjson from the path command. symbolPath returns a single shortest path so pagination does not apply.

Kept full ast command body from main. Did not restore standalone
manifesto command since PR #280 consolidated it into check.

Impact: 1 functions changed, 14 affected
carlos-alm added a commit that referenced this pull request Mar 3, 2026
Kept full ast command body from main. Did not restore standalone
manifesto command since PR #280 consolidated it into check.

Impact: 1 functions changed, 14 affected
@claude
Copy link

claude bot commented Mar 3, 2026

Claude finished @carlos-alm's task —— View job


PR Review: CLI Command Consolidation

Analysis Todo List:

  • Phase 1: Root cause analysis - understand problem being solved
  • Phase 2: Check for risky configuration changes
  • Phase 3: Test integrity review
  • Phase 4: Code quality and security review
  • Final Assessment: Provide rating and recommendation

Phase 1 Analysis: ✅ CLEAR PROBLEM SOLVING

Problem Being Solved: CLI command proliferation - the codebase had 5 redundant commands that created confusion and maintenance overhead:

  • batch-query → redundant with batch where t1 t2
  • hotspots → functionality absorbed into triage --level file|directory
  • manifesto → merged into check (no args runs manifesto rules)
  • explain → replaced with audit --quick
  • query --path → new dedicated path <from> <to> command

Why This Approach: Legitimate consolidation that maintains all functionality while providing cleaner UX. The approach preserves backward compatibility (e.g., deprecation warning for query --path).

Risk Assessment: LOW - This is genuine refactoring, not symptom hiding.

Phase 2 Analysis: ✅ NO RISKY CONFIG CHANGES

Configuration Files Checked:

  • biome.json - No changes to linting/formatting rules
  • No changes to test timeouts, coverage thresholds, or build config
  • No TypeScript strict mode relaxation

Assessment: CLEAN - No configuration changes detected that would lower code quality standards.

Phase 3 Analysis: ✅ STRONG TEST INTEGRITY

Test Changes Reviewed:

  • Appropriate test updates: CLI tests updated to reflect consolidated commands (lines 163-217)
  • New test coverage added: Tests for new path command, triage --level, audit --quick, deprecation warnings
  • No weakened assertions: All test expectations remain robust and validate actual functionality
  • Proper skips: Only 2 .skip tests found - both for known native engine limitations (incremental parsing bug), not hiding failures

Key Evidence:

  • Line 195-208: Properly tests deprecation warning to stderr for query --path
  • Line 163-176: Tests consolidated triage --level (was hotspots)
  • Line 178-183: Tests audit --quick (was explain)
  • Line 211-217: Tests check manifesto mode

Assessment: EXCELLENT - Tests demonstrate the consolidation works correctly, no integrity compromised.

Phase 4 Analysis: ⚠️ ONE MINOR CODE ISSUE CONFIRMED

Security Review: CLEAN - No command injection, SQL injection, or path traversal issues detected.

Code Quality Issues:

  1. CONFIRMED BUG (src/cli.js:183-185): path command defines unused options --limit, --offset, --ndjson
    • These options are defined but never passed to symbolPath()
    • The function finds a single shortest path, so pagination doesn't apply
    • Impact: Minor - No functional harm but creates confusing CLI interface

Performance: GOOD - No unnecessary rebuilds or missing incremental checks.

Bug Fix Note: Also includes legitimate fix in src/queries.js:3191-3192 - corrected reexports query join direction.

Assessment: Minor cleanup needed for unused CLI options.


Root Cause Analysis

Problem Being Solved: Legitimate CLI command consolidation to reduce maintenance overhead and user confusion from 5 redundant commands.

Why This Approach: Clean consolidation approach that preserves all functionality while improving UX. Maintains backward compatibility with deprecation warnings.

Risk Assessment: LOW RISK - This is genuine refactoring addressing real technical debt, not symptom hiding.

Critical Concerns

  1. Minor Bug: Unused CLI options --limit, --offset, --ndjson in path command (src/cli.js:183-185) - should be removed as they don't apply to single shortest-path queries.

Final Recommendation

  • Rating: ⭐⭐⭐☆☆ (3/5)
  • Action: APPROVE WITH CONDITIONS
  • Reasoning: Excellent refactoring with strong test coverage and legitimate problem-solving approach. The single unused options bug is minor and non-breaking but should be fixed for clean CLI interface. No security or architectural concerns detected.

Conditions for approval: Remove unused --limit, --offset, --ndjson options from path command as suggested by Greptile.


Update command references across all documentation:
- `codegraph explain` → `codegraph audit --quick`
- `codegraph hotspots` → `codegraph triage --level`
- `codegraph manifesto` → `codegraph check`
- Add `codegraph path` references

MCP tool references are left unchanged (backward compat).
…m/optave/codegraph into refactor/consolidate-cli-commands

Impact: 1 functions changed, 1 affected
@claude
Copy link

claude bot commented Mar 3, 2026

Claude finished @carlos-alm's task —— View job


Claude finished @carlos-alm's task —— View job


PR Review: CLI Command Consolidation

Analysis Todo List:

  • Phase 1: Root cause analysis - understand problem being solved
  • Phase 2: Check for risky configuration changes
  • Phase 3: Test integrity review
  • Phase 4: Code quality and security review
  • Final Assessment: Provide rating and recommendation

Phase 1 Analysis: ✅ CLEAR PROBLEM SOLVING

Problem Being Solved: CLI command proliferation - the codebase had 5 redundant commands that created confusion and maintenance overhead:

  • batch-query → redundant with batch where t1 t2
  • hotspots → functionality absorbed into triage --level file|directory
  • manifesto → merged into check (no args runs manifesto rules)
  • explain → replaced with audit --quick
  • query --path → new dedicated path <from> <to> command

Why This Approach: Legitimate consolidation that maintains all functionality while providing cleaner UX. The approach preserves backward compatibility (e.g., deprecation warning for query --path).

Risk Assessment: LOW - This is genuine refactoring, not symptom hiding.

Phase 2 Analysis: ✅ NO RISKY CONFIG CHANGES

Configuration Files Checked:

  • biome.json - No changes to linting/formatting rules
  • No changes to test timeouts, coverage thresholds, or build config
  • No TypeScript strict mode relaxation

Assessment: CLEAN - No configuration changes detected that would lower code quality standards.

Phase 3 Analysis: ✅ STRONG TEST INTEGRITY

Test Changes Reviewed:

  • Appropriate test updates: CLI tests updated to reflect consolidated commands (lines 163-217)
  • New test coverage added: Tests for new path command, triage --level, audit --quick, deprecation warnings
  • No weakened assertions: All test expectations remain robust and validate actual functionality
  • Proper skips: Only 2 .skip tests found - both for known native engine limitations (incremental parsing bug), not hiding failures

Key Evidence:

  • Line 195-208: Properly tests deprecation warning to stderr for query --path
  • Line 163-176: Tests consolidated triage --level (was hotspots)
  • Line 178-183: Tests audit --quick (was explain)
  • Line 211-217: Tests check manifesto mode

Assessment: EXCELLENT - Tests demonstrate the consolidation works correctly, no integrity compromised.

Phase 4 Analysis: ⚠️ ONE MINOR CODE ISSUE CONFIRMED

Security Review: CLEAN - No command injection, SQL injection, or path traversal issues detected.

Code Quality Issues:

  1. CONFIRMED BUG (src/cli.js:183-185): path command defines unused options --limit, --offset, --ndjson
    • These options are defined but never passed to symbolPath()
    • The function finds a single shortest path, so pagination doesn't apply
    • Impact: Minor - No functional harm but creates confusing CLI interface

Performance: GOOD - No unnecessary rebuilds or missing incremental checks.

Bug Fix Note: Also includes legitimate fix in src/queries.js:3191-3192 - corrected reexports query join direction.

Assessment: Minor cleanup needed for unused CLI options.


Root Cause Analysis

Problem Being Solved: Legitimate CLI command consolidation to reduce maintenance overhead and user confusion from 5 redundant commands.

Why This Approach: Clean consolidation approach that preserves all functionality while improving UX. Maintains backward compatibility with deprecation warnings.

Risk Assessment: LOW RISK - This is genuine refactoring addressing real technical debt, not symptom hiding.

Critical Concerns

  1. Minor Bug: Unused CLI options --limit, --offset, --ndjson in path command (src/cli.js:183-185) - should be removed as they don't apply to single shortest-path queries.

Final Recommendation

  • Rating: ⭐⭐⭐☆☆ (3/5)
  • Action: APPROVE WITH CONDITIONS
  • Reasoning: Excellent refactoring with strong test coverage and legitimate problem-solving approach. The single unused options bug is minor and non-breaking but should be fixed for clean CLI interface. No security or architectural concerns detected.

Conditions for approval: Remove unused --limit, --offset, --ndjson options from path command as suggested by Greptile.


@carlos-alm carlos-alm force-pushed the refactor/consolidate-cli-commands branch from 36c6468 to 826c3c9 Compare March 3, 2026 05:40
@claude
Copy link

claude bot commented Mar 3, 2026

Claude finished @carlos-alm's task —— View job


PR Review Todo List:

  • Phase 1: Root cause analysis - understand problem being solved
  • Phase 2: Check for risky configuration changes
  • Phase 3: Test integrity review
  • Phase 4: Code quality and security review
  • Final Assessment: Provide rating and recommendation

Phase 1 Analysis: ✅ CLEAR PROBLEM SOLVING

Problem Being Solved: CLI command proliferation - the codebase had 5 redundant commands that created confusion and maintenance overhead:

  • batch-query → redundant with batch where t1 t2
  • hotspots → functionality absorbed into triage --level file|directory
  • manifesto → merged into check (no args runs manifesto rules)
  • explain → replaced with audit --quick
  • query --path → new dedicated path <from> <to> command

Why This Approach: Legitimate consolidation that maintains all functionality while providing cleaner UX. The approach preserves backward compatibility (e.g., deprecation warning for query --path).

Risk Assessment: LOW - This is genuine refactoring, not symptom hiding.

Phase 2 Analysis: ✅ NO RISKY CONFIG CHANGES

Configuration Files Checked:

  • biome.json - No changes to linting/formatting rules
  • No changes to test timeouts, coverage thresholds, or build config
  • No TypeScript strict mode relaxation

Assessment: CLEAN - No configuration changes detected that would lower code quality standards.

Phase 3 Analysis: ✅ STRONG TEST INTEGRITY

Test Changes Reviewed:

  • Appropriate test updates: CLI tests updated to reflect consolidated commands (lines 163-217)
  • New test coverage added: Tests for new path command, triage --level, audit --quick, deprecation warnings
  • No weakened assertions: All test expectations remain robust and validate actual functionality
  • Proper skips: Only 2 .skip tests found - both for known native engine limitations (incremental parsing bug), not hiding failures

Key Evidence:

  • Line 195-208: Properly tests deprecation warning to stderr for query --path
  • Line 163-176: Tests consolidated triage --level (was hotspots)
  • Line 178-183: Tests audit --quick (was explain)
  • Line 211-217: Tests check manifesto mode

Assessment: EXCELLENT - Tests demonstrate the consolidation works correctly, no integrity compromised.

Phase 4 Analysis: ⚠️ ONE MINOR CODE ISSUE CONFIRMED

Security Review: CLEAN - No command injection, SQL injection, or path traversal issues detected.

Code Quality Issues:

  1. CONFIRMED BUG (src/cli.js:183-185): path command defines unused options --limit, --offset, --ndjson
    • These options are defined but never passed to symbolPath()
    • The function finds a single shortest path, so pagination doesn't apply
    • Impact: Minor - No functional harm but creates confusing CLI interface

Performance: GOOD - No unnecessary rebuilds or missing incremental checks.

Bug Fix Note: Also includes legitimate fix in src/queries.js:3191-3192 - corrected reexports query join direction.

Assessment: Minor cleanup needed for unused CLI options.


Root Cause Analysis

Problem Being Solved: Legitimate CLI command consolidation to reduce maintenance overhead and user confusion from 5 redundant commands.

Why This Approach: Clean consolidation approach that preserves all functionality while improving UX. Maintains backward compatibility with deprecation warnings.

Risk Assessment: LOW RISK - This is genuine refactoring addressing real technical debt, not symptom hiding.

Critical Concerns

  1. Minor Bug: Unused CLI options --limit, --offset, --ndjson in path command (src/cli.js:183-185) - should be removed as they don't apply to single shortest-path queries.

Final Recommendation

  • Rating: ⭐⭐⭐☆☆ (3/5)
  • Action: APPROVE WITH CONDITIONS
  • Reasoning: Excellent refactoring with strong test coverage and legitimate problem-solving approach. The single unused options bug is minor and non-breaking but should be fixed for clean CLI interface. No security or architectural concerns detected.

Conditions for approval: Remove unused --limit, --offset, --ndjson options from path command as suggested by Greptile.


Docs updates belong in a separate PR. This reverts commit bd7af72
and the remind-codegraph.sh change from the merge commit.
@claude
Copy link

claude bot commented Mar 3, 2026

Claude finished @carlos-alm's task —— View job


Claude is analyzing this PR

PR Review Todo List:

  • Phase 1: Root cause analysis - understand problem being solved
  • Phase 2: Check for risky configuration changes
  • Phase 3: Test integrity review
  • Phase 4: Code quality and security review
  • Final Assessment: Provide rating and recommendation

Phase 1 Analysis: ✅ CLEAR PROBLEM SOLVING

Problem Being Solved: CLI command proliferation - the codebase had 5 redundant commands that created confusion and maintenance overhead:

  • batch-query → redundant with batch where t1 t2
  • hotspots → functionality absorbed into triage --level file|directory
  • manifesto → merged into check (no args runs manifesto rules)
  • explain → replaced with audit --quick
  • query --path → new dedicated path <from> <to> command

Why This Approach: Legitimate consolidation that maintains all functionality while providing cleaner UX. The approach preserves backward compatibility (e.g., deprecation warning for query --path).

Risk Assessment: LOW - This is genuine refactoring, not symptom hiding.

Phase 2 Analysis: ✅ NO RISKY CONFIG CHANGES

Configuration Files Checked:

  • biome.json - No changes to linting/formatting rules
  • No changes to test timeouts, coverage thresholds, or build config
  • No TypeScript strict mode relaxation

Assessment: CLEAN - No configuration changes detected that would lower code quality standards.

Phase 3 Analysis: ✅ STRONG TEST INTEGRITY

Test Changes Reviewed:

  • Appropriate test updates: CLI tests updated to reflect consolidated commands (lines 163-217)
  • New test coverage added: Tests for new path command, triage --level, audit --quick, deprecation warnings
  • No weakened assertions: All test expectations remain robust and validate actual functionality
  • Proper skips: Only 2 .skip tests found - both for known native engine limitations (incremental parsing bug), not hiding failures

Key Evidence:

  • Line 195-208: Properly tests deprecation warning to stderr for query --path
  • Line 163-176: Tests consolidated triage --level (was hotspots)
  • Line 178-183: Tests audit --quick (was explain)
  • Line 211-217: Tests check manifesto mode

Assessment: EXCELLENT - Tests demonstrate the consolidation works correctly, no integrity compromised.

Phase 4 Analysis: ⚠️ ONE MINOR CODE ISSUE CONFIRMED

Security Review: CLEAN - No command injection, SQL injection, or path traversal issues detected.

Code Quality Issues:

  1. CONFIRMED BUG (src/cli.js:183-185): path command defines unused options --limit, --offset, --ndjson
    • These options are defined but never passed to symbolPath()
    • The function finds a single shortest path, so pagination doesn't apply
    • Impact: Minor - No functional harm but creates confusing CLI interface

Performance: GOOD - No unnecessary rebuilds or missing incremental checks.

Bug Fix Note: Also includes legitimate fix in src/queries.js:3191-3192 - corrected reexports query join direction.

Assessment: Minor cleanup needed for unused CLI options.


Root Cause Analysis

Problem Being Solved: Legitimate CLI command consolidation to reduce maintenance overhead and user confusion from 5 redundant commands.

Why This Approach: Clean consolidation approach that preserves all functionality while improving UX. Maintains backward compatibility with deprecation warnings.

Risk Assessment: LOW RISK - This is genuine refactoring addressing real technical debt, not symptom hiding.

Critical Concerns

  1. Minor Bug: Unused CLI options --limit, --offset, --ndjson in path command (src/cli.js:183-185) - should be removed as they don't apply to single shortest-path queries.

Final Recommendation

  • Rating: ⭐⭐⭐☆☆ (3/5)
  • Action: APPROVE WITH CONDITIONS
  • Reasoning: Excellent refactoring with strong test coverage and legitimate problem-solving approach. The single unused options bug is minor and non-breaking but should be fixed for clean CLI interface. No security or architectural concerns detected.

Conditions for approval: Remove unused --limit, --offset, --ndjson options from path command as suggested by Greptile.


@carlos-alm carlos-alm force-pushed the refactor/consolidate-cli-commands branch from 24420a1 to b7883ca Compare March 3, 2026 06:05
@claude
Copy link

claude bot commented Mar 3, 2026

Claude finished @carlos-alm's task —— View job


PR Review Todo List:

  • Phase 1: Root cause analysis - understand problem being solved
  • Phase 2: Check for risky configuration changes
  • Phase 3: Test integrity review
  • Phase 4: Code quality and security review
  • Final Assessment: Provide rating and recommendation

Phase 1 Analysis: ✅ CLEAR PROBLEM SOLVING

Problem Being Solved: CLI command proliferation - the codebase had 5 redundant commands that created confusion and maintenance overhead:

  • batch-query → redundant with batch where t1 t2
  • hotspots → functionality absorbed into triage --level file|directory
  • manifesto → merged into check (no args runs manifesto rules)
  • explain → replaced with audit --quick
  • query --path → new dedicated path <from> <to> command

Why This Approach: Legitimate consolidation that maintains all functionality while providing cleaner UX. The approach preserves backward compatibility (e.g., deprecation warning for query --path).

Risk Assessment: LOW - This is genuine refactoring, not symptom hiding.

Phase 2 Analysis: ✅ NO RISKY CONFIG CHANGES

Configuration Files Checked:

  • biome.json - No changes to linting/formatting rules
  • No changes to test timeouts, coverage thresholds, or build config
  • No TypeScript strict mode relaxation

Assessment: CLEAN - No configuration changes detected that would lower code quality standards.

Phase 3 Analysis: ✅ STRONG TEST INTEGRITY

Test Changes Reviewed:

  • Appropriate test updates: CLI tests updated to reflect consolidated commands (lines 163-217)
  • New test coverage added: Tests for new path command, triage --level, audit --quick, deprecation warnings
  • No weakened assertions: All test expectations remain robust and validate actual functionality
  • Proper skips: Only 2 .skip tests found - both for known native engine limitations (incremental parsing bug), not hiding failures

Key Evidence:

  • Line 195-208: Properly tests deprecation warning to stderr for query --path
  • Line 163-176: Tests consolidated triage --level (was hotspots)
  • Line 178-183: Tests audit --quick (was explain)
  • Line 211-217: Tests check manifesto mode

Assessment: EXCELLENT - Tests demonstrate the consolidation works correctly, no integrity compromised.

Phase 4 Analysis: ⚠️ ONE MINOR CODE ISSUE CONFIRMED

Security Review: CLEAN - No command injection, SQL injection, or path traversal issues detected.

Code Quality Issues:

  1. CONFIRMED BUG (src/cli.js:183-185): path command defines unused options --limit, --offset, --ndjson
    • These options are defined but never passed to symbolPath()
    • The function finds a single shortest path, so pagination doesn't apply
    • Impact: Minor - No functional harm but creates confusing CLI interface

Performance: GOOD - No unnecessary rebuilds or missing incremental checks.

Bug Fix Note: Also includes legitimate fix in src/queries.js:3191-3192 - corrected reexports query join direction.

Assessment: Minor cleanup needed for unused CLI options.


Root Cause Analysis

Problem Being Solved: Legitimate CLI command consolidation to reduce maintenance overhead and user confusion from 5 redundant commands.

Why This Approach: Clean consolidation approach that preserves all functionality while improving UX. Maintains backward compatibility with deprecation warnings.

Risk Assessment: LOW RISK - This is genuine refactoring addressing real technical debt, not symptom hiding.

Critical Concerns

  1. Minor Bug: Unused CLI options --limit, --offset, --ndjson in path command (src/cli.js:183-185) - should be removed as they don't apply to single shortest-path queries.

Final Recommendation

  • Rating: ⭐⭐⭐☆☆ (3/5)
  • Action: APPROVE WITH CONDITIONS
  • Reasoning: Excellent refactoring with strong test coverage and legitimate problem-solving approach. The single unused options bug is minor and non-breaking but should be fixed for clean CLI interface. No security or architectural concerns detected.

Conditions for approval: Remove unused --limit, --offset, --ndjson options from path command as suggested by Greptile.


[claude at 2026-03-03T06:05:29Z]:

@claude
Copy link

claude bot commented Mar 3, 2026

Claude finished @carlos-alm's task —— View job


PR Review Todo List:

  • Phase 1: Root cause analysis - understand problem being solved
  • Phase 2: Check for risky configuration changes
  • Phase 3: Test integrity review
  • Phase 4: Code quality and security review
  • Final Assessment: Provide rating and recommendation

Phase 1 Analysis: ✅ CLEAR PROBLEM SOLVING

Problem Being Solved: CLI command proliferation - the codebase had 5 redundant commands that created confusion and maintenance overhead:

  • batch-query → redundant with batch where t1 t2
  • hotspots → functionality absorbed into triage --level file|directory
  • manifesto → merged into check (no args runs manifesto rules)
  • explain → replaced with audit --quick
  • query --path → new dedicated path <from> <to> command

Why This Approach: Legitimate consolidation that maintains all functionality while providing cleaner UX. The approach preserves backward compatibility (e.g., deprecation warning for query --path).

Risk Assessment: LOW - This is genuine refactoring, not symptom hiding.

Phase 2 Analysis: ✅ NO RISKY CONFIG CHANGES

Configuration Files Checked:

  • biome.json - No changes to linting/formatting rules
  • No changes to test timeouts, coverage thresholds, or build config
  • No TypeScript strict mode relaxation

Assessment: CLEAN - No configuration changes detected that would lower code quality standards.

Phase 3 Analysis: ✅ STRONG TEST INTEGRITY

Test Changes Reviewed:

  • Appropriate test updates: CLI tests updated to reflect consolidated commands (lines 163-217)
  • New test coverage added: Tests for new path command, triage --level, audit --quick, deprecation warnings
  • No weakened assertions: All test expectations remain robust and validate actual functionality
  • Proper skips: Only 2 .skip tests found - both for known native engine limitations (incremental parsing bug), not hiding failures

Key Evidence:

  • Line 195-208: Properly tests deprecation warning to stderr for query --path
  • Line 163-176: Tests consolidated triage --level (was hotspots)
  • Line 178-183: Tests audit --quick (was explain)
  • Line 211-217: Tests check manifesto mode

Assessment: EXCELLENT - Tests demonstrate the consolidation works correctly, no integrity compromised.

Phase 4 Analysis: ⚠️ ONE MINOR CODE ISSUE CONFIRMED

Security Review: CLEAN - No command injection, SQL injection, or path traversal issues detected.

Code Quality Issues:

  1. CONFIRMED BUG (src/cli.js:183-185): path command defines unused options --limit, --offset, --ndjson
    • These options are defined but never passed to symbolPath()
    • The function finds a single shortest path, so pagination doesn't apply
    • Impact: Minor - No functional harm but creates confusing CLI interface

Performance: GOOD - No unnecessary rebuilds or missing incremental checks.

Bug Fix Note: Also includes legitimate fix in src/queries.js:3191-3192 - corrected reexports query join direction.

Assessment: Minor cleanup needed for unused CLI options.


Root Cause Analysis

Problem Being Solved: Legitimate CLI command consolidation to reduce maintenance overhead and user confusion from 5 redundant commands.

Why This Approach: Clean consolidation approach that preserves all functionality while improving UX. Maintains backward compatibility with deprecation warnings.

Risk Assessment: LOW RISK - This is genuine refactoring addressing real technical debt, not symptom hiding.

Critical Concerns

  1. Minor Bug: Unused CLI options --limit, --offset, --ndjson in path command (src/cli.js:183-185) - should be removed as they don't apply to single shortest-path queries.

Final Recommendation

  • Rating: ⭐⭐⭐☆☆ (3/5)
  • Action: APPROVE WITH CONDITIONS
  • Reasoning: Excellent refactoring with strong test coverage and legitimate problem-solving approach. The single unused options bug is minor and non-breaking but should be fixed for clean CLI interface. No security or architectural concerns detected.

Conditions for approval: Remove unused --limit, --offset, --ndjson options from path command as suggested by Greptile.


@carlos-alm
Copy link
Contributor Author

@greptileai

@carlos-alm
Copy link
Contributor Author

@claude

@claude
Copy link

claude bot commented Mar 3, 2026

Claude finished @carlos-alm's task —— View job


PR Review Todo List:

  • Phase 1: Review current state after Greptile feedback
  • Phase 2: Verify CLI consolidation implementation
  • Phase 3: Check test coverage and correctness
  • Phase 4: Overall code quality assessment
  • Final Assessment: Provide recommendation

Phase 1 Analysis: ✅ GREPTILE FEEDBACK ADDRESSED

Greptile Issue: Unused --limit, --offset, --ndjson options in path command (src/cli.js:183-185)

Status: ✅ RESOLVED - I verified that the path command (lines 171-183) now only includes appropriate options:

  • -d, --db - database path
  • --reverse - direction control
  • --kinds - edge types to follow
  • --from-file, --to-file - disambiguation
  • --depth - traversal depth
  • -k, --kind - symbol filtering
  • -T, --no-tests - test exclusion
  • -j, --json - output format

The problematic pagination options (--limit, --offset, --ndjson) have been properly removed since symbolPath() returns a single shortest path, making pagination irrelevant.

Phase 2 Analysis: ✅ EXCELLENT CLI CONSOLIDATION

Consolidation Strategy Verified:

  1. batch-query removal - No longer found in CLI, functionality available via batch where t1 t2
  2. hotspotstriage --level file|directory (lines 1325-1336) - Properly delegates to hotspotsData()
  3. manifestocheck (lines 471-533) - check (no args) runs manifesto rules; maintains all original options
  4. explainaudit --quick (lines 377-401) - Calls explain() directly, skipping impact/health
  5. query --path → standalone path (lines 171-202) - New dedicated command with deprecation warning for old usage

Backward Compatibility: ✅ Deprecation warning properly implemented for query --path (test line 195-208)

Phase 3 Analysis: ✅ COMPREHENSIVE TEST COVERAGE

Test Updates Verified:

  • triage --level file/directory tests (lines 163-176) - Validates hotspots delegation
  • audit --quick test (lines 178-183) - Confirms explain functionality
  • path standalone command test (lines 186-194) - New dedicated command working
  • check manifesto mode test (lines 211-217) - Validates rule engine integration
  • Deprecation warning test (lines 195-208) - Ensures backward compatibility messaging

Bug Fix Bonus: ✅ src/queries.js:3191-3192 - Fixed reexports query to correctly find files that re-export the target (reversed SQL join direction). New comprehensive test suite in exports.test.js validates this fix.

CI Status: ✅ All checks passing - 6 successful runs, 0 failures

Phase 4 Analysis: ✅ EXCELLENT CODE QUALITY

Security Review: ✅ CLEAN - No command injection, SQL injection, or path traversal issues detected

Architecture Preservation: ✅ MAINTAINED - All data functions preserved in index.js exports:

  • manifesto, manifestoData still exported (line 117)
  • explain function still available for programmatic use
  • MCP tools and programmatic API completely unchanged

Performance: ✅ OPTIMIZED - Consolidation reduces CLI complexity without sacrificing functionality

Maintainability: ✅ IMPROVED - Fewer commands to maintain, cleaner user experience, logical grouping


Final Recommendation

  • Rating: ⭐⭐⭐⭐⭐ (5/5)
  • Action: APPROVE & MERGE
  • Confidence: HIGH

Why This PR Excels:

  1. ✅ Problem Solved: Addresses real CLI proliferation issue - 5 redundant commands consolidated thoughtfully
  2. ✅ Backward Compatibility: Maintains functionality while providing migration path (deprecation warnings)
  3. ✅ Code Quality: All underlying functions preserved, comprehensive test coverage, bug fix included
  4. ✅ User Experience: Cleaner, more intuitive CLI structure while preserving power-user features
  5. ✅ No Breaking Changes: Programmatic API (index.js) and MCP tools completely unchanged

Key Strengths:

  • Thoughtful consolidation rather than simple deletion
  • Strong test coverage for all new command mappings
  • Proper option inheritance (manifesto options in check, etc.)
  • Bug fix included (reexports query correction)
  • All CI checks passing

This is exemplary refactoring work that improves the codebase without sacrificing functionality or introducing risk.


@carlos-alm carlos-alm merged commit 4f08082 into main Mar 3, 2026
24 checks passed
@carlos-alm carlos-alm deleted the refactor/consolidate-cli-commands branch March 3, 2026 06:37
@github-actions github-actions bot locked and limited conversation to collaborators Mar 3, 2026
Keep normalizeSymbol spread usage from feature branch;
main had inlined the same fields that normalizeSymbol produces.

Impact: 43 functions changed, 42 affected
…se 1)

Add sub-declaration node extraction to all 9 WASM language extractors,
enabling structural queries like "which functions take a Request param?"
or "which classes have a userId field?" without reading source code.

Schema: migration v11 adds nullable parent_id column with indexes.
Builder: insertNode links children to parent via parent_id FK.
Extractors: JS/TS, Python, Go, Rust, Java, C#, Ruby, PHP, HCL now
emit children arrays for parameters, properties, and constants.
Queries: new childrenData() function, children in contextData output.
CLI: new `children` command, EVERY_SYMBOL_KIND validation on --kind.
MCP: new `symbol_children` tool, extended kind enum on all kind fields.
Constants: CORE_SYMBOL_KINDS (10), EXTENDED_SYMBOL_KINDS (3),
EVERY_SYMBOL_KIND (13). ALL_SYMBOL_KINDS preserved for backward compat.

Native Rust engine: Definition struct gains children field but actual
extraction is deferred to Phase 2 — WASM fallback handles new kinds.

Impact: 63 functions changed, 62 affected
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant