Repository navigation
feat(core): showcase shared discovery for Ollama, LM Studio and vLLM - #3
ivanmilevtues wants to merge 1 commit into
Conversation
Copy the six changed-file snapshots from anomalyco/opencode PR anomalyco#53438 for an architecture-diff demo against this fork's dev. This is not a runnable backport: upstream plugin APIs and prerequisite provider registrations are absent here. Upstream source: anomalyco#53438 Original author: Kit Langton Amp-Thread-ID: https://ampcode.com/threads/T-01a13d0d-fbe1-76fa-8fad-21c3940a2399 Co-authored-by: Amp <amp@ampcode.com>
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 2 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
CodeBoarding reviewStatus: 2 changed components See the full change in CodeBoarding. graph LR
n_Core_Platform_Runtime["Core Platform #38; Runtime"]
n_Application_Client["Application Client"]
n_Management_Console["Management Console"]
n_Terminal_User_Interface["Terminal User Interface"]
n_Desktop_Shell["Desktop Shell"]
n_Infrastructure_Tooling["Infrastructure #38; Tooling"]
n_API_Protocol_Specifications["API Protocol Specifications"]
n_Effect_SQLite_Drivers["Effect SQLite Drivers"]
n_Web_Share_Viewer["Web Share Viewer"]
n_Enterprise_Portal["Enterprise Portal"]
n_Integrations_Automation["Integrations #38; Automation"]
n_Serverless_Functions["Serverless Functions"]
n_Core_Platform_Runtime -- "resolves locale utilities from" --> n_Management_Console
n_Desktop_Shell -- "detects native locale using" --> n_Application_Client
n_Infrastructure_Tooling -- "validates translation plurals against" --> n_Application_Client
classDef added fill:#1f883d,stroke:#0b5d23,color:#ffffff;
classDef modified fill:#bf8700,stroke:#7d4e00,color:#ffffff;
classDef deleted fill:#cf222e,stroke:#82071e,color:#ffffff,stroke-dasharray:5 3;
class n_Core_Platform_Runtime modified;
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 27035673b8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| : yield* client.postJson("/api/show", ShowRequest, { model: model.model }, ShowResponse) | ||
| entries.set(model.model, { digest: model.digest, info }) | ||
| return { ...model, show: info } | ||
| }).pipe(Effect.orElseSucceed(() => undefined)), |
There was a problem hiding this comment.
When an existing Ollama model receives a new digest and its /api/show request temporarily times out or returns an error, this handler converts the failure to undefined. The discovery pass therefore succeeds with that model omitted, and refresh replaces the previously loaded inventory and removes the model until a later poll succeeds, contradicting the intended behavior of retaining inventory through transient outages. Preserve the previous entry for transient per-model failures while still allowing permanent failures such as 404s to remove it.
Useful? React with 👍 / 👎.
| import { createLocalProviderPlugin } from "./local.js" | ||
|
|
||
| const RemoteModel = Schema.Struct({ | ||
| type: Schema.Literals(["llm", "embedding"]), |
There was a problem hiding this comment.
When LM Studio has a decision model installed, decoding the complete /api/v1/models response fails before the later type === "llm" filter can discard it, so no LLM inventory is refreshed. LM Studio documents decision as a valid model type returned by this endpoint; include it in the decoding union while continuing to filter it from the provider catalog. See the LM Studio model-list response fields.
Useful? React with 👍 / 👎.
| max_context_length: Schema.Int, | ||
| capabilities: Schema.Struct({ | ||
| vision: Schema.Boolean, | ||
| trained_for_tool_use: Schema.Boolean, |
There was a problem hiding this comment.
When any model in the LM Studio response has an unknown maximum context length, the documented null value is rejected by this required integer schema and the entire discovery request fails, including otherwise valid LLMs. Decode max_context_length as nullable and let the existing optional context handling retain the default limit. See the LM Studio model-list response fields.
Useful? React with 👍 / 👎.
| parent_model: Schema.String.pipe(Schema.optional), | ||
| format: Schema.String, | ||
| family: Schema.String, | ||
| families: Schema.Array(Schema.String).pipe(Schema.optional), |
There was a problem hiding this comment.
When any installed Ollama model has details.families: null, this schema rejects the complete /api/tags response, so discovery loses every otherwise valid completion model; the same schema also rejects that model's /api/show response. Ollama's API can serialize an unset family slice as null, as shown in responses reported in the official Ollama repository, so this field must accept both an array and null.
Useful? React with 👍 / 👎.
| const RemoteModel = Schema.Struct({ | ||
| id: Schema.String, | ||
| owned_by: Schema.String, | ||
| max_model_len: Schema.NullOr(Schema.Int), |
There was a problem hiding this comment.
When a configured vLLM endpoint returns a valid model card without max_model_len, this required property makes the entire /v1/models response fail schema decoding even though the mapper can safely retain its default context limit. The upstream model-card field has a default and vLLM's production-stack router can omit it from discovery responses, as documented in the vLLM production-stack report; make the property optional as well as nullable.
Useful? React with 👍 / 👎.
1 participant
Architecture diff showcase
Copies the six changed-file snapshots from anomalyco#53438 (original author: Kit Langton). This branch starts directly from this fork's dev, independently of the other demo.
What to highlight
Demo-only compatibility limits
This is a source snapshot demo, not a clean cherry-pick or runnable backport. These providers are absent from this fork's dev, so the diff here shows additions rather than the upstream extraction. Newer upstream plugin/model APIs and provider registrations are not ported. Do not merge as a production feature.
Verification
All six TypeScript files pass Bun syntax transpilation and are copied unchanged from the upstream commit. Runtime tests/typechecking are not claimed to pass: prerequisite APIs are missing. CodeBoarding's generated review is the showcase output to inspect.