Skip to content

review-pr: subagent prompts omit annotated diff → findings return line:null → inline comments dropped #4289

Description

@Trecek

Problem

The review-pr skill spawns 6 parallel audit subagents (arch, tests, defense, bugs, cohesion, slop) but passes them code excerpts and descriptions instead of the full annotated diff with [LNNN] markers. As a result, subagents return findings with line: null — they have no way to anchor findings to specific diff lines.

The orchestrator then drops these findings from inline posting because they can't be placed on a valid diff line. In the PR #4287 review, this caused 2 of 3 actionable findings to be silently omitted from the initial review post. The orchestrator only posted 1 inline comment (the one it could manually map) out of 3 it should have posted.

Root Cause

The SKILL.md says:

Each line in the diff is prefixed with [LNNN] where NNN is the new-file line number.
When reporting findings, use the [LNNN] number as the line value in your finding.

But the subagent prompts did NOT include the annotated diff content with [LNNN] markers. They received narrative descriptions of the code changes. The {annotated_diff_content} template variable was effectively empty/missing from the subagent prompt construction.

Impact

  • Findings returned with line: null cannot be posted as inline comments
  • Orchestrator should have mapped null-line findings to their actual lines (it has the annotated diff in its own context) but only did so for 1 of 3 findings
  • The _FIELD_RE warning (line 26) and double-parse info (line 890) were only posted after user intervention

Reproduction

  • PR: Rectify: resolve-review Codex interchangeability + silent-success immunity (#4282) #4287
  • AutoSkillit session: e781ef9459914ae5
  • Session working directory: /dev/shm/autoskillit-sessions/e781ef9459914ae5
  • Review artifacts: .autoskillit/temp/review-pr/*_4287.*
    • annotated_diff_4287.txt — the annotated diff that should have been passed to subagents
    • valid_lines_4287.json — the valid line map used for filtering
    • raw_findings_4287.json — only contains 1 finding (the one that was manually mapped)
    • diff_context_4287.json — handoff file (incomplete)
    • summary_4287_20260718_210704.md — review summary

Expected Behavior

  1. Each subagent prompt MUST include the full {annotated_diff_content} (or a chunked portion for its dimension) so findings always carry a [LNNN] line number
  2. Alternatively: the orchestrator MUST have a fallback mapping step that resolves line: null findings against the annotated diff before the posting step — grep for the finding's code context and assign the nearest valid line
  3. The Step 6.5 confirmation check ("I posted N inline comments") should HALT and investigate when postable findings exist but 0 comments were posted — currently it only states the discrepancy without blocking

Fix Direction

Two-pronged:

  1. Primary: Pass the annotated diff content into each subagent prompt template (the {annotated_diff_content} substitution that the SKILL.md already specifies)
  2. Fallback: Add a line-resolution pass between Step 4 (aggregate) and Step 6 (post) that maps any line: null finding to its actual line by searching the annotated diff for the finding's file + code pattern

Diagnostic Paths

# Review artifacts
.autoskillit/temp/review-pr/annotated_diff_4287.txt
.autoskillit/temp/review-pr/valid_lines_4287.json
.autoskillit/temp/review-pr/raw_findings_4287.json
.autoskillit/temp/review-pr/diff_context_4287.json
.autoskillit/temp/review-pr/summary_4287_20260718_210704.md

# Session directory
/dev/shm/autoskillit-sessions/e781ef9459914ae5

Metadata

Metadata

Assignees

No one assigned

    Labels

    recipe:remediationRoute: investigate/decompose before implementationstagedImplementation staged and waiting for promotion to main

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions