Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,14 @@ def get_none_result(question):
'result_list': [], 'result': ''}, {})


def reset_metadata(metadata):
meta = metadata.get('meta')
if isinstance(metadata.get('meta'), dict):
if not meta.get('allow_download', False):
metadata['meta'] = {'allow_download': False}
return metadata


class BaseRerankerNode(IRerankerNode):
def save_context(self, details, workflow_manage):
self.context['document_list'] = details.get('document_list', [])
Expand All @@ -83,8 +91,9 @@ def execute(self, question, reranker_setting, reranker_list, reranker_model_id,
if len(documents) == 0:
return get_none_result(question)
top_n = reranker_setting.get('top_n', 3)
self.context['document_list'] = [{'page_content': document.page_content, 'metadata': document.metadata} for
document in documents]
self.context['document_list'] = [
{'page_content': document.page_content, 'metadata': reset_metadata(document.metadata)} for
document in documents]
self.context['question'] = question
workspace_id = self.workflow_manage.get_body().get('workspace_id')
reranker_model = get_model_instance_by_model_workspace_id(reranker_model_id,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The provided code has a few potential improvements and corrections:

  1. Import Statement: The first line seems incomplete as it only contains @@. Ensure that the full import statement is included at the beginning of the file.

  2. Function reset_metadata:

    • It's good to have a function for resetting metadata, but ensure that the parameter should be named consistently.
    • Consider handling cases where meta might be None.
  3. Class Inheritance:

    • Ensure that IRerankerNode is defined before using it. If it's meant to be an interface class without implementation, make sure there are methods left unimplemented rather than relying on inherited behavior.
  4. Logic Flow:

    • Check if the logic inside methods like save_context and execute handles all possible states gracefully.
    • Validate user inputs such as reranker_model_id, which should refer to existing models or throw appropriate errors if not found.
  5. Edge Cases:

    • Handle edge cases such as when documents list is empty, ensuring no unexpected behavior occurs during execution.

Here's a revised version based on these considerations:

from typing import List

class IRerankerNode:
    def save_context(self, details, workflow_manage):
        pass

    def execute(self, question, reranker_setting, reranker_list, reranker_model_id):
        pass

def reset_metadata(metadata, allow_download=None):
    meta = metadata.get('meta', {})
    if not isinstance(meta, dict):
        meta = {}
    if allow_download is not None:
        meta['allow_download'] = bool(allow_download)
    return metadata

class BaseRerankerNode(IRerankerNode):
    def __init__(self):
        self.context = {}

    def save_context(self, details, workflow_manage):
        documents = details.get('document_list', [])
        self.context['document_list'] = [
            {'page_content': document.page_content, 'metadata': reset_metadata(document.metadata)}
            for document in documents
        ]
        self.context['question'] = question

    def execute(self, question, reranker_setting, reranker_list, reranker_model_id):
        documents = reranker_setting.get('docuements', [])  # Corrected typo from "documents" to "docuements"
        if len(documents) == 0:
            return get_none_result(question)

        top_n = reranker_setting.get('top_n', 3)
        model_id = kwargs.pop('model_id') if reranker_model_id is not None else None

        if workspace_id is not None:
            # Further logic depending on your use case
            pass

    def get_none_result(self, question):
        return {
            'code': '',
            'success': False,
            'message': '',
            'result_list': [],
            'result': ''
        }

This version includes proper syntax (>>>) with comments explaining each change suggested. Always ensure consistency in naming conventions across similar functions and classes, and validate inputs to prevent runtime errors.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ def reset_title(title):
return f"#### {title}\n"


def reset_meta(meta):
if not meta.get('allow_download', False):
return {'allow_download': False}
return meta


class BaseSearchKnowledgeNode(ISearchKnowledgeStepNode):
def save_context(self, details, workflow_manage):
result = details.get('paragraph_list', [])
Expand Down Expand Up @@ -122,7 +128,8 @@ def reset_paragraph(paragraph: Dict, embedding_list: List):
'create_time': paragraph.get('create_time').strftime("%Y-%m-%d %H:%M:%S"),
'id': str(paragraph.get('id')),
'knowledge_id': str(paragraph.get('knowledge_id')),
'document_id': str(paragraph.get('document_id'))
'document_id': str(paragraph.get('document_id')),
'meta': reset_meta(paragraph.get('meta'))
}

@staticmethod
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
<span :title="data?.document_name?.trim()">{{ data?.document_name }}</span>
</a>
</div>
<div v-else @click="infoMessage">
<div v-else @click="infoMessage(data)">
<span class="ellipsis-1 break-all" :title="data?.document_name?.trim()">
{{ data?.document_name?.trim() }}
</span>
Expand Down Expand Up @@ -87,8 +87,12 @@ const parsedMeta = computed(() => {
})

const meta = computed(() => (isMetaObject.value ? props.data.meta : parsedMeta.value))
function infoMessage() {
MsgInfo(t('chat.noDocument'))
function infoMessage(data: any) {
if (data?.meta?.allow_download === false) {
MsgInfo(t('chat.noPermissionDownload'))
} else {
MsgInfo(t('chat.noDocument'))
}
}
</script>
<style lang="scss" scoped></style>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code looks mostly correct, with some minor improvements to enhance readability and functionality:

  1. Function Signature Update: The infoMessage function signature has been updated to accept a parameter data. This is beneficial if you plan to pass different data objects to various parts of the code.

  2. Conditional Logic Adjustments:

    • In the conditional logic within the infoMessage function, it's better to use an early return (return) to avoid unnecessary checks after returning. For example:
      function infoMessage(data: any) {
        if (data?.meta?.allow_download === false) {
          MsgInfo(t('chat.noPermissionDownload'));
          return;
        }
        // Proceed with showing "No Document" message otherwise
        MsgInfo(t('chat.noDocument'));
      }

These updates make the code more efficient and easier to understand while maintaining the same behavior. If you have further modifications or need additional feedback, let me know!

Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
<span :title="item?.document_name?.trim()">{{ item?.document_name }}</span>
</a>
</div>
<div v-else @click="infoMessage">
<div v-else @click="infoMessage(item)">
<span class="ellipsis-1 break-all" :title="item?.document_name?.trim()">
{{ item?.document_name?.trim() }}
</span>
Expand Down Expand Up @@ -167,8 +167,12 @@ const currentComponent = shallowRef<any>(null)
const currentChatDetail = ref<any>(null)
const dialogType = ref('')

function infoMessage() {
MsgInfo(t('chat.noDocument'))
function infoMessage(data: any) {
if (data?.meta?.allow_download === false) {
MsgInfo(t('chat.noPermissionDownload'))
} else {
MsgInfo(t('chat.noDocument'))
}
}
function openParagraph(row: any, id?: string) {
dialogTitle.value = t('chat.KnowledgeSource.title')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The provided code is mostly correct, but there are a few minor suggestions and improvements:

  1. Function Parameter Type:

    function infoMessage(data: any) {

    It's good practice to specify the type of the data parameter, especially since it contains dynamic properties from item. You could do:

    function infoMessage(item: { document_name?: string; meta?: { allow_download?: boolean }; ... otherProps? })
  2. Nullish Coalescing Operator:
    In TypeScript, you can use nullish coalescing (??) to handle cases where item?.document_name might be undefined. This makes the code clearer and less error-prone.

  3. Inline If Statement for Download Permission:
    The download permission logic can be simplified by using inline conditions without an additional if-else.

Here’s the revised version with these suggestions:

@@ -45,7 +45,7 @@
                     <span :title="item?.document_name?.trim()? trim(item.document_name ?? '') : 'no_document'">{{ item?.document_name }}</span>
                   </div>
                 )
               } else {
                 return (
-                  <div v-else @click="infoMessage(item ?? {})">
+                  <div v-else @click="handleClick(infoMessage)">
                     <span class="ellipsis-1 break-all" :title="(item?.document_name ?? '').trim()">
                       {{ (item?.document_name ?? '').trim() }}

Function Definition

If the intent is to pass additional context when opening the paragraph, consider defining this behavior differently:

const handleClick = (callback: (arg: any) => void, data: any) => () => callback(data);

// Usage:
openParagraph(row, id);

This approach decouples the handling of clicking different elements and provides more flexibility.

Overall, the original code functions correctly, and these changes make it more robust and easier to maintain.

Expand Down
1 change: 1 addition & 0 deletions ui/src/locales/lang/en-US/ai-chat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export default {
quote: 'Quote',
download: 'Click to Download',
noDocument: 'Original Document Not Found',
noPermissionDownload: 'No permission to download',
passwordValidator: {
title: 'Enter Password to Access',
errorMessage1: 'Password cannot be empty',
Expand Down
1 change: 1 addition & 0 deletions ui/src/locales/lang/zh-CN/ai-chat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export default {
download: '点击下载文件',
transcribing: '转文字中',
noDocument: '原文档不存在',
noPermissionDownload: '无权限下载',
passwordValidator: {
title: '请输入密码打开链接',
errorMessage1: '密码不能为空',
Expand Down
1 change: 1 addition & 0 deletions ui/src/locales/lang/zh-Hant/ai-chat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export default {
quote: '引用',
download: '點擊下載文件',
noDocument: '原文檔不存在',
noPermissionDownload: '無許可權下載',
passwordValidator: {
title: '請輸入密碼打開連結',
errorMessage1: '密碼不能為空',
Expand Down
Loading