Skip to content

fix: After posting a new question within an existing conversation in the portal, the conversation thread did not move to the top of the list - #7890

Merged
peslyn merged 1 commit into
v3from
pr@v3@fix_portal_conversation
Nov 13, 2026
Merged

peslyn merged 1 commit into
v3from
pr@v3@fix_portal_conversation

Conversation

@shaohuzhang1

Copy link
Copy Markdown
Contributor

fix: After posting a new question within an existing conversation in the portal, the conversation thread did not move to the top of the list

After posting a new question within an existing conversation in the portal, the conversation thread did not move to the top of the list
@peslyn peslyn changed the title fix: fix: After posting a new question within an existing conversation in the portal, the conversation thread did not move to the top of the list Nov 13, 2026
@peslyn
peslyn merged commit f500430 into v3 Nov 13, 2026
3 of 4 checks passed
}
}

sendMessage({

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is an analysis of the provided code snippet, focusing on Reactivity, Performance, Logic, and Best Practices.

1. Reactivity & State Management (Critical Issue)

Issue: Direct mutation of object properties (conversation.update_time = ...) inside a reactive list often fails to trigger component re-renders in Vue (depending on version) if not handled correctly. While Vue 3's reactive proxies handle deep mutations usually well, mutating an item inside an array and then moving it via splice/unshift relies on specific dependency tracking.

  • Risk: If conversations.value is a ref containing plain objects (not deeply reactive), changing item.update_time directly might not notify watchers or computeds tracking that specific property.
  • Suggestion: Ensure conversations.value items are reactive. In Vue 3, it is safer and more explicit to replace the item in the array or create a new object instance rather than mutating the existing reference in place before moving it.

2. Performance & Immutability

Issue: The code uses indexOf to find the index after finding the object reference with .find(). This is redundant.

  • Explanation: You already have the conversation object. However, indexOf compares by reference. If conversations.value contains reactive proxies and conversation is also a proxy, this works. But splice(1) + unshift() creates two array manipulations.
  • Optimization: You can avoid indexOf by using findIndex initially, which returns both the object and its position.

3. Code Logic & Edge Cases

Issue: indexOf(conversation)

  • Context: The find method returns the actual item from the array. If conversations.value is an array of reactive objects, find returns the reactive proxy from the array. indexOf will correctly find it if the array itself is reactive and contains those exact proxies.
  • However: A cleaner approach with Array.prototype.toSorted (ES2023) or manual sorting is inefficient for large lists. Moving an item to the front via splice + unshift is O(N) due to indexOf and splice. For chat history (often long), this is acceptable but could be optimized with a Linked List or ID-keyed Map for O(1) moves if the list is very large (>10k items).

4. Time Handling

Issue: new Date().toISOString()

  • Consistency: Ensure that update_time format strictly matches backend expectations. Using ISO string for local UI display logic assumes timezone consistency. It is often better to store Unix timestamps (seconds/milliseconds) for comparison/ordering logic and only convert to strings for display.

5. Suggested Refactored Code (Optimized for Readability & Safety)

@@ -229,7 +229,8 @@ export function createMessageInputStore(deps: MessageInputDeps) {
   const isFirstMessage = messages.value.length === 0;
   const abstract = questionText.substring(0, 256);
   
-  if (isFirstMessage && !conversations.value.some((conversation) => conversation.id === chatId)) {
+  // Use findIndex to get both existence check and position in one pass
+  const existingIndex = conversations.value.findIndex((item) => item.id === chatId);
+  
+  if (isFirstMessage && existingIndex === -1) {
     // 本地新会话尚未由历史接口返回,使用首次发送时间展示相对时间。
     const createTime = new Date().toISOString();
     conversations.value.unshift({
@@ -238,6 +239,18 @@ export function createMessageInputStore(deps: MessageInputDeps) {
       create_time: createTime,
       update_time: createTime,
     });
+  } else if (existingIndex !== -1) {
+    // 在已有会话中继续提问:移到顶部并更新时间
+    // Note: Accessing the item via index ensures we mutate the correct reactive proxy in the array
+    const conversation = conversations.value[existingIndex];
+    
+    // Update time
+    conversation.update_time = new Date().toISOString();
+
+    // Move to top only if not already at top and index > 0
+    if (existingIndex > 0) {
+      conversations.value.splice(existingIndex, 1);
+      conversations.value.unshift(conversation);
+    }
   }

   sendMessage({

Key Improvements in Refactor:

  1. Single Pass Lookup: Replaced .some() (which loops) followed by potential .find() loop logic with a single .findIndex(). This is more efficient.
  2. Correct Index Reference: By using existingIndex, we avoid calling .indexOf() later, which is an O(N) operation and theoretically prone to reference mismatch issues if proxies aren't aligned perfectly.
  3. Explicit Existence Check: if (existingIndex !== -1) is clearer and faster than checking for object truthiness if the object could theoretically be falsy (though unlikely here).
  4. Preserved Logic: Maintains the original intent of updating update_time and moving the active chat to the top to match backend sorting behavior.

Final Recommendation:

If this is Vue 3, the code is logically sound but the refactor above is slightly more performant.
If the conversations list is very large (e.g., thousands of items), consider maintaining a separate "ordered IDs" array or a map to avoid O(N) splicing operations on large arrays during frequent chat switches.

@peslyn
peslyn deleted the pr@v3@fix_portal_conversation branch November 13, 2026 06:41
QuerySet(Chat).filter(id=chat_id).update(update_time=timezone.now())

parameters = {
"retrieval_identity": identity_from_server(chat_user_id, chat_user_type, debug),

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall the change is reasonable: after inserting a new ChatRecord, updating the parent Chat.update_time keeps history ordering correct. A few points to check:

  1. Prefer the model manager instead of QuerySet(Chat)

    Chat.objects.filter(pk=chat_id).update(update_time=timezone.now())

    QuerySet(Chat).filter(...) is not standard Django usage and may bypass custom managers or fail depending on your imports. Use Chat.objects or the project’s established manager.

  2. Wrap the two writes in a transaction

    from django.db import transaction
    
    with transaction.atomic():
        ChatRecord(...).save(force_insert=True)
        updated = Chat.objects.filter(pk=chat_id).update(update_time=timezone.now())
        if updated == 0:
            raise ValueError(f"Chat not found: {chat_id}")

    This avoids a stale update_time if the insert succeeds but the update fails. Checking updated == 0 also catches an invalid/missing chat_id.

  3. Validate chat_id and scope it by user/tenant if needed
    If chat_id comes from request data, add ownership/tenant constraints:

    Chat.objects.filter(pk=chat_id, user_id=chat_user_id).update(...)

    Otherwise an unexpected ID could update another user’s chat timestamp.

  4. QuerySet.update() bypasses save() and signals
    It does not trigger model save(), pre_save/post_save signals, or auto_now handling automatically. Since you explicitly set update_time=timezone.now(), that is fine, but make sure no audit/cache-invalidation logic depends on Chat.save().

  5. Consider an index for history ordering
    Since the history list sorts by update_time descending, ensure there is an appropriate DB index, e.g. (user_id, update_time) or (tenant_id, update_time) depending on filters.

  6. Optional: use database time
    If app-server vs database clock skew matters:

    from django.db.models.functions import Now
    Chat.objects.filter(pk=chat_id).update(update_time=Now())

    timezone.now() is usually fine.

No major logic problem is visible, but the main improvements are using Chat.objects, adding transaction safety, and validating the updated row count.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants