Skip to content

chore: Contributor documentation for adding models and params #138

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
YoungVor wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

chore: Contributor documentation for adding models and params #138

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 41 additions & 1 deletion CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ uv sync --extra=cloud
uv run pytest -m cloud tests
```

> ⚠️ Note: All tests require a valid OpenAI/Anthropic API key set in the environment variables.
> ⚠️ Note: All tests require a valid OpenAI API key set in the environment variables.

---

Expand Down Expand Up @@ -196,6 +196,46 @@ To run the demo notebooks:

---

## Adding model support
Copy link

@rohitrastogi rohitrastogi Sep 5, 2025

Choose a reason for hiding this comment

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

Can we add a bit more context on how inference works in fenic? I think it'd make sense to add a readme in the /_inference directory and link to it here.

Specifically it would be good to mention:

  • inference currently runs in a map_batches Polars udf
  • we have LanguageModel/EmbeddingModel classes that each own a ModelClient that the udfs use
  • ModelClients work though a producer/consumer mechanism - Polars threads produce requests, consumer threads submit them asynchronously in an event loop to maximize thoughput without spawning too many threads
  • ModelClients have RateLimitStrategies to make sure we don't overwhelm inference providers and exceed rate limits

It's also worth mentioning how each provider has its own set of embedding/language model configs set in the sessionconfig where we specify the model name, rate limits, and any special per model/provider functionality.

Lastly, I'd describe the nuances of token accounting and how an integration needs to fit in there. Would be useful to link some related PRs for past model integration to the guide.

Copy link
Contributor Author

@YoungVor YoungVor Sep 5, 2025

Choose a reason for hiding this comment

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

Definitely agree, there is a lot of detail around adding provider/model support and it deserves its own readme. That also means we can add more detail.

I'll take a second pass and add this detail as well


### Adding support for a provider

- update the `ModelProvider` enum
- create an instance of the ModelProviderClass (see `OpenAIModelProvider`)
- create an instance of ModelClient for LLM and/or Embedding clients (see `OpenAIBatchChatCompletionsClient` and `OpenAIBatchEmbeddingsClient`)
- create a provider specific instance of `ProfileManager`, if models from this provider will support configuration changes

### Adding a Language or Embedding Models

- Update `model_catalog.py`
- Add your model to the provider’s options
- for e.g., add to `OpenAILanguageModelName` or `OpenAIEmbeddingModelName`
- Add the model to the catalog object with `_add_model_to_catalog()` with the appropriate `CompletionModelParameters` or `EmbeddingModelParameters`

### Adding provider-specific params using the model Profile

Profile level parameters allow the user to configure their model. You can add fields specific to a provider and subset of models by updating that provider’s profile class.

- A provider `Profile` is defined in the provider's model configuration class (see `OpenAILanguageModel`)
- Add provider specific the model’s `Profile` with the new `Field`
- for e.g. look at [`config.py`](http://config.py) and see `reasoning_effort` defined in `OpenAILanguageModel.Profile`
- Update the resolved model profile in [`_resolved_session_config.py`](https://github.com/typedef-ai/fenic/pull/135/files#diff-454f845e61c21cebcf1003bf35368aead665489a72376d2ae0b031f3c26293c4)
- for e.g.: `ResolvedOpenAIModelProfile`

### Adding model-specific support for params

Some params may not be supported or have strict requirements on the input values depending on the model. To add model-specific logic:

- Add a flag to global `CompletionModelParameters` in `model_catalog.py`, then set the flag for each appropriate model added to the catalog (see `_add_model_to_catalog()` calls).
- this allows models to pass specific flags to the model and profile registry codepaths (for e.g.: `supports_reasoning`)
- models can be configured to not accept Profiles at all (`supports_profiles`)

- To change or enforce values for a model profile param values based on the model params (for e.g.: to ignore the field for unsupported models), update that model’s `ProfileManager` class
- use the model_parameters to set the parameters passed to the model client
- for openai, look at `OpenAICompletionsProfileManager` in `openai_profile_manager.py`

---

## 🙋 Need Help?

Have questions or want to contribute? Join us on [Discord](https://discord.gg/GdqF3J7huR)!