mcpToolUI basic implementation for OpenShift Lightpseed + obs-mcp + Perses#797
mcpToolUI basic implementation for OpenShift Lightpseed + obs-mcp + Perses#797iNecas wants to merge 1 commit intoopenshift:mainfrom
Conversation
Provides a hook to visualize an mcp-tool call with Perses elements. It also adds an ability to add the panel from the OLS chat into opened dashboards.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: iNecas 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 |
openshift-ci
bot
added
the
do-not-merge/work-in-progress
📝 WalkthroughWalkthroughThis pull request introduces an external panel addition feature enabling Perses dashboards to receive panels from external tools. It adds new Redux state management for dashboard operations, creates OLS tool UI components for executing range queries, and integrates a hook-based system to manage externally added panels alongside existing dashboard infrastructure. Changes
Sequence Diagram(s)sequenceDiagram
participant External as External Tool UI
participant Redux as Redux Store
participant Hook as useExternalPanelAddition
participant Dashboard as Dashboard panelEditor
participant Perses as Perses Dashboard
External->>Redux: dispatch dashboardsAddPersesPanelExternally(panelDefinition)
Hook->>Redux: listen for addPersesPanelExternally
Redux->>Hook: trigger addPanelExternally
Hook->>Hook: wrap panelDefinition with groupId
Hook->>Hook: store in local state (externallyAddedPanel)
alt isEditMode = false
Hook->>External: call onEditButtonClick
end
Hook->>Dashboard: apply change via panelEditor
Hook->>Redux: dispatch dashboardsPersesPanelExternallyAdded()
Dashboard->>Perses: update dashboard with new panel
Hook->>Hook: clear externallyAddedPanel state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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: 9
🧹 Nitpick comments (3)
web/src/components/ols-tool-ui/AddToDashboardButton.tsx (1)
42-44: Type the selector state instead of usingany.Line 42-44 bypasses state-shape validation. Strong typing here helps catch integration regressions early.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/ols-tool-ui/AddToDashboardButton.tsx` around lines 42 - 44, The selector currently uses an untyped state (s: any) in AddToDashboardButton.tsx causing loss of type checks; replace the any with the app/root Redux state type (e.g., RootState or AppState) and/or a narrowed interface for the plugins slice so useSelector((s: RootState) => s.plugins?.mp?.dashboards?.isOpened) is typed; update or import the appropriate state type used across the app (or declare PluginsState with mp.dashboards.isOpened) and apply it to the selector to restore compile-time validation for isCustomDashboardOpen.web/src/components/dashboards/perses/useExternalPanelAddition.ts (1)
16-24: Replaceanyin selector/panel path with concrete types.Line 16-24 uses
anyfor both store read and panel definition. This removes compile-time checks exactly where external payloads enter dashboard mutations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/dashboards/perses/useExternalPanelAddition.ts` around lines 16 - 24, Replace the loose any types with concrete interfaces: change the selector typed as useSelector((s: AppState) => s.plugins?.mp?.dashboards?.addPersesPanelExternally) using your app-wide AppState (or PluginsState) type and type the returned value as a known AddPersesPanelExternallyType; replace the panelDefinition parameter in addPanelExternally(panelDefinition: any) and the externallyAddedPanel state to use a defined PanelDefinition (or PersesPanel) type; update related uses in useDashboardActions / openAddPanel and useDashboardStore to accept those concrete types so compiler checks validate external payload shape and downstream mutations (referencing addPersesPanelExternally, useSelector, useDashboardActions, useDashboardStore, addPanelExternally, externallyAddedPanel, setExternallyAddedPanel).web/src/store/actions.ts (1)
21-23: Consider namespacing new action type strings consistently.Line 21-23 uses unscoped literals, while nearby dashboard actions are namespaced (for example
v2/...). Keeping these consistent reduces accidental collisions and simplifies debugging.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/store/actions.ts` around lines 21 - 23, The three action type strings (DashboardsOpened, DashboardsAddPersesPanelExternally, DashboardsPersesPanelExternallyAdded) are not namespaced like the surrounding dashboard actions; update their string values to follow the same namespace pattern (e.g., prefix with the existing namespace used by nearby actions such as "v2/dashboards/" or "dashboards/") so they match the project's convention and avoid collisions, keeping the enum/member names unchanged and only adjusting the right-hand string literals in actions.ts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/components/dashboards/perses/dashboard-app.tsx`:
- Line 129: The hook useExternalPanelAddition currently performs state mutations
and dispatches during render (mutating externallyAddedPanel, calling panelEditor
methods, and dispatching actions/state setters); move all conditional logic that
mutates state, calls panelEditor methods, or dispatches actions into one or more
useEffect blocks with correct dependency arrays (include externallyAddedPanel,
isEditMode, onEditButtonClick, and any panelEditor or dispatch refs) so these
side effects run after render; keep pure synchronous reads in the hook body and
ensure effects guard against repeated execution (e.g., check previous values or
use refs) to avoid re-render loops.
In `@web/src/components/dashboards/perses/PersesWrapper.tsx`:
- Around line 381-382: The InnerWrapper component currently destructures untyped
props; add an explicit props type (e.g., declare an interface or type alias
named InnerWrapperProps) that includes children: React.ReactNode and project:
string | undefined (or the specific Project type if one exists), then annotate
the component signature as function InnerWrapper({ children, project }:
InnerWrapperProps) to improve clarity and future-proof against stricter TS
settings; keep existing usage of useQueryParam/QueryParams.Dashboard/StringParam
unchanged.
In `@web/src/components/dashboards/perses/useExternalPanelAddition.ts`:
- Around line 39-56: The code performs mutations during render; move all side
effects into useEffect hooks: wrap the block that applies the
externallyAddedPanel (accessing dashboardStore.panelGroupOrder[0], set
externallyAddedPanel.groupId, call dashboardStore.panelEditor.applyChanges and
.close, then call setExternallyAddedPanel(null)) into a useEffect that depends
on externallyAddedPanel and dashboardStore.panelGroupOrder; similarly wrap the
addPersesPanelExternally handling (call
addPanelExternally(addPersesPanelExternally) and
dispatch(dashboardsPersesPanelExternallyAdded())) into a separate useEffect that
depends on addPersesPanelExternally; ensure effects bail out early if their
trigger values are falsy to avoid running on every render.
In `@web/src/components/ols-tool-ui/AddToDashboardButton.tsx`:
- Around line 11-13: The panel object in AddToDashboardButton.tsx sets
display.name to an empty string, causing externally added panels to be untitled;
update the assignment of display.name to use a meaningful default (for example
derive from the panel's title/label like panel.title || panel.name) and fallback
to a sensible string such as "External Panel" or "Untitled Panel" so that
display.name is never ''. Locate the object where display: { name: '' } is
created in AddToDashboardButton and replace the empty literal with the derived
value (e.g., use props or the panel data available in the component to populate
the name).
In `@web/src/components/ols-tool-ui/ExecuteRangeQuery.tsx`:
- Around line 9-37: The component ExecuteRangeQuery currently hardcodes
persesTimeRange and DataQueriesProvider step; update it to read duration and
step from tool.args (e.g., tool.args.duration and tool.args.step) and fall back
to the existing defaults ('1h' for duration and 15000 for step) so the UI
matches the tool call; replace persesTimeRange with a computed object using the
provided duration and pass the numeric step into DataQueriesProvider options
(mode: 'range', suggestedStepMs: step) and ensure definitions still use
tool.args.query.
In `@web/src/components/ols-tool-ui/OlsToolUIPersesWrapper.tsx`:
- Line 23: The interface in OlsToolUIPersesWrapper has a typo: change the prop
name `hight` to `height` in the props/interface declaration so it matches the
destructured `height` used in the OlsToolUIPersesWrapper component; update any
references to the interface/type (e.g., the props interface where `hight?:
string;` is declared) to use `height?: string;` to ensure the prop is passed
correctly to the component.
- Line 36: Update the PersesWrapperProps interface so its project property
accepts null (change type from string to string | null) to match usage sites
like OlsToolUIPersesWrapper.tsx where <PersesWrapper project={null}> is passed
and the runtime check in PersesWrapper (the if (!project) branch); update any
related type declarations referencing PersesWrapperProps.project to string |
null to keep typing consistent with activeProject passed from
dashboard-frame.tsx.
In `@web/src/store/actions.ts`:
- Around line 75-83: The Actions type map is missing entries for the three new
action creators; update the Actions interface/type (the map around the Actions
type definition) to include dashboardsOpened: typeof dashboardsOpened,
dashboardsPersesPanelExternallyAdded: typeof
dashboardsPersesPanelExternallyAdded, and dashboardsAddPersesPanelExternally:
typeof dashboardsAddPersesPanelExternally so the new action creators
(dashboardsOpened, dashboardsPersesPanelExternallyAdded,
dashboardsAddPersesPanelExternally) are represented for type-safe reducers and
dispatches.
In `@web/src/store/store.ts`:
- Around line 31-32: The state definition is inconsistent: change
addPersesPanelExternally to allow null (PanelDefinition | null) and ensure
defaultObserveState.dashboards initializes both isOpened (boolean, likely false)
and addPersesPanelExternally (null) to match reducers.ts which sets
addPersesPanelExternally to null; update the type in store.ts and the
defaultObserveState.dashboards object so the type contract and reducer behavior
align with the PanelDefinition symbol and isOpened flag.
---
Nitpick comments:
In `@web/src/components/dashboards/perses/useExternalPanelAddition.ts`:
- Around line 16-24: Replace the loose any types with concrete interfaces:
change the selector typed as useSelector((s: AppState) =>
s.plugins?.mp?.dashboards?.addPersesPanelExternally) using your app-wide
AppState (or PluginsState) type and type the returned value as a known
AddPersesPanelExternallyType; replace the panelDefinition parameter in
addPanelExternally(panelDefinition: any) and the externallyAddedPanel state to
use a defined PanelDefinition (or PersesPanel) type; update related uses in
useDashboardActions / openAddPanel and useDashboardStore to accept those
concrete types so compiler checks validate external payload shape and downstream
mutations (referencing addPersesPanelExternally, useSelector,
useDashboardActions, useDashboardStore, addPanelExternally,
externallyAddedPanel, setExternallyAddedPanel).
In `@web/src/components/ols-tool-ui/AddToDashboardButton.tsx`:
- Around line 42-44: The selector currently uses an untyped state (s: any) in
AddToDashboardButton.tsx causing loss of type checks; replace the any with the
app/root Redux state type (e.g., RootState or AppState) and/or a narrowed
interface for the plugins slice so useSelector((s: RootState) =>
s.plugins?.mp?.dashboards?.isOpened) is typed; update or import the appropriate
state type used across the app (or declare PluginsState with
mp.dashboards.isOpened) and apply it to the selector to restore compile-time
validation for isCustomDashboardOpen.
In `@web/src/store/actions.ts`:
- Around line 21-23: The three action type strings (DashboardsOpened,
DashboardsAddPersesPanelExternally, DashboardsPersesPanelExternallyAdded) are
not namespaced like the surrounding dashboard actions; update their string
values to follow the same namespace pattern (e.g., prefix with the existing
namespace used by nearby actions such as "v2/dashboards/" or "dashboards/") so
they match the project's convention and avoid collisions, keeping the
enum/member names unchanged and only adjusting the right-hand string literals in
actions.ts.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (12)
config/perses-dashboards.patch.jsonweb/package.jsonweb/src/components/dashboards/perses/PersesWrapper.tsxweb/src/components/dashboards/perses/dashboard-app.tsxweb/src/components/dashboards/perses/useExternalPanelAddition.tsweb/src/components/ols-tool-ui/AddToDashboardButton.tsxweb/src/components/ols-tool-ui/ExecuteRangeQuery.tsxweb/src/components/ols-tool-ui/OlsToolUIPersesWrapper.tsxweb/src/components/ols-tool-ui/index.tsweb/src/store/actions.tsweb/src/store/reducers.tsweb/src/store/store.ts
| } | ||
| }; | ||
|
|
||
| useExternalPanelAddition({ isEditMode, onEditButtonClick }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n web/src/components/dashboards/perses/useExternalPanelAddition.tsRepository: openshift/monitoring-plugin
Length of output: 2689
Move render-phase side effects into useEffect blocks.
The hook implementation (lines 39–56 of useExternalPanelAddition.ts) performs state mutations and dispatches directly during render—outside of any useEffect. Lines 39–50 mutate externallyAddedPanel state and call panelEditor methods conditionally on render, and lines 53–56 dispatch actions and trigger state setters during render. This violates React's rendering model and can cause re-render loops and unpredictable behavior. Move the conditional logic in lines 39–50 and 53–56 into useEffect blocks with appropriate dependencies before using this hook in the dashboard component.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/dashboards/perses/dashboard-app.tsx` at line 129, The hook
useExternalPanelAddition currently performs state mutations and dispatches
during render (mutating externallyAddedPanel, calling panelEditor methods, and
dispatching actions/state setters); move all conditional logic that mutates
state, calls panelEditor methods, or dispatches actions into one or more
useEffect blocks with correct dependency arrays (include externallyAddedPanel,
isEditMode, onEditButtonClick, and any panelEditor or dispatch refs) so these
side effects run after render; keep pure synchronous reads in the hook body and
ensure effects guard against repeated execution (e.g., check previous values or
use refs) to avoid re-render loops.
| function InnerWrapper({ children, project }) { | ||
| const [dashboardName] = useQueryParam(QueryParams.Dashboard, StringParam); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the current state of the file at the mentioned lines
wc -l web/src/components/dashboards/perses/PersesWrapper.tsxRepository: openshift/monitoring-plugin
Length of output: 129
🏁 Script executed:
# Check TypeScript strictness settings
fd 'tsconfig.*json$' -type f | head -20Repository: openshift/monitoring-plugin
Length of output: 241
🏁 Script executed:
# Get context around lines 381-382 in PersesWrapper.tsx
sed -n '375,390p' web/src/components/dashboards/perses/PersesWrapper.tsxRepository: openshift/monitoring-plugin
Length of output: 535
🏁 Script executed:
# Check TS strictness rules in tsconfig files
fd 'tsconfig.*json$' -type f -exec grep -l 'strict\|noImplicitAny' {} \;Repository: openshift/monitoring-plugin
Length of output: 241
🏁 Script executed:
# Find tsconfig files and check their content
fd 'tsconfig' | head -10Repository: openshift/monitoring-plugin
Length of output: 114
🏁 Script executed:
# Check TypeScript strictness in tsconfig files
fd 'tsconfig' | xargs catRepository: openshift/monitoring-plugin
Length of output: 660
🏁 Script executed:
# Search for InnerWrapper usage/calls in the file
rg -n 'InnerWrapper' web/src/components/dashboards/perses/PersesWrapper.tsxRepository: openshift/monitoring-plugin
Length of output: 226
🏁 Script executed:
# Get broader context around InnerWrapper to see how it's used
sed -n '365,420p' web/src/components/dashboards/perses/PersesWrapper.tsxRepository: openshift/monitoring-plugin
Length of output: 1858
Consider typing InnerWrapper props for better code clarity and maintainability.
Line 381 destructures untyped props. While the current TypeScript configuration (strict: false) allows implicit any types, adding explicit types improves clarity and protects against future stricter TS settings.
🔧 Suggested improvement
+interface InnerWrapperProps {
+ children?: React.ReactNode;
+ project: string;
+}
-
-function InnerWrapper({ children, project }) {
+function InnerWrapper({ children, project }: InnerWrapperProps) {📝 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.
| function InnerWrapper({ children, project }) { | |
| const [dashboardName] = useQueryParam(QueryParams.Dashboard, StringParam); | |
| interface InnerWrapperProps { | |
| children?: React.ReactNode; | |
| project: string; | |
| } | |
| function InnerWrapper({ children, project }: InnerWrapperProps) { | |
| const [dashboardName] = useQueryParam(QueryParams.Dashboard, StringParam); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/dashboards/perses/PersesWrapper.tsx` around lines 381 -
382, The InnerWrapper component currently destructures untyped props; add an
explicit props type (e.g., declare an interface or type alias named
InnerWrapperProps) that includes children: React.ReactNode and project: string |
undefined (or the specific Project type if one exists), then annotate the
component signature as function InnerWrapper({ children, project }:
InnerWrapperProps) to improve clarity and future-proof against stricter TS
settings; keep existing usage of useQueryParam/QueryParams.Dashboard/StringParam
unchanged.
| if (externallyAddedPanel) { | ||
| const groupId = dashboardStore.panelGroupOrder[0]; | ||
| externallyAddedPanel.groupId = groupId; | ||
|
|
||
| // Use the temporary panelEditor to add changes to the dashboard. | ||
| const panelEditor = dashboardStore.panelEditor; | ||
| panelEditor.applyChanges(externallyAddedPanel); | ||
| panelEditor.close(); | ||
|
|
||
| // Clear the externally added panel after applying changes | ||
| setExternallyAddedPanel(null); | ||
| } | ||
|
|
||
| // Listen for external panel addition requests | ||
| if (addPersesPanelExternally) { | ||
| addPanelExternally(addPersesPanelExternally); | ||
| dispatch(dashboardsPersesPanelExternallyAdded()); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "useExternalPanelAddition.ts" -type fRepository: openshift/monitoring-plugin
Length of output: 137
🏁 Script executed:
wc -l ./web/src/components/dashboards/perses/useExternalPanelAddition.tsRepository: openshift/monitoring-plugin
Length of output: 140
🏁 Script executed:
cat -n ./web/src/components/dashboards/perses/useExternalPanelAddition.tsRepository: openshift/monitoring-plugin
Length of output: 2689
Move side effects into useEffect hooks to prevent update-during-render warnings and infinite loops.
Lines 39-56 execute side effects during render: setExternallyAddedPanel(null), dispatch(dashboardsPersesPanelExternallyAdded()), and panelEditor.applyChanges()/panelEditor.close(). This causes these mutations to run on every render when their conditions are met, potentially triggering re-render cycles that execute the same mutations again.
Suggested fix
- // Apply externally added panel
- if (externallyAddedPanel) {
- const groupId = dashboardStore.panelGroupOrder[0];
- externallyAddedPanel.groupId = groupId;
-
- // Use the temporary panelEditor to add changes to the dashboard.
- const panelEditor = dashboardStore.panelEditor;
- panelEditor.applyChanges(externallyAddedPanel);
- panelEditor.close();
-
- // Clear the externally added panel after applying changes
- setExternallyAddedPanel(null);
- }
-
- // Listen for external panel addition requests
- if (addPersesPanelExternally) {
- addPanelExternally(addPersesPanelExternally);
- dispatch(dashboardsPersesPanelExternallyAdded());
- }
+ useEffect(() => {
+ if (!addPersesPanelExternally) return;
+ addPanelExternally(addPersesPanelExternally);
+ dispatch(dashboardsPersesPanelExternallyAdded());
+ }, [addPersesPanelExternally, dispatch, isEditMode, onEditButtonClick]);
+
+ useEffect(() => {
+ if (!externallyAddedPanel) return;
+ const groupId = dashboardStore.panelGroupOrder?.[0];
+ const panelEditor = dashboardStore.panelEditor;
+ if (groupId === undefined || !panelEditor) return;
+
+ panelEditor.applyChanges({ ...externallyAddedPanel, groupId });
+ panelEditor.close();
+ setExternallyAddedPanel(null);
+ }, [externallyAddedPanel, dashboardStore]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/dashboards/perses/useExternalPanelAddition.ts` around
lines 39 - 56, The code performs mutations during render; move all side effects
into useEffect hooks: wrap the block that applies the externallyAddedPanel
(accessing dashboardStore.panelGroupOrder[0], set externallyAddedPanel.groupId,
call dashboardStore.panelEditor.applyChanges and .close, then call
setExternallyAddedPanel(null)) into a useEffect that depends on
externallyAddedPanel and dashboardStore.panelGroupOrder; similarly wrap the
addPersesPanelExternally handling (call
addPanelExternally(addPersesPanelExternally) and
dispatch(dashboardsPersesPanelExternallyAdded())) into a separate useEffect that
depends on addPersesPanelExternally; ensure effects bail out early if their
trigger values are falsy to avoid running on every render.
| display: { | ||
| name: '', | ||
| }, |
There was a problem hiding this comment.
Externally added panels will be untitled.
Line 12 sets display.name to '', so added panels are hard to distinguish in dashboards.
✅ Suggested fix
display: {
- name: '',
+ name: query,
},📝 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.
| display: { | |
| name: '', | |
| }, | |
| display: { | |
| name: query, | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/ols-tool-ui/AddToDashboardButton.tsx` around lines 11 -
13, The panel object in AddToDashboardButton.tsx sets display.name to an empty
string, causing externally added panels to be untitled; update the assignment of
display.name to use a meaningful default (for example derive from the panel's
title/label like panel.title || panel.name) and fallback to a sensible string
such as "External Panel" or "Untitled Panel" so that display.name is never ''.
Locate the object where display: { name: '' } is created in AddToDashboardButton
and replace the empty literal with the derived value (e.g., use props or the
panel data available in the component to populate the name).
| type ExecuteRangeQueryTool = { | ||
| name: 'execute_range_query'; | ||
| args: { | ||
| query: string; | ||
| }; | ||
| }; | ||
|
|
||
| const persesTimeRange = { | ||
| pastDuration: '1h' as DurationString, | ||
| }; | ||
|
|
||
| export const ExecuteRangeQuery: React.FC<{ tool: ExecuteRangeQueryTool }> = ({ tool }) => { | ||
| const query = tool.args.query; | ||
| const definitions = [ | ||
| { | ||
| kind: 'PrometheusTimeSeriesQuery', | ||
| spec: { | ||
| query: query, | ||
| }, | ||
| }, | ||
| ]; | ||
|
|
||
| return ( | ||
| <> | ||
| <OlsToolUIPersesWrapper initialTimeRange={persesTimeRange}> | ||
| <DataQueriesProvider | ||
| definitions={definitions} | ||
| options={{ suggestedStepMs: 15000, mode: 'range' }} | ||
| > |
There was a problem hiding this comment.
execute_range_query arguments are only partially honored.
Line 16-37 hardcodes range (1h) and step (15000ms) instead of using tool-provided duration/step. This can render a chart that does not match the actual tool call.
✅ Suggested fix
type ExecuteRangeQueryTool = {
name: 'execute_range_query';
args: {
query: string;
+ duration?: DurationString;
+ step?: string;
};
};
-const persesTimeRange = {
- pastDuration: '1h' as DurationString,
-};
-
export const ExecuteRangeQuery: React.FC<{ tool: ExecuteRangeQueryTool }> = ({ tool }) => {
const query = tool.args.query;
+ const pastDuration = tool.args.duration ?? ('1h' as DurationString);
+ const suggestedStepMs = tool.args.step ? parseInt(tool.args.step, 10) * 1000 : 15000;
@@
- <OlsToolUIPersesWrapper initialTimeRange={persesTimeRange}>
+ <OlsToolUIPersesWrapper initialTimeRange={{ pastDuration }}>
<DataQueriesProvider
definitions={definitions}
- options={{ suggestedStepMs: 15000, mode: 'range' }}
+ options={{ suggestedStepMs, mode: 'range' }}
>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/ols-tool-ui/ExecuteRangeQuery.tsx` around lines 9 - 37,
The component ExecuteRangeQuery currently hardcodes persesTimeRange and
DataQueriesProvider step; update it to read duration and step from tool.args
(e.g., tool.args.duration and tool.args.step) and fall back to the existing
defaults ('1h' for duration and 15000 for step) so the UI matches the tool call;
replace persesTimeRange with a computed object using the provided duration and
pass the numeric step into DataQueriesProvider options (mode: 'range',
suggestedStepMs: step) and ensure definitions still use tool.args.query.
|
|
||
| interface OlsToolUIPersesWrapperProps { | ||
| children: React.ReactNode; | ||
| hight?: string; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n web/src/components/ols-tool-ui/OlsToolUIPersesWrapper.tsxRepository: openshift/monitoring-plugin
Length of output: 1789
Fix hight → height prop name typo.
Line 23 defines the interface prop as hight, but line 32 destructures height. This prevents the prop from being passed correctly to the component.
Fix
interface OlsToolUIPersesWrapperProps {
children: React.ReactNode;
- hight?: string;
+ height?: string;
initialTimeRange?: {
pastDuration: DurationString;
};
}📝 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.
| hight?: string; | |
| interface OlsToolUIPersesWrapperProps { | |
| children: React.ReactNode; | |
| height?: string; | |
| initialTimeRange?: { | |
| pastDuration: DurationString; | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/ols-tool-ui/OlsToolUIPersesWrapper.tsx` at line 23, The
interface in OlsToolUIPersesWrapper has a typo: change the prop name `hight` to
`height` in the props/interface declaration so it matches the destructured
`height` used in the OlsToolUIPersesWrapper component; update any references to
the interface/type (e.g., the props interface where `hight?: string;` is
declared) to use `height?: string;` to ensure the prop is passed correctly to
the component.
| }) => { | ||
| return ( | ||
| <QueryClientProvider client={queryClient}> | ||
| <PersesWrapper project={null}> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n web/src/components/ols-tool-ui/OlsToolUIPersesWrapper.tsx | head -50Repository: openshift/monitoring-plugin
Length of output: 1789
🏁 Script executed:
# Find PersesWrapper definition
rg -l "PersesWrapper" --type ts --type tsx | head -10Repository: openshift/monitoring-plugin
Length of output: 98
🏁 Script executed:
# Search for PersesWrapperProps type definition
rg "PersesWrapperProps" --type ts --type tsx -A 5Repository: openshift/monitoring-plugin
Length of output: 98
🏁 Script executed:
find web/src/components/dashboards/perses -name "*.tsx" -o -name "*.ts" | xargs cat -nRepository: openshift/monitoring-plugin
Length of output: 50384
🏁 Script executed:
# Try broader search with different approach
rg "PersesWrapper" -A 20 --max-count 5Repository: openshift/monitoring-plugin
Length of output: 14616
Update PersesWrapperProps.project type to accept null values.
The PersesWrapper component implementation (line 373) checks if (!project) and handles falsy values, yet the PersesWrapperProps.project type is defined as string rather than string | null. This creates a type mismatch with actual usage: OlsToolUIPersesWrapper.tsx passes null, and dashboard-frame.tsx passes activeProject which is typed as string | null.
Change the interface definition instead of the usage:
interface PersesWrapperProps {
children?: React.ReactNode;
- project: string;
+ project: string | null;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/ols-tool-ui/OlsToolUIPersesWrapper.tsx` at line 36, Update
the PersesWrapperProps interface so its project property accepts null (change
type from string to string | null) to match usage sites like
OlsToolUIPersesWrapper.tsx where <PersesWrapper project={null}> is passed and
the runtime check in PersesWrapper (the if (!project) branch); update any
related type declarations referencing PersesWrapperProps.project to string |
null to keep typing consistent with activeProject passed from
dashboard-frame.tsx.
| export const dashboardsOpened = (isOpened: boolean) => | ||
| action(ActionType.DashboardsOpened, { isOpened }); | ||
|
|
||
| export const dashboardsPersesPanelExternallyAdded = () => | ||
| action(ActionType.DashboardsPersesPanelExternallyAdded, {}); | ||
|
|
||
| export const dashboardsAddPersesPanelExternally = (panelDefinition: PanelDefinition) => | ||
| action(ActionType.DashboardsAddPersesPanelExternally, { panelDefinition }); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd web/src/store && wc -l actions.tsRepository: openshift/monitoring-plugin
Length of output: 85
🏁 Script executed:
cd web/src/store && head -100 actions.ts | tail -30Repository: openshift/monitoring-plugin
Length of output: 1135
🏁 Script executed:
cd web/src/store && sed -n '70,90p' actions.tsRepository: openshift/monitoring-plugin
Length of output: 894
🏁 Script executed:
cd web/src/store && grep -n "type Actions\|type ObserveAction" actions.ts | head -5Repository: openshift/monitoring-plugin
Length of output: 140
🏁 Script executed:
cd web/src/store && sed -n '210,240p' actions.tsRepository: openshift/monitoring-plugin
Length of output: 1909
🏁 Script executed:
cd web/src/store && sed -n '210,254p' actions.tsRepository: openshift/monitoring-plugin
Length of output: 2523
Actions type map is missing the three new dashboard action creators.
Lines 75-83 introduce dashboardsOpened, dashboardsPersesPanelExternallyAdded, and dashboardsAddPersesPanelExternally action creators, but they are not included in the Actions type definition (starting at line 210). This breaks type safety for reducers and dispatch calls using these actions.
Add the three missing entries to the Actions type:
dashboardsOpened: typeof dashboardsOpened;dashboardsPersesPanelExternallyAdded: typeof dashboardsPersesPanelExternallyAdded;dashboardsAddPersesPanelExternally: typeof dashboardsAddPersesPanelExternally;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/store/actions.ts` around lines 75 - 83, The Actions type map is
missing entries for the three new action creators; update the Actions
interface/type (the map around the Actions type definition) to include
dashboardsOpened: typeof dashboardsOpened, dashboardsPersesPanelExternallyAdded:
typeof dashboardsPersesPanelExternallyAdded, and
dashboardsAddPersesPanelExternally: typeof dashboardsAddPersesPanelExternally so
the new action creators (dashboardsOpened, dashboardsPersesPanelExternallyAdded,
dashboardsAddPersesPanelExternally) are represented for type-safe reducers and
dispatches.
| isOpened: boolean; | ||
| addPersesPanelExternally: PanelDefinition; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n -C2 'isOpened|addPersesPanelExternally' web/src/store/store.ts web/src/store/reducers.ts web/src/store/actions.tsRepository: openshift/monitoring-plugin
Length of output: 1650
🏁 Script executed:
rg -n -B5 -A15 'export const defaultObserveState' web/src/store/store.tsRepository: openshift/monitoring-plugin
Length of output: 703
State type is inconsistent with reducer behavior and defaults.
addPersesPanelExternally is typed as non-null PanelDefinition, but the reducer sets it to null (line 95 in reducers.ts), and defaultObserveState.dashboards does not initialize either isOpened or addPersesPanelExternally. This creates a TypeScript contract mismatch that will prevent compilation.
🐛 Proposed fix
export type ObserveState = {
dashboards: {
endTime: number | null;
pollInterval: number;
timespan: number;
variables: Record<string, Variable>;
isOpened: boolean;
- addPersesPanelExternally: PanelDefinition;
+ addPersesPanelExternally: PanelDefinition | null;
};
@@
export const defaultObserveState: ObserveState = {
dashboards: {
endTime: null,
pollInterval: 30 * 1000,
timespan: MONITORING_DASHBOARDS_DEFAULT_TIMESPAN,
variables: {},
+ isOpened: false,
+ addPersesPanelExternally: null,
},📝 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.
| isOpened: boolean; | |
| addPersesPanelExternally: PanelDefinition; | |
| export type ObserveState = { | |
| dashboards: { | |
| endTime: number | null; | |
| pollInterval: number; | |
| timespan: number; | |
| variables: Record<string, Variable>; | |
| isOpened: boolean; | |
| addPersesPanelExternally: PanelDefinition | null; | |
| }; | |
| // ... rest of the type definition | |
| }; | |
| export const defaultObserveState: ObserveState = { | |
| dashboards: { | |
| endTime: null, | |
| pollInterval: 30 * 1000, | |
| timespan: MONITORING_DASHBOARDS_DEFAULT_TIMESPAN, | |
| variables: {}, | |
| isOpened: false, | |
| addPersesPanelExternally: null, | |
| }, | |
| // ... rest of the default state | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/store/store.ts` around lines 31 - 32, The state definition is
inconsistent: change addPersesPanelExternally to allow null (PanelDefinition |
null) and ensure defaultObserveState.dashboards initializes both isOpened
(boolean, likely false) and addPersesPanelExternally (null) to match reducers.ts
which sets addPersesPanelExternally to null; update the type in store.ts and the
defaultObserveState.dashboards object so the type contract and reducer behavior
align with the PanelDefinition symbol and isOpened flag.
1 participant
Provides a hook to visualize an mcp-tool call with Perses elements.
It also adds an ability to add the panel from the OLS chat into opened dashboards.
See openshift/lightspeed-console#1576 for more details.
To test the
ExecuteRangeQuerylocally, one can feed the following sample data:Depends on:
Summary by CodeRabbit
Release Notes
New Features
Refactor