镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

FEAT GUI: Preserve chat drafts when exploring datasets (#2750) - #3334

Merged
Roman Lutz (romanlutz) merged 7 commits into
microsoft:mainfrom
RohithPariki:auto-fix-2750
Oct 27, 2026
Merged

Roman Lutz (romanlutz) merged 7 commits into
microsoft:mainfrom
RohithPariki:auto-fix-2750

Conversation

@RohithPariki

@RohithPariki RohithPariki commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Description

Resolves #2750 (Item 6 of 17 in dataset epic #2744).

Navigating from Chat to Datasets to browse or inspect seed datasets previously caused unsubmitted chat drafts (including prompt text, media attachments, system prompts, active target selection, and converter configurations) to be lost upon leaving the chat route.

This PR implements robust chat draft preservation across navigation:

  • ChatDraftProvider & useChatDraftHandoff: Pure React context and custom hook managing preserved chat draft state and pending import payloads across client-side route transitions without storing drafts in URLs or databases.
  • Draft Preservation Lifecycle: When ChatWindow unmounts or navigates away, dirty drafts (containing user input, attachments, system prompt, or converter settings) are preserved alongside originating route metadata (distinguishing unsaved attacks null from persisted attacks attackId).
  • Single-Use Consumption: Preserved drafts and pending dataset imports are applied exactly once, preventing duplicate application or race conditions during re-renders, popstate events, or subsequent navigations.
  • Context Isolation: Destination validation ensures drafts belonging to one attack or conversation cannot leak into or overwrite an unrelated active conversation.
  • Memory & Resource Safety: Active monotonic sequence tracking guards against stale asynchronous completions. Superseded and cleared attachment blob URLs (blob:) are explicitly revoked via URL.revokeObjectURL while preserving non-blob assets (/api/media).
  • Bidirectional Route Restoration: Navigating back to Chat from Datasets restores the originating conversation and unsubmitted draft. Returning to Datasets from Chat remembers exact search queries and filter parameters (lastDatasetsLocation).

Tests and Documentation

  • Added comprehensive unit test suite in frontend/src/hooks/useChatDraftHandoff.test.tsx (22 tests) covering:
    • Dirty draft detection across text, media attachments, system prompt, and converters.
    • Context isolation and destination validation (new_attack, current_conversation, new_conversation).
    • Single-use consumption guarantees.
    • Superseded blob URL revocation and unmount cleanup.
  • Added route helper unit tests in frontend/src/utils/routeParams.test.ts for isDatasetsPath and isChatPath.
  • Added integration tests in frontend/src/App.test.tsx (92/92 passed):
    • Preserving unsaved chat draft across navigation to Datasets and back.
    • Preserving persisted attack conversation origin and restoring dataset filter query parameters.
  • Validated with:
    • npm run type-check (tsc --noEmit passed with 0 errors)
    • npm run lint (ESLint passed with 0 warnings, max-warnings 0)

Browser evidence

Recorded the actual, unmodified production frontend at commit cf4976b0b7b8f87222300f9724f470dfd45989fd in an isolated Chromium browser. All API responses, draft text, media, and datasets are synthetic fixtures. No live backend, credentials, or model calls were used. The dark captions are recording annotations, not application UI.

The 23-second walkthrough shows the current behavior:

  • Text, the system prompt, and the attachment chip survive a Chat → Datasets → Chat round trip. However, a browser fetch confirms that the attached image's blob URL is revoked during restoration, despite the chip remaining visible.
  • After returning to Chat and deleting all prompt text, another dataset round trip brings the deleted text back when no target is selected.
  • Returning to a dataset's second page loses its navigation history: Previous page becomes disabled, and Back to datasets drops the original catalog search.

These are observed regressions, not a claim that the feature passes end-to-end validation. This production recording does not exercise the separate development StrictMode cleanup-loop finding.

Walkthrough

pr3334-browser-walkthrough.webm

Restored draft snapshot

Chat after returning from Datasets, showing the restored synthetic prompt, system prompt, and attachment chip; the recording annotation notes the revoked image blob URL

Additional validation: npm run build passed. The Playwright browser walkthrough passed its observation assertions, confirming the successful restoration and the regressions described above. The uploaded WebM was decoded and seeked successfully in Chromium before attachment.

Comment thread frontend/src/App.tsx Resolved
@RohithPariki

Copy link
Copy Markdown
Contributor Author

Thanks Roman Lutz (@romanlutz) for the thorough review and guidance!

I have resolved all merge conflicts, rebased onto upstream/main, addressed both review items, and force-updated this PR in place (no duplicate PRs):

  1. [P1] Stop preservation effect from retriggering its own cleanup:

    • Preserving draft on unmount now uses a stable reference (preserveDraftRef) with an empty effect dependency array (useEffect(() => () => { ... }, [])), strictly firing on unmount.
    • Context values in useChatDraftHandoff are wrapped in useMemo.
    • Verified that running under root StrictMode and staging imports does not trigger cascading cleanup loops.
  2. [P2] Restore dataset history state along with URL:

    • lastDatasetsLocation and lastChatLocation in App.tsx now preserve both route path and location.state ({ path: string; state: unknown }).
    • ChatDraftContextState now includes originatingState?: unknown so location.state survives round-trip navigation.
    • Added integration test assertions in App.test.tsx verifying that dataset catalog path, filter query, and seed cursors in location.state survive navigation between Chat and Datasets.
  3. Validation & Checks:

    • All merge conflicts with upstream/main resolved cleanly.
    • npm run type-check (tsc --noEmit): 0 errors.
    • npm run lint (eslint): 0 errors / 0 warnings.
    • All tests passing: App.test.tsx, App.targets.test.tsx, useChatDraftHandoff.test.tsx, routeParams.test.ts.

@romanlutz

Copy link
Copy Markdown
Contributor

Thanks for the update, RohithPariki. I checked e7ac5fd and resolved the cleanup-loop and dataset-navigation threads.

These two issues still reproduce and their comments remain open:

Could you address these two as well?

@RohithPariki

Copy link
Copy Markdown
Contributor Author

Hi Roman Lutz (@romanlutz),

Thank you for the review and catching those subtle edge cases! I have addressed both issues in the latest push:

  1. Keep restored attachment URLs alive:

    • initialTextMetadata is now passed directly as a prop to ChatInputArea on mount from ChatWindow, eliminating the redundant inputBoxRef.current?.restoreDraft call during initial render.
    • changeAttachments in ChatInputArea now compares remaining URLs by string (!nextUrls.has(attachment.url)) rather than reference equality (!next.includes(attachment)), ensuring restored blob URLs are not revoked while they are still attached.
    • restoreDraft retains existing draftId properties on identified attachments.
    • Added unit tests in ChatInputArea.test.tsx verifying that initial attachments and identical draft restores retain their blob URLs and only revoke them upon explicit removal.
  2. Consume saved draft after handing back to composer:

    • Handing the preserved draft back to Chat now calls draftHandoff.consumePreservedDraft(attackResultId, viewedConversationId) inside the restoration effect.
    • Composer unmount preservation strictly requires isChatDraftDirty(current), and shouldApplyPreservedDraft requires !preserved || !isChatDraftDirty(preserved.draft).
    • When a user deletes all composer text and navigates away, the empty draft is not saved and the consumed snapshot is no longer present, so cleared drafts remain cleanly cleared across repeated round trips.
    • Added unit test coverage in useChatDraftHandoff.test.tsx and an end-to-end round trip test in App.test.tsx.

All checks (type-check, lint, and test suites) pass cleanly. Ready for your re-review!

Preserves chat input text, media attachments, system prompts, target configuration, and converters across navigation between Chat and Datasets. Implements single-use handoff consumption, context-switch protection, bidirectional route restoration, and explicit blob URL revocation.

@romanlutz Roman Lutz (romanlutz) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rechecked 4acdae8. The four earlier findings are fixed. The three remaining issues below still reproduce with the real App and composer.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 6192834b-f355-4da6-8cca-3f21ee95fc77
Restore the full composer only after its destination is ready. Preserve target and converter setup, text metadata, and attachment ownership. Guard stale asynchronous work and dirty-draft imports, and cover actual App navigation and recovery.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 6192834b-f355-4da6-8cca-3f21ee95fc77
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 6192834b-f355-4da6-8cca-3f21ee95fc77
Use a complete draft attachment list in the isolated App mock, and assert the loading gate while a revisited attack is unresolved. This lets the target revalidation complete without leaking a deferred mock into the next test.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 6192834b-f355-4da6-8cca-3f21ee95fc77
Separate routed draft identity from hydrated attack metadata and gate only handoff restoration. Retain the mounted composer through created-attack transitions and cover historical loading and failed-prompt recovery with the real App.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 6192834b-f355-4da6-8cca-3f21ee95fc77
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 6192834b-f355-4da6-8cca-3f21ee95fc77
@romanlutz
Roman Lutz (romanlutz) marked this pull request as ready for review October 27, 2026 01:22
@romanlutz
Roman Lutz (romanlutz) added this pull request to the merge queue Oct 27, 2026
Merged via the queue into microsoft:main with commit 8397c2a Oct 27, 2026
61 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FEAT GUI: Preserve chat drafts when exploring datasets

2 participants