Skip to content

fix: honor llm registry provider blocks - #2502

Merged
stranske merged 1 commit into
mainfrom
fix/sync-llm-provider-registry-blocks
Jun 22, 2026
Merged

fix: honor llm registry provider blocks#2502
stranske merged 1 commit into
mainfrom
fix/sync-llm-provider-registry-blocks

Conversation

@stranske

@stranske stranske commented Jun 22, 2026

Copy link
Copy Markdown
Owner

Summary

  • preserve the LLM registry empty-string blocked-model signal instead of falling back to default provider models
  • allow Anthropic-only LangChain environments to construct clients without requiring langchain_openai
  • add focused regression coverage for provider block handling and Anthropic-only client resolution

Validation

  • uv run pytest tests/tools/test_langchain_client.py tests/tools/test_llm_provider.py -q
  • uv run --with pyyaml python scripts/validate_template_sync.py
  • uv run ruff check tools/llm_provider.py tools/langchain_client.py tests/tools/test_langchain_client.py tests/tools/test_llm_provider.py
  • uv run black --check tools/llm_provider.py tools/langchain_client.py tests/tools/test_langchain_client.py tests/tools/test_llm_provider.py
  • git diff --check

Summary by CodeRabbit

  • Bug Fixes

    • Improved resilience for Anthropic-only environments by removing hard dependency on OpenAI imports.
    • Enhanced error handling when configured models are unavailable.
  • Tests

    • Added test coverage for client construction without OpenAI package.
    • Added test coverage for model availability validation.

@stranske stranske added automation Automation and workflow automation maintenance Maintenance tasks github:actions GitHub Actions related langchain sync codex codex-automation labels Jun 22, 2026
@stranske
stranske temporarily deployed to agent-standard June 22, 2026 12:52 — with GitHub Actions Inactive
@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

tools/langchain_client.py defers langchain_openai import failures to a None sentinel and gates all OpenAI/GitHub client construction on that sentinel. tools/llm_provider.py adds early-return guards when model resolution returns a falsy value and fixes None-vs-empty-string handling in _configured_langchain_model. Four new tests cover both Anthropic-without-OpenAI and blocked-model-signal scenarios.

Changes

Optional langchain_openai and blocked-model guards

Layer / File(s) Summary
Deferred OpenAI import and chat_openai_cls gating
tools/langchain_client.py
ImportError from langchain_openai now sets chat_openai_cls = None instead of returning early. All OpenAI- and GitHub-backed construction paths in both build_chat_client and build_chat_clients are gated on chat_openai_cls being truthy, and _build_openai_client/_build_github_client receive chat_openai_cls rather than a hardcoded ChatOpenAI reference.
Falsy model-name early-return and None-vs-empty fix
tools/llm_provider.py
_configured_langchain_model() uses an intermediate variable so an empty-string configured value is preserved rather than replaced by the fallback. OpenAIProvider._get_client() and AnthropicProvider._get_client() return None immediately when the resolved model name is falsy, before reaching build_chat_client.
Tests for missing-package and blocked-model scenarios
tests/tools/test_langchain_client.py, tests/tools/test_llm_provider.py
Two tests assert build_chat_client/build_chat_clients resolve an Anthropic client when sys.modules["langchain_openai"] is None. Two tests assert OpenAIProvider.analyze_completion and AnthropicProvider.analyze_completion raise RuntimeError with provider-specific messages when configured_model_for_provider returns an empty string, without calling build_chat_client.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • stranske/Workflows#2499: Modifies the same build_chat_client/build_chat_clients slot-iteration and provider-selection gating logic in tools/langchain_client.py.
  • stranske/Workflows#2501: Modifies tools/llm_provider.py's OpenAI/Anthropic model-resolution logic and related _get_client/_configured_langchain_model behavior.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: honor llm registry provider blocks' directly describes the main change—implementing proper handling of LLM registry provider blocks—which is the primary objective of the pull request.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/sync-llm-provider-registry-blocks

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

@stranske-keepalive

Copy link
Copy Markdown
Contributor

Workflow source needed

PR #2502 needs either a linked GitHub issue or one valid non-issue Workflow Source before PR metadata automation can manage it safely.

Please do one of:

  • Add <!-- meta:issue:123 --> or a normal Closes #123 / Related to #123 line.
  • Check one Workflow Source option in the PR body.
  • Add a hidden marker such as <!-- workflow-source:local_request -->, <!-- workflow-source:manual_remote -->, <!-- workflow-source:review_followup -->, <!-- workflow-source:sync_campaign -->, or <!-- workflow-source:dependabot -->.
  • Add a workflow source label such as workflow:source-direct-pr, workflow:source-local-request, workflow:source-review-followup, workflow:source-sync, or workflow:no-automation.

Once a valid source is present, this warning will not be reposted.

@stranske
stranske enabled auto-merge (squash) June 22, 2026 12:53
@stranske-keepalive

stranske-keepalive Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Automated Status Summary

Head SHA: 4c66fa8
Latest Runs: ⏳ pending — Gate
Required contexts: Gate / gate, Health 45 Agents Guard / guard
Required: core tests (3.12): ⏳ pending, core tests (3.13): ⏳ pending, docker smoke: ⏳ pending, gate: ⏳ pending

Workflow / Job Result Logs
(no jobs reported) ⏳ pending

Coverage Overview

  • Coverage history entries: 1

Coverage Trend

Metric Value
Current 70.59%
Baseline 85.00%
Delta -14.41%
Minimum 70.00%
Status ✅ Pass

Top Coverage Hotspots (lowest coverage)

File Coverage Missing
scripts/check_api_wrapper_guard.py 0.0% 123
scripts/cleanup_labels.py 0.0% 127
scripts/issue_dedup_smoke.py 0.0% 4
scripts/list_registered_consumer_repos.py 0.0% 33
scripts/repo_review_backlog_scan.py 0.0% 203
scripts/repo_review_round1_runner.py 0.0% 225
scripts/repo_review_round1_schema.py 0.0% 194
scripts/runner_lib/__main__.py 0.0% 3
scripts/update_langchain_versions.py 0.0% 34
scripts/validate_dependency_test_setup.py 0.0% 112
scripts/validate_template_completeness.py 0.0% 88
scripts/validate_template_sync.py 0.0% 77
scripts/validate_workflow_yaml.py 0.0% 98
scripts/repo_review_round2_schema.py 8.8% 185
scripts/langchain/topic_splitter.py 19.1% 57

Low Coverage Files (<50.0%)

File Coverage Missing
scripts/check_api_wrapper_guard.py 0.0% 123
scripts/cleanup_labels.py 0.0% 127
scripts/issue_dedup_smoke.py 0.0% 4
scripts/list_registered_consumer_repos.py 0.0% 33
scripts/repo_review_backlog_scan.py 0.0% 203
scripts/repo_review_round1_runner.py 0.0% 225
scripts/repo_review_round1_schema.py 0.0% 194
scripts/runner_lib/__main__.py 0.0% 3
scripts/update_langchain_versions.py 0.0% 34
scripts/validate_dependency_test_setup.py 0.0% 112
scripts/validate_template_completeness.py 0.0% 88
scripts/validate_template_sync.py 0.0% 77
scripts/validate_workflow_yaml.py 0.0% 98
scripts/repo_review_round2_schema.py 8.8% 185
scripts/langchain/topic_splitter.py 19.1% 57

Updated automatically; will refresh on subsequent CI/Docker completions.


Keepalive checklist

Scope

No scope information available

Tasks

  • No tasks defined

Acceptance criteria

  • No acceptance criteria defined

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 82ab8b1686

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tools/langchain_client.py
from langchain_openai import ChatOpenAI
except ImportError:
return None
chat_openai_cls = None

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Sync the consumer template copies

This source change is consumer-facing, but the matching templates/consumer-repo/tools/langchain_client.py still returns immediately when langchain_openai is missing, and templates/consumer-repo/tools/llm_provider.py still converts a registry "" block signal back to the fallback model. Repos bootstrapped from templates/consumer-repo therefore keep the old behavior: Anthropic-only installs still fail client resolution and blocked fallback models can still be used. Please mirror these tools/ changes into the consumer template copies.

Useful? React with 👍 / 👎.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
tools/langchain_client.py (1)

293-299: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Gate OpenAI slot availability on chat_openai_cls in auto-selection predicates.

Lines 295 and 487 still treat OpenAI as available based only on openai_token. When langchain_openai is missing but OPENAI_API_KEY is set, an unbuildable OpenAI slot can still be selected during prefiltering, which can skew slot/model override assignment in build_chat_clients.

Suggested fix
@@
-                slot.provider == PROVIDER_OPENAI and openai_token,
+                slot.provider == PROVIDER_OPENAI and openai_token and chat_openai_cls,
@@
-                slot.provider == PROVIDER_OPENAI and openai_token,
+                slot.provider == PROVIDER_OPENAI and openai_token and chat_openai_cls,

Also applies to: 485-490

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tools/langchain_client.py` around lines 293 - 299, The OpenAI slot
availability check in the slot_available condition is missing a validation for
the chat_openai_cls class, similar to how the Anthropic and GitHub slots check
for their respective chat_*_cls classes. Update the condition checking
slot.provider == PROVIDER_OPENAI to also verify that chat_openai_cls is
available (not None) by adding and chat_openai_cls to the condition. This same
issue also exists at lines 485-490 in another auto-selection predicate location,
so both conditions need to be fixed to prevent unbuildable OpenAI slots from
being selected during prefiltering.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@tools/langchain_client.py`:
- Around line 293-299: The OpenAI slot availability check in the slot_available
condition is missing a validation for the chat_openai_cls class, similar to how
the Anthropic and GitHub slots check for their respective chat_*_cls classes.
Update the condition checking slot.provider == PROVIDER_OPENAI to also verify
that chat_openai_cls is available (not None) by adding and chat_openai_cls to
the condition. This same issue also exists at lines 485-490 in another
auto-selection predicate location, so both conditions need to be fixed to
prevent unbuildable OpenAI slots from being selected during prefiltering.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: f1173805-6614-489f-86f9-7e23200c555e

📥 Commits

Reviewing files that changed from the base of the PR and between f4fa438 and 82ab8b1.

📒 Files selected for processing (4)
  • tests/tools/test_langchain_client.py
  • tests/tools/test_llm_provider.py
  • tools/langchain_client.py
  • tools/llm_provider.py
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • stranske/Template (auto-detected)
  • stranske/Fine-Art-Archive (auto-detected)
  • stranske/Ready (auto-detected)
  • stranske/Workflows-Integration-Tests (auto-detected)

@stranske
stranske merged commit 9eda7b4 into main Jun 22, 2026
116 of 125 checks passed
@stranske
stranske deleted the fix/sync-llm-provider-registry-blocks branch June 22, 2026 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automation Automation and workflow automation codex codex-automation github:actions GitHub Actions related langchain maintenance Maintenance tasks sync

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant