fix(ui): work the mail-surface audit, one nit at a time - #186
Merged
Conversation
…ssal The accent left edge asked for border-l-solid, which is not a real utility, so the edge rendered dashed and the offer read as a dropzone. The dismissal was bare text; it now carries a resting underline.
The inbox panel is a card on the paper; the reader was loose content. The reader now gets the same gutter and card shell, and the summary and message cards move up one rung to surface-float.
A one-message thread drops the meta line; the sender and time already show in the card below. The compact hand-off button keeps a verb. Trash leaves the strip that holds Close, and the three icon groups share one segmented vocabulary.
The message header glyph now reads as addressing instead of a forward. The sender name joins the display serif the rows and the title use. The date-and-chevron toggle grows to a real hit target.
EmailFrame now zooms the message body down when its natural width is wider than the frame, and refits when the panel resizes. The inner horizontal scrollbar becomes a fallback for engines without CSS zoom.
Compose gets a hairline out of the navigation set. The category strip scrolls without a bar, so custom labels cannot wrap Folders onto a lonely second row. A count shows only while it is a real number; four identical 99+ badges carried no information.
The circle next to search now says it checks for new mail, so it does not read as a search rerun. The placeholder drops two filler words.
The action strip floats over the date on reveal; the invisible 112px reserve truncated senders next to apparent free space. The unread dot moves to a fixed leading slot in the highlight voice, so unread forms a scannable column. Snippets drop a leading subject echo through dedupeSnippet, with tests. shortFrom now shows the bare address for an address-only header. The account chip mix rises so accounts are tellable apart.
Weekday separators now carry the absolute date, so the header and the row pills speak one grammar. inboxDateGroupLabel moves to lib/shared/format with unit tests. A short fade at the top of the list makes a half-scrolled first row read as scrolled, not clipped.
The floating pill yields while the expanded rail shows the capture button, and returns when the rail collapses or goes off-canvas. The rail imports CAPTURE_BUTTON_LABEL instead of restating it twice. The notification count leaves the danger voice for the highlight voice.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR updates inbox formatting and navigation, reader split-versus-sheet behavior, capture controls, and proof-offer styling. It also adds Albatross work shapes, frontier documents, checklist bindings, completion metrics, and runtime coverage. ChangesMail and capture interface
Reader and work surfaces
Albatross planning and persistence
Estimated code review effort: 5 (Critical) | ~90 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
✅ Coverage Report
New Files
Changed Files
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/thread/ThreadView.tsx (1)
998-1020: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDisconnect old observers before handling a new iframe load.
Changing
srcDocreloads the same iframe. EachonLoadcall replacesobserverRef.currentandframeObserverRef.currentwithout disconnecting the prior observers. Cleanup disconnects only the latest pair. Earlier frame observers continue to observe the iframe and schedule duplicatefitcalls.Disconnect and clear both refs at the start of
onLoadbefore creating replacements. Add focused tests for an HTML change, pane resize, and unmount cleanup.As per coding guidelines, add or update focused tests for every behavioral, state, data, routing, or contract change.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/thread/ThreadView.tsx` around lines 998 - 1020, Update the onLoad handler in the iframe sizing flow to disconnect and clear observerRef.current and frameObserverRef.current before creating replacement ResizeObservers, ensuring prior iframe-load observers cannot schedule duplicate fit or measure calls. Preserve the existing observer setup and cleanup behavior, and add focused tests covering HTML changes, pane resizing, and unmount cleanup.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/inbox/Inbox.tsx`:
- Around line 1315-1340: Add focused component tests for the action controls
near the Inbox row metadata, covering visibility and usability on row hover and
keyboard focus, including interaction with the relevant controls. Also add tests
for the unread-count rendering in MailNav at counts 0, 99, and 100, asserting
each boundary displays the expected UI.
In `@components/thread/ThreadView.tsx`:
- Around line 490-587: Update the header action layout around the Hand-off
button and the reply, mail, and view action groups so it remains usable within a
320px reader pane without horizontal overflow; use the existing design-system
controls to wrap groups, move secondary actions into a reachable menu, or hide
the Hand-off text at narrow widths. Preserve all existing actions and behavior,
including fullscreen and close handling, and add a focused narrow-viewport test
verifying every action remains reachable.
In `@lib/shared/format.ts`:
- Around line 45-55: Update the prefix-removal logic in dedupeSnippet so it only
strips a matched subject when the following subject character is the string end,
whitespace, or a supported separator; otherwise return the original snip
unchanged. Add a focused test covering dedupeSnippet('Order', 'Orderly
dispatched') and preserving the full snippet.
- Around line 123-135: Update the date-grouping logic around startOfDay to use
local calendar boundaries via setDate rather than subtracting fixed dayMs
values. Restrict the Today branch to dates from today’s start through the next
local midnight, and derive Yesterday and the six-day window from
calendar-adjusted boundaries so daylight-saving transitions remain correct.
In `@tests/albatross-shell-vocabulary.test.ts`:
- Around line 79-83: Strengthen the tests around the rail rendering and
CAPTURE_BUTTON_LABEL contract: assert that the constant is used by both the
tooltip and visible label JSX attributes, rather than merely appearing in
imports or comments. Add focused coverage for expanded desktop, collapsed
desktop, and mobile sidebar visibility states, preserving the existing
label-value assertions and avoiding unrelated changes.
In `@tests/format.test.ts`:
- Around line 105-120: Extend the inboxDateGroupLabel tests with fixed local
timestamps covering both the current-year month label and the older
month-with-year label. Assert the exact expected formatting for each branch,
using the same local-date construction approach while keeping the existing
today, yesterday, weekday, and undated tests unchanged.
---
Outside diff comments:
In `@components/thread/ThreadView.tsx`:
- Around line 998-1020: Update the onLoad handler in the iframe sizing flow to
disconnect and clear observerRef.current and frameObserverRef.current before
creating replacement ResizeObservers, ensuring prior iframe-load observers
cannot schedule duplicate fit or measure calls. Preserve the existing observer
setup and cleanup behavior, and add focused tests covering HTML changes, pane
resizing, and unmount cleanup.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8b69694c-40af-4697-97d8-1cf2c7ca920c
📒 Files selected for processing (12)
app/globals.csscomponents/albatross/AreaHome.tsxcomponents/albatross/IntentCapture.tsxcomponents/inbox/Inbox.tsxcomponents/inbox/MailNav.tsxcomponents/shell/NotificationCenter.tsxcomponents/shell/Rail.tsxcomponents/thread/ProofOffer.tsxcomponents/thread/ThreadView.tsxlib/shared/format.tstests/albatross-shell-vocabulary.test.tstests/format.test.ts
Comment on lines
1315
to
+1340
| </div> | ||
| <span className="truncate text-[12.5px] leading-tight"> | ||
| <span className="truncate pl-3 text-[12.5px] leading-tight"> | ||
| <span className={item.unread ? 'font-medium text-[var(--color-text)]' : 'text-[var(--color-text)]'}> | ||
| {item.subject || '(no subject)'} | ||
| </span> | ||
| {item.snippet ? <span className="text-[var(--color-text-muted)]"> — {item.snippet}</span> : null} | ||
| {preview ? <span className="text-[var(--color-text-muted)]"> — {preview}</span> : null} | ||
| </span> | ||
| </div> | ||
|
|
||
| {/* Compact meta: date, then a single category chip (its reason lives in the | ||
| popover) + an Important dot + the account chip in all-accounts mode. */} | ||
| <div className="flex min-h-[40px] flex-col items-end justify-center gap-1 self-center"> | ||
| <div className="flex h-7 items-center justify-end gap-1.5"> | ||
| <div className="relative flex h-7 items-center justify-end gap-1.5"> | ||
| {/* The action strip floats over the date on reveal instead of | ||
| reserving an invisible 112px gutter on every row — that reserve | ||
| is what truncated senders mid-word next to apparent free space. | ||
| The pseudo layers rebuild the row surface underneath so the | ||
| buttons never sit on top of text. */} | ||
| <div | ||
| className={cn( | ||
| 'pointer-events-none flex w-[112px] items-center justify-end gap-0.5 opacity-0 transition-opacity duration-75 ease-out group-hover:pointer-events-auto group-hover:opacity-100 group-focus-within:pointer-events-auto group-focus-within:opacity-100 has-[[data-state=open]]:pointer-events-auto has-[[data-state=open]]:opacity-100', | ||
| 'pointer-events-none absolute inset-y-0 right-0 z-10 flex items-center justify-end gap-0.5 opacity-0 transition-opacity duration-75 ease-out group-hover:pointer-events-auto group-hover:opacity-100 group-focus-within:pointer-events-auto group-focus-within:opacity-100 has-[[data-state=open]]:pointer-events-auto has-[[data-state=open]]:opacity-100', | ||
| "before:absolute before:-inset-x-1 before:-inset-y-0.5 before:-z-10 before:rounded-md before:bg-[var(--color-bg-elevated)] before:content-['']", | ||
| "after:absolute after:-inset-x-1 after:-inset-y-0.5 after:-z-10 after:rounded-md after:content-['']", | ||
| selected || active | ||
| ? 'after:bg-[var(--color-selected-soft)]' | ||
| : 'after:bg-[var(--color-hover-soft)]', |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add focused tests for the changed inbox behaviors.
The supplied tests cover shared formatting only. Add component tests for these user-visible behaviors.
components/inbox/Inbox.tsx#L1315-L1340: verify action controls remain usable on hover and keyboard focus.components/inbox/MailNav.tsx#L109-L113: verify unread-count boundaries for 0, 99, and 100.
📍 Affects 2 files
components/inbox/Inbox.tsx#L1315-L1340(this comment)components/inbox/MailNav.tsx#L109-L113
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/inbox/Inbox.tsx` around lines 1315 - 1340, Add focused component
tests for the action controls near the Inbox row metadata, covering visibility
and usability on row hover and keyboard focus, including interaction with the
relevant controls. Also add tests for the unread-count rendering in MailNav at
counts 0, 99, and 100, asserting each boundary displays the expected UI.
Source: Coding guidelines
dedupeSnippet now requires a word boundary, so Orderly does not lose its head to Order. Date groups use calendar arithmetic, so a 25-hour fall-back day cannot shift the buckets, and a future date is never Today. The header clusters wrap at 320px instead of clipping. The badge threshold moves into unreadBadgeLabel with boundary tests, the vocabulary test asserts both JSX slots of the capture constant plus the pill gating, and the row action strip gets focused source tests.
Contributor
Author
|
@coderabbitai review Round one worked in 38f137c:
|
Contributor
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '=== PR commits ==='
git log --oneline --decorate -12
printf '%s\n' '=== Files changed by follow-up commit ==='
git show --stat --oneline --find-renames 38f137c
printf '%s\n' '=== Follow-up diff ==='
git show --format=fuller --find-renames --find-copies --unified=60 38f137c -- \
lib/shared/format.ts \
components/thread/ThreadView.tsx \
components/albatross/IntentCapture.tsx \
components/shell/Rail.tsx \
tests
printf '%s\n' '=== Current PR diff file list ==='
git diff --name-status "$(git merge-base HEAD origin/main)" HEAD 2>/dev/null || \
git diff --name-status "$(git merge-base HEAD origin/master)" HEAD
printf '%s\n' '=== Source outlines ==='
ast-grep outline lib/shared/format.ts --items all
ast-grep outline components/thread/ThreadView.tsx --items all
ast-grep outline components/albatross/IntentCapture.tsx --items all
ast-grep outline components/inbox/Inbox.tsx --items all
printf '%s\n' '=== Relevant current source and tests ==='
rg -n -C 8 \
'dedupeSnippet|inboxDateGroupLabel|unreadBadgeLabel|CAPTURE_BUTTON_LABEL|useSidebar|group.*wrap|flex-wrap|data-testid|hover|focus|open-menu|selected' \
lib/shared/format.ts \
components/thread/ThreadView.tsx \
components/albatross/IntentCapture.tsx \
components/shell/Rail.tsx \
components/inbox/Inbox.tsx \
testsLength of output: 225312 🏁 Script executed: #!/bin/bash
set -euo pipefail
base="$(git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD origin/master)"
printf '%s\n' '=== unreadBadgeLabel call sites and MailNav ==='
rg -n -C 6 '\bunreadBadgeLabel\s*\(' . -g '!node_modules' -g '!dist' -g '!build'
ast-grep outline components/inbox/MailNav.tsx --items all
cat -n components/inbox/MailNav.tsx
printf '%s\n' '=== Full current ThreadView header and action region ==='
sed -n '450,650p' components/thread/ThreadView.tsx
printf '%s\n' '=== Inbox row layout and action region ==='
sed -n '1180,1480p' components/inbox/Inbox.tsx
printf '%s\n' '=== Current PR diff: all changed files, compact context ==='
git diff --find-renames --unified=16 "$base" HEAD -- \
app/globals.css \
components/albatross/AreaHome.tsx \
components/albatross/IntentCapture.tsx \
components/inbox/Inbox.tsx \
components/inbox/MailNav.tsx \
components/shell/NotificationCenter.tsx \
components/shell/Rail.tsx \
components/thread/ProofOffer.tsx \
components/thread/ThreadView.tsx \
lib/shared/format.ts \
tests/albatross-shell-vocabulary.test.ts \
tests/format.test.ts \
tests/inbox-row-actions.test.ts
printf '%s\n' '=== Project scripts and static configuration ==='
fd -a -d 2 '^(package\.json|biome\.jsonc?|tsconfig\.json)$' . -x sh -c 'echo --- $1; cat "$1"' sh {}Length of output: 149910
The follow-up changes address the round-one findings. I found no additional actionable issues. I verified the updated call sites and source contracts for:
I did not run tests, lint, type checks, or the production build in this environment.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/inbox/MailNav.tsx (1)
78-123: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd focused
MailNavcoverage.Lines 78-123 change the navigation layout from wrapping to horizontal scrolling and change rendered badge behavior. The supplied row-action test only reads
components/inbox/Inbox.tsx. Add aMailNavrendering test for the non-wrapping overflow strip and suppressed saturated badges.As per coding guidelines, “Tests must not regress; add or update focused tests for every behavioral, state, data, routing, or contract change.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/inbox/MailNav.tsx` around lines 78 - 123, Add focused rendering coverage for MailNav, targeting the navigation strip containing the SMART_CATEGORIES and custom labels. Verify the strip uses non-wrapping horizontal overflow and that saturated unread counts are suppressed according to unreadBadgeLabel behavior, while preserving existing row-action test coverage.Source: Coding guidelines
components/thread/ThreadView.tsx (1)
978-1002: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdd focused tests for email fitting and observer cleanup.
The repository has no focused
ThreadViewtests for this behavior. Add DOM-capable tests for narrow and expanded panes, repeated refits, zoom-adjusted height measurement, andResizeObservercleanup on unmount.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/thread/ThreadView.tsx` around lines 978 - 1002, Add focused DOM-capable tests for the ThreadView email fitting flow around the fit, measure, and onLoad callbacks. Cover narrow panes applying zoom, expanded panes removing zoom, repeated refits when the frame width changes, height calculation using zoom-adjusted scroll dimensions, and disconnecting the ResizeObserver during unmount.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/albatross-shell-vocabulary.test.ts`:
- Around line 90-96: Replace the source-text assertions in the capture-pill test
with runtime rendering of IntentCaptureLauncher using mocked useSidebar states.
Assert the wrapper applies hidden for an open desktop rail and omits hidden for
both collapsed desktop and mobile states, covering each visibility behavior.
In `@tests/format.test.ts`:
- Around line 125-142: Update the date-group tests around inboxDateGroupLabel to
use one frozen local reference time instead of separate live Date.now-based
values. Derive tomorrow and the older dates from that shared reference with
setDate, and compute expected labels from the same reference so midnight cannot
change the test’s assumptions.
---
Outside diff comments:
In `@components/inbox/MailNav.tsx`:
- Around line 78-123: Add focused rendering coverage for MailNav, targeting the
navigation strip containing the SMART_CATEGORIES and custom labels. Verify the
strip uses non-wrapping horizontal overflow and that saturated unread counts are
suppressed according to unreadBadgeLabel behavior, while preserving existing
row-action test coverage.
In `@components/thread/ThreadView.tsx`:
- Around line 978-1002: Add focused DOM-capable tests for the ThreadView email
fitting flow around the fit, measure, and onLoad callbacks. Cover narrow panes
applying zoom, expanded panes removing zoom, repeated refits when the frame
width changes, height calculation using zoom-adjusted scroll dimensions, and
disconnecting the ResizeObserver during unmount.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 78fa126e-893b-4f20-9830-57488832ae92
📒 Files selected for processing (6)
components/inbox/MailNav.tsxcomponents/thread/ThreadView.tsxlib/shared/format.tstests/albatross-shell-vocabulary.test.tstests/format.test.tstests/inbox-row-actions.test.ts
Comment on lines
+90
to
+96
| test('one capture door per screen: the pill yields to the expanded rail', () => { | ||
| const capture = read('components/albatross/IntentCapture.tsx'); | ||
| // Expanded desktop rail shows its own capture button, so the pill hides; | ||
| // a collapsed rail or the mobile off-canvas drawer brings the pill back. | ||
| expect(capture).toContain('useSidebar'); | ||
| expect(capture).toContain('railOpen && !isMobile'); | ||
| }); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add runtime coverage for the capture-pill states.
expect(capture).toContain(...) checks source text only. It does not prove that hidden is applied for an open desktop rail or omitted for a collapsed or mobile rail. Render IntentCaptureLauncher with mocked sidebar states and assert the wrapper visibility.
As per coding guidelines, tests must not regress; add or update focused tests for every behavioral, state, data, routing, or contract change.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/albatross-shell-vocabulary.test.ts` around lines 90 - 96, Replace the
source-text assertions in the capture-pill test with runtime rendering of
IntentCaptureLauncher using mocked useSidebar states. Assert the wrapper applies
hidden for an open desktop rail and omits hidden for both collapsed desktop and
mobile states, covering each visibility behavior.
Source: Coding guidelines
The pill-visibility decision moves into capturePillHidden, a pure function with a truth-table test — real state coverage without a DOM harness this repo does not have. inboxDateGroupLabel accepts an injectable reference time, and the date-group tests derive every case from one frozen local noon, so no assertion races midnight.
Contributor
Author
|
Round two worked:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/albatross/IntentCapture.tsx`:
- Line 434: Update the focus-management flow around the capture-opening handler
and close effect to track the visible element that opened the dialog, including
the rail trigger when it opens through captureOpen. On dismissal, restore focus
to that tracked trigger when it remains visible, and fall back to launcherRef
only when the launcher is visible; avoid focusing an element inside the hidden
launcher wrapper.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c05e519d-e133-4b75-89fc-1d7a6a1cbf71
📒 Files selected for processing (6)
components/albatross/IntentCapture.tsxlib/client-state.tslib/shared/format.tstests/albatross-client-store.test.tstests/albatross-shell-vocabulary.test.tstests/format.test.ts
dateToEpoch's numeric-string path and formatDate's unparseable-string path now have assertions, so lib/shared/format sits at full coverage.
emailNeedsIsolatedFrame and emailDeclaresOwnBackground decide which mail rides the isolated frame the new fit-to-width logic serves; they had no tests. The SSR early-returns of the sanitizers are asserted too, so server and client output cannot diverge.
The rail can open capture while the floating launcher is hidden, and the close path focused that hidden element, so keyboard focus fell to the body. The opener is recorded at open and restored at close; the launcher is only the fallback while it is visible.
Contributor
Author
|
Worked the incremental finding too: capture now records the element that opened it and restores focus there on dismissal. The floating launcher is only the fallback, and only while it is visible (offsetParent check). The initial mount no longer moves focus at all. |
The capture splitter and the planner now classify each intent's shape. Every plan composes the native document; an open question no longer suppresses it. The host appends a deterministic gate: options as a decision block, free text always, and an outline of the steps that wait. After apply, keyed checklist items bind to their created cards, so a checkbox in the brief toggles the shared task record everywhere. Completion records shape, task counts, and capture-to-done time. WorkDetail drops the sandboxed HTML frame and the duplicated task lists; the document is the page body, with a change ledger and a complete/reopen block.
The live-page rule rerouted shouldComposeWorkBrief away from projectPromotionDecision, and the deleted HTML-artifact prompt shrank the covered pool. Direct tests take promotion, the split fallbacks, the timezone fallback, and the area task/project context lines.
Opening a thread no longer spawns a second card on the paper. The mail surface is one elevated card; a thread halves it, and the resize separator paints the seam: card surface, continuous top and bottom borders, a 1px interior rule. The outer gutter drops to 6/8px evenly. The reader interior goes flat. One slim strip holds archive, trash, expand, and close — the only fixed chrome. The title moves into the scroll at full width, wrapping, never truncated; the header actions cost about 430px of a 650px pane, which is what truncated titles next to free space. Messages and the summary separate with hairlines, not boxes. The thread's verbs float at the bottom, where a finished read lands: Reply, Reply all, Forward, then the recognition — This is an Albatross. Hand to Albatross described plumbing; the declaration says what the click means: this email carries tasks to get done. The composer replaces the bar while it is open. The row date is plain text again and the hover actions may cover it; the mailbox popover moves to the avatar with an account-tint ring, which nothing ever overlays. tests/thread-reader-shape.test.ts pins the seam, the flat interior, and the verbs.
Contributor
Author
|
The design conversation landed as 74c5e58 — a structural correction over the earlier reader-card commits:
Verified: 2305 tests green, typecheck and build pass, and the split was exercised headlessly through the Compose path (screenshot confirmed one continuous card with the seam). |
Reviewed capture items, the splitter shape pass-through, project task counting on completion, release/reopen and archive round-trips, the duplicate-question refresh, service detection in the composer pack, and the small parse fallbacks.
The strip held four icons and a wide empty left side while the title sat orphaned below it. The title moves into the strip, truncating with a full-text tooltip; the toolbar has room for it now. px-3 py-2 around 32px controls is MailNav's exact geometry, so the reader's bottom border meets the inbox's across the seam. py-1.5 with the same controls sat 4px high, which is why the two rules stepped. The compose header takes the same geometry. The floating verbs justify to the trailing edge instead of centering.
The bind helpers run inside Convex mutations. A JSON round-trip clones the validated document losslessly in every runtime.
… visit The two headers still stepped apart because each padded around its tallest child, and a bordered segment group is 2px taller than a bare button. Both rows are h-12 now, so the rules meet whatever they hold. The view actions sat 48px off the right edge on a pr-12 reserve for the assistant, which became a bottom-right overlay long ago. The reserve and its dead aiBarOpen read are gone; the actions inset by px-3, matching the title. Off Mail there is no card to halve. Today and Areas were tearing their layout into a split that only made sense beside an inbox, so the reader takes a variant: split on Mail, sheet everywhere else — a whole card over the page, the surface underneath keeping its width and staying usable. The capture pill yields to an open reader, whose action bar already holds This is an Albatross in that corner.
Contributor
Author
|
@coderabbitai full review |
Contributor
✅ Action performedFull review finished. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/thread/ThreadView.tsx (1)
1012-1026: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse separate animation-frame handles for the two observers.
Both observers share the single
rafvariable. The body observer schedulesmeasure, and the frame observer schedulesfit. Each callback callscancelAnimationFrame(raf)before it assigns its own handle. If the body resizes in the same frame as a pane resize (for example, a late image load during a seam drag), the body callback cancels the pendingfitand replaces it withmeasure. The zoom then stays computed for the old pane width until the next resize event.Track the two schedules separately.
Note: the static analysis finding at line 1020 (
effect-observer-needs-disconnect) is already handled. The cleanup at lines 1036-1037 disconnectsframeObserverRef.♻️ Proposed refactor
let raf = 0; + let fitRaf = 0;if ('ResizeObserver' in window && doc.body) { const ro = new ResizeObserver(() => { cancelAnimationFrame(raf); raf = requestAnimationFrame(measure); }); ro.observe(doc.body); observerRef.current = ro; // Panel resizes change the frame width; refit the zoom to match. const frameRo = new ResizeObserver(() => { - cancelAnimationFrame(raf); - raf = requestAnimationFrame(fit); + cancelAnimationFrame(fitRaf); + fitRaf = requestAnimationFrame(fit); }); frameRo.observe(iframe); frameObserverRef.current = frameRo; }Cancel
fitRafin the effect cleanup besideraf.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/thread/ThreadView.tsx` around lines 1012 - 1026, Use distinct animation-frame handles for the body observer’s measure scheduling and the frame observer’s fit scheduling in the surrounding effect: keep the existing handle for measure, add a separate fitRaf handle, and have the frame observer cancel and assign only fitRaf. Update effect cleanup to cancel both handles alongside disconnecting the observers.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/albatross/IntentCapture.tsx`:
- Around line 245-247: Update readerOpen in IntentCapture to use the same
mail-ish surface predicate as AppShell, ideally by extracting and reusing a
shared helper, so stale selectedThreadId values outside Mail, Today, or Areas do
not hide the pill. Add focused coverage confirming a selected thread on a
non-mail-ish surface still renders the capture action.
In `@components/albatross/WorkDetail.tsx`:
- Around line 377-390: Update the WorkDetail state-rendering branch to include
work.workState === 'released' alongside 'done', rendering the same Finished
message and Reopen button that calls setWorkState('active'). Preserve the
existing behavior and styling for done and leave archived without a reopen
control.
- Around line 108-125: Extract a shared postJson helper in WorkDetail.tsx that
safely handles non-JSON response bodies, checks response.ok, and preserves the
caller’s fallback error text. Use it in setWorkState at
components/albatross/WorkDetail.tsx#L108-L125, retaining “Could not update this
Albatross.”, and in LegacyPlanNotice at
components/albatross/WorkDetail.tsx#L418-L436, retaining “Could not rebuild the
plan.”
- Around line 254-262: Update the status-dot span in the action rendering near
action.title to include a non-color accessible label that communicates whether
the action is applied or not applied, using the existing done state. Preserve
the current visual styling and ensure the label is available to screen readers
without adding visible text.
- Around line 293-317: Update the successful undo path in WorkDetail’s
undo_operation handler to remove or filter the undone artifact from the
displayed application artifacts after callTool resolves. Ensure the artifact row
no longer renders the Undo button, while preserving the existing error handling
and undoing-state cleanup.
In `@components/inbox/Inbox.tsx`:
- Around line 1292-1319: Update the mailbox popover trigger button in the
showAccount/accountColor branch to stop keyboard event propagation in addition
to its existing onClick handling. Ensure Enter and Space key events do not reach
the row container’s onKeyDown handler, while preserving normal popover
activation and row keyboard behavior elsewhere.
In `@components/report/brief-canvas/BriefNodeView.tsx`:
- Around line 461-466: Add focused tests for the checklist renderer covering the
completion precedence in the `checked` computation: assert that
`context.completedRefs` set to false overrides a live entity value of true, a
live entity value of false overrides `item.checked` true, and `item.checked` is
used when no live entity state exists.
In `@lib/shared/format.ts`:
- Around line 61-67: Update unreadBadgeLabel to return the saturated marker
"99+" for unread counts of 100 or more instead of null, while preserving null
for zero, negative, null, and undefined values and numeric labels below 100.
Update the matching unreadBadgeLabel(100) and unreadBadgeLabel(2500) cases in
tests/format.test.ts.
In `@tests/albatross-intent-plan.test.ts`:
- Around line 262-266: Update the focused tests around the duplicate
place_region calls and their corresponding assertions to verify replacement
rather than mere non-empty output. Assert that the resulting regions have unique
ids, including the additional case around the later replace-in-place test, while
preserving the existing behavior checks.
In `@tests/capture-work.test.ts`:
- Around line 114-125: Update the focused splitter test around captureWork to
assert that the generateCalls input includes WORK_SHAPE_GUIDE and the six-value
shape schema in the splitter system prompt. Reuse the existing captured
generateCalls data and keep the current committed-item shape assertion intact.
In `@tests/thread-reader-shape.test.ts`:
- Around line 24-27: Update the test covering the date interaction in the
thread-reader shape check to remove the assertion against the source comment
text. Replace it with an assertion on the rendered markup or relevant
non-interactive date element in Inbox.tsx, while retaining the existing
assertion for “Mailbox this thread arrived in”.
---
Outside diff comments:
In `@components/thread/ThreadView.tsx`:
- Around line 1012-1026: Use distinct animation-frame handles for the body
observer’s measure scheduling and the frame observer’s fit scheduling in the
surrounding effect: keep the existing handle for measure, add a separate fitRaf
handle, and have the frame observer cancel and assign only fitRaf. Update effect
cleanup to cancel both handles alongside disconnecting the observers.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 19c1237c-2be7-4e19-a819-f260f0b7279c
📒 Files selected for processing (37)
app/globals.csscomponents/albatross/AreaHome.tsxcomponents/albatross/IntentCapture.tsxcomponents/albatross/WorkDetail.tsxcomponents/albatross/primitives.tsxcomponents/inbox/Inbox.tsxcomponents/inbox/MailNav.tsxcomponents/report/brief-canvas/BriefNodeView.tsxcomponents/shell/AppShell.tsxcomponents/shell/NotificationCenter.tsxcomponents/shell/Rail.tsxcomponents/thread/ProofOffer.tsxcomponents/thread/ThreadView.tsxconvex/albatrossIntents.tsconvex/albatrossWork.tsconvex/albatrossWorkV2.tsconvex/schema.tslib/albatross/capture-work.tslib/albatross/forgiveness.tslib/albatross/intent-plan.tslib/albatross/plan-frontier.tslib/albatross/work-shape.tslib/albatross/work-v2.tslib/client-state.tslib/shared/brief-document.tslib/shared/format.tstests/albatross-client-store.test.tstests/albatross-intent-plan.test.tstests/albatross-live-brief-runtime.test.tstests/albatross-plan-frontier.test.tstests/albatross-shell-vocabulary.test.tstests/albatross-work-v2.test.tstests/capture-work.test.tstests/email-frame-choice.test.tstests/format.test.tstests/inbox-row-actions.test.tstests/thread-reader-shape.test.ts
Comment on lines
+245
to
+247
| // An open thread shows "This is an Albatross" in its own action bar, in the | ||
| // same corner this pill occupies. | ||
| const readerOpen = useClientStore((s) => !!(s.selectedThreadId || s.compose.mode)); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Match readerOpen to the rendered reader state.
readerOpen becomes true for any stale selectedThreadId. AppShell renders that reader only on Mail, Today, and Areas. After Mail → Albatrosses navigation, the pill stays hidden although no reader capture action exists. This blocks capture on mobile after the rail closes.
Derive this value from the same mail-ish predicate as AppShell, preferably through one shared helper. Add focused coverage for a selected thread outside a mail-ish surface.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/albatross/IntentCapture.tsx` around lines 245 - 247, Update
readerOpen in IntentCapture to use the same mail-ish surface predicate as
AppShell, ideally by extracting and reusing a shared helper, so stale
selectedThreadId values outside Mail, Today, or Areas do not hide the pill. Add
focused coverage confirming a selected thread on a non-mail-ish surface still
renders the capture action.
Source: Coding guidelines
Comment on lines
+61
to
+67
| // The category-tab badge only earns pixels while it is a real number; a | ||
| // saturated counter ("99+") says nothing. Null means "render no badge". | ||
| export function unreadBadgeLabel(unread: number | null | undefined): string | null { | ||
| if (!unread || unread <= 0) return null; | ||
| if (unread >= 100) return null; | ||
| return String(Math.floor(unread)); | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Render a saturated marker instead of nothing at 100 or more.
unreadBadgeLabel returns null for counts of 100 or more. MailNav then renders no badge. A category with 250 unread messages looks identical to a category with 0 unread messages. The busiest category loses its signal exactly when the signal matters most.
Return a saturated label so the count stays visible.
🛠️ Proposed fix
export function unreadBadgeLabel(unread: number | null | undefined): string | null {
if (!unread || unread <= 0) return null;
- if (unread >= 100) return null;
+ if (unread >= 100) return '99+';
return String(Math.floor(unread));
}Update the matching cases in tests/format.test.ts (unreadBadgeLabel(100), unreadBadgeLabel(2500)).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // The category-tab badge only earns pixels while it is a real number; a | |
| // saturated counter ("99+") says nothing. Null means "render no badge". | |
| export function unreadBadgeLabel(unread: number | null | undefined): string | null { | |
| if (!unread || unread <= 0) return null; | |
| if (unread >= 100) return null; | |
| return String(Math.floor(unread)); | |
| } | |
| // The category-tab badge only earns pixels while it is a real number; a | |
| // saturated counter ("99+") stays visible. Null means "render no badge". | |
| export function unreadBadgeLabel(unread: number | null | undefined): string | null { | |
| if (!unread || unread <= 0) return null; | |
| if (unread >= 100) return '99+'; | |
| return String(Math.floor(unread)); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/shared/format.ts` around lines 61 - 67, Update unreadBadgeLabel to return
the saturated marker "99+" for unread counts of 100 or more instead of null,
while preserving null for zero, negative, null, and undefined values and numeric
labels below 100. Update the matching unreadBadgeLabel(100) and
unreadBadgeLabel(2500) cases in tests/format.test.ts.
Guard every fetch against non-JSON error bodies through one postJson helper. Label the applied dot for screen readers. Strike undone changes out of the ledger and stop offering their Undo. Give released work a pick-it-back-up path that clears the release, with runtime coverage. Stop keyboard events on the mailbox popover trigger so it opens from the keyboard. Assert the replace branch, the splitter shape-guide prompt, checklist completion precedence, and the date span's markup instead of a comment.
Contributor
Author
|
Round one is worked in cae5b8f: postJson guard on every WorkDetail fetch, an accessible label on the applied dot, undone changes struck from the ledger, a pick-it-back-up path for released work (the mutation clears the release), keyboard events stopped on the mailbox popover trigger, and the four test-contract findings. One finding is skipped on purpose: unreadBadgeLabel returns null at 100+ because quieting saturated counts is a deliberate product decision (commit ed3731b), not an omission. The capture-pill readerOpen finding was fixed separately in c760da0. @coderabbitai full review |
Contributor
|
✏️ Learnings added
✅ Action performedFull review finished. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/inbox/Inbox.tsx (1)
1378-1441: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftSeparate row actions from the row keyboard handler.
The action buttons remain descendants of the row
role="button"container. When an action has focus, itskeydownevent reaches the row handler.Spaceselects the row.Enteropens the thread. Keyboard users cannot reliably use Select, Archive, Delete, orQuickFixMenu.Make the row container non-interactive and provide a separate row-open control. Add interaction tests that press
EnterandSpaceon each action control. The current source-text test cannot execute this event path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/inbox/Inbox.tsx` around lines 1378 - 1441, Refactor the row container in Inbox so it no longer uses role="button" or handles row keyboard activation; add a separate focusable row-open control that preserves Enter-to-open behavior. Ensure Select, Archive, Delete, and QuickFixMenu controls are outside that keyboard handler and retain their own actions without triggering row selection or opening. Replace the source-text test with interaction tests that press Enter and Space on every action control and verify the row-open and action behaviors remain isolated.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@convex/albatrossWorkV2.ts`:
- Around line 108-125: Add a compound index covering userId, projectId, and
artifactKind for albatrossProjectLinks, update the query in the task-link
loading flow to use that index with artifactKind set to task, and call take(200)
before loading cards instead of collecting all matching links. Preserve the
existing card filtering and task totals logic.
In `@lib/albatross/plan-frontier.ts`:
- Around line 149-152: Update walkNodes to return immediately when node is null
or not an object, before invoking visit or reading children; preserve recursive
traversal for valid BriefNode objects. Add coverage for hasFrontierGate with a
frontier-gate region whose tree is missing or null, ensuring it returns safely
without throwing.
In `@lib/shared/format.ts`:
- Around line 130-150: Update lib/shared/format.ts lines 130-150 in
inboxDateGroupLabel to accept number | string and normalize string dates through
the same conversion path as formatDate. Update components/inbox/Inbox.tsx lines
1047-1051 to pass it.lastDate ?? it.date directly without Number coercion. Add a
focused test covering an ISO-string date and its expected group label.
---
Outside diff comments:
In `@components/inbox/Inbox.tsx`:
- Around line 1378-1441: Refactor the row container in Inbox so it no longer
uses role="button" or handles row keyboard activation; add a separate focusable
row-open control that preserves Enter-to-open behavior. Ensure Select, Archive,
Delete, and QuickFixMenu controls are outside that keyboard handler and retain
their own actions without triggering row selection or opening. Replace the
source-text test with interaction tests that press Enter and Space on every
action control and verify the row-open and action behaviors remain isolated.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ca466556-8857-4bf5-b5c1-0d3e86a78290
📒 Files selected for processing (38)
app/globals.csscomponents/albatross/AreaHome.tsxcomponents/albatross/IntentCapture.tsxcomponents/albatross/WorkDetail.tsxcomponents/albatross/primitives.tsxcomponents/inbox/Inbox.tsxcomponents/inbox/MailNav.tsxcomponents/report/brief-canvas/BriefNodeView.tsxcomponents/shell/AppShell.tsxcomponents/shell/NotificationCenter.tsxcomponents/shell/Rail.tsxcomponents/thread/ProofOffer.tsxcomponents/thread/ThreadView.tsxconvex/albatrossIntents.tsconvex/albatrossWork.tsconvex/albatrossWorkV2.tsconvex/schema.tslib/albatross/capture-work.tslib/albatross/forgiveness.tslib/albatross/intent-plan.tslib/albatross/plan-frontier.tslib/albatross/work-shape.tslib/albatross/work-v2.tslib/client-state.tslib/shared/brief-document.tslib/shared/format.tstests/albatross-client-store.test.tstests/albatross-intent-plan.test.tstests/albatross-live-brief-runtime.test.tstests/albatross-plan-frontier.test.tstests/albatross-shell-vocabulary.test.tstests/albatross-work-v2.test.tstests/brief-canvas-renderer.test.tsxtests/capture-work.test.tstests/email-frame-choice.test.tstests/format.test.tstests/inbox-row-actions.test.tstests/thread-reader-shape.test.ts
Comment on lines
+108
to
+125
| const links = await ctx.db | ||
| .query('albatrossProjectLinks') | ||
| .withIndex('by_user_project', (q) => | ||
| q.eq('userId', work.userId).eq('projectId', work.primaryProjectId!), | ||
| ) | ||
| .collect(); | ||
| const taskLinks = links.filter((link) => link.artifactKind === 'task').slice(0, 200); | ||
| if (taskLinks.length) { | ||
| const cards = await Promise.all( | ||
| taskLinks.map(async (link) => { | ||
| const cardId = ctx.db.normalizeId('cards', link.artifactId); | ||
| const card = cardId ? await ctx.db.get(cardId) : null; | ||
| return card && card.userId === work.userId ? card : null; | ||
| }), | ||
| ); | ||
| const present = cards.filter((card) => card !== null); | ||
| tasksTotal = present.length; | ||
| tasksCompleted = present.filter((card) => card!.completedAt).length; |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline convex/albatrossWorkV2.ts --match recordWorkCompletion --view expanded
rg -n -C 4 'albatrossProjectLinks|by_user_project|artifactKind' convex/schema.ts convexRepository: Lab86-io/lab86-mail
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- completion helper ---'
sed -n '90,145p' convex/albatrossWorkV2.ts
printf '%s\n' '--- project-link schema ---'
sed -n '760,795p' convex/schema.ts
printf '%s\n' '--- project-link writes and relevant limits ---'
rg -n -C 5 "insert\('albatrossProjectLinks'|patch\(.*albatrossProjectLinks|PROGRESS_TASK_RESOLVE_CAP|slice\(0, 200\)|\\.take\\(" convex/albatrossWorkV2.ts convex/albatrossWork.ts convex/albatrossRoutines.ts
printf '%s\n' '--- completion helper callers ---'
rg -n -C 5 'recordWorkCompletion|recordCompletionEvent' convex/albatrossWorkV2.ts convexRepository: Lab86-io/lab86-mail
Length of output: 32403
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
source = Path("convex/albatrossWorkV2.ts").read_text()
schema = Path("convex/schema.ts").read_text()
helper = source[source.index("async function recordWorkCompletion"):source.index("export const updateWorkState")]
collect_pos = helper.index(".collect()")
filter_pos = helper.index("links.filter")
slice_pos = helper.index(".slice(0, 200)")
event_pos = helper.index("await recordCompletionEvent")
print({
"collect_before_task_filter": collect_pos < filter_pos,
"task_filter_before_limit": filter_pos < slice_pos,
"link_read_before_completion_event": collect_pos < event_pos,
"current_project_link_index": bool(
re.search(r"\.index\('by_user_project',\s*\['userId',\s*'projectId'\]\)", schema)
),
"project_kind_index": bool(
re.search(r"\.index\([^\\n]*\['userId',\s*'projectId',\s*'artifactKind'\]", schema)
),
})
writes = list(re.finditer(r"insert\('albatrossProjectLinks'", source))
print({"project_link_inserts_in_albatrossWorkV2": len(writes)})
PY
printf '%s\n' '--- exact project-link insertion paths ---'
rg -n -C 8 "insert\('albatrossProjectLinks'" convex/albatrossWork.ts convex/albatrossRoutines.ts convex/albatrossWorkV2.tsRepository: Lab86-io/lab86-mail
Length of output: 2553
Bound the project-link query before reading cards.
The query collects every project link before applying the 200-task limit. Add a userId/projectId/artifactKind index and use .take(200) before loading cards.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@convex/albatrossWorkV2.ts` around lines 108 - 125, Add a compound index
covering userId, projectId, and artifactKind for albatrossProjectLinks, update
the query in the task-link loading flow to use that index with artifactKind set
to task, and call take(200) before loading cards instead of collecting all
matching links. Preserve the existing card filtering and task totals logic.
Bound the project-link read behind the completion record. Guard the document walk against malformed stored nodes. Accept ISO-string dates in the inbox group labels so provider rows do not fall to Undated.
16 tasks
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.
What this is
A full pass over the mail-surface audit: depth, hierarchy, truncation, duplication, and voice-contract nits, each as its own commit.
The fixes
Depth and structure
surface-float.border-l-solidis not a Tailwind utility, so the edge rendered dashed and the card read as a dropzone. The dismissal now carries a resting underline affordance.Thread header
SEGMENT_GROUP).Message header and body
EmailFramescales fixed-width mail down to the pane with CSS zoom and refits on panel resize. The inner horizontal scrollbar becomes a fallback for engines without zoom.List header
List rows
accent-3), per the palette voice contract. It now forms a scannable column.dedupeSnippetstrips a leading subject echo from provider snippets, with tests.shortFromreturns the bare address for an address-only header instead of raw angle brackets, with a test.Separators and edges
inboxDateGroupLabelmoves tolib/shared/formatwith unit tests.Shell
CAPTURE_BUTTON_LABELinstead of restating it; the vocabulary test now asserts the constant.Audit items resolved as no-change, with reasons
Verification
biome check .clean,tsc --noEmitclean, productionnext buildpasses.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit
New Features
Bug Fixes
Style
Added in this branch: the live Work page (f2a2235)
The Albatross detail page now trusts the brief.
shape; the planner's verdict wins.completionEvents— raw material for a future progress page.