Fix the Review Lens plugin's wire contract, and 20 defects behind it

[dovvnloading]

An audit of the Review Lens (code review) plugin across its engine, fetch, dispatch, domain, persistence and React layers.

Problem

The headline defect made the plugin unusable end to end. scene_payload forwarded the code_review node's nested rows with dict(row), so they reached the wire carrying the review engine's snake_case keys — group_title, patch_truncated, previous_path — where CodeReviewWalkthroughGroupRow and CodeReviewFileRow declare groupTitle, patchTruncated and previousPath. The generated client validator treats a missing required field as fatal and bindTopic.ts drops a snapshot that fails validation, so from the first successful PR fetch onward every scene snapshot for the session was rejected client-side: the whole canvas froze, not just this node.

Nothing caught it because the one wire test passed empty nested lists, and the frontend test built its fixtures by hand in camelCase — so the drift was invisible from both sides.

The rest were found in the same pass:

Area	Defect
Engine	Release risk ignored high/critical review findings, so a critical defect filed as a finding rendered "Needs Revision, 95/100, low risk" above its own red card
Engine	looks_like_a_review accepted a list whose entries normalization then discarded, resurrecting the fabricated 72/100 grade it exists to stop
Engine	An infinite category score or line number raised OverflowError — not a ValueError — out of the one path that must always degrade to a fallback
Engine	The model diff cap (45,000) was smaller than the fetch cap (60,000), so a 50KB diff was cut a second time while every user-visible surface said the review saw all of it
Engine	A capped walkthrough group reported the cap as its file count — 40 changed files read as "12 file(s)"
Security	The diff was appended to the prompt under a plain heading with no fence, and attacker-controlled file paths — newlines intact — were interpolated outside it
Security	toDiffFence used a fixed 3-backtick fence, which any diff line containing ``` closes, rendering the rest of a third-party PR as live markdown
Security	parse_pr_url returned .. as an owner or repo, retargeting the api.github.com URL built by concatenation downstream
Security	The diff download followed redirects with the token attached, and buffered the whole body before applying any cap
Dispatch	Cancel was a dead control during a fetch or an Ask; cancelling a run freed the registry slot but left the node busy for up to 600s
Dispatch	A timed-out run left no error banner at all; deleting the node mid-run produced a spurious "unknown node" toast
UI	The fallback pre-screen shipped an invented eight-category 82/100 scorecard, and the node rendered "No review yet — run a review first." directly above that pre-screen's own findings
UI	The scorecard rendered raw wire keys as labels; the Retry button was the only one outside the styled row; qa-entry had no CSS rule; the Ask draft was destroyed on every rejection path; tabs had no panel association or keyboard navigation
Persistence	Restore was the only write path into these fields with no caps
Tooling	review_engine.py sat inside the mypy gate with all 30 of its function bodies unchecked, because none of them are annotated

Change

Nested rows are built explicitly at the wire builder, in camelCase, following the toolCalls precedent — which also drops the per-file patch bodies (up to ~600KB per node on every republish, read by nothing). Everything else is fixed at the layer that owns it, and every fix carries a comment naming the behavior it replaces.

The engine's fallback scorecard now reports only categories a heuristic actually lowered, and the node distinguishes "not reviewed" from "reviewed but not graded". Six false-positive and six false-negative fallback heuristics were corrected (JS regex.exec, commented-out code, pprint; multi-line except Exception: pass, check_output(shell=True), os.popen). check_untyped_defs is enabled for the plugin's modules, which found a real narrowing bug immediately.

Test plan

• 3,189 backend tests pass. The 54 anyio DeprecationWarning collection errors are pre-existing on clean main and unrelated — verified by stashing.
• 2,204 frontend tests across 93 files pass.
• mypy, ruff, eslint and the codegen drift check are clean; the bundle-size ceiling still has headroom.
• New coverage: a wire-boundary test validating a fully populated review row against SceneNodeRow itself (fails on the old code); adversarial prompt-fence and path-injection tests; a parametrized table pinning each corrected heuristic; verdict/risk gate pins; restore-cap pins.

🤖 Generated with Claude Code