load_entity regenerates a class for a category that a packaged class already registers, and overwrites it in the type lookup #138

Activity

1. simontaurus commented on Aug 7, 2026

   

   simontaurus

   on Aug 7, 2026

   ContributorAuthor

   More actions

   Proposed fix

   Where it goes wrong

   core.py, in load_entity:

   cls_name: str = schema["title"]
   if not param.model_to_use:
       if not hasattr(model, cls_name):          # (1)
           if param.autofetch_schema:
               self.fetch_schema(...)            # compiles into osw.model.entity
       if not hasattr(model, cls_name):
           schemas_fetched = False
   ...
   elif len(schemas) == 1:
       cls = getattr(model, schemas[0]["title"])  # (2)
       entity = cls(**jsondata)

   (1) asks "does osw.model.entity have a class with this name", which is
   the wrong question twice over: it is keyed by class name rather than by
   category IRI, and it only ever looks in osw.model.entity, so a packaged
   class registered for the category is invisible. (2) then always takes the
   generated class, even when a packaged one exists.

   Patch

   Consult oold's registry by category IRI before deciding to compile:

   from oold.model import _types as _oold_types_v2
   from oold.model.v1 import _types as _oold_types_v1
   
   
   def _registered_model(category_iri: str):
       """Return a class already registered for this category IRI, if any.
   
       osw models are pydantic v1, so prefer that registry.
       """
       return _oold_types_v1.get(category_iri) or _oold_types_v2.get(category_iri)

   in the per-category loop:

   cls_by_category = {}
   for category in jsondata["type"]:
       schema = ...  # unchanged
       schemas.append(schema)
       cls_name = schema["title"]
       if param.model_to_use:
           continue
   
       registered = _registered_model(category)
       if registered is not None:
           cls_by_category[category] = registered
           continue                      # already have a class: do not compile
   
       if not hasattr(model, cls_name):
           if param.autofetch_schema:
               self.fetch_schema(...)
       if not hasattr(model, cls_name):
           schemas_fetched = False
           print(...)

   and at construction:

   elif len(schemas) == 1:
       category = jsondata["type"][0]
       cls = cls_by_category.get(category) or getattr(model, schemas[0]["title"])
       entity = cls(**jsondata)
   else:
       bases = [
           cls_by_category.get(category) or getattr(model, schema["title"])
           for category, schema in zip(jsondata["type"], schemas)
       ]

   Do not overwrite an existing registration

   Independently of the above, fetch_schema should refuse to take a
   registry slot that is already claimed by a different class, or at least
   log it. Silent last-one-wins is what makes this hard to find: the
   packaged class is imported, then a later autofetch replaces it, and
   nothing in the program reports that the class identity for a category
   changed.

   Two side benefits

   • One less page fetch per category, since the jsonschema slot only
     needs fetching when no class is registered. Worth doing, but keep it
     separate from the correctness fix, since schemas is also used to
     build the multiple-inheritance base list.
   • The name-keyed check goes away, so two categories whose schema titles
     happen to collide stop aliasing each other.

   Test

   Register a class for a category, then load_entity an instance of it
   with autofetch_schema=True, and assert
   type(entity) is the_registered_class and that
   osw.model.entity was not given a new class for it.
2. LukasGold

   added a commit that references this issue on Sep 22, 2026

   fix(core): prefer a registered class over a generated one

   ...

   This issue will close once commit 90fa310 is merged into the 'main' branch.

   90fa310