feat(search): add support for saved search terms in grammar

.agent/skills/webstatus-search-grammar/SKILL.md

@@ -26,7 +26,7 @@ For a technical breakdown of the ANTLR grammar, search node transformation, and
    - Add the new term to the `search_criteria` rule in the grammar file (e.g., add `| discouraged_term`).
    - Define the new rule: `discouraged_term: 'is' ':' 'discouraged';`.
 2. **Regenerate Parser**:
-   - Run `make antlr-gen`. This will update the files in `lib/gen/featuresearch/parser/`.
+   - Run `make antlr-gen`. This will update the files in `lib/gen/featuresearch/parser/`. Note: If you do not have the `devcontainer` open or do not have the right Java dependency versions, **ask the user** to run this step for you.
 3. **Update Visitor (`lib/gcpspanner/searchtypes/`)**:
    - Add a new `SearchIdentifier` for your term in `searchtypes.go` (e.g., `IdentifierIsDiscouraged`).
    - In **[FeaturesSearchVisitor.go](../../lib/gcpspanner/searchtypes/features_search_visitor.go)**, implement the `VisitDiscouraged_termContext` method. This visitor is the **source-of-truth** for how grammar terms are translated into Spanner SQL.
@@ -46,3 +46,9 @@ When you add a new search grammar term or modify parsing:
 
 - Trigger the "Updating the Knowledge Base" prompt in `GEMINI.md` to ensure I am aware of the changes.
 - Ensure that `docs/ARCHITECTURE.md` is updated if there are broader system impacts.
+
+## Error Handling & Query Validation
+
+- **Query Execution Errors:** When translating search logic into Spanner SQL or exploring `saved:` references via `expandSavedSearches`, unexpected edge cases (e.g. `backendtypes.ErrSavedSearchNotFound`, or cyclic references) MUST NOT crash the request and MUST NOT return `500 Internal Server Error`.
+- **400 Bad Request Mapping:** Endpoint handlers (e.g., `get_features.go`, `create_saved_search.go`) MUST explicitly catch these semantic validation errors and map them to HTTP `400 Bad Request` JSON responses. The frontend relies on these 400s to render in-app warning banners for invalid subsets of an otherwise valid query string.
+- **Reference Validation:** Always ensure that `ValidateQueryReferences` (or equivalent dry-run query validators) intercepts invalid cycles or missing queries before a database mutation is committed.

antlr/FeatureSearch.g4

@@ -41,6 +41,8 @@ description_term: 'desc' COLON ANY_VALUE;
 group_term: 'group' COLON ANY_VALUE;
 snapshot_term: 'snapshot' COLON ANY_VALUE;
 id_term: 'id' COLON ANY_VALUE;
+saved_term: 'saved' COLON ANY_VALUE;
+hotlist_term: 'hotlist' COLON ANY_VALUE;
 term:
 	available_date_term
 	| available_on_term
@@ -50,6 +52,8 @@ term:
 	| id_term
 	| snapshot_term
 	| description_term
+	| saved_term
+	| hotlist_term
 	| name_term;
 
 date_range_query: startDate = DATE '..' endDate = DATE;

lib/gcpspanner/feature_search.go

@@ -119,10 +119,14 @@ func (c *Client) FeaturesSearch(
 ) (*FeatureResultPage, error) {
 	// Build filterable
 	filterBuilder := NewFeatureSearchFilterBuilder()
-	filter := filterBuilder.Build(searchNode)
+	filter, err := filterBuilder.Build(searchNode)
+	if errors.Is(err, ErrNilSearchNode) {
+		filter = nil
+	} else if err != nil {
+		return nil, errors.Join(ErrInternalQueryFailure, err)
+	}
 
 	var offsetCursor *FeatureResultOffsetCursor
-	var err error
 	if pageToken != nil {
 		offsetCursor, err = decodeInputFeatureResultCursor(*pageToken)
 		if err != nil {

lib/gcpspanner/feature_search_query.go

@@ -15,6 +15,7 @@
 package gcpspanner
 
 import (
+	"errors"
 	"fmt"
 	"maps"
 	"strings"
@@ -24,6 +25,12 @@ import (
 	"github.com/GoogleChrome/webstatus.dev/lib/gcpspanner/searchtypes"
 )
 
+// ErrNilSearchNode is returned when a nil search node is passed to the builder.
+var ErrNilSearchNode = errors.New("search node cannot be nil")
+
+// ErrInvalidRootNode is returned when the root node of the search AST is invalid.
+var ErrInvalidRootNode = errors.New("invalid root node in search AST")
+
 type FeatureSearchFilterBuilder struct {
 	paramCounter int
 	params       map[string]any
@@ -62,37 +69,46 @@ func (b *FeatureSearchFilterBuilder) addParamGetName(param any) string {
 }
 
 // Build constructs a Spanner query for the FeaturesSearch function.
-func (b *FeatureSearchFilterBuilder) Build(node *searchtypes.SearchNode) *FeatureSearchCompiledFilter {
+func (b *FeatureSearchFilterBuilder) Build(node *searchtypes.SearchNode) (*FeatureSearchCompiledFilter, error) {
 	// Ensure it is not nil
-	if node == nil ||
-		// Check for our root node.
-		node.Keyword != searchtypes.KeywordRoot ||
+	if node == nil {
+		return nil, ErrNilSearchNode
+	}
+	// Check for our root node.
+	if node.Keyword != searchtypes.KeywordRoot ||
 		// Currently root should only have at most one child.
 		// lib/gcpspanner/searchtypes/features_search_visitor.go
 		len(node.Children) != 1 {
-		return nil
+		return nil, ErrInvalidRootNode
 	}
 
 	//  Initialize the map and (re)set counter to 0
 	b.params = make(map[string]any)
 	b.paramCounter = 0
 
-	generatedFilters := b.traverseAndGenerateFilters(node.Children[0])
+	generatedFilters, err := b.traverseAndGenerateFilters(node.Children[0])
+	if err != nil {
+		return nil, err
+	}
 
 	return &FeatureSearchCompiledFilter{
 		params:  b.params,
 		filters: generatedFilters,
-	}
+	}, nil
 }
 
-func (b *FeatureSearchFilterBuilder) traverseAndGenerateFilters(node *searchtypes.SearchNode) []string {
+func (b *FeatureSearchFilterBuilder) traverseAndGenerateFilters(node *searchtypes.SearchNode) ([]string, error) {
 	var filters []string
 
 	switch {
 	case node.IsKeyword(): // Handle AND/OR keyword
 		childFilters := make([]string, 0, len(node.Children)) // Collect child filters first
 		for _, child := range node.Children {
-			childFilters = append(childFilters, b.traverseAndGenerateFilters(child)...)
+			res, err := b.traverseAndGenerateFilters(child)
+			if err != nil {
+				return nil, err
+			}
+			childFilters = append(childFilters, res...)
 		}
 
 		// Join child filters using the current node's operator
@@ -113,7 +129,11 @@ func (b *FeatureSearchFilterBuilder) traverseAndGenerateFilters(node *searchtype
 		// Handle parenthesized sub-expressions.
 		childFilters := make([]string, 0, len(node.Children))
 		for _, child := range node.Children {
-			childFilters = append(childFilters, b.traverseAndGenerateFilters(child)...)
+			res, err := b.traverseAndGenerateFilters(child)
+			if err != nil {
+				return nil, err
+			}
+			childFilters = append(childFilters, res...)
 		}
 
 		// Join without spaces because if there are multiple terms, they will
@@ -131,6 +151,9 @@ func (b *FeatureSearchFilterBuilder) traverseAndGenerateFilters(node *searchtype
 		case searchtypes.IdentifierAvailableDate:
 			// Currently not a terminal identifier.
 			break
+		case searchtypes.IdentifierSavedSearch, searchtypes.IdentifierHotlist:
+			// Handled upstream by ValidateQueryReferences and recursive expansion.
+			return nil, fmt.Errorf("unexpected unexpanded search identifier: %s", node.Term.Identifier)
 		case searchtypes.IdentifierAvailableOn:
 			filter = b.availabilityFilter(node.Term.Value, node.Term.Operator)
 		case searchtypes.IdentifierName:
@@ -155,7 +178,7 @@ func (b *FeatureSearchFilterBuilder) traverseAndGenerateFilters(node *searchtype
 		}
 	}
 
-	return filters
+	return filters, nil
 }
 
 func searchOperatorToSpannerBinaryOperator(in searchtypes.SearchOperator) string {

lib/gcpspanner/feature_search_query_test.go

@@ -15,6 +15,7 @@
 package gcpspanner
 
 import (
+	"errors"
 	"reflect"
 	"slices"
 	"testing"
@@ -49,6 +50,25 @@ var (
 		},
 	}
 
+	unexpandedSavedSearchQuery = TestTree{
+		Query: "saved:XYZ",
+		InputTree: &searchtypes.SearchNode{
+			Keyword: searchtypes.KeywordRoot,
+			Term:    nil,
+			Children: []*searchtypes.SearchNode{
+				{
+					Term: &searchtypes.SearchTerm{
+						Identifier: searchtypes.IdentifierSavedSearch,
+						Operator:   searchtypes.OperatorNone,
+						Value:      "XYZ",
+					},
+					Children: nil,
+					Keyword:  searchtypes.KeywordNone,
+				},
+			},
+		},
+	}
+
 	simpleNameQuery = TestTree{
 		Query: `name:"CSS Grid"`,
 		InputTree: &searchtypes.SearchNode{
@@ -537,7 +557,10 @@ WHERE BrowserName = @param0) AND (fbs.Status = @param1 OR fbs.Status = @param2))
 	for _, tc := range testCases {
 		t.Run(tc.inputTestTree.Query, func(t *testing.T) {
 			b := NewFeatureSearchFilterBuilder()
-			filter := b.Build(tc.inputTestTree.InputTree)
+			filter, err := b.Build(tc.inputTestTree.InputTree)
+			if err != nil {
+				t.Fatalf("Build failed: %v", err)
+			}
 			if !slices.Equal[[]string](filter.Filters(), tc.expectedClauses) {
 				t.Errorf("\nexpected clause [%s]\n  actual clause [%s]", tc.expectedClauses, filter.Filters())
 			}
@@ -547,3 +570,32 @@ WHERE BrowserName = @param0) AND (fbs.Status = @param1 OR fbs.Status = @param2))
 		})
 	}
 }
+
+func TestBuild_Error(t *testing.T) {
+	b := NewFeatureSearchFilterBuilder()
+	_, err := b.Build(unexpandedSavedSearchQuery.InputTree)
+	if err == nil {
+		t.Fatal("expected error for unexpanded saved search, got nil")
+	}
+	expectedErr := "unexpected unexpanded search identifier: saved"
+	if err.Error() != expectedErr {
+		t.Errorf("expected error %q, got %q", expectedErr, err.Error())
+	}
+
+	// Test nil node
+	_, err = b.Build(nil)
+	if !errors.Is(err, ErrNilSearchNode) {
+		t.Errorf("expected error %v, got %v", ErrNilSearchNode, err)
+	}
+
+	// Test invalid root node
+	invalidRootNode := &searchtypes.SearchNode{
+		Keyword:  searchtypes.KeywordNone,
+		Term:     nil,
+		Children: nil,
+	}
+	_, err = b.Build(invalidRootNode)
+	if !errors.Is(err, ErrInvalidRootNode) {
+		t.Errorf("expected error %v, got %v", ErrInvalidRootNode, err)
+	}
+}

lib/gcpspanner/searchtypes/features_search_parse_test.go

@@ -1592,6 +1592,42 @@ func TestParseQuery(t *testing.T) {
 				},
 			},
 		},
+		{
+			InputQuery: "saved:my-saved-query",
+			ExpectedTree: &SearchNode{
+				Keyword: KeywordRoot,
+				Term:    nil,
+				Children: []*SearchNode{
+					{
+						Children: nil,
+						Term: &SearchTerm{
+							Identifier: IdentifierSavedSearch,
+							Value:      "my-saved-query",
+							Operator:   OperatorEq,
+						},
+						Keyword: KeywordNone,
+					},
+				},
+			},
+		},
+		{
+			InputQuery: "hotlist:my-hotlist",
+			ExpectedTree: &SearchNode{
+				Keyword: KeywordRoot,
+				Term:    nil,
+				Children: []*SearchNode{
+					{
+						Children: nil,
+						Term: &SearchTerm{
+							Identifier: IdentifierHotlist,
+							Value:      "my-hotlist",
+							Operator:   OperatorEq,
+						},
+						Keyword: KeywordNone,
+					},
+				},
+			},
+		},
 	}
 
 	for _, testCase := range testCases {

lib/gcpspanner/searchtypes/features_search_visitor.go

@@ -148,6 +148,14 @@ func (v *FeaturesSearchVisitor) createIDNode(idNode antlr.TerminalNode) *SearchN
 	return v.createSimpleNode(idNode, IdentifierID, OperatorEq)
 }
 
+func (v *FeaturesSearchVisitor) createSavedNode(savedNode antlr.TerminalNode) *SearchNode {
+	return v.createSimpleNode(savedNode, IdentifierSavedSearch, OperatorEq)
+}
+
+func (v *FeaturesSearchVisitor) createHotlistNode(hotlistNode antlr.TerminalNode) *SearchNode {
+	return v.createSimpleNode(hotlistNode, IdentifierHotlist, OperatorEq)
+}
+
 func (v *FeaturesSearchVisitor) createSnapshotNode(snapshotNode antlr.TerminalNode) *SearchNode {
 	return v.createSimpleNode(snapshotNode, IdentifierSnapshot, OperatorEq)
 }
@@ -502,6 +510,16 @@ func (v *FeaturesSearchVisitor) VisitId_term(ctx *parser.Id_termContext) any {
 	return v.createIDNode(ctx.ANY_VALUE())
 }
 
+// nolint: revive // Method signature is generated.
+func (v *FeaturesSearchVisitor) VisitSaved_term(ctx *parser.Saved_termContext) any {
+	return v.createSavedNode(ctx.ANY_VALUE())
+}
+
+// nolint: revive // Method signature is generated.
+func (v *FeaturesSearchVisitor) VisitHotlist_term(ctx *parser.Hotlist_termContext) any {
+	return v.createHotlistNode(ctx.ANY_VALUE())
+}
+
 // nolint: revive // Method signature is generated.
 func (v *FeaturesSearchVisitor) VisitSnapshot_term(ctx *parser.Snapshot_termContext) any {
 	return v.createSnapshotNode(ctx.ANY_VALUE())

lib/gcpspanner/searchtypes/searchtypes.go

@@ -103,4 +103,6 @@ const (
 	IdentifierGroup                SearchIdentifier = "group"
 	IdentifierSnapshot             SearchIdentifier = "snapshot"
 	IdentifierID                   SearchIdentifier = "id"
+	IdentifierSavedSearch          SearchIdentifier = "saved"
+	IdentifierHotlist              SearchIdentifier = "hotlist"
 )