From 48807a26c5adf5c1f4b0294d30884102d8f7a428 Mon Sep 17 00:00:00 2001 From: Josh Mabry Date: Thu, 23 Jul 2026 00:12:28 -0700 Subject: [PATCH] feat: board_create_from_plan: batch-create features from a structured decomposition (#92) --- api.py | 13 +++ store.py | 196 ++++++++++++++++++++++++++++++++++++++++++++ tests/test_api.py | 47 +++++++++++ tests/test_store.py | 159 +++++++++++++++++++++++++++++++++++ 4 files changed, 415 insertions(+) diff --git a/api.py b/api.py index d656d45..c1198ce 100644 --- a/api.py +++ b/api.py @@ -220,6 +220,19 @@ async def _progress(fid: str): async def _create_feature(body: dict = Body(...)): return _guard(lambda: store().create_feature(**body)) + @router.post("/features/batch") + async def _create_from_plan(body: dict = Body(default={})): + """Batch-create a whole decomposition (#92). Body: ``{"plan": [{title, spec, + acceptance_criteria, files, difficulty, depends_on, foundation}, …], + "mark_ready": false}``. All-or-report: a malformed item fails itself with a + named reason, the rest proceed; inter-item ``depends_on`` (by 0-based index or + title) resolves after every create; ``mark_ready`` promotes only clean items.""" + return _guard( + lambda: store().create_from_plan( + (body or {}).get("plan") or [], mark_ready=bool((body or {}).get("mark_ready", False)) + ) + ) + @router.post("/features/{fid}/dep") async def _dep(fid: str, body: dict = Body(...)): """Add a `blocks` edge: `fid` waits for `depends_on` to be merged→done. diff --git a/store.py b/store.py index 8ef2064..6bb09c9 100644 --- a/store.py +++ b/store.py @@ -103,6 +103,49 @@ DIFFICULTY_TIER = {"small": "smart", "medium": "reasoning", "large": "reasoning", "architectural": "opus"} TIER_LADDER = ["smart", "reasoning", "opus"] +# A plan-item `depends_on` entry that is a plain integer is a 0-based INDEX into the +# plan. STRICT — a single optional leading '-' only. The old `lstrip('-').isdigit()` +# guard also accepted multi-dash junk like '--5' (lstrip strips BOTH dashes → '5') +# and then crashed `int('--5')` with an uncaught ValueError, taking the whole batch +# down (#92). Gating int() on this pattern keeps a malformed ref from ever reaching +# int(); a still-numeric-looking miss is named as malformed for that item alone. +_PLAN_INDEX_RE = re.compile(r"-?\d+") + + +def _norm_plan_title(t) -> str: + """Normalize a title for plan-internal dep matching (trim, lowercase, collapse + internal whitespace) — the same normalization the tool-boundary dedup uses.""" + return " ".join(str(t or "").strip().lower().split()) + + +def _plan_item_title(item) -> str: + """The raw title of a plan item, or '' — safe on a non-dict item (used only to + label a malformed item in the failure report).""" + return str(item.get("title") or "") if isinstance(item, dict) else "" + + +def _plan_files(val) -> list[str]: + """Normalize a plan item's `files` (a list of paths, or a comma/newline string) + to a clean list — a bare string must NOT reach create_feature, which iterates it + (char-by-char for a str).""" + if isinstance(val, str): + return [x.strip() for x in val.replace("\n", ",").split(",") if x.strip()] + return [str(p).strip() for p in (val or ()) if str(p).strip()] + + +def _plan_deps(val) -> list: + """Normalize a plan item's `depends_on` (a list, or a comma/newline string) to a + clean list — integer entries (plan indices) are preserved as ints; strings are + trimmed. (bool is dropped: it's an int subclass but never a valid index/id.)""" + if isinstance(val, str): + return [x.strip() for x in val.replace("\n", ",").split(",") if x.strip()] + out: list = [] + for v in val or (): + if isinstance(v, bool): + continue + out.append(v if isinstance(v, int) else str(v).strip()) + return [d for d in out if d != ""] + class BoardError(Exception): """A rejected op (bad gate, unknown feature, `br` failure). Caller → 4xx / tool error.""" @@ -338,6 +381,159 @@ def add_dependency(self, fid: str, depends_on: str) -> None: the foundation feature, so they only become `ready` once it merges → done.""" self._run("dep", "add", fid, depends_on, "--type", "blocks") + # ── batch create from a structured decomposition (#92) ───────────────────── + @staticmethod + def _validate_plan_item(item, index: int) -> str: + """A plan item must be an object carrying a non-empty title. Anything else is + malformed and fails ITSELF (all-or-report) — raise a named reason the caller + records against this item while the rest of the batch proceeds.""" + if not isinstance(item, dict): + raise BoardError(f"plan item {index} is not an object (got {type(item).__name__})") + title = str(item.get("title") or "").strip() + if not title: + raise BoardError(f"plan item {index} has no title") + return title + + @staticmethod + def _resolve_plan_dep(dep, index_to_id: dict, title_to_id: dict) -> str: + """Resolve one plan-item `depends_on` entry to a real feature id. A dep may be + a 0-based plan-item INDEX (an int, or a plain numeric string), the TITLE of + another plan item, or an existing board feature id (passed through untouched — + add_dependency validates it). Raises BoardError with a named reason on anything + unresolvable, so the CALLER fails just that item's edge in place (#92) instead + of letting an uncaught error kill the whole batch.""" + # bool is an int subclass — reject before the int branch swallows True/False. + if isinstance(dep, bool): + raise BoardError(f"dependency {dep!r} is not a valid feature reference") + if isinstance(dep, int): + if dep in index_to_id: + return index_to_id[dep] + raise BoardError(f"plan-item index {dep} is out of range (or its item failed to create)") + s = str(dep).strip() + if not s: + raise BoardError("empty dependency reference") + # A plain integer STRING is a plan-item index. Gate int() on the STRICT + # _PLAN_INDEX_RE (single optional leading '-') so multi-dash junk like '--5' + # never reaches int() and blows up (#92 AC8). + if _PLAN_INDEX_RE.fullmatch(s): + idx = int(s) + if idx in index_to_id: + return index_to_id[idx] + raise BoardError(f"plan-item index {idx} is out of range (or its item failed to create)") + # '--5' passes the OLD loose `lstrip('-').isdigit()` guard but not the strict + # one — name it as a malformed index for THIS item rather than passing it + # downstream (where it would be mis-read as a `br` flag). + if s.lstrip("-").isdigit(): + raise BoardError( + f"dependency {dep!r} looks like a plan-item index but is malformed " + "(only a single optional leading '-' is allowed)" + ) + # otherwise: the title of another plan item, else an existing board feature id. + key = _norm_plan_title(s) + if key in title_to_id: + return title_to_id[key] + return s # assume an existing board feature id; add_dependency validates it + + def create_from_plan(self, plan, mark_ready: bool = False) -> dict: + """Batch-create a whole decomposition in ONE call — ``plan`` is a list of + feature sections (each: title / spec / acceptance_criteria / files / + difficulty / depends_on / foundation). Reuses ``create_feature``'s validation, + enrichment, and success-with-warning contract PER ITEM (#85): a malformed item + fails ITSELF with a named reason and the rest proceed (all-or-report, never + all-or-nothing). The single-create tool is unchanged. + + Dependency edges BETWEEN plan items are resolved AFTER every create — the ids + aren't known up front, so a ``depends_on`` entry may reference another plan + item by 0-based index (int or numeric string) or by title, or name an existing + board feature id; an unresolvable/malformed ref fails that item's edge with a + named reason (success-with-warning), never the batch. With ``mark_ready=True`` + only items that created CLEANLY (no enrichment/dep warning) are promoted.""" + if not isinstance(plan, (list, tuple)): + raise BoardError("plan must be a list of feature sections") + + created: list[tuple[int, dict, dict]] = [] # (plan index, source item, feature) + index_to_id: dict[int, str] = {} + title_to_id: dict[str, str] = {} + results: list[dict] = [] + + # ── phase 1: validate + create each item (deps deferred to phase 2) ────── + for i, item in enumerate(plan): + try: + title = self._validate_plan_item(item, i) + except BoardError as exc: + results.append({"index": i, "created": False, "title": _plan_item_title(item), "error": str(exc)}) + continue + try: + f = self.create_feature( + title, + spec=str(item.get("spec") or ""), + acceptance_criteria=str(item.get("acceptance_criteria") or ""), + design=str(item.get("design") or ""), + files_to_modify=_plan_files(item.get("files", item.get("files_to_modify"))), + parent=str(item.get("parent") or ""), + priority=int(item.get("priority", 2) or 2), + difficulty=str(item.get("difficulty") or ""), + depends_on=(), # wired in phase 2, once every plan-item id is known + foundation=bool(item.get("foundation", False)), + ) + except BoardError as exc: + results.append({"index": i, "created": False, "title": title, "error": str(exc)}) + continue + index_to_id[i] = f["id"] + title_to_id[_norm_plan_title(title)] = f["id"] + created.append((i, item, f)) + r = dict(f) + r["index"] = i + r["created"] = True + results.append(r) + + # ── phase 2: wire inter-item dep edges now every id is resolvable ───────── + result_by_id = {r["id"]: r for r in results if r.get("created")} + for _i, item, f in created: + failed: list[str] = [] + for dep in _plan_deps(item.get("depends_on")): + try: + self.add_dependency(f["id"], self._resolve_plan_dep(dep, index_to_id, title_to_id)) + except BoardError as exc: + failed.append(f"{dep} ({exc})") + if failed: + r = result_by_id[f["id"]] + r["enrichment_failed"] = True + r["missing_fields"] = list(r.get("missing_fields") or []) + [f"depends_on({d})" for d in failed] + prior = f"{r['warning']} " if r.get("warning") else "" + r["warning"] = ( + f"{prior}feature {f['id']} was created but these dependency edges failed: " + f"{'; '.join(failed)} — repair with " + f"board_update_feature(feature_id={f['id']!r}, depends_on=...)." + ) + + # ── phase 3: promote ONLY the cleanly-created items ────────────────────── + if mark_ready: + for _i, _item, f in created: + r = result_by_id[f["id"]] + if r.get("enrichment_failed"): + continue # a warned item isn't clean → don't auto-promote it + try: + self.mark_ready(f["id"]) + r["board_state"] = "ready" + r["ready"] = True + except BoardError as exc: + r["ready"] = False + r["ready_error"] = str(exc) + + n_created = len(created) + return { + "items": results, + "created_ids": [f["id"] for _i, _item, f in created], + "summary": { + "requested": len(plan), + "created": n_created, + "failed": len(plan) - n_created, + "ready": sum(1 for r in results if r.get("ready")), + "warnings": sum(1 for r in results if r.get("enrichment_failed")), + }, + } + # ── partial update (the repair path) ────────────────────────────────────── def update_feature( self, diff --git a/tests/test_api.py b/tests/test_api.py index 1f65534..3aad2ff 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -57,6 +57,15 @@ def create_feature(self, **k): self.calls.append(("create_feature", (), k)) return {"id": "bd-new", "board_state": "backlog", "title": k.get("title", "")} + def create_from_plan(self, plan, mark_ready=False): + self.calls.append(("create_from_plan", (), {"plan": plan, "mark_ready": mark_ready})) + ids = [f"bd-{i}" for i in range(len(plan))] + return { + "items": [{"index": i, "created": True, "id": fid} for i, fid in enumerate(ids)], + "created_ids": ids, + "summary": {"requested": len(plan), "created": len(plan), "failed": 0, "ready": 0, "warnings": 0}, + } + def add_dependency(self, fid, dep): return self._rec("add_dependency", fid, dep) @@ -173,6 +182,44 @@ def test_unknown_feature_is_404(monkeypatch): assert c.get("/api/plugins/project_board/features/missing").status_code == 404 +# ── batch create from a structured decomposition (#92): POST /features/batch ──── + + +def test_batch_route_forwards_plan_and_mark_ready(monkeypatch): + store = FakeStore() + c = _client(monkeypatch, store) + plan = [{"title": "A", "spec": "sa"}, {"title": "B", "spec": "sb"}] + r = c.post("/api/plugins/project_board/features/batch", json={"plan": plan, "mark_ready": True}) + assert r.status_code == 200 + body = r.json() + assert body["created_ids"] == ["bd-0", "bd-1"] + assert body["summary"]["requested"] == 2 + call = next(c for c in store.calls if c[0] == "create_from_plan") + assert call[2] == {"plan": plan, "mark_ready": True} + + +def test_batch_route_defaults_empty_plan_and_is_operator_gated(monkeypatch): + store = FakeStore() + c = _client(monkeypatch, store) + # no body → empty plan, mark_ready False (a valid request, not a 422) + r = c.post("/api/plugins/project_board/features/batch") + assert r.status_code == 200 + call = next(c for c in store.calls if c[0] == "create_from_plan") + assert call[2] == {"plan": [], "mark_ready": False} + # NOT served on the public prefix (that would skip the operator bearer gate) + assert c.post("/plugins/project_board/features/batch", json={"plan": []}).status_code == 404 + + +def test_batch_route_surfaces_a_boarderror_as_400(monkeypatch): + class BrokenStore(FakeStore): + def create_from_plan(self, plan, mark_ready=False): + raise BoardError("plan must be a list of feature sections") + + c = _client(monkeypatch, BrokenStore()) + r = c.post("/api/plugins/project_board/features/batch", json={"plan": "not a list"}) + assert r.status_code == 400 and "plan must be a list" in r.json()["detail"] + + # ── live coder-monitoring snapshot (#84): GET /features/{fid}/progress ─────────── diff --git a/tests/test_store.py b/tests/test_store.py index 122eaff..1a9a94c 100644 --- a/tests/test_store.py +++ b/tests/test_store.py @@ -827,3 +827,162 @@ def test_update_feature_adds_dependency_edges(make_board, monkeypatch): b.update_feature("bd-1", depends_on=["bd-7", "bd-8"]) dep_calls = [c for c in br.calls if c and c[0] == "dep"] assert [(c[2], c[3]) for c in dep_calls] == [("bd-1", "bd-7"), ("bd-1", "bd-8")] + + +# ── create_from_plan: batch-create a decomposition, all-or-report (#92) ────────── + + +def _plan_board(make_board, monkeypatch): + """A board wired for ``create_from_plan``: ``_create`` mints ``bd-`` and registers + a ready-eligible bead (spec + acceptance_criteria + files, so ``mark_ready`` can + promote a clean item), ``get_feature`` returns it, and enrichment / ``dep add`` / + ready ``br update`` calls flow through the recording ``Br`` for assertion.""" + br = Br() + b = make_board(br) + beads: dict[str, dict] = {} + counter = {"n": 0} + + def _create(title, *, itype="feature", parent="", priority=2, description="", external_ref=""): + counter["n"] += 1 + fid = f"bd-{counter['n']}" + beads[fid] = { + "id": fid, + "title": title, + "board_state": "backlog", + "spec": description or "spec", + "acceptance_criteria": "WHEN x THE SYSTEM SHALL y", + "files_to_modify": ["a.py"], + } + return fid + + monkeypatch.setattr(b, "_create", _create) + monkeypatch.setattr(b, "get_feature", lambda fid: beads.get(fid)) + return b, beads, br + + +def test_create_from_plan_creates_every_well_formed_item(make_board, monkeypatch): + b, _beads, _br = _plan_board(make_board, monkeypatch) + out = b.create_from_plan( + [ + {"title": "Feature A", "spec": "sa", "files": "a.py"}, + {"title": "Feature B", "spec": "sb", "files": ["b.py"]}, + ] + ) + assert out["created_ids"] == ["bd-1", "bd-2"] + assert out["summary"] == {"requested": 2, "created": 2, "failed": 0, "ready": 0, "warnings": 0} + assert [r["title"] for r in out["items"]] == ["Feature A", "Feature B"] + assert all(r["created"] for r in out["items"]) + + +def test_create_from_plan_malformed_item_fails_itself_and_the_rest_proceed(make_board, monkeypatch): + b, _beads, _br = _plan_board(make_board, monkeypatch) + out = b.create_from_plan( + [ + {"title": "Good one", "spec": "s", "files": "a.py"}, + {"spec": "no title here"}, # malformed — no title + "not even an object", # malformed — not a dict + {"title": "Also good", "spec": "s", "files": "b.py"}, + ] + ) + assert out["summary"]["requested"] == 4 + assert out["summary"]["created"] == 2 + assert out["summary"]["failed"] == 2 + assert out["created_ids"] == ["bd-1", "bd-2"] # only the well-formed items minted ids + bad = [r for r in out["items"] if not r["created"]] + assert len(bad) == 2 + assert any("no title" in r["error"] for r in bad) + assert any("not an object" in r["error"] for r in bad) + # a failed item still preserves its plan index so the caller can map the reason back + assert {r["index"] for r in bad} == {1, 2} + + +def test_create_from_plan_resolves_inter_item_deps_by_index_and_title(make_board, monkeypatch): + b, _beads, br = _plan_board(make_board, monkeypatch) + out = b.create_from_plan( + [ + {"title": "Foundation", "spec": "s", "files": "f.py", "foundation": True}, + {"title": "Builds via index", "spec": "s", "files": "b.py", "depends_on": [0]}, + {"title": "Builds via title", "spec": "s", "files": "c.py", "depends_on": ["Foundation"]}, + ] + ) + assert out["summary"]["created"] == 3 + assert all(not r.get("enrichment_failed") for r in out["items"]) + # both dependents wired to the foundation's minted id (bd-1) — resolved AFTER all creates + edges = {(a[2], a[3]) for a in br.cmds("dep")} + assert edges == {("bd-2", "bd-1"), ("bd-3", "bd-1")} + + +def test_create_from_plan_double_dash_dep_fails_that_item_not_the_whole_batch(make_board, monkeypatch): + """#92 AC8: a dep like '--5' passes the old ``lstrip('-').isdigit()`` guard but crashes + ``int()`` — it must fail ITS item with a named reason (success-with-warning) while the + rest of the batch proceeds, never take the batch down with an uncaught ValueError.""" + b, _beads, br = _plan_board(make_board, monkeypatch) + out = b.create_from_plan( + [ + {"title": "Fine", "spec": "s", "files": "a.py"}, + {"title": "Bad dep", "spec": "s", "files": "b.py", "depends_on": ["--5"]}, + ] + ) + # the batch survived: both beads were created, no ValueError escaped + assert out["summary"]["created"] == 2 + assert out["created_ids"] == ["bd-1", "bd-2"] + # the '--5' item fails itself, named + repairable; the other stays clean + warned = next(r for r in out["items"] if r["title"] == "Bad dep") + assert warned["created"] is True and warned["enrichment_failed"] is True + assert any("--5" in m for m in warned["missing_fields"]) + assert "--5" in warned["warning"] and "board_update_feature" in warned["warning"] + assert next(r for r in out["items"] if r["title"] == "Fine").get("enrichment_failed") is None + # a malformed ref never reaches `br dep add` + assert br.cmds("dep") == [] + + +def test_create_from_plan_mark_ready_promotes_only_clean_items(make_board, monkeypatch): + b, _beads, br = _plan_board(make_board, monkeypatch) + real_add = b.add_dependency + + def flaky_add(fid, dep): + if dep == "ghost": + raise BoardError("no such issue 'ghost'") + return real_add(fid, dep) + + monkeypatch.setattr(b, "add_dependency", flaky_add) + out = b.create_from_plan( + [ + {"title": "Clean", "spec": "s", "files": "a.py"}, + {"title": "Warned", "spec": "s", "files": "b.py", "depends_on": ["ghost"]}, + ], + mark_ready=True, + ) + clean = next(r for r in out["items"] if r["title"] == "Clean") + warned = next(r for r in out["items"] if r["title"] == "Warned") + assert clean["ready"] is True and clean["board_state"] == "ready" + assert warned.get("ready") is not True # a warned item is NOT auto-promoted + assert warned["enrichment_failed"] is True + assert out["summary"]["ready"] == 1 + # exactly one `ready`-label update fired, for the clean item only + ready_updates = [a for a in br.cmds("update") if "--add-label" in a and "ready" in a] + assert len(ready_updates) == 1 and ready_updates[0][1] == "bd-1" + + +def test_resolve_plan_dep_index_title_and_passthrough_id(): + index_to_id = {0: "bd-1", 1: "bd-2"} + title_to_id = {"foundation feature": "bd-1"} + assert BeadsBoard._resolve_plan_dep(0, index_to_id, title_to_id) == "bd-1" # int index + assert BeadsBoard._resolve_plan_dep("1", index_to_id, title_to_id) == "bd-2" # numeric-string index + assert BeadsBoard._resolve_plan_dep("Foundation Feature", index_to_id, title_to_id) == "bd-1" # by title + assert BeadsBoard._resolve_plan_dep("bd-9", index_to_id, title_to_id) == "bd-9" # literal id passthrough + + +@pytest.mark.parametrize("bad", ["--5", "---7", "--10"]) +def test_resolve_plan_dep_multi_dash_index_is_named_not_a_crash(bad): + """#92 AC8 (unit): multi-dash junk the loose guard accepted raises a NAMED BoardError, + not an uncaught ValueError from ``int()``.""" + with pytest.raises(BoardError, match="malformed"): + BeadsBoard._resolve_plan_dep(bad, {}, {}) + + +def test_resolve_plan_dep_out_of_range_index_raises_named(): + with pytest.raises(BoardError, match="out of range"): + BeadsBoard._resolve_plan_dep("7", {0: "bd-1"}, {}) + with pytest.raises(BoardError, match="out of range"): + BeadsBoard._resolve_plan_dep(-5, {0: "bd-1"}, {})