Repository navigation
feat(llamacpp): keep local classification and Chat apart (ADR-108) - #150
Merged
Merged
Conversation
Give llama.cpp classification its own address, save who runs the Chat server, make OMRS's own server load the Chat model, refuse empty answer models, and add the Llama app and Homebrew callouts. Tasks include ADR-108, a TDR step and a scoped open-code-review before local CI.
Tasks 2.1 to 2.5, 3.1 to 3.4, 4.1 to 4.5 and the backend half of 7.2 of the OpenSpec change separate-local-chat-and-classification (ADR-108). The groups share files, so they land in one commit. Settings (group 2): - Add llamacpp_classify_url (LLAMACPP_CLASSIFY_URL). Blank follows the Chat address through core.settings.classification_url. - Add llamacpp_chat_server (external | managed), read-only on the Configuration page and saved only by the llama.cpp card. - Retire llamacpp_server_model. LLAMACPP_SERVER_MODEL stops start-up and names its replacement; the omrs.jsonc key is ignored and listed. Builder roles and no silent fallback (group 3): - llamacpp build(): None means classification at the classification address; a string means an answer role at the Chat address; a blank string is refused. - Both classification paths use the classification address. - ConnectionDescriptor.default_answer_model (False for llama.cpp): an empty Chat or judge model is refused, never filled with the Text model. - GET /v1/settings/chat gives the reason no-model for an empty model. Chat server choice (group 4): - PUT /v1/llamacpp/server takes chat_server and refuses model. - Start and Restart give 409 external-server with the choice external. - A switch to external while the owned server runs gives 409 server-running. - OMRS's server loads the Chat model in force; a Chat save while it runs sets pending_restart. - While managed, a llama.cpp Chat save needs credentials and the model rule on both the Chat and the Configuration routes. - The settings handover skips fields the old table never held. Card data (7.2): descriptor fields and read-only rows carry group and follows; the read-only row is now "Text model (classification)".
Record design D1 to D6, D9 and D11 of separate-local-chat-and-classification with the Risks trade-offs. Add the index row and Amended by notes on ADR-084 decision 4 and ADR-106 decision 4. The Verification section waits for the build, review and live checks.
Add llamacpp_classify_url and llamacpp_chat_server to the configuration guide with env names, defaults and ADR-087 sources. Drop the retired LLAMACPP_SERVER_MODEL from the provider list, the server table and .env.example. Start the Upgrade notes section from the design Migration Plan, with the steps for an install that used OMRS's own server.
Call LLAMACPP_CHAT_MODEL the Text model (classification) and add LLAMACPP_CLASSIFY_URL and LLAMACPP_CHAT_SERVER to the llama.cpp table. Describe the descriptor flags default_answer_model and test_checks_model_list, and the group and follows fields. State that both llama.cpp classification paths use the classification address.
…bodies Add rule 15 to the HTTP transport guide: the Chat server choice needs credentials, and while it is managed every route that writes a llama.cpp Chat choice needs credentials and the model rule. Describe the changed llama.cpp server, Chat settings, Chat test and Configuration route bodies and the 409 reasons in the transport README.
…oice Describe the Embeddings, Classification (metadata) and Chat blocks, the Chat model line, the Chat server choice with both callouts, the shared-address warning and the Local server panel without a Model row. Make Use this model follow the saved choice. Explain model-not-listed and no-model in the Chat model section, the llama.cpp judge rule, and the llama.cpp classification server item on the Integrations page. Update the llama.cpp server and Chat route rows.
…cpp card - Group provider fields and read-only values into blocks by their new `group` (Embeddings, Classification (metadata), Chat) in ProviderFieldGroups. A field that follows another says so. - Add the Chat block: the Chat model line with a link to Settings > Chat, the "Who runs the Chat server" choice with the two design D9 callouts, and the Local server panel only for OMRS's own server. - The panel loses its Model row, shows the Chat model read-only, repeats the program callout while the path is not usable, and warns when classification shares OMRS's own Chat server (design D6). - 409 reasons `external-server` and `server-running` get plain lines. - Use this model saves the llama.cpp Chat model and follows the saved choice, not the server state. - Keep the api types and fixtures in step with openapi.yaml.
- The Chat and judge forms empty the model name on a provider change, keep Save disabled and show "Choose a model for this provider.", so an Ollama name such as qwen3:4b is never saved for llama.cpp. - The Reasoning control ignores levels kept from the previous choice while no model is typed. - Add plain sentences for the Chat test reasons no-model and model-not-listed, and for the Chat status reason no-model.
Tasks 5.1 to 5.3 (ADR-108 D7). With the Chat server choice external, the llama.cpp Chat test reads /models first and gives model-not-listed for a name the server does not list, with no prompt. With managed, OMRS's own server loads the Chat model by rule, so the list read is skipped. Ticks tasks 1.1 to 5.3 in tasks.md.
Task 6.3 (ADR-108 D14, added at the maintainer's request). The Ollama descriptor sets default_answer_model=False, so an empty Ollama judge model skips the claim check and an empty Ollama Chat model refuses, instead of using metadata.ollama_classify_model with no sign. Adds D14 and D15 to the plan.
Task 6.4 (ADR-108 D15, added at the maintainer's request). PUT /v1/settings/configuration with llamacpp_server_binary now needs the credentials scope (403), a clear included, and a path that passes the ADR-084 program rule (400), as the llama.cpp card route already did.
Tasks 6.1 and 6.2 (ADR-108 D12). When the classification address differs from the Chat address, the runtime list shows a third llama.cpp item that carries the metadata user, readiness names that address when it is down, and the llama.cpp connection test probes its /models once.
Lint, typecheck, 2,236 Vitest tests and the build pass in this worktree.
…sification Task 8.1. By test-ID diff against origin/v3: 85 added, 14 removed or replaced, net +71 (_BASE_EXECUTED 9444 -> 9515); 23 of them need fastapi (_WEB_EXTRA_CASES 1786 -> 1809).
Task 8.2 review finding 2 (score 12). A Classification address save can flip shared_with_classification, so useRefreshProviders now invalidates the llama.cpp server query. Records the open-code-review session and its six findings in review.md. Ticks 8.2 and 8.4 (strict validation passes).
…-chat-and-classification Tasks 7.10 and 8.5. Classification reached the Llama app on 9931 while OMRS's own Homebrew llama-server on a scratch port answered Chat with the Chat model; the Chat test gave model-not-listed for qwen3:4b. Screenshots at 1280 px and 390 px for the managed, external and shared-address states.
Task 8.6 writes TDR-042 (browser check at fixed widths without Chrome). Task 8.7: the ADR-108 Verification section records the tests, the review, the live and browser checks, the two added fixes and the build findings. ADR-108 stays Proposed until the maintainer approves it.
Task 8.7. The maintainer approved ADR-108 on 2026-10-29.
Contributor
|
Important Review skippedToo many files! This PR contains 112 files, which is 12 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry.
 |
The floor job, which runs without fastapi, executed 4 more cases than pinned, so 19 of the 23 new route cases need fastapi, not 23.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## v3 #150 +/- ##
========================================
Coverage 94.80% 94.81%
========================================
Files 370 371 +1
Lines 30049 30203 +154
Branches 3845 3877 +32
========================================
+ Hits 28488 28637 +149
- Misses 1028 1031 +3
- Partials 533 535 +2
|
This was referenced Oct 30, 2026
This was referenced Oct 30, 2026
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.
Implements the OpenSpec change
separate-local-chat-and-classification(42/42 tasks). ADR-108 is Accepted.What changes
llamacpp_classify_url(LLAMACPP_CLASSIFY_URL). A blank value follows the Chat address, so existing installs keep working.llamacpp_chat_server:external(the Llama app, the default) ormanaged(OMRS's ownllama-server).managedloads the Chat model (answer.model).llamacpp_server_modelis gone.LLAMACPP_SERVER_MODELstops start-up and names its replacement.no-model, "Choose a judge model"). A classification model never answers or judges.external, the test reads/modelsfirst and givesmodel-not-listedfor an unknown name.llamacpp_server_binaryneedscredentialsand the program/model rule. This includes the Configuration page..env.example, upgrade notes.Release
feat:(minor), as the maintainer chose.Evidence
./scripts/local_ci.sh: Python 9,511 passed, web 2,237 passed, tripwire 5/5,openspec validate --all --strict90/90, noSKIPPED.openspec/changes/separate-local-chat-and-classification/review.md.live-checks.mdandscreenshots/(1280 px and 390 px).