Skip to content

fix(recipe bug): treat empty string as "any" in Criteria.Specificity()#492

Merged
yuanchen8911 merged 1 commit intoNVIDIA:mainfrom
yuanchen8911:fix/specificity-empty-string
Apr 6, 2026
Merged

fix(recipe bug): treat empty string as "any" in Criteria.Specificity()#492
yuanchen8911 merged 1 commit intoNVIDIA:mainfrom
yuanchen8911:fix/specificity-empty-string

Conversation

@yuanchen8911
Copy link
Copy Markdown
Contributor

Summary

Fix Specificity() to treat empty string "" as equivalent to "any" for YAML-parsed criteria fields, consistent with Matches() and MatchesCriteriaField().

Motivation / Context

When overlay YAML is parsed, omitted criteria fields get the Go zero value "", not the "any" constant. Specificity() checked only field \!= "any", so empty fields contributed to the score — all overlays reported specificity 4-5 regardless of how many criteria fields they actually specify. This made the sort in FindMatchingOverlays non-deterministic for overlays at different specificity levels.

The bug was masked by the current linear overlay tree structure (every matching overlay is an ancestor of the most-specific leaf). It becomes observable with any overlay structure that has competing matches at different specificity levels.

Discovered during prototype implementation of ADR-005 (PR #439).

Fixes: N/A
Related: #439, #305

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Refactoring (no functional changes)
  • Build/CI/tooling

Component(s) Affected

  • CLI (cmd/aicr, pkg/cli)
  • API server (cmd/aicrd, pkg/api, pkg/server)
  • Recipe engine / data (pkg/recipe)
  • Bundlers (pkg/bundler, pkg/component/*)
  • Collectors / snapshotter (pkg/collector, pkg/snapshotter)
  • Validator (pkg/validator)
  • Core libraries (pkg/errors, pkg/k8s)
  • Docs/examples (docs/, examples/)
  • Other: ____________

Implementation Notes

The fix adds && field \!= "" to each specificity check, aligning with the existing logic in MatchesCriteriaField() which already treats both "" and "any" as wildcards.

Regression tests cover YAML-parsed zero-value criteria (struct literals with omitted fields) alongside the existing tests that use explicit Criteria*Any values.

Observable behavior change: appliedOverlays metadata may list overlays in a different order for queries that match b200-any-training or gb200-any-training (these overlays now sort at their correct specificity). Hydrated recipe content (constraints, components, validation, deployment order) is unchanged for all current leaf overlays.

Testing

go test -race -v ./pkg/recipe/... -run TestCriteriaSpecificity

Golden-file comparison of all current leaf overlays confirms zero semantic change to hydrated recipe output.

Risk Assessment

  • Low — Isolated change, well-tested, easy to revert

Rollout notes: No migration needed. The fix only affects sort ordering of overlays with different specificity levels. Current overlay tree is linear, so recipe output is unchanged.

Checklist

  • Tests pass locally (make test with -race)
  • Linter passes (make lint)
  • I did not skip/disable tests to make CI green
  • I added/updated tests for new functionality
  • I updated docs if user-facing behavior changed
  • Changes follow existing patterns in the codebase
  • Commits are cryptographically signed (git commit -S) — GPG signing info

When overlay YAML is parsed, omitted criteria fields get the Go zero
value "" (empty string), not the "any" constant. Specificity() checked
only field \!= "any", so empty fields contributed to the score — making
all overlays report specificity 4-5 regardless of how many criteria
fields they actually specify. This made the sort in FindMatchingOverlays
non-deterministic for overlays with different numbers of specified
criteria.

Align Specificity() with Matches() and MatchesCriteriaField(), which
already treat "" as equivalent to "any".

Add regression tests covering YAML-parsed zero-value criteria (empty
strings) alongside the existing tests that use explicit Criteria*Any
values.

Signed-off-by: Yuan Chen <yuanchen97@gmail.com>
@yuanchen8911 yuanchen8911 requested a review from a team as a code owner April 6, 2026 18:16
@yuanchen8911 yuanchen8911 requested a review from mchmarny April 6, 2026 18:25
@yuanchen8911 yuanchen8911 changed the title fix(recipe): treat empty string as "any" in Criteria.Specificity() fix(recipe bug): treat empty string as "any" in Criteria.Specificity() Apr 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants