fix: resolve canvas performance degradation #13460#13759
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughTwo monitor API endpoints ( ChangesMonitor API Pagination and Frontend Query Gating
Scratch Debug File
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 3 warnings)
✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
scratch.mjs (1)
1-2: ⚡ Quick winRemove the leftover debug scratch file from this PR.
This import +
console.logappears to be debugging-only and unrelated to the performance fix, so it should be dropped before merge.Suggested cleanup
-import { BASENAME } from "./src/frontend/src/customization/config-constants.ts"; -console.log(BASENAME);🤖 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 `@scratch.mjs` around lines 1 - 2, Delete the scratch.mjs file entirely as it contains only debugging code with an import of BASENAME and a console.log statement that are unrelated to the performance fix and should not be included in the final commit.
🤖 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 `@src/backend/base/langflow/api/v1/monitor.py`:
- Around line 231-232: The pagination logic at multiple locations (including
lines 267-270, 536-537, and 564-567) uses truthy checks on the limit parameter,
which means limit=0 is treated as falsy and skips the pagination guard entirely.
Replace all truthy checks (if limit:) with explicit `is not None` checks to
properly validate the limit parameter. Additionally, add bounds validation to
ensure the limit parameter is greater than 0 before applying it in the query,
preventing zero values from bypassing pagination and causing full-table scans.
This applies to all locations where pagination is applied to query results.
- Around line 261-266: Validate both the order_by and order parameters in the
get_messages function before constructing the ORDER BY clause at line 261. Add
validation to ensure order_by is a valid column name on the MessageTable class,
raising a 400 Bad Request error if it is not, and validate that order is either
"asc" or "desc" (case-insensitive), defaulting to "asc" or raising an error for
invalid values. This prevents the unguarded getattr call from triggering server
errors and ensures the order parameter is explicitly handled rather than
silently defaulting for any non-desc value.
---
Nitpick comments:
In `@scratch.mjs`:
- Around line 1-2: Delete the scratch.mjs file entirely as it contains only
debugging code with an import of BASENAME and a console.log statement that are
unrelated to the performance fix and should not be included in the final commit.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1dcdef60-fe2d-4104-92ed-2e6a22ea2b0f
📒 Files selected for processing (3)
scratch.mjssrc/backend/base/langflow/api/v1/monitor.pysrc/frontend/src/components/core/playgroundComponent/chat-view/chat-messages/hooks/use-chat-history.ts
| limit: Annotated[int | None, Query()] = 30, | ||
| offset: Annotated[int | None, Query()] = 0, |
There was a problem hiding this comment.
limit=0 currently disables pagination and can trigger full-table scans.
At Lines 267-270 and 564-567, pagination is applied only on truthy values. Passing limit=0 skips .limit(...) entirely, which defeats the new pagination guard and can cause heavy queries. Use bounds and is not None checks.
Suggested fix
- limit: Annotated[int | None, Query()] = 30,
- offset: Annotated[int | None, Query()] = 0,
+ limit: Annotated[int, Query(ge=1, le=200)] = 30,
+ offset: Annotated[int, Query(ge=0)] = 0,
...
- if offset:
- stmt = stmt.offset(offset)
- if limit:
- stmt = stmt.limit(limit)
+ stmt = stmt.offset(offset).limit(limit)Also applies to: 267-270, 536-537, 564-567
🤖 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 `@src/backend/base/langflow/api/v1/monitor.py` around lines 231 - 232, The
pagination logic at multiple locations (including lines 267-270, 536-537, and
564-567) uses truthy checks on the limit parameter, which means limit=0 is
treated as falsy and skips the pagination guard entirely. Replace all truthy
checks (if limit:) with explicit `is not None` checks to properly validate the
limit parameter. Additionally, add bounds validation to ensure the limit
parameter is greater than 0 before applying it in the query, preventing zero
values from bypassing pagination and causing full-table scans. This applies to
all locations where pagination is applied to query results.
| order_col = getattr(MessageTable, order_by) | ||
| if order and order.lower() == "desc": | ||
| order_col = order_col.desc() | ||
| else: | ||
| order_col = order_col.asc() | ||
| stmt = stmt.order_by(order_col) |
There was a problem hiding this comment.
Validate order_by and order in get_messages before building the ORDER BY clause.
At Line 261, getattr(MessageTable, order_by) is unguarded. Invalid values can trigger server errors instead of a 4xx client error, and order currently accepts any string (non-desc silently becomes ascending). Mirror the shared-endpoint validation path.
Suggested fix
+ allowed_order_fields = {"timestamp", "sender", "sender_name", "session_id", "text"}
if order_by:
+ if order_by not in allowed_order_fields:
+ raise HTTPException(status_code=400, detail=f"Invalid order_by field: {order_by}")
+ if order not in {"asc", "desc"}:
+ raise HTTPException(status_code=400, detail=f"Invalid order: {order}")
order_col = getattr(MessageTable, order_by)
- if order and order.lower() == "desc":
+ if order == "desc":
order_col = order_col.desc()
else:
order_col = order_col.asc()
stmt = stmt.order_by(order_col)🤖 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 `@src/backend/base/langflow/api/v1/monitor.py` around lines 261 - 266, Validate
both the order_by and order parameters in the get_messages function before
constructing the ORDER BY clause at line 261. Add validation to ensure order_by
is a valid column name on the MessageTable class, raising a 400 Bad Request
error if it is not, and validate that order is either "asc" or "desc"
(case-insensitive), defaulting to "asc" or raising an error for invalid values.
This prevents the unguarded getattr call from triggering server errors and
ensures the order parameter is explicitly handled rather than silently
defaulting for any non-desc value.
- Add _VALID_ORDER_BY_FIELDS whitelist {'timestamp', 'sender', 'sender_name',
'session_id', 'flow_id'} and validate order_by before calling getattr(),
returning HTTP 422 for unknown fields instead of a 500 AttributeError.
- Change 'if limit:' to 'if limit is not None:' so that limit=0 (fetch all)
is applied correctly rather than silently skipped; add a bounds check that
rejects negative values with HTTP 422.
Addresses CodeRabbit review comments on PR langflow-ai#13759.
✅ CodeRabbit Review Fixes AppliedThank you for the detailed review! Both flagged issues have been addressed: 1.
|
github-actions
Bot
added
bug
|
Thanks for the review! I've hardened |
1 participant
Summary by CodeRabbit
New Features
Improvements