Close the three smoke-coverage gaps — and fix the premise the plan got wrong - #410
Open
blove wants to merge 8 commits into
Open
Close the three smoke-coverage gaps — and fix the premise the plan got wrong#410blove wants to merge 8 commits into
blove wants to merge 8 commits into
Conversation
Controlled checklist filtering, group collapse by child identity rather than row counts, and a surface where a server actually applies the query — the last of which has no test today because nothing on the site fetches rows per query. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Vercel preview readyPreview: https://pretable-d36dj0iwr-cacheplane.vercel.app Updated automatically by the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 drivesStatus— anenumcolumn with nooptions, 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.
queryControlledonly suppresses the engine applying the transition fromgrid.setQuery(); thequeryprop is reconciled separately (use-pretable.ts:352-372) against therowsprop 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.routeinterceptor returningSERVER_ROWSuntouched — rendered amounts came out perfectly sorted900…55and 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 reads3,s6,s1,…while the screen still renders900…55— so this attribute, not the screen, is what proves the server did the work.toEqual'd, so the query the user expressed is provably the query the server received.SERVER_ROWSis already stored in region order, so without that a do-nothing server satisfies the clustering assertion.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
groupPanelbeing 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
--workers=1: 134 passed, 4 skipped, 0 failedpnpm test/typecheck/lint/format— all cleanscale grid virtualizesfailure did not reproduce against the production buildTwo open calls for the reviewer
queryprop") and break if that ever changes. Currently captured in the spec header comment instead./api/rowsis a generic public path for a test-only endpoint./api/fixtures/rowswould keep the intent visible — the/fixturesnoindex layout does not cover it.Filed separately: #408 (
scroll-behavior: smoothignoresprefers-reduced-motion— an accessibility bug that also causes a WebKit popover race in tests).🤖 Generated with Claude Code