Say "rewritten" instead of highlighting the whole plan - #192
Conversation
The "changed since you last looked" highlights were making plan pages look messy in the two most common cases. Open a plan while an agent is still drafting it — or come back after a rewrite — and every section differs from your baseline, so every section lit up. The diff was right and the page said nothing. Plans::ChangedSections now reports that case as a rewrite instead of a list of keys: most of the document is new, measured against both the section count and the volume of text (either measure alone misreads a common shape — a swarm of one-line sections changing isn't a rewrite, and neither is one long section getting edited), and only for documents of four or more sections, since banding a short plan in full is a few inches of tint rather than noise. The page drops the highlights and carries one line above the content: "Rewritten since you last looked", linking to the history. Second, unrelated fix in the same feature: the controller tinted each top-level block separately, so one changed section rendered as a stack of rounded boxes with untinted gaps and a broken-up left bar. Adjacent changed blocks are now grouped into runs, and the inter-block margins inside a run become padding within the tint, so a run reads as one continuous band — rounded at the ends only, with mid-run headings keeping their breathing room inside the band instead of punching a gap through it. Verified against the real page in both states, before and after. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 58811eb364
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| total = new_sections.sum { |_key, body| body.length } | ||
| return true if total.zero? | ||
|
|
||
| changed.sum { |key| new_sections[key].length } > total * REWRITE_RATIO |
There was a problem hiding this comment.
Exclude unchanged bodies when headings are renamed
When a plan has at least four sections and more than half of their headings are renamed, changed contains the new slug keys, so this sum counts each section's entire body as changed even if those bodies are byte-for-byte identical. For example, renaming three headings in a four-section plan with substantial unchanged bodies satisfies both thresholds and tells the reader the plan was rewritten; calculate changed volume from the actual content differences or match renamed sections by body before classifying a rewrite.
Useful? React with 👍 / 👎.
The "changed since you last looked" highlights were making plan pages look messy, and there were two separate reasons.
Saturation
The diff was correct; the message was worthless. Open a plan while an agent is still drafting it (the presence channel records the visit), or come back after a rewrite, and every section differs from your baseline — so every section lit up. "Everything changed" is what a reader already assumes on a v5 plan they half-saw once.
Plans::ChangedSectionsnow returns a small result (keys+rewritten?) instead of a bare array, and reports a rewrite when most of the document is new. "Most" is measured two ways — more than half the sections and more than half the text — because either measure alone misreads a common shape: a swarm of one-line sections changing isn't a rewrite, and neither is one long section of ten getting edited. It only applies to documents of four or more sections; below that, banding the whole thing is a few inches of tint, which is legible and more useful than a sentence about it.In that case the page drops the highlights entirely and carries one line above the content: "Rewritten since you last looked. See what changed", linking to the plan's history.
The two thresholds are
REWRITE_RATIOandREWRITE_MIN_SECTIONSin the service, easy to move once we've seen this on real plans.The barcode
Independent bug in the same feature. The Stimulus controller added
.section-changedto each top-level block, and the CSS gave each one its own tinted, rounded, left-barred box — so a single changed section of six paragraphs rendered as six stripes with untinted gutters between them and a broken-up accent bar. It looked messy even when only one section had changed.Adjacent changed blocks are now grouped into runs (
--start/--endmarkers), and within a run the inter-block margins become padding inside the tint. A run reads as one continuous band with one unbroken bar, rounded at the ends only. Mid-run headings keep their breathing room as padding inside the band rather than a gap through it. The band CSS is scoped under.markdown-renderedso it outranks the per-element markdown margins.Wrapping the run in a container div would have been simpler, but
reference_preview_controller#renderSectionwalksheading.nextElementSiblingto build section previews, so the flat structure has to stay.Reviewer notes
ChangedSections.callchanged return type (array →Result). OnlyPlansController#showandshow.html.erbconsume it;NONEis the empty result for the "first visit / nothing new" paths.last_seen_at. Worth knowing while testing —PlanPresenceChannel#pingalso advances it while a plan page sits open, so a tab left on the plan will eat the highlight before you can reload.spec/services/plans/changed_sections_spec.rband the data attributes inspec/requests/changed_sections_highlight_spec.rb.🤖 Generated with Claude Code