⚠️No adversarial review ran on this PR.executus-skills has no .gitea/workflows/ and no repo secrets, so gadfly cannot be triggered here.
Reviewed by hand only.
23% of all code_exec calls across two production runs (9 of 39) failed on
gifsmith API misuse, hitting 7 of 14 workers. The recovery was always the same
shape: another round trip spent printing gifsmith's own source — or, in one
case, running dir(gifsmith) — to learn what the module contains. Three causes,
three fixes.
1. Unguessable re-exports → __all__, plus the two that were missing
The module exported some matplotlib artists and not others, and the only way
to find out which was to fail: two workers wrote from gifsmith import PathPatch and got an ImportError while FancyBboxPatch right beside it
worked. PathPatch and Path are now exported, on the rule "the drawing
vocabulary, whole".
__all__ states the surface — and it fixes what made self-service expensive
too. dir(gifsmith) returned 24 names of which nine were incidental imports
(glob, os, shutil, subprocess, sys, traceback…), several of them
things a scene author should never touch. SKILL.md now carries the same list
explicitly, so the question shouldn't arise in the first place.
2. Near-miss constructor kwargs → an error that names the miss
background= is accepted as an alias for bg. It's the obvious synonym
and an agent reached for it.
force= is rejected with the reason it was tempting: render(force="mp4")
really does take it, which is exactly what makes it read like an
Animation-level setting.
Anything else gets a difflib near-match plus the valid list.
Animation() got unexpected keyword argument(s): 'backgrnd' — did you mean 'background'?.
Valid: width, height, fps, dpi, bg (or background). Everything about the OUTPUT
(format, size, audio) is set on render(), not here.
3. Scenes fail at render time → dry-run every scene at t=0 first
A NameError in scene 4 surfaced only after scenes 0–3 were fully rendered —
and with a streaming pipe open, after frames had already gone into the encoder.
Two workers lost a whole render each to RuntimeError: every scene failed,
learning one broken name per attempt.
One frame per scene now reports every broken scene in a single pass, before
any scratch dir or ffmpeg pipe exists.
Per-scene isolation is preserved exactly — the issue is right that it's good
design already. A scene that fails the dry run is marked and skipped rather than
re-run to fail identically; the others still ship; and only an all-scenes-broken program raises early, which is precisely the case that was
paying for a full setup to learn nothing.
Verification
Run against the actual production failures (with matplotlib/PIL stubbed, since
this machine has neither and the repo has no test harness):
Animation(background="black") → works, sets bg
Animation(force="mp4") → explains that force belongs to render()
Animation(backgrnd=…) → suggests background
every name in __all__ resolves; PathPatch/Path present
no incidental import (glob, os, subprocess, …) leaks into __all__
dry run with all scenes broken → raises before setup, naming both tracebacks
dry run with one broken → marks scene 1, leaves scene 0 untouched
Not executed here: a full render (no matplotlib/PIL/ffmpeg locally). Worth one
real render before .skillpack apply gif.
Note
Touches the same file as #3
(mort#1519, the contact sheet). Different regions, but whichever merges second
may want a trivial rebase — happy to do it.
Closes [mort#1522](https://gitea.stevedudenhoeffer.com/steve/mort/issues/1522).
⚠️ **No adversarial review ran on this PR.** `executus-skills` has no
`.gitea/workflows/` and no repo secrets, so gadfly cannot be triggered here.
Reviewed by hand only.
**23% of all `code_exec` calls across two production runs (9 of 39) failed on
gifsmith API misuse, hitting 7 of 14 workers.** The recovery was always the same
shape: another round trip spent printing gifsmith's own source — or, in one
case, running `dir(gifsmith)` — to learn what the module contains. Three causes,
three fixes.
## 1. Unguessable re-exports → `__all__`, plus the two that were missing
The module exported *some* matplotlib artists and not others, and the only way
to find out which was to fail: two workers wrote `from gifsmith import
PathPatch` and got an `ImportError` while `FancyBboxPatch` right beside it
worked. `PathPatch` and `Path` are now exported, on the rule "the drawing
vocabulary, whole".
`__all__` states the surface — and it fixes what made self-service expensive
too. `dir(gifsmith)` returned 24 names of which nine were incidental imports
(`glob`, `os`, `shutil`, `subprocess`, `sys`, `traceback`…), several of them
things a scene author should never touch. `SKILL.md` now carries the same list
explicitly, so the question shouldn't arise in the first place.
## 2. Near-miss constructor kwargs → an error that names the miss
- `background=` is **accepted** as an alias for `bg`. It's the obvious synonym
and an agent reached for it.
- `force=` is **rejected with the reason it was tempting**: `render(force="mp4")`
really does take it, which is exactly what makes it read like an
Animation-level setting.
- Anything else gets a `difflib` near-match plus the valid list.
```
Animation() got unexpected keyword argument(s): 'backgrnd' — did you mean 'background'?.
Valid: width, height, fps, dpi, bg (or background). Everything about the OUTPUT
(format, size, audio) is set on render(), not here.
```
## 3. Scenes fail at render time → dry-run every scene at `t=0` first
A `NameError` in scene 4 surfaced only after scenes 0–3 were fully rendered —
and with a streaming pipe open, after frames had already gone into the encoder.
Two workers lost a whole render each to `RuntimeError: every scene failed`,
learning one broken name per attempt.
One frame per scene now reports **every** broken scene in a single pass, before
any scratch dir or ffmpeg pipe exists.
**Per-scene isolation is preserved exactly** — the issue is right that it's good
design already. A scene that fails the dry run is marked and skipped rather than
re-run to fail identically; the others still ship; and only an
*all-scenes-broken* program raises early, which is precisely the case that was
paying for a full setup to learn nothing.
## Verification
Run against the actual production failures (with matplotlib/PIL stubbed, since
this machine has neither and the repo has no test harness):
- `Animation(background="black")` → works, sets `bg`
- `Animation(force="mp4")` → explains that `force` belongs to `render()`
- `Animation(backgrnd=…)` → suggests `background`
- every name in `__all__` resolves; `PathPatch`/`Path` present
- no incidental import (`glob`, `os`, `subprocess`, …) leaks into `__all__`
- dry run with all scenes broken → raises before setup, naming both tracebacks
- dry run with one broken → marks scene 1, leaves scene 0 untouched
Not executed here: a full render (no matplotlib/PIL/ffmpeg locally). Worth one
real render before `.skillpack apply gif`.
## Note
Touches the same file as
[#3](https://gitea.stevedudenhoeffer.com/steve/executus-skills/pulls/3)
(mort#1519, the contact sheet). Different regions, but whichever merges second
may want a trivial rebase — happy to do it.
Closes mort#1522.
23% of all code_exec calls across two production runs (9 of 39) failed on
gifsmith API misuse, hitting 7 of 14 workers. The recovery was always the same
shape: another round trip spent printing gifsmith's own source — or, in one
case, running `dir(gifsmith)` — to learn what the module contains.
Three distinct causes, three fixes.
**Unguessable re-exports → `__all__`, plus the two that were missing.** The
module exported SOME matplotlib artists and not others, and the only way to find
out which was to fail: two workers wrote `from gifsmith import PathPatch` and
got an ImportError while `FancyBboxPatch` beside it worked. `PathPatch` and
`Path` are now exported, on the rule "the drawing vocabulary, whole". `__all__`
states the surface — and it also fixes what made self-service expensive:
`dir(gifsmith)` returned 24 names of which nine were incidental imports (glob,
os, shutil, subprocess, sys, traceback...), several being things a scene author
should never touch. SKILL.md now carries the same list, so the question should
not arise.
**Near-miss constructor kwargs → an error that names the miss.** `background=`
is accepted as an alias for `bg` (it is the obvious synonym, and an agent
reached for it). `force=` is rejected with the reason it was tempting —
`render(force="mp4")` really does take it, which makes it read like an
Animation-level setting. Anything else gets a difflib near-match and the valid
list. Both failures were one round trip each, and both are the kind a message
fixes for free.
**Scenes fail at render time → dry-run every scene at t=0 first.** A NameError
in scene 4 used to surface only after scenes 0-3 were fully rendered, and with
a streaming pipe open, after frames had gone into the encoder. Two workers lost
a whole render each to `RuntimeError: every scene failed`, learning one broken
name per attempt. One frame per scene now reports EVERY broken scene in a single
pass, before any scratch dir or ffmpeg pipe exists. Per-scene isolation is
preserved exactly: a scene that fails the dry run is marked and skipped rather
than re-run to fail identically, the others still ship, and only an
all-scenes-broken program raises early — which is precisely the case that was
paying for a full setup to learn nothing.
Verified against the production failures: `Animation(background=…)` now works,
`Animation(force=…)` explains itself, `Animation(backgrnd=…)` suggests
`background`, every name in `__all__` resolves, no incidental import leaks into
it, and the dry run marks exactly the broken scenes while leaving the good ones
untouched.
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#1522.
⚠️ No adversarial review ran on this PR.
executus-skillshas no.gitea/workflows/and no repo secrets, so gadfly cannot be triggered here.Reviewed by hand only.
23% of all
code_execcalls across two production runs (9 of 39) failed ongifsmith API misuse, hitting 7 of 14 workers. The recovery was always the same
shape: another round trip spent printing gifsmith's own source — or, in one
case, running
dir(gifsmith)— to learn what the module contains. Three causes,three fixes.
1. Unguessable re-exports →
__all__, plus the two that were missingThe module exported some matplotlib artists and not others, and the only way
to find out which was to fail: two workers wrote
from gifsmith import PathPatchand got anImportErrorwhileFancyBboxPatchright beside itworked.
PathPatchandPathare now exported, on the rule "the drawingvocabulary, whole".
__all__states the surface — and it fixes what made self-service expensivetoo.
dir(gifsmith)returned 24 names of which nine were incidental imports(
glob,os,shutil,subprocess,sys,traceback…), several of themthings a scene author should never touch.
SKILL.mdnow carries the same listexplicitly, so the question shouldn't arise in the first place.
2. Near-miss constructor kwargs → an error that names the miss
background=is accepted as an alias forbg. It's the obvious synonymand an agent reached for it.
force=is rejected with the reason it was tempting:render(force="mp4")really does take it, which is exactly what makes it read like an
Animation-level setting.
difflibnear-match plus the valid list.3. Scenes fail at render time → dry-run every scene at
t=0firstA
NameErrorin scene 4 surfaced only after scenes 0–3 were fully rendered —and with a streaming pipe open, after frames had already gone into the encoder.
Two workers lost a whole render each to
RuntimeError: every scene failed,learning one broken name per attempt.
One frame per scene now reports every broken scene in a single pass, before
any scratch dir or ffmpeg pipe exists.
Per-scene isolation is preserved exactly — the issue is right that it's good
design already. A scene that fails the dry run is marked and skipped rather than
re-run to fail identically; the others still ship; and only an
all-scenes-broken program raises early, which is precisely the case that was
paying for a full setup to learn nothing.
Verification
Run against the actual production failures (with matplotlib/PIL stubbed, since
this machine has neither and the repo has no test harness):
Animation(background="black")→ works, setsbgAnimation(force="mp4")→ explains thatforcebelongs torender()Animation(backgrnd=…)→ suggestsbackground__all__resolves;PathPatch/Pathpresentglob,os,subprocess, …) leaks into__all__Not executed here: a full render (no matplotlib/PIL/ffmpeg locally). Worth one
real render before
.skillpack apply gif.Note
Touches the same file as
#3
(mort#1519, the contact sheet). Different regions, but whichever merges second
may want a trivial rebase — happy to do it.