From 41f5a266ecc07f3bfef18e1ea37b95ad32815a6a Mon Sep 17 00:00:00 2001 From: Paul Lizer Date: Wed, 19 Aug 2026 11:27:27 -0400 Subject: [PATCH] Reset the conversation Documents pane on New chat Clicking New chat left the previous conversation's documents listed in the conversation side drawer, kept the header documents toggle and count badge visible, and prevented the drawer from closing. The Contents pane reset correctly, so the two modes behaved inconsistently. createNewConversation() fires chat:conversation-context-changed with a null conversation id before the new conversation exists, but refreshConversationDocuments() resolved a falsy id by falling back to getCurrentConversationId(). At that moment window.currentConversationId still pointed at the conversation being left, so the reset signal was executed as a refresh of the old conversation and its documents were re-rendered. refreshConversationDocuments() now accepts allowCurrentConversationFallback (default true), and the context-changed listener passes false so an empty id means reset rather than reload current. The existing empty-id branch already performed the full reset; it was simply unreachable. Conversation selection, the chat:conversation-documents-refresh path, and module init keep the fallback. Fixes #1298 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- application/single_app/config.py | 2 +- .../js/chat/chat-conversation-contents.js | 13 +- ...CONVERSATION_DOCUMENTS_DRAWER_RESET_FIX.md | 118 ++++++++++ docs/explanation/fixes/index.md | 1 + docs/explanation/release_notes.md | 10 + ...new_conversation_documents_drawer_reset.py | 211 ++++++++++++++++++ 6 files changed, 351 insertions(+), 4 deletions(-) create mode 100644 docs/explanation/fixes/NEW_CHAT_CONVERSATION_DOCUMENTS_DRAWER_RESET_FIX.md create mode 100644 functional_tests/test_chat_new_conversation_documents_drawer_reset.py diff --git a/application/single_app/config.py b/application/single_app/config.py index e3f9a3646..7a4b93be9 100644 --- a/application/single_app/config.py +++ b/application/single_app/config.py @@ -96,7 +96,7 @@ EXECUTOR_TYPE = 'thread' EXECUTOR_MAX_WORKERS = 30 SESSION_TYPE = 'filesystem' -VERSION = "0.260.003" +VERSION = "0.260.004" IS_DEVELOPMENT = is_development_env_enabled() SESSION_COOKIE_SAMESITE = os.getenv('SESSION_COOKIE_SAMESITE', 'Lax') diff --git a/application/single_app/static/js/chat/chat-conversation-contents.js b/application/single_app/static/js/chat/chat-conversation-contents.js index 3034d119e..c217c6668 100644 --- a/application/single_app/static/js/chat/chat-conversation-contents.js +++ b/application/single_app/static/js/chat/chat-conversation-contents.js @@ -567,7 +567,12 @@ function renderConversationDocuments(documents) { } async function refreshConversationDocuments(options = {}) { - const conversationId = String(options.conversationId || getCurrentConversationId()).trim(); + const requestedConversationId = String(options.conversationId || "").trim(); + // An empty id from an explicit context reset (New chat) must clear the pane + // instead of silently reloading the conversation the user is leaving. + const allowCurrentConversationFallback = options.allowCurrentConversationFallback !== false; + const conversationId = requestedConversationId + || (allowCurrentConversationFallback ? getCurrentConversationId() : ""); const conversationKind = String(options.conversationKind || "").trim(); const requestToken = ++metadataRequestToken; const autoOpen = Boolean(options.autoOpen); @@ -735,10 +740,12 @@ function initializeConversationContents() { } }); window.addEventListener("chat:conversation-context-changed", event => { + const detail = event.detail || {}; void refreshConversationDocuments({ - conversationId: event.detail?.conversationId || "", + conversationId: detail.conversationId || "", + allowCurrentConversationFallback: false, autoOpen: false, - conversationKind: event.detail?.conversationKind || "", + conversationKind: detail.conversationKind || "", }); }); window.addEventListener("chat:conversation-documents-refresh", event => { diff --git a/docs/explanation/fixes/NEW_CHAT_CONVERSATION_DOCUMENTS_DRAWER_RESET_FIX.md b/docs/explanation/fixes/NEW_CHAT_CONVERSATION_DOCUMENTS_DRAWER_RESET_FIX.md new file mode 100644 index 000000000..a9a9c3fe8 --- /dev/null +++ b/docs/explanation/fixes/NEW_CHAT_CONVERSATION_DOCUMENTS_DRAWER_RESET_FIX.md @@ -0,0 +1,118 @@ +# New Chat Conversation Documents Drawer Reset Fix + +Fixed in version: **0.260.004** + +Associated issue: [#1298](https://github.com/microsoft/simplechat/issues/1298) + +## Issue Description + +The chat conversation side drawer has two modes, **Contents** and **Documents**. Clicking **New chat** correctly emptied the **Contents** pane and closed the drawer, but the **Documents** pane kept showing the documents from the conversation the user had just left. + +Observed symptoms: + +- Documents from the previous conversation remained listed after starting a new chat. +- The header documents toggle (`#conversation-documents-toggle`) stayed visible. +- The `#conversation-documents-count` badge kept the stale count. +- The drawer stayed open instead of closing. +- Switching between existing conversations behaved correctly; only the **New chat** path was affected. + +## Root Cause Analysis + +`createNewConversation()` in `chat-conversations.js` signals the reset **before** the new conversation exists, so the event carries a null id: + +```js +notifyConversationContextChanged("new", null, { preserveSelections }); +``` + +The drawer listener in `chat-conversation-contents.js` forwarded that as an empty string, and `refreshConversationDocuments()` resolved the conversation id with a fallback: + +```js +const conversationId = String(options.conversationId || getCurrentConversationId()).trim(); +``` + +At that moment `window.currentConversationId` still pointed at the **previous** conversation, because it is only reassigned after the `/api/create_conversation` response returns. The falsy id therefore fell through to the old conversation, and the "reset" signal was executed as "refresh the conversation I am already on": the old metadata was re-fetched and the old documents were re-rendered. + +Because `documentEntries` never became empty, `updateDrawerTriggers()` never reached its terminal branch: + +```js +if (!hasContents && !hasDocuments) { + closeDrawer({ restoreFocus: false }); +} +``` + +so the toggle, the count badge, and the open drawer all persisted. + +The **Contents** pane reset correctly only because it is driven by a completely different mechanism: the `MutationObserver` on `#chatbox` reacting to `chatbox.innerHTML = ""` inside `createNewConversation()`. + +## Technical Details + +### Files Modified + +- `application/single_app/static/js/chat/chat-conversation-contents.js` +- `application/single_app/config.py` +- `functional_tests/test_chat_new_conversation_documents_drawer_reset.py` +- `docs/explanation/release_notes.md` + +### Code Changes Summary + +- Added an `allowCurrentConversationFallback` option (default `true`) to `refreshConversationDocuments()` so a caller can require an explicit conversation id instead of silently resolving the active one. +- Updated the `chat:conversation-context-changed` listener to pass `allowCurrentConversationFallback: false`, so a null/empty id now means *reset* rather than *reload current*. +- No new reset logic was needed: the existing `if (!conversationId)` branch already clears `documentEntries`, resets the usage-tracking flags, clears the status line, shows the empty state, and calls `updateDrawerTriggers()`. The fix simply makes that branch reachable. +- Updated `config.py` to version `0.260.004` for this fix. + +### Behavior Preserved + +The current-conversation fallback is intentionally retained for the callers that depend on it: + +- The module's initial `refreshConversationDocuments()` call on page load. +- The `chat:conversation-documents-refresh` event path used by streaming, retry, message edit, and collaboration. +- `chat:conversation-context-changed` with `reason: "select"`, which always supplies an explicit id. + +A second context-changed event is deliberately **not** dispatched after the new conversation is created. Doing so would re-trigger the workspace, prompt, and toolbar reset listeners and wipe selections the user made immediately after starting the new chat. A brand-new conversation has no cited documents, and the first response already fires `chat:conversation-documents-refresh` from `chat-streaming.js`. + +## Validation + +### Test Results + +`functional_tests/test_chat_new_conversation_documents_drawer_reset.py` — 7/7 checks passed: + +- New chat still dispatches the context reset with a null conversation id. +- `refreshConversationDocuments()` exposes the fallback opt-out and no longer contains the unconditional fallback expression. +- The context-changed listener opts out of the fallback. +- The targeted document-refresh path and module init still resolve the active conversation. +- The empty-id branch clears document state and lets `updateDrawerTriggers()` close the drawer and hide the toggle/badge. +- The drawer markup still exposes every element the reset updates. +- `config.py` version is at least `0.260.004`. + +A jsdom runtime harness exercised the real module against both the pre-fix and post-fix source: + +| Check after clicking New chat | Before fix | After fix | +| --- | --- | --- | +| Documents pane empty | FAIL (2 entries) | PASS | +| Contents pane empty | PASS | PASS | +| Documents toggle hidden | FAIL | PASS | +| Documents count badge hidden | FAIL | PASS | +| Drawer closed | FAIL | PASS | +| No metadata refetch for the conversation being left | FAIL (1 fetch for `conv-a`) | PASS | +| Selecting another conversation still loads its documents | PASS | PASS | + +### Regression Coverage + +- `functional_tests/test_chat_cited_source_tracking.py` — passed. +- `functional_tests/test_chat_new_conversation_action_state_reset.py` — passed. +- `functional_tests/test_conversation_contents_drawer_settings.py` and `functional_tests/test_chat_new_conversation_tag_reset.py` fail on this branch, but both were verified to fail identically on the unmodified baseline and are unrelated to this change (a `route_backend_users.py` test-namespace gap and a `chat-documents.js` assertion drift respectively). + +### Before/After Comparison + +| Step | Before | After | +| --- | --- | --- | +| Open a conversation with cited documents | Documents pane lists them | Unchanged | +| Open the drawer in Documents mode | Drawer opens | Unchanged | +| Click **New chat** | Documents stay listed, toggle and badge stay visible, drawer stays open | Documents pane empties, toggle and badge hide, drawer closes | +| Switch to an existing conversation | Documents reload | Unchanged | + +### User Experience Improvements + +- A new chat no longer appears to already have documents associated with it. +- The **Documents** pane now behaves consistently with the **Contents** pane on **New chat**. +- One redundant conversation-metadata request per **New chat** click is eliminated. diff --git a/docs/explanation/fixes/index.md b/docs/explanation/fixes/index.md index e80b770e4..8bfda582f 100644 --- a/docs/explanation/fixes/index.md +++ b/docs/explanation/fixes/index.md @@ -32,3 +32,4 @@ category: Version History - [Workflow Task Document Picker Fix](WORKFLOW_TASK_DOCUMENT_PICKER_FIX.md) - [Workflow File Sync Prompt Context Fix](WORKFLOW_FILE_SYNC_PROMPT_CONTEXT_FIX.md) - [Chat Citation Whitespace Collapse Fix](CHAT_CITATION_WHITESPACE_COLLAPSE_FIX.md) +- [New Chat Conversation Documents Drawer Reset Fix](NEW_CHAT_CONVERSATION_DOCUMENTS_DRAWER_RESET_FIX.md) diff --git a/docs/explanation/release_notes.md b/docs/explanation/release_notes.md index 2c9f83afd..3acfc9949 100644 --- a/docs/explanation/release_notes.md +++ b/docs/explanation/release_notes.md @@ -2,6 +2,16 @@ For feature-focused and fix-focused drill-downs by version, see [Features by Version](/explanation/features/) and [Fixes by Version](/explanation/fixes/). +### **(v0.260.004)** + +#### Bug Fixes + +* **New Chat Now Clears The Conversation Documents Side Pane** + * Fixed the conversation side drawer keeping the previous conversation's documents after clicking **New chat**. The stale list, the header documents toggle, and its count badge all stayed visible, and the drawer would not close. + * Root cause was the New chat reset signal carrying a null conversation id while `window.currentConversationId` still pointed at the conversation being left, so the drawer fell back to the old conversation and re-fetched its documents instead of clearing. The **Contents** pane was unaffected because it resets from a separate chatbox observer. + * The **Documents** pane now empties out and the drawer closes, matching **Contents** behavior. Switching between existing conversations is unchanged, and one redundant conversation-metadata request per New chat click is eliminated. + * (Ref: `chat-conversation-contents.js`, `refreshConversationDocuments`, `chat:conversation-context-changed`, `updateDrawerTriggers`, Fixes #1298) + ### **(v0.260.003)** #### Bug Fixes diff --git a/functional_tests/test_chat_new_conversation_documents_drawer_reset.py b/functional_tests/test_chat_new_conversation_documents_drawer_reset.py new file mode 100644 index 000000000..db16d02ab --- /dev/null +++ b/functional_tests/test_chat_new_conversation_documents_drawer_reset.py @@ -0,0 +1,211 @@ +#!/usr/bin/env python3 +# test_chat_new_conversation_documents_drawer_reset.py +""" +Functional test for the New chat reset of the conversation Documents side pane. +Version: 0.260.004 +Implemented in: 0.260.004 + +This test ensures that clicking New chat clears the conversation side drawer's +Documents pane instead of re-rendering the previous conversation's documents, +so the pane empties out and closes just like the Contents pane. + +Refs: https://github.com/microsoft/simplechat/issues/1298 +""" + +import os +import sys + +sys.path.append(os.path.dirname(os.path.abspath(__file__))) + +from test_support.versioning import assert_app_version_at_least + + +ROOT_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +APP_ROOT = os.path.join(ROOT_DIR, "application", "single_app") + +CONVERSATION_CONTENTS_JS = os.path.join( + APP_ROOT, "static", "js", "chat", "chat-conversation-contents.js" +) +CONVERSATIONS_JS = os.path.join(APP_ROOT, "static", "js", "chat", "chat-conversations.js") +CHATS_TEMPLATE_FILE = os.path.join(APP_ROOT, "templates", "chats.html") + + +def read_file(path): + """Read a source file as UTF-8 text.""" + with open(path, "r", encoding="utf-8") as file_handle: + return file_handle.read() + + +def test_new_conversation_dispatches_context_reset_without_id(): + """Verify New chat still signals a context reset with no conversation id.""" + print("Testing New chat context reset dispatch...") + source = read_file(CONVERSATIONS_JS) + + required_snippets = [ + 'window.dispatchEvent(new CustomEvent("chat:conversation-context-changed", {', + 'notifyConversationContextChanged("new", null, { preserveSelections });', + 'notifyConversationContextChanged("select", conversationId);', + ] + + missing = [snippet for snippet in required_snippets if snippet not in source] + assert not missing, f"Missing new-conversation reset dispatch snippets: {missing}" + + print("New chat context reset dispatch verified.") + return True + + +def test_documents_refresh_supports_fallback_opt_out(): + """Verify refreshConversationDocuments can require an explicit conversation id.""" + print("Testing conversation documents fallback opt-out...") + source = read_file(CONVERSATION_CONTENTS_JS) + + required_snippets = [ + 'const requestedConversationId = String(options.conversationId || "").trim();', + "const allowCurrentConversationFallback = options.allowCurrentConversationFallback !== false;", + "const conversationId = requestedConversationId", + '|| (allowCurrentConversationFallback ? getCurrentConversationId() : "");', + ] + + missing = [snippet for snippet in required_snippets if snippet not in source] + assert not missing, f"Missing fallback opt-out snippets: {missing}" + + assert ( + "String(options.conversationId || getCurrentConversationId()).trim()" not in source + ), ( + "refreshConversationDocuments must not silently fall back to the current " + "conversation when an explicit reset supplies an empty conversation id." + ) + + print("Conversation documents fallback opt-out verified.") + return True + + +def test_context_change_listener_resets_documents_pane(): + """Verify the context-changed listener opts out of the current-conversation fallback.""" + print("Testing context-changed listener reset wiring...") + source = read_file(CONVERSATION_CONTENTS_JS) + + listener_start = source.find('window.addEventListener("chat:conversation-context-changed"') + assert listener_start != -1, "chat:conversation-context-changed listener not found." + + listener_end = source.find('window.addEventListener("chat:conversation-documents-refresh"') + assert listener_end > listener_start, ( + "chat:conversation-documents-refresh listener should follow the context-changed listener." + ) + + listener_source = source[listener_start:listener_end] + required_snippets = [ + 'conversationId: detail.conversationId || "",', + "allowCurrentConversationFallback: false,", + "autoOpen: false,", + ] + + missing = [snippet for snippet in required_snippets if snippet not in listener_source] + assert not missing, f"Missing context-changed listener snippets: {missing}" + + print("Context-changed listener reset wiring verified.") + return True + + +def test_documents_refresh_path_still_uses_current_conversation(): + """Verify targeted document refreshes keep resolving the active conversation.""" + print("Testing document refresh fallback retention...") + source = read_file(CONVERSATION_CONTENTS_JS) + + required_snippets = [ + 'const conversationId = String(event.detail?.conversationId || getCurrentConversationId()).trim();', + "if (!conversationId || conversationId !== getCurrentConversationId()) {", + "void refreshConversationDocuments();", + ] + + missing = [snippet for snippet in required_snippets if snippet not in source] + assert not missing, f"Missing document refresh fallback snippets: {missing}" + + print("Document refresh fallback retention verified.") + return True + + +def test_empty_conversation_clears_documents_and_closes_drawer(): + """Verify the empty-id path clears document state and lets the drawer close.""" + print("Testing empty conversation reset behavior...") + source = read_file(CONVERSATION_CONTENTS_JS) + + required_snippets = [ + "if (!conversationId) {", + 'documentLoadState = "idle";', + "documentUsageTrackingAvailable = false;", + "exactDocumentUsageAvailable = false;", + "legacyDocumentFallbackAvailable = false;", + "renderConversationDocuments([]);", + "documentEntries = documents.map(createDocumentEntry);", + "if (!hasContents && !hasDocuments) {", + "closeDrawer({ restoreFocus: false });", + 'documentsCountBadge.classList.toggle("d-none", !hasDocuments);', + 'documentsToggleButton.classList.toggle("d-none", !hasDocuments);', + ] + + missing = [snippet for snippet in required_snippets if snippet not in source] + assert not missing, f"Missing empty conversation reset snippets: {missing}" + + print("Empty conversation reset behavior verified.") + return True + + +def test_drawer_markup_supports_documents_reset_surfaces(): + """Verify the drawer markup still exposes the elements the reset updates.""" + print("Testing conversation drawer markup...") + source = read_file(CHATS_TEMPLATE_FILE) + + required_snippets = [ + 'id="conversation-documents-toggle"', + 'id="conversation-documents-count"', + 'id="conversation-documents-list"', + 'id="conversation-documents-empty"', + 'id="conversation-contents-drawer"', + ] + + missing = [snippet for snippet in required_snippets if snippet not in source] + assert not missing, f"Missing conversation drawer markup snippets: {missing}" + + print("Conversation drawer markup verified.") + return True + + +def test_config_version_updated(): + """Verify config.py reflects the documents drawer reset implementation version.""" + print("Testing config.py version update...") + assert_app_version_at_least("0.260.004") + print("Config version update verified.") + return True + + +def main(): + """Run all regression checks.""" + tests = [ + test_new_conversation_dispatches_context_reset_without_id, + test_documents_refresh_supports_fallback_opt_out, + test_context_change_listener_resets_documents_pane, + test_documents_refresh_path_still_uses_current_conversation, + test_empty_conversation_clears_documents_and_closes_drawer, + test_drawer_markup_supports_documents_reset_surfaces, + test_config_version_updated, + ] + results = [] + + for test in tests: + print(f"\nRunning {test.__name__}...") + try: + results.append(test()) + except Exception as exc: + print(f"Test failed: {exc}") + import traceback + traceback.print_exc() + results.append(False) + + success = all(results) + print(f"\nResults: {sum(1 for result in results if result)}/{len(results)} tests passed") + return 0 if success else 1 + + +if __name__ == "__main__": + sys.exit(main())