[ayam04]

What changed

frames_at_indices selected pictures with ffmpeg's select on the decoded-picture counter n, which assumes decoded picture n is message n. On a pass-through camera that loses a keyframe the decoder drops the pictures that depend on it (and conceals one), so every index after the gap returned a picture shifted by the shortfall while still reporting the requested message's log_time_ns.

The selection now keys on time: the MP4 is constant rate, so message k is the picture nearest k / fps, and each requested index (or run of consecutive indices) compiles to between(t,(start-0.5)/fps,(end+0.5)/fps). A picture the decoder never produced matches nothing, so the existing "ffmpeg extracted X of Y selected frames" check raises instead of handing back a shifted frame, which is the contract #727's Expected behavior asks for.

Validation

RED before the change, GREEN after, on the regression added here:

tests/test_end_to_end.py::test_frames_at_indices_pairs_pictures_by_time_when_a_keyframe_is_missing
before (6cf6e98): assert [50, 59] == [42, 51]   # asked for messages 41 and 50, got pictures 8 frames late
after:            passed

The regression pins content, not counts: each source picture carries its own frame number as ten black/white blocks and the test reads that number back out of the returned JPEG. It also requests frames inside the gap, which now raises.

Gate (WSL, the worktree's uv sync --locked --all-extras environment):

uv run --no-sync pytest tests/test_episode.py tests/test_end_to_end.py -q  -> 51 passed
uv run --no-sync ruff check src/hflow/episode.py tests/test_end_to_end.py  -> All checks passed
uv run --no-sync ruff format --check src/hflow/episode.py tests/test_end_to_end.py -> 2 files already formatted
uv run --no-sync ty check                                                  -> All checks passed

Notes

• Credit for the diagnosis and the direction: @xb-gjf's report identified the counter-vs-message mismatch and validated timestamp-based selection on a damaged stream. If you would rather they carry it, close this and I will hand over the diff.
• Out of scope here, from the other options in [Bug]: frames_at_indices returns frames shifted by several positions, without an error, after a pass-through H.264 camera loses a keyframe #727: refusing or quarantining a channel with a lost reference at transform time, and catching the single concealed picture that a gap can still admit (that needs H.264 frame_num or the keyframe-cadence check).
• The 20,000-index sparse-selection test now passes the camera's fps to _frame_selection_expression, since the expression is built from timestamps.

Refs #727

---

^{Need help on this PR? Tag @codesmith-bot with what you need.}

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no blocking issues remain.

Summary

frames_at_indices now selects pictures by time instead of the decoded-picture count.

• Camera frame requests now match pictures by time.

_{Reviews (3) · Last reviewed commit: "fix(episode): retire frame caches writte..." · Reviewed by Greptile}

[kstonekuan]

Could you invalidate the old indexed-frame cache and add upgrade coverage? With the regression's damaged stream and indices [41, 50], I populated a workdir using main's counter selection, then reopened it on this head: it still returns pictures [50, 59]; a fresh workdir returns the correct [42, 51].

Please also change Fixes #727 to Refs #727: #731 tracks sampled frames, but the concealed-picture case from #727 still needs to stay open.

Dismissed because a newer commit was pushed; Greptile will re-review the current head.

[ayam04]

Both done in 35d358a.

Cache invalidation. You were right that the stale workdir was the whole risk: the cache directory name is the key, and the selection change left it unchanged, so a workdir built on main kept serving the counter selection's pictures. The name now carries a layout tag:

_FRAMES_AT_INDICES_CACHE_LAYOUT = "v2-time"

f"frames_{_sanitize_topic(topic)}_indices_{selection_digest}_"
f"count_{len(selected_frame_indices)}_{_FRAMES_AT_INDICES_CACHE_LAYOUT}"

It is a constant rather than part of the digest because the key has to change when the selection semantics change, not when the indices do. Every directory written before this change is retired by name; nothing is deleted, so a concurrent reader of an old workdir is never raced or surprised mid-read.

Upgrade coverage. The new regression builds your scenario directly: it takes the damaged stream from #727, writes the pictures main produced for indices [41, 50] -- frame numbers 50 and 59, read back out of the JPEGs -- into the directory main named, under the workdir the episode then opens, and asks the same episode for the same indices.

• without the tag: assert [50, 59] == [42, 51], exactly what you saw;
• with it: [42, 51], and every returned path is outside the retired directory, so the check is about provenance as well as content.

Fixes -> Refs. The PR body and both commit messages already say Refs #727; #731 keeps the sampled-frames case open, and the concealed-picture case from #727 stays with it.

Gate on this head: pytest tests/test_episode.py tests/test_end_to_end.py -q -> 53 passed; ruff check, ruff format --check, ty check -> clean. Branch is MERGEABLE/CLEAN.

[greptile-apps]

Want your agent to iterate on Greptile's feedback? Try greploops.