Constant, ramp, and return offsets now append ordinary channel layers to a lane or cel in explicit owner frames. The inspector exposes the commands as one undoable transaction, shows conflicts, and offers removal or retry while preserving generated bases through regeneration. Ordered-stack compatibility is shared by validation, conflict reporting, and regeneration, including adjacent replacement coverage. Cel-sheet gaps and headers select their lane, so commands cannot fall through to another column's stale selection. 437 tests, 5,804 assertions; both browser flows; 56 Django tests; optimized frontend build.
232 lines
13 KiB
Markdown
232 lines
13 KiB
Markdown
# Correction authoring implementation plan
|
|
|
|
Written against `2f1c9b9` (2026-09-30), following the lane handoff in
|
|
`7a54bfc`. Implemented on `codex/correction-authoring`; this now records the
|
|
scope and acceptance criteria of that implementation.
|
|
Read [lane-handoff.md](lane-handoff.md) and the correction section of
|
|
[lane-model.md](lane-model.md) first. Their ownership and document rules remain
|
|
the foundation. The choices below settle the first implementation's scope.
|
|
|
|
## Outcome
|
|
|
|
A person can select a lane or cel, specify a range, and apply Constant
|
|
adjustment, Ramp, or Return motion to rotation or position. The result is one
|
|
correction layer and one undo step. It works from either timing view. A lane
|
|
correction crosses drawing boundaries; a cel correction travels with its cel.
|
|
Regeneration preserves the hand work and presents incompatible layers for an
|
|
explicit decision. Frames outside the support evaluate exactly as before.
|
|
|
|
Finish this vertical slice before adding more property types or gestures.
|
|
Numeric range fields and an Apply button are sufficient for this pass. Dragging
|
|
a range or manipulating a peak on the stage can later issue the same command.
|
|
|
|
## 1. Fix sheet targeting first
|
|
|
|
In `ui/timeline.cljs`, `cel-sheet` currently drops each lane row's `:select`.
|
|
An occupied cell selects its cel; a gap only seeks, leaving the previous target
|
|
selected. Thus clicking lane B's gap after selecting lane A can send an insert
|
|
or overwrite to A.
|
|
|
|
Carry the row's complete selection address into its column and gap cells.
|
|
An occupied cell selects its cel; a gap selects its lane. Make the column header
|
|
select the lane too: this is the explicit way to author across drawings.
|
|
Preserve full paths, not just `(peek path)`, as view identity. Keep seek and
|
|
selection dispatch order deterministic.
|
|
|
|
Add a two-lane browser case: select A, click a gap in B, overwrite, assert that
|
|
only B changes, and undo once. Add a header-selection assertion. Retain the
|
|
existing occupied-cell hold test. Do not redesign sheet rendering in this step.
|
|
|
|
## 2. Make stack compatibility consistent
|
|
|
|
There is a concrete discrepancy at this HEAD:
|
|
|
|
- `channel/problems` uses `stack-conflict`, accounting for prior replacements.
|
|
- `channel/conflicts` and `flow/regenerate.cljs`'s `rebased` use
|
|
`conflict-with`, comparing an offset directly with the base.
|
|
|
|
A two-component base, a covering three-component replacement, then a
|
|
three-component offset is valid and evaluates correctly, but the latter paths
|
|
can report or mark that offset incompatible. Conversely a replacement can make
|
|
an offset incompatible even when it fits the original base.
|
|
|
|
Extract one ordered-stack compatibility operation and use it for validation,
|
|
conflict discovery, regeneration, and resolution. Keep `conflict-with` if useful
|
|
for the narrower question its name/docstring describe. Do not use it alone to
|
|
decide whether a stacked layer is applicable.
|
|
|
|
Compute compatibility against the values that can actually reach a layer over
|
|
its support. Partition at overlapping support boundaries if needed: two adjacent
|
|
replacements can jointly cover an offset even though neither covers it alone.
|
|
An empty replacement channel does not supply a value and must not erase the
|
|
possible input shape. Preserve the evaluator's absence behavior. Explicitly
|
|
marked conflicts are skipped, so later layers must be checked against the stack
|
|
that actually runs. During regeneration, recompute compatibility in order, using
|
|
each preceding layer's resulting active/conflicted state. Preserve IDs, values,
|
|
support, and order; update compatibility reasons without dropping hand work.
|
|
|
|
Use structural shape reasoning for dense data rather than requiring every block
|
|
to be sampled. If unknown shape or missing samples limit what can be proven,
|
|
retain the current absence contract and document that limit; do not claim an
|
|
unconditional proof of runtime safety from incomplete metadata.
|
|
|
|
Tests: covering replacement of a different shape; partial coverage; adjacent
|
|
covering replacements; empty replacement; inactive conflicted replacement;
|
|
regeneration changing base shape; and the same cases through cursor evaluation.
|
|
Assert that an accepted compatible stack is not listed as a conflict, and that
|
|
an incompatible regenerated layer remains persisted but is skipped.
|
|
|
|
## 3. Pure correction commands
|
|
|
|
Add `frontend/src/arthur/domain/correction.cljs`. It owns authoring and resolving
|
|
corrections; `channel.cljs` continues to own evaluation and compatibility.
|
|
Suggested API (names may follow repository conventions):
|
|
|
|
```clojure
|
|
(add clip sid node-id channel-path
|
|
{:id layer-id :support [a b] :motion :return
|
|
:start 0 :peak angle :peak-frame p})
|
|
(remove-layer clip sid node-id channel-path layer-id)
|
|
(retry-layer clip sid node-id channel-path layer-id)
|
|
```
|
|
|
|
Return `{:clip updated :selection node-id}` or `{:refused reason}`. IDs come
|
|
from the event caller (`random-uuid`), never from the pure command. Reject a nil
|
|
ID or one already used within that channel stack. Address layers by the full
|
|
symbol/node/channel/layer tuple; no global layer registry is needed.
|
|
|
|
Resolve the base via `node/channels`, which supplies defaults. A lane with no
|
|
explicit rotation channel already has a zero rotation; materialize that channel
|
|
with its new `:over`. Preserve every existing base field, generated provenance,
|
|
and previous layer. Never route this through a setter that bakes the correction
|
|
into base keys. Append to the ordered stack and validate the resulting document.
|
|
Do not run correction edits through `lane/finish`, whose extent policy belongs
|
|
to cel arrangement. Use `clip/problems` for the candidate document instead.
|
|
|
|
First authoring properties: `[:xform :rot]` (scalar radians) and `[:xform :pos]`
|
|
(two numeric components). First blend operation: `:offset`. UI labels must say
|
|
offset/delta, since a target offset of 20 degrees does not mean an absolute
|
|
rotation of 20 degrees. Keep existing `:replace` evaluation and loaded stacks;
|
|
there is no new replacement-authoring UI in this slice.
|
|
|
|
All command support endpoints are finite integer OWNER frames, `[a b)`, with
|
|
`a < b`. Do not ban negative owner frames merely because displayed shot frames
|
|
start at zero. Validate all supplied values for finite numbers and exact shape.
|
|
Refuse unknown targets, unsupported properties/motions, malformed ranges, and
|
|
incompatible stacks with useful messages. Refusal must not mutate store/history.
|
|
|
|
Motion construction uses existing channels only:
|
|
|
|
| Command | Values | Minimum samples |
|
|
| --- | --- | --- |
|
|
| Constant adjustment | `(ch/framed delta)` | 1 |
|
|
| Ramp | `(ch/keyed {a start, (dec b) end} :linear)` | 2 |
|
|
| Return motion | `(ch/keyed {a start, p peak, (dec b) start} :linear)` | 3 |
|
|
|
|
For Return, require integer `a < p < b-1`. Default the UI peak to
|
|
`a + floor((b-a-1)/2)`; on an even-length range the earlier middle sample wins.
|
|
Expose the peak frame so this is visible and adjustable. Never place an endpoint
|
|
at `b`: it is outside the selected samples. `[10 13)` with start 0 and peak 0.5
|
|
must yield offsets `0, 0.5, 0` at 10, 11, 12. Support controls the boundary;
|
|
there is no need to insert zero keys into the base before/after it.
|
|
|
|
## 4. Owner and range UI
|
|
|
|
Add a Corrections section to the right pane (`ui/params.cljs`), extracting a
|
|
`ui/corrections.cljs` component if that keeps the pane readable. Provide an
|
|
explicit target readout (symbol, lane or cel), Rotation/Position, motion choice,
|
|
From/Through fields, relevant value fields, peak frame for Return, and Apply.
|
|
Offer a selected cel's owning lane as an explicit target choice. Do not silently
|
|
promote a cel edit to a lane edit. Shared drawing content is outside this first
|
|
UI; it has different sharing consequences.
|
|
|
|
For this first pass the range fields explicitly read **owner frames**, with
|
|
inclusive From/Through converted to `[from, through+1)`. This is a deliberate
|
|
UI scope choice, not a claim that a displayed shot range and owner range are
|
|
interchangeable. It lets nested and retimed owners be addressed without an
|
|
unproven range conversion. Match the existing zero-based numbering and show the
|
|
owner beside the range. The handoff must record that displayed-range dragging
|
|
is still outstanding.
|
|
|
|
Use an explicit three-sample initial draft in owner coordinates; for a cel,
|
|
prefer its span start where it is integral. Show the range, allow adjustment,
|
|
and do not extend a cel or shot to make the correction visible. Reset the draft
|
|
when the target changes. Rotation is shown in degrees and converted to radians
|
|
at the event boundary, following the existing inspector convention. Position
|
|
uses x/y inputs in the owner's transform coordinates.
|
|
|
|
Draft inputs must not write document state, start history groups, or invoke the
|
|
existing inspector `number-input`'s hold/settle behavior. Apply dispatches one
|
|
event; success uses `edit/transaction` once and preserves the selected node's
|
|
full address. Do not use a layer ID as node selection. Validate again at Apply,
|
|
since the target/document may have changed since the draft was opened.
|
|
|
|
If adding a “use playhead” convenience, prove its mapping separately. The clock
|
|
of a cel's transform is its own node clock, not the drawing source clock selected
|
|
by `:playback`. `nest/inside` on the complete cel path enters the source and is
|
|
therefore the wrong shortcut. The existing `selection-frame` resolves only the
|
|
owning symbol; node/ancestor time conversion remains necessary. Floors, loops,
|
|
and nonintegral mappings must never silently snap an authored range. Omit this
|
|
convenience rather than expanding the first pass into a new timing system.
|
|
|
|
## 5. Conflict actions and regeneration proof
|
|
|
|
Show a document-wide list from `clip/conflicts` in the pane, including symbol,
|
|
node, property, layer ID, and reason. Keep it accessible even when a different
|
|
node is selected. Also list the selected target's layers in stack order with
|
|
support, motion values, and status; do not require a new persisted motion label.
|
|
|
|
Provide Remove correction and Retry compatibility. Remove is explicit and
|
|
undoable; Retry rechecks the complete candidate stack and clears a conflict only
|
|
when it is valid. If retry would invalidate a downstream offset, refuse and say
|
|
why. Similarly, removing a replacement that makes a later offset invalid must
|
|
refuse, rather than commit an invalid document or silently remove more layers.
|
|
A retry that changes nothing must not manufacture an undo step.
|
|
|
|
These are minimal resolution actions, not topology remapping. Automatic geometry
|
|
remapping, reordering layers, editing arbitrary stored vector values, and resolving
|
|
removed targets are separate work. Preserve all existing generated-data behavior.
|
|
|
|
Exercise the actual `flow/regenerate.cljs` entry points in integration tests:
|
|
author a correction, regenerate compatible base data, and confirm the correction
|
|
survives with its ID/support/values intact and affects the new base. Then change
|
|
topology on a geometry fixture and assert persisted actionable conflicts. The
|
|
geometry fixture can use an existing layer directly; geometry authoring is not
|
|
required to expose and resolve a conflict already present in a document.
|
|
|
|
## 6. Verification and completion
|
|
|
|
Use focused tests that establish observable promises:
|
|
|
|
- Domain: each motion's sample values; refusal on short ranges/nonfinite values;
|
|
default channel materialization; unchanged base and prior layers; duplicate ID.
|
|
- Evaluation: compare before/after on every frame outside support, including
|
|
neighboring interpolated frames. Cursor/spec agreement in nonmonotonic order.
|
|
- Ownership: a lane Return crosses a drawing boundary; a cel correction moves
|
|
with the cel and survives split/trim. Neither alters another use of its drawing.
|
|
- Sampling: picture-rate/pose selection changes the generated base frame while
|
|
the authored correction still reads owner time. Keep HEAD's regression tests.
|
|
- Events: one Apply is one undo step; undo/redo restores complete layer data;
|
|
refusal leaves clip/history unchanged; stale target refuses; selection survives.
|
|
- Persistence: leaf and Transit round trips retain layers, order, IDs, conflicts.
|
|
- Browser: both views can select a target and apply the same correction using
|
|
actual controls. Assert evaluated results and history, not only a layer count.
|
|
Include the two-lane gap-targeting case from step 1.
|
|
- Regeneration and conflict actions: use the real flow and test undoable removal,
|
|
valid retry, invalid retry, and removal that would break a downstream layer.
|
|
|
|
Run the suites documented in `lane-handoff.md`: CLJS tests, lane and take browser
|
|
flows, Django tests, and optimized frontend build. Restore the dev app bundle
|
|
after the release build. Note that `take.mjs` writes a local project. Report
|
|
actual results and any unrun checks; do not copy previous test counts as evidence.
|
|
|
|
Suggested commit sequence: sheet targeting; consistent stack compatibility;
|
|
pure correction commands; pane/events plus browser proof; updated handoff.
|
|
Keep each commit coherent and tested. No schema version bump should be needed:
|
|
the layers already have a persisted representation.
|
|
|
|
Update `lane-handoff.md` and `lane-model.md` with what shipped, the owner-frame
|
|
range UI limitation, conflict actions available, and verified test counts. Done
|
|
means a person can author and undo the correction, regenerate its base, and see
|
|
either their preserved edit or a useful conflict. A constructor without reachable
|
|
controls, or controls without that regeneration proof, does not finish this work.
|