diff --git a/src/wordstat/collector.py b/src/wordstat/collector.py index c08fe60..3ffc7ad 100644 --- a/src/wordstat/collector.py +++ b/src/wordstat/collector.py @@ -23,6 +23,7 @@ BatchCollectionResult, CollectionManifest, CollectionResult, + CsvDataset, ExportSummary, PhraseFailure, WordstatView, @@ -53,6 +54,51 @@ } +def _is_untrustworthy_empty_export(view: WordstatView, dataset: CsvDataset) -> bool: + """True if an empty CSV for this view can never be a legitimate export. + + TOP_POPULAR/TOP_RELATED/DYNAMICS are checked — every view for which + _select_view hard-gates TABLE_ROW_SELECTOR.length > 0 on the DOM before + the "Скачать" click is ever issued (see the docstring on _select_view + and CLAUDE.md's issue #3/#13 section). REGIONS is the only view exempt + from that gate (it is a map with no table rows in its DOM), so it is the + only view exempt here too — this set must stay in lockstep with + _select_view's `if view != WordstatView.REGIONS:` condition, not be + picked per-view by hand. + + A phrase that clears the pre-download gate has already had the code + itself prove TABLE_ROW_SELECTOR.length > 0 moments earlier; genuinely + zero rows never reaches the download step at all — it dies inside + _select_view's retry loop with InterfaceChangedError instead. So by the + time a dataset for one of these views is parsed here, an empty + dataset.rows already contradicts a state the code itself proved moments + earlier; there is no code path left where that emptiness is legitimate, + for any of the three table-based views alike. + + This used to be conditioned on a second, post-download DOM read + (`rendered_rows > 0`) — but re-querying the DOM after the download's + unbounded polling window can observe a table that has since emptied + (rerender, auth transition, page degradation), which let the empty + export through as an apparently valid `row_count: 0` and silently + corrupt the manifest into `status: "complete"` — the exact failure mode + issue #11's fix was meant to close. The pre-download gate is already + proof enough; no second opinion from a later DOM read is needed or + trustworthy. See tests/test_collector_view.py. + + DYNAMICS was previously excluded on the strength of issue #11's live-CDP + data (24 rows across three runs, never empty) — but that is evidence the + gate rarely fires for DYNAMICS, not evidence that omitting it is safe. + The gate's premise is structural (the DOM proved rows>0 immediately + before the click), and that premise holds for DYNAMICS exactly as it + does for TOP_POPULAR/TOP_RELATED; selecting views by observed frequency + of emptiness rather than by the structural gate they share was the bug. + """ + return ( + view in (WordstatView.TOP_POPULAR, WordstatView.TOP_RELATED, WordstatView.DYNAMICS) + and not dataset.rows + ) + + def _without_traceback(error: Exception) -> Exception: """Drop the traceback before an exception is stashed in PhraseFailure. @@ -337,6 +383,11 @@ async def _collect_one( # Convert before disposing of the download, so a parse or # write failure leaves the raw CSV on disk to inspect. dataset = parse_wordstat_csv(source, view) + if _is_untrustworthy_empty_export(view, dataset): + raise InterfaceChangedError( + f"Wordstat returned an empty {view.value} CSV, but the page had rendered at " + "least one table row before the download was triggered; export is not trustworthy" + ) data_path, dtypes = write_dataset(dataset, run_directory) raw_path = finalize_raw(source, run_directory, view, self.keep_raw) except Exception: # noqa: BLE001 diff --git a/tests/test_collector_view.py b/tests/test_collector_view.py index f8bda31..26ca7dc 100644 --- a/tests/test_collector_view.py +++ b/tests/test_collector_view.py @@ -4,9 +4,9 @@ import pytest -from wordstat.collector import WordstatCollector +from wordstat.collector import WordstatCollector, _is_untrustworthy_empty_export from wordstat.errors import InterfaceChangedError -from wordstat.models import WordstatView +from wordstat.models import CsvDataset, WordstatView def test_select_view_retries_once_when_active_marker_does_not_change(monkeypatch, tmp_path): @@ -148,6 +148,37 @@ async def snapshot(self, page): assert soft_required is False +def _dataset(view: WordstatView, rows: list[dict[str, str]]) -> CsvDataset: + return CsvDataset(view=view, headers=["query", "count"], rows=rows) + + +@pytest.mark.parametrize( + "view", [WordstatView.TOP_POPULAR, WordstatView.TOP_RELATED, WordstatView.DYNAMICS] +) +def test_empty_export_is_untrustworthy_for_table_views_regardless_of_dom_state(view): + # Regression guard for issue #11 (and its cycle-2 follow-up): _select_view + # already hard-gates TABLE_ROW_SELECTOR.length > 0 on the DOM before + # "Скачать" is ever clicked for every view but REGIONS, so an empty CSV + # reaching this point is already a contradiction for all three of these + # table-based views — it must be rejected unconditionally, with no + # second, later DOM read able to wave it through as "legitimately empty" + # (that re-read can observe a table that has since emptied and silently + # accept a corrupted export — the exact bug this predicate replaces). + assert _is_untrustworthy_empty_export(view, _dataset(view, [])) is True + + +@pytest.mark.parametrize( + "view", [WordstatView.TOP_POPULAR, WordstatView.TOP_RELATED, WordstatView.DYNAMICS] +) +def test_non_empty_export_is_trusted_for_table_views(view): + assert _is_untrustworthy_empty_export(view, _dataset(view, [{"query": "a", "count": "1"}])) is False + + +def test_empty_regions_export_is_not_flagged_by_this_gate(): + # regions (map view) has no table in its DOM at all. + assert _is_untrustworthy_empty_export(WordstatView.REGIONS, _dataset(WordstatView.REGIONS, [])) is False + + def test_select_view_does_not_wait_for_table_rows_on_map_view(monkeypatch, tmp_path): # The map view (WordstatView.REGIONS) has no table rows in its DOM at # all, so gating on row presence would hang forever; it must only wait