Feed code-graph caller context into the review prompt

[exactml]

Summary

This is the issue that makes config.context.code_graph (default True, in the schema since day one but never consumed) actually do something -- and the one that makes the README's core claim ("reasons over your code graph... not just the diff") true for the first time.

• run_review now builds the symbol graph (Add a Python symbol graph indexer to marginal/graph #82) and resolves the diff's changed definitions to their out-of-diff callers (Resolve a diff's changed functions to their out-of-diff callers #83) whenever config.context.code_graph is enabled, via a new _build_graph_context helper

• _build_finding_prompt folds this into a new section naming each changed definition and a short, line-numbered source snippet at each out-of-diff caller -- example output for a diff changing pkg.a.helper, called from pkg/b.py:

  This diff changes definitions that other, unchanged files in the repo call -- check whether the change is compatible with how they're used there:
  
  `pkg.a.helper` (defined in pkg/a.py) is called from:
  
  pkg/b.py:2
  1: def use_helper():
  2:     return helper()
  

• Bounded to MAX_GRAPH_CONTEXT_CALLERS = 10 caller snippets total across every changed definition, so a large fan-out can't blow up the prompt

• Fully best-effort: a definition with only in-diff callers, code_graph: false, or the graph failing to build at all (not a git checkout, git missing, a tracked file with invalid syntax) all leave the prompt byte-for-byte unchanged from today -- this never fails the review itself, the same resilience philosophy as load_policies skipping a missing file with a warning instead of aborting

Closes #84

Test plan

• make lint -- clean
• make test -- 174 passed (3 new), covering:
  • flag on + a real out-of-diff caller present -> the section appears with the correct definition, file, and snippet
  • flag explicitly off -> prompt unchanged even with the same real caller present
  • outside a git repo (graph build fails) -> prompt unchanged, warning printed to stderr, review still exits 0
• Manually verified the rendered prompt against a real two-file fixture (shown above) before writing tests, to confirm the format reads sensibly

[github-actions]

🤖 marginal review

PR #106: Feed code-graph caller context into the review prompt · open · 3b5ffdac6a4ddabb42a0bd829f09ce5b9d8b71e9 → 41230ad660b0f62e0a0e2c29c48a57804e8871ae · 3 files changed

Changed files (3)

• CHANGELOG.md
• marginal/cli/review.py
• tests/test_cli.py

⚠️ Redacted a likely secret in tests/test_cli.py before this diff reached the review model -- rotate it if it was real.

[github-actions]

🟡 Medium · 70% confidence

_build_graph_context catches bare Exception, which will also swallow programming errors (e.g. AttributeError from a bug in build_symbol_graph or find_out_of_diff_callers) rather than just expected failure modes like missing git or syntax errors. This makes it harder to detect regressions in the graph-building code since failures are silently downgraded to a warning.

[exactml]

Same point re-raised in the latest review pass (now at line 314) -- answered there: not fixing, it's a deliberate tradeoff. See #106 (comment) for the full reasoning.

[github-actions]

🟡 Medium · 60% confidence

In _build_graph_context_section, remaining is decremented by len(caller_blocks) but the per-definition slice changed.callers[:remaining] uses the pre-decrement remaining value from before this iteration, and readable snippets can be fewer than sliced callers -- this is fine, but the slicing continues to use the same shrinking remaining correctly. However, if a changed definition has more callers than remaining but some of those callers' files are unreadable, blocks may be produced with fewer than remaining snippets while still consuming a full pass, potentially causing later definitions to get truncated more aggressively than necessary due to unreadable files counted against the global cap indirectly (via slicing) rather than only against successfully rendered snippets consuming the cap.

[exactml]

Fixed in 4ee6919: the per-definition loop no longer slices changed.callers[:remaining] up front. It now iterates callers in order and stops once remaining snippets are actually collected, so an unreadable caller earlier in the list can no longer block a later, readable caller of the same definition from being surfaced. Added a regression test (test_build_graph_context_section_skips_unreadable_callers_to_reach_a_readable_one) covering exactly this case.

[github-actions]

🤖 marginal review

PR #106: Feed code-graph caller context into the review prompt · open · 3b5ffdac6a4ddabb42a0bd829f09ce5b9d8b71e9 → 4ee69190affa5d904b6af5f8f62f5d2760098a2f · 3 files changed

Changed files (3)

• CHANGELOG.md
• marginal/cli/review.py
• tests/test_cli.py

⚠️ Redacted a likely secret in tests/test_cli.py before this diff reached the review model -- rotate it if it was real.

[github-actions]

🟡 Medium · 65% confidence

_build_graph_context catches a bare Exception, which will also swallow genuine bugs (e.g. TypeError from a bad call in build_symbol_graph or find_out_of_diff_callers) and silently degrade to no context instead of surfacing them. Consider narrowing to expected failure modes (subprocess errors, OSError, SyntaxError) so real programming errors aren't masked as 'best effort'.

[exactml]

Acknowledged, not fixing: this is a deliberate tradeoff, not an oversight.

_build_graph_context exists specifically so a failure to build the code graph can never fail the review itself -- the same resilience philosophy load_policies already uses for a missing policy file (warn and continue, don't abort). Narrowing the catch to specific expected types (subprocess errors, OSError, SyntaxError) would reintroduce exactly the gap we spent this session fixing for the Anthropic provider: any unanticipated failure mode (a bug in build_symbol_graph, an edge case in find_out_of_diff_callers) would then crash the whole review instead of degrading to "no graph context, everything else still works."

The warning message already includes {exc}, so a real bug here is still visible in CI logs -- it just doesn't take the review down with it.