Fail closed when discovery flags are not YAML booleans

milestones/README.md

@@ -5,10 +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:** **[wizard-default-guidance.md](wizard-default-guidance.md)** —
-`wizard.default_guidance` must be a YAML string at config load.
+**Active:** **[discovery-bool-flags.md](discovery-bool-flags.md)** —
+`discovery.auto_visual_map` / `merge_hint_segments` must be YAML
+booleans (`0` / `"false"` used to fail open).
 
 **Shipped:**
+- **[wizard-default-guidance.md](wizard-default-guidance.md)** —
+  `wizard.default_guidance` must be a YAML string at config load (#113).
 - **[visual-map-sources.md](visual-map-sources.md)** —
   `visual_map` mixed `sources` must be a YAML list of strings (#112).
 - **[pages-config-strings.md](pages-config-strings.md)** —

milestones/discovery-bool-flags.md

@@ -0,0 +1,47 @@
+# Milestone: discovery flags must be YAML booleans
+
+**Status:** Active  
+**PR:** [#114](https://github.com/jmjava/documentation-generator/pull/114)  
+**Depends on:** `milestones/wizard-default-guidance.md` (PR #113),
+`milestones/yaml-generate-mappings.md` (PR #91)
+
+## Problem
+
+`yaml-generate` treats **`discovery.auto_visual_map: false`** as the
+opt-out that keeps a committed `visual_map`, and
+**`discovery.merge_hint_segments: false`** as the opt-out that skips
+hint-driven segment / wiring merges.
+
+Those checks used identity **`is False`**. YAML integer `0` and the
+quoted string `"false"` are not the boolean `False`, so:
+
+1. `auto_visual_map: 0` (or `"false"`) still ran discovery and could
+   rewrite `visual_map`.
+2. `merge_hint_segments: 0` (or `"false"`) still merged hint segments
+   and wiring.
+
+Missing keys stay on (current defaults). YAML `false` / `true` still
+work.
+
+## Goal
+
+Fail closed at `Config.from_yaml` and again in `yaml-generate` (hint
+merge can inject flags after load). Present values must be YAML
+booleans.
+
+## Done when
+
+- [x] Present `discovery.auto_visual_map` / `merge_hint_segments` must
+      be YAML booleans.
+- [x] `yaml-generate` discovery / hint-merge helpers raise on `0` /
+      `"false"` instead of treating them as off or on.
+- [x] Tests for integer `0`, quoted `"false"`, and real YAML bools.
+- [x] `ruff check src/ tests/`
+- [x] `pytest tests/` (662 passed, 1 skipped)
+- [x] `docgen benchmark` (no clock change; meets baseline)
+
+## Out of scope
+
+- Coercing `0` / `"false"` / `"no"` into booleans.
+- Other config booleans (`manim.scene_lint`,
+  `validation.*.enabled`) still use Python `bool()`.

milestones/wizard-default-guidance.md

@@ -1,6 +1,6 @@
 # Milestone: wizard.default_guidance must be a string
 
-**Status:** Active  
+**Status:** Shipped  
 **PR:** [#113](https://github.com/jmjava/documentation-generator/pull/113)  
 **Depends on:** `milestones/wizard-prompt-strings.md` (PR #100),
 `milestones/visual-map-sources.md` (PR #112)

src/docgen/config.py

@@ -86,6 +86,21 @@ def require_optional_yaml_string(value: Any, *, label: str, source: str) -> None
         )
 
 
+def require_yaml_bool(value: Any, *, label: str, source: str) -> bool:
+    """Require a YAML boolean so ``0`` / ``\"false\"`` are not misread as off/on.
+
+    Identity checks like ``value is False`` are false for integer ``0`` and
+    the string ``\"false\"``, which used to leave ``discovery.auto_visual_map``
+    enabled and rewrite ``visual_map``.
+    """
+    if isinstance(value, bool):
+        return value
+    raise ConfigError(
+        f"{source}: {label} must be a YAML boolean, not {type(value).__name__} "
+        f"({value!r})"
+    )
+
+
 def require_yaml_string(value: Any, *, label: str, source: str) -> str:
     """Require a YAML string so unquoted ``01`` is not silently coerced to ``\"1\"``."""
     if isinstance(value, str):
@@ -439,6 +454,12 @@ def __post_init__(self) -> None:
             require_yaml_string(self.raw["env_file"], label="env_file", source=src)
         if self.raw.get("repo_root") is not None:
             require_yaml_string(self.raw["repo_root"], label="repo_root", source=src)
+        disc = self._block("discovery")
+        for dkey in ("auto_visual_map", "merge_hint_segments"):
+            if disc.get(dkey) is not None:
+                require_yaml_bool(
+                    disc[dkey], label=f"discovery.{dkey}", source=src
+                )
         ocr = self._sub_block(validation, "ocr", label="validation.ocr")
         if ocr.get("error_patterns") is not None:
             string_list_block(

src/docgen/yaml_generate.py

@@ -62,6 +62,24 @@ def _require_yaml_mapping(value: Any, *, label: str) -> dict[str, Any]:
     return value
 
 
+def _discovery_flag_off(raw: dict[str, Any], key: str) -> bool:
+    """True when ``discovery.<key>`` is YAML ``false``.
+
+    Missing or null keeps the feature on. A present non-bool (``0``,
+    ``\"false\"``) used to fail open: ``is False`` is false for those
+    values, so ``auto_visual_map`` still rewrote ``visual_map``.
+    """
+    from docgen.config import require_yaml_bool
+
+    disc = raw.get("discovery")
+    if not isinstance(disc, dict):
+        return False
+    val = disc.get(key)
+    if val is None:
+        return False
+    return require_yaml_bool(val, label=f"discovery.{key}", source="docgen.yaml") is False
+
+
 def narration_segment_pairs(narration_dir: Path) -> list[tuple[str, str]]:
     """Return sorted (seg_id, stem) from ``narration/<NN-name>.md`` (skip README)."""
     if not narration_dir.is_dir():
@@ -305,8 +323,7 @@ def collect_hint_project_blocks(hints_dir: Path) -> dict[str, Any]:
 
 def merge_hint_project(raw: dict[str, Any], cfg: "Config") -> list[str]:
     """Apply ``docgen.project`` from hints (env_file, narration_from_source, concat, discovery)."""
-    disc = raw.get("discovery")
-    if isinstance(disc, dict) and disc.get("merge_hint_segments") is False:
+    if _discovery_flag_off(raw, "merge_hint_segments"):
         return []
     project = collect_hint_project_blocks(cfg.hints_dir)
     if not project:
@@ -365,8 +382,7 @@ def _segment_lists_to_update(raw: dict[str, Any]) -> list[list[str]]:
 
 def merge_hint_declared_segments(raw: dict[str, Any], cfg: "Config") -> list[str]:
     """Insert ids into ``segments`` lists and ``segment_names`` from ``hints/*.md`` front matter."""
-    disc = raw.get("discovery")
-    if isinstance(disc, dict) and disc.get("merge_hint_segments") is False:
+    if _discovery_flag_off(raw, "merge_hint_segments"):
         return []
     declared = collect_hint_segment_declarations(cfg.hints_dir)
     if not declared:
@@ -599,8 +615,7 @@ def ensure_segment_hint_with_focus(
 
 def merge_hint_wiring(raw: dict[str, Any], cfg: "Config") -> list[str]:
     """Apply ``visual`` overrides from hints, re-sync Manim lists, then merge narration / manim_scene blocks."""
-    disc = raw.get("discovery")
-    hint_merge_off = isinstance(disc, dict) and disc.get("merge_hint_segments") is False
+    hint_merge_off = _discovery_flag_off(raw, "merge_hint_segments")
     wirings = {} if hint_merge_off else collect_hint_wirings_by_segment(cfg.hints_dir)
     changes: list[str] = []
 
@@ -718,8 +733,7 @@ def discover_visual_map(raw: dict[str, Any], cfg: "Config") -> list[str]:
 
     Set ``discovery: { auto_visual_map: false }`` to skip and keep existing ``visual_map``.
     """
-    disc = raw.get("discovery")
-    if isinstance(disc, dict) and disc.get("auto_visual_map") is False:
+    if _discovery_flag_off(raw, "auto_visual_map"):
         return []
 
     seg_block = raw.get("segments")

tests/test_config.py

@@ -780,3 +780,41 @@ def test_from_yaml_empty_wizard_default_guidance_allowed(tmp_path: Path) -> None
     p.write_text('wizard:\n  default_guidance: ""\n', encoding="utf-8")
     c = Config.from_yaml(p)
     assert c.wizard_config["default_guidance"] == ""
+
+
+def test_from_yaml_int_auto_visual_map_raises(tmp_path: Path) -> None:
+    p = tmp_path / "docgen.yaml"
+    p.write_text("discovery:\n  auto_visual_map: 0\n", encoding="utf-8")
+    with pytest.raises(
+        ConfigError, match="discovery.auto_visual_map must be a YAML boolean"
+    ):
+        Config.from_yaml(p)
+
+
+def test_from_yaml_string_auto_visual_map_raises(tmp_path: Path) -> None:
+    p = tmp_path / "docgen.yaml"
+    p.write_text('discovery:\n  auto_visual_map: "false"\n', encoding="utf-8")
+    with pytest.raises(
+        ConfigError, match="discovery.auto_visual_map must be a YAML boolean"
+    ):
+        Config.from_yaml(p)
+
+
+def test_from_yaml_int_merge_hint_segments_raises(tmp_path: Path) -> None:
+    p = tmp_path / "docgen.yaml"
+    p.write_text("discovery:\n  merge_hint_segments: 0\n", encoding="utf-8")
+    with pytest.raises(
+        ConfigError, match="discovery.merge_hint_segments must be a YAML boolean"
+    ):
+        Config.from_yaml(p)
+
+
+def test_from_yaml_discovery_bools_allowed(tmp_path: Path) -> None:
+    p = tmp_path / "docgen.yaml"
+    p.write_text(
+        "discovery:\n  auto_visual_map: false\n  merge_hint_segments: true\n",
+        encoding="utf-8",
+    )
+    c = Config.from_yaml(p)
+    assert c.raw["discovery"]["auto_visual_map"] is False
+    assert c.raw["discovery"]["merge_hint_segments"] is True

tests/test_yaml_generate.py

@@ -7,7 +7,7 @@
 import pytest
 import yaml
 
-from docgen.config import Config
+from docgen.config import Config, ConfigError
 from docgen.yaml_generate import (
     collect_hint_project_blocks,
     collect_hint_segment_declarations,
@@ -256,6 +256,51 @@ def test_discover_visual_map_skipped_when_disabled(tmp_path: Path) -> None:
     assert raw["visual_map"]["01"]["scene"] == "KeepScene"
 
 
+def test_discover_visual_map_int_auto_visual_map_raises(tmp_path: Path) -> None:
+    """``0 is False`` is false — used to keep discovery on and rewrite visual_map."""
+    raw = {
+        "repo_root": ".",
+        "dirs": {
+            "narration": "narration",
+            "audio": "audio",
+            "animations": "animations",
+            "recordings": "recordings",
+        },
+        "segments": {"all": ["01"], "default": ["01"]},
+        "discovery": {"auto_visual_map": True},
+        "visual_map": {"01": {"type": "manim", "scene": "KeepScene", "source": "KeepScene.mp4"}},
+    }
+    (tmp_path / "docgen.yaml").write_text(yaml.dump(raw), encoding="utf-8")
+    cfg = Config.from_yaml(tmp_path / "docgen.yaml")
+    cfg.raw["discovery"]["auto_visual_map"] = 0
+    with pytest.raises(ConfigError, match="discovery.auto_visual_map must be a YAML boolean"):
+        discover_visual_map(cfg.raw, cfg)
+    assert cfg.raw["visual_map"]["01"]["scene"] == "KeepScene"
+
+
+def test_merge_hint_declared_segments_string_false_raises(tmp_path: Path) -> None:
+    hints = tmp_path / "hints"
+    hints.mkdir()
+    (hints / "decl.md").write_text(
+        "---\ndocgen:\n  segment:\n    create: true\n    id: \"05\"\n    stem: 05-x\n---\n",
+        encoding="utf-8",
+    )
+    raw = {
+        "repo_root": ".",
+        "dirs": {"hints": "hints"},
+        "segments": {"default": ["01"], "all": ["01"]},
+        "discovery": {"merge_hint_segments": True},
+    }
+    (tmp_path / "docgen.yaml").write_text(yaml.dump(raw), encoding="utf-8")
+    cfg = Config.from_yaml(tmp_path / "docgen.yaml")
+    cfg.raw["discovery"]["merge_hint_segments"] = "false"
+    with pytest.raises(
+        ConfigError, match="discovery.merge_hint_segments must be a YAML boolean"
+    ):
+        merge_hint_declared_segments(cfg.raw, cfg)
+    assert cfg.raw["segments"]["all"] == ["01"]
+
+
 def test_discover_visual_map_preserves_still_and_fills_manim_slots(tmp_path: Path) -> None:
     (tmp_path / "animations").mkdir()
     (tmp_path / "animations" / "scenes.py").write_text(