Fix name_usage index defects found reviewing #712's fixes

tests/v2/test_name_usage_tool.py

@@ -753,14 +753,20 @@ def test_the_listing_count_allows_a_thousandth_of_slack(
     ("Q101352-ids.json", '["Q1", "L5"]'),
     ("Q101352-ids.json", "[]"),
     ("Q101352-0-1.json", '{"rows": []}'),
-    ("Q101352-0-1.json", '{"fetched": "x", "span": ["Q1", "Q1"], "rows": null}'),
-    ("Q101352-0-1.json", '{"fetched": "x", "span": ["Q1", "Q1"], "rows": [["Q9", "殿"]]}'),
+    ("Q101352-0-1.json", '[]'),
+    ("Q101352-0-1.json", '{"fetched": "x", "digest": "DIGEST", "rows": null}'),
+    ("Q101352-0-1.json", '{"fetched": "x", "digest": "DIGEST", "rows": [["Q9", "殿"]]}'),
+    ("Q101352-0-1.json", '{"fetched": "x", "digest": "DIGEST", "rows": [["Q1"]]}'),
+    ("Q101352-0-1.json", '{"fetched": "x", "digest": "DIGEST", "rows": [["Q1", 5]]}'),
+    ("version.json", "{"),
 ])
 def test_an_unusable_cache_file_is_named_with_its_remedy(
         monkeypatch: pytest.MonkeyPatch, name: str, content: str) -> None:
     _stub_sparql(monkeypatch, [*_listing("Q1"), _labels(("Q1", "向殿"))])
     tool.load_index(download=True)
-    (tool.cache_dir() / tool.INDEX_DIR / name).write_text(content)
+    # DIGEST stands for the batch's own, so the row checks are reached.
+    (tool.cache_dir() / tool.INDEX_DIR / name).write_text(
+        content.replace("DIGEST", tool._batch_digest(["Q1"])))
     _stub_sparql(monkeypatch, [])
     with pytest.raises(ValueError, match=f"{name} is unusable.*--refresh-index"):
         tool.load_index(download=True)
@@ -871,7 +877,9 @@ def test_a_boundary_count_is_chunked_and_counts_a_person_once(
     monkeypatch.setattr(tool, "ITEM_CHUNK", 1)
     index = _index(("Q1", "family", "阿部"), ("Q2", "family", "岡部"),
                    ("Q3", "given", "長"))
-    queries = _stub_sparql(monkeypatch, [_people("Q9"), _people("Q9", "Q8")])
+    # The larger answer first: a count that kept only the last chunk's
+    # people would read 1.
+    queries = _stub_sparql(monkeypatch, [_people("Q9", "Q8"), _people("Q9")])
     usage = tool.ends_with_usage("部長", index, top=5)
     assert len(queries) == 2
     assert usage.splits[0].people == 2
@@ -1001,3 +1009,148 @@ def test_main_reports_an_unusable_cache_without_losing_the_other_sources(
     assert tool.main(["--json", "殿"]) == 2
     rows = json.loads(capsys.readouterr().out)["殿"]
     assert rows[-1]["status"] == "failed" and "unusable" in rows[-1]["note"]
+
+
+# -- second review of #712 ---------------------------------------------------
+
+@pytest.mark.usefixtures("one_class")
+@pytest.mark.parametrize("relisted", [
+    ("Q1", "Q3"),          # the first item kept
+    ("Q0", "Q2"),          # the last item kept
+])
+def test_a_batch_whose_items_changed_at_either_end_is_fetched_again(
+        monkeypatch: pytest.MonkeyPatch, relisted: tuple[str, str]) -> None:
+    _stub_sparql(monkeypatch, [*_listing("Q1", "Q2"), _labels(("Q1", "向殿"))])
+    tool.load_index(download=True)
+    (tool.cache_dir() / tool.INDEX_DIR / "Q101352-ids.json").unlink()
+    queries = _stub_sparql(monkeypatch, [*_listing(*relisted), _labels()])
+    tool.load_index(download=True)
+    assert _values(queries[2]) == [f"wd:{q}" for q in relisted]
+
+
+@pytest.mark.usefixtures("one_class")
+def test_a_batch_whose_middle_item_changed_is_fetched_again(
+        monkeypatch: pytest.MonkeyPatch) -> None:
+    # Same first and last items, another in between: the batch's whole
+    # membership is what its file is checked against.
+    monkeypatch.setattr(tool, "INDEX_BATCH", 3)
+    _stub_sparql(monkeypatch, [*_listing("Q1", "Q2", "Q4"),
+                               _labels(("Q2", "向殿"))])
+    tool.load_index(download=True)
+    (tool.cache_dir() / tool.INDEX_DIR / "Q101352-ids.json").unlink()
+    queries = _stub_sparql(monkeypatch, [*_listing("Q1", "Q3", "Q4"),
+                                         _labels(("Q3", "井殿"))])
+    index = tool.load_index(download=True)
+    assert index is not None
+    assert [e.label for e in index.entries] == ["井殿"]
+    assert _values(queries[2]) == ["wd:Q1", "wd:Q3", "wd:Q4"]
+
+
+@pytest.mark.usefixtures("one_class")
+def test_a_batch_halved_under_a_new_listing_leaves_no_stale_whole(
+        monkeypatch: pytest.MonkeyPatch) -> None:
+    # Listing A fetched the batch whole; listing B overran it and cached
+    # its halves. The whole batch's old file must not shadow the halves,
+    # or the complete index reads as missing offline.
+    _stub_sparql(monkeypatch, [*_listing("Q1", "Q2"), _labels(("Q1", "向殿"))])
+    tool.load_index(download=True)
+    (tool.cache_dir() / tool.INDEX_DIR / "Q101352-ids.json").unlink()
+    _stub_sparql(monkeypatch, [*_listing("Q1", "Q3"), _overrun(),
+                               _labels(("Q1", "向殿")), _labels(("Q3", "井殿"))])
+    tool.load_index(download=True)
+    _stub_sparql(monkeypatch, [])  # any query fails the test
+    offline = tool.load_index(download=False)
+    assert offline is not None
+    assert sorted(e.label for e in offline.entries) == ["井殿", "向殿"]
+    online = tool.load_index(download=True)
+    assert online is not None and len(online.entries) == 2
+
+
+@pytest.mark.usefixtures("one_class")
+def test_an_index_built_before_versioning_is_rebuilt(
+        monkeypatch: pytest.MonkeyPatch,
+        capsys: pytest.CaptureFixture[str]) -> None:
+    # The first index (2026-10-17) had no version file at all, and its
+    # character class missed supplementary-plane Han.
+    _stub_sparql(monkeypatch, [*_listing("Q1"), _labels(("Q1", "向殿"))])
+    tool.load_index(download=True)
+    (tool.cache_dir() / tool.INDEX_DIR / "version.json").unlink()
+    _stub_sparql(monkeypatch, [])
+    assert tool.load_index(download=False) is None
+    capsys.readouterr()
+    _stub_sparql(monkeypatch, [*_listing("Q1"), _labels(("Q1", "𠮷田"))])
+    index = tool.load_index(download=True)
+    assert index is not None and index.entries[0].label == "𠮷田"
+    assert "older version" in capsys.readouterr().err
+
+
+@pytest.mark.usefixtures("one_class")
+def test_a_listing_without_an_item_column_is_an_error(
+        monkeypatch: pytest.MonkeyPatch) -> None:
+    _stub_sparql(monkeypatch, [[{"n": "1"}], [{"x": "y"}]])
+    with pytest.raises(ValueError, match="not item"):
+        tool.load_index(download=True)
+
+
+@pytest.mark.parametrize(("word", "cjk"), [
+    ("\U00031350", True),   # Extension H
+    ("\U0002f800", True),   # the supplementary compatibility block
+])
+def test_is_cjk_covers_extension_h(word: str, cjk: bool) -> None:
+    assert tool.is_cjk(word) is cjk
+
+
+def _closable(code: int, body: bytes = b"") -> tuple[urllib.error.HTTPError, io.BytesIO]:
+    fp = io.BytesIO(body)
+    return urllib.error.HTTPError("u", code, "x", {}, fp), fp  # type: ignore[arg-type]
+
+
+@pytest.mark.usefixtures("one_class")
+@pytest.mark.parametrize(("code", "body"), [
+    (504, b""), (500, b"java.util.concurrent.TimeoutException"), (400, b"syntax")])
+def test_the_index_closes_each_http_error_it_reads(
+        monkeypatch: pytest.MonkeyPatch, code: int, body: bytes) -> None:
+    # Since 3.14 an HTTPError left open warns when collected.
+    error, fp = _closable(code, body)
+    _stub_sparql(monkeypatch, [*_listing("Q1", "Q2"), error, _labels(), _labels()])
+    try:
+        tool.load_index(download=True)
+    except urllib.error.HTTPError:
+        pass
+    assert fp.closed
+
+
+def test_a_failed_wikidata_row_closes_its_http_error(
+        monkeypatch: pytest.MonkeyPatch) -> None:
+    error, fp = _closable(400)
+    _stub_sparql(monkeypatch, [error])
+    assert tool.wikidata_usage("shehu", top=5).status == "failed"
+    assert fp.closed
+
+
+def test_a_failed_ends_with_row_and_pairing_close_their_http_errors(
+        monkeypatch: pytest.MonkeyPatch) -> None:
+    first, first_fp = _closable(400)
+    _stub_sparql(monkeypatch, [first])
+    tool.ends_with_usage("殿", _index(("Q1", "family", "向殿")), top=5)
+    second, second_fp = _closable(400)
+    _stub_sparql(monkeypatch, [second])
+    tool.ends_with_usage(
+        "部長", _index(("Q1", "family", "阿部"), ("Q3", "given", "長")), top=5)
+    assert first_fp.closed and second_fp.closed
+
+
+def test_main_reports_a_refresh_that_failed_for_a_latin_word(
+        monkeypatch: pytest.MonkeyPatch, tmp_path: Path,
+        capsys: pytest.CaptureFixture[str]) -> None:
+    # A Latin word has no ends-with row to carry the failure, and the
+    # refresh has already deleted the old index.
+    monkeypatch.setenv("NAMEPARSER_TOOLS_CACHE", str(tmp_path))
+    monkeypatch.setattr(tool, "load_census", lambda download: None)
+
+    def broken(download: bool, refresh: bool = False) -> None:
+        raise OSError("down")
+    monkeypatch.setattr(tool, "load_index", broken)
+    _stub_sparql(monkeypatch, [[], []])
+    assert tool.main(["--refresh-index", "knight"]) == 2
+    assert "CJK name index: OSError: down" in capsys.readouterr().err

tools/name_usage.py

@@ -66,6 +66,7 @@
 
 import argparse
 import csv
+import hashlib
 import http.client
 import io
 import json
@@ -454,7 +455,7 @@ def wikidata_usage(word: str, top: int) -> Usage:
             _bindings(_sparql(wikidata_countries_query(forms))), top)
     except (OSError, ValueError, KeyError, http.client.HTTPException) as exc:
         return Usage("Wikidata people", status="failed", lower_bound=True,
-                     note=f"query failed: {type(exc).__name__}: {exc}")
+                     note=_failure(exc))
     zero = "a zero proves nothing" if not (usage.given or usage.family) else ""
     usage.note = _join("matched as " + ", ".join(forms), zero)
     return usage
@@ -576,10 +577,11 @@ def _download(url: str, path: Path, required_member: str) -> None:
 assert set(NAME_CLASSES.values()) == {"given", "family"}, NAME_CLASSES
 
 INDEX_DIR = "wikidata_cjk_names"
-#: Bumped whenever what the index holds changes (version 2 widened the
-#: character class to supplementary-plane Han); a cache built by another
-#: version is rebuilt rather than trusted.
-INDEX_VERSION = 2
+#: Bumped whenever what the index holds changes (2 widened the character
+#: class to supplementary-plane Han, 3 to Extension H and keyed each batch
+#: by its whole membership); a cache built by another version is rebuilt
+#: rather than trusted.
+INDEX_VERSION = 3
 #: Items per label query. Measured 2026-10-17 on family-name items not
 #: queried before: 1,000 took 2.2-2.4s and 2,000 took 24s.
 INDEX_BATCH = 1000
@@ -591,14 +593,15 @@ def _download(url: str, path: Path, required_member: str) -> None:
 ENDS_WITH_SOURCE = "Wikidata people, name ends with"
 
 #: The iteration mark 々 and 〇, kana, the CJK ideograph blocks (Extension
-#: A, the unified block, compatibility, and Extensions B onward in the
-#: supplementary planes, where rare surname characters live), hangul
-#: syllables, and halfwidth katakana. The Yijing hexagrams between
-#: Extension A and the unified block are left out. One such letter makes
-#: a label or a word CJK; the query and is_cjk share the class.
-_CJK_CHARS = ("々〇぀-ヿ㐀-䶿一-鿿"
-              "가-힯豈-﫿･-ﾟ"
-              "\U00020000-\U0003134f\U0002f800-\U0002fa1f")
+#: A, the unified block, compatibility, and the supplementary planes from
+#: Extension B through Extension H as of Unicode 15, where rare surname
+#: characters live), hangul syllables, and halfwidth katakana. The Yijing
+#: hexagrams between Extension A and the unified block are left out. One
+#: such letter makes a label or a word CJK; the query and is_cjk share
+#: the class, so widening it is an INDEX_VERSION bump.
+_CJK_CHARS = ("\u3005\u3007\u3040-\u30ff\u3400-\u4dbf\u4e00-\u9fff"
+              "\uac00-\ud7af\uf900-\ufaff\uff65-\uff9f"
+              "\U00020000-\U000323af")
 _CJK = re.compile(f"[{_CJK_CHARS}]")
 
 
@@ -777,16 +780,22 @@ def _class_ids(cls: str, fetch: bool) -> list[str]:
     return ids
 
 
-def _cached_batch(path: Path, span: list[str],
+def _batch_digest(qids: list[str]) -> str:
+    """What a batch file is keyed by: its whole membership, since batches
+    are named by position and, after a class is listed again, the same
+    position can hold other items -- at either end or in between."""
+    return hashlib.sha256(" ".join(qids).encode()).hexdigest()
+
+
+def _cached_batch(path: Path, digest: str,
                   ids: list[str]) -> tuple[str, list[list[str]]] | None:
-    """A batch file's date and rows, or None when it was built against
-    another listing: batches are named by position, so after a class is
-    listed again the same position can hold other items."""
+    """A batch file's date and rows, or None when it was built for other
+    items."""
     data = _read_json(path)
     if not (isinstance(data, dict) and isinstance(data.get("fetched"), str)
             and isinstance(data.get("rows"), list)):
-        raise _unusable(path, "not a batch: needs fetched, span and rows")
-    if data.get("span") != span:
+        raise _unusable(path, "not a batch: needs fetched, digest and rows")
+    if data.get("digest") != digest:
         return None
     rows = data["rows"]
     batch = set(ids)
@@ -807,20 +816,23 @@ def _fetch_batch(cls: str, ids: list[str], start: int, end: int, fetch: bool,
     base = cache_dir() / INDEX_DIR / f"{cls}-{start}-{end}"
     done, halved = base.with_suffix(".json"), base.with_suffix(".halved")
     batch = ids[start:end]
-    span = [batch[0], batch[-1]]
+    middle = (start + end) // 2
+    # The marker is read first: a whole batch's file from an earlier
+    # listing must not shadow the halves this one was cached as. A marker
+    # left by an earlier listing only splits the batch, which costs a
+    # query and loses nothing, since each half is checked by itself.
+    if halved.exists():
+        first = _fetch_batch(cls, ids, start, middle, fetch, out, dates)
+        second = _fetch_batch(cls, ids, middle, end, fetch, out, dates)
+        return first or second
+    digest = _batch_digest(batch)
     if done.exists():
-        cached = _cached_batch(done, span, batch)
+        cached = _cached_batch(done, digest, batch)
         if cached is not None:
             dates.add(cached[0])
             out.extend(IndexEntry(q, NAME_CLASSES[cls], label)
                        for q, label in cached[1])
             return False
-        halved.unlink(missing_ok=True)  # it was halved for other items
-    elif halved.exists():
-        middle = (start + end) // 2
-        first = _fetch_batch(cls, ids, start, middle, fetch, out, dates)
-        second = _fetch_batch(cls, ids, middle, end, fetch, out, dates)
-        return first or second
     if not fetch:
         raise _Missing(done.name)
     try:
@@ -830,14 +842,14 @@ def _fetch_batch(cls: str, ids: list[str], start: int, end: int, fetch: bool,
         if end - start <= INDEX_MIN_BATCH:
             raise ValueError(f"wd:{cls} items {start}-{end} still "
                              f"overran ({exc})") from exc
+        done.unlink(missing_ok=True)
         halved.parent.mkdir(parents=True, exist_ok=True)
         halved.touch()
-        middle = (start + end) // 2
         _fetch_batch(cls, ids, start, middle, fetch, out, dates)
         _fetch_batch(cls, ids, middle, end, fetch, out, dates)
         return True
     today = date.today().isoformat()
-    _write(done, {"fetched": today, "span": span, "rows": rows})
+    _write(done, {"fetched": today, "digest": digest, "rows": rows})
     dates.add(today)
     out.extend(IndexEntry(q, NAME_CLASSES[cls], label) for q, label in rows)
     return True
@@ -920,6 +932,15 @@ def _count_split(split: Split) -> None:
 _QUERY_ERRORS = (OSError, ValueError, KeyError, http.client.HTTPException)
 
 
+def _failure(exc: BaseException) -> str:
+    """The note for a query that failed. An HTTPError holds its response
+    open, and since 3.14 one collected unclosed warns, so it is closed
+    here, where the report lets go of it."""
+    if isinstance(exc, urllib.error.HTTPError):
+        exc.close()
+    return f"query failed: {type(exc).__name__}: {exc}"
+
+
 def ends_with_usage(word: str, index: NameIndex, top: int) -> Usage:
     """Bearers of the indexed name items whose label ends in the word,
     and of the family/given pairs that end in it together. A failed
@@ -949,14 +970,14 @@ def ends_with_usage(word: str, index: NameIndex, top: int) -> Usage:
                                      about))
     except _QUERY_ERRORS as exc:
         return Usage(ENDS_WITH_SOURCE, status="failed", lower_bound=True,
-                     note=f"query failed: {type(exc).__name__}: {exc}")
+                     note=_failure(exc))
     usage.source = ENDS_WITH_SOURCE
     usage.splits = index.boundaries(word)
     for split in usage.splits:
         try:
             _count_split(split)
         except _QUERY_ERRORS as exc:
-            split.error = f"query failed: {type(exc).__name__}: {exc}"
+            split.error = _failure(exc)
     return usage
 
 
@@ -970,6 +991,8 @@ def _attempt(load: Callable[[], T | None]) -> tuple[T | None, str]:
     try:
         return load(), ""
     except (OSError, ValueError, http.client.HTTPException) as exc:
+        if isinstance(exc, urllib.error.HTTPError):
+            exc.close()
         return None, f"{type(exc).__name__}: {exc}"
 
 
@@ -1064,6 +1087,11 @@ def main(argv: list[str] | None = None) -> int:
         print("\n" + ssa_missing_message(), file=sys.stderr)
     failed = any(u.status == "failed" or any(s.error for s in u.splits)
                  for us in report.values() for u in us)
+    if args.refresh_index and index_error:
+        # A Latin word has no ends-with row to carry this, and the refresh
+        # has already deleted the old index.
+        print(f"CJK name index: {index_error}", file=sys.stderr)
+        failed = True
     return 2 if failed else 0