Cloonar/coding-lab

Fork 0

tracker: full-history PR walks (reaper done-signal, labctl pr list) grow unboundedly and hammer the forge #176

New issue

Closed

opened 2026-07-18 21:38:44 +02:00 by dominik.polakovics · 1 comment

dominik.polakovics commented

2026-07-18 21:38:44 +02:00

Owner

Symptom

Everything that touches the forge gets slower as the repo accumulates PRs, forever. Measured live: labctl pr list on this repo (~174 PRs) takes 6.7 seconds. The forge (separate host, git.cloonar.com) absorbs a continuous stream of its most expensive query, which also slows issue pages the UI proxies, git fetches, and land-pr flows — invisible in the lab server's own htop.

Mechanism (code-confirmed, measured)

forgejo.Client.Pulls (internal/tracker/forgejo/forgejo.go:246) lists with state=all and paginates until an empty page, 50 per page, sequentially — i.e. every PR ever created. Forgejo's pulls endpoint costs ~1.3s/page here, so the walk is ~1.3s x (N/50 + 1): linear growth, +1.3s per 50 PRs merged. At the current pace (~170 PRs in 10 days) it doubles roughly every two weeks; the empty-page termination adds one guaranteed wasted request per walk.

Call sites that run the walk:

AFK reaper (internal/afk/reaper.go:92): every tick (default 30s) per repo with active AFK runs, to answer the done-signal question "does an open-or-merged PR exist with head = this run's branch". 854 walks in 17h at diagnosis time. As the walk grows past the tick interval, done-signal detection lags and ticks coalesce.
labctl pr list (agent surface): used by agents and /land-pr, so agent sessions stall on it too.

Fix direction

Done-signal: replace the full listing with a per-branch query — Forgejo/Gitea supports GET /repos/{owner}/{repo}/pulls/{base}/{head} (and head filters on the list endpoint). The reaper needs one bounded request per run, not the repo's PR history. Preserve the exact done-signal semantics: open or merged counts, closed-unmerged does not (tracker.DonePull).
labctl pr list: bound the walk — open PRs plus a recent-merged window (e.g. sort by recency, stop after a page horizon), or make the state filter explicit and default it to open.
Check the sibling Issues(state) walk (state=all/closed views) for the same shape while in there; the open-issue list is currently one page and fine.

Constraints: the tracker seam is provider-neutral (forgejo/github/builtin all implement it) — the per-branch query needs a seam method (e.g. PullForHead(branch)) with a conformance test, not a forgejo special case. Watch the GitHub client for head-filter syntax differences (owner:branch).

Verification

time labctl pr list before/after (6.7s -> sub-second). Reaper: tracker metrics lab_tracker_requests_total{op="pulls"} should stop growing per tick; done-signal latency for a fresh AFK PR stays within one tick.

## Symptom Everything that touches the forge gets slower as the repo accumulates PRs, forever. Measured live: `labctl pr list` on this repo (~174 PRs) takes **6.7 seconds**. The forge (separate host, git.cloonar.com) absorbs a continuous stream of its most expensive query, which also slows issue pages the UI proxies, git fetches, and land-pr flows — invisible in the lab server's own htop. ## Mechanism (code-confirmed, measured) `forgejo.Client.Pulls` (`internal/tracker/forgejo/forgejo.go:246`) lists with `state=all` and paginates until an empty page, 50 per page, **sequentially** — i.e. every PR ever created. Forgejo's pulls endpoint costs ~1.3s/page here, so the walk is ~1.3s x (N/50 + 1): linear growth, +1.3s per 50 PRs merged. At the current pace (~170 PRs in 10 days) it doubles roughly every two weeks; the empty-page termination adds one guaranteed wasted request per walk. Call sites that run the walk: - **AFK reaper** (`internal/afk/reaper.go:92`): every tick (default 30s) per repo with active AFK runs, to answer the done-signal question "does an open-or-merged PR exist with head = this run's branch". 854 walks in 17h at diagnosis time. As the walk grows past the tick interval, done-signal detection lags and ticks coalesce. - **`labctl pr list`** (agent surface): used by agents and /land-pr, so agent sessions stall on it too. ## Fix direction - **Done-signal**: replace the full listing with a per-branch query — Forgejo/Gitea supports `GET /repos/{owner}/{repo}/pulls/{base}/{head}` (and head filters on the list endpoint). The reaper needs one bounded request per run, not the repo's PR history. Preserve the exact done-signal semantics: open or merged counts, closed-unmerged does not (`tracker.DonePull`). - **`labctl pr list`**: bound the walk — open PRs plus a recent-merged window (e.g. sort by recency, stop after a page horizon), or make the state filter explicit and default it to open. - Check the sibling `Issues(state)` walk (`state=all`/`closed` views) for the same shape while in there; the open-issue list is currently one page and fine. Constraints: the tracker seam is provider-neutral (forgejo/github/builtin all implement it) — the per-branch query needs a seam method (e.g. `PullForHead(branch)`) with a conformance test, not a forgejo special case. Watch the GitHub client for head-filter syntax differences (`owner:branch`). ## Verification `time labctl pr list` before/after (6.7s -> sub-second). Reaper: tracker metrics `lab_tracker_requests_total{op="pulls"}` should stop growing per tick; done-signal latency for a fresh AFK PR stays within one tick.

dominik.polakovics added the

needs-triage

bug

labels

2026-07-18 21:38:44 +02:00

dominik.polakovics referenced this issue

2026-07-18 21:39:07 +02:00

chat: /clear round-trip is 3-5s since the #149 clear-hook — pull + double transcript parse block the forward #177

dominik.polakovics referenced this issue

2026-07-18 21:46:57 +02:00

web: SSE-driven refetch/render churn makes sends and chat switching sluggish as the fleet grows #175

dominik.polakovics referenced this issue

2026-07-18 22:23:16 +02:00

perf(chat): tail-only SSE refetch with content-hash identity to stop render churn #178

dominik.polakovics commented

2026-07-19 00:01:15 +02:00

Author

Owner

This was generated by AI during triage.

Agent Brief

Category: bug
Summary: Replace unbounded full-history PR walks (reaper done-signal, labctl pr list) with bounded per-branch / windowed queries behind the tracker seam.

Current behavior:
The Tracker seam's Pulls() lists every PR the repo has ever had (state=all, sequential 50-per-page pagination until an empty page — so one guaranteed wasted request per walk). Two hot paths run it:

The AFK reaper runs one full walk per repo with active runs on every tick (default 30s), only to answer, per run, "does an open-or-merged PR exist with head = this run's branch" (tracker.DonePull, matched client-side).
labctl pr list (the agent surface behind GET /agent/v1/prs) runs the same walk, so agent sessions and /land-pr flows stall on it.

Cost is linear in repo history and grows forever: at triage time the repo has 80 PRs → 3 sequential requests → ~1.7s on an unloaded forge, measured at 6.7s during the perf incident when the reaper's own request stream had loaded the forge (the walk makes the forge slow, which makes the walk slow). Growth is one extra page per 50 merged PRs, and this repo merges ~50/week. The Forgejo and GitHub clients share the identical shape; the builtin tracker is a cheap store query and is not a problem.

Desired behavior:

Done-signal: the reaper answers the per-run question with a bounded number of requests per run (ideally one), independent of repo history — a per-head-branch query, not a full listing. Forgejo/Gitea supports GET /repos/{owner}/{repo}/pulls/{base}/{head} and head filters on the list endpoint; GitHub's list endpoint takes head=owner:branch (note the owner prefix — syntax differs from Forgejo).
Done-signal semantics preserved exactly: a head-matching PR that is open or merged counts; closed-unmerged does not (today's tracker.DonePull contract, including its collision preference when several PRs share a head).
labctl pr list: the request count is bounded and independent of total history. Default view: all open PRs plus a recent window of merged/closed ones (recency-sorted with a fixed page horizon is fine). The walk-to-empty-page pattern disappears from this path.

Key interfaces:

Tracker interface — add a seam method (e.g. PullForHead(ctx, headBranch) returning a PullRef or its absence) implemented by all three backends (forgejo, github, builtin), not a forgejo special case. Update the Pulls() doc contract if its "state=all is required for the done-signal" rationale moves to the new method.
tracker.DonePull / PRPresent — the pure projection functions; keep their semantics, adapt their input if the reaper no longer holds a full listing.
The tracker instrumentation op vocabulary — the new method needs its own op label so lab_tracker_requests_total tracks it; the reaper's per-tick op="pulls" count should stop growing with history (or disappear in favor of the new op).
The forge clients' shared pagination helper — the bounded pr list variant needs a page-horizon mode rather than walk-until-empty.

Branch-deletion caveat (pin in a test): merged PRs whose head branch was since deleted come back from Forgejo with a synthetic head ref (refs/pull/N/head), not the branch name. The done-signal is safe because the reaper only queries branches of active runs (the branch still exists until after the run is reaped), but the per-head query must be specced/tested against live branches only — don't assume a merged-and-deleted branch remains findable by name.

Acceptance criteria:

Reaper tick issues O(active runs) tracker requests, not O(repo history); done-signal for a fresh AFK PR still lands within one tick.
Done-signal semantics conformance-tested across forgejo, github, and builtin backends: open counts, merged counts, closed-unmerged does not.
time labctl pr list on this repo drops from multi-second to sub-second, and its forge request count no longer grows with merged-PR history.
pr list still shows all open PRs; recently merged PRs appear in the default view (bounded window documented in the command output or docs).
New seam method instrumented (own op in tracker metrics).
The sibling Issues(state) walk is checked for the same unbounded shape; fixed the same way if state=all/closed views walk full history, otherwise explicitly noted as fine.

Out of scope:

Delete-source-branch-on-merge policy (#130) and per-run base branch (#131).
GET /instances commits_behind perf (#152).
Forge-side caching or ETags; any Forgejo server configuration.
Changing DonePull's collision/state semantics.

> *This was generated by AI during triage.* ## Agent Brief **Category:** bug **Summary:** Replace unbounded full-history PR walks (reaper done-signal, `labctl pr list`) with bounded per-branch / windowed queries behind the tracker seam. **Current behavior:** The `Tracker` seam's `Pulls()` lists every PR the repo has ever had (`state=all`, sequential 50-per-page pagination until an empty page — so one guaranteed wasted request per walk). Two hot paths run it: - The AFK reaper runs one full walk per repo with active runs on every tick (default 30s), only to answer, per run, "does an open-or-merged PR exist with head = this run's branch" (`tracker.DonePull`, matched client-side). - `labctl pr list` (the agent surface behind `GET /agent/v1/prs`) runs the same walk, so agent sessions and /land-pr flows stall on it. Cost is linear in repo history and grows forever: at triage time the repo has 80 PRs → 3 sequential requests → ~1.7s on an unloaded forge, measured at 6.7s during the perf incident when the reaper's own request stream had loaded the forge (the walk makes the forge slow, which makes the walk slow). Growth is one extra page per 50 merged PRs, and this repo merges ~50/week. The Forgejo and GitHub clients share the identical shape; the builtin tracker is a cheap store query and is not a problem. **Desired behavior:** - **Done-signal:** the reaper answers the per-run question with a bounded number of requests per run (ideally one), independent of repo history — a per-head-branch query, not a full listing. Forgejo/Gitea supports `GET /repos/{owner}/{repo}/pulls/{base}/{head}` and head filters on the list endpoint; GitHub's list endpoint takes `head=owner:branch` (note the owner prefix — syntax differs from Forgejo). - **Done-signal semantics preserved exactly:** a head-matching PR that is open or merged counts; closed-unmerged does not (today's `tracker.DonePull` contract, including its collision preference when several PRs share a head). - **`labctl pr list`:** the request count is bounded and independent of total history. Default view: all open PRs plus a recent window of merged/closed ones (recency-sorted with a fixed page horizon is fine). The walk-to-empty-page pattern disappears from this path. **Key interfaces:** - `Tracker` interface — add a seam method (e.g. `PullForHead(ctx, headBranch)` returning a `PullRef` or its absence) implemented by **all three** backends (forgejo, github, builtin), not a forgejo special case. Update the `Pulls()` doc contract if its "state=all is required for the done-signal" rationale moves to the new method. - `tracker.DonePull` / `PRPresent` — the pure projection functions; keep their semantics, adapt their input if the reaper no longer holds a full listing. - The tracker instrumentation op vocabulary — the new method needs its own op label so `lab_tracker_requests_total` tracks it; the reaper's per-tick `op="pulls"` count should stop growing with history (or disappear in favor of the new op). - The forge clients' shared pagination helper — the bounded `pr list` variant needs a page-horizon mode rather than walk-until-empty. **Branch-deletion caveat (pin in a test):** merged PRs whose head branch was since deleted come back from Forgejo with a synthetic head ref (`refs/pull/N/head`), not the branch name. The done-signal is safe because the reaper only queries branches of *active* runs (the branch still exists until after the run is reaped), but the per-head query must be specced/tested against live branches only — don't assume a merged-and-deleted branch remains findable by name. **Acceptance criteria:** - [ ] Reaper tick issues O(active runs) tracker requests, not O(repo history); done-signal for a fresh AFK PR still lands within one tick. - [ ] Done-signal semantics conformance-tested across forgejo, github, and builtin backends: open counts, merged counts, closed-unmerged does not. - [ ] `time labctl pr list` on this repo drops from multi-second to sub-second, and its forge request count no longer grows with merged-PR history. - [ ] `pr list` still shows all open PRs; recently merged PRs appear in the default view (bounded window documented in the command output or docs). - [ ] New seam method instrumented (own op in tracker metrics). - [ ] The sibling `Issues(state)` walk is checked for the same unbounded shape; fixed the same way if `state=all`/`closed` views walk full history, otherwise explicitly noted as fine. **Out of scope:** - Delete-source-branch-on-merge policy (#130) and per-run base branch (#131). - `GET /instances` `commits_behind` perf (#152). - Forge-side caching or ETags; any Forgejo server configuration. - Changing `DonePull`'s collision/state semantics.

dominik.polakovics added

ready-for-agent

and removed

needs-triage

labels

2026-07-19 00:01:19 +02:00

dominik.polakovics referenced this issue from a commit

2026-07-19 00:47:06 +02:00

perf(tracker): bounded per-branch done-signal and windowed listings

dominik.polakovics referenced this issue from a pull request that will close it,

2026-07-19 00:47:27 +02:00

perf(tracker): tail the PR-history walks — per-branch done-signal, windowed listings #179