FXC-4111-Enable Tidy3dBaseModel.from_file() for various subclasses

[momchil-flex]

Greptile Overview

Greptile Summary

This PR refactors polymorphic deserialization by centralizing type registry and parsing logic in Tidy3dBaseModel, enabling automatic type resolution when loading files.

Key changes:

• Moved TYPE_TO_CLASS_MAP from Expression plugin to base class __init_subclass__()
• Added parse_obj() override to dispatch to correct subclass based on type field
• Simplified stub implementations by delegating to base class methods
• Removed 100+ lines of duplicate type mapping code

Critical issue found:

• Line 219 in tidy3d/components/base.py has a logical error that breaks the intended behavior. The condition compares a ModelField object to a string, which will always be False, causing parse_obj() to never use the polymorphic dispatch logic.

Minor issues:

• Debug print statement left in test file

Confidence Score: 1/5

• This PR has a critical logic bug that breaks core functionality
• The condition on line 219 of tidy3d/components/base.py contains a logic error that compares a ModelField object to a string literal "Tidy3dBaseModel", which will always evaluate to False. This means the polymorphic parse_obj() behavior will never be triggered when calling Tidy3dBaseModel.parse_obj(), breaking the core feature this PR is intended to add. The fix is simple (change to cls.__name__ == "Tidy3dBaseModel"), but this is a critical functional bug that needs to be resolved before merging.
• tidy3d/components/base.py requires immediate attention due to the critical logic bug on line 219

Important Files Changed

File Analysis

Filename	Score	Overview
tidy3d/components/base.py	2/5	Added TYPE_TO_CLASS_MAP registry and parse_obj() override with critical bug in line 219 condition check
tidy3d/web/api/tidy3d_stub.py	4/5	Simplified stub implementation by delegating to Tidy3dBaseModel.from_file(). Clean refactoring with minor variable naming issue
tests/test_web/test_webapi.py	3/5	Added debug print statement that should be removed before merging

Sequence Diagram

sequenceDiagram
    participant User
    participant Tidy3dBaseModel
    participant TYPE_TO_CLASS_MAP
    participant Subclass as Simulation/HeatSimulation/etc

    Note over Tidy3dBaseModel,Subclass: Registration Phase (at import time)
    Subclass->>Tidy3dBaseModel: __init_subclass__()
    Tidy3dBaseModel->>Tidy3dBaseModel: add_type_field()
    Tidy3dBaseModel->>Tidy3dBaseModel: generate_docstring()
    Tidy3dBaseModel->>TYPE_TO_CLASS_MAP: Register type -> class mapping
    Note over TYPE_TO_CLASS_MAP: {"Simulation": Simulation,<br/>"HeatSimulation": HeatSimulation, ...}

    Note over User,Subclass: Loading Phase (from_file)
    User->>Tidy3dBaseModel: from_file("sim.json")
    Tidy3dBaseModel->>Tidy3dBaseModel: dict_from_file("sim.json")
    Tidy3dBaseModel->>Tidy3dBaseModel: parse_obj({"type": "HeatSimulation", ...})
    Note over Tidy3dBaseModel: BUG: Line 219 condition always False
    Tidy3dBaseModel->>TYPE_TO_CLASS_MAP: Get class for "HeatSimulation"
    TYPE_TO_CLASS_MAP-->>Tidy3dBaseModel: HeatSimulation class
    Tidy3dBaseModel->>Subclass: HeatSimulation(**obj)
    Subclass-->>User: HeatSimulation instance

Loading

[Copilot]

Pull Request Overview

This PR refactors the class loading mechanism by consolidating type-to-class mapping logic from individual classes (like Expression and stub classes) into the base Tidy3dBaseModel class. This allows various simulation and data types to be loaded directly using Tidy3dBaseModel.from_file() without requiring class-specific implementations.

Key changes:

• Moved TYPE_TO_CLASS_MAP dictionary and parse_obj method from Expression to Tidy3dBaseModel
• Simplified Tidy3dStub.from_file() and Tidy3dStubData.from_file() to delegate to base class
• Updated documentation strings to use generic type references

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
tidy3d/components/base.py	Added global TYPE_TO_CLASS_MAP and parse_obj method to enable polymorphic loading of all subclasses
tidy3d/plugins/expressions/base.py	Removed duplicate TYPE_TO_CLASS_MAP and parse_obj implementation now handled by base class
tidy3d/web/api/tidy3d_stub.py	Simplified stub loading methods to delegate to Tidy3dBaseModel.from_file() and updated docstrings
CHANGELOG.md	Added entry documenting the new functionality

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[greptile-apps]

_{4 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[yaugenst-flex]

@marcorudolphflex could you have a look at the failing lazy loading test 🙏

[momchil-flex]

@marcorudolphflex could you have a look at the failing lazy loading test 🙏

I have this print statement about the types currently. In both develop and this branch I get
[<class 'tidy3d.components.base._make_lazy_proxy.<locals>._LazyProxy'>, <class 'dict'>, <class 'tuple'>]
but somehow on this branch the lazy proxy is not manifested as SimulationData in the isinstance check.

[greptile-apps]

_{5 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[momchil-flex]

I guess because of this change the data[0].class.name is now 'Tidy3dBaseModelProxy'. Will wait to hear from Marco before digging through more.

[marcorudolphflex]

If I catch that correctly, the fundamental issue is that it tries to create a Tidy3DBaseModel instead of the "correct" simulation class.
This fails for me without lazy loading:

from tests.test_web.test_tidy3d_stub import make_sim_data
from tidy3d.web.api.tidy3d_stub import Tidy3dStubData

tmp_path = "test.hdf5"
sim_data = make_sim_data()
sim_data.to_file(tmp_path)
Tidy3dStubData.postprocess(tmp_path, lazy=False)

pydantic.v1.error_wrappers.ValidationError: 5 validation errors for Tidy3dBaseModel
data
  extra fields not permitted (type=value_error.extra)
diverged
  extra fields not permitted (type=value_error.extra)
log
  extra fields not permitted (type=value_error.extra)
simulation
  extra fields not permitted (type=value_error.extra)
type
  extra fields not permitted (type=value_error.extra)

[momchil-flex]

Sorry @marcorudolphflex there was a bug in that state you tried. Now the test failures are exclusively due to lazy loading.

[marcorudolphflex]

Sorry @marcorudolphflex there was a bug in that state you tried. Now the test failures are exclusively due to lazy loading.

No problem.

So the issue with lazy loading is the following: In the current state of Tidy3dStubData.from_file, we cannot detect the individual class when creating the lazy object as we do not look into the file when working lazy. Therefore, we simply cannot inherit from the original class which let isinstance checks fail. The type name itself should not be really critical, that is more a gimmick when you print the type - but it is failing for the same reason.

Now the question is if we want to support isinstance on the simulation data type which I would generally tend to and it looks like we do some checks like that currently.

So I see 2 alternative solutions:

• check the json for the type as before - at least in the lazy loading case
• materialize on isinstance checks (delete __class__ from list of non-materialize-attributes)

What do you think?

[marcorudolphflex]

Edit:

materialize on isinstance checks (delete class from list of non-materialize-attributes)

this option just does not work in general. The object would still be a Tidy3DBaseModel after materialization such that it would be not usable. I would prefer the other option which seems more stable.

[momchil-flex]

Yes if the first option works it sounds better, are you going to give it a try?

[marcorudolphflex]

I can try to do that, but actually it would be better to do that on the pydantic v2 branch as we already observed varied behavior there.
Maybe we should postpone that if it does not work out of the box? Will post an update tomorrow.

[momchil-flex]

Thanks, sounds good, this is not exactly urgent it's just a nice cleanup.

[momchil-flex]

Cleaned up and rebased, @yaugenst-flex you wanna approve?

[github-actions]

Diff Coverage

Diff: origin/develop...HEAD, staged and unstaged changes

• tidy3d/components/base.py (87.5%): Missing lines 207,210-211,213,220-221,236,573
• tidy3d/plugins/expressions/base.py (100%)
• tidy3d/web/api/tidy3d_stub.py (100%)

Summary

• Total: 71 lines
• Missing: 8 lines
• Coverage: 88%

tidy3d/components/base.py

Lines 203-217

  203     @classmethod
  204     def _get_type_value(cls, obj: dict[str, Any]) -> str:
  205         """Return the type tag from a raw dictionary."""
  206         if not isinstance(obj, dict):
! 207             raise TypeError("Input must be a dict")
  208         try:
  209             type_value = obj[TYPE_TAG_STR]
! 210         except KeyError as exc:
! 211             raise ValueError(f'Missing "{TYPE_TAG_STR}" in data') from exc
  212         if not isinstance(type_value, str) or not type_value:
! 213             raise ValueError(f'Invalid "{TYPE_TAG_STR}" value: {type_value!r}')
  214         return type_value
  215 
  216     @classmethod
  217     def _get_registered_class(cls, type_value: str) -> type[Tidy3dBaseModel]:

---

Lines 216-225

  216     @classmethod
  217     def _get_registered_class(cls, type_value: str) -> type[Tidy3dBaseModel]:
  218         try:
  219             return TYPE_TO_CLASS_MAP[type_value]
! 220         except KeyError as exc:
! 221             raise ValueError(f"Unknown type: {type_value}") from exc
  222 
  223     @classmethod
  224     def _should_dispatch_to(cls, target_cls: type[Tidy3dBaseModel]) -> bool:
  225         """Return True if ``cls`` allows auto-dispatch to ``target_cls``."""

---

Lines 232-240

  232         target_cls = cls._get_registered_class(type_value)
  233         if cls._should_dispatch_to(target_cls):
  234             return target_cls
  235         if target_cls is cls:
! 236             return cls
  237         raise ValueError(
  238             f'Cannot parse type "{type_value}" using {cls.__name__}; expected subclass of {cls.__name__}.'
  239         )

---

Lines 569-577

  569         -------
  570         >>> simulation = Simulation.from_json(fname='folder/sim.json') # doctest: +SKIP
  571         """
  572         model_dict = cls.dict_from_json(fname=fname)
! 573         return cls._parse_model_dict(model_dict, **parse_obj_kwargs)
  574 
  575     @classmethod
  576     def dict_from_json(cls, fname: PathLike) -> dict:
  577         """Load dictionary of the model from a .json file.

---

[yaugenst-flex]

LGTM, minor test nit