engine: audit and repair diagram syncs after model edits

docs/design/layout-quality.md

@@ -157,6 +157,53 @@ values beside the current ones. The committed baseline
 (`examples/layout_eval_baseline.json`, see its README) is diffed the same way on
 every run.
 
+## Evaluating edits
+
+A diagram an agent or a notebook user edits is synced after every patch by
+`incremental_layout`. Its quality is not one static score: a sync must leave
+alone what the edit did not touch, keep the view consistent with the model, and
+put what it creates somewhere sensible. `layout::edit_audit` states that
+contract from an edit's inputs and outputs alone, in three layers:
+
+- **Scope.** An untouched element comes back exactly as it was. Touched is
+  derived from the two models and the patch: a deleted variable, a renamed one
+  (only its name changes), one whose kind changed (rebuilt, keeping its center
+  unless it became a flow or its new shape there would cover another shape), a flow whose attachment changed. A link whose
+  dependency survives keeps its uid, endpoints, polarity, and shape, and a link
+  drawing no dependency the model has survives unless the patch names its
+  reader. A connector the view did not draw is drawn only into a variable the
+  patch names or between elements drawn for the first time, and a variable the
+  view did not draw is drawn only when the patch names it: what an author left
+  out elsewhere stays out.
+- **Consistency.** Every variable drawn once with its kind, references resolve,
+  links and drawn dependencies agree, flows attach where the stock lists say,
+  and every flow the sync created or changed holds the strict flow invariants
+  (`editing::invariants`). Only findings the edit introduced count, so an
+  imported view's own inconsistencies are not charged to an edit.
+- **Placement.** What the sync created or changed does not cover another shape
+  it did not already cover before the edit, and a pipe it routed does not pass
+  through a stock that is not one of its ends.
+
+It also records what it does not charge: how far rebuilt elements moved, and
+the metric's cost before and after.
+
+`layout::edit_scenarios` generates the edits for any model -- restate a
+variable, add or delete a parameter, insert an intermediate, delete a flow or a
+middle stock, detach a flow, turn an aux into a stock, rename (with the rename
+operation, and the way an agent without one does it), add a flow between two
+stocks, close a loop, extend a chain, add a side flow, add a sector, add then
+undo -- picking targets deterministically, and runs each through `apply_patch`
+and the production sync rule, auditing every step and checking that two syncs
+of one edit agree and that an edit expected to return the original view does.
+
+The unit battery (`layout/edit_scenarios_tests.rs`) drives every scenario over
+hand-drawn and imported views and pins every finding in `KNOWN_DEFECTS`, one row
+per (fixture, scenario, finding) naming the defect: a finding no row expects
+fails, and so does a row that no longer reproduces. The harness runs the same
+scenarios over the whole corpus (`LAYOUT_EVAL_EDITS=0` skips them) and writes
+`edits.json` and `edits.html`, with each run's last step rendered before (removed
+elements marked) and after (created, changed, and every located finding marked).
+
 ## The improvement loop
 
 1. Run the harness on the current code into one directory, and on the changed

src/libsimlin/CLAUDE.md

@@ -71,7 +71,7 @@ Error formatting has no module here: `src/patch.rs` imports `simlin_engine::erro
 ### Layout
 
 - **`src/layout.rs`** - Automatic diagram layout:
-  - `simlin_project_diagram_sync(project, model_name, patch_json, out_error)` - Generate layout for a model, replacing its views in-place. When `patch_json` is non-null and the model already has a non-empty view, uses incremental layout (preserving existing element positions); otherwise generates a full layout from scratch. Preserves existing zoom. Works on all targets including WASM. Requires the project to be synced to the salsa db first (returns an error otherwise).
+  - `simlin_project_diagram_sync(project, model_name, patch_json, out_error)` - Generate layout for a model, replacing its first view in place (added when the model has none); any other view the project carries is the author's and comes back unchanged. When `patch_json` is non-null and the model already has a non-empty view, uses incremental layout (preserving existing element positions); otherwise generates a full layout from scratch. Preserves existing zoom. Works on all targets including WASM. Requires the project to be synced to the salsa db first (returns an error otherwise).
 
 ### Diagram editing
 

src/libsimlin/src/layout.rs

@@ -214,7 +214,13 @@ pub unsafe extern "C" fn simlin_project_diagram_sync(
         layout.zoom = zoom;
     }
 
-    // Model existence was verified above, so this should always succeed.
+    // Model existence was verified above, so this should always succeed. The
+    // layout syncs the first view only; any other view the project carries is
+    // the author's and stays as it was.
     let model = datamodel_locked.get_model_mut(model_name_str).unwrap();
-    model.views = vec![engine::datamodel::View::StockFlow(layout)];
+    let view = engine::datamodel::View::StockFlow(layout);
+    match model.views.first_mut() {
+        Some(first) => *first = view,
+        None => model.views.push(view),
+    }
 }

src/libsimlin/tests/integration/diagram.rs

@@ -47,6 +47,50 @@ fn test_diagram_sync_sir_model() {
     }
 }
 
+#[test]
+fn test_diagram_sync_keeps_every_view_but_the_first() {
+    // The sync lays out a model's first view; a project can carry more, and
+    // those are the author's, so they come back exactly as they were.
+    let mut datamodel = TestProject::new("two_views")
+        .with_sim_time(0.0, 10.0, 1.0)
+        .stock("population", "100", &["births"], &[], None)
+        .flow("births", "population * 0.02", None)
+        .build_datamodel();
+    let laid_out = engine::layout::generate_best_layout(&datamodel, "main", None).expect("layout");
+    let overview = datamodel::View::StockFlow(datamodel::StockFlow {
+        zoom: 0.5,
+        ..laid_out
+    });
+    let empty_first = datamodel::View::StockFlow(datamodel::StockFlow {
+        elements: Vec::new(),
+        ..match &overview {
+            datamodel::View::StockFlow(sf) => sf.clone(),
+        }
+    });
+    datamodel.models[0].views = vec![empty_first, overview.clone()];
+    let proj = open_project_from_datamodel(&datamodel);
+
+    unsafe {
+        let model_name = CString::new("main").unwrap();
+        let mut err: *mut SimlinError = ptr::null_mut();
+        simlin_project_diagram_sync(proj, model_name.as_ptr(), ptr::null(), &mut err);
+        assert!(err.is_null(), "diagram_sync should succeed");
+
+        let datamodel_locked = (*proj).datamodel.lock().unwrap();
+        let model = datamodel_locked.get_model("main").unwrap();
+        assert_eq!(model.views.len(), 2, "no view is dropped");
+        let datamodel::View::StockFlow(first) = &model.views[0];
+        assert!(!first.elements.is_empty(), "the first view is laid out");
+        assert!(
+            model.views[1] == overview,
+            "the second view comes back as it was"
+        );
+        drop(datamodel_locked);
+
+        simlin_project_unref(proj);
+    }
+}
+
 #[test]
 fn test_diagram_sync_test_project() {
     let test_project = TestProject::new("layout_test")

src/pysimlin/e2e/notebook-editor.spec.ts

@@ -285,11 +285,14 @@ test('pysimlin-widget.AC4.2: JupyterLab notebook edits a model file through the
 
   // Put the creation tool away, then a click (no drag) on the variable's
   // circle opens its details (a click on the label would start renaming
-  // it).  Clicking the rendered-equation preview swaps in the raw editor.
-  // The details panel's classes are CSS-module names, `<local>-<hash>`.
+  // it). The new aux's empty equation is an equation error, and a variable
+  // with an equation error opens its details on the raw equation editor rather
+  // than the rendered preview (VariableDetails' showPreview), so the editor is
+  // already there to type into. The details panel's classes are CSS-module
+  // names, `<local>-<hash>`.
   await widget.getByRole('button', { name: 'Variable', exact: true }).click();
   await canvas.locator('g.simlin-aux', { hasText: 'New Variable' }).locator('circle').first().click();
-  await widget.locator('[class*="eqnPreview"]').click();
+  await expect(widget.getByText('error: Variable has empty equation')).toBeVisible();
   const equationEditor = widget.locator('[data-slate-editor="true"][class*="eqnEditor"]');
   await expect(equationEditor).toBeVisible();
   await equationEditor.click();

src/simlin-engine/CLAUDE.md

@@ -188,8 +188,9 @@ Opt-in: a model that declares units on no variable gets no unit diagnostics. `un
 ## Analysis, layout, diagrams
 
 - `analysis.rs::analyze_model` bundles compilation, LTM discovery, and dominant-period selection into `ModelAnalysis`; a model that cannot compile returns `Ok` with `analysis_error` set so "could not analyze" is distinct from "no loops".
-- `layout/` generates and incrementally updates diagram layouts (force-directed placement, crossing reduction, a calibrated quality metric; deterministic per seed). The metric and the eval harness that measures layouts against it are described in [layout quality](/docs/design/layout-quality.md). The incremental path lives in `layout/incremental.rs`: chains an edit adds whole are laid out as chains beside the diagram, a new stock hung off a drawn chain continues its row, and what the edit added is decluttered around the fixed diagram (`declutter::declutter_part`). Incremental layout never rewrites an element the patch did not touch: position and `label_side` come back byte for byte (`layout_label_tests.rs` enumerates the arms), a label side is chosen only for elements created in that pass, and a new connector that runs through an existing label is accepted rather than re-optimizing its neighbours -- hand placement wins, and re-optimizing is exactly the churn that snaps a notebook user's dragged label somewhere else on the next edit. A label the layout wraps carries the stored two-character `\n` escape (`text::LABEL_LINE_BREAK`, the form the TypeScript editor's `encodeNameNewlines` produces), never a raw newline.
-- The flow arm of that rule (`layout_flow_tests.rs` enumerates it): incremental layout rebuilds a flow only when the patch creates it or changes the flow's own attachment -- moves it to another stock, drops it from a stock's list (that end becomes a cloud), lists it on a stock at its cloud end (that end becomes the stock), deletes an attached stock, or changes an attached stock's kind -- because its stored endpoints then name the wrong element. A flow the patch names keeps its geometry (a rename changes only its name, a delete removes it with its clouds), and every other flow, a sibling of a flow added to or removed from the same stock included, comes back byte for byte even where a fresh layout would draw it differently. A created side flow takes a face its stock's drawn side flows leave free where there is one. The endpoint snap (`resnap_flow_endpoints`), the slot placement (`face_slots`: a created flow's stock end keeps the design plan's `PIPE_SPACING` from the ends already on its face where the face has room, else the farthest slot, always within the corner clearance; a two-point flow between parallel faces takes one line only where that line keeps the spacing on both faces; a created cloud end keeps off other clouds) and the finishing pass (`finish_flow_geometry`) run only on the flows the pass creates. The one repair made to an untouched flow is wiring an endpoint the view left unattached to the flow's own cloud (`diff_clouds`), which moves nothing.
+- `layout/` generates and incrementally updates diagram layouts (force-directed placement, crossing reduction, a calibrated quality metric; deterministic per seed). The metric and the eval harness that measures layouts against it are described in [layout quality](/docs/design/layout-quality.md). The incremental path lives in `layout/incremental.rs`: chains an edit adds whole are laid out as chains beside the diagram, a new stock hung off a drawn chain continues its row, and what the edit added is decluttered around the fixed diagram (`declutter::declutter_part`). Incremental layout never rewrites an element the patch did not touch: position and `label_side` come back byte for byte (`layout_label_tests.rs` enumerates the arms), a label side is chosen only for elements created in that pass, and a new connector that runs through an existing label is accepted rather than re-optimizing its neighbours -- hand placement wins, and re-optimizing is exactly the churn that snaps a notebook user's dragged label somewhere else on the next edit. A dependency the view does not draw gets a connector only where the edit is about it -- into a variable the patch names (upserts, renames, re-lists), or where an end is drawn for the first time -- so a connector an author left out stays out whatever unrelated edit follows; likewise a link drawing no dependency the model has goes only with an edit to its reader, and a variable the view does not draw is drawn only when the patch names it. A connector or cloud the pass creates references only what the view draws: a variable left out of the view can still have a uid (a project MCP opened gives every variable one), and a link into it or a cloud of it would name no element. An element the pass rebuilds -- a variable whose kind changed, a flow whose attachment changed -- keeps its uid (`LayoutState::remove_for_rebuild`), so the links and aliases touching it survive, and a surviving curved link whose endpoint moved turns its takeoff with its chord (`rebow_moved_links`); a variable that became a stock, aux or module is redrawn at its old center and no polish pass moves it, save that one whose new, larger shape covers another shape there (a parameter turned into a stock) moves to the nearest clear spot. A label the layout wraps carries the stored two-character `\n` escape (`text::LABEL_LINE_BREAK`, the form the TypeScript editor's `encodeNameNewlines` produces), never a raw newline.
+- The flow arm of that rule (`layout_flow_tests.rs` enumerates it): incremental layout rebuilds a flow only when the patch creates it or changes the flow's own attachment -- moves it to another stock, drops it from a stock's list (that end becomes a cloud), lists it on a stock at its cloud end (that end becomes the stock), deletes an attached stock, or changes an attached stock's kind -- because its stored endpoints then name the wrong element. A drawn flow whose attachment changed is re-attached in place through the editing core (`retarget_flow`, over `editing::{heal, route, route_end}`): an end that becomes a cloud stays where it was, moved off a stock it no longer attaches to (`clear_of_stocks`), an end that attaches to another stock is routed to it keeping as much of the pipe as stays valid, and the flow keeps its uid, name and valve; only a flow that must attach to a stock this pass creates is rebuilt as new. A flow the patch names keeps its geometry (a rename changes only its name, a delete removes it with its clouds), and every other flow, a sibling of a flow added to or removed from the same stock included, comes back byte for byte even where a fresh layout would draw it differently. A created side flow takes a face its stock's drawn side flows leave free where there is one. The endpoint snap (`resnap_flow_endpoints`), the slot placement (`face_slots`: a created flow's stock end keeps the design plan's `PIPE_SPACING` from the ends already on its face where the face has room, else the farthest slot, always within the corner clearance; a two-point flow between parallel faces takes one line only where that line keeps the spacing on both faces; a created cloud end keeps off other clouds) and the finishing pass (`finish_flow_geometry`) run only on the flows the pass creates. The one repair made to an untouched flow is wiring an endpoint the view left unattached to the flow's own cloud (`diff_clouds`), which moves nothing.
+- What a sync after an edit must do is stated independently of `incremental.rs` by `layout/edit_audit.rs` (scope: untouched elements and surviving links come back as they were; consistency: the view agrees with the model and routed flows hold the strict invariants, charging only what the edit introduced; placement: created elements land on no shape and routed pipes through no foreign stock), and `layout/edit_scenarios.rs` generates the edits an agent makes for any model and drives them through `apply_patch` and the production sync rule. The battery (`edit_scenarios_tests.rs`) pins every finding in `KNOWN_DEFECTS`, each row naming its defect: a new finding fails, and so does a row that stops reproducing, so a fix deletes its rows. Element order is the view's draw order and what a saved file lists: a sync keeps surviving links and clouds in place and appends what it creates in a deterministic order.
 - `diagram/` renders a model's first stock-and-flow view as SVG (`render_svg`, byte-identical to the TypeScript static renderer; `src/diagram/tests/svg-rendering.test.ts` is that parity test), as PNG (`render_png`, behind `png_render`), and as a scene display list (`build_scene`, whose contract is [the diagram scene](/docs/design/diagram-scene.md)), and exposes the exact geometry the layout metric scores. Every drawing decision has one owner, and both serializers read it:
   - `resolve::resolve_view` decides which elements are drawn (a link or flow whose endpoints are missing from the view is not), their layer and draw order, whether each is arrayed, an alias's target, and the fit-to-content bounds.
   - Each element's geometry is one function: `aux_geometry`, `stock_geometry`, `module_geometry`, `alias_geometry`, `group_geometry`, `cloud_transform` (over the shared `CLOUD_PATH`), `flow_geometry`, `connector_geometry`, `arrowhead_geometry`, and `label::label_lines` for per-line label anchors. `render_*` prints from it and `scene.rs` reads it. Never compute a drawn number a second time in either serializer: a hand-maintained twin drifts exactly where the geometry is non-trivial; extend the geometry function instead.