dreampiler/kandev
1
0
Fork 0
Code Issues 43 Pull Requests Actions 6 Packages Projects Releases Wiki Activity

fix: send late clarification answers as messages (#3798) (#3799)

Browse Source
This commit is contained in:
Carlos Florêncio
2026-09-19 21:35:23 +01:00
committed by GitHub
parent a27423f960
commit 02d260ff7d
43 changed files with 3311 additions and 71 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/web/e2e/tests/chat/clarification-submit-failure.spec.ts
+20 -8
View File
@@ -68,7 +68,7 @@ test.describe("Clarification submit failure feedback", () => {
expect(attempt).toBe(2);
});
test("treats an inactive-bundle 409 as expired, never as a silent success", async ({
test("late answer fallback sends affirmative answers after an inactive-bundle 409", async ({
testPage,
apiClient,
seedData,
@@ -84,7 +84,9 @@ test.describe("Clarification submit failure feedback", () => {
await expect(session.clarificationOverlay()).toBeVisible({ timeout: 30_000 });
let attempts = 0;
await testPage.route("**/api/v1/clarification/*/respond", async (route) => {
attempts += 1;
await route.fulfill({
status: 409,
contentType: "application/json",
@@ -95,14 +97,24 @@ test.describe("Clarification submit failure feedback", () => {
});
});
const sessionId = await activeSessionId(testPage);
if (!sessionId) throw new Error("expected an active session for late clarification answer");
await session.clarificationOption("PostgreSQL").click();
const expiredBanner = testPage.getByTestId("clarification-expired");
await expect(expiredBanner).toBeVisible({ timeout: 15_000 });
await expect(testPage.getByTestId("clarification-retry")).toHaveCount(0);
// A dropped answer must never flip the chat back to idle -- that would be
// reporting success for an answer nobody received.
await expect(session.idleInput()).toHaveCount(0);
await expect(session.clarificationOverlay()).toBeVisible();
await expect
.poll(
async () => {
const { messages } = await apiClient.listSessionMessages(sessionId);
return messages.some(
(message) =>
message.author_type === "user" &&
message.content.includes("Question 1") &&
message.content.includes("PostgreSQL"),
);
},
{ timeout: 30_000, message: "inactive answer should be admitted as a new message" },
)
.toBe(true);
expect(attempts).toBe(1);
});
});
apps/web/e2e/tests/chat/clarification.spec.ts
+106 -2
View File
@@ -3,9 +3,9 @@ import { test, expect } from "../../fixtures/test-base";
import { useRegularMode } from "../../helpers/regular-mode";
import type { SeedData } from "../../fixtures/test-base";
import type { ApiClient } from "../../helpers/api-client";
import { seedClarificationSession } from "../../helpers/clarification";
import { activeSessionId, seedClarificationSession } from "../../helpers/clarification";
import { waitForSessionState } from "../../helpers/session";
import { dwell } from "../../helpers/causal-waits";
import { dwell, waitForHttp } from "../../helpers/causal-waits";
import { SessionPage } from "../../pages/session-page";
import { KanbanPage } from "../../pages/kanban-page";
import { SidebarFilterPopoverPage } from "../../pages/sidebar-filter-popover";
@@ -363,6 +363,110 @@ test.describe("Clarification flow", () => {
await expect(session.idleInput()).toBeVisible({ timeout: 30_000 });
});
test("inactive dismissal removes the stale panel and does not retry", async ({
testPage,
apiClient,
seedData,
}) => {
const session = await seedClarificationTask(
testPage,
apiClient,
seedData,
"Clarification Inactive Dismissal",
"clarification",
);
await expect(session.clarificationOverlay()).toBeVisible({ timeout: 30_000 });
let attempts = 0;
await testPage.route("**/api/v1/clarification/*/respond", async (route) => {
attempts += 1;
await route.fulfill({
status: 409,
contentType: "application/json",
body: JSON.stringify({ code: "not_active" }),
});
});
const inactiveResponse = waitForHttp(
testPage,
"POST",
/\/api\/v1\/clarification\/[^/]+\/respond$/,
);
await session.clarificationSkip().click();
await expect((await inactiveResponse).status()).toBe(409);
await expect(session.clarificationOverlay()).not.toBeVisible();
await expect(session.anyIdleInput()).toBeVisible();
await dwell(
testPage,
250,
"negative-assertion",
"observe that an inactive clarification is not submitted a second time",
);
expect(attempts).toBe(1);
});
test("late answer from an inactive historical question sends a new message", async ({
testPage,
apiClient,
seedData,
}) => {
test.setTimeout(60_000);
const session = await seedClarificationTask(
testPage,
apiClient,
seedData,
"Clarification Late Answer",
"clarification",
);
const sessionId = await activeSessionId(testPage);
if (!sessionId) throw new Error("expected an active session for late clarification answer");
let responseAttempts = 0;
await testPage.route("**/api/v1/clarification/*/respond", async (route) => {
responseAttempts += 1;
await route.fulfill({
status: 409,
contentType: "application/json",
body: JSON.stringify({ code: "not_active" }),
});
});
const inactiveResponse = waitForHttp(
testPage,
"POST",
/\/api\/v1\/clarification\/[^/]+\/respond$/,
);
await session.clarificationSkip().click();
await expect((await inactiveResponse).status()).toBe(409);
await expect(session.clarificationOverlay()).not.toBeVisible();
const answerAction = testPage.getByTestId("clarification-answer-as-new-message");
await expect(answerAction).toBeVisible({ timeout: 15_000 });
await answerAction.click();
await expect(testPage.getByTestId("clarification-late-submit")).toBeDisabled();
await testPage.getByTestId("clarification-option").filter({ hasText: "PostgreSQL" }).click();
await expect(testPage.getByTestId("clarification-late-submit")).toBeEnabled();
await testPage.getByTestId("clarification-late-submit").click();
await expect
.poll(
async () => {
const { messages } = await apiClient.listSessionMessages(sessionId);
return messages.some(
(message) =>
message.author_type === "user" &&
message.content.includes("Question 1") &&
message.content.includes("PostgreSQL"),
);
},
{ timeout: 30_000, message: "late clarification answer should be admitted as a message" },
)
.toBe(true);
expect(responseAttempts).toBe(1);
});
test("timeout detaches clarification and accepts a custom deferred answer", async ({
testPage,
apiClient,
apps/web/e2e/tests/chat/mobile-clarification.spec.ts
+108 -1
View File
@@ -1,6 +1,6 @@
import { test, expect } from "../../fixtures/test-base";
import { activeSessionId, seedClarificationSession } from "../../helpers/clarification";
import { watchWs } from "../../helpers/causal-waits";
import { dwell, waitForHttp, watchWs } from "../../helpers/causal-waits";
import { waitForSessionSettled } from "./quick-chat-helpers";
/**
@@ -219,6 +219,113 @@ test.describe("Mobile clarification multiline answer", () => {
expect(attempt).toBe(2);
});
test("inactive dismissal removes the stale panel and does not retry on mobile", async ({
testPage,
apiClient,
seedData,
}) => {
const session = await seedClarificationSession(
testPage,
apiClient,
seedData,
"Mobile Clarify Inactive Dismissal",
{ scenario: "clarification" },
);
await expect(session.clarificationOverlay()).toBeVisible({ timeout: 30_000 });
let attempts = 0;
await testPage.route("**/api/v1/clarification/*/respond", async (route) => {
attempts += 1;
await route.fulfill({
status: 409,
contentType: "application/json",
body: JSON.stringify({ code: "not_active" }),
});
});
const inactiveResponse = waitForHttp(
testPage,
"POST",
/\/api\/v1\/clarification\/[^/]+\/respond$/,
);
await session.clarificationSkip().tap();
await expect((await inactiveResponse).status()).toBe(409);
await expect(session.clarificationOverlay()).not.toBeVisible();
await expect(session.anyIdleInput()).toBeVisible();
await dwell(
testPage,
250,
"negative-assertion",
"observe that an inactive clarification is not submitted a second time on mobile",
);
expect(attempts).toBe(1);
});
test("late answer from an inactive historical question sends a new message on mobile", async ({
testPage,
apiClient,
seedData,
}) => {
test.setTimeout(60_000);
const session = await seedClarificationSession(
testPage,
apiClient,
seedData,
"Mobile Clarify Late Answer",
{ scenario: "clarification" },
);
const sessionId = await activeSessionId(testPage);
if (!sessionId) throw new Error("expected an active session for mobile late answer");
let responseAttempts = 0;
await testPage.route("**/api/v1/clarification/*/respond", async (route) => {
responseAttempts += 1;
await route.fulfill({
status: 409,
contentType: "application/json",
body: JSON.stringify({ code: "not_active" }),
});
});
const inactiveResponse = waitForHttp(
testPage,
"POST",
/\/api\/v1\/clarification\/[^/]+\/respond$/,
);
await session.clarificationSkip().tap();
await expect((await inactiveResponse).status()).toBe(409);
await expect(session.clarificationOverlay()).not.toBeVisible();
const answerAction = testPage.getByTestId("clarification-answer-as-new-message");
await expect(answerAction).toBeVisible({ timeout: 15_000 });
await answerAction.tap();
await expect(testPage.getByTestId("clarification-late-submit")).toBeDisabled();
await testPage.getByTestId("clarification-option").filter({ hasText: "PostgreSQL" }).tap();
await expect(testPage.getByTestId("clarification-late-submit")).toBeEnabled();
await testPage.getByTestId("clarification-late-submit").tap();
await expect
.poll(
async () => {
const { messages } = await apiClient.listSessionMessages(sessionId);
return messages.some(
(message) =>
message.author_type === "user" &&
message.content.includes("Question 1") &&
message.content.includes("PostgreSQL"),
);
},
{
timeout: 30_000,
message: "mobile late clarification answer should be admitted as a message",
},
)
.toBe(true);
expect(responseAttempts).toBe(1);
});
test("keeps the over-limit counter inside the phone viewport", async ({
testPage,
apiClient,