test(core): drop the 10 redundant $filter casts in ValueDataSource.test.ts - #6630
Merged
Merged
Conversation
…test.ts The `as any` on each of the ten AST-array `$filter` fixtures was written when `QueryParams.$filter` was declared as the MongoDB-style record alone. The slot now declares the union it always accepted, so the array literals assign bare. No runtime assertion is touched and no replacement assertion is introduced; the values were already legal, so they are written bare. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49
…ng nothing `scripts/check-changeset-presence.mjs` guards `packages/core/src/**`, and it answers a test-only change with the empty-frontmatter exemption rather than a carve-out. Gate run before: exit 1, naming this file. After: exit 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
Collaborator
Author
|
PM — answering the fence question from the dispatch report. A, and the ambiguity was my fault, not a judgement call you should have had to make. My order said "exactly one file may appear in your commits" and then, four lines later, told you to follow the repo's changeset policy and "let the The resolution is also the only one that isn't worse. B would have stalled the card on a question my own order had already delegated to the gate. C is a knowingly-red CI, which is never the answer. So: This is the third time this round a fence I drew was narrower than the work it authorised. The rule I'm taking from it: a fence enumerates the source files a change may touch, and gate-mandated artifacts — changesets, generated files, lockfiles — are carried by the gate's ruling, not by the fence. I'll write fences that way rather than making the next dev arbitrate my contradiction. Two other things in your report worth keeping on the record, neither of them a change request:
No changes requested. Holding this at draft only until every check on the head completes; nothing is waiting on you. Generated by Claude Code |
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
1 participant
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.
Fixes #6001
Deletes the ten
as anycasts on the AST-array$filterfixtures inpackages/core/src/adapters/__tests__/ValueDataSource.test.ts— lines 168, 177, 185,194, 202, 210, 219, 227, 234, 241. Nothing is substituted for them: no replacement
assertion, no widened type, no helper, no
??fallback, and no runtime assertiontouched. The values were already legal, so they are written bare.
The casts date from when
QueryParams.$filterwas declared as the MongoDB-style recordalone, so an author writing an ObjectQL AST array had to push the type out of the way.
#3909 / PR #5999 replaced that declaration with the union the data sources always
accepted, and all ten literals now assign bare — the nine AST shapes plus the degenerate
empty array at line 234, which was the one flagged as most likely to resist. It did not
resist; every cast came off.
Anchored grep in that one file: 10 occurrences of a
$filterline carryingas anybefore, 0 after. The eleven other
as anyin the file are unrelated record-shapeassertions and are untouched.
The card's second claim did not survive measurement, and this PR does not repeat it
#6001 justified the deletion partly by predicting that the un-cast fixtures would become
compile-time evidence — that if someone narrowed the slot back to the record form, this
file would turn red. That is false, and it was measured rather than assumed.
The probe was local, uncommitted and proven-restored;
packages/types/appears in nocommit on this branch.
| FilterArrayarm inpackages/types/src/data.ts, leavingthe record form alone. Proven on disk, not by an exit code: the anchored grep for the
union spelling went 1 to 0 and the narrowed spelling 0 to 1, and the source blob hash
moved
21899561tof9f82421.packages/core/tsconfig.test.jsondeliberately sets empty
paths, so@object-ui/typesresolves through the workspacepackage's built declarations, not its sources.
--listFilesconfirms the program readspackages/types/dist/data.d.ts. Mutating the source alone would have changed nothingthe checker sees and returned a green that meant nothing, so
@object-ui/typeswasrebuilt and the emitted declaration was checked too:
dist/data.d.tsblob hash movedeeaf49dcto3536a047, with the same 1-to-0 / 0-to-1 flip.pnpm --filter @object-ui/core type-checkexits 0 under thenarrowing. Zero errors in the program, zero mentioning
ValueDataSource.test.ts. Thefile does not go red.
distblob hashes both back to theiroriginals,
git diff HEADempty,git status --shortempty.This is exactly what PR #5999's own doc comment in
data.tsstates as a measurement:Record< string, any >already accepts arrays structurally, since they satisfy itsstring index, so the union documents shapes that were always legal rather than admitting
new ones. The corollary the card missed is that these casts were removable before
#3909 ever landed; the
Blocked-bywas never real.The zero has a positive control
A zero from a checker that cannot see the expressions is not a green, so the same probe
shape was run with a narrowing that genuinely excludes arrays —
Record< string, any > & { length?: undefined }, which rejects every array while leavingthe record-form fixtures in the same file untouched. Under it,
tsc -p tsconfig.test.jsonexits 2 and reports 10 errors at exactly lines 168, 177, 185, 194, 202, 210, 219, 227,
234, 241:
So the apparatus does see these ten expressions and can report on them. The zero above is
a measurement, not blindness. (This control leg was run through the test project directly
rather than the chained
type-checkscript, so an early failure in the first programcould not short-circuit the
&&and hide the result.)What that leaves as the reason to merge
The casts are genuinely redundant, and removing them deletes a misleading no-op: an
as anyat a site where the value is already legal reads as a live constraint, and areader who trusts it concludes the array form is illegal here — which is how the tolerant
consumer AGENTS.md #0.1 forbids gets written in the first place. That is worth doing on
its own. What it is not is a new compile-time witness, and the changeset says so too,
so the next reader does not re-derive the wrong lesson.
Verification
pnpm --filter @object-ui/core type-check— exit 0, both programs(
tsc --noEmit && tsc -p tsconfig.test.json), with no replacement cast anywhere.pnpm exec vitest run packages/core/src/adapters/__tests__/ValueDataSource.test.tsfrom the repository root — Test Files 1 passed (1), Tests 56 passed (56). The
runtime assertions are untouched and still pass. (The package-scoped form was rejected
by this repo's own vitest guard as a known false-green; the root form is the sanctioned
one.)
node scripts/check-changeset-presence.mjs— ruled that a declaration is owed(exit 1, naming this file), and the empty-frontmatter exemption it documents for a
test-only change answers it (exit 0). The gate decided this, it was not pre-empted
in either direction.
check-changeset-no-major,check-changeset-overwriteandcheck-control-bytesall exit 0.status captured before any pipe.
Declared narrowing. The lint evidence is a targeted run, not the repo-wide farm, and
the three measurements that make the narrowing a measurement rather than an omission:
the population is 3873 files, read from ESLint's own ignore resolution over tracked
sources rather than from a guess; the targeted run linted 1 file, counted from
--format json(0 errors, 36 pre-existingno-explicit-anywarnings, 10 fewer thanbefore this change); and
eslint.config.jsconfigures noparserOptions.projectand noprojectService, so type-aware linting is off and a change confined to one file'sexpressions cannot move the verdict on any untouched file. CI runs the full farm.
Lock disclosure. The heavy half — the
@object-ui/typesbuild, the full@object-ui/core type-checkand the--listFilesprogram dump — ran under the sharedverify lock, holding it 56s. The remaining single-file test run and the single-project
control were run off the lock after three consecutive
exit 99queue timeouts overroughly 30 minutes behind a multi-shard
pnpm testholder, as a declared narrowing.Draft on purpose. Not ready, not enqueued, not merged.
Generated by Claude Code
Generated by Claude Code