Conversation
There was a problem hiding this comment.
Pull request overview
This PR promotes “blast radius” from an external shell wrapper into a first-class Go subcommand (codemap blast-radius), consolidating defaults/output budgeting into the binary and removing the now-redundant script-based entry point.
Changes:
- Added a new
blast-radiussubcommand implementation in Go with bounded Markdown/text/JSON output. - Refactored shared importers/ast-grep hint logic in
main.goto be reused by the new subcommand. - Removed the old
scripts/codemap-blast-radius.shwrapper and updated README + tests accordingly.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/codemap-blast-radius.sh | Deleted legacy wrapper script in favor of native CLI subcommand. |
| README.md | Updated blast-radius usage examples to use codemap blast-radius. |
| main.go | Dispatches blast-radius subcommand pre global-flag parsing; refactors helper rendering/hint code usage. |
| main_test.go | Ensures --help output includes the new blast-radius command. |
| blast_radius.go | New Go implementation for bundle building, budgeting, rendering, and snippet extraction. |
| blast_radius_test.go | Adds integration-style coverage validating Markdown + JSON blast-radius output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func blastLineMatchesTerm(line string, term blastSnippetTerm) bool { | ||
| switch term.Kind { | ||
| case "symbol": | ||
| pattern := regexp.MustCompile(`\b` + regexp.QuoteMeta(term.Value) + `\b`) | ||
| return pattern.MatchString(line) | ||
| default: | ||
| return strings.Contains(line, term.Value) | ||
| } |
There was a problem hiding this comment.
blastLineMatchesTerm recompiles a regexp for every line/term check (regexp.MustCompile inside the hot loop). This is unnecessarily expensive for large files and can dominate runtime when snippet search runs across many impacted/context files. Consider precompiling the regex once per term (e.g., store a compiled *regexp.Regexp on the term, or compile once per term iteration in findBlastRadiusSnippet) and reusing it across lines.
blast_radius.go
Outdated
| fmt.Println("codemap blast-radius - Build a compact, bounded blast-radius bundle") | ||
| fmt.Println() | ||
| fmt.Println("Usage:") | ||
| fmt.Println(" codemap blast-radius [--json|--markdown|--text] [--ref <base-ref>] [path]") | ||
| fmt.Println() | ||
| fmt.Println("Examples:") | ||
| fmt.Println(" codemap blast-radius --ref main .") | ||
| fmt.Println(" codemap blast-radius --json --ref develop /path/to/repo") | ||
| fmt.Println() | ||
| fmt.Println("Flags:") | ||
| fs.PrintDefaults() | ||
| fmt.Println() | ||
| fmt.Println("Environment overrides:") | ||
| fmt.Println(" CODEMAP_BLAST_MAX_TOTAL_CHARS") | ||
| fmt.Println(" CODEMAP_BLAST_MAX_CHANGED_FILES") | ||
| fmt.Println(" CODEMAP_BLAST_MAX_AFFECTED") | ||
| fmt.Println(" CODEMAP_BLAST_MAX_CONTEXT") | ||
| fmt.Println(" CODEMAP_BLAST_MAX_SNIPPETS") | ||
| fmt.Println(" CODEMAP_BLAST_MAX_SNIPPETS_PER_CHANGED") | ||
| fmt.Println(" CODEMAP_BLAST_SNIPPET_RADIUS") | ||
| fmt.Println(" CODEMAP_BLAST_MAX_SNIPPET_CHARS") | ||
| fmt.Println(" CODEMAP_BLAST_MAX_DIFF_CHARS") | ||
| fmt.Println(" CODEMAP_BLAST_MAX_DEPS_CHARS") | ||
| fmt.Println(" CODEMAP_BLAST_MAX_IMPORTERS_CHARS") | ||
| fmt.Println(" CODEMAP_BLAST_MAX_IMPORTER_FILES") | ||
| fmt.Println(" CODEMAP_BLAST_MAX_IMPORTERS_PER_FILE") |
There was a problem hiding this comment.
printBlastRadiusUsage writes directly to stdout via fmt.Println, but the subcommand’s FlagSet is configured with fs.SetOutput(os.Stderr). This can split usage output across stdout/stderr (especially when Parse prints errors/usage), which is brittle for callers and tests. Consider writing usage to fs.Output() (or otherwise consistently routing help vs error output).
| fmt.Println("codemap blast-radius - Build a compact, bounded blast-radius bundle") | |
| fmt.Println() | |
| fmt.Println("Usage:") | |
| fmt.Println(" codemap blast-radius [--json|--markdown|--text] [--ref <base-ref>] [path]") | |
| fmt.Println() | |
| fmt.Println("Examples:") | |
| fmt.Println(" codemap blast-radius --ref main .") | |
| fmt.Println(" codemap blast-radius --json --ref develop /path/to/repo") | |
| fmt.Println() | |
| fmt.Println("Flags:") | |
| fs.PrintDefaults() | |
| fmt.Println() | |
| fmt.Println("Environment overrides:") | |
| fmt.Println(" CODEMAP_BLAST_MAX_TOTAL_CHARS") | |
| fmt.Println(" CODEMAP_BLAST_MAX_CHANGED_FILES") | |
| fmt.Println(" CODEMAP_BLAST_MAX_AFFECTED") | |
| fmt.Println(" CODEMAP_BLAST_MAX_CONTEXT") | |
| fmt.Println(" CODEMAP_BLAST_MAX_SNIPPETS") | |
| fmt.Println(" CODEMAP_BLAST_MAX_SNIPPETS_PER_CHANGED") | |
| fmt.Println(" CODEMAP_BLAST_SNIPPET_RADIUS") | |
| fmt.Println(" CODEMAP_BLAST_MAX_SNIPPET_CHARS") | |
| fmt.Println(" CODEMAP_BLAST_MAX_DIFF_CHARS") | |
| fmt.Println(" CODEMAP_BLAST_MAX_DEPS_CHARS") | |
| fmt.Println(" CODEMAP_BLAST_MAX_IMPORTERS_CHARS") | |
| fmt.Println(" CODEMAP_BLAST_MAX_IMPORTER_FILES") | |
| fmt.Println(" CODEMAP_BLAST_MAX_IMPORTERS_PER_FILE") | |
| var out io.Writer = fs.Output() | |
| if out == nil { | |
| out = os.Stderr | |
| } | |
| fmt.Fprintln(out, "codemap blast-radius - Build a compact, bounded blast-radius bundle") | |
| fmt.Fprintln(out) | |
| fmt.Fprintln(out, "Usage:") | |
| fmt.Fprintln(out, " codemap blast-radius [--json|--markdown|--text] [--ref <base-ref>] [path]") | |
| fmt.Fprintln(out) | |
| fmt.Fprintln(out, "Examples:") | |
| fmt.Fprintln(out, " codemap blast-radius --ref main .") | |
| fmt.Fprintln(out, " codemap blast-radius --json --ref develop /path/to/repo") | |
| fmt.Fprintln(out) | |
| fmt.Fprintln(out, "Flags:") | |
| fs.PrintDefaults() | |
| fmt.Fprintln(out) | |
| fmt.Fprintln(out, "Environment overrides:") | |
| fmt.Fprintln(out, " CODEMAP_BLAST_MAX_TOTAL_CHARS") | |
| fmt.Fprintln(out, " CODEMAP_BLAST_MAX_CHANGED_FILES") | |
| fmt.Fprintln(out, " CODEMAP_BLAST_MAX_AFFECTED") | |
| fmt.Fprintln(out, " CODEMAP_BLAST_MAX_CONTEXT") | |
| fmt.Fprintln(out, " CODEMAP_BLAST_MAX_SNIPPETS") | |
| fmt.Fprintln(out, " CODEMAP_BLAST_MAX_SNIPPETS_PER_CHANGED") | |
| fmt.Fprintln(out, " CODEMAP_BLAST_SNIPPET_RADIUS") | |
| fmt.Fprintln(out, " CODEMAP_BLAST_MAX_SNIPPET_CHARS") | |
| fmt.Fprintln(out, " CODEMAP_BLAST_MAX_DIFF_CHARS") | |
| fmt.Fprintln(out, " CODEMAP_BLAST_MAX_DEPS_CHARS") | |
| fmt.Fprintln(out, " CODEMAP_BLAST_MAX_IMPORTERS_CHARS") | |
| fmt.Fprintln(out, " CODEMAP_BLAST_MAX_IMPORTER_FILES") | |
| fmt.Fprintln(out, " CODEMAP_BLAST_MAX_IMPORTERS_PER_FILE") |
blast_radius.go
Outdated
| fs.BoolVar(&markdownMode, "markdown", false, "Emit Markdown output (default)") | ||
| fs.BoolVar(&markdownMode, "md", false, "Emit Markdown output (default)") |
There was a problem hiding this comment.
The --markdown/--md flags are labeled as “default”, but their default value is false and Markdown is emitted even when the flags are omitted. This makes --markdown redundant and can confuse users reading PrintDefaults() output. Consider removing --markdown/--md and treating Markdown as the implicit default, or replacing the three booleans with a single --format flag defaulting to markdown.
| fs.BoolVar(&markdownMode, "markdown", false, "Emit Markdown output (default)") | |
| fs.BoolVar(&markdownMode, "md", false, "Emit Markdown output (default)") | |
| fs.BoolVar(&markdownMode, "markdown", false, "Emit Markdown output") | |
| fs.BoolVar(&markdownMode, "md", false, "Emit Markdown output") |
blast_radius.go
Outdated
| if isGitHubURL(root) && localErr != nil { | ||
| repoName := extractRepoName(root) | ||
| tempDir, err := cloneRepo(root, repoName) | ||
| if err != nil { | ||
| return "", cleanup, err | ||
| } | ||
| cleanup = func() { _ = os.RemoveAll(tempDir) } | ||
| root = tempDir |
There was a problem hiding this comment.
resolveBlastRadiusRoot clones when os.Stat(root) returns any error (localErr != nil). That includes non-"not exist" errors (e.g., permission denied), which can unexpectedly trigger a network clone and ignore an existing local path. Consider only cloning when os.IsNotExist(localErr) is true, and otherwise return the stat error to the caller.
| if isGitHubURL(root) && localErr != nil { | |
| repoName := extractRepoName(root) | |
| tempDir, err := cloneRepo(root, repoName) | |
| if err != nil { | |
| return "", cleanup, err | |
| } | |
| cleanup = func() { _ = os.RemoveAll(tempDir) } | |
| root = tempDir | |
| if isGitHubURL(root) { | |
| if os.IsNotExist(localErr) { | |
| repoName := extractRepoName(root) | |
| tempDir, err := cloneRepo(root, repoName) | |
| if err != nil { | |
| return "", cleanup, err | |
| } | |
| cleanup = func() { _ = os.RemoveAll(tempDir) } | |
| root = tempDir | |
| } else if localErr != nil { | |
| return "", cleanup, localErr | |
| } |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dbf8d83ab3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
blast_radius.go
Outdated
| } else { | ||
| fmt.Fprintf(os.Stderr, "Error building blast radius: %v\n", err) | ||
| } | ||
| os.Exit(1) |
There was a problem hiding this comment.
Avoid os.Exit after deferring remote clone cleanup
When root is a remote URL, resolveBlastRadiusRoot clones into a temp directory and defer cleanup() is set, but this error path calls os.Exit(1), which skips deferred functions. In runs where bundle construction fails (for example invalid --ref or missing ast-grep with changed files), each invocation leaks a temp clone on disk instead of removing it.
Useful? React with 👍 / 👎.
blast_radius.go
Outdated
| func blastLineMatchesTerm(line string, term blastSnippetTerm) bool { | ||
| switch term.Kind { | ||
| case "symbol": | ||
| pattern := regexp.MustCompile(`\b` + regexp.QuoteMeta(term.Value) + `\b`) |
There was a problem hiding this comment.
Precompile symbol-match regex outside the line loop
findBlastRadiusSnippet checks every line of candidate files, and for symbol terms it calls blastLineMatchesTerm per line; that helper recompiles the same regex every call via regexp.MustCompile. On large files or many snippet candidates this creates thousands of unnecessary compilations and can materially slow blast-radius generation.
Useful? React with 👍 / 👎.
What changed
codemap blast-radiussubcommand implemented in Goscripts/codemap-blast-radius.shso the CLI command is the single supported entry pointWhy
The blast-radius flow was still living in a shell script with
jqandpython3dependencies. That made CI integration awkward and forced callers to know wrapper paths and environment defaults. This change makescodemap blast-radiusthe actual product surface with sensible built-in defaults and bounded output.User impact
Callers can now use:
without needing the old script path or external wrapper logic.
Validation
go test ./...go run . blast-radius --ref HEAD <temp-repo>