Skip to content

Say "rewritten" instead of highlighting the whole plan - #192

Open
HamptonMakes wants to merge 1 commit into
mainfrom
hampton/what-changed-highlights-plan-692aac
Open

Say "rewritten" instead of highlighting the whole plan#192
HamptonMakes wants to merge 1 commit into
mainfrom
hampton/what-changed-highlights-plan-692aac

Conversation

@HamptonMakes

Copy link
Copy Markdown
Collaborator

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::ChangedSections now 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_RATIO and REWRITE_MIN_SECTIONS in 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-changed to 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 / --end markers), 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-rendered so it outranks the per-element markdown margins.

Wrapping the run in a container div would have been simpler, but reference_preview_controller#renderSection walks heading.nextElementSibling to build section previews, so the flat structure has to stay.

Reviewer notes

  • ChangedSections.call changed return type (array → Result). Only PlansController#show and show.html.erb consume it; NONE is the empty result for the "first visit / nothing new" paths.
  • Highlights remain strictly once-only and per-viewer: the same request advances last_seen_at. Worth knowing while testing — PlanPresenceChannel#ping also advances it while a plan page sits open, so a tab left on the plan will eat the highlight before you can reload.
  • 1666 specs pass; RuboCop clean. New coverage for the rewrite threshold in spec/services/plans/changed_sections_spec.rb and the data attributes in spec/requests/changed_sections_highlight_spec.rb.
  • Verified visually against the real page in both states — a full A/B on identical content, with and without the change — plus a DOM geometry check confirming every adjacent pair in a run measures a 0.00px gap, including paragraph→heading and paragraph→code block.

🤖 Generated with Claude Code

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>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

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