Repository navigation
fix(cli): preserve media after the last transcript word - #5325
user-github-me wants to merge 16 commits into
Conversation
|
Thanks for this, @user-github-me. This PR now has merge conflicts with |
Preview approval requiredThis pull request comes from a fork, so Mintlify won't build a preview until someone with write access approves commit |
miguel-heygen
left a comment
There was a problem hiding this comment.
Thanks for rebasing this. The fix is in the right place: when --input is given, the measured source duration goes into compileCutList, so removing a filler keeps the footage after the last word and a --keep range past the last word is no longer clamped away. Without --input, the planner still falls back to the last word's end and needs no ffprobe, as the help text and the skill reference now say.
One behavior change worth knowing about, and I think it is the right call: --plan with --input now probes the file, so a plan run with a wrong input path fails instead of silently ignoring it. You updated the one test that relied on the old behavior, and nothing else in the repo calls the script that way.
The new tests run the script itself on a real six-second tone and check the segments, the kept and total durations, the plan output and that the last second still has sound, which is exactly the reported case. CI has no failures on this head and the branch merges cleanly with main.
Verdict: APPROVE
Reasoning: Small, correct fix at the one place the duration is chosen, tested through the real script with real audio, with no conflicts and no failing checks.
miguel-heygen
left a comment
There was a problem hiding this comment.
Re-checked at bcab75f4. Since my approval at 3a3991bc, the new commits are merges from main and a regenerated media-use hash in skills-manifest.json. The script change, the reference and the tests are byte-identical to what I approved, the branch merges cleanly with current main, and there are no failing checks on this head.
As on #5324: both PRs rewrite the same media-use hash line in skills-manifest.json, so whichever merges second will conflict there and need the manifest regenerated once more. Landing them back to back avoids another rebase round.
Verdict: APPROVE
Reasoning: Only merges and a regenerated manifest hash since the approved head; the fix is unchanged and no checks fail.
|
This was approved, but it conflicts with main again, most likely the skills manifest hash. Could you rebase on current main, regenerate the manifest, and push? We'll re-approve at the new head. — Rames |
|
This PR has had no activity for 14 days, so it is marked stale and will close in 7 days. Push a commit or leave a comment to keep it open. A maintainer can add |
Transcript cuts use the input media's measured duration when planning kept segments. Removing a filler from a six-second source whose last word ends at four seconds now preserves the two-second ending. Explicit kept ranges after the last word also remain valid. Transcript-only planning still works without media tools and uses the last word's end.
The media guide and skill reference explain when to include the source while reviewing a cut plan.
Validation:
mainand regenerated the seven-skill manifest.