-
Notifications
You must be signed in to change notification settings - Fork 14
chore: make initial_tasks NULL by default to indicate 'unknown yet' #215
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View your CI Pipeline Execution ↗ for commit 4665802
☁️ Nx Cloud last updated this comment at |
3de38b1 to
de5ffae
Compare
de5ffae to
2b71eb5
Compare
2b71eb5 to
dd1dde0
Compare
474b8fd to
5a6c7d3
Compare
5a6c7d3 to
7da0caf
Compare
dd1dde0 to
2adddf3
Compare
7da0caf to
a422670
Compare
2adddf3 to
4665802
Compare
🔍 Preview Deployment: Website✅ Deployment successful! 🔗 Preview URL: https://pr-215.pgflow.pages.dev 📝 Details:
_Last updated: _ |
🔍 Preview Deployment: Playground✅ Deployment successful! 🔗 Preview URL: https://pr-215--pgflow-demo.netlify.app 📝 Details:
_Last updated: _ |
Merge activity
|
…215) ### TL;DR Changed `initial_tasks` from hardcoded "1" to `NULL` for dependent map steps, improving semantic correctness. ### What changed? - Modified the database schema to allow `NULL` values for `initial_tasks` in the `step_states` table - Added a constraint to ensure `initial_tasks` is not `NULL` when a step is started - Updated SQL functions to: - Set `initial_tasks = NULL` for dependent map steps during flow creation - Resolve the `NULL` value to the actual array length when dependencies complete - Prevent steps with `NULL` initial_tasks from starting - Moved the implementation plan from `pkgs/core/PLAN_use_null_for_map_initial_tasks.md` to the root directory - Updated the PLAN.md to prioritize this change before other map-related work - Added new tests to verify the NULL behavior works correctly ### How to test? - Run the new test files: - `pkgs/core/supabase/tests/initial_tasks_null/dependent_maps_null.test.sql` - `pkgs/core/supabase/tests/start_flow/dependent_map_initial_tasks_null.test.sql` - Verify that dependent map steps start with `NULL` initial_tasks - Verify that when dependencies complete, `NULL` is resolved to the correct array length - Verify that steps cannot start with `NULL` initial_tasks ### Why make this change? This change improves semantic correctness in the system: - `NULL` correctly represents "unknown until dependencies complete" - The previous approach of using "1" as a placeholder was misleading - This change makes the code more explicit about the state of map steps - It's an important foundation for upcoming map features like array distribution and output aggregation

TL;DR
Changed
initial_tasksfrom hardcoded "1" toNULLfor dependent map steps, improving semantic correctness.What changed?
NULLvalues forinitial_tasksin thestep_statestableinitial_tasksis notNULLwhen a step is startedinitial_tasks = NULLfor dependent map steps during flow creationNULLvalue to the actual array length when dependencies completeNULLinitial_tasks from startingpkgs/core/PLAN_use_null_for_map_initial_tasks.mdto the root directoryHow to test?
pkgs/core/supabase/tests/initial_tasks_null/dependent_maps_null.test.sqlpkgs/core/supabase/tests/start_flow/dependent_map_initial_tasks_null.test.sqlNULLinitial_tasksNULLis resolved to the correct array lengthNULLinitial_tasksWhy make this change?
This change improves semantic correctness in the system:
NULLcorrectly represents "unknown until dependencies complete"