OLS-0000 Fix spec staleness: clarify Agent status.conditions - #362
OLS-0000 Fix spec staleness: clarify Agent status.conditions#362xrajesh wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe Agent CRD documentation now describes ChangesAgent CRD status documentation
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.ai/spec/what/crd-api.md:
- Line 25: Update the `Agent — status.conditions` specification entry to define
the `Ready` condition as indicating whether the referenced LLMProvider and
credential Secret resources are present, matching the contract in `Agent` status
definitions; remove the broader “accessible” wording and preserve the note that
the operator does not currently set these conditions.
🪄 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: CHILL
Plan: Enterprise
Run ID: fc2a5d60-c97d-4b84-b627-9594d8c03b8d
📒 Files selected for processing (2)
.ai/spec/health-report.md.ai/spec/what/crd-api.md
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift/lightspeed-agentic-sandbox(manual)
| 17. **Agent — `spec.maxTurns`**: Optional bound on tool-use turns per invocation. | ||
| 18. **Agent — `spec.reasoningConfig`**: Optional freeform map (`map[string]interface{}`, JSON key `reasoningConfig`). When present, the operator MUST serialize it as `LIGHTSPEED_REASONING_CONFIG` JSON env var on the sandbox pod (see `sandbox-execution.md` rule 16a). When absent, the env var MUST be omitted and the sandbox uses SDK defaults. Contents are provider- and model-specific (e.g., Claude `thinking`/`effort`, Gemini `thinking_budget`/`thinking_level`, OpenAI `reasoning.effort`/`verbosity`); the operator passes the map as-is without validation — the sandbox and upstream SDK/API validate at invocation time. This field is aligned with the classic OLS operator's `ModelParametersSpec.ReasoningConfig` ([OLS-3452]). | ||
| 19. **Agent — `status.conditions`**: [PLANNED] Observed readiness; `Ready` condition is defined on the API but no controller currently reconciles Agent status. When implemented, it SHOULD document whether referenced provider resources are accessible. | ||
| 19. **Agent — `status.conditions`**: Observed readiness; `Ready` condition documents whether referenced LLMProvider resources (credentials secrets) are accessible. The operator does not currently set these conditions, but the field is reserved for future health reporting. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Align Ready semantics with the API contract.
The specification says the referenced resources are “accessible,” while api/v1alpha1/agent_types.go:177-189 defines Ready as indicating whether the LLMProvider and credential Secret resources are present. “Accessible” could imply permission or credential validation that this contract does not establish.
-19. **Agent — `status.conditions`**: Observed readiness; `Ready` condition documents whether referenced LLMProvider resources (credentials secrets) are accessible. The operator does not currently set these conditions, but the field is reserved for future health reporting.
+19. **Agent — `status.conditions`**: Observed readiness; the `Ready` condition documents whether referenced LLMProvider and credential Secret resources are present. The operator does not currently set these conditions, but the field is reserved for future health reporting.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 19. **Agent — `status.conditions`**: Observed readiness; `Ready` condition documents whether referenced LLMProvider resources (credentials secrets) are accessible. The operator does not currently set these conditions, but the field is reserved for future health reporting. | |
| 19. **Agent — `status.conditions`**: Observed readiness; the `Ready` condition documents whether referenced LLMProvider and credential Secret resources are present. The operator does not currently set these conditions, but the field is reserved for future health reporting. |
🤖 Prompt for 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.
In @.ai/spec/what/crd-api.md at line 25, Update the `Agent — status.conditions`
specification entry to define the `Ready` condition as indicating whether the
referenced LLMProvider and credential Secret resources are present, matching the
contract in `Agent` status definitions; remove the broader “accessible” wording
and preserve the note that the operator does not currently set these conditions.
- Removed [PLANNED] marker from Agent status.conditions rule (field is reserved, not currently populated) - Console integration correctly documented in how/reconciler.md Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
fadf24f to
29471a5
Compare
|
/lgtm |
|
@xrajesh: you cannot LGTM your own PR. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: xrajesh The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
1 participant
Removed [PLANNED] marker from Agent status.conditions rule. Clarified that the field is reserved for future health reporting but not currently populated by any reconciler.