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) <noreply@anthropic.com>
This commit is contained in:
Classic298 2026-05-15 23:49:04 +02:00
parent 1c2a71305e
commit b7bd853e38
2 changed files with 25 additions and 22 deletions

View file

@ -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}')

View file

@ -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