feat: ship every activity diagram, linked per sequence-diagram variant (#26)

* feat: ship every activity diagram, linked per SD variant

webapp_export looked for a process's activity diagram at the bare {pid}.svg
only. But p11 names them after the document's own AD headings, so a process
with several sequence-diagram variants has {pid}_{slug} files — note the SINGLE
underscore, where the sequence artifacts use {pid}__{slug}. On the v0.0.9
dataset that mismatch left 115 of 173 rendered diagrams unreachable: they were
generated, committed and then dropped here without a word.

Resolve each variant's diagram via {pid}_{slug} with the bare {pid} as
fallback, and record the result per diagram as activitySvg (None when that
variant has none, so the app never links a 404). Copy EVERY rendered diagram
into the webapp, not just the linked ones, and emit activity_diagrams.json
indexing all of them with a `linked` flag — the ones whose p11 names predate
the UC de-truncation still have no process to hang off, but they are at least
reachable now. Unlinked names print as a worklist, like unresolved refs.

On the v0.0.9 dataset: 173 diagrams copied (was 58), 82 linked, and processes
showing an activity diagram go from 58 to 68 of 194.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: keep an AD-only process, and report ambiguous activity-diagram names

Review findings on the per-variant resolution:

- `any(ad_for_slug)` is False for a process with no sequence diagram at all,
  so a process whose Aktivitätsdiagramm exists without an SD would have lost
  its diagram — reintroducing, in a new place, the silent drop this branch
  removes. p11 renders ADs independently of p06, so fall back to the bare
  artifact when the process has no SD variants.
- `{pid}_{slug}` is indistinguishable from a bare id ending in that slug
  (`wechsel` + `lieferant` vs a process `wechsel_lieferant`). First claim wins,
  which is deterministic because processes are walked in sorted order, but the
  collision is now reported instead of silently absorbed. It does not occur on
  the v0.0.9 dataset.
- build_detail emits activitySvg as None rather than a path it cannot verify;
  run() attaches the real one, the same contract as `approval`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs: state the naming trap without dataset-specific counts

Copilot review: the comments pinned counts from one dataset version, and
disagreed with each other about which count. Numbers that drift with every
regeneration do not belong in code comments — the measured figures live in the
PR description, stamped to the dataset tag they were measured on.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: hf-kklein <konstantin.klein+claude@hochfrequenz.de>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: 3 people

src/makoralle/grouping.py

@@ -68,6 +68,19 @@ def sd_artifact_key(uc_id: str, slug: str, n_sds: int) -> str:
     return uc_id if n_sds <= 1 else f"{uc_id}__{slug}"
 
 
+def ad_artifact_key(uc_id: str, slug: str, n_sds: int) -> str:
+    """Artifact filename key for an activity diagram (``output/bpmn/``).
+
+    Same shape as :func:`sd_artifact_key` but with a SINGLE underscore, because
+    p11 names the activity diagrams after the document's own AD headings:
+    ``bestellung_zur_stammdatenänderung_an_lf_verantwortlich`` next to the SD's
+    ``bestellung_zur_stammdatenänderung__an_lf_verantwortlich``. Getting this
+    separator wrong silently strands every variant's diagram, so it is spelled out
+    here rather than inlined at the call site.
+    """
+    return uc_id if n_sds <= 1 else f"{uc_id}_{slug}"
+
+
 def _parent(section_id: str) -> str:
     return section_id.rsplit(".", 1)[0] if "." in section_id else section_id
 

src/makoralle/webapp_export.py

@@ -17,7 +17,7 @@
 
 import yaml
 
-from makoralle.grouping import sd_artifact_key
+from makoralle.grouping import ad_artifact_key, sd_artifact_key
 from makoralle.ref_links import build_ref_map, load_ref_overrides, resolve_ref
 
 
@@ -142,6 +142,10 @@ def build_detail(process: dict[str, Any], *, review_notes: list[str]) -> dict[st
                 "deadlines": _deadline_table(d_steps),
                 "pids": _pid_table(d_steps),
                 "svg": f"/diagrams/sequence/{key}.svg",
+                # Attached by run(), which can see which artifact actually exists;
+                # build_detail has no filesystem, so it emits None rather than a
+                # path that may 404 (same contract as `approval` below).
+                "activitySvg": None,
             }
         )
     # Back-compat: the top-level steps/deadlines/pids/participants mirror the
@@ -314,6 +318,12 @@ def run(  # pylint: disable=too-many-locals,too-many-branches,too-many-statement
     # Every current diagram artifact key we consult, so after the loop we can spot
     # approval ENTRIES that match no diagram at all (variant removed / slug renamed).
     consulted_keys: set[str] = set()
+    # Activity-diagram artifact -> the process that claims it. Whatever is left over
+    # is still copied and indexed, but is not reachable from a process page. Two
+    # processes claiming one artifact means p11's naming is ambiguous (a bare id that
+    # looks like another process's {pid}_{slug}), so record it rather than absorb it.
+    claimed_ads: dict[str, str] = {}
+    contested_ads: list[str] = []
 
     # Load every process up front so the subprocess-ref resolver sees ALL SD
     # variants (a ref names another process's SD, which may sort later).
@@ -352,7 +362,24 @@ def run(  # pylint: disable=too-many-locals,too-many-branches,too-many-statement
                     if step["ref_target"] is None:
                         unresolved_refs.add(ref)
         keys = [sd_artifact_key(pid, d.get("slug", ""), len(diagrams)) for d in diagrams]
-        has_bpmn = (bpmn_svg / f"{pid}.svg").exists()
+        # Activity diagrams are keyed per SD variant ({pid}_{slug}), with the bare
+        # {pid} as fallback — checking only the bare name leaves every variant's
+        # diagram unreachable. Record which artifacts got claimed so the leftovers
+        # can be listed (and still shipped) below.
+        ad_keys = [ad_artifact_key(pid, d.get("slug", ""), len(diagrams)) for d in diagrams]
+        ad_for_slug: list[str | None] = []
+        for ad_key in ad_keys:
+            found = next((k for k in (ad_key, pid) if (bpmn_svg / f"{k}.svg").exists()), None)
+            ad_for_slug.append(found)
+            if found and claimed_ads.setdefault(found, pid) != pid:
+                contested_ads.append(f"{found} (claimed by {claimed_ads[found]} and {pid})")
+        # A process with no sequence diagram at all still gets its bare activity
+        # diagram: p11 renders ADs independently of SDs, and `any([])` would have
+        # dropped it — the exact silent-drop this change exists to remove.
+        bare_only = not diagrams and (bpmn_svg / f"{pid}.svg").exists()
+        if bare_only:
+            claimed_ads.setdefault(pid, pid)
+        has_bpmn = any(ad_for_slug) or bare_only
         has_seq = any((seq_svg / f"{key}.svg").exists() for key in keys)
         # [REVIEW] notes ("Prüfung nötig" worklist) can live in ANY SD's .wsd;
         # aggregate across all, de-duplicating while preserving order.
@@ -393,6 +420,10 @@ def run(  # pylint: disable=too-many-locals,too-many-branches,too-many-statement
             d_svg = seq_svg / f"{key}.svg"
             if d_svg.exists():
                 shutil.copyfile(d_svg, dest_seq / f"{key}.svg")
+        # Point each diagram at the activity artifact that actually exists (None
+        # when this variant has none), so the app never links a 404.
+        for diagram, resolved_ad in zip(detail["diagrams"], ad_for_slug, strict=True):
+            diagram["activitySvg"] = f"/diagrams/bpmn/{resolved_ad}.svg" if resolved_ad else None
         # detail.approval = the PRIMARY diagram's approval (back-compat; single-SD
         # key == pid so this equals the old {pid}.wsd result).
         detail["approval"] = detail["diagrams"][0]["approval"] if detail["diagrams"] else None
@@ -413,8 +444,32 @@ def run(  # pylint: disable=too-many-locals,too-many-branches,too-many-statement
         # on the file itself, not has_seq (multi-SD has only {pid}__{slug}.svg).
         if (seq_svg / f"{pid}.svg").exists():
             shutil.copyfile(seq_svg / f"{pid}.svg", dest_seq / f"{pid}.svg")
-        if has_bpmn:
-            shutil.copyfile(bpmn_svg / f"{pid}.svg", dest_bpmn / f"{pid}.svg")
+
+    # Copy EVERY rendered activity diagram, not just the ones a process page links:
+    # anything unlinked used to be dropped here without a word. The ones that resolve
+    # to no process/variant stay reachable through activity_diagrams.json (browse view).
+    all_ads = sorted(p.stem for p in bpmn_svg.glob("*.svg")) if bpmn_svg.exists() else []
+    for ad_key in all_ads:
+        shutil.copyfile(bpmn_svg / f"{ad_key}.svg", dest_bpmn / f"{ad_key}.svg")
+    unclaimed = [k for k in all_ads if k not in claimed_ads]
+    (data_dir / "activity_diagrams.json").write_text(
+        json.dumps(
+            [{"name": k, "svg": f"/diagrams/bpmn/{k}.svg", "linked": k in claimed_ads} for k in all_ads],
+            ensure_ascii=False,
+            indent=2,
+        ),
+        "utf-8",
+    )
+    print(
+        f"activity diagrams: {len(all_ads)} copied, {len(claimed_ads)} linked to a process, {len(unclaimed)} unlinked"
+    )
+    # One artifact wanted by two processes: p11's {pid}_{slug} is indistinguishable
+    # from a bare id that ends in that slug. First claim wins (processes are walked in
+    # sorted order, so it is at least deterministic), but say so rather than hide it.
+    if contested_ads:
+        print(f"ambiguous activity-diagram names: {len(contested_ads)}")
+        for contested in contested_ads:
+            print(f"  - {contested}")
 
     index.sort(key=lambda e: (e["category"], e["name"].lower()))
     (data_dir / "processes.json").write_text(json.dumps(index, ensure_ascii=False, indent=2), "utf-8")
@@ -431,6 +486,13 @@ def run(  # pylint: disable=too-many-locals,too-many-branches,too-many-statement
     print(f"approved: {approved_count}/{len(index)}{note}")
     # Worklist: distinct subprocess refs that resolved to no target. Curate these
     # in sd_ref_links.yaml (ambiguous / garbled scenario-bundle refs).
+    # Worklist: rendered activity diagrams that resolve to no process/variant —
+    # mostly p11 names predating the UC de-truncation (see the dataset's
+    # REGENERATION.md). They ship, but only the browse view reaches them.
+    if unclaimed:
+        print(f"unlinked activity diagrams: {len(unclaimed)}")
+        for ad_key in unclaimed:
+            print(f"  - {ad_key}")
     if unresolved_refs:
         print(f"unresolved refs: {len(unresolved_refs)} (add to sd_ref_links.yaml):")
         for ref in sorted(unresolved_refs):

unittests/test_build_webapp_data.py

@@ -4,6 +4,7 @@
 
 import yaml
 
+from makoralle.grouping import ad_artifact_key, sd_artifact_key
 from makoralle.webapp_export import (
     build_detail,
     build_index_entry,
@@ -745,3 +746,139 @@ def test_run_reports_orphaned_approval_entries(tmp_path: pathlib.Path, capsys: A
 
     out_text = capsys.readouterr().out
     assert "orphaned" in out_text and "ghost__variant" in out_text
+
+
+def test_ad_artifact_key_uses_a_single_underscore() -> None:
+    """p11 names activity diagrams `{pid}_{slug}` where the SDs use `{pid}__{slug}`.
+    Conflating the two strands every variant's diagram."""
+    assert ad_artifact_key("wechsel", "lieferant", 1) == "wechsel"
+    assert ad_artifact_key("wechsel", "lieferant", 3) == "wechsel_lieferant"
+    assert ad_artifact_key("wechsel", "lieferant", 3) != sd_artifact_key("wechsel", "lieferant", 3)
+
+
+def test_run_links_per_variant_activity_diagrams(tmp_path: pathlib.Path) -> None:
+    """A multi-SD process's activity diagrams live at {pid}_{slug}; each variant gets
+    its own, and a variant without one gets None rather than a 404 link."""
+    out, web = tmp_path / "output", tmp_path / "webapp"
+    pid = "wechsel"
+    _write(out / "yaml" / f"{pid}.yaml", yaml.safe_dump(TWO_SD, allow_unicode=True))
+    # only the first variant has an activity diagram rendered
+    _write(out / "bpmn" / f"{pid}_lieferant.svg", "<svg>ad-lieferant</svg>")
+
+    run(output_dir=out, webapp_dir=web)
+
+    detail = json.loads((web / f"src/data/processes/{pid}.json").read_text("utf-8"))
+    by_slug = {d["slug"]: d["activitySvg"] for d in detail["diagrams"]}
+    assert by_slug["lieferant"] == f"/diagrams/bpmn/{pid}_lieferant.svg"
+    assert by_slug["netzbetreiber"] is None
+    assert json.loads((web / "src/data/processes.json").read_text("utf-8"))[0]["hasBpmn"] is True
+    assert (web / f"public/diagrams/bpmn/{pid}_lieferant.svg").exists()
+
+
+def test_run_falls_back_to_the_bare_pid_activity_diagram(tmp_path: pathlib.Path) -> None:
+    """A multi-SD process whose activity diagram is named after the process only
+    (no variant suffix) still resolves — every variant points at it."""
+    out, web = tmp_path / "output", tmp_path / "webapp"
+    pid = "wechsel"
+    _write(out / "yaml" / f"{pid}.yaml", yaml.safe_dump(TWO_SD, allow_unicode=True))
+    _write(out / "bpmn" / f"{pid}.svg", "<svg>ad</svg>")
+
+    run(output_dir=out, webapp_dir=web)
+
+    detail = json.loads((web / f"src/data/processes/{pid}.json").read_text("utf-8"))
+    assert {d["activitySvg"] for d in detail["diagrams"]} == {f"/diagrams/bpmn/{pid}.svg"}
+
+
+def test_run_ships_and_indexes_activity_diagrams_that_link_to_no_process(tmp_path: pathlib.Path) -> None:
+    """Diagrams whose p11 name matches no process/variant (stale pre-de-truncation
+    names) must still be copied and indexed — they used to be dropped silently."""
+    out, web = tmp_path / "output", tmp_path / "webapp"
+    pid = "abstimmung_der_netzzeitreihe"
+    _write(out / "yaml" / f"{pid}.yaml", yaml.safe_dump(SAMPLE, allow_unicode=True))
+    _write(out / "bpmn" / f"{pid}.svg", "<svg>linked</svg>")
+    _write(out / "bpmn" / "zuordnung_eines_bilanzkreises_zur_aufnah-.svg", "<svg>orphan</svg>")
+
+    run(output_dir=out, webapp_dir=web)
+
+    ads = json.loads((web / "src/data/activity_diagrams.json").read_text("utf-8"))
+    by_name = {a["name"]: a for a in ads}
+    assert by_name[pid]["linked"] is True
+    orphan = by_name["zuordnung_eines_bilanzkreises_zur_aufnah-"]
+    assert orphan["linked"] is False
+    assert orphan["svg"] == "/diagrams/bpmn/zuordnung_eines_bilanzkreises_zur_aufnah-.svg"
+    # shipped despite linking to nothing, so the browse view can reach it
+    assert (web / "public/diagrams/bpmn/zuordnung_eines_bilanzkreises_zur_aufnah-.svg").exists()
+
+
+def test_run_rewrites_activity_diagram_dir_without_orphans(tmp_path: pathlib.Path) -> None:
+    """The dest dir is fully generated: a diagram removed upstream must not survive."""
+    out, web = tmp_path / "output", tmp_path / "webapp"
+    pid = "abstimmung_der_netzzeitreihe"
+    _write(out / "yaml" / f"{pid}.yaml", yaml.safe_dump(SAMPLE, allow_unicode=True))
+    _write(out / "bpmn" / f"{pid}.svg", "<svg>ad</svg>")
+    _write(web / "public/diagrams/bpmn/gone.svg", "<svg>stale</svg>")
+
+    run(output_dir=out, webapp_dir=web)
+
+    assert not (web / "public/diagrams/bpmn/gone.svg").exists()
+    assert (web / f"public/diagrams/bpmn/{pid}.svg").exists()
+
+
+NO_SD: dict[str, Any] = {
+    "process": {"id": "nur_ad", "name": "Nur AD", "category": "GPKE", "source": ""},
+    "use_case": {"roles": []},
+}
+
+
+def test_run_links_the_activity_diagram_of_a_process_without_any_sequence_diagram(
+    tmp_path: pathlib.Path,
+) -> None:
+    """p11 renders activity diagrams independently of p06's sequence diagrams, so a
+    process can have an AD and no SD. Resolving per variant must not drop it."""
+    out, web = tmp_path / "output", tmp_path / "webapp"
+    _write(out / "yaml" / "nur_ad.yaml", yaml.safe_dump(NO_SD, allow_unicode=True))
+    _write(out / "bpmn" / "nur_ad.svg", "<svg>ad</svg>")
+
+    run(output_dir=out, webapp_dir=web)
+
+    entry = json.loads((web / "src/data/processes.json").read_text("utf-8"))[0]
+    assert entry["sdCount"] == 0
+    assert entry["hasBpmn"] is True
+    ads = json.loads((web / "src/data/activity_diagrams.json").read_text("utf-8"))
+    assert [a["linked"] for a in ads if a["name"] == "nur_ad"] == [True]
+
+
+def test_run_reports_an_activity_diagram_two_processes_could_claim(tmp_path: pathlib.Path, capsys: Any) -> None:
+    """`{pid}_{slug}` is ambiguous with a bare id that happens to contain the slug:
+    `wechsel` + variant `lieferant` and a process named `wechsel_lieferant` both want
+    wechsel_lieferant.svg. Report it instead of silently handing it to whoever sorts first."""
+    out, web = tmp_path / "output", tmp_path / "webapp"
+    _write(out / "yaml" / "wechsel.yaml", yaml.safe_dump(TWO_SD, allow_unicode=True))
+    collider = {
+        "process": {"id": "wechsel_lieferant", "name": "Wechsel Lieferant", "category": "GPKE", "source": ""},
+        "use_case": {"roles": []},
+        "sequence_diagram": {"participants": [], "steps": []},
+    }
+    _write(out / "yaml" / "wechsel_lieferant.yaml", yaml.safe_dump(collider, allow_unicode=True))
+    _write(out / "bpmn" / "wechsel_lieferant.svg", "<svg>contested</svg>")
+
+    run(output_dir=out, webapp_dir=web)
+
+    printed = capsys.readouterr().out
+    assert "ambiguous activity-diagram names: 1" in printed
+    assert "wechsel_lieferant (claimed by" in printed
+
+
+def test_run_prints_the_unlinked_activity_diagrams_as_a_worklist(tmp_path: pathlib.Path, capsys: Any) -> None:
+    out, web = tmp_path / "output", tmp_path / "webapp"
+    pid = "abstimmung_der_netzzeitreihe"
+    _write(out / "yaml" / f"{pid}.yaml", yaml.safe_dump(SAMPLE, allow_unicode=True))
+    _write(out / "bpmn" / f"{pid}.svg", "<svg>linked</svg>")
+    _write(out / "bpmn" / "zuordnung_eines_bilanzkreises_zur_aufnah-.svg", "<svg>orphan</svg>")
+
+    run(output_dir=out, webapp_dir=web)
+
+    printed = capsys.readouterr().out
+    assert "173 copied" not in printed  # sanity: this fixture has 2, not the real dataset
+    assert "2 copied, 1 linked to a process, 1 unlinked" in printed
+    assert "  - zuordnung_eines_bilanzkreises_zur_aufnah-" in printed