Add comprehensive test suite for transition scheduling

.claude/commands/review-branch.md

@@ -8,4 +8,6 @@ Do a thorough code review of this branch. If an argument is passed and it is a g
 If there is no argument, you should review the current changes on this branch (you can diff against the dev branch).
 Always do this in planning mode and present the review at the end.
 
+Additionally, once you have reviewed the branch: Review all tests updated in this branch. Making sure they test what they say they test and provide good coverage over the functionality.
+
 Arguments: $ARGUMENTS

apps/app/src/components/decisions/CurrentPhaseSurface.tsx

@@ -5,15 +5,12 @@ import {
   formatCurrency,
   formatDateRange,
 } from '@/utils/formatting';
-import type { processPhaseSchema } from '@op/api/encoders';
+import { type ProcessPhase } from '@op/api/encoders';
 import { Surface } from '@op/ui/Surface';
 import { useLocale } from 'next-intl';
-import type { z } from 'zod';
 
 import { useTranslations } from '@/lib/i18n';
 
-type ProcessPhase = z.infer<typeof processPhaseSchema>;
-
 interface CurrentPhaseSurfaceProps {
   currentPhase?: ProcessPhase;
   budget?: number;

apps/app/src/components/decisions/DecisionHeader.tsx

@@ -1,3 +1,4 @@
+import { type ProcessPhase } from '@op/api/encoders';
 import { createClient } from '@op/api/serverClient';
 import type { DecisionInstanceData } from '@op/common';
 import { cn } from '@op/ui/utils';
@@ -6,7 +7,6 @@ import { ReactNode } from 'react';
 
 import { DecisionInstanceHeader } from '@/components/decisions/DecisionInstanceHeader';
 import { DecisionProcessStepper } from '@/components/decisions/DecisionProcessStepper';
-import { ProcessPhase } from '@/components/decisions/types';
 
 interface DecisionHeaderProps {
   instanceId: string;

apps/app/src/components/decisions/DecisionProcessStepper.tsx

@@ -1,19 +1,8 @@
 'use client';
 
+import { type ProcessPhase } from '@op/api/encoders';
 import { type Phase, PhaseStepper } from '@op/ui/PhaseStepper';
 
-interface ProcessPhase {
-  id: string;
-  name: string;
-  description?: string;
-  phase?: {
-    startDate?: string;
-    endDate?: string;
-    sortOrder?: number;
-  };
-  type?: 'initial' | 'intermediate' | 'final';
-}
-
 interface DecisionProcessStepperProps {
   phases: ProcessPhase[];
   currentStateId: string;

apps/app/src/components/decisions/DecisionStats.tsx

@@ -1,10 +1,7 @@
 'use client';
 
 import { formatCurrency, formatDateRange } from '@/utils/formatting';
-import type { processPhaseSchema } from '@op/api/encoders';
-import type { z } from 'zod';
-
-type ProcessPhase = z.infer<typeof processPhaseSchema>;
+import { type ProcessPhase } from '@op/api/encoders';
 
 interface DecisionStatsProps {
   currentPhase?: ProcessPhase;
@@ -29,11 +26,11 @@ export function DecisionStats({
         <p className="mt-1 text-lg font-medium text-neutral-charcoal">
           {currentPhase?.name || 'Proposal Submissions'}
         </p>
-        {currentPhase?.phase && (
+        {(currentPhase?.phase?.startDate || currentPhase?.phase?.endDate) && (
           <p className="mt-1 text-sm text-neutral-gray3">
             {formatDateRange(
-              currentPhase.phase.startDate,
-              currentPhase.phase.endDate,
+              currentPhase.phase?.startDate,
+              currentPhase.phase?.endDate,
             ) || 'Timeline not set'}
           </p>
         )}

packages/common/src/services/decision/createInstanceFromTemplate.ts

@@ -1,4 +1,4 @@
-import { db, eq } from '@op/db/client';
+import { db } from '@op/db/client';
 import {
   EntityType,
   ProcessStatus,
@@ -12,7 +12,6 @@ import type { User } from '@op/supabase/lib';
 import { CommonError, UnauthorizedError } from '../../utils';
 import { assertUserByAuthId } from '../assert';
 import { generateUniqueProfileSlug } from '../profile/utils';
-import { createTransitionsForProcess } from './createTransitionsForProcess';
 import { createDecisionRole } from './decisionRoles';
 import { getTemplate } from './getTemplate';
 import {
@@ -186,27 +185,13 @@ export const createInstanceFromTemplateCore = async ({
     return newInstance;
   });
 
-  // Create scheduled transitions for phases that have date-based advancement AND actual dates set
-  const hasScheduledDatePhases = instanceData.phases.some(
-    (phase) => phase.rules?.advancement?.method === 'date' && phase.startDate,
-  );
-
-  if (hasScheduledDatePhases) {
-    try {
-      await createTransitionsForProcess({ processInstance: instance });
-    } catch (error) {
-      // Log but don't fail instance creation if transitions can't be created
-      console.error(
-        'Failed to create transitions for process instance:',
-        error,
-      );
-    }
-  }
+  // Note: Transitions are NOT created here because the instance is created as DRAFT.
+  // Transitions are created when the instance is published via updateDecisionInstance.
 
   // Fetch the profile with processInstance joined for the response
   // profileId is guaranteed to be set since we just created it above
-  const profile = await db._query.profiles.findFirst({
-    where: eq(profiles.id, instance.profileId!),
+  const profile = await db.query.profiles.findFirst({
+    where: { id: instance.profileId! },
     with: {
       processInstance: true,
     },

packages/common/src/services/decision/createTransitionsForProcess.ts

@@ -1,32 +1,38 @@
-import { db } from '@op/db/client';
+import { type TransactionType, db } from '@op/db/client';
 import { decisionProcessTransitions } from '@op/db/schema';
 import type { ProcessInstance } from '@op/db/schema';
 
 import { CommonError } from '../../utils';
-import type { InstanceData, PhaseConfiguration } from './types';
+import type { DecisionInstanceData } from './schemas/instanceData';
+import type { ScheduledTransition } from './types';
 
-export interface CreateTransitionsInput {
+/**
+ * Creates scheduled transition records for phases with date-based advancement.
+ * Each transition fires when the current phase's end date arrives.
+ *
+ * Rules are read from the instance's phase data (instanceData.phases[].rules),
+ * which are populated from the template when the instance is created.
+ */
+export async function createTransitionsForProcess({
+  processInstance,
+  tx,
+}: {
   processInstance: ProcessInstance;
-}
-
-export interface CreateTransitionsResult {
+  tx?: TransactionType;
+}): Promise<{
   transitions: Array<{
     id: string;
     fromStateId: string | null;
     toStateId: string;
     scheduledDate: Date;
   }>;
-}
+}> {
+  const dbClient = tx ?? db;
 
-/**
- * Creates transition records for all phases in a process instance.
- * Each transition represents the end of one phase and the start of the next.
- */
-export async function createTransitionsForProcess({
-  processInstance,
-}: CreateTransitionsInput): Promise<CreateTransitionsResult> {
   try {
-    const instanceData = processInstance.instanceData as InstanceData;
+    // Type assertion: instanceData is `unknown` in DB to support legacy formats for viewing,
+    // but this function is only called for new DecisionInstanceData processes
+    const instanceData = processInstance.instanceData as DecisionInstanceData;
     const phases = instanceData.phases;
 
     if (!phases || phases.length === 0) {
@@ -35,40 +41,55 @@ export async function createTransitionsForProcess({
       );
     }
 
-    const transitionsToCreate = phases.flatMap(
-      (phase: PhaseConfiguration, index: number) => {
-        const scheduledDate = phase.endDate ?? phase.startDate;
+    // Create transitions for phases that use date-based advancement
+    // A transition is created FROM a phase (when it ends) TO the next phase
+    const transitionsToCreate: ScheduledTransition[] = [];
 
-        // Skip phases that have no dates yet — they don't need transitions
-        if (!scheduledDate) {
-          return [];
-        }
+    phases.forEach((currentPhase, index) => {
+      const nextPhase = phases[index + 1];
+      // Skip last phase (no next phase to transition to)
+      if (!nextPhase) {
+        return;
+      }
 
-        const fromStateId = index > 0 ? phases[index - 1]?.phaseId : null;
-        const toStateId = phase.phaseId;
+      // Only create transition if current phase uses date-based advancement
+      if (currentPhase.rules?.advancement?.method !== 'date') {
+        return;
+      }
 
-        return [
-          {
-            processInstanceId: processInstance.id,
-            fromStateId,
-            toStateId,
-            scheduledDate: new Date(scheduledDate).toISOString(),
-          },
-        ];
-      },
-    );
+      // Schedule transition when the current phase ends
+      const scheduledDate = currentPhase.endDate;
+
+      if (!scheduledDate) {
+        throw new CommonError(
+          `Phase "${currentPhase.phaseId}" must have an end date for date-based advancement (instance: ${processInstance.id})`,
+        );
+      }
+
+      // DB columns are named fromStateId/toStateId but store phase IDs
+      transitionsToCreate.push({
+        processInstanceId: processInstance.id,
+        fromStateId: currentPhase.phaseId,
+        toStateId: nextPhase.phaseId,
+        scheduledDate: new Date(scheduledDate).toISOString(),
+      });
+    });
+
+    if (transitionsToCreate.length === 0) {
+      return { transitions: [] };
+    }
 
-    const createdTransitions = await db
+    const createdTransitions = await dbClient
       .insert(decisionProcessTransitions)
       .values(transitionsToCreate)
       .returning();
 
     return {
-      transitions: createdTransitions.map((t) => ({
-        id: t.id,
-        fromStateId: t.fromStateId,
-        toStateId: t.toStateId,
-        scheduledDate: new Date(t.scheduledDate),
+      transitions: createdTransitions.map((transition) => ({
+        id: transition.id,
+        fromStateId: transition.fromStateId,
+        toStateId: transition.toStateId,
+        scheduledDate: new Date(transition.scheduledDate),
       })),
     };
   } catch (error) {

packages/common/src/services/decision/index.ts

@@ -73,3 +73,9 @@ export type {
   DecisionInstanceData,
   PhaseInstanceData,
 } from './schemas/instanceData';
+export type {
+  DecisionSchemaDefinition,
+  PhaseDefinition,
+  PhaseRules,
+  ProcessConfig,
+} from './schemas/types';

packages/common/src/services/decision/transitionEngine.ts

@@ -73,12 +73,6 @@ export class TransitionEngine {
       const process = instance.process as any;
       const processSchema = process.processSchema as ProcessSchema;
       const instanceData = instance.instanceData as InstanceData;
-      console.log(
-        'TRANSITION',
-        instanceData.currentPhaseId,
-        instance.currentStateId,
-        instanceData,
-      );
       const currentStateId =
         instanceData.currentPhaseId || instance.currentStateId || '';