test: generic validator for declarative providers - #10010
Merged
Conversation
added 2 commits
June 25, 2026 10:28
The declarative provider tests hand-asserted specific model names, model counts, and orderings from each bundled provider JSON. Adding or reordering a model broke unrelated structural tests (see #9967). Replace the per-provider deserialize tests with a single test_all_bundled_providers_are_valid that walks every bundled provider and checks the invariants that actually matter: it parses, has a valid and unique id, a non-empty base_url, static models when dynamic_models is disabled, and a declared env_var for every ${VAR} placeholder in base_url/base_path/headers. Also drop the brittle default_model assertions from the registry-wiring tests, keeping their structural config-key checks.
The per-provider registry-wiring tests (tanzu/nvidia/nearai/alibaba) each restated one provider's display_name, doc link, and config-key names. Replace them with test_bundled_providers_wire_into_registry_metadata, which registers every bundled declarative provider and asserts the wiring contract that matters: it registers as Declarative, never leaks the OpenAI engine's OPENAI_HOST/OPENAI_BASE_PATH keys, and turns the JSON api_key_env + env_vars into config keys with the right secret/required flags. Keep the huggingface (Preferred) and openai-compatible config-key tests, which exercise different (non-declarative) registration paths.
jamadeo
approved these changes
Jun 25, 2026
DOsinga
enabled auto-merge
July 1, 2026 02:58
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d99c06fa06
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+870
to
+871
| let mut registry = crate::providers::provider_registry::ProviderRegistry::new(None); | ||
| register_declarative_provider(&mut registry, config, ProviderType::Declarative); |
There was a problem hiding this comment.
Because this replacement test registers each bundled provider directly into a fresh ProviderRegistry, it no longer covers the init_registry/providers().await path that the application uses. With the deleted init.rs registry-wiring tests gone, removing or breaking the register_declarative_providers call from initialization would still pass here while every bundled declarative provider disappears from the app; keep at least one assertion against the global registry or register through that path.
Useful? React with 👍 / 👎.
lifeizhou-ap
added a commit
that referenced
this pull request
Jul 1, 2026
* main: (31 commits) test: generic validator for declarative providers (#10010) UI acp migratoin: Decouple desktop UI types from generated OpenAPI types (Part 2) (#10149) Remove MCP sampling support (#10087) Support TLS for ACP serve (#10088) feat (ui): Migrate dictation local model manager to ACP (#10131) Fix MCP app sandbox bridge lifecycle (#10064) fix(bedrock): send inference config (max_tokens, temperature) on Converse (#9889) feat(providers): support OpenRouter request parameters (#9276) Migrate local inference model management to ACP (#10124) (attempt to) fix disk space errors in linux release builds (#10024) feat: add --edit session flag to edit conversation before forking (#9799) feat: add iFlytek Spark and Astron MaaS providers (#9837) fix(desktop): dedupe Nostr session deep link imports (#9918) [codex] Add SessionStart hook parity outside CLI (#9970) feat(providers): add Fireworks AI declarative provider (#9990) fix(providers): don't retry deterministically-permanent 400s (thinking-block immutability) (#10005) fix(deps): downgrade pkcs8 to v0.10 to match sec1/pkcs1 v0.7 (#10119) chore(deps): bump actions/cache from 5.0.2 to 6.0.0 (#10051) Make OpenAI Responses API store param configurable (#10040) remove unsupported model (#10121) ...
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
2 participants
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.
What
Replaces the hand-maintained per-provider declarative tests with a single generic validator,
test_all_bundled_providers_are_valid.Why
The previous tests asserted specific model names, model counts and orderings from each bundled provider JSON (e.g.
config.models[0].name == "zai-org/GLM-5.1-FP8"). Adding or reordering a model in a provider's JSON broke unrelated structural tests — most recently in #9967, where adding GLM-5.2 broke the nearai test. That's noise, not signal.What the generic test checks
For every bundled provider JSON it asserts the invariants that actually matter:
deserialize_provider_config, including thepreserves_thinkingdefault)validate_provider_id) that is unique across all bundled providersbase_urlmodelswhendynamic_modelsis disabled (otherwise construction fails at runtime)env_varfor every${VAR}placeholder inbase_url,base_pathandheadersThe placeholder/env_var check catches a real misconfiguration class (a placeholder with no backing env_var would fail expansion at runtime) — verified by temporarily injecting a bad placeholder, which the test correctly fails on.
Also drops the brittle
default_modelassertions from the registry-wiring tests ininit.rswhile keeping their structural config-key checks.Net: -274/+68 lines.