feat(mcp): add compare_results agent tool - #230
tonythethompson wants to merge 14 commits into
Conversation
…y tools - Mark v0.1.0 and v0.2 as shipped, remove "current" and "next" labels - Restructure v0.3 section around 5 new MCP agent autonomy tools (`execute_and_observe`, `plan_optimization`, `diagnose_and_fix`, `compare_results`, `get_model_info`) - Rename "Backlog / v0.3+" to clearer v0.4 and Backlog sections with specific features - Remove graduation criteria sections (Tauri desktop, MultiLoRA) for conciseness - Consolidate v0.2 shipped work (persistent MCP, component splits, security headers, agent platform phases) - Move experimental features to backlog with blocker notes (MultiLoRA, cloud sync, WebGPU) - Update CI pipeline description to include test tiers and pytest - Simplify status legend table formatting
Coordinated version bump from olive-ai 0.12.1 to 0.13.0:
• 8 new passes added to TS catalog, MCP knowledge base, and recipe builder
(MobiusBuilder, QairtPipeline, KQuant, OnnxKquantQuantization,
QuantizeEmbeddingInt8, ShareEmbeddingLmHead, SimplifiedLayerNormToRMSNorm,
OnnxDiscrepancyCheck)
• KQuant added as new quantization method ("kquant" in type union + allowlists)
• trust_remote_code default flip handled in recipe builder + pipeline advisory
• QNN ABI EP added to hardware profiles and provider conflicts
• CROSS_PASS_RULES for QairtPipeline and SimplifiedLayerNormToRMSNorm EP constraints
• Removed-pass warning rule for MobiusModelBuilder/QairtPreparation/QairtGenAIBuilder
• Migration module (src/lib/passMigration.ts) with pass name renames, removals,
and parameter migration infrastructure
• Migration integrated into pipelineStore replaceState + rehydration paths
• Venv spec bumped to v5 (olive-ai>=0.12.0,<1)
• Sync script version guard for 0.13.x
Tests: 1052 unit tests passing, 5 property-based tests (idempotence, validity,
preservation, exclusion, recipe schema), 6 integration test fixtures.
Lint: 0 errors. Recipe validation: passes.
Register execute_and_observe, plan_optimization, diagnose_and_fix, compare_results, and get_model_info in ALLOWED_MCP_TOOL_NAMES under a Phase 3 comment group. Requirements: 2.2, 4.2, 6.2, 8.2, 10.2
Add 5 new agent autonomous loop tools to _TOOL_IMPORTS in mcp_server.py and ALLOWED_MCP_TOOL_NAMES in allowedTools.ts: - execute_and_observe → agent_execute - plan_optimization → agent_planner - diagnose_and_fix → agent_diagnosis - compare_results → agent_compare - get_model_info → agent_model_info Part of v0.3-agent-mcp-tools (tasks 1.1 + 1.2).
Implement the get_model_info MCP tool that provides HuggingFace model metadata lookup with heuristic fallback for autonomous agent planning. - Validates model_id (1-256 chars) - Attempts HF API call (urllib, 3s timeout) for exact param count - Falls back to inferParamBillions regex heuristics (ported from src/lib/vramEstimate.ts) on any HF failure - Computes VRAM estimate (params_b * 2 FP16 baseline) - Classifies model_type via _normalize_model_type from strategy_advisor - Recommends int4 (>=6B) or int8 (<6B) quantization - Confidence: high (HF API), medium (explicit size token), low (family default) - Zero new pip dependencies, no module-level network I/O - Wrapped in top-level try/except for internal_error safety Implements: Requirements 9.1-9.8, 11.1, 11.3, 12.1, 12.5
Implements the diagnose_and_fix MCP tool (agent_diagnosis.py) for the Phase 3 autonomous agent loop. - Validates error_message (1-4000 chars) and recipe (must be dict) - Calls troubleshoot_olive_error internally with config context - Applies RFC 7386 JSON Merge Patch when KB entry has updated_config - Generates human-readable change descriptions - Best-effort recipe validation through Studio bridge - Maps fix_confidence: high/medium/low/none based on KB match quality - Top-level try/except for internal_error safety net - Zero new pip dependencies (stdlib only + internal imports) Requirements: 5.1-5.9, 6.3, 11.1, 11.3, 12.1-12.7
Implements the execute_and_observe MCP tool for the Phase 3 autonomous agent loop. Submits a recipe to Olive Studio via the loopback bridge, polls job status at 2-second intervals until a terminal state is reached or the effective timeout expires. Key behaviors: - Timeout clamping: min(max(timeout or 600, 10), 1800) - Terminal states: completed, failed, cancelled - Terminal-at-timeout-boundary: terminal wins (timed_out: false) - Pre-submission errors: no side_effect field - Post-submission results: side_effect: True - Logs capped at 200 entries, artifact refs as basenames only - Top-level try/except for internal_error safety Requirements: 1.1-1.12, 2.3, 2.4, 2.5, 11.1, 11.3, 12.1-12.7
Implements the compare_results MCP tool (task 6.1) for multi-job
comparison with preference-weighted scoring.
- Validates job_ids count (2-10) and format (^[A-Za-z0-9_-]{1,128}$)
- Normalizes preference (latency/size/accuracy/balanced)
- Fetches job status via studio_request loopback bridge
- Excludes non-terminal, failed, or unfetchable jobs
- Min-max normalizes metrics with lower-is-better inversion
- Applies 2x weight for preferred metric, 1x for others
- Selects highest scored job as winner
- Returns structured comparison with side_effect: False
- Top-level try/except for internal_error safety
- Zero new pip dependencies (stdlib + studio_loopback only)
Requirements: 7.1-7.8, 8.3, 11.1, 11.3, 12.1-12.7
|
Deployment failed for project olive-studio with the following error: Learn More: https://vercel.com/trackdub?upgradeToPro=build-rate-limit |
![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
📝 WalkthroughWalkthroughThe PR upgrades Olive integration from 0.12.1 to 0.13.0, adds K-Quant and QNN ABI support, migrates legacy pass state, expands recipe validation, updates knowledge bases, and adds autonomous MCP tools for execution, diagnosis, comparison, and model metadata. ChangesOlive 0.13.0 pipeline support
Knowledge-base and MCP agent updates
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 8✅ Passed checks (8 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Warning Review ran into problems🔥 ProblemsLinked repositories: Public OSS repositories can only analyze public repositories installed in this organization. Analyzed Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![codefactor-io[bot]](https://avatars.githubusercontent.com/in/25603?s=60&v=4){kind=small}
| let discardedParams = 0; | ||
|
|
||
| // Deep-clone overrides to avoid mutation. | ||
| let overrides: Record<string, Record<string, unknown>> = state.passRecipeOverrides |
There was a problem hiding this comment.
'overrides' is never reassigned. Use 'const' instead.
| let overrides: Record<string, Record<string, unknown>> = state.passRecipeOverrides | |
| const overrides: Record<string, Record<string, unknown>> = state.passRecipeOverrides |
PR Summary by QodoAdd compare_results MCP tool + Olive 0.13.0 pass catalog/migration updates
AI Description
Diagram
High-Level Assessment
Files changed (33)
|
Greptile SummaryThe PR adds autonomous MCP tools for executing, diagnosing, comparing, and inspecting optimization jobs while updating Olive 0.13-compatible catalogs and migration logic.
Confidence Score: 2/5The PR is not safe to merge because comparison can select cancelled or incompletely measured jobs and execution results can discard a successful zero exit code. Cancelled jobs still enter the scoreable set, missing metrics still reduce each job’s scoring denominator, and Files Needing Attention: olive-mcp-server/olive_mcp_server/tools/agent_compare.py; olive-mcp-server/olive_mcp_server/tools/agent_execute.py
|
| Filename | Overview |
|---|---|
| olive-mcp-server/olive_mcp_server/tools/agent_compare.py | Adds multi-job weighted comparison, but cancelled jobs and incomplete metric sets can still produce an invalid winner. |
| olive-mcp-server/olive_mcp_server/tools/agent_execute.py | Adds submission and terminal-state polling, but successful zero exit codes are still lost. |
| olive-mcp-server/olive_mcp_server/mcp_server.py | Registers the four new autonomous-loop tools, and the previously missing planner registration is no longer present. |
| olive-mcp-server/olive_mcp_server/tools/agent_diagnosis.py | Adds diagnosis-driven recipe patching and best-effort validation. |
| src/lib/passMigration.ts | Adds parameter migration infrastructure; the current migration table is empty and no present prototype-key path is reachable. |
| olive-mcp-server/olive_mcp_server/tools/agent_model_info.py | Adds Hugging Face metadata lookup with local heuristic fallbacks. |
Reviews (2): Last reviewed commit: "fix: Emit the supported KQuant parameter" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if val is None: | ||
| continue |
There was a problem hiding this comment.
When compared jobs expose different subsets of metrics, missing values are removed from each job's denominator while singleton metrics receive a full normalized score. An incompletely measured job can therefore tie or outrank a fully measured result without being comparable on the requested criteria.
Prompt To Fix With AI
This is a comment left during a code review.
Path: olive-mcp-server/olive_mcp_server/tools/agent_compare.py
Line: 119-120
Comment:
**Missing metrics inflate comparisons**
When compared jobs expose different subsets of metrics, missing values are removed from each job's denominator while singleton metrics receive a full normalized score. An incompletely measured job can therefore tie or outrank a fully measured result without being comparable on the requested criteria.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
![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: 56d03d6e4f
ℹ️ 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".
| "plan_optimization": ( | ||
| "olive_mcp_server.tools.agent_planner", | ||
| "plan_optimization", |
There was a problem hiding this comment.
Starting the MCP server calls _build_mcp(), which resolves every entry in _TOOL_IMPORTS; this entry imports olive_mcp_server.tools.agent_planner, but that module is absent from the commit and repository tree. Consequently, a normal stdio or SSE startup raises ModuleNotFoundError before FastMCP begins serving, even if plan_optimization is never called.
Useful? React with 👍 / 👎.
| Looks for latency_ms, model_size_mb, and accuracy in the latestMetrics | ||
| field (or latest_metrics fallback). | ||
| """ | ||
| raw = job_response.get("latestMetrics") or job_response.get("latest_metrics") or {} |
There was a problem hiding this comment.
For jobs returned by /api/olive/agent/status, latestMetrics is OliveJob.latestMetrics, whose repository type is GpuMetrics with only timestamp and gpus; it never contains latency_ms, model_size_mb, or accuracy. Thus completed jobs receive three None metrics and score 0.0, after which max() arbitrarily declares the first submitted ID the winner instead of comparing optimization results.
Useful? React with 👍 / 👎.
| if isinstance(updated_config, dict) and updated_config and applyable: | ||
| # Apply merge patch to produce fixed recipe | ||
| fixed_recipe = _apply_merge_patch(recipe, updated_config) |
There was a problem hiding this comment.
When a troubleshooting entry is applyable, its updated_config uses UI-oriented pass-type keys and params objects, such as passes.OnnxConversion.params, while generated Olive recipes use arbitrary pipeline keys whose entries contain type and config. Applying that object as a raw merge patch therefore leaves the existing pass unchanged and adds a malformed pass without a type; common fixes such as onnx-export-external-data return a recipe that cannot run.
Useful? React with 👍 / 👎.
| # If Studio is down or returned an error, treat as not validated | ||
| if isinstance(response.get("error"), str) and response["error"]: | ||
| return False | ||
| return True |
There was a problem hiding this comment.
Studio's validate_optimization_job returns a normal payload with valid: false and an errors array for an invalid recipe, without an error string. This function treats every such response as successful, so diagnose_and_fix reports recipe_validated: true for recipes that preflight explicitly rejected.
Useful? React with 👍 / 👎.
| # Capture exit code | ||
| exit_code = status_response.get("exitCode") or status_response.get("exit_code") | ||
| if exit_code is not None: | ||
| last_exit_code = exit_code |
There was a problem hiding this comment.
On every normally completed job, the status endpoint returns exitCode: 0; because 0 is falsy, this expression falls through to the absent snake-case field and produces None. The final execute_and_observe response consequently loses the successful exit code, preventing callers from distinguishing a clean process exit from an unavailable exit status.
Useful? React with 👍 / 👎.
| if status not in _TERMINAL_STATES: | ||
| reason = f"status_{status}" if status != "unknown" else "status_unknown" | ||
| excluded_jobs.append({"job_id": jid, "reason": reason}) | ||
| continue | ||
|
|
||
| # Job is completed (only non-failed terminal state that passes) |
There was a problem hiding this comment.
When a requested job has status cancelled, it passes this terminal-state check and is appended to scoreable, despite the tool contract saying cancelled jobs are excluded. Once real result metrics are available, a cancelled job can therefore be selected as the winner, or at minimum count toward the two-job threshold and produce a misleading comparison.
Useful? React with 👍 / 👎.
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| Looks for latency_ms, model_size_mb, and accuracy in the latestMetrics | ||
| field (or latest_metrics fallback). | ||
| """ | ||
| raw = job_response.get("latestMetrics") or job_response.get("latest_metrics") or {} |
There was a problem hiding this comment.
1. Comparison reads gpu telemetry 🐞 Bug ≡ Correctness
compare_results expects latency, model size, and accuracy in latestMetrics, but the status endpoint exposes GPU sampling telemetry there. Every completed job therefore receives a zero score and input order determines the reported winner.
Agent Prompt
## Issue description
`compare_results` reads optimization-result metrics from a field that contains only GPU telemetry, so comparisons return zero scores and an arbitrary winner.
## Issue Context
The Studio status contract types `latestMetrics` as `GpuMetrics` (`timestamp` and `gpus`). Add an actual persisted optimization-result metric contract or derive the supported metrics from completed artifacts/results, then consume that contract in the tool.
## Fix Focus Areas
- olive-mcp-server/olive_mcp_server/tools/agent_compare.py[34-60]
- src/server/routes/olive.ts[98-106]
- src/server/types.ts[124-135]
- src/lib/gpuMetrics.ts[1-14]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| if status not in _TERMINAL_STATES: | ||
| reason = f"status_{status}" if status != "unknown" else "status_unknown" | ||
| excluded_jobs.append({"job_id": jid, "reason": reason}) | ||
| continue |
There was a problem hiding this comment.
2. Cancelled jobs are scored 🐞 Bug ≡ Correctness
The terminal-state filter excludes failed jobs but lets cancelled jobs enter scoreable, contradicting the completed-only intent. A cancelled run can consequently be selected as the comparison winner despite never producing a completed result.
Agent Prompt
## Issue description
Cancelled jobs pass the comparison filter and participate in winner selection even though optimization did not complete.
## Issue Context
The status model has three terminal states, but only `completed` is a valid comparison candidate. Return an explicit exclusion reason for both failed and cancelled jobs.
## Fix Focus Areas
- olive-mcp-server/olive_mcp_server/tools/agent_compare.py[22-22]
- olive-mcp-server/olive_mcp_server/tools/agent_compare.py[221-238]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| // Pass renamed — move override entry to new key. | ||
| overrides[migration.newName] = overrides[migration.oldName]; | ||
| delete overrides[migration.oldName]; | ||
| renamedPasses.push({ oldName: migration.oldName, newName: migration.newName }); |
There was a problem hiding this comment.
6. Migration overwrites current overrides 🐞 Bug ☼ Reliability
When state contains both MobiusModelBuilder and MobiusBuilder, migration unconditionally replaces the current-name override with the legacy value. Hydration or state replacement can therefore silently discard the user's newer override configuration.
Agent Prompt
## Issue description
Pass-name migration loses configuration when both legacy and current override keys are present.
## Issue Context
Define conflict precedence explicitly, preferably preserving the current-name entry and deleting only the legacy key, and add a both-keys regression test.
## Fix Focus Areas
- src/lib/passMigration.ts[82-102]
- src/lib/__tests__/passMigrationIntegration.test.ts[13-43]
- src/lib/stores/pipelineStore.ts[53-63]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| # Select winner (highest score) | ||
| winner_entry = max(scored, key=lambda x: x["score"]) | ||
| winner = winner_entry["job_id"] |
There was a problem hiding this comment.
9. Compare_results winner tie-break is nondeterministic 🐞 Bug ≡ Correctness
When multiple scored jobs have the exact same highest score (e.g. identical metrics), `max(scored, key=lambda x: x["score"])` returns the first max-scoring entry in list order, which is deterministic given input order, but the function documents no defined tie-break rule and the reasoning text doesn't mention ties, making the winner selection appear arbitrary to callers when scores are equal (a common case when only 2 jobs are compared with identical metrics or all metrics missing, since score defaults to 0.0 for every job with total_weight <= 0).
Agent Prompt
## Issue description
When no scoreable job has any usable metric, every job's score computes to 0.0, and `compare_results` still confidently reports a 'winner' (the first job in input order) with reasoning text that doesn't disclose the tie/lack of signal.
## Issue Context
See `_normalize_and_score` and the winner-selection block in `compare_results`.
## Fix Focus Areas
- olive-mcp-server/olive_mcp_server/tools/agent_compare.py[110-141]
- olive-mcp-server/olive_mcp_server/tools/agent_compare.py[261-268]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Qodo Fixer✅ Committed (4) · ☑ Fixed (4) Commits pushed directly to this PR — no separate fix PR opened. Process — 4 fixed
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 39
🤖 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/olive_mcp_server/knowledge_base/hardware_profiles.json`:
- Around line 120-150: Unify the QNN ABI target name across both knowledge-base
entries so compatibility lookups resolve consistently: update
olive-mcp-server/olive_mcp_server/knowledge_base/hardware_profiles.json lines
120-150 and
olive-mcp-server/olive_mcp_server/knowledge_base/compatibility_matrix.json lines
1169-1191 to use the same chosen string, following the exact-name convention
used by existing Qualcomm targets.
In `@olive-mcp-server/olive_mcp_server/knowledge_base/troubleshooting.json`:
- Around line 769-777: Update the troubleshooting matcher that evaluates entries
containing _pattern_hit_count so the olive_versions constraint is enforced for
the two version-gated entries in troubleshooting.json. Ensure their guidance is
returned only for Olive versions >=0.13.0, while preserving existing
case-insensitive substring matching and behavior for entries without
olive_versions.
- Around line 781-788: Update the troubleshooting entry’s updated_config object
so trust_remote_code is nested under input_model.config.load_kwargs rather than
directly under input_model.config, while preserving applyable: true and the
existing value.
In `@olive-mcp-server/olive_mcp_server/mcp_server.py`:
- Around line 98-101: Verify that olive_mcp_server.tools.agent_planner exists
and exports plan_optimization; if not, remove its registry entry from
olive-mcp-server/olive_mcp_server/mcp_server.py (lines 98-101) or add the module
in this PR. Also remove "plan_optimization" from
src/server/services/mcp/allowedTools.ts (lines 33-38) until the server-side tool
resolves.
In `@olive-mcp-server/olive_mcp_server/tools/agent_compare.py`:
- Around line 180-221: Add a dedicated test module covering compare_results
validation and scoring behavior, including single-metric jobs, equal metrics,
all jobs excluded, exactly two scoreable jobs, and each preference weighting.
Mock studio_request to isolate the comparison logic, then run the suite from
olive-mcp-server with python -m pytest tests -q.
- Around line 278-279: The top-level handlers return raw internal-error
dictionaries that omit the documented side_effect field. In
agent_compare.py:278-279, agent_execute.py:200-201, and
agent_model_info.py:232-233, replace each raw dictionary with the shared err
helper using the existing error type and message; ensure err includes
side_effect, or update the helper once to provide it, using the documented value
for agent_execute.
- Around line 126-138: Update the equal-value handling in the normalization
logic around the invert_set check so tied metrics receive their intended score
after direction inversion: apply inversion only to non-tied values, while
assigning the tied-case score after determining whether the metric is
lower-is-better. Preserve the existing weighted_sum and total_weight
calculations.
- Around line 44-54: Update _as_float to use math.isfinite(f) for rejecting NaN
and positive or negative infinity, replacing the self-comparison and explicit
infinity checks; import math as needed while preserving the existing None return
behavior.
- Around line 222-238: Update the job-filtering logic around _TERMINAL_STATES so
only status == "completed" proceeds to _extract_metrics and scoreable;
explicitly exclude cancelled (alongside failed and other non-completed states)
with an appropriate exclusion reason, and update the nearby comment to reflect
that only completed jobs are scored.
In `@olive-mcp-server/olive_mcp_server/tools/agent_diagnosis.py`:
- Around line 56-70: Update the context construction in the config-context
helper so the hardware probe summary from hardware_probe is appended before
recipe_keys, ensuring truncation preserves hardware-aware matching information.
Keep the existing overall _MAX_CONFIG_CONTEXT_LEN truncation behavior and
formatting unchanged.
- Around line 116-130: Update _validate_fixed_recipe to inspect Studio’s
validation payload: return True only when valid is true and no validation errors
are reported, return False for an explicit invalid result, and return None when
Studio is unreachable or returns an error. Update diagnose_and_fix to preserve
this distinction when setting recipe_validated and fix_confidence.
In `@olive-mcp-server/olive_mcp_server/tools/agent_execute.py`:
- Around line 109-171: The polling loop around studio_request must keep total
blocking time within effective_timeout by deriving each request’s timeout from
the remaining budget and passing it to studio_request, while preserving
terminal-state handling. Update the poll-error response to set timed_out based
on whether the elapsed duration has reached effective_timeout instead of
hardcoding False, and document the maximum blocking duration for MCP callers
near the tool’s timeout configuration or execution entry point.
- Around line 155-158: Update the status response extraction in the agent
execution flow to select exitCode versus exit_code based on key presence rather
than truthiness, preserving a returned exit code of 0 so last_exit_code is
assigned and reported. Apply the same presence-based fallback to latestMetrics
so an empty metrics dictionary is retained.
- Around line 118-119: Remove the unused `elapsed_seconds` assignment at the
start of the `while True` loop in the agent execution flow, while retaining the
later computation that is consumed by the loop logic.
In `@olive-mcp-server/olive_mcp_server/tools/agent_model_info.py`:
- Line 20: Replace the cross-module import of private _normalize_model_type in
get_model_info with a public shared API: rename/promote the function in
strategy_advisor or move it to a shared helper module, then update both its
definition and all callers to use the public symbol without the leading
underscore.
- Around line 119-131: Update _fetch_hf_metadata to validate model_id against
the allowed Hugging Face model-id pattern, rejecting traversal, whitespace,
control characters, and other invalid values before creating the request.
Percent-encode the validated model_id as a URL path segment before interpolating
it into the fixed _HF_API_BASE endpoint, while preserving the existing
None-on-failure behavior.
- Around line 24-108: The duplicated Python and TypeScript model-size and VRAM
heuristics can drift without synchronization coverage. Add a shared
fixture-driven test covering representative model identifiers, explicit size
parsing, family defaults, and the params_b × 2.0 estimate, asserting the Python
_infer_param_billions implementation agrees with the TypeScript vramEstimate
implementation; retain both implementations unless the Studio bridge is used
instead.
In `@src/lib/__tests__/passMigrationIntegration.test.ts`:
- Around line 116-137: The “full pipeline round-trip” test must verify migration
effects in the built recipe, not only its structure. Extend the assertions after
buildOliveRecipe to confirm recipe.passes excludes QairtGenAIBuilder and that
the MobiusBuilder override is applied, while preserving the existing defined,
passes, and input_model checks.
In `@src/lib/__tests__/passMigrationPBT.test.ts`:
- Around line 135-175: Update applyMigrations to accept an optional
migration-table parameter, defaulting to PARAM_MIGRATIONS for existing callers.
In the Property 3 test, pass the synthetic migration table directly to
applyMigrations and remove the in-place PARAM_MIGRATIONS mutation and
restoration logic.
- Around line 152-166: Update PassRecipeOverride in src/types.ts to include an
index signature accepting arbitrary pass configuration keys and unknown values,
matching applyMigrations. In src/lib/__tests__/passMigrationPBT.test.ts:152-166,
rely on the expanded type without adding casts. In
src/lib/__tests__/pipelineValidation.test.ts:804-838, remove the as unknown as
UIState["passRecipeOverrides"] casts at lines 807, 817, and 825; no other
changes are needed there.
In `@src/lib/__tests__/pipelineValidation.test.ts`:
- Around line 749-788: Add tests in the “0.13.0 validation rules” suite for the
mobius-builder-incompatible-qnn rule, covering both its incompatibility case and
an allowed non-conflicting provider case. Use the existing baseState,
basePasses, getPipelineValidation, and issue-ID assertion patterns, and verify
the issue ID is present only when the rule’s inverted provider condition is met.
In `@src/lib/defaultPasses.ts`:
- Line 29: Change the default trustRemoteCode value in the default pipeline
state to false so remote-code execution is opt-in. Ensure it is enabled only
through an explicit user action accompanied by a visible trust warning; if
local-trust policy requires retaining the default, document that policy and
update the review snapshot.
In `@src/lib/oliveRecipeBuilder.ts`:
- Around line 611-619: Update buildOnnxDiscrepancyCheck so it no longer assigns
_state.userScript to config.test_data_dir, since userScript is a Python file
path rather than a directory. Remove this mapping unless a dedicated test-data
directory field already exists in the available state/config symbols, then use
that field instead.
- Around line 698-702: Update the Hugging Face model construction flow in
oliveRecipeBuilder to place trust_remote_code inside HfModel
inputModel.load_kwargs rather than inputConfig. Merge it with the load kwargs
returned by buildHfLoadKwargs() for every Hugging Face build, preserving any
existing memory-offload kwargs instead of overwriting them.
- Around line 151-163: Update both pass-order lists in the surrounding
recipe-building logic so mobius_builder runs in the source-model stage before
ONNX conversion, transformation optimization, and quantization. Make
mobius_builder mutually exclusive with conversion, preserving the existing
ordering of unrelated passes.
In `@src/lib/passCatalog.test.ts`:
- Line 4: Remove the unused PassCatalogEntry type from the import in
passCatalog.test.ts, while preserving the PASS_CATALOG and OLIVE_VERSION
imports.
- Around line 31-34: Update the PASS_CATALOG consistency test to compare pass
identifiers from PASS_CATALOG and passesJson.passes, asserting both sets contain
the same names in both directions rather than checking only their lengths. Keep
the existing test scope and use the identifier field exposed by each entry.
In `@src/lib/passCatalog.ts`:
- Around line 154-160: Update the pass name in the catalog entry for
OnnxKquantQuantization to the registered identifier OnnxKQuantQuantization,
preserving the existing category, description, inputs, and outputs.
In `@src/lib/passMigration.ts`:
- Around line 99-101: Add the ordering constraint to the ParamMigration
documentation: state that PARAM_MIGRATIONS runs after renames and that
migration.passType must use the already-renamed key, such as MobiusModelBuilder.
Keep the migration logic unchanged.
- Around line 83-96: Update the rename branch in the migration loop over
PASS_NAME_MIGRATIONS to detect an existing overrides[migration.newName] before
moving the legacy entry. Preserve the target entry and emit the established
warning or conflict behavior instead of overwriting it, while retaining the
current move-and-delete behavior when no target entry exists.
- Around line 78-80: Update the overrides initialization in passMigration to use
const and clone state.passRecipeOverrides with structuredClone instead of the
JSON round-trip, while retaining the empty-object fallback and preserving
undefined values.
In `@src/lib/pipelineValidation.ts`:
- Around line 696-714: Move the REMOVED_PASSES constant out of getAdvisoryIssues
and define it at module scope alongside the other module constants. Keep its
entries and the existing passRecipeOverrides validation behavior unchanged.
- Around line 460-506: Remove the unnecessary `(provider as string)` casts from
the provider comparisons in the `qairt-pipeline-requires-qnn`,
`simplified-layernorm-requires-qnn`, and `mobius-builder-incompatible-qnn`
validation rules. Compare `provider` directly with `"QnnAbiExecutionProvider"`
so TypeScript can validate the provider name.
- Around line 1023-1028: Update the types used by coercePassFields and the
sanitizer so persisted passes are represented as partial state, allowing
trustRemoteCode to be absent during migration. Preserve the existing HuggingFace
fallback that sets trustRemoteCode to true, while keeping the finalized Passes
type boolean-required after sanitization.
In `@src/lib/stores/pipelineStore.ts`:
- Around line 96-97: Update the rehydration path around applyMigrations and
replaceState so the migration report’s four counters are logged after migrations
complete. Reuse the existing replaceState logging behavior or extract a shared
helper, ensuring renamed and removed pass overrides produce the same signal
during reload.
- Around line 53-63: Update the migration logging in replaceState to use an
ESLint-approved console method, such as console.warn, instead of console.info,
and include the discardedParams count in the message alongside renamedPasses and
removedPasses.
In `@src/server/services/mcp/allowedTools.ts`:
- Around line 33-38: Update the allowlist comment immediately above
execute_and_observe in the relevant tools configuration to explicitly identify
it as write-capable and subject to the appropriate trust boundary, while keeping
the other four read-only tools grouped separately. Do not change tool behavior
or address plan_optimization in this change.
In `@src/server/services/venv/spec.ts`:
- Around line 60-61: Align the Olive version policy across
src/server/services/venv/spec.ts:60-61, scripts/sync-pass-catalog.mjs:132-166,
and src/server/services/venv/spec.test.ts:77-83. Update PINNED_OLIVE_AI_INSTALL
and the catalog-generation validation to use the same supported Olive range,
replace fixed 0.13.0 catalog metadata with the detected supported version, and
adjust the related test expectations accordingly.
In `@src/types.ts`:
- Around line 168-185: Make trustRemoteCode optional in src/types.ts (lines
168-185) so persisted states may omit it while preserving the runtime undefined
guard in src/lib/pipelineValidation.ts (lines 1023-1028). Update every full
passes object literal that omits the six newly required fields—such as
baseState() in src/lib/__tests__/passParameterValidation.test.ts—to provide
mobiusBuilder, qairtPipeline, quantizeEmbeddingInt8, shareEmbeddingLmHead,
simplifiedLayerNormToRMSNorm, and onnxDiscrepancyCheck; no direct change is
required in pipelineValidation.ts beyond retaining its existing guard.
🪄 Autofix
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: 897ba68f-7546-46c5-b0f0-a5152ed3cc9c
📒 Files selected for processing (33)
olive-mcp-server/olive_mcp_server/knowledge_base/compatibility_matrix.jsonolive-mcp-server/olive_mcp_server/knowledge_base/hardware_profiles.jsonolive-mcp-server/olive_mcp_server/knowledge_base/passes.jsonolive-mcp-server/olive_mcp_server/knowledge_base/troubleshooting.jsonolive-mcp-server/olive_mcp_server/mcp_server.pyolive-mcp-server/olive_mcp_server/tools/agent_compare.pyolive-mcp-server/olive_mcp_server/tools/agent_diagnosis.pyolive-mcp-server/olive_mcp_server/tools/agent_execute.pyolive-mcp-server/olive_mcp_server/tools/agent_model_info.pyolive-mcp-server/tests/test_compatibility_matrix.pyscripts/sync-pass-catalog.mjssrc/lib/__tests__/passMigrationIntegration.test.tssrc/lib/__tests__/passMigrationPBT.test.tssrc/lib/__tests__/passParameterValidation.test.tssrc/lib/__tests__/pipelineValidation.test.tssrc/lib/auditAutofix.tssrc/lib/chatActions.tssrc/lib/defaultPasses.tssrc/lib/issueReport.tssrc/lib/oliveRecipeBuilder.tssrc/lib/passCatalog.test.tssrc/lib/passCatalog.tssrc/lib/passMigration.tssrc/lib/pipelineValidation.tssrc/lib/providerRuntimeKind.tssrc/lib/qnnReadiness.test.tssrc/lib/stores/pipelineStore.tssrc/lib/vramEstimate.tssrc/server/services/mcp/allowedTools.tssrc/server/services/venv/spec.test.tssrc/server/services/venv/spec.tssrc/server/services/venv/status.tssrc/types.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
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: olive-pass-availability
⚠️ CI failures not shown inline (5)
GitHub Actions: CI / 1_python-tests.txt: feat(mcp):_add_execute_and_observe_tool
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]
........................................................................ [ 16%]
........................................................................ [ 32%]
..........................FFFF....F..................................... [ 48%]
........................................................................ [ 65%]
........................................................................ [ 81%]
........................................................................ [ 97%]
......... [100%]
=================================== FAILURES ===================================
________________________ test_docs_shipped_index_loads _________________________
tests/test_index_store.py:35: in test_docs_shipped_index_loads
assert loaded is not None
E assert None is not None
________________ test_get_or_build_uses_shipped_without_encode _________________
tests/test_index_store.py:51: in test_get_or_build_uses_shipped_without_encode
pairs, emb = docs_search.get_or_build_kb_index()
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
olive_mcp_server/tools/docs_search.py:168: in get_or_build_kb_index
embeddings = build_kb_index([t for _, t in texts])
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
tests/test_index_store.py:48: in boom
raise AssertionError("should not encode when shipped index matches")
E AssertionError: should not encode when shipped...
GitHub Actions: CI / validate: feat(mcp):_add_execute_and_observe_tool
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/components/features/execute/recipe-graph/RecipeValidationPanel.tsx(71,13): error TS2322: Type '"QnnAbiExecutionProvider"' is not assignable to type 'never'.
GitHub Actions: CI / python-tests: feat(mcp):_add_execute_and_observe_tool
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]
........................................................................ [ 16%]
........................................................................ [ 32%]
..........................FFFF....F..................................... [ 48%]
........................................................................ [ 65%]
........................................................................ [ 81%]
........................................................................ [ 97%]
......... [100%]
=================================== FAILURES ===================================
________________________ test_docs_shipped_index_loads _________________________
tests/test_index_store.py:35: in test_docs_shipped_index_loads
assert loaded is not None
E assert None is not None
________________ test_get_or_build_uses_shipped_without_encode _________________
tests/test_index_store.py:51: in test_get_or_build_uses_shipped_without_encode
pairs, emb = docs_search.get_or_build_kb_index()
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
olive_mcp_server/tools/docs_search.py:168: in get_or_build_kb_index
embeddings = build_kb_index([t for _, t in texts])
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
tests/test_index_store.py:48: in boom
raise AssertionError("should not encode when shipped index matches")
E AssertionError: should not encode when shipped...
GitHub Actions: CI / 2_validate.txt: feat(mcp):_add_execute_and_observe_tool
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/components/features/execute/recipe-graph/RecipeValidationPanel.tsx(71,13): error TS2322: Type '"QnnAbiExecutionProvider"' is not assignable to type 'never'.
Commit Status: Vercel: Vercel
Conclusion: failure
Deployment rate limited — retry in 24 hours.
🧰 Additional context used
📓 Path-based instructions (19)
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}: Route every UI state mutation throughcommitUiStateUpdateinsrc/lib/pipelineValidation.tsso invariants are enforced; usereplaceStatefor recipe imports and preset loads.
UseusePipelineState()as the shorthand hook for reading pipeline state.
Avoidexport *barrel imports; import directly from the actual module file.Follow the React conventions in
docs/REACT_BEST_PRACTICES.md, especially eliminating waterfalls, avoiding barrel imports, and deferring non-critical third-party libraries.
src/**/*.{ts,tsx}: Keep pipeline and recipe validation logic in shared libraries rather than duplicating it in IHV cell helpers or inspectors.
Keep the UI AI provider catalog synchronized with the server registry, preferably through a shared provider ID list or a synchronization test; register new providers in both places.
Add coverage forrecipe-graph/and under-tested libraries includingpassCatalog,oliveRecipeHub,jobHistoryStore, andvramEstimate.
Confirm whetherEnterpriseInfraPanelandPerformanceMetricsare required; remove them if they are orphaned and not mounted.
Files:
src/lib/issueReport.tssrc/lib/__tests__/passMigrationIntegration.test.tssrc/server/services/venv/status.tssrc/lib/__tests__/passParameterValidation.test.tssrc/lib/vramEstimate.tssrc/lib/chatActions.tssrc/lib/auditAutofix.tssrc/server/services/venv/spec.test.tssrc/lib/__tests__/pipelineValidation.test.tssrc/lib/__tests__/passMigrationPBT.test.tssrc/server/services/mcp/allowedTools.tssrc/lib/qnnReadiness.test.tssrc/lib/providerRuntimeKind.tssrc/server/services/venv/spec.tssrc/lib/oliveRecipeBuilder.tssrc/lib/passCatalog.tssrc/lib/defaultPasses.tssrc/lib/pipelineValidation.tssrc/lib/stores/pipelineStore.tssrc/lib/passCatalog.test.tssrc/lib/passMigration.tssrc/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/lib/issueReport.tssrc/lib/__tests__/passMigrationIntegration.test.tssrc/server/services/venv/status.tssrc/lib/__tests__/passParameterValidation.test.tssrc/lib/vramEstimate.tssrc/lib/chatActions.tssrc/lib/auditAutofix.tssrc/server/services/venv/spec.test.tssrc/lib/__tests__/pipelineValidation.test.tssrc/lib/__tests__/passMigrationPBT.test.tssrc/server/services/mcp/allowedTools.tssrc/lib/qnnReadiness.test.tssrc/lib/providerRuntimeKind.tssrc/server/services/venv/spec.tssrc/lib/oliveRecipeBuilder.tssrc/lib/passCatalog.tssrc/lib/defaultPasses.tssrc/lib/pipelineValidation.tssrc/lib/stores/pipelineStore.tssrc/lib/passCatalog.test.tssrc/lib/passMigration.tssrc/types.ts
**/*.{js,jsx,ts,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
pnpmfor project package management and commands; do not usenpm install, which is blocked by a preinstall guard.
Files:
src/lib/issueReport.tssrc/lib/__tests__/passMigrationIntegration.test.tssrc/server/services/venv/status.tssrc/lib/__tests__/passParameterValidation.test.tssrc/lib/vramEstimate.tssrc/lib/chatActions.tssrc/lib/auditAutofix.tssrc/server/services/venv/spec.test.tssrc/lib/__tests__/pipelineValidation.test.tssrc/lib/__tests__/passMigrationPBT.test.tssrc/server/services/mcp/allowedTools.tssrc/lib/qnnReadiness.test.tssrc/lib/providerRuntimeKind.tssrc/server/services/venv/spec.tssrc/lib/oliveRecipeBuilder.tssrc/lib/passCatalog.tssrc/lib/defaultPasses.tsolive-mcp-server/olive_mcp_server/knowledge_base/hardware_profiles.jsonsrc/lib/pipelineValidation.tssrc/lib/stores/pipelineStore.tssrc/lib/passCatalog.test.tssrc/lib/passMigration.tsolive-mcp-server/olive_mcp_server/knowledge_base/compatibility_matrix.jsonolive-mcp-server/olive_mcp_server/knowledge_base/troubleshooting.jsonsrc/types.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Account for React 19 and Vite 8 breaking changes rather than assuming conventions from earlier major versions; consult current documentation when API shapes are uncertain.
Do not trigger live Olive execution or batch runs in CI or virtual machines; limit CI validation to CPU-only recipe building, JSON export, and validation.Resolve ESLint errors; warnings are acceptable within the configured maximum of 20, but non-zero lint failures and reported errors must not be ignored.
Files:
src/lib/issueReport.tssrc/lib/__tests__/passMigrationIntegration.test.tssrc/server/services/venv/status.tssrc/lib/__tests__/passParameterValidation.test.tssrc/lib/vramEstimate.tssrc/lib/chatActions.tssrc/lib/auditAutofix.tssrc/server/services/venv/spec.test.tssrc/lib/__tests__/pipelineValidation.test.tssrc/lib/__tests__/passMigrationPBT.test.tssrc/server/services/mcp/allowedTools.tssrc/lib/qnnReadiness.test.tssrc/lib/providerRuntimeKind.tssrc/server/services/venv/spec.tssrc/lib/oliveRecipeBuilder.tssrc/lib/passCatalog.tssrc/lib/defaultPasses.tssrc/lib/pipelineValidation.tssrc/lib/stores/pipelineStore.tssrc/lib/passCatalog.test.tssrc/lib/passMigration.tssrc/types.ts
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Do not run real Olive jobs in CI or VMs because they download models and CUDA wheels.
**/*: Do not run real Olive GPU workloads or model downloads in CI; use mocks or CPU-only flows.
When changing the threat model or fixing critical findings, update the review snapshot and document the local-trust model in user-facing documentation.
Files:
src/lib/issueReport.tsolive-mcp-server/tests/test_compatibility_matrix.pysrc/lib/__tests__/passMigrationIntegration.test.tssrc/server/services/venv/status.tssrc/lib/__tests__/passParameterValidation.test.tssrc/lib/vramEstimate.tsolive-mcp-server/olive_mcp_server/mcp_server.pysrc/lib/chatActions.tsscripts/sync-pass-catalog.mjssrc/lib/auditAutofix.tssrc/server/services/venv/spec.test.tssrc/lib/__tests__/pipelineValidation.test.tssrc/lib/__tests__/passMigrationPBT.test.tssrc/server/services/mcp/allowedTools.tssrc/lib/qnnReadiness.test.tssrc/lib/providerRuntimeKind.tsolive-mcp-server/olive_mcp_server/tools/agent_execute.pysrc/server/services/venv/spec.tssrc/lib/oliveRecipeBuilder.tsolive-mcp-server/olive_mcp_server/tools/agent_model_info.pysrc/lib/passCatalog.tssrc/lib/defaultPasses.tsolive-mcp-server/olive_mcp_server/knowledge_base/hardware_profiles.jsonsrc/lib/pipelineValidation.tssrc/lib/stores/pipelineStore.tsolive-mcp-server/olive_mcp_server/tools/agent_diagnosis.pyolive-mcp-server/olive_mcp_server/tools/agent_compare.pysrc/lib/passCatalog.test.tssrc/lib/passMigration.tsolive-mcp-server/olive_mcp_server/knowledge_base/compatibility_matrix.jsonolive-mcp-server/olive_mcp_server/knowledge_base/troubleshooting.jsonsrc/types.ts
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use pnpm 11.17 as the package manager; do not use npm install because the preinstall guard blocks it.
Files:
src/lib/issueReport.tssrc/lib/__tests__/passMigrationIntegration.test.tssrc/server/services/venv/status.tssrc/lib/__tests__/passParameterValidation.test.tssrc/lib/vramEstimate.tssrc/lib/chatActions.tssrc/lib/auditAutofix.tssrc/server/services/venv/spec.test.tssrc/lib/__tests__/pipelineValidation.test.tssrc/lib/__tests__/passMigrationPBT.test.tssrc/server/services/mcp/allowedTools.tssrc/lib/qnnReadiness.test.tssrc/lib/providerRuntimeKind.tssrc/server/services/venv/spec.tssrc/lib/oliveRecipeBuilder.tssrc/lib/passCatalog.tssrc/lib/defaultPasses.tsolive-mcp-server/olive_mcp_server/knowledge_base/hardware_profiles.jsonsrc/lib/pipelineValidation.tssrc/lib/stores/pipelineStore.tssrc/lib/passCatalog.test.tssrc/lib/passMigration.tsolive-mcp-server/olive_mcp_server/knowledge_base/compatibility_matrix.jsonolive-mcp-server/olive_mcp_server/knowledge_base/troubleshooting.jsonsrc/types.ts
src/lib/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run the relevant targeted unit tests when library code changes rather than unnecessarily running the full slow test suite locally; rely on CI for complete verification.
Files:
src/lib/issueReport.tssrc/lib/__tests__/passMigrationIntegration.test.tssrc/lib/__tests__/passParameterValidation.test.tssrc/lib/vramEstimate.tssrc/lib/chatActions.tssrc/lib/auditAutofix.tssrc/lib/__tests__/pipelineValidation.test.tssrc/lib/__tests__/passMigrationPBT.test.tssrc/lib/qnnReadiness.test.tssrc/lib/providerRuntimeKind.tssrc/lib/oliveRecipeBuilder.tssrc/lib/passCatalog.tssrc/lib/defaultPasses.tssrc/lib/pipelineValidation.tssrc/lib/stores/pipelineStore.tssrc/lib/passCatalog.test.tssrc/lib/passMigration.ts
olive-mcp-server/**/*.{py,toml,txt}
📄 CodeRabbit inference engine (CLAUDE.md)
Pin the Python
mcpdependency to<2, because version 2.x removesmcp.server.fastmcpand breaks imports.
Files:
olive-mcp-server/tests/test_compatibility_matrix.pyolive-mcp-server/olive_mcp_server/mcp_server.pyolive-mcp-server/olive_mcp_server/tools/agent_execute.pyolive-mcp-server/olive_mcp_server/tools/agent_model_info.pyolive-mcp-server/olive_mcp_server/tools/agent_diagnosis.pyolive-mcp-server/olive_mcp_server/tools/agent_compare.py
olive-mcp-server/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
olive-mcp-server/**/*.py: Use Python 3.10 or newer for the Olive MCP server and keep themcpdependency pinned below version 2.
Do not trigger real Olive optimization or batch runs in CI, tests, or VM environments; use CPU-only recipe building, JSON export, and validation flows instead.
Files:
olive-mcp-server/tests/test_compatibility_matrix.pyolive-mcp-server/olive_mcp_server/mcp_server.pyolive-mcp-server/olive_mcp_server/tools/agent_execute.pyolive-mcp-server/olive_mcp_server/tools/agent_model_info.pyolive-mcp-server/olive_mcp_server/tools/agent_diagnosis.pyolive-mcp-server/olive_mcp_server/tools/agent_compare.py
olive-mcp-server/tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Run MCP server tests with pytest using
python -m pytest tests -qfromolive-mcp-server.
Files:
olive-mcp-server/tests/test_compatibility_matrix.py
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use the appropriate Vitest configuration for each testing tier: library unit tests, server unit tests, integration tests, or component tests.
Files:
src/lib/__tests__/passMigrationIntegration.test.tssrc/lib/__tests__/passParameterValidation.test.tssrc/server/services/venv/spec.test.tssrc/lib/__tests__/pipelineValidation.test.tssrc/lib/__tests__/passMigrationPBT.test.tssrc/lib/qnnReadiness.test.tssrc/lib/passCatalog.test.ts
src/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Vitest for frontend unit and component tests, selecting the appropriate configured test tier (
vitest.config.tsorvitest.component.config.ts).
Files:
src/lib/__tests__/passMigrationIntegration.test.tssrc/lib/__tests__/passParameterValidation.test.tssrc/server/services/venv/spec.test.tssrc/lib/__tests__/pipelineValidation.test.tssrc/lib/__tests__/passMigrationPBT.test.tssrc/lib/qnnReadiness.test.tssrc/lib/passCatalog.test.ts
src/server/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use the server testing configuration and targeted server tests for changes under
src/server/; integration tests must use the existing mocks for child processes, AI providers, and fetch.
Files:
src/server/services/venv/status.tssrc/server/services/venv/spec.test.tssrc/server/services/mcp/allowedTools.tssrc/server/services/venv/spec.ts
src/server/services/venv/**/*.{ts,js}
📄 CodeRabbit inference engine (REVIEW.md)
Pin the runtime
olive-aiinstallation to a supported version or version range, and document the supported Olive versions.
Files:
src/server/services/venv/status.tssrc/server/services/venv/spec.test.tssrc/server/services/venv/spec.ts
src/server/**/*.{test,spec}.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use the server Vitest configurations and commands for server unit and integration tests; integration tests should run against a real Express server with external dependencies mocked.
Files:
src/server/services/venv/spec.test.ts
src/lib/oliveRecipeBuilder.ts
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When changing how UI state maps to Olive JSON, update
oliveRecipeBuilder.ts.Run recipe validation whenever
oliveRecipeBuilder.tschanges and preserve the recipe builder's CPU-only validation behavior.
Files:
src/lib/oliveRecipeBuilder.ts
src/lib/pipelineValidation.ts
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Extend validation rules in
pipelineValidation.tswhen pass-to-provider compatibility changes.
Files:
src/lib/pipelineValidation.ts
src/lib/stores/pipelineStore.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use
usePipelineStoreas the single Zustand store for application state.
Files:
src/lib/stores/pipelineStore.ts
src/lib/stores/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Keep pipeline UI state in the single Zustand store at
src/lib/stores/pipelineStore.ts.
Files:
src/lib/stores/pipelineStore.ts
🧠 Learnings (1)
📚 Learning: 2026-08-10T03:41:03.611Z
Learnt from: tonythethompson
Repo: tonythethompson/Olive-Studio PR: 203
File: src/components/features/input/GitHubRecipeSync.tsx:5-5
Timestamp: 2026-08-10T03:41:03.611Z
Learning: In the Olive-Studio repository, treat imports from the `@/components/ui` barrel as conforming to the established UI import convention. Do not flag these imports solely because a general guideline prefers importing from concrete modules.
Applied to files:
src/lib/issueReport.tssrc/lib/__tests__/passMigrationIntegration.test.tssrc/server/services/venv/status.tssrc/lib/__tests__/passParameterValidation.test.tssrc/lib/vramEstimate.tssrc/lib/chatActions.tssrc/lib/auditAutofix.tssrc/server/services/venv/spec.test.tssrc/lib/__tests__/pipelineValidation.test.tssrc/lib/__tests__/passMigrationPBT.test.tssrc/server/services/mcp/allowedTools.tssrc/lib/qnnReadiness.test.tssrc/lib/providerRuntimeKind.tssrc/server/services/venv/spec.tssrc/lib/oliveRecipeBuilder.tssrc/lib/passCatalog.tssrc/lib/defaultPasses.tssrc/lib/pipelineValidation.tssrc/lib/stores/pipelineStore.tssrc/lib/passCatalog.test.tssrc/lib/passMigration.tssrc/types.ts
🪛 ast-grep (0.45.0)
olive-mcp-server/olive_mcp_server/tools/agent_model_info.py
[warning] 126-126: Request-controlled URL passed to urlopen; validate against an allowlist to prevent SSRF.
Context: urllib.request.urlopen(req, timeout=_HF_TIMEOUT_SECONDS)
Note: [CWE-918] Server-Side Request Forgery (SSRF).
(urlopen-unsanitized-data)
[warning] 52-52: XPath query is request-/variable-derived; use parameterized XPath to prevent injection.
Context: _SIZE_TOKEN_RE.findall(model_id)
Note: [CWE-643] Improper Neutralization of Data within XPath Expressions ('XPath Injection').
(xpath-injection-python)
olive-mcp-server/olive_mcp_server/tools/agent_diagnosis.py
[info] 63-63: use jsonify instead of json.dumps for JSON output
Context: json.dumps(hardware_probe, separators=(",", ":"))
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 83-83: use jsonify instead of json.dumps for JSON output
Context: json.dumps(value)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 Biome (2.5.6)
src/server/services/venv/spec.test.ts
[error] 52-52: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
(lint/correctness/noSwitchDeclarations)
🪛 Checkov (3.3.9)
olive-mcp-server/olive_mcp_server/knowledge_base/compatibility_matrix.json
[low] 484-485: Base64 High Entropy String
(CKV_SECRET_6)
[low] 495-496: Base64 High Entropy String
(CKV_SECRET_6)
[low] 506-507: Base64 High Entropy String
(CKV_SECRET_6)
[low] 935-936: Base64 High Entropy String
(CKV_SECRET_6)
[low] 990-991: Base64 High Entropy String
(CKV_SECRET_6)
[low] 1148-1149: Base64 High Entropy String
(CKV_SECRET_6)
[low] 1159-1160: Base64 High Entropy String
(CKV_SECRET_6)
🪛 GitHub Actions: CI / 4_olive-pass-availability.txt
olive-mcp-server/olive_mcp_server/knowledge_base/compatibility_matrix.json
[error] 1-1: python scripts/check_olive_pass_availability.py failed: the compatibility matrix claims support for passes not present in the pinned Olive 0.12.1 registry: KQuant, MobiusBuilder, OnnxDiscrepancyCheck, OnnxKquantQuantization, QairtPipeline, QuantizeEmbeddingInt8, ShareEmbeddingLmHead, and SimplifiedLayerNormToRMSNorm. Process completed with exit code 1.
🪛 GitHub Actions: CI / olive-pass-availability
olive-mcp-server/olive_mcp_server/knowledge_base/compatibility_matrix.json
[error] 1-1: Command 'python scripts/check_olive_pass_availability.py olive_mcp_server/knowledge_base/compatibility_matrix.json' failed: the compatibility matrix claims support for passes absent from the pinned Olive 0.12.1 registry: KQuant, MobiusBuilder, OnnxDiscrepancyCheck, OnnxKquantQuantization, QairtPipeline, QuantizeEmbeddingInt8, ShareEmbeddingLmHead, and SimplifiedLayerNormToRMSNorm. Process exited with code 1.
🪛 GitHub Check: CodeFactor
olive-mcp-server/olive_mcp_server/tools/agent_execute.py
[notice] 53-201: olive-mcp-server/olive_mcp_server/tools/agent_execute.py#L53-L201
Complex Method
olive-mcp-server/olive_mcp_server/tools/agent_model_info.py
[warning] 127-127: olive-mcp-server/olive_mcp_server/tools/agent_model_info.py#L127
Audit url open for permitted schemes. Allowing use of file:/ or custom schemes is often unexpected. (B310)
[notice] 44-108: olive-mcp-server/olive_mcp_server/tools/agent_model_info.py#L44-L108
Complex Method
src/lib/stores/pipelineStore.ts
[notice] 58-58: src/lib/stores/pipelineStore.ts#L58
Unexpected console statement. Only these console methods are allowed: warn, error. (no-console)
olive-mcp-server/olive_mcp_server/tools/agent_compare.py
[notice] 63-149: olive-mcp-server/olive_mcp_server/tools/agent_compare.py#L63-L149
Complex Method
src/lib/passCatalog.test.ts
[notice] 4-4: src/lib/passCatalog.test.ts#L4
'PassCatalogEntry' is defined but never used. Allowed unused vars must match /^_/u. (@typescript-eslint/no-unused-vars)
src/lib/passMigration.ts
[notice] 78-78: src/lib/passMigration.ts#L78
'overrides' is never reassigned. Use 'const' instead. (prefer-const)
🪛 GitHub Check: validate
src/lib/__tests__/passMigrationPBT.test.ts
[failure] 165-165:
Property 'otherParam' does not exist on type 'PassRecipeOverride'.
[failure] 161-161:
Property 'modernParam' does not exist on type 'PassRecipeOverride'.
[failure] 153-153:
Object literal may only specify known properties, and 'legacyParam' does not exist in type 'PassRecipeOverride'.
🪛 React Doctor (0.9.3)
src/lib/passMigration.ts
[warning] 79-79: JSON.parse(JSON.stringify(x)) deep-clones by re-serializing: it is slow on large objects and silently drops undefined, functions, Date/Map/Set, and cyclic references. Use structuredClone(x).
Replace JSON.parse(JSON.stringify(value)) with structuredClone(value). It is faster and preserves Dates, Maps, Sets, and cyclic references.
(no-json-parse-stringify-clone)
🪛 Ruff (0.16.1)
olive-mcp-server/olive_mcp_server/tools/agent_execute.py
[warning] 200-200: Do not catch blind exception: Exception
(BLE001)
olive-mcp-server/olive_mcp_server/tools/agent_model_info.py
[warning] 232-232: Do not catch blind exception: Exception
(BLE001)
olive-mcp-server/olive_mcp_server/tools/agent_diagnosis.py
[warning] 128-130: Return the negated condition directly
Inline condition
(SIM103)
[warning] 201-201: Do not catch blind exception: Exception
(BLE001)
olive-mcp-server/olive_mcp_server/tools/agent_compare.py
[warning] 50-50: Name compared with itself, consider replacing f != f
(PLR0124)
[warning] 101-104: Combine if branches using logical or operator
Combine if branches
(SIM114)
[warning] 103-106: Combine if branches using logical or operator
Combine if branches
(SIM114)
[warning] 278-278: Do not catch blind exception: Exception
(BLE001)
🔍 Remote MCP Context7, DeepWiki, GitHub Copilot
Additional review context
-
The matching PR is
#230. It addsagent_compare.py, but the registry also referencesagent_planner.py; that module is absent from the PR’s changed-file list. A related review found that MCP startup eagerly resolves registered tools, so missing modules can prevent the server from starting. -
compare_resultstreatscancelledas scoreable because it is considered terminal and onlyfailedis explicitly excluded. This conflicts with the stated objective to exclude cancelled jobs. Its top-level exception path also returns an error withoutside_effect: False. -
The PR declares no dedicated tests for
compare_results; its description says only syntax/import/basic validation were performed and that the full test suite is a follow-up. -
Current checks show failures in validate, python-tests, docker-build, olive-pass-availability, and CodeFactor; CodeQL and security passed.
-
Olive’s current workflow documentation places Hugging Face’s
trust_remote_codeunderinput_model.load_kwargs, while the PR’s recipe test/builder usesinput_model.config. This contract should be reconciled before relying on the generated recipe. -
DeepWiki returned no repository context because
tonythethompson/Olive-Studiowas not indexed.
🔇 Additional comments (25)
src/lib/qnnReadiness.test.ts (1)
43-43: LGTM!src/types.ts (1)
41-41: LGTM!Also applies to: 136-136
src/lib/oliveRecipeBuilder.ts (2)
366-366: LGTM!Also applies to: 378-378, 593-609, 635-641
408-426: 🗄️ Data Integrity & IntegrationNo change needed:
kquantis not treated as a PyTorch-native quantization method.> Likely an incorrect or invalid review comment.src/lib/auditAutofix.ts (1)
120-120: LGTM!Also applies to: 200-200
src/lib/chatActions.ts (1)
29-29: LGTM!src/lib/pipelineValidation.ts (1)
85-89: LGTM!Also applies to: 129-129, 191-198, 227-230, 596-596, 856-856, 1115-1115
src/lib/vramEstimate.ts (1)
209-209: LGTM!Also applies to: 294-295
src/lib/__tests__/pipelineValidation.test.ts (1)
160-174: LGTM!Also applies to: 790-802, 840-869
src/lib/__tests__/passMigrationIntegration.test.ts (1)
12-114: LGTM!src/lib/__tests__/passMigrationPBT.test.ts (1)
20-53: LGTM!Also applies to: 99-129, 178-238
src/lib/stores/pipelineStore.ts (1)
6-6: LGTM!src/lib/issueReport.ts (1)
158-159: LGTM!src/lib/providerRuntimeKind.ts (1)
25-25: LGTM!Also applies to: 72-72
src/server/services/venv/status.ts (1)
27-30: LGTM!Also applies to: 311-312, 347-368, 497-500
src/lib/__tests__/passParameterValidation.test.ts (1)
49-49: LGTM!src/server/services/venv/spec.test.ts (1)
48-55: 🎯 Functional CorrectnessKeep the switch-case scope change only if the test is meant to enforce Biome.
The project lint script runs
tsc --noEmit && eslint, and the ESLint config does not enablenoSwitchDeclarationsor Biome. Biome is not present in the repository dependencies/config, so this does not block the configured lint run.> Likely an incorrect or invalid review comment.olive-mcp-server/olive_mcp_server/knowledge_base/hardware_profiles.json (1)
182-200: LGTM!Also applies to: 210-441, 451-628
olive-mcp-server/olive_mcp_server/knowledge_base/troubleshooting.json (1)
791-812: LGTM!olive-mcp-server/tests/test_compatibility_matrix.py (1)
521-521: LGTM!olive-mcp-server/olive_mcp_server/mcp_server.py (1)
93-97: LGTM!Also applies to: 102-113
olive-mcp-server/olive_mcp_server/tools/agent_diagnosis.py (1)
26-49: LGTM!Also applies to: 73-113, 150-199
olive-mcp-server/olive_mcp_server/tools/agent_model_info.py (1)
134-163: LGTM!Also applies to: 170-230
olive-mcp-server/olive_mcp_server/knowledge_base/compatibility_matrix.json (1)
27-29: 📐 Maintainability & Code QualityThe versioned Olive documentation URLs resolve successfully.
olive-mcp-server/olive_mcp_server/tools/agent_execute.py (1)
145-148: 🗄️ Data Integrity & IntegrationNo change needed.
/api/olive/agent/statusreturns the existingjob.logs, andjob.logsis only appended to bypushLog; the assignment is not replacing incremental endpoint output.> Likely an incorrect or invalid review comment.
| { | ||
| "target": "Qualcomm Snapdragon QNN ABI", | ||
| "accelerator": "npu", | ||
| "execution_providers": [ | ||
| "QnnAbiExecutionProvider" | ||
| ], | ||
| "recommended_passes": [ | ||
| "QairtPipeline", | ||
| "SimplifiedLayerNormToRMSNorm", | ||
| "OnnxConversion", | ||
| "OnnxModelOptimizer" | ||
| ], | ||
| "typical_speedup": "5-12x", | ||
| "calibration_size": 128, | ||
| "optimal_batch_size": 1, | ||
| "memory_gb": 8, | ||
| "ops_supported": [ | ||
| "Conv", | ||
| "Gemm", | ||
| "Transpose", | ||
| "Reshape", | ||
| "Softmax", | ||
| "RMSNorm" | ||
| ], | ||
| "known_issues": [ | ||
| "QNN ABI EP requires QAIRT SDK and QNN ABI-compatible runtime.", | ||
| "Operator whitelist is narrow; use SimplifiedLayerNormToRMSNorm for graph compatibility.", | ||
| "Requires symmetric per-channel quantization." | ||
| ], | ||
| "notes": "New in olive-ai 0.13.0 (PR #2434). ABI-stable QNN execution provider; use QairtPipeline for single-pass LLM quantization+compilation. Prefer over legacy QNNExecutionProvider multi-step workflow." | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
The new QNN ABI hardware target uses two different names across the knowledge base. Both files introduce the QNN ABI path in this PR, but the target strings do not match. Any lookup that joins a compatibility entry to a hardware profile by target name resolves in one file and misses in the other.
olive-mcp-server/olive_mcp_server/knowledge_base/hardware_profiles.json#L120-L150: the profile target is"Qualcomm Snapdragon QNN ABI".olive-mcp-server/olive_mcp_server/knowledge_base/compatibility_matrix.json#L1169-L1191: the hardware key is"Qualcomm QNN ABI".
Pick one string and use it in both files. Existing entries such as "Qualcomm Snapdragon NPU" already match exactly across the two files, which confirms the intended convention.
📍 Affects 2 files
olive-mcp-server/olive_mcp_server/knowledge_base/hardware_profiles.json#L120-L150(this comment)olive-mcp-server/olive_mcp_server/knowledge_base/compatibility_matrix.json#L1169-L1191
🤖 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/knowledge_base/hardware_profiles.json`
around lines 120 - 150, Unify the QNN ABI target name across both knowledge-base
entries so compatibility lookups resolve consistently: update
olive-mcp-server/olive_mcp_server/knowledge_base/hardware_profiles.json lines
120-150 and
olive-mcp-server/olive_mcp_server/knowledge_base/compatibility_matrix.json lines
1169-1191 to use the same chosen string, following the exact-name convention
used by existing Qualcomm targets.
| "updated_config": { | ||
| "input_model": { | ||
| "config": { | ||
| "trust_remote_code": true | ||
| } | ||
| } | ||
| }, | ||
| "applyable": true |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find how the repository models trust_remote_code in generated recipes.
rg -n -C5 'trust_remote_code|trustRemoteCode|load_kwargs' --hidden -g '!node_modules'Repository: tonythethompson/Olive-Studio
Length of output: 166
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== tracked matching files =="
git ls-files | rg 'troubleshooting\.json|diagnose_and_fix|knowledge_base|recipe|input_model|agent_diagnosis' || true
echo
echo "== troubleshooting entry context =="
if [ -f olive-mcp-server/olive_mcp_server/knowledge_base/troubleshooting.json ]; then
sed -n '755,805p' olive-mcp-server/olive_mcp_server/knowledge_base/troubleshooting.json | cat -n
else
echo "missing troubleshooting file"
fi
echo
echo "== targeted text search (repo) =="
rg -n -C3 -i 'trust_remote|load_kwargs|input_model\.config|input_model:|fixed_config|diagnose_and_fix|applyable' --hidden -g '!node_modules' . || trueRepository: tonythethompson/Olive-Studio
Length of output: 50385
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== agent_diagnosis relevant implementation =="
sed -n '120,195p' olive-mcp-server/olive_mcp_server/tools/agent_diagnosis.py | cat -n
echo
echo "== HuggingFace/HfModel model path declarations in recipes =="
python3 - <<'PY'
import json, pathlib
from pathlib import Path
for file in [
Path("olive-mcp-server/olive_mcp_server/knowledge_base/integration_recipes.json"),
Path("olive-mcp-server/olive_mcp_server/knowledge_base/troubleshooting.json"),
]:
data = json.loads(file.read_text())
for entry in data:
if not isinstance(entry, dict):
continue
cfg = entry.get("updated_config") or entry.get("config") or entry.get("recipe_body")
# skip unless it has relevant candidate structures
try:
if isinstance(cfg, dict) and (
"input_model" in cfg
or any("hf" in k.lower() or k.lower() in ("model_path","model_config","hf_config","load_kwargs") for item in json.loads(file.read_text()) for k, v in [(item.get("path",""))]+[]
):
pass
except:
pass
PY
echo
echo "== files containing HfModel or input_model declarations =="
rg -n -C4 'HfModel|input_model.*config|model_path|hf_config|load_kwargs' --hidden -g '!node_modules' -g '!**/node_modules/**' -g '!public/**' -g '!src/**' olive-mcp-server | head -n 220Repository: tonythethompson/Olive-Studio
Length of output: 3572
🌐 Web query:
Olive AI HfModel input_model config load_kwargs trust_remote_code
💡 Result:
In Microsoft Olive, when working with the HfModel (Hugging Face model) input type, load_kwargs is an optional dictionary configuration field used to pass additional arguments directly to the Hugging Face transformers from_pretrained method [1][2]. To enable the loading of models that require execution of arbitrary code from the model repository, you must explicitly include trust_remote_code within the load_kwargs dictionary [3]. Example usage within your Olive configuration: { "input_model": { "type": "HfModel", "model_path": "your-model-id-or-path", "load_kwargs": { "trust_remote_code": true } } } Key details: - Purpose: load_kwargs allows you to customize how the model, configuration, and tokenizer are loaded [1]. - Remote Code: For security, Olive does not enable trust_remote_code by default [4][5][6]. It must be explicitly set to true if required by the specific model architecture [3]. - CLI Usage: If using Olive via the command line interface, you can also use the --trust_remote_code flag to achieve the same result [5]. - Other Arguments: You can include other valid transformers arguments in load_kwargs, such as torch_dtype or attention_implementation [1].
Citations:
- 1: https://microsoft.github.io/Olive/how-to/configure-workflows/how-to-configure-model.html
- 2: https://microsoft.github.io/Olive/reference/options.html
- 3: https://microsoft.github.io/Olive/how-to/configure-workflows/build-workflow.html
- 4: https://microsoft.github.io/Olive/reference/python_api.html
- 5: https://microsoft.github.io/Olive/reference/cli.html
- 6: Don't trust remote code explicitly microsoft/Olive#2413
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== targeted HuggingFace input_model declarations =="
rg -n -C3 -i '["'"'"'}]input_model|input_model|HfModel|model_path|load_kwargs|trust_remote_code' --hidden -g '!node_modules' -g '!public/**' -g '!src/**' olive-mcp-server/olive_mcp_server/knowledge_base/json | head -n 260Repository: tonythethompson/Olive-Studio
Length of output: 397
🌐 Web query:
site:microsoft.github.io/Olive HfModel input_model config load_kwargs trust_remote_code
💡 Result:
In the context of Olive, a tool for model optimization, HfModel (specifically HfModelHandler) is used to configure and load Hugging Face models [1][2]. The load_kwargs parameter in the HfModel configuration is a dictionary used to pass additional arguments directly to the from_pretrained methods (such as those for the model, config, or tokenizer) when the model is loaded [1][3][4]. Regarding trust_remote_code: 1. It is a boolean parameter used to specify whether to trust remote code execution when loading a Hugging Face model [5][6]. 2. In Olive workflows, it can be passed as a keyword argument within your configuration or, in some cases, directly via command-line flags (e.g., --trust_remote_code) [5][6][7]. 3. Because load_kwargs are passed to the underlying Hugging Face from_pretrained methods, you can include trust_remote_code within the load_kwargs dictionary in your JSON configuration [1][3][4]. Example usage within an Olive configuration: { "type": "HfModel", "model_path": "model_name_or_path", "load_kwargs": { "trust_remote_code": true, "torch_dtype": "float16" } }
Citations:
- 1: https://microsoft.github.io/Olive/how-to/configure-workflows/how-to-configure-model.html
- 2: https://microsoft.github.io/Olive/0.7.1/api/models.html
- 3: https://microsoft.github.io/Olive/0.9.2/how-to/configure-workflows/how-to-configure-model.html
- 4: https://microsoft.github.io/Olive/reference/options.html
- 5: https://microsoft.github.io/Olive/reference/python_api.html
- 6: https://microsoft.github.io/Olive/reference/cli.html
- 7: https://microsoft.github.io/Olive/how-to/configure-workflows/how-to-configure-data.html
Use the load_kwargs path for trust_remote_code.
trust_remote_code belongs under input_model.config.load_kwargs for Hugging Face models. With applyable: true, diagnose_and_fix merge-patches this object directly into the recipe, so input_model.config.trust_remote_code leaves the effective config unchanged.
🔧 Candidate correction
"updated_config": {
"input_model": {
- "config": {
+ "load_kwargs": {
"trust_remote_code": true
}
}
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "updated_config": { | |
| "input_model": { | |
| "config": { | |
| "trust_remote_code": true | |
| } | |
| } | |
| }, | |
| "applyable": true | |
| "updated_config": { | |
| "input_model": { | |
| "load_kwargs": { | |
| "trust_remote_code": true | |
| } | |
| } | |
| }, | |
| "applyable": true |
🤖 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/knowledge_base/troubleshooting.json` around
lines 781 - 788, Update the troubleshooting entry’s updated_config object so
trust_remote_code is nested under input_model.config.load_kwargs rather than
directly under input_model.config, while preserving applyable: true and the
existing value.
| "plan_optimization": ( | ||
| "olive_mcp_server.tools.agent_planner", | ||
| "plan_optimization", | ||
| ), |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
plan_optimization is wired into both registries, but agent_planner.py is not in this PR. The server registry and the client allowlist both advertise a tool whose implementation module is unconfirmed. Because _build_mcp resolves every registered tool eagerly, a missing module stops the whole MCP server from starting, not just this one tool.
olive-mcp-server/olive_mcp_server/mcp_server.py#L98-L101: confirmolive_mcp_server/tools/agent_planner.pyexists on this branch and exportsplan_optimization; if it does not, remove this entry or land the module in the same PR.src/server/services/mcp/allowedTools.ts#L33-L38: remove"plan_optimization"from the allowlist until the server-side tool resolves, so the client does not advertise a tool the server cannot serve.
📍 Affects 2 files
olive-mcp-server/olive_mcp_server/mcp_server.py#L98-L101(this comment)src/server/services/mcp/allowedTools.ts#L33-L38
🤖 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/mcp_server.py` around lines 98 - 101,
Verify that olive_mcp_server.tools.agent_planner exists and exports
plan_optimization; if not, remove its registry entry from
olive-mcp-server/olive_mcp_server/mcp_server.py (lines 98-101) or add the module
in this PR. Also remove "plan_optimization" from
src/server/services/mcp/allowedTools.ts (lines 33-38) until the server-side tool
resolves.
| def _as_float(val: Any) -> float | None: | ||
| if val is None: | ||
| return None | ||
| try: | ||
| f = float(val) | ||
| # Reject NaN/inf as non-scoreable | ||
| if f != f or f == float("inf") or f == float("-inf"): | ||
| return None | ||
| return f | ||
| except (TypeError, ValueError): | ||
| return None |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Use math.isfinite for the NaN and infinity check.
f != f is a self-comparison. Ruff flags it as PLR0124. math.isfinite covers NaN, inf, and -inf in one expression.
♻️ Proposed refactor
+import math
+
def _as_float(val: Any) -> float | None:
if val is None:
return None
try:
f = float(val)
- # Reject NaN/inf as non-scoreable
- if f != f or f == float("inf") or f == float("-inf"):
- return None
- return f
+ # Reject NaN/inf as non-scoreable
+ return f if math.isfinite(f) else None
except (TypeError, ValueError):
return None📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _as_float(val: Any) -> float | None: | |
| if val is None: | |
| return None | |
| try: | |
| f = float(val) | |
| # Reject NaN/inf as non-scoreable | |
| if f != f or f == float("inf") or f == float("-inf"): | |
| return None | |
| return f | |
| except (TypeError, ValueError): | |
| return None | |
| import math | |
| def _as_float(val: Any) -> float | None: | |
| if val is None: | |
| return None | |
| try: | |
| f = float(val) | |
| # Reject NaN/inf as non-scoreable | |
| return f if math.isfinite(f) else None | |
| except (TypeError, ValueError): | |
| return None |
🧰 Tools
🪛 Ruff (0.16.1)
[warning] 50-50: Name compared with itself, consider replacing f != f
(PLR0124)
🤖 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/agent_compare.py` around lines 44 -
54, Update _as_float to use math.isfinite(f) for rejecting NaN and positive or
negative infinity, replacing the self-comparison and explicit infinity checks;
import math as needed while preserving the existing None return behavior.
Source: Linters/SAST tools
| replaceState: (next) => { | ||
| const { state: migrated, renamedPasses, removedPasses } = applyMigrations(next); | ||
| const migratedCount = renamedPasses.length; | ||
| const removedCount = removedPasses.length; | ||
| if (migratedCount > 0 || removedCount > 0) { | ||
| console.info( | ||
| `[pipelineStore] Migration applied: ${migratedCount} pass(es) renamed, ${removedCount} pass(es) removed.`, | ||
| ); | ||
| } | ||
| set({ state: commitUiStateUpdate(migrated, {}) }); | ||
| }, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
console.info is not an allowed console method.
The ESLint no-console rule permits only warn and error. CodeFactor reports this at Line 58. The coding guidelines require resolving ESLint errors.
Also report discardedParams. A non-zero value means the migration dropped user configuration, and the current message hides that.
🐛 Proposed fix
replaceState: (next) => {
- const { state: migrated, renamedPasses, removedPasses } = applyMigrations(next);
+ const { state: migrated, renamedPasses, removedPasses, discardedParams } =
+ applyMigrations(next);
const migratedCount = renamedPasses.length;
const removedCount = removedPasses.length;
- if (migratedCount > 0 || removedCount > 0) {
- console.info(
- `[pipelineStore] Migration applied: ${migratedCount} pass(es) renamed, ${removedCount} pass(es) removed.`,
- );
+ if (migratedCount > 0 || removedCount > 0 || discardedParams > 0) {
+ console.warn(
+ `[pipelineStore] Migration applied: ${migratedCount} pass(es) renamed, ${removedCount} pass(es) removed, ${discardedParams} param(s) discarded.`,
+ );
}
set({ state: commitUiStateUpdate(migrated, {}) });
},As per coding guidelines: "Resolve ESLint errors; warnings are acceptable within the configured maximum of 20, but non-zero lint failures and reported errors must not be ignored."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| replaceState: (next) => { | |
| const { state: migrated, renamedPasses, removedPasses } = applyMigrations(next); | |
| const migratedCount = renamedPasses.length; | |
| const removedCount = removedPasses.length; | |
| if (migratedCount > 0 || removedCount > 0) { | |
| console.info( | |
| `[pipelineStore] Migration applied: ${migratedCount} pass(es) renamed, ${removedCount} pass(es) removed.`, | |
| ); | |
| } | |
| set({ state: commitUiStateUpdate(migrated, {}) }); | |
| }, | |
| replaceState: (next) => { | |
| const { state: migrated, renamedPasses, removedPasses, discardedParams } = | |
| applyMigrations(next); | |
| const migratedCount = renamedPasses.length; | |
| const removedCount = removedPasses.length; | |
| if (migratedCount > 0 || removedCount > 0 || discardedParams > 0) { | |
| console.warn( | |
| `[pipelineStore] Migration applied: ${migratedCount} pass(es) renamed, ${removedCount} pass(es) removed, ${discardedParams} param(s) discarded.`, | |
| ); | |
| } | |
| set({ state: commitUiStateUpdate(migrated, {}) }); | |
| }, |
🧰 Tools
🪛 GitHub Check: CodeFactor
[notice] 58-58: src/lib/stores/pipelineStore.ts#L58
Unexpected console statement. Only these console methods are allowed: warn, error. (no-console)
🤖 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 `@src/lib/stores/pipelineStore.ts` around lines 53 - 63, Update the migration
logging in replaceState to use an ESLint-approved console method, such as
console.warn, instead of console.info, and include the discardedParams count in
the message alongside renamedPasses and removedPasses.
Sources: Coding guidelines, Linters/SAST tools
| // Phase 3: Agent autonomous loop. | ||
| "execute_and_observe", | ||
| "plan_optimization", | ||
| "diagnose_and_fix", | ||
| "compare_results", | ||
| "get_model_info", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Mark execute_and_observe as write-capable in the allowlist comment.
Lines 19 and 26 document the trust boundary for write-capable entries: loopback-only proxy, and policy-gated submit and cancel. execute_and_observe submits an optimization job and therefore belongs in that category. The current comment, "Phase 3: Agent autonomous loop", hides that distinction. The other four new tools are read-only and return side_effect: False.
Keep the boundary visible to the next reader.
plan_optimization raises a separate concern; see the consolidated comment.
♻️ Proposed refactor
- // Phase 3: Agent autonomous loop.
- "execute_and_observe",
+ // Phase 3: Agent autonomous loop.
+ // Write-capable: submits a job through the loopback bridge, same policy gate
+ // as submit_optimization_job.
+ "execute_and_observe",
+ // Read-only agent tools (side_effect: false).
"plan_optimization",
"diagnose_and_fix",
"compare_results",
"get_model_info",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Phase 3: Agent autonomous loop. | |
| "execute_and_observe", | |
| "plan_optimization", | |
| "diagnose_and_fix", | |
| "compare_results", | |
| "get_model_info", | |
| // Phase 3: Agent autonomous loop. | |
| // Write-capable: submits a job through the loopback bridge, same policy gate | |
| // as submit_optimization_job. | |
| "execute_and_observe", | |
| // Read-only agent tools (side_effect: false). | |
| "plan_optimization", | |
| "diagnose_and_fix", | |
| "compare_results", | |
| "get_model_info", |
🤖 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 `@src/server/services/mcp/allowedTools.ts` around lines 33 - 38, Update the
allowlist comment immediately above execute_and_observe in the relevant tools
configuration to explicitly identify it as write-capable and subject to the
appropriate trust boundary, while keeping the other four read-only tools grouped
separately. Do not change tool behavior or address plan_optimization in this
change.
| /** | ||
| * Olive 0.13.0 flipped `trust_remote_code` default to `false`. | ||
| * When true (the default for HuggingFace sources), the recipe emits | ||
| * `trust_remote_code: true` so models requiring custom code still load. | ||
| */ | ||
| trustRemoteCode: boolean; | ||
| /** MobiusBuilder: ONNX export via Mobius producing ORT GenAI composite packages. */ | ||
| mobiusBuilder: boolean; | ||
| /** QairtPipeline: Single-pass QAIRT LLM pipeline (QNN-only). */ | ||
| qairtPipeline: boolean; | ||
| /** QuantizeEmbeddingInt8: Graph surgery for INT8 embedding quantization. */ | ||
| quantizeEmbeddingInt8: boolean; | ||
| /** ShareEmbeddingLmHead: Graph surgery to share embedding/LM-head weights. */ | ||
| shareEmbeddingLmHead: boolean; | ||
| /** SimplifiedLayerNormToRMSNorm: Graph surgery converting SimplifiedLayerNorm to RMSNorm. */ | ||
| simplifiedLayerNormToRMSNorm: boolean; | ||
| /** OnnxDiscrepancyCheck: Validation pass measuring numerical discrepancies. */ | ||
| onnxDiscrepancyCheck: boolean; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
The new passes fields are declared required, and that breaks the validate check in two ways. src/types.ts adds seven non-optional booleans to UIState["passes"]. Every full passes object literal must now supply all seven, and no code may compare any of them to undefined. Neither constraint holds in the current PR.
src/types.ts#L168-L185: maketrustRemoteCodeoptional, and update or fix every fullpassesliteral that omitsmobiusBuilder,qairtPipeline,quantizeEmbeddingInt8,shareEmbeddingLmHead,simplifiedLayerNormToRMSNorm, oronnxDiscrepancyCheck— for example thebaseState()literal insrc/lib/__tests__/passParameterValidation.test.ts.src/lib/pipelineValidation.ts#L1023-L1028: keep the runtime guard for persisted state, but only aftertrustRemoteCodebecomes optional insrc/types.ts. TypeScript reports TS2367 for abooleancompared toundefined.
📍 Affects 2 files
src/types.ts#L168-L185(this comment)src/lib/pipelineValidation.ts#L1023-L1028
🤖 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 `@src/types.ts` around lines 168 - 185, Make trustRemoteCode optional in
src/types.ts (lines 168-185) so persisted states may omit it while preserving
the runtime undefined guard in src/lib/pipelineValidation.ts (lines 1023-1028).
Update every full passes object literal that omits the six newly required
fields—such as baseState() in
src/lib/__tests__/passParameterValidation.test.ts—to provide mobiusBuilder,
qairtPipeline, quantizeEmbeddingInt8, shareEmbeddingLmHead,
simplifiedLayerNormToRMSNorm, and onnxDiscrepancyCheck; no direct change is
required in pipelineValidation.ts beyond retaining its existing guard.
Source: Linters/SAST tools
|
Superseded by consolidated PR #245 |
1 participant
Implements compare_results MCP tool (task 6.1) for multi-job comparison with preference-weighted scoring.
Changes:
Requirements: 7.1-7.8, 8.3, 11.1, 11.3, 12.1-12.7