fix(brief): parse only the leading id-list in Depends on / Conflicts with (#103)

`_id_list` tokenised the *entire* field value, so a trailing rationale —
`139 (no data dependency, but PR-order is kept …)` or an em-dash for "none" —
parsed into bogus ids (`no`, `data`, `but`, `PR-order`, `—`) and `_check_dep_graph`
rejected them, crashing the whole `pdca flow` batch before any bundle ran. These two
are the only list-parsed brief fields, yet the template's own `value (explanation)`
hint teaches authors and the headless planner to write a note on the value line.

Parse only the leading run of id tokens and stop at the first non-id token: an id is
a bare reference (issue number `139`, tracker key `PROJ-12`/`AA`); a rationale word —
lowercase letters and no digit — ends the run, so a value of pure prose or a bare `—`
yields `[]`. Non-numeric ids still parse (the driver keys bundles by arbitrary id).

Also clarify both brief templates' field hints to "ids only on the value line, any
trailing note is ignored" so the example stops modelling the failure.

Tests (test_brief.OrderingFields): trailing parenthetical, em-dash rationale, em-dash-
only "none", and a non-numeric id followed by rationale. Full offline suite: 168 OK.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Eduard Ralph <15236434+eduralph@users.noreply.github.com>

Author: eduralph

template/src/pdca_harness/brief.py

@@ -111,12 +111,27 @@ def onto_branch(brief_path: Path) -> tuple[str, str] | None:
 
 
 def _id_list(raw: str) -> list[str]:
-    """Issue ids out of a comma/space-separated field value, normalised to bare ids.
+    """Issue ids out of the **leading id-list** of a field value, normalised to bare ids.
 
     Tolerates a leading ``#`` and the ``issue_`` bundle prefix so a brief may write
     ``#36`` / ``36`` / ``issue_36`` interchangeably; matches how ``cfg.bundle(id)``
-    keys bundles. Mirrors :func:`test_files`' tokenise-the-value approach.
+    keys bundles.
+
+    Parses only the leading run of id tokens and **stops at the first non-id token**, so
+    a trailing rationale is ignored (issue #103). ``Depends on:`` / ``Conflicts with:``
+    are the only list-parsed brief fields, yet authors and the headless planner routinely
+    append a note — a parenthetical, or an em-dash meaning "none" — mirroring the
+    template's own ``value (explanation)`` hint; left whole, that prose parsed into bogus
+    ids and crashed the whole batch in ``_check_dep_graph``. An id is a bare reference
+    (an issue number ``139``, or a tracker key ``PROJ-12`` / ``AA``); a natural-language
+    rationale word — lowercase letters and no digit (``no``, ``kept``, ``PR-order``) —
+    ends the run, so a value of pure prose or a bare ``—`` for "none" yields ``[]``.
     """
-    if not raw:
-        return []
-    return [t.lstrip("#").removeprefix("issue_") for t in re.findall(r"#?[\w./-]+", raw)]
+    ids: list[str] = []
+    for tok in re.findall(r"#?[\w./-]+", raw or ""):
+        bare = tok.lstrip("#").removeprefix("issue_")
+        is_id = any(ch.isdigit() for ch in bare) or not any(ch.islower() for ch in bare)
+        if not is_id:
+            break  # a rationale word — the id-list has ended
+        ids.append(bare)
+    return ids

template/templates/brief.md.tpl

@@ -18,8 +18,8 @@
   widen it. Omit only for non-structural behavioural bug fixes (principles.md §1.1).>
 - **Repo + branch target:** <owner/repo> @ <branch>   (resolve here at Plan — do not leave to Do)
 - **Onto branch:** <remote>/<existing-pr-branch>   (optional — stack the fix as a commit onto an existing open PR's branch instead of opening a new PR; the fix is tested, committed, and pushed against THIS branch; docs 03)
-- **Depends on:** <id>[, <id>…]   (optional — batch/lane scheduling waits until these bundles are COMPLETE before this one runs; docs 09)
-- **Conflicts with:** <id>[, <id>…]   (optional — never co-schedule these in the same concurrent wave, e.g. they edit a shared file; docs 09)
+- **Depends on:** <id>[, <id>…]   (optional — ids only on the value line, any trailing note is ignored; batch/lane scheduling waits until these bundles are COMPLETE before this one runs; docs 09)
+- **Conflicts with:** <id>[, <id>…]   (optional — ids only on the value line, any trailing note is ignored; never co-schedule these in the same concurrent wave, e.g. they edit a shared file; docs 09)
 - **Surfaces:** <where the change is observable — `gui` (touches the frontend / an E2E
   through the app is needed), `data` (backend/logic only), or `both`. Drives which
   runtime gates apply (e.g. an E2E gate runs only when this is `gui`). Optional.>

template/templates/plan-pointer.md.tpl

@@ -18,8 +18,8 @@
 - **Success criterion:** <the observable condition that means it works — what the shipped test asserts>
 - **Repo + branch target:** <owner/repo> @ <branch>   (resolve here at Plan — do not leave to Do)
 - **Onto branch:** <remote>/<existing-pr-branch>   (optional — stack onto an open PR's branch; docs 03)
-- **Depends on:** <id>[, <id>…]   (optional — scheduling waits until these are COMPLETE; docs 09)
-- **Conflicts with:** <id>[, <id>…]   (optional — never co-schedule in one wave; docs 09)
+- **Depends on:** <id>[, <id>…]   (optional — ids only, any trailing note is ignored; scheduling waits until these are COMPLETE; docs 09)
+- **Conflicts with:** <id>[, <id>…]   (optional — ids only, any trailing note is ignored; never co-schedule in one wave; docs 09)
 - **Scope:** <the one logical change this realizes> / out of scope: <what is excluded>
 - **Test file:** <path where the regression test ships — must fail pre-change, pass post-change>
 - **Citations expected:** Do must cite path:line on the target branch AND the Planning artifact for every change.

template/tests/test_brief.py

@@ -64,6 +64,29 @@ def test_empty_value_is_empty_list(self) -> None:
         self.assertEqual(brief.depends_on(f), [])
         self.assertEqual(brief.conflicts_with(f), [])
 
+    def test_trailing_parenthetical_rationale_is_ignored(self) -> None:
+        # The crash in #103: the planner mimics the template's `value (explanation)`
+        # hint, so the field carries a note after the id. Only the leading id parses.
+        f = self._brief(
+            "- **Depends on:** 139   (no data dependency, but PR-order is kept so this "
+            "waits on #139)\n")
+        self.assertEqual(brief.depends_on(f), ["139"])
+
+    def test_trailing_em_dash_rationale_is_ignored(self) -> None:
+        f = self._brief("- **Depends on:** 12, 13 — kept in PR order\n")
+        self.assertEqual(brief.depends_on(f), ["12", "13"])
+
+    def test_em_dash_only_value_means_none(self) -> None:
+        # "—" is the conventional "none"; it must not parse to a bogus ['—'] id.
+        f = self._brief("- **Conflicts with:** —\n")
+        self.assertEqual(brief.conflicts_with(f), [])
+
+    def test_non_numeric_ids_survive_a_trailing_rationale(self) -> None:
+        # Ids needn't be numeric (the driver keys bundles by arbitrary id); a tracker-key
+        # id is kept, while the lowercase rationale that follows is dropped.
+        f = self._brief("- **Depends on:** AA, PROJ-12 — keep PR order\n")
+        self.assertEqual(brief.depends_on(f), ["AA", "PROJ-12"])
+
 
 if __name__ == "__main__":
     unittest.main()