diff --git a/mcp-server/src/legal_mcp/services/block_writer.py b/mcp-server/src/legal_mcp/services/block_writer.py index ad24357..05986df 100644 --- a/mcp-server/src/legal_mcp/services/block_writer.py +++ b/mcp-server/src/legal_mcp/services/block_writer.py @@ -369,6 +369,7 @@ async def write_block( block_id: str, instructions: str = "", effort_override: str | None = None, + model_override: str | None = None, ) -> dict: """כתיבת בלוק יחיד בהחלטה. @@ -381,6 +382,12 @@ async def write_block( THIS call only — used by the #208 model/effort calibration harness to A/B efforts without mutating the pinned defaults. Production callers leave it None and get the deterministic per-block effort. + model_override: optional per-call generation model id (e.g. + "claude-opus-5"). Same contract as effort_override — the #208 + harness A/Bs MODELS without mutating the pinned GENERATION_MODEL. + Pass the BASE id only: the 1M-context escalation (#216) is applied + on top automatically for large prompts, so an override never + silently loses the 1M window. Production callers leave it None. Returns: dict עם content, word_count, block_id, generation_type @@ -478,7 +485,12 @@ async def write_block( # escalate to the 1M-context build (`[1m]`) instead of failing the block — # block-yod legitimately carries the whole case as source-context. The 400K # ceiling was an artifact of the old 200K-only build, NOT a model limit. - gen_model = GENERATION_MODEL_1M if len(prompt) > _CTX_1M_THRESHOLD_CHARS else GENERATION_MODEL + # model_override (#208 harness) swaps the BASE id only — the 1M decision below + # still applies, so an A/B'd model keeps the same context-window behaviour as + # the pinned default instead of silently falling back to the 200K build. + _base_model = model_override or GENERATION_MODEL + _model_1m = GENERATION_MODEL_1M if _base_model == GENERATION_MODEL else f"{_base_model}[1m]" + gen_model = _model_1m if len(prompt) > _CTX_1M_THRESHOLD_CHARS else _base_model # Final guard: even the 1M build is finite (~2M Hebrew chars of input). Cap at # 1.5M chars (~750K tokens) to leave room for output + a safety margin under 1M. diff --git a/scripts/calibrate_effort.py b/scripts/calibrate_effort.py index 9785ad0..b7bf422 100644 --- a/scripts/calibrate_effort.py +++ b/scripts/calibrate_effort.py @@ -420,13 +420,24 @@ async def _finals_for_calibration(case_filter: str | None) -> list[dict]: async def _score_cell(case_id, block_id: str, effort: str, final_section: str, - final_total_words: int, outcome: str, repeats: int) -> dict: - """Generate `block_id` at `effort` `repeats` times; score each vs the final section.""" + final_total_words: int, outcome: str, repeats: int, + model: str | None = None) -> dict: + """Generate `block_id` at `effort` `repeats` times; score each vs the final section. + + `model` (optional) A/Bs the generation model via write_block(model_override=…). + None ⇒ the pinned GENERATION_MODEL, i.e. the production path unchanged. + """ from legal_mcp.services import block_writer from legal_mcp.services.style_distance import block_distance_to_final runs: list[dict] = [] + models_used: list[str] = [] for _ in range(repeats): - res = await block_writer.write_block(case_id, block_id, effort_override=effort) + res = await block_writer.write_block( + case_id, block_id, effort_override=effort, model_override=model, + ) + # Record what the CLI was actually asked to run, so a silent fallback to + # a different build is visible in the report rather than mis-attributed. + models_used.append(res.get("model_used") or "?") scored = block_distance_to_final( block_id, res.get("content", ""), final_section, outcome, section_target_total_words=final_total_words, @@ -434,6 +445,8 @@ async def _score_cell(case_id, block_id: str, effort: str, final_section: str, runs.append(scored) agg = aggregate_cell(runs) agg["effort"] = effort + agg["model"] = model + agg["models_used"] = sorted(set(models_used)) agg["runs"] = runs return agg @@ -446,6 +459,7 @@ async def _run(args, ts: str) -> dict: efforts = args.efforts blocks = args.blocks + models = args.models finals = await _finals_for_calibration(args.case) cases_meta = [] @@ -468,11 +482,12 @@ async def _run(args, ts: str) -> dict: section = _BLOCK_TO_SECTION.get(block_id) plan[block_id] = [c for c in cases_meta if section and c["sections"].get(section)] - total_cells = sum(len(plan[b]) for b in blocks) * len(efforts) * args.repeats + total_cells = sum(len(plan[b]) for b in blocks) * len(efforts) * args.repeats * len(models) grid_summary = { "n_finals": len(cases_meta), "finals": [c["case_number"] for c in cases_meta], "blocks": blocks, "efforts": efforts, "repeats": args.repeats, + "models": models, "total_generations": total_cells, "per_block_n": {b: len(plan[b]) for b in blocks}, } @@ -480,6 +495,27 @@ async def _run(args, ts: str) -> dict: if args.dry_run: return {"dry_run": True, "grid": grid_summary, "by_block": {}} + by_model: dict[str, dict] = {} + for model in models: + by_block = await _run_blocks_for_model( + model, blocks, efforts, plan, args, ts, grid_summary, by_model, _BLOCK_TO_SECTION, + ) + by_model[model] = by_block + + # `by_block` stays the single-model shape (first model) so --rerank and the + # existing per-block report path keep working unchanged (G2 — no second + # result schema); multi-model runs additionally carry by_model. + out = {"dry_run": False, "grid": grid_summary, "by_block": by_model[models[0]]} + if len(models) > 1: + out["by_model"] = by_model + return out + + +async def _run_blocks_for_model(model, blocks, efforts, plan, args, ts, grid_summary, + by_model_so_far, _BLOCK_TO_SECTION) -> dict: + """The per-block × per-effort grid for ONE generation model.""" + from uuid import UUID + by_block: dict[str, dict] = {} for block_id in blocks: section = _BLOCK_TO_SECTION.get(block_id) @@ -497,11 +533,12 @@ async def _run(args, ts: str) -> dict: cell = await _score_cell( UUID(c["case_id"]), block_id, effort, final_section, c["final_total_words"], c["outcome"], args.repeats, + model=model, ) except Exception as exc: # noqa: BLE001 — harness must survive any cell failure logger.warning( - "calibration cell skipped: case=%s block=%s effort=%s — %s", - c["case_number"], block_id, effort, exc, + "calibration cell skipped: case=%s block=%s effort=%s model=%s — %s", + c["case_number"], block_id, effort, model, exc, ) continue per_effort_runs[effort].append(cell) @@ -532,6 +569,11 @@ async def _run(args, ts: str) -> dict: "recommended": rec["effort"] if rec else None, "confidence": rec["confidence"] if rec else None, "confidence_margin": rec.get("confidence_margin") if rec else None, + "model": model, + # Model builds the CLI actually reported across this block's cells — + # a mismatch vs `model` means a silent fallback, not a real A/B. + "models_used": sorted({m for e in per_effort_runs.values() + for cell in e for m in cell.get("models_used", [])}), "efforts": effort_rows, "per_case": per_case, } @@ -541,11 +583,15 @@ async def _run(args, ts: str) -> dict: # Blocks not yet done are simply absent from by_block; _write_report tolerates # partial results. main() does the final flush once the loop finishes. try: - _write_report({"dry_run": False, "grid": grid_summary, "by_block": by_block}, ts) + snap = {"dry_run": False, "grid": grid_summary, "by_block": by_block} + if by_model_so_far or len(grid_summary.get("models", [])) > 1: + snap["by_model"] = {**by_model_so_far, model: by_block} + _write_report(snap, ts) except Exception as exc: # noqa: BLE001 — a write hiccup must not abort the run - logger.warning("incremental report write failed after block=%s — %s", block_id, exc) + logger.warning("incremental report write failed after block=%s model=%s — %s", + block_id, model, exc) - return {"dry_run": False, "grid": grid_summary, "by_block": by_block} + return by_block IL_TZ = ZoneInfo("Asia/Jerusalem") @@ -578,6 +624,7 @@ def _write_report(result: dict, ts: str) -> tuple[Path, Path]: "ההמלצה אדוויזורית; ההכרעה בידי היו\"ר/המפעיל.\n", f"- בלוקים: {', '.join(g['blocks'])}", f"- efforts: {', '.join(g['efforts'])} · repeats/cell: {g['repeats']}", + f"- models: {', '.join(m or 'pinned-default' for m in g.get('models', [None]))}", f"- סך ייצורי-מודל: {g['total_generations']}", "", ] @@ -613,6 +660,40 @@ def _write_report(result: dict, ts: str) -> tuple[Path, Path]: f"| {r['effort']}{star} | {r['distance']:.4f} | {r['anti_pattern_total']} | " f"{r['change_percent']} | {ratio if ratio is not None else '—'} | {r['n']} |") lines.append("") + by_model = result.get("by_model") or {} + if len(by_model) > 1: + lines += ["## השוואת-מודלים (אותו block, אותו effort, אותם סופיים)\n", + "| block | effort | model | anti_total | change% | ratioΔpp | distance | n |", + "|---|---|---|---|---|---|---|---|"] + for b in g["blocks"]: + for eff in g["efforts"]: + rows = [] + for m, bb in by_model.items(): + for r in (bb.get(b) or {}).get("efforts", []): + if r["effort"] == eff: + rows.append((m, r)) + if len(rows) < 2: + continue # nothing to compare for this cell — don't fake a row + best = min(rows, key=lambda mr: (mr[1]["anti_pattern_total"], + mr[1]["golden_ratio_deviation_pp"] or 0, + mr[1]["distance"]))[0] + for m, r in rows: + ratio = r["golden_ratio_deviation_pp"] + star = " ⭐" if m == best else "" + lines.append( + f"| {b} | {eff} | {m}{star} | {r['anti_pattern_total']} | " + f"{r['change_percent']} | {ratio if ratio is not None else '—'} | " + f"{r['distance']:.4f} | {r['n']} |") + lines.append("") + # A silent CLI fallback would make the whole comparison meaningless — surface it. + for m, bb in by_model.items(): + for b, bd in bb.items(): + used = bd.get("models_used") or [] + if used and any(not u.startswith(str(m)) for u in used): + lines.append(f"> ⚠️ **{b} / {m}**: ה-CLI דיווח `{', '.join(used)}` — " + "ייתכן fallback שקט; ההשוואה לתא זה אינה תקפה.\n") + lines.append("") + lines.append("> דירוג-ההמלצה **style-clean** (#213): anti_total ראשי → ratioΔ → distance (tiebreak). " "**change% מדווח-לא-מדורג** — מערבב סגנון עם שלמות-תוכן (07-learning §0.7), " "anti_total הוא הסיגנל הנקי-לסגנון. confidence=⚠️weak ⇒ הבחירה בתוך-הרעש " @@ -633,6 +714,9 @@ async def main() -> int: help="comma block ids to calibrate") ap.add_argument("--case", default=None, help="restrict to a single case_number") ap.add_argument("--repeats", type=int, default=1, help="generations per cell (avg out gen noise)") + ap.add_argument("--models", default="", + help="comma generation-model ids to A/B (e.g. claude-opus-4-8,claude-opus-5). " + "Empty (default) = the pinned GENERATION_MODEL, i.e. production unchanged.") args = ap.parse_args() logging.basicConfig(level=logging.INFO, format="%(asctime)s %(levelname)s %(message)s") @@ -653,6 +737,9 @@ async def main() -> int: if bad_b: print(f"non-calibratable block(s): {bad_b}. valid: {VALID_BLOCKS}", file=sys.stderr) return 2 + # [None] = "use the pinned GENERATION_MODEL" — keeps the default run byte-identical + # to the pre-#models behaviour instead of hard-coding the id in a second place (G2). + args.models = [m.strip() for m in args.models.split(",") if m.strip()] or [None] ts = _ts() result = await _run(args, ts)