refactor: cleanup in-progress keys and readvising

internal/dispatch/caching/caching.go

@@ -410,26 +410,28 @@
 	return nil
 }
 
-// LookupPlanCheck probes the cache for a Plan-Check answer using a lightweight
-// descriptor. This is the cache-hit fast path that avoids serializing the
-// iterator subtree on the sender side: the executor calls this first, and only
-// builds a full DispatchQueryPlanRequest (which is what forces plan
-// serialization) when this misses.
+// LookupPlanCheck probes the cache for a Plan-Check answer using the
+// serialized iterator bytes the caller has already produced. This is the
+// cache-hit fast path: the executor serializes once at dispatch time, calls
+// LookupPlanCheck with the bytes, and only on miss reuses those same bytes
+// for the full DispatchQueryPlanRequest. Cache hits skip the request/stream
+// machinery (~200 allocs in our memory-engine measurements) but still pay
+// the serialize cost.
 //
-// On a hit we increment both queryPlanTotal and queryPlanFromCache so the cache
-// hit ratio across LookupPlanCheck + DispatchQueryPlan stays sensible; on a
-// miss we *do not* increment queryPlanTotal here — the caller will issue the
-// full DispatchQueryPlan next, which counts the dispatch itself.
+// On a hit we increment both queryPlanTotal and queryPlanFromCache so the
+// cache hit ratio across LookupPlanCheck + DispatchQueryPlan stays sensible;
+// on a miss we do not increment queryPlanTotal here — the caller will issue
+// the full DispatchQueryPlan next, which counts the dispatch itself.
 //
-// We also forward to the delegate on miss so a chain of caching dispatchers
-// (rare today, but allowed by the layering) all get probed.
+// We forward to the delegate on miss so a chain of caching dispatchers (rare
+// today, but allowed by the layering) all get probed.
 func (cd *Dispatcher) LookupPlanCheck(ctx context.Context, lookup dispatch.PlanCheckLookup) (*v1.ResultPath, bool, error) {
 	if lookup.PlanContext == nil {
 		return cd.d.LookupPlanCheck(ctx, lookup)
 	}
 	cacheKey := keys.PlanCheckLookupKey(
 		lookup.PlanContext.Revision,
-		lookup.CanonicalKey,
+		lookup.Plan,
 		lookup.Resource,
 		lookup.Subject,
 		lookup.PlanContext.CaveatContext,

internal/dispatch/caching/cachingdispatch_test.go

@@ -307,10 +307,10 @@ func TestDispatchQueryPlanRecordsCachingMetrics(t *testing.T) {
 		Operation: v1.PlanOperation_PLAN_OPERATION_CHECK,
 		Resource:  tuple.ONRStringToCore("document", "doc1", "viewer"),
 		Subject:   tuple.ONRStringToCore("user", "alice", "..."),
+		Plan:      []byte("document#viewer"),
 		PlanContext: &v1.PlanContext{
-			Revision:       "1",
-			SchemaHash:     []byte(datalayer.NoSchemaHashForTesting),
-			InProgressKeys: []string{"document#viewer"},
+			Revision:   "1",
+			SchemaHash: []byte(datalayer.NoSchemaHashForTesting),
 		},
 	}
 

internal/dispatch/dispatch.go

@@ -84,17 +84,18 @@ type LookupSubjects interface {
 // PlanStream is an alias for the stream to which plan results will be written.
 type PlanStream = Stream[*v1.DispatchQueryPlanResponse]
 
-// PlanCheckLookup is the small, cheap-to-construct descriptor a caller passes
-// to LookupPlanCheck to consult dispatcher caches before paying to serialize a
-// full iterator subtree into a DispatchQueryPlanRequest. The fields mirror the
-// inputs the Plan-Check cache key hashes over (see keys.PlanCheckLookupKey):
-// callers that produce identical descriptors for the same underlying problem
-// will hit the same cache entry as the corresponding DispatchQueryPlan call.
+// PlanCheckLookup is the descriptor a caller passes to LookupPlanCheck to
+// consult dispatcher caches without going through DispatchQueryPlan's full
+// request/stream machinery. The serialized iterator bytes are the dispatch
+// identity: two lookups with the same Plan bytes (and same resource/subject/
+// plan context) target the same cache slot, identical to the slot the
+// corresponding DispatchQueryPlan request would compute (see
+// keys.PlanCheckLookupKey).
 type PlanCheckLookup struct {
-	CanonicalKey string
-	Resource     *core.ObjectAndRelation
-	Subject      *core.ObjectAndRelation
-	PlanContext  *v1.PlanContext
+	Plan        []byte
+	Resource    *core.ObjectAndRelation
+	Subject     *core.ObjectAndRelation
+	PlanContext *v1.PlanContext
 }
 
 // Plan interface describes the methods required to dispatch plan-based query planner requests.

internal/dispatch/dispatch_optimizer.go

@@ -30,21 +30,29 @@
 	})
 }
 
-// wrapAliasWithDispatch is a bottom-up OutlineMutation that wraps each
-// AliasIteratorType node in a DispatchIteratorType node. The wrap is the
-// static portion of the old DispatchExecutor.shouldDispatchAlias check:
-//   - the "is this an alias" test becomes "is this outline node an alias?"
-//   - the sentinel-recursion guard runs over the outline subtree
+// wrapAliasWithDispatch is a bottom-up OutlineMutation that wraps an
+// AliasIteratorType node in a DispatchIteratorType when dispatching the alias
+// is worth the per-dispatch overhead. Three exclusions:
+//   - non-aliases are passed through untouched;
+//   - aliases whose subtree contains an unmatched RecursiveSentinel can't be
+//     dispatched without severing the sentinel's collection context;
+//   - aliases whose body is "leaf-like" (a single Datastore read, or a Union
+//     whose direct children are all Datastores) are too cheap to amortize the
+//     dispatch wrap — see aliasBodyIsLeafLike for the rationale.
 //
-// The remaining runtime guard (in-progress dispatch-key cycle break) cannot
-// be made static and stays in the executor.
+// After Phase 1's executor migration, the wrap is the authoritative dispatch
+// boundary: every wrap → a dispatch, no wrap → local execution. So skipping
+// a wrap here means skipping the dispatch entirely.
 func wrapAliasWithDispatch(outline query.Outline) query.Outline {
 	if outline.Type != query.AliasIteratorType {
 		return outline
 	}
 	if containsUnmatchedRecursiveSentinelOutline(outline) {
 		return outline
 	}
+	if aliasBodyIsLeafLike(outline) {
+		return outline
+	}
 	// New node has ID==0; ApplyOptimizations runs FillMissingNodeIDs after the
 	// transform, which assigns it a fresh ID and records its canonical key.
 	return query.Outline{
@@ -53,12 +61,58 @@
 	}
 }
 
-// containsUnmatchedRecursiveSentinelOutline is the outline-time mirror of
-// containsUnmatchedRecursiveSentinel (executor.go): it walks an outline
-// subtree and reports whether any RecursiveSentinel has no RecursiveIterator
-// with the same (definition, relation) key in the same subtree. An unmatched
-// sentinel means dispatching this subtree would sever the sentinel from the
-// collection context its iterator establishes.
+// aliasBodyIsLeafLike reports whether the alias's single body is too cheap to
+// be worth wrapping for dispatch:
+//   - a bare DatastoreIterator (one relationship read), or
+//   - a Union whose direct children are all DatastoreIterators (a fan-in of
+//     reads to compose multiple direct relations like `view = viewer + editor`).
+//
+// "Direct children" only — we intentionally don't recurse into nested Unions.
+// Anything richer than this (an Arrow, an Intersection, a Recursive, a nested
+// Alias-now-wrapped-as-Dispatch) is heavy enough that dispatch + caching pays
+// for itself, so the wrap stays.
+//
+// Optimizers run bottom-up via MutateOutline, so the body we inspect here is
+// the post-mutation outline. If a descendant alias was itself a candidate for
+// wrapping it has already become DispatchIteratorType — which is not Datastore,
+// so this check correctly classifies the parent as "not leaf-like" and keeps
+// its wrap.
+func aliasBodyIsLeafLike(outline query.Outline) bool {
+	if len(outline.SubOutlines) != 1 {
+		return false
+	}
+	body := outline.SubOutlines[0]
+	switch body.Type {
+	case query.DatastoreIteratorType:
+		return true
+	case query.UnionIteratorType:
+		if len(body.SubOutlines) == 0 {
+			return false
+		}
+		for _, child := range body.SubOutlines {
+			if child.Type != query.DatastoreIteratorType {
+				return false
+			}
+		}
+		return true
+	}
+	return false
+}
+
+// recursiveKey uniquely identifies a recursion pair by definition and relation
+// name. Used by containsUnmatchedRecursiveSentinelOutline to pair up sentinels
+// with their managing RecursiveIterators at optimization time.
+type recursiveKey struct {
+	definitionName string
+	relationName   string
+}
+
+// containsUnmatchedRecursiveSentinelOutline walks an outline subtree and
+// reports whether any RecursiveSentinel has no RecursiveIterator with the
+// same (definition, relation) key in the same subtree. An unmatched sentinel
+// means dispatching this subtree would sever the sentinel from the collection
+// context its iterator establishes — the optimizer uses this to keep such
+// aliases unwrapped (and therefore non-dispatching) at planning time.
 func containsUnmatchedRecursiveSentinelOutline(root query.Outline) bool {
 	var iteratorKeys map[recursiveKey]struct{}
 	var sentinelKeys map[recursiveKey]struct{}

internal/dispatch/dispatch_optimizer_test.go

@@ -50,6 +50,15 @@ func dsOutline() query.Outline {
 	}
 }
 
+// arrowOutline builds an Arrow node — useful for constructing alias bodies
+// that are NOT "leaf-like" so the dispatch-wrap optimizer keeps the wrap.
+func arrowOutline(left, right query.Outline) query.Outline {
+	return query.Outline{
+		Type:        query.ArrowIteratorType,
+		SubOutlines: []query.Outline{left, right},
+	}
+}
+
 // runDispatchWrap canonicalizes outline then runs only the dispatch-wrap
 // optimization on it, returning the resulting root outline.
 func runDispatchWrap(t *testing.T, outline query.Outline) query.Outline {
@@ -62,17 +71,16 @@ func runDispatchWrap(t *testing.T, outline query.Outline) query.Outline {
 }
 
 func TestDispatchWrapOptimizer(t *testing.T) {
-	t.Run("wraps a single alias", func(t *testing.T) {
-		input := aliasOutline("document", "viewer", dsOutline())
+	t.Run("wraps alias whose body is an Arrow", func(t *testing.T) {
+		input := aliasOutline("document", "viewer",
+			arrowOutline(dsOutline(), dsOutline()),
+		)
 		got := runDispatchWrap(t, input)
 
 		require.Equal(t, DispatchIteratorType, got.Type)
 		require.Len(t, got.SubOutlines, 1)
 		require.Equal(t, query.AliasIteratorType, got.SubOutlines[0].Type)
 		require.Equal(t, "viewer", got.SubOutlines[0].Args.RelationName)
-		// child unchanged
-		require.Len(t, got.SubOutlines[0].SubOutlines, 1)
-		require.Equal(t, query.DatastoreIteratorType, got.SubOutlines[0].SubOutlines[0].Type)
 	})
 
 	t.Run("leaves non-alias outlines alone", func(t *testing.T) {
@@ -81,8 +89,47 @@ func TestDispatchWrapOptimizer(t *testing.T) {
 		require.Equal(t, query.DatastoreIteratorType, got.Type)
 	})
 
-	t.Run("wraps nested aliases at every level", func(t *testing.T) {
+	t.Run("skips wrap when alias body is a single Datastore", func(t *testing.T) {
+		// Alias("view") → Datastore — too cheap to amortize a dispatch.
+		input := aliasOutline("document", "view", dsOutline())
+		got := runDispatchWrap(t, input)
+		require.Equal(t, query.AliasIteratorType, got.Type,
+			"leaf-like alias (body = Datastore) must NOT be wrapped")
+		require.Equal(t, "view", got.Args.RelationName)
+	})
+
+	t.Run("skips wrap when alias body is a Union of direct Datastores", func(t *testing.T) {
+		// Alias("view") → Union[DS, DS] — still just data reads; the dispatch
+		// overhead would dwarf the work.
+		input := aliasOutline("document", "view", query.Outline{
+			Type:        query.UnionIteratorType,
+			SubOutlines: []query.Outline{dsOutline(), dsOutline()},
+		})
+		got := runDispatchWrap(t, input)
+		require.Equal(t, query.AliasIteratorType, got.Type,
+			"union-of-datastores body must NOT be wrapped")
+	})
+
+	t.Run("wraps when Union has a non-Datastore direct child", func(t *testing.T) {
+		// Alias("viewer") → Union[DS, Arrow(...)] — the Arrow makes it heavy
+		// enough that the wrap pays back.
+		input := aliasOutline("document", "viewer", query.Outline{
+			Type: query.UnionIteratorType,
+			SubOutlines: []query.Outline{
+				dsOutline(),
+				arrowOutline(dsOutline(), dsOutline()),
+			},
+		})
+		got := runDispatchWrap(t, input)
+		require.Equal(t, DispatchIteratorType, got.Type,
+			"mixed-children union body must be wrapped")
+	})
+
+	t.Run("nested aliases: outer wraps, leaf-like inner stays unwrapped", func(t *testing.T) {
 		// Alias("perm")(Union[ Alias("view")(DS), DS ])
+		// - Inner Alias("view")(DS) is leaf-like → stays unwrapped.
+		// - Outer Union has a non-Datastore direct child (the inner Alias),
+		//   so the outer Alias is NOT leaf-like → wraps.
 		inner := aliasOutline("document", "view", dsOutline())
 		union := query.Outline{
 			Type:        query.UnionIteratorType,
@@ -92,25 +139,24 @@ func TestDispatchWrapOptimizer(t *testing.T) {
 
 		got := runDispatchWrap(t, input)
 
-		// Outer alias is wrapped
-		require.Equal(t, DispatchIteratorType, got.Type)
+		require.Equal(t, DispatchIteratorType, got.Type, "outer alias should be wrapped")
 		outerAlias := got.SubOutlines[0]
 		require.Equal(t, query.AliasIteratorType, outerAlias.Type)
 		require.Equal(t, "perm", outerAlias.Args.RelationName)
 
-		// Inner alias inside the union is also wrapped
 		gotUnion := outerAlias.SubOutlines[0]
 		require.Equal(t, query.UnionIteratorType, gotUnion.Type)
 		require.Len(t, gotUnion.SubOutlines, 2)
-		require.Equal(t, DispatchIteratorType, gotUnion.SubOutlines[0].Type)
-		require.Equal(t, query.AliasIteratorType, gotUnion.SubOutlines[0].SubOutlines[0].Type)
-		require.Equal(t, "view", gotUnion.SubOutlines[0].SubOutlines[0].Args.RelationName)
+		// Inner alias is leaf-like and stays unwrapped.
+		require.Equal(t, query.AliasIteratorType, gotUnion.SubOutlines[0].Type,
+			"inner leaf-like alias must stay unwrapped")
+		require.Equal(t, "view", gotUnion.SubOutlines[0].Args.RelationName)
 	})
 
 	t.Run("wraps alias whose subtree has a matched sentinel/iterator pair", func(t *testing.T) {
 		// Alias("view")(Recursive("folder","viewer")(Sentinel("folder","viewer")))
-		// The recursive iterator inside the alias matches the sentinel's key, so
-		// the dispatch boundary is safe.
+		// The recursive iterator inside the alias matches the sentinel's key,
+		// so the dispatch boundary is safe; body is also not leaf-like.
 		input := aliasOutline("document", "view",
 			recursiveOutline("folder", "viewer", sentinelOutline("folder", "viewer")),
 		)
@@ -122,7 +168,8 @@ func TestDispatchWrapOptimizer(t *testing.T) {
 		// Alias("view")(Union[ Sentinel("folder","viewer"), DS ])
 		// No RecursiveIterator with key folder#viewer exists in the alias's
 		// subtree, so dispatching the alias would sever the sentinel from its
-		// collection context.
+		// collection context. (The sentinel guard runs before the leaf-like
+		// check.)
 		input := aliasOutline("document", "view",
 			query.Outline{
 				Type: query.UnionIteratorType,
@@ -138,7 +185,9 @@ func TestDispatchWrapOptimizer(t *testing.T) {
 	})
 
 	t.Run("assigns IDs and canonical keys to newly-wrapped nodes", func(t *testing.T) {
-		input := aliasOutline("document", "viewer", dsOutline())
+		input := aliasOutline("document", "viewer",
+			arrowOutline(dsOutline(), dsOutline()),
+		)
 		co, err := query.CanonicalizeOutline(input)
 		require.NoError(t, err)
 		res, err := ApplyDispatchWrap(co, queryopt.RequestParams{})
@@ -151,7 +200,9 @@ func TestDispatchWrapOptimizer(t *testing.T) {
 	})
 
 	t.Run("optimized outline compiles to DispatchIterator above AliasIterator", func(t *testing.T) {
-		input := aliasOutline("document", "viewer", dsOutline())
+		input := aliasOutline("document", "viewer",
+			arrowOutline(dsOutline(), dsOutline()),
+		)
 		co, err := query.CanonicalizeOutline(input)
 		require.NoError(t, err)
 		res, err := ApplyDispatchWrap(co, queryopt.RequestParams{})