fix(layout_optimizer): apply code review follow-ups

unilabos/layout_optimizer/tests/test_bugfixes_v2.py

@@ -1,7 +1,7 @@
 """Regression tests for V2 Stage 1 bugfixes.
 
 Covers:
-- Duplicate device ID stacking (uuid-based internal IDs)
+- Duplicate device ID stacking (catalog ID + #N internal IDs)
 - DE orientation preservation (prefer_orientation_mode constraint)
 - prefer_aligned auto-injection and adjustability
 - Preset switch reorientation
@@ -101,16 +101,18 @@ class TestDuplicateDeviceIDs:
                                            graduated=False)
         assert math.isinf(cost_binary)
 
-    def test_create_devices_uses_uuid(self):
-        """create_devices_from_list should use uuid as Device.id."""
+    def test_create_devices_uses_catalog_id_with_suffixes(self):
+        """create_devices_from_list should keep catalog IDs and suffix duplicates."""
         from ..device_catalog import create_devices_from_list
         specs = [
             {"id": "opentrons_liquid_handler", "uuid": "abc-123"},
             {"id": "opentrons_liquid_handler", "uuid": "def-456"},
         ]
         devices = create_devices_from_list(specs)
-        assert devices[0].id == "abc-123"
-        assert devices[1].id == "def-456"
+        assert devices[0].id == "opentrons_liquid_handler"
+        assert devices[1].id == "opentrons_liquid_handler#2"
+        assert devices[0].uuid == "abc-123"
+        assert devices[1].uuid == "def-456"
         # Both should have the same bbox from footprints
         assert devices[0].bbox == devices[1].bbox
 

unilabos/layout_optimizer/tests/test_device_catalog.py

@@ -181,6 +181,18 @@ class TestCreateDevicesFromList:
         devs = create_devices_from_list(specs)
         assert devs[0].bbox != (0.6, 0.4)  # 使用 footprints 中的真实尺寸
 
+    def test_duplicate_catalog_ids_use_suffixes_and_store_uuid(self):
+        specs = [
+            {"id": "opentrons_liquid_handler", "uuid": "u1"},
+            {"id": "opentrons_liquid_handler", "uuid": "u2"},
+        ]
+        devs = create_devices_from_list(specs)
+        assert [dev.id for dev in devs] == [
+            "opentrons_liquid_handler",
+            "opentrons_liquid_handler#2",
+        ]
+        assert [dev.uuid for dev in devs] == ["u1", "u2"]
+
 
 # ---------- server endpoint (需要 httpx) ----------
 

unilabos/layout_optimizer/tests/test_intent_interpreter.py

@@ -53,6 +53,8 @@ def test_close_together_priority_scales_weight():
     low = interpret_intents([Intent(intent="close_together", params={"devices": ["a", "b"], "priority": "low"})])
     high = interpret_intents([Intent(intent="close_together", params={"devices": ["a", "b"], "priority": "high"})])
     assert high.constraints[0].weight > low.constraints[0].weight
+    assert high.constraints[0].weight == pytest.approx(16.0)
+    assert low.constraints[0].weight == pytest.approx(0.5)
 
 
 def test_close_together_single_device_error():
@@ -124,6 +126,17 @@ def test_face_inward():
 def test_align_cardinal():
     result = interpret_intents([Intent(intent="align_cardinal")])
     assert result.constraints[0].rule_name == "prefer_aligned"
+    assert result.constraints[0].weight == pytest.approx(0.5)
+
+
+def test_keep_adjacent_generates_minimize_distance():
+    result = interpret_intents([Intent(intent="keep_adjacent", params={
+        "devices": ["opentrons_liquid_handler", "agilent_plateloc"],
+        "priority": "high",
+    })])
+    assert len(result.constraints) == 1
+    assert result.constraints[0].rule_name == "minimize_distance"
+    assert result.constraints[0].weight == pytest.approx(16.0)
 
 
 # --- min_spacing ---

unilabos/layout_optimizer/tests/test_interpret_api.py

@@ -103,7 +103,7 @@ def test_interpret_schema_returns_all_intents():
     data = resp.json()
     intents = data["intents"]
     expected = {
-        "reachable_by", "close_together", "far_apart",
+        "reachable_by", "close_together", "far_apart", "keep_adjacent",
         "max_distance", "min_distance", "min_spacing",
         "workflow_hint", "face_outward", "face_inward", "align_cardinal",
     }

unilabos/layout_optimizer/tests/test_optimizer.py

@@ -313,6 +313,96 @@ def test_optimize_endpoint_accepts_angle_granularity_with_pcr_fixture():
         )
 
 
+def test_optimize_endpoint_binary_reachability_uses_checker():
+    """Final binary evaluation should fail when hard reachability is impossible."""
+    resp = _post_app("/optimize", {
+        "devices": [
+            {"id": "arm_slider", "name": "Arm", "device_type": "articulation", "size": [1.0, 0.2]},
+            {"id": "opentrons_liquid_handler", "name": "Target", "size": [0.6, 0.5]},
+        ],
+        "lab": {"width": 8.0, "depth": 4.0},
+        "constraints": [
+            {
+                "type": "hard",
+                "rule_name": "reachability",
+                "params": {
+                    "arm_id": "arm_slider",
+                    "target_device_id": "opentrons_liquid_handler",
+                },
+                "weight": 1.0,
+            }
+        ],
+        "seeder": "row_fallback",
+        "run_de": False,
+        "arm_reach": {"arm_slider": 0.0},
+    })
+
+    assert resp.status_code == 200
+    data = resp.json()
+    assert data["success"] is False
+    assert data["cost"] is None or not math.isfinite(data["cost"])
+
+
+def test_expand_constraints_for_duplicates_fans_out_bare_catalog_ids():
+    """Bare catalog IDs should expand to all duplicate instances."""
+    from ..device_catalog import create_devices_from_list
+    from ..server import _expand_constraints_for_duplicates
+
+    devices = create_devices_from_list([
+        {"id": "opentrons_liquid_handler", "uuid": "u1"},
+        {"id": "opentrons_liquid_handler", "uuid": "u2"},
+        {"id": "arm_slider", "uuid": "arm-u1"},
+    ])
+    constraints = [
+        Constraint(
+            type="hard",
+            rule_name="reachability",
+            params={"arm_id": "arm_slider", "target_device_id": "opentrons_liquid_handler"},
+        )
+    ]
+
+    expanded = _expand_constraints_for_duplicates(constraints, devices)
+
+    assert len(expanded) == 2
+    assert {c.params["target_device_id"] for c in expanded} == {
+        "opentrons_liquid_handler",
+        "opentrons_liquid_handler#2",
+    }
+    assert all(c.params["arm_id"] == "arm_slider" for c in expanded)
+
+
+def test_maybe_add_prefer_aligned_constraint_skips_existing():
+    """Explicit prefer_aligned should win over server auto-injection."""
+    from ..server import _maybe_add_prefer_aligned_constraint
+
+    constraints = [Constraint(type="soft", rule_name="prefer_aligned", weight=0.5)]
+    result = _maybe_add_prefer_aligned_constraint(constraints, 3.0)
+
+    assert result is constraints
+    assert [c.weight for c in result if c.rule_name == "prefer_aligned"] == [0.5]
+
+
+def test_optimize_endpoint_duplicate_instances_return_catalog_id_and_unique_uuid():
+    """Duplicate catalog devices should round-trip with shared catalog ID and distinct UUIDs."""
+    resp = _post_app("/optimize", {
+        "devices": [
+            {"id": "opentrons_liquid_handler", "uuid": "u1"},
+            {"id": "opentrons_liquid_handler", "uuid": "u2"},
+        ],
+        "lab": {"width": 3.0, "depth": 3.0},
+        "seeder": "row_fallback",
+        "run_de": False,
+    })
+
+    assert resp.status_code == 200
+    placements = resp.json()["placements"]
+    assert [p["device_id"] for p in placements] == [
+        "opentrons_liquid_handler",
+        "opentrons_liquid_handler",
+    ]
+    assert {p["uuid"] for p in placements} == {"u1", "u2"}
+
+
 def test_full_pipeline_seed_only():
     """Full pipeline: seeder → snap_theta → correct count and bounds.