Open
Conversation
…uests and limits.
Contributor
📝 WalkthroughWalkthroughAdds UI and API support for editing agent resource configurations (replicas, CPU/memory requests & limits, autoscaling) via a new drawer; introduces ResourceMetricChip UI; relaxes metrics request shape and adds auto-refresh; wires hooks, types, and API client exports; multiple deploy UI refactors and label copy updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User / UI
participant DeployCard as DeployCard
participant Drawer as EditResourceConfigsDrawer
participant Hook as useUpdateAgentResourceConfigs
participant API as API Client
participant Backend as Backend
User->>DeployCard: Click "Configure"/"Manage"
DeployCard->>Drawer: open(resourceConfigs)
User->>Drawer: Edit form (replicas, cpu/memory, autoscaling)
User->>Drawer: Click "Save"
Drawer->>Hook: mutate({ params, body, query })
Hook->>API: updateAgentResourceConfigs(params, body, query)
API->>Backend: PUT /resource-configs (auth if provided)
Backend-->>API: 200 + body
API-->>Hook: return parsed response
Hook->>DeployCard: invalidate/refetch queries
DeployCard->>DeployCard: re-render with updated configs
sequenceDiagram
participant Component as DeployCard
participant MetricsHook as useGetAgentMetrics
participant API as Metrics API
participant Backend as Backend
participant Timer as Refetch Timer
Component->>MetricsHook: init (enableAutoRefresh=true)
MetricsHook->>API: getAgentMetrics(body with auto-populated times)
API->>Backend: POST /metrics
Backend-->>API: metrics data
API-->>MetricsHook: return parsed data
MetricsHook->>Component: update cache
Timer->>MetricsHook: interval (SLOW_POLL_INTERVAL rgba(0,0,0,0.5))
MetricsHook->>API: refetch metrics
API->>Backend: POST /metrics
Backend-->>API: updated metrics
API-->>MetricsHook: return new data
MetricsHook->>Component: update UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 4❌ Failed checks (2 warnings, 2 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (8)
console/workspaces/pages/llm-providers/src/index.ts (1)
25-26: Consider updating metadata description to match the new title terminology.Line 25 uses “LLM Service Providers”, while Line 26 still says “LLM Provider management”. Aligning both improves consistency in page metadata.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@console/workspaces/pages/llm-providers/src/index.ts` around lines 25 - 26, Update the page metadata so the description matches the new title terminology: change the description string used alongside the metadata key description in index.ts to reference "LLM Service Providers" (or otherwise mirror the phrasing used in the title) so title: 'LLM Service Providers' and description are consistent; locate the metadata object in index.ts where title and description are defined and adjust the description text accordingly.console/workspaces/pages/overview/src/AddLLMProvider.Component.tsx (1)
599-599: Unify wording across this screen after the header rename.Line 599 now says “LLM Service Provider”, but adjacent strings still use “LLM Provider” / “Provider” (for example, page title and selection button). Consider standardizing all visible labels in this component to avoid mixed terminology.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@console/workspaces/pages/overview/src/AddLLMProvider.Component.tsx` at line 599, The header was renamed to "LLM Service Provider" but other visible strings in AddLLMProvider.Component.tsx still use "LLM Provider"/"Provider"; search this component for all occurrences of those labels (including Form.Header, the page title string, the selection button text/aria-label, any placeholders or helper text) and replace them with the unified wording "LLM Service Provider" (or "Service Provider" where shorter contextually required) so all user-facing labels match; update any related accessibility labels and tests referencing these exact strings (e.g., the selection button label and pageTitle variable) to the same phrasing.deployments/docker-compose.yml (1)
109-110: Make Guardrails endpoints overridable in local runs.These tenant-specific external URLs are hardcoded into shared dev compose. Prefer
${VAR:-default}(or.env) so contributors can override without editing the file.♻️ Suggested compose tweak
- - GUARDRAILS_CATALOG_URL=https://db720294-98fd-40f4-85a1-cc6a3b65bc9a-prod.e1-us-east-azure.choreoapis.dev/api-platform/policy-hub-api/policy-hub-public/v1.0/policies?categories=Guardrails - - GUARDRAILS_DEFINITION_BASE_URL=https://db720294-98fd-40f4-85a1-cc6a3b65bc9a-prod.e1-us-east-azure.choreoapis.dev/api-platform/policy-hub-api/policy-hub-public/v1.0/policies + - GUARDRAILS_CATALOG_URL=${GUARDRAILS_CATALOG_URL:-https://db720294-98fd-40f4-85a1-cc6a3b65bc9a-prod.e1-us-east-azure.choreoapis.dev/api-platform/policy-hub-api/policy-hub-public/v1.0/policies?categories=Guardrails} + - GUARDRAILS_DEFINITION_BASE_URL=${GUARDRAILS_DEFINITION_BASE_URL:-https://db720294-98fd-40f4-85a1-cc6a3b65bc9a-prod.e1-us-east-azure.choreoapis.dev/api-platform/policy-hub-api/policy-hub-public/v1.0/policies}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@deployments/docker-compose.yml` around lines 109 - 110, Replace the hardcoded Guardrails URLs with overridable environment variable expansions so local contributors can change them without editing the compose file: update the GUARDRAILS_CATALOG_URL and GUARDRAILS_DEFINITION_BASE_URL entries to use shell-style defaults (e.g. ${GUARDRAILS_CATALOG_URL:-https://...} and ${GUARDRAILS_DEFINITION_BASE_URL:-https://...}) or rely on .env values, keeping the same default URLs if the variables are unset; ensure the exact variable names GUARDRAILS_CATALOG_URL and GUARDRAILS_DEFINITION_BASE_URL are used so existing code reading those vars continues to work.console/workspaces/pages/deploy/src/subComponent/BuildSelectorDrawer.tsx (1)
96-116: Replace hardcoded style literals with theme-tokenizedsx.At Line 100 and Line 113, fixed values (
minHeight={200},border: "1px solid") should use token-friendlysxstyles.As per coding guidelines, "Use theme tokens via the `sx` prop instead of hardcoded colors and spacing values".Proposed tokenized style update
- <Box - display="flex" - justifyContent="center" - alignItems="center" - minHeight={200} - > + <Box + display="flex" + justifyContent="center" + alignItems="center" + sx={{ minHeight: (theme) => theme.spacing(25) }} + > ... - sx={{ - border: "1px solid", - borderRadius: 1, - borderColor: "divider", - }} + sx={{ + border: 1, + borderRadius: 1, + borderColor: "divider", + }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@console/workspaces/pages/deploy/src/subComponent/BuildSelectorDrawer.tsx` around lines 96 - 116, Replace hardcoded layout and border literals in BuildSelectorDrawer: switch the Box prop minHeight={200} to use sx with theme spacing (e.g., minHeight: (theme) => theme.spacing(<n>) or a responsive token) and replace ListItem's border: "1px solid" and borderColor: "divider" with sx tokens such as borderWidth/borderStyle/borderColor using theme values (e.g., border: 1, borderStyle: 'solid', borderColor: 'divider' or border: (theme) => `${theme.shape.borderWidth}px solid ${theme.palette.divider}`). Update the JSX where builds.map renders the Box and ListItem (identify by the Box element rendering "No builds available" and the ListItem returned inside builds.map) to only use sx tokenized values instead of hardcoded numbers/strings.console/workspaces/libs/api-client/src/apis/metrics.ts (1)
38-43: Use a singlenowvalue when defaulting metric times.At Lines 39-42, computing
new Date()twice makes the window slightly non-deterministic and harder to read. Prefer a single timestamp.Proposed cleanup
- if (!bodyClone?.endTime){ - bodyClone.endTime = new Date().toISOString() - } - if (!bodyClone?.startTime){ - bodyClone.startTime = new Date(new Date().getTime() - 1000 * 10).toISOString() - } + const now = Date.now(); + if (!bodyClone?.endTime) { + bodyClone.endTime = new Date(now).toISOString(); + } + if (!bodyClone?.startTime) { + bodyClone.startTime = new Date(now - 10_000).toISOString(); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@console/workspaces/libs/api-client/src/apis/metrics.ts` around lines 38 - 43, Summary: Use a single "now" timestamp to set default metric times to avoid tiny inconsistencies. Fix: compute a single Date instance (e.g., const now = new Date()) once, then if bodyClone.endTime is missing set bodyClone.endTime = now.toISOString(), and if bodyClone.startTime is missing set bodyClone.startTime = new Date(now.getTime() - 1000 * 10).toISOString(); reference the identifiers bodyClone.endTime and bodyClone.startTime so you only call new Date() once and reuse that value.console/workspaces/pages/deploy/src/Deploy.Component.tsx (1)
38-38: Prefer theme-token styling over fixed sizing values.At Line 38,
width={32}andheight={2}are fixed values; move these intosxwith theme tokens for consistency.As per coding guidelines, "Use theme tokens via the
sxprop instead of hardcoded colors and spacing values".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@console/workspaces/pages/deploy/src/Deploy.Component.tsx` at line 38, The Box instance rendering the divider (Box with width={32} height={2} mt={14}) should stop using hardcoded width/height props and instead supply them via the sx prop using theme tokens; update the Box in Deploy.Component.tsx to remove the width and height props and add sx that maps those sizes to theme spacing or size tokens (for example sx={{ width: theme => theme.spacing(4), height: theme => theme.spacing(0.5), mt: theme => theme.spacing(14) }} or equivalent token values) so the divider uses theme-based sizing rather than fixed numbers.console/workspaces/libs/api-client/src/apis/resource-configs.ts (1)
75-75:cloneDeepappears unnecessary here.The
bodyobject is serialized to JSON byhttpPUTand isn't mutated. Deep cloning adds overhead without benefit. If mutation protection is needed for the caller, that responsibility should be at the call site.Suggested simplification
- const res = await httpPUT(baseUrl, cloneDeep(body), { + const res = await httpPUT(baseUrl, body, {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@console/workspaces/libs/api-client/src/apis/resource-configs.ts` at line 75, The call to cloneDeep when invoking httpPUT (const res = await httpPUT(baseUrl, cloneDeep(body), {...})) is unnecessary because httpPUT JSON-serializes the payload and does not mutate its argument; remove cloneDeep and pass body directly to httpPUT (replace cloneDeep(body) with body) to eliminate needless deep-cloning overhead and keep mutation protection responsibility at the caller side; update any nearby usage of res, baseUrl, or body if they depend on that clone.console/workspaces/pages/deploy/src/subComponent/EditResourceConfigsDrawer.tsx (1)
224-233:isValidduplicates schema validation logic, risking drift.This manual validation mirrors
resourceConfigsSchemabut could diverge if the schema is updated. Consider deriving validity directly fromvalidateFormor checkingObject.values(errors).every((e) => !e)after an initial full validation.Suggested approach using schema-based validation
+ const isValid = useMemo(() => { + const result = resourceConfigsSchema.safeParse(formData); + return result.success && Object.values(errors).every((e) => !e); + }, [formData, errors]); - const isValid = - formData.cpuRequest.trim().length > 0 && - formData.memoryRequest.trim().length > 0 && - formData.replicas >= 0 && - formData.replicas <= 10 && - (!formData.autoScalingEnabled || - ((formData.minReplicas ?? 1) <= (formData.maxReplicas ?? 3) && - (formData.minReplicas ?? 1) >= 1 && - (formData.maxReplicas ?? 3) >= 1)) && - Object.values(errors).every((e) => !e);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@console/workspaces/pages/deploy/src/subComponent/EditResourceConfigsDrawer.tsx` around lines 224 - 233, The inline isValid boolean duplicates the resourceConfigsSchema logic and can drift; replace it by using the schema/validation routine instead: run validateForm (or use resourceConfigsSchema.safeParse(formData)) to perform full validation, update the errors state from that result, and set isValid based solely on Object.values(errors).every((e) => !e) (or the safeParse success flag) rather than duplicating checks against formData.replicas, cpuRequest, memoryRequest, and autoscaling fields; update references to isValid, validateForm, resourceConfigsSchema, formData, and errors accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@console/workspaces/libs/api-client/src/apis/resource-configs.ts`:
- Around line 57-58: The current error handling in the res.ok checks (where the
code does `if (!res.ok) throw await res.json(); return res.json();`) assumes the
response body is valid JSON and will throw if it's not; change both occurrences
to attempt parsing JSON inside a try/catch: on non-ok responses try `await
res.json()` and if parsing fails fall back to `await res.text()` (or a default
message) and then throw an Error/object that includes res.status, res.statusText
and the parsed/fallback body; do the same for the successful return path: try to
return parsed JSON, but if `res.json()` throws, return the text or throw a
descriptive error including response metadata so callers get meaningful
information (update the functions that call `res.json()` in resource-configs.ts
accordingly).
In
`@console/workspaces/libs/shared-component/src/components/DeploymentConfig.tsx`:
- Around line 100-105: The useEffect initializes form state by calling
setEnvVariables with configurations?.configurations?.sort(...), but
Array.prototype.sort mutates the original query-derived array
(configurations.configurations) causing cache/state bleed; fix it by cloning
before sorting (e.g., create a shallow copy of configurations?.configurations
via [...configurations?.configurations] or .slice() and then sort) so update in
the useEffect (the setEnvVariables call inside the useEffect that references
configurations?.configurations and the sort) operates on a copied array rather
than mutating the query cache.
In `@console/workspaces/pages/deploy/src/Deploy.Component.tsx`:
- Around line 37-40: The mapped sibling elements use the shorthand fragment
syntax (<>...</>) so the key on DeployCard isn't applied to the fragment root;
update the mapping in Deploy.Component.tsx to wrap each pair with an explicit
fragment that carries the key (e.g. React.Fragment or imported Fragment) using
key={env.name}, and remove the redundant key prop from the DeployCard component;
ensure the file imports Fragment if using the named import or reference
React.Fragment where the fragment is used.
In `@console/workspaces/pages/deploy/src/subComponent/DeployCard.tsx`:
- Around line 485-493: The EditResourceConfigsDrawer is being passed
agentName={agentId ?? ""} which can lead to buildBaseUrl throwing "agentName is
required" when agentId is empty; either assert the router guarantee by passing
agentId! (use non-null assertion) to EditResourceConfigsDrawer or only
render/mount EditResourceConfigsDrawer when agentId is truthy (e.g., guard with
if (agentId) before rendering) so buildBaseUrl receives a valid agentName;
update the usage of agentId in the DeployCard component accordingly to reference
the EditResourceConfigsDrawer prop and the buildBaseUrl call paths.
---
Nitpick comments:
In `@console/workspaces/libs/api-client/src/apis/metrics.ts`:
- Around line 38-43: Summary: Use a single "now" timestamp to set default metric
times to avoid tiny inconsistencies. Fix: compute a single Date instance (e.g.,
const now = new Date()) once, then if bodyClone.endTime is missing set
bodyClone.endTime = now.toISOString(), and if bodyClone.startTime is missing set
bodyClone.startTime = new Date(now.getTime() - 1000 * 10).toISOString();
reference the identifiers bodyClone.endTime and bodyClone.startTime so you only
call new Date() once and reuse that value.
In `@console/workspaces/libs/api-client/src/apis/resource-configs.ts`:
- Line 75: The call to cloneDeep when invoking httpPUT (const res = await
httpPUT(baseUrl, cloneDeep(body), {...})) is unnecessary because httpPUT
JSON-serializes the payload and does not mutate its argument; remove cloneDeep
and pass body directly to httpPUT (replace cloneDeep(body) with body) to
eliminate needless deep-cloning overhead and keep mutation protection
responsibility at the caller side; update any nearby usage of res, baseUrl, or
body if they depend on that clone.
In `@console/workspaces/pages/deploy/src/Deploy.Component.tsx`:
- Line 38: The Box instance rendering the divider (Box with width={32}
height={2} mt={14}) should stop using hardcoded width/height props and instead
supply them via the sx prop using theme tokens; update the Box in
Deploy.Component.tsx to remove the width and height props and add sx that maps
those sizes to theme spacing or size tokens (for example sx={{ width: theme =>
theme.spacing(4), height: theme => theme.spacing(0.5), mt: theme =>
theme.spacing(14) }} or equivalent token values) so the divider uses theme-based
sizing rather than fixed numbers.
In `@console/workspaces/pages/deploy/src/subComponent/BuildSelectorDrawer.tsx`:
- Around line 96-116: Replace hardcoded layout and border literals in
BuildSelectorDrawer: switch the Box prop minHeight={200} to use sx with theme
spacing (e.g., minHeight: (theme) => theme.spacing(<n>) or a responsive token)
and replace ListItem's border: "1px solid" and borderColor: "divider" with sx
tokens such as borderWidth/borderStyle/borderColor using theme values (e.g.,
border: 1, borderStyle: 'solid', borderColor: 'divider' or border: (theme) =>
`${theme.shape.borderWidth}px solid ${theme.palette.divider}`). Update the JSX
where builds.map renders the Box and ListItem (identify by the Box element
rendering "No builds available" and the ListItem returned inside builds.map) to
only use sx tokenized values instead of hardcoded numbers/strings.
In
`@console/workspaces/pages/deploy/src/subComponent/EditResourceConfigsDrawer.tsx`:
- Around line 224-233: The inline isValid boolean duplicates the
resourceConfigsSchema logic and can drift; replace it by using the
schema/validation routine instead: run validateForm (or use
resourceConfigsSchema.safeParse(formData)) to perform full validation, update
the errors state from that result, and set isValid based solely on
Object.values(errors).every((e) => !e) (or the safeParse success flag) rather
than duplicating checks against formData.replicas, cpuRequest, memoryRequest,
and autoscaling fields; update references to isValid, validateForm,
resourceConfigsSchema, formData, and errors accordingly.
In `@console/workspaces/pages/llm-providers/src/index.ts`:
- Around line 25-26: Update the page metadata so the description matches the new
title terminology: change the description string used alongside the metadata key
description in index.ts to reference "LLM Service Providers" (or otherwise
mirror the phrasing used in the title) so title: 'LLM Service Providers' and
description are consistent; locate the metadata object in index.ts where title
and description are defined and adjust the description text accordingly.
In `@console/workspaces/pages/overview/src/AddLLMProvider.Component.tsx`:
- Line 599: The header was renamed to "LLM Service Provider" but other visible
strings in AddLLMProvider.Component.tsx still use "LLM Provider"/"Provider";
search this component for all occurrences of those labels (including
Form.Header, the page title string, the selection button text/aria-label, any
placeholders or helper text) and replace them with the unified wording "LLM
Service Provider" (or "Service Provider" where shorter contextually required) so
all user-facing labels match; update any related accessibility labels and tests
referencing these exact strings (e.g., the selection button label and pageTitle
variable) to the same phrasing.
In `@deployments/docker-compose.yml`:
- Around line 109-110: Replace the hardcoded Guardrails URLs with overridable
environment variable expansions so local contributors can change them without
editing the compose file: update the GUARDRAILS_CATALOG_URL and
GUARDRAILS_DEFINITION_BASE_URL entries to use shell-style defaults (e.g.
${GUARDRAILS_CATALOG_URL:-https://...} and
${GUARDRAILS_DEFINITION_BASE_URL:-https://...}) or rely on .env values, keeping
the same default URLs if the variables are unset; ensure the exact variable
names GUARDRAILS_CATALOG_URL and GUARDRAILS_DEFINITION_BASE_URL are used so
existing code reading those vars continues to work.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 798becb9-db58-4160-8f01-61adb7de753c
📒 Files selected for processing (27)
console/apps/webapp/src/Layouts/OxygenLayout/LeftNavigation.tsxconsole/workspaces/libs/api-client/src/apis/index.tsconsole/workspaces/libs/api-client/src/apis/metrics.tsconsole/workspaces/libs/api-client/src/apis/resource-configs.tsconsole/workspaces/libs/api-client/src/hooks/index.tsconsole/workspaces/libs/api-client/src/hooks/metrics.tsconsole/workspaces/libs/api-client/src/hooks/resource-configs.tsconsole/workspaces/libs/api-client/src/utils/utils.tsconsole/workspaces/libs/shared-component/src/components/DeploymentConfig.tsxconsole/workspaces/libs/shared-component/src/components/EnvironmentCard/EnvironmentCard.tsxconsole/workspaces/libs/shared-component/src/components/EnvironmentVariable.tsxconsole/workspaces/libs/shared-component/src/components/ResourceMetricChip.tsxconsole/workspaces/libs/shared-component/src/components/index.tsconsole/workspaces/libs/types/src/api/index.tsconsole/workspaces/libs/types/src/api/metrics.tsconsole/workspaces/libs/types/src/api/resource-configs.tsconsole/workspaces/libs/views/src/component/PageLayout/PageErrorBoundary.tsxconsole/workspaces/pages/deploy/src/Deploy.Component.tsxconsole/workspaces/pages/deploy/src/subComponent/BuildCard.tsxconsole/workspaces/pages/deploy/src/subComponent/BuildSelectorDrawer.tsxconsole/workspaces/pages/deploy/src/subComponent/DeployCard.tsxconsole/workspaces/pages/deploy/src/subComponent/EditResourceConfigsDrawer.tsxconsole/workspaces/pages/llm-providers/src/index.tsconsole/workspaces/pages/llm-providers/src/subComponents/LLMProviderTable.tsxconsole/workspaces/pages/overview/src/AddLLMProvider.Component.tsxconsole/workspaces/pages/overview/src/Configure/subComponents/AgentLLMProvidersSection.tsxdeployments/docker-compose.yml
console/workspaces/libs/api-client/src/apis/resource-configs.ts
Outdated
Show resolved
Hide resolved
console/workspaces/libs/shared-component/src/components/DeploymentConfig.tsx
Show resolved
Hide resolved
console/workspaces/pages/deploy/src/subComponent/DeployCard.tsx
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@console/workspaces/libs/api-client/src/apis/metrics.ts`:
- Around line 38-43: The fallback startTime should be derived from the effective
endTime rather than from now; update the logic that sets bodyClone.startTime
(and the nearby now variable) so that when bodyClone.endTime exists you parse it
into a Date and subtract 10 seconds from that parsed endTime (otherwise fall
back to using now), then assign bodyClone.startTime =
parsedEndTimeMinus10s.toISOString(); ensure you handle endTime being a string by
creating a Date(bodyClone.endTime) and only subtracting when startTime is
missing.
In `@console/workspaces/pages/deploy/src/subComponent/DeployCard.tsx`:
- Around line 246-282: The metrics query is currently auto-refreshing for every
card even when the environment isn't active; update the useGetAgentMetrics call
so its enabled flag also checks that the environment is active (e.g. ensure
currentDeployment?.state === 'ACTIVE' or equivalent) in addition to the existing
orgId/projectId/agentId/currentEnvironment.name checks, so metrics polling only
runs for the active environment displayed in the `ACTIVE` collapse; locate the
currentDeployment variable and the useGetAgentMetrics invocation to add this
condition to the enabled option.
In
`@console/workspaces/pages/deploy/src/subComponent/EditResourceConfigsDrawer.tsx`:
- Around line 151-160: The component currently returns null when resourceConfigs
is missing, making the parent card's "Configure" action a no-op; instead, change
EditResourceConfigsDrawer (and the render path around the early return that
checks resourceConfigs) to render the drawer with editable fields initialized
from the existing formData defaults so users can create initial settings; locate
the resourceConfigs usage and remove the early return, ensure the form state
(formData and setFormData) is used to populate inputs when resourceConfigs is
undefined, and wire the existing save/submit handler (e.g., handleSave/onSave)
to create the resource configs on submit so first-time configuration works.
- Around line 46-87: The schema currently only checks syntax so values like
"0m"/"0Mi" and limits smaller than requests slip through; add parseCpuQuantity
and parseMemoryQuantity helpers (use millicores for CPU and bytes for memory)
and call them from resourceConfigsSchema.refine checks to (1) reject zero
quantities for cpuRequest and memoryRequest, and (2) ensure when cpuLimit or
memoryLimit are provided they are >= the corresponding request; update the
refine logic for cpuRequest/cpuLimit and memoryRequest/memoryLimit using the new
helpers and reference the fields cpuRequest, cpuLimit, memoryRequest,
memoryLimit and the schema name resourceConfigsSchema.
- Around line 191-196: The submit handler currently validates formData with
resourceConfigsSchema.safeParse() but then forwards the original formData to
toRequestPayload(), losing transformations like .trim(); update handleSubmit to
use the parsed/transformed value returned by safeParse (or have validateForm
return the parsed data) and pass that parsed object into toRequestPayload
instead of the raw formData—look for handleSubmit, validateForm,
resourceConfigsSchema.safeParse, toRequestPayload, and formData to implement
this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cc07fa6b-4161-4f76-8ccb-5b5f4370cc41
📒 Files selected for processing (12)
console/workspaces/libs/api-client/src/apis/metrics.tsconsole/workspaces/libs/api-client/src/apis/resource-configs.tsconsole/workspaces/libs/shared-component/src/components/DeploymentConfig.tsxconsole/workspaces/libs/shared-component/src/components/ResourceMetricChip.tsxconsole/workspaces/pages/deploy/src/Deploy.Component.tsxconsole/workspaces/pages/deploy/src/subComponent/BuildCard.tsxconsole/workspaces/pages/deploy/src/subComponent/BuildSelectorDrawer.tsxconsole/workspaces/pages/deploy/src/subComponent/DeployCard.tsxconsole/workspaces/pages/deploy/src/subComponent/EditResourceConfigsDrawer.tsxconsole/workspaces/pages/llm-providers/src/index.tsconsole/workspaces/pages/metrics/src/components/MetricsView/MetricsView.tsxconsole/workspaces/pages/overview/src/AddLLMProvider.Component.tsx
✅ Files skipped from review due to trivial changes (4)
- console/workspaces/pages/deploy/src/subComponent/BuildCard.tsx
- console/workspaces/pages/metrics/src/components/MetricsView/MetricsView.tsx
- console/workspaces/pages/overview/src/AddLLMProvider.Component.tsx
- console/workspaces/pages/llm-providers/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- console/workspaces/pages/deploy/src/Deploy.Component.tsx
- console/workspaces/libs/shared-component/src/components/DeploymentConfig.tsx
- console/workspaces/libs/api-client/src/apis/resource-configs.ts
- console/workspaces/libs/shared-component/src/components/ResourceMetricChip.tsx
Comment on lines
+38
to
+43
| const now = new Date(); | ||
| if (!bodyClone?.endTime) { | ||
| bodyClone.endTime = now.toISOString(); | ||
| } | ||
| if (!bodyClone?.startTime) { | ||
| bodyClone.startTime = new Date(now.getTime() - 1000 * 10).toISOString(); |
Contributor
There was a problem hiding this comment.
Derive the fallback startTime from the effective endTime.
Now that timestamps are optional, the “only endTime provided” case is part of the supported surface. Line 43 currently subtracts 10 seconds from now, not from bodyClone.endTime, so historical queries can send the wrong window.
Suggested fix
if (!bodyClone?.endTime) {
bodyClone.endTime = now.toISOString();
}
+ const effectiveEndTime = new Date(bodyClone.endTime);
if (!bodyClone?.startTime) {
- bodyClone.startTime = new Date(now.getTime() - 1000 * 10).toISOString();
+ bodyClone.startTime = new Date(
+ effectiveEndTime.getTime() - 1000 * 10
+ ).toISOString();
}📝 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.
Suggested change
| const now = new Date(); | |
| if (!bodyClone?.endTime) { | |
| bodyClone.endTime = now.toISOString(); | |
| } | |
| if (!bodyClone?.startTime) { | |
| bodyClone.startTime = new Date(now.getTime() - 1000 * 10).toISOString(); | |
| const now = new Date(); | |
| if (!bodyClone?.endTime) { | |
| bodyClone.endTime = now.toISOString(); | |
| } | |
| const effectiveEndTime = new Date(bodyClone.endTime); | |
| if (!bodyClone?.startTime) { | |
| bodyClone.startTime = new Date( | |
| effectiveEndTime.getTime() - 1000 * 10 | |
| ).toISOString(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@console/workspaces/libs/api-client/src/apis/metrics.ts` around lines 38 - 43,
The fallback startTime should be derived from the effective endTime rather than
from now; update the logic that sets bodyClone.startTime (and the nearby now
variable) so that when bodyClone.endTime exists you parse it into a Date and
subtract 10 seconds from that parsed endTime (otherwise fall back to using now),
then assign bodyClone.startTime = parsedEndTimeMinus10s.toISOString(); ensure
you handle endTime being a string by creating a Date(bodyClone.endTime) and only
subtracting when startTime is missing.
console/workspaces/pages/deploy/src/subComponent/DeployCard.tsx
Outdated
Show resolved
Hide resolved
console/workspaces/pages/deploy/src/subComponent/EditResourceConfigsDrawer.tsx
Show resolved
Hide resolved
Comment on lines
+151
to
+160
| const [formData, setFormData] = useState<ResourceConfigsFormValues>({ | ||
| replicas: 1, | ||
| cpuRequest: "500m", | ||
| memoryRequest: "512Mi", | ||
| cpuLimit: "", | ||
| memoryLimit: "", | ||
| autoScalingEnabled: false, | ||
| minReplicas: 1, | ||
| maxReplicas: 3, | ||
| }); |
Contributor
There was a problem hiding this comment.
Don't turn the empty-state "Configure" action into a no-op.
Line 226 returns null whenever resourceConfigs is missing. The parent card still exposes “Configure” from the no-config state, so first-time resource settings cannot be created from this flow.
Suggested fix
+const DEFAULT_FORM_DATA: ResourceConfigsFormValues = {
+ replicas: 1,
+ cpuRequest: "500m",
+ memoryRequest: "512Mi",
+ cpuLimit: "",
+ memoryLimit: "",
+ autoScalingEnabled: false,
+ minReplicas: 1,
+ maxReplicas: 3,
+};
+
export function EditResourceConfigsDrawer({
...
}: EditResourceConfigsDrawerProps) {
- const [formData, setFormData] = useState<ResourceConfigsFormValues>({
- replicas: 1,
- cpuRequest: "500m",
- memoryRequest: "512Mi",
- cpuLimit: "",
- memoryLimit: "",
- autoScalingEnabled: false,
- minReplicas: 1,
- maxReplicas: 3,
- });
+ const [formData, setFormData] =
+ useState<ResourceConfigsFormValues>(DEFAULT_FORM_DATA);
useEffect(() => {
- if (open && resourceConfigs) {
- setFormData(toFormValues(resourceConfigs));
+ if (open) {
+ setFormData(
+ resourceConfigs ? toFormValues(resourceConfigs) : DEFAULT_FORM_DATA
+ );
clearErrors();
}
}, [open, resourceConfigs, clearErrors]);
-
- if (!resourceConfigs) return null;Also applies to: 172-177, 226-226
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@console/workspaces/pages/deploy/src/subComponent/EditResourceConfigsDrawer.tsx`
around lines 151 - 160, The component currently returns null when
resourceConfigs is missing, making the parent card's "Configure" action a no-op;
instead, change EditResourceConfigsDrawer (and the render path around the early
return that checks resourceConfigs) to render the drawer with editable fields
initialized from the existing formData defaults so users can create initial
settings; locate the resourceConfigs usage and remove the early return, ensure
the form state (formData and setFormData) is used to populate inputs when
resourceConfigs is undefined, and wire the existing save/submit handler (e.g.,
handleSave/onSave) to create the resource configs on submit so first-time
configuration works.
Comment on lines
+191
to
+196
| const handleSubmit = useCallback( | ||
| (e: React.FormEvent) => { | ||
| e.preventDefault(); | ||
| if (!validateForm(formData)) return; | ||
|
|
||
| const payload = toRequestPayload(formData); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Zod, does z.string().trim()mutate the original input value, or does it only affect the parsed output returned byparse/safeParse?
💡 Result:
In Zod, z.string.trim does not mutate the original input value. It only affects the parsed output returned by parse or safeParse. Zod's parse and safeParse methods return a new value (a deep clone or reconstructed object) with transformations like trimming applied to it. The original input remains unchanged. This behavior aligns with Zod's design philosophy of strict separation between validation and mutation, as noted in GitHub issues and discussions. Built-in methods like .trim add checks that transform during parsing without altering the input. Official docs list .trim under string transforms, confirming it modifies the output. Older Zod versions (e.g., pre-v3) had rare mutation bugs (e.g., adding times property), but these are fixed. Current stable Zod 4 (as of 2026) consistently produces a new output. Example: const schema = z.string.trim; const input = " hello "; const output = schema.parse(input); // output === "hello" console.log(input === " hello "); // true (original unchanged)
Citations:
- 1: Feature request: First class string trimming support? colinhacks/zod#653
- 2: https://v4.zod.dev/api?id=strings
- 3: https://zodjs.netlify.app/guide/strings
- 4: Zod transformations understanding colinhacks/zod#5734
- 5: https://zod.dev/?id=parse
- 6: https://zod.dev/?id=safeparse
- 7: https://v4.zod.dev/codecs?id=type-safe-inputs
- 8: https://v4.zod.dev/basics?id=parsing-data
🏁 Script executed:
fd "EditResourceConfigsDrawer" --type fRepository: wso2/agent-manager
Length of output: 140
🏁 Script executed:
cat -n console/workspaces/pages/deploy/src/subComponent/EditResourceConfigsDrawer.tsx | head -220 | tail -50Repository: wso2/agent-manager
Length of output: 1615
🏁 Script executed:
cat -n console/workspaces/pages/deploy/src/subComponent/EditResourceConfigsDrawer.tsx | head -100Repository: wso2/agent-manager
Length of output: 4020
🏁 Script executed:
cat -n console/workspaces/pages/deploy/src/subComponent/EditResourceConfigsDrawer.tsx | head -170 | tail -70Repository: wso2/agent-manager
Length of output: 2180
🏁 Script executed:
fd "useFormValidation" --type fRepository: wso2/agent-manager
Length of output: 122
🏁 Script executed:
cat -n console/workspaces/libs/views/src/hooks/useFormValidation.tsRepository: wso2/agent-manager
Length of output: 3729
Use the parsed and trimmed values from schema validation.
Line 194 validates formData through resourceConfigsSchema.safeParse(), which applies transformations like .trim() to string fields. However, Zod's .trim() only affects the parsed output and does not mutate the original input. Line 196 then sends the raw formData to toRequestPayload(), meaning whitespace-normalized fields are submitted with their original spaces despite validation success.
Suggested fix
const handleSubmit = useCallback(
(e: React.FormEvent) => {
e.preventDefault();
- if (!validateForm(formData)) return;
+ const parsed = resourceConfigsSchema.safeParse(formData);
+ if (!parsed.success) {
+ validateForm(formData);
+ return;
+ }
- const payload = toRequestPayload(formData);
+ const payload = toRequestPayload(parsed.data);
updateConfigs(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@console/workspaces/pages/deploy/src/subComponent/EditResourceConfigsDrawer.tsx`
around lines 191 - 196, The submit handler currently validates formData with
resourceConfigsSchema.safeParse() but then forwards the original formData to
toRequestPayload(), losing transformations like .trim(); update handleSubmit to
use the parsed/transformed value returned by safeParse (or have validateForm
return the parsed data) and pass that parsed object into toRequestPayload
instead of the raw formData—look for handleSubmit, validateForm,
resourceConfigsSchema.safeParse, toRequestPayload, and formData to implement
this change.
rasika2012
force-pushed
the
resource-manage-view
branch
from
b548d42 to
9db63d3
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
console/workspaces/pages/deploy/src/subComponent/EditResourceConfigsDrawer.tsx (2)
241-246:⚠️ Potential issue | 🟠 MajorSubmit sends raw form data instead of Zod-transformed values.
validateForm(formData)validates but doesn't return the parsed output. Line 246 then passes the originalformDatatotoRequestPayload, losing.trim()transformations applied by the schema.🐛 Use parsed values from safeParse
const handleSubmit = useCallback( (e: React.FormEvent) => { e.preventDefault(); - if (!validateForm(formData)) return; - - const payload = toRequestPayload(formData); + const parsed = resourceConfigsSchema.safeParse(formData); + if (!parsed.success) { + validateForm(formData); // populate error state + return; + } + + const payload = toRequestPayload(parsed.data); updateConfigs(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@console/workspaces/pages/deploy/src/subComponent/EditResourceConfigsDrawer.tsx` around lines 241 - 246, The submit handler handleSubmit is passing the raw formData to toRequestPayload after calling validateForm(formData) which only validates but doesn't return the parsed/truncated values from the Zod schema; update handleSubmit to use the parsed result from the schema (e.g., the output of safeParse or the return value of validateForm) and pass that parsed/trimmed object to toRequestPayload instead of formData, ensuring you reference validateForm, handleSubmit, and toRequestPayload when replacing the input with the schema-parsed values.
276-276:⚠️ Potential issue | 🟠 MajorEarly return blocks first-time resource configuration.
When
resourceConfigsisundefined(no existing config), this line returnsnull, making the "Configure" button fromDeployCardineffective. Users cannot create initial resource configurations.🐛 Remove early return and use defaults
+const DEFAULT_FORM_DATA: ResourceConfigsFormValues = { + replicas: 1, + cpuRequest: "500m", + memoryRequest: "512Mi", + cpuLimit: "", + memoryLimit: "", + autoScalingEnabled: false, + minReplicas: 1, + maxReplicas: 3, +}; + export function EditResourceConfigsDrawer({ ... }: EditResourceConfigsDrawerProps) { - const [formData, setFormData] = useState<ResourceConfigsFormValues>({ - replicas: 1, - ... - }); + const [formData, setFormData] = + useState<ResourceConfigsFormValues>(DEFAULT_FORM_DATA); useEffect(() => { - if (open && resourceConfigs) { + if (open) { - setFormData(toFormValues(resourceConfigs)); + setFormData(resourceConfigs ? toFormValues(resourceConfigs) : DEFAULT_FORM_DATA); clearErrors(); } }, [open, resourceConfigs, clearErrors]); - - if (!resourceConfigs) return null;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@console/workspaces/pages/deploy/src/subComponent/EditResourceConfigsDrawer.tsx` at line 276, The early return in EditResourceConfigsDrawer (“if (!resourceConfigs) return null;”) prevents opening the drawer when no config exists; remove that return and instead initialize sensible defaults (e.g., empty object/arrays or default form values) so the drawer and Configure button render for first-time setups; update the component (EditResourceConfigsDrawer) and any handlers that assume resourceConfigs is non-null (e.g., form state initialization or save handlers) to safely handle undefined by using resourceConfigs ?? <defaults> and allowing creation of the initial config.
🧹 Nitpick comments (1)
console/workspaces/libs/shared-component/src/components/ResourceMetricChip.tsx (1)
81-81: Import statement should be at the top of the file.The
import type { ReactNode } from "react"on line 81 appears after function definitions. Move it to the import block at the top (around line 19) for consistency and readability.♻️ Move import to top
+import type { ReactNode } from "react"; import { alpha, Box, Stack, Theme, Tooltip, Typography, useTheme } from "@wso2/oxygen-ui"; export function formatCpu(cores: number): string {Then remove line 81.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@console/workspaces/libs/shared-component/src/components/ResourceMetricChip.tsx` at line 81, The file has an import type { ReactNode } placed after component/function definitions; move this import into the main import block at the top of ResourceMetricChip.tsx (near the other imports around the top of the file) and remove the duplicate import statement at line 81 so ResourceMetricChip and any helper functions use the top-level ReactNode import consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@console/workspaces/pages/deploy/src/subComponent/DeployCard.tsx`:
- Around line 102-107: The sx style for the Box in DeployCard uses the SVG-only
property fillOpacity; remove fillOpacity: 0.1 or replace it with the CSS
property opacity (e.g., opacity: 0.1) on the same sx object so the visual effect
applies to the HTML element; update the sx object that currently contains
backgroundColor, fillOpacity, padding, borderRadius to use opacity or drop the
unused fillOpacity entry.
In
`@console/workspaces/pages/deploy/src/subComponent/EditResourceConfigsDrawer.tsx`:
- Around line 56-74: The memory unit lookup fails because parseMemoryQuantity
lowercases the matched unit but MEMORY_UNITS keys are capitalized; update the
mapping or lookup so the keys match the lookup case: either convert MEMORY_UNITS
keys to lowercase (e.g., "ki","mi","gi",...) or stop lowercasing the unit in
parseMemoryQuantity and use the unit as matched; modify MEMORY_UNITS and/or
parseMemoryQuantity accordingly (refer to MEMORY_UNITS and function
parseMemoryQuantity).
---
Duplicate comments:
In
`@console/workspaces/pages/deploy/src/subComponent/EditResourceConfigsDrawer.tsx`:
- Around line 241-246: The submit handler handleSubmit is passing the raw
formData to toRequestPayload after calling validateForm(formData) which only
validates but doesn't return the parsed/truncated values from the Zod schema;
update handleSubmit to use the parsed result from the schema (e.g., the output
of safeParse or the return value of validateForm) and pass that parsed/trimmed
object to toRequestPayload instead of formData, ensuring you reference
validateForm, handleSubmit, and toRequestPayload when replacing the input with
the schema-parsed values.
- Line 276: The early return in EditResourceConfigsDrawer (“if
(!resourceConfigs) return null;”) prevents opening the drawer when no config
exists; remove that return and instead initialize sensible defaults (e.g., empty
object/arrays or default form values) so the drawer and Configure button render
for first-time setups; update the component (EditResourceConfigsDrawer) and any
handlers that assume resourceConfigs is non-null (e.g., form state
initialization or save handlers) to safely handle undefined by using
resourceConfigs ?? <defaults> and allowing creation of the initial config.
---
Nitpick comments:
In
`@console/workspaces/libs/shared-component/src/components/ResourceMetricChip.tsx`:
- Line 81: The file has an import type { ReactNode } placed after
component/function definitions; move this import into the main import block at
the top of ResourceMetricChip.tsx (near the other imports around the top of the
file) and remove the duplicate import statement at line 81 so ResourceMetricChip
and any helper functions use the top-level ReactNode import consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: df43803b-4d6b-48b7-9d2d-41d04edf4a2c
📒 Files selected for processing (12)
console/workspaces/libs/api-client/src/apis/metrics.tsconsole/workspaces/libs/api-client/src/apis/resource-configs.tsconsole/workspaces/libs/shared-component/src/components/DeploymentConfig.tsxconsole/workspaces/libs/shared-component/src/components/ResourceMetricChip.tsxconsole/workspaces/pages/deploy/src/Deploy.Component.tsxconsole/workspaces/pages/deploy/src/subComponent/BuildCard.tsxconsole/workspaces/pages/deploy/src/subComponent/BuildSelectorDrawer.tsxconsole/workspaces/pages/deploy/src/subComponent/DeployCard.tsxconsole/workspaces/pages/deploy/src/subComponent/EditResourceConfigsDrawer.tsxconsole/workspaces/pages/llm-providers/src/index.tsconsole/workspaces/pages/metrics/src/components/MetricsView/MetricsView.tsxconsole/workspaces/pages/overview/src/AddLLMProvider.Component.tsx
✅ Files skipped from review due to trivial changes (2)
- console/workspaces/pages/llm-providers/src/index.ts
- console/workspaces/pages/deploy/src/subComponent/BuildCard.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- console/workspaces/pages/overview/src/AddLLMProvider.Component.tsx
- console/workspaces/libs/api-client/src/apis/metrics.ts
- console/workspaces/pages/metrics/src/components/MetricsView/MetricsView.tsx
- console/workspaces/pages/deploy/src/subComponent/BuildSelectorDrawer.tsx
- console/workspaces/pages/deploy/src/Deploy.Component.tsx
- console/workspaces/libs/api-client/src/apis/resource-configs.ts
Comment on lines
+102
to
+107
| sx={{ | ||
| backgroundColor: backgroundColor, | ||
| fillOpacity: 0.1, | ||
| padding: 1, | ||
| borderRadius: 0.5, | ||
| }} |
Contributor
There was a problem hiding this comment.
fillOpacity is not a valid CSS property for HTML elements.
Line 104 uses fillOpacity: 0.1, which is an SVG-specific property and has no effect on the Box element. If opacity is intended, use opacity instead, or remove the line if the alpha-blended backgroundColor already provides the desired effect.
🐛 Remove unused property
sx={{
backgroundColor: backgroundColor,
- fillOpacity: 0.1,
padding: 1,
borderRadius: 0.5,
}}📝 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.
Suggested change
| sx={{ | |
| backgroundColor: backgroundColor, | |
| fillOpacity: 0.1, | |
| padding: 1, | |
| borderRadius: 0.5, | |
| }} | |
| sx={{ | |
| backgroundColor: backgroundColor, | |
| padding: 1, | |
| borderRadius: 0.5, | |
| }} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@console/workspaces/pages/deploy/src/subComponent/DeployCard.tsx` around lines
102 - 107, The sx style for the Box in DeployCard uses the SVG-only property
fillOpacity; remove fillOpacity: 0.1 or replace it with the CSS property opacity
(e.g., opacity: 0.1) on the same sx object so the visual effect applies to the
HTML element; update the sx object that currently contains backgroundColor,
fillOpacity, padding, borderRadius to use opacity or drop the unused fillOpacity
entry.
Comment on lines
+56
to
+74
| const MEMORY_UNITS: Record<string, number> = { | ||
| Ki: 1024, | ||
| Mi: 1024 ** 2, | ||
| Gi: 1024 ** 3, | ||
| Ti: 1024 ** 4, | ||
| Pi: 1024 ** 5, | ||
| Ei: 1024 ** 6, | ||
| }; | ||
|
|
||
| /** Parse memory quantity to bytes (e.g. 256Mi -> 268435456). */ | ||
| function parseMemoryQuantity(value: string): number { | ||
| if (!value?.trim()) return NaN; | ||
| const v = value.trim(); | ||
| const match = v.match(/^([0-9]+(?:\.[0-9]+)?)\s*(Ki|Mi|Gi|Ti|Pi|Ei)$/i); | ||
| if (!match) return NaN; | ||
| const num = parseFloat(match[1]); | ||
| const unit = match[2].toLowerCase(); | ||
| return num * (MEMORY_UNITS[unit] ?? 1); | ||
| } |
Contributor
There was a problem hiding this comment.
Bug: Memory unit lookup fails due to case mismatch.
MEMORY_UNITS uses capitalized keys (Ki, Mi, Gi), but parseMemoryQuantity lowercases the matched unit on line 72 before lookup. Since MEMORY_UNITS["mi"] is undefined, all memory values fall back to factor 1, breaking validation comparisons.
For example, parseMemoryQuantity("256Mi") returns 256 instead of 268435456.
🐛 Fix: Use lowercase keys in MEMORY_UNITS
const MEMORY_UNITS: Record<string, number> = {
- Ki: 1024,
- Mi: 1024 ** 2,
- Gi: 1024 ** 3,
- Ti: 1024 ** 4,
- Pi: 1024 ** 5,
- Ei: 1024 ** 6,
+ ki: 1024,
+ mi: 1024 ** 2,
+ gi: 1024 ** 3,
+ ti: 1024 ** 4,
+ pi: 1024 ** 5,
+ ei: 1024 ** 6,
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@console/workspaces/pages/deploy/src/subComponent/EditResourceConfigsDrawer.tsx`
around lines 56 - 74, The memory unit lookup fails because parseMemoryQuantity
lowercases the matched unit but MEMORY_UNITS keys are capitalized; update the
mapping or lookup so the keys match the lookup case: either convert MEMORY_UNITS
keys to lowercase (e.g., "ki","mi","gi",...) or stop lowercasing the unit in
parseMemoryQuantity and use the unit as matched; modify MEMORY_UNITS and/or
parseMemoryQuantity accordingly (refer to MEMORY_UNITS and function
parseMemoryQuantity).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Fixes: #370
This pull request introduces several improvements and new features to the agent management platform, focusing on API client extensibility, deployment configuration UI enhancements, and better handling of agent metrics. The main highlights are the addition of resource configuration APIs and hooks, UI refactoring for deployment configuration, and improved defaulting and polling logic for agent metrics.
API Client and Resource Configurations:
resource-configs.tsin bothapisandhooks, and updating the export lists to include these new modules. [1] [2] [3] [4]Agent Metrics Improvements:
getAgentMetricsfunction to automatically set defaultstartTimeandendTimeif not provided, ensuring metrics queries always have a valid time range.useGetAgentMetricshook to support optional auto-refresh with a new, slower polling interval (SLOW_POLL_INTERVAL), and relaxed the enabled condition to not require explicit time fields. [1] [2]Deployment Configuration UI Enhancements:
DeploymentConfigcomponent to use the newFormUI primitives, improving structure and readability. This includes better sorting of environment variables, improved handling of sensitive variables, and clearer deployment/promotion text. [1] [2] [3] [4] [5] [6] [7] [8]EnvironmentVariablecomponent to support hiding the title, allowing for better UI composition in forms. [1] [2]Sidebar and Environment Card UI Tweaks:
EnvironmentCardto use the default color instead of warning, making the status display less alarming. [1] [2]Goals
Approach
User stories
Release note
Documentation
Training
Certification
Marketing
Automation tests
Security checks
Samples
Related PRs
Migrations (if applicable)
Test environment
Learning
Summary by CodeRabbit
New Features
Improvements
Updates
Chores