feat(tools): propose_reanchors — deterministic re-anchor proposer for safer/simpler pin bumps#82
feat(tools): propose_reanchors — deterministic re-anchor proposer for safer/simpler pin bumps#82Sandermage wants to merge 2 commits into
Conversation
… safer pin bumps Root-cause tooling for the recurring pain that every vLLM pin bump breaks a handful of Genesis text-patches: `TextPatch.anchor` is an EXACT substring, so when upstream renames a line, inserts a method, or reflows a call, the anchor stops matching (drift) and the patch silently SKIPs. Today re-deriving each anchor is manual archaeology (this bump: 16 drifted patches). `propose_anchor(old_anchor, pristine)` turns that into review-and-apply: it finds the anchor's SURVIVING LANDMARK lines (still present in the new source), takes the exact pristine region that spans them — carrying whatever upstream changed in between — and returns it as the corrected anchor, with a uniqueness guarantee and a confidence. Classifies each drift: * unchanged — anchor still matches * reanchor — a unique corrected anchor was derived (high/medium confidence) * manual — the code is gone / landmarks are ambiguous (low confidence) Crucially it is ANALYSIS ONLY — it never touches the runtime apply engine, so it cannot mis-apply a patch; it only advises a human. That is the safety property: faster, safer re-anchoring without adding risk to the apply path. CLI is file-scoped (uniqueness is only meaningful within the drifted file): python3 tools/propose_reanchors.py <new-pristine-file> --anchor-file <anchor> TDD: tests/unit/tools/test_propose_reanchors.py (5) — unchanged, renamed-line (PN367-like) → reanchor carrying the rename, inserted-method (PN12-like) → extended anchor, absent code → manual, ambiguous landmark → low-confidence. Part of making pin bumps safer/simpler (the dev925 bump exposed the need); pairs with the existing weekly upstream-drift watcher so drift is caught + re-anchored incrementally instead of accumulating to a big batch at bump time.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a deterministic re-anchor proposer tool (tools/propose_reanchors.py) and its unit tests to automate the correction of drifted text-patch anchors during pin bumps. The feedback recommends improving the anchor-matching logic to find the closest pair of landmarks rather than greedily selecting the first occurrence, adding error handling for file I/O operations in the CLI, and implementing a unit test to verify the new minimum-distance matching behavior.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| start = first_pos[0] | ||
| end_candidates = [p for p in last_pos if p >= start] | ||
| if not end_candidates: | ||
| return {"status": "manual", "confidence": "low", "reason": "landmarks out of order"} | ||
| end = end_candidates[0] |
There was a problem hiding this comment.
The current implementation greedily selects the very first occurrence of the first landmark (first_pos[0]) and the first subsequent occurrence of the last landmark. If the first landmark is a common line (e.g., def __init__(self): or return) that appears earlier in the file, this greedy approach can result in a proposed anchor that spans a massive, unrelated portion of the file.
Instead, we should find the pair of (start, end) positions that minimizes the distance between the two landmarks (where end >= start). This ensures we select the closest and most logical matching region.
| start = first_pos[0] | |
| end_candidates = [p for p in last_pos if p >= start] | |
| if not end_candidates: | |
| return {"status": "manual", "confidence": "low", "reason": "landmarks out of order"} | |
| end = end_candidates[0] | |
| # Find the pair (s, e) that minimizes the distance (e - s) where e >= s | |
| candidates = [ | |
| (e - s, s, e) | |
| for s in first_pos | |
| for e in last_pos | |
| if e >= s | |
| ] | |
| if not candidates: | |
| return {"status": "manual", "confidence": "low", "reason": "landmarks out of order"} | |
| _, start, end = min(candidates) |
| anchor = ( | ||
| pathlib.Path(args.anchor_file).read_text(encoding="utf-8") | ||
| if args.anchor_file | ||
| else sys.stdin.read() | ||
| ) | ||
| pristine = pathlib.Path(args.pristine_file).read_text(encoding="utf-8") |
There was a problem hiding this comment.
Reading files directly without error handling can cause the CLI tool to crash with a raw traceback if the specified files do not exist or are unreadable. Wrapping the file operations in a try-except OSError block provides a much cleaner user experience.
| anchor = ( | |
| pathlib.Path(args.anchor_file).read_text(encoding="utf-8") | |
| if args.anchor_file | |
| else sys.stdin.read() | |
| ) | |
| pristine = pathlib.Path(args.pristine_file).read_text(encoding="utf-8") | |
| try: | |
| anchor = ( | |
| pathlib.Path(args.anchor_file).read_text(encoding="utf-8") | |
| if args.anchor_file | |
| else sys.stdin.read() | |
| ) | |
| pristine = pathlib.Path(args.pristine_file).read_text(encoding="utf-8") | |
| except OSError as e: | |
| print(f"Error: Failed to read input files: {e}", file=sys.stderr) | |
| return 2 |
| p = m.propose_anchor(old, pristine) | ||
| assert p["status"] in ("manual", "reanchor") | ||
| if p["status"] == "manual": | ||
| assert p["confidence"] == "low" |
There was a problem hiding this comment.
Add a unit test to verify that the proposer correctly handles multiple occurrences of a landmark by selecting the closest matching pair (minimum distance) rather than greedily matching the first occurrence.
assert p["confidence"] == "low"
def test_minimum_distance_landmark_matching():
"""Ensure we don't greedily match the first occurrence of a landmark if a
much closer/better matching pair exists later in the file."""
m = _mod()
old = " landmark_A\n drifted_line\n landmark_B\n"
# landmark_A appears early, but the actual anchor region is much later
pristine = (
" landmark_A\n" # unrelated early occurrence
" some_other_code\n" * 10 +
" landmark_A\n" # correct occurrence
" drifted_line_modified\n"
" landmark_B\n" # correct occurrence
)
p = m.propose_anchor(old, pristine)
assert p["status"] == "reanchor"
# The proposed anchor should be the close pair, not spanning the whole file
assert "some_other_code" not in p["new_anchor"]
assert p["new_anchor"] == " landmark_A\n drifted_line_modified\n landmark_B\n"1 participant
Why
Root-cause tooling for the pain the dev925 pin bump exposed: every vLLM bump breaks a handful of Genesis text-patches because
TextPatch.anchoris an exact substring — upstream renames a line, inserts a method, or reflows a call, the anchor stops matching (drift), and the patch silently SKIPs. Re-deriving each anchor by hand is slow archaeology (this bump: 16 drifted patches).What
propose_anchor(old_anchor, pristine)turns re-anchoring into review-and-apply: it finds the anchor's surviving landmark lines (still present in the new source), takes the exact pristine region spanning them — carrying whatever upstream changed in between — and returns the corrected anchor with a uniqueness guarantee + confidence. Classifies each driftunchanged/reanchor/manual.Safety property: it is analysis-only — it never touches the runtime apply engine, so it cannot mis-apply a patch; it only advises a human. Faster, safer re-anchoring without adding risk to the apply path (which is the whole point — "патчи не должны ломаться, и бампить безопаснее").
TDD
tests/unit/tools/test_propose_reanchors.py(5) — modelled on the real dev925 drift patterns: unchanged · renamed-line (PN367-like → reanchor carrying the rename) · inserted-method (PN12-like → extended anchor) · absent-code → manual · ambiguous-landmark → low-confidence.Fits the bigger safety strategy
Pairs with the existing weekly upstream-drift watcher (catch drift incrementally, 1–2 at a time, instead of a 16-patch batch at bump time). Natural follow-ups: a
sndr pins proposewrapper that resolves each drifted patch's target file automatically, and migrating the most-drift-prone text-patches to class-rebind wiring (which does not drift on text).