Skip to content

feat(v2): direction C PR 2b — threading restyle: runtime tag, action strip, thread band, quote link, +N reactions - #1582

Merged
lilyshen0722 merged 5 commits into
mainfrom
feat/c-thread-restyle
Sep 7, 2026
Merged

lilyshen0722 merged 5 commits into
mainfrom
feat/c-thread-restyle

Conversation

@lilyshen0722

Copy link
Copy Markdown
Contributor

Direction C, PR 2b — the Thread-states artboards (canvas page 5, walk 3 direction-c-walk-3-threads.md), on top of PR 2a (#1579).

What changes

  • Runtime tag + two-tone avatar on agent rows. .v2-msg__tag reads agent.runtime.runtimeType (mono, lowercase); humans carry neither.
  • One action strip on hover — react · reply · thread · more (30px, r4, no shadow). More = copy link / copy text.
  • Phone (≤760): a ~500ms long-press reveals the strip inline under the body (28px targets); a tap on the text dismisses it. The click that ends a long-press on touch stacks is latched so it cannot hide the strip the instant it appeared. The revealed strip is pinned to the body's grid column — the row is a 38px | 1fr grid and an auto-placed strip pushed the body into the avatar column (one word per line, found in the 390 walk).
  • Thread = chip under its root (N replies · last 2h). Open: band #f9fafb with a 2px rule, newest 8 replies + N more replies, Collapse foot beside Reply in thread.
  • Quote is a link that lands on its source (#message-<id>landOnMessage), paging older history until the row is loaded.
  • Reactions past six fold into a +N chip that expands the row.
  • i18n en / zh-CN for strip, thread, quote and reactions strings.

Proof

  • V2ThreadRestyle.test.tsx (6): tag/agent class, +N reactions, quote landing/hash, band + more-replies + collapse, 390 long-press → click latch → tap dismiss; v2-layout-invariants guards the pinned strip (grid-column: 2; grid-row: 2), strip/band/chip rules; 116 tests across the touched suites green; tsc clean; eslint 0 errors.
  • Real-browser walk of the built bundle (scratchpad/pw/walk-pr2b.js, Sharpen, 1440 + 390 with CDP touch): 21 runtime tags on 30 agent rows; strip 30px r4 in order react·reply·thread·more; More menu = Copy link / Copy text; 12 thread chips (28px) → open band rgb(249,250,251) + 2px rule → Collapse closes; reaction chip 20px r4; jump pill 28px r14 #dde0e6; 390 long-press reveals the strip static under the body at full body width, tap dismisses.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JavWvyVL478UfEhJjcfMgX

…strip, thread band, quote link, +N reactions

The Thread-states artboards (direction C walk 3), on top of PR 2a:

- Agent rows carry a mono runtime tag (`.v2-msg__tag`, from
  `agent.runtime.runtimeType`) and the two-tone avatar; humans carry neither.
- Actions are one strip on hover (react · reply · thread · more), 30px r4,
  with a More menu (copy link / copy text). At ≤760 a ~500ms long-press
  reveals the strip inline under the body (28px targets); a tap on the text
  dismisses it. The click that ends a long-press on touch stacks is latched so
  it cannot hide the strip the instant it appeared (found in the 390 walk).
- A thread is a chip under its root (`N replies · last 2h`); open, the band is
  #f9fafb with a 2px rule, newest 8 replies + `N more replies`, and a Collapse
  foot beside Reply in thread.
- A quoted reply is a link that lands on its source (`#message-<id>`,
  `landOnMessage`), paging older history until the row is loaded.
- Reactions past six fold into a `+N` chip that expands the row.
- i18n en/zh-CN for strip, thread, quote and reactions strings; tests in
  V2ThreadRestyle (6) + invariants; walked at 1440 and 390 (real touch via CDP).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JavWvyVL478UfEhJjcfMgX

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

CODE GATE: clear at fe160e2e (head re-resolved before filing). Nothing blocking; one question for @ux-lead below.

Both defects the 390 walk found are pinned — I mutated rather than read:

mutation result
drop grid-column: 2; grid-row: 2 from .v2-thread .v2-msg--reveal .v2-msg__strip 1 red
remove the pressRevealedRef latch from onBubbleTap 1 red

That is a real change from where 2a started, where the headline fix was revertible in one line with 566 tests green. Suite at this head: 70 suites, 585 tests (577 for src/v2 alone), in-band in a clean worktree.

threadView.ts is untouched, so the flatIds freeze and the N+ replies count are still ahead of us rather than half-landed here. Good — those belong with scroll-load, not with the restyle.

One question, not a defect

The reveal is gated on capability and the placement fix on width, and they do not describe the same set:

  • isHoverless() is matchMedia('(hover: none)') — no width term (V2MessageRow.tsx:355)
  • the static/grid placement lives in @media (max-width: 760px) (v2.css:11347)

So a hoverless viewport wider than 760px — iPad landscape, a touchscreen laptop — can long-press to reveal and gets the base position: absolute strip instead. I do not think that reproduces the crush you fixed, because absolute takes the strip out of flow and there is no grid cell to fight over; it just serves the desktop hover presentation to a touch user. Worth a deliberate answer rather than leaving the two gates accidentally different.

Note on the dual class

V2MessageActions renders className="v2-msg__actions v2-msg__strip", documented in the file as keeping the old name for existing invariants while __strip is the direction-C contract name. Ten v2-msg__actions rules remain in the sheet and still apply, so this is two rule sets on one element. It resolves correctly today — the direction-C rules are .v2-thread … at higher specificity and come later — but it is the same cascade shape as the double pod-header meta from this morning, and a future rule added under the old name at equal specificity would land on this element silently.

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

UX-GATE: CHANGES_REQUESTED @ fe160e2 — walked the built bundle with mocked threads (10-reply + 1-reply roots, quoted reply, 8 reactions, agent rows with runtime) at 1440 + 390. Matches: runtime tag (tint, mono 11 muted, 16px, r3, after the time), reaction chips 20px r4 with mine in wash + #d4dbf7 + cobalt and +N folding, thread chip 28/32 with 16px faces + cobalt 'N replies' + mono '· last', band #f9fafb with 2px #d0d5dd rule + 14px indent + 24px reply avatars + 'N more replies' at the top + Collapse foot, quote block 2px rule 13px author 600, grouped rows with time on hover, focus shows the strip, More = Copy link / Copy text, 390 long-press strip inline under the body in the body column with 28px targets and tap-to-dismiss, no green, no shadows, nothing under 11px. Misses: (1) avatars are still the illustrated gradient set on every row — the cobalt rule targets :not(.v2-avatar--photo) but the component renders an illustration, not initials; spec is human tint #f2f4f7 + #4f5c6e initials, agent cobalt + white initials, photo only when the user has one (carried from 2a, named for 2b); (2) expanded thread: the root row sits OUTSIDE the band — band and rule start at the chip, and the rule lands under the avatar column (x421 vs body x469); board is root + replies in ONE #f9fafb band with the rule at the text column; (3) the chip row still carries 'Reply in thread' + 'Follow' buttons and a chevron when open — spec is the chip alone (faces + count + last), Reply in thread lives in the foot; (4) threads mount EXPANDED by default (2 open blocks on load) — the board's resting state is the chip, expanded on click; (5) the strip has three icons — react is missing; spec is react · reply · thread · more, four 24px icons (strip 30px vs 28 accepted); (6) quote → a reply hidden behind 'N more replies' sets #message-2032 but neither expands the fold nor lands (.v2-msg--landed never appears); (7) mention token has no wash — text-only cobalt with a row tint band; spec is #e8ecfb wash + cobalt on the token. Minor accepted: Collapse 500 vs 600.

…ct in the strip, flat avatars

@ux-lead CHANGES_REQUESTED at fe160e2, all seven applied and walked:

1. Transcript avatars are flat two-tone squares (`tone="flat"`): human tint
   #f2f4f7 + #4f5c6e initials, agent cobalt + white; a photo still wins, and
   the illustrated character tier stays out of the transcript. The cobalt
   allowlist moves to `.v2-thread .v2-avatar--flat-agent`.
2. The ROOT renders inside its band — the block is a full-width child and the
   indent moved to `.v2-thread-block__inner`, so the rule lands at the text
   column (measured: rule x419 == body x419; it was 421 vs 469).
3. The chip is faces + count + `· last` and nothing else. Reply in thread,
   Follow and Collapse live in the band's foot; the chevron is gone, and the
   44px touch floor moved with them.
4. A thread rests as a chip. Which roots are open is session state in the
   transcript; the server `collapsed` row is written to match on open/close
   via a new `setCollapsed` (it still drives the wake path) rather than
   deciding the resting view.
5. React is the strip's first icon whenever the strip renders — disabled, not
   absent, when the row cannot take a reaction.
6. A quote to a reply folded behind `N more replies` REVEALS: the transcript
   opens that thread and clears its fold, then the landing effect re-runs.
   It no longer pages older history for a message that is not old
   (sprint-review 64477).
7. Mention token carries the #e8ecfb wash with cobalt text.

Also @sprint-review 64468 + 64485: the reveal's placement now matches the
row's `matchMedia('(hover: none)')` exactly — no width term, no
`(pointer: coarse)` (which would style stylus/hybrid machines where the
reveal never fires), and the block sits after the `.v2-thread .v2-msg__strip`
rules it overrides, since those win on order at equal specificity. Sizing
stays in the width block. Guarded by an invariant asserting both.

579 tests green, tsc clean, eslint 0 errors. Walked at 1440 and 390 (real
touch via CDP): 0 open blocks at rest with 10 chips, band #f9fafb with the
root inside, foot `Collapse · 1 reply · Reply in thread · Following`, strip
react·reply·thread·more, agent avatar rgb(29,63,209) with initials, mention
#e8ecfb/cobalt, 390 strip static in the body column at x50 with 44px foot
targets and no horizontal scroll.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JavWvyVL478UfEhJjcfMgX

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

CODE GATE @ 2b130ee9 — the ordering fix is right and proved. Two changes are unpinned, one of them the item flagged as resting on its test.

The ordering correction landed, and the guard discriminates

@media (hover: none) at v2.css:11351, after .v2-thread .v2-msg__strip at 11229; no width term, no (pointer: coarse). I ran the 2×2 I used on the meta guard this morning, because an indexOf comparison is the shape that goes vacuous:

mutation result
capability block moved before the strip rules 1 red
same, plus the strip rule head reformatted so the needle misses 1 red

Both cells fail, so unlike the earlier meta guard this one does not decay on a whitespace change. Good.

The quote reveal is not pinned — and it is the item you said rests on its test

Removing the reveal branch entirely from V2Thread.tsx:612-616, so the effect falls straight through to handleLoadOlder() — the exact bug — leaves 571 tests green.

V2ThreadRestyle.test.tsx:179 does name the case, and it is a good test of the consumer: it renders V2ThreadMessages with revealMessageId: '22' handed in directly. Nothing exercises the producer, the decision in V2Thread that computes folded and sets revealRequest from the hash. So the half you could not verify live is the half no test covers.

This is the same producer/consumer asymmetry as the image manifest in 2a — consumer tested, producer bare — and it is worth naming as a pattern rather than a one-off, because it has now produced the two most serious findings on this stack. A test that sets location.hash to a folded reply's id and asserts revealRequest is set and loadOlder is NOT called closes it.

The collapsed write is also unpinned

Deleting the api.put('/api/messages/:id/collapsed') call from useV2ThreadState.setCollapsed leaves 571 green. The rest state would still look right in every test while never persisting.

Checked around it, and these are fine: the route exists on main (backend/routes/messages.ts:165, reactionRateLimit + dualAuth), the limiter is 120/min which no human interaction will reach, and setCollapsed's !row early-return means a first load with 10 chips issues no bulk write.

Verified clean

571 tests, 69 suites, in-band in a clean worktree at this sha. Frontend-only; no backend path touched, so the collapsed route and its auth are unchanged.

Neither unpinned item is a behavioural defect — both work at this head. They are one line from not working, with nothing to say so.

…-fetch and collapsed persistence

64500 / 64501: both changes were covered at the CONSUMER and bare at the
producer, so each was one line from silently not working. Same shape as 2a's
image manifest, which is now three findings from one pattern.

New suite `V2ThreadReveal.test.tsx` (6):

- The landing effect's decision, from the hash rather than an injected prop:
  a target folded inside a collapsed thread opens that thread and clears the
  fold with NO history fetch; a target absent from the window takes the fetch
  path and opens nothing; a target already on screen does neither and just
  lands.
- `setCollapsed` writes `{collapsed}` to `/api/messages/:id/collapsed` in both
  directions, writes nothing for a no-op or an unknown root (a 10-chip first
  load must not issue a bulk write), and reverts the optimistic state when the
  write fails.

Mutation-checked rather than read: deleting the reveal branch gives 1 red,
deleting the PUT gives 2 red, and both were green before this commit.

The api mock is ONE object for every render. A factory returning a fresh
object per call re-fires the hook's fetch effect on every render — 66 calls in
50ms when I got it wrong here, the same trap as the PR 1 attention mock.

577 v2 tests green, tsc clean, eslint 0 errors.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JavWvyVL478UfEhJjcfMgX

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

CODE GATE: clear at c8d4b619 (head re-resolved before filing). Delta from 2b130ee9 is one new test file.

Both producers are pinned. I re-ran the same mutations that were green before this commit:

mutation before (2b130ee9) now
delete the reveal branch from V2Thread.tsx 571 green 1 red
delete api.put(.../collapsed) from setCollapsed 571 green 2 red

577 tests, in-band in a clean worktree.

I also checked the suite for the two ways this kind of test passes for the wrong reason, and it survives both:

  • expect(detail.loadOlder).not.toHaveBeenCalled() at :91 would be vacuous if the effect never ran at all — but :87 asserts .v2-thread-block--open exists in the same test, so the effect demonstrably ran and chose the reveal. Same pairing at :110/:108 for the already-on-screen case.
  • The three landing cases are mutually exclusive and each asserts what the other two would do (--open present vs null, loadOlder called vs not), so a change that collapses two paths into one cannot pass all three.

The deliberate non-assertion is correctly scoped. :100 uses toHaveBeenCalled() rather than toHaveBeenCalledTimes(1); with loadingOlder pinned false in the fixture the effect can re-fire, so a count there would measure the fixture rather than the code. Stating that in the PR rather than quietly writing the weaker assertion is the right call — an unexplained toHaveBeenCalled is the kind of thing a later reader tightens into a flake.

Nothing outstanding from me on #1582.

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

UX-GATE: CHANGES_REQUESTED @ c8d4b61 — repeat quote navigation, scroll-load continuity, and the type floor still fail the Direction C contract.

Real Chromium walk of the production build at 1440×900, 1200×900 and 390×844; deterministic message fixtures, with an actual touch context at 390. This is a pre-merge gate, not live deployment verification.

Blocking findings:

  • frontend/src/v2/components/V2MessageRow.tsx:637 and V2Thread.tsx:607: click a quote targeting a reply hidden in a collapsed thread → first reveal lands correctly. Collapse, then click the same quote again → hash remains #message-2032, zero open blocks, target absent. A new navigation gesture must reveal and land again even when its target/hash matches the previous one. Pin the repeated gesture, not only the first landing.
  • frontend/src/v2/utils/threadView.ts:55 and :63: a loaded orphan reply renders flat while its root is outside the window. One older fetch returning the root removes the already-visible reply and hides it inside a resting chip. This violates walk-3 §5 (loaded rows never relocate on scroll-load). The specified flatIds handling in both passes is absent at this head. Browser reproduction: reply 2032 present before, absent after root 2030 arrives; 10 replies chip appears, zero open blocks.
  • frontend/src/v2/v2.css:11333: flat avatar initials compute to 11px sans on root rows, 9px sans on replies and 7px sans in chips. Ruling h is sans ≥12, or explicit mono ≥11 in every sizing rule. Running #1580's exact scan against this head reports five new selectors below the floor, plus the already-known stale composer-send allowance. Fix the sizing rules; do not expand the allowlist.

Additional geometry correction: .v2-msg__strip measures 30px desktop and 34px touch (24/28px buttons plus padding/border), against the 28px strip contract. Keep the four 24px icons and account for padding/border in the total. This mismatch was present in the earlier walk and was missed in that stamp.

Verified fixes: threads rest as chips; root and replies share the band; rule and root text align at x419 desktop/x50 mobile; chips carry no extra controls; first folded-quote reveal lands at both sizes; avatar colors and 28/24px boxes match; mention wash matches; four icons, keyboard focus, More menu, mobile long-press and tap dismissal work; no horizontal overflow at 390 or 1200. The earlier keyboard probe hit the newly disabled reaction button; focusing an enabled action passes after the opacity transition.

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

UX-GATE: APPROVED @ 78fd4ff — carried from reviewed #1585 after verifying identical commit and tree (769f7f85). Prior 1440/390 repeat-quote, orphan-continuity and type-floor checks pass; actual 390 touch re-walk confirms corrected 28px strip / 24px actions. Type-floor scan 5/5. Supersedes UX changes requested at c8d4b61. These were production PR-build browser checks with mocked history, not a live-site smoke; live interaction verification follows deployment. Code gate remains separate.

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

CODE GATE: carried to 78fd4ff8 by identity, not by re-derivation.

This PR's head is now the same commit I gated on #158578fd4ff82a32d360302c3751b4fca750176cf064 on both. Not equivalent content on a rebased tree; the same object. So the gate transfers with nothing to re-run:

  • placement guard mutation → 1 red (was green at c763d3b4; the mixed frozen/unfrozen case closed it)
  • touch override reverted to 28×28 → 1 red
  • type floor: main 86, this head 86, nothing new
  • 582 tests, in-band in a clean worktree

Filing it here rather than only on #1585 because this is the PR that gets pressed, and a reader checking this PR's reviews sees four COMMENTED events at , and — all superseded — with nothing at the current head. That gap is exactly how a stale gate gets mistaken for the live one; it has happened four times on this stack already.

Two housekeeping notes.

#1585 now has the same head and the same base (main) as this PR, so the two are duplicates rather than a stack. Whichever is pressed, the other should be closed deliberately rather than left as a zombie showing an empty diff.

@ux-lead's CHANGES_REQUESTED is stamped at c8d4b619, three heads back. Their 390 re-walk of the strip geometry at this head is the outstanding gate, not the code one.

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Correction to the sentence above: three shas dropped out of it. My shell expanded the backticked list as command substitution, so the line reads "four COMMENTED events at , and" with the values gone.

The superseded reviews on this PR are at fe160e2e, 2b130ee9 and c8d4b619 (two at the last). Everything else in that comment stands as written — the gate carries to 78fd4ff8 by object identity, and the mutation results are unchanged.

@lilyshen0722
lilyshen0722 merged commit 7357c1e into main Sep 7, 2026
14 checks passed
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