collapse union validation errors and filter out root level annotations

Logende · Logende · commit 9bc84e61f0c1 · 2026-05-20T17:20:04.000+02:00
diff --git a/meta_configurator/src/components/panels/code-editor/filterAnnotations.ts b/meta_configurator/src/components/panels/code-editor/filterAnnotations.ts
@@ -0,0 +1,43 @@
+import type {ErrorObject} from 'ajv';
+
+/**
+ * Returns the input errors with composite parent-level errors (oneOf/anyOf/allOf/etc.)
+ * removed when a more specific child error already exists for the same subtree.
+ *
+ * Each error has an "effective leaf path": for keywords that name a specific missing or
+ * unexpected property (`required`, `additionalProperties`, `unevaluatedProperties`,
+ * `dependentRequired`, `dependencies`, `propertyNames`) it's the instancePath extended
+ * with that property name; for all other keywords it's just the instancePath.
+ *
+ * An error is dropped when its effective leaf path is a strict ancestor of another
+ * error's effective leaf path. This keeps real per-property errors (e.g. "required
+ * property 'name' is missing" at the root) while filtering out composite-keyword
+ * summaries that point at the enclosing object/array.
+ */
+export function filterOutAncestorErrors(errors: ErrorObject[]): ErrorObject[] {
+  const effectivePaths = errors.map(effectiveLeafPath);
+  return errors.filter((_, i) => {
+    const descendantPrefix = effectivePaths[i] + '/';
+    return !effectivePaths.some(
+      (other, j) => j !== i && other.startsWith(descendantPrefix)
+    );
+  });
+}
+
+function effectiveLeafPath(error: ErrorObject): string {
+  const params = (error.params ?? {}) as Record<string, unknown>;
+  const segment =
+    (typeof params.missingProperty === 'string' && params.missingProperty) ||
+    (typeof params.additionalProperty === 'string' && params.additionalProperty) ||
+    (typeof params.unevaluatedProperty === 'string' && params.unevaluatedProperty) ||
+    (typeof params.propertyName === 'string' && params.propertyName) ||
+    undefined;
+  if (segment === undefined) {
+    return error.instancePath;
+  }
+  return error.instancePath + '/' + escapeJsonPointerSegment(segment);
+}
+
+function escapeJsonPointerSegment(segment: string): string {
+  return segment.replace(/~/g, '~0').replace(/\//g, '~1');
+}
diff --git a/meta_configurator/src/data/managedValidation.ts b/meta_configurator/src/data/managedValidation.ts
@@ -9,6 +9,7 @@ import {sizeOf} from '@/utility/sizeOf';
 // Import worker as ESM
 import ValidationWorker from '@/workers/validationWorker?worker';
 import {removeExternalReferences} from '@/schema/removeExternalReferences.ts';
+import {collapseUnionErrors} from '@/schema/collapseUnionErrors';
 
 export class ManagedValidation {
   private worker: Worker;
@@ -23,7 +24,7 @@ export class ManagedValidation {
       if (!resolver) return;
 
       if (type === 'VALIDATION_COMPLETE') {
-        resolver(new ValidationResult(result.errors || []));
+        resolver(new ValidationResult(collapseUnionErrors(result.errors || [])));
       } else if (type === 'VALIDATION_ERROR') {
         console.error('Validation worker error:', error);
         resolver(new ValidationResult([]));
diff --git a/meta_configurator/src/schema/__tests__/collapseUnionErrors.test.ts b/meta_configurator/src/schema/__tests__/collapseUnionErrors.test.ts
@@ -0,0 +1,278 @@
+import {describe, expect, it} from 'vitest';
+import type {ErrorObject} from 'ajv';
+import {collapseUnionErrors} from '@/schema/collapseUnionErrors';
+import {ValidationService} from '@/schema/validationService';
+
+function err(overrides: Partial<ErrorObject>): ErrorObject {
+  return {
+    instancePath: '',
+    schemaPath: '#',
+    keyword: 'type',
+    params: {},
+    message: 'invalid',
+    ...overrides,
+  } as ErrorObject;
+}
+
+describe('collapseUnionErrors', () => {
+  it('collapses a oneOf cluster into a single enriched error', () => {
+    const errors = [
+      err({
+        instancePath: '/a',
+        schemaPath: '#/properties/a/oneOf/0/maximum',
+        keyword: 'maximum',
+        message: 'must be <= 10',
+      }),
+      err({
+        instancePath: '/a',
+        schemaPath: '#/properties/a/oneOf/1/type',
+        keyword: 'type',
+        message: 'must be string',
+      }),
+      err({
+        instancePath: '/a',
+        schemaPath: '#/properties/a/oneOf',
+        keyword: 'oneOf',
+        message: 'must match exactly one schema in oneOf',
+      }),
+    ];
+
+    const result = collapseUnionErrors(errors);
+
+    expect(result).toHaveLength(1);
+    expect(result[0]!.keyword).toBe('oneOf');
+    expect(result[0]!.instancePath).toBe('/a');
+    expect(result[0]!.message).toBe(
+      [
+        'does not match any oneOf variant:',
+        '  • variant 1: must be <= 10',
+        '  • variant 2: must be string',
+      ].join('\n')
+    );
+  });
+
+  it('collapses anyOf clusters the same way', () => {
+    const errors = [
+      err({
+        schemaPath: '#/anyOf/0/type',
+        keyword: 'type',
+        message: 'must be number',
+      }),
+      err({
+        schemaPath: '#/anyOf/1/type',
+        keyword: 'type',
+        message: 'must be boolean',
+      }),
+      err({
+        schemaPath: '#/anyOf',
+        keyword: 'anyOf',
+        message: 'must match a schema in anyOf',
+      }),
+    ];
+
+    const result = collapseUnionErrors(errors);
+
+    expect(result).toHaveLength(1);
+    expect(result[0]!.message).toBe(
+      [
+        'does not match any anyOf variant:',
+        '  • variant 1: must be number',
+        '  • variant 2: must be boolean',
+      ].join('\n')
+    );
+  });
+
+  it('groups multiple sub-errors per branch into one variant line', () => {
+    const errors = [
+      err({
+        schemaPath: '#/oneOf/0/type',
+        keyword: 'type',
+        message: 'must be string',
+      }),
+      err({
+        schemaPath: '#/oneOf/0/minLength',
+        keyword: 'minLength',
+        message: 'must be at least 3 characters',
+      }),
+      err({
+        schemaPath: '#/oneOf',
+        keyword: 'oneOf',
+        message: 'must match exactly one schema in oneOf',
+      }),
+    ];
+
+    const result = collapseUnionErrors(errors);
+
+    expect(result).toHaveLength(1);
+    expect(result[0]!.message).toBe(
+      [
+        'does not match any oneOf variant:',
+        '  • variant 1: must be string; must be at least 3 characters',
+      ].join('\n')
+    );
+  });
+
+  it('handles multiple sibling oneOf clusters independently', () => {
+    const errors = [
+      // cluster at /a
+      err({
+        instancePath: '/a',
+        schemaPath: '#/properties/a/oneOf/0/type',
+        keyword: 'type',
+        message: 'must be string',
+      }),
+      err({
+        instancePath: '/a',
+        schemaPath: '#/properties/a/oneOf',
+        keyword: 'oneOf',
+      }),
+      // cluster at /b
+      err({
+        instancePath: '/b',
+        schemaPath: '#/properties/b/oneOf/0/type',
+        keyword: 'type',
+        message: 'must be number',
+      }),
+      err({
+        instancePath: '/b',
+        schemaPath: '#/properties/b/oneOf',
+        keyword: 'oneOf',
+      }),
+    ];
+
+    const result = collapseUnionErrors(errors);
+
+    expect(result).toHaveLength(2);
+    expect(result[0]!.instancePath).toBe('/a');
+    expect(result[1]!.instancePath).toBe('/b');
+    expect(result[0]!.message).toContain('must be string');
+    expect(result[1]!.message).toContain('must be number');
+  });
+
+  it('preserves the summary unchanged if it has no matching branch sub-errors', () => {
+    const errors = [
+      err({
+        instancePath: '/a',
+        schemaPath: '#/properties/a/oneOf',
+        keyword: 'oneOf',
+        message: 'must match exactly one schema in oneOf',
+      }),
+    ];
+
+    const result = collapseUnionErrors(errors);
+
+    expect(result).toEqual(errors);
+  });
+
+  it('passes unrelated errors through untouched', () => {
+    const errors = [
+      err({instancePath: '/x', keyword: 'type', schemaPath: '#/properties/x/type'}),
+      err({instancePath: '/y', keyword: 'maximum', schemaPath: '#/properties/y/maximum'}),
+    ];
+
+    const result = collapseUnionErrors(errors);
+
+    expect(result).toEqual(errors);
+  });
+
+  it('is idempotent', () => {
+    const errors = [
+      err({
+        instancePath: '/a',
+        schemaPath: '#/properties/a/oneOf/0/maximum',
+        keyword: 'maximum',
+      }),
+      err({
+        instancePath: '/a',
+        schemaPath: '#/properties/a/oneOf/1/type',
+        keyword: 'type',
+      }),
+      err({
+        instancePath: '/a',
+        schemaPath: '#/properties/a/oneOf',
+        keyword: 'oneOf',
+      }),
+    ];
+
+    const once = collapseUnionErrors(errors);
+    const twice = collapseUnionErrors(once);
+
+    expect(twice).toEqual(once);
+  });
+
+  it('returns an empty array unchanged', () => {
+    expect(collapseUnionErrors([])).toEqual([]);
+  });
+
+  it('collapses nested unions (oneOf in oneOf in anyOf) into a single tree-shaped error', () => {
+    const service = new ValidationService({
+      type: 'object',
+      properties: {
+        a: {
+          anyOf: [
+            {
+              oneOf: [
+                {
+                  oneOf: [
+                    {type: 'integer', maximum: 5},
+                    {type: 'string', minLength: 3},
+                  ],
+                },
+                {type: 'boolean'},
+              ],
+            },
+            {type: 'null'},
+          ],
+        },
+      },
+    } as any);
+
+    const raw = service.validate({a: 100}).errors;
+    const collapsed = collapseUnionErrors(raw);
+
+    expect(collapsed).toHaveLength(1);
+    expect(collapsed[0]!.instancePath).toBe('/a');
+    expect(collapsed[0]!.keyword).toBe('anyOf');
+    expect(collapsed[0]!.message).toBe(
+      [
+        'does not match any anyOf variant:',
+        '  • variant 1: does not match any oneOf variant:',
+        '    • variant 1.1: does not match any oneOf variant:',
+        '      • variant 1.1.1: must be <= 5',
+        '      • variant 1.1.2: must be string',
+        '    • variant 1.2: must be boolean',
+        '  • variant 2: must be null',
+      ].join('\n')
+    );
+  });
+
+  // integration: run real AJV against the schema/data from the issue discussion
+  it("produces a single annotation for the user's oneOf example", () => {
+    const service = new ValidationService({
+      type: 'object',
+      properties: {
+        a: {
+          oneOf: [
+            {type: 'integer', maximum: 10},
+            {type: 'string', maxLength: 4},
+          ],
+        },
+      },
+    } as any);
+
+    // ValidationService doesn't currently apply the collapse itself — call it explicitly
+    const raw = service.validate({a: 345}).errors;
+    const collapsed = collapseUnionErrors(raw);
+
+    expect(collapsed).toHaveLength(1);
+    expect(collapsed[0]!.instancePath).toBe('/a');
+    expect(collapsed[0]!.keyword).toBe('oneOf');
+    expect(collapsed[0]!.message).toBe(
+      [
+        'does not match any oneOf variant:',
+        '  • variant 1: must be <= 10',
+        '  • variant 2: must be string',
+      ].join('\n')
+    );
+  });
+});
diff --git a/meta_configurator/src/schema/collapseUnionErrors.ts b/meta_configurator/src/schema/collapseUnionErrors.ts
