Deferred from Copilot review on #2 (src/pr_repair/output/pr_commentary.py::_patch_status).
Problem
_patch_status infers an autofix finding's status from finding.file_path in execution.modified_files. If multiple autofix findings target the same file, one successful edit makes all findings for that file render ✅ applied — even ones that were skipped during instruction generation (e.g. missing line info, out-of-range, or no applicable instruction). The per-finding column can therefore overstate what was applied.
Why deferred
An accurate fix requires carrying per-finding apply results end to end, which spans components owned by other PRs in the stack:
patch_applier.apply_patch_instructions would return which finding_ids were actually applied,RepairExecution (types) would carry applied_finding_ids (or instruction-level results),repair_executor / llm_apply would populate it,_patch_status would key off applied finding ids instead of modified_files.
This is a cross-cutting change and doesn't belong in the output-contract PR.
Proposed resolution
Add applied_finding_ids: list[str] to RepairExecution, populate it from the applier's per-instruction results (each instruction already carries finding_id), and switch _patch_status to finding.finding_id in execution.applied_finding_ids.
Impact
Cosmetic only (PR comment table). No effect on what is actually applied/verified/pushed.
Labels: deferred, from-review
Deferred from Copilot review on #2 (
src/pr_repair/output/pr_commentary.py::_patch_status).Problem
_patch_statusinfers an autofix finding's status fromfinding.file_path in execution.modified_files. If multiple autofix findings target the same file, one successful edit makes all findings for that file render✅ applied— even ones that were skipped during instruction generation (e.g. missing line info, out-of-range, or no applicable instruction). The per-finding column can therefore overstate what was applied.Why deferred
An accurate fix requires carrying per-finding apply results end to end, which spans components owned by other PRs in the stack:
patch_applier.apply_patch_instructionswould return whichfinding_ids were actually applied,RepairExecution(types) would carryapplied_finding_ids(or instruction-level results),repair_executor/llm_applywould populate it,_patch_statuswould key off applied finding ids instead ofmodified_files.This is a cross-cutting change and doesn't belong in the output-contract PR.
Proposed resolution
Add
applied_finding_ids: list[str]toRepairExecution, populate it from the applier's per-instruction results (each instruction already carriesfinding_id), and switch_patch_statustofinding.finding_id in execution.applied_finding_ids.Impact
Cosmetic only (PR comment table). No effect on what is actually applied/verified/pushed.
Labels: deferred, from-review