Skip to content

fix: tracking table setup must occur after all target setups#93

Merged
bashandbone merged 3 commits intomainfrom
copilot/fix-execution-plan-initialization
Mar 13, 2026
Merged

fix: tracking table setup must occur after all target setups#93
bashandbone merged 3 commits intomainfrom
copilot/fix-execution-plan-initialization

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 13, 2026

Mirrors upstream fix cocoindex-io/cocoindex#1715. In apply_changes_for_flow, the tracking table was being set up before targets were fully initialized, allowing the execution plan to observe incomplete target state.

Change

crates/recoco-core/src/setup/driver.rs — reorder setup operations in apply_changes_for_flow:

// Before (tracking table set up before targets):
maybe_update_resource_setup("tracking table", ...).await?;
for (target_kind, resources) in setup_change_by_target_kind { ... }

// After (tracking table set up after all targets):
for (target_kind, resources) in setup_change_by_target_kind { ... }
maybe_update_resource_setup("tracking table", ...).await?;

The commit/metadata logic and everything else in apply_changes_for_flow is unchanged — this is purely an ordering fix.

Original prompt

This section details on the original issue you should resolve

<issue_title>[Upstream-Sync] [BUG] Execution plan initialized before target setup (upstream bug – PR #1715)</issue_title>
<issue_description>### Summary

Adopt the upstream bug fix from cocoindex-io/cocoindex#1715: Ensure that execution plan initialization always occurs after target setup is fully finished.

Motivation

  • A bug in upstream (tracked as PR knitli/recoco#1715) allowed the execution plan to be initialized before target setup was complete in certain cases.
  • This race could, in rare circumstances, cause the planner to use outdated or incomplete state, leading to subtle bugs when resources are quickly provisioned or flows reconfigured.
  • All affected pipelines rely on this setup order for correctness and data integrity.

What changes upstream?

  • In upstream, the relevant Rust files are:
    • rust/cocoindex/src/builder/exec_ctx.rs
    • rust/cocoindex/src/execution/db_tracking_setup.rs
    • rust/cocoindex/src/lib_context.rs
    • rust/cocoindex/src/setup/driver.rs
  • The fix ensures that tracking table setup is only performed (and the plan built) after all targets are successfully created/setup.
  • Lazy execution plan resolution is introduced, and the tracking table's export context is now built only after all target contexts exist.

Example diff (upstream)

-    if let Some(tracking_table) = &flow_setup_change.tracking_table {
-        maybe_update_resource_setup(
-            "tracking table",
-            write,
-            std::iter::once(tracking_table),
-            |setup_change| setup_change[0].setup_change.apply_change(),
-        )
-        .await?;
-    }
    let mut setup_change_by_target_kind = IndexMap::<&str, Vec<_>>::new();
    ...
+    for (target_kind, resources) in setup_change_by_target_kind.into_iter() {
+        maybe_update_resource_setup(
+            target_kind,
+            write,
+            resources.into_iter(),
+            |targets_change| async move {
+                ...
+            },
+        )
+        .await?;
+    }
+    // Now after all target changes:
+    if let Some(tracking_table) = &flow_setup_change.tracking_table {
+        maybe_update_resource_setup(
+            "tracking table",
+            write,
+            std::iter::once(tracking_table),
+            |setup_change| setup_change[0].setup_change.apply_change(),
+        )
+        .await?;
+    }

Action for Recoco

  • Mirror the above fixes in crates/recoco-core/src/setup/driver.rs and related files.
  • Refactor TrackingTableSetupChange and any flow/plan setup logic as needed.

Resources


Template fields below contain bug details and reproduction info.</issue_description>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

…ream bug fix)

Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages bot commented Mar 13, 2026

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
❌ Deployment failed
View logs
recoco-docs 8e1afb3 Mar 13 2026, 09:32 PM

Copilot AI changed the title [WIP] [Upstream-Sync] Fix execution plan initialization order fix: tracking table setup must occur after all target setups Mar 13, 2026
Copilot AI requested a review from bashandbone March 13, 2026 20:32
@bashandbone bashandbone marked this pull request as ready for review March 13, 2026 21:07
Copilot AI review requested due to automatic review settings March 13, 2026 21:07
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR mirrors an upstream fix to ensure flow setup applies target setup changes before tracking table setup, preventing the execution plan from observing partially-initialized target state.

Changes:

  • Reorders apply_changes_for_flow so target resource setup runs first.
  • Moves tracking table setup to run after all target kinds have been processed.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Adam Poulemanos <89049923+bashandbone@users.noreply.github.com>
@bashandbone bashandbone merged commit 61702db into main Mar 13, 2026
26 of 37 checks passed
@bashandbone bashandbone deleted the copilot/fix-execution-plan-initialization branch March 13, 2026 21:32
@github-project-automation github-project-automation bot moved this from Backlog to Done in Recoco v1.0.0 Mar 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[Upstream-Sync] [BUG] Execution plan initialized before target setup (upstream bug – PR #1715)

3 participants