From 336c5919a4176052b8708631113dfcdae9b2933e Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Sat, 26 Sep 2026 19:49:27 -0500 Subject: [PATCH 1/4] fix(scoring): protect imports and align synthetic session evidence Stream archive hashing, reject output aliases of inputs, and test the fixture against Measure Lab quantity and coordinate contracts. Leave routed cable length unsupported rather than substituting wall projection. Separate the short experiment record from input and metric references. Co-Authored-By: Claude Code --- experiments/scoring/FORMATS.md | 141 ++++++++++ experiments/scoring/METRICS.md | 76 ++++++ experiments/scoring/README.md | 188 +------------- .../scoring/fixtures/measure-lab/map.json | 12 +- .../measure-lab/synthetic-session-01.zip | Bin 10808 -> 11504 bytes .../synthetic-session-01/session.json | 106 +++++--- .../scoring/fixtures/truth/synthetic-01.json | 2 +- .../scoring/results/synthetic-01/checks.csv | 25 ++ .../results/synthetic-01/measurements.csv | 31 +++ .../scoring/results/synthetic-01/runs.csv | 4 + .../scoring/results/synthetic-01/summary.md | 54 ++++ .../scoring/src/scoring/measure_lab.py | 39 ++- experiments/scoring/tests/test_measure_lab.py | 244 ++++++++++++++++-- 13 files changed, 680 insertions(+), 242 deletions(-) create mode 100644 experiments/scoring/FORMATS.md create mode 100644 experiments/scoring/METRICS.md create mode 100644 experiments/scoring/results/synthetic-01/checks.csv create mode 100644 experiments/scoring/results/synthetic-01/measurements.csv create mode 100644 experiments/scoring/results/synthetic-01/runs.csv create mode 100644 experiments/scoring/results/synthetic-01/summary.md diff --git a/experiments/scoring/FORMATS.md b/experiments/scoring/FORMATS.md new file mode 100644 index 000000000..d3d6ad294 --- /dev/null +++ b/experiments/scoring/FORMATS.md @@ -0,0 +1,141 @@ +# Scoring input files + +`score` reads three kinds of JSON file: one rules file, one survey file per house, and one results file per pipeline run. `score import-measure-lab` writes a results file from a Measure Lab session. [METRICS.md](METRICS.md) explains what the scorer computes from them. + +Everything in `fixtures/` is synthetic. Keep real surveys and results beside the capture in `captures/` or `data/`, never in git. + +## Rules shared by all three files + +Every file starts with `"format": 1` and `"unit": "ft"`. + +The scorer reads numbers as exact decimals, so a tie with a threshold resolves the same way every time. In floats, 3.1 − 3.0 is 0.10000000000000009, which would pass a 3.1 ± 0.1 ft clearance against a 3 ft rule that is exactly borderline. + +The scorer rejects unknown fields, duplicate keys, `NaN`, negative lengths and numbers written as strings. Fields that may be null must still be present. Any input problem stops the run with exit code 2 and a message that names the file, the field and the fix, and the scorer writes no CSV. + +## Rules file + +The rules file holds named thresholds that use `rules.yaml` parameter names. It carries public or clearly synthetic values only. Base's unpublished values never go in a tracked file. + +```json +{ + "format": 1, "unit": "ft", "name": "Synthetic scoring fixture", + "thresholds": { + "gas_clearance_ft": {"value_ft": 3, "pass_when": "at_least", "source": "Public: Base help page"}, + "max_route_ft": {"value_ft": 20, "pass_when": "at_most", "source": "Public: Base help page"} + } +} +``` + +A threshold name must end in `_ft`. `pass_when` is `at_least` for a clearance, or `at_most` for a limit such as route length. Each results file carries the sha256 of the exact rules file it ran under (`shasum -a 256 rules.json`), so the scorer refuses a run made under different rules. + +Threshold names follow the [scoring protocol](https://github.com/SamGu-NRX/house-scanning-master/blob/9737e3f0eefe90f2a12a190bf8750e7fed64413f/docs/research/t3-scoring-protocol.md). `server/rules.yaml` is in PR #11 and not on `main`. In the fixture, `facing_gap_ft`, `pool_clearance_ft` and `review_route_ft` are synthetic placeholders. The format uses feet to match `rules.yaml` and `scene.json`. The protocol records surveys in meters, so convert once when you type a survey up. + +## Survey file + +Write one survey file per house. + +```json +{ + "format": 1, "unit": "ft", "house": "synthetic-01", + "captures": ["synthetic-01-walk-1"], + "scale_reference": "scale", + "candidates": [{"id": "c1", "marker": "Blue painter's tape outline", "location": "South wall, 7 ft 5 in from the SW corner"}], + "measurements": [ + {"id": "c1-gas", "candidate": "c1", "from": "footprint edge", "to": "gas regulator", + "status": "measured", "value_ft": 4.500, "plus_minus_ft": 0.010, + "method": "tape", "measured_by": ["surveyor-a", "surveyor-b"]} + ], + "checks": [ + {"candidate": "c1", "check": "gas", "measurement": "c1-gas", "threshold": "gas_clearance_ft"}, + {"candidate": "c1", "check": "route", "measurement": "c1-route", + "threshold": "max_route_ft", "review_threshold": "review_route_ft"} + ] +} +``` + +| Field | Meaning | +|---|---| +| `captures` | Recording ids this survey applies to, ideally each recording's sha256. Each id belongs to one house. | +| `scale_reference` | The one measurement some pipelines may use to set scale. It must be measured, cannot decide a check, and is never scored. | +| `candidates[]` | Spot `id`, its physical `marker`, and its `location` as tape offsets from permanent corners. | +| `measurements[]` | `candidate` (null for house-level distances such as wall length), `from` and `to` in words, `status` (`measured`, `absent` or `not_measured`), `method`, `measured_by`. Only `measured` entries have `value_ft` and `plus_minus_ft`. | +| `checks[]` | The spot, the check name, the deciding measurement and its `threshold`. Every spot needs the same check names with the same thresholds, so the denominator cannot shrink. An absent feature may decide only an `at_least` check. | +| `checks[].review_threshold` | Optional. It makes a band: values that clear `review_threshold` pass, values past `threshold` fail, and values between go to review. Both thresholds must point the same way, with the review line on the passing side. | + +## Results file + +Each pipeline run writes one results file. + +```json +{ + "format": 1, "unit": "ft", + "pipeline": "ar-taps", "capture": "synthetic-01-walk-1", + "rules_sha256": "<64 hex digits>", "scale_source": "ar_poses", + "measurements": [ + {"id": "c1-gas", "value_ft": 4.7, "plus_minus_ft": 0.3}, + {"id": "c2-facing", "value_ft": null, "missing": "failed"} + ], + "outcomes": [{"candidate": "c1", "check": "gas", "outcome": "pass"}], + "timing": {"capture_s": 420, "processing_s": 12.5} +} +``` + +| Field | Meaning | +|---|---| +| `pipeline` | Row name. Each variant is its own pipeline id. | +| `capture` | Must appear in exactly one survey's `captures`. | +| `scale_source` | `native_metric`, `scale_reference` or `ar_poses`. | +| `measurements[]` | One entry per survey measurement. The scale reference is optional. An entry has `value_ft` with `plus_minus_ft` (a number, or null if the run reports none), or `value_ft` null with `missing` set to `unsupported`, `failed` or `absent`. | +| `outcomes` | One `pass`, `unsure` or `fail` per survey check, exactly as the pipeline decided. Null for a run that makes no decisions. | +| `timing` | `capture_s` and `processing_s` in seconds, or null. Runs on the same capture must report the same `capture_s`. | + +## Measure Lab sessions + +`score import-measure-lab` turns a session zip shared from Measure Lab into a results file. It reads session format 2, which `experiments/measure-lab/SESSION-FORMAT.md` (PR #7) documents. The team writes a map file after the walk that ties each survey measurement to a session measurement. + +```sh +uv run score import-measure-lab fixtures/measure-lab/synthetic-session-01.zip \ + --map fixtures/measure-lab/map.json --rules fixtures/rules.json \ + --truth fixtures/truth/synthetic-01.json --out data/measure-lab.json +``` + +This excerpt shows each kind of map entry. `fixtures/measure-lab/map.json` is a complete map. Every survey measurement id except the scale reference needs a key. + +```json +{ + "format": 1, "unit": "ft", + "pipeline": "measure-lab", + "session": "synthetic-session-01", + "notes": "plus_minus_ft_by_key is the plan's untested 0.3 ft error bar for AR taps.", + "plus_minus_ft_by_key": {"straight": 0.3, "alongWall": 0.3}, + "measurements": { + "wall-length": {"session_measurement": "m2", "key": "alongWall"}, + "c1-gas": {"session_measurement": "m3", "key": "straight", "plus_minus_ft": 0.25}, + "c2-facing": {"refusal": "r1"}, + "c1-route": "unsupported", + "c1-pool": "absent" + } +} +``` + +| Map entry | Becomes | +|---|---| +| `{"session_measurement": id, "key": k}` | `values[k]` in feet. `k` must be the measurement's `compared` quantity, the one `accepted` vouches for. A measurement with `accepted: false` becomes null, missing `failed`. | +| `{"refusal": id}` | Null, missing `failed`. | +| `"absent"` or `"unsupported"` | Null, missing `absent` or `unsupported`. | + +A session measurement has only the quantities its endpoints allow, as `experiments/measure-lab/SESSION-FORMAT.md` lists. Point to point gives `straight`, `horizontal` and `vertical`, plus `alongWall` when a reference wall is chosen. Point to wall gives `gapToWall` and `heightAboveGround`. + +Map a route as `"unsupported"`. Measure Lab records no routed cable path, and an along-wall distance leaves out vertical legs and detours, so it is not a route length. + +`"absent"` is the operator's assertion that the feature does not exist. Write it only after someone has looked over the whole area the check needs and found no such feature. Never infer it from a missing measurement or a refusal. In the scorer, an absent feature passes an `at_least` clearance, so a wrong `"absent"` can become an unsafe pass. + +Every value needs a stated uncertainty: its entry's `plus_minus_ft`, or its key's value in `plus_minus_ft_by_key`. There is no default. The ±0.3 ft in the example is the plan's untested error bar for an AR tap, listed in `docs/00-overview.md` under "Conventions the code relies on". It is not a measured error bar. `session` must match the zip's session id. + +The importer converts meters exactly (1 ft = 0.3048 m) and rounds half up to a millionth of a foot. The capture id is the zip's sha256, which must already be in the survey's `captures`. `scale_source` is `ar_poses`. `capture_s` runs from the session's start to its last measurement. `processing_s` is null, because the app shows each value as it is tapped. A map or session problem stops the import with a specific message. The importer loads its output through the scorer before it writes the file. + +`--decide` also writes a row named `+rule`. Its outcomes apply the survey's strict rule, described in [METRICS.md](METRICS.md), to the run's own values and uncertainties. With no value the row says unsure, except that an absent feature passes an `at_least` check. The row emulates the check rule in `docs/00-overview.md` ("Every check answers PASS, FAIL or UNSURE") until a real solver exists. + +## Rebuild the fixture session zip + +`tests/helpers.py` builds `fixtures/measure-lab/synthetic-session-01.zip` reproducibly from `fixtures/measure-lab/synthetic-session-01/session.json`. After you edit that file, run `uv run python tests/helpers.py rebuild-session-zip`. Then put the printed sha256 in the survey's `captures`. diff --git a/experiments/scoring/METRICS.md b/experiments/scoring/METRICS.md new file mode 100644 index 000000000..42847467f --- /dev/null +++ b/experiments/scoring/METRICS.md @@ -0,0 +1,76 @@ +# What the scoring numbers mean + +`score` prints a markdown summary and writes `measurements.csv`, `checks.csv` and `runs.csv`. [`results/synthetic-01/`](results/synthetic-01/) is an example made from the synthetic fixtures. [FORMATS.md](FORMATS.md) describes the input files. + +The summary has one section per house: distances, checks, timing, and a list by name of every unsafe pass, missed review and decision made without its measurement. Nothing is averaged across houses, and missing outputs stay in the denominator. + +## Distances + +Error is run minus survey, in inches. The summary gives the median and maximum absolute error over the distances that both sides measured, and how often the survey value lies within the run's reported ±. + +Every survey measurement except the scale reference lands in one bucket: + +- scored; +- unsupported; +- failed; +- wrongly absent: the run says the feature is not there, but the survey measured it; +- phantom: the run measured a feature that the survey found absent; +- absent in both; +- not surveyed. + +The scale reference is left out of every error figure. A run given it can match it exactly, so its error says nothing about accuracy. + +## Survey outcome + +Let s be the survey value's margin on the passing side of the threshold, and u its uncertainty. A check passes when s > u, fails when s < −u, and is borderline otherwise. Both comparisons are strict, following the check rule in `docs/00-overview.md` ("Every check answers PASS, FAIL or UNSURE"). A value exactly on a line, or exactly u from it, never passes or fails. + +A band works the same way with two lines. For the route, with survey length L: + +| Survey | Outcome | +|---|---| +| L + u < `review_route_ft` | pass | +| L − u > `max_route_ft` | fail | +| anything else | review | + +An absent feature passes. An unmeasured feature is unknown and stays out of the decision counts. + +## Error relative to the threshold + +The scorer divides |error| by max(m, u), where m is the survey value's distance to the nearest line. At 1 or above, the error could flip the check, because a run exactly on a line neither passes nor fails. When m and u are both zero, the CSV reports `at_threshold`, and any nonzero error could flip the check. + +This ratio, and the `could_flip` column built on it, compares the size of the error with the margin. It is a warning sign, not a replay of the decision. `could_flip` false does not prove the run decided the same way as the survey, because the run's own ± and the pipeline's decision rule also move its answer. + +## Decisions + +A correct run passes a passing check, fails a failing one, and says unsure on a borderline or review check. The scorer counts wrong answers in three categories, which mean the same for every check: + +- **Unsafe pass.** A pass where the survey fails. This count matters most. +- **Missed review.** A pass where the survey is borderline or review. The scoring protocol counts a borderline pass as unsafe. The scorer keeps it in its own column, so "unsafe" always means the survey shows the spot breaks a rule. +- **Over-caution.** An unsure or a fail where the survey passes. The fails among them are also listed as false rejections. + +The categories do not cover every wrong answer on purpose. A fail where the survey is review, or an unsure where the survey fails, counts only as a disagreement: it lowers the agree count and lands in no category. + +A pass or fail is **decided without its measurement** when its deciding value is missing as `failed` or `unsupported`. A claimed absence counts the same way when it supports any decision on an `at_most` limit, or a fail on an `at_least` check. The scorer counts the decision as usual and also flags and lists it. + +An unsure is justified when the survey is borderline or review, or the run's measurement is missing as `failed` or `unsupported`. Any other unsure is avoidable, including one on a measurement the run marked `absent`. + +Timing is copied from the results files. Rows that share a recording share its capture time. + +## What one to three houses can and cannot show + +One to three houses can show which distances each pipeline returns, any unsafe pass at these spots, paired errors on identical input, and whether errors fit the plan's untested error bars in `docs/00-overview.md`: 0.3 ft for a tap, 0.5 ft for the mesh and 1.5 ft for photo detection. + +They cannot show: + +- an accuracy rate for homes in general, because distances from one house are not independent samples; +- validated error bars, because tuning and scoring on the same house proves nothing; +- safety, because zero unsafe passes at nine spots is a sample result; +- homeowner effort; +- installation eligibility, because electrical checks are out of scope. + +A claim the data can support has this shape: "On [N] houses, AR taps measured [k] of [n] distances, median error [x] in, [u] unsafe passes." + +## Not scored + +- Spot placement error: how far a pipeline's spot sits from the taped mark. +- Latency beyond one processing time per run, such as cold and warm starts, crashes and timeouts. diff --git a/experiments/scoring/README.md b/experiments/scoring/README.md index b64eca601..d636972e7 100644 --- a/experiments/scoring/README.md +++ b/experiments/scoring/README.md @@ -1,185 +1,21 @@ -# Scoring capture pipelines against a tape survey +# Scoring -Given an independent tape survey of a house and each capture pipeline's measurements of the same named distances and candidate spots: how large are the errors, how close are they to the thresholds that decide each check, and which outputs are missing or unsafe? +**Question.** Against an independent tape survey of a house, how far off is each capture pipeline's distance, how close is that error to the threshold that decides the check, and which outputs are missing or unsafe? -`score` answers that for any number of houses and pipeline runs. It does not run pipelines; each one writes a results file. The fieldwork protocol (what to survey, gates, freezing the rules) is the research note `docs/research/t3-scoring-protocol.md`. +**Pass criteria.** None set. The [scoring protocol](https://github.com/SamGu-NRX/house-scanning-master/blob/9737e3f0eefe90f2a12a190bf8750e7fed64413f/docs/research/t3-scoring-protocol.md) reports each house as a case series, and it counts unsafe passes, a pipeline PASS where the survey fails, as the number that matters most. -## Commands +**Run.** Needs uv. The scorer does not run pipelines. Each pipeline writes a results file. ```sh -cd experiments/scoring -uv sync --locked -uv run score --rules fixtures/rules.json \ - --truth fixtures/truth/synthetic-01.json \ - --results fixtures/results/ar-taps.json fixtures/results/photo-depth.json fixtures/results/mesh-scaled.json +cd experiments/scoring && uv sync --locked +uv run score --rules fixtures/rules.json --truth fixtures/truth/synthetic-01.json \ + --results fixtures/results/ar-taps.json fixtures/results/photo-depth.json fixtures/results/mesh-scaled.json \ + --out results/synthetic-01 > results/synthetic-01/summary.md +uv run pytest -q ``` -`score` prints a markdown summary and writes `measurements.csv`, `checks.csv` and `runs.csv` to `--out` (default `data/`, which git ignores). `--truth` and `--results` take several files. Any input problem stops the run with exit code 2 and a message naming the file, the field and the fix, and no CSV is written. +[FORMATS.md](FORMATS.md) specifies the input files and the Measure Lab importer. [METRICS.md](METRICS.md) explains every number. Real surveys and results go in git-ignored `data/`, the default `--out`. -CI runs `uv run ruff check .`, `uv run ruff format --check .` and `uv run pytest -q`. +**Result.** No real house scored yet. In [results/synthetic-01](results/synthetic-01/summary.md), three invented runs have median errors of 3.48, 12.60 and 1.20 in, and `ar-taps` makes 1 unsafe pass. Those numbers test the scorer, not any pipeline. All 274 tests and ruff pass. -Everything in `fixtures/` is synthetic. Keep real surveys and results beside the capture in `captures/` or `data/`, never in git. - -## Input formats - -All three files are JSON starting with `"format": 1` and `"unit": "ft"`. Numbers are read as exact decimals, so ties with a threshold resolve the same way every time: in floats 3.1 − 3.0 is 0.10000000000000009, which would pass a 3.1 ± 0.1 ft clearance against a 3 ft rule that is exactly borderline. Unknown fields, duplicate keys, `NaN`, negative lengths and numbers written as strings are rejected. Fields that may be null must still be present. - -### Rules file - -Named thresholds using `rules.yaml` parameter names, with public or clearly synthetic values only. Base's unpublished values never go in a tracked file. - -```json -{ - "format": 1, "unit": "ft", "name": "Synthetic scoring fixture", - "thresholds": { - "gas_clearance_ft": {"value_ft": 3, "pass_when": "at_least", "source": "Public: Base help page"}, - "max_route_ft": {"value_ft": 20, "pass_when": "at_most", "source": "Public: Base help page"} - } -} -``` - -A threshold name must end in `_ft`. `pass_when` is `at_least` for a clearance or `at_most` for a limit such as route length. Each results file carries the sha256 of the exact rules file it ran under (`shasum -a 256 rules.json`), so a run made under different rules cannot be scored by mistake. - -### Survey file (one per house) - -```json -{ - "format": 1, "unit": "ft", "house": "synthetic-01", - "captures": ["synthetic-01-walk-1"], - "scale_reference": "scale", - "candidates": [{"id": "c1", "marker": "Blue painter's tape outline", "location": "South wall, 7 ft 5 in from the SW corner"}], - "measurements": [ - {"id": "c1-gas", "candidate": "c1", "from": "footprint edge", "to": "gas regulator", - "status": "measured", "value_ft": 4.500, "plus_minus_ft": 0.010, - "method": "tape", "measured_by": ["surveyor-a", "surveyor-b"]} - ], - "checks": [ - {"candidate": "c1", "check": "gas", "measurement": "c1-gas", "threshold": "gas_clearance_ft"}, - {"candidate": "c1", "check": "route", "measurement": "c1-route", - "threshold": "max_route_ft", "review_threshold": "review_route_ft"} - ] -} -``` - -| Field | Meaning | -|---|---| -| `captures` | Recording ids this survey applies to, ideally each recording's sha256. Each id belongs to one house. | -| `scale_reference` | The one measurement some pipelines may use to set scale. It must be measured, cannot decide a check, and is never scored. | -| `candidates[]` | Spot `id`, its physical `marker`, and its `location` as tape offsets from permanent corners. | -| `measurements[]` | `candidate` (null for house-level distances such as wall length), `from` and `to` in words, `status` (`measured`, `absent` or `not_measured`; only `measured` has `value_ft` and `plus_minus_ft`), `method`, `measured_by`. | -| `checks[]` | The spot, check name, deciding measurement and `threshold`. Every spot needs the same check names with the same thresholds, so the denominator cannot shrink. An absent feature may only decide an `at_least` check. | -| `checks[].review_threshold` | Optional. Makes a band: values that clear `review_threshold` pass, values past `threshold` fail, values between go to review. Both thresholds must point the same way, with the review line on the passing side. | - -### Results file (one per pipeline run) - -```json -{ - "format": 1, "unit": "ft", - "pipeline": "ar-taps", "capture": "synthetic-01-walk-1", - "rules_sha256": "<64 hex digits>", "scale_source": "ar_poses", - "measurements": [ - {"id": "c1-gas", "value_ft": 4.7, "plus_minus_ft": 0.3}, - {"id": "c2-facing", "value_ft": null, "missing": "failed"} - ], - "outcomes": [{"candidate": "c1", "check": "gas", "outcome": "pass"}], - "timing": {"capture_s": 420, "processing_s": 12.5} -} -``` - -| Field | Meaning | -|---|---| -| `pipeline` | Row name. Each variant is its own pipeline id. | -| `capture` | Must appear in exactly one survey's `captures`. | -| `scale_source` | `native_metric`, `scale_reference` or `ar_poses`. | -| `measurements[]` | One entry per survey measurement (the scale reference is optional). `value_ft` with `plus_minus_ft` (a number, or null if the run reports none), or `value_ft` null with `missing`: `unsupported`, `failed` or `absent`. | -| `outcomes` | One `pass`, `unsure` or `fail` per survey check, exactly as the pipeline decided; null for a run that makes no decisions. | -| `timing` | `capture_s` and `processing_s` in seconds, or null. Runs on the same capture must report the same `capture_s`. | - -## Import a Measure Lab session - -`score import-measure-lab` turns a session zip shared from Measure Lab (session format 2, documented in `experiments/measure-lab/README.md` "Session format") into a results file, using a map file the team writes after the walk. - -```sh -uv run score import-measure-lab fixtures/measure-lab/synthetic-session-01.zip \ - --map fixtures/measure-lab/map.json --rules fixtures/rules.json \ - --truth fixtures/truth/synthetic-01.json --out data/measure-lab.json -``` - -An excerpt of a map; `fixtures/measure-lab/map.json` is complete. Every survey measurement id needs a key, except the scale reference. - -```json -{ - "format": 1, "unit": "ft", - "pipeline": "measure-lab", - "session": "synthetic-session-01", - "notes": "plus_minus_ft_by_key is docs/02's ±0.3 ft for AR taps, the plan's untested estimate.", - "plus_minus_ft_by_key": {"straight": 0.3, "alongWall": 0.3, "gapToWall": 0.3}, - "measurements": { - "wall-length": {"session_measurement": "m2", "key": "alongWall"}, - "c1-gas": {"session_measurement": "m3", "key": "straight", "plus_minus_ft": 0.25}, - "c2-facing": {"refusal": "r1"}, - "c1-pool": "absent", - "c2-pool": "unsupported" - } -} -``` - -| Map entry | Becomes | -|---|---| -| `{"session_measurement": id, "key": k}` | `values[k]` in feet. `k` must be the measurement's `compared` quantity, the one `accepted` vouches for. `accepted: false` becomes null, missing `failed`. | -| `{"refusal": id}` | Null, missing `failed`. | -| `"absent"` / `"unsupported"` | Null, missing `absent` / `unsupported`. | - -Every value needs a stated uncertainty, its entry's `plus_minus_ft` or its key's in `plus_minus_ft_by_key`; there is no default. The ±0.3 ft in the example is docs/02's estimate for AR taps, not a measured error bar. `session` must match the zip's session id. - -The importer converts meters exactly (1 ft = 0.3048 m) and rounds half up to a millionth of a foot. The capture id is the zip's sha256, which must already be in the survey's `captures`. `scale_source` is `ar_poses`. `capture_s` runs from the session's start to its last measurement; `processing_s` is null, because the rig shows each value as it is tapped. A map or session problem stops the import with a specific message, and the output is loaded through the scorer before it is written. - -`--decide` writes a row named `+rule` whose outcomes apply the survey's strict rule (below) to the run's own values and uncertainties. With no value it says unsure, except that an absent feature passes an `at_least` check. This emulates the lane C rule until a real solver exists. - -The fixture zip is built reproducibly from `fixtures/measure-lab/synthetic-session-01/session.json`. After editing it, run `uv run python tests/helpers.py rebuild-session-zip` and put the printed sha256 in the survey's `captures`. - -## What the numbers mean - -The summary has one section per house: distances, checks, timing, and every unsafe pass, missed review and decision made without its measurement, by name. Nothing is averaged across houses, and missing outputs stay in the denominator. - -**Distances.** Error is run minus survey, in inches. The summary gives the median and maximum absolute error over distances both sides measured, and how often the survey value lies within the run's reported ±. Every survey measurement except the scale reference lands in one bucket: scored, unsupported, failed, wrongly absent (the run says it is not there; the survey measured it), phantom (the run measured a feature the survey found absent), absent in both, or not surveyed. - -**The scale reference is excluded from every error statistic.** A run given it can match it exactly, so its error says nothing about accuracy. - -**Survey outcome.** With s the survey value's margin on the passing side and u its uncertainty, a check passes when s > u, fails when s < −u, and is borderline otherwise. Both comparisons are strict, following `docs/02-implementation-plan.md` Lane C ("PASS if the margin is larger than the error"), so a value exactly on a line or exactly u from it never passes or fails. A band works the same way with two lines. For the route: - -| Survey | Outcome | -|---|---| -| L + u < `review_route_ft` | pass | -| L − u > `max_route_ft` | fail | -| anything else | review | - -An absent feature passes. An unmeasured one is unknown and left out of the decision counts. - -**Error relative to the threshold.** |error| / max(m, u), where m is the survey value's distance to the nearest line. At 1 or above the error could flip the check, since a run exactly on a line neither passes nor fails. With m and u both zero the CSV reports `at_threshold`, and any nonzero error could flip. - -**Decisions.** A correct run passes a passing check, fails a failing one, and says unsure on a borderline or review one. Wrong answers are counted in three categories that mean the same for every check: - -- **Unsafe pass:** a pass where the survey fails. This count matters most. -- **Missed review:** a pass where the survey is borderline or review. The protocol note counts a borderline pass as unsafe; here it stays visible in its own column, so "unsafe" always means the survey shows the spot breaks a rule. -- **Over-caution:** an unsure or fail where the survey passes. False rejections, the fails among them, are also listed. - -**Decided without its measurement:** a pass or fail whose deciding value is missing as `failed` or `unsupported`, or marked `absent` for an `at_most` limit. It is scored as usual and also counted, flagged and listed on its own. - -An unsure is justified when the survey is borderline or review, or the run had no value; otherwise it is avoidable. - -**Timing.** As reported. Rows sharing a recording share its capture time. - -## What one to three houses can and cannot show - -They can show which distances each pipeline returns, any unsafe pass at these spots, paired errors on identical input, and whether errors fit the plan's hypothesized error bars (±0.3 ft taps, ±0.5 ft mesh, ±1.5 ft photo detection). - -They cannot show an accuracy rate for homes in general (distances from one house are not independent samples), validated error bars (tuning and scoring on the same house proves nothing), safety (zero unsafe passes at nine spots is a sample result), homeowner effort, or installation eligibility (electrical checks are out of scope). - -An honest pitch sentence: "On [N] houses, AR taps measured [k] of [n] distances, median error [x] in, [u] unsafe passes." - -## Not scored - -- Spot placement error: how far a pipeline's spot sits from the taped mark. -- Latency beyond one processing time per run: cold and warm starts, crashes, timeouts. -- `rules.yaml` does not exist yet. Threshold names follow the protocol note, and `facing_gap_ft`, `pool_clearance_ft` and `review_route_ft` in the fixture are synthetic placeholders. The format uses feet to match `rules.yaml` and `scene.json`; the protocol records surveys in meters, so convert once when typing a survey up. +**What changed.** Nothing in the product or the plan yet. The field eval in PR #12 calls `score import-measure-lab` to score a Measure Lab session. diff --git a/experiments/scoring/fixtures/measure-lab/map.json b/experiments/scoring/fixtures/measure-lab/map.json index c5ffe169f..72e7a1d66 100644 --- a/experiments/scoring/fixtures/measure-lab/map.json +++ b/experiments/scoring/fixtures/measure-lab/map.json @@ -3,7 +3,7 @@ "unit": "ft", "pipeline": "measure-lab", "session": "synthetic-session-01", - "notes": "Synthetic example. plus_minus_ft_by_key is docs/02-implementation-plan.md's +-0.3 ft for AR taps, the plan's untested estimate, not a measured error bar.", + "notes": "Synthetic example. plus_minus_ft_by_key is the plan's untested 0.3 ft error bar for an AR tap (docs/00-overview.md, Conventions the code relies on), not a measured error bar. Routes are unsupported: Measure Lab records no routed cable path, and an along-wall distance is not a route length.", "plus_minus_ft_by_key": { "straight": 0.3, "horizontal": 0.3, @@ -29,10 +29,7 @@ "session_measurement": "m4", "key": "gapToWall" }, - "c1-route": { - "session_measurement": "m5", - "key": "alongWall" - }, + "c1-route": "unsupported", "c1-pool": "absent", "c2-gas": { "session_measurement": "m6", @@ -41,10 +38,7 @@ "c2-facing": { "refusal": "r1" }, - "c2-route": { - "session_measurement": "m7", - "key": "alongWall" - }, + "c2-route": "unsupported", "c2-pool": "unsupported" } } diff --git a/experiments/scoring/fixtures/measure-lab/synthetic-session-01.zip b/experiments/scoring/fixtures/measure-lab/synthetic-session-01.zip index f75e62f66c9bef4a29329da13f633ff5708cda23..5dd599a67a49df42b7528f82339f2483ae11a36b 100644 GIT binary patch delta 504 zcmdlH@*#3U9q%srTlGHL3=ANwIC-O{#Ky(vc^FM5FObxpJYQ6L@&WNhJ4=g!G}VigKp3Q(W~ktj(`3CKyzOI3nOOlIUY-)t-XgmJO~ zw-}S5!DM@3)5)2F(vxkaL|`^d?iR9|yo0ZV$ delta 422 zcmewmxg%sk9q-J8hP6{P7#KiUaiX8ZwpovV3ESlJ?E5B9m9d*+5lxa)L(2WIvT2kUYDgo{6QgsnKLU4GB1VgR&$@&*tAM7Z`z> z6d8@d+6t8BO>R@yVzhu8&ZS|=WTG?qqly@dfu5x)P^~QH|@EyhJO6Be5VPKRhufXYyYy_4xFn#H!4^^q|B_B^@pWFi^@(EiO(> zPgSx~PzuRNRVYfVRLD(DEh$zg0rJwpW-FBB=PMLv08Pm+&#BZ?2rfx2P)JHl&Q?gw zOHn9EEKo>HPt45In|xMLZZfABH>ZW30mS^t+gTOap*BsPtNo7)5fhV_>bSBwYcc=< E08G<}iU0rr diff --git a/experiments/scoring/fixtures/measure-lab/synthetic-session-01/session.json b/experiments/scoring/fixtures/measure-lab/synthetic-session-01/session.json index d36bc319a..a3795dd76 100644 --- a/experiments/scoring/fixtures/measure-lab/synthetic-session-01/session.json +++ b/experiments/scoring/fixtures/measure-lab/synthetic-session-01/session.json @@ -123,7 +123,7 @@ "id": "p4", "kind": "wall", "position": [ - 2.02, + 2.024, 0.0, 0.0 ], @@ -137,7 +137,7 @@ "twoView": null, "wallCoordinates": { "wall": "w1", - "along": 2.02, + "along": 2.024, "heightAboveGround": 0.0, "offset": 0.0, "withinContacts": true @@ -233,7 +233,7 @@ ], "taps": [], "ground": { - "surface": "detectedPlane", + "surface": "estimatedPlane", "planeAnchor": "A1", "lookDown": 42.0 }, @@ -246,7 +246,9 @@ "offset": -0.95, "withinContacts": true }, - "flags": [], + "flags": [ + "estimatedPlane" + ], "wallWarnings": [] }, { @@ -274,6 +276,58 @@ }, "flags": [], "wallWarnings": [] + }, + { + "id": "p10", + "kind": "wall", + "position": [ + 2.26, + 0.0, + 0.0 + ], + "taps": [], + "ground": null, + "onWall": { + "wall": "w1", + "range": 2.5, + "angleFromNormal": 12.0 + }, + "twoView": null, + "wallCoordinates": { + "wall": "w1", + "along": 2.26, + "heightAboveGround": 0.0, + "offset": 0.0, + "withinContacts": true + }, + "flags": [], + "wallWarnings": [] + }, + { + "id": "p11", + "kind": "wall", + "position": [ + 6.4, + 0.0, + 0.0 + ], + "taps": [], + "ground": null, + "onWall": { + "wall": "w1", + "range": 2.5, + "angleFromNormal": 12.0 + }, + "twoView": null, + "wallCoordinates": { + "wall": "w1", + "along": 6.4, + "heightAboveGround": 0.0, + "offset": 0.0, + "withinContacts": true + }, + "flags": [], + "wallWarnings": [] } ], "walls": [ @@ -362,15 +416,13 @@ { "id": "m3", "time": 1120.0, - "from": "p5", - "to": "w1", - "referenceWall": "w1", + "from": "p10", + "to": "p5", + "referenceWall": null, "values": { "straight": 1.4, "horizontal": 1.4, - "vertical": 0.0, - "gapToWall": 1.4, - "heightAboveGround": 0.0 + "vertical": 0.0 }, "compared": "straight", "tape": null, @@ -384,11 +436,8 @@ "time": 1180.0, "from": "p6", "to": "w1", - "referenceWall": "w1", + "referenceWall": null, "values": { - "straight": 1.49352, - "horizontal": 1.49352, - "vertical": 0.0, "gapToWall": 1.49352, "heightAboveGround": 0.0 }, @@ -409,8 +458,7 @@ "straight": 3.73065, "horizontal": 3.71856, "vertical": 0.3, - "alongWall": 3.71856, - "heightAboveGround": 0.3 + "alongWall": 3.71856 }, "compared": "alongWall", "tape": null, @@ -422,22 +470,20 @@ { "id": "m6", "time": 1350.0, - "from": "p8", - "to": "w1", - "referenceWall": "w1", + "from": "p11", + "to": "p8", + "referenceWall": null, "values": { "straight": 0.95, "horizontal": 0.95, - "vertical": 0.04, - "gapToWall": 0.95, - "heightAboveGround": -0.04 + "vertical": 0.0 }, "compared": "straight", "tape": null, "errorMeters": null, "errorInches": null, "warnings": [ - "belowGround" + "estimatedPlane" ], "accepted": false }, @@ -451,8 +497,7 @@ "straight": 6.01205, "horizontal": 6.00456, "vertical": 0.3, - "alongWall": 6.00456, - "heightAboveGround": 0.3 + "alongWall": 6.00456 }, "compared": "alongWall", "tape": null, @@ -466,14 +511,11 @@ { "id": "r1", "time": 1300.0, - "tool": "gapToWall", + "tool": "ground", "tap": null, - "reason": "grazingRay", - "message": "The ray meets the ground too shallowly. Step back and tap again.", - "values": { - "lookDown": 8.0, - "minimumGroundLookDown": 20.0 - } + "reason": "trackingNotReady", + "message": "That frame wasn't in normal tracking. Hold steady and try again.", + "values": {} } ], "tracking": [ diff --git a/experiments/scoring/fixtures/truth/synthetic-01.json b/experiments/scoring/fixtures/truth/synthetic-01.json index 328419406..3439dc6b9 100644 --- a/experiments/scoring/fixtures/truth/synthetic-01.json +++ b/experiments/scoring/fixtures/truth/synthetic-01.json @@ -2,7 +2,7 @@ "format": 1, "unit": "ft", "house": "synthetic-01", - "captures": ["synthetic-01-walk-1", "7ded14d64c8b2810d9b26150b439bc9c4dc52a753f6189ee22d481fc14ce8e9d"], + "captures": ["synthetic-01-walk-1", "fc082df094988c98d204aa6fcd98a215f047826a7a4ade11c88c5c735e048686"], "scale_reference": "scale", "candidates": [ { diff --git a/experiments/scoring/results/synthetic-01/checks.csv b/experiments/scoring/results/synthetic-01/checks.csv new file mode 100644 index 000000000..6eb8b00e1 --- /dev/null +++ b/experiments/scoring/results/synthetic-01/checks.csv @@ -0,0 +1,25 @@ +house,capture,pipeline,candidate,check,measurement,threshold,threshold_ft,review_threshold,review_threshold_ft,pass_when,survey_ft,survey_plus_minus_ft,margin_ft,review_margin_ft,truth_outcome,run_ft,abs_error_in,error_to_margin,could_flip,run_outcome,expected_outcome,agrees,unsafe_pass,missed_review,over_caution,false_rejection,decided_without_measurement,abstention +synthetic-01,synthetic-01-walk-1,ar-taps,c1,gas,c1-gas,gas_clearance_ft,3.000,,,at_least,4.500,0.010,1.500,,pass,4.700,2.40,0.13,false,pass,pass,true,false,false,false,false,false, +synthetic-01,synthetic-01-walk-1,ar-taps,c1,facing_gap,c1-facing,facing_gap_ft,3.000,,,at_least,5.000,0.020,2.000,,pass,4.600,4.80,0.20,false,pass,pass,true,false,false,false,false,false, +synthetic-01,synthetic-01-walk-1,ar-taps,c1,route,c1-route,max_route_ft,20.000,review_route_ft,15.000,at_most,12.000,0.050,8.000,3.000,pass,12.250,3.00,0.08,false,pass,pass,true,false,false,false,false,false, +synthetic-01,synthetic-01-walk-1,ar-taps,c1,pool,c1-pool,pool_clearance_ft,5.000,,,at_least,,,,,pass,,,,,pass,pass,true,false,false,false,false,false, +synthetic-01,synthetic-01-walk-1,ar-taps,c2,gas,c2-gas,gas_clearance_ft,3.000,,,at_least,3.020,0.030,0.020,,borderline,3.350,3.96,11.00,true,pass,unsure,false,false,true,false,false,false, +synthetic-01,synthetic-01-walk-1,ar-taps,c2,facing_gap,c2-facing,facing_gap_ft,3.000,,,at_least,2.500,0.020,-0.500,,fail,,,,,pass,fail,false,true,false,false,false,true, +synthetic-01,synthetic-01-walk-1,ar-taps,c2,route,c2-route,max_route_ft,20.000,review_route_ft,15.000,at_most,19.900,0.050,0.100,-4.900,review,19.500,4.80,4.00,true,pass,unsure,false,false,true,false,false,false, +synthetic-01,synthetic-01-walk-1,ar-taps,c2,pool,c2-pool,pool_clearance_ft,5.000,,,at_least,,,,,unknown,,,,,fail,,,,,,,true, +synthetic-01,synthetic-01-walk-1,photo-depth,c1,gas,c1-gas,gas_clearance_ft,3.000,,,at_least,4.500,0.010,1.500,,pass,5.500,12.00,0.67,false,pass,pass,true,false,false,false,false,false, +synthetic-01,synthetic-01-walk-1,photo-depth,c1,facing_gap,c1-facing,facing_gap_ft,3.000,,,at_least,5.000,0.020,2.000,,pass,3.800,14.40,0.60,false,fail,pass,false,false,false,true,true,false, +synthetic-01,synthetic-01-walk-1,photo-depth,c1,route,c1-route,max_route_ft,20.000,review_route_ft,15.000,at_most,12.000,0.050,8.000,3.000,pass,,,,,unsure,pass,false,false,false,true,false,false,justified +synthetic-01,synthetic-01-walk-1,photo-depth,c1,pool,c1-pool,pool_clearance_ft,5.000,,,at_least,,,,,pass,12.000,,,,pass,pass,true,false,false,false,false,false, +synthetic-01,synthetic-01-walk-1,photo-depth,c2,gas,c2-gas,gas_clearance_ft,3.000,,,at_least,3.020,0.030,0.020,,borderline,2.400,7.44,20.67,true,fail,unsure,false,false,false,false,false,false, +synthetic-01,synthetic-01-walk-1,photo-depth,c2,facing_gap,c2-facing,facing_gap_ft,3.000,,,at_least,2.500,0.020,-0.500,,fail,2.900,4.80,0.80,false,unsure,fail,false,false,false,false,false,false,avoidable +synthetic-01,synthetic-01-walk-1,photo-depth,c2,route,c2-route,max_route_ft,20.000,review_route_ft,15.000,at_most,19.900,0.050,0.100,-4.900,review,21.000,13.20,11.00,true,fail,unsure,false,false,false,false,false,false, +synthetic-01,synthetic-01-walk-1,photo-depth,c2,pool,c2-pool,pool_clearance_ft,5.000,,,at_least,,,,,unknown,,,,,unsure,,,,,,,false, +synthetic-01,synthetic-01-walk-1,mesh-scaled,c1,gas,c1-gas,gas_clearance_ft,3.000,,,at_least,4.500,0.010,1.500,,pass,,,,,,pass,,,,,,, +synthetic-01,synthetic-01-walk-1,mesh-scaled,c1,facing_gap,c1-facing,facing_gap_ft,3.000,,,at_least,5.000,0.020,2.000,,pass,5.100,1.20,0.05,false,,pass,,,,,,, +synthetic-01,synthetic-01-walk-1,mesh-scaled,c1,route,c1-route,max_route_ft,20.000,review_route_ft,15.000,at_most,12.000,0.050,8.000,3.000,pass,11.500,6.00,0.17,false,,pass,,,,,,, +synthetic-01,synthetic-01-walk-1,mesh-scaled,c1,pool,c1-pool,pool_clearance_ft,5.000,,,at_least,,,,,pass,,,,,,pass,,,,,,, +synthetic-01,synthetic-01-walk-1,mesh-scaled,c2,gas,c2-gas,gas_clearance_ft,3.000,,,at_least,3.020,0.030,0.020,,borderline,3.000,0.24,0.67,false,,unsure,,,,,,, +synthetic-01,synthetic-01-walk-1,mesh-scaled,c2,facing_gap,c2-facing,facing_gap_ft,3.000,,,at_least,2.500,0.020,-0.500,,fail,2.600,1.20,0.20,false,,fail,,,,,,, +synthetic-01,synthetic-01-walk-1,mesh-scaled,c2,route,c2-route,max_route_ft,20.000,review_route_ft,15.000,at_most,19.900,0.050,0.100,-4.900,review,,,,,,unsure,,,,,,, +synthetic-01,synthetic-01-walk-1,mesh-scaled,c2,pool,c2-pool,pool_clearance_ft,5.000,,,at_least,,,,,unknown,,,,,,,,,,,,, diff --git a/experiments/scoring/results/synthetic-01/measurements.csv b/experiments/scoring/results/synthetic-01/measurements.csv new file mode 100644 index 000000000..310030e08 --- /dev/null +++ b/experiments/scoring/results/synthetic-01/measurements.csv @@ -0,0 +1,31 @@ +house,capture,pipeline,measurement,candidate,from,to,survey_status,survey_ft,survey_plus_minus_ft,run_ft,run_plus_minus_ft,run_missing,status,signed_error_in,abs_error_in,truth_within_reported +synthetic-01,synthetic-01-walk-1,ar-taps,scale,,scale mark A on the south wall,scale mark B on the south wall,measured,5.000,0.005,6.000,0.300,,scale_reference,,, +synthetic-01,synthetic-01-walk-1,ar-taps,wall-length,,southwest exterior corner,southeast exterior corner,measured,32.500,0.010,32.300,0.300,,scored,-2.40,2.40,true +synthetic-01,synthetic-01-walk-1,ar-taps,c1-gas,c1,footprint edge,gas regulator,measured,4.500,0.010,4.700,0.300,,scored,2.40,2.40,true +synthetic-01,synthetic-01-walk-1,ar-taps,c1-facing,c1,wall,"facing fence, minimum of three points across the footprint",measured,5.000,0.020,4.600,0.300,,scored,-4.80,4.80,false +synthetic-01,synthetic-01-walk-1,ar-taps,c1-route,c1,meter connection point,"battery connection point, along the supported route",measured,12.000,0.050,12.250,0.300,,scored,3.00,3.00,true +synthetic-01,synthetic-01-walk-1,ar-taps,c1-pool,c1,footprint edge,pool edge,absent,,,,,absent,absent_agreed,,, +synthetic-01,synthetic-01-walk-1,ar-taps,c2-gas,c2,footprint edge,gas meter,measured,3.020,0.030,3.350,0.300,,scored,3.96,3.96,false +synthetic-01,synthetic-01-walk-1,ar-taps,c2-facing,c2,wall,"facing hedge, minimum of three points across the footprint",measured,2.500,0.020,,,failed,missing_failed,,, +synthetic-01,synthetic-01-walk-1,ar-taps,c2-route,c2,meter connection point,"battery connection point, along the supported route",measured,19.900,0.050,19.500,0.300,,scored,-4.80,4.80,false +synthetic-01,synthetic-01-walk-1,ar-taps,c2-pool,c2,footprint edge,pool edge,not_measured,,,,,failed,not_surveyed,,, +synthetic-01,synthetic-01-walk-1,photo-depth,scale,,scale mark A on the south wall,scale mark B on the south wall,measured,5.000,0.005,,,,scale_reference,,, +synthetic-01,synthetic-01-walk-1,photo-depth,wall-length,,southwest exterior corner,southeast exterior corner,measured,32.500,0.010,30.500,1.500,,scored,-24.00,24.00,false +synthetic-01,synthetic-01-walk-1,photo-depth,c1-gas,c1,footprint edge,gas regulator,measured,4.500,0.010,5.500,1.500,,scored,12.00,12.00,true +synthetic-01,synthetic-01-walk-1,photo-depth,c1-facing,c1,wall,"facing fence, minimum of three points across the footprint",measured,5.000,0.020,3.800,1.500,,scored,-14.40,14.40,true +synthetic-01,synthetic-01-walk-1,photo-depth,c1-route,c1,meter connection point,"battery connection point, along the supported route",measured,12.000,0.050,,,unsupported,missing_unsupported,,, +synthetic-01,synthetic-01-walk-1,photo-depth,c1-pool,c1,footprint edge,pool edge,absent,,,12.000,1.500,,phantom,,, +synthetic-01,synthetic-01-walk-1,photo-depth,c2-gas,c2,footprint edge,gas meter,measured,3.020,0.030,2.400,1.500,,scored,-7.44,7.44,true +synthetic-01,synthetic-01-walk-1,photo-depth,c2-facing,c2,wall,"facing hedge, minimum of three points across the footprint",measured,2.500,0.020,2.900,1.500,,scored,4.80,4.80,true +synthetic-01,synthetic-01-walk-1,photo-depth,c2-route,c2,meter connection point,"battery connection point, along the supported route",measured,19.900,0.050,21.000,1.500,,scored,13.20,13.20,true +synthetic-01,synthetic-01-walk-1,photo-depth,c2-pool,c2,footprint edge,pool edge,not_measured,,,,,unsupported,not_surveyed,,, +synthetic-01,synthetic-01-walk-1,mesh-scaled,scale,,scale mark A on the south wall,scale mark B on the south wall,measured,5.000,0.005,5.000,,,scale_reference,,, +synthetic-01,synthetic-01-walk-1,mesh-scaled,wall-length,,southwest exterior corner,southeast exterior corner,measured,32.500,0.010,32.600,,,scored,1.20,1.20, +synthetic-01,synthetic-01-walk-1,mesh-scaled,c1-gas,c1,footprint edge,gas regulator,measured,4.500,0.010,,,absent,false_absent,,, +synthetic-01,synthetic-01-walk-1,mesh-scaled,c1-facing,c1,wall,"facing fence, minimum of three points across the footprint",measured,5.000,0.020,5.100,0.500,,scored,1.20,1.20,true +synthetic-01,synthetic-01-walk-1,mesh-scaled,c1-route,c1,meter connection point,"battery connection point, along the supported route",measured,12.000,0.050,11.500,0.500,,scored,-6.00,6.00,true +synthetic-01,synthetic-01-walk-1,mesh-scaled,c1-pool,c1,footprint edge,pool edge,absent,,,,,absent,absent_agreed,,, +synthetic-01,synthetic-01-walk-1,mesh-scaled,c2-gas,c2,footprint edge,gas meter,measured,3.020,0.030,3.000,0.500,,scored,-0.24,0.24,true +synthetic-01,synthetic-01-walk-1,mesh-scaled,c2-facing,c2,wall,"facing hedge, minimum of three points across the footprint",measured,2.500,0.020,2.600,0.500,,scored,1.20,1.20,true +synthetic-01,synthetic-01-walk-1,mesh-scaled,c2-route,c2,meter connection point,"battery connection point, along the supported route",measured,19.900,0.050,,,failed,missing_failed,,, +synthetic-01,synthetic-01-walk-1,mesh-scaled,c2-pool,c2,footprint edge,pool edge,not_measured,,,,,absent,not_surveyed,,, diff --git a/experiments/scoring/results/synthetic-01/runs.csv b/experiments/scoring/results/synthetic-01/runs.csv new file mode 100644 index 000000000..660dc801f --- /dev/null +++ b/experiments/scoring/results/synthetic-01/runs.csv @@ -0,0 +1,4 @@ +house,capture,pipeline,scale_source,capture_s,processing_s,measurements,scored,median_abs_error_in,max_abs_error_in,within_reported,with_reported_uncertainty,missing_unsupported,missing_failed,false_absent,phantom,absent_agreed,not_surveyed,checks,judged,agrees,unsafe_passes,missed_reviews,over_cautious,false_rejections,decided_without_measurement,abstentions_justified,abstentions_avoidable,could_flip +synthetic-01,synthetic-01-walk-1,ar-taps,ar_poses,420.0,12.5,9,6,3.48,4.80,3,6,0,1,0,0,1,1,8,7,4,1,2,0,0,2,0,0,2 +synthetic-01,synthetic-01-walk-1,photo-depth,native_metric,420.0,95.0,9,6,12.60,24.00,5,6,1,0,0,1,0,1,8,7,2,0,0,2,1,0,1,1,2 +synthetic-01,synthetic-01-walk-1,mesh-scaled,scale_reference,420.0,,9,5,1.20,6.00,4,4,0,1,1,0,1,1,8,7,,,,,,,,,0 diff --git a/experiments/scoring/results/synthetic-01/summary.md b/experiments/scoring/results/synthetic-01/summary.md new file mode 100644 index 000000000..d44b16bab --- /dev/null +++ b/experiments/scoring/results/synthetic-01/summary.md @@ -0,0 +1,54 @@ +# Capture pipeline scores + +Rules: `rules.json` (Synthetic scoring fixture. Not a real placement policy.), sha256 `05b466b3436155e190513a21b6333304ad9da4e93f1b5022b1813a433bdb325e`. +1 house, 2 candidate spots, 3 pipeline runs. + +Each house's declared scale reference is left out of every error figure: a run that was given it can match it exactly, so its error says nothing about accuracy. + +## House synthetic-01 + +Captures: synthetic-01-walk-1, fc082df094988c98d204aa6fcd98a215f047826a7a4ade11c88c5c735e048686. Candidates: c1, c2. Scale reference `scale` (5.000 ft), excluded. + +### Distances + +9 distances besides the scale reference; each row's counts add up to it. Errors are absolute, in inches, over the distances both the survey and the run measured. Wrongly absent: the run says the feature is not there, but the survey measured it. Phantom: the run measured a feature the survey found absent. + +| Pipeline | Scale source | Scored | Median error (in) | Max error (in) | Survey inside run's ± | Unsupported | Failed | Wrongly absent | Phantom | Absent, agreed | Not surveyed | +|---|---|---|---|---|---|---|---|---|---|---|---| +| `ar-taps` | ar_poses | 6/9 | 3.48 | 4.80 | 3/6 | 0 | 1 | 0 | 0 | 1 | 1 | +| `photo-depth` | native_metric | 6/9 | 12.60 | 24.00 | 5/6 | 1 | 0 | 0 | 1 | 0 | 1 | +| `mesh-scaled` | scale_reference | 5/9 | 1.20 | 6.00 | 4/4 | 0 | 1 | 1 | 0 | 1 | 1 | + +### Checks + +8 checks, 7 with a survey outcome. The survey passes a check when it clears the pass line by more than its ± and fails it when it misses the fail line by more than its ±. Anything between, including a value exactly on a line, is borderline (one threshold) or review (a band such as review_route_ft to max_route_ft), where the right answer is unsure. Unsafe pass: a pass where the survey fails. Missed review: a pass where the survey is borderline or review. Over-cautious: unsure or fail where the survey passes; false rejections are the fails among them. Decided without its measurement: a pass or fail with the run's measurement missing as failed or unsupported; a claimed absence also cannot support an at_most decision or an at_least fail. It is also scored as usual. An unsure is justified when the survey is borderline or review, or the run's measurement failed or is unsupported. The error could flip a check when it is at least as large as both the survey's distance to the nearest threshold and its ±. + +| Pipeline | Agree | Unsafe passes | Missed reviews | Over-cautious | False rejections | Decided without its measurement | Unsure, justified | Unsure, avoidable | Error could flip | +|---|---|---|---|---|---|---|---|---|---| +| `ar-taps` | 4/7 | 1 | 2 | 0 | 0 | 2 | 0 | 0 | 2 | +| `photo-depth` | 2/7 | 0 | 0 | 2 | 1 | 0 | 1 | 1 | 2 | +| `mesh-scaled` | no decisions | n/a | n/a | n/a | n/a | n/a | n/a | n/a | 0 | + +### Timing + +| Pipeline | Capture (s) | Processing (s) | +|---|---|---| +| `ar-taps` | 420.0 | 12.5 | +| `photo-depth` | 420.0 | 95.0 | +| `mesh-scaled` | 420.0 | not recorded | + +### Unsafe passes + +- `ar-taps` passed `facing_gap` at `c2`; the survey is fail at 2.500 ± 0.020 ft against `facing_gap_ft` at_least 3.000 ft (run measured no value). + +### Missed reviews + +- `ar-taps` passed `gas` at `c2`; the survey is borderline at 3.020 ± 0.030 ft against `gas_clearance_ft` at_least 3.000 ft (run measured 3.350 ft). +- `ar-taps` passed `route` at `c2`; the survey is review at 19.900 ± 0.050 ft against `review_route_ft` 15.000 ft and `max_route_ft` at_most 20.000 ft (run measured 19.500 ft). + +### Decided without its measurement + +- `ar-taps` reported pass for `facing_gap` at `c2` with measurement `c2-facing` missing (failed); the survey is fail at 2.500 ± 0.020 ft against `facing_gap_ft` at_least 3.000 ft. +- `ar-taps` reported fail for `pool` at `c2` with measurement `c2-pool` missing (failed); the survey is unknown (not measured) against `pool_clearance_ft` at_least 5.000 ft. + +These are paired results at the listed spots, a case series. They are not an accuracy rate for other homes, validated error bars, or evidence of safety. diff --git a/experiments/scoring/src/scoring/measure_lab.py b/experiments/scoring/src/scoring/measure_lab.py index 9ec7f4511..847a34fb5 100644 --- a/experiments/scoring/src/scoring/measure_lab.py +++ b/experiments/scoring/src/scoring/measure_lab.py @@ -9,6 +9,7 @@ import argparse import hashlib import json +import os import sys import tempfile import zipfile @@ -55,6 +56,10 @@ MAP_ENTRY_FIELDS = {"session_measurement", "key", "plus_minus_ft", "refusal"} +# A session zip holds every keyframe JPEG, and optional depth maps, so its size grows with the walk; +# hash it 1 MiB at a time instead of reading it whole. +HASH_CHUNK_BYTES = 1 << 20 + def meters_to_feet(meters: Decimal) -> Decimal: """Exact division by 0.3048, then half-up rounding to FEET_PLACES.""" @@ -120,11 +125,21 @@ def _session_member(archive: zipfile.ZipFile, path: Path) -> str: return found[0] -def load_session(path: Path) -> Session: +def sha256_file(path: Path) -> str: + """The file's sha256, read in HASH_CHUNK_BYTES pieces so a session zip with many keyframes is + never held in memory whole.""" + digest = hashlib.sha256() try: - raw_zip = path.read_bytes() + with path.open("rb") as handle: + while chunk := handle.read(HASH_CHUNK_BYTES): + digest.update(chunk) except OSError as error: raise InputError(f"{path}: cannot read ({error.strerror})") from None + return digest.hexdigest() + + +def load_session(path: Path) -> Session: + zip_sha256 = sha256_file(path) try: with zipfile.ZipFile(path) as archive: member = _session_member(archive, path) @@ -187,7 +202,7 @@ def load_session(path: Path) -> Session: return Session( zip_path=path, - zip_sha256=hashlib.sha256(raw_zip).hexdigest(), + zip_sha256=zip_sha256, member=member, id=session_id, started_at_uptime=started, @@ -434,6 +449,20 @@ def _string(value: str) -> str: return json.dumps(value, ensure_ascii=False) +def _refuse_output_over_input(out_path: Path, inputs: dict[str, Path]) -> None: + """Stop before any write when --out names one of the inputs, by path, symlink or hard link.""" + out_resolved = out_path.resolve() + for role, path in inputs.items(): + same = out_resolved == path.resolve() + if not same and out_path.exists() and path.exists(): + same = os.path.samefile(out_path, path) + if same: + raise InputError( + f"--out {out_path} is the {role} {path}; writing there would overwrite an input. " + "Choose another output path" + ) + + def import_session( session_path: Path, map_path: Path, @@ -443,6 +472,10 @@ def import_session( *, decide_outcomes: bool, ) -> dict[str, Any]: + _refuse_output_over_input( + out_path, + {"session zip": session_path, "map": map_path, "rules": rules_path, "truth": truth_path}, + ) rules = load_rules(rules_path) truth = load_truth(truth_path, rules) session = load_session(session_path) diff --git a/experiments/scoring/tests/test_measure_lab.py b/experiments/scoring/tests/test_measure_lab.py index 463002df3..3dc9839fe 100644 --- a/experiments/scoring/tests/test_measure_lab.py +++ b/experiments/scoring/tests/test_measure_lab.py @@ -1,16 +1,21 @@ """The Measure Lab importer on a synthetic format-2 session, with hand-computed feet. The committed session (fixtures/measure-lab/synthetic-session-01) holds, in meters: -m1 scale 1.524 (5 ft), m2 wall 9.87552 (32.4 ft), m3 c1 gas 1.4 (4.593175853... ft, written -4.593176), m4 c1 facing gap 1.49352 (4.9 ft), m5 c1 route 3.71856 (12.2 ft), m6 c2 gas 0.95 with -heightAboveGround -0.04 and accepted false, m7 c2 route 6.00456 (19.7 ft), and refusal r1 for the -c2 facing gap. The session starts at uptime 1000.25 and the last measurement is at 1412.75. +m1 scale 1.524 (5 ft) point to point; m2 wall 9.87552 (32.4 ft) along the wall; m3 c1 gas, straight +from the footprint edge p10 to the regulator p5, 1.4 (4.593175853... ft, written 4.593176); m4 c1 +facing, a gapToWall of 1.49352 (4.9 ft); m5 and m7, along-wall distances the map leaves unused; m6 +c2 gas, straight from p11 to p8, 0.95 but not accepted because p8 is on ARKit's estimated plane; +and refusal r1 for the c2 facing gap. The +map marks both routes unsupported, because Measure Lab records no routed cable path. The session +starts at uptime 1000.25 and the last measurement is at 1412.75. """ import copy import csv import hashlib import json +import math +import os from decimal import Decimal from pathlib import Path from typing import Any @@ -20,7 +25,13 @@ from scoring.cli import main from scoring.inputs import InputError, load_study -from scoring.measure_lab import import_session, meters_to_feet +from scoring.measure_lab import ( + HASH_CHUNK_BYTES, + import_session, + load_session, + meters_to_feet, + sha256_file, +) D = Decimal RULES = FIXTURES / "rules.json" @@ -126,8 +137,6 @@ def test_values_in_feet_with_the_mapped_uncertainty(self, setup: Setup): assert entries["wall-length"]["value_ft"] == D("32.400000") assert entries["c1-gas"]["value_ft"] == D("4.593176") assert entries["c1-facing"]["value_ft"] == D("4.900000") - assert entries["c1-route"]["value_ft"] == D("12.200000") - assert entries["c2-route"]["value_ft"] == D("19.700000") assert {entry.get("plus_minus_ft") for entry in entries.values() if entry["value_ft"]} == { D("0.3") } @@ -137,9 +146,11 @@ def test_missing_entries(self, setup: Setup): assert entries["c1-pool"] == {"id": "c1-pool", "value_ft": None, "missing": "absent"} assert entries["c2-facing"] == {"id": "c2-facing", "value_ft": None, "missing": "failed"} assert entries["c2-pool"] == {"id": "c2-pool", "value_ft": None, "missing": "unsupported"} + for route in ("c1-route", "c2-route"): + assert entries[route] == {"id": route, "value_ft": None, "missing": "unsupported"} def test_not_accepted_becomes_failed(self, setup: Setup): - # m6 has heightAboveGround -0.04 m, warning belowGround and accepted false. + # m6 starts at a ground point on ARKit's estimated plane: warning estimatedPlane. assert by_id(setup.run())["c2-gas"] == { "id": "c2-gas", "value_ft": None, @@ -152,11 +163,11 @@ def test_not_accepted_with_a_positive_value_also_becomes_failed(self, setup: Set assert by_id(setup.run())["c1-gas"]["missing"] == "failed" def test_accepted_straight_does_not_validate_unchecked_along_wall(self, setup: Setup): - m5 = setup.measurement("m5") - m5.update(compared="straight", accepted=True, warnings=[]) + m2 = setup.measurement("m2") + m2.update(compared="straight", accepted=True, warnings=[]) setup.session["walls"][0].update(validations=[], warnings=["wallNotValidated"]) error = setup.error() - assert "map.json: measurements.c1-route.key" in error + assert "map.json: measurements.wall-length.key" in error assert "'alongWall'" in error and "'straight'" in error def test_missing_compared_quantity_is_rejected(self, setup: Setup): @@ -196,11 +207,11 @@ def test_outcomes_follow_the_strict_rule(self, setup: Setup): assert outcomes == { ("c1", "gas"): "pass", # 4.593176 - 3 = 1.593176 > 0.3 ("c1", "facing_gap"): "pass", # 4.9 - 3 = 1.9 > 0.3 - ("c1", "route"): "pass", # 15 - 12.2 = 2.8 > 0.3 - ("c1", "pool"): "pass", # the rig saw no pool: an absent feature clears it + ("c1", "route"): "unsure", # unsupported: no routed path + ("c1", "pool"): "pass", # the operator asserted no pool: an absent feature clears it ("c2", "gas"): "unsure", # not accepted: no value ("c2", "facing_gap"): "unsure", # refused: no value - ("c2", "route"): "unsure", # 19.7: past 15 + 0.3, not past 20 + 0.3: review + ("c2", "route"): "unsure", # unsupported: no routed path ("c2", "pool"): "unsure", # unsupported } @@ -253,7 +264,7 @@ def edit_session(key: str, value: Any): "values key absent from the measurement", lambda s: s.map["measurements"]["c1-gas"].update(key="alongWall"), "measurements.c1-gas.key: measurement 'm3' has no 'alongWall' value; it has straight, " - "horizontal, vertical, gapToWall, heightAboveGround", + "horizontal, vertical", ), ("format version 1", edit_session("formatVersion", 1), "formatVersion is 1; this importer"), ("format version 3", edit_session("formatVersion", 3), "formatVersion is 3; this importer"), @@ -349,21 +360,28 @@ def test_end_to_end_scores_and_decide_row_lands_in_checks(tmp_path: Path, capsys assert main([*args, "--results", *map(str, outputs)]) == 0 stdout = capsys.readouterr().out assert ( - "| `measure-lab` | ar_poses | 5/9 | 1.20 | 2.40 | 5/5 | 0 | 2 | 0 | 0 | 1 | 1 |" in stdout + "| `measure-lab` | ar_poses | 3/9 | 1.20 | 1.20 | 3/3 | 2 | 2 | 0 | 0 | 1 | 1 |" in stdout ) - assert "| `measure-lab+rule` | 6/7 | 0 | 0 | 0 | 0 | 0 | 3 | 0 | 1 |" in stdout + assert "| `measure-lab+rule` | 5/7 | 0 | 0 | 1 | 0 | 0 | 4 | 0 | 0 |" in stdout assert "| `measure-lab` | no decisions |" in stdout with (csv_dir / "checks.csv").open(newline="") as handle: rows = [r for r in csv.DictReader(handle) if r["pipeline"] == "measure-lab+rule"] assert len(rows) == 8 - route = next(r for r in rows if (r["candidate"], r["check"]) == ("c2", "route")) - assert (route["truth_outcome"], route["run_outcome"], route["agrees"]) == ( + route = {r["candidate"]: r for r in rows if r["check"] == "route"} + # An unsupported route is a justified unsure: over-cautious at c1, where the survey passes, and + # correct at c2, where the survey is review. + assert [route["c1"][k] for k in ("truth_outcome", "run_outcome", "over_caution")] == [ + "pass", + "unsure", + "true", + ] + assert [route["c2"][k] for k in ("truth_outcome", "run_outcome", "agrees")] == [ "review", "unsure", "true", - ) - assert route["error_to_margin"] == "2.00" # 0.2 / max(min(0.1, 4.9), 0.05) + ] + assert route["c1"]["abstention"] == route["c2"]["abstention"] == "justified" def test_cli_reports_errors_with_exit_2(tmp_path: Path, capsys): @@ -372,3 +390,187 @@ def test_cli_reports_errors_with_exit_2(tmp_path: Path, capsys): assert code == 2 assert "score import-measure-lab:" in capsys.readouterr().err assert not (tmp_path / "o").exists() + + +# The committed session must hold only what Measure Lab's exporter at 43fda59 can write +# (measuredValues and measurementWarnings in MeasureGeometry, addMeasurement in LabSession). +POINT_TO_POINT = frozenset({"straight", "horizontal", "vertical"}) +POINT_TO_WALL = frozenset({"gapToWall", "heightAboveGround"}) +SESSION = committed_session() +WALL_BY_ID = {wall["id"]: wall for wall in SESSION["walls"]} +WALLS = frozenset(WALL_BY_ID) +POINTS = {point["id"]: point for point in SESSION["points"]} +WALL_WARNINGS = frozenset(warning for wall in SESSION["walls"] for warning in wall["warnings"]) + + +class TestFixtureMatchesTheExporter: + @pytest.mark.parametrize("measurement", SESSION["measurements"], ids=lambda m: m["id"]) + def test_quantity_set_follows_the_endpoints(self, measurement): + assert measurement["from"] in POINTS + if measurement["to"] in WALLS: + # Point to wall: facing gap and height above ground only, and no reference wall. + assert set(measurement["values"]) == POINT_TO_WALL + assert measurement["referenceWall"] is None + else: + # Point to point: straight, horizontal and vertical, plus alongWall exactly when a + # reference wall is recorded. + assert measurement["to"] in POINTS + expected = set(POINT_TO_POINT) + if measurement["referenceWall"] is not None: + assert measurement["referenceWall"] in WALLS + expected.add("alongWall") + assert set(measurement["values"]) == expected + + @pytest.mark.parametrize("measurement", SESSION["measurements"], ids=lambda m: m["id"]) + def test_compared_is_emitted_and_accepted_means_no_warnings(self, measurement): + values = measurement["values"] + assert measurement["compared"] in values + assert measurement["accepted"] == (measurement["warnings"] == []) + below = measurement["compared"] == "heightAboveGround" and values["heightAboveGround"] < 0 + assert ("belowGround" in measurement["warnings"]) == below + for key, value in values.items(): + assert key == "heightAboveGround" or value >= 0 + + @pytest.mark.parametrize("measurement", SESSION["measurements"], ids=lambda m: m["id"]) + def test_warnings_come_from_the_endpoints(self, measurement): + endpoints = [POINTS[measurement["from"]], POINTS.get(measurement["to"])] + inherited = { + flag for point in endpoints if point for flag in point["flags"] + point["wallWarnings"] + } + assert set(measurement["warnings"]) - {"belowGround"} <= inherited | WALL_WARNINGS + + @pytest.mark.parametrize("measurement", SESSION["measurements"], ids=lambda m: m["id"]) + def test_values_follow_from_the_coordinates(self, measurement): + # Recompute each value from point positions and the wall, as MeasureGeometry does. + a = POINTS[measurement["from"]]["position"] + if measurement["to"] in WALLS: + wall = WALL_BY_ID[measurement["to"]] + start, direction, normal = wall["start"], wall["direction"], wall["normal"] + rel = [a[i] - start[i] for i in range(3)] + along = sum(rel[i] * direction[i] for i in range(3)) + ground = start[1] + (wall["end"][1] - start[1]) * along / wall["length"] + expected = { + "gapToWall": abs(sum(rel[i] * normal[i] for i in range(3))), + "heightAboveGround": a[1] - ground, + } + else: + b = POINTS[measurement["to"]]["position"] + d = [b[i] - a[i] for i in range(3)] + expected = { + "straight": math.hypot(*d), + "horizontal": math.hypot(d[0], d[2]), + "vertical": abs(d[1]), + } + if measurement["referenceWall"] is not None: + direction = WALL_BY_ID[measurement["referenceWall"]]["direction"] + expected["alongWall"] = abs(sum(d[i] * direction[i] for i in range(3))) + for key, value in measurement["values"].items(): + assert value == pytest.approx(expected[key], abs=1e-4), key + + def test_refusals_name_a_real_tool(self): + tools = {"ground", "wall", "wallPoint", "twoView"} + assert {refusal["tool"] for refusal in SESSION["refusals"]} <= tools + + +def record_reads(monkeypatch, path: Path) -> tuple[list[int], list[int]]: + """Record each read's requested size and returned length on `path`, and fail on read_bytes.""" + sizes: list[int] = [] + lengths: list[int] = [] + real_open = Path.open + + def recording_open(self: Path, *args: Any, **kwargs: Any): + handle = real_open(self, *args, **kwargs) + if self != path: + return handle + real_read = handle.read + + def read(size: int = -1) -> bytes: + sizes.append(size) + data = real_read(size) + lengths.append(len(data)) + return data + + handle.read = read + return handle + + def no_read_bytes(self: Path) -> bytes: + raise AssertionError(f"{self} was read whole") + + monkeypatch.setattr(Path, "open", recording_open) + monkeypatch.setattr(Path, "read_bytes", no_read_bytes) + return sizes, lengths + + +def test_session_zip_is_hashed_in_bounded_chunks(tmp_path: Path, monkeypatch): + path = session_zip(committed_session(), tmp_path / "session.zip") + expected = hashlib.sha256(path.read_bytes()).hexdigest() + sizes, _ = record_reads(monkeypatch, path) + assert sha256_file(path) == expected + assert load_session(path).zip_sha256 == expected + assert sizes and all(size == HASH_CHUNK_BYTES for size in sizes) + + +def test_hash_accumulates_across_chunks(tmp_path: Path, monkeypatch): + # Two full chunks and a 17-byte tail, so the digest must combine three reads. + data = bytes(i % 251 for i in range(2 * HASH_CHUNK_BYTES + 17)) + path = tmp_path / "large.bin" + path.write_bytes(data) + expected = hashlib.sha256(data).hexdigest() + sizes, lengths = record_reads(monkeypatch, path) + assert sha256_file(path) == expected + assert sizes == [HASH_CHUNK_BYTES] * 4 + assert lengths == [HASH_CHUNK_BYTES, HASH_CHUNK_BYTES, 17, 0] + + +INPUT_ROLES = ("session zip", "map", "rules", "truth") + + +class TestOutputNeverOverwritesAnInput: + def inputs(self, root: Path) -> list[Path]: + paths = [root / name for name in ("session.zip", "map.json", "rules.json", "truth.json")] + paths[0].write_bytes(SESSION_ZIP.read_bytes()) + paths[1].write_bytes(MAP.read_bytes()) + paths[2].write_bytes(RULES.read_bytes()) + paths[3].write_bytes(TRUTH.read_bytes()) + return paths + + def check_refused(self, paths: list[Path], out: Path, role: str) -> None: + before = [path.read_bytes() for path in paths] + with pytest.raises(InputError, match=f"--out .* is the {role} "): + import_session(*paths, out, decide_outcomes=False) + assert [path.read_bytes() for path in paths] == before + + @pytest.mark.parametrize("role", INPUT_ROLES) + def test_same_path(self, tmp_path: Path, role: str): + paths = self.inputs(tmp_path) + self.check_refused(paths, paths[INPUT_ROLES.index(role)], role) + + @pytest.mark.parametrize("role", INPUT_ROLES) + def test_other_spelling_of_the_same_path(self, tmp_path: Path, role: str): + paths = self.inputs(tmp_path) + target = paths[INPUT_ROLES.index(role)] + (tmp_path / "sub").mkdir() + self.check_refused(paths, tmp_path / "sub" / ".." / target.name, role) + + @pytest.mark.parametrize("role", INPUT_ROLES) + def test_symlink(self, tmp_path: Path, role: str): + paths = self.inputs(tmp_path) + link = tmp_path / "link.json" + link.symlink_to(paths[INPUT_ROLES.index(role)]) + self.check_refused(paths, link, role) + + @pytest.mark.parametrize("role", INPUT_ROLES) + def test_hard_link(self, tmp_path: Path, role: str): + paths = self.inputs(tmp_path) + link = tmp_path / "hard.json" + os.link(paths[INPUT_ROLES.index(role)], link) + self.check_refused(paths, link, role) + + def test_cli_exit_2_and_inputs_intact(self, tmp_path: Path, capsys): + paths = self.inputs(tmp_path) + before = paths[1].read_bytes() + args = [str(paths[0]), "--map", str(paths[1]), "--rules", str(paths[2])] + code = main(["import-measure-lab", *args, "--truth", str(paths[3]), "--out", str(paths[1])]) + assert code == 2 + assert "would overwrite an input" in capsys.readouterr().err + assert paths[1].read_bytes() == before From 1905648ddbb0bc9db54d9286c74cf65a6a4fcc7f Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Sat, 26 Sep 2026 23:16:01 -0500 Subject: [PATCH 2/4] fix(scoring): reject unsupported route evidence and empty review bands Pin historical fixture sources so rule values remain traceable after documentation cleanup. Co-Authored-By: Claude Code --- experiments/scoring/FORMATS.md | 4 +- experiments/scoring/README.md | 2 +- .../scoring/fixtures/results/ar-taps.json | 2 +- .../scoring/fixtures/results/mesh-scaled.json | 2 +- .../scoring/fixtures/results/photo-depth.json | 2 +- experiments/scoring/fixtures/rules.json | 6 +-- .../scoring/results/synthetic-01/summary.md | 2 +- experiments/scoring/src/scoring/inputs.py | 10 +++-- .../scoring/src/scoring/measure_lab.py | 23 +++++++++++ experiments/scoring/src/scoring/metrics.py | 4 +- experiments/scoring/tests/test_inputs.py | 23 ++++++++++- experiments/scoring/tests/test_measure_lab.py | 40 +++++++++++++++---- experiments/scoring/tests/test_metrics.py | 6 +-- 13 files changed, 99 insertions(+), 27 deletions(-) diff --git a/experiments/scoring/FORMATS.md b/experiments/scoring/FORMATS.md index d3d6ad294..e0d0809dc 100644 --- a/experiments/scoring/FORMATS.md +++ b/experiments/scoring/FORMATS.md @@ -60,7 +60,7 @@ Write one survey file per house. | `candidates[]` | Spot `id`, its physical `marker`, and its `location` as tape offsets from permanent corners. | | `measurements[]` | `candidate` (null for house-level distances such as wall length), `from` and `to` in words, `status` (`measured`, `absent` or `not_measured`), `method`, `measured_by`. Only `measured` entries have `value_ft` and `plus_minus_ft`. | | `checks[]` | The spot, the check name, the deciding measurement and its `threshold`. Every spot needs the same check names with the same thresholds, so the denominator cannot shrink. An absent feature may decide only an `at_least` check. | -| `checks[].review_threshold` | Optional. It makes a band: values that clear `review_threshold` pass, values past `threshold` fail, and values between go to review. Both thresholds must point the same way, with the review line on the passing side. | +| `checks[].review_threshold` | Optional. It makes a band: values that clear `review_threshold` pass, values past `threshold` fail, and values between go to review. Both thresholds must point the same way, with the review line strictly on the passing side. Equal values are rejected, because they leave no band to review. | ## Results file @@ -126,7 +126,7 @@ This excerpt shows each kind of map entry. `fixtures/measure-lab/map.json` is a A session measurement has only the quantities its endpoints allow, as `experiments/measure-lab/SESSION-FORMAT.md` lists. Point to point gives `straight`, `horizontal` and `vertical`, plus `alongWall` when a reference wall is chosen. Point to wall gives `gapToWall` and `heightAboveGround`. -Map a route as `"unsupported"`. Measure Lab records no routed cable path, and an along-wall distance leaves out vertical legs and detours, so it is not a route length. +Map a route as `"unsupported"`. Measure Lab records no routed cable path, and an along-wall distance leaves out vertical legs and detours, so it is not a route length. The importer rejects any other entry for a survey measurement that decides a check named `route`, or a check that uses `max_route_ft` or `review_route_ft`. `"absent"` is the operator's assertion that the feature does not exist. Write it only after someone has looked over the whole area the check needs and found no such feature. Never infer it from a missing measurement or a refusal. In the scorer, an absent feature passes an `at_least` clearance, so a wrong `"absent"` can become an unsafe pass. diff --git a/experiments/scoring/README.md b/experiments/scoring/README.md index d636972e7..520b6e0f6 100644 --- a/experiments/scoring/README.md +++ b/experiments/scoring/README.md @@ -16,6 +16,6 @@ uv run pytest -q [FORMATS.md](FORMATS.md) specifies the input files and the Measure Lab importer. [METRICS.md](METRICS.md) explains every number. Real surveys and results go in git-ignored `data/`, the default `--out`. -**Result.** No real house scored yet. In [results/synthetic-01](results/synthetic-01/summary.md), three invented runs have median errors of 3.48, 12.60 and 1.20 in, and `ar-taps` makes 1 unsafe pass. Those numbers test the scorer, not any pipeline. All 274 tests and ruff pass. +**Result.** No real house scored yet. In [results/synthetic-01](results/synthetic-01/summary.md), three invented runs have median errors of 3.48, 12.60 and 1.20 in, and `ar-taps` makes 1 unsafe pass. Those numbers test the scorer, not any pipeline. All 285 tests and ruff pass. **What changed.** Nothing in the product or the plan yet. The field eval in PR #12 calls `score import-measure-lab` to score a Measure Lab session. diff --git a/experiments/scoring/fixtures/results/ar-taps.json b/experiments/scoring/fixtures/results/ar-taps.json index ad087cfb7..0172c9820 100644 --- a/experiments/scoring/fixtures/results/ar-taps.json +++ b/experiments/scoring/fixtures/results/ar-taps.json @@ -3,7 +3,7 @@ "unit": "ft", "pipeline": "ar-taps", "capture": "synthetic-01-walk-1", - "rules_sha256": "05b466b3436155e190513a21b6333304ad9da4e93f1b5022b1813a433bdb325e", + "rules_sha256": "487132e2ce0f1d686eb92dcd27efc2e2ce0af8a4c58681cbffb92a626064b89a", "scale_source": "ar_poses", "measurements": [ {"id": "scale", "value_ft": 6.000, "plus_minus_ft": 0.3}, diff --git a/experiments/scoring/fixtures/results/mesh-scaled.json b/experiments/scoring/fixtures/results/mesh-scaled.json index 6d97f29b5..67c5a42ca 100644 --- a/experiments/scoring/fixtures/results/mesh-scaled.json +++ b/experiments/scoring/fixtures/results/mesh-scaled.json @@ -3,7 +3,7 @@ "unit": "ft", "pipeline": "mesh-scaled", "capture": "synthetic-01-walk-1", - "rules_sha256": "05b466b3436155e190513a21b6333304ad9da4e93f1b5022b1813a433bdb325e", + "rules_sha256": "487132e2ce0f1d686eb92dcd27efc2e2ce0af8a4c58681cbffb92a626064b89a", "scale_source": "scale_reference", "measurements": [ {"id": "scale", "value_ft": 5.000, "plus_minus_ft": null}, diff --git a/experiments/scoring/fixtures/results/photo-depth.json b/experiments/scoring/fixtures/results/photo-depth.json index 3d0175d51..720c41958 100644 --- a/experiments/scoring/fixtures/results/photo-depth.json +++ b/experiments/scoring/fixtures/results/photo-depth.json @@ -3,7 +3,7 @@ "unit": "ft", "pipeline": "photo-depth", "capture": "synthetic-01-walk-1", - "rules_sha256": "05b466b3436155e190513a21b6333304ad9da4e93f1b5022b1813a433bdb325e", + "rules_sha256": "487132e2ce0f1d686eb92dcd27efc2e2ce0af8a4c58681cbffb92a626064b89a", "scale_source": "native_metric", "measurements": [ {"id": "wall-length", "value_ft": 30.500, "plus_minus_ft": 1.5}, diff --git a/experiments/scoring/fixtures/rules.json b/experiments/scoring/fixtures/rules.json index 823f2e343..c3b7ddeba 100644 --- a/experiments/scoring/fixtures/rules.json +++ b/experiments/scoring/fixtures/rules.json @@ -11,12 +11,12 @@ "facing_gap_ft": { "value_ft": 3, "pass_when": "at_least", - "source": "Synthetic placeholder; the public passage width is TBD in docs/01-feature-map.md" + "source": "Synthetic placeholder; the public passage width is TBD in https://github.com/SamGu-NRX/house-scanning-master/blob/9737e3f0eefe90f2a12a190bf8750e7fed64413f/docs/01-feature-map.md#L97" }, "review_route_ft": { "value_ft": 15, "pass_when": "at_most", - "source": "Synthetic placeholder; the confident reach before review, per docs/02-implementation-plan.md Lane C" + "source": "Synthetic placeholder; the confident reach before review, per Lane C in https://github.com/SamGu-NRX/house-scanning-master/blob/9737e3f0eefe90f2a12a190bf8750e7fed64413f/docs/02-implementation-plan.md#L114" }, "max_route_ft": { "value_ft": 20, @@ -26,7 +26,7 @@ "pool_clearance_ft": { "value_ft": 5, "pass_when": "at_least", - "source": "Synthetic placeholder; no public value, TBD in docs/01-feature-map.md" + "source": "Synthetic placeholder; no public value, TBD in https://github.com/SamGu-NRX/house-scanning-master/blob/9737e3f0eefe90f2a12a190bf8750e7fed64413f/docs/01-feature-map.md#L96" } } } diff --git a/experiments/scoring/results/synthetic-01/summary.md b/experiments/scoring/results/synthetic-01/summary.md index d44b16bab..1cef3f10c 100644 --- a/experiments/scoring/results/synthetic-01/summary.md +++ b/experiments/scoring/results/synthetic-01/summary.md @@ -1,6 +1,6 @@ # Capture pipeline scores -Rules: `rules.json` (Synthetic scoring fixture. Not a real placement policy.), sha256 `05b466b3436155e190513a21b6333304ad9da4e93f1b5022b1813a433bdb325e`. +Rules: `rules.json` (Synthetic scoring fixture. Not a real placement policy.), sha256 `487132e2ce0f1d686eb92dcd27efc2e2ce0af8a4c58681cbffb92a626064b89a`. 1 house, 2 candidate spots, 3 pipeline runs. Each house's declared scale reference is left out of every error figure: a run that was given it can match it exactly, so its error says nothing about accuracy. diff --git a/experiments/scoring/src/scoring/inputs.py b/experiments/scoring/src/scoring/inputs.py index 6886787a0..5c45bceaa 100644 --- a/experiments/scoring/src/scoring/inputs.py +++ b/experiments/scoring/src/scoring/inputs.py @@ -522,15 +522,17 @@ def _validate_review_band(fields: Fields, review_name: str, fail: Threshold, rul f"{fail.pass_when}; a review band needs both in the same direction", "review_threshold", ) + # Strict: an equal value leaves no band, and values up to the fail line would pass unreviewed. inside = ( - review.value_ft <= fail.value_ft + review.value_ft < fail.value_ft if fail.pass_when == "at_most" - else review.value_ft >= fail.value_ft + else review.value_ft > fail.value_ft ) if not inside: raise fields.error( - f"{review.name!r} ({review.value_ft} ft) must be on the passing side of " - f"{fail.name!r} ({fail.value_ft} ft, {fail.pass_when})", + f"{review.name!r} ({review.value_ft} ft) must be strictly on the passing side of " + f"{fail.name!r} ({fail.value_ft} ft, {fail.pass_when}); equal values leave no " + "band to review", "review_threshold", ) diff --git a/experiments/scoring/src/scoring/measure_lab.py b/experiments/scoring/src/scoring/measure_lab.py index 847a34fb5..e974b6e01 100644 --- a/experiments/scoring/src/scoring/measure_lab.py +++ b/experiments/scoring/src/scoring/measure_lab.py @@ -56,6 +56,11 @@ MAP_ENTRY_FIELDS = {"session_measurement", "key", "plus_minus_ft", "refusal"} +# The route check and its thresholds, named as in the scoring protocol. Measure Lab has no tool +# that records a routed cable path, so a route may only be mapped "unsupported". +ROUTE_CHECK = "route" +ROUTE_THRESHOLDS = frozenset({"max_route_ft", "review_route_ft"}) + # A session zip holds every keyframe JPEG, and optional depth maps, so its size grows with the walk; # hash it 1 MiB at a time instead of reading it whole. HASH_CHUNK_BYTES = 1 << 20 @@ -281,6 +286,17 @@ def load_map(path: Path) -> Map: return Map(path, top.text("pipeline"), top.text("session"), entries) +def _route_measurements(truth: Truth) -> set[str]: + """Survey measurements that decide a route check, by check name or by a route threshold.""" + return { + check.measurement + for check in truth.checks + if check.check == ROUTE_CHECK + or check.threshold in ROUTE_THRESHOLDS + or check.review_threshold in ROUTE_THRESHOLDS + } + + def _check_map(mapping: Map, session: Session, truth: Truth) -> None: where = str(mapping.path) if mapping.session != session.id: @@ -301,8 +317,15 @@ def _check_map(mapping: Map, session: Session, truth: Truth) -> None: f"{where}: no entry for survey measurements {', '.join(map(repr, left_out))}; map " 'each to a session measurement, a refusal, "absent" or "unsupported"' ) + routes = _route_measurements(truth) for survey_id, entry in mapping.entries.items(): at = f"{where}: measurements.{survey_id}" + if survey_id in routes and entry != "unsupported": + raise InputError( + f'{at}: {survey_id!r} decides a route check, so map it as "unsupported". Measure ' + "Lab records no routed cable path, and a point or wall distance leaves out the " + "route's vertical legs and detours" + ) if isinstance(entry, FromRefusal) and entry.refusal not in session.refusals: raise InputError(f"{at}.refusal: {session.member} has no refusal {entry.refusal!r}") if not isinstance(entry, FromSession): diff --git a/experiments/scoring/src/scoring/metrics.py b/experiments/scoring/src/scoring/metrics.py index d426aeec6..d48b8c2c2 100644 --- a/experiments/scoring/src/scoring/metrics.py +++ b/experiments/scoring/src/scoring/metrics.py @@ -103,8 +103,8 @@ def truth_outcome( The check passes when the margin to the pass line is larger than u and fails when the margin to the fail line is below -u. The fail line is `threshold`. The pass line is `review` when the check has a review band and `threshold` otherwise. Anything else is borderline for a - single threshold, or review for a band, as docs/02-implementation-plan.md "Lane C" sends a - route past the confident reach to UNSURE. + single threshold, or review for a band, as the plan's Lane C sends a route past the confident + reach to UNSURE (https://github.com/SamGu-NRX/house-scanning-master/blob/9737e3f0eefe90f2a12a190bf8750e7fed64413f/docs/02-implementation-plan.md#L114). Both comparisons are strict, following Lane C: "A check answers PASS if the margin is larger than the error." A value exactly on a line, or exactly u from it, is borderline or review, diff --git a/experiments/scoring/tests/test_inputs.py b/experiments/scoring/tests/test_inputs.py index 6b713b2cd..de09dceef 100644 --- a/experiments/scoring/tests/test_inputs.py +++ b/experiments/scoring/tests/test_inputs.py @@ -209,7 +209,28 @@ def make_route_absent(files: Files) -> None: f.rules["thresholds"]["review_route_ft"].update(value_ft=25), f.truth["checks"][1].update(review_threshold="review_route_ft"), ], - "'review_route_ft' (25 ft) must be on the passing side of 'max_route_ft' (20 ft, at_most)", + "'review_route_ft' (25 ft) must be strictly on the passing side of 'max_route_ft' " + "(20 ft, at_most)", + ), + ( + "review threshold equal to an at_most fail line", + lambda f: [ + f.rules["thresholds"]["review_route_ft"].update(value_ft=20), + f.truth["checks"][1].update(review_threshold="review_route_ft"), + ], + "'review_route_ft' (20 ft) must be strictly on the passing side of 'max_route_ft' " + "(20 ft, at_most); equal values leave no band to review", + ), + ( + "review threshold equal to an at_least fail line", + lambda f: [ + f.rules["thresholds"].update( + review_gas_ft={"value_ft": 3.0, "pass_when": "at_least", "source": "synthetic"} + ), + f.truth["checks"][0].update(review_threshold="review_gas_ft"), + ], + "'review_gas_ft' (3.0 ft) must be strictly on the passing side of 'gas_clearance_ft' " + "(3 ft, at_least); equal values leave no band to review", ), ("absent at_most", make_route_absent, "only a clearance (at_least) passes"), ( diff --git a/experiments/scoring/tests/test_measure_lab.py b/experiments/scoring/tests/test_measure_lab.py index 3dc9839fe..4ed8daa57 100644 --- a/experiments/scoring/tests/test_measure_lab.py +++ b/experiments/scoring/tests/test_measure_lab.py @@ -24,9 +24,10 @@ from helpers import FIXTURES, MEASURE_LAB, SESSION_ZIP, committed_session, session_zip from scoring.cli import main -from scoring.inputs import InputError, load_study +from scoring.inputs import InputError, PipelineMeasurement, Threshold, load_study from scoring.measure_lab import ( HASH_CHUNK_BYTES, + _rule_outcome, import_session, load_session, meters_to_feet, @@ -231,12 +232,10 @@ def test_equality_is_unsure(self, setup: Setup, meters, outcome): } assert outcomes[("c1", "gas")] == outcome - def test_absent_cannot_pass_an_at_most_rule(self, setup: Setup): - setup.map["measurements"]["c1-route"] = "absent" - outcomes = { - (o["candidate"], o["check"]): o["outcome"] for o in setup.run(decide=True)["outcomes"] - } - assert outcomes[("c1", "route")] == "unsure" + def test_absent_cannot_pass_an_at_most_rule(self): + limit = Threshold("max_route_ft", D(20), "at_most", "synthetic") + absent = PipelineMeasurement("c1-route", None, None, "absent") + assert _rule_outcome(absent, limit, None) == "unsure" def edit_session(key: str, value: Any): @@ -322,6 +321,33 @@ def test_errors(setup: Setup, edit, expected): assert expected in setup.error() +@pytest.mark.parametrize( + "entry", + [ + {"session_measurement": "m5", "key": "alongWall"}, # accepted, compared alongWall + {"session_measurement": "m2", "key": "alongWall"}, + {"refusal": "r1"}, + "absent", + ], + ids=["along-wall m5", "along-wall m2", "refusal", "absent"], +) +@pytest.mark.parametrize("route", ["c1-route", "c2-route"]) +def test_route_must_be_unsupported(setup: Setup, route: str, entry: Any): + setup.map["measurements"][route] = entry + error = setup.error(decide=True) + assert f'measurements.{route}: {route!r} decides a route check, so map it as "unsupported"' in ( + error + ) + + +def test_route_is_recognised_by_its_threshold_under_another_check_name(setup: Setup): + for check in setup.truth["checks"]: + if check["check"] == "route": + check["check"] = "cable" + setup.map["measurements"]["c1-route"] = {"session_measurement": "m5", "key": "alongWall"} + assert "'c1-route' decides a route check" in setup.error() + + def test_zip_without_session_json(tmp_path: Path): path = tmp_path / "empty.zip" session_zip(b"{}", path, folder="a/b") # session.json two folders down is not the session diff --git a/experiments/scoring/tests/test_metrics.py b/experiments/scoring/tests/test_metrics.py index aa22bda26..77f0a2d1f 100644 --- a/experiments/scoring/tests/test_metrics.py +++ b/experiments/scoring/tests/test_metrics.py @@ -22,9 +22,9 @@ class TestTruthOutcome: - # Strict, per docs/02-implementation-plan.md Lane C: pass when the margin s > u, fail when - # s < -u, borderline otherwise. A value exactly on the threshold, or exactly u from it, is - # borderline, including when u = 0. + # Strict, per docs/00-overview.md ("Every check answers PASS, FAIL or UNSURE"): pass when + # the margin s > u, fail when s < -u, borderline otherwise. A value exactly on the threshold, + # or exactly u from it, is borderline, including when u = 0. @pytest.mark.parametrize( ("value", "plus_minus", "expected"), From 1030edb4e4e352de1adeb484bffee70bbfe2636c Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Sat, 26 Sep 2026 23:35:32 -0500 Subject: [PATCH 3/4] fix(scoring): validate session geometry and bound decompression Co-Authored-By: Claude Code --- experiments/scoring/FORMATS.md | 4 +- experiments/scoring/README.md | 2 +- .../scoring/src/scoring/measure_lab.py | 100 ++++++++++++- experiments/scoring/tests/test_measure_lab.py | 141 +++++++++++++++++- 4 files changed, 232 insertions(+), 15 deletions(-) diff --git a/experiments/scoring/FORMATS.md b/experiments/scoring/FORMATS.md index e0d0809dc..281f51f37 100644 --- a/experiments/scoring/FORMATS.md +++ b/experiments/scoring/FORMATS.md @@ -124,7 +124,7 @@ This excerpt shows each kind of map entry. `fixtures/measure-lab/map.json` is a | `{"refusal": id}` | Null, missing `failed`. | | `"absent"` or `"unsupported"` | Null, missing `absent` or `unsupported`. | -A session measurement has only the quantities its endpoints allow, as `experiments/measure-lab/SESSION-FORMAT.md` lists. Point to point gives `straight`, `horizontal` and `vertical`, plus `alongWall` when a reference wall is chosen. Point to wall gives `gapToWall` and `heightAboveGround`. +A session measurement has only the quantities its endpoints allow, as `experiments/measure-lab/SESSION-FORMAT.md` lists. Point to point gives `straight`, `horizontal` and `vertical`, plus `alongWall` when a reference wall is chosen. Point to wall gives `gapToWall` and `heightAboveGround`. The importer rejects a session whose measurement has any other set of values, an endpoint missing from `points` or `walls`, or a `compared` quantity it does not hold. Map a route as `"unsupported"`. Measure Lab records no routed cable path, and an along-wall distance leaves out vertical legs and detours, so it is not a route length. The importer rejects any other entry for a survey measurement that decides a check named `route`, or a check that uses `max_route_ft` or `review_route_ft`. @@ -132,7 +132,7 @@ Map a route as `"unsupported"`. Measure Lab records no routed cable path, and an Every value needs a stated uncertainty: its entry's `plus_minus_ft`, or its key's value in `plus_minus_ft_by_key`. There is no default. The ±0.3 ft in the example is the plan's untested error bar for an AR tap, listed in `docs/00-overview.md` under "Conventions the code relies on". It is not a measured error bar. `session` must match the zip's session id. -The importer converts meters exactly (1 ft = 0.3048 m) and rounds half up to a millionth of a foot. The capture id is the zip's sha256, which must already be in the survey's `captures`. `scale_source` is `ar_poses`. `capture_s` runs from the session's start to its last measurement. `processing_s` is null, because the app shows each value as it is tapped. A map or session problem stops the import with a specific message. The importer loads its output through the scorer before it writes the file. +The importer converts meters exactly (1 ft = 0.3048 m) and rounds half up to a millionth of a foot. The capture id is the zip's sha256, which must already be in the survey's `captures`. The importer checks that before it unpacks anything, and it refuses a `session.json` larger than 16 MiB. That limit is a chosen safety bound, not calibrated against real captures. `scale_source` is `ar_poses`. `capture_s` runs from the session's start to its last measurement. `processing_s` is null, because the app shows each value as it is tapped. A map or session problem stops the import with a specific message. The importer loads its output through the scorer before it writes the file. `--decide` also writes a row named `+rule`. Its outcomes apply the survey's strict rule, described in [METRICS.md](METRICS.md), to the run's own values and uncertainties. With no value the row says unsure, except that an absent feature passes an `at_least` check. The row emulates the check rule in `docs/00-overview.md` ("Every check answers PASS, FAIL or UNSURE") until a real solver exists. diff --git a/experiments/scoring/README.md b/experiments/scoring/README.md index 520b6e0f6..d11f33ff5 100644 --- a/experiments/scoring/README.md +++ b/experiments/scoring/README.md @@ -16,6 +16,6 @@ uv run pytest -q [FORMATS.md](FORMATS.md) specifies the input files and the Measure Lab importer. [METRICS.md](METRICS.md) explains every number. Real surveys and results go in git-ignored `data/`, the default `--out`. -**Result.** No real house scored yet. In [results/synthetic-01](results/synthetic-01/summary.md), three invented runs have median errors of 3.48, 12.60 and 1.20 in, and `ar-taps` makes 1 unsafe pass. Those numbers test the scorer, not any pipeline. All 285 tests and ruff pass. +**Result.** No real house scored yet. In [results/synthetic-01](results/synthetic-01/summary.md), three invented runs have median errors of 3.48, 12.60 and 1.20 in, and `ar-taps` makes 1 unsafe pass. Those numbers test the scorer, not any pipeline. All 303 tests and ruff pass. **What changed.** Nothing in the product or the plan yet. The field eval in PR #12 calls `score import-measure-lab` to score a Measure Lab session. diff --git a/experiments/scoring/src/scoring/measure_lab.py b/experiments/scoring/src/scoring/measure_lab.py index e974b6e01..af7561c73 100644 --- a/experiments/scoring/src/scoring/measure_lab.py +++ b/experiments/scoring/src/scoring/measure_lab.py @@ -3,7 +3,8 @@ The rig's measurements carry app-generated ids and meter values. A map file, written by hand after the walk, says which session measurement and `values` key answers each survey measurement, and with what uncertainty. The session format is Measure Lab's formatVersion 2, documented in -experiments/measure-lab/README.md ("Session format"). Only the fields used here are validated. +experiments/measure-lab/README.md ("Session format"). Only the fields used here are validated, +including each measurement's endpoints and the value keys they allow. """ import argparse @@ -65,6 +66,15 @@ # hash it 1 MiB at a time instead of reading it whole. HASH_CHUNK_BYTES = 1 << 20 +# The most session.json may decompress to, so a crafted zip member cannot exhaust memory. A chosen +# safety bound, not calibrated against real captures: no field session has been measured yet. +MAX_SESSION_JSON_BYTES = 16 << 20 + +# The quantities Measure Lab's exporter (measuredValues in MeasureGeometry) writes for each kind of +# measurement. Point to point also gets alongWall exactly when it names a reference wall. +POINT_TO_POINT_KEYS = frozenset({"straight", "horizontal", "vertical"}) +POINT_TO_WALL_KEYS = frozenset({"gapToWall", "heightAboveGround"}) + def meters_to_feet(meters: Decimal) -> Decimal: """Exact division by 0.3048, then half-up rounding to FEET_PLACES.""" @@ -143,15 +153,87 @@ def sha256_file(path: Path) -> str: return digest.hexdigest() -def load_session(path: Path) -> Session: +def _ids(data: dict[str, Any], key: str, where: str) -> set[str]: + ids: set[str] = set() + for index, entry in enumerate(_list(data, key, where)): + if not isinstance(entry, dict): + raise InputError(f"{where}: {key}[{index}]: expected an object") + entry_id = _text(entry.get("id"), f"{where}: {key}[{index}].id") + if entry_id in ids: + raise InputError(f"{where}: {key}[{index}].id: {entry_id!r} is listed twice") + ids.add(entry_id) + return ids + + +def _check_endpoints( + entry: dict[str, Any], + measurement: SessionMeasurement, + points: set[str], + walls: set[str], + at: str, +) -> None: + """The endpoints and value keys must be a pair Measure Lab's exporter can write.""" + source = _text(entry.get("from"), f"{at}.from") + if source not in points: + raise InputError(f"{at}.from: {source!r} is not a point id in points") + target = _text(entry.get("to"), f"{at}.to") + # The exporter omits a nil referenceWall, so a missing key and null mean the same. + reference = entry.get("referenceWall") + if reference is not None: + reference = _text(reference, f"{at}.referenceWall") + if target in walls: + if reference is not None: + raise InputError( + f"{at}.referenceWall: a point-to-wall measurement has no reference wall, got " + f"{reference!r}" + ) + expected = POINT_TO_WALL_KEYS + kind = "point to wall" + elif target in points: + if reference is None: + expected = POINT_TO_POINT_KEYS + elif reference in walls: + expected = POINT_TO_POINT_KEYS | {"alongWall"} + else: + raise InputError(f"{at}.referenceWall: {reference!r} is not a wall id in walls") + kind = "point to point" + (" with a reference wall" if reference is not None else "") + else: + raise InputError(f"{at}.to: {target!r} is not a point or wall id") + keys = set(measurement.values) + if keys != expected: + raise InputError( + f"{at}.values: a {kind} measurement has exactly {', '.join(sorted(expected))}, got " + f"{', '.join(sorted(keys)) or 'none'}; Measure Lab does not write this combination" + ) + if measurement.compared not in keys: + raise InputError( + f"{at}.compared: {measurement.compared!r} is not one of its values " + f"({', '.join(sorted(keys))})" + ) + + +def load_session(path: Path, truth: Truth) -> Session: + """Read a session zip listed in the survey's captures. The listing is checked from the zip's + hash before anything is decompressed, so an unvetted zip is never unpacked.""" zip_sha256 = sha256_file(path) + if zip_sha256 not in truth.captures: + raise InputError( + f"{truth.path}: captures does not list {zip_sha256}, the sha256 of {path}; add it so " + "this run is scored against this survey" + ) try: with zipfile.ZipFile(path) as archive: member = _session_member(archive, path) - raw = archive.read(member) + with archive.open(member) as handle: + raw = handle.read(MAX_SESSION_JSON_BYTES + 1) except zipfile.BadZipFile: raise InputError(f"{path}: not a zip file") from None where = f"{path}!{member}" + if len(raw) > MAX_SESSION_JSON_BYTES: + raise InputError( + f"{where}: session.json is larger than {MAX_SESSION_JSON_BYTES} bytes, the importer's " + "limit; split the capture into shorter sessions" + ) data = parse_json(raw, where) if not isinstance(data, dict): raise InputError(f"{where}: expected a JSON object") @@ -174,6 +256,12 @@ def load_session(path: Path) -> Session: session_id = _text(info.get("id"), f"{where}: session.id") started = _number(info.get("startedAtUptime"), f"{where}: session.startedAtUptime") + points = _ids(data, "points", where) + walls = _ids(data, "walls", where) + shared = sorted(points & walls) + if shared: + raise InputError(f"{where}: {', '.join(map(repr, shared))} is both a point and a wall id") + measurements: dict[str, SessionMeasurement] = {} for index, entry in enumerate(_list(data, "measurements", where)): at = f"{where}: measurements[{index}]" @@ -188,7 +276,7 @@ def load_session(path: Path) -> Session: accepted = entry.get("accepted") if not isinstance(accepted, bool): raise InputError(f"{at} ({measurement_id}).accepted: expected true or false") - measurements[measurement_id] = SessionMeasurement( + measurement = SessionMeasurement( id=measurement_id, time=_number(entry.get("time"), f"{at} ({measurement_id}).time"), values={ @@ -198,6 +286,8 @@ def load_session(path: Path) -> Session: compared=_text(entry.get("compared"), f"{at} ({measurement_id}).compared"), accepted=accepted, ) + _check_endpoints(entry, measurement, points, walls, f"{at} ({measurement_id})") + measurements[measurement_id] = measurement refusals: set[str] = set() for index, entry in enumerate(_list(data, "refusals", where)): @@ -501,7 +591,7 @@ def import_session( ) rules = load_rules(rules_path) truth = load_truth(truth_path, rules) - session = load_session(session_path) + session = load_session(session_path, truth) results = build_results( session, load_map(map_path), rules, truth, decide_outcomes=decide_outcomes ) diff --git a/experiments/scoring/tests/test_measure_lab.py b/experiments/scoring/tests/test_measure_lab.py index 4ed8daa57..8221db9c6 100644 --- a/experiments/scoring/tests/test_measure_lab.py +++ b/experiments/scoring/tests/test_measure_lab.py @@ -12,10 +12,12 @@ import copy import csv +import dataclasses import hashlib import json import math import os +import zipfile from decimal import Decimal from pathlib import Path from typing import Any @@ -23,8 +25,16 @@ import pytest from helpers import FIXTURES, MEASURE_LAB, SESSION_ZIP, committed_session, session_zip +from scoring import measure_lab from scoring.cli import main -from scoring.inputs import InputError, PipelineMeasurement, Threshold, load_study +from scoring.inputs import ( + InputError, + PipelineMeasurement, + Threshold, + load_rules, + load_study, + load_truth, +) from scoring.measure_lab import ( HASH_CHUNK_BYTES, _rule_outcome, @@ -307,6 +317,86 @@ def edit_session(key: str, value: Any): lambda s: s.measurement("m3")["values"].update(straight=-1.4), "measurement 'm3' is accepted but its straight is -1.4 m", ), + ( + "point to wall carrying straight", + lambda s: [ + s.measurement("m4")["values"].update(straight=3.0), + s.measurement("m4").update(compared="straight"), + ], + "measurements[3] (m4).values: a point to wall measurement has exactly gapToWall, " + "heightAboveGround, got gapToWall, heightAboveGround, straight", + ), + ( + "compared not among the values", + lambda s: s.measurement("m4").update(compared="straight"), + "measurements[3] (m4).compared: 'straight' is not one of its values", + ), + ( + "unknown target", + lambda s: s.measurement("m4").update(to="no-such-wall"), + "measurements[3] (m4).to: 'no-such-wall' is not a point or wall id", + ), + ( + "unknown source", + lambda s: s.measurement("m3").update({"from": "p99"}), + "measurements[2] (m3).from: 'p99' is not a point id in points", + ), + ( + "along-wall without a reference wall", + lambda s: s.measurement("m5").update(referenceWall=None), + "measurements[4] (m5).values: a point to point measurement has exactly horizontal, " + "straight, vertical, got alongWall, horizontal, straight, vertical", + ), + ( + "reference wall missing from walls", + lambda s: s.measurement("m5").update(referenceWall="w9"), + "measurements[4] (m5).referenceWall: 'w9' is not a wall id in walls", + ), + ( + "reference wall on a point-to-wall measurement", + lambda s: s.measurement("m4").update(referenceWall="w1"), + "measurements[3] (m4).referenceWall: a point-to-wall measurement has no reference wall", + ), + ( + "from as a list", + lambda s: s.measurement("m3").update({"from": ["w1"]}), + "measurements[2] (m3).from: expected a non-empty string, got ['w1']", + ), + ( + "from as a object", + lambda s: s.measurement("m3").update({"from": {"id": "w1"}}), + "measurements[2] (m3).from: expected a non-empty string, got {'id': 'w1'}", + ), + ( + "to as a list", + lambda s: s.measurement("m4").update({"to": ["w1"]}), + "measurements[3] (m4).to: expected a non-empty string, got ['w1']", + ), + ( + "to as a object", + lambda s: s.measurement("m4").update({"to": {"id": "w1"}}), + "measurements[3] (m4).to: expected a non-empty string, got {'id': 'w1'}", + ), + ( + "referenceWall as a list", + lambda s: s.measurement("m5").update({"referenceWall": ["w1"]}), + "measurements[4] (m5).referenceWall: expected a non-empty string, got ['w1']", + ), + ( + "referenceWall as a object", + lambda s: s.measurement("m5").update({"referenceWall": {"id": "w1"}}), + "measurements[4] (m5).referenceWall: expected a non-empty string, got {'id': 'w1'}", + ), + ( + "id shared by a point and a wall", + lambda s: s.session["points"].append({**s.session["points"][0], "id": "w1"}), + "'w1' is both a point and a wall id", + ), + ( + "duplicate point id", + lambda s: s.session["points"].append(copy.deepcopy(s.session["points"][0])), + "points[11].id: 'p1' is listed twice", + ), ( "length units", lambda s: s.session["units"].update(length="feet"), @@ -348,25 +438,61 @@ def test_route_is_recognised_by_its_threshold_under_another_check_name(setup: Se assert "'c1-route' decides a route check" in setup.error() +def test_omitted_reference_wall_is_the_exporter_shape(setup: Setup): + # Swift's synthesized Codable leaves a nil referenceWall out instead of writing null. + for measurement in setup.session["measurements"]: + if measurement["referenceWall"] is None: + del measurement["referenceWall"] + assert by_id(setup.run())["c1-gas"]["value_ft"] == D("4.593176") + + +def test_oversized_session_json_is_refused(setup: Setup, monkeypatch): + monkeypatch.setattr(measure_lab, "MAX_SESSION_JSON_BYTES", 100) + assert "session.json is larger than 100 bytes, the importer's limit" in setup.error() + + +def test_session_json_at_the_limit_is_read(setup: Setup, monkeypatch): + size = len((json.dumps(setup.session, indent=2) + "\n").encode()) + monkeypatch.setattr(measure_lab, "MAX_SESSION_JSON_BYTES", size) + assert setup.run()["pipeline"] == "measure-lab" + + +def listed_truth(tmp_path: Path, zip_path: Path) -> Path: + """A copy of the survey whose captures list this zip, so the import gets past the listing.""" + truth = json.loads(TRUTH.read_text()) + truth["captures"].append(hashlib.sha256(zip_path.read_bytes()).hexdigest()) + path = tmp_path / "truth.json" + path.write_text(json.dumps(truth)) + return path + + def test_zip_without_session_json(tmp_path: Path): path = tmp_path / "empty.zip" session_zip(b"{}", path, folder="a/b") # session.json two folders down is not the session + truth = listed_truth(tmp_path, path) with pytest.raises(InputError, match=r"expected exactly one session\.json"): - import_session(path, MAP, RULES, TRUTH, tmp_path / "out.json", decide_outcomes=False) + import_session(path, MAP, RULES, truth, tmp_path / "out.json", decide_outcomes=False) def test_not_a_zip(tmp_path: Path): path = tmp_path / "session.zip" path.write_text("not a zip") + truth = listed_truth(tmp_path, path) with pytest.raises(InputError, match="not a zip file"): - import_session(path, MAP, RULES, TRUTH, tmp_path / "out.json", decide_outcomes=False) + import_session(path, MAP, RULES, truth, tmp_path / "out.json", decide_outcomes=False) -def test_capture_must_be_in_the_survey(tmp_path: Path): +def test_capture_must_be_in_the_survey(tmp_path: Path, monkeypatch): session = copy.deepcopy(committed_session()) session["session"]["appVersion"] = "changed" # different bytes, different sha256 path = session_zip(session, tmp_path / "session.zip") capture = hashlib.sha256(path.read_bytes()).hexdigest() + + def no_unpacking(*args: Any, **kwargs: Any) -> Any: + raise AssertionError("an unlisted zip was opened") + + # The listing is checked before the zip is opened, so nothing in it is decompressed. + monkeypatch.setattr(zipfile, "ZipFile", no_unpacking) with pytest.raises(InputError, match=f"captures does not list {capture}"): import_session(path, MAP, RULES, TRUTH, tmp_path / "out.json", decide_outcomes=False) @@ -420,8 +546,8 @@ def test_cli_reports_errors_with_exit_2(tmp_path: Path, capsys): # The committed session must hold only what Measure Lab's exporter at 43fda59 can write # (measuredValues and measurementWarnings in MeasureGeometry, addMeasurement in LabSession). -POINT_TO_POINT = frozenset({"straight", "horizontal", "vertical"}) -POINT_TO_WALL = frozenset({"gapToWall", "heightAboveGround"}) +POINT_TO_POINT = measure_lab.POINT_TO_POINT_KEYS +POINT_TO_WALL = measure_lab.POINT_TO_WALL_KEYS SESSION = committed_session() WALL_BY_ID = {wall["id"]: wall for wall in SESSION["walls"]} WALLS = frozenset(WALL_BY_ID) @@ -530,9 +656,10 @@ def no_read_bytes(self: Path) -> bytes: def test_session_zip_is_hashed_in_bounded_chunks(tmp_path: Path, monkeypatch): path = session_zip(committed_session(), tmp_path / "session.zip") expected = hashlib.sha256(path.read_bytes()).hexdigest() + truth = dataclasses.replace(load_truth(TRUTH, load_rules(RULES)), captures=(expected,)) sizes, _ = record_reads(monkeypatch, path) assert sha256_file(path) == expected - assert load_session(path).zip_sha256 == expected + assert load_session(path, truth).zip_sha256 == expected assert sizes and all(size == HASH_CHUNK_BYTES for size in sizes) From a575437acb717477e6d42a8c589df6541baf459c Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Sun, 27 Sep 2026 00:00:57 -0500 Subject: [PATCH 4/4] fix(scoring): protect report inputs and escape Markdown values Co-Authored-By: Claude Code --- experiments/scoring/FORMATS.md | 2 +- experiments/scoring/METRICS.md | 2 +- experiments/scoring/README.md | 2 +- experiments/scoring/src/scoring/cli.py | 13 +- experiments/scoring/src/scoring/inputs.py | 18 ++ .../scoring/src/scoring/measure_lab.py | 27 +-- experiments/scoring/src/scoring/report.py | 74 +++++-- experiments/scoring/tests/test_report.py | 194 ++++++++++++++++++ 8 files changed, 294 insertions(+), 38 deletions(-) create mode 100644 experiments/scoring/tests/test_report.py diff --git a/experiments/scoring/FORMATS.md b/experiments/scoring/FORMATS.md index 281f51f37..77096e0b5 100644 --- a/experiments/scoring/FORMATS.md +++ b/experiments/scoring/FORMATS.md @@ -10,7 +10,7 @@ Every file starts with `"format": 1` and `"unit": "ft"`. The scorer reads numbers as exact decimals, so a tie with a threshold resolves the same way every time. In floats, 3.1 − 3.0 is 0.10000000000000009, which would pass a 3.1 ± 0.1 ft clearance against a 3 ft rule that is exactly borderline. -The scorer rejects unknown fields, duplicate keys, `NaN`, negative lengths and numbers written as strings. Fields that may be null must still be present. Any input problem stops the run with exit code 2 and a message that names the file, the field and the fix, and the scorer writes no CSV. +The scorer rejects unknown fields, duplicate keys, `NaN`, negative lengths and numbers written as strings. Fields that may be null must still be present. Any input problem stops the run with exit code 2 and a message that names the file, the field and the fix, and the scorer writes no CSV. The same happens when `--out` would put a CSV on top of an input file, whether by the same path, a symlink or a hard link. ## Rules file diff --git a/experiments/scoring/METRICS.md b/experiments/scoring/METRICS.md index 42847467f..0a7de44ac 100644 --- a/experiments/scoring/METRICS.md +++ b/experiments/scoring/METRICS.md @@ -1,6 +1,6 @@ # What the scoring numbers mean -`score` prints a markdown summary and writes `measurements.csv`, `checks.csv` and `runs.csv`. [`results/synthetic-01/`](results/synthetic-01/) is an example made from the synthetic fixtures. [FORMATS.md](FORMATS.md) describes the input files. +`score` prints a markdown summary and writes `measurements.csv`, `checks.csv` and `runs.csv`. [`results/synthetic-01/`](results/synthetic-01/) is an example made from the synthetic fixtures. [FORMATS.md](FORMATS.md) describes the input files. Ids may be any non-empty string. The summary escapes them so a pipe, backtick or line break cannot break a table, heading or list, and the CSVs hold them unchanged. The summary has one section per house: distances, checks, timing, and a list by name of every unsafe pass, missed review and decision made without its measurement. Nothing is averaged across houses, and missing outputs stay in the denominator. diff --git a/experiments/scoring/README.md b/experiments/scoring/README.md index d11f33ff5..41bcde350 100644 --- a/experiments/scoring/README.md +++ b/experiments/scoring/README.md @@ -16,6 +16,6 @@ uv run pytest -q [FORMATS.md](FORMATS.md) specifies the input files and the Measure Lab importer. [METRICS.md](METRICS.md) explains every number. Real surveys and results go in git-ignored `data/`, the default `--out`. -**Result.** No real house scored yet. In [results/synthetic-01](results/synthetic-01/summary.md), three invented runs have median errors of 3.48, 12.60 and 1.20 in, and `ar-taps` makes 1 unsafe pass. Those numbers test the scorer, not any pipeline. All 303 tests and ruff pass. +**Result.** No real house scored yet. In [results/synthetic-01](results/synthetic-01/summary.md), three invented runs have median errors of 3.48, 12.60 and 1.20 in, and `ar-taps` makes 1 unsafe pass. Those numbers test the scorer, not any pipeline. All 347 tests and ruff pass. **What changed.** Nothing in the product or the plan yet. The field eval in PR #12 calls `score import-measure-lab` to score a Measure Lab session. diff --git a/experiments/scoring/src/scoring/cli.py b/experiments/scoring/src/scoring/cli.py index 8ccc3d185..f780b28e5 100644 --- a/experiments/scoring/src/scoring/cli.py +++ b/experiments/scoring/src/scoring/cli.py @@ -5,9 +5,9 @@ from pathlib import Path from scoring import measure_lab -from scoring.inputs import InputError, load_study +from scoring.inputs import InputError, load_study, refuse_output_over_inputs from scoring.metrics import score_run -from scoring.report import markdown, write_csvs +from scoring.report import csv_paths, markdown, write_csvs def main(argv: list[str] | None = None) -> int: @@ -37,6 +37,15 @@ def main(argv: list[str] | None = None) -> int: args = parser.parse_args(argv) try: + # Before loading or printing anything, so a colliding --out never touches an input. + refuse_output_over_inputs( + csv_paths(args.out), + [ + ("rules", args.rules), + *(("survey", path) for path in args.truth), + *(("results", path) for path in args.results), + ], + ) study = load_study(args.rules, args.truth, args.results) except InputError as error: print(f"score: {error}", file=sys.stderr) diff --git a/experiments/scoring/src/scoring/inputs.py b/experiments/scoring/src/scoring/inputs.py index 5c45bceaa..5bea15d66 100644 --- a/experiments/scoring/src/scoring/inputs.py +++ b/experiments/scoring/src/scoring/inputs.py @@ -10,6 +10,7 @@ import hashlib import json +import os import re from collections.abc import Container, Iterable from dataclasses import dataclass @@ -243,6 +244,23 @@ def header(self) -> None: ) +def refuse_output_over_inputs(outputs: Iterable[Path], inputs: Iterable[tuple[str, Path]]) -> None: + """Stop before any write when an output file is one of the inputs, whether by the same path, + another spelling of it, a symlink or a hard link.""" + inputs = list(inputs) + for out_path in outputs: + out_resolved = out_path.resolve() + for role, path in inputs: + same = out_resolved == path.resolve() + if not same and out_path.exists() and path.exists(): + same = os.path.samefile(out_path, path) + if same: + raise InputError( + f"--out {out_path} is the {role} {path}; writing there would overwrite an " + "input. Choose another output path" + ) + + def read_json(path: Path) -> tuple[Any, bytes]: try: raw = path.read_bytes() diff --git a/experiments/scoring/src/scoring/measure_lab.py b/experiments/scoring/src/scoring/measure_lab.py index af7561c73..1dbb73611 100644 --- a/experiments/scoring/src/scoring/measure_lab.py +++ b/experiments/scoring/src/scoring/measure_lab.py @@ -10,7 +10,6 @@ import argparse import hashlib import json -import os import sys import tempfile import zipfile @@ -35,6 +34,7 @@ load_truth, parse_json, read_json, + refuse_output_over_inputs, ) from scoring.metrics import decide @@ -562,20 +562,6 @@ def _string(value: str) -> str: return json.dumps(value, ensure_ascii=False) -def _refuse_output_over_input(out_path: Path, inputs: dict[str, Path]) -> None: - """Stop before any write when --out names one of the inputs, by path, symlink or hard link.""" - out_resolved = out_path.resolve() - for role, path in inputs.items(): - same = out_resolved == path.resolve() - if not same and out_path.exists() and path.exists(): - same = os.path.samefile(out_path, path) - if same: - raise InputError( - f"--out {out_path} is the {role} {path}; writing there would overwrite an input. " - "Choose another output path" - ) - - def import_session( session_path: Path, map_path: Path, @@ -585,9 +571,14 @@ def import_session( *, decide_outcomes: bool, ) -> dict[str, Any]: - _refuse_output_over_input( - out_path, - {"session zip": session_path, "map": map_path, "rules": rules_path, "truth": truth_path}, + refuse_output_over_inputs( + [out_path], + [ + ("session zip", session_path), + ("map", map_path), + ("rules", rules_path), + ("truth", truth_path), + ], ) rules = load_rules(rules_path) truth = load_truth(truth_path, rules) diff --git a/experiments/scoring/src/scoring/report.py b/experiments/scoring/src/scoring/report.py index 8c27ed0b9..fc426bb32 100644 --- a/experiments/scoring/src/scoring/report.py +++ b/experiments/scoring/src/scoring/report.py @@ -6,6 +6,7 @@ """ import csv +import re from collections.abc import Iterable from decimal import ROUND_HALF_UP, Decimal from pathlib import Path @@ -235,9 +236,17 @@ def _write(path: Path, columns: tuple[str, ...], rows: Iterable[dict[str, str]]) writer.writerows(rows) +# The files write_csvs creates in the output directory, in the order it returns them. +CSV_NAMES = ("measurements.csv", "checks.csv", "runs.csv") + + +def csv_paths(out_dir: Path) -> list[Path]: + return [out_dir / name for name in CSV_NAMES] + + def write_csvs(runs: list[RunScore], out_dir: Path) -> list[Path]: out_dir.mkdir(parents=True, exist_ok=True) - paths = [out_dir / "measurements.csv", out_dir / "checks.csv", out_dir / "runs.csv"] + paths = csv_paths(out_dir) measurement_rows = (measurement_row(r, s) for r in runs for s in r.measurements) _write(paths[0], MEASUREMENT_COLUMNS, measurement_rows) _write(paths[1], CHECK_COLUMNS, (check_row(r, s) for r in runs for s in r.checks)) @@ -245,9 +254,42 @@ def write_csvs(runs: list[RunScore], out_dir: Path) -> list[Path]: return paths +# Identifiers in the inputs may be any non-empty string, so every one is escaped where it lands in +# the summary: a pipe would split a table cell, a line break would end a row, heading or list item, +# and a backtick would close a code span early. +_LINE_BREAK = re.compile(r"\r\n|\r|\n") +_MARKDOWN_SPECIAL = re.compile(r"([\\`*_\[\]<>|#])") + + +def _one_line(value: str) -> str: + return _LINE_BREAK.sub(" ", value) + + +def _code(value: str) -> str: + """`value` as one inline code span. The fence is one backtick longer than any backtick run + inside, and a space pads a value that starts or ends with a backtick, or that starts and ends + with a space (CommonMark strips one such space from each side).""" + text = _one_line(value) + fence = "`" * (max((len(run) for run in re.findall("`+", text)), default=0) + 1) + edge_tick = text.startswith("`") or text.endswith("`") + edge_spaces = text.startswith(" ") and text.endswith(" ") and text.strip() != "" + pad = " " if edge_tick or edge_spaces else "" + return f"{fence}{pad}{text}{pad}{fence}" + + +def _text(value: str) -> str: + """`value` as plain inline text, on one line, with Markdown punctuation escaped.""" + return _MARKDOWN_SPECIAL.sub(r"\\\1", _one_line(value)) + + +def _cell(value: str) -> str: + # GitHub splits a table row at every unescaped pipe, even inside a code span. + return value.replace("|", "\\|") + + def _table(header: list[str], rows: list[list[str]]) -> list[str]: lines = ["| " + " | ".join(header) + " |", "|" + "---|" * len(header)] - lines += ["| " + " | ".join(cell or "n/a" for cell in row) + " |" for row in rows] + lines += ["| " + " | ".join(_cell(cell) or "n/a" for cell in row) + " |" for row in rows] return lines @@ -262,7 +304,7 @@ def markdown(study: Study, runs: list[RunScore]) -> str: lines = [ "# Capture pipeline scores", "", - f"Rules: `{rules.path.name}` ({rules.name}), sha256 `{rules.sha256}`.", + f"Rules: {_code(rules.path.name)} ({_text(rules.name)}), sha256 {_code(rules.sha256)}.", f"{_count(len(houses), 'house')}, {_count(spots, 'candidate spot')}, " f"{_count(len(runs), 'pipeline run')}.", "", @@ -275,10 +317,11 @@ def markdown(study: Study, runs: list[RunScore]) -> str: reference = truth.measurements[truth.scale_reference] lines += [ "", - f"## House {truth.house}", + f"## House {_text(truth.house)}", "", - f"Captures: {', '.join(truth.captures)}. Candidates: {', '.join(truth.candidates)}. " - f"Scale reference `{reference.id}` ({feet(reference.value_ft)} ft), excluded.", + f"Captures: {', '.join(map(_text, truth.captures))}. " + f"Candidates: {', '.join(map(_text, truth.candidates))}. " + f"Scale reference {_code(reference.id)} ({feet(reference.value_ft)} ft), excluded.", ] if not house_runs: lines += ["", "No pipeline runs for this house."] @@ -300,7 +343,7 @@ def _distances(runs: list[RunScore]) -> list[str]: within, with_uncertainty = run.within_reported rows.append( [ - f"`{run.results.pipeline}`", + _code(run.results.pipeline), run.results.scale_source, f"{run.count('scored')}/{denominator}", inches(run.median_abs_error_in), @@ -359,7 +402,7 @@ def _checks(runs: list[RunScore]) -> list[str]: ] else: decided = ["no decisions", "", "", "", "", "", "", ""] - rows.append([f"`{run.results.pipeline}`", *decided, str(run.could_flip)]) + rows.append([_code(run.results.pipeline), *decided, str(run.could_flip)]) header = [ "Pipeline", "Agree", @@ -397,7 +440,7 @@ def _checks(runs: list[RunScore]) -> list[str]: def _timing(runs: list[RunScore]) -> list[str]: rows = [ [ - f"`{run.results.pipeline}`", + _code(run.results.pipeline), seconds(run.results.capture_seconds) or "not recorded", seconds(run.results.processing_seconds) or "not recorded", ] @@ -409,9 +452,9 @@ def _timing(runs: list[RunScore]) -> list[str]: def _survey_text(score: CheckScore) -> str: """The survey outcome and the lines it was judged against, for the named lists.""" survey, threshold, review = score.survey, score.threshold, score.review - limits = f"`{threshold.name}` {threshold.pass_when} {feet(threshold.value_ft)} ft" + limits = f"{_code(threshold.name)} {threshold.pass_when} {feet(threshold.value_ft)} ft" if review is not None: - limits = f"`{review.name}` {feet(review.value_ft)} ft and {limits}" + limits = f"{_code(review.name)} {feet(review.value_ft)} ft and {limits}" if survey.value_ft is None: return f"the survey is {score.truth} ({survey.status.replace('_', ' ')}) against {limits}" return ( @@ -425,8 +468,8 @@ def _wrong_pass(run: RunScore, score: CheckScore) -> str: has_value = reported is not None and reported.value_ft is not None run_value = f"{feet(reported.value_ft)} ft" if has_value else "no value" return ( - f"- `{run.results.pipeline}` passed `{score.check.check}` at " - f"`{score.check.candidate}`; {_survey_text(score)} (run measured {run_value})." + f"- {_code(run.results.pipeline)} passed {_code(score.check.check)} at " + f"{_code(score.check.candidate)}; {_survey_text(score)} (run measured {run_value})." ) @@ -434,8 +477,9 @@ def _decision_without_measurement(run: RunScore, score: CheckScore) -> str: reported = score.measurement.reported assert reported is not None and reported.missing is not None return ( - f"- `{run.results.pipeline}` reported {score.reported} for `{score.check.check}` at " - f"`{score.check.candidate}` with measurement `{score.check.measurement}` missing " + f"- {_code(run.results.pipeline)} reported {score.reported} for " + f"{_code(score.check.check)} at {_code(score.check.candidate)} with measurement " + f"{_code(score.check.measurement)} missing " f"({reported.missing}); {_survey_text(score)}." ) diff --git a/experiments/scoring/tests/test_report.py b/experiments/scoring/tests/test_report.py new file mode 100644 index 000000000..5a815987f --- /dev/null +++ b/experiments/scoring/tests/test_report.py @@ -0,0 +1,194 @@ +"""The markdown summary keeps its shape for any identifier, and the scorer never writes over an +input. Identifiers may be any non-empty string, so the report escapes them where they land.""" + +import hashlib +import json +import os +import re +from pathlib import Path + +import pytest +from helpers import FIXTURES + +from scoring.cli import main +from scoring.report import CSV_NAMES, _cell, _code, _table, _text + +RULES = FIXTURES / "rules.json" +TRUTH = FIXTURES / "truth" / "synthetic-01.json" +RESULTS = [FIXTURES / "results" / f"{name}.json" for name in ("ar-taps", "photo-depth")] + +# Each quoted fixture id and the hostile id that replaces it: pipes, line breaks, backticks and +# heading marks in the pipeline, house, candidate, check and measurement ids. Threshold names are +# already restricted to lower_snake_case by the rules loader. +RENAMES = { + '"ar-taps"': "ar|taps\n`x`", + '"synthetic-01"': "house\n# injected", + '"c2"': "c|2", + '"facing_gap"': "facing`gap`", + '"c2-facing"': "c2\r\nfacing", +} +UNESCAPED_PIPE = re.compile(r"(?`g`\\") == r"a\|b \# c\_d\*\[e\]\\`g\`\\" + + def test_cell_escapes_pipes_even_in_code(self): + assert _cell(_code("a|b")) == r"`a\|b`" + + def test_table_rows_keep_the_header_width(self): + lines = _table(["A", "B"], [[_code("x|y|z"), "1"], [_code("p\nq"), ""]]) + assert [len(UNESCAPED_PIPE.findall(line)) for line in lines] == [3, 3, 3, 3] + assert lines[3] == "| `p q` | n/a |" + + +def hostile_inputs(root: Path) -> tuple[Path, Path, list[Path]]: + """Copies of the fixtures with every id in RENAMES replaced, and the rules hash updated.""" + + def rename(text: str) -> str: + for quoted, new in RENAMES.items(): + text = text.replace(quoted, json.dumps(new)) + return text + + rules = root / "rules.json" + rules.write_text(rename(RULES.read_text())) + old_sha = hashlib.sha256(RULES.read_bytes()).hexdigest() + new_sha = hashlib.sha256(rules.read_bytes()).hexdigest() + truth = root / "truth.json" + truth.write_text(rename(TRUTH.read_text())) + results = [] + for source in RESULTS: + path = root / source.name + path.write_text(rename(source.read_text()).replace(old_sha, new_sha)) + results.append(path) + return rules, truth, results + + +def test_summary_keeps_its_shape_with_hostile_ids(tmp_path: Path, capsys): + rules, truth, results = hostile_inputs(tmp_path) + args = ["--rules", str(rules), "--truth", str(truth), "--results", *map(str, results)] + assert main([*args, "--out", str(tmp_path / "out")]) == 0 + lines = capsys.readouterr().out.splitlines() + + # Every table row has as many cell borders as its header. + tables: list[list[str]] = [] + for index, line in enumerate(lines): + if line.startswith("|") and not lines[index - 1].startswith("|"): + tables.append([]) + if line.startswith("|"): + tables[-1].append(line) + assert len(tables) == 3 + for table in tables: + widths = {len(UNESCAPED_PIPE.findall(row)) for row in table} + assert len(widths) == 1, table + + # No id started a line of its own, so no heading, row or list item was split or injected. + starts = ("#", "|", "- ", "Rules: ", "Captures: ", "Each ", "These ") + prose = [line for line in lines if line and not line.startswith(starts)] + assert all(" " in line and not line.startswith(("`", "facing", "injected")) for line in prose) + assert r"## House house \# injected" in lines + + table_text = "\n".join(row for table in tables for row in table) + assert r"| `` ar\|taps `x` `` |" in table_text # a value ending in a backtick is padded + listed = "\n".join(line for line in lines if line.startswith("- ")) + assert "- `` ar|taps `x` `` passed `` facing`gap` `` at `c|2`;" in listed + assert "with measurement `c2 facing` missing" in listed + + +class TestScorerNeverOverwritesAnInput: + ROLES = ("rules", "survey", "results") + + def inputs(self, root: Path) -> dict[str, Path]: + paths = {"rules": root / "rules.json", "survey": root / "truth.json"} + paths["results"] = root / "ar-taps.json" + paths["rules"].write_bytes(RULES.read_bytes()) + paths["survey"].write_bytes(TRUTH.read_bytes()) + paths["results"].write_bytes(RESULTS[0].read_bytes()) + return paths + + def run(self, paths: dict[str, Path], out: Path) -> int: + args = ["--rules", str(paths["rules"]), "--truth", str(paths["survey"])] + return main([*args, "--results", str(paths["results"]), "--out", str(out)]) + + def check_refused(self, paths: dict[str, Path], out: Path, role: str, capsys) -> None: + before = {name: path.read_bytes() for name, path in paths.items()} + assert self.run(paths, out) == 2 + captured = capsys.readouterr() + assert f" is the {role} " in captured.err + assert "would overwrite an input" in captured.err + assert captured.out == "" # refused before the summary was printed + assert {name: path.read_bytes() for name, path in paths.items()} == before + + @pytest.mark.parametrize("name", CSV_NAMES) + @pytest.mark.parametrize("role", ROLES) + def test_input_named_as_an_output(self, tmp_path: Path, role: str, name: str, capsys): + out = tmp_path / "out" + out.mkdir() + paths = self.inputs(tmp_path) + moved = out / name + paths[role].rename(moved) + paths[role] = moved + self.check_refused(paths, out, role, capsys) + + @pytest.mark.parametrize("role", ROLES) + def test_other_spelling_of_the_output_directory(self, tmp_path: Path, role: str, capsys): + out = tmp_path / "out" + (out / "sub").mkdir(parents=True) + paths = self.inputs(tmp_path) + moved = out / "runs.csv" + paths[role].rename(moved) + paths[role] = moved + self.check_refused(paths, out / "sub" / "..", role, capsys) + + @pytest.mark.parametrize("name", CSV_NAMES) + @pytest.mark.parametrize("role", ROLES) + def test_symlink(self, tmp_path: Path, role: str, name: str, capsys): + out = tmp_path / "out" + out.mkdir() + paths = self.inputs(tmp_path) + (out / name).symlink_to(paths[role]) + self.check_refused(paths, out, role, capsys) + + @pytest.mark.parametrize("name", CSV_NAMES) + @pytest.mark.parametrize("role", ROLES) + def test_hard_link(self, tmp_path: Path, role: str, name: str, capsys): + out = tmp_path / "out" + out.mkdir() + paths = self.inputs(tmp_path) + os.link(paths[role], out / name) + self.check_refused(paths, out, role, capsys) + + def test_every_survey_and_results_file_is_checked(self, tmp_path: Path, capsys): + out = tmp_path / "out" + out.mkdir() + paths = self.inputs(tmp_path) + second = out / "checks.csv" + second.write_bytes(RESULTS[1].read_bytes()) + before = second.read_bytes() + args = ["--rules", str(paths["rules"]), "--truth", str(paths["survey"]), "--results"] + assert main([*args, str(paths["results"]), str(second), "--out", str(out)]) == 2 + assert " is the results " in capsys.readouterr().err + assert second.read_bytes() == before + + def test_separate_output_directory_still_writes(self, tmp_path: Path, capsys): + paths = self.inputs(tmp_path) + out = tmp_path / "out" + assert self.run(paths, out) == 0 + assert sorted(path.name for path in out.iterdir()) == sorted(CSV_NAMES)