From 68a4a2574540f316334f26a40ca7c0d2f9397326 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 1 Aug 2026 12:14:00 +0000 Subject: [PATCH] feat: CLI narration --revise and scene-spec title/box subtitles Expose wizard-style in-place narration revise on `docgen narration-generate --revise --revision-notes`, and add optional title.subtitle / box.subtitle for richer diagram hierarchy without inventing spoken beat labels. Co-authored-by: John Menke --- AGENTS.md | 2 +- README.md | 2 +- src/docgen/cli.py | 82 +++++++++++++++++-------------- src/docgen/manim_scene_support.py | 30 ++++++++--- src/docgen/narrate_from_source.py | 43 +++++++++++++++- src/docgen/scene_spec.py | 58 ++++++++++++++++++---- src/docgen/scene_spec_generate.py | 6 ++- tests/test_narrate_from_source.py | 82 +++++++++++++++++++++++++++++++ tests/test_scene_spec.py | 41 ++++++++++++++++ 9 files changed, 286 insertions(+), 60 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 9b50c08..7dcdaca 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -49,7 +49,7 @@ Commands registered on the **`docgen`** CLI include: - **`compose`** — mux narration audio with visual sources via ffmpeg. - **`validate`** / **`validate --pre-push`** — drift, narration lint, Manim hints, **`timing_sync`**, **`story_end`** (last paced reveal vs audio end; hard fail), **`av_sync`** (soft; prefers scene-spec labels as OCR anchors), **`subject_beat_coverage`** (declarative specs vs narration topic beats; hard fail when enabled), and related checks. - **`lint`** — narration lint helper. -- **`narration-generate`** — LLM-assisted narration from hints and repo context. +- **`narration-generate`** — LLM-assisted narration from hints and repo context; optional **`--revise --revision-notes`** for in-place edits (same contract as the wizard Revise button). - **`scene-spec-generate`** — LLM emits declarative **`*.scene.yaml`**; enforces frame budget + **subject-beat coverage** (dwell OK; cover topic shifts; reject invented labels). - **`scene-compile`** — compile specs into **`scenes.py`** (generated regions only). - **`yaml-generate`** — merge defaults and hint wiring into **`docgen.yaml`**. diff --git a/README.md b/README.md index d5c374a..0312f9a 100644 --- a/README.md +++ b/README.md @@ -114,7 +114,7 @@ docgen validate --pre-push | `docgen generate-all [--skip-tts] [--skip-manim] [--retry-manim] [--regen-scene-specs]` | Full pipeline: TTS → timestamps → **scene retime** (existing specs) → Manim → compose → validate. `--regen-scene-specs` also runs OpenAI scene-spec-generate | | `docgen rebuild-after-audio [--regen-scene-specs]` | Timestamps → scene retime → Manim → compose → validate (skips TTS) | | `docgen clean-bundle [-y] [--delete-config] [--keep-narration]` | Remove regenerable outputs under the bundle | -| `docgen narration-generate --segment 01 [--extra-path REL] [--hint TEXT] [--dry-run] [--force]` | Generate narration `.md` from repo sources + owner hints (OpenAI); see `narration_from_source` in YAML | +| `docgen narration-generate --segment 01 [--extra-path REL] [--hint TEXT] [--dry-run] [--force] [--revise --revision-notes TEXT]` | Generate narration `.md` from repo sources + owner hints (OpenAI); `--revise` edits the existing script in place | | `docgen yaml-generate [--merge-defaults] [--llm] [--dry-run] [--list-gaps]` | Merge defaults into `docgen.yaml`; optional OpenAI refresh of `tts.instructions` / `wizard.system_prompt` (rewrites the file — review in Git) | | `docgen scene-compile [SPEC.scene.yaml \| --all] [--retime] [--dry-run]` | Compile declarative scene YAML into `animations/scenes.py`. **`--all --retime`** re-derives `wait_word` from current `timing.json` with no OpenAI; unmatched labels fail closed (or set `pace: none`) | | `docgen scene-spec-generate [--segment 01 \| --all] [--compile] [--print-only] [--output PATH] [--hint …] [--model …]` | Call OpenAI to emit YAML only (same schema as `scene-compile`); rejects frame-budget overflow and **subject-beat coverage** failures (hold board on same topic; cover topic shifts; no invented labels — not a blind count); auto-paginate + word-alignment; optionally writes `animations/specs/.scene.yaml` and `--compile`s into `scenes.py` | diff --git a/src/docgen/cli.py b/src/docgen/cli.py index f812b64..301f723 100644 --- a/src/docgen/cli.py +++ b/src/docgen/cli.py @@ -348,6 +348,17 @@ def lint(ctx: click.Context, segment: str | None) -> None: is_flag=True, help="Overwrite an existing narration file for this segment.", ) +@click.option( + "--revise", + is_flag=True, + help="Edit existing narration.md in place (requires --revision-notes; implies --force).", +) +@click.option( + "--revision-notes", + default="", + show_default=False, + help="Feedback for --revise, or soft notes appended to a full generate.", +) @click.pass_context def narration_generate( ctx: click.Context, @@ -357,14 +368,19 @@ def narration_generate( extra_hints: tuple[str, ...], dry_run: bool, force: bool, + revise: bool, + revision_notes: str, ) -> None: - """Generate narration ``.md`` from repo sources + owner hints via OpenAI chat. + """Generate or revise narration ``.md`` from repo sources + owner hints via OpenAI chat. Configure ``narration_from_source`` in docgen.yaml (context paths/globs, hints, model). Requires ``OPENAI_API_KEY`` unless using a future offline stub. Use ``--segment `` to drive a single segment, or ``--all`` to iterate every id in ``segments.all`` (used by full-reset orchestration). + + ``--revise`` reads the current narration file and applies ``--revision-notes`` + with minimal edits (same contract as the wizard Revise button). """ if ctx.obj.get("config") is None: raise click.ClickException("No docgen.yaml found (use --config PATH).") @@ -372,10 +388,36 @@ def narration_generate( raise click.ClickException("--all and --segment are mutually exclusive") if not all_segments and not segment: raise click.ClickException("provide --segment or --all") + if revise and not str(revision_notes or "").strip(): + raise click.ClickException("--revise requires --revision-notes") from docgen.narrate_from_source import generate_narration_markdown, write_narration_markdown cfg = ctx.obj["config"] + mode = "revise" if revise else "generate" + # Revising always overwrites the existing script. + write_force = force or revise + + def _one(seg_str: str) -> None: + try: + body = generate_narration_markdown( + cfg, + seg_str, + extra_paths=list(extra_paths), + extra_hints=list(extra_hints), + revision_notes=revision_notes, + mode=mode, + ) + except ValueError as exc: + raise click.ClickException(f"segment {seg_str}: {exc}") from exc + if dry_run: + click.echo(body) + return + try: + out = write_narration_markdown(cfg, seg_str, body, force=write_force) + except FileExistsError as exc: + raise click.ClickException(f"segment {seg_str}: {exc} (use --force)") from exc + click.echo(f" -> {out}" if all_segments else f"[narration-generate] wrote {out}") if all_segments: ids = list((cfg.raw.get("segments") or {}).get("all") or []) @@ -384,45 +426,11 @@ def narration_generate( for seg_id in ids: seg_str = str(seg_id) click.echo(f"=== narration-generate --segment {seg_str} ===") - try: - body = generate_narration_markdown( - cfg, - seg_str, - extra_paths=list(extra_paths), - extra_hints=list(extra_hints), - ) - except ValueError as exc: - raise click.ClickException(f"segment {seg_str}: {exc}") from exc - if dry_run: - click.echo(body) - continue - try: - out = write_narration_markdown(cfg, seg_str, body, force=force) - except FileExistsError as exc: - raise click.ClickException(f"segment {seg_str}: {exc} (use --force)") from exc - click.echo(f" -> {out}") + _one(seg_str) return assert segment is not None # for type-checker - try: - body = generate_narration_markdown( - cfg, - segment, - extra_paths=list(extra_paths), - extra_hints=list(extra_hints), - ) - except ValueError as exc: - raise click.ClickException(str(exc)) from exc - - if dry_run: - click.echo(body) - return - - try: - out = write_narration_markdown(cfg, segment, body, force=force) - except FileExistsError as exc: - raise click.ClickException(f"{exc} (use --force)") from exc - click.echo(f"[narration-generate] wrote {out}") + _one(segment) @main.command("scene-compile") diff --git a/src/docgen/manim_scene_support.py b/src/docgen/manim_scene_support.py index ecad7c7..fd54227 100644 --- a/src/docgen/manim_scene_support.py +++ b/src/docgen/manim_scene_support.py @@ -161,8 +161,12 @@ def _load_timing_words(segment_key: str) -> list[dict]: return list(words) if isinstance(words, list) else [] -def _box(label, color, w=2.2, h=0.75, fs=18): - """Labeled rounded box — slightly stronger fill/stroke for readable diagram boards.""" +def _box(label, color, w=2.2, h=0.75, fs=18, subtitle=""): + """Labeled rounded box — slightly stronger fill/stroke for readable diagram boards. + + Optional ``subtitle`` is a second, smaller line under the primary label + (decorative; not used for wait_word beat matching). + """ r = RoundedRectangle( corner_radius=0.18, width=w, height=h, stroke_color=color, stroke_width=2.5, @@ -173,14 +177,24 @@ def _box(label, color, w=2.2, h=0.75, fs=18): # the palette token is already near-white. if str(color) in (C_WHITE, "C_WHITE", "#cdd6f4"): t.set_color(color) + sub = str(subtitle or "").strip() + if sub: + sub_fs = max(10, int(fs * 0.72)) + s = Text(sub, font_size=sub_fs, color=C_WHITE) + if str(color) in (C_WHITE, "C_WHITE", "#cdd6f4"): + s.set_color(color) + s.set_opacity(0.85) + text = VGroup(t, s).arrange(DOWN, buff=0.06) + else: + text = t inner_w = max(w - 0.3, 0.35) inner_h = max(h - 0.22, 0.35) - if t.width > inner_w: - t.scale(inner_w / t.width) - if t.height > inner_h: - t.scale(inner_h / t.height) - t.move_to(r.get_center()) - return VGroup(r, t) + if text.width > inner_w: + text.scale(inner_w / text.width) + if text.height > inner_h: + text.scale(inner_h / text.height) + text.move_to(r.get_center()) + return VGroup(r, text) def _arrow(start, end, color="#cdd6f4", style="solid"): diff --git a/src/docgen/narrate_from_source.py b/src/docgen/narrate_from_source.py index 15a7595..7aeac74 100644 --- a/src/docgen/narrate_from_source.py +++ b/src/docgen/narrate_from_source.py @@ -199,23 +199,50 @@ def build_owner_hints_guidance( return "\n".join(f"- {h}" for h in lines if h.strip()) +def _existing_narration_text(cfg: "Config", seg_id: str) -> str: + """Return current ``narration/.md`` text, or empty if missing.""" + seg_name = cfg.resolve_segment_name(seg_id) + path = cfg.narration_dir / f"{seg_name}.md" + if not path.is_file(): + # Match wizard asset discovery for NN-*.md fallbacks. + if cfg.narration_dir.is_dir(): + for cand in cfg.narration_dir.glob(f"{seg_id}-*.md"): + return cand.read_text(encoding="utf-8") + for cand in cfg.narration_dir.glob(f"{seg_id}*.md"): + return cand.read_text(encoding="utf-8") + return "" + return path.read_text(encoding="utf-8") + + def generate_narration_markdown( cfg: "Config", seg_id: str, *, extra_paths: list[str], extra_hints: list[str], + revision_notes: str = "", + mode: str = "generate", ) -> str: """Call OpenAI and return markdown body (does not write files). Owner hints from YAML and ``extra_hints`` from the caller are sent as guidance only; the returned markdown is model-generated. + + ``mode="revise"`` edits the existing narration file in place using + ``revision_notes`` (requires both an on-disk script and non-empty notes). """ from docgen.wizard import generate_narration_via_llm + mode_norm = str(mode or "generate").strip().lower() + if mode_norm not in ("generate", "revise"): + mode_norm = "generate" + notes = (revision_notes or "").strip() + settings = merged_narration_from_source_settings(cfg, seg_id) snippets = collect_source_snippets(cfg, settings, extra_paths=extra_paths) - if not snippets: + # Revise can proceed with empty sources (current script + notes are enough); + # full generate still requires context files. + if not snippets and mode_norm != "revise": raise ValueError( "No source files collected. Add narration_from_source.context.paths/globs " "to docgen.yaml or pass extra paths on the CLI." @@ -224,15 +251,27 @@ def generate_narration_markdown( guidance = build_owner_hints_guidance(settings, extra_hints) seg_name = cfg.resolve_segment_name(seg_id) topic = cfg.narration_topic_label(seg_id) + current = "" + if mode_norm == "revise": + current = _existing_narration_text(cfg, seg_id) + if not current.strip(): + raise ValueError( + f"no existing narration for segment {seg_id!r} to revise — " + "run a full generate first, or drop --revise" + ) + if not notes: + raise ValueError("--revise requires --revision-notes") return generate_narration_via_llm( source_texts=source_texts, guidance=guidance, system_prompt=settings.system_prompt, model=settings.model, segment_name=seg_name, - revision_notes="", + revision_notes=notes, temperature=settings.temperature, topic_label=topic, + current_narration=current, + mode=mode_norm, ) diff --git a/src/docgen/scene_spec.py b/src/docgen/scene_spec.py index d965529..e1e1e7c 100644 --- a/src/docgen/scene_spec.py +++ b/src/docgen/scene_spec.py @@ -67,10 +67,13 @@ _LAYOUT_BOTTOM_MARGIN = 0.55 -def _title_band_estimate(font_size: int) -> float: +def _title_band_estimate(font_size: int, *, has_subtitle: bool = False) -> float: """Rough vertical space from top of frame through title and first gap.""" fs = max(14, int(font_size)) - return 0.78 + (fs / 36.0) * 0.52 + band = 0.78 + (fs / 36.0) * 0.52 + if has_subtitle: + band += 0.38 + return band def layout_stack_budget(title: dict[str, Any], layout: dict[str, Any] | None) -> float: @@ -80,7 +83,8 @@ def layout_stack_budget(title: dict[str, Any], layout: dict[str, Any] | None) -> fs = title.get("font_size") if not isinstance(fs, (int, float)): fs = 36 - band = _title_band_estimate(int(fs)) + has_sub = bool(str(title.get("subtitle") or "").strip()) + band = _title_band_estimate(int(fs), has_subtitle=has_sub) return FRAME_HEIGHT - band - buff - _LAYOUT_BOTTOM_MARGIN @@ -1162,6 +1166,12 @@ def _validate_row_list(rows: list[Any], *, path_label: str, prefix: str) -> None v = box[num_f] if not isinstance(v, (int, float)) or v <= 0: raise SceneSpecError(f"{bp}: {num_f} must be a positive number") + bsub = box.get("subtitle") + if bsub is not None: + if not isinstance(bsub, str): + raise SceneSpecError(f"{bp}: subtitle must be a string if set") + if len(bsub.strip()) > 60: + raise SceneSpecError(f"{bp}: subtitle must be at most 60 characters") has_row_pacing = row.get("wait_word") is not None or row.get("wait_segment") is not None if has_row_pacing and box_pacing: @@ -1262,6 +1272,12 @@ def validate_scene_spec(data: dict[str, Any], *, path_label: str = "spec") -> No raise SceneSpecError( f"{path_label}: title.color must be one of {sorted(ALLOWED_COLORS)}" ) + tsub = title.get("subtitle") + if tsub is not None: + if not isinstance(tsub, str): + raise SceneSpecError(f"{path_label}: title.subtitle must be a string if set") + if len(tsub.strip()) > 80: + raise SceneSpecError(f"{path_label}: title.subtitle must be at most 80 characters") has_pages = data.get("pages") is not None has_rows = data.get("rows") is not None @@ -1393,6 +1409,7 @@ def compile_scene_class(spec: dict[str, Any]) -> str: title_text: str = str(title["text"]) title_fs = int(title["font_size"]) title_color = str(title["color"]) + title_subtitle = str(title.get("subtitle") or "").strip() layout = spec.get("layout") or {} first_row_title_buff = float(layout.get("first_row_title_buff", 0.5)) @@ -1414,10 +1431,27 @@ def compile_scene_class(spec: dict[str, Any]) -> str: " self.camera.background_color = C_BG", f" timing_words = _load_timing_words({timing_key!r})", "", - f" title = Text({title_text!r}, font_size={title_fs}, color={title_color}).to_edge(UP)", - " self.timed_play(Write(title), run_time=2.0)", - "", ] + if title_subtitle: + sub_fs = max(14, title_fs - 10) + lines.extend( + [ + f" _title_main = Text({title_text!r}, font_size={title_fs}, color={title_color})", + f" _title_sub = Text({title_subtitle!r}, font_size={sub_fs}, color={title_color})", + " _title_sub.set_opacity(0.85)", + " title = VGroup(_title_main, _title_sub).arrange(DOWN, buff=0.12).to_edge(UP)", + " self.timed_play(Write(title), run_time=2.0)", + "", + ] + ) + else: + lines.extend( + [ + f" title = Text({title_text!r}, font_size={title_fs}, color={title_color}).to_edge(UP)", + " self.timed_play(Write(title), run_time=2.0)", + "", + ] + ) # Map (page, later-box-var) → list of (edge_var, anim) where anim is grow|fade. edges_with_target: dict[tuple[int, str], list[tuple[str, str]]] = {} @@ -1445,9 +1479,15 @@ def compile_scene_class(spec: dict[str, Any]) -> str: lab = str(box["label"]) col = str(box["color"]) fs = int(box["font_size"]) - lines.append( - f" {var} = _box({lab!r}, {col}, {w}, {h}, {fs})" - ) + bsub = str(box.get("subtitle") or "").strip() + if bsub: + lines.append( + f" {var} = _box({lab!r}, {col}, {w}, {h}, {fs}, subtitle={bsub!r})" + ) + else: + lines.append( + f" {var} = _box({lab!r}, {col}, {w}, {h}, {fs})" + ) for r, row in enumerate(rows): boxes_raw = row["boxes"] diff --git a/src/docgen/scene_spec_generate.py b/src/docgen/scene_spec_generate.py index 7a6ffd7..6ec0394 100644 --- a/src/docgen/scene_spec_generate.py +++ b/src/docgen/scene_spec_generate.py @@ -69,17 +69,19 @@ Required keys: - segment_id: string (echo the value from the user message exactly) - class_name: string (echo the value from the user message exactly) -- title: mapping with text (string), font_size (int, >= 14), color (one of the palette tokens below) +- title: mapping with text (string), font_size (int, >= 14), color (one of the palette tokens below); + optional subtitle (string ≤80 chars) for a second line under the title - **Exactly one of:** ``rows`` (non-empty list of row mappings, single page) **or** ``pages`` (non-empty list of page mappings; each page has ``rows`` as above, optionally ``transition``: fade | none for pages after the first) Each row must have: - run_time: positive number (seconds for timed_play FadeIn of **each** box in that row) - boxes: non-empty list of box mappings, each with: - - label: string + - label: string (spoken phrase — used for wait_word matching) - color: one of the palette tokens - width: positive number (typical 2.0–6.0; safe row total ≤ ~13 wide at dogfood resolution) - height: positive number (typical 0.65–1.1; **smaller when a page has many rows**) - font_size: int >= 14 + - subtitle: optional second line ≤60 chars (decorative; not used for beat matching) Optional **image elements** (only when project-owner hints ask for generated imagery): a ``boxes`` entry may instead be an image element with: diff --git a/tests/test_narrate_from_source.py b/tests/test_narrate_from_source.py index b7e42a2..0a97904 100644 --- a/tests/test_narrate_from_source.py +++ b/tests/test_narrate_from_source.py @@ -118,6 +118,88 @@ def test_write_narration_markdown_creates_file(tmp_path: Path) -> None: assert p.read_text(encoding="utf-8").strip() == "Line." +def test_generate_narration_markdown_revise_mode(tmp_path: Path) -> None: + (tmp_path / ".git").mkdir() + (tmp_path / "docgen.yaml").write_text( + yaml.dump( + { + "dirs": {"narration": "narration"}, + "segments": {"default": ["01"], "all": ["01"]}, + "segment_names": {"01": "01-demo"}, + "narration_from_source": { + "model": "gpt-4o-mini", + "context": {"paths": ["lib.py"]}, + }, + } + ), + encoding="utf-8", + ) + (tmp_path / "lib.py").write_text("def f(): pass\n", encoding="utf-8") + narr = tmp_path / "narration" + narr.mkdir() + (narr / "01-demo.md").write_text("Original opening about pipelines.\n", encoding="utf-8") + cfg = Config.from_yaml(tmp_path / "docgen.yaml") + + with patch("docgen.wizard.generate_narration_via_llm") as m: + m.return_value = "Revised opening about Flask.\n" + out = generate_narration_markdown( + cfg, + "01", + extra_paths=[], + extra_hints=[], + revision_notes="Mention Flask", + mode="revise", + ) + assert "Flask" in out + kw = m.call_args.kwargs + assert kw["mode"] == "revise" + assert "Original opening" in kw["current_narration"] + assert kw["revision_notes"] == "Mention Flask" + + +def test_narration_generate_cli_revise(tmp_path: Path) -> None: + from click.testing import CliRunner + + from docgen.cli import main + + (tmp_path / ".git").mkdir() + (tmp_path / "docgen.yaml").write_text( + yaml.dump( + { + "dirs": {"narration": "narration"}, + "segments": {"default": ["01"], "all": ["01"]}, + "segment_names": {"01": "01-demo"}, + "narration_from_source": {"context": {"paths": ["x.md"]}}, + } + ), + encoding="utf-8", + ) + (tmp_path / "x.md").write_text("# src\nbody", encoding="utf-8") + narr = tmp_path / "narration" + narr.mkdir() + (narr / "01-demo.md").write_text("Old script.\n", encoding="utf-8") + runner = CliRunner() + + with patch("docgen.wizard.generate_narration_via_llm") as m: + m.return_value = "New script.\n" + r = runner.invoke( + main, + [ + "--config", + str(tmp_path / "docgen.yaml"), + "narration-generate", + "--segment", + "01", + "--revise", + "--revision-notes", + "Tighten the intro", + ], + ) + assert r.exit_code == 0, r.output + assert m.call_args.kwargs["mode"] == "revise" + assert (narr / "01-demo.md").read_text(encoding="utf-8").startswith("New script.") + + def test_narration_generate_cli_dry_run(tmp_path: Path) -> None: from click.testing import CliRunner diff --git a/tests/test_scene_spec.py b/tests/test_scene_spec.py index 4694784..899fe0d 100644 --- a/tests/test_scene_spec.py +++ b/tests/test_scene_spec.py @@ -1002,6 +1002,47 @@ def test_sync_row_labels_hyphenated_label_matches_spoken_parts() -> None: assert out["rows"][0]["boxes"][0]["wait_word"] == 1 +def test_compile_title_and_box_subtitles() -> None: + spec = { + "segment_id": "01", + "class_name": "SubScene", + "timing_key": "01-sub", + "title": { + "text": "Architecture", + "subtitle": "Four pipelines", + "font_size": 36, + "color": "C_WHITE", + }, + "rows": [ + { + "run_time": 0.8, + "boxes": [ + { + "label": "Flask", + "subtitle": "orchestrator", + "color": "C_GREEN", + "width": 3.0, + "height": 1.0, + "font_size": 18, + "wait_word": 0, + } + ], + } + ], + } + validate_scene_spec(spec) + out = compile_scene_class(spec) + assert "_title_main = Text('Architecture'" in out + assert "_title_sub = Text('Four pipelines'" in out + assert "subtitle='orchestrator'" in out + # Subtitle shrinks the vertical stack budget vs plain title. + with_sub = layout_stack_budget(spec["title"], {}) + plain = layout_stack_budget( + {"text": "Architecture", "font_size": 36, "color": "C_WHITE"}, {} + ) + assert with_sub < plain + + def test_compile_edges_emits_arrows_and_grow() -> None: spec = { "segment_id": "01",