fix(dashboard): name the offender when the slots flush fails to serialize

[javenciu]

Problem / Motivation

A non-serializable value in slot state breaks every slots read path at once (GET /api/chat/slots, the WS snapshot, and the debounced broadcast all serialize the same projection), but the operator gets two unhelpful tracebacks (#8745, and #6522 before it):

1. The stock error names neither the slot nor the field. Measured on main (235d36a), with one slot's title poisoned:

TypeError: Object of type object is not JSON serializable

2. When the owed flush of suspend_slots_push raises while the body's own exception is already unwinding, the flush's exception replaces the body's in the caller's view — the actual fault is demoted to __context__, which is exactly how api_chat_slot_create 500s (bare StopIteration) when config.agents is empty — main CI red since #6465 #6522 was first misread as a broadcast bug:

Traceback (most recent call last):
  File ".../state.py", line 7584, in suspend_slots_push
    yield
  File "probe.py", line 48, in unwind_case
    raise RuntimeError("the body's actual fault")
RuntimeError: the body's actual fault

During handling of the above exception, another exception occurred:
...
TypeError: Object of type object is not JSON serializable

Why it matters

This is the diagnostic half that issue #8745 explicitly asks to land regardless of the wider decision ("Land the masking diagnostic regardless, since it changes no observable behaviour"). Without it, the next poisoned-slot incident starts the same way #6522 did: an operator staring at a bare TypeError at the top of a traceback whose real story is one exception down, with no pointer to which slot or field to look at.

What changed (motivation → approach → change)

• Symptom: the two tracebacks above — no offender named, and the top of the unwind traceback eats the lede.
• Root cause: the dump raises deep in _do_slots_broadcast with no context attached, and suspend_slots_push's finally-block flush is positioned to raise over an in-flight body exception, where Python's implicit chaining preserves the original only as __context__ without saying so.
• Change: two additive PEP 678 traceback notes (requires-python >= 3.12); no semantics change — same exception types, same propagation, same chaining:
  • _do_slots_broadcast hoists the general-frame json.dumps and, on TypeError/ValueError, annotates it via a new _slots_serialization_note() helper that names the offending slot key, field name, and value type. Values are withheld by design: slot state can carry user text, and the type is enough to find the producer. Both coalescing branches (leading edge and trailing timer) funnel through this method, so both report identically by construction. The helper is defensive end-to-end — any surprise in the offender walk degrades to a generic note rather than raising.
  • suspend_slots_push captures sys.exc_info() before the owed flush and, if the flush raises during unwind over a distinct in-flight exception, annotates the flush's exception naming the buried original and pointing at __context__.

Same probe, after:

TypeError: Object of type object is not JSON serializable
[slots-broadcast] slot 'chat-poison' field 'title' is not JSON-serializable: type object (value withheld)
[slots-flush] the owed slots flush raised while unwinding over an in-flight RuntimeError; that original exception is chained below as __context__

Deliberately not in scope: the failure-semantics decision the issue frames as options 1/2/3 (catch-at-flush vs fail-loud vs validate-at-write). That is a behavior choice the issue leaves open, and this diagnostic is useful under any of the three outcomes.

Tests

Five tests added to test/test_open_slots_persistence.py (file: 67 passed; slots push/broadcast neighbors: 191 passed). Before the fix, the four diagnostic tests fail (4 failed, 1 passed); after, 5 passed:

• test_leading_edge_serialization_failure_names_the_offender — the immediate broadcast annotates the raising TypeError with slot key, field, and type, and never the value.
• test_trailing_flush_serialization_failure_names_the_offender — the trailing-edge callback funnels through the same annotated dump (pins the both-branches claim).
• test_flush_failure_during_unwind_names_the_masked_exception — implicit chaining survives (__context__ is the body's RuntimeError), the note names the buried original, and the suspend depth still unwinds to zero.
• test_flush_failure_without_inflight_exception_has_no_unwind_note — a clean-exit flush failure gets the offender note only; no phantom unwind note.
• test_healthy_flush_is_unchanged_by_the_diagnostics — benign control: exactly one broadcast on the happy path, no new denials, no behavior change.

Manual verification

N/A — unit coverage sufficient: the seam is deterministic (poisoned projection in → annotated exception out) and the probe outputs above are the manual scenario, pasted from a fresh run on this branch.

Related Issues

Refs #8745 (the separable diagnostic half; the failure-semantics decision stays open there). Context: #6522.

Pattern harvest

Pattern: a cleanup step positioned to raise during exception unwind masks the body's exception (visible only as an unlabeled __context__), and serialization errors deep in a shared projection name no offender.
Rule candidate: review-prompt — "any finally/__exit__/context-manager cleanup that can itself raise should annotate its exception when unwinding over an in-flight one; any json.dumps on a composite projection should name the offending element on failure."

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable) — N/A for spec docs: no system-spec module documents this flush seam; the updated docstrings on suspend_slots_push and the new helper are the doc-of-record
• No secrets, credentials, or internal references in the diff

Contribution License Agreement

Per the template placeholder (CLA text pending): offered under the same terms as my prior merged contributions to this repository (#8835).

[github-actions]

Design Review (Fable 5, fork) — ✅ PASS

Design-level review of 3000bd54ecb36ad116f8a4be21f637f138b46758 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

The diff checks out against the base: the hoisted dump reuses the same json.dumps(slots_data) that line 7760 already paid for, so the happy path is unchanged; the requires-python = ">=3.12" claim holds for add_note; the unwind-note capture in suspend_slots_push correctly snapshots sys.exc_info() before the flush. The only coverage gap I found: _do_slots_broadcast also serializes the enriched WS projection (via _slots_ws_frame at state.py:7917) and the owner projection, neither of which passes through the annotated dump — but poison in shared slot state (the evidenced failure class) hits the annotated general dump first, so only an enrichment-only field would still produce a bare TypeError. That is suggestion-level, not a concern.

Design-Verdict: PASS

Additive PEP 678 diagnostics on an evidenced failure seam, with semantics deliberately untouched and the open failure-semantics decision correctly left to #8745.

Suggestions

• Poison confined to an enrichment-only field (WS dashboard_user or owner include_check_status projections, serialized in _slots_ws_frame) still raises a bare TypeError; routing those two dumps through _slots_serialization_note would close the last unannotated path on this seam.

[DESIGN-REVIEWED] 3000bd5

[github-actions]

GPT 5.6 Review (fork) — ✅ no blocking findings

Reviewed 3000bd54ecb36ad116f8a4be21f637f138b46758 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 3000bd5

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed 3000bd54ecb36ad116f8a4be21f637f138b46758 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.

The sole candidate concerns only the wording of a PEP-678 traceback note: when suspend_slots_push() is entered inside an active except block, sys.exc_info()[1] names the ambient handled exception, so the note could say "unwinding over an in-flight X" even though the context-manager body exited normally. But (c) fails — there is no observable wrong program outcome: exception type, propagation, and __context__ chaining are unchanged, and flush_exc.__context__ genuinely does point at that ambient exception, so the note's "chained below as context" claim stays accurate. (a) also fails — no caller reaching suspend_slots_push() from inside an active except block was located. A diagnostic string that is at worst slightly imprecise in an unconfirmed edge case is not a reachable defect and falls below the 80 bar. Nothing else in this diagnostic-only diff grounds a new finding.

[OPUS-REVIEWED] 3000bd5

[github-actions]

First Principles Review (Fable 5, fork) — 🟡 CONCERNS

Premise-level review of 3000bd54ecb36ad116f8a4be21f637f138b46758 via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All checks done. The change holds up under the gate; one depth finding and one shrink survive.

First-Principles-Verdict: CONCERNS

The offender note lands only on the broadcast path while the description itself counts three broken read paths; the other two still raise the bare TypeError.

What this change ships

Intent: when a non-serializable value poisons slot state, make the one traceback name the offending slot/field and stop the flush's exception from burying the body's real fault — a FIX (diagnostic half of a reported defect, no semantics change).

1. A slots-broadcast serialization failure now names slot, field, and value type — justified (reported defect, api_chat_slot_create 500s (bare StopIteration) when config.agents is empty — main CI red since #6465 #6522 misdiagnosis)
2. A flush raising mid-unwind now names the buried original and points at __context__ — justified (derived from the same misread)
3. New private helper _slots_serialization_note() walking the projection — one consumer, oversized (see Subtractions)
4. The broadcast's JSON dump moved earlier in the method — declared, justified (required to attach the note; happy path pinned unchanged)
5. Five tests pinning both notes and the benign path — justified

Watch

• Point patch with 2 counted unfixed siblings. The description claims a poisoned slot "breaks every slots read path at once," yet the note attaches only in _do_slots_broadcast. Grep serialize_slots( outside state.py: GET /api/chat/slots serializes via web.json_response(payloads) (chat_handlers.py:1108) and the WS connect snapshot via send_json (ws.py:665) — both still fail with the un-noted stock TypeError. The cause-level fix (validate-at-write) is explicitly left open in Failure semantics: a slots-broadcast exception 500s an already-committed slot create #8745, so this is accepted-and-deferred, but the two sibling read paths could take the same helper now.

Subtractions

• Shrink _slots_serialization_note: the non-list branch and the non-dict-entry branch guard shapes its one caller can never produce — serialize_slots (state.py:7511) always returns a list of dicts — and the outer except Exception already degrades those to the generic note. Delete both branches (~14 lines).

[FIRST-PRINCIPLES-REVIEWED] 3000bd5

[iamwhatever]

Tier 1 auto-approve: fix (2 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: diagnostics-only fix for the slots-broadcast serialization failure -- names the offending slot/field in a PEP-678 note and stops an owed flush's exception from masking the body's in-flight exception during unwind; same exception types, same propagation, no behaviour change. CodeQL is not applicable on this fork PR (default-setup emits no check-run); SAST coverage is Semgrep only, latest run success with 0 annotations.