From 6c6ee6da03467fafc71a54a0caf3dbf4a3d51eb2 Mon Sep 17 00:00:00 2001 From: Lukas Gold Date: Wed, 23 Sep 2026 09:22:30 +0200 Subject: [PATCH 1/2] fix(core): return a list from load_entity and pair iris by title - LoadEntityResult.entities is now List[OswBaseModel]; pydantic v1 tried the Union's first member, so an empty result became a truthy OswBaseModel that is neither subscriptable nor sized - resolve() keys the loaded entities by full page title instead of zipping them against request.iris; load_entity skips pages it cannot build, which shifted every later iri onto the wrong entity - an iri with no matching entity is logged and left out of nodes - export_entity_jsonld drops the now dead isinstance branch that hid the empty result from its NotFound check Closes #198 --- src/osw/core.py | 19 +++-- src/osw/service/ops/entities.py | 2 - tests/test_load_entity_result_shape.py | 100 +++++++++++++++++++++++++ 3 files changed, 114 insertions(+), 7 deletions(-) create mode 100644 tests/test_load_entity_result_shape.py diff --git a/src/osw/core.py b/src/osw/core.py index ca8638b4..a25d5153 100644 --- a/src/osw/core.py +++ b/src/osw/core.py @@ -268,10 +268,19 @@ def resolve(self, request: ResolveParam): entities = osw_obj.load_entity( OSW.LoadEntityParam(titles=request.iris) ).entities - # create a dict with request.iris as keys and the loaded entities as values - # by iterating over both lists + # load_entity() skips pages it cannot build, so the returned + # list can be shorter than request.iris. Pair by full title + # instead of by position, so a skipped page does not shift + # every following entity onto the wrong iri. + entities_by_title = { + get_full_title(entity): entity for entity in entities + } nodes = {} - for iri, entity in zip(request.iris, entities): + for iri in request.iris: + entity = entities_by_title.get(iri) + if entity is None: + _logger.warning(f"Could not resolve iri '{iri}'") + continue nodes[iri] = entity return ResolveResult(nodes=nodes) @@ -1280,8 +1289,8 @@ def __init__(self, **data): class LoadEntityResult(BaseModel): """Result of load_entity()""" - entities: Union[model.OswBaseModel, List[model.OswBaseModel]] - """The dataclass instance(s)""" + entities: List[model.OswBaseModel] + """The list of dataclass instances""" # fmt: off @overload diff --git a/src/osw/service/ops/entities.py b/src/osw/service/ops/entities.py index 5ec2aee7..f34c71de 100644 --- a/src/osw/service/ops/entities.py +++ b/src/osw/service/ops/entities.py @@ -92,8 +92,6 @@ def export_entity_jsonld( OSW.LoadEntityParam(titles=[title], autofetch_schema=True) ) entities = result.entities - if not isinstance(entities, list): - entities = [entities] if not entities: raise errors.NotFound(f"Entity '{title}' not found.") export = ctx.osw.export_jsonld( diff --git a/tests/test_load_entity_result_shape.py b/tests/test_load_entity_result_shape.py new file mode 100644 index 00000000..9331ba45 --- /dev/null +++ b/tests/test_load_entity_result_shape.py @@ -0,0 +1,100 @@ +"""Unit tests for load_entity()'s result shape and resolve()'s iri pairing. + +Regression guard for #198: https://github.com/OpenSemanticLab/osw-python/issues/198 +LoadEntityResult.entities was declared as Union[OswBaseModel, List[OswBaseModel]], +so pydantic v1 tried the bare OswBaseModel variant first and an empty list +validated as OswBaseModel() instead of staying an empty list. Separately, +OswDefaultBackend.resolve() zipped request.iris against load_entity()'s result +positionally, so a page load_entity() skipped shifted every later iri onto the +wrong entity. + +These run fully offline: no site touches the network. +""" + +from unittest.mock import MagicMock + +from oold.backend.interface import ResolveParam + +import osw.core +import osw.model.entity as model +from osw.core import OSW +from osw.wtsite import WtSite + + +def _make_entity(namespace: str, title: str) -> model.Item: + entity = model.Item(label=[model.Label(text=title)]) + entity.meta = model.Meta(wiki_page=model.WikiPage(namespace=namespace, title=title)) + return entity + + +def _build_backend(monkeypatch): + """Constructs an OSW instance and returns its OswDefaultBackend. + + set_resolver/set_backend are replaced so construction does not touch the + real global oold registry; set_backend is used to capture the backend + instance it would otherwise register. + """ + captured = {} + + def _capture_set_backend(param): + captured["backend"] = param.backend + + monkeypatch.setattr(osw.core, "set_resolver", lambda param: None) + monkeypatch.setattr(osw.core, "set_backend", _capture_set_backend) + + site = MagicMock(spec=WtSite) + OSW(site=site) + + return captured["backend"] + + +def test_load_entity_result_keeps_empty_list_as_list(): + result = OSW.LoadEntityResult(entities=[]) + + assert result.entities == [] + assert bool(result.entities) is False + assert len(result.entities) == 0 + + +def test_load_entity_result_keeps_non_empty_list_and_subclass(): + item = model.Item(label=[model.Label(text="x")]) + + result = OSW.LoadEntityResult(entities=[item]) + + assert isinstance(result.entities, list) + assert len(result.entities) == 1 + assert isinstance(result.entities[0], model.Item) + + +def test_resolve_pairs_iris_by_title_not_position(monkeypatch): + backend = _build_backend(monkeypatch) + good = _make_entity("Item", "OSWAlignGood") + + def _stub_load_entity(self, param): + # "Item:OSWAlignBad" could not be built and load_entity() silently + # skips it, so the returned list is shorter than request.iris. + return OSW.LoadEntityResult(entities=[good]) + + monkeypatch.setattr(OSW, "load_entity", _stub_load_entity) + + result = backend.resolve( + ResolveParam(iris=["Item:OSWAlignBad", "Item:OSWAlignGood"]) + ) + + assert result.nodes == {"Item:OSWAlignGood": good} + assert "Item:OSWAlignBad" not in result.nodes + + +def test_resolve_returns_all_entities_when_every_iri_resolves(monkeypatch): + backend = _build_backend(monkeypatch) + first = _make_entity("Item", "OSWFirst") + second = _make_entity("Item", "OSWSecond") + + def _stub_load_entity(self, param): + return OSW.LoadEntityResult(entities=[first, second]) + + monkeypatch.setattr(OSW, "load_entity", _stub_load_entity) + + result = backend.resolve(ResolveParam(iris=["Item:OSWFirst", "Item:OSWSecond"])) + + assert result.nodes == {"Item:OSWFirst": first, "Item:OSWSecond": second} From 19130e64d6ed56527ef240d8eef901169211e680 Mon Sep 17 00:00:00 2001 From: Lukas Gold Date: Wed, 23 Sep 2026 10:37:46 +0200 Subject: [PATCH 2/2] fix(core): map an unresolved iri to None instead of omitting it oold types ResolveResult.nodes as Dict[str, Union[None, ...]] and indexes it by iri in three places without checking for the key, so omitting an unresolved iri turns a skipped page into a KeyError. --- src/osw/core.py | 4 +++- tests/test_load_entity_result_shape.py | 8 ++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/osw/core.py b/src/osw/core.py index a25d5153..fb77c2b6 100644 --- a/src/osw/core.py +++ b/src/osw/core.py @@ -280,7 +280,9 @@ def resolve(self, request: ResolveParam): entity = entities_by_title.get(iri) if entity is None: _logger.warning(f"Could not resolve iri '{iri}'") - continue + # ResolveResult.nodes is typed Dict[str, Union[None, ...]], + # and oold indexes it by iri without checking for the key, + # so an unresolved iri has to be present and None nodes[iri] = entity return ResolveResult(nodes=nodes) diff --git a/tests/test_load_entity_result_shape.py b/tests/test_load_entity_result_shape.py index 9331ba45..eef75495 100644 --- a/tests/test_load_entity_result_shape.py +++ b/tests/test_load_entity_result_shape.py @@ -81,8 +81,12 @@ def _stub_load_entity(self, param): ResolveParam(iris=["Item:OSWAlignBad", "Item:OSWAlignGood"]) ) - assert result.nodes == {"Item:OSWAlignGood": good} - assert "Item:OSWAlignBad" not in result.nodes + # ResolveResult validates its values, which copies the entity, so compare + # by equality rather than by identity + assert result.nodes["Item:OSWAlignGood"] == good + # oold indexes nodes by iri without checking for the key, and types the + # values as Union[None, ...], so an unresolved iri maps to None + assert result.nodes["Item:OSWAlignBad"] is None def test_resolve_returns_all_entities_when_every_iri_resolves(monkeypatch):