composer: enumerated parameters, and length_view becomes reachable
length_view was in both families' defaults and in neither COMMON_GROUPS, so the Full Length branch of model_length_mm could not be reached from the composer. Every model was a 100 mm preview whatever member_length_ft said. Found while specifying STL export: the sweep must equal model_length_mm(p), the value already published as LENGTH_MM, or the record's VOLUME_MM3 and MASS_G describe a different object than the file beside them. Adding it to the list would have been one line and the wrong fix. The type coercion falls through to the raw string for anything that is not a bool, int or float, and model_length_mm compares against the literal "Full Length" and silently falls back to preview for anything else -- the reference behaved the same way, so the port is right to keep it. A free-text box would have replaced an unreachable control with a silent one: "Full Length " with a trailing space builds a 100 mm model and says nothing. So ENUM_PARAMS declares which parameters take one of a fixed set of values. The browser renders a select instead of an input, and a value outside the set is dropped before the build rather than passed through -- the family default stands, which is a valid model. The guard sits before the coercion, not after, where it would be dead code. The next enumerated parameter needs no new machinery. Deliberately not mirrored as a check in the families. geometry_checks is shared with the oracle and the reference accepted any string here; adding a check there would put the 123 frozen cases at risk to fix a user-interface problem. The guard belongs in the composer, which is not oracle-bearing. 19 assertions, mutation-proven: removing the guard fails 8 of them. Six are the parametrised bad values. The other two assert the mechanism rather than its effect -- that the string never enters values at all, and that the fallback reaches input_id. Without them, a pass-through that happened to be ignored downstream would look identical to a working fallback, and a later change to model_length_mm would turn a passing test into a defect somewhere else. length_view is a build parameter and stays in both hashes, unlike the author. Two members of different lengths are different designs, and a test asserts it. Suite 566 passed, of which 19 are new. None of the 547 moved.
This commit is contained in:
+55
-7
@@ -41,15 +41,44 @@ from mechcomp import svg
|
||||
# as root:mechcomp 0640, and the service user can read it.
|
||||
ENV_FILE = "/etc/mechcomp/mechcomp.env"
|
||||
|
||||
# Parameters whose value is one of a fixed set rather than a number.
|
||||
#
|
||||
# WHY THIS EXISTS RATHER THAN A SPECIAL CASE FOR length_view
|
||||
# A free-text box for a parameter with two valid values is a trap. The
|
||||
# type coercion below falls through to the raw string for anything that is
|
||||
# not a bool, int or float, and ``_common.model_length_mm`` compares
|
||||
# against the literal "Full Length" and silently falls back to the preview
|
||||
# length for anything else. So "Full Length " with a trailing space would
|
||||
# produce a 100 mm model with no rejection and no error -- a file that
|
||||
# looks right and is the wrong object.
|
||||
#
|
||||
# Declaring the choices here does three things at once: the browser renders
|
||||
# a select instead of an input, a value outside the set never reaches the
|
||||
# build, and the next enumerated parameter needs no new machinery.
|
||||
#
|
||||
# NOT MIRRORED AS A CHECK IN THE FAMILIES, DELIBERATELY
|
||||
# ``geometry_checks`` is shared with the oracle, and the reference accepted
|
||||
# any string here and fell through to preview. Adding a check there would
|
||||
# put the 123 frozen cases at risk to fix a user-interface problem. The
|
||||
# guard belongs in the composer, which is not oracle-bearing.
|
||||
ENUM_PARAMS: Dict[str, List[str]] = {
|
||||
"length_view": ["Preview", "Full Length"],
|
||||
}
|
||||
|
||||
# Parameters that apply to every profile, in the order they make sense to a
|
||||
# person: what the stock is, how it fits, then how thick the printed walls are.
|
||||
#
|
||||
# ``length_view`` leads the Model group because it decides which of the two
|
||||
# lengths below it actually applies. Without it the Full Length branch was
|
||||
# unreachable from the composer and every model was a 100 mm preview whether
|
||||
# or not ``member_length_ft`` had been set.
|
||||
COMMON_GROUPS: List[Tuple[str, List[str]]] = [
|
||||
("Stock", ["strap_width_mm", "strap_thickness_mm", "bundle_count"]),
|
||||
("Fit", ["fit_clearance_mm"]),
|
||||
("Walls", ["inside_wall_thickness_mm", "outside_wall_thickness_mm",
|
||||
"edge_wall_thickness_mm", "min_wall_mm"]),
|
||||
("Model", ["preview_length_mm", "member_length_ft", "material_density_g_cm3",
|
||||
"facets"]),
|
||||
("Model", ["length_view", "preview_length_mm", "member_length_ft",
|
||||
"material_density_g_cm3", "facets"]),
|
||||
]
|
||||
|
||||
# Which parameter prefixes belong to which profile, so a person is not shown
|
||||
@@ -110,6 +139,16 @@ def build_payload(family_name: str, profile: str,
|
||||
for key, raw in overrides.items():
|
||||
if key not in family.defaults:
|
||||
continue
|
||||
|
||||
# An enumerated parameter takes its value or nothing. Dropping an
|
||||
# unrecognised value leaves the family default in place, which is a
|
||||
# valid model, rather than passing a string the geometry will ignore
|
||||
# without saying so.
|
||||
if key in ENUM_PARAMS:
|
||||
if raw in ENUM_PARAMS[key]:
|
||||
typed[key] = raw
|
||||
continue
|
||||
|
||||
default = family.defaults[key]
|
||||
try:
|
||||
if isinstance(default, bool):
|
||||
@@ -127,7 +166,7 @@ def build_payload(family_name: str, profile: str,
|
||||
"family": family_name,
|
||||
"profile": profile,
|
||||
"groups": [[title, keys] for title, keys in profile_params(family, profile)],
|
||||
"defaults": {k: family.defaults[k] for k in family.defaults},
|
||||
"choices": {k: v for k, v in ENUM_PARAMS.items() if k in family.defaults},
|
||||
"values": {**family.defaults, **typed},
|
||||
"profiles": sorted(family.catalogue),
|
||||
}
|
||||
@@ -255,6 +294,7 @@ async function refresh(sendValues) {
|
||||
prof.onchange = () => { state.profile = prof.value; refresh(true); };
|
||||
|
||||
state.values = {};
|
||||
const choices = data.choices || {};
|
||||
const box = document.getElementById("controls");
|
||||
box.innerHTML = "";
|
||||
for (const [title, keys] of data.groups) {
|
||||
@@ -267,9 +307,16 @@ async function refresh(sendValues) {
|
||||
const name = document.createElement("span");
|
||||
name.textContent = k.replace(/_mm$|_deg$|_ft$/, "").replace(/_/g, " ");
|
||||
name.title = k;
|
||||
const inp = document.createElement("input");
|
||||
inp.value = v;
|
||||
if (typeof v === "number") { inp.type = "number"; inp.step = "any"; }
|
||||
let inp;
|
||||
if (choices[k]) {
|
||||
inp = document.createElement("select");
|
||||
for (const c of choices[k]) inp.add(new Option(c, c));
|
||||
inp.value = v;
|
||||
} else {
|
||||
inp = document.createElement("input");
|
||||
inp.value = v;
|
||||
if (typeof v === "number") { inp.type = "number"; inp.step = "any"; }
|
||||
}
|
||||
inp.onchange = () => { state.values[k] = inp.value; refresh(true); };
|
||||
lab.append(name, inp);
|
||||
fs.append(lab);
|
||||
@@ -341,7 +388,8 @@ class Handler(BaseHTTPRequestHandler):
|
||||
except Exception as exc: # noqa: BLE001
|
||||
payload = {"ok": False,
|
||||
"message": "%s: %s" % (type(exc).__name__, exc),
|
||||
"profiles": [], "groups": [], "values": {}}
|
||||
"profiles": [], "groups": [], "values": {},
|
||||
"choices": {}}
|
||||
self._send(200, json.dumps(payload).encode("utf-8"),
|
||||
"application/json; charset=utf-8")
|
||||
return
|
||||
|
||||
Reference in New Issue
Block a user