fix(review): make review_cad's fitness and mechanism verdicts agree on joint-primitive assemblies

src/agent/review/reviewPipeline.ts

@@ -251,6 +251,7 @@ export async function runReviewPipeline(input: ReviewCadInput): Promise<ReviewCa
     diagnostics,
     input,
     mechanism,
+    mechanismFailures,
     mechanicalReview,
     physicalUseCases,
     poseEnvelope,
@@ -533,6 +534,7 @@ async function runFitnessAndRepairStage(input: {
   diagnostics: readonly ReviewDiagnostic[];
   input: ReviewCadInput;
   mechanism: MechanismVerdict;
+  mechanismFailures: readonly CompilerDiagnostic[];
   mechanicalReview: Awaited<ReturnType<typeof runMechanicalReviewStage>>;
   physicalUseCases: Awaited<ReturnType<typeof runPhysicalUseCaseStage>>;
   poseEnvelope: PoseEnvelopeReviewResult | undefined;
@@ -543,6 +545,7 @@ async function runFitnessAndRepairStage(input: {
     mechanicalIntentDiagnostics: input.mechanicalReview.mechanicalIntent.diagnostics,
     mechanicalTransmissionDiagnostics: input.mechanicalReview.mechanicalTransmission.diagnostics,
     jointTopologyDiagnostics: input.mechanicalReview.jointTopology.diagnostics,
+    mechanismTruthDiagnostics: input.mechanismFailures,
     physicalUseCaseDiagnostics: input.physicalUseCases.diagnostics,
     physicalUseCaseCount: input.physicalUseCases.checkedUseCaseCount,
     poseEnvelope: input.poseEnvelope,
@@ -552,6 +555,15 @@ async function runFitnessAndRepairStage(input: {
   // the loop's truth criterion is the merge gate, not the legacy
   // advisory aggregate. Spec §"the recompute is what defines the
   // passing state".
+  //
+  // KC-04: `ok` alone was never enough. The truth failures are now also fed
+  // INTO `summarizeMechanismFitness` above, so `fitness.functional`,
+  // `fitness.repairMode` and `fitness.repairDirective` move with the verdict
+  // instead of announcing "No repair needed. Preserve the current topology"
+  // in the same response that reports `mechanism: 'broken'`. `mechanism ===
+  // 'broken'` iff at least one failure has severity 'error', which is
+  // exactly the set folded in — so this conjunction is now a redundant
+  // backstop, deliberately kept.
   const ok = fitness.functional && input.mechanism !== 'broken';
   return {
     fitness,

src/modeling/mates/mechanismFitness.ts

@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: MIT
 // Copyright (c) 2026 Andrii Shylenko and kernelCAD contributors
+import type { CompilerDiagnostic } from '../../shared/diagnostics/diagnostic';
 import type { PoseEnvelopeReviewResult } from './poseEnvelope';
 import type { ValidatorDiagnostic } from './validator';
 import type { MechanicalPlausibilityDiagnostic } from './mechanicalPlausibility';
@@ -52,6 +53,18 @@ export interface MechanismFitnessInput {
   readonly mechanicalIntentDiagnostics?: readonly MechanicalIntentDiagnostic[];
   readonly mechanicalTransmissionDiagnostics?: readonly MechanicalTransmissionDiagnostic[];
   readonly jointTopologyDiagnostics?: readonly JointTopologyDiagnostic[];
+  /**
+   * KC-04 — the mechanism-truth criteria (`checkMechanismTruth`). Folding
+   * these in is what makes the "`fitness.functional: true` + `repairMode:
+   * 'none'` + 'No repair needed' ALONGSIDE `mechanism: 'broken'`"
+   * self-contradiction structurally impossible: a definitive (error-severity)
+   * truth failure is now a blocking reason like any other, so `functional`
+   * and the mechanism verdict can no longer disagree in one response.
+   * Non-error entries (e.g. the `mechanism.sweep-budget-exceeded` 'warn'
+   * carrier, which yields `'unverified'`, not `'broken'`) are NOT folded —
+   * "couldn't verify" must not read as "broken".
+   */
+  readonly mechanismTruthDiagnostics?: readonly CompilerDiagnostic[];
   readonly physicalUseCaseDiagnostics?: readonly PhysicalUseCaseDiagnostic[];
   readonly physicalUseCaseCount?: number;
   readonly poseEnvelope?: PoseEnvelopeReviewResult;
@@ -75,6 +88,7 @@ export function summarizeMechanismFitness(
   const mechanicalIntentDiagnostics = input.mechanicalIntentDiagnostics ?? [];
   const mechanicalTransmissionDiagnostics = input.mechanicalTransmissionDiagnostics ?? [];
   const jointTopologyDiagnostics = input.jointTopologyDiagnostics ?? [];
+  const mechanismTruthDiagnostics = input.mechanismTruthDiagnostics ?? [];
   const physicalUseCaseDiagnostics = input.physicalUseCaseDiagnostics ?? [];
   const physicalUseCaseCount = input.physicalUseCaseCount ?? 0;
   const poseEnvelope = input.poseEnvelope;
@@ -153,6 +167,18 @@ export function summarizeMechanismFitness(
     );
   }
 
+  // KC-04 — a definitive mechanism-truth failure blocks. See the field doc
+  // on `MechanismFitnessInput.mechanismTruthDiagnostics`.
+  for (const diagnostic of mechanismTruthDiagnostics) {
+    if (diagnostic.severity !== 'error') continue;
+    addBlockingReason(
+      diagnostic.code,
+      diagnostic.message,
+      diagnostic.hint,
+      diagnostic,
+    );
+  }
+
   if (physicalUseCaseCount > 0 && physicalUseCaseDiagnostics.length === 0) {
     passedChecks.push(PASSED_CHECKS.physicalUseCaseDeclared);
   }
@@ -284,7 +310,10 @@ function chooseRepairMode(
 
   if (blockingReasons.some((reason) =>
     reason.code.startsWith('assembly.connectivity.') ||
-    reason.code.startsWith('assembly.joint-topology.')
+    reason.code.startsWith('assembly.joint-topology.') ||
+    // KC-04: an unreachable part is a graph-topology defect — nudging local
+    // coordinates cannot connect it.
+    reason.code === 'mechanism.orphan-part'
   )) {
     return 'topology-redesign';
   }

src/modeling/mates/validator.ts

@@ -459,22 +459,31 @@ export async function validateAssemblyWithMates(
   //     Pure + cheap (no lowering), so it is safe on the early path.
   diagnostics.push(...validateJointConventionMix(arm));
 
-  // 3. Run the v0.6 mate solver. If there are no mates declared, skip — the
-  //    solver returns 'solved' on an empty mate set anyway, but the early
-  //    exit keeps `validateAssemblyWithMates` cheap for v0.5-only scenes
+  // 3. Run the v0.6 mate solver. If there are no mates declared, skip it —
+  //    the solver returns 'solved' on an empty mate set anyway, and skipping
+  //    keeps `validateAssemblyWithMates` cheap for v0.5-only scenes
   //    (regression check: legacy `arm.fixed` callers see identical output).
-  if (arm.__mates().length === 0) {
-    // No mates means no envelope to fold and no articulated mates to
-    // check for limits — but be defensive and still fold the envelope
-    // diagnostics if a caller hands us a result for an empty assembly.
-    foldEnvelopeDiagnostics(diagnostics, poseEnvelopeResult);
-    return finalizeResult(diagnostics, arm.__parts().length, arm.__joints().length, null);
-  }
-
-  const solveResult = await solveMates(arm);
-
-  // 4. Translate SolveStatus → v0.6 diagnostics.
-  switch (solveResult.status) {
+  //
+  //    KC-04: this used to be an EARLY RETURN, which silently disabled every
+  //    gate below it for any assembly built from joint primitives
+  //    (`.revolute()/.prismatic()/.fixed()/.ball()`) — those register in
+  //    `arm.__joints()` and have ZERO mates by construction, so "no mates"
+  //    was being read as "nothing left to validate". It is now a skip of the
+  //    SOLVER ONLY; the gates below run on both conventions. Each mate-driven
+  //    gate already loops `arm.__mates()` and is inert on an empty mate set,
+  //    so the no-mates cost is unchanged — but a gate that does NOT depend on
+  //    mates (today: `validateWorkspaceReachability`, and any future one) now
+  //    actually fires on a joint-primitive assembly instead of being dropped.
+  const hasMates = arm.__mates().length > 0;
+  const solveResult = hasMates ? await solveMates(arm) : null;
+
+  // 4. Translate SolveStatus → v0.6 diagnostics. `null` means the solver was
+  //    skipped (no mates), so there is nothing solver-derived to say — the
+  //    gates below still run.
+  const solveStatus = solveResult === null ? null : solveResult.status;
+  switch (solveStatus) {
+    case null:
+      break;
     case 'solved':
       // Nothing to add.
       break;
@@ -514,12 +523,12 @@ export async function validateAssemblyWithMates(
       diagnostics.push({
         code: 'assembly.solver.did-not-converge',
         severity: 'error',
-        message: `Assembly '${arm.name}' did not converge within the solver iteration cap (${solveResult.iterations ?? 0} iterations).`,
+        message: `Assembly '${arm.name}' did not converge within the solver iteration cap (${solveResult?.iterations ?? 0} iterations).`,
         hint: `invalid-args.assembly.did-not-converge — articulated closed loops are not yet supported by the v0.6.0 solver (lands in T7.x); for v0.6.0, restrict closed loops to fastened-only mates.`,
       });
       break;
     default: {
-      const _exhaustive: never = solveResult.status;
+      const _exhaustive: never = solveStatus;
       throw new Error(`validateAssemblyWithMates: unhandled SolveStatus '${String(_exhaustive)}'.`);
     }
   }
@@ -608,7 +617,7 @@ export async function validateAssemblyWithMates(
     diagnostics,
     arm.__parts().length,
     arm.__joints().length,
-    solveResult.status,
+    solveStatus,
   );
 }
 

src/modeling/runtime/mechanismTruth.test.ts

@@ -606,6 +606,71 @@ describe('mechanism truth — pose-sweep grounded loop (P0)', () => {
     90000,
   );
 
+  // ───────────────────────────────────────────────────────────────────
+  // KC-04 — the reachability walk must see BOTH assembly conventions.
+  //
+  // kernelCAD has two, both correct and both load-bearing:
+  //   - `.mate()` + connectors           → `arm.__mates()`
+  //   - `.revolute()/.prismatic()/...`   → `arm.__joints()` (URDF semantics)
+  // The walk used to read the mate edge list ONLY, so every joint-primitive
+  // mechanism was reported as `mechanism.orphan-part` / 'broken' even when
+  // it was perfectly sound.
+  // ───────────────────────────────────────────────────────────────────
+
+  it('KC-04: a joint-primitive hinge is NOT reported as an orphan and is not broken', async () => {
+    const { arm, kcad } = makeArm('hinge');
+    const base = arm.part('base', kcad.box(60, 40, 10));
+    const armPart = arm.part('arm', kcad.box(50, 10, 8));
+    arm.revolute('elbow', base, armPart, {
+      axis: [0, -1, 0],
+      origin: [0, 0, 10],
+      limitsDeg: [0, 90],
+    });
+
+    const result = await checkMechanismTruth(arm);
+    const orphans = result.failures.filter((f) => f.code === 'mechanism.orphan-part');
+    expect(orphans).toEqual([]);
+    expect(result.mechanism).toBe('real');
+  }, 90000);
+
+  it('KC-04: a prismatic + ball joint chain keeps every part reachable', async () => {
+    // Multi-hop: the walk must traverse joint edges transitively, and every
+    // joint KIND is an edge — not just revolute.
+    const { arm, kcad } = makeArm('slider-chain');
+    const rail = arm.part('rail', kcad.box(100, 20, 10));
+    const carriage = arm.part('carriage', kcad.box(20, 20, 10));
+    const tool = arm.part('tool', kcad.box(10, 10, 10));
+    arm.prismatic('slide', rail, carriage, {
+      axis: [1, 0, 0],
+      origin: [0, 0, 10],
+      limitsMm: [0, 40],
+    });
+    arm.ball('wrist', carriage, tool, { origin: [0, 0, 10] });
+
+    const orphans = (await checkMechanismTruth(arm)).failures
+      .filter((f) => f.code === 'mechanism.orphan-part');
+    expect(orphans).toEqual([]);
+  }, 90000);
+
+  it('KC-04: a genuinely unconnected part is STILL flagged in a joint-primitive assembly', async () => {
+    // The negative side of the fix: teaching the walk about joint edges must
+    // not blind it. A part wired by NEITHER a mate nor a joint is an orphan.
+    const { arm, kcad } = makeArm('hinge-plus-floater');
+    const base = arm.part('base', kcad.box(60, 40, 10));
+    const armPart = arm.part('arm', kcad.box(50, 10, 8));
+    arm.revolute('elbow', base, armPart, {
+      axis: [0, -1, 0],
+      origin: [0, 0, 10],
+      limitsDeg: [0, 90],
+    });
+    arm.part('floater', kcad.box(5, 5, 5).translate(0, 200, 0));
+
+    const orphans = (await checkMechanismTruth(arm)).failures
+      .filter((f) => f.code === 'mechanism.orphan-part');
+    expect(orphans).toHaveLength(1);
+    expect(orphans[0].message).toContain("'floater'");
+  }, 90000);
+
   it('integration: RecomputeEngine.run plumbs the mechanism field via the mechanismCheck callback', async () => {
     // Sanity-check the engine wiring: pass a stub probe and confirm the
     // verdict + failures show up on RecomputeResult.

src/modeling/runtime/mechanismTruth.ts

@@ -370,6 +370,16 @@ function checkOrphanParts(arm: Assembly): CompilerDiagnostic[] {
   if (parts.length <= 1) return [];
 
   // Build adjacency: part-name → set of neighbor part-names.
+  //
+  // KC-04: kernelCAD has TWO assembly conventions and BOTH connect parts —
+  // `.mate()` + connectors (`arm.__mates()`) and the joint primitives
+  // `.revolute()/.prismatic()/.ball()` (`arm.__joints()`). This
+  // walk used to read the mate edge list only, so a perfectly sound
+  // joint-primitive mechanism reported every non-first part as
+  // `mechanism.orphan-part` while `summarizeMechanismFitness` — which reads
+  // the validator/envelope stream, not this walk — reported
+  // `functional: true, repairMode: 'none'` in the SAME review_cad response.
+  // Both cannot be right; joint edges are connections, so they belong here.
   const adj = new Map<string, Set<string>>();
   for (const p of parts) adj.set(p.name, new Set());
   for (const m of mates) {
@@ -379,6 +389,31 @@ function checkOrphanParts(arm: Assembly): CompilerDiagnostic[] {
     adj.get(bPart)?.add(aPart);
   }
 
+  // Joint-primitive edges. Joints address parts by FeatureId, not by name.
+  const nameByPartId = new Map<FeatureId, string>();
+  for (const p of parts) nameByPartId.set(p.id, p.name);
+  for (const j of arm.__joints()) {
+    const aPart = nameByPartId.get(j.parentPartId);
+    const bPart = nameByPartId.get(j.childPartId);
+    if (aPart === undefined || bPart === undefined) continue;
+    adj.get(aPart)?.add(bPart);
+    adj.get(bPart)?.add(aPart);
+  }
+
+  // `arm.part(name, shape, { connect: { to } })` places a part rigidly on a
+  // parent without declaring either a mate or a joint. That is a structural
+  // connection too — the v0.5 validator has always treated it as one
+  // (`validateAssembly`'s floating/orphan pass) — so the truth walk must
+  // agree rather than call the placed part an orphan.
+  for (const p of parts) {
+    const parentName = p.connectParentId === undefined
+      ? undefined
+      : nameByPartId.get(p.connectParentId);
+    if (parentName === undefined) continue;
+    adj.get(p.name)?.add(parentName);
+    adj.get(parentName)?.add(p.name);
+  }
+
   // BFS from parts[0]. Anything unreached is an orphan.
   const root = parts[0].name;
   const visited = new Set<string>();
@@ -399,7 +434,7 @@ function checkOrphanParts(arm: Assembly): CompilerDiagnostic[] {
     if (!visited.has(p.name)) {
       out.push(makeFailure(
         'mechanism.orphan-part',
-        `Part '${p.name}' is not reachable from the mate graph (no mate edge connects it to '${root}' or anything '${root}' reaches).`,
+        `Part '${p.name}' is not reachable from the assembly graph (no mate, joint, or connect edge links it to '${root}' or anything '${root}' reaches).`,
       ));
     }
   }

src/shared/diagnostics/registry.ts

@@ -1205,11 +1205,11 @@ export const DIAGNOSTIC_REGISTRY = {
   },
   'mechanism.orphan-part': {
     hintTemplate:
-      "A part declared via arm.part(...) is unreachable from the mate graph. Add a mate that connects it to another part, or remove the part if it isn't structurally needed.",
-    nextAction: { kind: 'rewrite-feature', guidance: 'add a mate that connects the orphan part to the rest of the mate graph' },
+      "A part declared via arm.part(...) is unreachable from the assembly graph. Connect it to another part — either a mate (arm.mate(...)) or a joint primitive (arm.revolute/.prismatic/.ball) counts — or remove the part if it isn't structurally needed.",
+    nextAction: { kind: 'rewrite-feature', guidance: 'add a mate or joint primitive that connects the orphan part to the rest of the assembly graph' },
     defaultSeverity: 'error',
     group: 'mechanism',
-    description: 'A part declared on the assembly is not reachable from any other part via mate edges — the mate graph is disconnected.',
+    description: 'A part declared on the assembly is not reachable from any other part via mate, joint-primitive, or connect edges — the assembly graph is disconnected.',
   },
   // Physics-grounded loop — T3 slice (post-condition trust gate). Emitted by
   // `mechanismTruth.ts` when the BREP pose-sweep work estimate exceeds the