feat(cli): add --no-health opt-out to recipe list#1314
Conversation
`aicr recipe list` resolves every leaf overlay through pkg/health.Compute on each invocation to populate the STATUS/COVERAGE columns and the health block (NVIDIA#1228). Add a --no-health (alias --skip-health) flag that skips the ComputeHealth call entirely and renders the pre-NVIDIA#1228 enumeration shape: the table omits the STATUS/COVERAGE columns and json/yaml omit the health block. Default behavior is unchanged. Gating lives in pkg/cli only; no pkg/health or facade changes. Documented in docs/user/cli-reference.md and covered by table/json/yaml/alias/empty tests, including that the health block is absent in json/yaml when set. Closes NVIDIA#1311
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR implements a Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
mchmarny
left a comment
There was a problem hiding this comment.
Clean implementation. The showHealth gate is in exactly the right place (CLI layer, facade untouched), nil healthByName correctly signals both the table renderer and the json/yaml omitempty path, and test coverage is thorough (table, JSON, YAML, alias, empty-result intersection). One nit on a PR reference in a code comment; same pattern appears in the writeCatalogEntries godoc at ~line 254 — both can drop the #1228 number. No correctness issues; good to merge once CI clears.
| @@ -151,20 +157,30 @@ Include overlays from an external data directory: | |||
| fmt.Sprintf("unknown output format %q, valid formats are: json, yaml, table", cmd.String(flagFormat))) | |||
| } | |||
There was a problem hiding this comment.
nit: pre-#1228 will rot — PR references in code comments belong in the commit/PR description. Drop the issue number; the behaviour description ("no STATUS/COVERAGE columns, no health block") stands on its own.
Summary
Add a
--no-health(alias--skip-health) opt-out flag toaicr recipe listthat skips the per-leaf structural-health computation and renders only the #1208 enumeration columns/fields.Motivation / Context
As of #1228,
aicr recipe listresolves every leaf overlay throughpkg/health.Compute(viaclient.ComputeHealth) on every invocation to populate theSTATUS/COVERAGEcolumns (table) and thehealthblock (json/yaml). The previously-cheap catalog enumeration now always pays the full health-compute cost, with no way to opt out. For purely interactive "what overlays exist?" lookups — or scripted/CI callers that only consumename/criteria/is_leaf/source— the health resolution is wasted latency.Raised as a non-blocking follow-up in PR #1302 review.
Fixes: #1311
Related: #1228, #1302
Type of Change
Component(s) Affected
cmd/aicr,pkg/cli)docs/,examples/)Implementation Notes
client.ComputeHealth(...)call is gated behindshowHealth := !cmd.Bool(flagNoHealth)and skipped entirely when the flag is set — a real latency win, not just a column drop.healthByNamestaysnil: the table emits the pre-aicr recipe list: add structural-health + coverage columns #1228 8-column header (noSTATUS/COVERAGE), and json/yaml omit thehealthblock via the existingomitempty(reading a nil map yields the zero-value pointer, so each entry'sHealthis nil).pkg/clionly; nopkg/healthor facade changes. Default behavior (no flag) is unchanged from aicr recipe list: add structural-health + coverage columns #1228.Testing
New tests cover the flag across table/json/yaml, the
--skip-healthalias (byte-identical output), and the empty-result +--no-healthintersection — including that thehealthblock is absent in json/yaml when set. The YAML test decodes and asserts nohealthkey (rather than a substring scan) plus a leaf-present guard.pkg/clicoverage: 68.7% → 68.7% (flat, no regression). No new exported functions.Risk Assessment
Rollout notes: Backwards compatible. The flag defaults to
false, so existing invocations and scripts see the #1228 output unchanged.Checklist
make testwith-race)make lint)git commit -S)