Add graph_downloads() function

[d-morrison]

Summary

• Adds graph_downloads() to plot new and cumulative package download counts from CRAN (via packageRank), with optional GitHub release download tracking
• Supports configurable time aggregation (day, week, month, quarter, year), start date filtering, and custom titles
• Decomposed into small, single-file helper functions (.fetch_cran_downloads(), .fetch_github_downloads(), .aggregate_by_unit(), .prepare_download_data(), .plot_downloads(), .check_suggests())
• Exported with @keywords internal
• Unit tests with real CRAN data fixtures and vdiffr visual regression
• Adds CLAUDE.md and updates copilot instructions with no-nested-function-calls rule

Test plan

• graph_downloads() renders CRAN-only plot (default)
• graph_downloads(github = TRUE) renders 2x2 facet grid
• graph_downloads(cumulative = FALSE) renders single-metric plot
• graph_downloads(start = "2025-01-01") filters by date
• graph_downloads(unit = "week") aggregates by week
• graph_downloads(new = FALSE, cumulative = FALSE) errors
• Unit tests pass (devtools::test())
• Lint passes (lintr::lint_package())
• Docs generated (devtools::document())

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Adds a new internal-but-exported function graph_downloads() that fetches CRAN download stats (via cranlogs) and optionally GitHub release download stats (via gh), then renders a faceted ggplot of new and cumulative downloads.

Changes:

• New R/graph_downloads.R implementing graph_downloads(github = FALSE) with CRAN-only or CRAN+GitHub 2x2 facet output.
• New .positai/settings.json with editor/tooling permission settings.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

File	Description
R/graph_downloads.R	Implements the new plotting helper, exported with @keywords internal; depends on cranlogs and gh.
.positai/settings.json	Adds local Positai permission configuration.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

[d-morrison]

@claude review

[claude]

Claude finished @d-morrison's task in 2m 29s —— View job

---

Code Review

Tasks:

• Read CLAUDE.md for repo guidelines
• Read all new/modified R files
• Review code for bugs, style, and best practices
• Post review findings

---

Overall this PR is well-structured — the split into single-responsibility helper files is clean, the .data$ notation is used consistently, packageRank and gh are properly declared in DESCRIPTION, and the NULL start bug from the prior review is fixed. A few issues remain.

Bugs / Correctness

1. Stale cranlogs reference in docs

R/graph_downloads.R line 2 (and the generated man/graph_downloads.Rd line 39) still say "via cranlogs" in the description, but the implementation uses packageRank::cranDownloads(). The @details section is correct — the @description line needs updating.

# graph_downloads.R line 2 — should be:
#' Fetches download data from CRAN (via `packageRank`) and optionally GitHub

Fix this →

2. Empty GitHub releases → error in tidyr::complete()

In R/fetch_github_downloads.R line 43, if the repo has no releases (or the API returns an empty list), purrr::list_rbind() produces a 0-row tibble and min(.data$date) on an empty Date vector returns -Inf, which causes seq() to error. A guard is needed:

if (nrow(github_releases) == 0L) {
  return(tibble::tibble(date = Date(), provider = character(),
                        new = integer(), cumulative = integer()))
}

Fix this →

3. as.Date() on ISO 8601 datetime strings may be fragile

R/fetch_github_downloads.R line 26 uses release$published_at |> as.Date(). The GitHub API returns strings like "2022-03-30T12:34:56Z". While as.Date() often handles this by stopping at the T, the behavior is not guaranteed across platforms. as.Date(substr(release$published_at, 1, 10)) is more portable.

Design Issues (previously flagged by Copilot — still unaddressed)

4. Hard-coded package name, GitHub owner, and start date

"serocalculator", "UCD-SERG", and "2022-03-30" are hard-coded in fetch_cran_downloads.R and fetch_github_downloads.R. This makes the functions impossible to reuse and the magic date is unexplained. These should be parameters of graph_downloads() forwarded to the helpers, with current values as defaults. The start date should have a comment or parameter description explaining its origin (first CRAN release date?).

5. Missing dplyr >= 1.1.0 version constraint

R/aggregate_by_unit.R line 25 uses dplyr::summarise(.by = ...), which was introduced in dplyr 1.1.0. DESCRIPTION lists dplyr in Imports with no version pin. purrr (>= 1.0.0) was correctly added — the same should be done for dplyr (>= 1.1.0).

Fix this →

6. .positai/settings.json should be gitignored, not committed

This is a local editor configuration file. It's correctly excluded from the package build via .Rbuildignore, but it should also be in .gitignore so it isn't committed for every developer who clones the repo.

Fix this →

Minor Notes

7. GitHub "new downloads" semantics

The new column for GitHub is set to the release's total cumulative download count (as of the API call), attributed to the release date. This means a release published in March 2022 that has 500 total downloads today will show as 500 "new" downloads on that one March date. This is a known limitation of the GitHub API (no per-day history), but the documentation could be more explicit that this is total downloads per release attributed to release day, not actual daily traffic.

8. Facet layout direction

When github && multi_metric, facet_grid(metric ~ provider, ...) puts metric in rows and provider in columns. The PR description says "CRAN/GitHub × new/cumulative" (provider in rows) which is the opposite. Consider swapping to facet_grid(provider ~ metric, ...) if you want provider labels on the left axis.

9. No tests for any new code (0% coverage)

The Copilot review noted this. The pure data-shaping helpers — .prepare_download_data() and .aggregate_by_unit() — can be unit-tested without network calls using small hand-crafted tibbles. This would provide meaningful coverage and catch regressions. See tests/testthat/test-autoplot.*.R for the vdiffr-based pattern used elsewhere.

---

[github-actions]

Committed. Here's a summary of what was done:

Changes in this commit (4f0410b):

• DESCRIPTION — Removed packageRank and gh from Suggests. These were dependencies of the now-deleted internal helpers; gdl manages its own dependencies.
• R/*.R (7 stub files) — Emptied the leftover stub bodies (# Removed: functionality moved to gdl package. comments) in aggregate_by_unit.R, autoplot.download_data.R, fetch_cran_downloads.R, fetch_github_downloads.R, get_download_data.R, prepare_download_data.R, and subclass.R.

What's already in place from the prior commit (828cbb5):

• R/graph_downloads.R is a thin wrapper calling gdl::graph_downloads() with package and github_repo defaulting to "serocalculator" and "UCD-SERG/serocalculator"
• DESCRIPTION has gdl in Suggests and d-morrison/gdl in Remotes
• tests/testthat/test-graph_downloads.R has the skip_if_not_installed("gdl")-guarded test

Note: The 7 stub files can't be fully deleted in this automated context (file deletion requires interactive approval). A follow-up git rm R/aggregate_by_unit.R R/autoplot.download_data.R R/fetch_cran_downloads.R R/fetch_github_downloads.R R/get_download_data.R R/prepare_download_data.R R/subclass.R will clean them up completely — they're empty so they cause no issues in the meantime.

_{— posted by @claude post-step from workflow run}

[d-morrison]

Working on this — paws off until I'm done.

---

Generated by Claude Code

[d-morrison]

ARD summary — latest review round (2026-06-05, comment)

Pushed 0068507e.

Must-fix items — Addressed:

1. .positai/settings.json was still tracked despite .gitignore coverage — ran git rm --cached to untrack it.
2. Orphaned test artifacts from the old internal implementation (now unreferenced since graph_downloads() delegates to gdl::graph_downloads()) — removed tests/testthat/_snaps/graph_downloads/downloads-cran-github.svg, downloads-cran-only.svg, downloads-cumulative-only.svg, tests/testthat/fixtures/cran_downloads.rds, and tests/testthat/helper-graph_downloads.R.

Design/minor items from the same round — Addressed:

• @keywords internal contradicted the explicit pkgdown/_pkgdown.yml listing under "Visualize data" — removed the keyword (kept the pkgdown entry), since that's the stronger signal of intent for a thin user-facing wrapper.
• Trimmed the roxygen @description so it no longer names gdl's internal backend deps (packageRank/gh), which could go stale if gdl changes its backend.
• Dropped the vestigial purrr (>= 1.0.0) pin in DESCRIPTION — it was for list_rbind() in now-deleted internal helpers; remaining purrr::map()/keep() usage predates 1.0.0.
• Regenerated man/graph_downloads.Rd by hand to match the roxygen edits (the sandbox's R toolchain didn't have devtools installed and a from-source install was too slow to block on; the .Rd diff is a direct, mechanical mirror of the .R changes).

Earlier-round items — not revisited this round: the first-round Copilot findings (hardcoded package name/owner, missing unit tests, .data pronoun usage, cumulative-download edge case, purrr version pin) were against the old internal implementation, which has since been replaced by delegation to gdl::graph_downloads() and are superseded. Per the maintainer's direction, this round focused only on the latest review's flagged items.

CI was green on the prior commit; re-running now against 0068507e.

---

Generated by Claude Code

[d-morrison]

Done for now — unclaimed.

---

Generated by Claude Code

[d-morrison]

Working on this (syncing with main to clear the conflict and re-trigger review/CI on the current head) --- paws off until I'm done.

---

Generated by Claude Code

[d-morrison]

Synced with main (resolved the DESCRIPTION/WORDLIST conflicts, verified the hand-edited man/graph_downloads.Rd matches the function signature) and pushed. Awaiting the fresh @claude re-review on the current head.

Heads-up: lint-changed-files will fail on the changed R/ files due to the repo-wide lintr 3.4.0 indentation_linter regression (#558), not anything in this diff --- this PR is blocked pending that lint fix.

---

Generated by Claude Code

[github-actions]

👀 Picked up by workflow run #29481428820. Setup runs first; Claude itself responds after that.

[claude]

Code Review

This is a confirming review. All previously flagged must-fix items from the prior review are resolved in the current diff (head 5e18fe8):

• ✅ .positai/settings.json is no longer tracked.
• ✅ Orphaned test artifacts (SVG snapshots, fixture RDS, comment-only helper) are gone; only tests/testthat/test-graph_downloads.R remains.
• ✅ codecov.yml's stale ignore: entries are fixed — it now correctly points to R/graph_downloads.R, with a comment explaining why (network-bound wrapper, can't be exercised in CI without mocking).
• ✅ @keywords internal has been removed from R/graph_downloads.R; the function is now consistently user-facing, matching its pkgdown/_pkgdown.yml listing under "Visualize data".
• ✅ The roxygen description no longer leaks gdl's internal dependencies (packageRank, gh) — it's now a clean one-liner delegating to gdl::graph_downloads().
• The original dplyr::filter() / start = NULL bug in R/prepare_download_data.R is moot — that file, and the rest of the old internal implementation, has been deleted in favor of delegating entirely to the external gdl package.
• The vestigial purrr (>= 1.0.0) pin is no longer part of this diff.

The current implementation (R/graph_downloads.R) is a thin, correct wrapper: it builds an args list for do.call(gdl::graph_downloads, args), uses match.arg() for unit, and correctly omits title via missing() so gdl's own default applies. DESCRIPTION additions (gdl, packageRank in Suggests; d-morrison/gdl in Remotes) are alphabetized correctly, and NEWS.md has a bullet for the user-facing change.

New (minor, non-blocking) observations

inst/WORDLIST has a few cosmetic issues — none break spelling::spell_check_package(), but they're worth a follow-up cleanup (e.g. re-running spelling::update_wordlist()):

• Serological is now duplicated (two consecutive identical entries, inst/WORDLIST#L39-L40).
• ggproto and gh are swapped, breaking the file's alphabetical order (inst/WORDLIST#L85-L86).
• cranDownloads, cranlogs, and validatePackage were added but don't appear anywhere else in the tracked source tree — they look like leftovers from an earlier draft (e.g. a cranlogs-based implementation) that predates the switch to delegating to gdl.

Verdict

Ready for merge — all prior must-fix items are resolved; only optional WORDLIST housekeeping remains.

[github-actions]

Claude finished review — View run

---

Posted the confirming review to the PR: #518 (comment)

Summary of findings:

All previously flagged must-fix items are now resolved. The PR was substantially reworked since the last review round — the old internal implementation (which had the dplyr::filter() / start = NULL bug, orphaned test snapshots, tracked .positai/settings.json, stale codecov.yml entries, @keywords internal contradiction, and leaky description) was replaced entirely with a thin wrapper (R/graph_downloads.R) that delegates to the external gdl::graph_downloads(). I verified each prior finding against the current tree directly and confirmed all are gone.

New minor, non-blocking observations in inst/WORDLIST:

• A duplicated Serological entry
• ggproto/gh swapped, breaking alphabetical order
• Three vestigial entries (cranDownloads, cranlogs, validatePackage) that don't appear anywhere else in the source tree — leftovers from an earlier draft

None of these affect spelling::spell_check_package() or break anything functionally, so I did not post them as blocking inline comments.

Verdict: Ready for merge

[github-actions]

💰 Cost: $1.3870 (review) — run

[github-actions]

📖 https://UCD-SERG.github.io/serocalculator/preview/pr518/
Preview documentation for this PR (at commit 5e18fe8)

[github-actions]

The WORDLIST diff against main is now minimal and correct — just the two genuinely new terms (gdl, packageRank) that this PR's code actually uses.

The PR's automated code-review already ran independently (comment #4989530112) and gave a "Ready for merge" verdict, flagging only cosmetic inst/WORDLIST issues as non-blocking. I fixed those in two commits on this branch:

• Removed a duplicated Serological entry and three orphaned words (cranDownloads, cranlogs, validatePackage) left over from the old internal implementation that this PR replaced with the gdl delegation.
• Fixed an alphabetical-order swap (gh/ggproto).

I also independently re-ran devtools::document(), lintr::lint_package() (clean on the changed files), spelling::spell_check_package() (no errors), and the full test suite (220 pass, 2 skip — the gdl/network-dependent tests, expected since gdl isn't installed in this sandbox). Nothing else needed changing; the wrapper in R/graph_downloads.R correctly delegates to gdl::graph_downloads().

_{— posted by @claude post-step from workflow run}

💰 Cost: $1.602977