feat(review): deeper cross-file context via caller resolution + signature slicing#345
Merged
Merged
Conversation
…ture slicing
Review previously only resolved OUTBOUND dependencies (what changed code
calls/imports). This adds the INBOUND axis — who calls the changed code
(blast radius) — which is the highest-value context for flagging breaking
signature/type changes, while staying token-economical.
Bug fix (G2): review.rs passed `ignore.rules` (finding-type strings) to the
context resolver instead of `ignore.files` (globs like target/**,
node_modules/**). The resolver could inject build-artifact code, wasting
tokens and adding noise. Now uses ignore.files.
Changes
- G2: review.rs build_context_chain now receives &config.ignore.files.
- Config: new ContextConfig.include_callers (default true);
max_context_tokens default 3000 -> 5000.
- extraction.rs: extract_definitions_from_diff() — detects functions/types
DECLARED in added lines for Rust/Python/JS-TS/Go/Java-Kotlin (per-language
regexes).
- resolver.rs: resolve_callers() walks source files (gitignore-aware via the
`ignore` crate, so build artifacts are never scanned), bounded by
MAX_CALLER_FILES_SCAN (400) and MAX_CALLERS_PER_SYMBOL (3). Caller slices
are tiny (call line + 1 line of context, MAX_CALLER_LINES=4).
- resolver.rs: signature_only() budget fallback — when the full body won't
fit the remaining budget, inject the signature (up to `{` / 4 lines)
instead of skipping the entry entirely. New ContextPriority::CallerSite.
Design notes
- follow_depth (outbound recursion) remains at its default of 1; recursive
outbound resolution was previously a no-op placeholder and properly
implementing it (cycle-safe, budget-bounded) is a separate larger task.
The new include_callers axis delivers the real "deeper review" value,
token-economically.
- Caller resolution never self-reports (defining file excluded), respects
include_tests, and skips ignored paths.
Tests: 7 new (definition extraction across languages + dedup, caller
resolution finds/skips, signature_only header-only). All 659 tests pass;
clippy + fmt + audit clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Deepens PR review beyond the git diff — the context chain now resolves who calls the changed code (inbound / blast radius), not just what the changed code calls. This is the highest-value axis for flagging breaking signature/type changes, and it stays token-economical via bounded scanning + thin slices + a signature-only budget fallback.
Also fixes a real token-economy bug.
Bug fix (G2) — wrong ignore list
review.rspassedconfig.ignore.rules(finding-type strings like"security") to the context resolver instead ofconfig.ignore.files(globs liketarget/**,node_modules/**). The resolver could inject code from build artifacts — wasting tokens and adding noise. Now usesignore.files.New: inbound caller (blast-radius) resolution
extract_definitions_from_diff(extraction.rs)resolve_callers(resolver.rs)ignorecrate) so build artifacts are never scanned. Bounded: ≤400 files scanned, ≤3 call-sites/symbol. Injects only the call line + 1 line of contextContextPriority::CallerSiteContextConfig.include_callerstrueNew: signature-only budget fallback (Tier 3)
When the token budget can't fit a full function/type body,
signature_only()injects just the signature (up to{or 4 lines) instead of skipping the entry entirely → ~3–5× more symbols under the same budget.Tuning
max_context_tokensdefault 3000 → 5000 (more coverage; bounded by the above slicing).Design note: follow_depth
follow_depth(recursive outbound resolution) remains at default1. It was previously a no-op placeholder for depth > 1, and properly implementing it (cycle-safe + budget-bounded) is a separate, larger task. The newinclude_callersaxis is what delivers the real "deeper review" value here.Testing
include_callers=false), signature_only (header-only, no body).cargo auditclean.