render: make subtitle chunking, case and style EDL-driven

[shoaib90]

The problem

Caption chunking and case are hardcoded to 2-word UPPERCASE in build_master_srt, and force_style to a single module constant.

That combination is exactly right for the fast-cut social edit the skill ships as bold-overlay. It is wrong for anything with a narrative read — a documentary or a long-form talking head wants 4–7 word lines in sentence case. SKILL.md even describes a natural-sentence style as something you might invent, but reaching it currently means editing render.py.

Changes

An optional subtitle_style block on the EDL, so a caption style is data rather than a code change:

"subtitle_style": {
  "words_per_chunk": 6,
  "case": "sentence",
  "force_style": "FontName=Helvetica,FontSize=13,Bold=1,PrimaryColour=&H00FFFFFF,OutlineColour=&H00000000,BorderStyle=1,Outline=2,Shadow=1,Alignment=2,MarginV=28"
}

• words_per_chunk — words per line, default 2. Still breaks early on punctuation.
• case — "upper" (default) or "sentence". Sentence case leaves the ASR's own capitalization alone.
• force_style — ASS override string, defaulting to SUB_FORCE_STYLE.

One subtlety worth calling out. In sentence case the ASR's capitalization is correct for a continuation line, but a cut can promote a mid-sentence word to the start of a sentence — which then rendered lowercase. Real example from the edit that prompted this: a segment starting on a restart produced it is to maintain a digital diary. The builder now capitalizes any cue that opens the file or follows a cue ending in sentence-final punctuation.

Also documents something that cost me a render to notice: force_style's MarginV is relative to PlayResY=288, so the shipped default of 90 is tuned for vertical video and sits too high in a 16:9 frame. Around 28 puts the caption roughly 10% up from the bottom in landscape.

Compatibility

Every default reproduces current behaviour. I verified the generated SRT is byte-identical to the previous implementation on the same transcript and EDL with no subtitle_style present, so existing projects are unaffected.

tests/ passes (16 tests, 15 subtests).

Note

Touches build_final_composite's signature and main(), so it overlaps slightly with #158 if both land — whichever merges second needs a trivial rebase.

---

Summary by cubic

Makes subtitle chunking, case, and style EDL-driven via an optional subtitle_style block on the EDL, so a caption style is data instead of a render.py edit. Defaults reproduce the prior 2-word UPPERCASE output byte-identically, and invalid values are now rejected with clear errors.

• words_per_chunk, case, and force_style on the EDL control word grouping, capitalization, and ASS styling; case is case-insensitive and words_per_chunk must be an integer ≥ 1.
• break_on, balance, and min_words control which punctuation forces a break, whether cues are sized evenly, and folding short cues backward, each defaulting to prior behavior.
• force_style's MarginV is relative to PlayResY=288, so the default is tuned for vertical video; 16:9 needs a lower value (documented in SKILL.md).

Bug Fixes

• Capitalizes a sentence-case cue that opens the file or follows sentence-final punctuation, since a cut can promote a mid-sentence word to a sentence start.

^{Written for commit 6d03ec0. Summary will update on new commits.}

[cubic-dev-ai]

3 issues found across 2 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="helpers/render.py">

<violation number="1" location="helpers/render.py:434">
P3: When the EDL supplies a non-integer `subtitle_style.words_per_chunk` (e.g. a string), `int(style.get("words_per_chunk", 2))` raises an unhandled `ValueError` and aborts the whole render. Coerce/validate the value defensively so a malformed EDL can't crash caption building.</violation>

<violation number="2" location="helpers/render.py:501">
P2: Sentence mode fails to capitalize quoted or parenthesized cues and fails to detect sentence punctuation before closing quotes. Inspect the first alphabetic character and allow closing punctuation wrappers when evaluating `prev_text`.</violation>

<violation number="3" location="helpers/render.py:673">
P2: When `force_style` contains an apostrophe, the generated filter graph has an unmatched quote and FFmpeg aborts compositing. Escape the style value before putting it inside the single-quoted `subtitles` option, just as `subs_abs` is escaped.</violation>
</file>

<sub>Tip: instead of fixing issues one by one fix them all with cubic

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: Sentence mode fails to capitalize quoted or parenthesized cues and fails to detect sentence punctuation before closing quotes. Inspect the first alphabetic character and allow closing punctuation wrappers when evaluating prev_text.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At helpers/render.py, line 501:

<comment>Sentence mode fails to capitalize quoted or parenthesized cues and fails to detect sentence punctuation before closing quotes. Inspect the first alphabetic character and allow closing punctuation wrappers when evaluating `prev_text`.</comment>

<file context>
@@ -471,13 +481,29 @@ def build_master_srt(edl: dict, edit_dir: Path, out_path: Path) -> None:
+        recased: list[tuple[float, float, str]] = []
+        prev_text = ""
+        for a, b, t in entries:
+            if t and (not prev_text or prev_text.rstrip()[-1:] in ".!?"):
+                t = t[0].upper() + t[1:]
+            recased.append((a, b, t))
</file context>

Suggested change

	if t and (not prev_text or prev_text.rstrip()[-1:] in ".!?"):
	t = t[0].upper() + t[1:]
	if t and (
	not prev_text
	or re.search(r"[.!?][^\w\s]*$", prev_text.rstrip())
	):
	t = re.sub(
	r"^(\W*)([^\W\d_])",
	lambda m: m.group(1) + m.group(2).upper(),
	t,
	count=1,
	)

[cubic-dev-ai]

P2: When force_style contains an apostrophe, the generated filter graph has an unmatched quote and FFmpeg aborts compositing. Escape the style value before putting it inside the single-quoted subtitles option, just as subs_abs is escaped.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At helpers/render.py, line 673:

<comment>When `force_style` contains an apostrophe, the generated filter graph has an unmatched quote and FFmpeg aborts compositing. Escape the style value before putting it inside the single-quoted `subtitles` option, just as `subs_abs` is escaped.</comment>

<file context>
@@ -643,7 +670,7 @@ def build_final_composite(
         subs_abs = str(subtitles_path.resolve()).replace(":", r"\:").replace("'", r"\'")
         filter_parts.append(
-            f"{current}subtitles='{subs_abs}':force_style='{SUB_FORCE_STYLE}'[outv]"
+            f"{current}subtitles='{subs_abs}':force_style='{force_style}'[outv]"
         )
         out_label = "[outv]"
</file context>

Suggested change

	f"{current}subtitles='{subs_abs}':force_style='{force_style}'[outv]"
	escaped_style = force_style.replace("\\", "\\\\").replace("'", "\\'")
	filter_parts.append(
	f"{current}subtitles='{subs_abs}':force_style='{escaped_style}'[outv]"
	)

[cubic-dev-ai]

P3: When the EDL supplies a non-integer subtitle_style.words_per_chunk (e.g. a string), int(style.get("words_per_chunk", 2)) raises an unhandled ValueError and aborts the whole render. Coerce/validate the value defensively so a malformed EDL can't crash caption building.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At helpers/render.py, line 434:

<comment>When the EDL supplies a non-integer `subtitle_style.words_per_chunk` (e.g. a string), `int(style.get("words_per_chunk", 2))` raises an unhandled `ValueError` and aborts the whole render. Coerce/validate the value defensively so a malformed EDL can't crash caption building.</comment>

<file context>
@@ -419,10 +419,20 @@ def _words_in_range(transcript: dict, t_start: float, t_end: float) -> list[dict
+    Output times are computed as word.start - segment_start + segment_offset.
     """
+    style = edl.get("subtitle_style") or {}
+    words_per_chunk = max(1, int(style.get("words_per_chunk", 2)))
+    case_mode = str(style.get("case", "upper")).lower()
     transcripts_dir = edit_dir / "transcripts"
</file context>

Suggested change

	words_per_chunk = max(1, int(style.get("words_per_chunk", 2)))
	raw = style.get("words_per_chunk", 2)
	try:
	words_per_chunk = max(1, int(raw))
	except (TypeError, ValueError):
	words_per_chunk = 2

[shoaib90]

Pushed one more commit adding three keys to subtitle_style: break_on, balance and min_words. Happy to split it back out if you'd rather keep this PR to its original scope.

The motivation is a real edit rather than a hypothetical. Using this PR as it stands on a 3m34s personal-essay talking head, the closest I could get to a documentary caption was {words_per_chunk: 6, case: "sentence"}, which gave 115 cues including twelve one-word and fourteen two-word ones:

6                          7
00:00:11,099 --> 00:00:11,739    00:00:15,659 --> 00:00:15,979
I mean                           now.

That happens because the break fires on every character in .,!?;:, so a mid-sentence comma splits a phrase wherever it lands, and whatever is left of a sentence after the last full chunk goes out alone.

With {words_per_chunk: 8, break_on: ".!?", balance: true, min_words: 3} the same transcript gives 95 cues, nothing under three words, and a 5–7 word spread instead of 38 cues pinned at the cap with a tail out to 11.

All three default to today's behaviour. I checked that two ways on the same 30-segment EDL — a subtitle_style that omits the new keys, and no subtitle_style at all — and both produce byte-identical SRTs to this branch before the commit (115 and 305 cues).

One note in case it is useful to a reviewer: my first attempt at balance was a no-op. It ran after the greedy loop had already capped every chunk at words_per_chunk, so ceil(len/cap) was always 1. A unit test caught it. It now splits the punctuation-delimited runs before the cap is applied, which is what the description claims.

Also worth flagging: this touches build_master_srt, as does #161 (a caption-offset bug, independent of chunking). Whichever lands second needs a trivial rebase.

[cubic-dev-ai]

2 issues found across 2 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="helpers/render.py">

<violation number="1" location="helpers/render.py:538">
P2: When `min_words > 1`, a short cue after a punctuation-delimited run is appended to the preceding chunk, so `break_on` no longer forces a break and captions can span sentences. Apply the minimum within each run, or refuse the merge when the preceding chunk ends with configured break punctuation.</violation>
</file>

<file name="tests/test_render_subtitle_chunking.py">

<violation number="1" location="tests/test_render_subtitle_chunking.py:47">
P3: No test asserts output for the default path. Every cues() call that produces and compares captions passes `case: \"sentence\"` (lines 47, 52, 56, 57, 64); the one call without it (line 72) only asserts a raised ValueError. The file docstring and PR both claim the default (`upper`, no subtitle_style) reproduces the previous shipped captions byte-identically, but nothing verifies that. A regression in the default upper/2-word path (the mode all real EDLs without subtitle_style use) would slip through this suite. Add a test calling cues(None) or cues({}) and asserting the uppercase 2-word output.</violation>
</file>

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic</sub>

[cubic-dev-ai]

P2: When min_words > 1, a short cue after a punctuation-delimited run is appended to the preceding chunk, so break_on no longer forces a break and captions can span sentences. Apply the minimum within each run, or refuse the merge when the preceding chunk ends with configured break punctuation.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At helpers/render.py, line 538:

<comment>When `min_words > 1`, a short cue after a punctuation-delimited run is appended to the preceding chunk, so `break_on` no longer forces a break and captions can span sentences. Apply the minimum within each run, or refuse the merge when the preceding chunk ends with configured break punctuation.</comment>

<file context>
@@ -478,21 +500,46 @@ def build_master_srt(edl: dict, edit_dir: Path, out_path: Path) -> None:
+        if min_words > 1:
+            merged: list[list[dict]] = []
+            for chunk in chunks:
+                if merged and len(chunk) < min_words:
+                    merged[-1].extend(chunk)
+                else:
</file context>

Suggested change

	if merged and len(chunk) < min_words:
	if merged and len(chunk) < min_words and (merged[-1][-1].get("text") or "").strip()[-1:] not in break_on:

[cubic-dev-ai]

P3: No test asserts output for the default path. Every cues() call that produces and compares captions passes case: \"sentence\" (lines 47, 52, 56, 57, 64); the one call without it (line 72) only asserts a raised ValueError. The file docstring and PR both claim the default (upper, no subtitle_style) reproduces the previous shipped captions byte-identically, but nothing verifies that. A regression in the default upper/2-word path (the mode all real EDLs without subtitle_style use) would slip through this suite. Add a test calling cues(None) or cues({}) and asserting the uppercase 2-word output.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_render_subtitle_chunking.py, line 47:

<comment>No test asserts output for the default path. Every cues() call that produces and compares captions passes `case: \"sentence\"` (lines 47, 52, 56, 57, 64); the one call without it (line 72) only asserts a raised ValueError. The file docstring and PR both claim the default (`upper`, no subtitle_style) reproduces the previous shipped captions byte-identically, but nothing verifies that. A regression in the default upper/2-word path (the mode all real EDLs without subtitle_style use) would slip through this suite. Add a test calling cues(None) or cues({}) and asserting the uppercase 2-word output.</comment>

<file context>
@@ -0,0 +1,76 @@
+    def test_default_breaks_on_every_comma(self):
+        # the shipped behaviour: 2-word cues, and the comma after "five" forces a
+        # break that a phrase-aware read would not want
+        c = self.cues({"case": "sentence"})
+        self.assertEqual(c, ["If you", "are between", "eighteen and", "twenty five",
+                             "can I", "ask you", "a question?"])
</file context>