refactor!: merge fn into query, fold models into embed --models

[carlos-alm]

Summary

• query absorbs fn: now calls fnDeps() with --depth, --file, --kind options — full function-level call chain instead of basic name lookup
• embed --models replaces models: model table folded into the embed command as a --models flag
• fn and models commands removed from CLI (2 fewer top-level commands)

No breaking changes to MCP tools (query_function, fn_deps stay), programmatic API (queryNameData, fnDepsData exports stay), or fn-impact command.

Test plan

• codegraph query buildGraph -T — works with default depth
• codegraph query buildGraph --depth 5 -T — depth option works
• codegraph query buildGraph -f builder.js -T — file scoping works
• codegraph query buildGraph -k function -T — kind filter works
• codegraph fn buildGraph — errors with "unknown command"
• codegraph embed --models — prints model table
• codegraph models — errors with "unknown command"
• All 832 tests pass, lint clean

[claude]

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

---

PR Review: CLI Command Refactoring

Todo List:

• Phase 1: Root cause analysis - understand the problem being solved
• Phase 2: Configuration change detection - check for high-risk changes
• Phase 3: Test integrity - verify no weakened assertions
• Phase 4: Code quality review - analyze the refactoring implementation
• Final assessment and rating

Phase 1: Root Cause Analysis ✅

Problem Being Solved: Command surface area reduction - the CLI had 2 redundant/overlapping commands:

• fn command that provided function-level dependency traversal
• models command that just listed available embedding models

Why This Approach: The refactoring consolidates functionality logically:

• query was already doing name lookup via queryName() but with limited functionality; now it calls fnDeps() with full transitive traversal, gaining --depth, --file, --kind options that fn had
• embed --models makes more sense than a separate models command since model selection is contextual to embedding

Risk Assessment: LOW - This is genuine UI simplification, not masking underlying problems.

Phase 2: Configuration Change Detection ✅

No high-risk configuration changes detected:

• No linting rules disabled or relaxed
• No test timeouts or thresholds modified
• No TypeScript strict mode changes
• Only legitimate code consolidation

Phase 3: Test Integrity ✅

Test changes are appropriate:

• tests/integration/cli.test.js: Only one test changed from fn add to query add - maintains same assertion (toHaveProperty('results'))
• No weakened assertions, removed tests, or eslint-disable workarounds
• Test comment updated from // ─── Fn ────── to // ─── Query (fn-level) ─── - accurate reflection of change

Phase 4: Code Quality Review ✅

Implementation Analysis:

• Correct function mapping: query now calls fnDeps() instead of queryName(), gaining all the advanced functionality that fn had
• Option preservation: All fn options (--depth, --file, --kind) properly transferred to query
• Validation preserved: Kind validation logic correctly transferred
• Models consolidation: Clean integration of model listing into embed --models flag with early return
• MCP compatibility maintained: As stated, query_function and fn_deps MCP tools unchanged
• Programmatic API preserved: queryNameData and fnDepsData exports remain for backward compatibility

Code Quality Issues:

• NONE IDENTIFIED - the refactoring is clean and well-executed

Documentation Updates:

• CLAUDE.md: Command reference updated appropriately
• README.md: Command examples updated consistently
• FOUNDATION.md: References updated correctly

Potential Concerns

1. Breaking Change Scope: This is a CLI-breaking change but preserves MCP and programmatic APIs - appropriate for a major version bump (marked with !)

2. Command Discoverability: Users familiar with codegraph fn must learn codegraph query - but the consolidation reduces cognitive load overall

3. Testing Coverage: While the integration test was updated, it would be ideal to have explicit tests proving query now has the depth/file/kind functionality that fn provided

Verification of Claims

✅ "No breaking changes to MCP tools" - Confirmed, MCP tools call functions directly
✅ "No breaking changes to programmatic API" - Confirmed, data functions still exported
✅ "fn-impact command unchanged" - Confirmed, still exists
✅ "All tests pass" - Cannot verify due to tool restrictions, but no evidence of test weakening
✅ Command consolidation works - Code analysis confirms proper option/function mapping

Final Assessment

## Root Cause Analysis
**Problem Being Solved**: Reducing CLI surface area by eliminating 2 redundant commands (`fn`, `models`)
**Why This Approach**: Logical consolidation - `query` gains `fn`'s full functionality, `models` becomes `embed --models`
**Risk Assessment**: LOW - Genuine simplification, preserves all underlying functionality

## Critical Concerns
None identified. This is a well-executed refactoring that reduces command proliferation without losing functionality.

## Final Recommendation
- Rating: ⭐⭐⭐⭐☆ (4/5)
- Action: **APPROVE**
- Reasoning: Clean consolidation with proper option transfer, maintains backward compatibility for MCP/API users, appropriate breaking change management. Only missing more comprehensive testing of transferred functionality.

Minor Suggestions (Optional)

1. Consider adding integration tests explicitly verifying query --depth and query --file work
2. Add a changelog entry documenting the migration path for fn users

Bottom Line: This refactoring improves CLI ergonomics without sacrificing functionality. Well-executed consolidation.

---

[greptile-apps]

Greptile Summary

Successfully consolidates two CLI commands (fn and models) into existing commands (query and embed --models), reducing CLI surface area while preserving programmatic API and MCP tool compatibility. The query command now provides enhanced functionality with transitive call chains.

• Merged fn command into query — now supports --depth, --file, and --kind options for function-level dependency analysis
• Folded models command into embed --models flag
• Removed unused queryName import and command definitions
• Updated all documentation (README, CLAUDE.md, FOUNDATION.md) to reflect new command structure
• Security improvement: removed shell: true from execFileSync in bench-config.js
• Test updated to use new query command
• Potential issue: --limit, --offset, --ndjson options are defined but not functional in new query implementation

Confidence Score: 4/5

• Safe to merge with one minor issue to address
• Well-executed refactoring with comprehensive documentation updates and passing tests. One functional issue with unused CLI options should be cleaned up, but doesn't affect core functionality.
• src/cli.js - verify whether --limit, --offset, --ndjson options should be removed or implemented

Important Files Changed

Filename	Overview
src/cli.js	Merges fn command into query, moves models to embed --models; --limit, --offset, --ndjson options may not work
tests/integration/cli.test.js	Updated test to use query instead of fn command
scripts/lib/bench-config.js	Removed shell: true from execFileSync (security improvement)

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    Start([User runs codegraph command])
    Start --> Query[query <name>]
    Start --> Embed[embed]
    Start --> Legacy1[~~fn <name>~~]
    Start --> Legacy2[~~models~~]
    
    Query --> QueryOpts["Options: --depth, --file, --kind"]
    QueryOpts --> FnDeps[fnDeps function]
    
    Embed --> EmbedCheck{--models flag?}
    EmbedCheck -->|Yes| ShowModels[Display model table]
    EmbedCheck -->|No| BuildEmbed[buildEmbeddings function]
    
    Legacy1 -.->|Removed| Error1[Error: unknown command]
    Legacy2 -.->|Removed| Error2[Error: unknown command]
    
    style Legacy1 fill:#ffcccc,stroke:#cc0000,stroke-dasharray: 5 5
    style Legacy2 fill:#ffcccc,stroke:#cc0000,stroke-dasharray: 5 5
    style Error1 fill:#ffcccc
    style Error2 fill:#ffcccc
    style Query fill:#ccffcc
    style Embed fill:#ccffcc

Loading

_{Last reviewed commit: e474497}

[greptile-apps]

_{6 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Additional Comments (1)

src/cli.js
fnDeps() doesn't support --limit, --offset, or --ndjson options (unlike queryName() which did). These are being passed but silently ignored.

    fnDeps(name, opts.db, {
      depth: parseInt(opts.depth, 10),
      file: opts.file,
      kind: opts.kind,
      noTests: resolveNoTests(opts),
      json: opts.json,
    });

[claude]

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

---

PR Review: CLI Command Refactoring

Todo List:

• Phase 1: Root cause analysis - understand the problem being solved
• Phase 2: Configuration change detection - check for high-risk changes
• Phase 3: Test integrity - verify no weakened assertions
• Phase 4: Code quality review - analyze the refactoring implementation
• Address Greptile's finding about unused CLI options
• Final assessment and rating

Phase 1: Root Cause Analysis ✅

Problem Being Solved: Command surface area reduction - the CLI had 2 redundant/overlapping commands:

• fn command that provided function-level dependency traversal
• models command that just listed available embedding models

Why This Approach: The refactoring consolidates functionality logically:

• query was already doing name lookup via queryName() but with limited functionality; now it calls fnDeps() with full transitive traversal, gaining --depth, --file, --kind options that fn had
• embed --models makes more sense than a separate models command since model selection is contextual to embedding

Risk Assessment: LOW - This is genuine UI simplification, not masking underlying problems.

Phase 2: Configuration Change Detection ✅

No high-risk configuration changes detected:

• No linting rules disabled or relaxed
• No test timeouts or thresholds modified
• No TypeScript strict mode changes
• Only legitimate code consolidation

Phase 3: Test Integrity ✅

Test changes are appropriate:

• tests/integration/cli.test.js: Only one test changed from fn add to query add - maintains same assertion (toHaveProperty('results'))
• No weakened assertions, removed tests, or eslint-disable workarounds
• Test comment updated from // ─── Fn ────── to // ─── Query (fn-level) ─── - accurate reflection of change

Phase 4: Code Quality Review ✅

Implementation Analysis:

• Correct function mapping: query now calls fnDeps() instead of queryName(), gaining all the advanced functionality that fn had
• Option preservation: All fn options (--depth, --file, --kind) properly transferred to query
• Validation preserved: Kind validation logic correctly transferred
• Models consolidation: Clean integration of model listing into embed --models flag with early return
• MCP compatibility maintained: As stated, query_function and fn_deps MCP tools unchanged
• Programmatic API preserved: queryNameData and fnDepsData exports remain for backward compatibility

Code Quality Issues:

• NONE IDENTIFIED - the refactoring is clean and well-executed

Documentation Updates:

• CLAUDE.md: Command reference updated appropriately
• README.md: Command examples updated consistently
• FOUNDATION.md: References updated correctly

Critical Issue: Greptile's Finding ⚠️

The Issue: Lines 119-121 in src/cli.js pass limit, offset, and ndjson options to fnDeps(), but fnDeps() doesn't support these options (unlike queryName() which did). These are being silently ignored.

Evidence:

• queryName() supported: limit, offset, ndjson, noTests, json
• fnDeps() supports: depth, file, kind, noTests, json

Impact: Users expecting --limit 10 or --ndjson output from query command will get no effect, causing confusion.

Recommendation: Either remove the unsupported options from the query command definition or implement them in fnDeps().

Potential Concerns

1. Breaking Change Scope: This is a CLI-breaking change but preserves MCP and programmatic APIs - appropriate for a major version bump (marked with !)

2. Command Discoverability: Users familiar with codegraph fn must learn codegraph query - but the consolidation reduces cognitive load overall

3. Testing Coverage: While the integration test was updated, it would be ideal to have explicit tests proving query now has the depth/file/kind functionality that fn provided

Verification of Claims

✅ "No breaking changes to MCP tools" - Confirmed, MCP tools call functions directly
✅ "No breaking changes to programmatic API" - Confirmed, data functions still exported
✅ "fn-impact command unchanged" - Confirmed, still exists
✅ "All tests pass" - Cannot verify due to tool restrictions, but no evidence of test weakening
✅ Command consolidation works - Code analysis confirms proper option/function mapping

Final Assessment

## Root Cause Analysis
**Problem Being Solved**: Reducing CLI surface area by eliminating 2 redundant commands (`fn`, `models`)
**Why This Approach**: Logical consolidation - `query` gains `fn`'s full functionality, `models` becomes `embed --models`
**Risk Assessment**: LOW - Genuine simplification, preserves all underlying functionality

## Critical Concerns
**Functional Issue**: `query` command defines `--limit`, `--offset`, `--ndjson` options but passes them to `fnDeps()` which doesn't support them, causing silent failures.

## Final Recommendation
- Rating: ⭐⭐⭐☆☆ (3/5)
- Action: **REQUEST CHANGES**
- Reasoning: Clean consolidation with proper option transfer and maintains backward compatibility, BUT has a functional issue where advertised CLI options don't work. Fix the unsupported options issue and this becomes a solid 4/5 approval.

Required Fix

Remove lines 105-107 and 119-121 from src/cli.js:

- .option('--limit <number>', 'Max results to return')
- .option('--offset <number>', 'Skip N results (default: 0)')  
- .option('--ndjson', 'Newline-delimited JSON output')

- limit: opts.limit ? parseInt(opts.limit, 10) : undefined,
- offset: opts.offset ? parseInt(opts.offset, 10) : undefined,
- ndjson: opts.ndjson,

Bottom Line: This refactoring improves CLI ergonomics without sacrificing functionality, but needs the unused options removed to avoid user confusion.

---

View job

[claude]

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

---

Claude Code is reviewing this PR…

PR Review: CLI Command Refactoring

Todo List:

• Phase 1: Root cause analysis - understand the problem being solved
• Phase 2: Configuration change detection - check for high-risk changes
• Phase 3: Test integrity - verify no weakened assertions
• Phase 4: Code quality review - analyze the refactoring implementation
• Address Greptile's finding about unused CLI options
• Final assessment and rating

Phase 1: Root Cause Analysis ✅

Problem Being Solved: Command surface area reduction - the CLI had 2 redundant/overlapping commands:

• fn command that provided function-level dependency traversal
• models command that just listed available embedding models

Why This Approach: The refactoring consolidates functionality logically:

• query was already doing name lookup via queryName() but with limited functionality; now it calls fnDeps() with full transitive traversal, gaining --depth, --file, --kind options that fn had
• embed --models makes more sense than a separate models command since model selection is contextual to embedding

Risk Assessment: LOW - This is genuine UI simplification, not masking underlying problems.

Phase 2: Configuration Change Detection ✅

No high-risk configuration changes detected:

• No linting rules disabled or relaxed
• No test timeouts or thresholds modified
• No TypeScript strict mode changes
• Only legitimate code consolidation

Phase 3: Test Integrity ✅

Test changes are appropriate:

• tests/integration/cli.test.js: Only one test changed from fn add to query add - maintains same assertion (toHaveProperty('results'))
• No weakened assertions, removed tests, or eslint-disable workarounds
• Test comment updated from // ─── Fn ────── to // ─── Query (fn-level) ─── - accurate reflection of change

Phase 4: Code Quality Review ✅

Implementation Analysis:

• Correct function mapping: query now calls fnDeps() instead of queryName(), gaining all the advanced functionality that fn had
• Option preservation: All fn options (--depth, --file, --kind) properly transferred to query
• Validation preserved: Kind validation logic correctly transferred
• Models consolidation: Clean integration of model listing into embed --models flag with early return
• MCP compatibility maintained: As stated, query_function and fn_deps MCP tools unchanged
• Programmatic API preserved: queryNameData and fnDepsData exports remain for backward compatibility

Code Quality Issues:

• NONE IDENTIFIED - the refactoring is clean and well-executed

Documentation Updates:

• CLAUDE.md: Command reference updated appropriately
• README.md: Command examples updated consistently
• FOUNDATION.md: References updated correctly

Critical Issue: Greptile's Finding ⚠️

The Issue: Lines 119-121 in src/cli.js pass limit, offset, and ndjson options to fnDeps(), but fnDeps() doesn't support these options (unlike queryName() which did). These are being silently ignored.

Evidence:

• queryName() supported: limit, offset, ndjson, noTests, json
• fnDeps() supports: depth, file, kind, noTests, json

Impact: Users expecting --limit 10 or --ndjson output from query command will get no effect, causing confusion.

Recommendation: Either remove the unsupported options from the query command definition or implement them in fnDeps().

Potential Concerns

1. Breaking Change Scope: This is a CLI-breaking change but preserves MCP and programmatic APIs - appropriate for a major version bump (marked with !)

2. Command Discoverability: Users familiar with codegraph fn must learn codegraph query - but the consolidation reduces cognitive load overall

3. Testing Coverage: While the integration test was updated, it would be ideal to have explicit tests proving query now has the depth/file/kind functionality that fn provided

Verification of Claims

✅ "No breaking changes to MCP tools" - Confirmed, MCP tools call functions directly
✅ "No breaking changes to programmatic API" - Confirmed, data functions still exported
✅ "fn-impact command unchanged" - Confirmed, still exists
✅ "All tests pass" - Cannot verify due to tool restrictions, but no evidence of test weakening
✅ Command consolidation works - Code analysis confirms proper option/function mapping

Final Assessment

## Root Cause Analysis
**Problem Being Solved**: Reducing CLI surface area by eliminating 2 redundant commands (`fn`, `models`)
**Why This Approach**: Logical consolidation - `query` gains `fn`'s full functionality, `models` becomes `embed --models`
**Risk Assessment**: LOW - Genuine simplification, preserves all underlying functionality

## Critical Concerns
**Functional Issue**: `query` command defines `--limit`, `--offset`, `--ndjson` options but passes them to `fnDeps()` which doesn't support them, causing silent failures.

## Final Recommendation
- Rating: ⭐⭐⭐☆☆ (3/5)
- Action: **REQUEST CHANGES**
- Reasoning: Clean consolidation with proper option transfer and maintains backward compatibility, BUT has a functional issue where advertised CLI options don't work. Fix the unsupported options issue and this becomes a solid 4/5 approval.

Required Fix

Remove lines 105-107 and 119-121 from src/cli.js:

- .option('--limit <number>', 'Max results to return')
- .option('--offset <number>', 'Skip N results (default: 0)')  
- .option('--ndjson', 'Newline-delimited JSON output')

- limit: opts.limit ? parseInt(opts.limit, 10) : undefined,
- offset: opts.offset ? parseInt(opts.offset, 10) : undefined,
- ndjson: opts.ndjson,

Bottom Line: This refactoring improves CLI ergonomics without sacrificing functionality, but needs the unused options removed to avoid user confusion.

---

[carlos-alm]

Closing — content already squash-merged into main via PR #159 and related PRs. Branch is stale.