Feat add dynamic pr count badge endpoint with caching and UI integration - #51
Merged
livrasand merged 4 commits intoFeb 23, 2026
Conversation
…tion Agregado endpoint GET /badge/:owner/:repo para badge SVG dinámico con conteo de PRs anónimos por repositorio. Implementada función GetPRCountByRepo() en internal/database/supabase.go usando Content-Range header para obtener total count. Creado handler BadgePRCountHandler con sistema de caché en memoria (TTL 5 minutos) usando sync.Mutex. Agregado selector de tipo de badge en UI con opciones "Anonymous Contributor Friendly" y "Anonymous PR
…N caching Modificado header Cache-Control en BadgePRCountHandler de "no-cache, max-age=300" a "public, max-age=300" para permitir caching en CDNs y proxies intermedios mientras mantiene TTL de 5 minutos.
Agregado push-options en capabilities string de ReceivePackDiscoveryHandler para habilitar soporte de opciones de push en protocolo Git HTTP.
Modificada lógica de actualización de caché en BadgePRCountHandler para solo actualizar cuando la base de datos responde correctamente (dbOk=true) o cuando ya existe un valor previo en caché (ok=true). Evita sobrescribir caché con valores incorrectos en caso de fallo de DB.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
📝 WalkthroughWalkthroughThe changes introduce a new PR count badge feature across the database, HTTP, and frontend layers. A new database method retrieves PR counts, a new HTTP handler serves dynamic SVG badges with caching, a new route registers the endpoint, and the frontend UI allows users to select between badge types and generate PR count badges. Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant Handler as HTTP Handler
participant Cache as In-Memory Cache
participant DB as Supabase Database
Client->>Handler: GET /badge/:owner/:repo
Handler->>Handler: Validate owner/repo format
Handler->>Cache: Check cache for owner/repo
alt Cache Miss or Expired
Handler->>DB: GetPRCountByRepo(owner, repo)
DB-->>Handler: PR count
Handler->>Cache: Update cache with count & TTL
else Cache Hit
Cache-->>Handler: Return cached count
end
Handler->>Handler: Generate SVG badge<br/>(dynamic width based on count)
Handler-->>Client: SVG response<br/>(Content-Type: image/svg+xml<br/>Cache-Control: max-age=300)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
4 tasks
gitgost-anonymous
approved these changes
Feb 23, 2026
livrasand
deleted the
feat--add-dynamic-PR-count-badge-endpoint-with-caching-and-UI-integration
branch
February 23, 2026 14:57
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web/index.html (2)
1494-1526:⚠️ Potential issue | 🟡 Minor
getElementById('badge-copy-row')always returnsnull— copy section is permanently hidden.
badge-copy-rowis a CSS class name, not an elementid.getElementByIdreturnsnull, causing theif (copyInput && copyRow)guard (line 1524) to always befalse. As a result:
badge-markdownis never populated with the generated markdown.badge-copy-section(idbadge-copy-section) is never shown, so the copy button is inaccessible to users.This is pre-existing, but the new
markdownvariable introduced by this PR flows through the same broken path for both badge types.🐛 Proposed fix
- const copyRow = document.getElementById('badge-copy-row'); + const copySection = document.getElementById('badge-copy-section'); const copyInput = document.getElementById('badge-markdown'); // ... in the success branch: - if (copyInput && copyRow) { + if (copyInput && copySection) { copyInput.value = markdown; - copyRow.style.display = 'grid'; + copySection.style.display = 'block'; } // ... in the failure/error branches: - if (copyRow) copyRow.style.display = 'none'; + if (copySection) copySection.style.display = 'none';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/index.html` around lines 1494 - 1526, The copy row is selected with getElementById('badge-copy-row') but the element uses a class, so copyRow is always null; change the selection to document.querySelector('.badge-copy-row') (or to the proper id if you prefer to convert the element to an id) and keep using copyInput (getElementById('badge-markdown')) and the existing guard (if (copyInput && copyRow)) so that copyInput.value = markdown and copyRow.style.display = 'grid' run when a badge is fetched; also verify any reference to 'badge-copy-section' uses the correct id/class consistent with the DOM.
1527-1531:⚠️ Potential issue | 🟡 MinorMisleading error message for
pr-countbadge type.
BadgePRCountHandleralways returns HTTP 200 with an SVG (displaying 0 when no PRs exist), so!response.okfor apr-countrequest means a server-side validation failure (400), not a missing.gitgost.yml. The current error copy instructs users to add a file that has no effect on the PR-count badge.🐛 Proposed fix
- result.innerHTML = '<p>Repository not verified. Add a .gitgost.yml file to your repo root to enable the dynamic badge.</p>'; + if (badgeType === 'pr-count') { + result.innerHTML = '<p>Could not load badge. Check that owner/repo is correct.</p>'; + } else { + result.innerHTML = '<p>Repository not verified. Add a <code>.gitgost.yml</code> file to your repo root to enable the dynamic badge.</p>'; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/index.html` around lines 1527 - 1531, The current branch sets a misleading "add a .gitgost.yml" message when response.ok is false even for pr-count badges; update the error handling that sets result.innerHTML (and hides copyRow) to distinguish badge types (e.g., check the requested badgeType or if badgeType === 'pr-count') and for 'pr-count' show a server/validation error message including the HTTP status and brief guidance (e.g., "Server validation failed (status X) — check request parameters or server logs") instead of instructing to add .gitgost.yml; adjust the code around result.innerHTML and copyRow handling so other badge types keep the existing .gitgost.yml guidance.
🧹 Nitpick comments (5)
internal/http/handlers.go (3)
1046-1068: Thundering herd on cache expiry.After the lock is released at line 1050, multiple concurrent goroutines can all observe a stale entry and proceed to hit the DB simultaneously. At low traffic this is harmless; at higher badge request rates it amplifies DB load on every TTL boundary.
Consider wrapping the refresh path with
golang.org/x/sync/singleflightso only one goroutine fetches while others wait and reuse the result:// package-level var badgeSFG singleflight.Group // inside handler, replacing the refresh block if !ok || time.Since(cachedAt) > badgeCacheTTL { val, _, _ := badgeSFG.Do(cacheKey, func() (interface{}, error) { n, err := dbClient.GetPRCountByRepo(c.Request.Context(), owner, repo) if err != nil { return count, err // serve stale on error } badgeCacheMu.Lock() badgeCache[cacheKey] = n badgeCacheAt[cacheKey] = time.Now() badgeCacheMu.Unlock() return n, nil }) count = val.(int) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/http/handlers.go` around lines 1046 - 1068, The current refresh logic around badgeCache/badgeCacheAt guarded by badgeCacheMu allows a thundering herd after the lock is released; replace the refresh path so only one goroutine performs the DB fetch using a package-level golang.org/x/sync/singleflight.Group (e.g., badgeSFG.Do) keyed by cacheKey, have that single-flight call invoke dbClient.GetPRCountByRepo(ctx, owner, repo), update badgeCache and badgeCacheAt under badgeCacheMu inside the singleflight function on success, and on DB error return the previous cached count so callers can serve stale data while others reuse the singleflight result. Ensure count is set from the singleflight result after Do returns.
1028-1033: DecouplebadgeCacheTTLandCache-Control max-age— they diverge silently.
badgeCacheTTL = 5 * time.Minute(line 1032) and"public, max-age=300"(line 1115) both represent 5 minutes but are defined independently. A future edit to one that misses the other will make the HTTP cache and the in-memory cache disagree.♻️ Derive `max-age` from the TTL constant
var ( badgeCache = make(map[string]int) badgeCacheAt = make(map[string]time.Time) badgeCacheMu sync.Mutex badgeCacheTTL = 5 * time.Minute )- c.Header("Cache-Control", "public, max-age=300") + c.Header("Cache-Control", fmt.Sprintf("public, max-age=%d", int(badgeCacheTTL.Seconds())))Also applies to: 1115-1115
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/http/handlers.go` around lines 1028 - 1033, The TTL constant badgeCacheTTL is duplicated by a hardcoded Cache-Control value ("public, max-age=300"); make the HTTP header derive max-age from badgeCacheTTL so they cannot drift: replace the literal "public, max-age=300" with a value computed from badgeCacheTTL (e.g. using badgeCacheTTL.Seconds()/int conversion or fmt.Sprintf) wherever the badge response is written, leaving badgeCache, badgeCacheAt and badgeCacheMu unchanged.
1082-1112:%d0coordinate trick is non-obvious — consider using explicit multiplied arguments.Lines like
x="%d0"append a literal ASCII0to the format verb so thatlabelMid=50producesx="500", exploiting the fact that the SVG uses a 10× internal coordinate system withtransform="scale(.1)". While correct, this is a maintenance trap for anyone reading the template.♻️ Proposed clearer approach
- labelMid := 50 - valueMid := 100 + valueWidth/2 + labelMidSVG := 500 // 50px × 10 (SVG 10× scale) + valueMidSVG := (100 + valueWidth/2) * 10 svg := fmt.Sprintf(`... - <text x="%d0" y="150" ...>%s</text> - <text x="%d0" y="140" ...>%s</text> - <text x="%d0" y="150" ...textLength="%d0"...>%s</text> - <text x="%d0" y="140" ...textLength="%d0"...>%s</text> + <text x="%d" y="150" ...>%s</text> + <text x="%d" y="140" ...>%s</text> + <text x="%d" y="150" ...textLength="%d"...>%s</text> + <text x="%d" y="140" ...textLength="%d"...>%s</text> ...`, - ..., labelMid, label, labelMid, label, - valueMid, (valueWidth-16), value, valueMid, (valueWidth-16), value, + ..., labelMidSVG, label, labelMidSVG, label, + valueMidSVG, (valueWidth-16)*10, value, valueMidSVG, (valueWidth-16)*10, value, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/http/handlers.go` around lines 1082 - 1112, The SVG fmt.Sprintf uses the non-obvious "%d0" trick for coordinates/textLength (in the svg string built in handlers.go) which appends a literal "0" instead of explicitly multiplying by 10; update the format string to use "%d" for those x/textLength placeholders (the ones using labelMid, valueMid and textLength fields) and pass explicitly multiplied values (e.g., labelMid*10, valueMid*10, (valueWidth-16)*10) when calling fmt.Sprintf so the internal 10× coordinate system with transform="scale(.1)" remains correct and the template is readable and maintainable.web/index.html (1)
1337-1343:<select>element missing CSS styling — will render with browser defaults.All other
.badge-forminputs have explicit styling (background: var(--code-bg); border: 1px solid var(--border); ...), but there is no matching rule forselect. The dropdown will look visually inconsistent across browsers.♻️ Add select styling alongside existing input rules
.badge-generator input { background: var(--code-bg); border: 1px solid var(--border); border-radius: var(--radius-sm); padding: .75rem .9rem; color: var(--fg); font-family: 'IBM Plex Mono', monospace; } +.badge-generator select { + background: var(--code-bg); + border: 1px solid var(--border); + border-radius: var(--radius-sm); + padding: .75rem .9rem; + color: var(--fg); + font-family: 'IBM Plex Mono', monospace; + width: 100%; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/index.html` around lines 1337 - 1343, The select with id "badge-type" in the badge-form lacks the existing input styling and will render with browser defaults; add a CSS rule targeting the select (e.g., `#badge-type` or .badge-form select) that mirrors the input styles (background: var(--code-bg); border: 1px solid var(--border); color, padding, border-radius, font, and focus outline) so the dropdown visually matches other inputs like the .badge-form input elements and behaves consistently across browsers.internal/database/supabase.go (1)
403-445: Extract the duplicated Content-Range parsing into a private helper.The Content-Range extraction/parsing block (lines 433-442) is now duplicated verbatim in
GetTotalPRs,GetReportCount, andGetPRCountByRepo. A single helper would reduce noise and centralise the error messages.♻️ Proposed helper extraction
+// parseContentRangeCount parses the total from a Supabase "0-0/{total}" or "*/{total}" Content-Range header. +func parseContentRangeCount(contentRange string) (int, error) { + if contentRange == "" { + return 0, fmt.Errorf("missing Content-Range header in response") + } + slashIdx := strings.LastIndex(contentRange, "/") + if slashIdx == -1 { + return 0, fmt.Errorf("invalid Content-Range format: %s", contentRange) + } + count, err := strconv.Atoi(contentRange[slashIdx+1:]) + if err != nil { + return 0, fmt.Errorf("failed to parse total count from Content-Range '%s': %v", contentRange, err) + } + return count, nil +}Then replace the 3-method duplicated blocks with a single call, e.g.:
- contentRange := resp.Header.Get("Content-Range") - if contentRange == "" { - return 0, fmt.Errorf("missing Content-Range header in response") - } - slashIdx := strings.LastIndex(contentRange, "/") - if slashIdx == -1 { - return 0, fmt.Errorf("invalid Content-Range format: %s", contentRange) - } - totalStr := contentRange[slashIdx+1:] - count, err := strconv.Atoi(totalStr) - if err != nil { - return 0, fmt.Errorf("failed to parse total count from Content-Range '%s': %v", contentRange, err) - } - return count, nil + return parseContentRangeCount(resp.Header.Get("Content-Range"))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/database/supabase.go` around lines 403 - 445, Extract the duplicated Content-Range parsing logic into a single unexported helper (e.g., parseContentRange(contentRange string) (int, error)) and replace the repeated blocks in GetPRCountByRepo, GetTotalPRs, and GetReportCount with calls to that helper; the helper should validate non-empty header, find the last "/" and convert the trailing substring to an int, and return a clear error message on any failure so all three callers use the centralized parsing and error text.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@web/index.html`:
- Around line 1494-1526: The copy row is selected with
getElementById('badge-copy-row') but the element uses a class, so copyRow is
always null; change the selection to document.querySelector('.badge-copy-row')
(or to the proper id if you prefer to convert the element to an id) and keep
using copyInput (getElementById('badge-markdown')) and the existing guard (if
(copyInput && copyRow)) so that copyInput.value = markdown and
copyRow.style.display = 'grid' run when a badge is fetched; also verify any
reference to 'badge-copy-section' uses the correct id/class consistent with the
DOM.
- Around line 1527-1531: The current branch sets a misleading "add a
.gitgost.yml" message when response.ok is false even for pr-count badges; update
the error handling that sets result.innerHTML (and hides copyRow) to distinguish
badge types (e.g., check the requested badgeType or if badgeType === 'pr-count')
and for 'pr-count' show a server/validation error message including the HTTP
status and brief guidance (e.g., "Server validation failed (status X) — check
request parameters or server logs") instead of instructing to add .gitgost.yml;
adjust the code around result.innerHTML and copyRow handling so other badge
types keep the existing .gitgost.yml guidance.
---
Nitpick comments:
In `@internal/database/supabase.go`:
- Around line 403-445: Extract the duplicated Content-Range parsing logic into a
single unexported helper (e.g., parseContentRange(contentRange string) (int,
error)) and replace the repeated blocks in GetPRCountByRepo, GetTotalPRs, and
GetReportCount with calls to that helper; the helper should validate non-empty
header, find the last "/" and convert the trailing substring to an int, and
return a clear error message on any failure so all three callers use the
centralized parsing and error text.
In `@internal/http/handlers.go`:
- Around line 1046-1068: The current refresh logic around
badgeCache/badgeCacheAt guarded by badgeCacheMu allows a thundering herd after
the lock is released; replace the refresh path so only one goroutine performs
the DB fetch using a package-level golang.org/x/sync/singleflight.Group (e.g.,
badgeSFG.Do) keyed by cacheKey, have that single-flight call invoke
dbClient.GetPRCountByRepo(ctx, owner, repo), update badgeCache and badgeCacheAt
under badgeCacheMu inside the singleflight function on success, and on DB error
return the previous cached count so callers can serve stale data while others
reuse the singleflight result. Ensure count is set from the singleflight result
after Do returns.
- Around line 1028-1033: The TTL constant badgeCacheTTL is duplicated by a
hardcoded Cache-Control value ("public, max-age=300"); make the HTTP header
derive max-age from badgeCacheTTL so they cannot drift: replace the literal
"public, max-age=300" with a value computed from badgeCacheTTL (e.g. using
badgeCacheTTL.Seconds()/int conversion or fmt.Sprintf) wherever the badge
response is written, leaving badgeCache, badgeCacheAt and badgeCacheMu
unchanged.
- Around line 1082-1112: The SVG fmt.Sprintf uses the non-obvious "%d0" trick
for coordinates/textLength (in the svg string built in handlers.go) which
appends a literal "0" instead of explicitly multiplying by 10; update the format
string to use "%d" for those x/textLength placeholders (the ones using labelMid,
valueMid and textLength fields) and pass explicitly multiplied values (e.g.,
labelMid*10, valueMid*10, (valueWidth-16)*10) when calling fmt.Sprintf so the
internal 10× coordinate system with transform="scale(.1)" remains correct and
the template is readable and maintainable.
In `@web/index.html`:
- Around line 1337-1343: The select with id "badge-type" in the badge-form lacks
the existing input styling and will render with browser defaults; add a CSS rule
targeting the select (e.g., `#badge-type` or .badge-form select) that mirrors the
input styles (background: var(--code-bg); border: 1px solid var(--border);
color, padding, border-radius, font, and focus outline) so the dropdown visually
matches other inputs like the .badge-form input elements and behaves
consistently across browsers.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
internal/database/supabase.gointernal/http/handlers.gointernal/http/router.goweb/index.html
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes