OLS-3588: Add reasoningConfig field to Agent CRD#335
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@onmete: This pull request references OLS-3588 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
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 ignored due to path filters (2)
📒 Files selected for processing (2)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughSummary by CodeRabbit
Walkthrough
ChangesAgent configuration
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Add spec.reasoningConfig freeform map to AgentSpec, allowing cluster admins to pass provider-specific reasoning parameters (Claude thinking/ effort, Gemini thinking_budget, OpenAI reasoning.effort) through the Agent CR. The operator will serialize this as LIGHTSPEED_REASONING_CONFIG env var on sandbox pods (wiring in OLS-3589). Co-authored-by: Cursor <cursoragent@cursor.com>
8834c6c to
02d4692
Compare
![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
🧹 Nitpick comments (2)
api/v1alpha1/agent_types.go (1)
85-94: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd API validation and application tests.
The PR explicitly leaves cluster tests unchecked. Cover omitted
reasoningConfig, nested arbitrary JSON, and{}rejection fromMinProperties=1; also verify the generated CRD accepts the intended payload.🤖 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 `@api/v1alpha1/agent_types.go` around lines 85 - 94, Add API and application tests covering omitted ReasoningConfig, nested arbitrary JSON values, and rejection of an empty object by the MinProperties=1 validation. Verify the generated CRD schema contains the intended validation and accepts valid reasoningConfig payloads, and ensure application behavior preserves omission and serialization of nested values..golangci-kal.yml (1)
24-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winScope the
nomapsexception toAgent.spec.reasoningConfig. That field is the only intentional freeform map here; disabling the rule at the KAL scope can let accidentalmap[...]additions in other API types slip through lint.🤖 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 @.golangci-kal.yml at line 24, Restrict the golangci-lint nomaps exclusion to Agent.spec.reasoningConfig instead of disabling it for the entire KAL scope. Use the linter’s field-specific exclusion mechanism and preserve nomaps enforcement for all other API types and map fields.
🤖 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 `@api/v1alpha1/agent_types.go`:
- Around line 86-94: Remove the public ReasoningConfig field and its associated
CRD documentation/validation from the API type until end-to-end support exists.
Do not expose spec.reasoningConfig while the controller and sandbox lack
LIGHTSPEED_REASONING_CONFIG wiring.
---
Nitpick comments:
In @.golangci-kal.yml:
- Line 24: Restrict the golangci-lint nomaps exclusion to
Agent.spec.reasoningConfig instead of disabling it for the entire KAL scope. Use
the linter’s field-specific exclusion mechanism and preserve nomaps enforcement
for all other API types and map fields.
In `@api/v1alpha1/agent_types.go`:
- Around line 85-94: Add API and application tests covering omitted
ReasoningConfig, nested arbitrary JSON values, and rejection of an empty object
by the MinProperties=1 validation. Verify the generated CRD schema contains the
intended validation and accepts valid reasoningConfig payloads, and ensure
application behavior preserves omission and serialization of nested 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: CHILL
Plan: Enterprise
Run ID: 78968bb0-38d5-4d0e-9534-08b1bb879637
⛔ Files ignored due to path filters (2)
api/v1alpha1/zz_generated.deepcopy.gois excluded by!**/zz_generated.deepcopy.goconfig/crd/bases/agentic.openshift.io_agents.yamlis excluded by!config/crd/bases/**
📒 Files selected for processing (2)
.golangci-kal.ymlapi/v1alpha1/agent_types.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift/lightspeed-agentic-sandbox(manual)
|
@onmete: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. I understand the commands that are listed here. |
|
/lgtm |
3 participants
Summary
spec.reasoningConfigoptional freeform map (map[string]apiextensionsv1.JSON) toAgentSpecin the Agent CRDadditionalProperties: x-kubernetes-preserve-unknown-fields: truenomapsKAL lint rule (deliberate freeform map per spec rule 18)Implements OLS-3588. Part of the reasoning effort epic OLS-3587.
Spec:
crd-api.mdrule 18 (merged in #332).Test plan
make testpasses (unit tests + API module tests)make api-lintpasses (0 issues)go build ./...compiles cleankustomize build config/crd | kubectl apply -f -reasoningConfigpasses validationreasoningConfigstill passes validationMade with Cursor