Skip to content

docs(admin): document the provider config shape that actually exists (#438) - #440

Open
pjdoland wants to merge 2 commits into
plmbr:mainfrom
pjdoland:docs/438-provider-config-shape
Open

pjdoland wants to merge 2 commits into
plmbr:mainfrom
pjdoland:docs/438-provider-config-shape

Conversation

@pjdoland

@pjdoland pjdoland commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

Summary

The admin guide's self-hosted LLM sections told operators to configure endpoints with a top-level providers block, and the air-gapped section added a default_provider key. Nothing reads either one. An admin who followed the guide wrote a config.json that was silently ignored, NBI ran on defaults, and the symptom (no models, or requests going to the wrong upstream) surfaced a long way from the cause.

Verified before rewriting anything:

  • default_provider, default_chat_model and default_inline_completion_model appear only in docs/admin-guide.md, with zero references in notebook_intelligence/ or src/.
  • No code reads a key named providers. The 42 substring matches are all other identifiers (llm_providers, disabled_providers, allowed_context_providers, allow_enabling_providers_with_env, and so on).
  • NBIConfig's accessors cover chat_model, inline_completion_model, claude_settings, acp_settings, mcp_server_settings and friends. Nothing of the documented shape exists.

Solution

The four examples (Azure, vLLM/TGI, LiteLLM proxy, Ollama) now use the real surface. Endpoint settings are per-provider LLMProviderProperty values persisted inside the model selection as {id, value} pairs, which ai_service_manager.py applies to the selected model at startup.

The old text was wrong three ways simultaneously: the outer key, the inner keys, and the model field, which for these two providers is a fixed placeholder id (openai-compatible-chat-model) because the real upstream model name belongs in the model_id property.

Ollama needed different treatment rather than a corrected JSON block. That provider declares no properties at all, so there is no base_url to set and writing one has no effect; a remote host comes from OLLAMA_HOST in the server's environment, which the client reads directly (_parse_host(host or os.getenv('OLLAMA_HOST'))). Its autocomplete ids also come from a fixed list in the provider, where the old example's codellama:7b should have been codellama:7b-code.

Testing

prettier clean on all tracked files including this one, tsc --noEmit clean, jest 423 passed, pytest 1753 passed. Documentation-only; no code changed.

Both intra-document anchors were checked against the real headings (## API-key handling at :167, ## Configuration readiness at :792) rather than inferred from section names.

Review

Two reviewers plus my own pass. A technical-accuracy reviewer verified every factual claim in the new text against the code: the {id, value} shape, all four placeholder model ids, model_id as a real declared property, and all four Ollama claims including the :7b-code suffix. All were reported verified. An admin-experience reviewer blocked on two items, both now fixed:

  • The examples contradicted the API-key handling section two above them, which promises env-var fallback. That promise does not hold for openai-compatible: the provider forwards an empty string and the OpenAI SDK only consults the environment when the key is None, so the fallback is skipped and the request fails on a missing credential. Note the asymmetry: base_url gets an empty-to-None coercion two lines earlier and api_key does not. The list now states this and cross-references that section. The code fix that would make the original promise true is fix(providers): openai-compatible sends an empty api_key, so the documented env-var fallback never happens #439.
  • Nothing told operators how to tell whether the file took effect, which is the same silent-failure class this change exists to fix. Added a pointer to the readiness endpoint and the Status card, which already report whether a provider and model are selected and whether the endpoint serves the configured id.

Also taken from the review: the dropdown shows the placeholder model id (so an admin sanity-checking their work does not read it as an ignored config); context_window deserves a callout because when unset NBI skips chat-history pruning entirely rather than assuming a default, so a small self-hosted window overflows; the OLLAMA_HOST requirement is now a blockquote callout matching how the guide flags other easy-to-miss requirements; and the placeholder secrets are obviously fake rather than plausible key prefixes.

Risks and follow-ups

  • The JSON is more verbose than what it replaced. Three of the examples are now 12-19 lines of near-identical scaffolding around three differing values, which makes the Azure/vLLM/LiteLLM comparison harder to eyeball than the old side-by-side blocks. That is a consequence of the real shape being nested; a reference block plus a table of the three per-provider values would restore some of it, and is worth considering separately.
  • The api_key guidance is accurate but unsatisfying. It tells operators the key must live in the file and that keeping it out means templating or mounting. If fix(providers): openai-compatible sends an empty api_key, so the documented env-var fallback never happens #439 lands, this paragraph should be revisited to describe the working fallback instead.
  • litellm-compatible was not traced end to end for the same empty-key behaviour. Its upstream uses api_key or get_secret(...) patterns where a falsy value does chain to a lookup, so it may already behave as the earlier section promises. Noted in fix(providers): openai-compatible sends an empty api_key, so the documented env-var fallback never happens #439.
  • Review depth: two Sonnet reviewers and my own pass, no third perspective.

Closes #438

…lmbr#438)

The self-hosted LLM sections told operators to configure endpoints with a
top-level `providers` block, and the air-gapped section added a
`default_provider` key. Nothing reads either one. An admin following the
guide wrote a config.json that was silently ignored and NBI ran on
defaults, so the symptom (no models, or requests going to the wrong
upstream) surfaced a long way from the cause.

Verified before rewriting: `default_provider`, `default_chat_model` and
`default_inline_completion_model` appear only in this file, with no
references in notebook_intelligence/ or src/; no code reads a key named
`providers`; and NBIConfig's accessors cover chat_model,
inline_completion_model, claude_settings and friends with nothing of that
shape.

The four examples (Azure, vLLM/TGI, LiteLLM, Ollama) now use the real
surface. Endpoint settings are per-provider LLMProviderProperty values
persisted inside the model selection as {id, value} pairs, which
ai_service_manager applies to the selected model at startup. The old text
was wrong three ways at once: the outer key, the inner keys, and the
`model` field, which for these two providers is a fixed placeholder id
because the upstream model name belongs in the `model_id` property.

Ollama needed different treatment rather than a corrected JSON block. That
provider declares no properties at all, so there is no `base_url` to set
and writing one has no effect; a remote host comes from OLLAMA_HOST in the
server's environment, which the client reads directly. Its autocomplete
ids also come from a fixed list in the provider, where the old example's
`codellama:7b` should have been `codellama:7b-code`.

Review remediation, from a technical-accuracy pass over every claim and an
admin-experience pass over the instructions:

- The examples contradicted the API-key handling section two above them,
  which promises env-var fallback. That promise does not hold for
  openai-compatible: the provider forwards an empty string and the OpenAI
  SDK only consults the environment when no key was supplied, so the
  fallback is skipped. The list now says so and cross-references that
  section; the code fix that would make the promise true is plmbr#439.
- Nothing told operators how to tell whether the file took effect, which
  is the same silent-failure class this change exists to fix. Added a
  pointer to the readiness endpoint and the Status card, which already
  report exactly that.
- Called out that the dropdown shows the placeholder model id, so an admin
  sanity-checking their work does not read it as a config that was ignored.
- Called out `context_window`: when unset, NBI skips chat-history pruning
  entirely rather than assuming a default, so a small self-hosted window
  overflows.
- Promoted the OLLAMA_HOST note to a blockquote callout, matching how the
  guide flags other easy-to-miss requirements, and made the placeholder
  secrets obviously fake rather than plausible key prefixes.
@pjdoland

Copy link
Copy Markdown
Collaborator Author

The check_release failure on this PR is unrelated to the change, and a re-run should clear it. I do not have permission to re-run workflows on this repository, so that needs a maintainer.

This PR only edits markdown, so it cannot affect dependency resolution. The failure comes from the release build: jupyter_builder derives a @jupyterlab/core-meta range of 4.5.x from the legacy @jupyterlab/builder devDependency, and no 4.5.x release of core-meta exists on npm (13 versions are published and the earliest is 4.6.0-alpha.4). The build falls back to fetching from the jupyterlab/jupyterlab GitHub repository, and that fallback timed out:

UserWarning: TimeoutError: The read operation timed out
subprocess.CalledProcessError: Command jlpm run build:prod returned non-zero exit status 1

The same job on #444 failed in the same window with HTTP Error 403: rate limit exceeded, and #443 passed check_release within that same hour, so this looks like network flakiness in that fallback rather than anything in either diff.

A durable fix would be moving the devDependency from @jupyterlab/builder to @jupyter/builder, which the build warning recommends. That is a build toolchain change, so it is better off as its own PR.

@pjdoland
pjdoland requested a review from mbektas September 14, 2026 18:18
The previous check_release run failed on a transient network timeout
during the frontend build, unrelated to this change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docs(admin): the admin guide documents a providers config block that nothing reads

1 participant