From 0e84f17a939151c6fb69919f9a2f3b510061f679 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 7 Sep 2026 18:43:57 +0000 Subject: [PATCH] Fail closed when concat targets or pages extra_links are not lists A string concat. was iterated as characters. Reject non-list concat targets at config load and in ConcatBuilder. pages.extra_links items must be href/label mappings. Co-authored-by: jmjava --- milestones/README.md | 8 +++++--- milestones/concat-segment-lists.md | 21 +++++++++++++++++++++ src/docgen/cli.py | 6 ++++-- src/docgen/concat.py | 7 ++++++- src/docgen/config.py | 9 +++++++++ src/docgen/pages.py | 15 +++++++++++++-- tests/test_concat.py | 20 +++++++++++++++++++- tests/test_pages.py | 14 ++++++++++++++ 8 files changed, 91 insertions(+), 9 deletions(-) create mode 100644 milestones/concat-segment-lists.md diff --git a/milestones/README.md b/milestones/README.md index 98c7645..8f2baa6 100644 --- a/milestones/README.md +++ b/milestones/README.md @@ -5,11 +5,13 @@ repositories that install `docgen` and maintain their own demo bundle. The library no longer ships an in-repo dogfood; consumers are the integration test of record. -**Active:** **[config-mapping-keys.md](config-mapping-keys.md)** — nested -`docgen.yaml` mapping/list keys must not traceback; lint/compose empty-all -must not succeed. +**Active:** **[concat-segment-lists.md](concat-segment-lists.md)** — concat +targets must be lists of segment ids; pages extra_links items must be mappings. **Shipped:** +- **[config-mapping-keys.md](config-mapping-keys.md)** — nested `docgen.yaml` + mapping/list keys must not traceback; lint/compose empty-all must not + succeed (#83). - **[manim-lint-config.md](manim-lint-config.md)** — missing `scenes.py` fails validate; invalid `docgen.yaml` is `ConfigError`; TTS empty-all fails (#82). diff --git a/milestones/concat-segment-lists.md b/milestones/concat-segment-lists.md new file mode 100644 index 0000000..8ed8b25 --- /dev/null +++ b/milestones/concat-segment-lists.md @@ -0,0 +1,21 @@ +# Milestone — Concat segment lists and pages extra_links + +**Goal:** `concat.` must be a YAML list of segment ids (a string must +not be iterated as characters). `pages.extra_links` items must be mappings. + +**Branch / PR:** `cursor/concat-segment-lists-2ccd` + +Follows **[config-mapping-keys.md](config-mapping-keys.md)** (#83). + +## Shipped + +- [x] `concat.` that is a string/scalar raises `ConfigError` at load + and `ConcatError` in `ConcatBuilder` +- [x] `pages.extra_links` items that are not mappings raise instead of + `TypeError` on `lnk["href"]` +- [x] `docgen pages` maps that `RuntimeError` to `ClickException` + +## Not this milestone + +- Empty `concat:` map remains a no-op (bundles that do not stitch a full demo) +- Archived Playwright / Embabel / slides diff --git a/src/docgen/cli.py b/src/docgen/cli.py index 6e2daea..f02d886 100644 --- a/src/docgen/cli.py +++ b/src/docgen/cli.py @@ -1325,8 +1325,10 @@ def pages(ctx: click.Context, force: bool) -> None: from docgen.pages import PagesGenerator cfg = _require_config(ctx) - gen = PagesGenerator(cfg) - gen.generate_all(force=force) + try: + PagesGenerator(cfg).generate_all(force=force) + except RuntimeError as exc: + raise click.ClickException(str(exc)) from exc @main.command("generate-all") diff --git a/src/docgen/concat.py b/src/docgen/concat.py index 7e0bf30..52b9047 100644 --- a/src/docgen/concat.py +++ b/src/docgen/concat.py @@ -34,7 +34,12 @@ def build(self, name: str | None = None) -> None: else: targets = concat_map for out_name, seg_ids in targets.items(): - self._build_one(out_name, seg_ids) + if not isinstance(seg_ids, list): + raise ConcatError( + f"[concat] {out_name}: segment list must be a YAML list, " + f"not {type(seg_ids).__name__}" + ) + self._build_one(out_name, [str(s) for s in seg_ids]) def _build_one(self, out_name: str, seg_ids: list[str]) -> None: recordings_dir = self.config.recordings_dir diff --git a/src/docgen/config.py b/src/docgen/config.py index 768e4b6..13e50de 100644 --- a/src/docgen/config.py +++ b/src/docgen/config.py @@ -154,6 +154,15 @@ def __post_init__(self) -> None: # List-valued keys: a string must not be iterated as characters. _ = self.segments_all _ = self.manim_scenes + concat = self._block("concat") + for name, segs in concat.items(): + if segs is None: + continue + if not isinstance(segs, list): + raise ConfigError( + f"{self._source_label()}: concat.{name} must be a YAML list, " + f"not {type(segs).__name__}" + ) def _source_label(self) -> str: return self.yaml_path.name if self.yaml_path else "docgen.yaml" diff --git a/src/docgen/pages.py b/src/docgen/pages.py index 1daec39..d1fdd30 100644 --- a/src/docgen/pages.py +++ b/src/docgen/pages.py @@ -38,7 +38,11 @@ def generate_index_html(self, force: bool = False) -> None: subtitle = self.pages_cfg.get("subtitle", "") repo_url = self.pages_cfg.get("repo_url", "") demos_sub = self.pages_cfg.get("demos_subdir", "demos") - extra_links = self.pages_cfg.get("extra_links", []) + extra_links = self.pages_cfg.get("extra_links", []) or [] + if not isinstance(extra_links, list): + raise RuntimeError( + f"pages.extra_links must be a YAML list, not {type(extra_links).__name__}" + ) segments_cfg = self._resolve_segments_cfg() concat_map = self.config.concat_map @@ -75,7 +79,14 @@ def generate_index_html(self, force: bool = False) -> None: footer_links = "" for lnk in extra_links: - footer_links += f' | {_esc(lnk["label"])}' + if not isinstance(lnk, dict): + raise RuntimeError( + "pages.extra_links items must be mappings with href and label, " + f"not {type(lnk).__name__}" + ) + href = lnk.get("href", "") + label = lnk.get("label", "") + footer_links += f' | {_esc(str(label))}' html = _INDEX_TEMPLATE.format( title=_esc(title), diff --git a/tests/test_concat.py b/tests/test_concat.py index 0295b3a..bfca077 100644 --- a/tests/test_concat.py +++ b/tests/test_concat.py @@ -8,7 +8,7 @@ import yaml from docgen.concat import ConcatBuilder, ConcatError -from docgen.config import Config +from docgen.config import Config, ConfigError def _cfg(tmp_path: Path, concat: dict) -> Config: @@ -40,3 +40,21 @@ def test_concat_missing_recording_raises_before_ffmpeg(tmp_path: Path) -> None: def test_concat_empty_map_is_noop(tmp_path: Path) -> None: cfg = _cfg(tmp_path, {}) ConcatBuilder(cfg).build() + + +def test_concat_string_segment_list_raises(tmp_path: Path) -> None: + with pytest.raises(ConfigError, match="concat.full must be a YAML list"): + _cfg(tmp_path, {"full": "01"}) + + +def test_concat_builder_rejects_non_list_target() -> None: + from pathlib import Path as P + from types import SimpleNamespace + + cfg = SimpleNamespace( + concat_map={"full": "01"}, + recordings_dir=P("/tmp"), + find_segment_asset=lambda *a, **k: None, + ) + with pytest.raises(ConcatError, match="must be a YAML list"): + ConcatBuilder(cfg).build() # type: ignore[arg-type] diff --git a/tests/test_pages.py b/tests/test_pages.py index 742c0aa..4c96b7a 100644 --- a/tests/test_pages.py +++ b/tests/test_pages.py @@ -4,6 +4,7 @@ from pathlib import Path +import pytest import yaml from docgen.config import Config @@ -150,3 +151,16 @@ def test_index_html_segment_titles_escape_user_strings(tmp_path: Path) -> None: assert "<script>alert(1)</script>" in html assert "