From be582ce2ed529a9c8f437336e5c3c9d947f4a89b Mon Sep 17 00:00:00 2001 From: mattgodbolt-molty Date: Wed, 29 Jul 2026 18:38:52 -0500 Subject: [PATCH 1/2] Handle safety-classifier refusals with a distinct error and metric Sonnet 5 ships cyber-safety classifiers that can decline a request: HTTP 200 with stop_reason "refusal" and empty (or discarded partial) content. CE users compile arbitrary code, so exploit-adjacent input can plausibly trip this. Previously a refusal fell into the generic empty-response path, indistinguishable from thinking starving max_tokens. Now handled explicitly before text extraction: a clear user-facing message (with a hint that trimming input may help), partial output discarded rather than served, usage populated, and a dedicated ClaudeExplainRefusal metric. Error responses are already not cached, so retries hit the API. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_012jJRqmqhE11VAc3biKbxUY --- CLAUDE.md | 5 +++++ app/explain.py | 32 ++++++++++++++++++++++++++++++++ app/test_explain.py | 45 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index a2acb28..a5a0e5c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -99,6 +99,11 @@ call → response with metrics. See `claude_explain.md` for detailed architectur `redacted_thinking` blocks (encrypted reasoning when safety filters trip); the same filter excludes them correctly, but be aware "no text block" can mean either max_tokens starvation *or* a redacted-thinking-only response — the error message is the same. +- **Safety refusals are handled before the empty-response path.** Claude 5-family classifiers can decline a + request (HTTP 200 with `stop_reason: "refusal"`, empty or partial content) — plausible here since CE users + compile arbitrary, sometimes exploit-adjacent code. `app/explain.py` returns a distinct user-facing message, + discards any partial output, and emits `ClaudeExplainRefusal` so it's separable from token starvation on + dashboards. - **Empty responses are not 500s.** When the model returns no text block, `app/explain.py` returns `ExplainResponse(status="error")` with `usage` populated and emits `ClaudeExplainEmptyResponse`. The cache layer skips storing error responses so retries hit the API. Don't change this to raise — the structured error diff --git a/app/explain.py b/app/explain.py index 0ab9bfc..86d27c4 100644 --- a/app/explain.py +++ b/app/explain.py @@ -162,6 +162,38 @@ async def _call_anthropic_api( output_tokens = message.usage.output_tokens total_tokens = input_tokens + output_tokens + if message.stop_reason == "refusal": + # Claude 5-family safety classifiers can decline a request: a normal + # HTTP 200 whose stop_reason is "refusal", with empty (or discarded + # partial) content. CE users compile arbitrary code, so exploit-adjacent + # input can occasionally trip this. Handle it before the generic + # empty-response path so it gets a clear user-facing message and its + # own metric rather than looking like token starvation on dashboards. + message_text = ( + "Claude declined to explain this code (safety filters). This can occasionally " + "trigger on benign security-related code; trimming the input to the relevant " + "part may help." + ) + LOGGER.warning("Refusal from model (in=%d, out=%d)", input_tokens, output_tokens) + metrics_provider.set_property("language", body.language) + metrics_provider.set_property("compiler", body.compiler) + metrics_provider.set_property("instructionSet", body.instructionSet or "unknown") + metrics_provider.set_property("cached", "false") + metrics_provider.put_metric("ClaudeExplainRequest", 1) + metrics_provider.put_metric("ClaudeExplainRefusal", 1) + metrics_provider.put_metric("ClaudeExplainInputTokens", input_tokens) + metrics_provider.put_metric("ClaudeExplainOutputTokens", output_tokens) + return ExplainResponse( + status="error", + message=message_text, + model=prompt_data["model"], + usage=TokenUsage( + inputTokens=input_tokens, + outputTokens=output_tokens, + totalTokens=total_tokens, + ), + ) + # Pick the last text block — when thinking is enabled the response # contains thinking blocks before the final text block. text_blocks = [c for c in message.content if getattr(c, "type", None) == "text"] diff --git a/app/test_explain.py b/app/test_explain.py index 2ed15ee..941a7c6 100644 --- a/app/test_explain.py +++ b/app/test_explain.py @@ -264,6 +264,51 @@ async def test_returns_error_when_no_text_block(self, sample_request, noop_metri assert response.usage.inputTokens == 100 assert response.usage.outputTokens == 50 + @pytest.mark.asyncio + async def test_refusal_returns_clear_error(self, sample_request, noop_metrics): + """A safety-classifier refusal (HTTP 200, stop_reason='refusal', empty + content) must return a user-facing message distinct from the generic + empty-response error, with usage populated.""" + mock_message = MagicMock() + mock_message.content = [] + mock_message.usage = MagicMock(input_tokens=80, output_tokens=0) + mock_message.stop_reason = "refusal" + + mock_client = MagicMock() + mock_client.messages.create = AsyncMock(return_value=mock_message) + + test_prompt = Prompt(Path("app/prompt.yaml")) + response = await process_request(sample_request, mock_client, test_prompt, noop_metrics) + + assert response.status == "error" + assert response.explanation is None + assert "declined" in response.message + assert "no text content" not in response.message + assert response.usage is not None + assert response.usage.inputTokens == 80 + + @pytest.mark.asyncio + async def test_refusal_discards_partial_output(self, sample_request, noop_metrics): + """A mid-stream refusal can carry partial text; it must be discarded + rather than served as if it were a complete explanation.""" + partial = MagicMock() + partial.type = "text" + partial.text = "This function starts by..." + + mock_message = MagicMock() + mock_message.content = [partial] + mock_message.usage = MagicMock(input_tokens=80, output_tokens=40) + mock_message.stop_reason = "refusal" + + mock_client = MagicMock() + mock_client.messages.create = AsyncMock(return_value=mock_message) + + test_prompt = Prompt(Path("app/prompt.yaml")) + response = await process_request(sample_request, mock_client, test_prompt, noop_metrics) + + assert response.status == "error" + assert response.explanation is None + @pytest.mark.asyncio async def test_returns_error_when_call_exceeds_deadline(self, sample_request, noop_metrics): """A Claude call that overruns the wall-clock budget must return a From 3c4f5481e8ecad480d7b53666e1a841a7afeaf2f Mon Sep 17 00:00:00 2001 From: mattgodbolt-molty Date: Wed, 29 Jul 2026 18:53:46 -0500 Subject: [PATCH 2/2] Refresh CLAUDE.md: record prompt-caching decision, condense stale content - New gotcha recording the 2026-07 prompt-caching evaluation: rejected at current traffic (~104 fresh calls/day vs a 5-minute TTL and a prefix fragmented by language/arch/audience/type; ~$0.40/fortnight potential saving of ~$22 spend). Includes the revisit threshold and how to rerun the analysis, so this doesn't get re-litigated from scratch. - Condense the whole document: fold Project Structure and workflow notes into Overview and Development Commands, drop historical narration (1536-token era, per-model archaeology), dedupe the thinking gotchas, and add the build_api_payload single-source-of-truth rule. 137 -> 103 lines with no guidance lost. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_012jJRqmqhE11VAc3biKbxUY --- CLAUDE.md | 175 +++++++++++++++++++++--------------------------------- 1 file changed, 67 insertions(+), 108 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index a5a0e5c..752a180 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -4,134 +4,93 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co ## Overview -This is a FastAPI-based service that provides AI-powered explanations of compiler assembly output for the Compiler -Explorer website. The service uses Anthropic's Claude API to analyze source code and its compiled assembly, providing -educational explanations of compiler transformations and optimizations. +A FastAPI service that provides AI-powered explanations of compiler assembly output for the Compiler Explorer +website, using Anthropic's Claude API. Runs locally for development or as an AWS Lambda function (Mangum adapter) +behind an API Gateway HTTP API. The production explainer model is Sonnet 5 (see `app/prompt.yaml`); the +prompt-testing framework's correctness reviewer is Opus 5. -There's a prompt testing framework that allows for us to explore and improve the prompts used to generate explanations. -This framework is designed to be extensible and allows for easy addition of new tests and prompt variations. +Request pipeline: input validation, then smart assembly filtering plus hard character caps, then the Claude API +call, then a response with usage/cost metrics. See `claude_explain.md` for detailed architecture documentation. -## Project Structure - -This is a FastAPI-based service that can run locally for development or as an AWS Lambda function via Mangum adapter. -See the source code for current project structure. +There is a prompt-testing framework (`prompt_testing/`, CLI `prompt-test`) for evaluating prompt and model +changes against curated test cases, with an Opus-based correctness review. Use it before changing +`app/prompt.yaml`: run the suite with `--review` before and after, and compare accuracy, latency, and tokens. ## Development Commands -### Setup - ```bash -# Set up environment with .env file containing: -# ANTHROPIC_API_KEY= - -# Install dependencies +# Setup: .env file containing ANTHROPIC_API_KEY=, then: uv sync --group dev -``` - -### Running Locally -```bash -# Start development server +# Run locally uv run fastapi dev +./test-explain.sh # exercise the local server (--pretty for readable output) -# Test the service -./test-explain.sh -# Or with pretty output: -./test-explain.sh --pretty -``` - -### Testing - -```bash -# Run tests -uv run pytest +# Tests and linting +uv run pytest # run all tests (matches CI) +uv run pytest app/test_explain.py::TestProcessRequest::test_process_request_success # one test +uv run pre-commit run --all-files # ruff lint/format, shellcheck etc (matches CI) -# Run specific test -uv run pytest app/explain_test.py::test_process_request_success +# Prompt evaluation +uv run prompt-test run --prompt current --review # full suite + Opus correctness review +uv run prompt-test list # available test cases ``` -### Code Quality - -```bash -# Run pre-commit hooks (ruff linting/formatting, shellcheck) -uv run pre-commit run --all-files - -# Manual linting -uv run ruff check -uv run ruff format -``` - -## Key Architecture Details - -The service processes compiler output through a pipeline: input validation → smart assembly filtering → Claude API -call → response with metrics. See `claude_explain.md` for detailed architecture documentation. +**Always run `uv run pytest` and `uv run pre-commit run --all-files` before pushing.** CI runs exactly these. +Pre-commit hooks may modify files (e.g. ruff format); re-`git add` if a hook reports fixes. ## Anthropic API gotchas -- **`max_tokens` includes thinking tokens.** When a prompt YAML sets `model.thinking: {type: adaptive}` (or - `{type: enabled, budget_tokens: N}`), thinking counts against `max_tokens`. The old production value `1536` - silently starved the visible text output on complex cases when thinking was on (production is now `4096`). `Prompt.__init__` now refuses to load a - thinking-enabled config with `max_tokens < 4096`; ≥4096 (8192 worked in past experiments) is the floor. -- **Neither production model accepts `temperature`.** Opus 5 (reviewer) and Sonnet 5 (explainer) both reject - non-default sampling parameters with a 400, so `prompt_testing/reviewer.py` omits it and `app/prompt.yaml` sets - none. Only pre-5 Sonnet models accept `temperature`; restore it in the YAML if you ever pin one of those. -- **Sonnet 5 runs adaptive thinking by default when `thinking` is omitted** (unlike 4.6, where omitted meant off). - `app/prompt.yaml` therefore sets `thinking: {type: disabled}` explicitly; dropping that line silently turns - thinking on and eats the `max_tokens` budget. Sonnet 5 also uses a new tokenizer (~30% more tokens for the same - text than 4.6) — don't reuse token counts or cost baselines measured on 4.6. -- **`model.effort` is plumbed but a no-op with thinking disabled.** The 2026-07 sweep (low/medium/high, 21 cases) - showed identical latency and cost across levels with `thinking: disabled` — effort mostly modulates thinking - depth, so there's nothing to modulate. Production leaves it unset (API default `high`). It becomes meaningful +- **`max_tokens` includes thinking tokens.** When thinking is enabled it counts against `max_tokens`, and can + starve the visible text on complex cases. `Prompt.__init__` refuses to load a thinking-enabled config with + `max_tokens < 4096` (production uses 4096). +- **Neither production model accepts `temperature`.** Opus 5 (reviewer) and Sonnet 5 (explainer) reject + non-default sampling parameters with a 400, so neither sets one. Only pre-5 Sonnet models accept + `temperature`; restore it in the YAML if you ever pin one of those. +- **Sonnet 5 runs adaptive thinking by default when `thinking` is omitted.** `app/prompt.yaml` therefore sets + `thinking: {type: disabled}` explicitly; dropping that line silently turns thinking on and eats the + `max_tokens` budget. The same trap applies to the reviewer, which always sends an explicit thinking config so + `--reviewer-thinking off` really means off. +- **Sonnet 5's tokenizer produces ~30% more tokens than 4.6 for the same text.** Don't reuse token counts or + cost baselines measured on 4.6-era models. +- **`model.effort` is plumbed but a no-op with thinking disabled.** The 2026-07 sweep (low/medium/high, 21 + cases) showed identical latency and cost across levels with thinking off: effort mostly modulates thinking + depth, so there is nothing to modulate. Production leaves it unset (API default `high`). It becomes meaningful on the `useThinking` path or if adaptive thinking is ever made the default. -- **Reviewer thinking is on by default.** `prompt-test run --review` and `prompt-test review` default to - `--reviewer-thinking adaptive` / `--thinking adaptive`. It catches factual errors the no-think reviewer misses - but adds ~70% to review cost. Pass `off` to compare runs or save money on large batches. -- **Production explainer thinking is opt-in per request.** On Sonnet 5 the 2026-07 eval showed adaptive thinking - bought no accuracy on our test set (15/21 vs 17/21 reviewer-correct) while adding output tokens; thinking-off is - the default. Latency risk is the enduring reason: thinking can push large queries past the **30s Lambda + API - Gateway v2 timeout** (no raising that — HTTP API has a 30s ceiling). Callers opt in by sending - `useThinking: true` on the request; the default (no field, or `false`) preserves current latency. Cache keys split on the flag, so on/off requests - cache independently. If we ever want default-on, we need either a smaller fixed thinking budget *or* an async - response architecture (Lambda Function URL with response streaming, SQS poll, etc.). -- **Multi-block responses.** When thinking is enabled the API returns thinking blocks before the text block. - `app/explain.py` and `prompt_testing/runner.py` both pick the last text block via `getattr(c, "type", None) == - "text"`. Preserve that pattern for any new code that consumes responses. The API may also return - `redacted_thinking` blocks (encrypted reasoning when safety filters trip); the same filter excludes them - correctly, but be aware "no text block" can mean either max_tokens starvation *or* a redacted-thinking-only - response — the error message is the same. +- **Production explainer thinking is opt-in per request** (`useThinking: true`; default off). The 2026-07 eval + showed adaptive thinking bought no accuracy on our test set while adding output tokens, and thinking can push + large queries past the **30s Lambda + API Gateway v2 timeout** (a hard ceiling; not raisable on HTTP APIs). + Cache keys split on the flag. Default-on would need a smaller thinking budget or an async response + architecture (Lambda response streaming, SQS poll, etc.). +- **Reviewer thinking is on by default** (`--reviewer-thinking adaptive`). It catches factual errors the + no-think reviewer misses but adds ~70% to review cost; pass `off` for cheap comparative runs. +- **Prompt caching: evaluated 2026-07 and rejected at current traffic.** ~104 fresh Claude calls/day + (CloudWatch, 14-day window), only ~35 hours/fortnight above 12 calls/hour, against a 5-minute cache TTL and a + prefix fragmented by language/arch/audience/type. Generous math: ~$0.40 saved per fortnight of ~$22 spend, + before counting the restructuring needed to clear Sonnet 5's 1024-token minimum cacheable prefix (the system + prompt is only ~620 tokens; the per-audience guidance lives in the user prompt). Revisit if traffic grows + ~50x, or if sustained >3 same-combo requests/hour makes the 1-hour TTL viable. Rerun the analysis with + `aws cloudwatch get-metric-statistics` on `CompilerExplorer/ClaudeExplainFreshResponse`. - **Safety refusals are handled before the empty-response path.** Claude 5-family classifiers can decline a - request (HTTP 200 with `stop_reason: "refusal"`, empty or partial content) — plausible here since CE users + request (HTTP 200 with `stop_reason: "refusal"`, empty or partial content), plausible here since CE users compile arbitrary, sometimes exploit-adjacent code. `app/explain.py` returns a distinct user-facing message, - discards any partial output, and emits `ClaudeExplainRefusal` so it's separable from token starvation on - dashboards. + discards any partial output, and emits `ClaudeExplainRefusal`. +- **Multi-block responses.** With thinking enabled the API returns thinking blocks before the text block; both + `app/explain.py` and `prompt_testing/runner.py` pick the last text block via + `getattr(c, "type", None) == "text"`. Preserve that pattern in new response-consuming code. "No text block" + can mean max_tokens starvation or a redacted-thinking-only response; the error message is the same. - **Empty responses are not 500s.** When the model returns no text block, `app/explain.py` returns - `ExplainResponse(status="error")` with `usage` populated and emits `ClaudeExplainEmptyResponse`. The cache - layer skips storing error responses so retries hit the API. Don't change this to raise — the structured error - is what the CE frontend can render. + `ExplainResponse(status="error")` with usage populated and emits `ClaudeExplainEmptyResponse`. The cache layer + skips storing error responses so retries hit the API. Don't change this to raise; the structured error is what + the CE frontend can render. +- **`build_api_payload` is the single source of truth for API kwargs.** Production (`app/explain.py`) and the + prompt-test runner both call it; don't reconstruct thinking/temperature/output_config logic elsewhere. ## Code Style Guidelines -- Prefer using modern Python 3.13+ type syntax. Good: `a: list[str] | None`. Bad: `a: Optional[List[str]]` -- Use ruff for linting and formatting with line length of 120 characters -- Prefer pathlib.Path over old-fashioned io like naked `open` and `glob` calls. Always supply an encoding -- Always import at the top of the file, don't litter imports throughout the file -- Strive for simplicity and clarity in code. Avoid unnecessary complexity. -- Don't assume backwards compatibility is required unless explicitly stated. Ask if unsure. - -## Development Workflow Notes - -### Before Pushing Code -**ALWAYS run the full test suite before pushing any code changes:** - -```bash -# Required before every push -uv run pytest # Run all tests (matches CI) -uv run pre-commit run --all-files # Run all linting/formatting -``` - -This prevents CI failures and ensures code quality. The CI runs exactly these commands, so running them locally will catch any issues. - -### General Notes -- The pre-commit hooks may modify the code and so: always run them before `git add`, and if a commit hook fails then - it's probably you'll need to `git add` again if it indicated it fixed issues (e.g. `ruff`) -- Ruff is configured for Python 3.13+ with 120 character line length +- Modern Python 3.13+ type syntax: `a: list[str] | None`, not `Optional[List[str]]` +- ruff for linting and formatting, line length 120 +- Prefer `pathlib.Path` over naked `open`/`glob`; always supply an encoding +- Import at the top of the file only +- Strive for simplicity and clarity; avoid unnecessary complexity +- Don't assume backwards compatibility is required unless explicitly stated; ask if unsure