Skip to content

docs: clarify build_merge persistence contract - #2867

Open
oleksii-tumanov wants to merge 1 commit into
Graphify-Labs:v8from
oleksii-tumanov:docs/build-merge-return-contract
Open

docs: clarify build_merge persistence contract#2867
oleksii-tumanov wants to merge 1 commit into
Graphify-Labs:v8from
oleksii-tumanov:docs/build-merge-return-contract

Conversation

@oleksii-tumanov

Copy link
Copy Markdown
Contributor

Summary

  • clarify that build_merge returns the merged graph without writing to graph_path
  • state that persistence remains the caller's responsibility

Fixes #2831

Result

The documented contract now matches the existing behavior: a merge can return a graph with 2 nodes while the input graph.json remains byte-identical with its original 1 node.

Testing

  • uv run --frozen pytest -q tests/test_build.py tests/test_build_merge_hyperedges_and_prune.py tests/test_build_merge_shrink_guard.py --tb=short (102 passed)
  • uv run --frozen pytest tests/ -q --tb=short --ignore=tests/test_ollama_retry_cap.py (4570 passed, 73 skipped; optional openai dependency unavailable locally)
  • uv run --frozen python -m tools.skillgen --check (134 artifacts matched)

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Graphify reviewed this change.

Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).

Formal verification. No changes could be formally verified in this run.


Graphify review — findings

Updates the build_merge docstring to state it returns the merged graph without persisting to graph_path, leaving writes to the caller. Note the docstring is the only change here—verify the actual save behavior matches this new contract, since the diff doesn't touch the function body.

No blocking issues surfaced. 2 lower-confidence candidates did not survive cross-model review.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 1025 functions depend on the 81 functions this change touches.

Health — this change adds coupling hotspots:

  • new: _rebuild_code() — 98 callers, 51 callees
  • new: build_from_json() — 174 callers, 18 callees
  • new: build_merge() — 46 callers, 14 callees
  • new: to_obsidian() — 29 callers, 12 callees
  • new: extract_files_direct() — 17 callers, 20 callees
  • new: to_wiki() — 41 callers, 7 callees
  • new: _call_claude_cli() — 31 callers, 9 callees
  • new: extract_corpus_parallel() — 26 callers, 10 callees
  • …and 38 more — each is listed as a finding

Verification — 1025 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: 669 function(s) in the blast radius were not formally verified this run

Formal verification

Could not verify: Could not verify build\_merge.

The verifier did not have enough to check build\_merge, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: parameter `graph_path` is annotated `str | Path | None` — outside the synthesizable primitive/collection set

· 46 more finding(s) on lines outside this diff (see the check run).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

build_merge() docstring says it saves to disk, but the function only returns

1 participant