---
cycle: 3
phase: 5
status: closed 2026-05-18 — PASS-WITH-REVISIONS, revisions applied
date_opened: 2026-05-18
date_closed: 2026-05-18
parent: k3_mc_phase4.md
reviewer: Claude Sonnet (general-purpose Agent, fresh context)
plan_author: Claude Opus 4.7 (this session)
verdict: PASS-WITH-REVISIONS
---

# Cycle 3, Phase 5 — Second-Model Review of MC Plan

Same handoff: in-session Agent (Sonnet, fresh context), files read
direct from disk, 5 review prompts + "anything else."

Outcome: **1 RED (off-by-3 `src_off` indexing bug)**, **2 YELLOW**
(shaderdb LUT gate for filter table, "MUST" assert language for
contracts). Cycle-1+2 RED patterns (write race, barrier UB,
subgroup-ops table error) did not recur.

**Phase 5 paid off again.** The RED would have caused a bit-exact
mismatch on the first run with cryptic "high index source pixels are
wrong" symptoms — likely 1-2 debug cycles to track down without the
review.

## Review (verbatim)

````markdown
## Verdict
PASS-WITH-REVISIONS — no RED-class correctness bugs. Two YELLOW findings
require plan amendments before Phase 6 proceeds. ...

[full review preserved — reviewer's RED finding 4 traces the off-by-3:
shader's `src_off = block_base + 3` + `src_stride_u8 = 16` + reading
`s[0..14]` causes high-index reads to spill into next row]
````
*(Verbatim review in agent output; key findings paraphrased below.)*

| # | Severity | Issue | Resolution |
|---|---|---|---|
| 1 (orientation) | GREEN | All 8 oN expressions stencil-aligned correctly | accepted |
| 2 (filter LUT) | YELLOW | `const int FILTER_REGULAR[16][8]` may inflate uniform count or compile to large LUT | Phase 6 to record uniform count via `V3D_DEBUG=shaderdb`; if >~144 uniforms, escalate filter to SSBO binding 3 |
| 3 (race safety) | GREEN-w/note | `stride ≥ 8` contract correct; phrasing softer than cycle-2 standard | applied: §5 MUST assert |
| 4 (`src_off` semantics) | **RED** | Plan said "src_off mirrors src+3"; with stride=16 shader's `s13`/`s14` read into next row's first 2 bytes | **applied: src_off = raw block base (no +3 shift); shader reads s[0..14] from there** |
| 5 (missing) | GREEN-w/note | Coefficient overflow safely fits int32 (worked bound); no missing barrier-UB or write-race issues | accepted |
| 6 (assert MUST language) | YELLOW | "Bench enforces with asserts" softer than cycle-2 MUST pattern | applied: §5 MUST language |
| 7 (no barrier OK) | GREEN | Cycle-1 finding-7 doesn't apply (no barrier) | accepted |
| 8 (filter table matches) | GREEN | `vp9_mc_ref.c` filter values match `vp9_subpel_filters_table.c[1]` verbatim | accepted |

## Resolution (applied to phase4 inline)

1. **§4** — Clarified `src_off` is the byte offset of the **first byte
   of the source block in the SSBO buffer** (NOT shifted by +3). The
   C bench's `src + 3` C-caller convention does NOT carry into the
   SSBO offset. Shader reads `s[k] = u_src.src[src_off + row*stride + k]`
   for k=0..14, which equals `master_src[block_base + row*stride + k]`,
   matching the C ref's per-row read of `master_src[block_base + row*stride + (x..x+7)]`
   for output col x ∈ 0..7.

2. **§5** — Hardened "Bench enforces" to "Phase 6 MUST add
   `assert(dst_stride_u8 >= 8 && src_stride_u8 >= 15)` in
   `bench_v3d_mc.c`'s meta-construction loop." Cycle-2 finding-4
   pattern applied.

3. **§5** — Added: "Phase 6 MUST run `V3D_DEBUG=shaderdb` after first
   compile and record uniform count. If uniform count > ~144,
   escalate filter to a dedicated SSBO binding 3."

After revisions: **Phase 4''' APPROVED for Phase 6''' implementation.**