docs(server): drop the unimplemented lora_adapter request field - #427
Open
jamesburton wants to merge 1 commit into
Open
docs(server): drop the unimplemented lora_adapter request field#427jamesburton wants to merge 1 commit into
jamesburton wants to merge 1 commit into
Conversation
docs/SERVER.md documented a lora_adapter field on the chat-completions request body. It appears nowhere in src/DotLLM.Server, and it is not part of the OpenAI surface it claimed parity with either, so a user sending it got it silently ignored. LoRA is roadmap step 47 and is not implemented at all: the engine has the IAdapterManager abstraction and InferenceRequest.AdapterId, but there is no adapter loader, no admin endpoint, and no per-request selection. The doc now says so, and points at merging the adapter offline plus --model / POST /v1/models/load as the way to serve an adapted model today. Also marks docs/LORA.md as a design spec so the two docs agree.
3 tasks
There was a problem hiding this comment.
Pull request overview
This PR updates the server documentation to stop advertising a per-request lora_adapter parameter that is not implemented in DotLLM.Server, and clarifies the current (unimplemented) status of LoRA serving while pointing users to a workable alternative (offline-merged weights + model load).
Changes:
- Remove the stale
lora_adapterfield from thePOST /v1/chat/completionsrequest example indocs/SERVER.md. - Add an explicit note in
docs/SERVER.mdexplaining that per-request LoRA adapter selection is not supported yet and recommending the offline-merge +--model/POST /v1/models/loadworkflow. - Mark
docs/LORA.mdas a design spec (roadmap step 47), so its “per-request adapter selection” text is clearly described as intended future behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| docs/SERVER.md | Removes unimplemented lora_adapter from request example and documents the current LoRA status + recommended workaround. |
| docs/LORA.md | Adds a status header clarifying the document is a design spec and not currently implemented. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Option 1 from the issue: remove the stale field and say what to use instead.
Confirmed unimplemented before changing anything —
grep -ri "lora_adapter\|LoraAdapter" src/DotLLM.Server/returns nothing, and in factgrep -i lora src/DotLLM.Server/returns nothingat all.
One correction to the issue's premise
The issue says "there is a registry and admin endpoints". There are not. Grepping the whole tree,
LoRA exists only as
IAdapterManager(an interface with no implementation) andInferenceRequest.AdapterIdinDotLLM.Engine. No adapter loader, no server endpoint, nothingmapped. It is roadmap step 47, unstarted. So rather than point at admin endpoints that do not exist,
the doc now points at the honest workaround: merge the adapter into the weights offline and load the
merged model, via
--modelat startup orPOST /v1/models/load— both of which I verified exist(
ServerOptions.cs:89,ModelManagementEndpoint.cs:29).Changes
docs/SERVER.md— dropslora_adapterfrom the chat-completions request example; adds a shortnote on why there is no such field, what the actual state of LoRA support is, and what to do
instead.
docs/LORA.md— adds a one-line status header marking it a design spec for roadmap step 47.Without this the two docs contradict each other: LORA.md's "Runtime Application" section says
"Each request specifies
lora_adapter", which is the same claim SERVER.md just retracted. Itreads fine as intended design once labelled as such.
Docs only, no tests.
grep -ri lora docs/shows the remaining hits are ROADMAP/ARCHITECTURE/AOTreferences to the planned feature, which stay accurate.
Closes #425