fix: refuse review findings tables that would lose structured findings

[apackeer]

Summary

parseReviewSection stored an empty findings array whenever the reviewer's ### Findings table lacked a required column, so a review could be recorded as READY or NOT-READY with prose findings in its body and no structured findings — the approval gate then demanded no dispositions. This change refuses such a completion with the exact required columns instead, and makes the row selection robust to inline Markdown in cells.

Observed on main in a live paired study of Requirements Analysis (classic scope, Opus 4.8 orchestrator, product-lead reviewer at medium): 21 of 40 reviewed runs stored findings: [] while the review body listed Minor/Major findings, because the reviewer wrote ID | Severity | Finding | Evidence/Location | Suggestion (five columns) instead of the declared six. An earlier audit of historical records found 167 review records with 292 textual findings absent from their structured list.

Changes

• core/tools/aidlc-lib.ts parseReviewSection: a table missing any of ID | Severity | Location | Finding | Required action | Status throws …findings table is missing required columns: <list>. Use columns: …; a table missing its Markdown separator row, or repeating a required column, also throws. A ### Findings heading with no table at all still yields no findings (unchanged).
• core/tools/aidlc-lib.ts visibleMarkdownLines: new singleLineTableCells option, used by the review parser, so each table row has its own inline context — a stray backtick or an unfinished inline HTML tag in one finding cell can no longer pair across rows and hide the next finding (the counterexample to the first version of this fix).
• core/aidlc-common/protocols/stage-protocol-reviewer.md: the dispatch list names the six required columns and the separator row; a shortened table is refused, not read as an empty findings list.
• docs/reference/04-stage-protocol.md: documents the refusal contract.
• tests/unit/t304-review-brief.test.ts: refusal cases for missing/duplicate columns and missing separator; cross-row backtick (single and double) and unfinished-HTML cases asserting both finding IDs and original cell text survive.

No version, README badge, or CHANGELOG change, per the release-metadata policy.

User experience

Before. When the reviewer emitted a non-canonical findings table, REVIEW_COMPLETED was recorded with zero structured findings. The stage was presented as reviewed and the gate opened with nothing to disposition; the reviewer's findings existed only as prose in the review file the human is not shown.

After. The same review is an INCOMPLETE attempt: aidlc-log.ts review --verdict refuses with Refusing REVIEW_COMPLETED for "<stage>": <artifact>: findings table is missing required columns: Location, Required action, Status. Use columns: ID | Severity | Location | Finding | Required action | Status. Per the reviewer protocol the orchestrator re-dispatches the reviewer once via --retry-pending (a full reviewer run, not a single turn); if that attempt is also incomplete, the existing fallback applies (terminal NOT-READY with --fallback-finding "review did not complete within its turn budget"). When the retry succeeds, the findings are structured, so the gate shows them and requires a disposition per finding before approval — the experience users already get when the table is well-formed. Reviewers that already write the six-column table see no change; reviewless stages are unaffected. The dispatch text now states the six columns up front, which should reduce malformed first attempts; that reduction is not yet measured.

Checklist

• I have reviewed the contributing guidelines
• I have performed a self-review of this change
• Changes have been tested
• Changes are documented
• If this change adds an input to any fingerprint, epoch, or receipt identity, the description names the human-visible change it detects

Test plan

• bun test tests/unit/t304-review-brief.test.ts — 41 pass (includes the new refusal and cross-row cases).
• bun run typecheck — pass (run after bun scripts/package.ts, since tests/ type-check against dist/).
• bun scripts/package.ts --check — deterministic across all seven harnesses.
• Reproduction: a real 5-column review body from the study — main parser returns findings.length === 0; patched parser throws naming Location, Required action, Status. Cross-row backtick and unfinished-HTML inputs return both R-01 and R-02 with original cell text.
• Manual: dispatch a reviewer whose table omits Status; confirm the completion is refused with the column list, the --retry-pending re-dispatch records the corrected table, and the gate presents the findings for disposition.

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the project license.