fix(audit): AuditStore survives torn tails and failed writes, refuses non-finite scores (#274) by Birfy · Pull Request #277 · Birfy/agentdescent

Birfy · 2026-10-15T02:19:47Z

Closes #274

What changed (`agentdescent/audit/store.py`)

Torn tail repair. _write_line opens in ab+, checks the last byte, and writes a \n first when the file ends in a fragment from a killed process. The fragment stays its own skipped line and the record written after the crash survives. Before this, the new record was glued onto the fragment and both were lost.
Encode first, allow_nan=False. The line is encoded before the file is touched, so an unencodable payload (including a non-finite verifier_score) raises with the file unchanged and no NaN / Infinity token can reach disk.
Disk, then memory. append, resolve, reopen, remember_priorities and remember_fingerprint update the in-memory index only after the write returns. A failed write still raises (this is durable data), but len(store) now matches a fresh reload.
Per-line decoding. load() reads bytes and decodes each line. A tear inside a multi-byte character, a line that parses to a non-object (123, [], null), a side-record missing its keys, or a current-schema record whose fields won't construct an AuditRecord is counted in corrupt and skipped. An unknown schema_version still raises, as before.
Non-finite scores refused. observe_unlabelled and resolve raise ValueError on nan / inf, so the stratum moments and residuals can't be poisoned. AuditTap already swallows side-channel exceptions, so a NaN verifier score during evolve() just skips the audit unit instead of failing the rollout.

Tests

New tests/test_audit_store_durability.py (24 tests): 22 fail on main and all 24 pass on this branch. They cover the torn tail plus append / resolve, the mid-code-point tear, eight non-record line shapes, bad-field current-schema records, failed append / resolve / reopen / side-record writes (simulated read-only file), an unencodable record, non-finite unlabelled and oracle scores, a no-NaN-token file check, and a moments/side-record round trip.

Locally on Python 3.13, every tests/test_audit_*.py file plus the CLI tests ran: 582 passed.

Out of scope, as the issue notes: the same unterminated-tail glue in runstore.RunDir.append_round (to follow #272) and Ledger._exclude_non_ledger_files.

… non-finite scores (#274) - _write_line encodes with allow_nan=False before touching the file and writes a newline first when the file ends in a torn fragment, so the next record is not glued onto it. - load() reads bytes and decodes per line; UnicodeDecodeError, non-object lines, side-records missing keys and current-schema records that will not construct are counted in corrupt. Unknown schema_version still raises. - append/resolve/reopen/remember_priorities/remember_fingerprint update memory only after the write returns. - observe_unlabelled and resolve raise ValueError on non-finite scores. Closes #274

chatgpt-codex-connector · 2026-10-15T02:19:57Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-15T02:24:15.839723Z	`882dcaa`	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 882dcaabb1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-10-15T02:24:12Z

                    try:
-                        payload = json.loads(line)
-                    except json.JSONDecodeError:
+                        payload = json.loads(raw.decode("utf-8"))


Reject non-finite constants while loading legacy stores

When opening a store written by the previous implementation, which permitted NaN and Infinity tokens, Python's json.loads still converts those tokens to non-finite floats and _load_line accepts them in audit records and moments. Consequently an upgrade does not repair or quarantine the exact persisted values this change is intended to refuse, and calibration means, residuals, and intervals remain poisoned; use parse_constant or explicit finite validation so these lines are counted as corrupt.

Useful? React with 👍 / 👎.

Addressed in 0d412e4. load() now parses each line with parse_constant / parse_float hooks that refuse NaN, Infinity, -Infinity and overflowing literals such as 1e999. A legacy line holding one is counted in corrupt and skipped like any other malformed line. A NaN resolution line therefore leaves the record pending (the earlier finite line wins) and it can be resolved again; a NaN moments snapshot falls back to the previous one. Regressions: test_a_legacy_non_finite_record_line_is_counted_not_loaded[NaN|Infinity|-Infinity|1e999], test_a_legacy_non_finite_resolution_leaves_the_record_pending, test_legacy_non_finite_moments_do_not_reach_the_estimator. All fail on the previous head and pass now.

chatgpt-codex-connector · 2026-10-15T02:24:13Z

+        A non-finite ``oracle_score`` raises ``ValueError``: it would flow into
+        every residual and estimator as ``nan``.
        """
+        oracle_score = _finite_score(oracle_score, "oracle_score")


Preserve invalid-score errors during batch resolution

When a pending record receives NaN or infinity through resolve_from_mapping, this new validation raises ValueError, but agentdescent/audit/sources.py:193-198 catches every ValueError as an already-resolved duplicate. The invalid answer is therefore silently discarded while BatchResult.already_resolved falsely reports that the record already had truth; distinguish the duplicate case from score-validation failures so batch operators know the pending answer must be corrected.

Useful? React with 👍 / 👎.

Addressed in 0d412e4. resolve_from_mapping now converts and checks every score before resolving anything. A non-numeric or non-finite answer raises ValueError naming each offending record_id=value, and the store is left byte-identical, so a corrupt batch file is no longer reported as a harmless duplicate. Duplicates are now detected explicitly (store.get(id).resolved), and the ValueError fallback around resolve covers only a concurrent resolver, since scores are already known finite at that point. This also fixes the older variant where float("oops") was counted as already_resolved. Regressions: test_a_non_finite_batch_answer_refuses_the_batch_not_counted_as_resolved[nan|inf|"nan"|"oops"|None] and test_a_replayed_batch_still_counts_duplicates. -k audit: 633 passed locally.

Birfy

LGTM (COMMENT; self-approve blocked)

Closes #274. Independently verified on 882dcaabb19a8498907bf4cff12bbc47fd68919b:

Correctness

Torn-tail repair: _write_line opens ab+, seeks the last byte, and prefixes a \n when the file ends in a fragment — the glued-append failure from the issue (next record lost) is closed; resolve after a torn fragment also survives.
Byte-wise load: load() reads rb and decodes per line; UnicodeDecodeError / ValueError (covers JSONDecodeError) / non-dict / malformed side-records / bad current-schema fields increment corrupt and continue. Unknown schema_version still raises via AuditRecord.from_dict (existing contract kept; covered by regression).
Encode-first + allow_nan=False: payload is serialised before the file is touched; unencodable / non-finite content cannot leave a partial line or a NaN/Infinity token on disk.
Disk then memory: append / resolve / reopen / remember_priorities / remember_fingerprint update the in-memory index only after _write_line returns — failed write leaves len(store) agreeing with a fresh reload.
Boundary refusal: observe_unlabelled and resolve go through _finite_score and raise ValueError on nan/±inf, so Welford moments and residuals cannot be poisoned.

Design fit

Same durability family as #272 (runstore), #270 (cached_completion), #264 (evalcache), #260 (trajectory), #266 (ledger): encode-first, fail closed on non-portable tokens, skip torn lines. Scoped to audit/store.py as the issue asked; runstore/Ledger trailing-newline follow-ups correctly left out.

Tests

tests/test_audit_store_durability.py (24): 22 fail on main / all 24 pass on branch (isolated venvs; confirmed the torn-tail, mid-codepoint, non-record, memory-ahead-of-disk, and non-finite paths).
Full tests/test_audit_*.py on branch: 580 passed.
Foreign schema_version still raises; moments / priority / fingerprint round-trip covered.

Docs

CHANGELOG Fixed entry under Unreleased accurately describes the five failure modes and the keep-raising schema contract.

Nits (non-blocking)

Issue suggested also rejecting non-finite verifier_score in AuditRecord.__post_init__. Today a NaN score can still be constructed in memory; append refuses to persist it via allow_nan=False, and AuditTap swallows the write error. A __post_init__ check would fail earlier and keep in-memory residuals finite — fine as a follow-up.
_flush_moments still updates disk from already-mutated in-memory Welford state (by design: crash loses ≤FLUSH_EVERY). Unrelated to this PR’s append/resolve ordering fix.
Torn-tail repair is best-effort across processes (no cross-process lock); same-process _lock covers the usual path.

CI matrix still pending on this push at review time.

Birfy · 2026-10-15T02:59:31Z

P2: Validate record IDs before indexing or persisting them.

At 882dcaa, an otherwise valid current-schema JSON record with record_id: [] (or {}) passes AuditRecord.from_dict: the dataclass does not validate the ID type. _load_line then calls _remember outside its exception handler, so dictionary membership raises TypeError and the whole store fails to open instead of counting that line as corrupt and reading subsequent valid records. The new disk-first append order also writes such a constructed record before the same error, leaving a file that cannot be reloaded.

Please validate the ID before writes and contain malformed-ID failures during loading. Regressions should cover an invalid ID between valid records (corrupt count increments; valid records survive) and append rejection leaving the file unchanged.

Source:

agentdescent/agentdescent/audit/store.py

Lines 186 to 190 in 882dcaa

    
           try: 
        
               rec = AuditRecord.from_dict(payload) 
        
           except (KeyError, TypeError, ValueError): 
        
               return False 
        
           self._remember(rec)

and

agentdescent/agentdescent/audit/store.py

Lines 216 to 223 in 882dcaa

    
           def append(self, record: AuditRecord) -> AuditRecord: 
        
               with self._lock: 
        
                   # Disk first, memory second: a write that raises (a full disk, a 
        
                   # read-only file, a non-finite score `allow_nan=False` refuses) 
        
                   # must leave the index agreeing with what a fresh load would see. 
        
                   self._write(record) 
        
                   self._remember(record) 
        
                   self._flush_moments()

Source inspection only; local execution remains paused, so no runtime reproduction is claimed.

Resolve CHANGELOG.md: keep #274 AuditStore entry above newly merged #263 FileCache.

Birfy

Re-check after merging `main` (post-#264/#253)

CHANGELOG conflict resolved by keeping this PR's Fixed entry above the newly merged #263 FileCache note. Exact-head CI (build + 3.9–3.13) is green again; still MERGEABLE. Prior LGTM stands (self-approve blocked → COMMENT).

A list/dict/int/empty record_id passed AuditRecord.from_dict, so load() crashed on dictionary membership (unhashable) and append() wrote an unreloadable line before failing. AuditRecord.__post_init__ now requires a non-empty str, so load() counts such a line as corrupt and append() can never be handed one. Addresses review feedback on #277.

Birfy · 2026-10-15T06:19:39Z

Addressed the P2 on record IDs in f4abf91: AuditRecord.__post_init__ now requires record_id to be a non-empty str (raises TypeError). So load() counts a line with record_id: []/{}/int/"" as corrupt and keeps reading the valid records after it, and append() can never be handed such a record, leaving the file unchanged. Regressions added for both (an invalid ID between valid records; construction rejection with the file byte-identical). The new append-side tests fail on the previous head; all -k audit tests pass locally.

…tore-durability # Conflicts: # CHANGELOG.md

Birfy

Re-review at 4504d6d: still LGTM. This push only merges current main (1049ab5, after #250/#260/#279 landed); the sole conflict was CHANGELOG.md, resolved by keeping both entries, and the branch diff against main is unchanged apart from that. Full local suite passed and CI is green. Mergeable, no open blocking feedback.

…es bad scores instead of counting them as duplicates (#274)

Birfy

Re-review at `0d412e4`: LGTM (COMMENT, since self-approve is blocked)

This push addresses the two Codex findings that were still open on 882dcaa. Each one has a thread reply.

P1: legacy `NaN` / `Infinity` lines on load (`store.py`)

load() now parses with parse_constant=_refuse_constant and parse_float=_finite_float. The NaN / Infinity / -Infinity tokens an older allow_nan=True build wrote, and overflowing literals such as 1e999, now raise ValueError inside the existing per-line handler, so the line is counted in corrupt and skipped like any other malformed line.
Semantics checked against last-occurrence-wins:
- a NaN resolution line leaves the record at its earlier, finite pending line, and it can be resolved again;
- a NaN dispatch line drops only that record;
- a NaN moments or priority snapshot falls back to the previous snapshot.
Nothing non-finite reaches residuals, _Welford, or PPI.

P2: batch resolution reporting a bad answer as a duplicate (`sources.py`)

resolve_from_mapping converts and checks every score before resolving anything. A non-numeric ("oops", None) or non-finite answer raises ValueError naming each record_id=value, and the file stays byte-identical. Duplicates are now detected explicitly (rec.resolved) instead of by catching every ValueError. That also fixes the pre-existing variant where float("oops") was silently counted in already_resolved.
The except ValueError that remains around store.resolve can only be the overwrite refusal from a concurrent resolver, because the score is already known finite at that point. The BatchResult / int contract is unchanged.

Tests

New: test_a_legacy_non_finite_record_line_is_counted_not_loaded[NaN|Infinity|-Infinity|1e999], test_a_legacy_non_finite_resolution_leaves_the_record_pending, test_legacy_non_finite_moments_do_not_reach_the_estimator, test_a_non_finite_batch_answer_refuses_the_batch_not_counted_as_resolved[nan|inf|"nan"|"oops"|None], test_a_replayed_batch_still_counts_duplicates.
11 of these fail on the previous head and pass now. tests/test_audit_store_durability.py: 44 passed. -k audit: 633 passed locally.
The existing test_audit_sparse replay expectation (== 3) is unchanged.

Nits (non-blocking)

Refusing the whole batch is a deliberate change: a CSV with one NaN cell used to land the good rows and drop the bad one silently, and now nothing lands until the file is fixed. The docstring and CHANGELOG say so. If partial landing is preferred, an invalid count on BatchResult would be the alternative.
AuditRecord.__post_init__ still accepts a NaN verifier_score when constructed directly. append refuses it via allow_nan=False and load now skips it, so it cannot persist; a follow-up could validate at construction.

Exact-head CI (build + 3.9–3.13) is green and the PR is MERGEABLE.

…tore-durability # Conflicts: # CHANGELOG.md

Birfy

Re-checked after merging main (924c25c, includes #272) at 0d43d08. The only conflict was CHANGELOG.md (### Fixed): both entries are kept, the #274 AuditStore entry above the #271 runstore entry, and the blank line before the #261 entry is preserved. No code conflicts. The #272 runstore torn-tail work and this branch's AuditStore torn-tail work live in different modules. All tests/test_audit*.py pass locally on the merge commit, and CI is fully green (build + 3.9–3.13). Both Codex threads (legacy non-finite load P1, batch-resolve P2) were addressed in 0d412e4, with replies, and there is no newer feedback. LGTM, still merge-ready.

Keep both #274 and #286 Fixed entries.

Birfy · 2026-10-16T04:18:53Z

Merged main (751b8dd) into this branch — CHANGELOG-only conflict resolved (kept #274 + #286 Fixed entries). Head 5302a4a. Audit test suite passed locally.

LGTM once CI is green.

Keep both CHANGELOG Fixed bullets (#274 AuditStore + #293 selection knobs).

…oth CHANGELOG bullets

… keep both CHANGELOG bullets

chatgpt-codex-connector Bot reviewed Oct 15, 2026

View reviewed changes