Repository navigation
checkout_next reports no eligible work while eligible work exists — assignee predicate applied after LIMIT #15965
Description
Problem
checkout_next can report "no eligible work item" while eligible work exists. On base now, merged in #15920 (5ab30ac80b) past an unresolved Major review thread.
autobot-backend/llc/services/work_item_queue.py:95-106:
eligible = (
select(LLCWorkItem)
.where(
LLCWorkItem.company_id == uuid.UUID(company_id),
LLCWorkItem.status == WorkItemStatus.READY.value,
LLCWorkItem.checkout_run_id.is_(None),
)
.order_by(*backlog_order())
.limit(CHECKOUT_CANDIDATES) # = 10
)
for item in (await session.execute(eligible)).scalars().all():
if item.assignee_agent_id is not None and str(item.assignee_agent_id) != agent_id:
continueThe assignee predicate is applied in Python, after the SQL LIMIT. The function's own docstring states the rule the query is missing:
Eligible means: this company's,
ready, unclaimed, and either unassigned or assigned to this agent.
That final clause is not in the WHERE. So the query fetches the first 10 ready, unclaimed items regardless of assignee, and the loop then discards the ones belonging to other agents.
Failure scenario
A company has 12 ready, unclaimed work items. The first 10 in backlog_order() are assigned to other agents; items 11 and 12 are unassigned.
Agent A calls checkout_next. The query returns rows 1-10, the loop discards all ten on the assignee test, the function returns None, and the route answers "No eligible work item" — while two items it is entitled to sit just past the limit.
The failure scales with contention: the more assigned work a company has queued ahead, the more reliably every agent is told there is nothing to do. At CHECKOUT_CANDIDATES = 10 it needs only ten assigned items to starve the queue completely.
Why it went unnoticed
None is a legitimate, expected return. The docstring says so explicitly:
Nonewhen nothing is eligible: an ordinary answer, not an error. An agent asking for work when there is none is the common case.
So the bug's output is indistinguishable from correct operation. There is no error, no log line, no metric — an agent that should be working simply idles, and the system reports the common case. Nothing in the response says whether the queue was empty or merely mis-queried.
Fix
Move the predicate into the query, so LIMIT applies to rows that are actually eligible:
.where(
LLCWorkItem.company_id == uuid.UUID(company_id),
LLCWorkItem.status == WorkItemStatus.READY.value,
LLCWorkItem.checkout_run_id.is_(None),
or_(
LLCWorkItem.assignee_agent_id.is_(None),
LLCWorkItem.assignee_agent_id == agent_id,
),
)The Python continue then becomes dead and should go with it — leaving it would preserve a second, now-unreachable copy of the rule.
The CheckoutConflict continue below is unrelated and must stay: that one handles a race between SELECT and fence, not eligibility.
Acceptance criteria
- The assignee predicate is in the SQL
WHERE, evaluated beforeLIMIT - The post-limit Python assignee filter is removed, not left as a redundant second copy
- The
CheckoutConflictretry loop is unchanged - Regression test: 10+ ready unclaimed items assigned to another agent, ordered ahead of one unassigned item;
checkout_nextreturns the unassigned item rather thanNone - A test pins the boundary at
CHECKOUT_CANDIDATESexactly, so raising the constant cannot silently mask the bug instead of fixing it - Docstring and query agree — whichever is authoritative, they state the same rule
Provenance
Found by review on #15920 and marked Major; the thread was never opened and the PR merged with it unresolved. Four sessions reviewed that PR and each cleared its own scope, so the gap sat exactly where every reviewer assumed another had been. Recorded because the review process failed here in the same way the code does: an aggregate of clearances has no field for what nobody examined.
Activity
mrveiss commentedon Sep 7, 2026
Scope confirmation from b9's backend sweep: this is a single site. No other service applies an eligibility predicate after a
LIMIT, so the fix does not need widening and a reviewer should not go looking for siblings.Companion issue from the same PR: the
/heartbeat/reporttelemetry fields, filed separately. Both are the same failure shape — a response indistinguishable from the correct one.mrveiss commentedon Sep 7, 2026
Do not start this — it is already fixed in flight by #15964. Verified against that PR's head (
issue-15905-followup):The predicate is inside the
WHEREand above theLIMIT, which is exactly what this issue asks for. It is 46's follow-up to the three threads #15920 merged past.Close this with that as evidence once #15964 lands — not before. Two acceptance criteria here are not obviously satisfied by the diff above and should be checked against the merged code rather than assumed:
CHECKOUT_CANDIDATESexactly, so raising the constant cannot substitute for the fix. This is the criterion worth defending: a larger limit makes the symptom rarer and reads as a repair.Leaving the issue open rather than closing it now, because a merge is not a closure argument here and the criteria above are what closure has to cite.
mrveiss commentedon Sep 7, 2026
Verified per criterion against merged
origin/Dev_new_gui(#15920 + #15964). Four of six met — leaving this open.WHERE, beforeLIMITwork_item_queue.py—.where(..., claimable_by(uuid.UUID(agent_id)))precedes.limit(CHECKOUT_CANDIDATES).service.checkout(...); noassignee_*comparison survives outsideclaimable_by.CheckoutConflictretry loop unchangedexcept CheckoutConflict: … continue, same shape as before.checkout_nextreturns the unassigned itemcheckout_next:test_agent_stub_routes_15905.py(route behaviour, backlog ordering, cross-company) andtest_checkout_excludes_user_assigned_15964.py(the predicate's shape). Neither seeds a population larger than the limit.CHECKOUT_CANDIDATESexactlyclaimable_byrequires both assignee columns null for "unassigned", which is what "unassigned" means. They agree, andclaimable_by's own docstring states the two-column rule explicitly.What is left is one test file. Criteria 4 and 5 are the same fixture: seed
CHECKOUT_CANDIDATESitems assigned to another agent, ordered ahead of one unassigned item, assertcheckout_nextreturns the unassigned one — then assert the boundary by reading the constant rather than hard-coding 10, so the test moves with it.Worth noting why the tests that exist cannot stand in: they were written against the predicate's shape (mine, in #15964) and against route behaviour. Neither constructs a population that exceeds the limit, which is the only condition under which the original bug appeared. A test of the fix's shape is not a test of the failure it fixed.
mrveiss commentedon Sep 7, 2026
Criteria 4 and 5 are addressed in PR #15988.
Criterion 5 is demonstrated rather than asserted. With the original defect restored and the constant raised 10 → 100, I built a hard-coded-
10variant of the same file and ran both under that mutation:10variantThat is the property the criterion asked for, measured: raising the constant cannot be substituted for fixing the predicate.
test(llc): pin checkout_next's candidate window, and prove the ancest…
mrveiss commentedon Sep 7, 2026
#15988 merged. All six criteria verified against merged
origin/Dev_new_gui. Closing.WHERE, beforeLIMITwork_item_queue.py—.where(..., claimable_by(uuid.UUID(agent_id)))precedes.limit(CHECKOUT_CANDIDATES).service.checkout(...); noassignee_*comparison survives outsideclaimable_by.CheckoutConflictretry loop unchangedexcept CheckoutConflict: … continue, same shape.test_checkout_next_limit_boundary_15965.py::test_an_eligible_item_below_a_full_window_is_still_returnedseeds exactlyCHECKOUT_CANDIDATESdecoys ahead of one unassigned item and asserts the unassigned one is returned.CHECKOUT_CANDIDATESexactlyrange(10)orposition=10. The window is full whatever the constant becomes.claimable_byrequires both assignee columns null for "unassigned", and states the two-column rule explicitly in its own docstring.Criterion 5 was demonstrated, not asserted. With the original defect restored and the constant raised 10 → 100, I built a hard-coded-
10variant of the same file, ran both under that mutation, and deleted it:10variantThat is the criterion's own wording turned into a measurement: a test pinned to the boundary rather than to a number that happens to equal it today. It is also the answer to how do you know raising the constant would have hidden it — because I raised it and watched the other test pass.
Why the tests that already existed could not stand in.
test_checkout_excludes_user_assigned_15964.pyasserts the predicate's shape — I wrote it, and it is a test of the fix.test_agent_stub_routes_15905.pycovers route behaviour and ordering. Neither builds a population exceedingCHECKOUT_CANDIDATES, which is the only condition under which the defect appears. A test of the fix's shape is not a test of the failure it fixed.Two contrasts accompany it, because "return something below the window" is otherwise satisfied by a worse bug than the one being fixed: a window of only another agent's items must still yield
None, and an item already assigned to this agent below a full window must be claimable.Suite at merge:
llc/tests/1828 passed / 9 failed against 1824 / 9 on base — the same nine, plus this file's four.