Skip to content

chantier: audit remediation — 98 constats vérifiés, 14 lots - #163

Merged
LeadcodeDev merged 15 commits into
mainfrom
chantier/audit-remediation
Aug 10, 2026
Merged

chantier: audit remediation — 98 constats vérifiés, 14 lots#163
LeadcodeDev merged 15 commits into
mainfrom
chantier/audit-remediation

Conversation

@LeadcodeDev

Copy link
Copy Markdown
Owner

Clôture du chantier ouvert par l'issue #142.

98 constats d'audit confirmés, corrigés en 14 lots. Chaque lot a été vérifié en isolation sur sa propre branche avant merge dans le chantier, puis CI verte sur les trois jobs.

main est entièrement contenu dans cette branche : aucune divergence à réconcilier.

Provenance des constats

Audit complet du dépôt, 180 constats bruts. Chacun est passé par une vérification adversariale à trois lentilles (correction / reproduction / contexte dépôt), chaque vérificateur ayant pour consigne par défaut de réfuter. Règle de verdict : réfuté si la majorité réfute, non vérifié si aucune lentille ne survit, confirmé sinon.

Résultat : 98 confirmés, 4 réfutés, 78 non vérifiés (verdicts perdus au rate-limit, jamais traités — voir « Hors périmètre » plus bas).

Répartition par domaine

Domaine Constats
Encodage vidéo/audio et sortie 11
Validation géométrique 11
Texte & polices 10
Schéma JSON & serde 10
Composants / painters 9
Animation 8
Transitions & vue world 8
Layout (CSS→taffy) 8
Dataviz 8
Front-end HTML/CSS 8
Passe de peinture 7

Le motif dominant

La majorité de ces défauts ne font pas planter le moteur. Ils produisent une vidéo fausse sans un mot :

  • un champ que l'auteur pose, que le schéma accepte, que validate déclare bon, et que le moteur ignore (le lot animation en comptait six sur huit) ;
  • une mesure et une peinture qui divergent, ce qui rend le validateur géométrique aveugle au débordement qu'il est censé attraper ;
  • un validateur qui reforke un morceau du moteur au lieu de l'appeler, puis dérive.

Pour un outil dont les scénarios sont majoritairement générés, c'est le pire mode de défaillance : la boucle de correction ne se referme jamais.

Quelques mesures avant / après

Sujet Avant Après
Couche save_layer non bornée (60 frames) 42–60 s 0,5 s
Saut caméra inter-frame en vue world 245,33 px 21,33 px
Saut d'avg-luma du fond sortant 109,0 ≤ 4,82
50vw sur une vidéo 1080×1920 960 px 540 px
amplitude: 60 sur float_3d -12 px (le défaut) 60 px
Exemples valides contre le schéma exporté 2 / 8 5 / 8 (reste : #162)

Décisions structurantes prises pendant le chantier

Trois choix qui ne sont pas de simples corrections mécaniques, à contester ici si besoin :

  1. ContentOverflowsCard est retiré, pas rapiécé. Son garde ne le rendait atteignable que sous overflow: visible — exactement le cas que CLAUDE.md et geometry-safety.md déclarent légal. Il n'existait aucune configuration où déclencher soit à la fois atteignable et cohérent avec le contrat documenté. Le cas dangereux — du contenu qui sort du device — reste couvert par check_viewport, et un test épingle cette garantie.

  2. Composition de deux animations keyframes sur la même propriété : le dernier déclaré gagne, la règle de cascade CSS, indépendamment du delay. Avant, le résultat était une somme ou un écrasement selon qu'un delay était posé — un champ sans rapport avec la composition.

  3. deny_unknown_fields étendu aux structs d'animation. Conséquence assumée : deserialize_children saute un enfant qu'il ne sait pas parser, donc une faute de frappe retire le composant au lieu de produire une animation par défaut. C'est le contrat que CssStyle porte déjà — c'est pourquoi margin-top supprime un composant — donc cette extension prolonge une politique existante au lieu d'en inventer une.

Corrections documentaires

Trois documents d'entrée mentaient sur le code, dont deux lus par les LLM qui génèrent les scénarios :

  • CLAUDE.md et rules/module-structure.md désignaient engine::text::cosmic comme le moteur de texte. Il n'a aucun appelant sur le chemin de rendu.
  • rules/html-css-mental-model.md enseignait margin-top / margin-left et padding: [32, 48], formes que CssStyle rejette — ce qui supprime le composant entier de la vidéo.

Vérification

  • cargo test --workspace : 21 cibles, 0 échec
  • cargo fmt --all --check et cargo clippy --workspace --all-targets -- -D warnings : propres
  • 6 nouveaux fichiers de test, dont degenerate_inputs.rs (les painters face à des entrées valides au schéma mais dégénérées) et exported_schema_examples.rs (les exemples du dépôt contre le schéma exporté)
  • 7 des 8 exemples passent rustmotion validate, avec et sans --strict-anim. Le huitième est l'issue examples/ferriskey-presentation.json does not pass rustmotion validate (pre-existing on main) #157, dont l'échec est prouvé préexistant à main.

Hors périmètre, suivi séparément

Note sur la méthode de merge

Un squash collapse les 14 lots en un seul commit sur main : la bisection sur ce chantier deviendrait inutilisable, et chaque message de lot — qui porte les mesures avant/après et les justifications de décision — serait perdu dans un seul corps. Un merge commit les préserve. C'est ton appel ; je signale juste que l'information a un coût ici.

`cargo fmt --all --check` is the first CI job and it has been failing on
main: five sites across counter.rs, animator.rs and transition.rs were
merged unformatted, so every pull request opened since inherits a red
build regardless of its own contents.

No behavioural change — rustfmt output only.
* fix(encode): order ffmpeg inputs ahead of output options

FFmpeg parses argv positionally: an option applies to the next `-i` that
follows it. The audio input was emitted after the codec block, so `-c:v`,
`-crf`, `-profile:v` and `-pix_fmt` were read as *input* options for
audio.raw and ffmpeg refused to start with "Option profile:v cannot be
applied to input url". Every scenario carrying an audio track — or an
embedded video with a soundtrack — failed to encode, on all four codecs,
through the default path.

Move the audio input next to the video input and keep `-c:a`/`-b:a` with
the output description. The argv assembly moves into `ffmpeg_args`, a pure
function, so the ordering invariant is unit-testable without an ffmpeg
binary on the machine.

A broken pipe here almost always means ffmpeg already died on its own
arguments, so `FfmpegWrite` now carries the tail of ffmpeg's stderr. It
used to be printed only outside `--quiet`, which left the diagnosis of
this very bug reading "Failed to write to FFmpeg pipe: Broken pipe".

* style(encode): apply rustfmt to the ffmpeg argv builder
)

Each of these renders a scenario that `rustmotion validate` accepts, and
each one killed the encode mid-frame. A painter that panics takes the whole
render with it, so the guard belongs in the painter rather than upstream.

- shape: skia asserts `pos.len() == colors.len()` inside the gradient
  shader, so a `stops` list of a different length than `colors` aborted the
  process. Drop stops we cannot honour and let skia space the colours.
- table: `"row_colors": []` deserializes to Some(vec![]), not None, so the
  default palette was never substituted and the modulo guard still indexed
  an empty slice.
- tag_cloud: same shape — `palette()` handed back the caller's empty vec and
  the painter took `index % len()` on it.
- dot_map: `dot_spacing: 0` makes the division +inf, and `inf as u32`
  saturates to u32::MAX, scheduling ~1.8e19 iterations. The geometry pass
  rejects 0.01 but not 0, so the floor has to live in the painter. Also caps
  the grid per axis so a large box cannot schedule unbounded work.
- codeblock diff: `col`, `delete` and `insert` all counted bytes while the
  reveal interpolates a fraction of that total, so mid-animation offsets
  landed inside a multi-byte glyph and `replace_range` aborted. Switch the
  accounting to characters and convert to byte offsets only when slicing.
  This also fixes the animation itself: a CJK glyph used to take three
  reveal steps instead of one.

The new integration test drives all five through the real pipeline — serde,
box_builder, run_layout, paint_tree — so a fix that guarded the painter but
left the component undeserialisable would still fail. dot_map paints on a
worker with a deadline, because a runaway loop would otherwise wedge CI
instead of reporting.
…145)

`skills install` wrote CLAUDE.md wholesale and `skills uninstall` deleted
it, treating a file the user authors as rustmotion's property: a project
with its own build notes lost them on install and lost the file on
uninstall. rustmotion now claims a delimited block and never touches
anything outside it — install merges the block in place, uninstall removes
only the block and deletes the file solely when that block was all it held.

`--fix` serialises `LoadedScenario::raw`, which is the document *after*
variable substitution and include resolution — not the document on disk.
For a plain JSON scenario the two coincide; for anything templated the
write silently replaces the source with its own expansion. That single
cause produced three separate defects: an HTML input came back as JSON, a
`config` block and every `$var` disappeared (making `--var` a no-op on the
rewritten file), and `include` got inlined so path-based patches landed on
nodes the source never contained.

One rule closes all three: only write back a source `--fix` can reproduce.
Anything else is refused with a message naming the file and what to do
instead. The check reads the bytes on disk rather than the loaded tree,
because by then substitution has already erased the markers that make the
write unfaithful.
…ppened (#146)

`batch` produces N videos in one shot, and it was the one path that told
the truth about none of them.

With `--jobs > 1` a worker panic was swallowed: `let _ = h.join()` was true
to its comment ("panics are surfaced as failures below") in name only —
nothing below inspected the join result, and the closure's bookkeeping only
runs on the return path a panic skips. A batch where every render panicked
printed "0/N succeeded" and exited 0. Panics are now recorded as failures,
with a final guard should a row ever go neither counted nor reported.

Preflight ran a parse-only dry run, so `batch` skipped the schema and
geometry pass `render` applies — the viewport-overflow gate CLAUDE.md makes
mandatory was bypassed by the very mode that renders in bulk. It now runs
`validation::run_checks` per row, with render's no-flag defaults, and fails
the batch before a single frame is drawn.

`--name-template` interpolates values straight from the data file, so a row
carrying `../escaped` or `/tmp/absolute` wrote outside `--output-dir`.
Names are now rejected in preflight when they contain a parent-dir, root or
prefix component. The check is lexical rather than canonicalising: a
canonicalize-and-compare fails with ENOENT on a subdirectory that does not
exist yet, which would have broken the legitimate `{lang}/{id}.mp4` form.
The HTML dialect degraded four classes of input without a word, and
`rustmotion validate` answered "Valid scenario" for all of them. An author
who writes a construct the transpiler cannot honour has to be told.

`<style>` content was transpiled into a `text` component and painted into
the video. It is now refused: `<style>` has a real, expected visual effect
that the dialect cannot deliver (there is no cascade engine), so dropping
it silently defeats a genuine intent. `<script>`, `<title>`, `<noscript>`,
`<template>` and `<head>` are skipped instead — no browser paints them, so
ignoring them defeats nothing and merely stops their text leaking onto the
canvas.

A `<scene>` nested inside any container disappeared from the scenario. It
is now refused, naming the offending parent, and the search is recursive:
that also catches the case an unclosed `<p>` creates, where HTML5 error
recovery hoists the `<h1>` out of the scene and leaves an empty one behind.
Recursing silently would have hidden exactly that corruption.

`<img>`, `<video>` and `<svg>` became empty `div`s. They are now refused
with the equivalent `rm-*` element named in the message.

No `bool` schema field was reachable: `coerce_value` left `"false"` a
string, so `auto_scroll`, `diff`, `loop`, `show_grid` and friends could not
be expressed at all. It now matches `coerce_dsl_value`, and a bare HTML
boolean attribute resolves to `true`.
…150)

Eight confirmed findings across the data components, plus the same defects
found by the audit in files the workstream did not own.

Five components drove their reveal off raw scene time, so a chart with
`start_at: 2.0` was already fully drawn when it appeared. They now measure
elapsed time from `start_at`, matching `Counter::ramp_progress`. The same
bug in `gauge` and `dot_map` is fixed here rather than left for a later
pass — it is one defect in seven copies.

`progress` painted at its declared `width`/`height` instead of the box
taffy computed, so a bar inside a sized container ignored its own layout.
It now paints at `layout.width`/`layout.height`.

`stacked_bar` had no signed extent: negative totals rendered outside the
box. Stacks now grow either side of an anchored zero.

`heatmap` renormalised its data min→max, so a uniform grid of 5.0 painted
identically to a grid of 0.0 and `color_scale` did not mean what the docs
say. The scale is now the documented absolute 0..1. A neighbouring bug in
`interpolate_color` went with it: `t = 1.0` resolved to the second-to-last
colour because the local fraction was recomputed from the clamped segment.

A flat sparkline series divided by a floored range, normalising every point
to 0 and gluing the line to the bottom edge — it read as "collapsed to
zero" rather than "unchanged". Flat series now centre. Fixed in `sparkline`
and in the `stat` card that reimplements the same maths.

Axis labels were collected with `filter_map`, so one datum without a label
shifted every subsequent label onto the wrong bar. Labels now keep one slot
per datum. Fixed in `bar`, and in `line` and `waterfall` which carry the
identical bug.

`treemap` drew its label and value on a single baseline, so the value
overprinted the label. Fragments now stack.
Seven confirmed audit findings, all in the same failure family: an output
path that quietly disagreed with the frame stream, the sample rate, or the
duration it advertised.

- Audio muxed at the wrong rate. `mix_audio_tracks` resamples every track
  to one constant; two muxer call sites hardcoded a *different* rate when
  declaring the track. The PCM was correct, the declaration was not, so
  playback drifted. Both sites now read `audio::OUTPUT_SAMPLE_RATE`, and
  hardcoding a rate next to it is no longer possible without noticing.
- `still --time` picked its frame by walking scenes directly, while the
  encoder walks `build_frame_tasks`. Any scenario with a transition made
  the two diverge: the still showed a frame the video never contains. It
  now goes through the same task stream.
- `--format raw` emitted 120 frames for a 2s+2s scenario with a 1s fade
  where the MP4 emits 90, and skipped post-effects entirely.
- GIF playback ran short: `100.0 / fps` was rounded once and reused as a
  flat per-frame delay, so the rounding error accumulated (1.8s for a 2.0s
  scenario). Delays are now derived from cumulative rounded boundaries,
  with the 2cs floor preserved.
- JPEG stills failed unconditionally and left a zero-byte file behind:
  the encoder was handed RGBA, which it cannot take. Now flattened onto an
  opaque background first. `--format` also no longer loses to the output
  file's extension.
- Incremental encoding rendered every dirty frame into one `all_yuv`
  buffer before starting the encoder, holding the whole video in memory.
  Rendering and encoding now interleave.
- `ffmpeg` audio extraction wrote straight to the destination path, so a
  failed run left a truncated WAV that later runs treated as a valid
  cache. It now writes to a sibling scratch file and promotes on success.

Tests: 124 + 126 pass on this branch alone, without the other round-3 lots.
…rames (#152)

Eight confirmed audit findings on scene transitions and the `world` view —
the one mechanism meant to read as a single continuous shot, and the one
where every defect below is plainly visible on screen.

- A transition longer than the scene it leaves silently deleted frames from
  the *entering* scene. The outgoing scene cannot spend more frames than it
  has, but the budget was computed as if it could, and the overrun was
  taken out of the next scene. Now clamped to the outgoing scene's own
  frame count, with the accounting asserted frame by frame.
- `camera_pan_duration` longer than a scene teleported the camera:
  measured jump of 245.33px between two consecutive frames, now 21.33px.
- The outgoing scene's background held at full strength through the whole
  pan, then vanished in one frame — an avg-luma step of 109.0 between two
  frames, now at most 4.82.
- The scene cross-fade drove the *whole* frame to 50% opacity at every pan
  midpoint (both scenes at 0.5, nothing behind them). Mid-pan opacity is
  now 0.670 instead of sitting on that floor.
- `persist: true` snapped opacity back to 1.0 while the scene was still
  fading out — a 0.326 discontinuity, now 0.034.
- A view-to-view transition duplicated the junction frame on both sides.
  `ViewTransition` progress is now an open interval, matching what
  `SlideTransition` already guaranteed.
- `scene.freeze_at` was read by no world code path at all.
- `scene.effects` (post-effects) were dropped on world frames and on view
  transition frames.

Every figure above is a before/after measurement from the tests added here,
not an estimate.

Tests: 144 pass on this branch alone.
…onts asked for (#153)

Ten confirmed audit findings on text measure/paint. Three of them make text
that is simply never legible; four more make the geometry validator blind to
overflow it is supposed to catch, because measure and paint disagree.

- No missing-glyph fallback: any non-Latin text (CJK, Arabic, Devanagari)
  rendered as tofu boxes. The engine picked one typeface and drew whatever
  it produced, including .notdef.
- `line-height` in `%` or `em` resolved to 0 at measure time, so taffy
  reserved a zero-height box and the text was never painted at all.
- `letter-spacing` in relative units was resolved at paint but ignored at
  measure — the box was too small, the glyphs overflowed it, and because
  the *measure* was what the geometry pass inspected, `validate` reported
  the scenario clean.
- `caption` ignored the width the layout gave it and painted one wide line
  past the device edge, again invisible to `validate`. Its `line-height`
  was also hardcoded to `font_size * 1.4` at paint while the box was
  measured with `line_height_for`.
- `font-weight` and `font-style` were dropped entirely as soon as a custom
  or Google font was declared — bold and italic silently became regular.
- `codeblock` and `terminal` never used the scenario's custom/Google
  fonts. `Terminal` and `TerminalIntrinsic` also resolved their typeface
  through two separate code paths that could disagree; they now share one
  `resolve_typeface`, so the box measured and the glyphs drawn cannot
  diverge.
- `codeblock` auto-scroll derived its offset from the full code height
  instead of the revealed lines, leaving the block empty for most of the
  reveal.
- `is_emoji` classified ©, ™ and ✓ as emoji, so the style's colour was
  dropped (or a tofu drawn) — including in a shipped example.

Also corrects two entry documents that named `engine::text::cosmic` as the
text engine. It has no callers on the render path; the real path is
`engine::renderer::text` + `rustmotion-components::intrinsic`. The module
keeps building, now with a doc comment saying plainly that it is dormant, so
the next reader does not spend a session editing dead code.

Tests: 116 + 6 + 3 + 5 + 211 + 3 pass on this branch alone.
…g fix real (#154)

Seven confirmed audit findings in the paint pass, plus one regression guard
for a side effect the scroll-wrap fix would otherwise have introduced.

- `overflow: hidden` erased the node's own outset `box-shadow`: the clip
  was installed before the shadow was drawn, so the shadow was clipped away
  by the very box it was supposed to sit outside.
- `backdrop-filter` was neutralised whenever `opacity < 1` on the same node.
- The opacity `save_layer` was allocated with no bounds — one full-viewport
  layer per node. On a 60-frame scenario this dominated everything else:
  42-60s down to 0.5s.
- Leaf painters were handed the border-box origin with the padding insets
  zeroed, so every leaf ignored `padding`. `Codeblock` stays a deliberate,
  now-documented exception: it reads `style.padding` itself and paints its
  own background from the border box, so honouring the general contract for
  it too would double-apply padding.
- Transform percentages were resolved against `max(width, height)` on both
  axes instead of per-axis.
- Scrolling tiled backgrounds walked out of frame: the offset grew linearly
  with time forever while the draw loops only overscan by one tile, so the
  pattern left a growing blank band. The offset now wraps into one tile
  period, and `draw_bg_grid_dots`'s x-loop overscans symmetrically like its
  y-loop already did (it started at 0, with no left margin).
- Both documented gradient banding mitigations were inert. The render
  surfaces are created with no Skia `ColorSpace`, which short-circuits the
  conversion, so tagging the shader's colors with `srgb_linear` was a
  silent no-op — and subdividing an already-sRGB lerp is a mathematical
  identity (17x the stops, zero visual effect). `subdivide_gradient_stops`
  now does the gamma round-trip itself on plain `f32`s, which works whatever
  color space the destination surface ends up carrying.

The wrap above needed one companion fix, found while verifying it rather
than reported: geometry is periodic on `spacing` and survives a wrap, but
the dot pulse is a `sin` of position and is not. Fed canvas-local
coordinates, every dot's radius and alpha stepped at once, every
`spacing / speed` seconds. The pulse now reads the unwrapped scroll track,
and the test asserting this also asserts that the naive version still
steps — so it cannot quietly become vacuous.

Tests: 153 + 117 + 6 + 3 + 5 + 215 + 3 pass on this branch alone.
… the cascade (#156)

Five confirmed audit findings on CSS→taffy translation and intrinsic sizing.

- `vw`/`vh` were resolved against a hardcoded 1920x1080 and `em`/`rem`
  against a hardcoded 16px, because both production layout sites passed
  `ConversionContext::default()` while the real dimensions sat in scope one
  line away. On a 1080x1920 vertical video — a routine format here — `50vw`
  came out 960px instead of 540px, a 78% error. A shared
  `viewport_conversion_context` now builds the context from the real
  viewport; at 1920x1080 it is bit-identical to the old default, so the
  common case does not move.
- The CSS cascade was never executed. `cascade::inherit_from` was correct
  and tested, but nothing called it: `color` and `font-*` set on a
  container reached no child. It is now applied in `build_child` after
  position and before timeline states and animations, so an own value still
  beats an inherited one.
- `box-sizing`, `justify-items` and `justify-self` were declared by the
  schema and never translated. They are now. `order` cannot be — taffy has
  no style-level reordering primitive, its `order` is source order assigned
  during layout — so it warns instead of vanishing, once per process rather
  than once per node per frame.
- The default-size guard for components with no intrinsic measurer wrote a
  hardcoded width *and* height, each gated only by `is_none()` on its own
  axis, so an explicit `aspect-ratio` was overwritten: `width: 400` with
  `aspect-ratio: 16/9` produced height 80 instead of 225. The other axis is
  now derived from the ratio when one axis is explicit.
- `rules/html-css-mental-model.md` taught `margin-top` / `margin-left` and
  `padding: [32, 48]`. `CssStyle` has `deny_unknown_fields` and only
  `margin: Edges`, so those forms fail deserialization and drop the whole
  component from the video. This is a rule read by the LLMs that generate
  scenarios: it was actively teaching them to write JSON that deletes
  components. Verified by running the CLI, not by reading the struct.

Cascade and aspect-ratio both change how existing scenarios could render,
so both were checked against every shipped example: no node inherits a
property it was missing, and no example uses `aspect-ratio`.

Tests: full workspace green on this branch alone.
)

Eight confirmed audit findings. Six of them are dead knobs: a field the
author sets, the schema accepts, `validate` calls clean, and the engine
ignores. Nobody gets an error; the video simply lacks the animation that was
asked for. For a tool driven by generated JSON that is the worst failure
mode there is, because the correction loop never closes.

- `float_3d`'s `amplitude` never reached `PresetConfig`: `AnimationTiming`
  had no such field and the only converter wrote `None`. Every `float_3d`
  moved by the 12px default, so the documented parallax recipe produced no
  parallax. Measured: 60 requested, -12 delivered.
- `pulse` / `float` / `shake` / `spin` built keyframes at the literal times
  0.0 / 0.25 / 0.5 / 1.0 and never read `delay` or `duration`. With
  `delay: 1.0`, all four were already mid-animation at t=0.5s.
- `"loop": true` was inert on keyframe effects and `tilt_in`: the resolver
  was handed `None` for the preset config, fell back to `repeat: false`,
  and never called `loop_time`.
- `--strict-anim` resolved effects at global scene time while the engine
  resolves at remapped local time, so it flagged violations at instants
  that are never rendered — and missed real ones. It now reads the
  `time_params` the builder already computes.
- The completion budget added `start_at` to `delay + duration`, which the
  engine does not do: since PR #27 `start_at` gates visibility only and
  `delay` is absolute scene time. A 1s animation at `start_at: 1.5` in a 2s
  scene was reported as overrunning.
- Two keyframe animations on the same property summed or overwrote each
  other depending on whether one carried a `delay` — a field with nothing
  to do with composition, routing effects into two separately-resolved
  buckets. Now a single bucket with one rule: last declared wins, the CSS
  cascade rule, which was already the behaviour within a bucket.
- The spring solver returned NaN for `mass: 0` or `stiffness: 0` and
  diverged on negative damping, with no validation anywhere. A NaN reaching
  layout contaminates the whole tree. Both ends are handled: the solver
  floors its inputs, and `validate` now rejects the configs outright.
- Unknown keys inside `style.animation[*]` were never reported.

That last one has a deliberate consequence worth stating: `deny_unknown_fields`
on the nine effect-config structs means a typo now fails deserialization, and
`deserialize_children` skips a child it cannot parse. A misspelled key stops
producing a default-valued animation and starts removing the component, with
a stderr warning. That is the same contract `CssStyle` has carried all along
— it is why `margin-top` drops a component — so this extends an existing
policy rather than inventing one, and `validate` catches it first.

Tests: full workspace green on this branch alone.
…plementing it (#160)

Nine confirmed audit findings on the geometry pass. Almost all of them are
the same structural mistake: the validator forked a piece of the engine — a
padding constant, a root style, a transform pivot — instead of calling the
function the engine uses. Then the two drifted, and the validator started
reasoning about a geometry that is not the one being rendered. The fix is
mostly deletion of the duplicate, not correction of its value.

False negatives — a broken video shipped with no warning:

- `timeline` style states and audio-reactive transforms were invisible even
  under `--strict-anim`: the box tree was built once with `anim: None`, and
  both features only apply when a real animation context is present. It is
  now rebuilt per sample through `build_scene_from_refs` with a real
  `BuildAnimationCtx` — the same call `render_with_new_pipeline_iter` makes
  per rendered frame.
- Animated rotation was not modelled at all: `transform_bbox` read only
  translate and scale, so a `spin` left the frame undetected at every
  sample. It now folds through `apply_static_node_transform`, the same
  4-corner AABB the static path already used.
- The 40-sample cap dropped `--strict-anim` to one sample every 1.5s on a
  60s scene, so any excursion shorter than the step slipped through. Raised
  to 480, which holds the promised 8/s up to 60s. Measured cost on a 60s
  scene with 15 animated components: 309ms release, 543ms debug.

False positives — a correct scenario blocked, and `--fix` then damaging it:

- `unwrappable_text_overflow` was the only content-vs-own-box check outside
  the clipping-ancestor guard, so text legitimately clipped by an
  `overflow: hidden` ancestor was rejected and `--fix` stripped a correct
  `white-space`.
- `transform-origin` was ignored by the static transform fold, which always
  pivoted at the box centre.
- `content_overflows_card` is retired rather than patched. Its guard made it
  reachable only when neither the card nor anything between it and the node
  clips — which is exactly the `overflow: visible` case both CLAUDE.md and
  geometry-safety.md document as legal ("a badge sticking out of a card is
  legal"). When the card does clip, the guard already suppresses the whole
  block. There is no configuration where firing is both reachable and
  consistent with the documented contract. Content leaving the *device* is
  still caught — that is `check_viewport`'s job, and a test now pins that
  guarantee so the removal cannot silently become a blind spot.

Wrong geometry reported:

- `world` views were validated against the slide root layout. The centred
  default the world renderer synthesises is now a single shared
  `world_default_scene_layout`, used by both.
- `check_auto_scroll` assumed 16+16px of padding and, for terminal, the
  wrong default font size and a CSS line-height the painter ignores. It now
  calls `CodeblockIntrinsic` and `TerminalIntrinsic`, the measurers layout
  itself uses.

The `--fix` refusals from PR #145 were verified rather than rewritten: two
end-to-end tests now drive `cmd_validate` over a templated scenario and a
two-file `include` scenario, both with a real violation, and assert the
source files come back byte-identical. Doing better than refusing is not
possible for `include`, and not safe for templates: variable substitution
can replace a scalar with an object, so a path computed on the resolved tree
need not exist in the source.

Tests: full workspace green on this branch alone. 7/8 examples validate; the
eighth is issue #157, pre-existing on main.
…161)

Nine confirmed audit findings on the JSON contract. Each is a silent sink: a
legitimate value — usually the one CSS would have you write — swallowed
without a word, producing a wrong render instead of an error. An LLM does not
read serde's output; it sets a field, nothing objects, and it concludes the
field was honoured.

- `AnimatedBackground`'s hand-written `Deserialize` had three sinks, not the
  two the audit named. An unknown `preset` fell through `_ =>` to
  `gradient_shift` with no colors; `zones` parsed with
  `.ok().unwrap_or_default()`; and — the third, found by reading the
  function — `colors` and `gradient_type` did the same *even with a
  correctly spelled preset*. The symptom is the worst available: an entirely
  black video, no diagnostic. A correctly spelled `heropattern` in the flat
  legacy form also fell through to `gradient_shift`. Every branch now routes
  through a typed struct with error propagation.
- `Edges` accepted any object at all: `{"padding": {"horizontal": 20}}`
  deserialized to zero on all four sides. `CssStyle`'s `deny_unknown_fields`
  gave the illusion of protection, but the enum one level down had four
  defaulted fields and no guard of its own.
- `border-radius` per-corner was the only composite in `CssStyle` using
  snake_case. The kebab form every neighbour uses failed, fell to another
  variant, and produced radius 0. Both spellings now work, unknown ones are
  reported.
- `width: "max-content"` was unreachable: `Length`'s own string catch-all
  absorbed it before the `Keyword` variant was tried, so the box collapsed
  to 0. `LineHeight` had the identical defect with `"normal"` — found by
  sweeping the other fifteen untagged enums in the repo, which is now the
  complete list: no catch-all-before-specific ordering remains.
- Animation `property` was a free `String` on three types, so an unknown or
  wrongly-cased name made the animation inert. Now constrained at the schema
  layer only — the solver is untouched — with a did-you-mean when the sole
  difference is the naming convention.
- The exported JSON schema declared `background` invalid on `Scene` and
  `View`, because `deny_unknown_fields` emits `additionalProperties: false`
  and `background` was `schemars(skip)`. The repo's own examples failed
  against the schema the generators consume: 31 violations across 6 files.
- A literal `$` in any string was fatal if and only if the document happened
  to contain a `config` block — a price, or `$PATH` in a terminal, blocked
  by an unrelated key elsewhere. The scan now always runs, and warns rather
  than rejecting: a declared variable can never survive `merge_variables`
  unresolved, so anything the scan still finds is by definition outside the
  declared set. An override naming an undeclared variable stays a hard
  error.
- `PositionMode::Named` accepted any string while only `"absolute"` does
  anything, so `"position": "relative"` — legitimate CSS — dropped `x`/`y`
  in silence. It warns now, deduplicated per distinct value: `prepare_scene`
  re-runs this `Deserialize` over the whole tree once per frame, so an
  unguarded warning would print over a thousand times on a 1200-frame
  render.

Also completes PR #158's hardening at the level below: `deny_unknown_fields`
on `Animation` and `Keyframe`, the keys inside a `keyframes[*]`.

Tests: full workspace green on this branch alone; every example still
validates, and now also validates against the exported schema.
@LeadcodeDev LeadcodeDev added the bug Something isn't working label Aug 10, 2026
@LeadcodeDev LeadcodeDev self-assigned this Aug 10, 2026
@LeadcodeDev
LeadcodeDev merged commit 7f06053 into main Aug 10, 2026
3 checks passed
@LeadcodeDev
LeadcodeDev deleted the chantier/audit-remediation branch August 10, 2026 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant