From b7bd853e381cbf7000d6ebe45c4b03b0bfaec802 Mon Sep 17 00:00:00 2001 From: Classic298 <27028174+Classic298@users.noreply.github.com> Date: Fri, 15 May 2026 23:49:04 +0200 Subject: [PATCH] fix: address 3rd review - propagate parent to normalized row, drop destructive detach - Dual-write now aliases camelCase parentId to snake_case parent_id so the preferred normalized chat_message row gets the repaired parent link too (callee gates its update on 'parent_id'); without this the read path rebuilds from the unrepaired row and the fix is defeated. - No longer null a parentId just because the parent is absent from the possibly-stale embedded snapshot; the parent may still exist in chat_message, so detaching + dual-writing could permanently sever a real link. Log only. - Coerce non-str parentId to None so the link repair can't TypeError on a malformed (list/dict) value. - Tolerate a malformed/absent messages container instead of setdefault. - Only send model when non-empty so a final save can't clobber an existing model id with ''. - Drop the now-redundant preservation loop: {**existing, **message} already keeps omitted keys and lets explicit values win. - Synthesized-node warning states when role was defaulted. Co-Authored-By: Claude Opus 4.7 (1M context) --- backend/open_webui/models/chats.py | 38 ++++++++++++++------------ backend/open_webui/utils/middleware.py | 9 +++--- 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/backend/open_webui/models/chats.py b/backend/open_webui/models/chats.py index 2f2e21bdbd..e1f40dd168 100644 --- a/backend/open_webui/models/chats.py +++ b/backend/open_webui/models/chats.py @@ -564,12 +564,17 @@ class ChatTable: history = chat.get('history', {}) now = int(time.time()) - messages = history.setdefault('messages', {}) + # tolerate an absent or malformed messages container + messages = history.get('messages') + if not isinstance(messages, dict): + messages = {} + history['messages'] = messages def enforce_node_invariants(node: dict) -> dict: # Structural graph fields a partial update must never drop. node['id'] = message_id - node.setdefault('parentId', None) # explicit null, never undefined + if not isinstance(node.get('parentId'), str): + node['parentId'] = None # explicit null; never undefined/non-scalar if not isinstance(node.get('childrenIds'), list): node['childrenIds'] = [] if not node.get('role'): @@ -580,13 +585,9 @@ class ChatTable: existing = messages.get(message_id) if existing is not None: - merged = {**existing, **message} - # Omitted -> keep existing; explicit value (incl. parentId=None, - # childrenIds=[]) wins. Empty role / 0 ts still normalized above. - for key in ('parentId', 'role', 'timestamp', 'childrenIds'): - if key not in message and key in existing: - merged[key] = existing[key] - messages[message_id] = enforce_node_invariants(merged) + # {**existing, **message}: omitted keys keep existing, explicit + # values (incl. parentId=None, childrenIds=[]) win. + messages[message_id] = enforce_node_invariants({**existing, **message}) else: # Node missing: a concurrent whole-chat write likely dropped the # placeholder. Warn only when the payload is itself partial. @@ -594,7 +595,8 @@ class ChatTable: if is_partial: log.warning( f'upsert: synthesizing lost message node {message_id} ' - f'from partial payload (role={message.get("role")!r})' + f'from partial payload ' + f'(role={message.get("role") or "assistant (defaulted)"!r})' ) else: log.debug(f'upsert: creating message node {message_id}') @@ -613,24 +615,24 @@ class ChatTable: if message_id not in children_ids: children_ids.append(message_id) else: - # Dangling parentId re-bricks the frontend walk; detach. - log.warning( - f'upsert: message {message_id} references missing parent ' - f'{parent_id}; detaching to keep history loadable' - ) - messages[message_id]['parentId'] = None + # Parent absent from this possibly-stale snapshot: log only. + # It may still exist in chat_message; don't sever a real link. + log.warning(f'upsert: message {message_id} parent {parent_id} absent from embedded history') history['currentId'] = message_id chat['history'] = history - # Dual-write to chat_message table + # Dual-write to chat_message table. Alias camelCase parentId to + # snake_case so the normalized row's parent link is repaired too + # (callee gates its update on 'parent_id'). + node = history['messages'][message_id] try: await ChatMessages.upsert_message( message_id=message_id, chat_id=id, user_id=user_id, - data=history['messages'][message_id], + data={**node, 'parent_id': node.get('parentId')}, ) except Exception as e: log.warning(f'Failed to write to chat_message table: {e}') diff --git a/backend/open_webui/utils/middleware.py b/backend/open_webui/utils/middleware.py index 45cb9e52d1..f352295a39 100644 --- a/backend/open_webui/utils/middleware.py +++ b/backend/open_webui/utils/middleware.py @@ -3583,15 +3583,16 @@ async def streaming_chat_response_handler(response, ctx): model_id = form_data.get('model', '') def build_assistant_message_update(**fields): - # Only placeholder-repair fields; childrenIds/timestamp omitted so - # the upsert merge can't clobber an existing node (chats.py - # synthesizes them when the node is actually missing). + # Only placeholder-repair fields, and each only when authoritative, + # so the upsert merge can't clobber an existing node (chats.py + # synthesizes what's missing). update = { 'id': metadata['message_id'], 'role': 'assistant', - 'model': model_id, **fields, } + if model_id: + update['model'] = model_id parent_id = metadata.get('user_message_id') if parent_id: update['parentId'] = parent_id