Repository navigation
fix(suggest): render a pure-deletion suggestion as a deletion, not an empty box - #201
Merged
Merged
Conversation
… empty box
`prereview suggest` has always accepted a suggestion whose `proposed` is empty
(suggest_cmd.go validates only `file` and `from_line`), but the card rendered the
after-pane unconditionally — so a deletion painted an EMPTY GREEN BOX, which
reads as a bug rather than as "this text goes away".
The before-pane was already conditional, for the mirror case of a pure insertion.
This makes the after-pane conditional too and adds the missing affordance:
- `.sg-diff` gains `is-deletion`, which strikes the before-pane through
- a `.sg-del-note` caption ("delete — accepting removes L4") stands in for the
absent green box, so the card still says what accepting will do
This is load-bearing rather than cosmetic for the plan-simplify workflow, where
a pass over a document is mostly *cuts* — deletions are the common case there,
not an edge case.
Rendering for every non-deletion suggestion is byte-identical: the render goldens
pass untouched, and only the signature golden moves (an intentional content
change, regenerated with -update-sig).
Tests: e2e/e2e_deletion_suggestion_test.go asserts no `.sg-new` is emitted at all
for a deletion (the actual regression), the `is-deletion` marker, a non-empty
caption, that accept/reject still work, and that an ordinary replacement card is
untouched. Verified failing against the pre-fix template before landing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014g9DCfyYjDZsMQ4xocgCMj
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
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.
prereview suggesthas always accepted a suggestion whoseproposedis empty —suggest_cmd.go:139-142validates onlyfileandfrom_line. But the card rendered the after-pane unconditionally, so a pure deletion painted an empty green box, which reads as a bug rather than as "this text goes away".The before-pane was already conditional, for the mirror case of a pure insertion. This makes the after-pane conditional too, and adds the affordance that was missing:
.sg-diffgainsis-deletion, which strikes the before-pane through.sg-del-notecaption (delete — accepting removes L4) stands in for the absent green box, so the card still says what accepting will doWhy this is load-bearing rather than cosmetic
Deletions are the common case for a document-simplify workflow — a pass over a plan is mostly cuts. A tool that proposes cuts through
prereview suggestis unusable while every cut renders as an empty box.Rendering safety
Every non-deletion suggestion renders byte-identically:
TestTemplateRenderGolden) pass untouchedtestdata/prereview.tmpl.sig.goldenmoves — an intentional content change, regenerated with-update-sigper the repo's template guard conventionTests
e2e/e2e_deletion_suggestion_test.go(chromedp) asserts:.sg-newelement is emitted at all for a deletion — this is the actual regression; previously one existed and was empty.sg-diff.is-deletionmarker is presentVerified failing against the pre-fix template before landing — reverting
partials.tmpltoHEADproduces exactly the two intended failures, so the test goes red if this regresses.Full unit suite green; full browser e2e suite green (573s). Confirmed by eye in a running prereview, not only by reading the template.
🤖 Generated with Claude Code
https://claude.ai/code/session_014g9DCfyYjDZsMQ4xocgCMj