From de69f42e4f1b5d5f9f8d9fc373d0f6b2807069aa Mon Sep 17 00:00:00 2001
From: Micha Reiser <micha@reiser.io>
Date: Tue, 6 May 2025 19:41:37 +0200
Subject: [PATCH] fix: incorrect caching for queries participating in fixpoint

---
 src/function/backdate.rs            |  9 +++++++++
 src/function/maybe_changed_after.rs | 29 +++++++++++++----------------
 2 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/src/function/backdate.rs b/src/function/backdate.rs
index fc8d2005a..709111908 100644
--- a/src/function/backdate.rs
+++ b/src/function/backdate.rs
@@ -17,6 +17,15 @@ where
         revisions: &mut QueryRevisions,
         value: &C::Output<'db>,
     ) {
+        // We've seen issues where queries weren't re-validated when backdating provisional values
+        // in ty. This is more of a bandaid because we're close to a release and don't have the time to prove
+        // right now whether backdating could be made safe for queries participating in queries.
+        // TODO: Write a test that demonstrates that backdating queries participating in a cycle isn't safe
+        // OR write many tests showing that it is (and fixing the case where it didn't correctly account for today).
+        if !revisions.cycle_heads.is_empty() {
+            return;
+        }
+
         if let Some(old_value) = &old_memo.value {
             // Careful: if the value became less durable than it
             // used to be, that is a "breaking change" that our
diff --git a/src/function/maybe_changed_after.rs b/src/function/maybe_changed_after.rs
index 1252ca17d..7c1a56258 100644
--- a/src/function/maybe_changed_after.rs
+++ b/src/function/maybe_changed_after.rs
@@ -412,20 +412,14 @@ where
                 // in rev 1 but not in rev 2.
                 VerifyResult::changed()
             }
-            QueryOrigin::FixpointInitial => {
-                let is_provisional = old_memo.may_be_provisional();
-
-                // If the value is from the same revision but is still provisional, consider it changed
-                // because we're now in a new iteration.
-                if shallow_update_possible && is_provisional {
-                    return VerifyResult::Changed(CycleHeads::initial(database_key_index));
-                }
-
-                VerifyResult::Unchanged(
-                    InputAccumulatedValues::Empty,
-                    CycleHeads::initial(database_key_index),
-                )
-            }
+            // Return `Unchanged` similar to the initial value that we insert
+            // when we hit the cycle. Any dependencies accessed when creating the fixpoint initial
+            // are tracked by the outer query. Nothing should have changed assuming that the
+            // fixpoint initial function is deterministic.
+            QueryOrigin::FixpointInitial => VerifyResult::Unchanged(
+                InputAccumulatedValues::Empty,
+                CycleHeads::initial(database_key_index),
+            ),
             QueryOrigin::DerivedUntracked(_) => {
                 // Untracked inputs? Have to assume that it changed.
                 VerifyResult::changed()
@@ -458,8 +452,11 @@ where
                                     last_verified_at,
                                     !cycle_heads.is_empty(),
                                 ) {
-                                    VerifyResult::Changed(_) => {
-                                        break 'cycle VerifyResult::Changed(cycle_heads)
+                                    VerifyResult::Changed(heads) => {
+                                        // Carry over the heads from the inner query to avoid
+                                        // backdating the outer query.
+                                        cycle_heads.extend(&heads);
+                                        break 'cycle VerifyResult::Changed(cycle_heads);
                                     }
                                     VerifyResult::Unchanged(input_accumulated, cycles) => {
                                         cycle_heads.extend(&cycles);