From 2e4e54a3651d780a56f0893ca914a2b6432eaba7 Mon Sep 17 00:00:00 2001 From: Andrii Shylenko <14119286+w1ne@users.noreply.github.com> Date: Tue, 25 Aug 2026 22:24:22 +0200 Subject: [PATCH] fix(studio): Validity panel reports the loaded model, not a vacuous green The Validity tab rendered 'solved - 0 parts - 0 joints - 0 diagnostics' for a saved project whose model has 2 parts and 1 revolute joint, while the Scene tab listed both parts on the same page. Two defects stacked. 1. reviewToValidity() hardcoded partCount: 0 and jointCount: 0. Its own header called this a conservative Phase 1.1 adapter pending a deeper server payload; the deeper payload had in fact already landed (reviewPipeline returns a validator block with real counts on both its ok and not-ok branches) but the adapter never read it, so EVERY model reported zero regardless of how it was declared. 2. When a mesh response carries no review block - which is every session-backed load, plus the dev endpoint's live=1 short-circuit - GeometryContext substitutes { ok: true, diagnostics: [] } and deriveStatus turned that ok into 'solved'. A green verdict computed over nothing. Counts now come from the server's validator block when present and from the loaded model's own featureRecords otherwise, so the panel reports what the model declares. Provenance is derived (not flagged) from the payload: a real reviewCadTool result always carries validator, fitness and a mechanism verdict, a review that found something carries diagnostics; a payload with none of those is marked unvalidated and the chip reads 'not run' with an explanatory notice. Only the PASSING verdict is suppressed - a real failure still reports as itself. Joints tab: extractJointSnapshots() only read metadata.mates, so a model built with joint primitives (asm.revolute) produced an empty list, which emptied the tab and greyed it out via getVisibleTabs. Joint primitives capture as their own assemblyJoint records with their pose on the solvedAssembly record's pose map; both vocabularies are now read. --- .../adapters/featureRecordsToCounts.test.ts | 56 +++++ .../adapters/reviewToValidity.test.ts | 36 ++++ .../fixtures/assemblyFeatureRecordFixtures.ts | 113 +++++++++++ .../__tests__/tabs/ValidityTab.test.tsx | 11 +- .../tabs/ValidityTabLoadedModel.test.tsx | 191 ++++++++++++++++++ src/studio/adapters/featureRecordsToCounts.ts | 70 +++++++ src/studio/adapters/featureRecordsToMates.ts | 100 +++++++++ src/studio/adapters/reviewToValidity.ts | 51 ++++- src/studio/context/GeometryContext.tsx | 22 ++ src/studio/hooks/useRecomputeResult.ts | 14 +- src/studio/tabs/ValidityTab.tsx | 47 ++++- src/studio/types.ts | 18 +- 12 files changed, 713 insertions(+), 16 deletions(-) create mode 100644 src/studio/__tests__/adapters/featureRecordsToCounts.test.ts create mode 100644 src/studio/__tests__/fixtures/assemblyFeatureRecordFixtures.ts create mode 100644 src/studio/__tests__/tabs/ValidityTabLoadedModel.test.tsx create mode 100644 src/studio/adapters/featureRecordsToCounts.ts diff --git a/src/studio/__tests__/adapters/featureRecordsToCounts.test.ts b/src/studio/__tests__/adapters/featureRecordsToCounts.test.ts new file mode 100644 index 000000000..a710bdfa5 --- /dev/null +++ b/src/studio/__tests__/adapters/featureRecordsToCounts.test.ts @@ -0,0 +1,56 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2026 Andrii Shylenko and kernelCAD contributors +// KC-06 regression: the Validity panel reported `0 parts · 0 joints` for a +// model the Scene tab was listing, because `reviewToValidity` hardcoded both +// counts. These cover the counts recovered from the loaded model's records +// for BOTH joint vocabularies (joint primitives and mates). +import { describe, expect, it } from 'vitest'; +import type { FeatureRecord } from '../../../shared/intent/featureRecord'; +import { countModelTopology } from '../../adapters/featureRecordsToCounts'; +import { + jointPrimitiveModelRecords, + mateModelRecords, +} from '../fixtures/assemblyFeatureRecordFixtures'; + +describe('countModelTopology', () => { + it('empty / null records → 0 parts, 0 joints', () => { + expect(countModelTopology([])).toEqual({ partCount: 0, jointCount: 0 }); + expect(countModelTopology(null)).toEqual({ partCount: 0, jointCount: 0 }); + }); + + it('joint-primitive model (2 parts, 1 revolute) reports 2 and 1', () => { + expect(countModelTopology(jointPrimitiveModelRecords())).toEqual({ + partCount: 2, + jointCount: 1, + }); + }); + + it('mate-built model (2 parts, 1 revolute mate) reports 2 and 1', () => { + expect(countModelTopology(mateModelRecords())).toEqual({ + partCount: 2, + jointCount: 1, + }); + }); + + it('does not double count a mate repeated across two solvedAssembly records', () => { + const records = mateModelRecords(); + const solved = records.find((r) => r.kind === 'solvedAssembly'); + if (solved === undefined) throw new Error('fixture has no solvedAssembly record'); + const repeated: FeatureRecord[] = [...records, { ...solved, id: 'solved-2' }]; + expect(countModelTopology(repeated).jointCount).toBe(1); + }); + + it('counts same-named parts in two assemblies separately', () => { + const records: FeatureRecord[] = [ + ...jointPrimitiveModelRecords(), + { + id: 'other-base', + kind: 'assemblyPart', + params: {}, + inputs: {}, + metadata: { assemblyName: 'gripper', partName: 'base' }, + }, + ]; + expect(countModelTopology(records).partCount).toBe(3); + }); +}); diff --git a/src/studio/__tests__/adapters/reviewToValidity.test.ts b/src/studio/__tests__/adapters/reviewToValidity.test.ts index 00b655f51..374cfadda 100644 --- a/src/studio/__tests__/adapters/reviewToValidity.test.ts +++ b/src/studio/__tests__/adapters/reviewToValidity.test.ts @@ -42,6 +42,42 @@ describe('reviewToValidity', () => { expect(v?.diagnostics[0].severity).toBe('error'); }); + // KC-06: counts and provenance. + it('a bare ok:true placeholder is NOT marked validated', () => { + // `{ ok: true, diagnostics: [] }` is what GeometryContext substitutes + // for a missing `review` block and what the dev endpoint's `live=1` + // short-circuit returns. Nothing validated; the flag must say so. + expect(reviewToValidity({ ok: true })?.validated).toBe(false); + expect(reviewToValidity({ ok: true, diagnostics: [], live: true })?.validated).toBe(false); + }); + + it('a review carrying validator / fitness / mechanism evidence IS validated', () => { + expect( + reviewToValidity({ ok: true, validator: { partCount: 2, jointCount: 1 } })?.validated, + ).toBe(true); + expect(reviewToValidity({ ok: true, fitness: { functional: true } })?.validated).toBe(true); + expect(reviewToValidity({ ok: true, mechanism: 'real' })?.validated).toBe(true); + expect(reviewToValidity({ ok: true, mechanism: 'unverified' })?.validated).toBe(false); + expect( + reviewToValidity({ ok: false, diagnostics: [{ message: 'x' }] })?.validated, + ).toBe(true); + }); + + it('counts come from the loaded model when the payload has no validator block', () => { + const v = reviewToValidity({ ok: true }, { partCount: 2, jointCount: 1 }); + expect(v?.partCount).toBe(2); + expect(v?.jointCount).toBe(1); + }); + + it("the server's validator counts win over the local model counts", () => { + const v = reviewToValidity( + { ok: true, validator: { partCount: 5, jointCount: 4 } }, + { partCount: 2, jointCount: 1 }, + ); + expect(v?.partCount).toBe(5); + expect(v?.jointCount).toBe(4); + }); + it('mechanism: broken overrides ok:true → status=error', () => { // P1 surface convergence: the loop's mechanism verdict is the // merge gate, so a broken mechanism flips status to error even diff --git a/src/studio/__tests__/fixtures/assemblyFeatureRecordFixtures.ts b/src/studio/__tests__/fixtures/assemblyFeatureRecordFixtures.ts new file mode 100644 index 000000000..634dddba1 --- /dev/null +++ b/src/studio/__tests__/fixtures/assemblyFeatureRecordFixtures.ts @@ -0,0 +1,113 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2026 Andrii Shylenko and kernelCAD contributors +// The KC-06 model, as `featureRecords`: two parts ("base", "arm") joined by +// one revolute joint, in BOTH declaration vocabularies. +// +// `jointPrimitiveModelRecords()` — what `assembly().part(...)` + +// `asm.revolute(...)` + `asm.solvedModel(...)` captures: one +// `assemblyJoint` record per joint, plus the pose map on the +// `solvedAssembly` record. `metadata.mates` is ABSENT — joint primitives +// live in `Assembly.__joints()`, never in `__mates()`. +// +// `mateModelRecords()` — what `asm.mate(...)` captures: no `assemblyJoint` +// record, the joint shows up as `metadata.mates` on `solvedAssembly`. +// +// Shapes mirror `buildAssemblyJointFeatureSpec` / +// `buildSolvedAssemblyFeatureSpec` in +// `src/modeling/capture/assemblyFeatureRecords.ts`. +import type { FeatureRecord } from '../../../shared/intent/featureRecord'; +import type { Param } from '../../../shared/intent/types'; + +function record(partial: Partial & Pick): FeatureRecord { + return { + inputs: {}, + params: {}, + transforms: [], + suppressed: false, + ...partial, + }; +} + +function part(id: string, partName: string, assemblyName = 'rig'): FeatureRecord { + return record({ + id, + kind: 'assemblyPart', + inputs: { shape: { kind: 'feature', id: `${id}-shape` } }, + metadata: { assemblyName, partName }, + }); +} + +/** A pose bound to a param table entry, as capture encodes a ParamRef: + * `evaluated` stays 0 and the real value is looked up by `paramRef`. */ +function paramRefPose(name: string): Param { + return { expression: name, unit: 'deg', evaluated: 0, paramRef: name }; +} + +export function jointPrimitiveModelRecords(): FeatureRecord[] { + return [ + part('p-base', 'base'), + part('p-arm', 'arm'), + record({ + id: 'j-elbow', + kind: 'assemblyJoint', + inputs: { + a: { kind: 'feature', id: 'p-base' }, + b: { kind: 'feature', id: 'p-arm' }, + }, + metadata: { + assemblyName: 'rig', + jointName: 'elbow', + jointKind: 'revolute', + axis: [0, 0, 1], + origin: [0, 0, 10], + limitsDeg: [-90, 90], + }, + }), + record({ + id: 'solved', + kind: 'solvedAssembly', + inputs: { + part_0: { kind: 'feature', id: 'p-base' }, + part_1: { kind: 'feature', id: 'p-arm' }, + joint_0: { kind: 'feature', id: 'j-elbow' }, + }, + metadata: { + assemblyName: 'rig', + partIds: ['p-base', 'p-arm'], + jointIds: ['j-elbow'], + poses: { elbow: { kind: 'scalar', value: paramRefPose('elbowDeg') } }, + }, + }), + ]; +} + +export function mateModelRecords(): FeatureRecord[] { + return [ + part('p-base', 'base'), + part('p-arm', 'arm'), + record({ + id: 'solved', + kind: 'solvedAssembly', + inputs: { + part_0: { kind: 'feature', id: 'p-base' }, + part_1: { kind: 'feature', id: 'p-arm' }, + }, + metadata: { + assemblyName: 'rig', + partIds: ['p-base', 'p-arm'], + jointIds: [], + poses: { elbow: { kind: 'scalar', value: paramRefPose('elbowDeg') } }, + mates: [ + { + name: 'elbow', + a: 'base.top', + b: 'arm.bottom', + type: 'revolute', + pose: { kind: 'scalar', value: paramRefPose('elbowDeg') }, + limitsDeg: [-90, 90], + }, + ], + }, + }), + ]; +} diff --git a/src/studio/__tests__/tabs/ValidityTab.test.tsx b/src/studio/__tests__/tabs/ValidityTab.test.tsx index d1893820e..7b3372419 100644 --- a/src/studio/__tests__/tabs/ValidityTab.test.tsx +++ b/src/studio/__tests__/tabs/ValidityTab.test.tsx @@ -3,7 +3,7 @@ /** @vitest-environment jsdom */ import { cleanup, fireEvent, render, screen } from '@testing-library/react'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import type { StudioRecomputeResult } from '../../types'; +import type { StudioRecomputeResult, StudioValidity } from '../../types'; import type { ValidatorDiagnostic, ValidatorResult } from '../../../modeling/mates/validator'; const mockUseRecomputeResult = vi.fn<() => StudioRecomputeResult>(); @@ -39,17 +39,20 @@ function emptyResult(): StudioRecomputeResult { }; } -function withValidity(v: ValidatorResult): StudioRecomputeResult { +function withValidity(v: StudioValidity): StudioRecomputeResult { return { ...emptyResult(), validity: v }; } +/** `validated: true` — these cases stand for a validation that actually ran, + * which is what makes the verdict paintable. The unvalidated case (the + * KC-06 vacuous green) is covered in `ValidityTabLoadedModel.test.tsx`. */ function makeValidity( status: ValidatorResult['status'], diagnostics: ValidatorDiagnostic[] = [], partCount = 0, jointCount = 0, -): ValidatorResult { - return { status, diagnostics, partCount, jointCount }; +): StudioValidity { + return { status, diagnostics, partCount, jointCount, validated: true }; } afterEach(() => { diff --git a/src/studio/__tests__/tabs/ValidityTabLoadedModel.test.tsx b/src/studio/__tests__/tabs/ValidityTabLoadedModel.test.tsx new file mode 100644 index 000000000..8e60ce571 --- /dev/null +++ b/src/studio/__tests__/tabs/ValidityTabLoadedModel.test.tsx @@ -0,0 +1,191 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2026 Andrii Shylenko and kernelCAD contributors +/** @vitest-environment jsdom */ +// +// KC-06 regression, end to end through the real adapters. +// +// Observed on app.kernelcad.com: opening a saved project whose model has 2 +// parts and 1 revolute joint rendered `solved · 0 parts · 0 joints · 0 +// diagnostics` in the Validity tab while the Scene tab listed both parts. Two +// defects stacked: +// +// 1. `reviewToValidity` hardcoded `partCount: 0, jointCount: 0`, so EVERY +// model reported zero regardless of what it declared. +// 2. When a mesh response carries no `review` block (every session-backed +// load), `GeometryContext` substitutes `{ ok: true, diagnostics: [] }` +// and `deriveStatus` turned that `ok` into a green `solved` — a passing +// verdict computed over nothing. +// +// This test drives the real `countModelTopology` + `reviewToValidity` pair so +// a regression in either one fails here, not just in an adapter unit test. +import { cleanup, render, screen } from '@testing-library/react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { StudioRecomputeResult } from '../../types'; + +const mockUseRecomputeResult = vi.fn<() => StudioRecomputeResult>(); + +vi.mock('../../hooks/useRecomputeResult', () => ({ + useRecomputeResult: () => mockUseRecomputeResult(), +})); + +vi.mock('../../hooks/useFeatureSelection', () => ({ + useFeatureSelection: () => ({ selectedFeatureId: null, selectFeature: vi.fn() }), +})); + +import { ValidityTab } from '../../tabs/ValidityTab'; +import { shellStore } from '../../store/shellStore'; +import { countModelTopology } from '../../adapters/featureRecordsToCounts'; +import { reviewToValidity } from '../../adapters/reviewToValidity'; +import { extractJointSnapshots } from '../../adapters/featureRecordsToMates'; +import { getVisibleTabs } from '../../logic/adaptiveTabs'; +import type { ScriptReviewSummary } from '../../context/GeometryContext'; +import type { FeatureRecord } from '../../../shared/intent/featureRecord'; +import { + jointPrimitiveModelRecords, + mateModelRecords, +} from '../fixtures/assemblyFeatureRecordFixtures'; + +/** Build the shell snapshot the way `useRecomputeResult` does for a given + * set of loaded feature records + review payload. */ +function resultFor( + records: FeatureRecord[], + review: ScriptReviewSummary | null, +): StudioRecomputeResult { + return { + features: records, + geometries: [], + validity: reviewToValidity(review, countModelTopology(records)), + paramTable: null, + diagnostics: [], + recomputeMs: 0, + rawInterferencePairs: [], + joints: extractJointSnapshots(records, null), + mechanismBanner: null, + suggestedRepairPrompt: null, + repairEvidence: null, + }; +} + +/** What the hosted mesh path substitutes when the payload carries no + * `review` block. `ok: true` with nothing behind it. */ +const PLACEHOLDER_REVIEW: ScriptReviewSummary = { ok: true, diagnostics: [] }; + +/** A review that really ran: `reviewPipeline` always returns the validator + * block, a fitness summary and a mechanism verdict. */ +const VALIDATED_REVIEW: ScriptReviewSummary = { + ok: true, + diagnostics: [], + fitness: { functional: true, repairMode: 'none' }, + mechanism: 'real', + validator: { status: 'solved', partCount: 2, jointCount: 1 }, +}; + +afterEach(() => { + cleanup(); + shellStore.reset(); +}); + +beforeEach(() => { + mockUseRecomputeResult.mockReset(); + shellStore.reset(); +}); + +describe('ValidityTab reflects the model actually loaded (KC-06)', () => { + it('joint-primitive model with 2 parts + 1 joint reports 2 and 1, not 0', () => { + mockUseRecomputeResult.mockReturnValue( + resultFor(jointPrimitiveModelRecords(), PLACEHOLDER_REVIEW), + ); + + render(); + + expect(screen.getByTestId('validity-counts').textContent).toBe( + '2 parts · 1 joints · 0 diagnostics', + ); + }); + + it('mate-built model with 2 parts + 1 mate reports 2 and 1, not 0', () => { + mockUseRecomputeResult.mockReturnValue( + resultFor(mateModelRecords(), PLACEHOLDER_REVIEW), + ); + + render(); + + expect(screen.getByTestId('validity-counts').textContent).toBe( + '2 parts · 1 joints · 0 diagnostics', + ); + }); + + it('a model no validation has run for does NOT render a green "solved"', () => { + mockUseRecomputeResult.mockReturnValue( + resultFor(jointPrimitiveModelRecords(), PLACEHOLDER_REVIEW), + ); + + render(); + + const chip = screen.getByTestId('validity-chip'); + expect(chip.textContent).toBe('not run'); + expect(chip.getAttribute('data-color')).not.toBe('green'); + expect(chip.getAttribute('data-status')).not.toBe('solved'); + expect(chip.getAttribute('data-validated')).toBe('false'); + expect(screen.getByTestId('validity-not-run-notice')).toBeTruthy(); + }); + + it('a review that really ran still renders the green passing verdict', () => { + mockUseRecomputeResult.mockReturnValue( + resultFor(jointPrimitiveModelRecords(), VALIDATED_REVIEW), + ); + + render(); + + const chip = screen.getByTestId('validity-chip'); + expect(chip.textContent).toBe('solved'); + expect(chip.getAttribute('data-color')).toBe('green'); + expect(chip.getAttribute('data-validated')).toBe('true'); + expect(screen.queryByTestId('validity-not-run-notice')).toBeNull(); + expect(screen.getByTestId('validity-counts').textContent).toBe( + '2 parts · 1 joints · 0 diagnostics', + ); + }); + + it('a real failure is still reported as itself when unvalidated evidence is absent', () => { + // Guard against over-correcting: only the PASSING verdict may be + // suppressed. An `ok: false` review with an error diagnostic carries + // its own evidence and must keep reading `error`. + mockUseRecomputeResult.mockReturnValue( + resultFor(jointPrimitiveModelRecords(), { + ok: false, + diagnostics: [ + { code: 'assembly.part.floating', severity: 'error', message: 'x', hint: 'y' }, + ], + }), + ); + + render(); + + const chip = screen.getByTestId('validity-chip'); + expect(chip.textContent).toBe('error'); + expect(chip.getAttribute('data-color')).toBe('red'); + }); +}); + +describe('Joints tab enablement (KC-06)', () => { + it('a joint-primitive model surfaces its joint and enables the Joints tab', () => { + const records = jointPrimitiveModelRecords(); + const snapshots = extractJointSnapshots(records, null); + + expect(snapshots).toHaveLength(1); + expect(snapshots[0].mate.name).toBe('elbow'); + expect(snapshots[0].mate.type).toBe('revolute'); + expect(snapshots[0].mate.a.split('.')[0]).toBe('base'); + expect(snapshots[0].mate.b.split('.')[0]).toBe('arm'); + expect(snapshots[0].poseParamNames).toEqual(['elbowDeg']); + + expect(getVisibleTabs(resultFor(records, PLACEHOLDER_REVIEW))).toContain('joints'); + }); + + it('a mate-built model still surfaces its mate', () => { + expect(getVisibleTabs(resultFor(mateModelRecords(), PLACEHOLDER_REVIEW))).toContain( + 'joints', + ); + }); +}); diff --git a/src/studio/adapters/featureRecordsToCounts.ts b/src/studio/adapters/featureRecordsToCounts.ts new file mode 100644 index 000000000..61106848b --- /dev/null +++ b/src/studio/adapters/featureRecordsToCounts.ts @@ -0,0 +1,70 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2026 Andrii Shylenko and kernelCAD contributors +// Adapter: FeatureRecord[] → the part / joint counts of the LOADED model. +// +// The Validity panel used to render `0 parts · 0 joints` for every model +// because `reviewToValidity` hardcoded both counts (its own comment called +// this a "Phase 1.1 conservative adapter" pending a deeper server payload). +// The counts are recoverable client-side from the same `featureRecords` the +// Scene tab already lists, so the panel no longer has to wait on the server. +// +// Two joint vocabularies exist and BOTH are counted: +// - joint primitives — `asm.revolute(...)` / `.prismatic(...)` / `.ball(...)` +// land in `Assembly.__joints()` and capture as `assemblyJoint` records +// (and as `metadata.jointIds` on the `solvedAssembly` record). +// - mates — `asm.mate(...)` lands in `Assembly.__mates()` and capture as +// `metadata.mates` on the `solvedAssembly` / `assemblyModel` record. +// Counting only one of them under-reports a model that uses the other. + +import type { FeatureRecord } from '../../shared/intent/featureRecord'; +import type { EncodedMateRecord } from '../../modeling/capture/captureSession'; + +export interface ModelTopologyCounts { + readonly partCount: number; + readonly jointCount: number; +} + +export const EMPTY_MODEL_TOPOLOGY: ModelTopologyCounts = { partCount: 0, jointCount: 0 }; + +/** + * Count the assembly parts and the joints (primitives + mates) declared by + * the records of the currently loaded model. + * + * Deduped by name so a script that resolves the same assembly twice (two + * `solvedAssembly` records over the same parts) is not double counted — + * that mirrors the last-wins precedence `extractJointSnapshots` applies. + */ +export function countModelTopology( + records: readonly FeatureRecord[] | null | undefined, +): ModelTopologyCounts { + if (records == null || records.length === 0) return EMPTY_MODEL_TOPOLOGY; + + const partKeys = new Set(); + const jointNames = new Set(); + + for (const rec of records) { + if (rec.kind === 'assemblyPart') { + const meta = rec.metadata as { assemblyName?: string; partName?: string } | undefined; + const partName = typeof meta?.partName === 'string' ? meta.partName : null; + // Qualify by assembly so two assemblies with a part of the same + // name still count as two parts. + partKeys.add(partName === null ? rec.id : `${meta?.assemblyName ?? ''}::${partName}`); + continue; + } + if (rec.kind === 'assemblyJoint') { + const meta = rec.metadata as { assemblyName?: string; jointName?: string } | undefined; + const jointName = typeof meta?.jointName === 'string' ? meta.jointName : null; + jointNames.add(jointName === null ? rec.id : `${meta?.assemblyName ?? ''}::${jointName}`); + continue; + } + if (rec.kind !== 'solvedAssembly' && rec.kind !== 'assemblyModel') continue; + const meta = rec.metadata as + | { assemblyName?: string; mates?: readonly EncodedMateRecord[] } + | undefined; + for (const mate of meta?.mates ?? []) { + jointNames.add(`${meta?.assemblyName ?? ''}::${mate.name}`); + } + } + + return { partCount: partKeys.size, jointCount: jointNames.size }; +} diff --git a/src/studio/adapters/featureRecordsToMates.ts b/src/studio/adapters/featureRecordsToMates.ts index cc858d7d2..6e87b2122 100644 --- a/src/studio/adapters/featureRecordsToMates.ts +++ b/src/studio/adapters/featureRecordsToMates.ts @@ -125,6 +125,92 @@ function encodedToSnapshot( }; } +/** `Assembly.solvedModel(poses)` stores its pose map on the `solvedAssembly` + * record keyed by joint OR mate name, in the same encoded shape a mate's own + * `pose` uses. Joint primitives carry no pose of their own, so this is the + * only place a primitive's articulation value lives. Later records win, as + * with mates. */ +function solvedAssemblyPoses( + records: readonly FeatureRecord[], +): Map> { + const out = new Map>(); + for (const rec of records) { + if (rec.kind !== 'solvedAssembly') continue; + const meta = rec.metadata as + | { poses?: Record> } + | undefined; + for (const [name, pose] of Object.entries(meta?.poses ?? {})) { + if (pose === undefined) continue; + out.set(name, pose); + } + } + return out; +} + +/** Joint kinds `assembly.revolute/prismatic/ball/fixed` capture as, mapped to + * the MateType vocabulary the JointsTab row renderer speaks. */ +const JOINT_KIND_TO_MATE_TYPE: Record = { + revolute: 'revolute', + prismatic: 'prismatic', + ball: 'ball', + fixed: 'fastened', +}; + +/** A zero Param, used as the resting pose for a declared joint that + * `solvedModel` gave no pose. `evaluated: 0` with no `paramRef` renders the + * row read-only — truthful: the joint exists, nothing drives it. */ +const REST_POSE_PARAM: Param = { expression: '0', unit: 'deg', evaluated: 0 }; + +function jointPrimitiveToSnapshot( + rec: FeatureRecord, + namesByPartId: ReadonlyMap, + posesByJointName: ReadonlyMap>, + paramTable: ParamTable | null, +): JointPoseSnapshot | null { + const meta = rec.metadata as + | { + jointName?: string; + jointKind?: string; + limitsDeg?: readonly [number, number]; + limitsMm?: readonly [number, number]; + ballLimitsDeg?: readonly [number, number]; + } + | undefined; + const name = meta?.jointName; + if (typeof name !== 'string' || name === '') return null; + const type = JOINT_KIND_TO_MATE_TYPE[meta?.jointKind ?? '']; + // `fixed` has zero articulation — same exclusion the mate path applies to + // `fastened` / `planar` (no pose ⇒ no row). + if (type === undefined || type === 'fastened') return null; + + const partA = refPartName(rec.inputs?.a, namesByPartId); + const partB = refPartName(rec.inputs?.b, namesByPartId); + const limits = { + ...(meta?.limitsDeg !== undefined ? { limitsDeg: meta.limitsDeg } : {}), + ...(meta?.limitsMm !== undefined ? { limitsMm: meta.limitsMm } : {}), + ...(meta?.limitsDeg === undefined && meta?.ballLimitsDeg !== undefined + ? { limitsDeg: meta.ballLimitsDeg } + : {}), + }; + const pose = posesByJointName.get(name) + ?? (type === 'ball' + ? ({ kind: 'ball', value: [REST_POSE_PARAM, REST_POSE_PARAM, REST_POSE_PARAM] } as const) + : ({ kind: 'scalar', value: REST_POSE_PARAM } as const)); + + return encodedToSnapshot( + { name, a: partA, b: partB, type, pose, ...limits }, + paramTable, + ); +} + +function refPartName( + ref: FeatureRecord['inputs'][string] | undefined, + namesByPartId: ReadonlyMap, +): string { + if (ref === undefined || ref.kind !== 'feature') return ''; + return namesByPartId.get(ref.id) ?? ref.id; +} + function partNameById(records: readonly FeatureRecord[]): Map { const out = new Map(); for (const rec of records) { @@ -184,6 +270,7 @@ export function extractJointSnapshots( paramTable: ParamTable | null = null, ): readonly JointPoseSnapshot[] { const namesByPartId = partNameById(records); + const posesByJointName = solvedAssemblyPoses(records); // 1. Collect every posed mate, indexed by name. Walking forward means // later records' entries overwrite earlier ones — exactly the // last-wins precedence the lowerer applies for duplicate mate names. @@ -192,6 +279,19 @@ export function extractJointSnapshots( // declaration order from the FIRST solvedAssembly that introduced // the mate (subsequent overrides update value but not slot). const order: string[] = []; + // 0. Joint PRIMITIVES first, in declaration order. `asm.revolute(...)` / + // `.prismatic(...)` / `.ball(...)` capture as their own `assemblyJoint` + // records and never appear in `metadata.mates` — a model built with + // primitives instead of `.mate()` produced an empty list here, which + // both emptied the Joints tab and (via `getVisibleTabs`) greyed it out. + // Mates declared under the same name still win: they are applied after. + for (const rec of records) { + if (rec.kind !== 'assemblyJoint') continue; + const snap = jointPrimitiveToSnapshot(rec, namesByPartId, posesByJointName, paramTable); + if (snap === null) continue; + if (!byName.has(snap.mate.name)) order.push(snap.mate.name); + byName.set(snap.mate.name, snap); + } for (const rec of records) { if (rec.kind !== 'solvedAssembly' && rec.kind !== 'assemblyModel') continue; const meta = rec.metadata as diff --git a/src/studio/adapters/reviewToValidity.ts b/src/studio/adapters/reviewToValidity.ts index 2308683c3..453828a19 100644 --- a/src/studio/adapters/reviewToValidity.ts +++ b/src/studio/adapters/reviewToValidity.ts @@ -16,9 +16,10 @@ import type { ScriptReviewSummary } from '../context/GeometryContext'; import type { ValidatorDiagnostic, ValidatorDiagnosticCode, - ValidatorResult, ValidatorStatus, } from '../../modeling/mates/validator'; +import type { StudioValidity } from '../types'; +import { EMPTY_MODEL_TOPOLOGY, type ModelTopologyCounts } from './featureRecordsToCounts'; export interface MechanismBannerEntry { code: string; @@ -50,16 +51,58 @@ export function reviewToMechanismBanner( }; } -export function reviewToValidity(review: ScriptReviewSummary | null): ValidatorResult | null { +/** + * Did a validation actually run for this review payload? + * + * Not every `ScriptReviewSummary` the shell holds came from a validator. + * Three code paths hand the Studio a synthetic pass: + * + * - `GeometryContext` falls back to `{ ok: true, diagnostics: [] }` when a + * hosted / dev mesh response carries no `review` block at all. + * - the dev `/__kernelcad/review?live=1` path (also used for the + * session-backed INITIAL load) short-circuits the expensive review and + * returns `{ ok: true, diagnostics: [], live: true }`. + * + * Both satisfy `review.ok`, so `deriveStatus` used to answer `'solved'` — + * a green verdict over an empty set. Rather than trust a flag the server + * could forget to set, evidence is derived: a real `reviewCadTool` result + * always carries the validator block, a fitness summary and a mechanism + * verdict; a review that found something always carries diagnostics. + */ +export function reviewWasValidated(review: ScriptReviewSummary): boolean { + if (review.validator != null) return true; + if (review.fitness !== undefined) return true; + if ((review.diagnostics ?? []).length > 0) return true; + if (review.mechanism !== undefined && review.mechanism !== 'unverified') return true; + return false; +} + +/** + * @param review latest `/__kernelcad/review` (or mesh-embedded) payload. + * @param model part / joint counts of the model actually loaded in the + * shell, derived from `featureRecords`. Used when the review + * payload carries no `validator` block of its own — which is + * every session-backed load. Without it the panel reports + * `0 parts · 0 joints` for a model the Scene tab is listing. + */ +export function reviewToValidity( + review: ScriptReviewSummary | null, + model: ModelTopologyCounts = EMPTY_MODEL_TOPOLOGY, +): StudioValidity | null { if (review == null) return null; const diagnostics: ValidatorDiagnostic[] = (review.diagnostics ?? []).map(diagnosticFromReview); + const validated = reviewWasValidated(review); return { status: deriveStatus(review), diagnostics, - partCount: 0, - jointCount: 0, + // The server-side pipeline already returns real counts on its + // `validator` block; prefer them, and fall back to the loaded + // model's own records when the payload has no validator block. + partCount: review.validator?.partCount ?? model.partCount, + jointCount: review.validator?.jointCount ?? model.jointCount, + validated, }; } diff --git a/src/studio/context/GeometryContext.tsx b/src/studio/context/GeometryContext.tsx index 7b0aa5307..99891524e 100644 --- a/src/studio/context/GeometryContext.tsx +++ b/src/studio/context/GeometryContext.tsx @@ -45,6 +45,22 @@ export interface ScriptReviewSummary { repairMode?: string; blockingReasons?: Array<{ code?: string; message?: string; repairHint?: string }>; }; + /** + * Assembly-validator block. `reviewPipeline` returns this on BOTH its + * ok and not-ok branches with the real counts (`validateAssembly`'s own + * `partCount` / `jointCount`), but the Studio used to drop it and render + * `0 parts · 0 joints` for every model. Absent on the `live=1` + * short-circuit and on the placeholder review synthesised below, which is + * exactly what `reviewWasValidated` keys off. + */ + validator?: { + status?: string; + partCount?: number; + jointCount?: number; + }; + /** Set by the dev review endpoint's `live=1` short-circuit: interference + * channel only, no validator pass. Never a verdict. */ + live?: boolean; suggestedRepairPrompt?: string; /** * Raw pairwise interference results at the script's current/default pose, @@ -985,6 +1001,12 @@ export function GeometryProvider({ children, code }: { children: ReactNode; code } const hostedGeometries = featureMeshesToGeometries(rootVisibleFeatures(payload)); const hostedRecords = (payload.featureRecords as FeatureRecord[]) ?? []; + // Placeholder, NOT a verdict: a mesh response with no + // `review` block means nothing validated this model. + // `reviewWasValidated` recognises the shape (no + // validator / fitness / mechanism / diagnostics) so the + // Validity panel reports "not run" instead of a green + // "solved" over an empty set. const hostedReview = payload.review ?? { ok: true, diagnostics: [] }; const emptyNotice = detectEmptyBuild(hostedGeometries.length, hostedRecords, hostedReview); setGeometries(hostedGeometries); diff --git a/src/studio/hooks/useRecomputeResult.ts b/src/studio/hooks/useRecomputeResult.ts index ed592585d..25b913a96 100644 --- a/src/studio/hooks/useRecomputeResult.ts +++ b/src/studio/hooks/useRecomputeResult.ts @@ -6,6 +6,7 @@ import { reviewToValidity, reviewToMechanismBanner } from '../adapters/reviewToV import { serializedParamsToTable } from '../adapters/serializedParamsToTable'; import { reviewDiagnosticsToCompiler } from '../adapters/reviewDiagnosticsToCompiler'; import { extractJointSnapshots } from '../adapters/featureRecordsToMates'; +import { countModelTopology } from '../adapters/featureRecordsToCounts'; import { fingerprintStudioScript, shellStore } from '../store/shellStore'; import type { ScriptReviewSummary } from '../context/GeometryContext'; import type { StudioRecomputeResult, StudioRepairEvidence } from '../types'; @@ -33,9 +34,18 @@ export function useRecomputeResult(): StudioRecomputeResult { const workbench = useWorkbench(); const lastPublishedReviewRef = useRef(UNPUBLISHED_REVIEW); + // Part / joint counts of the model the shell actually has loaded. The + // review payload only carries its own counts on a full server-side + // review; every session-backed load arrives without one, and the panel + // used to fall back to a hardcoded zero. + const modelCounts = useMemo( + () => countModelTopology(workbench.featureRecords), + [workbench.featureRecords], + ); + const validity = useMemo( - () => reviewToValidity(workbench.scriptReview ?? null), - [workbench.scriptReview], + () => reviewToValidity(workbench.scriptReview ?? null, modelCounts), + [workbench.scriptReview, modelCounts], ); // Physics-loop banner (P1). `null` unless the recompute's mechanism diff --git a/src/studio/tabs/ValidityTab.tsx b/src/studio/tabs/ValidityTab.tsx index c25d8efa6..111ac0574 100644 --- a/src/studio/tabs/ValidityTab.tsx +++ b/src/studio/tabs/ValidityTab.tsx @@ -33,8 +33,14 @@ export function ValidityTab(): JSX.Element { ); } - const { status, diagnostics, partCount, jointCount } = validity; - const color = statusColor(status); + const { status, diagnostics, partCount, jointCount, validated } = validity; + // A review payload with no validator evidence behind it derives to + // `status: 'solved'` from its `ok: true` alone. Painting that green + // would tell the user their mechanism validated when nothing ran, so + // the chip reports the absence instead. Only the PASSING verdict is + // suppressed — a real failure still shows as itself. + const verdict: ValidityVerdict = !validated && isPassingStatus(status) ? 'not run' : status; + const color = verdictColor(verdict); const suggestionCards = buildValiditySuggestions({ validity, mechanismBanner, @@ -65,15 +71,26 @@ export function ValidityTab(): JSX.Element { - {status} + {verdict} {partCount} parts · {jointCount} joints · {diagnostics.length} diagnostics + {!validated && ( +
+ No validation has run for this model yet — the counts above + are what the model declares, not what was checked. Press + Validate to check it. +
+ )} {suggestionCards.length > 0 && (
{suggestionCards.map((card) => ( @@ -558,11 +575,31 @@ function diagnosticTargetKey(d: ValidatorDiagnostic): string { } type StatusColor = { - name: 'green' | 'amber' | 'red'; + name: 'green' | 'amber' | 'red' | 'grey'; bg: string; text: string; }; +/** What the chip actually reports: the validator's status, or the explicit + * absence of one. `'not run'` is a shell-only verdict — it never comes back + * from the validator, it is what the shell says when nothing validated. */ +export type ValidityVerdict = ValidatorStatus | 'not run'; + +/** Statuses that read as a pass. Only these are suppressed when unvalidated; + * a failing verdict is reported as itself either way. */ +// eslint-disable-next-line react-refresh/only-export-components +export function isPassingStatus(status: ValidatorStatus): boolean { + return status === 'solved' || status === 'redundant-ok'; +} + +// eslint-disable-next-line react-refresh/only-export-components +export function verdictColor(verdict: ValidityVerdict): StatusColor { + if (verdict === 'not run') { + return { name: 'grey', bg: 'bg-[#242424]', text: 'text-gray-400' }; + } + return statusColor(verdict); +} + // eslint-disable-next-line react-refresh/only-export-components export function statusColor(status: ValidatorStatus): StatusColor { switch (status) { diff --git a/src/studio/types.ts b/src/studio/types.ts index 34cef86a6..53166058a 100644 --- a/src/studio/types.ts +++ b/src/studio/types.ts @@ -13,6 +13,22 @@ import type { CompilerDiagnostic } from '../shared/diagnostics/diagnostic'; import type { ParamTable } from '../shared/runtime/paramTable'; import type { JointPoseSnapshot } from './adapters/featureRecordsToMates'; +/** + * A `ValidatorResult` plus the shell-only provenance flag the panel needs to + * stay honest. + * + * `validated` is `false` when the review payload the shell is holding carries + * no evidence a validation actually ran — the `{ ok: true, diagnostics: [] }` + * placeholder `GeometryContext` substitutes for a missing `review` block, or + * the `live=1` short-circuit the dev review endpoint returns on a + * session-backed load. Both read as `ok`, so the derived `status` is + * `'solved'`; consumers that paint a verdict MUST check `validated` before + * showing that green, or they publish a pass nothing computed. + */ +export type StudioValidity = ValidatorResult & { + readonly validated: boolean; +}; + export interface StudioRepairEvidence { readonly repairMode: string | null; readonly blockingReasons: ReadonlyArray<{ @@ -51,7 +67,7 @@ export type TabId = export interface StudioRecomputeResult { readonly features: readonly FeatureRecord[]; readonly geometries: readonly GeometryResult[]; - readonly validity: ValidatorResult | null; + readonly validity: StudioValidity | null; readonly paramTable: ParamTable | null; readonly diagnostics: readonly CompilerDiagnostic[]; readonly suggestedRepairPrompt: string | null;