Three of this task's defects were cross-layer wiring rather than logic, so they belong in the spec where the next session will read them, not only in the task's research notes. Frontend: a new client spec covering layer ownership (baseline is never written, preview hides rather than overwrites), why a handle carries only its id, why a handle's position must derive from the clamped value, the ban on computing inside a setState updater, and why per-pointermove React state is the wrong owner of per-frame feedback. Backend: the left/right convention now states the three places that must agree and why a sign convention can be wrong on both sides of the wire at once; a table of the four profile fields the geometry stage actually reads, so nobody builds UI for interval-scoped editing again before the compiler honours it; and the save-is-not-applying contract, including that compileFresh must be installed by every path that sets session.area. Also records that an identity test cannot catch an ignored editsFile, and that a regression test is only trustworthy once it has been seen to fail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
6.6 KiB
Direct Edit Client
Trigger: any change under
workbench/client/src/edit/,MapCanvas.tsx, or the direct-edit wiring inApp.tsx. Read this before editing handles, ghosts, previews, or the save path.
Related: Direct Edit Solver And API for the wire contract this client consumes.
1. Layer ownership
Four layer groups, and only three of them are ever written by the client:
| Group | Written by | Rule |
|---|---|---|
baseline (createLayers()) |
updateLayers() only |
Never written by edit code |
editHandles |
EditHandleLayer.render() |
Handle features carry only handleId |
editGhost |
EditGhostLayer |
Client-side, non-authoritative |
editPreview |
EditPreviewLayer.show() |
Server geometry; hides baseline, never overwrites it |
EditPreviewLayer builds its own layer set from the same createLayers() factory
so preview and baseline cannot drift in styling, and show() hides only the
baseline layers it actually supersedes. clear() restores visibility from the
layer switches, never from a hardcoded default.
Forbidden: writing compiler output into a baseline source. It is the invariant the whole split exists to protect, and nothing in the type system enforces it.
2. A handle carries nothing but its id
Handle features hold handleId and nothing else. Kind, anchor, axis, range and
disabled reason are looked up in the manifest. That is why EditHandleLayer owns
both the source and the index — they cannot fall out of step.
Forbidden: copying manifest fields onto the feature. It creates a second constraint model on the client, which is the failure the cross-layer guide warns about.
3. Handle position comes from the value, never the cursor
handlePositionFor(handle, value) is the inverse of projectHandleValue(). A
drag projects onto the manifest's axis, clamps to the manifest's range, and the
handle is then placed from the clamped value.
This is not cosmetic. The ol-ext probe translated its proxy by the raw pointer
delta, and a drag reading -24.146 m left the handle 24 m out while the
constraint clamped at -5.400 m — the handle pointed at a road shape that cannot
exist. Any input adapter must own the position update for this reason.
4. Sides: left is tangent - 90
The manifest reports axisAzimuth = tangent + 90 for both sides, and that
azimuth is the geometry compiler's right. See the backend spec for why. On the
client this means outwardSign() negates the left side, and a test that asserts a
side must name the direction geographically (OUTWARD_LEFT / OUTWARD_RIGHT)
rather than by axis sign.
Mistake made: the first implementation assumed left was the positive axis, so dragging the visually-left handle moved the right kerb. Both the server placement and the client sign were wrong together, so neither side's tests caught it.
5. A constraint must travel with its operation
validateEditDocument() rejects any constraint whose provenance.operationId is
not a recorded operation, and the check covers already-saved constraints too.
Build the pair with draftConstraint() + operationFor(), and send
EditSession.fragment(), which assembles both halves.
One gesture mints one operation id, released on pointerup. Reusing an id across gestures puts two operations with the same id in the document, which is also rejected.
Mistake made: sending constraints alone returned HTTP 400 on every drag.
6. Never compute inside a setState updater
React defers updater functions and, under StrictMode, calls them more than once. By the time one runs, closure variables captured during the gesture may already be cleared.
// WRONG — threw `toLonLat(null)` and white-screened the page
setReport((current) => ({ ...current, meters: projectHandleValue(handle, start!, now) }));
// RIGHT — compute in the event handler, pass plain values in
const meters = projectHandleValue(handle, start, now);
setReport((current) => ({ ...current, meters }));
The start! non-null assertion is what hid the runtime problem from the type
checker. The same rule covers reading live OpenLayers state inside an updater: it
samples a different moment than the event did.
7. Per-frame feedback does not go through React
The ghost writes its OpenLayers source directly. Driving a readout from state on
every pointermove produced ~1600 renders in a single probe session.
requestAnimationFrame coalescing is the minimum; writing straight to the source
is the rule for production.
8. EditSession owns the constraint set, and is mutated in place
EditSession holds the saved baseline, its operations, the command stack, and the
previewSeq watermark. PreviewRequester captures the session object once, so
adopting a server document uses load() rather than constructing a new session —
a swapped object would leave the requester arbitrating for a session nobody reads.
previewSeq arbitration lives in acceptPreview(), not in the request layer: the
request layer only sends and cancels. The discard rule is worth unit-testing and
must not depend on network timing.
Undo of a saved gesture appends an inverse operation; persisted history is never rewritten. Undo of an unsaved one just moves the cursor.
9. The directEdit flag means nothing exists
With the flag off: no edit source is created, no layer is added, no interaction is
registered, and no manifest request is made. The network trace and the canvas must
match main exactly. Guard once at construction, not per call site.
intervalEditingSupported is a second, narrower switch: range handles stay hidden
while compileGeometry() ignores profile.interval. A control whose drag changes
nothing is worse than no control.
10. Saving is not the same as applying
POST /api/edits writes the document; /api/state serves the outputs of the last
compile. Saving without recompiling leaves a refresh showing the pre-edit geometry,
which is indistinguishable from a failed save. Report the save and the recompile
separately — if the recompile fails, the save still succeeded and the message must
say so.
Checklist before committing edit-client changes
- No baseline source is written
- No manifest field is copied onto a feature
- Handle positions derive from clamped values
- Sides asserted geographically, not by axis sign
- Constraints sent with their operations
- No geometry or OL reads inside a
setStateupdater - No per-
pointermoveReact state updates - Flag off ⇒ no sources, no layers, no interactions, no requests