Share the code-suggestion thread context across providers

.github/workflows/release-drafter.yml

@@ -9,6 +9,7 @@ on:
       - opened
       - reopened
       - synchronize
+      - edited
 
 permissions:
   contents: read
@@ -26,7 +27,9 @@ jobs:
           token: ${{ secrets.GITHUB_TOKEN }}
 
   auto_label:
-    if: github.event_name == 'pull_request_target'
+    if: >-
+      github.event_name == 'pull_request_target' &&
+      (github.event.action != 'edited' || github.event.changes.title)
     permissions:
       contents: read
       pull-requests: write

docs/docs/installation/azure.md

@@ -122,6 +122,10 @@ configure a stable identity (GUID/ID, unique name, or descriptor). A list can be
 agent_identity = "<agent identity>"
 ```
 
+The same identity gates the prior-suggestion thread context that `/improve` reads: a thread is only included when its
+opening comment is attributable to that identity, so leave `agent_identity` configured if you rely on the context.
+`pr_code_suggestions.max_discussion_context_chars` caps the injected block, and `0` disables it.
+
 For webhook security, create a sporadic username/password pair and configure the webhook username and password on both the server and Azure DevOps webhook. These will be sent as basic Auth data by the webhook with each request:
 
 ```toml

docs/docs/tools/review.md

@@ -116,7 +116,7 @@ for the authoritative default values.
       </tr>
       <tr>
         <td><b>inline_key_issues</b></td>
-        <td>If set to true, each key issue is published as an inline comment where the provider supports verified inline-comment publication. A finding leaves the review summary when a matching comment exists or the provider accepts the new comment. Findings that cannot be anchored or published stay in the summary.</td>
+        <td>If set to true, each key issue is published as an inline comment where the provider supports verified inline-comment publication (GitHub, Bitbucket Cloud, Azure DevOps, GitLab). A finding leaves the review summary when a matching comment exists or the provider accepts the new comment. Findings that cannot be anchored or published stay in the summary.</td>
       </tr>
     </table>
 

docs/docs/usage-guide/configuration_reference.md

@@ -117,7 +117,7 @@ to-do list.
 | `add_user_to_requests` | false | send the current command and PR URL in the OpenAI-compatible "user" request field, for provider-side attribution of requests (e.g. OpenRouter "external_user") |
 | `reasoning_effort` | "medium" | "none", "minimal", "low", "medium", "high", "xhigh", "max" |
 | `additional_reasoning_effort_models` | [] | Optional: additional model IDs that accept config.reasoning_effort. Reasoning support is otherwise decided by litellm's bundled model metadata (and the maintained Grok registry), so add an ID here only when litellm does not know the model (custom OpenAI-compatible endpoints). Model IDs match exactly or through any provider prefix (e.g. "deepseek-v4-flash-0731" matches "openai/deepseek-v4-flash-0731"). LiteLLM whitelists reasoning_effort through allowed_openai_params for OpenAI-compatible models it does not recognize, so the parameter reaches the endpoint. The default "medium" may be rejected by providers that accept a different subset (e.g. "none"/"low"/"high"/"max"); adding a custom model id now surfaces a provider-side error instead of the previous silent drop. |
-| `no_temperature_models` | ["deepseek/deepseek-reasoner", "o1-mini", "o1-mini-2024-09-12", "o1", "o1-2024-12-17", "o3-mini", "o3-mini-2025-01-31", "o1-preview", "o3", "o3-2025-04-16", "o4-mini", "o4-mini-2025-04-16", "gpt-5.1-codex", "gpt-5.1-codex-mini", "gpt-5.2-codex", "gpt-5.3-codex", "gpt-5-mini"] | Optional: model IDs that must never receive the temperature parameter, on top of what litellm's parameter metadata reports. Temperature support is otherwise decided by litellm.get_supported_openai_params() (mirroring reasoning_effort), so add an ID here when litellm reports temperature as supported but the provider rejects it, or when an OpenAI-compatible endpoint accepts but you still want it dropped. Adaptive-thinking Claude models (Opus 4.7/4.8 and Opus/Sonnet/Fable 5) never receive temperature. Model IDs match exactly or through any provider prefix. The defaults below preserve the former static registry entries that litellm's metadata still marks temperature-capable; the probe diff for this change is posted on the issue. |
+| `no_temperature_models` | ["deepseek/deepseek-reasoner", "o1-mini", "o1-mini-2024-09-12", "o1", "o1-2024-12-17", "o3-mini", "o3-mini-2025-01-31", "o3", "o3-2025-04-16", "o4-mini", "o4-mini-2025-04-16", "gpt-5.1-codex", "gpt-5.1-codex-mini", "gpt-5.2-codex", "gpt-5.3-codex", "gpt-5-mini"] | Optional: model IDs that must never receive the temperature parameter, on top of what litellm's parameter metadata reports. Temperature support is otherwise decided by litellm.get_supported_openai_params() (mirroring reasoning_effort), so add an ID here when litellm reports temperature as supported but the provider rejects it, or when an OpenAI-compatible endpoint accepts but you still want it dropped. Adaptive-thinking Claude models (Opus 4.7/4.8 and Opus/Sonnet/Fable 5) never receive temperature. Model IDs match exactly or through any provider prefix. The defaults below preserve the former static registry entries that litellm's metadata still marks temperature-capable; the probe diff for this change is posted on the issue. |
 **extended thinking for Claude reasoning models**
 
 | Key | Default | Description |
@@ -159,7 +159,7 @@ to-do list.
 | `persistent_comment` | true |  |
 | `review_heading` | "PR Reviewer Guide" | Visible base heading for full and incremental review comments. Identity is tracked separately. |
 | `persistent_finding_state` | true | Persist review finding state across complete review runs. |
-| `inline_key_issues` | false |  |
+| `inline_key_issues` | false | Publish each review finding as an inline comment where the provider can verify inline-comment publication (GitHub, Bitbucket Cloud, Azure DevOps, GitLab). |
 | `extra_instructions` | "" |  |
 | `num_max_findings` | 3 |  |
 | `final_update_message` | true |  |
@@ -266,6 +266,7 @@ to-do list.
 | `max_suggestions_per_file` | 0 | Maximum suggestions retained per file after all chunks are merged; 0 disables the cap. Skip unresolvable line locations before applying a positive cap to summarized output; leave inline selection unchanged. |
 | `max_number_of_calls` | 3 |  |
 | `parallel_calls` | true |  |
+| `max_discussion_context_chars` | 24000 | Character budget for the prior code-suggestion thread context injected into the /improve prompt; 0 disables it. |
 | `decouple_hunks` | false |  |
 **self-review checkbox**
 
@@ -429,7 +430,7 @@ _This section only documents commented-out examples; see the [TOML source](https
 | `force_streaming_custom_llm_provider` | "" | Force streaming when the request matches this provider AND its api_base contains one of the substrings below. Some OpenAI-compatible endpoints return a response that LiteLLM cannot normalize in non-streaming mode. Both must be set for the workaround to apply. |
 | `force_streaming_api_base_substrings` | [] |  |
 | `callback_timeout_seconds` | 30 | max seconds to wait for pending litellm callbacks to flush before exiting |
-| `cache_control_injection_points` | [] | Optional: enable Anthropic prompt caching via LiteLLM, e.g. [{location = "message", role = "system"}] (https://docs.litellm.ai/docs/tutorials/prompt_caching) |
+| `cache_control_injection_points` | [] | Optional: enable Anthropic prompt caching via LiteLLM, e.g. [{location = "message", role = "system"}] (https://docs.litellm.ai/docs/tutorials/prompt_caching). PR-Agent forwards these points only for models whose name contains "claude"; LiteLLM adds the cache_control blocks. LiteLLM's own default injection (`litellm.enable_anthropic_prompt_caching`, env `LITELLM_ENABLE_ANTHROPIC_PROMPT_CACHING`, off by default) applies only when no points are configured here, so the two never double-inject. A warning is logged once per process when the points cannot take effect (non-Anthropic model, no prompt-cache support, or a prefix below the model's minimum). |
 
 
 ## `[openrouter]`

pr_agent/agent/pr_agent.py

@@ -10,6 +10,7 @@
 
 from pr_agent.algo.ai_handlers.base_ai_handler import BaseAiHandler
 from pr_agent.algo.ai_handlers.litellm_ai_handler import LiteLLMAIHandler
+from pr_agent.algo.artifacts import reapply_artifact_context
 from pr_agent.algo.cli_args import CliArgs
 from pr_agent.algo.comment_identity import add_comment_identity, comment_matches_identity
 from pr_agent.algo.utils import update_settings_from_args
@@ -372,6 +373,8 @@ async def _run_command(
                 span.set_attribute("error.message", f"Unknown command: {action}")
             return False
 
+        reapply_artifact_context()
+
         # Only after validation: an unknown action is arbitrary user input and
         # must not become a span name, span attribute, or metric label.
         span.update_name(f"pr_agent {action}")

pr_agent/algo/ai_handlers/langchain_ai_handler.py

@@ -64,7 +64,9 @@ async def _create_chat_async(self, deployment_id=None):
             raise ValueError(error_msg) from e
 
     @retry(
-        retry=retry_if_exception_type(openai.APIError) & retry_if_not_exception_type(openai.RateLimitError),
+        retry=retry_if_exception_type(openai.APIError) & retry_if_not_exception_type(
+            (openai.RateLimitError, openai.BadRequestError, openai.UnprocessableEntityError)
+        ),
         stop=stop_after_attempt(OPENAI_RETRIES),
     )
     async def chat_completion(

pr_agent/algo/ai_handlers/litellm_ai_handler.py

@@ -141,6 +141,17 @@
 _IMAGE_HEAD_TIMEOUT_SECONDS = 5
 OPENAI_DEFAULT_API_BASE = "https://api.openai.com/v1"
 
+# Token-count allowances used when estimating the cached prompt prefix for the
+# cache_control_injection_points pre-call warning. Mirrors pr_help_message.py.
+_CACHE_MESSAGE_FRAMING_ALLOWANCE = 16
+_CACHE_REPLY_FRAMING_ALLOWANCE = 16
+# Providers that serve Anthropic Claude models and honor cache_control injection.
+_ANTHROPIC_CACHE_REQUEST_PROVIDERS = ("anthropic", "bedrock", "bedrock_mantle", "vertex_ai")
+# One-time warnings telling the operator when an enabled prompt-cache config cannot take
+# effect, keyed by (model, reason) so the same warning is logged once per process. See
+# _warn_prompt_cache_conditions.
+_ANTHROPIC_CACHE_WARNING_LOG: set[tuple[str, str]] = set()
+
 PROVIDER_SETTING_PATHS = {
     "anthropic": {"api_key": "ANTHROPIC.KEY"},
     "codestral": {"api_key": "CODESTRAL.KEY"},
@@ -258,14 +269,26 @@ def _should_retry_same_model(exc: BaseException) -> bool:
 
     With config.retry_same_model_on_timeout set to false, a timed-out call is handed to the
     fallback-models loop instead of being replayed on the model that just missed the deadline.
+    Request validation errors also surface immediately rather than replaying the same request.
     """
-    if isinstance(exc, openai.RateLimitError):
+    if isinstance(exc, (openai.RateLimitError, openai.BadRequestError, openai.UnprocessableEntityError)):
         return False
     if isinstance(exc, openai.APITimeoutError):
         return _as_bool(get_settings().config.get("retry_same_model_on_timeout", True), default=True)
     return isinstance(exc, openai.APIError)
 
 
+def _log_anthropic_cache_warning(model: str, reason: str) -> None:
+    """Log one warning per process for a prompt-cache config that cannot take effect."""
+    key = (model, reason)
+    if key in _ANTHROPIC_CACHE_WARNING_LOG:
+        return
+    _ANTHROPIC_CACHE_WARNING_LOG.add(key)
+    get_logger().warning(
+        f"cache_control_injection_points may not take effect for {model}: {reason}"
+    )
+
+
 class LiteLLMAIHandler(BaseAiHandler):
     """Handle chat completions across supported providers through LiteLLM.
 
@@ -2406,6 +2429,80 @@ def _resolve_cache_control_injection_points():
             raise ValueError("LITELLM.CACHE_CONTROL_INJECTION_POINTS must be a JSON/TOML array")
         return cache_control_injection_points
 
+    @staticmethod
+    def _warn_prompt_cache_conditions(
+        model: str, system: str, user: str, injection_points, request_provider: str | None = None
+    ) -> None:
+        """Warn once per process when an enabled prompt-cache config cannot take effect.
+
+        LiteLLM skips Anthropic prompt caching silently when the model does not support it or
+        the cached prefix stays below the model's ``prompt_cache_min_tokens``. Both conditions
+        are knowable before the call, so surface them instead of leaving the operator blind.
+        Best effort: a metadata gap or estimate failure skips the check, never fails the call.
+        """
+        if not isinstance(model, str) or not model:
+            return
+        is_claude_named = "claude" in model.lower()
+        is_anthropic_provider = request_provider in _ANTHROPIC_CACHE_REQUEST_PROVIDERS
+        if not is_claude_named and not is_anthropic_provider:
+            # cache_control_injection_points is an Anthropic-only kwarg; a config pointing at
+            # another provider (or a model identifier that cannot resolve as Anthropic) will
+            # never attach, so warn instead of silently dropping it in a debug line.
+            _log_anthropic_cache_warning(
+                model, "the request does not route to an Anthropic Claude model"
+            )
+            return
+        try:
+            supports = litellm.utils.supports_prompt_caching(model)
+        except Exception:
+            return
+        if supports is False and is_claude_named:
+            # Conclusive only for a model identifier we recognize; a provider-aliased model
+            # (e.g. anthropic/my-deployment) may simply be absent from litellm's cost map.
+            _log_anthropic_cache_warning(model, "the model does not support prompt caching")
+            return
+        try:
+            min_tokens = litellm.get_model_info(model).get("prompt_cache_min_tokens")
+        except Exception:
+            return
+        if not isinstance(min_tokens, int) or isinstance(min_tokens, bool) or min_tokens <= 0:
+            return
+        cached_tokens = LiteLLMAIHandler._estimate_cached_prefix_tokens(system, user, injection_points)
+        if 0 < cached_tokens < min_tokens:
+            _log_anthropic_cache_warning(
+                model,
+                f"the cached prefix is below the model's {min_tokens} token minimum",
+            )
+
+    @staticmethod
+    def _estimate_cached_prefix_tokens(system: str, user: str, injection_points) -> int:
+        """Estimate the tokens in the prompt segment the injection points will cache.
+
+        A cache_control breakpoint caches everything from the start of the prompt up to the
+        targeted message, so the estimate counts the targeted messages plus every message
+        before them (system, then user in this handler's call shape). Returns 0 when none of
+        the points targets a supported role or the estimate cannot be produced, which skips
+        the below-minimum check entirely.
+        """
+        targets_user = any(
+            isinstance(point, dict) and point.get("role") == "user" for point in injection_points
+        )
+        if not any(isinstance(point, dict) and point.get("role") in ("system", "user")
+                   for point in injection_points):
+            return 0
+        try:
+            from pr_agent.algo.token_handler import TokenEncoder
+
+            encoder = TokenEncoder.get_token_encoder("anthropic/claude")
+            system_tokens = len(encoder.encode(system or "", disallowed_special=()))
+            user_tokens = len(encoder.encode(user or "", disallowed_special=()))
+        except Exception:
+            system_tokens = len(system or "") // 4
+            user_tokens = len(user or "") // 4
+        cached_tokens = system_tokens + (user_tokens if targets_user else 0)
+        cached_framing = _CACHE_MESSAGE_FRAMING_ALLOWANCE * (2 if targets_user else 1) + _CACHE_REPLY_FRAMING_ALLOWANCE
+        return cached_tokens + cached_framing
+
     async def chat_completion(self, model: str, system: str, user: str, temperature: float = 0.2, img_path: str = None):
         configured_deployment_id = self.deployment_id
         return await self._chat_completion_with_retry(
@@ -2762,16 +2859,18 @@ async def _chat_completion_with_retry(
                 # Anthropic prompt caching via LiteLLM's cache_control_injection_points. The value
                 # is validated before the try/except (see above) so a malformed config surfaces as
                 # a ValueError instead of being retried. The kwarg is Anthropic-specific (Claude via
-                # the Anthropic API, Bedrock or Vertex), so gate on the model to avoid passing an
-                # unsupported param to other providers when litellm.drop_params is off. setdefault
-                # guards against overwriting a value already merged into kwargs.
+                # the Anthropic API, Bedrock or Vertex), so gate the forwarding on the model to
+                # avoid passing an unsupported param to other providers when litellm.drop_params is
+                # off. The pre-call warning runs for every configured model instead, so an operator
+                # who misconfigures an aliased or non-Anthropic model gets a signal rather than a
+                # silently skipped debug line. setdefault guards against overwriting a value already
+                # merged into kwargs.
                 if cache_control_injection_points:
                     if isinstance(model, str) and "claude" in model.lower():
                         kwargs.setdefault("cache_control_injection_points", cache_control_injection_points)
-                    else:
-                        get_logger().debug(
-                            f"cache_control_injection_points configured but not applied: {model} is not an "
-                            "Anthropic (Claude) model")
+                    self._warn_prompt_cache_conditions(
+                        model, system, user, cache_control_injection_points, request_provider=request_provider
+                    )
 
                 # Classic `bedrock/` calls use model_id for Bedrock Runtime inference profiles.
                 # Bedrock Mantle uses Projects, so `bedrock_mantle/` intentionally omits it.

pr_agent/algo/ai_handlers/openai_ai_handler.py

@@ -40,7 +40,9 @@ def deployment_id(self):
         return get_settings().get("OPENAI.DEPLOYMENT_ID", None)
 
     @retry(
-        retry=retry_if_exception_type(openai.APIError) & retry_if_not_exception_type(openai.RateLimitError),
+        retry=retry_if_exception_type(openai.APIError) & retry_if_not_exception_type(
+            (openai.RateLimitError, openai.BadRequestError, openai.UnprocessableEntityError)
+        ),
         stop=stop_after_attempt(OPENAI_RETRIES),
     )
     async def chat_completion(self, model: str, system: str, user: str, temperature: float = 0.2, img_path: str = None):