feat(builtin-models): add YAML-based declarative config with ${ENV} interpolation#1453
Merged
Merged
Conversation
…nterpolation
Allow built-in models to be declared in config/builtin_models.yaml
instead of inserting rows via SQL. On every startup the file is read
and each entry is UPSERT-ed into the models table (is_builtin=true)
by stable id.
Any string field may reference an environment variable with ${NAME}.
Unset variables are left as the literal placeholder so
misconfiguration surfaces clearly in provider calls rather than
failing silently with an empty token.
The file is optional: missing file, parse errors, and per-entry
upsert failures all log a warning without aborting startup.
docker-compose.yml adds env_file (.env, required:false) so
deployment-specific variables are passed through automatically.
7 tasks
lyingbug
added a commit
to lyingbug/WeKnora
that referenced
this pull request
May 26, 2026
…efix Original PR Tencent#1453 used fmt.Printf, which lands as unstructured noise in release/JSON log pipelines. The natural fix is logger.Infof/Warnf, but internal/logger itself imports internal/types — using it from here would create an import cycle (this is the same constraint that forces model.go's crypto error logs onto stdlib log). Switch all loader output to log.Printf with a stable "[builtin-models]" prefix (mirroring the "[crypto]" convention already established in model.go). The prefix gives operators a grep handle even though the lines stay unstructured. Levels are encoded inline as "WARN:" for warnings so log shippers and humans can still discriminate.
lyingbug
added a commit
that referenced
this pull request
May 26, 2026
Introduce a `managed_by` varchar column on `models` so future declarative
loaders can claim ownership of a subset of rows without disturbing entries
created via the UI/API or seeded by hand-written SQL.
- versioned/000052_models_managed_by.{up,down}.sql add the column with a
default of '' and a partial index on non-empty values to keep startup
reconciliation cheap.
- sqlite/000000_init.up.sql is updated in place (the Lite init migration
is a single file per project convention) so fresh SQLite databases get
the column from the start.
- Model.ManagedBy mirrors the column. Existing rows default to '' which
the YAML loader treats as "manually managed, never touch".
Schema half of the YAML-driven built-in-model lifecycle work that follows
up on #1453; the reconciler that uses the column lands in the next commit.
lyingbug
added a commit
that referenced
this pull request
May 26, 2026
PR #1453 introduced the map form of env_file: env_file: - path: .env required: false which is only recognised by Docker Compose v2.24+ (Jan 2024). Older deploys would refuse to parse docker-compose.yml even when builtin_models YAML is not enabled — breaking the "no impact unless opted in" promise. Switch to the array form (`env_file: [.env]`) which has been supported since the earliest Compose schemas. The trade-off is that Compose now errors when .env is absent, so: - scripts/start_all.sh already calls check_env_file (cp .env.example .env if missing) before docker compose up; that path is unaffected. - The bare `make docker-run` / `make docker-restart` targets in the Makefile are taught the same fallback: touch / copy .env.example before invoking docker-compose, so fresh clones keep working. The docker-compose.yml comment block is updated to explain the version trade-off so future maintainers don't re-introduce the map form.
lyingbug
added a commit
that referenced
this pull request
May 26, 2026
…efix Original PR #1453 used fmt.Printf, which lands as unstructured noise in release/JSON log pipelines. The natural fix is logger.Infof/Warnf, but internal/logger itself imports internal/types — using it from here would create an import cycle (this is the same constraint that forces model.go's crypto error logs onto stdlib log). Switch all loader output to log.Printf with a stable "[builtin-models]" prefix (mirroring the "[crypto]" convention already established in model.go). The prefix gives operators a grep handle even though the lines stay unstructured. Levels are encoded inline as "WARN:" for warnings so log shippers and humans can still discriminate.
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.
Description
Today built-in models can only be added by inserting rows into the
modelstable via SQL (seedocs/BUILTIN_MODELS.md). For operators who manage WeKnora throughdocker-composeand.env, this means hand-crafting SQL with secrets inlined and re-running it on every deployment.This PR adds a declarative alternative built around a deliberate split between two files:
config/builtin_models.yaml(committable) declares the shape of the built-in models —id,type,provider, and which env vars feed which fields..env(gitignored) provides the values — API keys, base URLs, deployment-specific model names.Any string field in YAML may reference an env var with
${NAME}. On startup the YAML is interpolated againstos.Getenvand each entry is UPSERT-ed into themodelstable (is_builtin=true) by stableid. The split means operators can version-control their model topology without leaking secrets, and rotating an API key is a single.envedit + restart.Implementation:
internal/types/builtin_models_config.go(new) — loader,${ENV}interpolation (os.Getenvonly; no shell-style defaults), and per-entry GORMclause.OnConflict { Columns: id, UpdateAll: true }upsert.created_atis preserved on conflict; every other field refreshes. Unset env vars are intentionally left as the literal${NAME}placeholder so misconfiguration surfaces visibly in provider calls instead of failing silently with an empty token.internal/container/container.go— callsLoadBuiltinModelsConfigonce, post-migration. The loader treats file-missing / is-a-directory / parse error / per-entry upsert error all as warnings; startup never aborts because of YAML.internal/types/model.go—BeforeCreatenow generates a UUID only whenID == "", so YAML-declared stable ids are preserved as the UPSERT key. API-driven model creation (which never sets ID) is unaffected.internal/config/config.go— newConfigDir()helper exposes the directory of the loadedconfig.yamlso the builtin-models loader resolvesbuiltin_models.yamlnext to it without re-implementing viper's search rules. Overridable withBUILTIN_MODELS_CONFIG=/abs/path.docker-compose.yml— addsenv_file: [{ path: .env, required: false }]so variables referenced by${ENV}in YAML reach the container automatically without listing each one underenvironment:.required: falsekeeps fresh clones working with no.envfile. Also adds a commented-out volume mount line forconfig/builtin_models.yamlas an opt-in hint.config/builtin_models.yaml.example(new) — generic template with inline documentation, covering both${ENV}-driven and literal-value styles. The non-example path (config/builtin_models.yaml) is intentionally NOT shipped; operators copy the example and edit..env.example— adds a commented "Built-in Models" section pointing atbuiltin_models.yaml.exampleand showing a reference set of LLM / Embedding / Rerank variable names. The block is comment-only so deployments that don't enable YAML are unaffected, and the wording calls out that the variable names are operator-defined (the YAML decides which names matter), not reserved keys.docs/BUILTIN_MODELS.md— rewritten with YAML as the recommended path, SQL retained as an alternative.The feature is fully opt-in. Without
config/builtin_models.yamlmounted, behavior is identical to before — the loader logs a single "skipping" line and returns.Type of Change
Testing
config/builtin_models.yaml→ log showsBuilt-in models config not present ... skipping.; existing SQL-seeded built-in models continue to work.${ENV}vars set in.env→ on startup the three rows appear inmodelswithis_builtin=trueand the resolved values; restart is idempotent (same rows, no duplicates).${VAR_NAME}is persisted, and the resulting 401/connection error from the provider points clearly at the unset variable.Checklist
docs/BUILTIN_MODELS.md, newconfig/builtin_models.yaml.example,.env.example)Screenshots / Recordings
N/A — backend / config-only change.