test(labeling): don't assert batch completion order - #3269
Conversation
`test_label_communities_batches_when_over_batch_size` fails about 75% of
the time (9 of 12 runs here, in isolation — not an interaction with any
other test).
`label_communities` fans its batches out across a ThreadPoolExecutor and
collects them with `as_completed`, so the mocked `_call_llm` is invoked
from several threads and finishing order is not dispatch order. The
50-community batch is the smallest and usually completes before the
second 100, so the recorded list is `[100, 50, 100]`:
assert [100, 50, 100] == [100, 100, 50]
The batching itself is always correct — 250 communities at batch_size=100
gives 100/100/50 every run. Only the order varies, which the test never
meant to pin: its own comment says "3 batches (100, 100, 50)".
Asserts on the sorted sizes instead, and guards the shared list with a
lock rather than relying on `list.append` being atomic under the GIL.
20/20 runs pass with the change; the rest of the suite is unaffected.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
Guards the shared calls list in test_label_communities_batches_when_over_batch_size with a lock and asserts on the sorted contents, reflecting that label_communities now dispatches batches across a ThreadPoolExecutor where completion order is nondeterministic.
No blocking issues surfaced.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 76 functions depend on the 76 functions this change touches.
Health — grade A; no new coupling hotspots.
Verification — 76 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 76 function(s) in the blast radius were not formally verified this run
1 participant
test_label_communities_batches_when_over_batch_sizefails about 75% of the time — 9 of 12 runs on a cleanv8checkout here, in isolation, with no other test involved.label_communitiesfans its batches out across aThreadPoolExecutorand collects them withas_completed, so the mocked_call_llmruns on several threads and completion order is not dispatch order. The 50-community batch is the smallest and usually finishes before the second 100.The batching itself is correct on every run: 250 communities at
batch_size=100always produces 100/100/50. Only the recorded order varies — which the test never intended to pin, going by its own comment, "250 communities / 100 per batch -> 3 batches (100, 100, 50)".Change
Assert on the sorted sizes rather than the sequence:
and guard the shared list with a
threading.Lockinstead of relying onlist.appendbeing atomic under the GIL. That second part isn't fixing an observed failure — CPython makes it safe today — but the test appends from a thread pool and shouldn't depend on that.Verification
tests/test_labeling.py🤖 Generated with Claude Code