Skip to content

Commit 761a0ba

Browse files
hotlongclaude
andauthored
fix(spec): 把 strictObject 的 history 句移到消息末尾,修法前置 (#5955) (#6375)
`strictUnknownKeyError` 的消息此前拼成「前置语 → history → 修法」, 每个 surface 自己声明的沿革句(「这个键以前会被静默丢弃」)正好夹在 作者真正需要的两件事之间:哪个键错了,和该写什么。 #5762 把 `flow-time-relative-descriptor-invalid` 升为 error 之后这一点 开始碍事:多个消费方把 finding 打成单行 —— `os validate` 的 `• where: message`、CI 日志,以及 `validateFlowTriggerReadiness` (它刻意把 schema 原文里的换行压平,好让 CLI 的项目符号列表保持对齐)。 `TimeRelativeTriggerSchema` 的 history 句是 224 字符,于是在 #5496 的 描述符上,`Did you mean` 落在 480 字符行的第 443 位,前面挡着一句没有 动作价值的沿革。作者(常常是 AI)只读这行的开头就动手了。 现在沿革句排到最后,两个修法通道(rename 与 guidance 处方)都排在它 前面 —— 一条处方和一次改名一样可执行,不能留在句子后面。 一处拼接点改动,消息长度逐字节不变(480/366/544/598),没有删任何文本、 没有加任何条件分支。新增顺序钉测试 5 条置于 `strict-object.test.ts`; 反向验证:把旧拼接式放回去,其中 3 条转红。 Claude-Session: https://claude.ai/code/session_01JTSZAjgtL3oR6YcpNDhW3T Co-authored-by: Claude <noreply@anthropic.com>
1 parent e15bf7e commit 761a0ba

4 files changed

Lines changed: 166 additions & 7 deletions

File tree

‎.changeset/action-param-strict-unknown-keys.md‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,10 @@ offending key, and — when the key is a recognisable spelling of a declared one
1818
the canonical key to use instead:
1919

2020
```
21-
Unrecognized key(s) on this action param: `reference_to`. Until #3405 these were
22-
dropped silently — the param still parsed, so a mis-spelled config shipped as a
23-
control that quietly ignored it. Did you mean `reference_to` → `reference`?
21+
Unrecognized key(s) on this action param: `reference_to`. Did you mean
22+
`reference_to` → `reference`? Until #3405 these were dropped silently — the param
23+
still parsed, so a mis-spelled config shipped as a control that quietly ignored
24+
it.
2425
```
2526

2627
**Migration.** A param that previously carried an extra key now fails to parse.
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
---
2+
"@objectstack/spec": patch
3+
---
4+
5+
fix(spec): put the unknown-key fix before the surface history sentence (#5955)
6+
7+
`strictUnknownKeyError` — the error map behind every `strictObject` authoring
8+
surface — assembled its message as *front matter → history → fix*. The
9+
`history` sentence a surface declares ("why this key used to be dropped
10+
silently") therefore sat between the two things an author actually needs: the
11+
name of the key that is wrong, and the key to write instead.
12+
13+
That was tolerable while these rejections were warnings. It stopped being
14+
tolerable when #5762 promoted `flow-time-relative-descriptor-invalid` to
15+
**error**, because several consumers render a finding on ONE line — `os
16+
validate`'s `• where: message`, CI logs, and `validateFlowTriggerReadiness`,
17+
which deliberately flattens the newlines out of the schema's own text so the
18+
CLI's bulleted list stays aligned. `TimeRelativeTriggerSchema`'s history
19+
sentence is 224 characters, so on the descriptor from #5496 the words
20+
`Did you mean` landed at character 443 of a 480-character line, behind a
21+
sentence about 2026 that carries no instruction. The author — often an AI —
22+
reads the front of that line and acts on it.
23+
24+
The sentence now goes last:
25+
26+
```text
27+
Unrecognized key(s) on {surface}: `k1`, `k2`. which keys are wrong
28+
[ Did you mean `k1` → `canonical`? ] fix, channel 1 (renames)
29+
[ newline + " • " + {guidance} ] fix, channel 2 (prescriptions)
30+
{history} why it used to be silent
31+
```
32+
33+
Measured on `TimeRelativeTriggerSchema`, before and after:
34+
35+
| case | length before | length after | `Did you mean` at |
36+
|---|---|---|---|
37+
| #5496 descriptor (`field` + missing `dateField` + scalar `offsetDays`) | 480 | 480 | 443 → 219 |
38+
| single misspelled key (`offsetDay`) | 366 | 366 | 329 → 105 |
39+
| guidance hit (`schedule`) | 544 | 544 | n/a (bullet at 92) |
40+
41+
**Nothing was deleted, nothing became conditional.** Every declared `history`
42+
is still emitted, verbatim, exactly once per message — message lengths are
43+
byte-identical, only the position moved. Both fix channels moved ahead of it:
44+
a `guidance` prescription is as actionable as a rename, so it could not be left
45+
behind the sentence either.
46+
47+
**Migration.** No authoring change, and no schema change: `history` is still a
48+
required option, spelled the same way, on all 62 `strictObject` surfaces and
49+
44 direct `strictUnknownKeyError` call sites. A test that asserts the full
50+
message text in order needs its expectation reordered; a test that asserts
51+
fragments with `toContain` is unaffected. The order itself is now pinned in
52+
`strict-object.test.ts`, so it cannot silently regress.

‎packages/spec/src/shared/strict-object.test.ts‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,81 @@ describe('strictObject', () => {
170170
});
171171
});
172172

173+
/**
174+
* The ORDER the message emits its parts in — pinned as order, not as presence.
175+
*
176+
* Every assertion above this block is a `toContain` on one fragment, so all of
177+
* them stayed green while `history` sat in the MIDDLE of the message, between
178+
* "which key is wrong" and "here is the fix". That mattered once #5762 promoted
179+
* `flow-time-relative-descriptor-invalid` to **error**: several consumers render
180+
* a finding on ONE line (`os validate`'s `• where: message`, CI logs, and
181+
* `validateFlowTriggerReadiness`, which flattens the newlines out of the schema's
182+
* own text), and `TimeRelativeTriggerSchema`'s history sentence is 224
183+
* characters — so the author, often an AI, read the front of the line and found
184+
* a sentence about 2026 instead of the key to write (#5955).
185+
*
186+
* Direction A of that issue's ruling: move the sentence to the end. Nothing is
187+
* deleted and nothing is conditional — which is exactly why it needs an ORDER
188+
* pin rather than another presence check. A future edit that folds `history`
189+
* back into the front matter passes every `toContain` in this file; it fails
190+
* here.
191+
*/
192+
describe('message order — the fix comes before the history (#5955)', () => {
193+
const HISTORY = 'Until #4001 these were dropped silently — the widget still rendered.';
194+
195+
const messageFor = (body: Record<string, unknown>) => {
196+
const r = WidgetSchema.safeParse({ name: 'x', ...body });
197+
expect(r.success).toBe(false);
198+
return r.error!.issues[0]!.message;
199+
};
200+
201+
it('names the wrong key first, then the rename, then the history', () => {
202+
const m = messageFor({ colummSpan: 2 });
203+
// 1. which key is wrong — and nothing before it
204+
expect(m.startsWith('Unrecognized key(s) on this widget: `colummSpan`.')).toBe(true);
205+
// 2. the fix, immediately after it (this is the whole point of the reorder)
206+
expect(m).toContain('`colummSpan`. Did you mean `colummSpan` → `columnSpan`?');
207+
// 3. the history sentence, verbatim, last — moved, never dropped
208+
expect(m.endsWith(` ${HISTORY}`)).toBe(true);
209+
expect(m.indexOf('Did you mean')).toBeLessThan(m.indexOf(HISTORY));
210+
});
211+
212+
it('puts a guidance prescription ahead of the history too', () => {
213+
// The other fix channel. A tombstone/wrong-layer prescription is as
214+
// actionable as a rename, so it cannot sit behind the sentence either.
215+
const m = messageFor({ span: 2 });
216+
expect(m.startsWith('Unrecognized key(s) on this widget: `span`.')).toBe(true);
217+
expect(m).toContain('\n • `span` was retired in vX. Use `columnSpan`.');
218+
expect(m.endsWith(` ${HISTORY}`)).toBe(true);
219+
expect(m.indexOf('was retired in vX')).toBeLessThan(m.indexOf(HISTORY));
220+
});
221+
222+
it('keeps BOTH fix channels ahead of the history in one message', () => {
223+
const m = messageFor({ span: 2, colummSpan: 3 });
224+
expect(m.startsWith('Unrecognized key(s) on this widget: `span`, `colummSpan`.')).toBe(true);
225+
expect(m.indexOf('Did you mean')).toBeLessThan(m.indexOf(HISTORY));
226+
expect(m.indexOf('was retired in vX')).toBeLessThan(m.indexOf(HISTORY));
227+
expect(m.endsWith(` ${HISTORY}`)).toBe(true);
228+
});
229+
230+
it('emits the history exactly once, whatever the key count', () => {
231+
// It is a per-SURFACE sentence, not a per-key one: zod raises a single
232+
// `unrecognized_keys` issue naming every offending key, so the sentence is
233+
// appended to that one message once — the property that makes "last" a
234+
// well-defined position at all.
235+
const m = messageFor({ colummSpan: 2, alsoWrong: 3, andThis: 4 });
236+
expect(m.split(HISTORY)).toHaveLength(2);
237+
});
238+
239+
it('is unchanged when there is no fix to offer', () => {
240+
// No rename, no prescription — the sentence follows the key statement
241+
// directly, exactly as it always did. Full-message pin, so any stray
242+
// separator or duplicated clause fails here.
243+
expect(messageFor({ nonsense: 1 }))
244+
.toBe(`Unrecognized key(s) on this widget: \`nonsense\`. ${HISTORY}`);
245+
});
246+
});
247+
173248
/**
174249
* Never suggest a key the schema cannot accept.
175250
*

‎packages/spec/src/shared/suggestions.zod.ts‎

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,12 @@ export interface StrictUnknownKeyErrorOptions {
241241
* key. Matched case-sensitively (exact authored spelling).
242242
*/
243243
guidance?: Readonly<Record<string, string>>;
244-
/** One sentence of history: why this key would previously have failed silently. */
244+
/**
245+
* One sentence of history: why this key would previously have failed
246+
* silently. Rendered **last**, after both fix channels (`Did you mean` and
247+
* the `guidance` bullets) — see the ordering note on
248+
* {@link strictUnknownKeyError}.
249+
*/
245250
history: string;
246251
}
247252

@@ -269,6 +274,24 @@ export interface StrictUnknownKeyErrorOptions {
269274
* First consumers: `ui/action.zod.ts` (#3746, the template this generalizes),
270275
* `security/permission.zod.ts`, `automation/flow.zod.ts`.
271276
*
277+
* ## Message order: the fix comes before the history (#5955)
278+
*
279+
* One message per rejected object — every unknown key of that object is named
280+
* in it, and the surface's `history` sentence appears exactly once, whatever
281+
* the key count. The parts are emitted in the order an author has to read
282+
* them:
283+
*
284+
* ```text
285+
* Unrecognized key(s) on {surface}: `k1`, `k2`. ← which keys are wrong
286+
* [ Did you mean `k1` → `canonical`? ] ← fix, channel 1 (renames)
287+
* [ \n • {guidance} ] ← fix, channel 2 (prescriptions)
288+
* {history} ← why it used to be silent
289+
* ```
290+
*
291+
* `history` sat in the middle until #5955, which pushed the fix past character
292+
* ~220 on the single-line displays several consumers use. It is still emitted
293+
* verbatim and unconditionally — only its position moved.
294+
*
272295
* ## The table is recorded as it is built (#5483)
273296
*
274297
* Every call registers its `{ surface, knownKeys, aliases, guidance }` with
@@ -317,10 +340,18 @@ export function strictUnknownKeyError(options: StrictUnknownKeyErrorOptions): z.
317340
aliases[aliasProbe(key)] ?? findClosestMatches(key, knownKeys, maxDistance, 1)[0];
318341
if (canonical && canonical !== key) renames.push(`\`${key}\` → \`${canonical}\``);
319342
}
320-
let message =
321-
`Unrecognized key(s) on ${surface}: ${keys.map((k) => `\`${k}\``).join(', ')}. ${history}`;
343+
// Order: WHICH KEY IS WRONG → HOW TO FIX IT → why it used to be silent.
344+
// `history` used to sit in the middle, between the key statement and the
345+
// suggestion, which put the fix past character ~220 of a message several
346+
// consumers render on ONE line (`os validate`'s `• where: message`, CI
347+
// logs, and `validateFlowTriggerReadiness`, which flattens the newlines).
348+
// Since #5762 promoted one of those rules to error level the author — often
349+
// an AI — reads the front of that line and acts on it, so the prescription
350+
// has to be there. Nothing is dropped or made conditional: the sentence is
351+
// still emitted verbatim, once per message, just last (#5955).
352+
let message = `Unrecognized key(s) on ${surface}: ${keys.map((k) => `\`${k}\``).join(', ')}.`;
322353
if (renames.length) message += ` Did you mean ${renames.join(', ')}?`;
323354
if (prescriptions.length) message += `\n${prescriptions.map((p) => ` • ${p}`).join('\n')}`;
324-
return message;
355+
return `${message} ${history}`;
325356
};
326357
}

0 commit comments

Comments
 (0)