Update pointer — mark what changed on reload

[than]

What

When the watched file changes, the viewer now points at what moved instead of leaving you to hunt for it.

• Persistent ▸ marker (bullets): a changed bullet line has glamour's • swapped for a bright ▸ (same width, no shift). Persists until the next change.
• Subtle one-shot flash: on a change, every changed line gets a gentle background lightening (~500ms), then settles to the ▸. No strobe. Disable with --no-flash (the ▸ still works).

How

• diff.go — a line-level LCS over the rendered output (ANSI-stripped for comparison), so an inserted line marks only itself instead of cascading onto everything below it.
• Resize-proof: the model keeps a prevBaseline and re-diffs render(prevBaseline) vs render(raw) at the current width on every render, so markers survive a rewrap.
• SGR-aware flash background: applyLineBg re-applies the bg after every \x1b[0m reset so the tint spans the whole line without losing content colors; the bullet swap runs first so its reset is re-tinted too.
• Initial load marks nothing; scroll position preserved across reloads and flash on/off.

Scope / safety

• Viewer guarantees untouched: scroll preservation, pane-width cap, the existing amber status-bar flash, renderMarkdown/watcher.go.
• No new dependencies. Colors are two tunable consts in style.go.

Tests

52+ passing. Covers: LCS (identical / modified / insertion-no-cascade / deletion / restyle-ignored), bullet swap + non-bullet-no-marker, flash bg present/absent, marker persistence after flash-off, --no-flash, initial-load-unmarked, and the file-deleted-mid-flash edge (below).

Review

Built subagent-driven (4 tasks, reviewed each). Final whole-branch review (opus) found one Important bug — recompose() could resurrect stale content over the "waiting for file"/error view if the file was deleted mid-flash — fixed (guard recompose() + reset cached state on reload's failure paths) with a regression test. Two Minors documented as out-of-scope: resize double-renders the baseline; overlapping <500ms flashes clear early (cosmetic).

Spec: docs/superpowers/specs/2026-07-31-update-pointer-design.md
Plan: docs/superpowers/plans/2026-07-31-update-pointer.md

🤖 Generated with Claude Code

[claude]

Reviewed the whole branch against the hard requirements. The core is sound — the watcher is untouched, scroll preservation holds on every SetContent path, the pane-width cap is respected, and there's no data race. The LCS approach is the right call and the resize re-diff is a genuinely good idea. Five things below, one of which is a real rendering bug.

What checks out

• Watcher — watcher.go is unchanged. Parent-directory watch, 100ms debounce, rename/delete/recreate handling, dir-vanished restart: all intact.
• Scroll — reload and both recompose call sites capture YOffset before SetContent and restore after, so viewport clamping still handles shrinking content. The new flash-toggle path doesn't change line count, so it can't drift.
• Width — applyLineBg pads to m.renderWidth() (width-2) and only when visibleWidth(ln) < width, so it can never exceed the pane. ▸ is the same cell width as • , so the swap can't push a line over either. No new wrap risk.
• Concurrency — the watcher goroutine still only calls p.Send. m.changed and m.renderedLines are replaced wholesale, never mutated in place, so the value-copy semantics of model stay safe. All state transitions happen in Update.
• LCS — the backtrack tie-break (lcs[i+1][j] >= lcs[i][j+1]) matches the fill direction, so an insertion really does mark only itself instead of cascading. Comparing on ANSI-stripped text is the right choice.
• Mid-flash delete — the error paths reset renderedLines/changed/lineFlash and recompose is guarded on all three. That fix looks correct.

a. The bullet swap drops the foreground for the rest of the line

composeMarked does strings.Replace(ln, "• ", updatedMark, 1), and updatedMark is a lipgloss render, so it terminates with \x1b[0m. The comment right above reasons about the background — "applyLineBg re-establishes the background after the reset the swap introduces" — but nothing re-establishes the foreground. Any span glamour had open across the bullet prefix (Document sets colorText #D0D0D0) is killed by that reset, so the body text after ▸ falls back to the terminal's default foreground. The result is a brightness shift on exactly the lines you're trying to make look deliberate. applyLineBg's ReplaceAll(reset, reset+bg) doesn't help here — it only patches the background back in.

Fix: don't emit a bare reset. Build the marker next to applyLineBg and end it by restoring the body color:

r, g, b := hexToRGB(colorUpdated)
tr, tg, tb := hexToRGB(colorText)
updatedMark := fmt.Sprintf("\x1b[1;38;2;%d;%d;%dm▸ \x1b[22;38;2;%d;%d;%dm", r, g, b, tr, tg, tb)

b. updatedMark is the one new color that can degrade to a 256-palette index

renderMarkdown deliberately forces termenv.TrueColor, and applyLineBg emits raw \x1b[48;2;… — both correct. But composeMarked builds the marker through lipgloss.NewStyle(), which uses lipgloss's global renderer and its own profile detection. On a terminal reporting xterm-256color without COLORTERM, #5FE3A1 gets quantized to a palette index, which is precisely the remap bug the README calls out — and this one lands inside the content pane, not the status bar. The snippet in (a) fixes this too, since it bypasses lipgloss entirely.

c. Resize now costs two glamour renders plus an O(N²) allocation

reload(true) runs on every tea.WindowSizeMsg, and those arrive repeatedly during an interactive drag. Each one now renders raw, renders prevBaseline again at the new width, then builds a (len(old)+1) × (len(new)+1) int table. For a typical SIDECAR.md that's nothing. But the table is unbounded in file size — at ~5000 rendered lines it's a 200MB single allocation, per render, and sidecar will happily open any .md you point it at.

Two cheap fixes, either one is enough:

• Cache the rendered baseline alongside renderedLines, keyed on the width it was rendered at. Resize then re-renders it only when the width actually changed, and repeated events at the same width are free.
• Trim the common prefix and suffix before building the table in changedLines. Real edits touch a handful of lines, so this collapses the table to near-nothing in the common case and bounds the worst case in practice.

d. A changed blank line becomes a solid tinted bar

tidy collapses blank lines to "". If the LCS marks one changed — which happens whenever the number of blanks between blocks shifts — applyLineBg pads it to full width, producing a solid block of colorFlashLineBg across an otherwise empty row. That's louder than the "subtle, no strobe" design intends. Guard on content:

if changed[i] && visibleWidth(ln) > 0

e. Unknown flags silently become the path

The new arg loop in main.go sends anything that isn't --no-flash to path, so sidecar --no-flash --help tries to open a file literally named --help (the -h/--version/--static switch only inspects os.Args[1]). Multiple paths silently take the last one, too. Rejecting unrecognized --prefixed args with a usage error would be one if and keeps the "no swallowed errors" line clean.

---

None of these are structural — (a) and (b) are a few lines in composeMarked, (c) is a cache field or a prefix/suffix trim, (d) is one condition. The diff logic, the resize-proofing, and the state hygiene around the error paths are all good work.

[than]

Thanks — addressed the four actionable findings (commit 3b1ba9e):

a + b. Marker color — Fixed together. The ▸ is now emitted as raw truecolor ANSI (\x1b[1;38;2;…m▸ \x1b[22;38;2;<colorText>m) instead of via lipgloss, so it can't be quantized to a 256-palette index on a non-COLORTERM terminal, and it restores the body foreground after the marker. (Dropped the now-unused lipgloss import from diff.go.)

c. LCS memory — Bounded via common prefix/suffix trim in changedLines: the DP table is now proportional to the edited region, not the whole file. Indices are offset by the prefix length; all existing diff tests stay green plus a new trimmed-context case. (Left the resize double-render as-is — acceptable at the tool's scale.)

d. Blank-line bar — Flash background now guarded on visibleWidth(ln) > 0, so a changed blank row no longer becomes a solid tinted bar.

e. Unknown flags — main.go now rejects any unrecognized --prefixed arg with sidecar: unknown flag "…" + usage and exit 2, instead of treating it as a filename.

Full suite green (55), go vet + gofmt clean.

[claude]

Reviewed the whole branch. This is solid work — the hard guarantees all survive, and diffing with an LCS over rendered lines (rather than index-wise) is the right call. go vet is clean and go test ./... passes. Notes below, one of which I would fix before merging.

Confirmed non-regressions

• Pane width. applyLineBg pads only when visibleWidth(ln) < width, and width is m.renderWidth() = pane−2, so a tinted line lands at exactly pane−2 and an already-long line is never widened. The bullet swap is width-neutral: U+2022 and U+25B8 are both East_Asian_Width=Ambiguous, so go-runewidth reports 1 for each (and 2 for each in an ambiguous-wide terminal). No wrap risk.
• Scroll. Both reload and recompose save YOffset and restore it after SetContent; the viewport still owns the clamp. Preserved across reloads, resizes, and flash on/off.
• Watcher. watcher.go is untouched — still parent-directory-based, still debounced. No new goroutines, and nothing mutates m.changed / m.renderedLines after construction (changedLines returns a fresh map each call), so the value-copy semantics of Update are safe here. No new races, no dropped events.
• Colors. colorUpdated and colorFlashLineBg are hex, emitted as 38;2; / 48;2; truecolor. No palette indexes.
• Bare URLs. composeMarked only rewrites a leading bullet and wraps the line in SGR — URL text is never split or padded mid-line, so linkification still works.
• Compact spacing. tidy still owns blank-line collapsing and trailing-space trimming; the flash padding is transient, on changed lines only, and stays inside the pane.

Worth fixing: the LCS table is unbounded in file size

changedLines allocates (len(om)+1) × (len(nm)+1) ints. Prefix/suffix trimming helps for one localized edit, but it does nothing for the case sidecar exists for: moving an item from In progress to Done changes a line near the top and near the bottom, so neither trim fires and om/nm are the whole document.

That is L² × 8 bytes plus L+1 slice allocations, per render:

rendered lines	table
300 (the REVIEW.md fixture)	~0.7 MB
2,000	~32 MB
5,000	~200 MB
10,000	~800 MB

(arithmetic, not a measurement — but the shape is the point). And since the model re-diffs on every render, a terminal drag-resize replays this per WindowSizeMsg.

Two-line guard that degrades to "mark nothing" instead of to a stall:

const maxDiffCells = 4 << 20 // ~32 MB table; past this, skip the markers
if len(om)*len(nm) > maxDiffCells {
    return changed // empty
}

int32 for the table would halve it too, but the cap is the part that actually matters.

Minor

a. A transient read failure silently swallows the next update's markers. The error path in reload sets m.raw = "" but leaves m.prevBaseline alone; the next successful read then does m.prevBaseline = m.raw → "", so changedMap is skipped and that reload shows the status-bar flash with no marker and no tint. Realistic trigger is the 1s stat fallback catching an unlink-then-create write mid-flight. Narrow, and it self-heals on the following change — but if you want it tight, keep the last-known-good content rather than letting "" propagate.

b. Double compose per file event. reload(false) composes and calls SetContent, then Update sets m.lineFlash = true and recompose() composes and SetContents the same document again — the first result is discarded. Harmless at fixture size, but it doubles the per-event string work and compounds with the baseline re-render on the same path.

c. The marker pins the item text to colorText. updatedMark ends with ESC[22;38;2;<colorText>m, so a marked bullet whose item span carried a different color or bold loses it until glamour's next SGR. Invisible for a plain bullet (same D0D0D0); visible for a bullet that is entirely a link. Low priority, but re-emitting the SGR that preceded the bullet would be more faithful than hardcoding.

d. Unrelated deletion in main.go. The comment explaining why there is no mouse capture — "keeps the terminal's native text selection and clickable links working" — went with it. That rationale guards a deliberate decision README also leans on, and nothing in this PR touches mouse handling. Worth restoring.

e. The flag-parsing rewrite is an improvement (unknown flags now error instead of being taken as a path), though it is scope the PR body does not mention.

Not blocking, your call

docs/superpowers/ adds 848 lines of generated plan/spec to the repo. Fine if you want them tracked — just noting it is roughly 65% of the diff.

---

The two acknowledged minors (resize double-renders the baseline; overlapping sub-500ms flashes clear early) both read as genuinely cosmetic to me — I agree with deferring them. Fix the LCS bound and this is good to merge.

[than]

Addressed the blocker + the doc regression (commit db5f9c8):

LCS bound — changedLines now caps the table at ~32 MB of cells (maxDiffCells = 4 << 20) after the prefix/suffix trim; past that it returns an empty set (no markers, render still happens) instead of allocating hundreds of MB per render. This covers the move-item case where neither trim fires. New TestChangedLinesCapDegradesToEmpty.

(d) mouse-capture comment — restored on the tea.NewProgram call; that was an accidental drop in the --no-flash rewrite, thanks for catching it.

Deferring the remaining minors as genuinely cosmetic/self-healing, per your read: transient-read baseline loss (self-heals on next change), double-compose per event, and the marker's colorText restore (glamour re-colors the next segment immediately, so it's effectively a no-op).

56 tests green, vet + gofmt clean.

[claude]

Reviewed the whole branch. This is solid work — the hard guarantees survive:

• watcher.go is untouched; the parent-directory watch, debounce, and dir-vanished restart are unchanged.
• Scroll preservation is intact on both paths — reload and recompose each do offset := m.vp.YOffset; SetContent; SetYOffset(offset), and composeMarked never changes the line count, so cached changed indices stay aligned with the viewport.
• Pane-width cap holds. applyLineBg pads to m.renderWidth() (width-2) and only when visibleWidth(ln) < width, so it can never widen a line past the pane. The width <= 0 case can't panic either (v < width is false when width is non-positive).
• No new cross-goroutine state; everything lives in the model and mutates inside Update.
• Colors are hex consts, tidy/compact spacing untouched, and blank lines are excluded from the flash (visibleWidth(ln) > 0), so the tint doesn't turn separators into blocks.

A few real things:

a. The marker never lands on a wrapped continuation line

isBulletLine requires the ANSI-stripped line to start with "• " after trimming indentation. In a narrow pane, glamour wraps a long to-do item onto continuation lines that start with spaces + text. If the edit falls on the tail of a wrapped item — appending (done), changing the last word — changedLines correctly marks that index, isBulletLine returns false, and the item gets no persistent marker at all: just the 500ms flash, then nothing. Narrow panes are the whole use case here, so this will be a common shape of edit, not an edge case.

Fix: when a changed line isn't a bullet line, walk backwards through lines to the nearest preceding bullet line and mark that one instead (bounded by a blank line so you don't jump blocks).

b. Pure deletions mark nothing

TestChangedLinesDeletionNoSpuriousMark locks in that removing a line marks nothing. Defensible — nothing new to point at — but for this tool the common event is "agent moved an item from in-progress to done", and if the destination line happens to be a wrap-tail (see a) the user gets a status-bar flash and no pointer anywhere. Worth confirming that's the intent.

c. maxDiffCells is generous for something on the resize path

4 << 20 cells of int is ~32 MB allocated in one shot, and changedLines runs on every WindowSizeMsg — a drag-resize delivers a stream of those, each triggering two full glamour renders plus this table. A 2000-line rewrite (a git checkout swapping the file) sits right at the cap. Either drop the cap an order of magnitude (1 << 18 still covers any realistic queue), or make the rows []int32 to quarter the footprint.

d. berr is swallowed and silently drops every marker

if base, berr := renderMarkdown(m.prevBaseline, m.renderWidth()); berr == nil {
    changedMap = changedLines(strings.Split(base, "\n"), lines)
}

If the baseline render fails, changedMap stays nil and all markers vanish with no signal. Degrading to "no markers" is the right behaviour, but the failure is invisible. Worth at least a comment saying it's a deliberate degrade, since every other error path in reload sets m.loadErr.

e. Every change composes the content twice

In reload, m.lineFlash is still the pre-change value when composeMarked runs; Update then sets lineFlash = true and calls recompose(), which composes and SetContents the same lines again. It works, but passing the intended flash state into reload would drop one full compose + SetContent per reload on the hot path.

f. Arg parsing changed shape slightly

The rewritten loop takes the last positional, so sidecar a.md b.md now silently opens b.md (was a.md). And since the subcommand switch only inspects os.Args[1], sidecar --no-flash --static f.md exits 2 on "unknown flag --static". Both are trivial, but the flag/subcommand split is now order-sensitive in a way the help text doesn't hint at.

---

Item (a) is the one I'd fix before merging — it's the feature not firing in the case the tool is built for. The rest are minors.

[than]

Fixed the blocker + two more (commit dc8e734):

a. Wrapped-item edits now mark the item — composeMarked resolves the owning bullet for a changed line: a changed bullet marks itself; a changed non-bullet line (a wrapped continuation of a long item) walks back to its nearest preceding bullet, bounded by a blank line, and marks that. So an edit on the tail of a wrapped to-do now gets the persistent ▸ on the item, not just the flash. New tests cover the wrapped-continuation case and the prose-after-blank (no-marker) case.

c. LCS cap — lowered maxDiffCells to 1 << 18 (~2 MB), still ample for any real queue, gentler on the resize path.

d. Baseline degrade — added a comment marking the baseline-render-fails → no-markers path as deliberate.

Deferring the rest as you flagged them (minors): deletion-marks-nothing is intended and the wrapped-item fix covers the practical move case; double-compose-per-event is cosmetic; the arg-order edges are trivial/nonsensical invocations.

58 tests green, vet + gofmt clean.

[claude]

Review — solid; a few things worth fixing, nothing blocking.

Verified locally: go vet ./... clean, go test -race ./... passes.

The hard guarantees hold

• Watcher untouched. watcher.go is byte-identical — parent-dir watch, debounce, rename/delete/recreate all intact. No new goroutines; p.Send from the debounce timer is still the only cross-goroutine path, and everything new lives on model state mutated inside Update. -race clean.
• Scroll preservation. Both reload and the new recompose do save-YOffset → SetContent → SetYOffset, and composeMarked emits exactly len(lines) lines, so a marker or flash toggle cannot shift or re-clamp the offset.
• Width cap. composeMarked is called with m.renderWidth() (width-2), and applyLineBg only pads up to that. ▸ and • both measure 1 cell under PrintableRuneWidth, so the swap is width-neutral.
• Rendering style. tidy untouched; both new consts are hex; the flash pad is transient and does not survive lineFlashOffMsg.
• The recompose() guard (m.fileMissing || m.loadErr != nil), plus resetting renderedLines/changed/lineFlash on every reload failure path, is the right fix for delete-mid-flash, and TestUpdatePointerFileMissingDuringFlash pins it.

The LCS itself is correct: greedy match-on-equal is optimal, the prefix/suffix trim is right, and the forward backtrack marks only insertions into nm. TestChangedLinesInsertionNoCascade is the test that mattered and it is there.

Worth fixing

a. Nothing asserts the composed output stays within the pane. TestNeverWiderThanWidth (render_test.go:42) covers renderMarkdown output only. This PR introduces the one thing that appends cells to a rendered line — applyLineBg's right-pad — which is exactly the shape of change that reintroduces the wrap / fake-double-spacing bug this tool exists to avoid. It is correct today; it is just unguarded. Add the analogue over composeMarked(lines, allChanged, true, w) at a few widths.

b. The diff cap silently switches the feature off at ~512 lines. maxDiffCells = 1 << 18 is checked against len(om)*len(nm) — the un-trimmed middle. Prefix/suffix trimming only helps when edits are clustered; edit near the top and near the bottom of the same file and the middle is effectively the whole document. So a ~520-line SIDECAR.md with two small edits at opposite ends gets no markers at all, with nothing telling the user why. That is a reachable state, not a pathological one — your own template instructs the agent to "let later ones accumulate as a log." Cheapest fix: make the table [][]int32 (half the bytes per cell) and raise the cap to 1<<20 — ~4 MB, ~1000 lines a side, and the DP is still only ~1M integer ops.

c. composeMarked's owning-bullet walk is O(n²). diff.go:109 rescans backwards per changed line and recomputes visibleWidth(lines[j]) for the same j over and over. On a long blank-line-free run (fenced code block, table) where most lines changed, that is ~n²/2 rune-width scans — and it runs on every resize event, on top of the two glamour renders. One backward pass precomputing owner[i] (reset at blanks, set at bullets) makes it O(n) and reads more clearly than the nested loop.

Nits

d. if m.prevBaseline != "" (ui.go:214) conflates "no baseline yet" with "the baseline was an empty file." Clear the file, then add a line: the new line is not marked. A hasBaseline bool says what you mean.

e. When only a wrapped continuation line changed, the owning bullet gets the ▸ but not the flash tint (changed[i] is false for it) — so the one line carrying the marker is the one line that does not light up. Folding the resolved swap indices into the flash set would be more consistent.

f. applyLineBg re-applies the background only after the exact literal ESC[0m. termenv emits standalone resets today so this is fine, but a combined ESC[0;…m would silently drop the tint mid-line. Worth stating the assumption in the comment.

g. help in main.go documents --no-flash, but README does not mention the ▸ marker, the flash, or the flag — and README's "What it does" / "Rendering style" sections are where a user would look for a change this visible.

Of these, (b) is the only one I would want addressed before merge; the rest are follow-ups.

[than]

Addressed the blocker and folded in the rest to converge (commit bd6b8cd):

b (blocking). Diff cap — table is now [][]int32 (half the bytes) and the cap is raised to 1 << 20 (~1000 lines a side, ~4 MB). A ~520-line file with edits at both ends now diffs fine instead of silently dropping all markers.

a. Width guard — added TestComposeMarkedNeverWiderThanWidth: composes with the flash on (the only path that appends cells) at several widths and asserts nothing exceeds pane−2.

c. O(n) owning-bullet — replaced the per-changed-line backward walk with a single owner[] pre-pass (resets at blanks); same behavior, no more O(n²) on blank-free runs or the resize path.

d. hasBaseline — added an explicit hasBaseline flag so "no baseline yet" no longer aliases "the baseline was an empty file"; clearing a file then adding a line now marks the line. Reset on the failure paths too.

e. Marker line flashes — folded the resolved owning-bullet indices into the flash set, so the line carrying the ▸ lights up with the rest.

f. Documented applyLineBg's standalone-ESC[0m assumption. g. README now covers the ▸ marker, the flash, and --no-flash.

Still deferring the double-compose-per-event and arg-order edges as you flagged (cosmetic/trivial). Full suite green, vet + gofmt clean.

[claude]

Review — solid, ship it

I read diff.go, the ui.go changes, and both test files against the hard requirements. This is careful work: the LCS is correct, the tests cover the failure modes that actually matter, and the guarantees the tool exists to protect are intact. go test -race, go vet, and gofmt -l are all clean.

Hard requirements — verified

• Watching survives rename-swaps/deletes/recreates. watcher.go is untouched; still a parent-directory watch with the debounce. No regression.
• Scroll preservation and clamping. reload and recompose both do save-YOffset → SetContent → SetYOffset, and viewport.SetYOffset clamps, so shrinking content lands correctly. TestScrollPreservedAcrossReload covers it.
• Never wider than the pane. applyLineBg pads to m.renderWidth() (pane − 2) and only when visibleWidth(ln) < width, so an over-wide line (a long bare URL) is tinted but not extended. TestComposeMarkedNeverWiderThanWidth locks it in.
• Concurrency. No new goroutines and no new shared state — the diff runs entirely inside Update on the Bubble Tea loop. -race passes.
• Rendering style. Colors are hex consts. tidy still owns compact spacing. The ▸ swap only touches the leading "• ", so a bare URL on its own line is byte-identical and stays linkifiable.

The hasBaseline flag earning its own comment (empty-file baseline vs. no baseline) is exactly the distinction that would otherwise become a bug report, and the recompose guard against resurrecting stale content over the waiting view is the right fix with the right regression test.

Notes

a. r renders the markers but never flashes, and silently consumes the baseline.

In Update, the "r" case is m.reload(true); return m, nil — it discards the bool. fileEventMsg and tickMsg both use it to set m.flash / m.lineFlash. So if you edit the file and hit r inside the 100 ms debounce window, you get the ▸ markers but no flash at all — and when the debounced fileEventMsg lands, contentChanged is now false, so it does not flash either. The status-bar half of this predates the PR, but the new line flash inherits it. Fix is to mirror the fileEventMsg branch.

b. Resize re-renders the baseline through glamour and re-runs the full LCS on every WindowSizeMsg.

Re-diffing on resize is the right call — it is what makes the markers survive a rewrap. But a drag-resize fires one message per column, and each one costs two renderMarkdown calls plus an O(n·m) table. On the intended small SIDECAR.md this is noise; on something like testdata/REVIEW.md it is ~90k cells per column of drag.

Cheap guard: if m.changed is empty, the two sources rendered identically at the old width, and since the diff compares ANSI-stripped text of the same two sources, they will render identically at any width too. So the baseline render can be skipped entirely when !contentChanged && len(m.changed) == 0. Optional — call it if you ever see jank.

c. hexToRGB drops the Sscanf error.

Not a real path today (all callers pass consts), but the README tells users to edit those consts and rebuild, and a typo silently yields #000000 — a black ▸ on a dark background reads as "the feature broke." A one-line fallback to colorText, or just a comment saying the input is const-only, would close it.

d. Deletion-only edits give no line-level signal. TestChangedLinesDeletionNoSpuriousMark asserts this deliberately, and avoiding the cascade is clearly right. Worth confirming it is the intent that removing a finished item flashes the status bar amber but leaves the body completely still.

e. Nit: the flash tint stops 2 columns shy of the pane edge (padded to renderWidth, not m.width), while the status bar spans the full width. Purely cosmetic, and passing m.width there would need care not to disturb the width guarantee.

None of these block. (a) is the only one I would fix before merging.

[than]

Fixed the one pre-merge item + the cheap defensive one (commit 03b6032):

a. r force-reload now flashes — the "r" case mirrors the fileEventMsg branch: on a real change it sets the status + line flash, recomposes, and schedules the flash-off timers (an r on unchanged content still doesn't flash). So editing then hitting r inside the debounce window no longer shows markers with no flash. New TestUpdatePointerRKeyFlashesOnChange.

c. hexToRGB — now falls back to a visible mid-grey when Sscanf doesn't parse three components, so a typo'd color const gives a visible (if wrong) marker instead of a silent black one.

Deferring the rest as you flagged (non-blocking follow-ups): the resize baseline-render skip when changed is empty, deletion-only giving no line signal (intended), and the tint stopping 2 cols shy of the pane edge (cosmetic).

61 tests green, -race/vet/gofmt clean.

[claude]

Review

The shape of this is good: the LCS-over-rendered-output approach is the right call (an insert marks only itself), the diff is derived state recomputed from prevBaseline on every render so it survives a rewrap, watcher.go is genuinely untouched, no new goroutines or shared state — p.Send still hands everything to the Bubble Tea loop, so no new races — scroll save/restore is correctly duplicated into recompose(), and composeMarked caps padding at renderWidth() with a dedicated test. Colors are hex consts. No new deps. go build, go vet, go test ./... all clean.

One real bug, plus some smaller notes.

---

Important — a forced re-render before the first content change marks the whole document

prevBaseline is only ever assigned inside if contentChanged, and on the very first load the old content is the zero value:

first WindowSizeMsg -> reload(true)
  raw = A, m.raw = ""      -> contentChanged = true
  m.prevBaseline = m.raw   -> ""
  m.hasBaseline == false   -> changedMap = nil   (correct: initial load marks nothing)
  m.hasBaseline = true

Now the user resizes the pane, or presses r, before the agent has written anything:

WindowSizeMsg -> reload(true)
  raw == m.raw             -> contentChanged = false, prevBaseline stays ""
  m.hasBaseline == true    -> renderMarkdown("") -> "" -> []string{""}
  changedLines([""], lines) -> marks ~every line

ui.go:213-215 and ui.go:231-238. Result: every bullet in the file gets a persistent bright ▸ even though nothing changed. It does not flash (reload returns false, so lineFlash is never set), so it is silent — the markers just appear and stay until the next real edit. Same path via the r key. Given the tool sits in a split pane that gets resized, and r is a documented key, this is easy to hit on a freshly opened file.

Fix — seed the baseline to the current content on the first successful render, right after m.raw = raw:

m.raw = raw

// First successful render: the baseline is the content itself, so a forced
// re-render (resize, r) before any change diffs against itself and marks nothing.
if !m.hasBaseline {
    m.prevBaseline = raw
}

This also does the right thing after an error/missing-file recovery, where hasBaseline is reset to false but prevBaseline keeps a stale value.

Test gap that let this through: no test sends a second WindowSizeMsg and then inspects markers. TestStatusBarWidth (ui_test.go:337) actually does run the buggy path — it just asserts status-bar width on content with no bullets. TestUpdatePointerInitialLoadUnmarked stops after the first load, and TestUpdatePointerRKeyFlashesOnChange always changes the file first. Worth adding: render -> resize -> assert no ▸, and render -> r -> assert no ▸.

---

Minor

a. Flash tint stops 2 columns short of the pane edge. applyLineBg pads to width, which callers pass as renderWidth() = m.width - 2, while the viewport pads its content to m.width. The last two columns of a flashed line stay untinted. Harmless for the width guarantee (it is the safe direction), but it is a visible ragged right edge on the tint band. If you want it flush, composeMarked needs the pane width for padding and the render width for everything else.

b. sidecar --no-flash --static x.md fails. Subcommand dispatch only looks at os.Args[1], so --no-flash in position 1 falls through to the viewer loop, which then rejects --static as an unknown flag (main.go:53-62). --static --no-flash is worse: runStatic takes args[0] as the path and tries to read a file named --no-flash. Both are odd-but-defensible given the documented order; a one-line note in help or skipping --no-flash in runStatic would close it.

c. Multiple positional args silently last-wins. sidecar a.md b.md now opens b.md (previously a.md). Since the loop already errors on unknown flags, erroring on a second path would be consistent.

d. Non-bullet changes leave no persistent trace. A changed heading, table row, or code line gets the 500 ms flash and nothing after; under --no-flash it gets no indication at all. That reads like a deliberate scope choice and the README describes it accurately — flagging only so it is a decision on record, not an oversight.

The documented-and-accepted items (applyLineBg depending on bare ESC[0m resets, maxDiffCells degrading to no markers, overlapping flashes clearing early, resize double-rendering the baseline) all fail safe and are commented at the call site. No objection to any of them.

Fix the baseline seeding and this is good to merge.

[than]

Fixed the baseline-seeding regression (commit a6de483):

reload now seeds prevBaseline = raw on the first successful render (and after an error/missing-file recovery, where hasBaseline was reset) — right after m.raw = raw, gated on !m.hasBaseline. So a forced re-render (resize or r) before any real change diffs the content against itself and marks nothing, instead of diffing an empty baseline against the whole document. A real edit still sets prevBaseline to the prior content and marks the delta as before.

Added two regression tests that failed before the fix and pass after: render→resize→assert no ▸, and render→r→assert no ▸.

Deferring the minors you accepted as non-blocking (tint stops 2 cols shy, the --no-flash/--static arg-order edges, multiple-positional last-wins, non-bullet changes leaving no persistent trace — a deliberate scope choice the README documents).

63 tests green, vet + gofmt clean.

[claude]

Review — solid, with one behavioral bug worth fixing

I verified the hard requirements hold:

• Watcher untouched — watcher.go is byte-identical; parent-dir watch, debounce, and the re-add-on-dir-vanish loop are intact.
• Scroll preservation — reload and recompose both do SetContent → SetYOffset(offset), same shape as before; the viewport still clamps on shrink.
• Width cap — applyLineBg pads to visibleWidth(ln) < width only, and composeMarked is called with renderWidth() (= width-2), so a flashed line is exactly 2 columns short of the pane. TestComposeMarkedNeverWiderThanWidth pins it. The ▸/• swap is width-neutral (both U+25B8 and U+2022 are ambiguous-width → 1 cell), so no wrap regression.
• Colors — both new consts are hex; applyLineBg/updatedMark emit 38;2;/48;2; truecolor SGR only. No palette indexes.
• Bare URLs — applyLineBg only inserts SGR after existing \x1b[0m resets, so URL text stays contiguous on its line.
• LCS — I hand-checked insert / delete / modify against the walk. Insertion marks only itself, pure deletion marks nothing, modify marks the new line. The prefix/suffix trim and the maxDiffCells degrade-to-empty guard are both right.
• Concurrency — no new goroutines or shared state; everything new lives in Update on the value-receiver model copy. No new race surface.

The one real bug: overlapping flashes cancel each other

lineFlashOff() schedules a bare tea.Tick with no generation token, and lineFlashOffMsg unconditionally does m.lineFlash = false. Two changes inside 500 ms means the first timer clears the second flash almost immediately — e.g. a save at t=0 and t=400ms leaves the second flash visible for ~100ms instead of 500ms.

The PR notes this as a cosmetic minor, but this is the primary use case, not an edge: an agent writing SIDECAR.md several times a turn hits it constantly, and the flash is the feature. Fix is small — add a counter to the model:

flashGen int  // bump per flash; a stale lineFlashOffMsg is ignored

carry it through lineFlashOff(gen int) into lineFlashOffMsg{gen int}, and in the handler if msg.gen != m.flashGen { return m, nil }. The status-bar flashOff has the same shape (pre-existing) and can reuse the same token.

Minor

• Double compose per change. Every change path does reload() (which calls composeMarked + SetContent with lineFlash still false) and then immediately m.lineFlash = true; m.recompose() — a second full composeMarked + SetContent. Correct, just wasted work on every reload. The ordering is defensible since reload can return false, but a flash bool param on reload would collapse it to one pass.
• Baseline re-render on resize. renderMarkdown(m.prevBaseline, …) runs on every reload, including each WindowSizeMsg during a drag — two glamour renders plus an LCS per column. Acknowledged as out of scope; noting that a (prevBaseline, width) → lines one-entry cache would make it free, since consecutive resizes share the baseline.
• Comment nit, diff.go:92: "the bullet is swapped first so applyLineBg re-establishes the background after the reset the swap introduces" — updatedMark deliberately ends in \x1b[22;38;2;…m, not \x1b[0m, so it introduces no reset. Ordering still matters (the swap must see the un-tinted line), but the stated reason is wrong, and the 22;-not-0m choice is exactly the invariant a future edit could break.

Not flagging, but noting

applyLineBg re-adds trailing padding that tidy() strips, so selecting a flashed line during the 500ms window copies trailing spaces. That's inherent to a full-width tint and it can't cause wrap at width-2; just be aware it's a deliberate exception to the no-padding rule.

The two swallowed errors (berr on the baseline render, hexToRGB's grey fallback) are both documented deliberate degrades on non-critical paths — fine. Error/missing-file state resets in reload correctly guard recompose against resurrecting stale content.

Ship it after the flash-generation fix.

[than]

Fixed the flash-cancellation bug (commit a032a2c):

Added a flashGen generation token. Each change path (fileEventMsg, the tickMsg changed-branch, and r) bumps m.flashGen and threads that generation into both flashOff(gen) and lineFlashOff(gen); the off-handlers ignore a message whose gen doesn't match the current one. So a save at t=0 and t=400ms no longer has the first timer clear the second flash — each flash lives its full duration from the latest change. New TestUpdatePointerOverlappingFlashNotCancelled covers it (stale off-msg doesn't cancel; current-gen one does). Updated the three existing tests that constructed bare flashOffMsg{}/lineFlashOffMsg{}.

Remaining items all previously accepted as non-blocking follow-ups (double-compose per change, baseline re-render on resize, the diff.go:92 comment wording, trailing-pad-on-selection during the flash window). Happy to take the comment-wording nit now if you'd like.

Full suite green, vet + gofmt clean.

[claude]

Review — solid, ship it with a few small notes

I read diff.go, the ui.go reload/recompose changes, style.go, main.go, and ran go vet + go test ./... (clean).

The hard requirements hold. Checked each one specifically:

• Watcher untouched. watcher.go has zero diff — the parent-directory watch, the 100 ms debounce, and the re-watch on directory removal all survive. No regression on rename-swap / delete / recreate.
• Scroll preserved and clamped. Both reload and the new recompose do the same save/SetContent/SetYOffset dance, and the existing TestScrollPreservedAcrossReload / TestOffsetClampedWhenContentShrinks still pass — including through the new flash-on and flash-off recompose paths.
• Never wider than the pane. applyLineBg pads to visibleWidth(ln) only when it is under width, and width is renderWidth() (pane − 2). ▸ and • are the same two cells, so the bullet swap is width-neutral. TestComposeMarkedNeverWiderThanWidth asserts this over real glamour output at three widths — good test to have added.
• Concurrency. No new cross-goroutine state; the watcher still only calls p.Send. m.changed and m.renderedLines are aliased across model value-copies but are always replaced wholesale, never mutated in place, so there is nothing to race on.
• Style. tidy unchanged, colors are hex truecolor (colorUpdated, colorFlashLineBg follow the existing convention), bare-URL lines are only prefixed/suffixed with SGR — the URL text stays contiguous, so Ghostty still linkifies.

The LCS design is the right call. Diffing the rendered output ANSI-stripped is what makes insertions mark only themselves and makes markers survive a rewrap, and the prefix/suffix trim plus the maxDiffCells bail keep the table bounded. The hasBaseline vs empty-prevBaseline distinction is subtle and the comment earns its place.

Notes

a. Task-list items may silently lose the ▸. isBulletLine matches only glamour's • prefix, but style.go defines Task markers (✓ / □ ). If glamour emits a task item without the • block prefix, a checkbox-style queue gets no marker — and composeMarked still sets swap[i], so strings.Replace is a silent no-op while the line still flashes. Worth a test with - [ ] foo either to confirm it works or to widen isBulletLine. The shipped SIDECAR.md template and testdata/REVIEW.md use plain -, so this is latent, not visible today.

b. A resize racing a pending change swallows the flash. WindowSizeMsg calls m.reload(true) and discards the return value. If the file changed on disk just before the resize, that reload consumes the change — markers and content land correctly, but no flashGen bump and no flash — and the fileEventMsg arriving 100 ms later sees raw == m.raw and returns false. Markers still show (they persist), so this is cosmetic, and the same hole existed for the amber status flash before this PR. Cheap fix if you want it: capture the bool from that reload(true) and run the same flash branch.

c. Baseline re-render is avoidable on the common path. reload calls renderMarkdown twice every time. On a plain file change (no resize), the previous m.renderedLines is render(prevBaseline) at the current width — cache it alongside the width it was rendered at and only re-render the baseline when renderWidth() differs. Keeps the resize-proofing, halves the work on the hot path. You flagged this as out-of-scope, which is fair; noting the concrete shape in case it is a one-liner.

d. --no-flash does not compose with the subcommand dispatch. sidecar --static --no-flash passes --no-flash through as the path to runStatic, and sidecar --no-flash init opens a file literally named init. Both are edge cases and the help text lists the forms separately, so low priority — just noting since --no-flash is the new flag.

e. Flash tint stops two columns short. composeMarked pads to renderWidth() (pane − 2), so the tinted band leaves a two-column notch at the right edge of the pane. Reads deliberate given the width cap, and widening it is the one change I would not make casually — flagging only so it is a decision rather than an accident.

f. applyLineBg reset handling. The comment already owns the ESC[0m-only assumption. Note that ansiRE itself matches ESC[m (empty params), so if termenv ever emits the short form the tint drops mid-line. Adding ESC[m to the ReplaceAll is a two-character hardening if you want the belt.

None of these block. Test coverage is genuinely good — the insertion-no-cascade, restyle-ignored, file-deleted-mid-flash, empty-file-baseline, and stale-generation cases are exactly the ones I would have gone looking for.