diff --git a/.env.example b/.env.example index 1f46c6b..4a8d5be 100644 --- a/.env.example +++ b/.env.example @@ -152,7 +152,7 @@ DEFAULT_MODEL=bedrock/converse/nvidia.nemotron-nano-3-30b # ============================================================================= # Minimum Confidence # ============================================================================= -# REVIEW_MIN_CONFIDENCE=0.81 +# REVIEW_MIN_CONFIDENCE=0.5 # ============================================================================= diff --git a/CHANGELOG.md b/CHANGELOG.md index b136da0..ce79288 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,26 @@ ## [Unreleased] +## [2.0.1] - 2026-10-05 + +### Fixed + +- **code_review, supply_chain and scope refinement never found issues.** Every tool call (`read_file`, `search_literal`, `find_*`, `recall_memory`) failed inside the RLM sandbox with `This event loop is already running`. RLM agents now run off the event loop using `asyncio.to_thread()`, and tool calls are bridged back to the event loop via `asyncio.run_coroutine_threadsafe()`. New module `agents/rlm_tools.py` provides `build_rlm_agent()`, `run_rlm()`, and `log_rlm_outcome()`. +- **Reviewers silently discarded issues without an explicit confidence**, because the `Issue.confidence` default (0.8) was below the `review.min_confidence` default (0.81). The code_review, doc and supply_chain prompts and the `Issue.confidence` field description now request a confidence ("≥0.9 verified with tools; 0.7–0.9 strong evidence; <0.7 weak"). The new `filter_by_confidence()` helper (`agents/review/helpers.py`) is used by all three reviewers and logs raw and kept counts per scope. +- **Opus 5.5 on Bedrock refused every RLM call with `content_filter`, so code_review returned nothing (`Empty LM response`).** The trigger was `dspy.RLM`'s stock `reasoning` field description ("Think step-by-step: what do you know? What remains? Plan your next action."). New `CodespyRLM` (`agents/context_safe.py`) replaces it with "Short plan for the next step." and is used by `build_rlm_agent()` and the `ContextSafe` RLM fallback. +- **RLM REPL steps all failed with an empty `Invalid Python syntax`.** dspy's `PythonInterpreter` injects inputs as `name = repr(value)`, and an Enum repr (``) is not valid Python, so every execution of code_review (`categories`) and supply_chain (`category`) failed before the model's code ran. `CodespyRLM` now passes Enum inputs as their plain values. +- **RLM tool calls with arguments failed in the sandbox** (`get_tree() got an unexpected keyword argument 'max_depth'`, `Tool 'read_file' rejected arguments: ['path']`). dspy.RLM copies each tool's Python signature into the sandbox, and the async-to-sync bridge exposed `sync_wrapper(**kwargs)`, so the sandbox wrapper accepted only a literal `kwargs` argument. `bridge_tools_to_loop()` now sets the bridged function's signature from the tool's args. + +### Added + +- **Empty LM response diagnostics.** `DiagnosticTwoStepAdapter` (`agents/diagnostic_adapter.py`) is now the default adapter in `configure_dspy()` and `lm_context()`, replacing `TwoStepAdapter`. When a response is empty or null, it logs the model, `max_tokens`, `reasoning_effort`, usage, `finish_reason`, reasoning/thinking blocks, tool calls and request size. A `content_filter` refusal raises `AdapterParseError("model refused (content_filter)")` and writes a dump to `/refusals/-.json`. Dumps contain the full prompt, including reviewed code. Only keys ending in `_key`, `_secret` or `_token` are redacted. +- **Startup config log.** The review pipeline logs the loaded config file path (new `config.get_loaded_config_path()`), `min_confidence`, and `max_iters`/`max_llm_calls` for code_review, doc and supply_chain. +- **RLM outcome logging.** `log_rlm_outcome()` warns when an RLM run ends in the "Extract forced final output" fallback after tool errors, with the error count and the first error. RLM `verbose` is enabled when the log level is DEBUG (`build_rlm_agent()` and the `ContextSafe` fallback). + +### Changed + +- **Default `review.min_confidence` (`REVIEW_MIN_CONFIDENCE`) is now 0.5 (was 0.81).** This allows issues to be reported even when the LLM does not explicitly set a confidence score (the `Issue.confidence` default is 0.8, which now passes the threshold). `codespy.yaml`, `.env.example` and the action `min-confidence` description are updated. Override it in codespy.yaml or with the environment variable if needed. + ## [2.0.0] - 2026-10-03 ### Changed (Breaking) diff --git a/action.yml b/action.yml index b43c252..71fdfbb 100644 --- a/action.yml +++ b/action.yml @@ -106,7 +106,7 @@ inputs: required: false min-confidence: - description: 'Minimum confidence threshold (0.0-1.0) for reported issues. Default from codespy.yaml: 0.81' + description: 'Minimum confidence threshold (0.0-1.0) for reported issues. Default from codespy.yaml: 0.5' required: false # ========================================== diff --git a/codespy.yaml b/codespy.yaml index 3e29285..52695cf 100644 --- a/codespy.yaml +++ b/codespy.yaml @@ -162,7 +162,7 @@ memory: # REVIEW # ============================================================================ review: - min_confidence: 0.81 # REVIEW_MIN_CONFIDENCE + min_confidence: 0.5 # REVIEW_MIN_CONFIDENCE output_format: markdown # REVIEW_OUTPUT_FORMAT output_stdout: true # REVIEW_OUTPUT_STDOUT output_git: true # REVIEW_OUTPUT_GIT diff --git a/pyproject.toml b/pyproject.toml index d37015e..9e10fd9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "codespy-ai" -version = "2.0.0" +version = "2.0.1" description = "Code review agent powered by DSPy" readme = "README.md" license = "MIT" diff --git a/src/codespy/__init__.py b/src/codespy/__init__.py index b783fe4..2452495 100644 --- a/src/codespy/__init__.py +++ b/src/codespy/__init__.py @@ -1,3 +1,3 @@ """codespy - Code review agent powered by DSPy.""" -__version__ = "2.0.0" +__version__ = "2.0.1" diff --git a/src/codespy/agents/context_safe.py b/src/codespy/agents/context_safe.py index 0e5b720..8e6f0a9 100644 --- a/src/codespy/agents/context_safe.py +++ b/src/codespy/agents/context_safe.py @@ -4,6 +4,7 @@ exceed the model's context window. Used to wrap all DSPy signature modules. """ +import enum import logging import re @@ -19,6 +20,47 @@ # Below this, context rot is not a concern regardless of ratio. _MIN_RLM_THRESHOLD_TOKENS = 8192 +# Replaces dspy.RLM's stock `reasoning` desc ("Think step-by-step: what do you +# know? What remains? Plan your next action."), which on its own triggers Bedrock +# content_filter on Claude Opus 5.5. +RLM_REASONING_DESC = "Short plan for the next step." + + +def _plain(value): # type: ignore[no-untyped-def] + """Replace Enum members with their values, recursively. + + dspy's PythonInterpreter injects inputs as `name = repr(value)`; an Enum repr + (``) is not valid Python, so every REPL execution + fails with an empty SyntaxError. + """ + if isinstance(value, enum.Enum): + return value.value + if isinstance(value, (list, tuple, set)): + return [_plain(v) for v in value] + if isinstance(value, dict): + return {_plain(k): _plain(v) for k, v in value.items()} + return value + + +class CodespyRLM(dspy.RLM): + """dspy.RLM adjusted for codespy. + + - Neutral action-signature `reasoning` description (Bedrock content_filter). + - Enum inputs passed as plain values (REPL variable injection). + """ + + def forward(self, **kwargs): # type: ignore[no-untyped-def] + return super().forward(**{k: _plain(v) for k, v in kwargs.items()}) + + async def aforward(self, **kwargs): # type: ignore[no-untyped-def] + return await super().aforward(**{k: _plain(v) for k, v in kwargs.items()}) + + def _build_signatures(self): # type: ignore[no-untyped-def] + action_sig, extract_sig = super()._build_signatures() + if "reasoning" in action_sig.output_fields: + action_sig = action_sig.with_updated_fields("reasoning", desc=RLM_REASONING_DESC) + return action_sig, extract_sig + def estimate_context_overflow(model: str, max_tokens: int, input_text: str) -> bool: """Estimate whether input + max_tokens would exceed the model's context window. @@ -197,9 +239,10 @@ def _get_current_signature(self): def _create_rlm_fallback(self) -> dspy.RLM: """Create RLM with current signature and tools.""" - return dspy.RLM( + return CodespyRLM( self._get_current_signature(), tools=self._tools, max_iters=self._max_iters, max_llm_calls=self._max_llm_calls, + verbose=logger.isEnabledFor(logging.DEBUG), ) diff --git a/src/codespy/agents/diagnostic_adapter.py b/src/codespy/agents/diagnostic_adapter.py new file mode 100644 index 0000000..b4a6247 --- /dev/null +++ b/src/codespy/agents/diagnostic_adapter.py @@ -0,0 +1,328 @@ +"""Diagnostic adapter for logging empty LM response details. + +Wraps TwoStepAdapter to capture diagnostics when the main LM returns an empty +or null response, which helps diagnose why Claude occasionally returns empty +text with hidden thinking content. +""" + +import json +import logging +from datetime import datetime, timezone +from pathlib import Path +from typing import Any + +import dspy # type: ignore[import-untyped] +from dspy.adapters.two_step_adapter import TwoStepAdapter # type: ignore[import-untyped] +from dspy.adapters.base import AdapterParseError # type: ignore[import-untyped] + +from codespy.config import get_settings + +logger = logging.getLogger(__name__) + + +def _getattr_or_key(obj: Any, name: str, default: Any = None) -> Any: + """Get attribute or dict key, tolerating both access patterns.""" + if hasattr(obj, name): + return getattr(obj, name, default) + if isinstance(obj, dict): + return obj.get(name, default) + return default + + +def _safe_repr(value: Any, max_len: int = 500) -> str: + """Safe repr with length limit.""" + try: + s = repr(value) + if len(s) > max_len: + return s[:max_len] + f"... ({len(s)} chars total)" + return s + except Exception: + return "" + + +def _get_finish_reason(entry: dict) -> str: + """Extract finish_reason from history entry.""" + resp = entry.get("response") + if resp is None: + return "unknown" + + resp_class_name = type(resp).__name__ + if resp_class_name == "Response": + # lm15 style Response object + return _getattr_or_key(resp, "finish_reason", "unknown") + else: + # Standard dict-style response + choices = _getattr_or_key(resp, "choices", []) + if choices and len(choices) > 0: + choice = choices[0] + return _getattr_or_key(choice, "finish_reason", "unknown") + return "unknown" + + +def _write_refusal_dump(entry: dict, model: str, lm_kwargs: dict, signature_name: str) -> Path | None: + """Write a refusal dump to disk for later analysis. + + Args: + entry: The history entry containing messages and response. + model: The model name. + lm_kwargs: The LM kwargs (will have API keys stripped). + signature_name: The signature name for the dump filename. + + Returns: + Path to the written dump file, or None if writing failed. + """ + try: + settings = get_settings() + cache_dir = Path(settings.review.cache_dir).expanduser().resolve() + refusals_dir = cache_dir / "refusals" + refusals_dir.mkdir(parents=True, exist_ok=True) + + timestamp = datetime.now(timezone.utc).strftime("%Y%m%dT%H%M%S") + safe_sig = signature_name.replace("/", "_").replace("\\", "_")[:50] + dump_path = refusals_dir / f"{timestamp}-{safe_sig}.json" + + # Strip API keys from lm_kwargs + safe_kwargs = {} + for key, value in lm_kwargs.items(): + lower_key = key.lower() + # Redact actual secret keys, but not legitimate settings like max_tokens + if lower_key.endswith("_key") or lower_key.endswith("_secret") or lower_key.endswith("_token") or lower_key == "api_key": + safe_kwargs[key] = "" + else: + safe_kwargs[key] = value + + dump = { + "model": model, + "lm_kwargs": safe_kwargs, + "messages": entry.get("messages", []), + "response": entry.get("response"), + } + + with open(dump_path, "w", encoding="utf-8") as f: + json.dump(dump, f, indent=2, default=str) + + logger.warning("Empty LM response: refusal dump written to %s", dump_path) + return dump_path + except Exception as e: + logger.debug("Empty LM response: failed to write refusal dump: %s", e) + return None + + +def _log_empty_response(lm: Any, signature: Any) -> None: + """Log detailed diagnostics for an empty LM response. + + Wraps everything in try/except so diagnostics can never mask the original error. + """ + try: + model = getattr(lm, "model", "unknown") + max_tokens = getattr(lm, "kwargs", {}).get("max_tokens") if hasattr(lm, "kwargs") else None + reasoning_effort = getattr(lm, "kwargs", {}).get("reasoning_effort") if hasattr(lm, "kwargs") else None + + # Get output field names from signature + output_fields: list[str] = [] + signature_name = "unknown" + if signature is not None: + try: + signature_name = getattr(signature, "__name__", type(signature).__name__) + if hasattr(signature, "output_fields"): + output_fields = list(getattr(signature, "output_fields", [])) + elif hasattr(signature, "fields"): + # Try to identify output fields from signature fields + fields = getattr(signature, "fields", {}) + if isinstance(fields, dict): + output_fields = [k for k, v in fields.items() if getattr(v, "is_input", False) is False] + except Exception: + pass + + logger.warning( + "Empty LM response: model=%s max_tokens=%s reasoning_effort=%s output_fields=%s", + model, max_tokens, reasoning_effort, output_fields + ) + + # Read from LM history + history = getattr(lm, "history", None) + if not history: + logger.warning("Empty LM response: no LM history available") + return + + try: + entry = history[-1] + except IndexError: + logger.warning("Empty LM response: LM history is empty") + return + + if not isinstance(entry, dict): + logger.warning("Empty LM response: unexpected history entry type %s", type(entry).__name__) + return + + # Log usage info + usage = entry.get("usage") + if usage: + prompt_tokens = _getattr_or_key(usage, "prompt_tokens") or _getattr_or_key(usage, "input_tokens", 0) + completion_tokens = _getattr_or_key(usage, "completion_tokens") or _getattr_or_key(usage, "output_tokens", 0) + reasoning_tokens = _getattr_or_key(usage, "reasoning_tokens", 0) + logger.warning( + "Empty LM response: usage prompt_tokens=%s completion_tokens=%s reasoning_tokens=%s", + prompt_tokens, completion_tokens, reasoning_tokens + ) + else: + logger.warning("Empty LM response: no usage info in history entry") + + # Log response details + resp = entry.get("response") + if resp is None: + logger.warning("Empty LM response: no response object in history entry") + return + + # Handle lm15 Response object vs dict + resp_class_name = type(resp).__name__ + if resp_class_name == "Response": + # lm15 style Response object + finish_reason = _getattr_or_key(resp, "finish_reason", "unknown") + message = _getattr_or_key(resp, "message", None) + parts_info: list[str] = [] + if message and hasattr(message, "parts"): + parts = getattr(message, "parts", []) + parts_info = [type(p).__name__ for p in parts] + usage_obj = _getattr_or_key(resp, "usage", None) + logger.warning( + "Empty LM response: lm15 Response finish_reason=%s message_parts=%s usage=%s", + finish_reason, parts_info, usage_obj + ) + else: + # Standard dict-style response + choices = _getattr_or_key(resp, "choices", []) + if choices and len(choices) > 0: + choice = choices[0] + finish_reason = _getattr_or_key(choice, "finish_reason", "unknown") + message = _getattr_or_key(choice, "message", {}) + content = _getattr_or_key(message, "content", "") + + logger.warning( + "Empty LM response: finish_reason=%s content=%s", + finish_reason, _safe_repr(content, 500) + ) + + # Check for reasoning_content (Anthropic) + reasoning_content = _getattr_or_key(message, "reasoning_content", None) + if reasoning_content: + rc_len = len(str(reasoning_content)) + logger.warning( + "Empty LM response: reasoning_content len=%s first_300=%s", + rc_len, _safe_repr(str(reasoning_content)[:300], 300) + ) + else: + # Check for thinking_blocks + thinking_blocks = _getattr_or_key(message, "thinking_blocks", None) + if thinking_blocks: + block_types = [type(b).__name__ for b in thinking_blocks] + logger.warning( + "Empty LM response: thinking_blocks count=%s types=%s", + len(thinking_blocks), block_types + ) + else: + logger.warning("Empty LM response: no reasoning_content or thinking_blocks") + + # Check for tool_calls + tool_calls = _getattr_or_key(message, "tool_calls", None) + if tool_calls: + logger.warning("Empty LM response: tool_calls count=%s", len(tool_calls)) + else: + logger.warning("Empty LM response: no tool_calls") + + # Log provider_specific_fields if present + provider_fields = _getattr_or_key(message, "provider_specific_fields", None) + if provider_fields: + logger.warning("Empty LM response: provider_specific_fields keys=%s", list(provider_fields.keys()) if isinstance(provider_fields, dict) else type(provider_fields).__name__) + else: + logger.warning("Empty LM response: no choices in response") + + # Log request size + messages = entry.get("messages", []) + request_size = len(str(messages)) + logger.warning("Empty LM response: request_size_chars=%s", request_size) + + # If finish_reason is content_filter, write a refusal dump + finish_reason = _get_finish_reason(entry) + if finish_reason == "content_filter": + lm_kwargs = getattr(lm, "kwargs", {}) if hasattr(lm, "kwargs") else {} + _write_refusal_dump(entry, model, lm_kwargs, signature_name) + + except Exception as diag_exc: + # Never let diagnostics mask the original error + logger.debug("Empty LM response: diagnostic logging failed: %s", diag_exc, exc_info=True) + + +def _get_finish_reason_from_lm(lm: Any) -> str | None: + """Extract finish_reason from LM history entry.""" + history = getattr(lm, "history", None) + if not history: + return None + try: + entry = history[-1] + if not isinstance(entry, dict): + return None + return _get_finish_reason(entry) + except Exception: + return None + + +class DiagnosticTwoStepAdapter(TwoStepAdapter): + """TwoStepAdapter that logs diagnostics on empty LM responses. + + Wraps the parent's __call__ and __acall__ to catch AdapterParseError for empty/null + responses and log detailed diagnostics before re-raising the original + exception unchanged. + """ + + def _check_and_log_empty_response( + self, exc: AdapterParseError, lm: dspy.LM, signature: Any + ) -> None: + """Check if exception is for empty response and log diagnostics. + + If finish_reason is content_filter, raises a more descriptive error. + """ + # Only log for empty/null response errors + if "empty or null response" in str(exc).lower(): + _log_empty_response(lm, signature) + # If finish_reason is content_filter, raise a more descriptive error + finish_reason = _get_finish_reason_from_lm(lm) + if finish_reason == "content_filter": + raise AdapterParseError( + adapter_name=getattr(self, "__class__.__name__", "DiagnosticTwoStepAdapter"), + signature=signature, + lm_response="", + message="model refused (content_filter)", + ) from exc + + def __call__( + self, + lm: dspy.LM, + lm_kwargs: dict[str, Any], + signature: Any, + demos: list[dict[str, Any]], + inputs: dict[str, Any], + ) -> dict[str, Any]: + """Call parent adapter, logging diagnostics on empty response.""" + try: + return super().__call__(lm, lm_kwargs, signature, demos, inputs) + except AdapterParseError as exc: + self._check_and_log_empty_response(exc, lm, signature) + # Always re-raise the original exception unchanged + raise + + async def acall( + self, + lm: dspy.LM, + lm_kwargs: dict[str, Any], + signature: Any, + demos: list[dict[str, Any]], + inputs: dict[str, Any], + ) -> dict[str, Any]: + """Async call parent adapter, logging diagnostics on empty response.""" + try: + return await super().acall(lm, lm_kwargs, signature, demos, inputs) + except AdapterParseError as exc: + self._check_and_log_empty_response(exc, lm, signature) + # Always re-raise the original exception unchanged + raise diff --git a/src/codespy/agents/dspy_config.py b/src/codespy/agents/dspy_config.py index dbdc2a9..b279378 100644 --- a/src/codespy/agents/dspy_config.py +++ b/src/codespy/agents/dspy_config.py @@ -7,8 +7,8 @@ import dspy # type: ignore[import-untyped] import litellm # type: ignore[import-untyped] -from dspy.adapters.two_step_adapter import TwoStepAdapter # type: ignore[import-untyped] +from codespy.agents.diagnostic_adapter import DiagnosticTwoStepAdapter from codespy.config import Settings, get_settings from codespy.config_memory import MEMORY_CONSOLIDATION, MEMORY_MENTAL_MODELS, MEMORY_RECALL, MEMORY_RETAIN, REFLECTION_MODULES, LLMSettings from codespy.config_utils import secret_value @@ -216,7 +216,7 @@ def lm_context(name: str) -> LMScope: llm_config = settings.get_llm_config(name) lm = new_lm(settings, llm_config) extraction_lm = new_extraction_lm(settings) - ctx = dspy.context(lm=lm, adapter=TwoStepAdapter(extraction_lm)) + ctx = dspy.context(lm=lm, adapter=DiagnosticTwoStepAdapter(extraction_lm)) return LMScope(ctx=ctx, lm=lm, extraction_lm=extraction_lm) @@ -270,7 +270,7 @@ def configure_dspy(settings: Settings) -> None: dspy.settings.configure( lm=lm, - adapter=TwoStepAdapter(extraction_lm), # TwoStepAdapter solves ChatAdapter parsing failures + adapter=DiagnosticTwoStepAdapter(extraction_lm), # TwoStepAdapter solves ChatAdapter parsing failures ) # Enable memory-only caching for LLM calls (no disk caching) diff --git a/src/codespy/agents/review/code_review/agent.py b/src/codespy/agents/review/code_review/agent.py index 4a2a88d..f22adcd 100644 --- a/src/codespy/agents/review/code_review/agent.py +++ b/src/codespy/agents/review/code_review/agent.py @@ -14,14 +14,16 @@ from codespy.agents.memory.hippocampus import ContextMemory, Hippocampus from codespy.agents.memory.hippocampus.episode import submit_episode_save from codespy.agents.memory.prefrontal import with_prefrontal_memory -from codespy.agents.review.models import Issue, IssueCategory, ReviewContext -from codespy.agents.review.scope.models import ScopeResult from codespy.agents.review.helpers import ( + filter_by_confidence, issues_to_markdown, make_scope_relative, resolve_scope_root, restore_repo_paths, ) +from codespy.agents.review.models import Issue, IssueCategory, ReviewContext +from codespy.agents.review.scope.models import ScopeResult +from codespy.agents.rlm_tools import build_rlm_agent, log_rlm_outcome, run_rlm from codespy.config import get_settings from codespy.config_memory import get_cerebral, get_episode_store, get_prefrontal from codespy.tools.mcp_utils import cleanup_mcp_contexts, connect_mcp_server @@ -129,6 +131,7 @@ class CodeReviewSignature(dspy.Signature): - description: ≤25 words, imperative tone, no filler ("Fix X", "Rename Y to Z") - No polite or conversational language - Do not populate code_snippet — use line numbers instead + - Always set confidence: ≥0.9 verified with tools; 0.7–0.9 strong evidence; <0.7 weak """ scope: ScopeResult = dspy.InputField( @@ -282,15 +285,9 @@ async def _review_scope( sig = with_prefrontal_memory(CodeReviewSignature) if review_context.prefrontal_memory else CodeReviewSignature recall_tool = pf.recall_tool() if pf else None agent_tools = [*tools, recall_tool] if recall_tool else tools - agent = ContextSafe( - dspy.RLM( - sig, - tools=agent_tools, - max_iters=max_iters, - max_llm_calls=self._settings.get_max_llm_calls("code_review"), - ), + agent = build_rlm_agent( sig, - tools=agent_tools, + agent_tools, name="code_review", max_iters=max_iters, max_llm_calls=self._settings.get_max_llm_calls("code_review"), @@ -336,16 +333,11 @@ async def _review_scope( initial_memory=scope_initial_memory, topics=topics, ) - result = await agent.acall( - scope=scoped, - categories=categories, - **pf_kwargs, + result = await run_rlm(agent, scope=scoped, categories=categories, **pf_kwargs) + log_rlm_outcome(result, "code_review", scope.subroot) + issues = filter_by_confidence( + result.issues, self._settings.review.min_confidence, "code_review", scope.subroot ) - issues = [ - issue - for issue in (result.issues or []) - if issue.confidence >= self._settings.review.min_confidence - ] if hippo is not None: await hippo.aobserve(result) # Fire-and-forget background episode save diff --git a/src/codespy/agents/review/doc/agent.py b/src/codespy/agents/review/doc/agent.py index db00803..f7ffa3a 100644 --- a/src/codespy/agents/review/doc/agent.py +++ b/src/codespy/agents/review/doc/agent.py @@ -19,6 +19,7 @@ from codespy.agents.review.doc.doc_extractor import extract_documentation from codespy.agents.review.helpers import ( build_patches, + filter_by_confidence, issues_to_markdown, make_scope_relative, resolve_scope_root, @@ -78,6 +79,7 @@ class DocReviewSignature(dspy.Signature): - description: ≤25 words, imperative tone ("Update X section", "Add Y to README") - Empty list if documentation is up to date. No approval text ("LGTM", "looks good") - No polite or conversational language + - Always set confidence: ≥0.9 verified with tools; 0.7–0.9 strong evidence; <0.7 weak """ patches: str = dspy.InputField( @@ -261,11 +263,9 @@ async def _review_scope( categories=[IssueCategory.DOCUMENTATION], **pf_kwargs, ) - issues = [ - issue - for issue in (result.issues or []) - if issue.confidence >= self._settings.review.min_confidence - ] + issues = filter_by_confidence( + result.issues, self._settings.review.min_confidence, "doc", scope.subroot + ) if hippo is not None: await hippo.aobserve(result) # Fire-and-forget background episode save diff --git a/src/codespy/agents/review/helpers.py b/src/codespy/agents/review/helpers.py index 3dffd09..ae1f371 100644 --- a/src/codespy/agents/review/helpers.py +++ b/src/codespy/agents/review/helpers.py @@ -267,3 +267,36 @@ def scope_package_names(scope: ScopeResult | Any) -> list[str]: if manifest is not None and getattr(manifest, "package_name", None): return [manifest.package_name] return [] + + +def filter_by_confidence( + issues: list[Issue] | None, + min_confidence: float, + module: str, + scope_subroot: str, +) -> list[Issue]: + """Filter issues by confidence threshold with logging. + + Args: + issues: Raw issues list (may be None) + min_confidence: Minimum confidence threshold (0.0-1.0) + module: Module name for logging (e.g., 'code_review') + scope_subroot: Scope identifier for logging + + Returns: + Filtered issues with confidence >= min_confidence + """ + if issues is None: + logger.info("%s scope %s: 0 raw issues, 0 kept (min_confidence=%.2f)", + module, scope_subroot, min_confidence) + return [] + + raw_count = len(issues) + kept = [issue for issue in issues if issue.confidence >= min_confidence] + kept_count = len(kept) + + logger.info( + "%s scope %s: %d raw issues, %d kept (min_confidence=%.2f)", + module, scope_subroot, raw_count, kept_count, min_confidence + ) + return kept diff --git a/src/codespy/agents/review/models.py b/src/codespy/agents/review/models.py index 8845c53..261ebbf 100644 --- a/src/codespy/agents/review/models.py +++ b/src/codespy/agents/review/models.py @@ -128,7 +128,12 @@ class Issue(BaseModel): cwe_id: str | None = Field( default=None, description="CWE ID for security issues (e.g., CWE-79)" ) - confidence: float = Field(default=0.8, ge=0.0, le=1.0, description="Confidence score (0-1)") + confidence: float = Field( + default=0.8, + ge=0.0, + le=1.0, + description="Confidence score (0-1). ≥0.9 verified with tools; 0.7–0.9 strong evidence; <0.7 weak" + ) @property def location(self) -> str: diff --git a/src/codespy/agents/review/scope/agent.py b/src/codespy/agents/review/scope/agent.py index 1e1a85a..ddeefb7 100644 --- a/src/codespy/agents/review/scope/agent.py +++ b/src/codespy/agents/review/scope/agent.py @@ -18,13 +18,13 @@ from pydantic import BaseModel, Field from codespy.agents import SignatureContext, get_cost_tracker -from codespy.agents.context_safe import ContextSafe from codespy.agents.memory.hippocampus import ContextMemory, Hippocampus from codespy.agents.memory.hippocampus.episode import submit_episode_save from codespy.agents.memory.prefrontal import build_facets, with_prefrontal_memory from codespy.agents.review.models import ReviewContext from codespy.agents.review.scope.manifest_parser import extract_package_name from codespy.agents.review.scope.models import PackageManifest, ScopeResult, ScopeType +from codespy.agents.rlm_tools import build_rlm_agent, log_rlm_outcome, run_rlm from codespy.config import get_settings from codespy.config_memory import get_cerebral, get_episode_store, get_prefrontal from codespy.tools.git.client import get_client @@ -1060,15 +1060,9 @@ async def _refine_scopes( ) recall_tool = pf.recall_tool() if pf else None agent_tools = [*tools, recall_tool] if recall_tool else tools - agent = ContextSafe( - dspy.RLM( - sig, - tools=agent_tools, - max_iters=max_iters, - max_llm_calls=self._settings.get_max_llm_calls("scope"), - ), + agent = build_rlm_agent( sig, - tools=agent_tools, + agent_tools, name="scope", max_iters=max_iters, max_llm_calls=self._settings.get_max_llm_calls("scope"), @@ -1123,7 +1117,8 @@ async def _refine_scopes( run_id=run_id, initial_memory=scope_initial_memory, ) - result = await agent.acall( + result = await run_rlm( + agent, candidates=candidates_str, orphan_files=[f.filename for f in orphans], pr_title=pr.title or "No title", @@ -1131,6 +1126,7 @@ async def _refine_scopes( project_instructions=project_instructions, **pf_kwargs, ) + log_rlm_outcome(result, "scope", pr.repo_full_name) if hippo is not None: await hippo.aobserve(result) diff --git a/src/codespy/agents/review/supply_chain/agent.py b/src/codespy/agents/review/supply_chain/agent.py index 265efc8..1c2e721 100644 --- a/src/codespy/agents/review/supply_chain/agent.py +++ b/src/codespy/agents/review/supply_chain/agent.py @@ -10,18 +10,19 @@ import dspy # type: ignore[import-untyped] from codespy.agents import SignatureContext, get_cost_tracker -from codespy.agents.context_safe import ContextSafe from codespy.agents.memory.hippocampus import ContextMemory, Hippocampus from codespy.agents.memory.hippocampus.episode import submit_episode_save from codespy.agents.memory.prefrontal import with_prefrontal_memory -from codespy.agents.review.models import Issue, IssueCategory, ReviewContext -from codespy.agents.review.scope.models import ScopeResult from codespy.agents.review.helpers import ( + filter_by_confidence, issues_to_markdown, resolve_scope_root, restore_repo_paths, strip_prefix, ) +from codespy.agents.review.models import Issue, IssueCategory, ReviewContext +from codespy.agents.review.scope.models import ScopeResult +from codespy.agents.rlm_tools import build_rlm_agent, log_rlm_outcome, run_rlm from codespy.config import get_settings from codespy.config_memory import get_cerebral, get_episode_store, get_prefrontal from codespy.tools.mcp_utils import cleanup_mcp_contexts, connect_mcp_server @@ -70,6 +71,7 @@ class SupplyChainSecuritySignature(dspy.Signature): - No polite or conversational language ("I suggest", "Please consider", "Great"). - Do not populate code_snippet—use line numbers instead. - File paths in issues must be relative to scope root. + - Always set confidence: ≥0.9 verified with tools; 0.7–0.9 strong evidence; <0.7 weak ## 2. DEPENDENCY SECURITY (package manifests) @@ -385,15 +387,9 @@ async def _review_scope( recall_tool = pf.recall_tool() if pf else None if recall_tool: all_tools = [*all_tools, recall_tool] - supply_chain_agent = ContextSafe( - dspy.RLM( - sig, - tools=all_tools, - max_iters=supply_chain_max_iters, - max_llm_calls=self._settings.get_max_llm_calls("supply_chain"), - ), + supply_chain_agent = build_rlm_agent( sig, - tools=all_tools, + all_tools, name="supply_chain", max_iters=supply_chain_max_iters, max_llm_calls=self._settings.get_max_llm_calls("supply_chain"), @@ -441,18 +437,18 @@ async def _review_scope( initial_memory=scope_initial_memory, topics=topics, ) - result = await supply_chain_agent.acall( + result = await run_rlm( + supply_chain_agent, manifest_path=manifest_path, lock_file_path=lock_file_path, package_manager=package_manager, category=IssueCategory.SECURITY, **pf_kwargs, ) - issues = [ - issue - for issue in result.issues - if issue.confidence >= self._settings.review.min_confidence - ] + log_rlm_outcome(result, "supply_chain", scope.subroot) + issues = filter_by_confidence( + result.issues, self._settings.review.min_confidence, "supply_chain", scope.subroot + ) if hippo is not None: await hippo.aobserve(result) # Fire-and-forget background episode save diff --git a/src/codespy/agents/rlm_tools.py b/src/codespy/agents/rlm_tools.py new file mode 100644 index 0000000..0086035 --- /dev/null +++ b/src/codespy/agents/rlm_tools.py @@ -0,0 +1,330 @@ +"""RLM tool bridge and agent factory for async tool support. + +This module bridges async MCP tools to sync RLM agents running off the event loop, +preventing 'This event loop is already running' errors when tools are invoked. +""" + +from __future__ import annotations + +import asyncio +import inspect +import logging +import threading +from concurrent.futures import TimeoutError as FutureTimeoutError +from typing import Any + +import dspy # type: ignore[import-untyped] + +from codespy.agents.context_safe import CodespyRLM, ContextSafe + +logger = logging.getLogger(__name__) + +# Thread id of the event loop at bridge time +_bridge_thread_id: int | None = None + + +def _is_coroutine_function(obj: Any) -> bool: + """Check if an object is a coroutine function or callable that returns a coroutine.""" + if inspect.iscoroutinefunction(obj): + return True + # Handle dspy.Tool with async func + if hasattr(obj, "func"): + return inspect.iscoroutinefunction(obj.func) + return False + + +def _get_tool_name(tool: Any) -> str: + """Extract name from a tool object.""" + if hasattr(tool, "name"): + return str(tool.name) + if hasattr(tool, "__name__"): + return str(tool.__name__) + return str(type(tool).__name__) + + +def _get_tool_desc(tool: Any) -> str | None: + """Extract description from a tool object.""" + if hasattr(tool, "desc"): + return str(tool.desc) if tool.desc is not None else None + if hasattr(tool, "__doc__"): + return str(tool.__doc__) if tool.__doc__ is not None else None + return None + + +def _get_tool_args(tool: Any) -> dict[str, Any] | None: + """Extract args from a tool object.""" + if hasattr(tool, "args"): + return tool.args # type: ignore[return-value] + return None + + +def _get_tool_arg_types(tool: Any) -> dict[str, type] | None: + """Extract arg_types from a tool object.""" + if hasattr(tool, "arg_types"): + return tool.arg_types # type: ignore[return-value] + return None + + +def _get_tool_arg_desc(tool: Any) -> dict[str, str] | None: + """Extract arg_desc from a tool object.""" + if hasattr(tool, "arg_desc"): + return tool.arg_desc # type: ignore[return-value] + return None + + +def _signature_from_args( + args: dict[str, Any] | None, arg_types: dict[str, Any] | None +) -> inspect.Signature: + """Build a real signature from dspy.Tool args (JSON schema per arg). + + dspy.RLM copies `inspect.signature(tool.func)` into the sandbox wrapper. A bare + `sync_wrapper(**kwargs)` becomes `def tool(kwargs):` there, so keyword calls + fail ("unexpected keyword argument") and positional ones arrive as `kwargs=`. + """ + required: list[inspect.Parameter] = [] + optional: list[inspect.Parameter] = [] + for name, schema in (args or {}).items(): + annotation = (arg_types or {}).get(name, inspect.Parameter.empty) + if isinstance(schema, dict) and "default" in schema: + optional.append( + inspect.Parameter( + name, inspect.Parameter.POSITIONAL_OR_KEYWORD, + default=schema["default"], annotation=annotation, + ) + ) + else: + required.append( + inspect.Parameter(name, inspect.Parameter.POSITIONAL_OR_KEYWORD, annotation=annotation) + ) + return inspect.Signature(required + optional) + + +def bridge_tools_to_loop( + tools: list[Any], + loop: asyncio.AbstractEventLoop, + timeout: float, +) -> list[dspy.Tool]: + """Bridge async tools to run on the provided event loop from sync contexts. + + For each tool: + - If already a dspy.Tool with sync func: pass through unchanged + - If a dspy.Tool with async func: wrap with sync bridge + - If a plain callable: wrap in dspy.Tool, then apply above + + Args: + tools: List of tools (dspy.Tool or plain callables) + loop: The event loop to schedule async tool calls on + timeout: Timeout in seconds for tool calls + + Returns: + List of bridged dspy.Tool instances with sync wrappers + """ + global _bridge_thread_id + _bridge_thread_id = threading.get_ident() + + bridged: list[dspy.Tool] = [] + + for tool in tools: + # Normalize to dspy.Tool + if isinstance(tool, dspy.Tool): + dspy_tool = tool + else: + # Wrap plain callable in dspy.Tool + dspy_tool = dspy.Tool(tool) + + # Get the underlying function + func = dspy_tool.func + + # Check if async + if not _is_coroutine_function(func): + # Sync tool - pass through unchanged + bridged.append(dspy_tool) + continue + + # Async tool - create sync wrapper that schedules on the loop + tool_name = _get_tool_name(dspy_tool) + tool_desc = _get_tool_desc(dspy_tool) + tool_args = _get_tool_args(dspy_tool) + tool_arg_types = _get_tool_arg_types(dspy_tool) + tool_arg_desc = _get_tool_arg_desc(dspy_tool) + + def make_sync_wrapper( + async_func: Any, + tool_loop: asyncio.AbstractEventLoop, + tool_timeout: float, + name: str, + ) -> Any: + """Create a sync wrapper for an async function.""" + + def sync_wrapper(**kwargs: Any) -> Any: + # Deadlock guard: if we're on the bridge thread, raise + if threading.get_ident() == _bridge_thread_id: + raise RuntimeError( + "RLM tools must run off the event loop; use run_rlm()" + ) + + # Schedule the coroutine on the loop and wait for result + future = asyncio.run_coroutine_threadsafe( + async_func(**kwargs), tool_loop + ) + try: + return future.result(timeout=tool_timeout) + except FutureTimeoutError as e: + future.cancel() + raise TimeoutError( + f"Tool '{name}' timed out after {tool_timeout}s" + ) from e + except Exception as e: + # Re-raise with tool name context + raise type(e)(f"Tool '{name}' failed: {e}") from e + + return sync_wrapper + + sync_func = make_sync_wrapper(func, loop, timeout, tool_name) + + # Preserve function metadata for signature inspection + if hasattr(func, "__name__"): + sync_func.__name__ = func.__name__ + if hasattr(func, "__doc__"): + sync_func.__doc__ = func.__doc__ + sync_func.__signature__ = _signature_from_args(tool_args, tool_arg_types) # type: ignore[attr-defined] + + # Create new dspy.Tool with sync wrapper + bridged_tool = dspy.Tool( + sync_func, + name=tool_name, + desc=tool_desc, + args=tool_args, + arg_types=tool_arg_types, + arg_desc=tool_arg_desc, + ) + bridged.append(bridged_tool) + + return bridged + + +def build_rlm_agent( + signature: Any, + tools: list[Any], + *, + name: str, + max_iters: int, + max_llm_calls: int, + rlm_threshold: float, +) -> ContextSafe: + """Build an RLM agent with bridged tools. + + Must be called inside the running event loop. + + Args: + signature: DSPy signature for the agent + tools: List of tools (will be bridged to the current loop) + name: Agent name for logging + max_iters: Maximum RLM iterations + max_llm_calls: Maximum LLM calls for RLM fallback + rlm_threshold: ContextSafe RLM fallback threshold + + Returns: + ContextSafe wrapped RLM agent with bridged tools + """ + from codespy.config import get_settings + + settings = get_settings() + loop = asyncio.get_running_loop() + timeout = settings.llm.timeout + + # Bridge tools to the current loop + bridged_tools = bridge_tools_to_loop(tools, loop, timeout) + + # Build RLM with bridged tools + rlm = CodespyRLM( + signature, + tools=bridged_tools, + max_iters=max_iters, + max_llm_calls=max_llm_calls, + verbose=logger.isEnabledFor(logging.DEBUG), + ) + + # Wrap in ContextSafe with same bridged tools + return ContextSafe( + rlm, + signature, + tools=bridged_tools, + name=name, + max_iters=max_iters, + max_llm_calls=max_llm_calls, + rlm_threshold=rlm_threshold, + ) + + +async def run_rlm(agent: ContextSafe, **kwargs: Any) -> Any: + """Run an RLM agent off the event loop using asyncio.to_thread. + + This allows the RLM agent to run synchronously (including its sync tool + wrappers) while the event loop remains free to service bridged tool calls. + + Args: + agent: The ContextSafe-wrapped RLM agent + **kwargs: Arguments to pass to the agent + + Returns: + The agent result + """ + return await asyncio.to_thread(agent, **kwargs) + + +def log_rlm_outcome( + result: Any, + module: str, + scope: str, +) -> None: + """Log diagnostics for RLM outcomes, especially extract fallback cases. + + Args: + result: The prediction result from the RLM agent + module: Module name (e.g., 'code_review', 'supply_chain') + scope: Scope identifier for context + """ + # Check if this was an extract fallback + final_reasoning = getattr(result, "final_reasoning", None) + if final_reasoning != "Extract forced final output": + return + + # Get trajectory if available + trajectory = getattr(result, "trajectory", []) + if not trajectory: + return + + # Count error entries + error_count = 0 + first_error: str | None = None + + for entry in trajectory: + # Entry is a REPLEntry-like object or dict + if isinstance(entry, dict): + output = entry.get("output", "") + else: + output = getattr(entry, "output", "") + + if output and "Error" in str(output): + error_count += 1 + if first_error is None: + first_error = str(output) + + # Log warning with diagnostic info + if error_count > 0: + msg = ( + f"RLM[{module}] scope {scope}: extract fallback after {len(trajectory)} " + f"trajectory entries, {error_count} tool errors. " + ) + if first_error: + truncated = first_error[:300] + if len(first_error) > 300: + truncated += "..." + msg += f"First error: {truncated}" + logger.warning(msg) + else: + logger.debug( + f"RLM[{module}] scope {scope}: extract fallback after {len(trajectory)} " + "trajectory entries, no tool errors" + ) diff --git a/src/codespy/config.py b/src/codespy/config.py index b3fd213..5f89da0 100644 --- a/src/codespy/config.py +++ b/src/codespy/config.py @@ -47,6 +47,9 @@ # Custom config path (set via CLI --config flag) _custom_config_path: str | None = None +# Module-level variable to track which config file was loaded +_loaded_config_path: str | None = None + # Re-export for convenience __all__ = [ "Settings", @@ -92,12 +95,15 @@ def _load_yaml_config() -> dict[str, Any]: exact path and raise FileNotFoundError if it doesn't exist. Otherwise, search the default locations. """ + global _loaded_config_path + if _custom_config_path is not None: path = Path(_custom_config_path) if not path.exists(): raise FileNotFoundError(f"Config file not found: {path}") logger.debug(f"Loading config from {path} (via --config)") with open(path) as f: + _loaded_config_path = str(path) return yaml.safe_load(f) or {} config_paths = [ @@ -111,8 +117,10 @@ def _load_yaml_config() -> dict[str, Any]: if path.exists(): logger.debug(f"Loading config from {path}") with open(path) as f: + _loaded_config_path = str(path) return yaml.safe_load(f) or {} + _loaded_config_path = None return {} @@ -522,6 +530,11 @@ def get_settings(config_file: str | None = None) -> Settings: return settings +def get_loaded_config_path() -> str | None: + """Get the path to the loaded config file, or None if no file was found.""" + return _loaded_config_path + + def reload_settings(config_file: str | None = None) -> Settings: """Reload settings (useful after environment changes). diff --git a/src/codespy/config_dspy.py b/src/codespy/config_dspy.py index f302125..420bc0e 100644 --- a/src/codespy/config_dspy.py +++ b/src/codespy/config_dspy.py @@ -52,7 +52,7 @@ class ReviewConfig(BaseModel): """ # Minimum confidence threshold for reported issues - min_confidence: float = Field(default=0.81, ge=0.0, le=1.0) + min_confidence: float = Field(default=0.5, ge=0.0, le=1.0) # Output settings output_format: OutputFormat = "markdown" diff --git a/src/codespy/workflows/review/pipeline.py b/src/codespy/workflows/review/pipeline.py index 3983cce..095fffa 100644 --- a/src/codespy/workflows/review/pipeline.py +++ b/src/codespy/workflows/review/pipeline.py @@ -29,7 +29,7 @@ ) from codespy.agents.review.helpers import build_patches from codespy.agents.review.scope import MANIFEST_FILES, MANIFEST_GLOBS, build_sparse_patterns -from codespy.config import Settings, get_settings +from codespy.config import Settings, get_loaded_config_path, get_settings from codespy.config_memory import ( get_cerebral, get_episode_store, @@ -185,6 +185,16 @@ def forward(self, config: ReviewConfig) -> ReviewResult: # Verify memory storage access self._verify_memory_access() + # Log configuration details + config_path = get_loaded_config_path() + if config_path: + logger.info(f"Config loaded from: {config_path}") + logger.info(f"min_confidence={self.settings.review.min_confidence:.2f}") + for sig_name in ["code_review", "doc", "supply_chain"]: + max_iters = self.settings.get_max_iters(sig_name) + max_llm_calls = self.settings.get_max_llm_calls(sig_name) + logger.info(f" {sig_name}: max_iters={max_iters}, max_llm_calls={max_llm_calls}") + # Determine mode and fetch/build PR accordingly if isinstance(config, RemoteReviewConfig): # Remote mode: fetch from GitHub/GitLab diff --git a/tests/test_config.py b/tests/test_config.py index 42f4253..0261347 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -401,3 +401,20 @@ def test_get_llm_config_for_memory_mental_models_fallback(self, monkeypatch, tmp assert llm_config.model == "openai/gpt-4o-mini" monkeypatch.delenv("CODESPY_CONFIG", raising=False) + + +class TestMinConfidence: + """Tests for the min_confidence default value.""" + + def test_min_confidence_default_is_0_5(self): + """Test that min_confidence defaults to 0.5.""" + settings = Settings() + assert settings.review.min_confidence == 0.5 + + def test_min_confidence_can_be_overridden_via_env(self, monkeypatch, tmp_path): + """Test that min_confidence can be overridden via env var.""" + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("REVIEW_MIN_CONFIDENCE", "0.75") + settings = Settings() + assert settings.review.min_confidence == 0.75 + monkeypatch.delenv("REVIEW_MIN_CONFIDENCE") diff --git a/tests/test_context_safe.py b/tests/test_context_safe.py index f512302..c8a10af 100644 --- a/tests/test_context_safe.py +++ b/tests/test_context_safe.py @@ -1,12 +1,14 @@ """Tests for ContextSafe proactive RLM fallback thresholds.""" import os +import shutil from unittest.mock import MagicMock, patch +import dspy # type: ignore[import-untyped] import pytest from pydantic import ValidationError -from codespy.agents.context_safe import ContextSafe +from codespy.agents.context_safe import RLM_REASONING_DESC, CodespyRLM, ContextSafe from codespy.config_dspy import RLMFallbackConfig from codespy.config_utils import apply_env_overrides, build_env_map @@ -314,3 +316,64 @@ def test_layer2_hard_overflow_as_fallback(self, mock_litellm): # 20k + 64k + 4k = 88k > 80k assert should_fallback is True assert "context overflow predicted" in reason + + +class TestCodespyRLM: + """Stock RLM `reasoning` desc triggers Bedrock content_filter; ensure it is replaced.""" + + class _Sig(dspy.Signature): + """Do the thing.""" + + question: str = dspy.InputField() + answer: str = dspy.OutputField() + + def _rlm(self): + return CodespyRLM(self._Sig, max_iters=2) + + def test_action_reasoning_desc_replaced(self): + sig = self._rlm().generate_action.signature + assert sig.output_fields["reasoning"].json_schema_extra["desc"] == RLM_REASONING_DESC + # Other action fields and instructions are untouched + assert list(sig.output_fields) == ["reasoning", "code"] + assert "Do the thing." in sig.instructions + + def test_rendered_prompt_has_no_stock_desc(self): + from dspy.adapters.two_step_adapter import TwoStepAdapter + sig = self._rlm().generate_action.signature + text = TwoStepAdapter(dspy.LM("openai/gpt-4o-mini")).format_task_description(sig) + assert "Think step-by-step" not in text + assert "`reasoning` (str): Short plan for the next step." in text + + def test_fallback_uses_codespy_rlm(self): + cs = ContextSafe(dspy.Predict(self._Sig), self._Sig, tools=[], name="t", max_iters=2, max_llm_calls=5) + assert isinstance(cs._create_rlm_fallback(), CodespyRLM) + + def test_plain_converts_enums(self): + from enum import StrEnum + + from codespy.agents.context_safe import _plain + + class IssueCategory(StrEnum): + BUG = "bug" + SMELL = "smell" + + assert _plain({"c": [IssueCategory.BUG, ("x", IssueCategory.SMELL)]}) == {"c": ["bug", ["x", "smell"]]} + assert _plain("bug") == "bug" + + @pytest.mark.skipif(shutil.which("deno") is None, reason="needs deno") + def test_repl_runs_with_enum_inputs(self): + """Enum inputs must not break REPL variable injection (empty SyntaxError).""" + from dspy.utils.dummies import DummyLM + + from codespy.agents.review.models import IssueCategory + + class Sig(dspy.Signature): + """Echo.""" + + categories: list[IssueCategory] = dspy.InputField() + answer: str = dspy.OutputField() + + lm = DummyLM([{"reasoning": "r", "code": "SUBMIT(answer=','.join(categories))"}]) + with dspy.context(lm=lm, adapter=dspy.ChatAdapter()): + out = CodespyRLM(Sig, max_iters=2)(categories=[IssueCategory.BUG, IssueCategory.SMELL]) + assert out.answer == "bug,smell" diff --git a/tests/test_diagnostic_adapter.py b/tests/test_diagnostic_adapter.py new file mode 100644 index 0000000..2adb601 --- /dev/null +++ b/tests/test_diagnostic_adapter.py @@ -0,0 +1,516 @@ +"""Tests for DiagnosticTwoStepAdapter.""" + +import json +import logging +import tempfile +from pathlib import Path +from unittest.mock import MagicMock, patch + +import dspy # type: ignore[import-untyped] +import pytest +from dspy.adapters.base import AdapterParseError # type: ignore[import-untyped] + +from codespy.agents.diagnostic_adapter import ( + DiagnosticTwoStepAdapter, + _log_empty_response, + _get_finish_reason, + _get_finish_reason_from_lm, + _write_refusal_dump, +) + + +def _make_empty_response_error(): + """Create an AdapterParseError for empty response.""" + mock_sig = MagicMock() + mock_sig.__class__.__name__ = "TestSignature" + return AdapterParseError( + adapter_name="TwoStepAdapter", + signature=mock_sig, + lm_response="", + message="The LM returned an empty or null response", + ) + + +def _make_other_error(): + """Create an AdapterParseError for other parsing error.""" + mock_sig = MagicMock() + mock_sig.__class__.__name__ = "TestSignature" + return AdapterParseError( + adapter_name="TwoStepAdapter", + signature=mock_sig, + lm_response="{invalid json", + message="Some other parsing error", + ) + + +class TestDiagnosticTwoStepAdapter: + """Tests for DiagnosticTwoStepAdapter.""" + + def test_empty_response_error_logs_diagnostics(self, caplog): + """AdapterParseError with empty response message triggers diagnostic logging.""" + # Use actual dspy.LM or properly mock the BaseLM class + extraction_lm = MagicMock(spec=dspy.BaseLM) + adapter = DiagnosticTwoStepAdapter(extraction_lm) + + # Create a mock LM with history + mock_lm = MagicMock() + mock_lm.model = "test-model" + mock_lm.kwargs = {"max_tokens": 4096, "reasoning_effort": "medium"} + mock_lm.history = [ + { + "messages": [{"role": "user", "content": "test"}], + "response": { + "choices": [ + { + "finish_reason": "length", + "message": { + "content": "", + "reasoning_content": "some reasoning" * 10, + }, + } + ], + "usage": { + "prompt_tokens": 100, + "completion_tokens": 0, + "reasoning_tokens": 50, + }, + }, + } + ] + + with caplog.at_level(logging.WARNING, logger="codespy.agents.diagnostic_adapter"): + with patch.object( + DiagnosticTwoStepAdapter.__bases__[0], # TwoStepAdapter + "__call__", + side_effect=_make_empty_response_error(), + ): + with pytest.raises(AdapterParseError) as exc_info: + adapter(mock_lm, {}, MagicMock(), [], {}) + + assert "empty or null response" in str(exc_info.value) + assert "Empty LM response" in caplog.text + assert "finish_reason=length" in caplog.text + + def test_non_empty_parse_error_does_not_log(self, caplog): + """Other AdapterParseError messages do not trigger diagnostic logging.""" + extraction_lm = MagicMock(spec=dspy.BaseLM) + adapter = DiagnosticTwoStepAdapter(extraction_lm) + + mock_lm = MagicMock() + + with caplog.at_level(logging.WARNING, logger="codespy.agents.diagnostic_adapter"): + with patch.object( + DiagnosticTwoStepAdapter.__bases__[0], + "__call__", + side_effect=_make_other_error(), + ): + with pytest.raises(AdapterParseError) as exc_info: + adapter(mock_lm, {}, MagicMock(), [], {}) + + assert "Some other parsing error" in str(exc_info.value) + # Should not have empty response diagnostics + assert "finish_reason=" not in caplog.text + + def test_empty_history_still_raises(self, caplog): + """Empty LM history still propagates the original exception.""" + extraction_lm = MagicMock(spec=dspy.BaseLM) + adapter = DiagnosticTwoStepAdapter(extraction_lm) + + mock_lm = MagicMock() + mock_lm.history = [] + + with caplog.at_level(logging.WARNING, logger="codespy.agents.diagnostic_adapter"): + with patch.object( + DiagnosticTwoStepAdapter.__bases__[0], + "__call__", + side_effect=_make_empty_response_error(), + ): + with pytest.raises(AdapterParseError) as exc_info: + adapter(mock_lm, {}, MagicMock(), [], {}) + + assert "empty or null response" in str(exc_info.value) + assert "Empty LM response" in caplog.text + assert "no LM history available" in caplog.text + + def test_malformed_history_still_raises(self, caplog): + """Malformed history entry still propagates the original exception.""" + extraction_lm = MagicMock(spec=dspy.BaseLM) + adapter = DiagnosticTwoStepAdapter(extraction_lm) + + mock_lm = MagicMock() + mock_lm.history = ["not a dict"] + + with caplog.at_level(logging.WARNING, logger="codespy.agents.diagnostic_adapter"): + with patch.object( + DiagnosticTwoStepAdapter.__bases__[0], + "__call__", + side_effect=_make_empty_response_error(), + ): + with pytest.raises(AdapterParseError) as exc_info: + adapter(mock_lm, {}, MagicMock(), [], {}) + + assert "empty or null response" in str(exc_info.value) + assert "Empty LM response" in caplog.text + + def test_lm15_response_object(self, caplog): + """Handle lm15 Response object format.""" + extraction_lm = MagicMock(spec=dspy.BaseLM) + adapter = DiagnosticTwoStepAdapter(extraction_lm) + + # Create lm15-style Response mock + mock_response = MagicMock() + mock_response.__class__.__name__ = "Response" + mock_response.finish_reason = "stop" + mock_response.message.parts = [MagicMock(), MagicMock()] + mock_response.usage = {"prompt_tokens": 50} + + mock_lm = MagicMock() + mock_lm.model = "test-model" + mock_lm.history = [ + { + "messages": [{"role": "user", "content": "test"}], + "response": mock_response, + } + ] + + with caplog.at_level(logging.WARNING, logger="codespy.agents.diagnostic_adapter"): + with patch.object( + DiagnosticTwoStepAdapter.__bases__[0], + "__call__", + side_effect=_make_empty_response_error(), + ): + with pytest.raises(AdapterParseError) as exc_info: + adapter(mock_lm, {}, MagicMock(), [], {}) + + assert "empty or null response" in str(exc_info.value) + assert "lm15 Response" in caplog.text + assert "finish_reason=stop" in caplog.text + + def test_no_reasoning_content_logs_no_reasoning(self, caplog): + """When no reasoning content is present, log indicates that.""" + extraction_lm = MagicMock(spec=dspy.BaseLM) + adapter = DiagnosticTwoStepAdapter(extraction_lm) + + mock_lm = MagicMock() + mock_lm.model = "claude-opus" + mock_lm.history = [ + { + "messages": [{"role": "user", "content": "test"}], + "response": { + "choices": [ + { + "finish_reason": "stop", + "message": { + "content": "", + }, + } + ], + "usage": { + "prompt_tokens": 100, + "completion_tokens": 0, + }, + }, + } + ] + + with caplog.at_level(logging.WARNING, logger="codespy.agents.diagnostic_adapter"): + with patch.object( + DiagnosticTwoStepAdapter.__bases__[0], + "__call__", + side_effect=_make_empty_response_error(), + ): + with pytest.raises(AdapterParseError) as exc_info: + adapter(mock_lm, {}, MagicMock(), [], {}) + + assert "empty or null response" in str(exc_info.value) + assert "no reasoning_content or thinking_blocks" in caplog.text + assert "no tool_calls" in caplog.text + + +class TestLogEmptyResponse: + """Direct tests for _log_empty_response function.""" + + def test_handles_no_lm(self, caplog): + """Function handles None LM gracefully.""" + with caplog.at_level(logging.WARNING, logger="codespy.agents.diagnostic_adapter"): + _log_empty_response(None, None) + + # Should log that no history available + assert "Empty LM response" in caplog.text + + def test_handles_history_exception(self, caplog): + """Function handles exceptions accessing history gracefully.""" + mock_lm = MagicMock() + # Make history access raise + type(mock_lm).history = property(lambda self: (_ for _ in ()).throw(ValueError("boom"))) + + with caplog.at_level(logging.WARNING, logger="codespy.agents.diagnostic_adapter"): + _log_empty_response(mock_lm, None) + + # Should log that diagnostics failed + # (but the try/except in _log_empty_response catches it) + + def test_signature_with_output_fields(self, caplog): + """Extract output fields from signature.""" + mock_lm = MagicMock() + mock_lm.model = "test-model" + mock_lm.history = [] + + mock_sig = MagicMock() + mock_sig.output_fields = ["field1", "field2"] + + with caplog.at_level(logging.WARNING, logger="codespy.agents.diagnostic_adapter"): + _log_empty_response(mock_lm, mock_sig) + + assert "output_fields=['field1', 'field2']" in caplog.text + + +class TestGetFinishReason: + """Tests for _get_finish_reason function.""" + + def test_dict_style_response(self): + """Extract finish_reason from dict-style response.""" + entry = { + "response": { + "choices": [ + {"finish_reason": "content_filter", "message": {"content": ""}} + ] + } + } + assert _get_finish_reason(entry) == "content_filter" + + def test_lm15_response_object(self): + """Extract finish_reason from lm15 Response object.""" + mock_resp = MagicMock() + mock_resp.__class__.__name__ = "Response" + mock_resp.finish_reason = "stop" + + entry = {"response": mock_resp} + assert _get_finish_reason(entry) == "stop" + + def test_no_response(self): + """Return 'unknown' when no response.""" + entry = {} + assert _get_finish_reason(entry) == "unknown" + + def test_no_choices(self): + """Return 'unknown' when no choices.""" + entry = {"response": {"choices": []}} + assert _get_finish_reason(entry) == "unknown" + + +class TestGetFinishReasonFromLm: + """Tests for _get_finish_reason_from_lm function.""" + + def test_extracts_from_history(self): + """Extract finish_reason from LM history.""" + mock_lm = MagicMock() + mock_lm.history = [ + { + "response": { + "choices": [{"finish_reason": "content_filter", "message": {"content": ""}}] + } + } + ] + assert _get_finish_reason_from_lm(mock_lm) == "content_filter" + + def test_no_history(self): + """Return None when no history.""" + mock_lm = MagicMock() + mock_lm.history = None + assert _get_finish_reason_from_lm(mock_lm) is None + + def test_empty_history(self): + """Return None when empty history.""" + mock_lm = MagicMock() + mock_lm.history = [] + assert _get_finish_reason_from_lm(mock_lm) is None + + +class TestWriteRefusalDump: + """Tests for _write_refusal_dump function.""" + + @patch("codespy.agents.diagnostic_adapter.get_settings") + def test_writes_dump_file(self, mock_get_settings, tmp_path): + """Write refusal dump to file.""" + # Setup mock settings + mock_settings = MagicMock() + mock_settings.review.cache_dir = str(tmp_path) + mock_get_settings.return_value = mock_settings + + entry = { + "messages": [{"role": "user", "content": "test"}], + "response": {"choices": [{"finish_reason": "content_filter"}]}, + } + lm_kwargs = {"max_tokens": 4096, "api_key": "secret123"} + + result = _write_refusal_dump(entry, "claude-3-opus", lm_kwargs, "TestSignature") + + assert result is not None + assert result.exists() + assert "refusals" in str(result) + + # Verify content + with open(result) as f: + dump = json.load(f) + assert dump["model"] == "claude-3-opus" + assert dump["lm_kwargs"]["max_tokens"] == 4096 + assert dump["lm_kwargs"]["api_key"] == "" + assert dump["messages"] == [{"role": "user", "content": "test"}] + + @patch("codespy.agents.diagnostic_adapter.get_settings") + def test_handles_write_failure(self, mock_get_settings): + """Gracefully handle write failure.""" + # Setup mock settings that will cause failure + mock_settings = MagicMock() + mock_settings.review.cache_dir = "/nonexistent/path/that/cannot/be/created" + mock_get_settings.return_value = mock_settings + + entry = {"messages": [], "response": {}} + result = _write_refusal_dump(entry, "model", {}, "Sig") + + assert result is None + + +class TestContentFilterHandling: + """Tests for content_filter finish_reason handling.""" + + @patch("codespy.agents.diagnostic_adapter.get_settings") + def test_content_filter_raises_descriptive_error(self, mock_get_settings, caplog, tmp_path): + """When finish_reason is content_filter, raise descriptive error.""" + # Setup mock settings to use tmp_path for cache_dir + mock_settings = MagicMock() + mock_settings.review.cache_dir = str(tmp_path) + mock_get_settings.return_value = mock_settings + + extraction_lm = MagicMock(spec=dspy.BaseLM) + adapter = DiagnosticTwoStepAdapter(extraction_lm) + + mock_lm = MagicMock() + mock_lm.model = "claude-3-opus" + mock_lm.kwargs = {} + mock_lm.history = [ + { + "messages": [{"role": "user", "content": "test"}], + "response": { + "choices": [ + { + "finish_reason": "content_filter", + "message": {"content": ""}, + } + ], + "usage": {"prompt_tokens": 100, "completion_tokens": 8}, + }, + } + ] + + with caplog.at_level(logging.WARNING, logger="codespy.agents.diagnostic_adapter"): + with patch.object( + DiagnosticTwoStepAdapter.__bases__[0], + "__call__", + side_effect=AdapterParseError( + adapter_name="TwoStepAdapter", + signature=MagicMock(), + lm_response="", + message="The LM returned an empty or null response", + ), + ): + with pytest.raises(AdapterParseError) as exc_info: + adapter(mock_lm, {}, MagicMock(), [], {}) + + assert "model refused (content_filter)" in str(exc_info.value) + assert "Empty LM response" in caplog.text + + def test_non_content_filter_re_raises_original(self): + """When finish_reason is not content_filter, re-raise original error.""" + extraction_lm = MagicMock(spec=dspy.BaseLM) + adapter = DiagnosticTwoStepAdapter(extraction_lm) + + mock_lm = MagicMock() + mock_lm.model = "claude-3-opus" + mock_lm.kwargs = {} + mock_lm.history = [ + { + "messages": [{"role": "user", "content": "test"}], + "response": { + "choices": [ + { + "finish_reason": "length", + "message": {"content": ""}, + } + ], + }, + } + ] + + original_error = AdapterParseError( + adapter_name="TwoStepAdapter", + signature=MagicMock(), + lm_response="", + message="The LM returned an empty or null response", + ) + + with patch.object( + DiagnosticTwoStepAdapter.__bases__[0], + "__call__", + side_effect=original_error, + ): + with pytest.raises(AdapterParseError) as exc_info: + adapter(mock_lm, {}, MagicMock(), [], {}) + + assert "empty or null response" in str(exc_info.value) + + +class TestAcallHandling: + """Tests for async acall handling.""" + + @patch("codespy.agents.diagnostic_adapter.get_settings") + def test_acall_content_filter_raises_descriptive_error(self, mock_get_settings, caplog, tmp_path): + """Async acall raises descriptive error for content_filter.""" + import asyncio + + # Setup mock settings to use tmp_path for cache_dir + mock_settings = MagicMock() + mock_settings.review.cache_dir = str(tmp_path) + mock_get_settings.return_value = mock_settings + + extraction_lm = MagicMock(spec=dspy.BaseLM) + adapter = DiagnosticTwoStepAdapter(extraction_lm) + + mock_lm = MagicMock() + mock_lm.model = "claude-3-opus" + mock_lm.kwargs = {} + mock_lm.history = [ + { + "messages": [{"role": "user", "content": "test"}], + "response": { + "choices": [ + { + "finish_reason": "content_filter", + "message": {"content": ""}, + } + ], + }, + } + ] + + async def run_test(): + with caplog.at_level(logging.WARNING, logger="codespy.agents.diagnostic_adapter"): + with patch.object( + DiagnosticTwoStepAdapter.__bases__[0], + "acall", + side_effect=AdapterParseError( + adapter_name="TwoStepAdapter", + signature=MagicMock(), + lm_response="", + message="The LM returned an empty or null response", + ), + ): + with pytest.raises(AdapterParseError) as exc_info: + await adapter.acall(mock_lm, {}, MagicMock(), [], {}) + return exc_info + + exc_info = asyncio.run(run_test()) + assert "model refused (content_filter)" in str(exc_info.value) + assert "Empty LM response" in caplog.text + diff --git a/tests/test_filter_by_confidence.py b/tests/test_filter_by_confidence.py new file mode 100644 index 0000000..bbb5ef0 --- /dev/null +++ b/tests/test_filter_by_confidence.py @@ -0,0 +1,159 @@ +"""Tests for filter_by_confidence helper.""" + +import logging + +import pytest + +from codespy.agents.review.helpers import filter_by_confidence +from codespy.agents.review.models import Issue, IssueCategory, IssueSeverity + + +class TestFilterByConfidence: + """Tests for the filter_by_confidence function.""" + + def test_filter_keeps_issues_above_threshold(self, caplog): + """Issues with confidence >= threshold should be kept.""" + issues = [ + Issue( + category=IssueCategory.BUG, + severity=IssueSeverity.HIGH, + title="Issue 1", + description="Description 1", + filename="file1.py", + confidence=0.9, + ), + Issue( + category=IssueCategory.SECURITY, + severity=IssueSeverity.CRITICAL, + title="Issue 2", + description="Description 2", + filename="file2.py", + confidence=0.6, + ), + ] + + with caplog.at_level(logging.INFO): + result = filter_by_confidence(issues, 0.5, "code_review", "scope1") + + assert len(result) == 2 + assert result[0].title == "Issue 1" + assert result[1].title == "Issue 2" + assert "2 raw issues, 2 kept" in caplog.text + + def test_filter_drops_issues_below_threshold(self, caplog): + """Issues with confidence < threshold should be dropped.""" + issues = [ + Issue( + category=IssueCategory.BUG, + severity=IssueSeverity.HIGH, + title="Issue 1", + description="Description 1", + filename="file1.py", + confidence=0.9, + ), + Issue( + category=IssueCategory.SECURITY, + severity=IssueSeverity.CRITICAL, + title="Issue 2", + description="Description 2", + filename="file2.py", + confidence=0.4, + ), + ] + + with caplog.at_level(logging.INFO): + result = filter_by_confidence(issues, 0.5, "code_review", "scope1") + + assert len(result) == 1 + assert result[0].title == "Issue 1" + assert "2 raw issues, 1 kept" in caplog.text + + def test_filter_handles_none_issues(self, caplog): + """None issues list should return empty list.""" + with caplog.at_level(logging.INFO): + result = filter_by_confidence(None, 0.5, "code_review", "scope1") + + assert result == [] + assert "0 raw issues, 0 kept" in caplog.text + + def test_filter_handles_empty_list(self, caplog): + """Empty issues list should return empty list.""" + with caplog.at_level(logging.INFO): + result = filter_by_confidence([], 0.5, "code_review", "scope1") + + assert result == [] + assert "0 raw issues, 0 kept" in caplog.text + + def test_filter_logs_module_and_scope(self, caplog): + """Log should include module and scope.""" + issues = [ + Issue( + category=IssueCategory.BUG, + severity=IssueSeverity.HIGH, + title="Issue 1", + description="Description 1", + filename="file1.py", + confidence=0.9, + ), + ] + + with caplog.at_level(logging.INFO): + filter_by_confidence(issues, 0.5, "supply_chain", "packages/auth") + + assert "supply_chain scope packages/auth" in caplog.text + + def test_filter_logs_min_confidence(self, caplog): + """Log should include min_confidence value.""" + issues = [ + Issue( + category=IssueCategory.BUG, + severity=IssueSeverity.HIGH, + title="Issue 1", + description="Description 1", + filename="file1.py", + confidence=0.9, + ), + ] + + with caplog.at_level(logging.INFO): + filter_by_confidence(issues, 0.81, "code_review", "scope1") + + assert "min_confidence=0.81" in caplog.text + + def test_issue_default_confidence_is_0_8(self): + """Issue.confidence should default to 0.8.""" + issue = Issue( + category=IssueCategory.BUG, + severity=IssueSeverity.HIGH, + title="Test Issue", + description="Test Description", + filename="test.py", + ) + # Default is 0.8, which is above 0.5 but below 0.81 + assert issue.confidence == 0.8 + + def test_issue_with_default_confidence_passes_new_threshold(self): + """Issue with default confidence (0.8) should pass threshold of 0.5.""" + issue = Issue( + category=IssueCategory.BUG, + severity=IssueSeverity.HIGH, + title="Test Issue", + description="Test Description", + filename="test.py", + ) + + result = filter_by_confidence([issue], 0.5, "code_review", "scope1") + assert len(result) == 1 + + def test_issue_with_default_confidence_fails_old_threshold(self): + """Issue with default confidence (0.8) should fail old threshold of 0.81.""" + issue = Issue( + category=IssueCategory.BUG, + severity=IssueSeverity.HIGH, + title="Test Issue", + description="Test Description", + filename="test.py", + ) + + result = filter_by_confidence([issue], 0.81, "code_review", "scope1") + assert len(result) == 0 diff --git a/tests/test_rlm_tools.py b/tests/test_rlm_tools.py new file mode 100644 index 0000000..20da415 --- /dev/null +++ b/tests/test_rlm_tools.py @@ -0,0 +1,336 @@ +"""Tests for RLM tool bridge functionality.""" + +import asyncio +import shutil +import threading +from unittest.mock import MagicMock, patch + +import pytest + +from codespy.agents.rlm_tools import ( + _is_coroutine_function, + bridge_tools_to_loop, + build_rlm_agent, + log_rlm_outcome, + run_rlm, +) + + +class TestIsCoroutineFunction: + """Test the _is_coroutine_function helper.""" + + def test_sync_function_returns_false(self): + """Sync functions should return False.""" + def sync_func(): + return "sync" + assert _is_coroutine_function(sync_func) is False + + def test_async_function_returns_true(self): + """Async functions should return True.""" + async def async_func(): + return "async" + assert _is_coroutine_function(async_func) is True + + def test_dspy_tool_with_async_func(self): + """dspy.Tool wrapping async func should be detected.""" + try: + import dspy + async def async_func(): + return "async" + tool = dspy.Tool(async_func) + assert _is_coroutine_function(tool) is True + except ImportError: + pytest.skip("dspy not installed") + + +class TestBridgeToolsToLoop: + """Test the bridge_tools_to_loop function.""" + + def test_sync_tools_pass_through(self): + """Sync tools should be passed through unchanged.""" + def sync_tool(x: int) -> int: + """Sync tool docstring.""" + return x * 2 + + loop = asyncio.new_event_loop() + try: + bridged = bridge_tools_to_loop([sync_tool], loop, timeout=30.0) + assert len(bridged) == 1 + # The tool should be wrapped in dspy.Tool but remain sync + result = bridged[0].func(x=5) + assert result == 10 + finally: + loop.close() + + def test_async_tools_get_sync_wrapper(self): + """Async tools should get a sync wrapper that schedules on the loop.""" + async def async_echo(x: int) -> int: + """Echo async tool.""" + return x + + async def test_in_loop(): + loop = asyncio.get_running_loop() + bridged = bridge_tools_to_loop([async_echo], loop, timeout=30.0) + assert len(bridged) == 1 + + # The wrapper should be sync + bridged_tool = bridged[0] + assert not asyncio.iscoroutinefunction(bridged_tool.func) + + # Calling on a different thread should work + def call_tool(): + return bridged_tool.func(x=42) + + # Use to_thread to run off the event loop + result = await asyncio.to_thread(call_tool) + assert result == 42 + + asyncio.run(test_in_loop()) + + def test_bridge_preserves_tool_metadata(self): + """Bridging should preserve name, desc, args, arg_types, arg_desc.""" + async def async_echo(x: int) -> str: + """Echo the input.""" + return str(x) + + try: + import dspy + tool = dspy.Tool( + async_echo, + name="custom_echo", + desc="Custom echo tool", + args={"x": "integer"}, + arg_types={"x": int}, + arg_desc={"x": "The number to echo"}, + ) + + async def test_in_loop(): + loop = asyncio.get_running_loop() + bridged = bridge_tools_to_loop([tool], loop, timeout=30.0) + assert len(bridged) == 1 + + bridged_tool = bridged[0] + assert bridged_tool.name == "custom_echo" + assert bridged_tool.desc == "Custom echo tool" + assert bridged_tool.args == {"x": "integer"} + assert bridged_tool.arg_types == {"x": int} + assert bridged_tool.arg_desc == {"x": "The number to echo"} + + asyncio.run(test_in_loop()) + except ImportError: + pytest.skip("dspy not installed") + + def test_bridge_deadlock_guard(self): + """Calling bridged tool on the bridge thread should raise RuntimeError.""" + async def async_echo(x: int) -> int: + return x + + async def test_in_loop(): + loop = asyncio.get_running_loop() + bridged = bridge_tools_to_loop([async_echo], loop, timeout=30.0) + bridged_tool = bridged[0] + + # Calling directly on the event loop (same thread as bridge) + # should raise RuntimeError + with pytest.raises(RuntimeError, match="RLM tools must run off the event loop"): + bridged_tool.func(x=1) + + asyncio.run(test_in_loop()) + + def test_bridge_timeout(self): + """Slow tools should timeout and cancel the future.""" + async def slow_tool(): + await asyncio.sleep(10) # Very slow + return "done" + + async def test_in_loop(): + loop = asyncio.get_running_loop() + bridged = bridge_tools_to_loop([slow_tool], loop, timeout=0.05) + bridged_tool = bridged[0] + + def call_slow(): + return bridged_tool.func() + + # Should timeout + with pytest.raises(TimeoutError, match="slow_tool"): + await asyncio.to_thread(call_slow) + + asyncio.run(test_in_loop()) + + +class TestBuildRlmAgent: + """Test the build_rlm_agent function.""" + + @patch("codespy.config.get_settings") + def test_build_rlm_agent_creates_contextsafe(self, mock_get_settings): + """build_rlm_agent should return a ContextSafe-wrapped RLM.""" + mock_settings = MagicMock() + mock_settings.llm.timeout = 240.0 + mock_get_settings.return_value = mock_settings + + try: + import dspy + + class TestSignature(dspy.Signature): + """Test signature.""" + question: str = dspy.InputField() + answer: str = dspy.OutputField() + + async def test_in_loop(): + agent = build_rlm_agent( + TestSignature, + [], + name="test_agent", + max_iters=5, + max_llm_calls=8, + rlm_threshold=0.3, + ) + # Should be a ContextSafe instance + from codespy.agents.context_safe import ContextSafe + assert isinstance(agent, ContextSafe) + assert agent._name == "test_agent" + assert agent._max_iters == 5 + assert agent._max_llm_calls == 8 + assert agent._rlm_threshold == 0.3 + + asyncio.run(test_in_loop()) + except ImportError: + pytest.skip("dspy not installed") + + +class TestRunRlm: + """Test the run_rlm function.""" + + @patch("codespy.config.get_settings") + def test_run_rlm_calls_agent_off_loop(self, mock_get_settings): + """run_rlm should execute the agent off the event loop.""" + mock_settings = MagicMock() + mock_settings.llm.timeout = 240.0 + mock_get_settings.return_value = mock_settings + + try: + import dspy + from codespy.agents.context_safe import ContextSafe + + class TestSignature(dspy.Signature): + """Test signature.""" + question: str = dspy.InputField() + answer: str = dspy.OutputField() + + # Create a mock agent + mock_agent = MagicMock(spec=ContextSafe) + mock_prediction = MagicMock() + mock_prediction.answer = "test answer" + mock_agent.return_value = mock_prediction + + async def test_in_loop(): + result = await run_rlm(mock_agent, question="test") + assert result.answer == "test answer" + # Verify the agent was called (mock_agent is called as a function via __call__) + assert mock_agent.called + # Check the call args + call_args = mock_agent.call_args + assert call_args.kwargs.get("question") == "test" + + asyncio.run(test_in_loop()) + except ImportError: + pytest.skip("dspy not installed") + + +class TestLogRlmOutcome: + """Test the log_rlm_outcome function.""" + + def test_log_rlm_outcome_no_op_without_extract_reasoning(self): + """Should not log when final_reasoning is not extract fallback.""" + mock_result = MagicMock() + mock_result.final_reasoning = "Natural completion" + + # Should not raise and should not log + log_rlm_outcome(mock_result, "test_module", "test_scope") + + def test_log_rlm_outcome_logs_errors(self, caplog): + """Should log warning when extract fallback has tool errors.""" + mock_result = MagicMock() + mock_result.final_reasoning = "Extract forced final output" + + # Create mock trajectory with errors + entry_with_error = MagicMock() + entry_with_error.output = "Error: something went wrong" + + entry_ok = MagicMock() + entry_ok.output = "Normal output" + + mock_result.trajectory = [entry_ok, entry_with_error, entry_with_error] + + with caplog.at_level("WARNING"): + log_rlm_outcome(mock_result, "code_review", "scope1") + + assert "RLM[code_review] scope scope1" in caplog.text + assert "2 tool errors" in caplog.text + # The log message doesn't contain "Extract forced final output", it contains the diagnostic info + assert "extract fallback" in caplog.text + + def test_log_rlm_outcome_dict_trajectory(self, caplog): + """Should handle trajectory entries as dicts.""" + mock_result = MagicMock() + mock_result.final_reasoning = "Extract forced final output" + mock_result.trajectory = [ + {"reasoning": "r1", "code": "c1", "output": "Normal"}, + {"reasoning": "r2", "code": "c2", "output": "Error: timeout"}, + ] + + with caplog.at_level("WARNING"): + log_rlm_outcome(mock_result, "supply_chain", "scope2") + + assert "1 tool errors" in caplog.text + + +class TestBridgedToolSignature: + """Bridged tools must keep their real signature inside the RLM sandbox.""" + + @staticmethod + async def get_tree(path: str = "", max_depth: int = 3) -> str: + return f"tree:{path}:{max_depth}" + + @staticmethod + async def read_file(path: str, max_lines: int | None = None) -> str: + return f"file:{path}:{max_lines}" + + def test_signature_matches_tool_args(self): + import inspect + + async def run(): + return bridge_tools_to_loop([self.get_tree, self.read_file], asyncio.get_running_loop(), 5) + + tree, read = asyncio.run(run()) + assert str(inspect.signature(tree.func)) == "(path: str = '', max_depth: int = 3)" + assert list(inspect.signature(read.func).parameters) == ["path", "max_lines"] + assert inspect.signature(read.func).parameters["path"].default is inspect.Parameter.empty + + @pytest.mark.skipif(shutil.which("deno") is None, reason="needs deno") + def test_sandbox_calls_with_keyword_and_positional_args(self): + import dspy # type: ignore[import-untyped] + from dspy.utils.dummies import DummyLM + + from codespy.agents.context_safe import CodespyRLM + + class Sig(dspy.Signature): + """Echo.""" + + q: str = dspy.InputField() + answer: str = dspy.OutputField() + + code = "SUBMIT(answer=get_tree('.', max_depth=2) + '|' + read_file('a.txt'))" + lm = DummyLM([{"reasoning": "r", "code": code}]) + + async def run(): + tools = bridge_tools_to_loop([self.get_tree, self.read_file], asyncio.get_running_loop(), 10) + rlm = CodespyRLM(Sig, tools=tools, max_iters=2) + + def call(): + with dspy.context(lm=lm, adapter=dspy.ChatAdapter()): + return rlm(q="x") + + return await asyncio.to_thread(call) + + assert asyncio.run(run()).answer == "tree:.:2|file:a.txt:None"