fix(schema): add minProperties guard + finite numeric maximums (P3 schema hardening) - #3
Merged
Merged
Conversation
Runtime now rejects: - animations map with zero entries (InvalidGraphError: "animations must declare at least one animation") — mirrors the existing minProperties:1 already present in the JSON Schema file but previously absent from the compiler. - frame duration or defaultFrameDuration above MAX_DURATION (86 400 000 ms, i.e. 24 hours) — eliminates accidental giant values that would make a single-frame clip play for a day. - state speed multiplier above MAX_SPEED (1 000 ×) — mirrors the > 0 lower-bound guard already present. JSON Schema updated to match: duration and defaultFrameDuration gain maximum: 86400000; speed gains maximum: 1000. The animations block already carried minProperties: 1. 8 new tests added (4 RED → GREEN). All 140 tests green; prepublishOnly passes (typecheck, lint, coverage 100 %, build, verify:dist/exports/llms, check:size). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
yshengliao
marked this pull request as ready for review
June 13, 2026 16:28
There was a problem hiding this comment.
Pull request overview
This PR hardens SpriteGraph validation to better align runtime checks with the published JSON Schema, preventing empty animations maps and rejecting finite-but-absurd numeric values for durations and state speed.
Changes:
- Add runtime guard rejecting
animations: {}with a dedicatedInvalidGraphErrormessage. - Enforce maximum allowed values for
defaultFrameDuration, per-frameduration, and statespeedin the compiler. - Update the JSON Schema to add matching
maximumconstraints, plus add tests covering these bounds.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/validation.test.ts | Adds tests asserting rejection/acceptance at the new MAX_DURATION and MAX_SPEED ceilings and the new empty-animations error message. |
| src/sprite/compile.ts | Adds compiler-time validation for non-empty animations and maximum caps for durations and speed. |
| schemas/aispritejs-graph.schema.json | Adds maximum constraints for frames.*.duration, defaultFrameDuration, and states.*.speed to match runtime validation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
56
to
+60
| export function compileGraph(graph: SpriteGraph): CompiledGraph { | ||
| if (Object.keys(graph.animations).length === 0) { | ||
| throw new InvalidGraphError("animations must declare at least one animation"); | ||
| } | ||
|
|
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
Closes the P3 "Schema hardening" open item from
REVIEW.md.animationsmust be non-empty — the compiler now rejectsanimations: {}withInvalidGraphError("animations must declare at least one animation"), mirroringminProperties: 1already in the JSON Schema file.defaultFrameDurationand per-framedurationare capped atMAX_DURATION = 86_400_000 ms(24 hours). Values above this are rejected withInvalidGraphError.speedmultiplier is capped atMAX_SPEED = 1_000. Values above this are rejected withInvalidGraphError.duration(frames),defaultFrameDuration, and statespeedgain matchingmaximumconstraints so editors and CI validators agree with the runtime.Rationale for chosen bounds
duration/defaultFrameDurationspeedNon-finite values (
Infinity,NaN) were already rejected by the existing guard — these new maximums close the gap for finite-but-absurd values.RED → GREEN evidence
4 tests were added first (RED), then the runtime and schema were fixed (GREEN).
RED run (before fix):
rejects an empty animations map with 'at least one animation' message— FAIL (wrong error message)rejects a frame duration above the 24-hour ceiling— FAIL (did not throw)rejects defaultFrameDuration above the 24-hour ceiling— FAIL (did not throw)rejects a state speed above 1000×— FAIL (did not throw)GREEN run (after fix): 140/140 tests pass.
Test plan
pnpm test— 140 tests, all greenpnpm prepublishOnly— typecheck, lint, coverage 100%, build, verify:dist/exports/llms, check:size all passaispritejs/schemasubpath exports the updated JSON Schema withminProperties:1,maximum:86400000,maximum:1000Notes for leader
STABILITY.mdandREADME.mdalready note that schema hardening improves editor feedback without replacing runtime validation. The new bounds are consistent with that framing; no release-doc edits needed beyond what the leader decides to add at release time.llms-full.txtverified up-to-date (verify:llms passed).🤖 Generated with Claude Code