@claude review. Focus on critical issues: bugs, security vulnerabilities, logical errors, data loss risks, performance problems. Do NOT nitpick style, naming conventions, minor formatting, or subjective preferences — only flag issues that could break functionality or cause real harm in production.

Claude encountered an error after 3m 27s —— View job

Reviewing PR #457...

• Read the diff against origin/main
• Review the retry-loop fix in adapter.py
• Review conftest.py scrubbing changes
• Review test_read_cassettes.py changes
• Check cassettes for leaked secrets/PII
• Post review

@claude review. The previous review run errored out before posting (empty ANTHROPIC_API_KEY + an internal action error), so this is a re-request. Focus on critical issues: bugs, security vulnerabilities, logical errors, data loss risks, performance problems. Do NOT nitpick style, naming, or formatting.

Claude finished @axisrow's task in 2m 8s —— View job

Review: PR #457 — v4 Live read cassettes + bounded retry-loop

• Read the diff against origin/main
• Review the retry-loop fix in adapter.py
• Review conftest.py scrubbing changes
• Review test_read_cassettes.py changes
• Check cassettes for leaked secrets/PII
• Post review

Verdict: LGTM. No bugs, security issues, or data-loss risks found. The diagnosis (unbounded retry, not a vcrpy×v4 incompatibility) is correct and the fix is minimal and well-targeted.

Correctness — retry fix (adapter.py:178-187)

The change bounds the code in (54, 55) branch with repeat_number < retries_if_server_error, identical to the existing server-error branch immediately below it. Since that branch is pre-existing and proven to terminate, the new branch inherits the same termination guarantee — no off-by-one or new infinite-loop risk. The explicit return False after the budget is exhausted correctly falls through to error_handling, which raises V4LiveRequestsLimitError. Good.

Security / PII — cassettes ✅

Verified all 7 cassettes plus the conftest scrubbers:

• OAuth token in the v4 JSON body → "token":"<REDACTED>" (_before_record_request, conftest.py:134-142).
• Account login echoed in the GetClientsUnits Logins param → "param":["REDACTED"] (conftest.py:145-153) — this is the new body-scrub path, and it works.
• authorization / client-login headers → REDACTED; Set-Cookie → REDACTED; X-Accel-Info reqid zeroed; response "Login" field → REDACTED.
• Grep for token|Bearer minus REDACTED across the cassette dir returns nothing. All clean.

The _REAL_LOGIN or _REDACTED trick at test_read_cassettes.py:200 is sound: match_on excludes headers and before_record_request scrubs the login on both record and replay sides, so the body matches whether or not a replaying contributor has an env login set.

Non-blocking note (no change required)

Code 54 is overloaded by v4 for both rate-limit and "Not enough rights". With this fix, a permanent permission error now burns 5 × sleep(10) ≈ 50s before failing (exit 1). That's strictly better than the previous infinite hang, and v4 gives no way to disambiguate the two meanings at the protocol level, so bounded retry is the pragmatic call. Callers who want to skip it entirely can pass retry_if_exceeded_limit=False. Worth a mention only so the ~50s tail isn't surprising in production logs.

Follow-up (already flagged by author)

adapter.py is vendored — the fix must also land in axisrow/tapi-yandex-direct or it'll be reverted on the next update_vendor.sh. The PR body already tracks this.

Note: I couldn't execute pytest in this run (command needs approval), so I relied on static review + the author's reported 39 passed offline. The cassettes and matching logic check out by inspection.