refactor: load embedding model asynchronously#138
Open
Anisha-45 wants to merge 3 commits into
Open
Conversation
Contributor
WalkthroughAsynchronous, non-blocking model loading and robust fallback behavior were added to Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant MainThread as Main<br/>(initialize_model)
participant BgThread as Background<br/>Loader
participant Model as SentenceTransformer
participant Vectorizer as vectorize
Caller->>MainThread: initialize_model()
MainThread->>MainThread: acquire _model_lock\nset _model_loading=True\nspawn BgThread
MainThread->>Caller: return (non-blocking)
BgThread->>Model: load model (blocking)
alt load success
Model-->>BgThread: model ready
BgThread->>MainThread: acquire _model_lock\nset _model_loading=False\nstore model
else load failure
BgThread->>MainThread: acquire _model_lock\nset _model_loading=False\nset _model_load_error
end
Caller->>Vectorizer: vectorize(query)
Vectorizer->>MainThread: check model state
alt model loaded
Vectorizer->>Model: model.encode(query)
Model-->>Vectorizer: embeddings
Vectorizer->>Caller: return embeddings
else model loading or errored
Vectorizer->>Vectorizer: use fallback deterministic vectorizer
Vectorizer->>Caller: return fallback embeddings
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🎉 Thanks for opening your first pull request in NexumDB!
We appreciate your contribution! Here's what happens next:
- ✅ Automated checks will run (CI, tests, linting)
- 👀 A maintainer will review your changes
- 💬 We may suggest some improvements or ask questions
- 🚀 Once approved, we'll merge your contribution!
Before we can merge:
- Make sure all CI checks pass
- Sign your commits with DCO (
git commit -s) - Address any review feedback
Check out our Contributing Guide for more details.
Thanks for making NexumDB better! 🙌
Owner
|
@Anisha-45 Please mention the issue the PR fixes in the description |
Author
|
Done — updated the PR description to include |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@nexum_ai/optimizer.py`:
- Around line 151-153: There are multiple consecutive blank lines between method
definitions in the class in nexum_ai/optimizer.py; edit the file to leave
exactly one blank line between each method definition (for example between
methods like optimize, _prepare_batch, _run_epoch or other def ... blocks inside
the Optimizer class) so the class follows PEP 8 spacing rules; remove any extra
empty lines but do not change method signatures or logic.
- Around line 140-141: vectorize() can spawn unbounded background threads
because initialize_model() will retry every call if model is None even after
_load() failed; modify initialize_model() (or add a guard in vectorize()) to
check a new/existing boolean flag _model_load_error set by _load() on failure
and refuse to start a new background loader while _model_load_error is true
(optionally log the original exception), and ensure _load() sets
_model_load_error = True on exception and clears it only when a successful load
completes so failed loads are not retried on every vectorize() call.
- Around line 136-149: vectorize currently mixes _fallback_vectorize vectors
with SentenceTransformer.encode vectors which breaks caching; change vectorize
so it does not return or store fallback vectors when the real model will later
be used: call initialize_model() and wait for model readiness (or await its
loading-future/flag) before encoding, and only use _fallback_vectorize as a
last-resort when model initialization fails; also ensure put()/get() paths do
not cache or compare fallback vectors by checking self.model is ready before any
cache operations. Reference: vectorize, initialize_model, _fallback_vectorize,
self.model, get(), put().
Comment on lines
151
to
153
|
|
||
|
|
||
|
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Extraneous blank lines between methods.
PEP 8 recommends a single blank line between method definitions inside a class. There are multiple consecutive blank lines here.
🤖 Prompt for AI Agents
In `@nexum_ai/optimizer.py` around lines 151 - 153, There are multiple consecutive
blank lines between method definitions in the class in nexum_ai/optimizer.py;
edit the file to leave exactly one blank line between each method definition
(for example between methods like optimize, _prepare_batch, _run_epoch or other
def ... blocks inside the Optimizer class) so the class follows PEP 8 spacing
rules; remove any extra empty lines but do not change method signatures or
logic.
Author
|
Sure @aviralgarg05 — I’m working on the requested fixes and will push an updated commit soon. |
github-actions
Bot
added
documentation
github-actions
Bot
removed
documentation
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@nexum_ai/optimizer.py`:
- Around line 639-694: The two helper functions test_vectorization and
test_cache_persistence are test/demo utilities that should be removed from the
production module and relocated to a proper test file; create a new test module
(e.g., tests/test_optimizer.py) and move the functions there as pytest-style
tests that instantiate SemanticCache (use tmp_path or tmp_path_factory for
cache_file to avoid polluting cache/ and mock or stub model
downloads/vectorization if possible), update imports to reference SemanticCache
from nexum_ai.optimizer, and remove these helpers from the production file so
the optimizer module no longer performs disk I/O or model downloads on import.
- Around line 55-59: The default cache_file in Optimizer.__init__ is incorrectly
"semantic_cache.pkl" while the implementation always uses JSON; change the
default to "semantic_cache.json" and remove any .replace(".pkl", ".json")
transformations in methods that read/write the cache (search for uses of
cache_file and replace(".pkl", ".json") in save/load logic). Update the
constructor signature to use cache_file: str = "semantic_cache.json", and ensure
all code paths directly use the cache_file value (already treated as JSON)
without runtime string replacement.
- Around line 317-319: The current bare except swallows error types and
unconditionally wipes self.cache; change it to explicitly handle
json.JSONDecodeError and OSError/PermissionError: import json, catch
json.JSONDecodeError and log logger.exception with the cache file path and error
details (do not overwrite self.cache so you preserve the in-memory data), catch
OSError/PermissionError and log the filepath and permission/IO details and only
then decide whether to set self.cache = [] (or propagate); avoid a bare except
Exception and ensure logger.exception includes the filename and specific
exception type in the message for easier diagnosis.
- Line 14: Remove the module-level call to logging.basicConfig() in optimizer.py
and instead obtain the module logger via logging.getLogger(__name__) and attach
a logging.NullHandler() to it (e.g., addHandler(logging.NullHandler())) so the
library does not configure the root logger; ensure no other module-level logging
configuration calls remain in optimizer.py.
- Around line 242-257: load_cache currently appends ".json" to any non-".pkl"
filepath which can create mismatched names versus save_cache; update the
json_filepath logic in load_cache to mirror save_cache: if filepath endswith
".pkl" replace ".pkl" with ".json", otherwise leave filepath unchanged (so
passing "my_cache.json" stays "my_cache.json"); adjust the computation that sets
json_filepath used before calling load_cache_json and keep references to
load_cache, save_cache, load_cache_json and self.cache_path to locate the
change.
- Around line 307-309: In load_cache_json, do not blindly assign
self.similarity_threshold from the JSON; validate and clamp it to [0.0, 1.0]
(and ensure it's a numeric type) before setting the instance value.
Specifically, read the candidate from data.get("similarity_threshold",
self.similarity_threshold), check it's a float/int, coerce if necessary, then
clamp via min(max(candidate, 0.0), 1.0) and only then assign to
self.similarity_threshold; if the value is invalid, keep the existing
self.similarity_threshold.
- Around line 264-271: The save routine (save_cache_json) is mutating self.cache
by reassigning cleaned back to self.cache; change it to be read-only: build a
local cleaned list from self.cache (as you already do) and use that local
cleaned only for serialization/output without reassigning self.cache, and
instead add validation at insertion time in put to prevent invalid vectors
entering the cache (validate vector shape/type in put before appending to
self.cache). Ensure you reference the existing cleaned local, the
save_cache_json function, and the put method when implementing these changes.
| import threading | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
| logging.basicConfig(level=logging.INFO) |
Contributor
There was a problem hiding this comment.
Library module should not call logging.basicConfig().
This module is imported by the Rust bridge and potentially other consumers. Calling basicConfig at module level hijacks the root logger for the entire process, overriding any configuration the host application may have set. Per Python logging best practices, libraries should only attach a NullHandler to their own logger.
Proposed fix
logger = logging.getLogger(__name__)
-logging.basicConfig(level=logging.INFO)
+logger.addHandler(logging.NullHandler())🤖 Prompt for AI Agents
In `@nexum_ai/optimizer.py` at line 14, Remove the module-level call to
logging.basicConfig() in optimizer.py and instead obtain the module logger via
logging.getLogger(__name__) and attach a logging.NullHandler() to it (e.g.,
addHandler(logging.NullHandler())) so the library does not configure the root
logger; ensure no other module-level logging configuration calls remain in
optimizer.py.
Comment on lines
+55
to
+59
| def __init__( | ||
| self, | ||
| similarity_threshold: float = 0.95, | ||
| cache_file: str = "semantic_cache.pkl", | ||
| ) -> None: |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
The default cache_file parameter is "semantic_cache.pkl" but all persistence is JSON-based.
Every save/load path converts .pkl → .json. The .pkl default is a vestige of the removed pickle support and misleads callers about the actual storage format. Consider updating the default to "semantic_cache.json" and removing the .replace(".pkl", ".json") indirection throughout.
🤖 Prompt for AI Agents
In `@nexum_ai/optimizer.py` around lines 55 - 59, The default cache_file in
Optimizer.__init__ is incorrectly "semantic_cache.pkl" while the implementation
always uses JSON; change the default to "semantic_cache.json" and remove any
.replace(".pkl", ".json") transformations in methods that read/write the cache
(search for uses of cache_file and replace(".pkl", ".json") in save/load logic).
Update the constructor signature to use cache_file: str = "semantic_cache.json",
and ensure all code paths directly use the cache_file value (already treated as
JSON) without runtime string replacement.
Comment on lines
242
to
+257
| def load_cache(self, filepath: Optional[str] = None) -> None: | ||
| """Load cache from disk using JSON (safe) or pickle (legacy)""" | ||
| """Load cache from disk using JSON format""" | ||
| if filepath is None: | ||
| filepath = str(self.cache_path) | ||
|
|
||
| # Try JSON first (safer format) | ||
| json_filepath = filepath.replace('.pkl', '.json') if filepath.endswith('.pkl') else f"{filepath}.json" | ||
| if os.path.exists(json_filepath): | ||
| self.load_cache_json(json_filepath) | ||
|
|
||
| json_filepath = ( | ||
| filepath.replace(".pkl", ".json") | ||
| if filepath.endswith(".pkl") | ||
| else f"{filepath}.json" | ||
| ) | ||
|
|
||
| if not os.path.exists(json_filepath): | ||
| self.cache = [] | ||
| return | ||
|
|
||
| # Fall back to pickle for legacy files (with restricted unpickler for safety) | ||
| if os.path.exists(filepath) and filepath.endswith('.pkl'): | ||
| try: | ||
| import pickle | ||
|
|
||
| # Use RestrictedUnpickler to limit allowed classes | ||
| class RestrictedUnpickler(pickle.Unpickler): | ||
| """Restricted unpickler that only allows safe types""" | ||
| ALLOWED_CLASSES = { | ||
| ('builtins', 'dict'), | ||
| ('builtins', 'list'), | ||
| ('builtins', 'str'), | ||
| ('builtins', 'int'), | ||
| ('builtins', 'float'), | ||
| ('builtins', 'bool'), | ||
| ('builtins', 'tuple'), | ||
| ('builtins', 'set'), | ||
| ('builtins', 'frozenset'), | ||
| } | ||
|
|
||
| def find_class(self, module: str, name: str) -> type: | ||
| if (module, name) not in self.ALLOWED_CLASSES: | ||
| raise pickle.UnpicklingError( | ||
| f"Forbidden class: {module}.{name}" | ||
| ) | ||
| return super().find_class(module, name) | ||
|
|
||
| with open(filepath, 'rb') as f: | ||
| data = RestrictedUnpickler(f).load() | ||
|
|
||
| self.cache = data.get('cache', []) | ||
| self.similarity_threshold = data.get('similarity_threshold', self.similarity_threshold) | ||
|
|
||
| logger.info( | ||
| "Semantic cache loaded from %s (%d entries)", | ||
| filepath, | ||
| len(self.cache), | ||
| ) | ||
|
|
||
| logger.info( | ||
| "Converting legacy pickle cache to JSON format for security" | ||
| ) | ||
|
|
||
| # Validate cache entries | ||
| valid_entries = [] | ||
| for entry in self.cache: | ||
| if all(key in entry for key in ['query', 'vector', 'result']): | ||
| valid_entries.append(entry) | ||
| else: | ||
| logger.info("Warning: Invalid cache entry found and removed") | ||
|
|
||
| self.cache = valid_entries | ||
|
|
||
| # Auto-convert to JSON format for future use | ||
| self.save_cache_json(json_filepath) | ||
|
|
||
| except Exception: | ||
| logger.exception( | ||
| "Error loading semantic cache" | ||
| ) | ||
|
|
||
| logger.debug( | ||
| "Starting with empty cache" | ||
| ) | ||
| self.cache = [] | ||
| else: | ||
| logger.debug(f"No cache file found at {filepath}, starting with empty cache") | ||
|
|
||
|
|
||
| self.load_cache_json(json_filepath) |
Contributor
There was a problem hiding this comment.
Bug: load_cache and save_cache derive different JSON paths for non-.pkl filenames.
save_cache (Line 238) keeps a non-.pkl filepath as-is, but load_cache (Line 250) unconditionally appends .json, producing paths like "foo.json.json". This means save_cache("my_cache.json") writes to my_cache.json, while load_cache("my_cache.json") reads from my_cache.json.json — the cache will silently fail to load.
Proposed fix: align with save_cache logic
def load_cache(self, filepath: Optional[str] = None) -> None:
"""Load cache from disk using JSON format"""
if filepath is None:
filepath = str(self.cache_path)
json_filepath = (
filepath.replace(".pkl", ".json")
if filepath.endswith(".pkl")
- else f"{filepath}.json"
+ else filepath
)
if not os.path.exists(json_filepath):
self.cache = []
return
self.load_cache_json(json_filepath)🤖 Prompt for AI Agents
In `@nexum_ai/optimizer.py` around lines 242 - 257, load_cache currently appends
".json" to any non-".pkl" filepath which can create mismatched names versus
save_cache; update the json_filepath logic in load_cache to mirror save_cache:
if filepath endswith ".pkl" replace ".pkl" with ".json", otherwise leave
filepath unchanged (so passing "my_cache.json" stays "my_cache.json"); adjust
the computation that sets json_filepath used before calling load_cache_json and
keep references to load_cache, save_cache, load_cache_json and self.cache_path
to locate the change.
Comment on lines
+264
to
+271
| # remove bad entries (vector None) | ||
| cleaned = [] | ||
| for entry in self.cache: | ||
| vec = entry.get("vector") | ||
| if isinstance(vec, list) and len(vec) > 0: | ||
| cleaned.append(entry) | ||
|
|
||
| self.cache = cleaned |
Contributor
There was a problem hiding this comment.
save_cache_json silently mutates self.cache by pruning entries — unexpected side effect for a save operation.
Lines 265–271 filter out entries with invalid vectors and reassign self.cache. A save method should be read-only with respect to in-memory state. This can silently change cache size mid-operation and mask bugs where invalid vectors were inserted. Either clean at insertion time (put) or build cleaned only for serialization without overwriting self.cache.
Proposed fix: don't mutate self.cache during save
- # remove bad entries (vector None)
- cleaned = []
- for entry in self.cache:
- vec = entry.get("vector")
- if isinstance(vec, list) and len(vec) > 0:
- cleaned.append(entry)
-
- self.cache = cleaned
-
cache_data = {
- "cache": self.cache,
+ "cache": [
+ entry for entry in self.cache
+ if isinstance(entry.get("vector"), list) and len(entry.get("vector", [])) > 0
+ ],
"similarity_threshold": self.similarity_threshold,
- "cache_size": len(self.cache),
+ "cache_size": len(self.cache), # reflects actual in-memory count
"format_version": "1.0",
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # remove bad entries (vector None) | |
| cleaned = [] | |
| for entry in self.cache: | |
| vec = entry.get("vector") | |
| if isinstance(vec, list) and len(vec) > 0: | |
| cleaned.append(entry) | |
| self.cache = cleaned | |
| cache_data = { | |
| "cache": [ | |
| entry for entry in self.cache | |
| if isinstance(entry.get("vector"), list) and len(entry.get("vector", [])) > 0 | |
| ], | |
| "similarity_threshold": self.similarity_threshold, | |
| "cache_size": len(self.cache), # reflects actual in-memory count | |
| "format_version": "1.0", | |
| } |
🤖 Prompt for AI Agents
In `@nexum_ai/optimizer.py` around lines 264 - 271, The save routine
(save_cache_json) is mutating self.cache by reassigning cleaned back to
self.cache; change it to be read-only: build a local cleaned list from
self.cache (as you already do) and use that local cleaned only for
serialization/output without reassigning self.cache, and instead add validation
at insertion time in put to prevent invalid vectors entering the cache (validate
vector shape/type in put before appending to self.cache). Ensure you reference
the existing cleaned local, the save_cache_json function, and the put method
when implementing these changes.
Comment on lines
+307
to
+309
| self.similarity_threshold = data.get( | ||
| "similarity_threshold", self.similarity_threshold | ||
| ) |
Contributor
There was a problem hiding this comment.
load_cache_json overwrites similarity_threshold from the JSON file without validation.
Line 307–309 replaces the instance's similarity_threshold with whatever value is in the JSON. A corrupted or hand-edited file could set it to a negative number or > 1.0, breaking cache-hit logic. Consider clamping to [0.0, 1.0].
Proposed fix
- self.similarity_threshold = data.get(
- "similarity_threshold", self.similarity_threshold
- )
+ loaded_threshold = data.get("similarity_threshold", self.similarity_threshold)
+ self.similarity_threshold = max(0.0, min(1.0, float(loaded_threshold)))🤖 Prompt for AI Agents
In `@nexum_ai/optimizer.py` around lines 307 - 309, In load_cache_json, do not
blindly assign self.similarity_threshold from the JSON; validate and clamp it to
[0.0, 1.0] (and ensure it's a numeric type) before setting the instance value.
Specifically, read the candidate from data.get("similarity_threshold",
self.similarity_threshold), check it's a float/int, coerce if necessary, then
clamp via min(max(candidate, 0.0), 1.0) and only then assign to
self.similarity_threshold; if the value is invalid, keep the existing
self.similarity_threshold.
Comment on lines
+317
to
+319
| except Exception: | ||
| logger.exception("Error loading cache from JSON") | ||
| self.cache = [] |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Bare except Exception loses the specific error type; logger.exception is good but self.cache = [] silently discards any partial data.
If the JSON is partially valid (e.g., truncated write), a JSONDecodeError is caught and the cache is silently wiped. Consider logging the filepath and distinguishing between "file corrupt" vs. "permission error" so operators can diagnose issues.
🤖 Prompt for AI Agents
In `@nexum_ai/optimizer.py` around lines 317 - 319, The current bare except
swallows error types and unconditionally wipes self.cache; change it to
explicitly handle json.JSONDecodeError and OSError/PermissionError: import json,
catch json.JSONDecodeError and log logger.exception with the cache file path and
error details (do not overwrite self.cache so you preserve the in-memory data),
catch OSError/PermissionError and log the filepath and permission/IO details and
only then decide whether to set self.cache = [] (or propagate); avoid a bare
except Exception and ensure logger.exception includes the filename and specific
exception type in the message for easier diagnosis.
Comment on lines
+639
to
694
| def test_vectorization() -> Dict[str, Any]: | ||
| """Test function for vectorization""" | ||
| cache = SemanticCache() | ||
| test_query = "SELECT * FROM users WHERE age > 25" | ||
| vector = cache.vectorize(test_query) | ||
|
|
||
| return { | ||
| "query": test_query, | ||
| "vector": vector[:10], | ||
| "dimension": len(vector), | ||
| "note": "Vector created successfully (model or fallback).", | ||
| } | ||
|
|
||
|
|
||
| def test_cache_persistence() -> Dict[str, Any]: | ||
| """Test semantic cache persistence functionality""" | ||
| logger.debug("=" * 60) | ||
| logger.debug("Testing Semantic Cache Persistence") | ||
| logger.debug("=" * 60) | ||
|
|
||
| # Test 1: Create cache and add entries | ||
| logger.info("1. Creating cache and adding test entries...") | ||
|
|
||
| logger.info("=" * 60) | ||
| logger.info("Testing Semantic Cache Persistence") | ||
| logger.info("=" * 60) | ||
|
|
||
| cache1 = SemanticCache(cache_file="test_cache.pkl") | ||
|
|
||
| test_queries = [ | ||
| ("SELECT * FROM users WHERE age > 25", "User data for age > 25"), | ||
| ("SELECT name FROM products WHERE price < 100", "Product names under $100"), | ||
| ("SELECT COUNT(*) FROM orders WHERE status = 'active'", "Active order count: 42") | ||
| ("SELECT COUNT(*) FROM orders WHERE status = 'active'", "Active order count: 42"), | ||
| ] | ||
|
|
||
| for query, result in test_queries: | ||
| cache1.put(query, result) | ||
|
|
||
| # Save cache after adding entries | ||
|
|
||
| cache1.save_cache() | ||
|
|
||
| stats1 = cache1.get_cache_stats() | ||
| logger.info(f"Cache stats after adding entries: {stats1}") | ||
|
|
||
| # Test 2: Create new cache instance and verify persistence | ||
| logger.info("\n2. Creating new cache instance to test persistence...") | ||
| logger.info("Cache stats after adding entries: %s", stats1) | ||
|
|
||
| cache2 = SemanticCache(cache_file="test_cache.pkl") | ||
|
|
||
| stats2 = cache2.get_cache_stats() | ||
| logger.info(f"Cache stats after reload: {stats2}") | ||
|
|
||
| # Test 3: Verify cache hits work after reload | ||
| logger.info("\n3. Testing cache hits after reload...") | ||
| for query, expected_result in test_queries: | ||
| logger.info("Cache stats after reload: %s", stats2) | ||
|
|
||
| for query, _ in test_queries: | ||
| cached_result = cache2.get(query) | ||
| if cached_result: | ||
| logger.info(f"✓ Cache hit for: {query[:30]}...") | ||
| logger.info(f" Result: {cached_result[:50]}...") | ||
| logger.info("✓ Cache hit for: %s", query[:30]) | ||
| else: | ||
| logger.info(f"✗ Cache miss for: {query[:30]}...") | ||
|
|
||
| # Test 4: Test JSON export | ||
| logger.info("\n4. Testing JSON export...") | ||
| cache2.save_cache_json("test_cache.json") | ||
|
|
||
| # Test 5: Test cache optimization | ||
| logger.info("\n5. Testing cache optimization...") | ||
| cache2.optimize_cache(max_entries=2) | ||
|
|
||
| # Cleanup | ||
| logger.info("\n6. Cleaning up test files...") | ||
| logger.info("✗ Cache miss for: %s", query[:30]) | ||
|
|
||
| cache2.clear() | ||
|
|
||
| return { | ||
| 'test_passed': True, | ||
| 'entries_before_reload': stats1['total_entries'], | ||
| 'entries_after_reload': stats2['total_entries'], | ||
| 'persistence_working': stats1['total_entries'] == stats2['total_entries'] | ||
| "test_passed": True, | ||
| "entries_before_reload": stats1["total_entries"], | ||
| "entries_after_reload": stats2["total_entries"], | ||
| "persistence_working": stats1["total_entries"] == stats2["total_entries"], | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Move test/demo helpers out of the production module.
test_vectorization and test_cache_persistence live alongside production code and are only exercised via __main__. They create real SemanticCache instances (triggering model downloads and disk I/O), leave artifacts in cache/, and aren't wired into any test runner. Move them to a dedicated test file (e.g., tests/test_optimizer.py) using pytest or unittest so they run in CI and don't pollute the importable module.
🤖 Prompt for AI Agents
In `@nexum_ai/optimizer.py` around lines 639 - 694, The two helper functions
test_vectorization and test_cache_persistence are test/demo utilities that
should be removed from the production module and relocated to a proper test
file; create a new test module (e.g., tests/test_optimizer.py) and move the
functions there as pytest-style tests that instantiate SemanticCache (use
tmp_path or tmp_path_factory for cache_file to avoid polluting cache/ and mock
or stub model downloads/vectorization if possible), update imports to reference
SemanticCache from nexum_ai.optimizer, and remove these helpers from the
production file so the optimizer module no longer performs disk I/O or model
downloads on import.
|
CI was failing. Attempt #1 to fix. Fixed: Added missing method to QueryOptimizer class and added Q-values display to explain output. The following tests were failing due to missing code:
Changes made:
All 97 tests now pass locally. Note: Unable to push changes due to write access restrictions. Please apply the attached diff or manually implement the changes. CI will re-run automatically once changes are applied. |
|
CI is failing (Python tests). The Python tests are failing - please check the test output at: |
aviralgarg05
requested changes
Mar 5, 2026
aviralgarg05
left a comment
aviralgarg05
left a comment
Owner
There was a problem hiding this comment.
Looks like you have used a lot of AI while creating this PR. Moreover the CI workflow fails for your PR.
Lastly there are merge conflicts so resolve them as well. And try to be a genuine contributor, else I will close the PRs made by LLMs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Refactors
SemanticCache.initialize_model()to avoid blocking the main thread by loading the SentenceTransformer model asynchronously in a background worker thread.What Changed?
SemanticCachevectorize()triggers model loading without blockingWhy?
Previously,
initialize_model()loaded the SentenceTransformer synchronously, which could block the main thread during first use (especially on startup / first EXPLAIN call). This change keeps the CLI responsive and avoids startup delays.Type of Change
Testing
Local Testing Checklist
cargo fmt --all -- --checkcargo clippy --workspace --all-targets -- -D warningscargo test --workspace -- --test-threads=1cargo auditpython -m ruff check nexum_ai/python -m compileall nexum_aiTesting Details
py -3.12 -u nexum_ai/optimizer.pySentenceTransformerloads successfully using:py -3.12 -c "from sentence_transformers import SentenceTransformer; print('OK')"vectorize()while model loadsPerformance Impact
Security Considerations
Documentation
Pre-Merge Checklist
main🎯 OSCG'26 Quality Standards
Closes Refactor: Async Model Loading #120
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Chores