Skip to content

feat: CLI-first expansion — 10 subcommands + CommonJS require() parsing#95

Open
murilloimparavel wants to merge 2 commits intotirth8205:mainfrom
murilloimparavel:feat/cli-first-and-commonjs-require
Open

feat: CLI-first expansion — 10 subcommands + CommonJS require() parsing#95
murilloimparavel wants to merge 2 commits intotirth8205:mainfrom
murilloimparavel:feat/cli-first-and-commonjs-require

Conversation

@murilloimparavel
Copy link
Copy Markdown

Summary

Exposes all 22 MCP tool functions as direct CLI subcommands, enabling CLI-first workflows without requiring the MCP server. Also adds CommonJS require() parsing for JavaScript codebases that use dynamic imports.

New CLI subcommands

Command Description
query <pattern> <target> Query relationships (callers_of, callees_of, imports_of, etc.)
impact [--files] [--depth] Analyze blast radius of changed files
search <query> [--kind] Search code entities by name/keyword
flows [--sort] [--limit] List execution flows by criticality
flow [--id | --name] Get details of a single execution flow
communities [--sort] List detected code communities
community [--id | --name] Get details of a single community
architecture Architecture overview from community structure
large-functions [--min-lines] Find functions/classes exceeding line threshold
refactor <mode> Rename preview, dead code detection, suggestions

CLI post-processing (fixes #93)

build and update commands now run the same post-processing as the MCP tool:

  • Signature computation
  • FTS5 index rebuild
  • Execution flow detection
  • Community detection

CommonJS require() parsing

Three new methods in parser.py:

  • _js_get_require_target(): extracts module paths from require(), path.join(), path.resolve(), template literals, dynamic import()
  • _extract_js_require_constructs(): creates IMPORTS_FROM edges for CommonJS patterns
  • _collect_js_require_names(): populates import map for call resolution

Results on test monorepo (14.5K files):

  • IMPORTS_FROM edges increased 4x (525 → 2,075)
  • Flows detected: 1,185
  • Communities: 605

CLI UX improvements

  • Updated banner and docstring with all new commands
  • "Graph not built" warning when DB is missing
  • Argparse validation: flow/community require --id or --name
  • Argparse validation: refactor rename requires --old-name/--new-name
  • DELEGATED_COMMANDS set skips redundant GraphStore creation

Test plan

  • 29/29 CLI functional tests passing (all patterns, edge cases, empty args)
  • Full rebuild: 8,352 nodes, 50,259 edges, 1,185 flows, 605 communities
  • CommonJS require() — no regression on ESM import handling
  • Empty require('') and require() handled gracefully (no crash, no empty edges)
  • Depth-limited recursion (max_depth=50) prevents stack overflow
  • bin/crg repos, bin/crg --version, bin/crg --help work without --repo

🤖 Generated with Claude Code

@Jay-Gajera
Copy link
Copy Markdown

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

🤖 Generated with Claude Code

@murilloimparavel
Copy link
Copy Markdown
Author

Update: Shared post-processing pipeline (addresses #93)

After reviewing PR #98's approach to fixing #93, I've adopted the same architectural pattern in this PR — but went further:

What changed (commit dd6b639)

  • Created postprocessing.py — shared module with run_post_processing() that runs the 4-step pipeline (signatures, FTS, flows, communities). Single source of truth for CLI, MCP tools, and watch mode.
  • Refactored tools/build.py — removed 62 lines of inline pipeline, now delegates to run_post_processing().
  • Refactored cli.py — replaced the inline _run_post_processing() (which was a copy of build.py's logic) with a thin _cli_post_process() wrapper that calls the shared module.
  • Added watch callbackwatch() in incremental.py now accepts on_files_updated: Callable parameter, wired to run_post_processing from CLI. This means watch mode also runs post-processing after file updates.
  • Hard error on missing graph — delegated CLI commands (query, impact, search, etc.) now sys.exit(1) instead of printing a warning and proceeding to crash.
  • Bug fixes from QA review:
    • IndexError instead of KeyError in signature computation except clause (Row objects are accessed by index, not key)
    • Callable[..., Any] type annotation instead of Any for the watch callback
  • 9 new tests in test_postprocessing.py covering: happy path, idempotency, empty store, step isolation (each step failing independently while others succeed)

How this relates to PR #98

PR #98 and this PR independently arrived at the same solution (extract to postprocessing.py). This PR now includes that refactoring plus the original CLI-first features (10 subcommands + CommonJS require() parsing). Key differences:

Aspect This PR (#95) PR #98
Shared postprocessing.py
tools/build.py refactored ✅ (-62 lines) ✅ (-68 lines)
watch callback
10 CLI subcommands
CommonJS require() parsing
sys.exit(1) on missing graph
IndexError fix
Callable type annotation
Tests for post-processing ✅ (9 tests) ✅ (18 tests)

If #98 merges first, rebasing this PR is straightforward — just drop the postprocessing.py creation and keep everything else.

Test results

574 passed, 5 skipped, 2 xpassed (full suite)
9 passed (new postprocessing tests)
ruff: 0 errors

@igagankalra
Copy link
Copy Markdown

@murilloimparavel add some more tests, and would suggest to keep a bug fix separate from the feature you are adding.
I would still suggest to go forward with #98 and then merge this one. Following SRP always help code clean.

Copy link
Copy Markdown
Owner

@tirth8205 tirth8205 left a comment

Choose a reason for hiding this comment

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

Large feature PR — 10 CLI subcommands + CommonJS require() parsing. The CommonJS piece is unique value.

A few notes:

  1. The post-processing fix overlaps with PR #98 — we'll merge #98 first since it's more focused. Please rebase after that.
  2. Consider splitting the CommonJS require() parsing into its own PR — it stands on its own and is independently valuable.
  3. Please rebase on latest main (has conflicts).

@tirth8205
Copy link
Copy Markdown
Owner

Review: PR #95 — feat: CLI-first expansion and CommonJS require() parsing

Good feature work. The 10 new CLI subcommands directly expose the 22 MCP tool functions and CommonJS require() parsing is independently valuable.

Merge order dependency: The owner confirmed this should merge AFTER #98 (which handles the post-processing extraction more cleanly). Once #98 merges, this PR needs a rebase to avoid duplicating the postprocessing.py creation — as the author noted, 'rebasing this PR is straightforward — just drop the postprocessing.py creation and keep everything else.'

Code review findings:

  1. _cli_post_process duplication: Before the author updated this PR with postprocessing.py, there was an inline copy in cli.py. After the update, both PRs (fix: resolve SQLite transaction bugs, FTS5 sync, and atomic operations #94, feat: CLI-first expansion — 10 subcommands + CommonJS require() parsing #95, fix: CLI build/update/watch now run post-processing (signatures, FTS, flows, communities) #98) create postprocessing.py. After fix: CLI build/update/watch now run post-processing (signatures, FTS, flows, communities) #98 merges, this PR's postprocessing.py creation should be dropped to avoid conflicts.

  2. DELEGATED_COMMANDS set is correct: Skipping GraphStore creation for commands that don't need the DB (query, impact, etc. all need it — but serve/eval/enrich do not) is the right optimization.

  3. Argparse validation: The flow/community requiring --id or --name validation is correct. However, the validation appears to be done at runtime (inside the command handler) rather than at argparse parse time — consider using a mutually exclusive group for cleaner UX.

  4. CommonJS parsing: The three new methods (_js_get_require_target, _extract_js_require_constructs, _collect_js_require_names) handle dynamic import() and template literals. The 4x IMPORTS_FROM edge increase is significant and plausible.

Verdict: Rebase after #98 merges, resolve post-processing duplication, then this is ready.

@tirth8205
Copy link
Copy Markdown
Owner

Merge conflict detected. This PR has merge conflicts with main (mergeStateStatus: DIRTY). This is expected given the large cli.py changes and the post-processing overlap. Please rebase on main after PR #94 and #98 merge (as the owner instructed). The rebase strategy: keep the 10 new CLI subcommands and CommonJS parsing, drop the postprocessing.py creation (it will already exist from #98).

…rsing

Adds 10 new CLI subcommands that expose MCP tool functions directly,
enabling CLI-first workflows without requiring the MCP server:

  query, impact, search, flows, flow, communities, community,
  architecture, large-functions, refactor

Also adds CLI post-processing after build/update (fixes tirth8205#93):
  - Signature computation
  - FTS5 index rebuild
  - Execution flow detection
  - Community detection

CommonJS require() parsing (parser.py):
  - `_js_get_require_target()`: extracts module paths from require(),
    path.join(), path.resolve(), template literals, dynamic import()
  - `_extract_js_require_constructs()`: creates IMPORTS_FROM edges
  - `_collect_js_require_names()`: populates import map for call resolution
  - Empty-string guards at all call sites
  - Depth-limited recursion (max_depth=50) for nested require walks

Results on test monorepo (14.5K files):
  - IMPORTS_FROM edges increased 4x (525 → 2,075)
  - Flows detected: 1,185
  - Communities: 605
  - FTS indexed: 7,766 nodes

CLI UX improvements:
  - Updated banner and docstring with all new commands
  - "Graph not built" warning when DB is missing
  - Argparse validation for flow/community (require --id or --name)
  - Argparse validation for refactor rename (require --old-name/--new-name)
  - DELEGATED_COMMANDS skip redundant GraphStore creation

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@murilloimparavel
Copy link
Copy Markdown
Author

Rebased on latest main and resolved merge conflicts:

@murilloimparavel murilloimparavel force-pushed the feat/cli-first-and-commonjs-require branch from dd6b639 to b437ba9 Compare April 11, 2026 15:46
@tirth8205
Copy link
Copy Markdown
Owner

Skipping auto-merge: after updating against current main, CI shows lint + type-check failures in this PR's own code (tests all pass on 3.10–3.13).

ruff (4 errors):

  • cli.pyN806 DELEGATED_COMMANDS local variable should be lowercase
  • cli.pyF401 unused imports .incremental.full_build, .incremental.incremental_update
  • cli.pyF811 watch redefined from line 647

mypy (9 errors, all in cli.py:217–251): the new subcommand dispatch uses object where GraphStore is expected ("object" has no attribute "rollback", Argument 1 to "rebuild_fts_index"/"trace_flows"/"store_flows"/"detect_communities"/"store_communities" has incompatible type "object"). Looks like a missing type annotation on whatever holds the store.

Please run ruff check --fix code_review_graph/ and mypy code_review_graph/ --ignore-missing-imports --no-strict-optional locally, push the fixes, and I'll re-review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CLI build missing post-processing steps that build_or_update_graph_tool MCP tool includes

4 participants