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

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,12 @@ 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)
+	binding := provider.PrimaryBinding(method)
+	if binding == nil || provider.IsSingletonFromSegments(binding.PathTemplate.Segments) {
+		return nil
+	}
+
+	segments := provider.GetLiteralSegments(binding.PathTemplate.Segments)
 	if len(segments) == 0 {
 		return nil
 	}
@@ -73,7 +78,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 +130,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 ""
 }

internal/surfer/gcloud/provider/resource.go

@@ -323,3 +323,42 @@ func GetSingularResourceNameForPrefix(model *api.API, prefix []string) string {
 	}
 	return ""
 }
+
+// IsSingletonFromSegments determines whether a resource is a singleton from its path segments.
+// It returns true if there are two literal path segments back to back, or two elements
+// within a variable's segments list back to back that are not "*".
+func IsSingletonFromSegments(segments []api.PathSegment) bool {
+	for i := 0; i < len(segments)-1; i++ {
+		if segments[i].Literal != nil && segments[i+1].Literal != nil {
+			return true
+		}
+	}
+
+	for _, seg := range segments {
+		if seg.Variable == nil {
+			continue
+		}
+		for i := 0; i < len(seg.Variable.Segments)-1; i++ {
+			curr := seg.Variable.Segments[i]
+			next := seg.Variable.Segments[i+1]
+			if curr != "*" && next != "*" {
+				return true
+			}
+		}
+	}
+
+	for _, seg := range segments {
+		// Variable "name" indicates this is a resource-based method (e.g. Get, Update, Delete)
+		// rather than a collection-based method (e.g. List, Create, which use "parent").
+		if seg.Variable != nil && len(seg.Variable.FieldPath) > 0 && seg.Variable.FieldPath[0] == "name" {
+			if len(seg.Variable.Segments) > 0 {
+				lastInternal := seg.Variable.Segments[len(seg.Variable.Segments)-1]
+				if lastInternal != "*" && lastInternal != "**" && !strings.HasPrefix(lastInternal, ":") {
+					return true
+				}
+			}
+		}
+	}
+
+	return false
+}

internal/surfer/gcloud/provider/resource_test.go

@@ -707,3 +707,98 @@ func TestGetSingularResourceNameForPrefix(t *testing.T) {
 		})
 	}
 }
+
+func TestIsSingletonFromSegments(t *testing.T) {
+	for _, test := range []struct {
+		name     string
+		segments []api.PathSegment
+		want     bool
+	}{
+		{
+			name:     "Empty",
+			segments: nil,
+			want:     false,
+		},
+		{
+			name:     "Standard Pattern (Not Singleton)",
+			segments: parseResourcePattern("projects/{project}/locations/{location}/instances/{instance}"),
+			want:     false,
+		},
+		{
+			name: "Adjacent Literals (Singleton)",
+			segments: []api.PathSegment{
+				*api.NewPathSegment().WithLiteral("projects"),
+				*api.NewPathSegment().WithLiteral("locations"),
+			},
+			want: true,
+		},
+		{
+			name: "Adjacent Non-Wildcards in Variable (Singleton)",
+			segments: []api.PathSegment{
+				*api.NewPathSegment().WithVariable(api.NewPathVariable("name").WithLiteral("projects").WithLiteral("locations")),
+			},
+			want: true,
+		},
+		{
+			name: "Variable with Wildcard (Not Singleton)",
+			segments: []api.PathSegment{
+				*api.NewPathSegment().WithVariable(api.NewPathVariable("name").WithLiteral("projects").WithMatch()),
+			},
+			want: false,
+		},
+		{
+			name: "Variable with Adjacent Wildcards (Not Singleton)",
+			segments: []api.PathSegment{
+				*api.NewPathSegment().WithVariable(api.NewPathVariable("name").WithMatch().WithMatch()),
+			},
+			want: false,
+		},
+		{
+			name: "Heuristic: Variable 'name' ends in literal (Singleton)",
+			segments: []api.PathSegment{
+				{
+					Variable: &api.PathVariable{
+						FieldPath: []string{"name"},
+						Segments:  []string{"projects", "*", "locations", "*", "singletonResource"},
+					},
+				},
+			},
+			want: true,
+		},
+		{
+			name: "Heuristic: Variable 'name' ends in wildcard (Not Singleton)",
+			segments: []api.PathSegment{
+				{
+					Variable: &api.PathVariable{
+						FieldPath: []string{"name"},
+						Segments:  []string{"projects", "*", "locations", "*", "resources", "*"},
+					},
+				},
+			},
+			want: false,
+		},
+		{
+			name: "Heuristic: Variable 'parent' ends in literal (Not Singleton by name variable rule)",
+			segments: []api.PathSegment{
+				{
+					Variable: &api.PathVariable{
+						FieldPath: []string{"parent"},
+						Segments:  []string{"projects", "*"},
+					},
+				},
+				{
+					Literal: func() *string { s := "zones"; return &s }(),
+				},
+			},
+			want: false,
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+			got := IsSingletonFromSegments(test.segments)
+			if diff := cmp.Diff(test.want, got); diff != "" {
+				t.Errorf("mismatch (-want +got):\n%s", diff)
+			}
+		})
+	}
+}

internal/surfer/gcloud/provider/service_info.go

@@ -50,3 +50,12 @@
 	}
 	return strcase.ToCamel(shortServiceName)
 }
+
+// ResolveRootServiceName 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)
+			}
+		})
+	}
+}