Update progress log with code review fix plan implementation

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: CodeReclaimers

progress-20260315.md

@@ -97,3 +97,53 @@ Ran the Inventor round-trip test suite (`tests/test_inventor_roundtrip.py`, 54 t
 - Consider adding early-binding COM support (`win32com.client.gencache.EnsureDispatch`) for better performance and type safety, though duck-typing works well enough for now.
 - The `_collect_owned_sketch_point_indices` method does O(n*m) comparisons; could be optimized if sketches get very large.
 - Spline round-trip currently uses fit points only (start/end match but interior points shift slightly due to Inventor's spline fitting). True B-spline control point export would need `SplineDefinition` API.
+
+---
+
+## Code Review Fix Plan Implementation
+
+### Summary
+
+Implemented the "Fix Plan: Code Defects and Spec Drift" across three commits corresponding to three phases.
+
+### Phase 1: Core library fixes (commit 7228199)
+
+**Files modified**: `morphe/validation.py`, `morphe/primitives.py`, `morphe/serialization.py`, `morphe/constraints.py`, `morphe/document.py`
+
+Changes:
+- **validation.py**: Added `_validate_ellipse()` and `_validate_elliptical_arc()` with dispatch in `_validate_primitive()`. Checks: non-finite center, positive radii, major >= minor, non-finite params, start_param != end_param.
+- **primitives.py**: Added `ValueError` guard in `Ellipse.eccentricity` and `Ellipse.focal_distance` when `minor_radius > major_radius` (previously crashed with opaque `math domain error`). Added `NotImplementedError` in `Spline.evaluate()` when `is_rational` is True (previously silently ignored weights).
+- **serialization.py**: `_parse_point()` now raises `ValueError` on truncated data instead of silently returning origin. `dict_to_primitive()` now requires mandatory geometry fields (start/end for Line, center/start_point/end_point for Arc, etc.) — raises `ValueError` if missing instead of fabricating geometry with defaults.
+- **constraints.py**: Changed `_generate_id()` from `uuid4()[:8]` (32 bits) to `uuid4()[:12]` (48 bits). Birthday collision threshold moves from ~65K to ~16M.
+- **document.py**: `add_primitive_with_id()` now validates that ID prefix matches primitive type (e.g., rejects assigning "A0" to a Line). `get_point()` now raises `NotImplementedError` for `PointType.ON_CURVE` instead of falling through to opaque `ValueError`.
+
+### Phase 2: Adapter fixes (commit 34f46f2)
+
+**Files modified**: `morphe/adapters/solidworks/adapter.py`, `morphe/adapters/freecad/adapter.py`, `morphe/adapters/fusion/adapter.py`, `morphe/adapters/inventor/adapter.py`
+
+Changes:
+- **SolidWorks `_add_concentric`**: Arc branch was reading `geom2['radius']`, `geom2['start_angle']`, `geom2['end_angle']` which don't exist in the stored geometry dict. Fixed to compute radius and angles from stored `center`, `start`, `end` points.
+- **FreeCAD `_add_tangent`**: Was using `pt[1]` (the connection point's vertex index) for both geometries. Fixed to use 3-arg Tangent form: identifies which geometry owns the connection point and passes `(geo_with_point, vertex, other_geo)`.
+- **FreeCAD/Fusion CCW**: `ccw = end_angle > start_angle` fails for arcs crossing 0/2π. Both FreeCAD's `ArcOfCircle` and Fusion's `SketchArc` parameterize sweeps as always CCW. Changed to `ccw = True`. Same fix for Fusion elliptical arcs.
+- **SolidWorks DOF**: All sketch points were counted as 2 DOF each, but most are segment endpoints already included in per-segment DOF (4 for line, 5 for arc). Fixed to subtract segment endpoint count before adding standalone point DOF.
+- **FreeCAD `_geo_to_prim_type`**: Silently defaulted unrecognized geometry to `Line`. Changed to raise `ValueError`.
+- **Inventor spline**: Import now logs a warning when importing non-fit splines (NURBS fidelity lost). Export now sets `is_fit_spline=True` since we read fit points.
+- **SolidWorks elliptical arc CCW**: Was hardcoded `ccw=True`. Now computes actual direction via cross product of (center→start) × (center→mid).
+- **SolidWorks constraint dedup**: Key `(rel_type, segment_id)` was too aggressive — dropped valid constraints of the same type on the same segment with different partners. Changed to `(rel_type, tuple(sorted(all_involved_ids)))`.
+
+### Phase 3: SPECIFICATION.md sync (commit a996afa)
+
+Changes:
+- Fixed `ELLIPTICAL_ARC` prefix from `"EA"` to `"e"` to match code.
+- Added 5 missing `CONSTRAINT_RULES` entries: COLLINEAR, DISTANCE_X, DISTANCE_Y, SYMMETRIC, MIDPOINT.
+- Added Ellipse/EllipticalArc to `_next_index` and `add_primitive` prefix map.
+- Added Ellipse/EllipticalArc branches to `primitive_to_dict`. Fixed `constraint_to_dict` to show conditional field inclusion (omit value/inferred/confidence when default).
+- Added MIDPOINT to EllipticalArc `get_valid_point_types`.
+- Added sketch name header, `[C]` construction marker, and Ellipse/EllipticalArc formatting to `_describe_primitive`.
+- Added reference validation to `add_constraint` (raises `KeyError` for non-existent elements).
+
+### Key Decisions
+
+- **FreeCAD/Fusion CCW = always True**: Rather than computing cross products (which would be more general), relied on the documented API behavior that both FreeCAD and Fusion parameterize arc sweeps as CCW. This is simpler and avoids depending on midpoint computation. The cross-product approach was used for SolidWorks elliptical arcs where the API convention is less clear.
+- **Tangent 3-arg form**: Chose FreeCAD's 3-argument `Tangent(geo, vertex, other_geo)` over the 4-argument form because it correctly establishes tangency at a specific point without requiring vertex enumeration on the other geometry. Falls back to 4-arg form when the connection point doesn't belong to either referenced geometry.
+- **Dedup key**: Changed from `(rel_type, segment_id)` to `(rel_type, tuple(sorted(all_involved_ids)))`. This allows multiple constraints of the same type on the same segment with different partners while still deduplicating the same constraint seen from multiple segments.