feat: add community activity score badges and filters#2008
feat: add community activity score badges and filters#2008shravanithouta108 wants to merge 47 commits into
Conversation
Closes S3DFX-CYBER#138 Signed-off-by: shravanithouta108 <shravanithouta108@gmail.com>
Signed-off-by: shravanithouta108 <shravanithouta108@gmail.com>
Signed-off-by: shravanithouta108 <shravanithouta108@gmail.com>
Signed-off-by: shravanithouta108 <shravanithouta108@gmail.com>
|
@shravanithouta108 is attempting to deploy a commit to the s3dfx-cyber's projects Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
💬 Faster Reviews & AssignmentsHi @shravanithouta108, for faster coordination and smoother communication, consider joining our Discord community: Useful Channels
|
👋 Thanks for opening a PR, @shravanithouta108!Your PR has entered the 🚦 PR Review Pipeline.
🔄 Review Flow
A pipeline status comment may appear automatically as your PR progresses. ✅ Contributor Checklist
|
✅ DCO Sign-off VerifiedHi @shravanithouta108 👋 All commits in this PR contain valid Thank you for following the DCO requirements 🚀 |
|
| Detected Labels | Resolution |
|---|---|
gssoc, nsoc |
GSSoC priority applied — classified as a GSSoC PR |
ℹ️ Only one program label may be active at a time. GSSoC takes priority when both are present.
If this is incorrect, please:
- Remove the unintended program keyword from your PR description
- Push a new commit or re-open the PR to re-trigger classification
📖 See our Contributing Guide for labeling details.
📏 Large PR DetectedThis PR has 1300 lines changed (687+ / 613-). Large PRs are harder to review and more likely to introduce issues. Consider splitting into smaller, focused PRs when possible. |
|
|
deepak0x
left a comment
There was a problem hiding this comment.
- The PR merge rate is computed from a creation-ordered 100-item cap instead of filtering to the 90-day window at
compute-activity-scores.js:91, which biases high-traffic repositories to artificially high scores. - API calls for commits, issues, and PRs lack pagination at
compute-activity-scores.js:71, so repositories with more than 100 items in the 90-day window are silently truncated, severely undercounting activity for the most active organizations. - The star signal at
compute-activity-scores.js:103uses cumulative total stars rather than the period delta, so dormant repositories with large historical star counts remain artificially high-ranked. - The
getCommunityBadgefunction atsrc/js/app.js:3770insertsd.scoreunescaped into atitleattribute template literal later wrapped inrawHTML(), creating an XSS surface ifcommunity_activity.jsonever contains a crafted value. - No before/after screenshots or screen recording are provided in the PR to confirm badge rendering, alignment in both light and dark themes, and mobile layout, despite issue #138 explicitly showing card mockups.
- A duplicate assignment to
globalThis.matchAllLanguagesexists atapp.js:3779and should be removed. - Changes to
privacy.htmlandsrc/js/landing.jsare unrelated to issue #138 and should be reverted or submitted as a separate PR.
Address the statistical biases in activity scoring, add pagination to API calls, provide visual evidence, and revert the out-of-scope changes before resubmission.
⏰ Mentor Review TimeoutThe following reviewer(s) did not respond within 24 hours:
Replacement reviewer(s) assigned: Mentors are expected to review within 24 hours. This is an automated rotation. |
⏰ Mentor Review TimeoutThe following reviewer(s) did not respond within 24 hours: Replacement reviewer(s) assigned: Mentors are expected to review within 24 hours. This is an automated rotation. |
deepak0x
left a comment
There was a problem hiding this comment.
index.html:5632has an unclosed arrow function in the event listener block that crashes the entire inline script at parse time.- If
orgs.jsloads after the inline script, theORGSvariable is undefined whenfilteredOrgsinitializes, causing zero cards to render on load. - The Active Communities filter logic exists only in
index.html's inline script whileapp.jsdefines its ownapplyFilters, violating CONTRIBUTING.md's synchronization requirement. renderMentorFinder()was removed entirely, breaking an existing Mentor Finder feature that's outside the scope of issue #138.- The "Most Active" sort option was added to
applySecondarySort()atapp.js:836but the sort control UI is missing from the diff. - Screenshots are required by CONTRIBUTING.md for UI changes. this PR adds badges to 180+ cards, a new modal section, and new filter/sort controls but provides no screenshots.
Fix the parse error at index.html:5632 first and verify the inline script loads, then post a screenshot of the new badges, modal, filter chip, and sort option working together.
⏰ Mentor Review TimeoutThe following reviewer(s) did not respond within 24 hours:
Replacement reviewer(s) assigned: Mentors are expected to review within 24 hours. This is an automated rotation. |
⏰ Mentor Review TimeoutThe following reviewer(s) did not respond within 24 hours: Replacement reviewer(s) assigned: Mentors are expected to review within 24 hours. This is an automated rotation. |
⏰ Mentor Review TimeoutThe following reviewer(s) did not respond within 24 hours:
Replacement reviewer(s) assigned: Mentors are expected to review within 24 hours. This is an automated rotation. |
deepak0x
left a comment
There was a problem hiding this comment.
- The script at
compute-activity-scores.js:71–95calls GitHub endpoints withper_page=100and never follows Link headers, silently undercounting high-traffic repos (those with >100 commits in 90 days) and biasing results against the organizations this feature is meant to highlight. - The stars metric calculation at
compute-activity-scores.js:103uses totalstargazers_count / 1000instead of growth over the last 6 months as issue #138 specifies, so a 50k-star legacy repo scores the same as one gaining 1k stars monthly. - Commit history includes roughly 30 upstream-sync commits (MariaDB, OpenWISP, privacy navbar, landing page updates, mentor leaderboards) that belong in separate PRs, making the feature code nearly unreadable and preventing proper audit of the activity logic.
- Index.html duplicates filter block IDs (
chip-veteran,selectedLangsStrip,orgCount,categoryFilter) at lines 1344–1395 and again at 1399–1525, sogetElementById()andquerySelector()calls will unpredictably bind to the first occurrence and break filter behavior. - The
fetchJSONfunction atcompute-activity-scores.js:38–42lacks a timeout and the workflow job has notimeout-minutesconfigured, so a hung GitHub API call stalls all scoring indefinitely and wastes CI minutes. - PR description references screenshots from PR #1861 instead of including before-and-after captures of the badge, modal panels, filter chips, and sort behavior, making it impossible for reviewers to verify the implementation matches the issue requirements.
Rebase out the upstream-sync commits and fix the pagination, stars metric, timeouts, DOM duplication, and screenshot gaps before resubmitting.
⏰ Mentor Review TimeoutThe following reviewer(s) did not respond within 24 hours: Replacement reviewer(s) assigned: Mentors are expected to review within 24 hours. This is an automated rotation. |
deepak0x
left a comment
There was a problem hiding this comment.
index.html:1344-1395introduces duplicate filter IDs (chip-veteran,selectedLangsStrip,orgCount,categoryFilter) that override the canonical desktop/mobile filter blocks, causing JavaScript to bind to wrong elements, and an unclosed<select>tag leaves malformed HTML.compute-activity-scores.jsfetches withper_page=100but never follows Link headers, truncating high-traffic orgs (LLVM, NixOS, Python, Jenkins) at 100 items and causing their scores to be systematically undercounted.compute-activity-scores.js:116and the GitHub Actions workflow have no timeout bounds, so a single hung API call can stall the entire daily job indefinitely without alerting you.app.js:502injects the activity score into an HTML title attribute with no escaping, and the community modal (app.js:1567-1594) uses raw template literals forcd.scoreandtierMap[cd.tier]while the rest of the codebase usessafeHTML, creating an XSS vector ifcommunity_activity.jsonis compromised.- The README states tier thresholds as 75/50/25/0 but
getTier()incompute-activity-scores.js:137-141implements 80/60/40/0, and the commit message claims to fix this misalignment but doesn't. - The starsGrowth metric computes
repoData.stargazers_count / 1000(total historical stars), not the 6-month growth specified in issue #138, and the variable name misleads future maintainers about what it actually measures.
Fix the duplicate filter IDs, implement pagination, add timeouts, escape the XSS vectors, align README with code, and verify starsGrowth measures period growth before resubmitting.
⏰ Mentor Review TimeoutThe following reviewer(s) did not respond within 24 hours:
Replacement reviewer(s) assigned: Mentors are expected to review within 24 hours. This is an automated rotation. |
⏰ Mentor Review TimeoutThe following reviewer(s) did not respond within 24 hours: Replacement reviewer(s) assigned: Mentors are expected to review within 24 hours. This is an automated rotation. |
deepak0x
left a comment
There was a problem hiding this comment.
- Screenshots are required by CONTRIBUTING.md for UI changes to
index.html,app.js, andstyles.css. add them to the PR description. - This PR bundles multiple UI changes unrelated to the activity score feature: the mobile filter dropdown was removed entirely, the "Remove" button was deleted from
renderWatchlist()atapp.js:3868, the search and guide nav buttons were removed, nav breakpoints were changed fromlgtomd, andprivacy.htmlwas modified. Each of these should be in a separate focused PR. - The README documents tier thresholds as 75/50/25 but
compute-activity-scores.jsuses 80/60/40. update the README to match the actual code thresholds. - Deleting the "Remove" button from the watchlist panel is a behavior regression. users can no longer remove orgs from their watchlist without navigating away and the functionality needs to be restored.
- The
data-org-countattribute is hardcoded to "184" atindex.html:1989andindex.html:2025instead of being dynamic. it will become incorrect when the organization count changes. - Unrelated changes like
privacy.htmlmodifications andcheckAPI()minification should not be in this PR. revert them.
Separate the activity score feature into its own PR without the mobile/nav refactoring, add screenshots, fix the README thresholds, restore the Remove button, and make the org count dynamic.
⏰ Mentor Review TimeoutThe following reviewer(s) did not respond within 24 hours:
Replacement reviewer(s) assigned: Mentors are expected to review within 24 hours. This is an automated rotation. |
deepak0x
left a comment
There was a problem hiding this comment.
-
Duplicate filter markup in
index.htmlaround lines 1344-1395 introduces duplicate IDs (chip-veteran,selectedLangsStrip,orgCount,categoryFilter) that cause the "Active Communities" chip to bind to the wrong control in at least one viewport, breaking the filter. -
README documents tier thresholds as Very Active 75-100, Active 50-74, Moderate 25-49, Low 0-24, but
getTier()incompute-activity-scores.js:138implements ≥80, ≥60, ≥40 instead, creating a documentation-code mismatch that misleads users trying to understand how orgs are scored. -
starsGrowthatcompute-activity-scores.js:180computes cumulative total stars divided by 1000, not a 6-month growth delta as promised in the issue and README, causing old inactive repos with high star counts to receive perpetually high scores. -
GitHub API calls in
compute-activity-scores.js:148-169useper_page=100with no pagination follow-through, silently truncating data for repos with >100 commits/issues/PRs in the 90-day window and systematically underestimating scores for the most active orgs the feature is meant to surface. -
mCommunity.innerHTMLatapp.jsaround line 1567 assigns a raw template literal with unescapedcd.scoreand tier values while every other dynamic value in the same modal usessafeHTML, violating the established escaping pattern. -
No screenshots are included in this PR despite UI changes across
index.html,styles.css, andapp.js, and referencing closed PR #1861 for visual evidence is not a substitute since reviewers cannot access it. -
fetchJSON()incompute-activity-scores.js:115-119has no AbortController timeout and the workflow job has no timeout-minutes, allowing a single hung GitHub API request to stall the entire Actions run indefinitely.
Fix the duplicate filter IDs and threshold documentation, implement API pagination with proper escaping, correct the starsGrowth metric, and add screenshots before resubmitting.
⏰ Mentor Review TimeoutThe following reviewer(s) did not respond within 24 hours: Replacement reviewer(s) assigned: Mentors are expected to review within 24 hours. This is an automated rotation. |
deepak0x
left a comment
There was a problem hiding this comment.
- GitHub API calls lack pagination in
compute-activity-scores.jsline ~71, silently truncating results at 100 items and producing systematically low scores for high-traffic repositories. - PR merge rate calculation uses a partial, creation-ordered result set instead of filtering the stated 90-day window, causing scores to be incorrect for repositories with heavy activity.
- Stars metric sums cumulative repository stars instead of measuring period-over-period growth, so an inactive repository with historical stars scores identically to one gaining active users.
- No screenshot, GIF, or video demonstrates that the new community badges, filter chips, and sort dropdown function correctly on both desktop and mobile as required by acceptance criterion 9.
- Unrelated changes appear in
styles.css(removing 118 lines of premium-footer CSS) andprivacy.html(altering Tailwind breakpoints) that have no stated connection to issue #138 and should be reverted or explicitly explained. - The PR body incorrectly claims all issues from prior reviews have been fixed when cubic's review on this PR identified 5 new unfixed issues, including an unescaped score XSS surface in the HTML.
- The CONTRIBUTING.md requirement to run
npm testand confirmindex.htmlandapp.jsremain in sync was not satisfied before submission.
Fix the three data accuracy issues in compute-activity-scores.js, add visual evidence for the UI changes, explain the CSS modifications, and correct the PR body before resubmitting.
⏰ Mentor Review TimeoutThe following reviewer(s) did not respond within 24 hours:
Replacement reviewer(s) assigned: Mentors are expected to review within 24 hours. This is an automated rotation. |
⏰ Mentor Review TimeoutThe following reviewer(s) did not respond within 24 hours: Replacement reviewer(s) assigned: Mentors are expected to review within 24 hours. This is an automated rotation. |
⏰ Mentor Review TimeoutThe following reviewer(s) did not respond within 24 hours:
Replacement reviewer(s) assigned: Mentors are expected to review within 24 hours. This is an automated rotation. |
⏰ Mentor Review TimeoutThe following reviewer(s) did not respond within 24 hours: Replacement reviewer(s) assigned: Mentors are expected to review within 24 hours. This is an automated rotation. |
deepak0x
left a comment
There was a problem hiding this comment.
-
Screenshots are required for UI changes per CONTRIBUTING but this PR provides none; referencing a different closed PR #1861 does not meet the requirement.
-
compute-activity-scores.js:71fetches withper_page=100without handling GitHub API pagination, so events beyond the first 100 are silently dropped, systematically undercounting scores for high-traffic repos. -
The star signal at
compute-activity-scores.js:103sums cumulative stargazer count instead of 90-day growth as specified in issue #138, causing stale high-star repos to score incorrectly. -
Acceptance criteria state "orgs with multiple repos use the primary GSoC repo" but
compute-activity-scores.jsdoesn't implement this; it processes whatever is in the github field without selection logic. -
Org/repo pairs at
compute-activity-scores.js:22-36are extracted via regex assuming a fixed field order; parsing silently breaks if the order changes or an entry spans multiple lines. -
The index.html duplicate filter section at lines 1344-1395 reuses IDs that exist in the canonical block (chip-veteran, selectedLangsStrip, orgCount, categoryFilter), creating invalid DOM that breaks event binding.
Attach screenshots, fix the data-collection bugs (pagination, star metric, multi-repo handling), resolve the fragile regex and duplicate DOM IDs, then resubmit.
⏰ Mentor Review TimeoutThe following reviewer(s) did not respond within 24 hours:
Replacement reviewer(s) assigned: Mentors are expected to review within 24 hours. This is an automated rotation. |
deepak0x
left a comment
There was a problem hiding this comment.
- The "Active Communities" filter threshold doesn't match the feature spec: the issue requires 60+ score but your code filters on
tier !== 'low'which is ≥40. - The stars metric is wrong: the issue specifies "growth in last 6 months" but
compute-activity-scores.js:180uses cumulativestargazers_count / 1000, which measures total stars instead. index.htmlloads the inline script that callsgetActivityBadge()beforesrc/js/app.jsis loaded, sowindow.communityActivityis undefined on first render.- Duplicate filter DOM IDs appear around
index.html:1344–1395(chip-veteran, selectedLangsStrip, orgCount, categoryFilter), which breaks HTML validity and causes JS to silently bind to the first copy. - GitHub API calls in
compute-activity-scores.js:116–118fetchper_page=100with no pagination, so high-traffic repos like NixOS/nixpkgs get truncated scores and won't surface correctly. - README tier ranges don't match the code implementation: README defines Very Active 75–100 / Active 50–74 / Moderate 25–49 / Low 0–24, but
getTier()uses ≥80 / ≥60 / ≥40 / <40. - Your PR description claims "all issues flagged by cubic and TENET are fixed" but cubic's latest review still flags the XSS at
src/js/app.js:502and missing pagination, and TENET flags the script loading order.
Fix the spec mismatches (threshold, signal), script loading order, duplicate IDs, and pagination before merge.
⏰ Mentor Review TimeoutThe following reviewer(s) did not respond within 24 hours: Replacement reviewer(s) assigned: Mentors are expected to review within 24 hours. This is an automated rotation. |
⏰ Mentor Review TimeoutThe following reviewer(s) did not respond within 24 hours:
Replacement reviewer(s) assigned: Mentors are expected to review within 24 hours. This is an automated rotation. |
deepak0x
left a comment
There was a problem hiding this comment.
-
index.html:1344–1395contains duplicate filter markup with an unclosed<select>tag and duplicate element IDs, creating invalid DOM where the new "Active Communities" filter silently fails because event listeners bind to the first matching element. -
No visual evidence is included in this PR; the description references a closed PR (#1861) for screenshots, leaving reviewers unable to assess the rendered badges, filter chips, and detail panel.
-
src/js/app.js:502ingetCommunityBadge()insertscd.scoreunescaped into a title attribute while the rest of the file consistently usessafeHTML()for dynamic content, creating an inconsistency and potential XSS vector. -
src/js/app.js:1567–1594assignsmCommunity.innerHTMLvia raw template literal with unescapedcd.scoreand tier text, inconsistent with themMetrics.innerHTMLpattern two lines above which correctly usessafeHTML(). -
agent/scripts/compute-activity-scores.js:71–91fetches GitHub API data withper_page=100but never follows pagination, systematically undercounting activity in high-volume repos and directly undermining the score's ability to differentiate active projects. -
agent/scripts/compute-activity-scores.js:38–42has no timeout or AbortController on network requests, allowing a hung GitHub API call to stall the entire daily workflow indefinitely.
Fix the DOM duplication, add visual evidence to this PR instead of referencing a closed issue, and apply safeHTML() consistently across all dynamic content assignments before resubmitting.
deepak0x
left a comment
There was a problem hiding this comment.
-
compute-activity-scores.js:180implementsstarsGrowthasrepoData.stargazers_count / 1000, which measures total stars, not growth over the last 6 months as described in issue #138 and the README. -
The PR removes extensive functionality unrelated to issue #138: mobile filter dropdowns, bookmark remove buttons, hero search submit button, animations, mentor card hover effects, copy link buttons, and the entire
renderMentorFinderandrenderMentorContactSectionfunctions. -
CONTRIBUTING.md(line ~38) requires core frontend changes to be kept in sync betweenapp.jsandindex.htmlwithnpm testvalidation, butrenderMentorFinderandrenderMentorContactSectionwere deleted fromapp.jswhile remaining declared as/*global*/in the inline script, breaking this invariant. -
README documentation lists tier thresholds as 75–100 / 50–74 / 25–49 / 0–24, but the actual code in
compute-activity-scores.js:138–142uses ≥80 / ≥60 / ≥40. -
No screenshots or video evidence were provided to verify the visual changes to badges, filter chips, mobile navigation, and removed UI elements.
-
checkAPIfunction body inapp.jswas reformatted to remove whitespace, inconsistent with surrounding code style and unrelated to this feature.
Split the unrelated changes into separate PRs, fix the starsGrowth metric to compute actual growth data, update the documentation thresholds to match the code, and add screenshots of the badge and filter UI before resubmitting.
⏰ Mentor Review TimeoutThe following reviewer(s) did not respond within 24 hours: Replacement reviewer(s) assigned: Mentors are expected to review within 24 hours. This is an automated rotation. |
deepak0x
left a comment
There was a problem hiding this comment.
- The PR adds tier badges, a filter chip, and a modal panel without providing screenshots or videos to demonstrate they work, which is required for UI feature review.
index.html:1344-1395introduces duplicate DOM IDs includingchip-veteran,selectedLangsStrip,orgCount, andcategoryFilter, which causes the active-communities filter to bind to the wrong control.- The stars contribution formula uses total stargazers count (
repoData.stargazers_count / 1000) instead of 6-month growth rate as specified in issue #138, which systematically undervalues mature repositories with stable but older stars. - Ideas freshness uses
repoData.pushed_at(the repository's last commit) rather than the GSoC ideas page last-updated date from issue #138, so the signal measures repository churn rather than actual idea generation. - API calls in
compute-activity-scores.js:148-151fetch withper_page=100but do not parseLinkheaders, which means any repository with over 100 commits or issues in the 90-day window gets silently undercounted. - The PR checklist claims "I have tested my changes in a browser" but provides no screenshots, videos, or console output to verify the badges, filter, and modal actually render and work.
- The regex at
compute-activity-scores.js:22-36is fragile and will silently skip organizations if the field order changes or the object formatting shifts inorg.js.
Add a screenshot to verify the UI, fix the duplicate DOM IDs, and address the API pagination scope and specification deviations from issue #138.
📝 Description
This PR re-submits the Community Activity Score feature (originally #1861, which closed
#138) with all issues flagged by the TENET Agent, cubic, and greptile reviews fixed:
let communityActivitydeclaration acrossindex.html's inline script andsrc/js/app.js. Since both are loaded as classic (non-module)<script>tagssharing one global scope, the duplicate
lettriggered aSyntaxErrorthat abortedparsing of
app.jsentirely — which is whysafeHTML/rawHTML(defined insideapp.js) were never available at runtime.communityActivityload / race condition: Removed theredundant fetch + declaration from
index.html.app.js'sloadCommunityActivity()is now the single source of truth, mirrored onto
window.communityActivitysoindex.html's badge/filter/sort code can read it safely without refetching or racing.repo.includes('/')check incompute-activity-scores.js, so org-only values likec2siorg,ML4SCI,UNICEFInnovationare skipped (with a console warning) instead ofsilently producing misleading fallback-based activity scores.
Branch has also been synced with
upstream/mainto resolve the merge conflicts that wereblocking the previous PR.
🔗 Related Issue
Closes #138
🚀 Program Classification
🔄 Type of Change
🧪 How to Test
index.htmlin a browser (or run the dev server if configured)ReferenceError: safeHTML is not defined(or similar) appears in thebrowser console
SyntaxError: Identifier 'communityActivity' has already been declaredappears in the console
score breakdown) renders
node agent/scripts/compute-activity-scores.js(with aGITHUB_TOKENset) andconfirm bare-owner entries (e.g.
c2siorg,ML4SCI) are skipped with a warninginstead of producing a score
📸 Screenshots (if applicable)
See prior screenshots on #1861 showing tier badges, Most Active sort, Active
Communities filter, and the modal activity panel — UI is unchanged in this PR, only the
runtime bugs above were fixed.
✅ Checklist
feat: add scroll button)