From 49338b5f656ffa2d3348e67b42d7d51bcdb28817 Mon Sep 17 00:00:00 2001 From: Varun Chawla Date: Sat, 7 Feb 2026 11:20:15 -0800 Subject: [PATCH 1/5] fix: initialize user_can_view before conditional hook check --- backend/chainlit/server.py | 1 + 1 file changed, 1 insertion(+) diff --git a/backend/chainlit/server.py b/backend/chainlit/server.py index 6accf1ccc4..f9393e5e3b 100644 --- a/backend/chainlit/server.py +++ b/backend/chainlit/server.py @@ -1012,6 +1012,7 @@ async def get_shared_thread( if not isinstance(metadata, dict): metadata = {} + user_can_view = False if getattr(config.code, "on_shared_thread_view", None): try: user_can_view = await config.code.on_shared_thread_view( From 7da71c9f73d42bed9065a042eefcca11c6b907da Mon Sep 17 00:00:00 2001 From: Varun Chawla Date: Sat, 7 Feb 2026 17:23:37 -0800 Subject: [PATCH 2/5] fix: preserve existing metadata when update_thread called without metadata parameter Fixes #2283 When update_thread() is called without explicitly passing a metadata parameter (e.g., just updating the thread name), the existing metadata in the database was being overwritten with an empty object. This occurred because: 1. When metadata=None (the default), the merge logic was skipped 2. The data dict set metadata to json.dumps(None or {}) which is "{}" 3. This empty JSON object overwrote existing metadata in the database The fix: - Introduce merged_metadata variable to track whether metadata should be updated - Only set metadata in the data dict when metadata is explicitly provided - When metadata=None, it will be excluded from the update query via the existing "remove None values" filter Tested with three scenarios: - update_thread with metadata=None preserves existing metadata - update_thread with metadata dict properly merges with existing - update_thread with metadata keys set to None deletes those keys --- backend/chainlit/data/chainlit_data_layer.py | 7 +- .../tests/data/test_chainlit_data_layer.py | 140 ++++++++++++++++++ 2 files changed, 145 insertions(+), 2 deletions(-) create mode 100644 backend/tests/data/test_chainlit_data_layer.py diff --git a/backend/chainlit/data/chainlit_data_layer.py b/backend/chainlit/data/chainlit_data_layer.py index d2fb2d9f62..c2efd27193 100644 --- a/backend/chainlit/data/chainlit_data_layer.py +++ b/backend/chainlit/data/chainlit_data_layer.py @@ -591,6 +591,7 @@ async def update_thread( ) # Merge incoming metadata with existing metadata, deleting incoming keys with None values + merged_metadata = None if metadata is not None: existing = await self.execute_query( 'SELECT "metadata" FROM "Thread" WHERE id = $1', @@ -609,14 +610,16 @@ async def update_thread( to_delete = {k for k, v in metadata.items() if v is None} incoming = {k: v for k, v in metadata.items() if v is not None} base = {k: v for k, v in base.items() if k not in to_delete} - metadata = {**base, **incoming} + merged_metadata = {**base, **incoming} data = { "id": thread_id, "name": thread_name, "userId": user_id, "tags": tags, - "metadata": json.dumps(metadata or {}), + "metadata": json.dumps(merged_metadata) + if merged_metadata is not None + else None, "updatedAt": datetime.now(), } diff --git a/backend/tests/data/test_chainlit_data_layer.py b/backend/tests/data/test_chainlit_data_layer.py new file mode 100644 index 0000000000..822cbde3a2 --- /dev/null +++ b/backend/tests/data/test_chainlit_data_layer.py @@ -0,0 +1,140 @@ +import json +from unittest.mock import AsyncMock + +import pytest + +from chainlit.data.chainlit_data_layer import ChainlitDataLayer + + +@pytest.mark.asyncio +async def test_update_thread_preserves_metadata_when_none(): + """Test that update_thread does not overwrite existing metadata when metadata=None.""" + # Create a mock data layer + data_layer = ChainlitDataLayer( + database_url="postgresql://test", storage_client=None, show_logger=False + ) + + # Mock the execute_query method + data_layer.execute_query = AsyncMock() + + # Simulate calling update_thread with only a name, metadata=None (default) + await data_layer.update_thread(thread_id="test-thread-123", name="Updated Name") + + # Verify execute_query was called + assert data_layer.execute_query.called + + # Get the query and params from the call + call_args = data_layer.execute_query.call_args + query = call_args[0][0] + params = call_args[0][1] + + # The query should NOT include metadata in the update + # because metadata was None and should be excluded from the data dict + assert "metadata" not in query.lower() or "metadata" not in str(params.values()) + + +@pytest.mark.asyncio +async def test_update_thread_merges_metadata_when_provided(): + """Test that update_thread merges metadata correctly when provided.""" + # Create a mock data layer + data_layer = ChainlitDataLayer( + database_url="postgresql://test", storage_client=None, show_logger=False + ) + + # Mock the execute_query method to return existing metadata + existing_metadata = {"is_shared": True, "custom_field": "original"} + + async def mock_execute_query(query, params): + if "SELECT" in query and "metadata" in query: + # Return existing thread metadata + return [{"metadata": json.dumps(existing_metadata)}] + # For the UPDATE/INSERT, return None + return None + + data_layer.execute_query = AsyncMock(side_effect=mock_execute_query) + + # Call update_thread with partial metadata update + new_metadata = {"custom_field": "updated", "new_field": "added"} + await data_layer.update_thread( + thread_id="test-thread-123", name="Updated Name", metadata=new_metadata + ) + + # Verify execute_query was called twice (once for SELECT, once for UPDATE) + assert data_layer.execute_query.call_count == 2 + + # Get the UPDATE call + update_call = data_layer.execute_query.call_args_list[1] + update_params = update_call[0][1] + + # The metadata should be merged + # Expected: {"is_shared": True, "custom_field": "updated", "new_field": "added"} + # Find the JSON metadata in the params + metadata_json = None + for value in update_params.values(): + if isinstance(value, str) and value.startswith("{"): + try: + metadata_json = json.loads(value) + break + except json.JSONDecodeError: + pass + + assert metadata_json is not None + assert metadata_json.get("is_shared") is True + assert metadata_json.get("custom_field") == "updated" + assert metadata_json.get("new_field") == "added" + + +@pytest.mark.asyncio +async def test_update_thread_deletes_keys_with_none_values(): + """Test that update_thread deletes keys when value is None.""" + # Create a mock data layer + data_layer = ChainlitDataLayer( + database_url="postgresql://test", storage_client=None, show_logger=False + ) + + # Mock the execute_query method to return existing metadata + existing_metadata = { + "is_shared": True, + "to_delete": "will be removed", + "keep": "stays", + } + + async def mock_execute_query(query, params): + if "SELECT" in query and "metadata" in query: + # Return existing thread metadata + return [{"metadata": json.dumps(existing_metadata)}] + # For the UPDATE/INSERT, return None + return None + + data_layer.execute_query = AsyncMock(side_effect=mock_execute_query) + + # Call update_thread with None value to delete a key + new_metadata = {"to_delete": None, "new_field": "added"} + await data_layer.update_thread(thread_id="test-thread-123", metadata=new_metadata) + + # Verify execute_query was called twice + assert data_layer.execute_query.call_count == 2 + + # Get the UPDATE call + update_call = data_layer.execute_query.call_args_list[1] + update_params = update_call[0][1] + + # The metadata should have deleted "to_delete" key and added "new_field" + # Expected: {"is_shared": True, "keep": "stays", "new_field": "added"} + metadata_json = None + for value in update_params.values(): + if isinstance(value, str) and value.startswith("{"): + try: + metadata_json = json.loads(value) + break + except json.JSONDecodeError: + pass + + if metadata_json: + # Verify "to_delete" is not in the merged metadata + assert "to_delete" not in metadata_json + # Verify "new_field" was added + assert metadata_json.get("new_field") == "added" + # Verify "is_shared" and "keep" are preserved + assert metadata_json.get("is_shared") is True + assert metadata_json.get("keep") == "stays" From 76c1c458be95da54818c7d980ed85967e23172cb Mon Sep 17 00:00:00 2001 From: Varun Chawla Date: Sat, 7 Feb 2026 20:17:27 -0800 Subject: [PATCH 3/5] test: fix test logic - use separate assertions instead of 'or' operator The original test used 'or' which allowed it to pass even if metadata was present in either query or params. Split into two separate assertions to ensure metadata is excluded from both query and params. --- backend/tests/data/test_chainlit_data_layer.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/backend/tests/data/test_chainlit_data_layer.py b/backend/tests/data/test_chainlit_data_layer.py index 822cbde3a2..6ec61dc223 100644 --- a/backend/tests/data/test_chainlit_data_layer.py +++ b/backend/tests/data/test_chainlit_data_layer.py @@ -30,7 +30,8 @@ async def test_update_thread_preserves_metadata_when_none(): # The query should NOT include metadata in the update # because metadata was None and should be excluded from the data dict - assert "metadata" not in query.lower() or "metadata" not in str(params.values()) + assert "metadata" not in query.lower() + assert "metadata" not in str(params.values()) @pytest.mark.asyncio From 7c5fb35b1df04d163d081b978668ac61dced6534 Mon Sep 17 00:00:00 2001 From: Varun Chawla Date: Mon, 23 Feb 2026 23:04:16 -0800 Subject: [PATCH 4/5] fix: remove out-of-scope user_can_view initialization from server.py Revert the user_can_view = False initialization as it is unrelated to the metadata preservation fix and should be handled separately. --- backend/chainlit/server.py | 1 - 1 file changed, 1 deletion(-) diff --git a/backend/chainlit/server.py b/backend/chainlit/server.py index f9393e5e3b..6accf1ccc4 100644 --- a/backend/chainlit/server.py +++ b/backend/chainlit/server.py @@ -1012,7 +1012,6 @@ async def get_shared_thread( if not isinstance(metadata, dict): metadata = {} - user_can_view = False if getattr(config.code, "on_shared_thread_view", None): try: user_can_view = await config.code.on_shared_thread_view( From cf44f799efd20e32f15a38185d1f0ffef5e33371 Mon Sep 17 00:00:00 2001 From: Varun Chawla Date: Mon, 23 Feb 2026 23:30:44 -0800 Subject: [PATCH 5/5] fix: initialize user_can_view before conditional hook check Prevents UnboundLocalError when no on_shared_thread_view callback is configured. Without this initialization, accessing a shared thread when the callback is absent causes a 500 instead of the expected 404. --- backend/chainlit/server.py | 1 + 1 file changed, 1 insertion(+) diff --git a/backend/chainlit/server.py b/backend/chainlit/server.py index 6accf1ccc4..f9393e5e3b 100644 --- a/backend/chainlit/server.py +++ b/backend/chainlit/server.py @@ -1012,6 +1012,7 @@ async def get_shared_thread( if not isinstance(metadata, dict): metadata = {} + user_can_view = False if getattr(config.code, "on_shared_thread_view", None): try: user_can_view = await config.code.on_shared_thread_view(