vector performance improvemewnts - #38
Merged
Merged
Conversation
Cache now stores pre-serialized bytes instead of raw dicts. FastAPI passes bytes through as Response directly - zero serialization overhead on cache hits. Per-thread event loop avoids asyncio.run() creating and tearing down a new loop on every cache call. Benchmark: 4.8x faster under 200req concurrency (0.46s to 0.097s).
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical and moderate findings affect authentication, async memory operations, stock-query correctness, startup resilience, and test collection.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This pull request improves vector-search and stock-cache performance while updating scheduling, authentication, sandbox/MCP integration, and regression coverage.
Changes:
- Adds normalized vector search and matrix caching.
- Presorts stock data and updates query handling.
- Refactors service scheduling and authentication/session wiring.
- Expands tests across memory, stocks, MCP, cookies, and concurrency.
File summaries
| File | Review notes |
|---|---|
TODO.md |
nit (2 votes, line 13): Correct spelling and capitalization. |
tests/test_workspace_endpoints.py |
No final comment. |
tests/test_vector_normalize.py |
No final comment. |
tests/test_user_deps.py |
No final comment. |
tests/test_thread_safety.py |
moderate (3 votes, line 99; 1 vote, line 124): Restore underscores so both tests are collected. |
tests/test_sync_cache.py |
No final comment. |
tests/test_stocks_presorted.py |
No final comment. |
tests/test_stocks_mcp.py |
No final comment. |
tests/test_stocks_mcp_middleware.py |
No final comment. |
tests/test_stocks_compress.py |
No final comment. |
tests/test_stocks_cache_zstd.py |
No final comment. |
tests/test_stocks_api_coverage.py |
No final comment. |
tests/test_sso_state_cookie.py |
No final comment. |
tests/test_sandbox.py |
No final comment. |
tests/test_sandbox_persistence.py |
No final comment. |
tests/test_relevance_score.py |
moderate (1 vote, line 13): Preserve explicit None and provide createdAt to exercise the fallback. |
tests/test_prometheus_tools.py |
No final comment. |
tests/test_prometheus_resume.py |
No final comment. |
tests/test_prometheus_auth_coverage.py |
No final comment. |
tests/test_oauth_cookie_security.py |
No final comment. |
tests/test_memory_search.py |
moderate (1 vote, line 145): Rename the duplicate test class so both fallback tests are collected. |
tests/test_memory_maintenance.py |
No final comment. |
tests/test_memory_dedup.py |
No final comment. |
tests/test_mcp_pool.py |
moderate (1 vote, line 65): Keep failed server connections retryable after partial initialization. |
tests/test_matrix_cache.py |
No final comment. |
tests/test_logout_cookie_domain.py |
No final comment. |
tests/test_logging_config_coverage.py |
No final comment. |
tests/test_controllers_coverage.py |
No final comment. |
tests/conftest.py |
No final comment. |
README.md |
No final comment. |
migrations/versions/f1a2b3c4d5e6_session_gc_index.py |
No final comment. |
main/utils/service_manager.py |
No final comment. |
main/utils/scheduler.py |
No final comment. |
main/service/user_service.py |
No final comment. |
main/service/stocksapi_service.py |
No final comment. |
main/service/scraper_service.py |
No final comment. |
main/service/prometheus_service.py |
critical (1 vote, line 71): Avoid uncaught synchronous model loading during startup. |
main/models/user.py |
No final comment. |
main/models/user_session.py |
No final comment. |
main/models/stocksapi_key.py |
No final comment. |
main/models/prometheus.py |
No final comment. |
main/models/memory.py |
No final comment. |
main/models/__init__.py |
No final comment. |
main/controller/user_controller.py |
No final comment. |
main/controller/stocksapi_controller.py |
No final comment. |
main/controller/prometheus_controller.py |
No final comment. |
main/controller/authentication_controller.py |
No final comment. |
main/app/user/user.py |
No final comment. |
main/app/stocks_api/util.py |
No final comment. |
main/app/stocks_api/sync_cache.py |
No final comment. |
main/app/stocks_api/query.py |
critical (1 vote each, line 225): Snapshot the frame and index atomically; retain all ticker rows before date filtering. |
main/app/stocks_api/key.py |
No final comment. |
main/app/stocks_api/cache.py |
moderate (1 vote, line 175): Normalize mixed TIME dtypes before sorting or reject the cache. |
main/app/scraper_b3/scraper.py |
No final comment. |
main/app/prometheus/vector.py |
No final comment. |
main/app/prometheus/sandbox.py |
No final comment. |
main/app/prometheus/memory.py |
critical (3 votes, line 45; 2 votes, line 54): Remove nested event-loop calls from async search and invalidation paths. moderate (1 vote, line 242): Build memory candidates from query-specific indexes before applying the cap. |
main/app/prometheus/mcp.py |
moderate (1 vote, line 65): Retry failed configured servers after partial initialization. |
main/app/prometheus/compact.py |
moderate (3 votes, lines 19, 83): Preserve the complete fallback registry and do not permanently cache failed loads. nit (1 vote, line 116; 2 votes, lines 126, 149): Reuse module-level compiled regexes. |
main/app/prometheus/chat.py |
No final comment. |
main/app/prometheus/agent.py |
No final comment. |
main/app/authentication/util.py |
No final comment. |
main/app/authentication/session.py |
No final comment. |
main/app/authentication/authentication.py |
No final comment. |
graphify-out/manifest.json |
No final comment. |
graphify-out/.graphify_labels.json |
No final comment. |
forgevm.yaml |
critical (1 vote, line 20): Resolve the mismatch between unconditional auth and empty default tokens. |
docker-compose.yml |
No final comment. |
config.py |
No final comment. |
.vscode/settings.json |
No final comment. |
.gitattributes |
No final comment. |
Review details
Suppressed comments (8)
main/app/prometheus/compact.py:116
- Compiling this regex on every
extractTickerscall adds avoidable work to the compaction hot path. Hoist the constant pattern to module scope, alongsideDECISION_KEYWORDS, and reuse the compiled object.
return list(dict.fromkeys(re.compile(r"\b([A-Z]{4}[0-9])\b").findall(text)))
main/app/prometheus/mcp.py:70
- When one server fails during initial connection,
initialize()still setsself.clientsto the partial mapping, so this path skips initialization on later calls andhealthCheck()only visits surviving clients. The failed server is never retried after a transient startup outage; health-check every configured server or keep the pool retryable until all required connections are restored.
async def getClients(self):
if self.clients is None:
await self.initialize()
if time.time() - self.lastHealthCheck > 60:
asyncio.create_task(self.healthCheck())
return self.clients, [c.session for c in self.clients.values()]
main/app/prometheus/memory.py:242
- This limit is applied before either full-text or embedding matching. Once a user has more than 500 memories, an exact match with a lower persistent score is never considered and can be absent from the results; build the candidate set from the query-specific indexes (then cap the union) or otherwise document/enforce the recall trade-off.
.limit(MEMORY_SEARCH_PREFILTER_CAP)
main/app/stocks_api/cache.py:177
- On mixed
TIMEdtypes, this fallback keeps the original load order whilebuildTickerIndexand query deduplication still assume the first row is the newest. That can return stale rows silently; normalize the time column before sorting or reject/rebuild the cache instead of accepting an unsorted frame.
except TypeError:
logger.warning("sortCacheFrame: mixed TIME dtypes, keeping load order")
return df.reset_index(drop=True)
tests/test_memory_search.py:145
- The second class declaration overwrites the first
TestFullTextSearchFallbackin the module namespace before pytest collects tests, so the matching and non-matching fallback tests at lines 32–43 are no longer collected. Rename this class, for example toTestMySQLMatchCompilation.
tests/test_relevance_score.py:15 - Using
lastAccessedAt or NOWconverts an explicitNoneintoNOW, sotest_never_accessed_uses_stabilitynever exercisesgetRelevanceScore'screatedAtfallback. PreserveNonewhen supplied and provide acreatedAtvalue on the fake memory.
tests/test_thread_safety.py:124 - This method no longer matches the repository's
python_functions = "test_*"collection pattern (pyproject.toml:5), so pytest silently stops running the concurrency test. Restore the underscore aftertest.
tests/test_thread_safety.py:124 - Removing the separator in
test_concurrentemits_are_thread_safeis inconsistent with the surrounding test names and obscures that this covers concurrent emits. Restore the descriptivetest_concurrent_emits_are_thread_safename.
- Files reviewed: 71/73 changed files
- Comments generated: 12
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| auth: | ||
| enabled: false | ||
| enabled: true |
|
|
||
| def getMatrix(userId: Any, loader: Callable[[], tuple[list[int], np.ndarray]]) -> tuple[list[int], np.ndarray]: | ||
| cacheKey = matrixKey(userId) | ||
| cached = asyncio.run(matrixCache.get(cacheKey, default=MATRIX_MISS)) |
|
|
||
|
|
||
| def invalidateUser(userId: int) -> None: | ||
| asyncio.run(matrixCache.delete_tags(f"matrix-user:{userId}")) |
Comment on lines
225
to
226
| if search: | ||
| df = self.filterBySearchTerms(df, search) |
Comment on lines
225
to
226
| if search: | ||
| df = self.filterBySearchTerms(df, search) |
| fieldData = {"historical": historicalFields, "fundamental": fundamentalCols} | ||
| except Exception as e: | ||
| logger.warning("Failed to load field data from STOCKS_API /fields: %s", e) | ||
| fieldData = {"historical": [], "fundamental": []} |
| assert event is not None | ||
|
|
||
| def test_emit_adds_to_queue(self): | ||
| def testemit_adds_to_queue(self): |
| - [ ] drop {service_name}_PORT and {serivce_name}_HOST and replace it with nginx managed services | ||
| - [ ] p99 latency and context switches monitor at /status | ||
| - [ ] feather cache for synced cache and with redis for a flock replacement | ||
| - [ ] prevent scraping by using a cookie token linked session, if theres more than one tab active with a cookie token, one page will be blocked, forcing hte user to use a single tab, similar to how whatsapp web works |
| fallbackEscaped = [re.escape(f) for f in FALLBACK_FIELDS if len(f) > 1] | ||
| fallbackEscaped.sort(key=len, reverse=True) | ||
| fallbackPattern = r"\b(" + "|".join(fallbackEscaped) + r")\b" | ||
| regex = re.compile(fallbackPattern) |
| for tr in toolResults: | ||
| content = str(tr.get("content", "")) | ||
| for match in SNAPSHOT_VALUE_RE.finditer(content): | ||
| for match in re.compile(r"([\w\s/.,]+?):\s*([\-]?[\d.,]+)\s*(x|%|pts|R\$)?").finditer(content): |
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
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.
No description provided.