PR 7 — Enrichment budget and fairness

[batbrainy]

Linked issue

Closes #17 — IMPLEMENTATION_PLAN.md §13, PR 7 — Enrichment budget and fairness.

Problem

Every actor and repository has been persisted as a stub marked pending since PR 3, and nothing ever transitioned one. The enum, the five statuses, the partial index, actor_share_used / repository_share_used and Request::ENRICHMENT_CLASSES all existed saying "PR 7 does this."

The allocation problem is what makes it more than a fetch loop. §10's demand arithmetic: one observed live page of /events held ~92–95 push events referencing ~89 distinct actors against ~92 distinct repositories — 181 cold entity requests per page, ~2,172 an hour, against 40 available. Repository candidates alone exceed the whole hourly allowance, so a queue ordered purely by recency starves actor enrichment to zero indefinitely, which fails Story 3 outright.

Scope

In: the entity state machine and its write matrix; per-class fairness guarantees with floor/remainder rounding and eligibility-aware borrowing; newest-first eligibility; skipped_budget with the distinct-event reactivation rule; the freshness cache and its refresh TTLs; error-context classification (entity vs source); effective_enrichment_time; a bounded backlog (B8); bin/enrich and an enrich compose service.

Deliberately out, per the plan's own assignments: Solid Queue, the worker container, EnrichActorJob / EnrichRepositoryJob, post-commit enqueue and the entity-scoped reconciler (PR 8); rich /status with coverage percentages and ENRICHMENT_COVERAGE_WINDOW_SECONDS (PR 10) — .env.example says why that one variable is absent even though §10 prints it in the same block as the three that landed; starved-class stress and the redirecting_repository / hostile_redirect fixture scenarios (PR 11).

Technical decisions

Fairness lives inside BudgetLedger#reserve!, under the same SELECT … FOR UPDATE as the debit, as a fifth denial reason :share_exhausted. ADR 0004 rejected a second writer of github_api_budget before this PR existed, and a check outside the row lock would be advisory rather than enforcement. The share cap is computed once and handed to both denial_reason and the DEBIT_SQL guard, so the file's existing claim — "the WHERE guard repeats the check denial_reason already made" — stays literally true for the newest predicate.

The borrow is a parameter, not a query. Whether the other class has a currently eligible candidate is a fact about github_actors and github_repositories, and reaching for two more tables from inside the most contended row lock in the application would invert the locking order. Enrichment::Fairness establishes it; the ledger enforces the arithmetic. It rides on Github::Request rather than on a RequestExecutor argument because #redirected_to uses with and a retry reuses the request unchanged — so the authorization survives both, where an executor argument would deny mid-chain after the first hop was already spent.

A borrowing reservation is capped at enrichment_allowance, not guarantee + the other's unused capacity. The two authorize the identical set: actor_share_used + repository_share_used == enrichment_used is an invariant of the debit statements and the class guard binds first, so a borrower is already limited to allowance − other_share_used, which is exactly what §10's phrasing computes. ADR 0007 carries the proof, because a reader checking the code against the plan's wording will otherwise think they diverge.

A share exhaustion is a denial, not a deferral. It writes no global block (RateLimitPolicy#reserve_breach globalizes only :reserve_reached, so this needed no production change there) and it is absent from EnrichmentSchedule. Admitting it would make borrowing unreachable — the schedule would answer "not due" before the runner ever computed one — and there is no honest instant to name, since a share is relieved either by the window rolling or by the other class running dry.

The claim is a lease on next_retry_at, not a new column and not a new advisory lock. The fetch happens outside every transaction, so the claim cannot be a held lock. One column with one meaning — "may not be attempted before T" — is what lets a single predicate exclude leases, backoffs and secondary-limit deferrals from both candidate pools and the age-out sweep; a separate leased_until column would need four queries to stay in sync, and the first that forgot would either skip an entity mid-flight or hand it to a second worker. The lease duration is derived from the gate wait, both HTTP timeouts, the retries and the redirects rather than chosen, and a release restores the exact prior instant, so a deferred cycle leaves the row byte-for-byte unchanged.

TTL staleness is a derived predicate over complete rows, never a stored transition. §7 line 572 and §10 line 816 are only compatible under this reading: "never-enriched pending candidates always precede TTL-stale refreshes" requires the two pools to stay distinguishable, and collapsing stale rows into pending erases the distinction the priority rule is stated over. It also avoids a background writer that mutates rows purely because time passed — the shape PollState and BudgetLedger#log_block_cleared both refuse.

Boundedness (B8) is structural. Eligibility and aging both key on COALESCE(last_seen_at, created_at) while ordering keys strictly on last_seen_at. The asymmetry is load-bearing: created_at in the bound can only shorten a row's life and is what makes the predicate total (a stub created on a duplicate-event page has a NULL last_seen_at, and NULL > floor is NULL — such a row would be neither eligible nor ageable and would sit pending forever); created_at in the order would let a replay-created stub outrank a genuinely hot entity, and §10 pins the ordering key by name. Every due candidate is therefore either claimable or ageable, and every not-due one self-clears within 3600s.

Two calls I want flagged as deviations from the obvious:

• ACTOR_ENRICHMENT_SHARE is parsed with Rational, not Float. Verified in Ruby: (100 * 0.29).floor == 28 where the decimal says 29, because 0.29 has no exact binary representation, and the value is multiplied by an integer allowance and floored. Rational("0.50") == 0.5, so specs and logs read unchanged. The Float failure is precision rather than soundness — the sum invariant survives because the repository guarantee is a subtraction — but it silently loses an attempt off the number an operator typed.
• Reactivation is a second statement rather than a CASE folded into touch_activity!. One statement was tidier, but §11 lists "reactivated" among the INFO events, and a set-based UPDATE that also touched non-skipped rows could not report how many it actually reactivated. The separate statement's row count is that number. It matches at most one row, only on a genuinely new event, and almost always zero.

A retryable outcome never downgrades a complete record. The plan is silent here. A transient 500 on a refresh would otherwise flip a perfectly enriched entity to retryable_failure, dropping coverage for a network blip and jumping the row into the high-priority pending pool ahead of never-enriched candidates — while conveying nothing next_retry_at, last_error and the attempt count do not already carry. Terminal outcomes do overwrite complete, because they invalidate the stored document's premise.

An IP-wide condition is not the entity's fault. A primary rate limit counts no attempt and writes nothing — PollState makes exactly this call for the source, and inflating an innocent entity's backoff would, repeated, push it toward the hour-long cap. A secondary limit still writes next_retry_at, because §10 names that explicitly and ROLL_WINDOW_SQL clears the global block at the window boundary while the entity's own component survives it.

Testing performed

1232 examples, 0 failures, up from 918 on main. Rubocop clean across 176 files; Brakeman 0 warnings.

Fourteen new spec files plus extensions to seven existing ones. §12's named cases are covered: stub → complete; reuse of fresh records and TTL-driven staleness; failed, repeated, skipped, reactivated and replay-non-reactivated; terminal skip; 404 → permanent_failure with the source staying enabled; a URL-policy violation → permanent_failure with no budget debit; concurrent claim prevention; class fairness and borrowing in both directions; class-blocking isolation both ways; and a fixture-mode end-to-end running the whole poll → persist → stub → enrich chain with expect(WebMock).not_to have_requested(:any, //).

Two structural guarantees are asserted rather than commented: the runner never calls SourceLock.acquire (§8 step 1), and it opens no transaction across the request — the latter proved by BudgetLedger#assert_committable!, which raises if one is, on a path every attempt takes.

The end-to-end spec needed no new fixture authoring. Page 1 persists four push events → three actors and three repositories; four resolve 200 and two resolve 404, which is what fixtures/github/README.md documents corpus event 58000000008 for.

Docker verification

GITHUB_MODE=fixture docker compose run --rm ingest
GITHUB_MODE=fixture docker compose run --rm enrich --limit 6

Enrichment cycles:                6
Entities enriched:                4
Entities failed:                  2
Actors pending/complete/skipped:  0 / 2 / 0
Repos pending/complete/skipped:   0 / 2 / 0
Actor requests used:              3 of 20
Repository requests used:         3 of 20

   class    | enrichment_status | count      enrichment_used | actor_share_used | repository_share_used
------------+-------------------+-------   -----------------+------------------+-----------------------
 actor      | complete          |     2                   6 |                3 |                     3
 actor      | permanent_failure |     1
 repository | complete          |     2     status | enabled
 repository | permanent_failure |     1     idle   | t

Also verified by hand: a second enrich reports Nothing to enrich — no eligible candidate and spends nothing (the freshness cache); and a forced replay past GitHub's X-Poll-Interval floor absorbs 4 duplicates while leaving a skipped_budget actor exactly as it was — reviewer verification step 7, and §7 merge rule 4.

Both one-shots are now CI smoke steps, ordered so enrich runs against the window and stubs ingest just created — exercising the whole chain across two real processes offline.

Documentation updates

• docs/adr/0007-enrichment-fairness-shares-and-borrowing.md — new. Records the seven decisions above with their rejected alternatives, the borrow-cap equivalence proof, and three costs stated plainly: the borrow fact is stale by construction (bounded to one request, self-correcting, can never exceed the class cap); a caller passing borrow: true unconditionally would defeat fairness and the ledger cannot detect it; and one state where the schedule says "due" while the reservation is still refused (:window_uninitialized).
• README.md — Status rewritten; an "Enrichment, offline" verification section with the exact counts above; the four new environment variables and the split formula; the enrichment.* log events; the request-path diagram and component list extended; ADR 0007 linked; the | Enrichment flow | PR 7 | placeholder removed.
• .env.example — replaces the "arrive with PR 7" placeholder with the real variables and the reasoning, including why ENRICHMENT_COVERAGE_WINDOW_SECONDS is still absent.
• One migration, add_enrichment_refresh_indexes: a partial index on (fetched_at, next_retry_at) WHERE enrichment_status = 'complete' for both entity tables. index_*_on_enrichment_candidates excludes complete by its own predicate, so the refresh pool had no usable index and would seq-scan a table whose population is ~2,000 candidates an hour against at most 40 completions. The migration comment states the algorithm: :concurrently tradeoff rather than pretending it is not there.

Known limitations

• Enrichment coverage is partial by design, not by shortfall: ~1.8% theoretical cold coverage against the observed demand. skipped_budget is a normal documented outcome and bin/enrich prints the per-class usage so the sampling rate is visible.
• Nothing fires the schedule. effective_enrichment_time is real and enforced, but only the one-shot invokes it until PR 8's worker and recurring task.
• The age-out sweep is bounded at 1,000 rows per call, ordered most-overdue-first so progress is monotone. Under the one-shot alone the operator drives the cadence; PR 8 provides a cycle per poll.
• A lease can expire mid-flight — its duration equals the worst-case runtime by construction. That is handled explicitly by the lease-guarded write and a lease_lost result at WARN, rather than by padding the constant and hoping.
• The concurrent-claim specs run two sequential claims on one connection, which is the interleaving the guard defends against. The variant needing two committed sessions belongs with §13's PR 11 tier, and the spec says so rather than implying wider coverage.

🤖 Generated with Claude Code

[batbrainy]

Left one inline comment.

[batbrainy]

This summary only looks at pending/retryable rows via earliest_retry_at, but complete rows are the refresh pool. That means the operator output can say due now when the backlog is fully complete and fresh, or while a failed refresh is deferred with next_retry_at, even though the next legal refresh is still in the future (fetched_at + TTL or that retry instant). Can we include refresh-pool timing here before treating nil as due now?