steve/executus-skills
1
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(gif): make the contact sheet an actual contact sheet

Browse Source 
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]>
This commit is contained in:
steve
2026-07-26 12:09:28 -04:00
co-authored by Claude Opus 5
parent 7dee907d44
commit c1b2d41cc5
2 changed files with 87 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
gif/SKILL.md
+10 -3
View File
@@ -149,9 +149,16 @@ gifsicle are already in the sandbox.
## 5. Self-critique (AT MOST 3 render passes)
Before delivering, LOOK at what you made: call `image_describe` on the
**`still.png` file_id** — the harness already built it as a labelled contact
sheet, one tile per scene. Ask specifically about subject size, caption
readability and whether the depicted action matches the request. Then answer:
**`still.png` file_id** — the harness already built it as a contact sheet of
frames sampled evenly ACROSS the animation, each tile labelled with its `t`
(0.00 = first frame). Ask specifically about subject size, caption readability
and whether the depicted action matches the request.
**A thing missing from one tile is not a thing missing from the animation.**
The tiles are time points, so a transient — a tear rolling, a blink, a wipe —
appears in some and not others. If the critique says something is absent, check
the other tiles for it before re-rendering; that mistake once cost a worker
three full re-renders chasing a tear that was there all along. Then answer:
1. Does it depict what was asked — every named character, prop and action?
2. Is the subject **big enough to read** (≥ ⅓ frame height)?
gif/scripts/gifsmith.py
+77 -15
View File
@@ -67,6 +67,11 @@ GIF_MAX_BYTES = 9 * 1024 * 1024
# turns a borderline short piece into an MP4.
PNG_BYTES_PER_PIXEL = 0.29
# Contact-sheet budget. Nine tiles is a 3x3 grid that vision models read
# reliably; spread across TIME for one scene, across scenes for many.
CONTACT_SHEET_MAX_TILES = 9
CONTACT_TILE_W = 300
_FONT_CANDIDATES = [
"/usr/share/fonts/truetype/dejavu/DejaVuSans-Bold.ttf",
"/usr/share/fonts/truetype/dejavu/DejaVuSans.ttf",
@@ -110,7 +115,10 @@ class _Scene:
self.mode, self.hold = mode, float(hold)
self.frames = 0
self.error = None
self.first_frame = None
# (t, thumbnail) pairs for the contact sheet. Thumbnails, not full
# frames: nine 640x480 RGB copies is ~8 MiB of RAM held for the whole
# render, and the sheet downscales them anyway.
self.samples = []
class Animation:
@@ -241,20 +249,31 @@ class Animation:
print(f"[gifsmith] {len(self._scenes)} scene(s), {total_s:.1f}s at "
f"{self.fps}fps, canvas {self.width}x{self.height}", flush=True)
# Tile budget for the contact sheet, spread across the scenes. One scene
# gets the whole budget in TIME, which is the case that was broken: a
# loop's interesting content is by construction not at any single
# instant.
per_scene = max(1, CONTACT_SHEET_MAX_TILES // len(self._scenes))
for i, sc in enumerate(self._scenes):
start = self._count
n = max(1, int(round(sc.seconds * self.fps)))
try:
want = self._sample_frames(n, per_scene)
for k in range(n):
t = k / max(n - 1, 1)
img = (self._render_pil if sc.mode == "pil" else self._render_mpl)(sc, t)
if sc.first_frame is None and k >= n // 2:
sc.first_frame = img.copy()
if k in want:
sc.samples.append((t, img.resize(
(CONTACT_TILE_W, self._tile_h()), Image.LANCZOS)))
self._emit(img)
for _ in range(int(round(sc.hold * self.fps))):
self._emit(img)
except Exception: # noqa: BLE001
sc.error = traceback.format_exc(limit=6)
# Its partial frames are dropped below; its samples must go too,
# or the sheet advertises a scene that never shipped.
sc.samples = []
if not self._streaming:
# Drop this scene's partial frames so it can't ship half-drawn.
# (Streamed frames are already in the encoder — a broken scene
@@ -292,15 +311,19 @@ class Animation:
os.remove(leftover)
shutil.rmtree(self._scratch, ignore_errors=True)
raise
self._contact_sheet()
sheet_tiles = self._contact_sheet() or 0
broken = [f"{i}:{s.title!r}" for i, s in enumerate(self._scenes) if s.error]
print(f"[gifsmith] {self._count} frames, scratch "
+ ("streamed (0 MiB on disk)" if self._streaming
else f"peak {self._peak_mb:.1f} MiB"))
print(f"[gifsmith] scene errors: {', '.join(broken) if broken else 'none'}")
print(f"[gifsmith] ART {art}")
print("[gifsmith] still: /workspace/still.png (contact sheet — call "
"image_describe on ITS file_id from files_out)")
ts = ", ".join(f"{t:.2f}" for sc in self._scenes for (t, _th) in sc.samples)
print(f"[gifsmith] still: /workspace/still.png — {sheet_tiles} frames "
f"sampled across the animation at t={ts} (each tile is labelled "
"with its t). Call image_describe on ITS file_id from files_out; "
"if something is missing from one tile, check whether it is "
"present in another before re-rendering.")
shutil.rmtree(self._scratch, ignore_errors=True)
return art
@@ -411,25 +434,64 @@ class Animation:
# ------------------------------------------------------------- critique aid
def _tile_h(self):
return max(1, round(self.height * CONTACT_TILE_W / self.width))
@staticmethod
def _sample_frames(n, count):
"""Frame indices to keep for the contact sheet, spread evenly over the
scene: t = 0, 1/count, ... (count-1)/count.
A scene with very few frames yields fewer samples than asked for, since
distinct t values collide onto the same frame index; the sheet simply
has fewer tiles, which is the honest outcome.
Deliberately NOT ending at t=1.0. A loop is built to return to its
starting state, so its last frame is the one instant guaranteed to show
none of the motion — which is exactly how a worker spent three
re-renders chasing a tear its critic could not see, because the still
was the frame after the tear had faded."""
if n <= 1:
return {0}
count = max(1, min(count, n))
return {min(n - 1, round(j / count * (n - 1))) for j in range(count)}
def _contact_sheet(self, path="/workspace/still.png"):
tiles = [(i, s) for i, s in enumerate(self._scenes) if s.first_frame is not None]
"""A grid sampled across TIME, one labelled tile per sample.
It used to be one frame per SCENE, and a single-scene animation — the
common case — short-circuited to a bare frame save. Every single-scene
worker in two production runs told us so unprompted, in the words of
the vision model: "this is a single frame (not a contact sheet)". The
critique loop was being fed one time point out of ~84 and asked to
judge motion.
"""
tiles = [(i, sc, t, thumb)
for i, sc in enumerate(self._scenes)
for (t, thumb) in sc.samples]
if not tiles:
return
if len(tiles) == 1:
tiles[0][1].first_frame.save(path)
return
cols = min(3, len(tiles))
rows = math.ceil(len(tiles) / cols)
tw = 300
th = max(1, round(self.height * tw / self.width))
tw, th = CONTACT_TILE_W, self._tile_h()
sheet = Image.new("RGB", (cols * tw, rows * (th + 20)), (20, 20, 24))
d = ImageDraw.Draw(sheet)
f = self.font(14)
for k, (i, sc) in enumerate(tiles):
multi = len(self._scenes) > 1
for k, (i, sc, t, thumb) in enumerate(tiles):
x, y = (k % cols) * tw, (k // cols) * (th + 20)
sheet.paste(sc.first_frame.resize((tw, th), Image.LANCZOS), (x, y + 20))
d.text((x + 4, y + 3), f"scene {i}: {sc.title[:38]}", fill=(235, 235, 240), font=f)
sheet.paste(thumb, (x, y + 20))
# The t label is the point: it tells the critic WHEN it is looking,
# so "the tear is missing" can be read as "missing at t=0.75"
# rather than as a fault in the drawing.
label = f"t={t:.2f}"
if multi:
label = f"scene {i} {label}: {sc.title[:26]}"
elif sc.title:
label = f"{label} {sc.title[:30]}"
d.text((x + 4, y + 3), label, fill=(235, 235, 240), font=f)
sheet.save(path)
return len(tiles)
def _run(cmd):