Skip to content

Commit 2b71eb5

Browse files
committed
chore: make initial_tasks NULL by default to indicate 'unknown yet'
1 parent 474b8fd commit 2b71eb5

17 files changed

+1290
-260
lines changed

.claude/schema_development.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,21 @@ git rm -f supabase/migrations/*_pgflow_{TEMP,temp}_*.sql
110110
**Temp Migrations**: Use TEMP_ prefix for stacked PRs, remove before final merge, CI enforces this
111111
**Avoid**: Manual migration edits, forgetting to remove old migration, skipping hash reset, failing tests, mixing changes, merging temp migrations to main
112112

113+
### Performance-First SQL Design
114+
115+
**Use Section Comments Instead of Helper Functions**: Keep complex functions monolithic for performance. Use clear section comments:
116+
117+
```sql
118+
-- ==========================================
119+
-- MAIN SECTION: Description
120+
-- ==========================================
121+
WITH
122+
-- ---------- Subsection ----------
123+
cte_name AS (...)
124+
```
125+
126+
Avoids function call overhead, preserves CTE optimization, simpler atomicity.
127+
113128
## Troubleshooting
114129

115130
### Migration name exists

PLAN.md

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,13 @@
8787
- Array validation and count propagation working
8888
- Cascade handles taskless dependent maps
8989

90+
- [x] **PR #213: NULL for Unknown initial_tasks** - `09-16-make-initial-tasks-nullable`
91+
- Changed initial_tasks from "1 as placeholder" to NULL for dependent map steps
92+
- Benefits: Semantic correctness (NULL = unknown, not "1 task")
93+
- Implemented: Schema change to allow NULL, updated all SQL functions
94+
- Added validation for non-array and NULL outputs to map steps
95+
- Comprehensive tests for NULL behavior and error cases
96+
9097
#### ❌ Remaining Work
9198

9299
- [ ] **Array Element Distribution** (CRITICAL - BLOCKS REAL MAP USAGE)
@@ -102,6 +109,8 @@
102109
- Store aggregated output for dependent steps to consume
103110
- Maintain task_index ordering in aggregated arrays
104111
- Tests for aggregation with actual map task outputs
112+
- **IMPORTANT**: Must add test for map->map NULL propagation when this is implemented
113+
- **IMPORTANT**: Must handle non-array outputs to map steps (should fail the run)
105114

106115
- [ ] **DSL Support for .map() Step Type**
107116

@@ -124,23 +133,41 @@
124133
- Basic happy path coverage
125134
- This should be minimal and added to the Edge Worker integration test suite for now
126135

127-
- [ ] **Semantic Improvement: NULL for Unknown initial_tasks** (OPTIONAL - Can be deferred)
128-
129-
- Change initial_tasks from "1 as placeholder" to NULL for dependent map steps
130-
- Benefits: Semantic correctness (NULL = unknown, not "1 task")
131-
- Scope: Schema change to allow NULL, update 5+ SQL functions
132-
- See detailed plan in `pkgs/core/PLAN_use_null_for_map_initial_tasks.md`
133-
- **Note**: This is a semantic improvement only - current approach works functionally
134-
- **Warning**: If deferred, new tests for Array Distribution and Output Aggregation will
135-
assume initial_tasks = 1 for dependent maps, making this change harder later
136-
137136
- [ ] **Migration Consolidation**
138137

139138
- Remove all temporary/incremental migrations from feature branches
140139
- Generate a single consolidated migration for the entire map infrastructure
141140
- Ensure clean migration path from current production schema
142141
- If NULL improvement is done, include it in the consolidated migration
143142

143+
- [ ] **Update README's** and **Docs**
144+
145+
- `pkgs/core/README.md`
146+
147+
- Add new section describing the step types
148+
- Describe single step briefly, focus on describing map step type and how it differs
149+
- Make sure to mention that maps are constrained to have exactly one dependency
150+
- Show multiple cases of inputs -> task creation
151+
- Explain edge cases (empty array propagation, invalid array input)
152+
- Explain root map vs dependent map and how it gets handled and what restrictions those apply on the Flow input
153+
- Explain cascade completion of taskless steps and its limitations
154+
155+
- `pkgs/dsl/README.md`
156+
157+
- Briefly describe the new `.array()` and `.map()` methods
158+
- Mention `.array` is mostly sugar, and `.map` is the new step type
159+
- Link to `pkgs/core/README.md` sections for more explanation about map steps
160+
- Make sure to mention that maps are constrained to have exactly one dependency
161+
162+
- **Add basic docs page**
163+
164+
- put it into `pkgs/website/src/content/docs/concepts/array-and-map-steps.mdx`
165+
- describe the DSL and how the map works and why we need it
166+
- show example usage of root map
167+
- show example usage of dependent map
168+
- focus mostly on how to use it, instead of how it works under the hood
169+
- link to the README's for more details
170+
144171
- [ ] **Graphite Stack Merge**
145172

146173
- Configure Graphite merge queue for the complete PR stack

pkgs/core/PLAN_use_null_for_map_initial_tasks.md

Lines changed: 0 additions & 195 deletions
This file was deleted.

pkgs/core/schemas/0060_tables_runtime.sql

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ create table pgflow.step_states (
2727
step_slug text not null,
2828
status text not null default 'created',
2929
remaining_tasks int null, -- NULL = not started, >0 = active countdown
30-
initial_tasks int not null default 1 check (initial_tasks >= 0), -- Planned task count: 1 for singles, N for maps
30+
initial_tasks int null check (initial_tasks is null or initial_tasks >= 0),
3131
remaining_deps int not null default 0 check (remaining_deps >= 0),
3232
error_message text,
3333
created_at timestamptz not null default now(),
@@ -43,6 +43,9 @@ create table pgflow.step_states (
4343
constraint remaining_tasks_state_consistency check (
4444
remaining_tasks is null or status != 'created'
4545
),
46+
constraint initial_tasks_known_when_started check (
47+
status != 'started' or initial_tasks is not null
48+
),
4649
constraint completed_at_or_failed_at check (not (completed_at is not null and failed_at is not null)),
4750
constraint started_at_is_after_created_at check (started_at is null or started_at >= created_at),
4851
constraint completed_at_is_after_started_at check (completed_at is null or completed_at >= started_at),

pkgs/core/schemas/0100_function_cascade_complete_taskless_steps.sql

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,23 @@ DECLARE
88
v_iterations int := 0;
99
v_max_iterations int := 50;
1010
BEGIN
11+
-- ==========================================
12+
-- ITERATIVE CASCADE COMPLETION
13+
-- ==========================================
14+
-- Completes taskless steps in waves until none remain
1115
LOOP
12-
-- Safety counter to prevent infinite loops
16+
-- ---------- Safety check ----------
1317
v_iterations := v_iterations + 1;
1418
IF v_iterations > v_max_iterations THEN
1519
RAISE EXCEPTION 'Cascade loop exceeded safety limit of % iterations', v_max_iterations;
1620
END IF;
1721

22+
-- ==========================================
23+
-- COMPLETE READY TASKLESS STEPS
24+
-- ==========================================
1825
WITH completed AS (
19-
-- Complete all ready taskless steps in topological order
26+
-- ---------- Complete taskless steps ----------
27+
-- Steps with initial_tasks=0 and no remaining deps
2028
UPDATE pgflow.step_states ss
2129
SET status = 'completed',
2230
started_at = now(),
@@ -32,19 +40,20 @@ BEGIN
3240
-- Process in topological order to ensure proper cascade
3341
RETURNING ss.*
3442
),
43+
-- ---------- Update dependent steps ----------
44+
-- Propagate completion and empty arrays to dependents
3545
dep_updates AS (
36-
-- Update remaining_deps and initial_tasks for dependents of completed steps
3746
UPDATE pgflow.step_states ss
3847
SET remaining_deps = ss.remaining_deps - dep_count.count,
3948
-- If the dependent is a map step and its dependency completed with 0 tasks,
4049
-- set its initial_tasks to 0 as well
4150
initial_tasks = CASE
4251
WHEN s.step_type = 'map' AND dep_count.has_zero_tasks
43-
THEN 0
44-
ELSE ss.initial_tasks
52+
THEN 0 -- Empty array propagation
53+
ELSE ss.initial_tasks -- Keep existing value (including NULL)
4554
END
4655
FROM (
47-
-- Count how many completed steps are dependencies of each dependent
56+
-- Aggregate dependency updates per dependent step
4857
SELECT
4958
d.flow_slug,
5059
d.step_slug as dependent_slug,
@@ -62,8 +71,8 @@ BEGIN
6271
AND s.flow_slug = ss.flow_slug
6372
AND s.step_slug = ss.step_slug
6473
),
74+
-- ---------- Update run counters ----------
6575
run_updates AS (
66-
-- Update run's remaining_steps count
6776
UPDATE pgflow.runs r
6877
SET remaining_steps = r.remaining_steps - c.completed_count,
6978
status = CASE
@@ -80,9 +89,10 @@ BEGIN
8089
WHERE r.run_id = cascade_complete_taskless_steps.run_id
8190
AND c.completed_count > 0
8291
)
92+
-- ---------- Check iteration results ----------
8393
SELECT COUNT(*) INTO v_iteration_completed FROM completed;
8494

85-
EXIT WHEN v_iteration_completed = 0;
95+
EXIT WHEN v_iteration_completed = 0; -- No more steps to complete
8696
v_total_completed := v_total_completed + v_iteration_completed;
8797
END LOOP;
8898

0 commit comments

Comments
 (0)