Stop Prettier padding the issues ledger table, closing #133 - #1479
Conversation
#133 asked to "stop padding this table (Prettier will still render it readably)". Measuring it showed the fix was infeasible as written, because Prettier is what pads the table: stripping the padding and running `prettier --write` restored the file byte-for-byte identical to the original. Un-padding therefore requires prettier-ignoring the file, which this does — following the precedent already set for docs/branch-review-ledger.md, which is ignored for the same reason. Nothing structural is lost. check:outstanding-issues gates row shape, ids, the next-id marker and the merge attribute far more strictly than column alignment ever did, and it is required in verify:cheap and CI static-pr. The mechanism is also narrower than #133 claimed, and the row is corrected rather than left overstating it. "A single row's edit re-pads all 59 open rows" holds only when the edit raises a column's *maximum* width; an edit inside the existing maxima is 2 changed lines either way. Measured on the real file: lengthen a Detail cell padded 2 lines un-padded 2 lines widen a Summary past max padded 144 lines un-padded 2 lines That distinction is the point rather than a caveat: appending a new row is precisely the max-raising operation, and appending is what agents do to this file constantly, so the worst case was also the common case. A 144-line hunk conflicts with anything else touching the table, and each such conflict stops all CI on the PR until resolved (#116). The one-time reformat is 178 rows. It will conflict with any open PR that touches this file, so merge it when the queue is quiet rather than mid-sweep. Verified: check:outstanding-issues 144 rows (70 open, 74 archived), unique ids, next-id=147, no merge driver; docs:check-links 1407 references resolve; whole-tree prettier clean with the file now ignored. Not caused by this change: verify:cheap exits 1 on tests/installed-lock-parity.test.ts ("expected 10000000 to be less than or equal to 40000"). It fails identically on clean main with none of these changes applied, so it is pre-existing and left untouched rather than absorbed here. Everything else passed: 436 passed (437) files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 3 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. 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: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
Comment |
A real conflict, as expected: this branch reformats every row of docs/outstanding-issues.md, so any main advance that touches the table collides with it. Grafting rows is the wrong resolution for a whole-file transformation, so main's content is taken wholesale and the transformation re-applied on top — un-pad every row, then rewrite #133, which main still carried in its pre-#1479 form. That also keeps main's own ledger movement intact: 146 rows, 58 open and 88 archived, where this branch previously saw 70/74. Those are main's archivals, not losses from this resolution. Verified after resolving: check:outstanding-issues 146 rows (58 open, 88 archived), unique ids, next-id=149, no merge driver; docs:check-links 1414 references resolve; whole-tree prettier clean with the file ignored; zero padded rows remain. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ
Second conflict in minutes, same structural cause: this branch reformats every row, so any main advance touching the ledger collides with it. Same deterministic resolution — take main content, re-apply the transformation. Main movement preserved: 146 rows, 57 open / 89 archived. Verified: check:outstanding-issues 146 rows, unique ids, next-id=149, no merge driver; whole-tree prettier clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ
…onflict loop Third consecutive real conflict blocking this PR's CI, always the same file, and the fix is to stop touching it rather than to resolve it a fourth time. main's newest commit is `fix(ledger): stop Prettier padding the issues table, closing #133 (#1479)` — a whole-file reformat of docs/outstanding-issues.md. Every row changed shape, so any edit to any row conflicts. That file is also touched by nearly every main PR (row appends, archival moves), and this branch is competing with a queue that merges several times an hour. This PR's entire stake in it was ONE cosmetic line: a note on #146 saying the cited spec moved to ui-phone-scroll-page-owned.spec.ts in the split. Three CI-blocking conflicts — each one stopping GitHub building refs/pull/1466/merge so no pull_request workflow ran at all — is a bad trade for that. So the file is taken from main verbatim and the note is not re-applied. This PR's diff no longer contains docs/outstanding-issues.md, which removes its only remaining contact point with the hottest file in the repo. Cost, stated rather than hidden: #146 keeps citing the old path. That row already records that the exact test title is the durable identity because declaration lines drift, so the stale path misleads nobody who reads it. The note belongs in a docs-only PR when the ledger is not mid-reformat. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JHLPEV4o1rzipPDqshCSHY
The conflict was TWO lines — my side removed the open #130 row because it moved to the archive table, main kept it. Taking my side; #130 remains a single 5-cell archived row, #149 and #150 remain 7-cell open rows, and main had claimed neither id (its marker was 149). Worth recording the size: this same conflict before PR #1479 would have been the whole 148-row table, because one row edit re-padded every row. Un-padded it is 2 lines. That is the first conflict since #1479 landed and it behaved exactly as the change predicted. Verified: check:outstanding-issues 148 rows (57 open, 91 archived), unique ids, next-id=151, no merge driver, no ids deleted from base; prettier clean; no conflict markers remain. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ
…ge gaps (#1496) * issues: close #130 by decision; capture the stale-install and reviewer gaps #130 is archived because the owner decided not to pursue it, NOT because the acceptance was run. The row says so in its first sentence, because a "CLOSED" line is exactly the thing a future reader could mistake for evidence: docs/phone-chrome-physical-acceptance.md on main is still the blank template, and nothing in CI covers what it covers — headless Chromium was explicitly stated as unable to certify Safari chrome-minimisation or cold-launch PWA paint. Residual risk is unchanged and unmeasured, and the row says to open a fresh one if phone-chrome misbehaves on a real device. Two risks that surfaced during today's work were never written down, and neither had an existing row (checked before adding, after creating a duplicate earlier today by not checking): #149 — the SessionStart hook reported "node_modules matches the lockfile" while brace-expansion was installed at 1.1.16 against a lockfile pinning 1.1.18, the CVE-2026-14257 patch. Every local gate run in that window was weaker than reported, including verify:cheap runs called green during #1430/#1444. It surfaced only because installed-lock-parity asserts the patched behaviour directly, and its failure was first misread as a pre-existing repo defect because it also failed on clean main in the same stale container. Notes that check:installed-lock-parity is only in verify:ui, so the broad local gate cannot catch its own stale foundation. #150 — CodeRabbit reviewed none of #1404, #1430, #1444, #1445 or #1479; every comment was a spending-cap rate-limit notice that renders as ordinary bot activity. Codex was the sole substantive reviewer and found three real defects that survived local gates and self-review. Fast checks run against this tree: check:runtime, check:installed-lock-parity (now passing, confirming the drift is resolved), check:outstanding-issues, check:branch-review-ledger and check:migration-role all PASS. check:supabase-project and check:production-readiness fail only on absent provider credentials, which AGENTS.md states is expected in demo mode. Verified: check:outstanding-issues 148 rows (57 open, 91 archived), unique ids, next-id=151, no merge driver, no ids deleted from base; docs:check-links clean; whole-tree prettier clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ * issues: correct #149 — parity check scope, not its placement Codex review is right on both counts and this rewrites the row rather than patching around it. The earlier text said check:installed-lock-parity lives only in verify:ui and proposed adding it to verify:cheap; verify:cheap:internal already runs it, so that follow-up was a no-op — and it ran green throughout the stale window. The real gap is scope. criticalInstalledPackages lists seven top-level names (next, react, react-dom, eslint, playwright, typescript, vitest), so a stale transitive dependency is invisible. Proved by faking node_modules/brace-expansion back to 1.1.16 against the 1.1.18 lockfile pin: check:installed-lock-parity exits 0. That also explains why both the hook and the gate reported parity while the tree was stale — neither was looking at the package that drifted. Next step is now to broaden the check (full tree, or a lockfile-hash install stamp) rather than move it, and the row warns against reading a green parity run as proof the install is current. Verified: check:outstanding-issues 148 rows, unique ids, next-id=151, no ids deleted from base; docs:check-links 1413 references; prettier clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ --------- Co-authored-by: Claude <noreply@anthropic.com>
* issues: capture the unreadable-CI token, at-risk worktree work, and the unpushed hook fix Three findings from the 2026-07-30 organisation session that were recorded nowhere durable: - #149 the session GitHub PAT lacks Checks: Read, so no agent can confirm a PR is green. The endpoint that does work returns an empty result rather than an error, so it reads like an absence of checks rather than an absence of permission. - #150 four worktrees on already-merged branches hold uncommitted work that exists in no branch and no PR, the largest being +395/-200 across 19 files including CI config. - #151 the pre-commit fail-open for #143 lives only on a never-pushed local branch, which is also 17 behind main and conflicts on the file whose count sentence main's new docs:update generator now owns. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(ledger): record the session-followup capture review for PR #1490 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(ledger): record #143/#151/#149 reconciliation for PR #1490 Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * docs(ledger): supersede PR #1490 reconciliation after remote sync Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * issues: record the worktree snapshots and redirect #151 to PR #1494 #150 — the four at-risk worktrees were snapshotted onto their own already-merged branches (748ef018f, 5dbd9f965, b7eae51a4, d949859c3), so the work survives a worktree reclaim. All four are clean now. None is pushed or reviewed; the next action is per-snapshot promote-or-reset. #151 — the never-pushed branch is superseded rather than salvageable: its script and hook reached main by other routes, so the fail-open guard was applied to main's committed hook in PR #1494 instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: remove credential metadata and correct audit dates * docs: consolidate session follow-up findings * docs: record consolidated follow-up review * issues: record that #101 hydration shipped PR #1463 merged as dba7356, so #86's "Next X3 unit — rag-hydration.ts" is now stale. The row records the extraction as shipped and keeps the corrected boundary: hydration re-homed only two of prepareCoverageGateResults's five rag.ts-only dependencies, so it did not unblock that function — exactly as the Codex review on PR #1461 predicted. This row was deliberately dropped from #1463 itself (commit 6290d02) after docs/outstanding-issues.md conflicted on five consecutive main syncs. Recording it separately here is the same pattern used for #1454 via #1461. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GGEBHp4Seoh1jK1vGTNtYS * docs(ledger): record the landed X3 hydration review Appended with npm run ledger:append (never hand-written), keyed to the squash commit dba7356 so ledger:lookup can resolve it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GGEBHp4Seoh1jK1vGTNtYS * docs: fix the #101 mislabel and key the ledger row to a resolvable ref Both defects were raised by Codex on PR #1495 and both are real; verified against the files before accepting. 1. #101 is NOT this extraction. docs/outstanding-issues.md:138 shows #101 is "Canary-gated retrieval parallelisation candidates" (P3, rec) — a separate, still-open recommendation gated on a live canary pair. Calling the hydration extraction "#101" marked that unrelated work as shipped and could have caused the live-evaluation work to be skipped. The label came from the original task brief and was propagated without checking it against the ledger. Both the #86 row and the X3 work-order entry now identify the change as the X3 hydration unit (PR #1463) instead. #101's own row is untouched and still open. 2. The ledger row did not resolve. `npm run ledger:lookup -- dba7356` returned NOT REVIEWED, because the ref cell held only the slash-form branch token and that branch no longer resolves locally, so the throttling record could not prevent a repeat review. Appended a superseding record keyed to the landed SHA; the same lookup now returns ALREADY REVIEWED. The original row is retained, per the ledger's append-only rule. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GGEBHp4Seoh1jK1vGTNtYS * docs: record consolidated PR reviews * docs: record ingestion recovery review * docs(visual): document the platform-scoped baseline layout and how to seed it `playwright.visual.config.ts` records snapshots under `__screenshots__/{platform}/`, so a baseline taken on Windows lands in `win32/` and is never consulted by the `ubuntu-24.04` CI job, which reads `linux/`. Nothing said so, and committing `win32/` images looks like protection while providing none. Records the constraint, names the CI artifact as the supported recorder for `linux/` baselines, and notes that comparison stays advisory until the jobs come off `continue-on-error`. Also creates the tracked directory `.gitignore` already claims exists, which sets `ui_changed=true` (`scripts/ci-change-scope.mjs`) so the visual job can run and produce that first artifact. No baselines are added here — they cannot be produced on this platform. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: correct visual baseline adoption steps * docs: record visual baseline guidance review * fix(ui): repair mockup accent token references * docs: record token-reference repair review * docs: archive advisory UI scoping task * docs: record advisory UI closure review * issues: archive #151 after #1494 and mark #143 fully resolved PR #1494 landed the fail-open guard on main, so close the open salvage row and update the #143 archive from PARTIAL to resolved across #1442 and #1494. Also carries the merge of origin/main that cleared the GitHub DIRTY mergeability state. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * docs(ledger): record PR #1490 main-sync and #151 closeout Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * docs(ledger): record #1496 id-collision renumber for PR #1490 Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * issues: record the withdrawn live-region finding as #151 so it is not re-filed Archive-only row. There is no defect and no work to do — the row exists purely as a guard rail against repeating a misreading that already happened once. search-results-header-band.tsx sets aria-live={faulted ? "off" : "polite"} on its count/status span, which reads like a silenced failure announcement. It is not: the band mounts a separate fault panel with role="alert" carrying the failure title, body and Retry, and the mute is deliberate so the two do not both speak. The reasoning is in a comment directly above the attribute, and tests/search-results-header-band.dom.test.tsx pins it with singular role queries that throw on duplicates. During session 2026-07-30 (PR #1481) this was filed as a real P2 defect on the strength of the attribute alone, and the proposed fix — escalating the count span to role="alert"/aria-live="assertive" — would have produced a duplicate announcement and a red test, making it worse than no change. Codex caught it. An earlier withdrawal row was then lost to the squash that merged #1481, which is the row-deletion shape #148 now guards against. Also records that the mockup's escalation is correct in the mockup and must not be ported: search-refine-adaptive-mockups.tsx has no fault panel, so there the count span is the only announcement channel. #148 needed no work — the merge-base deletion check landed on main independently, and its output now reports the base it compared against. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JdPa3mHCX5ZQZZvU5GHU3r * docs(rag): record refuted lexical probe collapse (#98) * issues: capture the residual id-allocation hazard as #151 #133 is resolved: #1444 removed merge=union and #1479 excluded the ledger from Prettier, which together fixed conflict frequency. Neither changes id allocation, which is still read-modify-write against the next-id marker, so concurrent branches still claim the same number. Measured on PR #1451: one row was renumbered #135 -> #141 -> #145 -> #147 -> #149 across four sync cycles. The sharper finding is that GitHub's Update-branch button resolved one such collision into duplicate #141 rows with the marker left below main's highest id — git reported success and only check:outstanding-issues caught it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(issues): attribute the mobile CLS breach — a 128px reserve round trip #147 asked which elements shift. Driving Chromium against the same offline production build with a PerformanceObserver on layout-shift (Lighthouse mobile emulation, reading entry.sources[].node) gives one dominant cause on all four breaching routes: the entire main content region moves down 128px and straight back up 128px within 15-60ms. Both moves score, so it is pure cost with zero net movement — 100% of /documents/search's 0.220 and about 75% of /dsm's. The shifting element is the max-sm:pt-[var(--phone-overlay-chrome-h)] wrapper around <main>. A MutationObserver timeline on the root style attribute pins the mechanism rather than inferring it: the property goes CSS seed -> 200px -> 72px, and the 200px is written when the header stack ALREADY measures 72px (t=1552ms reserve=200px stack=72, corrected at t=1612ms). usePhoneOverlayChromeReserve reads stack.offsetHeight while the stack is transiently tall, publishes a value that is stale by the time it lands, and its ResizeObserver then corrects it. The CSS seed at globals.css:375 is correct for the settled stack, which corrects the mechanism recorded on the now-archived #130 — that framed the defect as the seed under-reserving by 0-8px. Measured, the driver is a 128px transient over-reserve written by the hook, not the seed. / is the control: it never writes the property and is the one clean route. Variance is stated rather than smoothed: /dsm measured 0.363 and 0.219 across two runs, and this harness has no network throttling so /forms and /therapy-compass run high locally. Only /dsm, /documents/search and / reproduced the live dispatch exactly. Also recorded: attaching a MutationObserver to document.documentElement inside a Playwright addInitScript throws before the document element exists, silently killing the CLS observer and reporting a uniform CLS=0.000 — a false clean bill that voided one run of this harness. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01361jh3eYVjJCzXWjAhdZiF * docs(ledger): record the #151 capture review for PR #1506 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(review): clarify snapshot branch state * docs(ledger): record PR #1490 main sync after snapshot wording Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * docs: archive rendered style contract task * docs: record style contract closure review * docs: record synced style contract review * docs: record post-121 style closure review * docs: normalize style review ledger after sync * docs: record post-1490 style closure review * docs: record consolidated PR 1490 review * docs: record replacement consolidation review * docs: record reconciled consolidation review * docs: record post-1511 consolidation review * docs: normalize PR 1510 ledger after main sync * docs: record PR 1510 post-sync review * docs: correct false #98 canary evidence and NOTES triage Remove the incorrect probe-collapse canary attribution from #98 and point the unread --med-accent-soft note at #157 without breaking the seven-token TOKENS_MISSING accounting. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * docs(ledger): record PR #1510 evidence-correction review Supersede the prior approve-with-no-findings row after correcting the false #98 canary attribution and NOTES triage drift. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * docs: keep concurrency note inside issue table * docs: record post-1513 consolidation review * docs: address CodeRabbit notes on PR #1510 Fix the computed-value-time wording in design-sync notes, give #33 a unique recommended-queue order, and drop the duplicated #98 Done block. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * docs(ledger): record PR #1510 CodeRabbit fix review Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Summary
#133— the fixed-width column padding that turns a ledger edit into a whole-table hunk. Remove merge=union from the issues ledger, per its own #133 #1444 closed themerge=unionhalf; this closes the padding half.#133's stated fix was infeasible as written, and measuring it is what showed why. It asked to "stop padding this table (Prettier will still render it readably)". Prettier is what pads the table: stripping the padding and runningprettier --writerestored the file byte-for-byte identical to the original. Un-padding therefore requires prettier-ignoring the file.docs/branch-review-ledger.mdis already in.prettierignorefor the same class of reason. This addsdocs/outstanding-issues.mdbeside it, with the rationale written down.#133's own claim rather than leaving it overstated (see below).Measured, not asserted
#133says "a single row's edit re-pads all 59 open rows." That holds only when the edit raises a column's maximum width. An edit inside the existing maxima is 2 changed lines either way. Measured on the real file:That distinction is the point, not a caveat: appending a new row is precisely the max-raising operation, and appending is what agents do to this file constantly — so the worst case was also the common case. A 144-line hunk conflicts with anything else touching the table, and each such conflict stops all CI on the PR until resolved (
#116).Nothing structural is lost
Prettier's only contribution to this file was column alignment. Row shape, ids, the
issues:next-idmarker and the merge attribute are all gated bynpm run check:outstanding-issues, which is far stricter than a formatter and is required in bothverify:cheapand CIstatic-pr. A gate's job, not a formatter's.Verification
npm run check:outstanding-issues—144 rows (70 open, 74 archived), unique ids, next-id=147 above the highest, no merge driver.npm run docs:check-links—1407 repo path references resolve.npx prettier --check .— clean, with the file now ignored.prettier --write→diffagainst the original reports identical.Pre-existing failure, not caused by this change:
npm run verify:cheapexits 1 ontests/installed-lock-parity.test.ts—expected 10000000 to be less than or equal to 40000. I verified it fails identically on cleanmainwith none of these changes applied, so it is left untouched rather than absorbed into this PR. Everything else passed:436 passed (437)files.Risk and rollout
src/, no gate logic, no runtime behaviour.npm run format, restoring the previous state exactly.Clinical Governance Preflight
Not applicable — documentation and formatter configuration only.
scripts/pr-policy.mjsclassifies no clinical-risk path, nosupabase/**, nosrc/app/api/**, and no RAG-ranking protected surface, so noRAG impact:declaration is required.Generated by Claude Code