test: regression suite for known-bad API request patterns from prior COEs

[scottwofford]

Summary

COE audit deliverable covering two Trello cards:

• Write regression tests for all 5 known-bad API request patterns from prior COEs
• Verify that the new architecture handles all 5 known request sanitization patterns from prior COEs

Adds tests/luthien_proxy/unit_tests/test_known_bad_request_patterns.py (19 tests) with each known-bad pattern as a verbatim fixture, plus a changelog fragment. No production code changes.

Audit context

None of the LiteLLM-era sanitizer fixes (PRs #201, #167, #178, #151) survive on current main (verified: those commits are not ancestors of origin/main). That is not a regression to restore: the current direct-SDK architecture is deliberately transparency-first (_prepare_request_kwargs forwards unknown fields via extra_body; the pipeline forwards the client's anthropic-beta header per #269). The contract is "behavior through the proxy == behavior of a direct connection."

To ground the verdicts, each pattern was probed against the live Anthropic API on 2026-07-06 (claude-haiku-4-5), and the current-day Claude Code request shape was checked against a real recorded request in the local proxy's event log. Two historical patterns flipped: the API now accepts whitespace-only text blocks, and context_management is now a real API feature (context editing) that Claude Code sends on every request — the old PR #151 "strip it" fix would today silently disable context editing.

Per-pattern verdicts

#	Pattern (origin)	Live API 2026-07-06	Current architecture verdict	Test
1	Empty text content blocks (PR #201)	Still rejected (400 "text content blocks must be non-empty")	Handled by design: forwarded verbatim, upstream 400 relayed as clean invalid_request_error (never a proxy 500). Same outcome as a direct connection; current Claude Code no longer emits these.	test_empty_text_block_survives_forwarding, test_pipeline_relays_400_end_to_end[empty-text-block]
1b	Whitespace-only text blocks (PR #201 ext.)	Now accepted	N/A: constraint removed upstream. Old sanitizer would rewrite valid requests.	test_messages_and_tools_forwarded_unmodified[whitespace-text-block]
2	Orphaned tool_results after /compact (PR #167)	Still rejected (400 "unexpected tool_use_id")	Handled by design: forwarded verbatim, upstream 400 relayed cleanly. The /compact bug was client-side and is fixed in current Claude Code.	test_orphaned_tool_result_survives_forwarding, test_pipeline_relays_400_end_to_end[orphaned-tool-result]
3	cache_control extra fields (PR #178)	scope still rejected without matching beta header	Handled by design: forwarded verbatim + client anthropic-beta header forwarded (#269), so beta-gated cache features work when the client opts in. Current Claude Code no longer sends scope on tools (verified against recorded request).	test_cache_control_scope_survives_forwarding, test_pipeline_relays_400_end_to_end[cache-control-extra-field]
4	context_management stripping (PR #151)	Old truncation shape rejected; current edits shape accepted	N/A, inverted: stripping would now be a bug. Claude Code sends context_management (edits shape) on every request; the proxy must forward it via extra_body, and does.	test_context_management_forwarded_via_extra_body
5	Duplicate tools in request	Still rejected (400 "Tool names must be unique.")	Handled by design: forwarded verbatim, upstream 400 relayed cleanly.	test_messages_and_tools_forwarded_unmodified[duplicate-tools], test_pipeline_relays_400_end_to_end[duplicate-tools]
6	Parallel tool_use ordering (PR #356, card mWjeUBG1's 5th item)	n/a (proxy-side streaming bug)	Fixed on main and architecturally covered: SimpleLLMPolicy applies judge decisions at message_delta, and validate_anthropic_event_ordering runs on every outbound stream (advisory + streaming.protocol_violation event). Existing coverage: test_anthropic_stream_validator.py, e2e_tests/test_mock_simple_llm_parallel_tools.py.	Tripwire added: test_content_block_after_message_delta_is_flagged

Note: the two cards disagree on the 5th pattern (card mWjeUBG1 lists parallel tool_use ordering #356; card qGTbhaTa lists duplicate tools). This PR covers the union — 6 patterns.

On card mWjeUBG1's original definition of done ("no 400 returned to the client")

That definition was written for the old sanitize-and-repair architecture. Under the current transparency-first design, faithfully relaying the upstream 400 is the correct handling: the clients that generated these malformed requests have since been fixed upstream, a direct connection would 400 identically, and silent request rewriting is what the refactor deliberately removed. The tests therefore pin (a) verbatim forwarding and (b) clean 400 relay (never a proxy-side 500), rather than suppression of the 400. If we later decide the proxy should again repair malformed requests, these tests are the deliberate decision point — they will fail loudly and must be updated intentionally, not silently.

Test plan

• 19 new unit tests pass (uv run pytest tests/luthien_proxy/unit_tests/test_known_bad_request_patterns.py)
• ./scripts/dev_checks.sh clean (format, lint, pyright, full unit suite, complexity)
• Fixtures verified byte-for-byte against live API rejections (2026-07-06) and a recorded production Claude Code request

🤖 Generated with Claude Code

[claude]

Code review — tests-only PR, well-crafted

Read through the diff end-to-end and spot-checked the source that the tests reference (AnthropicClient._prepare_request_kwargs, _handle_anthropic_error, validate_anthropic_event_ordering, process_anthropic_request). Nothing to block on — approve. A few small observations below.

What works well

• Module docstring is the strongest part of the PR: it names the architectural shift (transparency-first, extra_body + anthropic-beta forwarding), the two behavioral inversions (whitespace text blocks now accepted, context_management now a real feature), and calls out that this is a decision point, not suppression of a 400. Future readers will not have to spelunk PRs fix: strip context_management param from Anthropic requests #151/fix: prune orphaned tool_results after /compact #167/fix: sanitize cache_control on tools before sending to Anthropic API #178/fix: sanitize empty text content blocks in Anthropic requests #201 to understand why the tests exist.
• Per-pattern verdict table in the PR body is grounded in live API probes on 2026-07-06 rather than assumptions — this is the right way to audit "do prior sanitizers still apply?"
• Fixtures are verbatim from origin bug reports with PR references, and the test_context_management_forwarded_via_extra_body assertion pins exactly the right invariant (extra_body["context_management"] == …, not just "present somewhere").
• Two-layer coverage per pattern (unit-level _prepare_request_kwargs + pipeline-level end-to-end 400 relay via process_anthropic_request) gives both a targeted failure signal and a real integration guarantee.
• _make_400 correctly constructs an AnthropicStatusError with a realistic body — the classifier test would fail loudly if _ANTHROPIC_STATUS_ERROR_TYPE_MAP ever mis-mapped 400.

Nits (non-blocking)

1. Module-level mutable fixtures shared across parametrized tests. EMPTY_TEXT_BLOCK_REQUEST et al. are module-level dicts. test_messages_and_tools_forwarded_unmodified passes them by reference to _prepare_request_kwargs. Today _prepare_request_kwargs doesn't mutate its input (verified — it only reads keys, and extra_body is a fresh dict), so the assert request_fixture == original tripwire holds. But if a future refactor introduced in-place mutation, later parametrize cases in the same run would start from mutated state and the failure could be order-dependent. test_pipeline_relays_400_end_to_end already does copy.deepcopy(request_fixture) — extending that to the unit-level test (or moving fixtures into pytest fixture functions) would make the tests order-independent by construction.

2. Testing a private method. _prepare_request_kwargs is underscore-prefixed and the unit-testing guidelines say to test through public APIs. Defensible here — this method IS the point-of-decision for the transparency contract, and the pipeline test at line 287 gives the public-API cross-check — but worth noting that if the method ever gets renamed or inlined, these tests will break for a mechanical reason rather than a behavioral one. Consider a short comment on Contract 1 acknowledging "we test the private method deliberately, at the smallest surface that pins the contract."

3. context_management doesn't get a pipeline-level tripwire. Given the PR body emphasizes PR fix: strip context_management param from Anthropic requests #151's inversion as one of the two most important findings, and the pipeline path can theoretically rewrite the outgoing body, an e2e-style test that asserts the forwarded backend body still contains context_management (mirroring the empty-text-block pipeline test at line 287) would strengthen the tripwire. The unit-level test_context_management_forwarded_via_extra_body covers it at the client layer, so this is genuinely optional.

4. test_empty_text_block_survives_forwarding uses in on a list of dicts. That's fine — it works — but a targeted assert assistant_blocks[0] == {\"type\": \"text\", \"text\": \"\"} would fail with a clearer diff when the block list shape changes for an unrelated reason.

Coverage / correctness

• All six patterns from both Trello cards (mWjeUBG1 and qGTbhaTa) are covered, with the disagreement between the two cards on "the 5th pattern" resolved by covering the union — good call, and worth the explicit note in the PR body.
• _LIVE_400_MESSAGES uses sorted() in the parametrize call, which gives deterministic test IDs across Python versions.
• The mock_client.complete.call_args.args[0] assertion in test_pipeline_relays_400_end_to_end correctly matches the real call site at anthropic_processor.py:192, which invokes self._anthropic_client.complete(final_request, extra_headers=...) — request is positional, so .args[0] is safe.

Nice write-up in the PR body — treating the "definition of done" reinterpretation (line: "faithfully relaying the upstream 400 is the correct handling") as a first-class part of the audit deliverable is the right call.

[scottwofford]

Claude-generated merge-queue triage of all open Luthien PRs, requested by Scott (Jul 7, 2026). Advisory only; Scott has not yet acted on these recommendations.

Recommendation: merge.

Tests plus changelog only, no production code. Beyond regression coverage, the module docstring pins the transparency contract (forward verbatim, relay upstream 400s cleanly), which is the architectural stance that closed #204. Useful to have in the suite before deciding #797 and #799, both of which get judged against exactly this contract. Caveat inherited from the PR body: the two "pattern flipped upstream" verdicts came from a live-API probe on Jul 6 and would need re-probing to re-confirm.