Repository navigation
FEAT GUI: Import dataset prompts and conversations into chat (#2751) - #3670
Nimit Jain (Nimit3418) wants to merge 5 commits into
Conversation
| const hasGroupIds = promptRecords.some((m) => Boolean(m.prompt_group_id)) | ||
|
|
||
| if (hasGroupIds) { | ||
| const groupMap = new Map<string, SeedRecord[]>() |
There was a problem hiding this comment.
[P1] Could we group turns by sequence rather than prompt_group_id? SeedGroup assigns the same group ID to every member of an example, including different conversation turns. For example, (group=g, sequence=0, role=user) followed by (group=g, sequence=1, role=assistant) is a normal stored conversation, but this code puts both records in one turn and rejects their roles as conflicting. Two user turns in the same group are instead merged into one message. The tests use different group IDs for different turns, which does not match the dataset API's example grouping. Please cover one shared group ID with multiple sequences.
| messages: consumed.conversation.messages, | ||
| objective: consumed.conversation.objective ?? currentObjective, | ||
| initialObjective: consumed.conversation.objective ?? currentObjective, | ||
| sourceConversationId: null, |
There was a problem hiding this comment.
[P1] This loses conversation imports into a saved conversation. The existing effect above discards an editor whenever editDraft.sourceConversationId !== viewedConversationId. Here the source is always null, so importing into a saved conversation consumes the pending import, opens the editor, and immediately discards it on the next effect run. Please associate the editor with its actual destination and add a test that imports while /attacks/<id>/conversations/<id> is loaded, rather than only testing /chat.
| } else if (destination === 'new_conversation' && effectiveAttackId) { | ||
| navigate(attackRoutePath(effectiveAttackId)) | ||
| } | ||
| }, |
There was a problem hiding this comment.
[P1] Navigating to the attack root does not create a new conversation. App resolves /attacks/<id> to that attack's main conversation, but canApplyPendingImport requires conversationId == null for a new_conversation import without a target conversation ID. Once the attack loads, that condition cannot be met, so this menu action never applies the import. Please create an explicit destination conversation and stage/navigate using its ID, with a test that starts from an already saved attack.
| textMetadata: outcome.textMetadata, | ||
| }, | ||
| focus: true, | ||
| replaceDirty: false, |
There was a problem hiding this comment.
[P1] Please handle the existing draft before navigating. For example: type an unsent prompt in /chat, visit a dataset, then choose Import to new attack. The old prompt is restored, and canApplyImportNow rejects this import because the composer is dirty and replaceDirty is false. There is no visible pending-import control or explanation, so the action appears to do nothing. The same issue affects a current conversation with an unsent prompt. A keep/replace choice or an explicit error before navigation would preserve the old draft without silently blocking the requested import.
| const consumed = draftHandoff.consumePendingImport(pending.id, draftAttackId, draftConversationId) | ||
| if (!consumed) return | ||
| converters.clearAll() | ||
| if (consumed.conversation) { |
There was a problem hiding this comment.
[P1] Imported turns need to start as unsaved changes, not as the editor's clean baseline. useConversationDraft.begin() sets baselineMessages to these same messages and initialObjective to the same objective, so editor.dirty is false immediately after import. On a clean /chat, importing a conversation and then clicking outside its controls calls handleExitEditor, which discards a clean editor instead of saving it. The imported turns disappear without a warning, and navigation is not blocked either. Please protect imported content before the first manual edit and test leaving an untouched imported editor.
| initialObjective: consumed.conversation.initialObjective ?? currentObjective, | ||
| sourceConversationId: consumed.conversation.sourceConversationId ?? null, | ||
| sourceAttackId: consumed.conversation.sourceAttackId ?? draftAttackId, | ||
| target: activeTarget, |
There was a problem hiding this comment.
[P2] Could we preserve the editor's target and temperature along with its messages? The snapshot stores conversation.temperature, but this restore never passes it back and editor.begin() resets it to ''. It also restores the main chat target rather than the editor's independently selected target, which is not captured in the snapshot. A preserved editor configured for target B at temperature 0.8 can therefore come back configured for target A with no override. Please round-trip those settings against the current registry instead of silently changing the conversation's target configuration.
| converters.clearAll() | ||
| if (consumed.conversation) { | ||
| editor.begin({ | ||
| messages: consumed.conversation.messages, |
There was a problem hiding this comment.
[P1] Importing into the current conversation needs to retain its existing turns. The pending payload contains only the dataset's messages, and this replaces the editor contents with those messages without loading or prepending the destination history. Saving a conversation serializes the editor messages; it does not append them to source_conversation_id. After the source-ID discard issue is fixed, this path would still save a conversation containing only the imported example, losing the context the user chose to continue. Please cover a destination with existing messages and verify they remain before the imported turns.
|
I tried this on head Shouldn't the stored template text appear in the editable input box after this action? I would expect the import to populate it without sending anything. Am I missing a required step here? I have not confirmed the cause of this particular failure. |
… import - Group conversation turns by sequence rather than prompt_group_id (microsoft#3670) - Populate composer with template prompt and avoid collision with clean preserved drafts - Add ConfirmDialog before navigation when destination has an unsent dirty draft - Prepend imported turns to existing history when importing into current conversation - Associate editor with viewed conversation ID to prevent lifecycle discard - Start imported turns as unsaved changes to protect against accidental dismissal - Round-trip editor target and temperature in conversation edit state - Create explicit conversation before navigating for new_conversation destination - Add comprehensive integration tests covering template import from clean chat
…nd target validation - Preserve destination existing messages first and append imported history after them in current_conversation - Handle history loading errors gracefully without consuming pending import - Cover dirty-draft protection and replace confirmation across new_attack, current_conversation, and new_conversation - Track unsaved imported state explicitly in useConversationDraft lifecycle - Validate and report missing conversation targets against registry without silent fallback - Add comprehensive user journey tests for all import flows and edge cases
…and editor lifecycle
|
I have addressed all the feedback and resolved the root cause of the empty input box shown in your screenshot. The changes are organized across the following 3 commits: Commit Breakdown & Issues Addressed1.
|

Description
Closes #2751. Implements work item 7 of 17 in #2744: importing dataset prompts, multimodal messages, and multi-turn conversations into the chat interface without manual copy-pasting, reattaching media, or issuing extra model calls.
Key Changes
Shared Contract (
types/index.ts):PendingImportPayloadwith an optional typedconversation?: PendingImportConversationcarrying ordered turns, objective metadata, and system prompt.ChatDraftContentwithconversation?: ConversationEditStateto protect in-progress conversation-editor drafts across navigation round-trips.Pure Data Transformation (
utils/seedImport.ts):buildSeedImport(members)to transform dataset detailSeedRecord[]into either a single composer prompt or a multi-turn conversation draft.seed_type === 'objective'into objective metadata rather than conversation turns.simulated_conversationconfigs with explicit explanation.prompt_group_idandsequence.simulated_assistant,simulated_tool).source_piece_id.Draft Handoff & Preservation (
utils/chatDraft.ts,hooks/useChatDraftHandoff.tsx):isChatDraftDirtyandhasChatDraftStatenow recognize active conversation drafts.canApplyPendingImportvalidates all three destinations:new_attack,current_conversation, andnew_conversation.Datasets UI (
SeedExampleDetail.tsx,SeedExampleBrowser.tsx):Chat View (
ChatWindow.tsx):ConversationEditordrafts on unmount when exploring Datasets.editor.begin().Documentation (
doc/gui/0_gui.md):Test Coverage
src/utils/seedImport.test.ts(11 tests).useChatDraftHandoff.test.tsx,SeedExampleDetail.test.tsx, andApp.drafts.test.tsxfor round-trip integration across all destinations.npx tsc --noEmitpassed with 0 errors.npm run lintpassed with 0 errors, 0 warnings.