diff --git a/.changeset/olive-donkeys-repeat.md b/.changeset/olive-donkeys-repeat.md new file mode 100644 index 0000000000..bcb5a5111d --- /dev/null +++ b/.changeset/olive-donkeys-repeat.md @@ -0,0 +1,16 @@ +--- +"@objectstack/spec": patch +--- + +修正未知键「你是不是想写」兜底对 camelCase 键的系统性偏弱 (#4990) + +`findClosestMatches()` 此前只把**输入**小写化,**候选不做同样处理**,于是候选键里的每一个大写字母都要额外付一次编辑距离。叠加 `strictUnknownKeyError` 长度相对的预算(短键即 2),短 camelCase 键上一个普通笔误就够不着建议了:`hideOn` 对 `hiddenOn` 真实距离 2、加大写罚分后 3、超预算返回空,而同一个词写成全小写的 `hiddenon` 反而拿得到建议 —— 把键写错大小写的作者,比写对了大小写、只错一两个字母的作者得到更好的诊断。 + +现在打分对两侧做同样的归一化(大小写 + `-`/空格 → `_`),回显仍用候选的原始拼写。两处附带修正: + +- 折叠后距离为 0 的候选(只有大小写之差,例如 `hiddenon` → `hiddenOn`)不再被 `distance > 0` 过滤丢掉 —— 那是兜底能给出的最有把握的一条建议。过滤改为只排除作者逐字写过的那个字符串;顺带修掉一个未被记录的同源缺陷:旧实现会把作者写对的键原样回显成「你是不是想写」。 +- 折叠后打平时,以作者自己写的大小写作为**次级**排序依据(`yxAis` 同时距 `yAxis`、`xAxis` 为 2,大写 A 指向前者)。 + +由于「TS config keys → camelCase」是全仓约定,这条影响 #4001 战役已落地的每一条未知键错误信息。在全部 325 组真实候选集上按单字符笔误实测:329 例从「没有建议」变为有建议(328 例正确),**0 例失去建议**,31 例改变选中项(30 例更准)。`ui/responsive.zod.ts` 中批 13 为此写的逐例 `hideOn: 'hiddenOn'` alias 已随之退役,其实测值改由 `responsive.test.ts` 的断言保存。 + +`data/object.zod.ts` 的 `suggestKey` 经核查**不同病**(它本来就对两侧都做了小写化),已补测试锁定,防止两处再次分叉。 diff --git a/packages/spec/src/data/object.test.ts b/packages/spec/src/data/object.test.ts index 30151d9f4d..13ef210757 100644 --- a/packages/spec/src/data/object.test.ts +++ b/packages/spec/src/data/object.test.ts @@ -892,6 +892,34 @@ describe('ObjectSchema.create()', () => { expect(message).toContain('#1535'); }); + // #4990 note 1 asked whether this file's own `suggestKey` shares the + // camelCase weakness that `findClosestMatches` had. It does NOT: it already + // lowercases BOTH sides (`editDistance(unknown.toLowerCase(), + // key.toLowerCase())`), so a declared key's capitals were never charged to + // the author here. Pinning it means the two suggesters cannot drift apart + // again — this is the property #4990 fixed in the other one. + it('suggestKey judges a typo identically in either case (#4990 note 1)', () => { + const bullet = (key: string): string => { + try { + ObjectSchema.create({ + name: 'demo', + fields: {}, + [key]: 1, + } as Record as Parameters[0]); + } catch (e) { + return ((e as Error).message.split('\n').find((l) => l.trim().startsWith('•')) ?? '').trim(); + } + throw new Error(`expected ObjectSchema.create to reject \`${key}\``); + }; + // A camelCase key the fallback CAN reach, and its all-lowercase twin: + // both must land on the same canonical key. + expect(bullet('nameFeild')).toContain('did you mean `nameField`'); + expect(bullet('namefeild')).toContain('did you mean `nameField`'); + // And one it cannot reach — the verdict must again not depend on case. + expect(bullet('primaryFeild')).not.toContain('did you mean'); + expect(bullet('primaryfeild')).not.toContain('did you mean'); + }); + // Tombstones: a RETIRED key's rejection must carry the upgrade // prescription — the compile/validation error is the one channel every // upgrading consumer (human or agent) is guaranteed to hit. diff --git a/packages/spec/src/shared/suggestions.test.ts b/packages/spec/src/shared/suggestions.test.ts index 54297688a5..821e1d2116 100644 --- a/packages/spec/src/shared/suggestions.test.ts +++ b/packages/spec/src/shared/suggestions.test.ts @@ -58,6 +58,120 @@ describe('findClosestMatches', () => { }); }); +describe('camelCase parity in the distance fallback (#4990)', () => { + // The budget `strictUnknownKeyError` actually spends. Reproduced rather than + // imported because the point of these tests is the INTERACTION between the + // budget and the scoring — a test that shared the constant could not show it. + const budget = (key: string) => Math.max(2, Math.floor(key.length / 3)); + const suggest = (input: string, cands: readonly string[]): string | undefined => + findClosestMatches(input, cands, budget(input), 1)[0]; + + it('re-measures the four rows the issue tabled, at their real budgets', () => { + // Before the fix, row 1 was `[]`: `hideOn` scored 3 against a budget of 2, + // because `hiddenOn`'s capital O was charged to the author as an edit. + expect(suggest('hideOn', ['hiddenOn'])).toBe('hiddenOn'); + expect(suggest('hiddenon', ['hiddenOn'])).toBe('hiddenOn'); + expect(suggest('hiddenOnn', ['hiddenOn'])).toBe('hiddenOn'); + expect(suggest('maxLenght', ['maxLength'])).toBe('maxLength'); + }); + + // THE INVARIANT, and the substance of #4990. + // + // Stating it took one correction worth recording. The issue phrases it as + // "the all-lowercase form must not get a better suggestion than the correctly + // cased one", and the obvious encoding — compare `suggest(T)` with + // `suggest(T.toLowerCase())` — is VACUOUS against the buggy code, which + // lowercased the input as its first act: both calls collapsed to the same + // one and agreed trivially, on 462 probes, while the bug was fully present. + // + // The asymmetry is not between two spellings of the INPUT. It is between two + // spellings of the DECLARED KEY: the identical typo was judged one way + // against `hiddenOn` and another against `hiddenon`, because only the + // candidate kept its capitals and each one cost the author an edit. That is + // the title's "systematically weak on camelCase keys", and it is what this + // pins: a key's capitalisation must not change the verdict on a typo. + // + // Measured against the pre-fix implementation, this corpus breaks parity 55 + // times in 462 probes — in BOTH directions (`bordeRradius` resolved against + // camelCase `borderRadius` but not against flat `borderradius`, the capital + // paying off by luck). Case must decide nothing either way. + it('judges a typo the same whether the declared key is camelCase or flat', () => { + const corpus = [ + 'hiddenOn', 'maxLength', 'iteratorVariable', 'primaryField', 'defaultValue', + 'borderRadius', 'customVars', 'referenceTo', 'maxIterations', 'displayName', + 'onDelete', 'sortOrder', 'isRequired', 'allowedPaths', 'triggerPhrases', + 'xAxis', 'viewName', 'pluginId', + ]; + const failures: string[] = []; + for (const key of corpus) { + const flat = key.toLowerCase(); + // How an author actually mistypes: dropped character, swapped pair, + // dropped pair — spelled with the camelCase they were aiming for. + const variants = new Set(); + for (let i = 1; i < key.length; i++) variants.add(key.slice(0, i) + key.slice(i + 1)); + for (let i = 1; i < key.length - 1; i++) { + variants.add(key.slice(0, i) + key[i + 1] + key[i] + key.slice(i + 2)); + variants.add(key.slice(0, i) + key.slice(i + 2)); + } + variants.delete(key); + for (const typo of variants) { + // Skip the degenerate comparison: when the typo lowercases to the flat + // key itself (`bordeRradius` → `borderradius`), the flat side is the + // author echoing their own string and is correctly refused, while the + // camelCase side is a real — and maximally confident — suggestion. + // That asymmetry is the self-match rule, not a case-parity break. + if (typo.toLowerCase() === flat) continue; + const againstCamel = suggest(typo, [key]) === key; + const againstFlat = suggest(typo.toLowerCase(), [flat]) === flat; + if (againstCamel !== againstFlat) { + failures.push( + `${key}: '${typo}' resolves=${againstCamel} but flat '${flat}' resolves=${againstFlat}`, + ); + } + } + } + expect( + failures, + `a declared key's capitalisation changed the verdict:\n${failures.join('\n')}`, + ).toEqual([]); + }); + + // The issue's own headline comparison, kept as a named case because it is the + // sentence the bug was reported in: the author who wrote the key ALL LOWERCASE + // was served better than the author who cased it right and slipped two letters. + it('serves the correctly-cased author no worse than the all-lowercase one', () => { + expect(suggest('hiddenon', ['hiddenOn'])).toBe('hiddenOn'); // was already fine + expect(suggest('hideOn', ['hiddenOn'])).toBe('hiddenOn'); // was `undefined` + }); + + it('suggests a candidate that differs from the input ONLY in case', () => { + // Folding both sides makes such a candidate distance 0, and the old + // `distance > 0` filter discarded it along with the true self-match. It is + // the single most confident suggestion the fallback can make. + expect(suggest('hiddenon', ['hiddenOn'])).toBe('hiddenOn'); + expect(suggest('MAXLENGTH', ['maxLength'])).toBe('maxLength'); + expect(suggest('reference_to', ['referenceTo'])).toBe('referenceTo'); + }); + + it('still refuses to echo back the exact string the author typed', () => { + expect(findClosestMatches('maxLength', ['maxLength', 'minLength'], 3, 3)) + .not.toContain('maxLength'); + }); + + it('breaks a folded tie on the author\'s own capitalisation', () => { + // `allowedaPths` is equidistant from `allowedPaths` and `allowedAPIs` once + // case is folded away. The capitals the author did type are the only + // evidence left, and they point at `allowedPaths`. + expect(suggest('allowedaPths', ['allowedAPIs', 'allowedPaths'])).toBe('allowedPaths'); + }); + + it('leaves genuinely unrelated keys unsuggested — the fold is not a widening', () => { + const keys = ['hiddenOn', 'columns', 'order', 'breakpoint']; + expect(suggest('workflows', keys)).toBeUndefined(); + expect(suggest('responsiveStyles', keys)).toBeUndefined(); + }); +}); + describe('suggestFieldType', () => { it('should suggest via alias map for common alternatives', () => { expect(suggestFieldType('string')).toEqual(['text']); diff --git a/packages/spec/src/shared/suggestions.zod.ts b/packages/spec/src/shared/suggestions.zod.ts index ab875575fc..6d48bcf444 100644 --- a/packages/spec/src/shared/suggestions.zod.ts +++ b/packages/spec/src/shared/suggestions.zod.ts @@ -52,9 +52,28 @@ export function levenshteinDistance(a: string, b: string): number { return prev[lb]; } +/** + * Fold away the differences an author is *never* signalling with: letter case + * and the dash/space spellings of an underscore separator. + * + * Applied to BOTH sides of the comparison in {@link findClosestMatches}. Folding + * only the input was a real defect (#4990): candidates are camelCase across most + * of the spec (AGENTS.md Prime Directive #3, "TS config keys → camelCase"), so + * every capital in a declared key charged the author one extra substitution + * against a budget that is only `max(2, len/3)`. The observable symptom was + * inverted quality — `hiddenon` (all-lowercase, plain wrong) resolved to + * `hiddenOn` at distance 1, while `hideOn` (correctly cased, one real typo) + * scored 3 against a budget of 2 and got no suggestion at all. + */ +const foldForScoring = (value: string): string => value.toLowerCase().replace(/[-\s]/g, '_'); + /** * Find the closest matches from a list of candidates. * + * Scoring is case- and separator-insensitive on both sides; the returned + * strings are the candidates' ORIGINAL spelling, because that spelling is what + * the author has to type back. + * * @param input - The user-provided (possibly invalid) value * @param candidates - Array of valid values to compare against * @param maxDistance - Maximum edit distance to consider (default: 3) @@ -67,15 +86,28 @@ export function findClosestMatches( maxDistance = 3, maxResults = 3, ): string[] { - const normalized = input.toLowerCase().replace(/[-\s]/g, '_'); + const normalized = foldForScoring(input); const scored = candidates .map((candidate) => ({ value: candidate, - distance: levenshteinDistance(normalized, candidate), + distance: levenshteinDistance(normalized, foldForScoring(candidate)), + // Tie-break only. Folding is right for RANKING (case is not what the + // author meant to signal), but when two declared keys are equidistant + // under the fold the author's own capitalisation is the last piece of + // evidence left about which one they were reaching for — `yxAis` ties + // `yAxis` and `xAxis` at 2 folded, and the capital A picks the intended + // one. Kept strictly secondary so it can never resurrect the #4990 bug + // of case outranking a real edit. + cased: levenshteinDistance(input, candidate), })) - .filter((s) => s.distance <= maxDistance && s.distance > 0) - .sort((a, b) => a.distance - b.distance); + // Drop only the candidate the author ALREADY typed verbatim — suggesting a + // string back to the author who wrote it is noise. A folded distance of 0 + // on a differently-spelled candidate (`hiddenon` vs `hiddenOn`) is not that + // case: it is the strongest suggestion available, and pre-#4990 the + // `distance > 0` test threw it away together with the true self-match. + .filter((s) => s.distance <= maxDistance && s.value !== input) + .sort((a, b) => a.distance - b.distance || a.cased - b.cased); return scored.slice(0, maxResults).map((s) => s.value); } diff --git a/packages/spec/src/ui/responsive.test.ts b/packages/spec/src/ui/responsive.test.ts index b8e417694f..772163dbfd 100644 --- a/packages/spec/src/ui/responsive.test.ts +++ b/packages/spec/src/ui/responsive.test.ts @@ -190,14 +190,22 @@ describe('unknown keys are rejected, not stripped (#4001 batch 13)', () => { it('reaches `hiddenOn` from both wrong spellings', () => { // `hidden` is objectui's RESOLVED spelling (`useResponsiveConfig` returns - // `{ hidden, columns, order, breakpoint }`). `hideOn` is the same word, - // and the distance fallback measurably cannot reach it — it lowercases - // the input but not the candidates, so the capital in `hiddenOn` costs an - // extra edit against a budget of 2 (filed as #4990). + // `{ hidden, columns, order, breakpoint }`) — a different WORD, which no + // edit distance can reach, so it keeps its entry in the alias table. expect(unknownKeyIssue(ResponsiveConfigSchema, { hidden: true })!.message) .toContain('`hidden` → `hiddenOn`'); + // `hideOn` is the SAME word and had an alias entry of its own until #4990, + // because the fallback charged the author for `hiddenOn`'s capital O: + // `hideOn` scored 3 against a budget of 2 and returned nothing, while the + // all-lowercase `hiddenon` scored 1 and resolved. #4990 folds case on both + // sides, so the alias was retired and this now rides the fallback alone. + // These two assertions ARE batch 13's measurement, kept executable — the + // first fails if the general fix regresses, the second is the comparison + // that made the old behaviour indefensible. expect(unknownKeyIssue(ResponsiveConfigSchema, { hideOn: ['xs'] })!.message) .toContain('`hideOn` → `hiddenOn`'); + expect(unknownKeyIssue(ResponsiveConfigSchema, { hiddenon: ['xs'] })!.message) + .toContain('`hiddenon` → `hiddenOn`'); }); }); diff --git a/packages/spec/src/ui/responsive.zod.ts b/packages/spec/src/ui/responsive.zod.ts index ffe159f882..e83631d492 100644 --- a/packages/spec/src/ui/responsive.zod.ts +++ b/packages/spec/src/ui/responsive.zod.ts @@ -206,16 +206,17 @@ export const ResponsiveConfigSchema = lazySchema(() => strictObject( // `useResponsiveConfig.ts`). `hidden` is that RESULT's spelling of the // authored `hiddenOn`, which is where the wrong word comes from. hidden: 'hiddenOn', - // `hideOn` is not a different word — it is the SAME word, and the - // distance fallback still cannot reach it. Measured, not assumed: - // `findClosestMatches('hideOn', ['hiddenOn'], 2)` returns `[]`, because - // the fallback lowercases the INPUT but not the CANDIDATES, so every - // capital in a declared key costs one extra edit — `hideOn` scores 3 - // against a budget of 2 while the all-lowercase `hiddenon` scores 1 and - // resolves fine. That asymmetry is general to camelCase keys (i.e. to - // most of the spec, per AGENTS.md naming) and is filed as #4990; this - // entry covers the one instance this file owns. - hideOn: 'hiddenOn', + // `hideOn` USED to need an entry here. It is not a different word — it is + // the same word, and the distance fallback could not reach it only + // because of #4990: the fallback lowercased the INPUT but not the + // CANDIDATES, so `hiddenOn`'s capital O cost an extra edit and `hideOn` + // scored 3 against a budget of 2, while the all-lowercase `hiddenon` + // scored 1 and resolved fine. #4990 fixed that at the source by folding + // case on both sides, so this per-case workaround is retired: `hideOn` + // now reaches `hiddenOn` on distance alone. The measurement that + // justified the entry is preserved as an assertion in `responsive.test.ts` + // ("reaches `hiddenOn` from both wrong spellings") rather than as a + // comment, so it fails if the general fix ever regresses. }, guidance: { ...BREAKPOINT_AT_TOP_LEVEL,