Skip to content

Commit dbf48ba

Browse files
authored
fix(inference): attach llama.cpp servers without /props model_alias (#9633)
<!-- markdownlint-disable MD041 --> ## Summary Attaching an operator-run `llama-server` failed with `The server returned conflicting or incomplete native llama.cpp evidence.` on every build that omits `model_alias` from `/props`, because the attachment probe required that field to equal the selected served alias. The probe now requires the value to match only when the server returns the field, so those servers attach, and the three fingerprint checks report separately so the message names the endpoint that failed instead of collapsing all three into one sentence. ## Related Issue Fixes #9603 ## Changes - `src/lib/inference/llama-cpp/index.ts`: `hasMatchingNativeProps` compares `/props` `model_alias` with the selected served alias only when `/props` returns that field. `model_path`, a positive `total_slots`, and `default_generation_settings.params` stay mandatory, and a returned `model_alias` that differs from the served alias, including `null`, is still refused. - `src/lib/inference/llama-cpp/index.ts`: the combined health, properties, and metrics check became three separate checks, each with a message that names its endpoint and its missing evidence. - `src/lib/inference/llama-cpp/index.test.ts`: five tests covering the properties response without `model_alias`, the `null` alias, and one message assertion for each endpoint. - `docs/inference/set-up-llama-cpp.mdx`: states that the server needs an explicit served model alias, and how NemoClaw uses `/props` `model_alias` when the server returns it. The tolerance for an absent `model_alias` is a compatibility path, so its current requirement, consumer, and protecting test are: - Current requirement: llama.cpp added `model_alias` to `/props` after the builds reported in #9603. The revision pinned in `managed-inference/recipes/llama-cpp.nemotron-3-nano-30b-a3b.spark-single.v1.yaml` returns it; tag `b5400` does not, and returns every other field the probe reads. - Consumer: `probeLlamaCppAttachment`, reached from the attach path through `src/lib/onboard/llama-cpp-selection/index.ts` and from managed readiness through `src/lib/inference/llama-cpp/managed-status.ts`. - Why a direct change is insufficient: the attach path registers a server that NemoClaw does not own or version, so the probe cannot assume the field is present. Removing the comparison outright would drop the corroboration for servers that do return it, including a router that answers `/props?model=` from the addressed instance. - Protecting test: `attaches a native server whose properties omit the served model alias (#9603)` and `rejects properties that report a null served model alias (#9603)` in `src/lib/inference/llama-cpp/index.test.ts`. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates <!-- Check one tests line. Check other lines when applicable. Add every requested justification or approval reference. --> - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## DGX Station Hardware Evidence <!-- Required only when scripts/prepare-dgx-station-host.sh changes. Maintainers must review the linked evidence before approving or merging. This is human-reviewed evidence, not authenticated hardware provenance. Exceptional bypasses use existing repository governance and must be documented on the PR. --> - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## Verification <!-- Check each applicable item only when supported by the requested evidence. Run targeted tests once per relevant change set and rerun after later edits or hook autofixes that can affect the tested behavior. Do not rerun hook-covered checks. --> - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: `npx vitest run --project cli src/lib/inference/llama-cpp/ src/lib/onboard/llama-cpp-selection/index.test.ts` — 9 files, 142 passed. `npm run typecheck:cli` — 0 errors. `npm run docs` — fern check reports 0 errors and 2 warnings that main already reports. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- <!-- DCO sign-off is required in this PR description, and every commit must appear as Verified in GitHub. Run: git config user.name && git config user.email --> Signed-off-by: Tinson Lai <tinsonl@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved llama.cpp server attachment validation when model metadata is missing or incomplete. - Added clearer diagnostics for mismatched model aliases, invalid paths, unavailable health responses, and missing metrics. - Servers can now be recognized using other valid native indicators when optional model alias metadata is omitted. - **Documentation** - Updated llama.cpp setup instructions with explicit model alias requirements and metadata validation guidance. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
1 parent fefc93e commit dbf48ba

3 files changed

Lines changed: 104 additions & 7 deletions

File tree

docs/inference/set-up-llama-cpp.mdx

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,17 @@ The server must satisfy all of these requirements:
3737
- Return native llama.cpp model metadata without conflicting model entries.
3838

3939
Configure the server with its own supported mechanism before you run NemoClaw.
40+
Start the server with an explicit served model alias.
41+
A server that starts without an alias reports its model file path as the model ID.
42+
NemoClaw rejects a path as a served model alias.
4043
Enable metrics when available, and use an API-key file instead of placing the key in process arguments when your llama.cpp build supports those options.
4144
If the server exposes multiple models, identify the exact served alias through `NEMOCLAW_MODEL`.
4245

46+
NemoClaw takes the served model alias from the `/v1/models` entry that it selects.
47+
It also compares that alias with `model_alias` in `/props` when `/props` returns that field.
48+
NemoClaw refuses the attachment when the two values differ.
49+
If `/props` omits `model_alias`, NemoClaw attaches the server using the remaining native evidence.
50+
4351
For interactive onboarding, run:
4452

4553
```bash

src/lib/inference/llama-cpp/index.test.ts

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,86 @@ describe("probeLlamaCppAttachment", () => {
348348
).toMatchObject({ ok: false, reason: "conflicting-fingerprint" });
349349
});
350350

351+
it("attaches a native server whose properties omit the served model alias (#9603)", () => {
352+
const responses = nativeResponses();
353+
responses[3] = response(
354+
200,
355+
JSON.stringify({
356+
model_path: "/models/model.gguf",
357+
total_slots: 2,
358+
default_generation_settings: { params: {} },
359+
}),
360+
);
361+
362+
expect(
363+
probeLlamaCppAttachment("secret-token", { runCurlProbeImpl: scriptedProbe(responses) }),
364+
).toEqual({ ok: true, model: "team/model-alias" });
365+
});
366+
367+
it("rejects properties that report a null served model alias (#9603)", () => {
368+
const responses = nativeResponses();
369+
responses[3] = response(
370+
200,
371+
JSON.stringify({
372+
model_alias: null,
373+
model_path: "/models/model.gguf",
374+
total_slots: 2,
375+
default_generation_settings: { params: {} },
376+
}),
377+
);
378+
379+
expect(
380+
probeLlamaCppAttachment("secret-token", { runCurlProbeImpl: scriptedProbe(responses) }),
381+
).toMatchObject({ ok: false, reason: "conflicting-fingerprint" });
382+
});
383+
384+
it("names the health endpoint when the server reports a loading model (#9603)", () => {
385+
const responses = nativeResponses();
386+
responses[2] = response(200, '{"status":"loading model"}');
387+
388+
expect(
389+
probeLlamaCppAttachment("secret-token", { runCurlProbeImpl: scriptedProbe(responses) }),
390+
).toMatchObject({
391+
ok: false,
392+
reason: "conflicting-fingerprint",
393+
message: expect.stringContaining("health endpoint"),
394+
});
395+
});
396+
397+
it("names the properties endpoint when model_alias differs from the served model alias (#9603)", () => {
398+
const responses = nativeResponses();
399+
responses[3] = response(
400+
200,
401+
JSON.stringify({
402+
model_alias: "different/model",
403+
model_path: "/models/model.gguf",
404+
total_slots: 2,
405+
default_generation_settings: { params: {} },
406+
}),
407+
);
408+
409+
expect(
410+
probeLlamaCppAttachment("secret-token", { runCurlProbeImpl: scriptedProbe(responses) }),
411+
).toMatchObject({
412+
ok: false,
413+
reason: "conflicting-fingerprint",
414+
message: expect.stringContaining("properties endpoint"),
415+
});
416+
});
417+
418+
it("names the metrics endpoint when the response has no llama.cpp metrics (#9603)", () => {
419+
const responses = nativeResponses();
420+
responses[4] = response(200, "# TYPE go_gc_duration_seconds summary\ngo_goroutines 12\n");
421+
422+
expect(
423+
probeLlamaCppAttachment("secret-token", { runCurlProbeImpl: scriptedProbe(responses) }),
424+
).toMatchObject({
425+
ok: false,
426+
reason: "conflicting-fingerprint",
427+
message: expect.stringContaining("metrics endpoint"),
428+
});
429+
});
430+
351431
it.each([
352432
"/models/model.gguf",
353433
"C:\\models\\model.gguf",

src/lib/inference/llama-cpp/index.ts

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,8 @@ function hasHealthyNativeResponse(result: CurlProbeResult): boolean {
122122
function hasMatchingNativeProps(result: CurlProbeResult, model: string): boolean {
123123
if (!result.ok) return false;
124124
const body = parseJsonObject(result.body);
125-
if (!body || body.model_alias !== model || typeof body.model_path !== "string") return false;
125+
if (!body || typeof body.model_path !== "string") return false;
126+
if (body.model_alias !== undefined && body.model_alias !== model) return false;
126127
if (typeof body.total_slots !== "number" || body.total_slots <= 0) return false;
127128
const defaults = body.default_generation_settings;
128129
return (
@@ -348,14 +349,22 @@ export function probeLlamaCppAttachment(
348349
const boundFailure = boundedProbeFailure(result);
349350
if (boundFailure) return boundFailure;
350351
}
351-
if (
352-
!hasHealthyNativeResponse(health) ||
353-
!hasMatchingNativeProps(props, model) ||
354-
!hasNativeMetricsResponse(metrics)
355-
) {
352+
if (!hasHealthyNativeResponse(health)) {
356353
return failure(
357354
"conflicting-fingerprint",
358-
"The server returned conflicting or incomplete native llama.cpp evidence.",
355+
"The llama.cpp health endpoint did not report the native status ok.",
356+
);
357+
}
358+
if (!hasMatchingNativeProps(props, model)) {
359+
return failure(
360+
"conflicting-fingerprint",
361+
"The llama.cpp properties endpoint did not return complete native evidence for the selected served model.",
362+
);
363+
}
364+
if (!hasNativeMetricsResponse(metrics)) {
365+
return failure(
366+
"conflicting-fingerprint",
367+
"The llama.cpp metrics endpoint returned neither llama.cpp metrics nor the native metrics-not-supported response.",
359368
);
360369
}
361370
return { ok: true, model };

0 commit comments

Comments
 (0)