What happened
On PR #752, the correctness sub-agent in review run 31815407032 (Aug 14, 15:37 UTC) explicitly identified that the GitLab variant of the issue-labels skill (skills/issue-labels/gitlab/SKILL.md) did not have the trigger phrase being added to the GitHub variant. However, the sub-agent classified this as "outside PR scope" and did not emit a finding.
Human reviewer waynesun09 had flagged this same gap as a MEDIUM finding approximately 30 minutes earlier, correctly identifying it as a necessary companion change. The sub-agent saw the same problem the human saw but chose to suppress it.
Per the correctness sub-agent's own definition (skills/pr-review/sub-agents/correctness.md), its "Do not own" clause states: "Do not own: Naming style, doc staleness, PR scope, injection defense." PR scope evaluation belongs to the intent-coherence sub-agent. The correctness sub-agent is explicitly instructed to look beyond the diff for consumer completeness, rename staleness, and cross-file verification. Suppressing a finding because it's "outside PR scope" is applying a dimensional filter the sub-agent does not own.
What could go better
The correctness sub-agent's instructions already require cross-file verification and explicitly exclude scope evaluation from its responsibilities. Yet the LLM still applied scope-based filtering to suppress a valid finding. This suggests the existing "Do not own: PR scope" clause is not strong enough — the sub-agent interprets it as "don't evaluate whether the PR's scope is right" but also uses it as license to suppress findings about files outside the diff.
The root cause is ambiguity: "Do not own: PR scope" means "don't judge whether the PR should be bigger or smaller" (that's intent-coherence's job). It does NOT mean "suppress correctness findings about files outside the diff." The sub-agent conflated these two meanings.
Confidence level: High. The sub-agent's log explicitly states it noticed the gap and chose not to report it. The instructions already require beyond-diff analysis for certain patterns. The fix is to clarify the boundary between "don't evaluate PR scope" and "do report correctness-relevant findings regardless of which files they affect."
Proposed change
Add an explicit clarification to skills/pr-review/sub-agents/correctness.md in the "Do not own" section or adjacent guidance:
-
After the existing "Do not own" clause, add language like: "'Do not own PR scope' means you should not judge whether the PR's scope is appropriate — that is intent-coherence's responsibility. It does NOT mean you should suppress correctness findings about files outside the PR diff. When you detect a correctness-relevant inconsistency in a file not modified by the PR (e.g., a sibling variant, a consumer, a paired implementation), report it. Let the orchestrator decide whether the finding is actionable."
-
Consider adding "cross-variant parity" as an explicit correctness check pattern alongside the existing consumer-completeness and rename-staleness patterns: "When the diff modifies one variant of a multi-variant implementation (e.g., forge-specific skill variants), verify that sibling variants remain consistent."
Validation criteria
On the next 5 review runs where the correctness sub-agent identifies a beyond-diff consistency gap (cross-variant, consumer staleness, or similar), it should emit the finding rather than suppressing it as out of scope. Verify by checking correctness sub-agent output in review run logs for language like "outside scope" or "not in PR" — these should not appear as reasons for suppressing a detected consistency gap. The challenger sub-agent can still filter findings that are genuinely not correctness-relevant, but the initial detection should not be self-censored.
Generated by retro agent from #752
What happened
On PR #752, the correctness sub-agent in review run 31815407032 (Aug 14, 15:37 UTC) explicitly identified that the GitLab variant of the
issue-labelsskill (skills/issue-labels/gitlab/SKILL.md) did not have the trigger phrase being added to the GitHub variant. However, the sub-agent classified this as "outside PR scope" and did not emit a finding.Human reviewer waynesun09 had flagged this same gap as a MEDIUM finding approximately 30 minutes earlier, correctly identifying it as a necessary companion change. The sub-agent saw the same problem the human saw but chose to suppress it.
Per the correctness sub-agent's own definition (
skills/pr-review/sub-agents/correctness.md), its "Do not own" clause states: "Do not own: Naming style, doc staleness, PR scope, injection defense." PR scope evaluation belongs to the intent-coherence sub-agent. The correctness sub-agent is explicitly instructed to look beyond the diff for consumer completeness, rename staleness, and cross-file verification. Suppressing a finding because it's "outside PR scope" is applying a dimensional filter the sub-agent does not own.What could go better
The correctness sub-agent's instructions already require cross-file verification and explicitly exclude scope evaluation from its responsibilities. Yet the LLM still applied scope-based filtering to suppress a valid finding. This suggests the existing "Do not own: PR scope" clause is not strong enough — the sub-agent interprets it as "don't evaluate whether the PR's scope is right" but also uses it as license to suppress findings about files outside the diff.
The root cause is ambiguity: "Do not own: PR scope" means "don't judge whether the PR should be bigger or smaller" (that's intent-coherence's job). It does NOT mean "suppress correctness findings about files outside the diff." The sub-agent conflated these two meanings.
Confidence level: High. The sub-agent's log explicitly states it noticed the gap and chose not to report it. The instructions already require beyond-diff analysis for certain patterns. The fix is to clarify the boundary between "don't evaluate PR scope" and "do report correctness-relevant findings regardless of which files they affect."
Proposed change
Add an explicit clarification to
skills/pr-review/sub-agents/correctness.mdin the "Do not own" section or adjacent guidance:After the existing "Do not own" clause, add language like: "'Do not own PR scope' means you should not judge whether the PR's scope is appropriate — that is intent-coherence's responsibility. It does NOT mean you should suppress correctness findings about files outside the PR diff. When you detect a correctness-relevant inconsistency in a file not modified by the PR (e.g., a sibling variant, a consumer, a paired implementation), report it. Let the orchestrator decide whether the finding is actionable."
Consider adding "cross-variant parity" as an explicit correctness check pattern alongside the existing consumer-completeness and rename-staleness patterns: "When the diff modifies one variant of a multi-variant implementation (e.g., forge-specific skill variants), verify that sibling variants remain consistent."
Validation criteria
On the next 5 review runs where the correctness sub-agent identifies a beyond-diff consistency gap (cross-variant, consumer staleness, or similar), it should emit the finding rather than suppressing it as out of scope. Verify by checking correctness sub-agent output in review run logs for language like "outside scope" or "not in PR" — these should not appear as reasons for suppressing a detected consistency gap. The challenger sub-agent can still filter findings that are genuinely not correctness-relevant, but the initial detection should not be self-censored.
Generated by retro agent from #752