github-starred/open-webui
2
0
Fork 0
mirror of https://github.com/open-webui/open-webui.git synced 2026-07-17 08:21:12 -05:00
Code Issues 2.4k Packages Projects Releases 129 Wiki Activity

[PR #24617] refac: Prevent malformed chat history nodes when assistant placeholders are lost during concurrent saves #131409

New Issue
Open
opened 2026-05-21 16:51:38 -05:00 by GiteaMirror · 0 comments
GiteaMirror commented 2026-05-21 16:51:38 -05:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/open-webui/open-webui/pull/24617
Author: @Classic298
Created: 5/12/2026
Status: 🔄 Open

Base: dev ← Head: fix-chat-history

📝 Commits (8)

  • 22d84d2 fix
  • 610e0b8 fix: enforce graph invariants on message upsert, stop clobbering existing nodes
  • 0ac92e2 fix: address 2nd review - presence-based merge, bidirectional edge, gated log
  • 1c2a713 fix: harden parent-edge repair, detach dangling parentId, trim comments
  • b7bd853 fix: address 3rd review - propagate parent to normalized row, drop destructive detach
  • 466883a fix: address 4th review - don't clear normalized parent, guard malformed node
  • ed03275 test: extract pure history-upsert helpers and add regression tests (#24758-style)
  • 824c961 Revert "test: extract pure history-upsert helpers and add regression tests (#24758-style)"

📊 Changes

2 files changed (+91 additions, -21 deletions)

View changed files

📝 backend/open_webui/models/chats.py (+63 -8)
📝 backend/open_webui/utils/middleware.py (+28 -13)

📄 Description

Summary
This PR hardens chat message persistence against a race condition where partial assistant-message updates can create malformed history nodes. It ensures that fallback upserts always create a valid Open WebUI message object and that final streaming/cancel saves include the required graph fields.

Background
Open WebUI stores chat history as a message graph inside chat.chat.history.messages. Each message node is expected to contain structural fields such as:

{
  "id": "...",
  "parentId": "...",
  "childrenIds": [],
  "role": "assistant",
  "content": "...",
  "done": true
}

A valid graph is required for the frontend to reconstruct the visible conversation by walking backward from history.currentId through each message’s parentId.

We observed a production chat export where the final assistant message existed under the correct message ID key, and the parent user message referenced it in childrenIds, but the assistant node itself only contained partial response fields:

{
  "done": true,
  "content": "...",
  "output": [...],
  "statusHistory": [...],
  "usage": {...}
}

It was missing id, parentId, childrenIds, and role. This made the chat import successfully as JSON, but the Open WebUI frontend never finished loading the conversation because the history graph was semantically broken.

Root Cause
Chats.upsert_message_to_chat_by_id_and_message_id() supports partial message updates. If the target message already exists, this is safe because it merges the partial update into the existing full message object:

history['messages'][message_id] = {
    **history['messages'][message_id],
    **message,
}

However, if the message does not exist, the old code stored the partial update as the complete message:

history['messages'][message_id] = message

That is unsafe because several callers, especially in streaming response handling, intentionally pass only partial fields such as:

{
    "done": True,
    "content": "...",
    "output": [...],
    "usage": {...}
}

Those payloads assume that an assistant placeholder already exists.

If that placeholder is missing, the partial payload becomes a malformed full history node.

When This Can Happen
This can happen under concurrent async chat-save conditions, especially with long-running streaming/tool-call responses.

A typical sequence is:

  1. The frontend creates a user message and an assistant placeholder.
  2. The backend persists the user message, links it to the assistant ID, and stores the assistant placeholder.
  3. The streaming response starts and emits multiple chat:completion, status, source, or tool-related events.
  4. Several backend paths update the same chat.chat JSON using read-modify-write.
  5. A concurrent background task, outlet filter, status update, source update, follow-up generation, or final response save can read an older copy of the full chat JSON and later write it back.
  6. If that stale write does not include the newest assistant placeholder, the placeholder can be lost.
  7. The final streaming save then calls upsert_message_to_chat_by_id_and_message_id() with only partial fields like done, content, output, and usage.
  8. Since the placeholder is missing, the old fallback branch writes this partial payload as the entire message object.
  9. The resulting chat has a broken graph: the parent points to the assistant message ID, but the assistant object lacks its own graph fields.

This is most likely under these conditions:

  • Streaming responses are enabled.
  • Tool calls are involved.
  • The response performs multiple tool iterations.
  • Background tasks such as title generation, follow-up generation, tags, outlet filters, or status updates run after the main response.
  • Multiple persistence updates touch the same chat.chat JSON field close together.
  • The user submits or queues another message soon after a response is marked done.
  • The chat has large sources, files, or output payloads, increasing update duration and race-window size.

Why It Matters
Malformed message nodes are hard to diagnose because:

  • The JSON remains syntactically valid.
  • The chat import/API request may succeed.
  • The browser may show no obvious console error.
  • The network tab may show successful responses.
  • The frontend can still receive a history.currentId, but the current message object is missing expected graph fields.
  • The UI can get stuck loading or render an incomplete conversation.

A broken node also makes exports/imports fragile and can require manual JSON surgery to recover the chat.

Changes
This PR makes two defensive changes.

  1. Chats.upsert_message_to_chat_by_id_and_message_id() now creates a valid message skeleton if the message ID does not already exist.

Instead of storing a partial payload directly, it now ensures the fallback message has:

{
    "id": message_id,
    "parentId": message.get("parentId"),
    "childrenIds": message.get("childrenIds", []),
    "role": message.get("role", "assistant"),
    "timestamp": message.get("timestamp", now),
    **message,
}
  1. Final streaming and cancellation saves in middleware.py now pass complete assistant-message graph fields.

Final assistant updates now include:

{
    "id": metadata["message_id"],
    "parentId": metadata["user_message_id"],
    "childrenIds": [],
    "role": "assistant",
    "model": model_id,
    "timestamp": int(time.time()),
    "done": True,
    "content": "...",
    "output": [...],
    "usage": {...}
}

This means the final save is safe even if the placeholder was lost before the final update.

Why This Is Safe
The normal path is unchanged: if the assistant placeholder exists, the upsert still merges into it.

The fallback path is now safer: if the placeholder is missing, the newly created message is at least structurally valid and can be traversed by the frontend.

Existing fields from the provided message payload still win because the skeleton is merged before **message.

Limitations
This PR does not fully eliminate all possible lost-update races on the whole chat.chat JSON field. A more complete long-term fix would be to avoid whole-document read-modify-write for per-message updates, for example by:

  • using row-level locking,
  • adding optimistic version checks and retries,
  • applying atomic JSONB patches,
  • or making the chat_message table the source of truth and rebuilding chat.chat.history from it.

However, this PR prevents the dangerous failure mode where a partial update becomes an invalid message node.

Validation
Validated by:

  • Reproducing the broken shape from an affected export.
  • Confirming the invalid node was missing id, parentId, childrenIds, and role.
  • Repairing the export by adding those fields, after which Open WebUI imported and loaded the chat successfully.
  • Compiling the modified Python files:
python -m compileall backend/open_webui/models/chats.py backend/open_webui/utils/middleware.py

Expected Outcome
After this change, even if concurrent chat updates temporarily lose an assistant placeholder, subsequent final/partial upserts will not persist a malformed message object. The chat history graph remains loadable and structurally valid.

Contributor License Agreement

Note

Deleting the CLA section will lead to immediate closure of your PR and it will not be merged in.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/open-webui/open-webui/pull/24617 **Author:** [@Classic298](https://github.com/Classic298) **Created:** 5/12/2026 **Status:** 🔄 Open **Base:** `dev` ← **Head:** `fix-chat-history` --- ### 📝 Commits (8) - [`22d84d2`](https://github.com/open-webui/open-webui/commit/22d84d250cb2940dd24d2892fb10ec8dc99123ab) fix - [`610e0b8`](https://github.com/open-webui/open-webui/commit/610e0b8fab7ad5065cbb37bc9f5dc2f9e4b1ee87) fix: enforce graph invariants on message upsert, stop clobbering existing nodes - [`0ac92e2`](https://github.com/open-webui/open-webui/commit/0ac92e28ab14f65786928a270a68af56416185b3) fix: address 2nd review - presence-based merge, bidirectional edge, gated log - [`1c2a713`](https://github.com/open-webui/open-webui/commit/1c2a71305ef9e78e6514685491ea45951f05b2e9) fix: harden parent-edge repair, detach dangling parentId, trim comments - [`b7bd853`](https://github.com/open-webui/open-webui/commit/b7bd853e381cbf7000d6ebe45c4b03b0bfaec802) fix: address 3rd review - propagate parent to normalized row, drop destructive detach - [`466883a`](https://github.com/open-webui/open-webui/commit/466883ad18a9466edbd5bc2a4fc9ab708bbe6f1c) fix: address 4th review - don't clear normalized parent, guard malformed node - [`ed03275`](https://github.com/open-webui/open-webui/commit/ed032753b929f1ca1e896bb99e83772836ff0b47) test: extract pure history-upsert helpers and add regression tests (#24758-style) - [`824c961`](https://github.com/open-webui/open-webui/commit/824c961a137d59e6061f6209f29a34e29f5b47d3) Revert "test: extract pure history-upsert helpers and add regression tests (#24758-style)" ### 📊 Changes **2 files changed** (+91 additions, -21 deletions) <details> <summary>View changed files</summary> 📝 `backend/open_webui/models/chats.py` (+63 -8) 📝 `backend/open_webui/utils/middleware.py` (+28 -13) </details> ### 📄 Description **Summary** This PR hardens chat message persistence against a race condition where partial assistant-message updates can create malformed history nodes. It ensures that fallback upserts always create a valid Open WebUI message object and that final streaming/cancel saves include the required graph fields. **Background** Open WebUI stores chat history as a message graph inside `chat.chat.history.messages`. Each message node is expected to contain structural fields such as: ```json { "id": "...", "parentId": "...", "childrenIds": [], "role": "assistant", "content": "...", "done": true } ``` A valid graph is required for the frontend to reconstruct the visible conversation by walking backward from `history.currentId` through each message’s `parentId`. We observed a production chat export where the final assistant message existed under the correct message ID key, and the parent user message referenced it in `childrenIds`, but the assistant node itself only contained partial response fields: ```json { "done": true, "content": "...", "output": [...], "statusHistory": [...], "usage": {...} } ``` It was missing `id`, `parentId`, `childrenIds`, and `role`. This made the chat import successfully as JSON, but the Open WebUI frontend never finished loading the conversation because the history graph was semantically broken. **Root Cause** `Chats.upsert_message_to_chat_by_id_and_message_id()` supports partial message updates. If the target message already exists, this is safe because it merges the partial update into the existing full message object: ```python history['messages'][message_id] = { **history['messages'][message_id], **message, } ``` However, if the message does not exist, the old code stored the partial update as the complete message: ```python history['messages'][message_id] = message ``` That is unsafe because several callers, especially in streaming response handling, intentionally pass only partial fields such as: ```python { "done": True, "content": "...", "output": [...], "usage": {...} } ``` Those payloads assume that an assistant placeholder already exists. If that placeholder is missing, the partial payload becomes a malformed full history node. **When This Can Happen** This can happen under concurrent async chat-save conditions, especially with long-running streaming/tool-call responses. A typical sequence is: 1. The frontend creates a user message and an assistant placeholder. 2. The backend persists the user message, links it to the assistant ID, and stores the assistant placeholder. 3. The streaming response starts and emits multiple `chat:completion`, `status`, `source`, or tool-related events. 4. Several backend paths update the same `chat.chat` JSON using read-modify-write. 5. A concurrent background task, outlet filter, status update, source update, follow-up generation, or final response save can read an older copy of the full chat JSON and later write it back. 6. If that stale write does not include the newest assistant placeholder, the placeholder can be lost. 7. The final streaming save then calls `upsert_message_to_chat_by_id_and_message_id()` with only partial fields like `done`, `content`, `output`, and `usage`. 8. Since the placeholder is missing, the old fallback branch writes this partial payload as the entire message object. 9. The resulting chat has a broken graph: the parent points to the assistant message ID, but the assistant object lacks its own graph fields. This is most likely under these conditions: - Streaming responses are enabled. - Tool calls are involved. - The response performs multiple tool iterations. - Background tasks such as title generation, follow-up generation, tags, outlet filters, or status updates run after the main response. - Multiple persistence updates touch the same `chat.chat` JSON field close together. - The user submits or queues another message soon after a response is marked done. - The chat has large `sources`, files, or output payloads, increasing update duration and race-window size. **Why It Matters** Malformed message nodes are hard to diagnose because: - The JSON remains syntactically valid. - The chat import/API request may succeed. - The browser may show no obvious console error. - The network tab may show successful responses. - The frontend can still receive a `history.currentId`, but the current message object is missing expected graph fields. - The UI can get stuck loading or render an incomplete conversation. A broken node also makes exports/imports fragile and can require manual JSON surgery to recover the chat. **Changes** This PR makes two defensive changes. 1. `Chats.upsert_message_to_chat_by_id_and_message_id()` now creates a valid message skeleton if the message ID does not already exist. Instead of storing a partial payload directly, it now ensures the fallback message has: ```python { "id": message_id, "parentId": message.get("parentId"), "childrenIds": message.get("childrenIds", []), "role": message.get("role", "assistant"), "timestamp": message.get("timestamp", now), **message, } ``` 2. Final streaming and cancellation saves in `middleware.py` now pass complete assistant-message graph fields. Final assistant updates now include: ```python { "id": metadata["message_id"], "parentId": metadata["user_message_id"], "childrenIds": [], "role": "assistant", "model": model_id, "timestamp": int(time.time()), "done": True, "content": "...", "output": [...], "usage": {...} } ``` This means the final save is safe even if the placeholder was lost before the final update. **Why This Is Safe** The normal path is unchanged: if the assistant placeholder exists, the upsert still merges into it. The fallback path is now safer: if the placeholder is missing, the newly created message is at least structurally valid and can be traversed by the frontend. Existing fields from the provided message payload still win because the skeleton is merged before `**message`. **Limitations** This PR does not fully eliminate all possible lost-update races on the whole `chat.chat` JSON field. A more complete long-term fix would be to avoid whole-document read-modify-write for per-message updates, for example by: - using row-level locking, - adding optimistic version checks and retries, - applying atomic JSONB patches, - or making the `chat_message` table the source of truth and rebuilding `chat.chat.history` from it. However, this PR prevents the dangerous failure mode where a partial update becomes an invalid message node. **Validation** Validated by: - Reproducing the broken shape from an affected export. - Confirming the invalid node was missing `id`, `parentId`, `childrenIds`, and `role`. - Repairing the export by adding those fields, after which Open WebUI imported and loaded the chat successfully. - Compiling the modified Python files: ```bash python -m compileall backend/open_webui/models/chats.py backend/open_webui/utils/middleware.py ``` **Expected Outcome** After this change, even if concurrent chat updates temporarily lose an assistant placeholder, subsequent final/partial upserts will not persist a malformed message object. The chat history graph remains loadable and structurally valid. ### Contributor License Agreement <!-- 🚨 DO NOT DELETE THE TEXT BELOW 🚨 Keep the "Contributor License Agreement" confirmation text intact. Deleting it will trigger the CLA-Bot to INVALIDATE your PR. Your PR will NOT be reviewed or merged until you check the box below confirming that you have read and agree to the terms of the CLA. --> - [x] By submitting this pull request, I confirm that I have read and fully agree to the [Contributor License Agreement (CLA)](https://github.com/open-webui/open-webui/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT), and I am providing my contributions under its terms. > [!NOTE] > Deleting the CLA section will lead to immediate closure of your PR and it will not be merged in. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2026-05-21 16:51:38 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
bug
confirmed
confirmed issue
core
documentation
enhancement
good first issue
help wanted
non-core
pull-request
Mirrored from GitHub Pull Request
 python
question
testing wanted
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
GiteaMirror ninjasurge
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/open-webui#131409
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?