Redesign review comment cards: author border, hover linking, unresolve - #15
Merged
Conversation
Implements the attn-bb6t epic across the native and hosted surfaces. Card border (attn-bb6t.1): the 3px left accent strip becomes a full 2px border in the comment author's identity colour. The strip was a positioned pseudo-element because neither border-left nor an inset shadow survives the 6px corner radius — one mitres, the other tapers. A uniform border has neither failure mode, so the pseudo-element, its stacking context and the -1px offsets that kept it as tall as the card all go with it. Active no longer recolours the border (that would erase author identity); it is an outer ring instead. Excerpts and hover linking (attn-bb6t.2): the quoted excerpt is gone from anchored cards, replaced by hover in both directions. Stale cards keep their quote — a stale anchor paints no inline mark, so the quote is the only thing left that says what the comment was about. Card-to-document hover toggles a class straight on the mark DOM rather than rebuilding the decoration set, which is what the existing perf note in that file asks for. The is-focused class, emitted by the plugin since it was written but never styled anywhere, is now implemented too. HTML annotations (attn-bb6t.3): hovered becomes its own AnchorRenderState, distinct from active, with a third CSS Custom Highlight bucket. Restoring an anchor's base state on un-hover is owned by the bridge, which retains the rendered specs — a shell that got this wrong would leave a resolved anchor painted as unresolved. The frame-to-shell direction needs a new additive anchorHover message: text ranges are highlights, not DOM nodes, so they receive no events and are hit-tested against getClientRects(). Unresolve (attn-bb6t.4/.5): resolve was one-way by design, so reopening needs a new CommentReopened event — authorised exactly like resolve (non-agent, self-attributed), threaded through the session facades, and pinned by a new signing-corpus vector. The projection now folds resolve and reopen last-writer-wins by event comparison: the log is not delivery-ordered, so a Set of closed ids would flip a thread open or closed depending on arrival. Resolved cards gain an Unresolve action, the ticks become Lucide check icons, and the resolved badge spans the rail like the cards it sits among. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…n line The landing promised reviewers who "aren't human" and never showed one (critique 2026-08-18, P1). This makes the proof real end to end: - Hero recapture from a live session: a human comment (Sam Porter) and an agent suggestion (Claude, violet/hex) in one margin, both themes, deeper document. New reproducible pipeline: capture-collab-screenshots.sh (modernized for the durable-share flow, name-prompt personas, agent participant, native snapshots, stills-only mode) + sharp-based build-landing-screenshots.mjs for the AVIF/WebP set. - Agent runtime grew the honest mechanics the capture needed: suggest-diff (anchored diff suggestions under the agent identity) and kind:"agent" join (join_self_as_agent / ReviewCommand::JoinAsAgent) so the roster announce and event signatures stay one participant. - "Watch a live review" link surfaces the existing guided demo (/app?surface=landing-review-demo); verified cold, desktop + mobile. - Join line split by register: "Agents join by invite, as peers" on the Review-room stage label; verbatim `attn review join <invite> --as-agent` beside the install commands. - Clarify: category sentence opens the lede; desk labels unified. - Harden: scroll-behavior gated on prefers-reduced-motion; desk-count CTA refreshes on bfcache restore. - Share-proof asset cropped to the dialog's trust core (kills the dimmed backdrop slab); review-card meta chips lifted to AA contrast. Note: Hero.svelte and manager.rs also carry pre-existing uncommitted hunks from this branch's in-progress work; they ride along here rather than being hunk-split. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment-only pass across the web, relay, and Rust sources: drop agent commentary and restate what survives in the present tense. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…he desk seam Implements attn-a9f7, the three-tier plan from the /impeccable critique of the hosted app routes (31/40). Phase 1 — make existing promises true: - Enter opens the arrow-selected workspace from anywhere on the desk; it used to require focus in the filter field, so ↓ then Enter did nothing. - ConfirmPanel owns one destructive-confirm contract (focus in, Tab trapped, Escape out, focus restored). Storage's two alertdialogs had none of it. - The desk error state says what is safe, offers Try again and Check storage, and keeps the raw exception behind a disclosure. - Export all builds ONE archive and claims "backed up" only after it is delivered; the per-workspace download loop could be swallowed by the browser while every row still reported success. - The delete confirm can export in place instead of sending the user away. - Copy and type-ramp sweep across /open and /app/storage. - The first-run desk spends the pencil once: on the caret, not also the tile. - ⌘K is advertised in the editor header, and one .kbd-chip replaces three. Phase 2 — one grammar: - The desk header moves onto the accent plane DESIGN.md already claimed all headers shared, re-pointing the hosted aliases the way app.css re-points the canonical tokens. Its badge tiers keep three non-colour channels. - chrome-dialect.test.ts pins the rule: tokens are canonical, the ramp is closed. It found three off-ramp sizes and two forked colour literals. Phase 3 — the continuous desk: - Desk↔workspace is an in-app transition; the route is state, not a mount-time prop, and one delegated link handler keeps anchors real anchors. - One shell renders the header once for desk, storage and open. - The populated desk leads with review work: a "Waiting on you" group, and the counts move from the fifth column to the second. - ⌘K works on the desk, loaded on first use so the static graph is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Snapshot commit taken to unblock a fast-forward merge — this captures another session's uncommitted working tree as-is and may be mid-change. Reversible with `git reset --soft HEAD~1`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tinuous desk Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> # Conflicts: # web/src/hosted/app/AppShell.svelte # web/src/hosted/app/DeskHome.svelte # web/src/hosted/app/EditorShell.svelte # web/src/hosted/app/OpenPage.svelte # web/src/hosted/app/StoragePage.svelte # web/src/hosted/app/app-shell.css
The header reads mark | workspace | file. The mark is a link home; the workspace opens the switcher and the file says where you are, both set in one face, size, weight and colour — printed at two weights they read as a mistake rather than as a hint. The chevron is gone from the trigger, so hover and focus carry the affordance. No file segment at all while the canvas is still asking which document to open: the route mints a placeholder untitled.md and navigates to it, so a non-empty path was never evidence anyone had chosen anything. Renaming edits the word. Both fields drop their border, fill and fixed width: the workspace name grows with what is typed (a sizer pseudo-element in a one-cell grid, since an <input> measures from `size` and no CSS width can shrink past it), and the file name takes its own tree row, which stops being a <button> for exactly as long as it is a field. The file rename used to render at the FOOT of the rail, hundreds of pixels from the row it renamed — the rail's half of attn-rjuo.2.1, finally done. Also: a "Back to desk" row at the top of the rail, which is where the way out of a workspace belongs; the picker's action rows inset so their focus ring stops being clipped by the popover's own overflow (and, with the same inset, finally aligned to the project labels above them); the desk's two empty-state routes set alike, with order rather than weight marking which one leads; and the loading line moved from a 0.85rem caption to the desk title's register with a spinner beside it, since a whole empty viewport carrying one caption reads as a page that failed to load. Contrast measured on the accent plane and the rail: every text pair now clears 4.5:1, the focus underline and spinner clear 3:1, and both honour prefers-reduced-motion. Includes prior in-progress work from this branch (review-trouble dialog, file-drop, workspace-origin, landing and DESIGN.md edits) that was already in the working tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four findings from the Codex review of this branch (attn-1l2f). Unresolve reopened decided suggestions (attn-1l2f.1). Accepted and rejected suggestions read as `resolved` too, so their expanded card carried the Unresolve button, and the `comment_reopened` it minted won last-writer-wins in `reconstructThreads` — an already-applied edit came back with its Accept/Reject actions intact. Only a comment thread reopens now, stated once in the pure layer and enforced at three depths: the affordance, the projection (threads and counts alike), and `reopen_comment`, which asks the store whether the id names a suggestion before minting anything. A file read could land in the wrong workspace (attn-1l2f.2). `applyEntry` guarded against a newer `applyEntry` and `navigate` against a newer `navigate`, and neither could see the other, so a read issued in workspace A resolving after B was open wrote A's body, path and URL into B — and the next autosave saved A's text against B. One transition token is now shared by every move, and reads carry the workspace they were issued against, because generation alone cannot say "this body belongs to a document we are no longer showing". Entry paths were interpolated raw into URLs (attn-1l2f.3). `draft#1.md` became a fragment and `plan?.md` a query string, so a reload or a copied link reopened the wrong document or none. `parseAppRoute` already decoded per segment; `appWorkspaceUrl` is the encoding half that was missing, and every /app/w writer goes through it. Enter on the desk pre-empted focused controls (attn-1l2f.4). The handler is on the window, and once an arrow key had set a selection it claimed Enter from every button and link on the page — Tab, Delete, Enter silently opened a workspace. Enter is the desk's only when no control is holding it; the filter field stays the exception. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codex's review of this branch found five defects in the seam where the desktop app switches workspaces WITHOUT unmounting the editor — same EditorShell, new workspace prop — plus two folds that answered by arrival order (attn-e9r2). An import is several awaits long, and every step read the workspace id live: dropping a folder on A and clicking B before it finished deleted B's untitled.md, renamed B after A's import, and moved the user out of the workspace they had just opened. The pipeline moves to import-into-workspace.ts, where the id is a PARAMETER; the live reads (the placeholder heuristic, the name to adopt, the navigation) are gated on that workspace still being on screen. The files still land, because a half-read import is the one outcome nobody can recover from. Acquiring the pen is slow by design — a polite ask, a handoff doorbell, a grace period — and the acquisition was tracked as a bare promise with no workspace attached. B's request was answered with A's pending promise, and when it resolved it installed A's runtime as B's session, so autosave committed B's open file through A's fence. owner-session-gate.ts holds the pair and discards a grant that arrives too late — including A -> B -> A, where the id alone says the stale attempt is current again. A discarded session is not merely dropped: nothing installed it, so nothing else would ever close it. Leaving a workspace now hands its runtime back (closeOwnerRuntime closes and evicts; close() flushes the final share republish and says goodbye to followers first). Before this, every departed workspace stayed held against the user's other tabs and repeated switches accumulated live runtimes. review-counts folded comment lifecycle in ARRIVAL order, so a newer reopen read before a delayed older resolve left the thread closed on the desk and open in the rail — the badge disagreeing with the surface it links to. It now folds last-writer-wins through compareEventKeys, exported from selectors.ts so both projections order by literally the same rule. A dropped folder past the file/depth ceilings stopped the walk and returned what it had, and the import reported success over a tree with files missing that nobody could name. The walk throws DropLimitError instead, and FileDropOptions.onError is required so every drop surface has to say where the message goes. Tests: 20 new/extended unit cases across the four suites, including the two races driven deterministically; hosted-workspace-switch.spec.ts is the end-to-end floor in the real app. npm --prefix web run check clean; 132 unit files green; build:browser + check:route-bundles clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four surfaces that answered a failure by carrying on as though nothing had happened (attn-ze60, from a Codex review of this branch). A dropped folder whose read failed partway was imported as a shorter tree: both readers answered the entries API's error callback by resolving empty, which is also how the walk signals end-of-directory. They reject now, with a DropReadError the drop surfaces already had a callback for — the same silent truncation the ceilings were written to prevent, arriving through a different door. The editor's chunk had no rejection handler, so a fetch that failed — offline, or hashed chunks 404ing because the deployment the document came from has been replaced — left the workspace under "Opening ..." indefinitely. Retrying the import cannot help; the module map answers with the same rejection without going back to the network. So: one automatic reload, with the ceiling of one as the load-bearing part, then a surface that says the workspace is safe and offers a way out. The desk read #join once at boot and handed the answer down, but the desk mounts more than once per page load. The snapshot was wrong in both directions — it reopened the panel over an address bar that had said plain /app for two screens, and left it shut on a Back to an entry that genuinely asked for #join. It asks the URL now, every mount. And a re-render under the pointer painted over its own hover: nothing downstream would put it back, so the card stayed lit while the document went dark until the pointer left and came back. Each case is pinned by a test that fails without its fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The loading line ran a spinner in front of its sentence, and every surface that mounts it centres its single child — so what was centred was the group, and the words sat ~15px right of the middle by a distance that changed with the sentence. Moving from one stage of a wait to the next slid the line sideways. The spinner is gone, with its keyframes and its reduced-motion counterpart. The two loading surfaces also stop asking percentage heights of every ancestor in turn, which only fills the screen while each of them holds a definite height; the viewport is the floor now, and a grid item still stretches into a taller parent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Implements the attn-bb6t epic (all five children) across the native app, the hosted owner shell, and the browser reviewer. Closes attn-bb6t.
What changed
Card border — the 3px left accent strip becomes a full 2px border in the comment author's identity colour, radius unchanged. The strip existed as a positioned pseudo-element because neither
border-leftnor an inset shadow survives the 6px corner radius: one mitres into the curve, the other tapers at both ends. A uniform border has neither failure mode, so the pseudo-element, its stacking context, and the-1pxoffsets that kept it as tall as the card are all gone with it. Active/selected no longer recolours the border — that would erase author identity — and is an outer ring instead.Excerpts and hover linking — the quoted excerpt is removed from anchored cards and replaced by hover in both directions. Stale cards keep their quote: a stale anchor paints no inline mark, so the quote is the only surviving trace of what the comment was about.
Card→document hover toggles a class directly on the mark DOM rather than rebuilding the decoration set, which is what the existing perf note in
App.svelteasks for (buildDecorationsrebuilds every mark in the document; hovering a card shouldn't cost that). While wiring this I found the plugin has always emitted anis-focusedclass with no CSS anywhere — clicking a card gave zero document feedback. That's implemented too.HTML annotations —
hoveredbecomes its ownAnchorRenderState, deliberately distinct fromactive(which means "focused"), with a third CSS Custom Highlight bucket. Restoring an anchor's base state on un-hover is owned by the bridge rather than the three shells, because the bridge is the only thing retaining the rendered specs — a shell that got this wrong would leave a resolved anchor painted as unresolved.The frame→shell direction needed a new additive
anchorHovermessage (DOC_PROTOCOL_VERSIONstays 1; both sides already ignore unknown types). Text ranges are CSS Custom Highlights, not DOM nodes, so they receive no events at all and are hit-tested geometrically againstgetClientRects().Unresolve — resolve was one-way by design (there's a comment in
manager.rssaying reopening would need a futureCommentReopenedevent), so this adds that event: authorised exactly like resolve (non-agent, self-attributed), threaded through the full session facade chain, and pinned by a new shared signing-corpus vector.The subtle part is the projection. Resolved state was an add-only
Setof thread ids, and the event array is not in log order — replay and live delivery interleave peers. A naive add/delete pass would flip a thread open or closed depending on arrival order.reconstructThreadsnow folds resolve/reopen last-writer-wins by the same comparator that orders the log.Resolved cards gain an Unresolve action (the resolved footer branch was deliberately empty under attn-42y, which was a statement about the protocol rather than a design choice), the ticks become Lucide check icons, and the resolved badge spans the rail like the cards it sits among.
Verification
Verified against an isolated checkout of this commit alone, not the tree it was developed in — the commit deliberately excludes unrelated in-flight work from a parallel session, so "it works locally" wouldn't have proven much.
svelte-checkclean, doc-runtime artifact in sync, production web build OKNotes for review
CommentReopenedis a new externally-tagged event variant, so receivers on 0.9.0 and earlier reject it outright and will keep showing a reopened thread as resolved — silently. Same family as the snapshot-field problem in attn-mz25; recorded on attn-mz25.1 so the rollout answer covers new event variants too.runtime.generated.js,doc-runtime.generated.ts) are rebuilt and committed, as the build gate requires.🤖 Generated with Claude Code