feat: combine ProviderSpec datatypes #3378

cdoern · 2025-09-08T19:55:05Z

What does this PR do?

currently RemoteProviderSpec has an AdapterSpec embedded in it. Remove AdapterSpec, and put its leftover fields into RemoteProviderSpec.

Additionally, many of the fields were duplicated between InlineProviderSpec and RemoteProviderSpec. Move these to ProviderSpec so they are shared.

Fixup the distro codegen to use RemoteProviderSpec directly rather than remote_provider_spec which took an AdapterSpec and returned a full provider spec

Test Plan

existing distro tests should pass.

cdoern · 2025-09-10T21:13:15Z

llama_stack/core/datatypes.py

-    @property
-    def pip_packages(self) -> list[str]:
-        raise AssertionError("Should not be called on AutoRoutedProviderSpec")
+    pip_packages: list[str] = Field(


this, seems like ... a separate issue but one I had to wrangle with to get this working.

actually, let me try just removing this?

Copilot

Pull Request Overview

This PR consolidates provider specification data structures by removing the AdapterSpec class and moving its fields directly into RemoteProviderSpec. Additionally, it promotes shared fields between InlineProviderSpec and RemoteProviderSpec to their common base class ProviderSpec to eliminate duplication.

Eliminates the AdapterSpec class and inlines its fields into RemoteProviderSpec
Moves shared fields (pip_packages, provider_data_validator) to ProviderSpec base class
Updates provider registry files to use RemoteProviderSpec constructor directly
Replaces remote_provider_spec factory function with direct RemoteProviderSpec usage

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
llama_stack/providers/datatypes.py	Removes `AdapterSpec` class, adds shared fields to `ProviderSpec`, removes `remote_provider_spec` function
llama_stack/core/distribution.py	Updates provider spec loading to use `RemoteProviderSpec` directly
llama_stack/providers/registry/*.py	Updates all provider registries to use new `RemoteProviderSpec` constructor
tests/unit/distribution/test_distribution.py	Updates test assertions and YAML specs to match new structure
tests/external/	Updates external provider configurations
llama_stack/distributions/starter/starter.py	Updates provider filtering to use new field structure
llama_stack/core/datatypes.py	Removes unused property override

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-11T14:58:36Z

llama_stack/providers/registry/inference.py

-                description="HuggingFace Inference Endpoints provider for dedicated model serving.",
-            ),
+            provider_type="remote::hf::endpoint",
+            adapter_type="hf::endpoint",


[nitpick] The provider_type and adapter_type fields have the same value but different formats (remote::hf::endpoint vs hf::endpoint). This inconsistency could be confusing - consider standardizing the format or documenting why they differ.

Suggested change

adapter_type="hf::endpoint",

adapter_type="remote::hf::endpoint",

Copilot · 2025-09-11T14:58:37Z

llama_stack/providers/registry/inference.py

+            provider_type="remote::azure",
+            adapter_type="azure",


[nitpick] Same inconsistency as above - provider_type includes the remote:: prefix while adapter_type does not. This pattern appears to be consistent across the file but could benefit from documentation or standardization.

Copilot · 2025-09-11T14:58:37Z

tests/unit/distribution/test_distribution.py

            f.write(malformed_spec)

-        with pytest.raises(KeyError) as exc_info:
+        with pytest.raises(ValidationError) as exc_info:


The exception type change from KeyError to ValidationError should include the corresponding import statement. Ensure ValidationError is imported at the top of the file.

mattf

looking good. co-pilot raises an interesting question. should provider_type be inferred from the spec type and the adapter_type?

RemoteProviderSpec(
   ...
   provider_type="remote::XYZ",
   adapter_type="XYZ",
   ...
)

->

RemoteProviderSpec(
   ...
   adapter_type="XYZ",
   ...
)

the old remove_provider_spec function did this.

cdoern · 2025-09-11T18:54:40Z

looking good. co-pilot raises an interesting question. should provider_type be inferred from the spec type and the adapter_type?
RemoteProviderSpec(
   ...
   provider_type="remote::XYZ",
   adapter_type="XYZ",
   ...
)
->
RemoteProviderSpec(
   ...
   adapter_type="XYZ",
   ...
)
the old remove_provider_spec function did this.

I also considered this @mattf , but I was thinking the other way: get rid of adapter_type and only use provider_type since it's more descriptive. I don't feel too strongly about this besides getting rid of the remote_provider_spec function as it provides minimal value now that these are aligned more.

I think we keep this as is for now and if we think of a clever further refactor we can pull adapter_type out

cdoern · 2025-09-11T18:55:15Z

rebased.

cdoern · 2025-09-15T18:36:11Z

@mattf , any thoughts here?

leseb

Quick hold for discussion, is there any impact on external providers that should be reflected here? Thanks

cdoern · 2025-09-17T13:28:29Z

@leseb yes, external providers will need to switch their provider_spec methods to use RemoteProviderSpec directly rather than the helper.

Given that 0.2.22 is cut, I think we can merge this and let the various external provider authors know about this change.

mattf · 2025-09-17T13:41:44Z

@mattf , any thoughts here?

please don't rebase. it makes it very time consuming to figure out what you've changed since the last review.

there's no indication what you changed.

cdoern · 2025-09-17T13:43:47Z

@mattf , I changed nothing besides rebasing since you last reviewed, I believe.

FWIW, if we don't want folks to rebase, we should disable the alerts on the PRs indicating there are conflicts with the main branch, that is the only reason I do it! Sorry for the confusion!

mattf · 2025-09-17T13:52:43Z

@mattf , I changed nothing besides rebasing since you last reviewed, I believe.

FWIW, if we don't want folks to rebase, we should disable the alerts on the PRs indicating there are conflicts with the main branch, that is the only reason I do it! Sorry for the confusion!

right now there's an option to merge or rebase to update the branch, please prefer the merge option.

i'll check w/ admins about https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/configuring-commit-rebasing-for-pull-requests

cdoern · 2025-09-18T13:00:25Z

rebasing due to conflicts

all I am changing is the rebase, nothing more.

currently `RemoteProviderSpec` has an `AdapterSpec` embedded in it. Remove `AdapterSpec`, and put its leftover fields into `RemoteProviderSpec`. Additionally, many of the fields were duplicated between `InlineProviderSpec` and `RemoteProviderSpec`. Move these to `ProviderSpec` so they are shared. Fixup the distro codegen to use `RemoteProviderSpec` directly rather than `remote_provider_spec` which took an AdapterSpec and returned a full provider spec Signed-off-by: Charlie Doern <[email protected]>

leseb · 2025-09-18T14:09:56Z

@cdoern we need to followup on updating the external provider doc. Thanks!

# What does this PR do? currently `RemoteProviderSpec` has an `AdapterSpec` embedded in it. Remove `AdapterSpec`, and put its leftover fields into `RemoteProviderSpec`. Additionally, many of the fields were duplicated between `InlineProviderSpec` and `RemoteProviderSpec`. Move these to `ProviderSpec` so they are shared. Fixup the distro codegen to use `RemoteProviderSpec` directly rather than `remote_provider_spec` which took an AdapterSpec and returned a full provider spec ## Test Plan existing distro tests should pass. Signed-off-by: Charlie Doern <[email protected]>

meta-cla bot added the CLA Signed This label is managed by the Meta Open Source bot. label Sep 8, 2025

cdoern mentioned this pull request Sep 8, 2025

feat: allow for multiple external provider specs #3341

Merged

cdoern force-pushed the align-datatypes branch 10 times, most recently from 2d7b7c1 to f9d8cda Compare September 10, 2025 20:20

cdoern marked this pull request as ready for review September 10, 2025 21:11

cdoern requested review from ashwinb, bbrowning, ehhuang, hardikjshah, leseb, mattf, raghotham, reluctantfuturist, slekkala1, terrytangyuan and yanxi0830 as code owners September 10, 2025 21:11

cdoern force-pushed the align-datatypes branch from f9d8cda to 7f0b608 Compare September 10, 2025 21:11

cdoern commented Sep 10, 2025

View reviewed changes

cdoern force-pushed the align-datatypes branch 4 times, most recently from e4f5e2a to b845533 Compare September 11, 2025 13:23

cdoern force-pushed the align-datatypes branch from b845533 to 911f8c7 Compare September 11, 2025 13:39

mattf requested a review from Copilot September 11, 2025 14:57

Copilot AI reviewed Sep 11, 2025

View reviewed changes

mattf approved these changes Sep 11, 2025

View reviewed changes

cdoern force-pushed the align-datatypes branch from 911f8c7 to 99c31e6 Compare September 11, 2025 18:55

leseb requested changes Sep 16, 2025

View reviewed changes

cdoern force-pushed the align-datatypes branch from 99c31e6 to 06ef5a0 Compare September 17, 2025 13:29

cdoern force-pushed the align-datatypes branch from 06ef5a0 to 686f87d Compare September 18, 2025 13:01

cdoern requested a review from leseb September 18, 2025 13:05

leseb approved these changes Sep 18, 2025

View reviewed changes

leseb merged commit 8422bd1 into llamastack:main Sep 18, 2025
43 checks passed

m-misiura mentioned this pull request Sep 30, 2025

Update provider.py to ensure it can be loaded using RemoteProviderSpec introduced in lls 0.23.0 trustyai-explainability/llama-stack-provider-trustyai-fms#31

Merged

cdoern mentioned this pull request Oct 1, 2025

Update provider spec & upgrade to llama-stack 0.2.22 trustyai-explainability/llama-stack-provider-ragas#17

Merged

	adapter_type="hf::endpoint",
	adapter_type="remote::hf::endpoint",

feat: combine ProviderSpec datatypes #3378

feat: combine ProviderSpec datatypes #3378

Uh oh!

Conversation

cdoern commented Sep 8, 2025

What does this PR do?

Test Plan

Uh oh!

cdoern Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

cdoern Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 11, 2025

Choose a reason for hiding this comment

Uh oh!

mattf left a comment

Choose a reason for hiding this comment

Uh oh!

cdoern commented Sep 11, 2025

Uh oh!

cdoern commented Sep 11, 2025

Uh oh!

cdoern commented Sep 15, 2025

Uh oh!

leseb left a comment

Choose a reason for hiding this comment

Uh oh!

cdoern commented Sep 17, 2025

Uh oh!

mattf commented Sep 17, 2025

Uh oh!

cdoern commented Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mattf commented Sep 17, 2025

Uh oh!

cdoern commented Sep 18, 2025

Uh oh!

leseb commented Sep 18, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cdoern commented Sep 17, 2025 •

edited

Loading