Engineering Expert

Two things, in priority order:

1. Tests, please. This endpoint touches payments and there's no test coverage for the happy path, let alone the error cases. Before this merges, I'd want to see at minimum:

• one happy-path integration test that runs the full flow with a test card
• one test for the "insufficient funds" branch — this is the second-most-common real-world response from our processor and we don't handle it explicitly
• one test for the idempotency-key behaviour (replay the same request, ensure we return the cached response, not a new charge)

If you want, I can pair on the integration test setup tomorrow morning — the test harness has gotten weird since we switched processors and it's faster with two heads.

2. The error-handling pattern in handle_charge. Right now you're returning Result<Response, anyhow::Error> and letting our generic 500-handler convert it. That hides which errors are user errors (4xx) vs. our errors (5xx). Our convention (see handle_refund for the pattern) is to return Result<Response, ApiError> where ApiError carries the status code explicitly. This matters here more than other places because the difference between a 402 (card declined) and a 500 (our bug) changes how the client retries.

Minor stuff (variable names, doc comments) I'll leave as inline suggestions in the diff.