fix(wiki): make wiki_migrate --dry-run transactionally neutral - #108
Merged
Conversation
migrate_wiki() called conn.commit() unconditionally after all four passes, so passes 1-3 (upsert_page, delete_links_from/upsert_link, resolve_unresolved_links) persisted their writes even when dry_run=True. Only pass 4 (purge_ghost_pages) had its own dry_run gate. Fixes #105: dry-run rolls back the connection instead of committing when dry_run=True, making the whole run transactionally neutral end to end. Co-Authored-By: Claude <noreply@anthropic.com>
cdeust
added a commit
that referenced
this pull request
Jul 14, 2026
…dening Cut immediately after v4.14.0 to carry a fix that landed on main just after that tag: - fix(mcp): safe_handler raises ToolError from handler failure instead of returning a schema-violating error dict, so a failing MCP tool call surfaces its real diagnostic instead of the generic "Output validation error" every client previously saw regardless of cause (49f29e9, c7dfc24). - fix(wiki): strip quoted/duplicated-label frontmatter scalar values that broke FS↔PG sync (#104, 53712df). - fix(wiki): wiki_migrate --dry-run made transactionally neutral (#108, b60d268). - feat(wiki): write_governed_page validates and normalizes frontmatter at write time instead of only at read time (#109, 3eb43f2). - refactor(wiki): wiki_pages.py extracted into cohesive collaborators, no behaviour change (#111, 28145f0). Marketplace: cortex 4.14.0 -> 4.14.1 (metadata + pin + version_note). Pre-tag guard (exact tree): LongMemEval MRR 0.9167/R@10 0.982 (matches v4.14.0 0.9166/0.982); LoCoMo 3-run mean MRR 0.7984/R@10 0.9142 (v4.14.0 was 0.8005/0.9131 — delta MRR -0.0021 is smaller than v4.14.0's own documented +-0.0025 single-run variance, not a regression; R@10 improved); BEAM 0.5406 (3 runs, spread 0.0001) vs v4.14.0's 0.5471. BEAM deviation investigated: same-day A/B + bisect + control rerun (9 runs total) show an intra-day production-DB variance band of 0.5391-0.5445 spanned by BOTH trees — a same-day v4.14.0 control run measured 0.5445, then on rerun ~2.5h later 0.5391 (SAME code, SAME tree), and a bisect at 53712df/49f29e98 landed inside that band with no monotonic step at any commit boundary. Code effect excluded on two independent grounds: (1) the bench import closure (BenchmarkDB -> mcp_server.core.memory_ingest + mcp_server.core.pg_recall) is untouched by any 4.14.1 commit — the full mcp_server diff between v4.14.0 and this tag is confined to wiki_*, handlers/consolidation/ page_io.py, handlers/wiki_migrate.py, handlers/wiki_write.py, and tool_error_handler.py, none of which is in that import graph; (2) the SAME v4.14.0 tree reproduces both the high and low ends of the band on rerun. BEAM is not gated by reproduce.sh's floor check (proxy metric, within-system comparison only) so this does not block the tag. Floor-gate result (reproduce.sh::check_floors logic, applied directly to the JSONs since no runner prints it itself; tolerance 0.005): LongMemEval R@10 0.9820 vs floor 0.9820 PASS; LongMemEval MRR 0.9167 vs floor 0.9140 PASS; LoCoMo R@10 0.9142 vs floor 0.9150 PASS; LoCoMo MRR 0.7984 vs floor 0.8050 FAIL (-0.0066, exceeds the 0.005 tolerance) — flagged, not hidden: fails the codebase's hardcoded absolute floor while passing the release's actual stopping rule (no regression beyond noise vs the v4.14.0 baseline). The absolute floor has not been re-baselined since the 4.11-4.13 measurement era and LoCoMo MRR has trended down release-over-release with no corresponding retrieval-code change; recommend recalibrating the floor in a future session against an isolated benchmark DB rather than the live growing production store used here. Reranker active. Results committed under benchmarks/results/repro/20260714-v4.14.1-pretag/ (11 files: 5 primary runs, beam-100K-rep2/rep3, and 4 investigation runs from the bisect: beam-control-v4140, beam-bisect-53712df8, beam-bisect-49f29e98, beam-bisect-da3f2104-rep2). Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #105
Root cause
migrate_wiki()(mcp_server/handlers/wiki_migrate.py) calledconn.commit()unconditionally after all four passes. Only pass 4 (purge_ghost_pages) checkeddry_runinternally; passes 1–3 (_upsert_all_pages,_upsert_all_links,resolve_unresolved_links) wrote through the shared connection and the final commit persisted their writes regardless of the caller's intent. Verified this was the single point deciding transactional behavior (no commit/rollback inpg_store_wiki*.py).Fix
The commit/rollback decision is now dry_run-gated:
dry_run=True→conn.rollback(),dry_run=False→conn.commit(). One central branch makes the entire 4-pass run transactionally neutral under dry-run — a mechanism fix, not per-call-site patches.Invariant
Exactly one of commit/rollback is called per invocation, never both, never neither.
Tests
test_migrate_wiki_dry_run_rolls_back_never_commits— fails against pre-fix code (rollback called 0 times), passes with the fix.test_migrate_wiki_real_run_commits_never_rolls_back— mirror postcondition.Known limits
Unit-tier proof via MagicMock call-count assertions, consistent with the file's existing test tier; no live-Postgres end-to-end rollback proof (standard DB-API behavior). Flag for future direct callers: if
migrate_wikiwere ever handed a connection inside an outer transaction, the rollback would discard that pending work — not reachable via the MCP handler today (get_shared_storefresh per invocation).🤖 Generated with Claude Code