Skip to content

Restore verify job compatibility with current Supabase types - #140

Merged
BigSimmo merged 10 commits into
mainfrom
copilot/fix-failing-github-actions-job-verify
Jul 2, 2026
Merged

Restore verify job compatibility with current Supabase types#140
BigSimmo merged 10 commits into
mainfrom
copilot/fix-failing-github-actions-job-verify

Conversation

Copilot AI commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Fix the failing verify job by bringing handwritten Supabase usage back into alignment with the current generated database types and current route behavior.
  • Type compatibility: loosen generated vector/RPC typings where runtime code already passes number[] / null, exclude scratch/** from the TS program, and tighten worker/update helpers so typecheck no longer fails on valid production paths.
  • Route/query fixes: make searchable-only reindex paging explicit to document_images, restore null-owner filtering in RAG alias loading, and replace the upload naming cast with a narrow adapter.
  • Behavioral contract updates: update private-access rollback tests to match current UUID validation and error response behavior.
const query =
  args.searchableOnly
    ? args.supabase.from("document_images").select(args.select).eq("document_id", args.documentId).eq("searchable", true)
    : args.supabase.from(args.table).select(args.select).eq("document_id", args.documentId);

Verification

  • npm run verify:cheap
  • npm run verify:ui when UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changed
  • npm run verify:release before release or handoff confidence claims
  • npm run format:check
  • npm run check:production-readiness when clinical workflow, privacy, environment, Supabase, source governance, or deployment behavior changed
  • npm run check:deployment-readiness when deployment startup, hosting, or rollout behavior changed

Clinical Governance Preflight

Complete this section when the change touches ingestion, answer generation, search/ranking, source rendering, document access, privacy, production env, or clinical output.

  • Source-backed claims still require linked source verification before clinical use
  • No patient-identifiable document workflow was introduced or expanded without explicit governance approval
  • Supabase target remains Clinical KB Database (sjrfecxgysukkwxsowpy)
  • Service-role keys and private document access remain server-only
  • Demo/synthetic content remains clearly separated from real clinical sources
  • Source metadata, review status, and outdated/unknown-source behavior remain conservative
  • Deployment classification/TGA SaMD impact was checked when clinical decision-support behavior changed

Notes

  • CodeQL reported 0 alerts but the analysis job itself failed; this change addresses the actionable verify breakage, not the CodeQL runner failure.
  • verify:cheap was run under the repo-required Node 24 / npm 11 toolchain.

Copilot AI changed the title [WIP] Fix failing GitHub Actions job 'verify' Restore verify job compatibility with current Supabase types Jul 2, 2026
Copilot AI requested a review from BigSimmo July 2, 2026 08:41
@BigSimmo

BigSimmo commented Jul 2, 2026

Copy link
Copy Markdown
Owner

@copilot resolve the merge conflicts in this pull request

@BigSimmo
BigSimmo marked this pull request as ready for review July 2, 2026 10:32

Copilot AI commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

@copilot resolve the merge conflicts in this pull request

Resolved in 42e9031.

@BigSimmo
BigSimmo enabled auto-merge July 2, 2026 10:41
@BigSimmo
BigSimmo merged commit f6f2f65 into main Jul 2, 2026
4 checks passed
@BigSimmo
BigSimmo deleted the copilot/fix-failing-github-actions-job-verify branch July 2, 2026 16:31
BigSimmo pushed a commit that referenced this pull request Jul 30, 2026
…ocation

CI caught what I did not: `static-pr` failed on `check:outstanding-issues` with
#131-#134 duplicated and two `issues:next-id` markers.

Cause: main's PR #1424 allocated #131-#134 for its own findings at the same time
this branch held #131-#135, and `merge=union` did what union does — kept both
sides under the same ids. That is #112's documented limit: union preserves
concurrent appends but cannot allocate unique ids, so the structural gate is the
only thing that catches it.

My error was pushing without re-running that gate. The previous push resolved a
`docs/branch-review-ledger.md` conflict, and I validated only that file before
pushing to win the race against main — but the same merge also touched
`docs/outstanding-issues.md`. `verify:cheap` would have caught it locally.

Main's rows keep #131-#134 (already merged and referenced elsewhere); this
branch's five renumber to #136-#140, one marker at 141, and the cold-cache
cross-reference in process-hardening follows its row.

Two of main's new rows also make a planned addition here redundant: #134 is the
absent ledger merge driver and #133 is the outstanding-issues merge churn — both
hit during this branch's work, both already captured upstream, so nothing new is
filed for them.
BigSimmo added a commit that referenced this pull request Jul 30, 2026
…t did (#1427)

* test(phone-scroll): prove the drag delivered before asserting the chrome hid

CI run 30518866604 failed `ui-phone-scroll.spec.ts:423` on

  expect(getByTestId('universal-header-collapse'))
    .toHaveAttribute('data-scroll-hidden', 'true')  // received ""

after the full 10s auto-retry, and the classifier recorded it as "needs
investigation". The assertion was right; the scroll never happened.

`dragScrollBy` moved the scroller with `scrollTop +=`, which clamps silently
at the end of the range, and returned nothing. When a page lays out shorter
than the test assumed — content still settling under full-suite CI load — a
720px request delivers a fraction of that, the chrome correctly stays visible
because document-detail chrome only hides past `scrollTop > 120`, and the
failure surfaces ten seconds later looking like a product regression. The
helper also resolved the scroll owner once up front, so a mid-drag layout
change left it pushing an element that had stopped scrolling.

- `dragScrollBy` now re-resolves the owner each step and returns the distance
  actually travelled.
- `dragScrollUntilHidden` waits for the remaining downward runway (a condition
  wait, not a settle sleep), drags, and fails naming the shortfall if the drag
  could not cross the threshold. Used at the four sites that assert a hide
  immediately after a fixed-distance drag.
- `addPhoneScrollRunway` waits for its 1600px filler to reach layout instead of
  sleeping 50ms. All 14 call sites already depend on that runway existing.

Every assertion is byte-identical: a genuinely stuck header still fails exactly
as before, once the drag is proven to have happened. No `.first()` was added
(#93's stop rule) and no tolerance was relaxed.

* ci: shard Production UI across three runners

Measured on 2026-07-30 from the Actions API, two full UI-scope PR runs
(30520443076, 30519912667): `Production UI` took 15m26-16m31 of a 16.8-18.6
minute run — 83-89% of wall clock — while every other job finished by minute 4
and then waited. Playwright itself reported `339 passed (13.5m)`; the balance is
the isolated production build.

That single job is also where the churn cost lands: 42% of PR runs in the
sampled window were cancelled (25 of 60 completed), almost all superseded
mid-Production-UI.

Sharding is across runners, not workers. `workers: 1`, `fullyParallel: false`
and `retries: 0` are unchanged inside each shard, so determinism is identical
and per-runner load falls — which matters because #93's duplicate page root is
load-dependent. `run-playwright.mjs` already forwards argv to `playwright test`,
so `--shard` needed no runner change.

The shard count is measured, not chosen. `fullyParallel: false` makes a spec
file indivisible, so shard sizes are lumpy and more shards is not monotonically
faster. Over the 340 required chromium tests:

  N=3 -> 121/106/113       largest 121
  N=4 -> 121/106/96/17     largest 121  (same critical path, one more runner)
  N=6 -> 65/56/106/5/91/17 largest 106
  N=5 -> 121/106/0/96/17   and N=8 -> two empty shards

N=4 buys nothing over N=3, and any N with an empty shard would go red because
`test:e2e:pr` deliberately omits `--pass-with-no-tests`. Expected critical path
~15.5 -> ~7 min, assuming per-test cost is roughly uniform.

`fail-fast: false` so a failing shard cannot cancel its siblings and re-create
the cancelled-vs-failed ambiguity #95 removed. Artifact names are shard-scoped
because upload-artifact runs with `overwrite: false`. Branch protection requires
only the `pr-required` aggregate, and `needs` on a matrix job yields the roll-up
of all shards, so the aggregate is unchanged.

Also adds `restore-keys` to both Playwright browser caches: without a prefix
fallback a lockfile bump forced a cold browser download in every UI job at once,
now three times over.

* ci: bound the codex auto-resolve jobs and serialise the visual config

Two inconsistencies found while mapping the pipeline, neither load-bearing but
both silent:

- `codex-autofix-review-comments.yml` was the only workflow in the repo with no
  `timeout-minutes` on either job, so both inherited GitHub's 360-minute default
  for work that reads PR metadata and posts one comment.
- `playwright.visual.config.ts` set neither `workers` nor `fullyParallel`, so it
  inherited Playwright's default `workers = 50% of CPUs`. The production config
  pins both to serial deliberately; the visual lane was quietly opting out of
  the anti-flake posture the rest of the suite is configured for.

* chore(gates): pin the documented gate count to the real chain

Both numbers were wrong. `CLAUDE.md` said 24 static/consistency gates against an
actual 25 — `check:assets` landed before that line was written, so it was wrong
at authoring — and the `gates` skill said "check 2 of 26" against an actual 28.

A stale count is not cosmetic here. The skill's whole point at that line is that
`verify:cheap` stops at the first failure and everything after it never ran; an
agent that believes the chain is 26 long cannot say how much a mid-chain failure
skipped.

`check:gate-manifest` already derives the real count from
`verify:cheap:internal`, so it now asserts the documented numbers against it.
The assertions fail closed: if the anchor phrasing disappears, the guard reports
a lost anchor rather than passing on a document it no longer checks.

Mutation-proven: reverting the skill to "26" fails with
".claude/skills/gates/SKILL.md says 26 where the chain has 28".

* docs(issues): capture the CI review's deferred findings

Five items from the CI/testing review that should not be changed blind:

- #125 `ui_changed` matches all of `src/app`, so an API-only diff pays the
  15-minute UI gate. Narrowing it can hide a real regression, so it needs a
  decision plus a compensating check rather than a quieter filter.
- #126 the Playwright build writes to a per-run distDir, so Next's build cache
  is cold every run (~2 min, now ~29% of the sharded critical path). Fixing it
  means suppressing the runner's documented always-cleanup, which must not ship
  without executing the runner.
- #127 the advisory UI lane spends ~3 min per UI PR on 5 mockup tests; there are
  currently zero `@quarantine` tests for it to cover.
- #128 CI Triage is complete and self-tested but inert pending a repo variable.
- #129 four `changes` outputs are computed and consumed by nothing, and
  `coverage_changed` fires on any non-doc file.

* docs(ledger): record the ci-testing-review pass at this HEAD

* ci: re-measure the shard split on the merged tree and refresh stale gate counts

The merge changed both numbers this branch had recorded.

Shard balance, re-measured against 342 required chromium tests (was 340):
  N=3 -> 121/111/110    largest 121
  N=4 -> 121/106/98/17  largest 121
N=3 remains correct — one 121-test spec group bounds both, so N=4 spends an
extra runner for the same critical path. The re-measure command is now in the
workflow comment so the next person does not have to rediscover it.

Gate counts: merging main added `check:gitleaks-pinned` and
`check:pr-mergeability` to `verify:cheap:internal`, so the documented counts
went stale the moment the merge landed — 25 -> 27 static, 28 -> 30 total. The
guard added earlier in this branch caught it immediately rather than letting the
docs drift again, which is the whole reason it exists.

Also records the `ui-critical-fast` interaction: the UI critical path is now that
15-test fail-fast job plus the slowest shard, not the full 13.5-minute suite, so
neither of this branch's pre-merge timings can be read on its own.

* docs(issues): rebuild the ledger after a union-merge duplication

The `merge=union` driver on `docs/outstanding-issues.md` preserves concurrent
appends, but when both sides restructure the same region it concatenates them
wholesale. Merging the latest main did exactly that: every open row appeared
twice and both `issues:next-id` markers survived — 66 duplicate-id errors from
`check:outstanding-issues`, which is precisely the failure that gate exists to
catch (#112).

Resolved by rebuilding on main's canonical file rather than by hand-editing the
duplicated table: reset to `origin/main`, then re-apply this branch's five
captured rows at #131-#135 (main had advanced its allocation to #130 while this
branch was open, so the earlier #128-#132 numbering collided again) and
re-apply the #127 narrowing note. Marker bumped to 136.

Union merge cannot allocate unique ids; only the structural gate can catch when
it has produced an invalid file. It did.

* ci: record the measured shard result, correcting the predicted one

First real run of the sharded shape (CI 30530618838, all green, whole run
13m39 against a 16.8-18.6 min unsharded baseline):

  ui-critical-fast  15 tests   3m14
  Production UI (1) 121 tests  9m36
  Production UI (2) 111 tests  6m54
  Production UI (3) 110 tests  6m20

The prediction was wrong by ~40%. ~6.8 min was expected for the largest shard
from 121/342 tests x 13.5 min; 9m36 happened. Per-test cost is not uniform —
111 tests took 6m54 while 121 took 9m36 — so a count-balanced split understates
the slowest shard whenever the slow specs land in one group. `--shard` can only
balance by count; balancing by duration would mean splitting the slow spec files
themselves.

The win is real but smaller than claimed, and the workflow comment and
process-hardening now carry the measured numbers plus the reason the arithmetic
misleads, so the next person re-measures instead of re-deriving.

Also merges origin/main. The ledger conflict was GitHub-visible only: that file
carries merge=union locally, which GitHub does not honour (#129). Resolved by
keeping the one genuinely new record and dropping three that main already had
elsewhere in the file — append-only forbids dropping a record that exists once,
not keeping a second copy. Superseding record appended for this HEAD, since the
prior one asserted a root cause that #127's trace evidence refutes.

* docs(issues): renumber this branch's rows above main's concurrent allocation

CI caught what I did not: `static-pr` failed on `check:outstanding-issues` with
#131-#134 duplicated and two `issues:next-id` markers.

Cause: main's PR #1424 allocated #131-#134 for its own findings at the same time
this branch held #131-#135, and `merge=union` did what union does — kept both
sides under the same ids. That is #112's documented limit: union preserves
concurrent appends but cannot allocate unique ids, so the structural gate is the
only thing that catches it.

My error was pushing without re-running that gate. The previous push resolved a
`docs/branch-review-ledger.md` conflict, and I validated only that file before
pushing to win the race against main — but the same merge also touched
`docs/outstanding-issues.md`. `verify:cheap` would have caught it locally.

Main's rows keep #131-#134 (already merged and referenced elsewhere); this
branch's five renumber to #136-#140, one marker at 141, and the cold-cache
cross-reference in process-hardening follows its row.

Two of main's new rows also make a planned addition here redundant: #134 is the
absent ledger merge driver and #133 is the outstanding-issues merge churn — both
hit during this branch's work, both already captured upstream, so nothing new is
filed for them.

* docs(issues): rebuild against main's current id allocation

The union merge duplicated the whole open and archive tables again (two header
rows, every id twice) because main restructured the file while this branch held
rows in it. Same resolution as before and for the same reason: rebuild on main's
canonical file rather than hand-editing a doubled table, then re-apply this
branch's five rows.

Main is now at next-id=135, so they land as #135-#139 with the marker at 140.
None of the five is duplicated upstream — checked by summary before re-applying.

This is the third renumber of the same five rows in one PR. That is not a
mistake being repeated, it is #133 ("outstanding-issues conflicts on nearly
every main advance") happening: any branch that holds rows in this file
re-collides every time main lands one. Worth weighing whether captures should
land in their own PR ahead of the work rather than riding along with it.

* docs: record PR 1427 review

---------

Co-authored-by: Claude <noreply@anthropic.com>
BigSimmo pushed a commit that referenced this pull request Jul 30, 2026
…it asks for

A real conflict this time, not staleness: main's PR #1427 rewrote the same
helpers this branch touches, and it very likely found the actual cause of
#127. `addPhoneScrollRunway` slept 50ms and merely hoped the appended 1600px
runway had reached layout; `dragScrollBy` clamped silently at the end of the
range while reporting nothing. Under CI load the drag therefore delivered
less than it asked for and the chrome was right to stay visible. #1427 polls
for the runway, returns the distance actually travelled, and
`dragScrollUntilHidden` refuses to expect a hide until remaining runway and
delivered travel both clear 160px.

Main's helpers are taken whole. This branch keeps only what #1427's own
comment says is still missing: "Separating THOSE two still needs the pin
state exposed in the DOM; today only the composite `data-scroll-hidden`
(`scrollHidden && !sharedChromePinned`) is observable, so both look
identical." `data-scroll-signal` publishes the raw signal, and
`expectChromeHidden` is cut down to answer only that question, now layered
after `dragScrollUntilHidden` rather than duplicating its travel proof.

#127 is rewritten again and withdraws a second wrong diagnosis of my own: a
short/clamped drag was ruled out early using a maxOffset of 2753 read at a
different trace moment than the failing drag, when the pre-runway reading in
that same trace was 1153 — and a runway not fully landed puts the offset in
the near-bottom band where computeScrollHideUpdate legitimately refuses.
That is exactly what #1427 fixes. The row now points at #1427 as the likely
fix, keeps the observability gap as the only open part, and says to close it
if no recurrence appears on a post-#1427 head.

Main also claimed #135 for an unrelated issue, so the union-driver finding
renumbers to #140, marker 141.

Verified: typecheck 0 errors, lint 0 problems, whole-tree prettier clean,
check:outstanding-issues 138 rows / unique ids / next-id=141, and the merged
phone-scroll spec 56 passed (4.1m) against an isolated production build —
under Chromium 1194, not CI's bundled 1234 (#121).

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
* issues: close #140 as a duplicate of #133, resolved by #1444

#140 was opened mid-session for the union-driver damage before I noticed
#133 had already recorded the same finding, earlier and with the same
conclusion. Two open rows described one condition, and PR #1444 has since
removed that condition: `merge=union` is gone from `.gitattributes`,
`check:outstanding-issues` now requires an unspecified `merge` attribute,
and regression tests cover `union`, `-merge` and an unparsed reading.

Moved to the archive table rather than deleted, pointing readers at #133 —
whose still-open half is the real conflict-frequency cause: fixed-width
column padding makes any one-row edit re-pad every row, so git sees the
whole table as one hunk. The surviving evidence (four merges on PR #1430
each reporting success while duplicating the entire open-items table) lives
there too.

Verified: check:outstanding-issues 138 rows, 66 open / 72 archived (was
67/71 — moved, not copied), unique ids, next-id=141, no merge driver;
docs:check-links 1361 references resolve; prettier clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XrPbbfU9yWuEjEVypCr4ZQ

* docs: record duplicate issue review

---------

Co-authored-by: Claude <noreply@anthropic.com>
BigSimmo pushed a commit that referenced this pull request Jul 30, 2026
Resolves the docs/outstanding-issues.md conflict with PR #1453. That file
deliberately carries no merge driver (#133), so overlapping appends conflict
loudly rather than being silently concatenated.

Resolved as the issues skill requires: rebuilt the file from origin/main and
re-applied only the two rows this branch owns (the #86 in-place update and the
new #145), so none of #1453's rows were dropped. Verified #140-#144 all still
present and #144's cell content byte-identical to main. Re-checked that #145 was
still free on main before reusing the id — main's next-id marker was untouched at
145, so there was no id collision to reallocate around.

docs/branch-review-ledger.md auto-resolved through its merge=ledger driver.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GGEBHp4Seoh1jK1vGTNtYS
BigSimmo pushed a commit that referenced this pull request Jul 30, 2026
The merge of origin into this branch hit exactly the damage #133/#140
describe: merge=union concatenated both sides of the ledger rather than
merging it.

Two collisions, both repaired without dropping either side's rows:

  - Another agent had already allocated #141-#144 on main for different
    items while this branch used #141-#143. The incoming rows renumber, per
    the ledger rule, so the capture becomes #145 (adopt a consolidated
    answer-home notice block), #146 (answer mode ships no verify-before-use
    caveat) and #147 (verify:pr-local exits 0 when its build step refuses to
    run). Their cross-references were updated to match, and the two
    duplicated next-id markers collapse to one at 148.
  - #140 appeared in both tables: it was closed on main as a duplicate of
    #133 (PR #1444) while this branch still carried it open. The resolution
    is honoured — the stale open row goes, the archive row stays.

check:outstanding-issues: 145 rows (72 open, 73 archived), unique ids,
next-id=148 above the highest, no merge driver.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NPyFcMfn1jMmphr6AqiWBg
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