Skip to content

feat(evals): add offline skill recovery evaluation - #293

Merged
mdsmithaustin merged 27 commits into
mainfrom
codex/issue-287-skill-recovery
Oct 21, 2026
Merged

mdsmithaustin merged 27 commits into
mainfrom
codex/issue-287-skill-recovery

Conversation

@mdsmithaustin

@mdsmithaustin mdsmithaustin commented Oct 21, 2026 •

Copy link
Copy Markdown
Owner

Why

Saved Codex recordings need a reproducible way to check whether required skill guidance was recovered before the next governed action. This adds an offline evaluator that separates exact content, response packaging, transport, and ordering.

What changed

  • Add a standard-library Python CLI with hash-bound manifests, recovery-window assessment, JSON reports, and distinct exit codes for satisfied, failed, inconclusive, and non-applicable results.
  • Preserve mixed-read violations and distinguish repairs completed before an action from late or boundary-crossing reads.
  • Add sanitized failure projections, a synthetic repair control, CLI regression tests, and documentation for inputs, manifests, and interpretation.

Scope

Covers reviewed mode-entry and current-step recovery prerequisites in supported native Codex JSONL recordings. It does not change runtime skill loading or establish model obedience. Public fixtures are sanitized projections and synthetic controls, not new native trials.

Tradeoffs

Reviewers supply complete guidance requirements, boundaries, and action classifications. This makes assessments reproducible without model or service calls, but conclusions depend on those reviewed inputs and the supported recording formats.

Blast Radius

Touches only the new evaluator, fixtures, documentation, and tests. Existing runtime behavior is unchanged; the main risk is treating a scoped offline result as broader proof of recovery compliance.

Verification

Inspected the final branch diff. No validation commands were run in this PR-drafting phase. The added CLI suite covers fixture outcomes, malformed evidence, exact bytes and hashes, repair timing, chunk coverage, and multiple recovery windows; execution results belong to the separate validation phase.

✅ Low: This is an offline evaluator with no runtime callers. Each new rule (the two-level escaped-body scan over decoded and raw blocks, per-gap notice stripping, inconsistent_read_claims, split_read_claim) either raises a manifest error or downgrades a result to INCONCLUSIVE. The traces I followed (notice-only reread, escaped body in envelope metadata, mismatched beside exact, halves of one read, a body split across blocks) never produced a false SATISFIED, and the attempt decision table is unchanged.

Updates from git push no-mistakes

@mdsmithaustin
mdsmithaustin force-pushed the codex/issue-287-skill-recovery branch from 7434ccd to d47b97a Compare October 21, 2026 01:55
@mdsmithaustin

Copy link
Copy Markdown
Owner Author

Verdict: PASS+NOTES
Head: 23c1d9f
Patch-id: 8d5c4d62d51b7e2ce17c3d948bdeebf8d145364a
Docs: pass

Scope

  • PR 293, branch codex/issue-287-skill-recovery, closes Evaluate native enforcement of standalone recovery reads #287. The merge-base is main 5b99800, which is still main's tip, so the branch is up to date. The head did not change during the review. mergeStateStatus is CLEAN.
  • There are 9 commits since the r2 head d47b97a. Four are test commits (07362e2, 42e307d, 028c7d3, 7dd611e), four are fixes (c78d87e, 9321172, df8acfb, e6c53d0), and one is docs (23c1d9f). The r2 delta touches evaluate.py (+63/-26), README.md (18 lines) and the tests (+69). The fixtures are byte-identical to d47b97a.
  • Disposable clone: /private/tmp/claude-501/-Users-msmith1-HMV-System-mds-pstack/429cfdb7-6f12-40e3-9442-f09cfea07c89/scratchpad/review293c.oDxf/repo. It has a main worktree at ../main and a bisect worktree at ../redfirst.
  • My probes are in /private/tmp/claude-501/-Users-msmith1-HMV-System-mds-pstack/429cfdb7-6f12-40e3-9442-f09cfea07c89/scratchpad/probes293r3:
    • edges_r3.py and edges_r3b.py: synthetic CLI probes.
    • fuzz.py: differential fuzz against evaluate_d47b97a.py, with an independent false-SATISFIED oracle.
    • replay_mem.py: in-memory replay of real sessions. It patches _read_file and writes nothing.
    • escape_depth.py, body_forms*.py and noncanon.py: count-only censuses.
  • No session content went into any file I wrote. Each census prints counts only.
  • I read evaluate.py, README.md, the README diffs and the test diff since d47b97a in full.

Prior findings at head (every listed probe re-run)

  • r1 probes (probes293/probe*.py) match r2's fixed results:
    • P1 exits 2 (unsupported_output_carrier). P2 and P2b exit 2 (transport_notice).
    • P3, P3b, P3c, P3d, P3f, P3g x2 and P3h x2 exit 2 with undeclared_governing_body. P3e exits 1 (MIXED_FILES).
    • P4 and P4b exit 2. P5 and P5b exit 1, which README:68 documents. P6, P6b and P6c exit 2.
    • P7 exits 2 with json_nesting_too_deep. P7b and DEPTH 900..20000 exit 0, because they are unsupported blocks in incidental calls. SPLIT exits 1 under the operator rule.
  • r2 A, the notice-only reread after a valid read, is fixed by 9321172 (evaluate.py:146, 485-490). The unicode, ASCII-line and warning-suffix variants of A now exit 2, with STANDALONE packaging and UNVERIFIED transport. A0 exits 2 and A1 exits 0 SATISFIED_WITH_REPAIRS. A2 exits 2.
  • r2 B, escaped bodies in supported carriers, is fixed by c78d87e (evaluate.py:365-390, 418-443). All 8 B and B' shapes and B[string_text] exit 2 with undeclared_governing_body.
  • r2 C, an exact claim beside a mismatched claim, is fixed by df8acfb (evaluate.py:391-394). C1 exits 2 with inconsistent_read_claims. C0 exits 0 and C2 exits 2.
  • r2 D, adjacent claims of one read, is fixed by e6c53d0 (evaluate.py:395-398). E1 and E2 exit 2 with split_read_claim. E0 exits 0.
  • deep.py does not crash on either 3.14.8 or 3.12.14. At the probe's maximum depth on 3.14 the CLI now reports json_nesting_too_deep, which exits 2.
  • Delegate claims checked by re-running probes293c/edges.py:
    • a, c, d, i and j exit 2 with undeclared_governing_body.
    • b, the three-level escape, exits 0. This is the accepted limit.
    • e and f exit 1, because a non-notice trailing \n or a real extra block is proven extra content.
    • g and h exit 2.
  • Red-first: at 7dd611e all four new tests fail, with 4, 12, 3 and 2 failures. Each fix commit turns exactly its own test green, in the order c78d87e, 9321172, df8acfb, e6c53d0.

Adversarial hunt (new rules)

  • _region mislocation.
    • R1: an envelope with a metadata copy of the output placed before output. The region lands on the metadata copy, so the real output copy is undeclared and the probe exits 2. That is conservative.
    • A false SATISFIED would need a hit inside the region that is not mirrored in the decoded body. The region is a byte copy of the body's encoding, so any hit inside it is also inside the real encoding, which the decoded scan covers. I found no counterexample.
    • Real data: across 120 sessions, 0 decoded bodies lack their encoding in the raw text (probes293c/region.py).
    • In-memory replay of 1673 sessions, treating 2852 real single-body returns as whole entry reads:
      • 2839 are clean SATISFIED: 1194 command_result and 1645 text.
      • 13 are undeclared_governing_body on another row, and every one of those rows holds a real copy.
      • 0 errors land on the read's own row. 1 is INCONCLUSIVE because of a failed-script header.
      • The longest single evaluation took 0.93 s.
  • Notice regex swallowing real content. A governing file whose lines are Total output lines: 3, Warning: truncated output ... and ... 5 tokens truncated gives these results:
    • A whole exact read is SATISFIED (R2a), because claimed spans are never stripped.
    • Real notice-like extra lines in unclaimed output become INCONCLUSIVE, not FAIL (R2b, R2d). The notice adds transport_notice, which blocks credit, so swallowing can never yield credit or a clean SATISFIED.
    • A body that looks like a notice but does not start its stretch still fails as extra content (R9). Notices beside a mixed batch keep MIXED_FILES (R3).
    • The undeclared scan never strips notices.
  • matches set (evaluate.py:392-394). The set ranges over every claim on a decoded block in one call, across files. Claims on unsupported blocks are excluded and stay UNVERIFIED attempts (edges h).
    • The rule only raises exit 2, so it cannot create a FAIL or a SATISFIED.
    • Mutation M8, which drops the check, is killed by 3 failures.
  • Adjacency rule. Chunks across blocks are never flagged.
    • R4a: step chunks in two blocks under explicit_chunks give SATISFIED.
    • R4d: chunks in separate calls give SATISFIED. R4e: overlapping rereads give SATISFIED.
    • R4b: contiguous chunks in one block exit 2, and the merged claim (R4c) gives SATISFIED, as README:66 documents.
    • R8: adjacent claims with swapped sources are not flagged, and the probe correctly exits 1 because no claim covers the whole entry.
    • The operator rule still holds. A whole entry split across blocks exits 1 (SPLIT).
  • Differential fuzz. 60,000 random synthetic cases (seeds 1-5) mixed bodies, chunks, notices, whitespace, junk, and 1- or 2-level escapes across 8 carriers. Claims were 85% present and 5% mismatched.
    • Every head-versus-d47b97a transition is unchanged or moves toward INCONCLUSIVE: FAIL to INCONCLUSIVE, SATISFIED to INCONCLUSIVE, and SATISFIED_WITH_REPAIRS to INCONCLUSIVE.
    • 0 cases are newly SATISFIED and 0 are newly FAIL.
    • The oracle's false-SATISFIED hits fall into two classes only:
      • A whole entry plus claimed same-file fragments in one response. The decision table counts this as STANDALONE.
      • Bodies at three escape levels, the documented limit. Examples are an unsupported carrier with doubly encoded output (R6b exits 0) and a JSON array text block around a 2-level piece.
  • Mutations of the new rules (unit suite at head).
    • Killed: one escape level (M1), the region taken as the whole raw (M2), the region without the ASCII form (M3), dropping the Total output lines notice (M5), no whitespace strip in a notice stretch (M6), split ignoring source continuity (M7), and dropping the inconsistent check (M8).
    • Survived: M4, see note 4, and M9, which is equivalent and only turns exit 2 into exit 2.

Three-level limit: acceptable

The census covers all 1673 sessions in ~/.codex/sessions (2.0 GB, all dated 2026-10). They hold 126,715 output blocks, checked against 381 historical blob versions of skills/poteto-mode/**/*.md since 2026-08-15, 52 of them entry versions. Counts only:

  • 1036 blocks contain a three-level newline escape at all. None of them holds a governing body at JSON level 3 or 4 that lacks a level-0-2 form, so the escape-depth census finds 0 undetected bodies.
  • 1635 blocks hold a governing body in some form, with at least 90% of its long plain lines present.
    • 1560 (95.4%) are found by the evaluator's forms. 0 are at level 3 or 4.
    • 2 blocks in 2 sessions use a JS single-quote (util.inspect-style) escape, which is not JSON.
    • 73 are inexact: 53 match a blob at head and tail but differ in the middle, 10 match the head only, 11 the tail only, and 1 differs only by a missing final newline. Most likely these are versions not in git history, or truncated copies.
  • Non-canonical JSON escapes (Go HTML-safe </>/&, escaped \/, uppercase hex) appear in 0 blocks holding a governing body.

At zero observed frequency, and with the README stating the limit, the three-level miss is an acceptable accepted limit.

Notes (non-blocking)

  1. Doc precision: canonical escapes only. _encodings (evaluate.py:418-422) builds only Python json.dumps forms. A valid but non-canonical JSON escape hides a complete mixed batch: R5, Go HTML-safe escaping after a valid read, exits 0 SATISFIED. README:68 says "JSON-escaped up to two levels deep" without that qualifier. Real frequency is 0, as the census shows. Suggested wording: "JSON-escaped in canonical form (as json.dumps writes it, with or without \u escapes for non-ASCII)". Also add non-JSON escapes such as repr or util.inspect to the reviewer-owned list, since 2 real blocks use one.
  2. Doc precision. README:68 says "A JSON-escaped body cannot be claimed exactly, so it is always undeclared." That holds for decoded bodies only. Raw hits inside the body's own encoding, and claimed unsupported blocks, are exempt (evaluate.py:387-390), as the next two sentences say. Suggest "A JSON-escaped body in a decoded body ...".
  3. Doc precision. The notice definition at README:80 omits three regex details (evaluate.py:146, 486):
    • Matching is case-insensitive ((?i)).
    • It accepts [output truncated.
    • ^ anchors at the start of every unclaimed stretch, including mid-line in the body, as well as at line starts.
      The effect is always conservative (R2b, R2d).
  4. Test gap. Mutation M4 survives: claimed = any(...) without body.data is None. At head, a metadata copy beside a claimed output correctly exits 2 (edges i). No test pins that, because the envelope_metadata shape (tools/test_skill_recovery.py:477) claims nothing. A one-line subTest with a claimed output would close it.
  5. PR body at head.
    • True: Why, What changed and Scope. "Preserve mixed-read violations" holds within the documented scan limits.
    • The appendix's "Each new rule ... either raises a manifest error or downgrades a result to INCONCLUSIVE" is confirmed by the differential.
    • Stale: the attestation's pr running and ci pending. CI is green.
  6. Hygiene, outside the PR. /private/tmp/claude-501/-Users-msmith1-HMV-System-mds-pstack/429cfdb7-6f12-40e3-9442-f09cfea07c89/scratchpad/replays holds 173 entry.md files. /private/tmp/claude-501/-Users-msmith1-HMV-System-mds-pstack/429cfdb7-6f12-40e3-9442-f09cfea07c89/scratchpad/probes293c/replay.py:33 wrote them, and each holds a real decoded output body copied from ~/.codex/sessions. That breaks the no-copy rule. I neither read nor deleted them. The lead should remove that directory.

Observations under the unchanged rules, not reopened:

  • Claim granularity on same-file fragments decides the result. R7, with an unclaimed duplicate fragment, exits 1 EXTRA_CONTENT. R7b, with the same fragment claimed, exits 0.
  • In the unsatisfied branch, a mixed batch with a notice exits 2 (R3).
  • 8 of the 1673 sessions fail _trace: 7 with unsupported_response_item and 1 with duplicate_outer_identity. Any manifest on them exits 2.

Commands and outcomes

  • python3 -m unittest tools/test_skill_recovery.py: Ran 68, OK on 3.14.8 and on 3.12.14 (uv). The README:91 discover form also ran 68, OK.
  • Fixtures, all with empty stderr and all matching README:12-31:
    • repaired-entry: exit 0 SATISFIED_WITH_REPAIRS, applicable 1. attempts[0] entry has repaired_before_action true and the step attempt has it false.
    • mixed-full: exit 1 FAIL, MIXED_FILES.
    • pickup-late-entry: exit 1 FAIL, first_action_row 2, and the last attempt is STANDALONE/LATE.
  • A usage error exits 2 with no JSON, matching README:33/39.
  • tools/gate-check.sh lint: all 8 steps PASS, rc 0. The personal-paths step checked 880 files. The mode entry is 15756/16000 bytes.
  • tools/gate-check.sh test: rc 1.
    • PASS: tools unit tests (332), setup-pstack (185), bun install, bun tests (541 pass, 0 fail) and typecheck.
    • FAIL: promise unit tests, with 4 errors in test_claude_code, all IsolationUnavailable: unsupported executable for the ChatGPT.app codex-path rg.
    • Main 5b99800 with the gate's pinned 3.14.7 shows the same 4 errors (843 tests). The fault is in the host, not the PR, which does not touch evals/promises.
  • gh pr checks 293 -R mdsmithaustin/pstack: skills was pending at first, and the background poll finished it. All 7 checks pass at 23c1d9f: CodeQL, Analyze x3, skill-checks, skills (6m59s) and worktree-audit.
  • A sanitization grep over the PR files found nothing. No commit carries an attribution trailer.

Documentation (review mode, base 5b99800, head 23c1d9f)

  • evals/README.md: pass. It is unchanged since r2, and lines 21-23 and 39 still match the fixtures and the scope.
  • evals/skill-recovery/README.md: pass, with notes 1-3.
    • Verified against the code:
      • :33 and :39 (usage exit).
      • :66: exact versus mismatched claims, inconsistent_read_claims, overlaps, split_read_claim and the single-claim rule.
      • :68: carrier-independent scan, decoded-body claim coverage across blocks, the raw-region rule, the unsupported claimed attempt, and the two-level limit.
      • :74 (agent_message).
      • :78: per-stretch notice and whitespace handling, and notice-only packaging with unverified transport.
      • :82: one whole-body claim in one block for every whole_standalone.
      • :84: precedence wording, which matches evaluate.py:568-590.
    • The r2 doc items are closed: :66, :76/:78, :82 and :84 now hold at head, and :74 and the usage-exit item are fixed.
  • fixtures/README.md: unchanged and still accurate.

Not driven

  • No reviewed real recovery window was replayed with real governing files. The real-data evidence is the synthetic whole-read replay and count-only censuses. They do not show that any escaped or truncated body fell inside a real recovery window.
  • The census uses git-history blobs of skills/poteto-mode only. Installed variants outside history show up as the 73 inexact blocks, which were not classified further.
  • No Docker suites. .audit/ and /Users/msmith1/HMV/System/mds-pstack/.worktrees/issue-287-skill-recovery were not touched.

Ran on: claude-opus-5-5@xhigh

@mdsmithaustin
mdsmithaustin merged commit e649e23 into main Oct 21, 2026
7 checks passed
@mdsmithaustin
mdsmithaustin deleted the codex/issue-287-skill-recovery branch October 21, 2026 03:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant