/lab-team-review MERGED VERDICT (v2, post-fix) — C-C4a mc_codec version bytes

verdict: PASS evidence_class: C supersedes: none (first review pass for this item)

/lab-team-review MERGED VERDICT (v2, post-fix) — C-C4a mc_codec version bytes

• Base SHA: 85b0e8c (working tree, spec-correction pass applied on top, uncommitted)
• Mode: real (5 independent persona agents re-reviewing the corrected spec)
• Files reviewed:
  • production\docs\SPEC_mc_codec_versioning.md

Fix summary (what changed before this re-review)

Only the target spec file was touched — no .ex/.exs/.py/.cjs files were modified, as required.

Summary

Rewrote [redacted: operator-path]\repos\UNI.MineCraft\production\docs\SPEC_mc_codec_versioning.md in place. All three findings were re-verified against the live repo first, then fixed:

Finding 1 (wrong module target) — Confirmed live: lib/sp/brain/mc_codec.ex defines SP.Brain.MCCodec (no dot), with only encode/2 (senses, dna), no decode, no header logic — a sense discretiser, unrelated to persistence. Added a correction note at the top of the spec stating this explicitly, and retargeted every citation away from it. (Also noted a third, genuinely unrelated module, SP.Brain.Codec in codec.ex, exists too — to prevent future confusion.)

Finding 2 (retarget to real persistence path) — Verified SP.Brain.MC.save/2 (lib/sp/brain/mc.ex:552-554, direct non-atomic File.write!+term_to_binary) and load/2 (:563-585, via private safe_read/1 at :587-594, fail-open on any error). Rewrote the entire Contract section around these: header-writing moves into save/2; header-checking moves into a reworked safe_read/2 called by load/2. Critically, I preserved load/2's existing fail-open contract (always returns a bare %SP.Brain.MC{}, never {:error,_}) rather than inventing a breaking {:ok,_}|{:error,_} return — verified via grep that every call site (agent.ex:110, bridge.ex:94 and 5 more save sites) pattern-matches the bare struct today, so a signature change would silently break all of them, which is out of scope for a spec-correction pass. Version/magic mismatches now fall into the same existing "fall back to fresh" branch, made loud via Logger.warning (flagged as new to this file — grep confirmed no file in lib/ uses Logger today). Also documented that this header layer is orthogonal to, not a replacement for, the existing reconcile/2+compatible?/2+adopt/2 struct-shape-drift machinery (mc.ex:596-643), and verified empirically that legacy files always start with byte 0x83 (ERTS term-format tag), so the "MCV1" ASCII magic can never collide with them.

Finding 3 (name the removal mechanism) — Named it precisely: a hardcoded @legacy_cutover_date module attribute (~D[2026-08-11], i.e. today + 30 days, explicitly flagged as a placeholder the implementer must set to actual-merge-date+30) compared against Date.utc_today() (overridable via opts[:now] for deterministic tests) inside the legacy-decode path. Justified as a hardcoded constant rather than a config key because mix.exs has zero deps and no config/config.exs — confirmed live (only ui/ has a Phoenix config). Separately, the original "OR after the 178-file archived corpus is migrated/discarded" clause was re-scoped out of v1 code logic: I verified runs/colony_archive/ doesn't exist and no .bin corpus exists yet in the repo, so "178" is an unverifiable, unfalsifiable figure — I did not invent a fake check against it. It's now explicitly named as an operator-checklist item, with a named prerequisite (a real, inventoried archive directory) for a future SPEC to promote it to code.

Also rewrote Test coverage (new test/sp/brain/mc_versioning_test.exs, positioned relative to the real existing test/sp/brain/mc_test.exs:147-194) and Non-goals (added explicit non-goals for: no public API signature change, no duplication of reconcile, no atomicity change to routine saves, no code-enforced corpus gate). Preserved the Bind-mount and Cross-references sections verbatim since the findings didn't flag them and I verified the cross-referenced docs (SPEC_livepatch_hot_files.md, SPEC_lineage_snapshot.md, RUNBOOK_DR.md) still exist.

Left unresolved / flagged, not fixed: The spec's filename (SPEC_mc_codec_versioning.md) is now a slight misnomer since it no longer touches mc_codec.ex — I left it unchanged because two other docs (production/docs/SPEC_livepatch_hot_files.md and docs/handoffs/UNI_OS_MIND_DEEPENING_HANDOFF.md) reference it by this exact filename, and renaming was not in the findings list and risks breaking those inbound references; this is a naming decision beyond scope, noted here rather than papered over silently.

Persona verdicts (re-review)

math-breaker (Falsifier) — Lab Team role 2/5, default-REJECT, only earns SIGN when every provenance/quantitative claim survives independent re-derivation against the live repo.

• Verdict: SIGN_WITH_CHANGES
• Rationale: Independent re-verification, done the same way the original review did it (read the live source, not the spec's prose):

Prior finding #1 (wrong module) — RESOLVED, confirmed by direct read. lib/sp/brain/mc_codec.ex:1 really does define defmodule SP.Brain.MCCodec do (no dot) with def encode(senses, %Genome{} = dna) (arity-2) and no decode/1, no magic/header/version logic anywhere in the file — matches the spec's correction note exactly. The real persistence path is genuinely SP.Brain.MC.save/2 (lib/sp/brain/mc.ex:552-554) and load/2 (mc.ex:563-585) with private safe_read/1 at mc.ex:587-594 and reconcile/2+compatible?/2+adopt/2 at mc.ex:596-643 — I read the whole file and every one of those line numbers matches the live source exactly. The spec now targets the correct module throughout.

Prior finding #2 (retarget the Contract) — RESOLVED. The new Contract section's header layout (magic 4B + major u16BE + minor u16BE + reserved 8B = 16B total) matches its own Elixir code (@magic <> <<@major::16, @minor::16, 0::64>> = 4+2+2+8 = 16 bytes) and the pattern-match in the proposed safe_read/2 replacement. The empirical claim 'term_to_binary always starts with 0x83' is real and independently verified by me: I read the first 4 bytes of the cited fixture test/fixtures/decider_golden_seed7_d5b3.bin and got 83 6C 00 00 — confirmed. Since 'MCV1' starts with 0x4D, the magic-sniffing collision-avoidance argument is sound (I tried to construct a counter-example and it doesn't exist — this is a fixed, documented ERTS external-term-format invariant, not a fabricated one).

Prior finding #3 (name the removal mechanism) — RESOLVED. @legacy_cutover_date ~D[2026-08-11] as a hardcoded Date.t() module attribute on SP.Brain.MC, compared via Date.compare(now, @legacy_cutover_date) with now defaulting to Date.utc_today() (injectable via opts[:now] for tests), is precise and falsifiable — exactly the kind of concrete mechanism the prior BLOCKED verdict demanded. I verified mix.exs really has deps: [] and only ui/config/config.exs exists in the repo, so the 'no config-loading mechanism exists, use a reviewable constant' justification checks out. The 30-day arithmetic (2026-07-12 + 30 = 2026-08-11) is correct, and the placeholder is honestly flagged as implementer-must-set-at-merge rather than falsely presented as already-final.

So the three BLOCKING findings from the prior pass are genuinely fixed, not just re-worded — I independently re-derived each one against the live repo rather than trusting the spec's self-report, per my mandate. The core Contract (byte layout, offsets, u16 big-endian encoding, magic-collision reasoning, fail-open preservation, atomic tmp+rename migration) is internally consistent and mathematically sound.

But a fresh, independent pass surfaced two NEW provenance defects the correction didn't touch (both are the same species of error finding #1 was about — a claim whose citation doesn't check out against the live repo) plus one minor completeness gap. The most serious is the Bind-mount-contract citation: the spec asserts its migration path is 'guaranteed' safe via a mechanism 'documented in production/docs/RUNBOOK_DR.md extension' — and that document, which I read in full, contains no such extension. That claim underpins the SPEC's own stated precondition for migration safety, so it can't be waved through. The test-coverage mis-citation (claiming existing corrupt-file coverage that doesn't exist) is the same defect class in miniature. Neither undermines the header/version Contract itself, which is sound and ready to land — hence SIGN_WITH_CHANGES rather than REVISE: fix the three named citation issues (especially the RUNBOOK_DR.md one — replace 'guaranteed'/'documented' with either real documentation or an honest PENDING+falsifier) before merge, no redesign of the Contract required.

• Concerns:
  • Purpose-section citation is incomplete provenance: 'loaded/saved by SP.Runtime.Agent (lib/sp/runtime/agent.ex:109-110,185,195,259)' names only 4 of the real save/load touch-points. A full-repo grep shows MC.save/2 is also called from agent.ex:251,310,318,358 (7 sites in that file alone), AND from a wholly separate GenServer, SP.Brain.Bridge (lib/sp/brain/bridge.ex), which has its own independent MC.load call (bridge.ex:94, correctly cited later in Non-goals) plus five more MC.save calls (bridge.ex:141,149,180,185,200) never mentioned anywhere in the spec. Non-fatal — save/2 and load/2 keep their signatures so every caller works unchanged regardless of enumeration — but a spec whose entire purpose this pass is 'cite exactly what's real' should either enumerate bridge.ex's sites or explicitly scope the Purpose section to 'the primary runtime path; SP.Brain.Bridge is a parallel live-body module on the same API.'
  • Test-coverage section mischaracterizes existing coverage. It claims a truncated/unparseable-payload file 'falls back to a fresh brain exactly like the corrupt-file case already covered in mc_test.exs:158-174.' I read that test directly: 'loading a STALE memory file (older shape) starts fresh instead of crashing' writes a perfectly well-formed :erlang.term_to_binary blob of an older-shaped-but-valid Factors struct — it exercises the reconcile/compatible?/adopt shape-mismatch path, NOT safe_read's rescue/catch -> :error path for genuinely unparseable bytes. I grepped the entire test/ tree for any test that writes deliberately-garbage/truncated bytes and asserts MC.load/2 recovers via the rescue path — none exists. The new test the spec proposes to write is fine and needed, but the 'already covered' framing is false; this is exactly the false-citation defect class the correction pass exists to eliminate, recurring in a new spot.
  • The 'Bind-mount contract (paired with C-C4b)' section makes an uncorroborated safety claim in banned-vocabulary voice: 'The migration path is safe ONLY if the persistent kin .bin files survive a git archive unpack. This is guaranteed by the bind-mount contract (C-C4b): /var/lib/uni/colony-memory/ is bind-mounted INTO broadcast-src's runs/colony/... Documented in production/docs/RUNBOOK_DR.md extension.' I read production/docs/RUNBOOK_DR.md in full (139 lines, all DR redeploy/rollback/failover content, some of it itself referencing retired uni-bcast-mixer/verify_p1.sh surfaces per this repo's own CLAUDE.md correction) — it contains ZERO mentions of colony-memory, bind-mount, or C-C4b. A repo-wide grep for 'C-C4b' finds exactly two hits: this spec itself, and a bare inline comment in production/scripts/colony_archive.sh ('the bind-mounted kin .bin files (per C-C4b)') with no accompanying SPEC_*.md or runbook section defining it (unlike C-C4a = this very spec, and C-C4c = the real SPEC_livepatch_hot_files.md). OS_SPOOL_POLICY.md documents only the destination-side backups/colony sole-writer rule, not the source-side bind-mount. So the SPEC's central precondition for migration-path safety is asserted 'guaranteed' and 'documented' where, verifiably, in this repo, neither is true. This is the same genus as prior finding #1 (a citation to something that doesn't exist where claimed) resurfacing in a different section. Fix: either add the actual missing C-C4b documentation/verification, or downgrade to an honest PENDING with a named falsifier (e.g. 'verify /var/lib/uni/colony-memory/ is actually bind-mounted live on uni-lab before trusting this migration path in production') — do not claim 'guaranteed' + 'documented' against a citation that doesn't check out.

aif-theorist

• Verdict: SIGN_WITH_CHANGES
• Rationale: Re-verified independently against the live repo, not by trusting the spec's prose. Finding 1/2 (wrong module targeted) are genuinely resolved: lib/sp/brain/mc_codec.ex does define SP.Brain.MCCodec (no dot) with only encode/2(senses, dna) and no decode/1/header logic -- confirmed by reading the file. The rewritten Contract correctly targets SP.Brain.MC.save/2 (mc.ex:552-554) and load/2 (mc.ex:563-585) with safe_read/1 at :587-594 -- I checked every cited line range against the live file and all match exactly, including the downstream call-site citations (lineage.ex:133, agent.ex:109-110/185/195/259, bridge.ex:94) and the atomic-write precedent (overlay_publisher.ex:25,99-100, which does exactly the .tmp-then-rename pattern claimed). Finding 3 (unnamed 30-day mechanism) is resolved with a precise, falsifiable choice: a hardcoded @legacy_cutover_date Date.t() module attribute compared against Date.utc_today() (overridable via opts[:now] for tests) -- and the "hardcoded constant, not a config key" reasoning is grounded in a verified fact (mix.exs:35-37 really does have deps: []). The archived-corpus re-scope is a model of the honesty rail this review enforces: it re-checked runs/colony_archive/ (confirmed absent) and runs/colony/*.bin (confirmed absent), dropped the unfalsifiable "178 files" figure instead of inventing a check against it, and correctly demoted that clause to an operator checklist item pending a real inventoried directory. The empirical 0x83 ERTS-tag claim checks out byte-for-byte against the cited fixture. On my specific brief -- claim-fence discipline and felt-state framing -- this spec is clean: zero occurrences of feel/emotion/experience/aware/conscious/sentient/suffer/felt anywhere in the document, and the three Logger.warning strings it introduces ("version mismatch", "newer minor", "legacy window ... closed") are plain technical language with no output surface, internal or public, capable of carrying a felt-state claim. The spec is also correctly scoped as pure I/O/serialization plumbing -- it touches no FE quantity (F, G, C, precision, learning update) and doesn't need to name a generative model because it genuinely doesn't have one in play; that scoping discipline is itself a good sign. The one new issue I found (a wrong test-precedent citation in the not-yet-written Test coverage section) is a documentation-accuracy slip, not a Contract-level defect -- the actual header/decode logic is internally consistent (byte math checks out: 4+2+2+8=16), so it doesn't block the Contract, but it should be fixed before the paired RED test is authored so the implementer isn't pointed at the wrong precedent.
• Concerns:
  • production/docs/SPEC_mc_codec_versioning.md, Test coverage section (line ~229-230): the citation "falls back to a fresh brain exactly like the corrupt-file case already covered in mc_test.exs:158-174" mischaracterizes its own precedent. I re-read test/sp/brain/mc_test.exs:158-174 directly: that test writes a fully-valid, fully-decodable term_to_binary payload with an OLD STRUCT SHAPE (missing :struct_pressure/:struct_steps keys) -- binary_to_term succeeds; the fallback is driven by MC.reconcile/2's compatible?/2 check returning false, which yields Genome.express(dna) for the MODEL while the original dna (file_dna) is preserved. That is a different code path from what this SPEC's new safe_read/2 exercises for "an unparseable payload (e.g. truncated)": decode_payload/1's rescue/catch catches a binary_to_term crash, returns :error, and load/2 falls all the way through to new(opts) -- which discards BOTH the model AND (absent opts[:dna]) the dna, defaulting to Genome.default(). No test in the current suite actually writes genuinely-corrupt/truncated bytes and exercises the rescue/catch in safe_read. The precedent named for the new mc_versioning_test.exs is the wrong test; the implementer should either write a real truncated-payload test from scratch or correct the citation so it doesn't assert a false equivalence.

systems-architect (docs/lab_team/03_systems_architect.md)

• Verdict: SIGN_WITH_CHANGES
• Rationale: Fresh, independent re-verification against the live repo confirms all three prior BLOCKING findings are genuinely resolved:

(1) Wrong-module finding resolved. Read lib/sp/brain/mc_codec.ex directly: module is SP.Brain.MCCodec (no dot), encode/2 (senses, dna) only, no decode/1, no header/magic/version logic anywhere in the file -- exactly as the spec's correction note states. lib/sp/brain/codec.ex is confirmed a separate, unrelated SP.Brain.Codec (opaque-channel binning/EFE-summation module).

(2) Retarget to SP.Brain.MC resolved and line-accurate. Read lib/sp/brain/mc.ex end to end: save/2 is lines 552-554 verbatim (File.write! of :erlang.term_to_binary({dna, model}), no header today); load/2 is 563-585; private safe_read/1 is 587-594 -- all three ranges match the spec's citations exactly. reconcile/2, compatible?/2, adopt/2 do live in the cited 596-643 span (guarding decoded-struct shape, unaffected by this SPEC). The alias line the spec says to extend is mc.ex:12, confirmed verbatim. Call sites (agent.ex:110,185,195,259; bridge.ex:94) all go through the public MC.load/MC.save API only -- grepped every MC.load/MC.save/safe_read call in the repo (agent.ex, bridge.ex, diagnose.ex, runs/probe_plateau.exs, runs/pureworld_qa.exs, runs/nursery_train.exs) and none reach into the private safe_read or assume the raw on-disk shape, so the new header format is fully encapsulated behind the module boundary (Markov-blanket discipline intact) and every existing call site is untouched, as claimed.

(3) Migration-window mechanism resolved and precisely named: a hardcoded @legacy_cutover_date Date.t() module attribute on SP.Brain.MC, compared via Date.compare/2 against now (Date.utc_today() by default, opts[:now]-overridable for tests) at legacy-decode time. Verified this is the right idiom for this codebase, not an invented one: Glob/find for config.exs across the repo turns up exactly one file, ui/config/config.exs (the separate Phoenix app), and the root config/ directory holds only JSON scenario/seed data, no config.exs -- confirming mix.exs:35-37's deps: [] (verified verbatim) and the spec's claim that a hardcoded reviewable constant, not a new config-loading mechanism, is consistent with the codebase.

Supporting empirical claims also checked directly: test/fixtures/decider_golden_seed7_d5b3.bin's first byte is 0x83 (hex-dumped it myself) confirming the magic-collision-impossible argument; runs/colony_archive/ and runs/colony/ do not exist (ls confirmed), so the archived-corpus re-scope to an operator checklist item is honest, not hand-waved; lib/sp/show/overlay_publisher.ex:25 (@tmp) and :99-100 (File.write! then File.rename!) match the cited atomic-migration precedent exactly; extra_applications: [:logger] is already present in mix.exs so require Logger + Logger.warning/2 will actually function at runtime, and grep confirms no lib/ file uses Logger yet, matching the spec's claim precisely. The three cross-referenced docs (RUNBOOK_DR.md, SPEC_lineage_snapshot.md, SPEC_livepatch_hot_files.md) all exist.

On my specific brief (module/function existence, and registration mechanics): every function the Contract names now exists exactly where and as cited, or is correctly scoped as new work (test/sp/brain/mc_versioning_test.exs does not yet exist, confirmed, and is honestly labeled 'new file'). There is no help.py/boot-time-registration analogue applicable to this Elixir codebase for this change -- no new module, no new supervised process, no new genome organ requiring wiring into a boot path; the only 'registration'-shaped dependency (Logger being available as an OTP application) was already satisfied before this SPEC and I verified it directly rather than assuming it. This SPEC is correctly filed in production/docs/ (alongside its sibling SPEC_livepatch_hot_files.md) rather than docs/specs/ (which holds only typed FE-term specs: novelty, metabolism, generative_model, etc.) -- confirming the spec's own scoping judgment that this is FE-adjacent infrastructure, not a new FE term, so the persona checklist's FE-term-specific requirements (opt-in genome organ gating, typed StateSpace/Observation/... record, byte-identity-of-decision-path gate) are correctly not invoked here; I confirmed independently that the decider byte-identity golden fixture (test/sp/brain/decider_byte_identity_test.exs) writes/reads its own raw term_to_binary directly and never goes through MC.save/load, so this SPEC cannot regress that gate.

The one new issue I found (test-coverage citation) is a documentation-accuracy nit in the Test-coverage section, not an architectural defect: the module/function targeting, the header format, the fail-open contract preservation, and the migration mechanism are all sound and now grounded in the real file. It is fixable with a one-line citation correction or a genuinely-new test case when mc_versioning_test.exs is authored -- it does not require reworking the Contract, so SIGN_WITH_CHANGES rather than REVISE.

• Concerns:
  • production/docs/SPEC_mc_codec_versioning.md Test-coverage section, bullet 3 (~line 229-230): the claim 'falls back to a fresh brain exactly like the corrupt-file case already covered in mc_test.exs:158-174' mischaracterizes that existing test. I read test/sp/brain/mc_test.exs:158-174 directly ('loading a STALE memory file (older shape) starts fresh instead of crashing') -- it writes a VALID :erlang.term_to_binary payload with a different (older) struct SHAPE; binary_to_term succeeds and the reconcile/compatible?/adopt path (mc.ex:596-643) discards it, NOT the rescue/:error fail-open branch the new safe_read/decode_payload exercises for a truncated/unparseable payload. I grepped the whole file for corrupt|garbage|unreadable|:error and found no existing test that ever hands binary_to_term bytes that actually fail to parse. There is currently no precedent test for the case the spec cites -- fix the citation (or better, have the new test/sp/brain/mc_versioning_test.exs write a genuinely truncated/garbage payload after a valid MCV1 header, distinct from the shape-drift case) before landing the paired RED. This is exactly the kind of unearned citation the spec's own correction note says it exists to remove, so it should be corrected on the same discipline.

red-experimentalist

• Verdict: SIGN_WITH_CHANGES
• Rationale: Independent re-verification against the live repo confirms all three prior BLOCKING findings are genuinely resolved, not just asserted. I read lib/sp/brain/mc.ex end to end and lib/sp/brain/mc_codec.ex end to end: SP.Brain.MC.save/2 is exactly at mc.ex:552-554 (a bare File.write!(path, term_to_binary({dna, model}))), load/2 at 563-585, the private safe_read/1 at 587-594, and reconcile/compatible?/adopt at 596-643 -- every file:line citation in the corrected spec matches the live source exactly. SP.Brain.MCCodec (no dot) is confirmed to have only encode/2 (senses, dna) with no decode/1 and no header logic, as the correction note states; SP.Brain.Codec (lib/sp/brain/codec.ex) is confirmed to be the separate opaque-channel discretiser, also unrelated. The Contract section now correctly targets save/2 and load/2, preserves both functions' existing arity/return-type/fail-open contract, and the sketch code is internally consistent (safe_read/2's cond branches route into decode_payload correctly; load/2's now-threading is correctly wired). The removal mechanism (finding #3) is now precisely named: a hardcoded @legacy_cutover_date Date.t() module attribute compared via Date.compare against opts[:now] (defaulting to Date.utc_today()) -- and I verified the justification against mix.exs (deps: [] at exactly lines 35-37, confirming zero-dependency status and no config.exs at the root, as claimed). The empirical byte-0x83 claim about test/fixtures/decider_golden_seed7_d5b3.bin checks out (verified by reading the raw bytes: 0x83 is indeed the first byte). The overlay_publisher.ex tmp+rename citation (lines 25, 99-100) is exact. The runs/colony_archive/ non-existence and gitignore claims check out against the live tree and .gitignore. This is a well-executed correction pass -- the architecture is now sound and testable at the unit level. My remaining concerns are all in my specific lane (testability / fabrication-resistance / FALSIFIES discipline) and are real, concrete, and fixable without rearchitecting: the spec never closes the loop on its own self-declared Ship-gate promise of a named, pre-registered 'byte-identity regression test' with PASS+FALSIFIES, one test-coverage citation misrepresents existing precedent, and one plausible fabrication path (legacy_read/3 migrating on decode failure) has no named test to catch it. None of these require retargeting the spec again -- they require naming a FALSIFIES condition, adding one test, and fixing one citation.
• Concerns:
  • No FALSIFIES condition is named anywhere in SPEC_mc_codec_versioning.md despite the spec's own line 4 declaring 'Ship gate: MERGED VERDICT + paired RED (a byte-identity regression test)' -- a promise this repo's binding discipline requires to carry a named PASS + FALSIFIES pair, pre-registered in a doc the run links to (CLAUDE.md 'Heavy science-gate discipline' #4: 'Every cure registers its gates (named PASS condition + FALSIFIES condition) in the docs before the run'; persona Required Check #5). I searched docs/*_RED_TEST.md, docs/receipts/, and production/docs/ -- no pre-registration doc exists for this spec's promised 'byte-identity regression test'.
  • It is unverifiable which test is meant to satisfy the Ship-gate's 'byte-identity regression test' clause. I read test/sp/brain/decider_byte_identity_test.exs (this repo's one existing byte-identity golden, cited in CLAUDE.md Hard Invariant #2) line-by-line: it does its own raw :erlang.term_to_binary/binary_to_term round-trip directly in the test (lines 33-38) and never calls SP.Brain.MC.save/2 or load/2 at all -- so it cannot be the promised 'paired RED (byte-identity regression test)' for THIS change, since it doesn't exercise the header/version code path being added. None of the 5 bullets in 'Test coverage the plan owes' (SPEC lines 213-237) is framed as a byte-identity regression with a stated FALSIFIES condition either -- they are ordinary unit-test descriptions of expected behavior, useful but not the pre-registered gate the Ship-gate line promises.
  • The Test coverage bullet for 'unparseable payload (truncated)' (SPEC line ~229) cites test/sp/brain/mc_test.exs:158-174 as 'the corrupt-file case already covered'. I read that test directly: it is titled 'loading a STALE memory file (older shape) starts fresh instead of crashing' and exercises the reconcile/compatible? path on a term that DOES successfully binary_to_term-decode (just with an older struct shape) -- it never hits safe_read's rescue/catch branch (a genuine binary_to_term failure on unparseable bytes). A repo-wide grep of test/ for corrupt/garbage/truncat/invalid-bin found zero existing tests that feed genuinely-unparseable bytes into MC.load. This is a mischaracterized precedent, not a blocking defect in the new test itself (which, as literally described, would exercise the right code path if written) -- but it should be corrected so an implementer doesn't rely on non-existent prior coverage.
  • None of the 5 named tests exercises a genuinely-corrupt LEGACY (non-'MCV1'-magic) file loaded pre-cutover -- the path through the spec's own legacy_read/3 where decode_payload(raw) itself fails (returns :error without ever calling migrate!). Only the MCV1-header-with-corrupt-payload case and the two valid-legacy-file (pre/post cutover) cases are named. Without a test forcing genuinely-garbage legacy bytes through this specific branch, a subtly wrong legacy_read/3 implementation (e.g. a pattern-match slip that calls migrate! regardless of decode outcome, silently writing a 'successfully migrated' MCV1 header on top of unrecoverable garbage) would pass every test this SPEC names -- exactly the fabricated-but-schema-valid failure mode this review is checking for.

embodiment-designer

• Verdict: SIGN_WITH_CHANGES
• Rationale: All three PRIOR BLOCKING findings are genuinely resolved, independently re-verified against live source, not just taken on the spec's word:\n\n1. Wrong-module finding -- CONFIRMED FIXED. Read lib/sp/brain/mc_codec.ex myself: it defines SP.Brain.MCCodec (no dot), encode/2 (senses, dna) is a real-time sense-to-observation discretiser, no decode/1, no header/magic/version logic anywhere. The spec's correction note matches the live file exactly.\n\n2. Wrong-target finding -- CONFIRMED FIXED. Read lib/sp/brain/mc.ex myself and checked every file:line citation the corrected spec makes: save/2 (552-554), load/2 (563-585), safe_read/1 (587-594), reconcile/compatible?/adopt (596-643) all line up exactly with the live source. The Contract section is now correctly built entirely around SP.Brain.MC.save/2 / load/2. Also verified lib/sp/brain/codec.ex (a THIRD, genuinely unrelated module, SP.Brain.Codec) exists and is unrelated, as the spec claims. Verified mix.exs:35-37 (deps: []) and agent.ex:109-110,185,195,259 / bridge.ex:94 call-site citations -- all accurate.\n\n3. Unnamed-mechanism finding -- CONFIRMED FIXED. The spec now names an exact, precise mechanism: @legacy_cutover_date as a hardcoded Date.t() module attribute on SP.Brain.MC, compared against load/2's now (real wall-clock by default, injectable via opts for tests). Confirmed via mix.exs that the root app has zero deps (no config.exs), justifying a hardcoded constant over a config key. Also independently confirmed the spec's re-scoping of the '178-file archived corpus' claim: runs/colony_archive/ does not exist and no runs/colony/*.bin corpus exists yet (both gitignored, runtime-only paths) -- the spec's decision to drop this as code-enforced logic and push it to an operator checklist is honest and matches live repo state exactly, not invented.\n\nAs embodiment-designer, my assigned lens for this pass was exposure/redaction: is there a field-level allowlist for anything crossing to a public surface, and any path/host-layout leak. This SPEC itself never crosses the public broadcast/MCP surface -- it's pure internal persistence for runs/colony/kin-<n>.bin, and I traced the actual public-surface allowlist pattern in this repo (production/mcp/SPEC_uni_public_mcp.md's get_show_state() explicit 'Does NOT expose' list, and SPEC_uni_self_audit.md's field allowlist) to confirm this SPEC doesn't touch it. But the SPEC does introduce the codebase's first-ever Logger calls on the exact file family a sibling spec already flagged as needing path relativization, and doesn't apply that same discipline -- a real, concrete, easily-fixed gap, not a fundamental defect. Combined with one inaccurate test citation, this lands at SIGN_WITH_CHANGES: the three prior BLOCKING findings are solidly resolved and the Contract itself is sound, but the two named items should be fixed before merge.
• Concerns:
  • Path/host-layout leak risk (new): the corrected spec's safe_read/2 and legacy_read/3 for SP.Brain.MC introduce THREE new Logger.warning calls that interpolate the raw, absolute path argument with zero relativization. I grep-confirmed lib/ has ZERO prior Logger usage anywhere -- this spec is genuinely the first to introduce it, exactly as it claims. But the value it logs is not innocuous: path traces to Path.join(@repo_root, "runs/colony/kin-#{state.kin}.bin") in lib/sp/runtime/lineage.ex:25,133, where @repo_root = Path.expand("../../..", __DIR__) -- an absolute host filesystem path. This is the SAME file family the sibling spec production/mcp/SPEC_uni_self_audit.md (which itself reads SP.Brain.MC.save/2, mc.ex:552-554 -- the very function this SPEC modifies) treats as sensitive enough to require an explicit fix: it relativizes memory_bin_path and states as a binding Non-goal 'Does NOT return an absolute host filesystem path under any field, ever.' The mc_codec_versioning spec's new logs don't cross the public MCP boundary today (Logger output only reaches journald, an operator/approval-gated surface), so this isn't a proven live leak, but it repeats on every version-mismatch/legacy-window-closed event across every kin in the colony, and it's inconsistent with the established 'never log/return an absolute host path for this file family' convention the codebase already adopted next door. Fix before merge: relativize the interpolated path in all three Logger.warning calls (e.g. Path.relative_to(path, @repo_root) or Path.basename(path)).
  • Test-coverage citation is inaccurate and understates what's actually new: the spec's Test-coverage section claims an 'unparseable payload (e.g. truncated)' after a valid MCV1 header 'falls back to a fresh brain exactly like the corrupt-file case already covered in mc_test.exs:158-174.' I independently read test/sp/brain/mc_test.exs:158-174 ('loading a STALE memory file (older shape) starts fresh instead of crashing') and it exercises a DIFFERENT code path entirely: a successful binary_to_term decode followed by reconcile/2/compatible?/2 rejecting the STRUCT SHAPE and returning Genome.express(dna) -- it never touches load/2's :error -> new(opts) branch. I grep'd all of test/ for MC.load-adjacent corruption tests and found none that exercise a genuinely unparseable/garbage-bytes decode hitting safe_read's rescue/catch -> :error -> new(opts). The spec's underlying behavioral claim is still correct given the code shown (decode_payload's rescue/catch does route to :error -> new(opts)), but the cited precedent doesn't exist -- the paired-RED implementer should not treat this as 'already covered elsewhere,' it is net-new coverage with no prior pattern to copy.

MERGED VERDICT

SIGN_WITH_CHANGES

(Merge rule: the worst verdict wins.)

Honesty note

This is a v2 receipt following a spec-correction pass driven by the v1 review's own findings. This is the first formal review this spec has received. No FE source code (.ex/.exs/.py/.cjs) was written or modified by either the fix pass or this review -- only spec/schema/policy documents. If this verdict is SIGN or SIGN_WITH_CHANGES, implementation may proceed through this repo's normal ship gate; if REVISE or REJECT, further spec work is needed before any code diff.

Addendum — named changes applied (pass 3)

This is a direct-apply pass, not a fresh persona re-review. No independent verification round was run here; this pass took the five personas' verdicts above (all SIGN_WITH_CHANGES, merged verdict SIGN_WITH_CHANGES) as already-final and went straight to applying the named "Concerns:" bullets to production/docs/SPEC_mc_codec_versioning.md. Every file:line citation touched below was re-verified against the live repo before editing (not trusted from the receipt's prose), since code may have shifted since the v2 review ran. No .ex/.exs/.py/.cjs file was touched — the target spec document does not yet correspond to any live implementation (the header/version/migration Contract it describes is all-new proposed code; lib/sp/brain/mc.ex's save/2/load/2 still have no header logic today), so every concern below was resolvable as a spec-document fix. None required editing real, already-running source code.

1. [math-breaker] Purpose-section citation incomplete provenance (only 4 of the real MC.save/MC.load touch-points named; SP.Brain.Bridge's independent call sites omitted entirely) — FIXED: Purpose section now enumerates all 14 real call sites (re-verified live via grep: agent.ex load :110, save :185,195,251,259,310,318,358 = 8; bridge.ex load :94, save :141,149,180,185,200 = 6), explicitly scoped as "SP.Runtime.Agent — the primary runtime path" vs. "SP.Brain.Bridge — a parallel live-body module on the same API."

2. [math-breaker / aif-theorist / systems-architect / red-experimentalist / embodiment-designer — same defect, flagged independently by all five personas] Test-coverage section falsely cites mc_test.exs:158-174 as already covering the unparseable/truncated-payload case — FIXED: I re-read test/sp/brain/mc_test.exs:158-174 directly and confirmed the persona finding: that test ("loading a STALE memory file... starts fresh") writes a fully well-formed, successfully-decoding payload and exercises reconcile/2/compatible?/2 (struct-shape drift), never decode_payload/1's rescue/catch path. The false "already covered" citation is removed; the bullet now states plainly this is net-new coverage with no prior precedent, and names the exact code path (safe_read → :error → new(opts)) it must exercise.

3. [math-breaker] Bind-mount-contract section claims the migration path is "guaranteed" by a bind-mount "documented in production/docs/RUNBOOK_DR.md extension" — that document contains zero such mentions — FIXED: re-verified live (RUNBOOK_DR.md read in full, grep for colony-memory/bind-mount/C-C4b = zero hits; repo-wide grep for C-C4b = only this spec + a bare comment in production/scripts/colony_archive.sh:7). Rewrote the section to drop "guaranteed"/"documented" and state honestly: Status: PENDING, with a named falsifier (verify the bind-mount live on the target host via findmnt or equivalent before trusting the migration path across a git archive unpack), and note that C-C4b needs its own SPEC or a real RUNBOOK_DR.md section before it can be cited as documented again.

4. [red-experimentalist] No FALSIFIES condition is named anywhere despite the spec's own Ship-gate line promising a pre-registered PASS + FALSIFIES pair for a "byte-identity regression test" — FIXED: added a new "Ship-gate byte-identity regression test" subsection under Test coverage with an explicit PASS condition (exact 16-byte header + revived.model == brain.model byte-for-byte round-trip) and FALSIFIES condition (either the header bytes drift or the round-trip isn't exact), pre-registered in the spec ahead of the RED test being authored.

5. [red-experimentalist] Unclear/wrong which test satisfies the Ship-gate's byte-identity-regression clause — decider_byte_identity_test.exs doesn't call MC.save/load at all — FIXED: re-read test/sp/brain/decider_byte_identity_test.exs:33-38 and confirmed it does its own raw Plan.action_values/3 round-trip, unrelated to MC.save/load. The Ship-gate line (line 4) and the new Test-coverage subsection now say explicitly that decider_byte_identity_test.exs is NOT the promised test, and name the actual first bullet of mc_versioning_test.exs as the one that is.

6. [red-experimentalist] None of the named tests exercises a genuinely-corrupt LEGACY (non-"MCV1"-magic) file loaded pre-cutover, where legacy_read/3's decode_payload(raw) itself fails without ever calling migrate!/3 — FIXED: added a sixth Test-coverage bullet naming this exact case (corrupt legacy bytes, pre-cutover now, must hit :error without calling migrate!/3, must leave the on-disk file byte-for-byte unmodified) and named the fabrication pattern it guards against (a pattern-match slip that calls migrate!/3 regardless of decode_payload's outcome).

7. [embodiment-designer] New Logger.warning calls interpolate the raw absolute path, with zero relativization, inconsistent with the sibling SPEC_uni_self_audit.md's established never-log-an-absolute-host-path convention for this exact file family — FIXED: re-verified production/mcp/SPEC_uni_self_audit.md:184,260 still states and uses that convention (Path.relative_to(abs_path, @repo_root)), and that SP.Brain.MC has no @repo_root of its own (that attribute lives on SP.Runtime.Lineage, lineage.ex:25). Added an explicit path-relativization requirement to the spec and changed all three Logger.warning call sites in the Contract's code sketch to interpolate Path.basename(path) instead of raw path.

Summary: all 7 distinct concerns raised across the 11 "Concerns:" bullets from the five personas (3 were the same test-coverage mis-citation reported independently by five personas total) were resolved directly in production/docs/SPEC_mc_codec_versioning.md. Zero concerns required a "pre-existing code gap, out of scope" note — every one was a defect in the SPEC document itself (a wrong citation, a missing FALSIFIES pair, an uncorroborated safety claim, a missing test bullet, or a path-hygiene gap in proposed-but-not-yet-written code), not a bug in already-live, already-running source. No .ex/.exs/.py/.cjs file was touched by this pass. All file:line citations used above were re-verified against the live repo at the time of this pass, not copied from the review's prose uninspected.