From 33346da381537a9c8a93669ca2356af941a65333 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 21 Sep 2026 02:15:10 -0400 Subject: [PATCH] =?UTF-8?q?fix(backup):=20a=20restore=20could=20lose=20a?= =?UTF-8?q?=20column=20and=20still=20report=20success=20=E2=80=94=20code?= =?UTF-8?q?=5Fshapes=20lost=20three=20(#4197)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The column guard (#3182) makes a dropped column unexpressible on the way OUT. Nothing watched the way back IN, and that is the worse half: an export gap leaves an obviously thin backup, an import gap means holding a complete, correct file and restoring an incomplete database from it, with a success message. WHAT IT FOUND, the first time it ran. `code_shapes` was exporting `reason_code`, `recheck_at` and `diverges_from` and importing none of them. A restored ledger would have carried every judgment's verdict and not the code for WHY — the column the accounting reads to tell a scoped-css exemption from convention-plumbing — with every recheck flag cleared and every divergence pointer gone. `diverges_from` is a FK to notes.id, so it is re-mapped rather than carried: the raw source id would point at whatever snippet took that number in the destination, which is wrong rather than missing and is the milestone 333 trap one table over. WHY THE GUARD WAS ONE-SIDED. Not an oversight — the code was. Export goes through per-table pure helpers, so a test can hand one a stand-in and read which keys came out. `_restore_v2` built all 26 models inline in one 690-line procedural function, and there was no per-table unit to hand anything to. So the construction moved out, into a `_build_*` helper per table taking the exported row plus the id maps built so far. What deliberately did NOT move is the loops, the flushes and the id-map bookkeeping: that is the order-dependent part, where a mistake is a restore that half-works, and it gains nothing from being split. Returning None is "skip" and a None field is "degrade" — which one a table wants stays the table's own call, because both are right somewhere: a shape event without its project says nothing, while a usage event without one is still a real pull and dropping it would deflate the pull-through the table exists to report. The refactor was checked to be behaviour-preserving before the guard went in: all 26 constructor kwarg sets identical to HEAD, and all 28 skip conditions accounted for — 25 now in builders, 3 in loops that construct nothing (the rule_systems raw insert and the two in the final project patch). The guard then composes the two halves end to end: export a stand-in row, feed THAT dict to the builder, read which columns the model actually received. `test_the_usage_importer_restores_the_reading_project` read the source of `_restore_v2` with `inspect.getsource` because there was nothing to call; it is replaced by tests that call the builders and assert the skip/degrade behaviour directly. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy --- src/scribe/services/backup.py | 1206 +++++++++++++++++++++------------ tests/test_services_backup.py | 204 +++++- 2 files changed, 945 insertions(+), 465 deletions(-) diff --git a/src/scribe/services/backup.py b/src/scribe/services/backup.py index b7190c0..80bd84b 100644 --- a/src/scribe/services/backup.py +++ b/src/scribe/services/backup.py @@ -236,6 +236,81 @@ _COLUMN_EXCLUSIONS: dict[str, set[str]] = { } +# The same accounting for the way BACK IN (#4197). `_COLUMN_EXCLUSIONS` above +# makes a dropped column unexpressible on the way out; this one does it for the +# import, where the same mistake is worse — an export gap leaves an obviously +# thin backup, an import gap means holding a complete, correct file and +# restoring an incomplete database from it, with a success message. +# +# A name here means the `_build_*` helper for that table deliberately does not +# pass it. THREE REASONS ARE LEGITIMATE and nothing else is: +# +# 1. The column is not in the backup at all, because `_COLUMN_EXCLUSIONS` +# above leaves it out. Nothing can restore what was never written. +# 2. The database issues the value: a surrogate `id`, which must be re-issued +# because the destination's ids are its own. +# 3. It is written LATER in the same restore, by something that is not a +# construction — the project's design-system pointer and inception record +# point at rows created after the project itself. +# +# "We forgot" is not on the list, which is the whole point. `code_shapes` +# carried three that had no reason at all — reason_code, recheck_at and +# diverges_from, exported and dropped — and this table is how they were found. +_IMPORT_COLUMN_EXCLUSIONS: dict[str, set[str]] = { + "users": {"id"}, + "projects": { + "id", + # Not exported (see above), so not restorable. + "deleted_at", "deleted_batch_id", "forge_connection_id", + # Reason 3: both point at rows this restore creates AFTER the project, + # so they are patched in the final pass with their ids re-mapped. + "design_system_id", "inception", + }, + "milestones": {"id", "deleted_at", "deleted_batch_id"}, + "notes": {"id", "deleted_at", "deleted_batch_id"}, + "task_logs": {"id"}, + "note_drafts": {"id"}, + "note_versions": {"id"}, + # A composite primary key (user_id, key) — no surrogate to re-issue, and + # every column travels. + "settings": set(), + "rulebooks": {"id", "deleted_at", "deleted_batch_id"}, + "rulebook_topics": {"id", "deleted_at", "deleted_batch_id"}, + "rules": {"id", "deleted_at", "deleted_batch_id"}, + "rule_versions": {"id"}, + # `canonical_id` IS set, from the exported slug. The timestamps are not + # exported, so the row is re-stamped as of the restore. + "systems": { + "id", "deleted_at", "deleted_batch_id", "created_at", "updated_at", + }, + "canonical_systems": { + "id", "deleted_at", "deleted_batch_id", "created_at", "updated_at", + }, + "record_systems": {"id", "created_at"}, + "note_supersessions": {"id", "created_at"}, + "rule_relations": {"id", "created_at"}, + "note_usage_events": {"id"}, + "rule_usage_events": {"id"}, + "retrieval_tuning_events": {"id"}, + "design_systems": { + "id", "deleted_at", "deleted_batch_id", "created_at", "updated_at", + }, + "design_tokens": { + "id", "deleted_at", "deleted_batch_id", "created_at", "updated_at", + }, + "repo_bindings": {"id", "created_at", "updated_at"}, + "code_shapes": { + "id", + # Not exported: the proposer's standing suggestion is derived, and the + # next refresh recomputes it against the restored snippet ids. + "proposed_snippet_id", "proposal_basis", "proposal_score", + "proposal_group", "proposed_at", "proposed_sha", + }, + "code_shape_events": {"id"}, + "code_shape_uses": {"id"}, +} + + def _dt(val: str | None) -> datetime: return datetime.fromisoformat(val) if val else datetime.now(timezone.utc) @@ -918,6 +993,591 @@ async def export_user_backup(user_id: int) -> dict: } +# --------------------------------------------------------------------------- +# Per-table builders — the import side's twin of the row helpers above (#4197) +# --------------------------------------------------------------------------- +# +# The column guard (#3182) makes a dropped column unexpressible on the way OUT +# because export goes through per-table pure helpers: hand one a stand-in and +# read which KEYS came back. Nothing watched the way back IN, and that is the +# worse half — an export gap leaves an obviously thin backup, an import gap +# means holding a complete, correct file and restoring an incomplete database +# from it, with a success message. +# +# The guard was one-sided because the code was. `_restore_v2` was one long +# procedural function with the model construction inlined in each loop, so +# there was no per-table unit to hand a stand-in to. These builders are that +# unit. Each takes an exported row plus the id maps built so far and returns +# either the model instance to add or None for a row that cannot be restored. +# +# WHAT DELIBERATELY DID NOT MOVE. The loops, the flushes and the id-map +# bookkeeping stay in `_restore_v2`: that is the order-dependent part, where a +# mistake is a restore that half-works, and it gains nothing from being split. +# Only the construction moved, because the construction is the only thing the +# guard has to see. +# +# RETURNING None IS "SKIP", A None FIELD IS "DEGRADE", and the difference is +# per table by design. `code_shape_events` skips a row whose project will not +# map — a shape event without its project says nothing. `note_usage_events` +# degrades to None — the project is optional, null already means "not +# reported", and dropping the row would lose a real pull and deflate the +# pull-through the table exists to report. Both are right for their table, so +# each builder states its own policy rather than a rule being imposed on all. + + +class _Maps: + """The source-id to new-id maps a restore accumulates as it goes. + + One object rather than nine locals so a builder's signature says what it + depends on, and so the guard can hand every builder the same fully + populated stand-in without knowing which maps that table happens to read. + """ + + __slots__ = ( + "users", "projects", "milestones", "notes", "rulebooks", "topics", + "rules", "systems", "design_systems", "shapes", "canonical_by_slug", + ) + + def __init__(self) -> None: + self.users: dict[int, int] = {} + self.projects: dict[int, int] = {} + self.milestones: dict[int, int] = {} + self.notes: dict[int, int] = {} + self.rulebooks: dict[int, int] = {} + self.topics: dict[int, int] = {} + self.rules: dict[int, int] = {} + self.systems: dict[int, int] = {} + self.design_systems: dict[int, int] = {} + self.shapes: dict[int, int] = {} + self.canonical_by_slug: dict[str, int] = {} + + +def _build_user(row: dict, maps: _Maps) -> User: + """Users are the root of every map, so nothing can fail to resolve.""" + return User( + username=row["username"], + email=row.get("email"), + password_hash=row.get("password_hash"), + oauth_sub=row.get("oauth_sub"), + role=row.get("role", "user"), + session_version=row.get("session_version", 1), + created_at=_dt(row.get("created_at")), + ) + + +def _build_project(row: dict, maps: _Maps) -> Project | None: + uid = maps.users.get(row.get("user_id", 0)) + if uid is None: + return None + return Project( + user_id=uid, + title=row.get("title", ""), + description=row.get("description", ""), + goal=row.get("goal", ""), + status=row.get("status", "active"), + color=row.get("color"), + created_at=_dt(row.get("created_at")), + updated_at=_dt(row.get("updated_at")), + ) + + +def _build_milestone(row: dict, maps: _Maps) -> Milestone | None: + uid = maps.users.get(row.get("user_id", 0)) + pid = maps.projects.get(row.get("project_id", 0)) + if uid is None or pid is None: + return None + return Milestone( + user_id=uid, + project_id=pid, + title=row.get("title", ""), + description=row.get("description"), + body=row.get("body"), + status=row.get("status", "active"), + order_index=row.get("order_index", 0), + created_at=_dt(row.get("created_at")), + updated_at=_dt(row.get("updated_at")), + ) + + +def _build_note(row: dict, maps: _Maps) -> Note | None: + """`parent_id` and `arose_from_id` land NULL and are patched afterwards. + + Both are ids in the SOURCE database (#3182), and the target note may not + have been created yet when this row is built. + """ + uid = maps.users.get(row.get("user_id", 0)) + if uid is None: + return None + return Note( + user_id=uid, + title=row.get("title", ""), + body=row.get("body", ""), + tags=row.get("tags", []), + parent_id=None, + arose_from_id=None, + description=row.get("description"), + note_type=row.get("note_type") or "note", + task_kind=row.get("task_kind") or "work", + data=row.get("data"), + started_at=_dt_or_none(row.get("started_at")), + completed_at=_dt_or_none(row.get("completed_at")), + recurrence_rule=row.get("recurrence_rule"), + recurrence_next_spawn_at=_dt_or_none(row.get("recurrence_next_spawn_at")), + project_id=maps.projects.get(row["project_id"]) if row.get("project_id") else None, + milestone_id=maps.milestones.get(row["milestone_id"]) if row.get("milestone_id") else None, + status=row.get("status"), + priority=row.get("priority"), + due_date=_d(row.get("due_date")), + created_at=_dt(row.get("created_at")), + updated_at=_dt(row.get("updated_at")), + verify_with=row.get("verify_with"), + expires_when=row.get("expires_when"), + # _dt_or_none — see the note on the other restore path. + verified_at=_dt_or_none(row.get("verified_at")), + ) + + +def _build_task_log(row: dict, maps: _Maps) -> TaskLog | None: + uid = maps.users.get(row.get("user_id", 0)) + tid = maps.notes.get(row.get("task_id", 0)) + if uid is None or tid is None: + return None + return TaskLog( + user_id=uid, + task_id=tid, + content=row.get("content", ""), + duration_minutes=row.get("duration_minutes"), + created_at=_dt(row.get("created_at")), + updated_at=_dt(row.get("updated_at")), + ) + + +def _build_note_draft(row: dict, maps: _Maps) -> NoteDraft | None: + uid = maps.users.get(row.get("user_id", 0)) + nid = maps.notes.get(row.get("note_id", 0)) + if uid is None or nid is None: + return None + return NoteDraft( + user_id=uid, + note_id=nid, + proposed_body=row.get("proposed_body", ""), + original_body=row.get("original_body", ""), + instruction=row.get("instruction", ""), + scope=row.get("scope", "document"), + created_at=_dt(row.get("created_at")), + updated_at=_dt(row.get("updated_at")), + ) + + +def _build_note_version(row: dict, maps: _Maps) -> NoteVersion | None: + uid = maps.users.get(row.get("user_id", 0)) + nid = maps.notes.get(row.get("note_id", 0)) + if uid is None or nid is None: + return None + return NoteVersion( + user_id=uid, + note_id=nid, + title=row.get("title", ""), + body=row.get("body", ""), + tags=row.get("tags", []), + pin_kind=row.get("pin_kind"), + pin_label=row.get("pin_label"), + created_at=_dt(row.get("created_at")), + ) + + +def _build_setting(row: dict, maps: _Maps) -> Setting | None: + uid = maps.users.get(row.get("user_id", 0)) + if uid is None: + return None + return Setting(user_id=uid, key=row["key"], value=row.get("value", "")) + + +def _build_retrieval_tuning_event(row: dict, maps: _Maps) -> RetrievalTuningEvent | None: + """No id remapping beyond the user: `surface` is a registry NAME, not a + foreign key, which is what lets this history survive a restore into an + install whose row ids all differ.""" + uid = maps.users.get(row.get("user_id") or 0) + if uid is None: + return None + return RetrievalTuningEvent( + user_id=uid, + surface=row.get("surface", ""), + dial=row.get("dial", ""), + old_value=row.get("old_value"), + new_value=row.get("new_value"), + actor=row.get("actor") or "model", + reason=row.get("reason", ""), + # .get with no default, deliberately (v17): an archive written before + # the stamp existed has no key here, and None is the right answer for + # it — the same "unstamped" a pre-#4104 row carries in place. + embedding_model=row.get("embedding_model"), + shape_version=row.get("shape_version"), + created_at=_dt(row.get("created_at")), + ) + + +def _build_rulebook(row: dict, maps: _Maps) -> Rulebook | None: + uid = maps.users.get(row.get("owner_user_id", 0)) + if uid is None: + return None + return Rulebook( + owner_user_id=uid, + title=row.get("title", ""), + description=row.get("description", ""), + created_at=_dt(row.get("created_at")), + updated_at=_dt(row.get("updated_at")), + ) + + +def _build_topic(row: dict, maps: _Maps) -> RulebookTopic | None: + rbid = maps.rulebooks.get(row.get("rulebook_id", 0)) + if rbid is None: + return None + return RulebookTopic( + rulebook_id=rbid, + title=row.get("title", ""), + description=row.get("description"), + order_index=row.get("order_index", 0), + created_at=_dt(row.get("created_at")), + updated_at=_dt(row.get("updated_at")), + ) + + +def _build_rule(row: dict, maps: _Maps) -> Rule | None: + """A topic-rule (topic_id) XOR a project-rule (project_id). Neither + mapping surviving means the rule is orphaned and is not restored.""" + topic = maps.topics.get(row["topic_id"]) if row.get("topic_id") else None + proj = maps.projects.get(row["project_id"]) if row.get("project_id") else None + if topic is None and proj is None: + return None + return Rule( + topic_id=topic, + project_id=proj, + title=row.get("title", ""), + statement=row.get("statement", ""), + why=row.get("why") or None, + how_to_apply=row.get("how_to_apply") or None, + when_to_apply=row.get("when_to_apply") or None, + # A file written before milestone 394 carries `tier` and `always_on`; + # neither is read. Dropping a field the schema no longer has is the + # tolerant direction — an archive records what WAS, and refusing it + # because it remembers a deleted column would make every pre-394 + # backup unrestorable. + # + # Same shape, same reason: a file written before 0098 has no kind, and + # every rule in it was a rule. Defaulting the other way would restore + # an old backup with things that had always bound quietly no longer + # binding. + kind=row.get("kind") or "rule", + verify_with=row.get("verify_with") or None, + expires_when=row.get("expires_when") or None, + # Restored as-is, NOT reset to null. `verified_at` records when someone + # last ran the check; a restore does not make that untrue, and clearing + # it would put every constraint at the top of the sweep with nothing + # having actually changed. + verified_at=_dt_or_none(row.get("verified_at")), + # Remapped through the note map like every other note edge. Exported + # since 0088 but dropped on the way back in until milestone 312 — a + # restore silently lost every rule's provenance link. SET NULL + # semantics apply here too: a source note that didn't restore leaves + # the rule intact. + arose_from_id=maps.notes.get(row.get("arose_from_id") or 0), + order_index=row.get("order_index", 0), + created_at=_dt(row.get("created_at")), + updated_at=_dt(row.get("updated_at")), + ) + + +def _build_canonical_system(row: dict, maps: _Maps) -> CanonicalSystem | None: + """Matched on SLUG, not id. This install already has the standard + vocabulary from its migrations, so the common case creates nothing and the + restore simply learns which local id each slug is; only an entry an admin + added on the source instance is built here.""" + slug = row.get("slug") or "" + if not slug or slug in maps.canonical_by_slug: + return None + return CanonicalSystem( + name=row.get("name", ""), + slug=slug, + description=row.get("description"), + order_index=row.get("order_index", 0), + ) + + +def _build_rule_relation(row: dict, maps: _Maps) -> RuleRelation | None: + """Both ends must map, and a self-edge is not a relation.""" + src = maps.rules.get(row.get("from_rule_id", 0)) + dst = maps.rules.get(row.get("to_rule_id", 0)) + if src is None or dst is None or src == dst: + return None + return RuleRelation( + from_rule_id=src, + to_rule_id=dst, + kind=row.get("kind", "co_surfaces"), + note=row.get("note") or None, + ) + + +def _build_rule_version(row: dict, maps: _Maps) -> RuleVersion | None: + rid = maps.rules.get(row.get("rule_id", 0)) + if rid is None: + return None + return RuleVersion( + rule_id=rid, + # Unlike NoteVersion, an unmappable user does NOT drop the row. + # user_id is the ACTOR and is nullable by design: the column is SET + # NULL precisely so history outlives the account that wrote it. + # Skipping here would delete the record the FK preserves. + user_id=maps.users.get(row.get("user_id") or 0), + title=row.get("title", ""), + statement=row.get("statement", ""), + why=row.get("why"), + how_to_apply=row.get("how_to_apply"), + when_to_apply=row.get("when_to_apply"), + # NOT defaulted, unlike the rule itself. A version records what was; + # absent means nobody wrote it down, and inventing "rule" here would + # put an artifact where a measurement belongs. + kind=row.get("kind"), + verify_with=row.get("verify_with"), + expires_when=row.get("expires_when"), + created_at=_dt(row.get("created_at")), + ) + + +def _build_system(row: dict, maps: _Maps) -> System | None: + uid = maps.users.get(row.get("user_id", 0)) + pid = maps.projects.get(row.get("project_id", 0)) + if uid is None or pid is None: + return None + return System( + user_id=uid, + project_id=pid, + name=row.get("name", ""), + description=row.get("description"), + color=row.get("color"), + status=row.get("status", "active"), + order_index=row.get("order_index", 0), + # An unknown slug restores UNMAPPED rather than failing: the System and + # its records are the payload, the mapping is an aid. + canonical_id=maps.canonical_by_slug.get(row.get("canonical_slug") or ""), + ) + + +def _build_record_system(row: dict, maps: _Maps) -> RecordSystem | None: + nid = maps.notes.get(row.get("note_id", 0)) + sid = maps.systems.get(row.get("system_id", 0)) + if nid is None or sid is None: + return None + return RecordSystem(note_id=nid, system_id=sid) + + +def _build_note_supersession(row: dict, maps: _Maps) -> NoteSupersession | None: + """Both ends must map. A claim is about a PAIR — half of one is not a + weaker claim, it is a dangling row pointing at whatever note happens to + hold that id next.""" + new_id = maps.notes.get(row.get("superseder_id", 0)) + old_id = maps.notes.get(row.get("superseded_id", 0)) + if new_id is None or old_id is None or new_id == old_id: + return None + return NoteSupersession(superseder_id=new_id, superseded_id=old_id) + + +def _build_design_system(row: dict, maps: _Maps) -> DesignSystem | None: + """The export orders these parent-first, so a parent's new id is always in + the map by the time a child needs it — and a child whose parent is missing + lands as a root rather than failing the whole restore.""" + uid = maps.users.get(row.get("owner_user_id", 0)) + if uid is None: + return None + return DesignSystem( + owner_user_id=uid, + title=row.get("title", ""), + description=row.get("description"), + guidance=row.get("guidance"), + parent_id=maps.design_systems.get(row.get("parent_id") or 0), + ) + + +def _build_design_token(row: dict, maps: _Maps) -> DesignToken | None: + dsid = maps.design_systems.get(row.get("design_system_id", 0)) + if dsid is None: + return None + return DesignToken( + design_system_id=dsid, + name=row.get("name", ""), + value_by_mode=row.get("value_by_mode") or {}, + group_name=row.get("group_name"), + purpose=row.get("purpose"), + rationale=row.get("rationale"), + supersedes=row.get("supersedes") or [], + order_index=row.get("order_index", 0), + ) + + +def _build_usage_event(row: dict, maps: _Maps) -> NoteUsageEvent | None: + """Kept because pull-through is the evidence base for whether recall works + at all, and it is only ever accumulated — a restore that dropped it would + silently reset that measurement to zero while everything still looked + fine.""" + nid = maps.notes.get(row.get("note_id", 0)) + if nid is None: + return None + return NoteUsageEvent( + user_id=maps.users.get(row.get("user_id") or 0), + note_id=nid, + event=row.get("event", ""), + source=row.get("source", ""), + # DEGRADES to None rather than dropping the row: unlike a shape event, + # a usage event without a project is still a real pull, and discarding + # it would deflate the pull-through this table exists to report. Null + # already means "not reported". + project_id=(maps.projects.get(row["project_id"]) + if row.get("project_id") else None), + created_at=_dt(row.get("created_at")), + ) + + +def _build_rule_usage_event(row: dict, maps: _Maps) -> RuleUsageEvent | None: + """Resolved through the RULE map, not the note map. A rule id run through + the note map would either drop (best case) or land on whatever note took + that number, producing telemetry that is wrong rather than missing and + that nothing downstream could detect (milestone 333).""" + rid = maps.rules.get(row.get("rule_id", 0)) + if rid is None: + return None + return RuleUsageEvent( + user_id=maps.users.get(row.get("user_id") or 0), + rule_id=rid, + event=row.get("event", ""), + source=row.get("source", ""), + # `.get(...) or None` rather than a bare default: an archive written + # before 0106 has no key at all, and one written after may carry "" for + # a non-departure row. Both mean "no reason", and both must land as + # NULL so the readout does not have to tell an empty string from an + # absent one. + detail=(row.get("detail") or None), + created_at=_dt(row.get("created_at")), + ) + + +def _build_repo_binding(row: dict, maps: _Maps) -> RepoBinding | None: + """Small, but losing these means every bound repo quietly stops loading + its project at session start.""" + uid = maps.users.get(row.get("user_id", 0)) + pid = maps.projects.get(row.get("project_id", 0)) + if uid is None or pid is None: + return None + return RepoBinding( + user_id=uid, + project_id=pid, + repo_key=row.get("repo_key", ""), + ref=row.get("ref"), + ) + + +def _build_code_shape(row: dict, maps: _Maps) -> CodeShape | None: + """The ledger's classifications are judgments worth carrying. A judgment + whose snippet target didn't survive the re-mapping (canonical/instance/ + variant with a gone snippet) is downgraded to unclassified so it rejoins + the todo honestly instead of dangling; exempt needs no target and keeps.""" + pid = maps.projects.get(row.get("project_id", 0)) + if pid is None: + return None + status = row.get("status", "unclassified") + snippet_id = maps.notes.get(row.get("snippet_id") or 0) + classified_by = row.get("classified_by") + classified_at = row.get("classified_at") + if status in ("canonical", "instance", "variant") and snippet_id is None: + status = "unclassified" + classified_by = None + classified_at = None + return CodeShape( + project_id=pid, + repo_key=row.get("repo_key", ""), + path=row.get("path", ""), + symbol=row.get("symbol", ""), + kind=row.get("kind", "sym"), + status=status, + snippet_id=snippet_id, + reason=row.get("reason"), + # Exported since #2874 and dropped on the way back in until #4197 — + # every judged row restored with its verdict and without the code for + # WHY, which is the column the accounting reads to tell a scoped-css + # exemption from a convention-plumbing one. Kept on a downgrade for + # the same reason `reason` is: the free text and its code are one + # statement, and the existing downgrade clears the judgment's author + # and date, not its argument. + reason_code=row.get("reason_code"), + classified_by=classified_by, + classified_at=_dt(classified_at) if classified_at else None, + first_seen_commit=row.get("first_seen_commit", ""), + last_seen_commit=row.get("last_seen_commit", ""), + vanished_at=_dt(row["vanished_at"]) if row.get("vanished_at") else None, + # Fingerprints restore; proposals (#2792) deliberately do not — they + # are mechanical, and the next refresh recomputes them against the + # restored snippet ids. + signature=row.get("signature", ""), + body_sha=row.get("body_sha", ""), + classified_sha=row.get("classified_sha", ""), + # Both exported and both dropped until #4197. `recheck_at` is the + # standing "this judgment asks to be confirmed again" flag, so losing + # it restores a tree that looks settled and is not. + recheck_at=_dt(row["recheck_at"]) if row.get("recheck_at") else None, + # RE-MAPPED, not carried: it is a FK to notes.id like `snippet_id`, + # and a raw source id would point at whatever snippet took that + # number in the destination — wrong rather than missing, which is the + # milestone 333 trap one table over. + diverges_from=maps.notes.get(row.get("diverges_from") or 0), + created_at=_dt(row.get("created_at")), + updated_at=_dt(row.get("updated_at")), + ) + + +def _build_code_shape_event(row: dict, maps: _Maps) -> CodeShapeEvent | None: + """The ledger's history rides its shapes. `snippet_id` is kept as the + history's own claim (FK-free by design) but re-mapped when the snippet + survived, so a restored timeline points at restored records. + + SKIPS a row whose project will not map, unlike its usage-event sibling: a + shape event without its project says nothing.""" + shape_id = maps.shapes.get(row.get("shape_id") or 0) + pid = maps.projects.get(row.get("project_id", 0)) + if shape_id is None or pid is None: + return None + old_sid = row.get("snippet_id") + return CodeShapeEvent( + shape_id=shape_id, + project_id=pid, + path=row.get("path", ""), + symbol=row.get("symbol", ""), + kind=row.get("kind", "sym"), + event=row.get("event", "classified"), + status=row.get("status"), + snippet_id=maps.notes.get(old_sid, old_sid) if old_sid else None, + classified_by=row.get("classified_by"), + reason=row.get("reason"), + commit=row.get("commit", ""), + at=_dt(row.get("at")), + ) + + +def _build_code_shape_use(row: dict, maps: _Maps) -> CodeShapeUse | None: + """Consumption edges (#2870) ride their shape AND their snippet — both + ends must have survived, or the edge is no longer a fact.""" + shape_id = maps.shapes.get(row.get("shape_id") or 0) + snippet_id = maps.notes.get(row.get("snippet_id") or 0) + if shape_id is None or snippet_id is None: + return None + return CodeShapeUse( + shape_id=shape_id, + snippet_id=snippet_id, + basis=row.get("basis", "import"), + evidence=row.get("evidence"), + created_at=_dt(row.get("created_at")), + ) + + # --------------------------------------------------------------------------- # Restore # --------------------------------------------------------------------------- @@ -1038,6 +1698,12 @@ async def _restore_v2(data: dict) -> dict: are silently skipped — those subsystems were removed. v3+ sections (rulebooks/topics/rules/join-tables) are guarded by data.get so a v2 payload restores without them. + + Every model is constructed by a `_build_*` helper above, never inline. + That is what gives the import side a per-table unit the column guard can + hand a stand-in to (#4197); what stays here is the ordering, the flushes + and the id-map bookkeeping, which is the part a split would only make + harder to read. """ stats: dict[str, int] = { "users": 0, "projects": 0, "milestones": 0, "notes": 0, @@ -1053,112 +1719,46 @@ async def _restore_v2(data: dict) -> dict: } async with async_session() as session: - user_id_map: dict[int, int] = {} - project_id_map: dict[int, int] = {} - milestone_id_map: dict[int, int] = {} - note_id_map: dict[int, int] = {} - rulebook_id_map: dict[int, int] = {} - topic_id_map: dict[int, int] = {} - rule_id_map: dict[int, int] = {} + maps = _Maps() # 1. Users for u_data in data.get("users", []): - old_id = u_data["id"] - user = User( - username=u_data["username"], - email=u_data.get("email"), - password_hash=u_data.get("password_hash"), - oauth_sub=u_data.get("oauth_sub"), - role=u_data.get("role", "user"), - session_version=u_data.get("session_version", 1), - created_at=_dt(u_data.get("created_at")), - ) + user = _build_user(u_data, maps) session.add(user) await session.flush() - user_id_map[old_id] = user.id + maps.users[u_data["id"]] = user.id stats["users"] += 1 # 2. Projects for p_data in data.get("projects", []): - mapped_uid = user_id_map.get(p_data.get("user_id", 0)) - if mapped_uid is None: + proj = _build_project(p_data, maps) + if proj is None: continue - proj = Project( - user_id=mapped_uid, - title=p_data.get("title", ""), - description=p_data.get("description", ""), - goal=p_data.get("goal", ""), - status=p_data.get("status", "active"), - color=p_data.get("color"), - created_at=_dt(p_data.get("created_at")), - updated_at=_dt(p_data.get("updated_at")), - ) session.add(proj) await session.flush() - project_id_map[p_data["id"]] = proj.id + maps.projects[p_data["id"]] = proj.id stats["projects"] += 1 # 3. Milestones for m_data in data.get("milestones", []): - mapped_uid = user_id_map.get(m_data.get("user_id", 0)) - mapped_pid = project_id_map.get(m_data.get("project_id", 0)) - if mapped_uid is None or mapped_pid is None: + ms = _build_milestone(m_data, maps) + if ms is None: continue - ms = Milestone( - user_id=mapped_uid, - project_id=mapped_pid, - title=m_data.get("title", ""), - description=m_data.get("description"), - body=m_data.get("body"), - status=m_data.get("status", "active"), - order_index=m_data.get("order_index", 0), - created_at=_dt(m_data.get("created_at")), - updated_at=_dt(m_data.get("updated_at")), - ) session.add(ms) await session.flush() - milestone_id_map[m_data["id"]] = ms.id + maps.milestones[m_data["id"]] = ms.id stats["milestones"] += 1 # 4a. Notes — first pass (no parent_id yet) notes_with_parents: list[tuple[int, int]] = [] # (new_note_id, old_parent_id) notes_with_origins: list[tuple[int, int]] = [] # (new_note_id, old_arose_from_id) for n_data in data.get("notes", []): - mapped_uid = user_id_map.get(n_data.get("user_id", 0)) - if mapped_uid is None: + note = _build_note(n_data, maps) + if note is None: continue - note = Note( - user_id=mapped_uid, - title=n_data.get("title", ""), - body=n_data.get("body", ""), - tags=n_data.get("tags", []), - parent_id=None, - arose_from_id=None, # patched below, same reason as parent_id - description=n_data.get("description"), - note_type=n_data.get("note_type") or "note", - task_kind=n_data.get("task_kind") or "work", - data=n_data.get("data"), - started_at=_dt_or_none(n_data.get("started_at")), - completed_at=_dt_or_none(n_data.get("completed_at")), - recurrence_rule=n_data.get("recurrence_rule"), - recurrence_next_spawn_at=_dt_or_none( - n_data.get("recurrence_next_spawn_at") - ), - project_id=project_id_map.get(n_data["project_id"]) if n_data.get("project_id") else None, - milestone_id=milestone_id_map.get(n_data["milestone_id"]) if n_data.get("milestone_id") else None, - status=n_data.get("status"), - priority=n_data.get("priority"), - due_date=_d(n_data.get("due_date")), - created_at=_dt(n_data.get("created_at")), - updated_at=_dt(n_data.get("updated_at")), - verify_with=n_data.get("verify_with"), - expires_when=n_data.get("expires_when"), - # _dt_or_none — see the note on the other restore path. - verified_at=_dt_or_none(n_data.get("verified_at")), - ) session.add(note) await session.flush() - note_id_map[n_data["id"]] = note.id + maps.notes[n_data["id"]] = note.id if n_data.get("parent_id"): notes_with_parents.append((note.id, n_data["parent_id"])) if n_data.get("arose_from_id"): @@ -1169,13 +1769,13 @@ async def _restore_v2(data: dict) -> dict: # parent_id always has been: these are ids in the SOURCE database # (#3182). An edge whose target did not survive stays NULL. for new_note_id, old_parent_id in notes_with_parents: - new_parent_id = note_id_map.get(old_parent_id) + new_parent_id = maps.notes.get(old_parent_id) if new_parent_id: note_row = await session.get(Note, new_note_id) if note_row: note_row.parent_id = new_parent_id for new_note_id, old_origin_id in notes_with_origins: - new_origin_id = note_id_map.get(old_origin_id) + new_origin_id = maps.notes.get(old_origin_id) if new_origin_id: note_row = await session.get(Note, new_note_id) if note_row: @@ -1183,65 +1783,34 @@ async def _restore_v2(data: dict) -> dict: # 5. TaskLogs for tl_data in data.get("task_logs", []): - mapped_uid = user_id_map.get(tl_data.get("user_id", 0)) - mapped_tid = note_id_map.get(tl_data.get("task_id", 0)) - if mapped_uid is None or mapped_tid is None: + tl = _build_task_log(tl_data, maps) + if tl is None: continue - tl = TaskLog( - user_id=mapped_uid, - task_id=mapped_tid, - content=tl_data.get("content", ""), - duration_minutes=tl_data.get("duration_minutes"), - created_at=_dt(tl_data.get("created_at")), - updated_at=_dt(tl_data.get("updated_at")), - ) session.add(tl) stats["task_logs"] += 1 # 6. NoteDrafts for nd_data in data.get("note_drafts", []): - mapped_uid = user_id_map.get(nd_data.get("user_id", 0)) - mapped_nid = note_id_map.get(nd_data.get("note_id", 0)) - if mapped_uid is None or mapped_nid is None: + nd = _build_note_draft(nd_data, maps) + if nd is None: continue - nd = NoteDraft( - user_id=mapped_uid, - note_id=mapped_nid, - proposed_body=nd_data.get("proposed_body", ""), - original_body=nd_data.get("original_body", ""), - instruction=nd_data.get("instruction", ""), - scope=nd_data.get("scope", "document"), - created_at=_dt(nd_data.get("created_at")), - updated_at=_dt(nd_data.get("updated_at")), - ) session.add(nd) stats["note_drafts"] += 1 # 7. NoteVersions for nv_data in data.get("note_versions", []): - mapped_uid = user_id_map.get(nv_data.get("user_id", 0)) - mapped_nid = note_id_map.get(nv_data.get("note_id", 0)) - if mapped_uid is None or mapped_nid is None: + nv = _build_note_version(nv_data, maps) + if nv is None: continue - nv = NoteVersion( - user_id=mapped_uid, - note_id=mapped_nid, - title=nv_data.get("title", ""), - body=nv_data.get("body", ""), - tags=nv_data.get("tags", []), - pin_kind=nv_data.get("pin_kind"), - pin_label=nv_data.get("pin_label"), - created_at=_dt(nv_data.get("created_at")), - ) session.add(nv) stats["note_versions"] += 1 # 8. Settings for s_data in data.get("settings", []): - mapped_uid = user_id_map.get(s_data.get("user_id", 0)) - if mapped_uid is None: + setting = _build_setting(s_data, maps) + if setting is None: continue - session.add(Setting(user_id=mapped_uid, key=s_data["key"], value=s_data.get("value", ""))) + session.add(setting) stats["settings"] += 1 # 8b. Retrieval tuning history (v16) — restored beside the settings it @@ -1249,114 +1818,41 @@ async def _restore_v2(data: dict) -> dict: # these rows are the argument for it. From milestone 416 those dials # move on the operator's behalf, so an install restored with the values # and without the reasons is one tuned by nobody it can name. - # - # No id remapping beyond the user: `surface` is a registry NAME, not a - # foreign key, which is what lets this history survive a restore into - # an install whose row ids all differ. for t_data in data.get("retrieval_tuning_events", []): - mapped_uid = user_id_map.get(t_data.get("user_id") or 0) - if mapped_uid is None: + event = _build_retrieval_tuning_event(t_data, maps) + if event is None: continue - session.add(RetrievalTuningEvent( - user_id=mapped_uid, - surface=t_data.get("surface", ""), - dial=t_data.get("dial", ""), - old_value=t_data.get("old_value"), - new_value=t_data.get("new_value"), - actor=t_data.get("actor") or "model", - reason=t_data.get("reason", ""), - # .get with no default, deliberately (v17): an archive written - # before the stamp existed has no key here, and None is the - # right answer for it — the same "unstamped" a pre-#4104 row - # carries in place. - embedding_model=t_data.get("embedding_model"), - shape_version=t_data.get("shape_version"), - created_at=_dt(t_data.get("created_at")), - )) + session.add(event) stats["retrieval_tuning_events"] += 1 # 9. Rulebooks (v3) for rb_data in data.get("rulebooks", []): - mapped_uid = user_id_map.get(rb_data.get("owner_user_id", 0)) - if mapped_uid is None: + rb = _build_rulebook(rb_data, maps) + if rb is None: continue - rb = Rulebook( - owner_user_id=mapped_uid, - title=rb_data.get("title", ""), - description=rb_data.get("description", ""), - created_at=_dt(rb_data.get("created_at")), - updated_at=_dt(rb_data.get("updated_at")), - ) session.add(rb) await session.flush() - rulebook_id_map[rb_data["id"]] = rb.id + maps.rulebooks[rb_data["id"]] = rb.id stats["rulebooks"] += 1 # 10. Topics (v3) for t_data in data.get("rulebook_topics", []): - mapped_rbid = rulebook_id_map.get(t_data.get("rulebook_id", 0)) - if mapped_rbid is None: + topic = _build_topic(t_data, maps) + if topic is None: continue - topic = RulebookTopic( - rulebook_id=mapped_rbid, - title=t_data.get("title", ""), - description=t_data.get("description"), - order_index=t_data.get("order_index", 0), - created_at=_dt(t_data.get("created_at")), - updated_at=_dt(t_data.get("updated_at")), - ) session.add(topic) await session.flush() - topic_id_map[t_data["id"]] = topic.id + maps.topics[t_data["id"]] = topic.id stats["rulebook_topics"] += 1 # 11. Rules (v3) — topic-rule (topic_id) XOR project-rule (project_id) for r_data in data.get("rules", []): - mapped_topic = topic_id_map.get(r_data["topic_id"]) if r_data.get("topic_id") else None - mapped_proj = project_id_map.get(r_data["project_id"]) if r_data.get("project_id") else None - if mapped_topic is None and mapped_proj is None: + rule = _build_rule(r_data, maps) + if rule is None: continue # orphaned — its parent didn't restore - rule = Rule( - topic_id=mapped_topic, - project_id=mapped_proj, - title=r_data.get("title", ""), - statement=r_data.get("statement", ""), - why=r_data.get("why") or None, - how_to_apply=r_data.get("how_to_apply") or None, - when_to_apply=r_data.get("when_to_apply") or None, - # A file written before milestone 394 carries `tier` and - # `always_on`; neither is read. Dropping a field the schema - # no longer has is the tolerant direction — an archive - # records what WAS, and refusing it because it remembers a - # deleted column would make every pre-394 backup - # unrestorable. Previously: always_on - # is the pre-0088 behaviour, so an old backup restores rules - # that bind exactly as they did when it was taken. - # Same shape, same reason: a file written before 0098 has no - # kind, and every rule in it was a rule. Defaulting the other - # way would restore an old backup with things that had always - # bound quietly no longer binding. - kind=r_data.get("kind") or "rule", - verify_with=r_data.get("verify_with") or None, - expires_when=r_data.get("expires_when") or None, - # Restored as-is, NOT reset to null. `verified_at` records - # when someone last ran the check; a restore does not make - # that untrue, and clearing it would put every constraint at - # the top of the sweep with nothing having actually changed. - verified_at=_dt_or_none(r_data.get("verified_at")), - # Remapped through note_id_map like every other note edge. - # Exported since 0088 but dropped on the way back in until - # milestone 312 — a restore silently lost every rule's - # provenance link. SET NULL semantics apply here too: a - # source note that didn't restore leaves the rule intact. - arose_from_id=note_id_map.get(r_data.get("arose_from_id") or 0), - order_index=r_data.get("order_index", 0), - created_at=_dt(r_data.get("created_at")), - updated_at=_dt(r_data.get("updated_at")), - ) session.add(rule) await session.flush() - rule_id_map[r_data["id"]] = rule.id + maps.rules[r_data["id"]] = rule.id stats["rules"] += 1 # 12-14. Rulebook subscriptions, rule and topic suppressions (v3-v14) @@ -1374,39 +1870,32 @@ async def _restore_v2(data: dict) -> dict: # --- v5 sections. Every one is data.get()-guarded, so a v2/v3/v4 # payload restores without them rather than failing on an absent key. - system_id_map: dict[int, int] = {} - - # 14c. The global area catalog, matched on SLUG. This install already - # has the standard vocabulary from its migrations, so the common case - # adds nothing and simply learns which local id each slug is; only an - # entry an admin added on the source instance is created here. Runs - # BEFORE systems, which resolve their mapping through this map. - canonical_id_by_slug: dict[str, int] = {} + # 14c. The global area catalog, matched on SLUG. Runs BEFORE systems, + # which resolve their mapping through this map. The map is seeded from + # what this install already has, so the common case creates nothing. existing_canonical = (await session.execute( select(CanonicalSystem).where(CanonicalSystem.deleted_at.is_(None)) )).scalars().all() for entry in existing_canonical: - canonical_id_by_slug[entry.slug] = entry.id + maps.canonical_by_slug[entry.slug] = entry.id for cs_data in data.get("canonical_systems", []): - slug = cs_data.get("slug") or "" - if not slug or slug in canonical_id_by_slug: + entry = _build_canonical_system(cs_data, maps) + if entry is None: continue - entry = CanonicalSystem( - name=cs_data.get("name", ""), slug=slug, - description=cs_data.get("description"), - order_index=cs_data.get("order_index", 0), - ) session.add(entry) await session.flush() - canonical_id_by_slug[slug] = entry.id + maps.canonical_by_slug[entry.slug] = entry.id stats["canonical_systems"] += 1 # 14d. A rule's area tags and its typed edges. Runs HERE, not beside the # rules in section 11, because it needs both maps: the rule ids from # there and the canonical slugs from 14c just above. + # + # This one is a raw insert on a join table with no model class, so it + # has no builder and no column guard — both ends ARE the row. for rs in data.get("rule_systems", []): - mapped_rule = rule_id_map.get(rs.get("rule_id", 0)) - canonical_id = canonical_id_by_slug.get(rs.get("canonical_slug") or "") + mapped_rule = maps.rules.get(rs.get("rule_id", 0)) + canonical_id = maps.canonical_by_slug.get(rs.get("canonical_slug") or "") if mapped_rule is None or canonical_id is None: continue await session.execute(rule_systems_t.insert().values( @@ -1415,286 +1904,129 @@ async def _restore_v2(data: dict) -> dict: stats["rule_systems"] += 1 for rr in data.get("rule_relations", []): - mapped_from = rule_id_map.get(rr.get("from_rule_id", 0)) - mapped_to = rule_id_map.get(rr.get("to_rule_id", 0)) - if mapped_from is None or mapped_to is None or mapped_from == mapped_to: + relation = _build_rule_relation(rr, maps) + if relation is None: continue - session.add(RuleRelation( - from_rule_id=mapped_from, to_rule_id=mapped_to, - kind=rr.get("kind", "co_surfaces"), note=rr.get("note") or None, - )) + session.add(relation) stats["rule_relations"] += 1 # A rule's edit history (milestone 323). Must come after the rules - # themselves — rule_id_map is only populated above — and both ids are + # themselves — the rule map is only populated above — and both ids are # ids in the SOURCE database, which is #3182's arose_from_id trap. for rv in data.get("rule_versions", []): - mapped_rid = rule_id_map.get(rv.get("rule_id", 0)) - if mapped_rid is None: + version = _build_rule_version(rv, maps) + if version is None: continue - # Unlike NoteVersion, an unmappable user does NOT drop the row. - # user_id is the ACTOR and is nullable by design: the column is - # SET NULL precisely so history outlives the account that wrote - # it. Skipping here would delete the record the FK preserves. - session.add(RuleVersion( - rule_id=mapped_rid, - user_id=user_id_map.get(rv.get("user_id") or 0), - title=rv.get("title", ""), - statement=rv.get("statement", ""), - why=rv.get("why"), - how_to_apply=rv.get("how_to_apply"), - when_to_apply=rv.get("when_to_apply"), - # NOT defaulted, unlike the rule above. A version records what - # was; absent means nobody wrote it down, and inventing "rule" - # here would put an artifact where a measurement belongs. - kind=rv.get("kind"), - verify_with=rv.get("verify_with"), - expires_when=rv.get("expires_when"), - created_at=_dt(rv.get("created_at")), - )) + session.add(version) stats["rule_versions"] += 1 # 15. Systems for sy_data in data.get("systems", []): - mapped_uid = user_id_map.get(sy_data.get("user_id", 0)) - mapped_pid = project_id_map.get(sy_data.get("project_id", 0)) - if mapped_uid is None or mapped_pid is None: + system = _build_system(sy_data, maps) + if system is None: continue - system = System( - user_id=mapped_uid, project_id=mapped_pid, - name=sy_data.get("name", ""), - description=sy_data.get("description"), - color=sy_data.get("color"), - status=sy_data.get("status", "active"), - order_index=sy_data.get("order_index", 0), - # An unknown slug restores UNMAPPED rather than failing: the - # System and its records are the payload, the mapping is an aid. - canonical_id=canonical_id_by_slug.get(sy_data.get("canonical_slug") or ""), - ) session.add(system) await session.flush() - system_id_map[sy_data["id"]] = system.id + maps.systems[sy_data["id"]] = system.id stats["systems"] += 1 - # 16. Record↔system links + # 16. Record<->system links for rs in data.get("record_systems", []): - mapped_nid = note_id_map.get(rs.get("note_id", 0)) - mapped_sid = system_id_map.get(rs.get("system_id", 0)) - if mapped_nid is None or mapped_sid is None: + link = _build_record_system(rs, maps) + if link is None: continue - session.add(RecordSystem(note_id=mapped_nid, system_id=mapped_sid)) + session.add(link) stats["record_systems"] += 1 # 16b. Supersession claims. Guarded by `data.get` like every other # post-v2 section, so a v5 or older payload restores cleanly without it. - # - # Both ends must map. A claim is about a PAIR — half of one is not a - # weaker claim, it is a dangling row pointing at whatever note happens - # to hold that id next. for sup in data.get("note_supersessions", []): - mapped_new = note_id_map.get(sup.get("superseder_id", 0)) - mapped_old = note_id_map.get(sup.get("superseded_id", 0)) - if mapped_new is None or mapped_old is None or mapped_new == mapped_old: + claim = _build_note_supersession(sup, maps) + if claim is None: continue - session.add( - NoteSupersession( - superseder_id=mapped_new, superseded_id=mapped_old - ) - ) + session.add(claim) stats["note_supersessions"] += 1 - # 17. Design systems. The export orders these parent-first, so a - # parent's new id is always in the map by the time a child needs it — - # no second pass, and a child whose parent is missing lands as a root - # rather than failing the whole restore. - design_system_id_map: dict[int, int] = {} + # 17. Design systems, parent-first (the export orders them that way). for ds_data in data.get("design_systems", []): - mapped_uid = user_id_map.get(ds_data.get("owner_user_id", 0)) - if mapped_uid is None: + design = _build_design_system(ds_data, maps) + if design is None: continue - design = DesignSystem( - owner_user_id=mapped_uid, - title=ds_data.get("title", ""), - description=ds_data.get("description"), - guidance=ds_data.get("guidance"), - parent_id=design_system_id_map.get(ds_data.get("parent_id") or 0), - ) session.add(design) await session.flush() - design_system_id_map[ds_data["id"]] = design.id + maps.design_systems[ds_data["id"]] = design.id stats["design_systems"] += 1 # 18. Design tokens for t_data in data.get("design_tokens", []): - mapped_dsid = design_system_id_map.get(t_data.get("design_system_id", 0)) - if mapped_dsid is None: + token = _build_design_token(t_data, maps) + if token is None: continue - session.add(DesignToken( - design_system_id=mapped_dsid, - name=t_data.get("name", ""), - value_by_mode=t_data.get("value_by_mode") or {}, - group_name=t_data.get("group_name"), - purpose=t_data.get("purpose"), - rationale=t_data.get("rationale"), - supersedes=t_data.get("supersedes") or [], - order_index=t_data.get("order_index", 0), - )) + session.add(token) stats["design_tokens"] += 1 - # 19. Usage events. Kept because pull-through is the evidence base for - # whether recall works at all, and it is only ever accumulated — a - # restore that dropped it would silently reset that measurement to zero - # while everything still looked fine. + # 19. Usage events for ev in data.get("note_usage_events", []): - mapped_nid = note_id_map.get(ev.get("note_id", 0)) - if mapped_nid is None: + event = _build_usage_event(ev, maps) + if event is None: continue - session.add(NoteUsageEvent( - user_id=user_id_map.get(ev.get("user_id") or 0), - note_id=mapped_nid, - event=ev.get("event", ""), - source=ev.get("source", ""), - # Degrades to None rather than dropping the row: unlike a shape - # event, a usage event without a project is still a real pull, - # and discarding it would deflate the pull-through this table - # exists to report. Null already means "not reported". - project_id=(project_id_map.get(ev["project_id"]) - if ev.get("project_id") else None), - created_at=_dt(ev.get("created_at")), - )) + session.add(event) stats["note_usage_events"] += 1 - # The rule twin — and the reason it is a separate table at all. - # Resolved through rule_id_map, NOT note_id_map. A rule id run through - # the note map would either drop (best case) or land on whatever note - # took that number, producing telemetry that is wrong rather than - # missing and that nothing downstream could detect (milestone 333). - # Must come after the rules themselves; rule_id_map is populated there. + # The rule twin. Must come after the rules themselves; the rule map is + # populated there. for ev in data.get("rule_usage_events", []): - mapped_rid = rule_id_map.get(ev.get("rule_id", 0)) - if mapped_rid is None: + event = _build_rule_usage_event(ev, maps) + if event is None: continue - session.add(RuleUsageEvent( - user_id=user_id_map.get(ev.get("user_id") or 0), - rule_id=mapped_rid, - event=ev.get("event", ""), - source=ev.get("source", ""), - # `.get(...) or None` rather than a bare default: an archive - # written before 0106 has no key at all, and one written - # after may carry "" for a non-departure row. Both mean "no - # reason", and both must land as NULL so the readout does not - # have to tell an empty string from an absent one. - detail=(ev.get("detail") or None), - created_at=_dt(ev.get("created_at")), - )) + session.add(event) stats["rule_usage_events"] += 1 - # 20. Repo bindings — small, but losing them means every bound repo - # quietly stops loading its project at session start. + # 20. Repo bindings for rb_data in data.get("repo_bindings", []): - mapped_uid = user_id_map.get(rb_data.get("user_id", 0)) - mapped_pid = project_id_map.get(rb_data.get("project_id", 0)) - if mapped_uid is None or mapped_pid is None: + binding = _build_repo_binding(rb_data, maps) + if binding is None: continue - session.add(RepoBinding( - user_id=mapped_uid, project_id=mapped_pid, - repo_key=rb_data.get("repo_key", ""), - ref=rb_data.get("ref"), - )) + session.add(binding) stats["repo_bindings"] += 1 - # 21. Code shapes (v7, #2787) — the ledger's classifications are - # judgments worth carrying. A judgment whose snippet target didn't - # survive the re-mapping (canonical/instance/variant with a gone - # snippet) is downgraded to unclassified so it rejoins the todo - # honestly instead of dangling; exempt needs no target and keeps. - shape_id_map: dict[int, int] = {} + # 21. Code shapes (v7, #2787) for cs_data in data.get("code_shapes", []): - mapped_pid = project_id_map.get(cs_data.get("project_id", 0)) - if mapped_pid is None: + shape = _build_code_shape(cs_data, maps) + if shape is None: continue - status = cs_data.get("status", "unclassified") - mapped_sid = note_id_map.get(cs_data.get("snippet_id") or 0) - classified_by = cs_data.get("classified_by") - classified_at = cs_data.get("classified_at") - if status in ("canonical", "instance", "variant") and mapped_sid is None: - status = "unclassified" - classified_by = None - classified_at = None - shape = CodeShape( - project_id=mapped_pid, - repo_key=cs_data.get("repo_key", ""), - path=cs_data.get("path", ""), - symbol=cs_data.get("symbol", ""), - kind=cs_data.get("kind", "sym"), - status=status, - snippet_id=mapped_sid, - reason=cs_data.get("reason"), - classified_by=classified_by, - classified_at=_dt(classified_at) if classified_at else None, - first_seen_commit=cs_data.get("first_seen_commit", ""), - last_seen_commit=cs_data.get("last_seen_commit", ""), - vanished_at=_dt(cs_data["vanished_at"]) if cs_data.get("vanished_at") else None, - # Fingerprints restore; proposals (#2792) deliberately do not — - # they are mechanical, and the next refresh recomputes them - # against the restored snippet ids. - signature=cs_data.get("signature", ""), - body_sha=cs_data.get("body_sha", ""), - classified_sha=cs_data.get("classified_sha", ""), - created_at=_dt(cs_data.get("created_at")), - updated_at=_dt(cs_data.get("updated_at")), - ) session.add(shape) await session.flush() if cs_data.get("id"): - shape_id_map[int(cs_data["id"])] = shape.id + maps.shapes[int(cs_data["id"])] = shape.id stats["code_shapes"] += 1 - # v8: the ledger's history rides its shapes. snippet_id is kept as - # the history's own claim (FK-free by design) but re-mapped when the - # snippet survived, so a restored timeline points at restored records. + # v8: the ledger's history rides its shapes. for ev in data.get("code_shape_events", []): - new_shape_id = shape_id_map.get(ev.get("shape_id") or 0) - mapped_pid = project_id_map.get(ev.get("project_id", 0)) - if new_shape_id is None or mapped_pid is None: + event = _build_code_shape_event(ev, maps) + if event is None: continue - old_sid = ev.get("snippet_id") - session.add(CodeShapeEvent( - shape_id=new_shape_id, - project_id=mapped_pid, - path=ev.get("path", ""), - symbol=ev.get("symbol", ""), - kind=ev.get("kind", "sym"), - event=ev.get("event", "classified"), - status=ev.get("status"), - snippet_id=note_id_map.get(old_sid, old_sid) if old_sid else None, - classified_by=ev.get("classified_by"), - reason=ev.get("reason"), - commit=ev.get("commit", ""), - at=_dt(ev.get("at")), - )) + session.add(event) stats["code_shape_events"] += 1 - # v9: consumption edges (#2870) ride their shape AND their snippet — - # both ends must have survived, or the edge is no longer a fact. + # v9: consumption edges (#2870). for use in data.get("code_shape_uses", []): - new_shape_id = shape_id_map.get(use.get("shape_id") or 0) - new_sid = note_id_map.get(use.get("snippet_id") or 0) - if new_shape_id is None or new_sid is None: + edge = _build_code_shape_use(use, maps) + if edge is None: continue - session.add(CodeShapeUse( - shape_id=new_shape_id, snippet_id=new_sid, - basis=use.get("basis", "import"), evidence=use.get("evidence"), - created_at=_dt(use.get("created_at")), - )) + session.add(edge) stats["code_shape_uses"] += 1 # v10: a project's design-system pointer and its inception record ride # the project but point at design systems and rulebooks restored AFTER # it — so they are written last, with ids re-mapped. An id that did # not survive drops out of the record rather than dangling. + # + # An UPDATE on a row this restore already wrote, not a construction, + # which is why it has no builder: there is no set of kwargs for a + # column guard to read. for p_data in data.get("projects", []): - new_pid = project_id_map.get(p_data.get("id") or 0) + new_pid = maps.projects.get(p_data.get("id") or 0) if new_pid is None: continue proj = await session.get(Project, new_pid) @@ -1702,7 +2034,7 @@ async def _restore_v2(data: dict) -> dict: continue old_ds = p_data.get("design_system_id") if old_ds: - proj.design_system_id = design_system_id_map.get(old_ds) + proj.design_system_id = maps.design_systems.get(old_ds) inception = p_data.get("inception") if isinstance(inception, dict): choices = dict(inception.get("choices") or {}) @@ -1715,7 +2047,7 @@ async def _restore_v2(data: dict) -> dict: # Same for subscribe_rulebooks since milestone 414. choices.pop("subscribe_rulebooks", None) ds = choices.get("design_system_id") - choices["design_system_id"] = design_system_id_map.get(ds) if ds else None + choices["design_system_id"] = maps.design_systems.get(ds) if ds else None proj.inception = {**inception, "choices": choices} await session.commit() diff --git a/tests/test_services_backup.py b/tests/test_services_backup.py index 33e63e9..fa4a328 100644 --- a/tests/test_services_backup.py +++ b/tests/test_services_backup.py @@ -188,8 +188,17 @@ def _stand_in(model): of a dict. Typed rather than a bare instance because the helpers call `.isoformat()` on the timestamps, which `None` does not have. """ + import itertools import sqlalchemy as sa + # DISTINCT integers, not a constant 1. The import guard feeds this row's + # exported form to the builder, and several builders reject a self-edge — + # a supersession or a rule relation whose two ends are the same id is not + # a weaker claim, it is a row pointing at itself. With one value shared by + # every column those builders would refuse a legitimate stand-in and the + # guard would read as a fixture bug. Only the KEYS matter to either guard. + ints = itertools.count(1) + row = model() for column in model.__table__.columns: t = column.type @@ -200,7 +209,7 @@ def _stand_in(model): elif isinstance(t, sa.Boolean): value = False elif isinstance(t, sa.Integer): - value = 1 + value = next(ints) elif isinstance(t, sa.ARRAY) or isinstance(getattr(t, "impl", None), sa.ARRAY): value = [] elif isinstance(t, (sa.Text, sa.String)): @@ -245,40 +254,179 @@ def test_every_column_is_exported_or_declared_excluded(table): ) -def test_the_usage_importer_restores_the_reading_project(): - """The column guard above checks the EXPORT side only. +def _import_guard_targets(): + """table -> (model, export helper, import builder). - A column can be exported faithfully and then dropped on the way back in, - which restores a backup that reports success and has quietly lost a - dimension — #3182's failure mode, one direction over. There is no general - import-side guard yet; this covers the column #4196 added, by source - inspection, because the behavioural path needs Postgres. - - It also pins the DEGRADE. `code_shape_events` skips a row whose project - will not map, because a shape event without its project says nothing. A - usage event is not like that: the project is optional by design and null - already means "not reported", so an unmappable one must restore as - unreported rather than vanish — dropping it would lose a real pull and - deflate the very pull-through this table exists to report. + Built from the export registry so the two cannot drift apart: a table with + a row helper and no builder shows up here as a KeyError with its own name + in it, rather than as a table nobody checks. """ - import inspect + builders = { + "users": backup._build_user, + "projects": backup._build_project, + "milestones": backup._build_milestone, + "notes": backup._build_note, + "task_logs": backup._build_task_log, + "note_drafts": backup._build_note_draft, + "note_versions": backup._build_note_version, + "settings": backup._build_setting, + "rulebooks": backup._build_rulebook, + "rulebook_topics": backup._build_topic, + "rules": backup._build_rule, + "rule_versions": backup._build_rule_version, + "systems": backup._build_system, + "canonical_systems": backup._build_canonical_system, + "record_systems": backup._build_record_system, + "note_supersessions": backup._build_note_supersession, + "rule_relations": backup._build_rule_relation, + "note_usage_events": backup._build_usage_event, + "rule_usage_events": backup._build_rule_usage_event, + "retrieval_tuning_events": backup._build_retrieval_tuning_event, + "design_systems": backup._build_design_system, + "design_tokens": backup._build_design_token, + "repo_bindings": backup._build_repo_binding, + "code_shapes": backup._build_code_shape, + "code_shape_events": backup._build_code_shape_event, + "code_shape_uses": backup._build_code_shape_use, + } + return { + table: (model, helper, builders[table]) + for table, (model, helper) in _column_guard_targets().items() + } - src = inspect.getsource(backup._restore_v2) - marker = 'for ev in data.get("note_usage_events", []):' - assert marker in src, "the usage import loop moved; this guard is blind" - block = src[src.index(marker):][:1200] - assert 'ev.get("project_id")' in block, ( - "the usage importer drops project_id — a restore would report success " - "and come back without the reading project" +def _everything_maps(row: dict) -> "backup._Maps": + """Id maps in which every id the row mentions resolves. + + The guard is about which COLUMNS a builder sets, not about what it does + when a foreign key is missing — that is the skip/degrade question, which + the tests below ask directly. So every lookup succeeds here, and a builder + that returned None would be a bug in the fixture rather than a finding. + """ + maps = backup._Maps() + ids = {v for v in row.values() if isinstance(v, int)} | {0, 1} + for name in ("users", "projects", "milestones", "notes", "rulebooks", + "topics", "rules", "systems", "design_systems", "shapes"): + getattr(maps, name).update({i: i + 1000 for i in ids}) + # `canonical_slug` only, never `slug`: a canonical_systems row is built + # exactly when its slug is NOT already known to the destination, so + # seeding it from the row's own slug would make that builder skip. + slug = row.get("canonical_slug") + if slug: + maps.canonical_by_slug[slug] = 7 + return maps + + +@pytest.mark.parametrize("table", sorted(_import_guard_targets())) +def test_every_exported_column_is_imported_or_declared_excluded(table): + """THE COLUMN GUARD, THE OTHER WAY (#4197). + + The export guard above makes a dropped column unexpressible on the way + OUT. Nothing watched the way back IN, and that is the worse half: an + export gap leaves an obviously thin backup, an import gap means holding a + complete, correct file and restoring an incomplete database from it, with + a success message. + + It was one-sided because the code was — `_restore_v2` built every model + inline, so there was no per-table unit to hand a stand-in to. There is + now, and this composes the two halves end to end: export a stand-in row, + feed THAT dict to the builder, and read which columns the constructed + model actually received. + + What it caught on the first run: `code_shapes` was exporting `reason_code`, + `recheck_at` and `diverges_from` and importing none of them. Every judged + shape would have restored with its verdict and without the code for why, + every recheck flag cleared, and every divergence pointer gone. + """ + model, helper, builder = _import_guard_targets()[table] + [row] = helper([_stand_in(model)]) + + built = builder(row, _everything_maps(row)) + assert built is not None, ( + f"{table}: the builder skipped a row whose ids all resolve — " + "the guard fixture is wrong, or the builder is" ) - assert "project_id_map" in block, ( - "project_id must be re-mapped; a raw id points at whatever project " - "happens to hold that number in the destination install" + + received = set(built.__dict__) - {"_sa_instance_state"} + columns = {c.name for c in model.__table__.columns} + missing = columns - received + declared = backup._IMPORT_COLUMN_EXCLUSIONS[table] + + assert missing == declared, ( + f"{table}: imported columns and _IMPORT_COLUMN_EXCLUSIONS disagree.\n" + f" dropped but not declared: {sorted(missing - declared)}\n" + f" declared but imported anyway: {sorted(declared - missing)}" ) - assert "continue" not in block.split('project_id=')[1][:200], ( - "an unmappable project must degrade to None, not skip the row" + + +def test_the_import_guard_covers_every_table_the_export_guard_does(): + """The two registries have to hold the same tables, or a table can be + guarded in one direction and silently unguarded in the other — which is + the state this whole pair of guards exists to end.""" + assert set(_import_guard_targets()) == set(_column_guard_targets()) + assert set(backup._IMPORT_COLUMN_EXCLUSIONS) == set(_column_guard_targets()) + + +def test_an_unmappable_project_degrades_a_usage_event_and_skips_a_shape_event(): + """The skip-or-degrade choice is per table, and both answers are right. + + `code_shape_events` SKIPS a row whose project will not map — a shape event + without its project says nothing. `note_usage_events` DEGRADES to None — + the project is optional by design, null already means "not reported", and + dropping the row would lose a real pull and deflate the very pull-through + the table exists to report. + + Until #4197 this was asserted by reading the source of `_restore_v2` with + `inspect.getsource`, because there was no unit to call. Now there is. + """ + maps = backup._Maps() + maps.notes[5] = 55 + maps.users[9] = 99 + maps.shapes[3] = 33 + # project 7 is deliberately absent from maps.projects + + event = backup._build_usage_event( + {"note_id": 5, "user_id": 9, "project_id": 7, + "event": "pulled", "source": "search"}, + maps, ) + assert event is not None, "an unmappable project must not drop a real pull" + assert event.project_id is None + assert event.note_id == 55 + + shape_event = backup._build_code_shape_event( + {"shape_id": 3, "project_id": 7, "path": "a.py", "symbol": "f"}, maps, + ) + assert shape_event is None, ( + "a shape event whose project did not map says nothing and must be " + "skipped, not restored project-less" + ) + + +def test_a_shapes_divergence_pointer_is_remapped_not_carried(): + """`diverges_from` is a FK to notes.id, like `snippet_id`. Carrying the + source id would point at whatever snippet took that number in the + destination — wrong rather than missing, and nothing downstream could + tell. It was dropped entirely until #4197; restoring it raw would have + been the worse fix.""" + maps = backup._Maps() + maps.projects[1] = 11 + maps.notes[4] = 44 + + shape = backup._build_code_shape( + {"project_id": 1, "status": "exempt", "diverges_from": 4, + "reason_code": "scoped-css", "recheck_at": "2026-01-01T00:00:00+00:00"}, + maps, + ) + assert shape.diverges_from == 44, "not re-mapped through the note map" + assert shape.reason_code == "scoped-css" + assert shape.recheck_at is not None + + # A pointer whose target did not survive lands NULL rather than dangling. + orphan = backup._build_code_shape( + {"project_id": 1, "status": "exempt", "diverges_from": 999}, maps, + ) + assert orphan.diverges_from is None def test_the_column_guard_covers_every_table_with_a_row_helper():