Build a provider-aware config surface for local vs. external models - #17
Merged
Merged
Conversation
Resolves #11. Depends on #9 (Manifold.Model behaviour) and #10 (the constrained-decoding degrade-strategy ADR), both already merged. config :manifold, :model is now {module, opts} instead of a bare module: - config/config.exs nests model_path/llama_host/llama_port under the Manifold.Llama.Client tuple entry instead of as flat top-level keys. - Manifold.Model gains opts/0 alongside impl/0, and lenient parsing so a bare module atom (as tests already used) still works with empty opts. - Manifold.Llama.Server sources its host/port/model_path from Manifold.Model.opts() when Manifold.Llama.Client is the active backend, and parks in :no_model (as it already does for a missing file) when a different backend is configured, rather than guessing at config that belongs to a provider it has no part in. - config/runtime.exs adds MANIFOLD_MODEL_PROVIDER (default "llama") to select the backend module from a known_providers map, gated the same way local vs. external opts are built: the "llama" branch keeps MANIFOLD_MODEL's existing meaning (a GGUF path) and adds MANIFOLD_LLAMA_HOST; any other branch reads MANIFOLD_API_KEY and MANIFOLD_MODEL_NAME into opts ahead of a backend actually landing. Selecting an unrecognized provider is a boot-time error, not a silent fall back to local, so a shell with real credentials set finds out immediately if they went unused. - config/test.exs nests its hermetic model_path under the same tuple shape and documents why the existing config_env() != :test guard in runtime.exs already extends the "never touches real credentials" guarantee to the external branch: there's no external Manifold.Model implementation in this codebase yet to test against (see docs/adr/0001-external-model-decoding-strategy.md), so the note is a placeholder for the day one lands. Verified the config chain (config.exs -> test.exs / runtime.exs, with and without MANIFOLD_MODEL_PROVIDER/credentials in the environment) by evaluating it directly with Config.Reader, since this sandbox's egress policy blocks hex.pm and mix deps.get/mix test cannot run here. Updates README's key-modules table and adds a "Provider config" section documenting the new env vars and the provider-selection table. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H2do9pteuSjUyqzPPUBdnE
model_test.exs asserts opts/0 returns all three of model_path, llama_host, and llama_port for the default backend, matching config/config.exs's shape. config/test.exs's hermetic override only set model_path, so the assertion failed in CI. Llama.Server parks in :no_model on the missing model_path before it ever reads host/port, so adding them here costs nothing and keeps test.exs's opts shape identical to config.exs's. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KfPiLSwPSN7bcdoYtPvCYy
Owner
Author
|
Fixed the failing CI is now green and the PR is mergeable with no open review threads. Generated by Claude Code |
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.
Resolves #11. Depends on #9 (Manifold.Model behaviour) and #10 (the
constrained-decoding degrade-strategy ADR), both already merged.
config :manifold, :model is now {module, opts} instead of a bare module:
Manifold.Llama.Client tuple entry instead of as flat top-level keys.
bare module atom (as tests already used) still works with empty opts.
Manifold.Model.opts() when Manifold.Llama.Client is the active backend,
and parks in :no_model (as it already does for a missing file) when a
different backend is configured, rather than guessing at config that
belongs to a provider it has no part in.
select the backend module from a known_providers map, gated the same
way local vs. external opts are built: the "llama" branch keeps
MANIFOLD_MODEL's existing meaning (a GGUF path) and adds
MANIFOLD_LLAMA_HOST; any other branch reads MANIFOLD_API_KEY and
MANIFOLD_MODEL_NAME into opts ahead of a backend actually landing.
Selecting an unrecognized provider is a boot-time error, not a silent
fall back to local, so a shell with real credentials set finds out
immediately if they went unused.
shape and documents why the existing config_env() != :test guard in
runtime.exs already extends the "never touches real credentials"
guarantee to the external branch: there's no external Manifold.Model
implementation in this codebase yet to test against (see
docs/adr/0001-external-model-decoding-strategy.md), so the note is a
placeholder for the day one lands.
Verified the config chain (config.exs -> test.exs / runtime.exs, with and
without MANIFOLD_MODEL_PROVIDER/credentials in the environment) by
evaluating it directly with Config.Reader, since this sandbox's egress
policy blocks hex.pm and mix deps.get/mix test cannot run here.
Updates README's key-modules table and adds a "Provider config" section
documenting the new env vars and the provider-selection table.
Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01H2do9pteuSjUyqzPPUBdnE