Skip to content

chore: sync workflow templates #869

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

Closed
stranske wants to merge 1 commit into from
Closed

chore: sync workflow templates #869

Show file tree
Hide file tree
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
6 changes: 3 additions & 3 deletions scripts/langchain/progress_reviewer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,11 +427,11 @@ def review_progress_with_llm(
rounds_without_completion,
)
try:
from tools.langchain_client import build_chat_client
from scripts.langchain._llm_client import build_client
except ImportError:
build_chat_client = None
build_client = None

resolved = build_chat_client(model=model) if build_chat_client else None
resolved = build_client(model=model) if build_client else None
if not resolved:
score, aligned, unaligned = heuristic_alignment_check(
acceptance_criteria, recent_commits, files_changed
Expand Down
50 changes: 34 additions & 16 deletions tools/llm_provider.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,14 @@
SHORT_ANALYSIS_CONFIDENCE_CAP = 0.4


def _configured_langchain_model(provider: str, *, fallback: str) -> str:
try:
from tools.llm_registry import configured_model_for_provider
except ImportError:
return fallback
return configured_model_for_provider(provider, fallback=fallback) or fallback


def _setup_langsmith_tracing() -> bool:
"""
Configure LangSmith tracing if API key is available.
Expand Down Expand Up @@ -585,16 +593,17 @@ def supports_quality_context(self) -> bool:
def _get_client(self):
"""Get LangChain ChatOpenAI client."""
try:
from langchain_openai import ChatOpenAI
from tools.langchain_client import build_chat_client
except ImportError:
logger.warning("langchain_openai not installed")
logger.warning("LangChain client helper not available")
return None

return ChatOpenAI(
model="gpt-5.1-codex", # Purpose-built for analyzing Codex coding sessions
api_key=os.environ.get("OPENAI_API_KEY"),
temperature=0.1,
)
model_name = _configured_langchain_model("openai", fallback="gpt-5.1-codex")
resolved = build_chat_client(provider="openai", model=model_name)
if resolved:
self._model_name = resolved.model
return resolved.client
return None

def analyze_completion(
self,
Expand Down Expand Up @@ -626,7 +635,11 @@ def analyze_completion(
confidence=result.confidence,
reasoning=result.reasoning,
provider_used=self.name,
model_name="gpt-5.1-codex", # Actual model used by OpenAIProvider
model_name=getattr(
self,
"_model_name",
_configured_langchain_model("openai", fallback="gpt-5.1-codex"),
),
Comment on lines +638 to +642

@coderabbitai coderabbitai Bot Jun 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Verify call sites using eager default expression
rg -n 'getattr\(\s*self,\s*"_model_name",\s*_configured_langchain_model' tools/llm_provider.py

# Demonstrate Python eager evaluation of function-call args (language behavior)
python - <<'PY'
def fallback():
    print("fallback evaluated")
    return "fallback"

class Obj:
    _model_name = "already-set"

print(getattr(Obj(), "_model_name", fallback()))
PY

Repository: stranske/Collab-Admin

Length of output: 47

🏁 Script executed:

#!/bin/bash
# List files to understand repo structure
git ls-files | head -20

# Look for llm_provider files
find . -name "*llm_provider*" -o -name "*langchain*" | head -20

Repository: stranske/Collab-Admin

Length of output: 996

🏁 Script executed:

#!/bin/bash
# Check if tools directory exists
ls -la tools/ 2>/dev/null || echo "tools/ does not exist"

# List Python files in root and common locations
find . -maxdepth 3 -name "*.py" -type f | grep -E "(llm|provider|langchain)" | head -20

Repository: stranske/Collab-Admin

Length of output: 1795

🏁 Script executed:

#!/bin/bash
# Read the specific lines mentioned in the review
echo "=== Lines 638-642 ==="
sed -n '638,642p' tools/llm_provider.py

echo ""
echo "=== Lines 719-723 ==="
sed -n '719,723p' tools/llm_provider.py

echo ""
echo "=== Check for _configured_langchain_model definition ==="
grep -n "_configured_langchain_model" tools/llm_provider.py | head -20

Repository: stranske/Collab-Admin

Length of output: 1050

🏁 Script executed:

#!/bin/bash
# Look at the context around line 641 to understand when _model_name gets set
echo "=== Context around line 641 (OpenAI) ==="
sed -n '595,650p' tools/llm_provider.py

echo ""
echo "=== Context around line 722 (Anthropic) ==="
sed -n '665,730p' tools/llm_provider.py

Repository: stranske/Collab-Admin

Length of output: 4903

🏁 Script executed:

#!/bin/bash
# Look at the _configured_langchain_model function definition
echo "=== _configured_langchain_model function ==="
sed -n '45,80p' tools/llm_provider.py

Repository: stranske/Collab-Admin

Length of output: 1367

🏁 Script executed:

#!/bin/bash
# Check how often analyze_completion is called and confirm _model_name flow
echo "=== Check if _model_name is always set before getattr call ==="
sed -n '620,645p' tools/llm_provider.py

# Count how many times analyze_completion is called in the codebase
echo ""
echo "=== Call sites of analyze_completion ==="
rg -n "\.analyze_completion\(" --type py | head -20

Repository: stranske/Collab-Admin

Length of output: 1527

🏁 Script executed:

#!/bin/bash
# Trace the exact control flow - when is analyze_completion called?
echo "=== Callers of analyze_completion in llm_provider.py ==="
sed -n '10,25p' tools/llm_provider.py

echo ""
echo "=== Line 935 context ==="
sed -n '930,945p' tools/llm_provider.py

echo ""
echo "=== Line 949 context ==="
sed -n '945,955p' tools/llm_provider.py

Repository: stranske/Collab-Admin

Length of output: 1672

🏁 Script executed:

#!/bin/bash
# Verify that _model_name IS always set before analyze_completion returns CompletionAnalysis
echo "=== Full analyze_completion flow for OpenAI ==="
sed -n '612,655p' tools/llm_provider.py

# Check if there's any path where analyze_completion exits before setting _model_name
echo ""
echo "=== Check exception handling path ==="
sed -n '650,660p' tools/llm_provider.py

Repository: stranske/Collab-Admin

Length of output: 2053

🏁 Script executed:

#!/bin/bash
# Demonstrate the eager evaluation behavior with the exact code pattern
python3 << 'PY'
def expensive_fallback():
    print("WARNING: expensive_fallback was evaluated")
    return "fallback_value"

class Provider:
    def __init__(self):
        self._model_name = "already_set"

# Current pattern - default is ALWAYS evaluated
print("=== Current getattr() pattern ===")
provider = Provider()
result = getattr(provider, "_model_name", expensive_fallback())
print(f"Result: {result}")

print("\n=== Proposed pattern - lazy evaluation ===")
provider2 = Provider()
result2 = getattr(provider2, "_model_name", None) or expensive_fallback()
print(f"Result: {result2}")

# Verify the attribute IS set after _get_client
print("\n=== Checking if _model_name is always set when analyze_completion runs ===")
provider3 = Provider()
# Simulating: if _get_client() returns successfully, _model_name is already set
# Then analyze_completion calls getattr without needing the fallback
print(f"_model_name exists: {hasattr(provider3, '_model_name')}")
print(f"_model_name value: {getattr(provider3, '_model_name', 'NOT_SET')}")
PY

Repository: stranske/Collab-Admin

Length of output: 357

Optimize model name resolution to avoid unnecessary registry lookups.

The getattr() pattern evaluates _configured_langchain_model(...) as a default argument on every call to analyze_completion(), even though _model_name is guaranteed to be set by _get_client() before reaching this code. This causes redundant registry lookups on a frequently-called path.

Proposed fix 
-                model_name=getattr(
-                    self,
-                    "_model_name",
-                    _configured_langchain_model("openai", fallback="gpt-5.1-codex"),
-                ),
+                model_name=(
+                    getattr(self, "_model_name", None)
+                    or _configured_langchain_model("openai", fallback="gpt-5.1-codex")
+                ),
-                model_name=getattr(
-                    self,
-                    "_model_name",
-                    _configured_langchain_model("anthropic", fallback="claude-sonnet-4-5-20250929"),
-                ),
+                model_name=(
+                    getattr(self, "_model_name", None)
+                    or _configured_langchain_model(
+                        "anthropic", fallback="claude-sonnet-4-5-20250929"
+                    )
+                ),

Also applies to: 722–726

🤖 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/llm_provider.py` around lines 638 - 642, The analyze_completion()
method uses getattr() with a default parameter that calls
_configured_langchain_model() on every invocation, but since _model_name is
guaranteed to be set by _get_client() before analyze_completion() is reached,
this creates unnecessary registry lookups on a frequently-called path. Replace
the getattr() pattern with direct attribute access to self._model_name (around
line 638 and also at line 722-726 where the same pattern applies) since the
attribute is guaranteed to already exist, eliminating the redundant function
call and improving performance.

raw_confidence=result.raw_confidence,
confidence_adjusted=result.confidence_adjusted,
quality_warnings=result.quality_warnings,
Expand All @@ -651,16 +664,17 @@ def supports_quality_context(self) -> bool:

def _get_client(self):
try:
from langchain_anthropic import ChatAnthropic
from tools.langchain_client import build_chat_client
except ImportError:
logger.warning("langchain_anthropic not installed")
logger.warning("LangChain client helper not available")
return None

return ChatAnthropic(
model="claude-sonnet-4-5-20250929",
anthropic_api_key=os.environ.get(ANTHROPIC_API_KEY_ENV),
temperature=0.1,
)
model_name = _configured_langchain_model("anthropic", fallback="claude-sonnet-4-5-20250929")
resolved = build_chat_client(provider="anthropic", model=model_name)
if resolved:
self._model_name = resolved.model
return resolved.client
return None

def analyze_completion(
self,
Expand Down Expand Up @@ -702,7 +716,11 @@ def analyze_completion(
confidence=result.confidence,
reasoning=result.reasoning,
provider_used=self.name,
model_name="claude-sonnet-4-5-20250929",
model_name=getattr(
self,
"_model_name",
_configured_langchain_model("anthropic", fallback="claude-sonnet-4-5-20250929"),
),
raw_confidence=result.raw_confidence,
confidence_adjusted=result.confidence_adjusted,
quality_warnings=result.quality_warnings,
Expand Down
Loading