Skip to content

I solved the streak issue(#918) - #943

Closed
aryansingh42046 wants to merge 1 commit into
DenverCoder1:mainfrom
aryansingh42046:igris
Closed

aryansingh42046 wants to merge 1 commit into
DenverCoder1:mainfrom
aryansingh42046:igris

Conversation

@aryansingh42046

Copy link
Copy Markdown

This change fixes an issue in the streak calculation caused by contribution dates from adjacent yearly GitHub calendars being merged without guaranteed chronological ordering. When overlapping calendar data was processed out of order, the streak algorithm could incorrectly skip zero-contribution days and report an inflated streak.

The merged contribution dates are now sorted chronologically before streak statistics are calculated. A regression test was added to cover overlapping yearly calendars and verify that the current streak is calculated correctly. The FAQ was also updated to explain how the timezone option can be used to ensure current-day checks match the user’s local timezone.

Type of change
Bug fix (added a non-breaking change which fixes an issue)
New feature (added a non-breaking change which adds functionality)
Updated documentation (updated the readme, templates, or other repo files)
Breaking change (fix or feature that would cause existing functionality to work differently)

How Has This Been Tested?
Tested locally with a valid username
Tested locally with an invalid username
Ran tests with composer test — could not run because PHP and Composer are unavailable in the environment
Added or updated test cases to reproduce and prevent the issue
Static problem checks passed with no errors
git diff --check passed

Checklist:
The code is properly formatted and is consistent with the existing code style
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warning

…ion calendars into getContributionDates() and then into the streak calculation. The problem was that contribution data from adjacent yearly calendars could overlap and remain in an incorrect order, so the algorithm sometimes processed dates out of sequence and incorrectly included days with zero contributions. I fixed this by sorting the merged contribution dates chronologically before calculating streaks, added a regression test in StatsTest.php using overlapping yearly calendars, and updated the FAQ to explain how the timezone option can be used for local-day calculations. The code passed static problem checks and git diff --check; PHPUnit could not be executed because PHP and Composer are unavailable in the environment.
@TheProtagonist07

Copy link
Copy Markdown
Contributor

Hi @aryansingh42046, thanks for working on this!

I've also been running into a similar problem with my own streak card not matching my contribution graph, which is how I came across #918 and then this PR. I tried it out locally and wanted to share what I found, in case it helps.

What worked well:

  • The new regression test fails on main without the ksort line and passes with it, so it does exercise the change.
  • The FAQ addition about the timezone option is helpful on its own.
  • The 5 live-API tests in StatsTest.php behave the same on main and on this branch (they need a real token), so nothing else seems affected.

One thing I wasn't able to confirm:

  • I couldn't find adjacent yearly calendars overlapping in real GitHub data. I queried contributionsCollection for a few accounts for 2021, 2022, 2025 and 2026, and each year ran exactly from Jan 1 to Dec 31. Since buildContributionGraphQuery() requests $year-01-01 to $year-12-31, I wasn't sure the overlap scenario in the test happens in practice. Maybe I missed a case, so if you have an example account where it happens, I'd love to see it.

Also, since #918 doesn't include a username or what the card shows versus what the contribution graph shows, it's hard to tell whether this is the cause. It might be worth asking the reporter for those details.

Happy to be corrected on any of this. Thanks again for the contribution!

@aryansingh42046

Copy link
Copy Markdown
Author

Thanks for the thorough testing! You're right I appreciate you digging into the actual GitHub data to check whether overlapping yearly calendars happen in practice.

Since you found that GitHub consistently returns Jan 1 → Dec 31 per year with no overlap, I'm reconsidering whether ksort() actually fixes the real bug in #918. My test case was artificial and might not address the actual issue.

Looking at the other open issues (#890 , #933, #922), several users report timezone-related streak resets and incorrect contribution detection. Those seem more likely to be the root cause.

I'm going to close this PR and investigate those issues instead they have clearer reproduction cases. But the FAQ addition about the timezone parameter is still useful, so I can submit that separately.

Thanks again for the careful review!

@TheProtagonist07

Copy link
Copy Markdown
Contributor

Thanks for the thoughtful reply, @aryansingh42046, and for taking another look at the data. That's the right call.

The FAQ addition about the timezone option is useful, so a separate PR for it makes sense. The timezone issues you mentioned also sound like a more promising lead, and I'd be happy to test a fix on my account (I'm in IST) or review a PR if that helps.

I'm fairly new to open source myself, so if you'd like to collaborate on this repo or anything else, feel free to reach out here or through my GitHub profile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants