Skip to content

chore: sync workflow templates - #846

Closed
stranske wants to merge 1 commit into
mainfrom
sync/workflows-1243c652ce3c
Closed

chore: sync workflow templates#846
stranske wants to merge 1 commit into
mainfrom
sync/workflows-1243c652ce3c

Conversation

@stranske

@stranske stranske commented Jun 22, 2026

Copy link
Copy Markdown
Owner

Sync Summary

Files Updated

  • progress_reviewer.py: Progress reviewer - evaluates agent progress for keepalive rounds
  • llm_provider.py: LLM provider configuration - GitHub Models and OpenAI client setup

Files Skipped

  • renovate.json: File exists and sync_mode is create_only
  • cross-repo-smoke.yml: File exists and sync_mode is create_only
  • llm_slots.json: None

Review Checklist

  • CI passes with updated workflows
  • No repo-specific customizations were overwritten

Source: stranske/Workflows
Source SHA: 680260c913e1c136e0b0ba11c07a760e23c26b0c
Template hash: 1243c652ce3c
Sync branch: sync/workflows-1243c652ce3c
Consumer repo: stranske/Template
Manifest: .github/sync-manifest.yml

Summary by CodeRabbit

  • Refactor
    • Updated LLM client construction for OpenAI and Anthropic providers.
    • Improved dynamic model name resolution across LLM utilities.

Automated sync from stranske/Workflows
Template hash: 1243c652ce3c

Changes synced from sync-manifest.yml
@stranske stranske added sync Automated sync from Workflows automated Automated sync from Workflows labels Jun 22, 2026
@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 7ed0a83b-7745-4f99-850f-e06c687edaf7

📥 Commits

Reviewing files that changed from the base of the PR and between 89d68b1 and 9e25328.

📒 Files selected for processing (2)
  • scripts/langchain/progress_reviewer.py
  • tools/llm_provider.py
🔗 Linked repositories identified

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

  • stranske/Workflows (auto-detected)
📜 Recent review details
⏰ Context from checks skipped due to timeout. (1)
  • GitHub Check: check
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

In Manager-Database repository using Prefect 2.x, import schedules from prefect.client.schemas.schedules rather than other locations

Files:

  • scripts/langchain/progress_reviewer.py
  • tools/llm_provider.py
🔀 Multi-repo context stranske/Workflows

Based on my exploration of the stranske/Workflows repository, I can now provide you with the cross-repository context for this sync PR.

Linked repositories findings

stranske/Workflows (Source Repository)

Key changes being synced:

  1. progress_reviewer.py - Import path change [::stranske/Workflows::]

    • Line ~330-335: Changed from importing build_chat_client from tools.langchain_client to importing build_client from scripts.langchain._llm_client
    • Pattern: resolved = build_client(model=model) if build_client else None
    • This is a higher-level abstraction over the tools layer
  2. llm_provider.py - OpenAI and Anthropic provider updates [::stranske/Workflows::]

    • OpenAIProvider._get_client() (line ~580-586): Now uses build_chat_client(provider="openai", model=model_name) with resolved model from _configured_langchain_model()
    • AnthropicProvider._get_client() (line ~640-646): Same pattern with anthropic provider
    • Both now store self._model_name = resolved.model for later use
    • In analyze_completion(), the returned CompletionAnalysis now sets model_name to the resolved model instead of hardcoded defaults
  3. New helper function [::stranske/Workflows::]

    • _configured_langchain_model(provider, fallback) in llm_provider.py (line ~58-62) that:
      • Tries to import tools.llm_registry.configured_model_for_provider
      • Falls back gracefully if import fails
      • Returns resolved model for the provider or fallback

Cross-repo dependencies identified:

  1. scripts/langchain/_llm_client.py must export build_client [::stranske/Workflows::]

    • Used by progress_reviewer.py at line ~328
    • Used by followup_issue_generator.py at lines ~100-111
    • Returns object with .client and .provider and .model attributes
  2. tools/langchain_client.py must export build_chat_client [::stranske/Workflows::]

    • Used by llm_provider.py OpenAIProvider (line ~583)
    • Used by llm_provider.py AnthropicProvider (line ~643)
    • Returns object with .client and .model attributes
    • Also used in test mocks (tests/tools/test_llm_provider.py lines ~27-29, ~40-42)
  3. tools/llm_registry.py must export configured_model_for_provider [::stranske/Workflows::]

    • Used by llm_provider.py._configured_langchain_model() at line ~60
    • Signature: configured_model_for_provider(provider, fallback=...)
    • Must return a model string or None/falsy value

Test coverage [::stranske/Workflows::]

  • tests/scripts/test_progress_reviewer.py: Mocks scripts.langchain._llm_client.build_client (line ~22)
  • tests/tools/test_llm_provider.py: Mocks both tools.llm_registry.configured_model_for_provider and tools.langchain_client.build_chat_client (lines ~27-29, ~40-42)

Critical compatibility notes:

  • The changes maintain backward compatibility through try/except blocks with fallback behavior
  • If scripts.langchain._llm_client is unavailable, progress_reviewer falls back to heuristic-only mode
  • If tools.llm_registry.configured_model_for_provider is unavailable, llm_provider returns the fallback model
  • The resolved object must have .client, .provider, and .model attributes for proper operation
🔇 Additional comments (2)
tools/llm_provider.py (1)

45-52: LGTM!

Also applies to: 596-606, 638-642, 667-677, 719-723

scripts/langchain/progress_reviewer.py (1)

430-434: LGTM!


📝 Walkthrough

Walkthrough

tools/llm_provider.py gains a _configured_langchain_model helper that resolves model names via an optional registry import with fallback. OpenAIProvider and AnthropicProvider now construct their clients through build_chat_client and emit dynamic model_name values in analyze_completion. scripts/langchain/progress_reviewer.py switches its conditional import from build_chat_client to build_client from the internal scripts.langchain._llm_client module.

Changes

LangChain client centralization and dynamic model resolution

Layer / File(s) Summary
Model resolution helper and provider client construction
tools/llm_provider.py
Adds _configured_langchain_model(provider, *, fallback) that optionally queries tools.llm_registry.configured_model_for_provider and falls back to the provided string. Updates OpenAIProvider._get_client and AnthropicProvider._get_client to use build_chat_client from tools.langchain_client with the resolved model, storing it on self._model_name. Changes analyze_completion in both providers to return self._model_name or the registry lookup instead of a hard-coded model string.
progress_reviewer import switch
scripts/langchain/progress_reviewer.py
Replaces the conditional import of build_chat_client from tools.langchain_client with a conditional import of build_client from scripts.langchain._llm_client, updating the resolved construction call to match.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • stranske/Template#841: Introduces configured_model_for_provider in tools/llm_registry and the updated build_chat_client behavior in tools/langchain_client that this PR depends on directly.
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning The PR title 'chore: sync workflow templates' does not match the actual changes, which involve updating LLM client imports and model resolution logic in progress_reviewer.py and llm_provider.py files—not workflow templates. Update the title to accurately reflect the main changes, such as 'chore: update LLM client resolution and model name handling' or 'refactor: consolidate LLM client configuration'.
Docstring Coverage ⚠️ Warning Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
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 sync/workflows-1243c652ce3c

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

@agents-workflows-bot

agents-workflows-bot Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Automated Status Summary

Head SHA: 87095a5
Latest Runs: ⏳ pending — Gate
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 100.00%
Baseline 0.00%
Delta +100.00%
Minimum 70.00%
Status ✅ Pass

Top Coverage Hotspots (lowest coverage)

File Coverage Missing
src/my_project/__init__.py 100.0% 0

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

@stranske

Copy link
Copy Markdown
Owner Author

Closing as superseded by the newer post-Workflows#2501 sync wave sync/workflows-c11e0664a4d8.

@stranske stranske closed this Jun 22, 2026
@stranske
stranske deleted the sync/workflows-1243c652ce3c branch June 22, 2026 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automated Automated sync from Workflows sync Automated sync from Workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant