mdcode: KC push follow-ups (validate gate, model + link reconciliation) by libei · Pull Request #278 · GoogleCloudPlatform/knowledge-catalog

libei · 2026-08-08T04:51:16Z

Follow-ups to #275 (Knowledge Catalog push for the semantic model), addressing
the four review threads left as agreed follow-ups. Each is a self-contained
commit.

1. Drop `importedExpression` from the KC emit

expression is the ANSI/GoogleSQL form KC consumes; importedExpression is the
vendor/MAQL form and has no KC consumer, so it is no longer written into
semantic-metric or schema aspects. The vendor form is still kept in the
source model and used as a BigQuery-SQL fallback; only the KC emit drops it.
Golden regenerated.

2. Shared validate gate

A single validatePushRequirements gate now funnels every push (real and
--validate-only):

every model must declare at least one deployment target;
a metric on a BigQuery-graph-targeting model must resolve to exactly one
entity (promotes today's silent warn+skip to a hard error).

Malformed GOOGLE extension JSON becomes a per-model error rather than an uncaught
throw.

3. Whole-model removal lifecycle + `--force-remove`

Push now snapshots the destination entry group once before writing. 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.

4. Removed-relationship link reconciliation

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 the
new lookupEntryLinks client method and deduped. Deletion reconciliation now
consumes the pre-write snapshot instead of listing again.

5. BigQuery data-source access validation (pre-flight)

The validate gate also confirms, before either leg runs, that every entity's
BigQuery source table is reachable, so a model that cannot deploy is caught up
front rather than only when the BigQuery leg executes its DDL. Each distinct
source is probed with a dry-run query, so BigQuery resolves the reference
exactly as the generated DDL will — this covers a three-part
project.dataset.table, a four-part federated REST-catalog / Lakehouse name
(e.g. an Apache Iceberg table via BigLake), and quoted identifiers, not just a
three-part name. A missing or inaccessible table fails the push, naming the table
and entity. The probe is billed to the model's deployment-target project (the
same project the deploy runs against). It runs for every --target and for
--validate-only; a source that is a query (not a table) is skipped.

User guide

docs/semantic-model.md (linked from the README) documents the semantic-model
push end to end, structured for a reader's flow: overview, prerequisites,
authoring (with a worked model document, including the GOOGLE deployment-target
extension), pushing (--target/--validate-only/--print/--force-remove in a
flags table), an at-a-glance table mapping each model element to the BigQuery and
Knowledge Catalog resource it produces, the validation gate (including the
data-source pre-flight and federated REST-catalog sources), how a re-push
reconciles removed entities/metrics/relationships/models, and a consolidated
permissions reference.

Tests

npx tsc --noEmit clean; bun test 238 pass / 0 fail (new client-method,
validate, data-source-access, link-reconciliation, and force-remove tests
included). The KC write path stays hermetic (CL-gated); #3/#4/#5 are exercised
through the mocked client.

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.

libei · 2026-08-09T04:03:28Z

@@ -27,6 +29,10 @@ export interface InitOptions {
 export interface PushOptions {
  force?: boolean;


What's the difference between force and a forceRemove below? Make sure each field in this push option has a comment clearly describing what it does.

Done (1fc62b1). Different scopes: `force` is the generic push flag for non-semantic-model (CatalogSync) scopes — the semantic-model legs ignore it. `forceRemove` is semantic-model KC only and specifically authorizes deleting models the push no longer includes. Every field in `PushOptions` now has a comment spelling this out.

libei · 2026-08-09T04:04:59Z

                "metadataType": "STRING",
                "semantics": {
                  "expression": "ss_item_sk",
-                  "importedExpression": "{label/store_sales.attr.store_sales.ss_item_sk}",


If this imported expression is not stored in Knowledge Catalog, does that mean if I do a fresh pull, this field will no longer be restored at the client side? That does mean the KCMD push to Knowledge Catalog is lossy. If that's the case, we need to be very clear in the documentation/user guide.

Good catch — yes, dropping `importedExpression` makes a catalog-only reconstruction lossy for the vendor SQL (the local document keeps it and it is still used for BigQuery SQL generation, so a normal edit-and-repush is not lossy). Documented explicitly in the gues note (1fc62b1): "the catalog is not a full copy of your model … keep your model document as the source of truth: a model reconstructed only from the catalog would come back without its vendor SQL."

…ST catalog) names

libei · 2026-08-09T04:11:18Z

+
+| Model element | Catalog resource | Kind | Id |
+|---|---|---|---|
+| the model | `semantic-model` | entry — anchor / parent of the rest | `<model>` |


remove "the", "an", "an" etc from the model element column.

Done (1fc62b1) — articles dropped from the Model element column in both mapping tables (Model / Entity / Metric / Relationship).

libei · 2026-08-09T04:11:49Z

+Destinations always deploy BigQuery-first and fail fast, so a rejected model
+never half-deploys.
+
+### What gets created in BigQuery


I like the format of the table below in the section of what gets created in Knowledge Catalog. Can we do a similar table for the BigQuery section?

Done (1fc62b1) — "What gets created in BigQuery" is now the same style of table: Model → PROPERTY GRAPH, Entity → NODE TABLE, Relationship → EDGE TABLE, Metric → MEASURE.

libei · 2026-08-09T04:12:21Z

+
+## Updating and removing models


This updating and removing models section is still hard to read. It reads like a design doc rather than a user guide. Can you make sure it's really written from a user's perspective?

Rewritten from the user's perspective (1fc62b1): framed as "When you edit / delete an entity or metric / rename or delete a relationship / remove a whole model", each saying what you do and what push does for you, ending with how to read the summary line.

libei · 2026-08-09T04:17:17Z

 import {LoadedModel} from './loader';


 export interface KcDeployOptions {


Please make sure each field or property here has a proper comment explaining what it does, including existing fields.

Done (fd10b2c). `KcDeployOptions` now has a comment on every field — project / location / entryGroup, the systemType* overrides, validateOnly, forceRemove, and the two entryCreate* retry knobs.

libei · 2026-08-09T04:18:05Z

  entryCreateRetryMs?: number;
 }

 export interface KcDeployResult {


Same here for each field. Please add a comment to explain what it means, including existing fields not in this PR.

Done (fd10b2c). Every field of `KcDeployResult` is now commented, including the previously-bare `success` and `details`, plus created/updated/deleted/linked/unlinked/plan.

libei · 2026-08-09T04:19:25Z

 // Emits no console output; warnings and the dry-run plan are returned for the
 // caller to print. `defaultProject` qualifies a dataset `source` that omits its
 // project (the scope's declared project, a deterministic user-authored value).
 export async function deployKnowledgeCatalog(


This function becomes really long. Can you refactor to have a multiple meaningful subroutines? I want to be able to look at this structure of this function and understand what it does clearly.

Refactored (fd10b2c). `deployKnowledgeCatalog` is now just the phase sequence, each phase a named helper: `emitModels` → `checkEntryIdCollisions` → `buildPlan` (stops here for --validate-only) → `listEntryGroup` → `guardForeignModels` → `writeModels` → `reconcileDeletions`. The body reads top-to-bottom as those steps; behavior is unchanged (238 tests still pass), and running tallies are threaded through a `Counts` object so a mid-way failure still reports partial progress.

- 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.

libei · 2026-08-09T04:20:35Z

+// entry-group listing captured once before writes -- so it never issues its own
+// list call; a re-emitted entry is never a deletion candidate, so the pre-write
+// snapshot stays correct.
+function reconcileDeletions(


The comment of this function can be much better. It is hard to read now. We need to describe it from a user perspective.

Rewritten from the user perspective (fd10b2c): "Removes the catalog entries left behind when you delete an entity or metric from a model and push again…", then what it will and won't touch (only entries this push owns; other models in a shared group and the anchors are left alone). Also fixed a stale comment on the function above that referenced a `defaultProject` param that does not exist.

libei · 2026-08-09T04:22:38Z

+
+| Model element | BigQuery construct | Notes |
+|---|---|---|
+| Model | one `PROPERTY GRAPH` | named by the deployment-target URI |


Similarly, remove "a," "an," and "the" from this BigQuery construct column.

Done (d4f7a53) — dropped the leading articles from the BigQuery construct column: PROPERTY GRAPH / NODE TABLE / EDGE TABLE / MEASURE.

libei · 2026-08-09T04:23:40Z

+
+Your model document is the source of truth. To change what is deployed, edit
+the document and run `kcmd push` again — you never edit the catalog or the
+BigQuery graph by hand. Re-running is safe: each push makes the destinations


Please make sure when you refer to BigQuery Graph, you use uppercase G in Graph.

Done (d4f7a53) — "BigQuery Graph" is now capitalized consistently throughout the guide; the only remaining lowercase "graph" is the SQL construct "property graph" / "PROPERTY GRAPH".

…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.

libei · 2026-08-09T04:27:52Z

+touched**, so a model that cannot deploy fails fast instead of half-deploying:
+
+* **At least one deployment target per model.** *(static)*
+* **Every metric on a BigQuery-graph model resolves to exactly one entity** —


Please do not use this term. Use "BigQuery Graph" with an uppercase G for Graph.

Done (d4f7a53) — changed "BigQuery-graph model" to "BigQuery Graph model" (and "dropped from the graph" to "dropped from the BigQuery Graph").

…uct column

…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).

libei added 4 commits August 8, 2026 03:54

libei requested review from amirhormati and dlychagin-gg August 8, 2026 04:51

libei added 5 commits August 8, 2026 05:12

mdcode: correct semantic-model guide (--validate-only vs --print)

0684141

mdcode: restructure semantic-model guide for readability

38d8d3a

libei commented Aug 9, 2026

View reviewed changes

libei added 2 commits August 9, 2026 04:07

mdcode: validate BigQuery sources via dry-run to cover multi-part (RE…

4725373

…ST catalog) names

mdcode: document REST-catalog sources; clarify Knowledge Catalog mapping

0c2a8f2

libei commented Aug 9, 2026

View reviewed changes

mdcode: capitalize BigQuery Graph; drop articles from BigQuery constr…

d4f7a53

…uct column

libei mentioned this pull request Aug 9, 2026

mdcode: add Knowledge Catalog pull for the semantic model #277

Draft

		@@ -27,6 +29,10 @@ export interface InitOptions {
		export interface PushOptions {
		force?: boolean;

		import {LoadedModel} from './loader';


		export interface KcDeployOptions {

Uh oh!

Conversation

libei commented Aug 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1. Drop importedExpression from the KC emit

2. Shared validate gate

3. Whole-model removal lifecycle + --force-remove

4. Removed-relationship link reconciliation

5. BigQuery data-source access validation (pre-flight)

User guide

Tests

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

libei Aug 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

libei commented Aug 8, 2026 •

edited

Loading

1. Drop `importedExpression` from the KC emit

3. Whole-model removal lifecycle + `--force-remove`

libei Aug 9, 2026 •

edited

Loading