feat: v0.3.2 — full-fidelity archive export/import (the migration path) - #1
Conversation
wiki_export grows format="archive": one versioned, slug-keyed, id-free JSON carrying EVERYTHING — pages, revisions, links, ledger (strength / misconceptions / evidence), FSRS card state, review log. New wiki_import restores it VERBATIM (raw inserts, never through record_retrieval — an import is not retrieval, so strength and scheduling land exactly as exported; the ledger invariant holds). mode="merge" adds new slugs and skips collisions (reported); mode="replace" overwrites colliding pages; other pages never touched. Foreign/future-version files refused. Proven live: exported a 10-page/28-link/4-card corpus through the tool on one instance, imported on a fresh second instance — strength 0.15 and all four due cards arrived intact, not reset. 100 host-free tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
QA panel review — WARN
code-review-structural · head c2fb08624a96 · formal
Overall risk is moderate: the new archive import/export surface carries two confirmed correctness defects in store.py — replace-mode silently destroys other pages' links (the fix-first item; it contradicts the documented "Other pages are never touched."), and merge-mode grafts archive edges onto pages the import claims to have skipped — plus an undocumented result-key divergence in wiki_export and thin test coverage. The panel disagreed on store.py:671 (one finder called the graft arguably intended graph-merge); the verifier kept it minor with that caveat. Verification changed the picture on tests/test_archive.py:69: the tautology is real, but the adjacent content assert already guards the replace, so the "would still pass" harm is overstated — kept as a nit. Grounding caveat (a gap): the pinned SHA c2fb086 is orphaned (404 on SHA lookup), so verification used the current tip + PR diff; every quoted line was found verbatim, so no finding was unverifiable. No coverage gaps: store.py, tools.py, and both test files all carry findings.
Findings
| Severity | Location | Finding | Verified | |
|---|---|---|---|---|
| 🟡 | minor | store.py:595 |
In replace mode, deleting the colliding page cascade-wipes ALL local links incident to it in both directions, and since the archive's later link pass restores … | confirmed |
| 🟡 | minor | store.py:671 |
In merge mode, archive links whose endpoint slug was skipped as already-existing are still resolved against the local page and inserted, so a page the import c… | confirmed |
| 🟡 | minor | tools.py:239 |
wiki_export returns a different result-key schema depending on the format argument (dir/files for markdown vs path/pages/links/cards for archive), so callers p… | confirmed |
| 🟡 | minor | tests/test_tools.py:22 |
The new wiki_import tool and the wiki_export(format="archive") branch ship with no test exercising them through tools.py — test_archive.py drives only the stor… | confirmed |
| ⚪ | nit | tests/test_archive.py:69 |
The replace-mode assertion is tautological — it reduces to out["imported"] >= 1 and can never verify that the "attention" slug specifically was replaced, thoug… | confirmed |
findings JSON (machine-readable)
[
{
"file": "store.py",
"line": 595,
"severity": "minor",
"category": "correctness",
"claim": "In replace mode, deleting the colliding page cascade-wipes ALL local links incident to it in both directions, and since the archive's later link pass restores only edges whose both endpoints are in the archive, local pages NOT in the archive silently lose their edges to the replaced page \u2014 permanent local-graph data loss contradicting the documented 'Other pages are never touched.' (flagged by both the correctness and removed-behavior reviews)",
"evidence": "Docstring: \"mode=\\\"replace\\\" replaces any colliding page (cascade wipes its old revisions/cards/logs/links) with the archive's version. Other pages are never touched.\" \u2014 Code: self._conn.execute(\"DELETE FROM pages WHERE id = ?\", (existing[\"id\"],)) \u2014 Schema: from_page INTEGER NOT NULL REFERENCES pages(id) ON DELETE CASCADE / to_page INTEGER NOT NULL REFERENCES pages(id) ON DELETE CASCADE \u2014 __init__: self._conn.execute(\"PRAGMA foreign_keys = ON\") \u2014 the archive link pass re-inserts only archive-known edges, so a local-only page's edge into a replaced slug is destroyed and never restored.",
"verdict": "confirmed",
"note": "Re-read at current tip (pinned SHA orphaned; code present verbatim in PR diff + default branch). DELETE cascades links in both directions (both FK columns ON DELETE CASCADE, foreign_keys=ON); the link pass re-inserts only doc[\"links\"] edges with both endpoints resolving, so local-only edges to the replaced slug are permanently lost \u2014 contradicts the quoted docstring."
},
{
"file": "store.py",
"line": 671,
"severity": "minor",
"category": "correctness",
"claim": "In merge mode, archive links whose endpoint slug was skipped as already-existing are still resolved against the local page and inserted, so a page the import claims to have skipped gains foreign edges from the archive's graph \u2014 the skipped page's knowledge-map is mutated despite the documented 'SKIPS existing ones\u2026 Other pages are never touched.'",
"evidence": "a = self._conn.execute(\"SELECT id FROM pages WHERE slug = ?\", (ln.get(\"from\"),)).fetchone()\nb = self._conn.execute(\"SELECT id FROM pages WHERE slug = ?\", (ln.get(\"to\"),)).fetchone()\nif a and b:\n self._conn.execute(\"INSERT OR IGNORE INTO links (from_page, to_page, rel) VALUES (?, ?, ?)\", (a[\"id\"], b[\"id\"], ln.get(\"rel\") or \"related\")) \u2014 the slug lookup matches pages skipped in the merge loop (store.py:592-594), grafting the archive's edge onto the local page's graph.",
"verdict": "confirmed",
"note": "Mechanism verified in code: the post-loop link pass resolves by slug against pages never deleted in merge mode, so an edge to a skipped slug is inserted onto the local page. Note the panel split on intent \u2014 one finder called this arguably intended graph-merge \u2014 so keep the minor severity and the intent caveat in the synthesis."
},
{
"file": "tools.py",
"line": 239,
"severity": "minor",
"category": "conventions",
"claim": "wiki_export returns a different result-key schema depending on the format argument (dir/files for markdown vs path/pages/links/cards for archive), so callers parsing the tool's response get different keys per format and the docstring never documents the divergence.",
"evidence": "if format == \"archive\":\n target = out_dir or str(_data_dir(cfg) / \"export\")\n res = get_store().export_archive(Path(target) / \"wiki-archive.json\")\n return _ok(**res)\n if format != \"markdown\":\n return _err(\"format must be 'markdown' or 'archive'\")\n target = out_dir or str(_data_dir(cfg) / \"export\")\n n = get_store().export_markdown(target)\n return _ok(dir=target, files=n)",
"verdict": "confirmed",
"note": "Quoted code is verbatim in tools.py at the current tip. export_archive returns {path,pages,links,cards} (store.py) \u2192 _ok(**res); markdown returns _ok(dir,files). Docstring describes formats but not the divergent result keys."
},
{
"file": "tests/test_tools.py",
"line": 22,
"severity": "minor",
"category": "tests",
"claim": "The new wiki_import tool and the wiki_export(format=\"archive\") branch ship with no test exercising them through tools.py \u2014 test_archive.py drives only the store methods directly, and the only test_tools.py change adds a name to a registry set, leaving the get_store()/_data_dir/_ok/_err tool wiring untested.",
"evidence": "\"review_grade\",\n \"wiki_map\",\n \"wiki_export\",\n+ \"wiki_import\",\n \"wiki_research\",",
"verdict": "confirmed",
"note": "Diff + current tip confirm: the only test_tools.py change is adding \"wiki_import\" to EXPECTED_TOOLS; test_wiki_export_tool invokes wiki_export with default (markdown) format only; test_archive.py calls store.import_archive/export_archive directly. No test drives wiki_import or the archive branch through the tool layer."
},
{
"file": "tests/test_archive.py",
"line": 69,
"severity": "nit",
"category": "tests",
"claim": "The replace-mode assertion is tautological \u2014 it reduces to out[\"imported\"] >= 1 and can never verify that the \"attention\" slug specifically was replaced, though the adjacent content assert already guards the replace itself, so the consequence is limited to the redundant tautology (flagged by both the correctness and conventions reviews).",
"evidence": "out = dst.import_archive(tmp_path / \"a.json\", mode=\"replace\")\n assert \"attention\" in [s for s in (\"attention\",) if out[\"imported\"] >= 1]\n assert dst.get_page(\"attention\")[\"content_md\"].startswith(\"# Attention\") # replaced",
"verdict": "confirmed",
"note": "Tautology verified character-by-character: [s for s in (\"attention\",) if out[\"imported\"] >= 1] is [\"attention\"] iff imported>=1 else [], so the assert \u2261 imported>=1 and never checks the slug. BUT the consequence is overstated: the following assert (content_md.startswith(\"# Attention\") against prior local content \"local notes\") does guard the replace, so a silently skipped replace would fail the test. Keep at nit, qualify the harm."
}
]
Operator ask: build a corpus on a sandbox, then move it into the desktop app losslessly.
wiki_export(format="archive")→ one versioned, slug-keyed, id-freewiki-archive.json: pages, revisions, links, ledger (strength/misconceptions/evidence), FSRS card state, review log. The markdown export stays the human reading copy.wiki_import(path, mode)restores it verbatim — raw inserts, never throughrecord_retrieval, so imported strength/scheduling are restored, not re-earned (the ledger invariant extends to migration: an import is not retrieval).mergeskips colliding slugs (reported);replaceoverwrites them; other pages untouched. Foreign and future-version files refused.Live-proven: sandbox→sandbox migration through the actual tools — 10 pages / 28 links / 4 cards; strength 0.15 and all due-card scheduling arrived exact. 100 host-free tests (round-trip fidelity, merge/replace semantics, envelope validation).
🤖 Generated with Claude Code