Make accessor required for table-level index defs in typescript (#4525)

# Description of Changes

This patch contains two main changes:

1. It makes `accessor` a required argument for table-level index defs in
typescript to align with rust.

2. It removes unsound index typing in the TypeScript bindings by
splitting index data into two explicit representations:

- `indexes`: declarative user config (`IndexOpts`) used for type
inference
- `resolvedIndexes`: normalized runtime metadata (`UntypedIndex`)
derived from `RawTableDefV10`

`TableCacheImpl` now builds index accessors from `resolvedIndexes`
instead of re-casting `indexes` at runtime.

    This addressed the following comment:
    ```
// TODO: horrible horrible horrible. we smuggle this
`Array<UntypedIndex>`
// by casting it to an `Array<IndexOpts>` as `TableToSchema` expects.
// This is then used in `TableCacheImpl.constructor` and who knows where
else.
    // We should stop lying about our types.
    ```

    > Why?

    We were conflating two different concepts under `tableDef.indexes`:

      1. declared table-level index options authored by users
2. resolved runtime index definitions (including field-level inferred
indexes)

This required unsafe casts (`T['idxs']` / `UntypedIndex`) and made the
type model unsound.

Note, (2) was largely ai assisted.

# API and ABI breaking changes

Technically breaks the module api, although I believe this is the
behavior that is outlined in the spec, and so the current behavior
should really be considered a bug.

# Expected complexity level and risk

2

# Testing

Added unit tests covering the following:
1. Table-level explicit index without accessor throws.
2. Table-level duplicate accessor throws.
3. Table-level explicit accessor is accepted and used.
4. Field-level `.index(...)` derives accessor from the field name
(`displayName`).

Author: joshua-spacetime

crates/bindings-typescript/src/lib/indexes.ts

@@ -9,7 +9,7 @@ import type { ColumnIsUnique } from './constraints';
  * existing column names are referenced.
  */
 export type IndexOpts<AllowedCol extends string> = {
-  accessor?: string;
+  accessor: string;
   name?: string;
 } & (
   | { algorithm: 'btree'; columns: readonly AllowedCol[] }

crates/bindings-typescript/src/lib/schema.ts

@@ -61,20 +61,35 @@ export interface TableToSchema<
   accessorName: AccName;
   columns: T['rowType']['row'];
   rowType: T['rowSpacetimeType'];
+  // Declarative user-provided table-level indexes.
   indexes: T['idxs'];
+  // Resolved runtime index metadata used by runtime consumers (e.g. TableCache).
+  resolvedIndexes: readonly UntypedIndex<keyof T['rowType']['row'] & string>[];
   constraints: T['constraints'];
 }
 
 export function tablesToSchema<
   const T extends Record<string, UntypedTableSchema>,
 >(ctx: ModuleContext, tables: T): TablesToSchema<T> {
+  // `TablesToSchema<T>['tables']` is intentionally readonly in the public type,
+  // but we need a mutable builder while materializing it from entries.
+  type MutableTableDefs = {
+    -readonly [AccName in keyof TablesToSchema<T>['tables']]: TablesToSchema<T>['tables'][AccName];
+  };
+  const tableDefs = Object.create(null) as MutableTableDefs;
+  for (const [accName, schema] of Object.entries(tables) as [
+    keyof T & string,
+    T[keyof T & string],
+  ][]) {
+    tableDefs[accName] = tableToSchema(
+      accName,
+      schema,
+      schema.tableDef(ctx, accName)
+    ) as TablesToSchema<T>['tables'][typeof accName];
+  }
+
   return {
-    tables: Object.fromEntries(
-      Object.entries(tables).map(([accName, schema]) => [
-        accName,
-        tableToSchema(accName, schema, schema.tableDef(ctx, accName)),
-      ])
-    ) as TablesToSchema<T>['tables'],
+    tables: tableDefs as TablesToSchema<T>['tables'],
   };
 }
 
@@ -90,6 +105,46 @@ export function tableToSchema<
     schema.rowType.algebraicType.value.elements[i].name;
 
   type AllowedCol = keyof T['rowType']['row'] & string;
+  // Build fully-resolved runtime index metadata from the host-facing RawTableDef.
+  // This is intentionally separate from `schema.idxs`, which keeps the original
+  // user-declared `IndexOpts` shape for type-level inference.
+  const resolvedIndexes: UntypedIndex<AllowedCol>[] = tableDef.indexes.map(
+    idx => {
+      const accessorName = idx.accessorName;
+      if (typeof accessorName !== 'string' || accessorName.length === 0) {
+        throw new TypeError(
+          `Index '${idx.sourceName ?? '<unknown>'}' on table '${tableDef.sourceName}' is missing accessor name`
+        );
+      }
+
+      const columnIds =
+        idx.algorithm.tag === 'Direct'
+          ? [idx.algorithm.value]
+          : idx.algorithm.value;
+
+      const unique = tableDef.constraints.some(
+        c =>
+          c.data.tag === 'Unique' &&
+          c.data.value.columns.every(col => columnIds.includes(col))
+      );
+
+      const algorithm = (
+        {
+          BTree: 'btree',
+          Hash: 'hash',
+          Direct: 'direct',
+        } as const
+      )[idx.algorithm.tag];
+
+      return {
+        name: accessorName,
+        unique,
+        algorithm,
+        columns: columnIds.map(getColName) as AllowedCol[],
+      };
+    }
+  );
+
   return {
     // For client,`schama.tableName` will always be there as canonical name.
     // For module, if explicit name is not provided via `name`, accessor name will
@@ -98,29 +153,16 @@ export function tableToSchema<
     accessorName: accName,
     columns: schema.rowType.row, // typed as T[i]['rowType']['row'] under TablesToSchema<T>
     rowType: schema.rowSpacetimeType,
+    // Keep declarative indexes in their original shape for type-level consumers.
+    indexes: schema.idxs,
     constraints: tableDef.constraints.map(c => ({
       name: c.sourceName,
       constraint: 'unique',
       columns: c.data.value.columns.map(getColName) as [string],
     })),
-    // TODO: horrible horrible horrible. we smuggle this `Array<UntypedIndex>`
-    // by casting it to an `Array<IndexOpts>` as `TableToSchema` expects.
-    // This is then used in `TableCacheImpl.constructor` and who knows where else.
-    // We should stop lying about our types.
-    indexes: tableDef.indexes.map((idx): UntypedIndex<AllowedCol> => {
-      const columnIds =
-        idx.algorithm.tag === 'Direct'
-          ? [idx.algorithm.value]
-          : idx.algorithm.value;
-      return {
-        name: idx.accessorName!,
-        unique: tableDef.constraints.some(c =>
-          c.data.value.columns.every(col => columnIds.includes(col))
-        ),
-        algorithm: idx.algorithm.tag.toLowerCase() as 'btree',
-        columns: columnIds.map(getColName),
-      };
-    }) as T['idxs'],
+    // Expose resolved runtime indexes separately so runtime users don't have to
+    // reinterpret `indexes` with unsafe casts.
+    resolvedIndexes,
     tableDef,
     ...(tableDef.isEvent ? { isEvent: true } : {}),
   };

crates/bindings-typescript/src/lib/table.ts

@@ -17,6 +17,7 @@ import type {
   Indexes,
   IndexOpts,
   ReadonlyIndexes,
+  UntypedIndex,
 } from './indexes';
 import ScheduleAt from './schedule_at';
 import type { TableSchema } from './table_schema';
@@ -116,7 +117,25 @@ export type UntypedTableDef = {
   columns: Record<string, ColumnBuilder<any, any, ColumnMetadata<any>>>;
   // This is really just a ProductType where all the elements have names.
   rowType: RowBuilder<RowObj>['algebraicType']['value'];
+  /**
+   * Declarative multi-column indexes supplied by user code in `table({ indexes: [...] }, ...)`.
+   *
+   * This is intentionally the *declarative* shape (`IndexOpts`) because a lot of
+   * type-level behavior is derived from these entries (for example query-builder
+   * inference over composite indexes).
+   */
   indexes: readonly IndexOpts<any>[];
+  /**
+   * Fully-resolved runtime indexes materialized from `RawTableDefV10`.
+   *
+   * This contains both:
+   * 1) field-level indexes inferred from column metadata, and
+   * 2) explicit table-level indexes.
+   *
+   * Runtime consumers like `TableCacheImpl` should use this field instead of
+   * reinterpreting `indexes` as runtime index metadata.
+   */
+  resolvedIndexes: readonly UntypedIndex<any>[];
   constraints: readonly ConstraintOpts<any>[];
   tableDef: RawTableDefV10;
   isEvent?: boolean;
@@ -400,6 +419,14 @@ export function table<Row extends RowObj, const Opts extends TableOpts<Row>>(
 
   // convert explicit multi‑column indexes coming from options.indexes
   for (const indexOpts of userIndexes ?? []) {
+    const accessor = indexOpts.accessor;
+    if (typeof accessor !== 'string' || accessor.length === 0) {
+      const tableLabel = name ?? '<unnamed>';
+      const indexLabel = indexOpts.name ?? '<unnamed>';
+      throw new TypeError(
+        `Index '${indexLabel}' on table '${tableLabel}' must define a non-empty 'accessor'`
+      );
+    }
     let algorithm: RawIndexAlgorithm;
     switch (indexOpts.algorithm) {
       case 'btree':
@@ -418,16 +445,19 @@ export function table<Row extends RowObj, const Opts extends TableOpts<Row>>(
         algorithm = { tag: 'Direct', value: colIds.get(indexOpts.column)! };
         break;
     }
-    // unnamed indexes will be assigned a globally unique name
-    // The name users supply is actually the accessor name which will be used
-    // in TypeScript to access the index. This will be used verbatim.
-    // This is confusing because it is not the index name and there is
-    // no actual way for the user to set the actual index name.
-    // I think we should standardize: name and accessorName as the way to set
-    // the name and accessor name of an index across all SDKs.
+
+    // Unnamed indexes are assigned a globally unique source name.
+    // `accessor` controls the TypeScript property used to access the index.
+    // `name` (if present) is preserved as the canonical schema name.
+    //
+    // IMPORTANT: we intentionally do not reject duplicate accessor names here.
+    // This preserves existing behavior for raw table definitions. Downstream
+    // runtime consumers decide how duplicates are resolved:
+    // - server runtime merges duplicate accessors onto one accessor object
+    // - client cache assignment is last-write-wins for duplicate accessors
     indexes.push({
       sourceName: undefined,
-      accessorName: indexOpts.accessor,
+      accessorName: accessor,
       algorithm,
       canonicalName: indexOpts.name,
     });
@@ -496,7 +526,9 @@ export function table<Row extends RowObj, const Opts extends TableOpts<Row>>(
         isEvent,
       };
     },
-    idxs: {} as OptsIndices<Opts>,
+    // Preserve the declared index options as runtime data so `tableToSchema`
+    // can expose them without type-smuggling.
+    idxs: userIndexes as OptsIndices<Opts>,
     constraints: constraints as OptsConstraints<Opts>,
     schedule,
   };

crates/bindings-typescript/src/sdk/table_cache.ts

@@ -12,7 +12,6 @@ import type {
   ReadonlyIndexes,
   ReadonlyRangedIndex,
   ReadonlyUniqueIndex,
-  UntypedIndex,
 } from '../lib/indexes.ts';
 import type { Bound } from '../server/range.ts';
 import type { Prettify } from '../lib/type_util.ts';
@@ -84,23 +83,27 @@ export class TableCacheImpl<
     this.tableDef = tableDef;
     this.rows = new Map();
     this.emitter = new EventEmitter();
-    // Build indexes
-    const indexesDef = this.tableDef.indexes || [];
-    for (const idx of indexesDef) {
-      // TODO: don't do this. See comment in `tableToSchema` in `schema.ts`
-      const idxDef = idx as UntypedIndex<
-        keyof TableDefForTableName<RemoteModule, TableName>['columns'] & string
-      >;
+    // Build index views from the resolved runtime index metadata.
+    //
+    // We intentionally use `resolvedIndexes` rather than `indexes`:
+    // - `indexes` is declarative table-level config (`IndexOpts`) used mainly for typing.
+    // - `resolvedIndexes` is the runtime shape (`UntypedIndex`) that includes both
+    //   field-level and explicit table-level indexes.
+    for (const idxDef of this.tableDef.resolvedIndexes) {
       const index = this.#makeReadonlyIndex(this.tableDef, idxDef);
+      // IMPORTANT: for duplicate accessor names, client cache uses assignment
+      // semantics and later entries overwrite earlier ones. This matches prior
+      // behavior and is intentionally different from server runtime merge logic.
       (this as any)[idxDef.name] = index;
     }
   }
 
   // TODO: this just scans the whole table; we should build proper index structures
   #makeReadonlyIndex<
-    I extends UntypedIndex<
-      keyof TableDefForTableName<RemoteModule, TableName>['columns'] & string
-    >,
+    I extends TableDefForTableName<
+      RemoteModule,
+      TableName
+    >['resolvedIndexes'][number],
   >(
     tableDef: TableDefForTableName<RemoteModule, TableName>,
     idx: I

crates/bindings-typescript/src/server/runtime.ts

@@ -516,6 +516,7 @@ function makeTableView(
   ) as Table<any>;
 
   for (const indexDef of table.indexes) {
+    const accessorName = indexDef.accessorName!;
     const index_id = sys.index_id_from_name(indexDef.sourceName!);
 
     let column_ids: number[];
@@ -795,10 +796,13 @@ function makeTableView(
       } as RangedIndex<any, any>;
     }
 
-    if (Object.hasOwn(tableView, indexDef.accessorName!)) {
-      freeze(Object.assign(tableView[indexDef.accessorName!], index));
+    // IMPORTANT: duplicate accessor handling.
+    // When multiple raw indexes share the same accessor name, we merge index
+    // methods onto a single accessor object instead of throwing.
+    if (Object.hasOwn(tableView, accessorName)) {
+      freeze(Object.assign((tableView as any)[accessorName], index));
     } else {
-      tableView[indexDef.accessorName!] = freeze(index) as any;
+      (tableView as any)[accessorName] = freeze(index);
     }
   }
 

crates/bindings-typescript/src/server/schema.test-d.ts

@@ -60,3 +60,40 @@ spacetimedb.init(ctx => {
   // @ts-expect-error update() is NOT allowed on non-PK unique indexes
   const _update = ctx.db.person.name2.update;
 });
+
+/**
+ * Regression coverage for the declared-vs-resolved index split:
+ * - declared table-level indexes must still produce typed accessors
+ * - field-level indexes must still produce typed accessors
+ * - non-indexed fields must not accidentally become index accessors
+ */
+const account = table(
+  {
+    indexes: [
+      {
+        accessor: 'byEmailAndOrg',
+        algorithm: 'btree',
+        columns: ['email', 'orgId'] as const,
+      },
+    ] as const,
+  },
+  {
+    accountId: t.u32(),
+    email: t.string().index('hash'),
+    orgId: t.u32(),
+    nickname: t.string(),
+  }
+);
+
+const spacetimedbIndexSplit = schema({ account });
+
+spacetimedbIndexSplit.init(ctx => {
+  // Explicit table-level index accessor from `table({ indexes: [...] })`.
+  ctx.db.account.byEmailAndOrg.filter(['a@example.com', 1]);
+
+  // Field-level index accessor derived from column metadata.
+  ctx.db.account.email.filter('a@example.com');
+
+  // @ts-expect-error `nickname` is not indexed, so no index accessor should exist.
+  const _nickname = ctx.db.account.nickname;
+});

crates/bindings-typescript/src/server/view.test-d.ts

@@ -7,6 +7,7 @@ const person = table(
     name: 'person',
     indexes: [
       {
+        accessor: 'name_id_idx',
         name: 'name_id_idx',
         algorithm: 'btree',
         columns: ['name', 'id'] as const,
@@ -48,6 +49,7 @@ const order = table(
     name: 'order',
     indexes: [
       {
+        accessor: 'id_person_id',
         name: 'id_person_id', // We are adding this to make sure `person_id` still isn't considered indexed.
         algorithm: 'btree',
         columns: ['id', 'person_id'] as const,