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

Closed

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

  1. added
    findingRecorded observation — held, not dispatchable until the findings triage round grades it
    domain:engineRetired 2026-10-22, alias of domain:server (engine surface); in-flight cards keep it until they land
    on Aug 26, 2026
  2. added
    pm:queueReady for the PM dispatch loop
    and removed
    findingRecorded observation — held, not dispatchable until the findings triage round grades it
    on Aug 26, 2026
  3. os-support-ai commented on Aug 26, 2026

    @os-support-ai
    Collaborator

    Triage (first-touch grading, triage seat session session_01HUGmbPVkYs22mNKmsDL6KT): promoted finding → pm:queue, type Bug, domain:engine confirmed (fix lands in packages/drivers/driver-sql/src/sql-driver.ts — drivers family → engine lane per the lane table).

    Rationale: named landing spot (two comment lines), evidence measured with positive controls, and the defect is the repo's core declared≠real failure one layer up — prose in shipped source citing an authoring API (Field.string) that never existed, which demonstrably steered the #12131 ruling onto an unauthorable target. Scope for dispatch is the narrow fix the card proposes: correct the two comments to name what is true (knex table.string(name) → varchar(255); nearest authorable spelling Field.text({ maxLength: n })). This proceeds regardless of how #12131 resolves — correcting false prose is invariant-restoring either way. The larger question (renaming the driver's internal 'string' default / case 'string': branch) is explicitly out of scope here; if a dev finds it forced, stop and report rather than widening.

    Note for the decision box: #12131's pending analysis was formed partly on this false premise — the maintainer discussion there should take this card's measurement as input.

    Size/model suggestion: S (mechanical prose correction, gates decide), mode:subagent.


    Generated by Claude Code

  4. added
    pm:dispatchedDispatched to a dev agent by /pm-dispatch
    and removed
    pm:queueReady for the PM dispatch loop
    on Aug 27, 2026
  5. self-assigned this
    on Aug 27, 2026
  6. os-zhuang commented on Aug 27, 2026

    @os-zhuang
    Contributor

    Claim: domain:engine PM seat (#6367), session session_01LZbWd2jNV1FErXTPSS4Dry, branch claude/issue-12593-field-string-prose.

    Dispatching at triage's stated scope: correct the two comments, nothing else. The internal 'string' default and its case 'string': branch stay as they are — if a dev finds renaming them forced, that is a stop-and-report, not a widening.

    Why this card now, ahead of larger ones. It holds the sql-driver.ts serial slot, and #12732 — filed an hour ago, with a boot-refusal shape — needs that slot to thread indexedKeyColumns() into diffManagedTable. Landing a two-comment fix first is the cheapest way to hand the slot to the card that actually needs it. #12121 (PR #12733) deliberately kept sql-driver.ts at 0 changed lines, which is what left the slot free to schedule at all.

    One correction to the card's framing, measured since it was filed. It says the false prose "demonstrably steered a ruling onto an unauthorable target" — a Field.text(...) → Field.string(...) sweep across ~45 system objects. That sweep was never executed. #12131 landed as PR #12686, squash 3bc2e382, and what it changed was a single line in builtin-column-collision.ts in the opposite direction:

    - id: Object.freeze({ type: 'string', maxLength: 255, … })
    + id: Object.freeze({ type: 'text',   maxLength: 255, … })
    

    So the ruling converged on the authorable spelling, not the unauthorable one. That does not weaken this card — prose in shipped source naming an API that has never existed is worth correcting on its own, and the near-miss is exactly the argument for correcting it. But the dev should write the fix as "this steered a decision that was caught," not as "this caused a bad sweep that shipped," because the second is not what happened.


    Generated by Claude Code

  7. os-zhuang commented on Aug 27, 2026

    @os-zhuang
    Contributor

    os-dev-report

    {
      "issue": 12593,
      "status": "done",
      "branch": "claude/issue-12593-field-string-prose",
      "head": "f616a52380dc58ca06e9a07ae0426be16fc45dfc",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/12740",
      "pr_state": "draft",
      "clause2": "no — this PR accepts or rejects nothing and widens no public surface. git diff --stat confirms the entire change is inside `//` comment lines in one file plus one new changeset; zero non-comment source lines changed.",
      "premise_still_valid": {
        "value": true,
        "measurements": [
          {
            "claim": "Field has no string member",
            "result": "Object.keys(Field).length === 37, does not include 'string'; typeof Field.string === 'undefined'",
            "control": "typeof Field.text === 'function' — passed"
          },
          {
            "claim": "'string' is not in the field vocabulary",
            "result": "FieldType.options.length === 49, does not include 'string'",
            "controls": "'text' present — passed; 'zzznope' absent — passed"
          },
          {
            "claim": "the spec refuses type: 'string'",
            "result": "FieldSchema.safeParse({ type: 'string', label: 'ID', name: 'id' }).success === false, issue at [type] is invalid_value",
            "control": "FieldSchema.safeParse({ type: 'text', ... }).success === true — passed"
          },
          {
            "claim": "no commit ever added a Field.string builder",
            "result": "git log -S on both '  string: (config: FieldInput' and the quoted-string-literal shape against packages/spec/src/data/field.zod.ts returns zero commits",
            "control": "the same -S syntax against '  text: (config: FieldInput' on the same file finds a real historical commit (4cb2a904) — confirms the search mechanism itself works on this file/shape"
          }
        ],
        "note": "All four reproduced fresh on this worktree's origin/main (fetched at commit a65db765, then re-verified unaffected after merging forward to 72f91652 — the merge touched no packages/spec files). No premise drift; this PR proceeds as scoped."
      },
      "gates": {
        "green": [
          "pnpm --filter @objectstack/driver-sql typecheck",
          "pnpm --filter @objectstack/driver-sql exec vitest run --maxWorkers=2 src/sql-driver-11565-row-byte-budget.test.ts src/sql-driver-keyed-text-mysql.test.ts (6 passed, 2 skipped/live-MySQL-gated)",
          "node scripts/check-nul-bytes.mjs (repo scan, includes both changed files)",
          "pnpm check:changeset-gate-self-tests",
          "pnpm check:cross-package-test-inputs",
          "pnpm check:driver-conformance",
          "pnpm check:objectui-changeset",
          "pnpm check:page-declaration-shape",
          "pnpm check:pm-half-states (self-test form)",
          "pnpm check:published-files",
          "pnpm check:slot-lookup",
          "pnpm check:test-source-alias",
          "pnpm check:type-source-resolution",
          "node scripts/check-adr-0087-registration.mjs --self-test and bare (bare verdict: this PR adds no declared-breaking changeset)",
          "node scripts/check-changeset-no-major.mjs --self-test and bare",
          "node scripts/check-empty-changeset.mjs --self-test and bare",
          "node scripts/check-ci-filter-parity.mjs --self-test and bare",
          "node scripts/check-comment-mask-adoption.mjs --self-test and bare",
          "node scripts/check-plugin-teardown-shape.mjs --self-test and bare",
          "node scripts/docs-audit/check-affected-docs.mjs (self-test + standing repo report, no failure)",
          "node scripts/docs-audit/check-drift-comment.mjs",
          "node scripts/pm/release-rehearsal-clone.mjs --self-test",
          "pnpm --filter '@objectstack/driver-sql^...' build (dependency closure — ran before and after merging origin/main forward)"
        ],
        "red": [],
        "not_measured": [
          "node scripts/pm/check-half-states.mjs (the direct, non-self-test form half-state-patrol.yml runs) — reads a live external project board unrelated to this source diff; only its --self-test ran (green, via check:pm-half-states)",
          "the 2 live-MySQL-gated cases inside sql-driver-keyed-text-mysql.test.ts — skipped, OS_TEST_MYSQL_URL not set",
          "full @objectstack/driver-sql test suite (148 files) — deliberately not run; the diff is comment-only (confirmed by git diff --stat), and the two pins listed above directly and behaviorally cover the exact switch arm the corrected comments describe",
          "repo-wide pnpm lint — not run locally; CI's job per this repo's local-verification-scope convention"
        ]
      },
      "deviations": [
        "Merged origin/main (72f91652, 4 commits ahead of the fork point) into the branch before opening the PR, solely to clear a STALE TREE warning from scripts/pm/dispatch-gates.mjs (one file it derives from, scripts/engine-double-contract.pinned.json, had moved upstream). Clean merge, no conflicts; none of the incoming commits touch packages/drivers/driver-sql or packages/spec. Rebuilt the dependency closure and re-ran typecheck plus both behavioral pins after merging — all green on the post-merge tree, which is also the PR's final head.",
        "Amended the branch's sole commit to drop a 'Fixes #12593' trailer from the commit message body, keeping the card relationship only in the PR body's first line — per AGENTS.md's rule that a squash-eligible branch states card relationships once, in the PR body, never as a per-commit trailer.",
        "Did not run the full driver-sql package test suite (148 files); ran only the two tests that behaviorally pin the exact switch arm the corrected comments describe, plus typecheck, given the diff is provably comment-only. Declaring this narrowing explicitly rather than leaving it implicit."
      ],
      "open_questions": [],
      "out_of_scope_findings": [],
      "summary": "Corrected the two comments in SqlDriver.createColumn's keyed/bounded text-family branch (signature/qrcode/richtext/code) that asserted a Field.string builder exists. It does not and never has, per the four re-verified measurements below. The corrected prose states only individually-checkable facts: knex's bare table.string(name) with no length argument is varchar(255) as a fact about knex itself, not about any Field.* builder; this branch never calls it bare, it calls table.string(name, keyable) with the field's own maxLength; and the nearest authorable spelling reaching this exact shape is Field.text({ maxLength: n }) on a keyed column. Wrote the changeset per triage's corrected framing — the false prose steered the #12131 analysis toward a sweep that was never executed (caught in review, PR #12686 went the opposite direction) — not as 'a bad sweep shipped'.",
      "tests": "pnpm --filter @objectstack/driver-sql typecheck: green. pnpm --filter @objectstack/driver-sql exec vitest run --maxWorkers=2 src/sql-driver-11565-row-byte-budget.test.ts src/sql-driver-keyed-text-mysql.test.ts: 6 passed, 2 skipped (live MySQL gated) both before and after merging origin/main forward. Both pins already exercise the exact behavior the corrected comments describe (the 11565 test mirrors varcharColumnChars against real columnInfo for every declared FieldType including this switch arm's keyed-null case; the keyed-text-mysql test is the direct behavioral pin for table.string(name, keyable) on a keyed bounded text field). This is a prose-only change with no code diff, so no new test was written and none would be meaningful; full gate list (21 families dispatch-gates.mjs matched to the actual changed paths) run and green, see the gates field.",
      "mcp_calls": 5
    }

    Generated by Claude Code

  8. os-zhuang commented on Aug 27, 2026

    @os-zhuang
    Contributor

    os-dev-report

    {
      "issue": 12593,
      "status": "done",
      "branch": "claude/issue-12593-field-string-prose",
      "head": "01f35ddca9cc51e4e58a12a17e570deffd3789df",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/12740",
      "pr_state": "draft",
      "clause2": "no — this PR accepts or rejects nothing and widens no public surface. git diff --stat vs origin/main confirms two files only: one changeset, and sql-driver.ts with zero non-comment lines added or removed across both commits.",
      "premise_still_valid": {
        "value": true,
        "measurements": [
          {
            "claim": "Field has no string member",
            "result": "Object.keys(Field).length === 37, does not include 'string'; typeof Field.string === 'undefined'",
            "control": "typeof Field.text === 'function' — passed"
          },
          {
            "claim": "'string' is not in the field vocabulary",
            "result": "FieldType.options.length === 49, does not include 'string'",
            "controls": "'text' present — passed; 'zzznope' absent — passed"
          },
          {
            "claim": "the spec refuses type: 'string'",
            "result": "FieldSchema.safeParse({ type: 'string', label: 'ID', name: 'id' }).success === false, issue at [type] is invalid_value",
            "control": "FieldSchema.safeParse({ type: 'text', ... }).success === true — passed"
          },
          {
            "claim": "no commit ever added a Field.string builder",
            "result": "git log -S on both '  string: (config: FieldInput' and the quoted-string-literal shape against packages/spec/src/data/field.zod.ts returns zero commits",
            "control": "the same -S syntax against '  text: (config: FieldInput' on the same file finds a real historical commit (4cb2a904) — confirms the search mechanism itself works on this file/shape"
          }
        ],
        "note": "All four reproduced fresh on origin/main. No premise drift across either commit; the second commit only removed two now-drifting magic-number counts from the prose, per PM review — it changed no claim's truth value."
      },
      "gates": {
        "green": [
          "pnpm --filter @objectstack/driver-sql typecheck (re-run on final head 01f35ddc — VERDICT command-exit 0)",
          "pnpm --filter @objectstack/driver-sql exec vitest run --maxWorkers=2 src/sql-driver-11565-row-byte-budget.test.ts src/sql-driver-keyed-text-mysql.test.ts (re-run on final head 01f35ddc — 6 passed, 2 skipped/live-MySQL-gated, VERDICT command-exit 0)",
          "node scripts/check-nul-bytes.mjs (repo scan, includes both changed files, both commits)",
          "pnpm check:changeset-gate-self-tests",
          "pnpm check:cross-package-test-inputs",
          "pnpm check:driver-conformance",
          "pnpm check:objectui-changeset",
          "pnpm check:page-declaration-shape",
          "pnpm check:pm-half-states (self-test form)",
          "pnpm check:published-files",
          "pnpm check:slot-lookup",
          "pnpm check:test-source-alias",
          "pnpm check:type-source-resolution",
          "node scripts/check-adr-0087-registration.mjs --self-test and bare (bare verdict: this PR adds no declared-breaking changeset)",
          "node scripts/check-changeset-no-major.mjs --self-test and bare",
          "node scripts/check-empty-changeset.mjs --self-test and bare",
          "node scripts/check-ci-filter-parity.mjs --self-test and bare",
          "node scripts/check-comment-mask-adoption.mjs --self-test and bare",
          "node scripts/check-plugin-teardown-shape.mjs --self-test and bare",
          "node scripts/docs-audit/check-affected-docs.mjs (self-test + standing repo report, no failure)",
          "node scripts/docs-audit/check-drift-comment.mjs",
          "node scripts/pm/release-rehearsal-clone.mjs --self-test",
          "pnpm --filter '@objectstack/driver-sql^...' build (dependency closure)"
        ],
        "red": [],
        "not_measured": [
          "node scripts/pm/check-half-states.mjs (the direct, non-self-test form half-state-patrol.yml runs) — reads a live external project board unrelated to this source diff; only its --self-test ran (green, via check:pm-half-states)",
          "the 2 live-MySQL-gated cases inside sql-driver-keyed-text-mysql.test.ts — skipped both runs, OS_TEST_MYSQL_URL not set",
          "full @objectstack/driver-sql test suite (148 files) — deliberately not run; the diff is comment-only (confirmed by git diff --stat on both commits), and the two pins named above directly and behaviorally cover the exact switch arm the corrected comments describe",
          "repo-wide pnpm lint — not run locally; CI's job per this repo's local-verification-scope convention"
        ]
      },
      "deviations": [
        "Merged origin/main (72f91652) into the branch before opening the PR, solely to clear a STALE TREE warning from scripts/pm/dispatch-gates.mjs. Clean merge, no conflicts, touched neither driver-sql nor packages/spec. Rebuilt the dependency closure and re-ran typecheck plus both pins after — all green.",
        "Amended the branch's first commit to drop a 'Fixes #12593' trailer from the commit message body before the initial push, keeping the card relationship only in the PR body's first line — per AGENTS.md's squash-safe card-relationship rule. No amend or force-push after that first push; the PM-requested follow-up landed as a second, ordinary commit (01f35ddc).",
        "Did not run the full driver-sql package test suite (148 files); ran only the two tests that behaviorally pin the exact switch arm, plus typecheck, given the diff is provably comment-only on both commits.",
        "PM review on the open PR (comment 5441434683) asked to drop two drifting magic-number counts ('Field has 37 keys', 'FieldType.options (49 entries)') from the new comment, keeping the stable predicates. Pushed as commit 01f35ddc; verified 0 hits for either count on the pushed tree; re-ran typecheck and both pins on the new head, both green. Total time queued on the shared verify-lock across this task's 7 invocations: 21+0+297+186+370+13+307 = 1194s (about 19m54s) — all waits, zero failures; every command that eventually ran came back green."
      ],
      "open_questions": [],
      "out_of_scope_findings": [],
      "summary": "Corrected the two comments in SqlDriver.createColumn's keyed/bounded text-family branch (signature/qrcode/richtext/code) that asserted a Field.string builder exists. It does not and never has, per the four re-verified measurements below. The corrected prose states only individually-checkable facts: knex's bare table.string(name) with no length argument is varchar(255) as a fact about knex itself, not about any Field.* builder; this branch never calls it bare, it calls table.string(name, keyable) with the field's own maxLength; and the nearest authorable spelling reaching this exact shape is Field.text({ maxLength: n }) on a keyed column. Wrote the changeset per triage's corrected framing — the false prose steered the #12131 analysis toward a sweep that was never executed (caught in review, PR #12686 went the opposite direction) — not as 'a bad sweep shipped'. A follow-up PM review round asked to drop two drifting magic-number counts from the new prose (keeping the stable predicates); pushed as a second commit and re-verified green.",
      "tests": "pnpm --filter @objectstack/driver-sql typecheck: green on final head 01f35ddc. pnpm --filter @objectstack/driver-sql exec vitest run --maxWorkers=2 src/sql-driver-11565-row-byte-budget.test.ts src/sql-driver-keyed-text-mysql.test.ts: 6 passed, 2 skipped (live MySQL gated), green on final head 01f35ddc. Both pins already exercise the exact behavior the corrected comments describe (the 11565 test mirrors varcharColumnChars against real columnInfo for every declared FieldType including this switch arm's keyed-null case; the keyed-text-mysql test is the direct behavioral pin for table.string(name, keyable) on a keyed bounded text field). Prose-only change, no code diff on either commit, so no new test was written. Full gate list (21 families dispatch-gates.mjs matched to the actual changed paths) run and green; see the gates field for the complete roster.",
      "mcp_calls": 8
    }

    Generated by Claude Code

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

Metadata

Metadata

Assignees

Labels

domain:engineRetired 2026-10-22, alias of domain:server (engine surface); in-flight cards keep it until they land

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions