From e0c49a6c91a9a8f08fa1785fa9a80b0883db65de Mon Sep 17 00:00:00 2001 From: Andrii Shylenko <14119286+w1ne@users.noreply.github.com> Date: Fri, 10 Jul 2026 01:28:33 +0200 Subject: [PATCH] refactor(capture): extract assembly validation --- .../capture/assemblyCaptureValidation.ts | 162 +++++++++++ src/modeling/capture/captureSession.ts | 81 ++---- .../capture/assemblyCaptureValidation.test.ts | 262 ++++++++++++++++++ 3 files changed, 440 insertions(+), 65 deletions(-) create mode 100644 src/modeling/capture/assemblyCaptureValidation.ts create mode 100644 tests/unit/capture/assemblyCaptureValidation.test.ts diff --git a/src/modeling/capture/assemblyCaptureValidation.ts b/src/modeling/capture/assemblyCaptureValidation.ts new file mode 100644 index 00000000..07ce99f5 --- /dev/null +++ b/src/modeling/capture/assemblyCaptureValidation.ts @@ -0,0 +1,162 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2026 Andrii Shylenko and kernelCAD contributors +import type { FeatureRecord } from '../../shared/intent/featureRecord'; +import type { FeatureId } from '../../shared/intent/types'; +import { + buildAssemblyConnectFeatureSpec, + buildAssemblyExportFeatureSpec, + buildAssemblyJointFeatureSpec, + buildAssemblyModelFeatureSpec, + buildAssemblyPartFeatureSpec, + buildSolvedAssemblyFeatureSpec, + type AssemblyConnectorRefLike, + type AssemblyFeatureSpec, + type AssemblyJointCaptureOpts, + type AssemblyJointKind, + type AssemblyPartCaptureOpts, + type AssemblyPartRefLike, + type SolvedAssemblyJointRef, + type SolvedAssemblyMateMetadata, + type SolvedAssemblyPoseInput, +} from './assemblyFeatureRecords'; + +export function createAssemblyPartCaptureSpec( + records: readonly FeatureRecord[], + assemblyName: string, + partName: string, + shapeId: FeatureId, + opts: AssemblyPartCaptureOpts = {}, +): AssemblyFeatureSpec { + if (!records.some(r => r.id === shapeId)) { + throw new Error(`assembly.part: shape '${shapeId}' is not from this CaptureSession`); + } + return buildAssemblyPartFeatureSpec(assemblyName, partName, shapeId, opts); +} + +export function createAssemblyConnectCaptureSpec( + records: readonly FeatureRecord[], + assemblyName: string, + connectName: string, + a: AssemblyConnectorRefLike, + b: AssemblyConnectorRefLike, +): AssemblyFeatureSpec { + for (const connector of [a, b]) { + requireRecordKind( + records, + connector.partId, + 'assemblyPart', + `assembly.connect: part '${connector.partId}' is not an assembly part in this CaptureSession`, + ); + } + return buildAssemblyConnectFeatureSpec(assemblyName, connectName, a, b); +} + +export function createAssemblyJointCaptureSpec( + records: readonly FeatureRecord[], + assemblyName: string, + jointName: string, + jointKind: AssemblyJointKind, + a: AssemblyPartRefLike, + b: AssemblyPartRefLike, + opts: AssemblyJointCaptureOpts, +): AssemblyFeatureSpec { + for (const part of [a, b]) { + requireRecordKind( + records, + part.id, + 'assemblyPart', + `assembly.${jointKind}: part '${part.id}' is not an assembly part in this CaptureSession`, + ); + } + return buildAssemblyJointFeatureSpec(assemblyName, jointName, jointKind, a, b, opts); +} + +export function createAssemblyModelCaptureSpec( + records: readonly FeatureRecord[], + assemblyName: string, + parts: readonly AssemblyPartRefLike[], + mateMetadata?: SolvedAssemblyMateMetadata, +): AssemblyFeatureSpec { + for (const part of parts) { + requireRecordKind( + records, + part.id, + 'assemblyPart', + `assembly.model: part '${part.id}' is not an assembly part in this CaptureSession`, + ); + } + return buildAssemblyModelFeatureSpec(assemblyName, parts, mateMetadata); +} + +export function createSolvedAssemblyCaptureSpec(args: { + records: readonly FeatureRecord[]; + assemblyName: string; + parts: readonly AssemblyPartRefLike[]; + joints: readonly { id: FeatureId; name: string }[]; + poses: SolvedAssemblyPoseInput; + mateMetadata?: SolvedAssemblyMateMetadata; +}): AssemblyFeatureSpec { + if (args.parts.length === 0) { + throw new Error('assembly.solvedModel requires at least one part'); + } + for (const part of args.parts) { + requireRecordKind( + args.records, + part.id, + 'assemblyPart', + `assembly.solvedModel: part '${part.id}' is not an assembly part in this CaptureSession`, + ); + } + + const solvedJoints: SolvedAssemblyJointRef[] = []; + for (const joint of args.joints) { + const record = requireRecordKind( + args.records, + joint.id, + 'assemblyJoint', + `assembly.solvedModel: joint '${joint.id}' is not an assembly joint in this CaptureSession`, + ); + const metadata = record.metadata as { jointName?: string; jointKind?: AssemblyJointKind }; + solvedJoints.push({ + id: joint.id, + name: metadata.jointName ?? joint.name, + kind: metadata.jointName !== undefined ? metadata.jointKind : undefined, + }); + } + + return buildSolvedAssemblyFeatureSpec({ + assemblyName: args.assemblyName, + parts: args.parts, + joints: solvedJoints, + poses: args.poses, + mateMetadata: args.mateMetadata, + }); +} + +export function createAssemblyExportCaptureSpec( + records: readonly FeatureRecord[], + sceneFeatureId: FeatureId, + op: 'compound' | 'union', +): AssemblyFeatureSpec { + const sourceRecord = records.find(r => r.id === sceneFeatureId); + if (!sourceRecord) { + throw new Error(`assemblyExport: source scene feature '${sceneFeatureId}' is not from this CaptureSession`); + } + if (sourceRecord.kind !== 'solvedAssembly' && sourceRecord.kind !== 'assemblyModel') { + throw new Error(`assemblyExport: source feature '${sceneFeatureId}' is kind '${sourceRecord.kind}'; expected 'solvedAssembly' or 'assemblyModel'.`); + } + return buildAssemblyExportFeatureSpec(sceneFeatureId, op); +} + +function requireRecordKind( + records: readonly FeatureRecord[], + id: FeatureId, + kind: K, + message: string, +): FeatureRecord & { kind: K } { + const record = records.find(r => r.id === id); + if (!record || record.kind !== kind) { + throw new Error(message); + } + return record as FeatureRecord & { kind: K }; +} diff --git a/src/modeling/capture/captureSession.ts b/src/modeling/capture/captureSession.ts index e89a7776..95c7e475 100644 --- a/src/modeling/capture/captureSession.ts +++ b/src/modeling/capture/captureSession.ts @@ -56,14 +56,14 @@ import { type VariableSweepCaptureArgs, } from './surfaceSweepRecords'; import { - buildAssemblyConnectFeatureSpec, - buildAssemblyExportFeatureSpec, - buildAssemblyJointFeatureSpec, - buildAssemblyModelFeatureSpec, - buildAssemblyPartFeatureSpec, - buildSolvedAssemblyFeatureSpec, - type AssemblyJointKind, - type SolvedAssemblyJointRef, + createAssemblyConnectCaptureSpec, + createAssemblyExportCaptureSpec, + createAssemblyJointCaptureSpec, + createAssemblyModelCaptureSpec, + createAssemblyPartCaptureSpec, + createSolvedAssemblyCaptureSpec, +} from './assemblyCaptureValidation'; +import { type SolvedAssemblyMateMetadata, } from './assemblyFeatureRecords'; @@ -659,10 +659,7 @@ export class CaptureSession { placedBy?: AssemblyPartOpts['connect']; } = {}, ): FeatureRecord { - if (!this.records.some(r => r.id === shape.id)) { - throw new Error(`assembly.part: shape '${shape.id}' is not from this CaptureSession`); - } - return this.register(buildAssemblyPartFeatureSpec(assemblyName, partName, shape.id, opts)); + return this.register(createAssemblyPartCaptureSpec(this.records, assemblyName, partName, shape.id, opts)); } assemblyConnect( @@ -671,13 +668,7 @@ export class CaptureSession { a: AssemblyConnectorRef, b: AssemblyConnectorRef, ): FeatureRecord { - for (const connector of [a, b]) { - const record = this.records.find(r => r.id === connector.partId); - if (!record || record.kind !== 'assemblyPart') { - throw new Error(`assembly.connect: part '${connector.partId}' is not an assembly part in this CaptureSession`); - } - } - return this.register(buildAssemblyConnectFeatureSpec(assemblyName, connectName, a, b)); + return this.register(createAssemblyConnectCaptureSpec(this.records, assemblyName, connectName, a, b)); } assemblyJoint( @@ -694,13 +685,7 @@ export class CaptureSession { ballLimitsDeg?: [[number, number], [number, number], [number, number]]; }, ): FeatureRecord { - for (const part of [a, b]) { - const record = this.records.find(r => r.id === part.id); - if (!record || record.kind !== 'assemblyPart') { - throw new Error(`assembly.${jointKind}: part '${part.id}' is not an assembly part in this CaptureSession`); - } - } - return this.register(buildAssemblyJointFeatureSpec(assemblyName, jointName, jointKind, a, b, opts)); + return this.register(createAssemblyJointCaptureSpec(this.records, assemblyName, jointName, jointKind, a, b, opts)); } assemblyModel( @@ -708,13 +693,7 @@ export class CaptureSession { parts: readonly AssemblyPartRef[], mateMetadata?: SolvedAssemblyMateMetadata, ): Shape { - for (const part of parts) { - const record = this.records.find(r => r.id === part.id); - if (!record || record.kind !== 'assemblyPart') { - throw new Error(`assembly.model: part '${part.id}' is not an assembly part in this CaptureSession`); - } - } - return this.createShape(buildAssemblyModelFeatureSpec(assemblyName, parts, mateMetadata)); + return this.createShape(createAssemblyModelCaptureSpec(this.records, assemblyName, parts, mateMetadata)); } /** @@ -739,32 +718,11 @@ export class CaptureSession { poses: Record | [Editable, Editable, Editable]>, mateMetadata?: SolvedAssemblyMateMetadata, ): Shape { - if (parts.length === 0) { - throw new Error('assembly.solvedModel requires at least one part'); - } - for (const part of parts) { - const record = this.records.find(r => r.id === part.id); - if (!record || record.kind !== 'assemblyPart') { - throw new Error(`assembly.solvedModel: part '${part.id}' is not an assembly part in this CaptureSession`); - } - } - const solvedJoints: SolvedAssemblyJointRef[] = []; - for (const joint of joints) { - const record = this.records.find(r => r.id === joint.id); - if (!record || record.kind !== 'assemblyJoint') { - throw new Error(`assembly.solvedModel: joint '${joint.id}' is not an assembly joint in this CaptureSession`); - } - const m = record.metadata as { jointName?: string; jointKind?: AssemblyJointKind }; - solvedJoints.push({ - id: joint.id, - name: m.jointName ?? joint.name, - kind: m.jointName !== undefined ? m.jointKind : undefined, - }); - } - return this.createShape(buildSolvedAssemblyFeatureSpec({ + return this.createShape(createSolvedAssemblyCaptureSpec({ + records: this.records, assemblyName, parts, - joints: solvedJoints, + joints, poses, mateMetadata, })); @@ -786,14 +744,7 @@ export class CaptureSession { * `.fillet()`, `.exportSTL()`, etc. on it. */ assemblyExport(sceneFeatureId: FeatureId, op: 'compound' | 'union'): Shape { - const sourceRecord = this.records.find(r => r.id === sceneFeatureId); - if (!sourceRecord) { - throw new Error(`assemblyExport: source scene feature '${sceneFeatureId}' is not from this CaptureSession`); - } - if (sourceRecord.kind !== 'solvedAssembly' && sourceRecord.kind !== 'assemblyModel') { - throw new Error(`assemblyExport: source feature '${sceneFeatureId}' is kind '${sourceRecord.kind}'; expected 'solvedAssembly' or 'assemblyModel'.`); - } - return this.createShape(buildAssemblyExportFeatureSpec(sceneFeatureId, op)); + return this.createShape(createAssemblyExportCaptureSpec(this.records, sceneFeatureId, op)); } edgeFeature( diff --git a/tests/unit/capture/assemblyCaptureValidation.test.ts b/tests/unit/capture/assemblyCaptureValidation.test.ts new file mode 100644 index 00000000..7174f86d --- /dev/null +++ b/tests/unit/capture/assemblyCaptureValidation.test.ts @@ -0,0 +1,262 @@ +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import ts from 'typescript'; +import { describe, expect, it } from 'vitest'; +import { + createAssemblyConnectCaptureSpec, + createAssemblyExportCaptureSpec, + createAssemblyJointCaptureSpec, + createAssemblyModelCaptureSpec, + createAssemblyPartCaptureSpec, + createSolvedAssemblyCaptureSpec, +} from '../../../src/modeling/capture/assemblyCaptureValidation'; +import { CaptureSession } from '../../../src/modeling/capture/captureSession'; +import { createApi } from '../../../src/modeling/api'; +import type { FeatureRecord } from '../../../src/shared/intent/featureRecord'; + +describe('assembly capture validation', () => { + it('validates assemblyPart session membership and returns the feature spec', () => { + expect(createAssemblyPartCaptureSpec( + [record('box_1', 'box')], + 'arm', + 'base', + 'box_1', + { at: [1, 2, 3] }, + )).toEqual({ + kind: 'assemblyPart', + params: {}, + inputs: { + shape: { kind: 'feature', id: 'box_1' }, + }, + metadata: { + assemblyName: 'arm', + partName: 'base', + at: [1, 2, 3], + }, + }); + + expect(() => createAssemblyPartCaptureSpec([], 'arm', 'base', 'box_1')) + .toThrow("assembly.part: shape 'box_1' is not from this CaptureSession"); + }); + + it('validates part-backed connect, joint, and model captures', () => { + const records = [ + record('assemblyPart_1', 'assemblyPart'), + record('assemblyPart_2', 'assemblyPart'), + record('box_1', 'box'), + ]; + + expect(createAssemblyConnectCaptureSpec(records, 'arm', 'base-link', { + assemblyName: 'arm', + partId: 'assemblyPart_1', + partName: 'base', + connector: 'top', + origin: [0, 0, 10], + worldOrigin: [1, 2, 13], + }, { + assemblyName: 'arm', + partId: 'assemblyPart_2', + partName: 'link', + connector: 'bottom', + origin: [0, 0, 0], + worldOrigin: [1, 2, 13], + axis: [0, 0, 1], + })).toMatchObject({ + kind: 'assemblyConnect', + inputs: { + a: { kind: 'feature', id: 'assemblyPart_1' }, + b: { kind: 'feature', id: 'assemblyPart_2' }, + }, + }); + + expect(createAssemblyJointCaptureSpec( + records, + 'arm', + 'yaw', + 'revolute', + { id: 'assemblyPart_1', name: 'base' }, + { id: 'assemblyPart_2', name: 'link' }, + { origin: [0, 0, 0], axis: [0, 0, 1] }, + )).toMatchObject({ + kind: 'assemblyJoint', + metadata: { + assemblyName: 'arm', + jointName: 'yaw', + jointKind: 'revolute', + }, + }); + + expect(createAssemblyModelCaptureSpec(records, 'arm', [ + { id: 'assemblyPart_1', name: 'base' }, + { id: 'assemblyPart_2', name: 'link' }, + ])).toMatchObject({ + kind: 'assemblyModel', + metadata: { + partIds: ['assemblyPart_1', 'assemblyPart_2'], + }, + }); + + expect(() => createAssemblyConnectCaptureSpec(records, 'arm', 'bad', { + assemblyName: 'arm', + partId: 'box_1', + partName: 'box', + connector: 'top', + origin: [0, 0, 0], + worldOrigin: [0, 0, 0], + }, { + assemblyName: 'arm', + partId: 'assemblyPart_2', + partName: 'link', + connector: 'bottom', + origin: [0, 0, 0], + worldOrigin: [0, 0, 0], + })).toThrow("assembly.connect: part 'box_1' is not an assembly part in this CaptureSession"); + }); + + it('validates solvedAssembly inputs while preserving malformed-joint behavior', () => { + const records = [ + record('assemblyPart_1', 'assemblyPart'), + record('assemblyJoint_1', 'assemblyJoint', { jointName: 'yaw', jointKind: 'revolute' }), + record('assemblyJoint_2', 'assemblyJoint', { jointKind: 'revolute' }), + ]; + + expect(createSolvedAssemblyCaptureSpec({ + records, + assemblyName: 'arm', + parts: [{ id: 'assemblyPart_1', name: 'base' }], + joints: [{ id: 'assemblyJoint_1', name: 'fallback-yaw' }], + poses: { yaw: 30 }, + })).toMatchObject({ + kind: 'solvedAssembly', + inputs: { + part_0: { kind: 'feature', id: 'assemblyPart_1' }, + joint_0: { kind: 'feature', id: 'assemblyJoint_1' }, + }, + metadata: { + jointIds: ['assemblyJoint_1'], + poses: { + yaw: { kind: 'scalar' }, + }, + }, + }); + + expect(() => createSolvedAssemblyCaptureSpec({ + records, + assemblyName: 'arm', + parts: [], + joints: [{ id: 'not_a_joint', name: 'yaw' }], + poses: {}, + })).toThrow('assembly.solvedModel requires at least one part'); + + expect(() => createSolvedAssemblyCaptureSpec({ + records, + assemblyName: 'arm', + parts: [{ id: 'assemblyPart_1', name: 'base' }], + joints: [{ id: 'assemblyJoint_2', name: 'yaw' }], + poses: { yaw: 1 }, + })).toThrow("assembly.solvedModel: joint 'yaw' not declared on assembly 'arm'."); + }); + + it('validates assemblyExport source membership and kind', () => { + const records = [ + record('solvedAssembly_1', 'solvedAssembly'), + record('assemblyModel_1', 'assemblyModel'), + record('box_1', 'box'), + ]; + + expect(createAssemblyExportCaptureSpec(records, 'solvedAssembly_1', 'compound')).toMatchObject({ + kind: 'assemblyExport', + inputs: { + scene: { kind: 'feature', id: 'solvedAssembly_1' }, + }, + }); + expect(createAssemblyExportCaptureSpec(records, 'assemblyModel_1', 'union')).toMatchObject({ + kind: 'assemblyExport', + metadata: { op: 'union' }, + }); + + expect(() => createAssemblyExportCaptureSpec(records, 'missing_1', 'compound')) + .toThrow("assemblyExport: source scene feature 'missing_1' is not from this CaptureSession"); + expect(() => createAssemblyExportCaptureSpec(records, 'box_1', 'compound')) + .toThrow("assemblyExport: source feature 'box_1' is kind 'box'; expected 'solvedAssembly' or 'assemblyModel'."); + }); + + it('leaves record ownership, ids, and paramRefs in CaptureSession', () => { + const session = new CaptureSession(); + const kcad = createApi({ session }); + const yawDeg = kcad.param('yawDeg', 15); + const baseShape = kcad.box(10, 10, 10); + const linkShape = kcad.box(20, 10, 10); + const base = session.assemblyPart('arm', 'base', baseShape); + const link = session.assemblyPart('arm', 'link', linkShape); + const yaw = session.assemblyJoint( + 'arm', + 'yaw', + 'revolute', + { id: base.id, name: 'base' }, + { id: link.id, name: 'link' }, + { origin: [0, 0, 0], axis: [0, 0, 1] }, + ); + + const beforeFailureCount = session.getRecords().length; + expect(() => session.assemblyExport(baseShape.id, 'compound')) + .toThrow(`assemblyExport: source feature '${baseShape.id}' is kind 'box'; expected 'solvedAssembly' or 'assemblyModel'.`); + expect(session.getRecords()).toHaveLength(beforeFailureCount); + + const scene = session.solvedAssembly( + 'arm', + [{ id: base.id, name: 'base' }, { id: link.id, name: 'link' }], + [{ id: yaw.id, name: 'yaw' }], + { yaw: yawDeg }, + ); + + const lastRecord = session.getRecords().at(-1)!; + expect(scene.id).toBe(lastRecord.id); + expect(lastRecord.id).toMatch(/^solvedAssembly_/); + expect((lastRecord.metadata as { paramRefs?: string[] }).paramRefs).toEqual(['yawDeg']); + }); + + it('keeps assembly validation outside CaptureSession', () => { + const sessionSource = readFileSync( + resolve(process.cwd(), 'src/modeling/capture/captureSession.ts'), + 'utf8', + ); + const validationSource = readFileSync( + resolve(process.cwd(), 'src/modeling/capture/assemblyCaptureValidation.ts'), + 'utf8', + ); + + expect(importSpecifiers(sessionSource)).toContain('./assemblyCaptureValidation'); + expect(sessionSource).not.toContain('assembly.connect: part'); + expect(sessionSource).not.toContain('assembly.solvedModel: joint'); + expect(sessionSource).not.toContain('assemblyExport: source'); + expect(validationSource).toContain("assembly.solvedModel: joint '"); + }); +}); + +function record(id: string, kind: FeatureRecord['kind'], metadata?: Record): FeatureRecord { + return { + id, + kind, + params: {}, + inputs: {}, + transforms: [], + suppressed: false, + ...(metadata !== undefined ? { metadata } : {}), + }; +} + +function importSpecifiers(sourceText: string): string[] { + const sourceFile = ts.createSourceFile('source.ts', sourceText, ts.ScriptTarget.Latest, true, ts.ScriptKind.TS); + const imports: string[] = []; + + function visit(node: ts.Node): void { + if (ts.isImportDeclaration(node) && ts.isStringLiteral(node.moduleSpecifier)) { + imports.push(node.moduleSpecifier.text); + } + ts.forEachChild(node, visit); + } + + visit(sourceFile); + return imports; +}