Skip to content

#486 stage A: the join_into_song task #513

Closed

Description

@dkackman

Stage A of #486. Filed by the lead agent, model claude-opus-5-5 via anthropic, decompose session.

Plan: plan v1, "Stages → Stage A" and "Design". The plan comment is authoritative; this body restates it.

Builds

One new task, join_into_song (dw/tasks/join_into_song.py, registered in dw/tasks/task.py), reusing concat_videos' frame/shot machinery and normalize_audio's pyloudnorm measurement.

argument type meaning
dialogue list of videos Spoken shots, in order; each keeps its own audio.
song_shots list of videos Sung shots, in order; their audio is discarded.
song audio The unbroken track the song shots were sliced from.
cue_seconds number ≥ 0, default 0 Song time that lands on song_shots[0] frame 0.
dialogue_target_lufs number, optional Static gain per dialogue shot to this integrated loudness (measure, then apply). Omitted = no matching.
duck_delay_ms ≥ 0, default 0 Delay after the song enters before the dialogue ducks.
duck_db ≤ 0, default −12 Duck depth.
duck_ramp_ms ≥ 0, default 250 Linear ramp into the duck.

Behaviour:

  • Picture = dialogue + song_shots at the dialogue's fps; mismatched fps or size is refused, as concat_videos refuses it.
  • D = measured length of the joined dialogue in samples (at the song's rate after resampling). The song is placed at D − cue_seconds·sr and runs to the end of the picture.
  • cue_seconds longer than the dialogue: directed error.
  • Song shorter than the song shots need: pad with silence, warn song_short.
  • A silent dialogue shot is filled with silence (not skipped, so the concat_videos desync the plan's sweep found is not inherited).
  • A dialogue shot under 400 ms is left unmatched with a warning rather than failing.
  • Mix = loudness-matched dialogue, ducked from song_entry + duck_delay with a linear ramp, plus the placed song. Song-shot audio contributes nothing.
  • No final normalization (Q3 default): the existing warn_without_headroom / warn_if_written_above_full_scale apply.
  • Shots: one record per input, named shot@<key> from the step's references as concat_videos names them, samples measured off the built waveform.
  • Result video/mp4, muxed by the existing path.
  • Empty dialogue or song_shots is refused.

Owes

  • Domains in dw/task_domains.py: cue_seconds ≥ 0, duck_delay_ms ≥ 0, duck_ramp_ms ≥ 0, duck_db ≤ 0.
  • An EXPECTED_SITES entry ("populates") in tests/test_shots.py.
  • Unit tests on synthetic clips: song landing sample equals D − cue·sr exactly; duck starts at the right sample and ramps linearly; song-shot audio absent from the mix; per-shot LUFS within 0.5 LU; shot map partitioned and measured; every refusal.
  • A docs/TASKS.md section.

Deploy

Server.

Passes when

The plan's stage A acceptance intent reproduces over MCP (a 2-dialogue + 2-song-shot join with cue_seconds > 0 validates, runs, lands in list_gallery / export_job; get_job_workflow shows every input; media.shots lists 4 named shots; the song's cue onset falls on the first song frame within one frame; the duck is ≈ duck_db; analyze_sync_drift clean; the refusals and edge cases hold) and every pending: #<this issue> case the tester writes passes.

Activity

  1. 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 Sep 27, 2026
  2. added a commit that references this issue on Sep 27, 2026
  3. added and removed
    owner:leadFeature lead's turn (harnest R11)
    on Sep 27, 2026
  4. dkackman commented on Sep 27, 2026

    @dkackman
    OwnerAuthor

    Shipped — lead agent, claude-opus-5-5 via anthropic.

    Branch feat/486-a-join-into-song merged into develop @ a5345da; deployed: [deploy 23:43:35] deployed develop @ a5345da (healthy, mcp=true).

    Plan items covered (stage A)

    • New task join_into_song (dw/tasks/join_into_song.py), registered in dw/tasks/task.py. Args dialogue, song_shots, song, cue_seconds (default 0), dialogue_target_lufs (optional, one static gain per shot, measure-then-apply), duck_delay_ms (0), duck_db (−12), duck_ramp_ms (250, linear).
    • Picture = dialogue frames then song-shot frames; mismatched frame size or fps refused. The song goes in at D − round(cue_seconds·sr) and runs to the end of the picture. A cue longer than the dialogue is a directed error. A song that's too short is padded with silence and warns song_short. A silent dialogue shot is filled with silence. A shot under 400 ms (or one that can't be measured) is left at its own level and warns dialogue_unmatched. The song shots' own audio is dropped. There is no final normalization. Empty dialogue/song_shots are refused.
    • Shots: one record per input, named shot@<key> from the step's dialogue then song_shots references. Samples are measured off the built waveform, and every shot after the first has hard_cut. Result video/mp4.
    • Domains in dw/task_domains.py: cue_seconds, duck_delay_ms, duck_ramp_ms NON_NEGATIVE, duck_db NON_POSITIVE. EXPECTED_SITES has ("dw/tasks/join_into_song.py", "join_into_song"): ("populates", 1).
    • Unit tests tests/test_join_into_song.py (31) run on synthetic clips. They cover: exact landing sample, duck start and linear ramp, song-shot audio absent, per-shot LUFS within 0.5 LU, shot map partitioned and measured, and every refusal. docs/TASKS.md has a join_into_song section.

    Deviations from the plan

    1. New optional fps argument (domain POSITIVE). A pipeline's frames carry no frame rate, and the song is placed from the dialogue's length in time, so without it a join of pipeline results can't run. When the inputs carry a rate (files do) it's taken from them. If none does and fps isn't given, the join is refused.
    2. How D is measured. The plan says to measure D off the joined dialogue. Here, each dialogue shot's track is fitted to its own frame span first: trimmed, or padded with silence. Then D is the sample the first song-shot frame sits at. Without the fit, a dialogue track that overran its frames would push the song off the song shots' picture by that overrun. A fit of at least one frame warns dialogue_fitted_to_frames; a smaller fit is only logged.
    3. Shot naming. dw/workflow.py used to name shots from the step's videos argument only. It now goes through dw.shots.shot_references, which returns videos for concat/dissolve and dialogue + song_shots for this task. Nothing changes for existing tasks.
    4. Channel handling, which the plan didn't specify: the output takes the song's channel count. A mono dialogue track is spread to match; a dialogue track with more channels is cut down to the song's count.

    No existing MCP shape changed: not a breaking change.

  5. dkackman commented on Sep 28, 2026

    @dkackman
    OwnerAuthor

    Verify in progress (tester, claude-opus-5-5 via anthropic) against lem develop @ cdfd02f — resumable trail, not the verdict.

    • C-F151: passes apart from the extra fps parameter (a documented lead deviation; will request a suite amendment on harnest).
    • C-F152: PASS (job 386e1a826821, measured in 2149f39c9c74): 496 f, shots contiguous, duck/ref/song windows as expected, sync drift clean.
    • C-F153: PASS (join 7e2f7cf72544, measure e73dec94c15e). (a) Δ −0.04/−0.04 dB pre-duck, −12.03 dB after; (b) ramp Δ −0.76 → −3.28 → −4.16 → −9.75 (monotone, first ≥ −6.5, last in [−13,−8]); (c) Δ −0.04 dB.
    • C-F154..C-F157: not yet run.
  6. added
    owner:leadFeature lead's turn (harnest R11)
    and removed on Sep 28, 2026
  7. dkackman commented on Sep 28, 2026

    @dkackman
    OwnerAuthor

    Verify result (tester, claude-opus-5-5 via anthropic) against lem develop @ cdfd02f: one acceptance case fails, so this goes back to owner:lead. The pending: #513 lines stay in the suite.

    Failing: C-F156 arm 5 (mixed frame rate)

    • Job 994007e5a344 (run cf156r5/20260928-001532-4ef14e47). I kept it as the repro.
    • Step co12 = pair_audio(video="asset:qa-cast/ep6-cold-open.mp4", audio=<same asset>). Result {"content_type":"video/mp4","fps":12}.
    • get_gallery_metadata on co12's file: fps 12.0, 124 frames, 10.33 s.
    • Step j = join_into_song(dialogue=[incident, reply], song_shots=["previous_result:co12", priya], song="asset:.../ep15-song.mp3").
      • Expected: refused, or failed with no film written, with an error naming 24 and 12.
      • Actual: the job succeeded with no rate warning and wrote cf156r5-j.1-0.0.mp4: fps 24.0, 496 frames, with co12 placed as shot [248,372).
      • The 12 fps shot was silently re-timed as 24 fps.

    Passing

    • C-F151: PASS. The one mismatch is the extra optional fps parameter. That is the plan's lead deviation, not a bug, and I filed a suite amendment on harnest for it.
    • C-F152: PASS (386e1a826821 / measure 2149f39c9c74).
    • C-F153: PASS (7e2f7cf72544 / e73dec94c15e).
      • (a) Δ −0.04/−0.04 dB before the duck, −12.03 dB after.
      • (b) Δ −0.76 → −3.28 → −4.16 → −9.75.
      • (c) Δ −0.04 dB.
    • C-F154: PASS (655dae7f084d / e069be2b3d74).
      • (a) W1 −22.98 LUFS, W2 −22.99 LUFS; peaks −9.4 and −8.1 dBFS.
      • (b) W1 −16.46 vs −16.42; W2 −27.41 vs −27.39.
      • (c) The 0.33 s clip was left at its own level with a warning ("under the 400 ms loudness measurement needs"). W1 −22.98, W2' −22.99, 5 shots.
    • C-F155: PASS.
      • Film 496 f. The silent shot [0,124) measures −114.8 dBFS; [124,236) measures −31.03 vs −31.01.
      • Shots start at 0/124/248/372.
      • The drift step ran clean.
    • C-F156:
      • Arm 1 PASS: 4 errors at steps[0].task.arguments.{cue_seconds,duck_delay_ms,duck_db,duck_ramp_ms}.
      • Arm 2 PASS.
      • Arm 3 PASS: an empty song_shots/dialogue fails at run time, with the argument named.
      • Arm 4 PASS: cue 10.375 fails naming 10.33 s; cue 10.29 gives 496 f.
    • C-F157: PASS.
      • (a) Windows match their sources to within 0.04 dB.
      • (b) A short song pads with silence and warns (tail −120 dBFS).
  8. added and removed
    owner:leadFeature lead's turn (harnest R11)
    on Sep 28, 2026
  9. dkackman commented on Sep 28, 2026

    @dkackman
    OwnerAuthor

    Bounce fixed and redeployed. Lead agent, claude-opus-5-5 via anthropic.

    Branch feat/486-a-join-into-song-fps is merged into develop @ 683a7ac. Deploy output: [deploy 00:21:23] deployed develop @ 683a7ac (healthy, mcp=true).

    C-F156 arm 5 (mixed frame rate). The cause was in the engine, not the join.

    • pair_audio carried the asset's 24 fps on the AudioVideo it returned. The co12 step then wrote the file at result.fps: 12, but it never wrote that rate back onto the in-memory artifact.
    • previous_result:co12 hands that same instance to the next step, so join_into_song saw 24 fps everywhere and joined without complaint. A step-cache hit or an output: reload would already have read 12 off the file.
    • Fix (dw/result.py, save_audio_video): when the step declares result.fps, that rate is now written back onto the artifact, the same way the Cut-based (previous_result) audio chains never get _fit_audio_to_frames, so per-shot lip-sync drift accumulates #197 audio fit already is. join_into_song then refuses with its existing mixed-rate error, which names each input with its rate (…: 24 fps, …: 12 fps). No film is written.
    • Only a declared result.fps is written back. An undeclared one (carried rate, or the default of 8) leaves the artifact as it was.

    Also tightened: join_into_song now refuses an explicit fps that contradicts the rate the videos carry. Before, it silently won. This is the same silent re-timing through the other door. fps stays what deviation 1 said it is: for videos that carry no rate.

    Tests

    • tests/test_result.py: a declared rate is handed on, and an undeclared one isn't written back.
    • tests/test_join_into_song.py:
      • the bounce's real path (a Result saved at fps: 12 from 24 fps frames, then fed to the join) is refused, naming 24 and 12;
      • a contradicting fps is refused;
      • a matching fps is accepted.
    • docs/TASKS.md: the refusal paragraph is updated.
    • Full suite: 6957 passed. ruff is clean.

    Deviation / scope note: the write-back is in Result.save, so it applies to every task that reads a previous_result: video written with result.fps, not only this one. concat_videos/dissolve_videos fed a shot re-rated this way will now see the written rate. They refuse a mismatch where they used to re-time it silently. Before this, a fresh run and a cache hit of the same step handed different rates downstream. No MCP shape changed, so this is not a breaking change.

  10. dkackman commented on Sep 28, 2026

    @dkackman
    OwnerAuthor

    Verified (tester, claude-opus-5-5 via anthropic) against lem develop @ 683a7ac. (This comment replaces a stale copy of the previous bounce text that I posted here by mistake.)

    C-F156 arm 5 (the bounce), re-run exactly as in the repro. co12 = pair_audio(ep6-cold-open, ep6-cold-open) with result.fps: 12, then join_into_song(dialogue=[incident, reply], song_shots=["previous_result:co12", priya], song=ep15-song.mp3) in regression-complete:

    • Job 774e06a3f54f failed. The manifest has only co12's file, so no film was written.
    • Error: join_into_song needs every video at one frame rate, got ep3-shot1-incident.mp4: 24 fps, ep3-shot2-reply.mp4: 24 fps, video 3: 12 fps, ep6-shot1-priya.mp4: 24 fps. … It names 24 and 12, so the arm passes.
    • Nit, not blocking: the re-rated shot is called video 3 here, while the shot map names the same input co12. Naming it after the step would read better.

    Adjacent cases:

    • An explicit fps: 12 on four 24 fps assets: job 4667e76c8547 failed with join_into_song was given fps 12, but the videos carry … 24 fps … 'fps' is only for videos that carry no rate. The manifest is empty.
    • A matching fps: 24 with cue_seconds: 3.0: job 748e5069483b succeeded. It shows 4 shots at frames 0/124/248/372, samples 0/227850/455700/683550, and hard cuts on shots 2–4. The only warning is the expected above-full-scale one.

    The rest of the acceptance. C-F151–C-F155, C-F157 and C-F156 arms 1–4 passed in the previous verify comment against cdfd02f. I didn't re-run them this session. The fix in this round only changes behaviour for a previous_result: video written with a declared result.fps, which none of those cases uses. The matching-fps join above exercises the same all-24 fps path they take. The C-F151 fps-parameter amendment is with the harnest suite request.

    With every pending: #513 case now passing, I'm removing those lines from the suite. All four runs (including the old repro 994007e5a344) were deleted with delete_output(job_id=…).

  11. added
    status:verifiedTester confirmed the fix via a real MCP call
    and removed on Sep 28, 2026
  12. added a commit that references this issue on Sep 28, 2026
  13. added 2 commits that reference this issue on Oct 10, 2026
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