Add repo intelligence map - #157
Conversation
Zero automated PR reviewVerdict: No blockers found Blockers
Validation
ScopeHead: This deterministic review checks validation status and basic diff hygiene. A human reviewer still owns product judgment and design quality. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughAdds a deterministic repository map: filesystem scanner, indexed snapshot with counts/tree, search/ranking, a budgeted prompt renderer, CLI command ChangesRepository Map Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 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 docstrings
🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (4)
internal/repomap/search.go (1)
178-188: 💤 Low valueMinor: Rune-to-string conversion in fuzzyMatch.
Line 185 converts each rune to a string to measure its UTF-8 byte length. This works correctly but could use
utf8.RuneLen(char)directly for clarity and a tiny performance gain.♻️ Optional simplification
+import ( + "unicode/utf8" +) func fuzzyMatch(filePath string, query string) bool { position := 0 for _, char := range query { index := strings.IndexRune(filePath[position:], char) if index < 0 { return false } - position += index + len(string(char)) + position += index + utf8.RuneLen(char) } return true }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/repomap/search.go` around lines 178 - 188, In fuzzyMatch replace the rune-to-string conversion used to advance position (len(string(char))) with utf8.RuneLen(char) for clarity and slight performance gain; update the imports to include "unicode/utf8" if missing and ensure the logic still advances position by index + utf8.RuneLen(char) inside the loop in function fuzzyMatch.internal/repomap/repomap.go (1)
62-69: 💤 Low valueMaxDepth: 0 silently becomes DefaultMaxDepth.
A caller passing
Options{MaxDepth: 0}explicitly might expect "scan only root-level files" rather than "use the default depth of 6". The current<= 0check conflates unset (zero value) with explicit zero.Consider distinguishing unset (use default) from explicit zero (root only) by checking for negative values only, or document that zero means "use default".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/repomap/repomap.go` around lines 62 - 69, The current logic treats zero for MaxDepth as "use default" which hides an explicit zero meaning "root only"; change the defaulting check to only apply for negative values so that options.MaxDepth == 0 is preserved (e.g., replace the if maxDepth <= 0 { maxDepth = DefaultMaxDepth } with a negative-only check such as if maxDepth < 0 { maxDepth = DefaultMaxDepth }), and consider applying the same negative-only defaulting for options.MaxFiles/DefaultMaxFiles (function/vars: options.MaxDepth, DefaultMaxDepth, options.MaxFiles, DefaultMaxFiles) to keep behavior consistent.internal/agent/system_prompt.go (1)
106-115: AlignrepoMapContextwith existing “best-effort” context behavior (and document/optionally instrument scan failures)
workspaceContextalready treats supplementary context as optional: it only appends the repo map whenrepoMapContext(cwd)is non-empty (lines ~100-102), and it similarly skips missing/unreadable project guideline files.gitBranchForPromptexplicitly documents the same approach—return""on any problem so the prompt simply omits that segment.Given this, silent
repomap.Scanfailure handling inrepoMapContext(lines ~111-113) is consistent. To avoid ambiguity for maintainers/operators, add a short comment inrepoMapContext(or emit debug-level logging/metrics) stating that scan failures are intentionally ignored/treated as “no repo map” rather than “feature disabled.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/agent/system_prompt.go` around lines 106 - 115, repoMapContext currently swallows repomap.Scan errors and returns an empty string; make that behavior explicit by adding a short comment in the repoMapContext function (or optionally a debug log/metric) stating that repomap.Scan failures are intentionally treated as “no repo map” (i.e., best-effort supplemental context) rather than a fatal error; reference the repomap.Scan call and the repomap.RenderPrompt/maxRepoMapContextBytes return so maintainers see where the silent-failure behavior is applied.internal/cli/repo_map.go (1)
161-187: 💤 Low valueQuery mode may silently skip matches if paths aren't found.
In query mode (lines 166-171), if a match path from
repomap.Searchdoesn't exist insnapshot.Files, it's silently skipped (line 168). This could causelen(report.Files)to be less thanlen(report.Matches), potentially confusing users. However, this should only happen if there's a bug in the Search implementation. Consider adding a comment explaining this assumption.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/cli/repo_map.go` around lines 161 - 187, The loop in buildRepoMapReport that maps repomap.Search results to snapshot.Files may silently drop matches when repoMapFileByPath(snapshot.Files, match.Path) returns false; add an in-line comment above that loop explaining the assumption that repomap.Search will only return paths present in snapshot.Files (i.e., matches and snapshot are expected to be consistent), and also add a lightweight safeguard (e.g., a debug log or a comment noting that if a missing path occurs it indicates a bug in repomap.Search) referencing repomap.Search, match.Path, and repoMapFileByPath to make the intent and expectation explicit.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/repomap/repomap.go`:
- Around line 74-85: The walk callback passed to filepath.WalkDir currently
swallows filesystem errors by returning nil; update the callback (the anonymous
func used in filepath.WalkDir for current/entry/err under cleanRoot) to mark the
scan as truncated (set truncated = true) and propagate/return the encountered
error instead of nil when err != nil or when filepath.Rel fails, so callers see
the failure and the snapshot is flagged incomplete.
---
Nitpick comments:
In `@internal/agent/system_prompt.go`:
- Around line 106-115: repoMapContext currently swallows repomap.Scan errors and
returns an empty string; make that behavior explicit by adding a short comment
in the repoMapContext function (or optionally a debug log/metric) stating that
repomap.Scan failures are intentionally treated as “no repo map” (i.e.,
best-effort supplemental context) rather than a fatal error; reference the
repomap.Scan call and the repomap.RenderPrompt/maxRepoMapContextBytes return so
maintainers see where the silent-failure behavior is applied.
In `@internal/cli/repo_map.go`:
- Around line 161-187: The loop in buildRepoMapReport that maps repomap.Search
results to snapshot.Files may silently drop matches when
repoMapFileByPath(snapshot.Files, match.Path) returns false; add an in-line
comment above that loop explaining the assumption that repomap.Search will only
return paths present in snapshot.Files (i.e., matches and snapshot are expected
to be consistent), and also add a lightweight safeguard (e.g., a debug log or a
comment noting that if a missing path occurs it indicates a bug in
repomap.Search) referencing repomap.Search, match.Path, and repoMapFileByPath to
make the intent and expectation explicit.
In `@internal/repomap/repomap.go`:
- Around line 62-69: The current logic treats zero for MaxDepth as "use default"
which hides an explicit zero meaning "root only"; change the defaulting check to
only apply for negative values so that options.MaxDepth == 0 is preserved (e.g.,
replace the if maxDepth <= 0 { maxDepth = DefaultMaxDepth } with a negative-only
check such as if maxDepth < 0 { maxDepth = DefaultMaxDepth }), and consider
applying the same negative-only defaulting for options.MaxFiles/DefaultMaxFiles
(function/vars: options.MaxDepth, DefaultMaxDepth, options.MaxFiles,
DefaultMaxFiles) to keep behavior consistent.
In `@internal/repomap/search.go`:
- Around line 178-188: In fuzzyMatch replace the rune-to-string conversion used
to advance position (len(string(char))) with utf8.RuneLen(char) for clarity and
slight performance gain; update the imports to include "unicode/utf8" if missing
and ensure the logic still advances position by index + utf8.RuneLen(char)
inside the loop in function fuzzyMatch.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 83154fa0-193a-47ab-b6c4-b66b9e5294b1
📒 Files selected for processing (11)
internal/agent/loop_test.gointernal/agent/system_prompt.gointernal/cli/app.gointernal/cli/repo_map.gointernal/cli/repo_map_test.gointernal/repomap/prompt.gointernal/repomap/prompt_test.gointernal/repomap/repomap.gointernal/repomap/repomap_test.gointernal/repomap/search.gointernal/repomap/search_test.go
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/repomap/repomap_test.go (1)
142-160: 💤 Low valueTest passes by coincidence—all files are at root level.
This test sets
Options{MaxFiles: 2}withoutMaxDepth. Since all test files (a.go,b.go,c.go) are at depth 0, it works. If a subdirectory with files existed, the implicitMaxDepth: 0would silently exclude them.If the
MaxDepthzero-value semantic stays as-is, consider adding a test that creates files in subdirectories and asserts they're not included whenMaxDepthis unset—making the behavior explicit rather than accidental.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/repomap/repomap_test.go` around lines 142 - 160, TestScanHonorsTraversalCaps currently passes only because all files are at root; update the test (TestScanHonorsTraversalCaps) to explicitly cover MaxDepth semantics by adding a subdirectory with a file (e.g., create dir "sub" and write "sub/d.go") and then call Scan(root, Options{MaxFiles: 2}) and assert that the file in the subdirectory is not included in got.Files and that got.Truncated is true; this makes the implicit MaxDepth=0 behaviour explicit rather than accidental (alternatively, if you prefer to assert inclusion, set Options{MaxFiles:2, MaxDepth:0} explicitly).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/repomap/repomap_test.go`:
- Around line 142-160: TestScanHonorsTraversalCaps currently passes only because
all files are at root; update the test (TestScanHonorsTraversalCaps) to
explicitly cover MaxDepth semantics by adding a subdirectory with a file (e.g.,
create dir "sub" and write "sub/d.go") and then call Scan(root,
Options{MaxFiles: 2}) and assert that the file in the subdirectory is not
included in got.Files and that got.Truncated is true; this makes the implicit
MaxDepth=0 behaviour explicit rather than accidental (alternatively, if you
prefer to assert inclusion, set Options{MaxFiles:2, MaxDepth:0} explicitly).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6d056eb4-433f-4b1d-bc73-2f193e81a5ee
📒 Files selected for processing (5)
internal/agent/system_prompt.gointernal/cli/repo_map.gointernal/repomap/repomap.gointernal/repomap/repomap_test.gointernal/repomap/search.go
🚧 Files skipped from review as they are similar to previous changes (3)
- internal/agent/system_prompt.go
- internal/repomap/search.go
- internal/cli/repo_map.go
gnanam1990
left a comment
gnanam1990
left a comment
There was a problem hiding this comment.
Reviewed the full diff (built + go test ./... && go vet ./... green). This is a clean, well-bounded package — nice work. Verdict: looks good; two minor fixes worth making + a couple of notes.
What's great
- Scanner is deterministic and well-bounded —
maxFiles/maxDepthcaps, skips VCS/dep/build dirs and binary extensions, and skips symlinks (both dirs and files), so it can't follow a link out of the workspace or into a cycle. - Prompt integration is exactly right:
repoMapContextis fail-open (scan error → omit the map, the run continues), bounded (300 files / depth 5), and the render is capped byclampBudgetwhich is byte-budget and UTF-8-safe (trimValidUTF8walks back to a rune boundary) — avoids the classic truncation-splits-a-rune bug. - Paths are normalized to relative with
..-escape guards (relativePromptPath), so nothing outside the root leaks into the prompt. - Search ranking is solid (exact → prefix → segment → substring → fuzzy, multi-term bonus, deterministic tiebreak), and the subsequence
fuzzyMatchis correct.
Minor — worth fixing
- An unreadable subdir aborts the whole scan. In
repomap.go:77-79, a non-nilwalkErris returned from theWalkDircallback, which stops the entire walk;Scanthen returns the partial snapshot plus the error. So a singleEACCESdirectory makeszero repo-mapfail outright (and silently drops the prompt map). For a best-effort map, skip just that entry and keep going:if walkErr != nil { truncated = true if entry != nil && entry.IsDir() { return filepath.SkipDir } return nil }
--max-filesdoesn't bound the scan.repo_map.go:55callsrepomap.Scan(root, repomap.Options{MaxDepth: options.maxDepth})—options.maxFilesis only applied to the output list and the search limit inbuildRepoMapReport, never torepomap.Options.MaxFiles. So--max-files 8still walks up to the 2000 default and reportsfiles=2000in the summary while listing 8. Either thread it intoScantoo, or rename/document it as an output cap — right now the same name means two different things across the two layers.
Notes (non-blocking)
- Three workspace-scan packages now:
repomapjoinsrepoinfo(#150) andcontextreport(#139), each re-implementing the directory walk + extension→language map + skip-list. Worth a follow-up to share one primitive before the three drift (they'll eventually disagree on which dirs to skip / how an extension maps). - Awareness only: file paths for dotfiles / a
secrets/-style dir will appear in the injected map (only a few specific dirs are skipped). Paths aren't secrets and it isn't exfiltrated, so this is fine — just flagging.
Thanks — the bounding and fail-open discipline here are exactly right.
gnanam1990
left a comment
gnanam1990
left a comment
There was a problem hiding this comment.
Requesting changes for the two minor-but-concrete items from my review above — both quick, and better fixed before this ships as a public command + an always-on system-prompt feature:
- Unreadable subdir aborts the whole scan (
repomap.go:77-79) — returnfilepath.SkipDir/nil+ settruncatedinstead of returningwalkErr, so oneEACCESdir doesn't failzero repo-map/ silently drop the prompt map. --max-filesdoesn't bound the scan (repo_map.go:55) — it's applied to output/search-limit only, notrepomap.Options.MaxFiles, so--max-files 8still walks the 2000 default and the summary reportsfiles=2000. Thread it intoScan, or rename/document it as an output cap.
Everything else looks great — the scanner bounding, fail-open prompt injection, and the UTF-8-safe render cap are all spot on. Happy to re-review as soon as these two land. The package-consolidation note (repomap/repoinfo/contextreport) is a non-blocking follow-up.
gnanam1990
left a comment
gnanam1990
left a comment
There was a problem hiding this comment.
Re-reviewed (rebuilt + go test ./internal/repomap ./internal/cli green). Both items are resolved cleanly — approving. 🎉
- Scan-abort fixed (and better than I suggested):
handleWalkErrorskips a subdir/file error (SkipDir/nil) and continues, while still treating a root error as fatal — a sharper distinction than just skipping everything. Nice touch adding regression tests for both the skip and the root-error paths. --max-filessemantics fixed: split into--max-files(files/matches shown) and the new--scan-max-files(scan bound, default 2000), both clearly documented in the help. No morefiles=2000surprise.
The package-consolidation note (repomap/repoinfo/contextreport sharing one walk + language map) remains a non-blocking follow-up whenever you're in there next. Thanks for the fast, thorough turnaround.
2 participants
Summary
Adds Go-native repository intelligence for Zero so the agent starts each run with a compact source map instead of only cwd/project docs.
internal/repomapfor deterministic workspace scanning, language/extension counts, important files, source-focused artifact skips, compact prompt rendering, and ranked path searchzero repo-mapwith text, JSON, and--querymodes for local inspection## Repo mapsection into the agent system prompt through the existingOptions.CwdpathWhy
This is the next high-leverage agent-quality slice: before editing, Zero needs lightweight awareness of repo layout, important files, and likely file targets without adding embeddings, MCP, plugins, or external dependencies.
Validation
gofmt -l internal\repomap\repomap.go internal\repomap\prompt.go internal\repomap\search.go internal\repomap\repomap_test.go internal\repomap\prompt_test.go internal\repomap\search_test.go internal\cli\repo_map.go internal\cli\repo_map_test.go internal\cli\app.go internal\agent\system_prompt.go internal\agent\loop_test.gogo test ./...go vet ./...go run ./cmd/zero-release buildgo run ./cmd/zero-release smokegit diff --check(passed; Windows printed CRLF conversion warnings only)go run ./cmd/zero repo-map --max-files 8 --max-depth 2go run ./cmd/zero repo-map --json --max-files 2 --max-depth 2go run ./cmd/zero repo-map --query "agent runtime" --max-files 5Summary by CodeRabbit
New Features
Tests