docs(issues): record the ui-smoke test-deletion near miss - #2483
Conversation
An agent push on PR #2481 rewrote tests/ui-smoke.spec.ts from 6,019 lines to 1,081, dropping 74 of its 82 journey tests, under a commit message that described a narrow comparison change. Squash auto-merge was armed at the time and a gutted suite goes green trivially, so only an unrelated merge conflict kept the deletion off main; the later merge of main restored the file and the squash landed the intended two-file change. No gate would have caught it: nothing asserts a floor on Playwright test count or fails on a large test deletion, and the "never skip, disable, or quarantine a test to get green" rule is agent policy rather than an enforced check. The queued request proposes a per-path ratchet on test counts, in the spirit of the bundle-budget baselines. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NkKHznd5E5KLnKbowRTLd8
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 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. Comment |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_8389685c-a3e8-4ebd-a910-00a544e3b013) |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e682b91c57
ℹ️ 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".
| "summary": "An agent push rewrote tests/ui-smoke.spec.ts from 6,019 lines to 1,081, deleting 74 of 82 UI journey tests, and only a merge conflict stopped it auto-merging", | ||
| "detail": "Observed 2026-08-31 on PR #2481 (the answer-surface colour fix). Commit d1485d6e8, message 'test(ui): compare answer status surfaces', authored via the BigSimmo account, changed exactly one file with 4 insertions and 4,924 deletions: tests/ui-smoke.spec.ts fell from 6,019 lines / 82 tests to 1,081 lines / 8 tests. The commit message describes a narrow comparison change, so the diff is almost certainly an accidental whole-file rewrite by a tool that regenerated the file instead of editing a region of it — the surviving 8 tests are a contiguous slice, which is the signature of that failure mode. WHY IT MATTERS BEYOND THE ONE COMMIT: squash auto-merge was armed on the PR at the time. A gutted suite goes green trivially, so had the branch merged cleanly the deletion would have reached main automatically with no human reading the diff. The only thing that actually prevented it was an unrelated merge conflict against a main that had moved on; the subsequent merge of main into the branch restored the file, and the squash landed only the intended two-file colour change (verified by reading main at e16d5f66d). So no harm occurred, by luck rather than by any gate. NOTHING IN CI WOULD HAVE CAUGHT IT: no check asserts a floor on the number of Playwright tests or fails on a large deletion in a test file, and AGENTS.md's 'never skip, disable, or quarantine a test to get green' rule is agent policy, not an enforced gate. check:dead-code-candidate guards exported symbols, not test bodies. SUGGESTED REMEDY, cheapest first: a static check that fails when a committed diff removes more than a threshold share of test cases from tests/**, in the spirit of the existing bundle-budget ratchets — a count of test( occurrences per spec file, ratcheted per path, so a deliberate reduction is a reviewed baseline change rather than a silent one. Separately worth considering: do not arm auto-merge on a branch that any agent session is still pushing to.", |
There was a problem hiding this comment.
When this request is reconciled or used to define the proposed ratchet, it records an incorrect denominator: tests/ui-smoke.spec.ts at the cited landed commit e16d5f66d has 89 direct test(...) declarations—82 with literal names and 7 with template-literal names inside viewport loops—not 82 total tests. The stated 74-test loss therefore comes from counting only test("... declarations and understates both the deletion and the baseline the guard must preserve; update the counts using a form that includes all test declarations.
AGENTS.md reference: AGENTS.md:L428-L433
Useful? React with 👍 / 👎.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
2 participants
Summary
addrequest underdocs/outstanding-issues-inbox/recording a near miss observed on PR fix(ui/answer): keep answer-surface counts neutral and the safety tile amber #2481: an agent push rewrotetests/ui-smoke.spec.tsfrom 6,019 lines to 1,081, dropping 74 of its 82 Playwright journey tests, under a commit message describing a narrow comparison change. Squash auto-merge was armed at the time, and a gutted suite goes green trivially, so only an unrelated merge conflict kept the deletion offmain. The later merge ofmaininto that branch restored the file, and the squash landed only the intended two-file colour change (verified by readingmainate16d5f66d).No product code changes. This PR adds one JSON file and nothing else.
Verification
npm run verify:pr-localIts risk router classified this as recognised low-risk scope, so lint, typecheck, the full unit suite, the RAG fixture scan and the build were skipped by selection rather than by omission.
check:ledger-write-disciplinepassed, and the snapshot check reports the inbox in step withdata/outstanding-issues-snapshot.json(81 open, 77 pending).UI verification not run: no UI, routing, styling, browser, reduced-motion or forced-colors behaviour changed.
Risk and rollout
npm run issues:reconcilerun.Clinical Governance Preflight
Not applicable — this change touches no ingestion, answer generation, search or ranking, source rendering, document access, privacy, production environment, or clinical output path. The diff is one queued issue-tracking record under
docs/outstanding-issues-inbox/.Notes
Two related observations from the same session, recorded here rather than in the ledger because neither is outstanding work:
ui-smokefailure that blocked PR fix(ui/answer): keep answer-surface counts neutral and the safety tile amber #2481 ("review-due source-only answers share one compact expandable status row") is already fixed onmain. It was a measurement race: at 768px the sidebar appears and reflows the page about 9px, and the test's three sequentialboundingBox()calls straddled that reflow, so two pills that were on the same line were compared across two different layouts.mainnow waits two animation frames after each resize before measuring. Confirmed green locally onf7f9d018a, twice.main, not by trusting the merge notice.Generated by Claude Code
Note
Low Risk
Append-only inbox JSON with no runtime or ledger mutation until a separate reconcile run; zero production impact.
Overview
Adds a single version 2
addrequest underdocs/outstanding-issues-inbox/(idc995abce-f7d4-4be2-b057-80b173a5246f) so the near miss on PR #2481 is queued for later ledger reconciliation, not applied in this diff.The payload is a P2 issue describing how an agent commit accidentally rewrote
tests/ui-smoke.spec.ts(82 → 8 Playwright tests), that squash auto-merge would likely have shipped it without a human diff review, and that only an unrelated merge conflict prevented landing onmain. It also notes the gap in CI (no floor on spec test counts) and suggests a per-path ratchet ontest(counts intests/**, similar to bundle-budget baselines.No application, test, or workflow code changes — documentation / issue-tracking queue only.
Reviewed by Cursor Bugbot for commit e682b91. Configure here.