fix(export): adopt a pre-manifest vault's own notes instead of orphaning them (#2863) - #2914
Conversation
…ing them (Graphify-Labs#2863) Graphify-Labs#1506 added .graphify_obsidian_manifest.json so a re-export can update the notes graphify owns while refusing to touch the user's. A vault created by an EARLIER graphify has no manifest, so _owned starts empty and every note graphify itself wrote last time reads as a user file. The re-export writes fresh notes BESIDE the stale ones, and warns that graphify "did not create" files it did. Reproduced on a 3-node graph: build a vault, drop the manifest to simulate a pre-Graphify-Labs#1506 vault, rename two nodes, re-export. before: 6 notes for a 3-node graph -- Alpha, Beta, "Beta renamed", Gamma, "Gamma renamed", community -- plus "WARNING: skipped 3 pre-existing file(s) graphify did not create" after: exactly the current generation, no stale notes, no false warning The migration runs once, only when no manifest exists, and adopts a file solely on graphify's own signature: - node notes carry YAML frontmatter with a tag in the graphify/ namespace; only that block is inspected, so a user's note that merely mentions graphify in prose is not adopted; - community notes carry NO frontmatter at all, so they are matched on graphify's filename prefix AND the Dataview query it writes into every one. Requiring both keeps a user's own _COMMUNITY_*.md from being adopted on the name alone. Scoped to top-level *.md, the only files identifiable from their content. .obsidian/graph.json is deliberately NOT adopted. graphify writes one, but so does Obsidian, and with no manifest there is no way to tell whose it is -- guessing wrong costs the user their vault configuration, which is a far worse outcome than one file staying unowned. It is therefore still reported as skipped on a pre-manifest vault; that warning is now correct, because for that one file graphify genuinely cannot confirm it was the author.
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 2 advisory finding(s) below merit a look before merge.
Formal verification. 1 change(s) tested, no difference found (not proven).
Graphify review — findings
Adds a one-time migration in to_obsidian so vaults exported before the ownership manifest existed don't accumulate a second generation of notes: when no .graphify_obsidian_manifest.json is found, _adopt_pre_manifest_notes seeds _owned from top-level *.md files that _is_graphify_note identifies by their graphify/ frontmatter tag (or, for community notes, the filename prefix plus the Dataview query marker). Deliberately excludes .obsidian/graph.json from adoption since Obsidian also writes it and ownership is ambiguous without a manifest. Adds tests/test_obsidian_vault_migration.py covering the bug, user-note safety, unchanged manifest/fresh-vault behavior, and the detector.
Worth a look
- Detector treats non-tag YAML list items as graphify ownership tags —
graphify/export.py:581· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- Graphify tag detector authorizes any frontmatter list item —
graphify/export.py:599· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 513 functions depend on the 71 functions this change touches.
Health — this change adds coupling hotspots:
- new:
_rebuild_code()— 98 callers, 50 callees - new:
build_merge()— 46 callers, 14 callees - new:
to_obsidian()— 36 callers, 13 callees - new:
to_json()— 50 callers, 7 callees - new:
dispatch_command()— 2 callers, 119 callees - new:
_make_graph()— 32 callers, 6 callees - new:
run_pipeline()— 8 callers, 13 callees - new:
to_canvas()— 17 callers, 4 callees - …and 5 more — each is listed as a finding
Verification — 513 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 360 function(s) in the blast radius were not formally verified this run
Formal verification
No difference found (not proven): No behavior difference found in to\_obsidian (not a proof).
The verifier ran both versions of to\_obsidian on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
· 13 more finding(s) on lines outside this diff (see the check run).
Fixes #2863.
Thanks @ — the diagnosis was exact, including that the leftover notes carry graphify's own frontmatter, which is what makes them recoverable.
The bug
#1506 added
.graphify_obsidian_manifest.jsonso a re-export can update the notes graphify owns while refusing to touch the user's. A vault created by an earlier graphify has no manifest, so_ownedstarts empty — and every note graphify itself wrote last time reads as a user file.Reproduced on a 3-node graph: build a vault, drop the manifest to stand in for a pre-#1506 vault, rename two nodes, re-export.
The warning is the part I'd flag as worst: it names files graphify wrote itself and tells the user it didn't. Someone cleaning up by hand has no way to tell the two generations apart, because both carry graphify frontmatter.
The change
A one-time migration, run only when no manifest exists, adopting a file solely on graphify's own signature. Two shapes, because the notes are not alike:
graphify/namespace. Only that block is inspected, so a user's note that merely mentions graphify in prose is not adopted — there is a test for exactly that._COMMUNITY_notes.mdis not adopted on the name alone.Scoped to top-level
*.md— the only files identifiable from their content..obsidian/graph.jsonis deliberately left unownedgraphify writes one, but so does Obsidian, and with no manifest there is no way to tell whose it is. Guessing wrong costs the user their vault configuration, which is far worse than one file staying unowned.
So a pre-manifest vault still reports one skipped file. That warning is now correct: for that file graphify genuinely cannot confirm it was the author. Worth knowing rather than being surprised by it.
Tests
tests/test_obsidian_vault_migration.py(16 tests): no second generation after a re-export, stale notes for renamed nodes gone, no false "did not create" warning, and the manifest written so the migration happens once.Then the safety half, which is the part that matters: a user's own
Alpha.mdsurvives untouched, a note merely mentioning graphify is not adopted, a user's_COMMUNITY_*.mdwithout the query marker is not adopted,.obsidian/graph.jsonis preserved byte-for-byte, a vault that already has a manifest is unaffected, and a fresh directory still works.Reverting
export.pyand keeping the tests fails 4 (6 more skip, since the detector does not exist pre-fix).Validation
Windows 11, Python 3.12, branched off
b2cd362(0.9.48).16 failed, 4835 passed->16 failed, 4851 passed. Identical failure set — no regressions; the +16 are the new tests. Both runs were taken on this machine against this commit, rather than reusing an older baseline.