chore: remove pr number from data#194
Conversation
✅ Deploy Preview for agentscan ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Review limit reached
More reviews will be available in 53 minutes and 17 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 (2)
📝 WalkthroughWalkthroughReplaces the numeric ChangesReplace pr_number with hashed pr_key
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 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 |
MatteoGabriele
force-pushed
the
chore/remove-pr-number-from-data
branch
from
75fcaf0 to
917d137
Compare
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/scan-users.ts (1)
192-213:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t swallow
hashPrIdconfiguration failures inside the profile-fetch catch block.At Line [201], if
hashPrIdthrows (e.g., missingPR_HASH_SECRET), it is reported as “Error fetching profile” and the scan continues with misleading diagnostics. This should fail fast (or be handled separately) rather than being treated as a per-user API failure.Suggested fix
- try { - const fullProfile = await octokit.rest.users.getByUsername({ - username: pr.user.login, - }); + const prKey = hashPrId(repoFullName, pr.number); + try { + const fullProfile = await octokit.rest.users.getByUsername({ + username: pr.user.login, + }); users.push({ id: fullProfile.data.id, login: fullProfile.data.login, created_at: fullProfile.data.created_at, - pr_key: hashPrId(repoFullName, pr.number), + pr_key: prKey, pr_status: pr.state, public_repos: fullProfile.data.public_repos, repo_name: repoFullName, });🤖 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 `@scripts/scan-users.ts` around lines 192 - 213, The hashPrId function call on line 201 is inside the try-catch block designed to handle octokit.rest.users.getByUsername API errors. If hashPrId throws a configuration error (like missing PR_HASH_SECRET), it gets caught and reported as a profile fetch error, which is misleading. Move the hashPrId call outside and before the try-catch block so that configuration failures fail fast instead of being swallowed as per-user API failures.
🧹 Nitpick comments (1)
test/unit/shared/utils/charts.test.ts (1)
225-231: ⚡ Quick winAvoid casting partial test objects to
EcosystemHealthItem.Using
as EcosystemHealthItemhere bypasses required-field checks and can mask contract drift. Prefer filling required defaults in the helper and returning a fully typed object without assertion.Suggested change
function createEcosystemHealthItem( created_at: string, repo_name: string, pr_key: string, pr_status: "open" | "closed", score: number, ): EcosystemHealthItem { return { created_at, repo_name, pr_key, pr_status, score, - } as EcosystemHealthItem; + user_created_at: "1970-01-01T00:00:00Z", + user_public_repos_count: 0, + events_count: 0, + }; }🤖 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 `@test/unit/shared/utils/charts.test.ts` around lines 225 - 231, Remove the `as EcosystemHealthItem` type assertion from the return statement in this test helper function. Instead, identify all required fields of the EcosystemHealthItem type and provide sensible default values for any missing properties beyond the current five (created_at, repo_name, pr_key, pr_status, score). Return the fully populated object without casting to ensure the test object matches the actual contract of EcosystemHealthItem and prevents future contract drift from going unnoticed.
🤖 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 `@scripts/pr-hash.ts`:
- Around line 5-12: The hashPrId function needs to validate that the prId
parameter is a finite number before proceeding with the hash calculation. Add a
validation check after the secret validation (after the PR_HASH_SECRET
environment variable check) to ensure prId is a finite number using the
Number.isFinite method, and throw an Error if the validation fails. This
prevents non-finite values like NaN or Infinity from being hashed and persisted
as invalid pr_key identifiers.
In `@shared/utils/charts.ts`:
- Around line 107-110: The guard condition checking if repo and prId are truthy
occurs after the String() coercion, but String(undefined) and String(null)
produce truthy strings ("undefined" and "null"), making the check ineffective.
Move the guard condition to check the raw values entry[repoKey] and entry[prKey]
for truthiness BEFORE applying String() coercion, so that undefined or null
values are properly filtered out before being used for deduplication.
---
Outside diff comments:
In `@scripts/scan-users.ts`:
- Around line 192-213: The hashPrId function call on line 201 is inside the
try-catch block designed to handle octokit.rest.users.getByUsername API errors.
If hashPrId throws a configuration error (like missing PR_HASH_SECRET), it gets
caught and reported as a profile fetch error, which is misleading. Move the
hashPrId call outside and before the try-catch block so that configuration
failures fail fast instead of being swallowed as per-user API failures.
---
Nitpick comments:
In `@test/unit/shared/utils/charts.test.ts`:
- Around line 225-231: Remove the `as EcosystemHealthItem` type assertion from
the return statement in this test helper function. Instead, identify all
required fields of the EcosystemHealthItem type and provide sensible default
values for any missing properties beyond the current five (created_at,
repo_name, pr_key, pr_status, score). Return the fully populated object without
casting to ensure the test object matches the actual contract of
EcosystemHealthItem and prevents future contract drift from going unnoticed.
🪄 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: f5f01d7a-52bf-4457-a1a4-6d6515f1b17d
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
data/scan-results.jsonpackage.jsonscripts/migrate-pr-numbers.tsscripts/pr-hash.tsscripts/scan-users.tsshared/types/ecosystem-health.tsshared/utils/charts.tstest/unit/mocks/ecosystemHealthItems.tstest/unit/shared/utils/charts.test.ts
✅ Files skipped from review due to trivial changes (2)
- scripts/migrate-pr-numbers.ts
- package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- shared/types/ecosystem-health.ts
|
yolo style 😎 |
|
This is us after merging. |
2 participants
Summary by CodeRabbit
pr_key) instead of a numeric PR id (pr_number).pr_keyfor consistent deduplication.prKey.pr_keyformat.pr_keyinstead ofpr_number.