fix: handle whole-file JSON documents that are not objects (#176)

[dmccoystephenson]

Summary

Six readers of a whole JSON file guarded only against json.JSONDecodeError, so a document whose top level parses as a valid JSON array, scalar, or null crashed the reader rather than being handled as corrupt. This is the whole-file companion to the JSONL-line case fixed in #177, and it is closed here the same way: parse, then check isinstance(parsed, dict), then route a non-object document into whatever degradation path that site already has.

persistence/state_manager.py is the sharpest of the six. The recovery branch there exists precisely so a corrupt state.json is quarantined to state.json.corrupt and the instance starts fresh instead of failing — but dict() coercion raised before the rename could run, so the instance neither recovered nor moved the bad file aside, and every subsequent start failed identically. A top-level [] was worse still: dict([]) returns {} silently, so an empty snapshot was reported as a successful load while the bad file stayed on disk.

Per-site resolution, each matching the behavior that site already gives an undecodable document:

Site	Before	After
persistence/state_manager.py	dict() raised before the quarantine branch	quarantined to state.json.corrupt, None returned, next start is clean
scripts/doctor.py	TypeError/ValueError escaped the guarded (OSError, JSONDecodeError) tuple and aborted the survey	None returned; the instance is still listed, with the placeholders a missing state.json produces
interfaces/web/server.py	state.get(...) raised AttributeError, failing the whole /instances listing	the one unreadable instance is skipped, the rest are listed
experiments/runner.py	subscripting raised TypeError, which the guarded tuple did not catch	0 returned, matching the existing missing/undecodable degradation
experiments/metrics.py (load_state)	annotation promised a mapping; AttributeError surfaced later inside a metric function	ValueError naming the path
experiments/compare.py (_read_json)	same annotation mismatch	ValueError naming the path

The two experiment readers were given a raise rather than an empty mapping, departing from the resolution sketched in the issue. The reasoning: json.JSONDecodeError already propagates from both functions, so a non-object document is now reported the same way malformed JSON is, and the error names the offending file. Returning {} instead would have routed a bad metrics.json silently into the recompute-from-journal fallback and rendered a bad state.json as a run with no mood at all — fabricated analysis output from a research tool, which is worse than a stopped comparison.

One correction to the issue body, noted for the record: the string case coerces via ValueError (dictionary update sequence element #0 has length 1; 2 is required), not TypeError. Both escape the guards at every affected site, so the described consequence holds.

experiments/runner.py:_read_thought_count additionally gained TypeError in its inner guard, since int() of a well-formed object's list-valued thought_count raises there too — the same class of defect one layer in.

Related issue

Closes #176

Testing

• python -m compileall core llm memory persistence interfaces scripts experiments tests — clean.
• python -m pytest tests/ -q could not be executed in this environment: the only interpreter available is Python 3.8.10 with neither pytest nor fastapi installed, and the project floor is 3.11 (asyncio.to_thread, used by StateManager.load, does not exist before 3.9). The test suite is therefore UNVERIFIED locally, and the tests workflow on this PR's head SHA is the real anchor. This PR must not be merged on a local green that was never obtained.
• Direct behavioral probes were run against the changed modules under the available interpreter (with a to_thread shim) to confirm each site: all six non-object document shapes are quarantined by StateManager.load, a second load() on the same directory then returns None cleanly, metrics.load_state raises with the path named, and _read_thought_count returns 0 for every unreadable shape and 42 for a valid one.
• Pre-fix behavior was confirmed empirically rather than by reasoning: dict() raises TypeError on [1, 2] / 123 / null / true, raises ValueError on "a bare string", and silently yields {} on [].

New coverage: 8 test functions (parametrized across the five non-object shapes, plus [] where the silent-coercion case applies) in test_consciousness.py, test_doctor.py, test_web_server.py, test_experiment_metrics.py, test_experiment_compare.py, and test_experiment_runner.py.

Scope note

Twelve files are touched, above the ten-file soft ceiling. Six are test files; the non-test change is ~64 net LOC across six one-shape-per-site guards. The six sites were kept in one PR because they are a single defect with a single shape, and splitting them would leave the invariant half-applied.

Checklist

• Tests pass locally — no, see Testing above; UNVERIFIED locally, gated on CI
• No code comment, log message, or variable name asserts a phenomenal state
• INDICATORS.md updated if this changes which Butlin et al. indicators are implemented — not applicable; this is a robustness fix with no capability shift. state_manager.py carries an AE-1/HOT-3 theory-mapping docstring, which is unchanged and still accurate: identity persistence across run boundaries now survives a strictly larger set of corrupt snapshots.
• Docs updated if behavior or CLI flags changed — no doc drift is introduced. consciousness-sim/README.md and the root README.md make no claim about corrupt-file handling. CLAUDE.md's invariant list carries "JSONL readers skip corrupted lines with a warning rather than crashing", which remains accurate as written; adding a whole-file sibling invariant was deliberately left out, because editing agent-loaded config requires separate maintainer authorization.

Backlog deferred this cycle

Recorded for auditability, per the loop's skip-reason rule:

• packaging: no console entry point and no packaged config, so an installed copy has nothing runnable #171 (packaging entry points) — turns on a maintainer decision between "library only" and "packaged application"; not an agent call.
• feat(infra): containerize the project — Ollama, consciousness instances, and web dashboard #121 (containerization) and feat(web): experiment management UI — list / view / launch / compare experiments from the dashboard #95 (web experiment-management UI) — both well past the size ceiling for a single polish-scoped PR.
• research: perception-influence metric needs to evolve beyond word-overlap (heuristic stops working as agent matures) #74 (perception-influence metric) and spike: investigate why the LLM's prior pulls toward the cosmic-tapestry register (and what would break it) #64 (cosmic-tapestry register spike) — research spikes whose output is an investigation, not a patch.
• feat: add CLI commands for managing consciousness instances #10 (CLI instance-management commands) — larger than this cycle's budget, and partially overtaken by scripts/doctor.py and scripts/stop.py.

---

This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

---

drafted by Claude on behalf of Daniel Stephenson

[dmccoystephenson]

Self-review rubric

Scored adversarially against the diff and command output, not judgment.

• Scope: PASS — the six source files are exactly the six sites tabulated in fix: whole-file JSON readers crash on a valid-JSON non-object document #176, and the six test files mirror them one-for-one. No formatting, renames, or comment churn outside the changed functions.
• Tests-new: PASS — each of the six changed functions gains coverage: StateManager.load (test_consciousness.py), _read_state via collect_instances (test_doctor.py), list_instances (test_web_server.py), load_state (test_experiment_metrics.py), _read_json via load_run (test_experiment_compare.py), _read_thought_count (test_experiment_runner.py).
• Tests-fix: PASS — established empirically, not by reasoning. The four source files were reverted to origin/main and the new assertions replayed: 16/16 failed, with the failure mode matching the issue's claim at each site (TypeError from dict() on [1, 2]/123/null/true, ValueError on a bare string, and — the silent case — dict([]) returning {} so state_manager reported a successful load of an empty snapshot with the bad file left in place). With the fix restored, 11/11 replayable assertions pass; the five _read_thought_count cases are blocked locally only by pydantic being absent and were confirmed separately in isolation (pre-fix raises TypeError on all five shapes, post-fix returns 0).
• Sibling structure: PASS — no new files. Each new test block carries the section-header style its neighbors use and follows the plain def test_<behavior>_<condition> plus @pytest.mark.parametrize shape established by the fix: JSONL readers crash on a valid-JSON non-object line instead of skipping it #175 tests directly above them.
• Sibling renames: N/A — nothing renamed.
• Docs: PASS — the documentation table was walked. Neither README makes a claim about corrupt-file handling; the CLAUDE.md invariant "JSONL readers skip corrupted lines" concerns the line-oriented path and remains accurate as written.
• Issue resolution: PASS — all six rows of the issue's affected-sites table are changed; none is left partially addressed.
• CI: PASS — pytest and smoke are both green on the head SHA. The pytest job reports 724 passed, against 687 passed on main, so the 37 new parametrized cases were collected and executed rather than silently skipped; mypy reports Success: no issues found in 48 source files under the strict gate.
• Theory-annotation: PASS — the AE-1 / HOT-3 mapping docstring in persistence/state_manager.py is untouched and still accurate; identity persistence across run boundaries now survives a strictly larger set of damaged snapshots. No other changed module carries or needs one.
• Indicator log: PASS — no file named in any per-indicator "Implementation mapping" subsection of CLAUDE.md is touched. INDICATORS.md was deliberately left unchanged, for the reason stated in the PR body: this is a robustness fix with no capability shift in either direction.
• Phenomenal-consciousness drift: PASS — a grep for is conscious|is experiencing|it feels|aware of itself over the added lines returns zero matches. The added comments describe JSON shapes and file handling only.
• No-silent-swallow: PASS — no bare except: or except Exception: pass is added, and no production LLM provider path is touched. The one place deserving a closer look is recorded below.

Findings from the read-through

consciousness-sim/experiments/compare.py:60 — a deliberate behavior change worth a maintainer's eye, reversible in one line. A metrics.json whose top level is specifically [] previously fell through the if not metrics: branch in load_run and was recomputed from journal.jsonl, which produced a correct comparison, since compute_all is the same function that writes that artifact. It now raises instead. The pre-fix behavior was not uniform — [1, 2] in the same file crashed later with AttributeError — so the choice lay between making every shape recompute and making every shape report. Reporting was chosen because it names the damaged file rather than quietly working around it, and because metrics.json is only ever written as a mapping, so a non-object there is external corruption rather than a supported input. Should the tolerance be judged more valuable than the diagnostic, routing a non-object metrics.json into the existing recompute fallback is a one-line change; state.json should keep raising either way, being underivable from the journal.

consciousness-sim/scripts/doctor.py:65 and consciousness-sim/interfaces/web/server.py:194 — the new non-object branches degrade silently, with no log line. This was checked rather than assumed: both sites already discard an undecodable state.json silently in the sibling except clause immediately above, so logging in only the new branch would leave the two paths inconsistent, and logging in both would widen the diff past the issue. The omission is flagged so it reads as considered rather than accidental; a follow-up adding a warning at both sites would be reasonable.

consciousness-sim/persistence/state_manager.py:53 — the pre-existing comment explaining that rename() must follow the closing of the read handle now sits above the extracted _quarantine helper rather than directly above the try. The guarantee is unaffected, and the pre-existing test that spies on Path.open and Path.rename to enforce the ordering is green in CI, but the comment's referent has moved one construct away from what it describes.

Anchor caveat

The test suite could not be executed locally: the only interpreter available here is Python 3.8.10 without pytest or fastapi, below the project's 3.11 floor. The green tests workflow on this PR's head SHA is therefore the sole anchor for the suite, and the empirical result recorded under Tests-fix was obtained by replaying the new assertions against reverted and restored source rather than through pytest.

This review was performed and posted during a Gardener session (https://github.com/Stephenson-Software/gardener).

---

drafted by Claude on behalf of Daniel Stephenson