Skip to content

External object-draft renders a field key that is not authorable: fields.<f>.primaryKey fails tsc against ServiceObject and ObjectSchema.safeParse #11000

Description

@os-warren

Found while working #10676. This is the reason that card could not be discharged as written, so it is filed as its own defect — it exists today, independently of #10676, and is reachable without any change to the tree.

What

ExternalDatasourceService.generateObjectDraft emits, and renderObjectSource renders, a field-level primaryKey: true:

fields[fieldName] = isPk ? { type: fieldType, primaryKey: true } : { type: fieldType };
const pk = f.primaryKey ? ', primaryKey: true' : '';
return `    ${fieldName}: { type: '${f.type}'${pk} },${comment}`;

primaryKey is not a key of the spec field schema. The rendered *.object.ts is annotated ServiceObject, so the generated source does not compile, and the definition does not parse.

Measured at 368e7a06f

tsc --noEmit over the rendered draft, with a no-primaryKey copy of the same file in the same program as a positive control:

with-pk.ts(8,25): error TS2353: Object literal may only specify known properties,
  and 'primaryKey' does not exist in type '{ type: "number" | ... ; externalId?: boolean | undefined; }'.

without-pk.ts produced no diagnostic — so the program is live and the clean result is meaningful.

ObjectSchema.safeParse on the same two definitions:

definition success issue
with fields.id.primaryKey: true false unrecognized_keys, path: ["fields","id"], keys ["primaryKey"]
identical, key removed true —

Reachable today

This is not hypothetical and does not depend on #10676. generateObjectDraft already emits the key whenever the caller passes the primaryKey option — the path POST /object-draft { "primaryKey": [...] } and os datasource introspect --primary-key take — and the existing suite pins that output (external-datasource-service.test.ts asserts both { type: 'text', primaryKey: true } in definition.fields and "order_id: { type: 'text', primaryKey: true }" in source). So the generator has a pinned path that produces a draft the platform's own validator and compiler both refuse.

It is usually invisible only because the introspected default never fires — which is precisely the bug #10676 reports.

The open question

There is currently no authorable place for a federated object's remote primary key:

  • not on the field — primaryKey is unrecognized, as measured above;
  • not on the binding — ObjectExternalBindingSchema is a strictObject whose keys are remoteName, remoteSchema, writable, columnMap, introspectedAt, ignoreColumns. No key holds a primary key.

FieldSchema does declare externalId: z.boolean().default(false) — "Is external ID for upsert operations" — which is semantically adjacent (the card #10676 describes the lost value as "the addressing/upsert key"). Routing the introspected primary key there would need no spec change. But it is a different concept, it maps badly onto a composite key, and a scan of non-test, non-dist TypeScript under packages/ turned up no site that reads a field's externalId property at all — the externalId hits are SCIM, the batch API, and the seed/mapping upsertKey aliases, which take a field name at the dataset level, not this boolean. So it may itself be declared-but-unenforced (ADR-0049 territory) and is not obviously a safe destination.

Three candidate repairs, none of which a dev should pick unilaterally:

  • A. Emit fields.<f>.primaryKey anyway — no spec change, but ships codegen output that fails tsc and ObjectSchema by default. Adds a third reason on top of [finding] External object-draft output fails os build as generated — object name lacks the ${namespace}_ prefix and no sharingModel is emitted #10712's two for why the draft does not build.
  • B. Emit fields.<f>.externalId — no spec change, draft stays compilable, but overloads a key with different stated semantics that may itself have no enforcing consumer.
  • C. Add an authorable spelling for a remote key (e.g. external.primaryKey: string[] on ObjectExternalBindingSchema) — contract-first, handles composite keys naturally, declared = enforced; but it is a packages/spec change and adds surface, which wants a maintainer ruling on whether anything consumes a federated primary key at runtime today.

Related: #10712 (the same generated draft also fails os build on the missing ${namespace}_ prefix and absent sharingModel), #10676 (the seam that keeps the key from ever reaching the draft).

Filed unassigned as a finding awaiting first-touch grading. Discovered from #10676.

Activity

  1. added theissue type on Aug 22, 2026
  2. huangyiirene commented on Aug 22, 2026

    @huangyiirene
    Collaborator

    Triage: the decision point sits on the contract (packages/spec — is there, and should there be, an authorable spelling for a federated object's remote primary key) ⇒ domain:spec, filed into the decision inbox (needs-user-decision) — option C adds public spec surface, which is the manual floor. Adding option D below, which the card's own measurements imply but do not name.

    [facets-block]

    • 实际业务需求:联邦对象(外部数据源)确实需要知道远端主键才能寻址/合并写入;但本席读卡内测量:今天没有任何运行时消费者读字段级主键(externalId 的命中全是 SCIM/批量 API/数据集级 upsertKey,不是这个布尔)。真实缺口是「草稿编译不过」,不是「运行时缺主键」。
    • 项目长远合理性:C(external.primaryKey: string[] 落在 binding 上)是契约先行、天然支持复合键的正解;B 把不同语义压进一个本身疑似 declared≠unenforced 的键,是把两笔债合成一笔。
    • 防 AI 写代码犯错:A(继续渲染非法键)让平台的代码生成器产出平台自己的编译器和校验器都拒绝的代码——AI 元数据开发的最坏示范,排除;D(不渲染该键,主键信息写进生成源码的注释)让草稿默认可编译、零契约扩面。
    • 创业阶段不扩散需求:C 在「运行时零消费者」的当下加声明面,违反 implementation-first;D 零扩面,等真实消费者(联邦 upsert 落地)出现时再走 C,届时 C 的形状也更有依据。

    一句话问题:引入远端主键的可授权拼写(spec 扩面),还是先让生成的草稿停止携带非法键(零扩面)?

    选项 客户可感知代价
    D codegen 不再渲染 primaryKey 键,信息降级为生成源码注释(现有 pin 测试同步改) 草稿可编译可解析;主键信息对运行时不可见(与今天的实际一致——今天也没人消费)
    C spec 在 ObjectExternalBindingSchema 加 external.primaryKey: string[] 新公开面 + schema-parity/文档/表单跟进;换来 declared=enforced 的正路,复合键有家
    B externalId 复用 语义错位,且该键自身疑似无强制消费者
    A 保持现状 生成即坏,排除

    推荐 D 先行止血,C 留给「联邦 upsert 成为近期路线」时(置信度中):四轴里创业轴与防错轴指 D,长远轴指 C——按「先修复、不扩散」排序。本分析看不见什么:cloud 侧是否有联邦主键的运行时消费(不在本会话授权面);以及 #10676(同缝的 isPrimary/primaryKey seam,已在决策箱)若裁出别的走向,本卡应随之复议——两卡建议同场裁。裁决格式:回「D」/「C」/「DC」(D 现在做、C 立后续卡)。

    裁后执行段:D ⇒ 晋级 pm:queue 派 cli/engine 席(落点 service-datasource codegen + 既有 pin 测试),派发令带「#10712 的两个编译错误同文件面,fold-or-serial 必答」;C ⇒ 转 domain:spec 席走条款②档位。


    Generated by Claude Code

  3. huangyiirene commented on Aug 22, 2026

    @huangyiirene
    Collaborator

    Maintainer ruling recorded — D: stop emitting the invalid key; the spec spelling waits for a real consumer

    Provenance: maintainer, 2026-08-22, live session (「同意所有」, item 8 = D).

    State: needs-user-decision → pm:queue (services lane — the fix lands in service-datasource's codegen), Bug. Scope: generateObjectDraft/renderObjectSource stop emitting fields.<f>.primaryKey; the introspected key survives as a comment in the generated source (information preserved for the reader, zero contract face); the existing pin tests that assert the invalid emission are updated as part of the fix (their flip cites this ruling). Option C (external.primaryKey: string[] on the binding schema) is deferred, not rejected — it returns as its own spec-lane card when federated upsert has a live runtime consumer. Coordination: same generator as #10712 (queued) and the #10676 driver-side ruling — the services seat answers fold-or-serial across the three at claim.


    Generated by Claude Code

  4. self-assigned this
    on Aug 22, 2026
  5. os-warren commented on Aug 22, 2026

    @os-warren
    CollaboratorAuthor

    Claim: dispatched.

    • session: user-29 [974948]
    • branch: claude/issue-11000-draft-drops-unauthorable-primarykey
    • worktree: ../objectstack-11000
    • Clause-②: no — the ruling removes an emission that the platform's own compiler and validator already refuse. Nothing widens; a draft that cannot parse today starts parsing.

    The ruling is settled — D

    Maintainer, 2026-08-22 live session (「同意所有」, item 8):

    generateObjectDraft/renderObjectSource stop emitting fields.<f>.primaryKey; the introspected key survives as a comment in the generated source (information preserved for the reader, zero contract face); the existing pin tests that assert the invalid emission are updated as part of the fix (their flip cites this ruling).

    ⛔ Option C (external.primaryKey: string[] on ObjectExternalBindingSchema) is deferred, not rejected — it returns as its own spec-lane card when federated upsert has a live runtime consumer. It is not yours, and this lane has zero packages/spec ownership.

    Fold-or-serial — answered at claim, as the ruling requires

    The ruling says "same generator as #10712 (queued) and the #10676 driver-side ruling — the services seat answers fold-or-serial across the three at claim." Answered from facts, not preference:

    against answer why
    #10712 ⛔ SERIAL, and the prerequisite is already satisfied PR #11059 touched the same two functions and merged as 3e26359a7. Folding was never available — it was in the merge queue when this ruling landed.
    #10676 naturally separate it moved to domain:engine and lands in driver-sql — different package, different lane. No coordination needed beyond not inheriting its claims.

    ⚠️ Your branch MUST be cut from a main that contains 3e26359a7. Verify it (git log --oneline | grep 11059, or git merge-base --is-ancestor 3e26359a7 HEAD). If it is not there, stop and re-fetch — a base without it means you are editing a generator that does not yet have the namespace/sharingModel fix, and your measurements will not mean what you think.

    What #11059 already measured — inherit it, and it defines your acceptance

    #11059's seat drove the real os build pipeline (three stages: defineStack() namespace validation → authoringRulesFor('build')'s 41 rules → tsc --noEmit over draft.source) and reported both paths separately:

    path after #11059
    opts.primaryKey unset stage 1/2/3 all PASS — the draft builds end-to-end
    opts.primaryKey set still fails: Unrecognized key(s) on this field: primaryKey; stage 2 never reached; TS2353

    That second row is exactly what this card removes. So your acceptance is: both paths build. Drive the same three stages and report both, the way #11059 did.

    ⭐ And inherit this observation, because it is the kind that misleads: before #11059, the primaryKey-set path failed on unrecognized_keys before the namespace check ran — so on that path the namespace defect was masked, not absent. Error ordering hides defects here. Do not read one path's verdict as covering the other.

    The pin flip — do it deliberately, and cite the ruling

    The existing suite pins the invalid emission. It will go red, and that is intended:

    The comment that replaces it

    The ruling says the introspected key survives as a comment in the generated source. Two things to get right:

    What must be pinned — both directions

    Standing proof standard

    Ablation with the signature predicted first; restore proved byte-identical by git hash-object, and re-run the restore leg to a real verdict. Prove src/ vs dist/ resolution argued from the files — ⚠️ #11059's seat established the shape here: the unit pins import '../external-datasource-service.js' (relative, cannot route through exports), while the os build harness drives the built dist/ and therefore needs a build before every measurement, with the marker proved present in the artifact. Re-verify; do not inherit. Zero-hit counter-check with the positive control run FIRST. Gate union derived on the final commit, clean tree, dispatch-gates.mjs with no path arguments, exit codes captured before any pipe. Class #10309 — it was short on five services PRs today and held on two; re-derive on the final committed diff, which is what made the difference, and say whether it named your gates. Changeset required. Draft PR only.

    ⚠️ A gate's refusal (PREREQUISITE NOT MET / --re-measure cannot run / Nothing was checked) is NOT MEASURED, never a pass — build the closure and re-run. ⛔ If a ratchet reddens, fix the code, never the baseline — a seat did exactly that on check:slot-lookup today rather than adding a line to the ledger. Never git stash (shared stack). Never report a grep hit count as a fact count.

    ⛔ Do not touch content/docs/releases/** or packages/spec.

    Refs

    The triage four-facet analysis and the maintainer ruling are both on this card · #10712 / PR #11059 (merged 3e26359a7 — read its body; it defines your acceptance) · #10676 (engine lane, driver-side) · #10997 (composite-key truncation, unfixed, affects how the comment should be worded) · #11046 (the stale-comment-beside-a-flipped-assertion precedent)


    Generated by Claude Code

  6. added a commit that references this issue on Aug 22, 2026
    eb16902
  7. os-warren commented on Aug 22, 2026

    @os-warren
    CollaboratorAuthor

    os-dev-report

    {
      "issue": 11000,
      "status": "done",
      "branch": "claude/issue-11000-draft-drops-unauthorable-primarykey",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/11073",
      "premise_still_valid": true,
      "base_verification": "origin/main IS 3e26359a7 exactly (PR #11059). `git merge-base --is-ancestor 3e26359a7 origin/main` -> 0, re-asserted on the worktree HEAD after branching. Final commit eb16902cc; merge base with origin/main confirmed 3e26359a7 by dispatch-gates.",
      "summary": "Ruling D implemented: generateObjectDraft no longer writes fields.<f>.primaryKey into the definition and renderObjectSource no longer renders `, primaryKey: true` onto the field line; the introspected key is preserved as a comment above the `fields` block. The existing pins that asserted the invalid emission were FLIPPED (not deleted) with their explanatory text rewritten to cite the 2026-08-22 ruling, and #11059's fixture docblock — which said #11000 was an open packages/spec question, deliberately untouched — was de-staled. A new 16-case pin file covers the four required directions. No packages/spec file touched; Option C untouched and still deferred.",
      "os_build_verdict": {
        "instrument": "three stages driven for real against the BUILT dist: defineStack() -> authoringRulesFor('build') (41 rules, count read off the live registry) -> tsc --noEmit over draft.source with an in-program positive control",
        "before_3e26359a7": {
          "primaryKey_UNSET": "stage1 PASS / stage2 PASS (41 rules, 0 gating) / stage3 PASS (0 diagnostics)",
          "primaryKey_SET": "stage1 FAIL `objects.0.fields.id: Unrecognized key(s) on this field: primaryKey` / stage2 FAIL (protocol schema; the 41 rules were NEVER REACHED) / stage3 FAIL `draft.ts(10,25): error TS2353: ... 'primaryKey' does not exist in type`. ObjectSchema.safeParse success=false, unrecognized_keys at [fields,id]."
        },
        "after_eb16902cc": {
          "primaryKey_UNSET": "stage1 PASS / stage2 PASS (41 rules, 0 gating, 0 advisory) / stage3 PASS (0 diagnostics)",
          "primaryKey_SET": "stage1 PASS / stage2 PASS (41 rules, 0 gating, 0 advisory) / stage3 PASS (0 diagnostics). ObjectSchema.safeParse success=true."
        },
        "note": "BEFORE is a live re-measurement on this branch (pre-fix file checked out from origin/main, artifact rebuilt, harness re-run), not inherited. It reproduces #11059's reported second row exactly. Error-ordering hazard CONFIRMED rather than assumed: on BEFORE/SET, stage 1 aborted on unrecognized_keys, so nothing downstream was measured on that path.",
        "acceptance": "BOTH PATHS BUILD. This is the row #11000 removes."
      },
      "comment_rendering": {
        "single_key": "`  // Remote primary key: id` followed by 6 lines saying it is a comment because ServiceObject has no authorable key for a federated remote primary key (#11000), that nothing reads it, that it names the column(s) THIS DRAFT WAS GIVEN, and that for a composite key some drivers report only the first column (#10997) so the list is a LOWER BOUND.",
        "composite_key": "`  // Remote primary key: order_id, line_no` — every member, in source order, same caveat block.",
        "no_key": "NOTHING is rendered. No banner, no empty comment. Nothing was introspected, so nothing is preserved; a 'none reported' line would be noise in every draft of every keyless table.",
        "renamed_field": "the comment names the FIELD name (post-rename), not the remote column — it sits directly above the `fields` block and must speak that block's vocabulary or it points at a line that is not there.",
        "overclaim_guard": "wording never says 'the primary key'; it says the column(s) the draft was given, plus an explicit #10997 lower-bound caveat. Both the #10997 reference and the phrase 'lower bound' are pinned assertions, not just prose.",
        "compiles_with_the_comment_in_it": "proved by tsc --noEmit over draft.source (0 diagnostics), not by inspection."
      },
      "pin_flip": {
        "external-datasource-service.test.ts": "`expect(fields.order_id).toEqual({ type: 'text', primaryKey: true })` -> `toEqual({ type: 'text' })`; `toContain(\"order_id: { type: 'text', primaryKey: true }\")` -> `toContain(\"order_id: { type: 'text' },\")` + `not.toContain('primaryKey: true')` + `toContain('// Remote primary key: order_id')`. NOT deleted — inverted, it is now the guard the key does not come back. Explanatory text REWRITTEN: cites the 2026-08-22 ruling by date and item, states what it used to assert and why it flipped, and records that `toEqual` (not `toMatchObject`) is what makes it fail on an EXTRA key.",
        "honours-options case": "added `toContain('// Remote primary key: order_id')` so the fix cannot be read as 'the option is ignored now'.",
        "external-object-draft-os-build.test.ts": "its fixture docblock claimed #11000 was 'an open contract question in packages/spec, deliberately untouched here'. Falsified by this card; rewritten to record that the ruling landed, while KEEPING the error-ordering warning. Added a `both paths build` describe block re-running stages 1/2 and still-generates on the key-set path."
      },
      "four_pinned_directions": [
        "1 ABSENT from the definition — every field record's keys are exactly ['type'] on the option path AND the introspection path; JSON of the definition contains no 'primaryKey'; importObject never writes it to the metadata store.",
        "2 PARSES — full ObjectSchema.safeParse success (value verdict, not absence-of-unknown-keys), single and composite. Positive control: the same definition with the key put back is refused AND refused for unrecognized_keys specifically.",
        "3 INFORMATION PRESERVED (load-bearing) — comment names the key, in order, under rename; carries #10997 caveat; says why; renders nothing with no key; never leaks into the definition. An implementation that just dropped opts.primaryKey passes every other direction and fails these.",
        "4 UNSET PATH UNCHANGED from #11059 — namespace prefix wh_orders, sharingModel 'private', safeParse green; #11059's own still-generates / namespace / OWD / importObject cases all re-run green (62/62 across the three files)."
      ],
      "ablation": {
        "A_prediction_written_first": "Remove the comment renderer. Direction: REDDENING (not diagnostics-increase, not inversion). Mode: `expect(draft.source).toContain(...)`. Enumerated BEFORE running: exactly 8 named cases. Absence-asserting cases (no-key, never-in-definition) and all of directions 1/2 stay GREEN.",
        "A_observation": "EXACTLY the 8 named cases, by name. `AssertionError: expected '// Generated by `os datasource intros...' to contain '// Remote primary key: order_id'`. 8 failed | 54 passed. Predicted-green set stayed green. NOTE: an earlier coarser prediction written before the suite existed said 3 — recorded and wrong; the refined enumeration was still written before the mutation.",
        "B_prediction_written_first": "Put the key back on the DEFINITION side only, renderer untouched. Predicted 8 named cases red, WITH A DISCRIMINATOR: the field-LINE case and every direction-3 comment case must stay GREEN, because the renderer no longer reads f.primaryKey.",
        "B_observation": "EXACTLY those 8; discriminator HELD. Proves the two halves of ruling D are independently pinned rather than one assertion wearing two hats.",
        "restore": "git hash-object before ablation = 942f2c6ea223e42a173b9b9b8e4375eb50f25ded; after restore of A = same; after restore of B = same. BYTE-IDENTICAL both times AND each restore leg RE-RUN to a real verdict (62/62 passed each), not trusted to the hash."
      },
      "zero_hit_counter_check": "POSITIVE CONTROL RUN FIRST: `grep -nE \"primaryKey: true|, primaryKey: true|primaryKey\\?: boolean\"` on the pre-fix file (git show origin/main:...) matched at 4 lines, exit 0 — read individually they are the 4 sites this PR removes (2 type annotations at 334/663, 1 definition write at 359, 1 render at 672). Same search on the fixed file: exit 1, no hits. The zero is measured, not assumed.",
      "src_vs_dist": {
        "instrument_1_unit_pins": "src/. Subject imported as '../external-datasource-service.js' — a RELATIVE specifier, which cannot route through the package `exports` map. Re-verified, not inherited: this package's vitest.config.ts declares exactly ONE alias, array form anchored /^@objectstack\\/core$/, which does not touch the subject. BUT the instrument half is dist: ObjectSchema comes from '@objectstack/spec/data', a bare specifier -> workspace link -> exports['./data'] -> packages/spec/dist/data/index.mjs. Closure built first and the symbol proved present in that artifact.",
        "instrument_2_os_build_harness": "dist/. Loads ExternalDatasourceService from packages/services/service-datasource/dist/index.js by absolute path, so it measures the BUILT ARTIFACT and was rebuilt before EVERY measurement. Fingerprinted each run: AFTER 126976 bytes, 'Remote primary key' present, ', primaryKey: true' absent; BEFORE 126139 bytes, inverse; RESTORE 126976 bytes matching AFTER. Instruments (spec/lint/typescript) resolved from packages/cli, the one workspace package linking both."
      },
      "instrument_defects_found_and_fixed_in_my_own_harness": [
        "An incomplete manifest ({name,version,namespace}) made stages 1 and 2 FAIL on BOTH paths — a harness fault that would have read as a code fault. Replaced with a real manifest (id/type/engines), after which the paths separated correctly.",
        "The tsc diagnostic classifier used startsWith('draft.ts'), but tsc prints paths RELATIVE TO CWD — so it counted 0 for BOTH files, which reads simultaneously as 'clean draft' and 'silent control'. The positive control was SILENT until this was fixed. Now matched on basename + (line,col) shape, with an explicit unclassified bucket; the control fires on every run."
      ],
      "tests": "All at eb16902cc (clean tree), each exit code captured before any pipe. (1) `pnpm --filter @objectstack/service-datasource test` -> `Test Files 25 passed (25) / Tests 559 passed (559)`, EXIT=0. (2) targeted verbose run of the three pin files -> `Test Files 3 passed (3) / Tests 62 passed (62)`, every case name echoed (guards the zero-match-exits-0 trap). (3) `pnpm --filter @objectstack/service-datasource typecheck` -> script name echoed as `tsc --noEmit`, EXIT=0. (4) os build harness on the built dist, both paths, BEFORE/AFTER/RESTORE — verdicts above. (5) Dependency closure built before every measurement; full workspace closure (70 tasks successful) built before the ratchet gate. Ablations state their rebuild: ablations A and B run on the UNIT instrument, which resolves the subject through a relative specifier to src/ and therefore needs no rebuild — argued from the import and from the single anchored alias in vitest.config.ts, not assumed; every dist-resolving measurement (the os build harness) WAS rebuilt, with the marker proved present or absent in the artifact on each leg.",
      "gates": "Union derived by `node scripts/pm/dispatch-gates.mjs` with NO path arguments on the FINAL commit eb16902cc, clean tree (script reported: 6 paths vs merge base 3e26359a7, committed 6, working tree 0, untracked 0). The dispatch carried no gate list, so this derivation IS the list — it named 11 path-matched families plus 5 convention-triggered ones (test-file kind). All 17 run, each verdict quoted from the gate's own printed line: check:changeset-gate-self-tests OK (212+116 assertions); check:objectui-changeset OK; check:slot-lookup 'ratchet holds — 107 unswept sites in 25 files, none new'; check:test-source-alias 'OK — 72 packages with tests scanned'; check:type-source-resolution 'OK — 77 packages with a tsconfig.json scanned'; check-adr-0087-registration 'no declared-breaking changeset (2 non-breaking seen)'; check-changeset-no-major 'This diff introduces no major bump'; check-ci-filter-parity 'OK: all 83 declared cross-package globs covered'; check-empty-changeset 'No empty-frontmatter changeset introduced (2 declaring added)'; check-plugin-teardown-shape '63 Plugin implementations ... baseline fully burned down'; check-affected-docs '339 cases pass'; check:query-options-erasure 'ratchet holds: 67 unswept sites in 17 files, none new'; check:type-check-coverage 'OK — 65/78 workspace packages type-checked'; check:type-check-debt --re-measure 'OK — 33 ledger entries re-measured in 248.1s, 1908 raw tsc errors total, none above its recorded number'; check:engine-double-contract 'OK self-test ... 6 consumer seams, 6 refusing'; check:where-matcher '277 matchers discovered, 277 conform'; check:nul-bytes self-test 75 assertions OK. NO gate returned a refusal (no PREREQUISITE NOT MET / cannot run / Nothing was checked) — the full workspace closure was built first exactly as lint.yml does, so the ratchet re-measured for real. NO baseline was edited. Class #10309: re-deriving on the final committed diff is what I did, and it named my gates — nothing was added or dropped versus an earlier derivation because the tree was already clean at that point.",
      "declared_narrowings": [
        "Local scope narrowed to the derived union rather than enumerating the whole check:* farm — CI runs the farm exactly once regardless.",
        "tsc --noEmit over draft.source lives in the PR harness, not the committed suite — following #11059's precedent, which committed no tsc case either. The committed suite pins parse + shape + comment; the compile verdict is the measurement reported here.",
        "Report delivered at draft-PR time per the 2026-08-10 maintainer ruling; CI convergence is the PM's to read. CI status at report time: in_progress / not yet read."
      ],
      "open_questions": [],
      "out_of_scope_findings": [],
      "notes_for_other_cards": [
        "#10997 (SQLite composite-PK truncation) untouched — engine lane. This card only ensures the generated comment does NOT overclaim in its presence: it says 'lower bound', never 'the primary key'. If #10997 lands, the caveat sentence in renderObjectSource is the one line worth revisiting, and it is pinned by name.",
        "#10998 (introspectSchema omits dialect/introspectedAt) untouched — engine lane, not reached by this diff.",
        "#10676 remains open and is NOT closed by this PR: generateObjectDraft still reads `col.primaryKey` directly (line ~358) rather than through this same file's `primaryKeyReader` table/column union used by refreshCatalog. That is #10676's seam and was deliberately not widened into. Consequence worth recording for whoever takes it: because the draft path reads only `col.primaryKey`, a real SqlDriver (which sets `isPrimary` + table-level `primaryKeys`) still produces NO key on the introspection-supplied path — so today the new comment fires in production only via `--primary-key`/opts. The comment machinery is already correct for both; #10676 is what makes the introspected path actually deliver a key to it.",
        "Option C (external.primaryKey: string[] on ObjectExternalBindingSchema) remains deferred, not rejected, and nothing in this PR forecloses it: the key is collected in a dedicated `primaryKeyFields` array inside generateObjectDraft, so routing it to a real binding key later is a one-line change at the definition site rather than a re-derivation.",
        "Minor behaviour, noted not filed: a caller passing primaryKey:['x'] AND excludeColumns:['x'] gets no mention of x at all — the column is not in the draft to name. Contradictory caller; left as-is rather than widened into."
      ]
    }

    Generated by Claude Code

  8. os-warren commented on Aug 22, 2026

    @os-warren
    CollaboratorAuthor

    PM review of PR #11073. Flipping ready and arming auto-merge (Clause-② no — the ruling removes an emission the platform's own compiler and validator already refuse).

    Acceptance met: both paths build

    path before 3e26359a7 after eb16902cc
    opts.primaryKey unset 1/2/3 PASS 1/2/3 PASS
    opts.primaryKey set stage 1 FAIL Unrecognized key(s) … primaryKey · stage 2 never reached · stage 3 TS2353 1/2/3 PASS, safeParse success=true

    And the BEFORE column is a live re-measurement on this branch — pre-fix file checked out from origin/main, artifact rebuilt, harness re-run — not inherited from #11059. It reproduces #11059's reported second row exactly.

    ⭐ The error-ordering hazard I flagged at dispatch was confirmed rather than assumed: on BEFORE/SET, stage 1 aborted on unrecognized_keys, so nothing downstream was measured on that path. That is why one path's verdict never covers the other here.

    Verified by content, with the positive control first: the pre-fix file carries exactly four sites (:334, :359 definition write, :663, :672 render) — matching the seat's enumeration — and the same expression on the fixed branch returns zero. The comment renderer is at :244.

    ⭐ The most important thing in this report is a defect in the seat's own instrument

    The tsc diagnostic classifier used startsWith('draft.ts'), but tsc prints paths relative to CWD — so it counted 0 for BOTH files, reading simultaneously as "clean draft" and "silent control". The positive control was SILENT until this was fixed.

    An instrument that reported success by being broken, in both directions at once, with its own control failing to fire. Had it shipped, this PR would have carried a green tsc verdict that measured nothing — and the "clean" reading would have looked identical to a real pass.

    The seat found it, fixed it (basename + (line,col) shape, with an explicit unclassified bucket), and the control now fires on every run. A second harness fault — an incomplete manifest making stages 1 and 2 fail on both paths — was also caught and named as "a harness fault that would have read as a code fault."

    This is the class this lane spends most of its dispatch text on. Both instances were found by the seat, unprompted, before any conclusion rested on them.

    The ablation had a discriminator, and the discriminator is the point

    Leg B put the key back on the definition side only, renderer untouched — and predicted that the field-line case and every direction-3 comment case would stay GREEN, because the renderer no longer reads f.primaryKey.

    It held. That proves the two halves of ruling D are independently pinned rather than one assertion wearing two hats. A single "it works now" pin could not have told those apart, and would have gone green if half the fix regressed.

    An earlier, coarser prediction said 3 cases and was wrong; it is recorded as wrong, with the refined enumeration still written before the mutation. Restores byte-identical on both legs and re-run to real verdicts (62/62 each).

    The comment does not overclaim, and that is pinned rather than promised

    I required it not to overclaim in the presence of #10997 (SQLite returns only the first column of a composite key). Delivered:

    ⚠️ A limitation on the delivered value, stated plainly by the seat and worth repeating here

    because the draft path reads only col.primaryKey, a real SqlDriver (which sets isPrimary + table-level primaryKeys) still produces NO key on the introspection-supplied path — so today the new comment fires in production only via --primary-key/opts.

    generateObjectDraft still reads col.primaryKey directly rather than through this same file's primaryKeyReader union that refreshCatalog uses. So the comment machinery is correct for both paths, but #10676 (engine lane) is what makes the introspected path actually deliver a key to it.

    That is a real bound on what a user sees today, and the seat put it in notes_for_other_cards rather than letting the PR read as fully solving the user-visible problem.

    The bounded in-place fix was the right call

    #11059's changeset — unreleased — ended with "the opts.primaryKey path still does not build … untouched here." This PR falsifies that sentence, and it is a user-visible release note. Corrected in place, declared in the PR body with its evidence, same defect class, same gate family, no behaviour.

    Leaving it would have shipped a release note that was false on the day it shipped. Widening beyond it would have been scope creep. This is the narrow correct middle.

    Option C stays genuinely open

    the key is collected in a dedicated primaryKeyFields array inside generateObjectDraft, so routing it to a real binding key later is a one-line change at the definition site rather than a re-derivation

    Ruling D said C is deferred, not rejected. This implementation honours that structurally rather than only in prose.

    Instrument note, now confirmed twice today

    The sanitizer ate every <f> from the PR title and body on creation — including inside a fenced code block quoting generated source, which made the quote misread. #10692's seat hit the same thing independently a few minutes earlier with Record<string, any>. AGENTS.md documents the inline case; the fenced-code case is undocumented and now has two independent measurements. Governed surface, so not mine to edit — recorded here and on #10692.

    Class #10309: re-derived on the final committed diff and it named the gates — nothing added or dropped versus the earlier derivation. That is the third card today where deriving on a clean final diff produced a complete union.


    Generated by Claude Code

  9. added a commit that references this issue on Aug 23, 2026
    4257e4e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions