Fix unresolved slots overriding prior slots

Author: MaxLinCode

packages/core/src/commit-policy.test.ts

@@ -2,7 +2,11 @@ import { describe, expect, it } from "vitest";
 
 import type { CommitPolicyInput } from "./commit-policy";
 import { applyCommitPolicy } from "./commit-policy";
-import type { WriteContract } from "./index";
+import type { TimeSpec, WriteContract } from "./index";
+
+function t(hour: number, minute: number): TimeSpec {
+  return { kind: "absolute", hour, minute };
+}
 
 const defaultContract: WriteContract = {
   requiredSlots: ["day", "time"],
@@ -26,7 +30,7 @@ describe("applyCommitPolicy", () => {
     const result = applyCommitPolicy(
       buildInput({
         turnType: "informational",
-        extractedValues: { time: "15:00" },
+        extractedValues: { time: t(15, 0) },
         confidence: { time: 0.95 },
       }),
     );
@@ -38,7 +42,7 @@ describe("applyCommitPolicy", () => {
     const result = applyCommitPolicy(
       buildInput({
         turnType: "confirmation",
-        extractedValues: { time: "15:00" },
+        extractedValues: { time: t(15, 0) },
         confidence: { time: 0.95 },
       }),
     );
@@ -50,12 +54,12 @@ describe("applyCommitPolicy", () => {
     const result = applyCommitPolicy(
       buildInput({
         turnType: "planning_request",
-        extractedValues: { time: "17:00", day: "tomorrow" },
+        extractedValues: { time: t(17, 0), day: "tomorrow" },
         confidence: { time: 0.9, day: 0.85 },
       }),
     );
 
-    expect(result.committedSlots.time).toBe("17:00");
+    expect(result.committedSlots.time).toEqual(t(17, 0));
     expect(result.committedSlots.day).toBe("tomorrow");
     expect(result.needsClarification).toEqual([]);
   });
@@ -64,7 +68,7 @@ describe("applyCommitPolicy", () => {
     const result = applyCommitPolicy(
       buildInput({
         turnType: "planning_request",
-        extractedValues: { time: "17:00" },
+        extractedValues: { time: t(17, 0) },
         confidence: { time: 0.6 },
       }),
     );
@@ -77,27 +81,27 @@ describe("applyCommitPolicy", () => {
     const result = applyCommitPolicy(
       buildInput({
         turnType: "clarification_answer",
-        extractedValues: { time: "15:00" },
+        extractedValues: { time: t(15, 0) },
         confidence: { time: 0.8 },
-        priorResolvedSlots: { time: "14:00" },
+        priorResolvedSlots: { time: t(14, 0) },
       }),
     );
 
-    expect(result.committedSlots.time).toBe("14:00");
+    expect(result.committedSlots.time).toEqual(t(14, 0));
     expect(result.needsClarification).toContain("time");
   });
 
   it("commits correction at or above correction threshold", () => {
     const result = applyCommitPolicy(
       buildInput({
         turnType: "clarification_answer",
-        extractedValues: { time: "15:00" },
+        extractedValues: { time: t(15, 0) },
         confidence: { time: 0.92 },
-        priorResolvedSlots: { time: "14:00" },
+        priorResolvedSlots: { time: t(14, 0) },
       }),
     );
 
-    expect(result.committedSlots.time).toBe("15:00");
+    expect(result.committedSlots.time).toEqual(t(15, 0));
     expect(result.needsClarification).not.toContain("time");
   });
 
@@ -120,7 +124,7 @@ describe("applyCommitPolicy", () => {
         turnType: "planning_request",
         extractedValues: { day: "friday" },
         confidence: { day: 0.9 },
-        priorResolvedSlots: { time: "14:00", day: "tomorrow" },
+        priorResolvedSlots: { time: t(14, 0), day: "tomorrow" },
         activeContract: { requiredSlots: ["day", "time"], intentKind: "edit" },
         priorContract: { requiredSlots: ["day", "time"], intentKind: "plan" },
       }),
@@ -136,13 +140,13 @@ describe("applyCommitPolicy", () => {
         turnType: "planning_request",
         extractedValues: { day: "friday" },
         confidence: { day: 0.9 },
-        priorResolvedSlots: { time: "14:00" },
+        priorResolvedSlots: { time: t(14, 0) },
         activeContract: { requiredSlots: ["day", "time"], intentKind: "plan" },
         priorContract: { requiredSlots: ["day"], intentKind: "plan" },
       }),
     );
 
-    expect(result.committedSlots.time).toBe("14:00");
+    expect(result.committedSlots.time).toEqual(t(14, 0));
     expect(result.committedSlots.day).toBe("friday");
   });
 
@@ -179,35 +183,35 @@ describe("applyCommitPolicy", () => {
     const result = applyCommitPolicy(
       buildInput({
         turnType: "clarification_answer",
-        extractedValues: { time: "17:00" },
+        extractedValues: { time: t(17, 0) },
         confidence: { time: 0.9 },
         priorResolvedSlots: { day: "tomorrow" },
       }),
     );
 
     expect(result.committedSlots.day).toBe("tomorrow");
-    expect(result.committedSlots.time).toBe("17:00");
+    expect(result.committedSlots.time).toEqual(t(17, 0));
   });
 
   it("commits slots for edit_request turn type", () => {
     const result = applyCommitPolicy(
       buildInput({
         turnType: "edit_request",
-        extractedValues: { time: "10:00" },
+        extractedValues: { time: t(10, 0) },
         confidence: { time: 0.88 },
         activeContract: { requiredSlots: ["time"], intentKind: "edit" },
       }),
     );
 
-    expect(result.committedSlots.time).toBe("10:00");
+    expect(result.committedSlots.time).toEqual(t(10, 0));
     expect(result.missingSlots).toEqual([]);
   });
 
   it("treats missing confidence as zero", () => {
     const result = applyCommitPolicy(
       buildInput({
         turnType: "planning_request",
-        extractedValues: { time: "17:00" },
+        extractedValues: { time: t(17, 0) },
         confidence: {},
       }),
     );
@@ -216,6 +220,22 @@ describe("applyCommitPolicy", () => {
     expect(result.needsClarification).toContain("time");
   });
 
+  it("does not flag unresolvable slot that is already resolved from prior turn", () => {
+    const result = applyCommitPolicy(
+      buildInput({
+        turnType: "clarification_answer",
+        extractedValues: { day: "friday" },
+        confidence: { day: 0.9 },
+        unresolvable: ["time"],
+        priorResolvedSlots: { time: t(14, 0) },
+      }),
+    );
+
+    expect(result.committedSlots.time).toEqual(t(14, 0));
+    expect(result.needsClarification).not.toContain("time");
+    expect(result.committedSlots.day).toBe("friday");
+  });
+
   it("handles unresolvable slots not in extractedValues", () => {
     const result = applyCommitPolicy(
       buildInput({

packages/core/src/commit-policy.ts

@@ -1,9 +1,11 @@
 import type {
   ResolvedSlots,
   SlotKey,
+  TimeSpec,
   TurnInterpretation,
   WriteContract,
 } from "./index";
+import { timeSpecsEqual } from "./time-spec";
 
 export type CommitPolicyInput = {
   turnType: TurnInterpretation["turnType"];
@@ -76,7 +78,8 @@ export function applyCommitPolicy(
     }
 
     const priorValue = priorSlots[slot];
-    const isCorrection = priorValue !== undefined && priorValue !== value;
+    const isCorrection =
+      priorValue !== undefined && !slotValuesEqual(slot, priorValue, value);
     if (isCorrection && slotConfidence < CORRECTION_THRESHOLD) {
       needsClarification.push(slot);
       continue;
@@ -86,7 +89,11 @@ export function applyCommitPolicy(
   }
 
   for (const slot of unresolvable) {
-    if (!slotKeys.includes(slot) && !needsClarification.includes(slot)) {
+    if (
+      !slotKeys.includes(slot) &&
+      !needsClarification.includes(slot) &&
+      committedSlots[slot] === undefined
+    ) {
       needsClarification.push(slot);
     }
   }
@@ -104,3 +111,10 @@ function deriveMissingSlots(
 ): SlotKey[] {
   return contract.requiredSlots.filter((slot) => committed[slot] === undefined);
 }
+
+function slotValuesEqual(slot: SlotKey, a: unknown, b: unknown): boolean {
+  if (slot === "time" && a && b) {
+    return timeSpecsEqual(a as TimeSpec, b as TimeSpec);
+  }
+  return a === b;
+}