Updating transition scheduling logic

.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/createInstance.ts

@@ -11,7 +11,6 @@ import { User } from '@op/supabase/lib';
 import { CommonError, NotFoundError, UnauthorizedError } from '../../utils';
 import { assertUserByAuthId } from '../assert';
 import { generateUniqueProfileSlug } from '../profile/utils';
-import { createTransitionsForProcess } from './createTransitionsForProcess';
 import type { InstanceData, ProcessSchema } from './types';
 
 export interface CreateInstanceInput {
@@ -93,9 +92,8 @@ export const createInstance = async ({
       return newInstance;
     });
 
-    // Create transitions for the process phases
-    // This is critical - if transitions can't be created, the process won't auto-advance
-    await createTransitionsForProcess({ processInstance: instance });
+    // Note: Transitions are created when the instance is published (status changes from DRAFT to PUBLISHED)
+    // Draft instances don't need transitions since they won't be processed
 
     return instance;
   } catch (error) {

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

@@ -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 { getTemplate } from './getTemplate';
 import {
   type PhaseOverride,
@@ -123,22 +122,8 @@ 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

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.
+ *
+ * All phase rules are expected to be present in instanceData.phases. Rules are
+ * populated when creating an instance from a template and preserved during updates.
+ */
+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,39 +41,55 @@ export async function createTransitionsForProcess({
       );
     }
 
-    const transitionsToCreate = phases.map(
-      (phase: PhaseConfiguration, index: number) => {
-        const fromStateId = index > 0 ? phases[index - 1]?.phaseId : null;
-        const toStateId = phase.phaseId;
-        // For phases like 'results' that only have a start date (no end), use the start date
-        const scheduledDate = 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[] = [];
 
-        if (!scheduledDate) {
-          throw new CommonError(
-            `Phase ${index + 1} (${toStateId}) must have either a scheduled end date or start date`,
-          );
-        }
+    phases.forEach((currentPhase, index) => {
+      const nextPhase = phases[index + 1];
+      // Skip last phase (no next phase to transition to)
+      if (!nextPhase) {
+        return;
+      }
 
-        return {
-          processInstanceId: processInstance.id,
-          fromStateId,
-          toStateId,
-          scheduledDate: new Date(scheduledDate).toISOString(),
-        };
-      },
-    );
+      // Only create transition if current phase uses date-based advancement
+      if (currentPhase.rules?.advancement?.method !== 'date') {
+        return;
+      }
+
+      // 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

@@ -68,3 +68,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 || '';