test(account): gate the faulted crew's worst case, and stop paying 88px for it - #173
Merged
Conversation
…px for it The manifest's horizontal-scroll gate only ever measured a healthy crew, and `showStatusColumn` is exception-only — so the widest column in the manifest was not in the DOM when the test ran. The member who has to reach STATUS and ACTIONS is the member with a fault, and that layout was unmeasured. Adds a gate that seeds one. It fails against main at 240.2px on the STATUS cell (budget 232) and was proven to before any production code changed. The column is content-sized, so "a faulted crew" is a range, not a width. Seeded `needs_reauth` — the widest member-fixable result — after measuring all of them; a `missing_label` seed passes at 283px and says nothing about the 299px a `needs_reauth` member is served. The 88px `min-width` gutter on the labels bought an alignment the widest label (71px "standings") never needed. A two-column grid over `display: contents` lines sizes it to content and takes 17px off every faulted state uniformly: label_mismatch 293 -> 275 missing_scope 308 -> 291 missing_label 300 -> 283 needs_reauth 316 -> 299 token_invalid 308 -> 291 The common case now clears one region-width (286px at 320px); the worst member-fixable one does not. 299px is still over, the remaining 13px is copy rather than layout, and both the rule and the gate say so rather than implying the gap was closed. Also fixes a regression the grid introduced: `justify-items` defaults to `stretch`, which sized the bordered `re-authorize` control to the standings chip beside it (168px next to "token refresh failed", 199px next to an unrecognized code). `.stack` never did that. `start` restores its own 112.6px and leaves every measured width unchanged.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe account status cell now uses a shared grid for labels and values. End-to-end coverage seeds faulted contact-sync results and verifies the 320px layout, status controls, row placement, and horizontal overflow limits. ChangesAccount status layout
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
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 `@e2e/account.spec.ts`:
- Around line 125-156: Update faultContacts to accept the seeded account ID and
constrain its character subquery with character.accountId in addition to the
names filter. Update faultOneAlt and its invocation around the referenced test
flow to pass the same account-scoped API, using acc.id, while preserving the
row-count assertion.
🪄 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 Plus
Run ID: 717b73c8-f7d7-442e-8b5f-fff85985f71f
📒 Files selected for processing (3)
e2e/account.spec.tssrc/app/account/page.tsxsrc/app/globals.css
guarzo
enabled auto-merge (squash)
August 7, 2026 21:39
guarzo
added a commit
that referenced
this pull request
Aug 7, 2026
… 3) (#174) * feat(account): report the healthy verdict as a green chip with a count * feat(account): let the crew manifest opt out of the narrow page cap * feat(account): move remediation detail into a sub-row under its character * fix(account): cancel the drawer row's dead margin and raise the fold floor .table-notes used to cancel .table-note's own bottom margin for the last child; deleting that container without a replacement left every drawer row with s-4 of dead space under a row whose whole point is minimal height. Adds .drawer-row .table-note { margin-bottom: 0 } and re-measures the sub-row (37px at 1440/1280, 57px at 390, not the flat 53px the prior comment claimed). Waits on document.fonts.ready before the fold measurement, which removed the run-to-run variance that made FAULTED_FOLD_TARGETS unraisable; raises the floor to the now-stable observed minimum (7/6/5, five consecutive runs each viewport) per Step 11's instruction to raise rather than leave a floor below the truth. Also re-indents two comment bodies whose opening `{/*` moved during the prior commit's reindent but whose body lines did not follow, and updates .table-note's docblock now that it also renders inside a table row. * fix(ui): stop the row-hover tint painting over drawer rows * fix(account): reconcile the remedy sub-row with #173's faulted layout The branch was cut before round 2 landed and #173 reworked the same faulted-crew layout. Rebasing onto it exposed three real interactions, none of which the pre-rebase measurements could have seen. #173's 320px gate counted `re-authorize` links across the whole manifest and asserted 1. Task 3 moved the remedy prose inside the manifest, and each contacts-faulted row's `ContactRemedy` carries its own link when the token is valid — so the honest count is 3. Scoped the existing assertion to `.status-line`, which is the cell the width measurement is of, and pinned the other two on the drawer rows rather than dropping them: a change that stops rendering them should fail here, not quietly loosen the line above. The 320px width budget itself is unaffected — a spanning sub-row adds no horizontal cost, and the gate still passes. Re-measured the fold on the new base: 7/6/5, identical across three runs and unchanged from the pre-rebase measurement. #173's -17px per faulted state is real but two faulted rows do not add up to a 63px character row, so the floor stands. Re-measured the sub-row: a flat 37px at 1440, 1280 and 390 alike, where it was 37/37/57 before. At 390 the table sits in a horizontal scroller, and #173's wider table gives the sub-row enough span that the prose stops wrapping. Corrected the comment, which claimed the wrap it no longer has. * fix(account): restore the character name to the remedy sub-row for AT The sub-row's prose lost its `{name}:` prefix when it moved under the character it describes, correct for sighted users since the row above already carries the name — but the table has no `<th scope="row">`, so a screen reader in table mode gets a spanning cell naming no one, and in links mode gets N identical "re-authorize" links. Restore the name as a visually-hidden prefix and pin it with a Playwright assertion. * test(account): wait on fonts before measuring the healthy fold The faulted fold test already waited on `document.fonts.ready` before measuring row geometry (a font swap moves the row height the gate counts against the fold); the healthy fold test did not, leaving its 390x844 floor of 4 unmeasured against the actual layout. Adding the wait and measuring 3 runs at each viewport gives a stable 8/6/5 with no run-to-run variance, so raise the 390x844 floor from 4 to 5. * docs(account): point the sweep note at the remedy sub-row, not the deleted block The "prose out of the table cells" bullet still cited the contactRemedies block below the Scroller, which this branch deleted — as written it read as a directive to revert the sub-row design it now describes elsewhere. Update the citation to the sub-row (page.tsx:830-853) and its measurement, keeping the real prohibition against folding the remedy back into the cell intact. * docs(account): fix the second stale reference to the deleted remedy block The re-review caught one bad line range in the bullet rewritten last commit (830-853 ends mid-props; the block runs to 863). Checking it turned up finding 6, which the rewrite missed: it cited the deleted 566-586 block and closed by saying the prose "should stay exactly where it is" -- a directive against the change this branch just made, in the only tracked doc on this area now that the round-3 spec sits under gitignored docs/superpowers/. Finding 6 itself stays open: aria-describedby is still on the <td> and is still unreliably surfaced. Only its stale structure references are corrected, plus a note that the sub-row's new visually-hidden name prefix makes the character recoverable when the description is dropped -- which softens the finding's cost but does not close it.
guarzo
added a commit
that referenced
this pull request
Aug 8, 2026
Sweep item #6 put one paragraph per non-main character under the crew manifest, explaining what pressing "make main" would do to the account's tier. But `previewMainChange` has only three outcomes, so the text has only three possible values — on a real 13-character account it rendered as thirteen paragraphs, eleven of them byte-identical after the name prefix. The comment claiming "up to nine of them can apply at once" understated it; the real ceiling is the crew size. Collapsing the duplicates was the obvious fix and the wrong one. The sentence is noise on a row nobody is acting on: it describes a hypothetical press of a button the member is not pressing, on a character that is not their main, and it landed directly on top of three rounds of density work (#169, #173, #174) whose whole point was reclaiming fold space. So the feature comes out rather than getting reformatted — including the per-page `getMainChangeContext` query that only ever backed it, which takes /account's parallel read burst from four connections back to three, and `previewMainChange` in core, whose only consumer this was. `decideTier`, which the membership job actually runs on, is untouched. The e2e assertion that scoped to the removed `.table-notes` container would have passed vacuously afterwards; it now counts the contact-remedy element directly.
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
The account manifest's horizontal-scroll gate only ever measured a healthy crew.
showStatusColumn(src/app/account/page.tsx) is exception-only, so on an all-ok seed the widest column in the manifest is not in the DOM at all — the assertion passed because the thing it was budgeting for wasn't rendered.This adds a gate that seeds a faulted crew, then removes 17px the faulted layout was paying for nothing.
The gate fails against main
Proven before any production code changed, and again against fully-reverted source:
Which fault, and why it matters
The STATUS column is content-sized, so "a faulted crew" is a range, not a width. Measured at 320px across every code
contactStateTokencan render:label_mismatchmissing_labeltoken_invalidmissing_scopeneeds_reauthThe gate seeds
needs_reauth, the widest member-fixable result. Amissing_labelseed passes at 283px and says nothing about the 299px aneeds_reauthmember actually gets — that would have been the same class of bug as the one being fixed.The 17px
The alignment (every token starting at the same x) was bought by
min-width: 5.5rem— a flat 88px — on each label. The widest of the three labels, "standings", measures 71px. A two-column grid overdisplay: contentslines sizes that column to content and takes 17px off every faulted state uniformly.What this does NOT claim
The common case clears one region-width (286px at 320px). The worst member-fixable case does not —
needs_reauthstill forces 299px. The remaining 13px is the difference between "re-auth needed" and "label needed": content, not layout. Both the CSS rule and the test say so explicitly rather than implying the gap was closed. Shortening that copy is a real lever, deliberately not taken here — the same strings render on the admin members drawer.Also noted, not gated: an unrecognized result code falls through to
result.replace(/_/g, " ")inside anowrapchip in a content-sized column, which is unbounded. That's an error path.Regression caught during review
The grid introduced one.
justify-itemsdefaults tostretch, and a grid item stretches to its track — so the borderedre-authorizecontrol grew to whatever the standings chip beside it measured: 168px next to "token refresh failed", 199px next to an unrecognized code..stacknever did this (a flex item doesn't stretch along the main axis).justify-items: startrestores its own 112.6px and leaves every measured width and scroll figure unchanged. The gate now asserts it.Constraints respected
<td>— no actions changed..char__locationuntouched, so the admin members drawer is unaffected.row-gapis still--s-2); all three fold-count tests and the 63px density budget still pass..status-line/.status-lineshave exactly one consumer, sodisplay: contentscan't leak.Verification
npx playwright test e2e/account.spec.tsnpm testnpm run test:e2enpm run typechecknpm run lintnpm run format:checknpm run buildThe helper now takes the
ContactSyncResultunion rather thanstring, so a typo'd seed is a compile error, and it assertsrowCount— a seed that faults nobody would otherwise leave the STATUS column out of the DOM and pass the width budget for exactly the reason the budget exists to rule out.Summary by CodeRabbit