# Codanna Review
> Diff-driven code review for a branch or PR using codanna's call graph plus difftastic's structural diff. Use when the user asks to "review this PR / branch / diff", "audit callers after my changes", "check what I missed", or otherwise wants to find untouched callers of functions whose signatures or bodies changed between two refs.
- Skill: `antono/codanna-review` (Agent Skill, multi-file: 2 files)
- Install (CLI): `npx skillmds@latest add antono/codanna-review`
- Raw SKILL.md: https://api.skillmd.com/api/skills/antono/codanna-review/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Security
- Author: antono (https://skillmd.com/u/antono)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/antono/codanna-review
---
# Codanna Review
Diff-driven review: given two refs (or a GitHub PR), find every function that
*really* changed (per difftastic's structural diff), enumerate its callers via
codanna, and flag callers whose own bodies were not updated.
Workflow diagram & rationale: [README.md](README.md)
## Scripts
`scripts/walk-review.sh` — end-to-end probe script implementing steps 2–13 for Rust projects. Usage: `REPO=/path/to/repo bash scripts/walk-review.sh [output.md]`.
## When to use
- "Review this branch / PR / diff."
- "Did I miss any callers after changing function X?"
- "What's the blast radius of this signature change?"
- Audit before merge: which callers were updated, which weren't.
**Don't use** for:
- Exploring an unfamiliar codebase → `codanna-explore`.
- Regex-only searches → `rg` directly.
- Pure stylistic / formatter review → out of scope (we explicitly drop those).
## Inputs
Accept either:
1. **Two refs**: `base` and `head`. Default base = `main`; default head = `HEAD`. Three-dot semantics (`base...head`) — compares against merge-base, matching GitHub/GitLab.
2. **GitHub PR**: URL or `owner/repo#N`. Resolve via `gh pr view --json baseRefName,headRefName,baseRefOid,headRefOid 2>/dev/null | toon`, then `gh pr checkout ` (or `git fetch origin pull//head:`) if refs aren't already local.
## Tool surface
| Purpose | Tool | Invocation |
|---|---|---|
| Index status | `get_index_info` MCP | `codanna mcp get_index_info` |
| Symbols in a file | `search_symbols` MCP | `codanna retrieve search "" --module --kind function,method,struct,trait,class --limit 200` |
| Exact symbol | `find_symbol` MCP | `codanna retrieve symbol ` |
| Callers | `find_callers` MCP | `codanna retrieve callers ` |
| Blast radius (high-severity only) | `analyze_impact` MCP | `codanna mcp analyze_impact ` |
| Structural diff | `difft` | `$DIFFT --check-only --exit-code ` and `DFT_UNSTABLE=yes $DIFFT --display=json ` |
| Regex sanity pass | `rg` (fallback `grep`) | `rg -n ''` |
| JSON validate / compact | `toon` | `… --json 2>/dev/null \| toon` |
**`$DIFFT` probe** (do once per session):
```bash
if command -v difft >/dev/null 2>&1; then DIFFT="difft"
else DIFFT="nix run nixpkgs#difftastic --"
fi
```
If `nix` is also missing, set a `NO_DIFFT=1` flag and skip steps 4b/5a — the
workflow degrades to line hunks (severity will report "unknown" for affected
files). Warn the user once before the first `nix run` (cold start can take
several seconds). **Do not** suggest `nix profile install` — out of scope.
## Workflow
### 1. Resolve input → two local refs
- Two refs given: skip ahead.
- PR input: `gh pr view --json baseRefName,headRefName,baseRefOid,headRefOid 2>/dev/null | toon`. Verify both refs are local with `git rev-parse --verify`; if not, `git fetch origin pull//head` or `gh pr checkout `.
- Confirm: `git merge-base ` succeeds.
### 2. Index check + freshness
- `get_index_info`. If `symbol_count: 0`, bootstrap per `codanna-explore` step 2a. Refuse to proceed without an index.
- The `semantic_search.enabled: false` flag is misleading — semantic tools load on demand. Don't bail on it.
- **Precondition 1**: `git rev-parse HEAD` must equal `git rev-parse `. If not: "codanna's working tree is at `` but you're reviewing `` — symbol ranges will be misaligned. Run `git checkout ` first." Refuse to continue.
- **Precondition 2 — force a fresh index.** Codanna's incremental indexing (`codanna index ` without `--force`) leaves stale entries with old line numbers, even for files that have since changed. Confirm with the user, then run **`codanna index --force`** once before step 6. Use `workspace_root` from `.codanna/settings.toml`, not a subdir — `--force` on a subdir wipes the rest of the index. (Observed in pass 2: incremental reindex left `get_context` at line 9 when it had moved to line 22.)
### 3. File enumeration
```bash
git diff --name-only --find-renames ...
```
Result is the candidate file list. Group by `--find-renames` output:
`R\t\t` lines are renames — track `old → new` for later
caller mapping.
### 4. Cosmetic filter (two tiers)
**4a — Whitespace pre-filter** (skip if `NO_DIFFT`):
```bash
for F in ; do
$DIFFT --check-only --exit-code \
<(git show :"$F" 2>/dev/null) \
<(git show :"$F" 2>/dev/null) >/dev/null
# exit 0 = pure whitespace, drop. exit 1 = tokens changed, keep.
done
```
Record dropped files for the report footer ("N files: formatter-only").
**4b — Comment-only filter** (uses step 5a JSON): after step 5a parses the JSON, if **every** `changes[]` entry across all chunks has `highlight: "comment"`, drop the file as cosmetic. Add to the same footer count.
### 5. Extract change spans
**5a — AST spans (preferred)**:
**Critical**: feed difftastic *real files with the right extension* — process substitutions `<(…)` produce FIFOs whose names lack extensions, so difftastic falls back to `language: "Text"` and loses syntax awareness. Use a tempdir:
```bash
tmp=$(mktemp -d)
ext="${F##*.}"
git show ":$F" > "$tmp/old.$ext"
git show ":$F" > "$tmp/new.$ext"
DFT_UNSTABLE=yes $DIFFT --display=json "$tmp/old.$ext" "$tmp/new.$ext" 2>/dev/null | toon
rm -rf "$tmp"
```
Real JSON schema (validated empirically against `difft` v0.66+):
```
{aligned_lines: [[old, new], ...],
chunks: [[{lhs:{line_number, changes:[{start,end,content,highlight}]},
rhs:{line_number, changes:[...]}}, ...]],
language, path, status}
```
`highlight` values seen in the wild: `normal`, `type`, `keyword`, `string`, `delimiter`, `comment` (likely more — accept any string). `line_number` is **0-indexed**; codanna's `range.start_line` is **1-indexed** — add 1 when comparing.
Extract per file:
- **rhs changes** (`chunks[].rhs.changes[]`) → list of `(head_line_0indexed, char_start, char_end, highlight)`. These map to symbols at head.
- **lhs changes** (`chunks[].lhs.changes[]`) → list of base-side change lines. Use these for `removed` symbol detection.
If `toon` parse fails or `language: "Text"` came back unexpectedly (file extension was preserved but difftastic still fell back) → drop to 5b for that file. If `chunks` is empty but `lhs` had changes (pure deletions, all on base), flag the file as `lhs-only` and process it via 5b for the head-side range mapping.
**5b — Line hunks (fallback)**:
```bash
git diff -U0 --merge-base -- "$F"
```
Parse `@@ -a,b +c,d @@` per hunk. Record `(file, base_range, head_range)`. Mark these files as "line-fallback" — severity will report "unknown" for symbols inside.
### 6. Enumerate symbols per file
```bash
codanna retrieve search "fn" --kind function --limit 200 --json 2>/dev/null | toon
```
Then **post-filter by `file_path`** in the response (`data[].symbol.file_path == `). Do **not** use `--module ` — codanna's `--module` filter expects a Rust-style module path (`crate::core::src::vault`), not the file path; passing a file path returns empty.
**Normalize paths before comparing**: codanna stores paths exactly as passed to `codanna index `. If indexed via `codanna index .`, paths come back as `./core/src/foo.rs`; if via `codanna index core/ cli/`, no leading `./`. Strip leading `./` (or use `os.path.normpath`) on both sides before the equality check. Collect `(name, symbol_id, range.start_line, range.end_line)`. Use a non-empty query (`"fn"`, `"struct"`, etc.) — empty queries return `not_found`.
**For files codanna can't symbolize** (returns empty for a non-trivial file, or language unsupported): route by extension to `rg -nP --type '' ` and synthesize pseudo-symbols `(file, line, name)`. Mark severity "unknown" for those.
### 7. Map change spans → symbols
A symbol is **changed** if its `[range.start_line, range.end_line]` intersects any change-span line number. Multiple spans inside one symbol count once. Build the set: `{symbol → list_of_changes_inside_it}`.
### 8. Classify each changed symbol
- `added` — symbol present only at head. **Cheap detection**: `git show : | grep -E "fn $name\b"` returns nothing while `git show : | grep` returns a line. Tag as `added` and drop severity to `low` (callers necessarily target the new signature, there's nothing to "miss").
- `removed` — symbol present only at base. Find by re-running step 6 against `git show :` content (lightweight: parse with `zat` if codanna can't index a temp file).
- `signature-changed` — any change inside the symbol has `head_line + 1 == symbol.range.start_line` (the declaration line). **Beware false positives**: if both base and head signature lines are identical, the change must be on a *neighbouring* line that the codanna range happens to cover (e.g. a doc comment or attribute). Compare the two signature lines via `grep`; if identical, downgrade to `modified`.
- `modified` — changes only on lines `> range.start_line`, none on the signature line.
- `modified-unknown-shape` — symbol came from line-fallback mode (5b); can't distinguish signature from body.
### 9. Find callers
- Always: `codanna retrieve callers symbol_id: --json` (use `symbol_id:` to avoid same-name collisions like `page_list` in two files).
- **Only if** classify is `signature-changed` or `removed`: also `codanna mcp analyze_impact ` for the blast radius.
- **Callers response shape** — `data[]` items are wrapped: each item is `{symbol: {id, name, file_path, range:{start_line, end_line}, ...}, file_path, relationships}`. Read fields from `data[].symbol`, not flat. (The MCP `find_callers` variant may differ — verify per response.)
### 9b. Filter self-callers
Drop any caller where `caller.symbol_id == changed_symbol.id` (same symbol). Codanna's call graph commonly includes the symbol itself as a caller (recursion detection or self-reference artifact). These are pure noise in the report.
### 10. Validate each caller
Two booleans per caller:
- **G1**: is `caller.file_path` in the diff's file list (step 3)?
- **G2**: does `caller.range` intersect a change span (step 5a) in its file?
| G1 | G2 | Status |
|---|---|---|
| no | — | 🔴 untouched |
| yes | no | 🟡 sibling (file changed, this caller's body didn't) |
| yes | yes | 🟢 adapted |
If the caller's file is line-fallback (5b), G2 uses the line hunks instead and the caller is marked as `-fallback` (e.g. `🟢-fallback`).
### 11. Severity
- `signature-changed` + 🔴 → **high**
- `removed` + 🔴 → **high**
- `modified` + 🔴 → **medium**
- `signature-changed` + 🟡 → **medium** (caller's file moved but the signature use didn't)
- anything + 🟢 → **low** (informational)
- any `*-unknown-shape` or `*-fallback` → **unknown**
Sort the report high → medium → unknown → low.
### 12. Always-on regex sanity pass
For each changed symbol's name:
```bash
rg -nw --hidden -g '!.git' -g '!target' '' .
```
`-w` matches whole words (so `run` doesn't hit `runtime`, `running`, etc.). If `rg` surfaces a file not in the codanna caller set, loop back to step 10 for it. This catches macros / dynamic dispatch / FFI / generated code / string-keyed dispatch.
**Interpret with care**: very common identifiers (`run`, `new`, `main`, `get`) produce hundreds of hits even with `-w` and are noise. Surface counts in the report but flag any count above ~50 as "common name — likely noise" rather than as findings. Rare/unique identifiers are where signal lives.
### 13. Report
Markdown, per changed function (severity-ordered):
```markdown
### `add` (signature-changed, **high**)
- Old: `fn add(a: i32, b: i32) -> i32`
- New: `fn add(a: i32, b: i32, c: i32) -> i32`
- File: `src/math.rs:1`
| Caller | Status | Path |
|---|---|---|
| `main` | 🟢 adapted | `src/main.rs:5` |
| `compute_total` | 🔴 untouched | `src/totals.rs:42` |
**Untouched callers may need updating.**
```
Footer:
- "N files dropped as cosmetic (formatter / whitespace / comment-only)" — names listed if N ≤ 10, else just count.
- Regex sanity pass: any extra files surfaced.
- `difft` version (`difft --version | head -1`) for reproducibility.
### 14. Offer markdown+mermaid report (only if `mermaid-diagrams` skill is available)
Ask the user. On yes, invoke `mermaid-diagrams` and emit:
- Impact graph: changed function → callers, colored 🟢/🟡/🔴.
- Optional before/after sequence diagram for high-severity findings.
Do not generate unprompted.
## Output shape — difftastic JSON quick ref
Validated empirically against `difft` v0.66+:
```
{
"aligned_lines": [[old_line|null, new_line|null], …],
"chunks": [
[
{
"lhs": {"line_number": <0-indexed>, "changes": [, …]},
"rhs": {"line_number": <0-indexed>, "changes": [, …]}
},
…
]
],
"language": "Rust",
"path": "",
"status": "changed" | "unchanged"
}
```
`` = `{"start": , "end": , "content": "…", "highlight": }`. Highlights observed: `normal`, `type`, `keyword`, `string`, `delimiter`, `comment`. Accept any string; only `comment` matters for the tier-2 cosmetic filter.
**No `node_type` field**. Signature detection is range-based (does `rhs.line_number + 1` equal codanna's `symbol.range.start_line`?). Comment-only filtering uses `highlight: "comment"`.
`status: "unchanged"` means difftastic considers the files equivalent (whitespace-only); treat the file as cosmetic.
## Common pitfalls
| Pitfall | Fix |
|---|---|
| `--display=json` errors with "unstable feature" | Set `DFT_UNSTABLE=yes` (literal "yes"). |
| difft reports `language: "Text"` for a `.rs` file | Process substitutions `<(…)` produce extensionless FIFOs. Write to a tempdir with the real extension preserved (see step 5a). |
| `--check-only --exit-code` returns 1 for comment-only diffs | Expected — `--check-only` only filters pure whitespace. Add a tier-2 filter: drop files where every `changes[]` has `highlight: "comment"`. |
| difftastic JSON breaks `toon` | Schema drift — degrade that file to line-fallback (step 5b), don't crash. |
| Empty `chunks` but file is in the diff | All changes were on `lhs` (pure deletions, no head-side adds) — process via line-fallback (5b) using `git diff -U0`. |
| `search_symbols` with `--module ` returns empty | `--module` expects Rust module_path (`crate::foo::bar`), not file path. Drop `--module` and post-filter by `file_path` instead. |
| Empty `search_symbols` on a real file | Language not supported by codanna; route by file extension to a regex fallback (`rg -nP --type `). |
| `symbol_id:N` collisions | Use `--json`; field is `data[].symbol.id` for `retrieve search` and `retrieve symbol`. |
| `retrieve callers` returns `None` for ranges | Items are wrapped: read `data[].symbol.range.start_line`, not `data[].range.start_line`. |
| Codanna's symbol ranges don't match diff lines | Codanna index is at a different ref than ``, OR incremental reindex left stale entries. Run `codanna index --force` (full path, not subdir — `--force` on a subdir wipes the rest). |
| `codanna index --force` halves the index | `--force` wipes everything outside the indexed path. Always use `workspace_root` from `.codanna/settings.toml`. |
| Codanna returns paths with `./` prefix | Index was created via `codanna index .`. Normalize (strip leading `./`) before comparing to git's file list. |
| Function is its own caller in codanna | Filter `caller.symbol_id == changed.symbol_id` from the results — codanna emits self-references that are pure noise. |
| `signature-changed` but base/head sig lines identical | Off-by-one with attributes / doc comments next to the function. Compare the actual signature lines via `grep "fn $name\b"`; downgrade to `modified` if they match. |
| `added` symbol misclassified as `signature-changed` | A function present only at head has no base signature. Detect via `git show : \| grep "fn $name\b"` empty → classify as `added`, severity `low`. |
| `rg` sanity-pass count for common names is huge | `run`, `new`, `main`, `get` will hit hundreds of lines even with `-w`. Surface counts but treat anything > ~50 as noise. |
| Off-by-one between difft and codanna | difft `line_number` is 0-indexed; codanna `range.start_line` is 1-indexed. Add 1 to difft lines before overlap checks. |
| `analyze_impact` for every symbol → slow | Only call it for `signature-changed` / `removed`. `find_callers` is enough for `modified`. |
| Renames within a file (not file-level) | Known gap — caller of old name reads 🔴 untouched, which is the safe direction. |
| Cold `nix run` hangs | One-line warning before first invocation. Don't suggest `nix profile install` (can break home-manager). |
| `analyze_impact` doesn't show macro callers | Static analysis gap — the regex sanity pass (step 12) catches it. |
| PR refs not local | `gh pr checkout ` or `git fetch origin pull//head:pr/`. |