Skip to content

feat(builtin-models): add YAML-based declarative config with ${ENV} interpolation#1453

Merged
lyingbug merged 1 commit into
Tencent:mainfrom
jackson-jia-914:feat/builtin-models-yaml
May 26, 2026
Merged

feat(builtin-models): add YAML-based declarative config with ${ENV} interpolation#1453
lyingbug merged 1 commit into
Tencent:mainfrom
jackson-jia-914:feat/builtin-models-yaml

Conversation

@jackson-jia-914
Copy link
Copy Markdown

Description

Today built-in models can only be added by inserting rows into the models table via SQL (see docs/BUILTIN_MODELS.md). For operators who manage WeKnora through docker-compose and .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 against os.Getenv and each entry is UPSERT-ed into the models table (is_builtin=true) by stable id. The split means operators can version-control their model topology without leaking secrets, and rotating an API key is a single .env edit + restart.

Implementation:

  • internal/types/builtin_models_config.go (new) — loader, ${ENV} interpolation (os.Getenv only; no shell-style defaults), and per-entry GORM clause.OnConflict { Columns: id, UpdateAll: true } upsert. created_at is 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 — calls LoadBuiltinModelsConfig once, 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.goBeforeCreate now generates a UUID only when ID == "", 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 — new ConfigDir() helper exposes the directory of the loaded config.yaml so the builtin-models loader resolves builtin_models.yaml next to it without re-implementing viper's search rules. Overridable with BUILTIN_MODELS_CONFIG=/abs/path.
  • docker-compose.yml — adds env_file: [{ path: .env, required: false }] so variables referenced by ${ENV} in YAML reach the container automatically without listing each one under environment:. required: false keeps fresh clones working with no .env file. Also adds a commented-out volume mount line for config/builtin_models.yaml as 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 at builtin_models.yaml.example and 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.yaml mounted, behavior is identical to before — the loader logs a single "skipping" line and returns.

Type of Change

  • ✨ New feature

Testing

  • Start without mounting config/builtin_models.yaml → log shows Built-in models config not present ... skipping.; existing SQL-seeded built-in models continue to work.
  • Mount a YAML with three entries (LLM / Embedding / Rerank) referencing ${ENV} vars set in .env → on startup the three rows appear in models with is_builtin=true and the resolved values; restart is idempotent (same rows, no duplicates).
  • Set an entry's env var to an unset name → the literal ${VAR_NAME} is persisted, and the resulting 401/connection error from the provider points clearly at the unset variable.
  • Feed deliberately malformed YAML → startup logs a parse-error warning and continues without aborting.

Checklist

  • Self-reviewed the code
  • Added/updated tests covering the change
  • Updated related documentation (docs/BUILTIN_MODELS.md, new config/builtin_models.yaml.example, .env.example)
  • Breaking changes are clearly called out in the description above

Screenshots / Recordings

N/A — backend / config-only change.

…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.
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 lyingbug merged commit d439b3a into Tencent:main May 26, 2026
1 check passed
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants