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
2 changes: 2 additions & 0 deletions apps/application/workflow/nodes/llm_node/llm_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,8 @@ def get_details(self, index: int = 0, position: dict = None, old_details: dict =
"message_tokens": self.data.get("message_tokens"),
"answer_tokens": self.data.get("answer_tokens"),
"messages": aggregation.get_contents(),
"system": self.get_parameters().get("system"),
"prompt": self.get_parameters().get("prompt"),
}
)
return details

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.

Looking at this diff, here are my observations:

Issues & Suggestions

1. Unnecessary repeated get_parameters() calls

"system": self.get_parameters().get("system"),
"prompt": self.get_parameters().get("prompt"),

get_parameters() is called twice. If it's non-trivial (parsing, copying, DB lookup), this is wasteful. Cache it:

params = self.get_parameters()
...
"system": params.get("system"),
"prompt": params.get("prompt"),

2. Potential inconsistency / staleness

The rest of details uses self.data.get(...) — i.e., the stored/persisted data. Mixing in fresh values from get_parameters() can produce an inconsistent snapshot (e.g., system reflects current parameters while message_tokens reflects self.data). Confirm whether system/prompt should come from self.data for consistency with the existing pattern.

3. Redundancy between system and prompt

If prompt already embeds or derives from system, including both may duplicate data and inflate the serialized output (especially if returned to clients or logged).

4. Missing schema / key contract

No context is shown about downstream consumers of details. Adding new keys to a return dict is usually safe, but:

  • Verify no consumer does strict key validation or equality checks on details.
  • Consider bumping a version field if details is a public API payload.

5. Nullability handling

If get_parameters() can return None, .get(...) will raise AttributeError. The existing self.data.get(...) assumes non-None; ensure the same invariant holds here, or use (self.get_parameters() or {}).get("system").

6. No test coverage visible

Suggest adding/extending a test that asserts details["system"] and details["prompt"] are populated as expected, and that missing parameters yield None rather than raising.

7. Minor: naming clarity

Confirm "system" and "prompt" are the most descriptive key names for the details payload (vs. e.g. "system_prompt"), to avoid ambiguity with other "prompt" concepts.

26 changes: 15 additions & 11 deletions ui/src/workflow-canvas/nodes/llm-node/details/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,20 @@ function formatJson(value: unknown) {
<template #header>
<BaseHeader :data="data" show-tokens />
</template>
<!-- 系统提示词 -->
<div class="mk-gray-card-sm">
<h6 class="mb-2">系统提示词</h6>
<div>{{ data.system || '-' }}</div>
</div>
<!-- 系统提示词 -->
<div class="mk-gray-card-sm">
<h6 class="mb-2">用户提示词</h6>
<div>{{ data.prompt || '-' }}</div>
</div>

<!-- 消息列表 -->
<div class="mk-gray-card-sm">
<h6 class="mb-2">消息列表</h6>
<h6 class="mb-2">上下文</h6>
<div class="space-y-2">
<template v-if="data.message_list?.length > 0">
<p v-for="(message, messageIndex) in data.message_list" :key="messageIndex" class="whitespace-pre-wrap">
Expand All @@ -33,13 +43,11 @@ function formatJson(value: unknown) {
<template v-else>-</template>
</div>
</div>

<!-- 思考过程 -->
<!-- 结束原因 -->
<div class="mk-gray-card-sm">
<h6 class="mb-2">思考过程</h6>
<div class="whitespace-pre-wrap">{{ data.reasoning_content || '-' }}</div>
<h6 class="mb-2">结束原因</h6>
<div>{{ data.finish_reason || '-' }}</div>
</div>

<!-- 回答内容 -->
<div class="mk-gray-card-sm">
<h6 class="mb-2">回答内容</h6>
Expand All @@ -63,10 +71,6 @@ function formatJson(value: unknown) {
</div>
</div>

<!-- 结束原因 -->
<div class="mk-gray-card-sm">
<h6 class="mb-2">结束原因</h6>
<div>{{ data.finish_reason || '-' }}</div>
</div>

</DetailContainer>
</template>

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 the analysis of your code diff (Vue template and minor JS context):

Potential Issues & Irregularities

  1. Hardcoded Chinese Strings:

    • Texts like "系统提示词", "用户提示词", "上下文", "结束原因", "回答内容" are hardcoded.
    • Suggestion: Use an internationalization library (e.g., vue-i18n) with keys like t('label.system_prompt') for maintainability and multi-language support.
  2. Redundant Comments:

    • The comment <!-- 系统提示词 --> appears twice immediately after each other, and one is attached to the "User Prompt" block. This looks like a copy-paste error.
    • Suggestion: Remove duplicate comments and ensure each comment accurately describes the block it precedes.
  3. Unnecessary Blank Lines/Whitespace:

    • There are excessive blank lines at the end of the file (@@ -63,10 +71,6 @@) and between sections in some places.
    • Suggestion: Clean up whitespace for a tighter, standard format. Most linters (like ESLint/Vue ESLint plugin) will flag or auto-fix this.
  4. Missing Type Safety/Validation in Template:

    • Directly accessing data.system, data.prompt, etc., assumes these properties always exist or gracefully return undefined. While || '-' handles empty values, if data itself is undefined/null, this will throw an error.
    • Suggestion: Ensure data is properly typed (e.g., via interface or type) and possibly add a guard clause or optional chaining if data might be null before rendering this component’s internal content.
  5. "Thinking Process" Removal:

    • You removed the reasoning_content display and replaced it with finish_reason. If reasoning_content was previously critical for debugging AI outputs, ensure this removal is intentional and not due to oversight.

Optimization Suggestions

  1. Extract Sub-components:

    • The template is getting long with multiple similar mk-gray-card-sm blocks. Consider creating a reusable sub-component:
    <InfoCard :title="t('label.system_prompt')" :content="data.system" />
    <InfoCard :title="t('label.user_prompt')" :content="data.prompt" />

    Where InfoCard handles the .mk-gray-card-sm, title styling, and fallback '-'.

  2. Dynamic Labels for Context:

    • You renamed "消息列表" to "上下文". Ensure this rename is consistent across all related components or data models to avoid confusion.
  3. Use v-if Guard for the Container:

    • If all fields may be empty and result in many '-' placeholders, consider wrapping the entire detail view in a check if meaningful data exists, or conditionally rendering each card only if its respective field has content.

Updated Code Snippet (Cleaned & Structured)

@@ -19,10 +19,20 @@
+    <!-- System Prompt -->
+    <div class="mk-gray-card-sm">
+      <h6 class="mb-2">{{ t('label.system_prompt') }}</h6>
+      <div>{{ data.system || '-' }}</div>
+    </div>
+    <!-- User Prompt -->
+    <div class="mk-gray-card-sm">
+      <h6 class="mb-2">{{ t('label.user_prompt') }}</h6>
+      <div>{{ data.prompt || '-' }}</div>
+    </div>

     <!-- Messages / Context -->
     <div class="mk-gray-card-sm">
-      <h6 class="mb-2">消息列表</h6>
+      <h6 class="mb-2">{{ t('label.context') }}</h6>
       <div class="space-y-2">
         <template v-if="data.message_list?.length > 0">
           <p v-for="(message, messageIndex) in data.message_list" :key="messageIndex" class="whitespace-pre-wrap">
@@ -33,13 +43,11 @@
         <template v-else>-</template>
       </div>
     </div>
+    <!-- Finish Reason -->
+    <div class="mk-gray-card-sm">
+      <h6 class="mb-2">{{ t('label.finish_reason') }}</h6>
+      <div>{{ data.finish_reason || '-' }}</div>
+    </div>
     <!-- Response Content -->
     <div class="mk-gray-card-sm">
       <h6 class="mb-2">{{ t('label.response') }}</h6>
@@ -63,10 +71,6 @@
-      </div>
-    </div>
-
-    <!-- 结束原因 -->
-    <div class="mk-gray-card-sm">
-      <h6 class="mb-2">结束原因</h6>
-      <div>{{ data.finish_reason || '-' }}</div>
-    </div>
   </DetailContainer>
 </template>

Note: Replace t(...) with your actual i18n function/method. If not using i18n, replace back with plain strings but keep the cleaned structure.

Loading