Files
ARCADIA-Studio/harness/experiments/placement-clearance-20260925/source-review.md
T

9.3 KiB
Raw Blame History

Independent placement-clearance source review

2026-09-25. Reviewed the initial working diff and then its settled, clean commit 2be8c7f2929684a99bfb92b63e1585980d6313cf against 2850b915, including the plan, routing extraction, diagnostic, flat repair, tests and example CLI. No edits, compilation, test execution, routing calls or GUI actions by this reviewer.

Verdict: no P1/P2 source blocker found for the declared bounded experiment. The extracted predicates appear behavior-preserving, and the flat repair's separation constraints preserve an initially disjoint axis for every node pair. Approval here is for verification of this candidate, not production adoption or a claim that lead clearance establishes routability. Parent's exact frozen-route comparison and17-case experiment remain necessary.

Findings and verification limits

P3 — semantic/coincident self-loop coverage is narrower than the requested diagnostic coverage. crates/oa-core/src/routing/leads.rs:142 verifies the coincident exception and owner-obstacle distinction using direct helper calls. It does not run lead_incompatibilities on an actual projected semantic self-loop or coincident authored attachment pair. The source reuses the real endpoint selector and reproduces the route-edge coincident condition, so this is a coverage gap, not an identified false result. If the parent claims these cases tested, add or reuse an actual Project/projection-level witness; otherwise preserve the limitation. Existing self-loop routing tests can establish routing regression coverage but not every new report field.

P3 — solver unit tests do not establish full repair/persistence behavior. crates/oa-core/examples/auto_layout_probe/clearance.rs:260 and :274 exercise one propagated inequality and an extent refusal. The17-case observer must verify final no-overlap identities, the exact sparse lead/warning/penetration changes, complete metadata/reset policy, and ten-call stability. Its emitted supported:true only means the repair/import returned successfully; no quality-pass follows. A duplicate-call failure currently appears as same_input_exact:false without its error text, sufficient to fail repeatability but less diagnostic than the ten-call error record.

No source change is requested solely to broaden this first experiment. Failed cases and the above limits must remain explicit in the report.

Router extraction

At routing.rs:425, effective_route retains actual-owner versus projected-owner promotion, suspension of the whole authored route when promoted, and semantic-port precedence over dormant explicit attachments. At :447, edge_endpoints calls the previous endpoints expressions in source/target order with the same semantic/projected-port fallback. These helpers are pure reads; source/target endpoint vectors have the same ordering.

At routing.rs:475 and :483, the allowed-pair and mandatory-lead expressions are the original expressions with the same Boolean logic. The route path still filters self-loop anchors before candidate scoring/sorting, and still checks the lead guard after best-cost pruning at :868. Pins, obstacle construction, preview reuse, search budget, costs and fallback are not changed by this extraction. Moving the two endpoint calls into a local array before route_edge does not introduce a mutation or reorder obstacle/pin processing. This is source-equivalence reasoning; only the pending recorded-route equality check establishes observed unchanged output.

Diagnostic fidelity

routing/leads.rs:67 constructs the same node/rectangle/header maps and edge-ID order as the router. It uses effective promotion/attachment policy, actual endpoint alternatives and edge_obstacles, including ancestor-header versus traversable-body treatment. Missing projected owners are skipped as in routing; empty diagnostics cannot certify that all semantic connections are present.

The diagnostic counts every permitted source/target pair and reports an incompatibility only when none passes the exact early lead guard (:100–118). It therefore does not promote one blocked automatic side to a hard incompatibility when another side remains usable. Coincident singleton tips use the same exception as route-edge selection. Pins, later stub-search containment, path reachability, global search budget and labels are deliberately outside this necessary-condition diagnostic.

describe at leads.rs:43 excludes an endpoint's own rectangle from lead blockers, matching the early guard. stub_containers deliberately includes all containing obstacles and is separately documented; consuming it as additional mandatory blockers would change the historical semantics. The repair correctly uses only blockers.

The integration tests meaningfully distinguish20-unit blocked clearance from30/31-unit clear geometry, assert independent tip/stub/inflated-obstacle coordinates, check read-only Project equality, semantic-port precedence over stored attachments, and an unconstrained alternative versus explicitly anchored non-port endpoint. The ancestor/promotion test at lead_feasibility.rs:182 is transparently a synthetic projected forest; it proves that supplied-view policy, not a complete authored folded-project lifecycle.

Monotone flat repair

clearance.rs:130 seeds one boundary inequality for every initially disjoint pair: X separation when available, otherwise Y. Initially overlapping rectangles are explicitly refused. Because positive extents make each chosen direction forward in that axis's frozen start-coordinate order, these original constraints are acyclic. Newly required lead gaps are accepted only in that same rank direction (:214).

solve at :27 visits targets in frozen order, so every constraint predecessor has already been resolved. It applies coordinate floors, nondecreasing adjacent starts, and start[to] >= start[from] + extent[from] + gap. Constraints only strengthen and dimensions are rechecked each round. Therefore, in the represented arithmetic, each pair retains at least its originally selected separating axis; strengthening a lead gap cannot remove that preserved pairwise separation. The recorded final overlap check remains appropriate for f32 effects and integration mistakes.

The31-unit required gap is PORT_STUB + CLEARANCE +1 at :217; it is deliberately conservative for a flat rectangular blocker in front of a cardinal singleton endpoint. Unexpected direction, degenerate lead, multiple endpoint choices, unknown owner/blocker or a constraint opposing frozen order returns an error. Reprojection after every adjustment observes automatic-port changes; it does not assume the initial tips remain attached to the same side. Up to four adjustments are allowed, followed by a final diagnosis; a still-blocked or unstrengthenable result fails explicitly.

Flat, complete, noncollapsed topology, finite positive geometry,256 nodes,1024 edges,100000-unit span and a cumulative work allowance are checked. There is no unbounded iterative optimizer. The work allowance is a coarse charge for bounded graph operations, not an exact CPU-operation counter or wall-clock guarantee; parent process supervision remains useful. Every failure drops the private clone.

Metadata and interpretation boundary

clearance::repair changes only positions in its clone. It does not incrementally rewrite stored routes or anchors while adjusting. Parent wrapper auto_layout_probe.rs:1027 invokes the audited import policy once from the original input to final projected geometry. That preserves authored sizes when projection minima are unchanged, actual attachments and other-view records; applies retained-route translation from the full final deltas; and clears only original eligible pins/full labels.

Even if no node movement was needed, the wrapper can still clear eligible pins/labels. Thus output differences on an original authored scene may include the declared reset, not solely clearance movement. Compare complete records and the appropriate reset control before attributing quality gains to gap repair. For scenes already reset this distinction disappears.

CLI results expose baseline/final geometry, lead reports, conservation, roundtrip, same-input equality and repeated repair outcomes, with explicit errors for unsupported cases. The accepted success criterion remains the plan's narrow one: remove the exact sparse terminal conflict without a new node overlap or routing failure. Empty lead reports alone cannot establish this, and even that compatibility result leaves shared runs, labels, area, compounds and native traceability unresolved.

Fixture correction addendum

Reviewed parent follow-up 074051261a0beee1047e2b5624c879deef7813bc: only five fixture lines change, converting the three DataType definitions to Record { supertype: None } before constructing the DataAssociation. The previous fixture relied on the default primitive definition, contrary to crates/oa-core/src/data.rs:1089 requiring Record endpoints. The original gate13 failure remains evidence of a fixture precondition defect, not a passing non-port test. This correction supplies valid intended association endpoints without changing geometry, assertions, production routing, diagnostic or repair logic. Test functions now start at107/144/167/187; earlier test line references refer to2be8c7f2. No rerun result is claimed here; gate14 is parent-owned. Static verdict is unchanged, now covering this exact correction.