Repository navigation
feat: implement issue #1090 — [Phase 2] Feed semantic symbol context (callers/callees/type-defs) into the deep + duck tiers - #1226
Conversation
…(callers/callees/type-defs) into the deep + duck tiers
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 58 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds an opt-in symbol-context pipeline that extracts touched functions, gathers GitHub code-search references, exports bounded context for deep and rubber-duck reviews, and includes comprehensive Bats coverage. ChangesSymbol Context Review
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ReviewScript
participant SymbolContext
participant GitHubCodeSearch
participant ReviewPrompts
ReviewScript->>SymbolContext: assemble PR diff context
SymbolContext->>GitHubCodeSearch: search touched symbols
GitHubCodeSearch-->>SymbolContext: reference snippets
SymbolContext-->>ReviewScript: export SYMBOL_CONTEXT_FILE
ReviewScript->>ReviewPrompts: provide optional context file
ReviewPrompts-->>ReviewScript: cross-file review findings
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
|
Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-07-14T07:53:53Z. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a semantic symbol-context assembler that extracts touched functions from a diff and gathers caller, callee, and type-definition reference contexts using GitHub code search to aid downstream review tiers. The review feedback focuses on improving the robustness of the shell scripts and BATS tests, including utilizing $BATS_TEST_TMPDIR for temporary files, guarding grep -c commands under set -e, asserting exact exit statuses for grep failures, and applying safer jq parsing patterns with optional chaining and null-coalescing conversions.
There was a problem hiding this comment.
Pull request overview
Adds an opt-in “symbol context” pre-pass to the PR review cascade that collects cross-file reference snippets for functions touched in the diff, and makes that context available to both the deep-review and rubber-duck tiers (issue #1090, Phase 2).
Changes:
- Introduces
scripts/lib/symbol-context.shto extract touched functions from a unified diff and (when enabled) assemble per-function reference contexts viagh search code, emitting measurable per-function counts. - Wires the symbol-context pass into
scripts/review-one-pr.shbehindSYMBOL_CONTEXT_ENABLED(default off). - Updates deep-review + rubber-duck prompts to optionally read
$SYMBOL_CONTEXT_FILE, and adds Bats unit tests for the mapper/assembler.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/dev-lead/unit/test_symbol_context_map.bats | Adds unit tests for the “pure” touched-function extraction helper. |
| tests/dev-lead/unit/test_symbol_context_assemble.bats | Adds unit tests for the I/O “assemble symbol context” path using a stubbed gh. |
| scripts/review-one-pr.sh | Sources the new lib and conditionally runs the symbol-context pass before the review tiers. |
| scripts/lib/symbol-context.sh | Implements touched-function extraction + gh search code context assembly with size/volume caps and graceful degradation. |
| prompts/rubber-duck.md | Allows the duck pass to read $SYMBOL_CONTEXT_FILE for cross-file adversarial checking. |
| prompts/deep-review.md | Allows deep-review to read $SYMBOL_CONTEXT_FILE for cross-file reasoning on touched symbols. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Dev-Lead — fix-reviews (applied)Changes committed and pushed. |
Dev-Lead — review-changes (applied)Changes committed and pushed. |
Dev-Lead — waiting on PR blockers (intent: fix-reviews)PR: #1226 |
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/lib/symbol-context.sh`:
- Around line 1-3: Add `set -euo pipefail` near the top of the Bash script,
immediately after the shebang in the symbol-context assembler, so it applies
when the file is sourced or run independently.
- Around line 171-288: Throttle searches in assemble_symbol_context so
authenticated code-search requests remain within GitHub’s 10-requests-per-minute
token limit. Add pacing or backoff between sc_search_symbol_contexts calls, or
lower the default SYMBOL_CONTEXT_MAX_FUNCS cap to prevent larger diffs from
exhausting the quota while preserving existing graceful degradation.
In `@scripts/review-one-pr.sh`:
- Around line 694-709: Move the SYMBOL_CONTEXT_ENABLED block that calls
assemble_symbol_context from the pre-TRIAGE_ESCALATE flow into the tier-2 branch
immediately before run_agentic/run_duck. Keep the existing repository guard,
output path, and best-effort behavior unchanged, and defer unsetting PR_DIFF
until after tier-2 symbol-context assembly completes.
In `@tests/dev-lead/unit/test_symbol_context_assemble.bats`:
- Around line 94-103: Strengthen the assertions in the test “emits a measurable
per-function context-count log line (AC `#2`)” by requiring the output to contain
a numeric digit in the context-count portion alongside “assemble_thing”.
Preserve the existing checks for the “[symbol-context]” marker and function
name, and use a regex assertion so malformed values such as “undefined” do not
pass.
- Around line 49-69: Update the introductory stub comment in make_gh_stub to
state that gh search code returns 4 result items, matching the four JSON entries
for scripts/a.sh through scripts/d.sh; leave the stub behavior unchanged.
- Around line 113-119: Update the test “the configured minimum
contexts-per-function is honored” so its fixture distinguishes
SYMBOL_CONTEXT_MIN=2 from the default value: make the GH stub honor --limit or
return only two matching items, then assert the output contains exactly two
matching contexts rather than using a >= 2 check.
- Around line 43-46: Add tests in test_symbol_context_assemble.bats covering
SYMBOL_CONTEXT_MAX_FUNCS with a diff that touches multiple functions. Verify the
configured function cap is enforced and each function’s context output and
logging remain isolated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 16a3157b-2af4-4880-a8a9-8b3f2ebc5023
📒 Files selected for processing (6)
prompts/deep-review.mdprompts/rubber-duck.mdscripts/lib/symbol-context.shscripts/review-one-pr.shtests/dev-lead/unit/test_symbol_context_assemble.batstests/dev-lead/unit/test_symbol_context_map.bats
Dev-Lead — rate-limited (intent: review-changes)PR: #1226 |
|
Note @don-petry I received your request but all AI engines are currently rate-limited. I'll retry automatically once the rate limit clears. |
Dev-Lead — review-changes (applied)Changes committed and pushed. |
Dev-Lead — waiting on PR blockers (intent: fix-reviews)PR: #1226 |
|
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: dcee6a2ceb081ed5d15e758ce5d9d0680a46fbc6
Review mode: triage-approved (single reviewer)
Summary
Confirms the triage assessment: an opt-in, default-inert symbol-context pass (issue #1090, Phase 2) that feeds caller/callee/type-def reference contexts from gh code search into the deep-review and rubber-duck tiers. New lib scripts/lib/symbol-context.sh + wiring in review-one-pr.sh + prompt docs + two bats suites. Gated behind SYMBOL_CONTEXT_ENABLED (default false), so default review behavior and cost are unchanged. All CI green, all 17 review threads resolved, latest CodeRabbit review on the head SHA is APPROVED.
Linked issue analysis
Closes #1090. All acceptance criteria substantively addressed: (1) >=3 contexts per touched function via SYMBOL_CONTEXT_MIN default 3, using the gh search code fallback path per the #839 LSP no-go; (2) measurable ::notice::[symbol-context] per-function count lines plus a SYMBOL_CONTEXT_COUNTS summary in the block, both covered by tests; (3) the same SYMBOL_CONTEXT_FILE is documented for the rubber-duck prompt so the adversarial pass reasons on equal footing; (4) inert by default and opt-in — when SYMBOL_CONTEXT_ENABLED is not true the file is never set and prompts/cost are byte-identical; graceful degradation on search failure (single ::warning::, always exit 0) is tested; (5) holdout-eval non-regression is satisfied by construction for default behavior since the feature is off by default.
Findings
No blocking findings.
Non-blocking nits:
- Doc/code mismatch: the assemble_symbol_context header comment says the per-PR function cap defaults to 20, but the code default is ${SYMBOL_CONTEXT_MAX_FUNCS:-10}.
- ${block:0:max_bytes} truncates by characters rather than bytes under multibyte locales; benign in the C-locale CI environment and consistent with the existing 8 KB downstream-impact cap pattern.
Security review: _symbol_context_gh reuses the GH_PAT convention from downstream-impact.sh without logging the token; the symbol is passed to gh search code as a quoted argument (no eval, no injection path); the search is scoped to $GITHUB_REPOSITORY and explicitly skipped with a warning when that variable is unset, avoiding an unscoped cross-repo search. Search-result snippets flowing into review prompts follow the same trust boundary as the existing downstream-impact block. Secret-scan MCP tool unavailable in this environment; the gitleaks CI check passed.
CI status
All required checks green: shellcheck, bats, unit-tests, CodeQL (actions + python), AgentShield, Agent Security Scan, gitleaks secret scan, SonarCloud quality gate, prompt-coverage, validate-fixtures, holdout-guard, and all workflow-structure guards. Remaining checks skipped by design (dependency-audit ecosystems not present).
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
Closes #1090
Implemented by dev-lead agent. Please review.
Summary by CodeRabbit