fix(spec): the two remaining shard readers name the file, entry and anchor for a non-string entry (#7076) - #7191
Merged
Conversation
…nchor for a non-string entry (#7076)
…rd-reader-entry-types
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckNo hand-written docs reference the 0 changed package(s). ✅ |
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.
Fixes #7076
aggregateCategoryShardswas fixed by #6751 (PR #7075). The same file holds twomore readers that cast a parsed JSON array to
string[]after validating onlythe container, and neither goes through
categoryOfDefKey— so that fix couldnot reach them. This adds the missing per-entry check at both, reusing #6751's
message skeleton.
The two sites
aggregateApiSurfaceShards—api-surface/is the fourth sharded artifact.ApiSurfaceShard.exportsis declaredstring[], butreadShardsgets there bya
JSON.parsecast, so the declaration is a claim about the file rather than averified fact. A hand-edited non-string row travelled into the returned
Record< string, string[] >and landed in the breadth diff inbuild-api-surface.ts: present in the snapshot, absent from the built surface,therefore counted as a removed export, ending the run on "a REMOVED export …
is a BREAKING change for third parties — bump @objectstack/spec to a new major".
Red, but it sends the author after an export that never existed.
readShardedKeysAtRev(both the sharded and the legacy branch) — thehistorical baseline reader. This one is the reason the card exists: the bad entry
was forwarded into the baseline SET, and the three gates that consume that set
fail three different ways, none of which can name the file the value came from.
Measured: the card's stated consequence is one of three, not the only one
The issue predicts "the deletion gate counts it as a vanished key and demands a
tombstone". Verified against
build-schemas.ts, that is right for one consumerand not the one it reads like. All three were measured:
keys)entry.replace(RETIRED_MARK, '')and dies onreplace is not a function— the same bare-JS-error shape #6751 removed one function up, not a tombstone demandschemas)generatedKeysnorRENAMED_DEFS, so it is reported as a schema that left the published set and aRETIRED_DEFS_BY_MAJORregistration is demanded for a def that never existed — this is the card's framing, exactlycompareAnchorKeys(authorable-surface.base.json)So the card's framing holds, on the
schemaspath; thekeyspath fails earlierand even more bluntly. Both are loud, neither is a bypass, and all three point at
the wrong thing — which is what makes the reader the right place to say it. The
table is recorded in a comment at the site rather than only here.
The message skeleton, and where it had to differ
One shared helper,
nonStringEntryDetail, builds the middle of the message — thefield, the entry index, the JSON type found instead and the offending value, e.g.
keys[1] is a number, not a string (#5837): 12345— and each reader adds its ownsource locator and its own remedy, because those two genuinely differ:
aggregateApiSurfaceShardsthrows, prefixes the shard file(
api-surface/data.json), and closes with "regenerate rather than reconcile byhand";
readShardedKeysAtRevreturns{ error }— its callers inbuild-schemas.tsprint that string under the gate's own name and exit, so athrow from here would escape that framing and print a baseline problem as a
problem with the commit under test. It prefixes the path and the revision
(
authorable-surface/ui.json at 3507e516f8ab) and offers no "regenerate"advice, because the legacy branch reads an already-merged commit and nobody can
regenerate history.
So: same diagnosis, two carriers. That was the open question the card flagged and
it did need a judgement rather than a copy-paste.
aggregateCategoryShardsis deliberately not refactored onto the helper. Itspells the same middle inline; rewriting a landed, pin-tested message to route it
through a new function would churn a contract for no reader's benefit, and the
region is outside this card. The pin tests assert the shared substring on all
three sites, so a future divergence goes red instead of being trusted.
Scope
Diagnostic quality only. No gate changes its verdict, its exit code or its
timing on any input that was previously accepted — the new refusals fire only on
values the generators never write. Nothing was made quieter.
Tests
Three cases, placed beside #6751's coverage in
packages/spec/scripts/sharded-artifacts.test.tsrather than in a new file,because what has to stay true is that these readers say the same thing and
that is only reviewable when the assertions sit next to each other.
Reverse verification — the fix removed with
git checkout origin/main --on thesource file only, tests kept — went red in the predicted direction, and the
direction is not the usual one. None of the three failed by throwing a worse
error; all three failed by returning the corrupt value:
entries: [ 12345, … ]is the card's premise made visible: the number reachingthe baseline set. The other 26 cases in the file — #6751's included — stayed
green throughout, so this adds a verdict without moving an existing one.
Gates run locally, all green:
check:api-surface,check:export-origins,check:generated(11/11 up to date),pnpm --filter @objectstack/spec typecheck,and the package suite (359 files / 9385 tests).
No changeset: build-tooling diagnostics with no user-visible surface, same
disposition as the sibling
4e6ca32(#6751 / PR #7075), which also shippedwithout one.
skip-changesetapplied.Generated by Claude Code