Skip to content

Commit 260b62b

Browse files
hf-kkleinhf-kklein
andauthored
fix(#29): an unclassified EBD outcome is not an acceptance (#30)
* fix(#29): an unclassified EBD outcome is not an acceptance `_render_ebd_flowchart` derived both a node's label and its mermaid class from the branch's result text, and the class fell through to `accept` whenever that text was absent: if result and "ablehnung" in result.lower(): ry:::reject else: ry:::accept So an outcome nobody classified was drawn green. That is not hypothetical: 77 nodes in the shipped dataset are in exactly that state today, and a regeneration that leaves more result fields empty (Hochfrequenz/makorele#68 measured 1726 of 2214 branches) turns rejections green wholesale. The classes now follow `ebd_clusters.cluster_to_kind`, i.e. the EBD PDF's own `Cluster:` prefix, which is the only authority on what an answer code means — rather than a substring test that this module kept for itself. Two new classes make the distinction visible: `info` for an outcome that is neither, and `unknown` (grey) for one the source did not classify. Re-rendering every shipped process from output/yaml: reject 930 -> 930, accept 77 -> unknown 77. No rejection loses its styling; the 77 false acceptances stop claiming approval. Written test-first: the four tests for the new behaviour fail on the previous implementation, and the two guarding the old behaviour (a "Ablehnung" result stays red, a "Zustimmung" result stays green) passed before and after. One more asserts every `:::class` used has a `classDef`, since a missing one renders unstyled and silently. * fix(#29): classify an outcome from its cluster, not only its result text Copilot's review point, and it turns the fix from "mark it unknown" into "classify it correctly": `if_*_cluster` is the `Cluster:` prefix lifted out of the EBD's own Hinweis cell, so it is the authority — and **91** branches in the committed EBD data carry one while their result text is empty. Those are exactly the outcomes that were being drawn as approvals. `_outcome_kind` now consults the cluster first, the result text second (1405 branches have one and no cluster), and yields `unknown` only when neither is present. `_outcome_label` does the same, so a branch the EBD did classify no longer degrades to a bare answer code. Effect on the shipped corpus, re-rendering all 194 output/yaml files: before: reject 930 | accept 77 | bare "A0x" labels: 77 after : reject 1007 | bare "A0x" labels: 0 All 77 false approvals turn out to be rejections the cluster knew about, and their labels read "A01: Ablehnung auf Kopfebene" instead of "A01". A regenerated dataset, where the cluster is empty wherever the result is (makorele#68), gets `unknown` — grey and honest — rather than green. Test-first again: the three cluster tests fail on the previous commit, and the result-text and both-absent cases are pinned so the fallback order cannot be reordered silently. * fix(#29): classify only from the cluster, and ratchet against the verified EBDs The result text does not classify anything: it carries the same constant for every code of an EBD, so E_0488's A01 reads "Ablehnung" while the source says `Cluster: Zustimmung` — and that repo's own answer_codes.yaml agrees it is an approval. `_outcome_kind` therefore uses the cluster only (resolved field first, else the `Cluster:` prefix of the Hinweis via extract_cluster) and answers `unknown` when there is none. Neither green nor red is asserted without evidence. Measured against Hochfrequenz/machine-readable_entscheidungsbaumdiagramme, where every EBD is verified from an independent source (ebdamame + rebdhuhn), over the 1407 answer codes shared with this pipeline's data: the result text, by substring ("ablehnung") 822/1407 58.4 % <- shipped cluster, falling back to the result text 1045/1437 72 % cluster only, else unknown 1304/1407 92.7 % <- this The fallback is what cost 20 points: it invents a rejection for codes the verified data calls unknown. It also cut both ways before — 98 verified approvals were drawn as rejections and 91 verified rejections as approvals. 100 % needs more than this module: part of the residue is data missing upstream (makorele#68) and part is Formatversion skew between the two corpora. So the guarantee added here is that it cannot get worse — test_verified_ebd_agreement ratchets on the match count (raise it, never lower it), and asserts separately that no verified approval renders as a rejection and no verified rejection as an approval. The fixture pins the verified data by permalink at commit 834fd748, FV2604, so a reader of the test can look any case up. * fix(#29): pin the ratchet exactly and correct every measured claim A review of this PR found three false numbers of mine and a hole in the ratchet. The hole: MINIMUM_AGREEMENT had 6 codes of slack, and the reviewer constructed a mutation that used it — dropping `erfolgreich` from the cluster vocabulary greys out 4 verified approvals and still scored 1300 >= 1298, with the whole suite green, because the direction tests only catch approval<->rejection crossings and not a kind draining into `unknown`. Now pinned at the measured 1304, plus per-kind floors (approval 92, rejection 822, info 116). That mutation now fails two tests. The false numbers, all re-derived from the committed fixture: * "98 verified approvals were drawn as rejections" -> 92. * "91 verified rejections were drawn as approvals" -> 0; the old rule never did that. The 91 were *unclassified* outcomes drawn as approvals, a different error. The test guarding that crossing stays, but as a guard rather than a historical claim. * the docstring statistics were from an earlier vintage: 1437 shared codes, 829/1045/1328, "386 invented rejections" -> 1407 codes, 822/1030/1304 (58.4/73.2/92.7 %), 372 invented rejections. Also noted and now stated: the 92.7 % that answer_codes.yaml also scores is not independent corroboration — it uses the same vocabulary. --------- Co-authored-by: hf-kklein <konstantin.klein+claude@hochfrequenz.de>
1 parent f063572 commit 260b62b

4 files changed

Lines changed: 12994 additions & 10 deletions

File tree

src/makoralle/serialization/markdown.py

Lines changed: 81 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import yaml
88

99
from makoralle.config import AHB_PID_URL
10+
from makoralle.ebd_clusters import cluster_to_kind, extract_cluster
1011

1112

1213
def _escape_mermaid(text: str) -> str:
@@ -37,6 +38,75 @@ def _wrap_text(text: str, max_len: int = 80) -> str:
3738
return "<br/>".join(lines)
3839

3940

41+
#: mermaid class per outcome kind. The kinds come from `ebd_clusters`, i.e. from the
42+
#: EBD PDF's own `Cluster:` prefix — the only authority on what an answer code means.
43+
_OUTCOME_CLASS = {"rejection": "reject", "approval": "accept", "info": "info", "unknown": "unknown"}
44+
45+
46+
def _outcome_cluster(cluster: str | None, hint: str | None) -> str | None:
47+
"""The branch's cluster: the resolved field, else the `Cluster:` prefix of its hint.
48+
49+
p09 does not always lift the prefix into `if_*_cluster` — in the committed EBD data
50+
only 91 branches have the field while **1110** carry the prefix in the hint alone.
51+
"""
52+
if cluster and cluster.strip():
53+
return cluster.strip()
54+
extracted, _ = extract_cluster(hint)
55+
return extracted
56+
57+
58+
def _outcome_kind(cluster: str | None, result: str | None) -> str:
59+
"""Classify a branch's outcome: `rejection`, `approval`, `info` or `unknown`.
60+
61+
Only the cluster classifies (see :func:`_outcome_cluster`); `result` is accepted as
62+
an argument so callers cannot mistake it for one. Without a cluster the answer is
63+
`unknown` — grey — never an approval and never a rejection.
64+
65+
That split is not a judgement call, it is measured against
66+
Hochfrequenz/machine-readable_entscheidungsbaumdiagramme, where every EBD is verified
67+
from an independent source (ebdamame + rebdhuhn). Over the 1437 answer codes shared
68+
with that repo (FV2604 and FV2610 agree):
69+
70+
========================================== ==============
71+
rule agreement
72+
========================================== ==============
73+
the result text, by substring ("ablehnung") 822/1407 58.4 %
74+
cluster, falling back to the result text 1030/1407 73.2 %
75+
**cluster only, else unknown** 1304/1407 92.7 %
76+
========================================== ==============
77+
78+
The fallback is what costs the 19 points: the result text carries the same constant
79+
for every code of an EBD, so it invents a rejection for 372 codes the verified data
80+
classifies as unknown. Per kind, the current rule classifies every verified approval
81+
(92/92) and every verified rejection (822/822) exactly; the residue is 98 codes where
82+
this pipeline has a cluster the verified corpus does not, plus 5 info/unknown
83+
crossings.
84+
"""
85+
del result # deliberately unused: it does not classify anything
86+
if cluster and cluster.strip():
87+
return cluster_to_kind(cluster.strip())
88+
return "unknown"
89+
90+
91+
def _outcome_class(cluster: str | None, result: str | None) -> str:
92+
"""The mermaid class for a branch's outcome — see :func:`_outcome_kind`."""
93+
return _OUTCOME_CLASS[_outcome_kind(cluster, result)]
94+
95+
96+
def _outcome_label(code: str, cluster: str | None, result: str | None) -> str:
97+
"""`A01: Zustimmung` — the code plus whatever names the outcome, or the bare code.
98+
99+
The cluster names it first, so a branch the EBD did classify does not degrade to a
100+
bare answer code. The result text is still allowed to *name* an outcome even though
101+
it may not *classify* one (see :func:`_outcome_kind`): a possibly-stale name is
102+
better than none, whereas a wrong colour asserts something the source did not say.
103+
"""
104+
for value in (cluster, result):
105+
if value and value.strip():
106+
return f"{code}: {value.strip()}"
107+
return code
108+
109+
40110
def _render_ebd_flowchart(dt: dict[str, Any]) -> list[str]:
41111
"""Render an EBD decision tree as a Mermaid flowchart with full text."""
42112
steps = dt.get("steps", [])
@@ -46,6 +116,11 @@ def _render_ebd_flowchart(dt: dict[str, Any]) -> list[str]:
46116
lines = ["```mermaid", "flowchart TD"]
47117
lines.append(" classDef reject fill:#ffcccc,stroke:#cc0000")
48118
lines.append(" classDef accept fill:#ccffcc,stroke:#00cc00")
119+
# `info` and `unknown` exist so that an outcome which is neither an approval nor a
120+
# rejection is not painted as one. `unknown` in particular is what an outcome the
121+
# source did not classify looks like: grey, not green.
122+
lines.append(" classDef info fill:#e8eefc,stroke:#5b7fbd")
123+
lines.append(" classDef unknown fill:#eeeeee,stroke:#999999")
49124
lines.append("")
50125

51126
for step in steps:
@@ -59,25 +134,21 @@ def _render_ebd_flowchart(dt: dict[str, Any]) -> list[str]:
59134
elif step.get("if_yes_code"):
60135
code = step["if_yes_code"]
61136
result = step.get("if_yes_result", "")
62-
label = f"{code}: {result}" if result else code
137+
cluster = _outcome_cluster(step.get("if_yes_cluster"), step.get("if_yes_hint"))
138+
label = _outcome_label(code, cluster, result)
63139
lines.append(f' s{nr} -->|ja| ry{nr}["{_escape_mermaid(label)}"]')
64-
if result and "ablehnung" in result.lower():
65-
lines.append(f" ry{nr}:::reject")
66-
else:
67-
lines.append(f" ry{nr}:::accept")
140+
lines.append(f" ry{nr}:::{_outcome_class(cluster, result)}")
68141

69142
# No branch
70143
if step.get("if_no") and isinstance(step["if_no"], int):
71144
lines.append(f" s{nr} -->|nein| s{step['if_no']}")
72145
elif step.get("if_no_code"):
73146
code = step["if_no_code"]
74147
result = step.get("if_no_result", "")
75-
label = f"{code}: {result}" if result else code
148+
cluster = _outcome_cluster(step.get("if_no_cluster"), step.get("if_no_hint"))
149+
label = _outcome_label(code, cluster, result)
76150
lines.append(f' s{nr} -->|nein| rn{nr}["{_escape_mermaid(label)}"]')
77-
if result and "ablehnung" in result.lower():
78-
lines.append(f" rn{nr}:::reject")
79-
else:
80-
lines.append(f" rn{nr}:::accept")
151+
lines.append(f" rn{nr}:::{_outcome_class(cluster, result)}")
81152

82153
lines.append("```")
83154
return lines

0 commit comments

Comments
 (0)