From 05816be3c259b1f820298dbe8a9bb20014914629 Mon Sep 17 00:00:00 2001 From: Rodrigo Gomes da Silva Date: Fri, 14 Aug 2026 20:44:56 -0300 Subject: [PATCH] fix(sessions): refill the unassigned window when the mixed pass under-delivers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Recovery pass 2 was gated on `first_pass_count >= unassigned_target`, reading a short first pass as "the database has nothing left". It can also mean the mixed pass ran out of RAW candidate window: compression segments are spent from it before the logical slice, so a lineage-heavy profile gets fewer conversations than it asked for and the unassigned quota stays short. Also run the refill when assigned conversations inside the window displaced unassigned candidates, which is the case a narrower unassigned-only query can actually repair. A profile with no assignments and a small state.db still issues no extra query. While here, spend the per-project assigned budget through `_merge_state_row`'s return value instead of pre-incrementing, so the budget counts logical conversations the way PROJECT_ASSIGNED_CLI_LIMIT documents, and seed it from the assigned rows already inside the window so the bound stays a total rather than one bound stacked on another. That accounting is behaviour-neutral today (the assigned query is newest-first, so window rows are always the first rows it returns) — it makes the documented contract hold by construction rather than by that ordering accident, and removes an unused return value. --- api/models.py | 55 ++++++++++++++----- ...test_project_assigned_cli_session_limit.py | 37 +++++++++++++ 2 files changed, 78 insertions(+), 14 deletions(-) diff --git a/api/models.py b/api/models.py index 54e86535..ea641160 100644 --- a/api/models.py +++ b/api/models.py @@ -8481,11 +8481,13 @@ def _load_cli_sessions_uncached( def _merge_state_row(row: dict) -> bool: """Add ``row`` unless its lineage is already represented. - Returns True only when a NEW logical conversation was added, so the - callers below budget conversations rather than rows. An already - represented lineage is upgraded in place: the recovery pass sees the - same conversation carrying its resolved project assignment, and the - projection loop must read that richer row. + Returns True only when a NEW logical conversation was added, which is + what lets recovery pass 1 spend its per-project budget on + conversations rather than rows. An already represented lineage is + upgraded in place: the recovery pass sees the same conversation + carrying its resolved project assignment, and the projection loop + must read that richer row. Pass 2 needs no budget of its own (its + query limit is the bound) and ignores the result. """ lineage_ids = _lineage_ids(row) existing = next( @@ -8530,7 +8532,17 @@ def _load_cli_sessions_uncached( ) ) try: + # An assigned conversation ALREADY inside the recent window + # occupies one of its project's slots, so this pass tops each + # project UP TO the bound instead of stacking a second bound on + # top of the window. kept_per_project: dict[str, int] = {} + for row in state_rows: + seeded_project_id = _interactive_row_project_id(row) + if seeded_project_id is not None: + kept_per_project[seeded_project_id] = ( + kept_per_project.get(seeded_project_id, 0) + 1 + ) for row in read_importable_agent_session_rows( db_path, limit=query_limit, @@ -8544,19 +8556,28 @@ def _load_cli_sessions_uncached( # the row to the normal window so it stays reachable # under "Unassigned" (#6659 review finding 3). continue - if per_project_limit is not None: - kept = kept_per_project.get(project_id, 0) - if kept >= per_project_limit: - continue - kept_per_project[project_id] = kept + 1 - _merge_state_row(row) + if ( + per_project_limit is not None + and kept_per_project.get(project_id, 0) >= per_project_limit + ): + continue + if _merge_state_row(row): + # Spend the budget on LOGICAL conversations only: a row + # that merely upgrades an already-represented lineage + # (a compression segment, or a window row learning its + # assignment) costs its project nothing. + kept_per_project[project_id] = ( + kept_per_project.get(project_id, 0) + 1 + ) except Exception: logger.debug("Project-assigned CLI recovery pass failed", exc_info=True) # --- Recovery pass 2: top the interactive window back up to # CLI_VISIBLE_SESSION_LIMIT *unassigned* conversations. Only runs when - # the window was actually saturated and came up short, so a schema with - # no assignments (and a small state.db) pays nothing. + # the window came up short AND something other than an empty database + # can explain it: either it was saturated, or assigned conversations + # inside it displaced unassigned candidates. A profile with no + # assignments and a small state.db still pays nothing. unassigned_target = ( visible_session_limit if visible_session_limit is not None else CLI_VISIBLE_SESSION_LIMIT @@ -8566,8 +8587,14 @@ def _load_cli_sessions_uncached( ) if ( unassigned_target - and first_pass_count >= unassigned_target and unassigned_seen < unassigned_target + and ( + first_pass_count >= unassigned_target + # A short MIXED first pass can still be under-delivering: the + # narrower unassigned-only query reaches conversations the mixed + # candidate window spent on assigned rows and segments. + or unassigned_seen < first_pass_count + ) ): try: for row in read_importable_agent_session_rows( diff --git a/tests/test_project_assigned_cli_session_limit.py b/tests/test_project_assigned_cli_session_limit.py index f805655c..972d9c49 100755 --- a/tests/test_project_assigned_cli_session_limit.py +++ b/tests/test_project_assigned_cli_session_limit.py @@ -716,6 +716,43 @@ def test_schema_without_project_id_retains_prior_behavior(fake_hermes_home, tmp_ ] +def test_unassigned_refill_runs_when_the_mixed_window_under_delivers( + fake_hermes_home, tmp_path, monkeypatch +): + """A SHORT mixed first pass is still a shortfall the refill must repair. + + The first pass can return fewer than ``CLI_VISIBLE_SESSION_LIMIT`` + conversations with plenty left in the db, because compression segments spend + its RAW candidate window (8x, then 32x) before the logical slice. Gating the + refill on "the first pass came back full" left the unassigned window short in + exactly that case. Three 43-segment assigned chains saturate both candidate + windows and yield 3 of the 4 requested conversations; the narrower + unassigned-only query skips those chains and can still deliver 4. + """ + monkeypatch.setattr(models, "CLI_VISIBLE_SESSION_LIMIT", 4) + _register_projects(tmp_path, "project-live") + + rows = [_session(f"plain-{index:02d}", BASE_TS + 100 + index) for index in range(10)] + for index in range(3): + rows.extend(_lineage( + f"deep{index}", + BASE_TS + 2000 + index * 1000, + 43, + project_id="project-live", + step=1.0, + )) + _write_state_db(fake_hermes_home / "state.db", rows) + + sessions = models.get_cli_sessions() + unassigned = sorted(s["session_id"] for s in sessions if s["project_id"] is None) + + # Was 0: the mixed pass returned 3 conversations, so the old "was it + # saturated?" gate concluded the db had nothing left to give. + assert unassigned == ["plain-06", "plain-07", "plain-08", "plain-09"] + # The lineage-heavy assigned conversations are still there, once each. + assert sum(1 for s in sessions if s["project_id"] == "project-live") == 3 + + # ── Rebase guard: upstream's kanban pass shares the system-chip helper ────────