Skip to content

Resolve merge conflicts + review fixes for PR #2009 - #2018

Merged
BigSimmo merged 89 commits into
codex/account-setup-polish-20260814from
claude/pr-2009-review-fix-vei5e3
Aug 17, 2026
Merged

Resolve merge conflicts + review fixes for PR #2009#2018
BigSimmo merged 89 commits into
codex/account-setup-polish-20260814from
claude/pr-2009-review-fix-vei5e3

Conversation

@BigSimmo

Copy link
Copy Markdown
Owner

Summary

PR #2009 (codex/account-setup-polish-20260814) has a real content conflict against main (confirmed with git merge-tree, not just a stale-branch false positive) — its branch point predates several already-landed changes that touched the same files (a competing account-setup-dialog redesign, the services quick-filter removal, and a check-outstanding-issues.mjs/issue-id.mjs refactor). This PR merges main into PR #2009's branch and resolves all six conflicting files, then applies two of CodeRabbit's PR #2009 review findings. Merging this into codex/account-setup-polish-20260814 will make PR #2009 itself mergeable.

  • Merge origin/main, resolving conflicts in docs/filter-contract.md, scripts/check-outstanding-issues.mjs, scripts/ledger-inbox.mjs, src/components/clinical-dashboard/account-setup-dialog.tsx, tests/ui-smoke.spec.ts, and tests/ui-tools.spec.ts. Where main had already superseded the PR's own approach (e.g. main's more refined account-setup-dialog styling, or its refactor of check-outstanding-issues.mjs's id-parsing into issue-id.mjs), I took main's version rather than reintroducing dead code. Where both sides added independent functionality (e.g. ledger-inbox.mjs's baseRowFingerprint capture vs. main's new issueUlid/request-version-2 handling), I combined them.
  • Fix CodeRabbit finding: scripts/ledger-inbox.mjs's stale-row fingerprint comparison now lowercases baseRowFingerprint before comparing, since issueRowFingerprint always emits lowercase hex — the exact-match comparison was rejecting an otherwise-valid uppercase fingerprint as stale.
  • Fix CodeRabbit finding: tests/ui-tools.spec.ts's toHaveURL(/q=13YARN/) now asserts query-parameter boundaries (/[?&]q=13YARN(?:&|#|$)/) so it can't pass on a value like 13YARN-extra.
  • Not applied: CodeRabbit's "require baseRowFingerprint for done/update requests" (Major) — the live inbox currently has 11 pending legacy requests with no fingerprint; making it mandatory would break npm run issues:reconcile for all of them. That's a real migration, not a merge-conflict fix, so it's out of scope here. Also not applied: the "replace arbitrary layout values with shared design tokens" nitpick (flagged "Heavy lift" complexity, "Trivial" priority).

Verification

  • node scripts/check-outstanding-issues.mjs --self-test — passed

  • node scripts/ledger-inbox.mjs --self-test — passed

  • npm run check:outstanding-issues — passed (11 pending requests, 189 applied, 341 rows, no drift)

  • node scripts/check-ledger-write-discipline.mjs — passed

  • node scripts/run-vitest.mjs run tests/outstanding-issues-writer.test.ts tests/repo-hygiene.test.ts tests/ledger-inbox-cancellation.test.ts tests/favourites-auth-gate.dom.test.tsx --reporter=dot — 88 tests passed

  • npx prettier --check on all resolved files — passed

  • npm run typecheck — passed

  • npx eslint on all resolved files — passed

  • npm run verify:pr-local — not run; the focused checks above cover every touched surface (docs gate, ledger self-tests, the specific DOM/spec test files, typecheck, lint, format) and this PR only resolves a merge conflict plus two narrow review fixes, not new feature behavior. verify:pr-local's heavy-scope escalation (full unit suite, RAG fixture validation) would be duplicate coverage for these changes.

  • npm run verify:ui — not run. The two .spec.ts (Playwright) files changed only in conflict resolution (taking main's already-passing assertions over the PR's own, plus one regex-boundary fix); no new UI behavior was introduced by this merge PR itself.

Risk and rollout

Notes


Generated by Claude Code

BigSimmo and others added 30 commits August 14, 2026 14:08
scripts/run-playwright.mjs writes an isolated tsconfig.json (extends:
"../../tsconfig.json") for each Playwright run root, but left include/
exclude unset. TypeScript resolves an extended config's *inherited*
relative include/exclude entries against the base config's own
directory (the repo root), so the isolated config still resolved
".next/dev/types/**/*.ts" (and the broad "**/*.ts") against the shared
top-level .next/ directory rather than this run's own NEXT_DIST_DIR
output under its dist/ folder.

Empirically confirmed with a stale .next/dev/types/*.ts fixture file:
before this change, `tsc --showConfig` on the generated child config
resolved that repo-root file into the isolated run's file list; after
this change it does not. The child config now declares its own
include (repo source globs plus this run's own dist/types and
dist/dev/types) and exclude (mirrors the root tsconfig's exclude list
plus the repo-root .next/**).

Verified this is not clobbered by Next's own tsconfig auto-config:
writeConfigurationDefaults() returns immediately when the parsed
config has "extends" (confirmed by reading
node_modules/next/dist/lib/typescript/writeConfigurationDefaults.js).
Hashed the generated child tsconfig.json immediately after write and
again after a full `next build --webpack` + server start + Playwright
test run (npm run test:e2e:pr equivalent, single focused spec) — both
hashes match byte-for-byte (sha256
1c744da9634c2d712b1fd9ae428a5808fadf82b9d623d0ce30ca923b56a3ce4a).

Ledger #210 (re-scoped 2026-08-13): items 1-2 already closed/refuted;
this closes item 3.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QLbw9qpfjv5CeNz6XpmteN
Second serial transaction. 25 active mutations (13 done, 6 add, 6 update)
plus 5 cancellation decisions. Ledger 106 open / 222 archived to
99 open / 235 archived; inbox 0 pending / 129 applied.

Three of the closures queued in #1940 were cancelled by review, and the
cancellations are right: #235, #237 and #238 each asked for visual or
browser proof, and they were closed on executable evidence instead —
proof shots, a real 320px browser pass, and product-overlay journeys are
not satisfied by a docs table, a jsdom assertion, or generic Sheet unit
coverage. Those three rows stay open. The other six closures applied.

Verified zero live same-target collisions before applying.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017paT42ZVMf8jaLtkjFxdy5
chore(issues): reconcile 35 queued ledger requests
Applies the pending inbox to docs/outstanding-issues.md as one serial
transaction from a fresh origin/main base (0011a05), which is the only
path allowed to edit the canonical ledger.

35 requests: 17 done, 7 add, 6 update, 5 cancel. Every request moves
verbatim to docs/outstanding-issues-inbox/applied/ as its immutable audit
record. One row carried competing mutations — #213, two done requests —
resolved by the cancel already queued against one of them.

Ledger goes from 328 to 334 rows, 115 open to 99.

Includes the five requests queued by PR #1944 but left pending when it
merged: closes #313 (the write-discipline dirty-tree refusal shipped),
carries #211 forward with its re-measured 1,445 errors while keeping the
2026-08-12 deprioritisation judgment, records the documented state of #168
and #258, and opens #335 for the gap between the two merge-loss detectors.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Chrn9bTYFFYzrLZqtpVptW
…s instrument now exists

Four corrections, queued as immutable inbox requests.

#235, #237 and #238 each had a `done` queued in PR #1940 and cancelled on
review. The cancellations were right: all three ask for visual or browser
proof and were closed on executable evidence. Each row now records the
attempt, the reason it was refused, and a Stop rule naming the evidence
class that must not be used again — so the next reader does not repeat it:

- #235: section 7.1 opens with "records executable evidence RATHER THAN
  committing image baselines", so the section that looks like the evidence
  says in its first line that it is not
- #237: jsdom does not lay out text, so a 320px assertion proves the string
  is present, not that it fits
- #238: the risk is ancestor-scoped CSS/contain/transform on five specific
  host surfaces, which no Sheet component test can see

All three also drop their IN FLIGHT do-not-start prefix; PRs #1841 and
#1842 have merged, so the warning was blocking rather than protecting.

#231's "Next: instrument and reproduce" is stale — commit a3bc4da added
scripts/probe-generation-quality.ts and adjudicated PR #1861 as superseded
for phase 1. The row now says so, so nobody reviews #1861 as the live fix
or rebuilds the probe. Next is running it where credentials exist.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017paT42ZVMf8jaLtkjFxdy5
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Chrn9bTYFFYzrLZqtpVptW
docs(issues): record why three closures were rejected, and that #231's instrument now exists
…be evidence (#316, #231)

Fills the 2026-08-14 owner-authorized incident evidence into the live-drift
forensics record: Phase 1.1 migration-history fingerprint (20260705180000
executed with 14 statements; the two trigram indexes existed at the 2026-08-04
guard and were dropped by 2026-08-09), Phase 1.3 partial inventory, Phase 4
partial restore (CREATE INDEX CONCURRENTLY, both indisvalid/indisready), and
Phase 5 before/after probes (supabase_rpc_latency_ms 31610 -> 1535 text /
8519 hybrid). Carries the PR #1939 review ledger record and queues merge-safe
issue updates for #316 and #231.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpjrUp4fYmiDGfc6NBqACX
Runs `npm run issues:reconcile` from a fresh base off origin/main
(0011a05), applying all 35 pending requests with 5 cancellation
decisions.

The motivating case is #310, which main was carrying with a false
statement. Request 8621298b ("NOT REACHABLE ON MAIN ... The whole
matcher is gone") had been applied, while the cancellation queued
against it was lost in the PR #1943 squash. The correcting closure
00428ff0 survived as a pending request, so this reconcile applies it:
#310 now records that 247a359 re-landed the matcher with
typoDistanceLimit >=5 -> 1 and both regression tests, and the false text
is gone from the file.

The lost cancellation was deliberately NOT re-filed. Its target is
already in applied/, so planRequestBatch would reject it with "cancel
request targets missing pending request" and block every future
reconcile.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0122UC4BL5R39kz41WbBYcPj
This session ran `npm run format` in a container with no node_modules.
Prettier resolved through npx as 3.8.1 while the repo pins ^3.9.6
(lockfile 3.9.6), and the older binary rewrote 31 files nobody had
touched — including src/lib/rag/rag-cache.ts, src/lib/rag/rag-provider.ts
and src/lib/openai.ts, which would have flipped a docs-only PR into
ragRanking/clinicalRisk scope under classifyPullRequestFiles.

It was an artifact, not drift: `npx prettier@3.9.6 --check` on the same
files reports them clean. The reformat was reverted and never committed;
this row records the trap so a later uninstalled session does not repeat
it, and proposes pinning the binary in format/format:changed or failing
closed on a lockfile mismatch.

Same failure class as archived row #87 (knip findings from an
uninstalled worktree), but worse: knip only reports, format writes, so
the false result arrives already applied to the working tree.

Filed as a pending request rather than folded into the preceding
reconcile: a reconcile must only consume requests already present on its
base, and check:ledger-write-discipline rejects the bundle otherwise.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0122UC4BL5R39kz41WbBYcPj
Hand-written follow-up to the machine-generated transaction in the previous
commit. Separate commit so the reconciliation stays independently revertible
and remains purely generated.

- newtask: the #292 open-PR duplicate check is DENIED, not merely
  unavailable, once the PR-handoff stop hook has armed in a session. The
  existing text only covers "GitHub is unreachable", which reads as a network
  problem and does not tell a session what to do when the tool itself is
  blocked. Records git ls-remote --heads as the fallback, and says explicitly
  not to unlock the hook for it.

- handoff: --force-with-lease fails with "stale info" when restarting a
  branch whose PR merged, because GitHub deleted the remote branch and the
  local remote-tracking ref is stale. The failure looks like a lease
  violation inviting --force; it is not. git remote prune origin, then push
  normally — there is nothing to force.

- issues: the visual-register refresh is unavailable off the operator's
  Windows machine, so a cloud session can reconcile but cannot refresh or
  even measure staleness. States the limit where the command is, and forbids
  improvising a substitute renderer: an artifact that looks refreshed but
  came from a different generator is worse than a visibly stale one.

Queues one P3 request for the register drift itself, which is cumulative
rather than a single-run failure. No predicted ledger id is cited anywhere —
ids are allocated at reconcile, which is the #168 hazard.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Chrn9bTYFFYzrLZqtpVptW
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpjrUp4fYmiDGfc6NBqACX
…s + one combined update

CI (docs:check-links -> ledger-inbox planner) rejected two pending #231
updates: this branch's incident-evidence update and main's instrument-exists
update (7f5e6922). Both are cancelled with reasons and replaced by a single
combined update carrying the earlier text verbatim plus the 2026-08-14
incident addendum, so no queued content is lost.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpjrUp4fYmiDGfc6NBqACX
… proof, record guard-migration debt

Removes the unsupported 2026-08-04 existence bound (the 20260804110240 guard
validates four other indexes, not this pair): the drop window is 2026-07-05 to
2026-08-02 and the dashboard attribution stays an inference pending audit
history. Records the post-build pg_indexes canonical definitions for both
restored indexes, and names the still-owed phase 4.4 fail-fast guard migration
as follow-up debt rather than claiming no migration is needed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpjrUp4fYmiDGfc6NBqACX
…hvja

docs(audit): 2026-08-14 incident forensics and live index restore evidence (#316, #231)
…isions)

Applies the pending inbox batch to docs/outstanding-issues.md: the combined
#231 update (instrument-exists text + 2026-08-14 incident rung-2 evidence,
with both superseded #231 mutations cancelled per their recorded reasons),
the #316 incident-restore update, and the #235/#237/#238 closure-rejection
updates. All applied request records move to the immutable applied/ audit
directory.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpjrUp4fYmiDGfc6NBqACX
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpjrUp4fYmiDGfc6NBqACX
chore(issues): reconcile 9 queued ledger requests (2 cancellation decisions)
BigSimmo and others added 23 commits August 15, 2026 14:00
…-fix-vei5e3

# Conflicts:
#	docs/filter-contract.md
#	scripts/check-outstanding-issues.mjs
#	scripts/ledger-inbox.mjs
#	src/components/clinical-dashboard/account-setup-dialog.tsx
#	tests/ui-smoke.spec.ts
#	tests/ui-tools.spec.ts
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

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

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 91838781-15cf-4f82-ba9c-ef681f3999f0

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

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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

@supabase

supabase Bot commented Aug 17, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@BigSimmo
BigSimmo merged commit 3662e35 into codex/account-setup-polish-20260814 Aug 17, 2026
2 checks passed
@BigSimmo
BigSimmo deleted the claude/pr-2009-review-fix-vei5e3 branch August 17, 2026 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants