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

[uael]

Important

This PR updates the backend to improve database schema compatibility and performance, including new tables, views, and indexes, and modifies worker and API components to handle these changes.

• Database Migrations:
  • Add v2_job_runtime and v2_job_status tables with associated policies and grants.
  • Create views for queue and completed_job to ensure compatibility with new tables.
  • Add triggers for synchronization between v2_* tables and legacy tables.
  • Update constraints and defaults for v2_job and v2_job_queue.
• SQL Query Updates:
  • Use explicit column names and types in queries across multiple files (e.g., monitor.rs, jobs.rs).
  • Add new indexes to improve query performance.
• Worker and API Changes:
  • Update worker_flow.rs and worker_lockfiles.rs to handle new job status and runtime tables.
  • Modify jobs.rs and auth.rs to align with updated database schema.
  • Ensure proper handling of job states and synchronization in monitor.rs and worker.rs.

^{This description was created by for 513a9bd. 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:	513a9bd
Status:	✅  Deploy successful!
Preview URL:	https://e7b05ef0.windmill.pages.dev
Branch Preview URL:	https://uael-v2-phase-2.windmill.pages.dev

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.