Speed up reel encoding (the dominant cost) and skip a duplicate intro render

[honzajavorek]

Performance audit

Profiled a full crowing run with cProfile against the local tests/fixtures/handbook-git.html fixture (2 paragraphs → 4 slides, reel ≈ 35s). Breakdown before this PR:

Step	Time
render_section (images)	0.73s
render_reel (reel frames)	0.81s
write_images (PNGs)	0.14s
write_carousel (PDF)	0.19s
write_reel (the MP4)	11.6s
Total	13.4s

write_reel alone is 86% of total runtime, even on this tiny 2-paragraph fixture. It's not a serial-vs-parallel issue (the steps are inherently sequential — each depends on the previous one's output) — it's that the video encoding was doing far more work than it needed to.

Root cause

write_reel() built the silent video by converting every frame to a numpy array and calling imageio's ffmpeg writer's append_data() once per output frame — including duplicates. A 3s hold at 30fps meant 90 separate Python→pipe round-trips of ~6.2MB of raw RGB pixels each, just to show the same static image. For a real handbook section (more paragraphs, up to the 90s reel cap) this scales linearly and easily explains a run "taking a minute".

Fix

write_reel() now writes each unique frame to disk once, then makes a single ffmpeg call: -loop 1 -framerate <fps> -t <duration> -i <frame> per slide, joined with the concat filter, encoded at the veryfast x264 preset. ffmpeg handles holding/duplicating frames internally in C instead of Python pushing raw pixels through a pipe one frame at a time. veryfast is safe quality-wise here because slideshows of static, hard-cut slides compress extremely well at any preset — the visual difference is negligible, the speed difference isn't.

Measured on the same fixture: write_reel drops from 11.6s → ~8s (verified frame-accurate: correct frame count, correct total duration, and correct slide shown at each timestamp — checked by sampling frames at multiple points in the video and comparing background colors against the expected slide).

I also explored an even faster approach (ffmpeg's concat demuxer with a duration directive directly on each PNG, skipping per-frame decode entirely — measured ~5s instead of ~8s), but it produced incorrect output durations at low frame rates (verified by writing/running tests at fps=5, which the test suite already exercises for speed) due to an ffmpeg quirk in how the demuxer's implicit per-image framerate interacts with the requested duration. Given a wrong-length reel is worse than a slower one, I shipped the slower-but-verified-correct version. Worth revisiting if someone wants to dig deeper into that ffmpeg behavior.

Secondary fix: duplicate intro render

render_section() (for the square Instagram images) and render_reel() (for the reel) both called render_intro(section.title, section.heading) with identical arguments, redoing the same font-fitting search for a pixel-identical image. render_reel() now accepts an optional already-rendered intro image, and cli.py passes in the one render_section() already produced.

Net result

Same fixture, full pipeline: 13.4s → ~9.4s (~30% faster), with the win growing for longer reels since the removed per-frame Python/pipe overhead scales with frame count while everything else stays roughly fixed.

Suggestions not implemented here (for consideration)

• fit_words/fit_intro/fit_cloud/fit_plain in rendering.py pick the largest font size that fits by scanning sizes linearly in steps of 2 (~50 candidate sizes), each doing a full text-measurement pass. Since "fits at size N" is monotonic, this is a textbook binary-search candidate (~6 iterations instead of ~50). Didn't change it here since it's a smaller win than the video fix and I wanted to keep this PR's diff verifiable; flagging it as a follow-up.
• Rendering the independent slides (intro/paragraphs/CTA) with multiprocessing was considered but isn't worth it for typical section sizes (a handful of slides) — process spawn/pickle overhead would likely cancel out the gain, and it'd complicate the "functional core" simplicity the README calls for.

Test plan

• uv run pytest — all 97 tests pass, including the reel duration/dimensions/music tests
• make smoke — full real run against the live junior.guru git handbook page produces images, carousel.pdf, and reel.mp4 correctly
• Manually verified the re-encoded video's frame count, fps, and total duration match expectations, and that the correct slide appears at multiple sampled timestamps

---

Generated by Claude Code

[coderabbitai]

Warning

Review limit reached

@honzajavorek, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 24 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: abaf356c-b5c8-4f6d-982e-919435f9bb12

📥 Commits

Reviewing files that changed from the base of the PR and between 8e487a8 and dae4bb3.

📒 Files selected for processing (1)

• tests/test_rendering.py

📝 Walkthrough

Walkthrough

render_reel now accepts an optional intro image and uses it for the first slide when provided; the CLI passes images[0] into that argument. A new test checks that the supplied intro image is reused.

write_reel now builds the reel through a temporary-directory ffmpeg flow. It writes a silent MP4 via _encode_silent, muxes music with _mux_music, adds a REEL_PRESET constant, and removes the prior imageio.v2 and numpy-based encoding path.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main performance and duplicate-intro changes in the PR.
Description check	✅ Passed	The description directly explains the reel-encoding optimization and intro reuse, matching the changeset.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/crowing-performance-audit-0ql14m

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/jg/crowing/writing.py`:
- Around line 53-57: The temporary silent MP4 is being created in output_dir,
which can leave a stray .reel-silent.mp4 behind on mux failure and can collide
across concurrent runs. Update the writing flow in _encode_silent and the
surrounding tempfile.TemporaryDirectory block so silent is created inside
tmp_dir, then call _mux_music before the temporary directory context exits and
remove the separate silent.unlink cleanup.
- Around line 74-81: The frame export logic in writing.py silently truncates
when `paths` and `durations` differ, while the ffmpeg concat input still assumes
every frame exists. Update the frame-building flow around the `zip(paths,
durations)` usage to fail fast with an explicit length check, or switch to
`zip(..., strict=True)` if supported, so the `concat` input stays consistent
with `frames`.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5b437518-abe6-409f-bb89-0d12a0906cf2

📥 Commits

Reviewing files that changed from the base of the PR and between 3191017 and 6803a92.

📒 Files selected for processing (3)

• src/jg/crowing/cli.py
• src/jg/crowing/rendering.py
• src/jg/crowing/writing.py

[honzajavorek]

Verified the core fix is correct (ran the full test suite + ruff locally against this branch, 97 passed, no new lint issues). Two small follow-ups left as inline comments: missing test coverage for the new render_reel(intro=...) parameter, and a comment that references the PR description instead of stating its rationale inline.

---

Generated by Claude Code

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/test_rendering.py`:
- Around line 325-329: The test currently only checks a yellow pixel in the
first frame, which does not prove the fallback intro renderer was skipped.
Update test_render_reel_reuses_a_given_intro_instead_of_rendering_one to patch
jg.crowing.rendering.render_intro, call render_reel with an intro image, and
assert render_intro is not called when intro is provided. Keep the existing
render_reel and intro setup, but make the assertion target the render_intro call
behavior rather than pixel color alone.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 19039097-1ede-4e5c-9d82-44fa7770eee4

📥 Commits

Reviewing files that changed from the base of the PR and between 6803a92 and 8e487a8.

📒 Files selected for processing (2)

• src/jg/crowing/writing.py
• tests/test_rendering.py

🚧 Files skipped from review as they are similar to previous changes (1)

• src/jg/crowing/writing.py