diff --git a/CHANGELOG.md b/CHANGELOG.md index 47581a8..9148e1a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,39 @@ Format: [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this project a ## [Unreleased] +### Added, a survey command that inventories a retrieval's candidate markers + +- **`python -m perimeter.survey` (`make survey`).** A refresh began from a crash. The + build refuses a cell holding something that reads like a missing-data marker in a field + that has not declared it, which is right, and which reports exactly one cell: a new + retrieval was worked through one refusal at a time. The survey reads the same files the + build reads and reports every candidate at once, per measured field, with counts. + + It **declares nothing**. It writes no schema, edits no registry, and no part of the + build reads its output. Every candidate carries `basis: unreviewed`, in `survey.json` + and in the `survey.md` draft written in `docs/MARKERS.md`'s shape, so it cannot be + mistaken for a decision somebody made. The intended sequence is survey, review, declare, + build, diff, and the reviewing step is a person's. + + **The survey and the gate read one list.** The sentinel decision moved out of + `FieldSpec.classify` into `FieldSpec.undeclared_marker`, which both now call. A survey + carrying its own copy of the rule would go quiet at exactly the moment the rule moved, + and a test holds the two readers to the same verdict over every value in the sentinel + vocabulary, for every field in both registries. + + Three distinctions it keeps, being the same three the artifacts keep. A field above the + listing bound publishes its distinct count and says the listing was withheld, never an + empty list. A field with no published domain publishes `null` for values outside one, + never `0`, per ADR-0010. A numeric field in which nothing parsed as a number has no zero + share, not a zero one. + + A zero the registry has already ruled on is reported and not flagged. `YEARBUILT` + declares the literal `0` for the 12,148 parcel records carrying no year, and re-asking + that question on every run is how a review aid stops being read. + +- **`SentinelDriftError` now names the survey command** in its message, so the first + refusal of a refresh points at the tool that lists the rest. + ### Fixed, a distribution name that could never have been released - **`pyproject.toml` declared `name = "perimeter"`, which is somebody else's package.** diff --git a/Makefile b/Makefile index 7fb4bff..c5b3f72 100644 --- a/Makefile +++ b/Makefile @@ -183,3 +183,26 @@ determinism: # Network. Run by hand, never from a build. See PROVENANCE.md. acquire: uv run python -m perimeter.acquire --out data/raw + +# Inventory the candidate markers in an acquired retrieval, before a build refuses one. +# +# Offline, and deliberately NOT a prerequisite of anything. It declares nothing, writes no +# schema, and no part of the build reads its output: it is a review aid for the person +# doing a refresh, and a target that ran it automatically would turn a list of open +# questions into something a build appears to have answered. +# +# It is not part of `verify` for the same reason `acquire` is not: it reads data/raw/, +# which is never in git and never in CI. Point it at the fixtures to see the shape: +# +# make survey SURVEY_DINS=fixtures/dins_postfire.sample.json \ +# SURVEY_PERIMETERS=fixtures/frap_perimeters.sample.json +# +SURVEY_DINS ?= data/raw/dins_postfire.json +SURVEY_PERIMETERS ?= data/raw/frap_perimeters.json +SURVEY_OUT ?= build/survey + +survey: + uv run python -m perimeter.survey \ + --perimeters $(SURVEY_PERIMETERS) \ + --dins $(SURVEY_DINS) \ + --out $(SURVEY_OUT) diff --git a/README.md b/README.md index 18ee25a..35b0140 100644 --- a/README.md +++ b/README.md @@ -163,6 +163,42 @@ make site-offline # build from committed fixtures; runs anywhere, no network make diff OLD=a.json NEW=b.json # compare two coverage artifacts leaf by leaf ``` +### Before a refresh: what the new file holds + +A refresh used to begin from a crash. The build refuses a cell holding something that +reads like a missing-data marker in a field nobody has reviewed for it, which is the right +behaviour and a poor way to find out: it reports one cell, in one field, and says nothing +about the next one, so a new retrieval gets worked through one refusal at a time. + +`make survey` reads the same files the build reads and reports every candidate at once: + +```sh +make survey # over data/raw/, after make acquire +make survey SURVEY_DINS=fixtures/dins_postfire.sample.json \ + SURVEY_PERIMETERS=fixtures/frap_perimeters.sample.json +``` + +Per measured field it writes the distinct values with counts, the values that read like a +missing-data marker the field has not declared, the values outside a published domain, and +for a numeric field how many of its records hold a zero. Output is `survey.json` and a +`survey.md` draft in `docs/MARKERS.md`'s shape, where every entry reads `basis: +unreviewed`. + +It declares nothing. It writes no schema, edits no registry, and no part of the build +reads it. The sequence is survey, review, declare, build, diff, and the reviewing step is +a person's. + +Three things it will not round off, because they are the same three the artifacts refuse +to round off: + +- A field with more distinct values than the listing bound reports **the bound and its + distinct count**, never an empty list. A reader must be able to tell "this field holds + nothing" from "this field holds too much to show". +- A field with **no published domain** reports `null` for values outside one, not `0`. + There was no comparison to have a result. +- A numeric field in which nothing parsed as a number has **no zero share**, not a zero + one. + ### What a refresh moved The figures on these pages move only when the pinned retrievals are deliberately @@ -288,6 +324,7 @@ here is how much of each published field is actually filled in, and what the bla | `src/perimeter/artifacts.py` | Deterministic JSON | | `src/perimeter/render.py` | The static pages | | `src/perimeter/acquire.py` | The only code that touches the network. Run by hand, never in CI | +| `src/perimeter/survey.py` | Inventory a retrieval's candidate markers before a build refuses one. Declares nothing | | `tools/a11y.mjs` | axe-core over the built pages in a headless DOM; an undecided rule is not a pass | | `tools/determinism.sh` | Compare two build trees; refuse an empty or missing one | | `site/` | The built pages and their JSON artifacts. Tracked in git, and published by `.github/workflows/pages.yml` | diff --git a/src/perimeter/schema.py b/src/perimeter/schema.py index e655883..e3bd6ca 100644 --- a/src/perimeter/schema.py +++ b/src/perimeter/schema.py @@ -124,6 +124,29 @@ def declares_vocabulary(self) -> bool: self.unknown_codes or self.unknown_markers or self.recorded_absences ) + def undeclared_marker(self, text: str) -> str | None: + """The normalized marker ``text`` would stop the build with, or ``None``. + + This is the sentinel decision itself, lifted out of :meth:`classify` so that the + gate and :mod:`perimeter.survey` read one list rather than two that can drift. + The survey exists to find these before a build refuses one, and a survey working + from its own copy of the rule would go quiet at exactly the moment the rule moved. + + ``text`` is expected already stripped, as :meth:`classify` hands it over. The + order below is the order that matters: a published domain code and a published + recorded absence are checked first, so a field that legitimately publishes + ``None`` or ``N/A`` as a finding keeps it as a finding and is not reported as a + candidate. + """ + if text in self.unknown_codes or text in self.recorded_absences: + return None + marker = normalize_marker(text) + if marker in self.unknown_markers: + return None + if marker in SUSPECTED_SENTINELS: + return marker + return None + def classify(self, raw: object, *, where: str) -> Cell: """Sort one cell into present, explicit-unknown, or not-recorded. @@ -143,13 +166,16 @@ def classify(self, raw: object, *, where: str) -> Cell: marker = normalize_marker(text) if marker in self.unknown_markers: return Cell.explicit_unknown(marker) - if marker in SUSPECTED_SENTINELS: + if self.undeclared_marker(text) is not None: raise SentinelDriftError( f"{where}.{self.name}: cell {text!r} reads as a missing-data marker but " f"is not one this project has reviewed for this field " f"(reviewed markers: {sorted(self.unknown_markers | self.unknown_codes)}; " f"published absences: {sorted(self.recorded_absences)}); " - "guessing here would publish an absence as a value" + "guessing here would publish an absence as a value. " + "`python -m perimeter.survey` inventories every candidate in a " + "retrieval at once, so a refresh does not have to be driven one " + "refusal at a time" ) return self._present(text, where=where) diff --git a/src/perimeter/survey.py b/src/perimeter/survey.py new file mode 100644 index 0000000..b032766 --- /dev/null +++ b/src/perimeter/survey.py @@ -0,0 +1,574 @@ +"""Inventory the candidate markers in a retrieval, before a build refuses one. + +A refresh currently begins from a crash. :class:`~perimeter.cells.SentinelDriftError` +fails the build the first time a field holds something that reads like a missing-data +marker nobody has reviewed for that field, which is the right behaviour and a poor way to +find out: it reports one cell, in one field, and says nothing about the next one. Working +through a new retrieval that way means running the build, reading a refusal, reviewing a +field, and running it again, once per undeclared marker in the file. + +This reads the same files the build reads and reports every candidate at once. It +declares nothing. It writes no schema, edits no registry, and touches neither artifact: +its output is a review aid, and every candidate it finds carries ``basis: unreviewed`` +so it cannot be mistaken for a decision somebody made. + +The intended sequence for a refresh is survey, review, declare, build, diff. + +Four rules govern what it prints, and they are the same rules the rest of this project +publishes under. + +* **The sentinel decision is not re-implemented here.** Every candidate is found through + :meth:`~perimeter.schema.FieldSpec.undeclared_marker`, which is what the build's gate + itself calls. A survey with its own copy of the rule would go quiet at exactly the + moment the rule moved. +* **A bound that hides values says so.** A field with more distinct values than the + listing bound reports its distinct count and ``values_listed: false``. It does not + report an empty list, which would read as a field holding nothing. +* **No published domain is not zero values outside one.** ``outside_published_domain`` is + ``null`` for a field that publishes no domain, and a list (possibly empty) for a field + that does. The distinction is ADR-0010's and it is the reason this file never writes + ``0`` for a comparison that did not happen. +* **A share over nothing does not exist.** The zero share of a numeric field with no + parsable numbers is ``null``, not ``0``. + +It is offline, reads only the files it is pointed at, and writes byte-identical output +for byte-identical input. +""" + +from __future__ import annotations + +import argparse +import json +from collections.abc import Mapping, Sequence +from dataclasses import dataclass +from pathlib import Path + +from perimeter.cells import normalize_marker, present_tenths_of_percent +from perimeter.records import load_rows +from perimeter.schema import ( + DINS_FIELDS, + DINS_REQUIRED_COLUMNS, + FRAP_FIELDS, + FRAP_REQUIRED_COLUMNS, + FieldSpec, + SchemaDriftError, + require_columns, +) + +LISTING_BOUND = 25 +"""How many distinct values a field may hold before the survey stops listing them. + +A free-text column holds tens of thousands of distinct values and listing them is not a +review aid, it is the file again. Above this bound the survey reports the distinct count, +the marker candidates, and the out-of-domain values, and says in the artifact that the +full listing was withheld. It never substitutes an empty list for the listing it declined +to print: a reader must be able to tell "this field holds nothing" from "this field holds +too much to show". +""" + +ZERO_SHARE_REPORTING_FLOOR_TENTHS_PCT = 10 +"""The share of recorded zeros, in tenths of a percent, worth a reviewer's attention. + +One percent. ADR-0006 is the reason this is here at all: a numeric field whose most +common value is zero may be recording a measurement of zero or a parcel record carrying +no number, and ``YEARBUILT`` was the second for 12,148 records. The floor is a reporting +threshold and nothing more. Nothing is declared, excluded, or classified by it, and the +zero count itself is published for every numeric field whatever the share. +""" + + +@dataclass(frozen=True) +class ValueCount: + """One distinct value as it arrived, and how many rows carried it.""" + + value: str + count: int + + def as_json(self) -> dict[str, object]: + return {"value": self.value, "count": self.count} + + +@dataclass(frozen=True) +class MarkerCandidate: + """A value that reads like a missing-data marker, and whether anyone reviewed it.""" + + value: str + normalized: str + count: int + + def as_json(self) -> dict[str, object]: + return { + "value": self.value, + "normalized": self.normalized, + "count": self.count, + # Every candidate in this list is by construction one the registry has not + # declared for this field: `undeclared_marker` returns None for a reviewed + # one. The key is written out anyway, and always with this value, so that a + # reader of survey.json does not have to know that to read it, and so that a + # future survey which does report reviewed markers has somewhere to put them. + "basis": "unreviewed", + } + + +@dataclass(frozen=True) +class ZeroReading: + """How often a numeric field recorded a zero, and out of how many numbers. + + Split by whether the registry has already reviewed that zero. ``YEARBUILT`` declares + the literal ``0`` as a marker, because 12,148 records carry a parcel with no year and + no structure in a California wildfire was built in year 0. Reporting that field again + as an open question every time somebody surveys a retrieval would train a reviewer to + skip the section, which is how a review aid stops being read. + """ + + zeros: int + """Rows whose value parses to zero, declared and undeclared together.""" + + declared_zeros: int + """Of those, the ones the registry already treats as a marker or absence here.""" + + numbers: int + + @property + def undeclared_zeros(self) -> int: + return self.zeros - self.declared_zeros + + @property + def tenths_pct(self) -> int | None: + """Zeros as a share of parsable numbers, or ``None`` when there were none. + + A field in which nothing parsed as a number has no zero share. It is not zero + percent, and publishing it as zero percent would say the field recorded numbers + and none of them were zero. + """ + return present_tenths_of_percent(self.zeros, self.numbers) + + @property + def undeclared_tenths_pct(self) -> int | None: + return present_tenths_of_percent(self.undeclared_zeros, self.numbers) + + @property + def worth_reviewing(self) -> bool: + """True when enough zeros nobody has ruled on sit in this field to ask about. + + Measured on the undeclared share, so a field whose zero has been reviewed is + reported and not flagged, and a field with a mix is flagged for the part that + is still a question. + """ + share = self.undeclared_tenths_pct + return share is not None and share >= ZERO_SHARE_REPORTING_FLOOR_TENTHS_PCT + + def as_json(self) -> dict[str, object]: + return { + "zeros": self.zeros, + "declared_zeros": self.declared_zeros, + "undeclared_zeros": self.undeclared_zeros, + "numbers": self.numbers, + "tenths_pct": self.tenths_pct, + "undeclared_tenths_pct": self.undeclared_tenths_pct, + "worth_reviewing": self.worth_reviewing, + } + + +@dataclass(frozen=True) +class FieldSurvey: + """What one measured column holds in this retrieval, and what needs a person.""" + + name: str + label: str + rows: int + empty: int + """Rows where the column was absent, null, or blank after stripping.""" + + distinct: int + values: tuple[ValueCount, ...] | None + """Every distinct value with its count, or ``None`` when above the listing bound.""" + + candidates: tuple[MarkerCandidate, ...] + outside_domain: tuple[ValueCount, ...] | None + """Values not in the published domain, or ``None`` where no domain is published.""" + + zeros: ZeroReading | None + """Only for a field declared ``numeric``; ``None`` otherwise.""" + + @property + def needs_review(self) -> bool: + """True when this field has anything a person should look at.""" + return bool(self.candidates) or ( + self.zeros is not None and self.zeros.worth_reviewing + ) + + def as_json(self) -> dict[str, object]: + payload: dict[str, object] = { + "field": self.name, + "label": self.label, + "rows": self.rows, + "empty": self.empty, + "distinct": self.distinct, + "values_listed": self.values is not None, + "candidates": [candidate.as_json() for candidate in self.candidates], + "outside_published_domain": ( + None + if self.outside_domain is None + else [item.as_json() for item in self.outside_domain] + ), + "needs_review": self.needs_review, + } + if self.values is None: + # Not an empty list. The listing was withheld and the artifact says which of + # the two happened, because a reader cannot tell them apart from `[]`. + payload["values_withheld_because"] = ( + f"the field holds {self.distinct} distinct values, " + f"more than the listing bound of {LISTING_BOUND}" + ) + else: + payload["values"] = [item.as_json() for item in self.values] + payload["recorded_zeros"] = None if self.zeros is None else self.zeros.as_json() + return payload + + +@dataclass(frozen=True) +class SourceSurvey: + """One source file, surveyed.""" + + source: str + rows: int + fields: tuple[FieldSurvey, ...] + + @property + def needs_review(self) -> tuple[FieldSurvey, ...]: + return tuple(field for field in self.fields if field.needs_review) + + def as_json(self) -> dict[str, object]: + return { + "source": self.source, + "rows": self.rows, + "fields_measured": len(self.fields), + "fields_needing_review": len(self.needs_review), + "fields": [field.as_json() for field in self.fields], + } + + +def _cell_text(row: Mapping[str, object], name: str) -> str: + """The cell as the build sees it: stripped, with absent and null both empty.""" + raw = row.get(name) + if raw is None: + return "" + return str(raw).strip() + + +def _is_number(text: str) -> bool: + try: + float(text) + except ValueError: + return False + return True + + +def _is_unknown(spec: FieldSpec, text: str) -> bool: + """True when the build would classify this value as explicit-unknown, not present.""" + return text in spec.unknown_codes or normalize_marker(text) in spec.unknown_markers + + +def _is_declared(spec: FieldSpec, text: str) -> bool: + """True when the registry has already reviewed this exact value for this field.""" + return _is_unknown(spec, text) or text in spec.recorded_absences + + +def survey_field( + rows: Sequence[Mapping[str, object]], + spec: FieldSpec, + *, + listing_bound: int = LISTING_BOUND, +) -> FieldSurvey: + """Count what one column holds, without deciding anything about it.""" + counts: dict[str, int] = {} + empty = 0 + for row in rows: + text = _cell_text(row, spec.name) + if text == "": + empty += 1 + continue + counts[text] = counts.get(text, 0) + 1 + + ordered = tuple( + ValueCount(value, counts[value]) for value in sorted(counts, key=_value_order) + ) + candidates = tuple( + MarkerCandidate(item.value, marker, item.count) + for item in ordered + if (marker := spec.undeclared_marker(item.value)) is not None + ) + outside: tuple[ValueCount, ...] | None + if spec.domain_values is None: + # ADR-0010. A field with no published domain has no values outside one; it does + # not have zero of them. `null` is the honest answer and `[]` is a false one. + outside = None + else: + # The population is the one `FieldSpec.outside_domain` asks about: values the + # build would classify as *present*. A declared unknown marker or code is + # explicit-unknown rather than a present value outside the domain, and an + # undeclared marker is reported above as a candidate rather than counted here, + # because it would stop the build before any domain comparison happened. + outside = tuple( + item + for item in ordered + if item.value not in spec.domain_values + and not _is_unknown(spec, item.value) + and spec.undeclared_marker(item.value) is None + ) + + zeros: ZeroReading | None = None + if spec.numeric: + numeric_items = [item for item in ordered if _is_number(item.value)] + zero_items = [item for item in numeric_items if float(item.value) == 0.0] + zeros = ZeroReading( + zeros=sum(item.count for item in zero_items), + declared_zeros=sum( + item.count for item in zero_items if _is_declared(spec, item.value) + ), + numbers=sum(item.count for item in numeric_items), + ) + + return FieldSurvey( + name=spec.name, + label=spec.label, + rows=len(rows), + empty=empty, + distinct=len(ordered), + values=ordered if len(ordered) <= listing_bound else None, + candidates=candidates, + outside_domain=outside, + zeros=zeros, + ) + + +def _value_order(value: str) -> tuple[int, float, str]: + """Numbers in numeric order, then everything else in code-point order. + + Sorting `"10"` before `"9"` in a listing a person reads to review a numeric field is + a small thing that makes the listing harder to read than the file. Text values keep + code-point order, which is stable across machines and locales; `sorted` is called on + the same key everywhere so the output is byte-identical run to run. + """ + if _is_number(value): + return (0, float(value), value) + return (1, 0.0, value) + + +def survey_rows( + rows: Sequence[Mapping[str, object]], + *, + specs: Sequence[FieldSpec], + required: tuple[str, ...], + source: str, + listing_bound: int = LISTING_BOUND, +) -> SourceSurvey: + """Survey every measured column of a source's rows. + + A missing column still refuses. The survey is a review aid for markers, not a way + around :class:`~perimeter.schema.SchemaDriftError`: a column that vanished upstream + would otherwise be surveyed as a column every row left blank, which is the reading + this project exists to refuse. It checks every row, as the parser does, because a + column that goes missing part way down a file is intact in the first row. + """ + required_columns = (*required, *(spec.name for spec in specs)) + needed = frozenset(required_columns) + for index, row in enumerate(rows): + if not needed <= row.keys(): + require_columns(set(row), required_columns, source=f"{source} row {index}") + return SourceSurvey( + source=source, + rows=len(rows), + fields=tuple( + survey_field(rows, spec, listing_bound=listing_bound) for spec in specs + ), + ) + + +REGISTRY_BY_SOURCE: dict[str, tuple[tuple[FieldSpec, ...], tuple[str, ...]]] = { + "perimeters": (FRAP_FIELDS, FRAP_REQUIRED_COLUMNS), + "dins": (DINS_FIELDS, DINS_REQUIRED_COLUMNS), +} +"""The same two registries the build measures with, keyed by the flag that selects them. + +Read from :mod:`perimeter.schema` rather than restated, so a field added to a registry is +surveyed without anyone remembering to add it here. +""" + + +def survey_file( + path: Path, *, source: str, listing_bound: int = LISTING_BOUND +) -> SourceSurvey: + """Read one acquired file and survey it.""" + try: + specs, required = REGISTRY_BY_SOURCE[source] + except KeyError: + raise SchemaDriftError( + f"no field registry for source {source!r}; this project measures " + f"{sorted(REGISTRY_BY_SOURCE)}" + ) from None + return survey_rows( + load_rows(path), + specs=specs, + required=required, + source=source, + listing_bound=listing_bound, + ) + + +def as_json(surveys: Sequence[SourceSurvey]) -> dict[str, object]: + """The survey artifact: what was read, and what needs a person.""" + return { + "listing_bound": LISTING_BOUND, + "zero_share_reporting_floor_tenths_pct": ZERO_SHARE_REPORTING_FLOOR_TENTHS_PCT, + # Said in the artifact and not only in this docstring, because the file outlives + # the terminal it was printed in and its first reader may not be the person who + # ran it. + "declares_nothing": ( + "Every candidate below is unreviewed. This file records what a retrieval " + "holds; it decides nothing, and no part of the build reads it." + ), + "sources": [survey.as_json() for survey in surveys], + } + + +def markdown(surveys: Sequence[SourceSurvey]) -> str: + """A draft review section per field with a candidate, in docs/MARKERS.md's shape. + + Every entry is a question, not an entry. The basis line reads ``unreviewed`` and the + evidence line is the count this survey measured, so the person editing it is adding + the judgment rather than confirming one the tool already made. + """ + lines = [ + "# Survey draft: candidate markers awaiting review", + "", + "Generated by `python -m perimeter.survey`. Nothing here is declared. Each entry", + "below is a value that reads like a missing-data marker in a field that has not", + "reviewed it, or a numeric field whose recorded zeros are common enough to be", + "worth a look. Editing an entry into `docs/MARKERS.md` and `src/perimeter/", + "schema.py` is the reviewing step, and it is a person's.", + "", + ] + for survey in surveys: + lines.append(f"## {survey.source}") + lines.append("") + needing = survey.needs_review + if not needing: + lines.append( + f"No candidates. Every value in all {len(survey.fields)} measured " + f"fields of {survey.rows} rows is either a reviewed marker, a published " + "recorded absence, or an ordinary value." + ) + lines.append("") + continue + for field in needing: + lines.append(f"### `{field.name}` ({field.label})") + lines.append("") + lines.append("- basis: unreviewed") + lines.append(f"- rows: {field.rows}; empty: {field.empty}") + for candidate in field.candidates: + lines.append( + f"- candidate marker: `{candidate.value}` " + f"(normalizes to `{candidate.normalized}`), " + f"{candidate.count} rows. What does the publisher mean by it in " + "this field, and is it a marker or a finding?" + ) + if field.zeros is not None and field.zeros.worth_reviewing: + share = field.zeros.undeclared_tenths_pct + assert share is not None # noqa: S101 - worth_reviewing implies it + lines.append( + f"- undeclared zeros: {field.zeros.undeclared_zeros} of " + f"{field.zeros.numbers} numbers ({share / 10:.1f}%). Is a zero here " + "a measurement of zero, or a record carrying no number?" + ) + lines.append("") + return "\n".join(lines).rstrip() + "\n" + + +def console(surveys: Sequence[SourceSurvey]) -> str: + """What a person at a terminal reads: one line per field that needs review.""" + lines: list[str] = [] + for survey in surveys: + needing = survey.needs_review + lines.append( + f"{survey.source}: {survey.rows} rows, {len(survey.fields)} fields " + f"measured, {len(needing)} needing review" + ) + for field in needing: + for candidate in field.candidates: + lines.append( + f" {field.name}: candidate marker {candidate.value!r} " + f"x{candidate.count} (unreviewed)" + ) + if field.zeros is not None and field.zeros.worth_reviewing: + share = field.zeros.undeclared_tenths_pct + assert share is not None # noqa: S101 - worth_reviewing implies it + lines.append( + f" {field.name}: {field.zeros.undeclared_zeros} undeclared zeros " + f"of {field.zeros.numbers} numbers ({share / 10:.1f}%) (unreviewed)" + ) + if not needing: + lines.append(" no candidates") + return "\n".join(lines) + + +def write(surveys: Sequence[SourceSurvey], out_dir: Path) -> list[Path]: + """Write ``survey.json`` and ``survey.md`` into ``out_dir``.""" + out_dir.mkdir(parents=True, exist_ok=True) + artifact = out_dir / "survey.json" + artifact.write_text( + json.dumps(as_json(surveys), indent=2, sort_keys=True) + "\n", encoding="utf-8" + ) + draft = out_dir / "survey.md" + draft.write_text(markdown(surveys), encoding="utf-8") + return [artifact, draft] + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser( + prog="perimeter.survey", + description=( + "Inventory the candidate markers in an acquired file. Declares nothing, " + "edits no registry, and is read by no part of the build." + ), + ) + parser.add_argument( + "--perimeters", type=Path, help="attribute rows for the FRAP layer" + ) + parser.add_argument("--dins", type=Path, help="attribute rows for the DINS layer") + parser.add_argument( + "--out", + type=Path, + default=Path("build/survey"), + help="directory for survey.json and survey.md (default: build/survey)", + ) + parser.add_argument( + "--listing-bound", + type=int, + default=LISTING_BOUND, + help=( + "how many distinct values a field may hold before the full listing is " + f"withheld (default: {LISTING_BOUND})" + ), + ) + args = parser.parse_args(argv) + requested = [ + (name, path) + for name, path in (("perimeters", args.perimeters), ("dins", args.dins)) + if path is not None + ] + if not requested: + parser.error("give at least one of --perimeters or --dins") + surveys = [ + survey_file(path, source=name, listing_bound=args.listing_bound) + for name, path in requested + ] + print(console(surveys)) + for path in write(surveys, args.out): + print(f"wrote {path}") + return 0 + + +if __name__ == "__main__": # pragma: no cover + raise SystemExit(main()) diff --git a/tests/test_survey.py b/tests/test_survey.py new file mode 100644 index 0000000..c750346 --- /dev/null +++ b/tests/test_survey.py @@ -0,0 +1,502 @@ +"""The survey reports what a retrieval holds, and decides nothing about it. + +Two properties carry most of the weight here. + +The first is that the survey and the build read **one list**. The build refuses an +undeclared missing-data marker; the survey exists so a person can see every one of them +before the build refuses the first. If the survey had its own copy of the rule, it would +go quiet exactly when the rule moved, and a refresh would still be driven one crash at a +time. `test_the_survey_finds_exactly_what_the_build_refuses` holds them together over +every value the fixtures contain plus a planted set. + +The second is that nothing the survey withholds is published as nothing. A field above +the listing bound reports the bound, not an empty list. A field with no published domain +reports `null`, not zero values outside one. A numeric field with no numbers has no zero +share, not a zero one. Those are the same three distinctions the artifacts already make, +and this module is a third place they can be lost. +""" + +from __future__ import annotations + +import json +import os +import subprocess +import sys +from collections.abc import Mapping +from pathlib import Path +from typing import Any + +import pytest + +from perimeter.cells import SUSPECTED_SENTINELS, SentinelDriftError, normalize_marker +from perimeter.records import load_rows +from perimeter.schema import ( + DINS_FIELDS, + DINS_FIELDS_BY_NAME, + DINS_REQUIRED_COLUMNS, + FRAP_FIELDS, + FieldSpec, + SchemaDriftError, +) +from perimeter.survey import ( + LISTING_BOUND, + ZERO_SHARE_REPORTING_FLOOR_TENTHS_PCT, + FieldSurvey, + SourceSurvey, + ZeroReading, + console, + main, + markdown, + survey_field, + survey_file, + survey_rows, + write, +) + +ROOT = Path(__file__).resolve().parents[1] +DINS_FIXTURE = ROOT / "fixtures" / "dins_postfire.sample.json" +FRAP_FIXTURE = ROOT / "fixtures" / "frap_perimeters.sample.json" + + +def dins_rows() -> list[dict[str, object]]: + return load_rows(DINS_FIXTURE) + + +def surveyed(rows: list[Mapping[str, object]]) -> SourceSurvey: + return survey_rows( + rows, specs=DINS_FIELDS, required=DINS_REQUIRED_COLUMNS, source="dins" + ) + + +def field_of(survey: SourceSurvey, name: str) -> FieldSurvey: + for field in survey.fields: + if field.name == name: + return field + raise AssertionError(f"{name} was not surveyed") + + +# -------------------------------------------------------------------------------------- +# One list, shared with the gate +# -------------------------------------------------------------------------------------- + + +def test_the_survey_finds_exactly_what_the_build_refuses() -> None: + """A candidate is reported if and only if `classify` would stop the build on it. + + The planted values are drawn from the sentinel vocabulary and from **each field's own + declared values in their published casing**. That second half is what makes this a + test rather than a formality, and it was found the hard way: with only the sentinel + vocabulary planted, a control that reordered `classify` so the sentinel net ran before + the recorded-absence check stayed green. `SUSPECTED_SENTINELS` is lowercase and + `recorded_absences` holds the publisher's casing, so `'None'`, the published street + type, was never probed. Only the exact-cased declarations reach the branch where the + order matters. + """ + vocabulary = sorted(SUSPECTED_SENTINELS) + for spec in (*DINS_FIELDS, *FRAP_FIELDS): + planted = [ + *vocabulary, + *sorted(spec.recorded_absences), + *sorted(spec.unknown_codes), + *sorted(spec.unknown_markers), + "Road", + "No Damage", + "0", + "1975", + "Butte", + ] + for text in planted: + refused = False + try: + spec.classify(text, where="probe") + except SentinelDriftError: + refused = True + except SchemaDriftError: + # A numeric field refusing a non-number is a different rule, and the + # survey is not claiming to mirror it. + continue + assert (spec.undeclared_marker(text) is not None) is refused, ( + f"{spec.name}: undeclared_marker and classify disagree about {text!r}" + ) + + +def test_the_sentinel_refusal_names_the_survey_command() -> None: + spec = DINS_FIELDS_BY_NAME["STREETTYPE"] + with pytest.raises(SentinelDriftError) as caught: + spec.classify("NA", where="probe") + assert "perimeter.survey" in str(caught.value) + + +# -------------------------------------------------------------------------------------- +# Candidates +# -------------------------------------------------------------------------------------- + + +def test_a_marker_the_field_has_not_reviewed_is_a_candidate_with_its_count() -> None: + """`STREETTYPE` declares `n/a`, `unk` and `-`; it has never declared `na`. + + That non-overlap is the case docs/MARKERS.md records for the `NA` and `N/A` + spellings, and it is what a refresh hits first. + """ + rows = dins_rows() + rows[0]["STREETTYPE"] = "NA" + rows[1]["STREETTYPE"] = "NA" + field = field_of(surveyed(rows), "STREETTYPE") + assert [(c.value, c.normalized, c.count) for c in field.candidates] == [ + ("NA", "na", 2) + ] + assert field.needs_review is True + assert all(c.as_json()["basis"] == "unreviewed" for c in field.candidates) + + +def test_a_marker_the_field_has_reviewed_is_not_a_candidate() -> None: + rows = dins_rows() + rows[0]["CITY"] = "N/A" # CITY declares n/a, na, none and unknown + assert field_of(surveyed(rows), "CITY").candidates == () + + +def test_a_published_recorded_absence_is_not_a_candidate() -> None: + """`None` is a published street type. It is a finding, not an absent value.""" + rows = dins_rows() + for row in rows: + row["STREETTYPE"] = "None" + assert field_of(surveyed(rows), "STREETTYPE").candidates == () + + +def test_a_field_whose_values_are_all_in_its_published_domain_has_no_candidates() -> ( + None +): + field = field_of(surveyed(dins_rows()), "DAMAGE") + assert field.candidates == () + assert field.outside_domain == () + assert field.needs_review is False + + +def test_the_console_says_no_candidates_rather_than_printing_nothing() -> None: + survey = survey_rows( + load_rows(FRAP_FIXTURE), + specs=FRAP_FIELDS, + required=(), + source="perimeters", + ) + assert survey.needs_review == () + assert "no candidates" in console([survey]) + + +# -------------------------------------------------------------------------------------- +# Nothing withheld is published as nothing +# -------------------------------------------------------------------------------------- + + +def test_no_published_domain_is_null_and_not_an_empty_list() -> None: + """ADR-0010, in the survey. `[]` would say the comparison ran and found nothing.""" + survey = surveyed(dins_rows()) + free_text = field_of(survey, "SITEADDRESS") + assert DINS_FIELDS_BY_NAME["SITEADDRESS"].domain_values is None + assert free_text.outside_domain is None + assert free_text.as_json()["outside_published_domain"] is None + + constrained = field_of(survey, "DAMAGE") + assert constrained.as_json()["outside_published_domain"] == [] + + +def test_a_value_outside_a_published_domain_is_listed_with_its_count() -> None: + rows = dins_rows() + rows[0]["DAMAGE"] = "Scorched" + field = field_of(surveyed(rows), "DAMAGE") + assert [(v.value, v.count) for v in field.outside_domain or ()] == [("Scorched", 1)] + + +def test_a_field_above_the_listing_bound_says_so_instead_of_listing_nothing() -> None: + spec = FieldSpec("F", "Field") + rows: list[Mapping[str, object]] = [ + {"F": f"value {index}"} for index in range(LISTING_BOUND + 3) + ] + field = survey_field(rows, spec) + assert field.distinct == LISTING_BOUND + 3 + assert field.values is None + payload = field.as_json() + assert payload["values_listed"] is False + assert "values" not in payload + assert str(LISTING_BOUND) in str(payload["values_withheld_because"]) + + +def test_a_field_at_the_listing_bound_is_still_listed() -> None: + spec = FieldSpec("F", "Field") + rows: list[Mapping[str, object]] = [ + {"F": f"value {index}"} for index in range(LISTING_BOUND) + ] + field = survey_field(rows, spec) + assert field.values is not None + assert len(field.values) == LISTING_BOUND + assert field.as_json()["values_listed"] is True + + +def test_a_numeric_field_with_no_numbers_has_no_zero_share() -> None: + """Nothing to divide by is not zero percent. It does not exist.""" + reading = ZeroReading(zeros=0, declared_zeros=0, numbers=0) + assert reading.tenths_pct is None + assert reading.undeclared_tenths_pct is None + assert reading.worth_reviewing is False + + +def test_a_field_that_is_not_numeric_reports_no_zero_reading_at_all() -> None: + assert field_of(surveyed(dins_rows()), "DAMAGE").zeros is None + + +# -------------------------------------------------------------------------------------- +# Zeros +# -------------------------------------------------------------------------------------- + + +def test_a_numeric_field_whose_zeros_nobody_has_ruled_on_is_flagged() -> None: + rows = dins_rows() + rows[0]["ASSESSEDIMPROVEDVALUE"] = 0 + rows[1]["ASSESSEDIMPROVEDVALUE"] = 0 + field = field_of(surveyed(rows), "ASSESSEDIMPROVEDVALUE") + assert field.zeros is not None + assert field.zeros.undeclared_zeros == 2 + assert field.zeros.declared_zeros == 0 + assert field.zeros.worth_reviewing is True + assert field.needs_review is True + + +def test_a_zero_the_registry_has_already_reviewed_is_reported_and_not_flagged() -> None: + """`YEARBUILT` declares the literal `0`. Asking about it again every run is noise. + + This is the case issue #66 named as the day-one finding. It was reviewed in the + meantime, and the survey has to be able to tell a reviewed judgment from an open + question or a reader learns to skip the section. + """ + rows = dins_rows() + rows[0]["YEARBUILT"] = 0 + field = field_of(surveyed(rows), "YEARBUILT") + assert field.zeros is not None + assert field.zeros.zeros == 1 + assert field.zeros.declared_zeros == 1 + assert field.zeros.undeclared_zeros == 0 + assert field.zeros.worth_reviewing is False + assert field.needs_review is False + + +def test_the_zero_floor_is_a_reporting_threshold_and_nothing_is_dropped_by_it() -> None: + """A field under the floor still publishes its zero count; it is only not flagged.""" + spec = FieldSpec("N", "Number", numeric=True) + rows: list[Mapping[str, object]] = [{"N": 0}] + [ + {"N": index + 1} for index in range(999) + ] + field = survey_field(rows, spec) + assert field.zeros is not None + assert field.zeros.zeros == 1 + assert field.zeros.undeclared_tenths_pct == 1 + assert field.zeros.undeclared_tenths_pct < ZERO_SHARE_REPORTING_FLOOR_TENTHS_PCT + assert field.zeros.worth_reviewing is False + assert field.as_json()["recorded_zeros"] is not None + + +# -------------------------------------------------------------------------------------- +# The survey is not a way around schema drift +# -------------------------------------------------------------------------------------- + + +def test_a_column_missing_from_the_last_row_is_refused_not_surveyed_as_empty() -> None: + """A column that vanishes part way down a file is intact in the first row. + + Surveying it as a column every later row left blank would report a field the agency + stopped publishing as a field the agency left empty, which is the one reading this + project exists to refuse. + """ + rows = dins_rows() + del rows[-1]["EAVES"] + with pytest.raises(SchemaDriftError, match="EAVES"): + surveyed(rows) + + +def test_an_empty_file_surveys_as_zero_rows_rather_than_refusing() -> None: + survey = surveyed([]) + assert survey.rows == 0 + assert survey.needs_review == () + assert all(field.rows == 0 and field.distinct == 0 for field in survey.fields) + + +def test_a_source_with_no_registry_is_refused_by_name() -> None: + with pytest.raises(SchemaDriftError, match="no field registry"): + survey_file(DINS_FIXTURE, source="counties") + + +# -------------------------------------------------------------------------------------- +# Ordering and byte stability +# -------------------------------------------------------------------------------------- + + +def test_numbers_sort_as_numbers_and_text_after_them() -> None: + spec = FieldSpec("N", "Number") + rows: list[Mapping[str, object]] = [ + {"N": "10"}, + {"N": "9"}, + {"N": "alpha"}, + {"N": "2"}, + {"N": "Beta"}, + ] + field = survey_field(rows, spec) + assert field.values is not None + assert [value.value for value in field.values] == ["2", "9", "10", "Beta", "alpha"] + + +def test_the_survey_is_byte_identical_across_interpreters(tmp_path: Path) -> None: + """Across processes, under different hash seeds, over a fixture with real breadth. + + Twice inside one interpreter proves nothing: set and dict iteration over the same + strings is stable within a process, so a dropped `sorted` would be invisible. The + fixture is the whole DINS sample rather than one crafted row, because a field with + one value has no order to get wrong. + """ + outputs = [] + for seed in ("0", "1", "524287"): + out = tmp_path / f"seed-{seed}" + env = {**os.environ, "PYTHONHASHSEED": seed, "PYTHONDONTWRITEBYTECODE": "1"} + result = subprocess.run( # noqa: S603 + [ + sys.executable, + "-m", + "perimeter.survey", + "--dins", + str(DINS_FIXTURE), + "--perimeters", + str(FRAP_FIXTURE), + "--out", + str(out), + ], + cwd=ROOT, + env=env, + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 0, result.stderr + outputs.append( + ( + (out / "survey.json").read_bytes(), + (out / "survey.md").read_bytes(), + ) + ) + assert len(set(outputs)) == 1 + + +def test_the_fixture_holds_enough_distinct_values_for_order_to_be_observable() -> None: + """Guards the determinism test above against becoming vacuous. + + A fixture whose every measured field holds one value would pass that test with every + `sorted` removed. This pins the property the test depends on rather than trusting it. + """ + survey = surveyed(dins_rows()) + assert sum(1 for field in survey.fields if field.distinct > 1) >= 8 + + +# -------------------------------------------------------------------------------------- +# What gets written +# -------------------------------------------------------------------------------------- + + +def test_the_draft_marks_every_candidate_unreviewed() -> None: + rows = dins_rows() + rows[0]["STREETTYPE"] = "NA" + draft = markdown([surveyed(rows)]) + assert "basis: unreviewed" in draft + assert "`NA`" in draft + assert "Nothing here is declared" in draft + + +def test_the_draft_says_so_when_a_source_has_no_candidates() -> None: + draft = markdown( + [ + survey_rows( + load_rows(FRAP_FIXTURE), + specs=FRAP_FIELDS, + required=(), + source="perimeters", + ) + ] + ) + assert "No candidates." in draft + + +def test_the_artifact_says_it_declares_nothing(tmp_path: Path) -> None: + written = write([surveyed(dins_rows())], tmp_path / "out") + assert [path.name for path in written] == ["survey.json", "survey.md"] + payload: dict[str, Any] = json.loads(written[0].read_text(encoding="utf-8")) + assert "decides nothing" in str(payload["declares_nothing"]) + assert payload["listing_bound"] == LISTING_BOUND + assert ( + payload["zero_share_reporting_floor_tenths_pct"] + == ZERO_SHARE_REPORTING_FLOOR_TENTHS_PCT + ) + + +def test_the_survey_writes_nothing_into_the_repository(tmp_path: Path) -> None: + """It is a review aid. No part of the build reads it, and it edits no registry.""" + before = {spec.name: spec.unknown_markers for spec in DINS_FIELDS} + rows = dins_rows() + rows[0]["STREETTYPE"] = "NA" + write([surveyed(rows)], tmp_path / "out") + assert {spec.name: spec.unknown_markers for spec in DINS_FIELDS} == before + + +def test_main_needs_at_least_one_source(tmp_path: Path) -> None: + with pytest.raises(SystemExit) as caught: + main(["--out", str(tmp_path)]) + assert caught.value.code == 2 + + +def test_main_surveys_both_sources_and_writes_both_files( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + out = tmp_path / "survey" + assert ( + main( + [ + "--dins", + str(DINS_FIXTURE), + "--perimeters", + str(FRAP_FIXTURE), + "--out", + str(out), + ] + ) + == 0 + ) + printed = capsys.readouterr().out + assert "dins: 10 rows" in printed + assert "perimeters: 10 rows" in printed + assert (out / "survey.json").is_file() + assert (out / "survey.md").is_file() + + +def test_main_honours_a_narrower_listing_bound(tmp_path: Path) -> None: + out = tmp_path / "survey" + assert ( + main(["--dins", str(DINS_FIXTURE), "--out", str(out), "--listing-bound", "1"]) + == 0 + ) + payload = json.loads((out / "survey.json").read_text(encoding="utf-8")) + withheld = [ + field + for field in payload["sources"][0]["fields"] + if field["values_listed"] is False + ] + assert withheld, "a bound of one should withhold at least one listing" + assert all("values" not in field for field in withheld) + + +def test_survey_file_reads_a_real_source_by_name() -> None: + survey = survey_file(FRAP_FIXTURE, source="perimeters") + assert survey.source == "perimeters" + assert survey.rows == 10 + + +def test_normalize_is_the_one_the_registry_uses() -> None: + """The candidate's normalized form is the form the registry is declared in.""" + rows = dins_rows() + rows[0]["STREETTYPE"] = " Na " + field = field_of(surveyed(rows), "STREETTYPE") + assert [c.normalized for c in field.candidates] == [normalize_marker("Na")]