feat: review info popup (i) with --description and aggregate stats by melonamin · Pull Request #155 · umputun/revdiff

melonamin · 2026-04-27T12:33:46Z

Summary

Adds a unified review-info popup on i showing invocation scope, file/status histogram, aggregate +/- line stats, optional markdown description, and the commit log.
Adds --description / --description-file CLI flags so an agent can explain the diff inside the popup; file reads are size-capped, regular-file-only, and ANSI-sanitized.
Pre-rename commit_info keybindings keep working via a deprecated alias that rewrites to the canonical info action with a one-time WARN.

Closes #154
Closes #130

Test plan

make test (race + coverage) passes
make lint passes
Open i against a git/hg/jj repo and confirm scope rows, file histogram, line stats, and commit log render correctly
Pass --description "**hello**" and --description-file path/to/notes.md and verify markdown highlights inside the popup
Add map i commit_info to ~/.config/revdiff/keybindings and confirm the deprecation warning + that i still opens the popup
R reload during an open popup does not surface stale stats

umputun

tests / lint / race all green, security clean, sanitization layers and the lazy-stats sequencing are well thought out. before merging I'd like a few cleanups, mostly project-convention items:

test file organization (per ~/.claude/rules/testing.md, "one test file per source file"):

app/reviewinfo_test.go (package main) tests reviewInfoFromOptions, resolveDescription, maxDescriptionFileSize — these live in app/main.go. extract them out of main.go (which already grew +98) into a new app/reviewinfo.go (package main). matches the existing themes.go+themes_test.go, stdin.go+stdin_test.go, config.go+config_test.go shape.
app/ui/reviewinfo.go (354 lines) has all its UI-side tests piled into app/ui/loaders_test.go (~13 ReviewStats tests, plus header/footer/lines text). split them into a new app/ui/reviewinfo_test.go.

methods vs standalone functions (project rule: functions called only from methods MUST be methods, prefer methods over standalone helpers):

app/diff/hg.go:203 translateRef() — should be *Hg method (app/diff/jj.go:256 does this correctly with the same shape)
app/ui/overlay/overlay.go:400 injectBorderEdgeText() — *Manager method
app/ui/reviewinfo.go:349 reviewListFlag() — Model method
app/ui/overlay/info.go:507/522/534/554 splitLines / padPlain / truncateForDisplay / sanitizeInfoText — all called only from *infoOverlay, should be methods
app/keymap/keymap.go:434 dumpKeyName() — *Keymap method (already recursive-ready)

test-only code in production types (per project preference against test-only fields):

ReviewInfoConfig.Enabled (app/ui/model.go:348) — doc-commented as a test-only off-switch but baked into a production type with m.review.cfg.Enabled checks scattered across reviewinfo.go. clean shape: ReviewInfo *ReviewInfoConfig (nil = off), thread nil-checks instead. small surface now, grows otherwise.
commitsState.hint (app/ui/model.go:331) — doc admits "no longer set" in production, only tests write to it. either remove the field or remove the test writes.

parameter-count smells (4+ → option struct, project rule):

app/review/stats.go:53 ComputeStats(differ, ref, staged, workDir, entries) — 5 params
app/main.go:277 reviewInfoFromOptions(opts, workDir, vcsType, description) — 4 params, awkward "options + extras" mix
app/review/stats.go:101 readUntracked(workDir, realWorkDir, relPath, prevPartial) — 4 params, three same-typed strings (silent-swap risk)
app/ui/overlay/overlay.go:381,390 injectBorderTitle / injectBorderFooter — 5 params each
app/ui/overlay/info.go:214 renderDetailRow(label, value, labelW, innerWidth, muted, reset) — 6 params with two same-type pairs (label,value, muted,reset), reorder risk

unnecessary export:

app/ui/overlay/info.go:514 NormalizeNewlines — doc-comment claims external use, but rg overlay.NormalizeNewlines returns nothing. unexport.

deprecated-alias WARN messaging:

PR body says "one-time WARN" but app/keymap/keymap.go:546 actually logs once per occurrence of commit_info in the keybindings file. either track logged-once across program lifetime (small loggedAliases map on the parser scope), or relax the wording in the PR description and the alias doc-comment to "logged for each occurrence."

minor style nits:

app/diff/hg.go:49 hgStatusToFileStatus has a redundant hg prefix when called as (h *Hg).hgStatusToFileStatus. sibling *Jj uses statusToFileStatus. align.
app/review/stats_test.go:80 resolveWorkDir() test helper duplicates inline production logic at stats.go:60-64. extract a small package-level helper used by both — the doc-comment already acknowledges the duplication.
app/ui/reviewinfo.go:162 highlightDescription() is a one-line accessor returning a public-ish field that's already read directly elsewhere. inline it.

none of these are blocking individually, but the test-organization items are repeats of the project's stated convention and the methods-vs-standalone batch is a list of seven that have a single clear rule. happy to merge once those are addressed.

Copilot

Pull request overview

Adds a unified “info” overlay (key i) that combines review scope metadata, aggregate change stats, optional markdown description, and (when applicable) the commit log; also updates CLI/docs/plugins to support --description / --description-file and preserves legacy commit_info keybindings via a deprecated alias.

Changes:

Replaces the commit-info overlay with a unified info overlay that can be updated in-place while async data (commits/stats) loads.
Adds description plumbing (--description, --description-file) with size-capped, regular-file-only reads and sanitization/highlighting.
Introduces lazy aggregate +/- stats computation and updates docs/plugins/keybindings accordingly.

Reviewed changes

Copilot reviewed 46 out of 46 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
site/index.html	Updates marketing copy to mention the new `i` info popup contents.
site/docs.html	Documents review description and updates `i` popup wording/options list.
plugins/pi/skills/revdiff/SKILL.md	Adds `--description` usage example for the PI plugin skill.
plugins/codex/skills/revdiff/references/usage.md	Documents `--description` / `--description-file` and updates `i` binding text.
plugins/codex/skills/revdiff/references/config.md	Adds new CLI-only flags to config reference; updates action list to `info`.
plugins/codex/skills/revdiff/SKILL.md	Updates launcher args and guidance to include description flags.
package.json	Bumps package version.
docs/ARCHITECTURE.md	Updates architecture docs to reflect `infoOverlay` and new overlay manager API.
app/ui/style/style_key_enum.go	Renames style key from commit-info box to info box.
app/ui/style/resolver_test.go	Updates style resolver tests to use `StyleKeyInfoBox`.
app/ui/style/resolver.go	Renames style mapping to `StyleKeyInfoBox`.
app/ui/style/enums.go	Renames internal enum to `styleKeyInfoBox`.
app/ui/reviewinfo.go	Adds model-side review info assembly (header/footer rows, lazy stats triggers, description highlighting).
app/ui/overlay/overlay_test.go	Updates overlay manager tests for `OpenInfo`.
app/ui/overlay/overlay.go	Replaces `CommitInfoSpec` with `InfoSpec`, adds `UpdateInfo`, and injects footer text into borders.
app/ui/overlay/info_test.go	Adds comprehensive tests for the new info overlay rendering/scrolling/sanitization.
app/ui/overlay/info.go	Implements the unified info overlay (description + details + commits) with scrolling and border header/footer injection.
app/ui/overlay/commitinfo_test.go	Removes legacy commit-info overlay tests.
app/ui/overlay/commitinfo.go	Removes legacy commit-info overlay implementation.
app/ui/mouse_test.go	Updates mouse-wheel overlay scrolling test to target info overlay.
app/ui/model_test.go	Updates overlay-open tests to return `(model, cmd, handled)` and uses `ActionInfo`.
app/ui/model.go	Adds review-info state/config, replaces `commit_info` action path with `info`, wires lazy stats + overlay refresh.
app/ui/loaders_test.go	Adds tests for lazy stats loading, stale seq handling, description highlighting, and overlay refresh behavior.
app/ui/loaders.go	Resets review stats on reload, batches deferred stats fetch after files load, and refreshes open info overlay when commits/stats land.
app/ui/handlers_test.go	Updates handler tests to validate info overlay behavior (opens in all modes, loading state, errors).
app/reviewinfo_test.go	Adds tests for `reviewInfoFromOptions` and `resolveDescription` (size cap, regular file checks, exclusivity).
app/review/stats_test.go	Adds tests for aggregate stats computation and path-safety behavior for untracked fallbacks.
app/review/stats.go	Adds `review.ComputeStats` with added-file staged fallback and guarded untracked reads.
app/renderer_setup.go	Tracks detected VCS type in setup to feed review-info config.
app/main.go	Plumbs description resolution, constructs `ReviewInfoConfig`, and implements `resolveDescription`.
app/keymap/keymap_test.go	Updates defaults to `ActionInfo` and adds deprecated alias tests for `commit_info`.
app/keymap/keymap.go	Renames action to `info`, adds deprecated alias rewrite + one-time warn during parse.
app/diff/jj.go	Switches commit text sanitization to exported `SanitizeCommitText` and updates doc references.
app/diff/hg_test.go	Removes obsolete `hgContextArg` test (now unified).
app/diff/hg.go	Uses `unifiedContextArg` and exported `SanitizeCommitText`.
app/diff/diff_test.go	Renames context-arg test to `unifiedContextArg`, updates sanitizer call name, and adds `CountChanges` tests.
app/diff/diff.go	Exports `SanitizeCommitText`, adds `CountChanges`, and unifies context arg helper for git/hg.
app/config.go	Adds `--description` / `--description-file` flags and exclusivity validation.
README.md	Documents info popup and review description flags; updates keybinding/action lists.
CLAUDE.md	Updates internal documentation for the unified info overlay and description/stats behavior.
.claude-plugin/skills/revdiff/references/usage.md	Documents review description usage for Claude plugin references.
.claude-plugin/skills/revdiff/references/install.md	Updates launcher override docs to include description flags.
.claude-plugin/skills/revdiff/references/config.md	Adds description flags and updates available actions list to `info`.
.claude-plugin/skills/revdiff/SKILL.md	Updates skill docs to pass description flags through launcher resolver.
.claude-plugin/plugin.json	Bumps plugin version.
.claude-plugin/marketplace.json	Bumps marketplace version.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+	cfg := m.review.cfg
+	switch {
+	case !cfg.Enabled:
+		return ""
+	case cfg.Stdin:
+		if cfg.StdinName != "" {
+			return "stdin: " + cfg.StdinName
+		}
+		return "stdin scratch buffer"
+	case cfg.AllFiles:
+		return "all tracked files"
+	case cfg.Staged:
+		return "staged changes"
+	case cfg.Ref == "":
+		return "working tree changes"
+	case strings.Contains(cfg.Ref, ".."):
+		return "ref range: " + cfg.Ref
+	default:
+		return "changes against " + cfg.Ref
+	}


+	info, err := os.Stat(path)
+	if err != nil || info.Size() > maxUntrackedBytes {
+		return nil, true
+	}
+	lines, err := diff.ReadFileAsAdded(path)
+	if err != nil {
+		return nil, true
+	}
+	return lines, prevPartial


+	fi, err := os.Stat(opts.DescriptionFile)
+	if err != nil {
+		return "", fmt.Errorf("stat --description-file: %w", err)
+	}
+	if !fi.Mode().IsRegular() {
+		return "", fmt.Errorf("--description-file %q must be a regular file", opts.DescriptionFile)
+	}
+	f, err := os.Open(opts.DescriptionFile)
+	if err != nil {
+		return "", fmt.Errorf("open --description-file: %w", err)
+	}
+	defer f.Close()
+
+	// LimitReader caps the read at maxDescriptionFileSize+1 so we can detect
+	// "exceeds cap" by checking len(data) > maxDescriptionFileSize after the
+	// read, regardless of what Stat reported.
+	data, err := io.ReadAll(io.LimitReader(f, maxDescriptionFileSize+1))
+	if err != nil {
+		return "", fmt.Errorf("read --description-file: %w", err)
+	}


+// sanitizeInfoText neutralizes terminal-unsafe content before rendering values
+// inside single-line info rows. Sanitization itself is delegated to
+// diff.SanitizeCommitText so all untrusted text in the overlay routes through
+// one strip path with identical semantics (full ANSI CSI sequences, ESC,
+// C0/DEL/C1 controls, VCS framing delimiters, invalid UTF-8). After stripping,
+// surviving LF and TAB are collapsed to a single space so multi-line values
+// (e.g. workdir paths copied with embedded newlines) render on one row;
+// repeated normal spaces inside paths or patterns are preserved as-is.
+// Trailing/leading whitespace is stripped at the boundary.
+func sanitizeInfoText(s string) string {
+	s = diff.SanitizeCommitText(s)
+	s = strings.Map(func(r rune) rune {
+		if r == '\n' || r == '\t' {
+			return ' '
+		}
+		return r
+	}, s)
+	return strings.TrimSpace(s)
+}


- Add CommitLogger optional capability to diff.Renderer surface so VCS backends (git/hg/jj) can enumerate commits in a ref range; consumers type-assert and gracefully fall back when unavailable. - Define CommitInfo with pre-sanitized Author/Subject/Body fields and a MaxCommits cap; add splitCommitDesc helper so all VCS parsers expose a uniform subject/body pair. - Export SanitizeCommitText (renamed from unexported helper) so the same ANSI/CSI/C0/DEL/C1/invalid-UTF-8 strip path serves commit metadata, description prose, and overlay detail rows. - Add CountChanges helper and FileEntryPaths to keep stats counting and path extraction in one place. - Introduce app/review package with ComputeStats / FileDiffer interface and safeWorkDirPath validation for untracked-file reads. Validation is best-effort lexical+symlink check against a non-racing filesystem; TOCTOU window between Stat and the later Open is documented.

…mit log User-facing info overlay reachable via the i key, replacing the prior commit-only popup with a richer review-info surface. CLI: - --description / --description-file populate a markdown description section inside the popup. File reads cap at 256 KB via io.LimitReader, reject non-regular targets (FIFOs, devices, dirs), and route through the shared SanitizeCommitText strip path. The two flags are mutually exclusive at parse time. Keymap: - Rename ActionCommitInfo to ActionInfo. Pre-rename keybinding files using "commit_info" still load: the parser accepts the deprecated alias, rewrites it to the canonical action, and emits a one-time WARN. UI: - New app/ui/overlay/info.go renders the unified popup: description prose (chroma-highlighted once at NewModel time), invocation scope rows, file/status histogram footer, and commit log section. - Commits and aggregate +/- line stats fetch in parallel at startup; stats reload is lazy (first popup open) and gated on ReviewInfoConfig.Enabled. Stale-result guards via load-seq tags drop any in-flight fetch whose seq no longer matches. - Reload (R) invalidates the review-stats cache so the popup never shows pre-reload numbers under post-reload entries. - Description and detail-row sanitizers wrap diff.SanitizeCommitText: the description preserves LF/TAB for paragraph structure; info rows collapse them to spaces for single-line display.

…ature - CLAUDE.md, README.md, docs/ARCHITECTURE.md: document the info overlay, the i keybinding, --description / --description-file flags, CommitLogger capability, and the safeWorkDirPath threat-model trade-off (best-effort, not TOCTOU-proof). - Bump Claude Code plugin metadata (marketplace.json, plugin.json) and pi package.json so installers see the new feature surface. - Update revdiff skill references (claude-plugin + codex) covering install, config, and usage so agent guidance lists the i key, description flags, and aggregate-stats footer. - site/index.html and site/docs.html: surface the popup in the feature grid and document the keybinding alongside other view toggles.

Apply umputun's review asks across the unified info popup feature: - Extract app/reviewinfo.go (package main) and app/ui/reviewinfo_test.go so reviewInfoFromOptions, resolveDescription, and the model-side review-stats tests live next to their implementation. - Convert single-call helpers to methods: (*Hg).translateRef, (*Hg).statusToFileStatus, (*Manager).injectBorderEdgeText, Model.reviewListFlag, (*infoOverlay).{splitLines,padPlain, truncateForDisplay,sanitizeInfoText}, and (*Keymap).dumpKeyName. - Replace ReviewInfoConfig.Enabled flag with *ReviewInfoConfig (nil = subsystem off); production constructor returns a non-nil pointer, focused tests pass nil. Drop dead commitsState.hint along with its production reads/clears and test writes. - Bundle 4+ param call shapes into option structs: reviewInfoInputs, borderEdgeText, detailRowLayout, StatsRequest, workDirRoots — eliminates the same-typed-pair swap risks. - Unexport NormalizeNewlines (now overlay-internal); inline the one-line highlightDescription accessor. - Track deprecated keymap aliases in a process-wide sync.Map so the commit_info → info WARN fires once per program, not per occurrence. - Inline review fixes: reviewHeaderText returns "standalone files" for no-VCS --only mode; readUntracked rejects non-regular files and flags binary placeholders as Partial; resolveDescription error wrappers include the path; sanitizeInfoText doc matches its 1:1 LF/TAB→space mapping. - Update CLAUDE.md gotchas to describe the *ReviewInfoConfig pattern, the readUntracked guards, and the once-per-program WARN.

umputun

all 17 items from the prior round addressed, tests / lint / race green.

re Copilot's 4 inline comments: each describes an issue this commit already fixed (Copilot anchored to a stale snapshot). Nothing actionable there.

LGTM.

melonamin requested a review from umputun as a code owner April 27, 2026 12:33

umputun requested a review from Copilot April 27, 2026 16:11

Copilot started reviewing on behalf of umputun April 27, 2026 16:12 View session

umputun requested changes Apr 27, 2026

View reviewed changes

Copilot AI reviewed Apr 27, 2026

View reviewed changes

umputun mentioned this pull request Apr 27, 2026

feat(annotations): --annotations flag preloads annotations from FormatOutput markdown #156

Merged

6 tasks

melonamin added 4 commits April 27, 2026 15:30

melonamin force-pushed the feat/review-info-overlay branch from ec09c55 to b5407f6 Compare April 27, 2026 20:28

umputun approved these changes Apr 27, 2026

View reviewed changes

umputun merged commit 8645569 into umputun:master Apr 27, 2026
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: review info popup (i) with --description and aggregate stats#155

feat: review info popup (i) with --description and aggregate stats#155
umputun merged 4 commits intoumputun:masterfrom
melonamin:feat/review-info-overlay

melonamin commented Apr 27, 2026 •

edited

Loading

Uh oh!

umputun left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

umputun left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

melonamin commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

umputun left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

umputun left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

melonamin commented Apr 27, 2026 •

edited

Loading