Skip to content

fix: cache merged pull requests immediately - #24

Merged
anurag629 merged 2 commits into
codercops:developfrom
VedantKhairnar:fix/cache-merged-pr-leaderboard
Oct 18, 2026
Merged

anurag629 merged 2 commits into
codercops:developfrom
VedantKhairnar:fix/cache-merged-pr-leaderboard

Conversation

@VedantKhairnar

Copy link
Copy Markdown
Contributor

Closes #9

Caches a freshly ranked leaderboard after a counted merge, adding the webhook PR when GitHub search has not indexed it yet. If search fails, the old cache is cleared and the merge shoutout still posts.

How I tested this

  • npm run typecheck
  • npm test (190 tests)

AI assistance

  • Used Codex to help implement the scoped cache update and test coverage; reviewed the diff and verified it locally.

Copilot AI balanced review requested due to automatic review settings October 15, 2026 06:30

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

@anurag629 anurag629 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @VedantKhairnar, the change itself is right. I tried it on top of current main (213 tests pass, typecheck clean), and both cache checks fail on main's code like they should. Dropping the append, appending every time and skipping the cache write each fail a test too.

One thing before I merge: the issue also asks for a test that the handler makes no more GitHub requests than before, and nothing checks that yet. If I add a second searchMergedPrs call to onPullRequest, every test still passes. One line in the first merged PR test is enough:

expect(fetched.calls.filter((c) => c.url.startsWith("https://api.github.com/"))).toHaveLength(1);

I tried it: it passes with your change and fails with the extra call.

@VedantKhairnar

Copy link
Copy Markdown
Contributor Author

Addressed in dee4e5d: the lagging-search regression test now asserts that the handler makes exactly one GitHub API request. npm run typecheck and npm test both pass locally (190 tests).

@anurag629 anurag629 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @VedantKhairnar, that's the one. With a second searchMergedPrs call in onPullRequest the first merged PR test fails now, and without it typecheck and all 222 tests pass on top of current develop. Merging.

It's your first merged PR here, so come say hi on our Discord too: https://discord.gg/mpGdJyjaRQ

@anurag629 anurag629 added the hacktoberfest-accepted PR counts for Hacktoberfest label Oct 18, 2026
@anurag629
anurag629 merged commit 31dc339 into codercops:develop Oct 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

hacktoberfest-accepted PR counts for Hacktoberfest

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Show a merged PR on the leaderboard right away

3 participants