Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/isValidMessage #5579

Closed
wants to merge 7 commits into from
Closed

Fix/isValidMessage #5579

Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7fa8234
add more criteria for detecting invalid messages
code-october Oct 3, 2024
a039690
add response judgment conditions for summarizing messages request
code-october Oct 3, 2024
4bb52b2
调整判断规则、优化代码规范
code-october Oct 11, 2024
7d7ca64
Merge branch 'ChatGPTNextWeb:main' into fix/isValidMessage
code-october Oct 12, 2024
dd793e0
完善判断逻辑，添加jest测试样例
code-october Oct 12, 2024
fe9c31c
fix isValidMessage.test.ts
code-october Oct 12, 2024
b0236f7
fix jest error
code-october Oct 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion app/store/chat.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -681,6 +681,7 @@ export const useChatStore = createPersistStore(
},
onFinish(message) {
console.log("[Memory] ", message);
if (!isValidMessage(message)) return;
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Potential unvalidated message processing detected in app/store/chat.ts at line 696.

Please ensure that isValidMessage is invoked consistently wherever messages are processed to maintain validation integrity.

🔗 Analysis chain

Verify that message validation is consistently applied

In the onFinish callback, the check if (!isValidMessage(message)) return; ensures that invalid messages are not processed. To maintain consistency and robustness, ensure that all instances where messages are processed include validation using isValidMessage.

Run the following script to check for other message handling without validation:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find instances where messages are handled without validation.
# Expected: All message processing should involve validation via isValidMessage.

rg --type ts --no-heading --line-number --color never 'function\s+\w+\(.*message.*\)' app/store/ | while read -r line; do
  file=$(echo "$line" | cut -d: -f1)
  lineno=$(echo "$line" | cut -d: -f2)
  function_def=$(echo "$line" | cut -d: -f3-)
  function_body=$(awk "NR>$lineno && NR<($lineno+20)" "$file")
  if ! echo "$function_body" | grep -q "isValidMessage"; then
    echo "Potential unvalidated message in $file:$lineno - $function_def"
  fi
done

Length of output: 2295

get().updateCurrentSession((session) => {
session.lastSummarizeIndex = lastSummarizeIndex;
session.memoryPrompt = message; // Update the memory prompt for stored it in local storage
Expand All @@ -693,7 +694,26 @@ export const useChatStore = createPersistStore(
}

function isValidMessage(message: any): boolean {
return typeof message === "string" && !message.startsWith("```json");
if (typeof message !== "string") {
return false;
}
if (message.startsWith("```") && message.endsWith("```")) {
const codeBlockContent = message.slice(3, -3).trim();
const jsonString = codeBlockContent.replace(/^json\s*/i, '').trim();
try {
// 返回 json 格式消息，含 error.message 字段，判定为错误回复，否则为正常回复
const jsonObject = JSON.parse(jsonString);
if (jsonObject?.error?.message) {
return false;
}
return true;
} catch (e) {
console.log("Invalid JSON format.");
// 非 json 格式，大概率是正常回复
return true;
}
}
return true;
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance robustness of isValidMessage function

The isValidMessage function is a good addition to validate messages before processing. However, consider the following improvements:

  1. Check if the code block starts with "```json" before attempting to parse it as JSON.
  2. Handle different types of code blocks, not just JSON.
  3. Improve error handling to be more specific about what went wrong.

Here's a suggested refactor:

 function isValidMessage(message: any): boolean {
   if (typeof message !== "string") {
     return false;
   }
   if (message.startsWith("```") && message.endsWith("```")) {
     const codeBlockContent = message.slice(3, -3).trim();
-    const jsonString = codeBlockContent.replace(/^json\s*/i, '').trim();
+    if (codeBlockContent.toLowerCase().startsWith('json')) {
+      const jsonString = codeBlockContent.replace(/^json\s*/i, '').trim();
+      try {
+        const jsonObject = JSON.parse(jsonString);
+        return !jsonObject?.error?.message;
+      } catch (e) {
+        console.log("Invalid JSON format:", e.message);
+        return false;
+      }
+    }
+    // Non-JSON code blocks are considered valid
+    return true;
   }
-    try {
-      // 返回 json 格式消息，含 error.message 字段，判定为错误回复，否则为正常回复
-      const jsonObject = JSON.parse(jsonString);
-      if (jsonObject?.error?.message) {
-        return false;
-      }
-      return true;
-    } catch (e) {
-      console.log("Invalid JSON format.");
-      // 非 json 格式，大概率是正常回复
-      return true;
-    }
-  }
   return true;
 }

This refactor:

  • Only attempts to parse JSON if the code block explicitly starts with "json".
  • Treats non-JSON code blocks as valid messages.
  • Improves error logging for invalid JSON.
  • Simplifies the overall logic.

}
},

Expand Down
Loading