Continue to id whether model is placed

Author: terwill

libtbx/langchain/agent/api_client.py

@@ -93,6 +93,10 @@ def build_session_state(session_info, session_resolution=None):
         if session_info.get("advice_changed"):
             session_state["advice_changed"] = True
 
+        # S2L: client-side model CRYST1 cell for server-side Tier 1 mismatch check
+        if session_info.get("unplaced_model_cell"):
+            session_state["unplaced_model_cell"] = session_info["unplaced_model_cell"]
+
     return session_state
 
 

libtbx/langchain/agent/graph_nodes.py

@@ -590,7 +590,8 @@ def perceive(state):
         analysis=analysis,
         maximum_automation=state.get("maximum_automation", True),
         use_yaml_engine=use_yaml_workflow,
-        directives=state.get("directives", {})
+        directives=state.get("directives", {}),
+        session_info=state.get("session_info", {}),
     )
 
     # Q1: When the user provides new advice on resume and the workflow is

libtbx/langchain/agent/workflow_engine.py

@@ -51,7 +51,7 @@ def __init__(self):
     # CONTEXT BUILDING
     # =========================================================================
 
-    def build_context(self, files, history_info, analysis=None, directives=None):
+    def build_context(self, files, history_info, analysis=None, directives=None, session_info=None):
         """
         Build a context dict from files, history, and analysis.
 
@@ -147,7 +147,9 @@ def build_context(self, files, history_info, analysis=None, directives=None):
             # Note: short-circuit against history_info is applied after the
             # dict is fully built (see below) — context self-reference is not
             # possible inside the literal.
-            "cell_mismatch": self._check_cell_mismatch(files),
+            "cell_mismatch": self._check_cell_mismatch(
+                files,
+                model_cell=(session_info or {}).get("unplaced_model_cell")),
 
             # Tier 3: diagnostic probe results (set by history after
             # phenix.model_vs_data / phenix.map_correlations ran as a probe).
@@ -284,7 +286,7 @@ def _has_search_model(self, files):
         """
         return bool(files.get("search_model"))
 
-    def _check_cell_mismatch(self, files):
+    def _check_cell_mismatch(self, files, model_cell=None):
         """
         Tier 1 placement check: compare model and data unit cells.
 
@@ -298,6 +300,10 @@ def _check_cell_mismatch(self, files):
 
         Args:
             files (dict): Categorised files dict from the agent.
+            model_cell (list|tuple|None): Pre-read CRYST1 cell transmitted from
+                the client (S2L).  When supplied, skips opening the PDB file —
+                which would always fail on the server because the file lives on
+                the client filesystem and os.path.exists() returns False there.
 
         Returns:
             bool: True → definitive mismatch; False → compatible or unknown.
@@ -306,16 +312,62 @@ def _check_cell_mismatch(self, files):
             from libtbx.langchain.agent.placement_checker import (
                 check_xray_cell_mismatch,
                 check_cryoem_cell_mismatch,
+                cells_are_compatible,
+                read_mtz_unit_cell,
+                read_map_unit_cells,
             )
         except ImportError:
             try:
                 from agent.placement_checker import (
                     check_xray_cell_mismatch,
                     check_cryoem_cell_mismatch,
+                    cells_are_compatible,
+                    read_mtz_unit_cell,
+                    read_map_unit_cells,
                 )
             except ImportError:
                 return False   # Module not available — fail-safe
 
+        # ── S2L: use pre-read cell from client when available ────────────────
+        # The server cannot open files that only exist on the client filesystem.
+        # When the client transmitted unplaced_model_cell in session_info, use it
+        # directly for comparison rather than trying to open the PDB file.
+        if model_cell is not None:
+            try:
+                _mc = tuple(float(v) for v in model_cell)
+            except (TypeError, ValueError):
+                _mc = None
+
+            if _mc is not None:
+                # X-ray: compare against MTZ (MTZ lives on the server in LocalAgent
+                # mode, or the comparison is run locally for RemoteAgent via the
+                # client-side CRYST1 cell here)
+                mtz_files = files.get("data_mtz", [])
+                if mtz_files:
+                    mtz_cell = read_mtz_unit_cell(mtz_files[0])
+                    if mtz_cell is not None:
+                        return not cells_are_compatible(_mc, mtz_cell)
+                    return False   # MTZ unreadable → fail-safe
+
+                # Cryo-EM: compare against map file (map is server-accessible
+                # because it was just created by resolve_cryo_em on the server,
+                # or transmitted as a path the server can reach)
+                map_files = (files.get("full_map", []) or
+                             files.get("optimized_full_map", []) or
+                             files.get("map", []))
+                if map_files:
+                    full_cell, present_cell = read_map_unit_cells(map_files[0])
+                    if full_cell is None and present_cell is None:
+                        return False   # Map unreadable → fail-safe
+                    if full_cell is not None and cells_are_compatible(_mc, full_cell):
+                        return False
+                    if present_cell is not None and cells_are_compatible(_mc, present_cell):
+                        return False
+                    if full_cell is not None or present_cell is not None:
+                        return True  # At least one cell readable; model matched neither
+                return False
+
+        # ── Fallback: read cell from file (works for LocalAgent / test mode) ─
         # Get the first model file (generic PDB, not a positioned subcategory)
         model_files = files.get("model", [])
         pdb_path = model_files[0] if model_files else None
@@ -327,8 +379,10 @@ def _check_cell_mismatch(self, files):
         if mtz_files:
             return check_xray_cell_mismatch(pdb_path, mtz_files[0])
 
-        # Cryo-EM: compare against map (full_map preferred, fall back to map)
-        map_files = files.get("full_map", []) or files.get("map", [])
+        # Cryo-EM: compare against map (full_map preferred, then optimized, then map)
+        map_files = (files.get("full_map", []) or
+                     files.get("optimized_full_map", []) or
+                     files.get("map", []))
         if map_files:
             return check_cryoem_cell_mismatch(pdb_path, map_files[0])
 
@@ -1805,7 +1859,7 @@ def get_target(self, experiment_type, metric, resolution=None):
     # =========================================================================
 
     def get_workflow_state(self, experiment_type, files, history_info, analysis=None,
-                           directives=None, maximum_automation=True):
+                           directives=None, maximum_automation=True, session_info=None):
         """
         Get complete workflow state (compatible with workflow_state.py output).
 
@@ -1816,11 +1870,14 @@ def get_workflow_state(self, experiment_type, files, history_info, analysis=None
             analysis: Current log analysis
             directives: Optional user directives dict
             maximum_automation: If False, use stepwise path (process_predicted -> phaser)
+            session_info: Optional session state dict; used for client-supplied
+                          data that the server cannot read directly (e.g.
+                          unplaced_model_cell — CRYST1 from the client-side PDB)
 
         Returns:
             dict: Workflow state compatible with existing code
         """
-        context = self.build_context(files, history_info, analysis, directives)
+        context = self.build_context(files, history_info, analysis, directives, session_info)
 
         # Add automation_path to context for program filtering
         context["automation_path"] = "automated" if maximum_automation else "stepwise"

libtbx/langchain/agent/workflow_state.py

@@ -939,7 +939,37 @@ def _analyze_history(history):
             _result = _entry.get("result", "")
 
             if _is_failed_result(_result):
-                continue   # Ignore failed cycles for probe detection
+                # ── S2L: probe failures that carry placement information ──────
+                # A failed map_correlations run before any refine/dock is the
+                # placement probe.  Some crashes are themselves definitive
+                # answers — extract the signal before discarding the entry.
+                #
+                # "entirely outside map": model coordinates are completely
+                #   outside the map box → model is definitively not placed →
+                #   needs_dock.
+                #
+                # Any other failure: mark placement_probed=True with result=None
+                #   (inconclusive) so the probe never runs a third time.
+                #   Routing: placement_uncertain becomes False, falls through to
+                #   obtain_model → predict/dock fallback.
+                if "map_correlations" in _ecomb and not _seen_refine_or_dock:
+                    _rl = (_result or "").lower()
+                    _outside_signals = [
+                        "entirely outside map",
+                        "outside map",
+                        "model is outside",
+                        "model entirely outside",
+                        "stopping as model",
+                    ]
+                    if any(s in _rl for s in _outside_signals):
+                        # Hard evidence: model is not in the map at all
+                        info["placement_probed"] = True
+                        info["placement_probe_result"] = "needs_dock"
+                    elif not info.get("placement_probed"):
+                        # Unknown failure — prevent infinite probe retry
+                        info["placement_probed"] = True
+                        # Leave placement_probe_result as None (inconclusive)
+                continue   # Ignore failed cycles for all other probe detection
 
             # Once refinement or docking has run, anything after is validation
             if ("refine" in _ecomb and "real_space" not in _ecomb and
@@ -1132,7 +1162,7 @@ def _clear_zombie_done_flags(history_info, available_files, log_func=None):
 
 
 def detect_workflow_state(history, available_files, analysis=None, maximum_automation=True,
-                         use_yaml_engine=True, directives=None):
+                         use_yaml_engine=True, directives=None, session_info=None):
     """
     Determine current workflow state based on history and files.
 
@@ -1182,7 +1212,7 @@ def detect_workflow_state(history, available_files, analysis=None, maximum_autom
 
             engine = WorkflowEngine()
             state = engine.get_workflow_state(experiment_type, files, history_info, analysis,
-                                             directives, maximum_automation)
+                                             directives, maximum_automation, session_info)
 
             state["categorized_files"] = state.get("categorized_files", files)  # S2c: respect promoted files
             # Set automation_path for both experiment types

libtbx/langchain/docs/project/CHANGELOG.md

@@ -1,5 +1,142 @@
 # PHENIX AI Agent - Changelog
 
+## Version 112.52 (S2L — Probe Crash Interpreted + Client Cell Transport)
+
+Fixes the apoferritin AIAgent_165 scenario where the placement probe
+(`phenix.map_correlations`) crashed with "model is entirely outside map",
+ran a second time with the same result, then caused the agent to quit —
+never routing to `dock_in_map`.
+
+### Root cause analysis — three independent failures
+
+**Failure 1 — Tier 1 (cell mismatch) is non-functional in production.**
+`_check_cell_mismatch` calls `check_cryoem_cell_mismatch(pdb_path, map_path)`
+which immediately calls `os.path.exists(pdb_path)`. On the server, `pdb_path`
+is `/Users/terwill/.../1aew_A.pdb` — a client-side path. `os.path.exists`
+returns `False`. Function returns `False` (fail-safe). The 4× unit cell
+mismatch (184 Å F432 crystal vs 32.5 × 39.65 × 36.4 Å P1 sub-box) is never
+detected. **Tier 1 has been silently broken for all RemoteAgent users.**
+
+**Failure 2 — The crash itself carries the answer, but the code discards it.**
+The probe runs correctly. `map_correlations` raises:
+
+```
+Sorry: Stopping as model is entirely outside map and wrapping=False
+```
+
+This is a stronger signal than a low CC — it's categorical proof the model
+is not placed. But `_analyze_history` has this at the top of the probe loop:
+
+```python
+if _is_failed_result(_result):
+    continue   # Ignore failed cycles for probe detection
+```
+
+The entire entry is discarded. `placement_probed` stays `False`.
+
+**Failure 3 — The loop.**
+With `placement_probed=False`, `placement_uncertain` is still `True` on the
+next cycle. The agent runs `map_correlations` again → same crash → same discard
+→ eventually the LLM consecutive-failure counter trips and the run stops with
+no docking ever attempted.
+
+### Fix 1 — Probe crash detection in `_analyze_history` (workflow_state.py)
+
+Before the `continue`, inspect failed `map_correlations` entries that occurred
+before any refine/dock cycle:
+
+```python
+if _is_failed_result(_result):
+    if "map_correlations" in _ecomb and not _seen_refine_or_dock:
+        _rl = (_result or "").lower()
+        _outside_signals = [
+            "entirely outside map", "outside map", "model is outside",
+            "model entirely outside", "stopping as model",
+        ]
+        if any(s in _rl for s in _outside_signals):
+            # Hard evidence: model is not in the map at all
+            info["placement_probed"] = True
+            info["placement_probe_result"] = "needs_dock"
+        elif not info.get("placement_probed"):
+            # Unknown failure — prevent infinite probe retry
+            info["placement_probed"] = True
+            # Leave placement_probe_result as None (inconclusive)
+    continue
+```
+
+Three outcomes:
+- **"outside map" crash** → `placement_probed=True, result="needs_dock"` → routes to `dock_model`
+- **Other crash** → `placement_probed=True, result=None` → inconclusive, falls through to `obtain_model`
+- **Second failed probe** → `placement_probed` already set, no overwrite; guard on `not info.get("placement_probed")` prevents the inconclusive case from clobbering an earlier definitive result
+
+### Fix 2 — Client-side model cell transport (S2L-b)
+
+The client reads the model's CRYST1 cell (which it has access to) and transmits
+it in `session_state["unplaced_model_cell"]`. The server uses this pre-read cell
+in `_check_cell_mismatch` instead of trying to open the file:
+
+**Client side** (`programs/ai_agent.py`): Before assembling `session_info`,
+read the CRYST1 cell from the first unplaced PDB in `active_files` and add it
+to `session_info["unplaced_model_cell"]`. Only populated when placement hasn't
+been confirmed by history (no `dock_done`, no `refine_done`).
+
+**Transport** (`agent/api_client.py`): `build_session_state` passes
+`unplaced_model_cell` through to `session_state`.
+
+**Server receipt** (`phenix_ai/run_ai_agent.py`): Maps `unplaced_model_cell`
+from `session_state` into `session_info`.
+
+**Server use** (`agent/workflow_engine.py`): `build_context` passes
+`session_info` down to `_check_cell_mismatch(files, model_cell=...)`.
+`_check_cell_mismatch` uses the pre-read cell for comparison against the map
+(which the server **can** read — it was just created by `resolve_cryo_em`
+on the server). Tier 1 now fires correctly in production:
+
+```
+model cell  = (184, 184, 184, 90, 90, 90)   # F432 crystal
+map cell    = (32.5, 39.65, 36.4, 90, 90, 90)  # P1 sub-box
+→ mismatch > 5% on all three axes → cell_mismatch=True → dock_model
+```
+
+### Also fixed: `optimized_full_map` not checked by `_check_cell_mismatch`
+
+After `resolve_cryo_em`, the output `denmod_map.ccp4` is categorized as
+`optimized_full_map` (not `full_map`). The original code only checked
+`files.get("full_map") or files.get("map")`. Added `optimized_full_map`
+as a priority-2 fallback so the freshly-generated density-modified map is
+always found.
+
+### Files changed
+
+| File | Change |
+|------|--------|
+| `agent/workflow_state.py` | `_analyze_history`: probe-crash handler before `continue`; `detect_workflow_state` gains `session_info` parameter |
+| `agent/workflow_engine.py` | `_check_cell_mismatch` rewritten with `model_cell` parameter and S2L fast-path; `optimized_full_map` added to map search; `build_context` accepts `session_info`; `get_workflow_state` accepts `session_info` |
+| `agent/api_client.py` | `build_session_state` passes `unplaced_model_cell` through |
+| `phenix_ai/run_ai_agent.py` | Maps `unplaced_model_cell` from `session_state` → `session_info` |
+| `programs/ai_agent.py` | Reads CRYST1 cell client-side, adds to `session_info["unplaced_model_cell"]` |
+| `agent/graph_nodes.py` | Passes `session_info=state.get("session_info",{})` to `detect_workflow_state` |
+| `tests/tst_audit_fixes.py` | 8 new S2L tests (131 total) |
+
+### Corrected cycle trace for AIAgent_165
+
+| Cycle | Program | Why (after fix) |
+|-------|---------|-----------------|
+| 1 | `phenix.mtriage` | Map analysis |
+| 2 | `phenix.resolve_cryo_em` | Half-maps → denmod_map |
+| **3** | **`phenix.dock_in_map`** | **Tier 1: cell_mismatch=True (client cell + server map) → dock_model** |
+| 4 | `phenix.real_space_refine` | Model docked, refine |
+
+Without the fix, cycle 3 ran `map_correlations` twice (probe crash not interpreted), then stopped.
+
+### Note on test coverage
+
+Tests 5 and 6 (api_client and workflow_engine) are marked SKIP in non-PHENIX
+environments because they require the production libtbx import path. They pass
+fully in a PHENIX installation. Tests 1–4, 7–8 pass in all environments.
+
+---
+
 ## Version 112.48 (S2k — _inject_user_params Empty Program Guard)
 
 Fixes a subtle Python truth-value bug introduced in v112.47 where the