From 2f3f86908491cc1d84a68551fb04061f1f47898f Mon Sep 17 00:00:00 2001
From: skishchampi <996985+skishchampi@users.noreply.github.com>
Date: Sun, 16 Aug 2026 23:15:03 -0400
Subject: [PATCH 01/10] fix: ten review findings, each one a silent wrong
answer
Every one of these returns a plausible result rather than an error. Each fix
has a test that fails first.
geoserver, four findings, and three of them defeat the module's own purpose:
- The sweep discarded the point location. It stored `properties` and dropped
`geometry`, so a POINT extractor returned attributes and no points.
- A tile capped at the minimum span was ingested as complete. A cap means
"there may be more", so the densest clusters were truncated in silence.
Capped leaves are now reported and the sweep is marked PARTIAL.
- The verification grid shifted by half the REGION instead of half a cell. A
4-degree box with 2-degree cells moved 2 degrees. It queried ground outside
the region, left the leading edge untested, then reported saturation.
- Saturation was claimed even when every verification tile failed. An empty
`new` set proves nothing when the second pass asked no questions. `sweep`
now reports failed and capped tiles through a `status` dict, and `verify`
refuses to certify a pass with holes in it.
http_client: the stdlib client ignored `json=`. A default install posted an
empty body, and the server answered as though the caller had sent nothing. The
fix goes in the shared client, not in each caller.
aspnet: a Hindi save button arrives as numeric HTML entities, so the literal
Devanagari hints never matched and a live WRITE control read as harmless. The
label is decoded before classification.
cdn_dashboard: a geo-fence 403 was read as an absent period. Outside the
publisher's country every object 403s, so a blocked run produced a clean empty
dataset. It now raises `GeoFenced`. An ordinary 403 still means absent.
cdn_dashboard: `cryptography` was imported and declared nowhere. It is now the
`crypto` extra, and it is in `all` and `dev`.
aspnet_cascade: reseating reused the poisoned session, so the recovery path
could not recover. It builds a new session, unless the caller injected one.
The User-Agent is now a parameter, because some deployments answer 500 to this
package's identifier.
---
commoner_probe/aspnet.py | 5 +-
commoner_probe/aspnet_cascade.py | 31 ++++-
commoner_probe/cdn_dashboard.py | 24 +++-
commoner_probe/geoserver.py | 55 +++++++--
commoner_probe/http_client.py | 8 ++
pyproject.toml | 5 +-
tests/test_geoserver.py | 66 ++++++++++
tests/test_review_findings_2026_08_17.py | 146 +++++++++++++++++++++++
8 files changed, 325 insertions(+), 15 deletions(-)
create mode 100644 tests/test_review_findings_2026_08_17.py
diff --git a/commoner_probe/aspnet.py b/commoner_probe/aspnet.py
index 31741f5..1741e47 100644
--- a/commoner_probe/aspnet.py
+++ b/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]]:
diff --git a/commoner_probe/aspnet_cascade.py b/commoner_probe/aspnet_cascade.py
index ca985d9..72df65a 100644
--- a/commoner_probe/aspnet_cascade.py
+++ b/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,25 @@ 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. An injected
+ `session` is never replaced, because a caller's authenticated session
+ is not this class's to discard.
+ """
self.report_url = report_url
self.controls = controls
- self.session = session or make_session(rate_limit_sec=rate_limit_sec)
+ self._factory = session_factory or (
+ None if session is not None
+ else lambda: make_session(rate_limit_sec=rate_limit_sec,
+ user_agent=user_agent))
+ self.session = session or self._factory()
self._selected: dict[str, str] = {}
self.page = self._get()
@@ -110,8 +125,16 @@ 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.
+ A session the caller injected is kept: replacing it would discard an
+ authentication this class did not create.
+ """
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:
diff --git a/commoner_probe/cdn_dashboard.py b/commoner_probe/cdn_dashboard.py
index 2a7e48c..0cad0de 100644
--- a/commoner_probe/cdn_dashboard.py
+++ b/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.
@@ -325,7 +338,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()
diff --git a/commoner_probe/geoserver.py b/commoner_probe/geoserver.py
index 1363ebc..d39938f 100644
--- a/commoner_probe/geoserver.py
+++ b/commoner_probe/geoserver.py
@@ -248,7 +248,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 +277,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 +290,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 +336,33 @@ 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 2 degrees queries ground
+ # outside the region, leaves the leading edge untested, and then calls
+ # the result saturated.
+ step = start_span / 2
+ shifted = (west + step, south + step, east + step, north + step)
+ status: dict = {}
+ got = self.sweep(layer, shifted, start_span=start_span, key=key, status=status)
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,
+ "failed_tiles": status.get("failed", []),
+ "capped_tiles": status.get("capped", []),
"new_ids": sorted(new)[:50],
}
diff --git a/commoner_probe/http_client.py b/commoner_probe/http_client.py
index 8b58854..6e7499d 100644
--- a/commoner_probe/http_client.py
+++ b/commoner_probe/http_client.py
@@ -354,6 +354,14 @@ 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")
+ headers.setdefault("Content-Type", "application/json")
if isinstance(body, str):
body = body.encode("utf-8")
req = urllib.request.Request(url, data=body, headers=headers, method=method)
diff --git a/pyproject.toml b/pyproject.toml
index 0634d1a..88ae64c 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -50,9 +50,12 @@ budget = ["lxml>=5.0"]
# Sikkim alone publishes Statement V as OLE2 .xls; the other 34 states ship xlsx
# that the stdlib reader handles, so this stays optional.
xls = ["xlrd>=2.0.1"]
+# AES-CBC envelope decryption for dashboards that wrap their reference data.
+crypto = ["cryptography>=42"]
academia = ["beautifulsoup4>=4.12", "pdfminer.six>=20231228"]
-dev = ["jsonschema>=4.20", "pytest>=7", "ruff>=0.4", "lxml>=5.0", "beautifulsoup4>=4.12", "xlrd>=2.0.1"]
+dev = ["jsonschema>=4.20", "pytest>=7", "ruff>=0.4", "lxml>=5.0", "beautifulsoup4>=4.12", "xlrd>=2.0.1", "cryptography>=42"]
all = [
+ "cryptography>=42",
"pdfminer.six>=20231228",
"requests>=2.31.0",
"lxml>=5.0",
diff --git a/tests/test_geoserver.py b/tests/test_geoserver.py
index 3a21de7..6a6e91e 100644
--- a/tests/test_geoserver.py
+++ b/tests/test_geoserver.py
@@ -138,3 +138,69 @@ def test_verify_reports_saturation_only_when_nothing_new_appears():
out2 = gs2.verify("ws:layer", (76.0, 12.0, 77.0, 13.0),
known={"1000"}, key="code")
assert out2["saturated"] is False and out2["new"] == 3
+
+
+# --- findings from review, 2026-08-17 ------------------------------------------
+
+
+def _fc_geo(n, start=0):
+ """A feature collection in the shape the standard actually specifies:
+ coordinates live in `geometry`, not in `properties`."""
+ return json.dumps({"features": [
+ {"id": f"f.{i}", "properties": {"code": str(1000 + i)},
+ "geometry": {"type": "Point", "coordinates": [76.0 + i, 12.0 + i]}}
+ for i in range(start, start + n)]})
+
+
+def test_the_point_location_survives_the_sweep():
+ """This module exists to extract POINTS. Returning attributes without
+ coordinates answers a different question than the one asked."""
+ sess = _Session([_fc_geo(2)])
+ gs = GeoServer("http://x/geoserver", session=sess, feature_count=400)
+ got = gs.sweep("ws:layer", (76.0, 12.0, 77.0, 13.0), start_span=2.0, key="code")
+ assert got["1000"]["geometry"]["coordinates"] == [76.0, 12.0]
+ assert got["1000"]["properties"]["code"] == "1000"
+
+
+def test_a_tile_capped_at_the_floor_is_reported_not_ingested():
+ """A cap means 'there may be more'. At `min_span` the walk can subdivide no
+ further, so the honest move is to record the leaf as incomplete."""
+ sess = _Session([_fc(2)])
+ gs = GeoServer("http://x/geoserver", session=sess, feature_count=2)
+ status: dict = {}
+ gs.sweep("ws:layer", (76.0, 12.0, 76.5, 12.5), start_span=0.5,
+ min_span=0.5, key="code", status=status)
+ assert status["capped"], "a capped leaf must be reported"
+ assert status["partial"] is True
+
+
+def test_the_offset_grid_shifts_by_one_cell_not_by_the_whole_region():
+ """A 4-degree box with 2-degree cells must move 1 degree, not 2. Moving by
+ half the region tests ground outside it and leaves the leading edge
+ unexamined, then calls the result saturated."""
+ import re
+ from urllib.parse import unquote
+
+ sess = _Session([])
+ gs = GeoServer("http://x/geoserver", session=sess)
+ gs.verify("ws:layer", (76.0, 12.0, 80.0, 16.0), known=[], start_span=2.0, key="code")
+ wests = sorted(float(unquote(re.search(r"bbox=([^&]+)", u, re.I).group(1)).split(",")[0])
+ for u in sess.urls)
+ assert wests[0] == 77.0, f"expected a one-cell shift to 77.0, got {wests[0]}"
+ assert wests[-1] == 79.0, f"the grid must not run past one cell beyond the box: {wests}"
+
+
+def test_saturation_is_refused_when_a_verification_tile_failed():
+ """With every offset tile failing, `new` is empty for the wrong reason.
+ Reporting saturation there certifies completeness from no evidence."""
+ class _Failing:
+ urls: list = []
+
+ def get(self, url, timeout=None):
+ raise RuntimeError("boom")
+
+ gs = GeoServer("http://x/geoserver", session=_Failing(), feature_count=400)
+ out = gs.verify("ws:layer", (76.0, 12.0, 80.0, 16.0), known=["1000"],
+ start_span=2.0, key="code")
+ assert out["saturated"] is False
+ assert out["partial"] is True
diff --git a/tests/test_review_findings_2026_08_17.py b/tests/test_review_findings_2026_08_17.py
new file mode 100644
index 0000000..df9d348
--- /dev/null
+++ b/tests/test_review_findings_2026_08_17.py
@@ -0,0 +1,146 @@
+"""Regression tests for the review findings of 2026-08-17."""
+from __future__ import annotations
+
+import json
+
+import pytest
+
+from commoner_probe import aspnet, aspnet_cascade, cdn_dashboard, spa_jwt_api
+from commoner_probe.http_client import StdlibSession
+
+
+class TestTheStdlibClientSendsJsonBodies:
+ """`json=` was read by the requests path and ignored by the stdlib one, so
+ a default install posted an empty body and the OTP flow could not work."""
+
+ def test_a_json_body_reaches_the_wire(self, monkeypatch):
+ monkeypatch.setattr("commoner_probe.http_client.is_safe_url", lambda u: True)
+ sent = {}
+
+ def fake_open(self, req, timeout=None):
+ sent["body"] = req.data
+ sent["type"] = req.headers.get("Content-type")
+ raise SystemExit # stop before the network
+
+ monkeypatch.setattr("urllib.request.OpenerDirector.open", fake_open)
+ s = StdlibSession()
+ with pytest.raises(SystemExit):
+ s.post("https://example.gov.in/api", json={"mobile": "9"},
+ respect_robots=False)
+ assert json.loads(sent["body"]) == {"mobile": "9"}
+ assert sent["type"] == "application/json"
+
+ def test_an_explicit_data_body_still_wins(self, monkeypatch):
+ monkeypatch.setattr("commoner_probe.http_client.is_safe_url", lambda u: True)
+ sent = {}
+
+ def fake_open(self, req, timeout=None):
+ sent["body"] = req.data
+ raise SystemExit
+
+ monkeypatch.setattr("urllib.request.OpenerDirector.open", fake_open)
+ s = StdlibSession()
+ with pytest.raises(SystemExit):
+ s.post("https://example.gov.in/api", data="a=1", respect_robots=False)
+ assert sent["body"] == b"a=1"
+
+
+class TestWriteButtonDetection:
+ """A missed write button means a crawler posts a form that inserts a record
+ into a live government system."""
+
+ def test_an_entity_encoded_label_is_decoded_before_matching(self):
+ page = ('')
+ assert aspnet.write_buttons(page), "the Hindi label decodes to a save button"
+
+ def test_a_plain_read_button_is_still_harmless(self):
+ page = ''
+ assert aspnet.write_buttons(page) == []
+
+
+class TestTheGeoFenceIsNotAnEmptyPeriod:
+ """Outside the publisher's country every object 403s. Reading that as
+ 'unpublished' turns a blocked run into a clean empty dataset."""
+
+ class _Resp:
+ def __init__(self, status, text=""):
+ self.status_code, self.text = status, text
+
+ def json(self):
+ return {}
+
+ def raise_for_status(self):
+ pass
+
+ class _Session:
+ def __init__(self, resp):
+ self.resp = resp
+
+ def get(self, url, **kw):
+ return self.resp
+
+ def test_a_geo_fence_403_raises(self):
+ body = "configured to block access from your country"
+ sess = self._Session(self._Resp(403, body))
+ place = cdn_dashboard.Place(17, "S", 112, "D", "24", "24445")
+ with pytest.raises(cdn_dashboard.GeoFenced):
+ cdn_dashboard.fetch(sess, 2026, 6, place, "growth")
+
+ def test_an_ordinary_403_is_still_an_absent_period(self):
+ sess = self._Session(self._Resp(403, "AccessDenied"))
+ place = cdn_dashboard.Place(17, "S", 112, "D", "24", "24445")
+ assert cdn_dashboard.fetch(sess, 2026, 6, place, "growth") is None
+
+
+class TestTheCascadeRecovery:
+ def test_reseating_builds_a_new_session(self):
+ """The stale state lives in the session cookie. Refetching with the same
+ session returns the same 500."""
+ built = []
+
+ class _Sess:
+ def __init__(self):
+ built.append(self)
+
+ def get(self, url, **kw):
+ class R:
+ content = b''
+ return R()
+
+ def post(self, url, **kw):
+ raise RuntimeError("HTTP 500")
+
+ crawler = aspnet_cascade.CascadeCrawler(
+ "https://x.gov.in/r.aspx", {"a": "ctl00$a"},
+ session_factory=lambda: _Sess())
+ first = crawler.session
+ crawler.reset()
+ assert crawler.session is not first, "a poisoned session must be replaced"
+
+ def test_an_injected_session_is_never_replaced(self):
+ """A test double, or a caller's authenticated session, must survive."""
+ class _Sess:
+ def get(self, url, **kw):
+ class R:
+ content = b""
+ return R()
+
+ mine = _Sess()
+ crawler = aspnet_cascade.CascadeCrawler("https://x.gov.in/r.aspx",
+ {"a": "ctl00$a"}, session=mine)
+ crawler.reset()
+ assert crawler.session is mine
+
+
+def test_the_cryptography_extra_is_declared():
+ """`decrypt_envelope` imports cryptography. A clean install must be able to
+ get it from a named extra rather than by guessing."""
+ import pathlib
+
+ import tomllib
+
+ root = pathlib.Path(spa_jwt_api.__file__).resolve().parent.parent
+ extras = tomllib.loads((root / "pyproject.toml").read_text())["project"]["optional-dependencies"]
+ assert any("cryptography" in dep for dep in extras.get("crypto", []))
+ assert any("cryptography" in dep for dep in extras.get("all", []))
From fb86d2fda070313e3066b8dd5b71ee6dc7488d0c Mon Sep 17 00:00:00 2001
From: skishchampi <996985+skishchampi@users.noreply.github.com>
Date: Sun, 16 Aug 2026 23:20:14 -0400
Subject: [PATCH 02/10] fix: two more review findings on the fixes themselves
The P1 is mine, and it would have broken CI rather than a crawl. My new test
imported tomllib, which arrived in 3.11. This package supports 3.10, and the
workflow runs the suite on 3.10, 3.11 and 3.12. The test now reads the extras
with a regex.
The P2: HTTP header names are case-insensitive and `setdefault` is not, so a
caller who sent `content-type` in another case would have received a SECOND,
conflicting header rather than keeping their own.
---
commoner_probe/http_client.py | 6 ++++-
tests/test_review_findings_2026_08_17.py | 32 ++++++++++++++++++++----
2 files changed, 32 insertions(+), 6 deletions(-)
diff --git a/commoner_probe/http_client.py b/commoner_probe/http_client.py
index 6e7499d..f3fe9b6 100644
--- a/commoner_probe/http_client.py
+++ b/commoner_probe/http_client.py
@@ -361,7 +361,11 @@ def _request(
# 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")
- headers.setdefault("Content-Type", "application/json")
+ # 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)
diff --git a/tests/test_review_findings_2026_08_17.py b/tests/test_review_findings_2026_08_17.py
index df9d348..de17431 100644
--- a/tests/test_review_findings_2026_08_17.py
+++ b/tests/test_review_findings_2026_08_17.py
@@ -137,10 +137,32 @@ def test_the_cryptography_extra_is_declared():
"""`decrypt_envelope` imports cryptography. A clean install must be able to
get it from a named extra rather than by guessing."""
import pathlib
+ import re
- import tomllib
-
+ # No tomllib. It arrived in 3.11, and CI runs this suite on 3.10 too.
root = pathlib.Path(spa_jwt_api.__file__).resolve().parent.parent
- extras = tomllib.loads((root / "pyproject.toml").read_text())["project"]["optional-dependencies"]
- assert any("cryptography" in dep for dep in extras.get("crypto", []))
- assert any("cryptography" in dep for dep in extras.get("all", []))
+ text = (root / "pyproject.toml").read_text(encoding="utf-8")
+ for extra in ("crypto", "all"):
+ block = re.search(rf"^{extra} = (\[.*?\])", text, re.S | re.M)
+ assert block, f"no {extra} extra is declared"
+ assert "cryptography" in block.group(1), f"{extra} does not carry cryptography"
+
+
+def test_a_lower_cased_content_type_is_not_duplicated(monkeypatch):
+ """Header names are case-insensitive. A caller's own `content-type` must
+ survive, rather than gain a second header beside it."""
+ sent = {}
+
+ def fake_open(self, req, timeout=None):
+ sent["headers"] = dict(req.headers)
+ raise SystemExit
+
+ monkeypatch.setattr("commoner_probe.http_client.is_safe_url", lambda u: True)
+ monkeypatch.setattr("urllib.request.OpenerDirector.open", fake_open)
+ s = StdlibSession()
+ with pytest.raises(SystemExit):
+ s.post("https://example.gov.in/api", json={"a": 1},
+ headers={"content-type": "application/vnd.api+json"},
+ respect_robots=False)
+ types = [v for k, v in sent["headers"].items() if k.lower() == "content-type"]
+ assert types == ["application/vnd.api+json"], types
From a54805439c429ae5128a4c3ca41a035b7a7bb7e7 Mon Sep 17 00:00:00 2001
From: skishchampi <996985+skishchampi@users.noreply.github.com>
Date: Sun, 16 Aug 2026 23:37:40 -0400
Subject: [PATCH 03/10] fix: the offset grid must not walk off a small region
My own previous fix caused this. It shifted by half a cell unconditionally, so
a box smaller than one cell moved clean off its own ground: (76,12,77,13) with
a 2-degree cell became (77,13,78,14). Finding nothing there means nothing, and
the method then reported saturation over ground the layer never claimed.
The shift is now bounded per axis at half the region. A box can be wide and
short, so the two axes are computed separately.
---
commoner_probe/geoserver.py | 18 ++++++++++++------
tests/test_geoserver.py | 20 ++++++++++++++++++++
2 files changed, 32 insertions(+), 6 deletions(-)
diff --git a/commoner_probe/geoserver.py b/commoner_probe/geoserver.py
index d39938f..4334bb2 100644
--- a/commoner_probe/geoserver.py
+++ b/commoner_probe/geoserver.py
@@ -336,12 +336,18 @@ def verify(self, layer: str, bbox: Sequence[float], known: Iterable[str], *,
"""
known = set(known)
west, south, east, north = bbox
- # Shift by half a CELL, not half the region. A 4-degree box with
- # 2-degree cells must move 1 degree. Moving 2 degrees queries ground
- # outside the region, leaves the leading edge untested, and then calls
- # the result saturated.
- step = start_span / 2
- shifted = (west + step, south + step, east + step, north + step)
+ # 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
+ shifted = (west + step_x, south + step_y, east + step_x, north + step_y)
status: dict = {}
got = self.sweep(layer, shifted, start_span=start_span, key=key, status=status)
new = set(got) - known
diff --git a/tests/test_geoserver.py b/tests/test_geoserver.py
index 6a6e91e..42c0cd4 100644
--- a/tests/test_geoserver.py
+++ b/tests/test_geoserver.py
@@ -204,3 +204,23 @@ def get(self, url, timeout=None):
start_span=2.0, key="code")
assert out["saturated"] is False
assert out["partial"] is True
+
+
+def test_the_offset_grid_still_overlaps_a_box_smaller_than_one_cell():
+ """The one-cell shift must not walk off the region it is verifying.
+
+ A 1-degree box with 2-degree cells would move a full degree and land
+ entirely outside itself. The pass then finds nothing, finds nothing NEW,
+ and certifies saturation from ground the layer was never claimed to cover.
+ """
+ import re
+ from urllib.parse import unquote
+
+ sess = _Session([])
+ gs = GeoServer("http://x/geoserver", session=sess)
+ gs.verify("ws:layer", (76.0, 12.0, 77.0, 13.0), known=[], start_span=2.0, key="code")
+ boxes = [[float(x) for x in unquote(re.search(r"bbox=([^&]+)", u, re.I).group(1)).split(",")]
+ for u in sess.urls]
+ assert boxes, "the verification pass made no request"
+ overlaps = [b for b in boxes if b[0] < 77.0 and b[2] > 76.0 and b[1] < 13.0 and b[3] > 12.0]
+ assert overlaps, f"no verification tile overlaps the original box: {boxes}"
From 7f741ca05812c465a34bb8a98ba77b5e0b1de11f Mon Sep 17 00:00:00 2001
From: skishchampi <996985+skishchampi@users.noreply.github.com>
Date: Sun, 16 Aug 2026 23:41:51 -0400
Subject: [PATCH 04/10] docs: state which session survives a reseat, and test
all three cases
Review found the code and its own docstring disagreeing. The docstring promised
that an injected session is never replaced. The code replaced it whenever a
factory was also supplied.
The contract now says what the code does, because the code is right. Passing a
factory IS the caller saying that a rebuild is theirs to define, so it wins
even beside an injected session: a caller whose session carries a login
supplies a factory that can re-establish it.
A session with no factory is still never replaced. A crawl in that state cannot
recover from an expired session, and that is the caller's trade to make.
Three combinations, three tests.
---
commoner_probe/aspnet_cascade.py | 13 +++++--
tests/test_review_findings_2026_08_17.py | 46 ++++++++++++++++++++++++
2 files changed, 56 insertions(+), 3 deletions(-)
diff --git a/commoner_probe/aspnet_cascade.py b/commoner_probe/aspnet_cascade.py
index 72df65a..75ccd2e 100644
--- a/commoner_probe/aspnet_cascade.py
+++ b/commoner_probe/aspnet_cascade.py
@@ -57,9 +57,16 @@ def __init__(self, report_url: str, controls: dict[str, str], *,
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. An injected
- `session` is never replaced, because a caller's authenticated session
- is not this class's to discard.
+ `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
diff --git a/tests/test_review_findings_2026_08_17.py b/tests/test_review_findings_2026_08_17.py
index de17431..377b00a 100644
--- a/tests/test_review_findings_2026_08_17.py
+++ b/tests/test_review_findings_2026_08_17.py
@@ -166,3 +166,49 @@ def fake_open(self, req, timeout=None):
respect_robots=False)
types = [v for k, v in sent["headers"].items() if k.lower() == "content-type"]
assert types == ["application/vnd.api+json"], types
+
+
+class TestTheSessionContract:
+ """Three combinations, three documented behaviours. The middle one was
+ contradicted by the docstring until review caught it."""
+
+ class _Sess:
+ def __init__(self, tag="x"):
+ self.tag = tag
+
+ def get(self, url, **kw):
+ class R:
+ content = b""
+ return R()
+
+ def _crawler(self, **kw):
+ return aspnet_cascade.CascadeCrawler("https://x.gov.in/r.aspx",
+ {"a": "ctl00$a"}, **kw)
+
+ def test_a_session_alone_is_never_replaced(self):
+ mine = self._Sess("mine")
+ c = self._crawler(session=mine)
+ c.reset()
+ assert c.session is mine
+
+ def test_a_factory_beside_a_session_wins_on_reseat(self):
+ """Passing a factory IS the instruction to rebuild. A caller whose
+ session carries a login supplies a factory that re-establishes it."""
+ built = []
+
+ def factory():
+ s = TestTheSessionContract._Sess("rebuilt")
+ built.append(s)
+ return s
+
+ mine = self._Sess("mine")
+ c = self._crawler(session=mine, session_factory=factory)
+ assert c.session is mine, "the injected session is used until a reseat"
+ c.reset()
+ assert c.session is built[-1]
+
+ def test_the_default_client_is_rebuilt(self):
+ c = self._crawler(session_factory=lambda: TestTheSessionContract._Sess())
+ first = c.session
+ c.reset()
+ assert c.session is not first
From cc2c9f3a2120867c8311187e17aaeb71a7605bae Mon Sep 17 00:00:00 2001
From: skishchampi <996985+skishchampi@users.noreply.github.com>
Date: Sun, 16 Aug 2026 23:45:58 -0400
Subject: [PATCH 05/10] fix: an injected session is chosen by identity, not by
truthiness
`session or make_session()` discards any session object whose truthiness is
false. A session that defines __len__ or __bool__ is ordinary, and the caller
then runs against a client this package built without saying so.
Review found one site. The idiom was in fourteen, across ten modules, so all
fourteen now test `is not None`. Fixing the reported line alone would have left
the same defect in nine other adapters.
Also: `reset()` promised that an injected session is never replaced, which the
class docstring had already corrected. The method now states all three cases.
---
commoner_probe/aspnet_cascade.py | 16 +++++++++++++---
commoner_probe/geoserver.py | 4 ++--
commoner_probe/members.py | 2 +-
commoner_probe/nada.py | 2 +-
commoner_probe/ogd_resource_api.py | 2 +-
commoner_probe/rest_dataset_api.py | 2 +-
commoner_probe/spa_jwt_api.py | 6 +++---
commoner_probe/wayback.py | 10 +++++-----
tests/test_review_findings_2026_08_17.py | 19 +++++++++++++++++++
9 files changed, 46 insertions(+), 17 deletions(-)
diff --git a/commoner_probe/aspnet_cascade.py b/commoner_probe/aspnet_cascade.py
index 75ccd2e..92b0a3b 100644
--- a/commoner_probe/aspnet_cascade.py
+++ b/commoner_probe/aspnet_cascade.py
@@ -74,7 +74,10 @@ class did not build that session, and discarding a caller's
None if session is not None
else lambda: make_session(rate_limit_sec=rate_limit_sec,
user_agent=user_agent))
- self.session = session or self._factory()
+ # `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()
@@ -136,8 +139,15 @@ def reset(self) -> None:
The stale state lives in the session cookie, so refetching with the
SAME session returns the same HTTP 500 and the crawl never recovers.
- A session the caller injected is kept: replacing it would discard an
- authentication this class did not create.
+
+ 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:
diff --git a/commoner_probe/geoserver.py b/commoner_probe/geoserver.py
index 4334bb2..c50939e 100644
--- a/commoner_probe/geoserver.py
+++ b/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(
@@ -195,7 +195,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
diff --git a/commoner_probe/members.py b/commoner_probe/members.py
index 307180d..46615f8 100644
--- a/commoner_probe/members.py
+++ b/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] = {}
diff --git a/commoner_probe/nada.py b/commoner_probe/nada.py
index c7f34e7..d8c1c6b 100644
--- a/commoner_probe/nada.py
+++ b/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:
diff --git a/commoner_probe/ogd_resource_api.py b/commoner_probe/ogd_resource_api.py
index 2c825cc..4366825 100644
--- a/commoner_probe/ogd_resource_api.py
+++ b/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"
diff --git a/commoner_probe/rest_dataset_api.py b/commoner_probe/rest_dataset_api.py
index 40a3802..f554bfc 100644
--- a/commoner_probe/rest_dataset_api.py
+++ b/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:
diff --git a/commoner_probe/spa_jwt_api.py b/commoner_probe/spa_jwt_api.py
index de7cc0b..3ce606d 100644
--- a/commoner_probe/spa_jwt_api.py
+++ b/commoner_probe/spa_jwt_api.py
@@ -218,7 +218,7 @@ def request_otp(mobile: str, *, base: str = DSP_BASE, session: Any = None,
if solve is None:
raise ValueError("request_otp needs solve= str>; "
"a human must read the captcha")
- sess = session or make_session()
+ sess = session if session is not None else make_session()
key, png = _captcha(base, sess, timeout)
value = solve(png)
r = sess.post(base + DSP_ENDPOINTS["send_otp"],
@@ -238,7 +238,7 @@ def verify_otp(mobile: str, otp: str, *, base: str = DSP_BASE,
``mobile_invalid_strict`` rather than "expired", and the real OTP is burnt by
the time the quoting is fixed.
"""
- sess = session or make_session()
+ sess = session if session is not None else make_session()
r = sess.post(base + DSP_ENDPOINTS["verify_otp"],
json={"mobile": mobile, "otp": otp}, timeout=timeout)
return r.json() if r.content else {}
@@ -247,7 +247,7 @@ def verify_otp(mobile: str, otp: str, *, base: str = DSP_BASE,
def probe_public(base: str = DSP_BASE, *, session: Any = None,
timeout: int = 45) -> dict[str, Any]:
"""Report which endpoints answer without credentials. Reconnaissance only."""
- sess = session or make_session()
+ sess = session if session is not None else make_session()
out: dict[str, Any] = {"base": base, "results": {}, "note": None}
for path in ("/csv-download/years", "/csv-download", "/api/public/captcha"):
try:
diff --git a/commoner_probe/wayback.py b/commoner_probe/wayback.py
index 65509ed..a6ee298 100644
--- a/commoner_probe/wayback.py
+++ b/commoner_probe/wayback.py
@@ -87,7 +87,7 @@ def _latest_capture(url: str, *, session: Any, timeout: float) -> tuple[dict | N
load — observed live 2026-07-26 on back-to-back CDX calls). A re-check that
treats a 503 as "nothing to compare" silently stops detecting change.
"""
- session = session or make_session()
+ session = session if session is not None else make_session()
params = {"url": url, "output": "json", "limit": -1, "fl": _CDX_FIELDS}
try:
r = session.get(CDX_API, params=params, timeout=timeout)
@@ -129,7 +129,7 @@ def request_save(url: str, *, session: Any = None, timeout: float = SAVE_TIMEOUT
SPN2 queues work and anonymous callers are throttled. Confirm with
latest_capture() rather than trusting this return value.
"""
- session = session or make_session()
+ session = session if session is not None else make_session()
try:
r = session.get(f"{SAVE_BASE}{quote(url, safe=':/?&=#%')}", timeout=timeout)
r.raise_for_status()
@@ -165,7 +165,7 @@ def snapshot_fields(
Merge the result into the record; never gate acquisition on it.
"""
- session = session or make_session()
+ session = session if session is not None else make_session()
before, _ = _latest_capture(url, session=session, timeout=timeout) if save else (None, "")
saved = request_save(url, session=session) if save else True
capture, failure = _latest_capture(url, session=session, timeout=timeout)
@@ -390,7 +390,7 @@ def iter_captures(
caller that read a 503 as "never archived" would record an outage as a fact
about the source, which is the failure ``recheck()`` exists to prevent.
"""
- session = session or make_session()
+ session = session if session is not None else make_session()
resume: str | None = None
emitted = 0
seen_batches = 0
@@ -491,7 +491,7 @@ def __init__(
self.out_dir = Path(out_dir)
self.sleep = sleep
self.manifest = self.out_dir / "manifest.jsonl"
- 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)
def load_seen(self) -> set:
seen: set = set()
diff --git a/tests/test_review_findings_2026_08_17.py b/tests/test_review_findings_2026_08_17.py
index 377b00a..4598820 100644
--- a/tests/test_review_findings_2026_08_17.py
+++ b/tests/test_review_findings_2026_08_17.py
@@ -212,3 +212,22 @@ def test_the_default_client_is_rebuilt(self):
first = c.session
c.reset()
assert c.session is not first
+
+
+def test_a_falsey_injected_session_is_still_used():
+ """`session or factory()` discarded any session object whose truthiness is
+ false. A session with `__len__` is ordinary; silently replacing it hands
+ the caller a client they did not build."""
+ class _Falsey:
+ def __len__(self):
+ return 0
+
+ def get(self, url, **kw):
+ class R:
+ content = b""
+ return R()
+
+ mine = _Falsey()
+ c = aspnet_cascade.CascadeCrawler("https://x.gov.in/r.aspx",
+ {"a": "ctl00$a"}, session=mine)
+ assert c.session is mine
From 65c0a5431cc3053f4fb213b700e58e977a40a510 Mon Sep 17 00:00:00 2001
From: skishchampi <996985+skishchampi@users.noreply.github.com>
Date: Sun, 16 Aug 2026 23:50:34 -0400
Subject: [PATCH 06/10] fix: the factory is chosen by identity too
The last commit fixed the session and left the same idiom on the factory one
line above it. A factory is a callable OBJECT as often as a lambda, and an
object that defines __len__ can be falsey. `or` then dropped the caller's
factory and built the default client instead.
---
commoner_probe/aspnet_cascade.py | 15 ++++++++----
tests/test_review_findings_2026_08_17.py | 29 ++++++++++++++++++++++++
2 files changed, 40 insertions(+), 4 deletions(-)
diff --git a/commoner_probe/aspnet_cascade.py b/commoner_probe/aspnet_cascade.py
index 92b0a3b..6160dc6 100644
--- a/commoner_probe/aspnet_cascade.py
+++ b/commoner_probe/aspnet_cascade.py
@@ -70,10 +70,17 @@ class did not build that session, and discarding a caller's
"""
self.report_url = report_url
self.controls = controls
- self._factory = session_factory or (
- None if session is not None
- else lambda: make_session(rate_limit_sec=rate_limit_sec,
- user_agent=user_agent))
+ # `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.
diff --git a/tests/test_review_findings_2026_08_17.py b/tests/test_review_findings_2026_08_17.py
index 4598820..a57343c 100644
--- a/tests/test_review_findings_2026_08_17.py
+++ b/tests/test_review_findings_2026_08_17.py
@@ -231,3 +231,32 @@ class R:
c = aspnet_cascade.CascadeCrawler("https://x.gov.in/r.aspx",
{"a": "ctl00$a"}, session=mine)
assert c.session is mine
+
+
+def test_a_falsey_session_factory_is_still_used():
+ """A factory is often a callable object, not a lambda, and an object with
+ `__len__` can be falsey. `or` dropped it and built the default client."""
+ class _Sess:
+ def get(self, url, **kw):
+ class R:
+ content = b""
+ return R()
+
+ class _Factory:
+ def __init__(self):
+ self.built = []
+
+ def __len__(self):
+ return 0 # falsey, and still a perfectly good factory
+
+ def __call__(self):
+ s = _Sess()
+ self.built.append(s)
+ return s
+
+ factory = _Factory()
+ c = aspnet_cascade.CascadeCrawler("https://x.gov.in/r.aspx",
+ {"a": "ctl00$a"}, session_factory=factory)
+ c.reset()
+ assert len(factory.built) == 2, "the caller's factory built both sessions"
+ assert c.session is factory.built[-1]
From f0a1f69d7f978deff2977f0408373068dc9b72fe Mon Sep 17 00:00:00 2001
From: skishchampi <996985+skishchampi@users.noreply.github.com>
Date: Sun, 16 Aug 2026 23:54:30 -0400
Subject: [PATCH 07/10] fix: the verification pass must cover the whole region
it certifies
Shifting the box east and north offset the cell boundaries correctly. It also
left the western and southern strips of the region with no second pass, while
`saturated` was still reported for the whole region. A first-pass miss in those
strips was never examined.
The grid ORIGIN now moves backwards by half a cell instead. The boundaries
still fall between the first pass's query points, and every part of the region
lands inside some tile. A test asserts coverage at the four corners and the
centre.
Third defect in this method, and the third caught by review rather than by me.
Each fix was right about the case reported and wrong about its neighbour: large
boxes, then small boxes, then the leading strips.
---
commoner_probe/geoserver.py | 8 +++++++-
tests/test_geoserver.py | 27 +++++++++++++++++++++++++--
2 files changed, 32 insertions(+), 3 deletions(-)
diff --git a/commoner_probe/geoserver.py b/commoner_probe/geoserver.py
index c50939e..7f4a35e 100644
--- a/commoner_probe/geoserver.py
+++ b/commoner_probe/geoserver.py
@@ -347,7 +347,13 @@ def verify(self, layer: str, bbox: Sequence[float], known: Iterable[str], *,
# 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
- shifted = (west + step_x, south + step_y, east + step_x, north + step_y)
+ # 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 = {}
got = self.sweep(layer, shifted, start_span=start_span, key=key, status=status)
new = set(got) - known
diff --git a/tests/test_geoserver.py b/tests/test_geoserver.py
index 42c0cd4..6d975c6 100644
--- a/tests/test_geoserver.py
+++ b/tests/test_geoserver.py
@@ -186,8 +186,10 @@ def test_the_offset_grid_shifts_by_one_cell_not_by_the_whole_region():
gs.verify("ws:layer", (76.0, 12.0, 80.0, 16.0), known=[], start_span=2.0, key="code")
wests = sorted(float(unquote(re.search(r"bbox=([^&]+)", u, re.I).group(1)).split(",")[0])
for u in sess.urls)
- assert wests[0] == 77.0, f"expected a one-cell shift to 77.0, got {wests[0]}"
- assert wests[-1] == 79.0, f"the grid must not run past one cell beyond the box: {wests}"
+ # Half a cell (1.0), not half the region (2.0), and BACKWARDS so the
+ # western strip of the region still gets a second pass.
+ assert wests[0] == 75.0, f"expected a half-cell shift to 75.0, got {wests[0]}"
+ assert 76.0 not in wests, "an unshifted grid re-asks the first pass's questions"
def test_saturation_is_refused_when_a_verification_tile_failed():
@@ -224,3 +226,24 @@ def test_the_offset_grid_still_overlaps_a_box_smaller_than_one_cell():
assert boxes, "the verification pass made no request"
overlaps = [b for b in boxes if b[0] < 77.0 and b[2] > 76.0 and b[1] < 13.0 and b[3] > 12.0]
assert overlaps, f"no verification tile overlaps the original box: {boxes}"
+
+
+def test_every_part_of_the_box_gets_a_verification_tile():
+ """Saturation is claimed for the WHOLE region, so the second pass must
+ cover the whole region. Shifting east and north left the western and
+ southern strips untested, and a first-pass miss there was never examined."""
+ import re
+ from urllib.parse import unquote
+
+ sess = _Session([])
+ gs = GeoServer("http://x/geoserver", session=sess)
+ gs.verify("ws:layer", (76.0, 12.0, 80.0, 16.0), known=[], start_span=2.0, key="code")
+ boxes = [[float(x) for x in unquote(re.search(r"bbox=([^&]+)", u, re.I).group(1)).split(",")]
+ for u in sess.urls]
+
+ def covered(lon, lat):
+ return any(b[0] <= lon <= b[2] and b[1] <= lat <= b[3] for b in boxes)
+
+ for lon, lat in ((76.01, 12.01), (76.01, 15.99), (79.99, 12.01), (79.99, 15.99),
+ (78.0, 14.0)):
+ assert covered(lon, lat), f"({lon},{lat}) is in the box and no tile covers it"
From a5f9b3f9be166d82c1f89724d80f41b9e1174369 Mon Sep 17 00:00:00 2001
From: skishchampi <996985+skishchampi@users.noreply.github.com>
Date: Mon, 17 Aug 2026 00:01:38 -0400
Subject: [PATCH 08/10] fix: the verification pass counts only features inside
the region
The grid starts half a cell before the region, so it queries ground west and
south of it. A feature there was never this extraction's to find. Counting it
as a first-pass miss made a complete sweep report saturated=False.
Pass-2 features are now filtered to the requested bbox before `pass2` and `new`
are computed. The offset layout is unchanged, so coverage of the region stays.
A feature with no point geometry cannot be placed. It is KEPT and counted, and
the count is returned as `unlocatable`. Dropping it would hide a real miss.
Keeping it in silence would hide the doubt, so the number travels with the
verdict.
---
commoner_probe/geoserver.py | 36 +++++++++++++++++++++++++++++++++++-
tests/test_geoserver.py | 31 +++++++++++++++++++++++++++++++
2 files changed, 66 insertions(+), 1 deletion(-)
diff --git a/commoner_probe/geoserver.py b/commoner_probe/geoserver.py
index 7f4a35e..721239b 100644
--- a/commoner_probe/geoserver.py
+++ b/commoner_probe/geoserver.py
@@ -178,6 +178,29 @@ 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")
+ if geom.get("type") != "Point" or not isinstance(coords, (list, tuple)) or len(coords) < 2:
+ return None
+ lon, lat = coords[0], coords[1]
+ try:
+ return bbox[0] <= float(lon) <= bbox[2] and bbox[1] <= float(lat) <= bbox[3]
+ except (TypeError, ValueError):
+ return None
+
+
@dataclass
class GeoServer:
"""A WMS-only GeoServer, swept for point features.
@@ -355,7 +378,15 @@ def verify(self, layer: str, bbox: Sequence[float], known: Iterable[str], *,
# of it.
shifted = (west - step_x, south - step_y, east, north)
status: dict = {}
- got = self.sweep(layer, shifted, start_span=start_span, key=key, status=status)
+ 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:
@@ -373,6 +404,9 @@ def verify(self, layer: str, bbox: Sequence[float], known: Iterable[str], *,
# 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],
diff --git a/tests/test_geoserver.py b/tests/test_geoserver.py
index 6d975c6..833dfaa 100644
--- a/tests/test_geoserver.py
+++ b/tests/test_geoserver.py
@@ -247,3 +247,34 @@ def covered(lon, lat):
for lon, lat in ((76.01, 12.01), (76.01, 15.99), (79.99, 12.01), (79.99, 15.99),
(78.0, 14.0)):
assert covered(lon, lat), f"({lon},{lat}) is in the box and no tile covers it"
+
+
+def test_a_feature_outside_the_region_is_not_a_first_pass_miss():
+ """The backward shift queries ground west and south of the region. A
+ feature there is not something the first pass missed, and counting it as
+ one makes a complete extraction report `saturated=False`."""
+ outside = json.dumps({"features": [
+ {"id": "f.out", "properties": {"code": "OUT"},
+ "geometry": {"type": "Point", "coordinates": [75.5, 11.5]}}]})
+ inside = json.dumps({"features": [
+ {"id": "f.in", "properties": {"code": "IN"},
+ "geometry": {"type": "Point", "coordinates": [78.0, 14.0]}}]})
+ sess = _Session([outside] + [inside] * 8)
+ gs = GeoServer("http://x/geoserver", session=sess, feature_count=400)
+ out = gs.verify("ws:layer", (76.0, 12.0, 80.0, 16.0), known=["IN"],
+ start_span=2.0, key="code")
+ assert "OUT" not in out["new_ids"], "an out-of-region feature is not a miss"
+ assert out["saturated"] is True
+
+
+def test_a_feature_with_no_geometry_is_kept_and_counted_as_unlocatable():
+ """Some servers omit geometry. Dropping those would hide real misses, and
+ keeping them silently would hide the doubt. Keep, and report the count."""
+ nogeo = json.dumps({"features": [
+ {"id": "f.x", "properties": {"code": "X"}}]})
+ sess = _Session([nogeo] * 9)
+ gs = GeoServer("http://x/geoserver", session=sess, feature_count=400)
+ out = gs.verify("ws:layer", (76.0, 12.0, 80.0, 16.0), known=[],
+ start_span=2.0, key="code")
+ assert out["unlocatable"] >= 1
+ assert "X" in out["new_ids"]
From cbf9e6468023a397bde35252bf9bedeeb093f98b Mon Sep 17 00:00:00 2001
From: skishchampi <996985+skishchampi@users.noreply.github.com>
Date: Mon, 17 Aug 2026 00:06:32 -0400
Subject: [PATCH 09/10] fix: place MultiPoint features before judging the
region
A point layer serves MultiPoint as readily as Point. The strict type check made
those features unplaceable, so an out-of-region MultiPoint still counted as a
first-pass miss and a complete sweep still reported saturated=False.
One point inside now makes the feature the region's. A feature whose points all
lie outside is excluded. Any other geometry stays unplaceable: a polygon has no
single answer here, and guessing one would be the invented geometry this module
exists to refuse.
---
commoner_probe/geoserver.py | 30 ++++++++++++++++++++++++------
tests/test_geoserver.py | 24 ++++++++++++++++++++++++
2 files changed, 48 insertions(+), 6 deletions(-)
diff --git a/commoner_probe/geoserver.py b/commoner_probe/geoserver.py
index 721239b..46596e0 100644
--- a/commoner_probe/geoserver.py
+++ b/commoner_probe/geoserver.py
@@ -192,14 +192,32 @@ def _point_in(feature: dict, bbox: Sequence[float]) -> bool | None:
"""
geom = (feature or {}).get("geometry") or {}
coords = geom.get("coordinates")
- if geom.get("type") != "Point" or not isinstance(coords, (list, tuple)) or len(coords) < 2:
- return None
- lon, lat = coords[0], coords[1]
- try:
- return bbox[0] <= float(lon) <= bbox[2] and bbox[1] <= float(lat) <= bbox[3]
- except (TypeError, ValueError):
+ 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:
diff --git a/tests/test_geoserver.py b/tests/test_geoserver.py
index 833dfaa..6081225 100644
--- a/tests/test_geoserver.py
+++ b/tests/test_geoserver.py
@@ -278,3 +278,27 @@ def test_a_feature_with_no_geometry_is_kept_and_counted_as_unlocatable():
start_span=2.0, key="code")
assert out["unlocatable"] >= 1
assert "X" in out["new_ids"]
+
+
+def test_a_multipoint_feature_is_placed_like_a_point():
+ """A point layer can serve MultiPoint. Treating it as unplaceable let an
+ out-of-region feature count as a first-pass miss."""
+ from commoner_probe.geoserver import _point_in
+
+ bbox = (76.0, 12.0, 80.0, 16.0)
+ inside = {"geometry": {"type": "MultiPoint", "coordinates": [[78.0, 14.0]]}}
+ outside = {"geometry": {"type": "MultiPoint", "coordinates": [[75.5, 11.5]]}}
+ straddling = {"geometry": {"type": "MultiPoint",
+ "coordinates": [[75.5, 11.5], [78.0, 14.0]]}}
+ assert _point_in(inside, bbox) is True
+ assert _point_in(outside, bbox) is False
+ assert _point_in(straddling, bbox) is True, "one point inside makes it ours"
+
+
+def test_an_unknown_geometry_is_still_unplaceable():
+ """A polygon cannot be hit-tested into this answer. It stays unlocatable,
+ which is counted and reported rather than guessed."""
+ from commoner_probe.geoserver import _point_in
+
+ poly = {"geometry": {"type": "Polygon", "coordinates": [[[76.0, 12.0]]]}}
+ assert _point_in(poly, (76.0, 12.0, 80.0, 16.0)) is None
From 5738ed2735ba2f696263233cd2288487a6773bd6 Mon Sep 17 00:00:00 2001
From: skishchampi <996985+skishchampi@users.noreply.github.com>
Date: Mon, 17 Aug 2026 00:11:21 -0400
Subject: [PATCH 10/10] docs: fetch() says which 403 raises
The geo-fence raise landed in an earlier commit here, and this docstring still
told callers that every 403 means an absent period, and that they should not
try to tell them apart. It now names the one case that raises and why.
---
commoner_probe/cdn_dashboard.py | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/commoner_probe/cdn_dashboard.py b/commoner_probe/cdn_dashboard.py
index 0cad0de..e7573ee 100644
--- a/commoner_probe/cdn_dashboard.py
+++ b/commoner_probe/cdn_dashboard.py
@@ -322,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