feat: hybrid search (dense + BM25 sparse) for memory retrieval pipeline#1016
feat: hybrid search (dense + BM25 sparse) for memory retrieval pipeline#1016
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
🧰 Additional context used📓 Path-based instructions (4)**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/synthorg/**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
⚙️ CodeRabbit configuration file
Files:
src/synthorg/memory/**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (29)📓 Common learnings📚 Learning: 2026-03-19T07:12:14.508ZApplied to files:
📚 Learning: 2026-04-03T11:16:15.899ZApplied to files:
📚 Learning: 2026-03-16T06:24:56.341ZApplied to files:
📚 Learning: 2026-03-15T16:55:07.730ZApplied to files:
📚 Learning: 2026-04-03T11:16:15.899ZApplied to files:
📚 Learning: 2026-03-15T18:38:44.202ZApplied to files:
📚 Learning: 2026-03-17T22:08:13.456ZApplied to files:
📚 Learning: 2026-03-31T20:07:03.035ZApplied to files:
📚 Learning: 2026-03-14T16:18:57.267ZApplied to files:
📚 Learning: 2026-03-16T07:22:28.134ZApplied to files:
📚 Learning: 2026-03-17T06:43:14.114ZApplied to files:
📚 Learning: 2026-03-17T22:08:13.456ZApplied to files:
📚 Learning: 2026-04-03T11:16:15.899ZApplied to files:
📚 Learning: 2026-04-01T09:39:21.584ZApplied to files:
📚 Learning: 2026-03-20T11:18:48.128ZApplied to files:
📚 Learning: 2026-03-15T18:28:13.207ZApplied to files:
📚 Learning: 2026-03-19T11:33:01.580ZApplied to files:
📚 Learning: 2026-03-31T21:07:37.470ZApplied to files:
📚 Learning: 2026-03-17T06:43:14.114ZApplied to files:
📚 Learning: 2026-03-15T18:38:44.202ZApplied to files:
📚 Learning: 2026-03-15T19:14:27.144ZApplied to files:
📚 Learning: 2026-04-03T11:16:15.899ZApplied to files:
📚 Learning: 2026-03-15T19:14:27.144ZApplied to files:
📚 Learning: 2026-03-20T21:44:04.528ZApplied to files:
📚 Learning: 2026-03-15T18:38:44.202ZApplied to files:
📚 Learning: 2026-03-15T19:14:27.144ZApplied to files:
📚 Learning: 2026-03-17T22:08:13.456ZApplied to files:
📚 Learning: 2026-03-15T19:14:27.144ZApplied to files:
🔇 Additional comments (17)
WalkthroughAdds hybrid memory retrieval combining dense vectors with BM25 sparse vectors fused via Reciprocal Rank Fusion (RRF). Implements a hash-based BM25 tokenizer and immutable Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Dependency ReviewThe following issues were found:
Snapshot WarningsEnsure that dependencies are being submitted on PR branches. Re-running this action after a short time may resolve the issue. See the documentation for more information and troubleshooting advice. License Issuesuv.lock
OpenSSF Scorecard
Scanned Files
|
There was a problem hiding this comment.
Code Review
This pull request introduces hybrid search capabilities to the memory retrieval pipeline by integrating BM25 sparse vector search alongside existing dense vector search. Key changes include adding mmh3 for sparse vector encoding, implementing a ToolBasedInjectionStrategy for on-demand memory retrieval, and adding agentic query reformulation capabilities. The review feedback highlighted potential issues with exception handling, specifically the use of builtins_MemoryError (which appears to be a typo) and the practice of catching system-level exceptions like MemoryError and RecursionError in multiple locations.
src/synthorg/memory/retriever.py
Outdated
| async with asyncio.TaskGroup() as tg: | ||
| dense_task = tg.create_task(dense_coro) | ||
| sparse_task = tg.create_task(sparse_coro) | ||
| except* builtins_MemoryError as eg: |
There was a problem hiding this comment.
The variable builtins_MemoryError appears to be a typo for MemoryError. Please ensure the correct exception type is being caught.
except* MemoryError as eg:
src/synthorg/memory/retriever.py
Outdated
| async with asyncio.TaskGroup() as tg: | ||
| personal_task = tg.create_task(personal_coro) | ||
| shared_task = tg.create_task(shared_coro) | ||
| except* builtins_MemoryError as eg: |
There was a problem hiding this comment.
The variable builtins_MemoryError appears to be a typo for MemoryError. Please ensure the correct exception type is being caught.
except* MemoryError as eg:| qdrant, | ||
| self._mem0_config.collection_name, | ||
| ) | ||
| except builtins.MemoryError, RecursionError: |
There was a problem hiding this comment.
Catching builtins.MemoryError and RecursionError is generally discouraged as these are system-level exceptions that should typically propagate to allow the application to crash or recover gracefully at a higher level. Consider removing these from the catch block unless there is a specific requirement to handle them.
| str(memory_id), | ||
| sparse_vec, | ||
| ) | ||
| except builtins.MemoryError, RecursionError: |
There was a problem hiding this comment.
Similar to the previous comment, catching builtins.MemoryError and RecursionError here is likely unnecessary and could mask critical system failures. Please review if these need to be caught explicitly.
| ) | ||
| entries = scored_points_to_entries(raw_points, agent_id) | ||
| return apply_post_filters(entries, query) | ||
| except builtins.MemoryError, RecursionError: |
There was a problem hiding this comment.
Catching builtins.MemoryError and RecursionError here should be reconsidered. These exceptions usually indicate that the process is in an unstable state.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1016 +/- ##
==========================================
- Coverage 91.79% 91.73% -0.06%
==========================================
Files 678 682 +4
Lines 37628 37912 +284
Branches 3744 3768 +24
==========================================
+ Hits 34539 34780 +241
- Misses 2446 2484 +38
- Partials 643 648 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds hybrid retrieval capabilities to the memory subsystem by introducing BM25-style sparse vectors (Qdrant sparse field), wiring optional sparse search into the Mem0 backend, and enabling RRF fusion in the context-injection retriever. Also introduces a tool-based injection strategy plus query-reformulation building blocks and updates docs/tests accordingly.
Changes:
- Implement BM25 sparse encoding and Qdrant sparse-vector lifecycle (ensure field, upsert, query) with Mem0 backend integration behind a config flag.
- Add RRF fusion path to the memory retriever to combine dense + sparse results (parallel fetch + fusion + post-filter).
- Introduce tool-based memory injection strategy and query reformulation protocols/LLM wrappers; expand unit test coverage and update docs.
Reviewed changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Locks new dependency mmh3 and its wheels/sdist. |
| pyproject.toml | Adds pinned mmh3==5.2.1 dependency. |
| CLAUDE.md | Updates repository/component inventory and required deps list. |
| docs/design/memory.md | Documents hybrid/RRF pipeline and tool-based retrieval behavior. |
| src/synthorg/observability/events/memory.py | Adds sparse-search and reformulation event constants. |
| src/synthorg/memory/tool_retriever.py | New ToolBasedInjectionStrategy exposing search_memory/recall_memory tools. |
| src/synthorg/memory/sparse.py | New BM25 tokenizer + validated SparseVector model. |
| src/synthorg/memory/retriever.py | Adds RRF execution path and sparse fetching for hybrid retrieval. |
| src/synthorg/memory/retrieval_config.py | Enables FusionStrategy.RRF, InjectionStrategy.TOOL_BASED, and adds reformulation config fields. |
| src/synthorg/memory/reformulation.py | Adds protocols + LLM implementations for query reformulation and sufficiency checking. |
| src/synthorg/memory/injection.py | Updates docs/enums commentary to reflect TOOL_BASED being implemented. |
| src/synthorg/memory/backends/mem0/sparse_search.py | New Qdrant sparse utilities (ensure field, upsert vector, sparse query, mapping). |
| src/synthorg/memory/backends/mem0/config.py | Adds sparse_search_enabled toggle to Mem0 backend config. |
| src/synthorg/memory/backends/mem0/adapter.py | Wires sparse field creation on connect, sparse upsert on store, and retrieve_sparse(). |
| src/synthorg/memory/init.py | Exports new sparse/tool/reformulation symbols. |
| tests/unit/memory/test_tool_retriever.py | Unit tests for tool-based strategy schemas/dispatch/error handling. |
| tests/unit/memory/test_sparse.py | Unit + Hypothesis tests for tokenizer and SparseVector validation. |
| tests/unit/memory/test_retriever.py | Adds hybrid/RRF pipeline tests (merge, fallback, degrade, post-filter, dedupe). |
| tests/unit/memory/test_retrieval_config.py | Updates validation tests to accept TOOL_BASED and RRF. |
| tests/unit/memory/test_reformulation.py | Unit tests for reformulator/checker protocols and LLM wrappers. |
| tests/unit/memory/test_init.py | Extends __all__ export expectations. |
| tests/unit/memory/backends/mem0/test_sparse_search.py | Tests sparse Qdrant ops + Mem0 sparse integration behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
docs/design/memory.md
Outdated
| - Exposes `search_memory` (semantic + hybrid search) and `recall_memory` (by ID) tools | ||
| - Handles retrieval internally (dense + optional sparse with RRF fusion) | ||
| - Supports optional agentic query reformulation via `QueryReformulator` and | ||
| `SufficiencyChecker` protocols (LLM-powered, disabled by default) |
There was a problem hiding this comment.
Tool-based retrieval is documented here as doing hybrid search (dense + optional sparse with RRF) and optional query reformulation, but ToolBasedInjectionStrategy currently only calls backend.retrieve(...) and has no sparse/RRF or reformulation loop. Please either update this section to match the current implementation, or wire the documented behavior into the tool-based strategy (and tests).
| - Exposes `search_memory` (semantic + hybrid search) and `recall_memory` (by ID) tools | |
| - Handles retrieval internally (dense + optional sparse with RRF fusion) | |
| - Supports optional agentic query reformulation via `QueryReformulator` and | |
| `SufficiencyChecker` protocols (LLM-powered, disabled by default) | |
| - Exposes `search_memory` and `recall_memory` (by ID) tools | |
| - Delegates `search_memory` requests to `MemoryBackend.retrieve(...)` | |
| - Relies on the configured backend for retrieval semantics; hybrid dense+sparse search, | |
| RRF fusion, and agentic query reformulation are not currently implemented in | |
| `ToolBasedInjectionStrategy` itself |
| "Enable agentic query reformulation for TOOL_BASED " | ||
| "strategy. When True, the tool-based strategy iteratively " | ||
| "rewrites queries using an LLM for better retrieval." | ||
| ), | ||
| ) | ||
| max_reformulation_rounds: int = Field( | ||
| default=2, | ||
| ge=1, | ||
| le=5, | ||
| description="Maximum query reformulation iterations (1-5).", | ||
| ) | ||
|
|
||
| @model_validator(mode="after") |
There was a problem hiding this comment.
query_reformulation_enabled/max_reformulation_rounds are described as enabling an LLM-driven reformulation loop in the TOOL_BASED strategy, but there is no code path that reads these fields (only this config model defines/validates them). Either wire these flags into ToolBasedInjectionStrategy (or another orchestrator), or adjust the field descriptions/validation so users aren’t misled into thinking the option is active.
| "Enable agentic query reformulation for TOOL_BASED " | |
| "strategy. When True, the tool-based strategy iteratively " | |
| "rewrites queries using an LLM for better retrieval." | |
| ), | |
| ) | |
| max_reformulation_rounds: int = Field( | |
| default=2, | |
| ge=1, | |
| le=5, | |
| description="Maximum query reformulation iterations (1-5).", | |
| ) | |
| @model_validator(mode="after") | |
| "Reserved for future query reformulation support in the " | |
| "TOOL_BASED strategy. This option is not currently wired into " | |
| "the retrieval pipeline and must remain False." | |
| ), | |
| ) | |
| max_reformulation_rounds: int = Field( | |
| default=2, | |
| ge=1, | |
| le=5, | |
| description=( | |
| "Reserved for future query reformulation support (1-5). " | |
| "Currently unused unless reformulation support is implemented." | |
| ), | |
| ) | |
| @model_validator(mode="after") | |
| def _validate_query_reformulation_support(self) -> Self: | |
| """Reject unsupported query reformulation settings.""" | |
| if not self.query_reformulation_enabled: | |
| return self | |
| msg = ( | |
| "query_reformulation_enabled is not currently supported: " | |
| "the retrieval pipeline does not yet consume this option." | |
| ) | |
| logger.warning( | |
| CONFIG_VALIDATION_FAILED, | |
| field="query_reformulation_enabled", | |
| value=self.query_reformulation_enabled, | |
| reason=msg, | |
| ) | |
| raise ValueError(msg) | |
| @model_validator(mode="after") |
| limit_raw = arguments.get("limit", 10) | ||
| limit = int(limit_raw) if isinstance(limit_raw, int | float) else 10 | ||
| categories = _parse_categories(arguments.get("categories")) | ||
|
|
||
| logger.info( | ||
| MEMORY_RETRIEVAL_START, | ||
| agent_id=agent_id, | ||
| tool=_SEARCH_TOOL, | ||
| query_length=len(query_text), | ||
| ) | ||
|
|
||
| try: | ||
| query = MemoryQuery( | ||
| text=query_text, | ||
| limit=min(max(limit, 1), 50), | ||
| categories=categories, | ||
| ) |
There was a problem hiding this comment.
ToolBasedInjectionStrategy receives a MemoryRetrievalConfig but _handle_search doesn’t use it to enforce system-side limits (e.g., config.max_memories / min_relevance) and instead trusts the tool argument (capped at 50). Consider capping limit by config.max_memories (and optionally applying min_relevance filtering) so the LLM can’t exceed configured retrieval policy.
| # Expose the Qdrant client for sparse vector operations. | ||
| if self._mem0_config.sparse_search_enabled: | ||
| try: | ||
| qdrant = client.vector_store.client # pyright: ignore[reportAttributeAccessIssue] | ||
| self._qdrant_client = qdrant | ||
| await asyncio.to_thread( | ||
| ensure_sparse_field, | ||
| qdrant, | ||
| self._mem0_config.collection_name, | ||
| ) | ||
| except builtins.MemoryError, RecursionError: | ||
| raise | ||
| except Exception as exc: | ||
| logger.warning( | ||
| MEMORY_SPARSE_UPSERT_FAILED, | ||
| operation="ensure_sparse_field", | ||
| error=str(exc), | ||
| error_type=type(exc).__name__, | ||
| ) |
There was a problem hiding this comment.
In connect(), failures while ensuring the sparse field are logged under MEMORY_SPARSE_UPSERT_FAILED, which makes it hard to distinguish schema/collection update failures from per-point upsert failures. Consider introducing/using a dedicated event (e.g., MEMORY_SPARSE_FIELD_ENSURE_FAILED) or at least a more specific event name for this operation.
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/memory/backends/mem0/adapter.py`:
- Around line 201-202: Replace the incorrect exception clause `except
builtins.MemoryError, RecursionError:` with the parenthesized tuple form `except
(builtins.MemoryError, RecursionError):` in adapter.py (the same pattern used
elsewhere in this file); update the exception handler in the same
function/method where that clause appears so it catches both MemoryError and
RecursionError without binding one as a variable.
In `@src/synthorg/memory/backends/mem0/config.py`:
- Around line 193-200: The factory build_config_from_company_config constructs
Mem0BackendConfig but omits the sparse_search_enabled flag, so company-level
enabling is ignored; update build_config_from_company_config to read the company
config's sparse_search_enabled (or equivalent field) and pass it into the
Mem0BackendConfig initializer along with data_dir and embedder (preserve default
behavior when the company value is absent), ensuring
Mem0BackendConfig.sparse_search_enabled is threaded through callers that use
this factory.
In `@src/synthorg/memory/backends/mem0/sparse_search.py`:
- Around line 252-261: The code currently calls
datetime.fromisoformat(created_str) without handling malformed input; update the
block around created_str (used with payload, created_at, and
logger/MEMORY_SPARSE_SEARCH_FAILED) to catch ValueError from
datetime.fromisoformat, log a clear diagnostic (include point id via
getattr(point, "id", "unknown") and the bad created_str) and then fall back to
datetime(1970, 1, 1, tzinfo=UTC) as the sentinel; ensure you only catch
ValueError for parsing errors and preserve the existing missing-created_at
branch behavior.
In `@src/synthorg/memory/retriever.py`:
- Around line 421-435: The early return when self._backend lacks
supports_sparse_search should be removed so shared sparse retrieval still runs;
instead, if getattr(self._backend, "supports_sparse_search", False) is False
then set personal_coro to a no-op/empty-result awaitable (so it yields empty
results) rather than returning, then proceed to evaluate self._shared_store and,
if self._config.include_shared and shared_store and getattr(shared_store,
"supports_sparse_search", False), create the shared coroutine via
shared_store.retrieve_sparse wrapped with _safe_call; ensure you still use the
same symbols (personal_coro, _safe_call, retrieve_sparse,
shared_store/_shared_store, _config.include_shared) so downstream code can await
both coroutines and combine hybrid results.
- Around line 385-395: The current concatenation (dense_personal + dense_shared
and sparse_personal + sparse_shared) biases RRF by position and
_fetch_sparse_memories() wrongly gates shared sparse on personal capability;
change the logic so that before calling fuse_ranked_lists you merge/score-order
each modality across personal+shared (e.g., produce dense_list and sparse_list
sorted by relevance score rather than simple concatenation), decouple shared
sparse retrieval from personal backend capability inside _fetch_sparse_memories
(always attempt shared sparse if the shared store supports it), and if sparse
results are truly unavailable route to the linear-scored pipeline instead of
running RRF on a single list (use _config.rrf_k and _config.max_memories as
before when RRF is valid).
In `@src/synthorg/memory/sparse.py`:
- Around line 162-168: Replace the simple ASCII lowercasing with Unicode-aware
normalization and casefolding before token-splitting: normalize the input text
(e.g. using unicodedata.normalize with NFC or NFKC) and then call casefold()
instead of lower() when producing the string passed to _TOKEN_SPLIT_RE.split;
keep the empty/whitespace check, preserve the subsequent stop-word filtering
against _STOP_WORDS, and add an import for unicodedata if needed so tokens and
comparisons (used by tokens, _TOKEN_SPLIT_RE, and _STOP_WORDS) will match
semantically equivalent non-ASCII forms.
In `@tests/unit/memory/backends/mem0/test_sparse_search.py`:
- Around line 146-180: The tests currently only assert that client.query_points
was called and limit forwarded; update them to assert the actual payload
includes the sparse vector and the user-id filter: after calling search_sparse,
grab call_kwargs = client.query_points.call_args.kwargs and assert that the
sparse-vector argument passed (e.g., call_kwargs["sparse_vector"] or whatever kw
name search_sparse uses to forward query_vec) equals the query_vec used in the
test, and also assert the user id filter is forwarded (e.g.,
call_kwargs["user_id_filter"] == "agent-1" or that the filter structure in
call_kwargs contains "agent-1"). Ensure you reference the search_sparse
function, client.query_points, query_vec, and user_id_filter when adding these
assertions.
In `@tests/unit/memory/test_reformulation.py`:
- Around line 84-89: Add tests that assert MemoryError and RecursionError are
not swallowed by LLMQueryReformulator: create AsyncMock side_effects that raise
MemoryError and RecursionError and verify they propagate using pytest.raises for
both reformulate() and check_sufficiency(); mirror the existing
test_error_returns_none structure (use LLMQueryReformulator, AsyncMock, and
await the called coroutine) so you cover the two passthrough branches in
reformulate() and check_sufficiency() instead of only RuntimeError.
In `@tests/unit/memory/test_retrieval_config.py`:
- Around line 170-177: The test test_rrf_skips_weight_sum_validation currently
only checks that MemoryRetrievalConfig accepted fusion_strategy RRF; update it
to also assert that the instance's relevance_weight and recency_weight
properties preserve the provided values (0.5 and 0.3) so a validator that
silently resets them would fail; locate the MemoryRetrievalConfig instance
created in test_rrf_skips_weight_sum_validation and add assertions like
c.relevance_weight == 0.5 and c.recency_weight == 0.3 to validate stored values
remain unchanged.
In `@tests/unit/memory/test_retriever.py`:
- Around line 693-727: The test test_rrf_min_relevance_after_fusion currently
only checks that one message object is returned, not that the low-scoring entry
was filtered; update the assertions after calling
ContextInjectionStrategy.prepare_messages to explicitly verify the low-scoring
entry was excluded (e.g., assert "low score" not in content) and that the
high-scoring entry is present (assert "high score" in content) so the post-RRF
min_relevance filter on MemoryRetrievalConfig/FusionStrategy.RRF is actually
validated.
- Around line 622-630: The mocks currently auto-create attributes so they don't
model "no sparse support"; update the dense-only helper (the one creating a
backend for dense-only tests, e.g., _make_backend) to explicitly set
backend.supports_sparse_search = False and remove or delattr(backend,
"retrieve_sparse") so the attribute isn't present, and update the sparse-capable
helper (e.g., _make_sparse_backend) to set backend.supports_sparse_search = True
and ensure backend.retrieve_sparse is an AsyncMock returning sparse_entries;
alternatively use AsyncMock(spec_set=YourBackendInterface) to constrain
attributes—make these changes for both helpers referenced (including
_make_sparse_backend and the dense-only helper used around lines 670-692).
In `@tests/unit/memory/test_sparse.py`:
- Around line 198-247: Remove the hard-coded `@settings`(max_examples=50)
decorators from the property tests so Hypothesis uses the repository profile;
specifically delete the `@settings`(...) annotations on
test_encoding_is_deterministic, test_non_empty_alpha_produces_non_empty_vector,
test_indices_always_sorted, test_values_always_positive and
test_indices_always_non_negative while keeping the `@given`(...) decorators and
the BM25Tokenizer.encode usage intact so example counts are driven by the
Hypothesis profile (ci/dev) instead of being pinned to 50.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: fa73ce05-106a-4740-bd89-b9d8a1f43fc5
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (21)
CLAUDE.mddocs/design/memory.mdpyproject.tomlsrc/synthorg/memory/__init__.pysrc/synthorg/memory/backends/mem0/adapter.pysrc/synthorg/memory/backends/mem0/config.pysrc/synthorg/memory/backends/mem0/sparse_search.pysrc/synthorg/memory/injection.pysrc/synthorg/memory/reformulation.pysrc/synthorg/memory/retrieval_config.pysrc/synthorg/memory/retriever.pysrc/synthorg/memory/sparse.pysrc/synthorg/memory/tool_retriever.pysrc/synthorg/observability/events/memory.pytests/unit/memory/backends/mem0/test_sparse_search.pytests/unit/memory/test_init.pytests/unit/memory/test_reformulation.pytests/unit/memory/test_retrieval_config.pytests/unit/memory/test_retriever.pytests/unit/memory/test_sparse.pytests/unit/memory/test_tool_retriever.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Agent
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Backend
- GitHub Check: Build Web
- GitHub Check: Build Sandbox
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (5)
pyproject.toml
📄 CodeRabbit inference engine (CLAUDE.md)
pyproject.toml: Python tests use asyncio_mode = 'auto' in pytest configuration; no manual@pytest.mark.asyncioneeded
Python tests have 30-second global timeout per test (configured in pyproject.toml); do not add per-file pytest.mark.timeout(30) markers; only non-default overrides like timeout(60) are allowed
All versions pinned with==in pyproject.toml; Groups: test (pytest + plugins, hypothesis), dev (includes test + ruff, mypy, pre-commit, commitizen, pip-audit)
Version sourcing for releases: pyproject.toml (tool.commitizen.version) and src/synthorg/init.py (version)
Files:
pyproject.toml
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotationsin Python code; Python 3.14 has PEP 649 native lazy annotations
Use PEP 758 except syntax: useexcept A, B:(no parentheses) in Python 3.14; ruff enforces this
All public functions in Python must have type hints; mypy strict mode enforced
Use Google-style docstrings on public classes and functions in Python; enforced by ruff D rules
Create new objects and never mutate existing ones; for non-Pydantic internal collections (registries, BaseTool), use copy.deepcopy() at construction + MappingProxyType wrapping for read-only enforcement
For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence)
Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves
Use Pydantic v2 (BaseModel, model_validator, computed_field, ConfigDict); use allow_inf_nan=False in all ConfigDict declarations to reject NaN/Inf in numeric fields at validation time
Use@computed_fieldfor derived values instead of storing + validating redundant fields in Pydantic models (e.g. TokenUsage.total_tokens)
Use NotBlankStr from core.types for all identifier/name fields in Python (including optional and tuple variants) instead of manual whitespace validators
Prefer asyncio.TaskGroup for fan-out/fan-in parallel operations in Python (e.g. multiple tool invocations, parallel agent calls); prefer structured concurrency over bare create_task
Python line length must not exceed 88 characters; enforced by ruff
Python functions must be under 50 lines; files must be under 800 lines
Handle errors explicitly in Python; never silently swallow exceptions
Validate at system boundaries in Python (user input, external APIs, config files)
Files:
src/synthorg/memory/backends/mem0/config.pysrc/synthorg/memory/injection.pytests/unit/memory/test_reformulation.pysrc/synthorg/memory/retrieval_config.pysrc/synthorg/memory/__init__.pytests/unit/memory/test_retrieval_config.pytests/unit/memory/test_tool_retriever.pytests/unit/memory/test_init.pysrc/synthorg/memory/sparse.pytests/unit/memory/test_sparse.pysrc/synthorg/memory/retriever.pytests/unit/memory/backends/mem0/test_sparse_search.pysrc/synthorg/memory/tool_retriever.pysrc/synthorg/memory/backends/mem0/adapter.pytests/unit/memory/test_retriever.pysrc/synthorg/memory/backends/mem0/sparse_search.pysrc/synthorg/memory/reformulation.pysrc/synthorg/observability/events/memory.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every Python module with business logic must have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging/logging.getLogger()/print()in Python application code; exceptions are observability/setup.py, observability/sinks.py, observability/syslog_handler.py, and observability/http_handler.py
Python logger variable name must always belogger(not_logger, notlog)
Use event name constants from domain-specific modules under synthorg.observability.events (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool); import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANT
Use structured logging with kwargs in Python: alwayslogger.info(EVENT, key=value)-- neverlogger.info('msg %s', val)
All error paths in Python must log at WARNING or ERROR with context before raising
All state transitions in Python must log at INFO level
Use DEBUG logging level in Python for object creation, internal flow, entry/exit of key functions
Pure data models, enums, and re-exports in Python do NOT need logging
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned Python code, docstrings, comments, tests, or config examples; use generic names: example-provider, example-large-001, example-medium-001, example-small-001, large/medium/small as aliases
Files:
src/synthorg/memory/backends/mem0/config.pysrc/synthorg/memory/injection.pysrc/synthorg/memory/retrieval_config.pysrc/synthorg/memory/__init__.pysrc/synthorg/memory/sparse.pysrc/synthorg/memory/retriever.pysrc/synthorg/memory/tool_retriever.pysrc/synthorg/memory/backends/mem0/adapter.pysrc/synthorg/memory/backends/mem0/sparse_search.pysrc/synthorg/memory/reformulation.pysrc/synthorg/observability/events/memory.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: All Python test files must use@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, or@pytest.mark.slowmarkers
Python tests must maintain 80% minimum code coverage (enforced in CI)
Prefer@pytest.mark.parametrizefor testing similar cases in Python
Use test-provider, test-small-001, etc. in Python tests instead of real vendor names
Property-based testing in Python uses Hypothesis (@given+@settings); profiles: ci (50 examples, default) and dev (1000 examples), controlled via HYPOTHESIS_PROFILE env var
Never skip, dismiss, or ignore flaky Python tests; always fix them fully and fundamentally; for timing-sensitive tests, mock time.monotonic() and asyncio.sleep() to make them deterministic instead of widening timing margins
For Python tasks that must block indefinitely until cancelled (e.g. simulating a slow provider or stubborn coroutine), use asyncio.Event().wait() instead of asyncio.sleep(large_number) -- it is cancellation-safe and carries no timing assumptions
Files:
tests/unit/memory/test_reformulation.pytests/unit/memory/test_retrieval_config.pytests/unit/memory/test_tool_retriever.pytests/unit/memory/test_init.pytests/unit/memory/test_sparse.pytests/unit/memory/backends/mem0/test_sparse_search.pytests/unit/memory/test_retriever.py
docs/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Documentation files in docs/ are Markdown, built with Zensical, configured in mkdocs.yml; design spec in docs/design/ (12 pages), Architecture in docs/architecture/, Roadmap in docs/roadmap/
Files:
docs/design/memory.md
🧠 Learnings (33)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/memory/**/*.py : Memory package (memory/): pluggable MemoryBackend protocol, backends/ (Mem0 adapter), retrieval pipeline (ranking, RRF fusion, injection, formatting, non-inferable filtering), shared org memory (org/), consolidation/archival (density-aware: DensityClassifier, AbstractiveSummarizer, ExtractivePreserver, DualModeConsolidationStrategy)
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to pyproject.toml : Required runtime dependencies: `mem0ai` (default and currently only memory backend), `cryptography` (Fernet encryption), `faker` (multi-locale name generation)
Applied to files:
pyproject.tomlCLAUDE.md
📚 Learning: 2026-03-15T18:17:43.675Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:17:43.675Z
Learning: Applies to pyproject.toml : Dependencies: all versions use == in pyproject.toml. Groups: test (pytest + plugins, hypothesis), dev (includes test + ruff, mypy, pre-commit, commitizen, pip-audit). Required: mem0ai (Mem0 memory backend — the default and currently only backend). Install: uv sync installs everything (dev group is default).
Applied to files:
pyproject.tomlCLAUDE.md
📚 Learning: 2026-04-02T18:54:07.757Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T18:54:07.757Z
Learning: Applies to pyproject.toml : All versions pinned with `==` in pyproject.toml; Groups: test (pytest + plugins, hypothesis), dev (includes test + ruff, mypy, pre-commit, commitizen, pip-audit)
Applied to files:
pyproject.tomlCLAUDE.md
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/memory/**/*.py : Memory package (memory/): pluggable MemoryBackend protocol, backends/ (Mem0 adapter), retrieval pipeline (ranking, RRF fusion, injection, formatting, non-inferable filtering), shared org memory (org/), consolidation/archival (density-aware: DensityClassifier, AbstractiveSummarizer, ExtractivePreserver, DualModeConsolidationStrategy)
Applied to files:
src/synthorg/memory/backends/mem0/config.pyCLAUDE.mdsrc/synthorg/memory/injection.pysrc/synthorg/memory/retrieval_config.pysrc/synthorg/memory/__init__.pytests/unit/memory/test_tool_retriever.pytests/unit/memory/test_init.pydocs/design/memory.mdsrc/synthorg/memory/sparse.pysrc/synthorg/memory/retriever.pytests/unit/memory/backends/mem0/test_sparse_search.pysrc/synthorg/memory/tool_retriever.pysrc/synthorg/memory/backends/mem0/adapter.pytests/unit/memory/test_retriever.pysrc/synthorg/memory/backends/mem0/sparse_search.pysrc/synthorg/memory/reformulation.pysrc/synthorg/observability/events/memory.py
📚 Learning: 2026-03-16T06:24:56.341Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:24:56.341Z
Learning: Applies to src/synthorg/memory/**/*.py : Use MemoryBackend protocol with pluggable backends (Mem0 adapter available at backends/mem0/) for persistent agent memory
Applied to files:
src/synthorg/memory/backends/mem0/config.pysrc/synthorg/memory/injection.pysrc/synthorg/memory/__init__.pydocs/design/memory.mdsrc/synthorg/memory/tool_retriever.pysrc/synthorg/memory/backends/mem0/adapter.pytests/unit/memory/test_retriever.pysrc/synthorg/memory/reformulation.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/**/*.py : Package structure: src/synthorg/ organized as: api/ (REST+WebSocket, Litestar), auth/ (auth subpackage), backup/ (scheduled/manual backups), budget/ (cost tracking, CFO), cli/ (superseded by Go CLI), communication/ (message bus, meetings), config/ (YAML loading), core/ (domain models, resilience config), engine/ (orchestration, task state, coordination, approval gates, stagnation detection, context budget, compaction), hr/ (hiring, performance, promotion), memory/ (pluggable backend, Mem0, retrieval, consolidation), persistence/ (operational data, SQLite, settings), observability/ (logging, correlation, sinks), providers/ (LLM abstraction, LiteLLM, auth types, presets, runtime CRUD), settings/ (runtime-editable, typed definitions, encryption, config bridge), security/ (SecOps, rule engine, output scanning, progressive trust, autonomy levels), templates/ (company templates, personalities), tools/ (registry, built-in tools, git, sandbox, code_runner, MCP...
Applied to files:
CLAUDE.mdsrc/synthorg/memory/__init__.pysrc/synthorg/memory/reformulation.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Engine: Agent orchestration, execution loops, parallel execution, task decomposition, routing, task assignment, centralized single-writer task state engine (TaskEngine), task lifecycle, recovery, shutdown, workspace isolation, coordination (multi-agent pipeline: TopologyDispatcher protocol, 4 dispatchers — SAS/centralized/decentralized/context-dependent, wave execution, workspace lifecycle integration, CoordinationSectionConfig company config bridge, build_coordinator factory), coordination error classification, prompt policy validation, checkpoint recovery (checkpoint/, per-turn persistence, heartbeat detection, CheckpointRecoveryStrategy), approval gate (escalation detection, context parking/resume, EscalationInfo/ResumePayload models), stagnation detection (stagnation/, StagnationDetector protocol, ToolRepetitionDetector, dual-signal analysis, corrective prompt injection), agent runtime state (AgentRuntimeState, lightweight per-agent execution status for dashboard queries and recove...
Applied to files:
CLAUDE.mddocs/design/memory.md
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/engine/**/*.py : Engine package (engine/): agent orchestration, parallel execution, task decomposition, routing, TaskEngine (centralized single-writer), task lifecycle/recovery/shutdown, workspace isolation, coordination (4 dispatchers: SAS/centralized/decentralized/context-dependent, wave execution), approval gates (escalation detection, context parking/resume), stagnation detection (ToolRepetitionDetector, corrective prompt injection), AgentRuntimeState (execution status), context budget management, conversation compaction (oldest-turns summarizer)
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-16T06:24:56.341Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:24:56.341Z
Learning: Applies to src/synthorg/core/**/*.py : Core module must contain shared domain models, base classes, resilience config (RetryConfig, RateLimiterConfig)
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to docs/** : Docs source in docs/ (Markdown, built with Zensical); design spec in docs/design/ (7 pages: index, agents, organization, communication, engine, memory, operations)
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-19T07:13:44.964Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:13:44.964Z
Learning: Applies to src/synthorg/hr/**/*.py : HR package (hr/): hiring, firing, onboarding, offboarding, agent registry, performance tracking (task metrics, collaboration scoring, LLM calibration, collaboration overrides, trend detection), promotion/demotion (criteria evaluation, approval strategies, model mapping)
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Documentation source in `docs/` (Markdown, built with Zensical). Design spec in `docs/design/` (7 pages: index, agents, organization, communication, engine, memory, operations). Architecture in `docs/architecture/` (overview, tech-stack, decision log). Roadmap in `docs/roadmap/`. Security in `docs/security.md`. Licensing in `docs/licensing.md`. Reference in `docs/reference/`. REST API reference in `docs/rest-api.md`. Library reference in `docs/api/` (auto-generated from docstrings). Custom templates in `docs/overrides/`. Config in `mkdocs.yml`.
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to docs/design/*.md : Design spec pages: 7 pages in `docs/design/` — index, agents, organization, communication, engine, memory, operations
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-16T06:24:56.341Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:24:56.341Z
Learning: Applies to src/synthorg/hr/**/*.py : HR engine must provide: hiring, firing, onboarding, offboarding, agent registry, performance tracking (task metrics, collaboration scoring, trend detection), promotion/demotion
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-21T12:54:22.557Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-21T12:54:22.557Z
Learning: Go 1.26+ required; CLI dependencies in `cli/go.mod` (Cobra, charmbracelet/huh, charmbracelet/lipgloss, sigstore-go, go-containerregistry, go-tuf)
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-19T11:19:40.044Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:19:40.044Z
Learning: Applies to go.mod : Maintain Go 1.26+ requirement. Dependencies: Cobra (CLI framework), charmbracelet/huh and charmbracelet/lipgloss (UI), sigstore-go (code signing), go-containerregistry (container image verification), go-tuf (TUF client for Sigstore).
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-21T14:12:17.848Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-21T14:12:17.848Z
Learning: Applies to cli/go.mod : CLI uses Go 1.26+. Dependencies: Cobra, charmbracelet/huh, charmbracelet/lipgloss, sigstore-go, go-containerregistry, go-tuf
Applied to files:
CLAUDE.md
📚 Learning: 2026-04-02T18:54:07.757Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T18:54:07.757Z
Learning: Applies to cli/**/*.go : Go CLI binary uses Go 1.26+, Cobra, charmbracelet/huh, charmbracelet/lipgloss, sigstore-go, go-containerregistry, go-tuf
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to web/package.json : Web dashboard Node.js 20+; dependencies in web/package.json (Vue 3, PrimeVue, Tailwind CSS, Pinia, VueFlow, ECharts, Axios, vue-draggable-plus, Vitest, ESLint, vue-tsc)
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-15T18:17:43.675Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:17:43.675Z
Learning: Applies to web/** : Web dashboard: Node.js 20+, dependencies in web/package.json (Vue 3, PrimeVue, Tailwind CSS, Pinia, VueFlow, ECharts, Axios, vue-draggable-plus, Vitest, fast-check, ESLint, vue-tsc).
Applied to files:
CLAUDE.md
📚 Learning: 2026-04-02T18:54:07.757Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T18:54:07.757Z
Learning: Applies to web/src/**/*.{ts,tsx} : React 19 dashboard uses TypeScript 6.0+, design tokens, Radix UI, Tailwind CSS 4, Zustand, tanstack/react-query, xyflow/react, dagrejs/dagre, d3-force, dnd-kit, Recharts, Framer Motion, cmdk, js-yaml, Axios, Lucide React, fontsource-variable fonts, CodeMirror 6, Storybook 10, MSW, Vitest, testing-library/react, fast-check, ESLint, and Playwright
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.
Applied to files:
src/synthorg/memory/retrieval_config.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (via `model_copy(update=...)`) for runtime state that evolves
Applied to files:
src/synthorg/memory/retrieval_config.py
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; separate mutable-via-copy models (using `model_copy(update=...)`) for runtime state
Applied to files:
src/synthorg/memory/retrieval_config.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).
Applied to files:
src/synthorg/memory/retrieval_config.py
📚 Learning: 2026-04-02T18:54:07.757Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T18:54:07.757Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from domain-specific modules under synthorg.observability.events (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool); import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`
Applied to files:
src/synthorg/observability/events/memory.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from synthorg.observability.events domain-specific modules (e.g., PROVIDER_CALL_START from events.provider). Import directly: from synthorg.observability.events.<domain> import EVENT_CONSTANT.
Applied to files:
src/synthorg/observability/events/memory.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from domain-specific modules under synthorg.observability.events (e.g., PROVIDER_CALL_START from events.provider, BUDGET_RECORD_ADDED from events.budget, etc.). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/observability/events/memory.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : Use event name constants from domain-specific modules under `ai_company.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`). Import directly: `from ai_company.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/observability/events/memory.py
📚 Learning: 2026-03-20T11:18:48.128Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T11:18:48.128Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/observability/events/memory.py
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`); import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`
Applied to files:
src/synthorg/observability/events/memory.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/observability/**/*.py : Observability package (observability/): structured logging, correlation tracking, log sinks; event constants organized by domain under observability/events/ (e.g., events.api, events.tool, events.git, events.context_budget, events.backup)
Applied to files:
src/synthorg/observability/events/memory.py
🪛 markdownlint-cli2 (0.22.0)
docs/design/memory.md
[warning] 701-701: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
🔇 Additional comments (19)
src/synthorg/memory/backends/mem0/adapter.py (4)
325-330: LGTM!The
supports_sparse_searchproperty correctly gates sparse retrieval based on both configuration and successful Qdrant field initialization.
384-416: LGTM!The
_try_sparse_upserthelper correctly implements non-fatal sparse upsert with proper logging. The design ensures dense search remains available even if sparse indexing fails, which aligns with the graceful degradation pattern documented in the PR objectives.
846-847:⚠️ Potential issue | 🔴 CriticalCritical: Same
exceptsyntax bug repeated inretrieve_sparse.🐛 Proposed fix
- except builtins.MemoryError, RecursionError: + except (builtins.MemoryError, RecursionError): raise> Likely an incorrect or invalid review comment.
407-408:⚠️ Potential issue | 🔴 CriticalCritical: Same
exceptsyntax bug as line 201.This will catch
MemoryErrorbut notRecursionError, contradicting the intent documented in the module docstring (line 7).🐛 Proposed fix
- except builtins.MemoryError, RecursionError: + except (builtins.MemoryError, RecursionError): raise⛔ Skipped due to learnings
Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-14T15:43:05.601Z Learning: Applies to **/*.py : Handle errors explicitly, never silently swallow exceptionsLearnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-16T07:22:28.134Z Learning: Applies to **/*.py : Handle errors explicitly; never silently swallow exceptionssrc/synthorg/memory/tool_retriever.py (5)
76-88: LGTM!The
_format_entrieshelper provides clean, human-readable output with optional relevance scores. The format[category](relevance: X.XX) contentis clear and informative.
162-182: LGTM!Tool definitions correctly implement the JSON Schema structure with required fields, types, and constraints. The schema for
search_memoryappropriately boundslimitto 1-50.
243-260: Good error handling - internal details are not leaked to LLM.The distinction between domain errors ("temporarily unavailable") and unexpected errors ("unexpected error") provides appropriate feedback while keeping internal details out of the LLM context.
286-287:⚠️ Potential issue | 🔴 CriticalCritical: Incorrect
exceptsyntax in_handle_recall.🐛 Proposed fix
- except builtins.MemoryError, RecursionError: + except (builtins.MemoryError, RecursionError): raise⛔ Skipped due to learnings
Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-16T07:22:28.134Z Learning: Applies to **/*.py : Use `except A, B:` syntax (no parentheses) for exception handling — PEP 758 exception syntax enforced by ruff on Python 3.14Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-31T14:17:24.182Z Learning: Applies to **/*.py : Use `except A, B:` (no parentheses) per PEP 758 exception syntax on Python 3.14Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-20T21:44:04.528Z Learning: Applies to **/*.py : Use `except A, B:` syntax (without parentheses) per PEP 758 for exception handling in Python 3.14Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-16T07:22:28.134Z Learning: Applies to **/*.py : Handle errors explicitly; never silently swallow exceptionsLearnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-14T16:18:57.267Z Learning: Applies to **/*.py : Handle errors explicitly—never silently swallow exceptions.Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-14T15:43:05.601Z Learning: Applies to **/*.py : Handle errors explicitly, never silently swallow exceptionsLearnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-19T21:11:37.538Z Learning: Applies to **/*.py : Errors: handle explicitly, never silently swallow.Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-04-02T18:54:07.757Z Learning: Applies to **/*.py : Handle errors explicitly in Python; never silently swallow exceptionsLearnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-14T16:18:57.267Z Learning: Applies to **/*.py : Use PEP 758 except syntax with `except A, B:` (no parentheses) for multiple exceptions—ruff enforces this on Python 3.14.Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-14T15:43:05.601Z Learning: Applies to **/*.py : Use PEP 758 except syntax: `except A, B:` (no parentheses) — enforced by ruff on Python 3.14
241-242:⚠️ Potential issue | 🔴 CriticalCritical: Incorrect
exceptsyntax - same issue as in adapter.py.🐛 Proposed fix
- except builtins.MemoryError, RecursionError: + except (builtins.MemoryError, RecursionError): raise⛔ Skipped due to learnings
Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-31T14:17:24.182Z Learning: Applies to **/*.py : Use `except A, B:` (no parentheses) per PEP 758 exception syntax on Python 3.14Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-16T07:22:28.134Z Learning: Applies to **/*.py : Use `except A, B:` syntax (no parentheses) for exception handling — PEP 758 exception syntax enforced by ruff on Python 3.14Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-20T21:44:04.528Z Learning: Applies to **/*.py : Use `except A, B:` syntax (without parentheses) per PEP 758 for exception handling in Python 3.14Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-14T15:43:05.601Z Learning: Applies to **/*.py : Use PEP 758 except syntax: `except A, B:` (no parentheses) — enforced by ruff on Python 3.14Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-14T15:43:05.601Z Learning: Applies to **/*.py : Handle errors explicitly, never silently swallow exceptionsLearnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-14T16:18:57.267Z Learning: Applies to **/*.py : Use PEP 758 except syntax with `except A, B:` (no parentheses) for multiple exceptions—ruff enforces this on Python 3.14.Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-14T16:18:57.267Z Learning: Applies to **/*.py : Handle errors explicitly—never silently swallow exceptions.Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-04-02T18:54:07.757Z Learning: Applies to **/*.py : Use PEP 758 except syntax: use `except A, B:` (no parentheses) in Python 3.14; ruff enforces thisLearnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-16T07:22:28.134Z Learning: Applies to **/*.py : Handle errors explicitly; never silently swallow exceptionsLearnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-15T16:55:07.730Z Learning: Applies to **/*.py : Use PEP 758 except syntax: use `except A, B:` (no parentheses) — ruff enforces this on Python 3.14.tests/unit/memory/test_tool_retriever.py (3)
52-66: LGTM!Protocol compliance tests verify the strategy satisfies
MemoryInjectionStrategyand returns the correctstrategy_name.
240-258: Excellent test for security boundary.This test correctly verifies that internal error details ("db down") are not leaked to the LLM-facing response, which is important for security and clean error handling.
306-331: Good coverage for system error propagation.These tests verify that
MemoryErrorandRecursionErrorare properly re-raised rather than being converted to generic messages, which is the expected behavior per the module docstring.src/synthorg/memory/reformulation.py (4)
91-96: LGTM!The summary formatting appropriately truncates content (200 chars) and limits entries (10) to keep prompt size manageable while providing sufficient context for the LLM.
198-200: LGTM - robust sufficiency parsing.Using
.split()to tokenize before checking membership correctly distinguishes "SUFFICIENT" from "INSUFFICIENT" since they become separate tokens. The inline comment explains this clearly.
140-141:⚠️ Potential issue | 🔴 CriticalCritical: Incorrect
exceptsyntax.🐛 Proposed fix
- except builtins.MemoryError, RecursionError: + except (builtins.MemoryError, RecursionError): raise⛔ Skipped due to learnings
Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-31T14:17:24.182Z Learning: Applies to **/*.py : Use `except A, B:` (no parentheses) per PEP 758 exception syntax on Python 3.14Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-16T07:22:28.134Z Learning: Applies to **/*.py : Use `except A, B:` syntax (no parentheses) for exception handling — PEP 758 exception syntax enforced by ruff on Python 3.14Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-20T21:44:04.528Z Learning: Applies to **/*.py : Use `except A, B:` syntax (without parentheses) per PEP 758 for exception handling in Python 3.14Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-14T15:43:05.601Z Learning: Applies to **/*.py : Use PEP 758 except syntax: `except A, B:` (no parentheses) — enforced by ruff on Python 3.14Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-14T16:18:57.267Z Learning: Applies to **/*.py : Use PEP 758 except syntax with `except A, B:` (no parentheses) for multiple exceptions—ruff enforces this on Python 3.14.Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-04-02T18:54:07.757Z Learning: Applies to **/*.py : Use PEP 758 except syntax: use `except A, B:` (no parentheses) in Python 3.14; ruff enforces thisLearnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-15T16:55:07.730Z Learning: Applies to **/*.py : Use PEP 758 except syntax: use `except A, B:` (no parentheses) — ruff enforces this on Python 3.14.Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-14T16:18:57.267Z Learning: Applies to **/*.py : Handle errors explicitly—never silently swallow exceptions.Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-16T07:22:28.134Z Learning: Applies to **/*.py : Handle errors explicitly; never silently swallow exceptionsLearnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-14T15:43:05.601Z Learning: Applies to **/*.py : Handle errors explicitly, never silently swallow exceptions
201-202:⚠️ Potential issue | 🔴 CriticalCritical: Incorrect
exceptsyntax incheck_sufficiency.🐛 Proposed fix
- except builtins.MemoryError, RecursionError: + except (builtins.MemoryError, RecursionError): raise⛔ Skipped due to learnings
Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-16T07:22:28.134Z Learning: Applies to **/*.py : Use `except A, B:` syntax (no parentheses) for exception handling — PEP 758 exception syntax enforced by ruff on Python 3.14src/synthorg/memory/backends/mem0/sparse_search.py (3)
32-73: LGTM!The
ensure_sparse_fieldfunction is properly idempotent, checking for existing fields before creation. UsingModifier.IDFfor server-side IDF scoring is the correct approach for BM25.
177-217: LGTM - good resilience pattern.The function gracefully handles malformed payloads by skipping individual points rather than failing the entire batch. The error logging when all points are skipped helps identify potential bugs in
_point_to_entry.
200-201:⚠️ Potential issue | 🔴 CriticalCritical: Incorrect
exceptsyntax.🐛 Proposed fix
- except builtins.MemoryError, RecursionError: + except (builtins.MemoryError, RecursionError): raise⛔ Skipped due to learnings
Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-31T14:17:24.182Z Learning: Applies to **/*.py : Use `except A, B:` (no parentheses) per PEP 758 exception syntax on Python 3.14Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-16T07:22:28.134Z Learning: Applies to **/*.py : Use `except A, B:` syntax (no parentheses) for exception handling — PEP 758 exception syntax enforced by ruff on Python 3.14Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-20T21:44:04.528Z Learning: Applies to **/*.py : Use `except A, B:` syntax (without parentheses) per PEP 758 for exception handling in Python 3.14Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-14T15:43:05.601Z Learning: Applies to **/*.py : Use PEP 758 except syntax: `except A, B:` (no parentheses) — enforced by ruff on Python 3.14Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-14T16:18:57.267Z Learning: Applies to **/*.py : Use PEP 758 except syntax with `except A, B:` (no parentheses) for multiple exceptions—ruff enforces this on Python 3.14.Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-04-02T18:54:07.757Z Learning: Applies to **/*.py : Use PEP 758 except syntax: use `except A, B:` (no parentheses) in Python 3.14; ruff enforces thisLearnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-15T16:55:07.730Z Learning: Applies to **/*.py : Use PEP 758 except syntax: use `except A, B:` (no parentheses) — ruff enforces this on Python 3.14.Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-14T16:18:57.267Z Learning: Applies to **/*.py : Handle errors explicitly—never silently swallow exceptions.Learnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-16T07:22:28.134Z Learning: Applies to **/*.py : Handle errors explicitly; never silently swallow exceptionsLearnt from: CR Repo: Aureliolo/synthorg PR: 0 File: CLAUDE.md:0-0 Timestamp: 2026-03-14T15:43:05.601Z Learning: Applies to **/*.py : Handle errors explicitly, never silently swallow exceptions
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
tests/unit/memory/test_sparse.py (1)
103-107:⚠️ Potential issue | 🟡 MinorExercise the real Unicode normalization path here.
"Uber Strasze"is ASCII-only, so this still passes iftokenize()regresses from NFKC+casefold()back to plain lowercase. Use an actual non-ASCII + compatibility-form sample so the regression stays covered.🔎 Stronger regression test
- def test_unicode_lowercased(self) -> None: + def test_unicode_normalization_and_casefold(self) -> None: tokenizer = BM25Tokenizer() - tokens = tokenizer.tokenize("Uber Strasze") - assert "uber" in tokens - assert "strasze" in tokens + tokens = tokenizer.tokenize("Über Straße Ａgent") + assert tokens == ("über", "strasse", "agent")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/memory/test_sparse.py` around lines 103 - 107, The test uses only ASCII so it won't catch regressions from NFKC+casefold normalization; update test_unicode_lowercased to call BM25Tokenizer.tokenize with a non-ASCII input that includes a compatibility-form character (for example a word containing the German ß or the Angstrom sign Å) and assert the expected normalized, lowercased tokens (reference BM25Tokenizer.tokenize and the test name test_unicode_lowercased to locate where to change the sample and assertions).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/architecture/tech-stack.md`:
- Line 53: Clarify that enabling hybrid dense+sparse retrieval requires flipping
two configuration switches: keep the note that sparse_search_enabled only
enables the sparse index component, and state that the retriever must also be
configured to use RRF/linear fusion (or set the retriever fusion mode) to
combine dense and sparse results; reference Mem0 and MemoryBackend for where
these settings apply and mention the retriever fusion mode (RRF/linear fusion)
so readers know to enable both sparse_search_enabled and the retriever fusion
setting to get hybrid retrieval.
In `@src/synthorg/memory/retrieval_config.py`:
- Around line 186-200: The validator _validate_personal_boost_rrf_consistency
currently warns for FusionStrategy.RRF even when personal_boost is the default;
change it to only warn if the caller explicitly set personal_boost by checking
the model's tracked fields (e.g., "personal_boost" in self.model_fields_set or
in getattr(self, '__pydantic_fields_set__', set())). Keep the rest of the logic
(fusion_strategy == FusionStrategy.RRF and personal_boost > 0.0) but gate the
logger.warning behind that explicit-field check so default values don't emit
warnings.
In `@src/synthorg/memory/sparse.py`:
- Around line 9-15: This module lacks the standard logger; import get_logger
from synthorg.observability and add logger = get_logger(__name__) near the top
of the file (alongside the existing imports) so business-logic classes like
BM25Tokenizer use the repo-standard logger; ensure the import statement is
present and the logger variable is defined before any class or function
definitions so BM25Tokenizer and other top-level logic can call logger.
---
Duplicate comments:
In `@tests/unit/memory/test_sparse.py`:
- Around line 103-107: The test uses only ASCII so it won't catch regressions
from NFKC+casefold normalization; update test_unicode_lowercased to call
BM25Tokenizer.tokenize with a non-ASCII input that includes a compatibility-form
character (for example a word containing the German ß or the Angstrom sign Å)
and assert the expected normalized, lowercased tokens (reference
BM25Tokenizer.tokenize and the test name test_unicode_lowercased to locate where
to change the sample and assertions).
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: dfd20b95-f21c-4bf2-8944-bd83813e58e2
📒 Files selected for processing (14)
README.mddocs/architecture/tech-stack.mddocs/design/memory.mdsrc/synthorg/memory/backends/mem0/adapter.pysrc/synthorg/memory/backends/mem0/sparse_search.pysrc/synthorg/memory/injection.pysrc/synthorg/memory/retrieval_config.pysrc/synthorg/memory/retriever.pysrc/synthorg/memory/sparse.pysrc/synthorg/memory/tool_retriever.pysrc/synthorg/observability/events/memory.pytests/unit/memory/test_retrieval_config.pytests/unit/memory/test_retriever.pytests/unit/memory/test_sparse.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Deploy Preview
- GitHub Check: Test (Python 3.14)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (python)
- GitHub Check: Build Sandbox
- GitHub Check: Build Web
- GitHub Check: Build Backend
- GitHub Check: Dependency Review
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotationsin Python code; Python 3.14 has PEP 649 native lazy annotations
Use PEP 758 except syntax: useexcept A, B:(no parentheses) in Python 3.14; ruff enforces this
All public functions in Python must have type hints; mypy strict mode enforced
Use Google-style docstrings on public classes and functions in Python; enforced by ruff D rules
Create new objects and never mutate existing ones; for non-Pydantic internal collections (registries, BaseTool), use copy.deepcopy() at construction + MappingProxyType wrapping for read-only enforcement
For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence)
Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves
Use Pydantic v2 (BaseModel, model_validator, computed_field, ConfigDict); use allow_inf_nan=False in all ConfigDict declarations to reject NaN/Inf in numeric fields at validation time
Use@computed_fieldfor derived values instead of storing + validating redundant fields in Pydantic models (e.g. TokenUsage.total_tokens)
Use NotBlankStr from core.types for all identifier/name fields in Python (including optional and tuple variants) instead of manual whitespace validators
Prefer asyncio.TaskGroup for fan-out/fan-in parallel operations in Python (e.g. multiple tool invocations, parallel agent calls); prefer structured concurrency over bare create_task
Python line length must not exceed 88 characters; enforced by ruff
Python functions must be under 50 lines; files must be under 800 lines
Handle errors explicitly in Python; never silently swallow exceptions
Validate at system boundaries in Python (user input, external APIs, config files)
Files:
src/synthorg/memory/injection.pytests/unit/memory/test_retriever.pysrc/synthorg/memory/retrieval_config.pytests/unit/memory/test_sparse.pysrc/synthorg/memory/sparse.pysrc/synthorg/memory/backends/mem0/adapter.pysrc/synthorg/memory/tool_retriever.pysrc/synthorg/observability/events/memory.pytests/unit/memory/test_retrieval_config.pysrc/synthorg/memory/retriever.pysrc/synthorg/memory/backends/mem0/sparse_search.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every Python module with business logic must have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging/logging.getLogger()/print()in Python application code; exceptions are observability/setup.py, observability/sinks.py, observability/syslog_handler.py, and observability/http_handler.py
Python logger variable name must always belogger(not_logger, notlog)
Use event name constants from domain-specific modules under synthorg.observability.events (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool); import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANT
Use structured logging with kwargs in Python: alwayslogger.info(EVENT, key=value)-- neverlogger.info('msg %s', val)
All error paths in Python must log at WARNING or ERROR with context before raising
All state transitions in Python must log at INFO level
Use DEBUG logging level in Python for object creation, internal flow, entry/exit of key functions
Pure data models, enums, and re-exports in Python do NOT need logging
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned Python code, docstrings, comments, tests, or config examples; use generic names: example-provider, example-large-001, example-medium-001, example-small-001, large/medium/small as aliases
Files:
src/synthorg/memory/injection.pysrc/synthorg/memory/retrieval_config.pysrc/synthorg/memory/sparse.pysrc/synthorg/memory/backends/mem0/adapter.pysrc/synthorg/memory/tool_retriever.pysrc/synthorg/observability/events/memory.pysrc/synthorg/memory/retriever.pysrc/synthorg/memory/backends/mem0/sparse_search.py
docs/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Documentation files in docs/ are Markdown, built with Zensical, configured in mkdocs.yml; design spec in docs/design/ (12 pages), Architecture in docs/architecture/, Roadmap in docs/roadmap/
Files:
docs/architecture/tech-stack.mddocs/design/memory.md
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: All Python test files must use@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, or@pytest.mark.slowmarkers
Python tests must maintain 80% minimum code coverage (enforced in CI)
Prefer@pytest.mark.parametrizefor testing similar cases in Python
Use test-provider, test-small-001, etc. in Python tests instead of real vendor names
Property-based testing in Python uses Hypothesis (@given+@settings); profiles: ci (50 examples, default) and dev (1000 examples), controlled via HYPOTHESIS_PROFILE env var
Never skip, dismiss, or ignore flaky Python tests; always fix them fully and fundamentally; for timing-sensitive tests, mock time.monotonic() and asyncio.sleep() to make them deterministic instead of widening timing margins
For Python tasks that must block indefinitely until cancelled (e.g. simulating a slow provider or stubborn coroutine), use asyncio.Event().wait() instead of asyncio.sleep(large_number) -- it is cancellation-safe and carries no timing assumptions
Files:
tests/unit/memory/test_retriever.pytests/unit/memory/test_sparse.pytests/unit/memory/test_retrieval_config.py
🧠 Learnings (34)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/memory/**/*.py : Memory package (memory/): pluggable MemoryBackend protocol, backends/ (Mem0 adapter), retrieval pipeline (ranking, RRF fusion, injection, formatting, non-inferable filtering), shared org memory (org/), consolidation/archival (density-aware: DensityClassifier, AbstractiveSummarizer, ExtractivePreserver, DualModeConsolidationStrategy)
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/memory/**/*.py : Memory package (memory/): pluggable MemoryBackend protocol, backends/ (Mem0 adapter), retrieval pipeline (ranking, RRF fusion, injection, formatting, non-inferable filtering), shared org memory (org/), consolidation/archival (density-aware: DensityClassifier, AbstractiveSummarizer, ExtractivePreserver, DualModeConsolidationStrategy)
Applied to files:
README.mdsrc/synthorg/memory/injection.pydocs/architecture/tech-stack.mdtests/unit/memory/test_retriever.pysrc/synthorg/memory/retrieval_config.pysrc/synthorg/memory/sparse.pydocs/design/memory.mdsrc/synthorg/memory/backends/mem0/adapter.pysrc/synthorg/memory/tool_retriever.pysrc/synthorg/observability/events/memory.pysrc/synthorg/memory/retriever.pysrc/synthorg/memory/backends/mem0/sparse_search.py
📚 Learning: 2026-03-16T06:24:56.341Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:24:56.341Z
Learning: Applies to src/synthorg/memory/**/*.py : Use MemoryBackend protocol with pluggable backends (Mem0 adapter available at backends/mem0/) for persistent agent memory
Applied to files:
README.mdsrc/synthorg/memory/injection.pydocs/architecture/tech-stack.mdtests/unit/memory/test_retriever.pydocs/design/memory.mdsrc/synthorg/memory/backends/mem0/adapter.pysrc/synthorg/memory/tool_retriever.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Engine: Agent orchestration, execution loops, parallel execution, task decomposition, routing, task assignment, centralized single-writer task state engine (TaskEngine), task lifecycle, recovery, shutdown, workspace isolation, coordination (multi-agent pipeline: TopologyDispatcher protocol, 4 dispatchers — SAS/centralized/decentralized/context-dependent, wave execution, workspace lifecycle integration, CoordinationSectionConfig company config bridge, build_coordinator factory), coordination error classification, prompt policy validation, checkpoint recovery (checkpoint/, per-turn persistence, heartbeat detection, CheckpointRecoveryStrategy), approval gate (escalation detection, context parking/resume, EscalationInfo/ResumePayload models), stagnation detection (stagnation/, StagnationDetector protocol, ToolRepetitionDetector, dual-signal analysis, corrective prompt injection), agent runtime state (AgentRuntimeState, lightweight per-agent execution status for dashboard queries and recove...
Applied to files:
README.mddocs/architecture/tech-stack.mddocs/design/memory.md
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Security: SecOps agent, rule engine (soft-allow/hard-deny, fail-closed), audit log, output scanner, output scan response policies (redact/withhold/log-only/autonomy-tiered), risk classifier, risk tier classifier, action type registry, ToolInvoker security integration, progressive trust (4 strategies: disabled/weighted/per-category/milestone), autonomy levels (presets, resolver, change strategy), timeout policies (park/resume).
Applied to files:
README.md
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to pyproject.toml : Required runtime dependencies: `mem0ai` (default and currently only memory backend), `cryptography` (Fernet encryption), `faker` (multi-locale name generation)
Applied to files:
docs/architecture/tech-stack.md
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Documentation source in `docs/` (Markdown, built with Zensical). Design spec in `docs/design/` (7 pages: index, agents, organization, communication, engine, memory, operations). Architecture in `docs/architecture/` (overview, tech-stack, decision log). Roadmap in `docs/roadmap/`. Security in `docs/security.md`. Licensing in `docs/licensing.md`. Reference in `docs/reference/`. REST API reference in `docs/rest-api.md`. Library reference in `docs/api/` (auto-generated from docstrings). Custom templates in `docs/overrides/`. Config in `mkdocs.yml`.
Applied to files:
docs/architecture/tech-stack.md
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/**/*.py : Package structure: src/synthorg/ organized as: api/ (REST+WebSocket, Litestar), auth/ (auth subpackage), backup/ (scheduled/manual backups), budget/ (cost tracking, CFO), cli/ (superseded by Go CLI), communication/ (message bus, meetings), config/ (YAML loading), core/ (domain models, resilience config), engine/ (orchestration, task state, coordination, approval gates, stagnation detection, context budget, compaction), hr/ (hiring, performance, promotion), memory/ (pluggable backend, Mem0, retrieval, consolidation), persistence/ (operational data, SQLite, settings), observability/ (logging, correlation, sinks), providers/ (LLM abstraction, LiteLLM, auth types, presets, runtime CRUD), settings/ (runtime-editable, typed definitions, encryption, config bridge), security/ (SecOps, rule engine, output scanning, progressive trust, autonomy levels), templates/ (company templates, personalities), tools/ (registry, built-in tools, git, sandbox, code_runner, MCP...
Applied to files:
docs/architecture/tech-stack.md
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.
Applied to files:
src/synthorg/memory/retrieval_config.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (via `model_copy(update=...)`) for runtime state that evolves
Applied to files:
src/synthorg/memory/retrieval_config.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to tests/**/*.py : Use Hypothesis for property-based testing: `given` + `settings`; dev profile has 1000 examples (via `HYPOTHESIS_PROFILE=dev env var`), ci profile (default) has 50
Applied to files:
tests/unit/memory/test_sparse.py
📚 Learning: 2026-04-02T18:54:07.757Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T18:54:07.757Z
Learning: Applies to tests/**/*.py : Property-based testing in Python uses Hypothesis (given + settings); profiles: ci (50 examples, default) and dev (1000 examples), controlled via HYPOTHESIS_PROFILE env var
Applied to files:
tests/unit/memory/test_sparse.py
📚 Learning: 2026-03-15T18:42:17.990Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:42:17.990Z
Learning: Applies to tests/**/*.py : Use Hypothesis for property-based testing with `given` + `settings` decorators; control profiles via `HYPOTHESIS_PROFILE` env var (`ci` for 200 examples, `dev` for 1000 examples)
Applied to files:
tests/unit/memory/test_sparse.py
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to tests/**/*.py : Use Hypothesis for property-based testing in Python with `given` + `settings`. Control via `HYPOTHESIS_PROFILE` env var (dev: 1000 examples, ci: 200 examples).
Applied to files:
tests/unit/memory/test_sparse.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Property-based testing: Python uses Hypothesis (given + settings). Hypothesis profiles: ci (200 examples, default) and dev (1000 examples), controlled via HYPOTHESIS_PROFILE env var. Run dev profile: HYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n auto -k properties.
Applied to files:
tests/unit/memory/test_sparse.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Run property tests with `HYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n auto -k properties`
Applied to files:
tests/unit/memory/test_sparse.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to **/*.py : Handle errors explicitly—never silently swallow exceptions.
Applied to files:
src/synthorg/memory/backends/mem0/adapter.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to **/*.py : Handle errors explicitly; never silently swallow exceptions
Applied to files:
src/synthorg/memory/backends/mem0/adapter.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to **/*.py : Handle errors explicitly, never silently swallow exceptions
Applied to files:
src/synthorg/memory/backends/mem0/adapter.py
📚 Learning: 2026-04-02T18:54:07.757Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T18:54:07.757Z
Learning: Applies to **/*.py : Handle errors explicitly in Python; never silently swallow exceptions
Applied to files:
src/synthorg/memory/backends/mem0/adapter.py
📚 Learning: 2026-03-19T21:11:37.538Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T21:11:37.538Z
Learning: Applies to **/*.py : Errors: handle explicitly, never silently swallow.
Applied to files:
src/synthorg/memory/backends/mem0/adapter.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to {**/*.py,web/src/**/*.{ts,tsx}} : Handle errors explicitly; never silently swallow exceptions
Applied to files:
src/synthorg/memory/backends/mem0/adapter.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to **/*.py : Use `except A, B:` (no parentheses) per PEP 758 exception syntax on Python 3.14
Applied to files:
src/synthorg/memory/backends/mem0/adapter.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to **/*.py : Use `except A, B:` syntax (without parentheses) per PEP 758 for exception handling in Python 3.14
Applied to files:
src/synthorg/memory/backends/mem0/adapter.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to **/*.py : Use `except A, B:` syntax (no parentheses) for exception handling — PEP 758 exception syntax enforced by ruff on Python 3.14
Applied to files:
src/synthorg/memory/backends/mem0/adapter.py
📚 Learning: 2026-03-14T16:18:57.267Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T16:18:57.267Z
Learning: Applies to **/*.py : Use PEP 758 except syntax with `except A, B:` (no parentheses) for multiple exceptions—ruff enforces this on Python 3.14.
Applied to files:
src/synthorg/memory/backends/mem0/adapter.py
📚 Learning: 2026-04-02T18:54:07.757Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T18:54:07.757Z
Learning: Applies to **/*.py : Use PEP 758 except syntax: use `except A, B:` (no parentheses) in Python 3.14; ruff enforces this
Applied to files:
src/synthorg/memory/backends/mem0/adapter.py
📚 Learning: 2026-03-14T15:43:05.601Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:43:05.601Z
Learning: Applies to **/*.py : Use PEP 758 except syntax: `except A, B:` (no parentheses) — enforced by ruff on Python 3.14
Applied to files:
src/synthorg/memory/backends/mem0/adapter.py
📚 Learning: 2026-03-15T16:55:07.730Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T16:55:07.730Z
Learning: Applies to **/*.py : Use PEP 758 except syntax: use `except A, B:` (no parentheses) — ruff enforces this on Python 3.14.
Applied to files:
src/synthorg/memory/backends/mem0/adapter.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/observability/**/*.py : Observability package (observability/): structured logging, correlation tracking, log sinks; event constants organized by domain under observability/events/ (e.g., events.api, events.tool, events.git, events.context_budget, events.backup)
Applied to files:
src/synthorg/observability/events/memory.py
📚 Learning: 2026-04-02T18:54:07.757Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T18:54:07.757Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from domain-specific modules under synthorg.observability.events (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool); import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`
Applied to files:
src/synthorg/observability/events/memory.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from domain-specific modules under synthorg.observability.events (e.g., PROVIDER_CALL_START from events.provider, BUDGET_RECORD_ADDED from events.budget, etc.). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/observability/events/memory.py
📚 Learning: 2026-04-02T18:54:07.757Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T18:54:07.757Z
Learning: Applies to **/*.py : Prefer asyncio.TaskGroup for fan-out/fan-in parallel operations in Python (e.g. multiple tool invocations, parallel agent calls); prefer structured concurrency over bare create_task
Applied to files:
src/synthorg/memory/retriever.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to **/*.py : Prefer `asyncio.TaskGroup` for fan-out/fan-in parallel operations in new code (e.g., multiple tool invocations, parallel agent calls); prefer structured concurrency over bare `create_task`
Applied to files:
src/synthorg/memory/retriever.py
🪛 LanguageTool
docs/architecture/tech-stack.md
[grammar] ~53-~53: Ensure spelling is correct
Context: ...ifier.IDF` for server-side IDF scoring. Qdrant embedded + SQLite for persistence. Cust...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.22.0)
docs/design/memory.md
[warning] 701-701: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
🔇 Additional comments (30)
src/synthorg/memory/retriever.py (6)
381-391: RRF list concatenation order may bias ranking toward personal results.The concatenation
dense_personal + dense_sharedplaces personal entries at lower positions (0..n), giving them higher RRF contributions purely from position ordering before fusion. This is a known property of RRF when input lists aren't independently sorted by a comparable score.If both personal and shared results are already sorted by relevance from their respective backends, this behavior may be acceptable (personal results appear first in each modality's list). However, if the intent is score-agnostic fusion, consider sorting each modality's combined list by relevance before passing to
fuse_ranked_lists().Verify the intended fusion semantics: is positional priority for personal results over shared results the desired behavior, or should results be interleaved by relevance score before RRF?
76-77: LGTM: PEP 758 exception syntax correctly applied.The
except builtins.MemoryError, RecursionError:syntax without parentheses is the correct PEP 758 form for Python 3.14 as enforced by the coding guidelines.
316-343: LGTM: Clean extraction of LINEAR pipeline logic.The
_execute_linear_pipelinemethod cleanly separates the dense-only retrieval path, maintaining the existing ranking behavior while enabling the new RRF hybrid path.
369-376: LGTM: Proper structured concurrency with TaskGroup and system error unwrapping.The use of
asyncio.TaskGroupfor parallel dense/sparse fetches aligns with coding guidelines. Theexcept*syntax correctly unwraps system-level errors from theExceptionGroupto maintain consistent exception semantics for callers.
417-430: Good: Sparse capability checks are now properly decoupled.Each backend (personal and shared) is now independently checked for sparse support via
personal_has_sparseandshared_has_sparse. This addresses the previous concern where shared sparse retrieval was gated on personal backend capability.
436-453: Conditional coroutine construction is well-structured.The pattern of using a no-op
_empty()coroutine for missing capabilities while still allowing the other backend to contribute results is clean and avoids premature short-circuiting.src/synthorg/memory/backends/mem0/adapter.py (6)
202-203: PEP 758 exception syntax is correct for Python 3.14.The
except builtins.MemoryError, RecursionError:syntax without parentheses is the correct form per PEP 758 as enforced by ruff on Python 3.14. The past review comment was based on pre-3.14 semantics where this would be interpreted as catchingMemoryErrorand binding to variableRecursionError.
132-135: LGTM: Sparse encoder initialization properly gated.The
BM25Tokenizeris only instantiated whensparse_search_enabledis configured, avoiding unnecessary overhead for dense-only deployments.
192-215: LGTM: Sparse field setup is gracefully degraded.The sparse field initialization in
connect():
- Properly re-raises system-level errors (
MemoryError,RecursionError)- Logs warnings and clears
_qdrant_clienton setup failures, allowing dense operations to continue- Doesn't block the connection from completing
329-334: LGTM: Capability property correctly gates on both config and runtime state.
supports_sparse_searchrequires both the config flag and a non-null_qdrant_client, ensuring it returnsFalseif sparse setup failed during connect.
388-421: LGTM: Non-fatal sparse upsert with proper error handling.The
_try_sparse_upserthelper:
- Guards on encoder and client availability
- Re-raises system-level errors
- Logs and suppresses other failures to avoid blocking dense operations
This aligns with the coding guideline to handle errors explicitly without silently swallowing them.
810-861: LGTM: Sparse retrieval method with comprehensive validation.The
retrieve_sparsemethod:
- Checks
supports_sparse_searchand encoder availability- Validates connection and agent_id
- Handles empty query text gracefully
- Returns empty results for empty query vectors
- Properly wraps and raises
MemoryRetrievalErrorfor failuresdocs/design/memory.md (3)
631-656: LGTM: Pipeline documentation accurately reflects implementation.The Linear and RRF hybrid pipeline documentation correctly describes:
- Dense-only retrieval for LINEAR strategy
- Parallel dense + sparse BM25 retrieval for RRF
- Post-fusion
min_relevancefiltering- Opt-in via
sparse_search_enabledconfig
699-719: LGTM: Tool-Based Retrieval section accurately documents implementation status.The documentation correctly reflects:
- Implemented via
ToolBasedInjectionStrategy- Exposed tools (
search_memory,recall_memory)- Delegation to
MemoryBackend.retrieve()- Query reformulation protocols exist but not yet wired
- MCP bypass for memory hot path
736-736: Documentation signature matches implementation.The
query_text: NotBlankStrtype in the protocol signature aligns with the implementation inretriever.pyandtool_retriever.py.src/synthorg/observability/events/memory.py (2)
88-97: LGTM: Sparse search event constants follow established patterns.The new constants use the
memory.sparse.<action>naming convention consistent with the existingmemory.<entity>.<action>pattern. Coverage includes the full lifecycle: field setup, upsert, search, and degradation events.
99-102: LGTM: Query reformulation event constants appropriately scoped.The reformulation and sufficiency check failure events follow the naming convention and will support observability for the LLM-based implementations in
reformulation.py.src/synthorg/memory/tool_retriever.py (7)
40-75: LGTM: Tool schemas follow JSON Schema conventions.The
search_memoryandrecall_memoryschemas correctly define:
- Required properties
- Type constraints
- Descriptive help text for LLM consumption
- Sensible defaults and bounds for
limit
93-113: LGTM: Graceful category parsing with debug logging.Invalid category values are skipped with debug-level logging rather than failing the entire request. This follows defensive coding principles for LLM-generated inputs.
134-141: LGTM: Robust limit validation and clamping.The limit handling:
- Guards against boolean coercion (
isinstance(limit_raw, bool))- Falls back to default for non-numeric types
- Clamps to
[1, min(50, config.max_memories)]
169-182: LGTM:__slots__usage and constructor parity.Using
__slots__reduces memory overhead. The unusedtoken_estimatorandmemory_filterparameters maintain constructor parity withContextInjectionStrategyfor factory compatibility.
218-232: LGTM: Deep copy of schemas prevents mutation.Using
copy.deepcopy()on the schema dictionaries ensures immutability at system boundaries, per coding guidelines for non-Pydantic collections.
280-309: LGTM: Error handling returns generic messages to LLM.Domain errors and unexpected exceptions return fixed user-facing strings without leaking internal details:
"Memory search is temporarily unavailable.""Memory search encountered an unexpected error."This follows the PR objective for safe error handling with no internal detail leakage.
290-291: PEP 758 exception syntax correctly applied.The
except builtins.MemoryError, RecursionError:syntax is correct for Python 3.14.src/synthorg/memory/backends/mem0/sparse_search.py (6)
34-75: LGTM: Idempotent sparse field creation with IDF modifier.The
ensure_sparse_fieldfunction:
- Checks for existing sparse field before creation
- Uses
Modifier.IDFfor server-side IDF scoring- Logs appropriately for both "skipped" and "created" cases
78-121: LGTM: Sparse vector upsert with empty vector guard.The
upsert_sparse_vectorfunction correctly:
- Skips empty vectors silently (no point in upserting zero terms)
- Uses
update_vectorsto add sparse field without replacing dense- Logs completion with term count for observability
124-176: LGTM: Sparse search with user_id filtering.The
search_sparsefunction:
- Guards against empty query vectors
- Applies
user_idfilter viaFieldCondition- Returns raw points for caller to convert
179-221: LGTM: Batch conversion with graceful degradation.The
scored_points_to_entriesfunction:
- Re-raises system-level errors (
MemoryError,RecursionError)- Skips malformed points with per-point logging
- Emits batch-level degradation event with recovery statistics
- Uses
logger.errorwhen all points failed,logger.warningotherwise
255-279: LGTM: Malformedcreated_athandling addressed.The
_parse_created_atfunction now includes atry/except ValueErrorblock fordatetime.fromisoformat(), falling back to the epoch sentinel and logging the malformed value. This addresses the previous review concern about unhandled parsing exceptions.
282-317: LGTM: Point-to-entry conversion with score clamping.The
_point_to_entryfunction:
- Extracts content from either
dataormemorypayload keys- Raises
ValueErrorfor empty content (appropriate for batch handler to catch)- Clamps relevance score to
[0.0, 1.0]for consistency with ranking pipeline
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/memory/retriever.py`:
- Around line 459-467: The condition for building shared_coro redundantly checks
shared_store is not None even though shared_has_sparse already implies
shared_store is not None; simplify the expression by removing the `and
shared_store is not None` clause so shared_coro is created when
`shared_has_sparse` is true (using
`_safe_call(shared_store.retrieve_sparse(...), source="sparse_shared",
agent_id=agent_id)`) and otherwise `_empty()` is used; keep the existing
references to `shared_coro`, `shared_has_sparse`,
`shared_store.retrieve_sparse`, `_safe_call`, and `_empty`.
In `@tests/unit/memory/test_sparse.py`:
- Around line 103-108: The test test_unicode_normalization_and_casefold should
include the German sharp-s (ß) to ensure casefolding converts it to "ss"; update
the BM25Tokenizer tokenize input to include a word with ß (e.g., "Straße" or
include both "Straße STRASSE agent") and add an assertion that "strasse" is
present in the returned tokens (use BM25Tokenizer.tokenize and assert the
normalized token), keeping the existing checks for "agent" and existing
normalization behavior.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: cbdd324c-82e6-42b4-8aa6-706ecb0518f2
📒 Files selected for processing (6)
docs/architecture/tech-stack.mdsrc/synthorg/memory/retrieval_config.pysrc/synthorg/memory/retriever.pysrc/synthorg/memory/sparse.pytests/unit/memory/test_retrieval_config.pytests/unit/memory/test_sparse.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
docs/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Documentation files in docs/ are Markdown, built with Zensical, configured in mkdocs.yml; design spec in docs/design/ (12 pages), Architecture in docs/architecture/, Roadmap in docs/roadmap/
Files:
docs/architecture/tech-stack.md
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotationsin Python code; Python 3.14 has PEP 649 native lazy annotations
Use PEP 758 except syntax: useexcept A, B:(no parentheses) in Python 3.14; ruff enforces this
All public functions in Python must have type hints; mypy strict mode enforced
Use Google-style docstrings on public classes and functions in Python; enforced by ruff D rules
Create new objects and never mutate existing ones; for non-Pydantic internal collections (registries, BaseTool), use copy.deepcopy() at construction + MappingProxyType wrapping for read-only enforcement
For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence)
Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves
Use Pydantic v2 (BaseModel, model_validator, computed_field, ConfigDict); use allow_inf_nan=False in all ConfigDict declarations to reject NaN/Inf in numeric fields at validation time
Use@computed_fieldfor derived values instead of storing + validating redundant fields in Pydantic models (e.g. TokenUsage.total_tokens)
Use NotBlankStr from core.types for all identifier/name fields in Python (including optional and tuple variants) instead of manual whitespace validators
Prefer asyncio.TaskGroup for fan-out/fan-in parallel operations in Python (e.g. multiple tool invocations, parallel agent calls); prefer structured concurrency over bare create_task
Python line length must not exceed 88 characters; enforced by ruff
Python functions must be under 50 lines; files must be under 800 lines
Handle errors explicitly in Python; never silently swallow exceptions
Validate at system boundaries in Python (user input, external APIs, config files)
Files:
tests/unit/memory/test_retrieval_config.pysrc/synthorg/memory/retrieval_config.pysrc/synthorg/memory/retriever.pysrc/synthorg/memory/sparse.pytests/unit/memory/test_sparse.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: All Python test files must use@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, or@pytest.mark.slowmarkers
Python tests must maintain 80% minimum code coverage (enforced in CI)
Prefer@pytest.mark.parametrizefor testing similar cases in Python
Use test-provider, test-small-001, etc. in Python tests instead of real vendor names
Property-based testing in Python uses Hypothesis (@given+@settings); profiles: ci (50 examples, default) and dev (1000 examples), controlled via HYPOTHESIS_PROFILE env var
Never skip, dismiss, or ignore flaky Python tests; always fix them fully and fundamentally; for timing-sensitive tests, mock time.monotonic() and asyncio.sleep() to make them deterministic instead of widening timing margins
For Python tasks that must block indefinitely until cancelled (e.g. simulating a slow provider or stubborn coroutine), use asyncio.Event().wait() instead of asyncio.sleep(large_number) -- it is cancellation-safe and carries no timing assumptions
Files:
tests/unit/memory/test_retrieval_config.pytests/unit/memory/test_sparse.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every Python module with business logic must have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging/logging.getLogger()/print()in Python application code; exceptions are observability/setup.py, observability/sinks.py, observability/syslog_handler.py, and observability/http_handler.py
Python logger variable name must always belogger(not_logger, notlog)
Use event name constants from domain-specific modules under synthorg.observability.events (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool); import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANT
Use structured logging with kwargs in Python: alwayslogger.info(EVENT, key=value)-- neverlogger.info('msg %s', val)
All error paths in Python must log at WARNING or ERROR with context before raising
All state transitions in Python must log at INFO level
Use DEBUG logging level in Python for object creation, internal flow, entry/exit of key functions
Pure data models, enums, and re-exports in Python do NOT need logging
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned Python code, docstrings, comments, tests, or config examples; use generic names: example-provider, example-large-001, example-medium-001, example-small-001, large/medium/small as aliases
Files:
src/synthorg/memory/retrieval_config.pysrc/synthorg/memory/retriever.pysrc/synthorg/memory/sparse.py
🧠 Learnings (22)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/memory/**/*.py : Memory package (memory/): pluggable MemoryBackend protocol, backends/ (Mem0 adapter), retrieval pipeline (ranking, RRF fusion, injection, formatting, non-inferable filtering), shared org memory (org/), consolidation/archival (density-aware: DensityClassifier, AbstractiveSummarizer, ExtractivePreserver, DualModeConsolidationStrategy)
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/memory/**/*.py : Memory package (memory/): pluggable MemoryBackend protocol, backends/ (Mem0 adapter), retrieval pipeline (ranking, RRF fusion, injection, formatting, non-inferable filtering), shared org memory (org/), consolidation/archival (density-aware: DensityClassifier, AbstractiveSummarizer, ExtractivePreserver, DualModeConsolidationStrategy)
Applied to files:
docs/architecture/tech-stack.mdsrc/synthorg/memory/retrieval_config.pysrc/synthorg/memory/retriever.pysrc/synthorg/memory/sparse.py
📚 Learning: 2026-03-16T06:24:56.341Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:24:56.341Z
Learning: Applies to src/synthorg/memory/**/*.py : Use MemoryBackend protocol with pluggable backends (Mem0 adapter available at backends/mem0/) for persistent agent memory
Applied to files:
docs/architecture/tech-stack.md
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Engine: Agent orchestration, execution loops, parallel execution, task decomposition, routing, task assignment, centralized single-writer task state engine (TaskEngine), task lifecycle, recovery, shutdown, workspace isolation, coordination (multi-agent pipeline: TopologyDispatcher protocol, 4 dispatchers — SAS/centralized/decentralized/context-dependent, wave execution, workspace lifecycle integration, CoordinationSectionConfig company config bridge, build_coordinator factory), coordination error classification, prompt policy validation, checkpoint recovery (checkpoint/, per-turn persistence, heartbeat detection, CheckpointRecoveryStrategy), approval gate (escalation detection, context parking/resume, EscalationInfo/ResumePayload models), stagnation detection (stagnation/, StagnationDetector protocol, ToolRepetitionDetector, dual-signal analysis, corrective prompt injection), agent runtime state (AgentRuntimeState, lightweight per-agent execution status for dashboard queries and recove...
Applied to files:
docs/architecture/tech-stack.md
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to pyproject.toml : Required runtime dependencies: `mem0ai` (default and currently only memory backend), `cryptography` (Fernet encryption), `faker` (multi-locale name generation)
Applied to files:
docs/architecture/tech-stack.md
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Documentation source in `docs/` (Markdown, built with Zensical). Design spec in `docs/design/` (7 pages: index, agents, organization, communication, engine, memory, operations). Architecture in `docs/architecture/` (overview, tech-stack, decision log). Roadmap in `docs/roadmap/`. Security in `docs/security.md`. Licensing in `docs/licensing.md`. Reference in `docs/reference/`. REST API reference in `docs/rest-api.md`. Library reference in `docs/api/` (auto-generated from docstrings). Custom templates in `docs/overrides/`. Config in `mkdocs.yml`.
Applied to files:
docs/architecture/tech-stack.md
📚 Learning: 2026-04-02T18:54:07.757Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T18:54:07.757Z
Learning: Applies to **/*.py : Prefer asyncio.TaskGroup for fan-out/fan-in parallel operations in Python (e.g. multiple tool invocations, parallel agent calls); prefer structured concurrency over bare create_task
Applied to files:
src/synthorg/memory/retriever.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to **/*.py : Prefer `asyncio.TaskGroup` for fan-out/fan-in parallel operations in new code (e.g., multiple tool invocations, parallel agent calls); prefer structured concurrency over bare `create_task`
Applied to files:
src/synthorg/memory/retriever.py
📚 Learning: 2026-03-20T11:18:48.128Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T11:18:48.128Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic MUST have `from synthorg.observability import get_logger` followed by `logger = get_logger(__name__)`.
Applied to files:
src/synthorg/memory/sparse.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`. Never use `import logging` / `logging.getLogger()` / `print()` in application code. Variable name: always `logger` (not `_logger`, not `log`).
Applied to files:
src/synthorg/memory/sparse.py
📚 Learning: 2026-03-17T06:43:14.114Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:43:14.114Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`. Never use `import logging` / `logging.getLogger()` / `print()` in application code. Variable name: always `logger`.
Applied to files:
src/synthorg/memory/sparse.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`
Applied to files:
src/synthorg/memory/sparse.py
📚 Learning: 2026-04-02T18:54:07.757Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T18:54:07.757Z
Learning: Applies to src/synthorg/**/*.py : Every Python module with business logic must have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`
Applied to files:
src/synthorg/memory/sparse.py
📚 Learning: 2026-03-19T11:33:01.580Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:33:01.580Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic must import logger via `from synthorg.observability import get_logger` and initialize with `logger = get_logger(__name__)`
Applied to files:
src/synthorg/memory/sparse.py
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic must import `from synthorg.observability import get_logger` and define `logger = get_logger(__name__)`
Applied to files:
src/synthorg/memory/sparse.py
📚 Learning: 2026-03-15T19:14:27.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T19:14:27.144Z
Learning: Applies to src/synthorg/**/*.py : Every module with business logic MUST have: `from synthorg.observability import get_logger` then `logger = get_logger(__name__)`. Never use import logging / logging.getLogger() / print() in application code.
Applied to files:
src/synthorg/memory/sparse.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to tests/**/*.py : Use Hypothesis for property-based testing: `given` + `settings`; dev profile has 1000 examples (via `HYPOTHESIS_PROFILE=dev env var`), ci profile (default) has 50
Applied to files:
tests/unit/memory/test_sparse.py
📚 Learning: 2026-04-02T18:54:07.757Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T18:54:07.757Z
Learning: Applies to tests/**/*.py : Property-based testing in Python uses Hypothesis (given + settings); profiles: ci (50 examples, default) and dev (1000 examples), controlled via HYPOTHESIS_PROFILE env var
Applied to files:
tests/unit/memory/test_sparse.py
📚 Learning: 2026-03-15T18:42:17.990Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:42:17.990Z
Learning: Applies to tests/**/*.py : Use Hypothesis for property-based testing with `given` + `settings` decorators; control profiles via `HYPOTHESIS_PROFILE` env var (`ci` for 200 examples, `dev` for 1000 examples)
Applied to files:
tests/unit/memory/test_sparse.py
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to tests/**/*.py : Use Hypothesis for property-based testing in Python with `given` + `settings`. Control via `HYPOTHESIS_PROFILE` env var (dev: 1000 examples, ci: 200 examples).
Applied to files:
tests/unit/memory/test_sparse.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Property-based testing: Python uses Hypothesis (given + settings). Hypothesis profiles: ci (200 examples, default) and dev (1000 examples), controlled via HYPOTHESIS_PROFILE env var. Run dev profile: HYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n auto -k properties.
Applied to files:
tests/unit/memory/test_sparse.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Run property tests with `HYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n auto -k properties`
Applied to files:
tests/unit/memory/test_sparse.py
🪛 LanguageTool
docs/architecture/tech-stack.md
[grammar] ~53-~53: Ensure spelling is correct
Context: ...ifier.IDF` for server-side IDF scoring. Qdrant embedded + SQLite for persistence. Cust...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (21)
docs/architecture/tech-stack.md (1)
53-53: LGTM! Documentation now correctly explains both config switches.The updated wording properly clarifies that hybrid retrieval requires both
sparse_search_enabled(for sparse indexing) andfusion_strategy=rrf(for the retriever), addressing the previous feedback about configuration clarity.src/synthorg/memory/retrieval_config.py (4)
115-131: Well-structured reserved fields with clear documentation.The
query_reformulation_enabledandmax_reformulation_roundsfields are properly marked as reserved with descriptive docstrings explaining their future purpose. The bounds validation (ge=1, le=5) is appropriate.
186-204: Correctly guards against default-value warning noise.The
"personal_boost" in self.model_fields_setcheck ensures warnings only fire when the user explicitly setspersonal_boostwith RRF fusion, preventing false alarms on default configurations. This properly addresses the previous feedback.
169-184: Clear rejection of unimplemented feature.The validator properly rejects
query_reformulation_enabled=Truewith an informative error message, preventing misconfiguration before the pipeline wiring is complete.
206-222: Strategy validation correctly expanded for TOOL_BASED.The supported strategies set now includes both
CONTEXTandTOOL_BASED, and the error message dynamically lists supported values for clarity.tests/unit/memory/test_retrieval_config.py (4)
127-133: Good coverage for strategy acceptance/rejection.Tests properly verify that
TOOL_BASEDis now accepted whileSELF_EDITINGremains rejected with the expected error message.
166-179: Test now properly asserts weight preservation.Lines 178-179 confirm that custom weights are stored unchanged when using RRF fusion, addressing the previous feedback about ensuring validators don't silently reset values.
218-246: Comprehensive reformulation config test coverage.Tests cover default values, rejection of enabled state, and boundary validation for
max_reformulation_rounds. The bounds tests at lines 241-245 correctly verify both boundaries (1 and 5).
248-287: Thorough warning behavior verification.The three tests properly distinguish between:
- Explicit
personal_boostwith RRF → warning expected- Default
personal_boostwith RRF → no warning- Zero
personal_boostwith RRF → no warningThis validates the
model_fields_setcheck in the validator.src/synthorg/memory/retriever.py (4)
76-77: Correct PEP 758 except syntax for system-level exceptions.The
except builtins.MemoryError, RecursionError:syntax properly re-raises system-level errors while allowing domain errors to be caught separately.
345-410: Well-structured RRF pipeline with proper parallel execution.The implementation correctly:
- Uses
asyncio.TaskGroupfor structured concurrency (lines 370-372)- Sorts personal+shared by
relevance_scorebefore RRF fusion (lines 383-396), eliminating position bias- Applies post-fusion
min_relevancefilter (lines 408-410)- Unwraps system errors from
ExceptionGroup(lines 373-376)
412-477: Sparse retrieval correctly decouples personal and shared capabilities.The implementation properly checks
personal_has_sparseandshared_has_sparseindependently (lines 431-441), only returning early when neither supports sparse search (lines 443-444). This addresses the previous feedback about not gating shared sparse on personal backend capability.
316-343: Clean extraction of linear pipeline logic.The
_execute_linear_pipelinemethod properly encapsulates the dense-only retrieval path, maintaining the existingrank_memoriesbehavior with personal boost applied.src/synthorg/memory/sparse.py (4)
1-19: Proper module setup with logger and clear documentation.The module correctly imports the observability logger and provides comprehensive documentation about the BM25 sparse encoding approach (vocabulary-free hashing, Qdrant IDF scoring).
85-134: Robust SparseVector validation.The model validator comprehensively checks:
- Length equality between indices and values
- Strictly positive values (>0)
- Non-negative indices
- Strictly ascending index order
The
is_emptyproperty provides convenient emptiness checking.
164-172: Correct Unicode normalization with NFKC + casefold.The implementation properly normalizes text using
unicodedata.normalize("NFKC", text).casefold()before tokenization, ensuring consistent handling of Unicode variants (e.g., "Straße" and "STRASSE" both normalize to "strasse").
174-203: Deterministic sparse encoding with mmh3.The
encodemethod correctly:
- Uses
mmh3.hash(token, signed=False)for unsigned 32-bit indices- Counts term frequencies with
Counter- Sorts by index for consistent representation
- Returns empty
SparseVector()for empty/stop-word-only inputtests/unit/memory/test_sparse.py (4)
16-57: Comprehensive SparseVector validation test coverage.Tests cover all validation paths: empty vectors, frozen immutability, length mismatch, negative/zero values, negative indices, unsorted indices, and duplicate indices. The
is_emptyproperty is also verified.
192-248: Property tests correctly use Hypothesis profiles.The
@givendecorators without hard-coded@settings(max_examples=...)allow example counts to be driven by theHYPOTHESIS_PROFILEenvironment variable (ci: 50, dev: 1000), addressing the previous feedback.
62-121: Thorough tokenization behavior coverage.Tests verify basic tokenization, empty/whitespace handling, punctuation splitting, stop word filtering (and disabling), Unicode normalization, number preservation, and duplicate token preservation.
126-187: Solid encoding behavior tests.Tests cover empty input, stop-word-only input, determinism, different texts producing different vectors, sorted indices, positive values, term frequency counts, and non-empty output for meaningful content.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/memory/retriever.py`:
- Around line 197-200: Before re-raising the propagating system exceptions, add
a structured log entry (WARNING or ERROR) that includes the agent_id and stage
context and the exception details; specifically, in retriever.py update the
except builtins.MemoryError and except RecursionError handlers (the blocks that
currently just "raise") to call the module/class logger (e.g., self.logger or
module logger) with a clear message and exc_info=True (or pass the exception)
before executing raise; apply the same change to the equivalent handlers at the
other two occurrences mentioned (around the blocks at the other noted
locations).
- Around line 345-410: The _execute_rrf_pipeline function is too large; extract
the TaskGroup coroutine orchestration and the dense/sparse
merging+sorting/filtering into two small private helpers: e.g., create a helper
_run_parallel_fetches(agent_id, query) that runs self._fetch_memories and
self._fetch_sparse_memories within an asyncio.TaskGroup and returns the two task
results, and a helper _merge_and_filter_modalities(dense_personal, dense_shared,
sparse_personal, sparse_shared) that builds sorted dense_list/sparse_list, calls
fuse_ranked_lists with self._config.rrf_k and self._config.max_memories, and
applies the post-RRF min_relevance filter; then call these from
_execute_rrf_pipeline. Apply the same refactor pattern to the similar large
block in _fetch_sparse_memories (lines 412-477) by extracting its TaskGroup
usage and any long modality-processing logic into small helpers with clear
names.
- Around line 437-466: The shared sparse branch calls
shared_store.retrieve_sparse which doesn't exist on SharedKnowledgeStore; to
avoid attribute/argument errors canceling other tasks, disable shared sparse
until the shared-store contract supports it: modify the shared_has_sparse
computation (symbol: shared_has_sparse) so it never enables the shared branch
(e.g., remove the getattr(... "supports_sparse_search", False) check or force
shared_has_sparse = False) and ensure shared_coro falls back to _empty() rather
than calling shared_store.retrieve_sparse; alternatively, if you prefer enabling
it now, implement a matching API on SharedKnowledgeStore (add retrieve_sparse
with signature matching backend.retrieve_sparse) and update callers to use that
method consistently (symbols: SharedKnowledgeStore, retrieve_sparse,
search_shared, shared_coro).
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4c32ff30-d196-4965-b663-7a82aa68175c
📒 Files selected for processing (2)
src/synthorg/memory/retriever.pytests/unit/memory/test_sparse.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Backend
- GitHub Check: Build Web
- GitHub Check: Build Sandbox
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotationsin Python code; Python 3.14 has PEP 649 native lazy annotations
Use PEP 758 except syntax: useexcept A, B:(no parentheses) in Python 3.14; ruff enforces this
All public functions in Python must have type hints; mypy strict mode enforced
Use Google-style docstrings on public classes and functions in Python; enforced by ruff D rules
Create new objects and never mutate existing ones; for non-Pydantic internal collections (registries, BaseTool), use copy.deepcopy() at construction + MappingProxyType wrapping for read-only enforcement
For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence)
Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (using model_copy(update=...)) for runtime state that evolves
Use Pydantic v2 (BaseModel, model_validator, computed_field, ConfigDict); use allow_inf_nan=False in all ConfigDict declarations to reject NaN/Inf in numeric fields at validation time
Use@computed_fieldfor derived values instead of storing + validating redundant fields in Pydantic models (e.g. TokenUsage.total_tokens)
Use NotBlankStr from core.types for all identifier/name fields in Python (including optional and tuple variants) instead of manual whitespace validators
Prefer asyncio.TaskGroup for fan-out/fan-in parallel operations in Python (e.g. multiple tool invocations, parallel agent calls); prefer structured concurrency over bare create_task
Python line length must not exceed 88 characters; enforced by ruff
Python functions must be under 50 lines; files must be under 800 lines
Handle errors explicitly in Python; never silently swallow exceptions
Validate at system boundaries in Python (user input, external APIs, config files)
Files:
src/synthorg/memory/retriever.pytests/unit/memory/test_sparse.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every Python module with business logic must have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging/logging.getLogger()/print()in Python application code; exceptions are observability/setup.py, observability/sinks.py, observability/syslog_handler.py, and observability/http_handler.py
Python logger variable name must always belogger(not_logger, notlog)
Use event name constants from domain-specific modules under synthorg.observability.events (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool); import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANT
Use structured logging with kwargs in Python: alwayslogger.info(EVENT, key=value)-- neverlogger.info('msg %s', val)
All error paths in Python must log at WARNING or ERROR with context before raising
All state transitions in Python must log at INFO level
Use DEBUG logging level in Python for object creation, internal flow, entry/exit of key functions
Pure data models, enums, and re-exports in Python do NOT need logging
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned Python code, docstrings, comments, tests, or config examples; use generic names: example-provider, example-large-001, example-medium-001, example-small-001, large/medium/small as aliases
Files:
src/synthorg/memory/retriever.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: All Python test files must use@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, or@pytest.mark.slowmarkers
Python tests must maintain 80% minimum code coverage (enforced in CI)
Prefer@pytest.mark.parametrizefor testing similar cases in Python
Use test-provider, test-small-001, etc. in Python tests instead of real vendor names
Property-based testing in Python uses Hypothesis (@given+@settings); profiles: ci (50 examples, default) and dev (1000 examples), controlled via HYPOTHESIS_PROFILE env var
Never skip, dismiss, or ignore flaky Python tests; always fix them fully and fundamentally; for timing-sensitive tests, mock time.monotonic() and asyncio.sleep() to make them deterministic instead of widening timing margins
For Python tasks that must block indefinitely until cancelled (e.g. simulating a slow provider or stubborn coroutine), use asyncio.Event().wait() instead of asyncio.sleep(large_number) -- it is cancellation-safe and carries no timing assumptions
Files:
tests/unit/memory/test_sparse.py
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/memory/**/*.py : Memory package (memory/): pluggable MemoryBackend protocol, backends/ (Mem0 adapter), retrieval pipeline (ranking, RRF fusion, injection, formatting, non-inferable filtering), shared org memory (org/), consolidation/archival (density-aware: DensityClassifier, AbstractiveSummarizer, ExtractivePreserver, DualModeConsolidationStrategy)
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/memory/**/*.py : Memory package (memory/): pluggable MemoryBackend protocol, backends/ (Mem0 adapter), retrieval pipeline (ranking, RRF fusion, injection, formatting, non-inferable filtering), shared org memory (org/), consolidation/archival (density-aware: DensityClassifier, AbstractiveSummarizer, ExtractivePreserver, DualModeConsolidationStrategy)
Applied to files:
src/synthorg/memory/retriever.py
📚 Learning: 2026-04-02T18:54:07.757Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T18:54:07.757Z
Learning: Applies to **/*.py : Prefer asyncio.TaskGroup for fan-out/fan-in parallel operations in Python (e.g. multiple tool invocations, parallel agent calls); prefer structured concurrency over bare create_task
Applied to files:
src/synthorg/memory/retriever.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to **/*.py : Prefer `asyncio.TaskGroup` for fan-out/fan-in parallel operations in new code (e.g., multiple tool invocations, parallel agent calls); prefer structured concurrency over bare `create_task`
Applied to files:
src/synthorg/memory/retriever.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to tests/**/*.py : Use Hypothesis for property-based testing: `given` + `settings`; dev profile has 1000 examples (via `HYPOTHESIS_PROFILE=dev env var`), ci profile (default) has 50
Applied to files:
tests/unit/memory/test_sparse.py
📚 Learning: 2026-04-02T18:54:07.757Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T18:54:07.757Z
Learning: Applies to tests/**/*.py : Property-based testing in Python uses Hypothesis (given + settings); profiles: ci (50 examples, default) and dev (1000 examples), controlled via HYPOTHESIS_PROFILE env var
Applied to files:
tests/unit/memory/test_sparse.py
📚 Learning: 2026-03-15T18:42:17.990Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:42:17.990Z
Learning: Applies to tests/**/*.py : Use Hypothesis for property-based testing with `given` + `settings` decorators; control profiles via `HYPOTHESIS_PROFILE` env var (`ci` for 200 examples, `dev` for 1000 examples)
Applied to files:
tests/unit/memory/test_sparse.py
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to tests/**/*.py : Use Hypothesis for property-based testing in Python with `given` + `settings`. Control via `HYPOTHESIS_PROFILE` env var (dev: 1000 examples, ci: 200 examples).
Applied to files:
tests/unit/memory/test_sparse.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Property-based testing: Python uses Hypothesis (given + settings). Hypothesis profiles: ci (200 examples, default) and dev (1000 examples), controlled via HYPOTHESIS_PROFILE env var. Run dev profile: HYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n auto -k properties.
Applied to files:
tests/unit/memory/test_sparse.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Run property tests with `HYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n auto -k properties`
Applied to files:
tests/unit/memory/test_sparse.py
🔇 Additional comments (8)
tests/unit/memory/test_sparse.py (7)
103-108: Comment mentions sharp-s (ß) but test input doesn't include it.The comment states "casefold converts sharp-s to 'ss'" but the input
"agent STRASSE"doesn't contain ß. Consider using"Straße"to actually exercise the casefold behavior, or update the comment to accurately describe what's being tested.
1-12: LGTM!Clean imports with appropriate test dependencies. Module docstring present.
16-57: LGTM!Comprehensive test coverage for
SparseVectorvalidation rules: frozen immutability, length mismatch, positive values constraint, non-negative indices, and sorted/unique indices. Theis_emptyproperty is properly tested for both states.
62-102: LGTM!Solid tokenization test coverage including edge cases (empty, whitespace-only), punctuation handling, stop word filtering with toggle, and numeric preservation.
110-121: LGTM!Good coverage for numeric token preservation and duplicate token handling.
126-187: LGTM!Thorough coverage of
BM25Tokenizer.encodebehavior including edge cases, determinism, uniqueness, invariants (sorted indices, positive values, non-negative indices), and term frequency mechanics.
192-248: LGTM!Well-designed property-based tests using Hypothesis with appropriate strategies. The letter-only alphabet with
remove_stop_words=Falseensures meaningful test inputs. Correctly relies onHYPOTHESIS_PROFILEenv var for example counts rather than hardcoding.src/synthorg/memory/retriever.py (1)
381-409: Good fix: modality-local ordering and post-RRF filtering are correct here.Sorting personal+shared results before
fuse_ranked_lists(...)removes the personal-before-shared position bias, and filtering oncombined_scorematches the RRF contract insrc/synthorg/memory/ranking.py.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/unit/memory/test_sparse.py`:
- Around line 17-251: Tests lack Google-style docstrings on public classes and
functions (triggers ruff D rules); add concise Google-style docstrings to each
public test class (e.g., TestSparseVector, TestTokenize, TestEncode,
TestSparseProperties) and to any public test functions that are intended to be
part of the module's public API (e.g., test_empty_vector,
test_basic_tokenization, test_empty_string_returns_empty_vector, the
property-based tests), following the repo pattern: one-line summary followed by
an optional short description and example parameters/expected behavior where
helpful; ensure docstrings use triple-quoted strings directly under the
class/function definitions and keep wording minimal and consistent across the
file.
- Around line 197-251: Add an import for settings from hypothesis and decorate
each property test with `@settings`(): update the five test functions
test_encoding_is_deterministic, test_non_empty_alpha_produces_non_empty_vector,
test_indices_always_sorted, test_values_always_positive, and
test_indices_always_non_negative so they each have a `@settings`() decorator
directly above their existing `@given`(...) decorator; keep the BM25Tokenizer
usage and existing given strategies unchanged so Hypothesis profiles
(HYPOTHESIS_PROFILE) control example counts.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ad115d2f-46a9-4559-9640-b71f7b2777f6
📒 Files selected for processing (1)
tests/unit/memory/test_sparse.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotationsin Python code -- Python 3.14 has PEP 649
Use PEP 758 except syntax:except A, B:(no parentheses) -- ruff enforces this on Python 3.14
All public functions must have type hints and pass mypy strict mode
Docstrings must use Google style and are required on all public classes/functions (enforced by ruff D rules)
Create new objects instead of mutating existing ones. For non-Pydantic internal collections (registries,BaseTool), usecopy.deepcopy()at construction +MappingProxyTypewrapping for read-only enforcement.
Fordict/listfields in frozen Pydantic models, rely onfrozen=Truefor field reassignment prevention andcopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).
Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (usingmodel_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.
Use Pydantic v2 (BaseModel,model_validator,computed_field,ConfigDict). Always useallow_inf_nan=FalseinConfigDictdeclarations to rejectNaN/Infin numeric fields at validation time.
Use@computed_fieldfor derived values instead of storing + validating redundant fields (e.g.TokenUsage.total_tokens)
UseNotBlankStr(fromcore.types) for all identifier/name fields -- including optional (NotBlankStr | None) and tuple (tuple[NotBlankStr, ...]) variants -- instead of manual whitespace validators.
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over barecreate_task.
Maximum line length: 88 characters (enforced by ruff)
Functions must be less than 50 lines, files must be less than 800 lines
Files:
tests/unit/memory/test_sparse.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: All test files must use markers:@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slow
Async tests: useasyncio_mode = "auto"-- no manual@pytest.mark.asyncioneeded
Test timeout: 30 seconds per test (global inpyproject.toml-- do not add per-filepytest.mark.timeout(30)markers; non-default overrides liketimeout(60)are allowed)
Prefer@pytest.mark.parametrizefor testing similar cases
For property-based testing in Python, use Hypothesis (@given+@settings). Profiles:ci(50 examples, default) anddev(1000 examples). Run dev profile:HYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n 8 -k properties
Never skip, dismiss, or ignore flaky tests -- always fix them fully and fundamentally. For timing-sensitive tests, mocktime.monotonic()andasyncio.sleep(). For tasks that must block indefinitely, useasyncio.Event().wait()instead ofasyncio.sleep(large_number).
Files:
tests/unit/memory/test_sparse.py
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/memory/**/*.py : Memory package (memory/): pluggable MemoryBackend protocol, backends/ (Mem0 adapter), retrieval pipeline (ranking, RRF fusion, injection, formatting, non-inferable filtering), shared org memory (org/), consolidation/archival (density-aware: DensityClassifier, AbstractiveSummarizer, ExtractivePreserver, DualModeConsolidationStrategy)
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to tests/**/*.py : Use Hypothesis for property-based testing: `given` + `settings`; dev profile has 1000 examples (via `HYPOTHESIS_PROFILE=dev env var`), ci profile (default) has 50
Applied to files:
tests/unit/memory/test_sparse.py
📚 Learning: 2026-04-03T07:34:51.012Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-03T07:34:51.012Z
Learning: Applies to tests/**/*.py : For property-based testing in Python, use Hypothesis (`given` + `settings`). Profiles: `ci` (50 examples, default) and `dev` (1000 examples). Run dev profile: `HYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n 8 -k properties`
Applied to files:
tests/unit/memory/test_sparse.py
📚 Learning: 2026-03-15T18:42:17.990Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:42:17.990Z
Learning: Applies to tests/**/*.py : Use Hypothesis for property-based testing with `given` + `settings` decorators; control profiles via `HYPOTHESIS_PROFILE` env var (`ci` for 200 examples, `dev` for 1000 examples)
Applied to files:
tests/unit/memory/test_sparse.py
📚 Learning: 2026-03-17T06:30:14.180Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:30:14.180Z
Learning: Applies to tests/**/*.py : Use Hypothesis for property-based testing in Python with `given` + `settings`. Control via `HYPOTHESIS_PROFILE` env var (dev: 1000 examples, ci: 200 examples).
Applied to files:
tests/unit/memory/test_sparse.py
📚 Learning: 2026-03-15T18:28:13.207Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:28:13.207Z
Learning: Property-based testing: Python uses Hypothesis (given + settings). Hypothesis profiles: ci (200 examples, default) and dev (1000 examples), controlled via HYPOTHESIS_PROFILE env var. Run dev profile: HYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n auto -k properties.
Applied to files:
tests/unit/memory/test_sparse.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Run property tests with `HYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n auto -k properties`
Applied to files:
tests/unit/memory/test_sparse.py
🔇 Additional comments (2)
tests/unit/memory/test_sparse.py (2)
18-57: StrongSparseVectorinvariant coverage.These tests cover the key model constraints (frozen behavior, shape/order/positivity checks, and
is_empty) clearly and directly.
64-190: Tokenizer and encoder behavior coverage looks solid.Normalization, stop-word handling, determinism, TF semantics, and structural invariants are all validated with focused cases.
| class TestSparseVector: | ||
| def test_empty_vector(self) -> None: | ||
| vec = SparseVector(indices=(), values=()) | ||
| assert vec.indices == () | ||
| assert vec.values == () | ||
|
|
||
| def test_frozen(self) -> None: | ||
| vec = SparseVector(indices=(1, 2), values=(1.0, 2.0)) | ||
| with pytest.raises(ValidationError, match="frozen"): | ||
| vec.indices = (3,) # type: ignore[misc] | ||
|
|
||
| def test_length_mismatch_rejected(self) -> None: | ||
| with pytest.raises( | ||
| ValueError, match="indices and values must have equal length" | ||
| ): | ||
| SparseVector(indices=(1, 2), values=(1.0,)) | ||
|
|
||
| def test_negative_values_rejected(self) -> None: | ||
| with pytest.raises(ValueError, match="values must be positive"): | ||
| SparseVector(indices=(1,), values=(-0.5,)) | ||
|
|
||
| def test_zero_values_rejected(self) -> None: | ||
| with pytest.raises(ValueError, match="values must be positive"): | ||
| SparseVector(indices=(1,), values=(0.0,)) | ||
|
|
||
| def test_negative_indices_rejected(self) -> None: | ||
| with pytest.raises(ValueError, match="non-negative"): | ||
| SparseVector(indices=(-1,), values=(1.0,)) | ||
|
|
||
| def test_unsorted_indices_rejected(self) -> None: | ||
| with pytest.raises(ValueError, match="sorted"): | ||
| SparseVector(indices=(5, 3), values=(1.0, 2.0)) | ||
|
|
||
| def test_duplicate_indices_rejected(self) -> None: | ||
| with pytest.raises(ValueError, match="sorted"): | ||
| SparseVector(indices=(3, 3), values=(1.0, 2.0)) | ||
|
|
||
| def test_is_empty(self) -> None: | ||
| assert SparseVector(indices=(), values=()).is_empty | ||
| assert not SparseVector(indices=(1,), values=(1.0,)).is_empty | ||
|
|
||
|
|
||
| # ── BM25Tokenizer.tokenize ─────────────────────────────────────── | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| class TestTokenize: | ||
| def test_basic_tokenization(self) -> None: | ||
| tokenizer = BM25Tokenizer() | ||
| tokens = tokenizer.tokenize("Hello World") | ||
| assert tokens == ("hello", "world") | ||
|
|
||
| def test_empty_string(self) -> None: | ||
| tokenizer = BM25Tokenizer() | ||
| assert tokenizer.tokenize("") == () | ||
|
|
||
| def test_whitespace_only(self) -> None: | ||
| tokenizer = BM25Tokenizer() | ||
| assert tokenizer.tokenize(" \t\n ") == () | ||
|
|
||
| def test_punctuation_splitting(self) -> None: | ||
| tokenizer = BM25Tokenizer() | ||
| tokens = tokenizer.tokenize("hello, world! foo-bar_baz") | ||
| # Splits on non-word chars and underscores | ||
| assert "hello" in tokens | ||
| assert "world" in tokens | ||
| assert "foo" in tokens | ||
| assert "bar" in tokens | ||
| assert "baz" in tokens | ||
|
|
||
| def test_stop_words_excluded(self) -> None: | ||
| tokenizer = BM25Tokenizer() | ||
| tokens = tokenizer.tokenize("the agent is in a task") | ||
| assert "the" not in tokens | ||
| assert "is" not in tokens | ||
| assert "in" not in tokens | ||
| assert "a" not in tokens | ||
| assert "agent" in tokens | ||
| assert "task" in tokens | ||
|
|
||
| def test_stop_words_can_be_disabled(self) -> None: | ||
| tokenizer = BM25Tokenizer(remove_stop_words=False) | ||
| tokens = tokenizer.tokenize("the agent is in a task") | ||
| assert "the" in tokens | ||
| assert "is" in tokens | ||
|
|
||
| def test_unicode_normalization_and_casefold(self) -> None: | ||
| tokenizer = BM25Tokenizer() | ||
| # casefold: sharp-s -> "ss"; NFKC: fullwidth chars normalized | ||
| tokens_ss = tokenizer.tokenize("Stra\u00dfe agent") | ||
| assert "strasse" in tokens_ss | ||
| assert "agent" in tokens_ss | ||
| # Verify casefold unifies with plain ASCII form | ||
| tokens_plain = tokenizer.tokenize("STRASSE agent") | ||
| assert "strasse" in tokens_plain | ||
|
|
||
| def test_numbers_preserved(self) -> None: | ||
| tokenizer = BM25Tokenizer() | ||
| tokens = tokenizer.tokenize("error code 404") | ||
| assert "error" in tokens | ||
| assert "code" in tokens | ||
| assert "404" in tokens | ||
|
|
||
| def test_duplicate_tokens_preserved(self) -> None: | ||
| tokenizer = BM25Tokenizer() | ||
| tokens = tokenizer.tokenize("test test test") | ||
| assert tokens.count("test") == 3 | ||
|
|
||
|
|
||
| # ── BM25Tokenizer.encode ───────────────────────────────────────── | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| class TestEncode: | ||
| def test_empty_string_returns_empty_vector(self) -> None: | ||
| tokenizer = BM25Tokenizer() | ||
| vec = tokenizer.encode("") | ||
| assert vec.is_empty | ||
|
|
||
| def test_stop_words_only_returns_empty(self) -> None: | ||
| tokenizer = BM25Tokenizer() | ||
| vec = tokenizer.encode("the a is in on") | ||
| assert vec.is_empty | ||
|
|
||
| def test_deterministic(self) -> None: | ||
| tokenizer = BM25Tokenizer() | ||
| vec1 = tokenizer.encode("hello world") | ||
| vec2 = tokenizer.encode("hello world") | ||
| assert vec1 == vec2 | ||
|
|
||
| def test_different_texts_differ(self) -> None: | ||
| tokenizer = BM25Tokenizer() | ||
| vec1 = tokenizer.encode("authentication tokens") | ||
| vec2 = tokenizer.encode("database migrations") | ||
| assert vec1 != vec2 | ||
|
|
||
| def test_indices_sorted(self) -> None: | ||
| tokenizer = BM25Tokenizer() | ||
| vec = tokenizer.encode("zebra apple mango banana") | ||
| assert list(vec.indices) == sorted(vec.indices) | ||
|
|
||
| def test_all_values_positive(self) -> None: | ||
| tokenizer = BM25Tokenizer() | ||
| vec = tokenizer.encode("hello world test") | ||
| assert all(v > 0 for v in vec.values) | ||
|
|
||
| def test_repeated_token_increases_tf(self) -> None: | ||
| tokenizer = BM25Tokenizer() | ||
| vec_single = tokenizer.encode("error") | ||
| vec_triple = tokenizer.encode("error error error") | ||
| # Same index, but higher TF value for repeated token | ||
| assert vec_single.indices == vec_triple.indices | ||
| assert vec_triple.values[0] > vec_single.values[0] | ||
|
|
||
| def test_tf_values_are_counts(self) -> None: | ||
| tokenizer = BM25Tokenizer() | ||
| vec = tokenizer.encode("test test test unique") | ||
| # With 2 unique tokens: "test" (3x) and "unique" (1x) | ||
| values_sorted = sorted(vec.values, reverse=True) | ||
| assert values_sorted[0] == 3.0 | ||
| assert values_sorted[1] == 1.0 | ||
|
|
||
| def test_non_empty_text_produces_non_empty_vector(self) -> None: | ||
| tokenizer = BM25Tokenizer() | ||
| vec = tokenizer.encode("meaningful content here") | ||
| assert not vec.is_empty | ||
| assert len(vec.indices) > 0 | ||
| assert len(vec.values) > 0 | ||
|
|
||
| def test_indices_are_non_negative(self) -> None: | ||
| tokenizer = BM25Tokenizer() | ||
| vec = tokenizer.encode("hello world test content") | ||
| assert all(idx >= 0 for idx in vec.indices) | ||
|
|
||
|
|
||
| # ── Property-based tests ───────────────────────────────────────── | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| class TestSparseProperties: | ||
| @given( | ||
| text=st.text( | ||
| min_size=1, | ||
| alphabet=st.characters(categories=("L",)), # type: ignore[arg-type] | ||
| ) | ||
| ) | ||
| def test_encoding_is_deterministic(self, text: str) -> None: | ||
| tokenizer = BM25Tokenizer(remove_stop_words=False) | ||
| assert tokenizer.encode(text) == tokenizer.encode(text) | ||
|
|
||
| @given( | ||
| text=st.text( | ||
| min_size=1, | ||
| alphabet=st.characters(categories=("L",)), # type: ignore[arg-type] | ||
| ) | ||
| ) | ||
| def test_non_empty_alpha_produces_non_empty_vector(self, text: str) -> None: | ||
| tokenizer = BM25Tokenizer(remove_stop_words=False) | ||
| vec = tokenizer.encode(text) | ||
| assert not vec.is_empty | ||
|
|
||
| @given( | ||
| text=st.text( | ||
| min_size=1, | ||
| alphabet=st.characters(categories=("L",)), # type: ignore[arg-type] | ||
| ) | ||
| ) | ||
| def test_indices_always_sorted(self, text: str) -> None: | ||
| tokenizer = BM25Tokenizer(remove_stop_words=False) | ||
| vec = tokenizer.encode(text) | ||
| assert list(vec.indices) == sorted(vec.indices) | ||
|
|
||
| @given( | ||
| text=st.text( | ||
| min_size=1, | ||
| alphabet=st.characters(categories=("L",)), # type: ignore[arg-type] | ||
| ) | ||
| ) | ||
| def test_values_always_positive(self, text: str) -> None: | ||
| tokenizer = BM25Tokenizer(remove_stop_words=False) | ||
| vec = tokenizer.encode(text) | ||
| if not vec.is_empty: | ||
| assert all(v > 0 for v in vec.values) | ||
|
|
||
| @given( | ||
| text=st.text( | ||
| min_size=1, | ||
| alphabet=st.characters(categories=("L",)), # type: ignore[arg-type] | ||
| ) | ||
| ) | ||
| def test_indices_always_non_negative(self, text: str) -> None: | ||
| tokenizer = BM25Tokenizer(remove_stop_words=False) | ||
| vec = tokenizer.encode(text) | ||
| if not vec.is_empty: | ||
| assert all(idx >= 0 for idx in vec.indices) |
There was a problem hiding this comment.
Add required Google-style docstrings to public test classes/functions.
This file defines public classes/functions without docstrings, which violates the repo’s Python rule and can fail style gates.
✏️ Minimal pattern to apply consistently
`@pytest.mark.unit`
class TestSparseVector:
+ """Unit tests for SparseVector validation and invariants."""
+
def test_empty_vector(self) -> None:
+ """Empty tuples should produce an empty sparse vector."""
vec = SparseVector(indices=(), values=())As per coding guidelines, "Docstrings must use Google style and are required on all public classes/functions (enforced by ruff D rules)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/memory/test_sparse.py` around lines 17 - 251, Tests lack
Google-style docstrings on public classes and functions (triggers ruff D rules);
add concise Google-style docstrings to each public test class (e.g.,
TestSparseVector, TestTokenize, TestEncode, TestSparseProperties) and to any
public test functions that are intended to be part of the module's public API
(e.g., test_empty_vector, test_basic_tokenization,
test_empty_string_returns_empty_vector, the property-based tests), following the
repo pattern: one-line summary followed by an optional short description and
example parameters/expected behavior where helpful; ensure docstrings use
triple-quoted strings directly under the class/function definitions and keep
wording minimal and consistent across the file.
| @given( | ||
| text=st.text( | ||
| min_size=1, | ||
| alphabet=st.characters(categories=("L",)), # type: ignore[arg-type] | ||
| ) | ||
| ) | ||
| def test_encoding_is_deterministic(self, text: str) -> None: | ||
| tokenizer = BM25Tokenizer(remove_stop_words=False) | ||
| assert tokenizer.encode(text) == tokenizer.encode(text) | ||
|
|
||
| @given( | ||
| text=st.text( | ||
| min_size=1, | ||
| alphabet=st.characters(categories=("L",)), # type: ignore[arg-type] | ||
| ) | ||
| ) | ||
| def test_non_empty_alpha_produces_non_empty_vector(self, text: str) -> None: | ||
| tokenizer = BM25Tokenizer(remove_stop_words=False) | ||
| vec = tokenizer.encode(text) | ||
| assert not vec.is_empty | ||
|
|
||
| @given( | ||
| text=st.text( | ||
| min_size=1, | ||
| alphabet=st.characters(categories=("L",)), # type: ignore[arg-type] | ||
| ) | ||
| ) | ||
| def test_indices_always_sorted(self, text: str) -> None: | ||
| tokenizer = BM25Tokenizer(remove_stop_words=False) | ||
| vec = tokenizer.encode(text) | ||
| assert list(vec.indices) == sorted(vec.indices) | ||
|
|
||
| @given( | ||
| text=st.text( | ||
| min_size=1, | ||
| alphabet=st.characters(categories=("L",)), # type: ignore[arg-type] | ||
| ) | ||
| ) | ||
| def test_values_always_positive(self, text: str) -> None: | ||
| tokenizer = BM25Tokenizer(remove_stop_words=False) | ||
| vec = tokenizer.encode(text) | ||
| if not vec.is_empty: | ||
| assert all(v > 0 for v in vec.values) | ||
|
|
||
| @given( | ||
| text=st.text( | ||
| min_size=1, | ||
| alphabet=st.characters(categories=("L",)), # type: ignore[arg-type] | ||
| ) | ||
| ) | ||
| def test_indices_always_non_negative(self, text: str) -> None: | ||
| tokenizer = BM25Tokenizer(remove_stop_words=False) | ||
| vec = tokenizer.encode(text) | ||
| if not vec.is_empty: | ||
| assert all(idx >= 0 for idx in vec.indices) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify Hypothesis-decorated tests that lack `@settings` in this file.
python - <<'PY'
import ast
from pathlib import Path
path = Path("tests/unit/memory/test_sparse.py")
tree = ast.parse(path.read_text(encoding="utf-8"))
for node in ast.walk(tree):
if isinstance(node, ast.FunctionDef):
decorators = [ast.unparse(d) for d in node.decorator_list]
has_given = any("given(" in d for d in decorators)
has_settings = any("settings(" in d for d in decorators)
if has_given and not has_settings:
print(f"{path}:{node.lineno} -> {node.name} missing `@settings`")
PYRepository: Aureliolo/synthorg
Length of output: 517
🏁 Script executed:
cat -n tests/unit/memory/test_sparse.py | sed -n '195,255p'Repository: Aureliolo/synthorg
Length of output: 2380
🏁 Script executed:
rg "@given|@settings" tests/unit/memory/test_sparse.py -A 1 -B 1Repository: Aureliolo/synthorg
Length of output: 275
🏁 Script executed:
head -50 tests/unit/memory/test_sparse.py | grep -E "^import|^from"Repository: Aureliolo/synthorg
Length of output: 218
🏁 Script executed:
rg "@settings" tests/ --type pyRepository: Aureliolo/synthorg
Length of output: 6144
Add @settings() decorator to all five property tests and import settings from hypothesis.
The test methods test_encoding_is_deterministic, test_non_empty_alpha_produces_non_empty_vector, test_indices_always_sorted, test_values_always_positive, and test_indices_always_non_negative use @given without @settings. Per coding guidelines, all property-based tests must include @settings() to enable profile-driven behavior via the HYPOTHESIS_PROFILE environment variable (ci: 50 examples, dev: 1000 examples).
Suggested changes
Import update:
-from hypothesis import given
+from hypothesis import given, settingsApply @settings() to each of the five test methods (lines 197, 207, 218, 229, 241):
+@settings()
`@given`(
text=st.text(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/memory/test_sparse.py` around lines 197 - 251, Add an import for
settings from hypothesis and decorate each property test with `@settings`():
update the five test functions test_encoding_is_deterministic,
test_non_empty_alpha_produces_non_empty_vector, test_indices_always_sorted,
test_values_always_positive, and test_indices_always_non_negative so they each
have a `@settings`() decorator directly above their existing `@given`(...)
decorator; keep the BM25Tokenizer usage and existing given strategies unchanged
so Hypothesis profiles (HYPOTHESIS_PROFILE) control example counts.
ad289c6 to
29cf98b
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/synthorg/memory/__init__.py (1)
84-146: 🧹 Nitpick | 🔵 TrivialConsider updating the module docstring to mention new exports.
The
__all__list correctly includes all 7 new symbols with helpful section comments. However, the module docstring (lines 1-10) doesn't mention the new categories of exports (reformulation protocols, sparse search components, tool-based strategy). This is optional but would improve discoverability.📝 Suggested docstring update
"""Agent memory system -- protocols, models, config, and factory. Re-exports protocols (``MemoryBackend``, ``MemoryCapabilities``, ``SharedKnowledgeStore``, ``MemoryInjectionStrategy``, ``OrgMemoryBackend``, ``ConsolidationStrategy``, ``ArchivalStore``), concrete backends (``Mem0MemoryBackend``), domain models, config -models, factory, retrieval pipeline, consolidation, org memory, and -error hierarchy so consumers can import from ``synthorg.memory`` -directly. +models, factory, retrieval pipeline (including hybrid sparse search +and query reformulation), tool-based injection strategy, consolidation, +org memory, and error hierarchy so consumers can import from +``synthorg.memory`` directly. """🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/memory/__init__.py` around lines 84 - 146, Update the module docstring at the top of src/synthorg/memory/__init__.py to mention the newly exported categories and symbols so they're discoverable; specifically add brief lines describing the "query reformulation" exports (e.g., LLMQueryReformulator, LLMSufficiencyChecker, QueryReformulator, SufficiencyChecker), the "sparse/hybrid search" exports (e.g., BM25Tokenizer, SparseVector, FusionStrategy, fuse_ranked_lists), and the "tool-based strategy" export (ToolBasedInjectionStrategy), and ensure the docstring reflects these sections and their purpose consistent with the existing wording for other categories.
♻️ Duplicate comments (8)
src/synthorg/memory/backends/mem0/config.py (1)
193-200:⚠️ Potential issue | 🟠 MajorField definition is correct, but
build_config_from_company_configdoesn't thread it.The
sparse_search_enabledfield is well-defined with an appropriate default and documentation. However, the factory functionbuild_config_from_company_config()(lines 365-368) only passesdata_dirandembedder, so callers using that factory path cannot enable sparse search from company-level configuration.🔧 Proposed fix to thread sparse_search_enabled
Either add a
sparse_search_enabledparameter tobuild_config_from_company_config:def build_config_from_company_config( config: CompanyMemoryConfig, *, embedder: Mem0EmbedderConfig, + sparse_search_enabled: bool = False, ) -> Mem0BackendConfig: ... return Mem0BackendConfig( data_dir=config.storage.data_dir, embedder=embedder, + sparse_search_enabled=sparse_search_enabled, )Or, if
CompanyMemoryConfigshould have asparse_search_enabledfield, read it from there.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/memory/backends/mem0/config.py` around lines 193 - 200, The factory build_config_from_company_config currently ignores sparse_search_enabled; update it to thread that setting by either adding a sparse_search_enabled parameter to build_config_from_company_config and passing it into the Mem0Config constructor, or read sparse_search_enabled from CompanyMemoryConfig (if that type should own the flag) and forward it when constructing Mem0Config; locate build_config_from_company_config and the Mem0Config constructor call and ensure the sparse_search_enabled boolean is passed through.src/synthorg/memory/backends/mem0/adapter.py (3)
202-203:⚠️ Potential issue | 🔴 CriticalCritical: Incorrect
exceptsyntax will fail to catchRecursionError.The syntax
except builtins.MemoryError, RecursionError:in Python < 3.14 catches onlyMemoryErrorand binds it to a variable namedRecursionError, shadowing the builtin. ActualRecursionErrorexceptions will propagate uncaught.Note: Line 173 in the same file correctly uses
except (builtins.MemoryError, RecursionError):. This inconsistency suggests a copy-paste error.🐛 Proposed fix
- except builtins.MemoryError, RecursionError: + except (builtins.MemoryError, RecursionError): raise🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/memory/backends/mem0/adapter.py` around lines 202 - 203, The except clause in adapter.py currently uses invalid syntax "except builtins.MemoryError, RecursionError:" which only binds MemoryError to a name and fails to catch RecursionError; update the handler to use a tuple form like "except (builtins.MemoryError, RecursionError):" (matching the earlier correct usage on line 173) so both exceptions are caught and re-raised in the same way; search for the clause around the error-handling block (the try/except that re-raises) to apply the fix.
411-412:⚠️ Potential issue | 🔴 CriticalSame
exceptsyntax issue here.🐛 Proposed fix
- except builtins.MemoryError, RecursionError: + except (builtins.MemoryError, RecursionError): raise🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/memory/backends/mem0/adapter.py` around lines 411 - 412, The except clause uses incorrect Python 2-style comma syntax ("except builtins.MemoryError, RecursionError:") — update the handler in adapter.py to use a tuple of exceptions or separate except blocks; replace it with a tuple form (e.g., except (builtins.MemoryError, RecursionError): raise) or two explicit excepts so MemoryError and RecursionError are re-raised correctly in the code path around the adapter exception handling.
850-851:⚠️ Potential issue | 🔴 CriticalSame
exceptsyntax issue here.🐛 Proposed fix
- except builtins.MemoryError, RecursionError: + except (builtins.MemoryError, RecursionError): raise🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/memory/backends/mem0/adapter.py` around lines 850 - 851, The except clause uses Python 3 incompatible syntax "except builtins.MemoryError, RecursionError:"; update the handler to catch both exceptions correctly by using a tuple form "except (builtins.MemoryError, RecursionError):" (or split into two separate except blocks) and ensure it still re-raises the exception as currently intended; locate the erroneous clause (the line containing "except builtins.MemoryError, RecursionError:") in the mem0 adapter module and apply this change.tests/unit/memory/test_reformulation.py (1)
84-89:⚠️ Potential issue | 🟡 MinorAdd test coverage for
MemoryError/RecursionErrorpropagation.The test only verifies that
RuntimeErrorreturnsNone. The production code inreformulation.pyexplicitly re-raisesbuiltins.MemoryErrorandRecursionErrorbefore the generic exception handler. Add tests to verify these system-level exceptions propagate rather than being swallowed.💡 Suggested additional tests
async def test_memory_error_propagates(self) -> None: """MemoryError is not swallowed.""" mock_fn = AsyncMock(side_effect=MemoryError("OOM")) reformulator = LLMQueryReformulator(completion_fn=mock_fn) with pytest.raises(MemoryError): await reformulator.reformulate("query", ()) async def test_recursion_error_propagates(self) -> None: """RecursionError is not swallowed.""" mock_fn = AsyncMock(side_effect=RecursionError()) reformulator = LLMQueryReformulator(completion_fn=mock_fn) with pytest.raises(RecursionError): await reformulator.reformulate("query", ())Also applies to
LLMSufficiencyChecker.check_sufficiency()(lines 118-123).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/memory/test_reformulation.py` around lines 84 - 89, Add tests that assert MemoryError and RecursionError are re-raised (not swallowed) by the reformulation and sufficiency code paths: for LLMQueryReformulator.reformulate create AsyncMock side_effect=MemoryError("OOM") and side_effect=RecursionError() and wrap calls in pytest.raises(MemoryError) and pytest.raises(RecursionError); do the same for LLMSufficiencyChecker.check_sufficiency (use its instance and call check_sufficiency with the AsyncMock) so both functions' special-case re-raises are covered.tests/unit/memory/test_sparse.py (2)
197-251: 🛠️ Refactor suggestion | 🟠 MajorProperty tests missing
@settings()decorator for profile-driven behavior.The property-based tests use
@givenwithout@settings(). Per coding guidelines, all Hypothesis property tests should include@settings()to enable profile-driven example counts viaHYPOTHESIS_PROFILEenvironment variable (ci: 50 examples, dev: 1000 examples).🔧 Add `@settings`() to each property test
+from hypothesis import given, settings -from hypothesis import given `@pytest.mark.unit` class TestSparseProperties: + `@settings`() `@given`( text=st.text( min_size=1, alphabet=st.characters(categories=("L",)), ) ) def test_encoding_is_deterministic(self, text: str) -> None:Apply
@settings()similarly to all five property test methods.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/memory/test_sparse.py` around lines 197 - 251, Each Hypothesis property test (test_encoding_is_deterministic, test_non_empty_alpha_produces_non_empty_vector, test_indices_always_sorted, test_values_always_positive, test_indices_always_non_negative) is missing the `@settings`() decorator; update each test to add a `@settings`() decorator above the `@given`(...) so the tests honor profile-driven example counts via HYPOTHESIS_PROFILE (e.g., ci/dev) while keeping the existing `@given`(...) and using the same BM25Tokenizer setup.
16-57: 🧹 Nitpick | 🔵 TrivialTest classes lack Google-style docstrings.
Per coding guidelines, docstrings are required on all public classes/functions (enforced by ruff D rules). Consider adding brief docstrings to test classes.
📝 Example docstring pattern
`@pytest.mark.unit` class TestSparseVector: + """Unit tests for SparseVector validation and invariants.""" + def test_empty_vector(self) -> None:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/memory/test_sparse.py` around lines 16 - 57, TestSparseVector is missing a Google-style docstring which violates ruff D rules; add a brief docstring directly under the class definition describing what the test class verifies (e.g., "Unit tests for SparseVector validation and behavior"), referencing the class name TestSparseVector so the change is local and minimal, and ensure the docstring follows Google style (one-line summary and optional short description) to satisfy the linter without modifying any test logic.tests/unit/memory/backends/mem0/test_sparse_search.py (1)
164-180: 🛠️ Refactor suggestion | 🟠 MajorTests should assert the sparse vector and user_id filter are passed to Qdrant.
The
test_applies_user_id_filtertest only verifieslimitwas forwarded but doesn't actually check that the user_id filter or sparse vector are present in thequery_pointscall. This weakens the test's ability to catch regressions in agent-isolation filtering.🔧 Suggested assertion additions
def test_applies_user_id_filter(self) -> None: client = MagicMock() result = MagicMock() result.points = [] client.query_points.return_value = result query_vec = SparseVector(indices=(10,), values=(1.0,)) search_sparse( client, "test_collection", query_vec, user_id_filter="agent-1", limit=5, ) call_kwargs = client.query_points.call_args.kwargs assert call_kwargs["limit"] == 5 + # Verify sparse vector is passed + assert call_kwargs["query"].indices == [10] + assert call_kwargs["query"].values == [1.0] + # Verify user_id filter is applied + filter_obj = call_kwargs["query_filter"] + assert filter_obj.must[0].key == "user_id" + assert filter_obj.must[0].match.value == "agent-1"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/memory/backends/mem0/test_sparse_search.py` around lines 164 - 180, The test_applies_user_id_filter currently only checks limit; update it to also assert that the sparse vector and user id were forwarded to Qdrant by inspecting client.query_points.call_args.kwargs: verify the sparse vector argument (e.g., call_kwargs["query_sparse"] or the appropriate key used in search_sparse) matches the original query_vec (compare indices/values or equality), and verify the user id filter was forwarded (e.g., call_kwargs["user_id_filter"] == "agent-1" or that "agent-1" appears inside call_kwargs["filter"] if the filter is nested).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/design/memory.md`:
- Line 736: Update the test mocks so their function/method signatures match the
protocol: change the mock parameter type for query_text from str to NotBlankStr
(the same signature used in the production protocol snippet: self, agent_id:
NotBlankStr, query_text: NotBlankStr, token_budget: int), import NotBlankStr in
the test module, and adjust any mock call sites or test fixtures to
construct/validate NotBlankStr values (or cast/wrap existing string literals) so
the mock satisfies the protocol type.
In `@src/synthorg/memory/reformulation.py`:
- Around line 91-96: The summary currently slices each MemoryEntry.content to
200 chars in _format_results_summary without indicating truncation; update the
formatting logic in _format_results_summary to append an ellipsis (e.g., "…")
when e.content is longer than 200 characters so users know the text was
truncated, using the MemoryEntry.content length check for each entry and
preserving the existing 200-char limit and the existing list/formatting
structure.
In `@tests/unit/memory/test_tool_retriever.py`:
- Around line 306-318: The test test_search_system_error_propagates currently
raises the unqualified MemoryError which can be confused with
synthorg.memory.errors.MemoryError; update the test to explicitly reference the
built-in exception (e.g., use builtins.MemoryError or add a clarifying comment)
and/or import builtins to make it clear the AsyncMock side_effect on
backend.retrieve should raise the builtin MemoryError; keep the test logic and
the call to ToolBasedInjectionStrategy.handle_tool_call unchanged.
---
Outside diff comments:
In `@src/synthorg/memory/__init__.py`:
- Around line 84-146: Update the module docstring at the top of
src/synthorg/memory/__init__.py to mention the newly exported categories and
symbols so they're discoverable; specifically add brief lines describing the
"query reformulation" exports (e.g., LLMQueryReformulator,
LLMSufficiencyChecker, QueryReformulator, SufficiencyChecker), the
"sparse/hybrid search" exports (e.g., BM25Tokenizer, SparseVector,
FusionStrategy, fuse_ranked_lists), and the "tool-based strategy" export
(ToolBasedInjectionStrategy), and ensure the docstring reflects these sections
and their purpose consistent with the existing wording for other categories.
---
Duplicate comments:
In `@src/synthorg/memory/backends/mem0/adapter.py`:
- Around line 202-203: The except clause in adapter.py currently uses invalid
syntax "except builtins.MemoryError, RecursionError:" which only binds
MemoryError to a name and fails to catch RecursionError; update the handler to
use a tuple form like "except (builtins.MemoryError, RecursionError):" (matching
the earlier correct usage on line 173) so both exceptions are caught and
re-raised in the same way; search for the clause around the error-handling block
(the try/except that re-raises) to apply the fix.
- Around line 411-412: The except clause uses incorrect Python 2-style comma
syntax ("except builtins.MemoryError, RecursionError:") — update the handler in
adapter.py to use a tuple of exceptions or separate except blocks; replace it
with a tuple form (e.g., except (builtins.MemoryError, RecursionError): raise)
or two explicit excepts so MemoryError and RecursionError are re-raised
correctly in the code path around the adapter exception handling.
- Around line 850-851: The except clause uses Python 3 incompatible syntax
"except builtins.MemoryError, RecursionError:"; update the handler to catch both
exceptions correctly by using a tuple form "except (builtins.MemoryError,
RecursionError):" (or split into two separate except blocks) and ensure it still
re-raises the exception as currently intended; locate the erroneous clause (the
line containing "except builtins.MemoryError, RecursionError:") in the mem0
adapter module and apply this change.
In `@src/synthorg/memory/backends/mem0/config.py`:
- Around line 193-200: The factory build_config_from_company_config currently
ignores sparse_search_enabled; update it to thread that setting by either adding
a sparse_search_enabled parameter to build_config_from_company_config and
passing it into the Mem0Config constructor, or read sparse_search_enabled from
CompanyMemoryConfig (if that type should own the flag) and forward it when
constructing Mem0Config; locate build_config_from_company_config and the
Mem0Config constructor call and ensure the sparse_search_enabled boolean is
passed through.
In `@tests/unit/memory/backends/mem0/test_sparse_search.py`:
- Around line 164-180: The test_applies_user_id_filter currently only checks
limit; update it to also assert that the sparse vector and user id were
forwarded to Qdrant by inspecting client.query_points.call_args.kwargs: verify
the sparse vector argument (e.g., call_kwargs["query_sparse"] or the appropriate
key used in search_sparse) matches the original query_vec (compare
indices/values or equality), and verify the user id filter was forwarded (e.g.,
call_kwargs["user_id_filter"] == "agent-1" or that "agent-1" appears inside
call_kwargs["filter"] if the filter is nested).
In `@tests/unit/memory/test_reformulation.py`:
- Around line 84-89: Add tests that assert MemoryError and RecursionError are
re-raised (not swallowed) by the reformulation and sufficiency code paths: for
LLMQueryReformulator.reformulate create AsyncMock side_effect=MemoryError("OOM")
and side_effect=RecursionError() and wrap calls in pytest.raises(MemoryError)
and pytest.raises(RecursionError); do the same for
LLMSufficiencyChecker.check_sufficiency (use its instance and call
check_sufficiency with the AsyncMock) so both functions' special-case re-raises
are covered.
In `@tests/unit/memory/test_sparse.py`:
- Around line 197-251: Each Hypothesis property test
(test_encoding_is_deterministic, test_non_empty_alpha_produces_non_empty_vector,
test_indices_always_sorted, test_values_always_positive,
test_indices_always_non_negative) is missing the `@settings`() decorator; update
each test to add a `@settings`() decorator above the `@given`(...) so the tests
honor profile-driven example counts via HYPOTHESIS_PROFILE (e.g., ci/dev) while
keeping the existing `@given`(...) and using the same BM25Tokenizer setup.
- Around line 16-57: TestSparseVector is missing a Google-style docstring which
violates ruff D rules; add a brief docstring directly under the class definition
describing what the test class verifies (e.g., "Unit tests for SparseVector
validation and behavior"), referencing the class name TestSparseVector so the
change is local and minimal, and ensure the docstring follows Google style
(one-line summary and optional short description) to satisfy the linter without
modifying any test logic.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e464c708-ce41-40e8-b881-8be486ffb8b4
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (23)
CLAUDE.mdREADME.mddocs/architecture/tech-stack.mddocs/design/memory.mdpyproject.tomlsrc/synthorg/memory/__init__.pysrc/synthorg/memory/backends/mem0/adapter.pysrc/synthorg/memory/backends/mem0/config.pysrc/synthorg/memory/backends/mem0/sparse_search.pysrc/synthorg/memory/injection.pysrc/synthorg/memory/reformulation.pysrc/synthorg/memory/retrieval_config.pysrc/synthorg/memory/retriever.pysrc/synthorg/memory/sparse.pysrc/synthorg/memory/tool_retriever.pysrc/synthorg/observability/events/memory.pytests/unit/memory/backends/mem0/test_sparse_search.pytests/unit/memory/test_init.pytests/unit/memory/test_reformulation.pytests/unit/memory/test_retrieval_config.pytests/unit/memory/test_retriever.pytests/unit/memory/test_sparse.pytests/unit/memory/test_tool_retriever.py
|
|
||
| async def prepare_messages( | ||
| self, agent_id: NotBlankStr, query_text: str, token_budget: int | ||
| self, agent_id: NotBlankStr, query_text: NotBlankStr, token_budget: int |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify all MemoryInjectionStrategy implementations use NotBlankStr for query_text
ast-grep --pattern $'async def prepare_messages(
self, $_, query_text: $TYPE, $_
) -> $_:
$$$'Repository: Aureliolo/synthorg
Length of output: 2383
🏁 Script executed:
# Find all classes that inherit from or implement MemoryInjectionStrategy
rg -A 5 'class.*MemoryInjectionStrategy' --type pyRepository: Aureliolo/synthorg
Length of output: 932
🏁 Script executed:
# Also check the base class definition to understand the protocol structure
rg -B 2 -A 15 'class MemoryInjectionStrategy' src/synthorg/memory/injection.pyRepository: Aureliolo/synthorg
Length of output: 615
🏁 Script executed:
# Find all implementations of prepare_messages outside of test files
rg -l 'def prepare_messages|async def prepare_messages' --type py -g '!test*' -g '!*_test.py'Repository: Aureliolo/synthorg
Length of output: 165
🏁 Script executed:
# Also check for any concrete strategy classes (not just test mocks)
rg 'class.*:' src/synthorg/memory/ --type py -A 3 | grep -A 3 'prepare_messages'Repository: Aureliolo/synthorg
Length of output: 44
🏁 Script executed:
# Check the prepare_messages signature in tool_retriever.py
rg -B 2 -A 8 'async def prepare_messages' src/synthorg/memory/tool_retriever.pyRepository: Aureliolo/synthorg
Length of output: 442
🏁 Script executed:
# Check the prepare_messages signature in retriever.py
rg -B 2 -A 8 'async def prepare_messages' src/synthorg/memory/retriever.pyRepository: Aureliolo/synthorg
Length of output: 380
Update test mocks to match protocol signature.
Production implementations in tool_retriever.py and retriever.py have been correctly updated to use NotBlankStr. However, the test mocks in tests/unit/memory/test_injection.py still use str for query_text and should be updated to match the protocol definition for type consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/design/memory.md` at line 736, Update the test mocks so their
function/method signatures match the protocol: change the mock parameter type
for query_text from str to NotBlankStr (the same signature used in the
production protocol snippet: self, agent_id: NotBlankStr, query_text:
NotBlankStr, token_budget: int), import NotBlankStr in the test module, and
adjust any mock call sites or test fixtures to construct/validate NotBlankStr
values (or cast/wrap existing string literals) so the mock satisfies the
protocol type.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tests/unit/memory/backends/mem0/test_sparse_search.py (1)
179-184:⚠️ Potential issue | 🟠 MajorVerify the user_id filter value in assertions.
The test only checks that
filter_obj is not Nonebut does not verify the filter actually contains the expecteduser_id_filter="agent-1"constraint. This is critical for agent isolation—the test should verify the filter constrains results to the specified user.Proposed fix
call_kwargs = client.query_points.call_args.kwargs assert call_kwargs["limit"] == 5 assert call_kwargs["using"] == "bm25" # Verify user_id filter is present in the query_filter filter_obj = call_kwargs["query_filter"] assert filter_obj is not None + # Verify the filter contains the expected user_id constraint + assert len(filter_obj.must) == 1 + condition = filter_obj.must[0] + assert condition.key == "user_id" + assert condition.match.value == "agent-1"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/memory/backends/mem0/test_sparse_search.py` around lines 179 - 184, The test currently only asserts that filter_obj is not None; update the assertions to verify the actual user_id constraint is present by extracting the user_id filter from call_kwargs["query_filter"] (e.g., inspect filter_obj["user_id_filter"] or the appropriate nested key used by the query_filter structure) and assert it equals "agent-1" so the query actually enforces agent isolation when calling client.query_points.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@tests/unit/memory/backends/mem0/test_sparse_search.py`:
- Around line 179-184: The test currently only asserts that filter_obj is not
None; update the assertions to verify the actual user_id constraint is present
by extracting the user_id filter from call_kwargs["query_filter"] (e.g., inspect
filter_obj["user_id_filter"] or the appropriate nested key used by the
query_filter structure) and assert it equals "agent-1" so the query actually
enforces agent isolation when calling client.query_points.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f0ac5fee-5e2c-4e85-a2c3-15308a64b535
📒 Files selected for processing (6)
src/synthorg/memory/__init__.pysrc/synthorg/memory/backends/mem0/config.pysrc/synthorg/memory/reformulation.pytests/unit/memory/backends/mem0/test_sparse_search.pytests/unit/memory/test_reformulation.pytests/unit/memory/test_tool_retriever.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Sandbox
- GitHub Check: Build Web
- GitHub Check: Build Backend
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotationsin Python code -- Python 3.14 has PEP 649
Use PEP 758 except syntax:except A, B:(no parentheses) -- ruff enforces this on Python 3.14
All public functions must have type hints and pass mypy strict mode
Docstrings must use Google style and are required on all public classes/functions (enforced by ruff D rules)
Create new objects instead of mutating existing ones. For non-Pydantic internal collections (registries,BaseTool), usecopy.deepcopy()at construction +MappingProxyTypewrapping for read-only enforcement.
Fordict/listfields in frozen Pydantic models, rely onfrozen=Truefor field reassignment prevention andcopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).
Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (usingmodel_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.
Use Pydantic v2 (BaseModel,model_validator,computed_field,ConfigDict). Always useallow_inf_nan=FalseinConfigDictdeclarations to rejectNaN/Infin numeric fields at validation time.
Use@computed_fieldfor derived values instead of storing + validating redundant fields (e.g.TokenUsage.total_tokens)
UseNotBlankStr(fromcore.types) for all identifier/name fields -- including optional (NotBlankStr | None) and tuple (tuple[NotBlankStr, ...]) variants -- instead of manual whitespace validators.
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over barecreate_task.
Maximum line length: 88 characters (enforced by ruff)
Functions must be less than 50 lines, files must be less than 800 lines
Files:
src/synthorg/memory/backends/mem0/config.pysrc/synthorg/memory/__init__.pytests/unit/memory/test_tool_retriever.pytests/unit/memory/test_reformulation.pysrc/synthorg/memory/reformulation.pytests/unit/memory/backends/mem0/test_sparse_search.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic MUST have:from synthorg.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging/logging.getLogger()/print()in application code (exceptions:observability/setup.py,observability/sinks.py,observability/syslog_handler.py, andobservability/http_handler.pymay use stdlibloggingandprint(..., file=sys.stderr))
Logger variable name must always belogger(not_logger, notlog)
Always use event constants from domain-specific modules undersynthorg.observability.events(e.g.,API_REQUEST_STARTEDfromevents.api). Import directly:from synthorg.observability.events.<domain> import EVENT_CONSTANT
Always use structured kwargs for logging:logger.info(EVENT, key=value)-- neverlogger.info("msg %s", val)
All error paths must log at WARNING or ERROR with context before raising
All state transitions must log at INFO level
Use DEBUG level for object creation, internal flow, entry/exit of key functions
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples. Use generic names:example-provider,example-large-001,example-medium-001,example-small-001,large/medium/small.
In Pydantic models, useallow_inf_nan=Falsein allConfigDictdeclarations
Pure data models, enums, and re-exports do NOT need logging
Files:
src/synthorg/memory/backends/mem0/config.pysrc/synthorg/memory/__init__.pysrc/synthorg/memory/reformulation.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: All test files must use markers:@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slow
Async tests: useasyncio_mode = "auto"-- no manual@pytest.mark.asyncioneeded
Test timeout: 30 seconds per test (global inpyproject.toml-- do not add per-filepytest.mark.timeout(30)markers; non-default overrides liketimeout(60)are allowed)
Prefer@pytest.mark.parametrizefor testing similar cases
For property-based testing in Python, use Hypothesis (@given+@settings). Profiles:ci(50 examples, default) anddev(1000 examples). Run dev profile:HYPOTHESIS_PROFILE=dev uv run python -m pytest tests/ -m unit -n 8 -k properties
Never skip, dismiss, or ignore flaky tests -- always fix them fully and fundamentally. For timing-sensitive tests, mocktime.monotonic()andasyncio.sleep(). For tasks that must block indefinitely, useasyncio.Event().wait()instead ofasyncio.sleep(large_number).
Files:
tests/unit/memory/test_tool_retriever.pytests/unit/memory/test_reformulation.pytests/unit/memory/backends/mem0/test_sparse_search.py
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/memory/**/*.py : Memory package (memory/): pluggable MemoryBackend protocol, backends/ (Mem0 adapter), retrieval pipeline (ranking, RRF fusion, injection, formatting, non-inferable filtering), shared org memory (org/), consolidation/archival (density-aware: DensityClassifier, AbstractiveSummarizer, ExtractivePreserver, DualModeConsolidationStrategy)
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/memory/**/*.py : Memory package (memory/): pluggable MemoryBackend protocol, backends/ (Mem0 adapter), retrieval pipeline (ranking, RRF fusion, injection, formatting, non-inferable filtering), shared org memory (org/), consolidation/archival (density-aware: DensityClassifier, AbstractiveSummarizer, ExtractivePreserver, DualModeConsolidationStrategy)
Applied to files:
src/synthorg/memory/backends/mem0/config.pysrc/synthorg/memory/__init__.pytests/unit/memory/test_tool_retriever.pytests/unit/memory/test_reformulation.pysrc/synthorg/memory/reformulation.pytests/unit/memory/backends/mem0/test_sparse_search.py
📚 Learning: 2026-03-16T06:24:56.341Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:24:56.341Z
Learning: Applies to src/synthorg/memory/**/*.py : Use MemoryBackend protocol with pluggable backends (Mem0 adapter available at backends/mem0/) for persistent agent memory
Applied to files:
src/synthorg/memory/backends/mem0/config.pysrc/synthorg/memory/__init__.pysrc/synthorg/memory/reformulation.pytests/unit/memory/backends/mem0/test_sparse_search.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/**/*.py : Package structure: src/synthorg/ organized as: api/ (REST+WebSocket, Litestar), auth/ (auth subpackage), backup/ (scheduled/manual backups), budget/ (cost tracking, CFO), cli/ (superseded by Go CLI), communication/ (message bus, meetings), config/ (YAML loading), core/ (domain models, resilience config), engine/ (orchestration, task state, coordination, approval gates, stagnation detection, context budget, compaction), hr/ (hiring, performance, promotion), memory/ (pluggable backend, Mem0, retrieval, consolidation), persistence/ (operational data, SQLite, settings), observability/ (logging, correlation, sinks), providers/ (LLM abstraction, LiteLLM, auth types, presets, runtime CRUD), settings/ (runtime-editable, typed definitions, encryption, config bridge), security/ (SecOps, rule engine, output scanning, progressive trust, autonomy levels), templates/ (company templates, personalities), tools/ (registry, built-in tools, git, sandbox, code_runner, MCP...
Applied to files:
src/synthorg/memory/__init__.py
📚 Learning: 2026-03-16T06:24:56.341Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:24:56.341Z
Learning: Applies to src/synthorg/core/**/*.py : Core module must contain shared domain models, base classes, resilience config (RetryConfig, RateLimiterConfig)
Applied to files:
src/synthorg/memory/__init__.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to tests/**/*.py : Fix flaky tests completely and fundamentally; for timing-sensitive tests, mock `time.monotonic()` and `asyncio.sleep()` to make them deterministic instead of widening timing margins
Applied to files:
tests/unit/memory/test_reformulation.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : Handle errors explicitly, never silently swallow. Validate at system boundaries (user input, external APIs, config files).
Applied to files:
tests/unit/memory/test_reformulation.py
📚 Learning: 2026-03-31T14:17:24.182Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:17:24.182Z
Learning: Applies to src/synthorg/**/*.py : Maintain 80% minimum test coverage (enforced in CI)
Applied to files:
tests/unit/memory/test_reformulation.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to tests/**/*.py : NEVER skip, dismiss, or ignore flaky tests — always fix them fully and fundamentally. For timing-sensitive tests, mock `time.monotonic()` and `asyncio.sleep()` to make them deterministic instead of widening timing margins
Applied to files:
tests/unit/memory/test_reformulation.py
🔇 Additional comments (13)
tests/unit/memory/test_tool_retriever.py (1)
1-332: Well-structured test suite covering the complete ToolBasedInjectionStrategy API.The tests comprehensively cover:
- Protocol compliance with
MemoryInjectionStrategyprepare_messages()behavior (instruction message vs. zero budget)- Tool definitions structure and schemas
handle_tool_call()dispatch for bothsearch_memoryandrecall_memory- Error handling including domain errors (returning safe messages), validation errors, and system error propagation
The clarifying comment on line 308 (
# builtins.MemoryError (not synthorg domain MemoryError)) addresses the earlier review feedback about distinguishing the built-in exception fromsynthorg.memory.errors.MemoryError.src/synthorg/memory/reformulation.py (3)
91-102: Truncation indicator properly implemented.The ellipsis is now appended when content exceeds 200 characters, addressing the earlier feedback about users not knowing content was cut off.
139-157: Error handling correctly propagates system-level exceptions.The pattern of re-raising
MemoryErrorandRecursionErrorwhile gracefully degrading for other exceptions is appropriate. These system-level errors indicate fundamental problems (OOM, stack overflow) that shouldn't be masked. The PEP 758except A, B:syntax is used correctly per project guidelines.
197-217: Safe fallback toTrueon error prevents infinite reformulation loops.The design decision to default to "sufficient" when the LLM call fails is well-documented and prevents the retrieval pipeline from getting stuck in infinite reformulation cycles. The same exception propagation pattern for system errors is consistently applied.
src/synthorg/memory/__init__.py (1)
10-17: Public API correctly expanded for hybrid search capabilities.The module properly re-exports all new components:
- Sparse search:
BM25Tokenizer,SparseVector- Query reformulation:
QueryReformulator,SufficiencyChecker,LLMQueryReformulator,LLMSufficiencyChecker- Tool-based injection:
ToolBasedInjectionStrategyThe
__all__list maintains alphabetical ordering with helpful section comments for discoverability.Also applies to: 80-90, 92-154
tests/unit/memory/test_reformulation.py (1)
1-154: Comprehensive test coverage for reformulation components.The tests thoroughly exercise:
- Protocol conformance for both
LLMQueryReformulatorandLLMSufficiencyChecker- Happy paths (reformulation returns rewritten query, sufficiency returns true/false)
- Edge cases (empty response →
None, ambiguous response →False, empty entries)- Error handling (general exceptions → graceful degradation with
None/True)- System error propagation (
MemoryError,RecursionErrorre-raised)The
MemoryErrorandRecursionErrorpropagation tests (lines 90-100 and 144-154) address the earlier review feedback about covering the passthrough exception branches.src/synthorg/memory/backends/mem0/config.py (2)
193-200: LGTM! Sparse search configuration field properly added.The field is correctly defined with frozen model semantics, appropriate default (
Falsefor opt-in), and a clear description explaining its purpose. The placement alongside other config fields is appropriate.
321-372: Sparse search flag is now properly threaded through the factory.The parameter is accepted with a sensible default and forwarded to
Mem0BackendConfig. This addresses the previous review concern about company-config callers being stuck on the default.tests/unit/memory/backends/mem0/test_sparse_search.py (5)
1-58: Well-structured test helpers.The helper functions provide clean, reusable mock factories. The
_make_scored_pointhelper correctly structures the payload to match Qdrant'sScoredPointformat with all necessary fields for memory entry conversion.
64-106: Good coverage forensure_sparse_fieldscenarios.The tests cover the key conditional branches: creating when missing, skipping when present, handling existing but different sparse fields, and custom field names.
112-138: LGTM!Tests properly verify the upsert behavior including the empty-vector skip optimization.
205-248: LGTM!Good coverage of
scored_points_to_entriesincluding the malformed-point skip logic which ensures robustness.
254-369: Good integration test coverage for adapter sparse behavior.The tests properly verify the gating logic (enabled vs disabled), non-fatal failure handling for sparse upsert, and the
retrieve_sparsemethod behavior. Setting internal state directly (_client,_connected,_qdrant_client) is acceptable here to isolate tests from the fullconnect()flow.
20d5a65 to
2a01797
Compare
… CodeRabbit Source fixes: - Fix ToolBasedInjectionStrategy docs/design claims vs code reality - Add __slots__ to BM25Tokenizer and ToolBasedInjectionStrategy - Remove unused get_logger from sparse.py - Use casefold()+NFKC normalization for Unicode correctness - Extract _parse_search_args to reduce _handle_search below 50 lines - Enforce config.max_memories in tool search limit - Exclude bool from limit isinstance check - Deep copy schema dicts in get_tool_definitions - Log invalid categories in _parse_categories - Add missing 'working' to categories schema description - Reject query_reformulation_enabled=True with ValueError - Add personal_boost+RRF consistency warning - Replace builtins_MemoryError alias with import builtins - Decouple shared sparse retrieval from personal backend - Use MEMORY_SPARSE_FIELD_ENSURE_FAILED for field init - Add MEMORY_SPARSE_POINT_FIELD_DEFAULTED + BATCH_DEGRADED events - Extract _extract_metadata + _parse_created_at helpers - Add proportional skip threshold in scored_points_to_entries - Handle malformed created_at with try/except - Fix section comment style in events/memory.py - Remove stale inline comments from InjectionStrategy enum Doc fixes: - Update design page ToolBased tab to match code reality - Fix protocol snippet str to NotBlankStr - Add hybrid search to tech-stack.md - Update README memory description Test fixes: - Add reformulation config field tests - Add personal_boost+RRF warning tests - Fix mock supports_sparse_search attributes - Tighten min_relevance and weight preservation assertions - Fix pytest.raises(Exception) to ValidationError - Remove hard-coded Hypothesis settings(max_examples=50)
…on bias RRF scores by list position (1/(k+rank)). Concatenating personal entries before shared entries biased RRF toward personal results regardless of relevance. Now both dense and sparse lists are sorted by relevance score before fusion so rank reflects actual backend scoring.
- Clarify both switches needed for hybrid retrieval in tech-stack.md - Only warn personal_boost+RRF when explicitly set (not on default 0.1) - Restore logger to sparse.py (BM25Tokenizer is business logic per convention) - Fix Unicode test to use actual non-ASCII input for NFKC/casefold coverage - Sort personal+shared by relevance before RRF to eliminate position bias
- Remove redundant shared_store None check (already guaranteed by shared_has_sparse) - Use sharp-s (STRASSE casefold) in Unicode test for better coverage
Change type: ignore[attr-defined] to type: ignore[union-attr] after removing the redundant None check -- mypy now reports union-attr since SharedKnowledgeStore | None lacks retrieve_sparse.
- Log at ERROR before re-raising system exceptions in _safe_call - Extract _merge_and_fuse helper from _execute_rrf_pipeline (50-line limit) - Simplify _fetch_sparse_memories: disable shared sparse (SharedKnowledgeStore lacks retrieve_sparse), remove dead TaskGroup orchestration - Skip test docstring and bare @settings() suggestions (not project requirements)
- Add truncation ellipsis in _format_results_summary - Update __init__.py docstring with new export categories - Thread sparse_search_enabled through build_config_from_company_config - Tighten test_applies_user_id_filter assertions (using, filter) - Add MemoryError/RecursionError propagation tests for reformulation - Clarify builtin MemoryError in test_tool_retriever comment
- Fix design doc: clarify dense uses SharedKnowledgeStore.search_shared(), sparse is personal-only (SharedKnowledgeStore lacks retrieve_sparse) - Log before re-raising system exceptions in prepare_messages - Unwrap ExceptionGroup to bare exception (match TaskGroup pattern) - Skip RRF when sparse is empty -- fall back to linear ranking - Extract async_try_sparse_upsert + async_retrieve_sparse into sparse_search.py, reducing adapter.py from 959 to 875 lines - Log system errors before re-raise in adapter sparse blocks - Update test patch paths for extracted sparse functions
- Defer _qdrant_client assignment until after sparse field init succeeds - Coerce tags_raw defensively (handle None, str, non-iterable) - Use getattr for retrieve_sparse method to avoid AttributeError - Assert forwarded sparse vector in search test
Sparse search results were missing expires_at (always None), causing apply_post_filters to skip expiry checks on BM25 results. Now extracts _synthorg_expires_at from metadata using the same parse_mem0_datetime helper as the dense path.
- Strip whitespace from _synthorg_source, treat blank as None - Log at WARNING before re-raising in async_init_sparse_field
- Force non-string source_raw to None (prevent int leaking to NotBlankStr) - Handle non-string created_at values, catch TypeError alongside ValueError - Ensure parsed datetime is always timezone-aware (assume UTC for naive)
…iency parsing - Escape </retrieved_memories> in memory content to prevent prompt injection - Use first-token exact match for sufficiency verdict (rejects 'NOT SUFFICIENT') - Clarify dense-only in design doc ToolBased tab
e36fdf8 to
bd1af9d
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (5)
src/synthorg/memory/backends/mem0/config.py (1)
321-322:⚠️ Potential issue | 🟠 Major
sparse_search_enabledis still not reachable through the standard config path.
build_config_from_company_config()accepts and forwards the flag, but the primary factory path shown insrc/synthorg/memory/factory.pydoes not pass it, andCompanyMemoryConfig(shown insrc/synthorg/memory/config.py) has no field to source it from. That keeps config-driven Mem0 creation effectively pinned toFalse.Please thread this from top-level memory config into the factory call path so sparse mode is actually user-configurable.
Also applies to: 368-371
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/memory/backends/mem0/config.py` around lines 321 - 322, CompanyMemoryConfig lacks a sparse_search_enabled field and the factory path does not pass it through; add a bool field sparse_search_enabled = False to CompanyMemoryConfig, update the factory function that instantiates Mem0 (the code in factory.py that calls build_config_from_company_config / constructs Mem0BackendConfig) to read CompanyMemoryConfig.sparse_search_enabled and pass it into build_config_from_company_config (and/or directly into Mem0BackendConfig), and ensure any helper/function signatures (e.g., build_config_from_company_config, the Mem0 constructor call) accept and forward sparse_search_enabled so config-driven Mem0 creation can enable sparse mode.src/synthorg/memory/backends/mem0/adapter.py (2)
376-392: 🛠️ Refactor suggestion | 🟠 MajorPlease extract the sparse adapter paths into their own module.
These additions push
src/synthorg/memory/backends/mem0/adapter.pypast the repo’s size cap; the file now reaches Line 876 and already documents earlier extractions done for the same reason. Moving_try_sparse_upsert()/retrieve_sparse()behind a small sparse adapter helper would keep this class focused on the Mem0 façade instead of accumulating more backend-specific branches.As per coding guidelines,
**/*.py: Keep Python functions under 50 lines and files under 800 lines.Also applies to: 782-798
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/memory/backends/mem0/adapter.py` around lines 376 - 392, The file has grown too large and backend-specific sparse logic should be moved to a dedicated adapter module: extract _try_sparse_upsert and retrieve_sparse (and their use of async_try_sparse_upsert, _sparse_encoder, _qdrant_client, and self._mem0_config.collection_name) into a new module (e.g., mem0_sparse_adapter) that exposes small async functions (e.g., try_sparse_upsert(...) and retrieve_sparse(...)); update the Mem0 adapter to import and delegate to those functions, remove backend branching from adapter.py, and ensure signatures remain the same so callers of _try_sparse_upsert and retrieve_sparse are unchanged.
189-203:⚠️ Potential issue | 🟠 MajorDon’t publish the connected state before sparse init settles.
This block runs after
self._clientandself._connectedhave already been assigned. Ifasync_init_sparse_field()raisesMemoryErrororRecursionError,connect()exits with a live client and_connected=True, so the nextconnect()short-circuits on a half-initialized backend. The generic downgrade branch also disables sparse support without any warning, which makes production diagnosis much harder.As per coding guidelines,
src/synthorg/**/*.py: Log all error paths at WARNING or ERROR in Python before raising; provide context in log messages.src/synthorg/memory/retriever.py (1)
305-306:⚠️ Potential issue | 🟡 MinorLog filter-stage system errors before re-raising them.
The generic filter-failure branch records
filter_strategy, butMemoryError/RecursionErrorskip straight toraise. Those failures are then only visible as a generic"pipeline"error inprepare_messages(), which drops the filter-stage context on the highest-severity path.As per coding guidelines,
src/synthorg/**/*.py: Log all error paths at WARNING or ERROR in Python before raising; provide context in log messages.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/memory/retriever.py` around lines 305 - 306, The except block that currently re-raises builtins.MemoryError and RecursionError in retriever.py skips logging and loses filter-stage context; update the exception handler in the function containing that except to log the error (using the module logger) at WARNING or ERROR level including the filter_strategy and any relevant pipeline identifiers, then re-raise the original exception; ensure this mirrors the generic filter-failure branch so prepare_messages() will see detailed context for MemoryError/RecursionError.src/synthorg/memory/backends/mem0/sparse_search.py (1)
63-72:⚠️ Potential issue | 🟡 MinorExisting sparse field modifier is not validated or migrated.
ensure_sparse_field()returns early if the field name exists without checking whether the existing field usesModifier.IDF. An older collection with abm25field usingModifier.NONEwould silently retain incorrect BM25 scoring.Per Qdrant documentation,
update_collection()can change an existing sparse vector field's modifier without recreating the collection.🛠️ Proposed fix to validate and migrate modifier
info = client.get_collection(collection_name) existing_sparse = info.config.params.sparse_vectors if existing_sparse is not None and field_name in existing_sparse: + if existing_sparse[field_name].modifier != models.Modifier.IDF: + client.update_collection( + collection_name=collection_name, + sparse_vectors_config={ + field_name: models.SparseVectorParams( + modifier=models.Modifier.IDF, + ), + }, + ) + logger.info( + MEMORY_SPARSE_FIELD_ENSURED, + collection=collection_name, + field_name=field_name, + action="migrated", + ) + return logger.debug( MEMORY_SPARSE_FIELD_ENSURED, collection=collection_name, field_name=field_name, action="skipped", ) return🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/memory/backends/mem0/sparse_search.py` around lines 63 - 72, ensure_sparse_field currently returns early when a sparse field exists but doesn't validate or migrate its modifier; update it to read existing_sparse[field_name].modifier (or equivalent) and if it's not Modifier.IDF call client.update_collection(...) to patch the field's params to use Modifier.IDF before returning. Reference ensure_sparse_field, existing_sparse, Modifier.IDF, client.update_collection and keep the existing MEMORY_SPARSE_FIELD_ENSURED log path but change the action to reflect a "migrated" result when you perform the update.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/memory/retrieval_config.py`:
- Around line 187-202: The warning in _validate_personal_boost_rrf_consistency
incorrectly asserts personal_boost "is not applied with RRF fusion" even when
the code may fall back to rank_memories which does apply personal_boost; change
the validation to either (a) only warn when a pure RRF-only path is guaranteed
(introduce or check an existing flag/condition that indicates no fallback will
occur before emitting the warning), or (b) relax the message to say
personal_boost "may not be applied when pure RRF is used; fallback ranking
(rank_memories) does apply personal_boost" so it is accurate; reference the
FusionStrategy.RRF enum, _validate_personal_boost_rrf_consistency,
personal_boost, model_fields_set and
ContextInjectionStrategy._execute_rrf_pipeline / rank_memories to locate and
implement the conditional or message change.
In `@src/synthorg/memory/sparse.py`:
- Around line 50-52: The _STOP_WORDS list currently contains the negators "no"
and "not", which collapses contradictory texts; remove "no" and "not" from the
_STOP_WORDS definition(s) (the occurrences in the _STOP_WORDS variable around
the top of the module and the second occurrence later in the file) so negation
is preserved during sparse term generation; update both places where _STOP_WORDS
is defined/extended (referencing the _STOP_WORDS symbol) to exclude those two
tokens.
In `@tests/unit/memory/test_reformulation.py`:
- Around line 45-47: Tests use the wrong reformulation API and wrong types:
replace the constructor arg completion_fn with complete when creating
LLMQueryReformulator, call reformulate(query, context) with context as a string
or None (not a tuple), and pass a list of MemoryEntry objects to
SufficiencyChecker.check_sufficiency (not tuples); update all instances in the
test file (including the blocks around lines 63–100 and 108–154) to construct
proper MemoryEntry instances, pass lists for results, and use the reformulate
signature that accepts (query, context: str | None) and the
SufficiencyChecker.check_sufficiency(results: list[MemoryEntry]) signature.
---
Duplicate comments:
In `@src/synthorg/memory/backends/mem0/adapter.py`:
- Around line 376-392: The file has grown too large and backend-specific sparse
logic should be moved to a dedicated adapter module: extract _try_sparse_upsert
and retrieve_sparse (and their use of async_try_sparse_upsert, _sparse_encoder,
_qdrant_client, and self._mem0_config.collection_name) into a new module (e.g.,
mem0_sparse_adapter) that exposes small async functions (e.g.,
try_sparse_upsert(...) and retrieve_sparse(...)); update the Mem0 adapter to
import and delegate to those functions, remove backend branching from
adapter.py, and ensure signatures remain the same so callers of
_try_sparse_upsert and retrieve_sparse are unchanged.
In `@src/synthorg/memory/backends/mem0/config.py`:
- Around line 321-322: CompanyMemoryConfig lacks a sparse_search_enabled field
and the factory path does not pass it through; add a bool field
sparse_search_enabled = False to CompanyMemoryConfig, update the factory
function that instantiates Mem0 (the code in factory.py that calls
build_config_from_company_config / constructs Mem0BackendConfig) to read
CompanyMemoryConfig.sparse_search_enabled and pass it into
build_config_from_company_config (and/or directly into Mem0BackendConfig), and
ensure any helper/function signatures (e.g., build_config_from_company_config,
the Mem0 constructor call) accept and forward sparse_search_enabled so
config-driven Mem0 creation can enable sparse mode.
In `@src/synthorg/memory/backends/mem0/sparse_search.py`:
- Around line 63-72: ensure_sparse_field currently returns early when a sparse
field exists but doesn't validate or migrate its modifier; update it to read
existing_sparse[field_name].modifier (or equivalent) and if it's not
Modifier.IDF call client.update_collection(...) to patch the field's params to
use Modifier.IDF before returning. Reference ensure_sparse_field,
existing_sparse, Modifier.IDF, client.update_collection and keep the existing
MEMORY_SPARSE_FIELD_ENSURED log path but change the action to reflect a
"migrated" result when you perform the update.
In `@src/synthorg/memory/retriever.py`:
- Around line 305-306: The except block that currently re-raises
builtins.MemoryError and RecursionError in retriever.py skips logging and loses
filter-stage context; update the exception handler in the function containing
that except to log the error (using the module logger) at WARNING or ERROR level
including the filter_strategy and any relevant pipeline identifiers, then
re-raise the original exception; ensure this mirrors the generic filter-failure
branch so prepare_messages() will see detailed context for
MemoryError/RecursionError.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6d3bfb9a-30d7-4879-a3df-b382d445b7b5
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (23)
CLAUDE.mdREADME.mddocs/architecture/tech-stack.mddocs/design/memory.mdpyproject.tomlsrc/synthorg/memory/__init__.pysrc/synthorg/memory/backends/mem0/adapter.pysrc/synthorg/memory/backends/mem0/config.pysrc/synthorg/memory/backends/mem0/sparse_search.pysrc/synthorg/memory/injection.pysrc/synthorg/memory/reformulation.pysrc/synthorg/memory/retrieval_config.pysrc/synthorg/memory/retriever.pysrc/synthorg/memory/sparse.pysrc/synthorg/memory/tool_retriever.pysrc/synthorg/observability/events/memory.pytests/unit/memory/backends/mem0/test_sparse_search.pytests/unit/memory/test_init.pytests/unit/memory/test_reformulation.pytests/unit/memory/test_retrieval_config.pytests/unit/memory/test_retriever.pytests/unit/memory/test_sparse.pytests/unit/memory/test_tool_retriever.py
src/synthorg/memory/sparse.py
Outdated
| "no", | ||
| "not", | ||
| "of", |
There was a problem hiding this comment.
Keep negators out of _STOP_WORDS.
Dropping "no" and "not" makes contradictory memories collapse to the same sparse terms—for example, "I am not vegetarian" and "I am vegetarian" both reduce to vegetarian. That lets the BM25 branch boost the exact opposite memory during hybrid retrieval.
Minimal fix
- "no",
- "not",Also applies to: 170-171
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/memory/sparse.py` around lines 50 - 52, The _STOP_WORDS list
currently contains the negators "no" and "not", which collapses contradictory
texts; remove "no" and "not" from the _STOP_WORDS definition(s) (the occurrences
in the _STOP_WORDS variable around the top of the module and the second
occurrence later in the file) so negation is preserved during sparse term
generation; update both places where _STOP_WORDS is defined/extended
(referencing the _STOP_WORDS symbol) to exclude those two tokens.
| reformulator = LLMQueryReformulator( | ||
| completion_fn=_mock_completion("rewritten"), | ||
| ) |
There was a problem hiding this comment.
These tests don’t match the published reformulation interfaces.
src/synthorg/memory/reformulation.py publishes LLMQueryReformulator.__init__(complete), reformulate(query, context: str | None), and SufficiencyChecker.check_sufficiency(query, results: list[MemoryEntry]). Here the reformulator cases use completion_fn= and pass tuples of MemoryEntry as the second argument, and the sufficiency cases pass tuples instead of lists. That is brittle at runtime and should fail strict typing on tests/.
Minimal fix pattern
- reformulator = LLMQueryReformulator(completion_fn=mock_fn)
- result = await reformulator.reformulate("query", ())
+ reformulator = LLMQueryReformulator(mock_fn)
+ result = await reformulator.reformulate("query", None)
- result = await checker.check_sufficiency("query", ())
+ result = await checker.check_sufficiency("query", [])Also applies to: 63-100, 108-154
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/memory/test_reformulation.py` around lines 45 - 47, Tests use the
wrong reformulation API and wrong types: replace the constructor arg
completion_fn with complete when creating LLMQueryReformulator, call
reformulate(query, context) with context as a string or None (not a tuple), and
pass a list of MemoryEntry objects to SufficiencyChecker.check_sufficiency (not
tuples); update all instances in the test file (including the blocks around
lines 63–100 and 108–154) to construct proper MemoryEntry instances, pass lists
for results, and use the reformulate signature that accepts (query, context: str
| None) and the SufficiencyChecker.check_sufficiency(results: list[MemoryEntry])
signature.
… log filter re-raise - Warning message now says 'may not be applied' (fallback to rank_memories does apply it) - Remove 'no' and 'not' from BM25 stop words to preserve negation semantics - Log system errors before re-raise in memory filter exception handler
🤖 I have created a release *beep* *boop* --- ## [0.5.9](v0.5.8...v0.5.9) (2026-04-03) ### Features * ceremony template defaults + strategy migration UX ([#1031](#1031)) ([da4a8e1](da4a8e1)), closes [#976](#976) [#978](#978) * hybrid search (dense + BM25 sparse) for memory retrieval pipeline ([#1016](#1016)) ([fccac4a](fccac4a)), closes [#694](#694) * implement network hosting and multi-user access ([#1032](#1032)) ([398c378](398c378)), closes [#244](#244) * implement visual workflow editor ([#247](#247)) ([#1018](#1018)) ([ef5d3c1](ef5d3c1)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Summary
Implements hybrid search (dense + BM25 sparse) for the memory retrieval pipeline, wiring RRF fusion, adding a tool-based injection strategy, and providing agentic query reformulation building blocks.
Changes
memory/sparse.py):BM25Tokenizerwith mmh3 hash-based vocabulary-free encoding.SparseVectorfrozen Pydantic model with sorted/non-negative/positive validationbackends/mem0/sparse_search.py):ensure_sparse_field()addsModifier.IDFsparse field to Qdrant collection;upsert_sparse_vector()attaches BM25 vectors to existing points;search_sparse()queries sparse field with user_id filteringbackends/mem0/adapter.py): Sparse encoder init onconnect(), sparse upsert onstore()(non-fatal),retrieve_sparse()method,supports_sparse_searchproperty. Opt-in viasparse_search_enabledconfigretriever.py):_execute_rrf_pipeline()runs dense + sparse fetches in parallel viaTaskGroup, merges via existingfuse_ranked_lists(), applies post-RRFmin_relevancefilter._execute_linear_pipeline()preserves existing dense-only pathtool_retriever.py):ToolBasedInjectionStrategywithsearch_memory(semantic + category filter) andrecall_memory(by ID) tools. Generic error messages to LLM (no internal detail leakage)reformulation.py):QueryReformulatorandSufficiencyCheckerruntime-checkable protocols withLLMQueryReformulatorandLLMSufficiencyCheckerimplementations. XML-tagged prompt boundaries for injection defenseFusionStrategy.RRFandInjectionStrategy.TOOL_BASEDnow accepted. New fields:query_reformulation_enabled,max_reformulation_roundswith cross-validatormmh3==5.2.1(murmurhash3)Test Plan
Review Coverage
Pre-reviewed by 10 agents, 24 findings addressed:
Closes #694