Skip to content

review: LLM context lacks enclosing control-flow — false positives on shared match arms #523

Description

@ajianaz

Observed (real case)

PR codecoradev/uteke#1084, cora review run 2 flagged a false positive:

handlers.rs:224 — the handler validates author_type up front, but only the non-contradiction branch calls set_author_type. When detect_contradiction is true, the result branch returns the id without ever setting it.

Wrong: set_author_type sits in the shared Ok(id) arm of a single match result; both remember and remember_with_contradiction(...).map(|(id,_)| id) produce id before the match, so both reach the same arm. Verified manually.

Context: run 1 on the same PR found a real adjacent bug (persist-before-validate). Correct 1/2 — the FP is a context problem, not model quality.

Root cause

build_review_prompt (src/engine/llm.rs:787) sends only the raw diff hunks (+3 context lines), file paths, static context, focus/rules. For control-flow claims ("branch X never reaches call Y") that is insufficient: the enclosing match arm, its producers, and the preceding if/else were outside the hunk. The LLM reconstructs structure from the diff shape and gets it wrong — plausible reasoning over missing evidence.

Proposal (staged)

  1. Enclosing-scope context (cheap, high-value): for each hunk, include the enclosing function/block from the post-image file (tree-sitter function ranges via the index). Cap ~120 lines/function, labeled as surrounding code for control-flow verification.
  2. Prompt guardrail: review rule — "Before claiming a path is unreachable or a call missing on a branch, verify branch structure in the surrounding code, not only diff context lines."
  3. Later — call-graph assist: cross-check symbol reachability against the indexed call graph; annotate or downgrade confidence instead of asserting.

Acceptance

  • Regression fixture: diff where a call is added inside a shared match arm fed by two producer paths — review must NOT flag branch-specific reachability without evidence.
  • No token blowup: surrounding context bounded, injected only for hunks touching branching constructs (match, if, ?, early return).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions