docs(plan): codemap impact — symbol/file blast-radius walker#49
Merged
SutuSebastian merged 2 commits intomainfrom May 3, 2026
Merged
docs(plan): codemap impact — symbol/file blast-radius walker#49SutuSebastian merged 2 commits intomainfrom
SutuSebastian merged 2 commits intomainfrom
Conversation
…/callees, depth-bounded, multi-graph) Replaces the "agent composes WITH RECURSIVE by hand" tax — `impact` is to the dep/calls graph what `show` (PR #39) was for symbol lookup. Single verb, multi-backend (calls + dependencies + imports), depth- and limit-bounded, cycle-detected. Pure transport-agnostic engine reused by CLI / MCP / HTTP per the post-PR #41 layering. Plan only — implementation lands in a follow-up PR after review.
|
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis PR adds documentation for a proposed ChangesCodemap Impact Feature Plan
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 52 minutes and 50 seconds.Comment |
Contributor
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/plans/impact.md`:
- Line 24: Update the broken markdown anchor for the link "[§
Alternatives](`#alternatives`)" so it matches the actual heading slug in the
document: locate the heading that corresponds to the Alternatives section (e.g.,
"Alternatives", "Alternatives considered", or similar) and replace the fragment
"#alternatives" in the link with the exact slug generated from that heading
(matching Markdown heading-to-slug rules) so the jump link navigates correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 969944aa-1548-4684-a540-38a4285a9ee3
📒 Files selected for processing (2)
docs/plans/impact.mddocs/roadmap.md
2 tasks
SutuSebastian
added a commit
that referenced
this pull request
May 3, 2026
…of PR #49 plan) (#50) * feat(impact): pure transport-agnostic walker engine (Tracer 1) application/impact-engine.ts — single WITH RECURSIVE per (direction, backend), JS-side merge + dedup + summary. Backends: - calls (symbol-level, up=callers / down=callees) - dependencies (file-level) - imports (file-level, resolved_path only) Plan §D2 backend split, §D6 cycle detection (path-string + instr), §D8 edge tags. 27 unit tests cover: walks (up/down/both), depth caps, limit caps, self-loop + 3-cycle, file-vs-symbol resolution, --via skipped_backends. * feat(impact): cmd-impact.ts CLI verb wired into main + bootstrap (Tracer 2) - src/cli/cmd-impact.ts — parser + runner mirroring cmd-show.ts shape - main.ts dispatch, bootstrap.ts validateIndexModeArgs + printCliUsage - 14 parser tests cover happy paths + 9 error cases (--depth/limit/direction/via validation) Smoke: `bun src/index.ts impact src/db.ts --direction up --depth 1` lists the real fan-in on this repo (every file in src/ that depends on db.ts). `bun src/index.ts impact handleQuery --json --summary` returns the summary shape ready for jq pipelines. * feat(impact): MCP impact tool wired through tool-handlers (Tracer 3) - impactArgsSchema + handleImpact in tool-handlers.ts (Zod + ToolResult shape) - registerImpactTool in mcp-server.ts (one-line wrapper, like show/snippet) - 5 MCP integration tests (tools/list, symbol target, file target, summary trims matches but keeps summary.nodes count, Zod rejects float depth) HTTP POST /tool/impact lights up automatically next tracer via the existing http-server.ts dispatcher (one switch arm). * feat(impact): HTTP POST /tool/impact dispatcher (Tracer 4) - TOOL_NAMES gains 'impact' (auto-listed in /tools) - dispatchTool switch arm with Zod validation (400 on bad body) - 4 integration tests: happy path, missing target, non-int depth, bad direction enum Both transports (MCP + HTTP) now expose impact via the same handleImpact pure function. * docs(impact): sync README + architecture + glossary + agent rule/skill + changeset (Tracer 5) - README.md: new 'Impact analysis' code block + impact in MCP tools list - docs/architecture.md: 'Impact wiring' section + impact-engine in application/ inventory - docs/glossary.md: 'codemap impact / impact tool' entry under I (full envelope spec) - docs/roadmap.md: drop the impact backlog item (it shipped) - docs/research/fallow.md: Adjacent-shipped bullet referencing PR #49 (plan) + #50 (impl) - .agents/rules/codemap.md + templates/agents/rules/codemap.md: new table row + 'Impact' paragraph + impact in MCP tools list (Rule 10 lockstep) - .agents/skills/codemap/SKILL.md + templates/agents/skills/codemap/SKILL.md: impact tool description (Rule 10 lockstep) - .changeset/codemap-impact.md: minor — full design rationale + decisions - docs/plans/impact.md: deleted per docs-governance Rule 3 (canonical descriptions now live in architecture / glossary / README) * chore(impact): slim self-authored comments per concise-comments rule Re-read every comment authored in this PR; cut/slim ones a teammate could re-derive in <30s. Targets: - impact-engine.ts: drop trivial type doc, slim findImpact JSDoc, slim termination heuristic from 6 lines → 2, drop redundant inline notes - cmd-impact.ts + tool-handlers.ts: shorten the --summary trim explanation No behaviour change; checks green (89 tests + lint + typecheck). * chore(impact): apply CodeRabbit nitpicks 1+4 - impact-engine.ts:333 — one-line comment explaining empty-string file_path fallback (external/dynamic call sites) - impact-engine.test.ts:209 — strengthen exact-fit limit test from not.toBe('limit') to toBe('exhausted') Pushed back on nitpicks 2+3 (hallucinated — see PR comment) and 5 (style; deterministic order is fine to lock).
SutuSebastian
added a commit
that referenced
this pull request
May 3, 2026
…(v1.x backlog) (#51) * docs(plan): codemap audit --base <ref> — worktree + reindex strategy (v1.x backlog) Plan for the next-best agent-value loop: PR-review structural-diff. Replaces today's 3-step --baseline dance with one verb. Reuses 90% of the existing audit infrastructure (PR #33); only new piece is the worktree+reindex snapshot path. Cache-by-resolved-sha; LRU 5/500 MiB; mutual-exclusive with --baseline; per-delta override compatible. Hard error on non-git projects (no graceful fallback — there's no meaningful 'ref' without git). Plan only — implementation follows after CodeRabbit review per the impact (#49→#50) / watch (#46→#47) workflow. * docs(plan): address CodeRabbit findings — atomic cache populate (D11), worktree-as-cache lifecycle clarity, TS type widening callout - D1/D2/D8 rewritten: worktree IS the cache entry (kept until LRU evicts); cleanup runs only on reindex failure rollback OR LRU eviction. The earlier ambiguity (D2 said 'cache by sha' while D8 said 'remove in finally') is resolved. - D11 added: atomic cache populate via per-pid temp dir + POSIX rename → free single-flight semantics; no lock files needed. Same pattern for eviction. Closes the race CodeRabbit flagged on concurrent CI matrix runs against the same sha. - AuditBase TS type widening to discriminated union called out explicitly above the Decisions table (Tracer 1 ships it). - CODEMAP_AUDIT_CACHE_SIZE env var mention dropped — was promising an unimplemented config knob; v1 hardcodes the limits, defer to v1.x+.
2 tasks
SutuSebastian
added a commit
that referenced
this pull request
May 4, 2026
…age` table) (#56) * docs(plan): static coverage ingestion (Istanbul JSON → `coverage` table) Plans the C.11 candidate from `research/fallow.md` — `codemap ingest-coverage <path>` reads Istanbul `coverage-final.json` into two new tables (`coverage` symbol-level + `file_coverage` rollup), joinable to `symbols` for the killer "what's structurally dead AND untested?" recipe in one query. Resolves the open question from `fallow.md § 6` ("symbols column vs separate table?") in favour of a separate table with `ON DELETE CASCADE` (D1) — coverage shape evolves independently of structural columns; LEFT JOIN keeps NULL semantics explicit; rows survive `--full` reindex via the `query_baselines` precedent (D6). Key decisions: - Istanbul JSON in v1; LCOV in v1.x; raw V8 traces never (D3, fallow's paid moat). - One-shot `ingest-coverage` verb decoupled from `codemap` index runs (D4) — coverage cadence (per `bun test --coverage`) ≠ index cadence (per file edit). - Statement coverage only in v1 (D5); branch/function deferred until a consumer asks. - MCP/HTTP exposure as a query column, not a separate `coverage` tool (D9) — composes with every existing recipe + ad-hoc SQL. - `codemap audit --delta coverage` deferred to v1.x (D10) — raw schema first. Five-tracer plan: schema bump → engine → CLI verb → fixture + golden recipe → docs. Plan only — implementation follows after CodeRabbit review per the established workflow (PRs #46/47, #49/50, #51/52, #53/54). * docs(plan): fact-check fixes — drop hallucinated SQL/projection/runner claims Self-audit against the actual codebase surfaced four claims that didn't hold: 1. Killer recipe SQL referenced `callee_id` — `calls` is name-keyed (`callee_name TEXT`, no symbol-id FK; see `db.ts` `CallRow`). Rewrote the "no callers" predicate as `NOT EXISTS (… WHERE callee_name = s.name)`. 2. D7 claimed line-range projection is "the same `markers` already uses" — `markers` is line-pinned (`line_number INTEGER`), no projection. Reworded as "novel for this plan" with the actual mechanic spelled out. 3. D3 listed `bun test --coverage` as an Istanbul JSON emitter — `bun test --help` shows only `text` / `lcov` reporters today. Removed bun from the Istanbul-emitters list; left vitest/jest/c8/nyc with the explicit reporter flags they need. 4. D12 contradicted D6 ("rows absent until re-ingest" vs "rows survive `--full`"). Reconciled: empty is the correct initial state on first bump; subsequent bumps preserve via the `dropAll()` exclusion. Quoted the `lessons.md` policy verbatim instead of paraphrasing. * docs(plan): v2 — fix CASCADE hazard + innermost-wins projection + nits Self-grilling found two real schema design holes that would block execution: 1. **D6 CASCADE hazard.** Original draft keyed `coverage` on `symbol_id REFERENCES symbols(id) ON DELETE CASCADE`. Every `--full` reindex calls `dropAll()` → drops `symbols` → CASCADE wipes coverage, regardless of whether `coverage` itself was excluded from `dropAll()`. Recreated `symbols` get fresh auto-increment IDs anyway → coverage permanently lost without re-ingest. Fix: natural-key PK `(file_path, name, line_start)` — no FK to `symbols.id`. Survives the `symbols` drop-recreate cycle. Trade-off: orphan rows when files are deleted; cleaned by one explicit `DELETE FROM coverage WHERE file_path NOT IN (SELECT path FROM files)` after every ingest. 2. **D7 overlapping symbols.** Original draft: `line_start ≤ stmt_line ≤ line_end` matches every enclosing scope. With nested symbols (class methods inside classes, closures inside functions), one Istanbul statement projects onto 3+ symbols, inflating `total_statements` 2-3×. Fix: innermost-wins via `(line_end - line_start) ASC LIMIT 1`. New `skipped.statements_no_symbol` counter for statements that fall outside every symbol range (top-level expressions, side-effect imports). Nits cleared in the same pass: - D2: drop `file_coverage` rollup table from v1 (aggregateable via GROUP BY on the symbol-level table; doubling sources of truth without a benchmark is premature). Promote to v1.x with a real query. - D11: spec the `total_statements = 0 → coverage_pct IS NULL` edge case + document the cross-file name-collision lossiness in the killer recipe. - Drop `--prune` flag (orphan cleanup is unconditional, no flag needed). - Drop per-row `source` column (single meta key sufficient; one ingest at a time). - Update killer recipe SQL to use the natural-key 3-column join. - Drop made-up "~50 LoC LCOV ingester" estimate and "<50 ms / <1 ms / ~500 KB" performance numbers (no benchmark backed them). - Tracer 1 / 2 / 3 acceptance criteria updated to match the new schema. Plan is now ready for tracer-1 implementation. CodeRabbit pass deferred (rate-limited 57m). * docs(plan): tighten Bun-native API references (file read + perf note) Plan correctly inherits the established Node vs Bun runtime split, but the single tracer-3 reference understated it. Now: - Tracer 3 cites `packaging.md § Node vs Bun` as the canonical pattern source instead of pointing at config.ts in passing. - Performance section calls out the actual lever — `Bun.file(path).json()` uses Bun's native JSON parser, materially faster than V8 `JSON.parse` on multi-MB Istanbul payloads (real coverage files for medium codebases routinely hit several MB). No new Bun-native API surfaces are added — the feature doesn't need globbing, file writes, spawn, or hashing beyond what the existing engines already use through their abstractions. * docs(plan): v3 — ship LCOV in v1 + drop --source flag + bundle killer recipe The "fully capable, no half-way APIs" principle reshapes three things: 1. **LCOV ingester ships in v1** alongside Istanbul. Original draft deferred LCOV to v1.x, which would exclude `bun test --coverage` users — i.e. codemap's own primary runtime. That's the textbook half-baked surface the principle bans. Two parser front-ends share one `upsertCoverageRows` core; LCOV is regex tokenizing over `SF:` / `DA:` / `end_of_record`. Tracer 2 splits into 2a (shared core + Istanbul parser) and 2b (LCOV parser), both writing identical normalised CoverageRow[] into the same upsert path. 2. **`--source istanbul|lcov` flag dropped.** Auto-detection from extension (`.json` → istanbul, `.info` → lcov, directory → probe both, error on ambiguous) is unambiguous; a flag for "tell codemap what it can already see" is API noise. Misnamed files can be renamed (one-liner) cheaper than codemap can grow a flag. 3. **Killer recipe ships as bundled `untested-and-dead.{sql,md}`** in `templates/recipes/`. Per the recipes-as-content registry (PR #37), the high-value queries become first-class agent surface. A buried doc snippet would be invisible to agents at session start; the bundled recipe shows up in `--recipes-json` and gets a `codemap query --recipe untested-and-dead` direct invocation. Tracer 4 also fans out: Istanbul + LCOV fixtures cover the same partial coverage shape; three golden recipes (`coverage-istanbul.json`, `coverage-lcov.json`, `untested-and-dead.json`) prove format equivalence. Out-of-scope, alternatives, performance section, title, and goal statement all updated to match. * docs(plan): v4 — agent-journey audit + bundled recipe shelf (D13) Walked every D / OOS / tracer item against "fully capable + agent first-class + no half-baked APIs". Found three half-baked surfaces: 1. **D2 deferral leaks "compose GROUP BY yourself" onto the agent.** Deferring the `file_coverage` table is correct (no benchmark proves it's needed) — but the agent-facing answer for "rank files by coverage" was missing. Fix: keep table deferral, ship a bundled `files-by-coverage.{sql,md}` recipe so the GROUP BY view IS first-class. 2. **D11 name-collision lossiness was acknowledged but unmitigated.** The killer recipe's `callee_name = s.name` cross-file lossiness was documented in the recipe SQL comment, but the recipe `.md` didn't give the agent any narrowing pattern. Now D11 ships three concrete narrowing patterns in the `.md` (file_path scope, default- export filter, exported-only restriction) so the agent has workable mitigations on day one. 3. **Missing recipe shelf for common agent questions.** Walking the journey: only "What's structurally dead AND untested?" had a recipe; "Rank files by coverage" and "Worst-covered exported symbols" forced ad-hoc SQL. Three recipes fully cover the agent journey end-to-end. New D13 codifies the bundled-recipe principle: every common agent question gets a `--recipe` verb. Three v1 recipes: - `untested-and-dead.{sql,md}` (killer, with name-collision mitigations) - `files-by-coverage.{sql,md}` (replaces D2's table deferral) - `worst-covered-exports.{sql,md}` (top-N agent ask) Each `.md` carries a frontmatter `actions` block (per PR #26) so agents get per-row follow-up hints. All three appear in `--recipes-json` automatically — agents discover them at session start. New "Agent journey" section makes the principle visible: a table mapping every common agent question to the v1 verb that answers it. If a row ever shows "compose SQL yourself" without a recipe, the surface is half-baked and needs a recipe before tracer 1 ships. Tracer 4 expanded: ships all three recipes + five golden snapshots (adds files-by-coverage.json + worst-covered-exports.json on top of the three existing). Tracer 5 expanded: glossary + agent rule trigger table gain three new rows. Plan now passes the principle audit end-to-end.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
codemap impact <target>— single-verb walker over the calls / dependencies / imports graphs (callers, callees, both), depth- and limit-bounded, cycle-detected.application/impact-engine.ts) reused by CLI / MCP / HTTP per the post-PR refactor: lift cli/* engine helpers into application/* (close all layer-reversal imports) #41 layering.Why
Post-PR #47 (
codemap watch), the remaining high-friction agent loop is "what's the blast radius if I change X?" — agents currently composeWITH RECURSIVESQL by hand, which they do unreliably and without depth/cycle bounds.impactis to the dep / calls graph whatshow(PR #39) was for symbol lookup: replaces hand-rolled SQL with one verb.See
docs/plans/impact.mdfor the full design — sketched API, output envelope, 10 decisions (D1-D10), 5 tracers, perf considerations, alternatives rejected, and out-of-scope list.Test plan
Summary by CodeRabbit
codemap impact <target>CLI command to analyze symbol and file dependency relationships with configurable traversal depth.