Skip to content

feat(observability): scheduled-task Runs page + retries scoped to run lifetime#692

Open
ThusharaSampath wants to merge 9 commits into
openchoreo:mainfrom
ThusharaSampath:feat/retries-time-window
Open

feat(observability): scheduled-task Runs page + retries scoped to run lifetime#692
ThusharaSampath wants to merge 9 commits into
openchoreo:mainfrom
ThusharaSampath:feat/retries-time-window

Conversation

@ThusharaSampath

@ThusharaSampath ThusharaSampath commented Jul 16, 2026

Copy link
Copy Markdown

Draft — depends on openchoreo/openchoreo#3933 landing so the backend accepts the optional `startTime`/`endTime` on the retries query. Ready for early review; will be moved out of draft once #3933 merges.

Purpose

Adds the Runs tab for scheduled-task components to the OpenChoreo observability plugin, and scopes the per-run retries fetch to the run's own lifetime.

The tab renders a hierarchical view — `Component → Runs (Jobs) → Retries (Pods) → per-retry logs` — over the two runs/retries endpoints introduced in the observer PR:

  • `POST /api/v1/scheduled-tasks/runs/query`
  • `POST /api/v1/scheduled-tasks/runs/{jobName}/retries/query` (now accepts optional `startTime`/`endTime`)

Approach

  • Runs UI — new component tree under `plugins/openchoreo-observability/src/components/Runs/` (page wrapper, table, expandable run row, expandable retry row with lazy per-pod log fetch), styles, types, three hooks (`useRuns`, `useRetries`, `useUrlFiltersForRuns`), and `usePodLogs` for the retry-level log panel.
  • Observability API client — `getRuns()`, `getRetries()`, `getPodLogs()` on `ObservabilityApi.ts`, following the same URL-cache pattern as the existing observability calls.
  • Retries scoped to run lifetime — `getRetries` accepts an optional `{ startTime, endTime }`. `RunRow` passes the run's own `startTime` / `completionTime` through `useRetries`. Bounds are only sent when both are truthy (backend rejects one-only per the both-or-none contract from #3933). Prevents the 1000-event-per-call adapter cap from silently truncating retries on high-frequency CronJobs.
  • Entity page — new `scheduledTaskEntityPage` variant that renders the Runs tab for CronJob-flavored components; other component types keep their existing tabs unchanged.
  • Plugin registration — `ObservabilityRuns` routable extension registered in `plugin.ts` / `index.ts`.
  • Import migrations — repointed 4 files at upstream renames (`useProjectEnvironments` from `@openchoreo/backstage-plugin-react`, `calculateTimeRange` now upstream, local `Environment` shape kept inlined in `Runs/types.ts` since the upstream `Environment` has a different shape than the id-based one this page needs).

Related PRs / Issues

  • Backend: openchoreo/openchoreo#3933 — introduces the runs/retries endpoints and the optional `startTime`/`endTime` on retries.
  • Design context: openchoreo/openchoreo discussion #1894 (Kube Events Persistence).

Checklist

  • Tests — the changed files are covered by the existing test setup; will add targeted tests before removing draft status if reviewers want them.
  • Screenshots — will attach once #3933 lands and a full end-to-end demo is possible.

Verification

  • `npx tsc --noEmit` on the workspace is clean for all files touched by this PR (task-obs paths under `components/Runs/`, `hooks/useRuns.ts`, `hooks/useRetries.ts`, `hooks/useUrlFiltersForRuns.ts`, `hooks/usePodLogs.ts`).
  • Manually verified end-to-end against a local dev stack (v1.2.0-m.1 quick-start cluster + observer built from #3933): Runs tab loads for a CronJob component, retries drill-down fetches per-run, per-retry log panel opens and refreshes correctly.

Remarks

  • Pre-existing (unrelated) app-level TSC errors exist in `packages/app/*` and a few other plugins (missing `@openchoreo/backstage-plugin-openchoreo-portal-assistant` module, Backstage `frontend-plugin-api` blueprint type shifts, missing `@openchoreo/test-utils` workspace package). Confirmed all of these predate this branch — not touched by this PR.

Summary by CodeRabbit

  • New Features

    • Added a Runs tab for scheduled-task components.
    • View run status, timing, duration, failures, retries, events, and pod logs.
    • Added environment, time-range, sort, pagination, and refresh controls.
    • Run filters are preserved in the page URL for easy sharing and navigation.
  • Bug Fixes

    • Improved observability error messaging and handling when the feature is unavailable.
  • Tests

    • Added comprehensive coverage for runs, retries, logs, filtering, pagination, and error states.

@github-actions

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown

Changeset detected — the following file(s) will be released with this PR:

.changeset/scheduled-task-runs-page.md

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a permission-gated Runs tab for scheduled-task entities, with API clients and hooks for runs, retries, and pod logs. The UI supports URL-synchronized filters, pagination, expandable retry details, events, and logs.

Changes

Scheduled task observability

Layer / File(s) Summary
Query contracts and client methods
plugins/openchoreo-observability/src/components/Runs/types.ts, plugins/openchoreo-observability/src/api/ObservabilityApi.ts, plugins/openchoreo-observability/src/api/ObservabilityApi.test.ts
Adds typed run and retry models plus API methods for scheduled-task runs, retries, and pod logs, including request mapping, defaults, scoped time bounds, and error handling.
Filtering and fetch hooks
plugins/openchoreo-observability/src/hooks/*
Adds URL-synchronized filters and hooks for fetching runs, retries, and pod logs, including pagination, refresh, validation, and stale-request protection.
Runs page and controls
plugins/openchoreo-observability/src/components/Runs/ObservabilityRunsPage.tsx, RunsFilter.tsx, RunsActions.tsx, RunsTable.tsx, RunsActions.test.tsx
Adds permission gating, environment/time/sort filters, pagination and refresh controls, loading states, and the runs table.
Run and retry details
plugins/openchoreo-observability/src/components/Runs/RunRow.tsx, RetryRow.tsx, styles.ts, index.ts
Adds expandable run rows with run-scoped retries, retry events, pod-log retrieval, status formatting, and log rendering.
Entity-page integration
packages/app/src/components/catalog/EntityPage.tsx, plugins/openchoreo-observability/src/plugin.ts, plugins/openchoreo-observability/src/index.ts, .changeset/scheduled-task-runs-page.md
Adds the scheduled-task entity layout, gated Runs route, lazy component export, routing selection, and package changeset.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant ObservabilityRunsPage
  participant useRuns
  participant ObservabilityClient
  participant Observer
  participant RunRow
  participant RetryRow
  participant usePodLogs

  User->>ObservabilityRunsPage: select filters or expand a run
  ObservabilityRunsPage->>useRuns: fetch runs
  useRuns->>ObservabilityClient: getRuns
  ObservabilityClient->>Observer: POST scheduled-runs query
  Observer-->>ObservabilityClient: return runs
  ObservabilityClient-->>useRuns: resolve runs
  useRuns-->>ObservabilityRunsPage: render RunsTable
  RunRow->>ObservabilityClient: getRetries for run lifetime
  ObservabilityClient->>Observer: POST retries query
  Observer-->>RunRow: return retries
  RunRow-->>RetryRow: render retry details
  RetryRow->>usePodLogs: fetch expanded pod logs
  usePodLogs->>ObservabilityClient: getPodLogs
  ObservabilityClient->>Observer: POST pod-log query
  Observer-->>RetryRow: return logs
Loading

Suggested reviewers: sameerajayasoma, stefinie123, kaviththiranga

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description covers purpose and approach, but omits many required template sections like Goals, Release note, Documentation, Automation tests, and Security checks. Add the missing template sections, especially Goals, Release note, Documentation, Automation tests, Security checks, and the remaining required fields.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding a Runs page for scheduled-task observability with run-lifetime-scoped retries.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Signed-off-by: Thushara Sampath <mkt.sampath@yahoo.com>
Signed-off-by: Thushara Sampath <mkt.sampath@yahoo.com>
Signed-off-by: Thushara Sampath <mkt.sampath@yahoo.com>
Signed-off-by: Thushara Sampath <mkt.sampath@yahoo.com>
Pass the run's `startTime` and `completionTime` from `RunRow` through
`useRetries` into `ObservabilityApi.getRetries`, and forward them as
`startTime`/`endTime` on the backend retries query so the events fetch
is scoped to a single run's window instead of the backend's 30-day
default lookback.

Motivation: the observer's kube-events-collector adapter caps a single
retries call at 1000 events. On high-frequency CronJobs, a 30-day
default window silently truncates data. Scoping to the run's own
lifetime keeps the retries view accurate without paginating.

Contract (from openchoreo/openchoreo#3933):
- Both `startTime` and `endTime` provided -> that window is used.
- Exactly one provided -> backend responds 400 (both-or-none).
- Both omitted -> backend defaults to a 30-day lookback (unchanged
  behavior for callers that do not pass the pair).

`useRetries` and `getRetries` only forward the pair when both values
are truthy, so a still-running run (no `completionTime` yet) transparently
falls back to the backend default rather than triggering a 400.

Signed-off-by: Thushara Sampath <mkt.sampath@yahoo.com>
Catch up the Runs page (originally landed as "Triggers") with upstream
refactors that moved / renamed helpers the page had been importing from
locations that no longer exist. The branch's task-obs commits pre-dated
these upstream moves, so `tsc` failed on stale imports. No behavioral
change beyond adopting the new empty/forbidden/unavailable environment
status pattern already used by Metrics, Alerts, etc.

- ObservabilityRunsPage.tsx: swap `useGetEnvironmentsByNamespace` (gone)
  for `useProjectEnvironments` from `@openchoreo/backstage-plugin-react`,
  and render `EnvironmentsStatusNotice` for the non-`ok` statuses
  (`empty-pipeline` / `forbidden` / `unavailable`) instead of the old
  generic "No environments found" alert.
- Runs/types.ts: inline the local `Environment` shape (id/name/resourceName)
  the Runs filter and URL sync depend on. Previously re-exported from
  `RuntimeLogs/types`, which no longer defines it; the upstream `Environment`
  has a different shape (name/displayName/namespace/dataPlaneRef) and is
  mapped onto this local one by `ObservabilityRunsPage`.
- hooks/useRuns.ts: import `calculateTimeRange` from
  `@openchoreo/backstage-plugin-react` (its new home); the old
  `components/RuntimeLogs/utils` module was removed upstream.
- hooks/useUrlFiltersForRuns.ts: point the `Environment` type import at
  the local `Runs/types` shape (`id`-based) it actually uses, not the
  removed `RuntimeLogs/types` re-export.

Signed-off-by: Thushara Sampath <mkt.sampath@yahoo.com>
- Ran prettier over the 6 files the Lint job flagged (ObservabilityApi.ts,
  RetryRow.tsx, RunRow.tsx, RunsActions.tsx, useRetries.ts, useRuns.ts).
  No behavioural change.
- Added .changeset/scheduled-task-runs-page.md so the Check-for-changeset
  job passes. Marks the openchoreo-observability plugin for a minor bump
  and references the backend PR openchoreo/openchoreo#3933.

Signed-off-by: Thushara Sampath <mkt.sampath@yahoo.com>
@ThusharaSampath
ThusharaSampath force-pushed the feat/retries-time-window branch from 4678ddd to 69b00ad Compare July 17, 2026 03:49
Covers the Runs-page API surface + supporting hooks that were pushing
patch coverage under threshold on the retries-time-window PR:

- src/api/ObservabilityApi.test.ts: add getRuns, getRetries, getPodLogs
  suites. getRetries includes explicit both/one-only/neither cases for
  the client-side both-or-none time-bounds guard, plus jobName encoding
  and response-shape defaults.
- src/hooks/useRuns.test.ts: initial state, fetch success, error paths,
  refresh flow, and the stale-request guard on requestVersionRef.
- src/hooks/useRetries.test.ts: skips call when jobName/namespace empty,
  bounds forwarding under both/one/neither, error paths, stale guard.
- src/hooks/usePodLogs.test.ts: skips call when podName/component empty,
  option forwarding, error paths, stale guard.
- src/hooks/useUrlFiltersForRuns.test.tsx: default filters, URL parsing,
  auto-select-first-env effect, updateFilters, resetFilters.

Signed-off-by: Thushara Sampath <mkt.sampath@yahoo.com>
…PI tests

- Prettier'd plugins/openchoreo-observability/src/api/ObservabilityApi.test.ts
  (Lint CI flagged it after the previous test-coverage push).
- New RunsActions.test.tsx covers the pagination + refresh + disabled
  behaviour of the Runs page actions bar. Focused on real behaviour that
  breaks silently: Prev disabled at page 0, Next disabled on last page,
  Next disabled when totalCount is 0, endItem capped on the partial last
  page, "showing X-Y" hidden when totalCount is 0, disabled prop overrides
  pagination state.

Together these bring patch coverage above the 52% threshold codecov flagged.

Signed-off-by: Thushara Sampath <mkt.sampath@yahoo.com>
@ThusharaSampath
ThusharaSampath marked this pull request as ready for review July 17, 2026 09:56

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🤖 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
`@plugins/openchoreo-observability/src/components/Runs/ObservabilityRunsPage.tsx`:
- Around line 201-223: Update ObservabilityRunsPage to select the environment
before invoking useLogsPermission, passing selectedEnvironment.resourceName for
the environment-scoped check. Move the permission/loading gate below environment
selection, and prevent runs and pod-log data fetching until the permission check
completes and canViewLogs is true.
- Around line 82-105: The filter-change path in the ObservabilityRunsPage
useEffect must not retain runs from the previous environment or filter context.
Update the useRuns implementation used by fetchRuns so its _reset path clears
runs and totalCount before loading the new context, ensuring the rendering logic
around the runs list cannot display stale jobs after a filter change or failed
request.

In `@plugins/openchoreo-observability/src/components/Runs/RetryRow.tsx`:
- Around line 72-105: Update the log-fetch flow in RetryRow around usePodLogs
and the expanded useEffect so an active run without runCompletionTime computes
logsEndTime from Date.now() immediately before every fetchLogs call, rather than
reusing the render-time value. Preserve the fixed completion-time end bound for
completed runs and continue using the existing start-time window.

In `@plugins/openchoreo-observability/src/components/Runs/RunsActions.tsx`:
- Around line 39-43: Normalize the current page after a successful runs response
when it exceeds the calculated totalPages. Update filters.page to the last valid
page (totalPages minus one) and refetch using that page, preventing invalid
ranges such as “showing 1981-25”; preserve existing pagination behavior for
valid pages and add a test covering an out-of-range bookmarked page.

In `@plugins/openchoreo-observability/src/hooks/usePodLogs.ts`:
- Around line 30-41: Invalidate requests before incomplete-scope returns: in
plugins/openchoreo-observability/src/hooks/usePodLogs.ts lines 30-41, validate
projectName, increment requestVersionRef, and clear logs, loading, and error
state; in plugins/openchoreo-observability/src/hooks/useRetries.ts lines 40-50,
apply the same projectName validation, version increment, and
retry/loading/error cleanup; in
plugins/openchoreo-observability/src/hooks/useRuns.ts lines 43-55, increment the
request version and clear runs, total, loading, and error state before
returning.

In `@plugins/openchoreo-observability/src/hooks/useUrlFiltersForRuns.ts`:
- Around line 59-104: Update updateFilters so a supplied page is ignored
whenever environmentId, timeRange, or sortOrder is present in the same update,
preserving the reset-to-first-page behavior for combined filter and page
changes. Keep standalone page updates unchanged, and add a test covering each
combined-update case to verify the stale page is not restored.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1ea6380e-7b34-4fed-8bd7-15d66cc0b5a6

📥 Commits

Reviewing files that changed from the base of the PR and between 33abf75 and e25c64a.

📒 Files selected for processing (25)
  • .changeset/scheduled-task-runs-page.md
  • packages/app/src/components/catalog/EntityPage.tsx
  • plugins/openchoreo-observability/src/api/ObservabilityApi.test.ts
  • plugins/openchoreo-observability/src/api/ObservabilityApi.ts
  • plugins/openchoreo-observability/src/components/Runs/ObservabilityRunsPage.tsx
  • plugins/openchoreo-observability/src/components/Runs/RetryRow.tsx
  • plugins/openchoreo-observability/src/components/Runs/RunRow.tsx
  • plugins/openchoreo-observability/src/components/Runs/RunsActions.test.tsx
  • plugins/openchoreo-observability/src/components/Runs/RunsActions.tsx
  • plugins/openchoreo-observability/src/components/Runs/RunsFilter.tsx
  • plugins/openchoreo-observability/src/components/Runs/RunsTable.tsx
  • plugins/openchoreo-observability/src/components/Runs/index.ts
  • plugins/openchoreo-observability/src/components/Runs/styles.ts
  • plugins/openchoreo-observability/src/components/Runs/types.ts
  • plugins/openchoreo-observability/src/hooks/index.ts
  • plugins/openchoreo-observability/src/hooks/usePodLogs.test.ts
  • plugins/openchoreo-observability/src/hooks/usePodLogs.ts
  • plugins/openchoreo-observability/src/hooks/useRetries.test.ts
  • plugins/openchoreo-observability/src/hooks/useRetries.ts
  • plugins/openchoreo-observability/src/hooks/useRuns.test.ts
  • plugins/openchoreo-observability/src/hooks/useRuns.ts
  • plugins/openchoreo-observability/src/hooks/useUrlFiltersForRuns.test.tsx
  • plugins/openchoreo-observability/src/hooks/useUrlFiltersForRuns.ts
  • plugins/openchoreo-observability/src/index.ts
  • plugins/openchoreo-observability/src/plugin.ts

Comment on lines +82 to +105
useEffect(() => {
const currentFilters = {
environmentId: filters.environmentId,
timeRange: filters.timeRange,
sortOrder: filters.sortOrder,
page: filters.page,
};
const filtersChanged =
previousFiltersRef.current === null ||
JSON.stringify(previousFiltersRef.current) !==
JSON.stringify(currentFilters);

if (
filters.environmentId &&
selectedEnvironment &&
namespace &&
project &&
componentName &&
filtersChanged
) {
fetchRuns(true);
setLastUpdated(new Date());
previousFiltersRef.current = currentFilters;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Clear runs when their filter context changes.

At Line 102, fetchRuns(true) does not reset anything: the supplied useRuns implementation ignores _reset and retains runs and totalCount. Lines 187-194 therefore render old jobs with the newly selected environment identifiers; expanding one can query an old jobName against the new environment, and a failed request leaves that mismatch visible indefinitely.

Honor _reset in useRuns, or only render data tagged with the active filter context.

Also applies to: 176-194

🤖 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
`@plugins/openchoreo-observability/src/components/Runs/ObservabilityRunsPage.tsx`
around lines 82 - 105, The filter-change path in the ObservabilityRunsPage
useEffect must not retain runs from the previous environment or filter context.
Update the useRuns implementation used by fetchRuns so its _reset path clears
runs and totalCount before loading the new context, ensuring the rendering logic
around the runs list cannot display stale jobs after a filter change or failed
request.

Comment on lines +201 to +223
export const ObservabilityRunsPage = () => {
const {
canViewLogs,
loading: permissionLoading,
deniedTooltip,
} = useLogsPermission();

if (permissionLoading) return <Progress />;

if (!canViewLogs) {
return (
<EmptyState
missing="data"
title="Permission Denied"
description={
<Box display="flex" alignItems="center" gridGap={8}>
<WarningIcon />
{deniedTooltip}
</Box>
}
/>
);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Apply the logs permission check to the selected environment.

Line 206 calls useLogsPermission() without an environment, so its environment-scoped ABAC evaluation is skipped while runs and pod logs are subsequently requested for a selected environment. Move the permission gate below environment selection, pass selectedEnvironment.resourceName, and suppress data fetching until that check allows access.

🤖 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
`@plugins/openchoreo-observability/src/components/Runs/ObservabilityRunsPage.tsx`
around lines 201 - 223, Update ObservabilityRunsPage to select the environment
before invoking useLogsPermission, passing selectedEnvironment.resourceName for
the environment-scoped check. Move the permission/loading gate below environment
selection, and prevent runs and pod-log data fetching until the permission check
completes and canViewLogs is true.

Comment on lines +72 to +105
const { logsStartTime, logsEndTime } = useMemo(() => {
const baseStart = retry.startTime || runStartTime;
const startMs = baseStart
? new Date(baseStart).getTime() - 60 * 1000
: Date.now() - 24 * 3600 * 1000;
const endMs = runCompletionTime
? new Date(runCompletionTime).getTime() + 5 * 60 * 1000
: Date.now();
return {
logsStartTime: new Date(startMs).toISOString(),
logsEndTime: new Date(endMs).toISOString(),
};
}, [retry.startTime, runStartTime, runCompletionTime]);

const {
logs,
loading: logsLoading,
error: logsError,
fetchLogs,
} = usePodLogs({
podName: expanded ? retry.podName : '',
namespaceName,
projectName,
environmentName,
componentName,
startTime: logsStartTime,
endTime: logsEndTime,
});

useEffect(() => {
if (expanded) {
fetchLogs();
}
}, [expanded, fetchLogs]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Recompute the active-run log window for every fetch.

When runCompletionTime is absent, Line 79 captures Date.now() when the row renders. Expanding or refreshing later reuses that frozen logsEndTime, so logs produced afterward are never requested. Build the live end time immediately before each fetchLogs call while retaining the fixed completion bound for completed runs.

Also applies to: 171-180

🤖 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 `@plugins/openchoreo-observability/src/components/Runs/RetryRow.tsx` around
lines 72 - 105, Update the log-fetch flow in RetryRow around usePodLogs and the
expanded useEffect so an active run without runCompletionTime computes
logsEndTime from Date.now() immediately before every fetchLogs call, rather than
reusing the render-time value. Preserve the fixed completion-time end bound for
completed runs and continue using the existing start-time window.

Comment on lines +39 to +43
const page = filters.page;
const pageSize = RUNS_PAGE_SIZE;
const totalPages = Math.max(1, Math.ceil(totalCount / pageSize));
const startItem = totalCount === 0 ? 0 : page * pageSize + 1;
const endItem = Math.min((page + 1) * pageSize, totalCount);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Normalize pages that exceed the current result count.

A bookmarked page or shrinking totalCount can produce Page 100 / 2 and showing 1981-25, while requiring repeated Prev clicks to recover. After a successful response, reset filters.page to the last valid page and refetch; add an out-of-range test.

Also applies to: 79-91

🤖 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 `@plugins/openchoreo-observability/src/components/Runs/RunsActions.tsx` around
lines 39 - 43, Normalize the current page after a successful runs response when
it exceeds the calculated totalPages. Update filters.page to the last valid page
(totalPages minus one) and refetch using that page, preventing invalid ranges
such as “showing 1981-25”; preserve existing pagination behavior for valid pages
and add a test covering an out-of-range bookmarked page.

Comment on lines +30 to +41
const fetchLogs = useCallback(async () => {
if (
!options.podName ||
!options.namespaceName ||
!options.environmentName ||
!options.componentName
) {
setLogs([]);
return;
}

const version = ++requestVersionRef.current;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Invalidate pending requests before returning for incomplete query scope.

Each hook returns before advancing its request version, so an earlier response can commit data belonging to the previous scope.

  • plugins/openchoreo-observability/src/hooks/usePodLogs.ts#L30-L41: validate projectName, increment the version, and clear log/loading/error state.
  • plugins/openchoreo-observability/src/hooks/useRetries.ts#L40-L50: validate projectName, increment the version, and clear retry/loading/error state.
  • plugins/openchoreo-observability/src/hooks/useRuns.ts#L43-L55: increment the version and clear runs, total, loading, and error state.
📍 Affects 3 files
  • plugins/openchoreo-observability/src/hooks/usePodLogs.ts#L30-L41 (this comment)
  • plugins/openchoreo-observability/src/hooks/useRetries.ts#L40-L50
  • plugins/openchoreo-observability/src/hooks/useRuns.ts#L43-L55
🤖 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 `@plugins/openchoreo-observability/src/hooks/usePodLogs.ts` around lines 30 -
41, Invalidate requests before incomplete-scope returns: in
plugins/openchoreo-observability/src/hooks/usePodLogs.ts lines 30-41, validate
projectName, increment requestVersionRef, and clear logs, loading, and error
state; in plugins/openchoreo-observability/src/hooks/useRetries.ts lines 40-50,
apply the same projectName validation, version increment, and
retry/loading/error cleanup; in
plugins/openchoreo-observability/src/hooks/useRuns.ts lines 43-55, increment the
request version and clear runs, total, loading, and error state before
returning.

Comment on lines +59 to +104
const updateFilters = useCallback(
(newFilters: Partial<RunsFilters>) => {
const newParams = new URLSearchParams(searchParams);

if (newFilters.environmentId !== undefined) {
if (newFilters.environmentId) {
newParams.set('env', newFilters.environmentId);
} else {
newParams.delete('env');
}
// Reset page when environment changes
newParams.delete('page');
}

if (newFilters.timeRange !== undefined) {
if (newFilters.timeRange === DEFAULT_TIME_RANGE) {
newParams.delete('timeRange');
} else {
newParams.set('timeRange', newFilters.timeRange);
}
// Reset page when time range changes
newParams.delete('page');
}

if (newFilters.sortOrder !== undefined) {
if (newFilters.sortOrder === 'desc') {
newParams.delete('sort');
} else {
newParams.set('sort', newFilters.sortOrder);
}
// Reset page when sort changes
newParams.delete('page');
}

if (newFilters.page !== undefined) {
if (newFilters.page === 0) {
newParams.delete('page');
} else {
newParams.set('page', String(newFilters.page));
}
}

setSearchParams(newParams, { replace: true });
},
[searchParams, setSearchParams],
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Prevent combined filter updates from restoring a stale page.

For { environmentId, page }, { timeRange, page }, or { sortOrder, page }, the reset deletes page, then Lines 93–98 add it back. Ignore the supplied page whenever a page-resetting filter is present, and add a combined-update test.

🤖 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 `@plugins/openchoreo-observability/src/hooks/useUrlFiltersForRuns.ts` around
lines 59 - 104, Update updateFilters so a supplied page is ignored whenever
environmentId, timeRange, or sortOrder is present in the same update, preserving
the reset-to-first-page behavior for combined filter and page changes. Keep
standalone page updates unchanged, and add a test covering each combined-update
case to verify the stale page is not restored.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant