⚠️No adversarial review ran on this PR.executus-skills has no .gitea/workflows/ and no repo secrets, so gadfly cannot be triggered here —
unlike the four mort PRs in this batch. Reviewed by hand only. Wiring gadfly
into this repo is a separate decision.
The bug
_contact_sheet built one tile per scene, and a single-scene animation —
the common case — short-circuited to tiles[0][1].first_frame.save(path), i.e.
a bare frame. The harness then printed still: … (contact sheet) next to it.
Every single-scene worker across two production runs corrected that claim
unprompted, in the vision model's own words — "this is a single frame (not a
contact sheet)", "there is exactly 1 distinct panel/tile", "a single static
illustration, not an animation contact sheet" — ten of them. One worked out why
its critic could not see the brine tear it had animated, then proved it with a
pixel count, after three full re-renders and 690 seconds. It was the last of
nine parallel workers to finish, holding the whole fan-out open.
The fix
The sheet is now sampled across time: nine tiles by default spread over the
whole animation, each labelled with its t. One scene gets all nine time
points; many scenes still get one tile each, so the multi-scene case that
already worked is unchanged.
Two details that matter more than they look:
Never sample t=1.0. A loop is built to return to its starting state, so
its closing frame is the one instant guaranteed to show none of the motion —
exactly the frame the tear had faded out of.
Label every tile with its t. Without it a critic cannot tell "the tear
is missing" from "the tear is missing at t=0.75", and only the second is
actionable. SKILL.md's critique step now says so directly: a transient
absent from one tile is not absent from the animation — check the others
before re-rendering.
Reconciling the t≈1.0 vs first_frame discrepancy
The issue asked for this to be checked before fixing. At HEAD it is neither:
ifsc.first_frameisNoneandk>=n//2:# the MIDDLE frame
That line arrived with the harness in 7906a1a, so the t≈1.0 pixel-count
observation came from an older deployed pack (this repo's changes are inert
until .skillpack apply gif). It doesn't change the fix — one time point is
one time point whichever one it is, and the single-scene short-circuit was the
real defect — but it's worth knowing the deployed pack lags this branch.
Also
A failed scene drops its samples along with its partial frames, so the sheet
can't advertise a scene that never shipped.
Thumbnails are downscaled at capture rather than held full-size: nine
640×480 RGB copies is ~8 MiB held for the whole render, and the sheet shrinks
them anyway.
Verification, and its limits
_sample_frames is verified locally across n ∈ {1, 2, 3, 10, 84, 500}: evenly
spread, always includes t=0, never the closing frame, always non-empty,
clamped to n (a very short scene yields fewer tiles, which is the honest
outcome). n=84, count=9 → [0, 9, 18, 28, 37, 46, 55, 65, 74].
The grid assembly itself I could not execute here — this machine has no
matplotlib or PIL, and the repo has no test harness or CI at all. The cols/rows
arithmetic is unchanged from the previous version; only the tile source and the
label changed. Worth a real render before .skillpack apply.
Closes [mort#1519](https://gitea.stevedudenhoeffer.com/steve/mort/issues/1519).
⚠️ **No adversarial review ran on this PR.** `executus-skills` has no
`.gitea/workflows/` and no repo secrets, so gadfly cannot be triggered here —
unlike the four mort PRs in this batch. Reviewed by hand only. Wiring gadfly
into this repo is a separate decision.
## The bug
`_contact_sheet` built one tile per **scene**, and a single-scene animation —
the common case — short-circuited to `tiles[0][1].first_frame.save(path)`, i.e.
a bare frame. The harness then printed `still: … (contact sheet)` next to it.
Every single-scene worker across two production runs corrected that claim
unprompted, in the vision model's own words — "this is a single frame (not a
contact sheet)", "there is exactly 1 distinct panel/tile", "a single static
illustration, not an animation contact sheet" — ten of them. One worked out why
its critic could not see the brine tear it had animated, then proved it with a
pixel count, after three full re-renders and 690 seconds. It was the last of
nine parallel workers to finish, holding the whole fan-out open.
## The fix
The sheet is now sampled across **time**: nine tiles by default spread over the
whole animation, each labelled with its `t`. One scene gets all nine time
points; many scenes still get one tile each, so the multi-scene case that
already worked is unchanged.
Two details that matter more than they look:
- **Never sample `t=1.0`.** A loop is built to return to its starting state, so
its closing frame is the one instant guaranteed to show none of the motion —
exactly the frame the tear had faded out of.
- **Label every tile with its `t`.** Without it a critic cannot tell "the tear
is missing" from "the tear is missing at t=0.75", and only the second is
actionable. `SKILL.md`'s critique step now says so directly: a transient
absent from one tile is not absent from the animation — check the others
before re-rendering.
## Reconciling the `t≈1.0` vs `first_frame` discrepancy
The issue asked for this to be checked before fixing. At HEAD it is **neither**:
```python
if sc.first_frame is None and k >= n // 2: # the MIDDLE frame
```
That line arrived with the harness in `7906a1a`, so the `t≈1.0` pixel-count
observation came from an **older deployed pack** (this repo's changes are inert
until `.skillpack apply gif`). It doesn't change the fix — one time point is
one time point whichever one it is, and the single-scene short-circuit was the
real defect — but it's worth knowing the deployed pack lags this branch.
## Also
- A failed scene drops its samples along with its partial frames, so the sheet
can't advertise a scene that never shipped.
- Thumbnails are downscaled at capture rather than held full-size: nine
640×480 RGB copies is ~8 MiB held for the whole render, and the sheet shrinks
them anyway.
## Verification, and its limits
`_sample_frames` is verified locally across `n` ∈ {1, 2, 3, 10, 84, 500}: evenly
spread, always includes `t=0`, never the closing frame, always non-empty,
clamped to `n` (a very short scene yields fewer tiles, which is the honest
outcome). `n=84, count=9 → [0, 9, 18, 28, 37, 46, 55, 65, 74]`.
The grid assembly itself I could **not** execute here — this machine has no
matplotlib or PIL, and the repo has no test harness or CI at all. The cols/rows
arithmetic is unchanged from the previous version; only the tile source and the
label changed. Worth a real render before `.skillpack apply`.
Closes mort#1519.
`_contact_sheet` built one tile per SCENE, and a single-scene animation — the
common case — short-circuited to `tiles[0][1].first_frame.save(path)`, a bare
frame. The harness then printed "still: … (contact sheet)" next to it.
Every single-scene worker across two production runs corrected that claim
unprompted, in the vision model's own words: "this is a single frame (not a
contact sheet)", "there is exactly 1 distinct panel/tile", "a single static
illustration, not an animation contact sheet" — ten of them. One worked out why
its critic could not see the brine tear it had animated, then proved it with a
pixel count, having already spent three full re-renders and 690 seconds on it.
It was the last of nine parallel workers to finish, holding the whole fan-out
open.
The sheet is now sampled across TIME: nine tiles by default, spread over the
whole animation, each labelled with its `t`. One scene gets all nine time
points; many scenes still get one tile each, as before, so nothing regresses
for the multi-scene case that already worked.
Two details that matter more than they look:
**Never sample t=1.0.** A loop is built to return to its starting state, so its
closing frame is the one instant guaranteed to show none of the motion. That is
exactly the frame the tear had faded out of.
**Label every tile with its t.** Without it the critic cannot tell "the tear is
missing" from "the tear is missing at t=0.75", and only the second is
actionable. The SKILL.md critique step now says so directly: check the other
tiles before re-rendering.
Reconciling the discrepancy the issue raised: the body said the still is the
t≈1.0 frame (backed by a pixel count on a live render), the code reads
`first_frame`. At HEAD it is NEITHER — `if sc.first_frame is None and
k >= n // 2` captures the MIDDLE frame, and that line arrived with the harness
in 7906a1a. The t≈1.0 observation therefore came from an older deployed pack.
It does not change the fix: one time point is one time point, whichever one it
is, and the single-scene short-circuit was the real defect.
Also: a failed scene now drops its samples along with its partial frames, so
the sheet can't advertise a scene that never shipped. Thumbnails are downscaled
at capture rather than held full-size — nine 640x480 RGB copies is ~8 MiB held
for the whole render, and the sheet shrinks them anyway.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes mort#1519.
⚠️ No adversarial review ran on this PR.
executus-skillshas no.gitea/workflows/and no repo secrets, so gadfly cannot be triggered here —unlike the four mort PRs in this batch. Reviewed by hand only. Wiring gadfly
into this repo is a separate decision.
The bug
_contact_sheetbuilt one tile per scene, and a single-scene animation —the common case — short-circuited to
tiles[0][1].first_frame.save(path), i.e.a bare frame. The harness then printed
still: … (contact sheet)next to it.Every single-scene worker across two production runs corrected that claim
unprompted, in the vision model's own words — "this is a single frame (not a
contact sheet)", "there is exactly 1 distinct panel/tile", "a single static
illustration, not an animation contact sheet" — ten of them. One worked out why
its critic could not see the brine tear it had animated, then proved it with a
pixel count, after three full re-renders and 690 seconds. It was the last of
nine parallel workers to finish, holding the whole fan-out open.
The fix
The sheet is now sampled across time: nine tiles by default spread over the
whole animation, each labelled with its
t. One scene gets all nine timepoints; many scenes still get one tile each, so the multi-scene case that
already worked is unchanged.
Two details that matter more than they look:
t=1.0. A loop is built to return to its starting state, soits closing frame is the one instant guaranteed to show none of the motion —
exactly the frame the tear had faded out of.
t. Without it a critic cannot tell "the tearis missing" from "the tear is missing at t=0.75", and only the second is
actionable.
SKILL.md's critique step now says so directly: a transientabsent from one tile is not absent from the animation — check the others
before re-rendering.
Reconciling the
t≈1.0vsfirst_framediscrepancyThe issue asked for this to be checked before fixing. At HEAD it is neither:
That line arrived with the harness in
7906a1a, so thet≈1.0pixel-countobservation came from an older deployed pack (this repo's changes are inert
until
.skillpack apply gif). It doesn't change the fix — one time point isone time point whichever one it is, and the single-scene short-circuit was the
real defect — but it's worth knowing the deployed pack lags this branch.
Also
can't advertise a scene that never shipped.
640×480 RGB copies is ~8 MiB held for the whole render, and the sheet shrinks
them anyway.
Verification, and its limits
_sample_framesis verified locally acrossn∈ {1, 2, 3, 10, 84, 500}: evenlyspread, always includes
t=0, never the closing frame, always non-empty,clamped to
n(a very short scene yields fewer tiles, which is the honestoutcome).
n=84, count=9 → [0, 9, 18, 28, 37, 46, 55, 65, 74].The grid assembly itself I could not execute here — this machine has no
matplotlib or PIL, and the repo has no test harness or CI at all. The cols/rows
arithmetic is unchanged from the previous version; only the tile source and the
label changed. Worth a real render before
.skillpack apply.