Atomizer/hq/workspaces/shared/reviews/v2-migration-code-review.md

TASK: V2 Migration Code-Focused Review STATUS: complete CONFIDENCE: high

---

V2 Migration Master Plan — Code-Focused Review

Reviewer: Technical Lead Date: 2026-02-22 Reviewed: ATOMIZER-V2-MIGRATION-MASTERPLAN-V2.md (audit-fixed, ~71KB) Cross-referenced: V1 codebase at /home/papa/repos/Atomizer/optimization_engine/

---

1. V2 Package Architecture

Verdict: Good structure, two concerns.

The atomizer/ package layout is clean and well-organized. Module boundaries are logical. The rename from optimization_engine/ to atomizer/ with semantic subpackages (optimization/, processors/, contracts/, etc.) is an improvement.

Circular Dependency Risk — LOW

Actual V1 cross-module imports are well-layered:

• core/ → nx/: ✅ (runner imports NXSolver, model_cleanup)
• processors/surrogates/ → nx/: ✅ (generic_surrogate imports NXSolver)
• nx/ → core/: ❌ None found — clean dependency direction

The V2 structure preserves this. The one concern is the dual placement of surrogates:

• atomizer/processors/surrogates/ (9 files — the training/inference surrogates)
• atomizer/optimization/surrogates/ (5 files — GNN-based surrogates from gnn/)

This split is confusing. In V1, gnn/ imports from extractors and nx.solver, while processors/surrogates/ also imports from extractors and nx.solver. They're functionally similar but architecturally separate. Recommend: document the distinction clearly in __init__.py docstrings, or consolidate into one atomizer/surrogates/ package.

contracts/ Positioning

Placing contracts/ inside atomizer/ is correct — it should be importable by all other modules. No circular risk since contracts should be leaf nodes (no imports from other atomizer modules).

---

2. Import Graph Analysis

212 Python files in V1. Ran full import analysis.

Key Findings

V1 uses heavy lazy imports. Many cross-module imports are inside functions (e.g., from optimization_engine.nx.solver import NXSolver inside method bodies in core/base_runner.py, intake/processor.py, validation/gate.py, processors/surrogates/generic_surrogate.py). This is good — it means the actual import graph at module load time is lighter than it appears, and V2's restructuring won't cause import-time failures for optional dependencies.

The extractors/__init__.py is a mega-importer — it imports from 14 submodules. In V2, this should be preserved but it means from atomizer.extractors import * will pull in a lot. Not a problem, just be aware.

future/ imports cross into production code:

• config/capability_matcher.py imports from future.workflow_decomposer
• config/setup_wizard.py imports from extractor_orchestrator and inline_code_generator (which live in future/)

⚠️ This is a problem. The plan says future/ is "DO NOT MIGRATE," but config/ (migrating as atomizer/spec/) has runtime imports from future/. Either:

1. Port the specific future/ files that config/ depends on (workflow_decomposer.py, at minimum)
2. Or stub them out in V2's spec/capability_matcher.py and spec/setup_wizard.py

Recommendation: Check if these are behind try/except or conditional imports. If they're hard imports, this will break.

---

3. File Inventory Accuracy

Plan claims ~168 files. Actual count: 212.

The 44-file discrepancy breaks down:

• devloop/: 8 files (plan says 7)
• future/: 11 files (plan counts correctly but skips them)
• Test files scattered in modules: extractors/test_phase3_extractors.py, gnn/test_*.py (3), hooks/test_*.py (2)
• __init__.py files across 35 directories

Module-level spot-check:

Module	Plan Count	Actual Count	Match?
core/	9	9	✅
extractors/	28	28	✅
gnn/	11	11	✅
plugins/	22	21	⚠️ Off by 1
processors/	12	13	⚠️ Off by 1
hooks/	12	12	✅
nx/	10	10	✅
context/	7→8	8	⚠️ Plan says 7 in §3.2
config/	9	9	✅
interview/	7→8	8	⚠️ Plan says 7 in §3.12

The discrepancies are minor (likely __init__.py counting inconsistencies). The inventory is substantially accurate. The 168 vs 212 gap is mostly future/ + devloop/ + scattered test files + __init__.py files, all of which are accounted for in disposition.

---

4. Data Contracts

Verdict: Feasible but requires significant refactoring.

The plan introduces atomizer/contracts/ with AtomizerGeometry, AtomizerMesh, AtomizerBCs, AtomizerResults, AtomizerMaterial.

V1 has NO unified data model. Data flows through:

• Raw dictionaries (spec configs parsed from JSON)
• pyNastran BDF/OP2 objects (mesh/results)
• NX .prt binary files (geometry, parsed via NXParameterUpdater)
• NX .exp files (expression format)
• Optuna trial objects
• Custom dataclass-like objects in spec_models.py

The contracts are aspirational, not extractive — they don't map to existing V1 structures. This is fine as a V2 goal, but they should be P1, not P0. The P0 migration can work without contracts by keeping the existing data flow patterns. Trying to introduce contracts simultaneously with the migration adds unnecessary risk.

Recommendation: Make contracts/ a Phase 2 or later concern. Get the codebase ported first, then introduce contracts as an internal refactor.

---

5. Processor Pattern (NX Wrapping)

Verdict: The plan wisely abandoned the aggressive wrapping approach.

The audit-revised plan keeps atomizer/nx/ as a direct port of V1's optimization_engine/nx/ (10 files, same structure). This is the right call.

Why wrapping would be hard: NXSolver doesn't use NXOpen API directly — it shells out to NX Nastran via subprocess. The NX coupling is:

• File-system based (.sim, .prt, .fem files)
• Path-dependent (NX install directory auto-detection)
• Session-managed (NXSessionManager for concurrent access)
• Iteration-folder based (HEEDS-style model copies)

The hooks/nx_cad/ modules ARE tightly coupled to NXOpen (import NXOpen, NXOpen.Session.GetSession(), etc.) — these are NX journal scripts meant to run inside the NX process. They can't be abstracted.

The plan's processors/geometry/nx_geometry.py and processors/meshing/nx_mesher.py and processors/solvers/nastran_solver.py appear in the tree but have no V1 source. These are NEW files that would wrap atomizer/nx/. This is fine as future work but should not block migration.

---

6. utils/ Dependency Web

Result: Only 11 cross-module imports from utils/. This is NOT a crisis.

Breakdown:

• utils.logger.get_logger: 4 imports (core, study, processors)
• utils.codebase_analyzer: 2 imports (config, future)
• utils.realtime_tracking: 1 import (core)
• utils.trial_manager: 1 import (self-reference)
• utils.nx_file_discovery: 1 import (self-reference)
• utils.dashboard_db: 1 import (self-reference)
• utils.require_nx_or_exit: 1 import (self-reference)

The audit's concern about utils/ being "imported everywhere" was overstated. logger.py is the only truly cross-cutting utility. The rest are module-specific.

Recommendation: utils/ port order is correct (Phase 1, before core). No special handling needed. Consider whether nx_file_discovery.py and nx_session_manager.py belong in atomizer/nx/ rather than atomizer/utils/ — they're NX-specific.

---

7. Test Strategy

V1 tests/ lives at /home/papa/repos/Atomizer/tests/ (repo root), NOT inside optimization_engine/. The plan's "11MB tests" claim couldn't be verified at the optimization_engine/tests/ path (doesn't exist).

Actual test files found at repo root tests/:

• ~20 Python test files
• Mix of unit tests (tests/unit/) and integration tests
• Topics: context engineering, zernike, hooks, beam workflow, surrogate training, phase 3 integration
• Several inline test files inside modules: extractors/test_phase3_extractors.py, gnn/test_*.py (3 files), hooks/test_*.py (2 files)

Portability: The unit tests in tests/unit/ (surrogate training, adaptive characterization, neural surrogate) are portable — they test pure Python logic. Integration tests (NX workflows, beam optimization) need the NX environment and aren't portable without the solver.

Recommendation: Port tests/unit/ tests first. Integration tests wait for Phase 4 (dalidou testing).

---

8. Pythonic Quality — pyproject.toml

Issues found:

1. Build backend is wrong:

   build-backend = "setuptools.backends._legacy:_Backend"
   

   This is an internal/undocumented API. Should be:

   build-backend = "setuptools.build_meta"
   

2. numpy>=1.24,<2.0 is too restrictive. NumPy 2.0 has been out since June 2024. Many dependencies (scipy, pandas) now require or prefer NumPy 2.x. Change to numpy>=1.24 or numpy>=1.26,<3.0.

3. Missing [project.scripts] entry point. The CLI (atomizer/cli/main.py) should have:

   [project.scripts]
   atomizer = "atomizer.cli.main:main"
   

4. pyNastran in [nx] optional group — pyNastran is used by extractors (OP2 reading), not just NX. It should probably be a core dependency or in a nastran group, not nx.

5. click is listed as core dependency but the CLI is P2 priority. Should be in [project.optional-dependencies.cli] or accepted as core.

6. Missing [tool.setuptools.packages]:

   [tool.setuptools.packages.find]
   include = ["atomizer*"]
   

   Without this, setuptools might not find the package.

---

RESULT: Summary of Findings

🟢 GREEN (Will Work)

• Module boundary design is clean
• Import graph is well-layered, no circular dependency risk
• File inventory is substantially accurate (minor counting discrepancies)
• utils/ dependency web is manageable (11 imports, not a crisis)
• NX module direct-port approach is correct
• Lazy import pattern in V1 makes restructuring safer

🟡 YELLOW (Needs Attention Before Execution)

• future/ dependency from config/ — capability_matcher.py and setup_wizard.py import from future/ which is marked "DO NOT MIGRATE". Must resolve.
• pyproject.toml build backend is wrong — setuptools.backends._legacy:_Backend → setuptools.build_meta
• NumPy upper bound too restrictive — <2.0 blocks modern NumPy
• Missing [project.scripts] and [tool.setuptools.packages.find]
• Dual surrogates placement — processors/surrogates/ and optimization/surrogates/ needs clearer documentation or consolidation
• nx_file_discovery.py and nx_session_manager.py belong in atomizer/nx/, not atomizer/utils/

🔴 RED (Risk)

• contracts/ at P0 is premature — V1 has no unified data model. Introducing contracts during migration adds risk. Recommend P1/P2.
• Test path confusion — plan references optimization_engine/tests/ which doesn't exist. Tests are at repo root. Test strategy needs correction.

---

NOTES: Specific Code-Level Recommendations

1. Fix pyproject.toml before Phase 0:

   • build-backend = "setuptools.build_meta"
   • Add [tool.setuptools.packages.find] with include = ["atomizer*"]
   • Add [project.scripts] entry point
   • Change numpy to >=1.24 (remove <2.0)

2. Resolve future/ dependency in Phase 1:

   • Run: grep -n "from optimization_engine.future" /home/papa/repos/Atomizer/optimization_engine/config/*.py
   • If hard imports: port workflow_decomposer.py (and its WorkflowStep class)
   • If conditional: add try/except stubs

3. Move NX-specific utils during port:

   • utils/nx_file_discovery.py → atomizer/nx/file_discovery.py
   • utils/nx_session_manager.py → atomizer/nx/session_manager.py (note: there's ALSO nx/session_manager.py — check for duplication)

4. Defer contracts/ to post-migration refactor. Keep atomizer/contracts/ in the tree as empty stubs with docstrings, but don't force V1 code through them during migration.

5. Phase 3 import fix script should be automated:

   find atomizer/ -name "*.py" -exec sed -i 's/from optimization_engine\./from atomizer./g' {} +
   # Then manual fixups for renamed modules (core.runner → optimization.engine, etc.)
   

6. Add a make check-imports target that runs:

   python -c "import atomizer" && grep -r "optimization_engine\." atomizer/ --include="*.py" | grep -v _compat
   

Overall Assessment: The migration plan is solid and will work. The architecture is well-thought-out, the file inventory is thorough, and the phasing is correct. The issues identified above are all fixable before or during execution. The biggest risk is scope creep from trying to introduce new abstractions (contracts/, processor wrappers) simultaneously with the port.

Confidence: HIGH — This plan, with the fixes noted, will produce a working V2.