Skip to content

feat(golf/messages): premium mobile rebuild — the thread was sunken, not flat - #1833

Open
njrini99-code wants to merge 36 commits into
mainfrom
agent/messages-instant-entry
Open

feat(golf/messages): premium mobile rebuild — the thread was sunken, not flat#1833
njrini99-code wants to merge 36 commits into
mainfrom
agent/messages-instant-entry

Conversation

@njrini99-code

@njrini99-code njrini99-code commented Sep 4, 2026

Copy link
Copy Markdown
Owner

Mobile messaging rebuild. 16 commits. Not merged, not deployed.

READ FIRST — production schema is AHEAD of main.
Three migrations in this branch are already applied to production and recorded in supabase_migrations.schema_migrations. Production has the tables/columns; main does not have the files. Merging this reconciles them. Do not re-apply.

Version What
20260904103000 golf_message_reactions + golf_conversation_has_me()
20260904120000 golf_messages.reply_to_id
20260904160000 kind/payload, golf_message_responses, golf_message_mentions, pinned_at/by, notification_level/muted_until

Applied via the Management API (/database/query), which executes SQL directly and does not write the ledger — I inserted the ledger rows by hand. If you add migrations, either use the same path and record them, or expect drift tooling to disagree.


The bug that made four earlier visual passes invisible

The thread well was bg-surfaceoklch(0.984), the lightest colour in the system — and incoming bubbles were bg-surface-sunken at 0.963. Every bubble was 0.021 darker than the surface under it. Pressed into the page, not resting on it.

design-tokens.css already had the answer at --fw-color-canvas: "the ~0.03 gap is the premium card/page separation the flat-cream look lacked."

This defect recurred three more times in the same session — the rail's avatars (surface-sunken on canvas, 1%), the loading skeleton bars, and the unpicked poll bar (surface on surface-sunken, 2%). This design system has four neutrals inside a 3% lightness band. Picking two of them for figure-and-ground produces something invisible, every time. Check any new fill against its actual ground.


What's in here

Thread — the screen is the conversation below md; three-column nav bar; short threads bottom-anchor via mt-auto; 17px text; spring arrival (420/34/0.75); Sent → Read crossfade in reserved geometry; reactions; reply/quote; details sheet; long-press lift; failed sends that stay put with Retry; new-messages button; incoming avatars; group typing by name.

Inbox — real member faces (AvatarGroup); PressTarget instead of an over-ridden Button; one continuous list (no date sections); loose filter pills; one right rail per row; solid unread badge with NumberFlow; layout="position" reorder; compose on the search row.

Composer — clears in the same tick as commit (was await onSend(...) then clear, with a bouncing-dot spinner); fwHaptic('light') at commit; no sending state.

Structured Helm objects — practice, event, RSVP, poll, travel, system narration. A structured message is a message: it rides golf_messages, so every existing RLS policy, realtime subscription, reply, reaction and search path works on it unchanged.

Removed — per-message email fanout (a ten-message thread mailed every participant ten times).


Verification — read this before trusting any screenshot in the history

Screenshots earlier in this branch were hand-written HTML approximating the components, not the components. That approximation drifted and hid at least one real bug (a wrapped "View in Calendar" button, fixed in the last commit).

The working method is in the last commit: render the actual component via react-dom/server, compile Tailwind against real source files, screenshot that. It found the bug on its first run.

Nothing here has run on a device. The composer's safe-area arithmetic changed, springs were added, keyboard geometry moved — the three most device-sensitive things in the spec. §55's physical-iPhone pass is exactly what a harness cannot do.


NOT DONE

Built schema, no UI: mentions (tables + policies exist; no @ autocomplete), pins (pinned_at/pinned_by exist; no affordance), mute (notification_level/muted_until exist; no sheet).

Not started: slash actions (/practice, /poll…), in-app notification banner, offline/reconnecting status, search-hit highlight, scroll-edge material, swipe-to-reply, layoutId avatar continuity (§32 — needs both trees alive across the master-detail transition; investigate before building).

Verification tier — none of it: observability instrumentation, render profiling, the 320–430px overflow lab, screenshot regression pack, physical-iPhone QA, latency measurement.


Traps worth knowing

  • sqlfluff fix will break your SQL. Rule RF03 rewrote WHERE m.id = message_id to m.id = m.message_id in two RLS policies — message_id belonged to the outer row. ERROR: 42703. Caught only by executing the rewritten SQL. Always run migrations after auto-fixing them.
  • Tailwind class names are not typechecked. I shipped text-danger-600, which doesn't exist; the real token is fw-danger-ink. Typecheck and lint both passed.
  • gh in the Bash sandbox works for one or two calls, then fails with x509: OSStatus -26276. Run bursts outside the sandbox.
  • Supabase Preview is red for a PRE-EXISTING reason20260903220000_feature_health_heartbeat_indexes.sql (fix(bridge): five production fixes found by reading the live Bridge, in one PR #1816) uses CREATE INDEX CONCURRENTLY, which cannot run inside the CLI's transaction. The index exists in production, so it arrived another way, but a clean rebuild from migrations dies there. Not caused by this branch, and it means the migration validator can't verify anyone's DDL right now.
  • Migration files here are sqlfluff-clean and ratchet-passing. Keep them that way; the ratchet only lets the count go down.

RLS was exercised, not just shaped

For reactions, in rolled-back transactions on production, with a control that passes so the denials aren't vacuous:

control  participant reacts as SELF ................. INSERTED
attack A participant forges a TEAMMATE's reaction ... BLOCKED 42501
attack B non-participant reacts to the message ...... BLOCKED 42501
attack C non-participant reads the reactions ........ 0 rows
table row count afterwards .......................... 0

golf_message_responses and golf_message_mentions follow the same policy shape but were not exercised this way. Worth doing before shipping.

Local gates green at every commit: lint 0, 1561 test files / 15,200 tests, knowledge 0, docs 0.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WUAKe2pAbPcQ6cH1s4D3P9

njrini99-code and others added 9 commits September 4, 2026 10:18
…ts in a row, and an auth hop nobody needed

Two causes, and the visible one was not the one I kept assuming.

**The rail's loading state did not look like the rail.** Its `loading` branch
hardcoded `padding="md"` and the "Conversations" bezel regardless of viewport,
while the loaded rail is flat and header-less below `md` and leads with a search
field. So entering Messages on a phone drew THREE different layouts in sequence:
the route skeleton (flat, search, rows), then this bezel card with no search,
then the real rail (flat, search, rows). Two full visible reconstructions on the
way to a screen whose shape was known the entire time.

That is the "it hot loads when you click Messages" report, and it is a mismatch
between three files rather than a rendering fault. The branch now uses the same
`isDesktop` gating as the loaded rail and reserves the search field's height,
so all three states are geometrically identical and the transitions are
invisible.

A loading state that does not match the thing it stands in for is worse than
none: it manufactures precisely the layout jump it exists to prevent.

**And the fetch could not start until an auth round trip finished.**
`useGolfConversations` called `auth.getUser()` on mount and gated everything on
the result — so the tab paid a full network hop of skeleton before its first
query was even sent, on top of the route skeleton Next had already shown.

The page already knew who the user was: `useGolfUser()` carries a
server-resolved id from the dashboard layout, available synchronously on the
first client render. It is now passed in, and the auth call is skipped entirely
when we have it. The fallback remains for callers with no context, so nothing
else changes behaviour.

Deliberately NOT the full server-render of the conversation list. That is a
~400-line refactor across the RPC, active-team scoping, a team-chat merge and
batched metadata, and getting the scoping wrong would show multi-team coaches
the wrong conversations. It remains the right end state; this removes the two
visible reconstructions and one round trip without betting the rail's
correctness on a rushed rewrite.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WUAKe2pAbPcQ6cH1s4D3P9
…und, and add reactions

Four changes, and the first is the one that made every previous "redesign"
invisible.

**The bubbles were darker than the surface they sat on.** The thread well was
`bg-surface` — oklch(0.984), the single LIGHTEST colour in the system — and the
incoming bubbles were `bg-surface-sunken` at 0.963. Measured in a browser, that
is a 0.021 lightness step in the wrong direction: every message was pressed
INTO the page rather than resting on it. No amount of radius or spacing work
was ever going to fix that, which is why four rounds of it changed nothing
anyone could see.

`design-tokens.css` already had the answer written down, at --fw-color-canvas:
"cards now clearly LIFT off it … the ~0.03 gap is the premium card/page
separation the flat-cream look lacked." A conversation is a stack of cards. The
well is now `canvas` (0.953) and incoming bubbles are `surface` (0.984) with
`shadow-soft` — a 0.031 step, in the right direction. Verified by computed
style in a headless render, not by eye.

**Metadata was louder than the messages.** Body text was `text-body-sm` = 13px
against a spec asking for 15-16. Every group carried a timestamp in Fragment
Mono at 0.06em tracking, which is why "9:52 PM" read like a build log. Day
separators were all-caps at 0.1em — the exact "all-caps microcopy with huge
letter spacing" the spec lists under things to reduce. And "Read" was stamped
under every outgoing group, so one screen showed it three times.

Now: 15px body, timestamps in the sans face at caption size, sentence-case day
markers, and exactly ONE read receipt — under the newest thing you sent, which
is all the information there ever was. Radius drops 28px -> 20px, because at 28
a two-letter reply has a radius bigger than its own half-height and renders as
a lozenge.

**Reactions (spec §18).** New `golf_message_reactions`, one row per
(message, user, emoji), with the unique constraint holding the toggle invariant
in the DATABASE rather than trusting the client not to double-insert. RLS
follows the message: readable and writable exactly when its conversation is,
via a new SECURITY DEFINER `golf_conversation_has_me` — which covers DMs, where
the existing `golf_conversation_on_my_team` cannot help because it requires a
team_id. Insert additionally pins `user_id = auth.uid()`, without which a
participant could forge a teammate's reaction. anon has no grant on either.

The emoji vocabulary is CLOSED (👍 ❤️ 😂 👀 ✅) and validated server-side. An
open set would make this a client-controlled text column on a shared production
table; the DB caps its length, the action caps its vocabulary.

Long-press now works on ANY message, not just your own. Reacting to what
someone ELSE said is the entire point, and gating the gesture on ownership put
the feature exactly where it could not be reached.

**No more email per message**, by owner instruction. Email was the wrong
channel and the loudest one we had: a ten-message thread mailed every
participant ten times. Push and the in-app bell already carry the same payload
to the app where a reply can actually happen. Deleted rather than flag-gated —
a dormant flag on a fanout path is a thing that gets switched back on by
accident.

Migration applied to production and verified before this commit: RLS on, three
policies, anon privileges NONE, helper granted to authenticated only, table in
the realtime publication.

One test changed rather than deleted. `keyboard-inset` pinned the literal
string `[.keyboard-open_&]:pb-4` and failed on a composer that still had the
exact behaviour the test is named for — only the number moved. It now asserts
the invariant (resting pad reserves the home indicator; the keyboard-open
override must NOT re-add the safe-area inset), so it still fails if the
override is removed, and no longer fails on a spacing change.

NOT done, and not pretended: "Unknown User" in the header is participant
resolution returning no profile for that QA thread, not styling. Reply/quote
and mentions remain open.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WUAKe2pAbPcQ6cH1s4D3P9
…en is the conversation

The previous commit changed colour, radius and type. Those are ingredients. This
changes the composition.

**The thread was a panel laid on a page; now it is the page.** Below `md` the
InstrumentPanel drops its card tone and its `min-h-[40vh]` floor, so the
conversation owns the viewport instead of floating on one. The elevation and
the two-pane framing survive from `md` up, where they are actually true.

**A short thread now sits at the bottom, where a conversation lives.** It used
to hug the top of the well with several hundred pixels of empty ground beneath
it — the newest message as far from the composer as the layout could put it.
`mt-auto` on the content inside a flex column fixes it with no positioning
tricks and no scroll maths: it pushes a short thread down and does exactly
nothing once the content overflows, so long threads, prepend and anchor
restoration are untouched.

**The date landmark stopped being the loudest thing on screen.** Two hairlines
spanning the full width made "TODAY" a stronger horizontal element than any
message in the thread. It is a centred label now. The unread landmark keeps its
rules deliberately — that one is meant to be found, and the contrast between
them is what makes it findable.

**"Unknown User" is gone.** It is a DEBUG string and it was rendering as
ordinary UI for a real DM. The participant is built from a coach/player
lookup, so an unresolved one is not unknown: they have no current roster row —
they left, or the account is gone. It says "Former team member" now, in the
thread header and the conversation list. Truthful beats a placeholder that
reads like a fault. (The underlying resolution is unchanged; this fixes what
the user is told, which is what was actually wrong.)

**Composer.** 20px radius rather than 28 — it is a field, not a sheet. ONE
focus treatment: it used to draw a colour-shifted border AND a 2px ring around
a field already inside a bordered track, which is three concentric rounded
rectangles. And the send button's hit target and its visible control are now
different sizes — 44px target, 36px circle — where before they were one 44px
tile, which is why send read as a slab instead of a button.

Message text 15px -> 17px (spec §10 asks 16-17), with `overflow-wrap: anywhere`
so a pasted URL cannot push the thread sideways.

Three tests changed, none weakened:
- the action-count tripwire moves 447 -> 449 for the two real new reaction
  actions, annotated like every prior entry in that ledger
- the fanout test's email assertion is INVERTED rather than deleted, so
  re-adding a per-message email fails there instead of shipping quietly
- the scroll test stubs the reactions hook, because that file tests scroll
  position and reactions drag a Supabase client into a jsdom environment that
  has none. The seam is at the feature boundary, not in Supabase's constructor.

Verified by headless render against the real tokens, not by eye: a short thread
now resolves at the bottom of the region rather than the top.

Still open from the spec, and not claimed: reply/quote (§30), mentions,
conversation details sheet (§37), swipe-to-reply (§28), the navigation push
transition (§20), structured Helm objects (§41). Physical-device keyboard work
(§25/§26) needs an iPhone and cannot be verified here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WUAKe2pAbPcQ6cH1s4D3P9
…cator was reserved twice

§7 of the spec, and it was a double-count, not a padding value.

The open-thread surface set its height to
`100dvh - env(safe-area-inset-bottom) - …`, so its box already ended above the
home indicator. The composer inside it then added
`pb-[calc(0.5rem + env(safe-area-inset-bottom))]`. Both reserved the same
~34px, and between the composer's bottom edge and the physical screen edge sat
a strip of the PAGE's colour, not the composer's — which is exactly the
"60-120px of unexplained space" the spec describes, and why it read as a band
rather than as padding.

The inset is now applied exactly once, by the composer, because that is the
half that should own it: a bottom bar's background should run to the physical
edge and pad its CONTENT above the indicator. The surface takes the full
viewport and gives back only the keyboard.

The keyboard term simplifies as a consequence. It was
`max(0, keyboard - safe-area-bottom)` — netting against an inset the surface
was itself subtracting. With nothing to net against it is just
`max(0, keyboard)`.

That simplification broke a sibling gate, and the gate was wrong rather than
the change. It counted occurrences of the literal string
`max(0px,calc(var(--keyboard-height,0px)` and expected exactly 2, so it failed
on arithmetic that still satisfies the property it is named for. It now asserts
the property: both mobile CONVERSATION budgets must reference
`--keyboard-height`. The two budgets are allowed to differ; neither is allowed
to ignore the keyboard.

Getting the scoping right took three attempts and each failure is worth
recording, because two of them would have shipped as a passing test:

  - filtering by `4rem-env(safe-area-inset-top` also excluded the LIST branch
  - matching class strings with /'[^']*'/ mis-pairs on the prose apostrophes
    in this file's comments and matched NOTHING — a gate green by being empty,
    which is the exact failure mode `quality-gates.md` exists to name

It now identifies a conversation surface by what follows its height
(`flex-col overflow-hidden`, the scrolling column) rather than by shape or
position, which correctly exempts the no-team EmptyState — centred content,
no composer, no keyboard to yield to.

Falsified before trusting it: reverting the thread surface to a bare
`100dvh` fails the gate with a message naming the surface, and restoring it
passes. A gate that cannot fail is not a gate.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WUAKe2pAbPcQ6cH1s4D3P9
…ge reactions

Shape was verified when the migration was applied (RLS on, three policies, anon
privileges NONE, helper granted to authenticated only, table in the realtime
publication). Shape is not behaviour, so the policies were then EXERCISED on
production in rolled-back transactions.

The control matters as much as the attacks: a policy that denied everything
would block all three attacks and look identical in the log. It inserts, so the
denials below mean the policy discriminates rather than simply refusing.

  control  participant reacts as SELF ................. INSERTED
  attack A participant forges a TEAMMATE's reaction ... BLOCKED 42501
  attack B non-participant reacts to the message ...... BLOCKED 42501
  attack C non-participant reads the reactions ........ 0 rows
  table row count afterwards .......................... 0

Attack A is the `user_id = auth.uid()` conjunct in the INSERT policy. Without
it, any participant could attribute a reaction to anybody else in the thread.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WUAKe2pAbPcQ6cH1s4D3P9
The inbox was a dashboard page with a list on it. Six changes, all §16.

**The giant green "New message" pill is gone.** A solid accent lozenge on its
own row was the largest, highest-contrast object on the Messages screen —
louder than any conversation in the list beneath it — and it spent a full band
of vertical space saying what a 44px glyph says. Compose is a pencil-square
icon button now, team broadcast beside it, both 44px with real `aria-label`s,
so nothing is lost for VoiceOver.

**Every group rendered the same generic two-person glyph.** Three conversations
meant three identical icons and an avatar column carrying no information at
all. A group now gets its own initials the way a person does — "Team Updates"
reads TU, "Demo University Golf" reads DU — on the accent wash that still
separates it from a DM at a glance. 48px, up from 40.

**The preview was `text-eyebrow`: 11px at 0.06em tracking.** That is the type
role built for ALL-CAPS labels, applied to a sentence somebody actually said.
It is 13px normal sans now, in secondary rather than tertiary ink, and the name
above it moved 13px -> 15px so the row finally has a hierarchy instead of two
lines of identical type.

**TODAY / EARLIER** were `font-fw-display` uppercase at 0.14em. A recency
landmark orients; it should not out-shout the conversations under it. Sentence
case, caption size.

**Timestamps** leave Fragment Mono, same as the thread — a 0.06em-tracked
monospace "9:12 AM" is a log line, not a time. They also stop turning accent
green on unread: §17 says avoid six simultaneous green treatments, and the
bolded name plus the count badge already carry it.

**Press physics (§19)** — rows get `active:bg-surface-sunken`, so the
acknowledgement paints on touch-down instead of waiting for navigation.

Search takes the sunken track as a pill rather than a bordered rounded
rectangle, which had it reading as a second card stacked above the rows.

Verified by headless render against the real tokens. Row padding went to `py-3`
in BOTH row components — the conversation row and the search-result row — so
the two lists keep the same rhythm.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WUAKe2pAbPcQ6cH1s4D3P9
…v bar

## Reply / quote (§30)

`golf_messages.reply_to_id`, a self-reference — NOT a join table. A reply IS a
message, so a column keeps every existing read, RLS policy, realtime
subscription and pagination path working untouched; a join table would have
needed policies duplicating the ones golf_messages already has.

`ON DELETE SET NULL`, deliberately not CASCADE. Deleting a message somebody
replied TO must not delete their reply — those are their words, and cascading
would let anyone erase other people's messages by deleting their own. The quote
degrades; the reply survives.

The column confers NO read access. The quoted row is resolved through
golf_messages RLS like any other message, so a pointer into another
conversation renders as "Not in this part of the conversation" rather than
leaking anything. Quotes resolve from the LOADED window instead of fetching:
a per-bubble fetch would open one request per quote for a feature most messages
do not use.

`reply_to_id` rides the optimistic row too, so the quote paints in the same
frame as the bubble rather than appearing a round trip later — the layout jump
the arrival motion exists to prevent.

The pending reply is owned by the PAGE and stamped with its conversation id.
The composer would have been the obvious place, and it is the wrong one: a
reply target surviving a thread switch is the composer-draft misdirection bug
in a new costume. It is also read and cleared BEFORE the send await, because
the composer clears optimistically and a preview left standing during the round
trip points at a message already sent.

## Conversation details (§37)

Tapping the thread identity opened nothing — a dead control, and the one place
a group's membership is obviously "behind". It is a Fairway Sheet now:
identity at a size that actually reads, member roster, and search.

Scope is what EXISTS. §37 also lists shared media, pinned messages and
per-conversation notification settings; none has a backing store, and rendering
them as inert rows would be a menu of things that do not work. The member count
renders only when the roster actually loaded — "0 members" for a group whose
participants are still in flight is a number stating something false.

## The nav bar (§4)

The header was `flex` with back, avatar and name packed left, so the title sat
at whatever x the back label happened to end at — a different position per
conversation — and the right side was empty. It is a three-column grid now:
back, centred identity, overflow. A fixed centre is what makes a bar read as
chrome instead of content that happens to be at the top.

The "Messages" label is gone from the back control. It was defended in this
file as naming the destination, but a chevron out of a conversation has one
meaning, and the label cost enough left-column width to shove the title off
centre. The destination is still named — by `aria-label`, where that belongs.

## Two things that were wasting a whole row each

The mobile action band is GONE. It held a giant green pill, then briefly two
icons; either way a full row of vertical space on the smallest screen, above
the list it was pushing down. Compose and team-broadcast now sit on the rail's
search row: one row that says "find one, or start one".

The composer is an attached BAR, not a bordered box. A hairline alone left it
reading as the last block of the page; an UPWARD shadow is what makes a bottom
bar look like it is in front of the thread scrolling under it.

Migration applied to production and the column verified before this commit.
Verified visually by headless render against the real tokens.

NOT done, and not claimed: mentions (§20) and conversation-list reorder motion
(§15). Reorder motion is skipped on the spec's own instruction — §14 says do
not run layout animation without profiling it, and I cannot profile on a device
from here. Shipping unprofiled layout animation across a live list is the
failure that section exists to prevent.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WUAKe2pAbPcQ6cH1s4D3P9
Four chat-app references, and the thing all four had that Helm did not was a
filter row under the search. Ours is keyed to what Helm actually knows rather
than to a generic All/Favorites: a golf program's inbox is a mix of one-to-one
coaching and team channels, and "just the team channels" is a real thing a
coach wants at 6am on a travel day.

**All · Unread n · Teams**, on the existing Fairway `Segmented` — no new
control, and `size="lg"` for the 44px target.

The count on the Unread chip comes from the same `unread_count` the rows
render, so the chip cannot claim a number the list below it does not show.

**Each option only appears if it would change what you see.** `showFilters`
requires that some, but not all, conversations match — an inbox of three
conversations that are all team channels does not get a "Teams" chip filtering
to the same three. A control that cannot change anything is chrome, and this
row sits directly above the content it would otherwise be pushing down.

**The rows have one right rail now.** The timestamp used to sit on the name's
line and the unread badge on the preview's, which gave every row FOUR alignment
edges and put the badge in direct competition with the message text for the
same horizontal space — a long preview shoved it around. Time over count, one
edge, and the preview gets the full width it needs.

Two things I checked before building rather than after:

- **No presence data exists** anywhere in the messaging hooks or golf types.
  Three of the four references show "Active Now" / online dots, and Helm's
  Avatar even has a `status` prop ready for it. Rendering that without a source
  would be inventing a fact about whether a teammate is at their phone, so it
  is not here.
- **ESLint caught a real bug**, not a style nit: my `visible` list started as a
  `useMemo` placed BELOW the loading/error/empty early returns — a hook called
  conditionally, whose call order genuinely changes between renders. It is a
  plain filter now, which is also what the two lines under it already do over
  the same array.

Also repairs two gates that broke on formatting rather than behaviour, both
made to assert intent instead of a literal:

- `send-integrity` pinned the whole string
  `'const sendMessage = async (content: string)'` and broke when §30 added a
  second parameter — a signature change with nothing to do with the
  retry/client-id invariant it guards. It matches the function, not its
  parameter list.
- `keyboard-inset` required whitespace before the composer's `pb-[calc(...)]`;
  that class moved into a `cn()` call and is now preceded by a quote. The
  class never changed — only what sits to its left.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WUAKe2pAbPcQ6cH1s4D3P9
… that exist

Looking at the four reference inboxes again, the thing I kept missing is what
they DON'T have.

**None of them has a date-section header.** Not one. They order by recency and
let the timestamp column carry it, which is why they read as a single calm
surface. Mine cut a five-row list into three labelled blocks — Unread, Today,
Earlier — so the eye had to re-acquire the rhythm twice on the way down. The
sections are gone, along with `groupConversationsByTime` and `GROUP_ORDER`,
which existed only to label them.

Floating unread to the top went with them. That was answering "show me what I
haven't read", and the Unread CHIP now answers it explicitly and on demand,
instead of permanently reordering the list under the reader. Recency is the one
ordering that never surprises. Unread still reads instantly — semibold name,
primary-ink preview, solid count. Three signals, no section.

**The filter is loose pills, not a Segmented track.** Segmented is this repo's
view-SWITCHER: a bordered sunken well with a sliding pill, sized to fill its
row. Stretched over three options it reads as a squeezed tab bar and draws two
more boxes onto a screen trying to be a list. Every reference uses free-standing
pills, left-aligned, and they're right — a filter is a set of optional
narrowings, not a segmented view of one thing. `SelectablePill shape="round"`
is the primitive that already means this. It scrolls rather than wraps, so a
fourth chip can't silently become a second row.

**The unread badge was a wash.** `tone="accent"` is accent-50 with accent-700
text: at 12px on a cream row it was the quietest thing in the right rail,
competing with a timestamp and losing. It's a solid fill now. It's the one
element on this screen that should be loud.

**Half the avatar column was invisible**, and it was the bubble defect again:
the shared Avatar falls back to `bg-surface-sunken` (0.963) and this row sits on
canvas (0.953) — a one percent difference. Overridden at the call site, not in
the primitive, because every other Avatar in the app sits on a 0.984 card where
the default is correct. DMs get a lifted cream disc with primary ink; groups get
accent-100/800 rather than 50/700 — an initials circle has to earn from tone the
presence a photograph gets for free.

Inset hairlines between rows, positioned rather than bordered so they start at
the text column and never cut through an avatar (§47).

No UI kit installed. It would not have fixed any of the above — every one of
these was a value/contrast or information-architecture decision, not a missing
component — and it would have brought a second visual vocabulary onto a screen
whose problem was already too many competing shapes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WUAKe2pAbPcQ6cH1s4D3P9
@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

@vercel

vercel Bot commented Sep 4, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
helmv3 Ignored Ignored Preview Sep 5, 2026 3:23am UTC

Request Review

@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 50fcde8b-05ee-4ae7-ae5c-fbb3630ea2c4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@supabase

supabase Bot commented Sep 4, 2026

Copy link
Copy Markdown

Updates to Preview Branch (agent/messages-instant-entry) ↗︎

Deployments Status Updated
Database Sat, 05 Sep 2026 03:45:48 UTC
Services Sat, 05 Sep 2026 03:45:48 UTC
APIs Sat, 05 Sep 2026 03:45:48 UTC

Tasks are run on every commit but only new migration files are pushed.
Close and reopen this PR if you want to apply changes from existing seed or migration files.

Tasks Status Updated
Configurations Sat, 05 Sep 2026 03:45:48 UTC
Migrations Sat, 05 Sep 2026 03:45:48 UTC
Seeding Sat, 05 Sep 2026 03:45:48 UTC
Edge Functions ⚠️ Sat, 05 Sep 2026 03:45:48 UTC

⚠️ Warning — Only Functions declared in config.toml will be automatically deployed to branches: [functions.my-slug]


View logs for this Workflow Run ↗︎.
Learn more about Supabase for Git ↗︎.

njrini99-code and others added 17 commits September 4, 2026 11:53
…tion, real faces

## The composer was a form

`setSending(true)` -> `await onSend(...)` -> *then* `setMessage('')`. The text a
player had already committed sat in the field for the ENTIRE round trip while
the send button ran a bouncing-dot loader. That is form-submission feedback, and
it is the single reason sending felt like a web app: the user had said the
thing, and the interface was still holding it.

It clears in the same tick as the commit now, with `fwHaptic('light')`, and the
field collapses to one line in the same frame — a multi-line send used to leave
a hole where the text had been. The `sending` state is gone entirely; the
optimistic bubble is the feedback, and sending/sent/read/failed belong to it.

`canSend` is deliberately no longer gated on an in-flight send: a gate makes two
quick messages impossible, and clearing on commit already prevents a
double-tap from double-sending.

Failure does not eat the draft. The text goes back — but ONLY if the composer is
still empty, because by then the user may be typing the next thought, and
overwriting that is worse than losing it. Same rule for pending attachments.

## Chat motion is not control motion

`src/lib/golf/chat-motion.ts`. The shared Fairway control transition is a
deliberately slow ~180ms cinematic curve — right for a segmented control, wrong
for a bubble. A message arriving on it reads as the list re-laying out.

  bubble arrival   spring 420/34/0.75, y 8->0, scale .985->1   (was a flat 200ms tween)
  Sent -> Read     140ms crossfade
  reaction chip    120ms, scale .92->1, y -2->0
  long-press       bubble lifts 1.015 / -1px BEFORE the menu
  row reorder      layout="position", reorder spring

One module because the alternative is the same gesture at 140ms in one file and
220ms in another, which reads as inconsistency long before anyone can name it.

The read receipt is the subtle one: `Sent` and `Read` are different widths, so
swapping them shifted the timestamp beside it. Both states now occupy one
`inline-grid` cell, which reserves the wider label — opacity changes, geometry
does not.

Row reorder is `layout="position"`, not `layout`. Full layout animation scales
the row's box, which distorts text and remounts the avatar image mid-flight —
the "mushy" result the spec warns about, and a guaranteed photo flash.

## The row is not a button

`PressTarget`, not `Button`. The generic Button was being fought with eleven
overrides — `h-auto min-h-0 border-0 font-normal justify-start text-left` — to
stop it being a green capsule, and it brought a coupled click haptic and that
same 180ms curve. PressTarget is unstyled on purpose: real button semantics,
focus and disabled handling, no opinion about shape.

## Real faces

I said AvatarGroup was blocked on data. It was not — the photos were in the app
the whole time, and the resolver already existed. It was just SCOPED to the one
open conversation, so the inbox never asked.

`useGolfGroupAvatars` batches that same resolution across the whole list: three
queries, never one per row, because a per-conversation fetch is an N+1 on the
first screen a coach sees every morning. Participants paginate at 1000 —
PostgREST truncates silently rather than erroring, and a program with many team
channels crosses that. Identities resolve once for the UNION of user ids, since
the same coach is in most of them. Members with a photo sort first, because the
stack shows three and a real face beats initials in a slot that holds one.

Fails soft and silent-empty: any error returns an empty map and rows keep their
initials. A missing face is cosmetic; a throw takes the inbox with it. A group
with one resolved member stays a plain avatar — a stack of one is an avatar
wearing a ring it does not need.

## Also

The reactions migration's `COMMENT` prose tripped the Review Gate's
SECURITY-DEFINER rule, which scans for the phrase and could not see that the
function pins `SET search_path` eleven lines above. Reworded, not suppressed —
a `nosemgrep` there would silence the real case for whoever adds the next
definer function to that file.

Not built, and not claimed: `layoutId` avatar continuity from inbox to thread
(§35) needs both trees alive during the transition, which this master-detail
does not guarantee; NumberFlow unread counts; mentions; slash actions.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WUAKe2pAbPcQ6cH1s4D3P9
…issing messages

## The worst behaviour in the feature

On a failed send the hook ran `setMessages(prev => prev.filter(...))`. The
player watched their words appear and then VANISH, with a toast as the only
trace of what they had written. That is the single most destructive thing this
surface did, and it did it quietly.

The message stays now. It dims, and its metadata line reads
`Not delivered · Retry` (§29/§30). Retry re-sends under the message's ORIGINAL
id — `golf_messages.id` is the primary key, so a retry that races a send which
actually committed collides on 23505 and the server reports it back as the
success it is. Pressing Retry twice cannot post twice. Same property the
transport retry already relied on.

The bubble softens rather than turning red: an undelivered message is not an
error, it just has not landed. Colour is never the only channel either — the
words are directly beneath it.

The composer no longer restores the draft on failure, because that would put
the same text in two places and the copy in the thread is the one being looked
at. Attachments are the exception: they cannot ride an optimistic bubble, so a
failed photo does come back to the composer.

## You stop silently missing messages

Arrivals while the reader is scrolled up were being counted by nobody — the
thread simply did not move (correct) and gave no indication anything had
happened (not correct). A floating `N new messages` control now appears above
the composer.

Only messages from SOMEBODY ELSE count. Your own send scrolls you down by
design, and counting it would offer to take you to your own message. Reaching
the bottom yourself clears it, because scrolling down IS catching up and a
button offering to take you where you already are is noise. It floats rather
than occupying a row: a permanent slot would cost every conversation height for
a state most never enter.

## Two small ones

Corners settle when a burst continues (§22). Worth recording WHY this is a CSS
transition and not Motion: last -> middle is a `border-radius` change, and
layout animation only interpolates transforms — it cannot smooth this at all.
A transition on the exact property is both correct and far cheaper than putting
every bubble under layout animation to chase it.

Unread counts roll 1 -> 2 with NumberFlow instead of swapping (§52), capped at
9 because past that the badge reads "9+" and there is no number to animate.

## One I caught before it shipped

I wrote `text-danger-600` for the failure text. Typecheck passed — Tailwind
class names are strings, and nothing in the type system knows that token does
not exist. It would have shipped as default-coloured text on the one line whose
whole job is to look wrong. The real token is `fw-danger-ink`.

## Not done, and it is a data limit

§74's group typing copy ("Owen and Cole are typing"). The typing broadcast
carries NO sender identity — the hook exposes a boolean. Naming somebody would
be a fabricated attribution in exactly the surface where getting it wrong
matters. It needs the broadcast payload extended first.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WUAKe2pAbPcQ6cH1s4D3P9
…at the auto-fixer broke

The Review Gate's `sqlfluff (ratchet)` caught 60 new violations in my two
migration files (LT02 indent +41, LT01 spacing +11, LT05 long lines +7,
CP02 capitalization +1). The ratchet only permits the count to go DOWN, which
is correct: raising a baseline converts a caught regression into permanent debt.

`sqlfluff fix` resolved 62 of them. It also SILENTLY BROKE THE SQL.

Rule RF03 ("unqualified reference in single-table select") rewrote

    WHERE m.id = message_id

to

    WHERE m.id = m.message_id

in both the SELECT and INSERT policies. But `message_id` there is
`golf_message_reactions.message_id` — the OUTER row the policy is evaluating —
not a column of `golf_messages m`. `golf_messages` has no such column. The
subquery was reaching out of its own scope by design, and the fixer read it as
a mistake.

    ERROR: 42703: column m.message_id does not exist

That is a formatter changing the meaning of a SECURITY PREDICATE. Every clean
rebuild would have failed at this file, and the only reason it did not ship is
that I executed the rewritten SQL rather than trusting the linter's exit code.
Both references are now qualified explicitly as
`golf_message_reactions.message_id`, which satisfies RF03 and says what was
always meant.

The fixer also lowercased `FROM PUBLIC` to `from public`. That one IS safe —
unquoted `public` in a GRANT/REVOKE is the PUBLIC pseudo-role, not the schema —
and it is verified below rather than assumed.

VERIFIED, not linted-and-hoped:
  - both files execute against production in a rolled-back transaction (201)
  - `golf_conversation_has_me` still shows EXECUTE for authenticated and
    service_role only, no PUBLIC, no anon
  - sqlfluff: 0 violations across both files
  - `npm run sql:ratchet`: OK — 7666 violations, no regressions

The remaining 3 long lines were wrapped by hand rather than left for the fixer.

Also recorded both migrations in `supabase_migrations.schema_migrations`. They
were applied through the Management API, which executes SQL directly and does
NOT write the ledger — so production had the table, column, policies and
indexes while the ledger showed nothing. Applied-but-unrecorded drift: a future
`db push` would have re-run them (harmless, they are idempotent) and drift
tooling would have reported a gap that was not real.

SEPARATE, PRE-EXISTING, NOT FIXED HERE: `Supabase Preview` fails on
20260903220000_feature_health_heartbeat_indexes.sql (landed on main yesterday
in #1816) with "CREATE INDEX CONCURRENTLY cannot be executed within a pipeline"
— the CLI wraps each migration in a transaction. The index exists in production,
so it arrived by some other path, but a clean rebuild from migrations dies
there. That breaks `supabase db reset` and recovery-from-migrations for
everyone, and it means the preview validator cannot verify anyone's DDL. It is
not messaging and not mine to decide: dropping CONCURRENTLY takes an ACCESS
EXCLUSIVE lock on that table.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WUAKe2pAbPcQ6cH1s4D3P9
Caught by rendering it, not by any gate — this is pure geometry, and neither
typecheck nor lint has an opinion about whether a flex child fits its parent.

A conversation row's avatar column is 48px, and the text beside it starts at a
fixed x. The group stack has to live inside that or half the list is misaligned.

Three attempts, and the two failures are worth recording because both looked
reasonable in code:

  three `md` (40px) faces + AvatarGroup's "+N" chip  ~130px  hung out of the row
  two `sm` (32px) faces at a 16px overlap             48px   back face reduced to
                                                             a sliver of half a
                                                             letter — reads as a
                                                             rendering bug, not
                                                             as layering
  two `xs` (24px) faces at the default 6px overlap    42px   fits, both legible

The "+N" chip is gone with `max`: it is another full-size element, so including
it puts the stack straight back outside the column. Nothing is lost — the row
already names the conversation and the details sheet lists every member.

MAX_FACES drops 3 -> 2 in the hook for the same reason, so it stops fetching
members that could never be rendered.

Gates: lint 0, 1561 test files / 15200 tests pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WUAKe2pAbPcQ6cH1s4D3P9
…ocked

## I was wrong about "blocked on data"

I reported group typing copy as blocked because the broadcast carried no sender
identity. It does. The handler destructured `userId` and then threw it away to
set a boolean. Nothing needed a backend change; I had read my own code as a
wall.

It tracks WHO now, with ONE EXPIRY TIMER PER TYPIST. The single shared timer was
a real bug: a second person starting to type reset the first person's countdown,
so in a busy group somebody could show as typing indefinitely after they had
stopped. Names resolve locally from the participant map — never from anything a
sender puts on the wire — and an unresolved id is DROPPED rather than rendered
as "Someone", because a line naming the wrong count is worse than plain dots.
Two names, then a count, so five typists cannot grow the line and push the
thread.

`isOtherTyping` is now DERIVED from that list rather than stored beside it. Two
sources for "is anyone typing" is two things that can disagree, and the old
boolean could stay true after the last typist was removed.

## Visual finish (§13-23)

- **Bubble elevation cut by roughly two thirds**, and outgoing has NONE.
  `shadow-soft` is the raised-CARD elevation; at that weight a thread reads as
  a stack of floating Surfaces. Colour already separates cream-on-champagne and
  green-on-champagne, so elevation only has to hint. A saturated green fill is
  already the highest contrast step on screen — elevation on top of it is the
  green-competition problem in §31.
- **Incoming avatar in DMs**, 28px, scale-only entrance. I had removed it to
  save 40px of width, arguing the header already names the person. That was an
  argument about information; §12's is about PRESENCE, and it is the better
  one — a thread with a face in it feels like somebody is there.
- **Reply quote** drops to medium weight and lower contrast. At 17px response
  over 12px quote, the response has to be what the eye lands on.
- **`Read · 7:43 PM`** as one tertiary line instead of an accent chip beside the
  time, which had made a routine status the second-greenest thing in the thread.
- **`⚠ Not delivered · Tap to retry`** as one PressTarget. Two controls side by
  side read as web links, and the whole metadata region is now a much bigger
  target than the word "Retry" was.
- **New-messages button is chrome**, not a primary CTA — it is a way back to the
  bottom, not an action, and a solid green pill competed with the outgoing
  bubbles it sits among.
- **Filters 32px and shadowless**; dividers at half opacity. Together these were
  putting the CONTROLS above the PEOPLE in the hierarchy (§3) — the eye reached
  "All / Unread / Teams" before it reached a single name.

## Two real bugs lint caught

An arbitrary `text-[11px]` (the token rule exists for exactly this), and
reading `ref.current` inside an effect cleanup — which can see a different
object than the effect installed timers into, clear the wrong map, and silently
leak the real ones. Both captured as locals now.

Gates: lint 0, 1561 files / 15200 tests, knowledge 0, docs 0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WUAKe2pAbPcQ6cH1s4D3P9
… poll, travel

The P2 differentiator. A message stops being only text.

## Schema

One migration, because these are one feature and splitting them would create an
ordering dependency between files that all touch golf_messages.

  golf_messages.kind + payload    text | system | practice | event | rsvp | poll | travel
  golf_message_responses          one row per person per message
  golf_message_mentions           stored, not re-parsed
  golf_messages.pinned_at/_by
  golf_conversation_participants.notification_level / muted_until

**A structured message IS a message.** It rides `golf_messages`, so every
existing read, RLS policy, realtime subscription, reply, reaction, search and
pagination path works on it unchanged. A parallel "team objects" table would
have needed its own copy of all of that, and would have put half a conversation
outside the conversation.

The constraints EARN their place, and I checked rather than assumed:
`kind='rsvp'` with a null payload is rejected with 23514 on production. Without
that pair of CHECKs a client could post a structured kind with nothing in it and
every reader would render an empty card.

`golf_message_responses` is unique on (message_id, user_id), which is what makes
changing your mind an UPDATE rather than a second vote — held by the database,
not by client discipline. Its INSERT and UPDATE policies both pin
`user_id = auth.uid()`; without that a participant could answer a poll as
somebody else.

Mentions are STORED rather than parsed from content at delivery time, because
"mentions only" has to be answerable by a query, not by running a regex over
every message. Insert is restricted to the message's own author — otherwise
anyone could write mention rows against a message they did not send and
manufacture a notification.

`notification_level` lives on the PARTICIPANT, not the conversation: muting is a
property of your membership, and a coach muting a thread must not mute it for
the team. `muted_until` expires on READ rather than by a job, because a cron
that has not run yet leaves somebody silently muted — the failure you never find
out about.

## Rendering

`parseStructuredPayload` narrows an untrusted jsonb blob and returns NULL rather
than throwing. Postgres will not check the shape of a payload, so a malformed
one has to degrade to an ordinary message — a throw inside a message list
unmounts the conversation. Every branch requires the fields the renderer
actually reads, so a half-written payload cannot produce a card with blank
headings.

A `Surface` is right here and deliberately wrong for ordinary bubbles: a
structured message represents an object with state and actions, which is what a
card means. A sentence somebody typed is not.

System messages are narration, not speech — centred, quiet, no bubble, no
author, no avatar (§40).

Times are stored as ISO instants and formatted in the READER's timezone. A coach
in Eastern posting "3:30" to a player who has travelled is the classic version
of that bug.

"View in Calendar" renders only when the payload actually carries an eventId. A
button that opens nothing is worse than no button.

Poll share is guarded against 0/0 — a NaN width silently renders a full bar.

## One caught by rendering it

The unpicked poll bar was `bg-surface` on a `bg-surface-sunken` track: 0.984
against 0.963, a two percent difference, which renders as no bar at all. Same
defect as the message bubbles and the inbox avatars, third surface. Both states
use the accent ramp now.

Two tests updated: the action tripwire 449 -> 451 for the two new actions, and
the scroll test stubs the responses hook — that file tests scroll position, and
structured answers drag a Supabase client into a jsdom environment with none.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WUAKe2pAbPcQ6cH1s4D3P9
…hidden it

`Button` wraps ALL of its children in a single <span>. Passing a text node and
an <svg> as children therefore makes them one inline flow, so `justify-between`
had exactly one thing to distribute and the arrow wrapped onto a second line.
It goes through the component's own `rightIcon` slot now.

The reason this shipped past three green gate runs is worth recording for
whoever picks this up. Every screenshot in this branch until now was
HAND-WRITTEN HTML approximating the components, not the components. My version
had drawn the label and the arrow as two flex items, so the button looked
correct in every render I produced — and the emoji I used as icon stand-ins are
what prompted the owner to ask why the icons looked wrong.

An approximation that drifts from the code is the same failure class as a gate
that passes by being empty: it reports success about something it never
examined.

Screenshots are now produced by rendering the ACTUAL component through
react-dom/server with Tailwind compiled against the real source files. The first
run of that found this bug immediately.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WUAKe2pAbPcQ6cH1s4D3P9
@njrini99-code

Copy link
Copy Markdown
Owner Author

Latest messaging material + identity pass

Commit e3aec2315 adds the final live-UI refinement pass:

  • confines chat scroll to the thread pane, so opening/searching a conversation does not scroll the dashboard document
  • gives the thread header, bubbles, reactions, and composer restrained Fairway material depth without card-wrapping the screen
  • defaults to a resolved team/person thread instead of an unresolved historical DM
  • resolves participant photos from roster records and existing avatars/<user-id>/… uploads; Messaging no longer substitutes synthetic initial avatars when no photo exists
  • keeps the mobile surface immersive by hiding the shell sub-navigation while a conversation is open

Verified locally: focused messaging tests (23/23), targeted ESLint, git diff --check, and npm run build (TypeScript + 179 routes). No new migrations and no production writes in this commit.

njrini99-code added a commit that referenced this pull request Sep 4, 2026
`open-pr-residue` requires the key set here to equal the live open-PR set, so
opening a PR without a row makes the check fail — but a row for THIS PR cannot
be on main while the PR is in flight. The file's own schema resolves that: a
row absent from main but present at the PR's own head is IN FLIGHT, not
unclassified. This is that row, at that head.

HUMAN_TEST_PENDING rather than ACTIVE, and the distinction is doing work: the
change is finished and green, and what it is waiting for is a thumb. A 450ms
hold with 10px of slop cannot be verified by a test that synthesises its own
pointer events. It is also waiting on a merge-order call against #1833.

worktree_policy PARK_IF_REPRODUCIBLE releases the checkout — it exists only to
have cherry-picked and pushed this branch, and parking keeps the branch. Both
gates have to permit, so the workspace marker is being released in the same
step; neither one implies the other.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019euu9jtJM6WvMVj17coqZ1
njrini99-code added a commit that referenced this pull request Sep 5, 2026
…e has ended

`open-pr-residue` requires the key set to equal the open-PR set, and a row for
an in-flight PR lives at that PR's own head rather than on main — so #1836's
row rides its own branch. Verified both directions: without it the check names
#1836 as unclassified; with it, it does not.

Also deletes #1725 and #1738, both MERGED, both past the transitional grace
this file's own `$comment` defines — the tool's output says in as many words to
delete them "in the PR you are already opening", and that is this one. Leaving
them is the exact failure the comment describes: a current-state registry
asserting things that stopped being true.

**`open-pr-residue` still FAILS, for reasons this PR does not own.** Twelve
open PRs carry no row at all — #1834, #1833, #1832, #1831, #1829, #1827, #1759
and five dependabot PRs. That was already true before this commit and is not
mine to reconcile; a row is a statement about someone else's work. Recording it
here so the red is legible rather than mysterious.

Note for merge order: #1835 also edits this file, so whichever lands second
resolves that conflict.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019euu9jtJM6WvMVj17coqZ1
njrini99-code and others added 2 commits September 4, 2026 23:22
…ale assertions

Every failing step reported `conclusion: success` in the UI, because each check
runs `continue-on-error: true` and the job's aggregate reads `outcome`. So the
PR looked like it had no failing steps and a mysteriously failing aggregate.
The aggregate's own log names them: `FAILED ratchet_lint` and
`FAILED world_model`.

**ratchet_lint — a real defect, fixed rather than baselined.**
`@typescript-eslint/no-unused-vars` went 25 -> 26. `npm run lint` never saw it:
that lints `src/**`, and the ratchet lints `src` AND `scripts`. The variable is
`viteOutput` in the QA capture harness, which accumulated every line Vite wrote
to stdout and stderr and then dropped them — while the `finally` below it
asserted the run "has already surfaced any Vite startup or browser failure
above". It had not: a config or port failure surfaced as a bare
`waitForServer` timeout with the actual reason discarded. Now the catch prints
it. The variable is used because it should always have been, not renamed `_`.

**world_model — a stale generated artifact.** `WORLD_MODEL.json`/`.md` no longer
matched their sources after this branch's registry and code changes. Regenerated.

**The five unit failures were assertions pinning markup, not contracts.**
Every underlying contract this branch was accused of breaking still holds:

    12-col proportional grid        held  (gap-6 -> gap-0 + md:border-y)
    rail spans 5/12 md, 4/12 lg     held  (md:border-r inserted between them)
    thread spans 7/12 md, 8/12 lg   held
    720px readable cap              held
    masthead never on phone height  held MORE strongly — ViewHeader is gone
    "official team channel" naming  held  — moved into an aria-label

Each assertion matched a whole class string or one capitalisation, so additive
styling and an icon-ified entry point read as regressions. They now pin the
tokens a real regression would remove: `grid-cols-12` with `md:grid`, each span
pair with anything permitted between them, no `<ViewHeader>` at all, and the
channel named in either register. A flexbox rewrite or an unhidden masthead
still fails them.

Verified locally at this head: lint 0, lint:ratchet 0 (62 warnings, no
regressions), full unit suite 1566 files / 15230 passed, docs:check 0,
knowledge:check 0, world-model check 0.

NOT addressed here: `Snapshot Testing` (a Sentry preprod check, external to
this repo's gates), and the production-schema-ahead-of-main reconciliation this
PR's description documents — that is what merging it does, and it is the
owner's call, not a CI fix.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019euu9jtJM6WvMVj17coqZ1
…te kept

#1827 ("honour the Messages notification toggles") landed on main while this
branch was open, and both changes answer the SAME report: a player sent 33
notifications in a day whose toggle was read by nothing.

They answer it differently, and the conflict is semantic rather than textual:

    #1827   gate email behind `email_messages`, a preference defaulting ON
    #1833   remove the email channel, by owner instruction — "stop sending
            emails every time there's a message. It should just be the app
            notification."

Removal wins: it is the stronger answer to the same complaint and the owner
asked for it in those words. But taking either side whole breaks the file —
`prefsFor` is DEFINED inside the region #1833 deletes and USED below it by the
push gate, which is #1827's durable half. So the resolution keeps the removal
AND the gate: no email at all, push still honouring `push_messages`, the in-app
bell still ungated because it is how the message is discovered at all.

Three of #1827's tests asserted email IS sent. Each was re-pointed at the
contract underneath it rather than deleted:

  - "emails every recipient even though RLS returns none" was never about
    email; it pins the ADMIN FALLBACK resolving a recipient RLS cannot see.
    Now asserted on the bell.
  - "does NOT email when email_messages is off" would still pass if the channel
    returned for everyone who never touched the toggle — `email_messages: true`
    is the DEFAULT. Re-pinned with the preference ON, which actually catches
    the regression.
  - quiet-exemption still matters; it now asserts on push, with
    `push_messages` set explicitly because its documented default is OFF and
    the test would otherwise have passed for the wrong reason.

Verified on the merge: typecheck, lint --max-warnings 0, lint:ratchet 0,
world-model regenerated and matching, full unit suite 1568 files / 15252
passed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019euu9jtJM6WvMVj17coqZ1
njrini99-code added a commit that referenced this pull request Sep 5, 2026
…wasn't the sheet

Two defects the approved-design pass left standing. Both are invisible to
every check we had, which is why they survived a screenshot review.

1. Long-press cancelled on the FIRST pointermove. That reads as correct —
   they moved, so they meant to scroll — and it passes in jsdom, which emits
   no spurious moves, and in a screenshot, which cannot photograph a gesture.
   On a real digitizer a resting finger emits a continuous dribble of
   sub-pixel moves for the whole press, so the deliberate stationary hold the
   feature exists for was the one gesture guaranteed to be cancelled. Now a
   DISTANCE threshold: 10px, the platform convention on both sides (Android
   getScaledTouchSlop() ~= 8dp, iOS allowableMovement = 10pt), extracted as
   exceedsLongPressSlop so a test pins the number rather than the prose.
   A zero threshold fails MessageThreadPane.longPress.test.ts.

2. The actions opened as an inline icon-only row at the MESSAGE's position.
   The approved design is a bottom sheet — grabber, dimmed thread, labelled
   Copy / Edit / Delete with Delete below a divider — and the difference is
   not decorative: an inline row beside a message near the top of the screen
   is out of thumb reach, and four unlabelled icons make Delete a guess.
   Now the Fairway Sheet (vaul), which also supplies the scrim, focus trap,
   Escape and drag-to-dismiss that were hand-rolled listeners before, so this
   deletes more code than it adds. One sheet per thread resolved from
   mobileActionsId, not one per bubble; a stale id resolves to null and
   closes rather than acting on a row that no longer exists.

Reply is deliberately absent here — it needs the reply_to column that lives
on #1833, and adding a fourth row now would collide with that branch in
exactly this region.

Verified: tsc --noEmit clean; eslint --max-warnings 0 clean on both changed
files (the first draft used a raw <button> and h-13, a class this Tailwind
config does not define — both caught and fixed, not shipped); 83 tests across
15 files pass over messages + overlays; markdown ratchet reports no
regressions. Not verified locally: the gesture itself on hardware, which is
the thing item 1 is about.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019euu9jtJM6WvMVj17coqZ1
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