fix(masters): read status from CampaignHeader.Status, not whole-page text (#848) - #849
Merged
Merged
Conversation
…text (#848) `masters archive`/`suspend` refused to act on campaign 107705868 with "unrecognised status text", while the same error's own diagnostic reported `CampaignHeader.ActionButton.stop` as present and enabled — i.e. the campaign was plainly ACTIVE. The refusal is correct behaviour, but it left the campaign unreachable by every lifecycle command. `_read_status_text` matched four fixed Russian phrases as substrings of `inner_text("body")`. Live recon 2026-08-14 established `CampaignHeader.Status` carries the status verbatim in every state, not just DRAFT as the module's older comments assumed — confirmed on real campaigns: "Кампания активна" (713234142), "Кампания остановлена" (107705868), "Кампания в архиве" (100571135). Read that element first; fall back to body text only when absent or unrecognised. This removes the substring-over-everything failure mode: the dashboard renders free-form moderation prose mentioning the campaign (confirmed live on 713234142), so which status won depended on which unrelated banners rendered. Marker comparison is also normalised over case, U+00A0 and collapsed whitespace, so a marker can no longer silently never match over an invisible character — a pitfall that already cost a debugging pass twice (#704, #730). Unrecognised text still returns None, so the refuse-to-click-blind guard is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018f6rRmUxj7Y33jfzcok8gC
Owner
Author
🔍 Local review (cycle 1) — round 1a36be67-1cf9-4eed-8ecd-d5fe2914516eReviewed locally (
|
…ader body fallback (#848 review) Two issues found in local cycle-review (Codex + built-in /review) of PR #849's header-first status fix: - _read_campaign_header_status_text's inner_text() had no explicit timeout, so an absent/not-yet-rendered CampaignHeader.Status element paid Playwright's ~30s default wait — defeating the much shorter 8s _STATUS_CHANGE_TIMEOUT_MS poll budget used by every caller (up to 4 retries via _STATUS_CLICK_MAX_ATTEMPTS). Now passes timeout=1_000, matching this file's existing probe-read convention. - _read_status_text fell through to whole-body matching whenever the header text didn't match a known marker, even though the header had been read successfully — reopening the exact substring-over-everything failure mode #848's fix was meant to close, just for an unrecognised header string instead of a missing one. Now only falls back to body text when the header element itself is absent. 7 new/updated regression tests in TestReadStatusText and TestFetchMasterDraft; full suite green (3628 passed, 23 skipped), black+flake8 clean on changed files. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018f6rRmUxj7Y33jfzcok8gC
Owner
Author
🔍 Local review (cycle 2) — round ba4542d2-bf1b-485f-8d00-6e3aa225d0b8Reviewed locally (
|
round-2 review) Round-1's fix conflated two different meanings of a None read from _read_campaign_header_status_text: 'this page shape structurally has no CampaignHeader.Status element' (safe to fall back to body text) and 'the element is present but its read raised/timed out mid- hydration' (a transient state — the exact window callers are polling through on a status transition). Both reviewers (Codex + built-in /review) flagged that a timeout during header hydration could still trip the unreliable whole-body substring fallback, on a live mutation polling path, reopening a narrower form of the bug #848 fixed. _read_status_text now checks Locator.count() (synchronous, no wait) before reading the header: count() == 0 means structurally absent and falls back to body text as before; count() > 0 means present, so a failed/unrecognised read returns None directly rather than trying body text — consistent with the existing refuse-to-click-blind philosophy, with callers' own poll loops retrying next tick. 2 new regression tests; full suite green (3630 passed, 23 skipped), black+flake8 clean on changed files. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018f6rRmUxj7Y33jfzcok8gC
Owner
Author
🔍 Local review (cycle 3) — round 128e69e8-bdf1-4147-8f85-2b23b0907c7eReviewed locally (
No FIX verdicts this round — cycle complete. |
Owner
Author
📋 Review summary — all cycles
Totals: 3 FIX (all resolved across 2 commits), 5 SKIP, 0 UNVERIFIED. Both reviewers ( |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Проблема
masters archiveиmasters suspendотказывались работать с кампанией 107705868:При этом сама диагностика в тексте ошибки сообщала, что кнопка «Остановить кампанию» отрисована и enabled — то есть кампания заведомо ACTIVE. Отказ сам по себе корректен (лучше не кликать вслепую), но кампания оказывалась недоступна для всех lifecycle-команд.
Причина
_read_status_textискал четыре фиксированные русские фразы как подстроки вinner_text("body")— по всей странице целиком.Живая разведка 2026-08-14 показала, что
CampaignHeader.Statusсодержит статус дословно во всех состояниях, а не только у DRAFT, как предполагали старые комментарии модуля. Подтверждено на реальных кампаниях:CampaignHeader.StatusКампания активнаКампания остановленаКампания в\xa0архивеРешение
Читать этот элемент первым; откат на body-текст — только если элемент отсутствует или не распознан.
Это убирает сам класс отказа «подстрока по всей странице»: дашборд рендерит свободный текст модерации, упоминающий кампанию (подтверждено живьём на 713234142: «…Кампания запущена из\xa0прошедших модерацию элементов объявления»), поэтому какой статус «победит», зависело от того, какие посторонние баннеры отрисовались.
Сравнение маркеров дополнительно нормализуется по регистру, U+00A0 и схлопыванию пробелов — маркер больше не может молча не сматчиться из-за невидимого символа. Эта ловушка уже дважды стоила отладочного прохода (#704, #730).
Нераспознанный текст по-прежнему даёт
None, так что защита refuse-to-click-blind не ослаблена.Проверка
TestReadStatusText); проверено, что без фикса 5 из них падают.pytest— 3626 passed, 23 skipped.black+flake8чисто.Остаётся пользователю
Кампания 107705868 всё ещё не заархивирована — живая production-мутация вне области фикса и требует ручного запуска
masters archive 107705868.Closes #848
🤖 Generated with Claude Code
https://claude.ai/code/session_018f6rRmUxj7Y33jfzcok8gC