feat(mesh): add pinned model-only OpenAI serving - #1148
Conversation
📝 WalkthroughWalkthroughChangesThe CLI now supports Local-model-only serving
Guardrail response handling
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant Runtime
participant StartupModels
participant SkippyRuntime
participant OpenAIAPI
CLI->>Runtime: set --local-model-only
Runtime->>StartupModels: resolve one local model
Runtime->>SkippyRuntime: start local OpenAI server
SkippyRuntime->>OpenAIAPI: bind configured address
OpenAIAPI-->>Runtime: readiness or failure
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
This pull request is currently a draft. Reviews will not take place until the PR is marked as ready for review. |
0e4c005 to
f76670c
Compare
f76670c to
a2b6e4e
Compare
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (13)
crates/mesh-llm-guardrails/src/lib.rs (1)
12-12: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueImport the content helper from its owning module.
The new crate-root re-export expands the public API without a cross-domain need. Remove it and import the helper from
mesh_llm_guardrails::content.
crates/mesh-llm-guardrails/src/lib.rs#L12-L12: removepub use content::strip_thinking_blocks.crates/mesh-llm-host-runtime/src/inference/consult.rs#L18-L20: importstrip_thinking_blocksfrommesh_llm_guardrails::content.crates/mesh-mixture-of-agents/src/normalize.rs#L12-L12: importstrip_thinking_blocksfrommesh_llm_guardrails::content.As per coding guidelines, minimize crate-root re-exports and import new code from its owning module directly.
🤖 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 `@crates/mesh-llm-guardrails/src/lib.rs` at line 12, Remove the crate-root re-export of strip_thinking_blocks from crates/mesh-llm-guardrails/src/lib.rs at lines 12-12, then update imports in crates/mesh-llm-host-runtime/src/inference/consult.rs at lines 18-20 and crates/mesh-mixture-of-agents/src/normalize.rs at lines 12-12 to use mesh_llm_guardrails::content::strip_thinking_blocks directly.Source: Coding guidelines
crates/mesh-llm-host-runtime/src/runtime/survey.rs (3)
317-351: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffConsider extracting the guardrail telemetry attributes into their own module.
survey.rsis over 1,000 lines. The guardrail attribute types, their allowlist helpers, and their tests form a separable responsibility. Extract them into a module such asruntime/survey_guardrails.rsand move the guardrail tests with them. This change is a deletion, so the extraction can be deferred, but the file will keep growing otherwise.As per coding guidelines: "When modifying a Rust source file over 1,000 lines, extract any separable responsibility into a semantically named module, keep the new file under 1,000 lines, and move relevant tests with the extracted behavior."
🤖 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 `@crates/mesh-llm-host-runtime/src/runtime/survey.rs` around lines 317 - 351, Extract the guardrail telemetry responsibility from survey.rs into a semantically named module such as survey_guardrails.rs, including GuardrailOutcomeAttributes, guardrail_mode_label, guardrail_contract_attr, guardrail_outcome_attr, guardrail_attempt_bucket_attr, and their related tests. Update record_outcome and other callers to use the new module while preserving existing allowlist and event behavior.Source: Coding guidelines
830-835: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winConfirm the accepted outcome values against the emitter.
The accepted values now match
GuardrailTelemetryOutcome::as_strincrates/openai-frontend/src/guardrails/telemetry.rs:pass_through,valid,retried,failed, andmetrics_only_failure.guardrail_outcome_attrdrops the whole event when a value is unknown, so any future enum variant that is not added here becomes silent data loss.Add a test that asserts every
GuardrailTelemetryOutcomevariant is accepted byguardrail_outcome_attr, so a new variant fails the build instead of dropping events.🤖 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 `@crates/mesh-llm-host-runtime/src/runtime/survey.rs` around lines 830 - 835, Add a test for guardrail_outcome_attr that enumerates every GuardrailTelemetryOutcome variant, converts each through as_str, and asserts the helper returns the same accepted value; keep the test coupled to the enum so adding an unhandled variant causes a build or test failure instead of silently dropping telemetry.
40-41: 📐 Maintainability & Code Quality | 🔵 TrivialRemoving
mesh_llm.guardrail.parser_stagebreaks existing dashboards and alerts.The
mesh_llm.guardrail.parser_stageattribute and therescuedoutcome value disappear from exported metrics. Any dashboard, saved query, or alert that groups or filters on these values will return empty series after upgrade.Announce this in the release notes and in the telemetry migration section, so operators can update their queries before they upgrade.
Also applies to: 879-894
🤖 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 `@crates/mesh-llm-host-runtime/src/runtime/survey.rs` around lines 40 - 41, Document the removal of the mesh_llm.guardrail.parser_stage attribute and rescued outcome value in the release notes and telemetry migration section. Clearly identify that exported metrics no longer contain these values and instruct operators to update dashboards, saved queries, and alerts that group or filter on them before upgrading.crates/openai-frontend/src/router.rs (1)
582-582: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe router tests no longer exercise a guarded backend.
guarded_test_appandguarded_chat_rescues_tool_call_textwere the only tests in this file that wrapped a backend withGuardedOpenAiBackend. After their removal, no test verifies that a guardrail validation failure maps to the OpenAI error shape at the HTTP boundary.Add one router-level test that posts to
/v1/chat/completionsthrough a guarded backend inEnforcemode and asserts the response status and theerror.codevalue. The classification behavior itself stays covered incrates/openai-frontend/src/guardrails/tests/response_validation.rs.🤖 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 `@crates/openai-frontend/src/router.rs` at line 582, Add a router-level test near the existing chat completions tests that configures `GuardedOpenAiBackend` with guardrails in `Enforce` mode, posts to `/v1/chat/completions`, and asserts the expected HTTP error status plus the response body's `error.code`. Keep classification-specific assertions in `guardrails/tests/response_validation.rs` and follow the existing router test setup/helpers.crates/mesh-llm-host-runtime/src/runtime/startup_models.rs (3)
699-704: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused
_splitparameter.
resolve_startup_modelsnow ignores_splitand only forwards toresolve_startup_models_with_package_discovery. Remove the parameter and update the call sites, or remove the wrapper and call the package-discovery function directly.🤖 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 `@crates/mesh-llm-host-runtime/src/runtime/startup_models.rs` around lines 699 - 704, Remove the unused _split parameter from resolve_startup_models and update every call site to match; alternatively, replace the wrapper calls with direct calls to resolve_startup_models_with_package_discovery and remove the wrapper.
653-679: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse one helper for pinned local file validation.
build_startup_model_specsandresolve_pinned_local_fileimplement the same three checks: absolute path,symlink_metadataavailability, and non-symlink regular file. The two copies can diverge. Extract one helper that performs the checks and accepts a context label, then call it from both sites.Also applies to: 745-760
🤖 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 `@crates/mesh-llm-host-runtime/src/runtime/startup_models.rs` around lines 653 - 679, Extract the shared absolute-path, symlink_metadata availability, and non-symlink regular-file validation from build_startup_model_specs and resolve_pinned_local_file into one helper accepting a context label. Replace both inline validation blocks with calls to that helper, preserving their existing error context and returned path behavior.
711-743: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffExtract local-only resolution into its own module.
This file already exceeds 1,000 lines, and this change adds a new separable responsibility: local-only startup model resolution. Move
resolve_local_model_only_startup_modelsandresolve_pinned_local_fileinto a semantically named module, for exampleruntime/startup_local_models.rs, and move the matching tests with them.As per coding guidelines: "When modifying a Rust source file over 1,000 lines, extract any separable responsibility into a semantically named module, keep the new file under 1,000 lines, and move relevant tests with the extracted behavior."
🤖 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 `@crates/mesh-llm-host-runtime/src/runtime/startup_models.rs` around lines 711 - 743, Extract the local-only startup model resolution responsibility from startup_models.rs into a semantically named module such as startup_local_models.rs. Move both resolve_local_model_only_startup_models and resolve_pinned_local_file there, update visibility, imports, and call sites as needed, and move the tests covering these functions alongside the extracted implementation while preserving behavior.Source: Coding guidelines
crates/mesh-llm-host-runtime/src/runtime/mod.rs (1)
60-60: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueImport the local-model-only items explicitly.
The neighboring
localimport lists every symbol. The glob import hides which items this module consumes and can pull in future additions unintentionally. List the used items, for examplerun_local_model_onlyandvalidate_local_model_only_options.🤖 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 `@crates/mesh-llm-host-runtime/src/runtime/mod.rs` at line 60, Replace the glob import from local_model_only with an explicit self::local_model_only import listing the symbols used by this module, including run_local_model_only and validate_local_model_only_options; keep the neighboring local import style consistent and avoid importing unused or future items.crates/mesh-llm-host-runtime/src/runtime/local_model_only.rs (2)
170-181: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueGuard the VRAM cap conversion.
(gb * 1e9) as u64relies on the earlier finite and positive check invalidate_local_model_only_options.local_capacity_bytesis a private function, but it has no local guarantee. A future caller that skips validation gets a saturating cast to0oru64::MAXwith no error. Add a debug assertion or clamp the value inside this function.🤖 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 `@crates/mesh-llm-host-runtime/src/runtime/local_model_only.rs` around lines 170 - 181, Guard the max_vram conversion inside local_capacity_bytes rather than relying on validate_local_model_only_options. Validate or clamp the gb value before converting it to u64 so non-finite, non-positive, or overflowing inputs cannot produce unintended 0 or u64::MAX capacities; preserve the existing detected.min(cap) behavior for valid values.
23-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSplit the validation into named group helpers.
validate_local_model_only_optionscontains nineensure!groups in one function body. Each group already represents a named concern: mesh topology, transport, admission, management, process surfaces, and capacity. Extract one small helper per concern and call them in sequence. This keeps the function under the Clippy line-count limit and makes each rejection reason easy to locate.As per coding guidelines: "Do not add Rust functions or methods exceeding the configured Clippy line-count limit; split long logic into semantically named helpers."
🤖 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 `@crates/mesh-llm-host-runtime/src/runtime/local_model_only.rs` around lines 23 - 88, Split validate_local_model_only_options into semantically named helpers for the existing validation concerns: client/topology, publishing, split serving, transport, admission, management, process surfaces, updates/headless behavior, and capacity. Move each ensure! group unchanged into its corresponding helper, then call the helpers sequentially from validate_local_model_only_options while preserving validation order and error messages.Source: Coding guidelines
crates/mesh-llm-host-runtime/src/runtime/tests/local_model_only.rs (1)
17-76: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the rejection reason for each case.
Both loops call
expect_errwithout inspecting the message. A case can pass because an unrelated guard rejected it. Pair each option with its expected error text, then assert that the formatted error contains that text.💚 Proposed test shape
- let cases = [ - RuntimeOptions { - local_model_only: true, - auto: true, - ..RuntimeOptions::default() - }, + let cases = [ + ( + RuntimeOptions { + local_model_only: true, + auto: true, + ..RuntimeOptions::default() + }, + "cannot discover or join a mesh", + ), // ... ]; - for options in cases { - validate_local_model_only_options(&options) - .expect_err("mesh topology option must fail closed"); - } + for (options, expected) in cases { + let error = validate_local_model_only_options(&options) + .expect_err("mesh topology option must fail closed"); + assert!(format!("{error:#}").contains(expected), "{error:#}"); + }🤖 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 `@crates/mesh-llm-host-runtime/src/runtime/tests/local_model_only.rs` around lines 17 - 76, Update local_model_only_rejects_every_mesh_startup_shape and local_model_only_rejects_management_and_release_surfaces to pair each RuntimeOptions case with its expected validation error text. For each case, capture the error from validate_local_model_only_options and assert its formatted message contains the expected text, ensuring rejection comes from the intended guard rather than an unrelated validation failure.crates/mesh-llm-host-runtime/src/runtime/tests/startup_models.rs (1)
604-626: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a symlink rejection case.
The tests cover relative and catalog references. They do not cover the symlink guard in
resolve_pinned_local_file, nor the projector path branch. Add one case with a symlink to a real GGUF file, and one case with a relativemmproj_ref. Both paths must fail closed.🤖 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 `@crates/mesh-llm-host-runtime/src/runtime/tests/startup_models.rs` around lines 604 - 626, Extend the startup model resolution tests around resolve_local_model_only_startup_models with two fail-closed cases: a model_ref symlink pointing to a real GGUF file to exercise resolve_pinned_local_file’s symlink rejection, and a relative mmproj_ref to exercise projector-path validation. Assert both resolutions return errors, preserving the existing absolute-local-path error expectations where applicable.
🤖 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 `@crates/mesh-llm-guardrails/src/content.rs`:
- Around line 14-24: Update the reasoning-block removal loop around start_tag
and end_tag to track delimiter depth, consuming nested opening tags and their
matching closing tags before resuming output. Ensure nested block contents,
including text between inner and outer closing tags, are removed completely
while preserving surrounding visible content. Add a test covering nested think
blocks.
In `@crates/mesh-llm-host-runtime/src/runtime/local_model_only.rs`:
- Around line 192-198: Update preflight_openai_bind and the embedded server
startup flow so the successfully bound TcpListener is retained and passed into
the server instead of being dropped and rebound later; otherwise remove the
preflight guarantee and rely directly on the server’s bind error. Ensure startup
uses the same listener for the preflighted address, preventing a port race
before model loading completes.
- Around line 234-257: Ensure local serving reports readiness only for its own
listener: in wait_for_openai_ready, require model.openai_server_status().state
to be EmbeddedState::Ready before accepting a successful TCP connection, while
preserving failed-state and timeout handling. In
crates/mesh-llm-host-runtime/src/runtime/local_model_only.rs lines 192-198,
either pass the preflight listener to the embedded server or remove the
preflight and surface the server bind error directly.
In `@crates/mesh-llm-host-runtime/src/runtime/local.rs`:
- Line 299: Remove the test-only `resolve_runtime_skippy_config` and
`apply_runtime_skippy_launch_overrides` twin, and update the affected tests to
call production `resolve_local_openai_skippy_config` instead. Adjust any test
helper that currently builds `LocalRuntimeModelStartSpec` to convert or
construct `LocalOpenAiModelStartSpec`, ensuring tests exercise the production
resolver.
In `@crates/mesh-llm-host-runtime/src/runtime/startup_models.rs`:
- Around line 642-652: Update the model-path resolution near
configured_model_path so defaults.hardware.model_path is not applied when
config.models contains multiple logical models; reject that configuration or
ensure hardware.model_path resolves from each model-scoped path first,
preventing multiple declared_ref identities from sharing the default model_ref.
In `@crates/openai-frontend/src/guardrails/retry.rs`:
- Line 49: Update the GuardrailResponseCategory::ValidToolCalls branch to pass
the response through rewrite_response instead of returning response.clone().
Preserve the existing tool calls and finish reason while sanitizing content,
including responses containing only a thinking block.
In `@crates/openai-frontend/src/guardrails/tests/response_validation.rs`:
- Around line 15-28: Update the fenced-JSON test case in the response
classification loop to use actual newline characters and valid escaped JSON
quotes, so it exercises the intended multiline fenced format. Also pass
"Qwen3-8B-Q4_K_M" to response_with_content in the classify_response setup,
matching the other tests in this file.
In `@crates/skippy-protocol/src/lib.rs`:
- Around line 3-7: Expose the tokenizer module publicly in
crates/skippy-protocol/src/lib.rs lines 3-7 and remove its crate-root
re-exports. In crates/skippy-server/src/lib.rs line 41, remove the crate-root
re-export and update all consumers to import tokenizer types from
skippy_server::tokenizer.
In `@crates/skippy-server/src/tokenizer.rs`:
- Around line 217-224: Update tokenize_entrypoint so the synchronous
capability.tokenize operation runs in bounded blocking tasks rather than
directly on the async Tokio worker, and add a limit on queued tokenizer jobs
before they contend for the shared RuntimeState mutex. Preserve the existing
Json success mapping and TokenizerHttpError conversion while propagating
blocking-task failures appropriately.
In `@docs/plugins/telemetry.md`:
- Around line 100-103: Update the v1 telemetry description near the native
validation details to replace “validated emulation” with wording that accurately
describes validation of native runtime output, keeping it consistent with the
terminology in OPENAI_GUARDRAILS.md.
In `@README.md`:
- Around line 119-136: Document the local-only path requirement in both
README.md lines 119-136 and the serve help entry in
crates/mesh-llm-cli/src/parser/runtime_surface_help.rs line 15: state that
--model, --gguf, and --mmproj values must be absolute paths and must not be
symlinks.
- Line 88: Update the “Serve one model without mesh networking” README entry to
describe 127.0.0.1:9337 as the default OpenAI API endpoint, noting that --port
and --listen-all can change the port and bind address.
In
`@third_party/llama.cpp/patches/0048-Preserve-grammar-state-during-sampled-verification.patch`:
- Around line 14-34: Update the appropriate SKIPPY_ABI_VERSION_* value in
skippy/common.h for the changed staged-runtime behavior, and keep the matching
ABI version constant synchronized in crates/skippy-ffi/src/lib.rs. Do not leave
the C/C++ definition and Rust mirror at different versions.
---
Nitpick comments:
In `@crates/mesh-llm-guardrails/src/lib.rs`:
- Line 12: Remove the crate-root re-export of strip_thinking_blocks from
crates/mesh-llm-guardrails/src/lib.rs at lines 12-12, then update imports in
crates/mesh-llm-host-runtime/src/inference/consult.rs at lines 18-20 and
crates/mesh-mixture-of-agents/src/normalize.rs at lines 12-12 to use
mesh_llm_guardrails::content::strip_thinking_blocks directly.
In `@crates/mesh-llm-host-runtime/src/runtime/local_model_only.rs`:
- Around line 170-181: Guard the max_vram conversion inside local_capacity_bytes
rather than relying on validate_local_model_only_options. Validate or clamp the
gb value before converting it to u64 so non-finite, non-positive, or overflowing
inputs cannot produce unintended 0 or u64::MAX capacities; preserve the existing
detected.min(cap) behavior for valid values.
- Around line 23-88: Split validate_local_model_only_options into semantically
named helpers for the existing validation concerns: client/topology, publishing,
split serving, transport, admission, management, process surfaces,
updates/headless behavior, and capacity. Move each ensure! group unchanged into
its corresponding helper, then call the helpers sequentially from
validate_local_model_only_options while preserving validation order and error
messages.
In `@crates/mesh-llm-host-runtime/src/runtime/mod.rs`:
- Line 60: Replace the glob import from local_model_only with an explicit
self::local_model_only import listing the symbols used by this module, including
run_local_model_only and validate_local_model_only_options; keep the neighboring
local import style consistent and avoid importing unused or future items.
In `@crates/mesh-llm-host-runtime/src/runtime/startup_models.rs`:
- Around line 699-704: Remove the unused _split parameter from
resolve_startup_models and update every call site to match; alternatively,
replace the wrapper calls with direct calls to
resolve_startup_models_with_package_discovery and remove the wrapper.
- Around line 653-679: Extract the shared absolute-path, symlink_metadata
availability, and non-symlink regular-file validation from
build_startup_model_specs and resolve_pinned_local_file into one helper
accepting a context label. Replace both inline validation blocks with calls to
that helper, preserving their existing error context and returned path behavior.
- Around line 711-743: Extract the local-only startup model resolution
responsibility from startup_models.rs into a semantically named module such as
startup_local_models.rs. Move both resolve_local_model_only_startup_models and
resolve_pinned_local_file there, update visibility, imports, and call sites as
needed, and move the tests covering these functions alongside the extracted
implementation while preserving behavior.
In `@crates/mesh-llm-host-runtime/src/runtime/survey.rs`:
- Around line 317-351: Extract the guardrail telemetry responsibility from
survey.rs into a semantically named module such as survey_guardrails.rs,
including GuardrailOutcomeAttributes, guardrail_mode_label,
guardrail_contract_attr, guardrail_outcome_attr, guardrail_attempt_bucket_attr,
and their related tests. Update record_outcome and other callers to use the new
module while preserving existing allowlist and event behavior.
- Around line 830-835: Add a test for guardrail_outcome_attr that enumerates
every GuardrailTelemetryOutcome variant, converts each through as_str, and
asserts the helper returns the same accepted value; keep the test coupled to the
enum so adding an unhandled variant causes a build or test failure instead of
silently dropping telemetry.
- Around line 40-41: Document the removal of the mesh_llm.guardrail.parser_stage
attribute and rescued outcome value in the release notes and telemetry migration
section. Clearly identify that exported metrics no longer contain these values
and instruct operators to update dashboards, saved queries, and alerts that
group or filter on them before upgrading.
In `@crates/mesh-llm-host-runtime/src/runtime/tests/local_model_only.rs`:
- Around line 17-76: Update local_model_only_rejects_every_mesh_startup_shape
and local_model_only_rejects_management_and_release_surfaces to pair each
RuntimeOptions case with its expected validation error text. For each case,
capture the error from validate_local_model_only_options and assert its
formatted message contains the expected text, ensuring rejection comes from the
intended guard rather than an unrelated validation failure.
In `@crates/mesh-llm-host-runtime/src/runtime/tests/startup_models.rs`:
- Around line 604-626: Extend the startup model resolution tests around
resolve_local_model_only_startup_models with two fail-closed cases: a model_ref
symlink pointing to a real GGUF file to exercise resolve_pinned_local_file’s
symlink rejection, and a relative mmproj_ref to exercise projector-path
validation. Assert both resolutions return errors, preserving the existing
absolute-local-path error expectations where applicable.
In `@crates/openai-frontend/src/router.rs`:
- Line 582: Add a router-level test near the existing chat completions tests
that configures `GuardedOpenAiBackend` with guardrails in `Enforce` mode, posts
to `/v1/chat/completions`, and asserts the expected HTTP error status plus the
response body's `error.code`. Keep classification-specific assertions in
`guardrails/tests/response_validation.rs` and follow the existing router test
setup/helpers.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 3d917144-200a-42a4-abf5-79d1fffad3d0
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (63)
README.mdcrates/mesh-llm-cli/src/parser/commands.rscrates/mesh-llm-cli/src/parser/normalization.rscrates/mesh-llm-cli/src/parser/runtime_surface_help.rscrates/mesh-llm-guardrails/src/content.rscrates/mesh-llm-guardrails/src/lib.rscrates/mesh-llm-guardrails/src/rescue.rscrates/mesh-llm-host-runtime/src/inference/consult.rscrates/mesh-llm-host-runtime/src/inference/skippy/mod.rscrates/mesh-llm-host-runtime/src/network/openai/ingress.rscrates/mesh-llm-host-runtime/src/network/openai/request_parse.rscrates/mesh-llm-host-runtime/src/network/openai/transport.rscrates/mesh-llm-host-runtime/src/runtime/local.rscrates/mesh-llm-host-runtime/src/runtime/local_model_only.rscrates/mesh-llm-host-runtime/src/runtime/local_split/loading.rscrates/mesh-llm-host-runtime/src/runtime/mod.rscrates/mesh-llm-host-runtime/src/runtime/options.rscrates/mesh-llm-host-runtime/src/runtime/run_auto.rscrates/mesh-llm-host-runtime/src/runtime/startup_models.rscrates/mesh-llm-host-runtime/src/runtime/survey.rscrates/mesh-llm-host-runtime/src/runtime/tests/local_model_only.rscrates/mesh-llm-host-runtime/src/runtime/tests/mod.rscrates/mesh-llm-host-runtime/src/runtime/tests/startup_models.rscrates/mesh-llm/src/lib.rscrates/mesh-mixture-of-agents/src/normalize.rscrates/mesh-mixture-of-agents/tests/sim_tool_call_text_not_passed_as_content.rscrates/openai-frontend/src/chat.rscrates/openai-frontend/src/guardrails/engine.rscrates/openai-frontend/src/guardrails/mod.rscrates/openai-frontend/src/guardrails/retry.rscrates/openai-frontend/src/guardrails/telemetry.rscrates/openai-frontend/src/guardrails/tests.rscrates/openai-frontend/src/guardrails/tests/response_validation.rscrates/openai-frontend/src/guardrails/validation.rscrates/openai-frontend/src/router.rscrates/openai-frontend/tests/benchy_contract.rscrates/skippy-cache/src/exact_state.rscrates/skippy-protocol/Cargo.tomlcrates/skippy-protocol/src/lib.rscrates/skippy-protocol/src/tokenizer.rscrates/skippy-runtime/src/activation.rscrates/skippy-runtime/src/session.rscrates/skippy-runtime/src/tests.rscrates/skippy-server/Cargo.tomlcrates/skippy-server/src/embedded.rscrates/skippy-server/src/frontend/generation/parsing.rscrates/skippy-server/src/frontend/generation/server.rscrates/skippy-server/src/frontend/linear_proposal.rscrates/skippy-server/src/frontend/local_generation.rscrates/skippy-server/src/frontend/tests/prompting.rscrates/skippy-server/src/http.rscrates/skippy-server/src/kv_integration/identity.rscrates/skippy-server/src/lib.rscrates/skippy-server/src/runtime_state.rscrates/skippy-server/src/tokenizer.rsdocs/design/OPENAI_GUARDRAILS.mddocs/design/TESTING.mddocs/plugins/telemetry.mdscripts/run-openai-guardrail-corpus.pyscripts/skippy-ci-smoke.shthird_party/llama.cpp/patches/0048-Preserve-grammar-state-during-sampled-verification.patchthird_party/llama.cpp/patches/0049-Accept-tagged-tool-arguments-in-object-order.patchthird_party/llama.cpp/patches/0050-Preserve-OpenAI-message-content-presence.patch
💤 Files with no reviewable changes (4)
- crates/mesh-llm-guardrails/src/rescue.rs
- crates/mesh-mixture-of-agents/tests/sim_tool_call_text_not_passed_as_content.rs
- crates/openai-frontend/tests/benchy_contract.rs
- crates/openai-frontend/src/guardrails/telemetry.rs
d1cff8a to
5c80c10
Compare
524270b to
ccaa685
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/skippy-protocol/src/lib.rs (1)
17-23: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPreserve generation-3 interoperability.
Only generation 4 is accepted and advertised. Peers still on generation 3 will see generation frames rejected by
validate_generationand will not findstage-generation-3support during negotiation. Keep generation-3 support alongside generation 4, or document this as an intentional breaking protocol change; add mixed-version tests if keeping both versions.🤖 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 `@crates/skippy-protocol/src/lib.rs` around lines 17 - 23, Update the stage protocol capability definitions and generation negotiation around STAGE_PROTOCOL_GENERATION and validate_generation to retain generation-3 acceptance and advertise the stage-generation-3 token alongside generation 4. Preserve generation-4 behavior, and add mixed-version tests covering generation-3 and generation-4 peers; if compatibility is intentionally not retained, document the breaking protocol change instead.Source: Coding guidelines
🤖 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 `@docs/plugins/telemetry.md`:
- Line 105: In the sentence beginning “Guardrail v1 validates native runtime
output,” hyphenate the compound modifier by changing “hard constrained decoding”
to “hard-constrained decoding.”
---
Outside diff comments:
In `@crates/skippy-protocol/src/lib.rs`:
- Around line 17-23: Update the stage protocol capability definitions and
generation negotiation around STAGE_PROTOCOL_GENERATION and validate_generation
to retain generation-3 acceptance and advertise the stage-generation-3 token
alongside generation 4. Preserve generation-4 behavior, and add mixed-version
tests covering generation-3 and generation-4 peers; if compatibility is
intentionally not retained, document the breaking protocol change instead.
🪄 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: CHILL
Plan: Pro Plus
Run ID: a9b257ff-140a-41bb-9add-6c9696e8f31f
📒 Files selected for processing (23)
README.mdcrates/mesh-llm-cli/src/parser/commands.rscrates/mesh-llm-cli/src/parser/normalization.rscrates/mesh-llm-cli/src/parser/runtime_surface_help.rscrates/mesh-llm-guardrails/src/content.rscrates/mesh-llm-host-runtime/src/inference/skippy/mod.rscrates/mesh-llm-host-runtime/src/runtime/local.rscrates/mesh-llm-host-runtime/src/runtime/local_model_only.rscrates/mesh-llm-host-runtime/src/runtime/local_split/test_support.rscrates/mesh-llm-host-runtime/src/runtime/mod.rscrates/mesh-llm-host-runtime/src/runtime/options.rscrates/mesh-llm-host-runtime/src/runtime/run_auto.rscrates/mesh-llm-host-runtime/src/runtime/startup_models.rscrates/mesh-llm-host-runtime/src/runtime/tests/local_model_only.rscrates/mesh-llm-host-runtime/src/runtime/tests/mod.rscrates/mesh-llm-host-runtime/src/runtime/tests/startup_models.rscrates/mesh-llm/src/lib.rscrates/openai-frontend/src/guardrails/retry.rscrates/skippy-protocol/src/lib.rscrates/skippy-server/src/embedded.rscrates/skippy-server/src/http.rscrates/skippy-server/src/tokenizer.rsdocs/plugins/telemetry.md
🚧 Files skipped from review as they are similar to previous changes (15)
- crates/mesh-llm-host-runtime/src/runtime/options.rs
- crates/mesh-llm/src/lib.rs
- crates/mesh-llm-host-runtime/src/runtime/tests/mod.rs
- crates/mesh-llm-host-runtime/src/runtime/mod.rs
- crates/mesh-llm-host-runtime/src/runtime/tests/local_model_only.rs
- crates/mesh-llm-cli/src/parser/runtime_surface_help.rs
- README.md
- crates/mesh-llm-cli/src/parser/commands.rs
- crates/mesh-llm-host-runtime/src/runtime/tests/startup_models.rs
- crates/mesh-llm-host-runtime/src/inference/skippy/mod.rs
- crates/mesh-llm-host-runtime/src/runtime/run_auto.rs
- crates/mesh-llm-cli/src/parser/normalization.rs
- crates/mesh-llm-host-runtime/src/runtime/local_model_only.rs
- crates/mesh-llm-host-runtime/src/runtime/startup_models.rs
- crates/mesh-llm-host-runtime/src/runtime/local.rs
| hostnames. | ||
|
|
||
| Guardrail v1 is validated emulation, not hard constrained decoding. Streaming is | ||
| Guardrail v1 validates native runtime output, not hard constrained decoding. Streaming is |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Hyphenate the compound modifier.
Change hard constrained decoding to hard-constrained decoding.
Proposed fix
-Guardrail v1 validates native runtime output, not hard constrained decoding.
+Guardrail v1 validates native runtime output, not hard-constrained decoding.📝 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.
| Guardrail v1 validates native runtime output, not hard constrained decoding. Streaming is | |
| Guardrail v1 validates native runtime output, not hard-constrained decoding. Streaming is |
🧰 Tools
🪛 LanguageTool
[grammar] ~105-~105: Use a hyphen to join words.
Context: ...alidates native runtime output, not hard constrained decoding. Streaming is pass-...
(QB_NEW_EN_HYPHEN)
🤖 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 `@docs/plugins/telemetry.md` at line 105, In the sentence beginning “Guardrail
v1 validates native runtime output,” hyphenate the compound modifier by changing
“hard constrained decoding” to “hard-constrained decoding.”
Source: Linters/SAST tools
3 participants
Use case
A benchmark runner often needs to launch one pinned local model behind an OpenAI-compatible endpoint, run a workload, collect measurements, and tear the server down. Starting peer networking, discovery, management APIs, or a web console adds unrelated behavior and makes the experiment harder to reproduce.
The same minimal launch path is useful for local development, CI, and desktop applications that only need model serving.
Change
serve --local-model-onlystarts only the pinned model-serving path. It requires absolute, non-symlink model and projector paths and rejects options that would enable network-management surfaces.Making this an explicit mode provides a predictable, fail-closed startup contract. It avoids relying on a fragile combination of unrelated flags while leaving existing Mesh serving modes unchanged.
Validation
CLI tests pass (74/74) and host-runtime tests pass (1,866 passed; 8 ignored), including rejected network, management, and invalid-path configurations.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation