mdcode: add Knowledge Catalog pull for the semantic model - #277
Conversation
The KC schema and semantic-metric aspects only need expression (our/ANSI form); importedExpression is the vendor/MAQL form and has no KC consumer. Stop emitting it from both the per-field schema semantics block and the semantic-metric aspect.
d196c07 to
90634ae
Compare
Add a shared push-time validation step run once over the loaded models, before any destination leg and on the --validate-only path: every model must declare at least one deploymentTarget, and a model that targets a BigQuery graph must have each metric resolve to a single entity (else it cannot lower to a MEASURE and would be silently dropped). Promotes what was a warn-and-skip into a hard, up-front failure.
Entry links have no list-collection API; the server only exposes them per referenced entry via the location-scoped :lookupEntryLinks custom verb. lookupEntryLinks drains its 10-per-page results into a flat list; deleteEntryLink addresses a link by its entry-group-scoped resource name. Both are prerequisites for reconciling orphaned schema-join links and removed models on push.
Push now snapshots the destination entry group once before writing and uses that listing for the full removal lifecycle: - Whole-model removal: a semantic-model anchor already in the group that this push does not re-emit (a removed or renamed model) is a hard error, unless --force-remove authorizes deleting its links and entries first. - Relationship links: after a model's current schema-join links are written, any link it owns (both endpoints under its entities namespace) that it no longer emits is deleted, covering dropped or renamed relationships. Links are looked up per entity via lookupEntryLinks and deduped. Deletion reconciliation now consumes the pre-write snapshot instead of listing again. KcDeployResult reports unlinked; the push summary surfaces it.
Follow-up fixes from code review of the KC-push reconcile work: - reconcileLinks now enumerates a model's schema-join links via its entity entries in the pre-write snapshot rather than the emitted set. This finds a link both of whose endpoints were removed in the same push (previously unreachable and leaked as a dangling link referencing deleted entries), and, because a brand-new model has no server-side entries, a first push issues no link lookups. - listEntryGroup only tolerates the entry-group not-yet-visible propagation error as empty (mirroring isPropagating); any other listing failure is surfaced instead of masked, which had silently bypassed the foreign-model guard and deletion reconciliation. - Collapse the duplicated GOOGLE deployment-target parsing into a single googleDeploymentTargets pass; bigQueryGraphTargets/deploymentTargetUris are thin views and the validate gate calls it once per model. Regression tests added for the both-endpoints-removed link and the no-lookup first push. 232 pass, tsc clean.
Add a user guide (docs/semantic-model.md) covering the semantic-model push: authoring and deployment targets, --target/--validate-only/--print, what the BigQuery and Knowledge Catalog legs write, the push-time validation gate, and how a re-push reconciles removed entities, metrics, relationships, and whole models (--force-remove). Link it from the README.
Add a live pre-flight to the push-time validate gate: before either the BigQuery or Knowledge Catalog leg runs, every entity's BigQuery source table (a plain project.dataset.table) is probed via a new getTable client method, and a missing or inaccessible table fails the push, naming the table and entity. This confirms a model can deploy up front rather than surfacing a bad table only when the BigQuery leg executes its DDL. Runs for every --target and for --validate-only; query / non-table sources are skipped. Static requirement checks (deployment target, metric-to-entity) are unchanged. Adds validateBigQueryDataSources + parseTableRef, BigQueryClient.getTable and its mock override, five unit tests, and updates the user guide.
…ST catalog) names
- PushOptions: comment every field; clarify --force (generic CatalogSync push) vs --force-remove (semantic-model KC deletion authorization). - Guide: add a BigQuery mapping table mirroring the Knowledge Catalog one; drop articles from the mapping tables' Model element column. - Guide: make the importedExpression note explicit that the catalog is not a full copy of the model (vendor SQL is not stored; document is source of truth). - Guide: rewrite 'Updating and removing models' from the user's perspective.
…C deploy fields - Split deployKnowledgeCatalog into named phase helpers (emitModels, checkEntryIdCollisions, buildPlan, guardForeignModels, writeModels) so the function body reads as an explicit sequence of steps; behavior unchanged. - KcDeployOptions and KcDeployResult: a comment on every field. - Rewrite reconcileDeletions' doc comment from the user's perspective. - Fix a stale doc comment that referenced a non-existent defaultProject param.
| project: string; | ||
| location: string; | ||
| entryGroup: string; | ||
| model?: string; // limit to a single model by name (default: all) |
There was a problem hiding this comment.
Can there be multiple models in your entry group?
There was a problem hiding this comment.
Yes — one entry group can hold many models. Each semantic-model entry is a separate anchor; the reader groups the semantic-entity/semantic-metric entries under their anchor by parentEntry and returns one reconstructed model per anchor (modelsFromCatalogResources). --model narrows both the fetch and the write to a single anchor. Now documented in the new Pull section of docs/semantic-model.md.
| @@ -0,0 +1,256 @@ | |||
| // Serializes the Semantic Model IR (./ir) back to the open AI-first semantics | |||
There was a problem hiding this comment.
The file name is a little bit too general. It is named as serialize, but serialize from what to what? Can we be clear?
There was a problem hiding this comment.
| } | ||
|
|
||
|
|
||
| // --------------------------------------------------------------------------- |
There was a problem hiding this comment.
I remember there are some files like load_knowledge_catalog or something of this sort. So, should we have a separate file for this new capability? Or if we put them in the same file, is it going to be too crowded? Please think through. We need to make sure the code is very clean.
There was a problem hiding this comment.
Agreed — pulled the new code into its own files: the KC reader is now kc_converter.ts and the network pull is pull_kc.ts. knowledge_catalog.ts and deploy_knowledge_catalog.ts return to their #278 state (emit-only / push-only). This is scaffolding for the eventual two-layer split — pure converters (osi_converter/kc_converter/bigquery) vs push/pull orchestration — with the remaining halves moving in after #278 merges and no further renames. (2f0531b)
| @@ -0,0 +1,290 @@ | |||
| // Behavior specification for the semantic-model serializer | |||
| // (src/libts/semantic/serialize.ts). | |||
There was a problem hiding this comment.
Please think through the tests. Usually, I prefer to have a test fixture so that you know the input and output files as a whole, not like validated one row at a time. You don't get the full picture if that's the case. Can you think through all these tests in this PR and organize in a better way?
There was a problem hiding this comment.
Reorganized around committed golden files. Each corpus fixture now has an .osi.golden.yaml (IR → OSI) and a .pull.golden.yaml (KC entries → IR → OSI), so you see the whole input and output as files rather than row-by-row asserts — and diffing the two shows exactly what a Knowledge Catalog round trip drops (keys, ai_context, labels, relationships; an is_time dimension collapses to a bare {}). The IR round-trip and the focused mapping/warning tests are kept as invariants alongside the goldens. Test files renamed to match their modules: osi_converter/kc_converter/pull_kc.test.ts. (2f0531b)
Add the inverse of the KC emitter: read semantic-model / -entity / -metric entries and their aspects back into the IR, serialize to YAML, and wire a 'pull' command (with --dry-run and --model) for the semantic-model scope. Re-stacked onto the KC-push follow-ups: the emitter no longer writes importedExpression, so the reader no longer recovers it; idOf is shared from knowledge_catalog.ts; and push entry/link writes use the same bounded mapConcurrent pool as pull hydration.
b28f4f5 to
58bfd99
Compare
…oldens + docs Addresses PR review feedback on the KC pull leg: - Rename serialize.ts -> osi_converter.ts (the OSI <-> IR converter). The name now says what it converts between; a header banner notes it currently holds only the serialize direction and that the loader migrates in post-GoogleCloudPlatform#278. - Extract the KC reader into kc_converter.ts and the network pull into pull_kc.ts, so the new capability lives in its own files rather than swelling knowledge_catalog.ts / deploy_knowledge_catalog.ts. Those two files return to their GoogleCloudPlatform#278 state (emit-only / push-only). This is scaffolding for the eventual two-layer split (pure converters vs push/pull orchestration); the remaining halves move in once GoogleCloudPlatform#278 merges, with no further file renames. - Reorganize the pull tests around committed golden artifacts: each corpus fixture now has an .osi.golden.yaml (IR -> OSI) and a .pull.golden.yaml (KC entries -> IR -> OSI). A reviewer sees the whole input and output as files and can diff the two to see exactly what a Knowledge Catalog round trip drops. Test files renamed to match their modules (osi_converter/kc_converter/pull_kc). - Document `kcmd pull` in docs/semantic-model.md: the --dry-run/--model flags, multiple models per entry group, last-write-wins overwrite policy, and the catalog-not-a-full-copy round-trip loss.
…erter-scaffold files Each new converter/orchestration file now carries an actionable TODO spelling out how the scaffold collapses once GoogleCloudPlatform#278 merges, so reviewers can see the plan: - osi_converter.ts: fold loader.ts (OSI read) in, delete loader.ts, repoint importers. - kc_converter.ts: fold generateCatalogResources (KC write) in, delete knowledge_catalog.ts, repoint importers, demote the shared idOf to a local. - pull_kc.ts: rename deploy_knowledge_catalog.ts -> push_kc.ts for push_kc/pull_kc symmetry (rename only, no logic moves).
|
Added
Each is a mechanical move + import repoint deferred until #278 merges, so this PR keeps the old push files at their #278 state and only introduces the new-pattern files. |
Reviewers asked the docs to be clear about lossless vs lossy. Both directions are lossy; say so plainly and enumerate exactly what each drops: - Push to BigQuery is lossy: captures the queryable structure (node/edge tables, measures) but not descriptive metadata; non-reducible metrics are skipped. - Push to Knowledge Catalog is lossy: stores a metadata subset (keeps 1:1/1:N as schema-join links) and drops keys, ai_context, labels, vendor SQL, M:N. - Pull is lossy: recovers even less than the catalog holds (no relationships, no deploymentTargets). A push followed by a pull does not return the original file.
Pull previously dropped two things push had already written to Knowledge Catalog: the model's deployment targets (stored in the semantic-model aspect) and its 1:1/1:N relationships (stored as schema-join entry links). The reader only opened per-entry aspects and pull only fetched entries, so both were silently lost even though the catalog held them. - kc_converter: read deploymentTargets back into the GOOGLE custom_extensions block, and invert schema-join links into Relationships -- endpoints resolved by data source, FK direction and columns from the join aspect. modelsFromCatalogResources grows an entryLinks argument. Relationship names come back normalized (lowercased/hyphenated): the emitter stores the name only in the link id. - pull_kc: add a second fetch pass over the entity entries via lookupEntryLinks, deduping the undirected links (each is returned from both endpoints). - Tests cover endpoint/direction recovery, name normalization, the M:N drop, deployment-target recovery, and the pull fetch+dedup path; the .pull.golden fixtures are regenerated and the docs pull note rewritten. M:N (association) relationships remain unrecovered -- push never emits them. Writer files are untouched.
Address code-review findings on the gap-3 pull leg: - Resolve schema-join endpoints from the link's entryReferences, matched by entry id, instead of a dataSource->entity index. The id is unique per entity (fixes two entities sharing a table collapsing last-wins) and is stable across the project-number/id normalization lookupEntry applies to entries but lookupEntryLinks does not apply to link references (fixes relationships silently dropping on the live path). The schema-join aspect is now used only for FK direction + join columns; undecidable direction keeps the reference order and warns rather than dropping the edge. - Dedup entry links by a sorted endpoint-pair key when a link has no name (shared linkDedupKey, reused by pull_kc) so a nameless link returned from both endpoints is not counted twice. - Rewrite the pull 'lossy' note in the user guide as recovered-exactly / recovered-but-normalized / not-recovered bullets. Tests: shared-table endpoints, un-normalized project-number references, prefix-stripping across tricky model names, and nameless-link dedup.
Two fixture-coverage gaps from the round-trip review: 1. Symmetry assertion. The golden pull files let a human eyeball what a Knowledge Catalog round trip drops, but nothing asserted it. Add a symmetry test over the converter corpus: load the authored IR, run a full emit -> read round trip, and assert the result equals the authored IR reduced to the "KC floor" (stripToKcFloor) -- the documented losses and normalizations applied to both sides. An undocumented regression (a dropped column, a lost description, an un-stripped M:N edge) now fails here even though each individual loss is already pinned by a targeted test. Export linkNamePrefix so the normalizer reproduces the relationship slug rather than reimplementing it. 2. sales_bq_graph_target had OSI/KC/pull goldens but no BigQuery golden. Add it to the BigQuery corpus and generate the golden: a valid single-node property graph with a MEASURE, so the fixture now carries a complete four-arm round-trip suite. No production behavior change; reader/emitter untouched apart from the linkNamePrefix export.
The BigQuery corpus golden path names the graph from the test's build opts (sqlgen-testing.demo.sales), ignoring the fixture's deployment target -- so the golden neither reflected the fixture's purpose nor matched a real deploy (demo.sales.sales_graph). That target-driven name is already covered by deploy_bigquery.test.ts, making this golden redundant. Revert the corpus addition and remove the generated file; the fixture keeps its OSI/KC/pull goldens and the pull symmetry assertion.
Address code-review findings on the KC->IR reader (kc_converter.ts): - readMetric: when the expression does not pin exactly one known entity (none, or several), fall back to the attach entity metricAspectData persisted instead of dropping it. A cross-entity metric now recovers its authored entity. - readField: skip a schema field with no name (warn) instead of emitting a Field with an undefined name into the entity. - linkNamePrefix: mirror linkSlug's 63-char cap and trailing-hyphen re-strip so the read-side prefix stays aligned with the emitter's link id. - Header comment: add importedExpression (vendor SQL) and String/Opaque- typed metrics to the documented round-trip loss list. Add regression tests for the metric-entity fallback and the nameless-field skip.
Follows PR4 (the KC push leg, #275, now merged). Rebased onto the merged
main, so this PR's diff is the pull delta only.What
The read counterpart of the Knowledge Catalog push leg.
kcmd pullin a semantic-model workspace now reads thesemantic-*entries back from Knowledge Catalog, reconstructs the IR, and serializes each model tocatalog/EntryGroups/<entryGroup>/<model>.yaml— the inverse of the push leg against the same built-in-type schema (go/semantic-model-kc-v2).Pieces
serialize.ts(new): pure IR → open-format YAML, the exact inverse ofloader.ts.knowledge_catalog.ts:modelsFromCatalogResources, the inverse of the emitter — parses the built-inschemaaspect (dataType/metadataType → logical type,DIMENSIONrole, per-fieldsemantics), reverses the BigQuery resource URI back to thedataSourcestring, and re-derives each metric's attach entity from its expression (as the loader does).deploy_knowledge_catalog.ts:pullKnowledgeCatalog— lists the entry group, hydrates each entry's aspects (an entity needs BOTH itssemantic-entityaspect and the built-inschemaaspect), and applies a--modelfilter (scoped to the target model's entries before hydration).SemanticModelLayout:modelPath/hasModel/writeModelDocumentwrite sink.commands.pull/main.ts:--dry-runand--model. Overwrite policy matches the core pull: last-write-wins; local-only documents are never deleted.Fidelity
IR-level, bounded by what the push leg persists. Entity keys,
ai_context, field labels,importedDialect, and relationships are not written by push (the graph edges live in the BigQuery property graph) and so do not come back. An authoredStringdatatype is indistinguishable from an un-typed field after emit and reads back as un-typed. A typeless metric round-trips asDecimal: the push leg makessemantic-metric.dataTyperequired and defaults it toNUMERIC, which the reader maps back toDecimal.Testing
Hermetic round trips are the acceptance bar (no live KC needed — PR4's server types are still nonprod-pending):
loader ↔ serializeround trip over 6 corpus fixtures (relationships, ai_context, datatypes, dimensions, custom_extensions, vendor dialects).emitter ↔ readerround trip + dataType-inverse / role / resource-URI / metric-attach unit tests.pullKnowledgeCatalogover a stubbed catalog client (aspect hydration,--modelscoping, skipped entries, foreign-entry ignore).SemanticModelLayoutwrite-path test.Full
test:semantic+test:libtsgreen;tsc --noEmitclean.