feat(metrics): ST1 — loop metrics + adversarially-hardened FCR/RP baseline - #16
Merged
Merged
Conversation
…and-reject (ST1 AC2-AC5)
…ple record to .loop/repair (ST1 AC1)
Adversarial review of the ST1 metrics slice found the honesty gates were satisfiable without real evidence. Harden compute_metrics/build_baseline: - evidence_backed no longer credits a RUNLOG prose mention or a bare TASKS verify declaration. Only (a) a structurally-valid held-out verdict artifact (per-check visible/holdout arrays + flags re-derivable from them — a 4-field stub is rejected) or (b) a NON-COMMENT gate line in a verify-* script whose gate script file exists. Records the verdict sha256 in provenance and states it is evidence, not proof (tamper detection is the anti-cheat layer's job). - FCR cross-join is fail-closed: a success claim is clean only if every attached verify bundle is green, EXCEPT a red bundle whose own task later reached green (an honestly-repaired intermediate). An unrelated green can no longer launder a claimed task's still-red gate. - Unrecognized ### Outcome tokens are surfaced under provenance.unrecognized_ outcomes instead of silently escaping the FCR denominator. - RP records are anchored: before/after scores must be corroborated by real verify-bundle scores (fabricated deltas -> rejected); no bundle to anchor -> flagged unanchored. - --baseline additionally refuses when the two FCR methods disagree, when no iteration claims success (vacuous 0/0), or when a counted RP record is unanchored; each refusal names the failed precondition. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
doctor validated repair records and receipt/rollout ledgers when present but reported only the 4 core schema ids, under-counting its own coverage. Return the record schema keys actually checked and append their schema ids (repair, rollout, receipt) to schemas_checked, in deterministic order. A loop with no record files is unchanged (still the 4 core ids). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
summarize() returned its productive fraction under the key repair_productivity, the exact mislabel the slice de-branding closes: the rollout ledger carries rollout-productivity, NOT the RP baseline (metrics.py derives RP from the canonical repair record). Rename the key and update callers and tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ne binding - Scaffolding (manifest template, repo-os-contract, prompt-templates) declared the stale .loop/artifacts/repair-record.json output path the metrics tool cannot read; point them at the canonical per-iteration repair path. - README Measured-baseline passage now states the committed verdict is evidence, not proof, and cites its sha256. - Add a test binding the README FCR/RP literals to docs/metrics-baseline.json so they cannot silently drift, plus a guard against the stale scaffolding path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Recompute docs/metrics-baseline.json with the hardened metrics command. Still evidence_backed via the real committed holdout verdict, FCR 0.0 / RP 1.0; adds the new provenance fields (holdout sha256, unanchored_records, unrecognized_ outcomes). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… anchoring (round-2 adversarial findings) A completion-class claim (task_passed/succeeded/terminal) is clean only if every attached bundle is green — no cross-iteration escape; the repaired-intermediate exception is progress-class (advanced) only and order-aware. --baseline accepts nothing weaker than a structurally-valid gate verdict artifact; the gate-script existence check no longer leaks to this repo's own toolkit. RP anchors to a same-task red->green bundle pair (order enforced when known), not global score membership. Short outcome tokens are surfaced. Every round-2 exploit (G0-G5, F, H1/H2, N2/N2b, N3) is pinned as a regression test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
The repo's live .loop run-state is gitignored, so CI has none — the live-contract gate skips there and stays a local/operator check. The foreign-cwd CLI test sets PYTHONPATH for module visibility (CI has no editable install); it tests cwd-independent scripts/ resolution, not packaging. Verified against a git-archive fresh-checkout simulation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
SollanSystems
added a commit
that referenced
this pull request
Jul 25, 2026
…0.0, slice 1/5) (#82) * feat(chain): canonical-JSON event hashing (pure stdlib leaf module) * feat(chain): incremental link verification + pure verify_chain over exported streams * feat(events): store generations — chain columns + user_version, shared feature-detected row reader with typed decode errors Task 3: - fresh-store DDL gains `prev_event_hash TEXT` + `event_hash TEXT NOT NULL` (design change D1: a pre-0.10.0 10-column INSERT now fails closed). The DDL is CREATE TABLE IF NOT EXISTS, so legacy tables are untouched — _connect() never upgrades a legacy store. - fresh stores get `PRAGMA user_version = 2`, set only when this connect actually created the events table (probed via sqlite_master beforehand). - EventRowDecodeError(ValueError) for an undecodable payload/artifact_hashes. - has_chain_columns(), store_user_version(), _BASE_COLUMNS, read_event_rows() — the single feature-detecting row projection shared by store, runtime and runner reads. Records always carry prev_event_hash/event_hash keys; legacy rows project None. It owns the JSON-decode translation each call site used to repeat. - read() rewritten onto read_event_rows(). - scripts/chain_fixtures.py: shared byte-faithful v0.9.0 legacy-store builder (test-only; the wheel force-includes scripts individually, so it does not ship). Folded in from Task 4 Step 3 — moved from Task 4 per controller end-green adjudication, and nothing else from Task 4: - append() computes prev_event_hash/event_hash via loop.chain on chained stores and writes a 12-column INSERT; legacy stores keep the v0.9.0 10-column INSERT and project both fields as None. - an unhashable record (ChainHashError) rolls back and raises EventValidationError. Why folded: `event_hash NOT NULL` is *defined* as the constraint that rejects a 10-column writer, and until Task 4 append() was itself a 10-column writer — so the DDL cannot land before the chained writer without breaking the plan's end-green invariant. Measured: the 2 DDL lines alone took the suite from 951 passed to 828 passed / 123 failed across 11 files. Still Task 4: EventStoreOperationalError, the sqlite3.OperationalError wrap, runcontrol/runner wiring, and all of Task 4's tests. Reviewed edits: NONE required. No existing test pinned the returned/read record key-set or the PRAGMA table_info column list, so the two new record keys and two new columns broke nothing. Gates: focused (test_event_chain + test_eventstore) green; full suite 955 passed / 14 skipped (was 951/14 — exactly the 4 new tests, no regressions); pyyaml-only fallback 886 passed / 83 skipped (skip count unchanged, so the new tests are unconditional in both validation modes). * feat(events): store-computed hash chain on append; typed operational error with a handled CLI path The chain computation inside append() itself landed in 4d96499 (Task 3) per controller adjudication; this commit adds its behavioral tests plus the typed error surface that was still missing. - EventStoreOperationalError(RuntimeError): the events table exists but cannot service the operation (schema drift, lock). append() wraps its transaction body so a drifted or locked store never leaks a raw sqlite3.OperationalError. - Handled CLI path: runcontrol._append_event and a new runner._store_append translate it into RuntimeStoreError("event_store_unusable", ...), which __main__ already catches for run/pause/resume/approve/cancel -> exit 2 with no traceback. - Tests pin store-computed chaining on fresh stores, caller-supplied chain fields being ignored, legacy stores staying unchained, the NOT NULL refusal of a pre-0.10.0 ten-column INSERT, both translation sites, and a subprocess probe that `loop run` / `loop pause` on a drifted store exit 2 traceback-free. * feat(schema): optional event@1 chain fields with structural-fallback parity * feat(migrate): explicit legacy-store chain migration verb loop/migrate.py + `loop migrate <workspace>`: the only path that upgrades a v0.9.0 legacy events.db to the chained v2 shape. Connect still never upgrades. Migration widens the table (nullable prev_event_hash/event_hash) and stamps user_version=2; it never rewrites rows — backfilling hashes is deliberately impossible because the append-only triggers forbid UPDATE. Pre-migration rows stay an unchained prefix and the first post-migration append is a chain genesis. Typed fail-loud: missing store -> RuntimeStoreError("missing_store"), corrupt store -> RuntimeStoreError("corrupt_store"); the CLI prints `migrate: <code>: ...` to stderr and exits 2 with no traceback. Reviewed edits (outside the brief's create-only file list): - scripts/test_loop_cli.py: added "migrate" to the command tuple in test_help_lists_every_command_with_a_description. That test is the only help/usage-parity assertion enumerating commands; leaving it unchanged would let the new verb drift out of the documented surface unchecked. Required by the task brief (Step 5) to land in this same commit. * feat(reducer): enforce hash chain at fold time via typed ChainBreakError chain.link_issue() runs per event inside _reduce_one, immediately after the non-monotonic-sequence check — the single chokepoint every folding verb (status/replay/run/simulate/doctor) passes through. A violation raises ChainBreakError(EventReplayError); the projection gains chain_head and unchained_prefix so verify_chain and the reducer report the same two numbers. Legacy/hand-built streams carry no event_hash, so link_issue returns None and they fold unchanged: verified by running, not assumed (test_reducer.py, test_adversarial_kernel.py 300-example property walks, test_adversarial_process.py all green). The raw-byte SQLite boundary pin is untouched — it asserts SQLiteEventStore.read() does not detect the tamper, which stays true; the chain adds detection one layer up, at the fold. No reviewed edits: zero existing tests required modification. Full suite +7 passed in both modes, zero regressions (extras 981/14 -> 988/14; pyyaml-only 911/84 -> 918/84). * feat(runtime): chain surfaced through status/replay/doctor/run; enforce event validation; race-safe immutable reads leave no sidecars Shared read path: new loop/runtime._read_only_connect (immutable=1 when no -wal sidecar exists, plain mode=ro otherwise) plus _read_store, which retries a failed read once as plain mode=ro so a lost race with a live append cannot mint a false corrupt_store (design change D4). Used by _read_events_readonly, _discover_run_id and runner._projection; read verbs now leave no -wal/-shm residue on a clean store (the PR #77 H4b finding). Row projection delegates to events.read_event_rows; EventRowDecodeError maps to RuntimeStoreError('corrupt_store', ...) at both call sites (design change D5), preserving the no-traceback invariant for in-row JSON corruption. runtime._events now enforces the per-event validation verdict it used to discard: a failing event raises RuntimeStoreError('invalid_event', ...) and the empty stream raises 'empty_store', replacing the 'assert validation is not None' that evaporates under python -O (decision 9a). All four folding surfaces report one code for a broken chain: status_report and replay_report catch ChainBreakError before EventReplayError, and runner._projection maps it before its generic except ValueError so run/simulate/run-control no longer relabel it invalid_event_stream (design change D3). status/replay reports gain chain_head + unchained_prefix (read via .get so the degraded projection cannot KeyError); doctor nests {'chain': {'head', 'unchained_prefix'}} under event_store. Reviewed edits: - scripts/test_doctor_eventstore.py test_synced_happy_path_is_doctor_clean gains two assertions on the new event_store['chain'] block (deliberate per Task 8 Step 4; the absent-store byte-stability pin is untouched and still passes). - loop/runner.py drops the now-unused EVENT_SCHEMA_ID import and its hand-rolled immutable-URI comment, both superseded by read_event_rows/_read_only_connect. Tests: 998 passed / 14 skipped with [schemas,yaml] extras (baseline 988/14); 928 passed / 84 skipped pyyaml-only (baseline 918/84). Delta is exactly the 10 new tests in both lanes, no skip movement. * fix(runner): D4 retry parity on the run/simulate read path + typed decode-error regression lock Finding 1: runner._projection used _read_only_connect directly, so a first-query sqlite3.DatabaseError became corrupt_store with no plain mode=ro reopen — the exact false alarm D4 exists to prevent, and simulate is a read-only monitoring verb that can race a live appender. Its two queries are now one read_stream closure passed to the shared runtime._read_store, giving run/simulate/run-control identical two-stage semantics to status/replay/doctor. Real corruption still fails both attempts and still maps to corrupt_store; the empty_store/ambiguous_run_id raises inside the closure are not DatabaseError and so are not retried. Shares the helper rather than duplicating the retry (imports _read_store in place of _read_only_connect, which is now reached through it). Finding 2: the runner EventRowDecodeError -> corrupt_store clause had no coverage. Adds scripts/test_doctor_eventstore.py::test_run_on_in_row_json_corruption_reports_ corrupt_store (same drop-trigger + corrupt-payload fixture as the doctor-side test, driven through dispatch_once). Negative control run: with the clause deleted the test fails with a bare loop.events.EventRowDecodeError escaping dispatch_once, outside the typed RuntimeStoreError family. Also adds scripts/test_event_chain.py::test_runner_read_path_retries_plain_mode_ro_ before_declaring_corruption, which pins Finding 1 falsifiably: it counts the sqlite3.connect calls a dispatch over a schema-drifted store makes and asserts exactly two (immutable first, plain mode=ro second, both mode=ro). It was red before the fix (1 == 2). Tests: 1000 passed / 14 skipped extras (was 998/14); 930 passed / 84 skipped pyyaml-only (was 928/84). +2 in both lanes = the two new tests, no skip movement. * feat(doctor): --expect-chain-head anchor gate, downgrade cross-check, absent-store sidecar tripwire event_consistency_issues gains expect_chain_head. The anchor fails hard on all four ways a store can fail to prove the expected head: absent, unreadable, unchained/broken (chain_head degrades to None), or diverged. A tampered store therefore cannot pass an anchored doctor even though its chain block is byte-identical to an unchained one. Absent-store branch also trips on -wal/-shm residue left behind by a deleted events.db (missing_event_store). With no store, no sidecars and no anchor the event_store block stays exactly {"present": False}. Readable branch cross-checks the lazy downgrade: user_version >= 2 with the chain columns dropped (chain_columns_missing). Reviewed edits vs the brief: - _store_declares_chain_without_columns routes its PRAGMA probe through _read_store, not a bare _read_only_connect, so a lost immutable=1 race retries plainly instead of being misreported as corruption (the D4 hole); it runs inside the existing try so a genuinely unreadable store surfaces as the typed unreadable branch rather than escaping doctor_report. - shared _anchor_mismatch helper for the three anchor issue sites. - added test_expect_chain_head_on_tampered_store_fails_doctor pinning the broken-chain composition end to end. * test(chain): adversarial coverage + four pinned honest limitations (recompute, truncation, downgrade, legacy) Eight integration tests over doctor's chain predicate (issue codes + the event_store.chain block, never global ok — design change D6). Detected: single-row splice with local recompute (the successor still cites the original), payload reorder across two same-type rows, and a mid-stream event_hash strip (the unchained-after-chained-prefix branch, until now only unit-pinned in loop/chain.py's own tests). Pinned honest limitations — these assert the attack SUCCEEDS, and keep reference/repo-os-contract.md #16 honest; if one starts failing the kernel gained a property and #16 must be updated in the same commit: - full history rewrite with a genesis re-chain plus a forged state.json and terminal_state.json (a FailedBlocked run laundered into Succeeded leaves doctor globally ok with zero issues), - trailing receipt_appended truncation (state-neutral, chain stays valid), - column-drop downgrade combined with PRAGMA user_version = 0 (defeats the D2 cross-check; without the PRAGMA reset chain_columns_missing still fires), - tamper on a never-migrated store (no retroactive coverage). In every one, --expect-chain-head is the control that catches it. Also pins the fourth fail-hard anchor shape: an anchor over a never-chained store fails rather than skipping. Reviewed edit outside the new file: scripts/test_doctor_eventstore.py's test_chain_columns_dropped_but_version_2_fails_doctor gains the same sqlite_version_info < (3, 35) skipif as the two DROP COLUMN tests here — on a contributor machine with older SQLite it would ERROR rather than skip. Tests only; loop/ and schemas/ byte-unchanged. * test(chain): pin event-store cleanliness flags on the full-rewrite honest limitation test_full_rewrite_with_recompute_passes_without_anchor_pinned asserted only the ABSENCE of named issue codes, so a future kernel that caught the full rewrite under a NEW code would have left the pin green — and the pin's headline claim (a FailedBlocked run laundered into Succeeded leaves doctor's event-store layer reporting nothing at all) rested on an out-of-tree probe rather than the test. Adds three positive assertions on the unanchored report's own event_store block: state_json_agrees / deterministic / legal_sequence are each True. Those are event-store-scoped predicates emitted by runtime.event_consistency_issues, so any new event-store-layer detection flips them, while unrelated validate_contract noise cannot. Global report["ok"] is deliberately still not asserted (design change D6). Verified they have teeth: on the same workspace with the history rewritten but the chain left unrepaired, state_json_agrees and legal_sequence both read False. Assertion-only change to one existing test: 1019 passed / 14 skipped with extras, unchanged from the previous commit. * test(zero-writes): read verbs proven side-effect-free on both store generations, zero carve-out on clean stores * feat(action): record the chain head on every run and optionally enforce it as an anchor * docs(contract): normative chain canonicalization, conformance vectors, integrity boundary, anchor trust assumptions * fix(review): whole-branch fix wave — claims-accuracy tightening, test locks, dedup 1. test_event_chain.py: delete the duplicated drifted-store CLI test (test_migrate_cli.py keeps the copy); drop the now-unused subprocess/sys imports and the duplicate `from loop.runtime import RuntimeStoreError`. 2. test_documented_conformance_vectors: also assert each vector's canonical preimage string appears literally in the contract. Verified first that all three published Preimage lines already are canonical_json output — no doc line needed regenerating. 3. test_cli_rejects_malformed_anchor_value: capture stderr and assert the "must be a 64-character lowercase hex sha256" message, so the test fails on a tree where the flag is swallowed as a positional target. 4. test_legacy_store_tamper_is_undetectable_pinned: staging self-guard — read the payload back and assert the tamper landed before the non-detection assertions. 5. New test_unhashable_record_breaks_chain: a bare NaN token in a payload (json.loads parses it, canonical_json refuses it) must surface as event_chain_broken, not a propagated ChainHashError. Required payload fields are kept intact because validate_event runs before the fold. 6. loop/reducer.py: drop the "or has an illegal gap" overclaim from ChainBreakError — a mid-stream deletion is a sequence error and a truncated tail is caught only by an anchor. 7. repo-os-contract.md 16: rewrap the mid-word break so "hash-chain-consistent" renders intact. 8. repo-os-contract.md 16: replace the full-rewrite bullet's parenthetical with what the committed pin actually asserts (three projection-disagreement codes absent, three event_store cleanliness flags true) plus the rule that a future standalone event-store cross-check must be added to the pin. 9. repo-os-contract.md 22: document the absent-store residue shape {"present": false, "sidecar_residue": true}; extend the fails-doctor code enumeration with the chain codes and invalid_event; state that chain.head is null for a BROKEN chain too and that event_chain_broken is the discriminator; state that a tamper also violating event@1 surfaces as invalid_event because validation runs before the fold.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ST1 — "Metrics real" (spec:
docs/superpowers/specs/2026-06-30-st1-metrics-baseline.md). FCR and repair-productivity go from claims to derivations, and the derivation itself was adversarially red-teamed twice before this PR.loop metrics <dir>— derives FCR + RP from real on-disk evidence (RUNLOG, verify bundles, held-out verdict, repair records, receipts), never narration. FCR computed two ways, disagreement surfaced; unmatched success claims fail closed. Deterministic (byte-identical) scorecard with a fullprovenanceblock.loop metrics --baseline— writesdocs/metrics-baseline.json; refuses over any run without a structurally-valid gate verdict artifact, with rejected/unanchored records, disagreeing FCR methods, or zero success claims.loop-engineer/repair@1,loop-engineer/rollout@1) end the two-shapes ambiguity;recheck_productiverecomputes-and-rejects (wired intorollout_ledger.summarize, key renamedrollout_productivity);validate_contractchecks record files when present.[project.scripts] loopconsole entry (editable install).Adversarial review
Two red-team rounds (4 + 2 Opus reviewers) produced 17 confirmed findings — including a
--baselinethat laundered a holdout-flagged run and anevidence_backedsatisfiable by a prose gate mention. All fixed; every exploit pinned as a regression test. Documented residual: a fully-fabricated, internally-consistent verdict artifact defeats offline shape-checking by construction — tamper detection belongs to the anti-cheat layer.Test plan
doctor .loopokloop metrics examples/coverage-repairbyte-identical across runs; scorecard == committed baseline🤖 Generated with Claude Code