From 638e3d74f7fd3527beb16a978e8655182d9b1be0 Mon Sep 17 00:00:00 2001 From: John Menke Date: Sat, 26 Sep 2026 20:41:11 -0400 Subject: [PATCH 1/2] Fail closed when scenes.py inlines or renames _TimedScene helpers. The stale-helper check returned no issues when none of the canonical helpers were top-level defs, so inlined or renamed helpers skipped staleness. Co-authored-by: Cursor --- src/docgen/scene_asset_validate.py | 62 ++++++++++++++++++++++++++++-- tests/test_scene_asset_validate.py | 34 ++++++++++++++++ 2 files changed, 93 insertions(+), 3 deletions(-) diff --git a/src/docgen/scene_asset_validate.py b/src/docgen/scene_asset_validate.py index e228fe5..7bfbbec 100644 --- a/src/docgen/scene_asset_validate.py +++ b/src/docgen/scene_asset_validate.py @@ -250,8 +250,63 @@ def extract_class_source(scenes_text: str, class_name: str) -> str | None: return None +_CANONICAL_HELPERS = ("_box", "_arrow", "_TimedScene", "_load_timing", "_load_timing_words") +_TIMED_SCENE_METHODS = frozenset({"timed_play", "wait_until_word"}) + + +def _unique(names: list[str]) -> list[str]: + return list(dict.fromkeys(names)) + + +def _inlined_or_renamed_helper_issue(tree: ast.AST) -> str | None: + """Fail closed when canonical helpers are not top-level defs. + + Nested ``_box`` / ``_TimedScene`` bodies, calls or bases that name them, and + classes that carry ``timed_play`` / ``wait_until_word`` under another name + used to skip :func:`helper_needs_refresh` entirely. + """ + inlined: list[str] = [] + referenced: list[str] = [] + renamed: list[str] = [] + for node in ast.walk(tree): + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)): + if node.name in _CANONICAL_HELPERS: + inlined.append(node.name) + if isinstance(node, ast.ClassDef) and node.name != "_TimedScene": + methods = { + child.name + for child in node.body + if isinstance(child, (ast.FunctionDef, ast.AsyncFunctionDef)) + } + if _TIMED_SCENE_METHODS.intersection(methods): + renamed.append(node.name) + elif isinstance(node, ast.Name) and node.id in _CANONICAL_HELPERS: + referenced.append(node.id) + if not inlined and not referenced and not renamed: + return None + parts: list[str] = [] + if inlined: + parts.append("inlined " + ", ".join(_unique(inlined))) + if renamed: + parts.append("renamed _TimedScene-style " + ", ".join(_unique(renamed))) + missing = [name for name in _unique(referenced) if name not in inlined] + if missing: + parts.append("referenced but not top-level " + ", ".join(missing)) + detail = "; ".join(parts) + return ( + "helpers: canonical _box / _arrow / _TimedScene / _load_timing / " + "_load_timing_words are missing or inlined " + f"({detail}) — stale-helper check cannot pass silently; " + "run `docgen scene-compile` to restore top-level helper bodies" + ) + + def helper_api_violations(scenes_text: str) -> list[str]: - """Stale ``_box`` / ``_arrow`` / ``_TimedScene`` that will mis-render new specs.""" + """Stale ``_box`` / ``_arrow`` / ``_TimedScene`` that will mis-render new specs. + + Helpers must be top-level. Inlined or renamed ``_TimedScene``-style helpers + fail closed with an explicit missing/inlined message. + """ from docgen.manim_scene_support import helper_needs_refresh try: @@ -262,8 +317,9 @@ def helper_api_violations(scenes_text: str) -> list[str]: for node in tree.body: if isinstance(node, (ast.FunctionDef, ast.ClassDef)): defined.add(node.name) - if not defined.intersection({"_box", "_arrow", "_TimedScene", "_load_timing", "_load_timing_words"}): - return [] + if not defined.intersection(set(_CANONICAL_HELPERS)): + issue = _inlined_or_renamed_helper_issue(tree) + return [issue] if issue else [] issues: list[str] = [] if "MANIM_FONT" not in scenes_text: issues.append( diff --git a/tests/test_scene_asset_validate.py b/tests/test_scene_asset_validate.py index 9f2ef6d..9148642 100644 --- a/tests/test_scene_asset_validate.py +++ b/tests/test_scene_asset_validate.py @@ -263,6 +263,40 @@ def test_helper_api_clean_for_current_bootstrap() -> None: assert helper_api_violations(BOOTSTRAP_HEADER) == [] +def test_helper_api_flags_inlined_or_renamed_helpers() -> None: + inlined = """ +class OverviewScene: + class _TimedScene: + def timed_play(self, *a, run_time=1.0): + pass + def wait_until_word(self, words, index): + return + def construct(self): + _box("Alpha", "#fff") +""" + inlined_issues = helper_api_violations(inlined) + assert any( + "missing or inlined" in issue and "inlined _TimedScene" in issue + for issue in inlined_issues + ) + assert any("_box" in issue for issue in inlined_issues) + + renamed = """ +class SceneClock: + def timed_play(self, *a, run_time=1.0): + pass + def wait_until_word(self, words, index): + return +""" + renamed_issues = helper_api_violations(renamed) + assert any( + "missing or inlined" in issue and "renamed _TimedScene-style SceneClock" in issue + for issue in renamed_issues + ) + + assert helper_api_violations("def render():\n return 1\n") == [] + + def test_compiled_sync_passes_when_scenes_match_compile() -> None: spec = _spec([_box("Alpha", wait_word=0), _box("Beta", wait_word=1)]) words = _wide_words() From 4ecdb1d222de2a0a36c7f155c5cf4e0e7f595c7c Mon Sep 17 00:00:00 2001 From: John Menke Date: Sat, 26 Sep 2026 21:26:20 -0400 Subject: [PATCH 2/2] Keep inlined-helper detection without raising cyclomatic complexity. --- src/docgen/scene_asset_validate.py | 67 +++++++++++++++++++----------- 1 file changed, 43 insertions(+), 24 deletions(-) diff --git a/src/docgen/scene_asset_validate.py b/src/docgen/scene_asset_validate.py index 7bfbbec..5de31a1 100644 --- a/src/docgen/scene_asset_validate.py +++ b/src/docgen/scene_asset_validate.py @@ -258,6 +258,39 @@ def _unique(names: list[str]) -> list[str]: return list(dict.fromkeys(names)) +def _note_renamed_timed_scene(node: ast.AST, renamed: list[str]) -> None: + if not isinstance(node, ast.ClassDef) or node.name == "_TimedScene": + return + methods = { + child.name + for child in node.body + if isinstance(child, (ast.FunctionDef, ast.AsyncFunctionDef)) + } + if _TIMED_SCENE_METHODS.intersection(methods): + renamed.append(node.name) + + +def _note_helper_node(node: ast.AST, inlined: list[str], referenced: list[str], renamed: list[str]) -> None: + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)): + if node.name in _CANONICAL_HELPERS: + inlined.append(node.name) + _note_renamed_timed_scene(node, renamed) + elif isinstance(node, ast.Name) and node.id in _CANONICAL_HELPERS: + referenced.append(node.id) + + +def _helper_issue_parts(inlined: list[str], referenced: list[str], renamed: list[str]) -> list[str]: + parts: list[str] = [] + if inlined: + parts.append("inlined " + ", ".join(_unique(inlined))) + if renamed: + parts.append("renamed _TimedScene-style " + ", ".join(_unique(renamed))) + missing = [name for name in _unique(referenced) if name not in inlined] + if missing: + parts.append("referenced but not top-level " + ", ".join(missing)) + return parts + + def _inlined_or_renamed_helper_issue(tree: ast.AST) -> str | None: """Fail closed when canonical helpers are not top-level defs. @@ -269,30 +302,10 @@ def _inlined_or_renamed_helper_issue(tree: ast.AST) -> str | None: referenced: list[str] = [] renamed: list[str] = [] for node in ast.walk(tree): - if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)): - if node.name in _CANONICAL_HELPERS: - inlined.append(node.name) - if isinstance(node, ast.ClassDef) and node.name != "_TimedScene": - methods = { - child.name - for child in node.body - if isinstance(child, (ast.FunctionDef, ast.AsyncFunctionDef)) - } - if _TIMED_SCENE_METHODS.intersection(methods): - renamed.append(node.name) - elif isinstance(node, ast.Name) and node.id in _CANONICAL_HELPERS: - referenced.append(node.id) + _note_helper_node(node, inlined, referenced, renamed) if not inlined and not referenced and not renamed: return None - parts: list[str] = [] - if inlined: - parts.append("inlined " + ", ".join(_unique(inlined))) - if renamed: - parts.append("renamed _TimedScene-style " + ", ".join(_unique(renamed))) - missing = [name for name in _unique(referenced) if name not in inlined] - if missing: - parts.append("referenced but not top-level " + ", ".join(missing)) - detail = "; ".join(parts) + detail = "; ".join(_helper_issue_parts(inlined, referenced, renamed)) return ( "helpers: canonical _box / _arrow / _TimedScene / _load_timing / " "_load_timing_words are missing or inlined " @@ -301,6 +314,13 @@ def _inlined_or_renamed_helper_issue(tree: ast.AST) -> str | None: ) +def _missing_top_level_helper_issues(tree: ast.AST) -> list[str]: + issue = _inlined_or_renamed_helper_issue(tree) + if issue: + return [issue] + return [] + + def helper_api_violations(scenes_text: str) -> list[str]: """Stale ``_box`` / ``_arrow`` / ``_TimedScene`` that will mis-render new specs. @@ -318,8 +338,7 @@ def helper_api_violations(scenes_text: str) -> list[str]: if isinstance(node, (ast.FunctionDef, ast.ClassDef)): defined.add(node.name) if not defined.intersection(set(_CANONICAL_HELPERS)): - issue = _inlined_or_renamed_helper_issue(tree) - return [issue] if issue else [] + return _missing_top_level_helper_issues(tree) issues: list[str] = [] if "MANIM_FONT" not in scenes_text: issues.append(