From 0ac92e28ab14f65786928a270a68af56416185b3 Mon Sep 17 00:00:00 2001
From: Classic298 <27028174+Classic298@users.noreply.github.com>
Date: Fri, 15 May 2026 23:09:53 +0200
Subject: [PATCH] fix: address 2nd review - presence-based merge, bidirectional
 edge, gated log

- Distinguish an omitted structural field from an explicitly provided one.
  The preservation loop now keys off membership (key not in message), so a
  partial streaming update still cannot drop childrenIds/timestamp/parentId
  it simply omitted, but a caller can again intentionally set parentId=None
  (detach/re-root) or childrenIds=[] (clear) on a node that has them.
- Keep the parent's forward childrenIds edge consistent with the child's
  parentId after upsert, so the graph is bidirectionally valid even when
  the same stale-write class also dropped the parent's child link.
- Only log at warning level when the created-through-upsert payload is
  actually partial (the lost-placeholder race); a complete create logs at
  debug, so normal upsert creation is not misleading warning telemetry.

Skipped the build_assistant_message_update -> _patch rename: "update"
already conveys a partial write and the rename only churns call sites.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 backend/open_webui/models/chats.py | 48 ++++++++++++++++++++----------
 1 file changed, 32 insertions(+), 16 deletions(-)

diff --git a/backend/open_webui/models/chats.py b/backend/open_webui/models/chats.py
index e7e2b3037e..9de4ba8fe6 100644
--- a/backend/open_webui/models/chats.py
+++ b/backend/open_webui/models/chats.py
@@ -586,16 +586,13 @@ class ChatTable:
         existing = messages.get(message_id)
         if existing is not None:
             merged = {**existing, **message}
-            # A partial update (done/content/output/usage) must not degrade
-            # structural fields the node already has: an empty or null
-            # incoming value never wins over a valid existing one.
-            for key in ('parentId', 'role', 'timestamp'):
-                if not merged.get(key) and existing.get(key):
+            # A partial update must not drop structural fields it simply
+            # omitted, but an explicitly provided value (including a falsy
+            # one such as parentId=None or childrenIds=[]) is intentional
+            # and must win.
+            for key in ('parentId', 'role', 'timestamp', 'childrenIds'):
+                if key not in message and key in existing:
                     merged[key] = existing[key]
-            # Only let childrenIds change when the caller actually sends one;
-            # an absent/empty incoming list must not wipe real children.
-            if not message.get('childrenIds') and existing.get('childrenIds'):
-                merged['childrenIds'] = existing['childrenIds']
             messages[message_id] = enforce_node_invariants(merged)
         else:
             # The target node does not exist. This upsert is also used for
@@ -603,15 +600,34 @@ class ChatTable:
             # placeholder already exists; if a concurrent whole-chat write
             # dropped it, synthesize a structurally valid node instead of
             # persisting the partial payload as a malformed history node.
-            incoming_role = message.get('role')
-            log.warning(
-                'upsert_message_to_chat_by_id_and_message_id: creating missing '
-                f'message node {message_id} from a partial payload '
-                f'(role={incoming_role!r}); an assistant placeholder was '
-                'likely lost by a concurrent chat write'
-            )
+            # Creating a node through an upsert is only anomalous when the
+            # payload is itself partial (the lost-placeholder race); a
+            # complete create-through-upsert is legitimate and must not spam
+            # warning-level telemetry.
+            is_partial = not all(k in message for k in ('id', 'parentId', 'childrenIds', 'role'))
+            if is_partial:
+                log.warning(
+                    'upsert_message_to_chat_by_id_and_message_id: creating '
+                    f'missing message node {message_id} from a partial '
+                    f'payload (role={message.get("role")!r}); an assistant '
+                    'placeholder was likely lost by a concurrent chat write'
+                )
+            else:
+                log.debug(
+                    'upsert_message_to_chat_by_id_and_message_id: creating '
+                    f'missing message node {message_id} from a complete payload'
+                )
             messages[message_id] = enforce_node_invariants({**message})
 
+        # Keep the parent's forward edge consistent with the child's
+        # parentId: the same stale-write class that drops a node can also
+        # drop its entry from the parent's childrenIds.
+        parent_id = messages[message_id].get('parentId')
+        if parent_id and isinstance(messages.get(parent_id), dict):
+            siblings = messages[parent_id].setdefault('childrenIds', [])
+            if message_id not in siblings:
+                siblings.append(message_id)
+
         history['currentId'] = message_id
 
         chat['history'] = history