diff --git a/backend/app/core/config.py b/backend/app/core/config.py index d0f7e8ea..acca94da 100644 --- a/backend/app/core/config.py +++ b/backend/app/core/config.py @@ -199,6 +199,28 @@ class Settings(BaseSettings): "legacy", "w18j_overlap", "shot_aware_plan" ] = "legacy" + # ── TASK3-B: missing-shot_aware_plan direct-plate fallback ── + # When ``background_render_reference_mode='shot_aware_plan'`` an + # fp-anchored bg whose ``shot_aware_bg_render_plan`` entry is MISSING + # (``plan is None`` — e.g. the upstream geometry readback transiently + # failed for that fp and the plan step dropped it) is otherwise emitted + # as ``failed`` with zero image calls, which turns a single dropped fp + # into a ``partial`` background_render and cascades to a + # ``scene_image_pipeline`` STALE_UPSTREAM block for the WHOLE episode. + # True (default): such bgs degrade to the existing direct-plate render + # path (text_only + prior-bg refs, the same path site-surface bgs use) + # WHEN the direct-plate inputs are present (a non-empty t2i_prompt). + # The degraded entry is explicitly marked + # (``shot_aware_plan_mode='missing_plan_direct_plate_fallback'`` + + # ``render_degraded=True`` + ``fallback_reason='shot_aware_plan_missing'`` + # + ``missing_fp_id``) — visible graceful degradation, never a silent + # fidelity loss. A bg with no direct-plate inputs stays ``failed``. This + # is a structural gate ONLY (plan-missing + renderable), never keyed on + # location name / kind. False: the legacy fail-closed behaviour (every + # bg under the missing fp emitted ``failed``). The ``plan present but + # not production_clear`` case is unaffected — it always fails closed. + background_render_missing_shot_aware_plan_fallback_enabled: bool = True + # ── W20A base location dossier opt-in selector ── # False (default): BaseLocationDossierStep returns not_applicable; # nothing in the W19 path or downstream consumers changes — bytewise @@ -250,6 +272,23 @@ class Settings(BaseSettings): # module's import graph. floor_plan_vlm_readback_real_provider_enabled: bool = False + # ── TASK3-A: floor_plan_geometry_readback per-fp bounded retry ── + # The real VLM readback (``litellm_vlm_provider``) is a single + # ``litellm.completion(num_retries=0)`` call per fp; a transient, + # stochastic provider error (e.g. a duplicate marker NUMBER that trips + # ``validate_provider_output``) drops that fp's readback with no retry, + # which cascades: the shot_aware plan step excludes the fp (no geometry + # block), background_render fail-closes, scene_image_pipeline blocks. + # This selector adds a per-fp bounded retry in the STEP loop (a distinct + # layer from the provider's ``num_retries=0`` per-call audit lock): on a + # ``GeometryReadbackError`` the step re-invokes ``compute_readback`` up to + # this many extra times. Each retry is a fresh counted provider call, so + # ``real_vlm_call_count`` stays honest. Retry only applies when a real + # provider is resolved (the synthetic-fixture path is deterministic — a + # retry would fail identically, so it is skipped to keep the default-OFF + # payload byte-identical). 0 = legacy single-attempt behaviour. + floor_plan_geometry_readback_max_retries: int = 2 + # ── W21B-wave-4 floor_plan_semantic_readback opt-in selector ── # False (default): FloorPlanSemanticReadbackStep returns # not_applicable; no marker-semantic fidelity gate runs. diff --git a/backend/app/core/steps/background_render_step.py b/backend/app/core/steps/background_render_step.py index a4ebd432..f4ecac36 100644 --- a/backend/app/core/steps/background_render_step.py +++ b/backend/app/core/steps/background_render_step.py @@ -153,6 +153,21 @@ class BackgroundRenderStep(StepRunner): payload[ "background_render_reference_mode" ] = settings.background_render_reference_mode + # TASK3-B: the missing-plan direct-plate fallback materially changes + # background_render output for a dropped fp (degrade-render vs + # fail-closed), so its policy must invalidate the checkpoint. Stamped + # ONLY in the shot_aware_plan path (the sole mode where the gate + # runs) — legacy/default stays byte-identical. + if settings.background_render_reference_mode == "shot_aware_plan": + payload[ + "background_render_missing_shot_aware_plan_fallback_enabled" + ] = bool( + getattr( + settings, + "background_render_missing_shot_aware_plan_fallback_enabled", + False, + ) + ) # W21B-w4 #4(C) — the substrate consumer materially changes a fresh # plate's render input (FP + ref_tree_parents vs the adapter's either-or # refs), so flipping it must invalidate existing background_render @@ -885,7 +900,24 @@ class BackgroundRenderStep(StepRunner): ingestion_counter = 0 - for bid in direct_plate_bids: + def _render_direct_plate( + bid: str, + *, + mode_label: str, + decision_fp_id: str = "", + degrade_meta: Optional[Dict[str, Any]] = None, + ) -> Tuple[Dict[str, Any], str, List[str]]: + """Render one bg via the direct-plate (text_only + prior-bg) + path and build its group entry. Shared by the surface-role + direct-plate lane and the TASK3-B missing-shot_aware_plan + fallback. Returns ``(entry, png_path, source_bg_stems)`` — + ``png_path`` is '' when the render did not succeed (failed / + rejected path); the caller owns ``rendered_paths`` registration, + catalog growth, ``ingestion_counter`` and ``failed``. + ``mode_label`` stamps ``shot_aware_plan_mode`` + + ``reference_decision.mode``; ``degrade_meta`` (optional) is + merged into the entry for visible graceful-degradation marking. + """ spec = bg_specs.get(bid) or {} prompt_entry = prompts_map.get(bid, {}) or {} t2i_prompt = prompt_entry.get("t2i_prompt", "") @@ -903,7 +935,7 @@ class BackgroundRenderStep(StepRunner): logger.error( "background_render: out_path escapes image_dir for %s", bid ) - groups_out[bid] = self._build_opt_in_failed_entry( + rejected = self._build_opt_in_failed_entry( bid=bid, spec=spec, prompt_entry=prompt_entry, @@ -911,8 +943,7 @@ class BackgroundRenderStep(StepRunner): error="out_path escapes image_dir", status="rejected_path", ) - failed += 1 - continue + return rejected, "", [] info = render_one_background( openai_client=client, @@ -927,6 +958,7 @@ class BackgroundRenderStep(StepRunner): status = info.get("status", "failed") png_path = info.get("png_path", "") if status == "ok" else "" ref_used = info.get("ref_used", "text_only") + source_bg_stems = [p.stem for p in prior_bg_paths] entry = { "status": status, "location_id": spec.get("loc_id", ""), @@ -944,22 +976,30 @@ class BackgroundRenderStep(StepRunner): "floor_plan_used": False, "camera_recommendations": camera_recs_by_bg.get(bid, {}), "reference_decision": { - "mode": "surface_role_direct_plate", + "mode": mode_label, "surface_role": surface_role, - "fp_id": "", - "source_bg_ids": [p.stem for p in prior_bg_paths], + "fp_id": decision_fp_id, + "source_bg_ids": source_bg_stems, }, "effective_render_prompt": t2i_prompt, - "shot_aware_plan_mode": "surface_role_direct_plate", + "shot_aware_plan_mode": mode_label, "attached_reference_lineage": _build_attached_reference_lineage( - fp_id="", + fp_id=decision_fp_id, fp_path=None, prior_bg_paths=prior_bg_paths, ref_used=ref_used, ), } + if degrade_meta: + entry.update(degrade_meta) + return entry, png_path, source_bg_stems + + for bid in direct_plate_bids: + entry, png_path, source_bg_stems = _render_direct_plate( + bid, mode_label="surface_role_direct_plate", + ) groups_out[bid] = entry - if status == "ok" and png_path: + if entry.get("status") == "ok" and png_path: rendered_paths[bid] = Path(png_path) catalog_dump.append({ "bg_id": bid, @@ -967,7 +1007,7 @@ class BackgroundRenderStep(StepRunner): "png_path": png_path, "mode": "surface_role_direct_plate", "node_index": None, - "source_bg_ids": [p.stem for p in prior_bg_paths], + "source_bg_ids": source_bg_stems, "ingestion_order": ingestion_counter, "source_kind": "surface_role_direct_plate", }) @@ -975,6 +1015,20 @@ class BackgroundRenderStep(StepRunner): else: failed += 1 + # TASK3-B: degrade a MISSING fp plan (plan is None) to the + # direct-plate path so a single transiently-dropped fp does not + # turn background_render ``partial`` and cascade to a whole-episode + # scene_image_pipeline STALE_UPSTREAM block. Structural gate only + # (plan-missing + a renderable direct-plate input) — never keyed on + # location name / kind. The ``plan not production_clear`` case is + # unaffected (always fails closed). + missing_plan_fallback = bool( + getattr( + settings, + "background_render_missing_shot_aware_plan_fallback_enabled", + False, + ) + ) for fp_id, fp_bg_ids in bgs_by_fp.items(): plan = plans_per_fp.get(fp_id) if not is_plan_production_clear(plan): @@ -996,6 +1050,41 @@ class BackgroundRenderStep(StepRunner): for bid in fp_bg_ids: spec = bg_specs.get(bid) or {} prompt_entry = prompts_map.get(bid, {}) or {} + # TASK3-B: degrade ONLY a missing plan (plan is None) + # AND only when the direct-plate input is present (a + # non-empty t2i_prompt). Otherwise stay fail-closed. + if ( + missing_plan_fallback + and plan is None + and (prompt_entry.get("t2i_prompt") or "").strip() + ): + entry, png_path, source_bg_stems = _render_direct_plate( + bid, + mode_label="missing_plan_direct_plate_fallback", + decision_fp_id="", + degrade_meta={ + "render_degraded": True, + "fallback_reason": "shot_aware_plan_missing", + "missing_fp_id": fp_id, + }, + ) + groups_out[bid] = entry + if entry.get("status") == "ok" and png_path: + rendered_paths[bid] = Path(png_path) + catalog_dump.append({ + "bg_id": bid, + "fp_id": "", + "png_path": png_path, + "mode": "missing_plan_direct_plate_fallback", + "node_index": None, + "source_bg_ids": source_bg_stems, + "ingestion_order": ingestion_counter, + "source_kind": "missing_plan_direct_plate_fallback", + }) + ingestion_counter += 1 + else: + failed += 1 + continue groups_out[bid] = self._build_opt_in_failed_entry( bid=bid, spec=spec, diff --git a/backend/app/core/steps/floor_plan_geometry_readback_step.py b/backend/app/core/steps/floor_plan_geometry_readback_step.py index 9e9a3275..5f44c647 100644 --- a/backend/app/core/steps/floor_plan_geometry_readback_step.py +++ b/backend/app/core/steps/floor_plan_geometry_readback_step.py @@ -117,6 +117,16 @@ class FloorPlanGeometryReadbackStep(StepRunner): "floor_plan_geometry_readback_enabled": bool( settings.floor_plan_geometry_readback_enabled ), + # TASK3-A: retry count alters readback behaviour for a real + # provider → invalidate the checkpoint when it changes. + "floor_plan_geometry_readback_max_retries": int( + getattr( + settings, + "floor_plan_geometry_readback_max_retries", + 0, + ) + or 0 + ), "schema_version": SCHEMA_VERSION, "prompt_version": PROMPT_VERSION, } @@ -213,6 +223,15 @@ class FloorPlanGeometryReadbackStep(StepRunner): # provider; mixing per-fp would make the call-count audit # ambiguous. base_provider = self._resolve_vlm_provider() + # TASK3-A: per-fp bounded retry. Only meaningful with a real + # provider — the synthetic-fixture path is deterministic, so a + # retry would fail identically; pinning it to 0 there keeps the + # default-OFF payload byte-for-byte identical to legacy. + max_retries = int( + getattr(settings, "floor_plan_geometry_readback_max_retries", 0) + or 0 + ) + effective_retries = max_retries if base_provider is not None else 0 for fp_id, dossier in dossiers.items(): counted_provider: Optional[Callable[..., Dict[str, Any]]] = None # Per-iteration counter — mutated by ``counted`` closure @@ -225,12 +244,43 @@ class FloorPlanGeometryReadbackStep(StepRunner): _counter[0] += 1 return base_provider(**kw) counted_provider = counted + readback_attempts = 0 try: - readback = compute_readback( - dossier=dossier, - fp_image_path=dossier.get("fp_image_path"), - vlm_provider=counted_provider, - ) + # TASK3-A: bounded retry around the (sole) VLM call. A + # transient provider failure (e.g. a stochastic duplicate + # marker NUMBER tripping the validator) is re-attempted up + # to ``effective_retries`` extra times. The broad catch + # mirrors the real failure surface — ``compute_readback`` + # re-raises GeometryReadbackError for shape errors but lets + # a provider-raised ``VlmProviderError`` (and any transient) + # propagate verbatim. Each attempt is a fresh counted call, + # so ``_counter`` / ``real_vlm_call_count`` stay honest. + readback = None + last_exc: Optional[Exception] = None + for _attempt in range(effective_retries + 1): + readback_attempts += 1 + try: + readback = compute_readback( + dossier=dossier, + fp_image_path=dossier.get("fp_image_path"), + vlm_provider=counted_provider, + ) + last_exc = None + break + except Exception as exc: + last_exc = exc + readback = None + if readback is None: + # Retries exhausted — re-raise so the except branches + # below build the failed entry with the original error + # semantics (GeometryReadbackError vs unexpected). + raise ( + last_exc + if last_exc is not None + else GeometryReadbackError( + "readback failed with no captured exception" + ) + ) geometry = compute_geometry_candidates( dossier=dossier, readback=readback ) @@ -242,7 +292,7 @@ class FloorPlanGeometryReadbackStep(StepRunner): _counter[0] if counted_provider is not None else 0 ) vlm_call_total += per_fp_vlm - per_fp[fp_id] = { + entry: Dict[str, Any] = { "fp_id": fp_id, "readback_status": readback["status"], "readback": readback, @@ -251,6 +301,11 @@ class FloorPlanGeometryReadbackStep(StepRunner): "real_vlm_call": bool(per_fp_vlm), "real_vlm_call_count": per_fp_vlm, } + # Additive only when a retry actually happened — keeps the + # single-attempt / synthetic payload byte-identical. + if readback_attempts > 1: + entry["readback_attempts"] = readback_attempts + per_fp[fp_id] = entry completed += 1 except GeometryReadbackError as exc: logger.error( @@ -262,12 +317,15 @@ class FloorPlanGeometryReadbackStep(StepRunner): _counter[0] if counted_provider is not None else 0 ) vlm_call_total += per_fp_vlm - per_fp[fp_id] = { + entry = { "fp_id": fp_id, "error": str(exc)[:300], "real_vlm_call": bool(per_fp_vlm), "real_vlm_call_count": per_fp_vlm, } + if readback_attempts > 1: + entry["readback_attempts"] = readback_attempts + per_fp[fp_id] = entry failed += 1 except Exception as exc: # pragma: no cover — defensive logger.exception( @@ -279,12 +337,15 @@ class FloorPlanGeometryReadbackStep(StepRunner): _counter[0] if counted_provider is not None else 0 ) vlm_call_total += per_fp_vlm - per_fp[fp_id] = { + entry = { "fp_id": fp_id, "error": f"unexpected: {type(exc).__name__}: {exc}"[:300], "real_vlm_call": bool(per_fp_vlm), "real_vlm_call_count": per_fp_vlm, } + if readback_attempts > 1: + entry["readback_attempts"] = readback_attempts + per_fp[fp_id] = entry failed += 1 # Either at least one fp_id was processed OK or every one diff --git a/backend/tests/core/test_background_render_step_w20c.py b/backend/tests/core/test_background_render_step_w20c.py index a26e7af0..398ba2e3 100644 --- a/backend/tests/core/test_background_render_step_w20c.py +++ b/backend/tests/core/test_background_render_step_w20c.py @@ -562,7 +562,14 @@ def test_shot_aware_plan_orders_by_node_index_and_grows_catalog( def test_shot_aware_plan_missing_plan_emits_failed_no_image_calls( tmp_path, monkeypatch, ): + """TASK3-B: with the missing-plan direct-plate fallback DISABLED, a + missing fp plan stays fail-closed (legacy behaviour).""" _apply_settings(monkeypatch, tmp_path) + monkeypatch.setattr( + "app.core.config.settings." + "background_render_missing_shot_aware_plan_fallback_enabled", + False, + ) bg_specs = [_bg_spec(bg_id="L01B01")] cp_map = _build_cp_map( tmp_path=tmp_path, fp_id="fp_a", bg_specs=bg_specs, plan=None @@ -586,6 +593,91 @@ def test_shot_aware_plan_missing_plan_emits_failed_no_image_calls( assert result["data"]["bg_reference_catalog"] == [] +# ─── TASK3-B: missing-plan direct-plate fallback (default ON) ────────── + + +def test_missing_plan_degrades_to_direct_plate_when_enabled( + tmp_path, monkeypatch, +): + """Default fallback ON: a missing fp plan degrades the fp's bgs to the + direct-plate (text_only + prior-bg) render path instead of failing — + rendered, explicitly marked, and folded into the reference catalog so + background_render completes (no STALE_UPSTREAM cascade).""" + _apply_settings(monkeypatch, tmp_path) + monkeypatch.setattr( + "app.core.config.settings." + "background_render_missing_shot_aware_plan_fallback_enabled", + True, + ) + bg_specs = [_bg_spec(bg_id="L01B01")] + cp_map = _build_cp_map( + tmp_path=tmp_path, fp_id="fp_a", bg_specs=bg_specs, plan=None + ) + step = _new_step(tmp_path, cp_map) + + with patch( + "app.core.steps.background_render_step._resolve_openai_client", + return_value=MagicMock(), + ), patch( + "app.modules.pipeline.background_render.render_one_background", + side_effect=_success_renderer(), + ) as mock_render: + result = step._execute() + + mock_render.assert_called_once() + entry = result["data"]["groups"]["L01B01"] + assert entry["status"] == "ok" + assert entry["png_path"] + assert entry["shot_aware_plan_mode"] == "missing_plan_direct_plate_fallback" + assert entry["render_degraded"] is True + assert entry["fallback_reason"] == "shot_aware_plan_missing" + assert entry["missing_fp_id"] == "fp_a" + assert result["completed_count"] == 1 + assert result["failed_count"] == 0 + # The degraded bg is in the reference catalog (downstream sees a png). + catalog = result["data"]["bg_reference_catalog"] + assert any(c["bg_id"] == "L01B01" for c in catalog) + assert any( + c.get("source_kind") == "missing_plan_direct_plate_fallback" + for c in catalog + ) + + +def test_missing_plan_no_direct_inputs_stays_failed(tmp_path, monkeypatch): + """Codex condition: the fallback degrades ONLY when the direct-plate + inputs are present. A bg whose t2i_prompt is empty cannot render + text_only → it stays fail-closed even with the fallback enabled.""" + _apply_settings(monkeypatch, tmp_path) + monkeypatch.setattr( + "app.core.config.settings." + "background_render_missing_shot_aware_plan_fallback_enabled", + True, + ) + bg_specs = [_bg_spec(bg_id="L01B01")] + cp_map = _build_cp_map( + tmp_path=tmp_path, fp_id="fp_a", bg_specs=bg_specs, plan=None + ) + # Strip the direct-plate input. + cp_map["background_prompt"]["data"]["backgrounds"]["L01B01"][ + "t2i_prompt" + ] = "" + step = _new_step(tmp_path, cp_map) + + with patch( + "app.core.steps.background_render_step._resolve_openai_client", + return_value=MagicMock(), + ), patch( + "app.modules.pipeline.background_render.render_one_background", + ) as mock_render: + result = step._execute() + + mock_render.assert_not_called() + entry = result["data"]["groups"]["L01B01"] + assert entry["status"] == "failed" + assert "shot_aware_plan" in entry["render_error"] + assert result["failed_count"] == 1 + + # ─── test #5: plan not production_clear → all bgs failed ─────────────── @@ -1213,6 +1305,11 @@ def test_config_hash_stamps_shot_aware_plan_selector(monkeypatch): shot_aware_payload = dict(legacy_payload) shot_aware_payload["background_render_reference_mode"] = "shot_aware_plan" + # TASK3-B: the missing-plan fallback policy is stamped in the + # shot_aware_plan path (default ON) — legacy / w18j stay byte-identical. + shot_aware_payload[ + "background_render_missing_shot_aware_plan_fallback_enabled" + ] = True shot_aware_expected = hashlib.sha256( _json.dumps(shot_aware_payload, sort_keys=True).encode("utf-8") ).hexdigest()[:16] diff --git a/backend/tests/core/test_floor_plan_geometry_readback_step.py b/backend/tests/core/test_floor_plan_geometry_readback_step.py index d1a17f74..3d665bc4 100644 --- a/backend/tests/core/test_floor_plan_geometry_readback_step.py +++ b/backend/tests/core/test_floor_plan_geometry_readback_step.py @@ -226,14 +226,19 @@ def test_opt_in_mock_provider_invokes_once_per_fp_and_records_counter(tmp_path): def test_opt_in_selector_true_helper_not_implemented_marks_fp_failed(tmp_path): """Flipping the real-provider selector before its follow-up wave - must fail closed at the call site (NotImplementedError → fp - entry 'failed', counter reflects the attempted call).""" + must fail closed at the call site (provider error → fp entry + 'failed', counter reflects the attempted call). ``max_retries=0`` + isolates this single-attempt fail-closed intent from the TASK3-A + retry behaviour (covered by its own tests).""" step = _new_step(tmp_path=tmp_path, cp_map=_cp_map_with_dossier()) with patch("app.core.config.settings.projects_dir", str(tmp_path)), \ patch("app.core.config.settings.background_mode", "on"), \ patch("app.core.config.settings.floor_plan_prompt_version", "6"), \ patch("app.core.config.settings.base_location_dossier_enabled", True), \ patch( + "app.core.config.settings.floor_plan_geometry_readback_max_retries", + 0, + ), patch( "app.core.config.settings.floor_plan_geometry_readback_enabled", True, ), patch( @@ -472,3 +477,138 @@ def test_step_has_no_llm_image_vlm_imports(): tokens = _import_tokens(step) leaks = tokens & _BANNED_IMPORT_TOKENS assert not leaks + + +# ─────────────────── TASK3-A: per-fp bounded retry ─────────────────── + + +def _ok_readback(dossier, grid_size): + return { + "status": "ok", + "fp_id": dossier["fp_id"], + "grid_size": list(grid_size), + "observed_markers": [ + {"number": 1, "row": 1, "col": 1, + "kind": "base_structural_unit"}, + ], + "missing_markers": [], + "extra_markers": [], + "confidence": 0.9, + "diagnostics": [], + } + + +def _readback_settings_patch(tmp_path, max_retries=2): + """Common settings context for the retry tests (real-provider lane).""" + return [ + patch("app.core.config.settings.projects_dir", str(tmp_path)), + patch("app.core.config.settings.background_mode", "on"), + patch("app.core.config.settings.floor_plan_prompt_version", "6"), + patch("app.core.config.settings.base_location_dossier_enabled", True), + patch( + "app.core.config.settings.floor_plan_geometry_readback_enabled", + True, + ), + patch( + "app.core.config.settings.floor_plan_geometry_readback_max_retries", + max_retries, + ), + ] + + +def _run_with(patches, step): + import contextlib + + with contextlib.ExitStack() as stack: + for p in patches: + stack.enter_context(p) + return step._execute() + + +def test_retry_recovers_transient_readback_failure(tmp_path): + """A provider that raises on the first attempt but succeeds on the + second yields an ``ok`` fp once retry is enabled. The retry catches a + NON-GeometryReadbackError (mirrors the real VlmProviderError path), so + a broad transient catch is required. Each attempt is a fresh counted + call → ``real_vlm_call_count`` reflects all attempts honestly.""" + step = _new_step(tmp_path=tmp_path, cp_map=_cp_map_with_dossier()) + calls = {"n": 0} + + def flaky_provider(*, dossier, fp_image_path, grid_size): + calls["n"] += 1 + if calls["n"] == 1: + raise RuntimeError("validator_failed: marker #3 duplicated") + return _ok_readback(dossier, grid_size) + + step.set_vlm_provider_for_testing(flaky_provider) + result = _run_with(_readback_settings_patch(tmp_path, 2), step) + + entry = result["data"]["per_fp"]["fp_a"] + assert entry["readback_status"] == "ok" + assert entry["readback_attempts"] == 2 + assert entry["real_vlm_call_count"] == 2 + assert result["data"]["real_vlm_call_count"] == 2 + assert result["completed_count"] == 1 + assert result["failed_count"] == 0 + + +def test_retry_exhausted_keeps_failed_shape(tmp_path): + """When every attempt fails the fp stays failed with the original + error semantics + attempt count recorded. With max_retries=2 the + provider is invoked 3 times.""" + step = _new_step(tmp_path=tmp_path, cp_map=_cp_map_with_dossier()) + calls = {"n": 0} + + def always_fail(*, dossier, fp_image_path, grid_size): + calls["n"] += 1 + raise RuntimeError("validator_failed: persistent") + + step.set_vlm_provider_for_testing(always_fail) + result = _run_with(_readback_settings_patch(tmp_path, 2), step) + + assert calls["n"] == 3 + entry = result["data"]["per_fp"]["fp_a"] + assert "error" in entry + assert entry["readback_attempts"] == 3 + assert entry["real_vlm_call_count"] == 3 + assert result["failed_count"] == 1 + assert result["completed_count"] == 0 + + +def test_retry_zero_is_single_attempt(tmp_path): + """max_retries=0 = legacy single-attempt: a failing provider is called + exactly once and the fp fails (no retry).""" + step = _new_step(tmp_path=tmp_path, cp_map=_cp_map_with_dossier()) + calls = {"n": 0} + + def always_fail(*, dossier, fp_image_path, grid_size): + calls["n"] += 1 + raise RuntimeError("boom") + + step.set_vlm_provider_for_testing(always_fail) + result = _run_with(_readback_settings_patch(tmp_path, 0), step) + + assert calls["n"] == 1 + assert result["failed_count"] == 1 + + +def test_synthetic_path_skips_retry_field_byte_identical(tmp_path): + """Provider None (synthetic / default-OFF) is deterministic: no retry + is attempted even with a high max_retries, and the per_fp entry carries + NO ``readback_attempts`` key (payload byte-identical to legacy).""" + step = _new_step(tmp_path=tmp_path, cp_map=_cp_map_with_dossier()) + # No provider injected + real-provider selector OFF → resolved provider + # None → synthetic fixture (deterministic, retry skipped). + patches = _readback_settings_patch(tmp_path, 5) + [ + patch( + "app.core.config.settings." + "floor_plan_vlm_readback_real_provider_enabled", + False, + ), + ] + result = _run_with(patches, step) + + entry = result["data"]["per_fp"]["fp_a"] + assert entry["readback_status"] == "synthetic_fixture" + assert "readback_attempts" not in entry + assert result["data"]["real_vlm_call_count"] == 0