feat(cli,api): consume aicr.Client facade (#1077) by hkii · Pull Request #1108 · NVIDIA/aicr

hkii · 2026-05-29T21:47:49Z

Summary

Migrate the CLI (recipe, query, validate, bundle, mirror) and REST server (/v1/recipe, /v1/query, /v1/bundle) onto the top-level aicr.Client facade instead of constructing recipe.Builder/bundler.Bundler directly or mutating the process-global provider. PR 2 of 2 for #1077.

Stacked on #1107 (per-DataProvider de-globalization). Base is refactor/aicr-recipe-deglobalization; will retarget to main once #1107 merges.

What changes

server.go builds an aicr.Client; no longer constructs recipe.Builder or bundler.Bundler → acceptance criterion reliably use flox in CI #2.
recipe/query/mirror build a per-invocation Client and seed their own per-provider criteria registry; initDataProvider and the last recipe.SetDataProvider calls are removed from pkg/cli → acceptance criterion fix: remove namespaceOverride from nvidia-dra-driver-gpu values #1.
Facade surface added: EmbeddedSource, WithVersion, WithAllowLists, ParseAllowListsFromEnv, ResolveRecipeFromCriteria/FromSnapshot, SelectFromRecipe, LoadRecipe/LoadRecipeBytes, AdoptRecipe, MakeBundle, ValidateState (+ phase/timeout/provider options), Client.LoadCatalog/CriteriaRegistry; Recipe/AllowLists/Criteria/CriteriaRegistry aliases.

Behavior

CLI flags and REST endpoints behave identically (criterion #3). One deliberate change, called out explicitly: CLI validation is no longer bounded by the facade's fixed ValidationOperationTimeout (it's opt-in, default uncapped for the CLI) — the previous CLI path relied only on per-validator timeouts, and the fixed cap would cancel a legitimate all-phase run that includes the 50m inference-perf check.

Testing

make qualify green (tree identical to the validated combined branch). New facade + handler tests assert byte-identical REST responses, allowlist rejection parity, the query hydrate-vs-selector 404/5xx split, and per-Client provider isolation.

Relates to #1077. Note: aicr.Client's *Recipe alias methods are provisional pending the facade-owned wrappers in #1078.

github-actions · 2026-05-29T21:48:35Z

🌿 Preview your docs: https://nvidia-preview-feat-aicr-cli-api-client.docs.buildwithfern.com/aicr

coderabbitai · 2026-05-29T21:55:58Z

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: cf75f7f6-373b-44f9-b49f-24826e6f0e58

📥 Commits

Reviewing files that changed from the base of the PR and between 2f9f028 and 98b5465.

📒 Files selected for processing (28)

docs/integrator/go-library.md
docs/integrator/public-api.md
pkg/aicr/aicr.go
pkg/aicr/aicr_internal_test.go
pkg/aicr/aicr_test.go
pkg/aicr/bundle.go
pkg/aicr/bundle_test.go
pkg/aicr/options.go
pkg/aicr/query.go
pkg/aicr/types.go
pkg/api/bundle_handler.go
pkg/api/bundle_handler_test.go
pkg/api/recipe_handler.go
pkg/api/recipe_handler_test.go
pkg/api/server.go
pkg/api/server_test.go
pkg/bundler/handler.go
pkg/cli/bundle.go
pkg/cli/config_integration_test.go
pkg/cli/mirror.go
pkg/cli/query.go
pkg/cli/recipe.go
pkg/cli/recipe_test.go
pkg/cli/root.go
pkg/cli/validate.go
pkg/cli/validate_evidence.go
pkg/recipe/handler_query.go
pkg/recipe/handler_query_test.go

📝 Walkthrough

Walkthrough

This PR centralizes recipe/catalog/bundling/validation behind a per-command aicr.Client facade, expands the facade with catalog/criteria, allowlists, validation options, and bundling APIs, implements recipe and bundle HTTP handlers using the facade, refactors CLI commands to construct per-command clients, adds tests for facade behavior and handler parity, and updates docs describing validation phases and recipe sources.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related issues

feat(cli,api): migrate CLI and REST server to consume aicr.Client #1077: Migration to per-command/server aicr.Client across CLI and REST layers — this PR implements that migration.
[Epic]: Post-facade adoption and surface hardening (follow-ups to #1072) #1076: Facade expansion and consumption — this PR adds the facade methods and updates consumers accordingly.

Possibly related PRs

NVIDIA/aicr#1072: Introduces the core aicr.Client facade APIs consumed by this PR.
NVIDIA/aicr#1107: Adds per-DataProvider criteria/catalog plumbing that this PR uses (LoadFromFileWithProvider, registry support).
NVIDIA/aicr#999: Related work on criteria registry/load/strict behavior referenced by CLI/catalog changes.

Suggested labels

area/tests, enhancement, area/validator

Suggested reviewers

mchmarny
lalitadithya

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'feat(cli,api): consume aicr.Client facade (`#1077`)' clearly describes the main change: migrating CLI and API components to use the new aicr.Client facade instead of direct construction.
Description check	✅ Passed	The description comprehensively explains the changes: migrating CLI and REST server to aicr.Client facade, lists the facade surface additions, clarifies behavior changes, and mentions testing coverage.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch feat/aicr-cli-api-client

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/cli/bundle.go`:
- Around line 605-626: The client construction here duplicates logic from
recipeClientFromCmd; replace the manual data-dir/source/NewClient block with a
call to recipeClientFromCmd(cmd, cfg) to obtain the client and error, e.g.
client, err := recipeClientFromCmd(cmd, cfg), then handle err the same way and
defer client.Close() as done now. Keep references to cmd, cfg.Recipe().DataDir()
behavior and ensure the returned client is closed in the same defer; remove the
duplicated aicr.NewClient / aicr.WithRecipeSource / aicr.WithVersion code so
behavior is consistent with recipeClientFromCmd.

In `@pkg/cli/recipe.go`:
- Around line 309-349: The function buildCriteriaFromCmd is only referenced by
tests; either wire it into the runtime criteria path or demote it to test-only:
if you want runtime usage, have mergeCriteriaFromCmdAndConfig call
buildCriteriaFromCmd(cmd, reg) to construct CLI-only criteria (replace or
consolidate existing applyCriteriaOverrides logic accordingly) so
buildCriteriaFromCmd is used by production code; otherwise move
buildCriteriaFromCmd into pkg/cli/recipe_test.go (make it unexported if
necessary) and update recipe_test.go to call the local helper, removing the
unused exported function from production code.

In `@pkg/cli/validate.go`:
- Around line 752-776: Duplicate client construction in validate.go should be
replaced with the shared helper recipeClientFromCmd to avoid code duplication:
locate the block that builds the dataDir/source and calls aicr.NewClient (and
defers client.Close) and replace it by invoking recipeClientFromCmd (the same
helper used in root.go) to obtain the client and error handling; ensure you pass
the same inputs (the command flags/context and version/config as used here) and
keep the existing defer close behavior using the returned client.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: cb86e00f-c5da-423d-82e5-cfb87cf4e05a

📥 Commits

Reviewing files that changed from the base of the PR and between 3b7f7d8 and 2f9f028.

📒 Files selected for processing (28)

aicr.go
aicr_internal_test.go
aicr_test.go
bundle.go
bundle_test.go
docs/integrator/go-library.md
docs/integrator/public-api.md
options.go
pkg/api/bundle_handler.go
pkg/api/bundle_handler_test.go
pkg/api/recipe_handler.go
pkg/api/recipe_handler_test.go
pkg/api/server.go
pkg/api/server_test.go
pkg/bundler/handler.go
pkg/cli/bundle.go
pkg/cli/config_integration_test.go
pkg/cli/mirror.go
pkg/cli/query.go
pkg/cli/recipe.go
pkg/cli/recipe_test.go
pkg/cli/root.go
pkg/cli/validate.go
pkg/cli/validate_evidence.go
pkg/recipe/handler_query.go
pkg/recipe/handler_query_test.go
query.go
types.go

github-actions · 2026-05-29T22:33:26Z

@hkii this PR now has merge conflicts with main. Please rebase to resolve them.

…undle (#1077) Migrate the CLI (recipe, query, validate, bundle, mirror) and REST server (/v1/recipe, /v1/query, /v1/bundle) onto the top-level aicr.Client facade instead of constructing recipe.Builder/bundler.Bundler directly or mutating the process-global data provider. server.go no longer builds recipe.Builder or bundler.Bundler; recipe/query/mirror build a per-invocation Client and seed their own per-provider criteria registry. Behavior preserved; the deliberate CLI validation-timeout uncap is opt-in via the facade. Facade surface: EmbeddedSource, WithVersion, WithAllowLists, ParseAllowListsFromEnv, ResolveRecipeFromCriteria/FromSnapshot, SelectFromRecipe, LoadRecipe/LoadRecipeBytes, AdoptRecipe, MakeBundle, ValidateState(+phase/timeout/provider), Client.LoadCatalog + CriteriaRegistry; Recipe/AllowLists/Criteria/CriteriaRegistry aliases.

github-actions · 2026-05-29T23:46:28Z

Coverage Report ✅

Metric	Value
Coverage	76.7%
Threshold	75%
Status	Pass

Coverage Badge

![Coverage](https://img.shields.io/badge/coverage-76.7%25-green)

No Go source files changed in this PR.

github-actions Bot added area/docs area/bundler area/cli area/api size/XL labels May 29, 2026

coderabbitai Bot reviewed May 29, 2026

View reviewed changes

Comment thread pkg/cli/bundle.go

Comment thread pkg/cli/recipe.go Outdated

Comment thread pkg/cli/validate.go

Base automatically changed from refactor/aicr-recipe-deglobalization to main May 29, 2026 22:33

github-actions Bot added the needs-rebase label May 29, 2026

This was referenced May 29, 2026

feat(aicr): wrap facade alias types as facade-owned structs #1111

Merged

refactor(aicr): move public facade from module root to pkg/aicr/ #1112

Merged

mchmarny assigned hkii May 29, 2026

mchmarny force-pushed the feat/aicr-cli-api-client branch from 2f9f028 to 98b5465 Compare May 29, 2026 23:35

mchmarny marked this pull request as ready for review May 29, 2026 23:36

mchmarny requested a review from a team as a code owner May 29, 2026 23:36

mchmarny approved these changes May 29, 2026

View reviewed changes

mchmarny merged commit 3c0c0a9 into main May 29, 2026
31 of 32 checks passed

mchmarny deleted the feat/aicr-cli-api-client branch May 29, 2026 23:38

This was referenced May 30, 2026

chore(recipe): remove back-compat shims introduced by #1107 #1114

Closed

feat(cli,api): migrate CLI and REST server to consume aicr.Client #1077

Closed

chore(recipe): remove back-compat shims introduced by #1107 #1118

Merged

This was referenced May 30, 2026

feat(recipe): context-aware DataProvider interface #1121

Merged

feat(client/v1): wrap remaining facade alias types #1124

Merged

This was referenced May 30, 2026

refactor: code quality and consistency cleanup #1126

Merged

refactor(server): merge pkg/api into pkg/server, drop legacy direct handlers #1127

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(cli,api): consume aicr.Client facade (#1077)#1108

feat(cli,api): consume aicr.Client facade (#1077)#1108
mchmarny merged 1 commit into
mainfrom
feat/aicr-cli-api-client

hkii commented May 29, 2026

Uh oh!

github-actions Bot commented May 29, 2026

Uh oh!

coderabbitai Bot commented May 29, 2026 •

edited

Loading

Review failed

Walkthrough

Estimated code review effort

Possibly related issues

Possibly related PRs

Suggested labels

Suggested reviewers

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented May 29, 2026

Uh oh!

Uh oh!

github-actions Bot commented May 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

hkii commented May 29, 2026

Summary

What changes

Behavior

Testing

Uh oh!

github-actions Bot commented May 29, 2026

Uh oh!

coderabbitai Bot commented May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review failed

Walkthrough

Estimated code review effort

Possibly related issues

Possibly related PRs

Suggested labels

Suggested reviewers

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented May 29, 2026

Uh oh!

Uh oh!

github-actions Bot commented May 29, 2026

Coverage Report ✅

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented May 29, 2026 •

edited

Loading