Skip to content

feat(diff): highlight where the walkthrough points, dim what a rule can prove is mechanical - #22

Merged
fiddur merged 2 commits into
developfrom
attention
Aug 21, 2026
Merged

feat(diff): highlight where the walkthrough points, dim what a rule can prove is mechanical#22
fiddur merged 2 commits into
developfrom
attention

Conversation

@fiddur

@fiddur fiddur commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

Attention is the scarce thing in a review, and a diff spends it evenly. Two mechanisms, with the
decider deliberately different for each.

Highlighting is the walkthrough's job. A tour step already records the lines it is about, so the
hunks covering them are tinted. The AI only ever adds attention — it can never take any away.

Dimming is a rule's job, never a model's. Dimming asserts that something needs less attention,
which is the one claim a review tool should have to show its reasoning for. A hunk is dimmed when:

  • every line it touches is an import (ES, require, Python, Rust, #include, Ruby, Go/Java package lines)
  • its added and removed lines are the same lines with different whitespace
  • the file is generated (reuses the existing lockfile/generated detection)

It recedes to 45% and comes back on hover, stays selectable and commentable, and carries the reason
as a title, so a reader can always find out why instead of having to trust it. A dim you cannot
interrogate is the tool lying about the size of the change.

Indentation-sensitive files are never whitespace-dimmed.py, .yml, .yaml, .md,
Makefile, .hs and friends. There the indentation is the syntax, so a reindent can change which
key a value hangs off or which block a statement belongs to. There is a test asserting a YAML
reindent stays at full attention.

A deliberate conservatism: an added or removed blank line does not count as whitespace-only,
because it changes the file's shape. git diff -w agrees — it needs --ignore-blank-lines for that.

Whitespace hiding now defaults on and is remembered, since it is the formatter's business
(prettier, oxfmt). The toolbar appends · whitespace hidden to the description whenever it is on:
a filtered diff renders fewer lines than GitHub does, and after the +14524-vs-+367 episode that
must never be silent.

Threading: attentionClass and attentionTitle reach every row group of a hunk. The hunks render
inside a <table>, so there is nowhere to hang a wrapper element — the class goes on each <tbody>
the hunk emits, in both the unified and split renderers.

152 UI tests pass.

Two things left for the follow-up: scrollToLine, so the stepper lands on a step's lines rather
than the top of its file — there are no per-line DOM anchors yet, so that is a real change rather
than a one-liner; and move detection, which is the bigger prize and feeds both channels
(verbatim moves dim, moved-and-edited highlight, with a nested diff of the move).

Stacked on #21.

🤖 Generated with Claude Code

https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs

fiddur and others added 2 commits August 21, 2026 14:12
…an prove is mechanical

Attention is the scarce thing in a review, and a diff spends it evenly. Two changes, with
the decider deliberately different for each.

Highlighting comes from the walkthrough: a step already records the lines it is about, so
the hunks covering them are tinted. Only ever added attention, never removed.

Dimming is decided by rules, never by a model, because dimming asserts that something needs
less attention - the one claim a review tool should have to show its reasoning for. A hunk is
dimmed when it is entirely imports, when its added and removed lines are the same lines with
different whitespace, or when the file is generated. It recedes to 45% and returns on hover,
stays selectable and commentable, and carries the reason as a title so a reader can always
find out why rather than having to trust it.

Indentation-sensitive files are never whitespace-dimmed. In YAML, Python, Makefiles and
markdown the indentation is the syntax, so a reindent there can change behaviour.

Whitespace hiding now defaults on and is remembered, since that is the formatter's business -
and the toolbar says "whitespace hidden" whenever it is, because a filtered diff renders
fewer lines than the forge does and that must not be silent.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs
The attention props were declared on the interfaces and used in the JSX but never
destructured, so every diff threw ReferenceError: attentionClass is not defined at render.
A scripted edit had looked for a two-line destructure where the file has a one-line one and
silently matched nothing.

Neither `vite build` nor tsc caught it, and nothing else stood between a render-time
ReferenceError and the browser. The UI package now runs component tests under jsdom:
the React Router plugin is dropped for test runs because its runtime preamble has no
framework to come from, and JSX is transformed by esbuild instead.

The tests assert what the feature is for rather than that it renders: an imports-only hunk
is dimmed and says why, real work is left alone, whitespace in a file where indentation is
syntax is not dimmed, and a hunk the walkthrough points at is highlighted in preference to
being dimmed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs
@fiddur

fiddur commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed a fix and the test harness that should have caught it.

attentionClass and attentionTitle were declared on the props interfaces and used in the JSX but never destructured — a scripted edit looked for a two-line destructure where both files have a one-line one, and silently matched nothing. Every diff threw ReferenceError: attentionClass is not defined at render. An earlier variant of the same mistake put hunkAttention inside a useEffect closure.

Neither vite build (no typecheck) nor tsc stopped it, and there was nothing between a render-time ReferenceError and the browser. The UI package now runs component tests under jsdom — the React Router plugin is dropped for test runs, since its runtime preamble has no framework to come from, and JSX goes through esbuild instead.

Both new test files were written before the fix and reproduced the exact error. They assert the behaviour rather than that it renders: an imports-only hunk is dimmed and says why, real work is untouched, whitespace in a file where indentation is syntax is not dimmed, and a hunk the walkthrough points at is highlighted in preference to being dimmed.

164 UI tests pass.

@fiddur
fiddur changed the base branch from dev-skill-sync-opt-in to develop August 21, 2026 13:36
@fiddur
fiddur marked this pull request as ready for review August 21, 2026 13:36
@fiddur
fiddur merged commit e203592 into develop Aug 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant