fix(internal/surfer): identify singletons and preserve command leaf nodes (#5018)

* Identify singleton resources using a variable name heuristic and omit
them from the command group hierarchy (not supported in declarative
framework).
* Ensure that flattened segments (such as locations or zones) are not
skipped over when they contain commands.
* Change root command group name to be snake case

Fixes #4980

Author: sarahheacock

internal/surfer/gcloud/command_group_builder.go

@@ -49,10 +49,8 @@ func newCommandGroupBuilder(model *api.API, service *api.Service, config *provid
 }
 
 func (b *commandGroupBuilder) buildRoot() *CommandGroup {
-	// TODO (https://github.com/googleapis/librarian/issues/4945): Name can be an empty string here.
-	// Need clear error handling rather than silent skipping
 	return &CommandGroup{
-		Name:     b.model.Name,
+		Name:     provider.ResolveRootPackage(b.model),
 		HelpText: fmt.Sprintf("Manage %s resources.", b.title),
 		Groups:   make(map[string]*CommandGroup),
 		Commands: make(map[string]*Command),

internal/surfer/gcloud/command_tree_builder.go

@@ -63,7 +63,16 @@ func (b *commandTreeBuilder) build() (*CommandGroupsByTrack, error) {
 // literal path segments of the method and walks the tree, creating missing
 // groups if they do not yet exist.
 func (b *commandTreeBuilder) insert(root *CommandGroup, groupBuilder *commandGroupBuilder, method *api.Method) error {
-	segments := b.getLiteralSegments(method)
+	if provider.IsSingletonResourceMethod(method, b.model) {
+		return nil
+	}
+
+	binding := provider.PrimaryBinding(method)
+	if binding == nil {
+		return nil
+	}
+
+	segments := provider.GetLiteralSegments(binding.PathTemplate.Segments)
 	if len(segments) == 0 {
 		return nil
 	}
@@ -73,7 +82,8 @@ func (b *commandTreeBuilder) insert(root *CommandGroup, groupBuilder *commandGro
 		if b.isTerminatedSegment(seg) {
 			return nil
 		}
-		if b.isFlattenedSegment(seg) {
+		isLeaf := i == len(segments)-1
+		if b.isFlattenedSegment(seg) && !isLeaf {
 			continue
 		}
 
@@ -124,19 +134,9 @@ var flattenedSegments = map[string]bool{
 }
 
 func (b *commandTreeBuilder) isFlattenedSegment(lit string) bool {
-	// TODO (https://github.com/googleapis/librarian/issues/4980): these should not be flattened if
-	// there are commands that should be generated for them.
 	return flattenedSegments[lit]
 }
 
 func (b *commandTreeBuilder) isTerminatedSegment(lit string) bool {
 	return lit == "operations" && !provider.ShouldGenerateOperations(b.config)
 }
-
-func (b *commandTreeBuilder) getLiteralSegments(method *api.Method) []string {
-	if method.PathInfo == nil || len(method.PathInfo.Bindings) == 0 {
-		return nil
-	}
-	// TODO(https://github.com/googleapis/librarian/issues/3415): handle multiple bindings
-	return provider.GetLiteralSegments(method.PathInfo.Bindings[0].PathTemplate.Segments)
-}

internal/surfer/gcloud/command_tree_builder_test.go

@@ -15,13 +15,14 @@
 package gcloud
 
 import (
+	"fmt"
 	"path"
 	"slices"
-	"strings"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/googleapis/librarian/internal/sidekick/api"
+	"github.com/googleapis/librarian/internal/sidekick/parser/httprule"
 	"github.com/googleapis/librarian/internal/surfer/gcloud/provider"
 )
 
@@ -43,9 +44,9 @@ func TestCommandTreeBuilder_Build_EmptyDefaultHost(t *testing.T) {
 
 func TestCommandTreeBuilder_Build_Structure(t *testing.T) {
 	service := mockService("parallelstore.googleapis.com",
-		mockMethod("CreateInstance", "v1/projects/locations/instances"),
-		mockMethod("ListInstances", "v1/projects/locations/instances"),
-		mockMethod("GetOperation", "v1/projects/locations/operations"),
+		mockMethod("CreateInstance", "v1/{parent=projects/*/locations/*}/instances"),
+		mockMethod("ListInstances", "v1/{parent=projects/*/locations/*}/instances"),
+		mockMethod("GetOperation", "v1/{name=projects/*/locations/*/operations/*}"),
 	)
 	model := &api.API{
 		Name:        "parallelstore",
@@ -81,7 +82,7 @@ func TestCommandTreeBuilder_Build_Structure(t *testing.T) {
 }
 
 func TestCommandTreeBuilder_Build_Operations_Disabled(t *testing.T) {
-	service := mockService("parallelstore.googleapis.com", mockMethod("GetOperation", "v1/projects/locations/operations"))
+	service := mockService("parallelstore.googleapis.com", mockMethod("GetOperation", "v1/{name=projects/*/locations/*/operations/*}"))
 
 	model := &api.API{
 		Name:     "parallelstore",
@@ -101,7 +102,7 @@ func TestCommandTreeBuilder_Build_Operations_Disabled(t *testing.T) {
 }
 
 func TestCommandTreeBuilder_Build_Operations_Enabled(t *testing.T) {
-	service := mockService("parallelstore.googleapis.com", mockMethod("GetOperation", "v1/projects/locations/operations"))
+	service := mockService("parallelstore.googleapis.com", mockMethod("GetOperation", "v1/{name=projects/*/locations/*/operations/*}"))
 
 	model := &api.API{
 		Name:     "parallelstore",
@@ -125,8 +126,8 @@ func TestCommandTreeBuilder_Build_Operations_Enabled(t *testing.T) {
 }
 
 func TestCommandTreeBuilder_Build_MultipleServices(t *testing.T) {
-	serviceOne := mockService("ParallelstoreService", mockMethod("CreateInstance", "v1/projects/locations/instances"))
-	serviceTwo := mockService("OtherParallelstoreService", mockMethod("CreateOtherInstance", "v1/projects/locations/otherInstances"))
+	serviceOne := mockService("ParallelstoreService", mockMethod("CreateInstance", "v1/{parent=projects/*/locations/*}/instances"))
+	serviceTwo := mockService("OtherParallelstoreService", mockMethod("CreateOtherInstance", "v1/{parent=projects/*/locations/*}/otherInstances"))
 
 	model := &api.API{
 		Name:     "parallelstore",
@@ -151,8 +152,8 @@ func TestCommandTreeBuilder_Build_MultipleServices(t *testing.T) {
 }
 
 func TestCommandTreeBuilder_Build_MultipleReleaseTracks(t *testing.T) {
-	serviceOne := mockService("ParallelstoreService", mockMethod("CreateInstance", "v1/projects/locations/instances"))
-	serviceTwo := mockService("ParallelstoreService", mockMethod("CreateInstance", "v1alpha/projects/locations/instances"))
+	serviceOne := mockService("ParallelstoreService", mockMethod("CreateInstance", "v1/{parent=projects/*/locations/*}/instances"))
+	serviceTwo := mockService("ParallelstoreService", mockMethod("CreateInstance", "v1alpha/{parent=projects/*/locations/*}/instances"))
 	serviceTwo.Package = "google.cloud.parallelstore.v1alpha"
 
 	model := &api.API{
@@ -203,23 +204,17 @@ func flattenTree(g *CommandGroup) []string {
 	return paths
 }
 
-func parseLiteralPath(path string) *api.PathTemplate {
-	pt := api.NewPathTemplate()
-	for _, part := range strings.Split(path, "/") {
-		if part != "" {
-			pt = pt.WithLiteral(part)
-		}
-	}
-	return pt
-}
-
 func mockMethod(name, path string) *api.Method {
+	pt, err := httprule.ParseResourcePattern(path)
+	if err != nil {
+		panic(fmt.Sprintf("failed to parse path %q: %v", path, err))
+	}
 	return &api.Method{
 		Name: name,
 		PathInfo: &api.PathInfo{
 			Bindings: []*api.PathBinding{
 				{
-					PathTemplate: parseLiteralPath(path),
+					PathTemplate: pt,
 				},
 			},
 		},

internal/surfer/gcloud/provider/method.go

@@ -124,10 +124,18 @@ func IsStandardMethod(m *api.Method) bool {
 	return IsGet(m) || IsList(m) || IsCreate(m) || IsUpdate(m) || IsDelete(m)
 }
 
+// PrimaryBinding returns the primary path binding for the method, or nil if not available.
+func PrimaryBinding(m *api.Method) *api.PathBinding {
+	if m.PathInfo == nil || len(m.PathInfo.Bindings) == 0 {
+		return nil
+	}
+	return m.PathInfo.Bindings[0]
+}
+
 // GetHTTPVerb returns the HTTP verb from the primary binding, or an empty string if not available.
 func GetHTTPVerb(m *api.Method) string {
-	if m.PathInfo != nil && len(m.PathInfo.Bindings) > 0 {
-		return m.PathInfo.Bindings[0].Verb
+	if b := PrimaryBinding(m); b != nil {
+		return b.Verb
 	}
 	return ""
 }
@@ -193,3 +201,14 @@ func FindResourceMessage(outputType *api.Message) *api.Message {
 	}
 	return nil
 }
+
+// IsSingletonResourceMethod determines whether a resource is a singleton.
+// It checks if the resource associated with the method is a singleton.
+func IsSingletonResourceMethod(method *api.Method, model *api.API) bool {
+	if method == nil {
+		return false
+	}
+
+	resource := GetResourceForMethod(method, model)
+	return isSingletonResource(resource)
+}

internal/surfer/gcloud/provider/method_test.go

@@ -305,3 +305,128 @@ func TestIsStandardMethod(t *testing.T) {
 		})
 	}
 }
+
+func TestIsSingletonResourceMethod(t *testing.T) {
+	model := &api.API{
+		ResourceDefinitions: []*api.Resource{
+			{
+				Type: "test.googleapis.com/Singleton",
+				Patterns: []api.ResourcePattern{
+					[]api.PathSegment{
+						*api.NewPathSegment().WithLiteral("projects"),
+						*api.NewPathSegment().WithVariable(api.NewPathVariable("project")),
+						*api.NewPathSegment().WithLiteral("singletonConfig"),
+					},
+				},
+			},
+			{
+				Type: "test.googleapis.com/AdjacentLiterals",
+				Patterns: []api.ResourcePattern{
+					[]api.PathSegment{
+						*api.NewPathSegment().WithLiteral("projects"),
+						*api.NewPathSegment().WithVariable(api.NewPathVariable("project")),
+						*api.NewPathSegment().WithLiteral("literal1"),
+						*api.NewPathSegment().WithLiteral("literal2"),
+						*api.NewPathSegment().WithVariable(api.NewPathVariable("instance")),
+					},
+				},
+			},
+			{
+				Type: "test.googleapis.com/Standard",
+				Patterns: []api.ResourcePattern{
+					[]api.PathSegment{
+						*api.NewPathSegment().WithLiteral("projects"),
+						*api.NewPathSegment().WithVariable(api.NewPathVariable("project")),
+						*api.NewPathSegment().WithLiteral("instances"),
+						*api.NewPathSegment().WithVariable(api.NewPathVariable("instance")),
+					},
+				},
+			},
+		},
+	}
+
+	for _, test := range []struct {
+		name   string
+		method *api.Method
+		model  *api.API
+		want   bool
+	}{
+		{
+			name:   "Nil Method",
+			method: nil,
+			model:  &api.API{},
+			want:   false,
+		},
+		{
+			name:   "Fallback: No Resource Found, Path Ends in Variable",
+			method: api.NewTestMethod("Test").WithPathTemplate(&api.PathTemplate{Segments: parseResourcePattern("projects/{project}/instances/{instance}")}),
+			model:  &api.API{},
+			want:   false,
+		},
+		{
+			name:   "Fallback: No Resource Found, Path Ends in Literal",
+			method: api.NewTestMethod("Test").WithPathTemplate(&api.PathTemplate{Segments: parseResourcePattern("projects/{project}/singletonConfig")}),
+			model:  &api.API{},
+			want:   false, // Was true when fallback existed.
+		},
+		{
+			name: "Resource Found: Singleton Pattern",
+			method: func() *api.Method {
+				m := api.NewTestMethod("GetSingleton")
+				m.InputType = &api.Message{
+					Fields: []*api.Field{{
+						Name: "name",
+						ResourceReference: &api.ResourceReference{
+							Type: "test.googleapis.com/Singleton",
+						},
+					}},
+				}
+				return m
+			}(),
+			model: model,
+			want:  true,
+		},
+		{
+			name: "Resource Found: Adjacent Literals Pattern",
+			method: func() *api.Method {
+				m := api.NewTestMethod("GetAdjacent")
+				m.InputType = &api.Message{
+					Fields: []*api.Field{{
+						Name: "name",
+						ResourceReference: &api.ResourceReference{
+							Type: "test.googleapis.com/AdjacentLiterals",
+						},
+					}},
+				}
+				return m
+			}(),
+			model: model,
+			want:  true,
+		},
+		{
+			name: "Resource Found: Standard Pattern",
+			method: func() *api.Method {
+				m := api.NewTestMethod("GetStandard")
+				m.InputType = &api.Message{
+					Fields: []*api.Field{{
+						Name: "name",
+						ResourceReference: &api.ResourceReference{
+							Type: "test.googleapis.com/Standard",
+						},
+					}},
+				}
+				return m
+			}(),
+			model: model,
+			want:  false,
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+			got := IsSingletonResourceMethod(test.method, test.model)
+			if diff := cmp.Diff(test.want, got); diff != "" {
+				t.Errorf("mismatch (-want +got):\n%s", diff)
+			}
+		})
+	}
+}

internal/surfer/gcloud/provider/resource.go

@@ -196,6 +196,35 @@ func GetResourceForMethod(method *api.Method, model *api.API) *api.Resource {
 	return nil
 }
 
+// isSingletonResource determines whether a resource is a singleton.
+// It checks if any of the resource's canonical patterns end in a literal segment
+// or contain two adjacent literal segments.
+func isSingletonResource(resource *api.Resource) bool {
+	if resource == nil {
+		return false
+	}
+
+	for _, pattern := range resource.Patterns {
+		if len(pattern) == 0 {
+			continue
+		}
+
+		// A pattern ending in a literal is a singleton (e.g., projects/{project}/singletonConfig).
+		if pattern[len(pattern)-1].Literal != nil {
+			return true
+		}
+
+		// Adjacent literals anywhere in the pattern signify a singleton (e.g., .../literal1/literal2/...).
+		for i := 0; i < len(pattern)-1; i++ {
+			if pattern[i].Literal != nil && pattern[i+1].Literal != nil {
+				return true
+			}
+		}
+	}
+
+	return false
+}
+
 // GetPluralResourceNameForMethod determines the plural name of a resource. It follows a clear
 // hierarchy of truth: first, the explicit `plural` field in the resource
 // definition, and second, inference from the resource pattern.

internal/surfer/gcloud/provider/service_info.go

@@ -50,3 +50,12 @@ func GetServiceTitle(model *api.API, shortServiceName string) string {
 	}
 	return strcase.ToCamel(shortServiceName)
 }
+
+// ResolveRootPackage extracts the service name from the package name (second to last element),
+// falling back to the provided fallback if there are not enough segments.
+func ResolveRootPackage(model *api.API) string {
+	if parts := strings.Split(model.PackageName, "."); len(parts) >= 2 {
+		return parts[len(parts)-2]
+	}
+	return model.Name
+}

internal/surfer/gcloud/provider/service_info_test.go

@@ -93,3 +93,23 @@ func TestGetServiceTitle(t *testing.T) {
 		})
 	}
 }
+
+func TestResolveRootPackage(t *testing.T) {
+	for _, test := range []struct {
+		name  string
+		model *api.API
+		want  string
+	}{
+		{"Standard package", &api.API{PackageName: "google.cloud.parallelstore.v1", Name: "fallback"}, "parallelstore"},
+		{"Synthetic package", &api.API{PackageName: "resource_standard.v1", Name: "fallback"}, "resource_standard"},
+		{"No dots", &api.API{PackageName: "parallelstore", Name: "fallback"}, "fallback"},
+		{"Empty package", &api.API{PackageName: "", Name: "fallback"}, "fallback"},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			got := ResolveRootPackage(test.model)
+			if got != test.want {
+				t.Errorf("ResolveRootPackage(%v) = %q, want %q", test.model, got, test.want)
+			}
+		})
+	}
+}