fix(docs/api): fix, clean up docs and add doc update audit rule#625
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis pull request updates documentation and the OpenAPI spec to add Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/contributor/api-server.md`:
- Around line 312-313: The accelerator enum documentation (entries `accelerator`
and `gpu` referencing `AcceleratorType`) is missing implemented type
`rtx-pro-6000`; update every changed enumeration list and any example
validation/error strings to include `rtx-pro-6000` (in the same format as
`b200`), and ensure all occurrences mentioned in the comment (around the shown
diff plus the other doc locations referenced) reflect the full, current
AcceleratorType set so docs and examples match the parser/type list.
🪄 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: Pro Plus
Run ID: 5ee99496-bd2e-4bcb-b673-f4c52e0866fb
📒 Files selected for processing (10)
docs/README.mddocs/contributor/api-server.mddocs/contributor/cli.mddocs/contributor/data.mddocs/contributor/index.mddocs/contributor/validations.mddocs/integrator/components/skyhook.mddocs/integrator/data-flow.mddocs/user/api-reference.mddocs/user/cli-reference.md
yuanchen8911
force-pushed
the
fix-docs-review-findings
branch
from
7a02a19 to
c331f47
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
docs/contributor/api-server.md (1)
312-313:⚠️ Potential issue | 🟠 MajorAccelerator values are still inconsistent with implemented types across docs and examples.
These updated sections still omit
rtx-pro-6000(and the validator snippet is also missingany). Please align all four locations to the same implemented set.Suggested patch
-| `accelerator` | AcceleratorType | Enum: h100, gb200, b200, a100, l40, any | `accelerator=h100` | +| `accelerator` | AcceleratorType | Enum: h100, gb200, b200, a100, l40, rtx-pro-6000, any | `accelerator=h100` | -- `accelerator` - GPU/accelerator type (h100, gb200, b200, a100, l40) +- `accelerator` - GPU/accelerator type (h100, gb200, b200, a100, l40, rtx-pro-6000) - "message": "invalid gpu type: must be one of h100, gb200, b200, a100, l40, any", + "message": "invalid gpu type: must be one of h100, gb200, b200, a100, l40, rtx-pro-6000, any", - GPU string `validate:"required,oneof=h100 gb200 b200 a100 l40"` + GPU string `validate:"required,oneof=h100 gb200 b200 a100 l40 rtx-pro-6000 any"`As per coding guidelines
**/*.md: “Ensure accuracy, use neutral tone, clearly structure content, consider audience awareness, and document operational clarity.”Also applies to: 334-335, 416-417, 2165-2166
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/contributor/api-server.md` around lines 312 - 313, Update the documented AcceleratorType values used in the `accelerator` and `gpu` table rows and the corresponding validator examples to match the actual implemented set: include `rtx-pro-6000` in the enum/list and ensure `any` is present where the validator accepts it; apply the same corrected list consistently to the other three occurrences referenced (the other `accelerator`/`gpu` table entries and the validator snippet) so all four locations use the identical, implemented set of values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/user/cli-reference.md`:
- Line 317: Update the accelerator enum list in the CLI docs table row that
shows the `--accelerator` / `--gpu` option to include the missing value
`rtx-pro-6000` so the documented accepted set matches the criteria parsing
implementation; locate the table cell with the `--accelerator` / `--gpu` entry
and append `, rtx-pro-6000` (or reorder alphabetically with the other values
h100, gb200, b200, a100, l40) to keep the docs and code in sync.
---
Duplicate comments:
In `@docs/contributor/api-server.md`:
- Around line 312-313: Update the documented AcceleratorType values used in the
`accelerator` and `gpu` table rows and the corresponding validator examples to
match the actual implemented set: include `rtx-pro-6000` in the enum/list and
ensure `any` is present where the validator accepts it; apply the same corrected
list consistently to the other three occurrences referenced (the other
`accelerator`/`gpu` table entries and the validator snippet) so all four
locations use the identical, implemented set of values.
🪄 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: Pro Plus
Run ID: be8a9555-1553-4668-80de-337fa9e58894
📒 Files selected for processing (10)
docs/README.mddocs/contributor/api-server.mddocs/contributor/cli.mddocs/contributor/data.mddocs/contributor/index.mddocs/contributor/validations.mddocs/integrator/components/skyhook.mddocs/integrator/data-flow.mddocs/user/api-reference.mddocs/user/cli-reference.md
yuanchen8911
force-pushed
the
fix-docs-review-findings
branch
from
c331f47 to
62fdf4e
Compare
|
Tip For best results, initiate chat on the files or code changes.
Both deferrals are well-reasoned:
Neither should come into this PR. Would you like me to open GitHub issues for both follow-up items so they don't get lost? I can file them now and link back to this PR for context. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docs/contributor/api-server.md (1)
312-312:⚠️ Potential issue | 🟠 MajorAlign all accelerator enums/examples with the implemented accepted set.
These updated lists are still inconsistent with implementation:
rtx-pro-6000is missing, and Line 334 also omitsany. Please update all four locations so query-parameter docs, error examples, and validator snippets match the same canonical set.As per coding guidelines
**/*.md: “Ensure accuracy, use neutral tone, clearly structure content, consider audience awareness, and document operational clarity.”Also applies to: 334-335, 416-417, 2165-2165
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/contributor/api-server.md` at line 312, The accelerator enum/examples in the docs are inconsistent with the implemented AcceleratorType; update all occurrences that document the `accelerator` query parameter, error examples, and validator snippets so they use the same canonical set (including adding `rtx-pro-6000` and ensuring `any` appears where omitted). Locate the `accelerator` field/AcceleratorType mentions in the four affected sections (around the table entry with `accelerator | AcceleratorType`, the error example block, and the validator snippet) and replace the listed values so every place lists the full canonical enum set: h100, gb200, b200, a100, l40, rtx-pro-6000, any. Ensure wording and examples remain neutral and consistent with surrounding docs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@docs/contributor/api-server.md`:
- Line 312: The accelerator enum/examples in the docs are inconsistent with the
implemented AcceleratorType; update all occurrences that document the
`accelerator` query parameter, error examples, and validator snippets so they
use the same canonical set (including adding `rtx-pro-6000` and ensuring `any`
appears where omitted). Locate the `accelerator` field/AcceleratorType mentions
in the four affected sections (around the table entry with `accelerator |
AcceleratorType`, the error example block, and the validator snippet) and
replace the listed values so every place lists the full canonical enum set:
h100, gb200, b200, a100, l40, rtx-pro-6000, any. Ensure wording and examples
remain neutral and consistent with surrounding docs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4b22981e-16c3-4b54-995f-db0605b828fe
📒 Files selected for processing (10)
docs/README.mddocs/contributor/api-server.mddocs/contributor/cli.mddocs/contributor/data.mddocs/contributor/index.mddocs/contributor/validations.mddocs/integrator/components/skyhook.mddocs/integrator/data-flow.mddocs/user/api-reference.mddocs/user/cli-reference.md
yuanchen8911
force-pushed
the
fix-docs-review-findings
branch
from
62fdf4e to
89035d0
Compare
Enum values (accelerator, service, OS, intent, platform, error code) are enumerated across many files, so "update the doc for this kind of change" doesn't translate to a single file. Extend the Documentation updates lookup table with a dedicated entry for this case. Two things it prescribes that the generic per-kind bullets don't: 1. Start from the authoritative Go type, not from a known-good sibling value. Grepping `docs/` for a sibling (e.g., `gb200` when adding `b200`) works for forward additions but misses pre-existing drift — rtx-pro-6000 was supported in code and overlays for months without appearing in any enum documentation, and a sibling-grep wouldn't have surfaced it. Enumerate every current value of the Go type and verify each. 2. Include the OpenAPI contract at api/aicr/v1/server.yaml. It's the API contract file, not part of docs/, but it mirrors the same enum and drifts the same way — PR NVIDIA#625 found it missing both b200 and rtx-pro-6000 from every enum block. Motivated by PR NVIDIA#625, which first missed B200 in nine places and later missed rtx-pro-6000 in the same places despite adding the rule — the sibling-grep heuristic was the gap.
yuanchen8911
changed the title
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/contributor/api-server.md`:
- Line 334: The accelerator enum examples are inconsistent: the documented valid
value "any" appears in one place but is missing from other enumerations of the
accelerator field; update every occurrence where accelerator values are listed
(the sections that enumerate accelerator values for the accelerator field) to
include "any" alongside the other values (h100, gb200, b200, a100, l40,
rtx-pro-6000) so all examples and the accelerator documentation are consistent
and aligned with the earlier statement that "any" is valid.
In `@docs/contributor/data.md`:
- Line 140: The documented example values for the accelerator field include a
deferred value that no longer matches the API enum; update the table entry for
`accelerator` in docs/contributor/data.md to remove `rtx-pro-6000` and list only
the current API-supported enum values (`h100`, `gb200`, `b200`, `a100`, `l40`)
so the docs remain an accurate source-of-truth for contributors.
In `@docs/contributor/validations.md`:
- Line 97: The docs entry for the "accelerator" condition incorrectly lists an
unsupported value; remove "rtx-pro-6000" from the `accelerator` enum list so the
documented values match the codebase's accepted enums (e.g., h100, gb200, b200,
a100, l40) and do not introduce features not implemented; update the single-line
list under the `accelerator` heading accordingly and verify it matches the
project's actual enum definition for accelerators.
In `@docs/README.md`:
- Line 11: Update the accelerator allowlist in the README table by removing the
deferred value `rtx-pro-6000` from the backticked list inside the **Criteria**
cell (the string "`h100/gb200/b200/a100/l40/rtx-pro-6000`"); keep remaining
values and punctuation/formatting unchanged so the documented accelerators match
the current OpenAPI enum.
In `@docs/user/api-reference.md`:
- Line 120: The docs incorrectly list the unsupported GPU type `rtx-pro-6000` in
the API valid-values for the `accelerator` field; remove `rtx-pro-6000` from the
comma-separated valid-value lists where `accelerator` is documented (the table
row containing `accelerator` and the other occurrence at the second location) so
the list becomes `h100`, `gb200`, `b200`, `a100`, `l40`, `any` to match the
current spec.
In `@docs/user/cli-reference.md`:
- Line 317: The CLI reference row for the `--accelerator`/`--gpu` flag currently
lists a deferred value `rtx-pro-6000`; remove `rtx-pro-6000` from the string of
supported values so the table only shows currently implemented options (e.g.,
keep h100, gb200, b200, a100, l40) in the `--accelerator` / `--gpu` table entry
to avoid documenting unimplemented features.
🪄 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: Pro Plus
Run ID: b781f696-c767-40d3-aae3-f8f3fdfc571d
📒 Files selected for processing (11)
api/aicr/v1/server.yamldocs/README.mddocs/contributor/api-server.mddocs/contributor/cli.mddocs/contributor/data.mddocs/contributor/index.mddocs/contributor/validations.mddocs/integrator/components/skyhook.mddocs/integrator/data-flow.mddocs/user/api-reference.mddocs/user/cli-reference.md
yuanchen8911
force-pushed
the
fix-docs-review-findings
branch
from
89035d0 to
25f79fd
Compare
yuanchen8911
changed the title
There was a problem hiding this comment.
♻️ Duplicate comments (6)
docs/contributor/validations.md (1)
97-97:⚠️ Potential issue | 🟠 MajorDo not list deferred accelerator values in validation condition docs.
Line 97 should remain aligned to currently supported enums;
rtx-pro-6000should be added only when the corresponding spec/implementation update ships.As per coding guidelines
**/*.md: “Ensure accuracy… Documentation must not invent features, guarantees, timelines, or roadmap commitments.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/contributor/validations.md` at line 97, The docs entry listing allowed values for the "accelerator" validation incorrectly includes a deferred value; remove "rtx-pro-6000" from the `accelerator` enum list in docs/contributor/validations.md so the documented values match the currently supported enums (keep only h100, gb200, b200, a100, l40), and do not add any mention of the deferred accelerator until the corresponding spec/implementation change is merged.docs/contributor/data.md (1)
140-140:⚠️ Potential issue | 🟠 MajorKeep
acceleratorexample values spec-accurate (exclude deferredrtx-pro-6000).Line 140 should list only currently accepted accelerator enums to avoid contributor-facing source-of-truth drift.
As per coding guidelines
**/*.md: “Ensure accuracy… Documentation must not invent features, guarantees, timelines, or roadmap commitments.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/contributor/data.md` at line 140, The `accelerator` example values list includes a deferred value; update the documentation entry for the `accelerator` field to list only the currently accepted enum values (e.g., remove `rtx-pro-6000`) so the example matches the spec; locate the `accelerator` row in docs/contributor/data.md and edit the pipe-separated example values accordingly.docs/user/cli-reference.md (1)
317-317:⚠️ Potential issue | 🟠 MajorRemove deferred
rtx-pro-6000from Query Mode accelerator docs.Line 317 should document only currently implemented accelerator values to prevent CLI-doc drift.
As per coding guidelines
**/*.md: “Ensure accuracy… Documentation must not invent features, guarantees, timelines, or roadmap commitments.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/user/cli-reference.md` at line 317, Update the Query Mode accelerator docs to remove the not-yet-implemented "rtx-pro-6000" entry from the `--accelerator` / `--gpu` allowed values list so it only documents currently supported accelerator types (h100, gb200, b200, a100, l40); edit the table row that contains `--accelerator` / `--gpu` to delete "rtx-pro-6000" and ensure the remaining values are comma-separated and accurate.docs/README.md (1)
11-11:⚠️ Potential issue | 🟠 MajorRemove deferred
rtx-pro-6000from the documented accelerator list.Line 11 currently documents a value that was deferred, which reintroduces docs/spec drift. Keep this list to currently accepted values only (including
b200, excludingrtx-pro-6000until the combined spec+docs PR lands).As per coding guidelines
**/*.md: “Ensure accuracy… Documentation must not invent features, guarantees, timelines, or roadmap commitments.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/README.md` at line 11, The documented accelerator list includes a deferred value `rtx-pro-6000`; update the README's accelerator enumeration (the text containing `accelerator` and values like `b200`) to remove `rtx-pro-6000` so only currently accepted accelerators remain (e.g., `b200`, `h100`, `gb200`, `a100`, `l40`) and ensure the list formatting/backticks stay consistent with the surrounding sentence.docs/user/api-reference.md (1)
120-120:⚠️ Potential issue | 🟠 MajorRevert deferred
rtx-pro-6000from API valid-value documentation.Line 120 and Line 817 should only list currently accepted accelerator values. Keeping
rtx-pro-6000here documents behavior that is intentionally deferred.As per coding guidelines
**/*.md: “Ensure accuracy… Documentation must not invent features, guarantees, timelines, or roadmap commitments.”Also applies to: 817-817
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/user/api-reference.md` at line 120, The docs list a deferred accelerator value (`rtx-pro-6000`) in the `accelerator` API valid-values table; remove the `rtx-pro-6000` token wherever it appears in the `accelerator` row(s) in docs/user/api-reference.md (both occurrences) so the table only lists currently accepted values (e.g., keep `h100`, `gb200`, `b200`, `a100`, `l40`, `rtx-pro-6000` removed, `any`), and ensure punctuation/backticks and spacing are adjusted so the table row remains syntactically valid Markdown after removal.docs/contributor/api-server.md (1)
334-335:⚠️ Potential issue | 🟡 MinorKeep
anyconsistent in accelerator enumerations.
anyis documented as valid in the parameter table, but it is still missing in the GET bullets and validator snippet. This leaves conflicting guidance on the same page.As per coding guidelines
**/*.md: “Ensure accuracy, use neutral tone, clearly structure content, consider audience awareness, and document operational clarity.”Also applies to: 2165-2166
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/contributor/api-server.md` around lines 334 - 335, The documentation lists `accelerator` and its alias `gpu` but omits the `any` option in the GET bullets and validator snippet, creating inconsistent guidance; update the GET parameter bullets and the validator example that reference `accelerator`/`gpu` to include `any` as an allowed value and ensure the same `any` mention is added wherever `accelerator`/`gpu` enumerations appear (including the other duplicated occurrence), so all examples and validation snippets consistently document `any` as valid.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@docs/contributor/api-server.md`:
- Around line 334-335: The documentation lists `accelerator` and its alias `gpu`
but omits the `any` option in the GET bullets and validator snippet, creating
inconsistent guidance; update the GET parameter bullets and the validator
example that reference `accelerator`/`gpu` to include `any` as an allowed value
and ensure the same `any` mention is added wherever `accelerator`/`gpu`
enumerations appear (including the other duplicated occurrence), so all examples
and validation snippets consistently document `any` as valid.
In `@docs/contributor/data.md`:
- Line 140: The `accelerator` example values list includes a deferred value;
update the documentation entry for the `accelerator` field to list only the
currently accepted enum values (e.g., remove `rtx-pro-6000`) so the example
matches the spec; locate the `accelerator` row in docs/contributor/data.md and
edit the pipe-separated example values accordingly.
In `@docs/contributor/validations.md`:
- Line 97: The docs entry listing allowed values for the "accelerator"
validation incorrectly includes a deferred value; remove "rtx-pro-6000" from the
`accelerator` enum list in docs/contributor/validations.md so the documented
values match the currently supported enums (keep only h100, gb200, b200, a100,
l40), and do not add any mention of the deferred accelerator until the
corresponding spec/implementation change is merged.
In `@docs/README.md`:
- Line 11: The documented accelerator list includes a deferred value
`rtx-pro-6000`; update the README's accelerator enumeration (the text containing
`accelerator` and values like `b200`) to remove `rtx-pro-6000` so only currently
accepted accelerators remain (e.g., `b200`, `h100`, `gb200`, `a100`, `l40`) and
ensure the list formatting/backticks stay consistent with the surrounding
sentence.
In `@docs/user/api-reference.md`:
- Line 120: The docs list a deferred accelerator value (`rtx-pro-6000`) in the
`accelerator` API valid-values table; remove the `rtx-pro-6000` token wherever
it appears in the `accelerator` row(s) in docs/user/api-reference.md (both
occurrences) so the table only lists currently accepted values (e.g., keep
`h100`, `gb200`, `b200`, `a100`, `l40`, `rtx-pro-6000` removed, `any`), and
ensure punctuation/backticks and spacing are adjusted so the table row remains
syntactically valid Markdown after removal.
In `@docs/user/cli-reference.md`:
- Line 317: Update the Query Mode accelerator docs to remove the
not-yet-implemented "rtx-pro-6000" entry from the `--accelerator` / `--gpu`
allowed values list so it only documents currently supported accelerator types
(h100, gb200, b200, a100, l40); edit the table row that contains `--accelerator`
/ `--gpu` to delete "rtx-pro-6000" and ensure the remaining values are
comma-separated and accurate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5902442f-387e-4ab9-9ca7-00b6d6c0b630
📒 Files selected for processing (13)
.claude/CLAUDE.mdAGENTS.mdapi/aicr/v1/server.yamldocs/README.mddocs/contributor/api-server.mddocs/contributor/cli.mddocs/contributor/data.mddocs/contributor/index.mddocs/contributor/validations.mddocs/integrator/components/skyhook.mddocs/integrator/data-flow.mddocs/user/api-reference.mddocs/user/cli-reference.md
yuanchen8911
force-pushed
the
fix-docs-review-findings
branch
2 times, most recently
from
21ecff6 to
d5d0df6
Compare
yuanchen8911
force-pushed
the
fix-docs-review-findings
branch
from
d5d0df6 to
de1541b
Compare
yuanchen8911
force-pushed
the
fix-docs-review-findings
branch
from
de1541b to
8be2f97
Compare
yuanchen8911
changed the title
yuanchen8911
changed the title
yuanchen8911
force-pushed
the
fix-docs-review-findings
branch
from
8be2f97 to
f48aecc
Compare
|
@coderabbitai Please re-review the four open threads about |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 8532
Docs pages updated: The Go type, OpenAPI contract, and all documentation are now fully in sync on [resolve] |
Apply the corrections identified in a cross-docs review. Three enum drifts between the Go source types and the documented/advertised values are closed here: - **Accelerator**: B200 (supported since PR NVIDIA#455) and RTX PRO 6000 (supported as CriteriaAcceleratorRTXPro6000 with real overlays under recipes/overlays/rtx-pro-6000-*.yaml) were missing from the OpenAPI contract and most documentation. - **Service**: `kind` and `lke` were defined in CriteriaServiceType (pkg/recipe/criteria.go) but missing from the OpenAPI `service` enum and every documented service listing. - **Deployer**: `argocd-helm` was defined in the OpenAPI deployer enum (api/aicr/v1/server.yaml:845) but not documented in the user API reference or CLI flag table. Both enums are now updated in every place they're surfaced: - api/aicr/v1/server.yaml — five accelerator enum blocks and three service enum blocks; - user docs (docs/README.md glossary, user cli-reference and api-reference) and contributor docs (api-server, cli, data, validations); - the site-docs glossary mirror (site/docs/getting-started/index.md); - Go-visible surfaces in the defining package: godoc in pkg/recipe/doc.go, field comment in pkg/recipe/criteria.go, package godoc in pkg/api/doc.go and pkg/server/doc.go, and the `aicr recipe` --help Description in pkg/cli/recipe.go; - .github/ISSUE_TEMPLATE/bug_report.yml GPU-type hint. While normalizing service listings, legacy `self-managed` references were removed from the two package godocs (pkg/api/doc.go, pkg/server/doc.go) and the contributor `validate` struct-tag example — `self-managed` is parsed as an alias for `any`, not a distinct enum value, so advertising it as a valid standalone option was misleading. Other docs corrections in scope: replace HTML entities (<) with literal < in the Performance Characteristics bullets across docs/integrator/data-flow.md, docs/contributor/api-server.md and docs/contributor/cli.md so they render correctly in MDX; refresh the CI pipeline description in docs/contributor/index.md to the current tool versions (Go 1.26, golangci-lint v2.10.1) per .settings.yaml; fix the GPU_TYPES shell array in docs/contributor/cli.md to add b200 and rtx-pro-6000 and drop the unsupported l4 so copy-pasted examples don't fail at runtime; replace two fictional invalid-accelerator error examples with the literal output produced by the binary today (captured by running `aicr recipe --accelerator invalid-gpu` and `GET /v1/recipe?accelerator=invalid-gpu` against HEAD); and rewrite three ungrammatical sentences in the Skyhook integrator page. Add a dedicated bullet to the "Documentation updates" rule in .claude/CLAUDE.md and AGENTS.md for enum/constant additions, so this class of drift is less likely to recur. The bullet names a broader audit surface than any single review pass: OpenAPI contract, user/ contributor docs, the site-docs mirror, package godoc and urfave/cli Description strings in the defining package, and .github/ISSUE_TEMPLATE/*.yml dropdowns. It also prescribes starting from the authoritative Go type rather than a known-good sibling value — grepping for gb200 catches forward b200 additions but misses pre-existing drift like rtx-pro-6000, which had no sibling documented anywhere.
yuanchen8911
force-pushed
the
fix-docs-review-findings
branch
from
f48aecc to
aa20f6a
Compare
3 participants
Summary
This PR does three things:
docs/— typos, stale CI tool versions, MDX-incompatible HTML entities, a broken shell example that failed at runtime, two fictional error-message examples the binary never produced, and three ungrammatical sentences in the Skyhook page.b200andrtx-pro-6000were defined inpkg/recipe/criteria.goand real overlays but missing fromapi/aicr/v1/server.yamland most docs.kindandlkewere defined inCriteriaServiceTypebut missing from the OpenAPI enum and every documented service listing; legacyself-managed(parsed as an alias forany) was advertised as a distinct enum value.argocd-helmwas in the OpenAPI deployer enum but not documented in the user-facing parameter or flag tables..claude/CLAUDE.md, Codex viaAGENTS.md— auto-synced) so this class of drift is less likely to recur. The bullet prescribes starting from the authoritative Go type rather than grepping for an already-documented sibling value, and it names the full audit surface: OpenAPI contract, user/contributor docs, the site-docs mirror, package godoc and urfave/cliDescriptionstrings, and.github/ISSUE_TEMPLATE/*.ymldropdowns.A single follow-up issue (#629) is filed for two Codex-flagged drifts that are structurally different and deserve their own PRs: the
platformenum + missingRecipeCriteriaproperty, and error-code naming (INTERNAL_ERROR/NO_MATCHING_RULEadvertised in docs but not defined inpkg/errors/errors.go).Motivation / Context
Started as a full pass over every page under
docs/onorigin/mainlooking for review-level issues. Mid-review, Claude Code / CodeRabbit / Codex cross-review rounds kept surfacing the same shape of problem: accelerator, service, and deployer enum values defined in Go types but absent from the docs/API contract. Each drift followed the same pattern the new agent rule is designed to prevent.Fixes: N/A (review-driven cleanup + three enum-drift fixes + preventative rule)
Related: #628 (Skyhook → Nodewright rename follow-up), #629 (platform + error-code enum drift follow-up)
Type of Change
Component(s) Affected
cmd/aicrd,pkg/api,pkg/server) — OpenAPI contract (api/aicr/v1/server.yaml) enum values; package godoc comments inpkg/api/doc.go,pkg/server/doc.gocmd/aicr,pkg/cli) —aicr recipe--helpDescription text inpkg/cli/recipe.gopkg/recipe) — field comments onServiceandAcceleratorstruct fields, package godoc inpkg/recipe/doc.godocs/,site/docs/, agent instruction files.claude/CLAUDE.md+AGENTS.md)Implementation Notes
Enum drifts closed
Accelerator — Go type
pkg/recipe/criteria.go:87,95-106,114definesCriteriaAcceleratorTypewith values{a100, b200, gb200, h100, l40, rtx-pro-6000}.b200was supported since PR #455,rtx-pro-6000has four real overlays underrecipes/overlays/rtx-pro-6000-*.yaml. Both missing from the OpenAPI enum and most docs.Service — Go type
pkg/recipe/criteria.go:39-48definesCriteriaServiceTypewith values{eks, gke, aks, oke, kind, lke}.kindandlkemissing from OpenAPI enum and every documented service listing. Legacyself-managed(parsed as an alias foranyatpkg/recipe/criteria.go:53-54, not a distinct enum value) was advertised as a valid standalone option in a few places; removed for consistency.Deployer —
api/aicr/v1/server.yaml:845definesenum: [helm, argocd, argocd-helm].argocd-helmmissing from the user-facing flag and query-parameter tables despite being documented in detail later in the CLI reference.All three are now updated wherever the enum is surfaced:
api/aicr/v1/server.yaml— 5 accelerator enum blocks + 3 service enum blocksdocs/README.md(glossary),docs/user/cli-reference.md,docs/user/api-reference.mddocs/contributor/api-server.md,docs/contributor/cli.md,docs/contributor/data.md,docs/contributor/validations.mdsite/docs/getting-started/index.md(glossary mirror)pkg/recipe/doc.go(package godoc including Criteria struct example + Criteria Types sections for Service, Accelerator, Intent, OS, Platform),pkg/recipe/criteria.go(ServiceandAcceleratorfield comments),pkg/api/doc.go,pkg/server/doc.gopkg/cli/recipe.go—aicr recipe--helpDescription string.github/ISSUE_TEMPLATE/bug_report.yml— GPU-type hint + Platform/service hintOther docs corrections
<) → literal<indocs/integrator/data-flow.md(lines 855, 856, 861, 863, 870),docs/contributor/api-server.md:794,docs/contributor/cli.md:1075,1077.docs/contributor/index.md:1114:Go setup (1.25), ..., golangci-lint (v2.6.2)→Go setup (1.26), ..., golangci-lint (v2.10.1)to match.settings.yaml(Go 1.26.2, golangci-lint v2.10.1).docs/contributor/cli.md:1697,1700:GPU_TYPES=("h100" "gb200" "a100" "l40" "l4")andK8S_SERVICES=("eks" "gke" "aks" "self-managed")— neitherl4norself-managedis in the respective enums, so copy-pasting the script failed or silently matched the wrong recipes. Normalized to the canonical enum sets.docs/contributor/api-server.md:416anddocs/contributor/cli.md:1052— previously advertised error text that the binary never produced. Replaced with literal output captured by running the binary today (aicr recipe --accelerator invalid-gpuandGET /v1/recipe?accelerator=invalid-gpuagainst HEAD), including the full wrapper chain that reflects the structured-error layering.docs/integrator/components/skyhook.md— three sentences rewritten at lines 3, 68, and 74.Agent audit rule (Claude Code + Codex)
Adds one bullet to the "Documentation updates" rule in
.claude/CLAUDE.md(canonical for Claude Code) andAGENTS.md(auto-synced mirror used by Codex, enforced bytools/check-agents-sync). Two things it prescribes that the generic per-kind bullets don't:docs/forgb200when addingb200catches forward additions, butrtx-pro-6000had no documented sibling anywhere, so the sibling-grep heuristic missed it. Enumerate the Go type's current values and verify each appears wherever it should.docs/. The bullet now names: the OpenAPI contract atapi/aicr/v1/server.yaml; user/contributor doc pages; the site-docs mirror undersite/docs/; Go-visible surfaces in the package that defines the type (package godoc, field/type comments, urfave/cliDescription/Usagestrings); and.github/ISSUE_TEMPLATE/*.ymldropdowns.Findings deferred (tracked separately)
RecipeCriteriaproperty —pkg/recipe/criteria.go:181-189defines platform valueskubeflow,dynamo,nimbut the OpenAPI enum is stillkubeflow/any, and theRecipeCriteriaschema atserver.yaml:1222has noplatformproperty at all. Adding a new schema property is a structural change, not a mechanical enum extension. → Audit non-accelerator enums for Go-type vs OpenAPI/docs drift (platform, error codes) #629.server.yaml:1028,1167advertiseINTERNAL_ERRORanddocs/user/api-reference.md:565,567listNO_MATCHING_RULE/INTERNAL_ERROR; none matchpkg/errors/errors.go:21(which definesINTERNAL,NOT_FOUND,UNAUTHORIZED,TIMEOUT,SERVICE_UNAVAILABLE,INVALID_REQUEST). Wire-value correction with potential external-client implications that warrant their own PR narrative. → Audit non-accelerator enums for Go-type vs OpenAPI/docs drift (platform, error codes) #629.NVIDIA/skyhook→NVIDIA/nodewrightrename (@lockwobr inline nit) — 611 hits across 92 files, cascades into component directories and Go constants. → Rename Skyhook references to Nodewright throughout the codebase #628.Testing
Docs + OpenAPI + Go godoc comments + one CLI
--helpstring. All Go changes are comments or user-facing string literals — no executable code paths affected.Skipped:
make test/make scan/make e2eon the full suite — no executable code paths touched; targeted package tests covered the affected code.The CI
lycheelink-check workflow (.github/workflows/fern-docs-ci.yaml) will run on this PR.Risk Assessment
aicr recipe --helptext) adds accelerator values to a hint string, which has no behavioral effect. Easy to revert.Rollout notes: N/A — no behavior change, no migration required.
Checklist
make testwith-race) — targeted on touched packages; all passmake lint) —yamllint,golangci-lint,check-agents-sync,check-docs-sidebarall cleangit commit -S)