Skip to content

Cap the file-history walk, share the lock, and let it be cancelled - #478

Merged
jonassaa merged 4 commits into
mainfrom
fix/file-history-visit-cap
Sep 17, 2026
Merged

jonassaa merged 4 commits into
mainfrom
fix/file-history-visit-cap

Conversation

@jonassaa

Copy link
Copy Markdown
Owner

Closes #474.

file_history walks from HEAD keeping commits whose tree differs from their
parent's at the path, and stopped at limit matches. A file with fewer
changes than the limit has nothing to stop on, so the walk ran to the root of
history with a tree comparison at every commit — and it held the exclusive
repository lock for all of it.

Measured on the linux benchmark fixture (warm, one click on a path with fewer
than 200 changes):

walk cost
uncapped — 1,482,923 commits, every one compared 135.6 s
capped at 50,000 visits (this PR's default) 18.3 s
…of which: libgit2's own revwalk preparation 14.5 s
…of which: 50,000 commits of tree comparison 4.1 s

What changed

A visit cap beside the match cap. The result now carries visited and
stoppedAt (Exhausted / MatchLimit / VisitLimit), because the three
outcomes look identical on screen — a list that ends — and mean completely
different things. The screen says which one happened, with the number that was
actually searched, and offers Search all of history: the old unbounded walk,
asked for deliberately, the way the blob ceiling's "diff it anyway" works.

with_repo_read. This is the longest read in the backend, and holding the
exclusive lock for it queued every other operation on the repository behind a
walk that could take minutes — the failure #400 set out to remove, surviving in
the op least able to afford it. The proof it writes nothing is in
docs/dev/backend.md; notably it never reads the index, so there is no
accidental refresh for anything else to have come to depend on (the status
regression next door is the cautionary tale).

Cancellable, because 18 s is still a wait worth stopping. A new
cancel::Scope::Walk — the first scope with nothing to signal, so the command
hands the backend a predicate to poll between commits and the backend knows
nothing about cancellation. The screen also cancels on the way out, so leaving
does not leave a walk running.

A bug this surfaced on the way: the command palette's Cancel row was gated
on "is this cancellable" but hardcoded to the network mechanism, so it would
have offered a Cancel for a history search and reached nothing. "Can it be
cancelled" and "what cancels it" are one table now, with one store action both
surfaces call.

The optimisation this deliberately does not make

Dropping Sort::TOPOLOGICAL looks like the fix and buys nothing. libgit2's
sorted revwalk pre-walks the whole graph before yielding anything — 14.2 s to
the first oid, 14.7 s for all 1.48 million, measured within 1% for TIME
and TIME | TOPOLOGICAL alike. Sort::NONE is incremental and unusable here:
it yields commits in traversal order, so the first 50,000 are not the newest
50,000 and a capped walk could miss last week's change while reporting one from
2011. The cap has to mean "the newest N commits" or it cannot be said out loud.

That fixed floor is very probably the one behind log_page's first page too,
which is worth knowing before anyone optimises the log by changing its sort. It
is written up in docs/dev/performance.md beside the commit-graph measurement
that rules out the other cheap fix.

Verification

All on the final tree.

  • cargo test — 96 test binaries, no failures. The visit cap, the two ceilings
    reported separately, the exactly-on-the-boundary case that would otherwise
    make the notice lie about every ordinary repository, cancellation (including
    that it stops rather than running on and discarding the answer), and a
    rendezvous test that the walk does not exclude another read — the
    predicate is called with the lock held, so it is a place to stand still and
    check whether a concurrent status can get in. Verified by planting
    with_repo back: that test fails on its bounded wait and only that one.
  • pnpm test — 407 files, 4,178 tests, all passing.
  • pnpm tsc --noEmit and pnpm exec tsc -p e2e/tsconfig.json --noEmit clean.
  • pnpm test:e2e:docker — snapshot rebuilt, then file-history (4 passing),
    cancel (2) and palette (8), 0 stalled driver scripts. Two of the
    file-history cases ask the commands directly: nothing on screen distinguishes
    a readable stoppedAt from an unreadable stopped_at, and the frontend
    deliberately swallows a cancel_walk rejection, so both would fail silently.
  • Two frontend plants confirmed the new tests bite: removing the cancel-ordering
    chain fails the ordering test, and the notice tests fail on a hardcoded cap.

🤖 Generated with Claude Code

jonassaa and others added 4 commits September 17, 2026 17:43
"Can this be cancelled" and "what cancels it" were two answers to one
question — a set of cancellable kinds in `repoActivity`, and a hardcoded
`cancelNetworkOps()` at each of the two surfaces offering the button. That
is free to disagree, and the disagreement's shape is a Cancel that runs and
stops nothing.

They are one table now (`cancelPath`), and one store action picks the
mechanism for both surfaces (`cancelActivity`). `isCancellable` is derived
from the table rather than sitting beside it.

**Why:** #474 adds a cancellable operation that is NOT a subprocess — a
libgit2 revwalk, stopped by setting a flag it polls rather than by signalling
a process group. Without this, the command palette would have offered
"Cancel network operation" for a history search and called the network path,
which reaches nothing. The palette gets its own row for it, with one label
rather than two: a walk polls a flag, so asking twice does what asking once
did, and borrowing the network row's "Force stop" would promise an
escalation that has no counterpart here.

Also resets `cancelRequested` in the palette test's `setRepo`. It was left
standing between tests, which is how a test that never clicked anything read
"Force stop network operation".

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes #474.

`file_history` walked from HEAD keeping commits whose tree differs from their
parent's at the path, and stopped at `limit` MATCHES. A file with fewer
changes than the limit had nothing to stop on, so the walk ran to the root of
history with a tree comparison at every commit. Measured on torvalds/linux,
warm, for one click: 135.6 s and 1,482,923 tree comparisons. That is most
files in most large repositories, not a corner case.

Three things, matching the issue:

* **A visit cap beside the match cap**, defaulting to 50,000 commits, and the
  result now says which ceiling ended the walk (`HistoryStop`) plus how many
  commits it examined. The screen says it out loud — "Searched the newest
  50,000 commits" — and offers "Search all of history", which is the old
  unbounded walk asked for deliberately. A cap nobody mentions is the silent
  wrong answer this area exists to avoid, and the number shown comes off the
  wire so it cannot drift from the policy applied.
* **`with_repo_read`.** This is the longest read in the backend and it held
  the EXCLUSIVE lock for all of it, so one click queued every other operation
  on the repository behind a walk that could take minutes — the exact failure
  #400 set out to remove, surviving in the op least able to afford it. It
  writes nothing: a revwalk, `find_commit`, `Tree::get_path`, and a
  tree-to-tree diff in the pathspec case. Notably it never reads the index,
  so there is no accidental refresh for anything else to depend on.
* **Cancellable**, because 18 s is still a wait worth being able to stop. A
  new `cancel::Scope::Walk` — the first scope with nothing to signal, so the
  command hands the backend a predicate to poll between commits and the
  backend knows nothing about cancellation. The screen also cancels on the
  way out, so leaving does not leave a walk running.

**Why the sort order is untouched:** dropping `TOPOLOGICAL` looks like the
obvious fix and buys nothing. libgit2's sorted revwalk pre-walks the whole
graph before yielding anything — 14.2 s to the first oid, 14.7 s for all 1.48
million, measured within 1% for TIME and TIME|TOPOLOGICAL alike. `Sort::NONE`
IS incremental and is unusable here: it yields commits in traversal order, so
the first 50,000 are not the newest 50,000, and a capped walk could miss last
week's change while reporting one from 2011. The cap has to mean "the newest
N commits" or it cannot be said out loud.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The component tests drive `file_history` through a mocked `invoke`, so none
of them can see the wire: the command answers with a struct now rather than
an array, takes a new `searchAll` argument, and `cancel_walk` is a new
command whose rejection the frontend deliberately swallows (a walk that
finished first is the ordinary case).

A serde rename arriving as `stopped_at`, or an argument Tauri would not
convert, breaks none of the 4,178 mocked tests and every real click — the
same shape of break as the argument-name break in #435 that only a real-IPC
case caught. Two of the four cases therefore ask the commands directly:
nothing on screen distinguishes a readable `stoppedAt` from an unreadable
one, since both render the same list with no notice.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`performance.md` said file history was "unbounded on a cold path", which is
no longer true, and said it without a number because "unbounded is not a
number". There are numbers now, taken on the `linux` fixture: 135.6 s
uncapped against 18.3 s capped, of which 14.5 s is libgit2's own revwalk
preparation and 4.1 s is 50,000 commits of tree comparison.

The second bullet is the more generally useful finding, and it rules out the
cheap fix the way the commit-graph measurement next to it already does: a
sorted libgit2 revwalk pre-walks the whole graph before it yields anything,
and the sort ORDER is not why — TIME and TIME|TOPOLOGICAL were within 1% of
each other at every size measured. That floor is very probably the one behind
`log_page`'s first page too, which is worth knowing before anyone tries to
optimise the log by changing its sort.

Also: `file_history` joins the shared-read list in `backend.md` with its proof
and the rendezvous test that pins it, `cancel.rs`'s section grows the second
KIND of cancellable work, `architecture.md` gains the two commands
(`test/docs.test.ts` requires it), `frontend.md` gains the notice and the
cancel-ordering trap, and `repo_bench.rs`'s `hottest_path` no longer explains
itself with a bug that is fixed — it has two reasons that outlived it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jonassaa
jonassaa merged commit 462548e into main Sep 17, 2026
18 checks passed
@jonassaa
jonassaa deleted the fix/file-history-visit-cap branch September 17, 2026 15:56
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.

file_history walks all of history on a file with fewer than 500 commits, holding the exclusive lock

1 participant