Conversation
Adds the Knowledge Catalog push leg for the semantic-model scope, the counterpart to the merged BigQuery Graph leg (GoogleCloudPlatform#270). Emits semantic-model/semantic-entity/semantic-metric entries against built-in dataplex-types/global system types, publishes relationships as schema-join entry links, shares model loading across legs, and reconciles removed entities/metrics on re-push. --target selects destinations (bq|kc|all); --print dumps each destination's native artifact. BigQuery leg is live-validated; the KC write path is covered hermetically (gated on the semantic-model system types CL).
libei
force-pushed
the
upstream-pr5-kc-pull
branch
2 times, most recently
from
August 9, 2026 04:44
b28f4f5 to
58bfd99
Compare
…leCloudPlatform#288) The okf aspect type is scoped to the project and location, while the entry group the demo creates is not, so every OKF bundle in the project carries its signal layer on the same type. Deleting it during cleanup stripped that signal from bundles the demo does not own, and the bare catch meant it happened silently. Delete only the entry group and print the command to remove the aspect type by hand. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
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.
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.
- Use 'Knowledge Catalog' throughout (drop 'Dataplex'); fix Ossie link to ossie.apache.org. - Author section: 1 model per entry group (my-project.us-central1.my_model); model name matches <model>.yaml; single deployment target for now; dialect: BIGQUERY (1P yaml); drop redundant 'ambient gcloud project' and 'emitted verbatim' asides. - Drop many-to-many from the Knowledge Catalog mapping (Ossie has no M:N yet); 'schema-join link carries the relationship detail'; tighten the metric validation note. - Remove the Permissions section for now (add back once the list is final); drop its inbound links.
Dataplex UpdateEntryLink has no update_mask field; it takes aspect_keys (each project.location.aspectType). writeEntryLink was sending ?updateMask=aspects on the re-push upsert path, which the server rejects with 400 'Unknown name updateMask'. Send the link's aspect-map keys instead. Verified against a live re-push: the schema-join link now PATCHes 200 and the push is idempotent.
Live validation against Dataplex showed the published
schema and semantic-metric system types do not yet carry the SQL
expression fields the emitter was writing: per-field
schema.semantics{expression,role} and semantic-metric.expression.
Emitting them fails the create with an aspect-parsing error.
Put both behind KcGenerateOptions.emitExpressions (default off),
threaded through deploy -> push -> the new --emit-expressions CLI
flag, so the default push matches the live types and the fields can
be flipped on once the templates gain them, without a re-add.
Regenerated the affected goldens to the default-off shape.
…he guide The default KC push no longer writes per-field schema.semantics or semantic-metric.expression (the published system-type templates lack them). Update the guide's 'not a full copy of your model' note and add --emit-expressions to the push flags table.
Only one model per entry group is supported for now, so a push emitting a different count is rejected up front: an empty workspace stays a clean no-op under --validate-only and a configuration error on a real push, and more than one model is always rejected. That guarantees two models can never race for the same entry id, so the now-redundant checkEntryIdCollisions pass is removed. Also narrows the systemTypeProject doc comment (the override exists only so hermetic tests can point at a fixture types project).
A model must declare exactly one deployment target, and it must be a BigQuery Graph URI. Any deploymentTarget that does not parse as one is now collected as malformed and rejected (the validate gate names it, so a --target kc push is covered too) rather than silently ignored, which removes the redundant "looks-like-a-graph" hint heuristic. The shared gate also rejects a model that declares zero or several targets. The data-source pre-flight probe gains a WHERE FALSE so the dry-run scans no data while still resolving multi-part (federated REST-catalog) names as the deploy will. Guide updated to say exactly one deployment target.
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.
…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).
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.
PR GoogleCloudPlatform#278 landed on main gating the KC SQL-expression fields off by default: the emitter now omits the per-field `semantics` block (field expression + role) and the metric expression unless `--emit-expressions` is set, and the committed emitter golden was regenerated without them. The pull leg was built against the old always-emit behavior, so after rebasing onto main it read back less than its tests and goldens assumed. Reconcile the reader to the new default: - readMetric no longer warns when a metric aspect has no expression -- that is now the expected default, not a malformed aspect. It still derives the attach entity from the expression when one is present, else from the persisted `entity`, and still warns on an expression that pins no known entity. - Reader header + user guide: field/metric expressions and the DIMENSION role are recovered only from an `--emit-expressions` push; a default push -> pull drops them. - Tests: split the round trip into roundTrip (default, drops the semantics block) and roundTripFull (`--emit-expressions`, keeps it); point the lossless-slice, DIMENSION, canonical-expression, and expression-derivation cases at the full round trip, and add a case pinning the default drop. The symmetry floor now strips field/metric expressions and the dimension role. - Regenerate the .pull.golden.yaml fixtures: they drop only expressions and bare dimension markers, matching the default emitter golden.
libei
force-pushed
the
upstream-pr5-kc-pull
branch
from
August 14, 2026 01:33
25e36c2 to
8768a40
Compare
Distinguish inherent pull losses from write-side limits: relationship names (not stored in the schema-join aspect) and non-canonical deployment targets (dropped on write) could round-trip faithfully with a writer change. Reader already recovers everything the catalog holds.
A non-canonical BigQuery Graph deployment target fails push at the validation gate, before any leg and for every --target, so nothing is written to BigQuery or Knowledge Catalog. Correct the earlier writer-side follow-up note, which wrongly implied such targets are silently dropped on write, and sharpen the relationship-name follow-up to name the server-side schema-join aspect-type template as the fix.
libei
added a commit
that referenced
this pull request
Sep 2, 2026
Address code review on the model spec: - Field table's expression link pointed at a nonexistent #22-metric anchor; point it at §2.5 Expressions (#25-expressions). - §7 listed relationship join keys among the binding constructs without noting they are declared on the logical model and are not profile-swappable (a profile may not set relationships). Split the constructs into the three profile-swappable bindings plus the fixed edge join keys, and explain why. - Appendix comment claimed the logical model had no physical detail while it carries from_columns/to_columns; correct the comment.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on PR4 (
upstream-pr4-kc-push, the KC push leg — GoogleCloudPlatform#275). Base is the PR4 branch so this PR's diff is the pull delta only; retarget to upstreammainonce PR4 lands.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.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.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,--modelfilter, skipped entries, foreign-entry ignore).SemanticModelLayoutwrite-path test.Full
test:semantic+test:libtsgreen;tsc --noEmitclean.