Merge pull request #54 from MaxLinCode/codex/turn-routing-review-fixes

fix: turn routing review fixes

Author: MaxLinCode

apps/web/src/app/api/telegram/webhook/route.test.ts

@@ -31,7 +31,8 @@ const {
   classifyTurnWithResponsesMock,
   summarizeConversationMemoryWithResponsesMock,
   respondToConversationTurnWithResponsesMock,
-  recoverConfirmedMutationWithResponsesMock
+  recoverConfirmedMutationWithResponsesMock,
+  extractSlotsWithResponsesMock
 } = vi.hoisted(() => ({
   editTelegramMessageMock: vi.fn(),
   sendTelegramMessageMock: vi.fn(),
@@ -40,7 +41,8 @@ const {
   classifyTurnWithResponsesMock: vi.fn(),
   summarizeConversationMemoryWithResponsesMock: vi.fn(),
   respondToConversationTurnWithResponsesMock: vi.fn(),
-  recoverConfirmedMutationWithResponsesMock: vi.fn()
+  recoverConfirmedMutationWithResponsesMock: vi.fn(),
+  extractSlotsWithResponsesMock: vi.fn()
 }));
 
 vi.mock("@atlas/integrations", async () => {
@@ -84,6 +86,7 @@ vi.mock("@atlas/integrations", async () => {
     recoverConfirmedMutationWithResponses: recoverConfirmedMutationWithResponsesMock,
     classifyTurnWithResponses: classifyTurnWithResponsesMock,
     routeTurnWithResponses: routeTurnWithResponsesMock,
+    extractSlotsWithResponses: extractSlotsWithResponsesMock,
     sendTelegramChatAction: sendTelegramChatActionMock,
     sendTelegramMessage: sendTelegramMessageMock,
     summarizeConversationMemoryWithResponses: summarizeConversationMemoryWithResponsesMock
@@ -283,6 +286,15 @@ beforeEach(async () => {
   summarizeConversationMemoryWithResponsesMock.mockReset();
   respondToConversationTurnWithResponsesMock.mockReset();
   recoverConfirmedMutationWithResponsesMock.mockReset();
+  extractSlotsWithResponsesMock.mockReset();
+  extractSlotsWithResponsesMock.mockResolvedValue({
+    time: { hour: 9, minute: 0 },
+    day: { kind: "relative", value: "tomorrow" },
+    duration: null,
+    target: null,
+    confidence: { day: 0.95, time: 0.95 },
+    unresolvable: []
+  });
   routeTurnWithResponsesMock.mockResolvedValue({
     route: "mutation",
     reason: "Direct scheduling request."
@@ -715,8 +727,16 @@ describe("telegram webhook route", () => {
     expect(listInboxItemsForTests()).toHaveLength(1);
   });
 
-  it("normalizes a Telegram text message and hands it to app services", async () => {
+  it("normalizes a Telegram text message and routes to clarification when slots are missing", async () => {
     process.env.TELEGRAM_WEBHOOK_SECRET = "test-webhook-secret";
+    extractSlotsWithResponsesMock.mockResolvedValueOnce({
+      time: null,
+      day: null,
+      duration: null,
+      target: null,
+      confidence: {},
+      unresolvable: []
+    });
 
     const response = await handleTelegramWebhook(
       buildRequest({
@@ -773,42 +793,23 @@ describe("telegram webhook route", () => {
         processingStatus: "received",
         linkedTaskIds: []
       },
-      processing: {
-        outcome: "planned"
-      },
-      outboundDelivery: {
-        status: "edited",
-        attempts: 1
-      }
-    });
-    expect(response.body).toMatchObject({
-      outboundDelivery: {
-        idempotencyKey: expect.any(String),
-        message: {
-          message_id: 88
+      routing: {
+        interpretation: {
+          turnType: "planning_request",
+          ambiguity: "high"
+        },
+        policy: {
+          action: "ask_clarification"
         }
+      },
+      processing: {
+        outcome: "conversation_replied"
       }
     });
     expect(listIncomingBotEventsForTests()).toHaveLength(1);
-    expect(listOutgoingBotEventsForTests()).toHaveLength(1);
     expect(listInboxItemsForTests()).toHaveLength(1);
-    expect(listPlannerRunsForTests()).toHaveLength(1);
-    expect(listTasksForTests()).toHaveLength(1);
-    expect(listScheduleBlocksForTests()).toHaveLength(1);
-    expect(sendTelegramChatActionMock).toHaveBeenCalledWith({
-      chatId: "999",
-      action: "typing"
-    });
-    expect(sendTelegramMessageMock).toHaveBeenCalledTimes(1);
-    expect(sendTelegramMessageMock).toHaveBeenCalledWith({
-      chatId: "999",
-      text: "Checking your schedule"
-    });
-    expect(editTelegramMessageMock).toHaveBeenCalledWith({
-      chatId: "999",
-      messageId: 88,
-      text: expect.stringContaining("Scheduled 'Review launch checklist'")
-    });
+    expect(listPlannerRunsForTests()).toHaveLength(0);
+    expect(listTasksForTests()).toHaveLength(0);
   });
 
   it("preserves mutation behavior when the turn router explicitly returns mutation", async () => {
@@ -865,6 +866,14 @@ describe("telegram webhook route", () => {
 
   it("does not keep clear scheduling requests in discuss-first mode", async () => {
     process.env.TELEGRAM_WEBHOOK_SECRET = "test-webhook-secret";
+    extractSlotsWithResponsesMock.mockResolvedValueOnce({
+      time: { hour: 18, minute: 0 },
+      day: { kind: "relative", value: "tomorrow" },
+      duration: { minutes: 60 },
+      target: null,
+      confidence: { day: 0.95, time: 0.95, duration: 0.9 },
+      unresolvable: []
+    });
 
     const response = await handleTelegramWebhook(
       buildRequest({

apps/web/src/lib/server/decide-turn-policy.ts

@@ -1,9 +1,10 @@
 import {
+  containsWriteVerb,
+  deriveAmbiguity,
+  deriveConsentRequirement,
   type CommitPolicyOutput,
-  type ConversationEntity,
   type TurnAmbiguity,
   type TurnClassifierOutput,
-  type TurnInterpretationType,
   type TurnPolicyDecision,
   type TurnRoutingInput
 } from "@atlas/core";
@@ -33,7 +34,12 @@ type StructuredWriteReadiness =
 export function decideTurnPolicy(input: DecideTurnPolicyInput): TurnPolicyDecision {
   const { classification, commitResult } = input;
   const targetEntityId = classification.resolvedEntityIds[0];
-  const ambiguity = deriveAmbiguity(classification, commitResult);
+  const ambiguity = deriveAmbiguity({
+    classifierConfidence: classification.confidence,
+    missingSlots: commitResult.missingSlots,
+    needsClarification: commitResult.needsClarification,
+    blockingSlots: []
+  });
 
   switch (classification.turnType) {
     case "informational":
@@ -109,17 +115,6 @@ export function decideTurnPolicy(input: DecideTurnPolicyInput): TurnPolicyDecisi
   }
 }
 
-function deriveAmbiguity(
-  classification: TurnClassifierOutput,
-  commitResult: CommitPolicyOutput
-): TurnAmbiguity {
-  if (classification.confidence < 0.6) return "high";
-  if (commitResult.missingSlots.length > 0) return "high";
-  if (commitResult.needsClarification.length > 0) return "high";
-  if (classification.confidence < 0.8) return "low";
-  return "none";
-}
-
 function deriveStructuredWriteReadiness(
   input: DecideTurnPolicyInput,
   ambiguity: TurnAmbiguity
@@ -152,8 +147,6 @@ function deriveStructuredWriteReadiness(
     };
   }
 
-  // Bug 4 fix: if this is a clarification answer and the proposal is already confirmed,
-  // skip re-presenting and go straight to execution
   if (classification.turnType === "clarification_answer") {
     const entityRegistry = input.routingContext.entityRegistry ?? [];
     const alreadyConfirmed = entityRegistry.some(
@@ -171,13 +164,19 @@ function deriveStructuredWriteReadiness(
     }
   }
 
-  const consentRequirement = deriveConsentRequirement(input);
+  const consentRequirement = deriveConsentRequirement({
+    classification,
+    entityRegistry: input.routingContext.entityRegistry ?? [],
+    normalizedText: input.routingContext.normalizedText
+  });
 
   if (consentRequirement.required) {
     return {
       state: "ready_needs_consent",
       reason: consentRequirement.reason,
-      ...(consentRequirement.targetProposalId ? { targetProposalId: consentRequirement.targetProposalId } : {})
+      ...(consentRequirement.required && "targetProposalId" in consentRequirement
+        ? { targetProposalId: consentRequirement.targetProposalId }
+        : {})
     };
   }
 
@@ -231,145 +230,3 @@ function buildPolicyFromStructuredReadiness(
       };
   }
 }
-
-function deriveConsentRequirement(input: DecideTurnPolicyInput) {
-  const { classification } = input;
-  // Bug 2 fix: match "presented" status in addition to "active"
-  const activeProposal = (input.routingContext.entityRegistry ?? []).find(
-    (entity): entity is Extract<ConversationEntity, { kind: "proposal_option" }> =>
-      entity.kind === "proposal_option" &&
-      (entity.status === "active" || entity.status === "presented") &&
-      entity.id === classification.resolvedProposalId &&
-      entity.data.confirmationRequired === true
-  );
-
-  if (!activeProposal) {
-    return {
-      required: false,
-      reason: "Deterministic product rules do not require additional consent."
-    };
-  }
-
-  if (!matchesProposalTarget(activeProposal.data.targetEntityId ?? null, classification.resolvedEntityIds)) {
-    return {
-      required: false,
-      reason: "Deterministic product rules do not require additional consent."
-    };
-  }
-
-  const compatibility = deriveProposalCompatibility(input, activeProposal);
-
-  if (!compatibility.compatible) {
-    return {
-      required: true,
-      reason: compatibility.reason
-    };
-  }
-
-  return {
-    required: true,
-    reason: "Write request is ready, but deterministic product policy still requires user consent.",
-    targetProposalId: activeProposal.id
-  };
-}
-
-function matchesProposalTarget(targetEntityId: string | null, resolvedEntityIds: string[]) {
-  if (!targetEntityId || resolvedEntityIds.length === 0) {
-    return true;
-  }
-
-  return resolvedEntityIds.includes(targetEntityId);
-}
-
-function deriveProposalCompatibility(
-  input: DecideTurnPolicyInput,
-  proposal: Extract<ConversationEntity, { kind: "proposal_option" }>
-) {
-  if (input.classification.turnType === "clarification_answer") {
-    return {
-      compatible: true,
-      reason: "Clarification answers may continue the same consent-required proposal."
-    };
-  }
-
-  const currentActionKind = deriveActionKind(input.routingContext.normalizedText, input.classification.turnType);
-  const proposalActionKind = deriveActionKind(
-    proposal.data.originatingTurnText ?? proposal.data.replyText,
-    inferProposalTurnType(proposal)
-  );
-
-  if (currentActionKind !== proposalActionKind) {
-    return {
-      compatible: false,
-      reason: "The new turn changes the action type, so it needs fresh consent."
-    };
-  }
-
-  const currentFingerprint = deriveParameterFingerprint(input.routingContext.normalizedText);
-  const proposalFingerprint = deriveParameterFingerprint(proposal.data.originatingTurnText ?? proposal.data.replyText);
-
-  if (currentFingerprint.explicit && proposalFingerprint.explicit && currentFingerprint.value !== proposalFingerprint.value) {
-    return {
-      compatible: false,
-      reason: "The new turn changes proposal parameters, so it needs fresh consent."
-    };
-  }
-
-  return {
-    compatible: true,
-    reason: "The pending proposal still matches the current turn."
-  };
-}
-
-function inferProposalTurnType(
-  proposal: Extract<ConversationEntity, { kind: "proposal_option" }>
-): TurnInterpretationType {
-  const source = (proposal.data.originatingTurnText ?? proposal.data.replyText).toLowerCase();
-
-  if (/\b(move|reschedule|shift|push|pull|complete|archive|cancel|delete|update|change|mark)\b/.test(source)) {
-    return "edit_request";
-  }
-
-  return "planning_request";
-}
-
-function deriveActionKind(text: string, turnType: TurnInterpretationType) {
-  if (turnType === "edit_request") {
-    return "edit";
-  }
-
-  if (turnType === "planning_request") {
-    return "plan";
-  }
-
-  const lower = text.toLowerCase();
-
-  if (/\b(move|reschedule|shift|push|pull|complete|archive|cancel|delete|update|change|mark)\b/.test(lower)) {
-    return "edit";
-  }
-
-  return "plan";
-}
-
-function deriveParameterFingerprint(text: string) {
-  const lower = text.toLowerCase();
-  const dayTokens = lower.match(
-    /\b(today|tonight|tomorrow|tmr|monday|tuesday|wednesday|thursday|friday|saturday|sunday|weekend|next week|next month|morning|afternoon|evening)\b/g
-  ) ?? [];
-  const timeTokens =
-    lower.match(/\b\d{1,2}(?::\d{2})?\s?(?:am|pm)?\b|\bnoon\b|\bmidnight\b/g) ?? [];
-  const durationTokens =
-    lower.match(/\bfor\s+\d+\s*(?:minutes?|mins?|hours?|hrs?)\b|\b\d+\s*(?:minutes?|mins?|hours?|hrs?)\b/g) ?? [];
-  const fingerprintParts = [...dayTokens, ...timeTokens, ...durationTokens].map((part) => part.trim()).sort();
-
-  return {
-    explicit: fingerprintParts.length > 0,
-    value: fingerprintParts.join("|")
-  };
-}
-
-function containsWriteVerb(text: string) {
-  return /\b(schedule|plan|move|reschedule|shift|create|add|book|put|mark|complete|archive|cancel|delete|change|update)\b/i.test(
-    text
-  );
-}