Skip to content

fix: Infinite cycle, maximum number of cycles failed and page scaling #4121

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

Merged
shaohuzhang1 merged 1 commit into from
Sep 26, 2025
Merged

fix: Infinite cycle, maximum number of cycles failed and page scaling #4121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 3 additions & 2 deletions apps/application/flow/step_node/loop_node/impl/base_loop_node.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,8 @@ def loop(workflow_manage_new_instance, node: INode, generate_loop):
is_interrupt_exec = False
loop_node_data = node.context.get('loop_node_data') or []
loop_answer_data = node.context.get("loop_answer_data") or []
current_index = node.context.get("current_index") or 0
start_index = node.context.get("current_index") or 0
current_index = start_index
node_params = node.node_params
start_node_id = node_params.get('child_node', {}).get('runtime_node_id')
loop_type = node_params.get('loop_type')
Expand All @@ -144,7 +145,7 @@ def loop(workflow_manage_new_instance, node: INode, generate_loop):
details=loop_node_data[current_index])

for item, index in generate_loop(current_index):
if 0 < max_loop_count <= index - current_index and loop_type == 'LOOP':
if 0 < max_loop_count <= index - start_index and loop_type == 'LOOP':
raise Exception(_('Exceeding the maximum number of cycles'))
"""
指定次数循环
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Sep 26, 2025

Choose a reason for hiding this comment

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

Here's my review and recommended changes for the provided code:

def loop(workflow_manage_new_instance, node: INode, generate_loop):
    is_interrupt_exec = False
    loop_node_data = node.context.get('loop_node_data') or []
    loop_answer_data = node.context.get("loop_answer_data") or []
    start_index = node.context.get("current_index", 0)
    current_index = start_index

    node_params = node.node_params
    start_node_id = node_params.get('child_node', {}).get('runtime_node_id')
    loop_type = node_params.get('loop_type')

    execute_once = lambda x: True  # Assume this function will always return True or handle the execution logic properly

    try:
        # Execute logic before entering the loop
        pass
    
        while not is_interrupt_exec:
            index = current_index
            
            if (
                loop_type == 'LOOP'
                and (max_loop_count > 0)  # Assuming max_loop_count is defined somewhere else
                and (index >= 1)  # Adjust conditions based on actual usage patterns
            ):
                
                # Fetch data at the current index with handling potential edge cases
                item = fetch_item_from_array(loop_node_data, index)
                
                if loop_type == 'LOOP':  # Ensure correct condition check
                    if 0 < max_loop_count <= index - start_index:
                        raise Exception(_('Exceeding the maximum number of cycles'))

                # Continue executing other steps within the loop
                do_more_steps_with(item)

            else:
                break
        
    except Exception as ex:
        traceback.print_exc()

    finally:
        # Cleanup actions if any
        pass

Changes Made:

  1. Start Index Initialization: The start_index variable was introduced to handle the initial value retrieval from context and assignment to current_index. This avoids redundant checks later.
  2. Loop Condition Logic: Fixed the logical errors in the loop condition (>=1) assuming appropriate array bounds.
  3. Comments and Docstrings: Added comments above sections where logic changes might be necessary.
  4. Function Definition: Assumed a simple function fetch_item_from_array() that safely retrieves items from the list using an index.

This refactoring aims to simplify the structure and improve readability. It also includes basic exception handling, ensuring that errors can be caught gracefully without crashing the program.

Expand Down
6 changes: 5 additions & 1 deletion ui/admin.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@
<head>
<meta charset="UTF-8" />
<link rel="icon" href="./favicon.ico" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<meta
name="viewport"
content="width=device-width, initial-scale=1.0, maximum-scale=1.0, minimum-scale=1.0, user-scalable=no,
viewport-fit=cover"
/>
<base target="_blank" />
<title>%VITE_APP_TITLE%</title>
<script>
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Sep 26, 2025

Choose a reason for hiding this comment

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

Your HTML code has several points that can be optimized:

  1. <meta> Tags:

    • The content="maximum-scale=1.0" and minimum-scale=1.0 meta attributes in <meta name="viewport"> should also specify user-scalable=yes. If you do not want users to scale the page, it is better to allow them to zoom but prevent accidental changes.
    • Consider removing viewport-fit=cover, as this may interfere with responsive design.

  2. Base Tag:

    • Having the <base target="_blank"> inside <head> might cause issues depending on how the rest of the document is structured, so consider moving it below other elements if it's necessary.

Here's an optimized version of your <head> section:

<head>
  <meta charset="UTF-8">
  <link rel="icon" href="./favicon.ico">
  <meta name="viewport" content="width=device-width, initial-scale=1.0, user-scalable=yes">
  <!-- Base tag usually goes after stylesheets and scripts -->
  
  <title>%VITE_APP_TITLE%</title>

  <style>
    /* Your CSS here */
    @import './styles/style.css';
  </style>

  <script src="path/to/your/script.js"></script>
</head>

Explanation:

  • Viewport Settings: This ensures the page behaves well across different screen sizes and devices. Ensure both user-scalable= and width=device-width are set appropriately.
  • Structure Clean-up: By placing JavaScript at the end of the file (just before closing the body tag), you ensure the DOM is fully loaded when your scripts run, improving performance and usability.
  • CSS Importing: Using <style> tags within HTML is outdated compared to using external CSS files (@import). It’s recommended to keep styling separate from HTML.

Expand Down
2 changes: 1 addition & 1 deletion ui/chat.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<meta
name="viewport"
content="width=device-width, initial-scale=1.0, maximum-scale=1.0, minimum-scale=1.0, user-scalable=no,
viewport-fit=cover"
viewport-fit=cover"
/>
<base target="_blank" />
<title>%VITE_APP_TITLE%</title>
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Sep 26, 2025

Choose a reason for hiding this comment

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

The provided code snippet does not contain any significant irregularities or major performance optimizations that require changes. The <meta> tag you've altered is a standard attribute used to control how a webpage is rendered on devices. The viewport-fit=cover property scales the content to fill the entire screen with a maximum width of the device's natural size, ensuring better fit.

This change doesn't introduce bugs and aligns with modern web design practices. However, there might be some specific use cases where you want to adjust other meta attributes for SEO reasons or responsiveness, but overall this modification looks correct based on the current state.

Expand Down
Loading