docs(service-datasource): make the primaryKeyReader docblock true again, and record why the isPrimary arm stays

packages/services/service-datasource/src/__tests__/external-introspection-seam.test.ts

@@ -101,10 +101,15 @@ describe('the introspection seam, as a real SqlDriver actually spells it', () =>
  // so that it would redden the moment the driver was aligned. That flip has
  // now happened (#10676/#10998), and it is pinned here in its new
  // direction: the driver emits the spec spelling and no longer emits the
- // retired one. The union read in `primaryKeyReader` below therefore no
- // longer has an in-tree producer needing its `isPrimary` arm; collapsing
- // it is this lane's call to make, deliberately not made from the driver
- // change, and until then this file keeps both arms pinned.
+ // retired one. The union read in `primaryKeyReader` therefore has no
+ // in-tree producer left for its `isPrimary` arm — and that arm stays
+ // anyway. The services lane made that call in #11123: the seam's producer
+ // population is open by design (a host builds the driver, and the handle
+ // types introspection as `Promise<unknown>`), so the compiler channel the
+ // retirement relies on cannot reach a host-built driver still emitting the
+ // old spelling. Dropping the arm is a narrowing of accepted input, not a
+ // dead-code deletion. `primaryKeyReader`'s docblock carries the full
+ // measurement; this file keeps all the arms pinned.
  expect(table.primaryKeys).toEqual(['id']);
  expect(id.primaryKey).toBe(true);
  expect(id.isPrimary).toBeUndefined();
@@ -154,13 +159,18 @@ describe('the introspection seam, as a real SqlDriver actually spells it', () =>
 describe('the seam read, where the two spellings disagree', () => {
  /**
  * No in-tree driver produces a disagreement — `SqlDriver` derives
- * `isPrimary` FROM `primaryKeys`, so the two always agree. This case is
- * therefore hand-built ON PURPOSE, and it is the one place in this file
- * where that is the right instrument: it fixes the behaviour under a
- * disagreement no live database can currently stage, so a future producer
- * that fills only one of the two signals cannot silently lose half a
- * composite key. The spelling seam itself is pinned above, against a real
- * driver, where a fake would have been blind.
+ * `primaryKey` FROM `primaryKeys`, so its two signals always agree, and
+ * since #10676/#10998 it does not emit `isPrimary` at all. These two cases
+ * are therefore hand-built ON PURPOSE, and it is the one place in this file
+ * where that is the right instrument: no live in-tree database can stage
+ * this disagreement, and feeding the retired spelling is the ONLY exercise
+ * the union's `isPrimary` arm now has anywhere in the tree. They pin the
+ * behaviour a host-built driver still emitting that spelling depends on —
+ * see `primaryKeyReader`'s docblock for why the arm is kept rather than
+ * collapsed (#11123) — and they pin that a producer filling only one of the
+ * two signals cannot silently lose half a composite key. If the arm is ever
+ * retired, these two cases go with it. The spelling seam itself is pinned
+ * above, against a real driver, where a fake would have been blind.
  */
  function serviceOverRaw(table: unknown): ExternalDatasourceService {
  return serviceOver({ tables: { order_lines: table } });

packages/services/service-datasource/src/external-datasource-service.ts

@@ -118,50 +118,77 @@ export interface ExternalDatasourceServiceConfig {
 const BUILTIN_COLUMNS = new Set(['id', 'created_at', 'updated_at']);
 
 /**
- * Read "is this column part of the remote primary key" across the TWO
- * introspection contracts that meet at this service.
+ * Read "is this column part of the remote primary key" across the
+ * introspection spellings that can arrive at this service.
  *
  * `plugin.ts` hands the driver's `introspectSchema()` result to this service
- * unmodified, and the driver does not speak the contract this file is typed
- * against:
+ * unmodified. There is now ONE declared contract on both sides of that
+ * handoff — `packages/spec`'s `IntrospectedColumn` — and the in-tree driver
+ * speaks it:
  *
- * | producer | per-column | table-level |
- * | ---------------------------------------------- | -------------- | -------------- |
- * | `SqlDriver` (+ `SqliteWasmDriver`, which extends it) | `isPrimary` | `primaryKeys` |
- * | `packages/spec` `IntrospectedColumn` (what this file's types say) | `primaryKey` | — |
+ * | producer | per-column | table-level |
+ * | --------------------------------------------------------------------- | ------------ | ------------- |
+ * | `SqlDriver` (+ `SqliteWasmDriver` / `TursoDriver`, which extend it) | `primaryKey` | `primaryKeys` |
+ * | `packages/spec` `IntrospectedColumn` (what this file's types say) | `primaryKey` | — |
  *
- * Measured against a live SQLite database at `368e7a06f`: the driver's column
- * for a `primary key (id)` table carries `isPrimary: true` and the table
- * carries `primaryKeys: ['id']`, while `primaryKey` is `undefined`. Reading
- * only `col.primaryKey` therefore reads a key no in-tree driver ever sets, and
- * the remote key is silently lost.
+ * That agreement is NEW, and this reader predates it. Measured against a live
+ * SQLite database at `368e7a06f`, the driver's column for a `primary key (id)`
+ * table carried `isPrimary: true` and no `primaryKey` key at all, so reading
+ * only `col.primaryKey` lost the remote key — the defect this reader was
+ * written for. `95437e7d2d` (#10676 / #10998) retired that spelling at the
+ * producer: `introspectSchema` now derives `col.primaryKey` FROM `primaryKeys`
+ * and emits no `isPrimary`.
  *
- * This reads the UNION of all three signals rather than picking one:
+ * This still reads the UNION of three signals rather than picking one, and
+ * each arm is here for its own reason:
  *
- * - No in-tree producer uses `primaryKey: false` / `isPrimary: false` to
- *  NEGATE a key another signal asserts — the falses are just "not a key",
- * written by producers that fill exactly one of the three. A precedence
- * chain would therefore drop a real key whenever the winning signal is the
- * one its producer left blank, which is the defect being repaired here.
- * - A producer that fills only `table.primaryKeys` (the shape a table-level
- *  reader would naturally emit) is covered without needing a per-column flag,
- * and vice versa.
+ * - `col.primaryKey` — the spec spelling, what every in-tree producer writes.
+ * - `table.primaryKeys` — LIVE and INDEPENDENT, not a legacy arm. A
+ * per-column boolean cannot express key ORDER, and since #10997 (PR #11104)
+ * this list reports every member of a COMPOSITE key in declared key order.
+ * It is the only signal here that carries one. ⛔ Collapsing it away drops
+ *  composite-key handling at this seam.
+ * - `col.isPrimary` — a RETIRED spelling, kept as a compatibility belt. See
+ * the note below before touching it.
  *
- * When the per-column flag and the table-level list DISAGREE, the union takes
- * both. That is deliberate: for a federated table, under-reporting the key
- * costs the caller its addressing key, and no in-tree consumer treats a
- * column's PK-ness as an exclusive claim. Note that no in-tree driver produces
- * such a disagreement today — `SqlDriver` derives `isPrimary` FROM
- * `primaryKeys`, so the two always agree, including where both are wrong (a
- * SQLite composite key reports only its first column, because
- * `introspectPrimaryKeys` filters `PRAGMA table_info` on `pk === 1` while
- * SQLite numbers composite members `1, 2, ...`). That truncation is upstream
- * of this seam and is not repaired here.
+ * No producer uses `primaryKey: false` / `isPrimary: false` to NEGATE a key
+ * another signal asserts — the falses are just "not a key", written by
+ * producers that fill one signal and leave the others blank. A precedence
+ * chain would therefore drop a real key whenever the winning signal is the one
+ * its producer left blank, which is the defect being repaired here. When the
+ * per-column flag and the table-level list DISAGREE the union takes both: for
+ * a federated table, under-reporting the key costs the caller its addressing
+ * key, and no in-tree consumer treats a column's PK-ness as an exclusive
+ * claim. (The SQLite composite-key truncation this note used to disclaim is
+ * gone — #10997 repaired `introspectPrimaryKeys` upstream, so the driver's two
+ * signals now agree on the WHOLE key rather than agreeing on a truncated one.)
  *
- * Deliberately structural: the extra spellings are read off the value without
- * widening any declared contract, because reconciling
- * `packages/objectql/src/util.ts` with
- * `packages/spec/src/contracts/schema-diff-service.ts` is a spec-owned change.
+ * WHY THE `isPrimary` ARM STAYS, with no producer left in this tree — measured
+ * for #11123 on `52a41b72ee`, so the next reader need not re-derive it:
+ *
+ * - Nothing in-tree writes it. Every surviving whole-identifier hit is prose,
+ * and `objectql`'s `isPrimaryKeyField` merely CONTAINS the substring.
+ * - It is still not dead code, because the producer population here is open
+ * by design. `contracts/datasource-driver-factory.ts` says the framework
+ * "ships no universal driver-by-id registry" — concrete drivers are built
+ * by the HOST — and types the handle as `introspectSchema?(): Promise<unknown>`.
+ * The retirement shipped as a BREAKING change whose stated migration
+ * channel is the compiler, "precisely and at every site"; against an
+ * `unknown` result that channel never fires, so a host-built driver still
+ * emitting the old spelling is reached by nothing and would silently lose
+ * its key here.
+ * - The belt's clock has not run either: the union (#11001) and the
+ * retirement (#11124) are BOTH still unconsumed changesets at `17.1.0`, so
+ * no released version has ever emitted `primaryKey` from this driver.
+ *
+ * Dropping the arm is therefore a NARROWING OF ACCEPTED INPUT rather than a
+ * dead-code deletion, and wants the contract-review gate. Its only exercise is
+ * the staged-disagreement pair in
+ * `__tests__/external-introspection-seam.test.ts`; those cases go with it.
+ *
+ * Deliberately structural: the retired spelling is read off the value without
+ * widening any declared contract — no declared type carries it any more, so
+ * there is nothing left to read it through.
  */
 function primaryKeyReader(table: IntrospectedTable): (col: IntrospectedColumn) => boolean {
  const declared = (table as unknown as { primaryKeys?: unknown }).primaryKeys;
@@ -481,11 +508,13 @@ export class ExternalDatasourceService implements IExternalDatasourceService {
  dialect: schema.dialect,
  tables: Object.values(schema.tables).map((t) => {
  const { schema: s, name } = parseQualified(t.name);
- // The introspection seam: a real driver spells this `isPrimary` /
- // `primaryKeys`, never `primaryKey`. `ExternalCatalogSchema` defaults
- // the key to `false`, so reading only `c.primaryKey` persisted a
- // catalog in which EVERY column claimed not to be part of the remote
- // key — including the ones that are.
+ // The introspection seam. `ExternalCatalogSchema` defaults this key to
+ // `false`, so when the in-tree driver still spelled it `isPrimary`,
+ // reading only `c.primaryKey` persisted a catalog in which EVERY column
+ // claimed not to be part of the remote key — including the ones that
+ // are. The driver has since been aligned to the spec spelling
+ // (`95437e7d2d`), but this must stay a `primaryKeyReader` call: see its
+ // docblock for the two arms that are still load-bearing.
  const isPk = primaryKeyReader(t);
  return {
  remoteSchema: s,