From e1cf9303f006989a41eec61f70f5baebf95433ea Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Thu, 3 Sep 2026 02:53:31 +0530 Subject: [PATCH] fix(recovery): exclude stub rows from the plausibility gate; prove it end to end Follow-ups on the #101423 salvage (#101409): - `stub_missing_parent_sessions` legitimately writes `started_at = 0.0` when no timestamped message survived; a salvage where only stubs remain is depleted, not mis-mapped. Stub rows leave the sessions denominator. - Reuse `_EPOCH_LOW` from session_lost_and_found instead of a second 2001-epoch constant; collapse the two per-table blocks into one loop. - Tests: a real upgraded-layout source (started_at physically appended) with page 1 zeroed goes through the real `.recover` lane and the report comes back `verified: False`; a stub-only output is not flagged; a mapped row with a NULL title (the mis-mapped shape) still counts as mapped. --- hermes_cli/session_recovery.py | 61 ++++----- .../test_session_recovery_lost_and_found.py | 122 ++++++++++++++++++ 2 files changed, 144 insertions(+), 39 deletions(-) diff --git a/hermes_cli/session_recovery.py b/hermes_cli/session_recovery.py index 20255ffe58..be6afb0b7e 100644 --- a/hermes_cli/session_recovery.py +++ b/hermes_cli/session_recovery.py @@ -1431,11 +1431,6 @@ def _finalize_derived_metadata(destination: sqlite3.Connection) -> dict[str, Any return result -# Floor for a plausible unix-epoch timestamp (2001-09-09). Salvage rows -# uniformly below it were mis-mapped, not merely unlucky (#101409). -_PLAUSIBLE_TIMESTAMP_FLOOR = 1_000_000_000.0 - - def _lost_and_found_plausibility_errors( conn: sqlite3.Connection, ) -> list[str]: @@ -1447,47 +1442,35 @@ def _lost_and_found_plausibility_errors( from the destination template's declared order, so positional cell mapping lands counters/strings where ``started_at``/``timestamp`` belong — and the NOT NULL substitutes turn gaps into 0.0. When every - row violates the epoch floor, the mapping was wrong. + mapped row violates the epoch floor, the mapping was wrong. + + Stub rows written by ``stub_missing_parent_sessions`` legitimately carry + ``started_at = 0.0`` when no timestamped message survived, so they are + excluded from the denominator. """ + from hermes_cli.session_lost_and_found import _EPOCH_LOW errors: list[str] = [] - - (session_total,) = conn.execute( - "SELECT COUNT(*) FROM sessions" - ).fetchone() - if session_total: + checks = ( + ("sessions", "started_at", "WHERE COALESCE(title, '') NOT LIKE '[best-effort recovered%'"), + ("messages", "timestamp", ""), + ) + for table, column, mapped_filter in checks: + (total,) = conn.execute(f"SELECT COUNT(*) FROM {table} {mapped_filter}").fetchone() + if not total: + continue (implausible,) = conn.execute( - "SELECT COUNT(*) FROM sessions " - "WHERE started_at IS NULL OR started_at < ?", - (_PLAUSIBLE_TIMESTAMP_FLOOR,), + f"SELECT COUNT(*) FROM {table} {mapped_filter} " + f"{'AND' if mapped_filter else 'WHERE'} ({column} IS NULL OR {column} < ?)", + (_EPOCH_LOW,), ).fetchone() - if implausible == session_total: + if implausible == total: errors.append( - f"sessions.started_at is implausible in all " - f"{session_total} salvaged row(s) (NULL or before 2001-09): " - "the source's physical column order did not match the " - "destination template, so cells were mapped onto the " - "wrong columns" + f"{table}.{column} is implausible in all {total} salvaged row(s) " + "(NULL or before 2001-09): the source's physical column order " + "did not match the destination template, so cells were mapped " + "onto the wrong columns" ) - - (message_total,) = conn.execute( - "SELECT COUNT(*) FROM messages" - ).fetchone() - if message_total: - (implausible,) = conn.execute( - "SELECT COUNT(*) FROM messages " - "WHERE timestamp IS NULL OR timestamp < ?", - (_PLAUSIBLE_TIMESTAMP_FLOOR,), - ).fetchone() - if implausible == message_total: - errors.append( - f"messages.timestamp is implausible in all " - f"{message_total} salvaged row(s) (NULL or before 2001-09): " - "the source's physical column order did not match the " - "destination template, so cells were mapped onto the " - "wrong columns" - ) - return errors diff --git a/tests/hermes_cli/test_session_recovery_lost_and_found.py b/tests/hermes_cli/test_session_recovery_lost_and_found.py index 79bb6810a7..b2823d3e29 100644 --- a/tests/hermes_cli/test_session_recovery_lost_and_found.py +++ b/tests/hermes_cli/test_session_recovery_lost_and_found.py @@ -736,3 +736,125 @@ def test_plausibility_gate_passes_correctly_mapped_salvage( assert session_recovery._lost_and_found_plausibility_errors(conn) == [] finally: conn.close() + + +def _rebuild_with_started_at_appended(conn: sqlite3.Connection) -> None: + """Give ``sessions`` the physical layout of an upgraded DB: ``started_at`` + lands at the END (as ``ALTER TABLE ADD COLUMN`` would place a column that + the current template declares mid-definition). Data is preserved.""" + info = list(conn.execute("PRAGMA table_info(sessions)")) + declared = [row[1] for row in info] + + def coldef(row): + _, name, ctype, notnull, dflt, pk = row + parts = [f'"{name}" {ctype}'] + if pk: + parts.append("PRIMARY KEY") + if notnull: + parts.append("NOT NULL") + if dflt is not None: + parts.append(f"DEFAULT {dflt}") + return " ".join(parts) + + reordered = [r for r in info if r[1] != "started_at"] + [r for r in info if r[1] == "started_at"] + cols = ", ".join(f'"{c}"' for c in declared) + conn.executescript("PRAGMA foreign_keys=OFF;") + conn.execute("CREATE TABLE sessions_new (" + ", ".join(coldef(r) for r in reordered) + ")") + conn.execute(f"INSERT INTO sessions_new({cols}) SELECT {cols} FROM sessions") + conn.executescript("DROP TABLE sessions; ALTER TABLE sessions_new RENAME TO sessions;") + + +@pytest.mark.skipif( + not HAVE_SQLITE3_CLI, + reason="sqlite3 CLI not on PATH; .recover is a shell-only feature", +) +def test_lost_and_found_lane_refuses_to_verify_a_physically_shifted_source( + tmp_path: Path, +) -> None: + """#101409 end to end: a source whose physical column order differs from + the template's declared order maps every cell onto the wrong column. The + output still passes integrity/FK/FTS, so only the plausibility gate can + stop the report from claiming ``verified``.""" + source = tmp_path / "upgraded.db" + output = tmp_path / "upgraded-recovered.db" + db = SessionDB(db_path=source) + try: + for n in range(3): + sid = f"20260812_1400{n:02d}_def{n:03x}" + db.create_session(sid, "cli", cwd=f"/tmp/shift-{n}") + db.set_session_title(sid, f"shift {n}") + for m in range(4): + db.append_message(sid, "user" if m % 2 == 0 else "assistant", f"payload {n} {m}") + finally: + db.close() + conn = sqlite3.connect(str(source), isolation_level=None) + try: + conn.execute("PRAGMA wal_checkpoint(TRUNCATE)") + conn.execute("PRAGMA journal_mode=DELETE") + _rebuild_with_started_at_appended(conn) + conn.execute("VACUUM") + physical = [r[1] for r in conn.execute("PRAGMA table_info(sessions)")] + assert physical[-1] == "started_at" + finally: + conn.close() + # The reporter's damage: page 1 (header + sqlite_master) overwritten, so + # ``.recover`` cannot name any table and every row lands in + # lost_and_found, to be mapped positionally onto the template. + with open(source, "r+b") as fh: + fh.write(b"\0" * _page_size(source.read_bytes())) + + report = recover_session_database(source, output, work_dir=tmp_path, allow_partial=True) + + assert report["mode"] == "lost_and_found_salvage" + # Mis-mapped rows that trip a NOT NULL / type constraint are stubbed, not + # mapped (the reporter saw 190 of 1,875) — at least one lands positionally. + assert report["lost_and_found"]["mapped"]["sessions"] >= 1 + assert report["verification"]["healthy"] is False + assert report["verified"] is False + assert any("sessions.started_at is implausible" in e for e in report["verification"]["errors"]) + out = sqlite3.connect(str(output)) + try: + # The mis-mapping the gate caught: every mapped (non-stub) session got + # the NOT NULL substitute where its real start time should be. + mapped = out.execute( + "SELECT started_at FROM sessions WHERE COALESCE(title, '') NOT LIKE '[best-effort recovered%'" + ).fetchall() + assert mapped and all(row[0] == 0.0 for row in mapped) + finally: + out.close() + + +def test_plausibility_gate_ignores_stub_only_sessions(tmp_path: Path) -> None: + """Stub rows from ``stub_missing_parent_sessions`` legitimately carry + ``started_at = 0.0``; a salvage where only stubs survived is depleted, + not mis-mapped, and must not be flagged.""" + output = tmp_path / "stubs.db" + SessionDB(db_path=output).close() + conn = sqlite3.connect(str(output)) + try: + now = 1_750_000_000.0 + conn.execute( + "INSERT INTO sessions (id, source, started_at, title) VALUES (?, ?, ?, ?)", + ("20260812_140000_aaa000", "recovered", 0.0, "[best-effort recovered 1] session metadata was unreadable"), + ) + conn.execute( + "INSERT INTO messages (session_id, role, content, timestamp) VALUES (?, ?, ?, ?)", + ("20260812_140000_aaa000", "user", "hi", now), + ) + conn.commit() + assert session_recovery._lost_and_found_plausibility_errors(conn) == [] + # One genuinely mapped row with a real timestamp keeps it clean too... + conn.execute( + "INSERT INTO sessions (id, source, started_at, title) VALUES (?, ?, ?, ?)", + ("20260812_140001_aaa001", "cli", now, None), + ) + conn.commit() + assert session_recovery._lost_and_found_plausibility_errors(conn) == [] + # ...and a mapped row at 0.0 with a NULL title (the mis-mapped shape: + # blank titles) is still counted as mapped, not as a stub. + conn.execute("UPDATE sessions SET started_at = 0.0 WHERE id = '20260812_140001_aaa001'") + conn.commit() + errors = session_recovery._lost_and_found_plausibility_errors(conn) + assert len(errors) == 1 and "sessions.started_at" in errors[0] + finally: + conn.close()