diff --git a/milestones/README.md b/milestones/README.md index 805cb7f..38d03f3 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:** **[generation-segment-strings.md](generation-segment-strings.md)** — -per-segment narration / scene-generation prompts must be YAML strings -at config load. +**Active:** **[pages-config-strings.md](pages-config-strings.md)** — +`pages.docs_dir` / title / extra_links must be typed at config load. **Shipped:** +- **[generation-segment-strings.md](generation-segment-strings.md)** — + per-segment narration / scene-generation prompts must be YAML strings + (#110). - **[path-config-strings.md](path-config-strings.md)** — `env_file` / `repo_root` / `dirs.*` must be YAML strings (#109). - **[visual-map-field-strings.md](visual-map-field-strings.md)** — diff --git a/milestones/generation-segment-strings.md b/milestones/generation-segment-strings.md index 2a4b903..c261da2 100644 --- a/milestones/generation-segment-strings.md +++ b/milestones/generation-segment-strings.md @@ -1,7 +1,7 @@ # Milestone: per-segment generation prompts must be strings -**Status:** Active -**PR:** [#110](https://github.com/jmjava/documentation-generator/pull/110) +**Status:** Shipped +**PR:** #110 **Depends on:** `milestones/generation-model-strings.md` (PR #107), `milestones/path-config-strings.md` (PR #109) diff --git a/milestones/pages-config-strings.md b/milestones/pages-config-strings.md new file mode 100644 index 0000000..3cf3f6f --- /dev/null +++ b/milestones/pages-config-strings.md @@ -0,0 +1,48 @@ +# Milestone: pages docs_dir / title / extra_links must be typed + +**Status:** Active +**PR:** [#111](https://github.com/jmjava/documentation-generator/pull/111) +**Depends on:** `milestones/generation-segment-strings.md` (PR #110), +`milestones/path-config-strings.md` (PR #109), +`milestones/concat-segment-lists.md` (PR #84) + +## Problem + +`env_file` / `dirs.*` are already strings at config load. Pages keys +were not: + +1. **`pages.docs_dir` / `demos_subdir`** — a YAML list Path-joined when + writing `index.html` / `pages.yml` (`Path / list` TypeError). +2. **`pages.title` / `subtitle` / `repo_url`** — a list was interpolated + into HTML as `"['Demo Videos']"`. +3. **`pages.segments..title` / `description`** — same HTML + interpolation; a non-mapping row only failed at `pages` generate + (`RuntimeError`), not at config load. +4. **`pages.extra_links`** — a non-list or non-mapping item only failed + at generate; `href` / `label` were untyped. + +## Goal + +Fail closed at `Config.from_yaml`. Missing keys still use defaults +(`docs`, `demos`, `"Demo Videos"`). Empty `title` / `subtitle` / +`repo_url` remain allowed. + +## Done when + +- [x] Present `pages.docs_dir` / `demos_subdir` must be non-empty YAML + strings. +- [x] Present `pages.title` / `subtitle` / `repo_url` must be YAML + strings (empty allowed). +- [x] `pages.segments` rows must be mappings; present `title` / + `description` must be YAML strings (empty allowed). +- [x] `pages.extra_links` must be a list of mappings with non-empty + `href` strings. +- [x] Tests for list / non-mapping values. +- [x] `ruff check src/ tests/` +- [x] `pytest tests/` +- [x] `docgen benchmark` (no clock change) + +## Out of scope + +- `wizard.default_guidance` type gating is separate. +- Unknown `pages.extra_links` keys besides `href` / `label` are ignored. diff --git a/src/docgen/config.py b/src/docgen/config.py index ead0a0e..efb3c89 100644 --- a/src/docgen/config.py +++ b/src/docgen/config.py @@ -255,14 +255,62 @@ def __post_init__(self) -> None: for key, val in self._block("segment_names").items(): sid = require_yaml_string(key, label="segment_names key", source=src) require_yaml_string(val, label=f"segment_names.{sid}", source=src) - pages_segs = self._block("pages").get("segments") + pages = self._block("pages") + if pages.get("docs_dir") is not None: + require_yaml_string(pages["docs_dir"], label="pages.docs_dir", source=src) + if pages.get("demos_subdir") is not None: + require_yaml_string(pages["demos_subdir"], label="pages.demos_subdir", source=src) + require_optional_yaml_string(pages.get("title"), label="pages.title", source=src) + require_optional_yaml_string(pages.get("subtitle"), label="pages.subtitle", source=src) + require_optional_yaml_string(pages.get("repo_url"), label="pages.repo_url", source=src) + extra_links = pages.get("extra_links") + if extra_links is not None: + if not isinstance(extra_links, list): + raise ConfigError( + f"{src}: pages.extra_links must be a YAML list, " + f"not {type(extra_links).__name__}" + ) + for i, item in enumerate(extra_links): + if not isinstance(item, dict): + raise ConfigError( + f"{src}: pages.extra_links[{i}] must be a YAML mapping, " + f"not {type(item).__name__}" + ) + require_yaml_string( + item.get("href"), + label=f"pages.extra_links[{i}].href", + source=src, + ) + require_optional_yaml_string( + item.get("label"), + label=f"pages.extra_links[{i}].label", + source=src, + ) + pages_segs = pages.get("segments") if pages_segs is not None and not isinstance(pages_segs, dict): raise ConfigError( f"{src}: pages.segments must be a YAML mapping, not {type(pages_segs).__name__}" ) if isinstance(pages_segs, dict): - for key in pages_segs: - require_yaml_string(key, label="pages.segments key", source=src) + for key, spec in pages_segs.items(): + sid = require_yaml_string(key, label="pages.segments key", source=src) + if spec is None: + continue + if not isinstance(spec, dict): + raise ConfigError( + f"{src}: pages.segments.{sid} must be a YAML mapping, " + f"not {type(spec).__name__}" + ) + require_optional_yaml_string( + spec.get("title"), + label=f"pages.segments.{sid}.title", + source=src, + ) + require_optional_yaml_string( + spec.get("description"), + label=f"pages.segments.{sid}.description", + source=src, + ) require_hint_and_context_lists( self._block("narration_from_source"), prefix="narration_from_source", diff --git a/tests/test_config.py b/tests/test_config.py index 2f198c7..f1459e6 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -717,3 +717,27 @@ def test_from_yaml_empty_nfs_segment_system_prompt_allowed(tmp_path: Path) -> No ) c = Config.from_yaml(p) assert c.raw["narration_from_source"]["segments"]["01"]["system_prompt"] == "" + + +def test_from_yaml_list_pages_docs_dir_raises(tmp_path: Path) -> None: + p = tmp_path / "docgen.yaml" + p.write_text("pages:\n docs_dir:\n - docs\n", encoding="utf-8") + with pytest.raises(ConfigError, match="pages.docs_dir must be a YAML string"): + Config.from_yaml(p) + + +def test_from_yaml_list_pages_title_raises(tmp_path: Path) -> None: + p = tmp_path / "docgen.yaml" + p.write_text("pages:\n title:\n - Demo Videos\n", encoding="utf-8") + with pytest.raises(ConfigError, match="pages.title must be a YAML string"): + Config.from_yaml(p) + + +def test_from_yaml_list_pages_segment_title_raises(tmp_path: Path) -> None: + p = tmp_path / "docgen.yaml" + p.write_text( + 'pages:\n segments:\n "01":\n title:\n - Overview\n', + encoding="utf-8", + ) + with pytest.raises(ConfigError, match="pages.segments.01.title must be a YAML string"): + Config.from_yaml(p) diff --git a/tests/test_pages.py b/tests/test_pages.py index fbf739f..e30ac8d 100644 --- a/tests/test_pages.py +++ b/tests/test_pages.py @@ -7,7 +7,7 @@ import pytest import yaml -from docgen.config import Config +from docgen.config import Config, ConfigError from docgen.pages import PagesGenerator, _esc @@ -154,27 +154,25 @@ def test_index_html_segment_titles_escape_user_strings(tmp_path: Path) -> None: def test_index_html_extra_links_rejects_non_mapping_items(tmp_path: Path) -> None: - cfg = _write_pages_cfg( - tmp_path, - { - "title": "Demos", - "demos_subdir": "demos", - "extra_links": ["https://example.com"], - }, - ) - with pytest.raises(RuntimeError, match="extra_links items must be mappings"): - PagesGenerator(cfg).generate_index_html(force=True) + with pytest.raises(ConfigError, match=r"pages.extra_links\[0\] must be a YAML mapping"): + _write_pages_cfg( + tmp_path, + { + "title": "Demos", + "demos_subdir": "demos", + "extra_links": ["https://example.com"], + }, + ) def test_index_html_rejects_non_mapping_pages_segments(tmp_path: Path) -> None: - cfg = _write_pages_cfg( - tmp_path, - { - "title": "Demos", - "demos_subdir": "demos", - "segments": {"01": "Overview"}, - }, - segments_all=["01"], - ) - with pytest.raises(RuntimeError, match="pages.segments.01 must be a YAML mapping"): - PagesGenerator(cfg).generate_index_html(force=True) + with pytest.raises(ConfigError, match="pages.segments.01 must be a YAML mapping"): + _write_pages_cfg( + tmp_path, + { + "title": "Demos", + "demos_subdir": "demos", + "segments": {"01": "Overview"}, + }, + segments_all=["01"], + )