Skip to content

Close the three smoke-coverage gaps — and fix the premise the plan got wrong - #410

Open
blove wants to merge 8 commits into
mainfrom
blove/smoke-audit-3878ef
Open

Close the three smoke-coverage gaps — and fix the premise the plan got wrong#410
blove wants to merge 8 commits into
mainfrom
blove/smoke-audit-3878ef

Conversation

@blove

@blove blove commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

A manual Chrome smoke of sorting/filtering/grouping on 2026-08-14 verified most flows but left three gaps. This closes them. Plan: docs/superpowers/plans/2026-08-14-smoke-coverage-gaps.md.

What was untestable, and now isn't

1. Controlled checklist filtering (e2e/controlled-query.spec.ts). The manual pass opened a non-checklist funnel and toggled nothing. This drives Status — an enum column with no options, so the checklist is built from distinct row values.

2. Group collapse/expand. The manual pass counted rendered rows, which virtualization makes meaningless: measured, [data-pretable-row] is 9 before collapse and 9 after. A count-based assertion reports "collapse does nothing" — which is exactly what it wrongly reported. Now asserted by child identity.

3. A server that actually applies the query (/api/rows, /fixtures/server-query, e2e/server-query.spec.ts). Nothing on the site fetched rows per query, so this had never been exercised.

The plan's central premise was wrong, and the tests are better for it

The plan asserted that in controlled mode the grid "applies nothing", so whatever the server returns is what renders. False. queryControlled only suppresses the engine applying the transition from grid.setQuery(); the query prop is reconciled separately (use-pretable.ts:352-372) against the rows prop the fixture also passes.

Consequence: every screen-level assertion the plan specified would have passed against a server that ignored the query entirely. Demonstrated with a page.route interceptor returning SERVER_ROWS untouched — rendered amounts came out perfectly sorted 900…55 and the East filter still showed 3 rows. The plan's Task 3d would have been a test of nothing.

The implementer caught it and the reviewer refuted it independently before reading the reasoning. The redesign:

  • data-server-row-ids — the server's answer verbatim, in arrival order, published before the engine touches it. Asserted as an exact id string per test. A saboteur reversing the response makes it read s3,s6,s1,… while the screen still renders 900…55 — so this attribute, not the screen, is what proves the server did the work.
  • Whole request body parsed and toEqual'd, so the query the user expressed is provably the query the server received.
  • Grouping sorts by amount first, deliberately scattering regions — SERVER_ROWS is already stored in region order, so without that a do-nothing server satisfies the clustering assertion.
  • Every remaining screen assertion is kept as corroboration and labelled in-comment as "corroborating, not probative", with a note on the probative line telling the next editor not to trim it and keep the weaker one.

Three other plan errors the implementation corrects: the sort cycle is absent→desc→asc (the plan expected ascending first), the column menu is gated on groupPanel being enabled, and the plan's "group headers only exist when the engine groups" note is stale — the engine does hold the query, so it does group.

Verification

  • Full website e2e against a production build, both engines, --workers=1: 134 passed, 4 skipped, 0 failed
  • pnpm test / typecheck / lint / format — all clean
  • Every assertion mutation-tested. Four independent mutations of Task 3 (server ignores sort, ignores filters, frozen fetch, extra body field) each produced a real failure; the reviewer ran its own set
  • The pre-existing webkit scale grid virtualizes failure did not reproduce against the production build

Two open calls for the reviewer

  1. Should the reversed-response interceptor ship as a permanent test? It is the sharpest statement of the trap, but it would pin current engine behaviour ("a controlled grid re-applies its query prop") and break if that ever changes. Currently captured in the spec header comment instead.
  2. /api/rows is a generic public path for a test-only endpoint. /api/fixtures/rows would keep the intent visible — the /fixtures noindex layout does not cover it.

Filed separately: #408 (scroll-behavior: smooth ignores prefers-reduced-motion — an accessibility bug that also causes a WebKit popover race in tests).

🤖 Generated with Claude Code

@vercel

vercel Bot commented Aug 14, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
pretable Ready Ready Preview Aug 14, 2026 8:27pm

@github-actions

Copy link
Copy Markdown
Contributor

Vercel preview ready

Preview: https://pretable-d36dj0iwr-cacheplane.vercel.app
Commit: a2afbb483b12ebea8c72b5b83f2cc2c2cbbfc686

Updated automatically by the deploy-preview job.

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.

1 participant