Skip to content

feat(scanner): Add Go parser fallback - #123

Draft
reneleonhardt wants to merge 2 commits into
JordanCoin:mainfrom
reneleonhardt:feat/scanner-go-fallback
Draft

feat(scanner): Add Go parser fallback#123
reneleonhardt wants to merge 2 commits into
JordanCoin:mainfrom
reneleonhardt:feat/scanner-go-fallback

Conversation

@reneleonhardt

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds a Go parser fallback so dependency analysis stays available when ast-grep is unavailable or times out. The public dependency scan routes through the fallback while preserving mixed Go and Cargo topology, avoiding duplicate Cargo loads, and keeping Cargo-only recovery graph-scoped.

Type of change

  • New feature
  • Bug fix
  • New language support
  • Documentation
  • Other (describe below)

Checklist

  • I've tested this locally with go build && ./codemap .
  • I've read CONTRIBUTING.md (for new language support)
  • I've updated documentation if needed

Additional notes

  • Parses Go source directly when the primary ast-grep scan fails.
  • Fallback outcomes carry explicit provenance so coverage and fail-closed behavior stay consistent with the scan contract.

Developed with carefully directed, manually reviewed AI assistance.

reneleonhardt and others added 2 commits August 12, 2026 14:17
Fall back to the Go parser when ast-grep is unavailable, so Go dependency
analysis stays available without external PATH tools.

Co-Authored-By: GPT-5.6 Sol <codex@openai.com>
Route public dependency scans through the Go parser fallback while preserving mixed Go and Cargo topology, avoiding duplicate Cargo loads, and keeping Cargo-only recovery graph-scoped.

Co-Authored-By: GPT-5.6 Sol <codex@openai.com>

test(scanner): port scanner tests to the consolidated post-JordanCoin#105 analysis API

The branch's scanner tests still used the pre-JordanCoin#105 surface (ScanForDepsOutcome,
Sources[].Source); the prod code is post-JordanCoin#105. Port the integration-verified
migration (ScanForDeps(ctx, root, Filters{}), Sources[].Name) so the branch's
test package compiles against its own prod.
@reneleonhardt
reneleonhardt force-pushed the feat/scanner-go-fallback branch from 35fd6b2 to ff6e0f1 Compare August 12, 2026 12:43
@JordanCoin

Copy link
Copy Markdown
Owner

Reviewed in depth. The core mechanism is sound and genuinely well-tested — with ast-grep missing, a Go repo now returns correct edges with honest provenance (ast-grep: unavailable + go-parser: fallback), byte-identical file list to the ast-grep run. That part is good work.

Two things I'd want fixed before it lands.

1. Non-Go languages silently answer "none" instead of failing honestly

In a repo containing both Go and TypeScript, with ast-grep unavailable, the Go fallback succeeds — which means the graph builds — but it contains no TS edges. --importers then reports a confident negative:

# ground truth (ast-grep available)
$ codemap --importers web/util.ts
📍 File: web/util.ts
   Imported by 1 file(s)
   • web/app.ts

# origin/main, ast-grep unavailable
$ codemap --importers web/util.ts
Error building file graph: ast-grep not found (checked bundled tools and PATH)

# this PR, ast-grep unavailable
$ codemap --importers web/util.ts
No files import web/util.ts.
   Note: files in the same package never import each other (Go resolves
   imports at package level), so only cross-package importers appear here.

web/app.ts does import it. An agent reading this concludes the file is unused and safe to delete. The Go-specific note printed about a TypeScript file makes it worse.

The JSON surface and MCP both stay honest (partial + coverage text) — only the CLI drops it, because renderImportersReportCLI early-returns before renderCoverage. Fix looks like ~3 lines: emit the coverage line in the empty branch too. Trading a hard error for a wrong answer is the one trade this project shouldn't make.

2. The fallback doesn't fire on timeout or failure, only on "not installed"

The PR body says the fallback covers when ast-grep "fails or times out". It doesn't. ScanDirectory converts every IncompleteScanError except ScanSourceUnavailable into a degraded-but-nil-error outcome, so the gate in cargofallback.go (which requires a non-nil *IncompleteScanError named ast-grep) is only reachable when the binary is absent. With a stub ast-grep emitting invalid JSON:

"coverage":{"status":"unavailable","sources":[{"name":"ast-grep","status":"failed",
  "detail":"ast-grep produced invalid JSON results"}]}, "files":[]

Zero fallback, with parseable Go files sitting right there. Timeout and non-zero exit are the realistic large-repo failures — those are exactly the cases the fallback would earn its keep, and they're the ones it misses. Either widen the gate or narrow the claim in the body.

Smaller notes

  • --deps emits two cargo-metadata sources with contradictory statuses (authoritative and fallback) and runs cargo metadata twice. The dedup guard sits in buildFileGraphFromOutcomeWithCargoMetadataAndFilters, but --deps reaches the graph through the exported BuildFileGraphFromOutcome, which hardcodes the loader.
  • Coverage notes claim recovered Cargo edges that never reach the --deps payload — they live in the unexported precomputedEdges and aren't serialized.
  • "recovered 4 of 5 Go files" reads as data loss; the 5th is a type/const-only file that ast-grep also omits. Nothing is lost, the denominator is just wrong.
  • The fallback emits methods as functions where ast-grep doesn't, so the files/functions set churns across authoritative→fallback→authoritative transitions — visible to anything caching analyses (watch state, handoff).

Merge-order note

This changes scanForGraphOutcomeWithFilters from 5 params to 6. I checked #124#127 and none of them call it, so there's no repeat of the #117/#118 break — but it's the same shape, so worth knowing.

More importantly: with ast-grep unavailable the Go fallback emits no Rust analyses, so all of the Rust resolution work in #124#127 is inert on that path, and the loader = nil guard here degrades rustWorkspaceIndex to path-globbing whenever the Cargo fallback fires. Suggest landing this after the Rust batch, and re-checking #127's build-script fixtures under the ast-grep-unavailable path.

@reneleonhardt
reneleonhardt marked this pull request as draft August 13, 2026 08:03
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.

2 participants