Skip to content

docs(issues): capture the Services viewport-anchor flake as #146 - #1446

Merged
BigSimmo merged 12 commits into
mainfrom
claude/ci-testing-review-2l8klp
Jul 30, 2026
Merged

docs(issues): capture the Services viewport-anchor flake as #146#1446
BigSimmo merged 12 commits into
mainfrom
claude/ci-testing-review-2l8klp

Conversation

@BigSimmo

@BigSimmoBigSimmo commented Jul 30, 2026

Copy link
Copy Markdown
Owner

Summary

  • Capture one finding from the PR ci: shard Production UI, and make the phone-scroll drag report what it did #1427 session that was never recorded: tests/ui-phone-scroll.spec.ts:973 "Services results keep a continuous browser viewport after shared chrome releases" failed the viewport-resize anchor assertion at :1133 on shard 1 of run 30534158395 (120 passed (6.5m), classified "needs investigation"). Filed as #146.

Docs-only. One row appended to docs/outstanding-issues.md, marker at 147.

RAG impact: no retrieval behaviour change — documentation only, no file under src/lib/rag/**, clinical-search, retrieval-selection, ranking-config, answer-ranking, the eval harness, or the golden fixture.

Numbering

Renumbered four times, and that is now the more interesting finding. #141#142#145#146, each time because main allocated the id first: #1443's design-token gap took #141, a loose-docs task took #142, and #1461 took #145 for "a branch sync during in-flight CI cancels a near-complete run" — which is, as it happens, the same CI-cancellation behaviour this branch hit while landing #1453.

Every collision was caught by check:outstanding-issues, which is exactly what #112 built it to do. But four renumbers on a two-row documentation change is strong evidence that ids should be allocated at merge time rather than authoring time; #133 covers the adjacent merge-driver half of this.

Each resolution took main's docs/outstanding-issues.md wholesale and re-applied this row at a fresh id, rather than hand-editing the conflict region — hand-editing a 144-row table around conflict markers is how the duplicate and wrong-width rows got in before.

Why it is not covered by an existing row

It is not #127 and not fixed by #1427:

  • The failing head already carried #1427's runway poll and travel assertion.
  • #127 is the reduced-motion chrome hide on the document route; this is the Services result canvas.
  • The assertion is result-content anchor stability across a viewport shrink, not a chrome hide.

Evidence

Intermittent on byte-identical code — the spec is unchanged across heads 79dd28f, 76f80f3, 17c8cb9; shard 1 passed, passed, failed, then passed again on re-run. One failure in four.

Filed as an issue rather than a tests/flake-ledger.json entry: that ledger takes reproduced @quarantine entries only, and this reproduces once in four.

A narrowing was claimed here and has been withdrawn

An earlier version of the row argued that because the sibling documentScrollTop assertion at :1137 did not fail, the scroll position held and scroll-restoration causes were ruled out.

That inference was invalid, and Codex was right to flag it. Playwright aborts a test at the first failing expect, so once the anchorTop assertion threw, the documentScrollTop assertion never executed. Its absence from the output shows nothing. The row now says so explicitly and that class of cause is not ruled out. A superseding ledger record carries the correction rather than editing the earlier rows.

Separately, CodeRabbit flagged :973 vs :1133 as inconsistent and then withdrew it: :973 is the test declaration and :1133 the thrown assertion — Playwright reports both, and declaration lines drift (898 / 973 / 1041 across three tree states), which is why the exact title is the durable identity.

Branch note

#1427 merged as 040ce97, so this branch was restarted from origin/main rather than stacking on already-merged history. Verified by content, not ancestry (squash breaks ancestry): the shard matrix, dragScrollUntilHidden, and the gate-count guard are all present on main.

Verification

At head 5eb736d, after resolving the conflict against maina0aceb3:

  • node scripts/check-outstanding-issues.mjs144 rows (71 open, 73 archived), unique ids, next-id=147 above the highest
  • npm run check:branch-review-ledger127 live table records + 1206 archived, no duplicates, no conflict markers, no mojibake
  • npx prettier --check .All matched files use Prettier code style!
  • Zero conflict markers remaining in docs/outstanding-issues.md; the diff against main is still the same two documentation files
  • npm run verify:cheap on the pre-conflict head — Test Files 435 passed (435), Tests 4569 passed | 4 skipped (4573), exit 0. Not re-run after this merge: the resolution touches one documentation table, and the three gates above are the ones that cover it.

UI verification not run: documentation-only change; ui_changed is false and the Chromium shards skip.

Not run, deliberately: verify:release, eval:*, and check:supabase-project are provider-backed and not authorised for a docs change.

Risk and rollout

  • Risk: low. Appended documentation rows; no application code, no schema, no CI configuration.
  • Rollback: revert the commits on this branch.
  • Provider or production effects: None.

Notes

Two review findings landed on this PR and they resolved in opposite directions — one correct and applied, one incorrect and declined with reasoning. Both are recorded in the ledger, because the useful artefact is the reasoning rather than the verdict.


Generated by Claude Code

claude added 2 commits July 30, 2026 12:34
Found while driving PR #1427 to green and not recorded anywhere: shard 1 of run
30534158395 failed `ui-phone-scroll.spec.ts:973` at the viewport-resize anchor
assertion on :1133, with 120 passed.
It is distinct from #127 and is not what #1427 fixed. That head already carried
#1427's runway poll and travel assertion, the journey is the Services result
canvas rather than the document route, and the assertion is anchor stability
across a resize rather than the chrome hide.
One narrowing is already in hand: the sibling assertion on `documentScrollTop`
at :1137 did not fail in the same run, so the scroll position held while the
element moved. That points at layout/anchoring around the resize and rules out
the scroll-restoration class of explanations.
Filed as an issue rather than a flake-ledger entry: it reproduced once in four
runs on a byte-identical spec, and that ledger takes reproduced @quarantine
entries only.
Branch restarted from origin/main because #1427 has merged (040ce97) — follow-up
work does not stack on already-merged history.
@supabase

supabaseBot commented Jul 30, 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 ↗︎.

@coderabbitai

coderabbitaiBot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in:57 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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 81ec412f-3154-47b5-b092-e0336bee411f

📥 Commits

Reviewing files that changed from the base of the PR and between fb580a8 and cc9501f.

📒 Files selected for processing (2)
  • docs/branch-review-ledger.md
  • docs/outstanding-issues.md
📝 Walkthrough

Walkthrough

Updated documentation ledgers to add outstanding issue #141, advance the next issue ID to 142, and record the related review session, verification results, and docs-only scope.

Changes

Documentation ledger updates

Layer / File(s)Summary
Issue tracking and review record
docs/outstanding-issues.md, docs/branch-review-ledger.md
Advanced the issue counter, added the viewport-resize anchor instability item, and documented its review session and verification results.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Possibly related PRs

Suggested reviewers:claude

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
Title check✅ PassedThe title is concise and accurately summarizes the docs-only issue capture in this PR.
Description check✅ PassedThe description matches the template with Summary, Verification, Risk/rollout, and Notes; only optional sections are omitted.

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

@BigSimmo
BigSimmo marked this pull request as ready for review July 30, 2026 13:08
@BigSimmo

Copy link
Copy Markdown
OwnerAuthor

@cursoragent resolve merge conflict

@BigSimmo
BigSimmo enabled auto-merge (squash) July 30, 2026 13:08
@cursor

cursorBot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Merge conflict on PR #1446 is resolved and pushed.

Cause: both sides claimed #141 — this PR’s Services viewport-anchor flake vs main’s design-sync token gap from #1443.

Resolution: rebuilt docs/outstanding-issues.md from origin/main, kept main’s #141, re-applied the Services capture as #142, bumped issues:next-id to 143.

Checks:check:outstanding-issues passed (140 rows … next-id=143); merge-tree vs origin/main is clean; PR is MERGEABLE (checks still running).

Also: title updated to #142; ledger supersede row appended. The PR body still says #141 — ManagePullRequest couldn’t rewrite it (not agent-managed). Worth a quick manual edit if you want the description to match.

Open in WebOpen in Cursor

…142
main claimed #141 for the design-sync token gap (PR #1443). Rebuild
outstanding-issues from origin/main and re-apply this branch's Services
viewport-anchor capture as #142; bump issues:next-id to 143.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/branch-review-ledger.md`:
- Line 176: Update the ledger entry for PR `#1446` / commit
11e7e8653c4742108feee4e9164ba066a92fc18a to identify the failing assertion
consistently as ui-phone-scroll.spec.ts:1133, replacing the incorrect :973
reference while preserving the rest of the entry.
🪄 Autofix (Beta)

❌ Autofix failed (check again to retry)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e7a3e2da-27de-463b-8e59-dcb6f6171db0

📥 Commits

Reviewing files that changed from the base of the PR and between d106548 and fb580a8.

📒 Files selected for processing (2)
  • docs/branch-review-ledger.md
  • docs/outstanding-issues.md

Comment threaddocs/branch-review-ledger.md
Record that the Services viewport-anchor flake moved to #142 when
main's design-sync #141 collided during the merge resolution.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
@cursorcursorBot changed the title docs(issues): capture the Services viewport-anchor flake as #141docs(issues): capture the Services viewport-anchor flake as #142Jul 30, 2026

@chatgpt-codex-connectorchatgpt-codex-connectorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:fb580a8e2e

ℹ️ 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".

Comment threaddocs/outstanding-issues.md Outdated
claudeand others added 4 commits July 30, 2026 13:14
Codex was right, and the finding is worth stating plainly: the earlier rows
argued that because the sibling `documentScrollTop` assertion did not fail, the
scroll position held and scroll-restoration causes were ruled out.
Playwright aborts a test at the first failing `expect`. Once the `anchorTop`
assertion threw, the `documentScrollTop` assertion never executed at all — its
absence from the failure output is evidence of nothing. The narrowing is
withdrawn and that class of cause is explicitly not ruled out.
The capture itself is unaffected: the Services viewport-anchor failure is real,
intermittent on byte-identical code (pass / pass / fail / pass-on-rerun), and
distinct from #127.
A superseding ledger record carries the correction rather than editing the
earlier rows, per the append-only rule.
# Conflicts:
#	docs/outstanding-issues.md
@BigSimmoBigSimmo changed the title docs(issues): capture the Services viewport-anchor flake as #142docs(issues): capture the Services viewport-anchor flake as #145Jul 30, 2026
@BigSimmo
BigSimmo disabled auto-merge July 30, 2026 14:12
Fourth id collision on this one row. `main` (#1461) claimed `#145` for "A
branch sync during in-flight CI cancels a near-complete run, and nothing
warns" — which is, as it happens, the same CI-cancellation behaviour this
branch hit an hour ago.
Resolved by taking main's `docs/outstanding-issues.md` wholesale and
re-applying this capture at `#146`, rather than hand-editing the conflict
region: hand-editing a 144-row table around a conflict marker is how the
duplicate and wrong-width rows got in before. Marker bumped to 147.
@BigSimmoBigSimmo changed the title docs(issues): capture the Services viewport-anchor flake as #145docs(issues): capture the Services viewport-anchor flake as #146Jul 30, 2026
@BigSimmo
BigSimmo enabled auto-merge (squash) July 30, 2026 14:17
@BigSimmo
BigSimmo merged commit d76b0f7 into mainJul 30, 2026
26 checks passed
@BigSimmo
BigSimmo deleted the claude/ci-testing-review-2l8klp branch July 30, 2026 14:19
@coderabbitai

Copy link
Copy Markdown
Contributor

Note

Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it.

Autofix skipped. No unresolved CodeRabbit review comments with fix instructions found.

Sign up for freeto 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.

3 participants

@BigSimmo@claude@cursoragent