mdcode: address KC pull review comments (post-merge follow-up)

toolbox/mdcode/docs/semantic-model.md

@@ -201,11 +201,10 @@ paired columns and foreign-key direction — in its aspect.
 > block or a `semantic-metric.expression` field, so the default push omits them
 > (pass `--emit-expressions` to write the canonical GoogleSQL/ANSI expression
 > once the templates gain the fields). It never stores entity keys, `ai_context`,
-> field labels, the original vendor SQL (`importedExpression` — e.g. the MAQL or
-> Snowflake form a metric was imported from), or M:N relationships. Those stay in
-> your authored document (and, for the edges, in the BigQuery property graph); the
-> vendor SQL and expressions are still used when generating BigQuery SQL. Keep
-> your model document as the source of truth.
+> field labels, or the original vendor SQL (`importedExpression` — e.g. the MAQL
+> or Snowflake form a metric was imported from). Those stay in your authored
+> document; the vendor SQL and expressions are still used when generating
+> BigQuery SQL. Keep your model document as the source of truth.
 
 ## Validation
 
@@ -293,15 +292,18 @@ from the same scope you authored under (`<projectId>.<locationId>.<entryGroupId>
 | Flag | Effect |
 |------|--------|
 | `--dry-run` | Reconstruct from the catalog and report what would be written, but write no files. |
-| `--model <name>` | Pull a single model by name; other models in the entry group are left alone. |
+| `--force-remove` | Replace a differently-named local model with the catalog's (see below); without it, a pull that would leave the entry group holding two models fails. |
 
-One entry group can hold **many models** — each `semantic-model` entry is a
-separate anchor, and pull reconstructs one document per anchor. `--model`
-narrows both the fetch and the write to a single anchor.
+An entry group holds **exactly one** semantic model. Pull reconstructs that one
+model's document; a group with more than one `semantic-model` anchor is an
+unexpected state, so pull stops and names the anchors rather than guess which to
+keep.
 
-Pull writes with the same last-write-wins policy as the core pull: a model that
-already exists locally is overwritten in place, and a local-only document (one
-with no matching catalog entry) is left untouched — pull never deletes.
+Pull overwrites a model that already exists locally in place. If the catalog's
+model has a **different name** than the one on disk, writing it would leave the
+entry group holding two models — so by default pull stops and reports the
+mismatch instead of deleting anything. Re-run with `--force-remove` to delete the
+local model and replace it with the catalog's. Pull never touches BigQuery.
 
 > **Note — pull reconstructs what the catalog holds, not your original file.**
 > Pull can only recover what push wrote (see the note under [What gets created in
@@ -310,8 +312,9 @@ with no matching catalog entry) is left untouched — pull never deletes.
 >
 > **Recovered exactly** — these come back as authored:
 > - Model structure: the model, its entities, and each entity's fields.
-> - Field data source and data type.
-> - Metrics: name, data type, and attach entity.
+> - Each field's data source (its data type round-trips with the two collapses
+>   noted below).
+> - Metrics: name and attach entity (a concrete data type round-trips; see below).
 > - 1:1 / 1:N relationships: endpoints, foreign-key direction, and join columns
 >   (from the `schema-join` links).
 > - Deployment targets.
@@ -328,15 +331,21 @@ with no matching catalog entry) is left untouched — pull never deletes.
 > **Recovered, but normalized** — the content survives, the form changes:
 > - Relationship *names* come back lowercased/hyphenated (the catalog stores the
 >   name only in the link id, e.g. `Places Order` → `places-order`).
-> - A metric authored with no data type comes back as an explicit `Decimal`
->   (push must write a type, and defaults it to `NUMERIC`).
+> - Field types round-trip except for two collapses: a field authored with no
+>   type comes back as `Opaque`, and a field authored as `String` comes back
+>   un-typed (`String` and un-typed both store as a plain catalog `STRING`).
+> - A metric's data type round-trips only for a concrete type (e.g. `Decimal`);
+>   an untyped, `String`, or `Opaque` metric comes back un-typed, because the
+>   metric aspect stores a data type but no metadata type to mark it `Opaque`.
+> - Ordering: field order within each entity is preserved, but the order of
+>   entities and metrics is not — they come back in the catalog's own order, not
+>   the authored one. Comments in the original YAML are not preserved.
 >
 > **Not recovered** — push never wrote these, so pull cannot return them:
 > - Entity keys / unique keys.
 > - `ai_context`.
 > - Field labels.
 > - The original vendor SQL (`importedExpression`).
-> - M:N relationships (the edge lives only in the BigQuery property graph).
 >
 > **So: a push followed by a pull does not return your original file.** Treat a
 > pulled document as a faithful copy of the catalog metadata, not of the authored
@@ -374,7 +383,9 @@ with no matching catalog entry) is left untouched — pull never deletes.
 
 **Knowledge Catalog / Dataplex** — for `--target kc` or `all`:
 
-* `dataplex.entryGroups.useSemanticModelAspect` on the destination entry group
+* `dataplex.entryGroups.useSemanticModelAspect`,
+  `dataplex.entryGroups.useSemanticEntityAspect`, and
+  `dataplex.entryGroups.useSemanticMetricAspect` on the destination entry group
 * `dataplex.entryGroups.useSchemaJoinEntryLink` and
   `dataplex.entryGroups.useSchemaJoinAspect` when the model has relationships
 

toolbox/mdcode/src/libts/layouts/semantic-model.ts

@@ -121,6 +121,17 @@ export class SemanticModelLayout implements CatalogLayout {
     this._index.set(name, localPath);
   }
 
+  // Deletes a model document from disk and the index. `pull --force-remove`
+  // uses it to drop a local model the catalog no longer names before writing
+  // the catalog's, so the entry group is never left holding two models.
+  removeModelDocument(name: string): void {
+    const localPath = this.modelPath(name);
+    if (fs.existsSync(localPath)) {
+      fs.rmSync(localPath);
+    }
+    this._index.delete(name);
+  }
+
   // The Knowledge Catalog entry-level members are not applicable to this
   // push-only layout; the model is authored as a single Ossie document, not as
   // per-entry Knowledge Catalog files. These are wired when KC-resource emit

toolbox/mdcode/src/libts/semantic/kc_converter.ts

@@ -235,9 +235,11 @@ function readMetric(
         `be placeable downstream`);
   }
   // The emitter writes a required dataType, defaulting a typeless metric to
-  // NUMERIC (see metricAspectData); NUMERIC maps back to Decimal, so a metric
-  // authored without a datatype round-trips as an explicit Decimal rather than
-  // un-typed. (Dimensions differ: their STRING default reads back as un-typed.)
+  // Opaque (see metricAspectData). The metric aspect carries no metadataType,
+  // so Opaque serializes as a bare STRING, which irDataType reads back as
+  // un-typed
+  // -- a metric authored without a datatype round-trips un-typed rather than as
+  // a guessed numeric type.
   const type = irDataType(data.dataType, undefined);
   if (type !== undefined) metric.type = type;
   const description = entry.entrySource?.description;
@@ -248,9 +250,11 @@ function readMetric(
 
 // The inverse of columnDataType/columnMetadataType: maps the schema aspect's
 // dataType (disambiguated by metadataType only for the STRING family) back to
-// the IR's logical DataType. STRING + OTHER is Opaque; a plain STRING is read
-// as un-typed (undefined) -- the loader's default -- since the emitter cannot
-// distinguish an authored `String` from an un-typed field (both emit STRING).
+// the IR's logical DataType. STRING + OTHER is Opaque -- which the emitter
+// writes both for an authored `Opaque` field and for a field the author left
+// untyped; a plain STRING (metadataType STRING) is an authored `String` and is
+// read as un-typed (undefined), the loader's default. (The metric aspect has no
+// metadataType, so a metric's bare STRING always reads back un-typed.)
 function irDataType(dataType: string|undefined, metadataType: string|undefined):
     DataType|undefined {
   switch (dataType) {

toolbox/mdcode/src/libts/semantic/knowledge_catalog.ts

@@ -156,7 +156,7 @@ export function generateCatalogResources(
       parentEntry: modelEntryName,
       entrySource: source(metric.name, metric.description),
       aspects: aspectMap(names, {
-        'semantic-metric': metricAspectData(metric, warnings, emitExpr),
+        'semantic-metric': metricAspectData(metric, emitExpr),
       }),
     });
   }
@@ -206,6 +206,19 @@ function relationshipLink(
           warnings))
     return undefined;
 
+  // The name lives only in the link id (schema-join's aspect has no name
+  // field), and link ids are normalized -- lowercase, hyphens only. When the
+  // authored name is not already in that form, a later pull recovers it
+  // lowercased and hyphenated, not verbatim; warn so the round-trip change is
+  // not a surprise.
+  const normalizedName = linkSlug(rel.name);
+  if (normalizedName !== rel.name) {
+    warnings.push(
+        `relationship '${rel.name}': Knowledge Catalog stores the name only ` +
+        `in the normalized link id, so a pull returns it lowercased/hyphenated ` +
+        `(e.g. '${normalizedName}'), not '${rel.name}'.`);
+  }
+
   return {
     name: names.entryLink(linkId),
     entryLinkType: names.typeName('entryLink', 'schema-join'),
@@ -291,8 +304,12 @@ function schemaAspectData(
     fields: (entity.fields ??
              []).map(f => compact({
                        name: f.name,
-                       dataType: columnDataType(f.type),
-                       metadataType: columnMetadataType(f.type),
+                       // An untyped field is published as Opaque (STRING +
+                       // metadataType OTHER), the explicit "type unknown"
+                       // marker, so a pull recovers it as Opaque rather than
+                       // dropping the type. Authored `String` maps to STRING.
+                       dataType: columnDataType(f.type ?? 'Opaque'),
+                       metadataType: columnMetadataType(f.type ?? 'Opaque'),
                        description: f.description,
                        // The per-field `semantics` block (expression + role) is
                        // not in the published `schema` aspect template yet, so
@@ -310,21 +327,14 @@ function schemaAspectData(
 }
 
 // semantic-metric: the model-level aggregate. `dataType` is required by the
-// aspect type; when the model does not declare one, fall back to NUMERIC
-// (decimal) and warn (metrics are aggregates, so an exact numeric is the
-// sensible default; dimensions, in schemaAspectData, default to STRING) rather
-// than emit an invalid aspect.
+// aspect type; a metric the author left untyped is published as Opaque -- the
+// explicit "type unknown" marker -- rather than guessing a numeric type. (The
+// metric aspect template carries only `dataType`, not a metadataType, so Opaque
+// serializes as STRING and a pull recovers the metric untyped; once the
+// template gains a metadataType it can round-trip as an explicit Opaque.)
 function metricAspectData(
-    metric: Metric, warnings: string[],
-    emitExpressions: boolean): Record<string, any> {
-  let dataType = metric.type ? columnDataType(metric.type) : undefined;
-  if (!dataType) {
-    warnings.push(
-        `metric '${
-            metric.name}': no datatype in the source model; defaulting the ` +
-        `required semantic-metric.dataType to 'NUMERIC'`);
-    dataType = 'NUMERIC';
-  }
+    metric: Metric, emitExpressions: boolean): Record<string, any> {
+  const dataType = columnDataType(metric.type ?? 'Opaque');
   return compact({
     entity: metric.entity,
     dataType,