From c270b73434c6c7375ea53afddaadfca352d53131 Mon Sep 17 00:00:00 2001 From: Guillaume Simonneau Date: Mon, 5 Oct 2026 20:32:14 +0200 Subject: [PATCH 1/4] rlm fix --- .env.example | 2 +- CHANGELOG.md | 9 + action.yml | 2 +- codespy.yaml | 2 +- scripts/replay_refusal.py | 222 ++++++++ src/codespy/agents/context_safe.py | 1 + src/codespy/agents/diagnostic_adapter.py | 328 ++++++++++++ src/codespy/agents/dspy_config.py | 6 +- .../agents/review/code_review/agent.py | 30 +- src/codespy/agents/review/doc/agent.py | 10 +- src/codespy/agents/review/helpers.py | 33 ++ src/codespy/agents/review/models.py | 7 +- src/codespy/agents/review/scope/agent.py | 16 +- .../agents/review/supply_chain/agent.py | 30 +- src/codespy/agents/rlm_tools.py | 302 +++++++++++ src/codespy/config.py | 13 + src/codespy/config_dspy.py | 2 +- src/codespy/workflows/review/pipeline.py | 12 +- tests/test_config.py | 17 + tests/test_diagnostic_adapter.py | 503 ++++++++++++++++++ tests/test_filter_by_confidence.py | 159 ++++++ tests/test_rlm_tools.py | 310 +++++++++++ 22 files changed, 1956 insertions(+), 60 deletions(-) create mode 100644 scripts/replay_refusal.py create mode 100644 src/codespy/agents/diagnostic_adapter.py create mode 100644 src/codespy/agents/rlm_tools.py create mode 100644 tests/test_diagnostic_adapter.py create mode 100644 tests/test_filter_by_confidence.py create mode 100644 tests/test_rlm_tools.py 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..eb550bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,15 @@ ## [Unreleased] +### 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). Prompts now request a confidence ("≥0.9 verified with tools; 0.7–0.9 strong evidence; <0.7 weak"), and `filter_by_confidence()` logs raw/kept counts. + +### 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). Update your codespy.yaml or environment variable to override 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/scripts/replay_refusal.py b/scripts/replay_refusal.py new file mode 100644 index 0000000..2f381a3 --- /dev/null +++ b/scripts/replay_refusal.py @@ -0,0 +1,222 @@ +#!/usr/bin/env python3 +"""Replay a refusal dump through litellm to bisect what triggers the refusal. + +Usage: + python scripts/replay_refusal.py [--alt-model ] + +The script replays the dump through litellm.completion with the same model +and kwargs and prints finish_reason and usage for each variant: + - baseline: the dump as-is, to confirm it refuses reproducibly + - no_reasoning: drop reasoning_effort + - no_security: system prompt with "B. SECURITY VULNERABILITIES" block removed + - no_repl: system prompt with RLM REPL template paragraph removed + - no_patch: user message with changed_files[*].patch replaced by placeholder + - alt_model: same messages on a model given with --alt-model +""" + +import argparse +import json +import re +import sys +from pathlib import Path +from typing import Any + +import litellm + + +def _strip_api_keys(kwargs: dict) -> dict: + """Remove API keys from kwargs for safe display.""" + safe = {} + for key, value in kwargs.items(): + if "key" in key.lower() or "secret" in key.lower() or "token" in key.lower(): + safe[key] = "" + else: + safe[key] = value + return safe + + +def _run_completion(model: str, messages: list[dict], lm_kwargs: dict) -> dict[str, Any]: + """Run litellm.completion and return result info.""" + try: + # Filter out kwargs that litellm doesn't accept directly + safe_kwargs = {k: v for k, v in lm_kwargs.items() + if k not in ("cache_control_injection_points",)} + response = litellm.completion( + model=model, + messages=messages, + **safe_kwargs, + ) + choice = response.choices[0] if response.choices else {} + message = choice.get("message", {}) if isinstance(choice, dict) else getattr(choice, "message", {}) + usage = response.usage + return { + "success": True, + "finish_reason": choice.get("finish_reason") if isinstance(choice, dict) else getattr(choice, "finish_reason", "unknown"), + "content": message.get("content", "") if isinstance(message, dict) else getattr(message, "content", ""), + "prompt_tokens": getattr(usage, "prompt_tokens", getattr(usage, "input_tokens", 0)), + "completion_tokens": getattr(usage, "completion_tokens", getattr(usage, "output_tokens", 0)), + "reasoning_tokens": getattr(usage, "reasoning_tokens", 0), + } + except Exception as e: + return { + "success": False, + "error": str(e), + } + + +def _make_variant_baseline(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: + """Baseline: use messages and kwargs as-is.""" + return messages, lm_kwargs + + +def _make_variant_no_reasoning(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: + """Remove reasoning_effort from kwargs.""" + new_kwargs = {k: v for k, v in lm_kwargs.items() if k != "reasoning_effort"} + return messages, new_kwargs + + +def _remove_security_block(content: str) -> str: + """Remove the 'B. SECURITY VULNERABILITIES' section from system prompt.""" + # Match from "B. SECURITY VULNERABILITIES" to next "[A-Z]. " or end + pattern = r"B\.\s*SECURITY\s*VULNERABILITIES.*?(?=\n[A-Z]\.\s|\Z)" + return re.sub(pattern, "", content, flags=re.DOTALL | re.IGNORECASE) + + +def _make_variant_no_security(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: + """Remove security vulnerabilities section from system prompt.""" + new_messages = [] + for msg in messages: + if msg.get("role") == "system": + content = msg.get("content", "") + if isinstance(content, str): + new_content = _remove_security_block(content) + new_msg = dict(msg) + new_msg["content"] = new_content + new_messages.append(new_msg) + else: + new_messages.append(msg) + else: + new_messages.append(msg) + return new_messages, lm_kwargs + + +def _remove_repl_paragraph(content: str) -> str: + """Remove the RLM REPL template paragraph from system prompt.""" + # Match "You have access to a Python REPL" paragraph + pattern = r"You have access to a Python REPL.*?call SUBMIT\(\)" + return re.sub(pattern, "", content, flags=re.DOTALL | re.IGNORECASE) + + +def _make_variant_no_repl(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: + """Remove REPL/code-execution framing from system prompt.""" + new_messages = [] + for msg in messages: + if msg.get("role") == "system": + content = msg.get("content", "") + if isinstance(content, str): + new_content = _remove_repl_paragraph(content) + new_msg = dict(msg) + new_msg["content"] = new_content + new_messages.append(new_msg) + else: + new_messages.append(msg) + else: + new_messages.append(msg) + return new_messages, lm_kwargs + + +def _remove_patches(content: str) -> str: + """Replace patch content with placeholder.""" + # Replace ```diff blocks with placeholder + pattern = r"```diff.*?```" + return re.sub(pattern, "[PATCH CONTENT REDACTED]", content, flags=re.DOTALL) + + +def _make_variant_no_patch(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: + """Replace patch content in user message with placeholder.""" + new_messages = [] + for msg in messages: + if msg.get("role") in ("user", "human"): + content = msg.get("content", "") + if isinstance(content, str): + new_content = _remove_patches(content) + new_msg = dict(msg) + new_msg["content"] = new_content + new_messages.append(new_msg) + else: + new_messages.append(msg) + else: + new_messages.append(msg) + return new_messages, lm_kwargs + + +def _print_result(variant: str, result: dict[str, Any]) -> None: + """Print the result of a variant run.""" + print(f"\n{'='*60}") + print(f"Variant: {variant}") + print(f"{'='*60}") + if result["success"]: + print(f" finish_reason: {result['finish_reason']}") + print(f" prompt_tokens: {result['prompt_tokens']}") + print(f" completion_tokens: {result['completion_tokens']}") + print(f" reasoning_tokens: {result['reasoning_tokens']}") + content_preview = result['content'][:200] if result['content'] else "" + print(f" content_preview: {content_preview}...") + else: + print(f" ERROR: {result['error']}") + + +def main() -> int: + """Main entry point.""" + parser = argparse.ArgumentParser( + description="Replay a refusal dump to bisect what triggers the refusal." + ) + parser.add_argument("dump", type=Path, help="Path to the refusal dump JSON file") + parser.add_argument( + "--alt-model", + help="Alternative model to test (e.g., bedrock/anthropic.claude-sonnet-4.1)", + ) + args = parser.parse_args() + + dump_path = args.dump + if not dump_path.exists(): + print(f"Error: Dump file not found: {dump_path}", file=sys.stderr) + return 1 + + # Load dump + with open(dump_path, "r", encoding="utf-8") as f: + dump = json.load(f) + + model = dump.get("model", "unknown") + lm_kwargs = dump.get("lm_kwargs", {}) + messages = dump.get("messages", []) + + print(f"Loaded refusal dump: {dump_path}") + print(f" Model: {model}") + print(f" Messages: {len(messages)}") + print(f" LM kwargs: {_strip_api_keys(lm_kwargs)}") + + # Run variants + variants = [ + ("baseline", _make_variant_baseline), + ("no_reasoning", _make_variant_no_reasoning), + ("no_security", _make_variant_no_security), + ("no_repl", _make_variant_no_repl), + ("no_patch", _make_variant_no_patch), + ] + + for variant_name, variant_fn in variants: + variant_messages, variant_kwargs = variant_fn(messages, lm_kwargs) + result = _run_completion(model, variant_messages, variant_kwargs) + _print_result(variant_name, result) + + # Run alt_model variant if specified + if args.alt_model: + result = _run_completion(args.alt_model, messages, lm_kwargs) + _print_result(f"alt_model ({args.alt_model})", result) + + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/src/codespy/agents/context_safe.py b/src/codespy/agents/context_safe.py index 0e5b720..90e71cc 100644 --- a/src/codespy/agents/context_safe.py +++ b/src/codespy/agents/context_safe.py @@ -202,4 +202,5 @@ def _create_rlm_fallback(self) -> dspy.RLM: 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..0114d17 --- /dev/null +++ b/src/codespy/agents/rlm_tools.py @@ -0,0 +1,302 @@ +"""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 ContextSafe + +logger = logging.getLogger(__name__) + +# Thread-local storage for 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 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__ + + # 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 = dspy.RLM( + 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_diagnostic_adapter.py b/tests/test_diagnostic_adapter.py new file mode 100644 index 0000000..b8997f0 --- /dev/null +++ b/tests/test_diagnostic_adapter.py @@ -0,0 +1,503 @@ +"""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.""" + + def test_content_filter_raises_descriptive_error(self, caplog): + """When finish_reason is content_filter, raise descriptive 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": "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.""" + + def test_acall_content_filter_raises_descriptive_error(self, caplog): + """Async acall raises descriptive error for content_filter.""" + import asyncio + + 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..f4f1f0e --- /dev/null +++ b/tests/test_rlm_tools.py @@ -0,0 +1,310 @@ +"""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, +) + + +# Skip tests that require Deno if it's not installed +requires_deno = pytest.mark.skipif( + shutil.which("deno") is None, + reason="Deno not installed" +) + + +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 + + +@requires_deno +class TestDenoIntegration: + """Integration tests that require Deno to be installed.""" + + @pytest.mark.skip(reason="Requires full DSPy + MCP setup - run manually") + def test_rlm_with_bridged_mcp_tool(self): + """Full integration: RLM agent with bridged MCP tool via Deno. + + This test would require: + 1. A running MCP server via Deno + 2. DSPy RLM with bridged tools + 3. Executing on the event loop thread + + Run manually when Deno is available. + """ + pass From d1e90c370490b85256fa72858f6f6d73be4d4094 Mon Sep 17 00:00:00 2001 From: Guillaume Simonneau Date: Mon, 5 Oct 2026 22:37:34 +0200 Subject: [PATCH 2/4] fix rlm --- CHANGELOG.md | 3 + scripts/replay_refusal.py | 588 +++++++++++++++++++++++++++-- src/codespy/agents/context_safe.py | 44 ++- src/codespy/agents/rlm_tools.py | 32 +- tests/test_context_safe.py | 65 +++- tests/test_diagnostic_adapter.py | 17 +- tests/test_rlm_tools.py | 51 +++ 7 files changed, 773 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eb550bb..fbc344e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ - **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). Prompts now request a confidence ("≥0.9 verified with tools; 0.7–0.9 strong evidence; <0.7 weak"), and `filter_by_confidence()` logs raw/kept counts. +- **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. `scripts/replay_refusal.py` replays refusal dumps under bisection variants. +- **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. ### Changed diff --git a/scripts/replay_refusal.py b/scripts/replay_refusal.py index 2f381a3..7347195 100644 --- a/scripts/replay_refusal.py +++ b/scripts/replay_refusal.py @@ -2,7 +2,7 @@ """Replay a refusal dump through litellm to bisect what triggers the refusal. Usage: - python scripts/replay_refusal.py [--alt-model ] + python scripts/replay_refusal.py [--alt-model ] [--raw] [--only v1,v2] The script replays the dump through litellm.completion with the same model and kwargs and prints finish_reason and usage for each variant: @@ -10,12 +10,33 @@ - no_reasoning: drop reasoning_effort - no_security: system prompt with "B. SECURITY VULNERABILITIES" block removed - no_repl: system prompt with RLM REPL template paragraph removed + - no_tools: system prompt with tool docs removed + - no_security_no_repl: both security and REPL removed - no_patch: user message with changed_files[*].patch replaced by placeholder + - neutral_user: replace user message with neutral content + - no_cache_control: drop cache_control_injection_points + - trivial: minimal system "You are a helpful assistant.", user "Reply with OK." + - core_only: no_security + no_repl + neutral_user combined + - no_sig: delete from "Specific instructions:" up to "You are tasked with producing" + - sig_A_only: keep only section A (BUGS & LOGIC ERRORS) + - sig_C_only: keep only section C (CODE SMELLS) + - core_no_preamble / core_no_intro / core_no_head: core_only minus the RLM + preamble, the signature intro, or both + - head_only / preamble_only / intro_only: system = only that head part, neutral user + - preamble_no_head / preamble_no_layout: preamble_only minus one stock sentence + - preamble_rewrite(_only): preamble with stock sentences neutralised (did not help) + - pre_drop_: preamble_only minus one RLM field line + (variables_info, repl_history, iteration, reasoning, code) + - pre_no_fields: preamble_only minus all five field lines + - full_reasoning_desc / full_code_desc: full request with that field's desc neutralised + Note: no_patch is a no-op on dumps whose user message only holds a truncated + variables_info preview (no patch text present). - alt_model: same messages on a model given with --alt-model """ import argparse import json +import os import re import sys from pathlib import Path @@ -35,27 +56,127 @@ def _strip_api_keys(kwargs: dict) -> dict: return safe +def _messages_changed(orig: list[dict], new: list[dict]) -> bool: + """Check if messages list differs from original.""" + return orig != new + + +def _kwargs_changed(orig: dict, new: dict) -> bool: + """Check if kwargs dict differs from original.""" + return orig != new + + def _run_completion(model: str, messages: list[dict], lm_kwargs: dict) -> dict[str, Any]: """Run litellm.completion and return result info.""" try: - # Filter out kwargs that litellm doesn't accept directly - safe_kwargs = {k: v for k, v in lm_kwargs.items() - if k not in ("cache_control_injection_points",)} + # Pass lm_kwargs through as-is (no filter) response = litellm.completion( model=model, messages=messages, - **safe_kwargs, + **lm_kwargs, ) choice = response.choices[0] if response.choices else {} message = choice.get("message", {}) if isinstance(choice, dict) else getattr(choice, "message", {}) usage = response.usage + + # Extract reasoning_tokens from completion_tokens_details if available + reasoning_tokens = 0 + if hasattr(usage, "completion_tokens_details") and usage.completion_tokens_details: + reasoning_tokens = getattr(usage.completion_tokens_details, "reasoning_tokens", 0) + return { "success": True, "finish_reason": choice.get("finish_reason") if isinstance(choice, dict) else getattr(choice, "finish_reason", "unknown"), "content": message.get("content", "") if isinstance(message, dict) else getattr(message, "content", ""), "prompt_tokens": getattr(usage, "prompt_tokens", getattr(usage, "input_tokens", 0)), "completion_tokens": getattr(usage, "completion_tokens", getattr(usage, "output_tokens", 0)), - "reasoning_tokens": getattr(usage, "reasoning_tokens", 0), + "reasoning_tokens": reasoning_tokens, + } + except Exception as e: + return { + "success": False, + "error": str(e), + } + + +def _run_raw_bedrock(model: str, messages: list[dict], lm_kwargs: dict) -> dict[str, Any]: + """Run boto3 bedrock-runtime converse directly and return result info.""" + try: + import boto3 + except ImportError: + return {"success": False, "error": "boto3 not installed"} + + try: + # Strip bedrock/converse/ prefix from model + model_id = model.replace("bedrock/converse/", "") + + # Build system and messages for converse API + system_text = "" + converse_messages = [] + + for msg in messages: + if msg.get("role") == "system": + content = msg.get("content", "") + if isinstance(content, str): + system_text = content + elif msg.get("role") in ("user", "human"): + content = msg.get("content", "") + if isinstance(content, str): + converse_messages.append({ + "role": "user", + "content": [{"text": content}] + }) + elif msg.get("role") == "assistant": + content = msg.get("content", "") + if isinstance(content, str): + converse_messages.append({ + "role": "assistant", + "content": [{"text": content}] + }) + + # Build inference config from lm_kwargs + inference_config = {} + if "max_tokens" in lm_kwargs: + inference_config["maxTokens"] = lm_kwargs["max_tokens"] + if "temperature" in lm_kwargs: + inference_config["temperature"] = lm_kwargs["temperature"] + + region = os.environ.get("AWS_REGION") or os.environ.get("AWS_DEFAULT_REGION") or "us-east-1" + client = boto3.client("bedrock-runtime", region_name=region) + + request_kwargs = { + "modelId": model_id, + "messages": converse_messages, + } + if system_text: + request_kwargs["system"] = [{"text": system_text}] + if inference_config: + request_kwargs["inferenceConfig"] = inference_config + + response = client.converse(**request_kwargs) + + # Extract response details + output = response.get("output", {}) + message = output.get("message", {}) + content_blocks = message.get("content", []) + content_text = "" + for block in content_blocks: + if "text" in block: + content_text += block["text"] + + usage = response.get("usage", {}) + stop_reason = output.get("stopReason", "unknown") + request_id = response.get("ResponseMetadata", {}).get("RequestId", "unknown") + + return { + "success": True, + "finish_reason": stop_reason, + "content": content_text, + "prompt_tokens": usage.get("inputTokens", 0), + "completion_tokens": usage.get("outputTokens", 0), + "reasoning_tokens": 0, + "raw_stop_reason": stop_reason, + "request_id": request_id, } except Exception as e: return { @@ -76,9 +197,12 @@ def _make_variant_no_reasoning(messages: list[dict], lm_kwargs: dict) -> tuple[l def _remove_security_block(content: str) -> str: - """Remove the 'B. SECURITY VULNERABILITIES' section from system prompt.""" - # Match from "B. SECURITY VULNERABILITIES" to next "[A-Z]. " or end - pattern = r"B\.\s*SECURITY\s*VULNERABILITIES.*?(?=\n[A-Z]\.\s|\Z)" + """Remove the 'B. SECURITY VULNERABILITIES' section from system prompt. + + Delete from '─── B. SECURITY' up to (not including) '─── C. CODE SMELLS' + """ + # Match from "─── B. SECURITY" up to but not including "─── C. CODE SMELLS" + pattern = r"(─── B\.\s*SECURITY.*?)(?=─── C\.\s*CODE SMELLS)" return re.sub(pattern, "", content, flags=re.DOTALL | re.IGNORECASE) @@ -101,9 +225,11 @@ def _make_variant_no_security(messages: list[dict], lm_kwargs: dict) -> tuple[li def _remove_repl_paragraph(content: str) -> str: - """Remove the RLM REPL template paragraph from system prompt.""" - # Match "You have access to a Python REPL" paragraph - pattern = r"You have access to a Python REPL.*?call SUBMIT\(\)" + """Remove the RLM REPL template from system prompt. + + Delete from "You have access to a Python REPL" to the end of the system message. + """ + pattern = r"You have access to a Python REPL.*" return re.sub(pattern, "", content, flags=re.DOTALL | re.IGNORECASE) @@ -125,11 +251,52 @@ def _make_variant_no_repl(messages: list[dict], lm_kwargs: dict) -> tuple[list[d return new_messages, lm_kwargs +def _remove_tools_section(content: str) -> str: + """Remove the tool documentation from system prompt. + + Delete from "Additional tools available" to the end of the system message. + """ + pattern = r"Additional tools available.*" + return re.sub(pattern, "", content, flags=re.DOTALL | re.IGNORECASE) + + +def _make_variant_no_tools(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: + """Remove tool docs from system prompt.""" + new_messages = [] + for msg in messages: + if msg.get("role") == "system": + content = msg.get("content", "") + if isinstance(content, str): + new_content = _remove_tools_section(content) + new_msg = dict(msg) + new_msg["content"] = new_content + new_messages.append(new_msg) + else: + new_messages.append(msg) + else: + new_messages.append(msg) + return new_messages, lm_kwargs + + +def _make_variant_no_security_no_repl(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: + """Remove both security section and REPL template from system prompt.""" + # First apply no_security + msgs1, kwargs1 = _make_variant_no_security(messages, lm_kwargs) + # Then apply no_repl + return _make_variant_no_repl(msgs1, kwargs1) + + def _remove_patches(content: str) -> str: - """Replace patch content with placeholder.""" - # Replace ```diff blocks with placeholder - pattern = r"```diff.*?```" - return re.sub(pattern, "[PATCH CONTENT REDACTED]", content, flags=re.DOTALL) + """Replace patch content in JSON strings with placeholder.""" + # Handle escaped newlines in JSON strings + # The patch field contains actual newlines that are escaped in JSON + # Match "patch": "..." with content containing escaped newlines + pattern = r'("patch"\s*:\s*")((?:[^"\\]|\\.)*?)(")' + + def replace_patch(match): + return match.group(1) + "[PATCH REDACTED]" + match.group(3) + + return re.sub(pattern, replace_patch, content, flags=re.DOTALL) def _make_variant_no_patch(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: @@ -150,16 +317,334 @@ def _make_variant_no_patch(messages: list[dict], lm_kwargs: dict) -> tuple[list[ return new_messages, lm_kwargs -def _print_result(variant: str, result: dict[str, Any]) -> None: +def _make_variant_neutral_user(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: + """Replace user message with neutral content.""" + new_messages = [] + for msg in messages: + if msg.get("role") in ("user", "human"): + new_msg = dict(msg) + new_msg["content"] = "variables_info: []\n\nrepl_history: entries=[] max_output_chars=10000\n\niteration: 1/5" + new_messages.append(new_msg) + else: + new_messages.append(msg) + return new_messages, lm_kwargs + + +def _make_variant_no_cache_control(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: + """Remove cache_control_injection_points from kwargs.""" + new_kwargs = {k: v for k, v in lm_kwargs.items() if k != "cache_control_injection_points"} + return messages, new_kwargs + + +def _make_variant_trivial(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: + """Minimal system and user messages to test if model accepts anything.""" + new_messages = [] + for msg in messages: + if msg.get("role") == "system": + new_msg = dict(msg) + new_msg["content"] = "You are a helpful assistant." + new_messages.append(new_msg) + elif msg.get("role") in ("user", "human"): + new_msg = dict(msg) + new_msg["content"] = "Reply with OK." + new_messages.append(new_msg) + else: + new_messages.append(dict(msg)) + # Keep lm_kwargs as-is + return new_messages, lm_kwargs + + +def _make_variant_core_only(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: + """Combine no_security + no_repl + neutral_user.""" + # Apply no_security + msgs1, kwargs1 = _make_variant_no_security(messages, lm_kwargs) + # Apply no_repl + msgs2, kwargs2 = _make_variant_no_repl(msgs1, kwargs1) + # Apply neutral_user + return _make_variant_neutral_user(msgs2, kwargs2) + + +def _remove_signature_instructions(content: str) -> str: + """Remove from 'Specific instructions:' up to 'You are tasked with producing'. + + Keeps the header, schema and REPL, drops the whole CodeReviewSignature docstring. + """ + # Match from "Specific instructions:" up to but not including "You are tasked with producing" + pattern = r"(Specific instructions:.*?)(?=You are tasked with producing)" + return re.sub(pattern, "", content, flags=re.DOTALL | re.IGNORECASE) + + +def _make_variant_no_sig(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: + """Remove the CodeReviewSignature docstring from system prompt.""" + new_messages = [] + for msg in messages: + if msg.get("role") == "system": + content = msg.get("content", "") + if isinstance(content, str): + new_content = _remove_signature_instructions(content) + new_msg = dict(msg) + new_msg["content"] = new_content + new_messages.append(new_msg) + else: + new_messages.append(msg) + else: + new_messages.append(msg) + return new_messages, lm_kwargs + + +def _keep_only_section(content: str, section_letter: str) -> str: + """Keep only the specified section (A or C) from the signature. + + Section A: BUGS & LOGIC ERRORS + Section C: CODE SMELLS + """ + # Find the ANALYZE CHANGES header and OUTPUT RULES footer + header_match = re.search(r"(ANALYZE CHANGES.*?)(?=─── A\.)", content, re.DOTALL | re.IGNORECASE) + footer_match = re.search(r"(OUTPUT RULES.*?)$", content, re.DOTALL | re.IGNORECASE) + + header = header_match.group(1) if header_match else "ANALYZE CHANGES\n" + footer = footer_match.group(1) if footer_match else "" + + if section_letter.upper() == "A": + # Keep section A (from ─── A. to ─── B.) + section_match = re.search(r"(─── A\.\s*BUGS.*?)(?=─── B\.\s*SECURITY)", content, re.DOTALL | re.IGNORECASE) + elif section_letter.upper() == "C": + # Keep section C (from ─── C. to OUTPUT RULES) + section_match = re.search(r"(─── C\.\s*CODE SMELLS.*?)(?=OUTPUT RULES)", content, re.DOTALL | re.IGNORECASE) + else: + section_match = None + + section = section_match.group(1) if section_match else "" + + # Combine header + section + footer + return header + section + footer + + +def _make_variant_sig_A_only(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: + """Keep only section A (BUGS & LOGIC ERRORS) from system prompt. + + Run on top of core_only. + """ + # First apply core_only + msgs1, kwargs1 = _make_variant_core_only(messages, lm_kwargs) + + new_messages = [] + for msg in msgs1: + if msg.get("role") == "system": + content = msg.get("content", "") + if isinstance(content, str): + new_content = _keep_only_section(content, "A") + new_msg = dict(msg) + new_msg["content"] = new_content + new_messages.append(new_msg) + else: + new_messages.append(msg) + else: + new_messages.append(msg) + return new_messages, kwargs1 + + +def _make_variant_sig_C_only(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: + """Keep only section C (CODE SMELLS) from system prompt. + + Run on top of core_only. + """ + # First apply core_only + msgs1, kwargs1 = _make_variant_core_only(messages, lm_kwargs) + + new_messages = [] + for msg in msgs1: + if msg.get("role") == "system": + content = msg.get("content", "") + if isinstance(content, str): + new_content = _keep_only_section(content, "C") + new_msg = dict(msg) + new_msg["content"] = new_content + new_messages.append(new_msg) + else: + new_messages.append(msg) + else: + new_messages.append(msg) + return new_messages, kwargs1 + + +_PREAMBLE_END = "Specific instructions:" +_INTRO_END = "═══" # first banner line, right before ANALYZE CHANGES + + +def _split_head(content: str) -> tuple[str, str, str]: + """Split system content into (rlm_preamble, sig_intro, rest). + + rlm_preamble: start .. "Specific instructions:" (DSPy RLM field listing) + sig_intro: "Specific instructions: Detect VERIFIED ..." .. first ═══ banner + rest: ANALYZE CHANGES banner onward + """ + p = content.find(_PREAMBLE_END) + i = content.find(_INTRO_END, p if p >= 0 else 0) + if p < 0 or i < 0: + return "", "", content + return content[:p], content[p:i], content[i:] + + +def _map_system(messages: list[dict], fn) -> list[dict]: + """Apply fn to the system message content.""" + out = [] + for msg in messages: + if msg.get("role") == "system" and isinstance(msg.get("content"), str): + out.append({**msg, "content": fn(msg["content"])}) + else: + out.append(msg) + return out + + +def _make_variant_core_no_preamble(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: + """core_only minus the RLM preamble (keeps sig intro).""" + msgs, kw = _make_variant_core_only(messages, lm_kwargs) + return _map_system(msgs, lambda c: "".join(_split_head(c)[1:])), kw + + +def _make_variant_core_no_intro(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: + """core_only minus the signature intro (keeps RLM preamble).""" + def fn(c: str) -> str: + pre, _, rest = _split_head(c) + return pre + rest + msgs, kw = _make_variant_core_only(messages, lm_kwargs) + return _map_system(msgs, fn), kw + + +def _make_variant_core_no_head(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: + """core_only minus preamble and intro: tests A+C together.""" + msgs, kw = _make_variant_core_only(messages, lm_kwargs) + return _map_system(msgs, lambda c: _split_head(c)[2]), kw + + +def _make_variant_head_only(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: + """System = preamble + intro only, neutral user.""" + msgs, kw = _make_variant_neutral_user(messages, lm_kwargs) + return _map_system(msgs, lambda c: "".join(_split_head(c)[:2])), kw + + +def _make_variant_preamble_only(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: + """System = RLM preamble only, neutral user.""" + msgs, kw = _make_variant_neutral_user(messages, lm_kwargs) + return _map_system(msgs, lambda c: _split_head(c)[0]), kw + + +def _make_variant_intro_only(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: + """System = signature intro only, neutral user.""" + msgs, kw = _make_variant_neutral_user(messages, lm_kwargs) + return _map_system(msgs, lambda c: _split_head(c)[1]), kw + + +# Stock TwoStepAdapter preamble sentences (dspy 3.4 format_task_description) +_STOCK_HEAD = "You are a helpful assistant that can solve tasks based on user input.\n" +_STOCK_INPUTS = "As input, you will be provided with:\n" +_STOCK_OUTPUTS = "Your outputs must contain:\n" +_STOCK_LAYOUT = "You should lay out your outputs in detail so that your answer can be understood by another agent\n" + + +def _rewrite_preamble(c: str) -> str: + """Mirror DiagnosticTwoStepAdapter.format_task_description.""" + return ( + c.replace(_STOCK_HEAD, "Complete the task below.\n", 1) + .replace(_STOCK_INPUTS, "Inputs:\n", 1) + .replace(_STOCK_OUTPUTS, "Outputs:\n", 1) + .replace(_STOCK_LAYOUT, "", 1) + ) + + +def _make_variant_preamble_rewrite(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: + """Full request with the preamble as codespy's adapter now renders it.""" + return _map_system(messages, _rewrite_preamble), lm_kwargs + + +def _make_variant_preamble_rewrite_only(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: + """System = rewritten preamble only, neutral user.""" + msgs, kw = _make_variant_preamble_only(messages, lm_kwargs) + return _map_system(msgs, _rewrite_preamble), kw + + +def _drop(s: str): + return lambda c: c.replace(s, "", 1) + + +def _make_variant_preamble_no_head(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: + """preamble_only minus the 'helpful assistant' sentence.""" + msgs, kw = _make_variant_preamble_only(messages, lm_kwargs) + return _map_system(msgs, _drop(_STOCK_HEAD)), kw + + +def _make_variant_preamble_no_layout(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: + """preamble_only minus the 'understood by another agent' sentence.""" + msgs, kw = _make_variant_preamble_only(messages, lm_kwargs) + return _map_system(msgs, _drop(_STOCK_LAYOUT.rstrip("\n"))), kw + + +# RLM action-signature field lines (dspy/predict/rlm.py), as rendered in the preamble +_RLM_FIELDS = ("variables_info", "repl_history", "iteration", "reasoning", "code") + + +def _drop_field_lines(c: str, fields: tuple[str, ...]) -> str: + """Drop numbered '`field` (type): desc' lines from the preamble.""" + for f in fields: + c = re.sub(rf"^\d+\. `{f}` \([^)]*\): .*\n", "", c, count=1, flags=re.M) + return c + + +def _pre_drop(field: str): + def fn(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: + msgs, kw = _make_variant_preamble_only(messages, lm_kwargs) + return _map_system(msgs, lambda c: _drop_field_lines(c, (field,))), kw + fn.__doc__ = f"preamble_only minus the `{field}` line." + return fn + + +def _make_variant_pre_no_fields(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: + """preamble_only minus all five field lines (headers + layout sentence only).""" + msgs, kw = _make_variant_preamble_only(messages, lm_kwargs) + return _map_system(msgs, lambda c: _drop_field_lines(c, _RLM_FIELDS)), kw + + +_REASONING_DESC = "Think step-by-step: what do you know? What remains? Plan your next action." +_CODE_DESC = "Python code to execute." + + +def _make_variant_full_reasoning_desc(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: + """Full request with the `reasoning` field description neutralised.""" + return _map_system(messages, lambda c: c.replace(_REASONING_DESC, "Short plan for the next step.")), lm_kwargs + + +def _make_variant_full_code_desc(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: + """Full request with the `code` field description neutralised.""" + return _map_system(messages, lambda c: c.replace(_CODE_DESC, "Python snippet for the next step.")), lm_kwargs + + +def _print_result( + variant: str, + result: dict[str, Any], + orig_messages: list[dict], + variant_messages: list[dict], + orig_kwargs: dict, + variant_kwargs: dict, +) -> None: """Print the result of a variant run.""" + msg_changed = _messages_changed(orig_messages, variant_messages) + kwargs_changed = _kwargs_changed(orig_kwargs, variant_kwargs) + changed = msg_changed or kwargs_changed + print(f"\n{'='*60}") print(f"Variant: {variant}") + print(f"changed={changed} (msg_changed={msg_changed}, kwargs_changed={kwargs_changed})") print(f"{'='*60}") if result["success"]: print(f" finish_reason: {result['finish_reason']}") print(f" prompt_tokens: {result['prompt_tokens']}") print(f" completion_tokens: {result['completion_tokens']}") print(f" reasoning_tokens: {result['reasoning_tokens']}") + if "raw_stop_reason" in result: + print(f" raw_stop_reason: {result['raw_stop_reason']}") + if "request_id" in result: + print(f" request_id: {result['request_id']}") content_preview = result['content'][:200] if result['content'] else "" print(f" content_preview: {content_preview}...") else: @@ -176,6 +661,15 @@ def main() -> int: "--alt-model", help="Alternative model to test (e.g., bedrock/anthropic.claude-sonnet-4.1)", ) + parser.add_argument( + "--raw", + action="store_true", + help="Run baseline through boto3 bedrock-runtime directly and print raw response", + ) + parser.add_argument( + "--only", + help="Comma-separated list of variants to run (e.g., 'baseline,trivial,core_only')", + ) args = parser.parse_args() dump_path = args.dump @@ -196,24 +690,76 @@ def main() -> int: print(f" Messages: {len(messages)}") print(f" LM kwargs: {_strip_api_keys(lm_kwargs)}") - # Run variants - variants = [ + # Handle --raw: run boto3 directly for baseline + if args.raw: + print(f"\n{'='*60}") + print(f"RAW BEDROCK CONVERSE (baseline)") + print(f"{'='*60}") + result = _run_raw_bedrock(model, messages, lm_kwargs) + if result["success"]: + print(f" stopReason: {result.get('raw_stop_reason', result['finish_reason'])}") + print(f" prompt_tokens: {result['prompt_tokens']}") + print(f" completion_tokens: {result['completion_tokens']}") + print(f" request_id: {result.get('request_id', 'N/A')}") + content_preview = result['content'][:200] if result['content'] else "" + print(f" content_preview: {content_preview}...") + else: + print(f" ERROR: {result['error']}") + + # Turn off litellm caching + litellm.cache = None + + # Define all variants + all_variants = [ ("baseline", _make_variant_baseline), ("no_reasoning", _make_variant_no_reasoning), ("no_security", _make_variant_no_security), ("no_repl", _make_variant_no_repl), + ("no_tools", _make_variant_no_tools), + ("no_security_no_repl", _make_variant_no_security_no_repl), ("no_patch", _make_variant_no_patch), + ("neutral_user", _make_variant_neutral_user), + ("no_cache_control", _make_variant_no_cache_control), + ("trivial", _make_variant_trivial), + ("core_only", _make_variant_core_only), + ("no_sig", _make_variant_no_sig), + ("sig_A_only", _make_variant_sig_A_only), + ("sig_C_only", _make_variant_sig_C_only), + ("core_no_preamble", _make_variant_core_no_preamble), + ("core_no_intro", _make_variant_core_no_intro), + ("core_no_head", _make_variant_core_no_head), + ("head_only", _make_variant_head_only), + ("preamble_only", _make_variant_preamble_only), + ("intro_only", _make_variant_intro_only), + ("preamble_no_head", _make_variant_preamble_no_head), + ("preamble_no_layout", _make_variant_preamble_no_layout), + ("preamble_rewrite_only", _make_variant_preamble_rewrite_only), + ("preamble_rewrite", _make_variant_preamble_rewrite), + *((f"pre_drop_{f}", _pre_drop(f)) for f in _RLM_FIELDS), + ("pre_no_fields", _make_variant_pre_no_fields), + ("full_reasoning_desc", _make_variant_full_reasoning_desc), + ("full_code_desc", _make_variant_full_code_desc), ] + # Filter variants if --only specified + if args.only: + only_set = set(args.only.split(",")) + variants = [(n, f) for n, f in all_variants if n in only_set] + if not variants: + print(f"Error: No variants matched '{args.only}'", file=sys.stderr) + return 1 + else: + variants = all_variants + for variant_name, variant_fn in variants: variant_messages, variant_kwargs = variant_fn(messages, lm_kwargs) result = _run_completion(model, variant_messages, variant_kwargs) - _print_result(variant_name, result) + _print_result(variant_name, result, messages, variant_messages, lm_kwargs, variant_kwargs) # Run alt_model variant if specified if args.alt_model: result = _run_completion(args.alt_model, messages, lm_kwargs) - _print_result(f"alt_model ({args.alt_model})", result) + _print_result(f"alt_model ({args.alt_model})", result, messages, messages, lm_kwargs, lm_kwargs) return 0 diff --git a/src/codespy/agents/context_safe.py b/src/codespy/agents/context_safe.py index 90e71cc..29322cb 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 (see scripts/replay_refusal.py). +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,7 +239,7 @@ 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, diff --git a/src/codespy/agents/rlm_tools.py b/src/codespy/agents/rlm_tools.py index 0114d17..4b34a9e 100644 --- a/src/codespy/agents/rlm_tools.py +++ b/src/codespy/agents/rlm_tools.py @@ -15,7 +15,7 @@ import dspy # type: ignore[import-untyped] -from codespy.agents.context_safe import ContextSafe +from codespy.agents.context_safe import CodespyRLM, ContextSafe logger = logging.getLogger(__name__) @@ -72,6 +72,33 @@ def _get_tool_arg_desc(tool: Any) -> dict[str, str] | None: 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, @@ -160,6 +187,7 @@ def sync_wrapper(**kwargs: Any) -> Any: 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( @@ -209,7 +237,7 @@ def build_rlm_agent( bridged_tools = bridge_tools_to_loop(tools, loop, timeout) # Build RLM with bridged tools - rlm = dspy.RLM( + rlm = CodespyRLM( signature, tools=bridged_tools, max_iters=max_iters, diff --git a/tests/test_context_safe.py b/tests/test_context_safe.py index f512302..2f176d7 100644 --- a/tests/test_context_safe.py +++ b/tests/test_context_safe.py @@ -3,10 +3,11 @@ import os 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 +315,65 @@ 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 codespy.agents.diagnostic_adapter import DiagnosticTwoStepAdapter + sig = self._rlm().generate_action.signature + text = DiagnosticTwoStepAdapter(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(__import__("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 index b8997f0..2adb601 100644 --- a/tests/test_diagnostic_adapter.py +++ b/tests/test_diagnostic_adapter.py @@ -375,8 +375,14 @@ def test_handles_write_failure(self, mock_get_settings): class TestContentFilterHandling: """Tests for content_filter finish_reason handling.""" - def test_content_filter_raises_descriptive_error(self, caplog): + @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) @@ -458,10 +464,16 @@ def test_non_content_filter_re_raises_original(self): class TestAcallHandling: """Tests for async acall handling.""" - def test_acall_content_filter_raises_descriptive_error(self, caplog): + @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) @@ -501,3 +513,4 @@ async def run_test(): 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_rlm_tools.py b/tests/test_rlm_tools.py index f4f1f0e..625fb62 100644 --- a/tests/test_rlm_tools.py +++ b/tests/test_rlm_tools.py @@ -308,3 +308,54 @@ def test_rlm_with_bridged_mcp_tool(self): Run manually when Deno is available. """ pass + + +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" From 943758672f3bb90dc2a45726c68371dba95d31ec Mon Sep 17 00:00:00 2001 From: Guillaume Simonneau Date: Mon, 5 Oct 2026 23:29:23 +0200 Subject: [PATCH 3/4] fix rlm --- CHANGELOG.md | 2 +- scripts/replay_refusal.py | 768 ----------------------------- src/codespy/agents/context_safe.py | 2 +- src/codespy/agents/rlm_tools.py | 2 +- tests/test_context_safe.py | 8 +- tests/test_rlm_tools.py | 25 - 6 files changed, 7 insertions(+), 800 deletions(-) delete mode 100644 scripts/replay_refusal.py diff --git a/CHANGELOG.md b/CHANGELOG.md index fbc344e..cac6691 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ - **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). Prompts now request a confidence ("≥0.9 verified with tools; 0.7–0.9 strong evidence; <0.7 weak"), and `filter_by_confidence()` logs raw/kept counts. -- **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. `scripts/replay_refusal.py` replays refusal dumps under bisection variants. +- **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. diff --git a/scripts/replay_refusal.py b/scripts/replay_refusal.py deleted file mode 100644 index 7347195..0000000 --- a/scripts/replay_refusal.py +++ /dev/null @@ -1,768 +0,0 @@ -#!/usr/bin/env python3 -"""Replay a refusal dump through litellm to bisect what triggers the refusal. - -Usage: - python scripts/replay_refusal.py [--alt-model ] [--raw] [--only v1,v2] - -The script replays the dump through litellm.completion with the same model -and kwargs and prints finish_reason and usage for each variant: - - baseline: the dump as-is, to confirm it refuses reproducibly - - no_reasoning: drop reasoning_effort - - no_security: system prompt with "B. SECURITY VULNERABILITIES" block removed - - no_repl: system prompt with RLM REPL template paragraph removed - - no_tools: system prompt with tool docs removed - - no_security_no_repl: both security and REPL removed - - no_patch: user message with changed_files[*].patch replaced by placeholder - - neutral_user: replace user message with neutral content - - no_cache_control: drop cache_control_injection_points - - trivial: minimal system "You are a helpful assistant.", user "Reply with OK." - - core_only: no_security + no_repl + neutral_user combined - - no_sig: delete from "Specific instructions:" up to "You are tasked with producing" - - sig_A_only: keep only section A (BUGS & LOGIC ERRORS) - - sig_C_only: keep only section C (CODE SMELLS) - - core_no_preamble / core_no_intro / core_no_head: core_only minus the RLM - preamble, the signature intro, or both - - head_only / preamble_only / intro_only: system = only that head part, neutral user - - preamble_no_head / preamble_no_layout: preamble_only minus one stock sentence - - preamble_rewrite(_only): preamble with stock sentences neutralised (did not help) - - pre_drop_: preamble_only minus one RLM field line - (variables_info, repl_history, iteration, reasoning, code) - - pre_no_fields: preamble_only minus all five field lines - - full_reasoning_desc / full_code_desc: full request with that field's desc neutralised - Note: no_patch is a no-op on dumps whose user message only holds a truncated - variables_info preview (no patch text present). - - alt_model: same messages on a model given with --alt-model -""" - -import argparse -import json -import os -import re -import sys -from pathlib import Path -from typing import Any - -import litellm - - -def _strip_api_keys(kwargs: dict) -> dict: - """Remove API keys from kwargs for safe display.""" - safe = {} - for key, value in kwargs.items(): - if "key" in key.lower() or "secret" in key.lower() or "token" in key.lower(): - safe[key] = "" - else: - safe[key] = value - return safe - - -def _messages_changed(orig: list[dict], new: list[dict]) -> bool: - """Check if messages list differs from original.""" - return orig != new - - -def _kwargs_changed(orig: dict, new: dict) -> bool: - """Check if kwargs dict differs from original.""" - return orig != new - - -def _run_completion(model: str, messages: list[dict], lm_kwargs: dict) -> dict[str, Any]: - """Run litellm.completion and return result info.""" - try: - # Pass lm_kwargs through as-is (no filter) - response = litellm.completion( - model=model, - messages=messages, - **lm_kwargs, - ) - choice = response.choices[0] if response.choices else {} - message = choice.get("message", {}) if isinstance(choice, dict) else getattr(choice, "message", {}) - usage = response.usage - - # Extract reasoning_tokens from completion_tokens_details if available - reasoning_tokens = 0 - if hasattr(usage, "completion_tokens_details") and usage.completion_tokens_details: - reasoning_tokens = getattr(usage.completion_tokens_details, "reasoning_tokens", 0) - - return { - "success": True, - "finish_reason": choice.get("finish_reason") if isinstance(choice, dict) else getattr(choice, "finish_reason", "unknown"), - "content": message.get("content", "") if isinstance(message, dict) else getattr(message, "content", ""), - "prompt_tokens": getattr(usage, "prompt_tokens", getattr(usage, "input_tokens", 0)), - "completion_tokens": getattr(usage, "completion_tokens", getattr(usage, "output_tokens", 0)), - "reasoning_tokens": reasoning_tokens, - } - except Exception as e: - return { - "success": False, - "error": str(e), - } - - -def _run_raw_bedrock(model: str, messages: list[dict], lm_kwargs: dict) -> dict[str, Any]: - """Run boto3 bedrock-runtime converse directly and return result info.""" - try: - import boto3 - except ImportError: - return {"success": False, "error": "boto3 not installed"} - - try: - # Strip bedrock/converse/ prefix from model - model_id = model.replace("bedrock/converse/", "") - - # Build system and messages for converse API - system_text = "" - converse_messages = [] - - for msg in messages: - if msg.get("role") == "system": - content = msg.get("content", "") - if isinstance(content, str): - system_text = content - elif msg.get("role") in ("user", "human"): - content = msg.get("content", "") - if isinstance(content, str): - converse_messages.append({ - "role": "user", - "content": [{"text": content}] - }) - elif msg.get("role") == "assistant": - content = msg.get("content", "") - if isinstance(content, str): - converse_messages.append({ - "role": "assistant", - "content": [{"text": content}] - }) - - # Build inference config from lm_kwargs - inference_config = {} - if "max_tokens" in lm_kwargs: - inference_config["maxTokens"] = lm_kwargs["max_tokens"] - if "temperature" in lm_kwargs: - inference_config["temperature"] = lm_kwargs["temperature"] - - region = os.environ.get("AWS_REGION") or os.environ.get("AWS_DEFAULT_REGION") or "us-east-1" - client = boto3.client("bedrock-runtime", region_name=region) - - request_kwargs = { - "modelId": model_id, - "messages": converse_messages, - } - if system_text: - request_kwargs["system"] = [{"text": system_text}] - if inference_config: - request_kwargs["inferenceConfig"] = inference_config - - response = client.converse(**request_kwargs) - - # Extract response details - output = response.get("output", {}) - message = output.get("message", {}) - content_blocks = message.get("content", []) - content_text = "" - for block in content_blocks: - if "text" in block: - content_text += block["text"] - - usage = response.get("usage", {}) - stop_reason = output.get("stopReason", "unknown") - request_id = response.get("ResponseMetadata", {}).get("RequestId", "unknown") - - return { - "success": True, - "finish_reason": stop_reason, - "content": content_text, - "prompt_tokens": usage.get("inputTokens", 0), - "completion_tokens": usage.get("outputTokens", 0), - "reasoning_tokens": 0, - "raw_stop_reason": stop_reason, - "request_id": request_id, - } - except Exception as e: - return { - "success": False, - "error": str(e), - } - - -def _make_variant_baseline(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: - """Baseline: use messages and kwargs as-is.""" - return messages, lm_kwargs - - -def _make_variant_no_reasoning(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: - """Remove reasoning_effort from kwargs.""" - new_kwargs = {k: v for k, v in lm_kwargs.items() if k != "reasoning_effort"} - return messages, new_kwargs - - -def _remove_security_block(content: str) -> str: - """Remove the 'B. SECURITY VULNERABILITIES' section from system prompt. - - Delete from '─── B. SECURITY' up to (not including) '─── C. CODE SMELLS' - """ - # Match from "─── B. SECURITY" up to but not including "─── C. CODE SMELLS" - pattern = r"(─── B\.\s*SECURITY.*?)(?=─── C\.\s*CODE SMELLS)" - return re.sub(pattern, "", content, flags=re.DOTALL | re.IGNORECASE) - - -def _make_variant_no_security(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: - """Remove security vulnerabilities section from system prompt.""" - new_messages = [] - for msg in messages: - if msg.get("role") == "system": - content = msg.get("content", "") - if isinstance(content, str): - new_content = _remove_security_block(content) - new_msg = dict(msg) - new_msg["content"] = new_content - new_messages.append(new_msg) - else: - new_messages.append(msg) - else: - new_messages.append(msg) - return new_messages, lm_kwargs - - -def _remove_repl_paragraph(content: str) -> str: - """Remove the RLM REPL template from system prompt. - - Delete from "You have access to a Python REPL" to the end of the system message. - """ - pattern = r"You have access to a Python REPL.*" - return re.sub(pattern, "", content, flags=re.DOTALL | re.IGNORECASE) - - -def _make_variant_no_repl(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: - """Remove REPL/code-execution framing from system prompt.""" - new_messages = [] - for msg in messages: - if msg.get("role") == "system": - content = msg.get("content", "") - if isinstance(content, str): - new_content = _remove_repl_paragraph(content) - new_msg = dict(msg) - new_msg["content"] = new_content - new_messages.append(new_msg) - else: - new_messages.append(msg) - else: - new_messages.append(msg) - return new_messages, lm_kwargs - - -def _remove_tools_section(content: str) -> str: - """Remove the tool documentation from system prompt. - - Delete from "Additional tools available" to the end of the system message. - """ - pattern = r"Additional tools available.*" - return re.sub(pattern, "", content, flags=re.DOTALL | re.IGNORECASE) - - -def _make_variant_no_tools(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: - """Remove tool docs from system prompt.""" - new_messages = [] - for msg in messages: - if msg.get("role") == "system": - content = msg.get("content", "") - if isinstance(content, str): - new_content = _remove_tools_section(content) - new_msg = dict(msg) - new_msg["content"] = new_content - new_messages.append(new_msg) - else: - new_messages.append(msg) - else: - new_messages.append(msg) - return new_messages, lm_kwargs - - -def _make_variant_no_security_no_repl(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: - """Remove both security section and REPL template from system prompt.""" - # First apply no_security - msgs1, kwargs1 = _make_variant_no_security(messages, lm_kwargs) - # Then apply no_repl - return _make_variant_no_repl(msgs1, kwargs1) - - -def _remove_patches(content: str) -> str: - """Replace patch content in JSON strings with placeholder.""" - # Handle escaped newlines in JSON strings - # The patch field contains actual newlines that are escaped in JSON - # Match "patch": "..." with content containing escaped newlines - pattern = r'("patch"\s*:\s*")((?:[^"\\]|\\.)*?)(")' - - def replace_patch(match): - return match.group(1) + "[PATCH REDACTED]" + match.group(3) - - return re.sub(pattern, replace_patch, content, flags=re.DOTALL) - - -def _make_variant_no_patch(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: - """Replace patch content in user message with placeholder.""" - new_messages = [] - for msg in messages: - if msg.get("role") in ("user", "human"): - content = msg.get("content", "") - if isinstance(content, str): - new_content = _remove_patches(content) - new_msg = dict(msg) - new_msg["content"] = new_content - new_messages.append(new_msg) - else: - new_messages.append(msg) - else: - new_messages.append(msg) - return new_messages, lm_kwargs - - -def _make_variant_neutral_user(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: - """Replace user message with neutral content.""" - new_messages = [] - for msg in messages: - if msg.get("role") in ("user", "human"): - new_msg = dict(msg) - new_msg["content"] = "variables_info: []\n\nrepl_history: entries=[] max_output_chars=10000\n\niteration: 1/5" - new_messages.append(new_msg) - else: - new_messages.append(msg) - return new_messages, lm_kwargs - - -def _make_variant_no_cache_control(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: - """Remove cache_control_injection_points from kwargs.""" - new_kwargs = {k: v for k, v in lm_kwargs.items() if k != "cache_control_injection_points"} - return messages, new_kwargs - - -def _make_variant_trivial(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: - """Minimal system and user messages to test if model accepts anything.""" - new_messages = [] - for msg in messages: - if msg.get("role") == "system": - new_msg = dict(msg) - new_msg["content"] = "You are a helpful assistant." - new_messages.append(new_msg) - elif msg.get("role") in ("user", "human"): - new_msg = dict(msg) - new_msg["content"] = "Reply with OK." - new_messages.append(new_msg) - else: - new_messages.append(dict(msg)) - # Keep lm_kwargs as-is - return new_messages, lm_kwargs - - -def _make_variant_core_only(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: - """Combine no_security + no_repl + neutral_user.""" - # Apply no_security - msgs1, kwargs1 = _make_variant_no_security(messages, lm_kwargs) - # Apply no_repl - msgs2, kwargs2 = _make_variant_no_repl(msgs1, kwargs1) - # Apply neutral_user - return _make_variant_neutral_user(msgs2, kwargs2) - - -def _remove_signature_instructions(content: str) -> str: - """Remove from 'Specific instructions:' up to 'You are tasked with producing'. - - Keeps the header, schema and REPL, drops the whole CodeReviewSignature docstring. - """ - # Match from "Specific instructions:" up to but not including "You are tasked with producing" - pattern = r"(Specific instructions:.*?)(?=You are tasked with producing)" - return re.sub(pattern, "", content, flags=re.DOTALL | re.IGNORECASE) - - -def _make_variant_no_sig(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: - """Remove the CodeReviewSignature docstring from system prompt.""" - new_messages = [] - for msg in messages: - if msg.get("role") == "system": - content = msg.get("content", "") - if isinstance(content, str): - new_content = _remove_signature_instructions(content) - new_msg = dict(msg) - new_msg["content"] = new_content - new_messages.append(new_msg) - else: - new_messages.append(msg) - else: - new_messages.append(msg) - return new_messages, lm_kwargs - - -def _keep_only_section(content: str, section_letter: str) -> str: - """Keep only the specified section (A or C) from the signature. - - Section A: BUGS & LOGIC ERRORS - Section C: CODE SMELLS - """ - # Find the ANALYZE CHANGES header and OUTPUT RULES footer - header_match = re.search(r"(ANALYZE CHANGES.*?)(?=─── A\.)", content, re.DOTALL | re.IGNORECASE) - footer_match = re.search(r"(OUTPUT RULES.*?)$", content, re.DOTALL | re.IGNORECASE) - - header = header_match.group(1) if header_match else "ANALYZE CHANGES\n" - footer = footer_match.group(1) if footer_match else "" - - if section_letter.upper() == "A": - # Keep section A (from ─── A. to ─── B.) - section_match = re.search(r"(─── A\.\s*BUGS.*?)(?=─── B\.\s*SECURITY)", content, re.DOTALL | re.IGNORECASE) - elif section_letter.upper() == "C": - # Keep section C (from ─── C. to OUTPUT RULES) - section_match = re.search(r"(─── C\.\s*CODE SMELLS.*?)(?=OUTPUT RULES)", content, re.DOTALL | re.IGNORECASE) - else: - section_match = None - - section = section_match.group(1) if section_match else "" - - # Combine header + section + footer - return header + section + footer - - -def _make_variant_sig_A_only(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: - """Keep only section A (BUGS & LOGIC ERRORS) from system prompt. - - Run on top of core_only. - """ - # First apply core_only - msgs1, kwargs1 = _make_variant_core_only(messages, lm_kwargs) - - new_messages = [] - for msg in msgs1: - if msg.get("role") == "system": - content = msg.get("content", "") - if isinstance(content, str): - new_content = _keep_only_section(content, "A") - new_msg = dict(msg) - new_msg["content"] = new_content - new_messages.append(new_msg) - else: - new_messages.append(msg) - else: - new_messages.append(msg) - return new_messages, kwargs1 - - -def _make_variant_sig_C_only(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: - """Keep only section C (CODE SMELLS) from system prompt. - - Run on top of core_only. - """ - # First apply core_only - msgs1, kwargs1 = _make_variant_core_only(messages, lm_kwargs) - - new_messages = [] - for msg in msgs1: - if msg.get("role") == "system": - content = msg.get("content", "") - if isinstance(content, str): - new_content = _keep_only_section(content, "C") - new_msg = dict(msg) - new_msg["content"] = new_content - new_messages.append(new_msg) - else: - new_messages.append(msg) - else: - new_messages.append(msg) - return new_messages, kwargs1 - - -_PREAMBLE_END = "Specific instructions:" -_INTRO_END = "═══" # first banner line, right before ANALYZE CHANGES - - -def _split_head(content: str) -> tuple[str, str, str]: - """Split system content into (rlm_preamble, sig_intro, rest). - - rlm_preamble: start .. "Specific instructions:" (DSPy RLM field listing) - sig_intro: "Specific instructions: Detect VERIFIED ..." .. first ═══ banner - rest: ANALYZE CHANGES banner onward - """ - p = content.find(_PREAMBLE_END) - i = content.find(_INTRO_END, p if p >= 0 else 0) - if p < 0 or i < 0: - return "", "", content - return content[:p], content[p:i], content[i:] - - -def _map_system(messages: list[dict], fn) -> list[dict]: - """Apply fn to the system message content.""" - out = [] - for msg in messages: - if msg.get("role") == "system" and isinstance(msg.get("content"), str): - out.append({**msg, "content": fn(msg["content"])}) - else: - out.append(msg) - return out - - -def _make_variant_core_no_preamble(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: - """core_only minus the RLM preamble (keeps sig intro).""" - msgs, kw = _make_variant_core_only(messages, lm_kwargs) - return _map_system(msgs, lambda c: "".join(_split_head(c)[1:])), kw - - -def _make_variant_core_no_intro(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: - """core_only minus the signature intro (keeps RLM preamble).""" - def fn(c: str) -> str: - pre, _, rest = _split_head(c) - return pre + rest - msgs, kw = _make_variant_core_only(messages, lm_kwargs) - return _map_system(msgs, fn), kw - - -def _make_variant_core_no_head(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: - """core_only minus preamble and intro: tests A+C together.""" - msgs, kw = _make_variant_core_only(messages, lm_kwargs) - return _map_system(msgs, lambda c: _split_head(c)[2]), kw - - -def _make_variant_head_only(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: - """System = preamble + intro only, neutral user.""" - msgs, kw = _make_variant_neutral_user(messages, lm_kwargs) - return _map_system(msgs, lambda c: "".join(_split_head(c)[:2])), kw - - -def _make_variant_preamble_only(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: - """System = RLM preamble only, neutral user.""" - msgs, kw = _make_variant_neutral_user(messages, lm_kwargs) - return _map_system(msgs, lambda c: _split_head(c)[0]), kw - - -def _make_variant_intro_only(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: - """System = signature intro only, neutral user.""" - msgs, kw = _make_variant_neutral_user(messages, lm_kwargs) - return _map_system(msgs, lambda c: _split_head(c)[1]), kw - - -# Stock TwoStepAdapter preamble sentences (dspy 3.4 format_task_description) -_STOCK_HEAD = "You are a helpful assistant that can solve tasks based on user input.\n" -_STOCK_INPUTS = "As input, you will be provided with:\n" -_STOCK_OUTPUTS = "Your outputs must contain:\n" -_STOCK_LAYOUT = "You should lay out your outputs in detail so that your answer can be understood by another agent\n" - - -def _rewrite_preamble(c: str) -> str: - """Mirror DiagnosticTwoStepAdapter.format_task_description.""" - return ( - c.replace(_STOCK_HEAD, "Complete the task below.\n", 1) - .replace(_STOCK_INPUTS, "Inputs:\n", 1) - .replace(_STOCK_OUTPUTS, "Outputs:\n", 1) - .replace(_STOCK_LAYOUT, "", 1) - ) - - -def _make_variant_preamble_rewrite(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: - """Full request with the preamble as codespy's adapter now renders it.""" - return _map_system(messages, _rewrite_preamble), lm_kwargs - - -def _make_variant_preamble_rewrite_only(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: - """System = rewritten preamble only, neutral user.""" - msgs, kw = _make_variant_preamble_only(messages, lm_kwargs) - return _map_system(msgs, _rewrite_preamble), kw - - -def _drop(s: str): - return lambda c: c.replace(s, "", 1) - - -def _make_variant_preamble_no_head(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: - """preamble_only minus the 'helpful assistant' sentence.""" - msgs, kw = _make_variant_preamble_only(messages, lm_kwargs) - return _map_system(msgs, _drop(_STOCK_HEAD)), kw - - -def _make_variant_preamble_no_layout(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: - """preamble_only minus the 'understood by another agent' sentence.""" - msgs, kw = _make_variant_preamble_only(messages, lm_kwargs) - return _map_system(msgs, _drop(_STOCK_LAYOUT.rstrip("\n"))), kw - - -# RLM action-signature field lines (dspy/predict/rlm.py), as rendered in the preamble -_RLM_FIELDS = ("variables_info", "repl_history", "iteration", "reasoning", "code") - - -def _drop_field_lines(c: str, fields: tuple[str, ...]) -> str: - """Drop numbered '`field` (type): desc' lines from the preamble.""" - for f in fields: - c = re.sub(rf"^\d+\. `{f}` \([^)]*\): .*\n", "", c, count=1, flags=re.M) - return c - - -def _pre_drop(field: str): - def fn(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: - msgs, kw = _make_variant_preamble_only(messages, lm_kwargs) - return _map_system(msgs, lambda c: _drop_field_lines(c, (field,))), kw - fn.__doc__ = f"preamble_only minus the `{field}` line." - return fn - - -def _make_variant_pre_no_fields(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: - """preamble_only minus all five field lines (headers + layout sentence only).""" - msgs, kw = _make_variant_preamble_only(messages, lm_kwargs) - return _map_system(msgs, lambda c: _drop_field_lines(c, _RLM_FIELDS)), kw - - -_REASONING_DESC = "Think step-by-step: what do you know? What remains? Plan your next action." -_CODE_DESC = "Python code to execute." - - -def _make_variant_full_reasoning_desc(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: - """Full request with the `reasoning` field description neutralised.""" - return _map_system(messages, lambda c: c.replace(_REASONING_DESC, "Short plan for the next step.")), lm_kwargs - - -def _make_variant_full_code_desc(messages: list[dict], lm_kwargs: dict) -> tuple[list[dict], dict]: - """Full request with the `code` field description neutralised.""" - return _map_system(messages, lambda c: c.replace(_CODE_DESC, "Python snippet for the next step.")), lm_kwargs - - -def _print_result( - variant: str, - result: dict[str, Any], - orig_messages: list[dict], - variant_messages: list[dict], - orig_kwargs: dict, - variant_kwargs: dict, -) -> None: - """Print the result of a variant run.""" - msg_changed = _messages_changed(orig_messages, variant_messages) - kwargs_changed = _kwargs_changed(orig_kwargs, variant_kwargs) - changed = msg_changed or kwargs_changed - - print(f"\n{'='*60}") - print(f"Variant: {variant}") - print(f"changed={changed} (msg_changed={msg_changed}, kwargs_changed={kwargs_changed})") - print(f"{'='*60}") - if result["success"]: - print(f" finish_reason: {result['finish_reason']}") - print(f" prompt_tokens: {result['prompt_tokens']}") - print(f" completion_tokens: {result['completion_tokens']}") - print(f" reasoning_tokens: {result['reasoning_tokens']}") - if "raw_stop_reason" in result: - print(f" raw_stop_reason: {result['raw_stop_reason']}") - if "request_id" in result: - print(f" request_id: {result['request_id']}") - content_preview = result['content'][:200] if result['content'] else "" - print(f" content_preview: {content_preview}...") - else: - print(f" ERROR: {result['error']}") - - -def main() -> int: - """Main entry point.""" - parser = argparse.ArgumentParser( - description="Replay a refusal dump to bisect what triggers the refusal." - ) - parser.add_argument("dump", type=Path, help="Path to the refusal dump JSON file") - parser.add_argument( - "--alt-model", - help="Alternative model to test (e.g., bedrock/anthropic.claude-sonnet-4.1)", - ) - parser.add_argument( - "--raw", - action="store_true", - help="Run baseline through boto3 bedrock-runtime directly and print raw response", - ) - parser.add_argument( - "--only", - help="Comma-separated list of variants to run (e.g., 'baseline,trivial,core_only')", - ) - args = parser.parse_args() - - dump_path = args.dump - if not dump_path.exists(): - print(f"Error: Dump file not found: {dump_path}", file=sys.stderr) - return 1 - - # Load dump - with open(dump_path, "r", encoding="utf-8") as f: - dump = json.load(f) - - model = dump.get("model", "unknown") - lm_kwargs = dump.get("lm_kwargs", {}) - messages = dump.get("messages", []) - - print(f"Loaded refusal dump: {dump_path}") - print(f" Model: {model}") - print(f" Messages: {len(messages)}") - print(f" LM kwargs: {_strip_api_keys(lm_kwargs)}") - - # Handle --raw: run boto3 directly for baseline - if args.raw: - print(f"\n{'='*60}") - print(f"RAW BEDROCK CONVERSE (baseline)") - print(f"{'='*60}") - result = _run_raw_bedrock(model, messages, lm_kwargs) - if result["success"]: - print(f" stopReason: {result.get('raw_stop_reason', result['finish_reason'])}") - print(f" prompt_tokens: {result['prompt_tokens']}") - print(f" completion_tokens: {result['completion_tokens']}") - print(f" request_id: {result.get('request_id', 'N/A')}") - content_preview = result['content'][:200] if result['content'] else "" - print(f" content_preview: {content_preview}...") - else: - print(f" ERROR: {result['error']}") - - # Turn off litellm caching - litellm.cache = None - - # Define all variants - all_variants = [ - ("baseline", _make_variant_baseline), - ("no_reasoning", _make_variant_no_reasoning), - ("no_security", _make_variant_no_security), - ("no_repl", _make_variant_no_repl), - ("no_tools", _make_variant_no_tools), - ("no_security_no_repl", _make_variant_no_security_no_repl), - ("no_patch", _make_variant_no_patch), - ("neutral_user", _make_variant_neutral_user), - ("no_cache_control", _make_variant_no_cache_control), - ("trivial", _make_variant_trivial), - ("core_only", _make_variant_core_only), - ("no_sig", _make_variant_no_sig), - ("sig_A_only", _make_variant_sig_A_only), - ("sig_C_only", _make_variant_sig_C_only), - ("core_no_preamble", _make_variant_core_no_preamble), - ("core_no_intro", _make_variant_core_no_intro), - ("core_no_head", _make_variant_core_no_head), - ("head_only", _make_variant_head_only), - ("preamble_only", _make_variant_preamble_only), - ("intro_only", _make_variant_intro_only), - ("preamble_no_head", _make_variant_preamble_no_head), - ("preamble_no_layout", _make_variant_preamble_no_layout), - ("preamble_rewrite_only", _make_variant_preamble_rewrite_only), - ("preamble_rewrite", _make_variant_preamble_rewrite), - *((f"pre_drop_{f}", _pre_drop(f)) for f in _RLM_FIELDS), - ("pre_no_fields", _make_variant_pre_no_fields), - ("full_reasoning_desc", _make_variant_full_reasoning_desc), - ("full_code_desc", _make_variant_full_code_desc), - ] - - # Filter variants if --only specified - if args.only: - only_set = set(args.only.split(",")) - variants = [(n, f) for n, f in all_variants if n in only_set] - if not variants: - print(f"Error: No variants matched '{args.only}'", file=sys.stderr) - return 1 - else: - variants = all_variants - - for variant_name, variant_fn in variants: - variant_messages, variant_kwargs = variant_fn(messages, lm_kwargs) - result = _run_completion(model, variant_messages, variant_kwargs) - _print_result(variant_name, result, messages, variant_messages, lm_kwargs, variant_kwargs) - - # Run alt_model variant if specified - if args.alt_model: - result = _run_completion(args.alt_model, messages, lm_kwargs) - _print_result(f"alt_model ({args.alt_model})", result, messages, messages, lm_kwargs, lm_kwargs) - - return 0 - - -if __name__ == "__main__": - sys.exit(main()) diff --git a/src/codespy/agents/context_safe.py b/src/codespy/agents/context_safe.py index 29322cb..8e6f0a9 100644 --- a/src/codespy/agents/context_safe.py +++ b/src/codespy/agents/context_safe.py @@ -22,7 +22,7 @@ # 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 (see scripts/replay_refusal.py). +# content_filter on Claude Opus 5.5. RLM_REASONING_DESC = "Short plan for the next step." diff --git a/src/codespy/agents/rlm_tools.py b/src/codespy/agents/rlm_tools.py index 4b34a9e..0086035 100644 --- a/src/codespy/agents/rlm_tools.py +++ b/src/codespy/agents/rlm_tools.py @@ -19,7 +19,7 @@ logger = logging.getLogger(__name__) -# Thread-local storage for the event loop at bridge time +# Thread id of the event loop at bridge time _bridge_thread_id: int | None = None diff --git a/tests/test_context_safe.py b/tests/test_context_safe.py index 2f176d7..c8a10af 100644 --- a/tests/test_context_safe.py +++ b/tests/test_context_safe.py @@ -1,6 +1,7 @@ """Tests for ContextSafe proactive RLM fallback thresholds.""" import os +import shutil from unittest.mock import MagicMock, patch import dspy # type: ignore[import-untyped] @@ -337,9 +338,9 @@ def test_action_reasoning_desc_replaced(self): assert "Do the thing." in sig.instructions def test_rendered_prompt_has_no_stock_desc(self): - from codespy.agents.diagnostic_adapter import DiagnosticTwoStepAdapter + from dspy.adapters.two_step_adapter import TwoStepAdapter sig = self._rlm().generate_action.signature - text = DiagnosticTwoStepAdapter(dspy.LM("openai/gpt-4o-mini")).format_task_description(sig) + 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 @@ -347,7 +348,6 @@ 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 @@ -360,7 +360,7 @@ class IssueCategory(StrEnum): assert _plain({"c": [IssueCategory.BUG, ("x", IssueCategory.SMELL)]}) == {"c": ["bug", ["x", "smell"]]} assert _plain("bug") == "bug" - @pytest.mark.skipif(__import__("shutil").which("deno") is None, reason="needs deno") + @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 diff --git a/tests/test_rlm_tools.py b/tests/test_rlm_tools.py index 625fb62..20da415 100644 --- a/tests/test_rlm_tools.py +++ b/tests/test_rlm_tools.py @@ -16,13 +16,6 @@ ) -# Skip tests that require Deno if it's not installed -requires_deno = pytest.mark.skipif( - shutil.which("deno") is None, - reason="Deno not installed" -) - - class TestIsCoroutineFunction: """Test the _is_coroutine_function helper.""" @@ -292,24 +285,6 @@ def test_log_rlm_outcome_dict_trajectory(self, caplog): assert "1 tool errors" in caplog.text -@requires_deno -class TestDenoIntegration: - """Integration tests that require Deno to be installed.""" - - @pytest.mark.skip(reason="Requires full DSPy + MCP setup - run manually") - def test_rlm_with_bridged_mcp_tool(self): - """Full integration: RLM agent with bridged MCP tool via Deno. - - This test would require: - 1. A running MCP server via Deno - 2. DSPy RLM with bridged tools - 3. Executing on the event loop thread - - Run manually when Deno is available. - """ - pass - - class TestBridgedToolSignature: """Bridged tools must keep their real signature inside the RLM sandbox.""" From 7b7bad23aa343192edabd11b2c2655ee790d70e1 Mon Sep 17 00:00:00 2001 From: Guillaume Simonneau Date: Mon, 5 Oct 2026 23:39:21 +0200 Subject: [PATCH 4/4] fix rlm --- CHANGELOG.md | 12 ++++++++++-- pyproject.toml | 2 +- src/codespy/__init__.py | 2 +- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cac6691..ce79288 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,17 +2,25 @@ ## [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). Prompts now request a confidence ("≥0.9 verified with tools; 0.7–0.9 strong evidence; <0.7 weak"), and `filter_by_confidence()` logs raw/kept counts. +- **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). Update your codespy.yaml or environment variable to override if needed. +- **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 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"