fix(reconstruct): export_reconstruction fails on body_kind='compose_solid' with 'Reconstruction has no body primitive' when fallback fires #70

New issue

Closed

opened 2026-09-16 15:28:54 +00:00 by Mark · 1 comment

Mark commented

2026-09-16 15:28:54 +00:00

Owner

When synthesize_with_fallback finds no clustering strategy that passes
validation, the fallback path produces a legacy Reconstruction object
that's stuffed into body_params["solid"] (a name suggesting a build123d
Solid). reconstruct_to_build123d then hits the default return None
branch and the caller sees:

export_reconstruction failed: Reconstruction has no body primitive; cannot export

This happened for every methodology_*.ply sample in the post-Phase-5
verification — the compose engine produced 0 bodies for real fixtures, the
fallback fired, and every export call returned no STEP file.

Reproduction

from pc2cad import FitConfig, fit_primitives, load_point_cloud
from pc2cad_reconstruct import compose_body_with_features, export_reconstruction
pcd = load_point_cloud("samples/methodology_assembly.ply")
result = fit_primitives(pcd, FitConfig(seed=42, use_deterministic_ransac=True))
rec = compose_body_with_features(result, pcd, tolerance=0.005,
                                engine="compose")
export_reconstruction(rec, "/tmp/out.step", format="step")
# before fix: fails silently with the message above
# after commit 0cf5dfd: STEP file produced and roundtrips through OCP

Status

Commit 0cf5dfd on main fixed the export dispatch (recursion through
reconstruct_to_build123d when body_params["solid"] is itself a
Reconstruction). Verified by running methodology_assembly.ply through
the pipeline: produces a 366KB STEP file with 76 faces that
render-compare parses cleanly.

This issue remains open because:

The fix only handles the legacy-Reconstruction case. A true
build123d Solid on body_params["solid"] would short-circuit through
reconstruct_to_build123d only if it lacks a body_kind attribute,
which a real Solid doesn't. But that path was never tested with a
real Solid (compose_with_fallback always falls back to legacy on
real fixtures — see the fitter bug issue).
The error message is still produced in the legacy code path when
rec.body_kind == "empty" — the empty-body case has no body to export.
The current message confuses callers (it sounds like a programming
bug, not an expected empty-body outcome).

What's needed

Improve the empty-body error message: replace
"Reconstruction has no body primitive; cannot export" with
"Reconstruction body_kind='empty'; nothing to export" so callers
can distinguish empty input from the dispatch bug.
Add a test that exercises the fallback path with a fit_result that
produces 0 bodies (a synthetic primitive list that has only
profile_extrude primitives, which the compose engine doesn't
currently understand) and asserts the STEP file is still produced.
When the compose engine produces a real build123d Solid (the
post-fitter-bug-fixes future), add a test that body_params["solid"]
is that Solid and the export produces a STEP file containing the
analytic cylindrical/spherical faces.

Acceptance

Running methodology_assembly.ply through the full pipeline produces a
valid STEP file, render-compare reads it back with non-zero solids/faces,
and the error path (rec.body_kind == "empty") raises a clear
empty-body exception, not a confusing "no body primitive" message.

Context

Filed as the verification of commit 0cf5dfd. The fix is committed; the
remaining work is test coverage + error message clarity.

When `synthesize_with_fallback` finds no clustering strategy that passes validation, the fallback path produces a legacy `Reconstruction` object that's stuffed into `body_params["solid"]` (a name suggesting a build123d Solid). `reconstruct_to_build123d` then hits the default `return None` branch and the caller sees: export_reconstruction failed: Reconstruction has no body primitive; cannot export This happened for every methodology_*.ply sample in the post-Phase-5 verification — the compose engine produced 0 bodies for real fixtures, the fallback fired, and every export call returned no STEP file. ## Reproduction from pc2cad import FitConfig, fit_primitives, load_point_cloud from pc2cad_reconstruct import compose_body_with_features, export_reconstruction pcd = load_point_cloud("samples/methodology_assembly.ply") result = fit_primitives(pcd, FitConfig(seed=42, use_deterministic_ransac=True)) rec = compose_body_with_features(result, pcd, tolerance=0.005, engine="compose") export_reconstruction(rec, "/tmp/out.step", format="step") # before fix: fails silently with the message above # after commit 0cf5dfd: STEP file produced and roundtrips through OCP ## Status Commit 0cf5dfd on main fixed the export dispatch (recursion through reconstruct_to_build123d when body_params["solid"] is itself a Reconstruction). Verified by running methodology_assembly.ply through the pipeline: produces a 366KB STEP file with 76 faces that render-compare parses cleanly. This issue remains open because: 1. The fix only handles the legacy-Reconstruction case. A true build123d Solid on body_params["solid"] would short-circuit through `reconstruct_to_build123d` only if it lacks a body_kind attribute, which a real Solid doesn't. But that path was never tested with a real Solid (compose_with_fallback always falls back to legacy on real fixtures — see the fitter bug issue). 2. The error message is still produced in the legacy code path when rec.body_kind == "empty" — the empty-body case has no body to export. The current message confuses callers (it sounds like a programming bug, not an expected empty-body outcome). ## What's needed 1. Improve the empty-body error message: replace "Reconstruction has no body primitive; cannot export" with "Reconstruction body_kind='empty'; nothing to export" so callers can distinguish empty input from the dispatch bug. 2. Add a test that exercises the fallback path with a fit_result that produces 0 bodies (a synthetic primitive list that has only `profile_extrude` primitives, which the compose engine doesn't currently understand) and asserts the STEP file is still produced. 3. When the compose engine produces a real build123d Solid (the post-fitter-bug-fixes future), add a test that body_params["solid"] is that Solid and the export produces a STEP file containing the analytic cylindrical/spherical faces. ## Acceptance Running methodology_assembly.ply through the full pipeline produces a valid STEP file, render-compare reads it back with non-zero solids/faces, and the error path (rec.body_kind == "empty") raises a clear empty-body exception, not a confusing "no body primitive" message. ## Context Filed as the verification of commit 0cf5dfd. The fix is committed; the remaining work is test coverage + error message clarity.

Mark added the

bug

triage-approved

in-progress

labels

2026-09-16 15:28:54 +00:00

Mark removed the

in-progress

labels

2026-09-16 18:10:22 +00:00

Mark referenced this issue from a commit

2026-09-16 19:30:35 +00:00

fix(reconstruct): clear empty-body message + fallback path coverage (Mark/pc2cad#70)

Mark commented

2026-09-16 21:48:15 +00:00

Author

Owner

Verified by watcher after kilo-gsd pipeline halted in Bugfix phase timeout (Phase 9/9 retry-exhausted on unrelated flaky methodology_cone.ply / methodology_shaft.ply fixture tests — both fixed separately by commits bdc0369 / bd805bb on the #66 PR). The #70 deliverable landed cleanly:

Commits on main (origin/main):

d43b5ab fix(reconstruct): clear empty-body message + fallback path coverage (Mark/pc2cad#70)
(predates run) 0cf5dfd fix(reconstruct): handle body_kind='compose_solid' in export path

Acceptance criteria — all met:

Clear empty-body error message. export_reconstruction() now raises ValueError("Reconstruction body_kind='empty'; nothing to export") for the empty-body case (pc2cad_reconstruct.py:1839), distinguishing the expected empty outcome from the legacy "Reconstruction has no body primitive" message that sounded like a programming bug.
Fallback-path test. tests/test_reconstruction.py::test_compose_engine_fallback_produces_step_file exercises the compose-engine fallback with a synthetic FitResult carrying only profile_extrude primitives (which the compose engine doesn't understand) and asserts a valid STEP file is produced.
Dispatch fix (body_kind='compose_solid') was already on main as 0cf5dfd before this run — methodology_assembly.ply and methodology_lbracket_extruded.ply both roundtrip to STEP (366KB / 209KB) with valid face/edge counts.

Local verification:

$ python -m pytest tests/test_reconstruction.py::test_export_empty_body_raises_clear_message                     tests/test_reconstruction.py::test_compose_engine_fallback_produces_step_file -v
======================== 2 passed, 4 warnings in 2.49s =========================

Closing — watcher (not kilo-gsd, because the Bugfix phase timed out before the post-success close path executed). Pipeline halted in retry-exhausted Bugfix; no retry needed because the #70 deliverable was already committed.

Verified by watcher after kilo-gsd pipeline halted in Bugfix phase timeout (Phase 9/9 retry-exhausted on unrelated flaky `methodology_cone.ply` / `methodology_shaft.ply` fixture tests — both fixed separately by commits bdc0369 / bd805bb on the #66 PR). The #70 deliverable landed cleanly: **Commits on main (origin/main):** - `d43b5ab` fix(reconstruct): clear empty-body message + fallback path coverage (Mark/pc2cad#70) - (predates run) `0cf5dfd` fix(reconstruct): handle body_kind='compose_solid' in export path **Acceptance criteria — all met:** 1. **Clear empty-body error message.** `export_reconstruction()` now raises `ValueError("Reconstruction body_kind='empty'; nothing to export")` for the empty-body case (pc2cad_reconstruct.py:1839), distinguishing the expected empty outcome from the legacy "Reconstruction has no body primitive" message that sounded like a programming bug. 2. **Fallback-path test.** `tests/test_reconstruction.py::test_compose_engine_fallback_produces_step_file` exercises the compose-engine fallback with a synthetic FitResult carrying only `profile_extrude` primitives (which the compose engine doesn't understand) and asserts a valid STEP file is produced. 3. **Dispatch fix (`body_kind='compose_solid'`)** was already on main as `0cf5dfd` before this run — methodology_assembly.ply and methodology_lbracket_extruded.ply both roundtrip to STEP (366KB / 209KB) with valid face/edge counts. **Local verification:** ``` $ python -m pytest tests/test_reconstruction.py::test_export_empty_body_raises_clear_message tests/test_reconstruction.py::test_compose_engine_fallback_produces_step_file -v ======================== 2 passed, 4 warnings in 2.49s ========================= ``` Closing — watcher (not kilo-gsd, because the Bugfix phase timed out before the post-success close path executed). Pipeline halted in retry-exhausted Bugfix; no retry needed because the #70 deliverable was already committed.

Mark

2026-09-16 21:48:15 +00:00