Land the ResolvedStyleSnapshot: schema v1, interned, with a generated TS mirror - #454
Land the ResolvedStyleSnapshot: schema v1, interned, with a generated TS mirror#454Alek99 wants to merge 3 commits into
Conversation
… TS mirror python/xy/styling/resolved.py defines the renderer-neutral styling IR the compatibility program converges on: authored styling arrives from five mechanisms and (soon) two resolvers, and renderers should consume exactly one shape regardless of where it came from. Three contract properties, each enforced at construction on both ends of the eventual wire: Concrete values only. No var()/calc()/env()/inherit and no relative units: a value that still depends on a cascade, or on metrics the consumer would have to re-derive, is rejected loudly with the reason (§28). One unresolved value smuggled into the IR would re-create exactly the per-renderer divergence the IR exists to end. Interned declarations. A snapshot stores each distinct declaration once; instances reference it by index and carry only identity qualifiers (e.g. ["y","major","3"]), resolved geometry, and content. Interning is canonicalized, so a builder fed the same styling in any order emits the same snapshot. The dense-axis fixture (400 tick labels + 60 legend rows = 460 instances, 2 declarations) serializes to ~38.6 KB against the spec's 50 KB uncompressed budget, and the test states the headroom so eating it is a visible decision. Closed vocabulary per version. Schema v1's property list (paint, typography, layout, effects) is a generated constant in both languages; growing it is a STYLE_SNAPSHOT_VERSION bump, so a snapshot's vocabulary is always recoverable from its version field. snapshot_from_payload refuses versions it does not know rather than guessing. js/src/14_style_snapshot.ts is the TypeScript mirror, rendered by scripts/gen_style_snapshot_types.py from the Python module — one schema, two languages, and the suite runs --check so the committed mirror cannot drift (the gen_capability_matrix.py contract, applied to types). The client build typechecks it; nothing imports it yet. Nothing rides the wire in this change, so PROTOCOL_VERSION stays at 12. wire-protocol.md gains §8 documenting the payload shape and reserving the style_snapshot_request / style_snapshot message names for the capture change, which bumps the protocol and carries this schema as its reply.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Greptile SummaryThe PR introduces schema v1 of an interned, renderer-neutral resolved-style snapshot and generates its TypeScript mirror.
Confidence Score: 4/5The PR does not yet appear safe to merge because malformed payload state metadata remains accepted and silently changed instead of being rejected. The token-concreteness issue is fixed, and declaration, environment, and token payload checks were added, but Files Needing Attention: python/xy/styling/resolved.py, tests/test_resolved_style_snapshot.py
|
| Filename | Overview |
|---|---|
| python/xy/styling/resolved.py | Adds the snapshot schema, canonical builder, validation, and decoder; token validation is repaired, but payload state metadata is still silently coerced instead of rejected. |
| tests/test_resolved_style_snapshot.py | Thoroughly covers interning, concrete declarations and tokens, malformed core payload fields, round trips, and generator freshness, but omits non-string state metadata. |
| js/src/14_style_snapshot.ts | Adds the generated TypeScript mirror with string-array metadata and the versioned property vocabulary. |
| scripts/gen_style_snapshot_types.py | Generates and checks the committed TypeScript mirror from the Python schema. |
| spec/design/wire-protocol.md | Documents schema v1 and reserves future transport message names without changing the active protocol. |
Reviews (2): Last reviewed commit: "Harden the snapshot's concreteness contr..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Merging this PR will not alter performance
Comparing 
Footnotes
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 7 files
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
# Conflicts: # CHANGELOG.md
Review follow-ups on the schema, all four in the same direction: the contract the module claims is now the contract it enforces. Relative units are rejected anywhere in a value, not only as a whole-string suffix. translate(50%, 20%), a "2em 1em" shorthand, and a gradient stop at 50% all carry the same document dependency a bare "1.5em" does; the end-anchored check let exactly those through. Cascade keywords went the other way: inherit/unset (plus initial/revert/revert-layer) reject only as the entire normalized value, so a face named "Inheritance Sans" is no longer refused for containing the letters. Tokens share the full string contract with declarations through one helper — a token "1.5em" or an empty string smuggles what a declaration would — and snapshot_from_payload now enforces the identical rules build() does: environment vocabulary and finiteness, and every token through the shared validator. The payload path is the untrusted end of the wire; a snapshot that could only exist by bypassing the builder must not become renderer-facing IR by arriving serialized. build() now emits canonical bytes: declaration slots assigned by content (instance indices remapped), instances sorted by identity. The docstring claimed order-independence while the payload depended on insertion order, and the old test compared declaration sets, which cannot see index drift — it now asserts byte-equal payloads from builders fed the same styling in different orders. Instance order carries no meaning; identity lives in (slot, qualifiers), which is what makes a snapshot cacheable across producers.
1 participant
Stacked on #453 (compatibility modes), which stacks on #452 (preflight) — third change of the styling-compatibility program, and the schema every later phase consumes (slot migration, browser capture, native cascade).
What this adds
python/xy/styling/resolved.py— the renderer-neutral styling IR — plus its generated TypeScript mirrorjs/src/14_style_snapshot.ts(scripts/gen_style_snapshot_types.py, with--checkwired into the suite so the two languages cannot drift; thegen_capability_matrix.pycontract applied to types).Three contract properties, enforced at construction on both ends of the eventual wire:
var()/calc()/env()/inherit, no relative units (em,%,vw, …). A value that still depends on a cascade — or on metrics the consumer would re-derive — is rejected with the reason (§28). One smuggled unresolved value would re-create inside the IR exactly the per-renderer divergence the IR exists to end.["y","major","3"]), resolved geometry, and content. Interning is canonicalized (insertion-order independent). The dense-axis fixture — 460 instances, 2 declarations — serializes to ~38.6 KB against the spec's 50 KB budget, asserted with the headroom stated so eating it is a decision, not drift.STYLE_SNAPSHOT_VERSIONbump.snapshot_from_payloadrefuses unknown versions rather than guessing — a snapshot is cacheable and supplyable across builds, so version humility is load-bearing.Protocol posture
Nothing rides the wire yet:
PROTOCOL_VERSIONstays at 12.spec/design/wire-protocol.mdgains §8 documenting the payload shape and reservingstyle_snapshot_request/style_snapshotfor the capture change, which bumps the protocol and carries this schema as its reply. The client build typechecks the mirror; nothing imports it yet.Evidence
25 new schema tests (interning, order-independence, concreteness rejections with reasons, wire round-trip, malformed-payload refusals, mirror freshness). Full suite: 4012 passed, 109 skipped.
node js/build.mjstypechecks + builds clean; ruff/format/pre-commit clean.Next in the stack: the existing ten native-capable slots routed through this IR behind the
_compile_cachedstyle cache, gated byte/pixel-equivalent.