Repository navigation
Add ImplRef.time_varying and Step/LookPlan.time_varying (#17) - #19
Merged
Merged
Conversation
ImplRef.timeline answers "can this be gated to a span", which is close to the inverse of "does this fragment read the clock": the one genuinely clock-reading implementation (motion.ffmpeg.crop, whose zoompan/crop expressions reference in_time) has no timeline support, while every static grade has it and never reads the clock. Step.time_varying ORs ImplRef.time_varying with "this step carries an at Span that actually bounds something" (mirroring gated()'s own enable= decision), and LookPlan.time_varying aggregates across steps — the question muvid#73 needs answered per compiled fragment. Closes #17 Claude-Session: https://claude.ai/code/session_016kZAXr4o6ED292Tkiqbumr
Addresses cn-video-gen-0907's review of PR #19: 1. time_varying is now Optional[bool] = None. None means "undeclared", never "does not read the clock" -- the same posture as DependencyReport.can_flicker. register_defaults' add() helper makes the kwarg required (no default) so every built-in implementation states it explicitly; Step/LookPlan.time_varying propagate unknown as None, with a certain True from a span-gate or a clock-reading impl winning over an unknown sibling. 2. Added a completeness sweep (no built-in impl ships unknown) and a measurement cross-check against frame_dependency.classify for every registered ffmpeg implementation. One genuine finding from running it: classify's TIME_VARYING probe is a spatially uniform still, so it cannot see motion.ffmpeg.crop's moving window (time_delta=0.0 measured) and misclassifies it CONTENT_ADAPTIVE -- a blind spot in that probe's still, not a wrong declaration (confirmed by compiling the effect and reading LookPlan.time_varying directly). Documented and excluded from the sweep rather than silently skipped. 3. looks.ffmpeg.gated() now shares Span.is_whole with Step.time_varying's own gating check, instead of re-deriving the same "does this Span bound anything" rule by hand. 4. Updated docs/decisions_and_rationale.md's ImplRef listing and CLAUDE.md's types paragraph to mention time_varying alongside timeline. Closes #17 Claude-Session: https://claude.ai/code/session_016kZAXr4o6ED292Tkiqbumr
Addresses cn-video-gen-0907's second pass on PR #19: 1. test_time_varying.py: hard-assert set(looks.effects()) == set(PARAMS) (mirroring test_ffmpeg.py's own sweep guard) instead of a per-impl pytest.skip that would let a future effect drop out of the measurement sweep silently. 2. Replace exact time_delta == 0.0 with <= NOISE_FLOOR, so the assertion reddens the day frame_dependency's probe stops missing the moving window, not the day a measured value moves by rounding noise. 3. Filed #20 for the classify blind spot found while building this sweep, and named it in frame_dependency.py's own "What this does not answer" list so the module with the gap documents it. Closes #17 Claude-Session: https://claude.ai/code/session_016kZAXr4o6ED292Tkiqbumr
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ImplRef.timelineanswers "can this be gated to a span" — close to the inverse of "does this fragment read the clock" (motion.ffmpeg.crophas no timeline support and is the one implementation that reads it; every static grade has timeline support and never reads it).ImplRef.time_varying: bool = False, declared per implementation (motion.ffmpeg.cropis the onlyTrue).Step.time_varying— ORs the implementation's own flag with "this step carries anatSpan that actually bounds something" (mirrorslooks.ffmpeg.gated'senable=decision).LookPlan.time_varying— aggregates across steps, the question a consumer (muvid#73) needs answered per compiled fragment.Closes #17
Test plan
python -m pytest -q --doctest-modules looks— 1333 passed, 3 skipped (Homebrew ffmpeg)PATH=/opt/homebrew/opt/ffmpeg@6/bin:$PATH(what CI runs) — 1333 passed, 3 skippedruff check looks/ruff format --check lookscleanlooks/tests/test_time_varying.pycoversImplRef/Step/LookPlanin isolation and end to end against the real registry (motion vs. static grade, and a static grade gated to a span)https://claude.ai/code/session_016kZAXr4o6ED292Tkiqbumr