Validate equipment and GPS form input at the API, not just the browser

[brickbots]

Closes #569. Also fixes the GPS-form 500 from the same 2.6.1 Gate 6 pass, and the config-load crash from #291.

The two defects

The equipment forms failed silently. Both handlers parsed with bare float()/int() inside except Exception: logger.error(...) and then fell through to the success template:

POST /equipment/add_eyepiece/-1  focal_length_mm=7,5  field_stop=8,0
  -> HTTP 200 + "Eyepiece added, restart your PiFinder to use"
  -> eyepiece count unchanged (3 -> 3).  Nothing was saved.

/gps/update had no try/except at all, so the same input reached the user as an unhandled 500. #536 fixed this class for /locations; its normalizeDecimal helper appears zero times in views/gps.html.

A decimal comma is the easiest trigger — PiFinder ships de/es/fr/zh — but anything unreadable did it, including the blank instrument name in #569's description.

The rules

Field rules live in one table in equipment.py. The edit forms render them into their client-side check and the API re-checks them before anything reaches config; the ranges are shared so the two can't drift. Tabulated in docs/ax/equipment/CONTEXT.md, with the reasoning in ADR 0027.

Record	Field	Required	Range
Telescope	make	no	≤ 64 chars
	name	yes	≤ 64 chars
	aperture_mm	yes	1 – 2000
	focal_length_mm	yes	1 – 20000
	obstruction_perc	no (0)	0 – 100
	mount_type	yes	alt/az | equatorial
Eyepiece	make	no	≤ 64 chars
	name	yes	≤ 64 chars
	focal_length_mm	yes	0.1 – 100
	afov	yes	1 – 180
	field_stop	no (0)	0 – 100

Two rules that aren't ranges: a blank field is not a zero (only obstruction_perc and field_stop have a documented zero meaning), and a rejected entry never reports success — the form comes back with the message and the values that were typed.

focal_length_mm: int on Telescope, float on Eyepiece

Settled as part of the issue: all measurements are floats. Optics are fractional — an 11" SCT is 279.4mm, a reducer turns 2032mm into 1280.2mm. That also closes #291, where an aperture of "279.5" written into config made the PiFinder unbootable: Equipment.from_dict raised invalid literal for int() inside main() before the UI came up. Whole millimetres still display as 1000, not 1000.0, through a format_measurement Jinja filter, and no migration is needed — the stored values are already numbers the float fields read.

What changed

• equipment.py — measurements are floats; TELESCOPE_LIMITS/EYEPIECE_LIMITS/MOUNT_TYPES/NAME_MAX_LENGTH and format_measurement() beside the dataclasses.
• server.py — parse_measurement / parse_name / eyepiece_from_form / telescope_from_form; both add_* handlers re-render the edit form on ValueError; every <id> route range-checks its index instead of raising IndexError as a 500; the DeepskyLog import re-checks the same limits and skips (and counts) records it can't read.
• gps_update() — parses everything before locking anything, so a bad clock entry can't half-apply a position; re-renders gps.html with the error.
• Templates — client-side validation shared by both equipment forms (equipment_validation.html), numeric inputs switched to type=text inputmode=decimal per fix(web): accept comma or period decimal separator when saving locations #536, normalizeDecimal ported into gps.html, and the two decimal→DMS bypasses at locations.html:282,288 fixed.
• config.py — an undecodable equipment section logs and falls back to the shipped defaults rather than aborting the boot.

Two incidental fixes in files this touches: the eyepiece dedup searched telescopes for an Eyepiece, and the GPS altitude field's input listener rewrote the longitude DMS fields.

Testing

68 new tests at the request level — a Flask test client POSTing 7,5 runs in CI, unlike the web suite, which runs en-US and structurally cannot catch this.

• pytest -m "smoke or unit" → 1227 passed (baseline 1159 at 4a83d25b)
• Selenium test_web_equipment.py + test_web_locations.py against a live PiFinder → 14 passed, 1 skipped
• Re-ran the original repro live: the comma eyepiece now saves 7.5/8.0, garbage re-renders the form with "must be a number", /gps/update with 34,22 returns 302 (was 500), and the out-of-range and bad-index cases return 200 with a message
• Client-side rules exercised directly; the inline JS on all four touched pages parses clean

i18n

18 new msgids added to de/es/fr/zh, tagged AI-TRANSLATED (claude): needs human review, .mo recompiled. The .po diffs are additive only (no pybabel update churn — messages.pot is gitignored here, so the .po diff is the only record).

Not included

/gps is kept and fixed rather than retired — it's the only place with the date/time control. G6.5 (the Volume menu-index drift) and the web suite's missing starting-language guard are deliberately left out; both are test-only and written up in the 2.6.1 test plan.

🤖 Generated with Claude Code

[brickbots]

Review

The core design is right, and the CONTEXT.md field-rules table is the kind of artifact that actually stops the next drift. Parse-then-commit in gps_update() is the correct shape, and the two incidental fixes are worth the PR on their own — cfg.equipment.telescopes.index(eyepiece) compared an Eyepiece against Telescopes, so it always raised ValueError and re-adding an identical eyepiece appended a duplicate instead of updating it; and the altitude input listener was rewriting the longitude DMS fields.

Verification

Check	Result
4 new test files	68 passed
pytest -m "smoke or unit" (pre-merge)	1227 passed — matches the PR description exactly
pytest -m "smoke or unit" (post-merge)	1354 passed — main's extra tests pass alongside
ruff check / ruff format --diff	clean
mypy (equipment/server/config)	clean

---

The two mechanical blockers — fixed in this branch

Pushed as 76c7bb45 and cf0c0ded. The PR was CONFLICTING / DIRTY; it is now MERGEABLE / CLEAN.

1. Conflict with main (39 commits behind). Only the four binary .mo files conflicted — the .po catalogues auto-merge cleanly since both sides only append. Resolved by regenerating the .mo from the merged .po with pybabel compile -d locale, deliberately not the full nox -s babel session, which would run extract/update and churn the .po diff on a feature branch.

Verified after resolution: no conflict markers; msgfmt --check passes on all four catalogues (693 translated, no duplicate msgids); and a runtime lookup finds both sides' strings in de/es/fr/zh — main's Lens → Objektiv/Objectif/镜头 alongside this branch's No such eyepiece → Okular nicht gefunden/Oculaire introuvable/找不到该目镜.

Resolved by merge rather than rebase, since rebasing a pushed PR branch needs a force-push. Same end state, and the merge commit disappears when the PR is squash-merged.

2. ADR 0027 collided — renumbered to 0030. main already carries 0027-fov-gate-derived-from-optical-train.md. Different filenames, so git merged this without a conflict and briefly left two ADR 0027s in the tree. Under the standing tiebreak the most-referenced ADR keeps the number, and the FOV gate wins by a wide margin — CONTEXT-MAP.md, docs/ax/sqm.md, docs/ax/positioning.md, positioning/CONTEXT.md, ADR 0029 and two test modules point at it, against three references here. Three inbound references updated (equipment.py:18, docs/ax/equipment.md:90, docs/ax/equipment/CONTEXT.md:96); ADR titles in this repo carry no number, so the rename is the whole of it. Every docs/adr/NNNN-*.md link in the repo still resolves.

Note: the 2.6.2 test plan (P1.4) earmarked 0029 for this branch, but 0029 was taken by the lens-confidence ADR (#624) in the meantime, so this took 0030. That line in release_notes/release-2.6.2-test-plan.md:80-82 is now stale — left alone rather than editing a release artifact from a feature branch.

Separately: #622 is also claiming 0029, which is likewise already taken on main. Worth catching before it lands.

---

High — the #291 fallback silently destroys the user's saved equipment

config.py:93-105 catches the decode failure and falls back to the shipped defaults. Booting beats not booting, but the recovery is write-through and lossy. Reproduced with a config whose telescope has aperture_mm: "not a number" alongside two perfectly decodable eyepieces:

loaded telescopes : ['Dobsonian']                  # user's "MY C11" gone
loaded eyepieces  : ['Plossl','Plossl','Plossl']   # both user eyepieces gone
on-disk after one save_equipment(): ['Plossl','Plossl','Plossl']
USER DATA STILL PRESENT? False

save_equipment() writes self.equipment.to_dict() over the whole section, and set_option("equipment.active_eyepiece", …) calls it — so simply changing the active eyepiece from the device UI permanently erases the list. One bad field costs the user every telescope and eyepiece they ever entered.

This PR already picked the right pattern for the DeepskyLog import — skip the unreadable record, keep the rest, count what was dropped. The same shape here (decode per-record, drop only what fails) keeps the good eyepieces and preserves the invariant. Failing that, stash the raw section under equipment_backup before overwriting.

Related, same block: the except branch's Equipment.from_dict(default_eq) is itself unguarded. If the shipped defaults ever fail to decode you are back to an unbootable PiFinder, thrown from inside the recovery path. The branch two levels up already has the right last-ditch answer — Equipment(telescopes=[], eyepieces=[]).

---

Medium

3. The new location limits are declared but /locations doesn't use them. LATITUDE_LIMITS / LONGITUDE_LIMITS / ALTITUDE_LIMITS (server.py:60-63) are consumed only by gps_update. location_add and location_rename still hard-code -90/90, -180/180, -1000/10000 inline — twice each. The comment at server.py:59 ("The /locations handlers enforce the same ranges inline") documents the duplication rather than removing it, which cuts against the PR's own thesis. Swapping in parse_measurement(…, LATITUDE_LIMITS) drops ~12 lines and the drift risk.

4. format_measurement never reaches the on-device UI.

str(eyepiece)                          -> "25mm Plossl"
f"{ep.focal_length_mm}mm {ep.name}"    -> "25.0mm Plossl"

ui/equipment.py:61, ui/log.py:261 and ui/menu_manager.py:71 each rebuild that string with a raw f-string instead of calling str(eyepiece). Not a regression — eyepiece focal length was already a float — but the PR's display rule now holds on the web and not on the device screen, which is the surface users actually read at the telescope. All three can just call str(eyepiece); that is precisely what __str__ now does.

5. The DeepskyLog import doesn't validate mount_type. check_equipment_limits covers the measurements and a non-blank name, but the import writes instrument["mount_type"]["name"].lower() straight through — a DSL "dobsonian" lands in config while telescope_from_form rejects it. CONTEXT.md states the two-value set as an invariant, so the import should honour it too.

---

Nits

• Blank required measurement reports the wrong thing. parse_measurement only produces "is required" when the key is absent (value is None); a box the user cleared is "" and falls through to parse_coordinate → "Focal length must be a number". test_parse_measurement_blank_without_default_is_an_error asserts only ValueError, so the wording isn't pinned.
• format_measurement raises on NaN/inf (int() throws ValueError/OverflowError). Unreachable via the forms — the range check rejects nan, inf, -inf, 1e400 — but json round-trips NaN happily, so a hand-edited config would 500 the equipment page. A math.isfinite guard is one line.
• {{ _('…') }} inside a <script> block is HTML-escaped, not JS-escaped. A translation containing an apostrophe renders literally as l'obligatoire. None of the four new msgids have one today and four existing templates share the pattern, so it's a latent trap for the next translator rather than a live bug — |tojson fixes it.
• success_message += " " + _("%s entries were skipped…") concatenates two translated sentences and isn't pluralised (ngettext).
• show_new_form=0 is passed to gps.html, which never reads it — copy/paste from the /locations handler.
• docs/ax/equipment/CONTEXT.md:5 still calls ../equipment.md "(planned)"; it has existed since #440 and this PR edits both files.

---

The remaining item I'd want addressed on substance is the config fallback: as written it converts #291's boot failure into silent, irreversible data loss — a worse failure mode for a user who can recover a bad boot by editing one field over ssh but can't recover a deleted equipment list at all.

🤖 Generated with Claude Code