Strengthen active-context policy and explainable injection#278
Merged
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds request-focused analysis and metadata to context assembly, inspection, RPC, and CLI rendering: introduces a RequestFocus record, enriches ContextSection with scopeType/inclusionReason/evidence, surfaces requestFocus in ContextInspection, and updates TerminalRepl to render a "Request Focus" block alongside enriched section details. Changes
Sequence DiagramsequenceDiagram
participant Client as Client/CLI
participant Handler as StreamingAgentHandler
participant Loader as SystemPromptLoader
participant Helper as ContextRpcHelper
participant CLI as TerminalRepl
Client->>Handler: inspectContext(sessionId, queryHint)
Handler->>Loader: analyzeRequestFocus(queryHint, activeFilePaths)
Loader->>Loader: extractActiveSymbols()<br/>inferPlanSignals()<br/>classifySectionScope()
Loader-->>Handler: ContextInspection(requestFocus, sections, prompt)
Handler->>Helper: buildContextResult(ContextInspection)
Helper->>Helper: sectionToNode() — add scopeType, inclusionReason, evidence
Helper-->>Handler: enriched JSON result
Handler-->>Client: ContextInspection result
Client->>CLI: render(contextInspection)
CLI->>CLI: renderRequestFocus()<br/>renderContextList()/renderContextDetail()
CLI-->>Client: formatted terminal output
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
aceclaw-daemon/src/main/java/dev/aceclaw/daemon/StreamingAgentHandler.java (1)
1739-1749:⚠️ Potential issue | 🟡 MinorDon't throw away request focus in the no-project fallback.
This path still has
queryHint, but it returnsRequestFocus.empty()./context.inspectwill therefore miss the new query-summary / symbol / plan metadata exactly when a session has no workspace. Extract the focus analysis into a reusable helper and populate it here too.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@aceclaw-daemon/src/main/java/dev/aceclaw/daemon/StreamingAgentHandler.java` around lines 1739 - 1749, The no-project fallback in StreamingAgentHandler currently returns a ContextInspection with RequestFocus.empty(), discarding queryHint-derived focus metadata; extract the focus-analysis logic (the code that builds RequestFocus from queryHint / querySummary / symbol / plan) into a reusable helper method (e.g., buildRequestFocus(session or queryHint) or computeRequestFocusForSession(session)) and call that helper both in the normal path and in the no-project fallback before constructing SystemPromptLoader.ContextInspection; ensure the helper returns the same RequestFocus type used elsewhere (not RequestFocus.empty()) and is invoked alongside getSystemPrompt(sessionId) so the returned ContextInspection contains populated focus metadata even when session.projectPath() == null.aceclaw-daemon/src/main/java/dev/aceclaw/daemon/SystemPromptLoader.java (1)
256-268:⚠️ Potential issue | 🟠 MajorRequest-focus metadata isn't wired into the actual selection policy.
requestFocusis created here, but the newactiveSymbolsandplanSignalsnever feed any section selection or ranking call. Injection still runs off rawqueryHint/activeFilePaths, so the active-symbol / active-plan prioritization goal is not implemented yet.Also applies to: 270-272, 295-298
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@aceclaw-daemon/src/main/java/dev/aceclaw/daemon/SystemPromptLoader.java` around lines 256 - 268, The requestFocus produced by analyzeRequestFocus(queryHint, activeFilePaths) contains activeSymbols and planSignals but is not used to influence which sections are included or their priorities; update the tierSections loop and subsequent plan.addSection calls (ContextAssemblyPlan.addSection usage) to consult requestFocus (e.g., requestFocus.activeSymbols and requestFocus.planSignals) and alter selection and weights accordingly: when a section references an active symbol boost its priority (or include it even if below threshold), and when planSignals indicate a particular tier/section type boost or force inclusion (e.g., for base, task-focus and other sections added later like the blocks at the later calls), so that injection and ranking use the requestFocus metadata rather than only queryHint/activeFilePaths.
🧹 Nitpick comments (1)
aceclaw-cli/src/test/java/dev/aceclaw/cli/TerminalReplTest.java (1)
305-309: Add an explicit assertion for request-focus file rendering.Please also assert
"Files:"here. Right nowsrc/main/App.javacan be satisfied by the separate “Active paths” block, so request-focus file coverage can be bypassed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@aceclaw-cli/src/test/java/dev/aceclaw/cli/TerminalReplTest.java` around lines 305 - 309, The test TerminalReplTest currently asserts output contains "Request Focus", "Query:", "Symbols:", "Plan:", and "Last compact:" but misses asserting the "Files:" header; update the test (in TerminalReplTest where the variable plain is asserted) to include an explicit assertion that plain contains "Files:" so the request-focus file rendering is verified (add an assertThat(plain).contains("Files:") alongside the other contains checks).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@aceclaw-daemon/src/main/java/dev/aceclaw/daemon/SystemPromptLoader.java`:
- Around line 255-256: The system is marking "memory:Auto-Memory" as always-on
even though its inclusion-reason indicates it is query-aware; update the logic
that builds scope metadata so that when tierResult (or the memory tier
identifier) equals "memory:Auto-Memory" you do not set the scope to always-on
but instead mark it as query-aware (aligning with the inclusion-reason text).
Locate where tierResult is processed and scope/always-on flags are assigned
(near the call sites around analyzeRequestFocus and the requestFocus usage) and
change that branch so Auto-Memory uses the query-aware scope metadata; apply the
same fix to the other similar handling sites that produce the same
misclassification.
---
Outside diff comments:
In `@aceclaw-daemon/src/main/java/dev/aceclaw/daemon/StreamingAgentHandler.java`:
- Around line 1739-1749: The no-project fallback in StreamingAgentHandler
currently returns a ContextInspection with RequestFocus.empty(), discarding
queryHint-derived focus metadata; extract the focus-analysis logic (the code
that builds RequestFocus from queryHint / querySummary / symbol / plan) into a
reusable helper method (e.g., buildRequestFocus(session or queryHint) or
computeRequestFocusForSession(session)) and call that helper both in the normal
path and in the no-project fallback before constructing
SystemPromptLoader.ContextInspection; ensure the helper returns the same
RequestFocus type used elsewhere (not RequestFocus.empty()) and is invoked
alongside getSystemPrompt(sessionId) so the returned ContextInspection contains
populated focus metadata even when session.projectPath() == null.
In `@aceclaw-daemon/src/main/java/dev/aceclaw/daemon/SystemPromptLoader.java`:
- Around line 256-268: The requestFocus produced by
analyzeRequestFocus(queryHint, activeFilePaths) contains activeSymbols and
planSignals but is not used to influence which sections are included or their
priorities; update the tierSections loop and subsequent plan.addSection calls
(ContextAssemblyPlan.addSection usage) to consult requestFocus (e.g.,
requestFocus.activeSymbols and requestFocus.planSignals) and alter selection and
weights accordingly: when a section references an active symbol boost its
priority (or include it even if below threshold), and when planSignals indicate
a particular tier/section type boost or force inclusion (e.g., for base,
task-focus and other sections added later like the blocks at the later calls),
so that injection and ranking use the requestFocus metadata rather than only
queryHint/activeFilePaths.
---
Nitpick comments:
In `@aceclaw-cli/src/test/java/dev/aceclaw/cli/TerminalReplTest.java`:
- Around line 305-309: The test TerminalReplTest currently asserts output
contains "Request Focus", "Query:", "Symbols:", "Plan:", and "Last compact:" but
misses asserting the "Files:" header; update the test (in TerminalReplTest where
the variable plain is asserted) to include an explicit assertion that plain
contains "Files:" so the request-focus file rendering is verified (add an
assertThat(plain).contains("Files:") alongside the other contains checks).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ee4fccef-aa33-4bd4-99b3-8b01b1cd6b5a
📒 Files selected for processing (6)
aceclaw-cli/src/main/java/dev/aceclaw/cli/TerminalRepl.javaaceclaw-cli/src/test/java/dev/aceclaw/cli/TerminalReplTest.javaaceclaw-daemon/src/main/java/dev/aceclaw/daemon/ContextRpcHelper.javaaceclaw-daemon/src/main/java/dev/aceclaw/daemon/StreamingAgentHandler.javaaceclaw-daemon/src/main/java/dev/aceclaw/daemon/SystemPromptLoader.javaaceclaw-daemon/src/test/java/dev/aceclaw/daemon/SystemPromptLoaderTest.java
added 2 commits
March 17, 2026 00:25
- Cache getSystemPrompt() in local var to avoid triple invocation - Accept dotted qualified names in looksLikeSymbol (e.g. AppService.validate) - Narrow inferPlanSignals "steps" match to "next steps" / "steps to" - Cap request focus priority boost at 12 to prevent tier inversions - Remove dead null checks in renderRequestFocus (JsonNode.path never null) - Extract sectionToNode() helper in ContextRpcHelper to deduplicate fields - Replace inline FQN java.util.LinkedHashSet with proper import - Remove unnecessary distinct() in inferPlanSignals
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
Testing
Closes #268
Summary by CodeRabbit
New Features
Bug Fixes / UX
Tests