Skip to content

test(ui): start the iPad tablet project with the iOS install hint dismissed - #3492

Merged
gitar-bot[bot] merged 6 commits into
mainfrom
claude/fix-webkit-tablet-install-hint
Oct 12, 2026
Merged

gitar-bot[bot] merged 6 commits into
mainfrom
claude/fix-webkit-tablet-install-hint

Conversation

@BigSimmo

@BigSimmo BigSimmo commented Oct 12, 2026 •

Copy link
Copy Markdown
Owner

User description

Requested by Josh · project thread

Summary

Before: once #3487 let WebKit launch, the tablet (iPad Mini) legs of the release browser matrix showed about 21 failures (run 38236585559). Most were clicks that timed out with "pwa-notice-stack subtree intercepts pointer events". The app offers the one-time "Add to Home Screen" sheet to iPad user agents as well as iPhones (isIosBrowser() in pwa-lifecycle.tsx). The iPhone projects already start from the dismissed state, but tablet started from a first visit, so the sheet covered the page in every tablet journey.

After: the tablet project starts from the same dismissed state as the iPhone projects. The one first-visit test in ui-pwa.spec.ts that proves the sheet shows, dismisses and stays dismissed now also runs on tablet, so hiding the sheet elsewhere cannot hide it breaking on iPad.

How: storageState: iosInstallHintDismissed added to the tablet project in playwright.config.ts, and tablet added to the hint test's project list. No test skipped, disabled or quarantined.

Areas touched: Delivery and assurance

Verification

Risk and rollout

  • Risk: low, test configuration only. No app code changes.
  • 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

Starts the iPad tablet Playwright project with the iOS install hint already dismissed, so the "Add to Home Screen" sheet no longer covers the page and breaks tablet test journeys, and makes the SQL stripper timing guard resistant to slow-runner noise.

  • The iPad user agent gets the same install sheet as iPhones; starting from a first visit caused ~21 WebKit test failures once the browser could launch, so tablet now starts from the same dismissed state as the iPhone projects.
  • The one first-visit install hint test now also runs on tablet, so hiding the sheet elsewhere cannot hide it breaking on iPad.
  • Times the SQL stripper as the fastest of five runs, so a single cold run under coverage no longer trips the 100 ms ReDoS ceiling.

Test changes only; no app code changes.

Written for commit aacd518. Summary will update on new commits.

View guided diff Turn on auto-fix


CodeAnt-AI Description

Keep iPad browser tests clear of the install hint and verify its dismissal

What Changed

  • iPad browser journeys now start with the “Add to Home Screen” hint dismissed so it does not cover unrelated interactions.
  • The first-visit hint test now also runs on iPad, checking that the hint appears, can be dismissed, and stays dismissed after a reload.

Impact

✅ Fewer iPad browser-test clicks blocked by the install hint
✅ iPad install prompt behavior remains verified

💡 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.

…missed

The app offers the manual Add to Home Screen sheet to iPad user agents as
well as iPhones. The tablet project started from a first visit, so once
WebKit could launch the sheet intercepted clicks across the tablet shards
(release-browser-matrix run 38236585559). Start tablet from the dismissed
state like the iPhone projects, and extend the one first-visit hint test
to the tablet project so hiding it elsewhere cannot hide it breaking.

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: 100a8b64-a503-45b3-880c-441dcb729724

  • 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.

@codeant-ai

codeant-ai Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

🏁 CodeAnt Quality Gate Results

Commit: aacd518a
Scan Time: 2026-10-12 12:45:52 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

@codspeed

codspeed Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Merging this PR will not alter performance

⚠️ 1 benchmark 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

✅ 8 untouched benchmarks


Comparing claude/fix-webkit-tablet-install-hint (aacd518) with main (8e3896f)

Open in CodSpeed

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)

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

codeant-ai Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

🤖 CodeAnt AI — Review Status

Status Commit Started (UTC) Finished (UTC)
✅ Incremental review completed aacd518 Oct 12, 2026 · 12:24 12:24
✅ Reviewed your PR 5e1ef0c 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

@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.

@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:27.542078Z 5e1ef0c 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.

@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

Start iPad tests with the iOS install hint dismissed

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

Grey Divider

AI Description

• Start tablet journeys with the install hint dismissed so its overlay cannot intercept clicks.
• Test the hint’s first-visit, dismissal, and reload behavior on iPad as well as iPhone.
Diagram

graph TD
  Config["Playwright config"] --> State["Dismissed state"] --> Journeys["Tablet journeys"]
  State --> Override["Test override"] --> Fresh["First visit"] --> Assertions["Hint assertions"]
Loading
High-Level Assessment

Using the existing dismissed storage state for tablet journeys while retaining a first-visit test is the appropriate approach. Dismissing the sheet in every journey would add repetitive setup and make unrelated tests depend on the overlay.

Files changed (2) +8 / -5

Bug fix (1) +4 / -1
playwright.config.tsInitialize tablet contexts with the iOS hint dismissed +4/-1

Initialize tablet contexts with the iOS hint dismissed

• Applies the existing iPhone install-hint dismissal storage state to the iPad Mini project, preventing the sheet from intercepting tablet journey clicks.

playwright.config.ts

Tests (1) +4 / -4
ui-pwa.spec.tsExercise the first-visit install hint on iPad +4/-4

Exercise the first-visit install hint on iPad

• Adds the tablet project to the test that starts with empty storage and checks that the hint appears, can be dismissed, and stays dismissed after reload.

tests/ui-pwa.spec.ts

@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 code fails on runner noise ✗ Dismissed
Description
The benchmark asserts that the maximum of five performance.now() samples is below 100 ms, so one
scheduler pause or busy runner can fail an otherwise healthy implementation. This affects CI
reliability because every measured run must pass independently, rather than allowing bounded
environmental variance in the performance check.
Code

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

+      expect(Math.max(...durations)).toBeLessThan(100); // Every warm run must be very fast
Relevance

●● Moderate

Timing-noise concerns are credible, but relaxing the maximum could weaken intentional regression
detection.

PR-#2305
PR-#2454

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The testing review skill explicitly requires checking for fragile or flaky tests. The changed
benchmark measures five wall-clock runs and fails on the maximum, so a single transient scheduling
delay can fail CI even when the SQL stripper remains performant.

tests/challenger-m3-2-sql-and-lighthouse-stress.test.ts[399-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 performance test fails when any one of five warm runs exceeds 100 ms, even if the implementation is fast and the runner experiences a transient scheduling pause. This makes the test vulnerable to intermittent CI failures.

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

## Recommended Fix
Keep multiple warm measurements, but use a timing assertion that is resilient to isolated runner noise while still detecting regressions, such as requiring a high percentile or a majority of runs to remain below the threshold. Preserve the output-content assertions and document the chosen tolerance so the test does not mask sustained performance regressions.

ⓘ 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-only timing assertion change with limited blast radius.

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 aligns the tablet project’s starting state with the iPhone projects and includes tablet coverage for the install hint.

Assessed commit: aacd518aadda

@gitar-bot
gitar-bot Bot enabled auto-merge (squash) October 12, 2026 11:40
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 102 ms against its 100 ms ceiling. #3493 hit the same check at about 104 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 6053696

Copy link
Copy Markdown
Owner Author

Run benchmarks failed during CodSpeed's own setup step: the service returned an error page and the benchmarks were skipped, so nothing in this PR ran. That is the benchmark service, 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 17b984f

@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 1 closed / 1 findings

🟢 Low risk · Playwright state and SQL-stripper timing test changes affect test setup and regression checks only, leaving application behavior and interfaces unchanged.

Starts the iPad tablet Playwright project with the iOS install hint dismissed and extends the first-visit hint test to cover tablet, so the install sheet no longer blocks tablet journeys. Consider asserting on the fastest or median warm SQL stripper run in tests/challenger-m3-2-sql-and-lighthouse-stress.test.ts instead of requiring all five runs under 100 ms, since a single GC pause or scheduler stall under coverage can fail the test without adding ReDoS coverage.

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

✅ 1 closed
Closed: Edge Case: Max-of-five timing brings back the CI flake the min was meant to fix

📄 tests/challenger-m3-2-sql-and-lighthouse-stress.test.ts:399-413
stripMigrationSql always runs on the same input, so catastrophic backtracking would make every run slow, and the fastest run would still be over 100 ms. That means the new comment is wrong when it says a fastest-run statistic could hide a regression. Requiring all five warm runs to stay under 100 ms adds no ReDoS coverage. It does mean a single GC pause or scheduler stall on a busy runner under coverage can fail the test, which is the noise the previous commit was working around (one cold run took about 102 ms). Keep the warm-up call, but assert on the fastest run (or the median) instead of the slowest.

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 1c0f3de into main Oct 12, 2026
43 of 45 checks passed
@gitar-bot
gitar-bot Bot deleted the claude/fix-webkit-tablet-install-hint branch October 12, 2026 12:45
gitar-bot Bot pushed a commit that referenced this pull request Oct 12, 2026
WebKit now runs in CI and its genuine failures were fixed test-side in
#3492-#3494 and #3501-#3504. The last three were the same race, a keypress
before React attached the handler; the update names the helper to use and
the one check left (the first main run after #3504).


Claude-Session: https://claude.ai/code/session_01DCghuougzFFXRF3dQQEcYr

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants