Skip to content

feat(#1762): prioritize functional correctness over surface-level checks in review - #1902

Merged
ben-alkov merged 1 commit into
mainfrom
agent/1762-review-correctness-priority
Jun 4, 2026
Merged

feat(#1762): prioritize functional correctness over surface-level checks in review#1902
ben-alkov merged 1 commit into
mainfrom
agent/1762-review-correctness-priority

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Add runtime mechanism verification instructions to the code-review skill's correctness dimension, budget allocation priority to the pr-review orchestrator's triage step, and a runtime mechanism checklist to the correctness sub-agent.

These changes address a gap where the review agent spent tokens on surface-level consistency checks (stale terminology, naming mismatches) while missing functional bugs that human reviewers caught — such as guard mechanisms that would never activate, missing output format contracts between components, and unhandled failure paths.

Three files changed:

  • code-review/SKILL.md: new "Runtime mechanism verification" bullet
    in the correctness dimension instructing reviewers to trace guards
    from producer to consumer and verify they function at runtime.
  • pr-review/SKILL.md: new "Budget allocation priority" section
    (3a-1) establishing priority order: functional correctness >
    security > intent coherence > docs/style/contracts.
  • pr-review/sub-agents/correctness.md: new "Runtime mechanism
    checklist" with three items covering producer-consumer tracing,
    format expectation matching, and failure path handling.

Note: make lint could not run due to Go toolchain permission error in sandbox (unrelated to these markdown-only changes).


Closes #1762

Post-script verification

  • Branch is not main/master (agent/1762-review-correctness-priority)
  • Secret scan passed (gitleaks — 7e768afe50ea067e39066e4adc5451f22f85cec0..HEAD)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

…cks in review

Add runtime mechanism verification instructions to the code-review
skill's correctness dimension, budget allocation priority to the
pr-review orchestrator's triage step, and a runtime mechanism
checklist to the correctness sub-agent.

These changes address a gap where the review agent spent tokens on
surface-level consistency checks (stale terminology, naming
mismatches) while missing functional bugs that human reviewers
caught — such as guard mechanisms that would never activate, missing
output format contracts between components, and unhandled failure
paths.

Three files changed:
- code-review/SKILL.md: new "Runtime mechanism verification" bullet
  in the correctness dimension instructing reviewers to trace guards
  from producer to consumer and verify they function at runtime.
- pr-review/SKILL.md: new "Budget allocation priority" section
  (3a-1) establishing priority order: functional correctness >
  security > intent coherence > docs/style/contracts.
- pr-review/sub-agents/correctness.md: new "Runtime mechanism
  checklist" with three items covering producer-consumer tracing,
  format expectation matching, and failure path handling.

Note: make lint could not run due to Go toolchain permission error
in sandbox (unrelated to these markdown-only changes).

Closes #1762
@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown

Site preview

Preview: https://0e145244-site.fullsend-ai.workers.dev

Commit: 7d8915e28b0e6fc4abe067be3ec8d4b6bb70af65

@ben-alkov ben-alkov self-assigned this Jun 4, 2026
@fullsend-ai-review

Copy link
Copy Markdown

Review

Findings

Low

  • [test-inadequate] internal/scaffold/fullsend-repo/skills/pr-review/sub-agents/correctness.md:21 — The runtime mechanism checklist is added as prompt-level instructions with no regression test to verify the correctness sub-agent actually performs these checks. Issue Review agent should prioritize functional correctness over surface-level consistency checks #1762 describes four specific bugs on PR feat(review): parallel specialized sub-agents for PR review #1550 that the review agent missed; without replaying a similar test case through the updated agent, there is no empirical validation that these instructions are effective. The issue's own validation criteria ("on the next 5 PRs...") acknowledges this gap.

  • [logic-error] internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md:207 — Section 3a-1 introduces "budget allocation priority" as advisory guidance, but the orchestrator has no mechanism to enforce budget allocation across sub-agents. Sub-agents are dispatched simultaneously with identical context packages and run independently. The priority ordering can only influence synthesis (step 6), where the orchestrator already determines outcome by severity rather than dimension priority. The instruction is coherent as LLM guidance but has no deterministic enforcement point.

Info

  • [design-direction] internal/scaffold/fullsend-repo/skills/pr-review/SKILL.md:13 — The pr-review orchestrator acknowledges its departure from ADR-0018 ("scripted pipelines for multi-agent orchestration") and notes "A superseding ADR is needed to formally retire ADR-0018's prohibition." This pre-existing architectural debt remains unresolved. Each subsequent change to the orchestrator (including this PR's section 3a-1) accumulates more LLM-driven control flow, making eventual ADR resolution more urgent.

When evaluating tests, check git history of modified test files for
assertion loosening or coverage reduction that coincides with production
changes — this is a security-adjacent concern (split-payload pattern).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] test-inadequate

The runtime mechanism checklist is added as prompt-level instructions with no regression test to verify effectiveness. Without replaying PR #1550 or a similar test case through the updated agent, there is no empirical validation these instructions work.

@@ -205,6 +205,29 @@ dimension by keyword, or to `correctness` as a fallback.

Each sub-agent receives ONLY the prior findings for its own dimension.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] logic-error

Section 3a-1 introduces budget allocation priority as advisory guidance, but the orchestrator has no mechanism to enforce it. Sub-agents run independently with identical context packages. The priority ordering can only influence synthesis, which already determines outcome by severity.

@fullsend-ai-review fullsend-ai-review Bot added the ready-for-merge All reviewers approved — ready to merge label Jun 4, 2026
@ben-alkov
ben-alkov added this pull request to the merge queue Jun 4, 2026
Merged via the queue into main with commit 645f578 Jun 4, 2026
10 of 12 checks passed
@ben-alkov
ben-alkov deleted the agent/1762-review-correctness-priority branch June 4, 2026 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-merge All reviewers approved — ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Review agent should prioritize functional correctness over surface-level consistency checks

1 participant