Skip to content

fix: require openai>=2.0 and add a generic OpenAI-compatible provider - #126

Merged
giswqs merged 4 commits into
mainfrom
fix-openai-compat-118-125
Aug 9, 2026
Merged

fix: require openai>=2.0 and add a generic OpenAI-compatible provider#126
giswqs merged 4 commits into
mainfrom
fix-openai-compat-118-125

Conversation

@giswqs

@giswqs giswqs commented Aug 9, 2026

Copy link
Copy Markdown
Member

Fixes #118. Closes #125.

#118OpenAIResponsesModel requires openai>=2.0.0 on Test Provider

The default openai-codex provider builds a Strands OpenAIResponsesModel, which validates the OpenAI SDK version at import time and raises on anything below 2.0. Both the openai extra and the QGIS plugin's PROVIDER_PACKAGES pinned only openai>=1.0, so a fresh install could resolve to a 1.x release and every Test Provider click failed with the raw ImportError the reporter screenshotted.

  • Raise the floor to openai>=2.0 in pyproject.toml and in the plugin's dependency manager.
  • Wrap the import in resolve_model so the message explains the upgrade and the openai provider fallback, instead of leaking the Strands internal error.

Verified on the real path: pip install ".[providers]" into a clean venv now yields openai 2.53.0, and resolve_model(provider="openai-codex") returns an OpenAIResponsesModel.

#125 — generic OpenAI-compatible endpoints

New openai-compatible provider for any server exposing the OpenAI Chat Completions API — llama.cpp, LM Studio, Text Generation WebUI, vLLM.

Setting Environment variable
Base URL (required) OPENAI_COMPATIBLE_BASE_URL
Model id (required) OPENAI_COMPATIBLE_MODEL
API key (optional) OPENAI_COMPATIBLE_API_KEY
agent = GeoAgent(
    provider="openai-compatible",
    model="qwen3-8b",
    openai_compatible_base_url="http://localhost:8000/v1",
)

A model id is required because a generic endpoint has no sensible default. The API key falls back to the EMPTY placeholder the vLLM path already uses, so keyless local servers work. Wired into the Solara UI and the QGIS plugin (provider list, default models, credential fields, environment mapping, settings reset).

Verified with a real round trip: a local stub server received POST /v1/chat/completions with the configured model and Authorization: Bearer EMPTY, and the agent returned the streamed reply.

Heads-up: vLLM extra is no longer in providers / all

strands-vllm (latest, 0.0.6) hard-pins openai<2.0.0, so it cannot resolve alongside the openai>=2.0 this PR requires. Leaving it in GeoAgent[providers] makes that extra uninstallable outright, so it is now excluded from providers, all, and the plugin's Core Providers group.

  • The standalone GeoAgent[vllm] extra is unchanged, for dedicated environments that do not need openai-codex.
  • The new openai-compatible provider reaches a vLLM server's /v1 URL with no extra dependency, so this is a soft landing rather than a capability loss.

This is the one judgment call in the PR that has user-visible packaging impact — happy to revisit if you would rather keep vLLM bundled and solve #118 a different way.

Testing

  • pytest tests/ — 329 passed, 2 skipped (7 new provider tests).
  • pre-commit run --files <changed> — clean.
  • Clean-venv pip install ".[providers]" and a live round trip, as described above.

Note: the deploy check fails on pre-existing mkdocs strict-mode docstring warnings in geoagent/core/registry.py (from 7abc595), unrelated to this branch.

Fixes #118. The default openai-codex provider builds a Strands
OpenAIResponsesModel, which raises "OpenAIResponsesModel requires
openai>=2.0.0" at import time on older SDKs. Both the `openai` extra and
the QGIS plugin's provider dependency group pinned only `openai>=1.0`, so
a fresh install could resolve to a 1.x release and every Test Provider run
failed. Raise the floor to `openai>=2.0` in both places and wrap the
import so the failure explains the upgrade instead of leaking the raw
ImportError.

Closes #125. Add an `openai-compatible` provider that targets any server
exposing the OpenAI Chat Completions API (llama.cpp, LM Studio, Text
Generation WebUI, vLLM) via a generic base URL. A model id is required
because a generic endpoint has no sensible default, and the API key falls
back to the `EMPTY` placeholder the vLLM path already uses so keyless
local servers work.

- Add `openai-compatible` to ProviderName, GeoAgentConfig
  (`openai_compatible_base_url`), and environment-based provider
  detection via OPENAI_COMPATIBLE_BASE_URL.
- Resolve it through Strands OpenAIModel, honoring
  OPENAI_COMPATIBLE_MODEL and OPENAI_COMPATIBLE_API_KEY.
- Surface it in the Solara UI provider list and in the QGIS plugin
  (provider list, default models, credential fields, environment
  mapping, and settings reset).
- Drop strands-vllm from GeoAgent[providers], GeoAgent[all], and the
  plugin's Core Providers group: it pins openai<2.0 and therefore cannot
  resolve alongside openai>=2.0. The standalone GeoAgent[vllm] extra is
  unchanged for dedicated environments, and openai-compatible reaches a
  vLLM server's /v1 URL with no extra dependency.
- Add provider tests for both fixes and refresh the README, docs, and
  QGIS plugin docs.
Copilot AI lite review requested due to automatic review settings August 9, 2026 15:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@giswqs, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 3 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d72af6ef-e2e4-4a6a-8b25-7d8ae7dc8130

📥 Commits

Reviewing files that changed from the base of the PR and between 7a69b2d and 0a2a8fd.

📒 Files selected for processing (15)
  • README.md
  • docs/index.md
  • docs/installation.md
  • docs/qgis-plugin.md
  • docs/ui.md
  • geoagent/core/config.py
  • geoagent/core/model.py
  • geoagent/ui/app.py
  • pyproject.toml
  • qgis_geoagent/README.md
  • qgis_geoagent/open_geoagent/deps_manager.py
  • qgis_geoagent/open_geoagent/dialogs/chat_dock.py
  • qgis_geoagent/open_geoagent/dialogs/settings_dock.py
  • qgis_geoagent/tests/test_settings_diagnostics.py
  • tests/test_model_providers.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Comment thread qgis_geoagent/open_geoagent/dialogs/chat_dock.py Outdated
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown

Code review

Bugs

  • None found. The new openai-compatible provider block in geoagent/core/model.py correctly validates required base_url/model_id, falls back to the EMPTY API key placeholder, and mirrors the existing vllm path's structure. Config, UI, and QGIS plugin wiring (provider lists, default models, env var mapping, settings reset, diagnostics redaction) are all consistently updated together, and the existing tests that check provider membership (tests/test_ui_app.py, qgis_geoagent/tests/test_chat_tool_inputs.py) use membership checks rather than exact-list equality, so they won't break from the new provider entry.

Security

  • None found. No injection risk, credentials are only read from env/QSettings and never logged; # pragma: allowlist secret markers are correctly applied to new key-bearing lines.

Performance

  • None found. No new hot paths or inefficiencies introduced.

Quality

  • Low confidence, nit: qgis_geoagent/open_geoagent/dialogs/chat_dock.py (_apply_environment_from_settings, env_map) wraps the new openai_compatible_api_key value in a 1-tuple while every sibling single-value entry uses a plain string. The loop already normalizes both forms so it's not a functional bug, just an inconsistency with the surrounding style — flagged inline with a suggested fix.
  • Low confidence: the base_url precedence order in the new openai-compatible block (client_args override → cfg field → env var) is the reverse of the order used in the adjacent vllm block (cfg field → client_args → env var). Unlikely to matter in practice since callers rarely set both, but worth a glance for intentionality.

CLAUDE.md

  • No CLAUDE.md found in the repository to check against.

- chat_dock.py: store `openai_compatible_api_key` env mapping as a plain
  string instead of a 1-tuple, matching every other single-env-var entry in
  `env_map`. The loop normalizes both forms, so this is style-only.
@giswqs

giswqs commented Aug 9, 2026

Copy link
Copy Markdown
Member Author

On the Claude review's second quality note (base_url precedence in the new openai-compatible block): that ordering is intentional. The block mirrors the adjacent openrouter path — both build an OpenAIModel and use client_args.get("base_url") -> cfg field -> env var. The vllm block is the outlier because it pops from client_args and passes base_url as an explicit VLLMModel argument. Leaving as-is so the two OpenAIModel providers stay consistent with each other.

Comment thread geoagent/core/model.py
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown

Code review

Bugs: None found. The openai-compatible provider branch in geoagent/core/model.py correctly validates required base_url/model with clear errors, defaults the API key to "EMPTY" for keyless local servers (matching the existing vLLM pattern), and is wired consistently through GeoAgentConfig, provider-detection order, the Solara UI (geoagent/ui/app.py), and the QGIS plugin (chat_dock.py, settings_dock.py, deps_manager.py). The openai>=2.0 floor is applied consistently across pyproject.toml and the plugin's PROVIDER_PACKAGES, and dropping strands-vllm from providers/all/Core Providers correctly avoids the version conflict it introduces. High confidence.

Security: None found. No injected input, no secrets committed; credential fields follow the existing redaction/env-fallback pattern (ENV_FALLBACKS, credential_presence) with # pragma: allowlist secret used consistently. High confidence.

Performance: None found; changes are configuration/branching only, no hot-path impact. High confidence.

Quality:

  • geoagent/core/model.py (~line 262): the except ImportError around OpenAIResponsesModel assumes any import failure means "openai<2.0" and reports that in the message. True for the reported issue, but could mislabel an unrelated import failure (e.g., an incompatible strands-agents release). Not a blocker. Low confidence.
  • Minor test-coverage asymmetry: qgis_geoagent/tests/test_chat_tool_inputs.py has dedicated tests for the vllm provider's defaults/env-application (test_vllm_provider_defaults_are_available, test_apply_environment_sets_vllm_values) but no equivalent for openai-compatible, even though qgis_geoagent/tests/test_settings_diagnostics.py did get a new env-application test for it. Not required, just an inconsistency. Low confidence.

CLAUDE.md: No CLAUDE.md file exists in this repository, so no project-specific guidelines apply.

Overall this is a tightly scoped, well-tested PR (new provider branch, config wiring, UI wiring, docs, and 7+ new tests all move together) with a reasonable, clearly-documented tradeoff on dropping bundled vllm from providers/all.

- model.py: reword the openai-codex ImportError so it no longer asserts a
  single root cause. It now says an old openai package is the usual reason,
  and points at the chained original error when openai 2.x is already
  installed.
Comment thread geoagent/core/model.py
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown

Code review

Bugs

  • None found. The new openai-compatible provider branch in resolve_model (geoagent/core/model.py:248-288) correctly mirrors the established openrouter/vllm patterns for base URL resolution, API-key fallback (EMPTY placeholder), token/temperature params, and error messages for missing base URL / model id.
  • Low confidence: the except ImportError wrapper added around the openai-codex Responses-API import (geoagent/core/model.py:93-106) will also catch unrelated import failures and present them as an "upgrade openai" hint. The message is well-hedged and preserves the original exception, so impact is minor — flagged inline.

Security

  • No issues found. No new injection, unsafe eval, or credential-leak surface; the new provider follows the same locally-configured base-URL/API-key pattern already used by vllm/litellm/openrouter, and QGIS settings continue to be stored/read the same way as existing credentials.

Performance

  • No issues found. Changes are limited to config resolution and doc/UI wiring; no hot-path or loop changes.

Quality

  • Very low confidence nit: base_url resolution priority order differs slightly between the vllm branch (cfg.vllm_base_url checked before client_args) and the new openai-compatible/openrouter branches (client_args checked first). Purely stylistic inconsistency inherited from pre-existing code, not introduced incorrectly by this PR — not worth a fix on its own.
  • The new provider is thoroughly wired end-to-end: ProviderName literal, default-provider env detection, resolve_model, pyproject.toml extras, Solara UI (geoagent/ui/app.py), and the QGIS plugin (deps_manager.py, chat_dock.py, settings_dock.py — credential fields, env fallbacks, save/load/reset, diagnostics) all stay consistent with the existing provider patterns. Docs (README, docs/*.md, qgis_geoagent/README.md) are updated consistently and explain the strands-vllm/openai>=2.0 conflict clearly.
  • Test coverage is solid: new tests cover config validation, env-based provider selection, resolve_model success/failure paths (missing base URL, missing model id, default API key fallback), the plugin's dependency-list changes, and the improved openai-codex import-error message.

CLAUDE.md

  • No CLAUDE.md exists in this repository, so no project-specific guidelines apply.

@giswqs
giswqs merged commit afa9e77 into main Aug 9, 2026
9 of 10 checks passed
@giswqs
giswqs deleted the fix-openai-compat-118-125 branch August 9, 2026 16:22
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.

openai api compability "OpenAIResponsesModel requires openai>=2.0.0" error during Test Provider

2 participants