Files
ARCADIA-Studio/harness/experiments/scenario-resolver-review-20260913/review.md
T

60 lines
12 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Frozen Scenario resolver correctness review
Scope: manifest-verified snapshot .local/scenario-resolver-review-snapshot-20260913, based on typed import695685a. Read all three production files; verified every byte count and SHA256 against manifest.json. No source changes, compiler, test execution or GUI. References below are snapshot lines unless explicitly marked unchanged peer projector. Parent reviews tests separately.
## P1: successful resolution can suppress sibling lifetime geometry
scenario_layout.rs:315–357 checks each operand scope only against its enclosing frame/caller. The unchanged scenario_lifecycle.rs Projection::sequence also requires sibling scopes to be monotone in declared branch order: it sets cursor to each previous branch.end before checking the next branch.start. The resolver does not enforce this prerequisite and does not inspect the resulting new MissingGeometry uncertainty.
Concrete admitted fixture: one root ALT at148, two operands each containing one asynchronous message, with two ordinary work lifelines and an accepted exchange. Actual traversal yields message rows293/475, operand scopes251..343 and433..525, frame148..567. Give only the first message send_y500. Its footprint ends500 and retains50px scope bottom padding, so first scope becomes251..550. Second remains433..525. Both pass enclosing checks; the actual projector then skips operand2 because433<550, marks MissingGeometry and makes the post-fragment lane lifetime Unresolved. There are no Calls, so the later activation guards cannot reject this. resolve returns Ok with missing lifetime geometry. This is a production defect, independent of the unsettled policy for asynchronous arrow crossings.
Smallest correction: validate effective sibling scope intervals in declared branch traversal order, returning a precise unsupported-geometry result when the unchanged projector cannot enter the next operand. Retain feasible nonoverlap and no-op fixtures; do not silently move the second operand or change semantic order. Author has been notified and plans this narrow guard.
## P1: expanded operand can erase a following lifecycle marker
The same missed cursor invariant extends beyond sibling checks. Start with the previous ALT and then a root Delete at631 for an existing receiver. Move only the second operand message475→700. Its scope grows433..750; the frame grows to792 using the existing92px descendant padding. The following root Delete remains631. The enclosing-operand checks still pass. The Delete is inside the root85..height−20 interval (scenario_layout.rs:359–387), and the direct lifecycle inversion check248–281 has no preceding direct lifecycle event to compare.
After visiting operand2, the actual projector cursor is750. Delete631 fails its y>=cursor check, producing MissingGeometry without a Death marker. No Call exists to trip the baseline-activation comparisons635–697. The resolver therefore reports success while the previously visible valid X disappears. Expected behavior for the unchanged projector is explicit unsupported geometry, or a separately justified projector change; retaining an unaligned Delete arrow without its marker is not successful realization.
Smallest correction: enforce the real per-scope event/fragment cursor precondition, including the position after all branches before subsequent relevant lifecycle events/fragments. Merely rejecting sibling overlaps leaves this case open. A feasible control is second message520 with Delete631, preserving the exact Death marker and all lifetimes. No blanket ban on every asynchronous crossing is implied. Parent and author were notified.
## P2: negative-lane extent is interpreted twice
Independently confirmed the parent's numeric finding. behavior.rs:1335–1345 sets scene.size.width to right−layout_left when authored lanes exist; scenario_layout.rs:724 interprets that width as an absolute right by seeding Rectangle(ORIGIN,scene.size). Lanes−500/+900 yield layout_left−660, right1140, width1800. The initial bounds instead extend to right1800, adding660 units of empty right margin before unioning real geometry. Large signed coordinates can materially worsen Fit scale and scrolling even though all geometry is retained.
Carry the base rectangle's horizontal origin alongside its width, or otherwise retain an unambiguous absolute-right contract. Seed structural_bounds with that rectangle. Preserve actual authored coordinates and union any extra activation/loop/marker extents; do not translate source positions to conceal this. This is an extent/Fit correctness issue, not a newly missing object.
## Confirmed boundaries and remaining limits
- Representation is explicit Derived(owner) or Saved(diagram); each resolution reads exactly that saved layout and returns the Scene with its representation. There is no global mutable layout state. Project is immutably borrowed; source/history writes are absent. Document/revision/view-visit freshness remains caller responsibility as documented.
- Sparse messages without a drawn line remain latent and are not normalized; direct addressing is checked and expanded paths explicitly rejected. Self identity is based on sender/receiver IDs in the adapter and lifecycle marker, rather than coincident x. Parent independently found the header-center f32 roundtrip bug at558–567/702–711 and has asked the author to preserve the exact builder lane map; this report does not duplicate ownership of that fix.
- Rendering order is complete rows/scopes/height → lifetime → lifecycle endpoint alignment → typed activation layout. Whole self-lifecycle send overrides explicitly convert to receipt(+24). Production typed activation representation is retained, not reconstructed from obsolete boxes.
- Current baseline-segment checks are narrower than proof of the complete activation/lifetime vector. Final tests must assert nonempty full expected vectors where applicable, plus exact markers/uncertainties for the two regressions above, not only call-start existence or equal vector lengths.
- Structural bounds include boxes, paths, labels, typed bars, open-call cues and lifecycle geometry; this is structural coverage, not guaranteed text ink/stroke containment or future viewport wiring.
- The source diagnostics for arbitrary event crossing are not a settled gesture policy. No runtime or semantic endorsement of arbitrary nested Call/Reply crossings follows from this review. The active saved-view UI, preview transactions and native workflow remain outside this production slice.
No additional verified blocker was established in this bounded pass. Findings describe the original immutable snapshot; fixes require their own diff/readback and parent-owned gates.
## P1 follow-up: public send precision can cause a false geometry rejection
Independently verified parent numeric finding using explicit float32 rounding at every arithmetic operation, without compiling. scenario_layout.rs:238–243 computes a baseline-relative delta and adds it back to baseline endpoints. For seed148, authored .1f32 (.10000000149011612) becomes Plan.send .100006103515625; authored1e−6f32 becomes0. Ordinary adapter rows retain the exact authored value, so those cases produce plan/Scene disagreement even when the ordinary painted send is correct.
A consequential feasible reproducer is a root async/Call/Reply at automatic148/240/332, then saved async90 and Call100.1f32. Semantic order remains90<100.1<332 and all endpoints remain within the root scope. For the ordinary Call, Plan.receive becomes100.100006103515625 while its actual Scene row and typed activation start are100.0999984741211. The final exact start comparison at672–695 then rejects this valid spacing as TypedActivationOutsideLifetime. This is not merely subpixel paint drift.
Self Create/Delete have an additional rendered-endpoint loss: authored send64.next_down() is63.999996185302734; the receipt conversion at548–552 rounds send+24 to88, then render.rs:153–158 reconstructs send88−24 as64. The receipt is feasible (root start85); the returned line does not realize the exact authored send.
Smallest correction: retain explicit authored send in the complete message pair, derive receive once according to message identity/type, and apply the resolved pair to actual line endpoints after lifecycle alignment but before typed activation derivation. Do not recover public send by subtracting a rounded receive or adding a cancelled delta. Preserve both top self-loop points at send and both lower points at receive; retain lifecycle markers at the receipt.
Required exact-bit tests: (1) .1 and1e−6 ordinary messages assert stored send, Plan.send and actual first endpoint agree; (2) async90/Call100.1/Reply332 resolves successfully and has an explicitly nonempty exact typed activation with start100.1; (3) self Create and self Delete separately at64.next_down() assert first two y values equal stored send, final two and marker y equal correctly rounded send+24; (4) source/Document and sparse absence remain unchanged. These are source/math findings, not executed test results. Parent and author have been notified; revised snapshot review remains pending.
## Revised immutable v2 review — original blockers addressed at source level
Verified all four v2 byte counts and SHA256 values against .local/scenario-resolver-review-snapshot-20260913-v2/manifest.json. The22 #[test] definitions exactly match its ordered test-name manifest. Reviewed the correction deltas and new or strengthened numeric regressions. No remaining verified blocker in this bounded correction pass. Parent-owned compiler and runtime gates remain pending.
- Exact lane identity: Scene.render_lanes retains the builder map (behavior.rs:1340); resolver coincidence checks/output use it directly (scenario_layout.rs:605,775). Tests927–956 independently assert .1 bits and distinct0/1e−6 line endpoints, so header-center rounding can no longer create this false coincidence. Assigned-only Scene.lanes remains separate.
- Lifetime geometry losses: scenario_layout.rs:728–743 compares actual projector MissingGeometry (lifeline,scope) pairs against the baseline. render.rs:173–182 selects only that reason, retaining existing semantic uncertainties. This catches both prior successful-loss cases without changing projector traversal or adopting a blanket event-crossing policy. Tests1026–1073 establish baseline numeric rows, positively exercise missing geometry, require the named rejection, and retain feasible520 with the exact Delete631 marker. No empty-set success or expected-panic swallowing was found.
- Bounds origin: Scene.layout_origin carries layout_left, and structural_bounds seeds Rectangle(layout_origin,scene.size) at787–789. Tests check the independently expected−660..1140 and−1000160..1000240 envelopes while also requiring nonempty activation geometry containment. This resolves the duplicate negative-origin margin.
- Public send bits: resolver631–648 explicitly replaces pairs with exact authored send and a single derived receive. behavior.rs:1398–1402 applies these y endpoints after lifecycle x/marker alignment and before typed activation derivation. align_message_anchors187–202 sets both self-loop send points and both receive points. Tests1076–1134 cover decimal/tiny ordinary values and feasible selfCreate64.next_down(); tests1138–1221 require success for async90/Call100.1/Reply332, exact Plan/Line/activation-start bits, one activation and exact Reply finish. The former valid-layout rejection is addressed.
Test scope limitations: the new precision regression specifically exercises selfCreate; selfDelete uses the same production branch but has no separate next_down-bit test in this22 set. Existing integer self-lifecycle test covers both kinds. The feasible ALT control asserts marker and absence of MissingGeometry, not an independently enumerated full lifetime vector; other tests compare complete nonempty scene activation/lifetime state. Stored fixture helpers run actual validation and Document/source round trips when executed; that admission has not yet run here. No claim about native paint, new gesture policy, saved-view UI freshness or full desktop compatibility follows.