fix(ui,app,occt): batch 1 of the GUI audit — the five P1s and the P2s that share their causes

Eleven findings from docs/GUI_AUDIT_2026-09-10.md, each proved by driving the real
shell rather than by a unit test alone. That distinction is the point: three of these
first landed with passing egui tests and did nothing at all in the app, because the
tests asked a question the shell never asks.

THE VALUE CARD COULD NOT SEE WHO OWNED THE KEYBOARD (HV-01, R2B-01, R2B-05). egui's
single-line TextEdit surrenders focus the instant it reads an Enter, and the card is
painted last so it floats — so the card asked who was focused, was told "nobody", and
committed a keystroke meant for the Variables field, the title-bar path box or the
Items new-folder field. shell::show now records focus once before any widget paints,
and the card takes Enter only when neither that snapshot nor the live answer names an
id the card did not paint itself. It is additionally deaf while the discard prompt
stands.

A TYPED EXPRESSION NEVER CARRIED ITS UNIT INTO THE DOCUMENT (R2A-02). A field whose
text is not a bare number is stored as text and re-evaluated by the server at a bare
scale of 1.0, so in inches the card computed 25.4 mm for "0.5+0.5" and the document
computed 1.0 mm from the same string, for ever. The stored text now says what the
field meant, and the rewrite is verified by evaluation rather than trusted.

OCCT WRITES ONE ASSEMBLY ROW PER EDGE OF THE ASSEMBLY TREE (MB-01). The determinism
guard compared those rows against the body count; the audit proposed the leaf-solid
count; both are wrong, measured. A second defect fired first: the product-name
normaliser accepted one positional level and refused two, which is what a sub-assembly
writes. STEP export of a compound body beside another body works again and is still
byte-identical across two exports.

Also: a delete no longer leaves a stale pending point that kills every later drawing
click (STC-02); a bare second Enter after a dispatch does nothing (PV-07); a typed
value outside a descriptor's range is refused by name instead of silently clamped to
0.001 (SF-07, FC-05, SB-10); a zero-length push/pull is refused at the card instead of
by the kernel (DISC-08); Escape out of a Create session disarms its Cut target (SF-01);
undo during a compile is refused with a readout instead of silently undoing the
operation being built (SF-10); and an export onto any document's storage name is
refused by shape rather than only against the open document (FPR-06).

Five drive scripts are promoted into the gate, each mutation-checked against the
pre-fix binary. What this batch deliberately leaves undone, and three pre-existing
failures it is careful not to claim credit for, are in
docs/FIXES_2026-09-11_audit-batch-1.md.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-09-11 09:10:27 +02:00
co-authored by Claude Opus 5
parent 688279d09f
commit f86b74a059
17 changed files with 2509 additions and 76 deletions
+111
View File
@@ -2275,6 +2275,28 @@ impl VernierApp {
self.shell.readout = "Waiting for the current document operation to finish.".to_owned();
return false;
}
// UNDO DOES NOT QUEUE BEHIND WORK NOBODY CAN SEE. An ordinary edit
// — a pattern, a fillet — raises `pending_jobs` but NOT
// `document_barrier`, so the guard above lets ctrl+z through and
// the queue runs it FIFO: the operation the user is waiting for
// lands and is undone in the same breath, with no frame between
// them that ever said the worker was busy. That reads as two dead
// key presses on an unchanged timeline.
//
// The barrier is deliberately NOT widened to every edit. It greys
// the whole shell, which is right for Save/Open/New (the document
// is being replaced under the user) and wrong for a recompute the
// viewport is meant to stay live through — invariant #6 is that
// the render thread keeps orbiting while the kernel works. So the
// refusal is scoped to the two commands whose meaning depends on
// the queue being settled, and says which of the two states the
// shell is in.
if self.pending_jobs != 0 && matches!(was, Tastendruck::Undo | Tastendruck::Redo) {
self.shell.readout =
"Still building — wait for the current operation to finish before undoing."
.to_owned();
return false;
}
match was {
// UNDO ON THE KEYBOARD. It existed as buttons only, and a
// feature nobody can find is close to a feature that is not
@@ -2307,6 +2329,20 @@ impl VernierApp {
}
Tastendruck::Abbrechen => {
self.shell.session = None;
// ESCAPE MUST CANCEL WHAT THE CANCEL BUTTON CANCELS. The
// session's Cut chip writes `producer_target` (and
// `extrude_cut`) into the shell, not into the session, so
// dropping the session alone leaves the target armed with
// nothing on screen naming it — and the next profile closed
// in that session silently extrudes as a cut of that body.
// `Action::CancelEdit` already calls this; the keyboard was
// the half that did not.
//
// Only this arm, not `SkizzeVerlassen` above: leaving a
// sketch keeps `shell.session` standing, so its chip is
// still the live session's and still painted. Clearing the
// target there would desync a session nobody cancelled.
vernier_ui::shell::reset_producer_target(&mut self.shell);
self.shell.card.armed = None;
self.shell.picked_faces.clear();
self.shell.picked_edges.clear();
@@ -2659,3 +2695,78 @@ impl ApplicationHandler for VernierApp {
self.repaint_after = None;
}
}
#[cfg(test)]
mod escape_and_busy_tests {
#![allow(clippy::unwrap_used)]
use super::*;
use crate::camera::Tastendruck;
/// An app whose worker answers a real server, with nothing drained:
/// `pending_jobs` is whatever the test submitted and never falls on
/// its own, so "the worker is busy" is a state the test owns rather
/// than a race it waits on.
fn rig() -> (VernierApp, vernier_doc::BodyId) {
let (mut server, _) = crate::tests::built_server();
let body = server.document.bodies()[0];
let worker =
vernier_ui::Worker::spawn("test-escape-busy", move |edit| server.reply(edit), || {})
.unwrap();
(VernierApp::new(worker), body)
}
#[test]
fn escape_disarms_the_cut_target_the_cancel_button_disarms() {
let (mut app, body) = rig();
// What the bodies footer's Cut chip writes: the target lives in
// the shell, not in the session, so dropping the session alone
// would leave it armed for every later extrude.
app.shell.producer_target = Some(vernier_doc::TargetRequest::Cut(body));
app.shell.extrude_cut = true;
app.apply_tastendruck(Tastendruck::Abbrechen);
assert_eq!(app.shell.producer_target, None);
assert!(!app.shell.extrude_cut);
}
#[test]
fn undo_is_refused_while_the_worker_is_still_building() {
let (mut app, _) = rig();
app.submit(Edit::Recompute);
assert_eq!(app.pending_jobs, 1);
// The point of the finding: an ordinary edit raises no barrier, so
// the old guard could not see this state at all.
assert_eq!(app.document_barrier, 0);
for key in [Tastendruck::Undo, Tastendruck::Redo] {
app.shell.readout.clear();
app.apply_tastendruck(key);
assert_eq!(app.pending_jobs, 1, "{key:?} must not queue behind the job");
assert!(
app.shell.readout.contains("Still building"),
"{key:?} said {:?}",
app.shell.readout
);
}
// The control: once the queue is settled the same key press is an
// ordinary undo again, so the refusal is a wait and not a block.
let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5);
while app.pending_jobs > 0 {
if let Some(scene) = app.worker.try_recv() {
app.apply_reply(scene);
} else {
assert!(
std::time::Instant::now() < deadline,
"the document server did not answer within 5s"
);
std::thread::sleep(std::time::Duration::from_millis(1));
}
}
app.shell.readout.clear();
app.apply_tastendruck(Tastendruck::Undo);
assert_eq!(app.pending_jobs, 1, "an idle worker takes the undo");
assert!(!app.shell.readout.contains("Still building"));
}
}
@@ -41,6 +41,21 @@ pub(crate) fn apply(server: &mut DocumentServer, edit: &Edit) -> Result<bool, St
server.sketch_chain.clear();
server.pending_points.clear();
server.pending_tool = None;
// The chaining tools (Line, Arc, …) resume from `sketch_pending`,
// which names a point of THIS sketch. Deleting that point leaves
// the id dangling, and the next drawing click builds
// `Line { start: Existing(gone), … }`, which the document refuses
// as "unknown entity" — every click after the delete, because
// nothing else clears the field. Only a deleted pending point is
// stale: a delete that spares it leaves a legal chain to continue,
// so the condition is membership rather than an unconditional
// reset.
if server
.sketch_pending
.is_some_and(|pending| points.contains(&pending))
{
server.sketch_pending = None;
}
return Ok(true);
}
Edit::PushPull { face, .. }
@@ -975,3 +990,108 @@ pub(super) fn validate_target(
}
Ok(())
}
#[cfg(test)]
mod sketch_deletion_tests {
#![allow(clippy::unwrap_used)]
use super::*;
use vernier_ui::shell::SketchTool;
/// Clicks one sketch point with the Line tool.
fn draw(server: &mut DocumentServer, at: [f64; 2]) -> crate::scene::Scene {
server.handle(Edit::SketchDraw {
at,
snap_tolerance_mm: None,
tool: SketchTool::Line,
infer: false,
sides: 6,
})
}
/// The ids of the open sketch, curves first.
fn contents(server: &DocumentServer) -> (DocEntityId, Vec<DocEntityId>, Vec<DocEntityId>) {
let sketch = server.sketch_active.unwrap();
let vernier_doc::FeaturePayload::Sketch(data) =
&server.document.features()[&sketch].payload
else {
panic!("the open sketch is a sketch")
};
(
sketch,
data.curves.keys().copied().collect(),
data.points.keys().copied().collect(),
)
}
fn open_sketch(server: &mut DocumentServer) {
server.handle(Edit::NewSketch {
plane: crate::edit::NewSketchPlane::World(vernier_doc::PrincipalPlane::Xy),
});
}
#[test]
fn drawing_survives_deleting_the_chain_it_was_hanging_from() {
let (mut server, _) = crate::tests::built_server();
open_sketch(&mut server);
draw(&mut server, [0.0, 0.0]);
draw(&mut server, [20.0, 0.0]);
let (sketch, curves, points) = contents(&server);
assert_eq!(curves.len(), 1);
assert!(points.contains(&server.sketch_pending.unwrap()));
let deleted = server.handle(Edit::DeleteSketchEntities {
sketch,
curves: curves.clone(),
points: points.clone(),
});
assert!(deleted.error.is_none(), "{:?}", deleted.error);
assert!(
server.sketch_pending.is_none(),
"the chain's end was deleted with it"
);
// The defect: the stale pending id made every later click build
// `Line { start: Existing(deleted) }`, which the document refuses
// as an unknown entity — so nothing could be drawn again.
let first = draw(&mut server, [0.0, 10.0]);
assert!(first.error.is_none(), "{:?}", first.error);
let second = draw(&mut server, [20.0, 10.0]);
assert!(second.error.is_none(), "{:?}", second.error);
let (_, curves, _) = contents(&server);
assert_eq!(curves.len(), 1, "the second chain drew a line");
}
#[test]
fn a_deletion_that_spares_the_chain_end_leaves_the_chain_drawable() {
let (mut server, _) = crate::tests::built_server();
open_sketch(&mut server);
draw(&mut server, [0.0, 0.0]);
draw(&mut server, [20.0, 0.0]);
draw(&mut server, [20.0, 20.0]);
let (sketch, curves, points) = contents(&server);
assert_eq!(curves.len(), 2);
let pending = server.sketch_pending.unwrap();
// The first curve and its far end only: the pending point is the
// chain's live end and is shared with the curve that stays.
let first_curve = curves[0];
let stranded = points[0];
assert_ne!(stranded, pending);
let deleted = server.handle(Edit::DeleteSketchEntities {
sketch,
curves: vec![first_curve],
points: vec![stranded],
});
assert!(deleted.error.is_none(), "{:?}", deleted.error);
assert_eq!(
server.sketch_pending,
Some(pending),
"a delete that spares the chain's end must not drop it"
);
let next = draw(&mut server, [0.0, 20.0]);
assert!(next.error.is_none(), "{:?}", next.error);
let (_, curves, _) = contents(&server);
assert_eq!(curves.len(), 2, "the surviving chain gained a line");
}
}
@@ -184,6 +184,21 @@ fn native_conflict(path: &Path, native: Option<&str>) -> io::Result<()> {
"choose an export path separate from the native document and its recovery files",
));
}
// THE OPEN DOCUMENT IS NOT THE ONLY ONE ON THE MACHINE (FPR-06). The
// check above asks whether this path IS the current document's storage,
// which is a question about identity, and identity cannot see another
// project's document at all — so a typed `.vernier` destination wrote
// binary STL over someone else's part and the next open of it said
// "cannot decode". A name shaped like native storage is refused
// whatever document is open; an export has never needed such a name,
// and unlike Save, an export leaves no `.bak` to recover from.
if vernier_ui::registry::export_path_is_native_storage(path) {
return Err(invalid(
"a .vernier name is a VernierCAD document, not an export destination: choose a name \
outside native storage (the document, its .names sidecar, its .bak or its \
transaction files)",
));
}
Ok(())
}
@@ -453,3 +453,79 @@ fn real_exports_preserve_the_complete_checkpoint_and_actionable_redo() {
.any(|row| row.name == "export history sentinel")
);
}
/// FPR-06: the destination is a typed path, so another project's document is
/// one keystroke away. The identity check could only see the OPEN document,
/// so an STL went over a stranger's part and the next open of it said "cannot
/// decode". Every native-storage NAME is refused now, whatever is open — and
/// the open document's own refusal keeps its wording, which drive scripts
/// read back.
#[test]
fn an_export_refuses_a_native_name_belonging_to_any_document() {
let scratch = Scratch::new();
let other = scratch.0.join("other-project.vernier");
fs::write(&other, b"{\"format\":27}").unwrap();
let open = scratch.0.join("open.vernier");
fs::write(&open, b"{\"format\":27}").unwrap();
for native in [None, Some(open.to_str().unwrap())] {
for leaf in [
"other-project.vernier",
"other-project.vernier.names",
"other-project.vernier.names.bak",
"other-project.vernier.transaction.document.tmp",
] {
let destination = scratch.0.join(leaf);
let Err(failure) = ExportPublication::acquire(
destination.to_str().unwrap(),
native,
ExportFormat::Stl,
) else {
panic!("a document name was accepted as an export destination");
};
assert_eq!(failure.operation, "select export destination");
assert_eq!(failure.phase, ExportPhase::BeforePublication);
assert!(
failure.cause.contains("not an export destination"),
"the refusal does not name the reason: {}",
failure.cause
);
}
}
// Refused BEFORE anything is staged: the fixture is byte-identical and
// the directory gained no staging entry.
assert_eq!(fs::read(&other).unwrap(), b"{\"format\":27}");
let mut left: Vec<String> = fs::read_dir(&scratch.0)
.unwrap()
.map(|entry| entry.unwrap().file_name().to_string_lossy().into_owned())
.collect();
left.sort();
assert_eq!(left, vec!["open.vernier", "other-project.vernier"]);
// The open document's own storage still refuses in its own words.
let Err(same) = ExportPublication::acquire(
open.to_str().unwrap(),
Some(open.to_str().unwrap()),
ExportFormat::Stl,
) else {
panic!("the open document was accepted as an export destination");
};
assert!(
same.cause
.contains("separate from the native document and its recovery files"),
"the current-document refusal changed wording: {}",
same.cause
);
// An ordinary export name in the same directory is still admitted.
assert!(
ExportPublication::acquire(
scratch.0.join("part.stl").to_str().unwrap(),
Some(open.to_str().unwrap()),
ExportFormat::Stl,
)
.is_ok(),
"an ordinary export destination was refused"
);
}
+72 -5
View File
@@ -4116,6 +4116,15 @@ constexpr const char* kStepProduct = "VernierCAD";
// strips exactly the counter — 'VernierCAD' for a root, 'VernierCAD 1',
// 'VernierCAD 2' for the children — and a single-solid export writes the
// same bytes it always did. Anything but digits and dots still fails.
//
// THE TAIL NESTS (measured 2026-09-11, the other half of MB-01): a child
// that is itself a compound is written as a SUB-ASSEMBLY, and its own
// children's positions extend the tail — 'VernierCAD 2.1.1'. The first
// version accepted exactly one dot, so every export of a document holding
// a multi-solid body (the shell's default Join makes one) failed here with
// kUnexpected before it ever reached the assembly guard. Depth is not
// something this rewrite needs to know: the counter is everything before
// the FIRST dot and the rest is positional, at any depth.
bool normalize_step_product(std::string& text, int& replaced) {
const std::string needle = std::string("'") + kStepProduct + " ";
const std::string fixed = std::string("'") + kStepProduct + "'";
@@ -4132,8 +4141,12 @@ bool normalize_step_product(std::string& text, int& replaced) {
const std::size_t begin = at + needle.size();
std::size_t dot = std::string::npos;
for (std::size_t i = begin; i < close; ++i) {
if (text[i] == '.' && i > begin && i + 1 < close && dot == std::string::npos) {
dot = i;
// A dot separates two position digits — never leading, never
// trailing, never doubled — and the FIRST one ends the counter.
if (text[i] == '.' && i > begin && i + 1 < close && text[i - 1] != '.') {
if (dot == std::string::npos) {
dot = i;
}
continue;
}
if (text[i] < '0' || text[i] > '9') {
@@ -4308,6 +4321,59 @@ bool normalize_step_assembly(std::string& text, int& replaced) {
return true;
}
// HOW MANY ROWS THE WRITER OWES, measured against OCCT 7.9.3 (2026-09-11,
// the MB-01 fix). OCCT writes a compound as an assembly TREE: the root
// product gets one NEXT_ASSEMBLY_USAGE_OCCURRENCE per child, and a child
// that is ITSELF a compound becomes a sub-assembly with one row per
// grandchild, recursively — a compound of {compound{s1, s2}, box} writes
// four rows (root->sub, sub->s1, sub->s2, root->box). So the count is the
// number of EDGES of that tree, which is every node but the root.
//
// This replaces a comparison against the BODY count, which was the MB-01
// defect: the shell's default Join target makes a body that is a compound
// of several solids whenever the tool is clear of the target, and such a
// document wrote more rows than it had bodies, so the guard below deleted
// a perfectly good file and returned kUnexpected on every export. Counting
// LEAF SOLIDS would be wrong in the same direction: the two-solid body
// above contributes three rows (its own plus one per solid), not two.
//
// The guard stays strict on purpose — it is the tripwire for an OCCT
// upgrade whose writer changes shape, which is what keeps invariant #1's
// normalization honest — so it is compared against a number this façade
// can predict exactly, not against a bound.
//
// ONE COMPONENT IS NOT AN ASSEMBLY (measured, same day): a compound whose
// only child is a plain shape is written AS that child — one product, no
// rows — while a compound whose only child is another compound is written
// as an assembly. That is OCCT's own `IsAssembly` rule and it has to be
// mirrored here rather than approximated, since it decides both whether a
// node owes rows and whether the edge to it exists at all.
bool writes_as_assembly(const TopoDS_Shape& shape) {
if (shape.ShapeType() != TopAbs_COMPOUND) {
return false;
}
TopoDS_Iterator child(shape);
if (!child.More()) {
return false;
}
const TopoDS_Shape only = child.Value();
child.Next();
return child.More() || only.ShapeType() == TopAbs_COMPOUND;
}
std::size_t assembly_row_count(const TopoDS_Shape& shape) {
// The root hangs from nothing and owes no row; every component of an
// assembly owes exactly one, plus whatever its own subtree owes.
if (!writes_as_assembly(shape)) {
return 0;
}
std::size_t rows = 0;
for (TopoDS_Iterator child(shape); child.More(); child.Next()) {
rows += 1 + assembly_row_count(child.Value());
}
return rows;
}
} // namespace
BooleanResult transform_rigid(std::uint64_t shape, double tx, double ty, double tz,
@@ -4420,8 +4486,9 @@ std::int32_t export_step_compound(rust::Slice<const std::uint64_t> shapes, rust:
return code;
}
// The assembly's own counter (see `normalize_step_assembly`): one usage
// row per child solid, and the count must be exactly that, or the
// writer's shape moved and these bytes must not ship.
// row per EDGE of the assembly tree — `assembly_row_count` is the
// arithmetic — and the count must be exactly that, or the writer's shape
// moved and these bytes must not ship.
try {
const std::string out_path(path);
std::string text;
@@ -4436,7 +4503,7 @@ std::int32_t export_step_compound(rust::Slice<const std::uint64_t> shapes, rust:
}
int replaced = 0;
if (!normalize_step_assembly(text, replaced) ||
static_cast<std::size_t>(replaced) != shapes.size()) {
static_cast<std::size_t>(replaced) != assembly_row_count(compound)) {
std::remove(out_path.c_str());
return kUnexpected;
}
@@ -0,0 +1,114 @@
//! MB-01: a document whose body is a multi-solid compound must export to
//! STEP beside a second body.
//!
//! The shell's default Join target fuses a profile into the selected body
//! even when the tool is clear of it, so a plain Enter makes a body that
//! is a COMPOUND of several solids. Exporting such a document wrote a
//! perfectly good file and then deleted it, because
//! `export_step_compound`'s determinism guard compared OCCT's
//! assembly-usage row count against the number of BODIES — and OCCT writes
//! one row per edge of the assembly tree, so a two-solid body already owes
//! three. The same document also tripped the product-name rewrite, which
//! accepted a positional tail one level deep ('VernierCAD 2.1') and not
//! two ('VernierCAD 2.1.1'), which a sub-assembly writes.
//!
//! Both counts are asserted here against the file's own text: a guard whose
//! arithmetic nobody can read is the thing that broke.
#![allow(clippy::unwrap_used)]
use vernier_occt_sys as facade;
/// Fuse, so the handle is OCCT's own compound rather than one this test
/// assembled — the join the shell performs, with the tool clear of the
/// target.
fn two_solid_body() -> u64 {
let near = facade::make_box(10.0, 10.0, 10.0).unwrap().shape;
let far = facade::make_box(5.0, 5.0, 5.0).unwrap().shape;
let moved = facade::translate_shape(far, 40.0, 0.0, 0.0).unwrap();
let fused = facade::boolean_op(
1, // FUSE
near,
moved.summary.shape,
0.0,
false,
)
.unwrap();
assert_eq!(fused.summary.solids, 2, "the join did not make a compound");
fused.summary.shape
}
fn rows(text: &str) -> usize {
text.matches("NEXT_ASSEMBLY_USAGE_OCCURRENCE('").count()
}
#[test]
fn a_multi_solid_body_exports_to_step_beside_a_second_body() {
let dir = std::env::temp_dir().join(format!(
"vernier-step-compound-assembly-{}",
std::process::id()
));
std::fs::create_dir_all(&dir).unwrap();
let body_one = two_solid_body();
let second = facade::make_box(2.0, 2.0, 2.0).unwrap().shape;
let body_two = facade::translate_shape(second, 0.0, 40.0, 0.0)
.unwrap()
.summary
.shape;
let path = dir.join("two-bodies.step");
facade::export_step_compound(&[body_one, body_two], path.to_str().unwrap()).unwrap();
let text = std::fs::read_to_string(&path).unwrap();
// The arithmetic the guard now uses, spelled out: root->body one,
// body one->each of its two solids, root->body two.
assert_eq!(rows(&text), 4, "the writer's assembly-row count moved");
for ordinal in 1..=4 {
assert!(
text.contains(&format!("NEXT_ASSEMBLY_USAGE_OCCURRENCE('{ordinal}'")),
"row {ordinal} is not numbered by its position in the file"
);
}
// Invariant #1: the process-global counters — the product's and the
// assembly row's — are out of both the names and the ids.
assert!(
!text.contains("NEXT_ASSEMBLY_USAGE_OCCURRENCE('5'"),
"the assembly counter was not normalised"
);
assert!(
text.contains("PRODUCT('VernierCAD 1.1'"),
"a sub-assembly's positional product name did not survive"
);
let again = dir.join("two-bodies-again.step");
facade::export_step_compound(&[body_one, body_two], again.to_str().unwrap()).unwrap();
assert_eq!(
text,
std::fs::read_to_string(&again).unwrap(),
"two exports of one document differ"
);
// It is a real file, not just bytes: read it back and count the solids.
let reimported = facade::import_step(path.to_str().unwrap()).unwrap();
assert_eq!(reimported.solids, 3);
assert!(
(reimported.volume - (1000.0 + 125.0 + 8.0)).abs() <= 1e-9 * 1133.0,
"reimported volume {} is not the three boxes",
reimported.volume
);
facade::release_shape(reimported.shape).unwrap();
// The plain case the guard was written for still counts: two
// single-solid bodies owe exactly two rows.
let flat = dir.join("flat.step");
facade::export_step_compound(&[body_two, body_two], flat.to_str().unwrap()).unwrap();
assert_eq!(rows(&std::fs::read_to_string(&flat).unwrap()), 2);
// And a lone body, whose one-component compound OCCT writes flat —
// no assembly rows at all, which the body-count guard also refused.
let lone = dir.join("lone.step");
facade::export_step_compound(&[body_two], lone.to_str().unwrap()).unwrap();
assert_eq!(rows(&std::fs::read_to_string(&lone).unwrap()), 0);
facade::release_shape(body_one).unwrap();
facade::release_shape(body_two).unwrap();
std::fs::remove_dir_all(&dir).unwrap();
}
+138 -12
View File
@@ -1901,6 +1901,15 @@ pub fn readiness_for_context(
"choose an export path outside this document's files",
);
}
// ANY document's storage, not only this one's (FPR-06): the
// destination field is a typed path, so another project's
// document is one keystroke away and an export over it is
// unrecoverable.
if export_path_is_native_storage(files.destination_path) {
return Readiness::NeedsSelection(
"a .vernier name is a document, not an export destination",
);
}
Readiness::Ready
}
_ => readiness_for(command, selection),
@@ -1927,23 +1936,59 @@ fn non_blank_path(path: &str, why: &'static str) -> Readiness {
pub fn export_conflicts_with_native(export_path: &str, native_path: &str) -> bool {
let export = absolute_path(Path::new(export_path));
let native = absolute_path(Path::new(native_path));
const RESERVED: [&str; 9] = [
"",
".names",
".tmp",
".names.tmp",
".names.bak",
".transaction",
".transaction.tmp",
".transaction.document.tmp",
".transaction.names.tmp",
];
RESERVED
NATIVE_SUFFIXES
.into_iter()
.map(|suffix| append_suffix(&native, suffix))
.any(|reserved| paths_conflict(&export, &reserved))
}
/// Every name the save protocol appends to a native document's path; the
/// empty one is the document itself.
const NATIVE_SUFFIXES: [&str; 9] = [
"",
".names",
".tmp",
".names.tmp",
".names.bak",
".transaction",
".transaction.tmp",
".transaction.document.tmp",
".transaction.names.tmp",
];
/// The extension every VernierCAD document carries; the native file dialog
/// filters on it and `Save` appends it.
const NATIVE_EXTENSION: &str = ".vernier";
/// Whether an export destination is shaped like a native document or one of
/// its recovery files — for ANY document, not just the open one.
///
/// [`export_conflicts_with_native`] answers by identity, and identity can
/// only speak for the document this process has open. That left every other
/// `.vernier` on the machine writable: typing another project's document
/// path as the STL destination replaced it with 684 bytes of binary STL and
/// the next open of that project said "cannot decode" (FPR-06). The other
/// document is not open and there is nothing to compare it against, so the
/// name's SHAPE has to answer instead — a `.vernier` leaf, or a `.vernier`
/// leaf carrying one of the sidecar or transaction suffixes above.
///
/// This is a refusal and not a warning because an export keeps no `.bak`:
/// the overwritten design is gone, and no export has ever needed such a
/// name.
#[must_use]
pub fn export_path_is_native_storage(export_path: &str) -> bool {
let Some(leaf) = Path::new(export_path).file_name() else {
return false;
};
let leaf = leaf.to_string_lossy();
// Strip one whole suffix, then ask about the extension underneath it:
// `part.vernier.names.tmp` is native storage, `part.names.tmp` is not.
NATIVE_SUFFIXES.into_iter().any(|suffix| {
leaf.strip_suffix(suffix)
.is_some_and(|stem| stem.ends_with(NATIVE_EXTENSION))
})
}
fn append_suffix(path: &Path, suffix: &str) -> PathBuf {
let mut value = path.as_os_str().to_os_string();
value.push(suffix);
@@ -2750,6 +2795,87 @@ mod tests {
));
}
/// FPR-06: the guard above can only speak for the OPEN document, so a
/// second project's document was writable by name. Every native shape is
/// refused whatever is open, and nothing an export is actually called is.
#[test]
fn exports_refuse_any_documents_storage_name_not_only_this_ones() {
for suffix in [
"",
".names",
".tmp",
".names.tmp",
".names.bak",
".transaction",
".transaction.tmp",
".transaction.document.tmp",
".transaction.names.tmp",
] {
let other = format!("/tmp/some-other-project/part.vernier{suffix}");
assert!(
export_path_is_native_storage(&other),
"another document's storage name {other} was accepted"
);
// With /tmp/vernier/part.vernier open, the identity guard
// PERMITS this stranger's document — which is the defect.
assert!(
!export_conflicts_with_native(&other, "/tmp/vernier/part.vernier"),
"the identity guard was expected to miss {other}"
);
}
for ordinary in [
"/tmp/part.stl",
"/tmp/part.step",
"/tmp/vernier-parts/part.names.tmp",
"/tmp/part.vernier.stl",
"/tmp/vernier/",
] {
assert!(
!export_path_is_native_storage(ordinary),
"ordinary export name {ordinary} was refused"
);
}
// The command is dimmed before the click, and the CURRENT document's
// refusal keeps its own wording — drive scripts read it.
let files = FileCommandContext {
native_path: "/tmp/vernier/part.vernier",
destination_path: "/tmp/some-other-project/part.vernier",
has_exportable_body: true,
};
for action in [Action::ExportStl, Action::ExportStep] {
let command = describe(action).unwrap_or(&BLANK);
assert_eq!(
readiness_for_context(command, Selection::None, files),
Readiness::NeedsSelection(
"a .vernier name is a document, not an export destination"
)
);
assert_eq!(
readiness_for_context(
command,
Selection::None,
FileCommandContext {
destination_path: "/tmp/vernier/part.vernier.names",
..files
}
),
Readiness::NeedsSelection("choose an export path outside this document's files")
);
assert_eq!(
readiness_for_context(
command,
Selection::None,
FileCommandContext {
destination_path: "/tmp/some-other-project/part.stl",
..files
}
),
Readiness::Ready
);
}
}
#[cfg(unix)]
#[test]
#[allow(clippy::expect_used)]
+46
View File
@@ -3255,6 +3255,50 @@ pub fn dimension_value_of(constraint: vernier_doc::SketchConstraint) -> f64 {
}
}
/// Where this frame's keyboard focus was BEFORE anything painted.
///
/// **THE KEYSTROKE DESTROYS THE ANSWER, SO THE ANSWER IS TAKEN FIRST**
/// (HV-01 / R2B-01). egui's single-line `TextEdit` calls
/// `Memory::surrender_focus` the moment it reads an Enter ("End input with
/// enter"), and every text field this shell paints — the title bar's path
/// box, the Variables name and expression fields, the Items new-folder box —
/// is painted BEFORE the value card, which is painted last so it floats.
/// So a card that asks `Memory::focused()` at the foot of the frame is told
/// "nobody" by the very field that just took the key, and takes the key too.
/// Measured with the real shell under `vernier-drive`: focus reads
/// `Some(id_879F)` (the Variables name field) on the frames carrying its
/// clicks and its typed text, and `None` on the one frame carrying the
/// Enter.
///
/// A REPEATED PASS IS NOT A NEW FRAME. `Context::request_discard` replays
/// the same `RawInput`, and by pass two the foreign field has already
/// surrendered — refreshing the snapshot per pass would hand the card the
/// very answer this exists to refuse. `RawInput::time` is stamped once per
/// frame and shared by its passes, so it is the discriminator; two frames
/// that somehow shared a timestamp would keep a stale snapshot for one
/// frame, which only ever REFUSES an Enter, never invents one.
fn record_focus_at_frame_start(ctx: &egui::Context) {
let key = egui::Id::new("vernier-focus-at-frame-start");
let now = ctx.input(|input| input.time);
let focused = ctx.memory(egui::Memory::focused);
ctx.data_mut(|data| {
let previous: Option<(f64, Option<egui::Id>)> = data.get_temp(key);
if previous.is_none_or(|(at, _)| at != now) {
data.insert_temp(key, (now, focused));
}
});
}
/// The widget that held keyboard focus when this frame began, as recorded by
/// [`record_focus_at_frame_start`]. `None` also means "not recorded", which
/// is the same answer a frame with nothing focused gives.
pub(super) fn focus_at_frame_start(ctx: &egui::Context) -> Option<egui::Id> {
ctx.data(|data| {
data.get_temp::<(f64, Option<egui::Id>)>(egui::Id::new("vernier-focus-at-frame-start"))
})
.and_then(|(_, id)| id)
}
/// Draws the shell for one frame (from within [`egui::Context::run_ui`])
/// and reports what the user asked for.
///
@@ -3272,6 +3316,8 @@ pub fn dimension_value_of(constraint: vernier_doc::SketchConstraint) -> f64 {
/// epaint panics on the first one, several frames of call stack away from the
/// omission. Failing here names the fix.
pub fn show(ui: &mut egui::Ui, view: &SceneView, state: &mut ShellState) -> ShellResponse {
// FIRST, BEFORE ANY WIDGET OF THIS FRAME PAINTS — see the function.
record_focus_at_frame_start(ui.ctx());
state.value_card_rect = None;
state.timeline_controls.clear();
state.timeline_scroll_rect = None;
+496 -48
View File
@@ -673,6 +673,149 @@ pub(super) fn field_changed(state: &mut ShellState, at: usize) {
}
}
/// Why a typed field did not become a value.
///
/// **TWO REFUSALS, NOT ONE**, because they need different sentences: a
/// string that is not an expression is the user's typing, while a number
/// outside the command's own range is a real quantity the command will not
/// take, and only the second can name the bound it missed (SF-07/FC-05/SB-10).
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(super) enum FieldRefusal {
/// Not a well-formed expression, or the wrong dimension for this field.
Invalid,
/// A finite value the descriptor's range excludes.
OutOfRange,
/// A length expression this field cannot hand to the document without
/// changing its value — see [`stored_expression`].
UnitAmbiguous,
}
/// The sentence one [`FieldRefusal`] gets.
fn refusal_message(spec: &registry::ParamSpec, why: FieldRefusal) -> String {
match why {
FieldRefusal::OutOfRange => range_message(spec),
FieldRefusal::Invalid => "Invalid expression, units, or parameter range".to_owned(),
FieldRefusal::UnitAmbiguous => {
"Write a unit on each number in this expression — the document keeps it in millimetres"
.to_owned()
}
}
}
/// The sentence a [`FieldRefusal::OutOfRange`] gets, naming the bound.
///
/// In canonical units, whatever the display unit is: the range lives with
/// the command (`registry::ParamSpec::range`) and is stated in millimetres
/// or degrees, and converting it for the readout would invent a bound the
/// command does not hold. A count has no unit worth printing — "between 1
/// and 64 ×" reads as a typo — so only a real unit is appended.
fn range_message(spec: &registry::ParamSpec) -> String {
let (low, high) = (*spec.range.start(), *spec.range.end());
let unit = match spec.unit {
registry::Unit::Mm | registry::Unit::Deg => format!(" {}", spec.unit.suffix()),
registry::Unit::Count | registry::Unit::Ratio => String::new(),
};
match (low > f64::MIN, high < f64::MAX) {
(true, true) => format!("{} must be between {low} and {high}{unit}", spec.key),
(true, false) => format!("{} must be at least {low}{unit}", spec.key),
(false, true) => format!("{} must be at most {high}{unit}", spec.key),
(false, false) => format!("{} is out of range", spec.key),
}
}
/// What one card field MEANS: the dimension its expression must have, and
/// what an unadorned number in it is worth in canonical units.
///
/// **ONE PREDICATE**, because [`parse_field`] and [`stored_expression`] have
/// to agree exactly: the first decides the number this Enter applies and the
/// second decides the text the document keeps, and a field where those two
/// disagree is a value that changes the next time it is evaluated.
fn field_domain(
state: &ShellState,
action: Action,
spec: &registry::ParamSpec,
) -> (vernier_doc::expressions::Dimension, f64) {
use vernier_doc::expressions::Dimension;
let angle = spec.unit == registry::Unit::Deg
|| (action == Action::EditSketchDimension
&& state
.picked_dimension
.is_some_and(|picked| picked.kind == DimensionKind::Angle));
if angle {
return (Dimension::Angle, std::f64::consts::PI / 180.0);
}
if matches!(spec.unit, registry::Unit::Count | registry::Unit::Ratio) {
return (Dimension::Unitless, 1.0);
}
(
Dimension::Length,
state.settings.appearance.length_unit.mm_per_unit(),
)
}
/// The expression text the DOCUMENT must keep for what was typed here.
///
/// **THE DISPLAY UNIT DIES AT THE CARD'S EDGE** (R2A-02). A field whose text
/// is not a bare number is kept as a live binding, and the server
/// re-evaluates that string for every later compile with a `bare_scale` of
/// 1.0 — millimetres, because millimetres are the document's only unit
/// (invariant #8). So the card's reading and the document's reading of one
/// string differ by exactly the display scale wherever an unadorned number
/// carries the value. Measured in inches: "15mm" stored 15, "1in" stored
/// 25.4, and "0.5+0.5" stored 1.0 where the card had just computed 25.4.
///
/// The fix is to make the stored text say what the field meant. A unit
/// suffix binds to one primary — "a number, variable or parenthesized
/// expression" — so `(0.5+0.5) in` is the same quantity with its unit
/// written down, and being explicit it reads 25.4 under either `bare_scale`.
///
/// **THE REWRITE IS VERIFIED, NOT REASONED** — and that is what makes the
/// third case visible. Three things can happen and the check tells them
/// apart by evaluating, never by inspecting the text:
///
/// * the value does not move with `bare_scale` at all (`15mm`, `2*height`) —
/// kept verbatim, and wrapping it would make it a length squared;
/// * the whole expression is unadorned (`0.5+0.5`) — the wrap reproduces the
/// card's own number exactly, so the wrap is what is stored;
/// * a bare number is promoted INSIDE an explicit one (`height+1`, where the
/// `1` means one display unit and `height` already holds millimetres) —
/// no wrap of the whole string can express that, because half of it must
/// scale and half must not. There is no honest text, so the field is
/// refused with a sentence rather than stored as a number nobody typed.
fn stored_expression(
state: &ShellState,
action: Action,
spec: &registry::ParamSpec,
text: &str,
) -> Result<String, FieldRefusal> {
use vernier_doc::expressions::{Dimension, evaluate_expression};
let (dimension, bare_scale) = field_domain(state, action, spec);
// Angles already agree: the card and the server both scale a bare number
// by pi/180, because degrees are the UI boundary everywhere rather than
// a setting.
if dimension != Dimension::Length || bare_scale == 1.0 {
return Ok(text.to_owned());
}
let variables = &state.expression_variables;
let canonical = evaluate_expression(text, variables, dimension, bare_scale)
.map_err(|_| FieldRefusal::Invalid)?;
let reads_the_same = |candidate: &str| {
evaluate_expression(candidate, variables, dimension, 1.0)
.is_ok_and(|millimetres| millimetres == canonical)
};
if reads_the_same(text) {
return Ok(text.to_owned());
}
let wrapped = format!(
"({text}) {}",
state.settings.appearance.length_unit.suffix()
);
if reads_the_same(&wrapped) {
return Ok(wrapped);
}
Err(FieldRefusal::UnitAmbiguous)
}
fn parse_field(
state: &ShellState,
action: Action,
@@ -680,40 +823,24 @@ fn parse_field(
text: &str,
diameter: bool,
existing: bool,
) -> Option<f64> {
) -> Result<f64, FieldRefusal> {
use vernier_doc::expressions::{Dimension, evaluate_expression};
let angle = spec.unit == registry::Unit::Deg
|| (action == Action::EditSketchDimension
&& state
.picked_dimension
.is_some_and(|picked| picked.kind == DimensionKind::Angle));
let dimension = if angle {
Dimension::Angle
} else if matches!(spec.unit, registry::Unit::Count | registry::Unit::Ratio) {
Dimension::Unitless
} else {
Dimension::Length
};
let bare_number = text.trim().parse::<f64>().is_ok();
let length = !angle && !matches!(spec.unit, registry::Unit::Count | registry::Unit::Ratio);
let display_scale = if length {
state.settings.appearance.length_unit.mm_per_unit()
} else {
1.0
};
let canonical = evaluate_expression(
text,
&state.expression_variables,
dimension,
if angle {
std::f64::consts::PI / 180.0
} else if bare_number && length {
display_scale
} else {
1.0
},
)
.ok()?;
let (dimension, bare_scale) = field_domain(state, action, spec);
let angle = dimension == Dimension::Angle;
let length = dimension == Dimension::Length;
let display_scale = if length { bare_scale } else { 1.0 };
// **THE DISPLAY UNIT IS THE FIELD'S UNIT, NOT THE LITERAL'S** (R2A-02).
// This used to hand `display_scale` to the evaluator only when the WHOLE
// string parsed as a bare float, so in inches "1" stored 25.4 mm and
// "0.5+0.5" stored 1.0 — a 25.4x error, painted with an "in" suffix
// beside it. `bare_scale` is already the right instrument for the whole
// job: `expressions::run` applies it to an IMPLICIT result only, and a
// subtree carrying a unit token or a variable (which already holds
// millimetres) is explicit and untouched. So the rule the field wants —
// "unadorned numbers mean what the field says they mean" — is the
// evaluator's own rule, and the gate was the bug.
let canonical = evaluate_expression(text, &state.expression_variables, dimension, bare_scale)
.map_err(|_| FieldRefusal::Invalid)?;
let typed = if angle {
canonical.to_degrees()
} else if length {
@@ -721,9 +848,13 @@ fn parse_field(
} else {
canonical
};
if spec.unit == registry::Unit::Count && (typed.fract() != 0.0 || !spec.range.contains(&typed))
{
return None;
if spec.unit == registry::Unit::Count {
if typed.fract() != 0.0 {
return Err(FieldRefusal::Invalid);
}
if !spec.range.contains(&typed) {
return Err(FieldRefusal::OutOfRange);
}
}
let value = if action == Action::EditDimension {
if diameter { canonical / 2.0 } else { canonical }
@@ -734,16 +865,28 @@ fn parse_field(
// Existing authored dimensions use the document domain. A valid
// sub-millimetre target must not be raised to the creation-card floor.
if action != Action::ConstrainAngle && value < 1e-9 {
return None;
return Err(FieldRefusal::Invalid);
}
value
} else if action == Action::ConstrainAngle {
if !spec.range.contains(&value) {
return None;
return Err(FieldRefusal::OutOfRange);
}
value
} else if spec.range.contains(&value) {
value
} else {
value.clamp(*spec.range.start(), *spec.range.end())
// **A TYPED NUMBER OUTSIDE THE RANGE IS REFUSED, NOT CLAMPED**
// (SF-07 / FC-05 / SB-10). The clamp is still right where it was
// written — `value_card::set_param`, which is what a gizmo drag and
// a re-seed go through, and `registry::mm`'s own comment says so:
// it keeps a DRAG from ever asking the command for something it
// would refuse. A typed 0 is not a drag. Clamping it turned an
// extrude height of 0 into a 0.001 mm wafer, a fillet of -2 into an
// invisible 0.001 mm row and a loft offset of 0 into 0.001, each
// with no error and no warning, and each leaving the card painting
// the number the user typed over a document holding another one.
return Err(FieldRefusal::OutOfRange);
};
let value = if spec.unit == registry::Unit::Count {
value.round()
@@ -755,7 +898,10 @@ fn parse_field(
} else {
value
};
canonical.is_finite().then_some(canonical)
canonical
.is_finite()
.then_some(canonical)
.ok_or(FieldRefusal::Invalid)
}
fn reconcile_diameter(
@@ -779,6 +925,7 @@ fn reconcile_diameter(
state.card.diameter,
draft.existing,
)
.ok()
})
} else {
Some(draft.source.numbers[0])
@@ -853,19 +1000,30 @@ pub(super) fn prepare(state: &mut ShellState, action: Action) -> bool {
continue;
}
if draft.edited[at] {
let Some(value) = draft.converted[at].or_else(|| {
parse_field(
let parsed = match draft.converted[at] {
Some(value) => Ok(value),
None => parse_field(
state,
action,
spec,
&state.card.buffers[at],
state.card.diameter,
draft.existing,
)
}) else {
state.readout = "Invalid expression, units, or parameter range".into();
state.card.draft = Some(draft);
return false;
),
};
let value = match parsed {
Ok(value) => value,
Err(why) => {
// THE RANGE IS NAMED WHEN IT IS THE RANGE THAT REFUSED.
// "Invalid expression, units, or parameter range" was
// the only sentence this card had, and it never reached
// a typed 0 at all, because that was clamped rather than
// refused (SF-07). A refusal a person cannot act on is
// barely better than the silence it replaces.
state.readout = refusal_message(spec, why);
state.card.draft = Some(draft);
return false;
}
};
candidate.numbers[at] = value;
let text = state.card.buffers[at].trim();
@@ -876,13 +1034,43 @@ pub(super) fn prepare(state: &mut ShellState, action: Action) -> bool {
} else {
spec.key.into()
},
expression: text.into(),
// NOT THE RAW BUFFER — see `stored_expression`. A bare
// number never reaches here (it parses as `f64` above
// and is stored as a value), so this is exactly the set
// of strings the document will re-evaluate later.
expression: match stored_expression(state, action, spec, text) {
Ok(expression) => expression,
Err(why) => {
state.readout = refusal_message(spec, why);
state.card.draft = Some(draft);
return false;
}
},
});
}
}
}
sync_extrude_candidate(&mut candidate, action);
sync_hole_candidate(&mut candidate, action);
// **A PUSH/PULL OF ZERO MOVES NOTHING** (DISC-08). Its range is
// `MIN..=MAX` on purpose — a push/pull runs both ways and a floor would
// forbid the pull half of its own name — so the range refusal above
// cannot catch the one value that is not a distance at all. The field
// opens at 0.00 because a GIZMO drag starts where the face already is,
// which is exactly why an Enter on the untouched field was so easy to
// press: the kernel then minted a feature, refused the prism, rolled it
// back and handed the user "kernel facade: invalid argument".
//
// **CREATION ONLY, NOT `SetPushPullDistance`.** Re-dimensioning an
// existing push/pull to 0 already refuses at the command with a sentence
// that names the number — "invalid dimension 0 mm" — and
// `functions-push-pull-edit.json` pins exactly that. It is the CREATION
// path that has no such check, and it is the one whose field opens at 0.
if action == Action::PushPull && candidate.numbers.first().is_some_and(|mm| *mm == 0.0) {
state.readout = "Type a distance to push or pull this face; 0 moves nothing".into();
state.card.draft = Some(draft);
return false;
}
let changed =
!draft.existing || !candidate.equivalent(&draft.source, action) || !expressions.is_empty();
state.field_expressions = expressions;
@@ -1150,6 +1338,266 @@ mod tests {
assert_eq!(card_parse(&state, Action::Section, &spec, 1.0), 10.0);
}
/// **R2A-02.** An unadorned number and an unadorned *sum* are the same
/// quantity, and the field's unit governs both. The old gate — "scale by
/// the display unit only when the whole string parses as a float" — made
/// "1" 25.4 mm in inches and "0.5+0.5" 1.0 mm, a 25.4x error painted
/// with an "in" suffix beside it.
///
/// EVERY SPELLING AT EVERY UNIT, because the rule is a relationship
/// between them rather than a property of one: a unit token and a
/// variable (which already holds millimetres) must be INDIFFERENT to the
/// display unit, and a bare number must follow it. Revert the gate and
/// six of these fifteen go red.
/// **R2A-02, WHERE THE NUMBER ACTUALLY GOES.** `parse_field` was right
/// and the document was still wrong: a field whose text is not a bare
/// number is kept as a live BINDING, and the server re-evaluates that
/// string with a `bare_scale` of 1.0 for every later compile, because
/// millimetres are the document's only unit. So the card's reading and
/// the document's reading of one string differed by exactly the display
/// scale. Driven in inches: "15mm" saved 15.0, "1in" saved 25.4 and
/// "0.5+0.5" saved 1.0 where the card had just computed 25.4.
///
/// THE ASSERTION IS THE ROUND TRIP, not the spelling: whatever text is
/// stored must mean, in millimetres, exactly what the card applied. The
/// spelling is checked too, but only to pin that an expression which
/// already carries a unit is left alone — wrapping `15mm` in another
/// unit would make it a length squared.
#[test]
fn the_expression_the_document_keeps_means_what_the_card_applied() {
use crate::settings::LengthUnit;
use vernier_doc::expressions::{Dimension, Quantity, evaluate_expression};
let spec = registry::ParamSpec {
key: "height",
unit: registry::Unit::Mm,
range: 0.001..=f64::MAX,
seed: registry::Seed::Last,
};
for unit in [LengthUnit::Mm, LengthUnit::In, LengthUnit::Cm] {
let mut state = ShellState::default();
state.settings.appearance.length_unit = unit;
state.expression_variables.insert(
"height".to_owned(),
Quantity {
value: 7.0,
dimension: Dimension::Length,
},
);
for text in ["0.5+0.5", "1in", "15mm", "2*height"] {
let applied = parse_field(&state, Action::Extrude, &spec, text, false, false)
.unwrap_or_else(|why| panic!("{unit:?} refused {text:?}: {why:?}"));
let stored = stored_expression(&state, Action::Extrude, &spec, text)
.unwrap_or_else(|why| panic!("{unit:?} would not store {text:?}: {why:?}"));
// THE DOCUMENT'S OWN READING: `bare_scale` 1.0, which is
// what `server::expressions::wrapped` passes for a length.
let reread = evaluate_expression(
&stored,
&state.expression_variables,
Dimension::Length,
1.0,
)
.unwrap_or_else(|why| {
panic!("{unit:?} stored {stored:?}, which will not evaluate: {why}")
});
assert!(
(reread - applied).abs() < 1e-9,
"{unit:?}: typed {text:?} applied {applied} mm but the document keeps {stored:?}, worth {reread} mm"
);
}
// **THE ONE THAT HAS NO HONEST TEXT**: `height` already holds
// millimetres and the `1` beside it means one display unit, so
// half the expression must scale and half must not. In
// millimetres the two agree and it is kept; anywhere else it is
// refused rather than silently stored as a different number
// (in inches the card reads 32.4 mm and the document would read
// 8).
let mixed = stored_expression(&state, Action::Extrude, &spec, "height+1");
if unit == LengthUnit::Mm {
assert_eq!(mixed.as_deref(), Ok("height+1"));
} else {
assert_eq!(
mixed,
Err(FieldRefusal::UnitAmbiguous),
"{unit:?} stored a mixed expression whose value it cannot preserve"
);
}
// An expression that already states a unit, or that is anchored
// to a variable holding millimetres, is kept verbatim.
for text in ["1in", "15mm", "2*height"] {
assert_eq!(
stored_expression(&state, Action::Extrude, &spec, text).as_deref(),
Ok(text),
"{unit:?} rewrote an expression that was already explicit"
);
}
}
// And the rewrite happens only where it is needed.
let mut state = ShellState::default();
state.settings.appearance.length_unit = LengthUnit::Mm;
assert_eq!(
stored_expression(&state, Action::Extrude, &spec, "0.5+0.5").as_deref(),
Ok("0.5+0.5"),
"millimetres are the document's own unit and need no suffix"
);
state.settings.appearance.length_unit = LengthUnit::In;
assert_eq!(
stored_expression(&state, Action::Extrude, &spec, "0.5+0.5").as_deref(),
Ok("(0.5+0.5) in")
);
}
#[test]
fn a_bare_expression_follows_the_display_unit_and_an_explicit_one_does_not() {
use crate::settings::LengthUnit;
use vernier_doc::expressions::{Dimension, Quantity};
let spec = registry::ParamSpec {
key: "height",
unit: registry::Unit::Mm,
range: 0.001..=f64::MAX,
seed: registry::Seed::Last,
};
for (unit, scale) in [
(LengthUnit::Mm, 1.0),
(LengthUnit::In, 25.4),
(LengthUnit::Cm, 10.0),
] {
let mut state = ShellState::default();
state.settings.appearance.length_unit = unit;
state.expression_variables.insert(
"height".to_owned(),
Quantity {
value: 7.0,
dimension: Dimension::Length,
},
);
let parse = |text: &str| {
parse_field(&state, Action::Extrude, &spec, text, false, false)
.unwrap_or_else(|why| panic!("{unit:?} refused {text:?}: {why:?}"))
};
// The two that used to disagree with each other.
assert!(
(parse("1") - scale).abs() < 1e-9,
"{unit:?}: a bare 1 is one display unit"
);
assert!(
(parse("0.5+0.5") - scale).abs() < 1e-9,
"{unit:?}: an arithmetic 1 is the same 1"
);
// An explicit unit says what it means, whatever the field shows.
assert!((parse("1in") - 25.4).abs() < 1e-9, "{unit:?}: 1in is 1in");
assert!(
(parse("15mm") - 15.0).abs() < 1e-9,
"{unit:?}: 15mm is 15mm"
);
// A binding is already millimetres and is never re-scaled.
assert!(
(parse("2*height") - 14.0).abs() < 1e-9,
"{unit:?}: a variable carries its own unit"
);
}
}
/// **SF-07 / FC-05 / SB-10.** A typed number the command would refuse is
/// refused HERE, with the bound in the readout, instead of being clamped
/// to the bound and applied — which is how an extrude height of 0 became
/// a 1.2 mm³ wafer and a fillet of -2 became an invisible 0.001 mm row.
///
/// THE CLAMP ITSELF IS NOT UNDER TEST and must survive: the sibling
/// assertion below drives the same descriptor through `set_param`, the
/// path a gizmo drag and a re-seed take, and still expects the clamp.
#[test]
fn a_typed_value_outside_the_range_is_refused_and_names_the_bound() {
let spec = registry::ParamSpec {
key: "height",
unit: registry::Unit::Mm,
range: 0.001..=f64::MAX,
seed: registry::Seed::Last,
};
let state = ShellState::default();
for text in ["0", "-5"] {
assert_eq!(
parse_field(&state, Action::Extrude, &spec, text, false, false),
Err(FieldRefusal::OutOfRange),
"{text} was accepted into a 0.001..=MAX field"
);
}
assert_eq!(
range_message(&spec),
"height must be at least 0.001 mm",
"the refusal has to say what would be accepted"
);
assert!(
parse_field(&state, Action::Extrude, &spec, "0.5", false, false).is_ok(),
"a value inside the range still parses"
);
// The drag path keeps its clamp: it never asks for a refusal.
let mut clamped = ShellState::default();
super::set_param(&mut clamped, Action::Extrude, &spec, -5.0);
assert!(
clamped.extrude_height_mm > 0.0,
"the gizmo-drag clamp was removed with the typed one"
);
// THE THREE REPORTED FIELDS, THROUGH THEIR OWN DESCRIPTORS rather
// than through the hand-built spec above: the finding is about what
// `registry::mm` declares, so a range edited there must reach this
// test rather than leave it passing on a local copy.
for (action, key) in [
(Action::SetExtrudeHeight, "height"),
(Action::SetFilletSpec, "radius"),
(Action::AddLoftSection, "offset"),
] {
let command = registry::describe(action).expect("a declared command");
let spec = command
.params
.iter()
.find(|spec| spec.key == key)
.expect("the reported parameter");
assert_eq!(
parse_field(&state, action, spec, "0", false, false),
Err(FieldRefusal::OutOfRange),
"{action:?}'s {key} still takes a typed 0"
);
}
}
/// **DISC-08.** The push/pull field opens at 0.00 — right for a gizmo
/// drag, which starts where the face is — and its range is deliberately
/// unbounded, so nothing else in the card can catch the one value that
/// is not a distance. Without this refusal the kernel minted a feature,
/// rolled it back, and answered "kernel facade: invalid argument".
#[test]
fn a_zero_length_push_pull_is_refused_at_the_card() {
let mut state = ShellState {
picked_faces: vec![7],
picked_planar: true,
..ShellState::default()
};
let view = SceneView::default();
reconcile_parameter_card(&mut state, &view);
assert_eq!(
state.card.action,
Some(Action::PushPull),
"the picked face did not float the push/pull card"
);
assert_eq!(state.card.buffers, vec!["0.00".to_owned()]);
assert!(
!prepare(&mut state, Action::PushPull),
"a 0 mm push/pull reached the dispatch"
);
assert!(
state.readout.contains("push or pull"),
"no sentence a person can act on: {:?}",
state.readout
);
// A distance the user typed still goes through.
state.card.buffers[0] = "4".to_owned();
field_changed(&mut state, 0);
assert!(prepare(&mut state, Action::PushPull));
assert!((state.distance_mm - 4.0).abs() < 1e-9);
}
#[test]
fn two_sided_backward_number_updates_extent_before_chip_write() {
let (mut state, _view) = feature(
+544 -11
View File
@@ -48,6 +48,21 @@ pub struct CardState {
/// card at the foot of the viewport, which is where it goes when the
/// operation has no gizmo to hang from.
pub anchor: Option<[f32; 2]>,
/// Whether the last thing this card did was DISPATCH, with no deliberate
/// input since.
///
/// **AN AIM IS SPENT, BUT THE SELECTION IS NOT**, and that is the whole
/// of PV-07: Enter runs the command, the picks that fed it are still
/// standing, and [`card_showing`] immediately derives the next card those
/// picks offer — hole on the two points that were just drilled, draft on
/// the face that was just chamfered. A reflexive second Enter, meant as
/// "yes, confirm", then dispatched a DIFFERENT command at its default.
/// So a dispatch spends the keystroke too: `committed` read from egui's
/// global input is ignored until something deliberate happens — a ribbon
/// press that aims the card, a keystroke into a field, a chip, or any
/// pointer press at all. A button on the card itself is always deliberate
/// and never consults this.
pub(super) spent: bool,
}
impl CardState {
@@ -59,6 +74,10 @@ impl CardState {
/// `draft · angle` caption is a number the next Enter would apply.
pub fn arm(&mut self, action: Action) {
self.armed = Some(action);
// A PRESS IS A DELIBERATE TOUCH, so it re-arms Enter after a dispatch
// spent it: "press fillet, press Enter" has to keep working, and the
// press is exactly the signal PV-07's bare second Enter lacks.
self.spent = false;
if self.action == Some(action) {
return;
}
@@ -72,6 +91,15 @@ impl CardState {
self.armed = None;
self.action = None;
self.buffers.clear();
// **THE LEDGER IS NOT CLEARED HERE** (PV-07). It used to be, on the
// reasoning that the card this flag belonged to is gone — but the
// card going is exactly the hazard: a commit recompiles, no command
// is derivable for a frame or two while the worker answers, and the
// card that comes back is the NEXT one these picks offer. Forgetting
// the dispatch across that gap is what let a second Enter run it
// (measured: a sketch fillet committed, then a bare Enter appended a
// hole). Only a deliberate touch re-arms the keystroke, and a ribbon
// press takes `arm` rather than this.
}
}
@@ -642,6 +670,46 @@ pub(crate) fn value_card(
selection: Selection,
response: &mut ShellResponse,
) {
// **THE SPENT-KEYSTROKE LEDGER, SETTLED BEFORE ANYTHING ELSE** (PV-07).
//
// A DISPATCH THE CARD DID NOT MAKE SPENDS THE KEYSTROKE TOO, and it is
// read on entry because the card is painted LAST: a ribbon press that
// runs its command outright — `hole` on two picked points, `fillet` on a
// picked edge — has already put its action in `response` by the time the
// card paints, and the picks that fed it are still standing, so
// `card_showing` immediately offers the next command those picks derive.
// The reflexive second Enter, meant as "yes, confirm", then ran that
// DIFFERENT command at its default: measured as features 3 → 4 → 5, two
// holes for one ribbon press. Marking `spent` only inside this card's own
// commit branches could never catch that, because the card never
// committed.
//
// A DELIBERATE TOUCH RE-ARMS ENTER. Anything that is not another bare
// Enter counts: a pointer press (picking a face, pressing a ribbon
// button, clicking a chip), typed text, a paste. They are read as EVENTS
// rather than as widget responses because the gesture that matters may
// land anywhere in the shell — the card only has to know that the user
// has moved on from the command it just ran.
//
// THE ORDER IS THE POINT: the dispatch is marked AFTER the re-arm, since
// the ribbon press that dispatched is itself a pointer press and would
// otherwise clear the flag in the very frame it is set. And both run
// BEFORE `card_showing`, so a frame with no card at all — which is every
// frame between a commit and the recompile that answers it — keeps the
// ledger rather than resetting it.
if ui.input(|input| {
input.events.iter().any(|event| {
matches!(
event,
egui::Event::PointerButton { pressed: true, .. }
| egui::Event::Text(_)
| egui::Event::Paste(_)
)
})
}) {
state.card.spent = false;
}
state.card.spent |= response.action.is_some();
let Some(command) = card_showing(state, selection) else {
// Nothing to type: forget the buffers so the next command seeds fresh
// rather than inheriting a number that belonged to something else.
@@ -685,8 +753,12 @@ pub(crate) fn value_card(
);
let mut committed = false;
let mut committed_by_key = false;
let mut preview_requested = false;
let mut cancelled = false;
// THE IDS OF THIS CARD'S OWN FIELDS, collected while they are painted —
// see the key read at the foot of the card for what they decide.
let mut fields: Vec<egui::Id> = Vec::new();
let mut painted = false;
let area = egui::Area::new(egui::Id::new("vernier-value-card"))
@@ -724,12 +796,15 @@ pub(crate) fn value_card(
ui.visuals_mut().text_cursor.stroke =
egui::Stroke::new(7.0, theme.accent);
ui.visuals_mut().selection.bg_fill = theme.accent.gamma_multiply(0.25);
ui.add(
egui::TextEdit::singleline(buffer)
.desired_width(140.0)
.font(theme::mono(15.0))
.frame(egui::Frame::NONE)
.margin(egui::Margin::ZERO),
fields.push(
ui.add(
egui::TextEdit::singleline(buffer)
.desired_width(140.0)
.font(theme::mono(15.0))
.frame(egui::Frame::NONE)
.margin(egui::Margin::ZERO),
)
.id,
);
}
let (rule, _) = ui.allocate_exact_size(
@@ -793,6 +868,7 @@ pub(crate) fn value_card(
.frame(egui::Frame::NONE)
.margin(egui::Margin::ZERO),
);
fields.push(field.id);
if field.changed() {
super::parameter_card::field_changed(state, at);
}
@@ -950,11 +1026,63 @@ pub(crate) fn value_card(
});
});
if ui.is_enabled() {
ui.input(|i| {
committed |= i.key_pressed(egui::Key::Enter);
cancelled |= i.key_pressed(egui::Key::Escape);
});
// **ENTER AND ESCAPE ARE THIS CARD'S ONLY WHILE THIS CARD
// HAS THE KEYBOARD** (HV-01 / R2B-01). `ui.input` reads
// egui's GLOBAL event list, and `InputState::filtered_
// events` only CLONES the events a focused `TextEdit`
// consumes — the Enter is still in the list afterwards.
// So an Enter typed into the title bar's path box, the
// Variables name field, the Items new-folder field or a
// saved-view name committed whatever card the selection
// was floating: a dimension silently rewritten, a feature
// silently appended, and no readout either way.
//
// THE GUARD IS FOCUS, NOT `wants_keyboard_input`. This
// card's own fields are `TextEdit`s, and egui reports
// keyboard focus for them too, so refusing every focused
// frame would break the primary gesture — type a height,
// press Enter — and every drive script that performs it.
// What separates the two is WHOSE field holds focus, so
// the card takes the keys only when the focused id is one
// it painted itself, or when nothing is focused at all
// (the ordinary case: a picked face and a bare Enter).
//
// **AND NOT WHILE THE TITLE BAR IS ASKING A QUESTION**
// (R2B-05). `confirm_open` turns the document row into
// "discard unsaved changes and open?", which is modal in
// meaning but painted as a row rather than through the
// `Modal` that disables the shell — so `ui.is_enabled()`
// is still true and the card went on answering the Enter
// aimed at the prompt, growing the very part the question
// was about. The card keeps painting (a pending number
// must not vanish because a question was asked); it only
// stops listening.
//
// **AND THE FOCUS IT READS IS THE FRAME'S, NOT THE
// FRAME'S LEFTOVERS.** The first version of this guard
// asked `Memory::focused()` right here and was told
// `None` on exactly the frames that mattered: egui's
// single-line `TextEdit` calls `surrender_focus` the
// moment it reads an Enter, and every foreign field is
// painted before this card, so the field that took the
// key had already dropped focus by the time the card
// asked who held it. `super::focus_at_frame_start` is
// the answer taken before anything painted; the live
// read stays beside it for a surface that gains focus
// mid-frame (a multi-line editor never surrenders, and
// a `request_focus` lands the same frame). Either one
// naming a field this card did not paint refuses.
if ui.is_enabled() && state.confirm_open.is_none() {
let started = super::focus_at_frame_start(ui.ctx());
let live = ui.ctx().memory(egui::Memory::focused);
let foreign =
|id: Option<egui::Id>| id.is_some_and(|id| !fields.contains(&id));
if !foreign(started) && !foreign(live) {
ui.input(|i| {
committed_by_key |= i.key_pressed(egui::Key::Enter);
cancelled |= i.key_pressed(egui::Key::Escape);
});
}
}
});
});
@@ -1010,6 +1138,10 @@ pub(crate) fn value_card(
.request_discard("value card changed under its area id");
}
// A BUTTON ON THE CARD IS ALWAYS A COMMIT; the keystroke is one only
// while the card has not just spent it.
let committed = committed || (committed_by_key && !state.card.spent);
if preview_requested && !bodies::blocked_for_rebuild(state, action) {
let card = state.card.clone();
if super::parameter_card::prepare(state, action) {
@@ -1059,10 +1191,20 @@ pub(crate) fn value_card(
set_text(state, action, &typed);
state.card.buffers = seed_buffers(state, command, action);
response.action = session_click(action, state, &[], None);
state.card.spent |= response.action.is_some();
state.card.armed = None;
return;
}
// A ZERO-LENGTH PUSH/PULL IS NOT A FEATURE (DISC-08). The field
// opens at 0.00 — right for a gizmo drag, which starts where the
// face is — and the kernel's refusal of the resulting prism reaches
// the user as "kernel facade: invalid argument" after a feature has
// been minted and rolled back. The card knows the number before any
// of that, so it says so in a sentence naming what to do. It lives
// in `prepare`, beside the parse it belongs with, and reaches here
// as a `None` action.
response.action = session_click(action, state, &[], None);
state.card.spent |= response.action.is_some();
// THE AIM IS SPENT. Enter ran the command the press aimed at, so the
// card goes back to what the selection derives — otherwise a second
// Enter, meant for whatever is selected now, would re-run the armed
@@ -1294,3 +1436,394 @@ fn card_chip(ui: &mut egui::Ui, theme: ShellTheme, label: &str, on: &mut bool) {
text,
);
}
#[cfg(test)]
mod tests {
#![allow(clippy::unwrap_used, clippy::expect_used)]
use super::*;
use crate::testing::driven;
/// A context the card can paint into: `install_fonts` is a precondition
/// of the shell's typography, and `RawInput::default()` has no screen
/// rect at all — the card refuses to paint into the rect egui falls back
/// to, so a test on it would pass for the wrong reason.
fn card_ctx() -> egui::Context {
let ctx = egui::Context::default();
crate::theme::install_fonts(&ctx);
ctx
}
fn frame_input(events: Vec<egui::Event>) -> egui::RawInput {
egui::RawInput {
screen_rect: Some(egui::Rect::from_min_size(
egui::pos2(0.0, 0.0),
egui::vec2(1600.0, 1000.0),
)),
events,
..Default::default()
}
}
fn tab() -> egui::Event {
egui::Event::Key {
key: egui::Key::Tab,
physical_key: None,
pressed: true,
repeat: false,
modifiers: egui::Modifiers::default(),
}
}
fn enter() -> egui::Event {
egui::Event::Key {
key: egui::Key::Enter,
physical_key: None,
pressed: true,
repeat: false,
modifiers: egui::Modifiers::default(),
}
}
fn click(at: egui::Pos2) -> Vec<egui::Event> {
vec![
egui::Event::PointerMoved(at),
egui::Event::PointerButton {
pos: at,
button: egui::PointerButton::Primary,
pressed: true,
modifiers: egui::Modifiers::default(),
},
egui::Event::PointerButton {
pos: at,
button: egui::PointerButton::Primary,
pressed: false,
modifiers: egui::Modifiers::default(),
},
]
}
/// A shell with one planar face picked — the commonest selection there
/// is, and the one that floats a card with a real typed field. The
/// distance is seeded non-zero because a zero-length push/pull is
/// refused at the card (DISC-08) and would mask what these tests are
/// about.
fn picked_face() -> ShellState {
ShellState {
picked_faces: vec![7],
picked_planar: true,
distance_mm: 4.0,
..ShellState::default()
}
}
/// One frame of that card, optionally with a FOREIGN single-line field
/// painted beside it — the title bar's path box, the Variables name
/// field and the Items new-folder box all reduce to this: someone
/// else's `TextEdit`, focused, while the card stands.
///
/// `reconcile_parameter_card` runs first, exactly as `App::update` runs
/// it before painting: it is what seeds the card's buffers, and without
/// it the card paints captions with no fields at all — a frame that
/// could not show whether focus is respected.
/// What the foreign surface does this frame.
#[derive(Clone, Copy, PartialEq, Eq)]
enum Foreign {
/// Not painted at all.
None,
/// Painted, and it CLAIMS focus — the frame the user clicks into it.
Claims,
/// Painted, holding the focus it already had.
///
/// **THE FRAME THAT MATTERS.** Re-requesting focus every frame is
/// what made the first version of this test pass while the real
/// shell went on committing: `request_focus` runs after the
/// `TextEdit` has read the Enter and surrendered, so it put back
/// the very signal the keystroke destroys. Nothing in the shell
/// does that, so neither does this.
Holds,
}
fn frame(
ctx: &egui::Context,
state: &mut ShellState,
events: Vec<egui::Event>,
foreign: Foreign,
) -> Option<Action> {
let view = SceneView::default();
super::parameter_card::reconcile_parameter_card(state, &view);
let mut response = ShellResponse::default();
let mut elsewhere = String::new();
let output = ctx.run_ui(frame_input(events), |ui| {
// EXACTLY WHAT `shell::show` DOES FIRST, and for the same
// reason — see `record_focus_at_frame_start`.
super::super::record_focus_at_frame_start(ui.ctx());
if foreign != Foreign::None {
let field = ui.add(egui::TextEdit::singleline(&mut elsewhere).id_salt("foreign"));
if foreign == Foreign::Claims {
field.request_focus();
}
}
value_card(
ui,
&view,
state,
ShellTheme::default(),
Selection::Faces {
count: 1,
all_planar: true,
},
&mut response,
);
});
output.drop_without_applying_deltas();
response.action
}
/// **HV-01 / R2B-01.** The card reads Enter from egui's GLOBAL event
/// list, and a focused `TextEdit` does not remove the key from it — so
/// an Enter typed to name a file, name a variable or name a folder
/// committed whatever card the selection was floating. Here it appends
/// a whole extrude feature.
///
/// THE CONTROL IS THE POINT. Guarding on "does anything want the
/// keyboard" would also stop the card's own field, which is the primary
/// way anybody uses it, so the test drives all three states: someone
/// else's field focused (refused), nothing focused (the ordinary bare
/// Enter, taken), and a real click into the card's own field (taken).
///
/// **AND THE FOREIGN FIELD ONLY CLAIMS FOCUS ONCE.** The first version
/// of this test called `request_focus` on it every frame and passed
/// against a shell that went on committing, because `request_focus`
/// runs AFTER the `TextEdit` has read the Enter and surrendered — it
/// restored the exact signal the keystroke destroys. Driven against the
/// real shell, `Memory::focused()` reads `Some(..)` on the frames
/// carrying the clicks and the typed text and `None` on the one frame
/// carrying the Enter, which is why the guard now reads
/// `focus_at_frame_start`. `Foreign::Holds` is that frame.
#[test]
fn enter_typed_into_someone_elses_field_does_not_commit_the_card() {
let (foreign, bare, own) = driven("enter_in_a_foreign_field", || {
let ctx = card_ctx();
let mut state = picked_face();
// Two frames: an `Area` paints nothing on its first pass, so the
// card is not on screen to be committed until the second.
frame(&ctx, &mut state, Vec::new(), Foreign::Claims);
let foreign = frame(&ctx, &mut state, vec![enter()], Foreign::Holds);
let mut state = picked_face();
frame(&ctx, &mut state, Vec::new(), Foreign::None);
let bare = frame(&ctx, &mut state, vec![enter()], Foreign::None);
let mut state = picked_face();
frame(&ctx, &mut state, Vec::new(), Foreign::None);
// INTO THE CARD'S OWN FIELD. Tab rather than a click on it:
// egui's own focus navigation puts the caret in the first
// focusable widget of the only surface painted here, which is
// the card's number field, and it does so without the test
// having to know where the card laid that field out this frame.
// That the Enter below is then TAKEN is the proof it is the
// card's own id: the guard admits exactly those and `None`.
frame(&ctx, &mut state, vec![tab()], Foreign::None);
let focused = ctx.memory(egui::Memory::focused);
let own = frame(&ctx, &mut state, vec![enter()], Foreign::None);
(foreign, bare, (focused, own))
});
assert_eq!(
foreign, None,
"an Enter typed into another surface's field committed the card"
);
assert_eq!(
bare,
Some(Action::PushPull),
"a bare Enter with nothing focused must still run the card"
);
assert!(
own.0.is_some(),
"Tab did not put the caret in the card's own field"
);
assert_eq!(
own.1,
Some(Action::PushPull),
"the card stopped committing on its own field's Enter"
);
}
/// **R2B-05.** `confirm_open` turns the title bar into "discard unsaved
/// changes and open?", and it is painted as a row rather than through
/// the `Modal` that disables the shell — so `ui.is_enabled()` is still
/// true and the Enter aimed at the question grew the part behind it.
#[test]
fn the_card_is_deaf_while_the_discard_prompt_stands() {
let (asked, answered) = driven("card_vs_discard_prompt", || {
let ctx = card_ctx();
let mut state = ShellState {
confirm_open: Some(Action::OpenDocument),
..picked_face()
};
frame(&ctx, &mut state, Vec::new(), Foreign::None);
let asked = frame(&ctx, &mut state, vec![enter()], Foreign::None);
state.confirm_open = None;
frame(&ctx, &mut state, Vec::new(), Foreign::None);
let answered = frame(&ctx, &mut state, vec![enter()], Foreign::None);
(asked, answered)
});
assert_eq!(
asked, None,
"the card answered an Enter meant for the discard prompt"
);
assert_eq!(
answered,
Some(Action::PushPull),
"the card stayed deaf after the prompt was answered"
);
}
/// **PV-07.** A commit spends the aim but not the SELECTION, so the card
/// immediately re-derives from the picks that are still standing and a
/// reflexive second Enter dispatched whatever that was — a second
/// identical hole, a draft after a chamfer, a zero push/pull after a
/// shell. Here the same profile would simply be extruded twice.
///
/// AND THE WAY BACK IS A DELIBERATE TOUCH: a pointer press anywhere
/// re-arms the keystroke, so "pick, Enter, pick, Enter" — the rhythm of
/// actually using the app — is untouched.
#[test]
fn a_second_bare_enter_after_a_commit_dispatches_nothing() {
let (first, second, after_touch) = driven("second_bare_enter", || {
let ctx = card_ctx();
let mut state = picked_face();
frame(&ctx, &mut state, Vec::new(), Foreign::None);
let first = frame(&ctx, &mut state, vec![enter()], Foreign::None);
let second = frame(&ctx, &mut state, vec![enter()], Foreign::None);
// A click well clear of the card: the point is that the user has
// done something, not what they clicked.
frame(
&ctx,
&mut state,
click(egui::pos2(40.0, 900.0)),
Foreign::None,
);
let after_touch = frame(&ctx, &mut state, vec![enter()], Foreign::None);
(first, second, after_touch)
});
assert_eq!(first, Some(Action::PushPull), "the first Enter must commit");
assert_eq!(
second, None,
"a bare second Enter ran the card's command again"
);
assert_eq!(
after_touch,
Some(Action::PushPull),
"a deliberate touch did not re-arm the keystroke"
);
}
/// **PV-07, THE HALF THE FIRST FIX MISSED.** `spent` was set only inside
/// this card's own commit branches, so a command dispatched by a RIBBON
/// press while the card stood left the keystroke armed — and the picks
/// that fed that press are still standing, so the card immediately
/// offers the next command they derive. Driven: a ribbon `hole` press on
/// two picked points took the feature count 3 → 4, and a bare Enter took
/// it 4 → 5.
#[test]
fn a_dispatch_the_card_did_not_make_also_spends_the_enter() {
let (after_foreign_dispatch, after_touch) = driven("foreign_dispatch", || {
let ctx = card_ctx();
let mut state = picked_face();
frame(&ctx, &mut state, Vec::new(), Foreign::None);
// A press that RAN something: the ribbon has already written its
// action into the shared response by the time the card paints.
let view = SceneView::default();
super::parameter_card::reconcile_parameter_card(&mut state, &view);
let mut response = ShellResponse {
action: Some(Action::MakeHole),
..ShellResponse::default()
};
let output = ctx.run_ui(frame_input(click(egui::pos2(40.0, 900.0))), |ui| {
super::super::record_focus_at_frame_start(ui.ctx());
value_card(
ui,
&view,
&mut state,
ShellTheme::default(),
Selection::Faces {
count: 1,
all_planar: true,
},
&mut response,
);
});
output.drop_without_applying_deltas();
let after_foreign_dispatch = frame(&ctx, &mut state, vec![enter()], Foreign::None);
frame(
&ctx,
&mut state,
click(egui::pos2(40.0, 900.0)),
Foreign::None,
);
let after_touch = frame(&ctx, &mut state, vec![enter()], Foreign::None);
(after_foreign_dispatch, after_touch)
});
assert_eq!(
after_foreign_dispatch, None,
"a bare Enter ran a command after something else had just dispatched one"
);
assert_eq!(
after_touch,
Some(Action::PushPull),
"a deliberate touch did not re-arm the keystroke"
);
}
/// **PV-07, AND THE REASON `disarm` NO LONGER CLEARS THE LEDGER.** A
/// commit recompiles; for the frames the worker takes to answer, the
/// selection derives no command and `value_card` disarms. Clearing the
/// flag there forgot the dispatch across exactly the gap the second
/// Enter falls into (driven: a sketch fillet committed, then a bare
/// Enter appended a hole, 3 → 4).
#[test]
fn the_spent_keystroke_survives_a_frame_with_no_card() {
let (second, after_touch) = driven("spent_survives_no_card", || {
let ctx = card_ctx();
let mut state = picked_face();
frame(&ctx, &mut state, Vec::new(), Foreign::None);
assert_eq!(
frame(&ctx, &mut state, vec![enter()], Foreign::None),
Some(Action::PushPull)
);
// THE GAP: nothing is picked, so no card is derivable at all.
let view = SceneView::default();
let mut response = ShellResponse::default();
let output = ctx.run_ui(frame_input(Vec::new()), |ui| {
super::super::record_focus_at_frame_start(ui.ctx());
value_card(
ui,
&view,
&mut state,
ShellTheme::default(),
Selection::None,
&mut response,
);
});
output.drop_without_applying_deltas();
let second = frame(&ctx, &mut state, vec![enter()], Foreign::None);
frame(
&ctx,
&mut state,
click(egui::pos2(40.0, 900.0)),
Foreign::None,
);
let after_touch = frame(&ctx, &mut state, vec![enter()], Foreign::None);
(second, after_touch)
});
assert_eq!(
second, None,
"the card forgot it had just dispatched because it went away for a frame"
);
assert_eq!(
after_touch,
Some(Action::PushPull),
"a deliberate touch did not re-arm the keystroke"
);
}
}
+103
View File
@@ -0,0 +1,103 @@
# Batch 1 of the GUI audit, fixed — 2026-09-11
The first batch of `docs/GUI_AUDIT_2026-09-10.md`'s recommended fix order, landed against
main `688279d`. Eleven findings, chosen because they are the five P1s plus the P2s that share
their causes, and because each is local enough to gate on its own.
**Every fix is proved by driving the real shell, not by a unit test alone.** That distinction
is the day's lesson and is recorded here because it cost a whole round: three of these fixes
passed hand-written egui tests and did nothing at all in the app. The unit tests were asking
the wrong question, and only `vernier-drive` said so.
## What was fixed
| Finding | Severity | Where | Driven proof |
|---|---|---|---|
| HV-01 / R2B-01 | P1 | `shell/mod.rs`, `shell/value_card.rs` | `s9b-profile-editor-vs-path-field` saves a document with no extrude (it gained one before); `b3-variables-tail` step 44 flips — the binding survives |
| R2A-02 | P1 | `shell/parameter_card.rs` | `s3-typed-units`: `c-expr.vernier` 1.0 mm → 25.4 mm, stored as `(0.5+0.5) in` |
| STC-02 | P1 | `server/apply_features.rs` | `q04-draw-after-delete`: `try1.vernier` has 1 curve where it had 0 — drawing works after a delete |
| MB-01 | P1 | `occt-sys/src/facade.cpp` | `mb-g9-sole-body-join-disjoint-export` writes `one-compound-body.step`, 49,717 bytes, where the export was refused |
| PV-07 | P2 | `shell/value_card.rs` | `p24-hole-double-enter` stays at 4 features across the bare Enter (was 4 → 5); `p25` stays at 3 |
| SF-07 (with FC-05, SB-10) | P2 | `shell/value_card.rs`, `parameter_card.rs` | `sf-extrude-zero-feature`: readout "height must be at least 0.001 mm", and the part is 18000 mm³ rather than 1.2 |
| DISC-08 | P2 | `shell/value_card.rs` | `p16-second-enter`: readout "Type a distance to push or pull this face; 0 moves nothing" instead of a raw façade string |
| SF-01 | P2 | `app.rs` | `sf-sticky-cut-session-escape` step 38 passes — two solids, not one consumed by a sticky cut |
| SF-10 | P2 | `app.rs` | `sf3-heavy-pattern-undo`: readout "Still building — wait for the current operation to finish before undoing", and the pattern survives |
| R2B-05 | P2 | `shell/value_card.rs` | `scripts/drive/card-deaf-during-discard-prompt.json` |
| FPR-06 | P2 | `ui/registry.rs`, `server/export_publication.rs` | `scripts/drive/export-refuses-a-foreign-document.json` |
## The three root causes worth remembering
**The value card could not see who owned the keyboard.** egui's single-line `TextEdit`
surrenders focus the instant it reads an Enter (`text_edit/builder.rs:1170`), and the card is
painted last so it floats — so by the time the card asked who was focused, the field that had
just eaten the keystroke had already let go, and the card was told "nobody". The card then
committed. The first attempt at this fix read `Memory::focused()` at the card's own paint time
and therefore changed nothing; its unit test passed only because the test's foreign field
called `request_focus()` every frame, which nothing in the shell does. The fix records focus
once at the top of `shell::show`, before any widget paints, and the card takes Enter only when
neither that snapshot nor the live answer names an id the card did not paint itself.
**A typed expression never carried its unit into the document.** A field whose text is not a
bare number is stored as an expression and re-evaluated by the server on every later compile
at a bare scale of 1.0 — millimetres, the document's only unit. So the card computed 25.4 mm
for `0.5+0.5` in inch mode and the document computed 1.0 mm from the same string, for ever.
Storing the value would have broken variable bindings; the fix stores text that *says what the
field meant* (`(0.5+0.5) in`) and verifies the rewrite by evaluating it, rather than trusting
it. An expression that mixes an implicit number into an explicit one is now refused by name
instead of silently becoming a number nobody typed.
**OCCT writes one assembly row per edge of the assembly tree.** MB-01's guard compared the
rewritten `NEXT_ASSEMBLY_USAGE_OCCURRENCE` rows against the body count, and the audit proposed
comparing against the leaf-solid count instead. Both are wrong: measured on OCCT 7.9.3,
`compound{box}` writes 0 rows (a one-child compound is not an assembly at all),
`compound{box,box}` writes 2, `compound{compound{s,s}}` writes 3 and
`compound{compound{s,s},box}` writes 4. A second defect sat in front of it and fires first:
`normalize_step_product` accepted a positional product name one level deep (`VernierCAD 2.1`)
and refused two (`VernierCAD 2.1.1`), which is exactly what a sub-assembly writes. Both are
fixed, the guard still fails closed, and determinism is unchanged — two exports of one shape
are still byte-identical.
## What this batch did NOT fix, deliberately
- **SF-11** — nothing on screen says the worker is busy. SF-10 now refuses undo during a
compile with a readout, but a user who presses nothing still sees an idle-looking frame. A
real indicator has to be painted every frame from the status strip.
- **SF-10's other half** — the ribbon and history-strip Undo buttons reach the worker through
`edit.rs` without passing the keyboard guard, so the button can still queue an undo behind a
compile.
- **R2A-02 in the body-tool fields** — `body_tools.rs` stores its typed numbers as expressions
through the same server path, so the same unit divergence is reachable there.
- **MB-01's sibling** — a Join whose tool is clear of its target still silently builds a
two-solid compound; only the export is fixed. The compound warning is still invisible (MB-07).
- **FPR-06's Save arm** — a typed *save* path can still replace another document. Only the
export half is refused.
## Pre-existing, measured, and not from this batch
- `process_worker::lifecycle_tests::bootstrap_is_compile_free_and_stop_reaps_a_forever_request_before_restore`
fails about 4 times in 6 at `688279d` and about 2 in 6 with these fixes (A/B measured by
reverting the four changed `vernier-app` files). The ten lifecycle tests race on something
shared; alone the test always passes.
- `scripts/drive/planar-lettering.json` exits 1 at step 23 — recorded as `rc=1` at `688279d`
in the audit's own driver inventory before any of this work.
- `b3-variables-tail` step 109 (a Variables binding committed on lost focus) is a separate
open defect in `shell/variables.rs`.
- `crates/vernier-drive/tests/l1.rs` races with itself and **currently stops `check.fish`**:
`the_l1_script_passes` and `the_l1_negative_control_fails_naming_the_step` both open
`scripts/drive/fixtures/l1-clamp.vernier`, and the open path takes a lock on the fixture's
directory (`server/native_files.rs:214`, a file this batch does not touch). Measured today:
the binary fails 1 run in 3 at default parallelism and passes 3 of 3 under
`--test-threads=1`, and two concurrent `vernier-drive` processes opening that one fixture
reproduce it on demand — one exits 2 with "operation would block". Until the two tests stop
sharing the fixture, `cargo test --workspace` is a coin toss on this one assertion. The rest
of the gate was verified around it (see below).
## Gate
`cargo fmt --check` clean · `cargo clippy --all-targets -- -D warnings` exit 0 ·
`cargo test --workspace` 1729 passed, 1 failed on the l1 race above (green when that binary is
serialized) · `vernier-cli --selftest` 45/45 · all 86 committed drive scripts exit 0 except the
four negative controls (correct, they must fail) and `planar-lettering` (pre-existing, `rc=1`
at `688279d`) · five new gate scripts registered in `scripts/check.fish`, each mutation-checked
against the pre-fix binary. `check.fish` itself stops at the l1 race rather than at anything in
this batch.
+15
View File
@@ -343,5 +343,20 @@ and cargo run -q -p vernier-drive -- scripts/drive/sketch-text.json \
# Live sketch geometry, snap label, deliberate extrusion, and exported volume.
and cargo run -q -p vernier-drive -- scripts/drive/ux-sketch-feedback.json \
--out target/drive/ux-sketch-feedback --require-adapter RADV
# GUI_AUDIT_2026-09-10 regression gates, one per fixed finding. HV-01/R2B-01,
# PV-07 and R2A-02 promote verified scripts from the audit repair; R2B-05 and
# FPR-06 are new, written against the fixed build.
and cargo run -q -p vernier-drive -- scripts/drive/card-enter-ownership.json \
--out target/drive/card-enter-ownership --require-adapter RADV
and cargo run -q -p vernier-drive -- scripts/drive/typed-units-reach-the-document.json \
--out target/drive/typed-units-reach-the-document --require-adapter RADV
and cargo run -q -p vernier-drive -- scripts/drive/bare-enter-after-a-dispatch.json \
--out target/drive/bare-enter-after-a-dispatch --require-adapter RADV
and cargo run -q -p vernier-drive -- scripts/drive/card-deaf-during-discard-prompt.json \
--out target/drive/card-deaf-during-discard-prompt --require-adapter RADV
and cargo run -q -p vernier-drive -- scripts/drive/export-refuses-a-foreign-document.json \
--out target/drive/export-refuses-a-foreign-document --require-adapter RADV
and cargo test -q -p vernier-app ui_half_coverage -- --ignored
and cargo test -q -p vernier-app a_plain_pointer_move -- --ignored
@@ -0,0 +1,219 @@
{
"name": "bare-enter-after-a-dispatch",
"size": [
1600,
1000
],
"camera": {
"target": [
20.0,
15.0,
7.5
],
"distance": 120.0
},
"notes": [
"FINDING PV-07. THE DEFECT: a commit spends the AIM but not the SELECTION,",
"so once a command dispatches, the card immediately re-derives the SAME",
"command from whatever is still picked and stands ready to fire again --",
"and a reflexive second Enter (the rhythm of confirming, muscle memory from",
"every dialog that asks 'are you sure') re-dispatched it: a second identical",
"hole, a draft after a chamfer, a zero push/pull after a shell. The fix",
"spends the keystroke itself on a successful dispatch, not just the pick.",
"",
"ASSERTED POSITIVELY. Two locked sketch points; a ribbon `hole` press drills",
"at defaults, 3 -> 4 features.",
"",
"WHAT THIS CATCHES IF THE FIX REGRESSES: the picks are still standing, so",
"the card immediately re-derives `hole` from them and paints",
"`hole - diameter`. A reflexive second Enter, meant as 'yes, confirm', must",
"NOT drill a second hole: the feature count stays 4. If the old bug",
"returns, this second Enter drills again and `expect_feature_count` sees 5",
"one step early, before the deliberate re-arm below.",
"",
"THE CONTROL is the next two steps: a click into the card's own field is a",
"deliberate touch, and the Enter after it DOES drill, 4 -> 5. So the",
"keystroke is spent, not the card -- a fix that simply ignored every Enter",
"after any dispatch (rather than only the reflexive, un-re-armed one) would",
"fail here instead."
],
"steps": [
{
"step": "wait_idle"
},
{
"step": "expect_no_error"
},
{
"step": "click",
"at": {
"by": "face",
"face": {
"by": "named",
"feature": "block",
"key": {
"by": "role",
"role": "prism-end"
}
}
}
},
{
"step": "wait_idle"
},
{
"step": "click",
"at": "ribbon:sketch"
},
{
"step": "wait_idle"
},
{
"step": "expect_feature_count",
"is": 3
},
{
"step": "click",
"at": "text:Point"
},
{
"step": "click",
"at": "world:12,15,15"
},
{
"step": "wait_idle"
},
{
"step": "click",
"at": "world:28,15,15"
},
{
"step": "wait_idle"
},
{
"step": "click",
"at": "text:Select"
},
{
"step": "click",
"at": "world:12,15,15"
},
{
"step": "frames",
"count": 2
},
{
"step": "click",
"at": "text:\u25a3"
},
{
"step": "frames",
"count": 2
},
{
"step": "key",
"key": "Enter"
},
{
"step": "wait_idle"
},
{
"step": "click",
"at": "world:28,15,15"
},
{
"step": "frames",
"count": 2
},
{
"step": "click",
"at": "text:\u25a3"
},
{
"step": "frames",
"count": 2
},
{
"step": "key",
"key": "Enter"
},
{
"step": "wait_idle"
},
{
"step": "click",
"at": "world:12,15,15"
},
{
"step": "click",
"at": "world:28,15,15",
"ctrl": true
},
{
"step": "frames",
"count": 3
},
{
"step": "expect_selection",
"is": "two-points"
},
{
"step": "click",
"at": "ribbon:hole"
},
{
"step": "wait_idle"
},
{
"step": "expect_no_error"
},
{
"step": "expect_feature_count",
"is": 4
},
{
"step": "key",
"key": "Enter"
},
{
"step": "wait_idle"
},
{
"step": "frames",
"count": 2
},
{
"step": "expect_no_error"
},
{
"step": "expect_feature_count",
"is": 4
},
{
"step": "expect_selection",
"is": "two-points"
},
{
"step": "click",
"at": "card:diameter"
},
{
"step": "frames",
"count": 2
},
{
"step": "key",
"key": "Enter"
},
{
"step": "wait_idle"
},
{
"step": "expect_no_error"
},
{
"step": "expect_feature_count",
"is": 5
}
]
}
@@ -0,0 +1,119 @@
{
"name": "card-deaf-during-discard-prompt",
"notes": [
"FINDING R2B-05. `confirm_open` turns the title bar's document row into",
"\"discard unsaved changes and open?\", painted as a ROW rather than through",
"a `Modal` that disables the shell -- so `ui.is_enabled()` stayed true and",
"an Enter aimed at the question reached the value card standing behind it",
"instead: the card treats an Enter with nothing else focused as its OWN",
"bare Enter (that is deliberate, real-app behaviour -- see",
"card-enter-ownership.json's control), and closing the command palette to",
"raise the prompt leaves nothing focused, so the card fired and grew the",
"part behind a question the user had not yet answered. The fix makes the",
"card check `confirm_open.is_none()` before it will answer a bare Enter.",
"",
"THE ROUTE (matching the audit's own s10-close-prompt-vs-card.json):",
"save the starter, commit a real push/pull so there is a real undo entry,",
"then leave a SECOND, uncommitted edit sitting in a value card, and use",
"the `$` bar's `open` on the document's OWN path (still dirty from the",
"committed push/pull) to raise the discard prompt over that pending card.",
"",
"ASSERTED POSITIVELY, via exported geometry rather than a UI flag, so a",
"fix that sets the right internal state but still lets the click through",
"cannot pass by accident: block 40x30x15, +5mm push/pull on the top face",
"committed (-> 40x30x20 = 24000 mm^3), then `61` typed into the BASE",
"extrude's height card and left uncommitted when the prompt is raised.",
"",
"WHAT THIS CATCHES IF THE FIX REGRESSES: with the prompt standing, an",
"Enter must NOT commit the pending `61` -- the height must still be its",
"original 15 mm and the exported volume must still be 24000 (`before.step`",
"and `after-prompt-enter.step` must agree exactly). If the old bug",
"returns, that Enter commits height=61 and, since the push/pull is a",
"RELATIVE offset off the resolved face, the volume jumps to",
"40*30*(61+5) = 79200 one export early -- a value `after-prompt-enter.step`",
"would then show instead of `after-control-enter.step`.",
"",
"THE CONTROL: dismissing the prompt (clicking its real `cancel` button,",
"changing nothing) and pressing the SAME Enter again must now go on to",
"commit the still-pending `61` exactly as it would have with no prompt in",
"the way -- 40*30*(61+5) = 79200. A fix that deafened the card forever",
"(rather than only while `confirm_open` is armed) would fail here instead,",
"stuck at 24000.",
"",
"Mutation-verified against the pre-fix build kept at",
"/tmp/prefix-binaries/vernier-drive: that binary commits the pending `61`",
"on the FIRST Enter (the one aimed at the prompt), so this script's",
"`after-prompt-enter.step` line fails there with 79200 where 24000 is",
"expected -- exactly the defect this gate exists to catch."
],
"size": [1600, 1000],
"camera": { "target": [20.0, 15.0, 7.5], "distance": 140.0 },
"steps": [
{ "step": "wait_idle" },
{ "step": "expect_no_error" },
{ "step": "click", "at": "text:untitled" },
{ "step": "type_path", "path": "{out}/part.vernier" },
{ "step": "click", "at": "text:save" },
{ "step": "wait_idle" },
{ "step": "expect_no_error" },
{ "step": "click", "at": { "by": "face", "face": { "by": "named", "feature": "block",
"key": { "by": "role", "role": "prism-end" } } } },
{ "step": "wait_idle" },
{ "step": "click", "at": "card:distance" },
{ "step": "key", "key": "ctrl+a" },
{ "step": "type", "text": "5.0" },
{ "step": "key", "key": "Enter" },
{ "step": "wait_idle" },
{ "step": "expect_no_error" },
{ "step": "expect_feature_count", "is": 3 },
{ "step": "export_step", "path": "{out}/before.step" },
{ "step": "wait_idle" },
{ "step": "expect_step", "path": "{out}/before.step",
"volume": 24000.0, "solids": 1, "tol": 1e-9 },
{ "step": "click", "at": "timeline:1" },
{ "step": "frames", "count": 3 },
{ "step": "expect_selection", "is": "feature:extrude" },
{ "step": "click", "at": "card:height" },
{ "step": "key", "key": "ctrl+a" },
{ "step": "type", "text": "61" },
{ "step": "frames", "count": 2 },
{ "step": "screenshot", "path": "{out}/a0-card-61-pending.png" },
{ "step": "click", "at": "text:$" },
{ "step": "frames", "count": 2 },
{ "step": "type", "text": "open" },
{ "step": "click", "at": "text:open / export path" },
{ "step": "key", "key": "ctrl+a" },
{ "step": "type_path", "path": "{out}/part.vernier" },
{ "step": "key", "key": "Enter" },
{ "step": "frames", "count": 3 },
{ "step": "screenshot", "path": "{out}/a1-prompt-standing.png" },
{ "step": "key", "key": "Enter" },
{ "step": "wait_idle" },
{ "step": "expect_no_error" },
{ "step": "export_step", "path": "{out}/after-prompt-enter.step" },
{ "step": "wait_idle" },
{ "step": "expect_step", "path": "{out}/after-prompt-enter.step",
"volume": 24000.0, "solids": 1, "tol": 1e-9 },
{ "step": "expect_document", "path": "{out}/part.vernier",
"pointer": "/feature:1/payload/Extrude/height", "equals": 15.0, "tol": 1e-9 },
{ "step": "click", "at": "text:cancel" },
{ "step": "frames", "count": 2 },
{ "step": "screenshot", "path": "{out}/a2-prompt-dismissed.png" },
{ "step": "key", "key": "Enter" },
{ "step": "wait_idle" },
{ "step": "expect_no_error" },
{ "step": "expect_feature_count", "is": 3 },
{ "step": "export_step", "path": "{out}/after-control-enter.step" },
{ "step": "wait_idle" },
{ "step": "expect_step", "path": "{out}/after-control-enter.step",
"volume": 79200.0, "solids": 1, "tol": 1e-9 },
{ "step": "expect_no_warnings" }
]
}
+89
View File
@@ -0,0 +1,89 @@
{
"name": "card-enter-ownership",
"size": [1600, 1000],
"camera": { "target": [20.0, 15.0, 7.5], "distance": 130.0 },
"notes": [
"FINDING HV-01 / R2B-01. THE DEFECT: an Enter typed into another surface's",
"text field (here, the Variables panel's new-binding NAME field) reached the",
"value card underneath as a 'bare' Enter and committed WHATEVER THAT CARD",
"HAD PENDING -- because egui's single-line TextEdit surrenders focus the",
"instant it reads the Enter, so `Memory::focused()` is already None by the",
"time the card (painted after the panel) asks who owns the keyboard, and a",
"guard reading only 'is nothing focused' cannot tell that apart from the",
"keystroke never having been typed anywhere at all. The fix reads focus at",
"the START of the frame the Enter arrives in, not after the foreign field",
"has already let go of it.",
"",
"ASSERTED POSITIVELY. Starter block 40x30x15; a variable `height` = 15mm",
"binds the extrude to height*2 -> 36000.",
"",
"WHAT THIS CATCHES IF THE FIX REGRESSES: with 25 typed into card:height and",
"NOT committed, an Enter pressed inside the Variables NAME field must change",
"nothing -- the binding stands and the solid is still 36000. If the old bug",
"returns, that Enter reaches the card again and the export volume moves to",
"40*30*25 = 30000 -- a value this script would catch on the",
"`after-foreign-enter.step` line.",
"",
"THE CONTROL is the same pending 25: clicking back into the card's own",
"field and pressing Enter DOES commit it, 40*30*25 = 30000. A guard that",
"simply deafened the card forever (rather than only while a foreign field",
"holds focus) would fail here instead."
],
"steps": [
{ "step": "wait_idle" },
{ "step": "expect_no_error" },
{ "step": "expect_feature_count", "is": 2 },
{ "step": "click", "at": "text:$" },
{ "step": "type", "text": "variables" },
{ "step": "key", "key": "Enter" },
{ "step": "frames", "count": 3 },
{ "step": "click", "at": "body-tool:variable name" },
{ "step": "type", "text": "height" },
{ "step": "click", "at": "body-tool:variable expression" },
{ "step": "type", "text": "15mm" },
{ "step": "click", "at": "body-tool:variable Apply" },
{ "step": "wait_idle" },
{ "step": "expect_no_error" },
{ "step": "click", "at": "timeline:1" },
{ "step": "frames", "count": 2 },
{ "step": "expect_selection", "is": "feature:extrude" },
{ "step": "click", "at": "card:height" },
{ "step": "key", "key": "ctrl+a" },
{ "step": "type", "text": "height*2" },
{ "step": "key", "key": "Enter" },
{ "step": "wait_idle" },
{ "step": "expect_no_error" },
{ "step": "export_step", "path": "{out}/bound.step" },
{ "step": "wait_idle" },
{ "step": "expect_step", "path": "{out}/bound.step", "volume": 36000, "solids": 1, "tol": 1e-9 },
{ "step": "click", "at": "card:height" },
{ "step": "key", "key": "ctrl+a" },
{ "step": "type", "text": "25" },
{ "step": "frames", "count": 2 },
{ "step": "click", "at": "body-tool:variable new" },
{ "step": "click", "at": "body-tool:variable name" },
{ "step": "type", "text": "zz" },
{ "step": "frames", "count": 2 },
{ "step": "key", "key": "Enter" },
{ "step": "wait_idle" },
{ "step": "frames", "count": 2 },
{ "step": "export_step", "path": "{out}/after-foreign-enter.step" },
{ "step": "wait_idle" },
{ "step": "save", "path": "{out}/after-foreign-enter.vernier" },
{ "step": "wait_idle" },
{ "step": "expect_step", "path": "{out}/after-foreign-enter.step", "volume": 36000, "solids": 1, "tol": 1e-9 },
{ "step": "expect_document", "path": "{out}/after-foreign-enter.vernier", "pointer": "/state/bindings/0/expression", "equals": "height*2" },
{ "step": "click", "at": "card:height" },
{ "step": "frames", "count": 2 },
{ "step": "key", "key": "Enter" },
{ "step": "wait_idle" },
{ "step": "expect_no_error" },
{ "step": "export_step", "path": "{out}/after-own-enter.step" },
{ "step": "wait_idle" },
{ "step": "expect_step", "path": "{out}/after-own-enter.step", "volume": 30000, "solids": 1, "tol": 1e-9 }
]
}
@@ -0,0 +1,142 @@
{
"name": "export-refuses-a-foreign-document",
"notes": [
"FINDING FPR-06. THE DEFECT: the only export guard was IDENTITY-based",
"(`export_conflicts_with_native`, comparing a typed destination against",
"THIS process's own open document's path), which can only ever speak for",
"the one document this session has open. Any OTHER project's `.vernier`",
"file -- or its `.names` sidecar, `.bak`, or transaction temporaries -- was",
"a perfectly ordinary-looking write target: typing a stranger's document",
"path into the export destination field and pressing Enter replaced it",
"with raw STL/STEP bytes, and the next `open` of that project failed with",
"\"cannot decode\". The fix (`export_path_is_native_storage`) refuses by the",
"destination's SHAPE alone -- a `.vernier`-family leaf -- whatever document",
"is open, before the write is ever attempted.",
"",
"THE ROUTE: save the starter as `victim.vernier` (this is \"the other",
"document\" -- created safely inside {out} rather than risking a committed",
"fixture). Then, WITHOUT a Save As command in the vocabulary, Save As is",
"reached the only way the real shell offers it: the title bar's path field",
"is a plain TextEdit and Ctrl+S/Ctrl+A are exempted from the normal",
"focus-consumption rule (see camera.rs's `route_key`, \"Global Save/Quit",
"work even while a text field has keyboard focus\"), so a second",
"select-all-and-retype-and-Ctrl+S re-saves to a NEW path without ever",
"needing to click a field whose text is now an unpredictable run-local",
"path. (Measured: the async save briefly DISABLES the whole shell -- the",
"status bar reads \"Waiting for the current document operation to finish\"",
"-- and a focused widget does not survive that disabled window on its own,",
"so the retype happens BEFORE the first save's `wait_idle`, while the field",
"is still the one thing holding focus.) This opens a SECOND, distinct",
"document -- `mine.vernier` -- as the actually-open one, so the identity",
"guard alone would see `mine.vernier`, not `victim.vernier`, as \"this",
"document\", and would find no conflict with a `victim.vernier` target.",
"",
"WHAT THIS CATCHES IF THE FIX REGRESSES: with `mine.vernier` open, typing",
"`victim.vernier` as an export destination must show the refusal reason",
"'a .vernier name is a document, not an export destination' painted right",
"in the command palette row -- BEFORE Enter is even pressed, since the",
"readiness gate that produces this text is the same one the ribbon and the",
"palette both read (`registry::readiness_for_context`), so a live UI",
"reason is the only observable this particular gesture ever produces: the",
"row simply never dispatches, and neither `expect_error` nor the status",
"readout has anything to see (this is why the assertion is a `move` onto",
"the exact painted sentence rather than `expect_readout` on its own --",
"`expect_readout` here only pins that the readout STAYS whatever it was",
"before the attempt, which is the negative half of the same claim). If the",
"identity-only guard were all that remained, this sentence would not paint",
"at all (there is no conflict with `mine.vernier`) and the `move` step",
"would fail by itself, before Enter is ever sent -- which is exactly what",
"happened against the pre-fix build below. The SAME reason must also show",
"for the `.names` sidecar shape.",
"",
"AND THE VICTIM MUST BE UNTOUCHED: `victim.vernier` must still parse as",
"the same document -- version 40, the base extrude's height still 15 mm --",
"proving the write never reached the filesystem.",
"",
"THE CONTROL: the very same export, retargeted at an ordinary `.stl` name",
"in the SAME directory, must succeed and produce the starter block's real",
"18000 mm^3 -- proving the refusal is about the destination's SHAPE, not a",
"general permission failure or a broken export pipeline.",
"",
"Mutation-verified two ways against the pre-fix build kept at",
"/tmp/prefix-binaries/vernier-drive (it predates every one of today's",
"fixes, HV-01/PV-07/R2A-02/R2B-05 included, so it is the cleanest possible",
"control for this one): (1) this exact script fails at the `move` step --",
"the refusal sentence paints ZERO times, because `export_path_is_native_",
"storage` does not exist yet and the row reads as perfectly live; (2) with",
"the `move` assertions stripped, the same script goes on to actually",
"dispatch the export -- the readout reads \"wrote 684 bytes of STL to",
"...victim.vernier\", and the file that follows is no longer JSON at all",
"(`file`(1) calls it plain \"data\") -- the exact corruption this finding",
"describes, reproduced end to end."
],
"size": [1600, 1000],
"camera": { "target": [20.0, 15.0, 7.5], "distance": 120.0 },
"steps": [
{ "step": "wait_idle" },
{ "step": "expect_no_error" },
{ "step": "click", "at": "text:untitled" },
{ "step": "type_path", "path": "{out}/victim.vernier" },
{ "step": "key", "key": "ctrl+s" },
{ "step": "wait_idle" },
{ "step": "expect_no_error" },
{ "step": "expect_readout", "contains": "victim.vernier" },
{ "step": "expect_document", "path": "{out}/victim.vernier",
"pointer": "/version", "equals": 40 },
{ "step": "expect_document", "path": "{out}/victim.vernier",
"pointer": "/feature:1/payload/Extrude/height", "equals": 15.0, "tol": 1e-9 },
{ "step": "key", "key": "Escape" },
{ "step": "key", "key": "Tab" },
{ "step": "key", "key": "ctrl+a" },
{ "step": "type_path", "path": "{out}/mine.vernier" },
{ "step": "key", "key": "ctrl+s" },
{ "step": "wait_idle" },
{ "step": "expect_no_error" },
{ "step": "expect_readout", "contains": "mine.vernier" },
{ "step": "expect_document", "path": "{out}/mine.vernier",
"pointer": "/version", "equals": 40 },
{ "step": "click", "at": "text:$" },
{ "step": "frames", "count": 2 },
{ "step": "type", "text": "export stl" },
{ "step": "click", "at": "text:open / export path" },
{ "step": "key", "key": "ctrl+a" },
{ "step": "type_path", "path": "{out}/victim.vernier" },
{ "step": "frames", "count": 2 },
{ "step": "screenshot", "path": "{out}/1-vernier-refused.png" },
{ "step": "move", "at": "text:a .vernier name is a document, not an export destination" },
{ "step": "key", "key": "Enter" },
{ "step": "wait_idle" },
{ "step": "expect_no_error" },
{ "step": "expect_readout", "contains": "mine.vernier" },
{ "step": "expect_document", "path": "{out}/victim.vernier",
"pointer": "/version", "equals": 40 },
{ "step": "expect_document", "path": "{out}/victim.vernier",
"pointer": "/feature:1/payload/Extrude/height", "equals": 15.0, "tol": 1e-9 },
{ "step": "key", "key": "ctrl+a" },
{ "step": "type_path", "path": "{out}/victim.vernier.names" },
{ "step": "frames", "count": 2 },
{ "step": "screenshot", "path": "{out}/2-names-sidecar-refused.png" },
{ "step": "move", "at": "text:a .vernier name is a document, not an export destination" },
{ "step": "key", "key": "Escape" },
{ "step": "frames", "count": 2 },
{ "step": "click", "at": "text:$" },
{ "step": "frames", "count": 2 },
{ "step": "type", "text": "export stl" },
{ "step": "click", "at": "text:open / export path" },
{ "step": "key", "key": "ctrl+a" },
{ "step": "type_path", "path": "{out}/ordinary.stl" },
{ "step": "key", "key": "Enter" },
{ "step": "wait_idle" },
{ "step": "expect_no_error" },
{ "step": "expect_readout", "contains": "bytes of STL" },
{ "step": "expect_stl", "path": "{out}/ordinary.stl",
"volume": 18000.0, "tol": 0.02, "min_triangles": 12 },
{ "step": "expect_no_warnings" }
]
}
@@ -0,0 +1,90 @@
{
"name": "typed-units-reach-the-document",
"size": [1600, 1000],
"camera": { "target": [20.0, 15.0, 7.5], "distance": 140.0 },
"notes": [
"FINDING R2A-02. THE DEFECT: under a non-millimetre display unit, a typed",
"arithmetic expression with no unit suffix must be interpreted in the",
"DISPLAY unit, not silently taken as millimetres -- and whatever the field",
"showed must be what the document actually keeps, so a later compile (which",
"always evaluates in mm) reproduces the same size rather than quietly",
"reinterpreting a bare number as mm.",
"",
"ASSERTED POSITIVELY. Display unit = inches; the starter block's extrude",
"profile is 40x30 = 1200 mm^2.",
"",
"WHAT THIS CATCHES IF THE FIX REGRESSES: three spellings typed into",
"card:height and committed -- '15mm' -> 15 mm (18000), '1in' -> 25.4 mm",
"(30480), '0.5+0.5' -> the same 25.4 mm (30480), because an unadorned",
"number in an inch field is inches, not millimetres. If unit inference",
"regresses to defaulting bare numbers to mm, '0.5+0.5' would instead build",
"a volume of 600 mm^3 (1200 * 0.5) and the `c-expr.step` line fails.",
"",
"AND THE STORED BINDING TEXT IS PINNED SEPARATELY: the expression is also",
"what the DOCUMENT keeps, and the document re-evaluates it in millimetres",
"for every later compile -- so the saved binding must be unit-explicit,",
"'(0.5+0.5) in', or the part would change size the next time it was",
"opened. A fix that gets the EXPORTED geometry right by evaluating in the",
"display unit but then saves the bare, unit-less expression would pass",
"every volume assertion here and still corrupt the part on next load; the",
"final `expect_document` line is what that regression would fail.",
"",
"A click on an already-selected timeline row deselects it, so each re-arm",
"clicks twice."
],
"steps": [
{ "step": "wait_idle" },
{ "step": "click", "at": "text:settings" },
{ "step": "frames", "count": 8 },
{ "step": "click", "at": "text:units & precision" },
{ "step": "frames", "count": 8 },
{ "step": "click", "at": "text:in" },
{ "step": "frames", "count": 8 },
{ "step": "click", "at": "text:close" },
{ "step": "frames", "count": 12 },
{ "step": "click", "at": "timeline:1" },
{ "step": "wait_idle" },
{ "step": "click", "at": "card:height" },
{ "step": "key", "key": "ctrl+a" },
{ "step": "type", "text": "15mm" },
{ "step": "key", "key": "Enter" },
{ "step": "wait_idle" },
{ "step": "expect_no_error" },
{ "step": "export_step", "path": "{out}/a-15mm.step" },
{ "step": "wait_idle" },
{ "step": "expect_step", "path": "{out}/a-15mm.step", "volume": 18000, "solids": 1, "tol": 1e-9 },
{ "step": "click", "at": "timeline:1" },
{ "step": "frames", "count": 4 },
{ "step": "click", "at": "timeline:1" },
{ "step": "wait_idle" },
{ "step": "click", "at": "card:height" },
{ "step": "key", "key": "ctrl+a" },
{ "step": "type", "text": "1in" },
{ "step": "key", "key": "Enter" },
{ "step": "wait_idle" },
{ "step": "expect_no_error" },
{ "step": "export_step", "path": "{out}/b-1in.step" },
{ "step": "wait_idle" },
{ "step": "expect_step", "path": "{out}/b-1in.step", "volume": 30480, "solids": 1, "tol": 1e-9 },
{ "step": "click", "at": "timeline:1" },
{ "step": "frames", "count": 4 },
{ "step": "click", "at": "timeline:1" },
{ "step": "wait_idle" },
{ "step": "click", "at": "card:height" },
{ "step": "key", "key": "ctrl+a" },
{ "step": "type", "text": "0.5+0.5" },
{ "step": "frames", "count": 4 },
{ "step": "key", "key": "Enter" },
{ "step": "wait_idle" },
{ "step": "expect_no_error" },
{ "step": "export_step", "path": "{out}/c-expr.step" },
{ "step": "wait_idle" },
{ "step": "expect_step", "path": "{out}/c-expr.step", "volume": 30480, "solids": 1, "tol": 1e-9 },
{ "step": "save", "path": "{out}/c-expr.vernier" },
{ "step": "wait_idle" },
{ "step": "expect_document", "path": "{out}/c-expr.vernier", "pointer": "/state/bindings/0/expression", "equals": "(0.5+0.5) in" }
]
}