diff --git a/.changeset/lint-suggest-name-consolidation-tier2.md b/.changeset/lint-suggest-name-consolidation-tier2.md new file mode 100644 index 0000000000..2703428de3 --- /dev/null +++ b/.changeset/lint-suggest-name-consolidation-tier2.md @@ -0,0 +1,44 @@ +--- +"@objectstack/lint": patch +--- + +fix(lint): consolidate five more private "Did you mean?" copies onto the shared `suggestName` (#14577, follow-up to #14268/#14575) + +`validate-action-name-refs.ts`, `validate-chart-bindings.ts` and +`validate-searchable-fields.ts` each carried a private `suggest`/`distance` +pair, byte-for-byte re-deriving the edit-distance-only budget that +`object-graph.ts` already exports as `suggestName` (the shared helper +#14268/#14575 consolidated three other rules onto). All three now import +`suggestName` from `./object-graph` and their private copies are deleted. + +`validate-ai-tool-references.ts` and `validate-translation-references.ts` +each carry a one-line pre-pass ahead of the private pair — the `action_` +tool-family prefix, and a snake_case namespace-segment match — that is +rule-local knowledge, not the shared helper's business. Both keep that +pre-pass and now delegate the fallback to `suggestName` instead of a private +Levenshtein copy. + +The shared helper's containment pre-pass (a candidate that contains the +target, or vice versa, scores ahead of any edit-distance match) is now every +one of these five rules' behaviour too, so a hint may now appear where one was +previously absent — it never removes a hint the private copy gave. Per site: + +- `validate-action-name-refs.ts` — `archive` → `archive_completed_deals` + (17 edits, over budget) now gets a hint; unaffected cases unchanged. +- `validate-chart-bindings.ts` — the issue's own headline example, + `amount` → `sum_amount` (4 edits, over the budget of 2) now gets a hint on + a raw-field-instead-of-measure binding. +- `validate-searchable-fields.ts` — `amount` → `sum_amount` (4 edits) now + gets a hint on a stale `searchableFields` entry. +- `validate-ai-tool-references.ts` — the `action_` prefix pre-pass is + unchanged and still wins first; a miss with no prefix match now also + reaches `suggestName`'s containment scan (e.g. `knowledge_base` → + `search_knowledge_base`), where the old private copy gave nothing. +- `validate-translation-references.ts` — the namespace-segment pre-pass is + unchanged and still wins first; a miss with no segment match now also + reaches `suggestName`'s containment scan (e.g. `amount` → + `amountsummary`), where the old private copy gave nothing. + +`object-graph.ts`'s helper is untouched (already ruled by #14268/#14575); +`validate-react-page-props.ts` and `validate-rule-schema-formats.ts` stay out +— both are a different contract on purpose (see #14577's triage). diff --git a/packages/lint/src/validate-action-name-refs.test.ts b/packages/lint/src/validate-action-name-refs.test.ts index be5981b2c9..61cc778fbd 100644 --- a/packages/lint/src/validate-action-name-refs.test.ts +++ b/packages/lint/src/validate-action-name-refs.test.ts @@ -82,6 +82,21 @@ describe('validateActionNameRefs — list view bulk/row actions', () => { expect(findings).toHaveLength(1); expect(findings[0].message).toContain('Did you mean "crm_convert_lead"?'); }); + + // #14577 — this rule used to carry a private Levenshtein-only `suggest`, + // which gave NO hint here: `archive` → `archive_completed_deals` is 17 edits + // apart, far outside the `max(2, floor(len/3))` budget. Now delegating to + // the shared `suggestName` (#14268), the containment pre-pass catches it — + // the same class of drift as the issue's `amount` → `sum_amount` example. + it('offers a did-you-mean via containment where edit distance alone would not', () => { + const findings = validateActionNameRefs({ + objects: [{ name: 'crm_lead', fields: { name: { type: 'text' } } }], + actions: [{ name: 'archive_completed_deals', label: 'Archive', type: 'script' }], + views: [{ name: 'crm_lead', list: { bulkActions: ['archive'] } }], + }); + expect(findings).toHaveLength(1); + expect(findings[0].message).toContain('Did you mean "archive_completed_deals"?'); + }); }); // These fixtures use the REAL page shape. An earlier version of this suite diff --git a/packages/lint/src/validate-action-name-refs.ts b/packages/lint/src/validate-action-name-refs.ts index 5960aae4fd..9b31888d8b 100644 --- a/packages/lint/src/validate-action-name-refs.ts +++ b/packages/lint/src/validate-action-name-refs.ts @@ -42,6 +42,7 @@ * miss; it is called out in the hint rather than guessed at. */ +import { suggestName } from './object-graph.js'; import { walkPageComponents } from './page-walk.js'; export const ACTION_NAME_UNDEFINED = 'action-name-undefined'; @@ -81,37 +82,6 @@ function strList(v: unknown): string[] { return Array.isArray(v) ? v.filter((x): x is string => typeof x === 'string' && x.length > 0) : []; } -function distance(a: string, b: string): number { - const m = a.length; - const n = b.length; - if (m === 0) return n; - if (n === 0) return m; - let prev = Array.from({ length: n + 1 }, (_, j) => j); - for (let i = 1; i <= m; i++) { - const curr = [i, ...new Array(n).fill(0)]; - for (let j = 1; j <= n; j++) { - const cost = a[i - 1] === b[j - 1] ? 0 : 1; - curr[j] = Math.min(curr[j - 1] + 1, prev[j] + 1, prev[j - 1] + cost); - } - prev = curr; - } - return prev[n]; -} - -function suggest(target: string, known: Iterable): string { - let best: string | undefined; - let bestScore = Infinity; - for (const candidate of known) { - const d = distance(target, candidate); - if (d < bestScore) { - bestScore = d; - best = candidate; - } - } - const limit = Math.max(2, Math.floor(target.length / 3)); - return best && bestScore <= limit ? ` Did you mean "${best}"?` : ''; -} - /** Every action name defined in the stack (global + object-embedded). */ function collectActionNames(stack: AnyRec): Set { const names = new Set(); @@ -164,7 +134,7 @@ export function validateActionNameRefs(stack: AnyRec): ActionNameRefFinding[] { `${surface} names action "${name}", which is defined by no action in this stack ` + `(neither \`stack.actions\` nor any object's \`actions\`). The button renders and ` + `does nothing when clicked — a dead affordance the runtime cannot dispatch.` + - suggest(name, known), + suggestName(name, known), hint: `Define an action named "${name}" (in \`stack.actions\` or the object's \`actions\`) ` + `${placement}, remove the reference, or ignore this if the ` + diff --git a/packages/lint/src/validate-ai-tool-references.test.ts b/packages/lint/src/validate-ai-tool-references.test.ts index 25205c93fd..45dd2bf1e4 100644 --- a/packages/lint/src/validate-ai-tool-references.test.ts +++ b/packages/lint/src/validate-ai-tool-references.test.ts @@ -124,6 +124,22 @@ describe('validate-ai-tool-references', () => { expect(findings[0].message).toContain('Did you mean "action_triage_case"?'); }); + // #14577 — the `action_` prefix pre-pass stays rule-local (it is + // knowledge about ADR-0109's materialised family, not something the shared + // helper should know), but a miss now falls through to `suggestName` + // (#14268) instead of a private Levenshtein copy. This case has no + // `action_`-prefixed match at all, so it pins that the fallback still fires + // — via containment, the class the private copy could not reach. + it('falls through to suggestName (containment) when the prefix pre-pass misses', () => { + const stack = { + tools: [{ name: 'search_knowledge_base', label: 'Search KB', description: 'x' }], + skills: [{ name: 's', tools: ['knowledge_base'] }], + }; + const findings = validateAiToolReferences(stack); + expect(findings).toHaveLength(1); + expect(findings[0].message).toContain('Did you mean "search_knowledge_base"?'); + }); + it('resolves trailing-wildcard families against the universe', () => { const withActions = { objects: [{ name: 'crm_case', actions: [exposed('triage_case', 'flow')] }], diff --git a/packages/lint/src/validate-ai-tool-references.ts b/packages/lint/src/validate-ai-tool-references.ts index a5d4ec5445..f9f41ff9de 100644 --- a/packages/lint/src/validate-ai-tool-references.ts +++ b/packages/lint/src/validate-ai-tool-references.ts @@ -34,6 +34,8 @@ import { PLATFORM_PROVIDED_TOOL_NAMES, PLATFORM_TOOL_FAMILY_PREFIXES } from '@objectstack/spec/system'; +import { suggestName } from './object-graph.js'; + export const AI_SKILL_TOOL_UNRESOLVED = 'ai-skill-tool-unresolved'; export type AiToolRefSeverity = 'error' | 'warning'; @@ -67,43 +69,17 @@ function strName(v: unknown): string | undefined { return typeof v === 'string' && v.length > 0 ? v : undefined; } -function distance(a: string, b: string): number { - const m = a.length; - const n = b.length; - if (m === 0) return n; - if (n === 0) return m; - let prev = Array.from({ length: n + 1 }, (_, j) => j); - for (let i = 1; i <= m; i++) { - const curr = [i, ...new Array(n).fill(0)]; - for (let j = 1; j <= n; j++) { - const cost = a[i - 1] === b[j - 1] ? 0 : 1; - curr[j] = Math.min(curr[j - 1] + 1, prev[j] + 1, prev[j - 1] + cost); - } - prev = curr; - } - return prev[n]; -} - function suggest(target: string, known: Set): string { // The high-frequency near-miss first: naming the raw ACTION where the // materialised TOOL (`action_`) is meant. Edit distance cannot catch // it (the prefix alone is 7 edits), and it is exactly the mistake the // ADR-0109 default path invites from authors who know their action names. + // Rule-local knowledge (the tool-family prefixes), not the shared helper's + // business — it stays here and wraps `suggestName` for everything else. for (const prefix of PLATFORM_TOOL_FAMILY_PREFIXES) { if (known.has(`${prefix}${target}`)) return ` Did you mean "${prefix}${target}"?`; } - - let best: string | undefined; - let bestScore = Infinity; - for (const candidate of known) { - const d = distance(target, candidate); - if (d < bestScore) { - bestScore = d; - best = candidate; - } - } - const limit = Math.max(2, Math.floor(target.length / 3)); - return best && bestScore <= limit ? ` Did you mean "${best}"?` : ''; + return suggestName(target, known); } /** diff --git a/packages/lint/src/validate-chart-bindings.test.ts b/packages/lint/src/validate-chart-bindings.test.ts index 8b5d88bb45..c2f1abdcb6 100644 --- a/packages/lint/src/validate-chart-bindings.test.ts +++ b/packages/lint/src/validate-chart-bindings.test.ts @@ -47,6 +47,36 @@ describe('validateChartBindings — report charts', () => { expect(findings[0].hint).toContain('est_hours'); }); + // #14577 — this rule used to carry a private Levenshtein-only `suggest`, + // which gave NO hint for the issue's own headline example: `amount` → + // `sum_amount` is 4 edits, over the `max(2, floor(len/3))` budget of 2. Now + // delegating to the shared `suggestName` (#14268), the containment pre-pass + // catches it — a dataset measure name containing the raw base-column name is + // exactly the ADR-0021 cutover drift this rule exists to catch. + it('offers a did-you-mean via containment for the base-column → measure-name drift', () => { + const findings = validateChartBindings({ + datasets: [ + { + name: 'sales_metrics', + object: 'crm_opportunity', + dimensions: [{ name: 'stage', field: 'stage' }], + measures: [{ name: 'sum_amount', aggregate: 'sum', field: 'amount' }], + }, + ], + reports: [ + { + name: 'r', + dataset: 'sales_metrics', + values: ['sum_amount'], + chart: { type: 'bar', xAxis: 'stage', yAxis: 'amount' }, + }, + ], + }); + expect(findings).toHaveLength(1); + expect(findings[0].rule).toBe(CHART_MEASURE_UNKNOWN); + expect(findings[0].hint).toContain('Did you mean "sum_amount"?'); + }); + // The dashboard rule's `Array.isArray(yAxis)` guard would skip this shape. it('handles the report string yAxis, not just the array form', () => { const clean = validateChartBindings({ diff --git a/packages/lint/src/validate-chart-bindings.ts b/packages/lint/src/validate-chart-bindings.ts index 5b893128b9..ad2ec5f90d 100644 --- a/packages/lint/src/validate-chart-bindings.ts +++ b/packages/lint/src/validate-chart-bindings.ts @@ -58,6 +58,7 @@ export interface ChartBindingFinding { hint: string; } +import { suggestName } from './object-graph.js'; import { walkPageComponents, type AnyRec } from './page-walk.js'; function asArray(v: unknown): AnyRec[] { @@ -80,37 +81,6 @@ function isRec(v: unknown): v is AnyRec { return !!v && typeof v === 'object' && !Array.isArray(v); } -function distance(a: string, b: string): number { - const m = a.length; - const n = b.length; - if (m === 0) return n; - if (n === 0) return m; - let prev = Array.from({ length: n + 1 }, (_, j) => j); - for (let i = 1; i <= m; i++) { - const curr = [i, ...new Array(n).fill(0)]; - for (let j = 1; j <= n; j++) { - const cost = a[i - 1] === b[j - 1] ? 0 : 1; - curr[j] = Math.min(curr[j - 1] + 1, prev[j] + 1, prev[j - 1] + cost); - } - prev = curr; - } - return prev[n]; -} - -function suggest(target: string, known: Iterable): string { - let best: string | undefined; - let bestScore = Infinity; - for (const c of known) { - const d = distance(target, c); - if (d < bestScore) { - bestScore = d; - best = c; - } - } - const limit = Math.max(2, Math.floor(target.length / 3)); - return best && bestScore <= limit ? ` Did you mean "${best}"?` : ''; -} - function list(names: Iterable): string { const all = [...names].sort(); return all.length ? all.join(', ') : '(none)'; @@ -185,7 +155,7 @@ export function validateChartBindings(stack: AnyRec): ChartBindingFinding[] { `binds dataset "${dsName}", which resolves to no declared dataset — ` + `the chart has no data to render.`, hint: - `Declared datasets: ${list(datasets.keys())}.${suggest(dsName, datasets.keys())} ` + + `Declared datasets: ${list(datasets.keys())}.${suggestName(dsName, datasets.keys())} ` + `Define it with defineDataset() or fix the reference (ADR-0021).`, }); return; @@ -203,7 +173,7 @@ export function validateChartBindings(stack: AnyRec): ChartBindingFinding[] { `Post-ADR-0021 result rows are keyed by DIMENSION NAME, not the base ` + `field, so this axis renders with no categories.`, hint: - `Dataset dimensions: ${list(ds.dimensions)}.${suggest(name, ds.dimensions)} ` + + `Dataset dimensions: ${list(ds.dimensions)}.${suggestName(name, ds.dimensions)} ` + `Declare the dimension on the dataset, or bind an existing one.`, }); }; @@ -220,7 +190,7 @@ export function validateChartBindings(stack: AnyRec): ChartBindingFinding[] { `Post-ADR-0021 result rows are keyed by MEASURE NAME (e.g. "sum_amount"), ` + `not the base field (e.g. "amount"), so this series comes back empty.`, hint: - `Dataset measures: ${list(ds.measures)}.${suggest(name, ds.measures)} ` + + `Dataset measures: ${list(ds.measures)}.${suggestName(name, ds.measures)} ` + `Declare the measure on the dataset, or bind an existing one.`, }); return; diff --git a/packages/lint/src/validate-searchable-fields.test.ts b/packages/lint/src/validate-searchable-fields.test.ts index 76ce996794..c5554ba328 100644 --- a/packages/lint/src/validate-searchable-fields.test.ts +++ b/packages/lint/src/validate-searchable-fields.test.ts @@ -91,6 +91,25 @@ describe('validateSearchableFields — object declaration', () => { expect(findings[0].message).toContain('Did you mean "billing_email"?'); }); + // #14577 — this rule used to carry a private Levenshtein-only `suggest`, + // which gave NO hint here: `amount` → `sum_amount` is 4 edits, over the + // `max(2, floor(len/3))` budget of 2 — the issue's own headline example. Now + // delegating to the shared `suggestName` (#14268), the containment pre-pass + // catches it. + it('offers a did-you-mean via containment where edit distance alone would not', () => { + const findings = validateSearchableFields({ + objects: [ + { + name: 'crm_opportunity', + fields: { sum_amount: { type: 'number' } }, + searchableFields: ['amount'], + }, + ], + }); + + expect(findings[0].message).toContain('Did you mean "sum_amount"?'); + }); + it('reports every stale entry, not just the first', () => { const findings = validateSearchableFields({ objects: [ diff --git a/packages/lint/src/validate-searchable-fields.ts b/packages/lint/src/validate-searchable-fields.ts index 1aff830e67..fa1f04e518 100644 --- a/packages/lint/src/validate-searchable-fields.ts +++ b/packages/lint/src/validate-searchable-fields.ts @@ -127,6 +127,7 @@ import { SEARCH_AUTO_EXCLUDED_FIELDS, type SearchFieldMeta, } from '@objectstack/spec/data'; +import { suggestName } from './object-graph.js'; import { SYSTEM_FIELDS, indexUnprovisionedAnchors, @@ -271,38 +272,6 @@ function resolveAllowedSet(target: ObjectSearchTarget): { return { allowed: new Set(allowed), source, declaredList: allowed }; } -/** Levenshtein-bounded "did you mean?" over the object's own field names. */ -function suggest(target: string, known: Iterable): string { - let best: string | undefined; - let bestScore = Infinity; - for (const candidate of known) { - const d = distance(target, candidate); - if (d < bestScore) { - bestScore = d; - best = candidate; - } - } - const limit = Math.max(2, Math.floor(target.length / 3)); - return best && bestScore <= limit ? ` Did you mean "${best}"?` : ''; -} - -function distance(a: string, b: string): number { - const m = a.length; - const n = b.length; - if (m === 0) return n; - if (n === 0) return m; - let prev = Array.from({ length: n + 1 }, (_, j) => j); - for (let i = 1; i <= m; i++) { - const curr = [i, ...new Array(n).fill(0)]; - for (let j = 1; j <= n; j++) { - const cost = a[i - 1] === b[j - 1] ? 0 : 1; - curr[j] = Math.min(curr[j - 1] + 1, prev[j] + 1, prev[j - 1] + cost); - } - prev = curr; - } - return prev[n]; -} - /** * object name → the search-target slice. `null` marks an object with no * readable field map, so "declared nothing" stays distinguishable from "not in @@ -384,7 +353,7 @@ export function checkSearchableFieldList( `The declaration is stale: searching it can never match, and the engine ` + `silently drops it — leaving a narrower search than declared, or the ` + `auto-default set once every entry is dropped.` + - (dotted ? '' : suggest(name, known)), + (dotted ? '' : suggestName(name, known)), hint: (dotted ? `'search' scans this object's own columns, so a related record's ` + diff --git a/packages/lint/src/validate-translation-references.test.ts b/packages/lint/src/validate-translation-references.test.ts index ca3d74496e..f07b74c2be 100644 --- a/packages/lint/src/validate-translation-references.test.ts +++ b/packages/lint/src/validate-translation-references.test.ts @@ -299,6 +299,22 @@ describe('validateTranslationReferences — cross-package objects (§4 ladder)', expect(findings[0].path).toBe('translations[0]["zh-CN"].objects.task'); expect(findings[0].message).toContain('Did you mean "todo_task"?'); }); + + // #14577 — the namespace-segment pre-pass stays rule-local (it is knowledge + // about how this stack prefixes object names, not the shared helper's + // business), but a miss now falls through to `suggestName` (#14268) instead + // of a private Levenshtein copy. "amountsummary" is not a `_`-segment match + // for "amount" (no underscore boundary), so the segment pre-pass misses; + // `suggestName`'s containment scan still catches it — 7 edits apart, far + // outside the `max(2, floor(len/3))` budget the private copy was bound by. + it('falls through to suggestName (containment) when the namespace-segment pre-pass misses', () => { + const findings = validateTranslationReferences({ + objects: [{ name: 'amountsummary', fields: { total: { type: 'number' } } }], + translations: bundleFor('amount'), + }); + expect(findings).toHaveLength(1); + expect(findings[0].message).toContain('Did you mean "amountsummary"?'); + }); }); describe('validateTranslationReferences — apps, dashboards, global actions', () => { diff --git a/packages/lint/src/validate-translation-references.ts b/packages/lint/src/validate-translation-references.ts index 7f3776ab36..91ca09c74f 100644 --- a/packages/lint/src/validate-translation-references.ts +++ b/packages/lint/src/validate-translation-references.ts @@ -102,6 +102,7 @@ import { expandViewContainer } from '@objectstack/spec'; import { hasPlatformObjectPrefix, isPlatformProvidedObjectName } from '@objectstack/spec/system'; import { walkFlowNodes } from './flow-walk.js'; +import { suggestName } from './object-graph.js'; import { walkPageComponents } from './page-walk.js'; import { SYSTEM_FIELDS } from './system-fields.js'; import { viewObjectName } from './view-walk.js'; @@ -156,32 +157,18 @@ function strName(v: unknown): string | undefined { return typeof v === 'string' && v.length > 0 ? v : undefined; } -function distance(a: string, b: string): number { - const m = a.length; - const n = b.length; - if (m === 0) return n; - if (n === 0) return m; - let prev = Array.from({ length: n + 1 }, (_, j) => j); - for (let i = 1; i <= m; i++) { - const curr = [i, ...new Array(n).fill(0)]; - for (let j = 1; j <= n; j++) { - const cost = a[i - 1] === b[j - 1] ? 0 : 1; - curr[j] = Math.min(curr[j - 1] + 1, prev[j] + 1, prev[j - 1] + cost); - } - prev = curr; - } - return prev[n]; -} - /** - * "Did you mean?" over the known names — Levenshtein-bounded, plus a namespace - * pass the distance metric cannot see. + * "Did you mean?" over the known names — a namespace pass the shared helper + * cannot see, falling back to `suggestName`'s containment/edit-distance + * budget. * * A stack prefixes its object names (`todo_task`, `crm_lead`), and the orphan * key is routinely the bare noun the author had in mind (`task`). That is 5 * edits from `todo_task` — far outside the typo bound, and exactly the case * where the suggestion is most useful — so a candidate that differs only by a - * snake_case namespace segment is offered before falling back to edit distance. + * snake_case namespace segment is offered before falling back to the shared + * helper. Rule-local knowledge (the namespace-segment shape), not the shared + * helper's business. */ function suggest(target: string, known: Iterable): string { const names = [...known]; @@ -189,18 +176,7 @@ function suggest(target: string, known: Iterable): string { (candidate) => candidate.endsWith(`_${target}`) || candidate.startsWith(`${target}_`), ); if (segmentMatch) return ` Did you mean "${segmentMatch}"?`; - - let best: string | undefined; - let bestScore = Infinity; - for (const candidate of names) { - const d = distance(target, candidate); - if (d < bestScore) { - bestScore = d; - best = candidate; - } - } - const limit = Math.max(2, Math.floor(target.length / 3)); - return best && bestScore <= limit ? ` Did you mean "${best}"?` : ''; + return suggestName(target, names); } /** At most `max` names, sorted, for the "known values are …" tail of a hint. */