backend: check queries to queue and completed_job where possible (v2 phase 2)

[uael]

Important

Enhance backend job and flow dependency management with new SQL queries, functions, and improved error handling.

• Database Changes:
  • Add new SQL queries for managing job dependencies and flow states in worker_lockfiles.rs.
  • Introduce new tables and views for v2_job_runtime, v2_job_status, and v2_job_queue.
  • Add migration scripts for new tables and views.
• Functionality Enhancements:
  • Implement update_script_dependency_map and add_relative_imports_to_dependency_map for handling script dependencies.
  • Add handle_dependency_job and handle_flow_dependency_job to manage job and flow dependencies.
  • Introduce lock_modules and reduce_flow for flow module management.
• Error Handling and Logging:
  • Improve error handling and logging in dependency management functions.
  • Add checks for job cancellation before updating flows or dependencies.
• Miscellaneous:
  • Update worker_lockfiles.rs to handle various script languages and their dependencies.
  • Enhance transaction management for database operations related to job dependencies.

^{This description was created by for ee6a649. It will automatically update as commits are pushed.}

[alwaysmeticulous]

Meticulous was unable to execute a test run for this PR because the most recent commit is associated with multiple PRs. To execute a test run, please try pushing up a new commit that is only associated with this PR.

_{Last updated for commit a1df0dd. This comment will update as new commits are pushed.}

[ellipsis-dev]

👍 Looks good to me! Reviewed everything up to a1df0dd in 2 minutes and 44 seconds

More details

• Looked at 5761 lines of code in 119 files
• Skipped 0 files when reviewing.
• Skipped posting 5 drafted comments based on config settings.

1. backend/windmill-api/src/jobs.rs:1092

• Draft comment:
  There is a missing space between args and as in the SQL query aliasing.

"SELECT created_by AS \"created_by!\", args AS \"args: sqlx::types::Json<Box<RawValue>>\"

• Reason this comment was not posted:
  Comment looked like it was already resolved.

2. backend/windmill-api/src/jobs.rs:1128

• Draft comment:
  There is a missing space between args and as in the SQL query aliasing.

            "SELECT created_by AS \"created_by!\", args AS \"args: sqlx::types::Json<Box<RawValue>>\"

• Reason this comment was not posted:
  Marked as duplicate.

3. backend/windmill-api/src/jobs.rs:5505

• Draft comment:
  There is a missing space between result and as in the SQL query aliasing.

        "SELECT result AS \"result: sqlx::types::Json<Box<RawValue>>\", flow_status AS \"flow_status: sqlx::types::Json<Box<RawValue>>\", language AS \"language: ScriptLang\", created_by AS \"created_by!\"

• Reason this comment was not posted:
  Comment looked like it was already resolved.

4. backend/windmill-api/src/jobs.rs:5689

• Draft comment:
  There is a missing space between result and as in the SQL query aliasing.

        "SELECT result AS \"result: sqlx::types::Json<Box<RawValue>>\", success AS \"success!\", language AS \"language: ScriptLang\", flow_status AS \"flow_status: sqlx::types::Json<Box<RawValue>>\", created_by AS \"created_by!\"

• Reason this comment was not posted:
  Marked as duplicate.

5. backend/windmill-queue/src/jobs.rs:2224

• Draft comment:
  There is a missing space between result and as in the SQL query aliasing.

    "SELECT result AS \"result: Json<Box<RawValue>>\" FROM completed_job WHERE id = $1 AND workspace_id = $2",

• Reason this comment was not posted:
  Marked as duplicate.

Workflow ID: wflow_Pb2o6puFmbChvi9b

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[cloudflare-workers-and-pages]

Deploying windmill with    Cloudflare Pages

Latest commit:	d28fea8
Status:	⚡️  Build in progress...

View logs

[ellipsis-dev]

👍 Looks good to me! Reviewed everything up to 6de1faa in 4 minutes and 1 seconds

More details

• Looked at 5768 lines of code in 119 files
• Skipped 0 files when reviewing.
• Skipped posting 6 drafted comments based on config settings.

1. backend/src/monitor.rs:1507

• Draft comment:
  The use of sqlx::query! here ensures compile-time checking of the SQL query, which is a good practice for preventing runtime errors.
• Reason this comment was not posted:
  Confidence changes required: 0%
  The query in the handle_zombie_jobs function is using sqlx::query! but the fetch_all method is being called directly on it. This is correct usage, but it's worth noting that the query! macro is used for compile-time checked queries, which is a good practice.

2. backend/src/monitor.rs:1626

• Draft comment:
  The use of sqlx::query! ensures compile-time checking of the SQL query, which is a good practice. Additionally, handling same_worker with unwrap_or(false) is a safe way to handle potential None values.
• Reason this comment was not posted:
  Confidence changes required: 0%
  The handle_zombie_flows function uses sqlx::query! for compile-time checked queries, which is a good practice. The function also correctly handles the case where same_worker is None by using unwrap_or(false).

3. backend/src/monitor.rs:1717

• Draft comment:
  The transaction is correctly committed after canceling the job, ensuring that changes are persisted.
• Reason this comment was not posted:
  Confidence changes required: 0%
  In the cancel_zombie_flow_job function, the transaction is correctly committed after the job is canceled. This ensures that the changes are persisted in the database.

4. backend/windmill-api/src/jobs.rs:1090

• Draft comment:
  The use of sqlx::query! ensures compile-time checking of the SQL query, which is a good practice. Handling tags with as_ref().map(...) is a safe way to handle potential None values.
• Reason this comment was not posted:
  Confidence changes required: 0%
  In the get_args function, the query uses sqlx::query! for compile-time checking, which is a good practice. The function also correctly handles the case where tags is None.

5. backend/windmill-api/src/jobs.rs:5505

• Draft comment:
  The use of sqlx::query! ensures compile-time checking of the SQL query, which is a good practice. Handling tags with as_ref().map(...) is a safe way to handle potential None values.
• Reason this comment was not posted:
  Confidence changes required: 0%
  In the get_completed_job_result function, the use of sqlx::query! ensures compile-time checking of the SQL query, which is a good practice. The function also correctly handles the case where tags is None.

6. backend/windmill-queue/src/jobs.rs:2364

• Draft comment:
  The use of sqlx::query! ensures compile-time checking of the SQL query, which is a good practice. Handling flow_job_result with not_found_if_none is a safe way to handle potential None values.
• Reason this comment was not posted:
  Confidence changes required: 0%
  In the get_result_by_id_from_running_flow_inner function, the use of sqlx::query! ensures compile-time checking of the SQL query, which is a good practice. The function also correctly handles the case where flow_job_result is None.

Workflow ID: wflow_bh5uevbzosDPpdiK

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

👍 Looks good to me! Incremental review on 741fad9 in 1 minute and 16 seconds

More details

• Looked at 2984 lines of code in 50 files
• Skipped 0 files when reviewing.
• Skipped posting 2 drafted comments based on config settings.

1. backend/src/monitor.rs:1650

• Draft comment:
  Using unwrap_or(false) on flow.same_worker could lead to unexpected behavior if None is a valid state that should be handled differently. Consider handling the None case explicitly if it is significant.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The use of unwrap_or(false) on flow.same_worker is a good practice to handle potential None values, but it should be noted that this could lead to unexpected behavior if None is a valid state that should be handled differently. Consider logging or handling the None case explicitly if it is significant.

2. backend/src/monitor.rs:1649

• Draft comment:
  Using unwrap_or(false) on flow.is_flow_step could lead to unexpected behavior if None is a valid state that should be handled differently. Consider handling the None case explicitly if it is significant.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The use of unwrap_or(false) on flow.is_flow_step is a good practice to handle potential None values, but it should be noted that this could lead to unexpected behavior if None is a valid state that should be handled differently. Consider logging or handling the None case explicitly if it is significant.

Workflow ID: wflow_WpVEKbSY54MxVgtL

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

👍 Looks good to me! Incremental review on 4388e84 in 1 minute and 25 seconds

More details

• Looked at 2957 lines of code in 49 files
• Skipped 0 files when reviewing.
• Skipped posting 2 drafted comments based on config settings.

1. backend/src/monitor.rs:1650

• Draft comment:
  Consider handling the None case for flow.same_worker explicitly instead of using unwrap_or(false) to improve code clarity and maintainability.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The use of unwrap_or(false) on flow.same_worker is a good practice to handle Option values safely. However, the unwrap_or method can hide potential issues if flow.same_worker is None unexpectedly. It would be better to handle the None case explicitly to ensure that the logic is clear and maintainable.

2. backend/src/monitor.rs:1649

• Draft comment:
  Consider handling the None case for flow.is_flow_step explicitly instead of using unwrap_or(false) to improve code clarity and maintainability.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The unwrap_or(false) usage on flow.is_flow_step could potentially hide issues if flow.is_flow_step is None unexpectedly. It's better to handle the None case explicitly to ensure clarity and maintainability.

Workflow ID: wflow_nTW1gyFD006KUNuo

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

👍 Looks good to me! Incremental review on bf056e1 in 1 minute and 29 seconds

More details

• Looked at 2957 lines of code in 49 files
• Skipped 0 files when reviewing.
• Skipped posting 2 drafted comments based on config settings.

1. backend/src/monitor.rs:1651

• Draft comment:
  Consider using unwrap_or(false) consistently for flow.same_worker to handle potential None values safely. This pattern is used in multiple places, so ensure consistency across the codebase.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The use of unwrap_or(false) on flow.same_worker is a good practice to handle potential None values, but it should be consistent across the codebase. This pattern is used in multiple places, so it's important to ensure consistency.

2. backend/src/monitor.rs:1626

• Draft comment:
  Consider using unwrap_or(false) consistently for flow.is_flow_step to handle potential None values safely. This pattern is used in multiple places, so ensure consistency across the codebase.
• Reason this comment was not posted:
  Confidence changes required: 50%
  The use of unwrap_or(false) on flow.is_flow_step is a good practice to handle potential None values, but it should be consistent across the codebase. This pattern is used in multiple places, so it's important to ensure consistency.

Workflow ID: wflow_9q9ttjsZIKE1ATBH

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

👍 Looks good to me! Incremental review on 9433026 in 52 seconds

More details

• Looked at 1527 lines of code in 39 files
• Skipped 0 files when reviewing.
• Skipped posting 1 drafted comments based on config settings.

1. backend/.sqlx/query-0ef638eb62cb8b285cb20855679486b78eae82901a0128b9c9c837c9e9e91212.json:3

• Draft comment:
  Consider placing the FROM clause on a new line for better readability.
• Reason this comment was not posted:
  Confidence changes required: 10%
  The SQL queries in the PR seem to be well-formed and follow best practices. However, there is a minor issue with the formatting of the SQL query in query-0ef638eb62cb8b285cb20855679486b78eae82901a0128b9c9c837c9e9e91212.json. The FROM clause is not on a new line, which affects readability.

Workflow ID: wflow_r1K1oCWhEmeKcwRj

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

👍 Looks good to me! Incremental review on 4ef4d1e in 2 minutes and 22 seconds

More details

• Looked at 2922 lines of code in 48 files
• Skipped 0 files when reviewing.
• Skipped posting 13 drafted comments based on config settings.

1. backend/windmill-worker/src/worker_flow.rs:190

• Draft comment:
  The update_flow_status_after_job_completion_internal function is extremely long and complex; consider refactoring into smaller helper functions.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

2. backend/windmill-worker/src/worker_flow.rs:3970

• Draft comment:
  Use a strongly‑typed mapping (e.g. query_as) instead of manually extracting the result with .0 for clarity and safety.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

3. backend/windmill-worker/src/worker_flow.rs:3080

• Draft comment:
  The match block in compute_next_flow_transform is complex; adding inline comments or refactoring may improve maintainability.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

4. backend/windmill-worker/src/worker_flow.rs:1301

• Draft comment:
  Clarify boundary conditions in next_retry; document what happens when fail_count equals MAX_RETRY_ATTEMPTS.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

5. backend/windmill-worker/src/worker_flow.rs:3567

• Draft comment:
  The next_forloop_status function has deeply nested match arms; consider refactoring and adding documentation to simplify its control flow.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

6. backend/windmill-worker/src/worker_flow.rs:1597

• Draft comment:
  Consider comparing 'counter' directly with '*crash_at' instead of using a reference comparison, to improve clarity.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

7. backend/windmill-worker/src/worker_flow.rs:1308

• Draft comment:
  The evaluated expression in 'compute_bool_from_expr' is matched against strict "true"/"false" strings. Consider handling additional boolean representations or documenting the expected output.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

8. backend/windmill-worker/src/worker_flow.rs:3034

• Draft comment:
  The 'compute_next_flow_transform' function is very complex; consider refactoring into smaller helper functions for maintainability.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

9. backend/windmill-worker/src/worker_flow.rs:3710

• Draft comment:
  Payload conversion functions like 'payload_from_simple_module' use unwraps and cloning; consider more robust error handling or additional documentation to avoid panics in production.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

10. backend/windmill-worker/src/worker_flow.rs:3903

• Draft comment:
  Add detailed inline comments to 'get_transform_context' to explain the constructed IdContext and its usage, aiding future maintenance.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

11. backend/windmill-worker/src/worker_flow.rs:2320

• Draft comment:
  Consider extracting inline SQL queries (using sqlx query! macros) into separate constants or modules for improved maintainability and clarity, and verify that proper indexes exist for performance.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

12. backend/windmill-worker/src/worker_flow.rs:2470

• Draft comment:
  Include inline documentation explaining the purpose and benefits of using the 'Marc' wrapper from mappable_rc to assist future maintainers.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

13. backend/windmill-worker/src/worker_flow.rs:3420

• Draft comment:
  The functions 'push_next_flow_job' and 'next_loop_iteration' are highly complex. Consider splitting them into smaller units with more inline comments to improve readability and maintainability.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_X8HGvtMPX2BnBKeO

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

👍 Looks good to me! Incremental review on a45df90 in 3 minutes and 28 seconds

More details

• Looked at 2922 lines of code in 48 files
• Skipped 0 files when reviewing.
• Skipped posting 12 drafted comments based on config settings.

1. backend/windmill-worker/src/handle_child.rs:69

• Draft comment:
  In the function 'child_joined_output_stream', the stderr handle is taken using an expect message that incorrectly refers to stdout. It currently reads “child did not have a handle to stdout” even though it is for stderr. This can be confusing; please change the error message to reference stderr.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

2. backend/windmill-worker/src/worker_flow.rs:210

• Draft comment:
  The function 'update_flow_status_after_job_completion_internal' is extremely lengthy and complex. It performs many nested matches and repeated SQL update queries. Consider refactoring parts of this function (e.g. extracting common update queries into helpers) to improve readability and maintainability. Additionally, check that similar SQL patterns (e.g. JSONB_SET updates) are uniformly maintained to reduce duplication.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

3. backend/windmill-worker/src/handle_child.rs:231

• Draft comment:
  Avoid using unwrap() when sending SIGINT. Instead, handle errors gracefully so that a failure to send the signal can be logged and recovered from.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

4. backend/windmill-worker/src/handle_child.rs:68

• Draft comment:
  Review the kill_process_tree implementation. Consider more robust handling of command output (trimming, CRLF differences) and error messages rather than a generic error message.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

5. backend/windmill-worker/src/handle_child.rs:232

• Draft comment:
  The cancellation logic uses a loop with one-second sleeps after sending SIGINT and SIGTERM. Consider using a more efficient/waitable mechanism to detect process exit without fixed delays.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

6. backend/windmill-worker/src/handle_child.rs:328

• Draft comment:
  Ensure that I/O errors during reading of child's output are handled correctly. The current loop may mask recurring errors which could lead to unexpected behavior.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

7. backend/windmill-worker/src/worker_flow.rs:223

• Draft comment:
  The function update_flow_status_after_job_completion_internal is extremely complex with many nested match arms and SQL queries. Consider refactoring it into smaller helper functions to improve maintainability and testability.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

8. backend/windmill-worker/src/worker_flow.rs:3929

• Draft comment:
  The 'from_now' function returns MAX_UTC when the duration cannot be converted. Consider logging a warning when the duration is too large so that such fallbacks are noticeable.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

9. backend/windmill-worker/src/worker_flow.rs:1302

• Draft comment:
  In next_retry, check boundary conditions carefully. The condition 'status.fail_count <= MAX_RETRY_ATTEMPTS' may allow retries when fail_count equals the maximum. Verify that this behavior is intended.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

10. backend/windmill-worker/src/worker_flow.rs:1243

• Draft comment:
  The regex usage in compute_skip_branchall_failure may panic if the capture group is missing. Add proper error handling to avoid potential panics when parsing the branch index from the script path.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

11. backend/windmill-worker/src/worker_flow.rs:1

• Draft comment:
  The overall complexity in this file is high. Consider refactoring large functions (e.g., update_flow_status_after_job_completion_internal and push_next_flow_job) into smaller, well-documented units to improve readability and maintainability.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

12. backend/windmill-worker/src/worker_flow.rs:2991

• Draft comment:
  In the function payload_from_modules, edge cases for empty modules are handled by returning None. Consider whether an error should be returned if payload construction fails to ensure that unexpected states are caught.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_RuMrErfiYTgPHCI3

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

👍 Looks good to me! Incremental review on 62dbfc9 in 3 minutes and 58 seconds

More details

• Looked at 2922 lines of code in 48 files
• Skipped 0 files when reviewing.
• Skipped posting 12 drafted comments based on config settings.

1. backend/windmill-worker/src/worker_flow.rs:210

• Draft comment:
  The function update_flow_status_after_job_completion_internal is extremely long and complex. This high complexity makes it hard to reason about error paths and state updates. Consider refactoring into smaller helper functions.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

2. backend/windmill-worker/src/worker_flow.rs:3970

• Draft comment:
  In get_previous_job_result, the query uses unwraps on the returned JSON value. If the query returns an unexpected type or fails parsing, this may panic. Consider more graceful error handling or adding context.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

3. backend/windmill-worker/src/worker_flow.rs:3580

• Draft comment:
  The function next_forloop_status deserializes an iterator from a JSON string. If the JSON isn’t an array, the error message may be uninformative. Consider enriching the error context to aid debugging.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

4. backend/windmill-worker/src/worker_flow.rs:1350

• Draft comment:
  Several raw SQL queries use JSONB_SET and bind parameters directly. Ensure that appropriate database indexes exist on the JSON columns to avoid performance degradation, and verify that the queries are parameterized consistently.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

5. backend/windmill-worker/src/worker_flow.rs:3930

• Draft comment:
  The helper function from_now returns MAX_UTC on overflow. This fallback might hide duration conversion issues. Consider documenting this behavior or returning an error so that callers can decide an appropriate strategy.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

6. backend/windmill-worker/src/worker_flow.rs:3549

• Draft comment:
  The is_simple_modules function checks that failure_module is None to consider the module simple. This might be too strict; document why having an assigned failure module makes the module non‐simple.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

7. backend/windmill-worker/src/handle_child.rs:231

• Draft comment:
  Avoid using unwrap() when sending signals (e.g. SIGINT) with signal::kill. Instead, handle potential errors gracefully to prevent an unexpected panic if the signal cannot be sent.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

8. backend/windmill-worker/src/worker_flow.rs:70

• Draft comment:
  The function 'update_flow_status_after_job_completion' is extremely long and complex, mixing business logic with multiple SQL operations. Consider refactoring it into smaller, more modular helper functions to improve readability, testability, and maintainability.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

9. backend/windmill-worker/src/worker_flow.rs:1360

• Draft comment:
  The SQL queries that update the JSONB 'flow_status' using nested JSONB_SET calls are very complex and repeated in several places. Consider extracting common update patterns into helper functions to improve code readability and reusability.
• Reason this comment was not posted:
  Comment was on unchanged code.

10. backend/windmill-worker/src/worker_flow.rs:2971

• Draft comment:
  The 'insert_iter_arg' function uses recursion to resolve key conflicts by appending '_parent' to keys. This recursive approach may lead to stack overflows if many conflicts occur. Consider refactoring to an iterative solution or add a recursion limit.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

11. backend/windmill-worker/src/worker_flow.rs:1633

• Draft comment:
  There are several instances where unwrap or unwrap_or_else is used (e.g. when retrieving module status) which might cause panics if data is unexpected. Consider using proper error handling (with the '?' operator and contextual messages) to increase robustness.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

12. backend/windmill-worker/src/worker_flow.rs:1433

• Draft comment:
  The code frequently queries and updates JSONB columns (e.g., using jsonb_array_length and JSONB_SET on 'flow_status'). Ensure that appropriate database indexes exist on these columns to maintain query performance.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_ba0IQyDq7OJr1bAM

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

👍 Looks good to me! Incremental review on 513a9bd in 2 minutes and 56 seconds

More details

• Looked at 2922 lines of code in 48 files
• Skipped 0 files when reviewing.
• Skipped posting 9 drafted comments based on config settings.

1. backend/windmill-worker/src/worker_flow.rs:1408

• Draft comment:
  The function update_flow_status_after_job_completion_internal contains several almost identical SQL update queries (for updating flow_status for different steps using JSONB_SET). Consider extracting a helper function to centralize the logic for JSONB updates. This would improve maintainability and reduce duplication.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

2. backend/windmill-worker/src/worker_flow.rs:1611

• Draft comment:
  The push_next_flow_job function and related branches handling (for loops, branch selection, etc.) are extremely long and complex, with similar payload construction logic repeated in multiple match arms. Refactor and break down the logic into smaller, self‑contained functions to improve readability and reduce the risk of bugs.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

3. backend/windmill-worker/src/worker_flow.rs:3700

• Draft comment:
  There are several unwrap_or_else and panic calls (e.g., in payload_from_simple_module and elsewhere) in async contexts. Consider handling these error cases gracefully rather than allowing a panic, to prevent accidental worker crashes.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

4. backend/windmill-worker/src/worker_flow.rs:3592

• Draft comment:
  Performance suggestion: In next_forloop_status, the iterator expression is re-evaluated via eval_timeout each time. Consider caching its result if it remains constant across iterations to reduce repeated heavy computation.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

5. backend/windmill-worker/src/worker_flow.rs:1390

• Draft comment:
  Security/robustness note: The code uses several JSONB_SET operations to update the 'flow_status' column. Ensure that the underlying JSON structure is always as expected; adding validation or fallback mechanisms could prevent unintended data corruption.
• Reason this comment was not posted:
  Comment did not seem useful.

6. backend/windmill-worker/src/worker_flow.rs:3783

• Draft comment:
  Code clarity: In raw_script_to_payload, consider adding inline comments explaining how each field of the payload is constructed. This will help maintainability and ensure correct mapping of script properties.
• Reason this comment was not posted:
  Confidence changes required: 33%
  None

7. backend/windmill-worker/src/worker_flow.rs:3904

• Draft comment:
  Documentation suggestion: In the get_transform_context function, document the structure and expected contents of IdContext to improve code readability and maintainability.
• Reason this comment was not posted:
  Confidence changes required: 33%
  None

8. backend/windmill-worker/src/worker_flow.rs:1300

• Draft comment:
  Logic clarity: In the next_retry function, review the computation (using min(d, MAX_RETRY_INTERVAL) and fail_count increment) to ensure it handles cases (e.g. potential overflows) as intended when fail_count increases.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

9. backend/windmill-worker/src/handle_child.rs:208

• Draft comment:
  Consider extracting the SQL query that updates cancellation reason (in the timeout kill_reason branch) into a named constant or helper function. This would improve code reuse and clarity.
• Reason this comment was not posted:
  Confidence changes required: 33%
  None

Workflow ID: wflow_lI15PANcePYDr2CN

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

❌ Changes requested. Incremental review on 74295ce in 2 minutes and 41 seconds

More details

• Looked at 2922 lines of code in 48 files
• Skipped 0 files when reviewing.
• Skipped posting 10 drafted comments based on config settings.

1. backend/windmill-worker/src/worker_flow.rs:140

• Draft comment:
  The SQL queries using sqlx::query! and jsonb_set are complex but correctly parameterized. Consider adding inline comments to clarify the intent of each JSONB manipulation, as these nested updates are hard to follow.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50%
  None

2. backend/windmill-worker/src/worker_flow.rs:2188

• Draft comment:
  When fetching the args from a previous failed job, the unwrap_or_else on the JSON extraction may hide malformed data. Consider returning a clarified error if the conversion yields an empty hashmap.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

3. backend/windmill-worker/src/worker_flow.rs:2920

• Draft comment:
  The helper function insert_iter_arg uses recursion to resolve key conflicts. Although clever, ensure that deeply nested conflicts won’t lead to stack overflows. A non-recursive approach might be safer for extreme cases.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

4. backend/windmill-worker/src/worker_flow.rs:3590

• Draft comment:
  The code dynamically evaluates JavaScript expressions (in eval_timeout) to compute iterator arrays and boolean predicates. Ensure that proper timeouts and error handling are in place to avoid runaway scripts; also consider documenting the security boundary if untrusted code can be evaluated.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

5. backend/windmill-worker/src/worker_flow.rs:3908

• Draft comment:
  Large functions like push_next_flow_job are highly complex and mix business logic with query execution. Consider refactoring into smaller, well-documented helper functions to ease maintenance and improve testability.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

6. backend/windmill-worker/src/handle_child.rs:231

• Draft comment:
  Avoid using unwrap() when sending OS signals. If signal::kill fails, the process will panic. Consider handling the error gracefully.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

7. backend/windmill-worker/src/handle_child.rs:246

• Draft comment:
  Similarly, in the SIGTERM block the signal::kill call uses unwrap(). Consider handling potential errors instead of panicking.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

8. backend/windmill-worker/src/handle_child.rs:190

• Draft comment:
  The function 'handle_child' is very long and features complex asynchronous logic. Consider refactoring it into smaller helper functions to improve readability and maintainability.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

9. backend/windmill-worker/src/worker_flow.rs:210

• Draft comment:
  The function 'update_flow_status_after_job_completion_internal' is extremely long and deeply nested. Refactoring parts of its logic into separate helper functions would improve clarity and ease future maintenance.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

10. backend/windmill-worker/src/worker_flow.rs:3572

• Draft comment:
  In the 'next_forloop_status' function, when the iterator is empty the code re-computes the iterator using eval_timeout. The error handling in this block could be clarified with more comments or refined error messages to improve debuggability.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_M4tFBYPVA6Llzktt

---

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

There are several similar SQL queries updating JSONB fields (e.g. using JSONB_SET) with complex parameters. Consider abstracting these repeated patterns into helper functions to reduce code duplication and potential errors.

[ellipsis-dev]

👍 Looks good to me! Incremental review on ee6a649 in 1 minute and 26 seconds

More details

• Looked at 2922 lines of code in 48 files
• Skipped 0 files when reviewing.
• Skipped posting 11 drafted comments based on config settings.

1. backend/windmill-worker/src/worker_flow.rs:2230

• Draft comment:
  There are several instances where unwraps and unwrap_or_else are used (e.g. converting JSON strings, cache_ttl extraction) that could lead to panics if unexpected data appears. Consider more robust error handling, possibly using context-aware error propagation.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

2. backend/windmill-worker/src/worker_flow.rs:2232

• Draft comment:
  The SQL queries using jsonb_set, e.g. in 'update_flow_status_after_job_completion_internal', are parameterized, but their complexity makes it hard to reason about correctness. Consider refactoring these updates into dedicated helper functions to improve readability and maintainability.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

3. backend/windmill-worker/src/worker_flow.rs:2910

• Draft comment:
  The function 'push_next_flow_job' is very long and complex. It might benefit from splitting into smaller, well-named helper functions to improve clarity and ease testing.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

4. backend/windmill-worker/src/worker_flow.rs:3640

• Draft comment:
  The use of 'unwrap_or' on optional values (for example, retrieving JSON array lengths or step counts) can lead to silent errors. Consider propagating errors explicitly instead of defaulting to zero, to avoid masking issues.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

5. backend/windmill-worker/src/worker_flow.rs:3950

• Draft comment:
  Several eval_timeout calls pass in user-provided expressions. Ensure that sufficient validation and sandboxing is in place to avoid unexpected script execution; have you reviewed potential injection risks?
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

6. backend/windmill-worker/src/worker_flow.rs:3620

• Draft comment:
  The overall complexity and nesting levels in functions like 'next_forloop_status' make the control flow hard to follow. Consider refactoring certain conditional branches (e.g. handling parallel vs nonparallel cases) into separate, documented helper functions.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

7. backend/windmill-worker/src/handle_child.rs:231

• Draft comment:
  Using unwrap() on the signal::kill call (e.g. SIGINT/SIGTERM) may panic if sending the signal fails. Consider propagating the error or handling it gracefully.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

8. backend/windmill-worker/src/worker_flow.rs:242

• Draft comment:
  When parsing the stored flow status JSON (using serde_json::from_str::<FlowStatus>(record.flow_status.0.get())), any malformed JSON will cause an error. Consider adding more context or fallback behavior if the JSON is not valid.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

9. backend/windmill-worker/src/worker_flow.rs:1320

• Draft comment:
  The function compute_bool_from_expr expects the evaluated expression to return exactly "true" or "false". Consider converting a JSON boolean directly or doing a case‐insensitive comparison to be more robust.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

10. backend/windmill-worker/src/worker_flow.rs:1600

• Draft comment:
  The overall control flow in functions like push_next_flow_job and next_loop_iteration is very complex and deeply nested. Refactoring into smaller helper functions with clear responsibilities would improve readability and maintainability.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

11. backend/windmill-worker/src/worker_flow.rs:3930

• Draft comment:
  There are several instances where unwrap or unwrap_or_default is used on JSON values (e.g. converting job results or args). Verify that these unwraps cannot panic in production, or consider propagating errors instead.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_Z9iWbNatE78zZpCd

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.