Skip to content

[cueweb] Add Stuck Frames page (CueCommander parity)#2416

Open
ramonfigueiredo wants to merge 8 commits into
AcademySoftwareFoundation:masterfrom
ramonfigueiredo:cueweb-stuck-frames
Open

[cueweb] Add Stuck Frames page (CueCommander parity)#2416
ramonfigueiredo wants to merge 8 commits into
AcademySoftwareFoundation:masterfrom
ramonfigueiredo:cueweb-stuck-frames

Conversation

@ramonfigueiredo

@ramonfigueiredo ramonfigueiredo commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

Related Issues

Main issue:

Issues related to this PR:

Summarize your change.

[cueweb] Add Stuck Frames page with full CueCommander parity

Add a /stuck-frames page providing CueCommander/CueGUI StuckFramePlugin parity for identifying and managing stuck frames.

  • Server-side aggregation route (/api/stuck-frames) lists unfinished jobs and fans out GetFrames/GetLayers requests, returning frame, layer, service, and average frame duration data in a single request.
  • Replace the initial runtime-only detection with the full StuckFramePlugin heuristic (LLU, % stuck, average completion, runtime >500s, % stuck <1.1), evaluated client-side.
  • Add configurable multi-service filters (% Run Since LLU, Min LLU, % Avg Completion, Total Runtime, Exclude Keywords, Enable) with CueGUI defaults persisted to localStorage.
  • Add threshold controls, auto-refresh, notifications, refresh/clear actions, and polling every 30s.
  • Provide a job-grouped table with Name, Comment, Frame, Host, LLU, Runtime, % Stuck, Average, and lazy-loaded rqlog tail information.
  • Implement frame actions (Tail/View/Last Log, Retry, Eat, Kill, Log + Log-and-X, Frame Not Stuck, Add/Exclude Job, Core Up, View Host) and job actions (View Comments, Job Not Stuck, Add/Exclude Job, Core Up).
  • Add /api/stuck-frames/lastline and /api/layer/action/setmincores endpoints.
  • Reuse retryFrames/killFrames APIs, recording the signed-in user in kill reasons.

LLM usage disclosure

Parts of this solution's implementation were developed with assistance from Claude Opus.

Add a /stuck-frames route listing frames in RUNNING state longer than a configurable threshold, with per-row Retry and Kill actions.

- Threshold slider (1-48h, default 8h) persisted to localStorage; filtering is client-side so the slider is instant.
- Server-side aggregation route /api/stuck-frames lists unfinished jobs (GetJobs) and fans out GetFrames per job filtered to RUNNING (FrameState 2), so the browser makes a single request.
- Retry/Kill reuse retryFrames/killFrames; kill records the signed-in user in the reason. Polls every 30s.
@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This PR implements a "Stuck Frames" feature for identifying and managing running frames exceeding configurable runtime thresholds. It includes backend APIs for frame aggregation across jobs with defensive state validation and safe log access, client utilities providing typed data access and updated RPC action helpers, a configurable filter component with service-specific detection thresholds, and a feature-complete frontend page with auto-refresh, desktop notifications, grouped display, context menus, and layer min-cores adjustment.

Changes

Stuck Frames Feature

Layer / File(s) Summary
Backend APIs - Frame Aggregation, Log Access, and Min-Cores
cueweb/app/api/stuck-frames/route.ts, cueweb/app/api/stuck-frames/lastline/route.ts, cueweb/app/api/layer/action/setmincores/route.ts, cueweb/.env.example
Stuck-frames POST endpoint concurrently aggregates running frames from unfinished jobs using bounded pagination and state filtering, enriches frames with job/service/layer context, and returns flattened result or HTTP 500 on top-level fetch failure. Lastline GET validates path parameters, enforces .rqlog file type, optionally restricts reads to configured log root prefixes via CUEWEB_LOG_ROOTS, and returns the last non-empty line via bounded tail execution. Setmincores POST validates layer.id (non-empty string) and cores (non-negative, finite number) and forwards to RPC gateway. Environment config documents optional log root restriction mechanism.
Client Utilities - Types, Fetching, and Action Helpers
cueweb/app/utils/get_utils.ts, cueweb/app/utils/action_utils.ts
Exports StuckFrame type (Frame extended with jobId, jobName, jobLogDir, jobHasComment, service, avgFrameSec, layerId, layerMinCores), getStuckFrames() to fetch aggregated frames from backend, and getStuckFrameLastLine(logPath) for log tail retrieval with graceful error fallback. Updates frame action helpers (killFrames, eatFrames, retryFrames) to return Promise<boolean> success result for optimistic UI gating instead of void. Introduces setLayerMinCores(layer, cores) RPC helper for layer parameter adjustment.
Filter Configuration Component
cueweb/components/ui/stuck-frame-filters.tsx
Defines StuckFilter shape with service selector, enable flag, numeric thresholds (percent-stuck, min LLU seconds, average completion seconds, total runtime minutes), and exclude-keyword regex. Provides service defaults via SERVICE_DEFAULTS and catch-all fallback via DEFAULT_FILTER and makeServiceFilter(service). Implements reusable NumberField input component with finite-value validation. Main StuckFrameFilters component renders per-filter UI row with service selector (catch-all labeled as default), numeric threshold inputs with context-specific labels, keyword exclusion text field, enable/disable toggle, and add/remove row controls respecting service uniqueness constraints.
Frontend Page - Detection Logic and Lifecycle
cueweb/app/stuck-frames/page.tsx (initialization, detection, load logic)
Initializes page with session user, utilities for duration formatting and metric extraction, and stuck-frame detection combining service-specific filter thresholds with regex-based job/layer exclusion. Establishes state for raw frames, current time, persisted filters (localStorage), hidden frame/job sets, last-line cache, and auto-refresh toggles. Implements load callback with cancellation support and auto-refresh loop with in-flight guard; optionally emits desktop notifications when new stuck frames become visible.
Frontend Page - Data Derivation and Effects
cueweb/app/stuck-frames/page.tsx (grouping, effects, helpers)
Derives availableServices from frame data, computes filtered/grouped results applying stuck detection and hidden-set filtering, groups by jobId with per-frame runtime sort. Implements lazy last-line fetching effect bounding concurrent work (50 frames per pass) and caching results. Manages context-menu lifecycle (click-outside, escape-key handlers); provides toFrame helper to adapt StuckFrame objects to RPC Frame shape.
Frontend Page - Actions and Rendering
cueweb/app/stuck-frames/page.tsx (actions, UI, menus)
Implements log navigation (openLog in new tab with context) and JSON export (exportLog keyed by jobId with cached metrics), hide/exclude operations updating persisted filters, and Core Up dialog for layer min-cores with validation and bulk apply. Shared act wrapper gates optimistic frame hide/reload on RPC success boolean. Renders header (auto-refresh/notification toggles, refresh/clear buttons, filters), grouped frame table with metrics and last-line column, job and frame context menus with action/navigation items, and Core Up modal with input validation and Apply/Cancel controls.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly Related PRs

  • AcademySoftwareFoundation/OpenCue#2397: Modifies cueweb/app/utils/action_utils.ts to change performAction return behavior, enabling the foundation for boolean-based RPC result gating used by this PR's action helpers and new setLayerMinCores.

Suggested Reviewers

  • lithorus
  • DiegoTavares

Poem

🐰 Through running frames that linger long,
We aggregated right where they belong,
With thresholds set and actions swift,
These stuck ones get a timely lift!
Retry, kill, and up the cores—
Frame management opens new doors! 🎯

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 32.56% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding a Stuck Frames page feature to match CueCommander parity, which is the primary objective of all file changes in this PR.
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 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 `@cueweb/app/api/stuck-frames/route.ts`:
- Around line 54-55: The GetFrames call currently uses a fixed page: 1 and
limit: MAX_FRAMES_PER_JOB which truncates results; update the handler that calls
GetFrames to paginate through all pages instead of a single page—start with page
= 1, call GetFrames(page, MAX_FRAMES_PER_JOB) (matching the existing GetFrames
parameter names), append results to an accumulator, increment page and repeat
until a page returns fewer than MAX_FRAMES_PER_JOB rows (or zero), then use the
full accumulated list for stuck-frame listing and Retry/Kill actions. Ensure you
preserve existing filtering/sorting params when calling GetFrames and avoid
unbounded loops by using the limit constant as the page terminator.
- Around line 48-49: The current perJob creation uses Promise.all(jobs.map(...))
which fans out one GetFrames call per unfinished job and can spike upstream;
replace this with a bounded-concurrency approach (e.g., use a concurrency
limiter like p-limit or an explicit batching loop) so only N GetFrames calls run
concurrently. Update the code around perJob and the jobs.map(...) callback in
route.ts to invoke GetFrames within the limiter (or process jobs in chunks of
size N), await the limited-run promises, and return the same perJob results
shape; pick a sensible default concurrency (e.g., 10–50) or make it
configurable.

In `@cueweb/app/utils/get_utils.ts`:
- Around line 196-199: The getStuckFrames helper is hiding fetch failures by
converting null responses into an empty array; change getStuckFrames so it
returns the raw response from accessGetApi (which can be null) or explicitly
throws on error instead of coercing to [] — update the function (getStuckFrames)
to call accessGetApi(ENDPOINT, JSON.stringify({})) and return response directly
(or rethrow any error) so callers can differentiate between a real error
(null/exception) and an empty list of StuckFrame.
🪄 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: 4f402f9c-be3a-4dbd-8d2e-bbc95e30b276

📥 Commits

Reviewing files that changed from the base of the PR and between f8e6a0b and 311cfb1.

📒 Files selected for processing (3)
  • cueweb/app/api/stuck-frames/route.ts
  • cueweb/app/stuck-frames/page.tsx
  • cueweb/app/utils/get_utils.ts

Comment thread cueweb/app/api/stuck-frames/route.ts Outdated
Comment thread cueweb/app/api/stuck-frames/route.ts Outdated
Comment thread cueweb/app/utils/get_utils.ts Outdated
@ramonfigueiredo ramonfigueiredo marked this pull request as draft June 12, 2026 08:06
Replace the running-time MVP with CueGUI's StuckFramePlugin behavior:

- Detection predicate (LLU / % stuck / avg-completion / runtime / >500s, %stuck<1.1) applied live client-side.
- Multi-service filter bar (catch-all + per-service filters) with % Run Since LLU, Min LLU, % Avg Completion, Total Runtime, Exclude Keywords, Enable; CueGUI service defaults; persisted to localStorage.
- Job-grouped table: Name, Comment, Frame, Host, LLU, Runtime, % Stuck, Average, Last Line (lazy rqlog tail).
- Frame menu: Tail/View/Last Log, Retry/Eat/Kill, Log + Log-and-X, Frame Not Stuck, Add/Exclude Job, Core Up, View Host. Job menu: View Comments, Job Not Stuck, Add/Exclude Job, Core Up. Auto-refresh, Notification, Refresh, Clear.
- Aggregation route attaches service/avgFrameSec/layer via GetLayers; add /api/stuck-frames/lastline and /api/layer/action/setmincores.
…e load failures

Address review feedback on the Stuck Frames endpoint and helper:

- route.ts: cap the per-job GetFrames/GetLayers fan-out with a fixed-size worker pool (MAX_CONCURRENT_JOBS = 16) instead of Promise.all over every unfinished job, so a large farm no longer fires hundreds/thousands of concurrent gateway calls on each 30s poll.
- route.ts: paginate GetFrames via getRunningFrames(), looping pages until a short page arrives (bounded by MAX_FRAME_PAGES) instead of fetching only page 1. Jobs with more than MAX_FRAMES_PER_JOB running frames are no longer silently truncated, so Retry/Kill stay available for every row.
- get_utils.ts: getStuckFrames now throws on a non-array response (mirroring getHosts) instead of collapsing a failed fetch into []. A backend outage reaches the page's catch/error path rather than rendering the empty state.
@ramonfigueiredo ramonfigueiredo marked this pull request as ready for review June 15, 2026 20:06

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (2)
cueweb/app/api/layer/action/setmincores/route.ts (2)

39-44: ⚡ Quick win

Redundant response re-serialization — handleRoute already returns the properly formatted NextResponse.

The current code parses the NextResponse from handleRoute, then immediately creates a new NextResponse with the same JSON structure. This double serialization/deserialization is unnecessary overhead.

♻️ Proposed simplification
-  const body = JSON.stringify(jsonBody);
-  const response = await handleRoute(method, endpoint, body, true);
-  const responseData = await response.json();
-
-  if (!response.ok) return NextResponse.json({ error: responseData.error }, { status: response.status });
-  return NextResponse.json({ data: responseData.data }, { status: response.status });
+  const body = JSON.stringify(jsonBody);
+  return handleRoute("POST", endpoint, body, true);
 }
🤖 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 `@cueweb/app/api/layer/action/setmincores/route.ts` around lines 39 - 44, The
code is unnecessarily deserializing and re-serializing the response from
handleRoute. Since handleRoute already returns a properly formatted
NextResponse, remove the response.json() parsing and the subsequent
NextResponse.json() wrapper calls. Instead, return the response from handleRoute
directly, or if error handling is needed, access the response status and body
only once before returning. Eliminate the redundant JSON serialization cycle
that occurs between the handleRoute call and the final return statements.

24-27: 💤 Low value

Redundant method check — named exports auto-route by HTTP verb in Next.js App Router.

The POST named export will only receive POST requests; Next.js returns 405 automatically for other methods. These lines are unreachable dead code.

♻️ Proposed removal
 export async function POST(request: NextRequest) {
   const endpoint = "/job.LayerInterface/SetMinCores";
-  const method = request.method;
-  if (method !== 'POST') {
-    return NextResponse.json({ error: 'Invalid method. Only POST is allowed.' }, { status: 405 });
-  }
🤖 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 `@cueweb/app/api/layer/action/setmincores/route.ts` around lines 24 - 27, The
method check in the route handler is redundant because Next.js App Router
automatically handles HTTP method routing through named exports. Since this file
exports a POST function, only POST requests will reach this handler, and Next.js
returns a 405 response automatically for other methods. Remove the dead code
block that checks if the request method is not 'POST' and returns a 405 error
response, as this condition can never be reached in a properly configured POST
named export function.
🤖 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 `@cueweb/app/stuck-frames/page.tsx`:
- Around line 332-339: The act function always hides the frame after fn()
completes, but backend actions can resolve without throwing even when they fail,
causing failed operations to still remove frames from view. Modify the action
helper functions (retryFrames, eatFrames, and killFrames) to return
Promise<boolean> indicating success or failure, then update the act function to
only call hideFrame(f) when the boolean returned from fn() is true.
- Around line 203-223: Change the grouping and identity logic from `jobName` to
`jobId` in the React.useMemo hook. Replace the Map key in the
`byJob.get(f.jobName)` operation with `byJob.get(f.jobId)` and update the return
object to use `jobId` as the unique identifier while retaining `jobName` for
display purposes. Then propagate this identity change throughout the component
by updating the `hideJob` and `openCoreUpForJob` selectors to key off `jobId`
instead of `jobName`, update the job context-menu payload and state to include
both `jobId` and `jobName`, and fix any job-level comment and navigation links
that currently reference `jobName` to use `jobId` for disambiguation.

In `@cueweb/components/ui/stuck-frame-filters.tsx`:
- Around line 82-88: The onChange handler in the Input element is directly
passing Number(e.target.value) to onChange without validating that the result is
a finite number. Guard the numeric parsing by checking whether the parsed number
is finite (using Number.isFinite()) before calling onChange. Only invoke the
onChange callback with the parsed value if it represents a valid finite number,
preventing NaN or Infinity values from corrupting the filter state.
- Around line 109-114: The addFilter() function appends a filter with an empty
service value when all available services are already assigned to existing
filters. Add a guard condition to check whether an unused service was found
before calling onChange; if availableServices.find() returns undefined (meaning
no unused service exists), the function should return early without modifying
the filters state. This prevents creating duplicate or non-specific filter rows
when all services are already in use.

---

Nitpick comments:
In `@cueweb/app/api/layer/action/setmincores/route.ts`:
- Around line 39-44: The code is unnecessarily deserializing and re-serializing
the response from handleRoute. Since handleRoute already returns a properly
formatted NextResponse, remove the response.json() parsing and the subsequent
NextResponse.json() wrapper calls. Instead, return the response from handleRoute
directly, or if error handling is needed, access the response status and body
only once before returning. Eliminate the redundant JSON serialization cycle
that occurs between the handleRoute call and the final return statements.
- Around line 24-27: The method check in the route handler is redundant because
Next.js App Router automatically handles HTTP method routing through named
exports. Since this file exports a POST function, only POST requests will reach
this handler, and Next.js returns a 405 response automatically for other
methods. Remove the dead code block that checks if the request method is not
'POST' and returns a 405 error response, as this condition can never be reached
in a properly configured POST named export function.
🪄 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: 3aebbe80-4912-4fef-828e-59bc385aecf2

📥 Commits

Reviewing files that changed from the base of the PR and between 311cfb1 and e57ab7f.

📒 Files selected for processing (7)
  • cueweb/app/api/layer/action/setmincores/route.ts
  • cueweb/app/api/stuck-frames/lastline/route.ts
  • cueweb/app/api/stuck-frames/route.ts
  • cueweb/app/stuck-frames/page.tsx
  • cueweb/app/utils/action_utils.ts
  • cueweb/app/utils/get_utils.ts
  • cueweb/components/ui/stuck-frame-filters.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • cueweb/app/api/stuck-frames/route.ts

Comment thread cueweb/app/stuck-frames/page.tsx Outdated
Comment thread cueweb/app/stuck-frames/page.tsx Outdated
Comment thread cueweb/components/ui/stuck-frame-filters.tsx
Comment thread cueweb/components/ui/stuck-frame-filters.tsx
- page: key job grouping, hide, Core Up and comment links by jobId instead of jobName, so two jobs sharing a name aren't merged or acted on together.
- page / action_utils: retryFrames / eatFrames / killFrames now return a success boolean, and act() only hides the frame + reloads when the action succeeded (a failed Retry/Eat/Kill no longer removes the frame from view).
- stuck-frame-filters: ignore non-finite numeric input so a NaN can't poison filter thresholds or persisted state; don't add an empty service filter when all services are taken (and disable the add button in that case).

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (1)
cueweb/app/api/layer/action/setmincores/route.ts (1)

24-27: 💤 Low value

Redundant method check in App Router route handler.

In Next.js App Router, the exported function name (POST) determines which HTTP methods are handled. Only POST requests will ever reach this handler, making the method check at lines 24-27 unreachable dead code.

♻️ Proposed cleanup
 export async function POST(request: NextRequest) {
   const endpoint = "/job.LayerInterface/SetMinCores";
-  const method = request.method;
-  if (method !== 'POST') {
-    return NextResponse.json({ error: 'Invalid method. Only POST is allowed.' }, { status: 405 });
-  }

   let jsonBody: any;

Then update line 40:

-  const response = await handleRoute(method, endpoint, body, true);
+  const response = await handleRoute("POST", endpoint, body, true);
🤖 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 `@cueweb/app/api/layer/action/setmincores/route.ts` around lines 24 - 27, The
method check verifying if the request.method is 'POST' (the if statement at
lines 24-27) is redundant dead code in a Next.js App Router POST handler. Since
the exported function name POST automatically ensures only POST requests reach
this handler, the explicit method validation will never execute. Remove the
entire conditional block that checks if method !== 'POST' and returns the 405
error response, as the routing is already enforced by the framework.
🤖 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 `@cueweb/app/stuck-frames/page.tsx`:
- Around line 170-187: The notification in the auto-refresh useEffect fires
unconditionally on every interval completion, but should only fire when stuck
frames are present to match the documented behavior. Add a condition before the
new Notification call that checks whether stuck frames exist in the filtered
data (using the groups variable after applying any active filters), so the
notification only shows when stuck frames are actually detected during the scan.
- Around line 284-288: The openLog function does not distinguish between the
three different log viewing actions ("Tail Log", "View Log", and "View Last
Log"), even though they should access different log versions or retry states.
Modify the openLog function signature to accept an additional mode parameter,
then update all three callers of openLog to pass appropriate mode values (such
as "tail", "current", or "previous") based on which menu item was clicked.
Include the mode parameter in the URLSearchParams object passed to the frames
page so it can handle each log viewing action differently.

In `@cueweb/components/ui/stuck-frame-filters.tsx`:
- Around line 188-193: Add aria-label attributes to the two icon-only Button
components to provide accessible names for screen readers. The first Button
(with Plus icon and addFilter onClick handler) should have an aria-label with
the same text as its current title attribute ("Add a service-specific filter"),
and the second Button (with X icon and removeFilter onClick handler at line 192)
should have an aria-label matching its title ("Remove this filter"). The title
attributes can remain for tooltip functionality, but aria-label is required for
proper screen reader support on icon-only buttons.
- Around line 144-158: The service selection dropdown in the select element
allows assigning any service from availableServices to any filter row, which
violates the one-filter-per-service constraint. Modify the availableServices.map
call to filter out services that are already assigned to other filter rows
(excluding the current row at index i), so that each service can only be
selected once across all filters. This ensures users cannot create duplicate
service-specific filters by changing the selection in an existing row.

---

Nitpick comments:
In `@cueweb/app/api/layer/action/setmincores/route.ts`:
- Around line 24-27: The method check verifying if the request.method is 'POST'
(the if statement at lines 24-27) is redundant dead code in a Next.js App Router
POST handler. Since the exported function name POST automatically ensures only
POST requests reach this handler, the explicit method validation will never
execute. Remove the entire conditional block that checks if method !== 'POST'
and returns the 405 error response, as the routing is already enforced by the
framework.
🪄 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: c34e0275-f1b7-4509-9304-f5d1d2456127

📥 Commits

Reviewing files that changed from the base of the PR and between 311cfb1 and 5870b61.

📒 Files selected for processing (7)
  • cueweb/app/api/layer/action/setmincores/route.ts
  • cueweb/app/api/stuck-frames/lastline/route.ts
  • cueweb/app/api/stuck-frames/route.ts
  • cueweb/app/stuck-frames/page.tsx
  • cueweb/app/utils/action_utils.ts
  • cueweb/app/utils/get_utils.ts
  • cueweb/components/ui/stuck-frame-filters.tsx

Comment thread cueweb/app/stuck-frames/page.tsx Outdated
Comment thread cueweb/app/stuck-frames/page.tsx
Comment thread cueweb/components/ui/stuck-frame-filters.tsx Outdated
Comment thread cueweb/components/ui/stuck-frame-filters.tsx Outdated
- page: auto-refresh notification now fires only when stuck frames are actually present. load() returns the fetched data so the interval applies the same detection + hidden filters and notifies with the count, instead of firing on every scan.
- stuck-frame-filters: the service dropdown only offers the row's own service plus unused ones, preventing duplicate service-specific filters.
- stuck-frame-filters: add aria-label to the icon-only add/remove filter buttons for screen-reader support.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 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 `@cueweb/app/api/layer/action/setmincores/route.ts`:
- Around line 35-37: The validation check in the conditional statement at line
35 of the route handler only verifies that jsonBody.cores is of number type, but
does not validate that it is a valid minimum-core value. Enhance the validation
condition to additionally check that jsonBody.cores is an integer using
Number.isInteger() and that it meets the lower bound requirement (cores should
be >= 1), rejecting invalid values like 0, negative numbers, and fractional
numbers before proxying the request to the RPC layer. Modify the typeof
jsonBody.cores !== 'number' check to also include these additional constraints
in the same conditional validation block.

In `@cueweb/app/api/stuck-frames/lastline/route.ts`:
- Around line 29-36: The path validation in the handler (checking
endsWith(".rqlog") and !includes("..")) is insufficient to prevent unauthorized
file access, as it allows absolute paths and other untrusted `.rqlog` paths
outside intended log directories. Before invoking execFile on the path variable,
implement canonical path validation by resolving the path to its absolute form
and verifying it starts with an allowlisted log root directory. Define an
explicit approved root or roots for valid job log locations, then perform a
prefix check on the resolved canonical path against this allowlist before
executing the tail command.

In `@cueweb/app/stuck-frames/page.tsx`:
- Around line 460-471: The button containing the MessageSquare icon in the
stuck-frames/page.tsx file is missing an explicit accessible name. Add an
aria-label attribute to the button element to provide screen readers with a
descriptive label like "View comments". This ensures the icon-only button is
properly accessible to all users, regardless of whether they're using assistive
technologies.
- Around line 177-202: The auto-refresh effect's interval callback does not
prevent overlapping load() calls from executing concurrently. Add a flag to
track whether a load operation is currently in flight, then check this flag at
the beginning of the interval callback to skip execution if a load is already in
progress. Set the flag to true before calling load() and set it to false after
the load promise settles (in both success and error paths), ensuring that only
one scan runs at a time even if the interval fires before the previous load
completes.
🪄 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: 3cafa05b-8e81-4f68-9aa6-13d68e597912

📥 Commits

Reviewing files that changed from the base of the PR and between 311cfb1 and f7de7fb.

📒 Files selected for processing (7)
  • cueweb/app/api/layer/action/setmincores/route.ts
  • cueweb/app/api/stuck-frames/lastline/route.ts
  • cueweb/app/api/stuck-frames/route.ts
  • cueweb/app/stuck-frames/page.tsx
  • cueweb/app/utils/action_utils.ts
  • cueweb/app/utils/get_utils.ts
  • cueweb/components/ui/stuck-frame-filters.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • cueweb/app/api/stuck-frames/route.ts

Comment thread cueweb/app/api/layer/action/setmincores/route.ts Outdated
Comment thread cueweb/app/api/stuck-frames/lastline/route.ts Outdated
Comment thread cueweb/app/stuck-frames/page.tsx
Comment thread cueweb/app/stuck-frames/page.tsx
- lastline route: canonicalize the requested path with realpath (replacing the brittle ".." substring check), re-check the .rqlog extension on the real file, and restrict reads to an optional CUEWEB_LOG_ROOTS allow-list when set (unset = unrestricted, best-effort). Documented in .env.example.
- setmincores route: reject non-finite (NaN/Infinity) and negative cores; keep fractional values allowed since the proto field is a float.
- page: guard the auto-refresh interval with an in-flight flag so a slow backend can't pile up overlapping scans.
- page: add an aria-label to the comments icon button for screen readers.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🤖 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 `@cueweb/app/api/layer/action/setmincores/route.ts`:
- Around line 38-45: The validation block checking jsonBody.layer only verifies
truthiness but does not validate the shape of the layer object itself. Payloads
with an empty layer object like { layer: {}, cores: 2 } incorrectly pass
validation and get forwarded upstream. Extend the validation condition to also
check that jsonBody.layer.id exists and is a valid type (likely a string). Add
this check alongside the existing checks for layer truthiness and cores
validation to ensure the layer object has the required id property before the
request is accepted.

In `@cueweb/app/api/stuck-frames/route.ts`:
- Around line 93-94: The code assumes upstream gateway responses are arrays but
doesn't validate the payload shapes, which can cause errors if a malformed
response is received. Add guard checks before spreading or iterating over these
values in cueweb/app/api/stuck-frames/route.ts at line 93-94 (before pushing
batch with the spread operator), line 110 (guard the array before use), and
lines 124-132 (validate that layers is an array before the for loop iteration).
For each affected location, verify that the value is actually an array using
Array.isArray() and handle non-array cases gracefully by either using an empty
array fallback or skipping that batch/iteration safely, ensuring per-job
failures don't cascade to a full 500 error for the entire route.

In `@cueweb/app/stuck-frames/page.tsx`:
- Around line 392-398: The applyCoreUp function awaits the Promise.all() result
from calling setLayerMinCores on each target but does not check whether those
operations succeeded. Capture the boolean results returned by Promise.all() and
verify that all setLayerMinCores calls returned true before proceeding with
setCoreUp(null) and the subsequent load() call. If any operation fails (returns
false), return early without closing the dialog or refreshing, so the user can
retry or see the error state.
- Around line 317-322: The export buckets are currently grouped by jobName on
line 320 where db[f.jobName] is used as the key, which causes distinct jobs with
the same name to be incorrectly merged into a single JSON bucket. Replace the
jobName key with jobId (or create a composite key that uniquely identifies each
job) in both the db object access and assignment to ensure each job gets its own
separate bucket regardless of name collisions.
- Around line 156-160: The catch block in the stuck frames loading function
incorrectly masks backend/API errors by setting raw to an empty array, which
renders the empty-state UI ("No stuck frames detected…") as if the load
succeeded. Instead of calling setRaw((prev) => prev ?? []), create and manage an
explicit error state (similar to how raw is managed) to track loading failures.
When the error is caught, set this error state to a truthy value instead of
setting raw to empty, and update the component's render logic to display an
error message when this error state is set, preserving the distinction between
"loading succeeded with no results" and "loading failed due to an error".
🪄 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: 0b2864ed-e1f1-4f07-a6d5-6d336b891992

📥 Commits

Reviewing files that changed from the base of the PR and between 311cfb1 and 0fc75ae.

📒 Files selected for processing (8)
  • cueweb/.env.example
  • cueweb/app/api/layer/action/setmincores/route.ts
  • cueweb/app/api/stuck-frames/lastline/route.ts
  • cueweb/app/api/stuck-frames/route.ts
  • cueweb/app/stuck-frames/page.tsx
  • cueweb/app/utils/action_utils.ts
  • cueweb/app/utils/get_utils.ts
  • cueweb/components/ui/stuck-frame-filters.tsx
✅ Files skipped from review due to trivial changes (1)
  • cueweb/.env.example

Comment thread cueweb/app/api/layer/action/setmincores/route.ts Outdated
Comment thread cueweb/app/api/stuck-frames/route.ts Outdated
Comment thread cueweb/app/stuck-frames/page.tsx
Comment thread cueweb/app/stuck-frames/page.tsx Outdated
Comment thread cueweb/app/stuck-frames/page.tsx
- setmincores route: validate layer is an object with a non-empty string id (reject { layer: {} }) alongside the existing non-negative cores check.
- stuck-frames route: guard gateway payloads with Array.isArray before spreading/iterating (frames, jobs, layers), so a malformed upstream response degrades to empty instead of 500-ing the whole route.
- page: surface an explicit error state instead of rendering "no stuck frames" when a load fails with no data; key the log-export buckets by jobId (not jobName) so same-named jobs aren't merged; gate Core Up close/refresh on all layer updates succeeding.
@ramonfigueiredo

Copy link
Copy Markdown
Collaborator Author

@DiegoTavares / @lithorus

Ready for review!

@ramonfigueiredo

Copy link
Copy Markdown
Collaborator Author

Note: The CueWeb docs will be updated on a new PR or later in this PR.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant