[CompleteDotTech]

Closes #309

What this adds

• src/jev_cookbook/frames.py (new, pure Python, no new dependency): sample_frames, frame_timestamps, select_frames (copies carry frame_index), frame_intervals, relative_frames, window_timestamps, frame_at, frame_windows, transition_time, earliest_transition.
• evaluation.transition_metrics + TransitionMetrics: exact and within-tolerance accuracy, review count with stated denominators (resolved and all), label-matched earliest-transition error in seconds (plus separately named any-label fields), per-example outcomes and timing_errors_seconds, max_sampling_interval_seconds / tolerance_below_sampling_interval.
• docs/vision.md (sampling rules, tie-breaking, no video decoding, elapsed-seconds convention, index spaces, recording the selection in fields) and docs/evaluation.md (the transition-time convention, tolerance versus sampling gap, review labels, multi-transition sequences, result type row).
• Tests: tests/test_frames.py (new), additions to tests/test_evaluation.py; tests/test_docs_examples.py bumped for the one new python block in docs/evaluation.md (8 to 9 blocks, empty preamble).

Deliberate deviation from the issue text: the helpers live in jev_cookbook.frames, not jev_cookbook.vision (#321 edits vision.py; it is untouched). jev_cookbook.frames is the single import path; no re-export is planned.

Review round 1 resolutions (head 8ff6588)

• Timing error counted wrong-label predictions (both reviews): the headline n_timed and means now cover only label-matched examples with times on both sides; the all-labels view is n_timed_any_label and *_any_label_*. Tests and docs updated (hand-computed: n_timed 2, MAE 0.2).
• Tie rule with frames closer than the slack (review 1 B2): the nearest search now considers the neighbouring frames and takes the earliest within the slack of the closest. Tests include [0, 1, 1+5e-10, 2] giving [0, 1, 3].
• No transition-time convention (review 2 B1): one convention, the timestamp_seconds of the first frame in which the new state is visible, for gold and predictions. frames.transition_time and frames.earliest_transition implement it; docs say tolerance should be at least the largest sampled gap; max_sampling_interval_seconds is recorded and tolerance_below_sampling_interval flags the opposite. A hand-computed end-to-end test pins it.
• Suggestions taken: JSON two-element lists accepted; uncertain_labels takes several review labels; ValueError (not OverflowError) for huge times, one shared validator; per-example outcomes; frame_at and frame_windows; frame_index on selected copies; docs on position-based count, elapsed seconds, multi-transition sequences, and mapping the model's uncertain option and Python's gate to the review labels.

Merged origin/main (7fa806e) with a signed merge commit; no conflicts (#318's docs/evaluation.md edits merged cleanly alongside this section).

Review round 2 resolutions (head 2cef5c7)

• earliest_transition accepts None for an undetermined pair (and NumPy booleans). An undetermined pair before the first True, or with no True, returns {"review": True, "first_undetermined_index": i, ...} with no time, never a later transition. Confirmed results now carry "review": False. Docs say to score it with an uncertain_labels label. Tested.
• frame_windows appends a final window aligned to end when the last stride-spaced window stops short. Every frame lies in some window when stride_seconds <= width_seconds; with a larger stride the windows are deliberately sparse and frames between them are in no window (documented with an example, tested). Tested with frames every 0.5 s to 8 s, width 3, stride 2 (windows start 0, 2, 4, 5) and with a small case.
• docs/vision.md states the actual tie guarantee (up to two frames within the slack around a target); "can only be late" now says a correct detection; docs say to drop frame_index too; select_frames validates a pre-existing frame_index; uncertain_labels=None (or a non-collection) raises ValueError.

Recipe checklist

Skipped (foundation work).

How it was verified

Final head 42dc36a (signed merge of origin/main 2601074 into a9c7922, after two fix rounds), Linux, fresh Python 3.14 venv:

• ruff check .: All checks passed. ruff format --check .: 399 files already formatted.
• pytest: 5547 passed, 49 skipped, 50 deselected, 31 xfailed, 0 failed.
• python tools/check_hygiene.py: passed, 608 files. python tools/check_recipe_assets.py: passed, 461 files. python tools/render_catalog.py --check: exit 0. python tools/notebook_ci.py fixtures: 51 recipe folders, 0 failed.
• No notebook is affected (no recipe imports the new code). tests/test_docs_examples.py executes the docs/evaluation.md blocks and the docs/vision.md frame walkthrough.

No live call, no key; nothing here claims anything about Jev.

🤖 Generated with Claude Code

[CompleteDotTech]

Orchestrator adjudication: the helpers live in a new src/jev_cookbook/frames.py rather than vision.py on the orchestrator's instruction (PR #321 is changing vision.py in parallel). That placement satisfies #309's intent; Closes #309 stands. Reviewers may still judge whether a re-export from vision is wanted. Note this PR and #318 both edit docs/evaluation.md; whichever merges second integrates current main and is re-reviewed.

[CompleteDotTech]

Opus reviewer (independent review 1 of 2). Resolved model: Claude Opus 5.5 (claude-opus-5-5).
Reviewed head 8ff6588c724175ed645404d39440b0d159512275 against base 9a7f152202f13a1b1e4fc004d978390fc7c3232f. I wrote this without reading the other review.

The sampling helpers are careful work. The window, count and float-drift rules hold under everything I tried except one constructible case (B2). Error handling is consistent: empty input and empty windows raise. The denominators in transition_metrics (resolved vs all, with uncertain as review) meet the multiframe row of docs/multimodal-track.md "What an evaluation must show": accuracy against labelled sequences, with uncertain outcomes and both denominators stated. Two things should change before recipes 85–92, 95 and 99 copy this. The first sets what every README will quote.

Blocking

B1. The timing error includes examples where the predicted label is wrong (src/jev_cookbook/evaluation.py:1999-2000, documented at docs/evaluation.md:666-667 "whatever the labels", pinned by tests/test_evaluation.py:1572).
errors.append(p_time - g_time) runs before the p_label != g_label check. A prediction of the wrong transition at the right second therefore counts as a perfect timing result:

transition_metrics([("opened", 2.0)], [("closed", 2.0)], tolerance_seconds=0.5)
-> exact_correct=0, n_timed=1, mean_absolute_error_seconds=0.0

The issue asks for "earliest-transition error in seconds". That means how far off the model was on the transition. The time attached to a different event is not that. In the PR's own hand-computed test, the wrong-label row (ex4, error 0.0) pulls the MAE down from 0.2 s to 0.133 s. A README that quotes mean_absolute_error_seconds beside n_timed would show timing quality better than the model's. This number appears in every multiframe README, and redefining it after recipes ship would silently change it, so it has to be settled now.

Change: take the error only over resolved examples where the labels match and both times are given. n_timed should count exactly those, and the docstring, docs/evaluation.md and the hand-computed test should say so (ex4 then drops out: n_timed == 2, MAE 0.2, signed 0.2). If you also want an all-labels timing view, add it as a separately named pair of fields. Keep the headline name for the label-matched number. n_missed_time / n_spurious_time can stay label-independent, because they describe the shape of the answer, but say so in the docs.

B2. The documented tie rule fails when two frames lie within the slack of each other (src/jev_cookbook/frames.py:130-139; the claim is at docs/vision.md:255-256, "cannot flip a boundary or a tie").
The cursor advances to the last frame at or before target + TIME_EPSILON and only compares it with the frame after it, never with the one before. When two frames are less than TIME_EPSILON apart around a target, the later one wins:

sample_frames([0.0, 1.0, 1.0 + 5e-10, 2.0], every_seconds=1)  -> [0, 2, 3]

Frame 1 sits exactly on the target. Frame 2 is 5e-10 s away, which the slack treats as a tie, and "a tie goes to the earlier frame" says index 1. _ordered accepts these timestamps as strictly increasing. Real elapsed-time data won't produce this, but the PR asks reviewers to attack the tie rule, and the docs make an unconditional promise. Fix either way and add the case to tests/test_frames.py. (a) In _ordered, reject consecutive timestamps closer than 2 * TIME_EPSILON, saying they are indistinguishable under the slack. That is consistent with "strictly increasing" under the slack and the simplest fix. (b) After the cursor loop, step back while the previous eligible frame is within TIME_EPSILON of the best distance.

Suggestions (non-blocking)

1. Accept a 2-element list, not only a tuple (evaluation.py:1935). Gold comes from JSON fixtures, so json.loads('[["opened", 1.0]]') now raises gold[0] must be a (label, seconds_or_None) tuple. Every recipe would need tuple(x). Accepting any non-string Sequence of length 2 removes that step.
2. uncertain_label takes one value. Rank 90 asks for "unknown or inconsistent outcomes for review", which is two review labels. Consider uncertain_labels: Collection (default {"uncertain"}), or document that recipes map every review outcome to one label first.
3. The error types differ between the two modules. transition_metrics raises OverflowError for a time of 10**400 (evaluation.py:1942), but the docs promise ValueError, and frames._seconds already catches this. frames accepts Fraction and evaluation rejects it. Use one validator in both.
4. Return per-example outcomes (or provide a companion function), such as exact / within / wrong / review per index. Recipes will want to show which sequences went to review or missed the window, not only the counts.
5. count is position-based, and the docs should state the consequence. With uneven gaps it is not uniform in time: sample_frames([0, .1, .2, .3, 10], count=3) -> [0, 2, 4] keeps 0.2 s, not anything near 5 s. One sentence in docs/vision.md saying "use every_seconds when even spacing in time matters" would prevent a misreading.
6. The slack is absolute. TIME_EPSILON is 1e-9 s, which is below the float spacing of epoch-scale timestamps (about 2.4e-7 at 1.7e9). The docs say "elapsed seconds", which is right. Add "since the start of the sequence" so nobody passes Unix times. My epoch-scale probes still sampled correctly, so this is wording only.
7. Module path. jev_cookbook.frames instead of vision.* is a reasonable deviation: no Pillow import, and no conflict with Foundation: enforce the Images API's 280 px minimum, hash image bytes in replay keys, bound tiles (#306) #321. Pick one spelling and keep it. A later vision re-export would give recipes two import paths for the same interface. Record the change on Foundation: no deterministic helpers or evaluation for ordered frames (sampling, intervals, relative times, windows, transitions) #309 so the acceptance criteria match what shipped.

Checks I ran (Windows 11, Python 3.14, fresh venv, pip install -e ".[dev]", no TYPESAFE_API_KEY)

• git diff --raw -M origin/main...HEAD: 7 paths. All are 100644: 5 modified, 2 added (src/jev_cookbook/frames.py, tests/test_frames.py). No symlink, submodule, mode change or type change, and all are within foundation scope.
• ruff check .: All checks passed. ruff format --check . (ruff 0.17.1): 397 files already formatted.
• python tools/render_catalog.py --check: rc 0. python tools/check_hygiene.py: passed, 606 files. python tools/notebook_ci.py fixtures: 51 folders, 0 failed.
• pytest: 5038 passed, 59 skipped, 50 deselected, 31 xfailed, 2 failed. The 2 failures are the known Windows-only tests/test_notebook_ci_sandbox.py ones (signal.SIGKILL, Foundation: two notebook-CI sandbox tests use signal.SIGKILL and fail on Windows #302), and this PR does not touch that code. tests/test_frames.py, tests/test_evaluation.py and tests/test_docs_examples.py on their own: 295 passed, 10 skipped (the SIGALRM docs-block tests).
• docs/evaluation.md block 8 by hand (test_block_executes skips on Windows for lack of SIGALRM): extracted it with the same fenced-block regex and ran it in a fresh namespace. All four assertions pass, in 0.15 s. It also ran in CI on Linux (Tests py3.10 / py3.14 green), where the test is not skipped.
• docs/vision.md new blocks by hand (no test executes them): I ran both with 13 synthetic rows at 0.5 s spacing. They gave chosen [0, 4, 8, 12], gaps [2.0, 2.0, 2.0], window {start 2.0, end 4.0, duration 2.0}, seconds_ago [6, 4, 2, 0], and the fields dict builds as shown.
• Edge probes, beyond the PR's tests: tie at exactly 2.5 → earlier. Near-ties with drift either way (0.1*3 between 0.2 and 0.4; a drifted frame at 0.1+0.2) → stable. 0.1 grids at offsets 0, 36000 and 1.7e9 with every_seconds 0.3 and 0.25 → identical selections. start between frames → nearest eligible frame, as documented. start = end + 5e-10 → [3]. An empty window raises for no-rule, count and every_seconds alike. end before the first frame raises (the message says "start must not be after end", which is slightly confusing because start was defaulted). Too-fine every_seconds (1e-7 over 1 s) → the 10M target cap raises. count with round-half-down, checked against the integer formula: correct, and exact-count distinctness holds for n < 30 (the PR's parametrised test). relative_frames with now 1e-10 s before the last frame clamps to 0.0. Generators are accepted by select_frames, window_timestamps and relative_frames.
• transition_metrics probes: uncertain with a time → review, and not counted in n_timed. Gold ("opened", None) vs the same → exact, with no consistency check on labels that "need" a time (acceptable). A 0.1+0.2 gap against tolerance_seconds=0.3 → within, as the inclusive slack intends. np.str_("uncertain") → review.
• Mutation evidence for the stated rules, run one at a time on a copy of the package (verified to be the module imported) against tests/test_frames.py / tests/test_evaluation.py -k transition. All 9 were killed: tie goes to the later frame (3 failures); no slack on the target count (1); no window-edge slack (1); count half rounds up (2); an empty window returns [] instead of raising (1); no slack in exact (1); exclusive tolerance (3); resolved denominator replaced by all (2); uncertain falls through into scoring (2). The rules the PR states are pinned by tests, except the B2 case, which no test covers.
• gh pr checks 323: 60 of 60 pass (Lint, Tests py3.10 and py3.14, Catalog, Hygiene, Fixtures, Scope, all Notebook jobs).
• No live call made.

Acceptance criteria (#309)

• sample_frames(...) is deterministic and its tie-breaking is documented. Empty, single-frame, exact-boundary and float-drift cases are tested: met in substance. It ships at jev_cookbook.frames.sample_frames, not vision.sample_frames (suggestion 7). B2 is a gap in the documented tie guarantee.
• frame_intervals and relative_frames (adds seconds_ago, order unchanged) are tested: met (same module-path note).
• window_timestamps returns start, end and duration, with tests: met.
• transition_metrics reports exact and within-tolerance accuracy (met), an explicit uncertain/review count with stated denominators (met: n_uncertain, uncertain_rate, *_accuracy over n_resolved and *_accuracy_all over n_examples), and an earliest-transition error in seconds (not met as specified, B1). It is documented in docs/evaluation.md and hand-tested: met.
• docs/vision.md covers sampling, no video decoding, and recording the selection in fields: met.
• No new dependency: met (frames.py is stdlib-only, and evaluation.py uses numpy, which it already depended on).

OPUS-REVIEW: CHANGES-NEEDED 8ff6588

[CompleteDotTech]

Opus reviewer (independent review 2 of 2). Resolved model: Claude Opus 5.5 (claude-opus-5-5).

Reviewed head 8ff6588c724175ed645404d39440b0d159512275 against base 9a7f152202f13a1b1e4fc004d978390fc7c3232f. I did not read the other reviewer's comment before writing this.

Summary

The arithmetic is careful: validation matches load_frames, the count rounding formula is correct (it is ceil(q - 1/2) in integers and yields distinct indices whenever count < n), every_seconds multiplies rather than accumulates, the float slack is tested, and the uncertain denominators are stated both ways. The problem is fit with the recipes that have to use it. I sketched ranks 85, 92 and 99 end to end (with probes for 87, 89 and 95): load_frames, then sampling, then one Jev question per window, then transition_metrics, with scripted answers. Sampling and window-to-seconds mapping fit well. Two things in the evaluation contract don't: the number it produces depends on a convention nobody has fixed, and its timing error counts predictions with the wrong label. Every multiframe README will quote that output, so both should be settled before ten recipes copy them.

Blocking

1. No convention says which second a detected transition is, so transition_metrics scores a choice the builder makes without knowing it. src/jev_cookbook/evaluation.py:1948 and docs/evaluation.md:635-668

Jev's Choice and Noul answers carry no time. In every one of these recipes Python has to localize the transition: it asks one question per adjacent sampled pair ("did the door state change between these frames?", or for 99 "is the defect visible in the later frame?") and takes the earliest pair that says yes. window_timestamps(sampled, i, i + 1) then gives an interval, [t_i, t_{i+1}], but transition_metrics needs a single number, and neither module or doc says which one to use. Sketch for rank 85: frames at about 0.5 s, sampled every_seconds=1.0, gold ("opened", 2.3) labelled at full rate, detected pair [2.01, 3.0], tolerance_seconds=0.5:

predicted second taken as	within tolerance	MAE
end (first frame showing the new state)	0	0.70
midpoint	1	0.21
start (last frame showing the old state)	1	0.29

So the same model answers can give 0 % or 100 % tolerance accuracy depending on a choice each builder will make independently. The gold labels have the same problem: does a labeller mark the first full-rate frame that shows the new state, or the moment of change? The issue also requires that sampling be "stated". When tolerance_seconds is smaller than the sampled interval, the metric measures the sampling grid more than the model, and nothing warns about it.

Change: fix one convention in code and docs. I'd suggest "a transition's time is the timestamp_seconds of the first frame in which the new state is visible", applied to gold (on the frames the labeller saw) and to predictions (on the sampled frames). Provide it as a helper, such as transition_time(state_frames, first_new_index) or an earliest_transition(state_frames, flags) that returns the time of the first frame flagged true, so builders don't write it themselves. In docs/evaluation.md, say that tolerance_seconds should be at least the largest sampled interval (max(frame_intervals(sampled))), or else report that interval beside the metric. Optionally, TransitionMetrics could carry a max_sampling_interval_seconds so the README quotes it. Add a hand-computed test that pins the convention.

2. The earliest-transition error includes predictions with the wrong label. src/jev_cookbook/evaluation.py:1993-2000 and docs/evaluation.md:666-667 ("whatever the labels")

transition_metrics([("opened", 2.0)], [("closed", 2.0)], tolerance_seconds=0.5) returns exact_correct=0, n_timed=1 and mean_absolute_error_seconds=0.0. For rank 85 (opened/closed) and 86 (appeared/disappeared), a prediction in the wrong direction at the right instant then counts as perfect timing. A README line such as "mean earliest-transition error 0.2 s (n_timed = 40)" would mix in examples where the model saw the wrong event. The behaviour is tested on purpose (tests/test_evaluation.py, "wrong label, time error 0.0"), so this is a design objection, not an oversight. Still, it is the field the issue names ("earliest-transition error in seconds"), and it will be quoted.

Change: take the timing error only over resolved examples whose label matches gold. If the all-labels view is wanted too, report it as separate fields, for example n_timed / mean_* restricted to matching labels and n_timed_any_label / mean_*_any_label for the current view. Update the docstring, the docs paragraph and the hand-computed test.

Suggestions (non-blocking, but each removes hand-rolled timestamp arithmetic a recipe would otherwise copy)

• No frame-at-time lookup (ranks 95, 99, and 88's action record). Rank 99 associates "textual test logs" with screenshots, and rank 95 compares frames with "recorded agent state". Both need "the frame at or before time t" (or nearest to t) for a log or state timestamp. Today that takes a hand-written bisect.bisect_right(times, t) - 1. sample_frames(times, start=t, end=t) raises unless a frame sits exactly at t. Consider frame_at(timestamps, t, *, side="before" | "after" | "nearest") with the same tie rule and slack. If it isn't added here, open a follow-up issue before 95 or 99 is opened.
• No sliding-window enumeration (ranks 89 and 92). Rank 92's candidate windows needed a hand-written lo = k * stride loop and a try/except ValueError around sample_frames(..., start=lo, end=lo + width), because a window inside a gap raises. That's right for a single call but awkward for enumeration. A frame_windows(timestamps, *, width_seconds, stride_seconds) that returns index lists, and skips or reports empty windows explicitly, would keep every builder from re-deciding the float stepping and the empty-window policy.
• Index spaces are easy to mix silently. sample_frames returns indices into the full list. select_frames returns copies that lose them, and window_timestamps interprets indices against whatever list it is given. In the rank 99 sketch, sampled-pair indices (5, 6) passed to the full list gave 15.0-18.0 s instead of 30.0-36.0 s, with no error. Consider having select_frames add a frame_index (the original position) to each copy, the way relative_frames adds seconds_ago, and show in docs/vision.md:239 and the "Record what was selected" block which list the indices refer to.
• Multi-transition sequences (rank 87). transition_metrics scores one (label, earliest time) per sequence, so a phase history (combat, then menu, then map) has to be split into one example per transition, or scored per window with plain accuracy. One sentence in docs/evaluation.md saying which to do would stop builders from inventing a sequence-alignment metric.
• Two review sources, one uncertain_label. Rank 85 has an uncertain Choice option and also, per CONTRIBUTING section 4, a Python confidence gate that sends low-confidence answers to review. uncertain_label accepts a single value, so the recipe has to map both to it first. That's fine, but say so in the docs.

Fit verdict per recipe (from the sketch)

• 85 / 86 (door, arrival/departure): sampling, relative_frames and window_timestamps fit. The time fed to the metric is hand-rolled and convention-dependent (blocking 1), and wrong-direction answers distort the timing error (blocking 2).
• 87 (game phases): sampling fits. Multi-transition scoring needs a documented decomposition.
• 89 (conveyor jam): the duration of a halted run is window_timestamps(...)["duration_seconds"], which fits. Note that it measures first to last jam frame and is short by up to one interval, which is the same convention question as blocking 1.
• 92 (highlight windows): mapping a window back to seconds fits well. Enumerating windows is hand-rolled.
• 95 / 99 (overlay vs agent state, earliest defect transition with logs): aligning a log or state timestamp to a frame is hand-rolled with bisect. Earliest-transition localization is hand-rolled and convention-dependent.

Overall the interface is sound for picking frames and labelling them, but as it stands it will push builders to hand-roll the timestamp arithmetic that decides the reported number: transition time, frame-at-time lookup and window enumeration. Blocking 1 and 2 are the ones that change what READMEs report.

Checks run (fresh venv, Windows, pip install -e ".[dev]", head 8ff6588)

• git diff --raw -M origin/main...HEAD: 7 paths (2 added, 5 modified), all 100644. No symlink, submodule, mode or type change, and nothing outside src/, docs/, tests/.
• ruff check .: All checks passed.
• ruff format --check .: 397 files already formatted.
• pytest tests/: 3276 passed, 59 skipped, 50 deselected (the default catalog_audit deselection), 31 xfailed, 2 failed. pytest recipes/: 1762 passed. The 2 failures are listed in the next item.
• tests/test_notebook_ci_sandbox.py: the 2 known Windows-only failures (signal.SIGKILL, Foundation: two notebook-CI sandbox tests use signal.SIGKILL and fail on Windows #302), not touched by this PR. tests/test_frames.py, tests/test_evaluation.py and tests/test_docs_examples.py pass.
• python tools/render_catalog.py --check: rc 0.
• python tools/check_hygiene.py: passed, 606 files.
• python tools/notebook_ci.py fixtures: 51 recipe folders, 0 failed.
• Notebook execution: no recipe imports jev_cookbook.frames or transition_metrics (grep of recipes/), so no notebook is affected.
• End-to-end sketch script (ranks 85, 92 and 99, plus 87/89/95 probes, scripted answers, no live call). The figures above come from it.
• gh pr checks 323: CI was still running when I reviewed (notebook jobs pending). The merge gate needs every check green on this head regardless.
• No live call, no key looked for.

Acceptance criteria (#309)

• sample_frames deterministic, with the tie rule documented and edge tests for empty, single, exact boundaries and float drift: met. It lives in jev_cookbook.frames rather than vision, a deviation that is explained and documented, and I accept it.
• frame_intervals and relative_frames (adds seconds_ago, keeps order) are tested: met.
• window_timestamps returns start, end and duration, with tests: met.
• transition_metrics reports exact and within-tolerance accuracy and an uncertain count with a stated denominator: met. Earliest-transition error in seconds: not met as specified. It is undefined without a transition-time convention (blocking 1) and is contaminated by wrong-label predictions (blocking 2). Documented and hand-tested: met.
• docs/vision.md covers sampling, no video decoding, and recording the selected timestamps in fields: met.
• No new dependency: met (frames.py is pure Python).

OPUS-REVIEW: CHANGES-NEEDED 8ff6588

[CompleteDotTech]

Opus reviewer (independent review 1 of 2, re-review). Resolved model: Claude Opus 5.5 (claude-opus-5-5).
Reviewed head 2cef5c7e53d2f766c15c64a6b07da30f14df312f against base 7fa806ea2e588ff3db24063d05abb002c3eee3c5. My previous review was of 8ff6588c (comment 6303367867). I wrote this without reading the other reviewer's comments.

What I reviewed: the fix commit 2cef5c7, the merge 0789311, and the full diff against origin/main. The merge is a clean auto-merge. git merge-tree --write-tree 8ff6588 7fa806e gives 4bd5a54b…, which is identical to the tree of 0789311. Both new commits are signed (G). Against origin/main, docs/evaluation.md changes by additions only (the Transitions section and the TransitionMetrics row), so the content main brought in from #318/#319 is intact. src/jev_cookbook/evaluation.py removes only the old collections.abc import line.

My round-1 findings

B1 (timing error included wrong-label examples): resolved. evaluation.py now appends to the headline errors list only when label_ok. The old all-labels view is still available under separately named fields (n_timed_any_label, mean_*_any_label_seconds). The docstring, docs/evaluation.md ("Timing error") and the result-type table describe this. Evidence:

• transition_metrics([("opened", 2.0)], [("closed", 2.0)], tolerance_seconds=0.5) now gives n_timed=0, MAE nan, n_timed_any_label=1.
• The hand-computed test now expects n_timed == 2 and MAE 0.2, which matches my round-1 arithmetic.
• A new test covers the wrong-label case directly.
• Mutation: making the headline error ignore the label is killed (2 failures).

B2 (tie rule broken when two frames lie within the slack of each other): resolved for the case I raised. The selection now looks at cursor-1..cursor+1 and takes the earliest frame within TIME_EPSILON of the closest one. Evidence:

• sample_frames([0, 1, 1+5e-10, 2], every_seconds=1) now gives [0, 1, 3], and there is a new test with that case and its mirror.
• Two mutations are killed: tie goes to the later frame (4 failures), and no look-back (1 failure).
• 20,000 random every_seconds trials (seed 309), with near-duplicate frames injected at 3e-10, 7e-10 and 1.5e-9 s, all match a brute-force reference ("nearest eligible frame, earliest within slack of the minimum"). Every output is ascending and has no repeats.

There is one residual case, which is suggestion 1 below.

My round-1 suggestions:

1. 2-element lists are now accepted. A JSON-loaded pair scores as ('within',). Strings and bytes are still rejected as pairs.
2. Several review labels are now accepted through uncertain_labels. A bare string is treated as one label, and a mutation that splits it into characters is killed.
3. Validation is now consistent: evaluation reuses frames._seconds. 10**400 raises ValueError for a time, the tolerance and the interval, and there are tests for each.
4. Per-example outcomes and timing_errors_seconds were added.
5. The docs now say that count spreads frames by position, with the [0, .1, .2, .3, 10] example and a test.
6. Both docs now say "elapsed seconds since the start of the sequence (never Unix time)".
7. The docs now state that jev_cookbook.frames is the one import path.

New in this round (from the other review), checked

• The transition_time / earliest_transition convention ("first frame in which the new state is visible"). It is consistent across frames.py, both docs and the end-to-end test.
  • The claim that sampled detection is late by less than the previous sampled gap holds for a correct detection. If the transition happens at g with s_{k-1} < g <= s_k, the reported time is s_k, and s_k - g < s_k - s_{k-1}.
  • Mutations are killed for reporting the time at the pair start and for allowing first_new_index=0.
• max_sampling_interval_seconds / tolerance_below_sampling_interval. The flag is strict with slack: a gap equal to the tolerance is not flagged, which is right given the strict bound above. A mutation that makes the comparison inclusive is killed.
• frame_at. before, after and nearest behave as documented. A tie goes to the earlier frame (a mutation is killed), and it raises when no frame qualifies.
• frame_windows. Windows that fall in a gap between frames are kept with indices == []. Float drift is tested.
• select_frames adds frame_index. Selecting from a selection preserves the original positions, and a mutation that drops that is killed.
• The docs/vision.md "Index spaces" paragraph is clear and matches the code.

Blocking

None.

Suggestions (non-blocking)

1. Clusters of three or more frames within the slack can still break the tie rule (frames.py, the near = range(cursor - 1, cursor + 2) window). sample_frames([0, 1-4e-10, 1, 1+4e-10, 2], every_seconds=1) gives [0, 2, 4]; the earliest frame within slack of the closest is index 1. docs/vision.md now says the tie rule holds "including frames closer together than the slack", which overclaims for this case. Real elapsed-time data will never contain it. The simplest fix is to reject consecutive timestamps closer than 2 * TIME_EPSILON in _ordered, which removes the class entirely. Otherwise, walk back while the previous frame is within slack.
2. earliest_transition rejects np.bool_. [np.bool_(True)] raises changed must contain only True/False, so a recipe that thresholds a NumPy array (probs >= 0.5) gets an error. It fails loudly, not silently. Still, accepting NumPy booleans, or documenting .tolist() in the docstring, would remove a predictable stumble.
3. frame_windows can leave trailing frames in no window. With frames [0..4], width 2 and stride 1.5, the windows are [0, 2] and [1.5, 3.5], and the frame at 4.0 is in neither. The docstring does say "only windows that fit", but ranks 89 and 92 would silently skip the end of the sequence. Say so explicitly ("frames after the last full window are not covered; choose stride so that (end - start - width) is a multiple of it, or add a final window").
4. "its time can only be late" (docs/evaluation.md, the paragraph after the convention). This holds for a correct detection only. A false early flag is early: ("opened", 1.0) against gold 3.0 gives a signed error of -2.0. Suggested wording: "a correct detection on sampled frames can only be late".
5. frame_index ends up in the state sent to the model. select_frames rows are what recipes pass as state, so Jev now also sees frame_index. That is harmless for most questions, but say it next to the existing "drop timestamp_seconds if the model should see only seconds_ago" sentence. select_frames also copies an existing frame_index without validating it ("x" passes through). A one-line integer check would match the rest of the module.

Checks I ran (Windows 11, Python 3.14, fresh venv reinstalled at this head, no TYPESAFE_API_KEY)

• git diff --raw -M origin/main...HEAD: 7 paths, all 100644, the same set as round 1: 5 modified, and frames.py and test_frames.py added. No symlink, submodule, mode change or type change.
• ruff check .: All checks passed. ruff format --check . (ruff 0.17.1): 397 files already formatted.
• python tools/render_catalog.py --check: rc 0. python tools/check_hygiene.py: passed, 606 files. python tools/notebook_ci.py fixtures: 51 folders, 0 failed.
• pytest: 5192 passed, 59 skipped, 50 deselected, 31 xfailed, 3 failed. Two of the failures are the known Windows-only tests/test_notebook_ci_sandbox.py ones (signal.SIGKILL, Foundation: two notebook-CI sandbox tests use signal.SIGKILL and fail on Windows #302). The third, tests/test_notebook_ci_netguard.py::test_notebook_that_clears_the_live_switch_still_cannot_reach_out, asserts a refusal in under 3 s, and it failed while I was running mutation tests in parallel on the same machine. Re-run on its own, it passed (1 passed, 22 s). It is green in CI, and this PR touches no netguard code, so it is not a finding.
• docs/evaluation.md block 8 by hand (test_block_executes is SIGALRM-skipped on Windows): it is still the only transitions block (9 blocks total, which matches the test's count assertion). Run in a fresh namespace, all six assertions pass, in 0.14 s.
• docs/vision.md blocks 8 and 9 by hand (no test executes them): run on 13 synthetic rows at 0.5 s spacing, they pass. fields["frames"] carries frame_index [0, 4, 8, 12].
• Probes: the B1/B2 cases above; 20,000 random sampling trials against a reference; the two frame_at tie and slack cases; frame_windows with width equal to the range; JSON pairs; a set passed as uncertain_labels; an interval 5e-10 s over the tolerance (not flagged, as intended); TransitionMetrics is hashable (frozen, with tuple fields).
• Mutations (one at a time on a copy of the package, after confirming the copy was the module imported): all 9 killed. These were: B2 tie goes later; B2 no look-back; frame_index not preserved; transition time at the pair start; frame_at nearest-tie goes later; transition_time allows index 0; B1 timing ignores the label; the interval flag made inclusive; a string review label split into characters.
• gh pr checks 323 on 2cef5c7e: 60 of 60 pass (Lint, Tests py3.10 and py3.14, Catalog, Hygiene, Fixtures, Scope, all Notebook jobs). On Linux CI, test_block_executes runs the evaluation.md block too.
• No live call made.

Acceptance criteria (#309)

• sample_frames is deterministic, its tie rule is documented, and empty, single-frame, boundary and drift cases are tested: met. It lives at jev_cookbook.frames, now documented as the one import path. Suggestion 1 is a residual edge outside realistic data.
• frame_intervals / relative_frames: met.
• window_timestamps: met.
• transition_metrics, exact and within-tolerance accuracy: met. Review count with stated denominators: met. Earliest-transition error in seconds: met, now label-matched (B1). Documented in docs/evaluation.md with hand-computed tests: met.
• docs/vision.md covers sampling, no video decoding, and recording the selection in fields: met.
• No new dependency: met. frames.py uses the standard library only, and evaluation imports frames, with no cycle.

OPUS-REVIEW: APPROVE 2cef5c7

[CompleteDotTech]

Opus reviewer (independent review 2 of 2, re-review). Resolved model: Claude Opus 5.5 (claude-opus-5-5).

Reviewed head 2cef5c7e53d2f766c15c64a6b07da30f14df312f against base 7fa806ea2e588ff3db24063d05abb002c3eee3c5. That covers the fix commit 2cef5c7, the signed merge 0789311 of origin/main 7fa806e, and the full diff against origin/main. I did not read the other reviewer's comments before writing this.

My round-1 findings (comment 6303821403)

Blocking 1 (no convention for a transition's time): resolved. The convention is now "the timestamp_seconds of the first frame in which the new state is visible". It is stated in transition_time and earliest_transition (src/jev_cookbook/frames.py), in the transition_metrics docstring and in docs/evaluation.md ("One convention for the time"), with the explicit instruction not to substitute the pair's start or midpoint. max_sampling_interval_seconds and tolerance_below_sampling_interval record the sampling gap and flag a tolerance smaller than it. The sketch re-run for rank 85 (frames about 0.5 s apart, sampled every 1 s, gold 2.49 s) gives earliest_transition = 3.0 s, gap 1.02 s, outcome within, MAE 0.51 and flag False. A 0.25 s tolerance sets the flag to True. test_transition_convention_end_to_end pins the convention.

Blocking 2 (timing error counted wrong labels): resolved. transition_metrics([("opened", 2.0)], [("closed", 2.0)], tolerance_seconds=0.5) now gives n_timed=0 and a headline MAE of NaN. The old view moves to separately named fields (n_timed_any_label=1, mean_absolute_error_any_label_seconds=0.0). It is tested by test_transition_metrics_wrong_label_at_the_right_second_is_not_good_timing.

Suggestions:

• Frame-at-time lookup: done (frame_at, with before, after and nearest-tie-to-earlier, raising when no frame qualifies). Rank 99's log lines now map without bisect: 14.2 s gives before 4 and after 5; 31.0 s gives nearest 10.
• Sliding windows: done (frame_windows). Empty windows stay visible with indices == [], which removed the try/except. See new blocking 2 below for a gap it leaves.
• Index spaces: done (frame_index on the rows select_frames returns, kept across re-selection, plus an "Index spaces" paragraph in docs/vision.md). In the rank 99 sketch, sampled pair 6 maps back to original frame 12.
• Several transitions per sequence: documented (split into one example per transition, no alignment metric).
• Review labels: uncertain_labels takes a collection, or a single string, which is handled correctly. The docs say to map both the model's option and Python's gate to these labels.

Blocking (new, in helpers added this round)

1. earliest_transition cannot represent an undetermined pair, so the shipped helper pushes builders to coerce "uncertain" to False and report a later transition as the earliest. src/jev_cookbook/frames.py:335-362

changed must be all bool (frames.py:354). In ranks 85, 86 and 99 the per-pair question is a Noul, and CONTRIBUTING section 4 sends a low-confidence or uncertain answer to review. Say the pair before the first True is uncertain. The earliest transition has then not been established, but the only way to call the helper is to pass False there. Sketch (rank 85, flags [False, <uncertain>, True, False]): passing None raises "changed must contain only True/False". Coercing it to False returns transition_seconds=3.0 with no sign that an earlier pair was never resolved. That is the silent wrong answer the convention was meant to prevent, and rank 99's use case ("the earliest supported visible transition") turns on exactly this case. Neither the docstring nor docs/vision.md says what to do.

Change: accept None (or a named review value) for an undetermined pair. If any undetermined pair comes before the first True, or there is no True but some pair is undetermined, return an explicit review result rather than a time or None, for example {"review": True, "first_undetermined_index": i, ...} or a dedicated sentinel. Document how it feeds transition_metrics (as one of uncertain_labels). Add a test where a gated pair before the first True yields review rather than the later pair.

2. frame_windows silently leaves the end of the sequence out of every window when the stride doesn't divide the range. src/jev_cookbook/frames.py:267-310, docs/vision.md:282-285

With frames every 0.5 s from 0.0 to 8.0 and width_seconds=3.0, stride_seconds=2.0, the windows are (0,3), (2,5), (4,7), and frames 7.5 s and 8.0 s fall in no window. The docs say "only windows that fit", but not that frames can then go unscored. For rank 92, a highlight in the last seconds of a clip would never be scored, and nothing in the output says so. The empty-window case was made visible on purpose (indices == []), and this gap is the same kind of thing.

Change (any one): add a final window aligned to end when the last window stops short (documented as possibly overlapping more), or return or offer the uncovered frame indices, or at least state the consequence in the docstring and docs/vision.md and show how to choose stride so that (end - start - width) is a multiple of it. Add a test for the chosen behaviour.

Suggestions

• frame_index now rides along in every row that goes into a Jev state. In the rank 85 sketch, the state keys are frame_index, image, seconds_ago and timestamp_seconds. It is harmless and deterministic, but a builder who wants the model to see only seconds_ago (as docs/vision.md suggests for timestamp_seconds) should be told to drop frame_index as well. One sentence would do it.
• uncertain_labels=None raises a bare TypeError from list(None). A ValueError with a message would match the rest of the module.

Fit after this round (sketch re-run: ranks 85, 92 and 99, plus 87/89/95 probes; script at scratchpad pr323-review2/sketch2.py)

• 85 / 86: sample_frames, select_frames, relative_frames, earliest_transition, frame_intervals and transition_metrics chain together with no hand-written time arithmetic, except for the uncertain pair (new blocking 1).
• 87: the documented per-transition split with uncertain_labels=("unknown", "inconsistent") works (outcomes=('within', 'review')).
• 89 / 92: frame_windows plus window_timestamps fit, empty windows are explicit, and there is no stepping loop. The tail coverage gap remains (new blocking 2).
• 95 / 99: frame_at covers log and agent-state alignment, and frame_index maps sampled pairs back to the original sequence. Earliest-supported localization still depends on new blocking 1.

Builders no longer need to hand-roll timestamp arithmetic for the main path. The two remaining gaps are both in uncertainty and coverage handling, which is where a copied recipe would go wrong silently.

Full diff against origin/main

git diff --raw -M origin/main...HEAD lists 7 paths, all 100644: docs/evaluation.md, docs/vision.md, src/jev_cookbook/evaluation.py, src/jev_cookbook/frames.py (A), tests/test_docs_examples.py, tests/test_evaluation.py and tests/test_frames.py (A). There is no symlink, submodule, mode or type change. The merge brought in nothing beyond main. I re-read all of frames.py and the transitions section of evaluation.py. evaluation now imports TIME_EPSILON and _seconds from .frames; frames imports nothing from the package, so there is no cycle. The rewritten every_seconds tie rule (closest within slack, earliest wins) is correct, and test_tie_rule_holds_when_frames_are_closer_than_the_slack covers it. The docs example in docs/evaluation.md reproduces its asserted outcome tuple and mean signed error of 0.25.

Checks run (fresh venv, Windows, pip install -e ".[dev]", head 2cef5c7)

• ruff check .: All checks passed.
• ruff format --check .: 397 files already formatted.
• pytest tests/: 3431 passed, 59 skipped, 50 deselected (default catalog_audit), 31 xfailed, 2 failed. The 2 failures are the known Windows-only tests/test_notebook_ci_sandbox.py ones (signal.SIGKILL, Foundation: two notebook-CI sandbox tests use signal.SIGKILL and fail on Windows #302), which this PR doesn't touch. The docs/evaluation.md transitions block, which is SIGALRM-skipped on Windows, was executed by hand: ok.
• pytest recipes/: 1762 passed.
• python tools/render_catalog.py --check: rc 0.
• python tools/check_hygiene.py: passed, 606 files.
• python tools/notebook_ci.py fixtures: 51 recipe folders, 0 failed.
• Notebooks: no recipe imports jev_cookbook.frames or transition_metrics, so none is affected.
• gh pr checks 323 at review time: 17 pass, 42 pending, none failed. Every check must be green on this head before merge regardless.
• Sketch re-run as described above, with scripted answers. No live call, no key looked for.

Acceptance criteria (#309)

• sample_frames deterministic, with the tie rule documented and edge tests: met.
• frame_intervals and relative_frames: met.
• window_timestamps: met.
• transition_metrics with exact and within-tolerance accuracy, review count with a stated denominator, and earliest-transition error in seconds, documented and hand-tested: met at the metric level. The helper that produces the earliest time for it mishandles undetermined pairs (blocking 1).
• docs/vision.md covers sampling, no video decoding, and recording the selection in fields: met.
• No new dependency: met.

OPUS-REVIEW: CHANGES-NEEDED 2cef5c7

[CompleteDotTech]

Opus reviewer (independent review 2 of 2, re-review 2). Resolved model: Claude Opus 5.5 (claude-opus-5-5).

Reviewed head 6b18804119d04fa7a1dc114558c84b53cd58cf71 against base 7fa806ea2e588ff3db24063d05abb002c3eee3c5. This covers commit 6b18804 (everything since 2cef5c7) plus the full diff against origin/main. I did not read the other reviewer's comments before writing this.

My round-2 findings (comment 6304297506)

Blocking 1 (earliest_transition could not represent an undetermined pair): resolved. changed now accepts True, False, None and NumPy booleans (src/jev_cookbook/frames.py, earliest_transition). An undetermined pair before the first True, or with no True at all, returns {"review": True, "first_undetermined_index": i, ...} with no time. A found transition carries "review": False. The docstring says never to coerce to False. docs/evaluation.md and docs/vision.md say to pass None and to score a review result with one of uncertain_labels. Re-running the sketch on rank 85's sampled frames:

• [False, None, True, False] gives review: True at pair 1 (1.02 to 2.01 s), not the later 3.0 s transition.
• [False, False, None, False] gives review: True at pair 2.
• [True, None, False, False] gives the transition at 1.02 s. That is correct, because the earlier True is established before the undetermined pair.

Tests: test_earliest_transition_undetermined_pair_is_review_not_a_later_transition (including [None, True, True] giving review at 0) and test_earliest_transition_accepts_numpy_booleans.

Blocking 2 (frame_windows could leave the end uncovered): resolved. When the last stride-spaced window stops short of end by more than the slack, one final window [end - width, end] is appended. Sketch re-run, frames every 0.5 s from 0 to 8 s, width 3, stride 2: the windows are (0,3), (2,5), (4,7), (5,8), and the list of frames in no window is now empty (it was [7.5, 8.0]). Extra probes:

• An end at 7.3 s with frames at whole seconds adds (4.3, 7.3) holding indices [5, 6, 7, 8].
• An exact fit adds no extra window ([0.0, 1.0, 2.0]).
• A float-drift fit (0.30000000000000004) adds none either.

test_frame_windows_cover_the_end_of_the_sequence asserts full coverage. The docstring and docs/vision.md state the behaviour, with the 0/2/4/5 worked example.

Suggestion: drop frame_index from the state. Resolved. docs/vision.md now says to drop frame_index along with timestamp_seconds when the model should see only seconds_ago.

Suggestion: uncertain_labels=None raised a bare TypeError. Resolved. It now raises ValueError: uncertain_labels must be a label or a collection of labels (test_transition_metrics_rejects_missing_or_bad_review_labels).

Other changes since 2cef5c7, reviewed

• select_frames now validates an inherited frame_index as a non-negative integer and normalises it with int() (test_select_frames_validates_an_existing_frame_index). Good.
• transition_time and docs/evaluation.md now say that a correct detection on sampled frames can only be late, and that a false early flag shows as a negative signed error. That is more accurate than before.
• docs/vision.md narrows the every_seconds tie-rule claim to "guaranteed for up to two such frames around a target" for frames within 1e-9 s of each other. That is an honest narrowing, and real elapsed-time data never hits it.

Blocking

None.

Suggestions (non-blocking)

• src/jev_cookbook/frames.py:289 is a 148-character docstring line, and one paragraph line in docs/evaluation.md (the one ending "Do not substitute the pair's start or midpoint: the three give") also runs well past the 100-column style used elsewhere. Ruff does not flag either. Reflow them if you touch the files again.
• earliest_transition now returns a truthy dict for a review as well as for a found transition, so if found: no longer means "a transition was found". The docstring is explicit about the review key. A first recipe that shows if found is None: ... elif found["review"]: ... else: ... would set the pattern well.

Fit (sketch re-run: ranks 85, 92 and 99, plus 87/89/95 probes)

• 85 / 86: sample_frames, select_frames, relative_frames, the per-pair Noul, earliest_transition (with None for gated pairs) and transition_metrics chain together with no hand-written time arithmetic, and undetermined pairs go to review.
• 87: a per-transition split plus uncertain_labels=("unknown", "inconsistent") works.
• 89 / 92: frame_windows covers every frame, empty windows are explicit, and window_timestamps maps each window back to seconds.
• 95 / 99: frame_at aligns log and agent-state times, frame_index maps sampled pairs back to the original frames (pair 6 to frame 12), and earliest-supported localization respects undetermined pairs.

Builders no longer need to hand-roll timestamp arithmetic on any path I sketched.

Checks run (fresh venv, Windows, pip install -e ".[dev]", head 6b18804)

• git diff --raw -M origin/main...HEAD: 7 paths, all 100644 (frames.py and test_frames.py added; docs/evaluation.md, docs/vision.md, evaluation.py, test_docs_examples.py and test_evaluation.py modified). No symlink, submodule, mode or type change.
• ruff check .: All checks passed.
• ruff format --check .: 397 files already formatted.
• pytest tests/: 3435 passed, 59 skipped, 50 deselected (default catalog_audit), 31 xfailed, 3 failed.
  • Two of the failures are the known Windows-only tests/test_notebook_ci_sandbox.py ones (signal.SIGKILL, Foundation: two notebook-CI sandbox tests use signal.SIGKILL and fail on Windows #302).
  • The third is tests/test_new_recipe.py::test_a_failed_start_leaves_no_exit_time_noise. It failed because a zmq Proactor-loop RuntimeWarning appeared on the subprocess's stderr under load. It passes when its file is re-run alone (84 passed), it passed in my full run on 2cef5c7, and this PR doesn't touch it or anything it imports. I read it as a Windows timing flake, not a finding.
  • An earlier full run on this head stalled at about 45% with no output for 30 minutes. I stopped it, and the verbose re-run above, with a 300 s faulthandler, completed without a hang.
• pytest recipes/: 1762 passed.
• The docs/evaluation.md transitions block, which is SIGALRM-skipped on Windows, was executed by hand: ok.
• python tools/render_catalog.py --check: rc 0.
• python tools/check_hygiene.py: passed, 606 files.
• python tools/notebook_ci.py fixtures: 51 recipe folders, 0 failed.
• Notebooks: no recipe imports jev_cookbook.frames or transition_metrics, so none is affected.
• gh pr checks 323 at review time: 26 pass, 33 pending, none failed. This approval is of the code; the merge gate still needs every check green on this exact head.
• No live call, no key looked for.

Acceptance criteria (#309)

• sample_frames deterministic, with the tie rule documented and edge tests (empty, single, exact boundaries, float drift): met. It lives in jev_cookbook.frames rather than vision, a documented deviation that I accept.
• frame_intervals and relative_frames (adds seconds_ago, keeps order), tested: met.
• window_timestamps (start, end, duration), tested: met.
• transition_metrics with exact and within-tolerance accuracy, a review count with stated denominators, and earliest-transition error in seconds under one documented convention, documented and hand-tested: met.
• docs/vision.md covers sampling, no video decoding, and recording the selection in fields: met.
• No new dependency (Pillow only; frames.py is pure Python): met.

OPUS-REVIEW: APPROVE 6b18804

[CompleteDotTech]

PR #323 — Opus review (independent review 2 of 2, fresh reviewer)

• Agent ID: agent-a0f4a227a4ff1772f (my worktree: .claude/worktrees/agent-a0f4a227a4ff1772f)
• Resolved model ID: claude-opus-5[1m] (Opus 5, 1M context), as stated in my system prompt
• PR: Jev-Engineering/cookbook Foundation: deterministic helpers and evaluation for ordered frames (#309) #323, branch foundation/frame-helpers
• Head reviewed: 4f338f92fb4bced5a06b6cbed76f69597bdc40f8 (signed merge of origin/main into the branch)
• Base used: origin/main = c3167129e3d272588cd2d0febe73d201b1b4011a
• Merge parents: 6b18804119d04fa7a1dc114558c84b53cd58cf71 (previous head) + c316712 (main)
• Issue: Foundation: no deterministic helpers or evaluation for ordered frames (sampling, intervals, relative times, windows, transitions) #309

My brief's focus for this review: docs/vision.md and docs/evaluation.md accuracy, consistency with
the image rules now on main (#321/#322), fit with docs/multimodal-track.md "What an evaluation must
show", the PR body against the diff, scope, no new dependency, and the acceptance criteria one by one.
I formed the view below from the diff, the issue and the code; the prior-review section was written
afterwards.

The merge itself

• git show --cc HEAD produces no combined-diff hunks: the merge commit contributes no content of
  its own.
• Independently reproduced: git merge-tree --write-tree 6b18804 c316712 →
  eba6c36107f832e657b435b1c4cb16530d9561c4, byte-identical to git rev-parse HEAD^{tree}
  (git diff eba6c36 HEAD empty). The merge is exactly what git produces unaided, including in
  docs/vision.md, the one file both sides touch (main's Foundation: enforce the Images API's 280 px minimum, hash image bytes in replay keys, bound tiles (#306) #321 edits sit in the image-limits, box_iou
  and tiles sections; this PR appends a new section after "Explicitly timestamped still frames" — no
  overlap, no duplication, no contradiction).
• git log --format='%G?' -1 → G (signed). The message ends with the required
  Co-Authored-By: Claude Sonnet 5.5. c3167129 is an ancestor of HEAD.

Nothing unintended came in with the merge.

Scope and dependencies

git diff --raw -M origin/main...HEAD — 7 paths, every mode 100644, no symlink, submodule, mode or
type change:

:100644 100644 M  docs/evaluation.md
:100644 100644 M  docs/vision.md
:100644 100644 M  src/jev_cookbook/evaluation.py
:000000 100644 A  src/jev_cookbook/frames.py
:100644 100644 M  tests/test_docs_examples.py
:100644 100644 M  tests/test_evaluation.py
:000000 100644 A  tests/test_frames.py

All inside src/, docs/, tests/ — foundation scope; no recipes/, .github/, catalog/ or
notebook. pyproject.toml is untouched, so no new dependency: frames.py is standard library only
(bisect, math, collections.abc, numbers, typing) and evaluation.py keeps numpy, which it
already had. No recipe imports either new name (git grep over recipes/), so no notebook is
affected and none needed executing.

Scope observation (not a defect): #309 asks for four helpers plus one metric; the PR ships ten public
functions plus TIME_EPSILON. frame_timestamps, select_frames, frame_at, frame_windows,
transition_time and earliest_transition are beyond the acceptance criteria, added across the three
earlier review rounds. They are coherent and well tested. I note the growth only because both
must-change items below land on frame_windows, the helper furthest from what #309 asked for.

Must change

M1. frame_windows promises coverage it does not deliver whenever stride_seconds > width_seconds

• src/jev_cookbook/frames.py:289 — "… is appended, so every frame lies in some window."
• src/jev_cookbook/frames.py:308 — starts.append(high - width) # final window aligned to the end: no frame is left out
• docs/vision.md:300-306 — "… one final window aligned to the end ([end - width, end], overlapping
  the previous one) is added, so no frame is left out"
• The PR body repeats it: "frame_windows appends a final window aligned to end when the last
  stride-spaced window stops short, so no frame is left out."

The claim is unconditional and it is false for every configuration with stride_seconds > width_seconds. The end-aligned window closes the tail only; frames between non-overlapping windows
are in no window at all — no error, no warning, nothing in the result that says so. Nothing in the code
constrains stride relative to width. ("overlapping the previous one" is false in the same regime.)

Reproduced on this head (src on PYTHONPATH, stdlib only):

frame_windows([0.0, 1.0, ..., 10.0], width_seconds=1.0, stride_seconds=5.0)
  -> [(0.0, 1.0, [0, 1]), (5.0, 6.0, [5, 6]), (9.0, 10.0, [9, 10])]
  uncovered frame indices: [2, 3, 4, 7, 8]

Sweeping width x stride over a uniform 0–10 s sequence sampled at 0.5 s, 7 of the 20
width x stride configurations leave frames in no window — every one of them a stride > width case (the stride > width cases that do not appear are ones where the 0.5 s grid happens to align):

width=0.5 stride=1.5 -> uncovered [2, 5, 8, 11, 14, 17]
width=0.5 stride=2.0 -> uncovered [2, 3, 6, 7, 10, 11, 14, 15, 18]
width=0.5 stride=5.0 -> uncovered [2..9, 12..18]
width=1.0 stride=2.0 -> uncovered [3, 7, 11, 15]
width=1.0 stride=5.0 -> uncovered [3..9, 13..17]
width=2.0 stride=5.0 -> uncovered [5, 6, 7, 8, 9, 15]
width=3.0 stride=5.0 -> uncovered [7, 8, 9]

This is the same silent failure the earlier round-2 blocking finding was about ("a highlight in the last
seconds of a clip would never be scored, and nothing in the output says so"). The fix closed the tail
case and then stated the guarantee unconditionally, which makes the interior case worse than before
it existed: a rank 89/92 builder who reads "so no frame is left out" and picks a subsampling stride
wider than the window gets unscored frames and has been told that cannot happen. Sixteen recipes
copy this module, and the point of the helper was to stop builders re-deciding the empty/uncovered
policy themselves.

tests/test_frames.py:450-473 (test_frame_windows_cover_the_end_of_the_sequence) asserts full
coverage only for stride <= width (3/2 and 2/1.5). No test covers stride > width with interior
frames, which is why the overclaim is not caught.

Change — pick one, and make the docstring, line 308's comment, docs/vision.md and the PR body agree:

1. Validate it. Raise ValueError when stride_seconds > width_seconds (the only regime where the
   guarantee can fail) and keep the claim as written. Add a test for the rejection. This is the safer
   default for shared tooling a recipe copies, and it costs one line.
2. Or condition the claim. "Every frame lies in some window when stride_seconds <= width_seconds; with a wider stride, frames between windows lie in none." Add a test that pins the
   gap (the width=1, stride=5 case above) so the documented consequence is itself checked.

While editing: frames.py:289 is also the file's only line over 100 characters (148) — the
reviewer-1 suggestion from the last round that was left open. Reflow it.

M2. The docs/vision.md walkthrough block raises ValueError on the frames the document itself defines

docs/vision.md:241-261. The block opens times = frame_timestamps(frames) and never defines
frames. The only frames on the page is the one from the load_frames example 28 lines above
(docs/vision.md:209-221), with timestamps 0.0, 0.75, 1.5. Read that way — the only way a reader can
read it — the block fails:

times   = frame_timestamps(frames)                 -> [0.0, 0.75, 1.5]
chosen  = sample_frames(times, every_seconds=2.0)  -> [0]     (one target, at 0.0)
sampled = select_frames(frames, chosen)            -> 1 row
gaps    = frame_intervals(sampled)                 -> []      (degenerate)
labelled= relative_frames(sampled)                 -> seconds_ago [0.0]
window  = window_timestamps(sampled, 1, 2)         -> ValueError: first_index 1 is outside 0..0

So the module's only end-to-end tour in the docs does not run on the page's own data, and the three
lines before the failure demonstrate nothing (every_seconds=2.0 over a 1.5 s sequence keeps a single
frame; frame_intervals of one frame is []).

Nothing catches it. tests/test_docs_examples.py extracts and executes only docs/evaluation.md (its
DOC constant, line 81); ruff format --check checks Markdown fences for formatting, not correctness;
and docs/development.md lists no other command that would. Both prior reviews ran this block against
their own 13-row synthetic sequence instead of the doc's three frames, which is how it survived three
rounds. The PR body's "the docs/vision.md blocks were executed by hand: ok" is therefore not true of
the document as published.

Change: make the block run on the page. The smallest honest fix is every_seconds=0.5 — verified on
this head against the page's own three frames:

chosen [0, 1, 2]    gaps [0.75, 0.75]    seconds_ago [1.5, 0.75, 0.0]
window {'start_seconds': 0.75, 'end_seconds': 1.5, 'duration_seconds': 0.75}

every line then demonstrating something. Update the "every_seconds": 2.0 in the fields block at
docs/vision.md:332 to match, or give the walkthrough its own longer frames = [...] literal and say
so in a comment. Please also consider extending tests/test_docs_examples.py to docs/vision.md's
frames blocks: the machinery and the per-block PREAMBLE convention already exist, and a shared
interface's one worked example should be executed by a test rather than by hand.

M3. tests/test_docs_examples.py still says there are eight blocks

tests/test_docs_examples.py:31 ("Two of the eight blocks have pre-existing findings") and :43
("Two of the eight blocks fail ruff check verbatim"). The PR added block 8 and correctly bumped
the assertion and its failure message from 8 to 9 (:170-174) but left the prose. That docstring is
what a maintainer reads to understand the allowlist convention, and this PR made it wrong in two
places. Change both to "nine".

Suggestions (non-blocking)

1. The central "Undefined is NaN" table was not extended (docs/evaluation.md:19-33). The text says
   "Each docstring says exactly when. The cases:" and then tabulates them, which reads as the
   complete list. transition_metrics adds NaN cases: exact_accuracy / tolerance_accuracy when
   every example is a review, the four mean_*_seconds when their count is 0, and
   max_sampling_interval_seconds when the caller passes none. They are documented locally
   (:819, :835) so this is completeness, not a contradiction, but two rows in the table would close it.
2. docs/evaluation.md:3-4 says the module "is plain numpy, imports nothing from the answer classes
   or the SDK". It now also imports from a sibling package module:
   src/jev_cookbook/evaluation.py:35 is from .frames import TIME_EPSILON, _seconds. Literally the
   sentence survives (frames is neither the answer classes nor the SDK), and there is no cycle
   (frames imports nothing from the package), but the sentence reads as "no intra-package imports".
   Add the clause. Separately, _seconds is a private name imported across modules; promoting it
   (frames.validate_seconds, or a shared _time helper) would make the dependency honest and let the
   docs name it.
3. docs/vision.md:3 still opens "Import these helpers from jev_cookbook.vision", which is no
   longer true of the whole document. The new section says so explicitly at :233-235, so the risk is
   small, but one clause at the top ("except the ordered-frame arithmetic in jev_cookbook.frames,
   below") removes the contradiction a reader meets first.
4. The fields example at docs/vision.md:323-333 drops the image path. docs/vision.md:57-60
   (from main) tells authors to "store their relative paths and timestamps in the fields of
   fixtures/inputs.jsonl". The new example assigns a whole fields = {...} whose "frames" rows
   carry only timestamp_seconds and frame_index, so a reader following it writes a fixture that
   cannot reload its own frames. Either include the relative asset path per row, or rename the key so
   it reads as an addition rather than a replacement of the loader's own frames vocabulary.
5. Say that the new state keys enter the replay key. select_frames adds frame_index and
   relative_frames adds seconds_ago to the rows a recipe passes as state, and replay_key
   (src/jev_cookbook/backends.py:156-195) hashes the canonical JSON of the state as written. So
   changing the sampling rule changes every replay key and invalidates recorded fixtures. That is the
   right behaviour and it is deterministic, but after Foundation: enforce the Images API's 280 px minimum, hash image bytes in replay keys, bound tiles (#306) #321 made the replay key hash image bytes, one
   sentence tying the new keys to it would save a builder a puzzling ReplayMiss. This is the only
   interaction I found between the new section and Foundation: enforce the Images API's 280 px minimum, hash image bytes in replay keys, bound tiles (#306) #321/Foundation: image-file contract, checker, scope rule and notebook image policy (#307) #322; the image rules themselves (280 px
   minimum, the image-file contract, byte hashing) are untouched by this diff, and the new section
   makes no image claim that contradicts them.
6. Two lines left unreflowed from the last round. docs/evaluation.md:778 is 134 characters (the
   line ending "Do not substitute the pair's start or midpoint: the three give"); docs/vision.md:270-271
   is ragged mid-paragraph ("real elapsed-time data does not contain them. Repeats are / dropped,").
   Ruff does not flag prose; fold these into the M1/M2 edit.
7. tests/test_frames.py:296 is a section banner reading # ------ round 2. Every other banner in
   the file names its subject (sample_frames, the rest). A review round means nothing to a later
   reader; name it for what it holds, or drop it.
8. docs/evaluation.md:763 links "What an evaluation must show" to multimodal-track.md with no
   section anchor, unlike sibling links on the page: multimodal-track.md#what-an-evaluation-must-show.
9. earliest_transition returns a truthy dict for a review as well as a find, so if found: does
   not mean "a transition was found" (reviewer 1 raised this last round; still open). The review key
   is documented, but showing the if found is None: ... elif found["review"]: ... else: ... pattern in
   docs/vision.md next to the helper would set it for sixteen recipes.

What I verified positively (independently, not by trusting the tests)

• count is exactly the documented formula. sample_frames(times, count=k) matches
  round(i*(n-1)/(k-1)) with exact halves rounding down, computed in exact Fraction arithmetic,
  for every n in 1..39 and every k in 1..n: 0 mismatches. The integer expression
  (2*i*(n-1) + span - 1) // (2*span) is floor(x + 1/2 - 1/(2*span)), i.e. round-half-down, and
  returns 0 and n-1 at the ends as documented. The docs/vision.md:276 worked example
  ([0, .1, .2, .3, 10], count=3 → 0.0, 0.2, 10.0) reproduces.
• every_seconds matches a brute-force reference ("nearest eligible frame; earliest within
  TIME_EPSILON of the minimum distance") over 4000 random sequences x 8 rates (seed 309):
  0 mismatches, every result ascending and duplicate-free. That also justifies the
  consecutive-only dedup: the chosen index is monotone in the target.
• The frame_windows example in docs/vision.md:300-306 (frames every 0.5 s to 8 s, width 3,
  stride 2 → windows starting 0, 2, 4, 5) reproduces exactly.
• "A correct detection can only be late, by less than the gap to the previous sampled frame"
  (docs/evaluation.md:781-783) is sound: with s_{k-1} < g <= s_k the reported time is s_k, so the
  error is s_k - g < s_k - s_{k-1}, strictly. It follows that the recommended tolerance_seconds >= max(frame_intervals(sampled)) always admits a correct detection, so the two statements are
  consistent and tolerance_below_sampling_interval flags exactly the regime they warn about.
• The docs/evaluation.md:787-811 worked example is arithmetically correct. I re-derived all six
  assertions by hand before executing it: 4 examples, 1 review, 3 resolved; exact 2 (equal time, and
  both-None), within 3; tolerance_accuracy 1.0 over resolved and 0.75 over all; outcomes
  ("within", "exact", "exact", "review"); mean_signed_error_seconds (0.5 + 0.0)/2 = 0.25 over
  n_timed = 2. earliest_transition(sampled, [False, False, True]) gives transition_seconds = 3.0
  under the stated convention, 0.5 s after the 2.5 s gold. It is the one new fenced block and it is
  executed by tests/test_docs_examples.py (block 8, empty preamble).
• TransitionMetrics in the result-type table (docs/evaluation.md:862) matches the dataclass
  exactly: all 23 fields, same order, nothing omitted or invented.
• Fit with docs/multimodal-track.md:95 ("Event or transition accuracy against labeled sequences,
  with sampling, order and uncertain outcomes stated"): accuracy — exact_* / tolerance_*; uncertain
  — n_uncertain / uncertain_rate with both denominators named and computed; sampling —
  max_sampling_interval_seconds and tolerance_below_sampling_interval; order — frames validates
  strictly increasing time and select_frames requires strictly increasing indices. The row is met,
  and docs/evaluation.md:763 points at it.
• No overclaim about Jev. The only model statements are factual API ones ("Jev's Choice and Noul
  answers carry no time"), and docs/evaluation.md's closing "every metric computed on synthetic
  fixtures is a check that the pipeline works, not a measure" still governs the new section. "No video
  is decoded anywhere in the cookbook" (docs/vision.md:235-237) is true: no cv2 / imageio /
  ffmpeg / VideoCapture import under src/, tools/ or recipes/ (the one git grep hit for
  cv2 is a substring inside a base64 PNG payload in recipes/08-faq-selection/notebook.ipynb).
• No revision-comparison language in the shipped docs or code ("previously", "no longer", "used to
  be", "was once": none in the diff), and no issue or review number cited in a shipped doc. The one
  review artefact in a committed file is suggestion 7.
• Tests prove the rules they claim. Spot-checked the places a rule could be asserted loosely:
  sample_frames has 19 parametrised rejection cases plus a boundary-slack test and a container-
  independence test; earliest_transition pins None-before-True → review, True-before-None →
  the earlier time, [None, True, True] → review at 0, and rejects [1, 0, 0] / "ab";
  select_frames rejects [True], [1.0], [2, 1], [1, 1] and an inherited non-integer
  frame_index; transition_metrics has 18 parametrised bad-input cases including 10**400,
  tolerance_seconds=True and a review label in gold. Nothing is skipped or xfailed in the new tests.

PR body against the diff

• The "What this adds" list matches the diff function for function, and the deliberate
  frames vs vision module deviation is stated plainly.
• The block-count bump (8 → 9, empty preamble) is described accurately.
• The frame_windows claim under "Review round 2 resolutions" repeats M1's overclaim ("so no frame
  is left out") and must be corrected with it.
• "How it was verified" reports a run on head 6b18804, not on the head under review. The merge is
  provably clean so the results almost certainly carry over, but the two Windows-only
  tests/test_notebook_ci_sandbox.py failures it reports are the ones main's c316712 (Foundation: two notebook-CI sandbox tests use signal.SIGKILL and fail on Windows #302 / Foundation: make the sandbox kill helper and docs-example timeout portable to Windows (#302) #325)
  fixes — i.e. the stated failure list is already stale with respect to the reviewed head. Worth a
  one-line note at the next push (the description itself must not be edited after the final push, per
  CONTRIBUTING.md).
• "the docs/vision.md blocks were executed by hand: ok" — not true against the document's own
  frames (M2).
• Closes #309 is correct for a PR intended to close the issue; no other closing keyword appears
  anywhere in the body or the diff.
• No count or "fixed" statement in the body that I could not verify, apart from the two above.

Checks I ran

Fresh venv in my own worktree (uv venv --python 3.14 .venv-r2, then
uv pip install --link-mode symlink -e ".[dev]" — ruff 0.17.1, pytest 9.1.1), Linux, Python 3.14.7,
every heavy job under flock .claude/heavy.lock nice -n 10, pytest -p no:cacheprovider for disk,
no TYPESAFE_API_KEY set, no live call, no credential looked for. All on head 4f338f92 (verified by
git rev-parse HEAD inside the same script).

Command	Result
ruff check .	All checks passed (rc 0)
ruff format --check .	399 files already formatted (rc 0) — this is the run that covers the Python code blocks in docs/*.md, so both new blocks are formatting-clean
python tools/check_hygiene.py	Hygiene check passed: 608 file(s) scanned (rc 0)
python tools/render_catalog.py --check	rc 0, no output
python tools/notebook_ci.py fixtures	51 recipe folders, 0 failed (rc 0)
python tools/check_recipe_assets.py	Image-asset check passed: 461 file(s) under recipes/ checked (rc 0)
pytest tests/test_docs_examples.py tests/test_frames.py tests/test_evaluation.py -q	339 passed, 3 skipped in 4.0 s (rc 0)
pytest -q (full, tests/ + recipes/)	5459 passed, 49 skipped, 50 deselected, 31 xfailed, 0 failed in 2 min 51 s (rc 0)

Notes on those runs, read rather than taken on trust:

• Nothing failed. The two Windows-only tests/test_notebook_ci_sandbox.py failures the PR body
  reports do not occur here (Linux), and main's c316712 (Foundation: two notebook-CI sandbox tests use signal.SIGKILL and fail on Windows #302 / Foundation: make the sandbox kill helper and docs-example timeout portable to Windows (#302) #325) is the fix for them anyway.
• The docs/evaluation.md block really executed. pytest tests/test_docs_examples.py -q -rs shows
  the only three skips are :280, :296 and :305, all "exercises the Windows-only SIGBREAK timer
  path" — not the block-execution tests. So test_block_executes[8] ran and passed on this head, and
  the 50 deselected tests in the full run are the default catalog_audit deselection, unrelated to this
  diff.
• ruff format --check is the only automated check that reads the docs/vision.md blocks, and it
  checks formatting only — which is why M2 is invisible to CI.
• gh pr checks 323 on this exact head: 60 of 60 pass, none pending or failed (Lint, Tests py3.10
  and py3.14, Catalog, Hygiene, Fixtures, Scope and every Notebook job). The GitHub check-runs API
  reports 30 success and no other conclusion.

Reproductions of my own, run directly against the diff (standard library only, src on PYTHONPATH,
scripts in /tmp, nothing written into the checkout):

• frame_windows coverage sweep and the width=1, stride=5 counterexample (M1).
• The docs/vision.md walkthrough against the page's own load_frames frames, and the
  every_seconds=0.5 variant that works (M2).
• The count formula against exact Fraction round-half-down for every n in 1..39 and k in 1..n.
• every_seconds against a brute-force nearest/earliest-within-slack reference, 4000 random sequences
  x 8 rates, seed 309.
• The two documented worked examples (count=3 over [0, .1, .2, .3, 10]; windows 0/2/4/5).

Git-level checks: git show --cc HEAD (empty), git merge-tree --write-tree 6b18804 c316712 against
HEAD^{tree} (identical), git log --format='%G?' -1 (G), git diff --raw -M origin/main...HEAD
(7 paths, all 100644), git grep over recipes/ for the new names (no hit, so no notebook to
execute), git merge-base --is-ancestor c3167129 HEAD (true).

I did not execute any notebook: no recipe imports jev_cookbook.frames or
evaluation.transition_metrics, and the diff touches no notebook, fixture or recipe folder.

One process note for the record: my first install invocation used uv pip install --extra dev -e .,
which uv rejected ("Requesting extras requires a ... pyproject.toml file"); re-running it as
-e ".[dev]" worked. No check was skipped and nothing failed for lack of space (/workspace was at
~500 MB free at the start and recovered to ~8 GB during the run).

Prior review comments — status at this head

Read only after forming the view above.

Comment 6303367867 (review 1, round 1, head 8ff6588c)

• B1 timing error included wrong-label examples — resolved. evaluation.py appends to the
  headline errors only under label_ok; the all-labels view is n_timed_any_label /
  mean_*_any_label_seconds; test_transition_metrics_wrong_label_at_the_right_second_is_not_good_timing
  pins it, and the hand-computed test now expects n_timed == 2, MAE 0.2 (I re-derived both).
• B2 tie rule broken for frames within the slack — resolved for the two-frame case. The search
  spans cursor-1..cursor+1 and takes the earliest within TIME_EPSILON of the closest;
  test_tie_rule_holds_when_frames_are_closer_than_the_slack covers [0, 1, 1+5e-10, 2] -> [0, 1, 3].
  My 4000-trial brute-force sweep found no disagreement.
• S1 two-element lists — resolved (_transition accepts any non-str/bytes Sequence of length 2).
• S2 several review labels — resolved (uncertain_labels; a bare string is one label, tested).
• S3 one validator, ValueError not OverflowError — resolved (frames._seconds reused; see my
  suggestion 2 on it being private).
• S4 per-example outcomes — resolved (outcomes, timing_errors_seconds).
• S5 count is position-based, state the consequence — resolved (docs/vision.md:276 with the
  worked example; I verified the numbers).
• S6 absolute slack, say "since the start of the sequence" — resolved in both docs, plus the note
  that the slack is meaningless at epoch scale.
• S7 one module spelling, record the deviation — resolved in the docs (docs/vision.md:233-235:
  jev_cookbook.frames is the one import path, no re-export planned) and in the PR body. Whether Foundation: no deterministic helpers or evaluation for ordered frames (sampling, intervals, relative times, windows, transitions) #309
  itself was annotated is outside the diff; I did not check.

Comment 6303821403 (review 2, round 1, head 8ff6588c)

• Blocking 1 no transition-time convention — resolved. One convention ("the timestamp_seconds
  of the first frame in which the new state is visible") in frames.transition_time,
  frames.earliest_transition, the transition_metrics docstring and docs/evaluation.md:768-779,
  with "do not substitute the pair's start or midpoint", plus max_sampling_interval_seconds /
  tolerance_below_sampling_interval and test_transition_convention_end_to_end.
• Blocking 2 wrong-label timing — resolved (same change as review 1 B1).
• S frame-at-time lookup — resolved (frame_at, before / after / nearest, ties to the earlier
  frame, raises when nothing qualifies).
• S sliding windows — resolved in shape, but its documented guarantee is now wrong: frame_windows
  exists and keeps empty windows visible with indices == [], which is good; see M1.
• S index spaces — resolved (frame_index on select_frames copies, preserved across re-selection
  and now validated; "Index spaces" paragraph at docs/vision.md:316-322).
• S multi-transition sequences — resolved (docs/evaluation.md:838-844: split per transition, "do
  not invent a sequence-alignment metric").
• S two review sources, one label — resolved (uncertain_labels plural; the docs say to map both
  the model's uncertain option and Python's gate to one of them).

Comment 6304216637 (review 1, re-review, head 2cef5c7e) — APPROVE

• S1 clusters of 3+ frames within the slack — resolved by narrowing the claim, not by code:
  docs/vision.md:269-271 now says the tie rule is "guaranteed for up to two such frames around a
  target". That is honest, and I agree the _ordered rejection was not needed.
• S2 earliest_transition rejected np.bool_ — resolved (dtype-kind check;
  test_earliest_transition_accepts_numpy_booleans).
• S3 frame_windows can leave trailing frames in no window — addressed for the tail, but the fix
  introduced M1. The suggested wording ("frames after the last full window are not covered… or add a
  final window") became an unconditional "no frame is left out", false for stride > width. Not
  resolved as a documentation matter.
• S4 "its time can only be late" — resolved ("a correct detection can only be late", plus the
  negative-signed-error note for a false early flag).
• S5 frame_index reaches the state; validate an inherited one — resolved (docs say to drop it;
  test_select_frames_validates_an_existing_frame_index).

Comment 6304297506 (review 2, re-review, head 2cef5c7e) — CHANGES-NEEDED

• Blocking 1 earliest_transition could not represent an undetermined pair — resolved. None
  (and NumPy booleans) accepted; an undetermined pair before the first True, or with no True at all,
  returns {"review": True, "first_undetermined_index": i, ...} with no time; found transitions carry
  "review": False; the docstring says never to coerce to False; both docs say to score it as one of
  uncertain_labels; two dedicated tests.
• Blocking 2 frame_windows leaves the end uncovered — partially resolved: the tail is covered
  and tested, but the accompanying guarantee is an overclaim for stride > width (M1).
• S drop frame_index from the state — resolved (docs/vision.md:290-292).
• S uncertain_labels=None raised a bare TypeError — resolved (ValueError with a message;
  test_transition_metrics_rejects_missing_or_bad_review_labels).

Comment 6304979903 (review 2, re-review 2, head 6b18804) — APPROVE

• S the 148-character docstring line at frames.py:289 and the overlong docs/evaluation.md paragraph
  line — not resolved. frames.py:289 is still 148 characters (and is the line carrying M1's false
  claim); docs/evaluation.md:778 is still 134. Both were non-blocking; fold them into the M1/M2 fix.
• S earliest_transition returns a truthy dict for a review as well as a find, so if found: is
  misleading — not addressed beyond the existing documented review key. Still non-blocking; see my
  suggestion 9.

Acceptance criteria of #309, one by one

1. sample_frames(timestamps, *, every_seconds, count, start, end) returns selected indices
   deterministically; the docs state the tie-breaking rule; edge tests cover empty, single, exact
   boundaries and float drift — met in substance, at jev_cookbook.frames.sample_frames rather
   than vision.sample_frames. The deviation is deliberate, documented as the single import path with
   no re-export (docs/vision.md:233-235), stated in the PR body, and accepted by both prior reviewers;
   I accept it too (no Pillow import, no collision with Foundation: enforce the Images API's 280 px minimum, hash image bytes in replay keys, bound tiles (#306) #321's vision.py). Determinism, the tie rule
   and all four edge classes are tested, and I re-derived the count and every_seconds rules against
   independent references with no mismatch.
2. frame_intervals and relative_frames (adds seconds_ago, order unchanged), both tested —
   met (tests/test_frames.py:217-251; the input rows are provably not mutated).
3. window_timestamps(state_frames, first_index, last_index) returns start, end and duration, with
   tests — met (tests/test_frames.py:254-280). Its only worked example in the docs raises (M2),
   which is a docs defect, not a helper defect.
4. transition_metrics reporting exact and within-tolerance accuracy; an explicit uncertain/review
   count with a stated denominator; earliest-transition error in seconds; documented in
   docs/evaluation.md; tested with hand-computed cases — met. Both denominators are named and
   computed (n_resolved and n_examples, with *_accuracy and *_accuracy_all); the timing error is
   label-matched with the any-label view kept under separate names; the documented convention makes the
   number well defined rather than builder-dependent; the hand-computed tests and the executed docs
   block agree with my own arithmetic.
5. docs/vision.md documents sampling, that no video decoding happens (frames are pre-extracted
   stills), and how a recipe records the selected timestamps in fields — not met at this head.
   All three subjects are present and the sampling and no-video statements are accurate, but the section
   contains a false guarantee about frame_windows (M1) and a worked example that raises on the page's
   own data (M2), and the fields example drops the image path the same document requires (suggestion
   4). These are exactly the claims a recipe author will copy.
6. No new dependency; Pillow only — met. pyproject.toml untouched; frames.py is
   stdlib-only; evaluation.py gains only an intra-package import.

Noticed outside this task (reported, not fixed)

• docs/development.md names a ruff version the repository no longer pins. It says "Run
  pre-commit with the project virtual environment activated so the pinned ruff==0.16.10 is first on
  PATH", but pyproject.toml:24 pins dev = ["ruff==0.17.1", "pytest"], and that is what a fresh
  pip install -e ".[dev]" gives (ruff 0.17.1 in my venv; both prior reviewers also reported 0.17.1).
  This is on main, not in this PR's diff, so I have not touched it; it belongs in its own foundation
  issue.

Verdict

Three must-change items, all in docs or docs-adjacent text and all introduced by the most recent fix
round, so neither prior reviewer saw them: a coverage guarantee stated three times that is false for
every stride > width configuration of a helper sixteen recipes will copy (M1), a worked example that
raises on the document's own data with no test covering it (M2), and a block count the PR itself made
wrong (M3). Everything else I checked — the transition-time convention, the denominators, the count
and every_seconds arithmetic, the tie rule, the fit with the multiframe row of
docs/multimodal-track.md, consistency with #321/#322's image rules, the merge, the scope and the
dependency rule — I verified independently and found sound. The fixes are small and local to
src/jev_cookbook/frames.py, docs/vision.md, tests/test_docs_examples.py and the PR body.

OPUS-REVIEW: CHANGES-NEEDED 4f338f9

[CompleteDotTech]

PR #323 review - reviewer 1 of 2 (fresh, independent)

• Agent ID: agent-a8934111fcb6837d9 (worktree .claude/worktrees/agent-a8934111fcb6837d9)
• Resolved model ID: claude-opus-5[1m] (Opus 5, 1M context)
• Head reviewed: 4f338f92fb4bced5a06b6cbed76f69597bdc40f8 (branch foundation/frame-helpers)
• Base reviewed against: c3167129e3d272588cd2d0febe73d201b1b4011a (origin/main, also the merge-base)
• Issue: Foundation: no deterministic helpers or evaluation for ordered frames (sampling, intervals, relative times, windows, transitions) #309. I formed my view of the diff and the issue before reading any prior review comment.

The merge itself

git show --cc HEAD prints the commit header and no diff hunks at all, so the merge resolved no
conflict and contributed no content of its own. git diff 6b18804 HEAD is exactly main's own
24-path change set (c316712 and its ancestors: .gitattributes, tools/check_recipe_assets.py,
src/jev_cookbook/vision.py, ...), with none of this PR's files reverted or altered.
git diff --raw -M origin/main...HEAD is 7 paths, all mode 100644:

:100644 100644 M docs/evaluation.md
:100644 100644 M docs/vision.md
:100644 100644 M src/jev_cookbook/evaluation.py
:000000 100644 A src/jev_cookbook/frames.py
:100644 100644 M tests/test_docs_examples.py
:100644 100644 M tests/test_evaluation.py
:000000 100644 A tests/test_frames.py

No symlink (120000), submodule (160000), mode change or type change; nothing outside src/,
docs/ and tests/; pyproject.toml untouched, so no new dependency. This is a foundation/
branch, so the recipe-scope allowlist does not apply. The merge commit verifies as a good signature
(%G? = G); 6b18804 reports U (good SSH signature, untrusted locally - gpg is absent in this
sandbox). Both sides of the merge touched docs/vision.md and tests/test_docs_examples.py; I read
the merged result of both and they combine cleanly (the new "Sampling, intervals and windows" section
follows main's revised tiling/load_frames prose with no duplication or contradiction, and the
block-count assertion in test_docs_examples.py is 9, matching the 9 fenced python blocks now in
docs/evaluation.md).

The PR body still describes the earlier merge ("Merged origin/main (7fa806e)") and reports its
verification on head 6b18804. That is correct behaviour under CONTRIBUTING ("a description is
written once, at open ... and never edited after the final push"), not a defect; it does mean the
body's claim set has to be read against 6b18804, which is what I did. One body claim is wrong on
its own terms, and it is finding M1 below: "frame_windows appends a final window aligned to end
... so no frame is left out".

---

Must change

M1. frame_windows states a coverage guarantee it does not provide, and no test proves it

src/jev_cookbook/frames.py:289 (docstring), src/jev_cookbook/frames.py:308 (code comment),
docs/vision.md:302-305, test gap at tests/test_frames.py:450-472.

The docstring says the appended end-aligned window means "so every frame lies in some window",
and docs/vision.md:304 says "so no frame is left out". Both are unconditional. They hold only
when stride_seconds <= width_seconds. With a stride larger than the width the stride-spaced windows
do not touch each other, the end-aligned window closes only the final hole, and the interior holes
stay open with no signal of any kind:

frame_windows([float(i) for i in range(11)], width_seconds=1.0, stride_seconds=5.0)
# -> [(0.0, 1.0, [0, 1]), (5.0, 6.0, [5, 6]), (9.0, 10.0, [9, 10])]
# frames 2, 3, 4, 7 and 8 are in no window

I quantified it: over 2000 random stride > width configurations, 1592 left at least one frame
uncovered; over 4000 random stride <= width configurations, 0 did (and the invariant is
provable - with stride <= width consecutive windows overlap and
high - starts[-1] <= width + stride <= 2 * width, so the end-aligned window cannot leave a hole).
The guarantee is therefore exactly conditional on stride_seconds <= width_seconds, and nothing in
the code, the docstring, the docs or the tests says so.

Two further inaccuracies in the same sentences: the parenthetical "overlapping the previous window by
more than the stride implies" is false in that regime (the appended window need not overlap the
previous one at all), and with an explicit start/end the claim can only ever be about frames in
[start, end] - frame_windows(times, width_seconds=3.0, stride_seconds=1.0, start=4.0) leaves
frames 0-3 in no window, correctly, but the text does not qualify it.

Why this is must-change rather than a nit: this is the exact defect class that blocked round 2
(comment 6304297506, blocking 2, "frame_windows silently leaves the end of the sequence out of
every window"). The fix closed the tail case and replaced a hedged sentence with an unconditional
promise that is false in another regime, and the new test
(test_frame_windows_cover_the_end_of_the_sequence) asserts full coverage only for
width=3, stride=2 and width=2, stride=1.5 - both stride < width. The one stride > width case
in the suite (tests/test_frames.py:366) has width equal to the whole range, so a single window
covers everything and the gap cannot appear. A rule stated in the contract of a module that sixteen
multimodal recipes will copy, which no test exercises in the regime where it fails, is not safe to
ship: a builder who swaps width_seconds and stride_seconds loses frames silently while the docs
tell them that cannot happen.

Change, either one:

• Reject it. if stride > width + TIME_EPSILON: raise ValueError(...) - consistent with the
  module's stated "raises ... instead of repairing it" stance, and it keeps the guarantee
  unconditional and true. Add a test for the rejection.
• Qualify it. Say in the docstring and in docs/vision.md that every frame in [start, end]
  lies in some window when stride_seconds <= width_seconds, and that a larger stride produces
  deliberately sparse windows with frames between them in none; drop or qualify the "overlapping the
  previous window" parenthetical; and add a test with stride > width asserting the uncovered
  indices, so whichever rule is stated is the rule the tests prove.

---

Suggestions (non-blocking)

S1. evaluation.py reaches into a private name of frames. src/jev_cookbook/evaluation.py:35
imports TIME_EPSILON, _seconds from .frames. Sharing one validator was the right answer to round
1's suggestion 3, but it is shared through a single-underscore door, so frames' internals are now
load-bearing for another module with nothing marking them as such. Promote it (a public
frames.seconds_value, or a thin public wrapper) or note the coupling in frames.py's module
docstring.

S2. tolerance_correct is cumulative; outcomes' "within" is not. evaluation.py:2060-2061
and the TransitionMetrics docstring. tolerance_correct counts exact plus within (correctly - it
is the within-tolerance numerator), while outcomes spells "within" exclusively ("within tolerance
but not exact"). A README that computes outcomes.count("within") / n_resolved will not match
tolerance_accuracy. The executed docs example does show the two side by side
((exact_correct, tolerance_correct) == (2, 3) against one "within" in outcomes), but one clause
in the dataclass docstring saying tolerance_correct >= exact_correct would remove the trap for the
sixteen READMEs that will quote this shape.

S3. Clusters of three or more frames inside TIME_EPSILON break "nearest", not only the tie rule.
src/jev_cookbook/frames.py:139 (near = range(max(0, cursor - 1), ...)).
sample_frames([0.0, 1.0, 1.0 + 3e-10, 1.0 + 6e-10, 2.0], every_seconds=1) returns [0, 2, 4]: the
frame exactly on the target (index 1, distance 0) is outside the three-wide search window and is
not selected. docs/vision.md:269-271 now narrows the promise to "up to two such frames around a
target", so this is disclosed rather than hidden, and real elapsed-time data will not contain it.
Rejecting consecutive timestamps closer than 2 * TIME_EPSILON in _ordered would delete the whole
class and let the docs make an unconditional promise again. (Raised as round-2 suggestion 1 in
comment 6304216637 and answered by narrowing the docs; I accept that as defensible and record the
residual, with the sharper observation that what fails there is "nearest", not just the tie rule.)

S4. Two window-bound errors name an argument the caller did not pass.
src/jev_cookbook/frames.py:121 and :300. sample_frames([5.0, 6.0], start=9.0) and
sample_frames([5.0, 6.0], end=1.0) both raise start must not be after end, although in each case
one side was defaulted from the frame times. Naming the defaulted side ("start 9.0 is after the last
frame, 6.0") would say what to fix.

S5. relative_frames silently overwrites an existing seconds_ago. frames.py:217-220.
relative_frames([{"timestamp_seconds": 0.0, "seconds_ago": 99}]) returns seconds_ago == 0.0. That
is probably the behaviour you want, but it is the opposite of the care select_frames takes with an
inherited frame_index (preserved and validated, frames.py:186-189), and the docstring says only
"Order and every other key are unchanged". One clause ("an existing seconds_ago is replaced") would
settle it.

S6. The two argument kinds are easy to swap and the error does not point at the fix.
sample_frames, frame_at and frame_windows take bare timestamps; frame_timestamps,
select_frames, frame_intervals, relative_frames, window_timestamps, transition_time and
earliest_transition take state frames. The split is what #309 specifies, so it is right, but
sample_frames(state_frames) raises timestamp at position 0 must be a finite number of seconds
with no hint that frame_timestamps is the missing step. Detecting a Mapping and saying so costs
one line.

S7. src/jev_cookbook/frames.py:289 is 148 characters. The project's line-length is 100;
E501 is not in the selected ruff rules, so neither ruff check nor ruff format catches a long
docstring line. Raised as a suggestion in comment 6304979903 and not done. Reflow it when M1 is
addressed - it is the same sentence.

S8. docs/vision.md's two new python blocks are executed by no test.
tests/test_docs_examples.py covers docs/evaluation.md only (DOC = REPO / "docs" / "evaluation.md"), and the first vision block cannot be executed as written anyway (it opens with
times = frame_timestamps(frames) with frames undefined). The module that sixteen recipes will
copy therefore has its only worked usage example unguarded, which is the exact gap
tests/test_docs_examples.py's own docstring was written to close. Not a regression introduced here
and not blocking, but worth a follow-up issue to extend that harness to docs/vision.md with a
preamble, the way PREAMBLE already does for evaluation.md.

S9. #309's acceptance checklist should be amended to match what shipped. The criteria name
vision.sample_frames, vision.frame_intervals, vision.relative_frames and
vision.window_timestamps; they ship as jev_cookbook.frames.*. I accept the deviation (frames.py
is stdlib-only and does not drag in Pillow, vision.py is untouched, and docs/vision.md:237-239
states jev_cookbook.frames is the one import path with no re-export planned), but the issue text
should record it rather than leave the checklist reading as unmet.

---

What I checked and found correct

I read all of src/jev_cookbook/frames.py, the transition_metrics section of
src/jev_cookbook/evaluation.py, all of tests/test_frames.py, the new tests in
tests/test_evaluation.py, and both doc sections, then probed against independent references.

• Sampling determinism and the nearest/tie rule. 20,000 random every_seconds cases (seeded,
  1-12 frames, step 0.1-5 s) matched a brute-force reference ("nearest eligible frame; earliest
  within TIME_EPSILON of the minimum distance") with 0 mismatches, and every result was
  ascending with no repeats. I also checked analytically that the cursor advance keeps the nearest
  frame inside near for every target, and that best is non-decreasing across ascending targets,
  which is what makes the chosen[-1] != ... dedup sufficient.
• count rounding. (2*i*(n-1) + span - 1) // (2*span) is exactly
  floor(x + 1/2 - 1/(2*span)) for x = i*(n-1)/span, i.e. round-half-down, in integers, as
  documented. i = 0 gives the first eligible frame and i = count-1 the last; with count < n the
  spacing (n-1)/span > 1, so the indices are strictly increasing and len == count. The
  parametrised test (tests/test_frames.py:119-127) pins exactly that for n < 30.
• Float drift. every_seconds and frame_windows both multiply (low + k * step) rather than
  accumulate, and the target/window counts carry an absolute TIME_EPSILON slack scaled into step
  units (TIME_EPSILON / step), which is the dimensionally right form. Drift cases are tested
  (0.1 + 0.2 against 0.3, tenth grids with 0.3, 1/3), and 0.1-grid selections are stable.
• Boundary and empty cases. _ordered rejects empty, non-finite, negative, duplicate and
  out-of-order times, and every public function routes through it; an empty sampling window raises
  rather than returning []; start/end edges are inclusive within the slack
  (end = 3.0 - 1e-10 keeps frame 3; start = 2.0 + 1e-6 drops frame 2). frame_intervals of one
  frame is []; relative_frames of one frame gives seconds_ago == 0.0; window_timestamps of a
  single-frame window gives duration_seconds == 0.0; earliest_transition of one frame with no
  flags is None. All tested.
• transition_metrics denominators. n_examples == n_uncertain + n_resolved; *_accuracy over
  n_resolved with NaN at zero (_safe_div); *_accuracy_all over n_examples, which _as_list
  guarantees is at least 1; uncertain_rate over n_examples; the headline means over n_timed
  (label-matched, both times present) and the *_any_label means over n_timed_any_label, all NaN
  at count 0; n_missed_time / n_spurious_time counted over resolved examples only and independent
  of the label. Each of those is asserted by a hand-computed test, including the all-review case (NaN
  resolved accuracy, exact_accuracy_all == 0.0) and the wrong-label-at-the-right-second case. Means
  use math.fsum, so pooling many examples does not drift.
• The transition-time convention. One rule - the timestamp_seconds of the first frame showing
  the new state - stated in transition_time, earliest_transition, the transition_metrics
  docstring, docs/evaluation.md and docs/vision.md, with test_transition_convention_end_to_end
  pinning gold-at-full-rate against prediction-on-sampled-frames (late by 0.5 s, within a 1.0 s
  tolerance, flag False). "A correct detection can only be late" is right as written
  (s_{k-1} < g <= s_k gives 0 <= s_k - g < s_k - s_{k-1}), which also makes
  tolerance_seconds >= max(frame_intervals(sampled)) the correct advice and the strict
  interval > tolerance + eps the correct flag condition; the docs say a false early flag shows as a
  negative signed error.
• Undetermined pairs. earliest_transition accepts None and NumPy booleans (dtype kind b),
  returns {"review": True, "first_undetermined_index": i, ...} with no time when an
  undetermined pair precedes the first True or no True exists, returns review: False with the
  time otherwise, and None only when every pair is False. It rejects a flag count other than
  n-1 and any non-boolean, non-None value. All four branches are tested.
• The docs example is executed. docs/evaluation.md's new block is block 8 of 9 and is both
  linted and executed by tests/test_docs_examples.py with an empty preamble; I re-derived its six
  assertions by hand (errors +0.5, 0.0 giving a signed mean of 0.25; outcomes
  within, exact, exact, review; tolerance_accuracy 1.0; tolerance_accuracy_all 0.75) and they
  are right.
• No new dependency. frames.py imports bisect, math, collections.abc, numbers and
  typing only; evaluation.py already depended on numpy; pyproject.toml is untouched; frames
  imports nothing from the package, so there is no cycle with evaluation.
• Offline. Nothing in the diff calls a backend, reads an environment variable, or touches the
  network. No recipe imports jev_cookbook.frames or transition_metrics (grep over recipes/, 51
  folders, no hit), so no notebook is affected and none needed executing for this diff.

---

Prior review findings, at this head

Comment 6303367867 (review 1, round 1, on 8ff6588c)

Finding	Status at 4f338f9
B1 timing error counted wrong-label predictions	Resolved. errors.append is now inside if label_ok; the all-labels view moved to n_timed_any_label / mean_*_any_label_seconds. transition_metrics([("opened", 2.0)], [("closed", 2.0)], tolerance_seconds=0.5) gives n_timed=0, MAE NaN, n_timed_any_label=1. Pinned by test_transition_metrics_wrong_label_at_the_right_second_is_not_good_timing and by the hand-computed test (n_timed == 2, MAE 0.2).
B2 tie rule broken for two frames inside the slack	Resolved for the case raised. near = range(cursor-1, cursor+2) plus "earliest within slack of the closest"; sample_frames([0, 1, 1+5e-10, 2], every_seconds=1) == [0, 1, 3], tested with its mirror. Residual for clusters of three or more: S3.
S1 accept a 2-element list	Resolved. _transition accepts any non-str/bytes Sequence of length 2; tested with JSON-shaped lists.
S2 several review labels	Resolved. uncertain_labels: Collection | str; a bare string is one label; tested.
S3 one validator, ValueError not OverflowError	Resolved (10**400 raises ValueError for the time, the tolerance and the interval, each tested) - but through a private import, see S1 above.
S4 per-example outcomes	Resolved. outcomes and timing_errors_seconds.
S5 document that count is position-based	Resolved. docs/vision.md with the [0, .1, .2, .3, 10] example, plus test_count_spreading_is_by_position_not_by_time.
S6 "elapsed seconds since the start of the sequence"	Resolved in both docs and the module docstring.
S7 one module spelling, no re-export	Resolved in the docs; the issue text still says vision.* (S9).

Comment 6303821403 (review 2, round 1, on 8ff6588c)

Finding	Status at 4f338f9
Blocking 1: no convention for which second a transition is	Resolved. transition_time / earliest_transition plus "One convention for the time" in docs/evaluation.md, with the explicit "do not substitute the pair's start or midpoint", max_sampling_interval_seconds, tolerance_below_sampling_interval, and an end-to-end test.
Blocking 2: timing error included wrong labels	Resolved (same as B1 above).
S frame-at-time lookup	Resolved. frame_at with before / after / nearest, tie to the earlier frame, raising when nothing qualifies; tested.
S sliding-window enumeration	Added (frame_windows), but its coverage contract is wrong - M1.
S index spaces	Resolved. select_frames adds and preserves frame_index, validated; "Index spaces" paragraph in docs/vision.md; tested.
S multi-transition sequences	Resolved. "Several transitions in one sequence" in docs/evaluation.md.
S two review sources, one label	Resolved. A collection of labels plus the docs sentence about mapping the model's option and Python's gate.

Comment 6304216637 (review 1, round 2, APPROVE on 2cef5c7e)

Finding	Status at 4f338f9
S1 clusters of 3+ frames inside the slack	Not fixed in code; docs narrowed to "up to two such frames around a target". Defensible; residual recorded as S3.
S2 earliest_transition rejected np.bool_	Resolved. dtype-kind b accepted; test_earliest_transition_accepts_numpy_booleans.
S3 frame_windows can leave trailing frames in no window	Addressed for the tail (end-aligned window) but the replacement claim overclaims - M1.
S4 "its time can only be late"	Resolved. Both docs now say a correct detection can only be late, and name the negative signed error for a false early flag.
S5 frame_index in the state; unvalidated inherited frame_index	Resolved. docs/vision.md:292 says to drop it; select_frames validates a non-negative integer; tested with "x", -1, 1.5, True, None.

Comment 6304297506 (review 2, round 2, CHANGES-NEEDED on 2cef5c7e)

Finding	Status at 4f338f9
Blocking 1: earliest_transition could not represent an undetermined pair	Resolved. None accepted, review result with no time, "never coerce to False" in the docstring, scoring guidance in both docs, four tested branches including [None, True, True].
Blocking 2: frame_windows left the end uncovered	Addressed; see M1 for the residual overclaim the fix introduced.
S drop frame_index from the state	Resolved.
S uncertain_labels=None raised a bare TypeError	Resolved. ValueError: uncertain_labels must be a label or a collection of labels, tested for None, 3, 2.5.

Comment 6304979903 (review 2, re-review 2, APPROVE on 6b18804)

Finding	Status at 4f338f9
S frames.py:289 is a 148-character line (and a long docs/evaluation.md paragraph line)	Not fixed. Still 148 characters. S7.
S earliest_transition returns a truthy dict for a review too, so if found: no longer means "found"	Unchanged by design; the review key is documented. Worth setting the None / review / found three-branch pattern in the first recipe that uses it, as that comment suggests.

---

Acceptance criteria of #309

1. sample_frames(timestamps, *, every_seconds, count, start, end) returns indices
   deterministically, docs state the tie-breaking rule, edge tests cover empty, single, exact
   boundaries and float drift - met (verified against a 20,000-case brute-force reference). It
   ships as jev_cookbook.frames.sample_frames, not vision.sample_frames: a documented, justified
   deviation I accept; amend the issue text (S9).
2. frame_intervals(state_frames) returns durations; relative_frames(state_frames, now=None) adds
   seconds_ago without changing order; both tested - met (module path as above).
3. window_timestamps(state_frames, first_index, last_index) returns start, end and duration, with
   tests - met.
4. evaluation.transition_metrics(gold, predicted, *, tolerance_seconds) reports exact and
   within-tolerance accuracy, an explicit uncertain/review count with a stated denominator, and
   earliest-transition error in seconds; documented in docs/evaluation.md and tested with
   hand-computed cases - met, under one documented transition-time convention, with both
   denominators stated and the docs example executed in CI.
5. docs/vision.md documents sampling, that no video decoding happens (frames are pre-extracted
   stills), and how a recipe records the selected timestamps in fields - met in substance, one
   statement wrong: the sampling, no-decoding and fields material is all there and accurate, but
   the frame_windows paragraph (docs/vision.md:302-305) states a coverage guarantee that does not
   hold (M1).
6. No new dependency; Pillow only - met. frames.py is standard library only, evaluation.py
   gained no import beyond collections.abc.Collection and .frames, and pyproject.toml is
   unchanged.

---

Checks run

Linux, Python 3.14.7, fresh uv venv .venv-r1 with
uv pip install --link-mode symlink --extra dev -e ., no TYPESAFE_API_KEY set, no live call, no
network use by any check. Every heavy command ran under flock .claude/heavy.lock nice -n 10, and
pytest with -p no:cacheprovider (free disk on /workspace stayed between 0.3 and 0.7 GB
throughout, so the venv used symlinked wheels and was deleted afterwards).

• ruff check . (ruff 0.17.1, the pinned version from the venv) - All checks passed, rc 0.
• ruff format --check . - 399 files already formatted, rc 0.
• pytest -q -p no:cacheprovider - 5459 passed, 49 skipped, 50 deselected, 31 xfailed, 0
  failed in 182 s. (The 50 deselected are pyproject.toml's default catalog_audit
  deselection. No failure at all here, including the two tests/test_notebook_ci_sandbox.py
signal.SIGKILL cases that fail on the author's Windows machine - this run is on Linux.)
• pytest -q -p no:cacheprovider -rs tests/test_frames.py tests/test_evaluation.py tests/test_docs_examples.py - 339 passed, 3 skipped. The three skips are
  tests/test_docs_examples.py:280/296/305, all "exercises the Windows-only SIGBREAK timer path;
  POSIX uses SIGALRM", so test_block_executes itself ran: the new docs/evaluation.md
  transitions block really was executed and linted here, not skipped.
• python tools/check_hygiene.py - Hygiene check passed: 608 file(s) scanned, rc 0.
• python tools/render_catalog.py --check - rc 0 (README is current; the diff touches no recipe
  and no catalog entry, so nothing was expected to change).
• python tools/notebook_ci.py fixtures - 51 recipe folders, 0 failed, rc 0.
• git show --cc HEAD - no diff hunks (the merge contributed nothing of its own).
• git diff --raw -M origin/main...HEAD - the 7 100644 paths listed above.
• Notebook execution: nothing under recipes/ imports jev_cookbook.frames or
  transition_metrics, and the diff touches no recipe, so no notebook is affected and none was
  executed.
• Independent probes (pure-stdlib scripts, outside the suite): the 2000/4000 frame_windows
  coverage trials behind M1; the 20,000-case sample_frames brute-force comparison; the three-frame
  sub-epsilon cluster of S3; the error-message cases of S4; the seconds_ago overwrite of S5; the
  wrong-argument-kind messages of S6; every_seconds on an epoch-scale grid (it happens to work
  here, but the docs rightly forbid it, since 1e-9 s is below the float spacing at 1.7e9).

---

Verdict

One must-change, M1: a false unconditional coverage guarantee in the contract of a helper that
sixteen recipes will copy, in the same place a prior round blocked, with no test in the regime where
it fails. The fix is small - a validation line, or a qualifying clause plus one test. Everything else
in the diff is careful, well tested and, as far as I can establish, correct; all nine prior blocking
findings across the two earlier rounds are resolved at this head, and the merge changed nothing
unintended.

OPUS-REVIEW: CHANGES-NEEDED 4f338f9

[CompleteDotTech]

PR #323 — Opus re-review (independent review 2 of 2, round 3)

• Agent ID: agent-a0f4a227a4ff1772f (worktree .claude/worktrees/agent-a0f4a227a4ff1772f)
• Resolved model ID: claude-opus-5[1m] (Opus 5, 1M context), as stated in my system prompt
• PR: Jev-Engineering/cookbook Foundation: deterministic helpers and evaluation for ordered frames (#309) #323, branch foundation/frame-helpers
• Head reviewed: 42dc36a6a09be3946ef35e11bb0cbb9fe7841075
• Base used: origin/main = 2601074bc1ff20f640e33d3141fa6d6bd5deb6b0 (PR Foundation: Box evaluation report and gold Box label format (#311) #331)
• Merge parents: a9c792295729a64eaf77a84103d0132e816a13b0 (fix commit) + 2601074 (main)
• My previous review of this PR: head 4f338f92 (CHANGES-NEEDED), file .claude/reviews/pr323-r2.md
• Issue: Foundation: no deterministic helpers or evaluation for ordered frames (sampling, intervals, relative times, windows, transitions) #309

I checked out 42dc36a myself and reviewed git diff 4f338f9 a9c7922, git show --cc 42dc36a, the
whole merged docs/vision.md and docs/evaluation.md, and the edited PR body. I did not take the
builder's report on trust: every resolution below is backed by a reproduction or a mutation probe of
my own.

Verdict first

All three of my must-change findings are resolved, and resolved honestly rather than papered over. The
fix commit is 124 insertions across four files, touches nothing I did not ask about, and the merge of
#331 is a byte-identical clean auto-merge. Every check is green on this exact head, including all 60
CI checks. Approve.

My findings from head 4f338f92

M1 — frame_windows promised coverage it did not deliver for stride > width: resolved

The claim is now scoped, in all four places it appeared, and the counterexample is documented:

• src/jev_cookbook/frames.py:288-297 has a new Coverage paragraph: "With stride_seconds <= width_seconds every frame in [start, end] lies in at least one window. With a larger stride the
  windows are deliberately sparse: frames between one window's end and the next window's start lie in
  no window, the final end-aligned window closes only the gap at the tail, and nothing is raised."
• docs/vision.md:407-416: "Every frame lies in some window only when stride_seconds <= width_seconds. A larger stride gives deliberately sparse windows: with frames every second to
  10 s, width 1 and stride 5 the windows are [0, 1], [5, 6] and [9, 10], so frames 2, 3, 4, 7 and
  8 are in none, and nothing is raised. Use a stride no larger than the width when every frame must be
  scored." That is exactly the counterexample I reported, with exactly the right numbers — I
  re-reproduced it on this head.
• The second false claim I flagged in the same sentence, "overlapping the previous one", is removed
  from both the docstring and docs/vision.md; the final window is now described plainly as
  [end - width, end].
• The code comment at frames.py:315 changed from "no frame is left out" to "final window aligned to
  the end of the range".
• Behaviour is unchanged, which is the right call: frame_windows is not in Foundation: no deterministic helpers or evaluation for ordered frames (sampling, intervals, relative times, windows, transitions) #309's acceptance criteria
  and a late validation change would have been riskier than documenting the regime.

I did not just read the new sentence — I checked that the guarantee it now makes is actually true, and
more broadly than the new test does. Randomized probe (seed 323, 20,000 trials, non-uniform frame
times, random width/stride, half the trials with explicit start/end): 5219 cases with
stride <= width, zero violations of "every frame in [start, end] lies in some window". It is also
provable: consecutive windows touch or overlap iff stride <= width, and w_{n-1} > high - width - stride, so high - w_{n-1} < width + stride <= 2*width, which makes the appended [high - width, high] contiguous with the run.

New tests, both of which I confirmed assert true values and both of which I mutation-probed:

• tests/test_frames.py:479-504 test_frame_windows_cover_every_frame_when_stride_is_at_most_width —
  pins the stride == width boundary explicitly (windows [0,2] [2,4] [4,6] [6,8] then the
  end-aligned [7,9], which I re-derived by hand) and then sweeps every width/stride pair on a
  grid with stride <= width, with and without start/end, asserting full coverage of the
  frames inside the bounds. Including the bounded variant is what makes this a real test of the
  docstring's [start, end] wording rather than of the default case only.
• tests/test_frames.py:507-521 test_frame_windows_with_a_stride_wider_than_the_width_leave_frames_out
  — pins the sparse regime with both of my examples: width 1 / stride 5 over 0–10 s leaving
  {2, 3, 4, 7, 8} uncovered, and the smallest overshoot width 0.5 / stride 1.5 over a half-second
  grid leaving [2, 5, 8, 11, 14, 17]. Those are the exact index sets my own sweep produced.

M2 — the docs/vision.md walkthrough raised ValueError on the page's own frames: resolved

• docs/vision.md:345-346 now says what the block continues from: "The block below continues from the
  frames loaded above (timestamps 0.0, 0.75 and 1.5 s)". A learner no longer has to infer it.
• every_seconds=2.0 → 0.5, and every comment now states the value the line produces: chosen
[0, 1, 2], gaps [0.75, 0.75], seconds_ago 1.5, 0.75, 0.0, window start 0.75, end 1.5, duration 0.75. Those are precisely the values I computed when I proposed the fix, and the
  "sampling" block's every_seconds was updated to 0.5 to match (docs/vision.md:441) — the
  consistency point I raised alongside M2.
• The gap that let this survive three rounds is closed by a real test, not by a note:
  tests/test_docs_examples.py:354-392 test_vision_frame_walkthrough_runs_on_the_pages_own_frames
  generates three 280 x 280 PNGs at the paths the page names, chdirs into tmp_path, extracts the
  three relevant fenced blocks from docs/vision.md by content marker and execs them in page
  order in one shared namespace, then asserts every value the comments claim, including
  fields["sampling"]. Points in its favour: _vision_block asserts exactly one block matches each
  marker, so a future ambiguity fails loudly instead of silently picking one; the fixtures are 280 px
  (the Foundation: enforce the Images API's 280 px minimum, hash image bytes in replay keys, bound tiles (#306) #321 minimum, and exactly the 78,400 px minimum area), so the test exercises the real
  load_frames validation; and monkeypatch.chdir(tmp_path) keeps it out of the checkout (git status --porcelain was empty after my full-suite run).
• I verified the markers are still unambiguous after the Foundation: Box evaluation report and gold Box label format (#311) #331 merge, which added three fenced
  blocks to docs/vision.md: the file now has 12 python blocks, and load_frames(,
  frame_timestamps(frames) and "sampling" match blocks 8, 9 and 10 respectively — one each, in page
  order.

M3 — tests/test_docs_examples.py still said "eight blocks": resolved

Both occurrences (:31, :43) now say "nine", and tests/test_docs_examples.py:345-351
test_docstring_block_count_matches_the_assertion ties the prose to len(BLOCKS) so it cannot go
stale again. The guard reads the module __doc__ (the function's own docstring mentions "nine"
too, but is not searched), and it fails on an absent mention as well as a wrong one, since it compares
a set against {words[len(BLOCKS)]}.

Mutation evidence that the three new guards actually bite

Rule 9 of the common brief: a new instrument must be run against a ref where it has to find something.
Each mutation below was applied one at a time to my own worktree, the guard was run, and the file was
restored immediately (git checkout -- in an EXIT trap; git status --porcelain empty afterwards).
Baseline first: the five guards pass unmutated (5 passed in 1.36s).

Mutation	Guard	Result
docs/vision.md: every_seconds=0.5 → 2.0 (the exact M2 bug)	test_vision_frame_walkthrough_runs_on_the_pages_own_frames	KILLED (1 failed)
docs/vision.md: window_timestamps(sampled, 1, 2) → (0, 2) (a wrong documented window)	same	KILLED (1 failed)
tests/test_docs_examples.py: module docstring "nine" → "eight" (the exact M3 bug)	test_docstring_block_count_matches_the_assertion	KILLED (1 failed)
frames.py: drop the end-aligned final window (if False:)	..._cover_every_frame_when_stride_is_at_most_width + ..._cover_the_end_of_the_sequence	KILLED (2 failed)
frames.py: silently clamp stride to width (hiding the sparse regime)	..._with_a_stride_wider_than_the_width_leave_frames_out	KILLED (1 failed)
frames.py: raise for stride > width (option 1 from my review)	same	KILLED (1 failed)

The last row is worth stating plainly rather than hiding: the new test now pins the documented
choice, so adopting the validation alternative I offered would require editing that test. That is
correct behaviour for a test of a documented contract, not a defect — but it means the sparse regime is
now a deliberate, tested part of the interface, which is exactly what the docs claim.

Everything that changed since 4f338f92

git diff --stat 4f338f9 a9c7922: docs/vision.md +24/-17ish, src/jev_cookbook/frames.py +17/-6,
tests/test_docs_examples.py +53/-2, tests/test_frames.py +47/-0. I read all of it. There is nothing
in the fix commit beyond the three findings: no behaviour change in frames.py (only the docstring and
one comment), no change to evaluation.py, docs/evaluation.md, tests/test_evaluation.py or any
__all__, and no new dependency. The 148-character docstring line at the old frames.py:289 was
reflowed in the process — frames.py now has no line over 100 characters, closing half of my
previous suggestion 6.

The merge of #331

• git show --cc 42dc36a produces no combined-diff hunks: the merge commit contributes nothing of
  its own.
• Independently reproduced: git merge-tree --write-tree a9c7922 2601074 →
  3e74fa02a6cffaec12cd0914bd6a984244749235, identical to git rev-parse 42dc36a^{tree}.
• Both a9c7922 and 42dc36a are signed (%G? → G); the merge message carries the required
  Co-Authored-By: Claude Sonnet 5.5. origin/main is an ancestor of the head.
• Scope against the new base is unchanged: git diff --raw -M origin/main 42dc36a lists the same 7
  paths, all mode 100644, 2 added (src/jev_cookbook/frames.py, tests/test_frames.py) and 5
  modified, no symlink, submodule, mode or type change, nothing outside src/, docs/, tests/.
  pyproject.toml untouched → still no new dependency.
• No name collision with Foundation: Box evaluation report and gold Box label format (#311) #331: vision.__all__ gained LocalizationReport, gold_box,
  localization_report and expand_box; none of frames.py's ten names appears in vision, and
  load_frames stays in vision. jev_cookbook.frames is still the one import path for the
  arithmetic, as the docs claim.

Reading the merged docs/vision.md as a learner

#331 inserts four new ### sections (Gold labels, The localization report, Tiled and hierarchical
recipes, The older localization_metrics) between "Presence, confidence and evaluation" and "Tiles and
coordinate transforms". The frames material sits after them, under "Explicitly timestamped still
frames" → "### Sampling, intervals and windows" (lines 314-445). The merge boundary is clean: the
load_frames block is immediately above the sampling subsection, so the new "continues from the
frames loaded above" sentence is true of the merged page as well as of the pre-merge one.

I found no contradiction between #331's sections and the frames section, and several places where
they reinforce each other:

• docs/vision.md:4 promises these helpers "do not ... decode video", which the frames section
  restates as "No video is decoded anywhere in the cookbook" — I re-verified that claim at this head:
  no cv2 / imageio / ffmpeg / VideoCapture import under src/, tools/ or recipes/.
• Foundation: Box evaluation report and gold Box label format (#311) #331's "Nothing is filtered, so no example can leave a denominator" and "a mean or rate whose
  denominator is zero is NaN ..., never 0" are the same contract transition_metrics states for
  n_resolved / n_examples and its NaN means.
• Both sections fix the unit of an example explicitly and for the same reason — Foundation: Box evaluation report and gold Box label format (#311) #331: "one example is
  one image, not one tile"; transitions: "the function scores one (label, time) per example. Split a
  sequence into one example per transition". A learner meets the same discipline twice, worded
  consistently.
• All internal links in both merged documents resolve: I checked every relative ](...) link in
  docs/vision.md and docs/evaluation.md against the headings of its target — 0 broken files, 0
  broken anchors. Foundation: Box evaluation report and gold Box label format (#311) #331's new links (vision.md#the-localization-report,
  vision.md#tiled-and-hierarchical-recipes, fixtures.md#box-labels) all resolve.

The edited PR body against the diff

The orchestrator's edit fixes both of my body-level complaints:

• The frame_windows bullet now reads "Every frame lies in some window when stride_seconds <= width_seconds; with a larger stride the windows are deliberately sparse and frames between them are
  in no window (documented with an example, tested)" — the overclaim is gone and matches the code and
  docs exactly.
• "How it was verified" now reports this head ("Final head 42dc36a ..., Linux, fresh Python 3.14
  venv") instead of the superseded 6b18804, the stale Windows-failure list is gone, and the
  "docs/vision.md blocks were executed by hand" claim is replaced by "tests/test_docs_examples.py
  executes the docs/evaluation.md blocks and the docs/vision.md frame walkthrough", which is true.
• Every number in it reproduces on my own run: ruff format --check 399 files, check_hygiene 608
  files, check_recipe_assets 461 files, notebook_ci fixtures 51 folders / 0 failed, and pytest
  5547 passed, 49 skipped, 50 deselected, 31 xfailed, 0 failed — identical to my own counts.
• Closes #309 is correct for a PR meant to close the issue; no other closing keyword appears.

Two observations on the body, neither blocking:

• The "What this adds" tests bullet still describes only the block-count bump ("8 to 9 blocks, empty
  preamble") and now understates the diff: tests/test_docs_examples.py also gained a test that
  executes docs/vision.md's walkthrough and a test that ties the docstring count to
  len(BLOCKS), and tests/test_frames.py gained the two coverage-regime tests. The body is not
  wrong — "How it was verified" mentions the walkthrough test — just thinner than the diff in the one
  place a reader looks for the test story.
• CONTRIBUTING.md says a description is "written once, at open ... and never edited after the final
  push", with fix-round notes going in pull-request comments, because an edited event fires a fresh
  Scope run that can leave an ambiguous duplicate row. I checked: this head carries exactly one
  Scope row, and gh pr checks 323 reports 60 of 60 pass, none pending or failed, so the edit
  caused no merge-gate ambiguity in practice. Worth recording in a comment rather than repeated edits
  next time; it does not affect mergeability here. (Also, folding the round-3 fixes under the "Review
  round 2 resolutions" heading makes the paper trail read as if the qualification landed a round
  earlier than it did. Content is accurate; only the attribution of rounds is loose.)

Status of my previous suggestions (all were non-blocking; the builder's report did not claim them)

#	Suggestion	Status at 42dc36a
1	docs/evaluation.md:19-33 central "Undefined is NaN" table not extended for transition_metrics	Open, and I now withdraw the framing: #331's own "Box localization" section (:739-769) likewise states its NaN rule locally without adding table rows, so "The cases:" being non-exhaustive is a repo-wide convention question, not this PR's defect. Leave it to a docs sweep.
2	docs/evaluation.md:3-4 "plain numpy, imports nothing from the answer classes or the SDK" vs evaluation.py:35 from .frames import TIME_EPSILON, _seconds; and _seconds being private	Open. Still literally true, still worth a clause, still worth promoting _seconds.
3	docs/vision.md:3 "Import these helpers from jev_cookbook.vision" is no longer true of the whole document	Open. The frames section's own disclaimer (:339-341) remains the mitigation.
4	The fields example (docs/vision.md:433-443) drops the relative image path that :57-60 requires in fields	Open (only the every_seconds value changed). Now slightly more visible, because #331 added a whole shared gold-label format one screen above.
5	Say that frame_index / seconds_ago enter the replay_key	Open (no "replay" in the frames section).
6	Two unreflowed long lines	Half resolved. frames.py now has no line over 100 chars. docs/evaluation.md:810 is still 134 ("Do not substitute the pair's start or midpoint: the three give"), and docs/vision.md:377-378 is still ragged mid-paragraph.
7	tests/test_frames.py:296 section banner reading # ----- round 2	Open. A review round still means nothing to a later reader; the new tests were appended without adding another round banner, which is the right instinct.
8	Anchorless cross-links	Open, and now the odd ones out: docs/vision.md:424 links [evaluation.md](evaluation.md) and docs/evaluation.md:795 links [What an evaluation must show](multimodal-track.md), while every link #331 added carries an anchor. evaluation.md#transitions and multimodal-track.md#what-an-evaluation-must-show both exist. Matters more now that docs/evaluation.md is ~900 lines.
9	earliest_transition returns a truthy dict for a review as well as a find; show the if found is None: ... elif found["review"]: ... pattern	Open.

New, non-blocking, surfaced only by the #331 merge

1. There is now a shared gold-label format for Box recipes and none for multiframe recipes. Foundation: Box evaluation report and gold Box label format (#311) #331
   added docs/fixtures.md "### Box labels" explicitly "so ten recipes do not invent ten formats",
   with vision.gold_box and validator enforcement. The ~10 multiframe recipes have the same problem:
   docs/evaluation.md:791-879 defines the gold shape as (label, seconds) "as a tuple or a
   two-element list (what JSON loads)", but the Transitions section contains no link to
   docs/fixtures.md at all and fixtures.md has no transition-label section, so each recipe will
   choose its own labels.jsonl encoding. Out of Foundation: no deterministic helpers or evaluation for ordered frames (sampling, intervals, relative times, windows, transitions) #309's scope — but it is the same risk Foundation: Box evaluation report and gold Box label format (#311) #331 just
   spent a PR removing for Box, and it is now visibly asymmetric inside the same two documents. Worth
   a follow-up issue before rank 85 is opened, not a change here.
2. A learner meets two different "gold labels" in docs/vision.md. Foundation: Box evaluation report and gold Box label format (#311) #331's ### Gold labels
   (:160) is the Box label format; the frames section's "Gold labels use the same rule" (:422) is
   the transition-time convention. Neither is wrong and the frames sentence is explicit about which
   rule it means, but one clause ("gold transition labels") would stop the two from cross-wiring in a
   skim.
3. docs/vision.md:412 states the coverage guarantee without the docstring's [start, end]
   qualifier. frames.py:290 says "every frame in [start, end]"; the bullet says "Every frame
   lies in some window only when stride_seconds <= width_seconds". With explicit bounds, frames
   outside them are in no window either — frame_windows([0..4], width=1, stride=1, start=1, end=3)
   returns windows covering only indices 1-3, as the docstring (correctly) implies. Two words in the
   bullet would make the page match the docstring.

None of these is a false statement in the shipped text, which is why none is a must-change.

Checks I ran

Fresh venv outside the repository (uv venv --python 3.14 /home/agent/r2b-venv, then
uv pip install --link-mode symlink -e ".[dev]" → ruff 0.17.1, pytest 9.1.1), Linux, Python 3.14.7.
Install and all checks in a single flock .claude/heavy.lock nice -n 10 acquisition (no nested flock),
run in the background with log polling; pytest -p no:cacheprovider. No TYPESAFE_API_KEY, no live
call, no credential looked for. HEAD was re-printed inside the script as 42dc36a6a09be3946....

Command	Result
uv pip install -e ".[dev]"	rc 0
ruff check .	All checks passed (rc 0)
ruff format --check .	399 files already formatted (rc 0) — the run that covers the docs/*.md python blocks
python tools/check_hygiene.py	Hygiene check passed: 608 file(s) scanned (rc 0)
python tools/check_recipe_assets.py	Image-asset check passed: 461 file(s) under recipes/ checked (rc 0)
python tools/render_catalog.py --check	rc 0, no output
python tools/notebook_ci.py fixtures	51 recipe folders, 0 failed (rc 0)
pytest tests/test_docs_examples.py tests/test_frames.py tests/test_evaluation.py -q -rs	343 passed, 3 skipped in 2.1 s (rc 0) — 4 more than the 339 at 4f338f9, exactly the four new tests
pytest -q (full, tests/ + recipes/)	5547 passed, 49 skipped, 50 deselected, 31 xfailed, 0 failed in 2 min 35 s (rc 0)
Mutation probes (6, one at a time, tree restored)	all killed; baseline 5 passed
gh pr checks 323	60 of 60 pass, none pending or failed; exactly one Scope row

Read rather than taken on trust:

• The three skips are not the block-execution tests. -rs names them: :280, :296, :305, all
  "exercises the Windows-only SIGBREAK timer path". So test_block_executes ran for every
  docs/evaluation.md block, block 8 (Transitions) included, and the new docs/vision.md walkthrough
  test ran too. The 50 deselected are the default catalog_audit deselection, unrelated to this diff.
• docs/evaluation.md still has exactly 9 python blocks after Foundation: Box evaluation report and gold Box label format (#311) #331, so the module's
  assert len(BLOCKS) == 9 and PREAMBLE[8] still refer to the Transitions block; Foundation: Box evaluation report and gold Box label format (#311) #331's Box
  localization section added prose and a table, no fence.
• git status --porcelain empty after the full run and after the mutation script — nothing wrote into
  the checkout, and I restored every mutated file.
• No notebook executed and none needed to be: git grep over recipes/ finds no import of
  jev_cookbook.frames or transition_metrics, and the diff touches no notebook, fixture or recipe
  folder.

Reproductions of my own (standard library only, src on PYTHONPATH, scripts under /tmp, nothing
written into the checkout): the frame_windows sparse-regime counterexample; the 20,000-trial
randomized coverage probe described under M1; the fenced-block marker uniqueness check on the merged
docs/vision.md; and a link/anchor resolver over every relative link in both merged documents.

Acceptance criteria of #309, one by one

1. sample_frames(...) deterministic, tie-breaking documented, edge tests for empty, single, exact
   boundaries and float drift — met. (Unchanged this round; at jev_cookbook.frames.sample_frames,
   a deviation documented as the one import path and accepted by both reviewers. In my previous review
   I re-derived the count and every_seconds rules against independent references with no
   mismatch.)
2. frame_intervals / relative_frames (adds seconds_ago, order unchanged), tested — met.
3. window_timestamps returns start, end and duration, with tests — met. Its worked example in the
   docs now runs and is executed by a test (M2), so the one defect I held against this criterion last
   round is gone.
4. transition_metrics with exact and within-tolerance accuracy, an explicit uncertain/review count
   with a stated denominator, and earliest-transition error in seconds; documented in
   docs/evaluation.md; tested with hand-computed cases — met. Untouched this round; the block is
   still executed by tests/test_docs_examples.py and still agrees with my own arithmetic.
5. docs/vision.md documents sampling, that no video decoding happens, and how a recipe records the
   selected timestamps in fields — met. This is the criterion I marked not met at 4f338f9; both
   reasons (the false frame_windows guarantee and the non-running walkthrough) are fixed, each with a
   test, and the section reads correctly beside Foundation: Box evaluation report and gold Box label format (#311) #331's new Box material.
6. No new dependency; Pillow only — met. pyproject.toml untouched (Pillow is already a core
   dependency); frames.py is standard-library only.

Verdict

Every must-change item is resolved, by the honest route in each case: the coverage claim was scoped to
the regime where it is true and the counterexample documented rather than quietly dropped; the docs
walkthrough was made to run on the page's own data and is now executed by a test; and the block count
is now derived from len(BLOCKS) instead of restated in prose. I verified each with my own
reproduction and killed every one of six mutations aimed at the new guards, including two that restore
the exact bugs I reported. The merge of #331 is a provably clean auto-merge, the scope and dependency
position are unchanged, the edited PR body now matches the diff and the head, every check I ran is
green, and all 60 CI checks pass on this exact head. The remaining items are the nine non-blocking
suggestions carried over (one of which I withdraw as a repo-wide convention question, and half of
another is now fixed) plus three small new ones surfaced by reading the merged documents; none is a
false statement in shipped text, and none should hold up a foundation interface that sixteen recipes
are waiting on.

OPUS-REVIEW: APPROVE 42dc36a

[CompleteDotTech]

PR #323 re-review - reviewer 1 of 2 (round 3)

• Agent ID: agent-a8934111fcb6837d9 (worktree .claude/worktrees/agent-a8934111fcb6837d9)
• Resolved model ID: claude-opus-5[1m] (Opus 5, 1M context)
• Head reviewed: 42dc36a6a09be3946ef35e11bb0cbb9fe7841075 (branch foundation/frame-helpers)
• Base reviewed against: 2601074bc1ff20f640e33d3141fa6d6bd5deb6b0 (origin/main, also the merge-base)
• Previously reviewed by me: 4f338f92fb4bced5a06b6cbed76f69597bdc40f8
  (.claude/reviews/pr323-r1.md, CHANGES-NEEDED on one finding, M1)
• I checked the head out myself and did not take the builder's report on trust; every status below
  is from the code, the tests I ran, or a probe.

What changed since the head I reviewed

Two commits:

• a9c7922 "frames: state frame_windows coverage only for stride <= width; make the vision
  walkthrough run and be tested (Foundation: no deterministic helpers or evaluation for ordered frames (sampling, intervals, relative times, windows, transitions) #309)" - 4 files, +124/-17: docs/vision.md,
  src/jev_cookbook/frames.py, tests/test_docs_examples.py, tests/test_frames.py.
• 42dc36a, the merge of origin/main 2601074 (Foundation: Box evaluation report and gold Box label format (#311) #331, Box evaluation report and gold Box label
  format) into a9c7922.

The merge is clean and contributed nothing of its own. git show --cc 42dc36a prints the commit
header and no diff hunks at all. Both sides of the merge touched docs/vision.md,
docs/evaluation.md and tests/test_vision.py, which is the interesting part this time, and the
result is intact in both directions: git diff origin/main HEAD -- docs/evaluation.md and
... -- docs/vision.md each contain zero deletion lines, so none of #331's new Box prose, its
### Box localization section (now at docs/evaluation.md:739, sitting immediately before this
PR's ### Transitions at 791) or its two new docs/vision.md python blocks was lost or displaced.
The whole-PR diff against origin/main is 7 paths, all mode 100644, 2 added:

:100644 100644 M docs/evaluation.md
:100644 100644 M docs/vision.md
:100644 100644 M src/jev_cookbook/evaluation.py
:000000 100644 A src/jev_cookbook/frames.py
:100644 100644 M tests/test_docs_examples.py
:100644 100644 M tests/test_evaluation.py
:000000 100644 A tests/test_frames.py

No symlink, submodule, mode change or type change; nothing outside src/, docs/ and tests/;
pyproject.toml still untouched, so still no new dependency. The merge commit verifies as a good
signature (%G? = G).

One merge hazard that came out right and is worth recording. tests/test_docs_examples.py
asserts len(BLOCKS) == 9 for docs/evaluation.md, and #331 added 32 lines to that file. I counted
the fenced python blocks at this head: still exactly 9, so the assertion and the new prose-count
guard both hold. Had #331 added a block, this merge would have produced a red Tests job rather
than a silent drift, which is the behaviour that assertion exists for.

Reading the merged docs/vision.md as a learner. The page is now titled "Local images, boxes and
ordered frames" and runs: image assets and root= -> boxes -> tiles -> ## Explicitly timestamped still frames -> ### Sampling, intervals and windows -> ## Honest accounting, with #331's Box
evaluation blocks later. The frames section reads correctly in that new neighbourhood. Two specific
things I checked because #331's material could have broken them:

• The load_frames example passes literal relative paths with no root=, while the top of the page
  says a path "that comes out of data ... must be loaded with root=". There is no contradiction: the
  sentence directly above that rule says "Literal paths in your own code can be passed as written",
  and the frames example is literal paths. A learner reading top to bottom gets the right rule.
• load_frames( now appears twice in the page, but only once inside a fenced python block (line 319;
  the other, line 35, is prose about root=), so the new test's "exactly one block contains this
  marker" assertion still resolves to the intended block - and it asserts the count, so an
  ambiguity introduced later fails loudly instead of silently picking the wrong block. Same for the
  other two markers (frame_timestamps(frames) at 362, "sampling" at 442: one each).

---

My findings from 4f338f9, one by one

M1 (must-change): frame_windows stated a coverage guarantee it did not provide - RESOLVED

The fix is a documentation and test change with the behaviour deliberately unchanged, and it is
accurate everywhere I could check it.

• src/jev_cookbook/frames.py:288-294: the unconditional "so every frame lies in some window" is
  gone, the misleading parenthetical "overlapping the previous window by more than the stride
  implies" is gone, and there is now a dedicated Coverage. paragraph: "With
  stride_seconds <= width_seconds every frame in [start, end] lies in at least one window. With a
  larger stride the windows are deliberately sparse: frames between one window's end and the next
  window's start lie in no window, the final end-aligned window closes only the gap at the tail,
  and nothing is raised. Choose stride_seconds <= width_seconds when every frame must be scored."
  That is exactly right, including the [start, end] scoping I raised, and the "frames between one
  window's end and the next window's start" phrasing is careful enough to stay true in the case where
  a stride slightly above the width happens to lose nothing.
• frames.py:315: the code comment "no frame is left out" is now "final window aligned to the end of
  the range".
• docs/vision.md:301-310 carries the same qualification in bold plus the worked counterexample -
  frames every second to 10 s, width 1, stride 5, windows [0,1], [5,6], [9,10], "so frames 2, 3,
  4, 7 and 8 are in none, and nothing is raised" - and the practical instruction "Use a stride no
  larger than the width when every frame must be scored".
• grep for the old claims across src/, docs/ and tests/ returns nothing, so no stale copy
  survives.
• Behaviour is byte-for-byte unchanged: the only non-docstring edit to frames.py is that one
  comment.

The claim is now proven by tests in both regimes, which was the second half of my finding:

• tests/test_frames.py:477-500, test_frame_windows_cover_every_frame_when_stride_is_at_most_width:
  pins the stride == width boundary explicitly (windows [0,2] [2,4] [4,6] [6,8] plus the
  end-aligned [7,9], full coverage of 19 frames), then sweeps every (width, stride) pair from
  width in (0.5, 1, 1.5, 2, 3) x stride in (0.5, 0.7, 1, 1.5, 2, 3) with stride <= width,
  asserting full coverage both with default bounds and with start=1.0, end=8.5. The bounded case
  asserts set equality with set(range(2, 18)), so it also catches over-coverage outside
  [start, end], not only gaps.
• tests/test_frames.py:503-519,
  test_frame_windows_with_a_stride_wider_than_the_width_leave_frames_out: pins the documented
  counterexample's windows, its indices, and set(range(11)) - covered == {2, 3, 4, 7, 8}, plus a
  second, tighter overshoot (width 0.5, stride 1.5 on a half-second grid) whose uncovered set is
  [2, 5, 8, 11, 14, 17].

I reproduced all of it independently rather than reading the assertions: the documented
counterexample gives exactly [2, 3, 4, 7, 8]; the tighter case gives exactly
[2, 5, 8, 11, 14, 17]; stride == width leaves nothing uncovered across 7 widths x 6 grid sizes;
and 6000 randomised stride <= width configurations, 40 % of them with explicit start/end,
produced zero coverage gaps and zero out-of-order window starts.

Do I accept qualifying the claim rather than rejecting stride > width? Yes. My finding was that
the contract was false, not that the behaviour was wrong, and either fix closes it. Qualifying is the
better of the two here, for four reasons: (a) stride > width is a legitimate sampling intent
(deliberately sparse windows), and _seconds/_ordered's "raise instead of repairing" stance is
about invalid input, which this is not; (b) the behaviour is now stated accurately, with a worked
counterexample and an explicit "nothing is raised", at the exact place a builder reads before
choosing a stride; (c) both regimes are pinned by tests, so the contract is proven rather than
asserted; and (d) rejecting it now would be a breaking change for the three existing tests that
depend on it - I verified that by actually inserting the rejection
(if stride > width + TIME_EPSILON: raise), which makes
test_frame_windows_keeps_empty_windows_visible, test_frame_windows_float_drift_and_errors and
test_frame_windows_with_a_stride_wider_than_the_width_leave_frames_out fail. The one residual risk
I named - a swapped width/stride typo silently dropping frames - is now the first thing the
docstring's Coverage paragraph and the bolded docs sentence warn about, which is as much as
documentation can do for a configuration that is also a valid choice.

S7 (frames.py:289 was 148 characters) - RESOLVED

The docstring was reflowed as part of M1. No line in src/jev_cookbook/frames.py,
tests/test_frames.py or tests/test_docs_examples.py now exceeds the project's 100-column
line-length (checked with awk 'length>100'; ruff does not select E501, so this needed checking
by hand).

S8 (docs/vision.md's python blocks were executed by no test) - RESOLVED for the frames path

tests/test_docs_examples.py:354-392 adds test_vision_frame_walkthrough_runs_on_the_pages_own_frames,
which generates three 280x280 PNGs at the paths the page names (280x280 is exactly the #321
minimum area, so this also stays inside the image contract), chdirs to them, and execs the
load_frames block, the frames walkthrough block and the fields block in page order in one
namespace, then asserts every value the page's comments now state: times == [0.0, 0.75, 1.5],
chosen == [0, 1, 2], frame_index == [0, 1, 2], gaps == [0.75, 0.75],
seconds_ago == [1.5, 0.75, 0.0], the window dict, and the recorded fields["sampling"]. The
walkthrough itself was also fixed to be runnable and meaningful: it now says in prose that it
continues from the frames loaded above, every_seconds changed from 2.0 (which on a 1.5 s
sequence selects one frame and makes window_timestamps(sampled, 1, 2) raise) to 0.5, and each
comment carries the real value instead of a placeholder description. _vision_block asserts that
exactly one block matches each marker, so the harness cannot silently bind to the wrong block as the
page grows. #331's own Box blocks in docs/vision.md are still not executed, which is outside this
PR.

Not resolved (all were non-blocking suggestions I did not expect to be taken in this round)

• S1 - src/jev_cookbook/evaluation.py:35 still imports the private frames._seconds.
• S2 - tolerance_correct is still cumulative while outcomes' "within" is exclusive; the
  TransitionMetrics docstring still does not say tolerance_correct >= exact_correct outright.
• S3 - clusters of three or more frames inside TIME_EPSILON still break "nearest"
  (sample_frames([0.0, 1.0, 1.0+3e-10, 1.0+6e-10, 2.0], every_seconds=1) == [0, 2, 4]);
  docs/vision.md still scopes the guarantee to two such frames, which I accept.
• S4 - sample_frames([5.0, 6.0], start=9.0) still says "start must not be after end".
• S5 - relative_frames still silently replaces an existing seconds_ago.
• S6 - sample_frames(state_frames) still raises without pointing at frame_timestamps.
• S9 - Foundation: no deterministic helpers or evaluation for ordered frames (sampling, intervals, relative times, windows, transitions) #309's checklist still names vision.sample_frames etc.; the shipped path is
  jev_cookbook.frames.*. Amend the issue text; no code change wanted.

None of these blocks the merge. They are worth a single tidy-up issue if the orchestrator wants them,
and S1/S2 are the two I would pick.

One new wording nit (optional, not a finding)

docs/vision.md:303-304 says "Every frame lies in some window only when
stride_seconds <= width_seconds." Read as a statement about the guarantee ("coverage is only
guaranteed when ..."), which is clearly the intent and is reinforced by the counterexample and the
"use a stride no larger than the width" instruction right after it, this is right. Read as a
biconditional it is slightly too strong: a stride just above the width need not actually lose a
frame (width=1.0, stride=1.0+1e-6 over one-frame-per-second data covers everything, since the
sub-microsecond gaps contain no frame). frames.py's own Coverage paragraph is phrased precisely and
has no such reading. "Every frame is guaranteed to lie in some window only when ..." would close it.
Not worth a round on its own.

---

Everything else, re-verified at this head

The rest of the diff is unchanged from 4f338f9, which I reviewed in full; I re-ran my probes
against this head rather than assuming. sample_frames still matches a brute-force
nearest/earliest-within-slack reference; the count round-half-down integer formula is unchanged;
transition_metrics' denominators (n_resolved vs n_examples, n_timed vs n_timed_any_label,
NaN at zero, math.fsum means) are unchanged and still each pinned by a hand-computed test; the
transition-time convention, the undetermined-pair review path and the max_sampling_interval_seconds
flag are unchanged. Nothing under recipes/ imports jev_cookbook.frames or transition_metrics,
so no notebook is affected and none needed executing.

Guard instruments: four mutations, all killed

Run one at a time against this head, each file restored from a copy afterwards (git status --porcelain empty at the end, confirmed):

Mutation	Result
docs/vision.md walkthrough every_seconds=0.5 -> 2.0	killed: test_vision_frame_walkthrough_runs_on_the_pages_own_frames fails (ValueError: first_index 1 is outside 0..0). The new docs test really does execute the page, not just parse it.
frames.py drops the end-aligned window (if high - (starts[-1] + width) > TIME_EPSILON: -> if False:)	killed, 3 failures: test_frame_windows_cover_the_end_of_the_sequence, test_frame_windows_cover_every_frame_when_stride_is_at_most_width, test_frame_windows_with_a_stride_wider_than_the_width_leave_frames_out.
tests/test_docs_examples.py docstring "Two of the nine blocks" -> "eight"	killed: test_docstring_block_count_matches_the_assertion fails. The new prose-count guard works.
frames.py made to reject stride > width (the alternative fix)	killed, 3 failures: test_frame_windows_keeps_empty_windows_visible, test_frame_windows_float_drift_and_errors, test_frame_windows_with_a_stride_wider_than_the_width_leave_frames_out. Evidence that allowing a sparse stride is now a tested contract, not an accident.

---

Acceptance criteria of #309

1. sample_frames(...) deterministic, tie-breaking documented, edge tests for empty, single, exact
   boundaries and float drift - met (module path jev_cookbook.frames, a documented deviation I
   accept; S9).
2. frame_intervals / relative_frames (adds seconds_ago, order unchanged), both tested -
   met, and now also executed through the docs/vision.md walkthrough test.
3. window_timestamps returns start, end and duration, with tests - met, likewise.
4. transition_metrics with exact and within-tolerance accuracy, an explicit uncertain/review count
   with a stated denominator, and earliest-transition error in seconds, documented in
   docs/evaluation.md and tested with hand-computed cases - met.
5. docs/vision.md documents sampling, that no video decoding happens (pre-extracted stills), and
   how a recipe records the selected timestamps in fields - met. This was the one criterion I
   marked "met in substance, one statement wrong" last round; the wrong statement is now correct,
   qualified and tested, and the walkthrough a recipe author copies is executed in CI.
6. No new dependency; Pillow only - met. frames.py is standard library only;
   pyproject.toml unchanged.

---

Checks run

Linux, Python 3.14.7, fresh uv venv /home/agent/r1b-venv (outside /workspace) with
uv pip install --link-mode symlink -r <file naming -e .[dev]>, numpy 2.5.4, Pillow 13.0.0, ruff
0.17.1 (the pinned version, from the venv). No TYPESAFE_API_KEY, no live call, no network use by
any check. Each heavy command ran under its own flock .claude/heavy.lock nice -n 10 (no nesting;
the wait for the queued install was done outside the lock), and pytest with -p no:cacheprovider.

• ruff check . - All checks passed.
• ruff format --check . - 399 files already formatted.
• pytest -q -p no:cacheprovider - 5547 passed, 49 skipped, 50 deselected, 31 xfailed, 0
  failed in 162 s. (The 50 deselected are pyproject.toml's default catalog_audit deselection.
  No failures at all, including the two tests/test_notebook_ci_sandbox.py signal.SIGKILL cases
  that fail on the author's Windows machine - this run is on Linux.)
• pytest -q -rs tests/test_frames.py tests/test_evaluation.py tests/test_docs_examples.py tests/test_vision.py - 610 passed, 3 skipped. The three skips are
  tests/test_docs_examples.py:280/296/305, all "exercises the Windows-only SIGBREAK timer path;
  POSIX uses SIGALRM", so both test_block_executes and the new
  test_vision_frame_walkthrough_runs_on_the_pages_own_frames actually ran here.
• python tools/check_hygiene.py - Hygiene check passed: 608 file(s) scanned.
• python tools/render_catalog.py --check - rc 0.
• python tools/notebook_ci.py fixtures - 51 recipe folders, 0 failed.
• git show --cc 42dc36a - no diff hunks. git diff 4f338f9 a9c7922 - the 4-file fix above.
  git diff origin/main HEAD -- docs/evaluation.md docs/vision.md - zero deletion lines.
  git diff --raw -M origin/main...HEAD - the 7 100644 paths above.
• Notebook execution: nothing under recipes/ (51 folders) imports jev_cookbook.frames or
  transition_metrics, and the diff touches no recipe, so no notebook is affected and none was
  executed.
• Independent probes: the documented stride > width counterexample and the tighter overshoot; 6000
  randomised stride <= width configurations with and without explicit bounds; stride == width
  across 7 widths x 6 grid sizes; a stride a hair above the width; plus the four mutations above.
• git status --porcelain empty at the end: the mutation runs left the tree clean.

---

Verdict

M1 is resolved properly - the false guarantee is replaced by an accurate, scoped one in both the
docstring and docs/vision.md, with a worked counterexample, and both regimes are now pinned by
tests that I confirmed fail when the behaviour is mutated. I accept the choice to qualify the claim
rather than reject stride > width. The vision walkthrough now runs and is executed in CI (which
also closes my S8), the long docstring line is gone (S7), the merge of #331 lost nothing in either
doc, and every check from docs/development.md is green on this exact head. My six remaining items
are non-blocking suggestions for a later tidy-up issue.

OPUS-REVIEW: APPROVE 42dc36a