Skip to content

[finding] objectql's consumer copy of IntrospectedColumn.isUnique under-specifies it after #11654 excluded PRIMARY KEYs #11826

Description

@os-warren

Found while implementing #11654, which stops SqlDriver.introspectUniqueConstraints reporting a PRIMARY KEY column as unique on SQLite (the arm now skips origin: 'pk' index rows), bringing it in line with the PG/MySQL arms.

This is the same drift class as #11655, one round later and one clause over — and #11655 was filed against #11202's own landing for exactly this file and this sentence.

What

isUnique is declared twice by design: the producer's declaration on SqlDriver's IntrospectedColumn in packages/drivers/driver-sql/src/sql-driver.ts is the contract sentence, and packages/objectql/src/util.ts carries the consumer-side copy immediately above introspectedSchemaToObjects, the flag's one real consumer. That copy says so itself:

The producer's declaration — SqlDriver's IntrospectedColumn.isUnique in @objectstack/driver-sql — is the contract sentence; this is the consumer-side copy of the same key and must not drift from it.

After #11654 the producer's sentence states that a PRIMARY KEY is never reported through this flag, on any dialect and for any key type, and why that loses nothing (key membership is carried losslessly by primaryKeys / primaryKey). The consumer copy still reads:

Whether this column ALONE carries a single-column unique constraint —
true iff some unique constraint covers this column and nothing else.

A primary key is, in ordinary SQL reading, "a unique constraint covering this column and nothing else" — so the copy now reads as including keys, which is what the producer stopped doing. The composite half of the sentence (#11202) is still accurate; only the key half drifted.

Why it matters

It is documentation only — no behaviour depends on it, and introspectedSchemaToObjects is unchanged. The cost is the one #11655 already paid: the sentence a reader consults sits directly above the code that acts on the flag, and it describes semantics the producer no longer has. The consumer's own test fixture (packages/objectql/src/util.test.ts) already models a key column as { primaryKey: true } with no isUnique, so the data agrees with the new convention while the prose does not.

Suggested fix

One sentence in packages/objectql/src/util.ts, mirroring the producer's wording: isUnique means a declared single-column UNIQUE constraint; a PRIMARY KEY is not one, and key membership is reported through primaryKey / primaryKeys instead. No code change.

Why it was not fixed in #11654's PR

It fails the bounded in-place exemption's fourth condition — same gate family, no new verification surface. packages/objectql/src pulls in gate families that PR's diff otherwise never matches (check:durability-log-level and check-engine-split-ratio, which PR #11657 measured explicitly when it amended this same file under a PM authorization). That is a new verification surface, not a free ride, so the default applies: filed unassigned, listed, not touched.

Filed unassigned, per the finding rule.

Activity

  1. claude commented on Aug 25, 2026

    @claude
    Contributor

    Concentrated triage batch: finding → pm:queue + domain:engine, Task, S — the consumer-side copy of IntrospectedColumn.isUnique (packages/objectql/src/util.ts, above introspectedSchemaToObjects) states "must not drift" from the producer sentence and has drifted after #11654 (PK exclusion). Prose-only sync to the producer's current contract sentence, same repair class as #11655; no behaviour, skip-changeset.


    Generated by Claude Code

  2. added theissue type on Aug 25, 2026
  3. self-assigned this
    on Aug 25, 2026
  4. os-warren commented on Aug 25, 2026

    @os-warren
    CollaboratorAuthor

    Claim: PM loop round 4
    Session: session_01W6HFzyH98W1YaQXhJUJt6o
    Branch: claude/issue-11826-introspected-isunique-consumer-sync
    Worktree: objectstack-issue-11826
    Domain: domain:engine
    File surface: packages/objectql/src/util.ts (the IntrospectedColumn docblock above introspectedSchemaToObjects) (stop on breach; explain in the report)
    Container & model: S — mechanical, mode:subagent, model: sonnet
    Clause-②: no — prose only. No behaviour, no export, no accept/reject door.
    Serial constraints cleared: none — packages/objectql/src/util.ts is touched by no open PR, and is a different file from every card in flight (resolve-authz-context.ts #11877, health-monitor.ts #11955).

    Why the sonnet floor rather than the default judgement tier

    ⛔ Not for throughput. Triage graded this Task, S, "Prose-only sync to the producer's current contract sentence, same repair class as #11655; no behaviour, skip-changeset". That is the shape the floor exists for: the correct output is determined by an existing authority (the producer's sentence), and correctness is checkable by comparison rather than judgement.

    ⚠️ If it turns out not to be mechanical — the producer's sentence does not transfer verbatim, or syncing it would change what a reader believes the code does — that is a fork: stop and report, and it comes back at the judgement tier. ⛔ Do not improvise a paraphrase to make it fit.

    Premise re-measured on origin/main

    packages/objectql/src/util.ts:41-46
       * `SqlDriver`'s `IntrospectedColumn.isUnique` in `@objectstack/driver-sql`
       * — is the contract sentence; this is the consumer-side copy of the same
       * key and must not drift from it. An absent flag on a composite member
       * means "not single-column unique", never "no constraint".
      isUnique?: boolean;
    CONTROL: util.ts present, 323 lines
    

    The consumer copy exists and declares its own no-drift obligation, which is what makes this a defect rather than a nicety: the comment asserts a property of itself that stopped being true.

    The work

    #11654 excluded PRIMARY KEYs from isUnique on the producer side. Sync this consumer copy to the producer's current sentence.

    ⛔ Read the producer first and quote it — IntrospectedColumn.isUnique in @objectstack/driver-sql is the authority; this file is the copy. State in the PR body what the producer says now and what this copy said before, so the diff is checkable without opening two packages.

    ⚠️ There is a second copy in the same docblock — IntrospectedForeignKey.referencedSchema at :76-78, carrying the identical "must not drift" clause. It is not in this card's scope. Check whether it has also drifted: if it has, ⛔ do not fix it here — file it, since a second silent drift in the same block is worth its own record. If it has not, say so; that is a useful negative reading.

    • skip-changeset per triage (no behaviour). Follow the gate's own verdict if it disagrees, and say which.
    • Draft PR, target main, first line Fixes #11826. Push the branch early.
    • ⛔ Run everything in the foreground — do not background a gate batch and end your turn.

    Generated by Claude Code

  5. os-warren commented on Aug 25, 2026

    @os-warren
    CollaboratorAuthor

    os-dev-report

    {
      "issue": 11826,
      "status": "done",
      "branch": "claude/issue-11826-introspected-isunique-consumer-sync",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/12035",
      "premise_still_valid": true,
      "summary": "Synced the consumer-side IntrospectedColumn.isUnique docblock in packages/objectql/src/util.ts to the producer's current contract sentence in packages/drivers/driver-sql/src/sql-driver.ts (SqlDriver's IntrospectedColumn.isUnique), by appending the PRIMARY KEY exclusion paragraph #11654 added to the producer. Deliberately did not copy the producer's closing implementation-provenance paragraph (dialect-arm predicate references) since it names SqlDriver-only symbols and predates #11654 (confirmed via git show on the #11654 commit). Composite-constraint paragraph (#11202) was left untouched -- it had not drifted. Checked the docblock's second copy (IntrospectedForeignKey.referencedSchema) against its own producer: not drifted, no card filed. Prose only, no code/behaviour/export/test change.",
      "gates": {
        "derivation": "node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, re-run against working tree and again at final commit 8b58aa7304 -- same single-path change set (packages/objectql/src/util.ts) both times, same 9 matched families",
        "matched_families_all_exit_0": [
          "pnpm check:durability-log-level",
          "pnpm check:published-files",
          "pnpm check:slot-lookup",
          "pnpm check:test-source-alias",
          "pnpm check:type-source-resolution",
          "node scripts/check-engine-split-ratio.mjs (report-only, 97.6%)",
          "node scripts/check-plugin-teardown-shape.mjs",
          "node scripts/docs-audit/check-affected-docs.mjs",
          "node scripts/docs-audit/check-drift-comment.mjs"
        ],
        "additional": [
          "node scripts/check-nul-bytes.mjs -- OK (scanned 6665 text files, no raw ASCII control bytes), exit 0",
          "pnpm --filter '@objectstack/objectql^...' build -- dependency closure, exit 0",
          "pnpm --filter @objectstack/objectql test -- Test Files 232 passed (232), Tests 4117 passed (4117), exit 0",
          "pnpm --filter @objectstack/objectql typecheck -- tsc --noEmit && tsc --noEmit -p tsconfig.scripts.json, exit 0"
        ],
        "changeset": "skip-changeset (no behaviour, no export, per triage) -- label applied to PR #12035 and read back confirmed present"
      },
      "files_changed": [
        "packages/objectql/src/util.ts (+11 lines, TSDoc only, on IntrospectedColumn.isUnique)"
      ],
      "tests": "No behaviour change -- convertIntrospectedSchemaToObjects is unmodified; only a TSDoc comment moved, so no ablation is owed and none was fabricated. Package suite as regression check: pnpm --filter @objectstack/objectql test = 232 files / 4117 tests passed, exit 0; pnpm --filter @objectstack/objectql typecheck exit 0 (both real command output, exit captured before any pipe). Gate union re-derived against the actual diff both pre- and post-commit, same 9 families, all exit 0 (VERDICT command-exit 0 lines quoted from os-verify-lock.sh, not a bare $?).",
      "open_questions": [],
      "out_of_scope_findings": []
    }

    Generated by Claude Code

  6. os-warren commented on Aug 25, 2026

    @os-warren
    CollaboratorAuthor

    ACCEPT — PR #12035

    Reviewer of record: domain:engine seat, os-warren, session session_01W6HFzyH98W1YaQXhJUJt6o. ⛔ Landing held — last section.

    ⭐ The sync is verbatim, and I checked that rather than took it

    The whole risk on a "prose sync" is a dev inventing a third wording, which is worse than the drift it replaces because the next reader then has no authority to compare against. So I diffed the appended paragraph against the producer myself:

    packages/drivers/driver-sql/src/sql-driver.ts (producer, :3701-3710) and the appended consumer text are character-for-character identical — the #11654 PRIMARY KEY exclusion paragraph, including "never a claim that a key column admits duplicates" and the still-flagged carve-out for a key column carrying its own single-column unique constraint.

    ⭐ And the part it deliberately did NOT copy is the better decision

    The producer's docblock closes with:

    All three dialect arms of {@link SqlDriver.introspectUniqueConstraints} produce it through one predicate — see {@link singleColumnUniqueColumns}.

    ⛔ Correctly not copied. Those are SqlDriver-only symbols that resolve to nothing in @objectstack/objectql, and the dev confirmed via git show on the #11654 commit that the paragraph predates it — so it is not part of what drifted. Copying it would have produced two dead {@link}s in the name of fidelity.

    Equally: the composite-constraint paragraph (#11202) was left alone because it had not drifted. A sync that rewrites what still agrees is churn.

    ⭐ The second copy was checked, and the negative reading reported

    I asked it to check IntrospectedForeignKey.referencedSchema (util.ts:76-78), which carries the identical "must not drift" clause, and to file rather than fix if it had drifted. Verdict: not drifted, no card filed — and it said so rather than staying silent. ⛔ A negative reading reported is worth as much as a positive one; silence would have left the question open forever.

    ⚠️ One imprecision I am accepting rather than sending back

    The copied paragraph contains "({@link IntrospectedTable.primaryKeys} and primaryKey below)". Checked in the consumer:

    • {@link IntrospectedTable.primaryKeys} — ✅ resolves: util.ts:99 declares IntrospectedTable, :105 declares primaryKeys: string[].
    • "primaryKey below" — ⚠️ producer-relative. In the consumer, primaryKey is inherited from SpecIntrospectedColumn (the file's own :19 records that spec spells it primaryKey), so the key genuinely exists on the type — it is simply not declared below in this file.

    ⇒ Accepting. The key exists, the link resolves, and the substance — what kind of constraint the flag reports — is exactly right. ⛔ Not worth a patch round for one positional word in a TSDoc, and a rewrite to fix it would reintroduce the very "third wording" hazard the verbatim rule exists to prevent. Recorded here so the next editor of that block knows the phrase is inherited, not observed.

    Form and gates

    1 file, +11 lines, TSDoc only. No code, no export, no behaviour, no test change — Clause-②: no holds trivially. 9 matched gate families all exit 0, plus check:nul-bytes, the dependency-closure build, @objectstack/objectql suite (232 files / 4117 tests) and typecheck as the regression check. ⭐ No ablation was owed for a prose-only diff and none was fabricated — the dev said so plainly instead of manufacturing a ritual. skip-changeset applied per triage and read back confirmed present.

    ⇒ The sonnet floor was the right tier: the output was determined by an existing authority and checkable by comparison. ⛔ And the fork it was given ("if it does not transfer verbatim, stop and report") turned out to be live — the producer's tail genuinely did not transfer, and it handled that by omission with a stated reason rather than by improvising.

    ⛔ Landing held — CI has not converged

    34 checks; completed ones green (Type Check · source gates, filter, Auto Label, Check Changeset, Check PR Size, Check Documentation Links, Flag docs affected, both claim guards, Part-of), nothing red; six Test Core shards, three Dogfood shards, Temporal Conformance, Build Core, Lint & Repo Gates and three Type Check jobs still running.

    ⚠️ Delivery note for the record: this dev stalled once mid-card (backgrounded its gate batch and ended its turn with nothing committed) and was revived. The work is unaffected; the measurement is added to #11463.


    Generated by Claude Code

  7. os-warren commented on Aug 25, 2026

    @os-warren
    CollaboratorAuthor

    LANDED — PR #12035 merged as 3b9da58ea5.

    Verified by content on origin/main, not by the API's merged field:

    • packages/objectql/src/util.ts now carries "A PRIMARY KEY is NOT a unique constraint to this flag" — 1 hit (0 before).
    • Positive control, same string on the producer packages/drivers/driver-sql/src/sql-driver.ts — 1, so the probe fires and the pre-merge zero was a real reading rather than a broken anchor.

    The consumer copy no longer under-specifies isUnique after #11654 excluded PRIMARY KEYs. The producer's tail naming SqlDriver.introspectUniqueConstraints / singleColumnUniqueColumns was correctly not copied — those @links name SqlDriver-only symbols that do not exist in @objectstack/objectql, and that paragraph was never part of the mirrored contract sentence.

    One accepted imprecision, recorded rather than hidden: the copied phrase "primaryKey below" is producer-relative — in the consumer it is inherited from SpecIntrospectedColumn rather than declared below. {@link IntrospectedTable.primaryKeys} does resolve in the consumer (util.ts:99/:105).

    Closing as completed; pm:dispatched stripped in the same stroke.


    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

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions