fix(observability): rename state.json phase → last_entered_phase (#236) (#238)

state.json's `phase` field reflects the **last entered** phase, not the
**currently active** phase. The engine writes it once on each
`transition()`; artifact writes within a phase do not refresh it. So
between phase transitions an operator polling state.json sees stale
labels — including a phase whose artifacts have already been written
to disk. From the issue:

> In one observed run on `reflective@76ed590`:
>
> | Time | `state.json.phase` | iter-1 artifacts on disk |
> |---|---|---|
> | 0:00 | INIT | (none) |
> | 0:14 | DESIGN | bundle.yaml, problem.md written at 0:14 |
> | 0:42 | DESIGN (still!) | findings.json written at 0:42 |
> | 0:51 | EXECUTE_ANALYZE | first state update post-DESIGN |
>
> Between 0:14 and 0:51, state.json claimed phase=DESIGN despite
> EXECUTE_ANALYZE having long since started AND completed.

Renaming the on-disk key to `last_entered_phase` makes the entry-only
semantics unambiguous. The schema description, the data-model doc, and
the architecture doc all spell out that operators wanting fine-grained
progress should watch the iteration artifact directory's mtimes rather
than `state.json`.

## What changed

- `orchestrator.engine` writes `last_entered_phase` and reads either
  key on load, normalizing legacy `phase` → `last_entered_phase` in
  memory. Migration happens automatically on the next `transition()`
  or `force_phase()`, which drops the legacy key. No flag day, no
  separate migration script — in-flight pre-#236 runs converge on
  their next phase advance.
- `Engine.last_entered_phase` is the canonical Python property;
  `Engine.phase` is kept as an alias for source compatibility (most
  internal callers already use it).
- `read_phase_field(state, default=None)` helper centralizes the
  backward-compat lookup for code that reads state.json without going
  through Engine. Wired into `status.py`, `warm_start.py`, `cli.py`,
  and `campaign_index.py`.
- `templates/state.json` and `schemas/state.schema.json` use the
  canonical name; the schema description documents entry-only
  semantics inline.
- `docs/data-model.md` and `docs/architecture.md` document the
  semantics and the migration story.

## What did NOT change

- The orchestrator's behavior. `transition()` still writes on entry
  only; `force_phase()` still bumps `iteration`. The fix is a renaming
  and a documentation pass — exactly what the issue asked for.

## Tests

- New `TestLastEnteredPhaseRename` (8 tests) covers the rename,
  backward-compat read, automatic migration on next save, both helper
  branches, and the missing-keys / unrecognized-phase error paths.
- Existing engine assertions on `saved["phase"]` updated to
  `saved["last_entered_phase"]`. Fixture-style `"phase":` keys in
  test_campaign.py / test_schemas.py / test_critic.py / test_pre_work.py
  / test_templates.py updated to the canonical name (the legacy key is
  exercised in the dedicated rename tests).
- End-to-end smoke check confirms a synthetic pre-#236 state.json loads,
  reports the correct phase via both `Engine.phase` and `read_phase_field`,
  and migrates to the canonical shape on the next transition.

1182 passed, 2 skipped. No regressions.

Closes #236.
Refs #228 (isolation tracker).

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: sriumcp

docs/architecture.md

@@ -84,7 +84,7 @@ INIT ──▶ DESIGN ──▶ HUMAN_DESIGN_GATE
 - DONE → DESIGN (next iteration, increments counter)
 
 **Key behaviors:**
-- `transition(to_state)` validates against the transition table, updates the timestamp, and atomically writes `state.json`.
+- `transition(to_state)` validates against the transition table, updates the timestamp, sets `last_entered_phase`, and atomically writes `state.json`. Both fields update only on phase entry — artifact writes within a phase do not refresh them (#236), so operators polling `state.json` for progress see entry-time values linger throughout long phases. Watch artifact mtimes for sub-second progress instead.
 - Iteration counter increments only on the DONE → DESIGN transition (starting a new iteration). Loopbacks from HUMAN_DESIGN_GATE → DESIGN (reject) do NOT increment — they are revisions within the same iteration.
 - The DONE state allows transition to DESIGN for the next iteration.
 

docs/data-model.md

@@ -52,19 +52,21 @@ The campaign configuration. Describes the target system and points to prompt lay
 
 **Schema:** `schemas/state.schema.json`
 
-A bookmark. It tells the orchestrator what phase we're in, which iteration we're on, and what we're investigating. If the process crashes, it resumes from here.
+A bookmark. It tells the orchestrator what phase we entered last, which iteration we're on, and what we're investigating. If the process crashes, it resumes from here.
 
 | Field | What it means |
 |---|---|
-| `phase` | Which step of the loop (INIT, DESIGN, HUMAN_DESIGN_GATE, EXECUTE_ANALYZE, HUMAN_FINDINGS_GATE, DONE) |
+| `last_entered_phase` | The last phase the engine entered (INIT, PRE_WORK, DESIGN, CRITIC, HUMAN_DESIGN_GATE, EXECUTE_ANALYZE, HUMAN_FINDINGS_GATE, DONE). **Not** necessarily the currently active phase — see the caveat below (#236). |
 | `iteration` | How many times we've gone around the loop (0 = haven't started yet) |
 | `run_id` | A name for this campaign |
 | `family` | What mechanism we're currently exploring (e.g., "routing-signals") |
-| `timestamp` | When this was last updated |
+| `timestamp` | When this was last updated (i.e., when `last_entered_phase` was last entered — *not* when an artifact was last written) |
 | `config_ref` | Path to the campaign configuration file (null before setup) |
 
 The orchestrator writes this atomically (temp file + rename) so a crash never leaves a corrupt checkpoint.
 
+**Entry-only semantics (#236).** `last_entered_phase` and `timestamp` are updated *only* when the engine transitions into a new phase. Artifact writes within a phase do not update them. So during a long phase, you'll see the entry-time values linger even though the phase is well underway and may have already produced artifacts. If you need a sub-second progress signal (e.g., for a status dashboard), watch the iteration artifact directory's mtimes (`runs/iter-N/*.json`, `runs/iter-N/*.yaml`) rather than `state.json`. The field was renamed from `phase` in #236 to make the entry-only semantics unambiguous; `orchestrator.engine` accepts the legacy key on load (in-flight pre-#236 runs) and migrates it to the canonical name on the next save.
+
 ## 2. ledger.json — "What happened in each iteration?"
 
 **Schema:** `schemas/ledger.schema.json`

orchestrator/campaign_index.py

@@ -114,10 +114,12 @@ def _summarize(root: Path) -> CampaignSummary | None:
     repo: str | None = None
     if root.parent.name == ".nous":
         repo = str(root.parent.parent.resolve())
+    # #236: read via helper so legacy ``phase`` keys still resolve.
+    from orchestrator.engine import read_phase_field
     return CampaignSummary(
         run_id=state.get("run_id", root.name),
         path=str(root.resolve()),
-        phase=state.get("phase", "UNKNOWN"),
+        phase=read_phase_field(state, default="UNKNOWN"),
         iteration=int(state.get("iteration", 0) or 0),
         completed_iterations=completed,
         active_principles=active,

orchestrator/cli.py

@@ -92,9 +92,12 @@ def _cmd_run(args):
         state_path = Path(repo_path) / ".nous" / run_id / "state.json"
         if state_path.exists():
             state = json.loads(state_path.read_text())
-            if state.get("phase") != "INIT":
+            # #236: read via helper so legacy ``phase`` keys still resolve.
+            from orchestrator.engine import read_phase_field
+            phase = read_phase_field(state)
+            if phase != "INIT":
                 print(
-                    f"Run '{run_id}' already in progress (phase={state['phase']}). "
+                    f"Run '{run_id}' already in progress (phase={phase}). "
                     f"Use 'nous resume' to continue.",
                     file=sys.stderr,
                 )

orchestrator/engine.py

@@ -2,19 +2,71 @@
 
 Owns phase transitions and state.json checkpoint/resume.
 This is NOT an LLM — it is a deterministic script.
+
+state.json semantics
+--------------------
+
+The on-disk ``last_entered_phase`` field reflects the **last phase the
+engine entered**, not the **currently active phase**. The two are not
+the same — artifact writes within a phase do not trigger state.json
+updates, so during a long phase you'll see the entry-time value linger
+even though the phase has been working for many seconds. After the
+phase finishes its work, the value continues to linger until the next
+``transition()`` is called and the new phase is entered.
+
+This was renamed from ``phase`` in #236; the legacy key is read for
+backward compat (in-flight runs from older versions) and migrated to
+the new key on the next ``transition()`` or ``force_phase()``.
+
+If you need a more aggressive progress signal (e.g. for a status
+dashboard polling at sub-second granularity), watch the iteration
+artifact directory's mtimes rather than ``state.json``.
 """
 import json
 import logging
+from collections.abc import Mapping
 from datetime import datetime, timezone
 from enum import Enum
 from pathlib import Path
 from types import MappingProxyType
+from typing import Any, overload
 
 from orchestrator.util import atomic_write
 
 logger = logging.getLogger(__name__)
 
-_REQUIRED_STATE_KEYS = {"phase", "iteration", "run_id", "family", "timestamp"}
+# #236: the canonical on-disk key is ``last_entered_phase``. The legacy
+# key ``phase`` is accepted on load for backward compat (in-flight runs)
+# and migrated to the new key on the next save. Required-keys validation
+# uses the canonical name; legacy state.json files are normalized in
+# memory before the check runs.
+_PHASE_KEY = "last_entered_phase"
+_LEGACY_PHASE_KEY = "phase"
+_REQUIRED_STATE_KEYS = {_PHASE_KEY, "iteration", "run_id", "family", "timestamp"}
+
+
+@overload
+def read_phase_field(state: Mapping[str, Any]) -> str | None: ...
+@overload
+def read_phase_field(state: Mapping[str, Any], default: str) -> str: ...
+def read_phase_field(
+    state: Mapping[str, Any],
+    default: str | None = None,
+) -> str | None:
+    """Return state.json's ``last_entered_phase``, falling back to the
+    legacy ``phase`` key (#236).
+
+    Use this in any code that reads state.json **without** going
+    through ``Engine`` — e.g., status dashboards, the campaign index,
+    warm-start probes. Code that holds an ``Engine`` should read
+    ``engine.last_entered_phase`` (or its alias ``engine.phase``)
+    instead.
+    """
+    if _PHASE_KEY in state:
+        return state[_PHASE_KEY]
+    if _LEGACY_PHASE_KEY in state:
+        return state[_LEGACY_PHASE_KEY]
+    return default
 
 
 class Phase(str, Enum):
@@ -73,9 +125,24 @@ def state(self) -> dict:
         """Shallow copy of the current state (safe: state is always a flat dict)."""
         return dict(self._state)
 
+    @property
+    def last_entered_phase(self) -> str:
+        """The last phase the engine entered.
+
+        NOT necessarily the currently active phase — see the module
+        docstring for the entry-only semantics caveat (#236).
+        """
+        return self._state[_PHASE_KEY]
+
     @property
     def phase(self) -> str:
-        return self._state["phase"]
+        """Alias for :pyattr:`last_entered_phase` (#236).
+
+        Kept for source compatibility — most callers in the orchestrator
+        already use ``engine.phase``. New code should prefer
+        ``engine.last_entered_phase`` for clarity.
+        """
+        return self._state[_PHASE_KEY]
 
     @property
     def iteration(self) -> int:
@@ -95,13 +162,19 @@ def _load_state(self) -> dict:
                 f"Corrupt state.json at {self.state_path}: {e}. "
                 f"Restore from backup or re-initialize from templates/state.json."
             ) from e
+        # #236 migration: legacy state.json files use ``phase``; rename
+        # in-memory before the required-keys check so validation reports
+        # the canonical key in error messages. The next save writes the
+        # canonical name, dropping the legacy key.
+        if _PHASE_KEY not in state and _LEGACY_PHASE_KEY in state:
+            state[_PHASE_KEY] = state.pop(_LEGACY_PHASE_KEY)
         missing = _REQUIRED_STATE_KEYS - state.keys()
         if missing:
             raise ValueError(f"state.json missing required keys: {missing}")
         # Validate phase is a recognized state
-        if state["phase"] not in ALL_STATES:
+        if state[_PHASE_KEY] not in ALL_STATES:
             raise ValueError(
-                f"state.json has unrecognized phase '{state['phase']}'. "
+                f"state.json has unrecognized phase '{state[_PHASE_KEY]}'. "
                 f"Valid phases: {sorted(s.value for s in Phase)}"
             )
         return state
@@ -113,7 +186,7 @@ def transition(self, to_state: str) -> None:
                 f"'{to_state}' is not a recognized phase. "
                 f"Valid phases: {sorted(s.value for s in Phase)}"
             )
-        current = self._state["phase"]
+        current = self._state[_PHASE_KEY]
         if current not in TRANSITIONS:
             raise ValueError(f"Unknown state: {current}")
         if to_state not in TRANSITIONS[current]:
@@ -132,8 +205,11 @@ def transition(self, to_state: str) -> None:
             new_state["iteration"] += 1
         elif current == "DONE" and to_state == "DESIGN":
             new_state["iteration"] += 1
-        new_state["phase"] = to_state
+        new_state[_PHASE_KEY] = to_state
         new_state["timestamp"] = datetime.now(timezone.utc).isoformat()
+        # #236: drop the legacy key on save so a migrated state.json
+        # doesn't carry both names.
+        new_state.pop(_LEGACY_PHASE_KEY, None)
         self._save_state(new_state)
         self._state = new_state
         logger.info("Transition: %s -> %s (iteration=%d)", current, to_state, new_state["iteration"])
@@ -151,8 +227,9 @@ def force_phase(self, phase: str) -> None:
             )
         new_state = dict(self._state)
         new_state["iteration"] += 1
-        new_state["phase"] = phase
+        new_state[_PHASE_KEY] = phase
         new_state["timestamp"] = datetime.now(timezone.utc).isoformat()
+        new_state.pop(_LEGACY_PHASE_KEY, None)
         self._save_state(new_state)
         self._state = new_state
         logger.info("Force phase: -> %s (iteration=%d)", phase, new_state["iteration"])

orchestrator/schemas/state.schema.json

@@ -2,18 +2,18 @@
   "$schema": "https://json-schema.org/draft/2020-12/schema",
   "$id": "https://github.com/AI-native-Systems-Research/agentic-strategy-evolution/schemas/state.schema.json",
   "title": "Investigation State",
-  "description": "Current phase, family, and iteration — the investigation checkpoint.",
+  "description": "Last-entered phase, family, and iteration — the investigation checkpoint. The phase field reflects what was last entered; it is not updated as artifacts are written within a phase. Operators wanting fine-grained progress should watch the iteration artifact directory's mtimes (#236).",
   "type": "object",
-  "required": ["phase", "iteration", "run_id", "family", "timestamp"],
+  "required": ["last_entered_phase", "iteration", "run_id", "family", "timestamp"],
   "additionalProperties": false,
   "properties": {
-    "phase": {
+    "last_entered_phase": {
       "type": "string",
       "enum": [
         "INIT", "PRE_WORK", "DESIGN", "CRITIC", "HUMAN_DESIGN_GATE",
         "EXECUTE_ANALYZE", "HUMAN_FINDINGS_GATE", "DONE"
       ],
-      "description": "Current state machine phase. PRE_WORK (issue #167) is an opt-in cheap-exploration phase between INIT and DESIGN. CRITIC (issue #87) is an opt-in falsifiability check between DESIGN and HUMAN_DESIGN_GATE."
+      "description": "The last phase the engine entered (#236). NOT necessarily the currently active phase — artifact writes within a phase do not update this field, so during a long phase you'll see the entry-time value linger even though the phase is well underway. Renamed from `phase` in #236 so its semantics aren't conflated with 'currently active phase.' Pre-#236 state.json files used the key `phase` instead; ``orchestrator.engine`` accepts those for backward compat (in-flight runs) and migrates them to this key on the next save, but the schema only describes the canonical post-#236 shape. PRE_WORK (#167) is an opt-in cheap-exploration phase between INIT and DESIGN. CRITIC (#87) is an opt-in falsifiability check between DESIGN and HUMAN_DESIGN_GATE."
     },
     "iteration": {
       "type": "integer",

orchestrator/status.py

@@ -169,8 +169,10 @@ def read_status_snapshot(
 
     state = _read_json(work_dir / "state.json")
     if isinstance(state, dict):
+        # #236: read via helper so legacy ``phase`` keys still resolve.
+        from orchestrator.engine import read_phase_field
         snap.run_id = str(state.get("run_id", "?"))
-        snap.phase = str(state.get("phase", "?"))
+        snap.phase = str(read_phase_field(state, default="?"))
         snap.iteration = int(state.get("iteration", 0) or 0)
         snap.raw = state
 

orchestrator/templates/state.json

@@ -1,5 +1,5 @@
 {
-  "phase": "INIT",
+  "last_entered_phase": "INIT",
   "iteration": 0,
   "run_id": "TODO-SET-RUN-ID",
   "family": null,

orchestrator/warm_start.py

@@ -143,10 +143,14 @@ def warm_start_from_prior(
     prior_dir = _find_prior_dir(prior_run_id, prior_search_paths)
     state = _load_state(prior_dir)
 
-    if state.get("phase") != "DONE":
+    # #236: read via helper so legacy ``phase`` keys still work for
+    # in-flight campaigns being warm-started off.
+    from orchestrator.engine import read_phase_field
+    prior_phase = read_phase_field(state)
+    if prior_phase != "DONE":
         raise RuntimeError(
             f"prior campaign {prior_run_id!r} is not complete "
-            f"(phase={state.get('phase')!r}); only DONE campaigns can "
+            f"(phase={prior_phase!r}); only DONE campaigns can "
             f"be warm-started from",
         )
 

tests/test_campaign.py

@@ -213,7 +213,7 @@ def test_mid_flight_design_resumes_at_correct_iteration(self, tmp_path):
         from orchestrator.campaign import _resume_completed_campaign
         work_dir = _setup_work_dir(tmp_path)
         state = json.loads((work_dir / "state.json").read_text())
-        state["phase"] = "DESIGN"
+        state["last_entered_phase"] = "DESIGN"
         state["iteration"] = 16
         (work_dir / "state.json").write_text(json.dumps(state))
 
@@ -228,7 +228,7 @@ def test_mid_flight_execute_analyze_resumes_at_correct_iteration(self, tmp_path)
         from orchestrator.campaign import _resume_completed_campaign
         work_dir = _setup_work_dir(tmp_path)
         state = json.loads((work_dir / "state.json").read_text())
-        state["phase"] = "EXECUTE_ANALYZE"
+        state["last_entered_phase"] = "EXECUTE_ANALYZE"
         state["iteration"] = 5
         (work_dir / "state.json").write_text(json.dumps(state))
 
@@ -243,7 +243,7 @@ def test_mid_flight_iteration_1_boundary(self, tmp_path):
         from orchestrator.campaign import _resume_completed_campaign
         work_dir = _setup_work_dir(tmp_path)
         state = json.loads((work_dir / "state.json").read_text())
-        state["phase"] = "DESIGN"
+        state["last_entered_phase"] = "DESIGN"
         state["iteration"] = 1
         (work_dir / "state.json").write_text(json.dumps(state))
 
@@ -265,7 +265,7 @@ def test_mid_flight_corrupt_iteration_falls_back_to_1(self, tmp_path, caplog):
         from orchestrator.campaign import _resume_completed_campaign
         work_dir = _setup_work_dir(tmp_path)
         state = json.loads((work_dir / "state.json").read_text())
-        state["phase"] = "DESIGN"
+        state["last_entered_phase"] = "DESIGN"
         state["iteration"] = 0
         (work_dir / "state.json").write_text(json.dumps(state))
 
@@ -287,7 +287,7 @@ def test_mid_flight_exceeds_max_iterations_warns(self, tmp_path, caplog):
         from orchestrator.campaign import _resume_completed_campaign
         work_dir = _setup_work_dir(tmp_path)
         state = json.loads((work_dir / "state.json").read_text())
-        state["phase"] = "DESIGN"
+        state["last_entered_phase"] = "DESIGN"
         state["iteration"] = 16
         (work_dir / "state.json").write_text(json.dumps(state))
 
@@ -304,7 +304,7 @@ def test_done_with_more_iterations_configured_resumes(self, tmp_path):
 
         # Simulate a completed single-iteration campaign.
         state = json.loads((work_dir / "state.json").read_text())
-        state["phase"] = "DONE"
+        state["last_entered_phase"] = "DONE"
         (work_dir / "state.json").write_text(json.dumps(state))
         ledger = {"iterations": [
             {"iteration": 0, "family": "baseline"},
@@ -320,7 +320,7 @@ def test_done_at_max_iterations_does_not_resume(self, tmp_path):
         from orchestrator.campaign import _resume_completed_campaign
         work_dir = _setup_work_dir(tmp_path)
         state = json.loads((work_dir / "state.json").read_text())
-        state["phase"] = "DONE"
+        state["last_entered_phase"] = "DONE"
         (work_dir / "state.json").write_text(json.dumps(state))
         ledger = {"iterations": [
             {"iteration": 0, "family": "baseline"},
@@ -338,7 +338,7 @@ def test_done_but_no_real_iterations_does_not_resume(self, tmp_path):
         from orchestrator.campaign import _resume_completed_campaign
         work_dir = _setup_work_dir(tmp_path)
         state = json.loads((work_dir / "state.json").read_text())
-        state["phase"] = "DONE"
+        state["last_entered_phase"] = "DONE"
         (work_dir / "state.json").write_text(json.dumps(state))
         ledger = {"iterations": [{"iteration": 0, "family": "baseline"}]}
         (work_dir / "ledger.json").write_text(json.dumps(ledger))
@@ -352,7 +352,7 @@ def test_corrupt_ledger_does_not_crash_resume(self, tmp_path, caplog):
         from orchestrator.campaign import _resume_completed_campaign
         work_dir = _setup_work_dir(tmp_path)
         state = json.loads((work_dir / "state.json").read_text())
-        state["phase"] = "DONE"
+        state["last_entered_phase"] = "DONE"
         (work_dir / "state.json").write_text(json.dumps(state))
         (work_dir / "ledger.json").write_text("{this is not valid json")
 
@@ -366,7 +366,7 @@ def test_ledger_with_malformed_rows_does_not_crash_resume(self, tmp_path):
         from orchestrator.campaign import _resume_completed_campaign
         work_dir = _setup_work_dir(tmp_path)
         state = json.loads((work_dir / "state.json").read_text())
-        state["phase"] = "DONE"
+        state["last_entered_phase"] = "DONE"
         (work_dir / "state.json").write_text(json.dumps(state))
         ledger = {"iterations": [
             {"iteration": 0, "family": "baseline"},