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.
This commit is contained in:
kshitijk4poor
@@ -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
|
||||
|
||||
|
||||
|
||||
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user