Skip to content

Commit a3a7e33

Browse files
committed
fix(video_compose): derive the Remotion theme from real playbook keys
`_build_theme_from_playbook` documents itself as reading "a playbook's actual color values [...] not picked from a preset menu", but three of the keys it reads are not in the playbook schema, so each silently fell back to a hardcoded default for every playbook: typo.get("heading") schema key is `headings` palette.get("muted_text") schema key is `muted` motion.get("pace") `pace` is an identity field; motion carries pacing_rules, not a pace enum Effect, before -> after: flat-motion-graphics headingFont Inter -> Space Grotesk muted #6B7280 -> #64748B spring 20/120 @0.4s -> 12/80 @0.3s (pace: fast) minimalist-diagram headingFont Inter -> IBM Plex Sans anime-ghibli headingFont Inter -> Noto Serif JP Every playbook rendered its headings in Inter regardless of what it declared, and no playbook's declared pace ever reached the spring config. This is the Remotion counterpart of the HyperFrames style-bridge defect in issue #306. That bridge lives in lib/hyperframes_style_bridge.py and is untouched here, as are the open PRs against it (#296, #307). Two adjacent things deliberately left alone: - `palette.get("surface", bg)` also names a key the schema does not define, but the fallback to the background color is an explicit, sensible default rather than a dropped value. Giving playbooks a real surface color would be a schema addition, not a bug fix. - The pace branch still handles only `fast` and `slow`, so `deliberate` and `rapid` land on the moderate default. Extending that mapping is a motion tuning decision, not part of reading the right key. Note on ordering: Root.tsx's resolveTheme currently prefers a named preset over themeConfig, which meant this derived theme was discarded whenever the playbook name matched a Remotion preset — masking the defect. Fixing resolveTheme before this would have made every video lose its heading typeface, muted color and motion pacing at once. Expectations in the new tests come from the playbook YAML rather than a hardcoded table, so a playbook that changes its typeface keeps them honest. Verified: 4 cases fail on the unfixed tree; full suite goes 964 -> 977 passed with no regressions.
1 parent 4eab34c commit a3a7e33

2 files changed

Lines changed: 93 additions & 5 deletions

File tree

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
"""The Remotion theme must be derived from the playbook's real schema keys.
2+
3+
`VideoCompose._build_theme_from_playbook` says it reads "a playbook's actual
4+
color values [...] not picked from a preset menu", but it read three keys the
5+
playbook schema does not define, so each silently fell back to a hardcoded
6+
default for every playbook:
7+
8+
typo.get("heading") schema key is `headings`
9+
palette.get("muted_text") schema key is `muted`
10+
motion.get("pace") `pace` is an identity field, not a motion one
11+
12+
Expectations come from the playbook YAML rather than a hardcoded table, so a
13+
playbook that changes its typeface keeps this honest.
14+
15+
This is the Remotion counterpart of the HyperFrames style-bridge defect in
16+
issue #306; that bridge lives in lib/hyperframes_style_bridge.py and is not
17+
touched here.
18+
"""
19+
20+
from pathlib import Path
21+
22+
import pytest
23+
import yaml
24+
25+
from styles.playbook_loader import list_playbooks
26+
from tools.video.video_compose import VideoCompose
27+
28+
REPO_ROOT = Path(__file__).resolve().parents[2]
29+
STYLES_DIR = REPO_ROOT / "styles"
30+
31+
PLAYBOOK_NAMES = sorted(list_playbooks())
32+
33+
34+
def _raw(name: str) -> dict:
35+
return yaml.safe_load((STYLES_DIR / f"{name}.yaml").read_text(encoding="utf-8"))
36+
37+
38+
def _theme(name: str) -> dict:
39+
theme = VideoCompose()._build_theme_from_playbook(name, {})
40+
if not theme:
41+
pytest.skip(f"{name} does not currently yield a theme")
42+
return theme
43+
44+
45+
@pytest.mark.parametrize("name", PLAYBOOK_NAMES)
46+
def test_heading_font_comes_from_the_playbook(name: str) -> None:
47+
"""Regression: read from `heading`, so every playbook rendered in Inter."""
48+
expected = _raw(name)["typography"]["headings"]["font"]
49+
assert _theme(name)["headingFont"] == expected
50+
51+
52+
@pytest.mark.parametrize("name", PLAYBOOK_NAMES)
53+
def test_muted_text_color_comes_from_the_playbook(name: str) -> None:
54+
"""Regression: read from `muted_text`, so this was always #6B7280."""
55+
palette = _raw(name)["visual_language"]["color_palette"]
56+
if "muted" not in palette:
57+
pytest.skip(f"{name} declares no muted color")
58+
assert _theme(name)["mutedTextColor"] == palette["muted"]
59+
60+
61+
def test_identity_pace_drives_the_motion_feel() -> None:
62+
"""Regression: read `pace` off motion, where the schema has no such key,
63+
so the spring never left its moderate default."""
64+
fast = [n for n in PLAYBOOK_NAMES if _raw(n)["identity"]["pace"] == "fast"]
65+
if not fast:
66+
pytest.skip("no playbook declares a fast pace")
67+
68+
moderate_default = {"damping": 20, "stiffness": 120, "mass": 1}
69+
for name in fast:
70+
theme = _theme(name)
71+
assert theme["springConfig"] != moderate_default, (
72+
f"{name} declares pace=fast but still got the moderate spring"
73+
)
74+
assert theme["transitionDuration"] < 0.4
75+
76+
77+
@pytest.mark.parametrize("name", PLAYBOOK_NAMES)
78+
def test_derived_theme_only_reads_documented_palette_keys(name: str) -> None:
79+
"""The derived colors must be values the playbook can actually express."""
80+
palette = _raw(name)["visual_language"]["color_palette"]
81+
theme = _theme(name)
82+
83+
primary = palette["primary"]
84+
accent = palette["accent"]
85+
assert theme["primaryColor"] == (primary[0] if isinstance(primary, list) else primary)
86+
assert theme["accentColor"] == (accent[0] if isinstance(accent, list) else accent)
87+
assert theme["backgroundColor"] == palette["background"]
88+
assert theme["textColor"] == palette["text"]

tools/video/video_compose.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1253,7 +1253,7 @@ def _build_theme_from_playbook(
12531253
bg = palette.get("background", "#FFFFFF")
12541254
text = palette.get("text", "#1F2937")
12551255
surface = palette.get("surface", bg)
1256-
muted = palette.get("muted_text", "#6B7280")
1256+
muted = palette.get("muted", "#6B7280")
12571257

12581258
# Build chart colors from all palette entries
12591259
chart_colors = []
@@ -1271,7 +1271,7 @@ def _build_theme_from_playbook(
12711271
"surfaceColor": surface,
12721272
"textColor": text,
12731273
"mutedTextColor": muted,
1274-
"headingFont": typo.get("heading", {}).get("font", "Inter"),
1274+
"headingFont": typo.get("headings", {}).get("font", "Inter"),
12751275
"bodyFont": typo.get("body", {}).get("font", "Inter"),
12761276
"monoFont": typo.get("code", {}).get("font", "JetBrains Mono"),
12771277
"chartColors": chart_colors[:6],
@@ -1287,9 +1287,9 @@ def _build_theme_from_playbook(
12871287
else f"rgba(15, 23, 42, 0.75)"
12881288
)
12891289

1290-
# Motion style from playbook
1291-
motion = playbook.get("motion", {})
1292-
pace = motion.get("pace", "moderate")
1290+
# Motion style from playbook. `pace` is an identity field in the
1291+
# playbook schema; motion carries pacing_rules, not a pace enum.
1292+
pace = playbook.get("identity", {}).get("pace", "moderate")
12931293
if pace == "fast":
12941294
theme["springConfig"] = {"damping": 12, "stiffness": 80, "mass": 1}
12951295
theme["transitionDuration"] = 0.3

0 commit comments

Comments
 (0)