chore(query): give caveats back canonical keys, simplifying code

Author: barakmich

pkg/query/canonicalize.go

@@ -258,8 +258,6 @@ func isNullOutline(outline Outline) bool {
 
 // assignNodeIDs recursively assigns a unique OutlineNodeID to each node bottom-up,
 // recording each node's serialized CanonicalKey in keys.
-// Filter operations (see isFilterOperation) are left with ID == 0 and are not
-// added to the map; they derive their canonical key on demand via Serialize().
 func assignNodeIDs(outline Outline, keys map[OutlineNodeID]CanonicalKey) Outline {
 	// Recurse on children first (bottom-up)
 	if len(outline.SubOutlines) > 0 {
@@ -270,11 +268,6 @@ func assignNodeIDs(outline Outline, keys map[OutlineNodeID]CanonicalKey) Outline
 		outline.SubOutlines = newSubs
 	}
 
-	// Filter operations don't get their own canonical ID
-	if isFilterOperation(outline) {
-		return outline // ID remains 0
-	}
-
 	id := OutlineNodeID(nodeIDCounter.Add(1))
 	outline.ID = id
 	keys[id] = outline.Serialize()

pkg/query/canonicalize_test.go

@@ -1237,20 +1237,22 @@ func TestCanonicalKey_WithCaveats(t *testing.T) {
 	canonical, err := CanonicalizeOutline(outline)
 	require.NoError(err)
 
-	// Caveat (filter) nodes have ID == 0 and are not in the map
-	require.Equal(OutlineNodeID(0), canonical.Root.ID,
-		"caveat (filter) root should have ID == 0")
-	require.True(canonical.CanonicalKeys[canonical.Root.ID].IsEmpty(),
-		"filter nodes should not be in the CanonicalKeys map")
+	// Caveat nodes now receive their own canonical ID and key
+	require.NotEqual(OutlineNodeID(0), canonical.Root.ID,
+		"caveat root should have a non-zero ID")
+	require.False(canonical.CanonicalKeys[canonical.Root.ID].IsEmpty(),
+		"caveat nodes should be in the CanonicalKeys map")
 
-	// The canonical key for a filter node equals its child's canonical key
+	// The caveat's canonical key is identified by name only, independent of its child
 	child := canonical.Root.SubOutlines[0]
 	require.False(canonical.CanonicalKeys[child.ID].IsEmpty(),
 		"child of caveat should have a canonical key in the map")
-	require.Equal(canonical.Root.Serialize(), canonical.CanonicalKeys[child.ID],
-		"caveat's Serialize() should equal child's canonical key")
-
-	// The canonical key reflects the underlying data, not the caveat wrapper
-	require.NotContains(canonical.Root.Serialize().String(), "cav:",
-		"filter operation canonical key should not mention the caveat")
+	require.NotEqual(canonical.Root.Serialize(), canonical.CanonicalKeys[child.ID],
+		"caveat's Serialize() should differ from child's canonical key")
+
+	// The canonical key includes the caveat name
+	require.Contains(canonical.Root.Serialize().String(), "cav:",
+		"caveat canonical key should mention the caveat name")
+	require.Equal("C(cav:age_check)", canonical.Root.Serialize().String(),
+		"caveat canonical key format")
 }

pkg/query/outline.go

@@ -85,17 +85,10 @@ func (co CanonicalOutline) Compile() (Iterator, error) {
 
 // compileOutline recursively builds an Iterator tree from an Outline,
 // looking up each node's CanonicalKey from the provided map.
-// Filter operations (ID == 0) derive their canonical key on the fly via Serialize().
 func compileOutline(outline Outline, keys map[OutlineNodeID]CanonicalKey) (Iterator, error) {
-	var key CanonicalKey
-	if isFilterOperation(outline) {
-		key = outline.Serialize()
-	} else {
-		var ok bool
-		key, ok = keys[outline.ID]
-		if !ok {
-			return nil, spiceerrors.MustBugf("outline node ID %d not found in CanonicalKeys map - outline must come from a CanonicalOutline", outline.ID)
-		}
+	key, ok := keys[outline.ID]
+	if !ok {
+		return nil, spiceerrors.MustBugf("outline node ID %d not found in CanonicalKeys map - outline must come from a CanonicalOutline", outline.ID)
 	}
 
 	// First, recursively compile all subiterators (bottom-up)
@@ -359,14 +352,6 @@ func Decompile(it Iterator) (Outline, error) {
 	}
 }
 
-// isFilterOperation returns true for Outline nodes that are pure filter
-// operations and therefore do not have their own canonical representation.
-// Filter operations are transparent: their canonical key is their child's key.
-// Currently only CaveatIteratorType qualifies.
-func isFilterOperation(o Outline) bool {
-	return o.Type == CaveatIteratorType
-}
-
 type OutlineMutation func(Outline) Outline
 
 // MutateOutline performs a bottom-up traversal of the outline tree, applying
@@ -544,17 +529,9 @@ func caveatCompare(a, b *core.ContextualizedCaveat) int {
 // Format: <Type>(<Args>)[<Sub1>,<Sub2>,...]
 // Returns a CanonicalKey wrapping the serialized string.
 //
-// Filter operations (e.g. CaveatIteratorType) are transparent: their canonical
-// key is their child's key, since filters do not affect the set identity.
+// CaveatIterator nodes are identified solely by their caveat name, independent
+// of their position in the tree. Their subiterators are not included in the key.
 func (outline Outline) Serialize() CanonicalKey {
-	// Filter operations are transparent — delegate to child
-	if isFilterOperation(outline) {
-		if len(outline.SubOutlines) > 0 {
-			return outline.SubOutlines[0].Serialize()
-		}
-		return CanonicalKey("")
-	}
-
 	var result strings.Builder
 
 	// Add type (single character)
@@ -570,6 +547,12 @@ func (outline Outline) Serialize() CanonicalKey {
 		}
 	}
 
+	// Caveat nodes are identified solely by their name, independent of
+	// their position in the tree. Do not include subiterators in the key.
+	if outline.Type == CaveatIteratorType {
+		return CanonicalKey(result.String())
+	}
+
 	// Add subiterators if present
 	if len(outline.SubOutlines) > 0 {
 		result.WriteByte('[')

pkg/query/outline_test.go

@@ -1377,9 +1377,9 @@ func TestSerializeOutline(t *testing.T) {
 			},
 		}
 
-		// Caveat is a filter operation — Serialize() delegates to its child
+		// Caveat nodes are identified by name only; subiterators are not included
 		key := outline.Serialize()
-		require.Equal("0", key.String())
+		require.Equal("C(cav:age_check)", key.String())
 	})
 
 	t.Run("AliasIteratorType", func(t *testing.T) {