Skip to content

fix(models): make adapter call-time options explicit - #65

Merged
knisar merged 1 commit into
wandb:mainfrom
dvd233:fix/rai-toolkit-63-call-options
Sep 11, 2026
Merged

knisar merged 1 commit into
wandb:mainfrom
dvd233:fix/rai-toolkit-63-call-options

Conversation

@dvd233

@dvd233 dvd233 commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Closes #63

What changed

  • Added a shared validation helper so built-in adapters reject unsupported call-time options with deterministic TypeError messages.
  • Added the documented positive-integer max_tokens override to OpenAICompatibleModel, while preserving the provider-default request shape when it is omitted.
  • Preserved AnthropicModel's existing max_tokens normalization and made it explicit that max_tokens is the portable option; temperature remains OpenAI-compatible-specific.
  • Kept adapter-controlled fields and arbitrary provider SDK parameters out of the call-time kwargs surface.
  • Updated the adapter guide and added fully mocked regression coverage for accepted, omitted, invalid, misspelled, and multiple unsupported options.

The compatibility change is intentional: callers that relied on ignored keyword arguments now fail clearly instead of silently doing nothing.

Validation

  • python -m pytest -q tests/test_openai_compatible.py tests/test_anthropic.py: 104 passed, 2 skipped
  • python -m pytest -q (proxy environment cleared): 206 passed, 6 skipped
  • python -m ruff check --select E9,F63,F7,F82 .: passed
  • reuse --no-multiprocessing lint: passed
  • git diff --check: passed
  • Changed-file Ruff, compileall, and pip check: passed

AI assistance

AI assistance was used to inspect the scoped issue, draft the implementation and tests, and review edge cases. I personally reviewed the six-file diff and verified the behavior with mocked provider transports and the validation commands above.

Checklist

  • I checked the issue's Development section and open pull requests for linked work.
  • This pull request is focused on one change.
  • Behavioural changes include tests, or I explained why a test is not
    needed.
  • I included the local validation commands and results.
  • I updated documentation for user-visible changes.
  • I checked ownership before adding or changing SPDX headers.

@dvd233
dvd233 marked this pull request as ready for review September 11, 2026 01:04
@knisar knisar added the status: in review Pull request is ready for maintainer review label Sep 11, 2026

@knisar knisar left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for taking this on. I checked the adapter behavior, error handling, documentation, and mocked coverage at 9ffc4f3. I also ran the full suite and tested the result against current main. The two Weave jobs used the PR's older base and hit the test package import issue that is already fixed on main. The merged result passes locally. This is ready to merge.

@knisar
knisar merged commit d42549e into wandb:main Sep 11, 2026
7 of 9 checks passed
@knisar knisar removed the status: in review Pull request is ready for maintainer review label Sep 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make built-in adapter call-time options explicit and reject unsupported kwargs

2 participants