Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions milestones/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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-state-segments.md](wizard-state-segments.md)** —
wizard `.docgen-state.json` `segments` must be a mapping of objects.
**Active:** **[wizard-json-object.md](wizard-json-object.md)** —
wizard POST/PUT bodies must be JSON objects; bool fields must be booleans.

**Shipped:**
- **[wizard-state-segments.md](wizard-state-segments.md)** —
wizard `.docgen-state.json` `segments` must be a mapping of objects
(#128).
- **[timing-inner-lists.md](timing-inner-lists.md)** —
`timing.json` `words` / `segments` must be JSON arrays of objects
(#127).
Expand Down
34 changes: 34 additions & 0 deletions milestones/wizard-json-object.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# Milestone: wizard POST bodies must be JSON objects

**Status:** Active
**PR:** [#129](https://github.com/jmjava/documentation-generator/pull/129)
**Depends on:** `milestones/wizard-state-segments.md` (PR #128)

## Problem

PR #128 typed ``POST /api/state``. Other wizard POST/PUT handlers still
did ``request.json or {}`` then ``.get``. A JSON **array** is truthy, so
the default never applied and Flask raised ``AttributeError``.

``bool(data.get("with_manim", False))`` treated the string ``"false"`` as
true.

## Goal

Every wizard JSON body must be an object (missing body still ``{}``).
Boolean fields (``with_manim``, ``update_requirements``, ``also_manim``,
``yaml_generate``, ``llm_scene_spec``) must be JSON booleans when present.

## Done when

- [x] List / scalar POST bodies return 400 on all wizard JSON endpoints
- [x] String ``"false"`` / ``"true"`` for bool fields return 400
- [x] Existing object-body tests still pass
- [x] `ruff check src/ tests/`
- [x] `pytest tests/` (745 passed, 1 skipped)
- [x] `docgen benchmark` (no clock change; meets baseline)

## Out of scope

- Invalid JSON still becomes ``{}`` via ``get_json(silent=True)``
- Wizard ``except Exception`` around ``narration_topic_label``
2 changes: 1 addition & 1 deletion milestones/wizard-state-segments.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Milestone: wizard state segments must be objects

**Status:** Active
**Status:** Shipped
**PR:** [#128](https://github.com/jmjava/documentation-generator/pull/128)
**Depends on:** `milestones/timing-inner-lists.md` (PR #127)

Expand Down
72 changes: 56 additions & 16 deletions src/docgen/wizard.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,32 @@ def require_state_segments(data: dict[str, Any], *, label: str) -> dict[str, Any
return segs


def request_json_object() -> dict[str, Any]:
"""Return the JSON request body as an object. A missing body is ``{}``.

A JSON array, string, number, bool, or ``null`` raises :class:`WizardError`
so handlers cannot ``.get`` on a list.
"""
raw = request.get_json(silent=True)
if raw is None:
return {}
if not isinstance(raw, dict):
raise WizardError(
f"request body must be a JSON object, not {_json_kind(raw)}"
)
return raw


def require_json_bool(data: dict[str, Any], key: str, *, default: bool) -> bool:
"""Return ``data[key]`` when present; reject non-bool JSON (including ``\"false\"``)."""
if key not in data or data[key] is None:
return default
val = data[key]
if not isinstance(val, bool):
raise WizardError(f"{key} must be a JSON boolean, not {_json_kind(val)}")
return val


def session_payload(config: Any | None) -> dict[str, Any]:
"""GUI session: frozen shell vs pip CLI, and whether a bundle is attached."""
from docgen.resources import is_frozen
Expand Down Expand Up @@ -477,7 +503,10 @@ def api_session():

@app.route("/api/open-bundle", methods=["POST"])
def api_open_bundle():
data = request.get_json(silent=True) or {}
try:
data = request_json_object()
except WizardError as exc:
return jsonify({"error": str(exc)}), 400
try:
cfg = open_bundle_config(str(data.get("path") or ""))
except ValueError as exc:
Expand Down Expand Up @@ -514,10 +543,13 @@ def api_tool_update():
if blocked is not None:
return blocked
cfg = _cfg()
data = request.json or {}
try:
data = request_json_object()
with_manim = require_json_bool(data, "with_manim", default=False)
update_req = require_json_bool(data, "update_requirements", default=True)
except WizardError as exc:
return jsonify({"error": str(exc)}), 400
ref = str(data.get("ref") or "main")
with_manim = bool(data.get("with_manim", False))
update_req = bool(data.get("update_requirements", True))
bundle = cfg.base_dir if cfg else None
try:
result = update_docgen_install(
Expand Down Expand Up @@ -577,7 +609,10 @@ def api_generate_narration():
if blocked is not None:
return blocked
cfg = _cfg()
data = request.json or {}
try:
data = request_json_object()
except WizardError as exc:
return jsonify({"error": str(exc)}), 400
source_paths: list[str] = list(data.get("source_paths") or [])
guidance: str = data.get("guidance", "")
segment_name: str = data.get("segment_name", "untitled")
Expand Down Expand Up @@ -686,12 +721,8 @@ def api_get_state():
def api_set_state():
cfg = _cfg()
base = cfg.base_dir if cfg else Path.cwd()
raw = request.get_json(silent=True)
if raw is None:
raw = {}
if not isinstance(raw, dict):
return jsonify({"error": "state must be a JSON object"}), 400
try:
raw = request_json_object()
segs = require_state_segments(raw, label="state")
except WizardError as exc:
return jsonify({"error": str(exc)}), 400
Expand Down Expand Up @@ -792,12 +823,15 @@ def api_put_focus(segment_id: str):
cfg = _cfg()
if not cfg:
return jsonify({"error": "no config"}), 400
data = request.json or {}
try:
data = request_json_object()
also_manim = require_json_bool(data, "also_manim", default=True)
do_yaml = require_json_bool(data, "yaml_generate", default=True)
except WizardError as exc:
return jsonify({"error": str(exc)}), 400
paths = data.get("paths")
if not isinstance(paths, list):
return jsonify({"error": "paths must be a list of repo-root-relative strings"}), 400
also_manim = data.get("also_manim", True)
do_yaml = data.get("yaml_generate", True)

root = cfg.repo_root.resolve()
clean: list[str] = []
Expand Down Expand Up @@ -879,7 +913,10 @@ def api_put_narration(segment_id: str):
cfg = _cfg()
if not cfg:
return jsonify({"error": "no config"}), 400
data = request.json or {}
try:
data = request_json_object()
except WizardError as exc:
return jsonify({"error": str(exc)}), 400
text = data.get("text", "")
seg_name = cfg.resolve_segment_name(segment_id)
found = _find_asset(cfg.narration_dir, seg_name, segment_id, ".md")
Expand Down Expand Up @@ -1056,8 +1093,11 @@ def api_run_from(step: str, segment_id: str):
cfg = _cfg()
if not cfg:
return jsonify({"error": "no config"}), 400
data = request.json or {}
llm_scene = bool(data.get("llm_scene_spec", False))
try:
data = request_json_object()
llm_scene = require_json_bool(data, "llm_scene_spec", default=False)
except WizardError as exc:
return jsonify({"error": str(exc)}), 400
from docgen.asset_graph import cascade_steps

try:
Expand Down
43 changes: 42 additions & 1 deletion tests/test_wizard.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ def test_api_state_post_rejects_list_body(tmp_path):
client, _cfg = _wizard_client(tmp_path)
res = client.post("/api/state", json=["not", "an", "object"])
assert res.status_code == 400
assert res.get_json()["error"] == "state must be a JSON object"
assert res.get_json()["error"] == "request body must be a JSON object, not list"


def test_api_state_post_rejects_list_segments(tmp_path):
Expand All @@ -158,6 +158,47 @@ def test_api_state_roundtrip_object_segments(tmp_path):
assert (cfg.base_dir / ".docgen-state.json").is_file()


def test_api_post_rejects_list_json_bodies(tmp_path):
client, _cfg = _wizard_client(tmp_path)
endpoints = (
("POST", "/api/open-bundle"),
("POST", "/api/tool/update"),
("POST", "/api/generate-narration"),
("POST", "/api/run-from/tts/01"),
("PUT", "/api/narration/01"),
("PUT", "/api/segments/01/focus"),
)
for method, path in endpoints:
res = client.open(path, method=method, json=["not", "an", "object"])
assert res.status_code == 400, path
assert res.get_json()["error"] == "request body must be a JSON object, not list"


def test_api_tool_update_rejects_string_with_manim(tmp_path):
client, _cfg = _wizard_client(tmp_path)
res = client.post("/api/tool/update", json={"with_manim": "false"})
assert res.status_code == 400
assert "with_manim must be a JSON boolean" in res.get_json()["error"]


def test_api_run_from_rejects_string_llm_scene_spec(tmp_path):
client, _cfg = _wizard_client(tmp_path)
res = client.post("/api/run-from/tts/01", json={"llm_scene_spec": "true"})
assert res.status_code == 400
assert "llm_scene_spec must be a JSON boolean" in res.get_json()["error"]


def test_api_put_focus_rejects_string_yaml_generate(tmp_path):
client, _cfg = _wizard_client(tmp_path)
res = client.put(
"/api/segments/01/focus",
json={"paths": ["README.md"], "yaml_generate": "true"},
)
assert res.status_code == 400
assert "yaml_generate must be a JSON boolean" in res.get_json()["error"]



def test_api_file_rejects_prefix_escape(tmp_path):
from docgen.config import Config
from docgen.wizard import create_app
Expand Down