Skip to content

Commit 9fc11bd

Browse files
authored
feat(sdk): expose snapshot criteria relaxation as a resolve option (NVIDIA#2247)
Signed-off-by: Mark Chmarny <mark@chmarny.com>
1 parent 142c792 commit 9fc11bd

16 files changed

Lines changed: 1650 additions & 549 deletions

docs/contributor/cli.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ packages (`pkg/recipe`, `pkg/bundler`, `pkg/snapshotter`,
138138
| `ResolveRecipe(ctx, RecipeRequest)` | `recipe`, `query` (request can hold criteria, file path, or snapshot input) |
139139
| `ResolveRecipeFromCriteria(ctx, *Criteria)` | criteria-only fast path |
140140
| `ResolveRecipeFromSnapshot(ctx, *Criteria, *Snapshot)` | `validate`, `recipe --snapshot` |
141+
| `ResolveRecipeFromSnapshotWithOptions(ctx, *Criteria, *Snapshot, opts...)` | `recipe --snapshot`, `query --snapshot` — with `WithSnapshotCriteriaRelaxation(stated...)` for the derived-criteria retry |
141142
| `LoadRecipe(ctx, path, kubeconfig)` | `bundle`, `validate`, `diff` (read a previously emitted recipe file) |
142143
| `BundleComponents(ctx, *RecipeResult)` | `bundle` |
143144
| `LoadSnapshot(ctx, path, kubeconfig)` | `validate`, `query`, `diff` (read a previously captured snapshot; file, URL, or `cm://` ConfigMap) |
@@ -155,6 +156,14 @@ rendering, validator orchestration, OCI pushes — is a boundary
155156
violation. If the facade is missing the surface you need, add it to
156157
`pkg/client/v1` first.
157158

159+
**What `--snapshot` still owns.** `buildRecipeFromCmdWithConfig` in
160+
`query.go` derives criteria from the snapshot fingerprint, layers config and
161+
flags on top, and records which of the five coverage dimensions were
162+
explicitly stated in a `touched` map. That map becomes the argument to
163+
`aicr.WithSnapshotCriteriaRelaxation` — the relax-and-retry itself lives in
164+
the facade (issue #2027). Only this layer can know a flag was set, so
165+
declaring the stated set is the CLI's job; acting on it is not.
166+
158167
## Output Writers
159168

160169
User-facing output goes through `cmd.Root().Writer`, never

docs/contributor/recipe.md

Lines changed: 45 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -499,21 +499,53 @@ non-`NotFound` error through `aicrerrors.PropagateOrWrap(..., ErrCodeInternal,
499499
...)` before returning it — an evaluator that hasn't adopted `pkg/errors`
500500
still surfaces a coded error instead of an uncoded 500 at the server layer.
501501

502-
**The engine stays strict; the CLI's snapshot path relaxes derived-only
502+
**The engine stays strict; the SDK's snapshot path can relax derived-only
503503
failures.** Everything above describes `pkg/recipe`'s behavior, which never
504504
relaxes the post-condition — a coverage failure there is always terminal.
505-
The CLI's `--snapshot` flow (`pkg/cli/query.go`) layers a caller-side
506-
retry on top: `service`, `accelerator`, and `os` can be derived from the
507-
snapshot fingerprint rather than stated by the user (`intent` and
508-
`platform` are always user-stated — the fingerprint never derives them).
509-
If a coverage error's uncovered dimensions are *all* fingerprint-derived
510-
(none came from `--config` or a CLI flag), the CLI clears those dimensions
511-
to unstated and retries resolution once, logging a warning per relaxed
512-
dimension; if any uncovered dimension was user-stated, the error still
513-
propagates unchanged. This lets an overlay tree that is deliberately
514-
agnostic to a dimension (e.g. Kind's OS-agnostic overlays) tolerate a
515-
snapshot that still reports a concrete value for it, without weakening the
516-
post-condition for anyone who asked for that dimension explicitly.
505+
The relax-and-retry lives one layer up, in `pkg/client/v1`
506+
(`relax.go`), behind an opt-in resolve option:
507+
508+
```go
509+
result, err := client.ResolveRecipeFromSnapshotWithOptions(ctx, criteria, snap,
510+
aicr.WithSnapshotCriteriaRelaxation(aicr.DimensionIntent))
511+
```
512+
513+
`service`, `accelerator`, and `os` can be derived from the snapshot
514+
fingerprint rather than stated by the user (`intent` and `platform` are
515+
always user-stated — the fingerprint never derives them). The option's
516+
arguments name the dimensions the *caller* stated; everything else is
517+
treated as derived. If a coverage error's uncovered dimensions are *all*
518+
safely relaxable, the facade clears them to unstated and retries resolution
519+
once, logging a warning per relaxed dimension and reporting them in
520+
`pkg/client/v1.RecipeResult.RelaxedDimensions` — the facade's result type,
521+
not the resolver's `RecipeResult` documented under
522+
[Observable RecipeResult Surfaces](#observable-reciperesult-surfaces) below.
523+
524+
**Not every uncovered dimension is relaxable**, and the distinction is why
525+
`verifyCriteriaCoverage` records `constraintExcluded` per entry. A dimension
526+
no overlay states at all is safe to clear — nothing in the recipe
527+
distinguishes the detected value. A dimension whose only provider was
528+
removed by constraint evaluation is not: clearing it converts a real
529+
incompatibility (the cluster failed that overlay's constraints) into a
530+
broader recipe that resolves at exit 0. The facade refuses in that case, and
531+
refuses again if clearing would leave no stated *coverage* dimension, which
532+
would match every overlay and resolve the generic fallback — the same
533+
fail-open as issue #1888. That check counts only the five coverage
534+
dimensions, not `Specificity()`, because `nodes` scores a specificity point
535+
while participating in no overlay match (#1781). A stated dimension is never
536+
relaxed either way.
537+
538+
This lets an overlay tree that is deliberately agnostic to a dimension
539+
(e.g. Kind's OS-agnostic overlays) tolerate a snapshot that still reports a
540+
concrete value for it, without weakening the post-condition for anyone who
541+
asked for that dimension explicitly.
542+
543+
Omitting the option keeps the strict behavior, so the coverage
544+
post-condition is unchanged for every caller that does not opt in — the
545+
REST recipe endpoint among them. `pkg/cli/query.go` passes the option and
546+
supplies the stated set from its `touched` map; declaring which dimensions
547+
a user typed is the one part of the policy that has to stay in the CLI,
548+
since only that layer knows a flag was set (issue #2027).
517549

518550
## Determinism
519551

docs/integrator/go-library.md

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,12 +398,104 @@ For a per-resolution Slurm accounting mode, use
398398
`aicr.WithAccountingMode("customer-managed")`. The original criteria and
399399
snapshot method signatures remain unchanged for source compatibility.
400400

401+
### Criteria relaxation on the snapshot path
402+
403+
A snapshot resolve is strict by default — every criteria dimension you state
404+
must be honored by an applied overlay, or resolution fails with
405+
`ErrCodeInvalidRequest` and a `details.uncovered` payload.
406+
407+
That is right for criteria a user typed, but wrong for criteria you *derived*
408+
from the snapshot fingerprint. An overlay tree can be deliberately agnostic to
409+
a dimension (Kind's overlays state no `os`) while the fingerprint still detects
410+
a concrete value on the node — nothing in the recipe distinguishes it, so
411+
failing there rejects a legitimate query.
412+
413+
Pass `aicr.WithSnapshotCriteriaRelaxation` and name the dimensions **you
414+
received explicitly**. Anything else is treated as derived, and a coverage
415+
failure limited to derived dimensions is retried once with those cleared:
416+
417+
```go
418+
import (
419+
aicr "github.com/NVIDIA/aicr/pkg/client/v1"
420+
421+
// Deriving criteria from a snapshot has no facade-owned helper yet, so
422+
// this step reaches past the stable surface — see the caveat below.
423+
"github.com/NVIDIA/aicr/pkg/fingerprint" // Internal
424+
"github.com/NVIDIA/aicr/pkg/recipe" // Public (evolving)
425+
)
426+
427+
criteria := fingerprint.FromMeasurements(snap.Unwrap().Measurements).
428+
ToCriteria(client.CriteriaRegistry())
429+
criteria.Intent = recipe.CriteriaIntentTraining // the user asked for this one
430+
431+
result, err := client.ResolveRecipeFromSnapshotWithOptions(
432+
ctx, aicr.WrapCriteria(criteria), snap,
433+
aicr.WithSnapshotCriteriaRelaxation(aicr.DimensionIntent))
434+
if err != nil {
435+
log.Fatalf("resolve: %v", err)
436+
}
437+
for _, dim := range result.RelaxedDimensions {
438+
log.Printf("resolved recipe is broader than requested: %s was relaxed", dim)
439+
}
440+
```
441+
442+
> **The fingerprint step is an escape hatch, not stable API.** `pkg/fingerprint`
443+
> is [Internal](public-api.md#stability-tiers) and may change without notice;
444+
> `pkg/recipe` is Public (evolving) and may change in a minor bump. Only the
445+
> `aicr.*` calls above carry the facade's compatibility guarantee. Pin the AICR
446+
> version and re-audit this block on upgrade, or derive criteria yourself and
447+
> hand the facade an `*aicr.Criteria`. If you need this without the coupling,
448+
> say so on [#2016](https://github.com/NVIDIA/aicr/issues/2016) — a facade-owned
449+
> snapshot-to-criteria helper is the obvious gap it exposes.
450+
451+
Relaxation is deliberately narrow. Three cases propagate the original coverage
452+
error rather than retrying:
453+
454+
- **A dimension you named.** Relaxing a value the caller asked for would
455+
silently resolve a different recipe than requested.
456+
- **A constraint-excluded dimension.** An overlay carrying it exists, but the
457+
observed cluster failed its constraints — a Kubernetes version below the
458+
overlay's floor, say. Relaxing there converts "your cluster does not meet
459+
this overlay's requirements" into a broader recipe that resolves cleanly,
460+
discarding the finding you most need.
461+
- **A relaxation that would leave no stated coverage dimension.** Such criteria
462+
match every overlay and resolve the generic fallback recipe at exit 0 — the
463+
fail-open the pre-resolution specificity guard exists to prevent (#1888).
464+
Note this is not the same as "criteria is empty": a fingerprint-derived
465+
`nodes` value survives the clear, but no overlay gates on `nodes`, so it
466+
selects nothing.
467+
468+
The distinction in the second case is *why* the dimension is uncovered: no
469+
overlay states it at all (safe to relax — nothing in the recipe distinguishes
470+
the value) versus an overlay states it but was constraint-excluded (not safe).
471+
The resolver reports which, per dimension, in the coverage error's
472+
`details.uncovered[].constraintExcluded`.
473+
474+
Two more properties:
475+
476+
- **Passing no dimensions is meaningful,** not a no-op: it means every
477+
dimension was derived and all are relaxable. That is the common case for a
478+
pure fingerprint query. Presence of the option is what enables the policy, so
479+
omitting it entirely is how you keep strict behavior.
480+
- **It is snapshot-only.** On `ResolveRecipeFromCriteria` there is no
481+
fingerprint, so the option is rejected with `ErrCodeInvalidRequest` rather
482+
than ignored.
483+
484+
Both attempts share the call's timeout budget, and relaxation happens at most
485+
once.
486+
401487
The returned `*RecipeResult` carries:
402488

403489
- `Name`, `Version`, `TranslatedAt` — stable identity
404490
- `Components``[]ComponentRef` (Name, Kind, Version, Source, Chart, Namespace)
405491
- `SelectedProfile` — selected name/value and declaration-wide `OwnedPaths`;
406492
nil for legacy recipes
493+
- `RelaxedDimensions` — criteria dimensions cleared by
494+
`WithSnapshotCriteriaRelaxation`. Non-empty only when the first attempt failed
495+
coverage on derived dimensions **and** the retry succeeded. Every other
496+
outcome — option not passed, first attempt succeeded, relaxation refused, or
497+
the retry itself failed — yields either an empty slice or `nil, error` with no
498+
`RecipeResult` at all, so this field is never the way to detect a failure
407499
- `Resolved()` — the upstream `*pkg/recipe.RecipeResult` for callers that
408500
need constraints, deployment order, validation config, or metadata
409501
(e.g., evidence emission). Do not mutate; do not retain past the

docs/integrator/public-api.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ unrelated exports in their evolving packages remain free to change.
7070
| `aicr.Phase`, `aicr.PhaseDeployment` / `PhasePerformance` / `PhaseConformance` | string consts | **Facade-owned**. Values match `pkg/validator/v1` constants verbatim for byte-identical wire round-trip. |
7171
| `aicr.ReportSummary` | `pkg/validator/ctrf.Summary` | **Facade-owned struct** with the CTRF count fields. |
7272
| `aicr.ValidateOption` | `pkg/validator.Option` | **Facade-owned** functional-option type that captures into an internal struct and translates at call time. |
73-
| `aicr.RecipeResult` | `pkg/recipe.RecipeResult` | **Facade-owned struct** exposing `Name`, `Version`, `TranslatedAt`, `SelectedProfile` (the recorded ADR-015 selection, nil for unprofiled recipes), and `Components` (enabled/deployable components only — disabled refs remain visible via `Resolved().ComponentRefs`). Call `Resolved()` for the full upstream `*pkg/recipe.RecipeResult` (constraints, deployment order, validation config, metadata). The previous `aicr.Recipe` alias was removed in #1115; `ResolveRecipeFromCriteria` and `ResolveRecipeFromSnapshot` now return `*RecipeResult`. |
73+
| `aicr.RecipeResult` | `pkg/recipe.RecipeResult` | **Facade-owned struct** exposing `Name`, `Version`, `TranslatedAt`, `SelectedProfile` (the recorded ADR-015 selection, nil for unprofiled recipes), `Components` (enabled/deployable components only — disabled refs remain visible via `Resolved().ComponentRefs`), and `RelaxedDimensions` (criteria dimensions cleared by `WithSnapshotCriteriaRelaxation`; non-empty only when that option was passed, the first attempt failed the coverage post-condition on derived dimensions, **and** the retry succeeded — a refused or failed retry returns `nil, error` with no `RecipeResult`, so this field never signals failure). Call `Resolved()` for the full upstream `*pkg/recipe.RecipeResult` (constraints, deployment order, validation config, metadata). The previous `aicr.Recipe` alias was removed in #1115; `ResolveRecipeFromCriteria` and `ResolveRecipeFromSnapshot` now return `*RecipeResult`. |
7474
| `aicr.AllowLists` | `pkg/recipe.AllowLists` | **Facade-owned struct** with `[]string` fields (Accelerators / Services / Intents / OSTypes). Use `aicr.WrapAllowLists` to lift a `*pkg/recipe.AllowLists`. |
7575
| `aicr.Criteria` | `pkg/recipe.Criteria` | **Facade-owned struct** whose enum-typed fields (Service / Accelerator / Intent / OS / Platform) project to plain strings; Nodes stays an `int` per the facade's string/int contract. Use `aicr.WrapCriteria` to lift a `*pkg/recipe.Criteria`. |
7676
| `aicr.BundleConfig` | `pkg/bundler/config.Config` | Deliberate transparent alias. It keeps the facade compatible with `config.NewConfig` and its functional options instead of duplicating the bundler's configuration builder. |

pkg/cli/consts.go

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -43,20 +43,6 @@ const (
4343
// criteriaAny is the wildcard value for any criteria dimension.
4444
const criteriaAny = "any"
4545

46-
// Coverage dimension names, matching pkg/recipe's coverageDimensions (see
47-
// pkg/recipe/coverage.go) name-for-name. Used to track which of the 5
48-
// dimensions subject to the criteria-coverage post-condition (issue #1542)
49-
// were explicitly user-stated (config or CLI flag) vs snapshot-derived, so a
50-
// coverage failure limited to snapshot-derived dimensions can be relaxed and
51-
// retried once (see relaxSnapshotDerivedCoverage in query.go).
52-
const (
53-
coverageDimService = "service"
54-
coverageDimAccelerator = "accelerator"
55-
coverageDimIntent = "intent"
56-
coverageDimOS = "os"
57-
coverageDimPlatform = "platform"
58-
)
59-
6046
// Keyless-signing / OCI-push flag names shared by `validate`, `bundle`,
6147
// and `evidence publish`. Extracted so the same literal is declared once
6248
// (goconst flags a string repeated ≥3 times across the package) and the

0 commit comments

Comments
 (0)