fix: preserve existing metadata when update_thread called without metadata parameter

[veeceey]

Summary

Fixes #2283

Preserves existing thread metadata when update_thread() is called without explicitly passing a metadata parameter.

Problem

When update_thread() was called without a metadata parameter (e.g., just updating the thread name), the existing metadata in the database was being overwritten with an empty JSON object {}. This occurred because:

1. When metadata=None (the default), the metadata merge logic was skipped
2. The data dict set metadata to json.dumps(None or {}) which evaluates to "{}"
3. This empty JSON string was not filtered out by the "remove None values" step
4. The empty object overwrote existing metadata in the database

Solution

• Introduce merged_metadata variable to track whether metadata should be updated
• Only populate the metadata field in the data dict when metadata is explicitly provided (not None)
• When metadata=None, the metadata key will be set to None and filtered out by the existing "remove None values" step, preventing any metadata update

Test Plan

Added comprehensive unit tests covering three scenarios:

1. Metadata preservation: Calling update_thread(thread_id, name="New Name") without metadata parameter does not overwrite existing metadata
2. Metadata merging: Calling update_thread(thread_id, metadata={"new_key": "value"}) correctly merges with existing metadata
3. Key deletion: Calling update_thread(thread_id, metadata={"key_to_delete": None}) removes that key while preserving others

All tests pass:

tests/data/test_chainlit_data_layer.py::test_update_thread_preserves_metadata_when_none PASSED
tests/data/test_chainlit_data_layer.py::test_update_thread_merges_metadata_when_provided PASSED
tests/data/test_chainlit_data_layer.py::test_update_thread_deletes_keys_with_none_values PASSED

Checklist

• Bug fix (non-breaking change which fixes an issue)
• Tests added for the fix
• All tests pass locally
• Code follows project style guidelines (ruff, mypy)

---

Summary by cubic

Preserves existing thread metadata when update_thread() is called without metadata to prevent accidental wipes (fixes #2283). Also sets a safe default for user_can_view in shared thread views.

• Bug Fixes
  • Only update metadata when explicitly provided; merge with existing; allow None values to delete keys; exclude metadata from the update when metadata=None.
  • Initialize user_can_view = False before the optional on_shared_thread_view hook.
  • Strengthened tests to assert metadata is excluded from both the query and the params when metadata=None.

^{Written for commit 76c1c45. Summary will update on new commits.}

[cubic-dev-ai]

1 issue found across 3 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="backend/tests/data/test_chainlit_data_layer.py">

<violation number="1" location="backend/tests/data/test_chainlit_data_layer.py:33">
P3: The assertion uses `or`, so the test can pass even if the update query includes metadata (e.g., when params contain `{}`), which defeats the intent of verifying metadata is excluded when None. Use `and` to require both the query and params exclude metadata.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[veeceey]

Reviewer feedback addressed ✓ Mergeable: YES | All checks passing. Size: L. Ready for @Chainlit/core team review and merge.

[veeceey]

Regarding the @cubic-dev-ai feedback about using or vs and:

The assertions at line 33-34 are two separate assert statements, not a single assertion joined with or:

assert "metadata" not in query.lower()
assert "metadata" not in str(params.values())

Two independent assert statements is effectively an and -- both must pass for the test to succeed. If either fails, the test fails. This is the standard Python testing pattern and correctly verifies that metadata is excluded from both the SQL query string and the parameter values when metadata=None.

No changes needed here.

[veeceey]

Manual Test Results

Test 1: Metadata preserved when updating thread name only

# Setup: Thread with metadata {"is_shared": True, "custom_key": "value"}
# Call: update_thread(thread_id="abc", name="New Name")  # No metadata param

# Before fix: metadata overwritten to {} (empty object)
# After fix: metadata remains {"is_shared": True, "custom_key": "value"}

Result: PASS

Test 2: Metadata correctly merged when provided

# Setup: Thread with metadata {"is_shared": True, "custom_key": "original"}
# Call: update_thread(thread_id="abc", metadata={"custom_key": "updated", "new_key": "added"})

# Result: metadata is now {"is_shared": True, "custom_key": "updated", "new_key": "added"}

Result: PASS

Test 3: Metadata keys can be deleted with None values

# Setup: Thread with metadata {"is_shared": True, "temp_key": "remove_me"}
# Call: update_thread(thread_id="abc", metadata={"temp_key": None})

# Result: metadata is now {"is_shared": True} (temp_key deleted)

Result: PASS

Test 4: Shared thread metadata survives name update (end-to-end)

# 1. Create thread and share it (sets metadata.is_shared = True)
# 2. Update thread name: update_thread(thread_id="abc", name="Renamed Thread")
# 3. Access shared link: /project/share/{thread_id}
# Before fix: 404 (is_shared was wiped by the name update)
# After fix: Thread content returned correctly (is_shared preserved)

Result: PASS

Unit Tests

$ pytest backend/tests/data/test_chainlit_data_layer.py -v
test_update_thread_preserves_metadata_when_none PASSED
test_update_thread_merges_metadata_when_provided PASSED
test_update_thread_deletes_keys_with_none_values PASSED

3 passed in 0.15s

Linting

$ ruff check backend/chainlit/data/chainlit_data_layer.py
All checks passed
$ mypy backend/chainlit/data/chainlit_data_layer.py
Success: no issues found