feat:add efficiency mcp tool (#3431)

Signed-off-by: sneax <paladesh600@gmail.com>
Co-authored-by: Alex Meijer <ameijer@users.noreply.github.com>

pkg/cmd/costmodel/costmodel.go

@@ -236,6 +236,29 @@ func StartMCPServer(ctx context.Context, accesses *costmodel.Accesses, cloudCost
 		return nil, mcpResp, nil
 	}
 
+	handleEfficiency := func(ctx context.Context, req *mcp_sdk.CallToolRequest, args EfficiencyArgs) (*mcp_sdk.CallToolResult, interface{}, error) {
+		queryRequest := &opencost_mcp.OpenCostQueryRequest{
+			QueryType: opencost_mcp.EfficiencyQueryType,
+			Window:    args.Window,
+			EfficiencyParams: &opencost_mcp.EfficiencyQuery{
+				Aggregate:                  args.Aggregate,
+				Filter:                     args.Filter,
+				EfficiencyBufferMultiplier: args.BufferMultiplier,
+			},
+		}
+
+		mcpReq := &opencost_mcp.MCPRequest{
+			Query: queryRequest,
+		}
+
+		mcpResp, err := mcpServer.ProcessMCPRequest(mcpReq)
+		if err != nil {
+			return nil, nil, fmt.Errorf("failed to process efficiency request: %w", err)
+		}
+
+		return nil, mcpResp, nil
+	}
+
 	// Register tools
 	mcp_sdk.AddTool(sdkServer, &mcp_sdk.Tool{
 		Name:        "get_allocation_costs",
@@ -252,6 +275,11 @@ func StartMCPServer(ctx context.Context, accesses *costmodel.Accesses, cloudCost
 		Description: "Retrieves cloud cost data.",
 	}, handleCloudCosts)
 
+	mcp_sdk.AddTool(sdkServer, &mcp_sdk.Tool{
+		Name:        "get_efficiency",
+		Description: "Retrieves resource efficiency metrics with rightsizing recommendations and cost savings analysis. Computes CPU and memory efficiency (usage/request ratio), provides recommended resource requests, and calculates potential cost savings. Optional buffer_multiplier parameter (default: 1.2 for 20% headroom) can be set to values like 1.4 for 40% headroom.",
+	}, handleEfficiency)
+
 	// Create HTTP handler
 	handler := mcp_sdk.NewStreamableHTTPHandler(func(r *http.Request) *mcp_sdk.Server {
 		return sdkServer
@@ -320,3 +348,10 @@ type CloudCostArgs struct {
 	Region     string `json:"region,omitempty"`
 	Account    string `json:"account,omitempty"`
 }
+
+type EfficiencyArgs struct {
+	Window           string   `json:"window"`                      // Time window (e.g., "today", "yesterday", "7d", "lastweek")
+	Aggregate        string   `json:"aggregate,omitempty"`         // Aggregation level (e.g., "pod", "namespace", "controller")
+	Filter           string   `json:"filter,omitempty"`            // Filter expression (same as allocation filters)
+	BufferMultiplier *float64 `json:"buffer_multiplier,omitempty"` // Buffer multiplier for recommendations (default: 1.2 for 20% headroom, e.g., 1.4 for 40%)
+}

pkg/mcp/server.go

@@ -6,6 +6,7 @@ import (
 	"encoding/hex"
 	"fmt"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/go-playground/validator/v10"
@@ -26,6 +27,14 @@ const (
 	AllocationQueryType QueryType = "allocation"
 	AssetQueryType      QueryType = "asset"
 	CloudCostQueryType  QueryType = "cloudcost"
+	EfficiencyQueryType QueryType = "efficiency"
+)
+
+// Efficiency calculation constants
+const (
+	efficiencyBufferMultiplier = 1.2         // 20% headroom for stability
+	efficiencyMinCPU           = 0.001       // minimum CPU cores
+	efficiencyMinRAM           = 1024 * 1024 // 1 MB minimum RAM
 )
 
 // MCPRequest represents a single turn in a conversation with the OpenCost MCP server.
@@ -49,13 +58,14 @@ type QueryMetadata struct {
 
 // OpenCostQueryRequest provides a unified interface for all OpenCost query types.
 type OpenCostQueryRequest struct {
-	QueryType QueryType `json:"queryType" validate:"required,oneof=allocation asset cloudcost"`
+	QueryType QueryType `json:"queryType" validate:"required,oneof=allocation asset cloudcost efficiency"`
 
 	Window string `json:"window" validate:"required"`
 
 	AllocationParams *AllocationQuery `json:"allocationParams,omitempty"`
 	AssetParams      *AssetQuery      `json:"assetParams,omitempty"`
 	CloudCostParams  *CloudCostQuery  `json:"cloudCostParams,omitempty"`
+	EfficiencyParams *EfficiencyQuery `json:"efficiencyParams,omitempty"`
 }
 
 // AllocationQuery contains the parameters for an allocation query.
@@ -93,6 +103,13 @@ type CloudCostQuery struct {
 	Labels          map[string]string `json:"labels,omitempty"`          // Label filters (key->value)
 }
 
+// EfficiencyQuery contains the parameters for an efficiency query.
+type EfficiencyQuery struct {
+	Aggregate                  string   `json:"aggregate,omitempty"`                  // Aggregation properties (e.g., "pod", "namespace", "controller")
+	Filter                     string   `json:"filter,omitempty"`                     // Filter expression for allocations (same as AllocationQuery)
+	EfficiencyBufferMultiplier *float64 `json:"efficiencyBufferMultiplier,omitempty"` // Buffer multiplier for recommendations (default: 1.2 for 20% headroom)
+}
+
 // AllocationResponse represents the allocation data returned to the AI agent.
 type AllocationResponse struct {
 	// The allocation data, as a map of allocation sets.
@@ -301,6 +318,47 @@ type CostMetric struct {
 	KubernetesPercent float64 `json:"kubernetesPercent"`
 }
 
+// EfficiencyResponse represents the efficiency data returned to the AI agent.
+type EfficiencyResponse struct {
+	Efficiencies []*EfficiencyMetric `json:"efficiencies"`
+}
+
+// EfficiencyMetric represents efficiency data for a single pod/workload.
+type EfficiencyMetric struct {
+	Name string `json:"name"` // Pod/namespace/controller name based on aggregation
+
+	// Current state
+	CPUEfficiency    float64 `json:"cpuEfficiency"`    // Usage / Request ratio (0-1+)
+	MemoryEfficiency float64 `json:"memoryEfficiency"` // Usage / Request ratio (0-1+)
+
+	// Current requests and usage
+	CPUCoresRequested float64 `json:"cpuCoresRequested"`
+	CPUCoresUsed      float64 `json:"cpuCoresUsed"`
+	RAMBytesRequested float64 `json:"ramBytesRequested"`
+	RAMBytesUsed      float64 `json:"ramBytesUsed"`
+
+	// Recommendations (based on actual usage with buffer)
+	RecommendedCPURequest float64 `json:"recommendedCpuRequest"` // Recommended CPU cores
+	RecommendedRAMRequest float64 `json:"recommendedRamRequest"` // Recommended RAM bytes
+
+	// Resulting efficiency after applying recommendations
+	ResultingCPUEfficiency    float64 `json:"resultingCpuEfficiency"`
+	ResultingMemoryEfficiency float64 `json:"resultingMemoryEfficiency"`
+
+	// Cost analysis
+	CurrentTotalCost   float64 `json:"currentTotalCost"`   // Current total cost
+	RecommendedCost    float64 `json:"recommendedCost"`    // Estimated cost with recommendations
+	CostSavings        float64 `json:"costSavings"`        // Potential savings
+	CostSavingsPercent float64 `json:"costSavingsPercent"` // Savings as percentage
+
+	// Buffer multiplier used for recommendations
+	EfficiencyBufferMultiplier float64 `json:"efficiencyBufferMultiplier"` // Buffer multiplier applied (e.g., 1.2 for 20% headroom)
+
+	// Time window
+	Start time.Time `json:"start"`
+	End   time.Time `json:"end"`
+}
+
 // MCPServer holds the dependencies for the MCP API server.
 type MCPServer struct {
 	costModel    *costmodel.CostModel
@@ -338,6 +396,8 @@ func (s *MCPServer) ProcessMCPRequest(request *MCPRequest) (*MCPResponse, error)
 		data, err = s.QueryAssets(request.Query)
 	case CloudCostQueryType:
 		data, err = s.QueryCloudCosts(request.Query)
+	case EfficiencyQueryType:
+		data, err = s.QueryEfficiency(request.Query)
 	default:
 		return nil, fmt.Errorf("unsupported query type: %s", request.Query.QueryType)
 	}
@@ -918,3 +978,211 @@ func transformCloudCostSetRange(ccsr *opencost.CloudCostSetRange) *CloudCostResp
 		Summary:    summary,
 	}
 }
+
+// QueryEfficiency queries allocation data and computes efficiency metrics with recommendations.
+func (s *MCPServer) QueryEfficiency(query *OpenCostQueryRequest) (*EfficiencyResponse, error) {
+	// 1. Parse Window
+	window, err := opencost.ParseWindowWithOffset(query.Window, 0)
+	if err != nil {
+		return nil, fmt.Errorf("failed to parse window '%s': %w", query.Window, err)
+	}
+
+	// 2. Set default parameters
+	var aggregateBy []string
+	var filterString string
+	var bufferMultiplier float64 = efficiencyBufferMultiplier // Default to 1.2 (20% headroom)
+
+	// 3. Parse efficiency parameters if provided
+	if query.EfficiencyParams != nil {
+		// Parse aggregation properties (default to pod if not specified)
+		if query.EfficiencyParams.Aggregate != "" {
+			aggregateBy = strings.Split(query.EfficiencyParams.Aggregate, ",")
+		} else {
+			aggregateBy = []string{"pod"}
+		}
+
+		// Set filter string
+		filterString = query.EfficiencyParams.Filter
+
+		// Validate filter string if provided
+		if filterString != "" {
+			parser := allocation.NewAllocationFilterParser()
+			_, err := parser.Parse(filterString)
+			if err != nil {
+				return nil, fmt.Errorf("invalid allocation filter '%s': %w", filterString, err)
+			}
+		}
+
+		// Set buffer multiplier if provided, otherwise use default
+		if query.EfficiencyParams.EfficiencyBufferMultiplier != nil {
+			bufferMultiplier = *query.EfficiencyParams.EfficiencyBufferMultiplier
+		}
+	} else {
+		// Default to pod-level aggregation
+		aggregateBy = []string{"pod"}
+		filterString = ""
+	}
+
+	// 4. Query allocations with the specified parameters
+	// Use the entire window as step to get aggregated data
+	step := window.Duration()
+	asr, err := s.costModel.QueryAllocation(
+		window,
+		step,
+		aggregateBy,
+		false, // includeIdle
+		false, // idleByNode
+		false, // includeProportionalAssetResourceCosts
+		false, // includeAggregatedMetadata
+		false, // sharedLoadBalancer
+		opencost.AccumulateOptionNone,
+		false, // shareIdle
+		filterString,
+	)
+	if err != nil {
+		return nil, fmt.Errorf("failed to query allocations: %w", err)
+	}
+
+	// 5. Handle empty results
+	if asr == nil || len(asr.Allocations) == 0 {
+		return &EfficiencyResponse{
+			Efficiencies: []*EfficiencyMetric{},
+		}, nil
+	}
+
+	// 6. Compute efficiency metrics from allocations using concurrent processing
+	var (
+		mu           sync.Mutex
+		wg           sync.WaitGroup
+		efficiencies = make([]*EfficiencyMetric, 0)
+	)
+
+	// Process each allocation set (typically one per time window) concurrently
+	for _, allocSet := range asr.Allocations {
+		if allocSet == nil {
+			continue
+		}
+
+		// Process this allocation set in a goroutine
+		wg.Add(1)
+		go func(allocSet *opencost.AllocationSet) {
+			defer wg.Done()
+
+			// Compute metrics for all allocations in this set
+			localMetrics := make([]*EfficiencyMetric, 0, len(allocSet.Allocations))
+			for _, alloc := range allocSet.Allocations {
+				if metric := computeEfficiencyMetric(alloc, bufferMultiplier); metric != nil {
+					localMetrics = append(localMetrics, metric)
+				}
+			}
+
+			// Append results to shared slice (thread-safe)
+			if len(localMetrics) > 0 {
+				mu.Lock()
+				efficiencies = append(efficiencies, localMetrics...)
+				mu.Unlock()
+			}
+		}(allocSet)
+	}
+
+	// Wait for all goroutines to complete
+	wg.Wait()
+
+	return &EfficiencyResponse{
+		Efficiencies: efficiencies,
+	}, nil
+}
+
+// safeDiv performs division and returns 0 if denominator is 0.
+func safeDiv(numerator, denominator float64) float64 {
+	if denominator == 0 {
+		return 0
+	}
+	return numerator / denominator
+}
+
+// computeEfficiencyMetric calculates efficiency metrics for a single allocation.
+func computeEfficiencyMetric(alloc *opencost.Allocation, bufferMultiplier float64) *EfficiencyMetric {
+	if alloc == nil {
+		return nil
+	}
+
+	// Calculate time duration in hours
+	hours := alloc.Minutes() / 60.0
+	if hours <= 0 {
+		return nil
+	}
+
+	// Get current usage (average over the period)
+	cpuCoresUsed := alloc.CPUCoreHours / hours
+	ramBytesUsed := alloc.RAMByteHours / hours
+
+	// Get requested amounts
+	cpuCoresRequested := alloc.CPUCoreRequestAverage
+	ramBytesRequested := alloc.RAMBytesRequestAverage
+
+	// Calculate current efficiency (will be 0 if no requests are set)
+	cpuEfficiency := safeDiv(cpuCoresUsed, cpuCoresRequested)
+	memoryEfficiency := safeDiv(ramBytesUsed, ramBytesRequested)
+
+	// Calculate recommendations with buffer for headroom
+	recommendedCPU := cpuCoresUsed * bufferMultiplier
+	recommendedRAM := ramBytesUsed * bufferMultiplier
+
+	// Ensure recommendations meet minimum thresholds
+	if recommendedCPU < efficiencyMinCPU {
+		recommendedCPU = efficiencyMinCPU
+	}
+	if recommendedRAM < efficiencyMinRAM {
+		recommendedRAM = efficiencyMinRAM
+	}
+
+	// Calculate resulting efficiency after applying recommendations
+	resultingCPUEff := safeDiv(cpuCoresUsed, recommendedCPU)
+	resultingMemEff := safeDiv(ramBytesUsed, recommendedRAM)
+
+	// Calculate cost per unit based on REQUESTED amounts (not used amounts)
+	// This gives us the cost per core-hour or byte-hour that the cluster charges
+	cpuCostPerCoreHour := safeDiv(alloc.CPUCost, cpuCoresRequested*hours)
+	ramCostPerByteHour := safeDiv(alloc.RAMCost, ramBytesRequested*hours)
+
+	// Current total cost
+	currentTotalCost := alloc.TotalCost()
+
+	// Estimate recommended cost based on recommended requests
+	recommendedCPUCost := recommendedCPU * hours * cpuCostPerCoreHour
+	recommendedRAMCost := recommendedRAM * hours * ramCostPerByteHour
+	// Keep other costs the same (PV, network, shared, external, GPU)
+	otherCosts := alloc.PVCost() + alloc.NetworkCost + alloc.SharedCost + alloc.ExternalCost + alloc.GPUCost
+	recommendedTotalCost := recommendedCPUCost + recommendedRAMCost + otherCosts
+
+	// Clamp recommended cost to avoid rounding issues making it higher than current
+	if recommendedTotalCost > currentTotalCost && (recommendedTotalCost-currentTotalCost) < 0.0001 {
+		recommendedTotalCost = currentTotalCost
+	}
+
+	// Calculate savings
+	costSavings := currentTotalCost - recommendedTotalCost
+	costSavingsPercent := safeDiv(costSavings, currentTotalCost) * 100
+
+	return &EfficiencyMetric{
+		Name:                       alloc.Name,
+		CPUEfficiency:              cpuEfficiency,
+		MemoryEfficiency:           memoryEfficiency,
+		CPUCoresRequested:          cpuCoresRequested,
+		CPUCoresUsed:               cpuCoresUsed,
+		RAMBytesRequested:          ramBytesRequested,
+		RAMBytesUsed:               ramBytesUsed,
+		RecommendedCPURequest:      recommendedCPU,
+		RecommendedRAMRequest:      recommendedRAM,
+		ResultingCPUEfficiency:     resultingCPUEff,
+		ResultingMemoryEfficiency:  resultingMemEff,
+		CurrentTotalCost:           currentTotalCost,
+		RecommendedCost:            recommendedTotalCost,
+		CostSavings:                costSavings,
+		CostSavingsPercent:         costSavingsPercent,
+		EfficiencyBufferMultiplier: bufferMultiplier,
+		Start:                      alloc.Start,
+		End:                        alloc.End,
+	}
+}

pkg/mcp/server_test.go

@@ -873,7 +873,6 @@ func TestQueryAllocations_InvalidWindow(t *testing.T) {
 	assert.Contains(t, err.Error(), "failed to parse window")
 }
 
-
 func TestProcessMCPRequest_ResponseMetadata(t *testing.T) {
 	dq := &dummyQuerier{}
 	s := &MCPServer{cloudQuerier: dq}
@@ -910,3 +909,453 @@ func TestCloudCostQuery_NewFields(t *testing.T) {
 	assert.Equal(t, "prod", query.Labels["environment"])
 	assert.Equal(t, "platform", query.Labels["team"])
 }
+
+// ---- Tests for Efficiency Tool ----
+
+func TestEfficiencyQueryStruct(t *testing.T) {
+	bufferMultiplier := 1.4
+	query := EfficiencyQuery{
+		Aggregate:                  "pod",
+		Filter:                     "namespace:production",
+		EfficiencyBufferMultiplier: &bufferMultiplier,
+	}
+
+	assert.Equal(t, "pod", query.Aggregate)
+	assert.Equal(t, "namespace:production", query.Filter)
+	assert.NotNil(t, query.EfficiencyBufferMultiplier)
+	assert.Equal(t, 1.4, *query.EfficiencyBufferMultiplier)
+}
+
+func TestEfficiencyQueryDefaultValues(t *testing.T) {
+	query := EfficiencyQuery{}
+
+	assert.Empty(t, query.Aggregate)
+	assert.Empty(t, query.Filter)
+	assert.Nil(t, query.EfficiencyBufferMultiplier)
+}
+
+func TestEfficiencyMetricStruct(t *testing.T) {
+	now := time.Now()
+	metric := EfficiencyMetric{
+		Name:                       "test-pod",
+		CPUEfficiency:              0.5,
+		MemoryEfficiency:           0.6,
+		CPUCoresRequested:          2.0,
+		CPUCoresUsed:               1.0,
+		RAMBytesRequested:          2147483648, // 2GB
+		RAMBytesUsed:               1288490188, // ~1.2GB
+		RecommendedCPURequest:      1.2,
+		RecommendedRAMRequest:      1546188226, // ~1.44GB
+		ResultingCPUEfficiency:     0.833,
+		ResultingMemoryEfficiency:  0.833,
+		CurrentTotalCost:           10.0,
+		RecommendedCost:            6.0,
+		CostSavings:                4.0,
+		CostSavingsPercent:         40.0,
+		EfficiencyBufferMultiplier: 1.2,
+		Start:                      now.Add(-24 * time.Hour),
+		End:                        now,
+	}
+
+	assert.Equal(t, "test-pod", metric.Name)
+	assert.Equal(t, 0.5, metric.CPUEfficiency)
+	assert.Equal(t, 0.6, metric.MemoryEfficiency)
+	assert.Equal(t, 2.0, metric.CPUCoresRequested)
+	assert.Equal(t, 1.0, metric.CPUCoresUsed)
+	assert.Equal(t, 2147483648.0, metric.RAMBytesRequested)
+	assert.Equal(t, 1288490188.0, metric.RAMBytesUsed)
+	assert.Equal(t, 1.2, metric.RecommendedCPURequest)
+	assert.Equal(t, 1546188226.0, metric.RecommendedRAMRequest)
+	assert.Equal(t, 0.833, metric.ResultingCPUEfficiency)
+	assert.Equal(t, 0.833, metric.ResultingMemoryEfficiency)
+	assert.Equal(t, 10.0, metric.CurrentTotalCost)
+	assert.Equal(t, 6.0, metric.RecommendedCost)
+	assert.Equal(t, 4.0, metric.CostSavings)
+	assert.Equal(t, 40.0, metric.CostSavingsPercent)
+	assert.Equal(t, 1.2, metric.EfficiencyBufferMultiplier)
+	assert.True(t, metric.Start.Before(metric.End))
+}
+
+func TestEfficiencyResponseStruct(t *testing.T) {
+	now := time.Now()
+	metric1 := &EfficiencyMetric{
+		Name:             "pod-1",
+		CPUEfficiency:    0.5,
+		MemoryEfficiency: 0.6,
+		Start:            now.Add(-24 * time.Hour),
+		End:              now,
+	}
+	metric2 := &EfficiencyMetric{
+		Name:             "pod-2",
+		CPUEfficiency:    0.7,
+		MemoryEfficiency: 0.8,
+		Start:            now.Add(-24 * time.Hour),
+		End:              now,
+	}
+
+	response := EfficiencyResponse{
+		Efficiencies: []*EfficiencyMetric{metric1, metric2},
+	}
+
+	require.NotNil(t, response.Efficiencies)
+	assert.Len(t, response.Efficiencies, 2)
+	assert.Equal(t, "pod-1", response.Efficiencies[0].Name)
+	assert.Equal(t, "pod-2", response.Efficiencies[1].Name)
+}
+
+func TestSafeDiv(t *testing.T) {
+	tests := []struct {
+		name        string
+		numerator   float64
+		denominator float64
+		expected    float64
+	}{
+		{"normal division", 10.0, 2.0, 5.0},
+		{"zero denominator", 10.0, 0.0, 0.0},
+		{"zero numerator", 0.0, 2.0, 0.0},
+		{"both zero", 0.0, 0.0, 0.0},
+		{"negative values", -10.0, 2.0, -5.0},
+		{"fractional result", 5.0, 2.0, 2.5},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := safeDiv(tt.numerator, tt.denominator)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}
+
+func TestComputeEfficiencyMetric_NilAllocation(t *testing.T) {
+	result := computeEfficiencyMetric(nil, 1.2)
+	assert.Nil(t, result)
+}
+
+func TestComputeEfficiencyMetric_ZeroMinutes(t *testing.T) {
+	now := time.Now()
+	alloc := &opencost.Allocation{
+		Name:  "test-pod",
+		Start: now,
+		End:   now, // Same time, so 0 minutes
+	}
+
+	result := computeEfficiencyMetric(alloc, 1.2)
+	assert.Nil(t, result)
+}
+
+func TestComputeEfficiencyMetric_ValidAllocation(t *testing.T) {
+	now := time.Now()
+	alloc := &opencost.Allocation{
+		Name:  "test-pod",
+		Start: now.Add(-24 * time.Hour),
+		End:   now,
+		// 24 hours = 1440 minutes
+		CPUCoreHours:           24.0,   // 1 core for 24 hours
+		RAMByteHours:           24.0e9, // ~1GB for 24 hours
+		CPUCoreRequestAverage:  2.0,    // Requested 2 cores
+		RAMBytesRequestAverage: 2.0e9,  // Requested 2GB
+		CPUCost:                10.0,
+		RAMCost:                5.0,
+	}
+
+	result := computeEfficiencyMetric(alloc, 1.2)
+
+	require.NotNil(t, result)
+	assert.Equal(t, "test-pod", result.Name)
+	assert.Equal(t, 2.0, result.CPUCoresRequested)
+	assert.Equal(t, 2.0e9, result.RAMBytesRequested)
+	assert.Equal(t, 1.0, result.CPUCoresUsed)            // 24 core-hours / 24 hours = 1 core
+	assert.Equal(t, 1.0e9, result.RAMBytesUsed)          // 24GB-hours / 24 hours = 1GB
+	assert.Equal(t, 0.5, result.CPUEfficiency)           // 1 / 2 = 0.5
+	assert.Equal(t, 0.5, result.MemoryEfficiency)        // 1GB / 2GB = 0.5
+	assert.Equal(t, 1.2, result.RecommendedCPURequest)   // 1 * 1.2 = 1.2
+	assert.Equal(t, 1.2e9, result.RecommendedRAMRequest) // 1GB * 1.2 = 1.2GB
+	assert.Equal(t, 1.2, result.EfficiencyBufferMultiplier)
+	assert.Greater(t, result.CostSavings, 0.0)
+}
+
+func TestComputeEfficiencyMetric_CustomBufferMultiplier(t *testing.T) {
+	now := time.Now()
+	alloc := &opencost.Allocation{
+		Name:                   "test-pod",
+		Start:                  now.Add(-24 * time.Hour),
+		End:                    now,
+		CPUCoreHours:           24.0,
+		RAMByteHours:           24.0e9,
+		CPUCoreRequestAverage:  2.0,
+		RAMBytesRequestAverage: 2.0e9,
+		CPUCost:                10.0,
+		RAMCost:                5.0,
+	}
+
+	// Test with 1.4 buffer multiplier (40% headroom)
+	result := computeEfficiencyMetric(alloc, 1.4)
+
+	require.NotNil(t, result)
+	assert.Equal(t, 1.4, result.RecommendedCPURequest)   // 1 * 1.4 = 1.4
+	assert.Equal(t, 1.4e9, result.RecommendedRAMRequest) // 1GB * 1.4 = 1.4GB
+	assert.Equal(t, 1.4, result.EfficiencyBufferMultiplier)
+
+	// Resulting efficiency should be usage / recommended
+	expectedCPUEff := 1.0 / 1.4
+	expectedMemEff := 1.0e9 / 1.4e9
+	assert.InDelta(t, expectedCPUEff, result.ResultingCPUEfficiency, 0.001)
+	assert.InDelta(t, expectedMemEff, result.ResultingMemoryEfficiency, 0.001)
+}
+
+func TestComputeEfficiencyMetric_MinimumThresholds(t *testing.T) {
+	now := time.Now()
+	alloc := &opencost.Allocation{
+		Name:  "test-pod",
+		Start: now.Add(-24 * time.Hour),
+		End:   now,
+		// Very small usage
+		CPUCoreHours:           0.00001, // 0.000000417 cores average
+		RAMByteHours:           100,     // ~4 bytes average
+		CPUCoreRequestAverage:  0.1,
+		RAMBytesRequestAverage: 1000,
+		CPUCost:                0.001,
+		RAMCost:                0.001,
+	}
+
+	result := computeEfficiencyMetric(alloc, 1.2)
+
+	require.NotNil(t, result)
+	// Should enforce minimum CPU (0.001 cores)
+	assert.Equal(t, efficiencyMinCPU, result.RecommendedCPURequest)
+	// Should enforce minimum RAM (1MB)
+	assert.Equal(t, float64(efficiencyMinRAM), result.RecommendedRAMRequest)
+}
+
+func TestComputeEfficiencyMetric_NoRequests(t *testing.T) {
+	now := time.Now()
+	alloc := &opencost.Allocation{
+		Name:                   "test-pod",
+		Start:                  now.Add(-24 * time.Hour),
+		End:                    now,
+		CPUCoreHours:           24.0,
+		RAMByteHours:           24.0e9,
+		CPUCoreRequestAverage:  0.0, // No requests set
+		RAMBytesRequestAverage: 0.0, // No requests set
+		CPUCost:                10.0,
+		RAMCost:                5.0,
+	}
+
+	result := computeEfficiencyMetric(alloc, 1.2)
+
+	require.NotNil(t, result)
+	// Efficiency should be 0 when no requests are set
+	assert.Equal(t, 0.0, result.CPUEfficiency)
+	assert.Equal(t, 0.0, result.MemoryEfficiency)
+	// Recommendations should still be calculated based on usage
+	assert.Equal(t, 1.2, result.RecommendedCPURequest)
+	assert.Equal(t, 1.2e9, result.RecommendedRAMRequest)
+}
+
+func TestComputeEfficiencyMetric_OverProvisioned(t *testing.T) {
+	now := time.Now()
+	alloc := &opencost.Allocation{
+		Name:                   "test-pod",
+		Start:                  now.Add(-24 * time.Hour),
+		End:                    now,
+		CPUCoreHours:           12.0,   // 0.5 cores average
+		RAMByteHours:           12.0e9, // 0.5GB average
+		CPUCoreRequestAverage:  4.0,    // Requested 4 cores (over-provisioned)
+		RAMBytesRequestAverage: 8.0e9,  // Requested 8GB (over-provisioned)
+		CPUCost:                40.0,
+		RAMCost:                20.0,
+	}
+
+	result := computeEfficiencyMetric(alloc, 1.2)
+
+	require.NotNil(t, result)
+	// Low efficiency due to over-provisioning
+	assert.Equal(t, 0.125, result.CPUEfficiency)     // 0.5 / 4 = 0.125
+	assert.Equal(t, 0.0625, result.MemoryEfficiency) // 0.5GB / 8GB = 0.0625
+	// Recommendations should be much lower
+	assert.Equal(t, 0.6, result.RecommendedCPURequest)   // 0.5 * 1.2 = 0.6
+	assert.Equal(t, 0.6e9, result.RecommendedRAMRequest) // 0.5GB * 1.2 = 0.6GB
+	// Should have significant cost savings
+	assert.Greater(t, result.CostSavings, 0.0)
+	assert.Greater(t, result.CostSavingsPercent, 50.0)
+}
+
+func TestComputeEfficiencyMetric_UnderProvisioned(t *testing.T) {
+	now := time.Now()
+	alloc := &opencost.Allocation{
+		Name:                   "test-pod",
+		Start:                  now.Add(-24 * time.Hour),
+		End:                    now,
+		CPUCoreHours:           48.0,   // 2 cores average
+		RAMByteHours:           48.0e9, // 2GB average
+		CPUCoreRequestAverage:  1.0,    // Requested 1 core (under-provisioned)
+		RAMBytesRequestAverage: 1.0e9,  // Requested 1GB (under-provisioned)
+		CPUCost:                10.0,
+		RAMCost:                5.0,
+	}
+
+	result := computeEfficiencyMetric(alloc, 1.2)
+
+	require.NotNil(t, result)
+	// High efficiency (>100%) due to under-provisioning
+	assert.Equal(t, 2.0, result.CPUEfficiency)    // 2 / 1 = 2.0
+	assert.Equal(t, 2.0, result.MemoryEfficiency) // 2GB / 1GB = 2.0
+	// Recommendations should be higher than current requests
+	assert.Equal(t, 2.4, result.RecommendedCPURequest)   // 2 * 1.2 = 2.4
+	assert.Equal(t, 2.4e9, result.RecommendedRAMRequest) // 2GB * 1.2 = 2.4GB
+}
+
+func TestComputeEfficiencyMetric_CostCalculations(t *testing.T) {
+	now := time.Now()
+	alloc := &opencost.Allocation{
+		Name:                   "test-pod",
+		Start:                  now.Add(-24 * time.Hour),
+		End:                    now,
+		CPUCoreHours:           24.0,
+		RAMByteHours:           24.0e9,
+		CPUCoreRequestAverage:  2.0,
+		RAMBytesRequestAverage: 2.0e9,
+		CPUCost:                10.0, // $10 for CPU
+		RAMCost:                5.0,  // $5 for RAM
+		NetworkCost:            1.0,  // $1 for network
+		SharedCost:             0.5,  // $0.5 shared
+		ExternalCost:           0.5,  // $0.5 external
+		GPUCost:                1.0,  // $1 for GPU
+	}
+
+	result := computeEfficiencyMetric(alloc, 1.2)
+
+	require.NotNil(t, result)
+
+	// Current total cost should include all costs
+	expectedCurrentCost := 10.0 + 5.0 + 1.0 + 0.5 + 0.5 + 1.0 // = 18.0
+	assert.Equal(t, expectedCurrentCost, result.CurrentTotalCost)
+
+	// Recommended cost should be lower due to right-sizing
+	assert.Less(t, result.RecommendedCost, result.CurrentTotalCost)
+
+	// Cost savings should be positive
+	assert.Greater(t, result.CostSavings, 0.0)
+	assert.Equal(t, result.CurrentTotalCost-result.RecommendedCost, result.CostSavings)
+
+	// Cost savings percent should be calculated correctly
+	expectedPercent := (result.CostSavings / result.CurrentTotalCost) * 100
+	assert.InDelta(t, expectedPercent, result.CostSavingsPercent, 0.001)
+}
+
+func TestComputeEfficiencyMetric_OtherCostsPreserved(t *testing.T) {
+	now := time.Now()
+	alloc := &opencost.Allocation{
+		Name:                   "test-pod",
+		Start:                  now.Add(-24 * time.Hour),
+		End:                    now,
+		CPUCoreHours:           24.0,
+		RAMByteHours:           24.0e9,
+		CPUCoreRequestAverage:  2.0,
+		RAMBytesRequestAverage: 2.0e9,
+		CPUCost:                10.0,
+		RAMCost:                5.0,
+		NetworkCost:            2.0, // Fixed cost
+		SharedCost:             1.0, // Fixed cost
+		ExternalCost:           1.0, // Fixed cost
+		GPUCost:                0.0,
+	}
+
+	result := computeEfficiencyMetric(alloc, 1.2)
+
+	require.NotNil(t, result)
+
+	// The "other costs" (Network, Shared, External, GPU) should be preserved
+	// in the recommended cost calculation
+	otherCosts := 2.0 + 1.0 + 1.0 + 0.0 // = 4.0
+
+	// CPU and RAM costs should be reduced based on right-sizing
+	// Original: 10.0 + 5.0 = 15.0
+	// Usage: 1 core + 1GB
+	// Recommended: 1.2 cores + 1.2GB
+	// Cost is calculated based on REQUESTED amounts (2 cores, 2GB)
+	cpuCostPerCoreHour := 10.0 / (2.0 * 24.0)  // CPU cost / (requested cores * hours)
+	ramCostPerByteHour := 5.0 / (2.0e9 * 24.0) // RAM cost / (requested bytes * hours)
+	expectedRecommendedCPUCost := 1.2 * 24.0 * cpuCostPerCoreHour
+	expectedRecommendedRAMCost := 1.2e9 * 24.0 * ramCostPerByteHour
+	expectedRecommendedTotal := expectedRecommendedCPUCost + expectedRecommendedRAMCost + otherCosts
+
+	assert.InDelta(t, expectedRecommendedTotal, result.RecommendedCost, 0.01)
+}
+
+func TestQueryEfficiency_InvalidWindow(t *testing.T) {
+	s := &MCPServer{}
+
+	req := &OpenCostQueryRequest{
+		QueryType: EfficiencyQueryType,
+		Window:    "invalid-window",
+	}
+
+	_, err := s.QueryEfficiency(req)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "failed to parse window")
+}
+
+func TestQueryEfficiency_DefaultBufferMultiplier(t *testing.T) {
+	// Test that default buffer multiplier is 1.2 when not specified
+	req := &OpenCostQueryRequest{
+		QueryType:        EfficiencyQueryType,
+		Window:           "24h",
+		EfficiencyParams: &EfficiencyQuery{
+			// EfficiencyBufferMultiplier not set - should default to 1.2
+		},
+	}
+
+	assert.Nil(t, req.EfficiencyParams.EfficiencyBufferMultiplier)
+}
+
+func TestQueryEfficiency_CustomBufferMultiplier(t *testing.T) {
+	bufferMultiplier := 1.4
+	req := &OpenCostQueryRequest{
+		QueryType: EfficiencyQueryType,
+		Window:    "24h",
+		EfficiencyParams: &EfficiencyQuery{
+			EfficiencyBufferMultiplier: &bufferMultiplier,
+		},
+	}
+
+	assert.NotNil(t, req.EfficiencyParams.EfficiencyBufferMultiplier)
+	assert.Equal(t, 1.4, *req.EfficiencyParams.EfficiencyBufferMultiplier)
+}
+
+func TestQueryEfficiency_WithFilter(t *testing.T) {
+	req := &OpenCostQueryRequest{
+		QueryType: EfficiencyQueryType,
+		Window:    "7d",
+		EfficiencyParams: &EfficiencyQuery{
+			Aggregate: "pod",
+			Filter:    "namespace:production",
+		},
+	}
+
+	assert.Equal(t, "pod", req.EfficiencyParams.Aggregate)
+	assert.Equal(t, "namespace:production", req.EfficiencyParams.Filter)
+}
+
+func TestQueryEfficiency_WithAggregation(t *testing.T) {
+	req := &OpenCostQueryRequest{
+		QueryType: EfficiencyQueryType,
+		Window:    "7d",
+		EfficiencyParams: &EfficiencyQuery{
+			Aggregate: "namespace,controller",
+		},
+	}
+
+	assert.Equal(t, "namespace,controller", req.EfficiencyParams.Aggregate)
+}
+
+func TestEfficiencyConstants(t *testing.T) {
+	// Test that efficiency constants are defined correctly
+	assert.Equal(t, 1.2, efficiencyBufferMultiplier)
+	assert.Equal(t, 0.001, efficiencyMinCPU)
+	assert.Equal(t, 1024*1024, efficiencyMinRAM)
+}
+
+func TestEfficiencyQueryType(t *testing.T) {
+	assert.Equal(t, QueryType("efficiency"), EfficiencyQueryType)
+}