[cueweb/docs] Add Limits page (CueCommander parity)#2412
Conversation
Replicates the CueGUI CueCommander Limits window at /limits (the sidebar entry previously 404'd), with the full set of limit operations.
- New proxy routes under app/api/limit/: getall -> limit.LimitInterface/GetAll (single-level {limits:[...]} unwrap), plus action routes create / delete / rename / setmaxvalue. All validate the body (400 on malformed JSON / missing or mistyped fields; setmaxvalue requires a non-negative integer) and propagate the gateway's real HTTP status.
- get_utils: Limit type and getLimits().
- action_utils: createLimit / deleteLimit / renameLimit / setLimitMaxValue (all keyed on the limit name, per the proto) and the row dispatchers; limit-action-events.ts holds the shared dialog event contract.
- SimpleDataTable gains an isLimitsTable variant with the LimitContextMenu (Edit Max Value, Delete Limit, Rename); limit-columns.tsx renders Limit Name, Max Value, Current Running.
- app/limits/page.tsx: the table with Refresh + Add Limit header buttons, 30s auto-refresh, loading skeletons, and an inline error + Retry state; re-fetches on cueweb:limits-changed.
- Dialogs mirroring CueGUI: Add Limit, Edit Max Value (validates a non-negative integer), Rename a Limit, and a Delete confirmation.
- Tests: Jest coverage for the limit action helpers and getLimits.
Documents CueWeb's CueCommander Limits page across the CueWeb docs.
- User guide (cueweb-user-guide.md): new Limits section with the menu and page screenshots, column descriptions, Add Limit (dialog + confirmation), and the row actions - Edit Max Value, Rename, Delete - each with its dialog screenshot.
- Overview (other-guides/cueweb.md): add a Limits feature entry.
- Reference (reference/cueweb.md): add a Limits behavior section (data source, columns, Add Limit, row actions, the name-keyed action helpers and the single-level {limits:[...]} unwrap), list the limit RPCs and the /api/limit/... proxy routes, and mark the Limits route implemented in the roadmap.
- Developer guide (cueweb-development.md): note the SimpleDataTable isLimitsTable flag / LimitContextMenu.
- Add Limits screenshots (light + dark) for the page, menu, Add Limit, and the Edit Max Value / Rename / Delete dialogs.
📝 WalkthroughWalkthroughAdds a complete Limits feature: Limit type and getLimits(), Next.js proxy routes for limit RPCs, action helpers and event contracts, event-driven dialogs (add/edit/delete/rename), LimitsPage with polling and table integration, context menu, tests, and documentation. ChangesLimits Management Feature
Sequence DiagramsequenceDiagram
participant User as User / UI
participant LimitsPage as LimitsPage
participant Dialog as Dialog Component
participant Actions as action_utils
participant Route as Next.js Route
participant RPC as limit.LimitInterface RPC
participant Toast as Toast Notification
User->>LimitsPage: open /limits
LimitsPage->>Route: POST /api/limit/getall
Route->>RPC: /limit.LimitInterface/GetAll
RPC-->>Route: { data.limits[] }
Route-->>LimitsPage: { data: limits[] }
LimitsPage->>LimitsPage: render table, start 30s poll
User->>LimitsPage: right-click row → Edit Max Value
LimitsPage->>Actions: editLimitMaxValueGivenRow(row)
Actions->>Dialog: dispatch OPEN_LIMIT_EDIT_MAX_VALUE_EVENT
Dialog->>Dialog: open, populate
User->>Dialog: enter new max value and confirm
Dialog->>Actions: setLimitMaxValue(name, value)
Actions->>Route: POST /api/limit/action/setmaxvalue
Route->>RPC: /limit.LimitInterface/SetMaxValue
RPC-->>Route: { data }
Route-->>Actions: success
Actions->>Dialog: resolve true
Dialog->>Toast: show success
Dialog->>LimitsPage: dispatch LIMITS_CHANGED_EVENT
LimitsPage->>Route: POST /api/limit/getall (refresh)
Route->>RPC: /limit.LimitInterface/GetAll
RPC-->>Route: { data.limits[] }
Route-->>LimitsPage: { data: limits[] }
Dialog->>Dialog: close
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
cueweb/components/ui/simple-data-table.tsx (1)
522-523: ⚡ Quick winConsider extracting the variant-specific text to a helper function.
The nested ternary chains for
placeholderandaria-labelare growing long and harder to maintain as new table variants are added. While this follows the existing pattern, extracting to a helper would improve readability.♻️ Optional refactor to improve maintainability
+const getFilterText = (variant: { + isHostsTable?: boolean; + isProcsTable?: boolean; + isShowsTable?: boolean; + isAllocationsTable?: boolean; + isLimitsTable?: boolean; + isFramesTable?: boolean; + isFramesLogTable?: boolean; +}) => { + if (variant.isHostsTable) return { placeholder: "Filter hosts...", label: "Filter hosts" }; + if (variant.isProcsTable) return { placeholder: "Filter procs...", label: "Filter procs" }; + if (variant.isShowsTable) return { placeholder: "Filter shows...", label: "Filter shows" }; + if (variant.isAllocationsTable) return { placeholder: "Filter allocations...", label: "Filter allocations" }; + if (variant.isLimitsTable) return { placeholder: "Filter limits...", label: "Filter limits" }; + if (variant.isFramesTable || variant.isFramesLogTable) return { placeholder: "Filter frames...", label: "Filter frames" }; + return { placeholder: "Filter layers...", label: "Filter layers" }; +}; + export function SimpleDataTable<TData, TValue>({ // ... props }: SimpleDataTableProps<TData, TValue>) { // ... existing code + const filterText = getFilterText({ + isHostsTable, + isProcsTable, + isShowsTable, + isAllocationsTable, + isLimitsTable, + isFramesTable, + isFramesLogTable, + }); // ... in the render: <Input type="search" value={globalFilter} onChange={(e) => setGlobalFilter(e.target.value)} - placeholder={isHostsTable ? "Filter hosts..." : isProcsTable ? "Filter procs..." : isShowsTable ? "Filter shows..." : isAllocationsTable ? "Filter allocations..." : isLimitsTable ? "Filter limits..." : (isFramesTable || isFramesLogTable) ? "Filter frames..." : "Filter layers..."} - aria-label={isHostsTable ? "Filter hosts" : isProcsTable ? "Filter procs" : isShowsTable ? "Filter shows" : isAllocationsTable ? "Filter allocations" : isLimitsTable ? "Filter limits" : (isFramesTable || isFramesLogTable) ? "Filter frames" : "Filter layers"} + placeholder={filterText.placeholder} + aria-label={filterText.label} className="h-8 w-44 pl-7 pr-7 text-xs" />🤖 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/components/ui/simple-data-table.tsx` around lines 522 - 523, The nested ternary chains used to compute the placeholder and aria-label make the JSX hard to read; extract that logic into a small helper function (e.g., getFilterText or getFilterLabel) inside the SimpleDataTable component that accepts the same boolean flags (isHostsTable, isProcsTable, isShowsTable, isAllocationsTable, isLimitsTable, isFramesTable, isFramesLogTable) or a single tableType value and returns the correct string, then replace the long ternary expressions for placeholder and aria-label with calls to that helper (use the helper twice or provide two helpers getFilterPlaceholder/getFilterAriaLabel if you need different text).
🤖 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/limit/action/create/route.ts`:
- Around line 34-42: Add server-side validation in the create route to ensure
jsonBody.max_value is a non-negative integer: when validating the request body
in the create route (the block that checks jsonBody, jsonBody.name and
jsonBody.max_value and returns NextResponse.json on error), extend the check to
verify Number.isInteger(jsonBody.max_value) and jsonBody.max_value >= 0; if it
fails, return a 400 with a clear message (consistent with setmaxvalue/route.ts
which enforces max_value >= 0) so fractional or negative max_value values are
rejected on create.
In `@cueweb/app/api/limit/action/delete/route.ts`:
- Around line 34-38: The delete route currently validates and forwards
jsonBody.name without trimming, which can mismatch names normalized during
creation; update the validation and payload assembly in the route (check
jsonBody and jsonBody.name) to trim the name first (e.g., const name =
String(jsonBody.name).trim()), validate that trimmed name is non-empty, and then
use that trimmed name when creating the JSON payload (replace usage of
jsonBody.name with the trimmed variable) so deletion matches creation
normalization.
In `@cueweb/app/api/limit/action/rename/route.ts`:
- Around line 34-45: Validation and body construction currently only trim
new_name, causing mismatches when old_name has surrounding whitespace; update
the validation logic to trim jsonBody.old_name before checking length (mirror
the new_name check) and construct the request payload using old_name:
jsonBody.old_name.trim() and new_name: jsonBody.new_name.trim() so both names
are normalized (refer to jsonBody checks and the const body assignment in
route.ts).
In `@cueweb/app/api/limit/action/setmaxvalue/route.ts`:
- Around line 35-44: The request validation in route.ts currently allows
fractional max_value and doesn't trim name; update the check to ensure
jsonBody.max_value is an integer (e.g., require
Number.isInteger(jsonBody.max_value) and >= 0) and trim the name before
using/forwarding it (e.g., const name = String(jsonBody.name).trim()) so you
validate against the trimmed value and use that trimmed name for RPC/DB calls;
keep the NextResponse.json error path but adjust its condition to include the
integer check and empty-trimmed-name check.
In `@cueweb/app/limits/page.tsx`:
- Around line 64-69: The event handler registered for LIMITS_CHANGED_EVENT calls
load() directly, which can call setLimits/setError after the component unmounts;
add a component-level mounted ref (e.g., mountedRef) in the page component, set
mountedRef.current = true on mount and false in the cleanup, and update load
(and any callers) to check mountedRef.current before calling setLimits or
setError; ensure the LIMITS_CHANGED_EVENT listener uses the same load function
so the mountedRef guard prevents state updates from event-driven loads after
unmount.
In `@docs/_docs/reference/cueweb.md`:
- Around line 1166-1167: Update the wording describing the Limits page so the
header action is clearly separated from the row context menu: in the sentence
referencing "Limits (`/limits`) - implemented; limits table with Add Limit and
Edit Max Value / Rename / Delete row actions", change it to mention "Add Limit"
as a header button and list "Edit Max Value / Rename / Delete" explicitly as
row/context-menu actions (e.g., "limits table with Add Limit header button and
row actions: Edit Max Value / Rename / Delete (see [Limits](`#limits`))") so "Add
Limit", "Edit Max Value", "Rename", and "Delete" are correctly classified.
In `@docs/_docs/user-guides/cueweb-user-guide.md`:
- Around line 1117-1127: Add dark-mode image embeds for the Limits section in
cueweb-user-guide.md by duplicating each existing light-mode embed and using the
corresponding *_dark.png filenames (e.g.,
cueweb_cuecommander_limits_menu_dark.png and
cueweb_cuecommander_limits_dark.png), placing each dark-mode image immediately
after its light-mode counterpart following the same pattern used in the Monitor
Hosts section; do the same for related dialog/menu images (e.g.,
add_limit_dialog_dark.png, per-row_actions_menu_dark.png) so every light image
in the Limits page has a matching dark-mode embed.
---
Nitpick comments:
In `@cueweb/components/ui/simple-data-table.tsx`:
- Around line 522-523: The nested ternary chains used to compute the placeholder
and aria-label make the JSX hard to read; extract that logic into a small helper
function (e.g., getFilterText or getFilterLabel) inside the SimpleDataTable
component that accepts the same boolean flags (isHostsTable, isProcsTable,
isShowsTable, isAllocationsTable, isLimitsTable, isFramesTable,
isFramesLogTable) or a single tableType value and returns the correct string,
then replace the long ternary expressions for placeholder and aria-label with
calls to that helper (use the helper twice or provide two helpers
getFilterPlaceholder/getFilterAriaLabel if you need different text).
🪄 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: 3538995c-e467-49a4-bd8b-331740547f8d
⛔ Files ignored due to path filters (16)
docs/assets/images/cueweb/cueweb_cuecommander_limits.pngis excluded by!**/*.pngdocs/assets/images/cueweb/cueweb_cuecommander_limits_add_limit.pngis excluded by!**/*.pngdocs/assets/images/cueweb/cueweb_cuecommander_limits_add_limit_confirmation.pngis excluded by!**/*.pngdocs/assets/images/cueweb/cueweb_cuecommander_limits_add_limit_confirmation_dark.pngis excluded by!**/*.pngdocs/assets/images/cueweb/cueweb_cuecommander_limits_add_limit_dark.pngis excluded by!**/*.pngdocs/assets/images/cueweb/cueweb_cuecommander_limits_dark.pngis excluded by!**/*.pngdocs/assets/images/cueweb/cueweb_cuecommander_limits_menu.pngis excluded by!**/*.pngdocs/assets/images/cueweb/cueweb_cuecommander_limits_menu_dark.pngis excluded by!**/*.pngdocs/assets/images/cueweb/cueweb_cuecommander_limits_menu_options.pngis excluded by!**/*.pngdocs/assets/images/cueweb/cueweb_cuecommander_limits_menu_options_dark.pngis excluded by!**/*.pngdocs/assets/images/cueweb/cueweb_cuecommander_limits_menu_options_delete_selected_limit.pngis excluded by!**/*.pngdocs/assets/images/cueweb/cueweb_cuecommander_limits_menu_options_delete_selected_limit_dark.pngis excluded by!**/*.pngdocs/assets/images/cueweb/cueweb_cuecommander_limits_menu_options_edit_max_value.pngis excluded by!**/*.pngdocs/assets/images/cueweb/cueweb_cuecommander_limits_menu_options_edit_max_value_dark.pngis excluded by!**/*.pngdocs/assets/images/cueweb/cueweb_cuecommander_limits_menu_options_rename_a_limit.pngis excluded by!**/*.pngdocs/assets/images/cueweb/cueweb_cuecommander_limits_menu_options_rename_a_limit_dark.pngis excluded by!**/*.png
📒 Files selected for processing (22)
cueweb/app/__tests__/api/utils/limit_action_utils.test.tscueweb/app/__tests__/utils/limit_get_utils.test.tscueweb/app/api/limit/action/create/route.tscueweb/app/api/limit/action/delete/route.tscueweb/app/api/limit/action/rename/route.tscueweb/app/api/limit/action/setmaxvalue/route.tscueweb/app/api/limit/getall/route.tscueweb/app/limits/limit-columns.tsxcueweb/app/limits/page.tsxcueweb/app/utils/action_utils.tscueweb/app/utils/get_utils.tscueweb/components/ui/context_menus/action-context-menu.tsxcueweb/components/ui/limit-action-events.tscueweb/components/ui/limit-add-dialog.tsxcueweb/components/ui/limit-delete-dialog.tsxcueweb/components/ui/limit-edit-max-value-dialog.tsxcueweb/components/ui/limit-rename-dialog.tsxcueweb/components/ui/simple-data-table.tsxdocs/_docs/developer-guide/cueweb-development.mddocs/_docs/other-guides/cueweb.mddocs/_docs/reference/cueweb.mddocs/_docs/user-guides/cueweb-user-guide.md
|
Ready for review! |
# Conflicts: # docs/_docs/reference/cueweb.md
be04c42
into
AcademySoftwareFoundation:master
Related Issues
Main issue:
Issues related to this PR:
Summarize your change.
[cueweb] Add Limits page (CueCommander parity)
Replicates the CueGUI CueCommander Limits window at /limits (the sidebar entry previously 404'd), with the full set of limit operations.
[cueweb/docs] Document the Limits page
Documents CueWeb's CueCommander Limits page across the CueWeb docs.
LLM usage disclosure
Parts of this solution's implementation were developed with assistance from Claude Opus.