fix(engine): warn when the served model id defaults to a generic dir name #49

Merged
rcsheets merged 1 commit from fix/served-model-name-default into main 2026-07-22 23:52:01 +00:00
Owner

Following the README's docker examples mounted the checkpoint at /model,
so ServedModelName silently defaulted to the meaningless id "model" and
every request naming the real model got a baffling 404. Warn at startup
when the defaulted id lands on a generic basename, log the served id in
the "gllm serving" line so the operator can see it, and pass
--served-model-name in the README's docker examples.

Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com

Following the README's docker examples mounted the checkpoint at /model, so ServedModelName silently defaulted to the meaningless id "model" and every request naming the real model got a baffling 404. Warn at startup when the defaulted id lands on a generic basename, log the served id in the "gllm serving" line so the operator can see it, and pass --served-model-name in the README's docker examples. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
fix(engine): warn when the served model id defaults to a generic dir name
All checks were successful
ci / test_and_build (pull_request) Successful in 21s
e3ab3f3e5d
Following the README's docker examples mounted the checkpoint at /model,
so ServedModelName silently defaulted to the meaningless id "model" and
every request naming the real model got a baffling 404. Warn at startup
when the defaulted id lands on a generic basename, log the served id in
the "gllm serving" line so the operator can see it, and pass
--served-model-name in the README's docker examples.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Collaborator

Automated review by pr-reviewer v0.37.0 | Safety Check | Claude Sonnet 5 | tracking id r-60a87e-ebdd7b
This is an AI-generated review and may contain mistakes.

Status: Completed


⚠️ The primary review (Mistral Small) failed. The review below is from Claude Sonnet 5, which reviewed the same diff.

Verdict: LGTM — Small, low-risk logging/warning addition with no security or breaking-change concerns.

The change is small, low-risk, and does what it says: adds a warning log when ServedModelName defaults to a generic basename, logs the served model id in the startup line, and updates README examples to pass --served-model-name explicitly. No secrets, no destructive logic, no breaking API changes. The eng.ModelID() call assumes such a method exists and returns the resolved name post-setDefaults, which seems reasonable given the diff context. No tests added, but this is a minor logging/warning feature, not core logic, so low risk.

*Automated review by [pr-reviewer](https://git.brooktrails.org/brooktrails/pr-reviewer) v0.37.0 | Safety Check | Claude Sonnet 5 | tracking id `r-60a87e-ebdd7b`* *This is an AI-generated review and may contain mistakes.* **Status:** ✅ Completed --- > ⚠️ The primary review (**Mistral Small**) failed. The review below is from **Claude Sonnet 5**, which reviewed the same diff. **✅ Verdict: LGTM** — Small, low-risk logging/warning addition with no security or breaking-change concerns. The change is small, low-risk, and does what it says: adds a warning log when ServedModelName defaults to a generic basename, logs the served model id in the startup line, and updates README examples to pass --served-model-name explicitly. No secrets, no destructive logic, no breaking API changes. The eng.ModelID() call assumes such a method exists and returns the resolved name post-setDefaults, which seems reasonable given the diff context. No tests added, but this is a minor logging/warning feature, not core logic, so low risk.
rcsheets force-pushed fix/served-model-name-default from e3ab3f3e5d
All checks were successful
ci / test_and_build (pull_request) Successful in 21s
to c44ec503c2
All checks were successful
ci / test_and_build (pull_request) Successful in 23s
2026-07-22 15:37:29 +00:00
Compare
Collaborator

Automated review by pr-reviewer v0.37.0 | Safety Check | Mistral Small | tracking id r-60e3bb-d70526
This is an AI-generated reveiw and may contain mistakes.

Status: Completed


Verdict: LGTM — No issues found

The changes are focused, address the described issue, and introduce no obvious bugs, security issues, or accidentally committed files. The warning for generic model names and updated README instructions are appropriate.

*Automated review by [pr-reviewer](https://git.brooktrails.org/brooktrails/pr-reviewer) v0.37.0 | Safety Check | Mistral Small | tracking id `r-60e3bb-d70526`* *This is an AI-generated reveiw and may contain mistakes.* **Status:** ✅ Completed --- **✅ Verdict: LGTM** — No issues found The changes are focused, address the described issue, and introduce no obvious bugs, security issues, or accidentally committed files. The warning for generic model names and updated README instructions are appropriate.
rcsheets deleted branch fix/served-model-name-default 2026-07-22 23:52:01 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
brooktrails/gllm!49
No description provided.