Skip to content

Document MODELS_ENABLED_BY_DEFAULT availability default - #114

Draft
weselben wants to merge 2 commits into
mainfrom
docs/models-enabled-by-default
Draft

weselben wants to merge 2 commits into
mainfrom
docs/models-enabled-by-default

Conversation

@weselben

@weselben weselben commented Oct 1, 2026

Copy link
Copy Markdown
Owner

TL;DR

MODELS_ENABLED_BY_DEFAULT exists in config/models.go, .env.template, and
config.example.yaml, but no narrative docs page mentions it. This PR
documents the availability default on the Virtual Models page and in the
advanced configuration reference, including the contrast with the listing-only
KEEP_ONLY_ALIASES_AT_MODELS_ENDPOINT.


This PR description was generated with AI assistance.

The flag shipped in the config struct and both config templates without any narrative docs. Document the availability default on the virtual models page and in the advanced configuration reference, contrasted with the listing-only alias flag.
@coderabbitai

coderabbitai Bot commented Oct 1, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

A redirect resolves by its own enabled flag, but the resolved target is validated like a direct call. With the availability default off, the target needs its own access entry.
@weselben

weselben commented Oct 3, 2026

Copy link
Copy Markdown
Owner Author

@forge-orion review

@forge-orion

Copy link
Copy Markdown

Walkthrough

This PR closes a documentation gap: MODELS_ENABLED_BY_DEFAULT is wired through config/models.go, .env.template, and config.example.yaml, but no narrative docs page explained it. Two .mdx pages now carry the story.

docs/advanced/configuration.mdx

Adds a paragraph under the provider-models section. States the default (true), the YAML key (models.enabled_by_default), the failure mode (400 model_access_denied), the listing effect (models drop out of GET /v1/models), and contrasts with KEEP_ONLY_ALIASES_AT_MODELS_ENDPOINT (listing-only, not an availability gate). Cross-links to the new section in virtual-models.mdx.

docs/features/virtual-models.mdx

Adds a new ## Model availability by default section. Explains the default, the env var to flip it, the semantics of an entry with empty targets (pure access rule), the redirect-vs-target interaction under the flipped default, and the contrast with the adjacent ## Expose only redirects section.

Correctness check

Verified against the code:

  • config/config.go:135 sets EnabledByDefault: true — matches "Every provider model is available by default."
  • internal/virtualmodels/authorize.go:49-53 returns 400 with code model_access_denied — matches the documented error.
  • internal/server/models_endpoint.go:38 applies FilterPublicModels — matches "stay out of GET /v1/models."
  • internal/virtualmodels/types.go:5-11 — "A row without Targets is an access rule" matches "empty targets is enough."
  • Redirect resolution uses its own enabled flag and the target goes through the same AllowsModel path — matches "give the target model its own access entry too."

All documented claims line up with the implementation. No drift between env name, YAML key, default, or error code.

CI

All 14 checks pass (Analyze, Build, CodeQL, Dashboard, Docs Validation, E2E Tests, Integration Tests, Unit Tests, Vulnerability Scan, lint, Performance Guard; CodeRabbit skipped because draft; semantic-pr-title skipping).

Nits

  • "With the default off" in the new section reads slightly ambiguous — "With the default flipped to false" would be unambiguous.
  • The paragraph in configuration.mdx is dense; splitting the KEEP_ONLY_ALIASES sentence into its own paragraph would improve scan-ability.

Neither blocks merge.

Mergeability Score: 9/10 — Approve (draft)

Dimension Rating
Correctness 10/10 — every claim verified against source
Completeness 9/10 — covers both docs surfaces and the contrast with the listing-only flag
Risk 10/10 — docs-only, zero behavior change
Tests 9/10 — no test needed for docs; existing config test at config/config_test.go:1021 exercises the env var
Docs quality 9/10 — clear, cross-linked; minor wording nit
CI 10/10 — all checks green
Scope discipline 10/10 — exactly the gap the PR claims to close, nothing else
Convention 8/10 — title follows docs: conventional-commit form; draft state is appropriate for a docs pass

@forge-orion forge-orion left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inline review

Line-level findings on the diff:

docs/features/virtual-models.mdx:311-313

With the default off, a provider model stays unavailable until an entry matches it. An entry with empty targets is enough — it acts as a pure access rule (enabled: true, optionally with user_paths).

Suggestion: "With the default flipped to false," removes ambiguity vs. "off" (which could also describe a disabled redirect).

docs/advanced/configuration.mdx:477-479
The KEEP_ONLY_ALIASES_AT_MODELS_ENDPOINT sentence and the sentence above it are one dense paragraph. Splitting after the 400 model_access_denied sentence would let the listing-only contrast stand on its own and improve scan-ability.

No correctness findings. Documented env names, YAML keys, defaults, and error codes all verified against source. See the walkthrough comment for the line-by-line verification.

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