Commit 917a8fa
refactor(migration_v5): extract ontology-agnostic artifact pipeline (#172)
* refactor(migration_v5): extract ontology-agnostic artifact pipeline
The migration v5 demo's TTL → binding/DDL/property-graph
pipeline was hard-coded to MAKO. The transformations it
applied (FILL_IN resolution, cross-namespace drop, inheritance
strip) are general-purpose; only the config (TTL path,
namespace IRI, entity allowlist, annotation prefix, graph
name) is per-ontology.
## What ships
- **``ontology_artifacts.py``** — new generic module.
``OntologyConfig`` dataclass + ``load_ontology``,
``make_binding``, ``make_table_ddl``, ``make_property_graph_sql``,
``regenerate_snapshots`` — all parameterized by config. The
three OWL TTL normalizations stay (they apply to any
reasonable ontology). Annotation keys use
``{config.annotation_prefix}:`` instead of a hard-coded
``mako_demo:``.
- **``mako_artifacts.py``** — now a thin shim (~200 lines,
down from 751). Defines ``MAKO_CONFIG`` and re-exports the
public API (``regenerate_snapshots``, ``load_mako_ontology``,
``make_binding``, …) with that config bound in. The
notebook's ``import mako_artifacts;
mako_artifacts.regenerate_snapshots(project, dataset)``
works unchanged.
- **``example_ontologies/simple_request_flow.ttl``** + config
— tiny domain-neutral TTL (3 entities, 2 relationships) and
matching ``OntologyConfig``. Proves the generic pipeline
isn't accidentally MAKO-coupled. No runnable agent — smoke
fixture only.
- **``tests/test_migration_v5_ontology_artifacts.py``** —
eight tests:
* MAKO regeneration produces byte-identical output to the
checked-in snapshots (guards against drift).
* Generic pipeline accepts the Simple Request Flow config
and produces a valid binding + DDL + property graph.
* Per-entity PK naming convention applies to non-MAKO
configs.
* Graph name and annotation prefix are honored per-config.
* ``OntologyConfig`` is frozen + path properties work.
## Doc changes
- **``examples/migration_v5/README.md``** — leads with the
pluggable contract: what ``OntologyConfig`` packages, the
three transformations, the two example ontologies. The
MAKO-specific design rationale stays but is reframed as
"Reference example: MAKO design decisions". Authorship
table updated to call out ``ontology_artifacts.py`` as the
generic pipeline and the new ``example_ontologies/``
directory.
- **``periodic_materialization/README.md``** — four MAKO
references softened to "the demo's" / "the bound
ontology" / "the migration v5 demo (MAKO is the canonical
reference example, but the pipeline is ontology-agnostic)".
## What does NOT change
- ``mako_core.ttl`` — unchanged.
- ``mako_demo_agent.py``, ``run_agent.py``,
``reference_extractor.py`` — unchanged. The agent's tools
mirror MAKO's decision flow; generalizing them would mean a
different demo, not a generalization of this one.
- ``examples/migration_v5_demo_notebook.ipynb`` — unchanged.
The shim preserves the notebook's import.
- Checked-in ``ontology.yaml`` / ``binding.yaml`` /
``table_ddl.sql`` / ``property_graph.sql`` for MAKO —
byte-identical after the refactor (asserted by
``test_mako_regenerate_matches_checked_in_snapshots``).
- SDK source under ``src/`` — untouched. This is an examples
refactor only.
## Verification
- ``PYTHONPATH=src pytest tests/test_migration_v5_ontology_artifacts.py
tests/test_migration_v5_reference_extractor.py`` — 25/25 pass
(8 new + 17 existing).
- ``PYTHONPATH=src python examples/migration_v5/mako_artifacts.py``
— runs; ``git diff`` on the four MAKO snapshot files is empty.
- ``pyink --check`` + ``isort --check-only`` clean on all
touched + new files.
* fix(migration_v5): bind declared owl:hasKey PK (PR #172 review)
Addresses two findings from the PR #172 review:
## P1 — owl:hasKey ontologies broke make_binding
The generic pipeline's first cut hard-coded ``"id"`` as the
bound primary-key property name. That worked for MAKO (where
the FILL_IN resolver synthesizes an ``id`` property on every
entity) but broke any TTL that declared ``owl:hasKey``: the
binding ended up declaring an ``id`` property the entity
didn't have, and ``load_binding_from_string`` raised
``Entity binding 'X': property 'id' is not declared on this
element``. Hard-failed the PR's "any OWL TTL" contract.
Fix in ``ontology_artifacts.py``:
* New ``_primary_key_property_name(entity)`` helper reads
``entity.keys.primary[0]`` (the normalization passes
populate this for every entity — either the synthesized
``id`` or the TTL-declared key).
* ``make_binding`` uses it instead of ``"id"`` when emitting
the PK property + when skipping the PK in the property
iteration loop.
* ``make_property_graph_sql`` builds an entity → PK property
name lookup from the ontology, then resolves the PK column
through that name (not by matching ``p.name == "id"``).
The fix preserves MAKO byte-identical output (MAKO entities
still have ``keys.primary == ["id"]`` after FILL_IN resolution
so the lookup returns the same property).
The smoke fixture (``simple_request_flow.ttl``) didn't catch
this because it only exercised the FILL_IN path. Updated to
declare ``owl:hasKey ( rf:requestId )`` (and same for Action,
Outcome) so the existing pluggability test now genuinely
exercises the real-key path. Added two new regression tests:
* ``test_simple_request_flow_binds_declared_owl_haskey_property``
— binding emits ``{name: requestId, column: request_id}``,
not the synthesized ``id``.
* ``test_simple_request_flow_property_graph_resolves_owl_haskey_pk_column``
— ``KEY (request_id)`` / ``REFERENCES request (request_id)``
resolve through the declared PK property, not a hard-coded
``id``.
## P2 — README overclaimed periodic materialization pluggability
The README and playbook both implied "any OntologyConfig
works" with the periodic materialization deploy. The deploy
script actually bundles the checked-in MAKO ``binding.yaml`` /
``ontology.yaml`` / ``table_ddl.sql`` (verified in
``deploy_cloud_run_job.sh:421-423``). Running against a
different config requires regenerating those snapshots and
re-pointing the deploy at the new files — and the deploy
script doesn't yet have a CLI flag for that. Softened both
README sections to call this out explicitly; flagged the deploy
script wiring as a natural follow-up.
## INFORMATIONAL — import mode consistency in tests
The test was using package-style imports (``examples.migration_v5.X``)
that masked the fact that the shim ``mako_artifacts.py`` only
supports sibling-style imports (matching the notebook +
``run_agent.py``). Switched the test to sibling-style only so
it exercises the same import surface real callers do.
## Verification
* ``PYTHONPATH=src pytest tests/test_migration_v5_ontology_artifacts.py
tests/test_migration_v5_reference_extractor.py`` — 27/27 pass
(10 new + 17 existing).
* ``PYTHONPATH=src python examples/migration_v5/mako_artifacts.py``
— outputs 18/6/7; ``git diff`` on the four MAKO snapshot
files is empty.
* ``pyink --check`` + ``isort --check-only`` clean.
* docs(migration_v5): correct test count in README validation snippet
PR #172 follow-up review caught the validation snippet still
read ``# → 8 passed`` after the owl:hasKey regression commit
added two tests (now 10).
* fix(ci): skip ontology-artifacts tests when rdflib is absent
CI runs ``pip install -e ".[dev]"`` only — the ``[owl]`` extra
that brings in ``rdflib`` isn't installed. The new test module
pulls ``ontology_artifacts``, which imports
``bigquery_ontology.owl_importer.import_owl``, which raises
``ImportError: rdflib is required for OWL import`` during
collection. Whole module errored across Python 3.10–3.14.
Add ``pytest.importorskip("rdflib")`` at the top, mirroring
the existing pattern in ``tests/test_owl_import_bridge.py`` and
``tests/bigquery_ontology/test_owl_importer.py``. Locally
(with ``[owl]`` installed) all 10 tests still pass; on CI the
module skips cleanly instead of erroring collection.
---------
Co-authored-by: Haiyuan Cao <haiyuan@google.com>1 parent 3ef4337 commit 917a8fa
8 files changed
Lines changed: 1426 additions & 683 deletions
File tree
- examples/migration_v5
- example_ontologies
- periodic_materialization
- tests
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
Lines changed: 125 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
Lines changed: 45 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
0 commit comments