Skip to content

[finding] sql-driver.ts twice cites a Field.string builder that has never existed — and the prose demonstrably steered a ruling onto an unauthorable target #12593

Description

@os-warren

Filed unassigned from the premise measurement carried out under #12131. Not fixed there: that card's own verdict is a fork returned to the maintainer, and this is a separate prose defect in a file that card does not touch.

What was measured

Two comments in packages/drivers/driver-sql/src/sql-driver.ts (the text-family branch of createColumn) assert that a Field.string builder exists:

14492:        // finally agreeing with the differ. `Field.string` has always taken
14502:        // have been had the field been declared `Field.string`.

Measured on origin/main @ da1126a032, each measurement paired with a positive control:

  • The builder has no string member. Object.keys(Field) returns 37 keys; typeof Field.string === 'undefined'. Control: typeof Field.text === 'function'.
  • 'string' is not in the field vocabulary. FieldType.options has 49 members and does not include 'string'. Controls: 'text' present, 'zzznope' absent.
  • The spec refuses it. FieldSchema.safeParse({ type: 'string', label: 'ID' }) fails with invalid_value at [type]. Control: { type: 'text', … } parses.
  • It never existed. git log -S " string: (config: FieldInput" -- packages/spec/src/data/field.zod.ts returns no commits, as does git log -S "'string'," -- on the same file. No commit ever added or removed such a member — this is not stale-after-removal, it is a reference to an API that was never there.

The name is not arbitrary: 'string' is the knex column-builder method (table.string(name)), and the driver uses that spelling internally as its untyped default (const type = field?.type || 'string', plus a case 'string': branch at sql-driver.ts:14102 / :14385). That branch is reachable only via the default, never from a spec-valid declaration. So the driver has two vocabularies in play and the comments spell an authoring-side API in the storage-side one.

Why it matters

This is not a cosmetic comment nit. The prose has already produced a bad decision: the triage and decision-box analyses on #12131 both proposed, and the maintainer then ruled, a mechanical sweep rewriting ~45 system objects from Field.text(...) to Field.string(...). That sweep cannot be executed — it would put the platform's own metadata into a state FieldSchema rejects. The ruling was formed on the belief the builder exists, and the most reachable source for that belief is this file.

A comment that names a non-existent authoring API is exactly the "declared ≠ real" failure the repo's gates exist to close, one layer up: it is metadata about the platform, read by humans and increasingly by AI authors, and it is false.

Shape of a fix (not decided here)

Correct the two comments to name what is actually true — the emitted column is knex's table.string(name) i.e. varchar(255), and the authorable spelling closest to it is Field.text({ maxLength: n }) on a keyed column. Whether the driver's internal 'string' default and its case 'string': branch should also be renamed to stop colliding with an authoring vocabulary that has no such member is a larger question and deliberately not proposed here.

Related: #12131 (where it was measured), #12015.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions