fix: guarantee evidence_json reads as a dict at the column boundary (#250 follow-up)

[branover]

What

Follow-up to #250. That PR fixed the GET /api/projects/{id} 500 by routing the highest-exposure findings reads through coerce_evidence(). But ~15 other sites read Finding.evidence_json directly, and a legacy / hand-edited / double-encoded row — one whose evidence deserializes to a JSON string — would still crash them:

• (f.evidence_json or {}).get(...) → AttributeError: 'str' object has no attribute 'get'
• dict(f.evidence_json or {}) → ValueError (dict() of a string)

Affected paths include POST /api/findings/{id}/verify (findings.py:114), report build, dedup, the suggester, and the fuzz / poc / reachability task paths.

The fix — coerce once at the column-read boundary

Add a JSONDict SQLAlchemy TypeDecorator (impl = JSON) in the new db/jsontypes.py whose process_result_value coerces a non-dict stored value to a dict on read (recovering a double-encoded dict, degrading garbage to {}), and apply it to Finding.evidence_json. No reader can then ever observe a non-dict, so all ~15 sites are safe by construction — without touching each call site.

• None and well-formed dicts pass through unchanged; only the malformed values that would otherwise crash are normalized. Writes are untouched (upstream Pydantic validation remains the write guard).
• coerce_evidence is relocated into db/jsontypes.py (the low-level home shared by the column type and the pure helpers) and re-exported from engine/findings/findings.py, so existing imports keep working. is_verified still routes through it for inputs not sourced from the column (tests / in-memory / imports).

Scope & alternatives

Scoped to evidence_json — the column with the actual reported bug. JSONDict is generic and the same treatment could be applied to other dict-typed JSON columns (metadata_json, attrs_json, params_json, …) in a later, separately-verified change; deliberately left out here to keep this PR tight, reviewable, and per-column tested. The alternative of patching each of the ~15 call sites was rejected — the boundary fix is the lower-altitude, can't-regress option.

No migration

The DDL is plain JSON (the decorator's impl), so alembic revision --autogenerate detects no drift (verified — empty upgrade()). No schema change → no migration.

Verification

• New tests/test_evidence_json_column.py: the column guarantee (string / double-encoded / non-object-JSON / dict round-trip through a fresh session) plus representative previously-unguarded sites — verify endpoint returns 400 not 500, build_report_md renders, dedupe_findings collapses. All 4 fail on plain JSON and pass with JSONDict (revert-checked).
• Existing tests/test_evidence_coerce.py (from fix: tolerate a non-dict evidence_json on findings read (no more 500) #250) still passes via the re-export.
• Full just test fast tier green: 1411 passed, 4 skipped, 14 deselected. The target→task→finding→graph loop is untouched (read coercion only), so just demo was not required.

🤖 Generated with Claude Code

[branover]

Independent merge-gate review — PR #251 (fix/evidence-json-column)

Verdict: APPROVE (no blocking issues). Reviewed by an independent agent that did not author the change. Ran both /code-review (correctness) and /security-review on git diff main...HEAD.

This is a clean, well-targeted follow-up to #250. It replaces the per-call-site coercion bandaid with the right deep fix: coerce once at the column-read boundary via a JSONDict TypeDecorator(impl=JSON) on Finding.evidence_json. Every reader is now safe by construction.

Correctness — verified

• TypeDecorator hook is correct. process_result_value is the right hook and runs after the impl JSON's json.loads, so it receives the deserialized Python value (dict / str / scalar / None), not raw text. Verified end-to-end by driving the full result-processor chain: a normal dict passes through, a double-encoded dict (json.dumps(json.dumps({...}))) is recovered, garbage degrades to {}, and SQL NULL stays None.
• No migration needed — confirmed. impl = JSON keeps the DDL byte-identical (JSONDict().compile(sqlite) == JSON().compile(sqlite) == "JSON"). Built the DB via prepare_database() then ran alembic revision --autogenerate: the generated upgrade() body is empty (pass). Throwaway migration deleted. The PR correctly ships no migration.
• cache_ok = True is correct. The decorator carries no instance-level parameters that affect SQL caching, so it is safe (and avoids the SQLAlchemy cache-disable warning).
• None handling is right, not a bug. The column is NOT NULL in practice (default=dict), so the explicit if value is None: return None is belt-and-suspenders. Returning None rather than {} is the correct choice because all ~15 read sites already guard value or {}, and preserving NULL-vs-{} avoids changing the column's value space.
• Write path is unaffected. process_bind_param is intentionally not overridden, so writes delegate to JSON's bind processor (json.dumps) exactly as before — byte-identical persistence. The write guard remains upstream Pydantic validation. All read-modify-write sites (dict(f.evidence_json or {}) → mutate → reassign in campaigns.py, fuzzing.py, findings.py, source.py, reachability.py) keep working and now read a guaranteed dict.
• Full coverage of the direct-read sites. Spot-checked the claimed sites — api/routers/findings.py:114, engine/findings/report.py:42, engine/graph/dedup.py:18, engine/fuzz/campaigns.py, engine/findings/poc.py:376/412, demo.py:269, suggester.py, runs.py, solving.py, harness_promote.py, reachability.py, llm_tasks.py — every one reaches evidence_json through the ORM column attribute, so the column boundary covers them all. Grep for raw SQL / json_extract / text(...) on evidence_json: none. No read path bypasses the decorator.
• No circular import. db/models.py imports db/jsontypes.py, which imports only sqlalchemy + stdlib json (not db.models). Import-checked clean; the fast tier (1411) and the targeted suite pass.
• Re-export preserved. from hexgraph.engine.findings.findings import coerce_evidence still resolves (re-exported from db.jsontypes); existing callers (mcp_tools.py, tests/test_evidence_coerce.py) and row_to_payload / is_verified are unbroken — confirmed by the passing test_evidence_coerce.py.
• Nothing depends on the raw stored value. No code distinguishes a stored JSON-string from a dict for evidence_json; the only None-vs-{} consumers are the value or {} guards, which the decorator respects.

Test quality — strong

tests/test_evidence_json_column.py genuinely reproduces the original crash: assigning a Python str to the JSON column double-encodes it, so a fresh read returns a string — the exact legacy shape. The column test asserts recovery of a double-encoded dict (data intact), {} for unparseable / non-object JSON, and pass-through for a healthy dict. Three integration tests (build_report_md, dedupe_findings, POST /verify) each exercise a real previously-unguarded .get(...) / dict(...) site and assert it no longer 500s (the verify test reaches the normal 400, not a 500). By construction these would fail on plain JSON (the column would return a str and the .get/dict() would raise), which matches the author's revert-check. 21 targeted tests pass here; reported full fast tier 1411 green.

Security — no findings

Pure DB read-coercion change. The only parsing is json.loads (no pickle / yaml / eval), and its result is type-checked (isinstance(parsed, dict) else {}) with ValueError/TypeError caught — no deserialization-RCE, type-confusion, or injection path. No new endpoint, subprocess, file/path handling, network bind, or auth/authz logic. The HexGraph security invariants are untouched: loopback-only bind, target bytes stay in the sandbox, opt-in execution/egress only at the policy seam, and secrets never logged/stored/returned (evidence_json is finding metadata, not secret material, and the new code never logs it). The change only makes a previously-crashing read robust; it does not widen data exposure (same row, same project scope).

Non-blocking nits (do not block merge)

1. Redundant or {} guards now scattered across ~15 read sites (e.g. is_verified(f.evidence_json or {}) at agent/mcp_tools.py:1099). Now that the column guarantees dict-or-None, these are defensively redundant. Leaving them is a reasonable choice (avoids a large churn diff and keeps the helpers safe for non-column callers), but a future cleanup could drop them once everyone trusts the boundary. No action needed in this PR.
2. Docs/CHANGELOG: no UI behavior changed, so docs/dev/ux-contract.md is correctly untouched; no migration is correctly absent. Nothing required by the gate is missing.

Ready to merge once CI is green.