fix(cli): flag mis-mapped lost_and_found salvage as not verified
The lost_and_found salvage lane maps cells positionally onto the destination template's declared column order, but a source upgraded via ALTER TABLE has its columns in the order they were added, which differs from SCHEMA_SQL whenever a column was inserted mid-definition (#101409). Every row still inserts, so integrity/FK/FTS/count checks stay green and the report ends up verified: true — while all 1,875 sessions in the reporter's DB got started_at = 0.0 with counters and URLs shifted into the wrong columns. Add a semantic plausibility gate to the salvage lane: when every salvaged sessions.started_at or messages.timestamp is NULL or before 2001-09, the cells were mapped onto the wrong columns and the recovery is reported with errors and healthy: false, so verified no longer claims a mis-mapped output. A partially damaged column (torn cells on some rows) does not trip the gate — only a systematic violation does. Fixing the positional mapping itself needs a historical-layouts table derived from schema-version history; that design decision is left to maintainers (suggested fix 2 in the issue).
This commit is contained in:
liuhao1024
@@ -1431,6 +1431,66 @@ 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]:
|
||||
"""Flag systematic timestamp mis-mapping in a salvaged database.
|
||||
|
||||
Structural checks (integrity, FK, FTS, row counts) pass on mis-mapped
|
||||
salvage because every row still inserts. Only semantics give it away:
|
||||
the physical column order of a source upgraded via ALTER TABLE differs
|
||||
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.
|
||||
"""
|
||||
|
||||
errors: list[str] = []
|
||||
|
||||
(session_total,) = conn.execute(
|
||||
"SELECT COUNT(*) FROM sessions"
|
||||
).fetchone()
|
||||
if session_total:
|
||||
(implausible,) = conn.execute(
|
||||
"SELECT COUNT(*) FROM sessions "
|
||||
"WHERE started_at IS NULL OR started_at < ?",
|
||||
(_PLAUSIBLE_TIMESTAMP_FLOOR,),
|
||||
).fetchone()
|
||||
if implausible == session_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"
|
||||
)
|
||||
|
||||
(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
|
||||
|
||||
|
||||
def _recover_via_lost_and_found(
|
||||
*,
|
||||
source: Path,
|
||||
@@ -1530,6 +1590,21 @@ def _recover_via_lost_and_found(
|
||||
)
|
||||
verification["complete"] = False
|
||||
|
||||
# Structural checks cannot see a positional mis-mapping (#101409):
|
||||
# every row still inserts, so integrity/FK/FTS stay green. A
|
||||
# systematic timestamp violation is the semantic tell — surface it
|
||||
# so a mis-mapped salvage is never reported as verified.
|
||||
plausibility_conn = sqlite3.connect(str(output), isolation_level=None)
|
||||
try:
|
||||
plausibility_errors = _lost_and_found_plausibility_errors(
|
||||
plausibility_conn
|
||||
)
|
||||
finally:
|
||||
plausibility_conn.close()
|
||||
if plausibility_errors:
|
||||
verification["errors"].extend(plausibility_errors)
|
||||
verification["healthy"] = False
|
||||
|
||||
source_unchanged = (
|
||||
_source_fingerprint(source) == inspection["source_fingerprint"]
|
||||
)
|
||||
|
||||
@@ -569,3 +569,170 @@ def test_fingerprint_error_enumerates_parent_cli_session(
|
||||
assert "CLI session" in message
|
||||
assert "fresh shell" in message
|
||||
assert "snapshot" in message
|
||||
|
||||
|
||||
# ── issue #101409: mis-mapped salvage must not be reported verified ─────────
|
||||
|
||||
|
||||
def _map_salvage_rows(
|
||||
tmp_path: Path,
|
||||
*,
|
||||
blank_session_started_at: bool,
|
||||
blank_message_timestamp: bool,
|
||||
) -> sqlite3.Connection:
|
||||
"""Map synthetic lost_and_found cells into a fresh template DB.
|
||||
|
||||
With either ``blank_*`` flag the cells mimic an upgraded source's
|
||||
*physical* column order (#101409): whatever lands on the declared
|
||||
``started_at``/``timestamp`` position is not an epoch timestamp, so
|
||||
the NOT NULL substitute turns it into 0.0 on every row.
|
||||
"""
|
||||
|
||||
schema_ref = tmp_path / "schema-ref.db"
|
||||
SessionDB(db_path=schema_ref).close()
|
||||
schema = sqlite3.connect(str(schema_ref))
|
||||
try:
|
||||
sessions_columns = [
|
||||
str(row[1]) for row in schema.execute("PRAGMA table_info(sessions)")
|
||||
]
|
||||
messages_columns = [
|
||||
str(row[1]) for row in schema.execute("PRAGMA table_info(messages)")
|
||||
]
|
||||
finally:
|
||||
schema.close()
|
||||
current_width = len(sessions_columns)
|
||||
|
||||
lf_path = tmp_path / "lost_and_found.db"
|
||||
lf_conn = sqlite3.connect(str(lf_path), isolation_level=None)
|
||||
try:
|
||||
lf_cells = ", ".join(f"c{i}" for i in range(current_width))
|
||||
lf_conn.execute(
|
||||
"CREATE TABLE lost_and_found (rootpgno INTEGER, pgno INTEGER, "
|
||||
"nfield INTEGER, id INTEGER, " + lf_cells + ")"
|
||||
)
|
||||
|
||||
def insert(nfield: int, rowid: int, values: list) -> None:
|
||||
padded = list(values) + [None] * (current_width - len(values))
|
||||
placeholders = ", ".join("?" for _ in range(4 + current_width))
|
||||
lf_conn.execute(
|
||||
"INSERT INTO lost_and_found VALUES (" + placeholders + ")",
|
||||
[2, 5, nfield, rowid, *padded],
|
||||
)
|
||||
|
||||
def session_row(session_id: str) -> list:
|
||||
# title is UNIQUE (idx_sessions_title_unique) — keep it distinct
|
||||
# per row so the probe isolates timestamp mis-mapping.
|
||||
row = {
|
||||
"id": session_id,
|
||||
"source": "telegram",
|
||||
"started_at": None
|
||||
if blank_session_started_at
|
||||
else 1_754_000_000.0,
|
||||
"message_count": 2,
|
||||
"title": f"mis-mapped probe {session_id}",
|
||||
}
|
||||
return [row.get(column) for column in sessions_columns]
|
||||
|
||||
for index in range(3):
|
||||
insert(
|
||||
current_width,
|
||||
index + 1,
|
||||
session_row(f"20260101_01010{index}_aaa00{index}"),
|
||||
)
|
||||
|
||||
for index in range(2):
|
||||
message = {
|
||||
"id": None,
|
||||
"session_id": "20260101_010100_aaa000",
|
||||
"role": "user",
|
||||
"content": "payload",
|
||||
"timestamp": None
|
||||
if blank_message_timestamp
|
||||
else 1_754_000_100.0 + index,
|
||||
}
|
||||
insert(
|
||||
23,
|
||||
100 + index,
|
||||
[message.get(column) for column in messages_columns[:23]],
|
||||
)
|
||||
finally:
|
||||
lf_conn.close()
|
||||
|
||||
output = tmp_path / "mapped.db"
|
||||
SessionDB(db_path=output).close()
|
||||
lf_conn = sqlite3.connect(str(lf_path), isolation_level=None)
|
||||
dest = sqlite3.connect(str(output), isolation_level=None)
|
||||
try:
|
||||
dest.execute("PRAGMA foreign_keys=OFF")
|
||||
map_lost_and_found_rows(lf_conn, dest)
|
||||
finally:
|
||||
lf_conn.close()
|
||||
dest.close()
|
||||
return sqlite3.connect(str(output), isolation_level=None)
|
||||
|
||||
|
||||
def test_plausibility_gate_flags_positional_mis_mapping(
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
"""A salvage whose timestamps all landed below the epoch floor was
|
||||
mapped onto the wrong columns and must be flagged, not verified
|
||||
(#101409)."""
|
||||
|
||||
conn = _map_salvage_rows(
|
||||
tmp_path,
|
||||
blank_session_started_at=True,
|
||||
blank_message_timestamp=False,
|
||||
)
|
||||
try:
|
||||
# The mapper happily inserted every row; structural checks pass.
|
||||
assert conn.execute("SELECT COUNT(*) FROM sessions").fetchone()[0] == 3
|
||||
assert conn.execute(
|
||||
"SELECT COUNT(*) FROM sessions WHERE started_at = 0.0"
|
||||
).fetchone()[0] == 3
|
||||
|
||||
errors = session_recovery._lost_and_found_plausibility_errors(conn)
|
||||
assert len(errors) == 1
|
||||
assert "sessions.started_at" in errors[0]
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
|
||||
def test_plausibility_gate_flags_mis_mapped_message_timestamps(
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
conn = _map_salvage_rows(
|
||||
tmp_path,
|
||||
blank_session_started_at=False,
|
||||
blank_message_timestamp=True,
|
||||
)
|
||||
try:
|
||||
errors = session_recovery._lost_and_found_plausibility_errors(conn)
|
||||
assert len(errors) == 1
|
||||
assert "messages.timestamp" in errors[0]
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
|
||||
def test_plausibility_gate_passes_correctly_mapped_salvage(
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
"""Well-mapped rows — and partially damaged ones (a torn cell on some
|
||||
rows is expected salvage noise) — must not trip the gate: it fires
|
||||
only on a *systematic* violation."""
|
||||
|
||||
conn = _map_salvage_rows(
|
||||
tmp_path,
|
||||
blank_session_started_at=False,
|
||||
blank_message_timestamp=False,
|
||||
)
|
||||
try:
|
||||
# Damage one of three sessions the way a torn cell would.
|
||||
conn.execute(
|
||||
"UPDATE sessions SET started_at = 0.0 WHERE id = ?",
|
||||
("20260101_010101_aaa001",),
|
||||
)
|
||||
conn.commit()
|
||||
|
||||
assert session_recovery._lost_and_found_plausibility_errors(conn) == []
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
Reference in New Issue
Block a user