feat: review-checks combined — rendering + prompt + anchoring#87
Merged
Conversation
Previously, multiple review checks were rendered as space-separated inline spans inside a single paragraph — fine for one check, hard to parse once there are two or more. Each check now gets its own line with a brand-claret bullet so the reader can take them one at a time. Single-check and reviewFocus-fallback cases keep the existing inline run-in pattern, which reads better for a lone sentence. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Expands the prompt guidance for "What to check" items so the model produces fewer generic platitudes and more specific, grounded, verifiable checks. Adds explicit criteria (names a specific risk in THIS diff, phrased as a directive/question, short, grounded, verifiable) and an equally explicit anti-pattern list (no vague prompts, no narrative restatements, no lint nits, no filler to hit the count, no invented anchors). Updates both the single-phase SYSTEM_PROMPT and the two-phase WRITER_SYSTEM so the guidance applies regardless of generation path. Also hardens the anchoring rules — prefer the most specific line inside a hunk over the top of the function, and prefer 2 great checks over 4 mediocre ones. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Follow-up to the bullet rendering. When the model emits a single reviewCheck whose text runs long or contains its own packed bullet list, the old inline run-in rendering produced a wall of text after the "What to check." label. - Single check > 180 chars OR containing markdown-list markers now promotes to a block render, with the text going through the Markdown component so embedded code spans, bullets, and emphasis render properly. - reviewFocus fallback uses the same promote-to-block rule, so a model-authored "- foo\n- bar" in reviewFocus actually renders as a list instead of collapsing to prose. - Short single checks still use the inline "What to check. Sentence." pattern — best reading for a genuinely single-sentence item. Refactored the callout body into a `renderReviewChecks` helper so the branching logic is readable instead of nested ternaries. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fixes the P0 where "What to check" anchors silently failed in every mode except unified + editable. Also hardens the edge cases. Renderer parity (P0) - Shared helper `lineAnchorAttrs(filePath, info, side?)` emits the anchor attributes consistently. Side-aware: split-LEFT cells carry `data-base-line` only; split-RIGHT cells carry `data-line-number` only; unified rows carry both when both are available. - Rewrote `DiffHunk.tsx` (the non-interactive unified path) to use the same parseShikiLines + per-line JSX as the other two renderers, so every line wrapper emits data-file-path + data-line-number. Was using raw innerHTML with zero anchors. - `SplitDiffHunk.tsx`: each cell now carries side-appropriate anchor attributes on the code cell wrapper. Contract fix for removals (P1) - Click handler tries `data-line-number="N"` first (new-file, matches the AI's "startLine is the new-file line number" contract), and falls back to `data-base-line="N"` so a check that anchors at a removed line still resolves. Off-view fallback (P1) - `buildAnchorableSet(hunks)` parses `@@ -b,bc +h,hc @@` headers to compute every line number resolvable inside this slide's diffs, for both sides. SlideView pre-filters `isClickable` with it — no more dead links on hallucinated or out-of-slide anchors. - If a click still misses (defensive), an aria-live="polite" "Line N in <path> isn't in this slide's visible diff" notice appears below the callout and clears after 3s. Highlight robustness (P2) - `.check-highlight` adds `isolation: isolate`, explicit z-index on the ::after, and a positioned children rule so the flash layers correctly even if the target line is inside another stacking context. `will-change: opacity` keeps the fade composited. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The model sometimes crams three or four distinct checks into one reviewCheck.text as a single prose paragraph with em-dashes between topics. The previous pass promoted such checks to a Markdown block, which still rendered as one long paragraph — Markdown can't invent bullets from prose. splitIntoProseChecks() now tries to split such paragraphs on sentence boundaries: period/?/! followed by whitespace then a capital letter or backtick (the next sentence starts with an identifier). Only used as a bullet list when the split yields 2+ segments and each is a reasonable bullet length (20–500 chars). Otherwise falls back to the Markdown block render so nothing gets worse. Applied in both the single-check-promote and reviewFocus-fallback paths. New prompt guidance (#85) will still make the model emit discrete checks; this is a render-time safety net for the reviews that were already generated with the old prompt and for cases where the model slips. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Keeps buildAnchorableSet from the anchoring work and the full renderReviewChecks/looksLikePackedList/splitIntoProseChecks helpers from the rendering work. Threads `anchorable` into renderReviewChecks so the P1 anchor pre-filter applies to every render branch, not just the inline one.
This was referenced Apr 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Combines #84, #85, #86 into a single branch for end-to-end testing. All three touch the "What to check" surface and are tied together.
What's inside
From #85 — prompt guidance (`lib/agent.ts`)
Rewrites the reviewChecks / reviewFocus guidance in both `SYSTEM_PROMPT` (single-phase) and `WRITER_SYSTEM` (parallel writer) with explicit positive criteria (specific risk in THIS diff, directive/question form, grounded in code, one sentence, verifiable) and an anti-pattern list (vague prompts, narrative restatements, lint nits, filler, fabricated anchors). Tightens anchoring rules — prefer the specific line over the top of the function.
From #86 — anchoring parity (`components/DiffHunk.tsx`, `InteractiveDiffHunk.tsx`, `SplitDiffHunk.tsx`, `shared-diff-utils.tsx`)
Previously, only `InteractiveDiffHunk` emitted `data-file-path`/`data-line-number`, so clicking a hint in split layout or in read-only unified mode silently did nothing. Now all three renderers emit consistent, side-aware anchor attributes via `lineAnchorAttrs`. Click handler tries `data-line-number` first (new-file, matches the prompt's contract) then falls back to `data-base-line` (covers the rare case of an anchor at a removed line). `.check-highlight` tightened with `isolation` / `z-index` so the flash layers correctly in every stacking context.
From #84 — rendering (`components/SlideView.tsx`, `src/globals.css`)
Multi-item `reviewChecks` render as a custom bulleted list. Single long or "packed" checks promote to a block through Markdown. A sentence-level fallback (`splitIntoProseChecks`) catches the common case where the model crams multiple distinct checks into one prose blob — splits on `.`/`?`/`!` followed by a capital letter or backtick, returning multiple bullets when each segment is a reasonable length (20–500 chars). `reviewFocus` fallback goes through the same logic, so `- foo\n- bar` renders as a real list.
Combined additions
Files
Test plan (end-to-end)
Closes #84, #85, #86 once merged.
🤖 Generated with Claude Code