fix: seeder growth pass compounds across runs

Andrew Dunnrequested to merge
fix/seeder-growth-compound into main

What this fixes

Repeated manifold seed-growth invocations did not accumulate. On the June 1,000-user run, six growth passes produced 3,558 HTTP 403 "not allowed to push" and 1,460 HTTP 409 "branch already exists", improvement topped out near 5% of users, and no user ever declined.

Three things were wrong. Two are defects, confirmed at the line level against the recorded diagnosis; the third is a missing capability.

Root cause

1. Refs were not unique per run (confirmed, and worse than recorded)

Branch and file names were a pure function of the user — seed/<user>-pushes, seed/<user>-ci — or of a merge-request counter that restarts at zero on every invocation, seed/mr-<n>-<user>. Run N therefore asked GitLab to create refs run N-1 already owned.

The recorded diagnosis stopped at the 409. The 409 was in fact tolerated (logged at debug, execution continued). The damage was one step later: GitLab rejects a second open merge request on the same source branch, so the merge request silently failed and the pass added no code-review signal. That, not the 409 itself, is why improvement capped at ~5%.

2. Membership reconstruction read a field the API does not return (confirmed; precise mechanism identified)

discoverUsers rebuilt each user's group membership from GET /api/v4/users/:id/memberships, decoding into:

type membership struct {
    ID int `json:"id"`
}

That endpoint has no id field. Its entries key the source namespace as source_id (lib/api/entities/user_membership.rb exposes source_id, source_full_name, source_members_url, created_at, expires_at, access_level). Every membership therefore decoded as the zero value, known[0] was never true, and every discovered user came back with an empty GroupIDs.

With GroupIDs empty, randomProjectForUser falls through to randomProject() — any seeded project on the instance. Users were handed projects their PAT had no write access to. That is the 3,558× 403.

Two knock-on effects the diagnosis did not record: the growth reviewer picker also filters on GroupIDs, so no growth MR ever got a reviewer (another reason code-review signal never moved), and the mocked fakeGitLab in growth_test.go encoded the wrong API shape, which is why a substantial test suite never caught this.

The recorded diagnosis said the seeder "no longer knows which users are members of which projects". Correct in effect. The correction: it did query the API, on every run; it just could not decode the answer.

3. No decay model (confirmed)

Accurate as recorded.

Design

(a) Per-run ref namespacing

New mints a run ID (UTC timestamp to the second plus four hex digits of entropy, lower-case alphanumeric so it is legal in both a git ref and a repository file path; overridable via Config.RunID for tests). Every branch and file a run writes carries it. createRunBranch centralises creation and logs a 409 at warn, not debug — with per-run naming, a conflict now means the run IDs collided, which is worth seeing.

(b) Membership reconstruction

Membership is indexed from the group side: GET /api/v4/groups/:id/members/all, where id really is the user ID. This is both the correct shape and cheaper — O(groups) requests rather than O(users), five instead of a thousand on the 1,000-user run.

Three layers:

  1. buildMembershipIndex — rebuilds user → groups at discovery.
  2. reconcileMemberships — repairs anyone the index cannot place, deterministically (by user ID modulo group count), before any token is minted. Waits for GitLab's membership cache only when it actually added something.
  3. ensureMembership — per-write idempotent net, consulted from the in-memory map first so the common case costs no API call. Treats "already a member" (409, and the 400 variant) as success.

(c) Decay — viability analysis

Verdict: viable seeder-side, but only for assignment-shaped signal. No analyzer change needed.

A user declines when ComputeTrends sees their average per-domain delta fall below -TrajectoryThreshold (−0.05, i.e. a −0.3 sum across six domains). Two properties of the pipeline bound what can move that:

  1. collector.window() is rolling — now − WindowDays (default 90). Two collects taken hours apart in a demo cover essentially the same window, so nothing ages out. Waiting for activity to expire is not available on a demo clock.
  2. Every scorer is monotone non-decreasing in counts, and most counts come from the append-only events API. Pushes and comments cannot be taken back.

So event-shaped signal can only grow. What can fall is assignment-shaped signal, because the collector reads the current state of each in-window record, not its state at creation:

  • collector/mrs.go fills ReviewerIDs from the MR's live reviewers array. Removing a reviewer removes both reviews_given and the cross_project_reviews that follow from it — 0.4 and 0.25 of code_review's raw weight.
  • collaboration counts distinct projects reached via IssuesByAssignee, so handing an assigned issue off shrinks its cross-project term.

Sizing against the 200-user reference distribution: a reviewer-archetype user carries code_review ≈ 0.44 normalized. Withdrawing it is a −0.44 single-domain delta, i.e. an average of −0.073 across six domains — past the −0.05 threshold on that domain alone, before the collaboration term.

This compounds with the normalizer rather than fighting it. NormalizeScores is a population min-max over winsorized, log1p-re-expressed raw scores, so a flat user's normalized score already falls as the growth cohort lifts the p95 ceiling: Δ = v_A · (L_A/L_B − 1) where L = log1p(p95). That effect alone is far too small at realistic growth volumes (it needs the p95 to rise ~50% in raw terms to move a typical user past the threshold), which is exactly why the June run saw zero decliners even where growth partially succeeded. The withdrawal supplies the decisive magnitude; the moving ceiling adds to it.

The honest limit, and why the cohort is filtered. A user with no withdrawable assignment can only be moved by the ceiling effect, which will not clear the threshold. So the decay cohort is drawn only from reviewer, platform_engineer, and emerging_user — the archetypes the baseline gives review and assignment signal to. Builders and solo contributors are excluded by design (the baseline deliberately gives them no reviewers; their score is almost entirely event-shaped). Dormant experts are excluded for a different reason: they carry plenty of withdrawable signal, but decaying them would dismantle the archetype the demo instance exists to show.

applyDecay also keeps walking candidates until it has actually withdrawn something from the requested number of users, so a candidate the baseline happened to leave no assignments on does not consume a cohort slot and quietly under-deliver.

Cohort selection draws one permutation and slices it, so the two cohorts are disjoint by construction and the growth cohort a given --seed selects is byte-identical to what it selected before decay existed — a pinned seed still reproduces earlier runs.

CLI

New optional flag, backward compatible:

--decay-fraction float
      Fraction of non-dormant seeded users to decay. A cohort disjoint from the grown
      users has its review and issue assignments withdrawn, so the second collect
      ranks them lower and the trends view shows declining users (0 disables) (default 0.1)

Every pre-existing seed-growth invocation parses and behaves as before, apart from now also decaying 10% by default. --decay-fraction 0 restores the purely additive pass.

Test coverage

New internal/seeder/compound_test.go, plus a corrected mock in growth_test.go.

Branch-name uniqueness across two simulated runs

  • TestRunBranchNamesAreUniquePerRun, TestRunIDIsRefSafe
  • TestTwoGrowthRunsProduceDisjointBranches — two full RunGrowth passes against a fake that answers duplicate branch creation with 409 as GitLab does; asserts zero conflicts.
  • TestSecondGrowthRunKeepsAddingMRs — the second pass must add as many MRs as the first, i.e. it compounds.

Membership re-add before push

  • TestBuildMembershipIndexReadsMembersListing — fed the real /groups/:id/members/all payload.
  • TestBuildMembershipIndexRejectsUserMembershipsShape — fed the source_id-shaped payload the old code read; asserts it produces an empty index rather than silently indexing user 0. If anyone points the index back at that endpoint, this fails instead of the next 1,000-user run.
  • TestEnsureMembershipAddsBeforePush, TestAddGroupMemberTreatsConflictAsSuccess (409 and 400 variants), TestAddGroupMemberPropagatesRealFailure, TestAccessLevelForMatchesBaseline
  • TestGrowthReestablishesMembershipBeforeWriting — full pass against a roster with no memberships (the state the bug effectively produced); asserts every written project's group had membership established first.

Decay cohort selection determinism

  • TestSelectCohortsDeterministic (cohort, candidate order, and target size), TestSelectCohortsDisjoint, TestSelectCohortsDecayEligibilityOnly, TestSelectCohortsZeroDecayDisables
  • TestSelectGrowthSubsetUnchangedByDecaySplit — guards seed reproducibility against the old behaviour.
  • TestApplyDecayWithdrawsAssignments (co-reviewers preserved; GitLab's documented [0] clear value used), TestApplyDecaySkipsUnseededProjects, TestApplyDecayIsIdempotent, TestApplyDecaySkipsCandidatesWithNoSignal, TestApplyDecayStopsAtRequestedCount, TestUnassignIDs, TestWithoutUser

Each guard was mutation-tested: reverting the branch-naming fix fails TestTwoGrowthRunsProduceDisjointBranches with 22 conflicts; disabling membership repair fails both membership tests.

Verification

  • make check — pass
  • go test ./... — pass
  • golangci-lint run (v2.12.2, the CI version) — 0 issues
  • go test -race — pass
  • Coverage 72.2%, above the 65% floor
  • Branch pipeline 2751480791 green (build, test, golangci-lint, lint, code_quality, dependency scanning, secret detection, SAST)
  • Astro docs site builds clean locally (the build-docs job runs only on MR pipelines)

Residual risks for the 1,000-user GCE re-seed

  1. Decay depends on baseline assignment density. The cohort is capped by how many reviewer/platform_engineer/emerging_user accounts actually received reviewer assignments. If the baseline seed was thin there, applyDecay logs decay cohort under-filled and delivers fewer decliners than asked. Watch that line; raise --decay-fraction or re-seed the baseline if it fires.
  2. Threshold proximity. A decayed reviewer lands around −0.07 average against a −0.05 threshold. Comfortable but not enormous. Users whose code_review was already near zero will not cross it.
  3. Membership propagation. membershipSettleDelay is 20s and paid once per pass, only when something was added. On a large instance under load, GitLab's membership cache may need longer; a first-write 403 immediately after an ensureMembership add is possible. It is logged and skipped, not fatal.
  4. Group-member pagination cost. buildMembershipIndex pages at 100. Five groups × ~1,000 users is ~50 requests — trivially cheap, but it now runs before every growth pass.
  5. Ref accumulation. Per-run branches mean refs accumulate across passes. Six passes × ~400 growth users is a few thousand branches spread over 20 projects. Harmless for a throwaway demo instance; seed --clean deletes the groups wholesale. Worth noting if anyone ever wants to run dozens of passes.
  6. Decay edits are not reversible by the seeder. Withdrawn reviewer/issue assignments are gone. Re-establishing them means re-seeding. The pass only touches projects it discovered this run, so a shared instance is protected, but there is no undo.
  7. Not exercised against real GitLab. All verification is against mocks plus the analyzer's actual scoring math. The first real signal will be the GCE re-seed itself. Recommend one dry pass at low --fraction with a --seed pinned, then check the run log for memberships_reestablished, decayed_users, and any 403/409 before the full six-pass sequence.

Merge request reports