Soften Planner change marks and move the changes chip into the margin #365
- MA
## Why
When the Planner or an MCP client edited the document, its change marks were heavy and patchy:
- Each block got a hard-edged rectangle that stopped flush at the text. - Earlier sections of a multi-step draft faded before later ones landed. - Inserted blocks pushed the page down in a single frame. - MCP edits read "Written by unknown". - The `chopin` cursor label covered the heading above the caret. - The changes chip used Unicode `↓`/`▸` with a tiny target, sat on top of the prose, and its list rows were fully tinted.
## What changed
**Wash** - Prose blocks (paragraphs, headings, lists, quotes) get a soft, opaque-over-page tint with `--radius-sm` corners and a 0.375rem bleed drawn as an outline, so it takes no layout space. - Consecutive changed prose blocks read as one region. The later block fills the margin above it with a `border-image` outset, which paints outside the box without taking space. - Blocks that draw their own frame (code, tables, cards) are only tinted, so they don't get a double border.
**Fade** - Marks fade in when first seen. When they expire, the store keeps every attribute on and adds `data-plan-fade`. A keyframe then fades the registered `--plan-wash` and `--plan-hole` colours, and the attributes come off on `animationend`, or after a 2s fallback. - The mark keeps its full shape throughout, and there is no per-element style read. - With reduced motion, marks are removed at once.
**One turn fades together** - Each new batch from the agent restarts the clock on marks already showing (`trail.renew()`). - Unseen marks, the cap of 50, placing marks only after the Yjs update, and non-durability are unchanged.
**Agent insertions expand smoothly** - `plan:update` gains an optional, descriptive `agent: true`. The protocol declares it first. - The server sets it only when the Planner or MCP wrote the update: the Planner's document-room `publish`, the decision-prose, heading and refine-placement jobs, and the MCP `rewrite`. - Only those updates expand newly inserted top-level blocks, and only blocks below the local caret's block (or blocks on screen when the editor is unfocused). - It is skipped for rewrites in place, more than 8 blocks, the initial load, and reduced motion. - A human collaborator's Enter is never animated. - Overflow is clipped through the WAAPI keyframes, with no inline style write.
**Author** - `ChangeAttribution` gains the verified `user` login, from `caller.user.login` in `mcp/hosted.ts`. - The list reads "Written by @login via <client>" with the verified login first, or "Written by @login" when the client gave no name. The tooltip gives the client version and revisions.
**Planner cursor label** - The agent's caret is marked `data-plan-agent` in `collaboration.tsx`. - Its label sits beside the caret at the end of the block the Planner just wrote, instead of over the line above. `labels.ts` is untouched, so this does not overlap #356.
**Changes chip** - Uses `ArrowUpIcon` (rotated) and `ChevronIcon` from `@chopin/icons`, shows the count, and has an accessible label and tooltip ("2 changes below"). - Targets are 24px, with `--shadow-raised`. - On wide panes it sits in the leading margin. Below 44rem (container query) it becomes a centred floating pill. - List rows are untinted, with a reserved unread-dot slot.
## Screenshots
**Wash.** Before / after. Consecutive blocks form one region:

**Insertion filmstrip.** Before: a one-frame jump.

After: the blocks open over `--acc-expand`. A rAF probe shows the list below moving 218→408px over about 300ms.

**Change list** (desktop, after):

**Planner cursor label.** Before (audit shot) / after:

**Phone** (390×844). Before / after, with the centred chip:

## Verification
40 focused change-mark and hosted tests, `bun run types`, `bun run ci`, and `bun run build` pass on the restacked branch. Initial JavaScript is 251,414 B raw / 79,337 B gzip. GitHub's browser suite will run on the final published head.
The design hashes were renewed after reviewing the integrated source. The focused browser checks against a fake GitHub server covered mark fading, Planner versus human insertions, and the author label.
octocatmentionedlast month on GitHub
octocatsubscribedlast month on GitHub
MaggieAppletonreferenced this in Share one menu surface recipe across pickers and the language menu #377last month on GitHub
- MA
**Review:** Changes needed
I checked this in the browser on a fake-GitHub server (8876) with two users (octocat and ana), real MCP `update_document` calls, desktop 1440×900 and phone 390×844, and compared it with main (8795). The attribution change, reduced motion, performance and the chip/list restyle are fine. Three behaviour problems need fixing first.
### Must fix
1. **The bleed snaps off at the start of the fade-out.** `packages/editor/src/changes.ts:102-118` (`settle`): the `to` keyframe is read *after* the attribute is removed, so `outline-style` is already `none`. Only `outlineColor` is animated, and it heads toward `currentColor`. I sampled computed values during the fade and the outline colour goes from the wash to near-black (`oklch(0.159 …)`). It is invisible only because the style is already `none`. I took screenshots every ~40ms: in one frame the rounded, padded wash becomes a square box flush with the text, and only then does the background fade. Suggested fix: keep `outlineStyle`, `outlineWidth` and `borderRadius` the same in both keyframes, and animate `outlineColor` and `backgroundColor` to `transparent` rather than to the computed after-values. While you are there, batch the work: read every `from`, remove every attribute, then animate. As written, each removal forces two style recalcs. Because `renew()` now makes a whole turn's marks expire together, that can be up to 50 back-to-back recalcs.
2. **Expansion moves the reader's caret line, and it now animates under them while they type.** `packages/editor/src/changes-observer.tsx:37-61` and `changes.ts:444`. I measured the caret rect on every animation frame while ana typed in a paragraph mid-viewport: - MCP inserts two paragraphs above, on screen. Main: the caret jumps 599→674px in one frame. This PR: the same 75px, but the caret glides over about 290ms. - A *human* collaborator presses Enter and types above. Main: one 38px jump. This PR: a 12px jump, then about 250ms of gliding.
Neither version holds the line, because scroll anchoring does not hold it in this layout. The PR makes the movement longer and also applies it to ordinary human typing, because the listener gates only on `COLLABORATION_TAG`. Every collaborator's Enter now fades in from opacity 0 and pushes the reader's text down. Suggested fix: (a) never expand a block that sits above the local selection's block (or above the scroll anchor when the editor is unfocused). Insert those at full height as main does. (b) Expand only agent or MCP edits. The client cannot tell them apart today: `plan:update` has no author, and `plan:changes` arrives after the Yjs update. Either add an agent flag to server-authored `plan:update` frames, or, at minimum, apply (a) and skip single-block inserts that sit next to a remote human caret. When an insert is off screen above the viewport, anchoring works: one 75px or 300px scrollTop adjustment and a steady caret, with no animation. That path is fine.
3. **On phone the chip covers the start of each line.** `styles.css:2270` (`inset-inline-start: 0.5rem`). At 390px the chip measures x=20–80 and prose starts at x=28, so the ↓ and count sit over the first word of the line beneath ("2. Replay…"). At 1440 with Chat open the leading gutter is just enough, and the chevron touches the text. The leading margin exists only on wide panes. Proposed fix: add a container query on the document pane. When the inline gutter is narrower than the chip (roughly `@container (width < 40rem)`), go back to the centred overlay pill (`inset-inline-start: 50%; translate: -50% 0`). A floating "jump to changes" pill reads as intentional, while a margin chip that hides the first letters of lines does not. Optionally, add `scroll-padding-block-end` to the scroller so the last line can scroll clear of it.
### Should fix
4. **Consecutive blocks do not "meet as one region".** `styles.css:2171`. With a 0.375rem bleed on each side, any block gap above 0.75rem leaves a white seam. Heading→list, list→table, table→code and stacked paragraphs all show visible stripes, so eight inserted paragraphs read as eight pills. Either size the bleed from the block gap (half the paragraph margin), or drop the claim and accept separate pills. Also, the code block's own border plus the outline ring reads as a double frame rather than a wash. Consider leaving the outline off blocks that draw their own edge (`pre`/code card, table), and only tint them.
5. **The verified identity is hidden while the self-reported one is shown.** `changes-chip.tsx:53`. When a client sends any name, the visible text is the client's own name, which is spoofable (a client can call itself "@ana" or "Chopin"). The verified `@login` appears only in the tooltip. Show both visibly, e.g. "Written by @ana via Codex".
6. **The PR body undersells the design-contract change.** `changes.ts:455` adds a new inline-style write (`element.style.overflow = "clip"`, then a restore). The body says the change "adds no style or class logic". Please mention this to whoever renews the `dynamic-editor.json` hash. A Lexical style rewrite on that element during the 300ms could also be overwritten by the restore.
### Checked and fine
- **Attribution:** `user` comes from `caller.user.login`, which is resolved by `auth.admission.user(oauthToken)` from the verified bearer. It is never client-supplied. No token reaches the frame. `plan:changes` is broadcast only (`plan/service.ts:1729`) and never persisted. The test asserts it. - **AGENTS.md "Change marks and presence":** marks still follow the Yjs update. Added and moved marks stay on live blocks. Removed marks are still box-shadow gaps with no tombstone. The cursor and marks stay separate. The 50-mark cap and the unseen/epoch rules are untouched. `renew()` restarts only `showing` marks. - **Reduced motion:** with `reducedMotion: reduce`, `document.getAnimations()` peaked at 0 during an insert. The CSS keyframes are covered by the existing `.plan *` rule, and the JS paths check `matchMedia`. - **Performance, 8 on-screen inserts:** 181 frames in 1.5s, worst frame 12ms, no long-animation-frame entries. Over 8 blocks it switches to no animation, as intended. - **Wash by block type:** the heading, list, quote (its bar is kept) and table all look right. - **Planner label beside the caret:** I forced `data-plan-agent` on the other user's caret at phone width and it reads well. One untested edge case: when the last line runs to the right edge, the label has nowhere to go and is clipped by the scroller. Consider flipping it back above in that case. - **Chip and list restyle:** the icons, 24px targets, `aria-label`/tooltip "1 change below", untinted rows with a reserved dot slot, and "Written by @octocat" for an unnamed client all look good.
CI: `container` pass, `e2e` pass. `format, lint, types, tests` fails only on the two reviewed design-contract hashes (`changes.ts:414`, `collaboration.tsx:261`). Under the audit rules that is acceptable.
anamentionedlast month on GitHub
anasubscribedlast month on GitHub
octocatmentionedlast month on GitHub
octocatsubscribedlast month on GitHub
- MA
Addressed in 5846a6c7:
1. **Fade pop.** Expired marks now keep all their attributes and get `data-plan-fade`. A keyframe fades the registered `--plan-wash` and `--plan-hole` colours, and the attributes are removed on `animationend`, with a 2s fallback. The outline stays `solid 6px` and keeps its radius through the fade; I sampled it at about 100ms steps. There are no per-element style reads, so nothing needs batching. Reduced motion removes marks at once. 2. **Expansion.** - `plan:update` gains a descriptive `agent?: true`, declared in the protocol first. The server sets it only for Planner and MCP-authored updates: the document-room `publish`, the decision-prose, heading and refine-placement jobs, and the MCP `rewrite`. The hosted test asserts it. - The client expands only those updates, and only blocks below the focused caret's block. With the editor unfocused, it expands only blocks on screen. - Measured on the reader: a human's Enter plus typing gives 0 animations, an MCP insert above the caret gives 0, and an MCP insert below gives 1. - The overflow clip is now inside the WAAPI keyframes, so there is no inline style write. 3. **Phone chip.** Under `@container (max-width: 40rem)` the chip is a centred floating pill. 4. **One region.** Prose marks are opaque over the page. A later consecutive prose mark fills the margin above it with a `border-image` outset of 1.5em, which takes no layout. Code, tables and cards get the tint only, with no outline. 5. **Label.** It now reads "Written by @login via <client>", or "Written by @login" when the client gave no name. 6. **PR body.** Corrected. It now lists the `data-plan-fade` attribute, the WAAPI keyframes, and the third pinned file (`plan-editor.tsx`) with new hashes.
I haven't handled the clipped Planner label when the last line runs to the right edge.
anamentionedlast month on GitHub
anasubscribedlast month on GitHub
octocatunsubscribedlast month on GitHub
- MA
**Review:** Looks good
I re-checked 5846a6c7 in a fresh worktree on 8876 with two users and real MCP `update_document` calls. Every earlier must-fix holds up in the browser.
- **Fade:** in a frame sequence (25 screenshots across the expiry) the wash keeps its rounded, bled shape and fades evenly. It no longer snaps to a square box. The rest colour, `--color-page`, matches the `.plan-document` surface (`oklch(1 0 0)`), so nothing pops at the end. - **Expansion.** I measured on the reader, counting height animations every frame: - MCP insert above the caret: **0** animations, a single one-frame shift as on main. - MCP insert below the caret: **1** animation; the caret did not move. - Human Enter plus typing, above or below: **0** animations. - Unfocused reader, on-screen MCP insert: **1** animation, as designed. - 8 on-screen inserts: worst frame 10ms. - Reduced motion: 0 animations. - **`agent` flag:** the server sets it only in `publish`, `publishStaged` (Planner jobs) and `rewrite` (MCP). The browser relay in `plan/service.ts:~1473` builds a fresh `plan:update` frame from `epoch`, `update` and `seq` alone, so a browser can never get `agent: true` relayed to its peers. The `hosted.test.ts` assertion covers the MCP path. - **One region:** heading and list now join into one wash. Table and code are tinted only, with no double frame. The quote keeps its bar. - **Phone chip:** at 390px it is a centred floating pill (x=165–225) and no longer covers the start of lines. - **Label:** "Written by @login via <client>" keeps the verified identity first. The e2e test was updated to match.
Minor, non-blocking: 1. The default desktop layout (1440, Chat open) gives a document pane of about 665px, just above the 40rem breakpoint. With the list open, the chip sits on the "P" of the line beside it. Consider about `44rem`. 2. `changes.ts` `FADED`: the fallback `setTimeout` is not cancelled. If a block is marked again and expires again within 2s, the first timer can cut the second fade short. Store the timer ID and clear it in `#faded`. 3. As you noted, the Planner label still clips when the last line reaches the right edge. That can be a follow-up.
CI: `container` pass, `e2e` pass. `format, lint, types, tests` fails only on the reviewed design-contract hashes (`changes.ts:394`, `collaboration.tsx:261`, `plan-editor.tsx:347/358`). That is acceptable under the audit rules.
- MA
Follow-ups in ae95a312: the chip's centred-pill breakpoint is now `@container (max-width: 44rem)`, and each element's 2s fade fallback timer is stored with its fade and cleared when the fade finishes or the block is marked again, so a second fade is no longer cut short. The `changes.ts` hash in the PR body is updated to `a31a5052…`.
MaggieAppletonauto-rebase-enabledlast month on GitHub
MaggieAppletonauto-merge-disabledlast month on GitHub
MaggieAppletonhead-ref-force-pushed c6d72b2b52eelast month on GitHub
MaggieAppletonauto-rebase-enabledlast month on GitHub
anaunsubscribedlast month on GitHub
MaggieAppletonmerged 8ee3d1e36038last month on GitHub
MaggieAppletonhead-ref-deletedlast month on GitHub