fix(analytics)!: declare the sqlDialect accept set and diagnose an out-of-contract answer (#16206)

Claude-Session: https://claude.ai/code/session_01ToDPcx9AESFubJkDiFMtKW
Co-authored-by: Claude <noreply@anthropic.com>

Author: claude

.changeset/analytics-sqldialect-declared-vocabulary.md

@@ -0,0 +1,63 @@
+---
+"@objectstack/service-analytics": minor
+---
+
+fix(analytics)!: `AnalyticsServiceConfig.sqlDialect` declares its three-name accept set, and a host that answers outside it is told once (#16206)
+
+<!-- adr-0087: not-required (runtime-interface-only packages/services/service-analytics/src/analytics-service.ts#AnalyticsServiceConfig) The narrowed member is one hook on a service CONSTRUCTOR CONFIG — a published runtime TypeScript interface with no metadata surface. It has no Zod schema, no `packages/spec` declaration and no stored representation, so `objectstack migrate meta`, `spec-changes.json` and the generated upgrade guide have nothing to rewrite; the affected party is a TypeScript host and the channel that reaches every one of them is the compiler at their own composition site. No metadata key is added, removed, renamed or re-shaped, and `packages/spec` is untouched by this diff. -->
+
+**BREAKING** for a TypeScript host that declares its `sqlDialect` hook as returning
+`string`: the hook's declared return is now the three canonical dialect names or
+`undefined`, so such a composition stops compiling until the host's own annotation
+says which names it can answer. Shipped as `minor` under the repo's launch-window
+convention, in which breaking-ness is carried by this banner and the disposition
+above rather than by the bump level. Runtime behaviour for every host is unchanged:
+the same three names were the only ones that ever did anything.
+
+## What was wrong
+
+`AnalyticsServiceConfig.sqlDialect` — the hook a host answers to say which SQL
+dialect backs an object — was typed as free `string`, while `normalizeSqlDialect`
+has only ever recognised `sqlite`, `postgres` and `mysql`. Nothing said so, and
+nothing told a host that answered otherwise.
+
+So a host that owns a SQLite datasource and answers the spelling its own stack uses
+— knex's canonical `sqlite3`, or `better-sqlite3`, both of which `driver-sql` itself
+lists in `SQLITE_EMIT_CLIENTS` — was read as `unknown`. And because `sqlDialectFor`
+is tiered "cannot answer, do not block", **a wrong answer and no answer were the
+same answer**: the host that tried hardest to help got the residue arm, silently.
+
+## What it does now
+
+- **The vocabulary is declared**, on the type and in the docblock, as
+  `AcceptedSqlDialect` — `sqlite` | `postgres` | `mysql` — so a host reading the
+  config learns the accept set without running anything. The type and the runtime
+  membership set are generated from one `const` tuple, so a future widening cannot
+  land in one and miss the other.
+- **A non-empty answer outside the set is diagnosed**: one `warn` naming the object,
+  the answer and the accepted set. It is emitted **once per distinct unrecognised
+  spelling** — the failure's identity — so the line count is bounded by the host's
+  own hook and never grows with query volume.
+- **`undefined` stays silent and legal.** The hook is optional and "cannot answer,
+  do not block" is a supported composition, not a misconfiguration. A pin holds both
+  halves, because a diagnostic that also shouted at hosts who wired nothing would be
+  a worse defect than the one being fixed.
+- **The accept set is NOT widened.** Teaching this package `driver-sql`'s knex
+  aliases would be a second copy of that driver's table, and an unrecognised
+  spelling is sometimes deliberate (`mariadb`, #11756). The answer is still read as
+  `unknown`; only the silence changed.
+- **The plugin bridge translates the driver's own residue.** `SqlDriver.dialectName`
+  carries a fourth name, `unknown`, meaning "I cannot say"; handed on verbatim it
+  would have presented a correctly-behaving driver as a host answering out of
+  contract. It now arrives as `undefined`, this hook's own spelling for the same
+  thing. The dialect the compilers end up with is unchanged either way.
+
+## Measured, and worth reading before relying on the residue arm
+
+Driven on sql.js through a host answering `sqlite3`, against the shared
+`FILTER_TEXT_CASES` fixture, with a host answering `sqlite` as the control: **five of
+the six case-EXACT cases come back with the wrong rows** — every case that
+discriminates on ASCII case. `{ name: { $contains: 'acme' } }` answers `['1','2']`
+where the table says `['2']`, and the negated form DROPS a row that belongs in the
+result. That is #15684's fold, live on the arm this population lands on, and it is
+reported rather than fixed here: closing it is that card's business, not this one's.

packages/services/service-analytics/src/__tests__/sql-dialect-vocabulary.test.ts

@@ -98,10 +98,16 @@ const CUBE: Cube = {
   public: false,
 } as unknown as Cube;
 
-/** A second cube over the SAME table, so "one answer, many objects" is drivable. */
+/**
+ * A second cube over a DIFFERENT object, so "one answer, many objects" is
+ * drivable. ⚠️ The hook is asked about the OBJECT the cube reads (`sql`), not
+ * about the cube — which is why the line below names `rows`, and why this
+ * twin has to point somewhere else to be a second object at all.
+ */
 const OTHER_CUBE: Cube = {
   ...(CUBE as unknown as Record<string, unknown>),
   name: 'other_texts',
+  sql: 'other_rows',
 } as unknown as Cube;
 
 const query = (where: unknown, cube = 'texts'): AnalyticsQuery =>
@@ -132,6 +138,10 @@ const serviceAnswering = (
     logger: logger as unknown as AnalyticsServiceConfig['logger'],
     cubes: [CUBE, OTHER_CUBE],
     queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: true, inMemory: false }),
+    // `NativeSQLStrategy.canHandle` requires a raw-SQL door to exist. Nothing
+    // below EXECUTES through it — the SQL is minted by `generateSql` and run on
+    // sql.js directly — so it only has to be present and typed.
+    executeRawSql: async () => [] as Record<string, unknown>[],
     sqlDialect: hook as unknown as AnalyticsServiceConfig['sqlDialect'],
   } satisfies AnalyticsServiceConfig;
   return { service: new AnalyticsService(config), logger };
@@ -145,6 +155,7 @@ const serviceAnsweringNothing = (): { service: AnalyticsService; logger: TestLog
       logger: logger as unknown as AnalyticsServiceConfig['logger'],
       cubes: [CUBE, OTHER_CUBE],
       queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: true, inMemory: false }),
+      executeRawSql: async () => [] as Record<string, unknown>[],
     }),
     logger,
   };
@@ -205,7 +216,8 @@ describe('[#16206] the ruling\'s named pin — both halves', () => {
     const warnings = dialectWarnings(logger);
     expect(warnings).toHaveLength(1);
     expect(warnings[0]).toContain('"sqlite3"');
-    expect(warnings[0]).toContain('"texts"');
+    // The OBJECT the hook was asked about — `texts` reads the object `rows`.
+    expect(warnings[0]).toContain('"rows"');
     for (const accepted of ACCEPTED_SQL_DIALECTS) expect(warnings[0], accepted).toContain(accepted);
   });
 
@@ -246,8 +258,10 @@ describe('[#16206] "once" is keyed on the failure\'s identity, and is bounded',
     const warnings = dialectWarnings(logger);
     expect(warnings).toHaveLength(1);
     // The FIRST object to elicit it is the one named — a concrete place to look,
-    // not a count that grows with the object registry.
-    expect(warnings[0]).toContain('"texts"');
+    // not a count that grows with the object registry. Two DIFFERENT objects
+    // were asked about (`rows` and `other_rows`); one line came out.
+    expect(warnings[0]).toContain('"rows"');
+    expect(warnings[0]).not.toContain('"other_rows"');
   });
 
   it('a SECOND, DIFFERENT wrong answer is a second failure and gets its own line', async () => {
@@ -369,20 +383,40 @@ describe('[#16206] the `unknown` arm\'s ROWS for a `sqlite3`-answering host, on
         wrong.push({ case: c.name, expected: [...c.expected], measured });
       }
     }
-    // At least the two case-folding rows of the shared table come back wrong.
-    expect(wrong.length).toBeGreaterThan(0);
-
-    // Named, so the report reads the rows rather than a count.
+    // FIVE of the shared table's SIX case-exact cases come back wrong — every
+    // one that discriminates on ASCII case. The sixth (`$contains 'a_b'`) is
+    // the LIKE-metacharacter row, which carries no cased letter to fold, and is
+    // the reason this is a count and not "all of them".
+    expect(wrong.map((w) => w.case)).toEqual([
+      '$contains is case-SENSITIVE — a lower-case comparand misses the upper-case row',
+      '$contains is case-SENSITIVE — an upper-case comparand misses the lower-case row',
+      '$startsWith is case-SENSITIVE',
+      '$endsWith is case-SENSITIVE',
+      '$notContains is case-SENSITIVE, and negation does not widen it',
+    ]);
+
+    // Named, so the record reads the rows rather than a count.
     expect(await executedIds({ name: { $contains: 'acme' } }, sqlite3Host)).toEqual(['1', '2']);
     expect(await executedIds({ name: { $contains: 'acme' } }, sqliteHost)).toEqual(['2']);
+    expect(await executedIds({ name: { $contains: 'ACME' } }, sqlite3Host)).toEqual(['1', '2']);
+    expect(await executedIds({ name: { $contains: 'ACME' } }, sqliteHost)).toEqual(['1']);
     expect(await executedIds({ name: { $startsWith: 'ACME' } }, sqlite3Host)).toEqual(['1', '2']);
     expect(await executedIds({ name: { $startsWith: 'ACME' } }, sqliteHost)).toEqual(['1']);
     expect(await executedIds({ name: { $endsWith: 'corp' } }, sqlite3Host)).toEqual(['1', '2']);
     expect(await executedIds({ name: { $endsWith: 'corp' } }, sqliteHost)).toEqual(['2']);
-    // Negation does not widen it back: the over-match becomes an under-match.
+    // ⭐ Negation turns the over-match into an UNDER-match: row 1 is DROPPED
+    // from a result set that should contain it. On a read scope that direction
+    // hides rows; on the query's own `where` it is a wrong chart.
     expect(await executedIds({ name: { $notContains: 'acme' } }, sqlite3Host))
       .toEqual(['3', '4', '5', '6', '7', '8', '9']);
     expect(await executedIds({ name: { $notContains: 'acme' } }, sqliteHost))
       .toEqual(['1', '3', '4', '5', '6', '7', '8', '9']);
+
+    // The construct that causes it, so the finding names a mechanism: the
+    // residue arm's plain `LIKE`, which SQLite folds ASCII case on.
+    const viaSqlite3 = await sqlite3Host.generateSql(query({ name: { $contains: 'acme' } }));
+    const viaSqlite = await sqliteHost.generateSql(query({ name: { $contains: 'acme' } }));
+    expect(viaSqlite3.sql).toContain('LIKE');
+    expect(viaSqlite.sql).toContain('GLOB');
   });
 });

packages/services/service-analytics/src/plugin.ts

@@ -12,6 +12,9 @@ import type { AnalyticsDriverCapabilities } from './strategies/types.js';
 import { pickDisplayField, type DimensionLabelDeps } from './dimension-labels.js';
 import { assertReadScopeCannotVacate } from './read-scope-sql.js';
 import { readScopeUnresolvedError } from './read-scope-refusal.js';
+// [#16206] The narrowing from a driver's FOUR-name `dialectName` to the THREE
+// this package's config hook declares — see the bridge below.
+import { asAcceptedSqlDialect, type AcceptedSqlDialect } from './text-match-sql.js';
 
 /**
  * The slice of the DECLARED engine contracts this plugin's auto-bridges
@@ -998,13 +1001,24 @@ export class AnalyticsServicePlugin implements Plugin {
      * `undefined` on every tier that cannot answer — no data engine, a driver
      * that names no dialect (memory, mongo), a throw — and `undefined` keeps
      * the plain `LIKE`, which is exactly the pre-#15684 behaviour.
+     *
+     * [#16206] ⭐ A FIFTH tier that cannot answer, and the reason this is not a
+     * verbatim pass-through: `SqlDriver.dialectName` is a FOUR-name vocabulary
+     * whose fourth name is `'unknown'` — that driver's own "I cannot say", which
+     * is what it returns for a client it does not model (`'mariadb'`, left
+     * unrecognised on purpose by #11756). The config hook's accept set is the
+     * other THREE, so handing `'unknown'` on verbatim would present a driver
+     * behaving correctly as a host answering out of contract, and every such
+     * deployment would carry a warning about itself. ⇒ The residue is
+     * translated to this hook's own spelling for the same thing, `undefined`.
+     * The dialect the compilers end up with is unchanged either way.
      */
-    const sqlDialect = (objectName: string): string | undefined => {
+    const sqlDialect = (objectName: string): AcceptedSqlDialect | undefined => {
       try {
         const svc = ctx.getService<DataEngineLike>('data');
         const driver = svc?.getDriverForObject?.(objectName) as DialectNamingDriver | undefined;
         const named = driver?.dialectName;
-        return typeof named === 'string' ? named : undefined;
+        return asAcceptedSqlDialect(typeof named === 'string' ? named : undefined);
       } catch {
         // Same tiering as the temporal hooks: an unresolvable driver keeps the
         // dialect-blind construct, which is today's behaviour.

packages/services/service-analytics/src/text-match-sql.ts

@@ -256,6 +256,24 @@ export function isUnrecognisedSqlDialectAnswer(name: string | undefined | null):
   return typeof name === 'string' && name.length > 0 && !KNOWN_DIALECTS.has(name);
 }
 
+/**
+ * [#16206] `name` if it is one of {@link ACCEPTED_SQL_DIALECTS}, else
+ * `undefined` — the narrowing a caller needs when it must hand an answer on to
+ * something that declares the accept set.
+ *
+ * ⚠️ It exists for the bridge in `plugin.ts`, which answers from a `SqlDriver`'s
+ * `dialectName` — a FOUR-name vocabulary whose fourth name is `'unknown'`, that
+ * driver's own "I cannot say". Passed through verbatim, that residue would
+ * arrive at the config hook looking like a considered answer outside the accept
+ * set, and the host would be warned about a driver doing exactly the right
+ * thing (and about `driver-sql`'s deliberately unrecognised spellings, #11756).
+ * ⇒ Translate the residue to the hook's own spelling for "cannot answer" —
+ * `undefined` — rather than teaching the diagnostic a list of exceptions.
+ */
+export function asAcceptedSqlDialect(name: string | undefined | null): AcceptedSqlDialect | undefined {
+  return typeof name === 'string' && KNOWN_DIALECTS.has(name) ? (name as AcceptedSqlDialect) : undefined;
+}
+
 /**
  * The dialect of the datasource backing `objectName`, read off the context's
  * `sqlDialect` hook — `'unknown'` when the host wired none.