test(llc): the sweep's fake session reads SQLAlchemy internals — if they move it stops honouring LIMIT silently

Problem

workspace_lease_sweep_test.py's fake session honours a SQL LIMIT so that
test_a_backlog_of_executed_proposals_does_not_hide_the_unexecuted_one can actually observe the
defect it was written for (#16818, PR #17725). It reads the limit off SQLAlchemy internals:

limit = getattr(statement, "_limit", None)
if limit is None:
    limit_clause = getattr(statement, "_limit_clause", None)
    limit = getattr(limit_clause, "value", None)

Both are private attributes. If SQLAlchemy renames or restructures them, every getattr returns
None, the fake silently stops applying limits, and the regression test passes whether or not the
bug is present — back to being unfailable.

The failure arrives through a dependency bump rather than a code change, so nothing in the diff that
breaks it will look like it touches this.

Why this is worth its own issue

This is the same lesson one layer down from the one that produced the test. The original test could
not fail because the fake ignored LIMIT; teaching it to read LIMIT fixed that and created a new
unverified assumption — that the reading still works. A harness that silently degrades is
indistinguishable from one that works, which is the property the whole #16818 review was about.

Acceptance criteria

A test in the same file hands the fake session a select(...).limit(1) against two rows and
asserts exactly one comes back.
It fails if both getattr paths return None — i.e. it tests the fake, not SQLAlchemy.
The test names the reason in its docstring, so it is not read as redundant and deleted.

Note on scope

Two lines of assertion. The reason it is filed rather than slipped into #17725 is that #17725 is
queued and reviewed; this is a new test for a test harness, not a fix to the change under review.

Premise check — not verifiable against `main`: this reviews code that has not landed. Read-only at `origin/main` `ab05ba3aef`.

The file this issue is about does not exist on main. git ls-tree -r origin/main finds no workspace_lease_sweep* and no workspace_disposal* anywhere in the tree. Both this issue and its sibling are reviews of #16818's implementation in PR #17725, which is reviewed and in the CI queue rather than merged.

That has three consequences worth recording before anyone schedules it:

A premise check against main cannot confirm or refute it. The evidence for it lives at feat(llc): make the workspace lease real — acquire, release, reclaim, and dispose on evidence (#16818) #17725's head. Any verdict I posted against main would be a statement about a tree that does not contain the subject.
If feat(llc): make the workspace lease real — acquire, release, reclaim, and dispose on evidence (#16818) #17725 merges as it stands, this becomes a live defect on main the moment it lands — not new work discovered later, but a known finding arriving with the code. Folding it into feat(llc): make the workspace lease real — acquire, release, reclaim, and dispose on evidence (#16818) #17725 before merge costs one review round; landing it and fixing it afterwards costs a second PR and leaves main briefly carrying a defect that was known in advance.
The cheaper sequencing is therefore to resolve it inside feat(llc): make the workspace lease real — acquire, release, reclaim, and dispose on evidence (#16818) #17725, and to keep this issue open only if the owner prefers the fix to land separately. That is a scheduling choice, not an engineering one, and it should be made before the merge rather than after.

Nothing here disputes the finding. It is a statement about where it can be verified and when it is cheapest to act on.

fix(llc): bound the approval window, and stop the fake session lying …

feat(llc): make the workspace lease real — acquire, release, reclaim,…

Navigation Menu

Repository navigation

test(llc): the sweep's fake session reads SQLAlchemy internals — if they move it stops honouring LIMIT silently #17739

Problem

Why this is worth its own issue

Acceptance criteria

Note on scope

Related

Premise check — not verifiable against `main`: this reviews code that has not landed. Read-only at `origin/main` `ab05ba3aef`.

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

test(llc): the sweep's fake session reads SQLAlchemy internals — if they move it stops honouring LIMIT silently #17739

Description

Problem

Why this is worth its own issue

Acceptance criteria

Note on scope

Related

Activity

mrveiss commented on Sep 28, 2026

Premise check — not verifiable against main: this reviews code that has not landed. Read-only at origin/main ab05ba3aef.

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions

Premise check — not verifiable against `main`: this reviews code that has not landed. Read-only at `origin/main` `ab05ba3aef`.