fix: address presentation review findings
This commit is contained in:
+34
-4
@@ -1,15 +1,16 @@
|
||||
import { cleanup, render, screen } from "@testing-library/react";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { cleanup, render, screen, within } from "@testing-library/react";
|
||||
import userEvent from "@testing-library/user-event";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { findBeat, findScene } from "../storyboard.js";
|
||||
import { PreparedAuthoringLifecycleScene } from "./PreparedAuthoringLifecycleScene.js";
|
||||
|
||||
afterEach(() => cleanup());
|
||||
|
||||
const renderBeat = (beatId: string) => {
|
||||
const renderBeat = (beatId: string, onAdvance?: () => void) => {
|
||||
const scene = findScene("prepared-lifecycle");
|
||||
const beat = findBeat("prepared-lifecycle", beatId);
|
||||
if (!scene || !beat) throw new Error(`missing prepared-lifecycle/${beatId}`);
|
||||
return render(<PreparedAuthoringLifecycleScene scene={scene} beat={beat} />);
|
||||
return render(<PreparedAuthoringLifecycleScene scene={scene} beat={beat} onAdvance={onAdvance} />);
|
||||
};
|
||||
|
||||
describe("PreparedAuthoringLifecycleScene", () => {
|
||||
@@ -85,6 +86,35 @@ describe("PreparedAuthoringLifecycleScene", () => {
|
||||
expect(workspace.querySelectorAll('[data-visual-role="lifecycle-primary"]')).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("projects a custom discover submission into the prepared conversation", async () => {
|
||||
const user = userEvent.setup();
|
||||
renderBeat("discover");
|
||||
|
||||
const input = screen.getByRole("textbox", { name: /message to authoring assistant/i });
|
||||
await user.type(input, "Inspect the report source first.");
|
||||
await user.click(screen.getByRole("button", { name: /send message/i }));
|
||||
|
||||
const conversation = screen.getByRole("log", { name: "prepared authoring conversation" });
|
||||
expect(within(conversation).getByText("Inspect the report source first.")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it.each([
|
||||
["draft", true],
|
||||
["artifact", true],
|
||||
["validate", false],
|
||||
["deployment", false],
|
||||
] as const)("%s submission advances only when its beat owns the transition", async (beatId, advances) => {
|
||||
const user = userEvent.setup();
|
||||
const onAdvance = vi.fn();
|
||||
renderBeat(beatId, onAdvance);
|
||||
|
||||
const input = screen.getByRole("textbox", { name: /message to authoring assistant/i });
|
||||
if (beatId === "validate") await user.type(input, "Review the validation result.");
|
||||
await user.click(screen.getByRole("button", { name: /send message/i }));
|
||||
|
||||
expect(onAdvance).toHaveBeenCalledTimes(advances ? 1 : 0);
|
||||
});
|
||||
|
||||
it.each([
|
||||
["discover", "Discover"],
|
||||
["draft", "Draft"],
|
||||
|
||||
@@ -63,6 +63,9 @@ export const PreparedAuthoringLifecycleScene = ({ scene, beat, onAdvance }: Prep
|
||||
onDraftChange={(draft) => dispatch({ type: "draft_edited", draft })}
|
||||
onSubmit={(submittedText) => {
|
||||
dispatch({ type: "draft_edited", draft: submittedText });
|
||||
if (beatId === "discover") {
|
||||
dispatch({ type: "discover_submitted" });
|
||||
}
|
||||
if (beatId === "draft") {
|
||||
dispatch({ type: "draft_submitted" });
|
||||
onAdvance?.();
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useEffect, useState, type FormEvent, type KeyboardEvent } from "react";
|
||||
import { useEffect, useState } from "react";
|
||||
import { Button } from "../../components/ui/button.js";
|
||||
import { Textarea } from "../../components/ui/textarea.js";
|
||||
import { AuthoringConversation } from "./AuthoringConversation.js";
|
||||
@@ -7,6 +7,7 @@ import type {
|
||||
Scene9MessageProjection,
|
||||
Scene9SubmittedOverrides,
|
||||
} from "./scene9-message-state.js";
|
||||
import { PREPARED_COMPOSER_HELP, usePreparedComposerSubmit } from "./usePreparedComposerSubmit.js";
|
||||
|
||||
export type PresentationAssistantPaneProps = {
|
||||
readonly phase: AuthoringPhaseId;
|
||||
@@ -52,16 +53,10 @@ export const PresentationAssistantPane = ({
|
||||
setDraft(nextDraft);
|
||||
onDraftChange(nextDraft);
|
||||
};
|
||||
const submit = (event?: FormEvent<HTMLFormElement>) => {
|
||||
event?.preventDefault();
|
||||
if (canSubmit) onSubmit(draft);
|
||||
};
|
||||
const handleKeyDown = (event: KeyboardEvent<HTMLTextAreaElement>) => {
|
||||
if (event.key === "Enter" && !event.shiftKey) {
|
||||
event.preventDefault();
|
||||
submit();
|
||||
}
|
||||
};
|
||||
const { submit, handleKeyDown } = usePreparedComposerSubmit(
|
||||
canSubmit,
|
||||
() => onSubmit(draft),
|
||||
);
|
||||
|
||||
return (
|
||||
<aside
|
||||
@@ -104,7 +99,7 @@ export const PresentationAssistantPane = ({
|
||||
</Button>
|
||||
</div>
|
||||
<p id="scene9-authoring-message-help" className="presentation-assistant-pane__composer-help">
|
||||
Shift+Enter adds a new line. This is a deterministic prepared replay, not a live model request.
|
||||
{PREPARED_COMPOSER_HELP}
|
||||
</p>
|
||||
{runRequested !== null ? (
|
||||
<p role="status" className="presentation-assistant-pane__run-status">
|
||||
|
||||
@@ -1,4 +1,3 @@
|
||||
import type { FormEvent } from "react";
|
||||
import { Button } from "../../components/ui/button.js";
|
||||
import { Textarea } from "../../components/ui/textarea.js";
|
||||
import { AuthoringConversation } from "./AuthoringConversation.js";
|
||||
@@ -7,6 +6,7 @@ import {
|
||||
type Scene8EntryAction,
|
||||
type Scene8EntryState,
|
||||
} from "./scene8-entry-state.js";
|
||||
import { PREPARED_COMPOSER_HELP, usePreparedComposerSubmit } from "./usePreparedComposerSubmit.js";
|
||||
|
||||
type Scene8ChatEntryProps = {
|
||||
readonly state: Scene8EntryState;
|
||||
@@ -17,11 +17,10 @@ type Scene8ChatEntryProps = {
|
||||
export const Scene8ChatEntry = ({ state, dispatch }: Scene8ChatEntryProps) => {
|
||||
const submitted = state.phase === "submitted";
|
||||
const canSubmit = canSubmitScene8Entry(state);
|
||||
|
||||
const submit = (event?: FormEvent<HTMLFormElement>) => {
|
||||
event?.preventDefault();
|
||||
if (canSubmit) dispatch({ type: "submit" });
|
||||
};
|
||||
const { submit, handleKeyDown } = usePreparedComposerSubmit(
|
||||
canSubmit,
|
||||
() => dispatch({ type: "submit" }),
|
||||
);
|
||||
|
||||
return (
|
||||
<section
|
||||
@@ -43,12 +42,7 @@ export const Scene8ChatEntry = ({ state, dispatch }: Scene8ChatEntryProps) => {
|
||||
value={state.draft}
|
||||
disabled={submitted}
|
||||
onChange={(event) => dispatch({ type: "draft_changed", draft: event.target.value })}
|
||||
onKeyDown={(event) => {
|
||||
if (event.key === "Enter" && !event.shiftKey) {
|
||||
event.preventDefault();
|
||||
submit();
|
||||
}
|
||||
}}
|
||||
onKeyDown={handleKeyDown}
|
||||
aria-describedby="scene8-authoring-request-help"
|
||||
/>
|
||||
<div className="agent-handoff-scene__composer-actions">
|
||||
@@ -57,7 +51,7 @@ export const Scene8ChatEntry = ({ state, dispatch }: Scene8ChatEntryProps) => {
|
||||
</Button>
|
||||
</div>
|
||||
<p id="scene8-authoring-request-help" className="agent-handoff-scene__composer-help">
|
||||
Shift+Enter adds a new line. This is a deterministic prepared replay, not a live model request.
|
||||
{PREPARED_COMPOSER_HELP}
|
||||
</p>
|
||||
</form>
|
||||
{submitted ? (
|
||||
|
||||
@@ -205,4 +205,20 @@ describe("projectPreparedAuthoringThread", () => {
|
||||
canonical.filter((message) => message.id.endsWith("-tools")),
|
||||
);
|
||||
});
|
||||
|
||||
it("keeps each staged override scoped to its phase", () => {
|
||||
const messages = projectPreparedAuthoringThread("deployment", undefined, {
|
||||
validate: "Edited validation request",
|
||||
deployment: "Edited deployment request",
|
||||
});
|
||||
const userText = messages
|
||||
.filter((message) => message.role === "user")
|
||||
.map((message) => message.parts[0]?.type === "text" ? message.parts[0].text : "");
|
||||
|
||||
expect(userText).toEqual([
|
||||
"We need to author a report workflow for the lda_report scenario. What sources and capabilities are available?",
|
||||
"Edited validation request",
|
||||
"Edited deployment request",
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -281,28 +281,40 @@ export const authoringToolGroupId = (phase: AuthoringPhaseId): string =>
|
||||
export const projectPreparedAuthoringThread = (
|
||||
throughPhase: AuthoringPhaseId = "deployment",
|
||||
requestOverride?: string,
|
||||
requestOverrides?: Readonly<Partial<Record<"validate" | "deployment", string>>>,
|
||||
requestOverrides?: Readonly<Partial<Record<"discover" | "validate" | "deployment", string>>>,
|
||||
): readonly AgentMessage[] => {
|
||||
const finalPhaseIndex = recording.findIndex(({ phase }) => phase === throughPhase);
|
||||
if (finalPhaseIndex < 0) throw new Error(`unknown phase: ${throughPhase}`);
|
||||
|
||||
let requestReplaced = false;
|
||||
const phaseOverrideApplied = new Set<AuthoringPhaseId>();
|
||||
return recording.slice(0, finalPhaseIndex + 1).flatMap((phase) => {
|
||||
const phaseOverride =
|
||||
phase.phase === "validate"
|
||||
phase.phase === "discover"
|
||||
? requestOverrides?.discover
|
||||
: phase.phase === "validate"
|
||||
? requestOverrides?.validate
|
||||
: phase.phase === "deployment"
|
||||
? requestOverrides?.deployment
|
||||
: undefined;
|
||||
const conversation = phase.conversation.map((turn, index) => {
|
||||
const shouldReplaceRequest =
|
||||
turn.role === "user" &&
|
||||
(phaseOverride !== undefined || (requestOverride !== undefined && !requestReplaced));
|
||||
if (shouldReplaceRequest && phaseOverride === undefined) requestReplaced = true;
|
||||
let replacement: string | undefined;
|
||||
if (turn.role === "user" && phaseOverride !== undefined && !phaseOverrideApplied.has(phase.phase)) {
|
||||
replacement = phaseOverride;
|
||||
phaseOverrideApplied.add(phase.phase);
|
||||
} else if (
|
||||
turn.role === "user"
|
||||
&& phaseOverride === undefined
|
||||
&& requestOverride !== undefined
|
||||
&& !requestReplaced
|
||||
) {
|
||||
replacement = requestOverride;
|
||||
requestReplaced = true;
|
||||
}
|
||||
return agentTextMessage(
|
||||
`authoring-${phase.phase}-message-${index}`,
|
||||
turn.role,
|
||||
shouldReplaceRequest ? (phaseOverride ?? requestOverride ?? turn.text) : turn.text,
|
||||
replacement ?? turn.text,
|
||||
);
|
||||
});
|
||||
const groupId = authoringToolGroupId(phase.phase);
|
||||
|
||||
@@ -69,6 +69,19 @@ describe("scene 9 staged message state", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("stores discover submissions under the discover destination", () => {
|
||||
const state = scene9MessageReducer(initialScene9MessageState, {
|
||||
type: "draft_edited",
|
||||
draft: "Inspect the report source first.",
|
||||
});
|
||||
|
||||
expect(scene9MessageReducer(state, { type: "discover_submitted" })).toEqual({
|
||||
draft: state.draft,
|
||||
submittedOverrides: { discover: state.draft },
|
||||
runRequested: null,
|
||||
});
|
||||
});
|
||||
|
||||
it("ignores blank submits and keeps duplicate submits idempotent", () => {
|
||||
const blank = { ...initialScene9MessageState, draft: " \n\t" };
|
||||
expect(scene9MessageReducer(blank, { type: "draft_submitted" })).toBe(blank);
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import type { AuthoringPhaseId } from "./authoring-recording.js";
|
||||
|
||||
export type Scene9MessagePhase = AuthoringPhaseId;
|
||||
export type Scene9DestinationPhase = "validate" | "deployment";
|
||||
export type Scene9DestinationPhase = "discover" | "validate" | "deployment";
|
||||
|
||||
export const SCENE9_PHASE_PROMPTS: Readonly<Record<Scene9MessagePhase, string>> = {
|
||||
discover: "",
|
||||
@@ -38,6 +38,7 @@ export const initialScene9MessageState: Scene9MessageState = {
|
||||
|
||||
export type Scene9MessageAction =
|
||||
| { readonly type: "draft_edited"; readonly draft: string }
|
||||
| { readonly type: "discover_submitted" }
|
||||
| { readonly type: "draft_submitted" }
|
||||
| { readonly type: "artifact_submitted" }
|
||||
| { readonly type: "run_requested" };
|
||||
@@ -66,6 +67,8 @@ export const scene9MessageReducer = (
|
||||
switch (action.type) {
|
||||
case "draft_edited":
|
||||
return state.runRequested === null ? { ...state, draft: action.draft } : state;
|
||||
case "discover_submitted":
|
||||
return submitOverride(state, "discover");
|
||||
case "draft_submitted":
|
||||
return submitOverride(state, "validate");
|
||||
case "artifact_submitted":
|
||||
@@ -92,7 +95,7 @@ export const projectScene9Message = (
|
||||
placeholder: SCENE9_PHASE_PLACEHOLDERS[phase],
|
||||
});
|
||||
|
||||
/** Keeps the transcript-facing map limited to the two staged handoff destinations. */
|
||||
/** Returns the transcript-facing request overrides by destination phase. */
|
||||
export const projectScene9SubmittedOverrides = (
|
||||
state: Scene9MessageState,
|
||||
): Scene9SubmittedOverrides => state.submittedOverrides;
|
||||
|
||||
@@ -0,0 +1,28 @@
|
||||
import type { FormEvent, KeyboardEvent } from "react";
|
||||
|
||||
type PreparedComposerSubmitHandlers = {
|
||||
readonly submit: (event?: FormEvent<HTMLFormElement>) => void;
|
||||
readonly handleKeyDown: (event: KeyboardEvent<HTMLTextAreaElement>) => void;
|
||||
};
|
||||
|
||||
export const PREPARED_COMPOSER_HELP =
|
||||
"Shift+Enter adds a new line. This is a deterministic prepared replay, not a live model request.";
|
||||
|
||||
/** Shares the prepared-replay composer keyboard and form submission contract. */
|
||||
export const usePreparedComposerSubmit = (
|
||||
canSubmit: boolean,
|
||||
onSubmit: () => void,
|
||||
): PreparedComposerSubmitHandlers => {
|
||||
const submit = (event?: FormEvent<HTMLFormElement>) => {
|
||||
event?.preventDefault();
|
||||
if (canSubmit) onSubmit();
|
||||
};
|
||||
const handleKeyDown = (event: KeyboardEvent<HTMLTextAreaElement>) => {
|
||||
if (event.key === "Enter" && !event.shiftKey) {
|
||||
event.preventDefault();
|
||||
submit();
|
||||
}
|
||||
};
|
||||
|
||||
return { submit, handleKeyDown };
|
||||
};
|
||||
Reference in New Issue
Block a user