From aee759bcb51c18c84f458c0fa7ec3a47b37cb41b Mon Sep 17 00:00:00 2001 From: Chelsea Kelly-Reif <3114598+ChelseaKR@users.noreply.github.com> Date: Mon, 7 Sep 2026 13:55:07 -0700 Subject: [PATCH 1/2] A refresh began from a crash, one refusal at a time `SentinelDriftError` fails the build the first time a cell holds something that reads like a missing-data marker in a field nobody has reviewed for it. That 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 is worked through one refusal at a time. `python -m perimeter.survey` (`make survey`) reads the same files the build reads and reports every candidate at once, per measured field, with counts. It declares nothing: no schema, no registry edit, 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. The sentinel decision moved out of `FieldSpec.classify` into `FieldSpec.undeclared_marker`, which both callers now use, so the survey and the gate read one list rather than two that can drift. Three distinctions carried over from the artifacts: a field above the listing bound reports its distinct count and says the listing was withheld, never an empty list; a field with no published domain reports null for values outside one, never 0 (ADR-0010); a numeric field in which nothing parsed as a number has no zero share, not a zero one. --- CHANGELOG.md | 33 +++ Makefile | 23 ++ README.md | 37 +++ src/perimeter/schema.py | 30 ++- src/perimeter/survey.py | 574 ++++++++++++++++++++++++++++++++++++++++ tests/test_survey.py | 486 ++++++++++++++++++++++++++++++++++ 6 files changed, 1181 insertions(+), 2 deletions(-) create mode 100644 src/perimeter/survey.py create mode 100644 tests/test_survey.py 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..01a6c03 --- /dev/null +++ b/tests/test_survey.py @@ -0,0 +1,486 @@ +"""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 deliberately drawn from the sentinel vocabulary itself rather + than invented, so this compares the two readers over the values that actually decide + the question rather than over a fixture that happens to be clean. + """ + planted = [*sorted(SUSPECTED_SENTINELS), "Road", "No Damage", "0", "1975", "Butte"] + for spec in (*DINS_FIELDS, *FRAP_FIELDS): + 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")] From 4e79c9cee94837470edc6680a4dfd792a8265601 Mon Sep 17 00:00:00 2001 From: Chelsea Kelly-Reif <3114598+ChelseaKR@users.noreply.github.com> Date: Mon, 7 Sep 2026 13:56:41 -0700 Subject: [PATCH 2/2] Widen the one-list fixture to the casing the registry is declared in The first run of the negative control on this test passed with the sabotage in place, and that first run is the honest measurement. The control reordered `classify` so the sentinel net ran before the recorded-absence check, which is the realistic way the two readers come apart. It stayed green because the planted values were only `SUSPECTED_SENTINELS`, which is lowercase, while `recorded_absences` holds the publisher's own casing: `None` for a street type, `N/A` for a propane-tank distance. Nothing in the fixture reached the branch where the order decides anything. Planting each field's own declared values alongside the vocabulary makes the control fire, on `PROPANETANKDISTANCE` and `N/A`. --- tests/test_survey.py | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/tests/test_survey.py b/tests/test_survey.py index 01a6c03..c750346 100644 --- a/tests/test_survey.py +++ b/tests/test_survey.py @@ -83,12 +83,28 @@ def field_of(survey: SourceSurvey, name: str) -> FieldSurvey: 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 deliberately drawn from the sentinel vocabulary itself rather - than invented, so this compares the two readers over the values that actually decide - the question rather than over a fixture that happens to be clean. + 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. """ - planted = [*sorted(SUSPECTED_SENTINELS), "Road", "No Damage", "0", "1975", "Butte"] + 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: