Sandpies Claude Opus 5 commited on
Commit
1c1d6f7
·
1 Parent(s): 2bab0f7

Three things that were quietly not true

Browse files

None of these changes a frame. All three were claims -- in a check, in an
error path, in a docstring -- that did not hold.

**A tripwire that asserted nothing.** `check_latent_sidecar` guards that the
retired pickle sidecar is never read back, by searching `store.py`'s latent
reader for `LEGACY_LATENT_EXT`. It searched for `def get_latent(`; the
reader is `_get_latent`. The substring never matched, the ternary always
took its `else True`, and the check had been passing on nothing since it was
written. Now it matches the real name, with an `assert` above it so a rename
fails loudly instead of going quiet again.

**An error path that blamed the wrong thing.** `routes.py`'s SWAP encoder
returned the `for` loop's variable. `video_frame_data_urls` documents `[]`
as its failure mode, and on an empty list that name is unbound -- so a clip
whose frames could not be decoded raised `UnboundLocalError` into the
handler, which logged "could not attach stills" (naming the identity
pictures, which were fine) and threw away the ones already encoded. It now
returns the first frame it actually got, or None.

**A docstring that claimed a refactor that did not land.** `_repair_loop`
said WRITE and SWAP each pass their own consume. Only SWAP does.
`write_plan` still runs its own copy, because it rewrites the conversation
between attempts -- remapping rail tags by filename, merging the register,
restoring pinned `mp` and `file` -- before anything validates, which is not
a shape `consume` can express. That is a real constraint, not an oversight,
so the prose now says there are two loops and that the repair protocol has
to be changed in both. CLAUDE.md section 6c said the same thing and is
corrected the same way.

Also drops `media.video_first_frame_data_url`, dead since SWAP started
sending three frames instead of one. Two frame extractors to keep in step,
one of them called by nothing.

Found by an ultrareview of the v2 branch.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RVz8qNGn2NhrP3KG2Lh5yT

Files changed (4) hide show
  1. CLAUDE.md +9 -3
  2. media.py +3 -35
  3. routes.py +6 -1
  4. tools/check_latent_sidecar.py +6 -3
CLAUDE.md CHANGED
@@ -255,9 +255,15 @@ generate/validate/repair loop, and must not Accept over a multi-shot plan
255
  without an explicit replace. A change that is about hops 2+, the pin, the
256
  tone anchor, the audio lock, or the hop cache is not a SWAP change.
257
 
258
- The generate/validate/repair *loop* (`planner._repair_loop`) is shared
259
- mechanism; SWAP's instruct (`prompt_pack/SWAP_PROMPT.md`) and validator
260
- (`validate_swap`) are separate policy. WRITE's `validate()` and
 
 
 
 
 
 
261
  `system_prompt()` are not on this path. `tools/check_swap_boundary.py`
262
  enforces the clauses that can be checked without a browser. Gaps it cannot
263
  cover are named in that file's docstring.
 
255
  without an explicit replace. A change that is about hops 2+, the pin, the
256
  tone anchor, the audio lock, or the hop cache is not a SWAP change.
257
 
258
+ The generate/validate/repair *loop* (`planner._repair_loop`) is
259
+ mechanism, extracted so SWAP did not fork WRITE's policy loop to get one.
260
+ **Only SWAP calls it.** `write_plan` still runs its own copy, because it
261
+ rewrites the conversation between attempts -- remapping rail tags by
262
+ filename, merging the register, restoring pinned `mp` and `file` -- before
263
+ anything validates, which `consume` cannot express. Two loops, and a change
264
+ to the repair protocol has to land in both. SWAP's instruct
265
+ (`prompt_pack/SWAP_PROMPT.md`) and validator (`validate_swap`) are separate
266
+ policy. WRITE's `validate()` and
267
  `system_prompt()` are not on this path. `tools/check_swap_boundary.py`
268
  enforces the clauses that can be checked without a browser. Gaps it cannot
269
  cover are named in that file's docstring.
media.py CHANGED
@@ -139,10 +139,9 @@ def video_frame_data_urls(name, start=0.0, end=0.0, count=3,
139
  differ by almost nothing, so they would cost three times the tokens to say
140
  the same thing once.
141
 
142
- Frames are counted rather than sought, matching `load_video` and
143
- `video_first_frame_data_url`: a keyframe seek can land a second off, and a
144
- caption of the wrong moment is worse than a caption of one moment. Failure
145
- is an empty list, never a raise.
146
  """
147
  path = resolve(name, kinds={"video"})
148
  if path is None:
@@ -183,37 +182,6 @@ def video_frame_data_urls(name, start=0.0, end=0.0, count=3,
183
  return []
184
 
185
 
186
- def video_first_frame_data_url(name, start=0.0, max_side=VISION_SIDE):
187
- """JPEG data-URL of one frame at `start` seconds. -> str or None.
188
-
189
- Prefix-checked through `resolve`. Frames are counted rather than sought,
190
- matching `load_video`: a keyframe seek can land a second off, and a
191
- caption of the wrong frame is worse than no caption. Failure is None,
192
- never a raise.
193
- """
194
- path = resolve(name, kinds={"video"})
195
- if path is None:
196
- return None
197
- try:
198
- import av # noqa: PLC0415
199
- from PIL import Image # noqa: PLC0415
200
-
201
- with av.open(path) as container:
202
- vs = container.streams.video[0]
203
- fps = float(vs.average_rate or 0) or 24.0
204
- first = max(0, int(round(float(start or 0.0) * fps)))
205
- for i, frame in enumerate(container.decode(video=0)):
206
- if i < first:
207
- continue
208
- arr = frame.to_ndarray(format="rgb24")
209
- im = Image.fromarray(arr)
210
- return _pil_jpeg_data_url(im, max_side=max_side)
211
- return None
212
- except Exception as exc:
213
- print(f"[{TAG}] could not attach a frame of {name!r}: {exc!r}",
214
- flush=True)
215
- return None
216
-
217
 
218
  def resolve(name, kinds=None):
219
  """Absolute path for a reference basename, or None.
 
139
  differ by almost nothing, so they would cost three times the tokens to say
140
  the same thing once.
141
 
142
+ Frames are counted rather than sought, matching `load_video`: a keyframe
143
+ seek can land a second off, and a caption of the wrong moment is worse
144
+ than a caption of one moment. Failure is an empty list, never a raise.
 
145
  """
146
  path = resolve(name, kinds={"video"})
147
  if path is None:
 
182
  return []
183
 
184
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
185
 
186
  def resolve(name, kinds=None):
187
  """Absolute path for a reference basename, or None.
routes.py CHANGED
@@ -650,6 +650,11 @@ def register():
650
  # order out loud is what turns three pictures into a movement.
651
  frames = _media.video_frame_data_urls(
652
  video, start=start, end=end, count=3)
 
 
 
 
 
653
  for k, frame in enumerate(frames, 1):
654
  images.append({
655
  "caption": (
@@ -661,7 +666,7 @@ def register():
661
  ),
662
  "data_url": frame,
663
  })
664
- return images, frame
665
 
666
  try:
667
  images, frame = await asyncio.get_running_loop().run_in_executor(
 
650
  # order out loud is what turns three pictures into a movement.
651
  frames = _media.video_frame_data_urls(
652
  video, start=start, end=end, count=3)
653
+ # Not the loop variable: `video_frame_data_urls` documents [] as
654
+ # its failure mode, and returning `frame` from an empty loop raised
655
+ # UnboundLocalError into the handler below -- which then blamed the
656
+ # identity stills and threw away the ones it had already encoded.
657
+ first = frames[0] if frames else None
658
  for k, frame in enumerate(frames, 1):
659
  images.append({
660
  "caption": (
 
666
  ),
667
  "data_url": frame,
668
  })
669
+ return images, first
670
 
671
  try:
672
  images, frame = await asyncio.get_running_loop().run_in_executor(
tools/check_latent_sidecar.py CHANGED
@@ -155,10 +155,13 @@ def main():
155
  "or the cache silently exceeds its own budget")
156
  ck("eviction removes it", "LEGACY_LATENT_EXT" in sweep,
157
  "or it outlives the entry it belongs to")
 
 
 
 
158
  ck("nothing ever reads it",
159
- "LEGACY_LATENT_EXT" not in src[src.index("def get_latent("):
160
- src.index("def entries(")]
161
- if "def get_latent(" in src else True,
162
  "reading it back would need weights_only=False")
163
 
164
  print("refusals -- None means cache the frames, skip the latent")
 
155
  "or the cache silently exceeds its own budget")
156
  ck("eviction removes it", "LEGACY_LATENT_EXT" in sweep,
157
  "or it outlives the entry it belongs to")
158
+ # `def _get_latent(` -- store.py's reader is private, and the
159
+ # public spelling never matched, so the ternary always took `else True`
160
+ # and this tripwire asserted nothing at all.
161
+ assert "def _get_latent(" in src, "store.py's latent reader was renamed"
162
  ck("nothing ever reads it",
163
+ "LEGACY_LATENT_EXT" not in src[src.index("def _get_latent("):
164
+ src.index("def entries(")],
 
165
  "reading it back would need weights_only=False")
166
 
167
  print("refusals -- None means cache the frames, skip the latent")