feat: provider modules — ProviderProfile ABC, 29 providers, fetch_models, transport single-path (salvage #14424)#16326
Open
teknium1 wants to merge 2 commits into
Open
feat: provider modules — ProviderProfile ABC, 29 providers, fetch_models, transport single-path (salvage #14424)#16326teknium1 wants to merge 2 commits into
teknium1 wants to merge 2 commits into
Conversation
Cycle 2 PR 1 (#14418). Introduces providers/ package with ProviderProfile ABC and auto-discovery registry, then wires ChatCompletionsTransport to delegate to profiles via a clean single-path method. Provider profiles (8 providers): - nvidia: default_max_tokens=16384 - kimi + kimi-cn: OMIT_TEMPERATURE, thinking + top-level reasoning_effort - openrouter: provider_preferences, full reasoning_config passthrough - nous: product tags, reasoning with Nous-specific disabled omission - deepseek: base_url + env_vars - qwen-oauth: vl_high_resolution extra_body, metadata top-level api_kwargs Transport integration: - _build_kwargs_from_profile() replaces the entire legacy flag-based assembly when provider_profile param is passed - Single path: no dual-execution, no overwrites, no legacy fallthrough - build_api_kwargs_extras() returns (extra_body, top_level) tuple to handle Kimi's top-level reasoning_effort vs OpenRouter's extra_body Auth types: api_key | oauth_device_code | oauth_external | copilot | aws (expanded from the lossy 'oauth' to match real Hermes auth modes). 64 new tests: - 30 profile unit tests (registry, all 8 profiles, auth types) - 19 transport parity tests (pin legacy flag-based behavior) - 15 profile wiring tests (verify profile path = legacy path)
…apter.copilot_client Main added two HOME-handling tests (test_run_prompt_prefers_profile_home_when_available, test_run_prompt_passes_home_when_parent_env_is_clean) after PR #14424 was written. These patch 'agent.copilot_acp_client.subprocess.Popen', but the shim module no longer has 'subprocess' imported. Update patch strings to target the real module location. Follow-up commit on the salvage PR; kshitijk4poor's original commit is preserved above.
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.
Salvage of #14424 onto current main.
Summary
Introduces
providers/as a canonical registry for all inference providers Hermes knows about, behind aProviderProfiledataclass + 4 overridable hooks. Wires NVIDIA + DeepSeek through a new profile-driven path inChatCompletionsTransport.build_kwargs, gated by_PROFILE_ACTIVE_PROVIDERSinrun_agent.py. Legacy flag-based dispatch untouched for the other 27 providers. Also moves the Copilot ACP client fromagent/copilot_acp_client.py→acp_adapter/copilot_client.py(7-line shim left behind for backward compat).Cycle 2 PR 1 of #14418. Remaining 27 providers get incrementally wired in #14515+.
Changes
providers/— 29 provider modules +base.py+README.mdacp_adapter/copilot_client.py— copied verbatim from main's currentagent/copilot_acp_client.py(includes_resolve_home_dir/_build_subprocess_envdrift that landed after PR author's base commit)agent/copilot_acp_client.py→ 7-line re-export shimagent/transports/chat_completions.py—_build_kwargs_from_profile()early-return branch; legacy path untouchedrun_agent.py— 43 lines (profile allowlist + dispatch block); removes obsolete_is_nvidia/is_nvidia_nimflag threadingpyproject.toml— includeproviders+providers.*in packages findtests/providers/(4 files, 79 tests — profile declarations, transport parity, profile-path E2E)Conflict resolution notes
Three conflicts resolved against current main (branch was 537 commits behind):
agent/transports/__init__.py— main already shipped a stricter fix for the same "registry partially populated when test imports transport directly" problem the PR was trying to solve (main discovers on every miss; PR regressed to discover-once behind a_discoveredflag). Kept main's version, dropped the PR's regression.agent/copilot_acp_client.py— main had drifted 606 lines in this file (added_resolve_home_dir,_build_subprocess_env,HOMEenv threading, formatting, a stricter stdin/stdout guard). Used main's current file contents as the source-of-truth for the newacp_adapter/copilot_client.pylocation to preserve all of that drift. The shim at the old location is unchanged.uv.lock— trivial regeneration drift; kept main's lockfile since the PR doesn't add new dependencies.Follow-up fix commit (by us)
fix(tests): update copilot_acp subprocess.Popen patch paths— main added two HOME-handling tests (test_run_prompt_prefers_profile_home_when_available,test_run_prompt_passes_home_when_parent_env_is_clean) after the PR was written. They patchagent.copilot_acp_client.subprocess.Popen, but the shim module no longer hassubprocessimported. Updated toacp_adapter.copilot_client.subprocess.Popen.Validation
scripts/run_tests.sh tests/providers/scripts/run_tests.sh tests/agent/scripts/run_tests.sh tests/run_agent/test_tool_arg_coercion.py::test_inf_stays_string_for_integer_only, unrelated)get_provider_profile()build_kwargsoutputextra_body.tags=['product=hermes-agent']attributionreasoning_efforttop-level +extra_body.thinking.type='enabled',temperature=None(fixed-temp)extra_body.providerpreferences_PROFILE_ACTIVE_PROVIDERSwiring present inrun_agent.py, dispatch block early-returns for nvidia/deepseekCredit
Original PR by @kshitijk4poor. Commit 1 (
feat: add provider modules + wire transport single-path) authored by kshitijk4poor and preserved via rebase-merge. Commit 2 (test patch follow-up) is ours.Closes part of #14418. Supersedes #14424.