Skip to content

fix(cli): correct ast command import path after src/ reorganization#532

Merged
carlos-alm merged 5 commits intomainfrom
fix/dogfood-ast-import
Mar 19, 2026
Merged

fix(cli): correct ast command import path after src/ reorganization#532
carlos-alm merged 5 commits intomainfrom
fix/dogfood-ast-import

Conversation

@carlos-alm
Copy link
Copy Markdown
Contributor

Summary

The codegraph ast command crashed with a missing module error because it imported from ../../ast.js (resolving to dist/ast.js), but the module lives at dist/features/ast.js after the src/ reorganization in v3.1.5.

Fixed by updating the dynamic import path to ../../features/ast.js.

Found during

Dogfooding v3.3.0 — see #529

Test plan

  • npx codegraph ast call returns results instead of crashing
  • npx vitest run tests/integration/ast.test.js — 19 tests pass
  • npm run lint — clean (1 unrelated warning)

The ast command crashed because it imported from '../../ast.js' which
resolved to dist/ast.js (non-existent). The actual module is at
dist/features/ast.js after the src/ reorganization in v3.1.5.

Closes #529

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

claude bot commented Mar 19, 2026

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


Reviewing PR: fix(cli): correct ast command import path

Todo List:

  • Read and analyze the changed file
  • Verify the codebase structure and import path
  • Check for similar issues in other files
  • Perform root cause analysis
  • Assess backlog compliance
  • Provide final review rating and recommendation

Root Cause Analysis

Problem Being Solved: The codegraph ast command was importing from ../../ast.js which resolved to the non-existent dist/ast.js after the src/ reorganization in v3.1.5. The AST module was moved to src/features/ast.js during the restructure.

Why This Approach: Direct import path fix is the correct approach. This follows the same pattern as PR #521 which fixed similar import path issues in benchmark scripts after the same reorganization.

Risk Assessment: Minimal risk. Single line change to correct a broken import path. The target file exists and exports the required symbols (AST_NODE_KINDS, astQuery).

Backlog Compliance

  • Zero-dep: ✓ — No new dependencies added
  • Foundation-aligned: ✓ — Simple bug fix maintaining existing functionality
  • Problem-fit: 2 — Bug fix that prevents crashes but doesn't directly address core AI navigation problems
  • Breaking: No — Pure bug fix, no API changes
  • Tier: 1

Critical Concerns

None. This is a straightforward import path correction.

Code Quality Assessment

  • Correctness: ✅ Import path now correctly resolves to existing src/features/ast.js
  • Completeness: ✅ No other files have the same broken import pattern
  • Consistency: ✅ Follows the pattern established in PR fix: update benchmark script import paths after src/ restructure #521 for similar fixes
  • Testing: ✅ Author claims integration tests pass (tests/integration/ast.test.js - 19 tests)

Final Recommendation

  • Rating: ⭐⭐⭐⭐☆ (4/5)
  • Action: APPROVE
  • Reasoning: Clean fix for a real crash bug with proper testing verification. Well-contained change with zero risk of introducing new issues.

Minor deduction: While technically correct, this suggests incomplete validation during the original reorganization that allowed this import to break. Future reorganizations should include automated import validation.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 19, 2026

Greptile Summary

This PR fixes a runtime crash in the codegraph ast command caused by a stale import path left behind after the src/ reorganization in v3.1.5. The single-line change updates the dynamic import from ../../ast.js to ../../features/ast.js, which correctly points to src/features/ast.js where the AST_NODE_KINDS constant and astQuery function now live.

The fix is minimal, targeted, and correct — the resolved file exists and exports exactly the named bindings the command requires.

Potential related issue worth investigating (out of scope for this PR):

  • src/cli/commands/info.js line 39 has a similar stale import: import('../../db.js'), but src/db.js does not exist after the reorganization (the correct path is ../../db/index.js or ../../db). Because this import is inside a try/catch block that silently swallows errors (/* diagnostics must never crash */), this bug does not crash the info command — instead it silently suppresses all build metadata output. This is worth fixing in a follow-up PR.

Confidence Score: 5/5

  • Safe to merge — single-line import path fix that has been verified against the actual file structure.
  • The change is a one-line targeted bugfix. The destination module src/features/ast.js exists and exports the exact bindings (AST_NODE_KINDS, astQuery) consumed on the very next lines. The relative path ../../features/ast.js from src/cli/commands/ast.js resolves correctly to src/features/ast.js. No logic changes, no new dependencies, and the PR description confirms integration tests pass.
  • No files in this PR require special attention, though src/cli/commands/info.js (outside the diff) may have a similar stale ../../db.js import worth investigating.

Important Files Changed

Filename Overview
src/cli/commands/ast.js One-line fix updating the dynamic import path from ../../ast.js to ../../features/ast.js. The target file exists at src/features/ast.js and exports both AST_NODE_KINDS and astQuery as expected.

Sequence Diagram

sequenceDiagram
    participant CLI as cli.js
    participant AstCmd as cli/commands/ast.js
    participant FeatAst as features/ast.js (✓ fixed)

    CLI->>AstCmd: execute([pattern], opts, ctx)
    AstCmd->>FeatAst: dynamic import('../../features/ast.js')
    FeatAst-->>AstCmd: { AST_NODE_KINDS, astQuery }
    AstCmd->>AstCmd: validate opts.kind against AST_NODE_KINDS
    AstCmd->>FeatAst: astQuery(pattern, opts.db, queryOpts)
    FeatAst-->>CLI: output results
Loading

Last reviewed commit: "fix(cli): correct as..."

…kill

The pre-flight native binary check was only in Phase 4b (benchmarks),
but a stale binary affects all phases. Move the check to Phase 0 step 6
so it happens before any testing begins. Simplify the Phase 4b section
to reference the earlier check.

Learned from v3.3.0 dogfood where the source repo had v3.1.2 binary
but all testing had already completed before catching the mismatch.
@carlos-alm carlos-alm merged commit 54e6115 into main Mar 19, 2026
@carlos-alm carlos-alm deleted the fix/dogfood-ast-import branch March 19, 2026 09:30
@github-actions github-actions bot locked and limited conversation to collaborators Mar 19, 2026
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