docs: clarify schema helper typing follow-ups#1984
Merged
Conversation
There was a problem hiding this comment.
LGTM. Follow-ups noted below. Tight follow-up to #1981 — the prose addition pulls its weight, the redundant intersection drop is type-safe via the mapped portion, and the type-test grows the regression surface that #1981 was about.
Things I checked
update_rightsremoval is safe.KnownToolRequestSchemas['update_rights']atsrc/lib/utils/tool-request-schemas.ts:101istypeof schemas.UpdateRightsRequestSchema(nowithOptionalAccountwrapper), soShapeOf<…>resolves to exactly the type the explicit intersection used to assert.creative_approvalcorrectly stays — it's not inKnownToolRequestSchemas.- Runtime literal at
src/lib/schemas/index.ts:95still spreadsupdate_rights: schemas.UpdateRightsRequestSchema.shape. Type-correct (mapped portion ofToolInputShapescovers it) and the runtime key stays present. scripts/schemas-ensure.ts:33regex now accepts3.1.0-beta.3directories. Matches the current schema cache pin without further script changes.- Type-test additions at
src/type-tests/zod-object-intersections.type-test.ts:29,33,38exercise.shape.<key>after.pick()/.omit()/.extend()— the exact regression path #1981 fixed. - Changeset amended (not replaced);
Check for changesetCI job is green.
Follow-ups (non-blocking — file as issues)
scripts/schemas-ensure.ts:33— prerelease class[0-9A-Za-z.-]+is a permissive superset of semver §9 (accepts3.1.0-..or trailing dots). Fine today since the only writer issync-schemasechoing the upstream pin, but the strict form[0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*matches the spec, and neither form accepts build metadata (+build.1). The comment promises pin updates land here without script changes — a future+-suffixed pin would silently miss the probe.
Minor nits (non-blocking)
- Runtime spread parity.
src/lib/schemas/index.ts:95keepsupdate_rights: schemas.UpdateRightsRequestSchema.shapeafter the type-level intersection was dropped. A one-line comment noting "kept for parity with the freeze-time entries; mapped type covers the static type" would prevent a future cleanup from deleting line 95 and silently dropping the key at runtime.
Interesting choice of docs: for a commit that drops a type intersection and widens a regex — the changeset amendment is the right shape, the conventional-commit prefix is a touch generous.
Approving.
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.
Summary
Validation