feat(audit): codemap audit --base <ref> — worktree+reindex strategy (impl of PR #51 plan)#52
Merged
SutuSebastian merged 6 commits intomainfrom May 3, 2026
Merged
Conversation
- parseAuditRest gains --base <ref> / --base=<ref>; rejects --base + --baseline as mutually exclusive (clean error message); per-delta --<delta>-baseline still composes orthogonally. - runAuditCmd dispatches to runAuditFromRef when opts.base is set; otherwise unchanged baseline-prefix path. - makeWorktreeReindex injects the runtime save/restore around the worktree-side reindex (initCodemap mutates global state — restore needed so live-DB ops after the reindex still target the original project). - main.ts forwards parsed.base. - Help text updated with --base examples. - .gitignore: .codemap/audit-cache/ (don't track cached worktrees; .codemap/recipes/ stays tracked). - Smoke: bun src/index.ts audit --base HEAD~1 --json --summary works on this repo (4 files added vs HEAD~1, 7 deps, 0 deprecated). Second run hits cache (~110ms). - 6 new parser tests cover --base alone, --base=, mutual exclusivity, per-delta composition, missing value.
- audit-engine.ts: makeWorktreeReindex lifted from cmd-audit.ts (single source of truth — both transports inject the same factory). - tool-handlers.ts: handleAudit gains base/baseline_prefix mutual-exclusion guard + dispatches to runAuditFromRef when args.base set. - mcp-server.ts: audit tool description rewritten to document the 3 snapshot sources (base / baseline_prefix / baselines per-delta) + their composition rules. - 2 new MCP tests: rejects base + baseline_prefix combo, returns clean error in non-git project. - HTTP POST /tool/audit lights up automatically via the existing dispatcher (Tracer 4 next).
The dispatcher already validates with auditArgsSchema (which gained the `base?: string` field in Tracer 3) — this commit just adds tests: - audit + base + baseline_prefix combo → 400 mutually exclusive - audit + base in non-git project → 400 'requires a git repository' Both transports (MCP + HTTP) now expose the new --base path through the same handleAudit pure function.
…skill + changeset (Tracer 5) - README.md: 'audit --base origin/main' code block + cache caveat - docs/architecture.md: Audit wiring extended with --base details (worktree + atomic populate + cache + LRU + non-git error + GIT_* env hygiene); audit-engine.ts inventory mentions runAuditFromRef + makeWorktreeReindex; audit-worktree.ts added to application/ inventory - docs/glossary.md: 'audit --base / git-ref baseline' entry; existing 'audit' entry now mentions 3 mutually-exclusive sources - docs/roadmap.md: drop the v1.x backlog item (it shipped) - docs/research/fallow.md: B.5 row updated to 'v1 + v1.x partial shipped'; Adjacent-shipped bullet referencing PR #51 (plan) + #52 (impl) - .agents/rules/codemap.md + templates/agents/rules/codemap.md: new 'Audit vs git ref' table row + paragraph rewrite (Rule 10 lockstep) - .agents/skills/codemap/SKILL.md + templates/agents/skills/codemap/SKILL.md: audit tool args + behavior reflect new 'base' field (Rule 10 lockstep) - src/agents-init.ts: ensureGitignoreCodemapPattern now adds '.codemap/audit-cache/' alongside '.codemap.*' (the audit cache lives inside .codemap/ but .codemap/recipes/ stays git-tracked); test updated. - .changeset/codemap-audit-base.md: minor — full design rationale - docs/plans/audit-base.md: deleted per docs-governance Rule 3
🦋 Changeset detectedLatest commit: 1c6263a The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
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. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThis PR implements the ChangesAudit --base Feature
Sequence DiagramsequenceDiagram
participant CLI as CLI/Handler
participant Cache as Worktree Cache<br/>(audit-worktree)
participant Git as Git CLI
participant Reindex as Reindex Engine
participant AuditDB as Worktree .codemap.db
participant LiveDB as Live .codemap.db
participant Engine as Audit Engine<br/>(runAuditFromRef)
CLI->>Cache: populateWorktree(sha, reindex)
alt Cache Hit
Cache-->>CLI: PopulatedCacheEntry
else Cache Miss
Cache->>Git: git rev-parse validate
activate Git
Git-->>Cache: sha valid
deactivate Git
Cache->>Git: git worktree add --detach
activate Git
Git-->>Cache: worktree created
deactivate Git
Cache->>Reindex: reindex(tmpPath)
activate Reindex
Reindex->>AuditDB: createTables + index src/*
Reindex-->>Cache: ✓
deactivate Reindex
Cache->>Cache: atomic rename<br/>tmpPath → finalPath
Cache-->>CLI: PopulatedCacheEntry
end
CLI->>Engine: runAuditFromRef(db, ref, reindex)
Engine->>AuditDB: SELECT * FROM files<br/>(canonical query)
AuditDB-->>Engine: base rows
Engine->>LiveDB: SELECT * FROM files<br/>(same query)
LiveDB-->>Engine: live rows
Engine->>Engine: diffRows(base, live)<br/>→ added/removed
Engine-->>CLI: AuditEnvelope<br/>{head, deltas:<br/>{files, dependencies,<br/>deprecated}}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 8 minutes and 3 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
docs/research/fallow.md (1)
14-27: 💤 Low valueDocumentation accurately reflects shipped features.
The status table update correctly documents the
--base <ref>feature shipped in PR#52, including the worktree+reindex implementation, mutual exclusion with--baseline, and the deferred verdict/threshold configuration. All technical details match the implementation shown in the relevant code snippets.Optional clarity improvement: The status "
⚠️ Partial — v1 + v1.x partial shipped" might confuse readers since--base <ref>itself is fully shipped (the "partial" refers to deferred verdict/threshold features). Consider rephrasing to something like "⚠️ Partial — v1 + v1.x shipped; verdict deferred" to make it clearer that the shipped portions are complete.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/research/fallow.md` around lines 14 - 27, Update the B.5 table row wording to clarify that the `--base <ref>` worktree+reindex feature from PR `#52` is fully shipped while the verdict/threshold configuration is deferred: replace the status text "⚠️ Partial — v1 + v1.x partial shipped" with something like "⚠️ Partial — v1 + v1.x shipped; verdict/threshold deferred" in the row labeled B.5 (referencing `codemap audit`, `--baseline <prefix>`, `--base <ref>`, and the deferred `verdict`/`threshold` items).src/cli/cmd-audit.test.ts (1)
204-208: 💤 Low valueConsider adding empty-value edge cases for
--baseto match--baselinecoverage
--baselinehas two empty-value tests (lines 128–132 for--baseline=and lines 140–144 for--baseline "");--baseonly has the no-value case. Adding the same two forms keeps coverage symmetric:✅ Suggested additions
+ it("errors when --base= has empty value", () => { + const r = parseAuditRest(["audit", "--base="]); + expect(r.kind).toBe("error"); + if (r.kind === "error") expect(r.message).toContain("non-empty"); + }); + + it("errors when --base gets an empty-string value (two-token form)", () => { + const r = parseAuditRest(["audit", "--base", ""]); + expect(r.kind).toBe("error"); + if (r.kind === "error") expect(r.message).toContain("--base"); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/cmd-audit.test.ts` around lines 204 - 208, Add two additional test cases for the `--base` flag in the same suite that uses `parseAuditRest`: one asserting an error when `["audit", "--base="]` is passed and one asserting an error when `["audit", "--base", ""]` (or `["audit", '--base', '']`) is passed; each should mirror the existing `--baseline` empty-value tests by checking `r.kind === "error"` and that `r.message` contains `"--base"`, using the same structure as the current `it("errors when --base has no value", ...)` test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/rules/codemap.md:
- Line 56: The doc contains a stale internal spec reference "B.6" in the Audit
description—replace the phrase `"from saved B.6 baselines"` with ``
`query_baselines` `` (also update the identical occurrence in
templates/agents/rules/codemap.md at the corresponding spot), ensuring the
wording now reads that baseline resolution comes from `` `query_baselines` ``;
keep surrounding sentence structure intact and run a quick repo-wide search for
any remaining "B.6" references in codemap docs to update them consistently.
In `@src/application/audit-engine.ts`:
- Around line 337-350: The code in makeWorktreeReindex temporarily swaps global
runtime state via initCodemap(...) and configureResolver(...) around
runCodemapIndex(...), which can interleave across concurrent requests; protect
the critical region by adding a process-level async mutex that is acquired
before loading wtUser/ calling initCodemap/ configureResolver/ runCodemapIndex
and released after wtDb.close() and restoring savedConfig; ensure the lock
covers the entire try/finally so savedConfig is always restored even on errors.
Alternatively (preferred long-term), refactor runCodemapIndex to accept an
explicit codemap config/resolver context so makeWorktreeReindex no longer calls
initCodemap or configureResolver and thus avoids global mutation (update call
sites of runCodemapIndex and remove the global init/restore in this function).
In `@src/application/audit-worktree.test.ts`:
- Around line 62-72: The helper currently ignores the exit code of the
spawnSync("git", ["commit", ...]) call and immediately reads HEAD; modify the
helper so you capture the commit result (e.g., const commit = spawnSync("git",
["commit", "-m", message, "--no-gpg-sign"], {...})), check commit.status (or
commit.error) and throw an Error including commit.stdout/commit.stderr if it is
non-zero/failed, and only proceed to call spawnSync("git", ["rev-parse",
"HEAD"]) and return sha after the commit succeeded.
In `@src/application/audit-worktree.ts`:
- Around line 203-210: evictIfOverLimits currently can delete the newly-created
worktree; change its API to accept an exclude/protected path (e.g., excludePath:
string | undefined) and modify its logic to skip removing or counting that path
when sweeping, then update the two call sites that pass finalPath (the return
block around PopulatedCacheEntry and the other block at 244-289) to pass the new
excludePath so the freshly-populated worktree cannot be evicted; ensure callers
compile and add a brief unit/test to cover the "do not evict protected path"
behavior.
- Around line 149-150: The temp worktree name generation (tmpName/tmpPath) can
collide when multiple concurrent requests share sha, pid and timestamp; update
the logic in the block that builds tmpName/tmpPath to include a
cryptographically-unique suffix (e.g., crypto.randomUUID() or
crypto.randomBytes-based hex) or use an atomic temp-directory primitive
(fs.mkdtemp-style) so each tmpPath is collision-proof; ensure you reference the
existing symbols tmpName, tmpPath and opts.sha when modifying the
naming/creation so downstream rename/race handling remains unchanged.
In `@src/application/mcp-server.ts`:
- Line 168: Update the audit docstring to reflect the real exclusivity rule:
change the text to state there are two mutually exclusive primary sources
("base" vs "baseline_prefix") and that "baselines" are optional per-delta
overrides that may compose with either primary source; update the descriptive
paragraph in src/application/mcp-server.ts (the block that documents
Structural-drift audit) and ensure any related skill docs or exported doc string
near handleAudit references the same phrasing so the API docs match the
handleAudit implementation.
---
Nitpick comments:
In `@docs/research/fallow.md`:
- Around line 14-27: Update the B.5 table row wording to clarify that the
`--base <ref>` worktree+reindex feature from PR `#52` is fully shipped while the
verdict/threshold configuration is deferred: replace the status text "⚠️ Partial
— v1 + v1.x partial shipped" with something like "⚠️ Partial — v1 + v1.x
shipped; verdict/threshold deferred" in the row labeled B.5 (referencing
`codemap audit`, `--baseline <prefix>`, `--base <ref>`, and the deferred
`verdict`/`threshold` items).
In `@src/cli/cmd-audit.test.ts`:
- Around line 204-208: Add two additional test cases for the `--base` flag in
the same suite that uses `parseAuditRest`: one asserting an error when
`["audit", "--base="]` is passed and one asserting an error when `["audit",
"--base", ""]` (or `["audit", '--base', '']`) is passed; each should mirror the
existing `--baseline` empty-value tests by checking `r.kind === "error"` and
that `r.message` contains `"--base"`, using the same structure as the current
`it("errors when --base has no value", ...)` test.
🪄 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: bae86c6e-fb2a-47f0-b573-91b4c972d86b
📒 Files selected for processing (25)
.agents/rules/codemap.md.agents/skills/codemap/SKILL.md.changeset/codemap-audit-base.md.gitignoreREADME.mddocs/architecture.mddocs/glossary.mddocs/plans/audit-base.mddocs/research/fallow.mddocs/roadmap.mdsrc/agents-init.test.tssrc/agents-init.tssrc/application/audit-engine.test.tssrc/application/audit-engine.tssrc/application/audit-worktree.test.tssrc/application/audit-worktree.tssrc/application/http-server.test.tssrc/application/mcp-server.test.tssrc/application/mcp-server.tssrc/application/tool-handlers.tssrc/cli/cmd-audit.test.tssrc/cli/cmd-audit.tssrc/cli/main.tstemplates/agents/rules/codemap.mdtemplates/agents/skills/codemap/SKILL.md
💤 Files with no reviewable changes (2)
- docs/roadmap.md
- docs/plans/audit-base.md
2 tasks
Threads (all 6 actionable, no hallucinations): 1. .agents/rules + templates: stale 'B.6' spec leak → 'query_baselines entries' (consistent terminology) 2. audit-engine.ts: process-level mutex (_reindexChain promise queue) around makeWorktreeReindex's runtime-singleton swap. Concurrent HTTP/MCP audits no longer interleave initCodemap/configureResolver state. Long-term cleanup (runCodemapIndex takes explicit context) tracked separately. 3. audit-worktree.test.ts: commitFiles fixture now checks git commit exit code; throws with stderr on failure (was silently passing against previous commit) 4. audit-worktree.ts: tmpName gains randomUUID() suffix (defensive against cross-process races; in-process serialised by mutex from #2) 5. audit-worktree.ts: evictIfOverLimits accepts protectPath; populateWorktree passes finalPath so the freshly-populated entry can't evict itself when it alone exceeds MAX_CACHE_BYTES. New test covers the protect path. 6. mcp-server.ts + .agents/skills + templates: rephrased 'three mutually exclusive sources' to 'two mutually exclusive PRIMARY sources + optional per-delta overrides' (matches handleAudit's actual exclusivity rule) Nitpicks: - fallow.md status: 'v1 + v1.x partial shipped' → 'v1 + v1.x shipped; verdict deferred' (clarity) - cmd-audit.test.ts: added --base= empty-value + --base "" two-token-empty-string parser tests for symmetry with --baseline coverage
Contributor
Author
|
Triaged the 6 inline threads + 2 nitpicks per `pr-comment-fact-check`. All 8 actionable, no hallucinations on this round — applied in 1c6263a. Threads (all 6 ✅ correct):
Nitpicks (both ✅ applied):
All 692 tests still green. Ready for re-review or merge. |
SutuSebastian
added a commit
that referenced
this pull request
May 3, 2026
…naged .gitignore (#53) * docs(plan): .codemap/ directory consolidation — single root + self-managed .codemap/.gitignore Collapse the dual-pattern surface (.codemap.db at root + .codemap/<thing>/ for caches) to a single .codemap/ dir with a tracked .codemap/.gitignore that blacklists generated artifacts. Mirrors flowbite-react's pattern (.flowbite-react/.gitignore lists class-list.json + pid; everything else tracked by default). Closes the per-feature agents-init.ts .gitignore patching surface — every new cache today (audit-cache shipped in #52) requires a user-facing .gitignore line; after this lands, new caches just bump the blacklist in .codemap/.gitignore on next codemap boot. DB path migration: .codemap.db → .codemap/index.db via atomic rename on first run; shim ships in v1.x, removed in v2. Recipes stay at .codemap/recipes/ (user-authored source, not generated). Plan only — implementation follows after CodeRabbit review per the established workflow. * docs(plan): refine — drop migration shim (pre-v1), make state-dir configurable, move config into <state-dir> Per user feedback: 1. Pre-v1 → no migration shim needed (D2 simplified to 'rm .codemap.db once'); existing two dev clones each pay one cleanup. 2. State directory is configurable: --state-dir CLI / CODEMAP_STATE_DIR env, default .codemap/ (D7). 3. Config file moves from <root>/codemap.config.{ts,json} → <state-dir>/config.{ts,js,json} (D8) — chicken-and-egg avoided by resolving state-dir from CLI/env only, not from config content. 4. flowbite-react pattern endorsed (D1 unchanged) — blacklist generated artifacts; tracked sources default visible. 5. Self-managed .gitignore caveat (less discoverable than root .gitignore) re-evaluated as a non-problem — git check-ignore + universal nested-gitignore convention cover it. Tracers reduced from 5 to 5 (re-shaped): state-dir resolver → config loader move → state-gitignore writer → agents-init updates → docs+changeset+plan deletion. No migration tracer. * docs(plan): add D11 self-healing files principle (codemap-flavored, not flowbite-react clone) Adopts the IDEA from flowbite-react (every config file owned by an idempotent setup function; setup logic IS the migration) but expressed in codemap's own architectural vocabulary: - ensure* naming (matches existing ensureGitignoreCodemapPattern) - Engine layer in src/application/state-dir.ts (not flowbite's commands/setup-*.ts) - Single Zod schema as both runtime validator and TS source via z.infer (codemap already uses Zod in tool-handlers.ts) - .gitignore reconciler enforces canonical verbatim with a 'codemap-managed — edits will be overwritten' header (no marker block, no merge logic) - TS configs validated-only (don't rewrite user code); JSON configs self-heal across versions Tracer 2 reshaped to expose pure { before, after, written } return values for testability — side effects at the edge per codemap's existing layering. Bootstrap orchestrator fans out to ensureStateGitignore + ensureStateConfig (and any future ensure*) so adding a new self-healing file is one-line registration. * docs(plan): apply CodeRabbit threads — D12 → D8, fix table escaping, tense - Thread 1: layout sketch comment '(D12)' → '(D8)' (only D1-D11 exist) - Thread 2: alternatives table inline-code escaping fixed (backslashes inside backticks render literally; not needed) - Nitpick 2: D2 tense aligned with 'design — no code' status ('this PR moves' → 'the implementation will move') - Nitpick 1 (split D11) declined: D11 is intentionally an integrated story for the self-healing principle; splitting fragments the narrative
2 tasks
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
Implementation of PR #51's
codemap audit --base <ref>plan. Closes the highest-frequency post-watch agent loop: "what changed structurally between this branch and `origin/main`?". Replaces today's 3-step `--baseline` dance with one verb.Three transports, one engine: CLI (
codemap audit --base <ref>), MCP (audittool with newbase?arg), HTTP (POST /tool/audit). All three dispatch the same purerunAuditFromRefengine inapplication/audit-engine.ts.Tracers
application/audit-worktree.ts+runAuditFromRef— sha-keyed cache; atomic populate via per-pid temp dir + POSIX rename (free single-flight, no lock files); LRU eviction (5 entries / 500 MiB);AuditBasewidened to discriminated union (baseline/ref); GIT_* env strip on every spawn so a containing git op (e.g. husky hook) doesn't route worktree calls at the wrong index. 15 unit tests including end-to-end against a real git fixture.cmd-audit.ts--base <ref>— parser + mutual-exclusion guard + runner dispatch; help text + smoke test (bun src/index.ts audit --base HEAD~1 --json --summaryworks on this repo). 6 new parser tests.base?: z.string()—auditArgsSchemaextended;handleAuditdispatches torunAuditFromRefwhen set; mutual-exclusion guard mirrors CLI; tool description rewritten.makeWorktreeReindexlifted intoaudit-engine.tsas the shared production reindex factory (both CLI and MCP/HTTP get the same one). 2 new MCP tests.POST /tool/audit— auto-wired via existing dispatcher; Zod validation lights up automatically. 2 new integration tests.Smoke
```bash
$ bun src/index.ts audit --base HEAD
1 --json --summary | jq '.deltas.files'1", "sha": "e107bead537af82ae9e3815df51af5e1a00c6f10", "indexed_at": ... },{
"base": { "source": "ref", "ref": "HEAD
"added": 4,
"removed": 0
}
Second run hits the cache (~110ms total vs ~3s on first run)
```
Decisions worth knowing (full set in changeset)
Test plan
Summary by CodeRabbit
Release Notes
New Features
--base <ref>flag tocodemap auditfor comparing against any git commit reference.gitignoremanagement for audit cache directoryDocumentation
--base,--baseline, and per-delta overrides