Skip to content

Commit 15d58db

Browse files
os-samclaude
andauthored
spec(data): retire the import mapping lookup transform's steering params (#10329, ADR-0049) (#11280)
Strict-delete ImportFieldMappingSchema.params' object/fromField/toField/ autoCreate — declared to steer the lookup transform, read by nothing: lookup copies the cell through and reference resolution runs off the target field's own metadata. All fifteen spellings (four canonical + eleven ex-aliases) land on guidance prescriptions; D2 conversion mapping-lookup-params-removed (protocol 18) strips the keys from stored sources; liveness sub-walk note records the retirement; docs warning deleted; changeset with FROM->TO. Claude-Session: https://claude.ai/code/session_01RadETjNRLALFLhFA3xehZP Co-authored-by: Claude <noreply@anthropic.com>
1 parent 91e97af commit 15d58db

8 files changed

Lines changed: 363 additions & 36 deletions

File tree

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
---
2+
"@objectstack/spec": minor
3+
---
4+
5+
feat(spec): retire the import mapping `lookup` transform's steering params (#10329, ADR-0049)
6+
7+
<!-- adr-0087: registered mapping-lookup-params-removed -->
8+
9+
**BREAKING** accept-set narrowing, landing after the v17.0.0 cut (the lockstep
10+
launch-window convention ships it as `minor`; the migration prescription is
11+
registered under protocol major 18, where `os migrate meta` users will look).
12+
13+
`ImportFieldMappingSchema.params` declared four keys whose only stated purpose
14+
was to steer the `lookup` transform — `object` ("Lookup Object"), `fromField`
15+
("Match on"), `toField` ("Value to take"), `autoCreate` ("Create if missing") —
16+
and the import path read none of them: `applyMappingToRows` handles `lookup` in
17+
the same branch as `none` (the cell is copied through unchanged), and reference
18+
resolution runs afterwards in `import-coerce.ts`, driven by the target field's
19+
own metadata. Implementing them would be a second reference-resolution dialect
20+
on the import path, which the code declines to build and the #10329 triage
21+
ruling declined again. `autoCreate` was the one with teeth: it read as
22+
"create the referenced record when nothing matches", and nothing was ever
23+
created — with or without the key, an unresolved cell fails its row with
24+
`import_reference_not_found`.
25+
26+
FROM → TO, per key (all four are pure deletions — none ever had an effect to
27+
lose, so removing them changes no import behaviour):
28+
29+
- `fieldMapping[].params.object`*(removed)*. The referenced object is the
30+
target field's own declared `reference`.
31+
- `fieldMapping[].params.fromField`*(removed)*. The pipeline matches the
32+
cell's display value against the referenced object itself.
33+
- `fieldMapping[].params.toField`*(removed)*. Resolution always writes the
34+
referenced record's id.
35+
- `fieldMapping[].params.autoCreate`*(removed)*. Create or import the
36+
referenced records first, then import the rows that point at them.
37+
38+
One-line fix: delete the four keys (and any of their old alias spellings) from
39+
`fieldMapping[].params`; `os migrate meta --from 17` lists the mechanical edits
40+
for existing sources.
41+
42+
The retirement kit:
43+
44+
- `params` is a `strictObject`, so the keys are strict-deleted with a
45+
`guidance` prescription per spelling — the four canonical keys and the
46+
eleven ex-alias spellings (`lookupObject`/`targetObject`,
47+
`match`/`matchOn`/`matchField`/`keyField`, `returnField`/`valueField`,
48+
`create`/`createIfMissing`/`upsert`) all land on the full prescription
49+
rather than a "did you mean" pointing at a key that is also gone (the
50+
17.0.0 #4509 treatment, one level down)
51+
- ADR-0087 registration: the D2 conversion `mapping-lookup-params-removed`
52+
(protocol 18), wired into the step-18 chain — `os migrate meta --from 17`
53+
strips the four keys from stored `mappings[].fieldMapping[].params`. No
54+
retired-key table entries: these keys sit one sub-walk level below the
55+
authorable-surface drill (`data/ImportFieldMapping:params` is the walked
56+
row, and it stays live), so there is no `defKey:name` row to register or
57+
age out
58+
- pin tests (`mapping.test.ts`): refusal pins per key asserting the
59+
prescription (the `autoCreate` pin asserts the row-fails truth), alias
60+
routing pins, and a surviving-surface pin (`value`/`valueMap`/`separator`
61+
untouched)
62+
- liveness ledger: `liveness/mapping.json`'s `fieldMapping` sub-walk boundary
63+
note now records the retirement instead of parking the finding
64+
- docs: the `import-mappings.mdx` warning about the inert params is deleted
65+
along with the keys; the generated mapping reference no longer lists them

content/docs/data-modeling/import-mappings.mdx

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -129,14 +129,6 @@ path, one is a deliberate pass-through, and one is refused:
129129
| `lookup` | **Pass-through.** The cell is copied unchanged, and the import's own reference resolution turns the display text into a record id afterwards — see [After the mapping](#after-the-mapping-cell-coercion). ||
130130
| `javascript` | **Refused.** There is no server-side sandbox, and silently skipping a declared transform would corrupt data. `defineStack()` fails the build; a runtime-saved mapping is rejected by the import request with `400 UNSUPPORTED_TRANSFORM`. ||
131131

132-
<Callout type="warn">
133-
The `lookup` transform's own `params` keys (`object`, `fromField`, `toField`,
134-
`autoCreate`) parse, but the import path reads none of them: reference resolution is
135-
driven by the **target object's field definitions**, not by the mapping. Do not write
136-
them expecting them to steer anything — tracked as
137-
[#10329](https://github.com/objectstack-ai/objectstack/issues/10329).
138-
</Callout>
139-
140132
For logic beyond these, transform the data before you post it, or model it as a
141133
[flow](/docs/automation) on the target object.
142134

content/docs/references/data/mapping.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ const result = ImportFieldMappingSchema.parse(data);
3030
| **source** | `string \| string[]` || Source column header(s) |
3131
| **target** | `string \| string[]` || Target object field(s) |
3232
| **transform** | `Enum<'none' \| 'constant' \| 'lookup' \| 'split' \| 'join' \| 'javascript' \| 'map'>` | optional (default: `"none"`) | |
33-
| **params** | `{ value?: any; object?: string; fromField?: string; toField?: string; … }` | optional | |
33+
| **params** | `{ value?: any; valueMap?: Record<string, any>; separator?: string }` | optional | |
3434

3535

3636
---

packages/spec/liveness/mapping.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
"status": "live",
3131
"verifiedAt": "2026-08-01",
3232
"evidence": "packages/rest/src/import-mapping.ts:98-105, packages/rest/src/import-mapping.ts:115-167",
33-
"note": "the pipeline itself: source/target/transform/params all consumed. none/constant/map/split/join applied in applyMappingToRows (`params.separator` :124, `.value` :132, `.valueMap` :137); `lookup` copies through for the pipeline's metaMap reference resolution; `javascript` is REJECTED with a 400 (no server sandbox — implement-or-reject-loudly). SUB-WALK BOUNDARY, recorded not hidden: `params`' lookup-specific keys (`object`/`fromField`/`toField`/`autoCreate`) are read by nothing — reference resolution comes from the target object's own field definitions, not from these — and they sit one level below the drill, so only this note governs them."
33+
"note": "the pipeline itself: source/target/transform/params all consumed. none/constant/map/split/join applied in applyMappingToRows (`params.separator` :124, `.value` :132, `.valueMap` :137); `lookup` copies through for the pipeline's metaMap reference resolution; `javascript` is REJECTED with a 400 (no server sandbox — implement-or-reject-loudly). SUB-WALK BOUNDARY, resolved: `params`' lookup-specific keys (`object`/`fromField`/`toField`/`autoCreate`) were read by nothing — reference resolution comes from the target object's own field definitions, not from these — and were REMOVED in the 17.x line (#10329, ADR-0049 enforce-or-remove; strict deletion, so nothing dead remains one level below the drill). Every spelling — the four canonical keys and their eleven ex-aliases — now lands on a guidance prescription at parse; the mapping-lookup-params-removed conversion (protocol 18) strips them from stored sources. The surviving params keys (`value`/`valueMap`/`separator`) are all consumed by applyMappingToRows."
3434
},
3535
"mode": {
3636
"status": "live",

packages/spec/src/conversions/registry.ts

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7556,6 +7556,120 @@ const recordHighlightsFieldIconRemoved: MetadataConversion = {
75567556
},
75577557
};
75587558

7559+
/**
7560+
* `mapping.fieldMapping[].params` lookup keys removed (#10329, ADR-0049
7561+
* enforce-or-remove — the sub-walk half of the 17.0.0 #4509 mapping cleanup).
7562+
*
7563+
* `object` / `fromField` / `toField` / `autoCreate` declared a per-entry
7564+
* reference-resolution dialect that the import path never implemented:
7565+
* `applyMappingToRows` handles `lookup` in the same branch as `none` (the cell
7566+
* is copied through unchanged), and reference resolution runs afterwards in
7567+
* `import-coerce.ts` off the TARGET FIELD's own metadata — never off these
7568+
* keys. Implementing them (a second reference-resolution dialect on the import
7569+
* path) is what the code comment in `packages/rest/src/import-mapping.ts`
7570+
* declines to build, and the #10329 triage ruling confirms that posture.
7571+
*
7572+
* `autoCreate` was the one with teeth: it read as "create the referenced
7573+
* record when nothing matches", and nothing was ever created — with or without
7574+
* the key, an unresolved cell fails its row with `import_reference_not_found`.
7575+
* The schema prescription says so outright, because an author who believed the
7576+
* key is one support ticket away from "my import used to create these".
7577+
*
7578+
* **A pure lossless delete.** None of the four ever had an effect to lose, so
7579+
* stripping them preserves observed import behaviour exactly.
7580+
*
7581+
* Scoped to the `mappings` collection, then drilled two levels down
7582+
* (`fieldMapping[]` is an ARRAY one level below the item, `params` a dict one
7583+
* below that), so the top-level-only `stripKeys` runs per entry's params bag —
7584+
* the `metric-filters-removed` shape, one level deeper. Deliberately narrow:
7585+
* `object` / `targetObject`-style keys are live all over the tree, and a
7586+
* stack-wide strip would delete enforced keys from other types.
7587+
*/
7588+
const mappingLookupParamsRemoved: MetadataConversion = {
7589+
id: 'mapping-lookup-params-removed',
7590+
toMajor: 18,
7591+
retiredFromLoadPath: true,
7592+
surface: 'mapping.fieldMapping[].params.object / .fromField / .toField / .autoCreate',
7593+
summary:
7594+
"mapping lookup params 'object'/'fromField'/'toField'/'autoCreate' removed (#10329, "
7595+
+ 'ADR-0049 — the import path never read them: `lookup` copies the cell through and '
7596+
+ "reference resolution runs off the target field's own metadata. `autoCreate` never "
7597+
+ 'created anything — an unresolved reference fails the row either way)',
7598+
apply(stack, emit) {
7599+
const RETIRED = ['object', 'fromField', 'toField', 'autoCreate'];
7600+
return mapCollection(stack, 'mappings', (m, path) => {
7601+
const entries = m.fieldMapping;
7602+
if (!Array.isArray(entries)) return m;
7603+
let touched = false;
7604+
const nextEntries = entries.map((entry, i) => {
7605+
if (!isDict(entry)) return entry;
7606+
const params = entry.params;
7607+
if (!isDict(params)) return entry;
7608+
const stripped = stripKeys(params, RETIRED, emit, `${path}.fieldMapping[${i}].params`);
7609+
if (stripped === params) return entry;
7610+
touched = true;
7611+
return { ...entry, params: stripped };
7612+
});
7613+
if (!touched) return m;
7614+
return { ...m, fieldMapping: nextEntries };
7615+
});
7616+
},
7617+
fixture: {
7618+
before: {
7619+
mappings: [{
7620+
name: 'csv_import_contacts',
7621+
targetObject: 'contact',
7622+
fieldMapping: [
7623+
// The retired shape: a lookup entry steering nothing.
7624+
{
7625+
source: 'Account Name',
7626+
target: 'account_id',
7627+
transform: 'lookup',
7628+
params: { object: 'account', fromField: 'name', toField: 'id', autoCreate: true },
7629+
},
7630+
// A live params bag on another transform rides through untouched —
7631+
// the strip dispatches on key presence, and copy-on-write keeps the
7632+
// reference.
7633+
{
7634+
source: 'Status',
7635+
target: 'status',
7636+
transform: 'map',
7637+
params: { valueMap: { Open: 'open' } },
7638+
},
7639+
// No params at all — untouched.
7640+
{ source: 'Email', target: 'email' },
7641+
],
7642+
}],
7643+
},
7644+
after: {
7645+
mappings: [{
7646+
name: 'csv_import_contacts',
7647+
targetObject: 'contact',
7648+
fieldMapping: [
7649+
// The emptied bag stays: the conversion strips KEYS, and deleting
7650+
// the bag itself would be a second, unprescribed edit. `params: {}`
7651+
// parses clean.
7652+
{
7653+
source: 'Account Name',
7654+
target: 'account_id',
7655+
transform: 'lookup',
7656+
params: {},
7657+
},
7658+
{
7659+
source: 'Status',
7660+
target: 'status',
7661+
transform: 'map',
7662+
params: { valueMap: { Open: 'open' } },
7663+
},
7664+
{ source: 'Email', target: 'email' },
7665+
],
7666+
}],
7667+
},
7668+
// Four notices: one per retired key on the single lookup entry.
7669+
expectedNotices: 4,
7670+
},
7671+
};
7672+
75597673
export const CONVERSIONS_BY_MAJOR: Readonly<Record<number, readonly MetadataConversion[]>> = {
75607674
11: [flowNodeHttpRename, pageKindJsxToHtml, flowNodeFilterAlias, objectCompactLayoutRename],
75617675
13: [stackRolesToPositions, owdLegacyReadAliases, sharingRecipientRoleToPosition],
@@ -7637,6 +7751,7 @@ export const CONVERSIONS_BY_MAJOR: Readonly<Record<number, readonly MetadataConv
76377751
fieldColumnListsCanonicalized,
76387752
metricFiltersRemoved,
76397753
recordHighlightsFieldIconRemoved,
7754+
mappingLookupParamsRemoved,
76407755
],
76417756
};
76427757

packages/spec/src/data/mapping.test.ts

Lines changed: 82 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -87,23 +87,92 @@ describe('ImportFieldMappingSchema', () => {
8787
expect(mapping.params?.value).toBe('active');
8888
});
8989

90-
it('should accept lookup transform', () => {
90+
it('should accept lookup transform (a pass-through with no params of its own)', () => {
9191
const mapping = ImportFieldMappingSchema.parse({
9292
source: 'account_name',
9393
target: 'account_id',
9494
transform: 'lookup',
95-
params: {
96-
object: 'account',
97-
fromField: 'name',
98-
toField: 'id',
99-
autoCreate: false
100-
}
10195
});
10296

10397
expect(mapping.transform).toBe('lookup');
104-
expect(mapping.params?.object).toBe('account');
105-
expect(mapping.params?.fromField).toBe('name');
106-
expect(mapping.params?.toField).toBe('id');
98+
});
99+
100+
// ── `params` lookup keys retired in the 17.x line (#10329, ADR-0049) ───────
101+
//
102+
// `object` / `fromField` / `toField` / `autoCreate` declared a per-entry
103+
// reference-resolution dialect the import path never implemented: `lookup`
104+
// copies the cell through and resolution runs afterwards off the target
105+
// field's own metadata. These pin the REJECTION carrying the prescription,
106+
// not just the absence — the schema is strict, so a bare "unrecognized key"
107+
// would already fail; what must survive refactors is the guidance.
108+
109+
const lookupEntry = { source: 'account_name', target: 'account_id', transform: 'lookup' as const };
110+
111+
it('rejects the retired `params.object` with the target-field-metadata prescription', () => {
112+
expect(() => ImportFieldMappingSchema.parse({
113+
...lookupEntry,
114+
params: { object: 'account' },
115+
})).toThrow(/params\.object.*removed.*TARGET FIELD/s);
116+
});
117+
118+
it('rejects the retired `params.fromField` with the display-value-matching prescription', () => {
119+
expect(() => ImportFieldMappingSchema.parse({
120+
...lookupEntry,
121+
params: { fromField: 'name' },
122+
})).toThrow(/params\.fromField.*removed.*display.*value/s);
123+
});
124+
125+
it('rejects the retired `params.toField` with the record-id prescription', () => {
126+
expect(() => ImportFieldMappingSchema.parse({
127+
...lookupEntry,
128+
params: { toField: 'id' },
129+
})).toThrow(/params\.toField.*removed.*record.*id/s);
130+
});
131+
132+
it('rejects the retired `params.autoCreate` saying what ACTUALLY happens (row fails)', () => {
133+
// The one with teeth: the key read as "create the referenced record when
134+
// nothing matches". Nothing was ever created — the row fails with an
135+
// unresolved-reference error either way — so the prescription must say so
136+
// rather than merely "removed".
137+
const parse = () => ImportFieldMappingSchema.parse({
138+
...lookupEntry,
139+
params: { autoCreate: true },
140+
});
141+
expect(parse).toThrow(/params\.autoCreate.*removed/s);
142+
expect(parse).toThrow(/nothing was ever created/is);
143+
expect(parse).toThrow(/import_reference_not_found/s);
144+
});
145+
146+
it('routes the retired ALIAS spellings to the same prescriptions', () => {
147+
// The alias table used to fold eleven spellings onto the four keys. Leaving
148+
// them as aliases would answer "did you mean `fromField`?" — a rename
149+
// suggestion pointing at a key that is also gone, i.e. a second rejection.
150+
expect(() => ImportFieldMappingSchema.parse({
151+
...lookupEntry, params: { lookupObject: 'account' },
152+
})).toThrow(/params\.object.*removed/s);
153+
expect(() => ImportFieldMappingSchema.parse({
154+
...lookupEntry, params: { matchOn: 'name' },
155+
})).toThrow(/params\.fromField.*removed/s);
156+
expect(() => ImportFieldMappingSchema.parse({
157+
...lookupEntry, params: { returnField: 'id' },
158+
})).toThrow(/params\.toField.*removed/s);
159+
expect(() => ImportFieldMappingSchema.parse({
160+
...lookupEntry, params: { createIfMissing: true },
161+
})).toThrow(/params\.autoCreate.*removed/s);
162+
});
163+
164+
it('leaves the surviving params surface intact', () => {
165+
const mapping = ImportFieldMappingSchema.parse({
166+
source: 'status',
167+
target: 'status_code',
168+
transform: 'map',
169+
params: { valueMap: { Open: 'open' } },
170+
});
171+
expect(mapping.params).not.toHaveProperty('object');
172+
expect(mapping.params).not.toHaveProperty('fromField');
173+
expect(mapping.params).not.toHaveProperty('toField');
174+
expect(mapping.params).not.toHaveProperty('autoCreate');
175+
expect(mapping.params?.valueMap).toHaveProperty('Open', 'open');
107176
});
108177

109178
it('should accept map transform', () => {
@@ -375,14 +444,11 @@ describe('MappingSchema', () => {
375444
params: { value: 'active' }
376445
},
377446
{
447+
// `lookup` is a pass-through: the import pipeline resolves the
448+
// reference from the target field's own metadata (#10329).
378449
source: 'account_name',
379450
target: 'account_id',
380-
transform: 'lookup',
381-
params: {
382-
object: 'account',
383-
fromField: 'name',
384-
toField: 'id'
385-
}
451+
transform: 'lookup'
386452
},
387453
{
388454
source: ['first_name', 'last_name'],

0 commit comments

Comments
 (0)