feat: paginate runStats, fix verification script on Windows, and clean up react-hooks warning#123
Conversation
…olve React hook state effect warning
📝 WalkthroughWalkthroughThis PR introduces cursor-based pagination for run queries to improve scalability, adds supporting database indexes, defers cell flash updates in the table UI, and improves test script compatibility. The pagination work includes constants for page size bounds, updates to Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@frontend/convex/runStats.ts`:
- Around line 84-94: The current code only caps the upper bound for args.limit
but can pass zero, negative, fractional or NaN to paginate; update the
computation of limit (used before ctx.db.query(...).paginate and the similar
block around paginate at lines ~110-118) to normalize and clamp numItems to an
integer in [1, MAX_PAGE_SIZE] by coercing args.limit to a number, using
Math.floor (or equivalent) and Math.max(1, ...), then Math.min(...,
MAX_PAGE_SIZE) so paginate always receives a positive integer; reference the
variables/functions limit, args.limit, DEFAULT_PAGE_SIZE, MAX_PAGE_SIZE and the
paginate call in runStats.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9de7d92b-f3e9-4212-a582-81a032afec81
📒 Files selected for processing (4)
frontend/components/table/use-row-change-detection.tsfrontend/convex/runStats.tsfrontend/convex/schema.tsscripts/verify-authz.sh
| const limit = Math.min(args.limit ?? DEFAULT_PAGE_SIZE, MAX_PAGE_SIZE); | ||
| const { page, isDone, continueCursor } = await ctx.db | ||
| .query("runStats") | ||
| .withIndex("by_dataset", (q) => q.eq("datasetId", args.datasetId)) | ||
| .collect(); | ||
| return runs.sort((a, b) => b.startedAt - a.startedAt); | ||
| .withIndex("by_dataset_started_at", (q) => | ||
| q.eq("datasetId", args.datasetId), | ||
| ) | ||
| .order("desc") | ||
| .paginate({ | ||
| cursor: args.cursor ?? null, | ||
| numItems: limit, | ||
| }); |
There was a problem hiding this comment.
Clamp and normalize numItems before calling paginate.
Line 84 and Line 110 only cap the upper bound. Passing limit <= 0 (or a fractional value) can trigger runtime failures or undefined pagination behavior.
Suggested fix
- const limit = Math.min(args.limit ?? DEFAULT_PAGE_SIZE, MAX_PAGE_SIZE);
+ const requested = args.limit ?? DEFAULT_PAGE_SIZE;
+ const limit = Math.max(1, Math.min(Math.floor(requested), MAX_PAGE_SIZE));
@@
- const limit = Math.min(args.limit ?? DEFAULT_PAGE_SIZE, MAX_PAGE_SIZE);
+ const requested = args.limit ?? DEFAULT_PAGE_SIZE;
+ const limit = Math.max(1, Math.min(Math.floor(requested), MAX_PAGE_SIZE));Also applies to: 110-118
🤖 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 `@frontend/convex/runStats.ts` around lines 84 - 94, The current code only caps
the upper bound for args.limit but can pass zero, negative, fractional or NaN to
paginate; update the computation of limit (used before
ctx.db.query(...).paginate and the similar block around paginate at lines
~110-118) to normalize and clamp numItems to an integer in [1, MAX_PAGE_SIZE] by
coercing args.limit to a number, using Math.floor (or equivalent) and
Math.max(1, ...), then Math.min(..., MAX_PAGE_SIZE) so paginate always receives
a positive integer; reference the variables/functions limit, args.limit,
DEFAULT_PAGE_SIZE, MAX_PAGE_SIZE and the paginate call in runStats.
1 participant
Description
This PR addresses three key areas to improve scalability, cross-platform local development, and clean up frontend compilation/linting:
listByDatasetandlistByUserqueries on therunStatstable in Convex to use cursor-based pagination (.paginate()) instead of loading all historic records into memory (.collect()). This bounds memory usage and improves database performance.scripts/verify-authz.shto check forpython3and fallback topython(which is standard on Windows hosts).cadencepayload argument inside the verification script torefreshCadencefordatasets:create. This fixed a schema validation exception on Convex that was previously masking auth checks.react-hooks/set-state-in-effectrule violation inuse-row-change-detection.tsby wrappingsetFlashingCellsinsidesetTimeout(..., 0)to align with the repository's established code patterns.Changes Made
by_dataset_started_at(["datasetId", "startedAt"]) andby_user_started_at(["userId", "startedAt"]) infrontend/convex/schema.ts.listByDatasetandlistByUserto cursor-based paginated queries infrontend/convex/runStats.ts.scripts/verify-authz.shwith the Python binary checker and updated testing arguments.frontend/components/table/use-row-change-detection.tsin deferred callbacks.Verification