Skip to content

fix(scanner): Keep ast-grep authoritative, dedupe coverage notes - #108

Open
reneleonhardt wants to merge 1 commit into
JordanCoin:mainfrom
reneleonhardt:fix/scanner-coverage-provenance
Open

fix(scanner): Keep ast-grep authoritative, dedupe coverage notes#108
reneleonhardt wants to merge 1 commit into
JordanCoin:mainfrom
reneleonhardt:fix/scanner-coverage-provenance

Conversation

@reneleonhardt

Copy link
Copy Markdown
Contributor

Follow-up to #105, closing the two open findings from the maintainer re-review on the coverage line.

What this PR does

Both findings were in the coverage line — cosmetic but user-facing:

1. The Rust note printed twice. ast-grep and rust-cargo both carried an identical Detail, and GraphCoverage.AddSource appended each source's detail to Notes with no dedup. A two-crate workspace rendered:

Coverage: partial — Rust macro-generated, string-routed, and #[path] module edges may be
unresolved; Rust macro-generated, string-routed, and #[path] module edges may be unresolved

2. ast-grep was labeled mixed carrying a Rust-specific detail. The DetectLanguage(...) == "rust" loop in scanner/astgrep.go demoted ast-grep's source status even though its extraction is not degraded — the macro/#[path]/string-routed caveat is a Rust resolution gap, which rust-cargo owns. Consequences: on a Go+Rust monorepo the Go analysis was reported degraded too, and the same sentence landed on two sources, producing the duplicate in (1).

Changes

  • scanner/astgrep.go — ast-grep stays authoritative on a successful scan regardless of language; the Rust caveat is no longer attached to its source. The file graph already records rust-cargo (mixed, with the caveat) for Rust repos, so coverage remains partial where it should.
  • scanner/outcome.goGraphCoverage.AddSource records each source detail once (dedup before append), so every Notes consumer (MCP structured coverage_notes, --importers, intent, watch state) inherits the dedup.
  • render/depgraph.gorenderCoverageLine renders each detail once, so a shared caveat prints a single warning in the text output even if two sources carry it.
  • scanner/types.go — the default newDepsProject coverage mirrors the production shape (ast-grep authoritative + rust-cargo mixed carrying the note) instead of attaching the note to ast-grep.

Before / after

JSON sources before:

{"name": "ast-grep",       "status": "mixed",         "detail": "Rust macro-generated, ..."}
{"name": "cargo-metadata", "status": "authoritative"}
{"name": "rust-cargo",     "status": "mixed",         "detail": "Rust macro-generated, ..."}

JSON sources after — the list reads as what each tool actually contributed:

{"name": "ast-grep",       "status": "authoritative"}
{"name": "cargo-metadata", "status": "authoritative"}
{"name": "rust-cargo",     "status": "mixed",         "detail": "Rust macro-generated, ..."}

Regression tests

  • TestAstGrepScanDirectoryRustStaysAuthoritative — Rust scans keep ast-grep authoritative with no Rust detail.
  • TestGraphCoverageAddSourceDedupesSharedDetail — shared caveat recorded once across sources.
  • TestDepgraphRendersSharedCoverageDetailOnce — a shared detail renders once in text output.
  • TestNewDepsProjectRustCaveatOwnedByRustCargoSource — default coverage shape matches production.

Verification

  • go build ./..., go vet ./scanner ./render ./analysis ., gofmt clean.
  • Targeted scanner coverage/contract tests and render TestDepgraph* pass.

Developed with carefully directed, manually reviewed AI assistance.

Co-authored-by: GPT-5.6 Sol codex@openai.com

Maintainer re-review (issue 5209011799) flagged two coverage-line issues:

- ast-grep was demoted to mixed with a Rust-specific detail whenever a
  scan contained Rust files, degrading Go analysis in Go+Rust monorepos
  and duplicating the Rust caveat across sources. ast-grep extraction is
  not degraded, so it stays authoritative; rust-cargo owns the caveat.
- AddSource appended every source detail to Notes with no dedup, so a
  shared caveat printed twice. Notes and rendered details are now
  deduped at record time and at render time.

newDepsProject mirrors the production shape (ast-grep authoritative +
rust-cargo mixed carrying the note) instead of attaching the note to
ast-grep. Covered by TestAstGrepScanDirectoryRustStaysAuthoritative,
TestGraphCoverageAddSourceDedupesSharedDetail,
TestDepgraphRendersSharedCoverageDetailOnce, and
TestNewDepsProjectRustCaveatOwnedByRustCargoSource.

Co-authored-by: GPT-5.6 Sol <codex@openai.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Keeps ast-grep provenance authoritative while assigning Rust resolution caveats to rust-cargo and deduplicating user-facing coverage notes.

Changes:

  • Corrects Rust coverage source ownership.
  • Deduplicates coverage notes in scanner and rendered output.
  • Adds regression coverage and ignores runtime caches.

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
.gitignore Ignores .cache/ runtime state.
scanner/astgrep.go Keeps successful ast-grep scans authoritative.
scanner/astgrep_test.go Tests Rust scan provenance.
scanner/contracts_test.go Tests default Rust coverage sources.
scanner/outcome.go Deduplicates coverage notes.
scanner/outcome_test.go Tests shared-detail deduplication.
scanner/types.go Assigns the Rust caveat to rust-cargo.
render/depgraph.go Deduplicates rendered source details.
render/depgraph_test.go Tests single rendering of shared details.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread render/depgraph.go
Comment on lines +385 to +387
// Sources are normalized (sorted, deduped by name/status/detail), but
// two distinct sources can still carry the same caveat; render each
// detail once so the warning is not doubled.
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