Skip to content

fix(timeline): preflight exact-frame placements - #355

Open
Flandern1211 wants to merge 1 commit into
hypit-ai:mainfrom
Flandern1211:fix/timeline-placement-preflight
Open

Flandern1211 wants to merge 1 commit into
hypit-ai:mainfrom
Flandern1211:fix/timeline-placement-preflight

Conversation

@Flandern1211

@Flandern1211 Flandern1211 commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Reuse the existing exact rational duration-to-frame rule to check Timeline end and Take at during author decoding when the Clock has an inline value.
  • Keep Build-time validation for runtime-only Clocks, whose effective frame rate is not yet known at check time.
  • Add regression coverage for valid and fractional absolute/relative placements, common frame rates, NTSC, and runtime-only Clocks.

Why

While producing a video, I asked the Agent to record the duration of each core phase (check, plan, and build) because the workflow felt slow. That investigation found invalid Timeline placements which passed check and plan but failed in @hypit/timeline-author@1#assemble-timeline during Build. This PR moves the same deterministic error to check; it does not automatically repair the authored Timeline.

End-to-end Agent recovery

To measure the user-facing workflow instead, two independent Agents started from identical project inputs and ran the official version and this PR in parallel. Runtime was prepared before timing. Each Agent interpreted the actual CLI error, manually corrected the SVML, and continued until a full Build completed; no scripted repair or shortened render was used. Benchmarks used upstream b00532e and PR a3525e on the same workstation.

Input Official: Agent start → successful Build PR #355: Agent start → successful Build Observed difference
Source A, 30 fps / 862 frames, end="28.769002s" (run 1) 306.695 s 251.871 s 54.824 s faster
Source A, same input (independent repeat) 341.934 s 282.780 s 59.154 s faster
Source B, 30 fps / 602 frames, reported end="20.066667s" 354.288 s 233.524 s 120.764 s faster
Valid control, 30 fps / 543 frames, end="18.1s" 661.328 s 665.787 s 4.459 s slower; both built on the first attempt

For both invalid sources, the official version first reported the frame-boundary error in Build, whereas this PR reported it in the initial author check. The new Source A repeat and Source B runs each avoided an extra run check, plan, and failed Build, totaling 14.503 s and 15.405 s of measured CLI work respectively. The larger end-to-end differences also contain Agent decision time and successful-render variance, so they are observations, not a guaranteed or purely code-attributable speedup. Final SVML and MP4 hashes matched between versions within each pair; decoded output contained the full 862, 602, or 543 frames respectively. Source media are not included in this PR, and these selected cases do not estimate real-world error frequency.

Design notes

A statically available Clock gives author decoding enough information to apply the same exact-frame rule as Build. A runtime-only Clock does not, so this change deliberately leaves that path deferred to Build. Valid exact-frame placements and existing semantic Timeline assembly remain unchanged.

Fixes #354

Validation

  • pnpm check — passed.
  • node --import tsx --test packages/timeline-author/test/*.test.ts — 10 passed.
  • Local Windows pnpm test — 1040 passed, 31 skipped, 7 failed. The failing cases were not compared against an unchanged-main baseline, so their cause is not established here.
  • Current PR CI: Ubuntu/Windows checks and packaging jobs passed.

Review checklist

  • Scope limited to Timeline authoring and its tests.
  • No model/provider behavior or protocol contract changed.
  • No credentials or generated media included.

@rponeawa

Copy link
Copy Markdown
Member

Thanks for contribution, I will check this now

@Flandern1211
Flandern1211 force-pushed the fix/timeline-placement-preflight branch from b81b7f7 to a3525e7 Compare September 28, 2026 06:42
@Flandern1211

Copy link
Copy Markdown
Contributor Author

Hi @rponeawa, thanks for saying you would take a look at this PR. I've updated it with end-to-end Agent tests that start from identical invalid inputs, continue through manual correction, and end with a successful Build. Have you had a chance to review it? Are there any concerns blocking the merge?

I understand that this change moves an input error from Build to check; it does not fix a case that would otherwise go undetected. If you consider this use case low priority, see overlap with planned Timeline work, or think the implementation or evidence needs changes, I would appreciate your guidance. I can revise or close this PR accordingly. Thanks.

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Preflight Timeline placements at exact Clock frame boundaries

2 participants