From 61ee4602f47a7dd6d0beab27d001fab2fe6b5559 Mon Sep 17 00:00:00 2001 From: Jorijn van der Graaf Date: Sat, 31 Oct 2026 17:33:11 +0100 Subject: [PATCH] feat(pool): no machine-wide width - 128 passes per available account Jorijn: "there is no need to do a global cap anymore, the cap should be 128 per available account". `--concurrency` no longer defaults to 128: the limits are each OAuth account's 128 (and the local login's), every driver's counted, an API seat's own REVIEW_GATES_API_CONCURRENCY, and free memory. `--concurrency N` still caps the machine when given; a value below 1 is a usage error. The dump's args.concurrency is null then. The board's driver settings read `128 per account`, which every driver's `seats` shows as that driver's width, and the `seats` header says a width is that driver's cap. The Scheduler test runs 300 passes on two accounts: 256 at once, 128 each. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 1 + README.md | 2 +- implementations/Crafter.Review-Driver.cpp | 10 ++++-- implementations/Crafter.Review-Pool.cpp | 8 ++--- implementations/Crafter.Review-Run.cpp | 6 ++-- implementations/Crafter.Review-SeatPool.cpp | 4 ++- implementations/Crafter.Review-Seats.cpp | 2 +- interfaces/Crafter.Review-Pool.cppm | 4 ++- interfaces/Crafter.Review-Run.cppm | 6 +++- process/prepush.md | 18 +++++++--- tests/Runs/main.cpp | 19 +++++++++++ tests/Scheduler/main.cpp | 37 +++++++++++++++++++++ 12 files changed, 98 insertions(+), 19 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 236b34f..0724dd6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -110,6 +110,7 @@ Conventions: - **Seat placement** (SeatPool, Seats, Drivers): - An account whose 5-hour limit Anthropic has removed reports the weekly window alone. Its burst window (`Seats::BurstWindow`) is the weekly one: it is ranked and admitted by the weekly window with the passes in flight projected on it (0.04 points per pass at xhigh, 0.125 otherwise), under the 95 % cutoff. - Every OAuth account and the local login takes at most 128 passes at once, every driver's counted. + - There is no machine-wide width (2026-10-31, Jorijn: "there is no need to do a global cap anymore, the cap should be 128 per available account"). `--concurrency` caps only when given, and the dump's `args.concurrency` is then null. The board's driver settings read `128 per account`, which every driver's `seats` shows as that driver's width (Scheduler, Runs). - A 'concurrents' rejection pauses the seat for 20 s or its retry-after, not until a window's reset. - Board entries and the folded `foreign` record carry `cost7d`, and so does the dump's seat summary. - The `seats` report and the log table say `no limit` in such an account's 5-hour column. diff --git a/README.md b/README.md index 974806e..98e0546 100644 --- a/README.md +++ b/README.md @@ -27,7 +27,7 @@ alias crafter-review='flatpak run net.catcrafts.CrafterReview' What the machine provides: - **The Claude Code CLI.** It is proprietary and not bundled. crafter-review looks for it in this order: `CLAUDE_BIN` or `REVIEW_GATES_CLAUDE_BIN`, the PATH, the editor extensions' bundled binary (newest first), then `~/.claude/local` and `~/.local/bin`. It runs inside the sandbox, so every pass is measured and every pass's process tree can be killed. -- **Seats.** The lens processes run on the seats in the repository's `.env` (`CLAUDE_CODE_OAUTH_TOKEN1..6`, `ANTHROPIC_API_KEY1..6`), else this machine's own login. `crafter-review seats` probes and reports them. It never prints a token. An account whose 5-hour limit Anthropic has removed shows `no limit` in that column and is placed by its weekly window; every account takes at most 128 passes at once. +- **Seats.** The lens processes run on the seats in the repository's `.env` (`CLAUDE_CODE_OAUTH_TOKEN1..6`, `ANTHROPIC_API_KEY1..6`), else this machine's own login. `crafter-review seats` probes and reports them. It never prints a token. An account whose 5-hour limit Anthropic has removed shows `no limit` in that column and is placed by its weekly window. Every account takes at most 128 passes at once, every driver's counted, and there is no machine-wide cap beyond that and free memory (`run --concurrency N` sets one). - **Chromium, for a lens with the browser.** Run `crafter-review browser install` once. It installs the Chromium build the pinned `@playwright/mcp@0.0.83` drives into `~/.cache/ms-playwright`, where the MCP server looks. It runs Playwright's own installer, which also removes the browser builds no installed Playwright on the machine refers to. - **A local Supabase stack, for a lens with the database** (`supabase start`, or the repository's own bring-up). The `sql` helper speaks the Postgres protocol itself: no `psql` and no driver to install. Every call is one transaction, rolled back. - **gpg, if your commits are signed.** `freeze` reads each commit's signature through gnupg's keyboxd socket, which is all the sandbox is given. The agent, which signs, stays out of reach. diff --git a/implementations/Crafter.Review-Driver.cpp b/implementations/Crafter.Review-Driver.cpp index 3321046..15520bc 100644 --- a/implementations/Crafter.Review-Driver.cpp +++ b/implementations/Crafter.Review-Driver.cpp @@ -294,8 +294,9 @@ inventory unit, against a frozen commit range.)"; {"--units", true, "UNITS", "comma-separated unit ids (test only)"}, {"--warm", false, "", "test runs only: let one pass per prompt prefix go first, as a full run does"}, {"--concurrency", true, "CONCURRENCY", - "lens processes in flight at once across all seats (default 128, 2026-10-15; the pool still places each one by seat headroom, launches a pass only when it fits in free " - "memory at its environment's measured cost, and gives a pass with the browser a stack of its own)", + "a cap on the lens processes in flight on this machine, every driver's counted (default none: each OAuth account takes at most 128 - " + "REVIEW_GATES_OAUTH_CONCURRENCY - and an API seat REVIEW_GATES_API_CONCURRENCY; the pool still places each pass by seat headroom, launches it only when it " + "fits in free memory at its environment's measured cost, and gives a pass with the browser a stack of its own)", Kind::Int}, {"--effort", true, "EFFORT", effort}, {"--reads", true, "READS", reads, Kind::Int}, @@ -355,7 +356,10 @@ inventory unit, against a frozen commit range.)"; if (namesNothing(a.only)) return UsageError(UsageLine(Prog, specs), Prog, std::format("argument --only: names no lens file ({})", Dyn::Repr(*a.only))); if (namesNothing(parsed.Value("units"))) return UsageError(UsageLine(Prog, specs), Prog, std::format("argument --units: names no unit id ({})", Dyn::Repr(*parsed.Value("units")))); a.warm = parsed.Flag("warm"); - if (const std::optional n = parsed.Value("concurrency")) a.concurrency = *Dyn::ParseInt(*n); + if (const std::optional n = parsed.Value("concurrency")) { + a.concurrency = *Dyn::ParseInt(*n); + if (*a.concurrency < 1) return UsageError(UsageLine(Prog, specs), Prog, "argument --concurrency: a number from 1 up (leave it out for no cap)"); + } a.effort = parsed.Value("effort").value_or(""); if (a.effort.empty()) a.effort = std::string(Plan::DefaultEffort()); a.noReuse = parsed.Flag("no-reuse"); diff --git a/implementations/Crafter.Review-Pool.cpp b/implementations/Crafter.Review-Pool.cpp index 2140cf7..0959456 100644 --- a/implementations/Crafter.Review-Pool.cpp +++ b/implementations/Crafter.Review-Pool.cpp @@ -784,7 +784,7 @@ namespace Crafter::Review::Pool { Sleep(std::min(60.0, Number(pauseUntil) - now)); continue; } - while (!queue.empty() && static_cast(running.size()) < o.concurrency && !paused) { + while (!queue.empty() && (!o.concurrency || static_cast(running.size()) < *o.concurrency) && !paused) { std::vector live; for (Slot& slot : running) live.push_back(&slot.job); // Without a board every queued pass waiting for a stack or @@ -820,8 +820,8 @@ namespace Crafter::Review::Pool { if (o.board) { // The board folded, the seats ranked, the admission // checked against every driver's passes and the pick - // registered in one locked section; the pool width is - // the machine's cap. + // registered in one locked section; a pool width, when + // given, is the machine's cap. const SeatPool::Board::Placed placed = o.board->Place(*o.seats, o.now(), o.seatConfig, o.apiConfig, [&](const std::vector& c, const SeatPool::Others& others) { return choose(c, &others); }, o.concurrency); cands = placed.ranked; @@ -833,7 +833,7 @@ namespace Crafter::Review::Pool { if (full) { if (roomWait.is_null()) roomWait = pauseUntil; RaisePause(o.now() + o.noRoomWait); - Log(std::format("pool: {} passes in flight on this machine, the cap is {} - re-checked in {:g} s, {} queued", full, o.concurrency, o.noRoomWait, queue.size())); + Log(std::format("pool: {} passes in flight on this machine, the cap is {} - re-checked in {:g} s, {} queued", full, o.concurrency.value_or(0), o.noRoomWait, queue.size())); paused = true; break; } diff --git a/implementations/Crafter.Review-Run.cpp b/implementations/Crafter.Review-Run.cpp index 2bc286e..d7ab541 100644 --- a/implementations/Crafter.Review-Run.cpp +++ b/implementations/Crafter.Review-Run.cpp @@ -184,7 +184,7 @@ namespace Crafter::Review::Run { {"without", opt(a.without)}, {"units", units}, {"warm", a.warm}, - {"concurrency", a.concurrency}, + {"concurrency", a.concurrency ? Json(*a.concurrency) : Json()}, {"effort", a.effort}, {"reads", a.reads}, {"no_reuse", a.noReuse}, @@ -655,7 +655,9 @@ namespace Crafter::Review::Run { {"dump", Basename(dumpPath)}, {"scratch", scratch.string()}, {"effort", a.effort}, - {"settings", {{"maxUtil", pc.at("maxUtil")}, {"apiConcurrency", apiCfg.at("concurrency")}, {"lensCostPct", pc.at("lensCostPct")}, {"concurrency", a.concurrency}}}}, + {"settings", {{"maxUtil", pc.at("maxUtil")}, {"apiConcurrency", apiCfg.at("concurrency")}, {"lensCostPct", pc.at("lensCostPct")}, + // Shown as the driver's width by every driver's `seats`. + {"concurrency", a.concurrency ? Json(*a.concurrency) : Json(std::format("{} per account", Dyn::Str(pc.at("oauthConcurrency"))))}}}}, Platform::Now()); } const bool openrouter = std::ranges::any_of(seats, [](const Json& s) { return s.at("kind") == "openrouter"; }); diff --git a/implementations/Crafter.Review-SeatPool.cpp b/implementations/Crafter.Review-SeatPool.cpp index 3a1ac89..8bc9803 100644 --- a/implementations/Crafter.Review-SeatPool.cpp +++ b/implementations/Crafter.Review-SeatPool.cpp @@ -1466,7 +1466,9 @@ namespace Crafter::Review::SeatPool { for (auto it = all.begin(); it != all.end(); ++it) { if (it.key() != me) drivers[it.key()] = it.value(); } - const std::string width = cap ? std::format("; {} in flight on this machine, the cap is {}", TotalInflight(data), *cap) : std::string(); + const std::int64_t perAccount = Dyn::Has(config, "oauthConcurrency") ? Int(AsDouble(config.at("oauthConcurrency"))) : OauthConcurrency; + const std::string width = cap ? std::format("; {} in flight on this machine, the cap is {}", TotalInflight(data), *cap) + : std::format("; {} in flight on this machine, at most {} per account", TotalInflight(data), perAccount); if (drivers.empty()) return {std::format("board: {}; no other driver running{}", path.string(), width)}; std::vector each; for (auto it = drivers.begin(); it != drivers.end(); ++it) { diff --git a/implementations/Crafter.Review-Seats.cpp b/implementations/Crafter.Review-Seats.cpp index 6a81336..86a176a 100644 --- a/implementations/Crafter.Review-Seats.cpp +++ b/implementations/Crafter.Review-Seats.cpp @@ -1009,6 +1009,6 @@ namespace Crafter::Review::Seats { FormatWhen(NumberOr0(Dyn::Get(d, "since")), now, zone), Dyn::Str(Dyn::Has(d, "inflight") ? d.at("inflight") : zero), held)); total += static_cast(NumberOr0(Dyn::Get(d, "inflight"))); } - return std::format("drivers ({} in flight on this machine; the width is the machine's cap): {}", total, Dyn::Join(parts, "; ")); + return std::format("drivers ({} in flight on this machine; a width is that driver's cap): {}", total, Dyn::Join(parts, "; ")); } } diff --git a/interfaces/Crafter.Review-Pool.cppm b/interfaces/Crafter.Review-Pool.cppm index 405dc1a..b025bc6 100644 --- a/interfaces/Crafter.Review-Pool.cppm +++ b/interfaces/Crafter.Review-Pool.cppm @@ -193,7 +193,9 @@ export namespace Crafter::Review::Pool { struct SchedulerOptions { Runner* runner = nullptr; - std::int64_t concurrency = 8; + // Passes in flight at once on this machine (with a board, every + // driver's counted); none: only each seat's own cap. + std::optional concurrency; Log log; std::optional progressPath; double pauseAt = 85; diff --git a/interfaces/Crafter.Review-Run.cppm b/interfaces/Crafter.Review-Run.cppm index bb850b4..24f10a7 100644 --- a/interfaces/Crafter.Review-Run.cppm +++ b/interfaces/Crafter.Review-Run.cppm @@ -46,7 +46,11 @@ export namespace Crafter::Review::Run { // --units, split on commas. std::optional> units; bool warm = false; - std::int64_t concurrency = 128; + // A cap on the passes in flight on this machine, every driver's + // counted; none by default (2026-10-31, Jorijn: "there is no need to + // do a global cap anymore, the cap should be 128 per available + // account"): each account's own cap is the limit. + std::optional concurrency; std::string effort; std::int64_t reads = Plan::Reads; bool noReuse = false; diff --git a/process/prepush.md b/process/prepush.md index 9c9e228..767105f 100644 --- a/process/prepush.md +++ b/process/prepush.md @@ -103,8 +103,9 @@ side (`slots.*`). Each instrument is shared in its own way: at what they were admitted at, and the budget's free memory is the reading from before they ran (`Board::MemoryBase`) — "nothing in flight" means nothing on the machine; their passes count in each seat's - projection and in an API key's slots; and `--concurrency` is the - machine's width, every driver's passes counted. A driver waiting on + projection, in an account's 128 and in an API key's slots; and + `--concurrency`, when given, is the machine's width, every driver's + passes counted. A driver waiting on what another holds re-checks every 30 s. A driver that died is swept by its lifetime lock under the board's `drivers/`, a pass by its deadline. A driver takes its passes with it however it ends: each pass runs under a @@ -241,8 +242,13 @@ therefore the hash: re-freeze with `--replace` into the same dir | 4 | the owner | approves this exact hash, then the push happens | Every lens runs on Opus 5.5 at effort `xhigh` (`--effort` overrides). -`--concurrency` is 128 by default (2026-10-15), the width of the machine: -two drivers running at once share it (2026-10-31, the board). One full run +There is no width of the machine by default (2026-10-31, Jorijn: "there is +no need to do a global cap anymore, the cap should be 128 per available +account"): every OAuth account takes at most 128 passes at once, every +driver's counted (`REVIEW_GATES_OAUTH_CONCURRENCY`), an API seat its +`REVIEW_GATES_API_CONCURRENCY`, and free memory the rest; `--concurrency N` +still caps the machine when given (it was 128 by default from 2026-10-15, +the width two drivers running at once shared). One full run per hash: a second on the same hash is refused while the first is live (`State::RunLock`; a test run takes a run number of its own), and no two drivers share a scratch dir. `flatpak run net.catcrafts.CrafterReview seats` shows the @@ -583,4 +589,6 @@ repository. account takes at most 128 passes at once (every run measured with 149 or more in flight on one account drew usage-limit rejections); a 'concurrents' rejection backs the seat off for seconds, not until a window's reset; `seats` shows `no limit` in such an account's - 5-hour column. + 5-hour column. Then (Jorijn: "there is no need to do a global cap anymore, + the cap should be 128 per available account") the machine-wide width + went: `--concurrency` caps only when given. diff --git a/tests/Runs/main.cpp b/tests/Runs/main.cpp index 82333da..f8577a5 100644 --- a/tests/Runs/main.cpp +++ b/tests/Runs/main.cpp @@ -341,11 +341,21 @@ echo "{\"type\":\"result\",\"subtype\":\"success\",\"is_error\":false,\"result\" // Stopped by `crafter-review stop`'s file, and by SIGTERM, while the // first gate's whole pass is under way: the pass is killed, no later // gate starts, nothing is left running. + std::vector widths; for (const bool bySignal : {false, true}) { before = calls(); const std::string name = bySignal ? "term" : "file"; const Process::Child child = start({"--no-reuse", "--no-salvage"}, {{"FAKE_SLEEP", "573"}}, name); const bool running = WaitFor([&] { return calls() > before; }, std::chrono::seconds(30)); + // While it runs, the board shows its width: per account, there + // being no machine-wide cap. + std::string width; + try { + const Json drivers = Dyn::LoadJsonFile(base / "board.json").at("drivers"); + for (auto it = drivers.begin(); it != drivers.end(); ++it) width = Text(Dyn::Get(Dyn::Get(it.value(), "settings"), "concurrency")); + } catch (const std::exception&) { + } + widths.push_back(width); std::string scratch; for (const std::string& line : Lines(base / std::format("{}.out", name))) { if (const std::size_t at = line.rfind("; scratch "); at != std::string::npos) scratch = line.substr(at + 10); @@ -368,6 +378,15 @@ echo "{\"type\":\"result\",\"subtype\":\"success\",\"is_error\":false,\"result\" dump.is_object() ? Text(dump.at("stoppedAt")) : Read(base / std::format("{}.err", name)))); } + // No machine-wide width by default (2026-10-31): the dump's args record + // none, a live run's board entry shows the per-account one, and a + // width below 1 is refused. + const Process::Captured noWidth = run({"--concurrency", "0"}); + Check("no global cap by default: the dump records none, a live run shows `128 per account` on the board; --concurrency below 1 is a usage error", + dumpB.is_object() && dumpB.at("args").at("concurrency").is_null() && widths == std::vector{"128 per account", "128 per account"} && noWidth.returnCode == 3 + && noWidth.err.contains("argument --concurrency: a number from 1 up"), + std::format("{} | {}", Dyn::Join(widths, ", "), noWidth.err)); + // An env file that is not UTF-8: the preflight stops the run. Write(base / "bad.env", std::string("\xff\n", 2)); const Process::Captured badEnv = run({"--no-reuse", "--no-salvage", "--env-file", (base / "bad.env").string()}); diff --git a/tests/Scheduler/main.cpp b/tests/Scheduler/main.cpp index d7f2fcc..6d4de13 100644 --- a/tests/Scheduler/main.cpp +++ b/tests/Scheduler/main.cpp @@ -145,6 +145,43 @@ namespace { Check("a no-room wait ends on the next completion, not at the window reset", r.DoneList().size() == 3 && Seconds(t0) < 5 && AnyContains(waited, "no seat has room") && AnyContains(waited, "seat room is re-checked") && r.PauseUntil() == 0, Dyn::Join(waited, " | ")); + // 2026-10-31: no machine-wide width by default - each account takes at + // most 128 passes at once, every driver's counted. + { + std::vector two; + for (const std::string_view id : {std::string_view("1"), std::string_view("2")}) { + Json s = SeatPool::NewSeat(std::string(id), "oauth", std::format("t{}", id)); + s["state"] = "ok"; + s["windows"] = {{"five_hour", {{"utilization", 10}, {"resetsAt", now + 3600}}}, {"seven_day", {{"utilization", 10}, {"resetsAt", now + 86400}}}}; + two.push_back(std::move(s)); + } + std::mutex lock; + std::map inSeat; + std::map peakSeat; + std::int64_t inFlight = 0; + std::int64_t peak = 0; + Pool::Scheduler wide({.log = [](const std::string&) {}, .seats = &two, .seatConfig = SeatPool::PoolConfig({}), .apiConfig = SeatPool::ApiConfig({})}); + std::vector many; + for (std::int32_t i = 0; i < 300; ++i) many.push_back({{"label", std::format("w{}", i)}}); + wide.Run(many, [&](const Json& j, std::int64_t) { + const std::string seat = Dyn::Str(j.at("seat")); + { + const std::lock_guard guard(lock); + peakSeat[seat] = std::max(peakSeat[seat], ++inSeat[seat]); + peak = std::max(peak, ++inFlight); + } + std::this_thread::sleep_for(300ms); + { + const std::lock_guard guard(lock); + --inSeat[seat]; + --inFlight; + } + return std::pair({{"label", j.at("label")}, {"status", "ok"}}, nullptr); + }, NoFollowUp); + Check("no machine-wide width by default: two accounts take 128 passes each, 256 at once, and all 300 finish", + wide.DoneList().size() == 300 && peak == 256 && peakSeat["1"] == 128 && peakSeat["2"] == 128, std::format("peak {}, per seat {} and {}", peak, peakSeat["1"], peakSeat["2"])); + } + std::map started; std::map answered; logs.Clear();