OU-1409: Match new ols routing and names#1052
Conversation
|
@jgbernalp: This pull request references OU-1409 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 task 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. |
openshift-ci-robot
added
the
jira/valid-reference
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
openshift-ci
Bot
added
the
approved
|
Warning Review limit reached
Next review available in: 16 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (7)
WalkthroughAlert investigation lookup now uses fingerprint-matched AgenticRun resources instead of proposals. New constants and query logic support namespace-aware fetching, while alert rows navigate to the first matching AgenticRun. ChangesAgentic run migration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant AlertTableRow
participant useAgenticRunCheck
participant KubernetesAPI
participant Browser
AlertTableRow->>useAgenticRunCheck: Provide alert
useAgenticRunCheck->>KubernetesAPI: Query AgenticRun by fingerprint and source
KubernetesAPI-->>useAgenticRunCheck: Return matching runs
useAgenticRunCheck-->>AlertTableRow: Return run state and items
AlertTableRow->>Browser: Navigate to first AgenticRun
🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 4
🧹 Nitpick comments (2)
web/src/components/agentic-runs/alert-identifier.ts (1)
47-52: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename
matchesProposalto align with AgenticRun terminology.The PR objective is to rename proposals to agentic runs, but the function name and parameter still use "Proposal." This creates inconsistency for callers and future maintainers.
♻️ Proposed refactor
-export const matchesProposal = (alert: Alert, proposal: K8sResourceCommon): boolean => { - const proposalFp = proposal.metadata?.labels?.[AGENTIC_RUN_LABEL_FINGERPRINT]; - if (!proposalFp) { +export const matchesAgenticRun = (alert: Alert, run: K8sResourceCommon): boolean => { + const runFp = run.metadata?.labels?.[AGENTIC_RUN_LABEL_FINGERPRINT]; + if (!runFp) { return false; } - return proposalFp === getAlertFingerprintPrefix(alert.labels); + return runFp === getAlertFingerprintPrefix(alert.labels); };🤖 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 `@web/src/components/agentic-runs/alert-identifier.ts` around lines 47 - 52, Rename the matchesProposal function and its proposal parameter to use AgenticRun terminology, updating all callers and references consistently while preserving the existing fingerprint-matching behavior.web/src/components/agentic-runs/alert-identifier.spec.ts (1)
20-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign test factory and describe block with AgenticRun terminology.
makeProposalcreates a resource withkind: 'Proposal', but the model now definesAgenticRunModelwithkind: 'AgenticRun'. Thedescribe('matchesProposal')block also uses stale terminology. Both should be renamed to match the PR's objective.♻️ Proposed refactor
-const makeProposal = (labels: Record<string, string>): K8sResourceCommon => ({ +const makeAgenticRun = (labels: Record<string, string>): K8sResourceCommon => ({ apiVersion: 'agentic.openshift.io/v1alpha1', - kind: 'Proposal', + kind: 'AgenticRun', metadata: { - name: 'test-proposal', + name: 'test-agentic-run', namespace: 'openshift-lightspeed', labels, }, });And update the describe block and all
makeProposalcall sites:-describe('matchesProposal', () => { +describe('matchesAgenticRun', () => { it('matches by fingerprint prefix', () => { const alert = makeAlert({ severity: 'critical' }); const fp8 = getAlertFingerprintPrefix(alert.labels); - const proposal = makeProposal({ + const run = makeAgenticRun({ 'agentic.openshift.io/alert-fingerprint': fp8, 'agentic.openshift.io/source': 'alertmanager', }); - expect(matchesProposal(alert, proposal)).toBe(true); + expect(matchesAgenticRun(alert, run)).toBe(true); });Also applies to: 71-71
🤖 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 `@web/src/components/agentic-runs/alert-identifier.spec.ts` around lines 20 - 28, Update the test factory makeProposal and every call site to use AgenticRun terminology, including kind: 'AgenticRun' and an appropriately renamed factory if needed. Rename the describe('matchesProposal') block to the corresponding AgenticRun terminology while preserving the existing test behavior.
🤖 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 `@web/src/components/agentic-runs/alert-identifier.spec.ts`:
- Line 5: Update the imports in alert-identifier.spec.ts so Alert and
K8sResourceCommon are brought in through a type-only import, while AlertStates
remains a regular value import because it is used as AlertStates.Firing.
In `@web/src/components/agentic-runs/constants.ts`:
- Around line 8-12: Update the stale comments surrounding
AGENTIC_RUN_LABEL_FINGERPRINT and AGENTIC_RUN_LABEL_SOURCE in constants.ts,
replacing “Proposal CR” with “AgenticRun CR” while leaving the constants and
values unchanged.
In `@web/src/components/agentic-runs/useAgenticRunCheck.ts`:
- Line 1: Update the import of Alert and K8sResourceCommon to use a type-only
import, since both symbols are used exclusively in type annotations and
parameters.
In `@web/src/components/alerting/AlertList/AlertTableRow.tsx`:
- Around line 64-79: The AgenticRun URL construction in the hasAgenticRun block
must require a defined namespace before adding the investigation link. Update
the guard around runName and getAgenticRunUrl to validate both
run.metadata.namespace and runName, preserving the existing dropdown behavior
when either value is missing.
---
Nitpick comments:
In `@web/src/components/agentic-runs/alert-identifier.spec.ts`:
- Around line 20-28: Update the test factory makeProposal and every call site to
use AgenticRun terminology, including kind: 'AgenticRun' and an appropriately
renamed factory if needed. Rename the describe('matchesProposal') block to the
corresponding AgenticRun terminology while preserving the existing test
behavior.
In `@web/src/components/agentic-runs/alert-identifier.ts`:
- Around line 47-52: Rename the matchesProposal function and its proposal
parameter to use AgenticRun terminology, updating all callers and references
consistently while preserving the existing fingerprint-matching behavior.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 516b7919-99ed-4ccb-9797-77fc2e44a19c
📒 Files selected for processing (9)
web/src/components/agentic-runs/alert-identifier.spec.tsweb/src/components/agentic-runs/alert-identifier.tsweb/src/components/agentic-runs/constants.tsweb/src/components/agentic-runs/useAgenticRunCheck.tsweb/src/components/ai-proposals/constants.tsweb/src/components/ai-proposals/useProposalCheck.tsweb/src/components/alerting/AlertList/AlertTableRow.tsxweb/src/components/console/models/index.tsweb/src/components/hooks/usePerspective.tsx
💤 Files with no reviewable changes (3)
- web/src/components/ai-proposals/constants.ts
- web/src/components/ai-proposals/useProposalCheck.ts
- web/src/components/hooks/usePerspective.tsx
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 4
🧹 Nitpick comments (2)
web/src/components/agentic-runs/alert-identifier.ts (1)
47-52: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename
matchesProposalto align with AgenticRun terminology.The PR objective is to rename proposals to agentic runs, but the function name and parameter still use "Proposal." This creates inconsistency for callers and future maintainers.
♻️ Proposed refactor
-export const matchesProposal = (alert: Alert, proposal: K8sResourceCommon): boolean => { - const proposalFp = proposal.metadata?.labels?.[AGENTIC_RUN_LABEL_FINGERPRINT]; - if (!proposalFp) { +export const matchesAgenticRun = (alert: Alert, run: K8sResourceCommon): boolean => { + const runFp = run.metadata?.labels?.[AGENTIC_RUN_LABEL_FINGERPRINT]; + if (!runFp) { return false; } - return proposalFp === getAlertFingerprintPrefix(alert.labels); + return runFp === getAlertFingerprintPrefix(alert.labels); };🤖 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 `@web/src/components/agentic-runs/alert-identifier.ts` around lines 47 - 52, Rename the matchesProposal function and its proposal parameter to use AgenticRun terminology, updating all callers and references consistently while preserving the existing fingerprint-matching behavior.web/src/components/agentic-runs/alert-identifier.spec.ts (1)
20-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign test factory and describe block with AgenticRun terminology.
makeProposalcreates a resource withkind: 'Proposal', but the model now definesAgenticRunModelwithkind: 'AgenticRun'. Thedescribe('matchesProposal')block also uses stale terminology. Both should be renamed to match the PR's objective.♻️ Proposed refactor
-const makeProposal = (labels: Record<string, string>): K8sResourceCommon => ({ +const makeAgenticRun = (labels: Record<string, string>): K8sResourceCommon => ({ apiVersion: 'agentic.openshift.io/v1alpha1', - kind: 'Proposal', + kind: 'AgenticRun', metadata: { - name: 'test-proposal', + name: 'test-agentic-run', namespace: 'openshift-lightspeed', labels, }, });And update the describe block and all
makeProposalcall sites:-describe('matchesProposal', () => { +describe('matchesAgenticRun', () => { it('matches by fingerprint prefix', () => { const alert = makeAlert({ severity: 'critical' }); const fp8 = getAlertFingerprintPrefix(alert.labels); - const proposal = makeProposal({ + const run = makeAgenticRun({ 'agentic.openshift.io/alert-fingerprint': fp8, 'agentic.openshift.io/source': 'alertmanager', }); - expect(matchesProposal(alert, proposal)).toBe(true); + expect(matchesAgenticRun(alert, run)).toBe(true); });Also applies to: 71-71
🤖 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 `@web/src/components/agentic-runs/alert-identifier.spec.ts` around lines 20 - 28, Update the test factory makeProposal and every call site to use AgenticRun terminology, including kind: 'AgenticRun' and an appropriately renamed factory if needed. Rename the describe('matchesProposal') block to the corresponding AgenticRun terminology while preserving the existing test behavior.
🤖 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 `@web/src/components/agentic-runs/alert-identifier.spec.ts`:
- Line 5: Update the imports in alert-identifier.spec.ts so Alert and
K8sResourceCommon are brought in through a type-only import, while AlertStates
remains a regular value import because it is used as AlertStates.Firing.
In `@web/src/components/agentic-runs/constants.ts`:
- Around line 8-12: Update the stale comments surrounding
AGENTIC_RUN_LABEL_FINGERPRINT and AGENTIC_RUN_LABEL_SOURCE in constants.ts,
replacing “Proposal CR” with “AgenticRun CR” while leaving the constants and
values unchanged.
In `@web/src/components/agentic-runs/useAgenticRunCheck.ts`:
- Line 1: Update the import of Alert and K8sResourceCommon to use a type-only
import, since both symbols are used exclusively in type annotations and
parameters.
In `@web/src/components/alerting/AlertList/AlertTableRow.tsx`:
- Around line 64-79: The AgenticRun URL construction in the hasAgenticRun block
must require a defined namespace before adding the investigation link. Update
the guard around runName and getAgenticRunUrl to validate both
run.metadata.namespace and runName, preserving the existing dropdown behavior
when either value is missing.
---
Nitpick comments:
In `@web/src/components/agentic-runs/alert-identifier.spec.ts`:
- Around line 20-28: Update the test factory makeProposal and every call site to
use AgenticRun terminology, including kind: 'AgenticRun' and an appropriately
renamed factory if needed. Rename the describe('matchesProposal') block to the
corresponding AgenticRun terminology while preserving the existing test
behavior.
In `@web/src/components/agentic-runs/alert-identifier.ts`:
- Around line 47-52: Rename the matchesProposal function and its proposal
parameter to use AgenticRun terminology, updating all callers and references
consistently while preserving the existing fingerprint-matching behavior.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 516b7919-99ed-4ccb-9797-77fc2e44a19c
📒 Files selected for processing (9)
web/src/components/agentic-runs/alert-identifier.spec.tsweb/src/components/agentic-runs/alert-identifier.tsweb/src/components/agentic-runs/constants.tsweb/src/components/agentic-runs/useAgenticRunCheck.tsweb/src/components/ai-proposals/constants.tsweb/src/components/ai-proposals/useProposalCheck.tsweb/src/components/alerting/AlertList/AlertTableRow.tsxweb/src/components/console/models/index.tsweb/src/components/hooks/usePerspective.tsx
💤 Files with no reviewable changes (3)
- web/src/components/ai-proposals/constants.ts
- web/src/components/ai-proposals/useProposalCheck.ts
- web/src/components/hooks/usePerspective.tsx
🛑 Comments failed to post (4)
web/src/components/agentic-runs/alert-identifier.spec.ts (1)
5-5: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use
import typefor type-only symbols.
AlertandK8sResourceCommonare only used for type annotations, whileAlertStatesis used as a value (AlertStates.Firing). Per coding guidelines, type-only symbols should useimport type. As per coding guidelines: "Usetypeimports for symbols that are only used for type checking."📝 Proposed fix
-import { Alert, AlertStates, K8sResourceCommon } from '`@openshift-console/dynamic-plugin-sdk`'; +import { AlertStates } from '`@openshift-console/dynamic-plugin-sdk`'; +import type { Alert, K8sResourceCommon } from '`@openshift-console/dynamic-plugin-sdk`';📝 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.import { AlertStates } from '`@openshift-console/dynamic-plugin-sdk`'; import type { Alert, K8sResourceCommon } from '`@openshift-console/dynamic-plugin-sdk`';🤖 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 `@web/src/components/agentic-runs/alert-identifier.spec.ts` at line 5, Update the imports in alert-identifier.spec.ts so Alert and K8sResourceCommon are brought in through a type-only import, while AlertStates remains a regular value import because it is used as AlertStates.Firing.Source: Coding guidelines
web/src/components/agentic-runs/constants.ts (1)
8-12: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update stale "Proposal CR" comments to "AgenticRun CR".
The comments on lines 8 and 12 still reference "Proposal CR" but the constants are named
AGENTIC_RUN_*, contradicting the PR's stated objective of renaming proposals to agentic runs.📝 Proposed fix
-// Proposal CR label keys +// AgenticRun CR label keys export const AGENTIC_RUN_LABEL_FINGERPRINT = 'agentic.openshift.io/alert-fingerprint'; export const AGENTIC_RUN_LABEL_SOURCE = 'agentic.openshift.io/source'; -// Proposal CR values +// AgenticRun CR values export const AGENTIC_RUN_SOURCE_ALERTMANAGER = 'alertmanager';📝 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.// AgenticRun CR label keys export const AGENTIC_RUN_LABEL_FINGERPRINT = 'agentic.openshift.io/alert-fingerprint'; export const AGENTIC_RUN_LABEL_SOURCE = 'agentic.openshift.io/source'; // AgenticRun CR values export const AGENTIC_RUN_SOURCE_ALERTMANAGER = 'alertmanager';🤖 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 `@web/src/components/agentic-runs/constants.ts` around lines 8 - 12, Update the stale comments surrounding AGENTIC_RUN_LABEL_FINGERPRINT and AGENTIC_RUN_LABEL_SOURCE in constants.ts, replacing “Proposal CR” with “AgenticRun CR” while leaving the constants and values unchanged.web/src/components/agentic-runs/useAgenticRunCheck.ts (1)
1-1: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use
import typefor type-only symbols.
AlertandK8sResourceCommonare only used for type annotations and type parameters. Per coding guidelines: "Usetypeimports for symbols that are only used for type checking." As per coding guidelines, type-only symbols should useimport type.📝 Proposed fix
-import { Alert, K8sResourceCommon } from '`@openshift-console/dynamic-plugin-sdk`'; +import type { Alert, K8sResourceCommon } from '`@openshift-console/dynamic-plugin-sdk`';📝 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.import type { Alert, K8sResourceCommon } from '`@openshift-console/dynamic-plugin-sdk`';🤖 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 `@web/src/components/agentic-runs/useAgenticRunCheck.ts` at line 1, Update the import of Alert and K8sResourceCommon to use a type-only import, since both symbols are used exclusively in type annotations and parameters.Source: Coding guidelines
web/src/components/alerting/AlertList/AlertTableRow.tsx (1)
64-79: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Guard against potential
undefinednamespace in AgenticRun URL.Line 68 passes
run.metadata.namespacetogetAgenticRunUrlwithout checking for existence. WhileAgenticRunModelis namespaced,K8sResourceCommon.metadata.namespaceis typically optional. If undefined, the URL becomes/lightspeed/runs/undefined/name, producing a broken navigation.🛡️ Proposed fix
if (hasAgenticRun) { const run = agenticRuns[0]; const runName = run.metadata?.name; - if (runName) { - const runUrl = getAgenticRunUrl(run.metadata.namespace, runName); + const runNamespace = run.metadata?.namespace; + if (runName && runNamespace) { + const runUrl = getAgenticRunUrl(runNamespace, runName);📝 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.if (hasAgenticRun) { const run = agenticRuns[0]; const runName = run.metadata?.name; const runNamespace = run.metadata?.namespace; if (runName && runNamespace) { const runUrl = getAgenticRunUrl(runNamespace, runName); dropdownItems.push( <DropdownItem key="view-ai-investigation" icon={<CustomIcon name="ai-experience" />} onClick={() => navigate(runUrl)} data-test={DataTestIDs.ViewAIInvestigationDropdownItem} > {t('View AI Investigation')} </DropdownItem>, ); }🤖 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 `@web/src/components/alerting/AlertList/AlertTableRow.tsx` around lines 64 - 79, The AgenticRun URL construction in the hasAgenticRun block must require a defined namespace before adding the investigation link. Update the guard around runName and getAgenticRunUrl to validate both run.metadata.namespace and runName, preserving the existing dropdown behavior when either value is missing.
Signed-off-by: Gabriel Bernal <gbernal@redhat.com>
jgbernalp
force-pushed
the
match-new-ols-routing-and-names
branch
from
ab9a890 to
595655e
Compare
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
|
@jgbernalp: 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. |
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. 🤖 Generated with Claude Code |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jgbernalp, zhuje 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 |
3 participants
requires #1044 to be merged first
Summary by CodeRabbit
New Features
Bug Fixes