🌿 fix: forking a long conversation breaks chat structure

[danny-avila]

ref: PR #4772

Closes #4761

Originally #4777

[xyqyear]

Is it possible to reference the same buildTree function in the tests instead of duplicating the code? Changes made to buildTree would have to be reflected in two different places.

Another thing is, this change would make imported (from file) messages not having createdAt field, which mighe become problematic when in the future we call
something like "deletedOlderThan". We might need to add createdAt manually when importing from files (adding incremental createdAt field when not existed or converting from original timestamp.).

@danny-avila

[danny-avila]

@xyqyear

Is it possible to reference the same buildTree function in the tests instead of duplicating the code? Changes made to buildTree would have to be reflected in two different places.

I don't plan on changing this function, and it's really not needed in the backend except for this test with the database logic, so it's not worth it to me.

Another thing is, this change would make imported (from file) messages not having createdAt field, which mighe become problematic when in the future we call
something like "deletedOlderThan". We might need to add createdAt manually when importing from files (adding incremental createdAt field when not existed or converting from original timestamp.).

can you elaborate more on this? I think this would be simple to handle the edge case here then:

LibreChat/api/server/utils/import/importBatchBuilder.js

Lines 99 to 110 in 7a5b697

	async saveBatch() {
	try {
	await bulkSaveConvos(this.conversations);
	await bulkSaveMessages(this.messages, true);
	logger.debug(
	`user: ${this.requestUserId} | Added ${this.conversations.length} conversations and ${this.messages.length} messages to the DB.`,
	);
	} catch (error) {
	logger.error('Error saving batch', error);
	throw error;
	}
	}

to this:

  async saveBatch() {
    try {
      // Set default timestamp for messages that don't have one
      const defaultTimestamp = new Date();
      this.messages = this.messages.map(message => ({
        ...message,
        createdAt: message.createdAt || defaultTimestamp,
        updatedAt: message.updatedAt || defaultTimestamp
      }));

      await bulkSaveConvos(this.conversations);
      // Keep overrideTimestamp as true to use our explicitly set timestamps
      await bulkSaveMessages(this.messages, true);
      logger.debug(
        `user: ${this.requestUserId} | Added ${this.conversations.length} conversations and ${this.messages.length} messages to the DB.`,
      );
    } catch (error) {
      logger.error('Error saving batch', error);
      throw error;
    }
  }
}

[danny-avila]

also maybe the above needs to be adjusted to account for child messages not being earlier than parent messages

[xyqyear]

@danny-avila

Absolutely! Changing saveBatch like that to account for edge cases is the best way to do it for now imo.

also maybe the above needs to be adjusted to account for child messages not being earlier than parent messages

This could be accomplished by d38953b However, through testing, this will make the page unresponsive for some reason. But if it's done correctly, the previously broken conversations could be fixed as well.