Async LLM-tier "llm_fallback" verdicts (judge.py:1073, judge.py:1131
via _deliver_fallbacks) deliberately reuse the heuristic verdict's
``verdict_id`` so the row gets "upgraded in place" from heuristic →
llm_fallback when the LLM judge times out, is cancelled, or returns
no content. The consumer ``_persist_intent_verdict`` was doing a
plain INSERT via ``create_intent_verdict``, hitting the
``intent_verdicts_pkey`` constraint on every llm_fallback delivery.
Postgres logged the duplicate-key error; the application try/except
swallowed it at log.debug — so the row never actually got upgraded
and the LLM judge's annotation ("(LLM judge did not return a
verdict)") was lost.
The collision rate exploded on stable/1.5 smoke tests because
PR #527 (just merged) added two new heuristic-INSERT paths in the
auto-approve early-return branches of ``approve_tools`` — previously
those branches dropped heuristic verdicts on the floor, leaving no
row for the fallback to collide with.
Fix:
- New ``upsert_intent_verdict`` method on the storage protocol +
sqlite + postgres impls, using dialect-specific
``insert(...).on_conflict_do_update(index_elements=["verdict_id"],
set_={...})``. Set_ clause updates ONLY the three fields that
genuinely change between heuristic and llm_fallback: ``tier``,
``reasoning``, ``judge_model``.
- Every other column is excluded from set_: identity columns
(verdict_id, ws_id, call_id, func_name, func_args), carried-
verbatim columns (intent_summary, risk_level, confidence,
recommendation, evidence, latency_ms), and ``user_decision``.
- ``user_decision`` exclusion is load-bearing: ``IntentVerdict
.to_dict()`` doesn't project it, so a fallback verdict reaching
``_persist_intent_verdict`` carries the kwarg's ``"pending"``
default. If the operator already resolved the approval between
heuristic INSERT and fallback delivery, the row's user_decision
has been stamped to ``"approved"``/``"denied"``/``"timeout"`` (or
an auto-approve reason at heuristic-INSERT time per PR #527).
Including ``user_decision`` in set_ would silently clobber that
back to ``"pending"``.
- ``_persist_intent_verdict`` switched from ``create_*`` to
``upsert_*``. Bulk path ``create_intent_verdicts_bulk`` stays as
plain INSERT — every heuristic ``verdict_id`` is freshly minted
in ``judge.evaluate`` so in-turn dups can't happen. The inverse
race (daemon-judge verdict lands BEFORE the bulk write) IS
reachable today but its observable behavior is unchanged by the
per-row UPSERT switch; documented at the bulk site for a future
hardening pass.
Test coverage:
- TestIntentVerdictUpsert × 4 — fresh-id insert, conflict-upgrade,
user_decision preservation across heuristic→approved→fallback,
identity + carried-field preservation.
- Existing tests in test_session_ui_base.py updated to mock the
new upsert method instead of create_intent_verdict.
Patrick Buckley
13ed62d200