Skip to content

fix(cli): preserve media after the last transcript word - #5325

Open
user-github-me wants to merge 16 commits into
heygen-com:mainfrom
user-github-me:fix/transcript-cut-source-duration
Open

user-github-me wants to merge 16 commits into
heygen-com:mainfrom
user-github-me:fix/transcript-cut-source-duration

Conversation

@user-github-me

@user-github-me user-github-me commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

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:

  • Updated against current main and regenerated the seven-skill manifest.
  • CLI rebuild and typecheck passed; the workspace was built before testing.
  • All 18 cut-list and transcript-cut tests passed, including real FFmpeg output with a retained ending and an explicit post-transcript range.
  • Lint, formatting, test reachability, manifest freshness, normal commit hooks and both comment checks passed.
  • Mint documentation validation and broken-link checks passed.

@jrusso1020

Copy link
Copy Markdown
Collaborator

Thanks for this, @user-github-me. This PR now has merge conflicts with main. Could you rebase it onto the latest main? Once it's conflict-free we'll review it and get it into the merge queue.

@mintlify

mintlify Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

Preview approval required

This pull request comes from a fork, so Mintlify won't build a preview until someone with write access approves commit e75f102.

Approve preview

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
miguel-heygen added this pull request to the merge queue Oct 25, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Oct 25, 2026

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 25, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Oct 25, 2026
@jrusso1020

Copy link
Copy Markdown
Collaborator

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

@github-actions

github-actions Bot commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

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 keep-open to exempt it.

@github-actions github-actions Bot added the stale label Nov 9, 2026

This branch has not been deployed

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants