mirror of
https://github.com/safishamsi/graphify.git
synced 2026-10-15 16:39:43 +00:00
feat(hooks): install a post-merge hook to heal merge-driver-resurrected symbols (#2418)
The graph.json union merge driver can resurrect a symbol deleted on one side of a merge; nothing re-extracts that file unless it is touched again, so the phantom node persisted. graphify hook install now also registers a post-merge hook that runs an unscoped background rebuild, re-deriving the graph from the merged tree and dropping the stale node. Install/uninstall/status go through the same hook machinery as the post-commit/post-checkout hooks. Squashed from #3556 with authorship preserved. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
1 parent
97db005309
commit
31cc79219c
4 files changed
+169
-8
No files matched your search
@@ -994,7 +994,7 @@ def _run_cli() -> None:
|
||||
print(" (or set NEO4J_PASSWORD instead of --password to keep it off argv)")
|
||||
print(" export falkordb emit Cypher or push to FalkorDB [--graph PATH] [--push URI] [--user U] [--password P]")
|
||||
print(" (or set FALKORDB_PASSWORD instead of --password to keep it off argv)")
|
||||
print(" hook install install post-commit/post-checkout git hooks (all platforms)")
|
||||
print(" hook install install post-commit/post-checkout/post-merge git hooks (all platforms)")
|
||||
print(" hook uninstall remove git hooks")
|
||||
print(" hook status check if git hooks are installed")
|
||||
print(
|
||||
|
||||
+78
-5
@@ -9,6 +9,8 @@ _HOOK_MARKER = "# graphify-hook-start"
|
||||
_HOOK_MARKER_END = "# graphify-hook-end"
|
||||
_CHECKOUT_MARKER = "# graphify-checkout-hook-start"
|
||||
_CHECKOUT_MARKER_END = "# graphify-checkout-hook-end"
|
||||
_MERGE_HOOK_MARKER = "# graphify-merge-hook-start"
|
||||
_MERGE_HOOK_MARKER_END = "# graphify-merge-hook-end"
|
||||
|
||||
# __PINNED_PYTHON__ is replaced at install time with the absolute path of the
|
||||
# Python interpreter that ran `graphify hook install`. For uv-tool and pipx
|
||||
@@ -552,6 +554,60 @@ echo "[graphify] Branch switched - launching background rebuild (log: $_GRAPHIFY
|
||||
"""
|
||||
|
||||
|
||||
_MERGE_SCRIPT = """\
|
||||
# graphify-merge-hook-start
|
||||
# Auto-rebuilds the knowledge graph (code only) after a merge, unscoped.
|
||||
# The git merge driver graphify registers for graph.json does a pure union
|
||||
# of both sides, which can resurrect a symbol that was deleted on one side
|
||||
# (#2418). Nothing else ever re-extracts that symbol's file unless it is
|
||||
# touched again, so the phantom node would otherwise persist indefinitely;
|
||||
# an unscoped rebuild here re-derives the graph from the merged tree and
|
||||
# drops it like any other stale node.
|
||||
# Installed by: graphify hook install
|
||||
(
|
||||
|
||||
# Deterministic clustering: networkx louvain iterates string-keyed sets whose
|
||||
# order is randomized per-process by PYTHONHASHSEED, so community assignments
|
||||
# churn run-to-run. Pinning it makes graphify-out reproducible.
|
||||
export PYTHONHASHSEED=0
|
||||
__VIZ_LIMIT_EXPORT__
|
||||
# Git for Windows/MSYS hooks can inherit fragile pipe handles from GUI clients
|
||||
# and agent shells. Keep hook-triggered rebuilds sequential by default there;
|
||||
# explicit GRAPHIFY_MAX_WORKERS still wins for users who want parallelism.
|
||||
if [ -n "${WINDIR:-}" ] || [ -n "${MSYSTEM:-}" ]; then
|
||||
export GRAPHIFY_MAX_WORKERS="${GRAPHIFY_MAX_WORKERS:-1}"
|
||||
fi
|
||||
|
||||
# Only run if graphify-out/ exists (graph has been built before)
|
||||
if [ ! -d "graphify-out" ]; then
|
||||
exit 0
|
||||
fi
|
||||
|
||||
# This hook only fires after a merge completed cleanly, so MERGE_HEAD is
|
||||
# already gone by the time it runs -- these checks are belt and braces for
|
||||
# any git version/workflow quirk that lands here mid rebase or cherry-pick
|
||||
# anyway, matching the commit and checkout hooks' own defensive posture.
|
||||
# git exports GIT_DIR to hooks; the rev-parse fallback only runs when invoked
|
||||
# by hand (each git exec costs 1s+ on AV-scanned Windows machines).
|
||||
GIT_DIR=${GIT_DIR:-$(git rev-parse --git-dir 2>/dev/null)}
|
||||
[ -d "$GIT_DIR/rebase-merge" ] && exit 0
|
||||
[ -d "$GIT_DIR/rebase-apply" ] && exit 0
|
||||
[ -f "$GIT_DIR/MERGE_HEAD" ] && exit 0
|
||||
[ -f "$GIT_DIR/CHERRY_PICK_HEAD" ] && exit 0
|
||||
|
||||
# Honor the same opt out the commit and checkout hooks already respect.
|
||||
[ "${GRAPHIFY_SKIP_HOOK:-0}" = "1" ] && exit 0
|
||||
|
||||
""" + _WORKTREE_GUARD + _PYTHON_DETECT + """
|
||||
_GRAPHIFY_LOG="${HOME}/.cache/graphify-rebuild.log"
|
||||
mkdir -p "$(dirname "$_GRAPHIFY_LOG")"
|
||||
export GRAPHIFY_REBUILD_LOG="$_GRAPHIFY_LOG"
|
||||
echo "[graphify] Merge completed - launching background rebuild (log: $_GRAPHIFY_LOG)"
|
||||
""" + _detached_launch(_REBUILD_BODY_CHECKOUT) + """)
|
||||
# graphify-merge-hook-end
|
||||
"""
|
||||
|
||||
|
||||
def _load_graphifyrc(root: Path) -> dict:
|
||||
"""Load ``<root>/.graphifyrc``; the parser lives in :mod:`graphify.rcfile`.
|
||||
|
||||
@@ -949,7 +1005,7 @@ def _user_hooks_dir(hooks_dir: Path) -> Path:
|
||||
|
||||
|
||||
def install(path: Path = Path(".")) -> str:
|
||||
"""Install graphify post-commit and post-checkout hooks in the nearest git repo."""
|
||||
"""Install graphify's post-commit, post-checkout, and post-merge hooks in the nearest git repo."""
|
||||
root = _git_root(path)
|
||||
if root is None:
|
||||
raise RuntimeError(f"No git repository found at or above {path.resolve()}")
|
||||
@@ -970,16 +1026,23 @@ def install(path: Path = Path(".")) -> str:
|
||||
pinned = _pinned_python()
|
||||
hook = _HOOK_SCRIPT.replace("__PINNED_PYTHON__", pinned).replace("__VIZ_LIMIT_EXPORT__", viz_export)
|
||||
checkout = _CHECKOUT_SCRIPT.replace("__PINNED_PYTHON__", pinned).replace("__VIZ_LIMIT_EXPORT__", viz_export)
|
||||
merge_hook = _MERGE_SCRIPT.replace("__PINNED_PYTHON__", pinned).replace("__VIZ_LIMIT_EXPORT__", viz_export)
|
||||
|
||||
commit_msg = _install_hook(hooks_dir, "post-commit", hook, _HOOK_MARKER, _HOOK_MARKER_END)
|
||||
checkout_msg = _install_hook(hooks_dir, "post-checkout", checkout, _CHECKOUT_MARKER, _CHECKOUT_MARKER_END)
|
||||
post_merge_msg = _install_hook(
|
||||
hooks_dir, "post-merge", merge_hook, _MERGE_HOOK_MARKER, _MERGE_HOOK_MARKER_END
|
||||
)
|
||||
merge_msg = _register_merge_driver(root)
|
||||
|
||||
return f"post-commit: {commit_msg}\npost-checkout: {checkout_msg}\nmerge driver: {merge_msg}"
|
||||
return (
|
||||
f"post-commit: {commit_msg}\npost-checkout: {checkout_msg}\n"
|
||||
f"post-merge: {post_merge_msg}\nmerge driver: {merge_msg}"
|
||||
)
|
||||
|
||||
|
||||
def uninstall(path: Path = Path(".")) -> str:
|
||||
"""Remove graphify post-commit and post-checkout hooks."""
|
||||
"""Remove graphify's post-commit, post-checkout, and post-merge hooks."""
|
||||
root = _git_root(path)
|
||||
if root is None:
|
||||
raise RuntimeError(f"No git repository found at or above {path.resolve()}")
|
||||
@@ -987,9 +1050,15 @@ def uninstall(path: Path = Path(".")) -> str:
|
||||
hooks_dir = _user_hooks_dir(_hooks_dir(root))
|
||||
commit_msg = _uninstall_hook(hooks_dir, "post-commit", _HOOK_MARKER, _HOOK_MARKER_END)
|
||||
checkout_msg = _uninstall_hook(hooks_dir, "post-checkout", _CHECKOUT_MARKER, _CHECKOUT_MARKER_END)
|
||||
post_merge_msg = _uninstall_hook(
|
||||
hooks_dir, "post-merge", _MERGE_HOOK_MARKER, _MERGE_HOOK_MARKER_END
|
||||
)
|
||||
merge_msg = _unregister_merge_driver(root)
|
||||
|
||||
return f"post-commit: {commit_msg}\npost-checkout: {checkout_msg}\nmerge driver: {merge_msg}"
|
||||
return (
|
||||
f"post-commit: {commit_msg}\npost-checkout: {checkout_msg}\n"
|
||||
f"post-merge: {post_merge_msg}\nmerge driver: {merge_msg}"
|
||||
)
|
||||
|
||||
|
||||
# The two per-install values in a graphify block: the interpreter that ran
|
||||
@@ -1056,9 +1125,13 @@ def status(path: Path = Path(".")) -> str:
|
||||
|
||||
commit = _check("post-commit", _HOOK_MARKER, _HOOK_MARKER_END, _HOOK_SCRIPT)
|
||||
checkout = _check("post-checkout", _CHECKOUT_MARKER, _CHECKOUT_MARKER_END, _CHECKOUT_SCRIPT)
|
||||
post_merge = _check("post-merge", _MERGE_HOOK_MARKER, _MERGE_HOOK_MARKER_END, _MERGE_SCRIPT)
|
||||
merge = _merge_driver_status(root)
|
||||
|
||||
res = f"post-commit: {commit}\npost-checkout: {checkout}\nmerge driver: {merge}"
|
||||
res = (
|
||||
f"post-commit: {commit}\npost-checkout: {checkout}\n"
|
||||
f"post-merge: {post_merge}\nmerge driver: {merge}"
|
||||
)
|
||||
if cfg_limit is not None:
|
||||
res += f"\nviz node limit: {cfg_limit}"
|
||||
return res
|
||||
+49
-2
@@ -7,7 +7,9 @@ import textwrap
|
||||
from types import SimpleNamespace
|
||||
from pathlib import Path
|
||||
import pytest
|
||||
from graphify.hooks import install, uninstall, status, _hooks_dir, _HOOK_MARKER, _CHECKOUT_MARKER
|
||||
from graphify.hooks import (
|
||||
install, uninstall, status, _hooks_dir, _HOOK_MARKER, _CHECKOUT_MARKER, _MERGE_HOOK_MARKER,
|
||||
)
|
||||
|
||||
|
||||
def _make_git_repo(tmp_path: Path) -> Path:
|
||||
@@ -114,6 +116,35 @@ def test_uninstall_removes_post_checkout_hook(tmp_path):
|
||||
assert not hook.exists()
|
||||
|
||||
|
||||
def test_install_creates_post_merge_hook(tmp_path):
|
||||
# #2418: a merge hook is needed to heal a symbol the union merge driver
|
||||
# resurrects on one side of a merge, since nothing else ever
|
||||
# re-extracts that file's nodes unless it is touched again.
|
||||
repo = _make_git_repo(tmp_path)
|
||||
install(repo)
|
||||
hook = repo / ".git" / "hooks" / "post-merge"
|
||||
assert hook.exists()
|
||||
assert _MERGE_HOOK_MARKER in hook.read_text()
|
||||
|
||||
|
||||
def test_install_post_merge_is_executable(tmp_path):
|
||||
repo = _make_git_repo(tmp_path)
|
||||
install(repo)
|
||||
hook = repo / ".git" / "hooks" / "post-merge"
|
||||
if os.name == "nt":
|
||||
assert hook.read_text(encoding="utf-8").startswith("#!/bin/sh\n")
|
||||
else:
|
||||
assert hook.stat().st_mode & 0o111
|
||||
|
||||
|
||||
def test_uninstall_removes_post_merge_hook(tmp_path):
|
||||
repo = _make_git_repo(tmp_path)
|
||||
install(repo)
|
||||
uninstall(repo)
|
||||
hook = repo / ".git" / "hooks" / "post-merge"
|
||||
assert not hook.exists()
|
||||
|
||||
|
||||
def test_status_shows_both_hooks(tmp_path):
|
||||
repo = _make_git_repo(tmp_path)
|
||||
install(repo)
|
||||
@@ -123,6 +154,17 @@ def test_status_shows_both_hooks(tmp_path):
|
||||
assert result.count("installed") >= 2
|
||||
|
||||
|
||||
def test_status_shows_post_merge_hook(tmp_path):
|
||||
repo = _make_git_repo(tmp_path)
|
||||
install(repo)
|
||||
result = status(repo)
|
||||
assert "post-merge: installed" in result
|
||||
|
||||
uninstall(repo)
|
||||
result = status(repo)
|
||||
assert "post-merge: not installed" in result
|
||||
|
||||
|
||||
|
||||
def test_hooks_dir_resolves_relative_git_hooks_path(tmp_path, monkeypatch):
|
||||
repo = _make_git_repo(tmp_path)
|
||||
@@ -229,12 +271,17 @@ import re # noqa: E402
|
||||
from graphify.hooks import ( # noqa: E402
|
||||
_HOOK_SCRIPT,
|
||||
_CHECKOUT_SCRIPT,
|
||||
_MERGE_SCRIPT,
|
||||
_REBUILD_BODY_COMMIT,
|
||||
_REBUILD_BODY_CHECKOUT,
|
||||
_detached_launch,
|
||||
)
|
||||
|
||||
_HOOK_SCRIPTS = [("post-commit", _HOOK_SCRIPT), ("post-checkout", _CHECKOUT_SCRIPT)]
|
||||
_HOOK_SCRIPTS = [
|
||||
("post-commit", _HOOK_SCRIPT),
|
||||
("post-checkout", _CHECKOUT_SCRIPT),
|
||||
("post-merge", _MERGE_SCRIPT),
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("name,script", _HOOK_SCRIPTS)
|
||||
|
||||
@@ -141,6 +141,47 @@ def test_doc_only_deletion_full_rebuild_evicts_md_nodes(tmp_path):
|
||||
assert "run()" in labels
|
||||
|
||||
|
||||
def test_post_merge_style_unscoped_rebuild_heals_a_resurrected_node(tmp_path):
|
||||
"""#2418: the graphify merge driver union merges graph.json, which can
|
||||
resurrect a node for a symbol that was deleted on one side of the merge.
|
||||
A rebuild SCOPED to an unrelated changed file (what the commit hook
|
||||
runs) must NOT heal it -- the resurrected symbol's own file was never
|
||||
touched. The UNSCOPED rebuild the new merge hook performs (the same
|
||||
body as the checkout hook: _rebuild_code with no changed_paths) must."""
|
||||
from graphify.watch import _rebuild_code
|
||||
|
||||
corpus = tmp_path / "corpus"
|
||||
(corpus / "src").mkdir(parents=True)
|
||||
(corpus / "src" / "util.py").write_text("def helper_alpha(): return 1\n", encoding="utf-8")
|
||||
assert _rebuild_code(corpus, acquire_lock=False) is True
|
||||
|
||||
graph_path = corpus / "graphify-out" / "graph.json"
|
||||
data = json.loads(graph_path.read_text(encoding="utf-8"))
|
||||
real_node = next(n for n in data["nodes"] if n["id"] == "src_util_helper_alpha")
|
||||
# Simulate what the union merge driver does: resurrect a node dict shaped
|
||||
# exactly like a real AST node (same _origin/source_location fields a
|
||||
# genuine prior extraction on the other side of the merge would carry),
|
||||
# for a symbol that was deleted on THIS side.
|
||||
phantom = {**real_node, "id": "src_util_helper_doomed", "label": "helper_doomed()"}
|
||||
data["nodes"].append(phantom)
|
||||
graph_path.write_text(json.dumps(data), encoding="utf-8")
|
||||
|
||||
def has_phantom() -> bool:
|
||||
g = json.loads(graph_path.read_text(encoding="utf-8"))
|
||||
return any(n["id"] == "src_util_helper_doomed" for n in g["nodes"])
|
||||
|
||||
assert has_phantom()
|
||||
|
||||
(corpus / "src" / "unrelated.py").write_text("x = 1\n", encoding="utf-8")
|
||||
assert _rebuild_code(
|
||||
corpus, changed_paths=[corpus / "src" / "unrelated.py"], acquire_lock=False
|
||||
) is True
|
||||
assert has_phantom(), "a rebuild scoped to an unrelated file must not have healed the phantom"
|
||||
|
||||
assert _rebuild_code(corpus, acquire_lock=False) is True
|
||||
assert not has_phantom(), "the unscoped rebuild must heal the resurrected node"
|
||||
|
||||
|
||||
def test_rebuild_code_reports_unclassified_files(tmp_path, capsys):
|
||||
"""#3511: `graphify extract` has surfaced files it saw but could not
|
||||
classify (no supported extension/shebang) since #1692; the update/watch
|
||||
|
||||
Reference in new issue
Block a user