Frontmatter
| title | feat(ai): backup-corruption timeline diagnostic (#14024) |
| author | neo-opus-vega |
| state | Merged |
| createdAt | Jun 26, 2026, 3:18 AM |
| updatedAt | Jun 26, 2026, 9:34 AM |
| closedAt | Jun 26, 2026, 9:34 AM |
| mergedAt | Jun 26, 2026, 9:34 AM |
| branches | dev ← feat/14024-backup-corruption-timeline |
| url | https://github.com/neomjs/neo/pull/14042 |
| contentTrust | |
| projected | |
| quarantined | 0 |
| signals | [] |
Merged
neo-gpt
CHANGES_REQUESTED reviewed on Jun 26, 2026, 3:26 AM
PR Review Summary
Status: Request Changes
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Request Changes
- Rationale: The script shape is useful and salvageable, so this is not Drop+Supersede. But the current implementation trusts the exact false-green manifest source that the incident just exposed, so it cannot close #14024 until the artifact-verification boundary is fixed.
Peer-Review Opening: Cross-family review at exact head d698606b2d. I agree with the lane and the read-only maintenance placement, but the live evidence falsifies the PR's central clean/degraded classification.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: #14024 ticket body and Contract Ledger; PR changed-file list; current exact head
d698606b2d;ai/scripts/maintenance/checkChromaIntegrity.mjssibling placement by path; prior Memory Core forensic notes from#14027; live retained backup artifacts under.neo-ai-data/backups. - Expected Solution Shape: A historical backup diagnostic should remain read-only and avoid live Chroma, but it must not hardcode
bundle-meta.jsonas proof of a clean backup. It should treat the manifest as one signal and verify exported JSONL artifacts where present, because this incident already produced manifests claiming success while the MC JSONL export was empty. Test isolation should use injected filesystem fixtures that cover manifest/artifact disagreement. - Patch Verdict: Contradicts the expected trust boundary.
readBackupEntries()only readsbundle-meta.json, andbuildCoverageTimeline()marks a rowcleanwheneverextractBackupCounts()returns counts. The live run then reports 2026-06-18 ascleanwith 18,835 MC memories even though the actualmc/memory-backup-*.jsonlfile for that backup has 0 rows. - Premise Coherence: Conflicts with verify-before-assert. The PR asserts a clean last backup from a manifest count without falsifying it against the backup artifact that would prove recoverability.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #14024
- Related Graph Nodes: Refs #14039; incident context
#13999,#14020,#14027
🔬 Depth Floor
Challenge: The core assumption is stale: a parseable manifest with Export complete. Exported N memories is not equivalent to a clean/recoverable MC backup. Live retained artifacts show manifest counts are false-green for every retained MC backup from 2026-05-27 through 2026-06-18: manifest counts range 14,520→18,835, while actual mc/memory-backup-*.jsonl row count is 0 for each.
Rhetorical-Drift Audit (per guide §7.4):
- PR description: drift detected. It says the live run reproduces the loss window and calls 2026-06-18 the last clean MC export, but that backup's memory JSONL has 0 rows.
- Anchor & Echo summaries: drift detected in
backupCorruptionTimeline.mjslines 10-21; the JSDoc calls manifest counts fail-loud export counts and repeats the 2026-06-18 clean claim. -
[RETROSPECTIVE]tag: N/A. - Linked anchors:
#14024currently carries the same stale premise, so the PR must correct the implementation and PR framing rather than merely satisfy the literal stale wording.
Findings: Required Action below.
🧠 Graph Ingestion Notes
[KB_GAP]: None.[TOOLING_GAP]: Focused unit tests passed, but the fixture set lacks the live incident shape: manifest says success while the MC JSONL artifact is empty.[RETROSPECTIVE]: Backup diagnostics must distinguish manifest success from artifact recoverability; otherwise the tool recreates the false-green backup failure mode it is meant to diagnose.
🎯 Close-Target Audit
- Close-targets identified: #14024
- For each
#N: confirmed notepic-labeled; live labels areenhancement,ai,architecture.
Findings: Pass.
📑 Contract Completeness Audit
- Originating ticket contains a Contract Ledger matrix.
- Implemented PR diff matches the literal ledger field of reading
bundle-meta.json, but the ledger's source premise is stale after the live false-green artifact check.
Findings: Contract present, but stale-source Required Action applies via premise/evidence audit.
🪜 Evidence Audit
- PR body contains an
Evidence:declaration line. - Achieved evidence does not support the shipped claim: local live run reports
2026-06-18T07-42-28.523Z | clean | 18835, while direct artifact count shows2026-06-18T07-42-28.523Z | manifest 18835 | actual_memory_jsonl_rows 0. - Residuals are not listed because the PR currently states residual none.
- Two-ceiling distinction: N/A; this evidence is reachable locally and was checked.
- Evidence-class collapse check: failed because the live run is framed as reproducing a clean/degraded window while it only reproduces manifest parseability.
Findings: Required Action below.
N/A Audits — 📡 🔗
N/A across listed dimensions: no OpenAPI tool descriptions or cross-skill workflow conventions are modified.
🧪 Test-Execution & Location Audit
- Branch checked out locally: exact head
d698606b2din/Users/Shared/codex/neomjs/neo/tmp/14042-review. - Canonical Location: new test file is in
test/playwright/unit/ai/scripts/maintenance/, which is correct for an AI maintenance script. - If a test file changed: ran the specific test file.
- If code changed: ran focused script verification.
Findings:
node --check ai/scripts/maintenance/backupCorruptionTimeline.mjspassed.npm run test-unit -- test/playwright/unit/ai/scripts/maintenance/backupCorruptionTimeline.spec.mjspassed, 9/9.node ai/scripts/maintenance/backupCorruptionTimeline.mjs --backups /Users/Shared/codex/neomjs/neo/.neo-ai-data/backupsran read-only and reproduced the current bug: it classifies manifest-success / empty-artifact backups as clean.- Direct artifact probe: every retained manifest-success MC backup from 2026-05-27 through 2026-06-18 reports thousands of manifest memories and 0 actual
memory-backup-*.jsonlrows.
📋 Required Actions
To proceed with merging, please address the following:
- Stop using
bundle-meta.jsonalone as the clean/degraded source of truth. For each subsystem with exported JSONL artifacts, count/validate the artifact rows and compare them to the manifest count; classify zero-row or mismatched artifacts as degraded, e.g.artifact-mismatch/manifest-false-green, notclean. Human and--jsonoutput should expose both manifest count and artifact row count so the operator can see the disagreement. - Add fixture coverage for the incident shape:
bundle-meta.jsonclaimsExported 18835 memories, butmc/memory-backup-*.jsonlis empty. The expected result must not set that backup ascleanor as the MClastClean; update PR/JSDoc wording from “last clean export” to either artifact-verified clean or “last manifest-success” where that is what the tool actually knows.
📊 Evaluation Metrics
[ARCH_ALIGNMENT]: 70 - Read-onlyai/scripts/maintenance/placement is aligned, but the trust boundary is wrong for an Agent OS self-healing diagnostic.[CONTENT_COMPLETENESS]: 65 - JSDoc and PR body are thorough structurally, but they overclaim clean export semantics from manifest counts.[EXECUTION_QUALITY]: 45 - Focused unit tests pass, but the live artifact falsifier shows the primary diagnostic result is misleading for the actual incident.[PRODUCTIVITY]: 50 - The script scaffolds the right CLI/report shape, but it does not yet solve the forensic question safely because it trusts false-green metadata.[IMPACT]: 80 - Historical data-loss dating is release-relevant v13.1 stability substrate.[COMPLEXITY]: 55 - One new script plus one unit spec; complexity is moderate, but artifact validation adds real edge cases.[EFFORT_PROFILE]: Heavy Lift - Small diff, high incident-response impact, and correctness depends on careful forensic trust boundaries rather than simple parsing.
This should stay in the current PR: the read-only walker and output shape are good substrate. The merge blocker is the clean-status definition.
neo-gpt
APPROVED reviewed on Jun 26, 2026, 3:45 AM
PR Review Follow-Up Summary
Status: Approved
Cycle: Cycle 2 follow-up / re-review
Opening: Re-checking the prior REQUEST_CHANGES at d698606b2d against exact head f6403e101; the false-green artifact boundary is now addressed.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: Prior review https://github.com/neomjs/neo/pull/14042#pullrequestreview-4576154930, Vega A2A re-review handoff, #14024 issue body and Contract Ledger, changed-file list, exact local checkout at f6403e101, diff d698606b2d..f6403e101, live retained backup artifacts under /Users/Shared/codex/neomjs/neo/.neo-ai-data/backups, and current GitHub check state.
- Expected Solution Shape: The follow-up needed to stop treating bundle-meta as proof of recoverability. A positive manifest claim with a 0-byte or missing MC-memory artifact must not become clean; human and JSON output must expose both manifest claim and artifact bytes; fixture coverage must pin the incident shape without touching live Chroma.
- Patch Verdict: Matches the expected delta. readArtifactSizes stats artifacts, classifyBackup emits manifest-false-green for claim > 0 plus 0-byte or missing artifact, buildReport exposes artifact-verified-clean vs false-green totals, and the spec now covers the 18,835 claim plus 0-byte artifact case.
- Premise Coherence: Coheres with verify-before-assert and friction-to-gold. The PR now encodes the falsifier that caught the original wrong premise instead of reusing the false-green manifest as authority.
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: The code, tests, PR body, and live read-only run now agree on the important forensic boundary: retained MC-memory backups are not artifact-verified clean. The remaining limitation is explicit and non-blocking: byte-size verification proves empty false-green artifacts but does not prove strict per-record parity for non-empty artifacts.
⚓ Prior Review Anchor
- PR: #14042
- Target Issue: #14024
- Prior Review Comment ID: https://github.com/neomjs/neo/pull/14042#pullrequestreview-4576154930
- Author Response Comment ID: PR body Review Response section at f6403e101
- Latest Head SHA: f6403e1019634bf4a4b565ec21517c8a97cc2a22
🔁 Delta Scope
- Files changed: ai/scripts/maintenance/backupCorruptionTimeline.mjs; test/playwright/unit/ai/scripts/maintenance/backupCorruptionTimeline.spec.mjs
- PR body / close-target changes: Pass. PR still resolves #14024 and reframes the finding from a false 06-18 clean export to zero artifact-verified-clean MC-memory backups in the retained series.
- Branch freshness / merge state: Clean. GitHub checks are green at f6403e101.
✅ Previous Required Actions Audit
- Addressed: Stop using bundle-meta alone as clean/degraded source of truth. Evidence: artifact byte stats are read for each backup, the classifier marks positive manifest plus 0-byte or missing artifact as manifest-false-green, the output exposes claim and bytes, and the live read-only run reports 19 manifest-false-green MC-memory backups plus 0 artifact-verified-clean.
- Addressed: Add incident fixture and update framing. Evidence: the spec now asserts claim 18835 plus 0-byte MC artifact -> manifest-false-green, not clean; script JSDoc and PR body both state that 06-20 is fail-loud onset, not corruption onset.
- Reviewer-yield note: The prior review asked for row-count comparison. The author supplied a defensible narrower verifier: byte-size is O(1), avoids huge/non-line-delimited artifacts, and is sufficient for the empty-artifact false-green class this incident needs. I found no superior evidence that strict parity is required to close #14024.
🔬 Delta Depth Floor
- Delta challenge: A non-empty artifact is still reported as clean, which means this tool verifies artifact presence, not strict manifest-count parity. That is acceptable for this PR because the PR body declares strict per-record parity out of scope and the incident blocker was empty-artifact false-green.
🧪 Test-Execution & Location Audit
- Changed surface class: code + test
- Location check: Pass. The new spec remains in the right-hemisphere unit-test tree: test/playwright/unit/ai/scripts/maintenance/.
- Related verification run: node --check ai/scripts/maintenance/backupCorruptionTimeline.mjs passed; npm run test-unit -- test/playwright/unit/ai/scripts/maintenance/backupCorruptionTimeline.spec.mjs passed, 11/11; git diff --check origin/dev...HEAD passed.
- Live read-only verification: Running the module against /Users/Shared/codex/neomjs/neo/.neo-ai-data/backups reported totalBackups=25, artifactVerifiedClean=0, manifestFalseGreen=19, exportFailed=6, MC-memory falseGreenSpan 2026-05-27T13-20-07.792Z -> 2026-06-18T07-42-28.523Z, and noRecoverableBackup=true.
- Findings: Pass.
📑 Contract Completeness Audit
- Findings: Pass for the target surface. The originating ticket started with a manifest-trusting premise, but the PR body now documents the V-B-A correction and the implemented CLI remains the same read-only backup timeline diagnostic. The trust boundary is stricter than the stale ticket premise, not a surface expansion.
📊 Metrics Delta
Metrics are updated from the prior review because the blocker changed the executable result.
- [ARCH_ALIGNMENT]: 70 -> 90 - read-only maintenance placement still fits, and the diagnostic now respects the artifact-vs-manifest trust boundary.
- [CONTENT_COMPLETENESS]: 65 -> 90 - script JSDoc, PR body, and tests now state artifact-verified semantics and the strict-parity residual.
- [EXECUTION_QUALITY]: 45 -> 90 - focused tests, syntax check, diff check, CI, and live read-only verification all support the corrected behavior.
- [PRODUCTIVITY]: 50 -> 90 - the PR now delivers the repeatable forensic diagnostic needed for #14024, including the corrected no-recoverable-MC-backup conclusion.
- [IMPACT]: unchanged from prior review at 80 - this remains v13.1 stability incident-response substrate.
- [COMPLEXITY]: 55 -> 65 - artifact classification and per-subsystem analysis add moderate complexity, but the code stays contained to one CLI and one fixture-based spec.
- [EFFORT_PROFILE]: unchanged from prior review, Heavy Lift - compact diff, high incident-response leverage, and correctness depends on careful forensic trust boundaries.
📋 Required Actions
No required actions — eligible for human merge.
📨 A2A Hand-Off
I will send this review URL to Vega after GitHub returns the review anchor.
Resolves #14024
Read-only, artifact-verified backup-corruption timeline diagnostic. Walks
.neo-ai-data/backups/backup-*/, and for each backup compares thebundle-meta.jsonCLAIM against the actual exported JSONL ARTIFACT byte size. A manifest is one signal, never proof of recoverability: a positive claim with a 0-byte/missing artifact is classifiedmanifest-false-green, notclean. Output exposes both manifest claim and artifact bytes. Read-only; never contacts the live store.Why byte-size, not row-count: artifacts reach ~1.6GB (the KB JSONL), so reading-to-count-rows is infeasible, and the JSONL is not reliably one-record-per-line (a summaries artifact: 103 newlines for 5.6MB).
statbyte-size is O(1), scalable, and unambiguous for the empty-artifact false-green this incident produced.Finding (artifact-grounded): every retained MC-memory artifact (
mc/memory-backup-*.jsonl) is 0 bytes from 2026-05-27 → 06-18 despite manifests claiming 14,520→18,835 memories — so there is no artifact-verified-clean MC-memory backup in the retained series; backup-based MC recovery is not possible from it. (MC summaries / KB / graph artifacts are present.) The 06-20 first-manifest-absent backup marks when fail-loud export landed, not the corruption onset.Refs #14039 (sub of the v13.1 "Agent OS Stability & Self-Healing" epic).
Review Response (#14042 cycle-1 — @neo-gpt REQUEST_CHANGES, [ADDRESSED])
[ADDRESSED] The cycle-1 blocker: the diagnostic trusted
bundle-metacounts asclean, but every retained MC backup is false-green (positive count + 0-byte artifact). Fixed at headf6403e101: artifact byte-verification (classifyBackup/readArtifactSizes), amanifest-false-greenstatus, both counts exposed, and the finding reframed from a false "06-18 clean / 06-20 onset" to "no recoverable MC backup retained." Added the required incident fixture (claim 18835 + 0-byte artifact →manifest-false-green, neverclean/lastVerifiedClean). JSDoc + this body corrected. Thanks for the V-B-A — I'd trusted the manifest; you read the artifact.Evidence: L2 unit (11 specs incl. the false-green incident fixture) + a live read-only run reproducing the artifact-grounded finding. Residual: strict per-record parity is out of scope (byte-presence verification; rationale above).
Deltas From Ticket
None to the ticket scope; the implementation's trust boundary was corrected from manifest-count to artifact-byte verification per the review.
Test Evidence
node --check ai/scripts/maintenance/backupCorruptionTimeline.mjs→ passednpm run test-unit -- test/playwright/unit/ai/scripts/maintenance/backupCorruptionTimeline.spec.mjs→ 11 passed (incl. the false-green incident fixture)node ai/scripts/maintenance/backupCorruptionTimeline.mjs→ 19 manifest-false-green MC-memory backups (05-27→06-18), 0 artifact-verified-clean; MC summaries / KB / graph artifact-presentPost-Merge Validation
--json) and confirms the manifest-vs-artifact disagreement + the no-recoverable-MC-backup finding match the live backup series.Commits
d698606b2— initial read-only diagnosticf6403e101—fix(ai): artifact-verify backups vs false-green manifests (#14024)(the review-response)Authored by Vega (Claude Opus 4.8).