Skip to content

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

Draft
libei wants to merge 23 commits into
GoogleCloudPlatform:mainfrom
libei:upstream-pr5-kc-pull
Draft

mdcode: add Knowledge Catalog pull for the semantic model#277
libei wants to merge 23 commits into
GoogleCloudPlatform:mainfrom
libei:upstream-pr5-kc-pull

Conversation

@libei

@libei libei commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

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 pull in a semantic-model workspace now reads the semantic-* entries back from Knowledge Catalog, reconstructs the IR, and serializes each model to catalog/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 of loader.ts.
  • knowledge_catalog.ts: modelsFromCatalogResources, the inverse of the emitter — parses the built-in schema aspect (dataType/metadataType → logical type, DIMENSION role, per-field semantics), reverses the BigQuery resource URI back to the dataSource string, 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 its semantic-entity aspect and the built-in schema aspect), and applies a --model filter (scoped to the target model's entries before hydration).
  • SemanticModelLayout: modelPath / hasModel / writeModelDocument write sink.
  • commands.pull / main.ts: --dry-run and --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 authored String datatype is indistinguishable from an un-typed field after emit and reads back as un-typed. A typeless metric round-trips as Decimal: the push leg makes semantic-metric.dataType required and defaults it to NUMERIC, which the reader maps back to Decimal.

Testing

Hermetic round trips are the acceptance bar (no live KC needed — PR4's server types are still nonprod-pending):

  • loader ↔ serialize round trip over 6 corpus fixtures (relationships, ai_context, datatypes, dimensions, custom_extensions, vendor dialects).
  • emitter ↔ reader round trip + dataType-inverse / role / resource-URI / metric-attach unit tests.
  • pullKnowledgeCatalog over a stubbed catalog client (aspect hydration, --model scoping, skipped entries, foreign-entry ignore).
  • SemanticModelLayout write-path test.

Full test:semantic + test:libts green; tsc --noEmit clean.

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.
@libei
libei force-pushed the upstream-pr5-kc-pull branch from d196c07 to 90634ae Compare August 8, 2026 04:14
libei added 13 commits August 8, 2026 04:14
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.
- 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)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can there be multiple models in your entry group?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The file name is a little bit too general. It is named as serialize, but serialize from what to what? Can we be clear?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed to osi_converter.ts — the OSI ⇄ IR converter. It currently holds only the serialize direction (IR → OSI YAML); a header banner spells out the direction and notes the loader (OSI → IR) migrates in once #278 merges, at which point it becomes the full two-way converter. (2f0531b)

}


// ---------------------------------------------------------------------------

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
@libei
libei force-pushed the upstream-pr5-kc-pull branch from b28f4f5 to 58bfd99 Compare August 9, 2026 04:44
libei added 2 commits August 9, 2026 06:22
…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

libei commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator Author

Added TODO(#278) blocks to the new converter/orchestration files (988280b) so the scaffold plan is legible in the code:

  • 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.tspush_kc.ts for push_kc/pull_kc symmetry (rename only, no logic moves).

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.

libei added 4 commits August 9, 2026 06:34
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.
libei added 2 commits August 9, 2026 07:40
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant