Skip to content

Preserve large squash authorship mappings - #1985

Open
svarlamov wants to merge 1 commit into
mainfrom
agent/preserve-large-squash-mappings
Open

Preserve large squash authorship mappings#1985
svarlamov wants to merge 1 commit into
mainfrom
agent/preserve-large-squash-mappings

Conversation

@svarlamov

@svarlamov svarlamov commented Jul 26, 2026

Copy link
Copy Markdown
Member

Summary

  • Preserve genuine leading squash-source authorship for generic update-ref / non-fast-forward rewrites up to the historical 64-commit safety boundary, including edited and empty metadata-only sources.
  • Preserve arbitrarily large leading squash groups only when a trace2-observed rebase provides an immutable old-range boundary.
  • Discard the leading prefix for ambiguous rebases and groups larger than 64 on generic ref moves, so divergent restack-undo history never reaches note reads or diff-tree work.
  • Keep the historical unbounded behavior for < entries after the first confirmed match; those are established between-match squash sources.
  • Bound downstream memory and platform argument usage by deserializing source notes lazily, deduplicating target-note parsing, and feeding tree/merge-parent batches to one Git process over stdin.
  • Update repro_1985.sh to use this checkout's target/debug/git-ai by default and assert that the divergent prefix is discarded.

Correctness model

A rebase may preserve every leading source only when its old range is immutable and scoped to the operation. The daemon derives that boundary without reading mutable repository state at processing time:

  • full upstream OIDs are already immutable;
  • HEAD / @ parent-ancestry expressions are anchored to the trace2/reflog-captured original tip;
  • standard named rebases use the exact cursor-bounded checkout OID when there is no explicit --onto;
  • explicit --fork-point, --root, mutable old boundaries used with --onto, and other ambiguous forms fail closed to leading-prefix discard.

Generic ref moves do not have equivalent provenance. They retain the old <=64 behavior for real Graphite/update-ref squashes, while the 65th leading unmatched source discards the whole group. Once range-diff establishes a real pair, later unmatched sources keep mapping to the preceding destination as before.

All boundary capture is pure trace2/reflog-derived parsing. This adds no Git spawn, object lookup, ref lookup, or file read to the critical ingestion path. Rewrite processing remains constant-spawn; the large batches changed here use stdin instead of argv.

Regression coverage

  • Graphite-style commit-tree + update-ref leading squash
  • edited leading squash source
  • metadata-only empty leading source
  • exact 64/65 generic boundary
  • 65-source interactive rebase using an immutable HEAD~N boundary
  • divergent named rebase --onto that must not copy old-upstream authorship
  • 65-commit restack undo
  • arbitrarily large post-match squash group
  • 2,048-item tree and merge-parent stdin batches
  • streaming merge across the chunk boundary

Validation

  • task build
  • task fmt
  • task lint
  • task test (2,120 unit tests; 3,250 integration tests passed, 85 intentionally ignored; doctests passed)
  • full commit_tree_update_ref test binary
  • focused rebase/restack regression matrix after restacking onto current main
  • bash -n repro_1985.sh
  • SCALE=small ./repro_1985.sh (150 divergent commits -> 4 real mappings; flat daemon RSS)

svarlamov commented Jul 26, 2026

Copy link
Copy Markdown
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@svarlamov
svarlamov force-pushed the agent/preserve-large-squash-mappings branch from fb0902e to 5661917 Compare July 26, 2026 22:32
@svarlamov
svarlamov marked this pull request as ready for review July 26, 2026 22:57

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 1 additional finding.

Open in Devin Review

@svarlamov
svarlamov force-pushed the agent/preserve-large-squash-mappings branch from 5661917 to 743a29f Compare July 26, 2026 23:22
@svarlamov
svarlamov changed the base branch from andrew/memory-leak-2 to main July 26, 2026 23:30
@svarlamov
svarlamov force-pushed the agent/preserve-large-squash-mappings branch 3 times, most recently from c1a5643 to f47f674 Compare July 27, 2026 02:16

svarlamov commented Jul 27, 2026

Copy link
Copy Markdown
Member Author

Addressed all review findings through current head 01721075f.

The final correctness policy distinguishes the ambiguous rewrite shapes instead of treating them uniformly:

  • Rebases with an immutable old-range boundary preserve every leading squash source.
  • Implicit/explicit fork-point rebases do not trust the observed checkout as an old boundary, but preserve a legitimate leading squash group only while it stays within the historical 64-commit bound.
  • Explicit rebase --onto remains DiscardLeading, including short prefixes, because its old upstream may be unrelated divergent history.
  • Pull rebases never treat the observed checkout as a trusted old boundary. A pull fetches first, so the checkout observed at rebase_start can be the newly fetched upstream while Git selects replay commits from the pre-fetch remote-tracking reflog. Without an immutable pre-fetch fork point, the range remains untrusted.
  • If a supposedly trusted boundary fails to resolve, range-diff falls back to merge-base and downgrades to DiscardLeading; it does not abort note migration or restore unsafe prefix mappings.
  • Generic non-rebase ref moves, including update-ref squashes, retain bounded leading-squash preservation.
  • Unparseable source notes are rejected before any tree resolution or diff-tree work is queued.
  • repro_1985.sh constructs scalable commits and notes with two bounded fast-import processes; no loop spawns Git per commit, note, file, object, or ref.

The pull fix is a constant-time check of the already-normalized command kind during deferred rewrite handling. It adds no trace2 ingestion-path I/O, Git spawn, object lookup, or ref check.

TDD coverage includes a real conflicted git pull --rebase origin <branch> after a force-push. The test creates 65 abandoned upstream commits with one batched fast-import invocation, proves Git replays only the local feature while range-diff exposes more than 64 leading < entries, and verifies abandoned upstream authorship is not copied into the rebased feature note. Before the fix it failed with a leaked shared.txt attestation; after the fix it passes. The existing successful, conflict, abort, alias, config, autostash, skip, worktree, and merge pull-rebase cases remain green.

Local verification on 01721075f:

  • task fmt, task lint, and git diff --check
  • Complete task test green: 2,128 library tests, 3,342 integration tests, 22 notes-sync tests, and all runnable doc tests
  • The workstation's real ~/.local/bin/droid was excluded from PATH because one integration test intentionally requires no discoverable system droid

Fresh performance matrix:

Scale Divergent < sources Retained mappings Daemon RSS baseline → peak Peak child Git RSS Async rewrite time after reset
small 150 4 25 → 26 MB 0 MB 0s
default 600 4 27 → 27 MB 17 MB 0s
large 1,200 4 28 → 29 MB 24 MB 0s

At large scale the guard discarded all 1,200 bogus leading sources, retained exactly four real mappings, and avoided an estimated ~31 GB of bogus mapped-diff structures while daemon RSS grew by only the 1 MB sampling granularity.

Fresh CI and Devin review are now running for 01721075f; this comment will be updated with the final matrix and review-thread state.

@svarlamov
svarlamov force-pushed the agent/preserve-large-squash-mappings branch from f47f674 to 1a4a206 Compare July 27, 2026 02:44

svarlamov commented Jul 27, 2026

Copy link
Copy Markdown
Member Author

Addressed the trusted_old_range_base validation finding, with the final implementation in fae2083b9.

A rejected trusted boundary no longer aborts rewrite processing. If range-diff fails, the handler resolves/peels the candidate to distinguish an invalid boundary from an unrelated Git error. An invalid candidate falls back to the old/new merge-base and downgrades to DiscardLeading; valid matched mappings survive without letting the fallback restore the divergent-prefix mapping bomb. Valid rewrites take the original path with zero additional Git spawns; the extra resolution/retry work exists only on the invalid-boundary failure path. There is no trace2 ingestion-path work and no per-commit/object spawning.

Added a real-repository TDD regression that first reproduced the fatal bad-revision error, then verifies both fallback mapping and leading-source discard. task fmt, task lint, all 47 rewrite unit tests, the 65-source rebase test, divergent---onto test, and 65-commit restack-undo test are green locally. The full local suite is also green (2,123 unit, 3,252 integration, notes-sync, and doc tests; expected ignores only).

@svarlamov
svarlamov force-pushed the agent/preserve-large-squash-mappings branch 6 times, most recently from f6ab1fe to 0172107 Compare July 27, 2026 21:43
@svarlamov
svarlamov force-pushed the agent/preserve-large-squash-mappings branch from 0172107 to 3776d87 Compare July 29, 2026 14:47
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.

1 participant