Harden Copilot PR-review JSON parser against resume-fence corruption

[gggdttt]

Problem

The Copilot PR-review reviewer silently posted 0 findings on a run where the agent actually produced 42 (run 28168894037: blocker × 5, major × 24, minor × 13). The job stayed green because "parsed to zero findings" is indistinguishable from "clean PR" — the findings were lost in parsing, not in review.

Two compounding causes in Parse-BCQualityReport:

1. Resume-fence corruption. The agent breaks off mid-emission and resumes by re-opening a bare ```json fence with no Placeholder ... marker. When that fence lands inside an unterminated string, Remove-StructuralFences (which deliberately preserves in-string fences, e.g. a ```al sample in suggested-code) cannot clear it, so ConvertFrom-Json rejects the entire document.

2. Wrong-fragment-wins. The candidate scan accepted the first parseable fragment. The transcript echoes every sub-skill's own JSON report inside fences, and bare arrays (e.g. ['articles'] from reasoning text) parse as valid JSON. So when the full document is unparseable, a wrong fragment is accepted and the real report is silently dropped — and because that fragment reports outcome: completed / findings: [], it even slips past the fail-on-parse-error safeguard.

Fix

• Repair-ResumeFenceJson (new, third repair path): a conservative, string-aware splice that clears a bare resume fence only when it sits between an unterminated property ("key": "... with an odd count of unescaped quotes) and a re-emission of that same key. That triple signature distinguishes interrupted-emission resumes from ordinary transcript noise. Clean output is never touched (callers try unmodified candidates first).

• Tiered candidate preference (replaces first-parseable-wins): orchestrator-shaped (sub-results/dispatch) → findings-bearing object → any parseable candidate. Leniency is preserved, but the correct fragment always wins and bare fragments like ['articles'] can only be a last-resort fallback.

Why both bugs are needed

The most recent production instance had its stray fence between two top-level members (outside any string), which Remove-StructuralFences already clears — so for that case the tiered preference alone recovers all 42 findings. Repair-ResumeFenceJson additionally covers the variant where the fence is trapped inside an unterminated string, which the fence-stripper cannot touch.

Testing

Validated locally against a regression suite that AST-extracts the parser functions and replays the corruption shapes (resume-fence-in-string, wrong-fragment selection, Placeholder-marker interruption, and the exact production instance with ['articles'] + a stray mid-object fence), plus guards confirming clean reports and legitimate in-string code fences are unaffected — 16/16 assertions pass.

Note: the regression test script is intentionally not included in this PR, as there is currently no pipeline wired up to execute it.

Refs AB#640481 (related: AB#638957).

[github-actions]

Copilot PR Review

Iteration 7 · Outcome: not-applicable

The diff contains only a PowerShell file (tools/Code Review/scripts/Invoke-CopilotPRReview.ps1). No AL source changes are present; all six AL sub-skills returned not-applicable.

Knowledge source: https://github.com/microsoft/BCQuality@822cae1b2771ac25f665f73369f69093bd4fd630

No findings were posted for this iteration.

Orchestrator pre-filter (13 file(s) excluded)

• layer-disabled (knowledge) : 13 file(s)

_{Findings produced by the Copilot CLI agent against BCQuality at 822cae1b2771ac25f665f73369f69093bd4fd630. Reply 👎 on any inline comment to flag false positives.}