feat(verdicts): a cross-base candidate collision is reported, never dropped in silence

[skip-docs] — the invariant row for this plan lands in Step 13, after the mutations.

Co-Authored-By: Claude <claude-opus-5>

src/portfolio_optimiser/run.py

@@ -109,6 +109,7 @@ from portfolio_optimiser.semretrieval import (
     load_embedder_config,
 )
 from portfolio_optimiser.verdicts import (
+    VerdictCollision,
     ExpeLContextProvider,
     ProposalFeatures,
     Verdict,
@@ -534,6 +535,32 @@ def cost_baseline_notice(anchored: bool) -> str | None:
     return None if anchored else _UNANCHORED_NOTICE
 
 
+def collision_notice(collisions: tuple[VerdictCollision, ...]) -> str | None:
+    """Render which candidates two bases both described, or ``None`` when none did.
+
+    ``None`` on an empty tuple — omission, never an empty row (``mandate.announce``'s rule, as
+    ``skipped_links_notice`` and ``cost_baseline_notice`` already follow). A dispatch where every
+    candidate belonged to exactly one base has nothing to report.
+
+    ONE renderer, taking the ALREADY-RESOLVED trace rather than a store to re-scan: a renderer that
+    recomputed the collisions would be a second resolution of the same fact, free to disagree with
+    the dispatch it describes (kø-(p)).
+
+    **Surface, stated plainly:** ``MultiBaseResult`` has no production caller today, so this notice
+    is library-facing. It is written now because the field would otherwise be a value nothing can
+    display — the same principle Step 4 applies one level down: a signal that was not stored must
+    not become an asserted absent one.
+    """
+    if not collisions:
+        return None
+    lines = ["Cross-base candidates (one verdict each; the later base's was dropped):"]
+    lines.extend(
+        f"  - {c.verdict_id}: first from {c.first_bundle_id!r}, again from {c.second_bundle_id!r}"
+        for c in collisions
+    )
+    return "\n".join(lines)
+
+
 def skipped_links_notice(skipped: tuple[okf.SkippedLink, ...]) -> str | None:
     """Render what the run could NOT read, or ``None`` when every cross-link was followed.
 
@@ -1455,6 +1482,12 @@ class MultiBaseResult:
     stopped_early: bool = False
     budget_stop: BudgetStop | None = None
     unreached: tuple[ApproachOutcome, ...] = ()
+    #: Candidates two bases both described (D2). DEFAULTS to an empty tuple, which is the
+    #: ``skipped_links`` half of the required-vs-default rule and not the
+    #: ``cost_baseline_anchored`` half: an empty trace is an honest POSITIVE statement ("no
+    #: candidate was described by two bases"), whereas a missing bool would have to assert
+    #: something about an event and both assertions would sometimes be untrue.
+    collisions: tuple[VerdictCollision, ...] = ()
 
 
 async def run_mandate_across_bundles(
@@ -1538,6 +1571,10 @@ async def run_mandate_across_bundles(
     store = store if store is not None else VerdictStore(verdicts=[])
     runs: list[BundleRun] = []
     unreached: list[ApproachOutcome] = []
+    collisions: list[VerdictCollision] = []
+    #: Which base FIRST produced each verdict id — the map the store deliberately does not keep,
+    #: and the reason this accounting lives in the dispatcher rather than in ``VerdictStore.add``.
+    first_base_by_verdict: dict[str, str] = {}
     budget_stop: BudgetStop | None = None
 
     for index, (bundle_id, sub_mandate) in enumerate(routed):
@@ -1565,6 +1602,13 @@ async def run_mandate_across_bundles(
         # A second lookup for the label would be the kø-(p) duplicate free to drift from the value
         # the run was actually dispatched with.
         project_id = str(okf.load_ir_projection(bundle_dir)["project_id"])
+        # D2: the id of the verdict THIS base minted, taken from ``run_project``'s existing
+        # ``notify`` seam rather than off the returned ``RunResult``. ``notify`` fires inside the
+        # capture block, so it is called exactly when a verdict exists (F2: never when nobody
+        # reviewed the run) and it carries the minted object, id included. The slot is built FRESH
+        # each iteration — a shared accumulator would let a later base read the previous base's
+        # verdict and manufacture a collision that never happened.
+        minted_here: list[str] = []
         result = cast(
             RunResult,
             await run_project(
@@ -1572,6 +1616,7 @@ async def run_mandate_across_bundles(
                 profile,
                 docs_dir=bundle_dir,
                 bundle_dir=bundle_dir,
+                notify=lambda verdict: minted_here.append(verdict.id),
                 verdict_input=verdict_input,
                 verdict_dir=verdict_dir,
                 dimension=dimension,
@@ -1584,6 +1629,29 @@ async def run_mandate_across_bundles(
                 meter=_run_meter(None, portfolio_meter, max_rounds),
             ),
         )
+        # D2, dispatcher-side accounting. The condition is the dispatcher's OWN map and nothing
+        # else: a base that already claimed this id is the only thing that makes "a SECOND base"
+        # a statement worth making. A Step-7 inbox verdict or a bundle seed sharing the id is NOT
+        # a cross-base collision, and is excluded exactly by never appearing in this map.
+        #
+        # The plan asked for a before/after snapshot of ``{v.id for v in store.verdicts}`` beside
+        # this. MEASURED redundant and therefore left out: ``store.add`` is first-write-wins and
+        # never removes, so any id in this map is necessarily in the store when a later base runs.
+        # The snapshot could not change the outcome of a single dispatch — a conjunct no mutation
+        # can redden is dead code wearing a guard's clothes, which is the class this repo writes
+        # rows against rather than ships.
+        for minted_id in minted_here:
+            first = first_base_by_verdict.get(minted_id)
+            if first is None:
+                first_base_by_verdict[minted_id] = bundle_id
+            else:
+                collisions.append(
+                    VerdictCollision(
+                        verdict_id=minted_id,
+                        first_bundle_id=first,
+                        second_bundle_id=bundle_id,
+                    )
+                )
         runs.append(
             BundleRun(
                 bundle_id=bundle_id,
@@ -1596,6 +1664,7 @@ async def run_mandate_across_bundles(
     return MultiBaseResult(
         runs=tuple(runs),
         store=store,
+        collisions=tuple(collisions),
         stopped_early=budget_stop is not None,
         budget_stop=budget_stop,
         unreached=tuple(unreached),

src/portfolio_optimiser/verdicts.py

@@ -110,6 +110,27 @@ def verdict_key(features: ProposalFeatures) -> str:
     return _mint_id(features)
 
 
+@dataclass(frozen=True)
+class VerdictCollision:
+    """Two knowledge bases described ONE candidate, and the second verdict was dropped.
+
+    ``VerdictStore.add`` is first-write-wins per id and ``_mint_id`` excludes the corpus BY
+    CONSTRUCTION, so this drop is CORRECT — one candidate, one learning key — but it used to be
+    silent, and a base whose finding never entered the store looked exactly like a base that found
+    nothing (operator decision D2).
+
+    A DIAGNOSTIC, never an aggregate over bases: it reports that two bases described one candidate
+    and never merges, ranks or sums them, which is why it needs no combination rule. ``add`` itself
+    is unchanged — it is called from inside ``run_project``/``run_portfolio``, where a drop can
+    equally be against a Step-7 inbox verdict or a bundle seed, so only the dispatcher holds the
+    id -> base map that makes "a SECOND base" a statement worth making.
+    """
+
+    verdict_id: str
+    first_bundle_id: str
+    second_bundle_id: str
+
+
 def capture_verdict(features: ProposalFeatures, decision: str, rationale: str) -> Verdict:
     """Layer-2 out-of-band verdict constructor: mint a stable content-hash id (the
     learning-loop key) and build the ``Verdict`` to persist in the store."""

tests/test_bundle_id_reconciliation_loadbearing.py

@@ -54,6 +54,7 @@ import pytest
 from portfolio_optimiser import explore, okf, run
 from portfolio_optimiser.mandate import Approach, Mandate
 from portfolio_optimiser.simulation import ScriptedChatClient
+from portfolio_optimiser.verdicts import VerdictCollision
 
 _EXAMPLES = Path(__file__).resolve().parents[1] / "shared" / "examples"
 _BYGG = _EXAMPLES / "bygg-energi-mikro"
@@ -273,3 +274,126 @@ async def test_the_dispatcher_refuses_a_disagreeing_base_before_it_starts_any_ru
             (str(base),),
         )
     assert started == [], "the dispatcher started a run before reconciling the base it routed to"
+
+
+# ==================================================================================================
+# Step 11 - a cross-base candidate collision stops being silent
+# ==================================================================================================
+#
+# **The defect (operator decision D2).** ``VerdictStore.add`` is first-write-wins per id and
+# ``_mint_id`` excludes the corpus BY CONSTRUCTION (it hashes ``affected_codes`` / ``measure_type``
+# / ``claimed_saving_nok``, and nothing else). Two independently produced bases that describe the
+# same candidate therefore collapse onto ONE verdict and the second is dropped in silence. No test
+# covered it.
+#
+# **``_mint_id`` is NOT changed** -- its form is normative in ``shared/method-spec.md`` and
+# commons-owned, so altering it is an amendment, not a local edit. **``add`` is not changed either:**
+# it is called from inside ``run_project`` and ``run_portfolio``, never from
+# ``run_mandate_across_bundles``, so a return value there would reach no code in this step's scope,
+# and a drop there can equally be against a Step-7 inbox verdict or a bundle seed rather than a
+# second base. The mechanism is dispatcher-side accounting, because the dispatcher is the one place
+# that holds the id -> base map the store does not keep.
+
+
+_COLLIDING_PROPOSAL = (
+    '{"measure":"LED-retrofit av kontorbelysning","affected_items":'
+    '[{"code":"ENERGI-TOTAL-EL","quantity":300000,"unit_cost":1.0}],"claimed_saving_nok":30000}'
+)
+_VERDICT_INPUT = {"decision": "approved", "rationale": "expert reviewed (fixture)"}
+
+
+def _colliding_factory() -> Any:
+    """Both bases answer with the SAME proposal, which is what makes the two ids collide -- the
+    corrected fixture note: ``_mint_id`` hashes the PROPOSAL's features, not the base's IR
+    projection, so matching projections alone would mint nothing and the arm would pass for the
+    wrong reason.
+
+    The role names are ``workflow._MAKER_CHECKER_ROLES`` verbatim (``proposer`` / ``checker``); a
+    typo there would feed the proposal JSON to the checker and the arm would still pass, since F2
+    mints a verdict on either outcome -- green for a different reason than the docstring claims.
+    The outcome itself is deliberately NOT asserted: this arm is about candidate IDENTITY, and
+    ``_mint_id`` reads the same three structural fields whether the gate validated or rejected."""
+    return lambda role: ScriptedChatClient(
+        role=role,
+        default_reply=(_COLLIDING_PROPOSAL if role != "checker" else "Holder. VERDICT: APPROVE"),
+    )
+
+
+def _two_bases(tmp_path: Path) -> tuple[Path, Path]:
+    return _base_copy(tmp_path, name="base-one"), _base_copy(tmp_path, name="base-two")
+
+
+def _mandate_over(*bundle_ids: str) -> Any:
+    return Mandate(
+        objective="find the same saving twice",
+        approaches=tuple(
+            Approach(id=f"a{i}", label=f"A{i}", bundle_id=bid) for i, bid in enumerate(bundle_ids)
+        ),
+        allow_own_proposals=False,
+    )
+
+
+@pytest.mark.asyncio
+async def test_two_bases_describing_one_candidate_are_reported_not_dropped(tmp_path: Path) -> None:
+    """(k) The measured silence, closed. Two bases, one candidate: the collision names BOTH bases
+    and the shared id, in dispatch order."""
+    one, two = _two_bases(tmp_path)
+    result = await run.run_mandate_across_bundles(
+        _mandate_over(one.name, two.name),
+        (str(one), str(two)),
+        verdict_input=_VERDICT_INPUT,
+        client_factory=_colliding_factory(),
+    )
+
+    assert len(result.collisions) == 1, f"expected one collision, got {result.collisions}"
+    collision = result.collisions[0]
+    assert collision.first_bundle_id == one.name
+    assert collision.second_bundle_id == two.name
+    assert collision.verdict_id == result.runs[0].result.verdict_key
+
+
+@pytest.mark.asyncio
+async def test_a_single_base_reports_no_collision(tmp_path: Path) -> None:
+    """(l) CONTROL. An empty trace is an honest POSITIVE statement ("no candidate was described by
+    two bases") -- the ``skipped_links`` half of the required-vs-default rule, not the
+    ``cost_baseline_anchored`` half. Without this arm, an implementation that always appended would
+    pass (k)."""
+    one = _base_copy(tmp_path, name="base-one")
+    result = await run.run_mandate_across_bundles(
+        _mandate_over(one.name),
+        (str(one),),
+        verdict_input=_VERDICT_INPUT,
+        client_factory=_colliding_factory(),
+    )
+    assert result.collisions == ()
+
+
+@pytest.mark.asyncio
+async def test_the_drop_itself_is_unchanged_only_its_visibility(tmp_path: Path) -> None:
+    """(m) This step changes VISIBILITY, never semantics. The store still holds exactly one verdict
+    for the shared id: ``add`` is untouched and first-write-wins still wins."""
+    one, two = _two_bases(tmp_path)
+    result = await run.run_mandate_across_bundles(
+        _mandate_over(one.name, two.name),
+        (str(one), str(two)),
+        verdict_input=_VERDICT_INPUT,
+        client_factory=_colliding_factory(),
+    )
+    shared = result.collisions[0].verdict_id
+    assert [v.id for v in result.store.verdicts].count(shared) == 1
+
+
+def test_the_collision_notice_is_omitted_when_nothing_collided() -> None:
+    """(n) Omission, never an empty row (``mandate.announce``'s rule, as ``skipped_links_notice``
+    and ``cost_baseline_notice`` already follow). Both directions are asserted: a renderer that
+    always returns a line is as wrong as one that never does."""
+    assert run.collision_notice(()) is None
+    line = run.collision_notice(
+        (
+            VerdictCollision(
+                verdict_id="abc123", first_bundle_id="base-one", second_bundle_id="base-two"
+            ),
+        )
+    )
+    assert line is not None
+    assert "abc123" in line and "base-one" in line and "base-two" in line