Skip to content

Commit 0132f46

Browse files
committed
test(recipe): classify strict failures from context, not message text
Signed-off-by: Brian Lockwood <lockwobr@gmail.com>
1 parent d10c5fb commit 0132f46

3 files changed

Lines changed: 53 additions & 18 deletions

File tree

pkg/recipe/coverage.go

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -482,12 +482,11 @@ func (s *MetadataStore) strictDimensionGaps(criteria *Criteria, applied []string
482482
for _, value := range s.dimensionValues(dim) {
483483
probe := *criteria
484484
setCriteriaDimension(&probe, dim.name, value)
485-
for _, match := range s.FindMatchingOverlays(&probe) {
486-
for _, name := range s.inheritanceChainNames(match) {
487-
if _, already := appliedSet[name]; !already {
488-
reachable[value] = struct{}{}
489-
}
490-
}
485+
// One overlay outside the applied set is enough to establish that
486+
// this value reaches something currently being skipped; the rest
487+
// of the matches and their chains cannot change the answer.
488+
if s.reachesUnappliedOverlay(&probe, appliedSet) {
489+
reachable[value] = struct{}{}
491490
}
492491
}
493492
if len(reachable) == 0 {
@@ -506,6 +505,19 @@ func (s *MetadataStore) strictDimensionGaps(criteria *Criteria, applied []string
506505
return gaps
507506
}
508507

508+
// reachesUnappliedOverlay reports whether resolving probe would pull in any
509+
// overlay that is not already applied.
510+
func (s *MetadataStore) reachesUnappliedOverlay(probe *Criteria, applied map[string]struct{}) bool {
511+
for _, match := range s.FindMatchingOverlays(probe) {
512+
for _, name := range s.inheritanceChainNames(match) {
513+
if _, already := applied[name]; !already {
514+
return true
515+
}
516+
}
517+
}
518+
return false
519+
}
520+
509521
// dimensionValues returns every value the catalog declares for a dimension,
510522
// sorted. Iteration order of s.Overlays is randomized, so the sort is what
511523
// makes strictDimensionGaps deterministic.

pkg/recipe/coverage_matrix_test.go

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import (
1919
stderrors "errors"
2020
"os"
2121
"sort"
22-
"strings"
2322
"testing"
2423

2524
aicrerrors "github.com/NVIDIA/aicr/pkg/errors"
@@ -30,9 +29,13 @@ const goldenPath = "testdata/coverage_golden.yaml"
3029

3130
// coverageClassification is the golden outcome for one projected query.
3231
type coverageClassification struct {
33-
Outcome string `yaml:"outcome"` // "success" | "error"
34-
Uncovered []string `yaml:"uncovered,omitempty"` // coverage errors
35-
RequiresOS bool `yaml:"requiresOS,omitempty"` // requireOSIfNeeded guard errors
32+
Outcome string `yaml:"outcome"` // "success" | "error"
33+
Uncovered []string `yaml:"uncovered,omitempty"` // completeness failures
34+
// StrictDimensions names the dimensions a joint-sufficiency failure
35+
// demanded, read from the error's structured context rather than its
36+
// message text so rewording the message cannot silently reclassify a
37+
// projection and retire the golden matrix's guard on the rule.
38+
StrictDimensions []string `yaml:"strictDimensions,omitempty"`
3639
// ValidCompletions pins the completion-suggestion content per uncovered
3740
// dimension (canonical tupleKey strings, in minimalTuples order) so a
3841
// regression in the suggestion machinery on the real catalog flips a
@@ -112,7 +115,8 @@ func TestCoverageGoldenMatrix(t *testing.T) {
112115
continue
113116
}
114117
if w.Outcome != got[k].Outcome || !equalStrings(w.Uncovered, got[k].Uncovered) ||
115-
w.RequiresOS != got[k].RequiresOS || !equalCompletions(w.ValidCompletions, got[k].ValidCompletions) {
118+
!equalStrings(w.StrictDimensions, got[k].StrictDimensions) ||
119+
!equalCompletions(w.ValidCompletions, got[k].ValidCompletions) {
116120

117121
t.Errorf("projection %q flipped: golden %+v, now %+v", k, w, got[k])
118122
}
@@ -151,10 +155,8 @@ func classify(ctx context.Context, t *testing.T, store *MetadataStore, q *Criter
151155
if err == nil {
152156
return coverageClassification{Outcome: "success"}
153157
}
154-
msg := err.Error()
155-
if strings.Contains(msg, "; specify ") {
156-
// Joint-sufficiency failure (formerly the requireOSIfNeeded guard).
157-
return coverageClassification{Outcome: "error", RequiresOS: true}
158+
if strict := strictDimensionsFromError(err); len(strict) > 0 {
159+
return coverageClassification{Outcome: "error", StrictDimensions: strict}
158160
}
159161
uncovered, completions := coverageDetailsFromError(err)
160162
if len(uncovered) == 0 {
@@ -163,6 +165,28 @@ func classify(ctx context.Context, t *testing.T, store *MetadataStore, q *Criter
163165
return coverageClassification{Outcome: "error", Uncovered: uncovered, ValidCompletions: completions}
164166
}
165167

168+
// strictDimensionsFromError extracts the dimension names a joint-sufficiency
169+
// failure demanded, from the error's `strictDimensions` context, or nil when
170+
// err is not one. Reading the structured context rather than the message keeps
171+
// the golden matrix pinned to the rule instead of to its phrasing.
172+
func strictDimensionsFromError(err error) []string {
173+
var se *aicrerrors.StructuredError
174+
if !stderrors.As(err, &se) || se.Context == nil {
175+
return nil
176+
}
177+
entries, ok := se.Context["strictDimensions"].([]map[string]any)
178+
if !ok {
179+
return nil
180+
}
181+
names := make([]string, 0, len(entries))
182+
for _, entry := range entries {
183+
if name, ok := entry["dimension"].(string); ok {
184+
names = append(names, name)
185+
}
186+
}
187+
return names
188+
}
189+
166190
// coverageDetailsFromError extracts the uncovered dimension names and their
167191
// completion suggestions (as canonical tupleKey strings, preserving
168192
// minimalTuples order) from a StructuredError's "uncovered" context entries,

pkg/recipe/coverage_subsumption_test.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ func TestJointSufficiencySubsumesRetiredGuard(t *testing.T) {
124124
base := &RecipeMetadata{}
125125
base.Metadata.Name = testRecipeBase
126126

127-
checked, guardFired := 0, 0
127+
guardFired := 0
128128
// Catalogs of three overlays drawn from the shape space. Three is enough
129129
// to express the split-coverage catalog (service alone, accelerator alone,
130130
// both plus an os) that the guard was written for.
@@ -147,7 +147,6 @@ func TestJointSufficiencySubsumesRetiredGuard(t *testing.T) {
147147
continue
148148
}
149149
guardFired++
150-
checked++
151150
if _, err := store.BuildRecipeResult(ctx, &query); err == nil {
152151
t.Fatalf("SUBSUMPTION VIOLATED: retired guard rejected %s but resolution succeeded\n"+
153152
" catalog: %s | %s | %s",
@@ -160,5 +159,5 @@ func TestJointSufficiencySubsumesRetiredGuard(t *testing.T) {
160159
if guardFired == 0 {
161160
t.Fatal("generated no catalog where the retired guard fires; the test proves nothing")
162161
}
163-
t.Logf("subsumption held: retired guard fired on %d (catalog, query) pairs, all still rejected", checked)
162+
t.Logf("subsumption held: retired guard fired on %d (catalog, query) pairs, all still rejected", guardFired)
164163
}

0 commit comments

Comments
 (0)