From 5579851208ae5adbc813c787be8c8581d8bd2aed Mon Sep 17 00:00:00 2001 From: pravit-amp <43916793+pravit-amp@users.noreply.github.com> Date: Sat, 15 Aug 2026 09:40:04 -0700 Subject: [PATCH] fix(export): harden YAML export input handling (#958) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor(export): centralize graph-payload key normalization Graph payloads circulate under two vocabularies, entities/relationships and nodes/edges, and consumers each reconciled them locally with competing idioms. The same payload could be exported, silently dropped, or rejected depending on which consumer read it. Add normalize_graph_payload() to utils.helpers as the single place that decision is made. Both spellings present with one empty resolves to the populated one, which is the shape JSONExporter emits; both non-empty and different is refused, since there is no basis to prefer either and picking one would silently discard the other; a non-empty mapping with no recognized key raises rather than returning empty collections, with require_recognized=False for callers that should degrade. Adopt it in the three exporters that genuinely alias. LPGExporter read nodes with entities as the default, so it dropped every entity when nodes was present but empty, losing everything on a JSON round-trip. ArangoAQLExporter had the same idiom plus a manual fallback. Neo4jCSVExporter routes its mapping branch through the shared resolver so the reference implementation cannot drift; its attribute branch stays local, since objects are not mappings. Also feed LPGExporter._generate_indexes the resolved entities. It read entities directly, so a nodes/edges payload produced no indexes even once node generation was fixed. CSVExporter and JSONExporter are deliberately excluded: they write entities, relationships, nodes and edges as separate outputs by design rather than reconciling two spellings of one collection, so normalizing there would rename output files. * fix(export): reject non-mapping input to the YAML exporters export_yaml declared Union[Dict[str, Any], List[Dict[str, Any]]], but both YAML exporters read their payload by key, so a list reached .get() and surfaced as a bare AttributeError from inside the exporter, naming neither the offending argument nor the shape expected. Reject rather than wrap. These formats distinguish entities from relationships from triplets, so inferring which collection a bare list represents would silently mislabel the records, and wrapping it under an unrecognised key would write a structurally valid file with every collection empty - trading a loud failure for silent data loss. Validate in the exporters, matching the existing precedent in Neo4jCSVExporter._normalize_graph, so direct users of the classes get the same contract as callers of the convenience wrapper. Narrow the wrapper type hint to Dict[str, Any] to match. * fix(export): address YAML exporter review findings - semantica/export/yaml_exporter.py — import Sequence from typing instead of collections.abc. `Sequence[str]` in _require_mapping's annotation is evaluated at function-definition time; collections.abc.Sequence only became subscriptable in Python 3.9, so on the 3.8 this project declares support for, importing this module raised TypeError. typing.Sequence has supported subscripting since 3.5.3. Mapping stays imported from collections.abc since it's only used for isinstance. - tests/export/test_yaml_exporter_input_validation.py — clean up each test's tempfile.mkdtemp() dir via addCleanup instead of leaking it, and read exported YAML through a context manager instead of an unclosed yaml.safe_load(open(...)). * fix(export): reject YAML export payloads with no recognized key Both YAML exporters built their output from a fixed set of `.get(key, [])` lookups, so a mapping keyed by anything else serialized to a structurally valid file with every collection empty. Nothing signalled the loss: no exception, no warning, and the progress log reported a completed export. The only way to notice was to open the file. The realistic trigger is re-exporting an `export_json` payload, whose `{"data", "count", "metadata"}` envelope drops every record. - SemanticNetworkYAMLExporter.export_semantic_network now resolves its collections through normalize_graph_payload(), which raises rather than returning empty collections for an unrecognized mapping. Adopting the shared resolver rather than repeating the check locally also brings the 'nodes'/'edges' aliases, so ContextGraph.to_dict() — the most direct path from this library's own graph type to YAML, used in examples/capability_gap_context_graphs_example.py — exports its records instead of an empty file. - export_for_pipeline built its nested semantic network from the same defaulted lookups and had the same defect; it goes through the resolver too. - YAMLSchemaExporter.export_ontology_schema gets the equivalent check over its own key set. Schemas are a separate vocabulary with no aliasing, so _require_recognized_keys lives in this module rather than in the shared graph resolver. - 'metadata' is deliberately not sufficient to make a payload recognized. An export_json envelope carries one, so accepting it would readmit the case this fix is most likely to be needed for. - An empty mapping is still exported: an empty graph is legitimate and has no records to lose. - SemanticNetworkYAMLExporter.export() serializes before creating the output directory, so a rejected export leaves nothing behind. The two rejections keep distinct exception types, following what the codebase already does: a payload of the wrong *type* cannot be exported at all and raises ProcessingError, matching Neo4jCSVExporter._normalize_graph; a mapping whose *contents* are unusable raises ValidationError, matching normalize_graph_payload. _require_mapping therefore runs first at every entry point, so a non-mapping never reaches the resolver. Docstring Raises sections, export_usage.md and docs/reference/export.md record the accepted input shapes and both failures. Closes #953. * fix(export): reject payloads whose records resolve to nothing Addresses the Qodo findings on #958. Presence-only recognition (finding 1): checking that a recognized key is present answered "did the caller use our vocabulary" when the question that matters is "did anything the caller supplied survive". A payload like {"entities": [], "data": [...records...]} cleared the check, resolved to empty, and dropped every record under 'data' -- the silent-empty export by a narrower route. - utils/helpers.py — split the check in two. _require_recognized_keys keeps the presence rule; _require_nothing_dropped runs after resolution and refuses a payload that resolved to nothing while an unread key still holds records. Only a non-empty list counts as evidence: ContextGraph.to_dict() always carries a populated 'statistics' dict, and an empty graph must stay exportable, so 'metadata', 'statistics' and 'count' are named as context rather than records. - export/yaml_exporter.py — the schema path had the same hole and now runs both checks through the shared helpers rather than its own copy, so the two vocabularies cannot drift apart in what counts as a silent-empty export. Progress reported success on a failed write (finding 3): export_semantic_ network stops its tracking as completed once serialization returns, but export() then creates the directory and writes the file. A failure there left the tracker showing a completed export with no output. - export/yaml_exporter.py — the serialization span now says it serialized, not that it exported, and export() opens its own span around the filesystem work that stops as failed on error. Nothing reports a completed export until the bytes are on disk. Finding 2 (export_yaml no longer accepts List[Dict]) is the intended resolution of #952 rather than a regression: wrapping a bare list under a guessed key is what would mislabel the records. The signature, docstring and PR description already record the narrowed contract. Tests cover both directions of each fix, including that an empty ContextGraph still exports and that a failing write is not reported as completed. * fix(export): validate collection values and make Neo4j mappings strict Two gaps at the boundary the shared normalizer is supposed to own. _resolve_collection() resolved on truthiness alone, so a recognized key could still hold something that is not a collection of records: {"entities": "abc"} normalized to three single-character "records", and {"entities": 42} surfaced as a raw TypeError from list() inside whichever exporter happened to read it, naming the exporter rather than the payload key at fault. Collection values are now validated before conversion -- strings, bytes, mappings, and non-iterable scalars are rejected by key name, and each element must be a mapping or an attribute-carrying object, the two record shapes the exporters actually read. None stays legal as an absent collection, the spelling a JSON round-trip produces for []; it cannot hide dropped records, since _require_nothing_dropped() still runs. Every spelling present is validated, not just the one that wins, so a malformed alias is not excused by a well-formed canonical key. Neo4jCSVExporter._normalize_graph() opted out of the recognized-key check for mappings, which left it able to turn {"data": [...]} into header-only CSVs indistinguishable from a genuinely empty graph -- the exact failure the rest of the change exists to prevent. Mapping payloads now go through normalize_graph_payload() on its default terms. The attribute path for graph objects is untouched. With no caller left opting out, the require_recognized flag is removed rather than kept as a way back into the silent-empty export. Regression tests cover the malformed values end to end through every export path that reads the normalizer, and assert the rejected Neo4j export writes no CSV files. * fix(export): close YAML schema and record validation gaps Fix 1 -- _require_usable_schema silent data loss (P1): _require_usable_schema() passed all values from _SCHEMA_KEYS into _require_nothing_dropped() as evidence that records survived. Scalar metadata fields such as version='1.0' and uri='http://...' are truthy strings, so any one of them caused _require_nothing_dropped() to return early and silently discard records stored under an unread key alongside them (e.g. {'version': '1.0', 'nodes': [{'id': 'c1'}]}). Fixed by building the resolved list from only non-empty list/tuple values of recognised schema keys. Fix 2 -- _is_record accepts modules and type objects (P2): _is_record() accepted any object with __dict__, which includes Python modules and class objects. Elements that passed _coerce_records then reached exporters and raised AttributeError (e.g. module 'math' has no attribute 'get') rather than a ValidationError at the validation boundary. Fixed by excluding types.ModuleType and type from the __dict__ branch while preserving support for all user-defined attribute-bearing record objects. Tests: 101 tests pass across tests/utils/test_normalize_graph_payload.py tests/export/test_yaml_exporter_key_recognition.py tests/export/test_yaml_exporter_input_validation.py tests/export/test_neo4j_csv_exporter.py * fix(export): close exception-type and record-shape gaps in normalize_graph_payload LPGExporter and ArangoAQLExporter called normalize_graph_payload() with no type guard, so non-mapping input raised ValidationError from inside the resolver while the YAML and Neo4j exporters raised ProcessingError for the identical mistake -- inconsistent with the exception-type contract this PR establishes. Both now use the shared _require_mapping() guard (moved from yaml_exporter.py into utils/helpers.py so all three can use it). Neo4jCSVExporter._normalize_graph checked isinstance(graph, dict), so a non-dict Mapping (MappingProxyType, ChainMap) fell through to the object-attribute branch and was rejected, even though the identical payload exported fine via the other three exporters. Now checks isinstance(graph, Mapping). normalize_graph_payload() accepts dataclass/attribute-bearing object records, but LPGExporter/ArangoAQLExporter call .get(...) directly on resolved entities -- an object-shaped record passed validation only to crash with a raw AttributeError once used, the exact failure this boundary exists to prevent. Records are now converted to plain dicts at the boundary (_coerce_records -> new _record_to_dict), so every consumer gets a uniform shape regardless of which reading the caller used. Two non-empty spellings of the same collection holding identical records in a different order were rejected as conflicting, since the check used plain list equality. Comparison is now an order-independent multiset of each record's canonical JSON form. * docs(changelog): add entry for #958 YAML export input hardening Documents the full arc of #958 -- the normalize_graph_payload() centralization, YAML input validation, both review rounds from @Sameer6305, and the exception-type/record-shape follow-up fixes -- plus closes #956, #952, #953. --------- Co-authored-by: Pravit Ampapathini Co-authored-by: Sameer Kadam Co-authored-by: KaifAhmad1 --- CHANGELOG.md | 16 + docs/reference/export.md | 13 + semantica/export/arango_aql_exporter.py | 24 +- semantica/export/export_usage.md | 68 +++ semantica/export/lpg_exporter.py | 40 +- semantica/export/methods.py | 22 +- semantica/export/neo4j_csv_exporter.py | 32 +- semantica/export/yaml_exporter.py | 195 ++++++- semantica/utils/__init__.py | 2 + semantica/utils/helpers.py | 373 +++++++++++++- tests/export/test_neo4j_csv_exporter.py | 57 ++ .../test_yaml_exporter_input_validation.py | 181 +++++++ .../test_yaml_exporter_key_recognition.py | 434 ++++++++++++++++ tests/utils/test_normalize_graph_payload.py | 487 ++++++++++++++++++ 14 files changed, 1897 insertions(+), 47 deletions(-) create mode 100644 tests/export/test_yaml_exporter_input_validation.py create mode 100644 tests/export/test_yaml_exporter_key_recognition.py create mode 100644 tests/utils/test_normalize_graph_payload.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 758fa407..a94fd882 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **`export_yaml` raised a raw `AttributeError` on list input, silently wrote empty exports for unrecognized dict keys, and graph payloads were reconciled differently by every exporter** (#958, closes #956, #952, #953) by @pravit-amp, reviewed by @Sameer6305 + - Graph payloads circulate under two vocabularies, `entities`/`relationships` and `nodes`/`edges`, and each exporter reconciled them locally with a different idiom — `LPGExporter` in particular dropped every entity whenever `nodes` was present but empty, the exact shape `JSONExporter` emits. A new `normalize_graph_payload()` in `utils/helpers.py` centralizes that decision once, adopted by `LPGExporter`, `ArangoAQLExporter`, `Neo4jCSVExporter`, and both YAML exporters; `ContextGraph.to_dict()` now round-trips through YAML correctly as a result + - `export_yaml(records, path)` on a bare list previously failed with `AttributeError` from inside the exporter; it and the other YAML methods now reject non-mapping input with an actionable `ProcessingError` naming the expected keys, since these formats distinguish entities/relationships/triplets and guessing which one a list represents would mislabel the records + - `export_yaml({"data": [...]}, path)` previously wrote a structurally valid file with every collection empty, no exception, no warning, and the progress log reporting a completed export. `export_semantic_network`, `export_for_pipeline`, and `export_ontology_schema` now raise `ValidationError` when the payload shares no recognized key with what the method reads, or resolves to nothing while an unread key still holds records — an empty mapping is still accepted, since a genuinely empty graph has no records to lose + - **Breaking**: the two cases above, plus a bare list, now raise instead of returning cleanly with data silently dropped or a raw `AttributeError` from exporter internals. Migration: pass records under a recognized key (`{"entities": [...]}` / `{"nodes": [...]}` for `semantic_network`, `{"classes": [...]}` for `schema`) + - **Fixed during review** (Qodo): progress tracking could report a completed export before the output directory existed or the file was written; `export()` now creates the directory and serializes before starting tracking, so a rejected export leaves nothing behind + - **Fixed during review** (@Sameer6305, round 1): `normalize_graph_payload()`'s collection resolver treated any truthy value as a collection — `{"entities": "abc"}` silently became three single-character records, `{"entities": 42}` leaked a raw `TypeError` from inside `list()`. Collection values are now validated before conversion, rejecting strings/bytes/mappings/non-iterable scalars by name. Separately, `Neo4jCSVExporter._normalize_graph` called the shared resolver with `require_recognized=False`, so it alone kept accepting an unrecognized mapping as a silent empty export; the opt-out (introduced earlier in this same PR, with no other caller) was removed + - **Fixed during review** (@Sameer6305, round 2): `YAMLSchemaExporter`'s usable-schema check could treat scalar schema metadata (`version`, `uri`, `title`, `description`) as evidence records had been exported, letting records under an unread key drop silently; and `_is_record()` accepted modules and class/type objects through the generic `__dict__` path, which would have reached exporter internals instead of failing at the boundary. Both closed, with regression coverage + - **Fixed during final maintainer review** (before merge): four more gaps in the shared boundary that the earlier rounds didn't reach + - `LPGExporter`/`ArangoAQLExporter` called `normalize_graph_payload()` with no type guard, so non-mapping input raised `ValidationError` from inside the resolver — while YAML and `Neo4jCSVExporter` raised `ProcessingError` for the identical mistake, per this PR's own stated contract. The `_require_mapping()` guard that already existed in `yaml_exporter.py` is now shared from `utils/helpers.py` and used by all three + - `Neo4jCSVExporter._normalize_graph` checked `isinstance(graph, dict)`, so a non-dict `Mapping` (`MappingProxyType`, `ChainMap`) fell through to the object-attribute branch and was rejected, even though the identical payload exported fine via `LPGExporter`/`ArangoAQLExporter`/YAML. Now checks `isinstance(graph, Mapping)` + - `normalize_graph_payload()` accepts dataclass and attribute-bearing object records (`Neo4jCSVExporter._record_to_dict` reads them), but `LPGExporter`/`ArangoAQLExporter` call `.get(...)` directly on resolved entities — an object-shaped record passed validation only to crash with a raw `AttributeError` once used, the exact failure this PR's boundary exists to prevent. Records are now converted to plain dicts at the boundary (`_coerce_records` → new `_record_to_dict`), so every consumer gets a uniform shape regardless of which reading the caller used + - Two non-empty spellings of the same collection (e.g. `entities` and `nodes`) holding identical records in a different order were rejected as conflicting, since the check used plain list equality; a caller round-tripping through a dict-keyed cache or a set has no reason to preserve order. Comparison is now an order-independent multiset of each record's canonical JSON form + - New regression coverage in `tests/utils/test_normalize_graph_payload.py`: exception-type parity for non-mapping input across `export_lpg`/`export_arango`/`export_neo4j_csv`, dataclass-record conversion verified end-to-end through the same three exporters, `Neo4jCSVExporter` accepting a `MappingProxyType` payload, and reordered-alias equality (plus a duplicate-count case confirming the multiset check still catches real conflicts); 4 existing tests updated to assert the corrected dict-conversion behavior instead of the previous object passthrough + - `pytest tests/export tests/utils tests/context tests/test_export_module.py tests/test_export_methods_wrapper.py tests/test_notebooks_simulation.py`: 718 passed, 4 skipped (up from 641 passed, 62 subtests at PR submission); `black`/`isort`/`flake8 --max-line-length=88` clean on every line this PR touches; `python -m build`: succeeds + - **`ContextGraph.add_edge` had no dedupe — identical edges were stored repeatedly under one shared edge ID, and re-ingest doubled the edge set** (#926, closes #922) by @pravit-amp - `_add_internal_edge` appended to `self.edges`, `edge_type_index`, and `_adjacency` unconditionally, with no check for an edge already present. Edge identity is content-derived (`_resolve_edge_identity` builds `edge_id` from `source_id`/`target_id`/`edge_type`/`weight`/`metadata`/`valid_from`/`valid_until`), so two identical `add_edge` calls produced two edge objects sharing one `edge_id` — the graph already considered them the same edge, it just kept both copies. `self.nodes` already deduped by ID; edges did not, so `stats()["edge_count"]` inflated, `density()` could exceed its mathematical maximum of `1.0`, and a refresh/restore job calling `build_from_entities_and_relationships()` (or reloading a saved graph) doubled the edge set on every cycle - Added an `edge_id -> ContextEdge` index (`_edge_index`), mirroring how `self.nodes` dedupes by node ID. `_add_internal_edge` now returns `False` when the `edge_id` already exists, checked before touching `edges`/`edge_type_index`/`_adjacency` and before firing the mutation callback, so a repeat `add_edge` is a silent no-op with no phantom `ADD_EDGE` audit event diff --git a/docs/reference/export.md b/docs/reference/export.md index 49df715e..eef19e94 100644 --- a/docs/reference/export.md +++ b/docs/reference/export.md @@ -203,6 +203,13 @@ export_lpg(graph, "import.cypher", method="cypher") exporter = SemanticNetworkYAMLExporter() exporter.export(graph, "graph.yaml") ``` + + The YAML exporters read `entities`/`relationships`/`triplets` (with + `nodes`/`edges` accepted as aliases, so `ContextGraph.to_dict()` exports + directly). A non-empty mapping supplying none of them raises + `ValidationError` rather than writing a file with every collection empty, + as does one whose collection value is not a list of records + (`{"entities": "abc"}`). **LPGExporter** writes Cypher `CREATE` statements for Neo4j and Memgraph: @@ -236,6 +243,12 @@ export_lpg(graph, "import.cypher", method="cypher") Both exporters write to a file and return `None`. + `LPGExporter`, `ArangoAQLExporter`, and `Neo4jCSVExporter` resolve mapping + payloads on the same terms as the YAML exporters above, so an unrecognized + or malformed mapping is rejected instead of exported as an empty graph. + `Neo4jCSVExporter` still reads graph *objects* off their + `nodes`/`entities` and `edges`/`relationships` attributes. + **`ArangoAQLExporter.export()` and `LPGExporter.export()` write to a file and return `None`.** They do not return the AQL/Cypher string. Write to a file and read it back if you need the string. diff --git a/semantica/export/arango_aql_exporter.py b/semantica/export/arango_aql_exporter.py index 69140481..d5f9a571 100644 --- a/semantica/export/arango_aql_exporter.py +++ b/semantica/export/arango_aql_exporter.py @@ -28,7 +28,7 @@ import json from pathlib import Path from typing import Any, Dict, List, Optional, Union -from ..utils.helpers import ensure_directory +from ..utils.helpers import _require_mapping, ensure_directory, normalize_graph_payload from ..utils.logging import get_logger from ..utils.progress_tracker import get_progress_tracker @@ -204,17 +204,19 @@ class ArangoAQLExporter: self._generate_collection_creation(vertex_collection, edge_collection) ) - # Extract entities and relationships - entities = knowledge_graph.get("entities", []) - relationships = knowledge_graph.get("relationships", []) - nodes = knowledge_graph.get("nodes", entities) - edges = knowledge_graph.get("edges", relationships) + # A non-mapping payload cannot reach normalize_graph_payload(): it + # raises ValidationError for that case, which would leave this + # exporter alone in raising a different exception type than the YAML + # and Neo4j exporters raise for the identical mistake. + _require_mapping( + knowledge_graph, ("entities", "relationships", "nodes", "edges") + ) - # Use nodes/edges if entities/relationships are empty - if not entities and nodes: - entities = nodes - if not relationships and edges: - relationships = edges + # Accept either vocabulary; resolution is centralized so every + # exporter agrees on what a given payload means. + normalized = normalize_graph_payload(knowledge_graph) + entities = normalized["entities"] + relationships = normalized["relationships"] # Generate vertex INSERT statements vertex_statements = self._generate_vertex_inserts(entities, vertex_collection) diff --git a/semantica/export/export_usage.md b/semantica/export/export_usage.md index 140c5a01..7c8881e2 100644 --- a/semantica/export/export_usage.md +++ b/semantica/export/export_usage.md @@ -297,6 +297,57 @@ export_yaml(semantic_network, "network.yaml", method="semantic_network") export_yaml(schema, "schema.yaml", method="schema") ``` +### Accepted Input + +Both YAML exporters read their payload by key, so the input must be a mapping; +anything else raises `ProcessingError`. A bare list is rejected rather than +wrapped, since these formats distinguish entities from relationships from +triplets and guessing which one a list holds would mislabel the records. + +Each exporter then reads a fixed set of keys, and raises `ValidationError` on a +non-empty mapping that supplies none of them — such a payload would otherwise +serialize to a valid file with every collection empty. Naming a recognized key +is not enough on its own: `{"entities": [], "data": [...]}` also raises, since +nothing resolves while the records sit under a key the exporter never reads. + +| Method | Recognized keys | +| :--- | :--- | +| `"semantic_network"` | `entities` (alias `nodes`), `relationships` (alias `edges`), `triplets` | +| `"schema"` | `classes`, `properties`, `namespaces`, `uri`, `title`, `description`, `version` | + +`metadata` is carried through on both, but does not by itself make a payload +recognized — an `export_json` envelope (`{"data": [...], "count": N, +"metadata": {...}}`) carries one and is rejected. + +```python +# ContextGraph.to_dict() exports directly via the nodes/edges aliases +export_yaml(context_graph.to_dict(), "graph.yaml") + +# A bare list has no unambiguous meaning here +export_yaml(records, "out.yaml") # ProcessingError + +# An export_json payload is refused rather than written out empty +export_yaml({"data": records}, "out.yaml") # ValidationError + +# ...and so is one that names a recognized key but leaves it empty +export_yaml({"entities": [], "data": records}, "out.yaml") # ValidationError +``` + +The value under a recognized key must be a collection of records — a list or +tuple of mappings or objects. A string, a bare mapping, or a scalar raises +`ValidationError` naming the key, rather than being iterated into +character-sized "records" or surfacing as a `TypeError` from inside the +exporter. `None` is read as an absent collection, the same as `[]`. + +```python +export_yaml({"entities": "abc"}, "out.yaml") # ValidationError +export_yaml({"entities": 42}, "out.yaml") # ValidationError +export_yaml({"nodes": {"id": "n1"}}, "out.yaml") # ValidationError — wrap it in a list +``` + +An empty mapping is still accepted: an empty graph is a legitimate export and +has no records to lose. + ## OWL Export ### OWL/XML Format @@ -479,6 +530,23 @@ Pass `validate=True` to run a post-export integrity check before returning: export_neo4j_csv(kg, "neo4j_import/", validate=True) ``` +#### Accepted Input + +Mapping payloads are read on the same terms as the YAML exporters (see [Accepted +Input](#accepted-input) above): `entities`/`relationships`, with `nodes`/`edges` +accepted as aliases. A non-empty mapping that supplies neither — or that supplies +a malformed collection value — raises `ValidationError` rather than writing +header-only CSVs indistinguishable from a genuinely exported empty graph. The +payload is normalized before any file is opened, so a rejected export writes +nothing. + +Graph *objects* are unaffected: they are still read off `nodes`/`entities` and +`edges`/`relationships` attributes. + +```python +export_neo4j_csv({"data": [{"id": "e1"}]}, "neo4j_import/") # ValidationError +``` + #### Importing into Neo4j Once the CSV files are generated, they can be imported into a new Neo4j database using the `neo4j-admin database import` command: diff --git a/semantica/export/lpg_exporter.py b/semantica/export/lpg_exporter.py index 40f688a5..06ac4554 100644 --- a/semantica/export/lpg_exporter.py +++ b/semantica/export/lpg_exporter.py @@ -24,7 +24,7 @@ from pathlib import Path from typing import Any, Dict, List, Optional, Union from ..utils.exceptions import ProcessingError, ValidationError -from ..utils.helpers import ensure_directory +from ..utils.helpers import _require_mapping, ensure_directory, normalize_graph_payload from ..utils.logging import get_logger from ..utils.progress_tracker import get_progress_tracker @@ -154,15 +154,26 @@ class LPGExporter: """ queries = [] - # Generate indexes if requested - if self.include_indexes: - queries.extend(self._generate_indexes(knowledge_graph)) + # A non-mapping payload cannot reach normalize_graph_payload(): it + # raises ValidationError for that case, which would leave this + # exporter alone in raising a different exception type than the YAML + # and Neo4j exporters raise for the identical mistake. + _require_mapping( + knowledge_graph, ("entities", "relationships", "nodes", "edges") + ) - # Extract entities and relationships - entities = knowledge_graph.get("entities", []) - relationships = knowledge_graph.get("relationships", []) - nodes = knowledge_graph.get("nodes", entities) - edges = knowledge_graph.get("edges", relationships) + # Accept either vocabulary. Reading 'nodes' with 'entities' as the + # default dropped every entity when 'nodes' was present but empty -- + # the shape JSONExporter emits -- so resolution is centralized. + normalized = normalize_graph_payload(knowledge_graph) + nodes = normalized["entities"] + edges = normalized["relationships"] + + # Generate indexes if requested. Fed the normalized entities so index + # generation sees the same records as node generation; reading + # 'entities' directly here skipped indexes for nodes/edges payloads. + if self.include_indexes: + queries.extend(self._generate_indexes(nodes)) # Generate node creation queries node_queries = self._generate_node_queries(nodes) @@ -174,13 +185,18 @@ class LPGExporter: return queries - def _generate_indexes(self, knowledge_graph: Dict[str, Any]) -> List[str]: - """Generate Cypher index and constraint creation queries.""" + def _generate_indexes(self, entities: List[Dict[str, Any]]) -> List[str]: + """Generate Cypher index and constraint creation queries. + + Args: + entities: Entity records, already resolved from whichever + vocabulary the caller supplied. + """ indexes = [] # Get unique entity types for labels entity_types = set() - for entity in knowledge_graph.get("entities", []): + for entity in entities: entity_type = entity.get("type") or entity.get("entity_type") if entity_type: entity_types.add(entity_type) diff --git a/semantica/export/methods.py b/semantica/export/methods.py index 6cec22c4..d2af2581 100644 --- a/semantica/export/methods.py +++ b/semantica/export/methods.py @@ -494,7 +494,7 @@ def export_graph( def export_yaml( - data: Union[Dict[str, Any], List[Dict[str, Any]]], + data: Dict[str, Any], file_path: Union[str, Path], method: str = "semantic_network", **kwargs, @@ -504,14 +504,32 @@ def export_yaml( This is a user-friendly wrapper that exports data to YAML format. + Unlike :func:`export_json` and :func:`export_csv`, which treat a list as + opaque records, both YAML methods are keyed formats: they distinguish + entities from relationships from triplets (and classes from properties + for ``method="schema"``). A bare list is therefore rejected rather than + guessed at, since inferring which collection it represents would silently + mislabel the records. + Args: - data: Data to export (semantic network, entities, relationships) + data: Data to export, as a mapping. For ``method="semantic_network"``, + keyed by 'entities'/'relationships'/'triplets'; for + ``method="schema"``, by 'classes'/'properties'. file_path: Output YAML file path method: Export method (default: "semantic_network") - "semantic_network": Semantic network YAML export - "schema": Schema YAML export **kwargs: Additional options passed to YAML exporters + Raises: + ProcessingError: if ``data`` is not a mapping, or if ``method`` is not + a known YAML export method. + ValidationError: if ``data`` is a mapping whose keys the selected + exporter does not read -- an ``export_json`` envelope + (``{"data": [...], "count": N, "metadata": {...}}``) is the + common case. Such a payload used to be written out as a valid + YAML file with every collection empty. + Examples: >>> from semantica.export.methods import export_yaml >>> export_yaml(semantic_network, "network.yaml", method="semantic_network") diff --git a/semantica/export/neo4j_csv_exporter.py b/semantica/export/neo4j_csv_exporter.py index 7a3461d1..8dbbd31b 100644 --- a/semantica/export/neo4j_csv_exporter.py +++ b/semantica/export/neo4j_csv_exporter.py @@ -32,12 +32,13 @@ from __future__ import annotations import csv import hashlib import json +from collections.abc import Mapping from dataclasses import asdict, is_dataclass from pathlib import Path from typing import Any, Dict, Iterable, List, Optional, Sequence, Union from ..utils.exceptions import ProcessingError, ValidationError -from ..utils.helpers import ensure_directory +from ..utils.helpers import ensure_directory, normalize_graph_payload from ..utils.logging import get_logger from ..utils.progress_tracker import get_progress_tracker @@ -173,6 +174,19 @@ class Neo4jCSVExporter: Returns: Mapping with ``"nodes"`` and ``"relationships"`` output paths. + + Raises: + ValidationError: if a mapping payload carries no recognized graph + key, resolves to nothing while an unread key still holds + records, or holds something other than records under one -- + see + :func:`~semantica.utils.helpers.normalize_graph_payload`. + Each would otherwise be written out as header-only CSVs + indistinguishable from a genuinely empty graph. The payload is + normalized before any file is opened, so a rejected export + writes nothing. + ProcessingError: if a non-mapping payload exposes none of the + graph attributes. """ output_dir = Path(output_dir) ensure_directory(output_dir) @@ -494,9 +508,19 @@ class Neo4jCSVExporter: return prepared def _normalize_graph(self, graph: Any) -> Dict[str, List[Dict[str, Any]]]: - if isinstance(graph, dict): - nodes = graph.get("nodes") or graph.get("entities") or [] - relationships = graph.get("edges") or graph.get("relationships") or [] + if isinstance(graph, Mapping): + # Mapping payloads go through the shared resolver on its default + # terms, so this backend cannot drift from the others: an + # unrecognized mapping raises here rather than writing header-only + # CSVs that read as a successful export of an empty graph. Checked + # against Mapping rather than dict, so a non-dict Mapping (a + # MappingProxyType, a ChainMap) takes this path too, instead of + # falling through to the attribute branch below and being rejected + # as an unrecognized object -- the LPG, Arango, and YAML exporters + # already accept such payloads via the same resolver. + resolved = normalize_graph_payload(graph) + nodes = resolved["entities"] + relationships = resolved["relationships"] else: nodes = getattr(graph, "nodes", None) if nodes is None: diff --git a/semantica/export/yaml_exporter.py b/semantica/export/yaml_exporter.py index 0249ec08..ecbd55fa 100644 --- a/semantica/export/yaml_exporter.py +++ b/semantica/export/yaml_exporter.py @@ -21,15 +21,83 @@ Author: Semantica Contributors License: MIT """ +from collections.abc import Mapping from datetime import datetime from pathlib import Path from typing import Any, Dict, List, Optional, Union -from ..utils.exceptions import ProcessingError, ValidationError -from ..utils.helpers import ensure_directory +from ..utils.exceptions import ValidationError +from ..utils.helpers import ( + _require_mapping, + _require_nothing_dropped, + _require_recognized_keys, + ensure_directory, + normalize_graph_payload, +) from ..utils.logging import get_logger from ..utils.progress_tracker import get_progress_tracker +# Keys YAMLSchemaExporter.export_ontology_schema reads. Graph payloads use the +# recognized set owned by normalize_graph_payload() instead; schemas are a +# separate vocabulary with no aliasing, so the set lives here. +_SCHEMA_KEYS = ( + "classes", + "properties", + "namespaces", + "uri", + "title", + "description", + "version", +) + + +def _require_usable_schema(ontology: Mapping) -> None: + """Reject a schema mapping this exporter cannot read. + + Two ways an ontology mapping produces an empty file: it shares no key with + the recognized set at all, or it names a recognized key that is empty + while the real records sit under a key this exporter does not read + (``{"classes": [], "nodes": [...]}``). Both are refused, using the same + checks the graph payloads go through, so the two vocabularies cannot drift + apart in what they consider a silent-empty export. + + An empty mapping is allowed through: it carries nothing that could be + lost, and an empty export is a legitimate result. + + Note the deliberate split in exception types, which the codebase already + makes: a wrong *type* cannot be exported at all and raises + ProcessingError, matching ``Neo4jCSVExporter._normalize_graph``; a mapping + whose *contents* are unusable raises ValidationError, matching + ``normalize_graph_payload``. + + Args: + ontology: Mapping already checked by :func:`_require_mapping`. + + Raises: + ValidationError: if the mapping shares no key with ``_SCHEMA_KEYS``, + or resolves to nothing while an unread key still holds records. + """ + _require_recognized_keys(ontology, _SCHEMA_KEYS, what="Ontology schema") + # Only non-empty list/tuple values from recognized schema keys count as + # evidence that records survived export. Scalar metadata fields such as + # 'uri', 'title', 'description', and 'version' are truthy strings, but + # their presence does not mean the caller's record collections were + # exported -- passing them as ``resolved`` would let any scalar value + # short-circuit the dropped-records check and silently discard a list + # under an unread key alongside e.g. {"version": "1.0", "nodes": [...]}. + resolved = [ + v + for key in _SCHEMA_KEYS + for v in (ontology.get(key),) + if isinstance(v, (list, tuple)) and v + ] + _require_nothing_dropped( + ontology, + _SCHEMA_KEYS, + resolved, + what="Ontology schema", + ) + class SemanticNetworkYAMLExporter: """ @@ -90,15 +158,39 @@ class SemanticNetworkYAMLExporter: Args: semantic_network: Semantic network dictionary containing: - - entities: List of entity dictionaries + - entities: List of entity dictionaries (alias: 'nodes') - relationships: List of relationship dictionaries + (alias: 'edges') - triplets: List of triplet dictionaries (optional) - metadata: Metadata dictionary (optional) + + Key resolution is delegated to + :func:`~semantica.utils.helpers.normalize_graph_payload`, so + ``ContextGraph.to_dict()`` output ('nodes'/'edges') exports + directly. **options: Additional export options (unused) Returns: String containing YAML representation of semantic network + Raises: + ProcessingError: if ``semantic_network`` is not a mapping. A bare + list of records cannot be exported here because this format + distinguishes entities, relationships, and triplets, and + guessing which one a list represents would silently mislabel + it. + ValidationError: if the mapping carries both spellings of a + collection with different contents; if it is non-empty and + shares no key with the recognized set; or if it resolves to + nothing while an unread key still holds records + (``{"entities": [], "data": [...]}``). Each previously + serialized to a file with every collection empty while the log + reported success. An empty mapping is still accepted -- it has + no records to lose. Note that 'metadata' alone is not a + recognized key: an ``export_json`` envelope carries one, and + accepting it would readmit the silent-empty export it is the + most likely source of. + Example: >>> network = { ... "entities": [...], @@ -107,6 +199,8 @@ class SemanticNetworkYAMLExporter: ... } >>> yaml_str = exporter.export_semantic_network(network) """ + _require_mapping(semantic_network, ("entities", "relationships", "triplets")) + # Track YAML export tracking_id = self.progress_tracker.start_tracking( file=None, @@ -119,15 +213,14 @@ class SemanticNetworkYAMLExporter: self.progress_tracker.update_tracking( tracking_id, message="Preparing YAML data..." ) + records = normalize_graph_payload(semantic_network) yaml_data = { "metadata": { "exported_at": datetime.now().isoformat(), "version": "1.0", **semantic_network.get("metadata", {}), }, - "entities": semantic_network.get("entities", []), - "relationships": semantic_network.get("relationships", []), - "triplets": semantic_network.get("triplets", []), + **records, } self.progress_tracker.update_tracking( @@ -140,7 +233,7 @@ class SemanticNetworkYAMLExporter: self.progress_tracker.stop_tracking( tracking_id, status="completed", - message="Exported semantic network to YAML", + message="Serialized semantic network to YAML", ) return result @@ -160,16 +253,46 @@ class SemanticNetworkYAMLExporter: data: Data to export file_path: Output file path **options: Additional options + + Raises: + ProcessingError: if ``data`` is not a mapping. + ValidationError: on the mappings :meth:`export_semantic_network` + rejects. Serialization runs before the output directory is + created, so a rejected export leaves nothing behind. + OSError: if the file cannot be written. The write is tracked + separately from serialization, so no progress entry reports a + completed export until the bytes are on disk. """ file_path = Path(file_path) - ensure_directory(file_path.parent) - yaml_content = self.export_semantic_network(data, **options) - with open(file_path, "w", encoding="utf-8") as f: - f.write(yaml_content) + # Serialization reports its own completion, but it says nothing about + # the file: without this second span, a failing write would leave the + # tracker showing a completed export and no output. + tracking_id = self.progress_tracker.start_tracking( + file=str(file_path), + module="export", + submodule="SemanticNetworkYAMLExporter", + message=f"Writing YAML to {file_path}", + ) - self.logger.info(f"Exported YAML to: {file_path}") + try: + ensure_directory(file_path.parent) + with open(file_path, "w", encoding="utf-8") as f: + f.write(yaml_content) + + self.logger.info(f"Exported YAML to: {file_path}") + self.progress_tracker.stop_tracking( + tracking_id, + status="completed", + message=f"Exported YAML to: {file_path}", + ) + + except Exception as e: + self.progress_tracker.stop_tracking( + tracking_id, status="failed", message=str(e) + ) + raise def export_entities( self, entities: List[Dict[str, Any]], include_metadata: bool = True, **options @@ -263,18 +386,34 @@ class SemanticNetworkYAMLExporter: • Structure for definition generation • Include extraction metadata • Return pipeline-ready YAML + + Args: + extracted_data: Semantic network mapping, read through + :func:`~semantica.utils.helpers.normalize_graph_payload` on + the same terms as :meth:`export_semantic_network`. + pipeline_stage: Stage number recorded in the output. + **options: Additional export options (unused) + + Returns: + Pipeline-ready YAML string. + + Raises: + ProcessingError: if ``extracted_data`` is not a mapping. + ValidationError: on the same mappings as + :meth:`export_semantic_network` -- this method built its + nested semantic network from the same defaulted lookups and + so had the same silent-empty failure. """ + _require_mapping(extracted_data, ("entities", "relationships", "triplets")) + + semantic_network = normalize_graph_payload(extracted_data) yaml_data = { "pipeline_stage": pipeline_stage, "metadata": { "extracted_at": datetime.now().isoformat(), **extracted_data.get("metadata", {}), }, - "semantic_network": { - "entities": extracted_data.get("entities", []), - "relationships": extracted_data.get("relationships", []), - "triplets": extracted_data.get("triplets", []), - }, + "semantic_network": semantic_network, } return self.yaml.dump(yaml_data, default_flow_style=False, sort_keys=False) @@ -308,7 +447,29 @@ class YAMLSchemaExporter: • Include hierarchies and constraints • Structure for easy editing • Return YAML schema + + Args: + ontology: Ontology mapping keyed by any of 'classes', + 'properties', 'namespaces', 'uri', 'title', 'description', + 'version'. + **options: Additional export options (unused) + + Returns: + YAML schema string. + + Raises: + ProcessingError: if ``ontology`` is not a mapping. + ValidationError: if ``ontology`` is a non-empty mapping sharing + no key with the recognized set, or resolves to nothing while + an unread key still holds records + (``{"classes": [], "nodes": [...]}``) -- each previously + produced a file with empty 'classes', 'properties' and + 'namespaces' and no indication anything was dropped. An empty + mapping is still accepted. """ + _require_mapping(ontology, ("classes", "properties")) + _require_usable_schema(ontology) + yaml_data = { "ontology": { "uri": ontology.get("uri", ""), diff --git a/semantica/utils/__init__.py b/semantica/utils/__init__.py index bd0e7e2d..002a139d 100644 --- a/semantica/utils/__init__.py +++ b/semantica/utils/__init__.py @@ -80,6 +80,7 @@ from .helpers import ( hash_data, merge_dicts, normalize_entities, + normalize_graph_payload, parse_timestamp, read_json_file, retry_on_error, @@ -183,6 +184,7 @@ __all__ = [ "format_data", "clean_text", "normalize_entities", + "normalize_graph_payload", "hash_data", "safe_filename", "ensure_directory", diff --git a/semantica/utils/helpers.py b/semantica/utils/helpers.py index 21f6cfc2..7462f6db 100644 --- a/semantica/utils/helpers.py +++ b/semantica/utils/helpers.py @@ -63,9 +63,16 @@ import importlib import json import os import re +import types +from collections import Counter +from collections.abc import Iterable as IterableABC +from collections.abc import Mapping +from dataclasses import asdict, is_dataclass from datetime import datetime, timezone from pathlib import Path -from typing import Any, Dict, List, Optional, Tuple, Type, Union +from typing import Any, Dict, Iterable, List, Optional, Sequence, Tuple, Type, Union + +from .exceptions import ProcessingError, ValidationError def format_data(data: Any, format_type: str = "json") -> str: @@ -584,3 +591,367 @@ def classify_path_distance(hop_count: int) -> str: if hop_count <= 6: return "mid-range" return "distant" + + +# Graph payloads circulate under two vocabularies: 'entities'/'relationships' +# (kg builders, most exporters) and 'nodes'/'edges' (ContextGraph.to_dict, +# Neo4jCSVExporter, the Explorer routes). Consumers each reconciled them +# locally, with at least three competing idioms, so the same payload could be +# exported, silently dropped, or rejected depending on which consumer read it. +# This is the single place that decision is made. +_ENTITY_KEYS = ("entities", "nodes") +_RELATIONSHIP_KEYS = ("relationships", "edges") +_TRIPLET_KEYS = ("triplets",) + +# Keys that legitimately travel alongside the collections without being +# records themselves, so their presence is never evidence that records were +# dropped: ContextGraph.to_dict() carries 'statistics', JSON envelopes carry +# 'metadata' and 'count'. +_CONTEXT_KEYS = ("metadata", "statistics", "count") + + +def _require_recognized_keys( + payload: Mapping, recognized_keys: Sequence[str], *, what: str +) -> None: + """Reject a mapping that shares no key with the recognized set. + + A consumer that reads a fixed set of keys turns an unrecognized mapping + into an empty result that looks like a legitimate one. An empty mapping is + allowed through -- it carries nothing that could be lost. + + Args: + payload: Mapping to check. + recognized_keys: Keys the consumer reads. + what: Noun for the error message, e.g. ``"Graph payload"``. + + Raises: + ValidationError: if ``payload`` is non-empty and shares no key with + ``recognized_keys``. + """ + if not payload or any(key in payload for key in recognized_keys): + return + + supplied = ", ".join(f"'{key}'" for key in sorted(map(str, payload))) + expected = ", ".join(f"'{key}'" for key in recognized_keys) + raise ValidationError( + f"{what} has no recognized key. Supplied: {supplied}. " + f"Expected at least one of: {expected}." + ) + + +def _require_nothing_dropped( + payload: Mapping, + recognized_keys: Sequence[str], + resolved: Iterable[Any], + *, + what: str, +) -> None: + """Reject a mapping that resolved to nothing while still holding records. + + Checking that a recognized key is *present* is not enough: + ``{"entities": [], "data": [...]}`` clears that bar and still resolves to + empty, dropping every record under 'data'. Presence answers "did the + caller use our vocabulary"; this answers the question that actually + matters, "did anything the caller supplied survive". + + Only non-empty lists count as evidence of dropped records. A payload can + carry scalars and dicts that are not collections -- ContextGraph.to_dict() + always includes 'statistics' -- and an empty graph must stay exportable. + + Args: + payload: Mapping to check. + recognized_keys: Keys the consumer reads. + resolved: The collections the consumer resolved from ``payload``. + what: Noun for the error message, e.g. ``"Graph payload"``. + + Raises: + ValidationError: if nothing resolved and an unread key holds a + non-empty list. + """ + if any(resolved): + return + + dropped = sorted( + str(key) + for key, value in payload.items() + if key not in recognized_keys + and key not in _CONTEXT_KEYS + and isinstance(value, (list, tuple)) + and value + ) + if not dropped: + return + + named = ", ".join(f"'{key}'" for key in dropped) + expected = ", ".join(f"'{key}'" for key in recognized_keys) + raise ValidationError( + f"{what} resolved to nothing, but {named} still holds records. " + f"Exporting it would drop them silently. Supply the records under " + f"one of: {expected}." + ) + + +def _is_record(value: Any) -> bool: + """Report whether a value can stand in for a graph record. + + Consumers read records either as mappings (``entity.get("type")`` in the + LPG and Arango exporters) or as objects with attributes + (``Neo4jCSVExporter._record_to_dict`` accepts dataclasses and anything + carrying a ``__dict__``). Both are legitimate, so both are accepted here; + strings, numbers, and nested sequences are not records under either + reading. + + Modules and class/type objects are excluded even though they carry + ``__dict__``: they are not graph records under any supported reading, and + passing them through the boundary would produce ``AttributeError`` inside + exporters rather than a ``ValidationError`` at the boundary where the + problem is visible. + """ + return isinstance(value, Mapping) or is_dataclass(value) or ( + hasattr(value, "__dict__") + and not isinstance(value, (types.ModuleType, type)) + ) + + +def _record_to_dict(record: Any) -> Dict[str, Any]: + """Convert an accepted record to a plain dict. + + :func:`_is_record` accepts mappings, dataclasses, and objects carrying + ``__dict__`` as legitimate record shapes, but consumers of + :func:`normalize_graph_payload` -- YAML serialization, ``entity.get(...)`` + in the LPG and Arango exporters -- read records as dicts. Converting here, + at the boundary, means every exporter gets the same shape regardless of + which reading the caller used; previously only ``Neo4jCSVExporter`` + converted object-shaped records locally, so a dataclass record passed + validation for the other exporters only to crash with a raw + ``AttributeError`` once used. + """ + if isinstance(record, Mapping): + return dict(record) + if is_dataclass(record): + return asdict(record) + return { + key: value for key, value in vars(record).items() if not key.startswith("_") + } + + +def _coerce_records(key: str, value: Any) -> List[Any]: + """Validate one collection value and materialize it as a list of records. + + This runs before any truthiness or ``list()`` call, because both mislead + on malformed input: ``list("abc")`` quietly turns a string into three + single-character "records", and ``list(42)`` raises a bare ``TypeError`` + from deep inside the exporter that named the exporter rather than the + offending payload key. Neither reaches the caller as an actionable + message, so the shapes that produce them are rejected by name instead. + + ``None`` is deliberately not rejected: JSON round-trips an absent + collection to null, and treating that as "no records under this key" is + the same answer an explicit ``[]`` gets. It is not silent data loss -- + a null collection alongside records under an unread key is still caught + by :func:`_require_nothing_dropped`. + + Args: + key: Payload key the value came from, for the error message. + value: The raw value stored under ``key``. + + Returns: + The records as a new list, so the result never aliases the input. + + Raises: + ValidationError: if ``value`` is a string, bytes, a mapping, or any + non-iterable scalar; or if any element is not a record. + """ + if value is None: + return [] + + if isinstance(value, (str, bytes, bytearray)): + raise ValidationError( + f"Graph payload key '{key}' holds a {type(value).__name__}, not a " + f"collection of records. Iterating it would yield characters, not " + f"records. Supply a list of records." + ) + + if isinstance(value, Mapping): + raise ValidationError( + f"Graph payload key '{key}' holds a mapping, not a collection of " + f"records. If it is a single record, wrap it in a list; if it is " + f"keyed by ID, supply its values as a list." + ) + + if not isinstance(value, IterableABC): + raise ValidationError( + f"Graph payload key '{key}' holds a " + f"{type(value).__name__}, not a collection of records. Supply a " + f"list of records." + ) + + records = list(value) + for index, record in enumerate(records): + if not _is_record(record): + raise ValidationError( + f"Graph payload key '{key}' holds a " + f"{type(record).__name__} at index {index}, not a record. " + f"Records must be mappings or objects with attributes." + ) + return [_record_to_dict(record) for record in records] + + +def _canonical_record_multiset(records: List[Dict[str, Any]]) -> "Counter[str]": + """Represent records as an order-independent multiset for equality checks. + + Two spellings of the same collection (``entities`` and ``nodes``) can + legitimately list identical records in a different order -- a caller + round-tripping through a dict-keyed cache or a set has no reason to + preserve list order. Comparing with plain list equality would treat that + as a conflict and reject a payload that carries no real data loss, so + records are compared as a multiset of their canonical JSON form instead. + """ + return Counter( + json.dumps(record, sort_keys=True, default=str) for record in records + ) + + +def _resolve_collection( + payload: Dict[str, Any], keys: Tuple[str, ...] +) -> List[Dict[str, Any]]: + """Pick one collection from a payload that may use either vocabulary. + + Both spellings may legitimately be present: ``JSONExporter`` writes + 'entities' and 'nodes' side by side, so a round-trip of its output carries + both, one of them empty. Where only one holds records, that one wins. + + Two non-empty, unequal spellings are a different matter -- there is no + basis for preferring either, and picking one would silently discard the + other -- so that is refused rather than guessed at. + + Every spelling present is validated, not just the one that wins: a + malformed 'nodes' alongside a well-formed 'entities' is a payload the + caller should hear about, and validating only the winner would let it + through on the strength of the other key. + + Args: + payload: Mapping to read from. + keys: Accepted spellings, most canonical first. + + Returns: + The resolved collection, or an empty list if no spelling is present. + + Raises: + ValidationError: if a spelling holds something other than a collection + of records; or if two spellings are both present, both non-empty, + and hold different records, order ignored. + """ + present = { + key: _coerce_records(key, payload[key]) for key in keys if key in payload + } + populated = {key: value for key, value in present.items() if value} + + if len(populated) > 1: + values = list(populated.values()) + canonical = [_canonical_record_multiset(value) for value in values] + if any(entry != canonical[0] for entry in canonical[1:]): + named = " and ".join(f"'{key}'" for key in populated) + raise ValidationError( + f"Graph payload carries {named} with different contents; " + f"cannot determine which to export. Supply one, or make them " + f"identical." + ) + + for key in keys: + value = present.get(key) + if value: + # Already a fresh list from _coerce_records, so the result cannot + # alias the caller's collection. + return value + + # Every spelling present is empty (or none is): an explicit empty + # collection is a legitimate answer, distinct from "unrecognized". + return [] + + +def _require_mapping(data: Any, expected_keys: Sequence[str]) -> None: + """Reject non-mapping export input with an actionable error. + + Shared by every consumer of :func:`normalize_graph_payload` so that a + wrong *type* fails the same way everywhere. Handed a sequence (or any + other non-mapping), every downstream key lookup would fail with a bare + ``AttributeError: 'list' object has no attribute 'get'``, which tells the + caller nothing about the shape expected -- and ``normalize_graph_payload`` + itself raises ``ValidationError`` for this case, which would leave + exporters that skip this guard raising a different exception type than + the ones that call it, for the identical mistake. + + A list is rejected rather than wrapped: these formats distinguish + entities from relationships from triplets (or nodes/edges), so inferring + which one a bare list represents would silently mislabel the records. + + Args: + data: Candidate export payload. + expected_keys: Key names the caller reads, named in the error so the + caller learns the expected shape. + + Raises: + ProcessingError: if ``data`` is not a mapping. + """ + if not isinstance(data, Mapping): + keys = "/".join(f"'{key}'" for key in expected_keys) + raise ProcessingError( + f"Cannot export object of type '{type(data).__name__}': " + f"expected a dict with {keys}." + ) + + +def normalize_graph_payload( + payload: Dict[str, Any], +) -> Dict[str, List[Dict[str, Any]]]: + """Reduce a graph payload to one canonical vocabulary. + + Accepts either 'entities'/'relationships' or 'nodes'/'edges' (or a mix) + and returns the canonical spelling, so consumers read one shape instead of + reimplementing the reconciliation. + + This is the validation boundary for graph payloads: it either returns + collections of records or raises. Nothing that reaches an exporter through + it needs re-checking, and nothing malformed passes through it as a + valid-looking empty graph. + + Args: + payload: Graph payload mapping. + + Returns: + ``{"entities": [...], "relationships": [...], "triplets": [...]}``. + + Raises: + ValidationError: if ``payload`` is not a mapping; if a recognized key + holds something other than a collection of records; if two + spellings of the same collection are both non-empty and differ; if + a non-empty mapping contains no recognized key; or if it resolves + to nothing while an unread key still holds records. The last two + would otherwise hand the caller a valid-looking result with their + records silently dropped. + + Example: + >>> normalize_graph_payload({"nodes": [{"id": "n1"}], "edges": []}) + {'entities': [{'id': 'n1'}], 'relationships': [], 'triplets': []} + """ + if not isinstance(payload, Mapping): + raise ValidationError( + f"Cannot normalize graph payload of type " + f"'{type(payload).__name__}': expected a mapping." + ) + + recognized = _ENTITY_KEYS + _RELATIONSHIP_KEYS + _TRIPLET_KEYS + _require_recognized_keys(payload, recognized, what="Graph payload") + + resolved = { + "entities": _resolve_collection(payload, _ENTITY_KEYS), + "relationships": _resolve_collection(payload, _RELATIONSHIP_KEYS), + "triplets": _resolve_collection(payload, _TRIPLET_KEYS), + } + + _require_nothing_dropped( + payload, recognized, resolved.values(), what="Graph payload" + ) + + return resolved diff --git a/tests/export/test_neo4j_csv_exporter.py b/tests/export/test_neo4j_csv_exporter.py index 529f7bd4..e7f4d0c9 100644 --- a/tests/export/test_neo4j_csv_exporter.py +++ b/tests/export/test_neo4j_csv_exporter.py @@ -301,3 +301,60 @@ def test_nested_properties_are_json_serialized(tmp_path): by_id = {row[0]: row for row in rows[1:]} assert by_id["node1"][2] == '{"k":"v"}' assert by_id["node1"][3] == "[1,2,3]" + + +def test_unrecognized_mapping_is_refused_rather_than_exported_empty(tmp_path): + """The Neo4j path reads mappings on the shared normalizer's default terms. + + An ``export_json`` envelope names no graph key, so it resolves to nothing. + Written out, that is a pair of header-only CSVs indistinguishable from a + genuinely empty graph -- the silent-empty export the shared contract + exists to prevent. + """ + exporter = Neo4jCSVExporter() + + with pytest.raises(ValidationError) as excinfo: + exporter.export({"data": [{"id": "e1"}]}, tmp_path) + + message = str(excinfo.value) + assert "data" in message + assert "entities" in message + + assert not (tmp_path / "nodes.csv").exists() + assert not (tmp_path / "relationships.csv").exists() + + +def test_records_under_an_unread_key_are_not_dropped_silently(tmp_path): + """Naming a recognized key is not enough if nothing resolves from it.""" + exporter = Neo4jCSVExporter() + + with pytest.raises(ValidationError): + exporter.export({"nodes": [], "data": [{"id": "e1"}]}, tmp_path) + + assert not (tmp_path / "nodes.csv").exists() + + +def test_malformed_collection_value_is_refused(tmp_path): + """``list("abc")`` would otherwise export one node per character.""" + exporter = Neo4jCSVExporter() + + for value in ("abc", 42, {"id": "n1"}): + with pytest.raises(ValidationError) as excinfo: + exporter.export({"nodes": value}, tmp_path) + assert "nodes" in str(excinfo.value) + + assert not (tmp_path / "nodes.csv").exists() + + +def test_graph_objects_still_use_the_attribute_path(tmp_path): + """Only mappings changed; objects are not mappings and are unaffected.""" + + class Graph: + def __init__(self): + self.nodes = [{"id": "e1", "type": "Person", "name": "Acme"}] + self.edges = [] + + exporter = Neo4jCSVExporter() + exporter.export(Graph(), tmp_path) + + assert "Acme" in (tmp_path / "nodes.csv").read_text(encoding="utf-8") diff --git a/tests/export/test_yaml_exporter_input_validation.py b/tests/export/test_yaml_exporter_input_validation.py new file mode 100644 index 00000000..13a375d7 --- /dev/null +++ b/tests/export/test_yaml_exporter_input_validation.py @@ -0,0 +1,181 @@ +"""Regression tests for YAML export input validation (issue #952). + +``export_yaml`` declared ``Union[Dict[str, Any], List[Dict[str, Any]]]`` but +both YAML exporters read their payload by key, so a list reached +``semantic_network.get(...)`` and surfaced as a bare +``AttributeError: 'list' object has no attribute 'get'`` from inside the +exporter — an error that names neither the offending argument nor the shape +expected. + +A list is rejected rather than wrapped. These formats distinguish entities +from relationships from triplets, so inferring which collection a bare list +represents would silently mislabel the records; and wrapping it under an +unrecognised key would write a structurally valid file with every collection +empty, trading a loud failure for silent data loss. + +Both directions are pinned: non-mappings raise ``ProcessingError`` with an +actionable message, and every mapping that worked before still exports. +""" + +import os +import shutil +import tempfile +import unittest +from collections import OrderedDict, defaultdict + +import yaml + +from semantica.export.methods import export_yaml +from semantica.export.yaml_exporter import ( + SemanticNetworkYAMLExporter, + YAMLSchemaExporter, +) +from semantica.utils.exceptions import ProcessingError + +# Non-mapping payloads that must be rejected. A list of dicts is the shape +# from #952; the rest guard the same path against other sequence/scalar types. +NON_MAPPINGS = { + "list_of_dicts": [{"id": "1", "name": "Acme"}], + "empty_list": [], + "tuple_of_dicts": ({"id": "1"},), + "list_of_scalars": ["a", "b"], + "string": "entities", + "bytes": b"entities", + "int": 42, + "none": None, + "set": {"a"}, +} + +# Both YAML methods, with a minimal valid payload and the key names the +# corresponding error message must mention. +METHODS = { + "semantic_network": { + "valid": { + "entities": [{"id": "1", "name": "Acme"}], + "relationships": [], + "triplets": [], + }, + "expected_key": "entities", + "top_level_key": "entities", + }, + "schema": { + "valid": {"classes": [{"name": "Thing"}], "properties": []}, + "expected_key": "classes", + "top_level_key": "classes", + }, +} + + +class TestExportYamlRejectsNonMappings(unittest.TestCase): + """Non-mapping input fails loudly, through the public wrapper.""" + + def setUp(self): + self.tmpdir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, self.tmpdir, ignore_errors=True) + + def _path(self, name="out.yaml"): + return os.path.join(self.tmpdir, name) + + def test_fixture_tables_are_populated(self): + """Guard against a vacuous suite. + + Every test below iterates a table; emptying or renaming one would let + those loops pass without asserting anything. + """ + self.assertGreaterEqual(len(NON_MAPPINGS), 9) + self.assertEqual(set(METHODS), {"semantic_network", "schema"}) + + def test_non_mapping_raises_processing_error(self): + for method in METHODS: + for label, payload in NON_MAPPINGS.items(): + with self.subTest(method=method, case=label): + with self.assertRaises(ProcessingError): + export_yaml(payload, self._path(), method=method) + + def test_error_names_the_offending_type_and_expected_keys(self): + """The message must be actionable, not just the right exception type.""" + for method, spec in METHODS.items(): + with self.subTest(method=method): + with self.assertRaises(ProcessingError) as ctx: + export_yaml([{"id": "1"}], self._path(), method=method) + message = str(ctx.exception) + self.assertIn("list", message) + self.assertIn(spec["expected_key"], message) + + def test_no_file_is_written_when_input_is_rejected(self): + """A rejected export must not leave a partial or empty artefact.""" + for method in METHODS: + with self.subTest(method=method): + path = self._path(f"{method}_rejected.yaml") + with self.assertRaises(ProcessingError): + export_yaml([{"id": "1"}], path, method=method) + self.assertFalse(os.path.exists(path)) + + def test_exporter_classes_reject_non_mappings_directly(self): + """Validation lives in the exporters, not only the convenience wrapper. + + Callers using the classes directly get the same contract. + """ + for label, payload in NON_MAPPINGS.items(): + with self.subTest(exporter="SemanticNetworkYAMLExporter", case=label): + with self.assertRaises(ProcessingError): + SemanticNetworkYAMLExporter().export_semantic_network(payload) + with self.subTest(exporter="YAMLSchemaExporter", case=label): + with self.assertRaises(ProcessingError): + YAMLSchemaExporter().export_ontology_schema(payload) + + +class TestExportYamlStillAcceptsMappings(unittest.TestCase): + """Everything that exported before must still export.""" + + def setUp(self): + self.tmpdir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, self.tmpdir, ignore_errors=True) + + def _path(self, name="out.yaml"): + return os.path.join(self.tmpdir, name) + + def _load(self, path): + with open(path, encoding="utf-8") as handle: + return yaml.safe_load(handle) + + def test_valid_mapping_exports_for_each_method(self): + for method, spec in METHODS.items(): + with self.subTest(method=method): + path = self._path(f"{method}.yaml") + export_yaml(spec["valid"], path, method=method) + self.assertTrue(os.path.exists(path)) + loaded = self._load(path) + self.assertIn(spec["top_level_key"], loaded) + + def test_semantic_network_records_survive_the_round_trip(self): + path = self._path("network.yaml") + export_yaml(METHODS["semantic_network"]["valid"], path) + loaded = self._load(path) + self.assertEqual(loaded["entities"], [{"id": "1", "name": "Acme"}]) + + def test_empty_mapping_is_still_accepted(self): + """An empty dict is a mapping; rejecting it would be a behaviour change.""" + for method in METHODS: + with self.subTest(method=method): + path = self._path(f"{method}_empty.yaml") + export_yaml({}, path, method=method) + self.assertTrue(os.path.exists(path)) + + def test_mapping_subclasses_are_accepted(self): + """Validation is by Mapping, not dict, so these must keep working.""" + valid = METHODS["semantic_network"]["valid"] + subclasses = { + "OrderedDict": OrderedDict(valid), + "defaultdict": defaultdict(list, valid), + } + for label, payload in subclasses.items(): + with self.subTest(case=label): + path = self._path(f"{label}.yaml") + export_yaml(payload, path) + loaded = self._load(path) + self.assertEqual(loaded["entities"], valid["entities"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/export/test_yaml_exporter_key_recognition.py b/tests/export/test_yaml_exporter_key_recognition.py new file mode 100644 index 00000000..d7dc2839 --- /dev/null +++ b/tests/export/test_yaml_exporter_key_recognition.py @@ -0,0 +1,434 @@ +"""Tests for YAML export key recognition (issue #953). + +``SemanticNetworkYAMLExporter`` built its output from ``.get(key, [])`` +lookups, so a mapping keyed by anything it did not read -- an ``export_json`` +envelope, a typo'd 'entitys', ``ContextGraph.to_dict()``'s 'nodes'/'edges' -- +serialized to a structurally valid file with every collection empty. Nothing +signalled the loss: no exception, no warning, and the progress log reported a +completed export. ``YAMLSchemaExporter`` had the same defect over a different +key set. + +The exporters are run for real rather than mocked, and the written files are +parsed back, since the behaviour under test is what actually lands on disk. +""" + +from pathlib import Path + +import pytest +import yaml + +from semantica.context.context_graph import ContextGraph +from semantica.export.methods import export_json, export_yaml +from semantica.export.yaml_exporter import ( + SemanticNetworkYAMLExporter, + YAMLSchemaExporter, +) +from semantica.utils.exceptions import ProcessingError, ValidationError + +ENTITIES = [{"id": "e1", "name": "Acme"}, {"id": "e2", "name": "Beta"}] +RELATIONSHIPS = [{"id": "r1", "source": "e1", "target": "e2", "type": "PARTNER"}] +TRIPLETS = [{"subject": "e1", "predicate": "partner_of", "object": "e2"}] + + +def _load(path): + with open(path, "r", encoding="utf-8") as handle: + return yaml.safe_load(handle) + + +class TestSemanticNetworkKeyRecognition: + """An unrecognized mapping is refused instead of silently emptied.""" + + def test_export_json_envelope_is_rejected(self, tmp_path): + """The realistic trigger: re-exporting an export_json payload. + + ``export_json`` wraps records as ``{"data": [...], "count": N, + "metadata": {...}}``. Feeding that straight to ``export_yaml`` used to + write a file with every record gone. Note the envelope's 'metadata' + key is deliberately not enough to make the payload recognized -- + treating it as sufficient would readmit exactly this case. + """ + json_path = tmp_path / "records.json" + export_json(ENTITIES, json_path) + envelope = yaml.safe_load(json_path.read_text(encoding="utf-8")) + assert "data" in envelope and "metadata" in envelope + + yaml_path = tmp_path / "records.yaml" + with pytest.raises(ValidationError) as excinfo: + export_yaml(envelope, yaml_path) + + message = str(excinfo.value) + assert "'data'" in message, "error should name the supplied keys" + assert "'entities'" in message, "error should name the expected keys" + assert not yaml_path.exists(), "a rejected export must write nothing" + + @pytest.mark.parametrize( + "payload", + [ + {"records": ENTITIES}, + {"entitys": ENTITIES}, + {"data": ENTITIES}, + {"metadata": {"source": "test"}}, + ], + ids=["records", "typo", "data", "metadata-only"], + ) + def test_unrecognized_mappings_are_rejected(self, payload): + exporter = SemanticNetworkYAMLExporter() + with pytest.raises(ValidationError): + exporter.export_semantic_network(payload) + + @pytest.mark.parametrize( + "payload", + [ + {"entities": [], "data": ENTITIES}, + {"nodes": [], "edges": [], "records": ENTITIES}, + {"triplets": [], "data": ENTITIES, "metadata": {"source": "test"}}, + ], + ids=["entities-empty", "nodes-edges-empty", "triplets-empty"], + ) + def test_recognized_but_empty_with_records_elsewhere_is_rejected(self, payload): + """Presence of a recognized key is not proof the records survived. + + ``{"entities": [], "data": [...]}`` clears a presence-only check and + still resolves to empty, dropping everything under 'data' -- the same + silent-empty export by a narrower route. + """ + exporter = SemanticNetworkYAMLExporter() + with pytest.raises(ValidationError) as excinfo: + exporter.export_semantic_network(payload) + + message = str(excinfo.value) + assert "holds records" in message + assert "'entities'" in message, "error should name where records belong" + + def test_empty_graph_with_non_record_keys_still_exports(self, tmp_path): + """The rejection must key on dropped *records*, not on unread keys. + + ``ContextGraph.to_dict()`` always carries a populated 'statistics' + dict, so an empty graph would be refused if any unread key counted. + """ + graph = ContextGraph() + path = tmp_path / "empty_graph.yaml" + export_yaml(graph.to_dict(), path) + + written = _load(path) + assert written["entities"] == [] + assert written["relationships"] == [] + + def test_empty_mapping_still_exports(self, tmp_path): + """An empty graph is legitimate and carries nothing that could be lost.""" + path = tmp_path / "empty.yaml" + export_yaml({}, path) + + written = _load(path) + assert written["entities"] == [] + assert written["relationships"] == [] + assert written["triplets"] == [] + + def test_recognized_keys_still_export(self, tmp_path): + path = tmp_path / "network.yaml" + export_yaml( + { + "entities": ENTITIES, + "relationships": RELATIONSHIPS, + "triplets": TRIPLETS, + "metadata": {"source": "test"}, + }, + path, + ) + + written = _load(path) + assert written["entities"] == ENTITIES + assert written["relationships"] == RELATIONSHIPS + assert written["triplets"] == TRIPLETS + assert written["metadata"]["source"] == "test" + + def test_nodes_edges_alias_exports_records(self, tmp_path): + path = tmp_path / "aliased.yaml" + export_yaml({"nodes": ENTITIES, "edges": RELATIONSHIPS}, path) + + written = _load(path) + assert written["entities"] == ENTITIES + assert written["relationships"] == RELATIONSHIPS + + def test_context_graph_to_dict_round_trips(self, tmp_path): + """The most direct path from this library's own graph type to YAML. + + Built from a real ``ContextGraph`` rather than a hand-written + 'nodes'/'edges' dict, so the test breaks if ``to_dict()`` changes + vocabulary. + """ + graph = ContextGraph() + graph.add_node("n1", node_type="Person", content="Alice") + graph.add_node("n2", node_type="Org", content="Acme") + graph.add_edge("n1", "n2", "WORKS_FOR") + + path = tmp_path / "context.yaml" + export_yaml(graph.to_dict(), path) + + written = _load(path) + assert len(written["entities"]) == 2 + assert len(written["relationships"]) == 1 + + def test_conflicting_spellings_are_refused(self): + """Two populated spellings of one collection: no basis to pick either.""" + exporter = SemanticNetworkYAMLExporter() + with pytest.raises(ValidationError): + exporter.export_semantic_network( + {"entities": ENTITIES, "nodes": [{"id": "other"}]} + ) + + def test_non_mapping_raises_processing_error(self): + """A wrong type is a different failure from a wrong-keyed mapping. + + ProcessingError says the object cannot be exported at all; + ValidationError says the mapping's contents are unusable. Pinned here + so the two do not quietly converge. + """ + exporter = SemanticNetworkYAMLExporter() + with pytest.raises(ProcessingError): + exporter.export_semantic_network(ENTITIES) + + def test_rejected_export_creates_no_output_directory(self, tmp_path): + """Validation runs before the output directory is created.""" + target = tmp_path / "nested" / "out.yaml" + exporter = SemanticNetworkYAMLExporter() + + with pytest.raises(ValidationError): + exporter.export({"data": ENTITIES}, target) + + assert not target.parent.exists() + + +class TestPipelineExportKeyRecognition: + """export_for_pipeline read the same defaulted lookups, so it had the bug too.""" + + def test_unrecognized_mapping_is_rejected(self): + exporter = SemanticNetworkYAMLExporter() + with pytest.raises(ValidationError): + exporter.export_for_pipeline({"data": ENTITIES}) + + def test_non_mapping_raises_processing_error(self): + exporter = SemanticNetworkYAMLExporter() + with pytest.raises(ProcessingError): + exporter.export_for_pipeline(ENTITIES) + + def test_aliases_resolve_into_the_semantic_network(self): + exporter = SemanticNetworkYAMLExporter() + written = yaml.safe_load( + exporter.export_for_pipeline({"nodes": ENTITIES, "edges": RELATIONSHIPS}) + ) + + assert written["semantic_network"]["entities"] == ENTITIES + assert written["semantic_network"]["relationships"] == RELATIONSHIPS + + def test_metadata_is_preserved(self): + exporter = SemanticNetworkYAMLExporter() + written = yaml.safe_load( + exporter.export_for_pipeline( + {"entities": ENTITIES, "metadata": {"source": "test"}} + ) + ) + + assert written["metadata"]["source"] == "test" + assert written["semantic_network"]["entities"] == ENTITIES + + +class TestSchemaKeyRecognition: + """method="schema" emitted empty classes/properties/namespaces the same way.""" + + def test_unrecognized_mapping_is_rejected(self, tmp_path): + path = tmp_path / "schema.yaml" + with pytest.raises(ValidationError) as excinfo: + export_yaml({"nodes": [{"id": "1"}]}, path, method="schema") + + message = str(excinfo.value) + assert "'nodes'" in message + assert "'classes'" in message + assert not path.exists() + + def test_non_mapping_raises_processing_error(self): + exporter = YAMLSchemaExporter() + with pytest.raises(ProcessingError): + exporter.export_ontology_schema([{"id": "1"}]) + + def test_recognized_but_empty_with_records_elsewhere_is_rejected(self): + """The schema path had the same presence-only hole.""" + exporter = YAMLSchemaExporter() + with pytest.raises(ValidationError) as excinfo: + exporter.export_ontology_schema({"classes": [], "nodes": [{"id": "1"}]}) + + assert "holds records" in str(excinfo.value) + + def test_ontology_metadata_without_records_still_exports(self): + """A schema described only by its identity is not a dropped export.""" + exporter = YAMLSchemaExporter() + written = yaml.safe_load( + exporter.export_ontology_schema( + {"uri": "http://example.org/o", "classes": []} + ) + ) + + assert written["ontology"]["uri"] == "http://example.org/o" + assert written["classes"] == [] + + def test_empty_mapping_still_exports(self, tmp_path): + path = tmp_path / "schema.yaml" + export_yaml({}, path, method="schema") + + written = _load(path) + assert written["classes"] == [] + assert written["properties"] == [] + assert written["namespaces"] == {} + + @pytest.mark.parametrize( + "payload", + [ + {"classes": ["Person"], "properties": ["WORKS_FOR"]}, + {"namespaces": {"ex": "http://example.org/"}}, + {"uri": "http://example.org/ontology"}, + ], + ids=["classes-properties", "namespaces-only", "uri-only"], + ) + def test_recognized_keys_still_export(self, payload, tmp_path): + path = tmp_path / "schema.yaml" + export_yaml(payload, path, method="schema") + + written = _load(path) + assert written["classes"] == payload.get("classes", []) + assert written["properties"] == payload.get("properties", []) + assert written["ontology"]["uri"] == payload.get("uri", "") + + # ── Fix regression: scalar recognized keys must not short-circuit the ── + # ── dropped-records check (version, uri, title, description). ────────── + + @pytest.mark.parametrize( + "scalar_key, scalar_value", + [ + ("version", "1.0"), + ("uri", "http://example.org/ontology"), + ("title", "My Ontology"), + ("description", "A test ontology"), + ], + ids=["version", "uri", "title", "description"], + ) + def test_scalar_recognized_key_does_not_excuse_records_under_unread_key( + self, scalar_key, scalar_value + ): + """A truthy scalar such as version='1.0' must not silence the dropped- + records check. Before the fix, any truthy value from _SCHEMA_KEYS + would make _require_nothing_dropped believe something resolved and + return early, silently discarding a list under an unread key. + """ + exporter = YAMLSchemaExporter() + with pytest.raises(ValidationError) as excinfo: + exporter.export_ontology_schema( + {scalar_key: scalar_value, "nodes": [{"id": "c1"}]} + ) + assert "holds records" in str(excinfo.value), str(excinfo.value) + + def test_valid_classes_with_scalar_metadata_is_accepted(self): + """classes/properties populated alongside version/uri must still work.""" + exporter = YAMLSchemaExporter() + written = yaml.safe_load( + exporter.export_ontology_schema( + { + "classes": [{"id": "Person"}], + "properties": [{"id": "name"}], + "version": "2.0", + "uri": "http://example.org/o", + } + ) + ) + assert written["classes"] == [{"id": "Person"}] + assert written["properties"] == [{"id": "name"}] + assert written["ontology"]["version"] == "2.0" + assert written["ontology"]["uri"] == "http://example.org/o" + + +class TestFailureIsObservable: + """The complaint in #953 was that the logs affirmatively reported success.""" + + def test_no_success_is_logged_for_a_rejected_export(self, tmp_path, caplog): + path = tmp_path / "out.yaml" + + with caplog.at_level("DEBUG"): + with pytest.raises(ValidationError): + export_yaml({"data": ENTITIES}, path) + + assert "Exported YAML to" not in caplog.text + assert any( + record.levelname in ("WARNING", "ERROR", "CRITICAL") + for record in caplog.records + ), "a rejected export should leave something at warning or above" + + +class _RecordingTracker: + """Records the exporter's own progress calls, which are what is under test.""" + + def __init__(self): + self.stopped = [] + self._next_id = 0 + + def start_tracking(self, **kwargs): + self._next_id += 1 + return str(self._next_id) + + def update_tracking(self, tracking_id, **kwargs): + pass + + def stop_tracking(self, tracking_id, status=None, message=None): + self.stopped.append((status, message)) + + +class TestProgressReflectsTheWrite: + """Serialization completing is not the same as the file landing on disk.""" + + def test_failed_write_is_not_reported_as_completed(self, tmp_path): + """A write failure after serialization must not leave a clean tracker. + + The path's parent is an existing *file*, so directory creation fails + after `export_semantic_network` has already reported its own + completion. + """ + blocker = tmp_path / "blocker" + blocker.write_text("not a directory", encoding="utf-8") + target = blocker / "nested" / "out.yaml" + + exporter = SemanticNetworkYAMLExporter() + tracker = _RecordingTracker() + exporter.progress_tracker = tracker + + with pytest.raises(OSError): + exporter.export({"entities": ENTITIES}, target) + + assert not target.exists() + statuses = [status for status, _ in tracker.stopped] + assert "failed" in statuses, f"write failure went unreported: {tracker.stopped}" + assert not any( + status == "completed" and "Exported YAML" in (message or "") + for status, message in tracker.stopped + ), "no span may claim a completed export when nothing was written" + + def test_successful_write_is_reported_as_completed(self, tmp_path): + target = tmp_path / "out.yaml" + exporter = SemanticNetworkYAMLExporter() + tracker = _RecordingTracker() + exporter.progress_tracker = tracker + + exporter.export({"entities": ENTITIES}, target) + + assert target.exists() + assert all(status == "completed" for status, _ in tracker.stopped) + assert any( + "Exported YAML" in (message or "") for _, message in tracker.stopped + ), "the write should report its own completion, not just serialization" + + +class TestUnaffectedExporters: + """export_json's own behaviour is untouched -- only the YAML path changed.""" + + def test_export_json_still_accepts_a_bare_list(self, tmp_path): + path = tmp_path / "records.json" + export_json(ENTITIES, path) + + assert Path(path).exists() diff --git a/tests/utils/test_normalize_graph_payload.py b/tests/utils/test_normalize_graph_payload.py new file mode 100644 index 00000000..02b53219 --- /dev/null +++ b/tests/utils/test_normalize_graph_payload.py @@ -0,0 +1,487 @@ +"""Tests for the shared graph-payload normalizer (issue #956). + +Graph payloads circulate under two vocabularies -- 'entities'/'relationships' +and 'nodes'/'edges' -- and consumers each reconciled them locally with at +least three competing idioms. The same payload could therefore be exported, +silently dropped, or rejected depending on which consumer read it: +``export_lpg`` dropped every entity when 'nodes' was present but empty, which +is precisely the shape ``JSONExporter`` emits. + +The end-to-end assertions run the real exporters rather than mocking them, +since the behaviour under test is that the exporters now agree. +""" + +import os +import shutil +import tempfile +import unittest +from dataclasses import dataclass + +from semantica.export import methods as export_methods +from semantica.utils import normalize_graph_payload +from semantica.utils.exceptions import ValidationError + +ENTITY = {"id": "e1", "name": "Acme"} +RELATIONSHIP = {"id": "r1", "source": "e1", "target": "e2"} + + +class TestVocabularyResolution(unittest.TestCase): + def test_canonical_keys_pass_through(self): + result = normalize_graph_payload( + {"entities": [ENTITY], "relationships": [RELATIONSHIP]} + ) + self.assertEqual(result["entities"], [ENTITY]) + self.assertEqual(result["relationships"], [RELATIONSHIP]) + self.assertEqual(result["triplets"], []) + + def test_aliases_are_mapped_to_canonical_keys(self): + result = normalize_graph_payload({"nodes": [ENTITY], "edges": [RELATIONSHIP]}) + self.assertEqual(result["entities"], [ENTITY]) + self.assertEqual(result["relationships"], [RELATIONSHIP]) + + def test_empty_alias_does_not_mask_a_populated_canonical_key(self): + """The JSONExporter round-trip shape, and the #956 data-loss case.""" + result = normalize_graph_payload( + {"entities": [ENTITY], "nodes": [], "relationships": [], "edges": []} + ) + self.assertEqual(result["entities"], [ENTITY]) + + def test_empty_canonical_key_does_not_mask_a_populated_alias(self): + result = normalize_graph_payload({"entities": [], "nodes": [ENTITY]}) + self.assertEqual(result["entities"], [ENTITY]) + + def test_identical_spellings_are_accepted(self): + result = normalize_graph_payload({"entities": [ENTITY], "nodes": [ENTITY]}) + self.assertEqual(result["entities"], [ENTITY]) + + def test_conflicting_spellings_are_refused(self): + """No basis to prefer either, and picking one would lose the other.""" + with self.assertRaises(ValidationError) as ctx: + normalize_graph_payload( + {"entities": [ENTITY], "nodes": [{"id": "different"}]} + ) + message = str(ctx.exception) + self.assertIn("entities", message) + self.assertIn("nodes", message) + + def test_reordered_identical_spellings_are_accepted(self): + """Same records, different order, is not a conflict. + + A caller round-tripping through a dict-keyed cache or a set has no + reason to preserve list order; comparing spellings with plain list + equality rejected this as if the records differed. + """ + other = {"id": "e2", "name": "Beta"} + result = normalize_graph_payload( + {"entities": [ENTITY, other], "nodes": [other, ENTITY]} + ) + self.assertCountEqual(result["entities"], [ENTITY, other]) + + def test_reordered_spellings_with_duplicate_records_still_conflict(self): + """Multiset comparison must still catch a real count mismatch.""" + with self.assertRaises(ValidationError): + normalize_graph_payload({"entities": [ENTITY, ENTITY], "nodes": [ENTITY]}) + + def test_triplets_are_carried_through(self): + result = normalize_graph_payload({"triplets": [{"s": "a", "p": "b", "o": "c"}]}) + self.assertEqual(result["triplets"], [{"s": "a", "p": "b", "o": "c"}]) + + def test_missing_collections_default_to_empty_lists(self): + result = normalize_graph_payload({"entities": [ENTITY]}) + self.assertEqual(result["relationships"], []) + self.assertEqual(result["triplets"], []) + + def test_result_does_not_alias_the_input_collections(self): + payload = {"entities": [ENTITY]} + result = normalize_graph_payload(payload) + result["entities"].append({"id": "e2"}) + self.assertEqual(len(payload["entities"]), 1) + + +class TestUnrecognizedInput(unittest.TestCase): + def test_unrecognized_keys_raise_by_default(self): + for payload in ({"data": [ENTITY]}, {"records": [ENTITY]}, {"foo": "bar"}): + with self.subTest(payload=payload): + with self.assertRaises(ValidationError): + normalize_graph_payload(payload) + + def test_error_names_supplied_and_expected_keys(self): + with self.assertRaises(ValidationError) as ctx: + normalize_graph_payload({"data": [ENTITY]}) + message = str(ctx.exception) + self.assertIn("data", message) + self.assertIn("entities", message) + self.assertIn("nodes", message) + + def test_empty_mapping_is_accepted(self): + """An empty graph is legitimate and carries nothing that could be lost.""" + result = normalize_graph_payload({}) + self.assertEqual(result, {"entities": [], "relationships": [], "triplets": []}) + + def test_non_mapping_input_raises(self): + for payload in ([ENTITY], (ENTITY,), "entities", 42, None): + with self.subTest(payload=repr(payload)): + with self.assertRaises(ValidationError): + normalize_graph_payload(payload) + + +class TestExportersAgree(unittest.TestCase): + """The divergence from #956, run against the real exporters.""" + + # export_csv is excluded: it writes entities/relationships/nodes/edges to + # four separate files by design, so it is not resolving two spellings of + # one collection and is out of scope for this change. + EXPORTERS = ("export_json", "export_arango", "export_neo4j_csv", "export_lpg") + + def setUp(self): + self.tmpdir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, self.tmpdir, ignore_errors=True) + + def _export_and_read(self, name, payload): + outdir = os.path.join(self.tmpdir, name) + os.makedirs(outdir, exist_ok=True) + getattr(export_methods, name)(payload, os.path.join(outdir, "out")) + blob = "" + for root, _, files in os.walk(outdir): + for filename in files: + with open(os.path.join(root, filename), errors="ignore") as handle: + blob += handle.read() + return blob + + def test_exporter_list_is_populated(self): + """Guard against a vacuous suite if the list is emptied.""" + self.assertGreaterEqual(len(self.EXPORTERS), 4) + + def test_every_exporter_keeps_records_when_an_alias_is_empty(self): + payload = { + "entities": [ENTITY], + "nodes": [], + "relationships": [], + "edges": [], + } + for name in self.EXPORTERS: + with self.subTest(exporter=name): + self.assertIn( + "Acme", + self._export_and_read(name, payload), + f"{name} dropped the entity when 'nodes' was present but empty", + ) + + def test_every_exporter_accepts_the_alias_vocabulary(self): + payload = {"nodes": [ENTITY], "edges": []} + for name in self.EXPORTERS: + with self.subTest(exporter=name): + self.assertIn( + "Acme", + self._export_and_read(name, payload), + f"{name} dropped the entity supplied as 'nodes'", + ) + + def test_every_exporter_raises_processing_error_for_non_mapping_input(self): + """A wrong-type payload is rejected the same way everywhere. + + export_yaml and export_neo4j_csv raised ProcessingError for a bare + list; export_lpg and export_arango called normalize_graph_payload() + directly with no type guard, so they alone raised ValidationError + (from inside the resolver) for the identical mistake. + """ + from semantica.utils.exceptions import ProcessingError + + for name in ("export_arango", "export_neo4j_csv", "export_lpg"): + with self.subTest(exporter=name): + outdir = os.path.join(self.tmpdir, name + "_bad_type") + os.makedirs(outdir, exist_ok=True) + with self.assertRaises(ProcessingError): + getattr(export_methods, name)([ENTITY], os.path.join(outdir, "out")) + + def test_every_exporter_converts_object_shaped_records(self): + """A dataclass record must not merely pass validation. + + normalize_graph_payload() accepts dataclass/attribute-bearing + records (Neo4jCSVExporter reads them off attributes), but + export_lpg and export_arango read records with ``.get(...)``. A + record that passed validation unconverted crashed with a raw + AttributeError once used -- the exact failure the boundary exists + to prevent. + """ + + @dataclass + class Node: + id: str + name: str + + payload = {"entities": [Node(id="e1", name="Acme")], "relationships": []} + for name in ("export_arango", "export_neo4j_csv", "export_lpg"): + with self.subTest(exporter=name): + self.assertIn("Acme", self._export_and_read(name, payload)) + + def test_neo4j_accepts_non_dict_mappings(self): + """Neo4jCSVExporter's mapping path must not be narrower than the rest. + + _normalize_graph checked isinstance(graph, dict), so a non-dict + Mapping (a MappingProxyType, a ChainMap) fell into the + object-attribute branch and was rejected as an unrecognized object, + even though the identical payload exports fine via LPG/Arango/YAML. + """ + import types + + payload = types.MappingProxyType({"entities": [ENTITY], "relationships": []}) + self.assertIn("Acme", self._export_and_read("export_neo4j_csv", payload)) + + +class TestRecordsCannotBeDroppedSilently(unittest.TestCase): + """Presence of a recognized key is not proof the records survived. + + ``{"entities": [], "data": [...]}`` clears a presence-only check and still + resolves to empty, so the records under 'data' would be dropped with no + signal -- the same failure the recognition check exists to prevent. + """ + + def test_empty_recognized_key_does_not_excuse_records_elsewhere(self): + with self.assertRaises(ValidationError) as ctx: + normalize_graph_payload({"entities": [], "data": [ENTITY]}) + + message = str(ctx.exception) + self.assertIn("'data'", message) + self.assertIn("holds records", message) + + def test_check_applies_to_every_recognized_spelling(self): + for key in ("entities", "nodes", "relationships", "edges", "triplets"): + with self.subTest(key=key): + with self.assertRaises(ValidationError): + normalize_graph_payload({key: [], "records": [ENTITY]}) + + def test_non_record_keys_are_not_mistaken_for_dropped_records(self): + """ContextGraph.to_dict() always carries 'statistics'. + + An empty graph must stay exportable, so only a non-empty list counts + as evidence that records were dropped. + """ + result = normalize_graph_payload( + {"nodes": [], "edges": [], "statistics": {"node_count": 0}} + ) + + self.assertEqual(result["entities"], []) + self.assertEqual(result["relationships"], []) + + def test_records_alongside_a_populated_collection_are_not_refused(self): + """Something resolved, so the export is not silently empty.""" + result = normalize_graph_payload({"entities": [ENTITY], "statistics": {"n": 1}}) + + self.assertEqual(result["entities"], [ENTITY]) + + +class TestCollectionValuesAreValidated(unittest.TestCase): + """A recognized key is not proof its value is a collection of records. + + Resolving on truthiness alone let ``{"entities": "abc"}`` through as three + single-character "records" and let ``{"entities": 42}`` surface as a raw + ``TypeError`` from ``list()`` inside an exporter, naming the exporter + rather than the payload key at fault. Both are rejected here, at the + boundary that owns the question. + """ + + COLLECTION_KEYS = ("entities", "nodes", "relationships", "edges", "triplets") + + # Every public export path that reads its payload through the normalizer. + # export_json is excluded: it treats the payload as opaque records rather + # than resolving graph collections, so it never calls the normalizer. + NORMALIZING_EXPORTERS = ( + "export_arango", + "export_neo4j_csv", + "export_lpg", + "export_yaml", + ) + + def test_string_value_is_not_treated_as_a_collection(self): + for key in self.COLLECTION_KEYS: + with self.subTest(key=key): + with self.assertRaises(ValidationError) as ctx: + normalize_graph_payload({key: "abc"}) + message = str(ctx.exception) + self.assertIn(f"'{key}'", message) + self.assertIn("str", message) + + def test_bytes_value_is_not_treated_as_a_collection(self): + for value in (b"abc", bytearray(b"abc")): + with self.subTest(value=repr(value)): + with self.assertRaises(ValidationError): + normalize_graph_payload({"entities": value}) + + def test_scalar_value_raises_validation_error_not_type_error(self): + for key in self.COLLECTION_KEYS: + for value in (42, 3.5, True, object()): + with self.subTest(key=key, value=repr(value)): + with self.assertRaises(ValidationError) as ctx: + normalize_graph_payload({key: value}) + self.assertIn(f"'{key}'", str(ctx.exception)) + + def test_mapping_value_is_not_treated_as_a_collection(self): + """``{"nodes": {"id": "n1"}}`` -- a single record, or an ID index.""" + for payload in ( + {"nodes": {"id": "n1"}}, + {"entities": {"e1": ENTITY}}, + {"edges": {"id": "r1"}}, + ): + with self.subTest(payload=payload): + with self.assertRaises(ValidationError) as ctx: + normalize_graph_payload(payload) + self.assertIn("mapping", str(ctx.exception)) + + def test_non_record_elements_are_rejected(self): + for value in (["Acme"], [ENTITY, "Acme"], [42], [None], [[ENTITY]]): + with self.subTest(value=repr(value)): + with self.assertRaises(ValidationError) as ctx: + normalize_graph_payload({"entities": value}) + self.assertIn("'entities'", str(ctx.exception)) + + def test_error_names_the_offending_index(self): + with self.assertRaises(ValidationError) as ctx: + normalize_graph_payload({"entities": [ENTITY, ENTITY, "Acme"]}) + self.assertIn("index 2", str(ctx.exception)) + + def test_object_records_are_accepted(self): + """Attribute-bearing objects are accepted and converted to dicts. + + LPGExporter and ArangoAQLExporter read records with ``.get(...)``, so + an object record that merely passed validation unconverted would + still crash with AttributeError once used; the boundary converts it. + """ + + class Node: + def __init__(self): + self.id = "e1" + self.name = "Acme" + + node = Node() + result = normalize_graph_payload({"entities": [node]}) + self.assertEqual(result["entities"], [{"id": "e1", "name": "Acme"}]) + + def test_dataclass_records_are_accepted(self): + @dataclass + class Node: + id: str + + node = Node(id="e1") + result = normalize_graph_payload({"entities": [node]}) + self.assertEqual(result["entities"], [{"id": "e1"}]) + + def test_tuple_collections_are_accepted_and_materialized(self): + result = normalize_graph_payload({"entities": (ENTITY,)}) + self.assertEqual(result["entities"], [ENTITY]) + + def test_none_is_read_as_an_absent_collection(self): + """JSON round-trips an absent collection to null.""" + result = normalize_graph_payload( + {"entities": None, "relationships": [RELATIONSHIP]} + ) + self.assertEqual(result["entities"], []) + self.assertEqual(result["relationships"], [RELATIONSHIP]) + + def test_null_collection_still_cannot_hide_dropped_records(self): + with self.assertRaises(ValidationError): + normalize_graph_payload({"entities": None, "data": [ENTITY]}) + + def test_every_spelling_is_validated_not_just_the_winner(self): + """A malformed alias is a defect even when the canonical key resolves.""" + with self.assertRaises(ValidationError) as ctx: + normalize_graph_payload({"entities": [ENTITY], "nodes": "abc"}) + self.assertIn("'nodes'", str(ctx.exception)) + + def test_malformed_value_reaches_no_exporter(self): + """The end-to-end half: no exporter sees a TypeError from list().""" + tmpdir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, tmpdir, ignore_errors=True) + + for name in self.NORMALIZING_EXPORTERS: + for value in ("abc", 42, {"id": "n1"}): + with self.subTest(exporter=name, value=repr(value)): + outdir = os.path.join(tmpdir, f"{name}_{type(value).__name__}") + os.makedirs(outdir, exist_ok=True) + with self.assertRaises(ValidationError): + getattr(export_methods, name)( + {"entities": value}, os.path.join(outdir, "out") + ) + + +class TestIsRecordBoundary(unittest.TestCase): + """_is_record gates the validation boundary introduced by this PR. + + Modules and class/type objects carry ``__dict__`` but are not graph + records. Passing them through previously produced ``AttributeError`` + inside exporters rather than a ``ValidationError`` at the boundary. + """ + + def test_python_module_in_entities_raises_validation_error(self): + """import math; {"entities": [math]} must be rejected at the boundary.""" + import math + + with self.assertRaises(ValidationError) as ctx: + normalize_graph_payload({"entities": [math]}) + self.assertIn("'entities'", str(ctx.exception)) + + def test_class_object_in_entities_raises_validation_error(self): + """A class (type object) is not a graph record.""" + + class MyNode: + pass + + with self.assertRaises(ValidationError) as ctx: + normalize_graph_payload({"entities": [MyNode]}) + self.assertIn("'entities'", str(ctx.exception)) + + def test_user_defined_instance_with_attributes_is_accepted(self): + """Attribute-bearing instances are the legitimate use-case, converted + to a dict so every exporter -- not just Neo4jCSVExporter -- can read + it with ``.get(...)``.""" + + class Node: + def __init__(self): + self.id = "n1" + self.name = "Alice" + + node = Node() + result = normalize_graph_payload({"entities": [node]}) + self.assertEqual(result["entities"], [{"id": "n1", "name": "Alice"}]) + + def test_dataclass_instance_is_accepted(self): + """Dataclasses are a common record type used by Neo4jCSVExporter, + converted to a dict at the boundary so LPGExporter and + ArangoAQLExporter can read it too.""" + node = dataclass_node() + result = normalize_graph_payload({"entities": [node]}) + self.assertEqual(result["entities"], [{"id": "dc1"}]) + + def test_mapping_record_is_accepted(self): + """Plain dicts are the canonical record shape.""" + result = normalize_graph_payload({"entities": [ENTITY]}) + self.assertEqual(result["entities"], [ENTITY]) + + def test_module_rejected_through_normalizing_exporter(self): + """End-to-end: a module element must not reach an exporter's internals.""" + import math + + tmpdir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, tmpdir, ignore_errors=True) + + for name in ("export_arango", "export_neo4j_csv", "export_lpg"): + with self.subTest(exporter=name): + outdir = os.path.join(tmpdir, name) + os.makedirs(outdir, exist_ok=True) + with self.assertRaises(ValidationError): + getattr(export_methods, name)( + {"entities": [math]}, os.path.join(outdir, "out") + ) + + +@dataclass +class _DataclassNode: + id: str + + +def dataclass_node(): + return _DataclassNode(id="dc1") + + +if __name__ == "__main__": + unittest.main()