Skip to content

#602 stage A: fit_to_model and restore_to_source tasks #631

Closed
Closed
#602 stage A: fit_to_model and restore_to_source tasks#631
Labels
featureWork bigger than a fix: designed with Don, built in stages (harnest R11)owner:testerTester's turn to actstageA child issue: one stage of a featurestatus:verifiedTester confirmed the fix via a real MCP call

Description

@dkackman
dkackman
opened on Oct 5, 2026

Stage A of #602. Section Stages → Stage A: the two tasks (server) of plan v2; the plan comment stays authoritative, including The design for both tasks' behaviour, record shape and refusals.

Filed by the lead agent, model claude-opus-5-5 via anthropic.

What it builds

  • fit_to_model(video, width, height, num_frames, mode="letterbox") and restore_to_source(video, fit) in dw/tasks/ (video_utils.py or a new fit.py).
    • fit: stretch / crop / letterbox into width×height (bicubic, antialiased); frame count set to exactly num_frames (cut a long source, hold the last frame of a short one). Returns {video, fit}, the record per the plan.
    • restore: scale inferred from video size ÷ model size (must be uniform); letterbox crops the content box and resizes to source × scale; stretch resizes; crop returns the kept source_box at source density × scale; frames trimmed to min(source_frames, len(video)); fps passes through.
    • Refusals before any work: non-positive/non-integer sizes or frame count, unknown mode, empty video, malformed fit record, non-uniform scale. Each names the argument.
  • register_command entries with summaries and parameter descriptions.
  • The mode domain in dw/task_domains.py.

What it owes

  • Unit tests: a round trip per mode on synthetic frames (size, frame count, content box, pixels within tolerance for letterbox and stretch at 1× and 2×); each refusal; discovery (tests/test_task_discovery.py, presence tests).
  • Docs: a docs/TASKS.md Video Processing row for each task, plus a short "round trip" paragraph.

Deploy

Server restart.

Passes when

Its own repro works (the plan's stage A acceptance intent: discovery, letterbox/stretch/crop round trips, inferred 2× scale, long source, refusals), and every pending: #<this issue> case the tester writes passes.

Activity

  1. dkackman
    added
    featureWork bigger than a fix: designed with Don, built in stages (harnest R11)
    owner:leadFeature lead's turn (harnest R11)
    stageA child issue: one stage of a feature
    on Oct 5, 2026
  2. dkackman
    added a parent issue on Oct 5, 2026
  3. dkackman
    marked this as blocking on Oct 5, 2026
  4. dkackman
    added 3 commits that reference this issue on Oct 6, 2026

    feat(fit): #631 - tests for fit_to_model and restore_to_source

    e2d7b81

    docs(fit): #631 - fit_to_model and restore_to_source in TASKS and the…

    79b3ae1

    Merge branch 'feat/602-A-fit-to-model' into develop (#631)

    95fc37f

  5. dkackman commented on Oct 6, 2026

    @dkackman
    dkackman
    on Oct 6, 2026
    OwnerAuthor

    Stage A built, deployed: deployed develop @ 95fc37f1 (merge of feat/602-A-fit-to-model; commits 8ee0089 tasks + domain, e2d7b81 tests, docs commit after). Lem healthy after restart.

    Plan items covered

    • fit_to_model(video, width, height, num_frames, mode="letterbox") and restore_to_source(video, fit) - dw/tasks/fit.py, registered in dw/tasks/task.py with summaries and per-parameter descriptions. Returns {video, fit} (read as previous_result:<step>.video / .fit); fit is a JsonRecord, saved whole as one .json.
    • Modes letterbox (default), stretch, crop; exact num_frames (long source cut to its first frames, short one holds its last). Restore infers a uniform scale from the output's size (2x restores to 2x source), crops letterbox bars, trims to the source frame count; crop returns the kept region (640x480 → 512x288 → 640x360 at 1x, 1280x720 at 2x).
    • Refusals, each naming the argument: non-positive / non-whole width, height, num_frames; unknown mode; empty or non-video video; a malformed fit record (not a dict, missing/wrong-typed field, bad mode, box outside the frame); non-uniform scale (names both sizes). A fit given as a saved .json path goes through validate_media_path + JSON extension/size checks.
    • mode domain in dw/task_domains.py (TASK_ARGUMENT_CHOICES), plus POSITIVE domains for the three sizes; validate_workflow reports a literal unknown mode or a non-whole/non-positive literal size statically.
    • docs/TASKS.md Video Processing rows + a "fit_to_model and restore_to_source" section with the round trip; a docs/ARCHITECTURE.md row.
    • Unit tests tests/test_fit_to_model.py (round trip per mode at 1x/2x, pixel tolerance, frame hold/cut, each refusal, a real dispatch + Result save), discovery presence in tests/test_task_discovery.py, tests/test_shots.py sites. Full suite: 8754 passed.

    Deviations / additions

    • get_task parameters now carry a choices list for any argument with a TASK_ARGUMENT_CHOICES entry (additive; this is how mode's three values show in get_task("fit_to_model")). Not a breaking change.
    • The fit record carries source_box in every mode (the whole source unless crop), alongside content_box.
    • Neither task carries audio, per the plan: both outputs have no soundtrack; pair with pair_audio.

    — claude-opus-5-5 via anthropic (feature lead, build)

  6. dkackman
    added
    owner:testerTester's turn to act
    status:fixed-pending-verifyImplementer fixed, awaiting tester verification
    arch-review
    and removed
    owner:leadFeature lead's turn (harnest R11)
    on Oct 6, 2026

  7. dkackman commented on Oct 6, 2026

    @dkackman
    dkackman
    on Oct 6, 2026
    OwnerAuthor

    Architecture review queued by the loop driver: this hand-off changed dw/ or dw_mcp/ on develop (a83ad1ccb4..95fc37f1cd: dw/introspection.py dw/task_domains.py dw/tasks/fit.py dw/tasks/task.py). The reviewer checks it against docs/ARCHITECTURE.md before the tester verifies.

  8. dkackman
    removed
    arch-review
    on Oct 6, 2026

  9. 5 remaining items

  10. dkackman
    added a commit that references this issue on Oct 6, 2026

    Merge branch 'feat/602-A-fit-to-model-bounce' into develop (#631)

    955c519

  11. dkackman commented on Oct 6, 2026

    @dkackman
    dkackman
    on Oct 6, 2026
    OwnerAuthor

    Bounce fixed, redeployed: deployed develop @ 955c5191 (merge of feat/602-A-fit-to-model-bounce, fix commit 0d9b412). Lem healthy after restart.

    C-F241 crop arm (640x368 instead of 640x360). The restore itself was right: it returned 360 rows. The save added the extra 8. Result wrote frame-only video through diffusers' export_to_video, which defaults imageio's macro_block_size to 16, so ffmpeg scaled any side that wasn't a multiple of 16 up to the next one (360 → 368). Both export_to_video calls in dw/result.py now pass macro_block_size=2 (VIDEO_MACRO_BLOCK), the even sides yuv420p needs. A saved video now keeps its own size, and only an odd side grows by one pixel. docs/TASKS.md's round-trip paragraph says so.

    • New test TestRealPath::test_a_crop_restore_is_saved_at_its_own_size saves the crop restore through Result and reads the written file's header: 640x360, 12 frames. Without the fix it fails with 640x368. Full suite: 8759 passed; ruff clean.
    • Nit: the non-video message now reads "needs 'video' as a video, not AudioVideo".

    Deviation, outside the stage's files: the writer change applies to every frame-only video save, not just these tasks. Any output whose sides aren't multiples of 16 (e.g. 640x360, 960x540) is now written at its true size instead of being upscaled at save. Sizes that are multiples of 16 are unchanged, and the muxed-audio path (encode_video) was already exact. No MCP shape changes.

    — claude-opus-5-5 via anthropic (feature lead, build)

  12. dkackman
    added
    owner:testerTester's turn to act
    status:fixed-pending-verifyImplementer fixed, awaiting tester verification
    arch-review
    and removed
    owner:leadFeature lead's turn (harnest R11)
    on Oct 6, 2026

  13. dkackman commented on Oct 6, 2026

    @dkackman
    dkackman
    on Oct 6, 2026
    OwnerAuthor

    Architecture review queued by the loop driver: this hand-off changed dw/ or dw_mcp/ on develop (6b6da603c1..955c519196: dw/result.py dw/tasks/fit.py). The reviewer checks it against docs/ARCHITECTURE.md before the tester verifies.

  14. dkackman
    removed
    arch-review
    on Oct 6, 2026

  15. dkackman commented on Oct 6, 2026

    @dkackman
    dkackman
    on Oct 6, 2026
    OwnerAuthor

    Architecture review: pass. Read 6b6da603..955c5191 under dw/ (commit 0d9b412: dw/result.py, dw/tasks/fit.py; also its docs/TASKS.md line) against docs/ARCHITECTURE.md and stage-c item 3.

    • Second owner: none. The change stays where video writes already live. dw/writers.py's docstring says the video writes (export_to_video, encode_video) stay in Result, and both edited calls are those writes, in Result._write_video_file's frame-only branch and save_audio_video's no-audio fallback. No other module in dw/ calls export_to_video or sets a macroblock, so no second copy of the even-sides rule exists. The fit.py change only rewords one error message.
    • Build vs. buy: none. The fix passes macro_block_size to diffusers' export_to_video, which hands it to imageio. Both are existing dependencies, and nothing is hand-rolled.
    • Map: no owner was added or moved, so no row is owed.
    • Context: the rationale is in a comment on VIDEO_MACRO_BLOCK in dw/result.py. No CLAUDE.md changed.

    Not an architecture finding, for the tester: as the hand-off says, this changes the saved size of every frame-only video whose sides aren't multiples of 16. That is a behaviour change outside the stage's two tasks.

    — claude-opus-5-5 via anthropic (reviewer, architecture review)

  16. dkackman commented on Oct 6, 2026

    @dkackman
    dkackman
    on Oct 6, 2026
    OwnerAuthor

    Verified: stage A (#631) passes. Run by claude-opus-5-5 via anthropic, against lem on develop @ 955c519, workspace regression-complete.

    The bounced case: C-F241

    I rebuilt the RT workflow inline: fit_to_model → get_dict_value video / fit → restore_to_source. Source asset:qa-fit/src-640x480-50f.mp4, model size 512x288, n 57.

    Crop arm (job 428f22d2297e):

    • Record: source_box {x:0, y:60, w:640, h:360}, content_box full-frame.
    • fitted is 512x288x57 at 24 fps.
    • restore is 640x360, 50 frames, 24 fps. It was 640x368 before the fix.
    • Frames 0 and 25 of restore match the source's central band by eye: the top light and the bottom rows are cut.

    Stretch arm (job c6e133b263ad):

    • fitted is squashed with no bars.
    • restore is 640x480x50 at 24 fps.

    Adjacent case: a frame-only save whose height isn't a multiple of 16

    This checks the writer change outside crop mode (job 8ba36a9b7566). Letterbox from 640x480 to 960x540, n 50:

    • fitted is saved at 960x540, 50 frames. Before the fix it would have been rounded up to 960x544.
    • content_box is {120, 0, 720, 540}.
    • restore is 640x480x50 at 24 fps.

    The other pending cases

    The previous verify (comment above) passed C-F237, C-F238, C-F239, C-F240, C-F242, C-F243, C-F244 and SE-F043 against 95fc37f. I didn't re-run them, for two reasons:

    • This bounce changed only Result's macroblock size and one error message.
    • Every size those cases save is a multiple of 16, so the writer change can't move them.

    I've removed the pending: #631 lines from all nine cases (C-F237–C-F244 and SE-F043), so they're now ordinary regression cases. All three jobs above are deleted.

  17. dkackman
    added
    status:verifiedTester confirmed the fix via a real MCP call
    and removed
    status:fixed-pending-verifyImplementer fixed, awaiting tester verification
    on Oct 6, 2026
  18. dkackman
    closed this as completedon Oct 6, 2026
  19. nikhilash6
    added 2 commits that reference this issue on Oct 10, 2026

    feat(fit): dkackman#631 - fit_to_model and restore_to_source tasks, t…

    8ee0089

    fix(result): dkackman#631 - save a video at its own size, not rounded…

    0d9b412
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    featureWork bigger than a fix: designed with Don, built in stages (harnest R11)owner:testerTester's turn to actstageA child issue: one stage of a featurestatus:verifiedTester confirmed the fix via a real MCP call

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions