Repository navigation
Name the type that exists, and guard the prose that names types - #356
Merged
Merged
Conversation
…types Auditing what #321 left behind, one live defect in the published docs: `grid/keyboard.mdx` told readers to write `PretableRenderSnapshot["rowMetrics"]` if they needed the type down on paper. #321 renamed that type to `PretableIndexedRenderSnapshot`. Anyone following the sentence got a compile error, and had been for a release. Everything the existing guard covers behaved perfectly through that rename: the reports were regenerated, every bound member table moved with them, the import check passed. The sentence is prose, and prose is exactly where escape hatches like this one live — "not exported under a name, take it as X" is a claim about the API that no table can express. So the sweep the miss argues for: every `Pretable*` identifier NAMED anywhere in the docs must be a real export of some reported package. Run across the whole corpus it found exactly one violation — this one — so it lands fail-closed with no allowlist, which is the shape a guard should have. It fails with `grid/keyboard.mdx: PretableRenderSnapshot` against the sentence as it was. It reads the whole page rather than only prose. A name is a name wherever it is written, and the narrower version would need the fence-tracking machinery this one does not. Also from the same audit: `heroGrid/selection.ts` still documented its inputs as `grid.getColumns()` and `grid.getSnapshot().visibleRows`. Both were removed by #321; the code moved to `columnLayout` and `range(0, visibleRowCount)` in #336 and the comment did not. A comment naming a removed API is a wrong instruction to the next reader, which is how this class of bug propagates. 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-7ba7q9o0o-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.
First pass of the #321 fallout audit, plus the one live defect it turned up.
The defect
grid/keyboard.mdxtold readers: "renderSnapshot.rowMetricsis public and fully typed, but its interface is not exported under a name — take it asPretableRenderSnapshot["rowMetrics"]if you need to write the type down."#321 renamed that type to
PretableIndexedRenderSnapshot. Anyone following that sentence got a compile error, and had been for a release.Everything the existing guard covers behaved perfectly through the rename: reports regenerated, every bound member table moved with them, the import check passed. The sentence is prose — and prose is exactly where escape hatches live. "Not exported under a name, take it as X" is a claim about the API that no table can express.
The guard
Every
Pretable*identifier named anywhere in the docs must be a real export of some reported package.Run across the whole corpus it found exactly one violation — this one — so it lands fail-closed with no allowlist, which is the shape a guard should have. Mutation-checked: restoring the old sentence fails it with
grid/keyboard.mdx: PretableRenderSnapshot. It also carries the same non-empty assertion the neighbouring checks use, so a corpus it can no longer read fails loudly instead of passing clean.It reads the whole page rather than prose only: a name is a name wherever it's written, and the narrower version would need the fence-tracking machinery this one doesn't.
Also from the audit
heroGrid/selection.tsstill documented its inputs asgrid.getColumns()andgrid.getSnapshot().visibleRows. Both were removed by #321; the code moved tocolumnLayoutandrange(0, visibleRowCount)in #336 and the comment didn't. A comment naming a removed API is a wrong instruction to the next reader — which is how this class propagates.What the audit checked and found clean
useSyncExternalStorecall site is either UI-event driven or already snapshot-scoped. The row-layout controller publishesrebuildingonce andreadyonce, not per slice; this also corrects a claim in Stop repainting the grid on every rebuild slice; stop the bench calling a stalled interaction settled #352's description, where I named it a suspected second amplifier. The residual scheduler hops are its own legitimate slices, not amplification.PretableGrid,PretableGridSnapshot,PretableRenderSnapshot,UsePretableOptions,RowMetricsReader,PretableBaseColumn,PlannedColumn). All remaining references are in dateddocs/superpowers/plans/, which are historical execution recipes, except the one fixed here.pretable-adapter.tsx:328,setQuery(surfaceQuery as never)). It is not hiding a defect: bench: the group script applies no grouping and reports completed #327 proved the payload correct. Left as-is rather than widened into this PR.setQuery— all remaining callers are fire-and-forget or awaitfinished.Verification
pnpm test(full monorepo; the onemulti-sort-surfacefailure is the known load flake — the package passes 829/829 on its own), lint, typecheck, format all clean. Rebased onto #353 and re-verified, since that renamed public types: the new guard passes, which is itself a small piece of evidence for it.🤖 Generated with Claude Code