fix(export): address the review findings on the metadata pass-through

This commit is contained in:
Fabio Rovai
2026-08-21 14:43:02 +01:00
parent d06434ae31
commit 1a220da477
2 changed files with 223 additions and 15 deletions
+93 -10
View File
@@ -187,16 +187,53 @@ def _escape_literal(value: str) -> str:
def _escape_xml(value: str) -> str:
return value.replace("&", "&amp;").replace("<", "&lt;").replace(">", "&gt;")
"""Escape a string for either XML element text or an attribute value.
The quotes matter. This helper feeds `rdf:about`, `rdf:resource` and
`xmlns:` attribute values, which are delimited by double quotes, so a value
carrying one would close the attribute early and produce a document that
does not parse. Escaping them in element text as well is harmless and
means one helper cannot be used in the wrong place.
"""
return (
value.replace("&", "&amp;")
.replace("<", "&lt;")
.replace(">", "&gt;")
.replace('"', "&quot;")
.replace("'", "&apos;")
)
def _is_ncname(value: str) -> bool:
"""Whether a string can be an XML NCName, which is what RDF/XML requires.
Checked over the ASCII range rather than the full XML production: the
grammar also admits combining characters and extenders, so this is
deliberately conservative. It refuses names it could have accepted, and it
never accepts one that would produce a document a parser rejects. The
earlier check tested only that the first character was not a digit, which
let through every other way a local name can fail to be a name.
"""
if not value:
return False
if not (value[0].isascii() and (value[0].isalpha() or value[0] == "_")):
return False
return all(c.isascii() and (c.isalnum() or c in "._-") for c in value[1:])
def _split_iri(iri: str) -> Optional[tuple]:
"""Split an IRI into (namespace, local name) for RDF/XML's QName syntax."""
"""Split an IRI into (namespace, local name) for RDF/XML's QName syntax.
Returns None when no split yields a usable local name. RDF/XML is the only
serialization here that cannot write an arbitrary predicate IRI, so this is
the one place a term can be unrepresentable, and the caller reports it
rather than dropping it quietly.
"""
for sep in ("#", "/"):
index = iri.rfind(sep)
if index != -1 and index + 1 < len(iri):
local = iri[index + 1 :]
if local and not local[0].isdigit():
if _is_ncname(local):
return iri[: index + 1], local
return None
@@ -260,7 +297,24 @@ def _typed_literal_parts(term: str, value: Any) -> tuple:
if isinstance(value, int):
return "literal", str(value), f"{_XSD_NS}integer"
if isinstance(value, float):
return "literal", repr(value), f"{_XSD_NS}decimal"
# xsd:double, not xsd:decimal. `repr(1e-05)` is "1e-05" and
# `repr(float("nan"))` is "nan", and xsd:decimal admits neither the
# exponent form nor the special values, so typing a float as decimal
# produced lexicals a strict parser rejects. A Python float is an IEEE
# 754 double; xsd:double has legal lexicals for all of them, and it is
# also the honest claim, since nothing that arrived as a float was ever
# exact. `normalize_confidence` keeps xsd:decimal for confidence
# deliberately: that is a bounded score where exactness is meaningful
# and NaN is not a confidence at all.
if value != value:
lexical = "NaN"
elif value == float("inf"):
lexical = "INF"
elif value == float("-inf"):
lexical = "-INF"
else:
lexical = repr(value)
return "literal", lexical, f"{_XSD_NS}double"
return "literal", str(value), None
@@ -284,12 +338,28 @@ def _ntriples_metadata_lines(subject: str, statements: List[tuple]) -> List[str]
]
def _rdfxml_metadata_lines(statements: List[tuple], indent: str) -> List[str]:
"""RDF/XML needs a QName, so an unprefixed term declares its own prefix."""
def _rdfxml_metadata_lines(
statements: List[tuple], indent: str, logger: Any = None
) -> List[str]:
"""RDF/XML needs a QName, so an unprefixed term declares its own prefix.
A term with no QName form has no RDF/XML representation at all, and this is
the only serialization with that restriction. Skipping it quietly would
reintroduce, in one format, exactly the silent metadata loss this module
was changed to stop, so it is reported and the other three formats still
carry the statement in full.
"""
lines: List[str] = []
for position, (term, value) in enumerate(statements):
split = _split_iri(term)
if split is None:
if logger is not None:
logger.warning(
"Term %r has no QName form, so it cannot be written in "
"RDF/XML and was omitted from that serialization only. "
"Turtle, N-Triples and JSON-LD carry it in full.",
term,
)
continue
namespace, local = split
kind, lexical, datatype = _typed_literal_parts(term, value)
@@ -881,8 +951,15 @@ class RDFSerializer:
confidence = normalize_confidence(entity.get("confidence", 1.0))
# RDF/XML syntax: rdf:Description with rdf:about
lines.append(f' <rdf:Description rdf:about="{entity_id}">')
lines.append(f' <rdf:type rdf:resource="{entity_type}"/>')
# Attribute values are delimited by quotes, and both of these
# are caller input. Element text is left alone deliberately: that
# is #1098, and it is being fixed on its own path.
lines.append(
f' <rdf:Description rdf:about="{_escape_xml(str(entity_id))}">'
)
lines.append(
f' <rdf:type rdf:resource="{_escape_xml(str(entity_type))}"/>'
)
lines.append(f" <semantica:text>{text}</semantica:text>")
if confidence is None:
self.logger.warning(
@@ -900,6 +977,7 @@ class RDFSerializer:
entity.get("metadata"), metadata_terms, self.logger
),
" ",
self.logger,
)
)
lines.append(" </rdf:Description>")
@@ -913,8 +991,12 @@ class RDFSerializer:
rel_type = rel.get("type", "semantica:related_to")
# Relationship as property on source entity
lines.append(f' <rdf:Description rdf:about="{source_id}">')
lines.append(f' <{rel_type} rdf:resource="{target_id}"/>')
lines.append(
f' <rdf:Description rdf:about="{_escape_xml(str(source_id))}">'
)
lines.append(
f' <{rel_type} rdf:resource="{_escape_xml(str(target_id))}"/>'
)
lines.append(" </rdf:Description>")
lines.append("")
@@ -924,6 +1006,7 @@ class RDFSerializer:
rdf_data.get("metadata"), metadata_terms, self.logger
),
" ",
self.logger,
)
if graph_uri
else []
+130 -5
View File
@@ -98,7 +98,9 @@ def test_every_format_writes_the_same_metadata_triples():
for fmt in FORMATS:
g = _serialize(RDFSerializer(), fmt, GRAPH_WITH_METADATA)
per_format[fmt] = {
(p, o) for s, p, o in g if str(p).startswith(SEMANTICA_NS) and "numEntities" in str(p)
(p, o)
for s, p, o in g
if str(p).startswith(SEMANTICA_NS) and "numEntities" in str(p)
}
assert len(set(map(frozenset, per_format.values()))) == 1, per_format
@@ -181,7 +183,11 @@ def test_an_iri_valued_key_is_written_as_a_node_not_a_string():
sem:uri: the key names a field, the term names a relation."""
data = {
"entities": [
{"id": ENTITY_IRI, "text": "Acme", "metadata": {"uri": "https://example.org/db"}}
{
"id": ENTITY_IRI,
"text": "Acme",
"metadata": {"uri": "https://example.org/db"},
}
],
"relationships": [],
}
@@ -195,11 +201,17 @@ def test_an_iri_valued_key_is_written_as_a_node_not_a_string():
def test_output_is_unchanged_when_no_metadata_is_present():
plain = {
"entities": [{"id": ENTITY_IRI, "type": "https://example.org/Org", "text": "Acme"}],
"relationships": [{"source_id": ENTITY_IRI, "target_id": "https://example.org/e2"}],
"entities": [
{"id": ENTITY_IRI, "type": "https://example.org/Org", "text": "Acme"}
],
"relationships": [
{"source_id": ENTITY_IRI, "target_id": "https://example.org/e2"}
],
}
serializer = RDFSerializer()
assert serializer.serialize_to_turtle(plain) == serializer.serialize_to_turtle(plain)
assert serializer.serialize_to_turtle(plain) == serializer.serialize_to_turtle(
plain
)
g = _parse(serializer.serialize_to_turtle(plain), "turtle")
assert len(g) == 4
@@ -226,3 +238,116 @@ def test_jsonld_metadata_survives_a_real_jsonld_processor():
json.loads(raw) # must be valid JSON before it can be valid JSON-LD
g = _parse(raw, "json-ld")
assert (URIRef(ENTITY_IRI), NUM_ENTITIES, Literal(3)) in g
# --- Findings from the Qodo review of PR #1165 -----------------------------
@pytest.mark.parametrize("fmt", FORMATS)
@pytest.mark.parametrize(
"value", [1e-05, 1e300, 0.1, -0.0, float("nan"), float("inf"), float("-inf")]
)
def test_a_float_metadata_value_is_a_double_and_keeps_a_legal_lexical(fmt, value):
"""`repr()` of a float is not an xsd:decimal lexical.
`repr(1e-05)` is "1e-05" and `repr(float("nan"))` is "nan", neither of which
xsd:decimal admits, so typing a float as decimal produced RDF a strict
parser rejects. A Python float is an IEEE 754 double, xsd:double has legal
lexicals for the exponent form and for the three special values, and saying
double is also the honest claim: nothing here was ever exact.
"""
data = {
"entities": [
{"id": ENTITY_IRI, "text": "Acme", "metadata": {"num_entities": value}}
],
"relationships": [],
}
g = _serialize(RDFSerializer(), fmt, data)
objects = list(g.objects(URIRef(ENTITY_IRI), NUM_ENTITIES))
assert len(objects) == 1, f"{fmt}: {objects}"
(written,) = objects
assert written.datatype == XSD.double, written.datatype
parsed = written.toPython()
if value != value: # NaN
assert parsed != parsed
else:
assert parsed == value
def test_every_format_agrees_on_a_float_metadata_value():
per_format = {}
for fmt in FORMATS:
g = _serialize(
RDFSerializer(),
fmt,
{
"entities": [
{"id": ENTITY_IRI, "text": "A", "metadata": {"num_entities": 1e-05}}
],
"relationships": [],
},
)
per_format[fmt] = {(p, o) for s, p, o in g if p == NUM_ENTITIES}
assert len(set(map(frozenset, per_format.values()))) == 1, per_format
def test_a_term_rdfxml_cannot_name_is_refused_out_loud(caplog):
"""RDF/XML needs a QName, and the PR's whole point is no silent drops.
A term whose local part is not an XML NCName has no RDF/XML form at all.
Skipping it quietly reintroduces, in one format, exactly the loss this
change exists to stop.
"""
unnameable = "http://example.org/ns/123"
data = {
"entities": [
{"id": ENTITY_IRI, "text": "Acme", "metadata": {"reviewed_by": "fabio"}}
],
"relationships": [],
}
with caplog.at_level("WARNING"):
xml = RDFSerializer().serialize_to_rdfxml(
data, metadata_terms={"reviewed_by": unnameable}
)
_parse(xml, "xml") # must still be well-formed
messages = " ".join(r.getMessage() for r in caplog.records)
assert unnameable in messages
assert "RDF/XML" in messages
@pytest.mark.parametrize("fmt", ["turtle", "ntriples", "jsonld"])
def test_the_other_formats_still_carry_a_term_rdfxml_cannot_name(fmt):
"""Only RDF/XML has the QName restriction; the rest write the full IRI."""
unnameable = "http://example.org/ns/123"
data = {
"entities": [
{"id": ENTITY_IRI, "text": "Acme", "metadata": {"reviewed_by": "fabio"}}
],
"relationships": [],
}
g = _serialize(
RDFSerializer(), fmt, data, metadata_terms={"reviewed_by": unnameable}
)
assert (URIRef(ENTITY_IRI), URIRef(unnameable), Literal("fabio")) in g
def test_a_quote_in_an_attribute_value_cannot_break_the_document():
"""`_escape_xml` feeds attribute values, which are delimited by quotes.
Escaping only &, < and > leaves a caller-supplied value able to close the
attribute early and produce XML that does not parse.
"""
data = {
"entities": [
{
"id": 'https://example.org/e"1',
"text": "Acme",
"metadata": {"uri": 'https://example.org/db"x'},
}
],
"relationships": [],
}
xml = RDFSerializer().serialize_to_rdfxml(data)
from xml.dom.minidom import parseString
parseString(xml) # well-formedness is the assertion