Revert #135: API contract semantics and pagination metadata - #149
Merged
Conversation
BigSimmo
enabled auto-merge
July 2, 2026 09:21
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:524b4b8b1d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| try { | ||
| const { id: rawId } = await params; | ||
| const { id } = parseRouteParams({ id: rawId }, labelsRouteParamsSchema, "Invalid document id."); | ||
| const { id } = await params; |
There was a problem hiding this comment.
When the path id is not a UUID, e.g. /api/documents/not-a-uuid/labels, this now passes the raw string into requireOwnedDocument, which filters documents.id even though supabase/schema.sql defines that column as uuid. Postgres/PostgREST rejects that as an input-syntax error, and this route's jsonError(error) default turns it into a 500 instead of the prior 400 validation response; PATCH and DELETE have the same raw-param path. Please validate the route id before any Supabase query.
Useful? React with 👍 / 👎.
| } | ||
| return jsonError(error); | ||
| return jsonError(error, 400); |
There was a problem hiding this comment.
When a valid upload reaches this catch because Supabase storage, document insert, or job insert fails, the thrown ordinary Error is passed to jsonError(error, 400), so the client sees a 400 client-input failure instead of a 5xx server persistence failure. That breaks status-based retries/alerts for real backend outages; validation failures already use PublicApiError, so unexpected errors should keep the default 500 behavior.
Useful? React with 👍 / 👎.
| return NextResponse.json({ error: "Document not found." }, { status: 404 }); | ||
| } | ||
| return jsonError(error); | ||
| return jsonError(error, 400); |
There was a problem hiding this comment.
When the request is valid but summarizeDocument fails because Supabase or the model provider is unavailable, this catch now sends ordinary Errors through jsonError(error, 400), so clients see a non-retryable bad request rather than the server/backend failure it is. The route already special-cases authentication and missing documents, so unexpected summary-generation failures should keep the default 500 envelope.
Useful? React with 👍 / 👎.
| .eq("documents.owner_id", user.id) | ||
| .order("created_at", { ascending: false }) | ||
| .range(offset, offset + limit - 1); | ||
| .limit(100); |
There was a problem hiding this comment.
For users with more than 100 ingestion jobs, /api/ingestion/jobs?offset=100 now ignores the requested page and always returns only the newest 100 rows. The dashboard's indexing monitor and retry controls are fed from this endpoint, so older failed jobs become unreachable from the API/UI once a busy workspace has enough newer jobs; restore the limit/offset handling and pagination metadata instead of hard-capping the feed.
Useful? React with 👍 / 👎.
| try { | ||
| const { id: rawId } = await params; | ||
| const { id } = parseRouteParams({ id: rawId }, summarizeRouteParamsSchema, "Invalid document id."); | ||
| const { id } = await params; |
There was a problem hiding this comment.
When the document id in the path is malformed, this now accepts the raw value and reaches consumeApiRateLimit before summarizeDocument eventually rejects the UUID filter. That means invalid URLs can consume the user's document_summarize quota, and a rate-limited user can get 429 instead of the validation error for a bad id. Validate the route param before demo/auth/rate-limit work as the previous schema boundary did.
Useful? React with 👍 / 👎.
| } catch (error) { | ||
| if (error instanceof AuthenticationError) return unauthorizedResponse(); | ||
| return jsonError(error); | ||
| return jsonError(error, 400); |
There was a problem hiding this comment.
For a valid reindex request, failures while reading the document, updating its status, or inserting the ingestion job now pass ordinary Errors to jsonError(error, 400), making a backend/Supabase outage look like a client bad request. The reindex UI and operators need to distinguish retryable server failures from invalid input, so unexpected reindex errors should keep the default 500 response.
Useful? React with 👍 / 👎.
| .eq("owner_id", user.id) | ||
| .order("created_at", { ascending: false }) | ||
| .range(offset, offset + limit - 1); | ||
| .limit(20); |
There was a problem hiding this comment.
For workspaces with more than 20 import batches, /api/ingestion/batches?offset=20 now ignores the requested page and always returns only the newest 20 rows. The indexing monitor builds its active/failed batch view from this feed, so older failed batches can disappear from the API/UI once enough newer batches exist; restore limit/offset handling and pagination metadata instead of hard-capping the query.
Useful? React with 👍 / 👎.
BigSimmo pushed a commit
that referenced
this pull request
Jul 30, 2026
Third ledger collision in a row, same shape as the last two: main allocated #146 for the `ui-phone-scroll` Services anchor issue while this branch used #146-#148 for the answer-home notice capture. Main's row is kept and the incoming rows renumber to #147 (adopt a consolidated answer-home notice block), #148 (answer mode ships no verify-before-use caveat) and #149 (verify:pr-local exits 0 when its build step refuses to run), with their cross-references and the marker (150) following. Verified against origin/main that #141-#146 each still appear exactly once; nothing was taken wholesale from either side. check:outstanding-issues: 147 rows (74 open, 73 archived), unique ids, next-id=150 above the highest, no merge driver. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NPyFcMfn1jMmphr6AqiWBg
BigSimmo pushed a commit
that referenced
this pull request
Jul 30, 2026
The three captured rows are dropped from docs/outstanding-issues.md, leaving it byte-identical to origin/main. They will be re-landed as their own single-file change once this PR merges. AGENTS.md recommends bundling append-only ledger rows because they are normally zero-risk, but that assumes a quiet file. Right now it is the hottest file in the repo: bundling them here cost three ID collisions and three full CI restarts in about thirty minutes (main took #141-#144, then #145, then #146, while this branch needed #145-#149 in turn). #133 already records that this file conflicts on nearly every main advance. The mockups diff itself touches four uncontended files and has not conflicted once today, so removing the ledger rows takes this PR out of a race it has no reason to be in. No content is lost: the row text is preserved verbatim and re-applied against a fresh main with clean ids. check:outstanding-issues: 144 rows (70 open, 74 archived), unique ids, next-id=147 above the highest, no merge driver. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NPyFcMfn1jMmphr6AqiWBg
2 tasks
BigSimmo pushed a commit
that referenced
this pull request
Jul 30, 2026
… was worse Codex caught this on PR #1481 and it is correct. I filed #149 claiming a failed clinical search is announced to nobody because the count span sets aria-live="off" when faulted. I never checked whether another node makes the announcement. It does. search-results-header-band.tsx mounts a fault panel with role="alert" carrying the failure title, body and Retry (lines 407-414), and the mute is deliberate, documented in place: "While faulted the live region is silenced (aria-live='off') and the freshly-mounted fault role='alert' below makes the single announcement, rather than both speaking." tests/search-results-header-band.dom.test.tsx already pins exactly that with singular role queries that throw on duplicates. Escalating the count span in production, as #149 recommended, would have added a second alert beside the fault panel — a duplicate announcement and a broken test. The row is withdrawn to the archive rather than deleted, with the reasoning recorded so nobody re-files it. The mockup is unaffected: search-refine-adaptive-mockups.tsx has no fault panel, so there the count span is the only announcement channel and its escalation is correct. It simply does not port to production. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JdPa3mHCX5ZQZZvU5GHU3r
BigSimmo pushed a commit
that referenced
this pull request
Jul 30, 2026
…ecord The two Services viewport-anchor data points from PR #1457 were dropped when the withdrawn #149 row and its id allocation were restored. They are unrelated to that decision, so this puts them back and changes nothing else. #149 stays exactly as set: archived as a withdrawn record, with issues:next-id preserved at 150 so the id is retired rather than reused. Restored to #146: the test failed once more on head c739340 (anchorTop expected -138, received -7) then passed on 9da02d9 and a6f2281 across all three shards with the spec byte-identical — six data points, two failures, shard 1 only, both failures on a mockups-only PR. Notes that the 131px delta is roughly 2x the 64px viewport shrink rather than sub-pixel drift, which constrains the element attribution that row already asks for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JdPa3mHCX5ZQZZvU5GHU3r
BigSimmo added a commit
that referenced
this pull request
Jul 30, 2026
…#1481) * issues: capture the production live-region defect; record the landed #1457 review Adds #147 — `search-results-header-band.tsx` sets aria-live="off" when faulted, so a clinical search that fails while focus is elsewhere is announced to nobody and the user is never told Retry appeared. Affects all twelve call sites that render the band. Raised by Codex against the copied line on PR #1457, fixed there, and confirmed to exist unchanged in production. The merged mockup carries the proven pattern and a non-vacuous assertion to port. Appends two further data points to #146 from PR #1457: the Services viewport anchor failed once more on head c739340 (expected -138, received -7) and then passed on two later heads with the diff byte-identical — six data points, two failures, shard 1 only. Both failures landed on a PR touching only mockups, which strengthens the unchanged-code reading. Notes that the 131px delta is roughly 2x the 64px shrink rather than sub-pixel drift. Records the prlanded verification for PR #1457 in the branch review ledger: squash e79e499, content diff against the branch tip empty, and the late aria-live/role="alert" commit confirmed present on main rather than orphaned by the squash. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JdPa3mHCX5ZQZZvU5GHU3r * issues: withdraw #149 — the live-region defect was false, and the fix was worse Codex caught this on PR #1481 and it is correct. I filed #149 claiming a failed clinical search is announced to nobody because the count span sets aria-live="off" when faulted. I never checked whether another node makes the announcement. It does. search-results-header-band.tsx mounts a fault panel with role="alert" carrying the failure title, body and Retry (lines 407-414), and the mute is deliberate, documented in place: "While faulted the live region is silenced (aria-live='off') and the freshly-mounted fault role='alert' below makes the single announcement, rather than both speaking." tests/search-results-header-band.dom.test.tsx already pins exactly that with singular role queries that throw on duplicates. Escalating the count span in production, as #149 recommended, would have added a second alert beside the fault panel — a duplicate announcement and a broken test. The row is withdrawn to the archive rather than deleted, with the reasoning recorded so nobody re-files it. The mockup is unaffected: search-refine-adaptive-mockups.tsx has no fault panel, so there the count span is the only announcement channel and its escalation is correct. It simply does not port to production. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JdPa3mHCX5ZQZZvU5GHU3r * docs: record PR 1481 review * docs: preserve issue 149 allocation * docs: format withdrawn issue record * issues: restore the #146 data points lost while preserving the #149 record The two Services viewport-anchor data points from PR #1457 were dropped when the withdrawn #149 row and its id allocation were restored. They are unrelated to that decision, so this puts them back and changes nothing else. #149 stays exactly as set: archived as a withdrawn record, with issues:next-id preserved at 150 so the id is retired rather than reused. Restored to #146: the test failed once more on head c739340 (anchorTop expected -138, received -7) then passed on 9da02d9 and a6f2281 across all three shards with the spec byte-identical — six data points, two failures, shard 1 only, both failures on a mockups-only PR. Notes that the 131px delta is roughly 2x the 64px viewport shrink rather than sub-pixel drift, which constrains the element attribution that row already asks for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JdPa3mHCX5ZQZZvU5GHU3r --------- Co-authored-by: Claude <noreply@anthropic.com>
BigSimmo pushed a commit
that referenced
this pull request
Jul 30, 2026
Second time a real conflict has blocked this PR's CI entirely — GitHub could not build refs/pull/1466/merge, so no pull_request workflow ran and the thin check list read as pending rather than blocked (#116). Only docs/outstanding-issues.md conflicted; scripts/ci-change-scope.mjs and docs/process-hardening.md auto-merged (main's regions are 150+ lines from this branch's). Took main's ledger wholesale rather than hand-editing a 140-row table around conflict markers. Only ONE of this branch's two ledger edits was re-applied: - #146 keeps its relocation note. The row is still open, and main's #1481 added two further data points to it (head c739340, anchorTop expected -138 received -7; six data points, two failures, shard 1 only) which are left untouched. - #127's edit is DROPPED as obsolete. Main's #1487 archived that row, and the archived form no longer cites tests/ui-phone-scroll.spec.ts at all, so there is nothing left to relocate. Re-applying it would have matched nothing or corrupted a differently-shaped row. Marker is main's 149 (not this branch's 147): #149 was allocated, withdrawn and retired rather than reused. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JHLPEV4o1rzipPDqshCSHY
BigSimmo added a commit
that referenced
this pull request
Jul 30, 2026
Main resolved#148 via #1491 and moved it to the archive table, so its side of the open-items hunk was empty. Resolution keeps main's archival and re-adds only #149, #150 and #151; the next-id marker stays at 152, which is the value this branch allocated and is still above main's highest. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
BigSimmo pushed a commit
that referenced
this pull request
Jul 30, 2026
Re-lands the rows that were reverted out of this PR earlier, now that auto-merge is off and there is no merge race to lose them to. Recording them here makes them durable: they were otherwise held only in an ephemeral session and in pre-revert commits that the squash-merge would make unreachable. #149 task — adopt a consolidated answer-home notice block. The hero stacks three notices in three type sizes with two opposing shield icons, and the least important reads loudest. Adoption moves PrivacyInputNotice, the single site-wide APP-5 line, across the answer, documents and calculators composers, so it needs a full clinical governance preflight rather than a design review. #150 issue — answer mode ships no verify-before-use caveat while differentials, prescribing, specifiers and calculators all do. Stands independently of any redesign. #151 issue — verify:pr-local exits 0 when guard-next-build refuses to run, reporting green with the production build never executed. Ids allocated above main's current highest (#148); the marker moves to 152. The three cross-references between the rows were updated to match. check:outstanding-issues: 149 rows (60 open, 89 archived), unique ids, next-id=152 above the highest, no merge driver. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NPyFcMfn1jMmphr6AqiWBg
BigSimmo pushed a commit
that referenced
this pull request
Jul 30, 2026
main landed its own #133 fix, which strips the Prettier column padding from the issues ledger. That reformats every row, so the whole table conflicted rather than just the rows anyone edited — the last and largest instance of exactly the problem #133 exists to end. Resolved by taking main's unpadded table wholesale and re-emitting this branch's three captured rows in the same format (#149 adopt a consolidated answer-home notice block, #150 answer mode ships no verify-before-use caveat, #151 verify:pr-local exits 0 when its build step refuses to run). Taking one side wholesale is safe here only because this branch's sole contribution to the file is those three rows; main's own rows are unmodified. check:outstanding-issues: 149 rows (59 open, 90 archived), unique ids, next-id=152 above the highest, no merge driver, no ids deleted from base 00ddb22 — main's new #148 gate confirming nothing was dropped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NPyFcMfn1jMmphr6AqiWBg
BigSimmo added a commit
that referenced
this pull request
Jul 30, 2026
edits Another session reconciled the same three rows while this one snapshotted the worktrees. Resolution keeps this side for #149 and #150 (theirs carried no snapshot SHAs) and unions #151: their PR #1442 provenance plus the correction that the archived #143 row implied the fail-open was durable when only the hook and script had landed, kept alongside this side's redirect to PR #1494. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
BigSimmo pushed a commit
that referenced
this pull request
Jul 30, 2026
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
BigSimmo pushed a commit
that referenced
this pull request
Jul 30, 2026
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
BigSimmo added a commit
that referenced
this pull request
Jul 30, 2026
…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>
BigSimmo pushed a commit
that referenced
this pull request
Jul 30, 2026
Fourth ledger id collision on this branch: main allocated #149 (check:installed-lock-parity only compares seven top-level packages) and #150 (CodeRabbit reviewed none of a full day's PRs) while this branch held #149-#151 for the answer-home notice capture. main's rows are kept and the incoming rows renumber per the ledger's own rule: #151 adopt a consolidated answer-home notice block, #152 answer mode ships no verify-before-use caveat, #153 verify:pr-local exits 0 when its build step refuses to run. Their three cross-references and the marker (154) follow. check:outstanding-issues: 151 rows (58 open, 93 archived), unique ids, next-id=154 above the highest, no ids deleted from base 4312a21. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NPyFcMfn1jMmphr6AqiWBg
BigSimmo added a commit
that referenced
this pull request
Jul 30, 2026
…closed (#1508) PR #1490 was closed unmerged, so none of its content reached main. Confirmed by content rather than id: main's #149 and #150 are unrelated rows (installed-lock parity and CodeRabbit rate limits) that happened to take those ids, so an id-presence check reported them as landed when they were not. - #151 corrects the earlier claim that CI is unreadable. The PAT lacks Checks: read but has Actions: read, so workflow runs are queryable; the endpoint that looked authoritative returns an empty result rather than an error, which is what made it read as a hard wall. - #152 re-lands the at-risk worktree inventory together with the four preservation snapshots taken on 2026-07-31, which existed in no other record. - #153 archives the pre-commit fail-open as resolved by PR #1494. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
BigSimmo added a commit
that referenced
this pull request
Jul 31, 2026
* 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>
BigSimmo added a commit
that referenced
this pull request
Jul 31, 2026
…s fixed (#1529) Several comprehensive-audit-* worktrees carry an uncommitted row archiving #149 as resolved, asserting check:installed-lock-parity now validates every concrete package location including nested transitives. That fix exists nowhere: not on main, not in any of the 42 worktrees, and not in any open PR. main still reads criticalInstalledPackages = ["next","react","react-dom","eslint","playwright", "typescript","vitest"]. Without this note the next session to sync those worktrees would archive a live issue on a claim the code contradicts. The sibling #103 disposition from the same worktrees was verified true and landed in #1528; this one was not. Row edited with npm run issues:update, not by hand. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
BigSimmo added a commit
that referenced
this pull request
Jul 31, 2026
…cade-ported, forced-colours owned (#1538) * feat(design-system): v2 token layer, 26 components, browser-crash fix Design-system scope only. Nothing is adopted by a product surface: the new components are built and tested but unimported, and the v2 token layer is class-scoped to `.ckb-v2` so importing it cannot repaint any surface that does not opt in. Correctness: - Fix a P1 browser crash. `source-metadata.ts` imported the server logger, whose `activeLevel()` reads `process.env.LOG_LEVEL` — a ReferenceError in a browser, so any off-vocabulary metadata value unmounted the whole React tree instead of falling back. Drops the import, adds a browser-safe diagnostic seam, and guards the env read in `logger.ts` as defence in depth. - Type the three source badges' `metadata` prop. It was `unknown` while the published `.d.ts` promised a shape, so the wrong key compiled cleanly. - Encode the disabled state in `controlBase` instead of `opacity-50`, which dimmed fill and label together and dropped secondary labels below 4.5:1. - Move `fieldLabel` off `--text-soft` (3.07:1) and the uppercase eyebrow treatment. - Delete `ui/card.tsx` and `ui/badge.tsx` — 7 exports, 0 importers, colliding by name with Chip and AnswerCard. Components (built, not adopted): Button, TextField, SearchField, Chip, Toast, Tabs, Tooltip, Pagination, ConfirmDialog, PageHeader, Breadcrumb, AnswerCard, DoseLine, AnswerFooter, Quantity, StatusMark, Citation, the four Link variants, Select, Checkbox, RadioGroup, Disclosure, Progress, StageList. AccessibleTable: per-column numeric alignment, `aria-controls` on the expander, sticky header when expanded, and an explicit unverified-extraction treatment. Verification: tsc pass, lint pass, prettier clean, verify:ui 344 passed, design-system contract pass. Unit 4689 passed / 1 failed / 3 skipped — the failure is `installed-lock-parity`, environmental (brace-expansion 1.1.16 vs lockfile 1.1.18), tracked as issue #149. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(design-system): reconcile v2 token file as canonical (C1), dark fall-through, leading step C1 reconciliation per docs/design-system/TOKENS.md - the design-side token file proved unrecoverable (design project 08d6f126 was last updated 2026-07-13; the 31 July copy was never written to it), so the repo file becomes the canonical reconciled copy: - Rename the two v2 --shadow-inset overrides to --shadow-well (recessed-well role). The DS bevel --shadow-inset is inherited from globals.css again, restoring the bevel highlight to its ~40 consumers. - Update the token contract test in the same commit: pin --shadow-well as a true inset and assert --shadow-inset is not redeclared in the v2 layer. - Author the evidence-spine and status-mark role families repo-side, derived from existing roles (spine from rule/accent/warning, status-mark from the gutter dot and StatusMark's previously-inline stroke). Confidence-meter is deliberately absent: no call site exists yet. - Add --clinical-chat-document to the v2 dark block so it resolves against the v2 dark inset; custom properties inherit computed values, so the live .dark declaration cannot re-resolve inside the subtree. - answer-card.tsx: use the named leading-prose step instead of the arbitrary leading-[var(--leading-prose,1.65)], which tripped the design-token contract ('has no arbitrary leading-[...] left in production') and restated a token value in markup. Verified: vitest tests/ckb-v2-token-contract.test.ts tests/design-token-contract.test.ts -> 47 passed (47). No adopted surface exists, so there is no visual change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(design-system): cascade port, forced-colours block, danger contrast, AsyncButton type Playbook PR 1+2 and the smallest slices of PR 3+4 (docs/design-system/SPEC.md §13): - Cascade: light is the default on .ckb-v2 itself; dark overrides via .dark .ckb-v2, .ckb-v2.dark. Never :not(.dark) (DS-01) - it matched a subtree inside html.dark and overrode inherited dark values. Contract test's block parser updated in the same commit (collects both .ckb-v2 blocks and the grouped dark selector). - Forced colours: v2 layer ships its own forced-colors block over all three selectors, last in file (C4/DS-02); roles map to system colours, elevation flattens to none. - Button.danger pairs --danger-solid-contrast (DS-03; dark was 2.87:1, the paired token is 6.57:1 and HCM-mapped). - AsyncButton defaults type=button so it can no longer submit a surrounding form (DS-04 adjacent; retirement to Button remains PR 4). Verified: vitest ckb-v2-token-contract + design-token-contract + ui-v2-components.dom + ui-primitives.dom -> 67 passed (67). No adopted surface exists; no visual change ships. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(design-system): map DoseLine unit size to text-sm-minus Static PR checks failed on the type-scale guard: DoseLine used text-[0.8125rem], which is exactly --text-sm-minus. Replace with the named step so the zero-arbitrary gate stays green. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com> * fix(design-system): clear Static PR lint on choice and StageList - Checkbox Check icon: aria-hidden="true" (decorative; label is the name) - StageList gutter: z-[1] → z-5 (z-index ladder) 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>
cursorBot
pushed a commit
that referenced
this pull request
Aug 8, 2026
Address Codex P2 review on PR #1725: the lock-parity completeness check must fail closed on missing non-entry package files, and the stale design-system handover must be bannered or archived rather than deleted. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
BigSimmo pushed a commit
that referenced
this pull request
Aug 12, 2026
…system rows Archived as verified-delivered or duplicate: #303, #284 (dup of #296), #149, #159, #286, #287, #288, #256, #261, #291. Re-scoped with re-measured evidence: #210 (gate was never missing; the defect is the .next/dev types include), #262 (aliases 228 -> 193), #266 (counts moved, 23 unadopted holds), #275 (premise 'only one implementation' is false — the values leaked to five files). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017paT42ZVMf8jaLtkjFxdy5
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 freeto 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.
Summary
Validation