From 8cc31d3d97412ecae13920cf73c56a1d1498efed Mon Sep 17 00:00:00 2001 From: tiennm99 Date: Sat, 18 Apr 2026 22:55:32 +0700 Subject: [PATCH] docs: refresh for context plumbing, rate-limit handling, phase 6 - Fetcher signatures across codebase-summary and system-architecture now show the ctx-first arguments and document the rate-limit retry loop in Client.query. - Attribution pseudo-code hoists the per-repo total out of the commit loop to match the current implementation (I6). - Failure-modes table enumerates primary rate-limit retry, per-year nil-user warn, and -timeout / Ctrl-C cancellation. - design-guidelines notes the single-slice donut special case. - deployment-guide's release section documents the new test gate and the SHA-pinned Docker/GHA actions; troubleshooting adds the rate-limit-reset-too-long error. Rate-limit section describes the sleep-and-retry policy and -timeout flag. - project-roadmap records Phase 6 (code-review remediation) as done, renumbers later planned phases, links the new review report. --- docs/codebase-summary.md | 10 +++++----- docs/deployment-guide.md | 16 +++++++++++----- docs/design-guidelines.md | 2 ++ docs/project-roadmap.md | 24 ++++++++++++++++++++---- docs/system-architecture.md | 27 ++++++++++++++++----------- 5 files changed, 54 insertions(+), 25 deletions(-) diff --git a/docs/codebase-summary.md b/docs/codebase-summary.md index 87c2ea7c..9eb1f709 100644 --- a/docs/codebase-summary.md +++ b/docs/codebase-summary.md @@ -46,15 +46,15 @@ ghstats/ ### `internal/github` -All network I/O. Exposes a `*Client` with three fetchers: +All network I/O. Exposes a `*Client` with three fetchers; every call takes a `context.Context` so pagination aborts cleanly on timeout or Ctrl-C: | Fetcher | Input | Populates | | --- | --- | --- | -| `FetchProfile(login, opts)` | username, visibility flags | Profile basics, totals, owned-repos aggregation, last-year daily calendar, `TopRepos` | -| `FetchContributionsAllTime(p, opts)` | Profile | `SeedRepos`, `DailyContributionsAllTime`, `TotalCommitsAllTime` | -| `FetchProductive(p, repos, loc, cap)` | Profile + seed + tz + cap | `Productive`, `CommitsByLanguage`, `ProductiveAllTime`, `CommitsByLanguageAllTime` | +| `FetchProfile(ctx, login, opts)` | username, visibility flags | Profile basics, totals, owned-repos aggregation, last-year daily calendar, `TopRepos` | +| `FetchContributionsAllTime(ctx, p, opts)` | Profile | `SeedRepos`, `DailyContributionsAllTime`, `TotalCommitsAllTime` | +| `FetchProductive(ctx, p, repos, loc, cap)` | Profile + seed + tz + cap | `Productive`, `CommitsByLanguage`, `ProductiveAllTime`, `CommitsByLanguageAllTime` | -Call order in `main.go`: Profile → AllTime → Productive. +Call order in `main.go`: Profile → AllTime → Productive. `Client.query` handles GitHub rate limits transparently — on 429 or 403 with `X-RateLimit-Remaining: 0`, it honors `Retry-After` / `X-RateLimit-Reset` (capped at 5 minutes) and retries once. ### `internal/card` diff --git a/docs/deployment-guide.md b/docs/deployment-guide.md index 7d352db3..378875d1 100644 --- a/docs/deployment-guide.md +++ b/docs/deployment-guide.md @@ -104,10 +104,13 @@ runs: ## Release process -1. Ensure `go vet ./...` and `go test ./...` pass on `main`. -2. Tag: `git tag -a v1.2.0 -m "..." && git push origin v1.2.0`. -3. `release.yml` handles GHCR push + binary artifacts automatically. -4. Update any public Actions marketplace metadata if the major version changed. +1. Tag: `git tag -a v1.2.0 -m "..." && git push origin v1.2.0`. +2. `release.yml` runs `go vet` + `go test` as a gate before the docker and + binaries jobs. If tests fail, no artifacts ship. +3. On green, GHCR push + cross-platform binary artifacts happen automatically. +4. Docker base images and third-party actions are SHA-pinned (with version + comments) so mutable-tag changes upstream can't rewrite a released image. +5. Update any public Actions marketplace metadata if the major version changed. ## Rollback @@ -125,13 +128,16 @@ runs: No REST calls today. Future `-accurate-languages` mode will push toward 1000+ REST per run; schedule that mode less frequently (weekly, not daily). +The client auto-handles rate-limit responses: on 429 or 403 with `X-RateLimit-Remaining: 0`, it sleeps up to 5 minutes (honoring `Retry-After` / `X-RateLimit-Reset`) and retries once. A reset window longer than 5 min surfaces as an error so CI can reschedule instead of burning runner time. Use the `-timeout` flag (default 30m) to cap total fetch duration; `SIGINT`/`SIGTERM` cancels in-flight requests cleanly. + ## Troubleshooting | Symptom | Check | | --- | --- | | "error: fetch profile: graphql: Could not resolve to a User" | Username typo | | "http 401" | Token expired or lacks `read:user` | -| "http 403: rate limit exceeded" | PAT scope too narrow; token quota consumed by another workflow | +| "rate limit resets in 42m (>5m0s max wait)" | Client refused to sleep through a long window; reschedule the Action | +| "http 403" on non-rate-limit path | PAT scope too narrow | | Blank contribution chart | User has 0 contributions in their window; expected | | Private repo data missing | `-include-private=true` not set, or PAT lacks `repo` | | Nothing committed by the Action | Check `permissions: contents: write` in the workflow | diff --git a/docs/design-guidelines.md b/docs/design-guidelines.md index 10be7230..cf820390 100644 --- a/docs/design-guidelines.md +++ b/docs/design-guidelines.md @@ -59,6 +59,8 @@ Cap rows at what fits: 7 for profile, 7 for stats (commits row splits into lifet Language colors come from linguist via GraphQL (`repo.languages.edges[].node.color`). Missing colors fall back to `theme.Accent`. +When there's **exactly one slice** (one language at 100%), the renderer emits two concentric `` elements instead of a pie arc, because SVG's `A` command from point P back to the same P draws nothing. Regression guarded by `TestDonutSingleSlice`. + ## Bar-chart cards (productive time) | Metric | Value | diff --git a/docs/project-roadmap.md b/docs/project-roadmap.md index 79d49fc5..fd409db3 100644 --- a/docs/project-roadmap.md +++ b/docs/project-roadmap.md @@ -35,9 +35,24 @@ - Each commit distributes fractionally across repo's language bytes, not just primary. - Improves mixed-code repo accuracy; still inaccurate for Markdown-heavy repos (linguist prose-exclusion). +## Phase 6 — Code-review remediation (✅ done) + +Follow-up after the full-project review (`plans/reports/code-review-260418-2223-full-project.md`): + +- Donut chart's single-slice (100%) rendering no longer produces an empty arc. +- `FetchContributionsAllTime` warns on stderr when a year returns nil user data. +- `attributeCommit` receives a precomputed per-repo byte total instead of re-summing every commit. +- `Profile.TotalContributions` → `TotalContributionsLastYear` (accurate semantics). +- `context.Context` threaded through all fetchers; `-timeout` flag (default 30m); Ctrl-C cancels in-flight requests. +- Rate-limit awareness: on 429 or exhausted primary limit, honor `Retry-After` / `X-RateLimit-Reset` up to 5 min and retry once. +- Release workflow gates docker + binaries on a test job; no more shipping broken tags. +- Docker base images and third-party GitHub Actions pinned to SHA with version comments. +- Stats card label "Contributed to (non-fork)" corrected to "Contributed to" (the query doesn't filter forks). +- Tests: fixed stale XML-escape assertion, added `TestDonutSingleSlice`, added `TestUTCOffsetLabel` for half-hour zones. + --- -## Phase 6 — Per-commit file classification (planned) +## Phase 7 — Per-commit file classification (planned) **Goal**: fix the Markdown-blog misattribution case (and any repo where linguist's byte view disagrees with what files user actually edited). @@ -49,7 +64,7 @@ **Status**: designed, not implemented. -## Phase 7 — Partial bare clone for lifetime all-repo stats (planned) +## Phase 8 — Partial bare clone for lifetime all-repo stats (planned) **Goal**: lifetime language stats across **every** repo a user has committed in, without the 500-commits-per-repo cap. @@ -61,7 +76,7 @@ **Status**: researched only; behind `-deep` flag when landed. -## Phase 8 — User-configurable repo exclusion (planned) +## Phase 9 — User-configurable repo exclusion (planned) **Goal**: let users drop throwaway repos (experiments, forks they stashed) from stats without disabling forks globally. @@ -71,7 +86,7 @@ **Status**: pending user demand. -## Phase 9 — Expand ownerAffiliations (planned) +## Phase 10 — Expand ownerAffiliations (planned) **Goal**: catch work done in org repos where user is a collaborator, not owner (e.g., company monorepos). @@ -97,3 +112,4 @@ All in `plans/reports/`: - `researcher-260418-2001-accurate-language-stats.md` — metrics vs GRS vs go-enry feasibility - `researcher-260418-2012-profile-stats-survey.md` — follow-up survey across 6 more tools - `analysis-260418-2140-most-commit-language-all-time.md` — hand-reconstruction of tiennm99's card output, showing exactly why each language lands where +- `code-review-260418-2223-full-project.md` — adversarial review of the whole codebase; findings all closed in Phase 6 diff --git a/docs/system-architecture.md b/docs/system-architecture.md index e018ee5b..85dc45df 100644 --- a/docs/system-architecture.md +++ b/docs/system-architecture.md @@ -16,13 +16,15 @@ One process, three phases: **flag parsing → data fetch → SVG render**. No database, no cache, no background workers. Stateless CLI; Action runtime just sets environment variables + runs the binary. +A root `context.Context` is built in `main.go` with an overall deadline (`-timeout`, default 30m) and cancelled on `SIGINT`/`SIGTERM`. Every fetcher and HTTP request inherits it so a slow run aborts cleanly instead of draining the 6h Action budget. + ## Data-fetch sequence ``` main.go │ ▼ -FetchProfile(login, opts) +FetchProfile(ctx, login, opts) │ profileQuery × N pages (owned repos, STARGAZERS desc, 100/page) │ yields: Profile.{identity, stars, forks, PRs, issues, │ TopRepos, ReposByLanguage, @@ -31,7 +33,7 @@ FetchProfile(login, opts) │ TotalCommits (last year)} │ ▼ -FetchContributionsAllTime(profile, opts) +FetchContributionsAllTime(ctx, profile, opts) │ contributionYearQuery × len(ContributionYears) │ per year: totalCommitContributions + │ contributionCalendar.weeks + @@ -41,7 +43,7 @@ FetchContributionsAllTime(profile, opts) │ TotalCommitsAllTime │ ▼ -FetchProductive(profile, profile.SeedRepos, loc, commitsPerRepo) +FetchProductive(ctx, profile, profile.SeedRepos, loc, commitsPerRepo) │ commitHistoryQuery × (#seeds × pages) │ per commit: t = committedDate in loc │ ProductiveAllTime[t.Hour]++ + language votes @@ -74,13 +76,14 @@ Typical run (8 active years, 30 seed repos, avg 50 commits each): Language attribution for the "most commit language" card is **byte-weighted**: ``` -for each commit C in repo R: - total_bytes = Σ R.languages[*].bytes - for each (lang, bytes) in R.languages: - commits_by_lang[lang] += scaleFactor × bytes / total_bytes +for each repo R: + total_bytes = Σ R.languages[*].bytes // precomputed once per repo + for each commit C in R: + for each (lang, bytes) in R.languages: + commits_by_lang[lang] += scaleFactor × bytes / total_bytes ``` -Implementation in `internal/github/productive.go:attributeCommit`. `scaleFactor = 10_000` preserves fractional precision in int64 storage — percentages rendered in the card are unaffected by magnitude. +Implementation in `internal/github/productive.go:attributeCommit`. The per-repo byte total is hoisted out of the commit loop so the hot path doesn't re-sum language edges for every commit. `scaleFactor = 10_000` preserves fractional precision in int64 storage — percentages rendered in the card are unaffected by magnitude. Known distortion: linguist excludes prose types (Markdown, AsciiDoc, reST) from byte counts. Blog-style repos with 95% Markdown and 5% JS still attribute all commits to JS. Future fix: per-commit REST file classification via `-accurate-languages` (see roadmap). @@ -92,7 +95,7 @@ Each card produces a self-contained SVG with: - Content layer (chart elements, text, legend) Shared primitives: -- `renderDonutCard(title, stats, theme)` — pie slices via polar arc math + legend with color swatches +- `renderDonutCard(title, stats, theme)` — pie slices via polar arc math + legend with color swatches. Single-slice case (one language at 100%) renders as two concentric `` elements instead of an arc, since SVG's `A` command from point P back to P draws nothing. - `renderProductiveTime(title, hours, theme)` — 24 bars + both axes + tick math from `niceTicks` - `renderContributions(title, days, theme)` — monthly aggregation, Catmull-Rom → cubic Bezier area path, two-sided Y axis @@ -115,10 +118,12 @@ Light themes (`default`, `github`, `nord_bright`, etc.) use `StrokeOpacity: 1` w | --- | --- | | Empty `-user` | Exit 2, usage printed | | Unknown theme | Exit 2, suggests `-list-themes` | -| GraphQL 4xx/5xx | Error wrapped with HTTP status and truncated body | -| Rate limit | Bubbles up as a GraphQL error | +| GraphQL 4xx/5xx | Error wrapped with HTTP status and truncated (UTF-8-safe) body | +| Primary rate limit (429 / 403 + remaining=0) | Sleep up to 5 min honoring `Retry-After` / `X-RateLimit-Reset`, retry once; longer windows surface as error | +| Per-year query returns nil user | Warn to stderr; other years still contribute | | `FetchProductive` network error | Warn to stderr; partial data rendered | | Unknown timezone | Warn to stderr; fall back to UTC | +| Overall timeout (`-timeout`) or Ctrl-C | `ctx` cancels in-flight requests; partial data may render | | User with 0 commits | Card renders "No data available" | ## Extension points