fix(recipe): attach exclusion context to strict-gap errors; pin rendering

Signed-off-by: Brian Lockwood <lockwobr@gmail.com>

Author: lockwobr

docs/contributor/recipe.md

@@ -480,7 +480,7 @@ field before upgrading.
 sufficient. It is satisfied when `service` and `accelerator` are each honored
 by *some* overlay independently, even when no single overlay carries the
 combination and the combination's content lives only on an OS-gated leaf. The
-caller then receives a recipe that silently never applied it.
+caller then receives a recipe that silently omits that OS-gated content.
 `verifyCriteriaCoverage` therefore also enforces a second condition
 (issue #1782): resolution fails when **no applied overlay jointly carries
 every stated dimension** *and* stating a strict dimension would reach an

pkg/recipe/coverage.go

@@ -65,12 +65,20 @@ var coverageDimensions = []coverageDimension{
 	// hand back. Resolving one anyway would emit a recipe whose driver story
 	// is wrong rather than merely generic.
 	//
-	// That is a property of installing NVIDIA drivers on Linux, not of this
-	// catalog's shape, so it holds for external --data catalogs too. The
-	// assumption that would break it is a cluster whose node pools run
-	// different operating systems; AICR models that as separate recipes
-	// today (see ADR-018, which partitions a resolved recipe by component
-	// class and explicitly does not change resolution).
+	// The driver argument is a property of installing NVIDIA drivers on Linux
+	// rather than of this catalog's shape, so it carries to external --data
+	// catalogs. Note what that does and does not claim: it says os is the
+	// right dimension to demand, NOT that every catalog shape is served well
+	// by demanding it. A split-coverage external catalog (service overlay,
+	// os-agnostic accelerator overlay, one os-gated tuned leaf) is rejected
+	// here asking for an os, because no single overlay carries the stated
+	// combination. That is deliberate and the escape hatch is explicit:
+	// declare an os-agnostic overlay carrying the combination, which is the
+	// same assertion eks.yaml makes.
+	//
+	// The assumption that would break the driver argument itself is a cluster
+	// whose node pools run different operating systems. AICR has no model for
+	// that; it is expressed as separate recipes.
 	{name: string(FieldOS), value: func(c *Criteria) string { return string(c.OS) }, strict: true},
 	{name: string(FieldPlatform), value: func(c *Criteria) string { return string(c.Platform) }},
 }
@@ -258,7 +266,7 @@ func (s *MetadataStore) verifyCriteriaCoverage(criteria *Criteria, appliedOverla
 		// overlay covers the combination. Joint sufficiency catches that, and
 		// absorbs the retired requireOSIfNeeded guard (issue #1782).
 		if gaps := s.strictDimensionGaps(criteria, appliedOverlays); len(gaps) > 0 {
-			return strictGapError(criteria, gaps)
+			return strictGapError(criteria, gaps, excluded, warnings)
 		}
 		return nil
 	}
@@ -562,7 +570,16 @@ func (s *MetadataStore) inheritanceChainNames(overlay *RecipeMetadata) []string
 // context uses its own key rather than `uncovered`: pkg/client/v1 relaxation
 // CLEARS uncovered dimensions and retries, which here would discard the check
 // and return the partial recipe that issue #1542 fixed.
-func strictGapError(criteria *Criteria, gaps []strictGap) error {
+//
+// excluded/warnings are attached exactly as the completeness path attaches
+// them. reachesUnappliedOverlay probes through the UNFILTERED overlay set, so
+// on the evaluator path an overlay that would cover the combination but was
+// removed by a failing constraint still counts as reachable. Without this
+// context the caller is told to state an os, supplies it, and only then meets
+// the real constraint failure. The demand itself is still correct — the
+// combination genuinely is not covered — so this is a diagnosis aid, not a
+// gate: the error stands either way.
+func strictGapError(criteria *Criteria, gaps []strictGap, excluded []ExcludedOverlay, warnings []ConstraintWarning) error {
 	clauses := make([]string, 0, len(gaps))
 	entries := make([]map[string]any, 0, len(gaps))
 	for _, gap := range gaps {
@@ -573,8 +590,15 @@ func strictGapError(criteria *Criteria, gaps []strictGap) error {
 			"validValues": gap.validValues,
 		})
 	}
+	ctx := map[string]any{"strictDimensions": entries}
+	if len(excluded) > 0 {
+		ctx["excludedOverlays"] = excluded
+	}
+	if len(warnings) > 0 {
+		ctx["constraintWarnings"] = warnings
+	}
 	return aicrerrors.NewWithContext(aicrerrors.ErrCodeInvalidRequest,
 		fmt.Sprintf("%s has no recipe covering that combination; specify %s",
 			criteria.String(), strings.Join(clauses, ", ")),
-		map[string]any{"strictDimensions": entries})
+		ctx)
 }

pkg/recipe/coverage_subsumption_test.go

@@ -16,9 +16,13 @@ package recipe
 
 import (
 	"context"
+	stderrors "errors"
 	"fmt"
 	"sort"
+	"strings"
 	"testing"
+
+	aicrerrors "github.com/NVIDIA/aicr/pkg/errors"
 )
 
 // retiredOSGuard is the deleted requireOSIfNeeded, preserved verbatim as a
@@ -161,3 +165,88 @@ func TestJointSufficiencySubsumesRetiredGuard(t *testing.T) {
 	}
 	t.Logf("subsumption held: retired guard fired on %d (catalog, query) pairs, all still rejected", guardFired)
 }
+
+// TestStrictGapErrorRendering pins the strict-gap message and context shape
+// directly, independent of catalog shape.
+//
+// Both are otherwise under-exercised. The embedded catalog produces zero
+// joint-sufficiency failures — completeness always fires first — so
+// testdata/coverage_golden.yaml has no strictDimensions entries and the
+// golden helper's extraction branch never runs against real data. And the
+// multi-value join executes only when one dimension has several reaching
+// values, which the single-strict-dimension catalog cannot produce.
+func TestStrictGapErrorRendering(t *testing.T) {
+	criteria := &Criteria{Service: CriteriaServiceEKS, Accelerator: CriteriaAcceleratorH100}
+
+	tests := []struct {
+		name         string
+		gaps         []strictGap
+		excluded     []ExcludedOverlay
+		wantContains []string
+		wantExcluded bool
+	}{
+		{
+			name:         "single value",
+			gaps:         []strictGap{{dimension: string(FieldOS), validValues: []string{"ubuntu"}}},
+			wantContains: []string{"specify os (valid: ubuntu)"},
+		},
+		{
+			name:         "multiple values join with a comma",
+			gaps:         []strictGap{{dimension: string(FieldOS), validValues: []string{"cos", "ubuntu"}}},
+			wantContains: []string{"specify os (valid: cos, ubuntu)"},
+		},
+		{
+			name: "multiple gaps join with a comma",
+			gaps: []strictGap{
+				{dimension: string(FieldOS), validValues: []string{"ubuntu"}},
+				{dimension: string(FieldIntent), validValues: []string{"training"}},
+			},
+			wantContains: []string{"os (valid: ubuntu)", "intent (valid: training)"},
+		},
+		{
+			// On the evaluator path a covering overlay may have been removed by
+			// a failing constraint. The demand still stands, but the caller
+			// needs the exclusion context or they will state the os and only
+			// then meet the real failure.
+			name:         "constraint exclusions are attached",
+			gaps:         []strictGap{{dimension: string(FieldOS), validValues: []string{"ubuntu"}}},
+			excluded:     []ExcludedOverlay{{Name: "h100-eks-ubuntu-training"}},
+			wantContains: []string{"specify os (valid: ubuntu)"},
+			wantExcluded: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := strictGapError(criteria, tt.gaps, tt.excluded, nil)
+			for _, want := range tt.wantContains {
+				if !strings.Contains(err.Error(), want) {
+					t.Errorf("message = %q, want it to contain %q", err.Error(), want)
+				}
+			}
+
+			var se *aicrerrors.StructuredError
+			if !stderrors.As(err, &se) {
+				t.Fatalf("expected StructuredError, got %v", err)
+			}
+			if se.Context["uncovered"] != nil {
+				t.Error("strict-gap failure must not populate `uncovered`; relaxation would clear it")
+			}
+			if (se.Context["excludedOverlays"] != nil) != tt.wantExcluded {
+				t.Errorf("excludedOverlays present = %v, want %v",
+					se.Context["excludedOverlays"] != nil, tt.wantExcluded)
+			}
+
+			// The golden matrix classifies via this extraction, so exercise it
+			// here rather than relying on catalog shape to reach it.
+			got := strictDimensionsFromError(err)
+			want := make([]string, 0, len(tt.gaps))
+			for _, g := range tt.gaps {
+				want = append(want, g.dimension)
+			}
+			if !equalStrings(got, want) {
+				t.Errorf("strictDimensionsFromError = %v, want %v", got, want)
+			}
+		})
+	}
+}