backend: migrate job tables to v2 schema (v2 phase 1)

[uael]

Important

Migrate job tables to v2 schema with new tables, views, policies, and updated queries and functions.

• Database Migrations:
  • Create new tables v2_job_runtime, v2_job_status, v2_job_queue, v2_job_completed with associated policies and constraints.
  • Add views for backward compatibility with old queue and completed_job tables.
  • Add triggers for syncing data between v1 and v2 tables.
  • Add indices for performance optimization on v2_job and v2_job_completed.
• Code Updates:
  • Update queries in monitor.rs, jobs.rs, worker.rs, worker_flow.rs, worker_lockfiles.rs to use new v2 tables and views.
  • Modify functions to handle new schema, including handle_dependency_job, handle_flow_dependency_job, and handle_app_dependency_job.
  • Update test fixtures to reflect new schema changes.
• Miscellaneous:
  • Add integrity check triggers for v2 tables in base.sql.
  • Update fix_job_completed_index function in db.rs to create new indices.

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

[cloudflare-workers-and-pages]

Deploying windmill with    Cloudflare Pages

Latest commit:	a90fd43
Status:	✅  Deploy successful!
Preview URL:	https://b6dcca31.windmill.pages.dev
Branch Preview URL:	https://uael-v2-phase-1.windmill.pages.dev

View logs

[ellipsis-dev]

👍 Looks good to me! Incremental review on c1b63cc in 1 minute and 11 seconds

More details

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

1. backend/migrations/20250117124431_v2_job.up.sql:2

• Draft comment:
  Ensure the script_lang type is created before using it in the v2_job table. Add a CREATE TYPE statement for script_lang if it doesn't already exist.
• Reason this comment was not posted:
  Comment did not seem useful.

2. backend/migrations/20250117124743_v2_job_queue_sync.up.sql:21

• Draft comment:
  Ensure consistent handling of trigger and trigger_kind across the codebase to avoid confusion and potential data integrity issues. Consider renaming trigger to something more descriptive if it stores different types of data based on trigger_kind.
• Reason this comment was not posted:
  Comment did not seem useful.

3. backend/migrations/20250117124744_v2_job_completed_sync.up.sql:25

• Draft comment:
  Ensure consistent handling of trigger and trigger_kind across the codebase to avoid confusion and potential data integrity issues. Consider renaming trigger to something more descriptive if it stores different types of data based on trigger_kind.
• Reason this comment was not posted:
  Marked as duplicate.

4. backend/migrations/20250117124748_v2_migrate_from_v1.up.sql:28

• Draft comment:
  Ensure consistent handling of trigger and trigger_kind across the codebase to avoid confusion and potential data integrity issues. Consider renaming trigger to something more descriptive if it stores different types of data based on trigger_kind.
• Reason this comment was not posted:
  Marked as duplicate.

Workflow ID: wflow_kPWJqOF6pxzleXmz

---

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

[ellipsis-dev]

❌ Changes requested. Incremental review on a45c359 in 1 minute and 54 seconds

More details

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

1. backend/tests/worker.rs:3768

• Draft comment:
  Nice reuse of a closure to run the test with different original label sets. For clarity it might be helpful to give this closure a more descriptive name (e.g. run_label_test) rather than just 'test'.
• Reason this comment was not posted:
  Confidence changes required: 30%
  None

2. backend/tests/worker.rs:3761

• Draft comment:
  Consider explicitly documenting deduplication of job labels. The test expects labels to be formed by chaining original labels with ["yolo", "greet", "Jean Neige"], which implies duplicate entries (like "greet") are deduplicated. Clarifying this behavior would improve readability.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  While the comment correctly identifies that deduplication is happening, this appears to be an internal implementation detail rather than a user-facing feature that requires documentation. The test is focused on verifying the basic functionality of job labels working correctly. Deduplication is likely handled automatically by the database schema or internal processing, and doesn't need to be explicitly documented.
  The comment does identify a real behavior in the code. Perhaps deduplication is an important feature that users should know about when working with job labels?
  While deduplication exists, it's an implementation detail rather than a core feature. Users just need to know they can add labels - the fact that duplicates are removed is not something they need to actively think about or work around. Over-documenting internal details can make code harder to maintain.
  Delete the comment. The deduplication behavior is an implementation detail that doesn't need explicit documentation. The test adequately verifies the behavior without needing additional documentation.

3. backend/tests/worker.rs:3805

• Draft comment:
  The test module is very large and covers a wide range of functionalities (multiple languages, flows, dependencies, schedule handlers, etc.). Consider splitting it into smaller, well‐organized modules and using common helper functions to improve maintainability and reusability.
• Reason this comment was not posted:
  Comment was on unchanged code.

Workflow ID: wflow_LLzP04tLFkqlabZG

---

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]

👍 Looks good to me! Incremental review on 6ce80c1 in 1 minute and 15 seconds

More details

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

1. backend/.sqlx/query-9d518842a9ad90ff9c28dc39690deb0ee6b62cf1d8ae1a02b28c23255d377b3d.json:1

• Draft comment:
  Query looks good; ensure 'labels' as TextArray is expected nullable.
• Reason this comment was not posted:
  Confidence changes required: 0%
  None

2. backend/.sqlx/query-9db563de47401fdfbedecbc52ec07aa5e03ec3adb1312b016639c3b4daa78bc5.json:1

• Draft comment:
  Formatting with newline is acceptable; verify consistency if intentional.
• Reason this comment was not posted:
  Confidence changes required: 0%
  None

3. backend/.sqlx/query-a17034c6c588162726b73c065e8e2995cfbeeafdd344f0c8b0ae863e1072ea40.json:1

• Draft comment:
  This query duplicates the one above; confirm both variants are needed.
• Reason this comment was not posted:
  Comment did not seem useful.

4. backend/.sqlx/query-bd5a0c06e2f2361c9fc670eb0b975b58d65ca93d68b29124d04bd526239b9df2.json:1

• Draft comment:
  Ensure the update condition with '$2::TEXT[] IS NOT NULL' is intentional to skip null updates.
• Reason this comment was not posted:
  Confidence changes required: 33%
  None

5. backend/.sqlx/query-cd5f02cf10cbf92dd1df53a54f2110efa11a7731ad0f0e5509f55efabdf535cd.json:1

• Draft comment:
  Simple select query; no issues found.
• Reason this comment was not posted:
  Confidence changes required: 0%
  None

6. backend/.sqlx/query-9d518842a9ad90ff9c28dc39690deb0ee6b62cf1d8ae1a02b28c23255d377b3d.json:7

• Draft comment:
  Query for 'labels' looks good. Confirm that 'nullable': true is intended when no labels are assigned.
• Reason this comment was not posted:
  Confidence changes required: 0%
  None

7. backend/.sqlx/query-9db563de47401fdfbedecbc52ec07aa5e03ec3adb1312b016639c3b4daa78bc5.json:3

• Draft comment:
  Consider normalizing whitespace formatting in the query (extra spaces before FROM clause).
• Reason this comment was not posted:
  Confidence changes required: 33%
  None

8. backend/.sqlx/query-a17034c6c588162726b73c065e8e2995cfbeeafdd344f0c8b0ae863e1072ea40.json:2

• Draft comment:
  This query is nearly identical to the one in query-9db563… file (aside from whitespace). Consider consolidating if both are not needed.
• Reason this comment was not posted:
  Marked as duplicate.

9. backend/.sqlx/query-bd5a0c06e2f2361c9fc670eb0b975b58d65ca93d68b29124d04bd526239b9df2.json:3

• Draft comment:
  Verify that the condition '$2::TEXT[] IS NOT NULL' is intentional, as it prevents updating labels to NULL.
• Reason this comment was not posted:
  Comment did not seem useful.

10. backend/.sqlx/query-cd5f02cf10cbf92dd1df53a54f2110efa11a7731ad0f0e5509f55efabdf535cd.json:2

• Draft comment:
  SELECT query for 'preprocessed' is straightforward; no issues found.
• Reason this comment was not posted:
  Confidence changes required: 0%
  None

Workflow ID: wflow_YNJhjSAIO5RUG0lx

---

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

[ellipsis-dev]

👍 Looks good to me! Incremental review on abfb5f5 in 1 minute and 13 seconds

More details

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

1. backend/tests/worker.rs:3767

• Draft comment:
  The test_job_labels function uses raw SQL and manual label sorting. Consider adding comments to clarify the intended labels transformation and confirm that duplicate labels (like "greet") are handled intentionally.
• 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/tests/worker.rs:3761

• Draft comment:
  This test file is very extensive and contains many almost-duplicated patterns across language integrations and payload types. Consider refactoring or splitting into submodules/files for improved maintainability and 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.

3. backend/tests/worker.rs:3885

• Draft comment:
  Many tests use multiple unwrap() calls. While acceptable in test code, using expect() with informative messages could aid in debugging failures.
• 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/tests/worker.rs:3820

• Draft comment:
  The test_job_labels function compares the database-returned labels to a sorted expected list. It is assumed that the labels stored in v2_job are sorted. It might be more robust to explicitly sort the fetched labels before comparing to avoid ordering issues.
• Reason this comment was not posted:
  Comment was on unchanged code.

5. backend/tests/worker.rs:100

• Draft comment:
  This test file is very large with many inline script strings and JSON literals. Consider extracting common scripts or JSON fixtures into constants/files to improve maintainability and readability.
• Reason this comment was not posted:
  Confidence changes required: 50%
  None

6. backend/tests/worker.rs:3837

• Draft comment:
  The version flags are managed using lazy_static with Arc<RwLock>. For modern code clarity consider using 'once_cell' or other patterns to reduce boilerplate.
• Reason this comment was not posted:
  Confidence changes required: 30%
  None

7. backend/tests/worker.rs:4135

• Draft comment:
  There are many unwrap() calls throughout the tests. While acceptable in a testing context, adding error messages can help diagnose failures quickly.
• Reason this comment was not posted:
  Confidence changes required: 30%
  None

8. backend/tests/worker.rs:3447

• Draft comment:
  The integration tests are very comprehensive. However, consider splitting this huge file into multiple test modules (e.g., job labels, script languages, schedule tests) to improve modularity and ease maintainability.
• Reason this comment was not posted:
  Confidence changes required: 40%
  None

Workflow ID: wflow_23Ll0hdZM8b4Aecd

---

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

[uael]

@HugoCasa

[ellipsis-dev]

👍 Looks good to me! Incremental review on 013d112 in 1 minute and 4 seconds

More details

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

1. backend/tests/worker.rs:3924

• Draft comment:
  This test file is extremely long and covers many cases. Consider splitting or modularizing some of these tests to 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.

2. backend/tests/worker.rs:3940

• Draft comment:
  Multiple tests use hard-coded magic numbers (e.g. version numbers like 1443253234253454) and literal strings for schedules. Consider extracting these into named constants for 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.

3. backend/tests/worker.rs:3189

• Draft comment:
  Many tests are using direct unwrap and expect calls. While acceptable in tests, adding more context to errors (via .expect with messages) can help debugging failures.
• 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/tests/worker.rs:3197

• Draft comment:
  Timeout values (e.g. Duration::from_millis(5000)) used in scheduled job tests could be potentially brittle under load. Consider reviewing these timeout durations for stability.
• 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/tests/worker.rs:3920

• Draft comment:
  The migration tests now reference the new v2 schema (e.g., queries on v2_job). Ensure that these tests are isolated and that any schema-specific SQL (like JSON extraction on the 'result' column) remains robust across PostgreSQL versions.
• 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/tests/worker.rs:4260

• Draft comment:
  There is significant duplication in test patterns (e.g., for flows, dependencies, script hash payloads, etc.). Consider refactoring repetitive patterns into helper functions to reduce code duplication and ease future changes.
• 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/tests/worker.rs:3925

• Draft comment:
  There’s a TODO regarding checking _metadata in flow_status (v2 phase 4). Ensure you either add a clear migration plan or remove the temporary check once v2 is finalized.
• 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/tests/worker.rs:4240

• Draft comment:
  Magic version numbers (e.g. 1443253234253454) appear in multiple tests. Consider replacing them with a named constant to 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.

9. backend/tests/worker.rs:3828

• Draft comment:
  The helper function 'test_for_versions' is very useful for iterative testing over version flags. Adding some inline documentation on its purpose and usage would improve code readability.
• Reason this comment was not posted:
  Confidence changes required: 30%
  None

10. backend/tests/worker.rs:3596

• Draft comment:
  Tests covering relative imports for Bun, Deno, and Python are thorough. To reduce duplication, consider parameterizing these tests or factoring common logic into shared helper functions.
• Reason this comment was not posted:
  Confidence changes required: 40%
  None

11. backend/tests/worker.rs:3768

• Draft comment:
  While using unwrap() is acceptable in tests, providing context messages (or using expect with descriptive errors) can improve debugging when tests fail.
• Reason this comment was not posted:
  Confidence changes required: 30%
  None

Workflow ID: wflow_aKwwhJ6cOdoA65wB

---

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

[ellipsis-dev]

👍 Looks good to me! Incremental review on 7519a8b in 49 seconds

More details

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

1. backend/windmill-worker/src/worker_lockfiles.rs:163

• Draft comment:
  Avoid using unwrap() when calling to_str() on the normalized path. Consider handling the None case to avoid panics when non-UTF8 paths are encountered.
• 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_lockfiles.rs:99

• Draft comment:
  Consider clarifying the SQL query cast syntax in dependency map insertion; embedding $4::text::IMPORTER_KIND might benefit from a comment or refactoring for readability.
• Reason this comment was not posted:
  Confidence changes required: 50%
  None

3. backend/windmill-worker/src/worker_lockfiles.rs:244

• Draft comment:
  Check handling of errors when canonicalizing relative import paths. Instead of logging an error with unwrap() (line 197), 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.

4. backend/windmill-worker/src/worker_lockfiles.rs:928

• Draft comment:
  Box::pin is used when calling async functions like lock_modules within reduce_flow. Consider if it is necessary, as excessive boxing may add overhead. A plain .await might suffice.
• Reason this comment was not posted:
  Confidence changes required: 50%
  None

5. backend/windmill-worker/src/worker_lockfiles.rs:1307

• Draft comment:
  In skip_creating_new_lock, the logic for TypeScript annotations switch between Bun and Bunnative might be error-prone if annotations change; ensure tests cover these edge 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.

6. backend/windmill-worker/src/worker_lockfiles.rs:163

• Draft comment:
  Avoid using unwrap() on to_str() in 'try_normalize'. Consider propagating an error instead of panicking on non-UTF8 paths.
• 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_lockfiles.rs:790

• Draft comment:
  Consider using #[async_recursion] on the 'lock_modules' function to avoid manually boxing recursive async calls with Box::pin.
• 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_lockfiles.rs:1267

• Draft comment:
  In 'reduce_app', the inlineScript branch unwraps 'content' without graceful error handling. Consider propagating an error if 'content' is missing.
• 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_lockfiles.rs:1

• Draft comment:
  This file is quite large and complex. Consider refactoring into smaller modules to improve maintainability and readability.
• 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_BAaHepeEC0NVp753

---

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

[ellipsis-dev]

👍 Looks good to me! Incremental review on a16f275 in 37 seconds

More details

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

1. backend/tests/worker.rs:5

• Draft comment:
  Multiple unwrap() calls are used throughout tests. While acceptable in tests, consider adding contextual error messages for clarity when failures occur.
• 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/tests/worker.rs:1020

• Draft comment:
  The worker spawning logic (e.g., in spawn_test_worker) is duplicated and complex. Consider refactoring this into a reusable helper with clear documentation to 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.

3. backend/tests/worker.rs:3200

• Draft comment:
  Fixed timeout durations (e.g., 5000 ms) in asynchronous test waits may lead to flakiness in CI. Consider parameterizing these or adding retries to improve test stability.
• 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/tests/worker.rs:3828

• Draft comment:
  The test_for_versions helper, which iterates over version flags, is a bit complex. Adding inline comments to clarify its purpose and expected behavior would help 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.

5. backend/tests/worker.rs:3745

• Draft comment:
  When executing queries that update or fetch labels (e.g., in test_job_labels), ensure that order or duplicate values are handled consistently. Document assumptions about label sorting if order matters.
• 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/tests/worker.rs:980

• Draft comment:
  Refactor common worker initialization logic in functions like spawn_test_worker and in_test_worker. This repetition makes maintenance harder.
• 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/tests/worker.rs:80

• Draft comment:
  Consider moving global test setup (e.g. initialize_tracing and JWT secret setup) into a shared test fixture to avoid redundant calls.
• 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/tests/worker.rs:2420

• Draft comment:
  Avoid overuse of unwrap() in tests; consider using expect with descriptive error messages or proper error propagation to ease 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.

9. backend/tests/worker.rs:3547

• Draft comment:
  The functions run_deployed_relative_imports and run_preview_relative_imports share similar structure. Consider consolidating them 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.

10. backend/tests/worker.rs:3880

• Draft comment:
  The test file is very long and covers numerous scenarios. Splitting tests into multiple modules or files could 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/tests/worker.rs:4250

• Draft comment:
  Consider adding brief documentation or inline comments to explain the purpose of complex test helpers like run_until_complete_with.
• Reason this comment was not posted:
  Confidence changes required: 50%
  None

Workflow ID: wflow_qq5PUgqoL9XZodHR

---

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

[ellipsis-dev]

❌ Changes requested. Incremental review on e9d0c72 in 1 minute and 16 seconds

More details

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

1. backend/migrations/20250201124743_v2_job_queue_sync.up.sql:160

• Draft comment:
  Consider adding an ELSE clause to preserve the current value of 'preprocessed'. This avoids setting it to NULL.
• Reason this comment was not posted:
  Confidence changes required: 0%
  None

2. backend/migrations/20250201124744_v2_job_completed_sync.up.sql:77

• Draft comment:
  Double-check the business logic change from checking 'script_entrypoint_override' to 'job.kind'. Ensure this condition matches expected behavior.
• Reason this comment was not posted:
  Confidence changes required: 33%
  None

3. backend/migrations/20250201124743_v2_job_queue_sync.up.sql:160

• Draft comment:
  Using ELSE preprocessed preserves the value when not FALSE. Confirm this is the intended logic.
• Reason this comment was not posted:
  Confidence changes required: 10%
  None

4. backend/migrations/20250201124744_v2_job_completed_sync.up.sql:77

• Draft comment:
  Changing the condition from checking script_entrypoint_override to job.kind alters the semantics. Verify that 'job.kind = "script"' correctly captures the intended scenario.
• Reason this comment was not posted:
  Confidence changes required: 50%
  None

5. backend/migrations/20250201124744_v2_job_completed_sync.up.sql:146

• Draft comment:
  The v2_job_completed_before_update trigger function is missing a RETURN NEW statement. All BEFORE triggers must return NEW.
• Reason this comment was not posted:
  Marked as duplicate.

Workflow ID: wflow_LRNW1W3yVDsaXdE0

---

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]

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

More details

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

1. backend/migrations/20250201124744_v2_job_completed_sync.up.sql:146

• Draft comment:
  Ensure that returning NULL in the BEFORE UPDATE trigger is intentional, as it cancels the row update. If you want the update to apply changes to v2_job_completed, consider returning NEW.
• Reason this comment was not posted:
  Comment did not seem useful.

2. backend/migrations/20250201124744_v2_job_completed_sync.down.sql:3

• Draft comment:
  Dropping the v2_job_completed_before_update function appears correct. Just double-check dependencies to ensure no unintended side effects.
• Reason this comment was not posted:
  Confidence changes required: 33%
  None

3. backend/migrations/20250201124744_v2_job_completed_sync.down.sql:3

• Draft comment:
  Consider using DROP FUNCTION IF EXISTS to avoid errors if the function is already absent.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable:
  Migration scripts should be deterministic and idempotent. However, down migrations are typically only run once to revert a specific change. IF EXISTS is more relevant for idempotent scripts or general maintenance scripts, not for specific version migrations. The function should definitely exist when running this down migration, as it was created in the corresponding up migration.
  IF EXISTS could make the script more robust against edge cases where the function was already manually dropped. It's a defensive programming practice.
  In a versioned migration system, we expect exact state transitions. Adding IF EXISTS could mask real problems and make debugging harder if something is unexpectedly missing.
  The comment should be deleted. Using IF EXISTS here would add unnecessary complexity and could hide real issues in the migration process.

4. backend/migrations/20250201124744_v2_job_completed_sync.up.sql:146

• Draft comment:
  Returning NULL in a BEFORE UPDATE trigger cancels the update on v2_job_completed. Confirm if this suppression is intended.
• Reason this comment was not posted:
  Marked as duplicate.

Workflow ID: wflow_EQ5Y6Mrl8Q4UlrDX

---

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

[ellipsis-dev]

👍 Looks good to me! Incremental review on aa95bb4 in 2 minutes and 17 seconds

More details

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

1. backend/windmill-worker/src/worker_lockfiles.rs:1307

• Draft comment:
  The function 'skip_creating_new_lock' uses an inverted condition which may confuse; consider renaming or adding a comment for 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.

2. backend/windmill-worker/src/worker_lockfiles.rs:184

• Draft comment:
  Repeated SQL cast notation could be refactored for clarity by extracting constants or helper macros.
• 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_lockfiles.rs:894

• Draft comment:
  Recursive calls with Box::pin in 'lock_modules' may risk stack overflow; consider refactoring to iterative approach.
• 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_lockfiles.rs:637

• Draft comment:
  The cancellation check using fetch_optional silently returns false on error; consider propagating the error if appropriate.
• Reason this comment was not posted:
  Comment was on unchanged code.

5. backend/windmill-worker/src/worker_lockfiles.rs:1256

• Draft comment:
  The 'reduce_flow' function is lengthy and complex; splitting it into smaller helper functions with documentation could 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.

6. backend/windmill-worker/src/worker_lockfiles.rs:160

• Draft comment:
  The function try_normalize returns None for Prefix/RootDir components. Consider returning a more descriptive error (or a Result) to aid debugging when a path cannot be normalized.
• 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_lockfiles.rs:1308

• Draft comment:
  The skip_creating_new_lock function’s logic is a bit confusing due to its naming and conditional behavior. It may be clearer to rename it (e.g., should_create_new_lock) or add inline comments explaining when it returns true versus false.
• 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_lockfiles.rs:1164

• Draft comment:
  In reduce_flow, module.value is repeatedly deserialized from JSON. It might improve performance and clarity to cache or memoize the parsed value to avoid duplicate work.
• 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_lockfiles.rs:230

• Draft comment:
  In handle_dependency_job, after updating the script lock and invalidating the cache, errors from handle_deployment_metadata are only logged. Consider whether deployment metadata failures should be propagated or retried rather than silently logged.
• 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_lockfiles.rs:416

• Draft comment:
  In trigger_dependents_to_recompute_dependencies, the query uses array_agg on importer_node_id. Please confirm that the aggregation handles NULL values correctly and that the resulting array is handled properly downstream.
• 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_lockfiles.rs:760

• Draft comment:
  The recursive locking functions (lock_modules and lock_modules_app) have complex control flow. Adding more inline comments explaining each branch would improve maintainability and ease future 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.

12. backend/windmill-worker/src/worker_lockfiles.rs:1668

• Draft comment:
  In capture_dependency_job, ensure that each branch for different ScriptLang variants (e.g., Python3, Bun, Php, etc.) properly handles edge cases and malformed data. Additional tests for these branches may help avoid silent failures.
• 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_QFEnzgwoUAHGUuJU

---

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

[ellipsis-dev]

👍 Looks good to me! Incremental review on 6dc830c in 55 seconds

More details

• Looked at 3506 lines of code in 81 files
• Skipped 0 files when reviewing.
• Skipped posting 7 drafted comments based on config settings.

1. backend/windmill-worker/src/worker_lockfiles.rs:189

• Draft comment:
  Avoid using unwrap() after try_normalize. Use a safe conversion (e.g. expect with a descriptive message) to convert PathBuf to string, so that an unexpected non-UTF8 path won’t panic.
• 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_lockfiles.rs:760

• Draft comment:
  The lock_modules function is very complex. Consider refactoring or adding 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.

3. backend/windmill-worker/src/worker_lockfiles.rs:189

• Draft comment:
  Consider handling non‐UTF8 paths gracefully in try_normalize. Currently, when calling to_str().unwrap() (line 189–195) in parse_bun_relative_imports, the code may panic if the normalized path isn’t valid UTF‑8.
• 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_lockfiles.rs:765

• Draft comment:
  In the lock_modules function, e.get_value() is called more than once. Storing the result of e.get_value()? in a local variable would avoid redundant computation and potential inconsistency if the value changes.
• 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_lockfiles.rs:1307

• Draft comment:
  The skip_creating_new_lock function’s logic is not immediately clear. Adding a comment describing why locks are not recreated for certain Bun/Bunnative conditions would improve readability.
• 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_lockfiles.rs:66

• Draft comment:
  Transaction handling in functions like update_script_dependency_map and later in lock_modules uses successive reassignments of tx (e.g. lines 66–81, 142–158). Consider refactoring these to reduce chaining of transaction reassignments for clarity and to reduce error‐proneness.
• 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_lockfiles.rs:1160

• Draft comment:
  The file is very long and bundles many responsibilities (dependency map updates, lockfile generation, flow locking, app dependency handling, etc.). Consider splitting it into multiple modules based on functionality to improve code 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_PYx5SESCY6kbLgHx

---

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

[ellipsis-dev]

👍 Looks good to me! Incremental review on 56653fd in 1 minute and 24 seconds

More details

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

1. backend/windmill-worker/src/worker_lockfiles.rs:1308

• Draft comment:
  The function skip_creating_new_lock always returns true if none of the special conditions for Bun/Bunnative are met. It might be worth clarifying the intent here – should other languages always skip creating a new lock, or is this function intended only for Bun/Bunnative scripts?
• 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_lockfiles.rs:163

• Draft comment:
  In try_normalize, encountering a RootDir or Prefix immediately returns None. Ensure this behavior is intended, since some absolute paths might be valid in your domain. Consider adding a comment to explain the rationale if appropriate.
• Reason this comment was not posted:
  Confidence changes required: 50%
  None

3. backend/windmill-worker/src/worker_lockfiles.rs:889

• Draft comment:
  The use of Box::pin(lock_modules(...)).await inside several match arms (e.g. in lock_modules) increases complexity. Consider refactoring these recursive calls into smaller helper functions for increased clarity 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.

4. backend/windmill-worker/src/worker_lockfiles.rs:1164

• Draft comment:
  The function reduce_flow iterates and recursively processes modules. Its complexity is high. Consider splitting it up or adding detailed inline comments describing the transformation logic to improve readability.
• 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_lockfiles.rs:189

• Draft comment:
  Consider using Rust’s Path API (e.g. Path::join or PathBuf::push) instead of string formatting to compute relative paths, for increased 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.

6. backend/windmill-worker/src/worker_lockfiles.rs:160

• Draft comment:
  If normalization fails (returns None), log a warning to aid debugging instead of failing silently.
• 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_lockfiles.rs:765

• Draft comment:
  The lock_modules function is complex. Consider refactoring by extracting helper functions to simplify the nested async calls and 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.

8. backend/windmill-worker/src/worker_lockfiles.rs:1167

• Draft comment:
  Add a comment explaining why replacing the module value with 'Identity' is safe and what it signifies.
• 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_lockfiles.rs:414

• Draft comment:
  Improve error handling in trigger_dependents_to_recompute_dependencies by adding more detailed logging if expected importer information is missing.
• 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_caohyKdnFi1rUHJ0

---

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

[ellipsis-dev]

👍 Looks good to me! Incremental review on a51f03c in 1 minute and 7 seconds

More details

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

1. backend/windmill-worker/src/worker_lockfiles.rs:160

• Draft comment:
  In the try_normalize function, components like Prefix/RootDir immediately return None without logging or further explanation. Consider adding logging or clearer documentation so that callers know why normalization failed.
• 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_lockfiles.rs:188

• Draft comment:
  In parse_bun_relative_imports, if try_normalize returns None (failure to canonicalize a relative import), an error is logged but the import is simply skipped. Consider handling such cases explicitly or warning the caller—silent skipping might hide 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.

3. backend/windmill-worker/src/worker_lockfiles.rs:1267

• Draft comment:
  The reduce_app function uses unwrap on inline script content (e.g. in line 1267) which may panic if the expected JSON field is missing. Consider handling the error gracefully instead of unwrapping.
• 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_lockfiles.rs:189

• Draft comment:
  In parse_bun_relative_imports, the call to normalized.to_str().unwrap() may panic if the normalized path is not valid UTF‐8. Consider handling non‐UTF-8 paths gracefully to avoid unexpected panics.
• 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_lockfiles.rs:1307

• Draft comment:
  The logic in skip_creating_new_lock looks non-obvious. It returns false for Bun/Bunnative only when the TypeScriptAnnotations indicate a native mismatch, and true for all other cases. Please verify that this behavior correctly reflects the intended semantics.
• 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_lockfiles.rs:1156

• Draft comment:
  Functions like reduce_flow and lock_modules involve deep nesting with multiple transaction reassignments and extensive cloning. Consider refactoring these into smaller, well‐named helper functions to improve readability and reduce potential for errors.
• 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_lockfiles.rs:1440

• Draft comment:
  There are several recursive sections (e.g. lock_modules_app) that perform multiple clone() operations on large data structures. Consider optimizing to reduce unnecessary cloning to avoid performance overhead.
• 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_lockfiles.rs:98

• Draft comment:
  In the SQL query within add_relative_imports_to_dependency_map, the importer_kind column is cast using "$4::text::IMPORTER_KIND". Ensure that the custom SQL type IMPORTER_KIND is defined and that this casting is robust across all environments.
• 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_lockfiles.rs:1

• Draft comment:
  This file is quite large and includes many complex functions handling dependency locking, script processing, and deployment metadata updates. Consider splitting this module into smaller, focused submodules to improve maintainability and facilitate 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.

Workflow ID: wflow_D8zgt5VmmBvN7rzS

---

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

[ellipsis-dev]

👍 Looks good to me! Incremental review on 7b01308 in 1 minute and 4 seconds

More details

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

1. backend/windmill-worker/src/worker_lockfiles.rs:160

• Draft comment:
  In try_normalize, consider handling the case where a component’s OS string might not be valid UTF‑8. Currently, the function calls to_str().unwrap() (via later usage) without checking.
• 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_lockfiles.rs:120

• Draft comment:
  The SQL query in clear_dependency_map_for_item casts importer_kind as '$3::text::IMPORTER_KIND'. Ensure that the IMPORTER_KIND type is defined as expected in the database schema; otherwise, this might risk type mismatch 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.

3. backend/windmill-worker/src/worker_lockfiles.rs:181

• Draft comment:
  In parse_bun_relative_imports, the approach trims '.ts' suffix and resolves relative imports by formatting a parent path with '../'. Verify that this logic covers all relevant edge cases and consider adding comments to explain the rationale.
• 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_lockfiles.rs:944

• Draft comment:
  In lock_modules, when checking if a new lock should be created, the function calls skip_creating_new_lock and uses unwrap_or_default on node ids. Ensure that use of unwrap_or_default here cannot mask critical errors and that the logic is documented.
• 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_lockfiles.rs:719

• Draft comment:
  In get_deployment_msg_and_parent_path_from_args, the code repeatedly parses JSON strings from the args map. Consider caching the parsed values or using helper functions to avoid 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.

6. backend/windmill-worker/src/worker_lockfiles.rs:189

• Draft comment:
  Consider using Rust’s Path API (e.g. using Path::new(script_path).join("..").join(import)) for joining paths instead of string formatting. This approach is more robust for path normalization.
• 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_lockfiles.rs:1307

• Draft comment:
  The logic in skip_creating_new_lock is a bit unclear. Consider renaming the function or adding inline comments to explain when a new lock should be skipped versus created, especially regarding the native flag in TypeScript annotations.
• 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_lockfiles.rs:189

• Draft comment:
  Using unwrap() on the result of to_str() in try_normalize may panic if the normalized path is not valid UTF-8. Consider handling non-UTF8 paths 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.

9. backend/windmill-worker/src/worker_lockfiles.rs:1668

• Draft comment:
  The capture_dependency_job function has a very large match on job_language handling many languages. Refactoring language-specific branches into separate helper functions 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.

10. backend/windmill-worker/src/worker_lockfiles.rs:764

• Draft comment:
  The use of Box::pin for async recursion in the lock_modules function makes the code harder to read. Consider adding a comment explaining this pattern or refactoring to simplify async recursion.
• 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_pXkyHHS5QWHdczUP

---

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

[ellipsis-dev]

👍 Looks good to me! Incremental review on 27582db in 1 minute and 51 seconds

More details

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

1. backend/windmill-worker/src/worker_lockfiles.rs:173

• Draft comment:
  Avoid using unwrap() on normalized paths (e.g. using to_str().unwrap() at L173) – consider propagating an error or using safe conversion to prevent panics on non-unicode paths.
• 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_lockfiles.rs:718

• Draft comment:
  Simplify the chaining in get_deployment_msg_and_parent_path_from_args(), as the multiple clones and map/flatten calls reduce readability.
• 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_lockfiles.rs:755

• Draft comment:
  Consider refactoring the complex logic in lock_modules() to split nested match arms into smaller helper functions for better 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.

4. backend/windmill-worker/src/worker_lockfiles.rs:790

• Draft comment:
  Review the use of Box::pin(...).await in recursive calls (e.g. in lock_modules and its branches) for potential inefficiencies or stack issues; document the intent.
• 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_lockfiles.rs:1307

• Draft comment:
  Ensure that the logic in skip_creating_new_lock() correctly reflects the intended cases for Bun/Bunnative languages; consider adding a comment explaining the rationale.
• 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_lockfiles.rs:1160

• Draft comment:
  Add inline documentation to functions such as reduce_flow() and insert_flow_node() to clarify how they transform flow module data.
• 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_lockfiles.rs:1400

• Draft comment:
  Avoid excessive use of unwrap_or_default() when handling optional fields; prefer proper error propagation so unexpected empty strings don’t mask underlying 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.

8. backend/windmill-worker/src/worker_lockfiles.rs:188

• Draft comment:
  Avoid using unwrap() when converting a normalized path to string. In parse_bun_relative_imports, consider using to_string_lossy() to handle non-UTF8 paths 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.

9. backend/windmill-worker/src/worker_lockfiles.rs:767

• Draft comment:
  Consider caching the result of e.get_value() instead of calling it twice in lock_modules. This will improve readability and avoid duplicate work.
• 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_lockfiles.rs:1307

• Draft comment:
  Add a clarifying comment in skip_creating_new_lock regarding the language-specific conditions. The current logic based on TypeScriptAnnotations and native flag can be confusing.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50%
  None

11. backend/windmill-worker/src/worker_lockfiles.rs:1053

• Draft comment:
  Ensure safe conversion of inserted values in insert_flow_node. If any conversion uses unwrap() on path string conversion, consider handling errors more gracefully (e.g. using to_string_lossy()).
• 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_lockfiles.rs:1244

• Draft comment:
  Consider refactoring large, deeply nested functions (e.g. lock_modules and reduce_flow) into smaller helpers to improve readability and maintainability.
• Reason this comment was not posted:
  Confidence changes required: 40% <= threshold 50%
  None

Workflow ID: wflow_hCkLWh2TY3kCyDhr

---

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