Conversation
- Increase batch size from 40 to 500 to reduce DB round-trips at volume - Add moveAndFetchRequestLogs Lua script that atomically moves items to the processing list and returns them in a single Redis round-trip, replacing a separate move + read call per batch - Reduce partial flush age threshold from 10 to 2 minutes so low-traffic logs reach Postgres within ~4 minutes instead of ~12 - Add bulkEnqueueRequestLogs using MULTI/EXEC pipeline for test setup; update flush tests to use it and drive counts from REQUEST_LOG_BATCH_SIZE Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 18 minutes and 9 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughVersion bumped to 1.6.0. Input parsing (float/int coercion, clamping, NaN filtering) moved from weather and log controllers into express-validator schemas. Two new Redis helpers added ( ChangesInput Validation & Controller Simplification
Redis Queue & Flush Job Refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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
🧹 Nitpick comments (1)
src/tests/flush-request-logs.test.mjs (1)
24-32: ⚡ Quick winAdd
stable_idto the test log factory to cover dedupe-key behavior.These tests now validate batch flushing, but the generated log payloads skip
stable_id, so they don’t exercise the uniqueness/deduplication contract used by the flush path.Suggested patch
const makeLog = (index, overrides = {}) => ({ + stable_id: `test-stable-id-${index}`, ip: '127.0.0.1', route: `/logs/${index}`, method: 'GET', code: 200, type: 'INFO', created_at: new Date().toISOString(), ...overrides, });As per coding guidelines,
src/jobs/flush-request-logs.mjs: "Use bulkCreate(..., { ignoreDuplicates: true }) withstable_idfor request log deduplication".🤖 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/tests/flush-request-logs.test.mjs` around lines 24 - 32, The makeLog factory function in the test file does not include a `stable_id` property in the returned log object, which means the test logs don't exercise the deduplication behavior implemented in the flush-request-logs code. Add a `stable_id` field to the object returned by the makeLog function, using the index parameter (or a derived value from it) to generate a stable identifier that can be used for deduplication testing. This ensures the test payloads properly cover the uniqueness contract used by the flush path's bulkCreate call with ignoreDuplicates.Source: Coding guidelines
🤖 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/utils/validationSchemas.mjs`:
- Around line 41-47: The units validation in the validationSchemas.mjs file
currently accepts any non-imperial value as true, allowing invalid inputs to
pass silently. Replace the customSanitizer with explicit validation that only
allows the values 'metric' or 'imperial' using an isIn validator, and add a
default value option to ensure the contract is strict and predictable. This
prevents invalid values like 'units=foo' from being accepted without proper
validation.
---
Nitpick comments:
In `@src/tests/flush-request-logs.test.mjs`:
- Around line 24-32: The makeLog factory function in the test file does not
include a `stable_id` property in the returned log object, which means the test
logs don't exercise the deduplication behavior implemented in the
flush-request-logs code. Add a `stable_id` field to the object returned by the
makeLog function, using the index parameter (or a derived value from it) to
generate a stable identifier that can be used for deduplication testing. This
ensures the test payloads properly cover the uniqueness contract used by the
flush path's bulkCreate call with ignoreDuplicates.
🪄 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: d6588d03-4e5d-423c-ae11-ebfdac7e0e1f
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
package.jsonsrc/controllers/info.controller.mjssrc/controllers/v1/errorLogs.controller.mjssrc/controllers/v1/requestLogs.controller.mjssrc/controllers/v1/weather.controller.mjssrc/jobs/flush-request-logs.mjssrc/routes/v1/weather.route.mjssrc/services/redis.service.mjssrc/tests/flush-request-logs.test.mjssrc/utils/validationSchemas.mjs
Express 5 changed req.query to a dynamic getter that re-parses the URL on every access, so express-validator's sanitizer mutations (customSanitizer, default, toInt) were written to a temporary object and immediately lost. Switch requestLogs and errorLogs controllers to use matchedData(req) from express-validator, which reads from its internal store rather than req.query. Page defaults to 1 via destructuring since matchedData omits unprovided optional fields. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Chores