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())