fix(aggregator): צד שלם נמחק בשקט — chunking לפי גודל + כשל שמדווח (#233)
בתיק 1069-04-26 נשלחו 310 טענות עוררים בקריאת-Claude אחת. הקריאה החזירה לא-JSON, הקוד רשם warning והחזיר [], והפעולה דיווחה status=completed עם אפס טיעונים לצד המרכזי בערר. 495 propositions_processed — כלומר "עיבדתי הכל". שתי תקלות מובחנות, שתיהן מתוקנות: 1. **הקריאה גדולה מדי.** הפיצול הפר-כתב-טענות שנוסף למשיבים (#224) הסתיר את זה במקרה — הוא שמר על קריאות קטנות — אבל עוררים וּועדה מדברים בקול אחד ואינם מפוצלים לעולם, כך שערר גדול יוצא בקריאה אחת ענקית. ההערה בקוד כבר תיעדה בדיוק את הכשל הזה אצל המשיבים; התיקון פשוט לא הוחל על העוררים. עכשיו כל צד מעל MAX_PROPS_PER_CALL=80 נצבר בכמה קריאות ומשורשר. הפיצול הוא trade-off ולא רווח חינם: כל chunk מקובץ בבידוד, ולכן צד שמתפצל עלול לקבל יותר טיעונים (וחופפים במקצת) מאשר במעבר יחיד. לאבד ליטיגנט שלם גרוע יותר, והחלופה — פרומפט קטן יותר לכל פרופוזיציה — הייתה מנוונת כל תיק כדי לתקן את הגדולים. הסדר נשמר בחיתוך, כי טענות מגיעות ממוינות לפי claim_index ושכנות שייכות בד"כ לאותו ראש-טיעון. 2. **הכשל נבלע.** החזרת [] על תשובה לא-רשימה אינה ניתנת להבחנה מ"לצד הזה אין טיעונים". עכשיו נזרקת AggregationFailed, הקורא רושם אותה ב-errors, והסטטוס יורד ל-completed_with_errors (כלל-הנדסה §6). ההודעה נוקבת בשם הצד ובמספר הפרופוזיציות, אחרת מפעיל שרואה completed_with_errors לא יודע איזה ליטיגנט נעלם. מבחני רגרסיה: chunking לא מאבד ולא מסדר-מחדש (310→4 קריאות, רצף נשמר); תשובה לא-רשימה זורקת ומזכירה את שם הצד. אם המבחן השני יחזור אי-פעם לטעון == [] — באג הבליעה הוחזר. invariants: כלל-הנדסה §6 — אין בליעה שקטה. G1 — תיקון במקור (גודל הקריאה) ולא בקריאה. G2/G12 — לא נגועים; שני השערים ירוקים. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -147,6 +147,38 @@ def _normalize_argument(raw: dict, fallback_topic: str = "") -> dict | None:
|
|||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
class AggregationFailed(RuntimeError):
|
||||||
|
"""One side's aggregation call failed. Never swallowed — see #233."""
|
||||||
|
|
||||||
|
|
||||||
|
#: Largest number of propositions sent to Claude in a single aggregation call.
|
||||||
|
#:
|
||||||
|
#: Above roughly this size the model stops returning a JSON array and the whole
|
||||||
|
#: side is lost. The per-brief split added for the respondent side (#224) hid
|
||||||
|
#: this by accident — it kept those calls small — but appellant and committee
|
||||||
|
#: speak with a single voice and are never split by brief, so a big appeal goes
|
||||||
|
#: out in one oversized call. 1069-04-26 sent 310 propositions and got nothing
|
||||||
|
#: back (#233).
|
||||||
|
#:
|
||||||
|
#: Chunking is a real trade-off, not a free win: each chunk is grouped in
|
||||||
|
#: isolation, so a side split across chunks can end up with more (and slightly
|
||||||
|
#: overlapping) arguments than a single pass would have produced. Losing an
|
||||||
|
#: entire litigant is worse, and the alternative — a smaller prompt per
|
||||||
|
#: proposition — would degrade every case to fix the large ones.
|
||||||
|
MAX_PROPS_PER_CALL = 80
|
||||||
|
|
||||||
|
|
||||||
|
def _chunk(propositions: list[dict], size: int) -> list[list[dict]]:
|
||||||
|
"""Split propositions into calls of at most ``size``, keeping claim order.
|
||||||
|
|
||||||
|
Order matters: claims arrive sorted by ``claim_index``, so neighbouring
|
||||||
|
propositions usually belong to the same head of argument. Slicing in order
|
||||||
|
keeps related material together instead of scattering one argument across
|
||||||
|
chunks.
|
||||||
|
"""
|
||||||
|
return [propositions[i:i + size] for i in range(0, len(propositions), size)]
|
||||||
|
|
||||||
|
|
||||||
async def _aggregate_party(
|
async def _aggregate_party(
|
||||||
party: str, propositions: list[dict], party_name: str = "",
|
party: str, propositions: list[dict], party_name: str = "",
|
||||||
) -> list[dict]:
|
) -> list[dict]:
|
||||||
@@ -155,9 +187,27 @@ async def _aggregate_party(
|
|||||||
``party_name`` names the specific pleading when this is a split side
|
``party_name`` names the specific pleading when this is a split side
|
||||||
(respondent / permit_applicant brief), so the prompt scopes to that
|
(respondent / permit_applicant brief), so the prompt scopes to that
|
||||||
litigant's position only (#224).
|
litigant's position only (#224).
|
||||||
|
|
||||||
|
Sides larger than ``MAX_PROPS_PER_CALL`` are aggregated in several calls and
|
||||||
|
concatenated; a failure in any chunk raises rather than returning a partial
|
||||||
|
side quietly.
|
||||||
"""
|
"""
|
||||||
if not propositions:
|
if not propositions:
|
||||||
return []
|
return []
|
||||||
|
|
||||||
|
if len(propositions) > MAX_PROPS_PER_CALL:
|
||||||
|
chunks = _chunk(propositions, MAX_PROPS_PER_CALL)
|
||||||
|
logger.info(
|
||||||
|
"argument_aggregator: party '%s'%s has %d propositions — "
|
||||||
|
"aggregating in %d calls of up to %d",
|
||||||
|
party, f" ({party_name})" if party_name else "",
|
||||||
|
len(propositions), len(chunks), MAX_PROPS_PER_CALL,
|
||||||
|
)
|
||||||
|
out: list[dict] = []
|
||||||
|
for chunk in chunks:
|
||||||
|
out.extend(await _aggregate_party(party, chunk, party_name=party_name))
|
||||||
|
return out
|
||||||
|
|
||||||
prompt = _build_prompt(party, propositions, party_name=party_name)
|
prompt = _build_prompt(party, propositions, party_name=party_name)
|
||||||
|
|
||||||
try:
|
try:
|
||||||
@@ -171,11 +221,18 @@ async def _aggregate_party(
|
|||||||
) from e
|
) from e
|
||||||
|
|
||||||
if not isinstance(raw_result, list):
|
if not isinstance(raw_result, list):
|
||||||
logger.warning(
|
# NOT a silent []: returning empty here is indistinguishable from "this
|
||||||
"argument_aggregator: Claude returned non-list (%s) for party '%s'",
|
# side genuinely has no arguments", and the caller would then report
|
||||||
type(raw_result).__name__, party,
|
# status=completed while a whole litigant vanished. That is exactly what
|
||||||
|
# happened to the appellant side of 1069-04-26 — 310 claims in, 0
|
||||||
|
# arguments out, "completed" (#233). Raise so the caller records it in
|
||||||
|
# ``errors`` and the status degrades to completed_with_errors (§6).
|
||||||
|
raise AggregationFailed(
|
||||||
|
f"Claude returned {type(raw_result).__name__}, not a list, for party "
|
||||||
|
f"'{party}'{f' ({party_name})' if party_name else ''} with "
|
||||||
|
f"{len(propositions)} propositions — the side would otherwise be "
|
||||||
|
f"dropped without trace",
|
||||||
)
|
)
|
||||||
return []
|
|
||||||
|
|
||||||
out: list[dict] = []
|
out: list[dict] = []
|
||||||
for entry in raw_result:
|
for entry in raw_result:
|
||||||
@@ -276,6 +333,12 @@ async def aggregate_claims_to_arguments(
|
|||||||
group_key = f"{party}·{party_name}" if party_name else party
|
group_key = f"{party}·{party_name}" if party_name else party
|
||||||
try:
|
try:
|
||||||
arguments = await _aggregate_party(party, props, party_name=party_name)
|
arguments = await _aggregate_party(party, props, party_name=party_name)
|
||||||
|
except AggregationFailed as e:
|
||||||
|
# A side that failed is NOT a side with no arguments. Record it so
|
||||||
|
# the status degrades and the caller can see which litigant is
|
||||||
|
# missing (#233).
|
||||||
|
errors.append(f"{group_key}: {e}")
|
||||||
|
continue
|
||||||
except RuntimeError as e:
|
except RuntimeError as e:
|
||||||
# Most likely cause: Claude CLI not installed (running from
|
# Most likely cause: Claude CLI not installed (running from
|
||||||
# the container). Don't crash — record the gap and continue.
|
# the container). Don't crash — record the gap and continue.
|
||||||
|
|||||||
54
mcp-server/tests/test_argument_aggregator.py
Normal file
54
mcp-server/tests/test_argument_aggregator.py
Normal file
@@ -0,0 +1,54 @@
|
|||||||
|
"""Regression tests for argument aggregation (#233).
|
||||||
|
|
||||||
|
Both tests cover the same 2026-08-05 incident from different angles: the
|
||||||
|
appellant side of 1069-04-26 sent 310 propositions in one Claude call, the call
|
||||||
|
came back as something other than a JSON array, and the code logged a warning
|
||||||
|
and returned ``[]``. The caller could not tell that apart from "this side has no
|
||||||
|
arguments", so ``aggregate_claims_to_arguments`` reported ``completed`` with the
|
||||||
|
central litigant of the appeal missing entirely.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from legal_mcp.services.argument_aggregator import (
|
||||||
|
MAX_PROPS_PER_CALL,
|
||||||
|
AggregationFailed,
|
||||||
|
_aggregate_party,
|
||||||
|
_chunk,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_chunk_preserves_every_proposition_and_their_order():
|
||||||
|
"""Chunking must not drop or reorder — losing claims here is invisible."""
|
||||||
|
props = [{"i": i} for i in range(310)]
|
||||||
|
|
||||||
|
chunks = _chunk(props, MAX_PROPS_PER_CALL)
|
||||||
|
|
||||||
|
assert sum(len(c) for c in chunks) == 310, "propositions were lost"
|
||||||
|
assert [p for c in chunks for p in c] == props, "order changed"
|
||||||
|
assert all(len(c) <= MAX_PROPS_PER_CALL for c in chunks)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_non_list_reply_raises_instead_of_dropping_the_side(monkeypatch):
|
||||||
|
"""A malformed reply must surface, never look like an empty side.
|
||||||
|
|
||||||
|
This is the exact 1069-04-26 failure. If this test ever goes back to
|
||||||
|
asserting ``== []``, the silent-drop bug has been reintroduced.
|
||||||
|
"""
|
||||||
|
async def _query_json(prompt, tools=""): # noqa: ARG001
|
||||||
|
return {"error": "not a list"}
|
||||||
|
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"legal_mcp.services.argument_aggregator.claude_session.query_json",
|
||||||
|
_query_json,
|
||||||
|
)
|
||||||
|
|
||||||
|
with pytest.raises(AggregationFailed) as excinfo:
|
||||||
|
await _aggregate_party("appellant", [{"id": "x", "claim_text": "t"}])
|
||||||
|
|
||||||
|
# The message has to name the side, or an operator reading
|
||||||
|
# completed_with_errors cannot tell which litigant went missing.
|
||||||
|
assert "appellant" in str(excinfo.value)
|
||||||
Reference in New Issue
Block a user