Repository navigation
feat(memory): align semantic and KB recall - #656
Conversation
Use the current runtime prompt for both semantic memory and KB retrieval, add an optional KB defer gate when semantic memory has grounded the same query, and lazily derive durable topic range indexes for semantic recall. Verification: uv run pytest tests/coding_agent/plugins/test_kb_plugin.py tests/coding_agent/plugins/test_semantic_memory.py tests/coding_agent/test_context_system_smoke.py tests/coding_agent/test_bootstrap.py tests/deploy/test_helm_chart.py -v Verification: uv run ruff check/format --check on touched files
|
Warning Review limit reached
Next review available in: 44 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: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughIntroduces a ChangesSemantic memory grounding marker and KB deferral
Sequence Diagram(s)sequenceDiagram
participant Pipeline
participant SemanticMemoryPlugin
participant ctx.config
participant KBPlugin
participant KB
Pipeline->>SemanticMemoryPlugin: build_context(ctx, tape)
SemanticMemoryPlugin->>ctx.config: clear SEMANTIC_MEMORY_GROUNDING_MARKER_KEY
SemanticMemoryPlugin->>SemanticMemoryPlugin: plan recall (lazy topic index)
SemanticMemoryPlugin->>ctx.config: set marker {query_digest, tape_len, hit_count}
Pipeline->>KBPlugin: build_context(ctx, tape)
KBPlugin->>KBPlugin: extract query from runtime messages
KBPlugin->>ctx.config: pop SEMANTIC_MEMORY_GROUNDING_MARKER_KEY
alt hit_count > 0 and defer_when_semantic_memory_hits
KBPlugin-->>Pipeline: return [] (skipped)
else
KBPlugin->>KB: search(query)
KB-->>KBPlugin: results
KBPlugin-->>Pipeline: context pack
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ 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 |
![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
🧹 Nitpick comments (2)
coding-agent/src/coding_agent/plugins/semantic_memory.py (1)
71-84: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSerialize lazy topic-index initialization.
_derived_topic_indexis filled only after the await, so concurrent cold-startbuild_context()calls on the same plugin instance can each rebuild the full store-backed index before the cache is populated.♻️ Proposed fix
+import asyncio import hashlib import inspect from collections.abc import Callable, Mapping from datetime import UTC, datetime from typing import Any @@ self._read_enabled = read_enabled self._limit = limit self._topic_store = _validate_topic_store(topic_store) self._topic_index = _validate_topic_index(topic_index) self._derived_topic_index: TopicRangeIndex | None = None + self._derived_topic_index_lock = asyncio.Lock() @@ async def _topic_index_for_context(self) -> TopicRangeIndex: if self._topic_index is not None: return self._topic_index - if self._derived_topic_index is None: - derived = await build_topic_range_index_from_store(self._topic_store) - self._derived_topic_index = ( - TopicRangeIndex() if derived is None else derived.index - ) + if self._derived_topic_index is not None: + return self._derived_topic_index + async with self._derived_topic_index_lock: + if self._derived_topic_index is None: + derived = await build_topic_range_index_from_store(self._topic_store) + self._derived_topic_index = ( + TopicRangeIndex() if derived is None else derived.index + ) return self._derived_topic_index🤖 Prompt for 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. In `@coding-agent/src/coding_agent/plugins/semantic_memory.py` around lines 71 - 84, The lazy initialization in SemanticMemoryPlugin._topic_index_for_context is not serialized, so concurrent cold-start build_context calls can all rebuild the store-backed index before _derived_topic_index is set. Add an async guard around the initialization path in _topic_index_for_context (using the SemanticMemoryPlugin instance state) so only one build_topic_range_index_from_store call runs and the rest await the cached result. Keep the fast path for already-populated _topic_index and _derived_topic_index unchanged.coding-agent/src/coding_agent/plugins/kb.py (1)
30-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract runtime-query selection into a shared helper.
KBPluginandSemanticMemoryPluginnow duplicate the same runtime-message filtering and payload-text extraction. Since the deferral contract depends on both plugins hashing/searching the exact same query, any drift here will silently reintroduce stale-query mismatches.Also applies to: 141-143, 251-270
🤖 Prompt for 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. In `@coding-agent/src/coding_agent/plugins/kb.py` around lines 30 - 35, The runtime-query selection and text-extraction logic is duplicated between KBPlugin and SemanticMemoryPlugin, so move the shared filtering and payload-to-query conversion into a single helper and have both plugins call it. Use the existing KBPlugin runtime message handling, SemanticMemoryPlugin search/hash path, and the _RUNTIME_QUERY_KINDS set as the anchor points, and ensure both deferral and search code paths use the exact same helper so the hashed query and searched query always stay aligned.
🤖 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 `@coding-agent/tests/coding_agent/test_context_system_smoke.py`:
- Around line 36-41: Add a list_topics() method to FakeTopicStore so it matches
the topic_store contract expected by SemanticMemoryPlugin when it lazily builds
its TopicRangeIndex. Update the fake in test_context_system_smoke.py to mirror
the semantic-memory test fake: keep load_topic() behavior and also expose
list_topics() returning the stored TopicRecord values so build_context() can
construct the index without failing before the assertion.
---
Nitpick comments:
In `@coding-agent/src/coding_agent/plugins/kb.py`:
- Around line 30-35: The runtime-query selection and text-extraction logic is
duplicated between KBPlugin and SemanticMemoryPlugin, so move the shared
filtering and payload-to-query conversion into a single helper and have both
plugins call it. Use the existing KBPlugin runtime message handling,
SemanticMemoryPlugin search/hash path, and the _RUNTIME_QUERY_KINDS set as the
anchor points, and ensure both deferral and search code paths use the exact same
helper so the hashed query and searched query always stay aligned.
In `@coding-agent/src/coding_agent/plugins/semantic_memory.py`:
- Around line 71-84: The lazy initialization in
SemanticMemoryPlugin._topic_index_for_context is not serialized, so concurrent
cold-start build_context calls can all rebuild the store-backed index before
_derived_topic_index is set. Add an async guard around the initialization path
in _topic_index_for_context (using the SemanticMemoryPlugin instance state) so
only one build_topic_range_index_from_store call runs and the rest await the
cached result. Keep the fast path for already-populated _topic_index and
_derived_topic_index unchanged.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6b9117dc-67b3-49a6-860d-e58e5a28305a
📒 Files selected for processing (11)
coding-agent/helm/templates/configmap-agent-config.yamlcoding-agent/helm/values.yamlcoding-agent/src/coding_agent/core/app.pycoding-agent/src/coding_agent/plugins/kb.pycoding-agent/src/coding_agent/plugins/semantic_memory.pycoding-agent/src/coding_agent/topics/range_index_builder.pycoding-agent/tests/coding_agent/plugins/test_kb_plugin.pycoding-agent/tests/coding_agent/plugins/test_semantic_memory.pycoding-agent/tests/coding_agent/test_bootstrap.pycoding-agent/tests/coding_agent/test_context_system_smoke.pycoding-agent/tests/deploy/test_helm_chart.py
Add list_topics to the smoke FakeTopicStore so the test double matches the lazy semantic topic-index contract. Verification: uv run pytest tests/coding_agent/test_context_system_smoke.py tests/coding_agent/plugins/test_semantic_memory.py tests/coding_agent/plugins/test_kb_plugin.py -v Verification: uv run ruff check tests/coding_agent/test_context_system_smoke.py && uv run ruff format --check tests/coding_agent/test_context_system_smoke.py
Summary
kb.deferWhenSemanticMemoryHitsin Helm and fail fast when custom plugin ordering would put KB before semantic memoryVerification
uv run pytest tests/coding_agent/plugins/test_kb_plugin.py tests/coding_agent/plugins/test_semantic_memory.py tests/coding_agent/test_context_system_smoke.py tests/coding_agent/test_bootstrap.py tests/deploy/test_helm_chart.py -vuv run ruff check src/coding_agent/topics/range_index_builder.py src/coding_agent/plugins/semantic_memory.py src/coding_agent/plugins/kb.py src/coding_agent/core/app.py tests/coding_agent/plugins/test_semantic_memory.py tests/coding_agent/plugins/test_kb_plugin.py tests/coding_agent/test_context_system_smoke.py tests/coding_agent/test_bootstrap.py tests/deploy/test_helm_chart.pyuv run ruff format --check src/coding_agent/topics/range_index_builder.py src/coding_agent/plugins/semantic_memory.py src/coding_agent/plugins/kb.py src/coding_agent/core/app.py tests/coding_agent/plugins/test_semantic_memory.py tests/coding_agent/plugins/test_kb_plugin.py tests/coding_agent/test_context_system_smoke.py tests/coding_agent/test_bootstrap.py tests/deploy/test_helm_chart.pyNo P1/P2after fixesNotes
Summary by CodeRabbit
New Features
Bug Fixes