From b7e3c142d1a06b1c98caa534d0d944136be5a0ca Mon Sep 17 00:00:00 2001 From: Michael Zargham Date: Sat, 19 Sep 2026 14:10:30 -0400 Subject: [PATCH 1/2] fix: Element.attrs returns all values of a multivalued attribute attrs walked the element's triples assigning attrs[name] = value, so each repeated value overwrote the last. A caller reading an attribute declared multiple=True got one arbitrary value with no indication the others existed, and which one it got depended on triple-store iteration order. An attribute declared multiple=True now comes back as a sorted list of its values, even when it currently holds one, so the type does not depend on how many values happen to be recorded. Every other attribute is unchanged. Sorting is what makes repeated reads agree, the store having no inherent order of its own. Adds SchemaBuilder.multivalued_attributes(type_name), since describe_type returns repr strings and nothing else exposed the descriptors. Version 0.2.0 rather than 0.1.1: main already carries a breaking change since the 0.1.0 release, the core namespace having moved from https://example.org/kc# to https://w3id.org/kc#. A downstream SHACL constraint written against the wrong namespace matches nothing rather than failing, so every edge silently appears untyped. --- docs/tutorial.md | 13 +++++++++++++ knowledgecomplex/graph.py | 27 +++++++++++++++++++++++-- knowledgecomplex/schema.py | 40 ++++++++++++++++++++++++++++++++++++++ pyproject.toml | 2 +- tests/test_element.py | 40 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 119 insertions(+), 3 deletions(-) diff --git a/docs/tutorial.md b/docs/tutorial.md index 3433318..cfb96fa 100644 --- a/docs/tutorial.md +++ b/docs/tutorial.md @@ -101,6 +101,19 @@ elem = kc.element("req-001") elem.id # "req-001" elem.type # "requirement" elem.attrs # {"title": "Boot time < 5s"} +``` + +`attrs` follows the schema: an attribute declared `multiple=True` comes back as a sorted +list of its values — even when it currently holds only one — and every other attribute +comes back as a single string. The list is sorted because the triple store has no +inherent order, so repeated reads would otherwise be free to disagree. + +```python +sb.add_vertex_type("doc", attributes={"tag": text(multiple=True), "title": text()}) +kc.add_vertex("d1", type="doc", title="Guide", tag=["b", "a"]) + +kc.element("d1").attrs # {"title": "Guide", "tag": ["a", "b"]} +sb.multivalued_attributes("doc") # frozenset({"tag"}) kc.element_ids(type="test_case") # ["tc-001", "tc-002"] kc.elements(type="test_case") # [Element('tc-001', ...), Element('tc-002', ...)] diff --git a/knowledgecomplex/graph.py b/knowledgecomplex/graph.py index dbbad80..b344d5c 100644 --- a/knowledgecomplex/graph.py +++ b/knowledgecomplex/graph.py @@ -123,13 +123,36 @@ def uri(self) -> str | None: @property def attrs(self) -> dict[str, Any]: + """ + Attributes as the schema declares them. + + An attribute declared ``multiple=True`` comes back as a sorted list of its + values, even when it currently holds one; every other attribute comes back + as a single string. Sorting matters because the underlying triple store has + no inherent order, so without it repeated reads could disagree. + """ ns_str = self._kc._schema._base_iri + try: + multivalued = self._kc._schema.multivalued_attributes(self.type) + except Exception: + # An element whose type is unregistered or absent still has attributes + # worth reading; fall back to treating them all as single-valued. + multivalued = frozenset() + attrs: dict[str, Any] = {} for _, p, o in self._kc._instance_graph.triples((self._iri, None, None)): p_str = str(p) - if p_str.startswith(ns_str): - attr_name = p_str[len(ns_str):] + if not p_str.startswith(ns_str): + continue + attr_name = p_str[len(ns_str):] + if attr_name in multivalued: + attrs.setdefault(attr_name, []).append(str(o)) + else: attrs[attr_name] = str(o) + + for attr_name in multivalued: + if attr_name in attrs: + attrs[attr_name].sort() return attrs def compile(self) -> None: diff --git a/knowledgecomplex/schema.py b/knowledgecomplex/schema.py index 7b4d046..4e7bcf7 100644 --- a/knowledgecomplex/schema.py +++ b/knowledgecomplex/schema.py @@ -356,6 +356,46 @@ def _validate_parent(self, parent: str | None, expected_kind: str) -> None: f"expected '{expected_kind}'" ) + def multivalued_attributes(self, type_name: str) -> frozenset[str]: + """ + Names of the attributes on type_name that may hold more than one value. + + Includes inherited attributes. Callers need this to read an element back + faithfully: an attribute declared ``multiple=True`` is a set of values, and + collapsing it to one silently loses data. + + Parameters + ---------- + type_name : str + A registered type name. + + Returns + ------- + frozenset[str] + + Example + ------- + >>> sb = SchemaBuilder(namespace="demo") + >>> _ = sb.add_vertex_type("Doc", {"tag": text(multiple=True), "title": text()}) + >>> sorted(sb.multivalued_attributes("Doc")) + ['tag'] + """ + from knowledgecomplex.exceptions import SchemaError + if type_name not in self._types: + raise SchemaError(f"Type '{type_name}' is not registered") + + specs = { + **self._collect_inherited_attributes(type_name), + **self._types[type_name].get("attributes", {}), + } + names = set() + for name, spec in specs.items(): + if isinstance(spec, dict): + spec = spec.get("vocab") or spec.get("text") + if getattr(spec, "multiple", False): + names.add(name) + return frozenset(names) + def _collect_inherited_attributes(self, type_name: str) -> dict: """Walk the parent chain and collect all inherited attributes.""" inherited = {} diff --git a/pyproject.toml b/pyproject.toml index d1de699..6d5a123 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "knowledgecomplex" -version = "0.1.0" +version = "0.2.0" description = "Typed simplicial complexes backed by OWL, SHACL, and SPARQL" readme = "README.md" requires-python = ">=3.11" diff --git a/tests/test_element.py b/tests/test_element.py index 71f180e..2226bab 100644 --- a/tests/test_element.py +++ b/tests/test_element.py @@ -196,3 +196,43 @@ def test_element_attrs_correct(self, qa_kc): assert len(elems) == 1 assert elems[0].attrs["criteria"] == "Accuracy" assert elems[0].attrs["title"] == "Guide A" + + +class TestMultivaluedAttrs: + """An attribute declared multiple=True is a set of values, not one of them. + + Before this, Element.attrs overwrote each value with the next as it walked the + triples, so a caller got an arbitrary single value and no indication that the + others existed. + """ + + @pytest.fixture + def kc(self): + sb = SchemaBuilder(namespace="demo") + sb.add_vertex_type( + "Doc", + attributes={"tag": text(multiple=True), "title": text()}, + ) + kc = KnowledgeComplex(schema=sb) + kc.add_vertex("d1", type="Doc", title="Guide", tag=["b", "a", "c"]) + kc.add_vertex("d2", type="Doc", title="Note", tag=["only"]) + return kc + + def test_all_values_are_returned(self, kc): + assert kc.element("d1").attrs["tag"] == ["a", "b", "c"] + + def test_a_single_value_is_still_a_list(self, kc): + """The type does not depend on how many values happen to be recorded.""" + assert kc.element("d2").attrs["tag"] == ["only"] + + def test_single_valued_attributes_are_unchanged(self, kc): + assert kc.element("d1").attrs["title"] == "Guide" + + def test_repeated_reads_agree(self, kc): + """The triple store has no inherent order, so the list is sorted.""" + assert kc.element("d1").attrs == kc.element("d1").attrs + + def test_schema_reports_which_attributes_are_multivalued(self): + sb = SchemaBuilder(namespace="demo") + sb.add_vertex_type("Doc", attributes={"tag": text(multiple=True), "title": text()}) + assert sb.multivalued_attributes("Doc") == {"tag"} From f38214b0a919888ea01972a52461ba6a007e36ee Mon Sep 17 00:00:00 2001 From: Michael Zargham Date: Sat, 19 Sep 2026 14:39:28 -0400 Subject: [PATCH 2/2] review: narrow the exception in Element.attrs to the cases it expects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Catching Exception could mask a real failure inside multivalued_attributes and report it as "this type has no multivalued attributes", which would silently return the very collapsed values this change set out to fix. Only two errors are expected: ValueError from .type when the element carries no user type, and SchemaError when that type is not registered. Both mean the same thing — the schema cannot say which attributes are multivalued — and the attributes are still worth reading, so the fallback stands for those and for nothing else. Two tests, one for each side of the boundary: an element with no registered type still reads, and an unexpected error is not swallowed. --- knowledgecomplex/graph.py | 9 ++++++--- tests/test_element.py | 23 +++++++++++++++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/knowledgecomplex/graph.py b/knowledgecomplex/graph.py index b344d5c..b0812f0 100644 --- a/knowledgecomplex/graph.py +++ b/knowledgecomplex/graph.py @@ -134,9 +134,12 @@ def attrs(self) -> dict[str, Any]: ns_str = self._kc._schema._base_iri try: multivalued = self._kc._schema.multivalued_attributes(self.type) - except Exception: - # An element whose type is unregistered or absent still has attributes - # worth reading; fall back to treating them all as single-valued. + except (ValueError, SchemaError): + # Only two things are expected here: ValueError from .type when the + # element carries no user type, and SchemaError when that type is not + # registered. Either way its attributes are still worth reading, so + # fall back to treating them all as single-valued. Anything else is a + # real bug and should surface rather than be swallowed. multivalued = frozenset() attrs: dict[str, Any] = {} diff --git a/tests/test_element.py b/tests/test_element.py index 2226bab..48117c9 100644 --- a/tests/test_element.py +++ b/tests/test_element.py @@ -236,3 +236,26 @@ def test_schema_reports_which_attributes_are_multivalued(self): sb = SchemaBuilder(namespace="demo") sb.add_vertex_type("Doc", attributes={"tag": text(multiple=True), "title": text()}) assert sb.multivalued_attributes("Doc") == {"tag"} + + def test_an_element_with_no_registered_type_still_reads(self): + """The fallback the narrowed except clause exists for. + + An element whose type is absent or unregistered has attributes worth + reading; it simply cannot be told which of them are multivalued. + """ + sb = SchemaBuilder(namespace="demo") + sb.add_vertex_type("Doc", attributes={"tag": text(multiple=True)}) + kc = KnowledgeComplex(schema=sb) + kc.add_vertex("d1", type="Doc", tag=["a"]) + + stranger = Element(kc, "not-an-element") + assert stranger.attrs == {} + + def test_an_unexpected_error_is_not_swallowed(self, kc, monkeypatch): + """Narrow, not bare: a real bug in the schema lookup must surface.""" + def boom(_type_name): + raise RuntimeError("something genuinely broken") + + monkeypatch.setattr(kc._schema, "multivalued_attributes", boom) + with pytest.raises(RuntimeError, match="genuinely broken"): + _ = kc.element("d1").attrs