Skip to content

Commit 6ffb1e8

Browse files
cursoragentjmjava
andcommitted
Fix multiple open issues: quality mapping, binary discovery, freeze guard, scene lint, configurable timeouts, stale VHS detection, layout validation
Addresses the following issues: - #6: Manim quality mapping now supports arbitrary <height>p<fps> strings (e.g. 1080p30) with explicit --resolution/--frame_rate flags. Unknown quality strings produce a clear warning and fall back to 720p30. - #7: manim_runner checks the active venv bin/ directory before PATH. VHS runner checks ~/go/bin, /usr/local/bin, /snap/bin as fallbacks. Both print actionable install instructions on failure. - #8: Pipeline generate-all now retries Manim rendering (with cache clear) when compose fails with FREEZE GUARD, fixing the chicken-and-egg timing data problem on first runs. - #9/#10/#4: New scene_lint module and 'docgen scene-lint' CLI command detect weight=BOLD (Pango font substitution) and positional color args to Text() (cryptic ValueError). Wired into validate as a soft check. - #13: ffmpeg timeout is now configurable via compose.ffmpeg_timeout in docgen.yaml (default 600s). Used by compose, manim_runner, and VHS. - #12: Compose warns when a .tape file is newer than its rendered mp4, configurable via compose.warn_stale_vhs (default true). - #2: Layout validation (overlap/edge detection from manim_layout.py) is now wired into the validator for Manim-type segments. - Config: Added manim_font, ffmpeg_timeout, compose_warn_stale properties. Compose path resolution is quality-aware and searches multiple quality directories including the no-scenes/ variant. All 100 tests pass. Ruff lint clean. Co-authored-by: John Menke <jmjava@gmail.com>
1 parent 23f39b6 commit 6ffb1e8

11 files changed

Lines changed: 668 additions & 27 deletions

File tree

src/docgen/cli.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,29 @@ def lint(ctx: click.Context, segment: str | None) -> None:
191191
raise SystemExit(1)
192192

193193

194+
@main.command("scene-lint")
195+
@click.pass_context
196+
def scene_lint(ctx: click.Context) -> None:
197+
"""Lint Manim scene files for known pitfalls (weight=BOLD, positional color args)."""
198+
from docgen.scene_lint import lint_scene_dir
199+
200+
cfg = ctx.obj["config"]
201+
results = lint_scene_dir(cfg)
202+
203+
if not results:
204+
click.echo("[scene-lint] No issues found")
205+
return
206+
207+
for r in results:
208+
click.echo(f" {r.path}")
209+
for issue in r.issues:
210+
click.echo(f" {issue}")
211+
212+
total = sum(len(r.issues) for r in results)
213+
click.echo(f"\n[scene-lint] {total} issue(s) in {len(results)} file(s)")
214+
raise SystemExit(1)
215+
216+
194217
@main.command()
195218
@click.option("--config-name", "concat_name", default=None, help="Concat config name.")
196219
@click.pass_context

src/docgen/compose.py

Lines changed: 59 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -223,14 +223,63 @@ def _find_audio(self, seg_id: str) -> Path | None:
223223

224224
def _manim_path(self, vmap: dict[str, Any]) -> Path:
225225
src = vmap.get("source", "")
226-
return self.config.animations_dir / "media" / "videos" / "scenes" / "720p30" / src
226+
base = self.config.animations_dir / "media" / "videos" / "scenes"
227+
return self._resolve_quality_path(base, src)
227228

228229
def _vhs_path(self, vmap: dict[str, Any]) -> Path:
229230
src = vmap.get("source", "")
230-
return self.config.terminal_dir / "rendered" / src
231+
rendered = self.config.terminal_dir / "rendered" / src
232+
if self.config.compose_warn_stale:
233+
self._check_stale_vhs(src, rendered)
234+
return rendered
235+
236+
def _check_stale_vhs(self, source_name: str, rendered: Path) -> None:
237+
"""Warn if a .tape file is newer than its rendered mp4."""
238+
if not rendered.exists():
239+
return
240+
stem = Path(source_name).stem
241+
terminal_dir = self.config.terminal_dir
242+
for tape in terminal_dir.glob(f"*{stem}*.tape"):
243+
if tape.stat().st_mtime > rendered.stat().st_mtime:
244+
print(
245+
f" WARNING: {tape.name} is newer than {rendered.name} — "
246+
f"run 'docgen vhs' to re-render before composing."
247+
)
248+
break
249+
250+
def _resolve_quality_path(self, base: Path, source: str) -> Path:
251+
"""Find the rendered Manim file, trying the configured quality first,
252+
then falling back to any available quality directory."""
253+
from docgen.manim_runner import ManimRunner
254+
preferred = ManimRunner(self.config).quality_subdir()
255+
256+
candidate = base / preferred / source
257+
if candidate.exists():
258+
return candidate
259+
260+
for subdir in ("1080p60", "1080p30", "1440p60", "720p30", "480p15", "2160p60"):
261+
candidate = base / subdir / source
262+
if candidate.exists():
263+
if subdir != preferred:
264+
print(
265+
f" NOTE: using {subdir}/{source} "
266+
f"(configured quality {preferred} not found)"
267+
)
268+
return candidate
269+
270+
no_scenes = base.parent / preferred / source
271+
if no_scenes.exists():
272+
return no_scenes
273+
for subdir in ("1080p60", "1080p30", "1440p60", "720p30", "480p15", "2160p60"):
274+
no_scenes = base.parent / subdir / source
275+
if no_scenes.exists():
276+
return no_scenes
277+
278+
return base / preferred / source
231279

232280
def _resolve_source(self, source: str) -> Path:
233-
manim_path = self.config.animations_dir / "media" / "videos" / "scenes" / "720p30" / source
281+
base = self.config.animations_dir / "media" / "videos" / "scenes"
282+
manim_path = self._resolve_quality_path(base, source)
234283
if manim_path.exists():
235284
return manim_path
236285
vhs_path = self.config.terminal_dir / "rendered" / source
@@ -254,13 +303,16 @@ def _probe_duration(path: Path) -> float | None:
254303
except (ValueError, subprocess.TimeoutExpired, FileNotFoundError):
255304
return None
256305

257-
@staticmethod
258-
def _run_ffmpeg(cmd: list[str]) -> None:
306+
def _run_ffmpeg(self, cmd: list[str]) -> None:
307+
timeout = self.config.ffmpeg_timeout
259308
try:
260-
subprocess.run(cmd, check=True, capture_output=True, text=True, timeout=300)
309+
subprocess.run(cmd, check=True, capture_output=True, text=True, timeout=timeout)
261310
except FileNotFoundError:
262311
print(" ERROR: ffmpeg not found in PATH")
263312
except subprocess.CalledProcessError as exc:
264313
print(f" ERROR: ffmpeg failed: {exc.stderr[:300]}")
265314
except subprocess.TimeoutExpired:
266-
print(" ERROR: ffmpeg timed out")
315+
print(
316+
f" ERROR: ffmpeg timed out after {timeout}s. "
317+
f"Increase compose.ffmpeg_timeout in docgen.yaml for long scenes."
318+
)

src/docgen/config.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,22 @@ def manim_scenes(self) -> list[str]:
8888
def manim_quality(self) -> str:
8989
return self.raw.get("manim", {}).get("quality", "720p30")
9090

91+
@property
92+
def manim_font(self) -> str:
93+
return self.raw.get("manim", {}).get("font", "Liberation Sans")
94+
95+
# -- Compose / ffmpeg ------------------------------------------------------
96+
97+
@property
98+
def ffmpeg_timeout(self) -> int:
99+
"""Timeout in seconds for ffmpeg subprocess calls."""
100+
return int(self.raw.get("compose", {}).get("ffmpeg_timeout", 600))
101+
102+
@property
103+
def compose_warn_stale(self) -> bool:
104+
"""Warn during compose if a VHS .tape file is newer than its rendered mp4."""
105+
return bool(self.raw.get("compose", {}).get("warn_stale_vhs", True))
106+
91107
# -- Validation ------------------------------------------------------------
92108

93109
@property

src/docgen/manim_runner.py

Lines changed: 95 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,25 @@
22

33
from __future__ import annotations
44

5+
import re
6+
import shutil
57
import subprocess
8+
import sys
9+
from pathlib import Path
610
from typing import TYPE_CHECKING
711

812
if TYPE_CHECKING:
913
from docgen.config import Config
1014

15+
_PRESET_FLAGS: dict[str, list[str]] = {
16+
"480p15": ["-ql"],
17+
"720p30": ["-qm"],
18+
"1080p60": ["-qh"],
19+
"2160p60": ["-qp"],
20+
}
21+
22+
_CUSTOM_RE = re.compile(r"^(\d+)p(\d+)$")
23+
1124

1225
class ManimRunner:
1326
def __init__(self, config: Config) -> None:
@@ -24,28 +37,99 @@ def render(self, scene: str | None = None) -> None:
2437
print(f"[manim] scenes.py not found at {scenes_file}")
2538
return
2639

27-
quality_flag = self._quality_flag()
40+
manim_bin = self._find_manim()
41+
if not manim_bin:
42+
return
43+
44+
quality_flags = self._quality_flags()
2845
for s in scenes:
29-
self._render_one(scenes_file, s, quality_flag)
46+
self._render_one(manim_bin, scenes_file, s, quality_flags)
3047

31-
def _render_one(self, scenes_file, scene_name: str, quality_flag: str) -> None:
32-
print(f"[manim] Rendering {scene_name}")
33-
cmd = ["manim", quality_flag, str(scenes_file), scene_name]
48+
def _find_manim(self) -> str | None:
49+
"""Locate the manim binary, checking the active venv first."""
50+
venv = Path(sys.prefix) / "bin" / "manim"
51+
if venv.is_file():
52+
return str(venv)
53+
54+
found = shutil.which("manim")
55+
if found:
56+
return found
57+
58+
print(
59+
"[manim] manim not found in PATH. "
60+
"Install with: pip install manim (in this venv) "
61+
"or set PATH to include the directory containing manim."
62+
)
63+
return None
64+
65+
def _render_one(
66+
self, manim_bin: str, scenes_file: Path, scene_name: str, quality_flags: list[str]
67+
) -> None:
68+
quality_label = self.config.manim_quality
69+
print(f"[manim] Rendering {scene_name} at {quality_label}")
70+
cmd = [manim_bin, *quality_flags, str(scenes_file), scene_name]
3471
try:
3572
subprocess.run(
3673
cmd,
3774
check=True,
3875
cwd=str(self.config.animations_dir),
39-
timeout=300,
76+
timeout=self.config.ffmpeg_timeout,
4077
)
4178
except FileNotFoundError:
42-
print("[manim] manim not found in PATH — install with: pip install manim")
79+
print(
80+
"[manim] manim not found in PATH. "
81+
"Install with: pip install manim"
82+
)
4383
except subprocess.CalledProcessError as exc:
4484
print(f"[manim] FAILED {scene_name}: exit code {exc.returncode}")
4585
except subprocess.TimeoutExpired:
46-
print(f"[manim] TIMEOUT {scene_name}")
86+
print(f"[manim] TIMEOUT {scene_name} (limit {self.config.ffmpeg_timeout}s)")
4787

48-
def _quality_flag(self) -> str:
88+
def _quality_flags(self) -> list[str]:
89+
"""Return CLI flags for Manim based on the configured quality string.
90+
91+
Recognised presets: 480p15, 720p30, 1080p60, 2160p60.
92+
Arbitrary ``<height>p<fps>`` strings (e.g. ``1080p30``) are parsed
93+
into explicit ``--resolution`` and ``--frame_rate`` flags.
94+
"""
4995
q = self.config.manim_quality
50-
mapping = {"480p15": "-pql", "720p30": "-pqm", "1080p60": "-pqh"}
51-
return mapping.get(q, "-pqm")
96+
if q in _PRESET_FLAGS:
97+
return list(_PRESET_FLAGS[q])
98+
99+
m = _CUSTOM_RE.match(q)
100+
if m:
101+
height = int(m.group(1))
102+
fps = int(m.group(2))
103+
width = _width_for_height(height)
104+
print(f"[manim] Using custom quality {width}x{height} @ {fps}fps")
105+
return ["--resolution", f"{width},{height}", "--frame_rate", str(fps)]
106+
107+
valid = ", ".join(sorted(_PRESET_FLAGS.keys()))
108+
print(
109+
f"[manim] WARNING: quality '{q}' not recognised, "
110+
f"falling back to 720p30. Valid presets: {valid} "
111+
f"(or use <height>p<fps> e.g. 1080p30)"
112+
)
113+
return ["-qm"]
114+
115+
def quality_subdir(self) -> str:
116+
"""Return the subdirectory name Manim uses for the configured quality."""
117+
q = self.config.manim_quality
118+
preset_dirs = {
119+
"480p15": "480p15",
120+
"720p30": "720p30",
121+
"1080p60": "1080p60",
122+
"2160p60": "2160p60",
123+
}
124+
if q in preset_dirs:
125+
return preset_dirs[q]
126+
m = _CUSTOM_RE.match(q)
127+
if m:
128+
return q
129+
return "720p30"
130+
131+
132+
def _width_for_height(height: int) -> int:
133+
"""Derive 16:9 width from height, rounding to even."""
134+
w = int(height * 16 / 9)
135+
return w + (w % 2)

src/docgen/pipeline.py

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
from __future__ import annotations
44

5+
import shutil
56
from typing import TYPE_CHECKING
67

78
if TYPE_CHECKING:
@@ -41,8 +42,27 @@ def run(
4142
print(f" WARNING: {r.tape} had errors: {r.errors}")
4243

4344
print("\n=== Stage: Compose ===")
44-
from docgen.compose import Composer
45-
Composer(self.config).compose_segments(self.config.segments_all)
45+
from docgen.compose import Composer, ComposeError
46+
composer = Composer(self.config)
47+
try:
48+
composer.compose_segments(self.config.segments_all)
49+
except ComposeError:
50+
if skip_manim:
51+
raise
52+
print(
53+
"\n=== Stage: Manim (retry) ===\n"
54+
"[pipeline] Compose hit FREEZE GUARD — this often happens on the "
55+
"first run because Manim scenes need timing data from timestamps.\n"
56+
"[pipeline] Clearing Manim cache and re-rendering…"
57+
)
58+
media_dir = self.config.animations_dir / "media"
59+
if media_dir.exists():
60+
shutil.rmtree(media_dir)
61+
from docgen.manim_runner import ManimRunner as _MR
62+
_MR(self.config).render()
63+
64+
print("\n=== Stage: Compose (retry) ===")
65+
composer.compose_segments(self.config.segments_all)
4666

4767
print("\n=== Stage: Validate ===")
4868
from docgen.validate import Validator

src/docgen/scene_lint.py

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
"""Lint Manim scene files for known pitfalls: bold weight, positional color args."""
2+
3+
from __future__ import annotations
4+
5+
import re
6+
from dataclasses import dataclass, field
7+
from pathlib import Path
8+
from typing import TYPE_CHECKING
9+
10+
if TYPE_CHECKING:
11+
from docgen.config import Config
12+
13+
14+
@dataclass
15+
class SceneLintResult:
16+
path: str
17+
passed: bool
18+
issues: list[str] = field(default_factory=list)
19+
20+
21+
_BOLD_PATTERN = re.compile(r"\bweight\s*=\s*BOLD\b")
22+
23+
_TEXT_POSITIONAL_COLOR = re.compile(
24+
r"""Text\(\s*(?:f?["'][^"']*["']|[A-Za-z_]\w*)\s*,\s*["']#[0-9a-fA-F]""",
25+
)
26+
27+
_TEXT_POSITIONAL_COLOR_VAR = re.compile(
28+
r"""Text\(\s*(?:f?["'][^"']*["']|[A-Za-z_]\w*)\s*,\s*C_[A-Z_]+""",
29+
)
30+
31+
32+
def lint_scene_file(path: Path) -> SceneLintResult:
33+
"""Scan a single Manim scene file for known issues."""
34+
result = SceneLintResult(path=str(path), passed=True)
35+
36+
if not path.exists():
37+
return result
38+
39+
text = path.read_text(encoding="utf-8")
40+
for lineno, line in enumerate(text.splitlines(), 1):
41+
stripped = line.strip()
42+
if stripped.startswith("#"):
43+
continue
44+
45+
if _BOLD_PATTERN.search(line):
46+
result.issues.append(
47+
f"Line {lineno}: weight=BOLD causes Pango font substitution — "
48+
f"use font_size and color for emphasis instead"
49+
)
50+
result.passed = False
51+
52+
if _TEXT_POSITIONAL_COLOR.search(line) or _TEXT_POSITIONAL_COLOR_VAR.search(line):
53+
result.issues.append(
54+
f"Line {lineno}: color passed as positional arg to Text() — "
55+
f"use keyword: Text(..., color=C_COLOR)"
56+
)
57+
result.passed = False
58+
59+
return result
60+
61+
62+
def lint_scene_dir(config: Config) -> list[SceneLintResult]:
63+
"""Lint all .py files under the animations directory."""
64+
anim_dir = config.animations_dir
65+
if not anim_dir.exists():
66+
return []
67+
68+
results: list[SceneLintResult] = []
69+
for py_file in sorted(anim_dir.glob("**/*.py")):
70+
if py_file.name.startswith("_"):
71+
continue
72+
result = lint_scene_file(py_file)
73+
if result.issues:
74+
results.append(result)
75+
return results

0 commit comments

Comments
 (0)