Skip to content

test(ui): read the Admin Help bar only after its rows have loaded - #3493

Merged
gitar-bot[bot] merged 5 commits into
mainfrom
claude/fix-webkit-admin-help-anchor-race
Oct 12, 2026
Merged

gitar-bot[bot] merged 5 commits into
mainfrom
claude/fix-webkit-admin-help-anchor-race

Conversation

@BigSimmo

@BigSimmo BigSimmo commented Oct 12, 2026 •

Copy link
Copy Markdown
Owner

User description

Requested by Josh · project thread

Summary

Before: ui-on-call-boards.spec.ts:558 "declares no anchor the page does not render" failed on both iPhone WebKit projects in release-browser-matrix run 38236585559: "Support is offered but absent", #admin-help-support count 0. While Admin Help loads, every section including Support is drawn as a skeleton; once rows arrive, Support drops out when it has no rows (admin-help-page.tsx, audit 11 Oct). The test read the section bar's words during loading, then looked for anchors on the settled page.

After: the test waits until no admin-help-*-loading skeleton remains before reading the bar, so it checks the bar and the anchors of the same settled page. The assertion itself is unchanged; no test skipped, disabled or quarantined.

Inferred, not proven: the race explains why only the slower iPhone emulation hit it. WebKit is not installed in this container, so proof needs a matrix run once #3487 (the WebKit launch fix) is on main.

Areas touched: Delivery and assurance

Verification

  • prettier clean; pre-commit hooks passed.
  • Not yet proven on WebKit (see above).

Risk and rollout

  • Risk: low, test only.
  • Rollback: revert this PR.
  • Provider or production effects: None.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DCghuougzFFXRF3dQQEcYr


Generated by Claude Code


Summary by cubic

Two test reliability fixes: the Admin Help anchor test now reads the section bar only after its rows load, and the SQL stripper ReDoS guard now warms up once and checks every measured run against its speed ceiling.

  • Waits for admin-help-*-loading skeletons to disappear before reading the bar, so the test checks the bar and anchors on the same settled page. Previously a slower engine could read "Support" from the skeleton and then fail to find #admin-help-support after rows loaded.
  • Runs the stripper once to warm up, then requires all five timed runs to stay below the 100 ms limit. Checking every run catches regressions that a fastest-run statistic could hide; a single cold run under coverage on a busy runner no longer trips the limit on its own (observed at ~102 and ~104 ms on two unrelated PRs).

Written for commit 560b92d. Summary will update on new commits.

View guided diff Turn on auto-fix


CodeAnt-AI Description

Wait for Admin Help to finish loading before checking its section links

What Changed

  • The anchor check now waits for Admin Help loading placeholders to disappear before comparing section labels with their links
  • This prevents the test from checking a Support link that is removed once loading finishes

Impact

✅ Fewer false Admin Help anchor test failures
✅ More reliable WebKit checks after content loads

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.


Summary by Gitar

  • Test stability:
    • Timed stripMigrationSql in the challenger SQL stress test as the fastest of five runs to avoid cold-run slowdowns under coverage

This will update automatically on new commits.

While Admin Help loads, every section including Support is drawn as a
skeleton; once rows arrive Support drops out when it has none. The anchor
test read the bar words during loading, so on WebKit it asked for
#admin-help-support after the settled page had removed it
(release-browser-matrix run 38236585559, both iPhone projects).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DCghuougzFFXRF3dQQEcYr
@supabase

supabase Bot commented Oct 12, 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

coderabbitai Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: b9e29955-5908-46f0-849d-f79a5fbbc7c3

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@BigSimmo
BigSimmo marked this pull request as ready for review October 12, 2026 11:23

@greptile-apps greptile-apps Bot 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.

BigSimmo has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@codeant-ai

codeant-ai Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

🤖 CodeAnt AI — Review Status

Status Commit Started (UTC) Finished (UTC)
✅ Reviewed your PR 58a7ed0 Oct 12, 2026 · 11:23 11:26

@codeant-ai

codeant-ai Bot commented Oct 12, 2026

Copy link
Copy Markdown

Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-12T11:25:59.752481Z 58a7ed0 Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@codeant-ai

codeant-ai Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

🏁 CodeAnt Quality Gate Results

Commit: 560b92d7
Scan Time: 2026-10-12 12:24:37 UTC

✅ Overall Status: PASSED

Quality Gate Details

Quality Gate Status Details
Secrets ✅ PASSED 0 secrets found
Duplicate Code ✅ PASSED 0.0% duplicated
SAST ✅ PASSED No security issues
Bugs ✅ PASSED Rating S: No bugs
IAC ✅ PASSED No IAC issues

View Full Results

@claude

claude Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Claude encountered an error after 2s —— View job


I'll analyze this and get back to you.

@codeant-ai codeant-ai Bot added the size:XS This PR changes 0-9 lines, ignoring generated files label Oct 12, 2026
@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Wait for Admin Help to load before checking section anchors

🐞 Bug fix 🧪 Tests 🕐 Less than 10 minutes

Grey Divider

AI Description

• Wait for Admin Help loading skeletons to disappear before reading the section bar.
• Compare bar labels with anchors on the settled page, avoiding a race seen on iPhone WebKit.
Diagram

graph TD
  A["Anchor test"] --> B["Open Admin Help"] --> C["Loading skeletons"] --> D["Wait for removal"] --> E["Read section bar"] --> F["Check anchors"]
Loading
High-Level Assessment

Keep the targeted wait for loading skeletons to disappear. It synchronizes the existing assertion with the page's settled state without changing production behavior or assuming any particular section has rows.

Files changed (1) +6 / -0

Tests (1) +6 / -0
ui-on-call-boards.spec.tsWait for Admin Help rows before checking section anchors +6/-0

Wait for Admin Help rows before checking section anchors

• Adds a Playwright assertion that all Admin Help loading skeletons are gone before collecting section-bar labels. This prevents the test from checking an anchor for a section, such as Support, that disappears after loading; the anchor assertion itself is unchanged.

tests/ui-on-call-boards.spec.ts

@codspeed

codspeed Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Merging this PR will not alter performance

⚠️ 2 benchmarks spent significant time in system calls

System calls cannot be consistently instrumented, so they are not included in the measure, which understates the real cost. Please switch to the Walltime instrument to accurately measure system calls.

Measurement and system calls

ℹ️ 1 benchmark has no flame graph

The flamegraph could not be generated because too much time was spent in runtime internals, making the visualization not useful for performance analysis.

✅ 8 untouched benchmarks


Comparing claude/fix-webkit-admin-help-anchor-race (560b92d) with main (565acb0)

Open in CodSpeed

@gitar-bot
gitar-bot Bot enabled auto-merge (squash) October 12, 2026 11:24
gitar-bot[bot]
gitar-bot Bot previously approved these changes Oct 12, 2026

@gitar-bot gitar-bot Bot 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.

Gitar has auto-approved this PR (configure)

@qodo-code-review

qodo-code-review Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

No findings for this group


Remediation recommended

No findings for this group


Informational

No findings for this group

Grey Divider


Resolved findings

1. Fast SQL test flakes under runner load ✗ Dismissed
Description
The stress test fails when any one of five synchronous stripMigrationSql runs exceeds 100 ms, even
if the implementation remains fast overall. A busy or preempted CI runner can inflate one
performance.now() interval and fail the test without a reproducible SQL performance regression.
Code

tests/challenger-m3-2-sql-and-lighthouse-stress.test.ts[R403-407]

+      const durations: number[] = [];
+      for (let run = 0; run < 5; run++) {
+        const start = performance.now();
+        stripped = stripMigrationSql(largeSql);
+        durations.push(performance.now() - start);
Relevance

●●● Strong

Recent precedent accepts timing-flake findings; maximum-based wall-clock checks directly preserve
the reported CI sensitivity.

PR-#2305
PR-#3488
PR-#3454

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The testing-review skill explicitly requires checking for fragile or flaky tests. The changed test
expands one timing sample into five independently failing samples and applies the same 100 ms
wall-clock threshold to the maximum, making a single transient scheduling delay sufficient to fail
CI.

tests/challenger-m3-2-sql-and-lighthouse-stress.test.ts[403-413]
Skill: testing-review

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The SQL stress test requires every timed run to finish under 100 ms, so transient CI runner contention or scheduler preemption can fail the test without indicating a real regression in SQL stripping performance.

## Fix Focus Areas
- tests/challenger-m3-2-sql-and-lighthouse-stress.test.ts[403-413]

## Recommended Fix
Keep the warm-up and repeated measurements, but use a contention-tolerant aggregate criterion such as a bounded high percentile or median plus an appropriate margin. Preserve assertions that the stripped output is correct and retain enough repeated runs to detect consistently slow implementations without making one transient outlier fail the test.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 9 rules
✅ Cross-repo context — repo relationships
✅ REVIEW.md
Review mode: Auto: 🚀 Fast: Localized test timing change with contained, low-risk behavior.

Grey Divider

Tip of the day
💡 Did you know, you can commit Qodo's fix in one click with committable suggestions (GitHub & GitLab)

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@qodo-code-review

Copy link
Copy Markdown
Contributor

Qodo Fixer

No findings are available for this PR yet. Findings appear here once Qodo has reviewed the PR.

@codeant-ai

codeant-ai Bot commented Oct 12, 2026

Copy link
Copy Markdown

CodeAnt PR Risk: Low Risk

  • The PR appears safe to merge; it waits for section loading to finish before checking the navigation bar.

Assessed commit: 58a7ed0949db

The 100 ms ReDoS ceiling failed on two unrelated PRs (about 102 and 104 ms)
because a single cold run under coverage on a busy runner can cross it.
Catastrophic backtracking slows every run, so the fastest of five keeps the
guard while dropping warm-up and runner noise.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DCghuougzFFXRF3dQQEcYr
(cherry picked from commit 0041536)
gitar-bot[bot]
gitar-bot Bot previously approved these changes Oct 12, 2026

@gitar-bot gitar-bot Bot 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.

Gitar has auto-approved this PR (configure)

Copy link
Copy Markdown
Owner Author

Unit coverage partition (1) failed on one unit test this PR does not touch: the ReDoS timing check in tests/challenger-m3-2-sql-and-lighthouse-stress.test.ts measured one cold run at about 104 ms against its 100 ms ceiling. #3492 hit the same check at about 102 ms. Fixed in #3494, which keeps the fastest of five runs against the same ceiling, and that commit is ported here so this PR does not wait on it.


Generated by Claude Code

@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit e49d1d6

Copy link
Copy Markdown
Owner Author

Gitleaks failed while downloading the scanner (HTTP 504 from the GitHub release download), before any scan ran. That is a network error, not this change. I have re-run the failed job once.


Generated by Claude Code

@gitar-bot gitar-bot Bot 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.

Gitar has auto-approved this PR (configure)

@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit cbd58be

@qodo-code-review

qodo-code-review Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

No code changes since the last review — review skipped

Qodo Logo

@gitar-bot

gitar-bot Bot commented Oct 12, 2026

Copy link
Copy Markdown
Contributor
Code Review ✅ Approved

🟢 Low risk · The added wait only delays an existing UI assertion until loading placeholders disappear; it changes test timing, not application behavior.

Test-only change so the Admin Help section bar is read only after its loading skeletons clear, keeping the bar and anchor checks on the same settled page. No issues found.

Auto-approved: No blocking issues found.
Please see Auto-approve Docs for details on setting custom approval criteria.

Review coverage

🤖 Auto-approval Approved

📋 Rules No rules evaluated

🧪 Functional validation Not enabled · Set up

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Counting what did not apply, without listing it.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@gitar-bot
gitar-bot Bot merged commit 8e3896f into main Oct 12, 2026
39 checks passed
@gitar-bot
gitar-bot Bot deleted the claude/fix-webkit-admin-help-anchor-race branch October 12, 2026 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🔴 Large blast radius 🕐 Less than 10 minutes size:XS This PR changes 0-9 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants