fix(audit): remediate the September 25 audit across all five crates - #325
Closed
LeadcodeDev wants to merge 43 commits into
Closed
LeadcodeDev wants to merge 43 commits into
LeadcodeDev wants to merge 43 commits into
Conversation
A timeline step has two halves and they obeyed different rules. Its `style` was gated by `apply_style_states`, which merges only the states whose `at` the clock has reached. Its `animation` was merged unconditionally by `effective_effects`, so every step contributed from t=0. With one step that is invisible: the step's own first keyframe is the base state, so holding it changes nothing. With two or more it decides the frame. All keyframes effects share one bucket where the last entry wins on a shared property, and the last entry is the last step — which has not begun, so it imposes the value of its first keyframe from the very start. A node that moves in three beats sits at the end of beat two for the whole scene, and only the final beat ever animates. Passing the component's local time in and filtering the steps the same way `apply_style_states` already does aligns the two halves. The last-effect-wins rule for `style.animation` is untouched: it was chosen deliberately so that composition does not depend on an incidental `delay`, and it is pinned by `last_declared_effect_wins_regardless_of_which_one_carries_the_delay`, which still passes. Found by porting an existing promo animation to a scenario rather than by reading: the logo had to enter centred, move to a corner, and come back, and it never left the corner.
A filled SVG (no stroke) could only draw-on as a wireframe outline: paint_draw_on always strokes each path with draw_stroke_width, so a solid logo traced in thin lines and only became solid the instant draw_progress hit 1.0. There was no way to express "the logo draws itself" for anything but stroke-art icons. Add `reveal: "stroke" | "fill"` (default "stroke", behavior byte-for-byte unchanged for the default: same branch, same functions). `"fill"` instead sweeps a per-path clip mask (left-to-right, path bounds) over the SVG's own already fully rasterized image (paint_resvg's output, now cached via the new `cached_full_image`, factored out of `paint_static` without behavior change). Clipping the pre-rendered raster keeps gradients/patterns intact, which a per-path flat-color fill (the alternative: reuse collect_paths' color resolution and skia-fill each path with a growing rect clip) would have lost — collect_paths already falls back to white for gradient/pattern paints, which is fine for a 2px trace but not for a filled reveal meant to show the real artwork. Path ordering/windowing (length-weighted, draw_overlap) is reused as-is from paint_draw_on so paths still reveal sequentially. Not verified: full ffmpeg render/encode of a `reveal: fill` scenario (only `still` frames and unit tests); behavior with self-intersecting/evenodd paths beyond the fill-rule set on the mask path.
taffy_bridge.rs only wrote style.flex_direction when css.flex_direction was Some(..); when a card/div/flex/grid omitted flex-direction, taffy's own Style::DEFAULT (Row) leaked through unnoticed. SKILL.md documents "column" as the default, and the scene root (box_builder.rs's default_root_css) already sets Column explicitly — that root-level override was masking the mismatch, since every top-level layout looked correct while any nested container silently behaved like Row. Measured before choosing: 75/259 card/div/flex/grid instances across examples/*.json omit flex-direction (0/61 for `flex`, since authors always state it explicitly there; the silent gap is entirely in `card` 45/98 and `div` 30/100). Rendered stills of every affected scene in all 7 affected example files before and after, at each scene's sampled midpoint: 25/27 frames are byte-identical; the remaining 2 (both in ferriskey-launch-60s.json, a pill-nav button row) differ by ~0.14% of pixels, a sub-pixel border AA shift on a single-child card whose content is unaffected by axis choice. No `validate` output regresses on any example (the one ferriskey-presentation.json geometry failure predates this change, confirmed by testing the unpatched binary). Alternative considered: fix SKILL.md to document "row" instead, since that's taffy/CSS's real default. Rejected — the scene root, SKILL.md, and effectively every example in the repo are already written assuming column, so "row" would be the surprising, silently-wrong default for the LLM authors this schema targets, not less so than today. Reserve: cargo test --workspace surfaces 7 failures in crates/rustmotion/src/cli/commands/geometry.rs (out of this commit's file scope), isolated to be caused by this change and not concurrent work (confirmed by reverting only this file against the current tree). Root cause: a fixed-height card/div/flex with a single oversized child and no explicit flex-direction used to have that child's cross-axis clamped to the card's declared size under Row+stretch, so a remeasurement pass reliably caught the mismatch as ContentOverflowsBox. Under Column, the child's main-axis (height) is no longer clamped and grows to its natural content size instead, so the layout box already "matches" its own content and that check stops firing. When the grown box also happens to cross the viewport edge, check_viewport still catches it (reclassified, not lost) - but when it doesn't, or when the node carries bleed: true, the overflow escapes detection entirely. Reproduced with a minimal fixture (card w=300 h=80, one wrapped text child needing ~343px, positioned so the grown box stays inside the viewport): the unpatched binary reports ContentOverflowsBox; the patched one reports nothing. This is a real, if narrow, hole in the geometry validator's coverage for a common pattern (45/98 card instances in examples/*.json have no explicit flex-direction), not mere reclassification, and fixing it requires touching geometry.rs's own overflow checks - outside this commit's file scope. Needs escalation before this lands.
… start_at start_at opened a component's visibility window (PaintWindow, already correct) but never touched the clock its animation effects resolve against — they kept running on raw scene time. An entrance already playing out by the time the node became visible snapped straight to its end state instead of animating; an exit whose own `delay` had elapsed before `start_at` left the node painting nothing for its whole visible window (measured: a badge with start_at: 2.0 rendered zero pixels at every sampled instant, its exit having completed at scene time 1.15). Fix: fold the node's own `start_at` into the same `extra_delay` that a container's `stagger` already contributes in `build_child` (box_builder.rs) — same mechanism, same call sites (effective_effects, apply_style_states, resolve_transition_css_overrides, ghost generation), so `start_at` rebases exactly like stagger already did. `BuiltScene.stagger_delays` now carries this combined delay per node, which legacy_dispatch.rs and geometry.rs (the --strict-anim overflow sampler, outside this change's scope) both already read from — geometry.rs picks up the fix for free without being touched. `paint_decorative_fullscreen` (scene.rs) gets the same treatment for full-viewport leaves, which have no stagger of their own to fold in. Considered rebasing `BuildAnimationCtx.time` itself instead of shifting each effect's `delay`. Rejected: `stagger` already uses the delay-shift form (see `effective_effects`), and the two forms are only equivalent as long as nothing downstream reads the un-shifted clock directly — mixing them would have made that invariant easy to break later. Reusing the proven form keeps `start_at` and `stagger` composing through the identical path. Does not touch `end_at`, `delay` without `start_at` (still resolves against scene time, unchanged), or the timeline-step gate from b9cf23a (`last_declared_effect_wins_regardless_of_which_one_carries_the_delay` still passes) — `t` passed into `effective_effects` stays the raw scene clock, only `extra_delay` grows. Measured against every examples/*.json (43 rendered frames across all 11 files, before/after, pixel-diffed): zero visual change. Only two example components declare start_at at all (both `counter`, mega-showcase.json and rustmotion-promo.json), and counter's own progress reads ctx.time directly rather than going through the effects pipeline this change touches, so even those are unaffected.
…s box 8afc4c1 (default flex-direction: column) fixed a real layout bug but opened a detection hole here. check_content_overflows_box/check_auto_scroll both compared a leaf's measured content against its OWN post-layout box (layout.content_box()). Under the old row default, align-items: stretch clamped a lone child's cross axis (height) to its container's declared size, so a too-small card produced a real box/content mismatch these checks caught. Under column, that axis is the MAIN axis, which stretch never clamps — the child's box now legitimately grows to match its content exactly (CSS min-height:auto-style overflow), so the self-vs-self comparison became vacuous: the box IS the content, by construction. The paragraph still paints past its card, invisibly to both checks, unless the grown box happens to also cross the viewport edge. Fix: thread the nearest containing block's own resolved content box (container_bound) down through walk(), and clamp an in-flow child's effective box to min(own, container) per axis before comparing. A card's own box stays true to its declared size regardless of its children's overflow (ordinary CSS containing-block behavior), so this recovers exactly the bound the old row-direction clamp used to provide, without resurrecting row as the default. Absolute children are excluded — taken out of flex flow entirely, their box is legitimately sized from their own content only, per the already-passing absolutely_positioned_*_spilling_past_a_visible_card_is_legal tests. That exclusion checks box_node.css.position == Some(Position::Absolute), the exact condition box_builder.rs uses to decide the same thing, not ChildComponent::is_flow() (a stricter, unrelated predicate that's false for any declared `position` shorthand, "absolute" or not, and isn't otherwise read by the layout pipeline). Considered parsing each node's own declared style.width/height directly instead of reading the parent's resolved layout box. Rejected: the overflowing leaf (text/table/codeblock/...) usually has no explicit size of its own — the fixed size lives on the ancestor card — and the parent's resolved box already gives the same number without re-deriving unit/ percentage resolution that CssStyle -> px conversion already does once, correctly, during layout. Of the 7 previously-red tests: 6 were real detection losses, now fixed by container_bound (in_flow_codeblock_shrunk_by_its_card_is_caught_by_auto_ scroll_check, in_flow_table_taller_than_its_card_is_flagged_via_content_ overflows_box, bleed_true_does_not_exempt_content_overflows_box, gradient_text_taller_than_its_fixed_height_card_is_flagged, without_text_autofit_the_same_fixture_still_overflows, text_autofit_does_not_silence_an_overflow_the_floor_cannot_fix). The 7th, wrapped_text_taller_than_its_fixed_height_card_is_flagged, was a genuine reclassification unrelated to this file's fix: its y=200 fixture's grown (unclamped) box now happens to cross the 540px frame edge by ~3px for real, so check_viewport correctly starts firing ViewportOverflow alongside — moved the fixture to y=100 (restoring the isolated-repro invariant its own docstring claims) rather than weakening the assertion, and added a dedicated regression test using the audit's own 1920x1080 numbers. Verified: the audit's own repro (1920x1080, card 300x80 at (660,50), overflowing text) is a named ContentOverflowsBox error again via the CLI. All 69 geometry:: tests and the full `cargo test -p rustmotion` (440 tests) pass. `rustmotion validate` on all 11 examples/*.json is byte-identical before/after this change (built from git-show'd pre-fix geometry.rs vs. the fixed version, everything else held constant) — including the pre-existing, unrelated ferriskey-presentation.json gradient_text overflow. cargo fmt --all and cargo clippy -p rustmotion --all-targets -- -D warnings are clean. Not verified: multi-child containers where siblings jointly (not individually) exceed a fixed-size container's declared axis — container_ bound clamps each child independently against the full container box, not against space already consumed by earlier siblings, so it can under-report in that specific shape. Not exercised by any test or example/ file found.
Every JSON block in the README and the skills was extracted and run through
`rustmotion validate` rather than read. Eighteen README examples and six skill
examples failed. The recurring causes: a root `size: {width,height}` on the
thirteen component types that have no such field (the box comes from
`style.width`/`style.height`), `fill`/`stroke`/`timeline` documented under
`style` when they are root fields, `box-shadow`/`text-shadow` documented as a
single snake_case object when both are kebab-case vectors, and
`grid-template-columns: [{"fr":1}]` where `GridTrack` wants a bare number or a
string.
Around twenty-five documented CLI invocations passed the scenario positionally.
No subcommand accepts a positional file; they all take `-f`. `completions zsh`
was missing its `generate` subcommand.
CLAUDE.md described `--strict-attrs` backwards. Unknown component attributes
have been hard errors by default since the flag existed; the flag is a
deprecated no-op that prints a notice.
This matters more than doc drift usually does: the declared consumer of these
files is a model generating scenarios, so a wrong example is not a typo, it is
an instruction reproduced on every generation.
Also records what the code says and the docs did not: `video.crf` is inert for
`render`/`still`/`batch` and read only by the studio's exporter;
`world-position`, `persist`, `camera_easing` and `camera_pan_duration` validate
clean inside a `slide` view and do nothing there; `animated-background` now
requires an explicit `preset`. The CI workflow's cargo-audit ignore rationales
named dependency chains that no longer exist — dioxus and webbrowser are gone
from the lockfile since the gpui-kit migration. The `--ignore` entries
themselves are untouched, being policy rather than documentation.
The skill now asks which format to write before writing anything, JSON or the
HTML dialect, instead of defaulting to JSON silently. It skips the question when
the request needs something the dialect cannot express, and rules/html-dialect.md
records what that is — every claim in it verified by transpiling and validating,
including the traps: an unknown tag becomes a silent `div`, a style declaration
missing its colon is dropped, numeric text content is rejected.
Not verified: the fragments the mechanical sweep could not wrap, roughly forty
in the README and a hundred in the skills, which use an ellipsis or carry no
root. A targeted sample of those turned up nothing beyond what is fixed here.
Refs #315
Two defects in `--watch`, both bookkeeping. A slot whose duration rounds to zero frames at the scenario's fps never appears in `flat_tasks`, so nothing ever inserts it into `rendered_segments`. The assembly loop used presence in that map as its signal for "just rendered", which is indistinguishable from "clean, take it from the previous run" — except on the first run, where `prev_segments` is `None` too and neither branch fires. The slot vanishes, `new_segments` comes out shorter than the slot list, and every later index is shifted by one relative to what `plan_dirty` believes. The next iteration indexes `prev[i]` past the end and panics. `validate` accepts the scenario that triggers it: three scenes of 1.0s, 1.0s and 0.02s at 10 fps. Patching only the `else` branch to `prev.get(i)` would have turned the panic into a silent drop, the same bug one layer down. The loop now drives off `needs_render[i]`, which `plan_dirty` computes per slot independently of any map: dirty takes the rendered data or an empty vector, clean takes the previous segment. Every slot pushes exactly one `SceneSegment` whatever its frame count, so the length holds by construction. Separately, a scene's frame window depends on its predecessor: `incoming_transition_frames` comes from `actual_outgoing_transition` on the previous scene, which clamps the transition to fit that scene's own budget. `slot_hash` hashed only the scene's own JSON, so shortening the predecessor left the successor's hash untouched and `--watch` served a segment with the wrong window. Measured: two 2.0s scenes with a 1.0s fade at 10 fps give 30 frames; shortening the first to 0.5s gives 20 on a cold render and 15 under `--watch`. The hash now folds in the effective outgoing frame count rather than the predecessor's raw duration. The raw duration would both over- and under-invalidate: two durations long enough for the fade to fit in full clamp to the same count and should not dirty anything, while a change that does move the clamp must. Refs #315
Without ffmpeg, `cmd_render` fell through to `encode::encode_video`, which takes no codec, crf or transparency parameter at all. The flags were parsed, accepted, and dropped on the floor. The bundled encoder is always h264, always 8-bit yuv420p, never alpha, and its muxer writes an MP4 byte stream whatever extension was asked for. `check_native_fallback` now applies the discipline `--hardware-acceleration` already documents for its own unmet request: refuse what would silently produce something other than what was asked for — a codec other than h264, `--transparent` — and warn where the result is cosmetically wrong but still worth producing, a non-mp4 extension or an inert `--crf`. Verified against the release binary with ffmpeg off the PATH: `--codec prores` and `--transparent` exit 1 with a message naming what is missing, `--crf 18` and a `.mov` output warn and still render, and the default invocation says nothing. The red test here was a compile failure rather than a failing assertion — the function did not exist yet, and an absent ffmpeg is not something the unit suite can simulate. The empirical run above is what stands in for it. Refs #315
Four defects on the ffmpeg side, all of them silent. `--transparent` was read nowhere on the default h264 arm: the alpha channel was dropped after every frame had been rendered. On h265 it emitted a `yuva420p` pixel format that libx265 rejects when the encoder opens, again only after the whole render. The refusal now sits as the first statement of `encode_with_ffmpeg_hw_impl`, before any frame is rendered and before ffmpeg is spawned. Putting it in `check_codec` would have covered only the CLI path — the studio and this crate's own tests call the encode entry point directly with a codec and a transparency flag built in code. `concat` drifted the audio by 33 to 45 ms per seam, cumulatively, on the command CLAUDE.md sells as the brick of a distributed render. Encoding N seconds of PCM to AAC needs a whole number of 1024-sample frames, so each segment carries a fraction of a frame of trailing padding. The container's edit list hides the leading priming delay but records nothing about where a stream should have stopped, so no amount of reading the audio side yields the trim point. Decoding and re-encoding through the demuxer was not enough either — measured at 0.5000 / 1.5217 / 2.5434 against an expected 0.5 / 1.5 / 2.5. `aresample=async` was the wrong instrument: it smooths a wrong sample rate between aligned streams, and this is a wrong sample count. The video track is the only ground truth available, because `-c:v copy` never pads. Each segment's audio is now decoded and truncated to exactly `round(video_duration * sample_rate)` samples, taking that duration from a real frame count rather than container metadata, then concatenated and encoded once. The three onsets land within a millisecond. The repository's own test compared stream duration metadata with a 50 ms tolerance, wider than a single seam, so it could not have caught this. It now decodes both renders to PCM and compares burst onsets at 20 ms. `-c:a aac` was hardcoded regardless of container. WebM muxes only Vorbis or Opus, so a vp9 scenario with audio rendered every frame and then died at the mux step — on a codec and container pair `check_codec_container` explicitly accepts. The concat demuxer can print an open or demux error for one input and still exit 0, continuing with what it read. Measured: a valid 15-frame segment joined with 500 random bytes named `.mp4` reported two segments joined, exit 0, and an output holding only the first. Every input's frame count is now counted before the join starts, `-xerror` aborts recent builds on the spot, and the output is reconciled against the known total afterwards for the builds where it does not. Refs #315
The declared consumer of a scenario is a model generating JSON, so the file is untrusted input, not something an operator wrote. Three paths let it read the local filesystem. An icon id went into a cache file name with only `:` replaced. `Path::join` returns its argument unchanged when that argument is absolute, discarding the receiver entirely, so an id carrying an absolute path made the cache directory disappear from the computation and the cache-hit read landed wherever the id pointed. A `/` in a relative id walks out of the directory the same way. Google font families had the identical defect, on a path used for both the existence check and the write. Neither is sanitized into something safe. A file name that would escape is refused by name, quoting the path it would have reached, because silently rewriting a caller's input is how the next hole gets missed. usvg resolves `<image href>` through two branches. `resolve_data` handles a `data:` URI whose payload is already inline and never touches disk. `resolve_string` handles everything else, and its default implementation opens the string as a local file path. `svg.data` is scenario JSON — nothing in it ever named a file on purpose — so an inline SVG could pull an arbitrary local file into a frame someone else watches. `sandboxed_svg_options` disarms only the string branch, leaving inline images working, and now backs every usvg parse in the workspace except the one in `list.rs`, which another lot holds. It also replaces `heropattern_svg_options`, which had grown the same defence separately. Local includes joined their path with no containment at all. The containment root is captured once, from the directory of the scenario passed in first, and threaded unchanged through every recursive level. `IncludeSource` deliberately changes at each level so an included file's own relative paths resolve against its own directory; if the boundary followed it, any included file could reset the root and reopen the door. A `../shared/header.json` that stays inside the root still resolves. Canonicalizing the candidate closes the symlink case too. `MAX_INCLUDE_DEPTH` bounded depth but not width, so a fan-out multiplying at each level stayed under it while expanding to ten thousand scenes. Scene count is now capped as well. Not closed: the window between canonicalizing and reading in `resolve_local_path`. It is the same TOCTOU already accepted for WAV extraction, and it is noted on #320 with the ungated font fetch. Refs #315
`still` rendered whatever it was handed. No schema pass, no geometry pass, no bound on anything — an unknown component attribute went through in silence, and a scenario whose width times height times frame count reached the trillions was accepted and then attempted, because `build_frame_tasks` allocates a task per frame of the whole scenario for a command that uses exactly one. It now runs the same `run_checks` as `render`, and carries the same two escapes under the same names, so the two commands stop disagreeing about whether validation can be skipped. `--no-validate` skips the pass; `--lenient` demotes geometry violations. The frame budget guard stays unconditional either way: it bounds a resource, it is not a validation pass, and `--no-validate` has no business opening it. The `LoadedScenario` built here stubs `raw` to null. `cmd_still` receives a `ResolvedScenario` the loader already built; the pre-deserialization JSON never existed at that point and there is nothing honest to pass. Only two checks read `raw` — unresolved variables and the misplaced-animation notice — and both are non-blocking by design, so stubbing it silences two advisories and nothing that could refuse a render. `cli::validation` is now exported so `rustmotion-studio` can reach the same pipeline instead of growing a second, softer one of its own. The `--fix` help text no longer promises to clamp positions and set `wrap=true`: `apply_fixes` explicitly refuses position clamping, and `wrap` was removed from `CssStyle`. Refs #315
The inspector wrapped every style value in a JSON string without consulting its type. `opacity` is `Option<f32>`, `z-index` is `Option<i32>`, and `font-weight` is an untagged enum wanting either a keyword or a real number, so the opacity slider, the z-index field, the weight menu and the bold toggle each produced a value that fails deserialization — and a component that fails deserialization is dropped from the render entirely. `Target::Style` now carries a `PropKind` like its siblings and routes through the same parser, and the generic CSS panel passes the kind the schema already gave it instead of discarding it. A mutation the in-memory model rejected was still queued for the disk write 250 ms later. The write is now conditional on the optimistic pass succeeding. The preview and the real render did not load a scenario the same way: both went through the loader's inline branch, which never rebases relative asset paths and rejects relative includes, while the path was in scope and discarded. Both now run the same sequence of public functions as the disk loader. The substance of the lot is that the save path runs the CLI's own validation — schema and geometry, the same `run_checks` behind `validate`, `render` and `watch` — and refuses to write when it blocks, naming the violation and its position. An editor that writes invalid files is worse than one with no validation at all, because it lends confidence without the guarantee. A separate component-level type check written for this lot was deleted rather than kept: measuring it showed `check_component_attrs` already folds exactly that error into `schema_errors` unconditionally. It was redundant. Its removal moves the responsibility, and the test names say so — a wrongly typed value is no longer blocked in memory, so it appears in the preview and vanishes from the render for the couple of hundred milliseconds before the write refuses it, as it did before this branch. What changed is that it never reaches disk. Cost on the promo example, measured in debug: 180 to 232 ms. It runs on the background executor after the debounce, never on the UI thread, so it adds to the delay between the typing pause and the write, not to typing itself. The release figure is not measured. Refs #315
…puted size
Seven painters took `_layout` and ignored it, drawing at whatever their
own `size`/`width`/`height` field said. `apply_intrinsic_overrides` sets
those as a default only — an explicit `style.width`, or a size a flex or
grid parent resolved, wins — so the validator measured one box and the
engine painted another. A QR code declared at 300 painted at 300 inside a
60-wide layout box.
Forcing `apply_intrinsic_overrides` to overwrite `css.width`/`height`
unconditionally, which is what the finding suggested, would have broken
the CSS contract every other component honours and already has a test,
and would have fought a flex parent that resizes on purpose: layout
computes a size, the override rewrites it next pass. Scaling in the
painter promises nothing at layout time and leaves `layout.width/height`
the single source of truth.
Rating, switch, success_check and avatar_group draw compositions whose
every dimension is a multiple of one field, so the canvas scales by the
ratio and the drawing follows. The QR code and the timeline read the
layout box directly, the timeline's vertical spacing included, which was
a hardcoded 80. `list` gets a clip instead: it has no scalar natural size,
its width depends on the text of each item, and scaling the canvas would
shrink the font with the box — turning an item that does not fit into a
blurry one rather than a reportable defect. Clipping reproduces what
`overflow: hidden` already does.
`shape: {"path": …}` drew at raw SVG coordinates with `x`, `y`, `w` and
`h` in scope and unused: a path whose data sat around 300,200 painted
there whatever box it was given. It is now mapped onto its declared box
with a non-uniform fit, the same contract the six other shape types
honour — an ellipse fills its box, it does not preserve a ratio, and the
source SVG's aspect has no relationship to the box the author declared. A
degenerate bounding box paints nothing rather than divide by zero.
Measured on the showcase example, which exercises six of the seven, with
two binaries: identical bytes at every sampled second but one, where the
only difference is a channel delta of 12 on the antialiased edge of a 4px
bullet against the new clip. No example uses a path shape at all.
Refs #315
CLAUDE.md opens on the validator being mandatory. Three things let it approve scenarios that overflow. `vw` and `vh` in a `font-size` were resolved against a hardcoded 1920×1080 during intrinsic measurement, so the same declaration measured wrong in both directions: too small on a 4K render, too large on a vertical one. `RichTextIntrinsic::measure` carried a second, independent copy of the same constant. The real viewport now reaches the measurers from `build_scene_from_refs`. The constructors are doubled into `*_for_viewport` variants rather than re-signed. Their existing callers live in a dozen component files another lot holds this wave, and most of them have no viewport to pass: they run at box-tree construction, before layout, only to reserve space, while the painters resolve `vw`/`vh` separately from `PaintCtx` when they paint. Doubling leaves those sites byte-for-byte identical and gives the real viewport to the two callers that hold a `VideoConfig`. `--strict-anim` sampled a fixed eight points per second whatever the scenario's frame rate, while CLAUDE.md sells it as a frame-by-frame check. It now samples at the real rate, with eight per second kept as a floor so a low-rate scenario is never checked less than before, and the existing cap on total samples left alone — each sample rebuilds the box tree and re-runs layout, so an unbounded count would make a long high-rate scene unaffordable. `fold_static_camera` read the camera's static fields and ignored both its rotation and its keyframes. A camera driven entirely by keyframes — the documented way to animate a pan or a zoom — leaves those fields at their defaults, so the fold applied an identity transform and checked nothing, at rest as much as under `--strict-anim`. Rotation folds through the same four-corner bounding box the node transform already uses, because the axis-aligned box of a rotated rectangle is not that rectangle: 100×100 at 45° occupies about 141×141. Measured on all eleven examples with two binaries. Ten produce identical output. `rustmotion-promo.json` goes from valid to two overflows: its first scene holds a camera zooming from 1.06 down to 1.0, so the very first rendered frame pushes a badge to x=-58 and bleeds the particle background out on every side. The renderer was already producing that frame; the validator could not see it. Fixing the asset is separate. One existing test moved with this. `strict_anim_respects_start_at_gate_before_visibility` used `slide_in_left`, and under denser sampling it reports a real overflow at t=1.53s, 30 ms into the replay window `start_at` grants, with the shape at x=-53.34. That overflow is genuine — `83f7ca6` rebased a gated node's clock onto its own `start_at`, so the entrance now actually plays while the node is visible, and sliding in from off-frame is what the preset does. The test is about the gate, not about preset geometry, so it moved to an opacity-only preset. `strict_anim_detects_slide_in_overflow`, which asserts that exact overflow class must be caught, is untouched. The violation label for unwrappable text named `wrap=false`. That field was removed from `CssStyle`; it is `white-space` now, which is also what `--fix` acts on. Refs #315
All three parsed, cascaded and appeared in the exported schema, and no painter read any of them. That is the worst failure mode this codebase has, because the declared consumer reads that schema to generate JSON: a field the schema advertises and the engine ignores is a promise the product does not keep, with no error and no warning to discover it by — only a picture that does not have the effect asked for. `mix-blend-mode` rides the layer already opened for opacity and filters rather than a second one. A blend mode only means anything once everything the node paints — background, border, content, descendants — is composed in an isolated buffer and blended into the backdrop in one go at restore, which is the same contract that layer was already fulfilling. The trigger excludes `normal`, which is Skia's own default, so a node that does not declare the property follows exactly the code path it followed before. `clip-path` opens inside that layer and stays open through the decorations, the content and the children, because CSS clips the whole box: a box shadow or a background reaching past the clip shape would otherwise stay visible outside it. Opening it inside the layer rather than around it also means a node that is both faded and clipped fades as the clipped shape, not as the full rectangle. All five shapes are supported; the path variant is fitted to its box the same way a path shape is. `visibility: hidden` is not `display: none`, so it is not an early return. A descendant may redeclare itself visible and be painted under a hidden ancestor — which is what the cascade already assumed, propagating the property only to children that do not set it. Cutting the subtree at the parent would have broken exactly that child. Each node skips only its own decorations, backdrop filter, shimmer and content; children are walked unconditionally and decide for themselves. `text-decoration` is removed instead. Drawing it correctly means following each real line after wrap — their count, their positions, their widths — and that data lives in the text painters, not in the paint pass, which sees only a bounding box. A single bar at a fixed height would be right for one line and wrong for every wrapped paragraph, which is the common case. In an engine that calibrates a legibility floor and ships a dedicated overflow validator, a decoration that renders plausibly and wrongly is worse than one that is absent. `deny_unknown_fields` now names the field and fails validation. An underline remains reachable through a wrapper with `border.bottom`; there is no equivalent for overline or line-through. The place to implement it properly is `Text::paint_content`, where the line positions exist. That removal breaks any scenario declaring the field. It used to pass in silence, so nothing it produced was ever what was asked for. No example declares any of the four. Refs #315
`FontWeight` derives `JsonSchema` but hand-writes `Deserialize`, and the
derive sees only the Rust enum. It published the externally-tagged form a
tuple variant gets by default, `{"Weight": 700}` — which the hand-written
parser rejects — while `"bold"` and `700`, the only forms it accepts,
appeared nowhere as valid. A schema that documents exactly the rejected
shape and rejects exactly the documented one is not incomplete, it points
every schema-driven consumer at a guaranteed failure, the generating model
included. The impl is now written by hand to match: a string enum or an
integer between 100 and 900.
The same visitor cast to `u16` with no bound. `70000` wrapped to `4464`
and `0.5` truncated to a weight of zero, both accepted without a word, so
a typo became a render quietly different from the one asked for and
`validate` saw nothing. The range was already written in the visitor's own
`expecting`; it was never checked.
`animated-background`'s nested form defaulted `speed` to zero, leaving the
background still, while the legacy flat form defaulted to thirty. The
README and CLAUDE.md document only thirty. The function returning that
default already existed, marked dead code, because the nested branch never
called it.
Every example that uses an animated background sets `speed` explicitly, and
every `font-weight` in the examples is inside the valid range.
Refs #315
The studio's property inspector still routed `text-decoration` to its typography section, and the transition-property classifier still had an arm for it. Neither field exists on `CssStyle` any more: the studio would have offered a control writing a value the parser rejects, and the classifier arm is unreachable because deserialization fails upstream. Refs #315
A badge pinned itself to the cross-axis start whatever its parent said. The default was added so a badge would not stretch full width under a column whose `align-items` defaults to stretch, but it was applied at parse time, where the parent is not visible, so it could not tell stretch by omission from centre by explicit choice. Since `align-self` beats `align-items` per child in real flexbox, an author writing `align-items: center` got every child centred except the badge, which sat flush against the left edge while its siblings centred around it. The default moves to `apply_intrinsic_overrides`, which already holds the parent's resolved style, and now fires only when the badge sets no `align-self` of its own and the parent asks for nothing or for stretch — the case it was written for. Any other explicit `align-items` is left alone and taffy inherits it like it would for any other child. The box was never the problem: instrumenting the painter showed it receives x=0 with a width of 407.5 on a 1920-wide scene, so the measurement matches what is drawn and the box is not stretched. Correct box, correct size, wrong cross-axis alignment. Measured across the seven examples carrying a badge, rendered before and after. Three are byte-identical: their badges sit in a row that sets no `align-items`, so they still get the same start alignment. One gains a badge that now centres vertically with its row instead of hanging from the top. The promo example changes in three scenes rather than the one that was reported — the same defect sat in a stat card and under a terminal, invisible to the validator because neither crossed the viewport edge, and identical to the eye. Refs #315
Two overflows the entry document says must never ship, both real. The promo's opening scene lays a full-viewport particle halo under a camera that opens at 1.06× zoom, so the background bleeds on every side. That is what a background is for, and `bleed` is the declared opt-in for saying so. The validator only started reporting it once the camera fold learned to read keyframes; the frame was always being rendered. The presentation's three figure columns share a 400px width so they line up, and `~10MB` at 122px needs 404. Shrinking a display-register figure to fit an arbitrary column width is the wrong trade, so the columns widen to 420 — the row still measures 1400 inside 1920. All eleven examples now validate clean. Refs #315
cargo fmt is CI's first job, and the hand-edit that dropped the field left its match arm wrapped across two lines where one now fits. Refs #315
…nslate Three silences in the bridge, plus the data a downstream silence needs. `display: inline-block` and `display: contents` fell into a catch-all arm next to the legitimate unset case and collapsed to `block` without a word. The match is exhaustive now and each of the two says so once. The same for `max-content`, `min-content` and `fit-content` on a size: a comment claimed taffy 0.10 supported them through `Dimension`, which it does not — those keywords exist only in grid track sizing, where the code already uses them correctly. The fallback to `auto` is unchanged, the lie and the silence are not. `border-style: none` reserved its width anyway. Neither the paint-space width nor the measurement inset read the style, only the border painter did, and only at the uniform level. Content lost room to a border that is never drawn, and an invisible border stayed reserved as a transparent frame. Both derivations now go through one function resolving the style per side, so they cannot drift apart — which is the defect class this audit keeps finding elsewhere. It also fixes a per-side `none`, which the painter ignored because it only tested the uniform style. `layout_pass` already resolved each node's real cascaded font size and threw it away after building the taffy style, so the paint pass recomputed its own through a helper that resolves every relative unit to zero — hence square corners on a `border-radius` in `em` under a `font-size` in `vw`. `LayoutResult` now carries the resolved sizes. Nothing reads them yet, so this commit changes no output; the paint pass has to take them, and that file belongs to another lot this wave. The double base for `em` between layout and intrinsic measurement is left alone. It is entirely in `components`, where `font_size_ctx` hardcodes 16px because the cascade hands it an unresolved length, and its own doc comment already admits it. Refs #315
Four silences, each producing a picture the author did not ask for. `clock_wipe` never showed its last frame. A 360° sweep is degenerate in Skia — start angle equals end angle — so the path holds only the centre point, the clip is empty, frame B is never drawn, and the transition ends on a flash of the outgoing scene. It now returns frame B directly at full progress rather than clamping the sweep to 359.99°, which would leave a hundredth of a degree of artefact and muddle the invariant. An empty keyframe list parsed fine and dropped the animator into its fallback of zero — opacity or scale at nothing, with no message. Unsorted times were worse in a quieter way: the shortcut for "past the end" reads the last element of the array rather than the one at the latest time. Both are refused at deserialization now, following the idiom already used for keyframe property names. `cubic-bezier` accepted any control point. The Newton solve assumes the curve is monotonic in x, which CSS guarantees only for x within 0 and 1; outside it the root finder can converge somewhere incoherent. The x components are bounded now and y left free, since overshoot on y is what produces back and elastic curves. `easing: "spring"` outside a keyframe segment — on a wiggle, on a motion path, on a camera pan — was a straight linear passthrough. The author asked for a spring and got a line. Fixed at the single `ease` entry point rather than at each call site, because the keyframe path intercepts spring before reaching it and therefore stays untouched, and because it closes the same hole for the world view's camera easing without touching that file. A spring with no explicit duration runs at its natural speed. When that outlasts its segment, the last frame inside the segment carries whatever the physics says — measured at 172% of target for a lightly damped one — and the next frame is pinned to the target, so it jumps. The segment duration is now imposed, but only when the spring's own settle time exceeds it, so a spring that already lands in time keeps the shape its author chose; a test holds that, and the unconditional version distorted it by 11% mid-segment. This changes `dynamic-glass.json`, whose six springs all outlast their segments. Their poses move because they were wrong: the card measured 1.15× mid-flight where the physics had not settled, and the closeness to target at the segment boundary was a sampling coincidence on a zero crossing — two hundredths of a second either side it is twice as far off. Left alone: two animations on the same property compose multiplicatively where the documentation promises the last one wins. Changing that would break two presets that are meant to combine, and it is a product call. Refs #322. Refs #315
A misspelled key was absorbed and ignored. That is the dominant defect
class of this audit, and it lands hardest here: the declared consumer
reads the exported schema to generate JSON, so a key that parses and does
nothing produces a video nobody asked for, silently.
`AnimatedBackground` deserialized by reading the keys it knew, one at a
time, and never looked at the rest of the object. Its seven preset config
structs, plus the halo zone and the background transition, carried no
unknown-field check either. The accepted set is now built from the common
keys plus the ones the chosen preset actually consumes, and anything else
is refused by name. A comment claimed the fallthrough arm of the legacy
form was unreachable; `grid_lines` and `pixel_grid` reached it, having no
flat form at all, and now say so.
`TimelineStep` took `{"at": 0.2, "styel": {...}}` as a valid, wholly inert
step. Thirteen nested config structs across the codeblock and video
schemas were equally permissive.
The did-you-mean for animation and camera properties normalised dashes,
underscores and spaces but never split a camelCase boundary — so
`translateX` and `originX`, the most likely spelling a generating model
produces, got a dump of every valid name instead of the one they meant.
Both validators now fold that boundary through one shared function.
`gradient_type` is refused on `concentric_circles`, which is where three
examples and the skill's own example declared it. It belongs to
`gradient_shift` and only that painter reads it; the concentric-circles
config has no such field and its painter never looks for one. The examples
drop the dead key and the skill's reference table now scopes it, since a
table row listing it beside genuinely shared keys is what put it in those
files.
Not done here, and deliberately: `world-position`, `persist`,
`camera_easing` and `camera_pan_duration` are inert inside a slide view
but documented as scene fields and used by a shipped example. Refusing
them at deserialization breaks documented usage to fix a silence; a
warning that names the inertness fixes the silence alone, and that belongs
in the CLI validator.
Refs #315
…d pool Text measurement took Skia's global strike-cache lock once per text node, per taffy pass, per frame, while the text, the face, the size and the tracking are identical from one frame to the next for almost any scenario. System font resolution had the same shape: custom faces and emoji were already cached per thread, everything else went back through the platform font manager every call, negative results included. Both are cached per thread now, keyed on what actually varies. That is the fix the pathology called for: the obvious move was to lower the default thread count, which trades throughput for less contention and repairs nothing. Measured on a text-dense synthetic scenario, sixty cards over nine hundred frames, median of three: 5.27s to 2.84s at one thread, 3.31s to 2.55s at four, 4.61s to 2.71s at twelve. The symptom the audit found — twelve threads costing 39% more than four — drops to 6%. The default parallelism is untouched. Word wrapping rebuilt the line and remeasured its whole prefix for every word, so a line of n words measured Θ(n²) characters. It accumulates width per word now. The segmented path for emoji and CJK is unchanged and still taken whenever the line needs it, decided once per line: if the whole line needs no segmentation then neither does any prefix of it, so the fast path is provably the same answer rather than a plausible one. Two tests compare it against a brute-force remeasurement. The repository's own examples gain nothing measurable — under 3%, inside the noise. They carry twenty to thirty text nodes per scene, not enough concurrent measurement for the lock to dominate, and their cost sits elsewhere. Rendered output is byte-identical across ten sampled frames on two examples. Left for the lot that owns `engine/render/scene.rs`: layout is rebuilt every frame although only animated width and height reach taffy at all. Refs #315
The PNG sequence encoder rendered its batch in parallel, collected the whole thing, then wrote every file one at a time on the calling thread. Moving the save inside the parallel iterator takes a 1080p, 831-frame render from 6.57s to 0.87s of wall time — 7.55× — on the same user time, which is what a serialised tail looks like once it is gone. The 831 output files are byte-identical and the ordering and determinism tests are unchanged. The GIF encoder cloned every RGBA frame to satisfy a mutable borrow on a buffer it already owned. `--format raw` had no parallelism at all, and wrote through the process stdout lock, whose buffer is always a `LineWriter` — a newline scan over every multi-megabyte binary frame, for nothing. It renders in parallel batches now, writing serially and in order, through a duplicated descriptor wrapped in a plain buffered writer. The raw gain is modest and honestly hard to pin down: the format is uncompressed and bound by disk bandwidth, not CPU, and the bench machine was shared with concurrent builds — runs against a real file varied from 3.5s to 7.9s. Output size is identical to the byte. The newline scan going away is the part that is certain. Refs #315
`hash_scene` hashed the scene's JSON, so it saw an asset's *path* and never its contents. Editing an image or a font in place under `--watch` left the slot clean and the old frames on screen. The hash now folds in size and mtime for every `src` it can reach, plus the scenario's fonts, reusing the recipe the audio cache already uses. Not covered: a typed background image, and scenario audio, which has its own mtime-aware cache. A transition was clamped to the outgoing scene's frame budget but not the incoming one. When the entering scene was shorter than the declared transition, its normal range came out empty and it emitted no frames at all — and `frame_in_transition` doubles as that scene's own frame index, so the composite read past content it never declared. A transition of exactly one frame showed only one side of the cut. The single frame now lands at half progress rather than at either endpoint: whichever endpoint it took, the frame on the other side — the outgoing scene's last or the entering scene's first — appears nowhere in the output, because the task builder skips both as normal frames. Half shows both. Dropping declared content without saying so is the thing this chantier exists to remove. `flip` cleared to black regardless of the scenario's own background, which reads as a full-screen flash on any light video. The core gained a clear colour with a black default earlier on this branch; the four sites that build transition options now fill it from `video.background`. Measured on the showcase example, whose background is `#0b0f1a`: frame differencing against the previous build is exactly zero across 1236 frames outside a window centred on the flip, and the ramp on either side of that window matches libx264's lookahead rather than a second change. A clean slot's H.264 buffer was deep-copied on every `--watch` iteration on top of the concat buffer. It is shared by refcount now. Refs #315
`concat` documented that its inputs must share codec, resolution and pixel format, and checked none of it. The demuxer copies packets through, so the result was a file whose header contradicted its own frames. Each segment is probed now and the first mismatch is named. `--frames` skipped the codec-container check entirely, so `--codec prores` into an `.mp4` rendered the whole range before ffmpeg failed on its own terms. The check moved into the shared encode entry point, deriving the container from the output extension — which is what ffmpeg uses to pick a muxer anyway — so `render`, `--watch` and `--frames` are covered at once. That is the same placement the `--transparent` refusal took earlier on this branch, and for the same reason: the CLI is not the only caller. A scratch directory for audio leaked whenever decoding failed, because the cleanup sat past the failure point. It is a drop guard now, so every exit path takes it. `--quiet` did not silence the audio loading line. The bundled encoder forced a keyframe on every frame of a full render. That is only needed when segments are spliced as a raw bitstream, which is the incremental path alone; a plain render paid for an all-intra stream it never asked for. ProRes always asked for the 4444 profile even when opaque, against a pixel format that wants 422 HQ, so ffmpeg quietly promoted the format to reconcile them. `--watch` always uses the bundled 8-bit encoder, whatever ffmpeg is on the PATH, because incremental encoding only exists as a raw bitstream splice. That stays — routing it through ffmpeg would cost the roughly 62% the incremental path saves — but it now says so once at startup instead of letting the preview silently differ from the file that ships. Not done, and out of this lot's reach: one ffmpeg process per video frame, whose call site is in the video component; audio decoded twice and held whole in memory; and overlapping render with encode, which is an architecture change rather than a repair. Refs #315
A scenario variable named like a `for-each` field silently won. The variable substitution walked the whole document skipping only the key `config`, so a `config` entry called `label` rewrote every `$label` inside a `template` body before the directive expander ever saw it — and `CLAUDE.md` presents that mechanism as the answer to the most common generation failure. The same flat substitution made a nested `for-each` lose to its outer one, because the outer expansion rewrote the whole cloned subtree including the inner template that had not been resolved yet. Both come from there being no scope at all. Bindings now chain: each `for-each` extends its parent's scope with its own, the inner one winning on a shared name, and each `use` starts from nothing but its resolved params. A component therefore sees only what it was handed, which it did not before — an outer binding used to reach inside a component body by accident, through the same over-substitution. Narrowing the skip list alone would have closed the first half and left the second, and would have broken an outer `for-each` legitimately reaching the props of a `use` nested inside an inner one. A test holds that case. A component parameter's declared type was never checked. The field carried a dead-code marker while `merge_variables` called itself the sole enforcement point for scenario config. A wrongly typed prop travelled to the scenario's own deserialization failure, which names neither the component, nor the parameter, nor where it was used. Both the prop and the declared default are checked now, against the same predicate config already used. Refs #315
…d index An included file ran variable substitution, then rebased relative paths, then expanded directives — the reverse of what the root loader does. A `src` bound by a `for-each` is still `$path` when the rebase runs, and the rebase is deliberately conservative about names that match no file, so it left the placeholder alone; the expander then substituted the real relative path and nothing rebased it again. It ended up relative to the process's working directory instead of the included file's own. Reordering to match the loader fixes it. Making the rebase accept a literal `$name` was the alternative, and it would have broken the contract that a path matching no file is left alone rather than rewritten on a guess. A repeated scene index in an include silently produced fewer scenes than asked. The filter built a vector of options and took each requested index out of it, so the second mention of the same index found it already taken and vanished — after the bounds check had passed for all of them. Each index now yields its own clone. Reusing an intro card is the case the finding names, not an author error to refuse. Refs #315
…ck honest Four presets wrapped their scroll on the wrong period. Ruled grids used the cell size where their major lines repeat every `major_every` cells, so each wrap shifted the major/minor phase and the heavy lines jumped. The gradient and the halo were translated by a scroll offset before the dispatch even though one handles its own rotation and the other places its zones as fractions of the surface — so a full-bleed rectangle or a halo zone drifted and snapped back once per cycle. The pixel grid had no real period at all: it hashed the loop's local index, so every wrap remapped a visible cell to an unrelated value. It hashes the absolute cell now, which is what makes the scroll infinite rather than merely longer. `apply_camera_transform` saved the canvas without saying so. Two callers wrapped it in a guard as well, saving twice against one restore — harmless only because nothing changed between the two saves — while two others depended on that hidden save and restored by hand through a flag. The save moved out to the callers, so the stack balances structurally instead of accidentally. Pixels are unchanged. A world view's animation clock started after the incoming pan but its visibility window closed on the outgoing one, and the two half-pans are clamped independently against their neighbours. A long scene followed by a short one therefore closed before its own clock reached the declared duration — measured at 9.0s of a declared 10.0 with pans of 3.0 and 1.0. The window now never closes before the scene's clock is done, and the timeline's total extends to match: computing only the window left the render loop stopping before the widened window could be reached. An asymmetric boundary now makes the video slightly longer than the sum of the declared durations; the effect on audio track placement is not measured. The decorative full-screen path used by world views resolved opacity only to decide whether to skip, never to blend, and never read the static style at all — the particle painter ignores its props because in the normal pipeline the caller opens the alpha layer, and this path bypasses that caller. Video frame prefetching capped its read at the number of frames it would render, while ffmpeg emits enough to cover the same window in source time — so at double playback rate the second half of the clip was never read and the picture froze on the last cached frame. `prefetch_icons` panics on an unresolvable icon, which is the intended contract for the CLI. The studio calls it from a detached thread that cannot catch it. The body is now a `try_` variant returning an error, with the panicking wrapper left exactly as it was for every current caller. Left measured and undone: caching layout across frames. Instrumented on a deliberately dense scenario, taffy is 8.3% of the frame and building the box tree is 28.8%, with painting taking the rest. A fingerprint that missed an animated dimension would produce a stale layout — a wrong picture, silently, which is the defect class this whole audit is about — for a ceiling under 8% before the cache's own cost. The cost is in the box tree and the paint, not the layout. Refs #315
…ne knows The paint pass recomputed each node's font size through a helper that resolves every relative unit to zero, while the layout pass had already resolved the real cascaded value and now carries it. That is what made a `border-radius` in `em` under a `font-size` in `vw` paint square corners. The studio warmed its icon cache through the panicking entry point, from a detached thread with nothing to catch it. It takes the fallible variant now, reports the unresolvable icon and stops warming. CLAUDE.md said a `config` key on a `use` would be skipped by substitution. It is refused at load as an unknown field, which is a better behaviour than the one documented and worth saying plainly. Refs #315
cargo fmt is CI's first job. Refs #315
…y available The publish workflow held the registry token in the same job that compiles about a thousand third-party build scripts and proc macros, with no permissions block, no environment, and no check that the pushed tag points anywhere in particular. It now declares read-only permissions, runs under a `release` environment, and refuses a tag whose commit is not an ancestor of main or whose check runs did not all pass. Both were tested against this repository before being written: main is its own ancestor, this branch's head is not, and the four required checks report success on main's head. The environment only restricts what an administrator configures behind it — required reviewers, a branch or tag rule, and ideally moving the token from a repository secret to one scoped to that environment. None of that is in a file, so none of it is in this commit. `cargo audit` was the one step in a workflow of thirteen SHA-pinned actions installed without a version. `--locked` pins its dependencies, not itself. Six of the ten advisories the CI suppressed had a semver-compatible fix available the whole time, reachable by a lockfile update and no manifest change: rustls, rustls-webpki — which alone accounted for four of them — and crossbeam-epoch, plus the yanked unicode-segmentation and the unsound memmap2, both of which the published crates reach through cosmic-text and usvg. Real vulnerabilities go from eleven to five, warnings from fifteen to thirteen, and the ignore list from ten entries to four. quick-xml stays suppressed because no compatible version exists: both of its copies come through third parties that pin narrower ranges. openh264 stays too, and its justification now carries the evidence rather than an assertion — the encoder is the only thing imported, the sys crate builds from source rather than loading a library, and the advisory itself says that configuration is safe from the version in the lockfile. The count in that comment said seventeen; it was fifteen. Refs #315
…could not
An unrecognized tag became a `div` without a word — a typo produced a
scenario that transpiled, validated and rendered almost right. It is
refused by name now, with the closest known tag suggested. The list of
recognized containers is unchanged: adding `nav` or `ul` would be a
feature, not a repair.
Whole classes of component field were unreachable. Any attribute on an
`rm-*` element now takes a JSON value when it starts with a brace or a
bracket, which is the escape hatch `background`, `effects` and `anim`
already used, generalised — so charts, tables, lists, steppers and the
per-component `timeline` become expressible. Inline styles take the same
hatch, which reaches box-shadow, transform, filter, border as an object,
clip-path and the gradients. A malformed JSON value names the element and
the attribute rather than becoming a literal string.
`<rustmotion audio='[…]'>` reaches the scenario's audio tracks, which had
no HTML spelling at all, so the two audio-reactive components had nothing
to bind to.
Source indentation became literal content, since text was only trimmed at
the outer edges. It collapses like CSS `normal` now, with
`white-space: pre` opting out. Two inline elements separated only by
whitespace still render flush, and that stays: real flexbox generates no
box for a whitespace-only run between two children either.
`<b>`, `<i>`, `<code>`, `<u>`, `<small>` and `<a>` were refused with a
message claiming their content would be lost. They flatten into the
parent run exactly as `<strong>` and `<em>` already did. `<br>` becomes a
line break inside text and is refused elsewhere, where it has no meaning.
Several silences closed: a style declaration missing its colon, a font
weight that fails to parse, root-level content outside a scene, and
`<rustmotion background='{…}'>`, which transpiled into an object the video
config can never hold — two tests encoded that dead path and now assert
the refusal.
Recursion had no depth guard, so deeply nested markup aborted the process
rather than failing. The studio's write-back matched `<rustmotion`
anywhere in the file, including inside a comment quoting an older draft.
Two findings did not survive contact. `world-position` is not inert: paired
with `transition: camera_pan` it drives the pan between two scenes inside
the flat scene list the dialect can already emit, proven by two renders
that differ. And the numeric coercion of a string field stays as it is —
forcing a string would need a quoting convention the dialect does not have,
and the obvious one breaks any value that legitimately opens with a quote.
That is a grammar decision, not a repair.
Refs #315
Six statements the HTML lot made false, plus one it made wrong all along. An unrecognized tag no longer becomes a `div` in silence, whitespace now collapses like CSS `normal`, the inline text tags flatten instead of being refused, nesting is bounded, audio tracks have a spelling, and the JSON escape hatch reaches every component field and every structured style property rather than five named attributes. The claim that `world-position` is accepted and never read was wrong before this branch touched anything. Paired with `transition: camera_pan` the renderer pans between two scenes' values inside the flat scene list the dialect already emits — verified by two otherwise-identical renders whose pixels differ. That matters here more than in most docs: this file is what the generator reads to decide whether it may offer HTML at all, so a capability listed as missing is one nobody will ask for. Refs #315
A chart that gets it wrong does not crash and does not look wrong. It produces a believable picture and a false number, shown to someone with no way to check it. Seven of those. Line and area charts scaled between the series' own minimum and maximum while the reference promises them zero anchoring, like bars and waterfalls. A pair of values that sit close together but far from zero — 100 and 105 — painted the smaller one on the axis floor, reading as near-nothing. The domain now stretches to include zero, and the area fills down to it instead of to the bottom of its box. A flat series still centres, which the degenerate-input behaviour depends on. Scatter keeps min-max, which the reference excludes by name. A radial bar normalised each value against the largest in its own series, so the biggest item was always a closed ring and a single datum was always 100%, whatever its value. It takes an explicit maximum now, the way the gauge already does. A negative treemap value shrank the total everyone else divided by, so the other cells overflowed the box while the negative one, having a negative width, never drew at all. A counter with no duration ramped over the scene's full length, one frame past the last frame that gets rendered, so it never quite landed — 989 of 1000 on the final frame. A number wheel ignored its own `start_at` and measured its travel from scene zero, arriving already parked. A gauge rounded its readout to a whole number with no way to ask for decimals, so 4.7 out of 5 displayed as 5 beside an arc drawn at 94%. Axis labels flipped between two formats on floating-point noise, printing `-2` next to `-1.9999999999999996` as `-2` and `-2.0`. And any tick scale finer than a hundredth collapsed to `0` everywhere. Tick precision now comes from the gap between ticks rather than from each value alone. Left as a hand-off: a radar series whose length disagrees with the axis count is dropped, and a stacked bar's is padded with zeros — two different wrong answers, both silent, and `validate` says the scenario is fine. The honest place to catch that is the validator, which cannot be reached from a painter. Refs #315
Each ring is now value over an explicit `max`, defaulting to 100, rather than a share of the largest item in the series. Without it a single datum was always a closed ring whatever its value. Refs #315
A scenario's `video.src` and `audio.src` reached ffmpeg unchanged on three of the four sites that shell out. A scenario is untrusted input — the declared consumer generates it — so a value pointing at a link-local metadata endpoint made the rendering machine issue the request. The previous lot confirmed that against a local listener; the regression tests here own a listener of their own and saw the connection arrive before the fix. Only `extract_video_frame` checked. Frame prefetching did not, and it is what the studio calls when a scenario is opened — no flag, no validation upstream, just opening a file. Audio extraction did not either. Metadata probing did not, and additionally passed the source as a trailing positional with no `--`, so a `src` beginning with a dash became an ffprobe option: `-version` printed the banner and exited zero, past the error branch entirely. The guard moved to the functions that spawn the process rather than to their callers. Filtering in the CLI would have covered neither the studio, which calls prefetching directly, nor the duration probe the video painter runs while painting. Both remaining ffmpeg invocations also restrict the protocol whitelist to local files. No example uses a video component, and a scenario pointing at a real local file still probes, validates and renders a frame. Refs #315
…inting The box tree applied intrinsic sizing before the cascade ran, so five components measured themselves against their own default font size while the painter drew with the inherited one — a tooltip sized for 13px and drawn at 48. The cascade now runs first, and those arms read the cascaded style rather than the raw component style. This was already a root cause in the September audit. An opacity or filter layer bounded itself to the union of its descendants' untransformed layout boxes, so any child moved by a transform — which is every animation preset — was painted outside the layer and clipped away. The layer goes unbounded when the subtree carries a transform, which is the alternative the finding names: computing the transformed bounds exactly means per-node matrices and pivots, and the optimisation is only lost where a transform actually exists. A node whose own box measured zero returned before recursing, so an auto-height wrapper holding only absolutely-positioned children — a box CSS legitimately collapses — took its children down with it. Only the node's own decorations and content are skipped now. `display: none` stays hidden because taffy already zeroes the whole subtree, so each descendant independently collapses. Eight components repaint their own background from the same style the paint pass already painted. On an opaque colour the generic square rect showed at the corners of the component's own rounded pill. The dispatcher now declares which components paint their own, and the generic step steps aside. Removing the internal painting instead would have taken the callout's and tooltip's arrow with it — they draw a bubble, not a rect. `border-radius` in percent resolved against the smaller side and produced one scalar per corner, so `50%` on a 400×120 box drew a stadium instead of the full-box ellipse CSS specifies. It resolves per axis now, and the padding box shrinks per axis rather than by the larger of two borders. Trail and motion-blur ghosts carried no paint window, so they stayed visible outside the `start_at`/`end_at` range of the node they trail, and carried no intrinsic measure, so a content-measured component's ghosts were sized zero. Container ghosts still paint as empty rectangles: giving them children means remapping the subtree's clock to the ghost's own time, which is a larger change than this repair. Two docs were lying. `paint_content` and `dispatch` both claimed the canvas arrives translated to the content box; the dispatcher does that a layer further down, with a special case for code blocks. And `stagger_offset` on the paint context is folded into the resolved properties already — reading it again double-counts. Left open: `stagger` is declared on fifty-seven component structs and read for four container kinds only, and `positioned` does not have the field at all. Closing it means touching every component file. Refs #315
The validator's own silences first. `check_legibility` resolved font sizes without a length context, so every relative unit came back as zero — and printed a developer-facing message while doing it. Animated validation passed the scene-relative clock as the scenario clock, so every audio-reactive component after the first scene was checked against the wrong stretch of its track; it now accumulates scene durations, resetting only on entering a world view. Transition durations are not subtracted, which bounds the residual error by the transitions crossed rather than by the whole elapsed time. `--fix` refused to run on any file containing a dollar sign — a price, a shell path — because it treated the character as proof of templating. Without a `config` block no `$` can ever be substituted, so the refusal was noise; it now looks for the block itself, and for real JSON keys rather than a substring match that fired on content equal to `"use"`. A report requested alongside `--fix` was written before the fix ran. `validate` exited the process directly instead of returning, which also made it untestable — a test asserting the blocking path killed the test binary. It hand-rolled the blocking decision too, duplicating the report's own method, which happens to agree today only because unknown attributes are folded into schema errors upstream. Four things that validate clean and do nothing now say so: `world-position` and `persist` outside a world view, `camera_easing` and `camera_pan_duration` likewise, `display: inline-block` and `contents`, and the intrinsic sizing keywords. Warnings, not errors — they are documented fields and a shipped example uses one. A radar series whose length disagrees with the axis count, and a stacked bar's with its categories, are errors: one is dropped and the other padded with zeros, two different wrong answers that both rendered quietly. `captions` wrote to a predictable path under the system temp directory and read its result without checking for a symlink. Its whisper lookup joined empty `PATH` components, which resolve against the working directory, checked only that the candidate was a file rather than executable, and ran it with no time limit. In the studio, the file watcher watched the inode rather than the directory, so the first atomic save — write to a temporary file, rename over the target, which is what every editor and every agent does — killed live reload for the rest of the session, silently. The self-write hash was only cleared on an undo failure, so a file that went away from and back to something the studio once wrote was mistaken for its own echo forever after. The annotations panel never recorded its writes at all on the JSON path, unlike the HTML one beside it. The inspector rebuilt its controls only when the selection pointer changed, so an undo or a hot reload on the same node left a stale content field that overwrote the new value on the next keystroke. Library thumbnails were rendered synchronously inside the render pass — half a second cold, up to twenty per uncached icon — and were evicted on every keystroke in the search box, because the keep-set was the filtered list rather than the library. Rendering moved to the background executor and eviction now reads the whole library. An unreadable scenario left the editor with no way back but quitting. The Dioxus-era dead code goes: a notes file, four unreachable entry points, and a thumbnail cache, an index and a flat index that were computed and never read. Refs #315
…the bleeding The typewriter reveal in code blocks and terminals counted its budget in UTF-8 bytes and spent it in characters, so accented or CJK content raced ahead of the clock and froze early. `gif`'s `fit` was parsed and never read — every GIF stretched to fill whatever box it was given. A QR code drew edge to edge with no quiet zone, so the finder pattern's dark corner touched the background, which is what scanners use to find the code. Audio spectrum bars computed their stride from an unclamped width and had no ceiling on bar height, so a dense enough configuration ran past the right edge and above the top. A shape's stroke is centre-aligned, because Skia offers nothing else, so half its width always bled outside the declared box. Fill and stroke now sit on a rect inset by half the stroke width, which is CSS `border-box`: they stay concentric, and the composite stays inside. A schema-level alignment field would be the real answer and belongs to a different lot. `callout` was the last component painting text through Skia directly — no emoji fallback, no glyph-coverage fallback, and centred on ink bounds rather than advance width. It goes through the shared primitive now. The first two attempts to test this passed against the broken code, because the installed fallback draws a `.notdef` box dense enough to fool a pixel count; the test that stands compares against a hand-assembled reference. `lottie` swallowed an unreadable source entirely — no warning, nothing drawn. It read and parsed the same file three times per frame and hashed on every cache hit; one resolution is now shared. Its frame cache key ignored `frames_dir` and inline data, so two components with no `src` collided. And its buffer allocation multiplied two `u32` dimensions as `u32`, which wraps to an empty buffer while telling the renderer the surface is full size. A missing font family fell through to whatever the system offered without a word, so the same scenario wrapped differently on different machines. That is loud now, once per family. It does not make rendering machine-independent — no font ships with the repository, and bundling one is a packaging decision — but the divergence is visible instead of silent. Expect the warning on every example here: neither Inter nor SF Mono is installed on this machine. The custom font registry is thread-local and silently ignored a second registration of the same family and weight, so a hot-swapped font under `--watch` was never picked up — and a purge from the main thread cannot reach the rayon workers that render. Registration now replaces the bytes and bumps a process-global epoch; each thread compares that epoch on its next lookup and clears its own cache. One atomic load on the hot path, no lock, and no caller has to be told to purge anything. Closes #323. Left paired: the terminal hardcodes its line height ratio while the code block honours `style.line-height`. Fixing only the painter would make it disagree with the measurement the validator already approved. Refs #315
…e rendering `--fix` rewrote the whole scenario through a serializer with no order preservation, so every key in the file came back alphabetised — a diff touching one property became a diff touching everything. The studio's manifest already asked for order preservation; the CLI's did not, so the behaviour depended on whether the studio happened to be in the build. `--transparent` on a codec that cannot carry alpha was refused inside the encoder, which is the right place for it — every caller goes through there — but `render` checks the codec against the container before preloading and did not check this one, so the refusal arrived after the whole preload. The module doc still described the studio as a Dioxus app. Refs #315
LeadcodeDev
force-pushed
the
chantier/audit-2026-09-25
branch
from
September 25, 2026 16:25
5f55e26 to
347c609
Compare
5 of 13 tasks
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.
Remediation of the 25 September audit: 217 findings across five crates, cross-referenced against 795 Remotion features. This branch closes most of them. The full record is on #315.
This is too large to review in one sitting, and that is worth saying plainly. It opened as six commits covering a spike and grew to forty-three across fourteen workstreams. Reviewing it as a unit is not realistic; reviewing it commit by commit is, since each one carries its own why, the alternative it rejected, and what it could not verify. A reader with limited time should read the commit subjects, pick the three that touch code they care about, and read those bodies.
What the audit actually found
The dominant defect class is silence, not crashes. A field that parses and does nothing. A flag accepted and dropped. A barrier that approves what it should refuse. Almost nothing here is a segfault; nearly everything is a picture that came out wrong with no way for the author to find out.
That shape drove the remediation's single rule: either the thing works, or its presence is refused by a message that names it and locates it. Silence is the bug.
What changed
The geometry validator — the entry document calls it mandatory — was bypassable five ways.
vwandvhmeasured against a hardcoded 1920×1080 during intrinsic measurement, so the same declaration was wrong in both directions on any other resolution.--strict-animsampled eight points per second whatever the frame rate, while the documentation sells it as frame-by-frame. A keyframed camera was never interpolated, so the documented way to animate a pan checked nothing at all. Seven painters ignored the box layout gave them. A centre-aligned stroke always bled half its width outside the box it was measured in.Encoding did not produce what the flags asked for.
--transparentwas dropped on the default path after every frame had been rendered. Codec and CRF vanished without ffmpeg.concatdrifted the audio by 33 to 45 ms per seam, cumulatively, on the command the entry document sells as the brick of a distributed render — and the cause was not the edit list everyone assumed but the AAC encoder's trailing padding, which nothing in the audio stream records. The video track, never padded, is the only ground truth available for the sample count.Three file reads reachable from a scenario.
Path::joinreturns its argument unchanged when that argument is absolute, so an icon id carrying a path made the cache directory vanish from the computation. usvg's default href resolver opens a plain<image href>as a local file, so inline SVG data could pull an arbitrary file into a frame. And three of the four sites that hand a scenario'ssrcto ffmpeg had no guard at all — including the one the studio reaches by opening a file, with no flag and no validation upstream.Seventeen inert fields.
mix-blend-mode,clip-pathandvisibilityare implemented.text-decorationis removed, because drawing it correctly needs per-line metrics that only the text painters have, and a bar at a fixed height is right for one line and wrong for every wrapped paragraph.The scenario expansion had no scope. A
configvariable named like afor-eachfield silently won, and a nested iteration lost to its parent. Bindings chain now, and a component sees only what it was handed.The paint pass clipped transformed descendants out of their own opacity layer, dropped whole subtrees when a container's own box measured zero, and painted eight components' backgrounds twice.
The charts drew plausible wrong answers — a radial bar whose largest item was always a closed ring, an area chart painting its smallest value on the floor as if it were near zero, a treemap whose negative value pushed the others out of the box.
The HTML dialect turned an unrecognized tag into a
divin silence, so a typo produced a scenario that transpiled, validated and rendered almost right.Supply chain: six of the ten advisories the CI suppressed had a semver-compatible fix available the whole time, reachable by a lockfile update and no manifest change. Real vulnerabilities go from eleven to five. The publish workflow now refuses a tag whose commit is not an ancestor of main or whose checks did not pass.
Documentation: every JSON block in the README and the skills was executed rather than read. Eighteen README examples failed. Around twenty-five documented CLI invocations passed the file positionally, which no subcommand accepts.
What the diff does not show
Two findings did not survive contact.
world-positionis not inert in the HTML dialect: paired withtransition: camera_panit drives the pan between two scenes, proven by two renders whose pixels differ — the finding generalised from a single test. And a spring's closeness to its target at the end of its segment was a sampling coincidence on a zero crossing; instrumenting the settle time showed it had not settled at all.Three fixes were measured and then not written. Caching layout across frames: taffy is 8.3% of a frame on a deliberately dense scenario, the box tree is 28.8%, and a fingerprint that missed an animated dimension would produce a stale layout — a wrong picture, silently. The text measurement cache gains nothing measurable on this repository's own examples, and the commit says so; it is kept for the complexity fix beside it, which is proven, and for the twelve-thread pathology it removes.
Rendering changes visibly in five examples, and each is a defect being corrected rather than a regression: an area chart's axis now starts at zero, badges lose a square corner behind their rounded pill, a badge that never centred now does, and the promo's particle background declares the bleed it always had.
A missing font family is now reported. Expect it on every example on a machine without Inter — the same scenario used to wrap differently on different machines without a word.
text-decorationis a compatibility break. It parsed and did nothing; it now fails validation. Nothing it produced was ever what was asked for.Explicitly out of scope
The
TESTlens — eleven findings on the suite itself — is untouched, deliberately: writing tests against an engine under repair freezes the intermediate state. Around fifteen findings were handed between workstreams and landed outside every write partition;stagger, declared on fifty-seven component structs and read for four, is the largest. Five encoding performance findings are architecture changes rather than repairs.Five findings became issues instead: #318, #320, #321, #322, #324. Two of them need a product call, one needs the copyright holder's name.
This audit never had an adversarial panel. The September one did, with 50% refutation. Two findings were refuted here by measurement, both by accident, while doing something else. How many of the rest stand only because nobody tried to break them is unknown.
Verification
Full suite on a detached worktree at rest,
cargo fmt --all --checkclean, clippy clean across the workspace, and all elevenexamples/*.jsonvalidate — which they did not before this branch.Refs #315
Closes #323