From 634f66774be8fbcdfe4bed06ade234cf91cefd75 Mon Sep 17 00:00:00 2001 From: rodrigogs Date: Tue, 22 Sep 2026 14:23:51 -0300 Subject: [PATCH] fix(sessions): ration the compression-heavy widening to one pass budget MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Greptile P1 on #6659: the retry that follows a binding scoped raw window jumped straight to the global `query_limit`, and every starved project got it in turn. `remaining` bounds a project's FIRST scoped query, but the widening was not bounded by the pass at all, so a build scanned len(starved) * query_limit — an order of magnitude past PROJECT_ASSIGNED_CLI_SCAN_CEILING on a profile with ten compression-heavy projects, and growing with the number of them. The widening now comes out of a single pass-level pool of `query_limit`, charged the full width of each retry, so the scoped reading of a build is capped at 2 * query_limit however many projects are starved. When the pool cannot widen a retry past the window that just bound it, the loop stops instead of paying for a query that cannot deliver. Completeness is deliberately untouched: only the WIDENING is rationed. Every starved project still pays its own first scoped query, which is the guarantee the fixed per-build project cap used to break (greptile P1 on #6659). The constants comment above PROJECT_ASSIGNED_CLI_SCAN_CEILING carried the same overclaim ("one small project-scoped query per starved project") and now states the widening budget. Verified: 88 tests pass across tests/test_project_assigned_cli_session_limit.py (75), tests/test_issue2628_cli_sessions_perf.py and tests/test_issue4842_cron_projection_perf.py. The new regression test `test_compression_heavy_widening_is_one_pass_budget_not_per_project` is parametrized over 1 and 3 compression-heavy starved projects and fails on the old one-liner by showing three full-width retries (16, 16, 16) where the pool funds one (16). --- api/models.py | 39 ++++++-- ...test_project_assigned_cli_session_limit.py | 93 +++++++++++++++++++ 2 files changed, 126 insertions(+), 6 deletions(-) diff --git a/api/models.py b/api/models.py index b8689a70..84f1a06b 100644 --- a/api/models.py +++ b/api/models.py @@ -79,7 +79,11 @@ PROJECT_ASSIGNED_CLI_SCAN_CEILING = 2000 # one project's remaining budget; and the sum of those budgets across every # starved project is at most effective_limit * len(projects), which the scan # ceiling above already caps. Worst case per build: 1 global query + 1 GROUP BY -# probe + one small project-scoped query per starved project. +# probe + one small project-scoped query per starved project, plus the pass's +# single widening budget (query_limit, see `widening_budget`) shared by the +# compression-heavy retries — a retry is NOT granted the global allowance per +# project, which would make a build scale with the number of starved projects +# (greptile P1 on #6659). # # --- Bounds for the UNASSIGNED refill pass (see _load_cli_sessions_uncached). # The SQL 'unassigned' filter reads state.db.project_id, but a WebUI-side move @@ -8754,6 +8758,21 @@ def _load_cli_sessions_uncached( # cost stays bounded because each query is limited to one # project's remaining budget (sum <= the scan ceiling) and # only fires when the global window was saturated. + # + # The WIDENING below is rationed per PASS instead (greptile + # P1 on #6659): `remaining` bounds each project's FIRST + # query, but a retry that jumps to `query_limit` is not + # bounded by the project's own share at all, so granting it + # to every starved project made a build scan + # len(starved) * query_limit — an order of magnitude past + # the ceiling on a profile with ten compression-heavy + # projects. One aggregate pool, spent at the full width of + # each retry, caps the pass at 2 * query_limit of scoped + # reading regardless of how many projects are starved. + # Only the widening is rationed; the first query of every + # starved project is never skipped, which is what keeps the + # completeness guarantee above intact. + widening_budget = query_limit for _kept, project_id in starved: remaining = effective_limit - kept_per_project.get(project_id, 0) if remaining <= 0: @@ -8768,10 +8787,10 @@ def _load_cli_sessions_uncached( # the loop would advance to the next project and leave # this one under-delivered on every rebuild — the same # failure the global query already guards against - # (greptile P1 on #6659). Re-query once at the widest - # limit this pass is allowed to scan, so the retry is - # bounded to one extra query per starved project and - # cannot out-read the pass's own scan budget. + # (greptile P1 on #6659). Re-query once, widened to + # whatever the pass's widening budget has left, which is + # what bounds the retry at the PASS level instead of per + # project (greptile P1 on #6659). scoped_limit = remaining while True: scoped_rows, scoped_window_exhausted = cast( @@ -8797,7 +8816,15 @@ def _load_cli_sessions_uncached( or scoped_limit >= query_limit ): break - scoped_limit = query_limit + widened = min(query_limit, widening_budget) + if widened <= scoped_limit: + # The pass has spent its widening budget. A + # retry now could only re-read a window this + # query already consumed, so stop instead of + # paying for a read that cannot deliver. + break + widening_budget -= widened + scoped_limit = widened except Exception: logger.debug("Project-assigned CLI recovery pass failed", exc_info=True) diff --git a/tests/test_project_assigned_cli_session_limit.py b/tests/test_project_assigned_cli_session_limit.py index 683c1a18..717beac5 100755 --- a/tests/test_project_assigned_cli_session_limit.py +++ b/tests/test_project_assigned_cli_session_limit.py @@ -2385,3 +2385,96 @@ def test_all_profiles_view_resolves_each_context_against_its_own_profile( assert by_id["older-assigned-b"]["project_id"] == "project-b" # A foreign id is still not an assignment. assert by_id["newest-foreign-b"]["project_id"] is None + + +@pytest.mark.parametrize("quiet_count", [1, 3]) +def test_compression_heavy_widening_is_one_pass_budget_not_per_project( + fake_hermes_home, tmp_path, monkeypatch, quiet_count +): + """The retry after a binding scoped window is paid from ONE pass budget. + + ``remaining`` bounds each starved project's first scoped query, but the + widening that follows a binding raw window jumped straight to the global + ``query_limit`` — and it was granted to every starved project in turn. A + profile with ten compression-heavy projects therefore scanned ten times the + ceiling in a single build, growing with the number of starved projects + (greptile P1 on #6659). The widening now comes out of one pass-level pool, + so the scoped reading of a build is bounded by ``2 * query_limit`` however + many projects are starved — while every starved project still pays its own + first query, the completeness guarantee that a fixed per-build project cap + used to break (greptile P1 on #6659). + + Parametrized over the number of starved projects because that is exactly + what the budget must not scale with. + """ + ceiling = 20 + monkeypatch.setattr(models, "CLI_VISIBLE_SESSION_LIMIT", 5) + monkeypatch.setattr(models, "PROJECT_ASSIGNED_CLI_SCAN_CEILING", ceiling) + per_project_limit = 4 + quiet = [f"project-quiet-{index}" for index in range(quiet_count)] + _register_projects(tmp_path, "project-busy", *quiet) + + project_count = len(quiet) + 1 + # The pass's own query budget: every project's equal share of the ceiling. + effective_limit = min(per_project_limit, max(1, ceiling // project_count)) + query_limit = min(effective_limit * project_count, ceiling) + + # Each quiet project's scoped raw window is consumed by its OWN compression + # segments, so its first scoped query comes back short with older assigned + # conversations of its own still waiting behind the window. + scoped_raw_window = per_project_limit * max(agent_sessions.CANDIDATE_WINDOW_MULTIPLIERS) + rows = [] + for index, project_id in enumerate(quiet): + rows.append(_session(f"{project_id}-old", BASE_TS + index, project_id=project_id)) + rows.extend(_lineage( + f"{project_id}-chain", + BASE_TS + 10_000 + index * 1_000, + scoped_raw_window + 100, + project_id=project_id, + step=1.0, + )) + rows.extend( + _session(f"busy-{index:03d}", BASE_TS + 5_000 + index, project_id="project-busy") + for index in range(25) + ) + rows.extend(_session(f"plain-{index:02d}", BASE_TS + 90_000 + index) for index in range(25)) + _write_state_db(fake_hermes_home / "state.db", rows) + + scoped_queries = [] + real_reader = models.read_importable_agent_session_rows + + def _counting_reader(*args, **kwargs): + if kwargs.get("project_ids"): + scoped_queries.append((kwargs["project_ids"][0], kwargs.get("limit"))) + return real_reader(*args, **kwargs) + + monkeypatch.setattr(models, "read_importable_agent_session_rows", _counting_reader) + + sessions = models._load_cli_sessions_uncached( + fake_hermes_home, + fake_hermes_home / "state.db", + "default", + project_assigned_limit=per_project_limit, + ) + + # Completeness is untouched: EVERY starved project still gets its query. + # (The busy project is starved in this fixture too and is queried as well; + # what matters here is that no project of interest is skipped.) + tried = {project_id for project_id, _ in scoped_queries} + assert set(quiet) <= tried + + # A widened query is one that asks for more than the project's own share. + widened = [limit for _, limit in scoped_queries if limit > effective_limit] + # The pool is one query_limit wide and each retry is charged its full width, + # so exactly one project can take the widest retry — not one per project. + assert widened == [query_limit] + # And the whole pass stays inside the documented 2 * query_limit bound, + # whatever the number of starved projects. + assert sum(limit for _, limit in scoped_queries) <= 2 * query_limit + + # The widening the budget did fund still recovers the conversation behind + # the neediest project's window — the retry was rationed, not disabled. + widened_project = quiet[0] + assert f"{widened_project}-old" in { + session["session_id"] for session in sessions + }, "the one funded widening must still reach behind the binding window"