Skip to content

fix(vendor): recover from tapi_yandex_direct 2026.5.29 bump regressions - #488

Merged
axisrow merged 2 commits into
mainfrom
fix/vendor-timeout-stub
May 30, 2026
Merged

fix(vendor): recover from tapi_yandex_direct 2026.5.29 bump regressions#488
axisrow merged 2 commits into
mainfrom
fix/vendor-timeout-stub

Conversation

@axisrow

@axisrow axisrow commented May 30, 2026

Copy link
Copy Markdown
Owner

Что и зачем

Vendor bump 1ee8c2a (обновление tapi-yandex-direct до 2026.5.29 через update_vendor.sh = rm -rf + cp -R форка) затёр локальные правки и рассинхронизировал вендор с тестами. CI показал только первую регрессию — job Quality падает на mypy ещё до шага с тестами.

Четыре регрессии и фиксы

# Регрессия Фикс
1 .pyi-стаб потерял timeout у get/post → mypy [call-arg] на auth.py:631 (runtime ОК — tapi2 форвардит **kwargs в requests) восстановлен стаб + self-heal _patch_stub_text в patch_vendor_imports.py + 2 тест-гейта
2 to_columns IndexError на коротких строках отчёта (апстрим откатил guard) восстановлен guard + self-heal _patch_runtime_text
3 transport-хосты стали шаблоном {tld} (v5→.com, v4→.ru) test_transport_contract.py переписан под новый контракт
4 кассеты .ru ≠ запросы .com → 42 теста падали при --record-mode=none TLD-insensitive VCR host-matcher в conftest.py (pytest_recording_configure)

Кореньupdate_vendor.sh затирает локальные правки при каждом bump. Поэтому #1 и #2 не просто восстановлены, а самовосстанавливаются после cp -R (идемпотентно), чтобы будущий bump не уронил CI снова.

Проверка

  • mypy . (Python 3.11) → Success: no issues found in 60 source files
  • pytest -m "not integration"2035 passed, 46 skipped, 0 failed
  • ruff check .All checks passed!
  • кассеточные тесты → 47 passed / 15 skipped (как до bump'а)
  • self-heal на tmp-копии: откат обеих правок → восстановлены (2 файла) → повтор no-op → байт-в-байт совпадает с репо

Остаётся опционально (вне репо)

Внести timeout в .pyi форка axisrow/tapi-yandex-direct как страховку от ребейза апстрима — но self-heal делает это необязательным.

Closes #487

🤖 Generated with Claude Code

The 2026.5.29 vendor bump (1ee8c2a) rebuilt direct_cli/_vendor via
update_vendor.sh's rm -rf + cp -R of the fork, wiping local patches and
desyncing the vendor from the tests. That introduced four regressions; CI
surfaced only the first because Quality fails on mypy before the test step:

1. timeout stub (mypy): the .pyi for YandexDirectClientExecutor.get/post
   dropped the `timeout` kwarg auth.py:631 passes, so mypy rejected the call
   even though tapi2 forwards it to requests at runtime.
2. to_columns IndexError: upstream reverted the short-row guard; report rows
   with fewer cells than the header crashed.
3. transport hosts: upstream turned DIRECT_API_PRODUCTION_ROOT into a {tld}
   template (v5 -> .com, v4 -> .ru); test_transport_contract compared against
   the un-substituted constant.
4. VCR cassettes: the v5 host moved .ru -> .com but all cassettes are .ru, so
   the host matcher failed and 42 replay tests broke under --record-mode=none.

Fixes:
- Re-apply #1 and #2 and make them self-healing in patch_vendor_imports.py
  (_patch_stub_text for the .pyi, _patch_runtime_text for to_columns) so the
  next vendor bump cannot silently drop them again; both are idempotent.
- Add two gates in test_vendor_imports.py (ast check on the shipped stub +
  idempotency of _patch_stub_text).
- Update test_transport_contract.py to the {tld} contract.
- Register a TLD-insensitive VCR host matcher in conftest.py so .ru cassettes
  replay against .com requests and vice versa.

Verified: mypy clean, ruff clean, pytest -m "not integration" all green
(2035 passed), self-heal restores both patches on a stripped tmp copy and is a
no-op on re-run.

Closes #487

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@axisrow

axisrow commented May 30, 2026

Copy link
Copy Markdown
Owner Author

@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

claude Bot commented May 30, 2026

Copy link
Copy Markdown

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


Review: vendor bump regression recovery

I reviewed the diff against origin/main, focusing on bugs, logic errors, and data-loss/production risks (skipping style per your request).

Verdict: no blocking issues found. The four fixes are correct and the self-heal logic is genuinely idempotent. Details below.

What I verified ✅

  • to_columns short-row guard (tapi_yandex_direct.py:452) — values[i] if i < len(values) else "" correctly pads trailing-omitted cells instead of raising IndexError. This pads, it does not drop data — safe.
  • timeout stub (.pyi:39,49) — matches the runtime, which forwards **kwargs to requests. Resolves the mypy [call-arg] on the credential hot path. Scope is correctly limited to YandexDirectClientExecutor (the only type auth.py calls); the report executor and v4 adapter are untouched.
  • Self-heal idempotency — I traced _patch_stub_text through both partial-state cases (sig patched but doc missing / doc present but sig missing) and both self-heal correctly; the lines[i+1] dedup against _TIMEOUT_DOC makes the second pass a byte-for-byte no-op, as the test asserts.
  • Transport contractendpoints.py confirms {tld} templates with v5→com (default) and v4→ru (v4/adapter.py:38). The rewritten test_transport_contract.py matches reality.
  • VCR host matcher_host_tld_insensitive raises AssertionError on mismatch (correct vcrpy matcher protocol) and falls back to verbatim comparison for non-Direct hosts, so strictness is preserved everywhere except the intended .ru/.com Direct hosts.

Non-blocking observations

1. Latent fragility: class-scope prefix match (patch_vendor_imports.py:198)

in_stub_class = line.startswith(_STUB_CLASS) with _STUB_CLASS = "class YandexDirectClientExecutor" also matches class YandexDirectClientExecutorResponse (stub line 28), since startswith doesn't respect the name boundary. It's harmless today — that Response class body is only __call__ (no get/post, no :param data:), so nothing matches, and it sits directly before the real class anyway. But if upstream ever reorders classes so a prefix-sharing class precedes an unrelated method with that exact signature, it could be wrongly patched. A boundary-aware check would remove the risk:

_STUB_CLASS_RE = re.compile(r"^class YandexDirectClientExecutor[:\(]")
...
if line.startswith("class "):
    in_stub_class = bool(_STUB_CLASS_RE.match(line))

Fix this →

2. FYI: v5 live host changed .ru.com

This isn't introduced by the fix (it came from the upstream bump), but the practical effect is that production v5 requests now go to api.direct.yandex.com instead of .ru. Both are valid Yandex entrypoints per endpoints.py, and the TLD-insensitive matcher is exactly why cassettes still replay. Just worth confirming this host switch is intended and validated against live prod, since cassette replay can't catch a host that's silently rejected in production.

3. Indentation coupling (minor)_TO_COLUMNS_UPSTREAM/_TIMEOUT_DOC are matched/inserted with hard-coded indentation. If upstream reflows whitespace, the self-heal silently no-ops — but the two AST/behavioral test gates (test_vendored_executor_stub_declares_timeout_kwarg, the to_columns test) will catch that loudly in CI, which is the right safety net.

Overall this is a careful, well-tested recovery. The self-heal approach is the right call given update_vendor.sh does rm -rf + cp -R.
· fix/vendor-timeout-stub

Review follow-up (#488): _patch_stub_text scoped the stub class with
line.startswith("class YandexDirectClientExecutor"), which also matches the
prefix-sharing class YandexDirectClientExecutorResponse. Harmless today (that
Response body has no get/post or :param data:), but a latent fragility if
upstream ever reorders classes. Replace the prefix check with a boundary-aware
regex ^class YandexDirectClientExecutor[:\(] so only the exact class is treated
as in-scope, and add a gate test feeding a prefix-sharing decoy class with the
exact executor signature (would fail on the old startswith check).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@axisrow

axisrow commented May 30, 2026

Copy link
Copy Markdown
Owner Author

Спасибо за внимательный разбор. По трём наблюдениям:

1. Class-scope prefix match — исправлено (commit 4145887). Подтвердил по коду: startswith("class YandexDirectClientExecutor") действительно матчит и class YandexDirectClientExecutorResponse (строка 28). Сегодня безвредно (тело Response — только __call__, нет get/post/:param data:, AST это подтверждает), но это латентная хрупкость self-heal. Заменил на boundary-aware ^class YandexDirectClientExecutor[:\(] и добавил тест-гейт с prefix-sharing классом-приманкой, несущим точную сигнатуру executor'а (на старом startswith тест бы упал).

2. v5 host .ru.com — подтверждаю, что это намеренно и пришло из апстрим-bump, а не из этого PR. Оба хоста — валидные entrypoint'ы Yandex Direct (см. docstring endpoints.py). Я отдельно проверил рантайм: v5 → https://api.direct.yandex.com/, v4 → https://api.direct.yandex.ru/ (адаптер шлёт tld="ru"). Прод-валидацию живого .com v5 беру на заметку как отдельный follow-up — здесь TLD-insensitive matcher как раз и нужен, чтобы кассеты пережили этот переход.

3. Indentation coupling — SKIP, по той причине, которую вы сами и назвали: если апстрим переформатирует whitespace и self-heal тихо станет no-op, два поведенческих/AST-гейта (test_vendored_executor_stub_declares_timeout_kwarg и to_columns-тест) уронят CI громко. Это и есть нужная подстраховка; усложнять матчинг whitespace-инвариантным regex'ом сейчас избыточно.

@axisrow

axisrow commented May 30, 2026

Copy link
Copy Markdown
Owner Author

@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

claude Bot commented May 30, 2026

Copy link
Copy Markdown

Claude finished @axisrow's task in 1m 50s —— View job


Re-review: vendor bump regression recovery

I re-reviewed the full diff against origin/main, with focus on the new commit 4145887 and the four fixes. Verified each correctness claim directly against source (not just the diff).

Verdict: no blocking issues. Commit 4145887 correctly resolves the one open observation from the prior review.

Commit 4145887 — boundary-aware class scope ✅

The latent fragility I flagged before is now fixed correctly:

  • _STUB_CLASS_RE = ^class YandexDirectClientExecutor[:\(] matches class YandexDirectClientExecutor: / (...) but not class YandexDirectClientExecutorResponse(...) — the char after the name must be : or (, and Response starts with R. (patch_vendor_imports.py:198)
  • The scope flag is only re-evaluated on line.startswith("class "), so it deterministically flips off when the Response class begins.
  • The new test_patch_stub_text_ignores_prefix_sharing_class is a genuine regression gate: the decoy class carries the exact executor get signature, so the old startswith check would have wrongly patched it and failed the test. Good belt-and-braces.

Other fixes re-verified ✅

  • to_columns guard (tapi_yandex_direct.py:452) — values[i] if i < len(values) else "" pads trailing-omitted cells; pads, never drops. _patch_runtime_text idempotency guard (if _TO_COLUMNS_PATCHED in text) is sound — the upstream string col.append(values[i]) is not a substring of the patched line ( if follows values[i], not )), so no double-patch risk even without the guard.
  • timeout stub — confirmed via endpoints.py: v5 default tld="com", v4 adapter passes tld="ru" (v4/adapter.py:38). The rewritten test_transport_contract.py matches reality exactly.
  • Self-heal idempotency — traced both partial states (sig-only / doc-only); each self-heals via the independent :param data:-anchored doc insertion and the "timeout" not in line sig guard. lines[i+1] dedup makes the second pass a byte-for-byte no-op. The AST-based test_vendored_executor_stub_declares_timeout_kwarg is robust to black reflowing the signature — the right safety net for the acknowledged indentation coupling.
  • VCR matcher (conftest.py) — _host_tld_insensitive raises AssertionError on mismatch (correct vcrpy protocol); non-Direct hosts fall through to verbatim comparison, so global registration via pytest_recording_configure doesn't weaken strictness anywhere except the intended .ru/.com Direct hosts. host or "" handles None.

Remaining open item (not a blocker, already acknowledged)

The only thing not closeable in this PR is live prod validation of the v5 .com host switch — it came from the upstream bump, both hosts are valid entrypoints per endpoints.py, and cassette replay can't catch a host that's silently rejected in production. You already flagged this as a separate follow-up; the TLD-insensitive matcher is exactly what lets cassettes survive the transition. Worth a single live smoke against api.direct.yandex.com before relying on it in prod.

Solid, well-tested recovery. The self-heal approach is the right call given update_vendor.sh does rm -rf + cp -R.
· fix/vendor-timeout-stub

@axisrow
axisrow merged commit efdf18a into main May 30, 2026
6 checks passed
@axisrow
axisrow deleted the fix/vendor-timeout-stub branch May 30, 2026 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Vendor bump tapi_yandex_direct 2026.5.29 затёр локальные правки → 4 регрессии

1 participant