feat(mcp): embedding-based semantic retrieval and passive context - #73
Conversation
Replace keyword-only docs and troubleshooting search with MiniLM embeddings and hybrid scoring, add get_context_for_pipeline for AI prompt injection, and pre-cache the model in Docker. Existing tool signatures stay unchanged.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @tonythethompson, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe MCP server now supports CPU MiniLM embeddings for semantic documentation search, pipeline context retrieval, and hybrid troubleshooting. It adds model caching, concurrent cache protection, keyword fallbacks, tool registration, tests, and two additional UI CUDA version values. ChangesSemantic knowledge tools
Application compatibility fixes
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 7 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (7 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
PR Summary by QodoAdd MiniLM semantic retrieval + pipeline passive context tool
AI Description
Diagram
High-Level Assessment
Files changed (13)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
170 rules✅ Skills:
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR upgrades the Olive MCP server’s retrieval capabilities by introducing MiniLM-based embeddings for semantic search (with keyword fallback), adding hybrid semantic+keyword troubleshooting scoring, and adding a passive pipeline-context tool for prompt injection. It also updates Python deps and the Docker image to support and pre-cache the embedding model.
Changes:
- Add shared embedding utilities (
tools/embeddings.py) and use them for semantic/hybrid retrieval in docs search and troubleshooting. - Add
get_context_for_pipelinepassive context tool (+ tests) and register it with the MCP server. - Update packaging (
pyproject.toml) and Docker build to include and pre-cache the MiniLM model for non-root runtime.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| olive-mcp-server/olive_mcp_server/tools/embeddings.py | New shared lazy-loaded MiniLM embedding + cosine + semantic_search utilities |
| olive-mcp-server/olive_mcp_server/tools/docs_search.py | Semantic search + keyword fallback; live-doc caching/indexing updates |
| olive-mcp-server/olive_mcp_server/tools/troubleshooting.py | Hybrid semantic+keyword troubleshooting scoring + embedding index cache |
| olive-mcp-server/olive_mcp_server/tools/passive_context.py | New pipeline-aware KB snippet retrieval tool |
| olive-mcp-server/olive_mcp_server/tools/init.py | Expose/register new passive context tool |
| olive-mcp-server/olive_mcp_server/mcp_server.py | Register new tool and expose tool import mapping |
| olive-mcp-server/pyproject.toml | Add embedding dependencies (sentence-transformers, numpy) |
| olive-mcp-server/Dockerfile | Pre-cache MiniLM in builder and ship HF cache for non-root user |
| olive-mcp-server/tests/test_embeddings.py | New unit tests for embedding utilities and lazy load behavior |
| olive-mcp-server/tests/test_docs_search_semantic.py | New tests for semantic docs search, cache invalidation, and live-doc race handling |
| olive-mcp-server/tests/test_troubleshooting_hybrid.py | New tests for hybrid semantic+keyword troubleshooting behavior |
| olive-mcp-server/tests/test_passive_context.py | New tests for get_context_for_pipeline return shape + confidence behavior |
| olive-mcp-server/tests/test_integration.py | Update tool-list expectations for new tool registration |
Suppressed comments (2)
olive-mcp-server/olive_mcp_server/tools/docs_search.py:251
- Potential cache/index race:
_get_live_indexcapturespagesand_LAST_FETCH_TIMEin separate critical sections, so a concurrent refresh can change_LAST_FETCH_TIMEafterpagesis returned. That can stamp embeddings with a fetch_time that doesn't match the snippet content, causing stale embeddings to be treated as fresh.
pages, fetch_time = _fetch_live_docs()
with _LIVE_INDEX_LOCK:
if (
olive-mcp-server/olive_mcp_server/tools/docs_search.py:349
- The block that forces at least one
live:result intoresultsbreaks strict top-k ranking: it can replace a higher-relevance local result with a lower-relevance live one. Since the sort key already prefers live docs on ties, this extra replacement should be removed (or gated by relevance).
live_results
and top_k > 0
and results
and not any(r["source"].startswith("live:") for r in results)
):
best_live = max(live_results, key=lambda x: x["relevance"])
if top_k == 1:
results = [best_live]
else:
results = results[: top_k - 1] + [best_live]
results.sort(
key=lambda x: (-x["relevance"], 0 if x["source"].startswith("live:") else 1)
)
return {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Qodo Fixer✅ Committed (3) · ☑ Fixed (3) Commits pushed directly to this PR — no separate fix PR opened. Process — 3 fixed
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d91a90a04b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@olive-mcp-server/Dockerfile`:
- Around line 36-37: Update the Dockerfile’s Hugging Face cache copy to set
ownership directly with COPY --chown=mcp:mcp, remove the subsequent recursive
chown layer, and define HF_HUB_OFFLINE=1 in the runtime image so Hugging Face
lookups never require network access.
- Around line 13-16: Update the Dockerfile’s pip install command to constrain
torch to an explicit +cpu build from the PyTorch CPU index, while retaining the
regular PyPI index for other dependencies and the existing mcp<2 constraint.
Replace the current unprioritized torch resolution in the install flow without
changing unrelated package installation behavior.
In `@olive-mcp-server/olive_mcp_server/tools/docs_search.py`:
- Around line 189-194: Update both exception handlers in the document search
flow, including the handler around the shown results logic and the matching
handler near the live-fetch fallback, to log the caught exception at debug or
warning level before invoking the existing keyword fallback. Preserve the
current fallback behavior and avoid leaving either exception silently swallowed.
- Around line 79-89: Update _kb_max_mtime to return a fingerprint containing
both the maximum searchable JSON mtime and the searchable file count, so
additions and deletions invalidate the cache even when the maximum mtime is
unchanged. Change _KB_INDEX_MTIME initialization and the related test fakes in
test_docs_search_semantic.py to use the new fingerprint type consistently,
preserving exclusion and OSError handling.
- Around line 336-349: Update the live-result insertion logic in the search
function so a live result is only selected when its relevance is competitive
with the local results it would replace. For top_k == 1, keep the existing local
result when it scores higher than best_live; for larger top_k values, do not
evict the lowest-ranked retained local result unless best_live meets or exceeds
its relevance, while preserving the existing relevance ordering and live-result
tie-breaking.
- Around line 176-194: Update _search_local to retain and reuse the
knowledge-base entries returned by get_or_build_kb_index when semantic_search
produces no results. Pass those entries to _keyword_search instead of calling
_load_kb_text; only invoke _load_kb_text when get_or_build_kb_index or semantic
search raises an exception and no indexed entries are available.
In `@olive-mcp-server/olive_mcp_server/tools/embeddings.py`:
- Line 10: Update the typing import in embeddings.py to import Sequence from
collections.abc instead of typing, while retaining Any from typing and
preserving the existing usage.
- Around line 22-32: Update _get_model to load SentenceTransformer strictly from
the local cache by passing local_files_only=True (and configure HF_HUB_OFFLINE=1
if required by the runtime), then invoke _get_model during application startup
so the model is warmed and missing-cache failures occur before tools serve
requests; retain the existing _model_lock singleton behavior.
In `@olive-mcp-server/olive_mcp_server/tools/passive_context.py`:
- Around line 96-106: Update the exception handler around get_or_build_kb_index
and semantic_search in the passive-context retrieval flow to log the caught
exception at warning level before returning the existing empty results. Preserve
the current return shape and fallback values, and use the module’s existing
logger if available.
In `@olive-mcp-server/olive_mcp_server/tools/troubleshooting.py`:
- Around line 416-455: Extract the duplicated diagnosis response construction
from the empty-message early return and the main return path into a shared
helper, passing best, matched_entry, matched_domain, applyable, pass_name, and
freq. Update both paths to call this helper, preserving the existing applyable
fallback behavior and all payload fields including frequency and relevant
quirks.
- Line 147: Remove the unnecessary global declaration for _ts_index_cache from
the surrounding troubleshooting function; retain the existing dictionary
mutation unchanged so lint passes without altering behavior.
- Around line 126-176: The troubleshooting loaders used by
_get_troubleshooting_index, specifically _cached_troubleshooting() and
_cached_studio_troubleshooting(), must detect JSON file mtime changes and reload
or invalidate their cached entries before indexing. Add an end-to-end test that
edits the troubleshooting file and verifies subsequent index loading reflects
the updated content rather than stale cached entries.
In `@olive-mcp-server/pyproject.toml`:
- Around line 14-15: Update the dependency bounds in pyproject.toml for
sentence-transformers and numpy to stop at the first untested release, rather
than allowing untested major versions; avoid a <6 upper bound unless CI covers
the 5.x API. Add CI checks that assert the resolved versions remain within the
tested ranges.
In `@olive-mcp-server/tests/test_docs_search_semantic.py`:
- Around line 211-252: Update
test_live_fetch_generation_ignores_stale_completion to call
docs_search._fetch_live_docs() with the patched fetcher, have fake_fetch
increment _LIVE_FETCH_GENERATION under _LIVE_FETCH_LOCK while the request is in
flight, and assert the stale result is not published. Extend the autouse fixture
to reset _LIVE_CACHE, _LAST_FETCH_TIME, and _LIVE_FETCH_GENERATION before and
after tests so live-cache state cannot leak.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 78bea72c-ca1c-4bd6-a90f-ebce1065c5a5
📒 Files selected for processing (13)
olive-mcp-server/Dockerfileolive-mcp-server/olive_mcp_server/mcp_server.pyolive-mcp-server/olive_mcp_server/tools/__init__.pyolive-mcp-server/olive_mcp_server/tools/docs_search.pyolive-mcp-server/olive_mcp_server/tools/embeddings.pyolive-mcp-server/olive_mcp_server/tools/passive_context.pyolive-mcp-server/olive_mcp_server/tools/troubleshooting.pyolive-mcp-server/pyproject.tomlolive-mcp-server/tests/test_docs_search_semantic.pyolive-mcp-server/tests/test_embeddings.pyolive-mcp-server/tests/test_integration.pyolive-mcp-server/tests/test_passive_context.pyolive-mcp-server/tests/test_troubleshooting_hybrid.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Trackdubllc/Trackdub(manual)tonythethompson/QuickShell(manual)tonythethompson/numan(manual)tonythethompson/dependency-chain-substrate(manual)
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: copilot-pull-request-reviewer
- GitHub Check: Greptile Review
- GitHub Check: python-tests
🧰 Additional context used
📓 Path-based instructions (4)
olive-mcp-server/pyproject.toml
📄 CodeRabbit inference engine (AGENTS.md)
Pin the mcp dependency to a version below 2 because mcp 2.x removes mcp.server.fastmcp and breaks imports and tests.
Files:
olive-mcp-server/pyproject.toml
**/*
📄 CodeRabbit inference engine (AGENTS.md)
Use the repository’s prescribed validation commands and preserve the CI order: lint, unit tests, server tests, integration tests, component tests, recipe validation, build, artifact assertion, production smoke testing, and CodeQL.
Files:
olive-mcp-server/pyproject.tomlolive-mcp-server/olive_mcp_server/mcp_server.pyolive-mcp-server/tests/test_integration.pyolive-mcp-server/tests/test_passive_context.pyolive-mcp-server/olive_mcp_server/tools/passive_context.pyolive-mcp-server/Dockerfileolive-mcp-server/tests/test_docs_search_semantic.pyolive-mcp-server/olive_mcp_server/tools/__init__.pyolive-mcp-server/olive_mcp_server/tools/embeddings.pyolive-mcp-server/tests/test_embeddings.pyolive-mcp-server/olive_mcp_server/tools/docs_search.pyolive-mcp-server/tests/test_troubleshooting_hybrid.pyolive-mcp-server/olive_mcp_server/tools/troubleshooting.py
olive-mcp-server/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Maintain compatibility with Python >=3.10 and the FastMCP server architecture when modifying the Olive MCP server.
Files:
olive-mcp-server/olive_mcp_server/mcp_server.pyolive-mcp-server/tests/test_integration.pyolive-mcp-server/tests/test_passive_context.pyolive-mcp-server/olive_mcp_server/tools/passive_context.pyolive-mcp-server/tests/test_docs_search_semantic.pyolive-mcp-server/olive_mcp_server/tools/__init__.pyolive-mcp-server/olive_mcp_server/tools/embeddings.pyolive-mcp-server/tests/test_embeddings.pyolive-mcp-server/olive_mcp_server/tools/docs_search.pyolive-mcp-server/tests/test_troubleshooting_hybrid.pyolive-mcp-server/olive_mcp_server/tools/troubleshooting.py
olive-mcp-server/tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Run and maintain pytest coverage for the Olive MCP tools using
python -m pytest tests -q.
Files:
olive-mcp-server/tests/test_integration.pyolive-mcp-server/tests/test_passive_context.pyolive-mcp-server/tests/test_docs_search_semantic.pyolive-mcp-server/tests/test_embeddings.pyolive-mcp-server/tests/test_troubleshooting_hybrid.py
🪛 ast-grep (0.45.0)
olive-mcp-server/tests/test_embeddings.py
[error] 21-26: Command coming from incoming request
Context: subprocess.run(
[sys.executable, "-c", code],
capture_output=True,
text=True,
check=False,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
🪛 GitHub Check: CodeFactor
olive-mcp-server/olive_mcp_server/tools/docs_search.py
[notice] 191-192: olive-mcp-server/olive_mcp_server/tools/docs_search.py#L191-L192
Try, Except, Pass detected. (B110)
🪛 Ruff (0.16.0)
olive-mcp-server/olive_mcp_server/tools/embeddings.py
[warning] 10-10: Import from collections.abc instead: Sequence
Import from collections.abc
(UP035)
olive-mcp-server/olive_mcp_server/tools/docs_search.py
[error] 191-192: try-except-pass detected, consider logging the exception
(S110)
[error] 226-227: try-except-pass detected, consider logging the exception
(S110)
olive-mcp-server/olive_mcp_server/tools/troubleshooting.py
[warning] 147-147: Using global for _ts_index_cache but no assignment is done
(PLW0602)
🔍 Remote MCP Context7, DeepWiki, GitHub Copilot
Review-relevant context
- PR
#73is open, one commit, and changes 13 files (+1,689/−156). At retrieval time,validatehad failed while Python tests and Docker build were still running; no review threads existed. - MCP history: PR
#8established the original 12-tool server and KB; PR#12centralized KB loading and normalization; PR#65introduced lazy tool imports and thediagnose_errordispatcher. These are the relevant compatibility baselines for the new registration. sentence-transformersandnumpyare added as unpinned lower-bounded runtime dependencies. The Docker build pre-cachesall-MiniLM-L6-v2, copies installed packages/cache into the runtime image, and setsHF_HOMEfor the non-root user.- PyTorch’s documented CPU installation uses
--index-url https://download.pytorch.org/whl/cpu; this PR uses--extra-index-url, so the resolver’s selected torch wheel should be verified. - Sentence Transformers supports
local_files_only=True, but the implementation constructsSentenceTransformer(MODEL_NAME, device="cpu")without it; a missing/incomplete cache can therefore still trigger a download. - Cache review focus: local KB invalidation compares only the maximum JSON mtime, so changes to a non-newest file may not invalidate the index. Troubleshooting loaders are also
lru_cache-backed, meaning mtime changes can rebuild embeddings from already-cached entries.
DeepWiki was attempted but could not index tonythethompson/Olive-Studio; the requested Babel-Player cross-language architecture is not involved in this Python-only PR.
🔇 Additional comments (24)
olive-mcp-server/olive_mcp_server/tools/troubleshooting.py (13)
500-516: 📐 Maintainability & Code Quality | ⚡ Quick winSee the duplication comment at lines 416-455; this block should call the same extracted helper.
4-25: LGTM!
35-54: LGTM!
102-124: LGTM!
178-214: LGTM!
240-252: LGTM!
273-302: LGTM!
312-312: LGTM!
334-338: LGTM!
347-387: LGTM!
498-498: LGTM!
525-525: LGTM!
541-541: LGTM!olive-mcp-server/tests/test_troubleshooting_hybrid.py (1)
1-380: LGTM!olive-mcp-server/olive_mcp_server/tools/embeddings.py (1)
40-99: LGTM!Also applies to: 102-122, 125-165
olive-mcp-server/tests/test_embeddings.py (1)
11-30: LGTM!Also applies to: 33-63, 66-104, 124-153
olive-mcp-server/olive_mcp_server/tools/docs_search.py (2)
32-47: LGTM!Also applies to: 92-137, 149-173, 197-230, 244-277
351-354: 🗄️ Data Integrity & IntegrationResolve the
countresponse contract before mergeConfirm whether
countdenotes total matches or returned results. The shown code sets it tolen(combined), whileresultsis truncated totop_k; aligncountwithresultsor addtotal_matches.olive-mcp-server/tests/test_docs_search_semantic.py (1)
24-44: LGTM!Also applies to: 47-104, 108-149, 152-208
olive-mcp-server/olive_mcp_server/tools/passive_context.py (1)
15-29: LGTM!Also applies to: 32-56, 59-94, 108-118
olive-mcp-server/olive_mcp_server/tools/__init__.py (1)
34-34: LGTM!Also applies to: 178-178
olive-mcp-server/olive_mcp_server/mcp_server.py (1)
45-48: LGTM!olive-mcp-server/tests/test_integration.py (1)
14-21: LGTM!olive-mcp-server/tests/test_passive_context.py (1)
12-20: LGTM!Also applies to: 23-34, 37-79, 82-103, 106-135, 138-145
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
olive-mcp-server/olive_mcp_server/tools/docs_search.py (1)
248-275: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDiscard an old live-index build after a concurrent refresh.
_get_live_indexcapturesfetch_time, builds embeddings outside_LIVE_INDEX_LOCK, and publishes the result without checking the current_LAST_FETCH_TIME. A newer fetch can complete during encoding, allowing the older build to overwrite the newer cache.After
build_kb_index, read_LAST_FETCH_TIMEunder_LIVE_FETCH_LOCK. Discard and retry the build when the timestamp changed. Add a concurrent-refresh test.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@olive-mcp-server/olive_mcp_server/tools/docs_search.py` around lines 248 - 275, Update _get_live_index so that after build_kb_index completes, it reads _LAST_FETCH_TIME while holding _LIVE_FETCH_LOCK and discards/retries the build if it differs from the captured fetch_time, before publishing under _LIVE_INDEX_LOCK. Add a test covering a concurrent refresh during embedding construction and verify the stale build is not cached or returned.
♻️ Duplicate comments (2)
olive-mcp-server/olive_mcp_server/tools/embeddings.py (2)
10-10: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winImport
Iterablefromcollections.abc.Ruff reports
UP035forfrom typing import Any, Iterable. KeepAnyintypingand moveIterabletocollections.abc. This lint failure blocks the first CI stage.Proposed fix
-from typing import Any, Iterable +from collections.abc import Iterable +from typing import Any#!/bin/bash set -euo pipefail rg -n '^from (typing|collections\.abc) import' olive-mcp-server/olive_mcp_server/tools/embeddings.pyAs per coding guidelines: “preserve the CI order: lint, unit tests, server tests, integration tests, component tests, recipe validation, build, artifact assertion, production smoke testing, and CodeQL.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@olive-mcp-server/olive_mcp_server/tools/embeddings.py` at line 10, Update the imports in embeddings.py so Any remains imported from typing while Iterable is imported from collections.abc, resolving Ruff’s UP035 lint violation.Sources: Coding guidelines, Linters/SAST tools
22-32: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftFail fast when the MiniLM cache is missing.
_get_modelconstructsSentenceTransformer(MODEL_NAME, device="cpu")withoutlocal_files_only=True. A missing or incomplete cache can trigger a Hugging Face download during the first semantic request. This makes offline and non-Docker deployments fail late.Pass
local_files_only=Trueor setHF_HUB_OFFLINE=1in the runtime. Warm_get_model()during startup so cache failures occur before tools serve requests.#!/bin/bash set -euo pipefail rg -n -A12 -B4 'def _get_model|SentenceTransformer\(' olive-mcp-server/olive_mcp_server/tools/embeddings.py rg -n 'local_files_only|HF_HUB_OFFLINE|_get_model\(' olive-mcp-server --glob '*.py'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@olive-mcp-server/olive_mcp_server/tools/embeddings.py` around lines 22 - 32, Update _get_model so SentenceTransformer loads the MiniLM model with local_files_only=True, preventing network downloads and making missing or incomplete caches fail immediately. Also invoke _get_model during application startup, using the existing startup initialization path, so cache validation completes before semantic tools accept requests.Source: MCP tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@olive-mcp-server/olive_mcp_server/tools/docs_search.py`:
- Around line 248-275: Update _get_live_index so that after build_kb_index
completes, it reads _LAST_FETCH_TIME while holding _LIVE_FETCH_LOCK and
discards/retries the build if it differs from the captured fetch_time, before
publishing under _LIVE_INDEX_LOCK. Add a test covering a concurrent refresh
during embedding construction and verify the stale build is not cached or
returned.
---
Duplicate comments:
In `@olive-mcp-server/olive_mcp_server/tools/embeddings.py`:
- Line 10: Update the imports in embeddings.py so Any remains imported from
typing while Iterable is imported from collections.abc, resolving Ruff’s UP035
lint violation.
- Around line 22-32: Update _get_model so SentenceTransformer loads the MiniLM
model with local_files_only=True, preventing network downloads and making
missing or incomplete caches fail immediately. Also invoke _get_model during
application startup, using the existing startup initialization path, so cache
validation completes before semantic tools accept requests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d3715a51-3cf5-4554-a7f1-8aed3491cd05
📒 Files selected for processing (3)
olive-mcp-server/olive_mcp_server/tools/docs_search.pyolive-mcp-server/olive_mcp_server/tools/embeddings.pyolive-mcp-server/olive_mcp_server/tools/troubleshooting.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Trackdubllc/Trackdub(manual)tonythethompson/QuickShell(manual)tonythethompson/numan(manual)tonythethompson/dependency-chain-substrate(manual)
💤 Files with no reviewable changes (1)
- olive-mcp-server/olive_mcp_server/tools/troubleshooting.py
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Greptile Review
⚠️ CI failures not shown inline (4)
GitHub Actions: CI / python-tests: feat(mcp): embedding-based semantic retrieval and passive context
Conclusion: failure
##[group]Run python -m pytest tests -q --tb=short
�[36;1mpython -m pytest tests -q --tb=short�[0m
shell: /usr/bin/bash -e {0}
env:
pythonLocation: /opt/hostedtoolcache/Python/3.12.13/x64
PKG_CONFIG_PATH: /opt/hostedtoolcache/Python/3.12.13/x64/lib/pkgconfig
Python_ROOT_DIR: /opt/hostedtoolcache/Python/3.12.13/x64
Python2_ROOT_DIR: /opt/hostedtoolcache/Python/3.12.13/x64
Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.12.13/x64
LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.12.13/x64/lib
##[endgroup]
........................................................................ [ 38%]
...............................................F........................ [ 77%]
.......................................... [100%]
=================================== FAILURES ===================================
_______________ test_search_olive_documentation_with_live_source _______________
tests/test_tools.py:173: in test_search_olive_documentation_with_live_source
assert any("live:" in r["source"] for r in result["results"])
E assert False
E + where False = any(<generator object test_search_olive_documentation_with_live_source.<locals>.<genexpr> at 0x7f06b46860c0>)
=========================== short test summary info ============================
FAILED tests/test_tools.py::test_search_olive_documentation_with_live_source - assert False
+ where False = any(<generator object test_search_olive_documentation_with_live_source.<locals>.<genexpr> at 0x7f06b46860c0>)
1 failed, 185 passed in 18.72s
##[error]Process completed with exit code 1.
GitHub Actions: CI / validate: feat(mcp): embedding-based semantic retrieval and passive context
Conclusion: failure
##[group]Run pnpm lint
�[36;1mpnpm lint�[0m
shell: /usr/bin/bash -e {0}
env:
PNPM_HOME: /home/runner/setup-pnpm/node_modules/.bin
##[endgroup]
$ tsc --noEmit && eslint
##[error]src/lib/auditAutofix.ts(220,55): error TS2769: No overload matches this call.
GitHub Actions: CI / 1_python-tests.txt: feat(mcp): embedding-based semantic retrieval and passive context
Conclusion: failure
##[group]Run python -m pytest tests -q --tb=short
�[36;1mpython -m pytest tests -q --tb=short�[0m
shell: /usr/bin/bash -e {0}
env:
pythonLocation: /opt/hostedtoolcache/Python/3.12.13/x64
PKG_CONFIG_PATH: /opt/hostedtoolcache/Python/3.12.13/x64/lib/pkgconfig
Python_ROOT_DIR: /opt/hostedtoolcache/Python/3.12.13/x64
Python2_ROOT_DIR: /opt/hostedtoolcache/Python/3.12.13/x64
Python3_ROOT_DIR: /opt/hostedtoolcache/Python/3.12.13/x64
LD_LIBRARY_PATH: /opt/hostedtoolcache/Python/3.12.13/x64/lib
##[endgroup]
........................................................................ [ 38%]
...............................................F........................ [ 77%]
.......................................... [100%]
=================================== FAILURES ===================================
_______________ test_search_olive_documentation_with_live_source _______________
tests/test_tools.py:173: in test_search_olive_documentation_with_live_source
assert any("live:" in r["source"] for r in result["results"])
E assert False
E + where False = any(<generator object test_search_olive_documentation_with_live_source.<locals>.<genexpr> at 0x7f06b46860c0>)
=========================== short test summary info ============================
FAILED tests/test_tools.py::test_search_olive_documentation_with_live_source - assert False
+ where False = any(<generator object test_search_olive_documentation_with_live_source.<locals>.<genexpr> at 0x7f06b46860c0>)
1 failed, 185 passed in 18.72s
##[error]Process completed with exit code 1.
GitHub Actions: CI / 3_validate.txt: feat(mcp): embedding-based semantic retrieval and passive context
Conclusion: failure
##[group]Run pnpm lint
�[36;1mpnpm lint�[0m
shell: /usr/bin/bash -e {0}
env:
PNPM_HOME: /home/runner/setup-pnpm/node_modules/.bin
##[endgroup]
$ tsc --noEmit && eslint
##[error]src/lib/auditAutofix.ts(220,55): error TS2769: No overload matches this call.
🧰 Additional context used
📓 Path-based instructions (2)
olive-mcp-server/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Maintain compatibility with Python >=3.10 and the FastMCP server architecture when modifying the Olive MCP server.
Files:
olive-mcp-server/olive_mcp_server/tools/docs_search.pyolive-mcp-server/olive_mcp_server/tools/embeddings.py
**/*
📄 CodeRabbit inference engine (AGENTS.md)
Use the repository’s prescribed validation commands and preserve the CI order: lint, unit tests, server tests, integration tests, component tests, recipe validation, build, artifact assertion, production smoke testing, and CodeQL.
Files:
olive-mcp-server/olive_mcp_server/tools/docs_search.pyolive-mcp-server/olive_mcp_server/tools/embeddings.py
🪛 Ruff (0.16.0)
olive-mcp-server/olive_mcp_server/tools/embeddings.py
[warning] 10-10: Import from collections.abc instead: Iterable
Import from collections.abc
(UP035)
🔍 Remote MCP Context7, GitHub Copilot
Additional review context
- PR
#73’s latest checks show Docker build, CodeQL, security, and CodeFactor passing, butvalidateandpython-testsfailing. - Sentence Transformers supports
local_files_only=True; this PR’s model construction does not use it, so a missing cache can still trigger network access at runtime. - The implementation’s
encode_textsnow materializes iterables before checking emptiness, avoiding model initialization for empty generators. - PyTorch’s documented CPU-only installation uses
--index-url https://download.pytorch.org/whl/cpu; the Dockerfile uses--extra-index-url, so the selected wheel should be verified. - A prior automated review identified live-cache snapshot consistency and an unused import; the PR comments report both as fixed in follow-up commits.
- The relevant architectural baseline is PR
#65, which established the lazy MCP tool registry, documentation search, troubleshooting dispatcher, and Olive Studio’s MCP knowledge-gathering path.
🔇 Additional comments (3)
olive-mcp-server/olive_mcp_server/tools/docs_search.py (2)
79-89: 🗄️ Data Integrity & IntegrationVerify the KB fingerprint contract before merge.
olive-mcp-server/tests/test_docs_search_semantic.pyat Line 172-208 still returns a float from the fake_kb_max_mtime()and compares_KB_INDEX_MTIMEto200.0. If production now returns a composite fingerprint, the test contract is stale. If production still uses only maximum mtime, deleting a non-newest file does not invalidate the index.Align
_kb_max_mtime,_KB_INDEX_MTIME, and the test fake. Include file-count or file-identity data in the fingerprint.#!/bin/bash set -euo pipefail rg -n -A25 -B5 'def _kb_max_mtime|_KB_INDEX_MTIME|get_or_build_kb_index' olive-mcp-server/olive_mcp_server/tools/docs_search.py rg -n -A30 -B5 'test_kb_stale_build_does_not_poison_cache' olive-mcp-server/tests/test_docs_search_semantic.pySource: Pipeline failures
197-225: LGTM!Also applies to: 229-230, 297-300
olive-mcp-server/olive_mcp_server/tools/embeddings.py (1)
40-57: LGTM!
- Add cu130/cu132 to UIState.cudaVersion type to match auditAutofix's CUDA_VERSIONS set (TS2769 build failure) - Fix test_search_olive_documentation_with_live_source: _fetch_live_docs now returns (pages, fetch_time) tuple, test mock still returned bare dict Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="olive-mcp-server/olive_mcp_server/tools/troubleshooting.py">
<violation number="1" location="olive-mcp-server/olive_mcp_server/tools/troubleshooting.py:152">
P2: KB edits made while the server is running never affect troubleshooting results: an mtime change only rebuilds embeddings for the stale `@lru_cache` loader output. Invalidate/reload the parsed troubleshooting caches when these files change before building the new index.</violation>
</file>
<file name="olive-mcp-server/tests/test_docs_search_semantic.py">
<violation number="1" location="olive-mcp-server/tests/test_docs_search_semantic.py:232">
P3: This test doesn't exercise the code it claims to cover. It installs fake_fetch and patches fetch_official_docs, but never calls _fetch_live_docs; instead it manually writes generation-protected cache entries, reimplementing the guard by hand. As written it would pass even if the real generation guard in _fetch_live_docs regressed, giving false confidence in the out-of-order-completion fix. Consider driving the real path (e.g., call _fetch_live_docs twice under controlled fetch/TTL conditions using a real fetch queue) so the actual guard logic is what's verified, and remove the now-unused fake_fetch/patch setup.</violation>
</file>
<file name="olive-mcp-server/olive_mcp_server/tools/docs_search.py">
<violation number="1" location="olive-mcp-server/olive_mcp_server/tools/docs_search.py:89">
P2: KB updates to a non-newest file can remain invisible because the cache fingerprint is only the maximum mtime. Track every searchable filename and mtime (preferably `st_mtime_ns`) so any file-set change rebuilds the index.</violation>
<violation number="2" location="olive-mcp-server/olive_mcp_server/tools/docs_search.py:222">
P2: Concurrent cache refreshes can discard the only successful live-doc response when a later request fails, making default live search return no fresh docs. Coordinate a single in-flight refresh, or retain the latest successful completion rather than gating publication solely on the latest started request.</violation>
<violation number="3" location="olive-mcp-server/olive_mcp_server/tools/docs_search.py:339">
P2: Results no longer contain the top-ranked combined hits when every live result scores below the local cutoff: this branch replaces a higher-relevance local result with `best_live`. Keep the sorted `combined[:top_k]` list unless the API explicitly defines source diversity over ranking.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
Greptile SummaryThis PR wires MiniLM-L6-v2 semantic embeddings into docs search and troubleshooting, adds a new
Confidence Score: 5/5Safe to merge; the only finding is a benign type mismatch in a test fixture that does not affect test outcomes or production behaviour. The threading model in docs_search (generation tokens, double-checked locking outside/inside the KB index lock) and the troubleshooting fingerprint cache are correctly implemented and covered by dedicated race-condition tests. The Docker cache-copy and offline-mode env vars are straightforward. The one test-fixture issue is harmless in practice because _KB_EMBEDDINGS is also reset to None, short-circuiting the comparison before the mismatched value is ever read. Files Needing Attention: olive-mcp-server/tests/test_passive_context.py — the autouse fixture resets _KB_INDEX_MTIME to a float instead of the expected tuple.
|
| Filename | Overview |
|---|---|
| olive-mcp-server/olive_mcp_server/tools/embeddings.py | New shared embedding module: lazy-load MiniLM singleton, encode_texts/encode_query, cosine_similarity_scores, build_kb_index, and semantic_search — all well-tested with mocks. |
| olive-mcp-server/olive_mcp_server/tools/docs_search.py | Upgraded to semantic search with mtime+count invalidation for KB index and generation-token guard for live-doc fetch races; keyword fallback preserved. Threading logic is carefully layered (build outside lock, double-check inside). |
| olive-mcp-server/olive_mcp_server/tools/troubleshooting.py | Hybrid 0.6-semantic/0.4-keyword scoring with fingerprint-keyed index cache (max 8 entries); empty error_message short-circuits at multiple layers; _build_diagnosis_payload consolidates the response shape. |
| olive-mcp-server/olive_mcp_server/tools/passive_context.py | New get_context_for_pipeline tool: normalizes pass descriptors, queries shared KB index, returns context_snippets + confidence clamped to [0,1]; straightforward and well-tested. |
| olive-mcp-server/Dockerfile | CPU-only torch install via index-url, MiniLM pre-cached in builder and chown-copied to runtime user, HF_HOME and HF_HUB_OFFLINE=1 set for the mcp user. Image is now ~1.5–3 GB (documented). |
| src/lib/chatActions.ts | salvageChatActionPatchFromLooseJson extended to also match conversion/quant keywords in step/action/task value strings; isNegated guard prevents skip-style false positives. cu130/cu132 removed to match UIState["cudaVersion"] type union. |
| src/server/services/ai/oliveMcpKnowledge.ts | docsSearchSufficient threshold changed from relevance >= 1 to relevance > 0, correctly reflecting the new [0,1] normalized semantic score scale. |
| olive-mcp-server/tests/test_passive_context.py | Good coverage of empty/string/dict pass descriptors and confidence bounds, but the autouse fixture resets _KB_INDEX_MTIME to a plain float -1.0 instead of the correct tuple (-1.0, -1). |
Reviews (4): Last reviewed commit: "fix: don't widen cudaVersion type to mat..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
olive-mcp-server/olive_mcp_server/tools/docs_search.py (3)
249-255: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick winCache empty live results.
When
_split_live_snippetsreturns an empty list,_LIVE_SNIPPETSis falsy. The cache-hit checks at Line 252 and Line 266 then miss even when_LIVE_EMBED_CACHE_TIME == fetch_time. Each live search rebuilds the empty index.Use an explicit
Nonesentinel or test_LIVE_SNIPPETS is not None.Also applies to: 263-269
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@olive-mcp-server/olive_mcp_server/tools/docs_search.py` around lines 249 - 255, Update the cache-hit checks in the live search flow around _LIVE_SNIPPETS to treat an empty list as a valid cached result by testing whether _LIVE_SNIPPETS is not None instead of relying on truthiness, including both checks near the shown branches. Preserve the existing _LIVE_EMBEDDINGS and _LIVE_EMBED_CACHE_TIME == fetch_time conditions.
263-273: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPrevent an older live snapshot from replacing a newer index.
This method builds outside
_LIVE_INDEX_LOCK. A call can capture an olderfetch_time, while another call publishes a newer embedding cache. The older call can then overwrite it at Line 271 through Line 273 because the publication path does not compare the current fetch generation.Compare the current generation before publication. Discard or rebuild older snapshots.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@olive-mcp-server/olive_mcp_server/tools/docs_search.py` around lines 263 - 273, Update the publication block guarded by _LIVE_INDEX_LOCK to compare the current fetch generation with _LIVE_EMBED_CACHE_TIME before assigning _LIVE_SNIPPETS, _LIVE_EMBEDDINGS, and _LIVE_EMBED_CACHE_TIME. If the existing cache is newer than the captured fetch_time, discard the stale snapshot or rebuild it; only publish snapshots that are not older than the current generation.
180-191: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winEnforce offline model loading. The Dockerfile pre-caches MiniLM and grants
mcpaccess, butSentenceTransformer(MODEL_NAME, device="cpu")leaveslocal_files_only=False. If the cache is missing or incomplete, the first local search can contact Hugging Face before keyword fallback. Setlocal_files_only=Trueor load the model during controlled startup, and test semantic search with network access disabled.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@olive-mcp-server/olive_mcp_server/tools/docs_search.py` around lines 180 - 191, Update the model-loading path used by get_or_build_kb_index and SentenceTransformer to enforce offline-only loading by setting local_files_only=True. Ensure missing or incomplete cached models fail locally and allow the existing _keyword_search fallback to run without contacting Hugging Face, and add coverage for semantic search with network access disabled.Source: MCP tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@olive-mcp-server/tests/test_tools.py`:
- Around line 169-172: Update the test around search_olive_documentation to
isolate the module-level _LIVE_SNIPPETS, _LIVE_EMBEDDINGS, and
_LIVE_EMBED_CACHE_TIME state before invoking it. Either reset those cache
variables or replace the mock’s fetch_time value with a timestamp unique to this
test, ensuring the live: assertion cannot reuse data from another test.
In `@src/types.ts`:
- Line 95: Remove cu130 and cu132 from the cudaVersion type and every related UI
state, chat action, audit autofix, and run-route input path. Ensure
RESOLVABLE_CUDA_TAGS and inferRequiredPackages remain aligned so only supported
tags through cu128 are accepted and forwarded as CUDA_VERSION; do not add CUDA
13 support.
---
Outside diff comments:
In `@olive-mcp-server/olive_mcp_server/tools/docs_search.py`:
- Around line 249-255: Update the cache-hit checks in the live search flow
around _LIVE_SNIPPETS to treat an empty list as a valid cached result by testing
whether _LIVE_SNIPPETS is not None instead of relying on truthiness, including
both checks near the shown branches. Preserve the existing _LIVE_EMBEDDINGS and
_LIVE_EMBED_CACHE_TIME == fetch_time conditions.
- Around line 263-273: Update the publication block guarded by _LIVE_INDEX_LOCK
to compare the current fetch generation with _LIVE_EMBED_CACHE_TIME before
assigning _LIVE_SNIPPETS, _LIVE_EMBEDDINGS, and _LIVE_EMBED_CACHE_TIME. If the
existing cache is newer than the captured fetch_time, discard the stale snapshot
or rebuild it; only publish snapshots that are not older than the current
generation.
- Around line 180-191: Update the model-loading path used by
get_or_build_kb_index and SentenceTransformer to enforce offline-only loading by
setting local_files_only=True. Ensure missing or incomplete cached models fail
locally and allow the existing _keyword_search fallback to run without
contacting Hugging Face, and add coverage for semantic search with network
access disabled.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ec1cf954-4237-48fc-a74e-4da716e0fda1
📒 Files selected for processing (4)
olive-mcp-server/olive_mcp_server/tools/docs_search.pyolive-mcp-server/tests/test_integration.pyolive-mcp-server/tests/test_tools.pysrc/types.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Trackdubllc/Trackdub(manual)tonythethompson/QuickShell(manual)tonythethompson/numan(manual)tonythethompson/dependency-chain-substrate(manual)
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Greptile Review
- GitHub Check: python-tests
🧰 Additional context used
📓 Path-based instructions (6)
olive-mcp-server/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Maintain compatibility with Python >=3.10 and the FastMCP server architecture when modifying the Olive MCP server.
Files:
olive-mcp-server/tests/test_tools.pyolive-mcp-server/tests/test_integration.pyolive-mcp-server/olive_mcp_server/tools/docs_search.py
olive-mcp-server/tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Run and maintain pytest coverage for the Olive MCP tools using
python -m pytest tests -q.
Files:
olive-mcp-server/tests/test_tools.pyolive-mcp-server/tests/test_integration.py
**/*
📄 CodeRabbit inference engine (AGENTS.md)
Use the repository’s prescribed validation commands and preserve the CI order: lint, unit tests, server tests, integration tests, component tests, recipe validation, build, artifact assertion, production smoke testing, and CodeQL.
Files:
olive-mcp-server/tests/test_tools.pyolive-mcp-server/tests/test_integration.pyolive-mcp-server/olive_mcp_server/tools/docs_search.pysrc/types.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
src/**/*.{ts,tsx}: Match existing naming, file layout, and TypeScript patterns insrc/.
Put shared recipe logic insrc/lib/, especiallypipelineValidation.ts,oliveRecipeBuilder.ts, andrecipePipeline.ts.
src/**/*.{ts,tsx}: Follow the React performance guidance in docs/REACT_BEST_PRACTICES.md, especially eliminating request waterfalls, avoiding barrel imports, and deferring non-critical third-party libraries.
Do not implement the listed backburner AI providers unless explicitly requested; prefer Custom or openai-compat for OpenAI-shaped hosts.
src/**/*.{ts,tsx}: Keep validation logic in shared libraries rather than duplicating it in UI cell helpers or inspectors.
Split theInputEnvironmentPanel,IHVIntegrationPanel, andExecutionWorkspacemega-panels into feature folders with colocated hooks and tests.
Keep server and UI AI provider catalogs synchronized, preferably through a shared provider ID list or synchronization test; register new providers in both catalogs.
Add test coverage forrecipe-graph/,passCatalog,oliveRecipeHub,jobHistoryStore, andvramEstimate, and strengthen component tests for the large panels.
Files:
src/types.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{ts,tsx,js,jsx}: Place imports at the top of modules; use inline imports only for a documented circular dependency.
Run linting and ensure typecheck-related CI checks pass before submitting changes.
For UI or server changes, manually smoke-test development startup, recipe loading/building, validation banners, and live execution when execution behavior is touched.
Files:
src/types.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use the project’s React 19, Vite, Express, and Tauri 2 conventions when modifying TypeScript or TSX application code.
Treat ESLint warnings as acceptable up to the configured limit; only lint errors or a non-zero lint exit indicate failure.
Files:
src/types.ts
🔍 Remote MCP Context7, DeepWiki, GitHub Copilot
Relevant review context
- PR
#73modifies only the Olive MCP server, tests, Docker configuration, andsrc/types.ts; it does not cross the C#/Python inference boundary or touch workflow/session/host lifecycle code. DeepWiki could not index the referenced repository, so no architectural facts were obtained there. - The current diff uses
SentenceTransformer(..., device="cpu")andencode(..., normalize_embeddings=True). Current Sentence Transformers documentation confirms both APIs, and also exposeslocal_files_only, which the implementation does not set; runtime model loading may therefore still attempt network access if the Docker cache is absent or incomplete. - The Dockerfile uses
--extra-index-url https://download.pytorch.org/whl/cpuwhile installing all dependencies. The diff does not demonstrate that CUDA wheels cannot still be selected from the primary index; this remains worth validating against the resulting image. - The latest retrieved checks show CodeQL, CodeFactor, security, and auto-merge passing, while validate failed;
python-testsanddocker-buildwere still in progress at retrieval time. - Prior review findings reported in PR comments—live-cache snapshot race, empty-iterator model loading, and unused
Pathimport—are marked fixed in follow-up commits. - CodeRabbit reported docstring coverage of 38.24% versus a 60% threshold, although this appeared as a warning in its pre-merge summary.
🔇 Additional comments (3)
olive-mcp-server/olive_mcp_server/tools/docs_search.py (2)
192-193: The previous swallowed-exception finding is still present.The handler still suppresses embedding or index failures without logging before keyword fallback. This repeats the previous S110/B110 finding.
92-137: LGTM!olive-mcp-server/tests/test_integration.py (1)
18-20: LGTM!
- src/types.ts: add missing cu130/cu132 CUDA version literals (unblocks tsc/lint, unrelated pre-existing drift vs auditAutofix.ts) - tests/test_tools.py: fix live-doc test to match _fetch_live_docs' new (dict, float) return signature (was silently caught by broad except, failing the "live:" source assertion in CI) - docs_search.py: kb mtime invalidation now includes file count so deletions/back-dated additions aren't missed; keyword fallback reuses already-loaded KB texts instead of re-reading from disk; log swallowed exceptions instead of silent pass; live-doc "freshness" injection no longer displaces a strictly better local/kept result (was unconditional at top_k==1) - embeddings.py: fix missing Sequence import (was silently deferred by `from __future__ import annotations`, would NameError under runtime introspection) - passive_context.py: log retrieval failures instead of returning an indistinguishable "no matches" response - troubleshooting.py: drop unnecessary `global` on a dict-mutate-only cache, bound the fingerprint cache size, extract a shared response payload builder for the empty-message and matched-entry paths (they'd drifted apart) - Dockerfile: force the CPU-only torch wheel (extra-index-url doesn't prioritize over PyPI's CUDA build), COPY --chown instead of a redundant chown -R layer, set HF_HUB_OFFLINE=1 - pyproject.toml: cap sentence-transformers/numpy below their next untested majors - oliveMcpKnowledge.ts: fix docsSearchSufficient's relevance threshold, which still assumed the old raw-hit-count scale (>=1) after this PR normalized relevance to ~[0,1] — was silently making local KB results always look "insufficient" Verified: 186/186 olive-mcp-server pytest, tsc --noEmit clean, pnpm lint 0 errors, 149/149 server vitest. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The previous version relied on the real embedding model ranking a single synthetic live snippet above ~2500 real local KB entries for "calibration data" — legitimately doesn't happen once the forced live-injection bug was fixed, so the test failed in CI (which has the real sentence-transformers model) even though it passed locally (where the model isn't installed and the code falls back to keyword search). Mock _search_local/_search_live directly instead, matching the pattern used by the other merge-logic regression tests. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- docs_search.py:_get_live_index: cache-hit/publish checks used `_LIVE_SNIPPETS` truthiness, so an empty (but successfully built) live index was never recognized as cached and got rebuilt on every call. Also, publish had no ordering guard: a slow build for an older fetch_time could complete after a newer generation already published and clobber it, silently rolling the live cache back to stale content. Both fixed: cache checks now key off `_LIVE_EMBEDDINGS is not None`, and publish is skipped when the build's fetch_time is older than what's already cached. - Added test_live_index_does_not_overwrite_newer_generation covering the race. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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 (3)
olive-mcp-server/olive_mcp_server/tools/docs_search.py (2)
226-237: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winThe generation guard can discard the only successful fetch.
The guard at Line 230 drops any completion that is not the newest generation. Consider this order: generation 1 starts, generation 2 starts, generation 2 fails, generation 1 returns valid pages. Generation 1 sees
my_generation != _LIVE_FETCH_GENERATIONand discards its result._LIVE_CACHEstays empty, and both callers get no live documents even though one fetch succeeded.Publish when the cache holds nothing, so a newer failure cannot suppress an older success.
♻️ Proposed fix
if fetched: with _LIVE_FETCH_LOCK: - # Ignore stale completions if a newer fetch was started. - if my_generation == _LIVE_FETCH_GENERATION: + # Ignore stale completions if a newer fetch was started, + # unless nothing has been published yet: a newer failure + # must not suppress an older success. + if my_generation == _LIVE_FETCH_GENERATION or not _LIVE_CACHE: _LIVE_CACHE = fetched _LAST_FETCH_TIME = time.monotonic()Add a test that lets the newer generation fail while the older one succeeds.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@olive-mcp-server/olive_mcp_server/tools/docs_search.py` around lines 226 - 237, Update the generation guard in the live-fetch completion logic around _LIVE_CACHE so a successful fetched result is published when the cache is currently empty, even if my_generation is older than _LIVE_FETCH_GENERATION; retain the generation check for replacing an existing cache. Add a concurrency test covering an older successful fetch and newer failed fetch, asserting the successful pages populate the cache.
336-345: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winReturn the payload count
count: len(combined)reports merged candidates, not results returned aftertop_ktruncation. Returncount: len(results). The only application consumer checkscount > 0and does not depend on the exact total. Addtotal_matchesonly if required, and update the exact-key test if you add it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@olive-mcp-server/olive_mcp_server/tools/docs_search.py` around lines 336 - 345, Update the result payload construction following the `combined[:top_k]` assignment in the search flow so its `count` field uses `len(results)` rather than `len(combined)`, reflecting the truncated results returned to callers. Do not add a separate `total_matches` field unless the implementation requires it.olive-mcp-server/olive_mcp_server/tools/passive_context.py (1)
99-110: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winThe failure response is still indistinguishable from a genuine empty result.
The warning log addresses observability on the server. The payload does not. A retrieval failure and a real "nothing matched" both return
confidence: 0.0andsnippet_count: 0. The frontend injects this into a system prompt, so a degraded knowledge base reads as an authoritative empty knowledge base. That is fake readiness at the API boundary.
get_context_for_pipelineis a new tool, so adding one field does not break the preserved 14 tool signatures.♻️ Proposed fix
try: kb_texts, embeddings = get_or_build_kb_index() results = semantic_search( query, kb_texts, embeddings, top_k, threshold=DEFAULT_THRESHOLD, ) + retrieval_ok = True except Exception: logger.warning("KB retrieval failed for pipeline context", exc_info=True) results = [] + retrieval_ok = Falsereturn { "context_snippets": results, "pipeline_summary": pipeline_summary, "confidence": confidence, "snippet_count": len(results), + "retrieval_ok": retrieval_ok, }Set
retrieval_oktoTrueon the early-return path at Lines 91-97 as well, and cover both cases inolive-mcp-server/tests/test_passive_context.py.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@olive-mcp-server/olive_mcp_server/tools/passive_context.py` around lines 99 - 110, Update get_context_for_pipeline so its response includes a retrieval_ok field that is False when the get_or_build_kb_index or semantic_search try block fails, while preserving the existing empty results payload; set retrieval_ok to True on the early-return path and successful retrieval path. Add or update tests in test_passive_context.py covering both the successful empty-result case and the failure case.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@olive-mcp-server/Dockerfile`:
- Around line 18-20: Update the Dockerfile’s torch dependency in the pip install
command to an explicit CPU-wheel version available for CPython 3.12 Linux
x86_64, such as torch 2.9.1, and verify compatibility with
sentence-transformers>=3.0.0,<6.
In `@olive-mcp-server/tests/test_docs_search_semantic.py`:
- Around line 262-277: Update the threaded test around _fetch_live_docs so it
verifies t1 completed successfully after release_gen1 is set: assert that t1 is
no longer alive after t1.join(timeout=5), and assert that result_gen1 was
populated before checking the cache. Keep the existing generation-order and
cache assertions unchanged.
In `@src/server/services/ai/oliveMcpKnowledge.ts`:
- Around line 140-144: Add a regression case to the docsSearchSufficient tests
in oliveMcpKnowledge.test.ts using a positive relevance below 1, such as 0.2,
and assert that the result is sufficient. Keep the existing high-relevance
coverage intact so the test specifically distinguishes the normalized-score
behavior from the previous >=1 threshold.
---
Outside diff comments:
In `@olive-mcp-server/olive_mcp_server/tools/docs_search.py`:
- Around line 226-237: Update the generation guard in the live-fetch completion
logic around _LIVE_CACHE so a successful fetched result is published when the
cache is currently empty, even if my_generation is older than
_LIVE_FETCH_GENERATION; retain the generation check for replacing an existing
cache. Add a concurrency test covering an older successful fetch and newer
failed fetch, asserting the successful pages populate the cache.
- Around line 336-345: Update the result payload construction following the
`combined[:top_k]` assignment in the search flow so its `count` field uses
`len(results)` rather than `len(combined)`, reflecting the truncated results
returned to callers. Do not add a separate `total_matches` field unless the
implementation requires it.
In `@olive-mcp-server/olive_mcp_server/tools/passive_context.py`:
- Around line 99-110: Update get_context_for_pipeline so its response includes a
retrieval_ok field that is False when the get_or_build_kb_index or
semantic_search try block fails, while preserving the existing empty results
payload; set retrieval_ok to True on the early-return path and successful
retrieval path. Add or update tests in test_passive_context.py covering both the
successful empty-result case and the failure case.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c938bbaf-6842-4356-bb2f-e54f93667d4c
📒 Files selected for processing (9)
olive-mcp-server/Dockerfileolive-mcp-server/olive_mcp_server/tools/docs_search.pyolive-mcp-server/olive_mcp_server/tools/embeddings.pyolive-mcp-server/olive_mcp_server/tools/passive_context.pyolive-mcp-server/olive_mcp_server/tools/troubleshooting.pyolive-mcp-server/pyproject.tomlolive-mcp-server/tests/test_docs_search_semantic.pyolive-mcp-server/tests/test_tools.pysrc/server/services/ai/oliveMcpKnowledge.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Trackdubllc/Trackdub(manual)tonythethompson/QuickShell(manual)tonythethompson/numan(manual)tonythethompson/dependency-chain-substrate(manual)
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
- GitHub Check: Greptile Review
- GitHub Check: python-tests
- GitHub Check: validate
- GitHub Check: docker-build
- GitHub Check: security
🧰 Additional context used
📓 Path-based instructions (7)
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
src/**/*.{ts,tsx}: Match existing naming, file layout, and TypeScript patterns insrc/.
Put shared recipe logic insrc/lib/, especiallypipelineValidation.ts,oliveRecipeBuilder.ts, andrecipePipeline.ts.
src/**/*.{ts,tsx}: Follow the React performance guidance in docs/REACT_BEST_PRACTICES.md, especially eliminating request waterfalls, avoiding barrel imports, and deferring non-critical third-party libraries.
Do not implement the listed backburner AI providers unless explicitly requested; prefer Custom or openai-compat for OpenAI-shaped hosts.
src/**/*.{ts,tsx}: Keep validation logic in shared libraries rather than duplicating it in UI cell helpers or inspectors.
Split theInputEnvironmentPanel,IHVIntegrationPanel, andExecutionWorkspacemega-panels into feature folders with colocated hooks and tests.
Keep server and UI AI provider catalogs synchronized, preferably through a shared provider ID list or synchronization test; register new providers in both catalogs.
Add test coverage forrecipe-graph/,passCatalog,oliveRecipeHub,jobHistoryStore, andvramEstimate, and strengthen component tests for the large panels.
Files:
src/server/services/ai/oliveMcpKnowledge.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{ts,tsx,js,jsx}: Place imports at the top of modules; use inline imports only for a documented circular dependency.
Run linting and ensure typecheck-related CI checks pass before submitting changes.
For UI or server changes, manually smoke-test development startup, recipe loading/building, validation banners, and live execution when execution behavior is touched.
Files:
src/server/services/ai/oliveMcpKnowledge.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use the project’s React 19, Vite, Express, and Tauri 2 conventions when modifying TypeScript or TSX application code.
Treat ESLint warnings as acceptable up to the configured limit; only lint errors or a non-zero lint exit indicate failure.
Files:
src/server/services/ai/oliveMcpKnowledge.ts
**/*
📄 CodeRabbit inference engine (AGENTS.md)
Use the repository’s prescribed validation commands and preserve the CI order: lint, unit tests, server tests, integration tests, component tests, recipe validation, build, artifact assertion, production smoke testing, and CodeQL.
Files:
src/server/services/ai/oliveMcpKnowledge.tsolive-mcp-server/pyproject.tomlolive-mcp-server/tests/test_tools.pyolive-mcp-server/Dockerfileolive-mcp-server/olive_mcp_server/tools/passive_context.pyolive-mcp-server/tests/test_docs_search_semantic.pyolive-mcp-server/olive_mcp_server/tools/embeddings.pyolive-mcp-server/olive_mcp_server/tools/docs_search.pyolive-mcp-server/olive_mcp_server/tools/troubleshooting.py
olive-mcp-server/pyproject.toml
📄 CodeRabbit inference engine (AGENTS.md)
Pin the mcp dependency to a version below 2 because mcp 2.x removes mcp.server.fastmcp and breaks imports and tests.
Files:
olive-mcp-server/pyproject.toml
olive-mcp-server/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Maintain compatibility with Python >=3.10 and the FastMCP server architecture when modifying the Olive MCP server.
Files:
olive-mcp-server/tests/test_tools.pyolive-mcp-server/olive_mcp_server/tools/passive_context.pyolive-mcp-server/tests/test_docs_search_semantic.pyolive-mcp-server/olive_mcp_server/tools/embeddings.pyolive-mcp-server/olive_mcp_server/tools/docs_search.pyolive-mcp-server/olive_mcp_server/tools/troubleshooting.py
olive-mcp-server/tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Run and maintain pytest coverage for the Olive MCP tools using
python -m pytest tests -q.
Files:
olive-mcp-server/tests/test_tools.pyolive-mcp-server/tests/test_docs_search_semantic.py
🪛 Hadolint (2.14.0)
olive-mcp-server/Dockerfile
[warning] 18-18: Pin versions in pip. Instead of pip install <package> use pip install <package>==<version> or pip install --requirement <requirements file>
(DL3013)
🔍 Remote MCP DeepWiki, GitHub Copilot
Additional review context
-
Unresolved P1/P2 concern: each chat knowledge request launches a new Python MCP subprocess, so module-level model and embedding caches are discarded. Documentation search may re-encode ~2,500 KB leaves on every request, potentially approaching the client’s 45-second timeout.
-
Unresolved correctness concern: troubleshooting’s new mtime-aware embedding cache still calls pre-existing
@lru_cacheloaders. If troubleshooting JSON changes while the server runs, the index may rebuild from stale parsed entries. -
Unresolved concurrency concern: if a newer live-doc refresh starts and fails after an older refresh succeeds, the generation guard can discard the only successful response, leaving live search empty.
-
Unresolved integration concern:
cu130andcu132were added toUIState.cudaVersion, but review tooling found runtime/package resolution support only throughcu128; the route may forward unsupported CUDA tags. -
Current PR checks show python-tests, validate, docker-build, security, and Greptile Review in progress; CodeFactor and auto-merge passed.
-
DeepWiki could not index
tonythethompson/Olive-Studio, so it provided no architectural validation.
🔇 Additional comments (12)
olive-mcp-server/olive_mcp_server/tools/troubleshooting.py (2)
138-172: Existing loader-cache invalidation issue.
file_mtimecan trigger an index rebuild whileload_troubleshooting()still returns entries from its pre-existing loader cache. A JSON edit can therefore re-embed stale entries. This is the same issue recorded in the previous review and should remain a separately scoped follow-up.
53-53: LGTM!Also applies to: 173-179, 415-442, 467-469, 513-513
olive-mcp-server/Dockerfile (1)
43-43: LGTM!Also applies to: 58-58
olive-mcp-server/pyproject.toml (1)
14-15: LGTM!olive-mcp-server/olive_mcp_server/tools/embeddings.py (1)
10-11: LGTM!olive-mcp-server/olive_mcp_server/tools/docs_search.py (3)
46-49: LGTM!Also applies to: 82-96
251-285: LGTM!Also applies to: 288-314
346-361: LGTM!olive-mcp-server/tests/test_docs_search_semantic.py (1)
13-33: LGTM!Also applies to: 150-161, 164-173, 196-232, 280-306
olive-mcp-server/tests/test_tools.py (1)
163-177: LGTM!olive-mcp-server/olive_mcp_server/tools/passive_context.py (2)
9-15: LGTM!
112-122: LGTM!
parseChatStructuredReply's loose-JSON salvage only matched
convert/quant keywords against the object *key* (e.g. "quantMethod",
"onnx"), so a payload shaped like {step: "convert_to_onnx"} or
{step: "apply_quantization"} — generic key, keyword in the value —
matched nothing and produced no patch. Widen both fallback branches
to also match keyword strings in the value. Pre-existing failure on
main (unrelated to the PR #73 branch), fixing here since it was
blocking this PR's CI.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- docs_search.py:_fetch_live_docs: publish also when the cache is currently empty, not only when this is still the latest generation. Otherwise a newer generation that fails after an older one already succeeded (but hasn't published yet) permanently discards the only good fetch, leaving the live cache empty. - test_docs_search_semantic.py: assert the stale-generation test's worker thread actually completed and returned data, so a silent exception or deadlock in that thread can't produce a false pass. - oliveMcpKnowledge.test.ts: add a regression case for a normalized sub-1 relevance score, which the previous `relevance: 2` fixture couldn't distinguish from the old >=1 threshold. Also widened BatchProcessingPanel.test.tsx's fetch assertion into a waitFor while investigating a "validate" CI failure — turned out to be a pre-existing bug unrelated to this PR (reproduces identically on origin/main), leaving as-is rather than chasing it here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
PR #73 squash-merged mid-session while more CodeRabbit-driven fixes kept landing on this branch, so main and this branch diverged despite main containing an earlier snapshot of the same work. Resolved by keeping this branch's content (a strict superset with later fixes) for all conflicted files, and restored cu130/cu132 in auditAutofix.ts's CUDA_VERSIONS set — main's squash-merge snapshot happened to capture this branch mid-flip-flop (a since-reverted CodeRabbit suggestion to remove cu130/cu132, itself reverted per explicit follow-up direction to keep them as driver-only tags), and the 3-way merge silently applied that removal since it wasn't a content conflict. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…closed (#75) * feat(mcp): add embedding-based semantic retrieval and passive context Replace keyword-only docs and troubleshooting search with MiniLM embeddings and hybrid scoring, add get_context_for_pipeline for AI prompt injection, and pre-cache the model in Docker. Existing tool signatures stay unchanged. * fix: Snapshot live docs and fetch time atomically * fix: Materialize text iterables before encoding * fix: Remove unused Path import * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix: repair CI failures on PR #73 - Add cu130/cu132 to UIState.cudaVersion type to match auditAutofix's CUDA_VERSIONS set (TS2769 build failure) - Fix test_search_olive_documentation_with_live_source: _fetch_live_docs now returns (pages, fetch_time) tuple, test mock still returned bare dict Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: address CI failures and CodeRabbit review on PR #73 - src/types.ts: add missing cu130/cu132 CUDA version literals (unblocks tsc/lint, unrelated pre-existing drift vs auditAutofix.ts) - tests/test_tools.py: fix live-doc test to match _fetch_live_docs' new (dict, float) return signature (was silently caught by broad except, failing the "live:" source assertion in CI) - docs_search.py: kb mtime invalidation now includes file count so deletions/back-dated additions aren't missed; keyword fallback reuses already-loaded KB texts instead of re-reading from disk; log swallowed exceptions instead of silent pass; live-doc "freshness" injection no longer displaces a strictly better local/kept result (was unconditional at top_k==1) - embeddings.py: fix missing Sequence import (was silently deferred by `from __future__ import annotations`, would NameError under runtime introspection) - passive_context.py: log retrieval failures instead of returning an indistinguishable "no matches" response - troubleshooting.py: drop unnecessary `global` on a dict-mutate-only cache, bound the fingerprint cache size, extract a shared response payload builder for the empty-message and matched-entry paths (they'd drifted apart) - Dockerfile: force the CPU-only torch wheel (extra-index-url doesn't prioritize over PyPI's CUDA build), COPY --chown instead of a redundant chown -R layer, set HF_HUB_OFFLINE=1 - pyproject.toml: cap sentence-transformers/numpy below their next untested majors - oliveMcpKnowledge.ts: fix docsSearchSufficient's relevance threshold, which still assumed the old raw-hit-count scale (>=1) after this PR normalized relevance to ~[0,1] — was silently making local KB results always look "insufficient" Verified: 186/186 olive-mcp-server pytest, tsc --noEmit clean, pnpm lint 0 errors, 149/149 server vitest. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: make live-doc merge test deterministic The previous version relied on the real embedding model ranking a single synthetic live snippet above ~2500 real local KB entries for "calibration data" — legitimately doesn't happen once the forced live-injection bug was fixed, so the test failed in CI (which has the real sentence-transformers model) even though it passed locally (where the model isn't installed and the code falls back to keyword search). Mock _search_local/_search_live directly instead, matching the pattern used by the other merge-logic regression tests. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: live-index stale-overwrite race and empty-cache rebuild bug - docs_search.py:_get_live_index: cache-hit/publish checks used `_LIVE_SNIPPETS` truthiness, so an empty (but successfully built) live index was never recognized as cached and got rebuilt on every call. Also, publish had no ordering guard: a slow build for an older fetch_time could complete after a newer generation already published and clobber it, silently rolling the live cache back to stale content. Both fixed: cache checks now key off `_LIVE_EMBEDDINGS is not None`, and publish is skipped when the build's fetch_time is older than what's already cached. - Added test_live_index_does_not_overwrite_newer_generation covering the race. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: salvage convert/quant actions from step-only value strings parseChatStructuredReply's loose-JSON salvage only matched convert/quant keywords against the object *key* (e.g. "quantMethod", "onnx"), so a payload shaped like {step: "convert_to_onnx"} or {step: "apply_quantization"} — generic key, keyword in the value — matched nothing and produced no patch. Widen both fallback branches to also match keyword strings in the value. Pre-existing failure on main (unrelated to the PR #73 branch), fixing here since it was blocking this PR's CI. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: address remaining CodeRabbit nitpicks on PR #73 - docs_search.py:_fetch_live_docs: publish also when the cache is currently empty, not only when this is still the latest generation. Otherwise a newer generation that fails after an older one already succeeded (but hasn't published yet) permanently discards the only good fetch, leaving the live cache empty. - test_docs_search_semantic.py: assert the stale-generation test's worker thread actually completed and returned data, so a silent exception or deadlock in that thread can't produce a false pass. - oliveMcpKnowledge.test.ts: add a regression case for a normalized sub-1 relevance score, which the previous `relevance: 2` fixture couldn't distinguish from the old >=1 threshold. Also widened BatchProcessingPanel.test.tsx's fetch assertion into a waitFor while investigating a "validate" CI failure — turned out to be a pre-existing bug unrelated to this PR (reproduces identically on origin/main), leaving as-is rather than chasing it here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: don't salvage chat actions from negated or non-action prose salvageChatActionPatchFromLooseJson's value-string fallback (added earlier this PR to catch {step: "convert_to_onnx"}-shaped payloads) was too broad: it scanned *any* string field, so {note: "do not quantize this model"} or {note: "conversion is unavailable"} would incorrectly enable those passes. Restrict value-string matching to step/action/task keys only, and add a negation guard (no/not/never/ disable/skip/without/unavailable) so negated prose can't enable a pass. Added regression tests for both cases. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: pre-existing BatchProcessingPanel test/validation bug (unrelated to PR #73) Root cause chain, found via instrumentation: 1. testUtils.tsx:createMockUIState set passes.conversion=true with no conversionFormat. buildOliveRecipe() then defaults to OpenVINOConversion, which requires torch/onnx input — but modelSource defaults to "huggingface" (hf output). Every component test using this default state was silently building an already-invalid pipeline. 2. In BatchProcessingPanel, this meant the "valid" job in the individual-validation test was itself blocked by a pass-chain mismatch, so the test's fetch assertion coincidentally passed on an already-broken path (fetch was never reached for either job, but the assertion only checked the invalid job's failure). 3. Fixing the state default advanced the job further into real handleStartQueue logic, exposing a second, independent bug: the test's EventSource mock was `vi.fn(() => obj)`, an arrow function, which cannot be used with `new` — throwing when the component opens its SSE stream for the (now correctly) running valid job. 4. Once EventSource was constructible, the sequential job loop still never advanced to the invalid job because nothing in the test ever fired the "done" SSE event the component awaits before moving to the next queued job. Fixes: add conversionFormat: "onnx" to the mock state default, make the EventSource mock a real constructor function, and have the test capture and fire the "done" listener so the loop can proceed to validate the invalid job. Verified this reproduces identically on origin/main (pre-existing, unrelated to the semantic-retrieval work in this PR) before fixing it here per request to get CI fully green. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: don't widen cudaVersion type to match a stray unsupported entry My earlier fix for the tsc failure in auditAutofix.ts (unrelated pre-existing bug on this branch) took the wrong direction: it widened UIState["cudaVersion"] to include cu130/cu132 to match auditAutofix.ts's CUDA_VERSIONS set, when the actual bug was that set itself listing tags the project doesn't support anywhere else. RESOLVABLE_CUDA_TAGS (oliveGpuRuntime.ts) and inferRequiredPackages (recipe.ts) both cap at cu128 — cu130/cu132 aren't resolvable yet (ORT + nvidia-*-cu13 PyPI pins don't exist). Revert cudaVersion to its original union and trim the two stray CUDA_VERSIONS sets (auditAutofix.ts, chatActions.ts) that had cu130/cu132 instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * feat: expose cu130/cu132 as selectable driver-identification CUDA tags nvidia-cudnn-cu13 and torch's cu130/cu132 wheel indexes are real and published now (verified against PyPI/download.pytorch.org), so the prior "do not enable" comment was stale. However, full package pinning isn't a small change: cublas-cu13 and cuda-runtime-cu13 are deprecated stubs pointing at unsuffixed packages (nvidia-cublas, nvidia-cuda-runtime) rather than following the cuXX-suffixed scheme cu118..cu128 use, so CUDA12_RUNTIME_PACKAGES can't just be extended — it needs its own verified pin set (onnxruntime-gpu 1.27+/1.28, unsuffixed nvidia-* packages) as a separate change. This adds cu130/cu132 as selectable values across UIState, the manual CUDA dropdown, chat action salvage, and audit autofix — labeled "driver only" in the UI — while leaving RESOLVABLE_CUDA_TAGS/ resolveCudaTag/inferRequiredPackages untouched, so selecting them still surfaces the existing clear "Unsupported CUDA tag" error at package- resolution time rather than silently mis-resolving CUDA-12 runtime libs against a CUDA-13 driver. Full pin support is a follow-up. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: Handle common negation variants * fix: Return early when top_k is zero * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Update olive-mcp-server/Dockerfile Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update olive-mcp-server/Dockerfile Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update olive-mcp-server/tests/test_passive_context.py Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix: address PR #75 review round 2 (Copilot/codex/cubic findings) - docs_search.py:_fetch_live_docs — the "empty cache means no gen has published" proxy only handled a fully-empty cache. If the TTL-expired cache was non-empty (the common case), an older-but-successful fetch racing behind a newer-but-failed one was still discarded, leaving the stale cache in place indefinitely. Track the actual last-published generation instead of the highest-started one, so a fetch only loses to a generation that genuinely published, not one that merely started later and failed. - test_passive_context.py — fix `@pytest.fixture` decorator mangled into literal backtick-wrapped text by an earlier automated commit; was a hard syntax error blocking the whole test collection. - chatActions.ts — the "quant"/"convert" acceptance regex still accepted any string containing the substring "quant" (e.g. "check quantization compatibility"), so non-actionable informational text could still enable the pass. Require an exact match against a small set of affirmative tokens instead. - types.ts — cudaVersion doc comment now reflects that cu130/cu132 are driver-identification only, not part of the fully-resolved tag set. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: address PR #75 round 3 review findings (real bugs + test hygiene) Real bugs: - docs_search.py:search_olive_documentation — removed the live-doc "freshness slot" block; CodeRabbit correctly traced it as unreachable dead code (the tie-break in the preceding sort already prefers live sources on equal relevance, so the guard condition can never be true after the earlier stale-overwrite fix). - chatActions.ts — quant/convert matching now tokenizes values instead of requiring either a full-string match or a bare "quant" substring: bare method names like `task: "gptq"` and combined phrases like `step: "apply awq"` were previously ignored entirely. Negation is now scoped per-target (checks for a quant/convert mention shortly *after* the negation word) instead of one flag suppressing every detector, so `"convert to onnx without quantization"` no longer wrongly drops the non-negated conversion instruction. - passive_context.py:get_context_for_pipeline — added a `status` field ("ok" | "retrieval_failed") so a KB/embedding-model failure is no longer indistinguishable from a genuinely empty result at the API boundary an AI assistant prompt is built from. - troubleshooting.py:_best_match — log a warning when semantic scoring fails instead of silently degrading to keyword-only matching. Test hygiene: - test_docs_search_semantic.py: two keyword-fallback tests depended on real on-disk KB content containing "calibration data" — patched _load_kb_text with a fixed corpus like the existing mtime test already does, so KB content changes can't break them. Tightened the stale-build assertion to check the actual invariant (cache stays unpublished) instead of a disjunction that hid it, and dropped the unused load_calls list. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: qodo-code-review[bot] <151058649+qodo-code-review[bot]@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Summary
get_context_for_pipelinefor injecting pipeline-aware KB snippets into the AI assistant prompt.Details
[0, 1].0.6 * semantic + 0.4 * keyword(patterns as OR); empty error messages do not match on pass name alone; fingerprint/mtime index invalidation.mcp_server.TOOLSandtools.__init__.Test plan
cd olive-mcp-server && python -m pytest tests -q→ 186 passeddocker build -t olive-mcp-server .(large image with CPU torch + MiniLM)search_olive_documentationwith a fuzzy quant querytroubleshoot_olive_errorwith paraphrased OOM textget_context_for_pipelinewith a quant pass listSummary by cubic
Adds MiniLM-based semantic retrieval for docs and hybrid troubleshooting, plus a passive pipeline context tool to improve assistant prompts. Tightens caching for live docs/indices, refines UI heuristics, and removes unsupported
cu130/cu132CUDA tags.New Features
all-MiniLM-L6-v2(lazy CPU load).get_context_for_pipelinereturns pipeline‑aware snippets, summary, confidence, and count; tool registered; existing tool signatures unchanged.torchvia index‑url, pre‑cacheall-MiniLM-L6-v2, setHF_HOMEandHF_HUB_OFFLINE=1for the non‑root user.Bug Fixes
fetch_timeis older; also publish when the cache is empty even if a newer generation later fails; never displace a stronger local top‑1 with a weaker live hit.Sequenceimport; log failures.docsSearchSufficient; salvage convert/quant actions fromstep/action/taskvalue strings but never from negated or non‑action prose; make live‑doc merge test deterministic; fix BatchProcessingPanel validation by settingconversionFormat: "onnx", using a constructible EventSource mock, and firing "done" to advance the sequential loop.sentence-transformersandnumpybelow next untested majors; remove unsupportedcu130/cu132fromauditAutofix.tsandchatActions.tsto align with resolvable CUDA tags (≤cu128).Written for commit f5567fa. Summary will update on new commits.