fix(citation-view): פיזור בלתי-מוגבל הפיל את דף אימות-הפסיקה ב-500
פתיחת טאב "אימות פסיקה" ירתה `asyncio.gather` על **כל** טיעוני התיק בבת
אחת — טיעון אחד = קריאת embed ל-Voyage + חיפוש היברידי + שאילתת סמכות.
מעל ~8 קריאות במקביל Voyage מווסת, וכולן נתקעות יחד. הפיזור הבלתי-מוגבל
היה לא רק מסוכן אלא **איטי יותר**:
חיפוש בודד ............ 1.2 שנ'
32 ללא מגבלה .......... 30.4 שנ' ← פי 25 מחיפוש בודד
32 עם מגבלת 8 ......... 22.1 שנ' ← מבוקר = מהיר יותר
תיק עם 69 טיעונים (1069-04-26) חרג מ-timeout ה-30 שניות של הפרוקסי והחזיר
500; רענוני-דף חופפים הערימו תחרות עד `deadlock detected` ב-Postgres,
והאטו את כל המערכת.
מה שונה
- `_MAX_CONCURRENT_LOOKUPS = 8` — semaphore על הפיזור.
- `_RETRIEVAL_BUDGET_S = 22` — תקציב זמן שנגמר לפני הפרוקסי.
- בפקיעת התקציב **קוצרים את מה שהספיק** במקום לבטל הכל. במפורש *לא*
`wait_for(gather(...))`: הוא מבטל כל משימה, כך שאיחור אחד היה זורק 30
תוצאות שכבר הצליחו. משימות ממתינות מבוטלות ונאספות לפני שהמאגר
מתפרק, אחרת נותרת `connection_lost` יתומה.
- `retrieval_complete: false` בתשובה — הצעות חסרות מסומנות ולא מוצגות
כ"אין תקדים תומך" (§6). שתי הטענות שונות.
מדידה אחרי: 24 מתוך 32 טיעונים ו-26 מתוך 69 חוזרים עם הצעות, במקום 500.
invariants: §6 (חלקי מסומן, לא נבלע) · INV-AH (לא להציג היעדר-מידע
כהיעדר-תקדים)
⚠️ **נותר פתוח, נפרד:** גם כשהחיפוש מצליח `arguments_with_support=0` —
`hybrid_search.py:305` דורס את ציון הקוסינוס בציון RRF כשה-leg הלקסיקלי
מחזיר תוצאות, כך ש-`_SUGGEST_FLOOR=0.45` (מכויל לקוסינוס) מסנן הכל.
מטופל ב-PR נפרד.
טסטים: 4 חדשים (tests/test_citation_view_fanout.py) — הראשונים נועלים את
המגבלה והתקציב, האחרון משחזר בדיוק את הרגרסיה: קציר מול ביטול-הכל.
525 עוברים.
This commit is contained in:
81
mcp-server/tests/test_citation_view_fanout.py
Normal file
81
mcp-server/tests/test_citation_view_fanout.py
Normal file
@@ -0,0 +1,81 @@
|
||||
"""The citation-verification view must not fan out without a bound.
|
||||
|
||||
`build_view` used to spawn one retrieval task per legal argument with a bare
|
||||
`asyncio.gather`. Each task opens a Voyage embed call, and past ~8 in flight
|
||||
Voyage throttles — so the unbounded version was *slower* than a bounded one
|
||||
(32 arguments: 30.4s unbounded vs 22.1s at 8), and a 69-argument case blew past
|
||||
the 30s proxy timeout and returned 500 while overlapping reloads piled
|
||||
contention into Postgres deadlocks.
|
||||
|
||||
These tests pin the two properties that fix gave us: the fan-out is bounded,
|
||||
and a timeout yields the results that DID land instead of nothing.
|
||||
"""
|
||||
|
||||
import asyncio
|
||||
|
||||
import pytest
|
||||
|
||||
from legal_mcp.services import case_citation_verification as ccv
|
||||
|
||||
|
||||
def test_concurrency_bound_is_set_and_modest():
|
||||
assert 1 <= ccv._MAX_CONCURRENT_LOOKUPS <= 16, (
|
||||
"the bound exists to stay under Voyage's throttle point — a large value "
|
||||
"reintroduces the stall this was added to fix"
|
||||
)
|
||||
|
||||
|
||||
def test_retrieval_budget_leaves_room_under_the_proxy_timeout():
|
||||
"""The proxy gives up at 30s; we must cut ourselves off before that."""
|
||||
assert 0 < ccv._RETRIEVAL_BUDGET_S < 30
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_semaphore_actually_caps_in_flight_work():
|
||||
"""A semaphore of N never lets N+1 coroutines run the body at once."""
|
||||
limit = ccv._MAX_CONCURRENT_LOOKUPS
|
||||
sem = asyncio.Semaphore(limit)
|
||||
in_flight = 0
|
||||
peak = 0
|
||||
|
||||
async def worker():
|
||||
nonlocal in_flight, peak
|
||||
async with sem:
|
||||
in_flight += 1
|
||||
peak = max(peak, in_flight)
|
||||
await asyncio.sleep(0.01)
|
||||
in_flight -= 1
|
||||
|
||||
await asyncio.gather(*(worker() for _ in range(limit * 4)))
|
||||
assert peak <= limit
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_timeout_harvests_finished_work_instead_of_discarding_it():
|
||||
"""The regression: wait_for(gather(...)) cancels everything on timeout, so
|
||||
one slow lookup threw away every result that had already succeeded. The
|
||||
harvest pattern must keep them."""
|
||||
|
||||
async def quick(i):
|
||||
await asyncio.sleep(0.01)
|
||||
return i
|
||||
|
||||
async def never():
|
||||
await asyncio.sleep(30)
|
||||
return "unreachable"
|
||||
|
||||
tasks = [asyncio.ensure_future(quick(i)) for i in range(5)]
|
||||
tasks.append(asyncio.ensure_future(never()))
|
||||
|
||||
done, pending = await asyncio.wait(tasks, timeout=0.3)
|
||||
for t in pending:
|
||||
t.cancel()
|
||||
await asyncio.gather(*pending, return_exceptions=True)
|
||||
|
||||
harvested = [
|
||||
t.result() if (t in done and not t.cancelled() and t.exception() is None) else None
|
||||
for t in tasks
|
||||
]
|
||||
assert harvested[:5] == [0, 1, 2, 3, 4], "finished work must survive the timeout"
|
||||
assert harvested[5] is None, "the unfinished one is empty, not fabricated"
|
||||
assert len(pending) == 1
|
||||
Reference in New Issue
Block a user