Summary
In recent roborev output, multiple findings within ## Review Findings are concatenated into one continuous block. There are no blank lines (or any visible separator) between consecutive • Severity: entries, which makes reviews hard to scan.
Expected behavior
Each finding should be separated from the next by a blank line, e.g.:
• Severity: Medium
• Location: pkg/server/handler.go:88
• Problem: ...
• Fix: ...
• Severity: Low
• Location: pkg/server/handler.go:142
• Problem: ...
• Fix: ...
Actual behavior
Findings run together with no blank line between them, so the four-line block of one finding flows directly into the • Severity: line of the next. Wrapped lines compound the issue — at a glance it looks like one giant finding.
Reproduction
- Run
roborev against any branch that produces ≥2 findings (a small repo with two trivial issues is enough).
- Inspect the rendered review in the terminal.
- Observe that the
• Severity: lines appear back-to-back with the previous finding's • Fix: line, with no intervening blank line.
Example (fabricated, illustrative)
Review #204 widgetd (claude-code)
/home/alice/proj/widgetd 7c1ab09 on alice/cache-ttl
Verdict: Fail
## Review Findings
• Severity: Medium
• Location: internal/cache/store.go:62-78 (Set method)
• Problem: Set writes the entry and then calls time.AfterFunc to schedule eviction, but the timer
handle is discarded. Repeatedly setting the same key leaks one timer per call until the original
TTL fires, which under hot-key workloads can pin tens of thousands of timers in the runtime heap.
• Fix: Track the timer in the cache entry struct and call timer.Stop() before reassigning, or
switch to a single janitor goroutine that scans for expired entries on a tick.
• Severity: Low
• Location: internal/cache/store_test.go:34-51
• Problem: TestSetOverwrite uses time.Sleep(10*time.Millisecond) to wait for eviction, which is
flaky on loaded CI runners and silently masks regressions where eviction never fires.
• Fix: Inject a fake clock (e.g. clockwork.NewFakeClock) and advance it explicitly, asserting
the post-eviction state deterministically.
• Severity: Low
• Location: cmd/widgetd/main.go:117
• Problem: The --cache-ttl flag is parsed as int seconds, diverging from every other duration
flag in the binary (--read-timeout, --write-timeout) which accept a time.Duration string. Users
will reasonably write --cache-ttl=30s and get a parse error.
• Fix: Change the flag type to time.Duration for consistency.
The above renders as one wall of text in the current build; each • Severity: should start on its own paragraph.
Environment
roborev version: v0.53.0- Shell: zsh
- TERM: xterm-256color (COLORTERM=truecolor)
- OS: Linux 6.19.14-300.fc44.x86_64 (Fedora 44)
Impact
Human reviewers miss findings or misattribute lines to the wrong finding.
Summary
In recent
roborevoutput, multiple findings within## Review Findingsare concatenated into one continuous block. There are no blank lines (or any visible separator) between consecutive• Severity:entries, which makes reviews hard to scan.Expected behavior
Each finding should be separated from the next by a blank line, e.g.:
Actual behavior
Findings run together with no blank line between them, so the four-line block of one finding flows directly into the
• Severity:line of the next. Wrapped lines compound the issue — at a glance it looks like one giant finding.Reproduction
roborevagainst any branch that produces ≥2 findings (a small repo with two trivial issues is enough).• Severity:lines appear back-to-back with the previous finding's• Fix:line, with no intervening blank line.Example (fabricated, illustrative)
The above renders as one wall of text in the current build; each
• Severity:should start on its own paragraph.Environment
roborevversion: v0.53.0Impact
Human reviewers miss findings or misattribute lines to the wrong finding.