Skip to content

fix(profile): reliable org crawl (REST discovery + two-pass GraphQL) plus repo search/pagination - #26

Merged
cvega merged 11 commits into
mainfrom
feat/profile-summary
Jun 15, 2026
Merged

cvega merged 11 commits into
mainfrom
feat/profile-summary

Conversation

@cvega

@cvega cvega commented Jun 15, 2026

Copy link
Copy Markdown
Owner

What

Follow-on work to PR #25 (which merged at 949dbec). These 11 commits make the Profiler's org crawl actually reliable against a real organization — the bundled GraphQL discovery was timing out with 502 Bad Gateway (GitHub's ~10s query limit) and failing the whole run before it could even list the repos — and add repo search + pagination to the profile page. Also renames the machine-readable verify script.

Every commit is green (bun run gates — formerly verify:json — all 11 gates).

The crawl reliability arc (the headline)

The profiler kept failing with POST /graphql … 502 … in ~10s on a real org. Diagnosed and fixed in layers:

  • e8f9c9d perf: shrink the augment query (first cut at the 502).
  • 3f50fdf perf: batch the cheap totalCount pass wide; add fork/ruleset signals.
  • a4e6efe fix: scope repo rulesets to includeParents: false (was double-counting org rulesets) and handle the nullable rulesets connection.
  • a76640c fix: survive GraphQL timeouts (split-on-timeout recursion) + add crawl observability.
  • bd856df fix: cap augment repo-batches at 10–15.
  • 9ae4d01 refactor: two-pass crawl — a cheap indexed-totalCount counts pass first (fills the page fast, basically can't time out), then an expensive verification pass (commits/LFS/workflows/release sizes/branch-protection detail) that's non-fatal and degrades per-repo.
  • c51b0c5 fix: make discovery 502-resilient and the repo list non-fatal (record every repo up front; enrichment never empties the list).
  • 5920242 refactor: list repos via REST (GET /orgs/{org}/repos) instead of GraphQL — an indexed, paginated read that never hits the 10s query limit — then gather the indexed counts via the batched GraphQL pass. This is the decisive fix.

Net effect: discovery is a reliable REST listing, enrichment is best-effort GraphQL, and a single transient 502 can no longer fail the run or hide the repo list.

UI

  • 856a448 feat: client-side search + pagination on the profile repo list (every repo is already loaded and live-refreshed, so filtering/paging is instant).
  • 720cc4b refactor: swap the bespoke footer for the shared $lib/components/Pagination.svelte (numbered pages + range summary) so the profile page matches the migrate pages.

Tooling

  • ff4ec7a chore: rename the verify:json script → gates (scripts/gates.ts) — it runs the 11-gate suite, so the name now says what it does. Also aligned two gate file names to their script keys (check-coverage.ts→coverage-check.ts, check-boundaries.ts→boundaries.ts). Updated package.json, bunfig.toml, Makefile, the CI verify workflow, and docs.

Verification

  • bun run gates — all 11 gates green; coverage 88.03 func / 86.69 line.
  • New/changed modules covered; discovery rewrite covered by tests exercising the real REST pagination shape.
  • Runtime smokes: real Octokit REST pagination against a local two-page fake GitHub (Link-header paging, visibility normalization, empty-proxy); and a 60-repo seeded run confirming search filters + numbered pagination ("1–25 of 60", page 2 → "26–50 of 60").

cvega added 11 commits June 14, 2026 21:05
The augment pass was timing out (POST /graphql -> 502 at ~10.4s, GitHub's
10s execution-timeout). Root cause was per-repo resolver cost multiplied
across aliased repos, not node count. Researched against GitHub's GraphQL
limits docs and cut the expensive parts:

- Remove commit count (history.totalCount). This is the single most
  expensive resolver — it walks the entire commit graph (no precomputed
  count) — and it's pure scale decoration (git history migrates
  wholesale). Dropped from the query, RepoSignals, scale rollup, and UI.
  Also drops the defaultBranchRef resolution it required.
- Trim the release-asset scan: releases 100->50, assets 50->30
  (deeply-nested connection = the docs' flagged timeout anti-pattern).
- Trim branch-protection rules first: 100->50 (totalCount stays exact;
  ~all repos have <10 rules).
- Smaller aliased batches: FULL 25->10, LITE 90->15 (fewer simultaneous
  large-first connections per request — the exact pattern GitHub warns
  causes timeouts), processed 3 concurrently to keep wall-clock down.

Net: each augment request does far less server-side work, staying well
under the 10s window. Commit count can be restored later via a cheap
per-repo REST call if needed.

11 gates green; coverage 87.76/86.40.
…t signals

Grounded in GitHub's GraphQL node-limit + timeout docs: a { totalCount }
selection (no nodes/edges) doesn't materialize nodes — it's an indexed
read — and the 10s timeout is driven by node materialization, not
counts. So the cheap pass can batch wide; only the release-asset scan
(a deeply-nested walk) must stay narrow.

- Widen the no-releases augment batch 15 -> 50; release-bearing 10 -> 15.
  (The old low cap existed only because the query also walked the commit
  graph via history.totalCount, which was removed in the prior commit.)
- Add two more cheap signals while we're paying for the round trip:
  forkCount (scalar) and repository rulesets.totalCount. The latter
  lights up the per-repo 'rulesets' consideration, previously stuck
  'not yet evaluated'.
- Deliberately NOT adding vulnerabilityAlerts: it's permission-sensitive
  and a per-repo auth error would null the whole repo node, poisoning the
  cheap counts. That belongs in its own isolated pass.
- UI: per-repo drill-down shows Forks + Rulesets.

Net for N repos with few release-bearing: far fewer augment requests,
each doing only indexed reads — no timeout risk — while gathering more
signal per call.

11 gates green; coverage 87.76/86.40.
Read the Repository.rulesets schema reference, which revealed two bugs in
the field added last commit:

1. rulesets(includeParents:) defaults to TRUE, so totalCount counts
   org/enterprise rulesets that apply to the repo - which we already
   gather separately as run.orgRulesetCount. Every repo was double-
   counting the org's rulesets. Set includeParents:false to scope the
   per-repo signal to rulesets actually defined on the repo.

2. RepositoryRulesetConnection is nullable in the schema (no bang, unlike
   discussions/packages/etc. which are Connection-bang). A viewer without
   ruleset read access gets null, which would have thrown on
   node.rulesets.totalCount. Guard with optional-chain plus nullish-zero.

Both determinable straight from the GraphQL schema reference - no live
org needed. Added a null-connection test.

11 gates green; coverage 87.76/86.40.
The profiler was failing the whole run when GitHub returned a 502 on an
augment query. That 502 is GitHub's own API edge (the request id format
and the ~10.4s duration are GitHub's 10-second GraphQL processing
timeout, which their docs say returns 502/504) - not a local proxy.
A 502 carries no partial data, so the chunk error propagated and failed
the entire crawl.

Resilience (the real fix):
- augmentRepoSignals now detects a timeout (status 502/504 or a timeout
  message) and splits the chunk, recursing on the halves until each query
  is cheap enough to evaluate in GitHub's window. A single repo that
  still times out degrades to zeroed augment signals (keeping its
  discovery spine) so one pathological repo cannot fail the run. This
  self-adapts to whatever the org/instance can handle.

Observability (the run was silent):
- runner now logs start, discovery total + timing, each augment-chunk
  failure (size + scanReleases), and completion/failure with timing.
- failures get a richer reason via describeError (HTTP status + GraphQL
  error message) instead of a bare message, shown on the detail page.
- discovery progress is wired so the run total is set from the first
  discovery page and watchers are nudged - the page shows "Discovering
  repositories..." then "Profiling N repositories..." instead of 0/0.

UI:
- the empty/working state is now phase-aware (discovering vs profiling).

Also adds X-Accel-Buffering:no to the SSE response headers. This does
nothing for local dev (no proxy) but is correct hardening for the
documented reverse-proxy deployment (docker-compose + ORIGIN), where a
proxy would otherwise buffer the event stream.

11 gates green; coverage 87.76/86.38.
The repo-batch count (how many repos are aliased into one totalCount
request) must stay at 10-15 max. Live 502s showed the per-repo cost (two
git-object reads plus, for release-bearing repos, the release-asset
scan) is higher than node-limit theory predicts, so the earlier widening
to 50 timed out. Back to FULL=10 (release-bearing) / LITE=15 (no
releases). The split-on-timeout fallback still backstops any chunk that
times out.

These are the repo-batch counts only; MAX_RULES and RELEASES_SCANNED are
per-repo query depths (first: args within one repo), a different axis,
left unchanged.

11 gates green.
…later

The bundled augment query did too much per repo (git-object reads, a full
commit-history walk, and a release-asset scan) and tripped GitHub's 10s
GraphQL execution limit, returning 502/504 on larger orgs.

Split augmentation into two passes:

- Counts pass (augmentRepoCounts, batches of 15): only indexed totalCount
  reads (discussions, projects, environments, stars, forks, watchers,
  packages, rulesets, branch-protection count). These materialize no nodes,
  so they're cheap and basically can't time out — they fill the page fast.
  This pass is fatal: if it fails the run fails.

- Details pass (augmentRepoDetails, batches of 10 with releases / 15 without):
  the expensive verification signals — commits (history.totalCount), LFS,
  workflow files, release-asset bytes, branch-protection detail. Best-effort
  and non-fatal: a chunk that errors is logged and skipped, and a single repo
  that times out degrades its fields rather than blocking the run.

Both passes run a concurrency-3 worker pool and split-on-timeout (recurse on
halves; a lone repo degrades) as a backstop. Commits is restored as a stat,
now gathered in the details pass where a timeout safely degrades it to 0.

Batches stay at 10-15 repos. Adds per-chunk error observability.
The org profile run failed with a bare "HTTP 502: ... nginx" and showed zero
repos. Root cause: discovery (the one GraphQL call with no timeout handling)
propagated GitHub's 10s-timeout 502 straight up and failed the whole run, and
repos were only persisted in the counts pass — which was fatal — so a single
502 anywhere left the user with no list at all.

Reworked to match the intended pipeline (get the list first, enrich later,
best-effort):

- discover.ts: on a 502/504 timeout, halve the page size and retry the SAME
  cursor (down to a floor of 10), keeping the smaller size for the rest of the
  crawl. A 100-wide page of indexed counts is normally cheap, but an org with
  repos that have very many refs/issues can tip one page over the limit; this
  lets discovery finish instead of failing. Non-timeout errors still surface.

- runner.ts: record every discovered repo immediately with the discovery spine
  (augmented fields zeroed), so the detail page shows the full list the moment
  discovery finishes. The counts pass is now best-effort — a chunk that fails
  for a non-timeout reason is logged and skipped, leaving those repos at their
  base signals rather than failing the run. The details pass was already
  non-fatal.

- graphql-errors.ts: extract the shared `isTimeoutError` predicate (used by
  both the discovery page-shrink and the augment chunk-split).

The augment passes already split-and-degrade on a 502, so they never propagate
one; this closes the last gap (discovery) and guarantees the list survives any
single transient failure.
…phQL

Discovery used a single 100-wide GraphQL page carrying several connection
totalCounts (issues, PRs, branches, tags, releases) per repo. On a large org
that one query sits right at GitHub's ~10s execution limit and 502s
intermittently — so the run failed before producing even a list of repos.

Split the two concerns the way they want to be split:

- discover.ts now lists the org via REST `GET /orgs/{org}/repos` (the
  `minimal-repository` shape), paged through the Link header. REST listing is
  an indexed, paginated read that never hits the GraphQL timeout — it reliably
  produces the repo list. It fills the discovery spine (name, visibility, fork,
  archived, size, feature toggles, default branch, timestamps); `isEmpty` is
  proxied from `size === 0` (REST has no is_empty).

- The five content-volume counts move from the discovery spine onto RepoSignals,
  gathered by the existing batched GraphQL counts pass (15-wide, splits-on-
  timeout, degrades a repo to zeros rather than failing). They're indexed
  totalCounts, cheap at that fan-out.

- The runner now takes a { gql, rest } client bundle: rest drives discovery, gql
  drives the augment passes. The details pass partitions by the releases count
  from the counts pass (it's no longer on the spine).

Verified the real Octokit REST pagination end-to-end against a local two-page
fake (Link-header paging, visibility normalization, empty proxy). 118 profile
tests pass; all gates green.
The repositories table rendered every profiled repo in one long list. Added a search box (filters by owner/name) and 25-per-page pagination, both client-side since every repo is already loaded and live-refreshed — so filtering and paging are instant. Typing resets to page 1; the page index clamps when the filtered set shrinks; a footer shows the visible range and total (with a 'filtered from N' note); a no-match empty state appears when a search yields nothing.
The profile repo list had a bespoke Prev/Next footer that looked different from the migrate pages, which use the shared $lib/components/Pagination.svelte (numbered pages with ellipsis + range summary). Swapped the profile list onto that same component so both surfaces are visually consistent. Its onPageChange callback drives the existing client-side page state, and a migrate-style count now sits in the section header. Removed the duplicated footer markup and the now-unused range deriveds.
verify:json described the output format, not the purpose (running the full gate suite). Renamed the script to 'gates' (scripts/gates.ts) — short, and it literally runs the 11-gate suite. Also aligned the two gate file names to their package.json script keys (convention: file name = script key with ':' replaced by '-'): check-coverage.ts to coverage-check.ts (key coverage:check unchanged), check-boundaries.ts to boundaries.ts (key boundaries unchanged). Updated every reference: package.json, bunfig.toml, Makefile, the CI verify workflow, and the development/architecture/CONTRIBUTING docs. The coverage:check and boundaries script keys are unchanged, so the verify chain and CI are untouched beyond the file paths. CHANGELOG.md left as-is (historical).
@cvega
cvega merged commit 79b9fd5 into main Jun 15, 2026
2 checks passed
@cvega cvega mentioned this pull request Jun 15, 2026
@cvega
cvega deleted the feat/profile-summary branch June 18, 2026 08:25
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.

1 participant