fix: ten review findings, each one a silent wrong answer

commoner_probe/aspnet.py

@@ -202,8 +202,11 @@ def write_buttons(page: str) -> list[tuple[str, str]]:
     before submitting anything is the difference between reading a government
     database and inserting a false record into one.
     """
+    # The label arrives as numeric HTML entities on Hindi-labelled forms, so a
+    # literal match against the Devanagari hints never fires and a live write
+    # control reads as harmless. Decode first.
     return [(n, v) for n, v in submit_buttons(page)
-            if any(h in (n + " " + v).lower() for h in WRITE_HINTS)]
+            if any(h in _html.unescape(n + " " + v).lower() for h in WRITE_HINTS)]
 
 
 def export_controls(page: str) -> list[tuple[str, str]]:

commoner_probe/aspnet_cascade.py

@@ -35,7 +35,7 @@
 
 import html
 import re
-from typing import Any, Iterable, Iterator
+from typing import Any, Callable, Iterable, Iterator
 
 from .http_client import make_session
 
@@ -49,10 +49,42 @@ class CascadeCrawler:
     """
 
     def __init__(self, report_url: str, controls: dict[str, str], *,
-                 session: Any | None = None, rate_limit_sec: float = 1.0) -> None:
+                 session: Any | None = None, rate_limit_sec: float = 1.0,
+                 user_agent: str | None = None,
+                 session_factory: Callable[[], Any] | None = None) -> None:
+        """`user_agent` is exposed because some deployments answer HTTP 500 to
+        this package's own identifier and 200 to a browser string. See failure
+        mode 10 in `aspnet`. Overriding it is a deliberate act, so there is no
+        default: a caller decides, and records the decision where it is made.
+
+        `session_factory` builds a REPLACEMENT session on reseat, and passing
+        one is the caller SAYING that a rebuild is theirs to define. So it wins
+        whenever it is given, including alongside an injected `session`: a
+        caller whose session carries a login supplies a factory that can
+        re-establish it.
+
+        With a `session` and NO factory, the session is never replaced. This
+        class did not build that session, and discarding a caller's
+        authentication is not its call. A crawl in that state cannot recover
+        from an expired session, and that is the caller's trade to make.
+        """
         self.report_url = report_url
         self.controls = controls
-        self.session = session or make_session(rate_limit_sec=rate_limit_sec)
+        # `is not None` on BOTH, for the same reason. A factory is a callable
+        # OBJECT as often as a lambda, and one that defines __len__ or __bool__
+        # can be falsey. `or` would then drop the caller's factory and rebuild
+        # with the default client, or with nothing.
+        if session_factory is not None:
+            self._factory = session_factory
+        elif session is not None:
+            self._factory = None
+        else:
+            self._factory = lambda: make_session(rate_limit_sec=rate_limit_sec,
+                                                 user_agent=user_agent)
+        # `session is not None`, never truthiness. A session object that
+        # defines __len__ or __bool__ can be falsey, and `or` would discard the
+        # caller's object and build another one in silence.
+        self.session = session if session is not None else self._factory()
         self._selected: dict[str, str] = {}
         self.page = self._get()
 
@@ -110,8 +142,23 @@ def select(self, level: str, value: str) -> None:
         self.page = resp.content.decode("utf8", "replace")
 
     def reset(self) -> None:
-        """Drop every selection and refetch, recovering an expired session."""
+        """Replace the session, drop every selection, and refetch.
+
+        The stale state lives in the session cookie, so refetching with the
+        SAME session returns the same HTTP 500 and the crawl never recovers.
+
+        WHICH session survives, in full:
+
+        * a `session_factory` was given — the factory builds a new session,
+          including when a `session` was injected beside it. Supplying a
+          factory is the instruction for how to rebuild.
+        * a `session` alone — it is kept. Replacing it would discard an
+          authentication this class did not create.
+        * neither — the default client is rebuilt.
+        """
         self._selected.clear()
+        if self._factory is not None:
+            self.session = self._factory()
         self.page = self._get()
 
     def reseat(self, path: Iterable[tuple[str, str]]) -> None:

commoner_probe/cdn_dashboard.py

@@ -89,6 +89,19 @@ class Place:
     district_code: str
 
 
+class GeoFenced(RuntimeError):
+    """The edge refused the client's country.
+
+    Separate from an absent period ON PURPOSE. Both answer 403, and conflating
+    them turns a run that was blocked outright into a clean empty dataset —
+    the failure this package exists to refuse.
+    """
+
+
+#: The edge names the country block in the body. Matched case-insensitively.
+GEO_FENCE_MARKERS = ("block access from your country", "not available in your region")
+
+
 def payload_url(year: int, month: int, state_id: int, district_id: int, endpoint: str) -> str:
     """The CDN object for one place, period and endpoint.
 
@@ -309,10 +322,15 @@ def parse_beneficiaries(payload: dict) -> dict:
 def fetch(session: Any, year: int, month: int, place: Place, kind: str) -> dict | None:
     """One endpoint for one district-month. None when the period is absent.
 
-    A 403 here means "no object at this path", not "forbidden": the bucket
+    **Most 403s mean "no object at this path", not "forbidden."** The bucket
     denies listing, so a period that was never published and a path that is
-    wrong are the same response. Callers cannot distinguish them and should
-    not try.
+    wrong produce the same response. Callers cannot separate those two, and
+    should not try.
+
+    **One 403 is different, and it raises.** The edge refuses a client outside
+    the publisher's country, and names the country block in the body. Every
+    object answers that way from such a client, so returning None would turn a
+    blocked run into a clean empty dataset. That case raises `GeoFenced`.
 
     `respect_robots=False` is deliberate and narrow. The CDN host publishes NO
     robots.txt — the request returns S3 AccessDenied, and RobotFileParser turns
@@ -325,7 +343,16 @@ def fetch(session: Any, year: int, month: int, place: Place, kind: str) -> dict
     """
     url = payload_url(year, month, place.state_id, place.district_id, ENDPOINTS[kind])
     resp = session.get(url, respect_robots=False)
-    if resp.status_code in (403, 404):
+    if resp.status_code == 403:
+        body = (getattr(resp, "text", "") or "").lower()
+        if any(m in body for m in GEO_FENCE_MARKERS):
+            raise GeoFenced(
+                f"the edge refused this client's country for {url}. "
+                "Every object will 403 from here, so an empty harvest would be "
+                "a vantage-point artefact. Fetch from within the publisher's "
+                "country.")
+        return None
+    if resp.status_code == 404:
         return None
     resp.raise_for_status()
     return resp.json()

commoner_probe/geoserver.py

@@ -118,7 +118,7 @@ def wfs_status(base: str, *, session: Any = None, timeout: int = 60) -> dict[str
     module: it returns real geometry for every feature type, including the lines
     and polygons that WMS extraction cannot honestly recover.
     """
-    sess = session or make_session()
+    sess = session if session is not None else make_session()
     out: dict[str, Any] = {"enabled": False, "versions": {}, "message": None}
     for version in ("2.0.0", "1.1.0", "1.0.0"):
         url = f"{base.rstrip('/')}/wfs?" + urlencode(
@@ -178,6 +178,47 @@ def offset(self, fraction: float = 0.5) -> "Tile":
                     self.east + dx, self.north + dy, self.depth)
 
 
+def _point_in(feature: dict, bbox: Sequence[float]) -> bool | None:
+    """Whether a feature's point lies in `bbox`. None when it cannot be judged.
+
+    The verification grid starts half a cell BEFORE the region, so it queries
+    ground outside it. A feature there was never this extraction's to find, and
+    counting it as a first-pass miss makes a complete sweep report
+    `saturated=False`.
+
+    A feature with no point geometry returns None. It is kept and counted,
+    because dropping it would hide a real miss and keeping it silently would
+    hide the doubt.
+    """
+    geom = (feature or {}).get("geometry") or {}
+    coords = geom.get("coordinates")
+    kind = geom.get("type")
+    # A point layer serves MultiPoint as readily as Point, and treating that as
+    # unplaceable let an out-of-region feature count as a first-pass miss. Any
+    # other geometry stays unplaceable: a polygon has no single answer here, and
+    # guessing one would be the invented geometry this module refuses.
+    if kind == "Point":
+        points = [coords]
+    elif kind == "MultiPoint" and isinstance(coords, (list, tuple)):
+        points = list(coords)
+    else:
+        return None
+
+    placed = False
+    for point in points:
+        if not isinstance(point, (list, tuple)) or len(point) < 2:
+            continue
+        try:
+            lon, lat = float(point[0]), float(point[1])
+        except (TypeError, ValueError):
+            continue
+        placed = True
+        # One point inside makes the feature this region's.
+        if bbox[0] <= lon <= bbox[2] and bbox[1] <= lat <= bbox[3]:
+            return True
+    return False if placed else None
+
+
 @dataclass
 class GeoServer:
     """A WMS-only GeoServer, swept for point features.
@@ -195,7 +236,7 @@ class GeoServer:
     _stats: dict[str, int] = field(default_factory=lambda: {"requests": 0, "capped": 0})
 
     def __post_init__(self) -> None:
-        self.session = self.session or make_session()
+        self.session = self.session if self.session is not None else make_session()
         self.base = self.base.rstrip("/")
 
     # ---------------------------------------------------------------- discovery
@@ -248,7 +289,8 @@ def features_at(self, layer: str, tile: Tile, *, timeout: int = 120) -> list[dic
     def sweep(self, layer: str, bbox: Sequence[float], *, start_span: float = 2.0,
               min_span: float = 1 / 32, key: str | None = None,
               on_batch: Callable[[dict[str, dict]], None] | None = None,
-              tolerate_tile_errors: bool = True) -> dict[str, dict]:
+              tolerate_tile_errors: bool = True,
+              status: dict | None = None) -> dict[str, dict]:
         """Recursively subdivide ``bbox`` until no tile is capped, collecting points.
 
         ``key`` names the attribute that identifies a feature (a school code, an
@@ -276,6 +318,7 @@ def sweep(self, layer: str, bbox: Sequence[float], *, start_span: float = 2.0,
 
         found: dict[str, dict] = {}
         failures: list[tuple[Tile, str]] = []
+        capped: list[Tile] = []
         while queue:
             tile = queue.pop()
             try:
@@ -288,19 +331,39 @@ def sweep(self, layer: str, bbox: Sequence[float], *, start_span: float = 2.0,
             if len(feats) >= self.feature_count and tile.span > min_span:
                 queue.extend(tile.quarter())
                 continue
+            if len(feats) >= self.feature_count:
+                # The tile is at `min_span` and still capped, so the walk cannot
+                # subdivide further. A cap means "there may be more", so this
+                # leaf is INCOMPLETE. Ingesting it as a complete result truncates
+                # the densest clusters, which are the ones a reader most wants.
+                capped.append(tile)
             for f in feats:
                 props = f.get("properties", {}) or {}
                 ident = str(props.get(key)) if key else str(f.get("id"))
                 if ident and ident not in ("None", ""):
-                    found[ident] = props
+                    # Keep the WHOLE feature. Storing only `properties` dropped
+                    # the coordinates, and a point extractor that returns no
+                    # points answers a different question than the one asked.
+                    found[ident] = f
             if on_batch and len(found) % 500 < len(feats):
                 on_batch(found)
 
         if failures:
             self.log(f"  {layer}: {len(failures)} tile(s) failed and were SKIPPED — "
                      f"this sweep is PARTIAL, not complete")
-            for t, msg in failures[:5]:
-                self.log(f"    {t.west},{t.south},{t.east},{t.north}: {msg}")
+            for tl, msg in failures[:5]:
+                self.log(f"    {tl.west},{tl.south},{tl.east},{tl.north}: {msg}")
+        if capped:
+            self.log(f"  {layer}: {len(capped)} tile(s) hit the feature cap at the "
+                     f"minimum span — this sweep is PARTIAL, and those tiles are "
+                     f"a LOWER BOUND")
+        if status is not None:
+            # The caller cannot see a log line. `verify` must not certify a
+            # sweep it cannot see the holes in.
+            box = lambda t: (t.west, t.south, t.east, t.north)  # noqa: E731
+            status.update(failed=[box(t) for t, _ in failures],
+                          capped=[box(t) for t in capped],
+                          partial=bool(failures or capped))
         return found
 
     def verify(self, layer: str, bbox: Sequence[float], known: Iterable[str], *,
@@ -314,16 +377,56 @@ def verify(self, layer: str, bbox: Sequence[float], known: Iterable[str], *,
         """
         known = set(known)
         west, south, east, north = bbox
-        shifted = Tile(west, south, east, north).offset(0.5)
-        got = self.sweep(layer, (shifted.west, shifted.south, shifted.east, shifted.north),
-                         start_span=start_span, key=key)
+        # Shift by half a CELL, not half the region: a 4-degree box with
+        # 2-degree cells must move 1 degree. Moving half the region queries
+        # ground outside it and leaves the leading edge untested.
+        #
+        # And never by more than half the region itself. A box smaller than one
+        # cell would otherwise move clean off its own ground: (76,12,77,13) with
+        # a 2-degree cell became (77,13,78,14), where finding nothing new
+        # certifies saturation over a region this layer never claimed. Bounded
+        # per axis, because a box can be wide and short.
+        step_x = min(start_span, east - west) / 2
+        step_y = min(start_span, north - south) / 2
+        # Shift the grid ORIGIN backwards, not the whole box forwards. Moving
+        # the box east and north offsets the cell boundaries, but it also
+        # leaves the western and southern strips of the region with no second
+        # pass at all — and saturation is claimed for the WHOLE region. Starting
+        # half a cell before the region keeps the offset and covers every part
+        # of it.
+        shifted = (west - step_x, south - step_y, east, north)
+        status: dict = {}
+        swept = self.sweep(layer, shifted, start_span=start_span, key=key, status=status)
+        got, unlocatable = {}, 0
+        for ident, feature in swept.items():
+            verdict = _point_in(feature, bbox)
+            if verdict is False:
+                continue
+            if verdict is None:
+                unlocatable += 1
+            got[ident] = feature
         new = set(got) - known
+        partial = bool(status.get("partial"))
+        if partial:
+            self.log(f"  {layer}: the verification pass is itself PARTIAL — "
+                     f"{len(status.get('failed', []))} failed and "
+                     f"{len(status.get('capped', []))} capped tile(s). "
+                     "Saturation is NOT claimed.")
         return {
             "pass1": len(known),
             "pass2": len(got),
             "new": len(new),
             "recall": (len(known & set(got)) / len(known)) if known else 0.0,
-            "saturated": not new,
+            # An empty `new` proves saturation only when the second pass
+            # actually asked every question. A pass with holes produces the
+            # same empty set for the opposite reason.
+            "saturated": not new and not partial,
+            "partial": partial,
+            # Features the second pass could not place. They are counted as
+            # in-region, so this number is the doubt in `new`.
+            "unlocatable": unlocatable,
+            "failed_tiles": status.get("failed", []),
+            "capped_tiles": status.get("capped", []),
             "new_ids": sorted(new)[:50],
         }
 

commoner_probe/http_client.py

@@ -354,6 +354,18 @@ def _request(
             url = url + sep + urlencode(params)
         timeout = kwargs.get("timeout") or 60
         body = kwargs.get("data")
+        if body is None and kwargs.get("json") is not None:
+            # The requests path reads `json=`; this one read only `data=` and
+            # dropped it in silence, so a default install posted an EMPTY body
+            # and the server answered as if the caller had sent nothing. Encode
+            # it here rather than in each caller: every adapter shares this
+            # client, and the next one would repeat the bug.
+            body = json.dumps(kwargs["json"]).encode("utf-8")
+            # HTTP header names are case-insensitive. `setdefault` is not, so a
+            # caller who sent `content-type` in another case got a SECOND,
+            # conflicting header instead of keeping their own.
+            if not any(k.lower() == "content-type" for k in headers):
+                headers["Content-Type"] = "application/json"
         if isinstance(body, str):
             body = body.encode("utf-8")
         req = urllib.request.Request(url, data=body, headers=headers, method=method)

commoner_probe/members.py

@@ -24,7 +24,7 @@ class MPRoster:
     """Fetches and matches members from Sansad rosters (LS/RS)."""
 
     def __init__(self, session: StdlibSession | None = None):
-        self.session = session or make_session()
+        self.session = session if session is not None else make_session()
         self._roster: dict[str, MemberInfo] = {}
         self._normalized_map: dict[str, str] = {}
 

commoner_probe/nada.py

@@ -106,7 +106,7 @@ def __init__(
         self.base_url = base_url.rstrip("/")
         self.api = f"{self.base_url}/index.php/api/catalog"
         self.pages = f"{self.base_url}/index.php/catalog"
-        self.session = session or make_session()
+        self.session = session if session is not None else make_session()
         self.sleep = sleep
 
     def _get_json(self, url: str, params: dict | None = None) -> dict:

commoner_probe/ogd_resource_api.py

@@ -268,7 +268,7 @@ def __init__(
     ) -> None:
         self.out_dir = Path(out_dir)
         self.sleep = sleep
-        self.session = session or make_session(rate_limit_sec=sleep)
+        self.session = session if session is not None else make_session(rate_limit_sec=sleep)
         self.api_key = resolve_api_key(api_key)
         self.manifest = self.out_dir / "manifest.jsonl"
         self.rows_dir = self.out_dir / "rows"

commoner_probe/rest_dataset_api.py

@@ -118,7 +118,7 @@ class MospiClient:
     """Thin typed wrapper over the eSankhyiki REST routes."""
 
     def __init__(self, *, sleep: float = 0.5, session=None) -> None:
-        self.session = session or make_session()
+        self.session = session if session is not None else make_session()
         self.sleep = sleep
 
     def _get_json(self, path: str, params: dict[str, Any]) -> dict: