Merge pull request #56 from MaxLinCode/claude/fix-stale-clarification

Fix stale/dupe clarifications

Author: MaxLinCode

apps/web/src/lib/server/conversation-state.test.ts

@@ -34,6 +34,7 @@ describe("deriveConversationReplyState", () => {
       snapshot: buildSnapshot(),
       policy: {
         action: "ask_clarification",
+        clarificationSlots: ["time"],
         committedSlots: {}
       },
       interpretation: {
@@ -201,6 +202,7 @@ describe("deriveConversationReplyState", () => {
       snapshot: buildSnapshot(),
       policy: {
         action: "ask_clarification",
+        clarificationSlots: ["time"],
         committedSlots: { day: "tomorrow" },
         resolvedContract: contract
       },
@@ -241,6 +243,78 @@ describe("deriveConversationReplyState", () => {
 
     expect(result.discourseState?.pending_write_contract).toBeUndefined();
   });
+
+  it("clears active clarifications when a clarification_answer resolves all slots", () => {
+    const snapshot = buildSnapshot();
+    snapshot.entityRegistry = [
+      {
+        id: "clar-1",
+        conversationId: "conversation-1",
+        kind: "clarification",
+        label: "Need a time",
+        status: "active",
+        createdAt: "2026-03-22T16:01:00.000Z",
+        updatedAt: "2026-03-22T16:01:00.000Z",
+        data: {
+          prompt: "What time should I schedule it?",
+          reason: "time",
+          open: true
+        }
+      }
+    ];
+    snapshot.discourseState = {
+      focus_entity_id: "clar-1",
+      currently_editable_entity_id: null,
+      last_user_mentioned_entity_ids: [],
+      last_presented_items: [],
+      pending_clarifications: [
+        {
+          id: "clar-1",
+          slot: "time",
+          question: "What time should I schedule it?",
+          status: "pending",
+          blocking: true,
+          createdAt: "2026-03-22T16:01:00.000Z",
+          createdTurnId: "assistant:1"
+        }
+      ],
+      mode: "clarifying"
+    };
+
+    const result = deriveConversationReplyState({
+      snapshot,
+      policy: {
+        action: "present_proposal",
+        committedSlots: { day: "tomorrow", time: "17:00" }
+      },
+      interpretation: {
+        turnType: "clarification_answer",
+        confidence: 0.93,
+        resolvedEntityIds: [],
+        ambiguity: "none"
+      },
+      reply: "I'll schedule gym at 5 PM tomorrow. Sound good?",
+      userTurnText: "5pm",
+      summaryText: null,
+      occurredAt: "2026-03-22T16:05:00.000Z"
+    });
+
+    expect(result.discourseState?.pending_clarifications).toEqual([
+      expect.objectContaining({
+        id: "clar-1",
+        status: "resolved"
+      })
+    ]);
+    expect(result.entityRegistry).toEqual(
+      expect.arrayContaining([
+        expect.objectContaining({
+          id: "clar-1",
+          kind: "clarification",
+          status: "resolved"
+        })
+      ])
+    );
+  });
 });
 
 describe("deriveMutationState", () => {

apps/web/src/lib/server/conversation-state.ts

@@ -43,13 +43,14 @@ export function deriveConversationReplyState(input: DeriveConversationReplyState
   const newClarifications: PendingClarification[] = [];
   const resolvedClarificationIds: string[] = [];
   const activePendingClarifications = getActivePendingClarifications(discourseState);
-  const persistableClarificationSlots = derivePersistableClarificationSlots(input.interpretation.missingSlots);
+  const persistableClarificationSlots = derivePersistableClarificationSlots(input.policy.clarificationSlots);
   const shouldPersistClarification =
     input.policy.action === "ask_clarification" && persistableClarificationSlots.length > 0;
   const shouldClearActiveClarifications =
     shouldPersistClarification ||
     input.policy.action === "present_proposal" ||
-    input.interpretation.turnType === "confirmation";
+    input.interpretation.turnType === "confirmation" ||
+    (input.interpretation.turnType === "clarification_answer" && !shouldPersistClarification);
   let nextFocusEntityId: string | null | undefined;
 
   if (shouldClearActiveClarifications) {
@@ -87,7 +88,7 @@ export function deriveConversationReplyState(input: DeriveConversationReplyState
           mutationInputSource: null,
           confirmationRequired: true,
           originatingTurnText: input.userTurnText,
-          missingSlots: input.interpretation.missingSlots
+          missingSlots: input.policy.clarificationSlots
         }
       })
     );
@@ -409,14 +410,16 @@ function upsertActiveProposalEntity(
   return nextProposal;
 }
 
+const PERSISTABLE_SLOT_KEYS = new Set(["day", "time", "duration", "target"]);
+
 function derivePersistableClarificationSlots(slots: string[] | undefined) {
   if (!slots) {
     return [];
   }
 
   return Array.from(
     new Set(
-      slots.map((slot) => slot.trim()).filter((slot) => slot.length > 0 && slot !== "unknown")
+      slots.map((slot) => slot.trim()).filter((slot) => PERSISTABLE_SLOT_KEYS.has(slot))
     )
   );
 }

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

@@ -37,8 +37,7 @@ export function decideTurnPolicy(input: DecideTurnPolicyInput): TurnPolicyDecisi
   const ambiguity = deriveAmbiguity({
     classifierConfidence: classification.confidence,
     missingSlots: commitResult.missingSlots,
-    needsClarification: commitResult.needsClarification,
-    blockingSlots: []
+    needsClarification: commitResult.needsClarification
   });
 
   switch (classification.turnType) {

apps/web/src/lib/server/turn-router.ts

@@ -1,10 +1,8 @@
 import {
   applyCommitPolicy,
   createEmptyDiscourseState,
-  getActivePendingClarifications,
   routedTurnSchema,
   type ConversationTurn,
-  type DiscourseState,
   type RoutedTurn,
   type SlotKey,
   deriveAmbiguity,
@@ -75,7 +73,7 @@ export async function routeMessageTurn(input: TurnRouterInput): Promise<TurnRout
   });
 
   // Build interpretation for backward compatibility
-  const interpretation = buildInterpretation(classification, commitResult, discourseState);
+  const interpretation = buildInterpretation(classification, commitResult);
 
   return routedTurnSchema.parse({
     interpretation,
@@ -96,22 +94,16 @@ function deriveConversationContext(recentTurns: ConversationTurn[]): string {
 
 function buildInterpretation(
   classification: TurnClassifierOutput,
-  commitResult: CommitPolicyOutput,
-  discourseState: DiscourseState
+  commitResult: CommitPolicyOutput
 ): TurnInterpretation {
-  const blockingSlots = getActivePendingClarifications(discourseState)
-    .filter((c) => c.blocking)
-    .map((c) => c.slot);
   const allMissingSlots = unique([
     ...commitResult.missingSlots,
-    ...commitResult.needsClarification,
-    ...blockingSlots
+    ...commitResult.needsClarification
   ]);
   const ambiguity = deriveAmbiguity({
     classifierConfidence: classification.confidence,
     missingSlots: commitResult.missingSlots,
-    needsClarification: commitResult.needsClarification,
-    blockingSlots
+    needsClarification: commitResult.needsClarification
   });
 
   return {
@@ -121,21 +113,16 @@ function buildInterpretation(
     ...(classification.resolvedProposalId ? { resolvedProposalId: classification.resolvedProposalId } : {}),
     ambiguity,
     ...(ambiguity !== "none"
-      ? { ambiguityReason: deriveAmbiguityReason(classification.turnType, ambiguity, blockingSlots) }
+      ? { ambiguityReason: deriveAmbiguityReason(classification.turnType, ambiguity) }
       : {}),
     ...(allMissingSlots.length > 0 ? { missingSlots: allMissingSlots } : {})
   };
 }
 
 function deriveAmbiguityReason(
   turnType: TurnInterpretation["turnType"],
-  ambiguity: TurnAmbiguity,
-  blockingSlots: string[]
+  ambiguity: TurnAmbiguity
 ): string {
-  if (blockingSlots.length > 0) {
-    return "Blocking clarification slots are still open.";
-  }
-
   if (ambiguity === "high") {
     switch (turnType) {
       case "unknown":

docs/current-work.md

@@ -299,6 +299,20 @@ PR #53 (`codex/turn-routing-refactor`) implements the 3-layer turn routing pipel
 3. Merge PR #53
 4. Open follow-up issue for items 6–10
 
+### Follow-up — clarification state cleanup (separate PR)
+
+The stale-slot-readiness fix revealed further simplification opportunities in the clarification persistence model:
+
+1. **`blocking` field on `PendingClarification` is dead weight.** It is hardcoded to `true` everywhere — never set to `false`. The only reader is `deriveMode()`, which checks `hasBlockingClarification` to return `"clarifying"` mode. The field can be removed and `deriveMode` can derive "clarifying" from `pending_write_contract` + `resolved_slots` (contract gaps) instead.
+
+2. **Resolved/cancelled clarifications accumulate as noise.** `getActivePendingClarifications()` filters to `status === "pending"`, so resolved entries just grow the array. No code inspects resolved entries for decisions. They could be pruned on state update or moved to a separate audit trail.
+
+3. **Full `pending_clarifications` array (including resolved entries) is passed to the LLM classifier.** The system prompt says "active clarifications the assistant is waiting on" but the data includes stale resolved entries. This could mislead the model into thinking there are still open questions. Fix: filter to `status === "pending"` before passing to the classifier, or pass only `getActivePendingClarifications()`.
+
+4. **`deriveMode` could derive "clarifying" from canonical slot state.** Instead of checking `hasBlockingClarification` (which depends on persisted clarification entities), check whether `pending_write_contract` exists and has unresolved required slots in `resolved_slots`. This aligns mode derivation with the same canonical source of truth used for routing.
+
+5. **`getActivePendingClarifications` remains needed** for reference resolution (disambiguating pronouns when there's a single active clarification for an entity) and state transitions (marking clarifications as resolved). It is no longer needed for routing decisions after the stale-slot-readiness fix.
+
 ### After merge — next priorities
 
 - Stabilize the planner contract and prompt iteration loop using `pnpm eval:planner` and `pnpm eval:all` as the gating loop

packages/core/src/ambiguity.ts

@@ -4,11 +4,9 @@ export type DeriveAmbiguityInput = {
   classifierConfidence: number;
   missingSlots: string[];
   needsClarification: string[];
-  blockingSlots: string[];
 };
 
 export function deriveAmbiguity(input: DeriveAmbiguityInput): TurnAmbiguity {
-  if (input.blockingSlots.length > 0) return "high";
   if (input.classifierConfidence < 0.6) return "high";
   if (input.missingSlots.length > 0) return "high";
   if (input.needsClarification.length > 0) return "high";