From 09277fcd77d0c8ec622c6777b613dc3c4d8ccecd Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sat, 3 Oct 2026 18:22:25 +0000 Subject: [PATCH 01/10] build(deps): bump actions/download-artifact from 4.0.0 to 8.0.1 Bumps [actions/download-artifact](https://github.com/actions/download-artifact) from 4.0.0 to 8.0.1. - [Release notes](https://github.com/actions/download-artifact/releases) - [Commits](https://github.com/actions/download-artifact/compare/v4.0.0...v8.0.1) --- updated-dependencies: - dependency-name: actions/download-artifact dependency-version: 8.0.1 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/publish.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index e7077360..12dc511a 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -38,7 +38,7 @@ jobs: # cost is deliberate — every install carries seven binaries it cannot # load — and buys one file to download instead of nine. - name: Download engine binaries - uses: actions/download-artifact@v4.0.0 + uses: actions/download-artifact@v8.0.1 with: name: native-all path: engine/native/ @@ -92,7 +92,7 @@ jobs: - run: corepack enable && pnpm install --frozen-lockfile - name: Download VSIX - uses: actions/download-artifact@v4.0.0 + uses: actions/download-artifact@v8.0.1 with: name: vsix path: dist/ From bac121d0978a277d06f733ac2420b0aea83cb29a Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sat, 3 Oct 2026 18:22:40 +0000 Subject: [PATCH 02/10] build(deps): bump actions/upload-artifact from 4.0.0 to 7.0.1 Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 4.0.0 to 7.0.1. - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](https://github.com/actions/upload-artifact/compare/v4.0.0...v7.0.1) --- updated-dependencies: - dependency-name: actions/upload-artifact dependency-version: 7.0.1 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/native-build.yml | 2 +- .github/workflows/publish.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/native-build.yml b/.github/workflows/native-build.yml index 9c9df178..5aa904c3 100644 --- a/.github/workflows/native-build.yml +++ b/.github/workflows/native-build.yml @@ -78,7 +78,7 @@ jobs: # job restores them in one step and the layout it gets is the layout # `.vscodeignore` whitelists and the loader reads. - name: Upload engine binaries - uses: actions/upload-artifact@v4.0.0 + uses: actions/upload-artifact@v7.0.1 with: name: native-all path: engine/native/*/git-graph.node diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 12dc511a..b89e4e53 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -71,7 +71,7 @@ jobs: echo "Packaged $vsix ($(du -h "$vsix" | cut -f1))" - name: Upload VSIX - uses: actions/upload-artifact@v4.0.0 + uses: actions/upload-artifact@v7.0.1 with: name: vsix path: ${{ steps.package.outputs.vsix }} From 77adb67595b6c5c3f61f6c74c578892bdb6ea2ea Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sat, 3 Oct 2026 18:22:29 +0000 Subject: [PATCH 03/10] build(deps): bump actions/cache from 4.2.0 to 6.1.0 Bumps [actions/cache](https://github.com/actions/cache) from 4.2.0 to 6.1.0. - [Release notes](https://github.com/actions/cache/releases) - [Changelog](https://github.com/actions/cache/blob/main/RELEASES.md) - [Commits](https://github.com/actions/cache/compare/v4.2.0...v6.1.0) --- updated-dependencies: - dependency-name: actions/cache dependency-version: 6.1.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/native-build.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/native-build.yml b/.github/workflows/native-build.yml index 5aa904c3..61d22efd 100644 --- a/.github/workflows/native-build.yml +++ b/.github/workflows/native-build.yml @@ -42,7 +42,7 @@ jobs: # Windows targets. Fixed content, so the key only has to change when the # build script that drives it does. - name: Cache the MSVC CRT - uses: actions/cache@v4.2.0 + uses: actions/cache@v6.1.0 with: path: ~/.cache/cargo-xwin key: xwin-${{ runner.os }}-${{ hashFiles('engine/scripts/build-addon.mjs') }} @@ -51,7 +51,7 @@ jobs: # ~350 MB: the pinned zig. Keyed on the script that names the version, # so a new pin fetches a new wheel instead of reusing the old one. - name: Cache the pinned zig - uses: actions/cache@v4.2.0 + uses: actions/cache@v6.1.0 with: path: engine/.toolchain key: zig-${{ runner.os }}-${{ hashFiles('engine/scripts/build-all.mjs') }} From dbbe35185589c8594e16c79d4f5aea564cc7301f Mon Sep 17 00:00:00 2001 From: Zamkorus Date: Wed, 23 Sep 2026 17:15:00 +0200 Subject: [PATCH 04/10] perf(engine): carry tag signatures and symbolic remote HEADs, dropping both ref fills MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `loadCommits` was the one read the engine lost: 12.9 ms against the CLI's 10.8 ms on a 1,036-commit repository, even though the engine call itself took 7.1 ms. The gap was two sequential `for-each-ref` spawns running after it (16.5d3/d5), putting back two fields the engine's types did not carry. Same defect as the repoInfo one fixed in 9fe6599, one layer down, so the same fix: teach the engine the field, delete the fill. `GitTagRef` and `GitCommitTag` gain `signed`. `refs.rs` reads it from the tag object it was already peeling, and records it on both records of an annotated tag, since the signature belongs to the tag rather than to either hash. `find_header` settles the object kind first, so a lightweight tag costs a header lookup, not a commit read. The semantics were checked against git rather than assumed: `%(contents:signature)` is non-empty only for a signed annotated tag object. A lightweight tag over a genuinely signed commit (`%G?` = `G`) reports unsigned, and the engine matches by construction. `read_remote_refs` now resolves symbolic refs instead of dropping them, which is what `%(objectname)` reports for the `refs/remotes//HEAD` every clone writes. Verified on a clone carrying all four tag shapes and a symbolic origin/HEAD: engine and CLI ref labels identical with the engine serving the read, and the engine path down from 2 git spawns to 0 (the CLI uses 3). loadCommits 300: 11.5 ms CLI vs 7.7 ms engine, 0.8x -> 1.5x; view load 4.4x. Two tests pin it, each mutation-checked to kill only its own. The signed-tag fixture writes the tag object by hand, so CI needs no keyring. Also fixes a defaults drift found while answering why `initialLoadCommits` is 300: `loadMoreCommits` is 100 in the manifest and README but fell back to 75 in config.ts, with the test pinning the wrong value. It never fired in a real install, since VS Code returns the manifest default for an unset key. The 300 itself is left alone and documented as inherited from upstream — the git read and the graph layout do not justify it, but DOM row insertion was not measured and is the one cost that still could. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 13 +- docs/AI_DEV_KNOWLEDGE_BASE.md | 138 ++++++++++++++++- engine/native/core/src/graph.rs | 1 + engine/native/core/src/refs.rs | 59 +++++++- engine/native/core/src/types.rs | 7 + engine/native/core/tests/common/mod.rs | 34 ++++- engine/native/core/tests/log_and_refs.rs | 107 +++++++++++++ src/backend/engine/commits.ts | 182 ++--------------------- src/backend/engine/index.ts | 23 --- src/config.ts | 2 +- tests/backend/config.test.ts | 2 +- tests/backend/engine/commits.test.ts | 113 +------------- 12 files changed, 363 insertions(+), 318 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b8dbd67..d00e5d6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,9 +9,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- **The graph opens roughly three and a half times faster.** Measured on a - 2,000-commit repository: the work behind opening the view went from 67 ms to - 19 ms, and reading the branch, tag, remote and stash lists from 59 ms to 8 ms. +- **The graph opens roughly four times faster.** Measured on a real + 1,036-commit repository: the work behind opening the view went from 69 ms to + 16 ms, reading the branch, tag, remote and stash lists from 60 ms to 8 ms, + and loading a page of commits from 12 ms to 8 ms. Loading commits no longer + starts a `git` process at all. - **A built-in Git engine, so the graph stops waiting on `git` processes.** Reading a repository — opening the graph, loading a page of commits, opening commit details, comparing two commits, reading a file at a revision — now @@ -25,6 +27,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Signed tags keep their badge, and `origin/HEAD` keeps its label**, without + the extension running extra `git` commands to find either out. Both are now + read directly from the repository along with everything else on the row. A + lightweight tag pointing at a signed commit is correctly *not* marked as a + signed tag, matching `git` itself. - **The author filter lists everyone who has contributed**, not only the people whose commits are reachable from the branch you have checked out. Anyone whose work is on another branch — a remote branch you have fetched but not checked diff --git a/docs/AI_DEV_KNOWLEDGE_BASE.md b/docs/AI_DEV_KNOWLEDGE_BASE.md index bfefd1e6..da24a1ae 100644 --- a/docs/AI_DEV_KNOWLEDGE_BASE.md +++ b/docs/AI_DEV_KNOWLEDGE_BASE.md @@ -2402,6 +2402,9 @@ Implementation record (`2026-09-22`, twelve subslice commits, all signed): pay no spawn: symbolic remote HEADs (`origin/HEAD`) attached in CLI order, and signed-tag badges flipped reusing the loader's own signature atom (probed end to end against a crafted PGP-signed tag). + **Both were removed on `2026-09-23`** — the engine now carries the two + fields itself and the `loadCommits` engine path spawns nothing. See "The + two ref fills, removed from the engine rather than optimised". - 16.5d4 post-call reroutes: unborn stays on the CLI, which owns the empty-graph error shape; unfiltered show-all pages that lost HEAD go back to the whole CLI read, mirroring the move-HEAD-onto-page contract. @@ -2793,9 +2796,134 @@ plus the stash, mirroring what `stats.rs` already did and what `--all` actually covers. `collects_authors_from_every_ref_not_just_head` pins it, and a mutation back to head-only fails it. -**`loadCommits` is still slower than the CLI** — `0.9x` at a 300-commit page, -`0.7x` at 1000 — and is untouched by any of the above. That is the remaining -item, and it is the hot path this phase exists for. +**`loadCommits` was still slower than the CLI** — `0.9x` at a 300-commit page, +`0.7x` at 1000 — and was untouched by any of the above. That is the next +subsection. + +#### The two ref fills, removed from the engine rather than optimised (`2026-09-23`) + +`loadCommits` was the one read where the engine *lost*: `12.9 ms` against the +CLI's `10.8 ms` on a real 1,036-commit repository. The engine call itself was +`7.1 ms` — comfortably ahead. The loss was entirely in what ran afterwards. + +Two CLI fills (16.5d3/d5) ran `for-each-ref` on the engine path, sequentially, +to put back two facts the engine's types did not carry: + +| fill | spawn | the missing field | +| --- | --- | --- | +| `attachRemoteHeadLabels` | `for-each-ref refs/remotes` | symbolic `origin/HEAD` | +| `attachSignedTagNames` | `for-each-ref refs/tags` | tag signature presence | + +This is the same defect as the `repoInfo` one above, one layer down: ask the +engine, then spawn `git` anyway. The fix is the same shape — teach the engine +the field and delete the fill, rather than make the spawn cheaper. + +**`GitTagRef.signed` and `GitCommitTag.signed`.** `refs.rs` reads the tag +object's signature while it is peeling the tag it would peel regardless, and +records the flag on *both* records of an annotated tag — the signature belongs +to the tag, not to either hash, and the graph attaches the peeled one. A +`find_header` call settles the object kind first, so a lightweight tag costs a +header lookup rather than a commit read. + +The semantics were verified against git rather than assumed. `for-each-ref`'s +`%(contents:signature)` is non-empty **only** for an annotated tag object that +was signed — a lightweight tag over a *signed commit* reports nothing: + +| tag | object type | `%(contents:signature)` | +| --- | --- | ---: | +| annotated, signed | `tag` | `1` | +| annotated, unsigned | `tag` | `0` | +| lightweight, on a signed commit | `commit` | `0` | +| lightweight, on an unsigned commit | `commit` | `0` | + +That third row is the one worth pinning: the commit was genuinely signed +(`%G?` = `G`) and the tag still reports unsigned. The engine matches by +construction — it reads the tag object, and a lightweight tag has none. + +**Symbolic remote HEADs, resolved instead of dropped.** `direct_target` +returns `None` for a symbolic ref, so `refs/remotes/origin/HEAD` — what every +`git clone` writes — never reached the graph. `for-each-ref %(objectname)` +reports the object such a ref resolves to, so `read_remote_refs` now does the +same. Only a remote's handful of `/HEAD` refs are ever symbolic, so this costs +one lookup each and nothing in the common case. + +**Verification.** A clone carrying all four tag shapes *and* a symbolic +`origin/HEAD` was read through `createRepoReader` on both backends, and the ref +labels compared: + +``` +engine served the read: true + ccaf060 remote origin/HEAD + ccaf060 tag annotated-signed signed=true + ccaf060 tag light-on-signed signed=false +IDENTICAL: true +``` + +Spawn counts on that repository, through `recordGitCommand`: + +| backend | git spawns | +| --- | ---: | +| `git-cli` | 3 (`refs`, `head`, `log`) | +| `auto` (engine) | **0** (was 2) | + +| operation | CLI | engine before | engine after | | +| --- | ---: | ---: | ---: | ---: | +| `loadCommits` (300) | `11.5 ms` | `12.9 ms` | **`7.7 ms`** | `1.5x` | +| `loadCommits` (1000) | `17.1 ms` | `19.3 ms` | **`13.6 ms`** | `1.3x` | +| view load | `68.7 ms` | — | **`15.7 ms`** | `4.4x` | + +`reports_tag_signature_presence_the_way_for_each_ref_does` and +`resolves_a_symbolic_remote_head_rather_than_dropping_it` pin both changes; +each was mutation-checked and kills only its own test. The signed-tag fixture +writes the tag object by hand through `git hash-object -t tag -w`, so it needs +no keyring and stays deterministic on CI — the engine reports signature +*presence*, so a fabricated block exercises the identical path. + +`attachRemoteHeadLabels`, `attachSignedTagNames`, their parsers, their +appliers and their tests are deleted. Nothing in the engine's `loadCommits` +path spawns a process any more. + +**A measurement trap worth recording.** The first benchmark after this change +showed the engine *five times slower*, because `pnpm run engine:build` builds +the **debug** profile (186 MB, unoptimised) while the benchmark numbers above +are all release (5.9 MB). Always `pnpm run engine:build:release` before +benchmarking; a debug addon is not a slow engine, it is a different one. + +#### What the page-size defaults are actually worth (`2026-09-23`) + +Asked why `initialLoadCommits` is `300` when 1,000 costs only milliseconds +more. Three separate costs were measured, and the answer is that two of them +are now negligible and the third was never measured here. + +**Where the number came from.** It is inherited, not chosen: +`git show upstream/main:package.json` gives `initialLoadCommits: 300`, and the +only commits that ever touched it in this fork are `3ba4217 Initial +implementation of Git Graph` and an i18n pass. It is mhutchie's number, picked +when every read was a `git` spawn. + +| | 300 | 1,000 | 10,000 | +| --- | ---: | ---: | ---: | +| backend read (CLI) | `11.5 ms` | `17.1 ms` | — | +| backend read (engine) | `7.7 ms` | `13.6 ms` | — | +| graph layout (jsdom) | `0.2 ms` | `0.4 ms` | `1.1 ms` | + +700 extra commits cost about `6 ms` of git and essentially nothing to lay out. + +**What was not measured, and why the default therefore stands.** DOM row +insertion — building and inserting the table rows and SVG paths — was not +measured, because jsdom is not representative of a real browser there. That is +the one cost that could still justify a cap, and it is the only one left. **Do +not raise `initialLoadCommits` on the evidence above alone**; measure row +insertion in a real webview first. + +**A defaults drift found while answering.** `loadMoreCommits` is declared +`100` in the manifest and documented as `100` in the README, but +`src/config.ts` fell back to `75`, and `tests/backend/config.test.ts` pinned +the `75` — a wrong value frozen by the test meant to protect it. The fallback +never fires in a real install, because VS Code returns the manifest default for +an unset key, so nothing user-visible was wrong; it is now `100` in all four +places. The accessor table in that test mirrors manifest defaults by hand, so +it can drift again. #### Benchmarking the two backends (`2026-09-23`) @@ -5227,7 +5355,9 @@ fills, parity byte-for-byte, `loadBranches` untouched). **16.5 (`loadCommits`) is done** (`2026-09-22`: pre-call declines plus post-call HEAD/unborn reroutes, seam mapping with two gated CLI fills, `topo` declined over a measured tie-break difference, byte-for-byte parity -with the served flag). **16.6 (commit details, comparison, line counts) is +with the served flag; the two fills were deleted on `2026-09-23` once the +engine carried tag signatures and symbolic remote HEADs itself, taking +`loadCommits` from `0.8x` to `1.5x`). **16.6 (commit details, comparison, line counts) is done** (`2026-09-23`: eager whole-list counts fill, merges/`*`/blank/stash reroutes, file content through the provider with binary CLI fallback, parity over renames/copies/binary/root/unborn/dirty plus file bytes). diff --git a/engine/native/core/src/graph.rs b/engine/native/core/src/graph.rs index 0e44903f..4ef53efa 100644 --- a/engine/native/core/src/graph.rs +++ b/engine/native/core/src/graph.rs @@ -297,6 +297,7 @@ fn annotate_refs(ref_data: &GitRefData, options: &LogOptions, commits: &mut [Git commits[index].tags.push(GitCommitTag { name: tag.name.clone(), annotated: tag.annotated, + signed: tag.signed, }); } } diff --git a/engine/native/core/src/refs.rs b/engine/native/core/src/refs.rs index 1041bef1..6811fc8f 100644 --- a/engine/native/core/src/refs.rs +++ b/engine/native/core/src/refs.rs @@ -79,13 +79,18 @@ pub fn read_refs(repo: &Repo, options: &RefReadOptions) -> Result { Some(name) => bstr_to_string(name), None => continue, }; - let Some(hash) = direct_target(&reference) else { + let Some(id) = direct_target_id(&reference) else { continue; }; + let hash = id.to_string(); + // The signature belongs to the tag object, so it is read once and carried by both + // records: the graph attaches the peeled one, but a caller matching by name sees either. + let signed = tag_signature_present(&git, id); ref_data.tags.push(GitTagRef { hash: hash.clone(), name: name.clone(), annotated: false, + signed, }); tag_names.push(name.clone()); @@ -96,6 +101,7 @@ pub fn read_refs(repo: &Repo, options: &RefReadOptions) -> Result { hash: peeled, name, annotated: true, + signed, }); } } @@ -142,7 +148,7 @@ fn read_remote_refs( let mut remote_tags_to_peel: Vec<(usize, String)> = Vec::new(); let platform = git.references().git_ctx("Could not read references")?; - for reference in platform + for mut reference in platform .prefixed("refs/remotes/") .git_ctx("Could not read remote branches")? .filter_map(std::result::Result::ok) @@ -168,8 +174,16 @@ fn read_remote_refs( if remote_ref.contains("/changes/") { continue; } - let Some(hash) = direct_target(&reference) else { - continue; + // `refs/remotes//HEAD` is symbolic, and `for-each-ref %(objectname)` reports the + // object it resolves to rather than skipping it — so it is resolved here too. Resolving + // costs one lookup and only ever applies to the handful of `/HEAD` refs a remote has; a + // symbolic ref that resolves to nothing is dropped, as the CLI drops an unborn one. + let hash = match reference.target() { + gix::refs::TargetRef::Object(id) => id.to_string(), + gix::refs::TargetRef::Symbolic(_) => match reference.peel_to_id() { + Ok(id) => id.detach().to_string(), + Err(_) => continue, + }, }; if let Some(tags_index) = remote_ref.find("/tags/") { @@ -184,6 +198,7 @@ fn read_remote_refs( hash, name, annotated: false, + signed: false, }); } else { ref_data.remotes.push(GitRef { @@ -199,16 +214,23 @@ fn read_remote_refs( let Ok(Some(mut reference)) = git.try_find_reference(full_name.as_str()) else { continue; }; + // The ref's own target, captured before the peel rewrites it: that is the tag object, + // and the signature belongs to it rather than to the commit it peels to. + let tag_object = direct_target_id(&reference); let Ok(peeled) = reference.peel_to_id() else { continue; }; let peeled = peeled.detach().to_string(); if peeled != ref_data.tags[index].hash { let name = ref_data.tags[index].name.clone(); + // Recorded on the unpeeled record too, so both records of one tag agree. + let signed = tag_object.is_some_and(|id| tag_signature_present(git, id)); + ref_data.tags[index].signed = signed; ref_data.tags.push(GitTagRef { hash: peeled, name, annotated: true, + signed, }); } } @@ -222,12 +244,39 @@ fn read_remote_refs( /// them only for the sake of the label, and the branch they alias is already listed in its own /// right. fn direct_target(reference: &gix::Reference<'_>) -> Option { + direct_target_id(reference).map(|id| id.to_string()) +} + +/// The same, as an id, for callers that go on to read the object. +fn direct_target_id(reference: &gix::Reference<'_>) -> Option { match reference.target() { - gix::refs::TargetRef::Object(id) => Some(id.to_string()), + gix::refs::TargetRef::Object(id) => Some(id.to_owned()), gix::refs::TargetRef::Symbolic(_) => None, } } +/// Whether a tag ref's own target is a tag object carrying a signature. +/// +/// This is `for-each-ref`'s `%(contents:signature)`, which is non-empty only for an annotated +/// tag that was signed: a lightweight tag reports nothing even when the commit it points at is +/// itself signed (verified against git directly, not assumed). +/// +/// The header settles the object kind before anything is decoded, so a lightweight tag costs a +/// header lookup rather than a commit read. +fn tag_signature_present(git: &gix::Repository, id: gix::ObjectId) -> bool { + match git.find_header(id) { + Ok(header) if header.kind() == gix::object::Kind::Tag => {} + _ => return false, + } + let Ok(object) = git.find_object(id) else { + return false; + }; + let Ok(tag) = object.try_into_tag() else { + return false; + }; + matches!(tag.decode(), Ok(decoded) if decoded.signature.is_some()) +} + fn bstr_to_string(bytes: &[u8]) -> String { String::from_utf8_lossy(bytes).into_owned() } diff --git a/engine/native/core/src/types.rs b/engine/native/core/src/types.rs index 93e8654c..3ba38d6e 100644 --- a/engine/native/core/src/types.rs +++ b/engine/native/core/src/types.rs @@ -33,6 +33,10 @@ pub struct GitCommit { pub struct GitCommitTag { pub name: String, pub annotated: bool, + /// True when the tag object carries a signature. Only an annotated tag has a tag object, so a + /// lightweight tag is never signed — not even over a signed commit, which `for-each-ref`'s + /// `%(contents:signature)` also reports as unsigned. + pub signed: bool, } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -80,6 +84,9 @@ pub struct GitTagRef { /// True for the peeled record of an annotated tag, which points at the commit rather than at /// the tag object. pub annotated: bool, + /// True when the tag object carries a signature. Both records of an annotated tag carry the + /// same value, because the signature belongs to the tag, not to either hash. + pub signed: bool, } #[derive(Debug, Clone, Default, Serialize, Deserialize)] diff --git a/engine/native/core/tests/common/mod.rs b/engine/native/core/tests/common/mod.rs index da46bde4..429aa1c2 100644 --- a/engine/native/core/tests/common/mod.rs +++ b/engine/native/core/tests/common/mod.rs @@ -6,8 +6,9 @@ #![allow(dead_code)] +use std::io::Write; use std::path::{Path, PathBuf}; -use std::process::Command; +use std::process::{Command, Stdio}; use tempfile::TempDir; @@ -79,6 +80,37 @@ impl TestRepo { String::from_utf8_lossy(&output.stdout).into_owned() } + /// Run a git command with something on stdin, for the plumbing that reads objects there. + pub fn git_stdin(&self, args: &[&str], input: &str) -> String { + let mut child = Command::new("git") + .args(args) + .current_dir(self.path()) + .env("GIT_CONFIG_NOSYSTEM", "1") + .env("GIT_TERMINAL_PROMPT", "0") + .env("HOME", self.path()) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap_or_else(|e| panic!("could not run `git {}`: {e}", args.join(" "))); + child + .stdin + .take() + .expect("stdin was not piped") + .write_all(input.as_bytes()) + .expect("could not write to git"); + let output = child + .wait_with_output() + .unwrap_or_else(|e| panic!("could not wait for `git {}`: {e}", args.join(" "))); + assert!( + output.status.success(), + "`git {}` failed: {}", + args.join(" "), + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8_lossy(&output.stdout).into_owned() + } + /// Run a git command and return its output whether or not it succeeded. pub fn git_allow_failure(&self, args: &[&str]) -> String { let output = Command::new("git") diff --git a/engine/native/core/tests/log_and_refs.rs b/engine/native/core/tests/log_and_refs.rs index 9f16a461..c7cc23ef 100644 --- a/engine/native/core/tests/log_and_refs.rs +++ b/engine/native/core/tests/log_and_refs.rs @@ -526,3 +526,110 @@ fn an_empty_repository_reports_no_head_rather_than_failing() { // `git status` report it — so the view names the branch the first commit will land on. assert_eq!(snapshot.branches, vec!["main"]); } + +/// Write a tag object carrying a PGP signature block, without needing a keyring. +/// +/// The engine only reports signature *presence*, so a fabricated block exercises exactly the +/// path a real signature would, and the fixture stays deterministic on a machine with no GPG. +fn write_signed_tag(repo: &TestRepo, tag_name: &str, target: &str) { + let object = format!( + "object {target}\n\ + type commit\n\ + tag {tag_name}\n\ + tagger Test 0 +0000\n\ + \n\ + a signed tag\n\ + -----BEGIN PGP SIGNATURE-----\n\ + \n\ + aBcD\n\ + -----END PGP SIGNATURE-----\n" + ); + let hash = repo.git_stdin(&["hash-object", "-t", "tag", "-w", "--stdin"], &object); + repo.update_ref(&format!("refs/tags/{tag_name}"), hash.trim()); +} + +#[test] +fn reports_tag_signature_presence_the_way_for_each_ref_does() { + require_git!(); + let mut repo = TestRepo::new(); + let first = repo.commit_file("a.txt", "1", "first"); + // Three of the four shapes `for-each-ref %(contents:signature)` distinguishes. The fourth — + // a lightweight tag over a *signed commit* — is false by construction here: the check reads + // the tag object, and a lightweight tag has none, so a commit's own signature never leaks + // into a tag badge. Verified against git directly before this was written. + repo.git(&["tag", "lightweight"]); + repo.git(&["tag", "-a", "annotated", "-m", "unsigned annotated"]); + write_signed_tag(&repo, "signed", &first); + + let engine = open(&repo); + let snapshot = read_refs(&engine, &RefReadOptions::default()).unwrap(); + + let signed_for = |name: &str| -> bool { + snapshot + .ref_data + .tags + .iter() + .find(|tag| tag.name == name) + .unwrap_or_else(|| panic!("the tag {name} was not read")) + .signed + }; + assert!( + !signed_for("lightweight"), + "a lightweight tag is never signed" + ); + assert!( + !signed_for("annotated"), + "an unsigned annotated tag is not signed" + ); + assert!( + signed_for("signed"), + "a signed tag object was reported unsigned" + ); + + // Both records of an annotated tag agree, because the signature belongs to the tag rather + // than to either hash, and the graph attaches the peeled record. + let records: Vec = snapshot + .ref_data + .tags + .iter() + .filter(|tag| tag.name == "signed") + .map(|tag| tag.signed) + .collect(); + assert_eq!( + records, + vec![true, true], + "the peeled record lost the signature" + ); +} + +#[test] +fn resolves_a_symbolic_remote_head_rather_than_dropping_it() { + require_git!(); + let mut repo = TestRepo::new(); + let head = repo.commit_file("a.txt", "1", "first"); + repo.add_fake_remote("origin", "main", &head); + // What `git clone` writes: a *symbolic* ref, unlike the direct one the neighbouring test + // uses. `for-each-ref %(objectname)` reports the object it resolves to, so the view shows + // the label — dropping it here is what used to cost a `for-each-ref` spawn to put back. + repo.git(&[ + "symbolic-ref", + "refs/remotes/origin/HEAD", + "refs/remotes/origin/main", + ]); + + let engine = open(&repo); + let options = RefReadOptions { + show_remote_branches: true, + show_remote_heads: true, + ..Default::default() + }; + let snapshot = read_refs(&engine, &options).unwrap(); + + let entry = snapshot + .ref_data + .remotes + .iter() + .find(|r| r.name == "origin/HEAD") + .expect("the symbolic remote HEAD was dropped"); + assert_eq!(entry.hash, head, "the symref resolved to the wrong object"); +} diff --git a/src/backend/engine/commits.ts b/src/backend/engine/commits.ts index 6680909c..b42b66cb 100644 --- a/src/backend/engine/commits.ts +++ b/src/backend/engine/commits.ts @@ -30,13 +30,9 @@ * topo loads stay on the CLI (pinned by the parity table). */ -import type { SimpleGit } from "simple-git"; - -import { gitRefSignatureAtom } from "@/backend/queries/loadCommits"; import type { CommitOrdering, DateType, GitCommitNode, GitRef } from "@/backend/types"; -import { type GitCommandRecorder, runGitRaw } from "@/backend/utils/gitRunner"; import { selectedLogRefs, uniqueNonEmpty } from "@/backend/utils/logFilters"; -import { isHiddenRemoteRef, normalizeHiddenRemotes } from "@/backend/utils/remoteRefs"; +import { normalizeHiddenRemotes } from "@/backend/utils/remoteRefs"; /** The route fields the engine decision, options and (16.5b) mapping need. */ export type EngineLoadCommitsInput = { @@ -103,6 +99,8 @@ export function shouldServeLoadCommitsFromEngine(input: EngineLoadCommitsInput): export type EngineCommitTag = { name: string; annotated: boolean; + /** Whether the tag object carries a signature. Lightweight tags are never signed. */ + signed: boolean; }; /** One remote label as the engine encodes it. `remote` names the owning remote, if known. */ @@ -155,7 +153,8 @@ function isEngineCommitTag(value: unknown): value is EngineCommitTag { typeof value === "object" && value !== null && typeof (value as { name?: unknown }).name === "string" && - typeof (value as { annotated?: unknown }).annotated === "boolean" + typeof (value as { annotated?: unknown }).annotated === "boolean" && + typeof (value as { signed?: unknown }).signed === "boolean" ); } @@ -297,11 +296,9 @@ function fullRefName(ref: GitRef): string { * pins to null; in-place stash marks are always stripped because the CLI * never marks — it only injects rows. * - * Two CLI parse artifacts are mirrored deliberately, so the parity table - * stays a strict `toEqual` and any future CLI change fails loudly instead of - * drifting silently: a root commit's parents are `[""]` (`"".split(" ")`), - * and tag `signed` is provisionally false (the engine reports presence - * nowhere — 16.5d decides between a CLI fill and a recorded deviation). + * One CLI parse artifact is mirrored deliberately, so the parity table stays + * a strict `toEqual` and any future CLI change fails loudly instead of + * drifting silently: a root commit's parents are `[""]` (`"".split(" ")`). */ export function mapEngineCommitData(data: EngineCommitData, showStashes: boolean): GitCommitNode[] { const nodes: GitCommitNode[] = []; @@ -325,7 +322,7 @@ export function mapEngineCommitData(data: EngineCommitData, showStashes: boolean const refs: GitRef[] = [ ...commit.heads.map((name): GitRef => ({ hash: commit.hash, name, type: "head" })), ...commit.tags.map( - (tag): GitRef => ({ hash: commit.hash, name: tag.name, type: "tag", signed: false }) + (tag): GitRef => ({ hash: commit.hash, name: tag.name, type: "tag", signed: tag.signed }) ), ...commit.remotes.map( (remote): GitRef => ({ hash: commit.hash, name: remote.name, type: "remote" }) @@ -347,169 +344,14 @@ export function mapEngineCommitData(data: EngineCommitData, showStashes: boolean return nodes; } -/** One remote `HEAD` symref target as the fill reads it. */ -export type RemoteHeadLabel = { - hash: string; - name: string; -}; - -const remoteHeadLineEndings = /\r\n|\r|\n/; - -/** - * Parse a `for-each-ref` symref scan over `refs/remotes`. Only symrefs carry - * a target, so a line with an empty third field is a plain ref the engine - * already recorded. Nothing here reimplements a CLI parse: the shape mirrors - * the loader's own ref records, narrowed to the symbolic labels. - */ -export function parseRemoteHeadLabels(stdout: string): RemoteHeadLabel[] { - const labels: RemoteHeadLabel[] = []; - for (const line of stdout.split(remoteHeadLineEndings)) { - if (line === "") continue; - const [hash = "", refName = "", symref = ""] = line.split("\0"); - if (hash === "" || symref === "" || !refName.startsWith("refs/remotes/")) continue; - labels.push({ hash, name: refName.slice("refs/remotes/".length) }); - } - return labels; -} - -/** - * Attach remote `HEAD` symref labels to the nodes at their targets, in - * `for-each-ref` byte order among the node's remote labels. Hidden remotes - * stay hidden via the CLI's own predicate; labels whose target is off-page - * or already recorded are skipped. - */ -export function insertRemoteHeadLabels( - nodes: GitCommitNode[], - labels: RemoteHeadLabel[], - hiddenRemotes?: string[] -): void { - if (labels.length === 0) return; - const byHash = new Map(); - for (const node of nodes) { - if (!byHash.has(node.hash)) byHash.set(node.hash, node); - } - for (const label of labels) { - insertOneRemoteHeadLabel(byHash, label, hiddenRemotes); - } -} - -function insertOneRemoteHeadLabel( - byHash: Map, - label: RemoteHeadLabel, - hiddenRemotes?: string[] -): void { - if (isHiddenRemoteRef(label.name, hiddenRemotes)) return; - const node = byHash.get(label.hash); - if (node === undefined) return; - if (node.refs.some((ref) => ref.type === "remote" && ref.name === label.name)) return; - const ref: GitRef = { hash: label.hash, name: label.name, type: "remote" }; - node.refs.splice(remoteHeadInsertIndex(node.refs, remoteRefName(label.name)), 0, ref); -} - -function remoteHeadInsertIndex(refs: GitRef[], fullName: string): number { - for (let index = 0; index < refs.length; index++) { - const existing = refs[index]; - if ( - refSortRank(existing) > 1 || - (refSortRank(existing) === 1 && compareRefNames(fullRefName(existing), fullName) > 0) - ) { - return index; - } - } - return refs.length; -} - -export type RemoteHeadFills = { - git: SimpleGit; - repo: string; - recordGitCommand?: GitCommandRecorder; -}; - -/** - * One narrow `for-each-ref` over `refs/remotes` for the symbolic `HEAD` - * labels the engine never records, attached in CLI order. A failed scan - * resolves to no labels rather than a failed graph — the same trade the - * stash rows make: the engine served the page, and losing it over pendant - * labels would be the wrong trade. - */ -export async function attachRemoteHeadLabels( - fills: RemoteHeadFills, - nodes: GitCommitNode[], - hiddenRemotes?: string[] -): Promise { - let stdout: string; - try { - stdout = await runGitRaw(fills.git, { - label: "loadCommits.remoteHeads", - args: ["for-each-ref", "--format=%(objectname)%00%(refname)%00%(symref)", "refs/remotes"], - repo: fills.repo, - record: fills.recordGitCommand - }); - } catch { - return; - } - insertRemoteHeadLabels(nodes, parseRemoteHeadLabels(stdout), hiddenRemotes); -} - -/** - * Tag names carrying a signature block, as the fill reads them. Only - * annotated tags can carry one, so every name here flips a badge the CLI - * would also show. - */ -export function parseSignedTagNames(stdout: string): string[] { - const signed: string[] = []; - for (const line of stdout.split(remoteHeadLineEndings)) { - if (line === "") continue; - const [refName = "", hasSignature = ""] = line.split("\0"); - if (hasSignature !== "1" || !refName.startsWith("refs/tags/")) continue; - signed.push(refName.slice("refs/tags/".length)); - } - return signed; -} - -/** Flip the signed badge on the named tag labels. Unknown names are ignored. */ -export function applySignedTagNames(nodes: GitCommitNode[], signed: string[]): void { - if (signed.length === 0) return; - const names = new Set(signed); - for (const node of nodes) { - for (const ref of node.refs) { - if (ref.type === "tag" && names.has(ref.name)) ref.signed = true; - } - } -} - -/** - * One narrow `for-each-ref` over `refs/tags` for the signature presence the - * engine never reports, reusing the loader's own signature atom so both - * scans classify identically. Same failure trade as the other fills: a - * failed scan keeps the page rather than failing the graph. - */ -export async function attachSignedTagNames( - fills: RemoteHeadFills, - nodes: GitCommitNode[] -): Promise { - let stdout: string; - try { - stdout = await runGitRaw(fills.git, { - label: "loadCommits.signedTags", - args: ["for-each-ref", `--format=%(refname)%00${gitRefSignatureAtom}`, "refs/tags"], - repo: fills.repo, - record: fills.recordGitCommand - }); - } catch { - return; - } - applySignedTagNames(nodes, parseSignedTagNames(stdout)); -} - /** * The `load_commits` options JSON. Every field is threaded from the route * input the CLI consumes, or pinned to the CLI-equivalent constant where the * CLI has no such knob: * - * - `showRemoteHeads: true`: the CLI's `for-each-ref` lists non-symbolic - * `/HEAD` refs, which the engine only includes with the flag on (symbolic - * remote HEADs stay an engine gap — probed in 16.5d); + * - `showRemoteHeads: true`: the CLI's `for-each-ref` lists every `/HEAD` ref, + * which the engine only includes with the flag on — symbolic ones included, + * since the engine resolves those the way `%(objectname)` reports them; * - `showUntrackedFiles: true`: the CLI counts every `status.files` entry, * untracked files included; * - `showTags` covers tags shown *or* selected as filters (the CLI scans diff --git a/src/backend/engine/index.ts b/src/backend/engine/index.ts index fab40586..58527784 100644 --- a/src/backend/engine/index.ts +++ b/src/backend/engine/index.ts @@ -30,8 +30,6 @@ import type { EngineBackend } from "@/types"; import { type EngineAddon, loadEngineAddon } from "./addon"; import { - attachRemoteHeadLabels, - attachSignedTagNames, buildLoadCommitsOptions, engineLoadCommitsRefs, type EngineLoadCommitsInput, @@ -341,27 +339,6 @@ async function readCommits( ) { return cliRead(); } - // The engine never records symbolic remote HEADs (`origin/HEAD`): one - // narrow scan attaches them in CLI order, but only when the page carries - // remote labels at all — a repository without remotes pays no spawn. - if ( - args.showRemoteBranches && - nodes.some((node) => node.refs.some((ref) => ref.type === "remote")) - ) { - await attachRemoteHeadLabels( - { git: args.git, repo: args.repoPath, recordGitCommand: args.recordGitCommand }, - nodes, - args.hiddenRemotes - ); - } - // The engine never reports tag signature presence either: one narrow - // scan flips the badges, but only when the page carries tag labels. - if (nodes.some((node) => node.refs.some((ref) => ref.type === "tag"))) { - await attachSignedTagNames( - { git: args.git, repo: args.repoPath, recordGitCommand: args.recordGitCommand }, - nodes - ); - } engineServedRead = true; return { commits: nodes, diff --git a/src/config.ts b/src/config.ts index d106dde8..c63792c2 100644 --- a/src/config.ts +++ b/src/config.ts @@ -159,7 +159,7 @@ export const config = { includeReflog: (): boolean => getConfig("repository.includeReflog", false), includeUnreachableCommits: (): boolean => getConfig("repository.includeUnreachableCommits", false), - loadMoreCommits: (): number => getConfig("loadMoreCommits", 75), + loadMoreCommits: (): number => getConfig("loadMoreCommits", 100), maxDepthOfRepoSearch: (): number => getConfig("maxDepthOfRepoSearch", 0), muteCommitsNotAncestorsOfHead: (): boolean => getConfig("repository.muteCommitsNotAncestorsOfHead", false), diff --git a/tests/backend/config.test.ts b/tests/backend/config.test.ts index 1b507f43..befb6638 100644 --- a/tests/backend/config.test.ts +++ b/tests/backend/config.test.ts @@ -265,7 +265,7 @@ describe("configuration", () => { { accessor: "initialLoadCommits", expected: 300 }, { accessor: "includeReflog", expected: false }, { accessor: "includeUnreachableCommits", expected: false }, - { accessor: "loadMoreCommits", expected: 75 }, + { accessor: "loadMoreCommits", expected: 100 }, { accessor: "maxDepthOfRepoSearch", expected: 0 }, { accessor: "muteCommitsNotAncestorsOfHead", expected: false }, { accessor: "muteMergeCommits", expected: false }, diff --git a/tests/backend/engine/commits.test.ts b/tests/backend/engine/commits.test.ts index 7dab845d..d8c2f866 100644 --- a/tests/backend/engine/commits.test.ts +++ b/tests/backend/engine/commits.test.ts @@ -1,21 +1,16 @@ import { describe, expect, it } from "vitest"; import { - applySignedTagNames, buildLoadCommitsOptions, type EngineCommit, type EngineCommitData, type EngineLoadCommitsInput, engineLoadCommitsRefs, - insertRemoteHeadLabels, mapEngineCommitData, parseEngineCommitData, - parseRemoteHeadLabels, - parseSignedTagNames, shortStashRef, shouldServeLoadCommitsFromEngine } from "@/backend/engine/commits"; -import type { GitCommitNode } from "@/backend/types"; const BASE: EngineLoadCommitsInput = { branchName: "", @@ -165,7 +160,7 @@ const COMMIT: EngineCommit = { date: 1790090408, message: "second", heads: ["main"], - tags: [{ name: "v1.0.0", annotated: false }], + tags: [{ name: "v1.0.0", annotated: false, signed: false }], remotes: [{ name: "origin/main", remote: "origin" }], stash: null }; @@ -232,108 +227,6 @@ describe("shortStashRef", () => { }); }); -describe("parseRemoteHeadLabels", () => { - it("keeps only symref lines under refs/remotes", () => { - const stdout = [ - `${"a".repeat(40)}\0refs/remotes/origin/HEAD\0refs/remotes/origin/main`, - `${"a".repeat(40)}\0refs/remotes/origin/main\0`, - `${"b".repeat(40)}\0refs/heads/main\0refs/heads/main`, - "garbage", - "" - ].join("\n"); - expect(parseRemoteHeadLabels(stdout)).toEqual([{ hash: "a".repeat(40), name: "origin/HEAD" }]); - }); -}); - -describe("parseSignedTagNames", () => { - it("keeps only signature-carrying lines under refs/tags", () => { - const stdout = [ - `refs/tags/faketag\0${"1"}`, - `refs/tags/v1.0.0\0${"0"}`, - `refs/remotes/origin/main\0${"1"}`, - "garbage", - "" - ].join("\n"); - expect(parseSignedTagNames(stdout)).toEqual(["faketag"]); - }); -}); - -describe("applySignedTagNames", () => { - it("flips the badge on named tag labels and ignores the rest", () => { - const nodes: GitCommitNode[] = [ - { - hash: "a".repeat(40), - parentHashes: [], - author: "Ada", - email: "ada@x.com", - date: 1, - message: "tip", - refs: [ - { hash: "a".repeat(40), name: "main", type: "head" }, - { hash: "a".repeat(40), name: "faketag", type: "tag", signed: false }, - { hash: "a".repeat(40), name: "v1.0.0", type: "tag", signed: false } - ] - } - ]; - applySignedTagNames(nodes, ["faketag", "missing"]); - expect(nodes[0]?.refs.map((ref) => ref.signed)).toEqual([undefined, true, false]); - const before = JSON.stringify(nodes); - applySignedTagNames(nodes, []); - expect(JSON.stringify(nodes)).toBe(before); - }); -}); - -describe("insertRemoteHeadLabels", () => { - const node = (): GitCommitNode => ({ - hash: "a".repeat(40), - parentHashes: [], - author: "Ada", - email: "ada@x.com", - date: 1, - message: "tip", - refs: [ - { hash: "a".repeat(40), name: "main", type: "head" }, - { hash: "a".repeat(40), name: "origin/main", type: "remote" }, - { hash: "a".repeat(40), name: "v1.0.0", type: "tag", signed: false } - ] - }); - - it("inserts in for-each-ref order among the remote labels", () => { - const nodes = [node()]; - insertRemoteHeadLabels(nodes, [{ hash: "a".repeat(40), name: "origin/HEAD" }]); - expect(nodes[0]?.refs.map((ref) => `${ref.type}:${ref.name}`)).toEqual([ - "head:main", - "remote:origin/HEAD", - "remote:origin/main", - "tag:v1.0.0" - ]); - }); - - it("inserts past off-page targets and duplicates", () => { - const nodes = [node()]; - insertRemoteHeadLabels(nodes, [ - { hash: "f".repeat(40), name: "origin/HEAD" }, - { hash: "a".repeat(40), name: "origin/main" }, - { hash: "a".repeat(40), name: "origin/HEAD" } - ]); - expect(nodes[0]?.refs.map((ref) => `${ref.type}:${ref.name}`)).toEqual([ - "head:main", - "remote:origin/HEAD", - "remote:origin/main", - "tag:v1.0.0" - ]); - }); - - it("keeps hidden remotes hidden and ignores empty fills", () => { - const nodes = [node()]; - insertRemoteHeadLabels(nodes, [{ hash: "a".repeat(40), name: "origin/HEAD" }], ["origin"]); - expect(nodes[0]?.refs).toHaveLength(3); - const before = JSON.stringify(nodes); - insertRemoteHeadLabels(nodes, []); - expect(JSON.stringify(nodes)).toBe(before); - }); -}); - describe("mapEngineCommitData", () => { it("maps labels onto project refs and leaves the signature key absent", () => { const [node] = mapEngineCommitData(PAGE, true); @@ -362,8 +255,8 @@ describe("mapEngineCommitData", () => { ...COMMIT, heads: ["zebra", "alpha"], tags: [ - { name: "v1.0.0", annotated: false }, - { name: "a-tag", annotated: true } + { name: "v1.0.0", annotated: false, signed: false }, + { name: "a-tag", annotated: true, signed: false } ], remotes: [ { name: "origin/main", remote: "origin" }, From d5593747a27d4f7c111c142628717c7f425ddcac Mon Sep 17 00:00:00 2001 From: Zamkorus Date: Wed, 23 Sep 2026 17:44:13 +0200 Subject: [PATCH 05/10] feat(view): load 1000 commits per "load more", keeping the first page at 300 The first page is the latency-critical one and stays at 300. The follow-on page goes from 100 to 1000, which is the better trade for how the table is actually rendered. `renderTable` builds one HTML string over every loaded commit and assigns it to `innerHTML`, so each "load more" rebuilds the whole table rather than appending to it. Reaching 3,000 commits therefore costs about 27 growing rebuilds at a step of 100 and about 3 at a step of 1,000 - a larger step is strictly less total work, not more. It also matters more than it looks, because `autoLoadMoreCommitsOnScroll` fires whenever the viewport comes within 96px of the bottom, so the small step stalls repeatedly during ordinary scrolling rather than only on a click. Measured through the webview harness, a full rebuild is 921 ms at 1,000 rows and 2,783 ms at 2,000. Those are jsdom figures and are not browser figures - jsdom parses HTML far more slowly and does no layout or paint - but they establish that the rebuild is at least linear in total rows with a constant that is not small. The engine-side read is negligible by comparison: 13.6 ms for 1,000 commits. The real fix is to append new rows instead of regenerating the table, making a page nearly free; that is a separate change to `renderTable` and is recorded in the knowledge base rather than attempted here. Manifest, accessor, README and the config test are set together, since that table mirrors the manifest by hand and had already drifted once. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 7 +++++ README.md | 2 +- docs/AI_DEV_KNOWLEDGE_BASE.md | 50 +++++++++++++++++++++++++++-------- package.json | 2 +- src/config.ts | 2 +- tests/backend/config.test.ts | 2 +- 6 files changed, 50 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d00e5d6a..94172e2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 that engine and the previous behavior. `"git-cli"` is a complete opt-out: the engine is not even loaded. +### Changed + +- **"Load more" now loads 1000 commits at a time instead of 100.** The graph + still opens with the first 300, so it appears just as quickly, but scrolling + further into history now pauses about a tenth as often. `loadMoreCommits` + remains configurable if you want the old behavior. + ### Fixed - **Signed tags keep their badge, and `origin/HEAD` keeps its label**, without diff --git a/README.md b/README.md index 9954a4cf..1e9f1fbb 100644 --- a/README.md +++ b/README.md @@ -87,7 +87,7 @@ All settings use the `git-graph-libre` prefix. | `graphColors` | 12 defaults | Colors for graph lines | | `graphStyle` | `"rounded"` | `"rounded"` or `"angular"` | | `initialLoadCommits` | `300` | Commits to load on open | -| `loadMoreCommits` | `100` | Commits to load on demand | +| `loadMoreCommits` | `1000` | Commits to load on demand | | `maxDepthOfRepoSearch` | `0` | Folder depth for repo search | | `repository.boldCheckedOutCommit` | `false` | Bold the checked-out commit's message | | `repository.fetchTagsByDefault` | `true` | Pre-check "Fetch all tags" in the Fetch dialog | diff --git a/docs/AI_DEV_KNOWLEDGE_BASE.md b/docs/AI_DEV_KNOWLEDGE_BASE.md index da24a1ae..bdbf8c68 100644 --- a/docs/AI_DEV_KNOWLEDGE_BASE.md +++ b/docs/AI_DEV_KNOWLEDGE_BASE.md @@ -2909,21 +2909,49 @@ when every read was a `git` spawn. 700 extra commits cost about `6 ms` of git and essentially nothing to lay out. -**What was not measured, and why the default therefore stands.** DOM row -insertion — building and inserting the table rows and SVG paths — was not -measured, because jsdom is not representative of a real browser there. That is -the one cost that could still justify a cap, and it is the only one left. **Do -not raise `initialLoadCommits` on the evidence above alone**; measure row -insertion in a real webview first. - -**A defaults drift found while answering.** `loadMoreCommits` is declared +**The render is the real cost, and it scales with the whole table.** +`renderTable` builds one HTML string over `this.commits` — *all* of them, not +the new page — and assigns it to `tableElem.innerHTML`. So every "load more" +rebuilds the entire table, and the cost of reaching N commits is the sum of +every rebuild along the way. Driving the real load-more flow through the +webview harness: + +| table size | full rebuild (jsdom) | +| ---: | ---: | +| 1,000 rows | `921 ms` | +| 2,000 rows | `2,783 ms` | + +**Those are jsdom numbers and must not be quoted as browser numbers** — jsdom +parses HTML far slower than a browser and does no layout or paint at all. What +they do establish is the *shape*: the rebuild is at least linear in total rows +and the constant is not small. + +The consequence decides the page sizes. Because the whole table is rebuilt +each time, a larger `loadMoreCommits` means strictly *less* total work, not +more: reaching 3,000 commits costs about 27 rebuilds of a growing table at a +step of 100, and about 3 at a step of 1,000. The trade is fewer, larger +stalls instead of many smaller ones — and since `autoLoadMoreCommitsOnScroll` +fires whenever the viewport comes within 96 px of the bottom, the small-step +version stalls repeatedly during ordinary scrolling. + +**Set on `2026-09-23` at the maintainer's direction**: `initialLoadCommits` +stays `300` (it is the latency-critical first paint, and 300 rows render +quickly), `loadMoreCommits` goes from `100` to `1000`. + +**The real fix, not done here.** The rebuild is `O(total)` per load when it +could be `O(step)` — appending the new rows instead of regenerating the table +would make page size nearly free and remove the stalls entirely. That is a +separate change to `renderTable` and its callers, and it is the thing to do if +these stalls are ever felt. + +**A defaults drift found while answering.** `loadMoreCommits` was declared `100` in the manifest and documented as `100` in the README, but `src/config.ts` fell back to `75`, and `tests/backend/config.test.ts` pinned the `75` — a wrong value frozen by the test meant to protect it. The fallback never fires in a real install, because VS Code returns the manifest default for -an unset key, so nothing user-visible was wrong; it is now `100` in all four -places. The accessor table in that test mirrors manifest defaults by hand, so -it can drift again. +an unset key, so nothing user-visible was wrong. All four now read `1000`. The +accessor table in that test mirrors manifest defaults by hand, so it can drift +again; keep the manifest, the accessor, the README table and that test in step. #### Benchmarking the two backends (`2026-09-23`) diff --git a/package.json b/package.json index 84e7d04d..cd09ed22 100644 --- a/package.json +++ b/package.json @@ -491,7 +491,7 @@ }, "git-graph-libre.loadMoreCommits": { "type": "number", - "default": 100, + "default": 1000, "description": "%config.loadMoreCommits%" }, "git-graph-libre.maxDepthOfRepoSearch": { diff --git a/src/config.ts b/src/config.ts index c63792c2..4d200095 100644 --- a/src/config.ts +++ b/src/config.ts @@ -159,7 +159,7 @@ export const config = { includeReflog: (): boolean => getConfig("repository.includeReflog", false), includeUnreachableCommits: (): boolean => getConfig("repository.includeUnreachableCommits", false), - loadMoreCommits: (): number => getConfig("loadMoreCommits", 100), + loadMoreCommits: (): number => getConfig("loadMoreCommits", 1000), maxDepthOfRepoSearch: (): number => getConfig("maxDepthOfRepoSearch", 0), muteCommitsNotAncestorsOfHead: (): boolean => getConfig("repository.muteCommitsNotAncestorsOfHead", false), diff --git a/tests/backend/config.test.ts b/tests/backend/config.test.ts index befb6638..5d39d6a4 100644 --- a/tests/backend/config.test.ts +++ b/tests/backend/config.test.ts @@ -265,7 +265,7 @@ describe("configuration", () => { { accessor: "initialLoadCommits", expected: 300 }, { accessor: "includeReflog", expected: false }, { accessor: "includeUnreachableCommits", expected: false }, - { accessor: "loadMoreCommits", expected: 100 }, + { accessor: "loadMoreCommits", expected: 1000 }, { accessor: "maxDepthOfRepoSearch", expected: 0 }, { accessor: "muteCommitsNotAncestorsOfHead", expected: false }, { accessor: "muteMergeCommits", expected: false }, From 4347769e3d5496d6a7717416b89a477dbce503b2 Mon Sep 17 00:00:00 2001 From: Zamkorus Date: Wed, 23 Sep 2026 19:40:05 +0200 Subject: [PATCH 06/10] feat(view): open with 250 commits and load 750 more at a time Replaces the 300/1000 set in 9751c15 at the maintainer's direction. The reasoning there is unchanged - `renderTable` rebuilds the whole table on every load, so a larger step is strictly less total work, and `autoLoadMoreCommitsOnScroll` makes the small step stall repeatedly during ordinary scrolling - only the two numbers move. 250 trims the latency-critical first paint slightly. 750 keeps the follow-on page well clear of the old 100 while sitting below the 1,000 the render figures were taken at, which is the conservative direction given those figures are jsdom's and not a browser's. Manifest, accessor, README and the config test move together, since that table mirrors the manifest by hand and had already drifted once. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 9 +++++---- README.md | 4 ++-- docs/AI_DEV_KNOWLEDGE_BASE.md | 10 +++++++--- package.json | 4 ++-- src/config.ts | 4 ++-- tests/backend/config.test.ts | 4 ++-- 6 files changed, 20 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 94172e2a..10239f7a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,10 +27,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- **"Load more" now loads 1000 commits at a time instead of 100.** The graph - still opens with the first 300, so it appears just as quickly, but scrolling - further into history now pauses about a tenth as often. `loadMoreCommits` - remains configurable if you want the old behavior. +- **"Load more" now loads 750 commits at a time instead of 100**, and the graph + opens with 250 rather than 300. The first page appears a touch sooner, and + scrolling further into history pauses roughly seven times less often. + `initialLoadCommits` and `loadMoreCommits` remain configurable if you want + the old behavior. ### Fixed diff --git a/README.md b/README.md index 1e9f1fbb..f1c28ab2 100644 --- a/README.md +++ b/README.md @@ -86,8 +86,8 @@ All settings use the `git-graph-libre` prefix. | `fetchAvatars` | `false` | Fetch avatars (sends email to external services) | | `graphColors` | 12 defaults | Colors for graph lines | | `graphStyle` | `"rounded"` | `"rounded"` or `"angular"` | -| `initialLoadCommits` | `300` | Commits to load on open | -| `loadMoreCommits` | `1000` | Commits to load on demand | +| `initialLoadCommits` | `250` | Commits to load on open | +| `loadMoreCommits` | `750` | Commits to load on demand | | `maxDepthOfRepoSearch` | `0` | Folder depth for repo search | | `repository.boldCheckedOutCommit` | `false` | Bold the checked-out commit's message | | `repository.fetchTagsByDefault` | `true` | Pre-check "Fetch all tags" in the Fetch dialog | diff --git a/docs/AI_DEV_KNOWLEDGE_BASE.md b/docs/AI_DEV_KNOWLEDGE_BASE.md index bdbf8c68..b3f8efd5 100644 --- a/docs/AI_DEV_KNOWLEDGE_BASE.md +++ b/docs/AI_DEV_KNOWLEDGE_BASE.md @@ -2929,14 +2929,18 @@ and the constant is not small. The consequence decides the page sizes. Because the whole table is rebuilt each time, a larger `loadMoreCommits` means strictly *less* total work, not more: reaching 3,000 commits costs about 27 rebuilds of a growing table at a -step of 100, and about 3 at a step of 1,000. The trade is fewer, larger +step of 100, and about 4 at a step of 750. The trade is fewer, larger stalls instead of many smaller ones — and since `autoLoadMoreCommitsOnScroll` fires whenever the viewport comes within 96 px of the bottom, the small-step version stalls repeatedly during ordinary scrolling. **Set on `2026-09-23` at the maintainer's direction**: `initialLoadCommits` -stays `300` (it is the latency-critical first paint, and 300 rows render -quickly), `loadMoreCommits` goes from `100` to `1000`. +`300` -> `250`, `loadMoreCommits` `100` -> `750`. The first page is the +latency-critical paint and is trimmed slightly; the follow-on page is the one +that was costing repeated stalls and is raised well clear of them. Both sit +below the round numbers this section measured, which is the conservative +direction given that the render figures below are jsdom's and not a +browser's. **The real fix, not done here.** The rebuild is `O(total)` per load when it could be `O(step)` — appending the new rows instead of regenerating the table diff --git a/package.json b/package.json index cd09ed22..b4ca0d50 100644 --- a/package.json +++ b/package.json @@ -486,12 +486,12 @@ }, "git-graph-libre.initialLoadCommits": { "type": "number", - "default": 300, + "default": 250, "description": "%config.initialLoadCommits%" }, "git-graph-libre.loadMoreCommits": { "type": "number", - "default": 1000, + "default": 750, "description": "%config.loadMoreCommits%" }, "git-graph-libre.maxDepthOfRepoSearch": { diff --git a/src/config.ts b/src/config.ts index 4d200095..c045eea6 100644 --- a/src/config.ts +++ b/src/config.ts @@ -155,11 +155,11 @@ export const config = { MAX_SHORT_HASH_LENGTH ) ), - initialLoadCommits: (): number => getConfig("initialLoadCommits", 300), + initialLoadCommits: (): number => getConfig("initialLoadCommits", 250), includeReflog: (): boolean => getConfig("repository.includeReflog", false), includeUnreachableCommits: (): boolean => getConfig("repository.includeUnreachableCommits", false), - loadMoreCommits: (): number => getConfig("loadMoreCommits", 1000), + loadMoreCommits: (): number => getConfig("loadMoreCommits", 750), maxDepthOfRepoSearch: (): number => getConfig("maxDepthOfRepoSearch", 0), muteCommitsNotAncestorsOfHead: (): boolean => getConfig("repository.muteCommitsNotAncestorsOfHead", false), diff --git a/tests/backend/config.test.ts b/tests/backend/config.test.ts index 5d39d6a4..7c7f3a6e 100644 --- a/tests/backend/config.test.ts +++ b/tests/backend/config.test.ts @@ -262,10 +262,10 @@ describe("configuration", () => { { accessor: "graphRowHeight", expected: 24 }, { accessor: "revealHighlightColor", expected: "oklch(90% 0.25 150 / 0.42)" }, { accessor: "shortHashLength", expected: 8 }, - { accessor: "initialLoadCommits", expected: 300 }, + { accessor: "initialLoadCommits", expected: 250 }, { accessor: "includeReflog", expected: false }, { accessor: "includeUnreachableCommits", expected: false }, - { accessor: "loadMoreCommits", expected: 1000 }, + { accessor: "loadMoreCommits", expected: 750 }, { accessor: "maxDepthOfRepoSearch", expected: 0 }, { accessor: "muteCommitsNotAncestorsOfHead", expected: false }, { accessor: "muteMergeCommits", expected: false }, From 614a6bb163d50a81bd32a10a33768fb9e48e0849 Mon Sep 17 00:00:00 2001 From: Zamkorus Date: Wed, 23 Sep 2026 20:10:18 +0200 Subject: [PATCH 07/10] feat(engine): search history in the engine, and fix the author search's escaping Slice 16.7 declined `search_history` because the engine's search and this project's search answer different questions: a regex over messages across every ref, against a literal substring search plus author matching, hash resolution, ref filters and a position for each hit. The decline was right; the conclusion that search therefore stays on four `git` processes was not. The engine gains `search_commits`, which reproduces these semantics. `search_history` stays where it is, unused. The CLI runs `--fixed-strings --grep`, `--author`, a hash lookup and one unbounded walk that numbers every commit, then merges by that numbering. The numbering is also a filter - a hit with no position is dropped, which is why a hash resolving to an unreachable commit is not a result. The engine does the same three matches in a single walk, which is where the speed comes from: 62.0 ms -> 7.8 ms (7.9x) on a 1,036-commit repository. Pinned at the boundaries the two would disagree on - eight parity cases and twelve engine-side tests: a literal dot a regex would widen, a query that is not valid regex, case folding, a body-only term `--grep` reaches and `%s` does not show, an author-only match, an abbreviated hash, a hash that resolves but is unreachable, and a `--glob=` pattern that still declines to the CLI. The parity table then failed on the CLI side, which is the point of having one. `searchCommits` escaped its `--author` query with a JavaScript-style `escapeRegExp`, but `--author` takes a *basic* regular expression, where `\(` opens a group rather than escaping a parenthesis. The escaping inverted the meaning, and since the four runs share a `Promise.all`, any query containing an unbalanced `(` or `[` failed the whole Find dialogue with `fatal: header, '\(': Unmatched ( or \(`. `--fixed-strings` expresses the literal match that was always intended, and still matches name and email substrings ignoring case - verified against git before changing anything. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 9 + docs/AI_DEV_KNOWLEDGE_BASE.md | 69 ++++++- engine/native/core/src/lib.rs | 1 + engine/native/core/src/log.rs | 2 +- engine/native/core/src/search.rs | 238 +++++++++++++++++++++++++ engine/native/core/src/types.rs | 38 ++++ engine/native/core/tests/common/mod.rs | 24 +++ engine/native/core/tests/search.rs | 227 +++++++++++++++++++++++ engine/native/node/src/lib.rs | 19 +- src/backend/engine/addon.ts | 9 + src/backend/engine/index.ts | 70 +++++++- src/backend/engine/search.ts | 134 ++++++++++++++ src/backend/queries/searchCommits.ts | 19 +- src/extension/messageHandler.ts | 13 +- tests/backend/engine/addon.test.ts | 1 + tests/backend/engine/backends.bench.ts | 16 ++ tests/backend/engine/parity.test.ts | 158 ++++++++++++++++ tests/backend/engine/reader.test.ts | 11 ++ 18 files changed, 1044 insertions(+), 14 deletions(-) create mode 100644 engine/native/core/src/search.rs create mode 100644 engine/native/core/tests/search.rs create mode 100644 src/backend/engine/search.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 10239f7a..79b695ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **Searching history is about eight times faster**, and no longer starts four + `git` processes for one search. Measured on a real 1,036-commit repository: + 62 ms to 8 ms. What it matches is unchanged — the same literal text search + over messages, the same author and commit-hash matching, in the same order. - **The graph opens roughly four times faster.** Measured on a real 1,036-commit repository: the work behind opening the view went from 69 ms to 16 ms, reading the branch, tag, remote and stash lists from 60 ms to 8 ms, @@ -35,6 +39,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Searching for text containing `(` or `[` no longer fails the Find + dialogue.** A query with an unbalanced bracket made Git reject the search + outright and the dialogue reported an error instead of results. Author + matching is unchanged otherwise — it still matches names and email addresses, + ignoring case. - **Signed tags keep their badge, and `origin/HEAD` keeps its label**, without the extension running extra `git` commands to find either out. Both are now read directly from the repository along with everything else on the row. A diff --git a/docs/AI_DEV_KNOWLEDGE_BASE.md b/docs/AI_DEV_KNOWLEDGE_BASE.md index b3f8efd5..1379ed25 100644 --- a/docs/AI_DEV_KNOWLEDGE_BASE.md +++ b/docs/AI_DEV_KNOWLEDGE_BASE.md @@ -2527,7 +2527,11 @@ Implementation record (`2026-09-23`, four subslice commits, all signed): global file staying CLI with the CLI's own error shape. - Declines, each with its reason — no code, no behavior change: `search_history` (the engine matches message regex while the CLI - searches fixed strings plus author, hash, positions and ref filters); + searches fixed strings plus author, hash, positions and ref filters) + — **superseded on `2026-09-23`**: rather than wire the mismatched + function, the engine gained a `search_commits` that reproduces this + project's semantics, and the search now routes through it. See "Search + in the engine" below. `search_history` itself stays unused; `load_tag_details` (the engine reports signatures present but unverified; verification is a permanent non-goal); `current_branch_*` and `remote_names` (their consumers are `kind: "action"` write flows, @@ -2889,6 +2893,69 @@ the **debug** profile (186 MB, unoptimised) while the benchmark numbers above are all release (5.9 MB). Always `pnpm run engine:build:release` before benchmarking; a debug addon is not a slow engine, it is a different one. +#### Search in the engine, and a CLI bug it exposed (`2026-09-23`) + +Slice 16.7 declined `search_history` because the engine's search and this +project's search answer different questions. That decline was correct and the +conclusion drawn from it was not: the fix is not to wire the mismatched +function, and not to leave search on four `git` processes, but to give the +engine a search with *these* semantics. `search_commits` is that function, new +in `engine/native/core/src/search.rs`, and `search_history` stays where it is, +unused. + +What had to be reproduced, and what the engine's own search does instead: + +| | this project | `log::search_history` | +| --- | --- | --- | +| message match | literal substring, case-insensitive | regular expression | +| author match | yes | no | +| hash match | yes, abbreviated resolves | no | +| refs searched | the ones the view shows | every ref, always | +| author filter | honoured | ignored | +| ordering | position in the graph's walk | commit date | + +The CLI answers with four `git log` runs at once — `--fixed-strings --grep`, +`--author`, a hash lookup, and one unbounded walk that numbers every commit. +That numbering is the `loadCount` each result carries, and it is also a +*filter*: a hit with no position is dropped, which is why a hash that resolves +to an unreachable commit is not a result. The engine does the same three +matches in a single walk, which is where the speed comes from. + +Verified deliberately at the boundaries the two disagree on, in the parity +table (`engine/CLI parity: searchCommits`, eight cases, plus twelve engine-side +tests): a literal dot that a regex would widen, a query that is not valid +regex at all, case folding, a body-only term that `--grep` reaches and `%s` +does not show, an author-only match, an abbreviated hash, a hash that resolves +but is unreachable, and a `--glob=` ref pattern that still declines to the CLI. + +**The CLI bug the parity table found.** The `(` case failed — not because the +engine was wrong, but because **git was rejecting the search outright**: + +``` +fatal: header, '\(': Unmatched ( or \( +``` + +`searchCommits` escaped the query for its `--author` run with a +JavaScript-style `escapeRegExp`, turning `(` into `\(`. But `--author` takes a +*basic* regular expression, in which `\(` **opens a group** rather than +escaping a parenthesis — so the escaping inverted the meaning, and because the +four runs share a `Promise.all`, any query containing an unbalanced `(` or `[` +failed the entire Find dialogue. A literal match was always the intent; +`--fixed-strings` is how git spells it, and it matches name and email +substrings case-insensitively exactly as before. Verified against git directly +before changing anything. + +This is worth recording as a pattern, not just a fix: the parity table earns +its keep by failing on the *CLI* side. Two implementations that must agree +catch bugs in whichever one is wrong. + +| | git CLI | engine | | +| --- | ---: | ---: | ---: | +| search (message term), 1,036 commits | `62.0 ms` | **`7.8 ms`** | `7.9x` | + +Second only to `loadRepoInfo` among the wired reads, because four processes +collapse to one walk. + #### What the page-size defaults are actually worth (`2026-09-23`) Asked why `initialLoadCommits` is `300` when 1,000 costs only milliseconds diff --git a/engine/native/core/src/lib.rs b/engine/native/core/src/lib.rs index 66ada751..8b38ad65 100644 --- a/engine/native/core/src/lib.rs +++ b/engine/native/core/src/lib.rs @@ -20,6 +20,7 @@ pub mod graph; pub mod log; pub mod refs; pub mod repository; +pub mod search; pub mod stash; pub mod stats; pub mod status; diff --git a/engine/native/core/src/log.rs b/engine/native/core/src/log.rs index da6e5937..59f3fc48 100644 --- a/engine/native/core/src/log.rs +++ b/engine/native/core/src/log.rs @@ -174,7 +174,7 @@ pub fn read_commit(commit: &gix::Commit<'_>) -> Result { /// trailing "<", a name that is a textual prefix of another author's name (e.g. "Bob" inside /// "Bobby ") would match commits it should not. This mirrors that exactly, case- /// insensitively (case sensitivity is the one place this still deviates from a bare `git log`). -fn commit_matches_author(commit: &gix::Commit<'_>, authors: &[String]) -> Result { +pub(crate) fn commit_matches_author(commit: &gix::Commit<'_>, authors: &[String]) -> Result { let author = commit .author() .git_ctx("Could not decode the commit author")?; diff --git a/engine/native/core/src/search.rs b/engine/native/core/src/search.rs new file mode 100644 index 00000000..a3e309e1 --- /dev/null +++ b/engine/native/core/src/search.rs @@ -0,0 +1,238 @@ +//! The Find dialogue's commit search, reproducing what the `git` CLI backend does. +//! +//! This is deliberately *not* the engine's original `log::search_history`, which matches a regular +//! expression against commit messages across every ref. This project's search is a different +//! question, and wiring the regex one would have changed what users see: +//! +//! | | this search | `log::search_history` | +//! | --- | --- | --- | +//! | message match | literal substring, case-insensitive | regular expression | +//! | author match | yes, literal substring | no | +//! | hash match | yes, abbreviated hashes resolve | no | +//! | refs searched | the ones the view is showing | every ref, always | +//! | author filter | honoured | ignored | +//! | ordering | position in the graph's own walk | commit date, newest first | +//! +//! The CLI reaches the answer with four `git log` invocations run together — a literal +//! `--fixed-strings --grep`, an `--author`, a hash lookup, and one unbounded walk that numbers +//! every commit. That numbering is the `loadCount` each result carries: "how far into the graph +//! you would have to load to reach this commit". Everything here is one walk instead of four +//! processes, producing the same three match sets and the same numbering. + +use std::collections::HashMap; + +use crate::error::{Result, ResultExt}; +use crate::log; +use crate::refs::read_refs; +use crate::repository::Repo; +use crate::types::{GitSearchResult, RefReadOptions, SearchOptions}; + +/// A query that could be an abbreviated object id. Mirrors the CLI's `/^[0-9a-f]{4,40}$/i`, which +/// is what decides whether the hash lookup runs at all. +fn is_hash_like(query: &str) -> bool { + let length = query.len(); + (4..=40).contains(&length) && query.chars().all(|c| c.is_ascii_hexdigit()) +} + +/// The tips the search walks, which are the refs the *view is showing* rather than everything. +/// +/// The CLI builds these as `--branches`, plus `--tags` and `--remotes` when those are shown, with +/// hidden remotes excluded. Two absences are deliberate and both are the CLI's: `HEAD` is not a +/// tip, so a detached HEAD's own commits are not searched, and neither is the stash. +fn search_tips(repo: &Repo, options: &SearchOptions) -> Result> { + // An explicit selection (branches and/or tags chosen in the dropdowns) replaces the lot, which + // is what the CLI's `refArgs` does when it has any selected refs. + if let Some(branches) = &options.branches { + return log::resolve_tips(repo, branches); + } + + let ref_options = RefReadOptions { + show_remote_branches: options.show_remote_branches, + // `--remotes` lists non-symbolic remote refs; a symbolic `origin/HEAD` aliases a branch + // that is already a tip in its own right, so including it would only duplicate work. + show_remote_heads: false, + hide_remotes: options.hide_remotes.clone(), + }; + let snapshot = read_refs(repo, &ref_options)?; + + let mut revisions: Vec = Vec::new(); + revisions.extend(snapshot.ref_data.heads.iter().map(|head| head.hash.clone())); + if options.show_tags { + revisions.extend(snapshot.ref_data.tags.iter().map(|tag| tag.hash.clone())); + } + if options.show_remote_branches { + revisions.extend( + snapshot + .ref_data + .remotes + .iter() + .map(|remote| remote.hash.clone()), + ); + } + log::resolve_tips(repo, &revisions) +} + +/// Does this commit's author line contain the query? +/// +/// ### Deviation +/// +/// git's `--author` matches the whole `author Name ` header, so a query of +/// bare digits can match a commit's timestamp there and not here. Matching the rendered timestamp +/// would mean reproducing git's date formatting exactly, which is a larger risk than the case it +/// covers: this matches `Name `, which is every realistic author query. +fn author_contains(author: &gix::actor::SignatureRef<'_>, needle_lower: &str) -> bool { + let haystack = format!("{} <{}>", author.name, author.email).to_lowercase(); + haystack.contains(needle_lower) +} + +/// One commit rendered into the result shape, at the position the walk gave it. +fn to_result( + commit: &gix::Commit<'_>, + load_count: u32, + use_author_date: bool, +) -> Result { + let author = commit + .author() + .git_ctx("Could not decode the commit author")?; + let date = if use_author_date { + author.time().map(|time| time.seconds).unwrap_or(0) + } else { + commit + .committer() + .git_ctx("Could not decode the commit committer")? + .time() + .map(|time| time.seconds) + .unwrap_or(0) + }; + Ok(GitSearchResult { + hash: commit.id().detach().to_string(), + parents: commit + .parent_ids() + .map(|id| id.detach().to_string()) + .collect(), + author: author.name.to_string(), + email: author.email.to_string(), + date, + // The subject alone, which is what `%s` prints and what the dialogue lists. The match above + // ran against the whole message, exactly as `--grep` does. + message: commit + .message() + .git_ctx("Could not decode the commit message")? + .summary() + .to_string(), + load_count, + }) +} + +/// Search the commits the view is showing, as the `git` CLI backend searches them. +pub fn search_commits(repo: &Repo, options: &SearchOptions) -> Result> { + let query = options.query.trim(); + if query.is_empty() || options.max_results == 0 { + return Ok(Vec::new()); + } + let needle = query.to_lowercase(); + let limit = options.max_results as usize; + + let tips = search_tips(repo, options)?; + if tips.is_empty() { + return Ok(Vec::new()); + } + + let git = repo.borrow(); + let walk = git + .rev_walk(tips.iter().copied()) + .sorting(gix::revision::walk::Sorting::ByCommitTime( + gix::traverse::commit::simple::CommitTimeOrder::NewestFirst, + )) + .all() + .git_ctx("Could not walk the commit graph")?; + + // The three match sets the CLI produces with three separate `git log` runs. Each is capped at + // `max_results` on its own, because each of the CLI's runs carries its own `--max-count`; the + // merge below then re-slices the union. + let mut message_hits: Vec = Vec::new(); + let mut author_hits: Vec = Vec::new(); + let mut positions: HashMap = HashMap::new(); + let mut position = 0u32; + + for info in walk { + let info = match info { + // A missing object truncates the search rather than failing it, as it truncates the + // graph walk. + Err(_) => break, + Ok(info) => info, + }; + let commit = match git.find_commit(info.id) { + Ok(commit) => commit, + Err(_) => continue, + }; + + // The author filter applies to the numbering walk as well as to the matches, because the + // CLI passes `--author` to its positions run too. A commit it hides is not merely + // unmatched, it has no position at all, and an otherwise-matching commit without a + // position is dropped by the merge. + if let Some(authors) = &options.authors { + if !log::commit_matches_author(&commit, authors)? { + continue; + } + } + + let hash = commit.id().detach().to_string(); + position += 1; + positions.entry(hash).or_insert(position); + let load_count = position; + + let full_message_matches = message_hits.len() < limit + && commit + .message_raw() + .git_ctx("Could not decode the commit message")? + .to_string() + .to_lowercase() + .contains(&needle); + if full_message_matches { + message_hits.push(to_result(&commit, load_count, options.use_author_date)?); + } + + if author_hits.len() < limit { + let author = commit + .author() + .git_ctx("Could not decode the commit author")?; + if author_contains(&author, &needle) { + author_hits.push(to_result(&commit, load_count, options.use_author_date)?); + } + } + } + + // The hash lookup, which the CLI runs without any ref or author constraint and then discards + // if the commit turns out to have no position — so an unreachable or filtered-out commit is + // not a result even when its hash is typed in full. + let mut results: Vec = Vec::new(); + if is_hash_like(query) { + if let Some(found) = git + .rev_parse_single(format!("{query}^{{commit}}").as_str()) + .ok() + .and_then(|id| git.find_commit(id).ok()) + { + let hash = found.id().detach().to_string(); + if let Some(&load_count) = positions.get(&hash) { + results.push(to_result(&found, load_count, options.use_author_date)?); + } + } + } + + // Merge in the CLI's order — hash, then message, then author — keeping the first record of any + // commit, then order the union by position and cut it to the page. + results.extend(message_hits); + results.extend(author_hits); + let mut seen: Vec = Vec::with_capacity(results.len()); + results.retain(|result| { + if seen.contains(&result.hash) { + return false; + } + seen.push(result.hash.clone()); + true + }); + results.sort_by_key(|result| result.load_count); + results.truncate(limit); + Ok(results) +} diff --git a/engine/native/core/src/types.rs b/engine/native/core/src/types.rs index 3ba38d6e..12a9ed72 100644 --- a/engine/native/core/src/types.rs +++ b/engine/native/core/src/types.rs @@ -119,6 +119,44 @@ pub struct RefReadOptions { pub hide_remotes: Vec, } +/* ---------- Commit search ---------- */ + +/// What the Find dialogue asks for. Mirrors the inputs the `git` CLI backend builds its four +/// `git log` runs from, so both backends answer the same question. +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct SearchOptions { + pub query: String, + /// Already normalised by the caller (the CLI clamps to 1..=200, defaulting to 50). + pub max_results: u32, + /// An explicit ref selection from the dropdowns, or `None` to search what the view shows. + pub branches: Option>, + /// The author filter, which narrows the numbering walk as well as the matches. + pub authors: Option>, + #[serde(default = "default_true")] + pub show_tags: bool, + pub show_remote_branches: bool, + pub hide_remotes: Vec, + /// Report the author date rather than the committer date, for `dateType: "Author Date"`. + pub use_author_date: bool, +} + +/// One search hit, in the shape the Find dialogue lists. +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct GitSearchResult { + pub hash: String, + pub parents: Vec, + pub author: String, + pub email: String, + pub date: i64, + /// The subject line only, as `%s` prints it. + pub message: String, + /// The commit's 1-based position in the graph's own walk — how far the view would have to load + /// to reach it, and the key the results are ordered by. + pub load_count: u32, +} + /* ---------- Repository info ---------- */ #[derive(Debug, Clone, Default, Serialize, Deserialize)] diff --git a/engine/native/core/tests/common/mod.rs b/engine/native/core/tests/common/mod.rs index 429aa1c2..ba79649e 100644 --- a/engine/native/core/tests/common/mod.rs +++ b/engine/native/core/tests/common/mod.rs @@ -161,6 +161,30 @@ impl TestRepo { self.head() } + /// Commit under a specific author, for tests that search or filter by one. + pub fn commit_as(&mut self, name: &str, email: &str, message: &str) -> String { + self.git(&["add", "-A"]); + self.clock += 60; + let date = format!("{} +0000", self.clock); + let output = Command::new("git") + .args(["commit", "--quiet", "--allow-empty", "-m", message]) + .current_dir(self.path()) + .env("GIT_CONFIG_NOSYSTEM", "1") + .env("HOME", self.path()) + .env("GIT_AUTHOR_DATE", &date) + .env("GIT_COMMITTER_DATE", &date) + .env("GIT_AUTHOR_NAME", name) + .env("GIT_AUTHOR_EMAIL", email) + .output() + .expect("could not run `git commit`"); + assert!( + output.status.success(), + "`git commit` failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + self.head() + } + /// Write a file and commit it in one step. pub fn commit_file(&mut self, path: &str, contents: &str, message: &str) -> String { self.write(path, contents); diff --git a/engine/native/core/tests/search.rs b/engine/native/core/tests/search.rs new file mode 100644 index 00000000..9eea85a1 --- /dev/null +++ b/engine/native/core/tests/search.rs @@ -0,0 +1,227 @@ +//! The Find dialogue's search, checked against the semantics the `git` CLI backend has. +//! +//! The CLI searches with `--fixed-strings` (literal) plus a separate `--author` pass and a hash +//! lookup. Every test here pins one of those behaviours, because the engine's *other* search +//! (`log::search_history`) is a regex over messages only, and wiring that one would have silently +//! changed what users see. + +#[macro_use] +mod common; + +use git_graph_core::repository::Repo; +use git_graph_core::search::search_commits; +use git_graph_core::types::SearchOptions; + +use common::TestRepo; + +fn open(repo: &TestRepo) -> Repo { + Repo::discover(repo.path()).expect("could not open the fixture repository") +} + +fn options(query: &str) -> SearchOptions { + SearchOptions { + query: query.to_string(), + max_results: 50, + branches: None, + authors: None, + show_tags: true, + show_remote_branches: true, + hide_remotes: Vec::new(), + use_author_date: false, + } +} + +fn subjects(repo: &TestRepo, query: &str) -> Vec { + let engine = open(repo); + search_commits(&engine, &options(query)) + .expect("the search failed") + .into_iter() + .map(|result| result.message) + .collect() +} + +#[test] +fn matches_message_text_literally_rather_than_as_a_regular_expression() { + require_git!(); + let mut repo = TestRepo::new(); + repo.commit_file("a.txt", "1", "release a.c happened"); + repo.commit_file("b.txt", "2", "release abc happened"); + + // `--fixed-strings`: the dot is a dot. A regex search would match both. + assert_eq!(subjects(&repo, "a.c"), vec!["release a.c happened"]); +} + +#[test] +fn a_query_that_is_not_valid_regex_searches_instead_of_failing() { + require_git!(); + let mut repo = TestRepo::new(); + repo.commit_file("a.txt", "1", "fix parsing of (unbalanced"); + repo.commit_file("b.txt", "2", "unrelated"); + + // `(` fails to compile as a regex. The CLI finds the commit; so must this. + assert_eq!(subjects(&repo, "("), vec!["fix parsing of (unbalanced"]); +} + +#[test] +fn matches_regardless_of_case() { + require_git!(); + let mut repo = TestRepo::new(); + repo.commit_file("a.txt", "1", "Fix The Thing"); + + assert_eq!(subjects(&repo, "fix the thing"), vec!["Fix The Thing"]); +} + +#[test] +fn searches_the_whole_message_but_reports_only_the_subject() { + require_git!(); + let repo = TestRepo::new(); + repo.write("a.txt", "1"); + repo.git(&["add", "-A"]); + repo.git(&[ + "commit", + "--quiet", + "-m", + "the subject", + "-m", + "a body mentioning windmills", + ]); + + // `--grep` reads the whole message; `%s` prints the subject. + let found = subjects(&repo, "windmills"); + assert_eq!(found, vec!["the subject"]); +} + +#[test] +fn matches_the_author_as_well_as_the_message() { + require_git!(); + let mut repo = TestRepo::new(); + repo.commit_file("a.txt", "1", "nothing relevant"); + repo.commit_as("Ada Lovelace", "ada@example.invalid", "also nothing"); + + assert_eq!(subjects(&repo, "lovelace"), vec!["also nothing"]); + // The email counts too, because `--author` matches `Name `. + assert_eq!(subjects(&repo, "ada@example"), vec!["also nothing"]); +} + +#[test] +fn resolves_an_abbreviated_hash() { + require_git!(); + let mut repo = TestRepo::new(); + let first = repo.commit_file("a.txt", "1", "the one being looked up"); + repo.commit_file("b.txt", "2", "a later commit"); + + let engine = open(&repo); + let results = search_commits(&engine, &options(&first[..8])).expect("the search failed"); + assert_eq!(results.len(), 1, "the hash prefix did not resolve"); + assert_eq!(results[0].hash, first); +} + +#[test] +fn a_hash_that_resolves_but_is_not_reachable_is_not_a_result() { + require_git!(); + let mut repo = TestRepo::new(); + let reachable = repo.commit_file("a.txt", "1", "on the branch"); + // A commit left on no branch at all: `git log ` still finds it, but the CLI drops it + // because the numbering walk never reaches it. + repo.git(&["checkout", "--quiet", "-b", "scratch"]); + let orphan = repo.commit_file("b.txt", "2", "about to be unreferenced"); + repo.git(&["checkout", "--quiet", "-"]); + repo.git(&["branch", "-D", "scratch"]); + + let engine = open(&repo); + let results = search_commits(&engine, &options(&orphan[..8])).expect("the search failed"); + assert!( + results.is_empty(), + "an unreachable commit was returned as a result" + ); + + let results = search_commits(&engine, &options(&reachable[..8])).expect("the search failed"); + assert_eq!(results.len(), 1); +} + +#[test] +fn orders_results_by_how_far_into_the_graph_they_are() { + require_git!(); + let mut repo = TestRepo::new(); + repo.commit_file("a.txt", "1", "match oldest"); + repo.commit_file("b.txt", "2", "unrelated"); + repo.commit_file("c.txt", "3", "match newest"); + + let engine = open(&repo); + let results = search_commits(&engine, &options("match")).expect("the search failed"); + let ordered: Vec<&str> = results.iter().map(|r| r.message.as_str()).collect(); + // Newest first, because that is the graph's own order, and `loadCount` counts down it. + assert_eq!(ordered, vec!["match newest", "match oldest"]); + assert!( + results[0].load_count < results[1].load_count, + "loadCount did not increase with depth: {:?}", + results.iter().map(|r| r.load_count).collect::>() + ); + // Position is counted over every commit walked, not over the matches. + assert_eq!(results[0].load_count, 1); + assert_eq!(results[1].load_count, 3); +} + +#[test] +fn the_author_filter_hides_commits_from_the_results_and_the_numbering() { + require_git!(); + let mut repo = TestRepo::new(); + repo.commit_as("Ada", "ada@example.invalid", "match from ada"); + repo.commit_as("Bob", "bob@example.invalid", "match from bob"); + + let engine = open(&repo); + let mut opts = options("match"); + opts.authors = Some(vec!["Ada".to_string()]); + let results = search_commits(&engine, &opts).expect("the search failed"); + + assert_eq!( + results + .iter() + .map(|r| r.message.as_str()) + .collect::>(), + vec!["match from ada"] + ); + // Bob's commit is not merely unmatched, it is not counted: Ada's is position 1, not 2. + assert_eq!(results[0].load_count, 1); +} + +#[test] +fn the_page_size_caps_the_results() { + require_git!(); + let mut repo = TestRepo::new(); + for i in 0..10 { + repo.commit_file("a.txt", &i.to_string(), &format!("match {i}")); + } + + let engine = open(&repo); + let mut opts = options("match"); + opts.max_results = 3; + let results = search_commits(&engine, &opts).expect("the search failed"); + assert_eq!(results.len(), 3); + // The three nearest the top of the graph, not an arbitrary three. + assert_eq!( + results.iter().map(|r| r.load_count).collect::>(), + vec![1, 2, 3] + ); +} + +#[test] +fn an_empty_query_finds_nothing_rather_than_everything() { + require_git!(); + let mut repo = TestRepo::new(); + repo.commit_file("a.txt", "1", "something"); + + assert!(subjects(&repo, "").is_empty()); + assert!(subjects(&repo, " ").is_empty()); +} + +#[test] +fn a_commit_matching_both_message_and_author_appears_once() { + require_git!(); + let mut repo = TestRepo::new(); + repo.commit_as("Ada", "ada@example.invalid", "a commit by ada about ada"); + + let engine = open(&repo); + let results = search_commits(&engine, &options("ada")).expect("the search failed"); + assert_eq!(results.len(), 1, "the commit was returned twice"); +} diff --git a/engine/native/node/src/lib.rs b/engine/native/node/src/lib.rs index a0349d71..6e53c7e5 100644 --- a/engine/native/node/src/lib.rs +++ b/engine/native/node/src/lib.rs @@ -12,9 +12,9 @@ use napi::bindgen_prelude::*; use napi_derive::napi; -use git_graph_core::types::{LogOptions, RefReadOptions}; +use git_graph_core::types::{LogOptions, RefReadOptions, SearchOptions}; use git_graph_core::{ - blob, config, details, diff, graph, log, refs, stash, stats, status, Error, ErrorKind, + blob, config, details, diff, graph, log, refs, search, stash, stats, status, Error, ErrorKind, RepoManager, }; @@ -66,6 +66,21 @@ pub async fn load_repo_info(path: String, options_json: String) -> Result Result { + run(move || { + let options: SearchOptions = decode(&options_json)?; + let repo = RepoManager::global().get(&path)?; + let results = search::search_commits(&repo, &options)?; + encode(&results) + }) + .await +} + /// A page of the graph. `options_json` is a serialised `LogOptions`. #[napi] pub async fn load_commits(path: String, options_json: String) -> Result { diff --git a/src/backend/engine/addon.ts b/src/backend/engine/addon.ts index 31cbdc4d..91f01d1f 100644 --- a/src/backend/engine/addon.ts +++ b/src/backend/engine/addon.ts @@ -43,6 +43,13 @@ export type EngineAddon = { * this name). Options are the JSON built by `buildLoadCommitsOptions`. */ loadCommits(repoPath: string, optionsJson: string): Promise; + /** + * `search_commits` encoded as JSON: the Find dialogue's hits, each carrying + * its position in the graph walk. Options are the JSON built by + * `buildSearchOptions`. Distinct from the engine's own `search_history`, + * which is a regex over messages and is deliberately not used. + */ + searchCommits(repoPath: string, optionsJson: string): Promise; /** * `load_commit_details` encoded as JSON: the commit's fields with its file * statuses, counts left null for `load_line_counts` to settle. @@ -262,6 +269,7 @@ function isEngineAddon(loaded: unknown): loaded is EngineAddon { authors?: unknown; loadConfig?: unknown; currentBranchName?: unknown; + searchCommits?: unknown; loadRefs?: unknown; closeRepository?: unknown; closeAllRepositories?: unknown; @@ -282,6 +290,7 @@ function isEngineAddon(loaded: unknown): loaded is EngineAddon { typeof candidate.authors === "function" && typeof candidate.loadConfig === "function" && typeof candidate.currentBranchName === "function" && + typeof candidate.searchCommits === "function" && typeof candidate.loadRefs === "function" && typeof candidate.closeRepository === "function" && typeof candidate.closeAllRepositories === "function" && diff --git a/src/backend/engine/index.ts b/src/backend/engine/index.ts index 58527784..9f38a04b 100644 --- a/src/backend/engine/index.ts +++ b/src/backend/engine/index.ts @@ -20,6 +20,7 @@ import type { SimpleGit } from "simple-git"; import { commitDetails } from "@/backend/queries/commitDetails"; import { commitComparison } from "@/backend/queries/commitComparison"; import { loadCommits } from "@/backend/queries/loadCommits"; +import { normalizeMaxResults, searchCommits } from "@/backend/queries/searchCommits"; import { emptyRepoInfo, loadRepoInfo } from "@/backend/queries/loadRepoInfo"; import type { DateType, GitCommitDetails, GitFileChange, QueryResult } from "@/backend/types"; import { getRemoteUrl } from "@/backend/utils/git"; @@ -29,6 +30,13 @@ import { toGitQueryError } from "@/backend/utils/queryError"; import type { EngineBackend } from "@/types"; import { type EngineAddon, loadEngineAddon } from "./addon"; +import { + buildSearchOptions, + type EngineSearchInput, + mapEngineSearchResults, + parseEngineSearchResults, + shouldServeSearchFromEngine +} from "./search"; import { buildLoadCommitsOptions, engineLoadCommitsRefs, @@ -72,6 +80,8 @@ export type RepoReader = { loadCommitDetails(args: CommitDetailsArgs): Promise>; /** One arbitrary revision pair, counts settled eagerly like the CLI. */ loadCommitComparison(args: CommitComparisonArgs): Promise>; + /** The Find dialogue's hits, ordered by their position in the graph walk. */ + searchCommits(args: SearchArgs): Promise>; }; export type RepoInfoArgs = { @@ -96,6 +106,12 @@ export type CommitDetailsArgs = { recordGitCommand?: GitCommandRecorder; }; +export type SearchArgs = EngineSearchInput & { + repoPath: string; + git: SimpleGit; + recordGitCommand?: GitCommandRecorder; +}; + export type CommitComparisonArgs = { repoPath: string; git: SimpleGit; @@ -192,7 +208,8 @@ export function createRepoReader(deps: RepoReaderDeps): RepoReader { loadCommitDetails: (args: CommitDetailsArgs) => readCommitDetails(deps.preference, provider, args), loadCommitComparison: (args: CommitComparisonArgs) => - readCommitComparison(deps.preference, provider, args) + readCommitComparison(deps.preference, provider, args), + searchCommits: (args: SearchArgs) => readSearch(deps.preference, provider, args) }; } @@ -555,3 +572,54 @@ async function readCommitComparison( return cliRead(); } } + +/** + * The Find dialogue's search. + * + * A decline, a failed engine call, or a payload this version does not + * recognise all land on the CLI with the same arguments, so the dialogue's + * behaviour is identical either way. + */ +async function readSearch( + preference: EngineBackend, + provider: AddonProvider, + args: SearchArgs +): Promise> { + const cliRead = (): Promise> => + searchCommits(args.git, { + query: args.query, + maxResults: args.maxResults, + showRemoteBranches: args.showRemoteBranches, + hiddenRemotes: args.hiddenRemotes, + showTags: args.showTags, + branches: args.branches, + authors: args.authors, + tags: args.tags, + dateType: args.dateType, + repo: args.repoPath, + recordGitCommand: args.recordGitCommand + }); + // The total no-op path: the addon is not even loaded. + if (preference === "git-cli") return cliRead(); + if (!shouldServeSearchFromEngine(args)) return cliRead(); + const addon = provider(); + if (addon === null) return cliRead(); + // The CLI clamps before building `--max-count`; the engine is given the same + // clamped number so both page identically. + const maxResults = normalizeMaxResults(args.maxResults); + // An empty query is not a search on either backend. + if (args.query.trim() === "") return { results: [], error: null }; + try { + const parsed = parseEngineSearchResults( + await addon.searchCommits(args.repoPath, buildSearchOptions(args, maxResults)) + ); + if (parsed === null) return cliRead(); + engineServedRead = true; + return { results: mapEngineSearchResults(parsed), error: null }; + } catch (error: unknown) { + if (!isEngineFallbackError(error)) { + return { results: [], error: toGitQueryError(error, "Unable to search commits") }; + } + return cliRead(); + } +} diff --git a/src/backend/engine/search.ts b/src/backend/engine/search.ts new file mode 100644 index 00000000..72206f7c --- /dev/null +++ b/src/backend/engine/search.ts @@ -0,0 +1,134 @@ +/** + * The Find dialogue's search, through the engine. + * + * The CLI answers this with four `git log` runs at once — a literal + * `--fixed-strings --grep`, an `--author`, a hash lookup, and one unbounded + * walk that numbers every commit — then merges them by that numbering. The + * engine does the same three matches in a single walk. + * + * ### Why this is not the engine's own `search_history` + * + * The engine ships a search already, and wiring *that* one would have changed + * what users see: it matches a **regular expression** against messages only, + * across every ref, ignoring the author filter, and numbers nothing. Slice + * 16.7 declined it for exactly that reason. `search_commits` was added to the + * engine instead, reproducing this project's semantics; the regex one is left + * where it is, unused. + * + * ### Declines + * + * `--glob=` patterns (`customBranchGlobPatterns`) are not understood by the + * engine's tip resolution, the same decline `loadCommits` makes. + */ + +import type { DateType, GitCommitSearchResult } from "@/backend/types"; +import { selectedLogRefs, uniqueNonEmpty } from "@/backend/utils/logFilters"; +import { normalizeHiddenRemotes } from "@/backend/utils/remoteRefs"; + +/** The route fields the engine decision, options and mapping need. */ +export type EngineSearchInput = { + query: string; + maxResults: number; + showRemoteBranches: boolean; + hiddenRemotes?: string[]; + showTags?: boolean; + branches?: string[] | null; + authors?: string[] | null; + tags?: string[] | null; + dateType: DateType; +}; + +/** + * The ref selection both backends search: null is "what the view is showing", + * an array is an explicit choice from the dropdowns. Single source of truth — + * the CLI builds its `refArgs` from exactly this, so the decline below sees + * the same selection. + */ +export function engineSearchRefs(input: EngineSearchInput): string[] | null { + return selectedLogRefs({ branches: input.branches, tags: input.tags }); +} + +/** + * Whether the engine may serve this search. Every false is a *decline*, not a + * bug: the caller routes those searches straight to the CLI, unchanged. + */ +export function shouldServeSearchFromEngine(input: EngineSearchInput): boolean { + // `--glob=` is not understood by the engine's tip resolution. + return !engineSearchRefs(input)?.some((ref) => ref.startsWith("--glob=")); +} + +/** + * The `search_commits` options JSON. `maxResults` arrives already clamped by + * the caller, because the CLI clamps it before building its `--max-count` and + * both backends must page identically. + */ +export function buildSearchOptions(input: EngineSearchInput, maxResults: number): string { + return JSON.stringify({ + query: input.query, + maxResults, + branches: engineSearchRefs(input), + authors: uniqueNonEmpty(input.authors), + showTags: input.showTags !== false, + showRemoteBranches: input.showRemoteBranches, + hideRemotes: normalizeHiddenRemotes(input.hiddenRemotes), + useAuthorDate: input.dateType === "Author Date" + }); +} + +/** One hit as the engine encodes it. */ +type EngineSearchResult = { + hash: string; + parents: string[]; + author: string; + email: string; + date: number; + message: string; + loadCount: number; +}; + +function isEngineSearchResult(value: unknown): value is EngineSearchResult { + if (typeof value !== "object" || value === null) return false; + const candidate = value as Record; + return ( + typeof candidate.hash === "string" && + Array.isArray(candidate.parents) && + candidate.parents.every((parent): parent is string => typeof parent === "string") && + typeof candidate.author === "string" && + typeof candidate.email === "string" && + typeof candidate.date === "number" && + typeof candidate.message === "string" && + typeof candidate.loadCount === "number" + ); +} + +/** + * Decode the payload, or null when it is not the shape this version expects — + * a skewed addon declines into the CLI rather than throwing into the view. + */ +export function parseEngineSearchResults(payload: string): EngineSearchResult[] | null { + let decoded: unknown; + try { + decoded = JSON.parse(payload); + } catch { + return null; + } + if (!Array.isArray(decoded) || !decoded.every(isEngineSearchResult)) return null; + return decoded; +} + +/** + * Into this project's own type. The engine calls the field `parents` and this + * project calls it `parentHashes`; mapping here is what keeps the seam + * independent of the engine's wire shape. + */ +export function mapEngineSearchResults(results: EngineSearchResult[]): GitCommitSearchResult[] { + return results.map((result) => ({ + hash: result.hash, + parentHashes: result.parents, + author: result.author, + email: result.email, + date: result.date, + message: result.message, + loadCount: result.loadCount + })); +} diff --git a/src/backend/queries/searchCommits.ts b/src/backend/queries/searchCommits.ts index b910feeb..048175d6 100644 --- a/src/backend/queries/searchCommits.ts +++ b/src/backend/queries/searchCommits.ts @@ -33,7 +33,12 @@ type GitQueryContext = { record?: GitCommandRecorder; }; -function normalizeMaxResults(maxResults: number): number { +/** + * The page size both backends use. Exported so the engine seam clamps with + * this function rather than a copy of it: a divergence here would page the + * two backends differently for the same request. + */ +export function normalizeMaxResults(maxResults: number): number { if (!Number.isFinite(maxResults) || maxResults < 1) return defaultMaxResults; return Math.min(Math.floor(maxResults), maxResultsLimit); } @@ -76,10 +81,6 @@ function parseLogEntries(stdout: string): GitLogEntry[] { return commits; } -function escapeRegExp(value: string): string { - return value.replace(/[\\^$.*+?()[\]{}|]/g, String.raw`\$&`); -} - async function runSearchLog( git: SimpleGit, label: string, @@ -210,7 +211,13 @@ export async function searchCommits( runSearchLog( git, "searchCommits.author", - ["--regexp-ignore-case", `--author=${escapeRegExp(query)}`], + // `--fixed-strings` rather than a hand-escaped pattern. `--author` + // takes a *basic* regular expression, in which `\(` opens a group + // instead of escaping a parenthesis — so escaping the query inverted + // the meaning and made git reject any search containing an unbalanced + // `(` or `[` outright, failing the whole dialogue. A literal match was + // always the intent; this is how git spells it. + ["--regexp-ignore-case", "--fixed-strings", `--author=${query}`], input, context ), diff --git a/src/extension/messageHandler.ts b/src/extension/messageHandler.ts index fb612f72..dedbb1b6 100644 --- a/src/extension/messageHandler.ts +++ b/src/extension/messageHandler.ts @@ -49,7 +49,6 @@ import { deleteUserDetails, editUserDetails } from "@/backend/actions/userConfig import { createRepoReader, didEngineServeRead } from "@/backend/engine/index"; import type { GitClient } from "@/backend/gitClient"; import { loadBranches } from "@/backend/queries/loadBranches"; -import { searchCommits } from "@/backend/queries/searchCommits"; import { tagDetails } from "@/backend/queries/tagDetails"; import { uncommittedDetails } from "@/backend/queries/uncommittedDetails"; import type { GitFileChangeType } from "@/backend/types"; @@ -642,10 +641,19 @@ export function registerMessageHandlers( }); bridge.onMessage("searchCommits", async (msg) => { + // The preference is read live on every search, so flipping + // git-graph-libre.backend needs no reload. The reader serves from the + // engine where it can and the CLI elsewhere, with an identical shape. + const reader = createRepoReader({ + preference: config.backend(), + gitPath: config.gitPath() + }); bridge.post({ command: "searchCommits", requestId: msg.requestId, - ...(await searchCommits(gitClient.getInstance(), { + ...(await reader.searchCommits({ + repoPath: msg.repo, + git: gitClient.getInstance(), query: msg.query, maxResults: msg.maxResults, showRemoteBranches: msg.showRemoteBranches, @@ -655,7 +663,6 @@ export function registerMessageHandlers( authors: msg.authors, tags: msg.tags, dateType: config.dateType(), - repo: msg.repo, recordGitCommand })) }); diff --git a/tests/backend/engine/addon.test.ts b/tests/backend/engine/addon.test.ts index 174ebcff..58e78240 100644 --- a/tests/backend/engine/addon.test.ts +++ b/tests/backend/engine/addon.test.ts @@ -92,6 +92,7 @@ describe("engine addon loader", () => { authors: async () => "[]", loadConfig: async () => "{}", currentBranchName: async () => null, + searchCommits: async () => "[]", loadRefs: async () => "{}", closeRepository: () => {}, closeAllRepositories: () => {}, diff --git a/tests/backend/engine/backends.bench.ts b/tests/backend/engine/backends.bench.ts index caddcd1c..cffb96e9 100644 --- a/tests/backend/engine/backends.bench.ts +++ b/tests/backend/engine/backends.bench.ts @@ -168,6 +168,22 @@ const CASES: Case[] = [ dateType: "Commit Date" }) }, + { + name: "search (message term)", + run: (reader) => + reader.searchCommits({ + repoPath, + git, + query: "fix", + maxResults: 50, + showRemoteBranches: true, + showTags: true, + branches: null, + authors: null, + tags: null, + dateType: "Commit Date" as const + }) + }, { name: "remote url", run: (reader) => reader.getRemoteUrl(repoPath) } ]; diff --git a/tests/backend/engine/parity.test.ts b/tests/backend/engine/parity.test.ts index 1806cb58..b57e5591 100644 --- a/tests/backend/engine/parity.test.ts +++ b/tests/backend/engine/parity.test.ts @@ -18,6 +18,7 @@ import { commitComparison } from "@/backend/queries/commitComparison"; import { commitDetails } from "@/backend/queries/commitDetails"; import { loadCommits } from "@/backend/queries/loadCommits"; import { loadRepoInfo } from "@/backend/queries/loadRepoInfo"; +import { searchCommits as searchCommitsQuery } from "@/backend/queries/searchCommits"; import type { GitCommitNode } from "@/backend/types"; import { getRemoteUrl } from "@/backend/utils/git"; @@ -960,3 +961,160 @@ describe("engine/CLI parity: repoInfo config", () => { await expectRepoInfoParity(dir, "absent global", false); }, 120000); }); + +describe("engine/CLI parity: searchCommits", () => { + // The Find dialogue is the one read where a wrong backend is *silently* + // wrong: a regex engine and a fixed-string CLI both return results, just + // different ones. Every case here is a query shape that would diverge if + // the engine's own `search_history` had been wired instead. + const dirs: string[] = []; + let dir: string; + + beforeAll(() => { + dir = makeRepo(); + dirs.push(dir); + // Message shapes: a literal dot, regex metacharacters, mixed case, and a + // body-only term that `--grep` reaches but `%s` does not show. + fs.writeFileSync(path.join(dir, "a.txt"), "1"); + git(["add", "-A"], dir); + git(["commit", "-m", "release a.c shipped"], dir); + fs.writeFileSync(path.join(dir, "b.txt"), "2"); + git(["add", "-A"], dir); + git(["commit", "-m", "release abc shipped"], dir); + fs.writeFileSync(path.join(dir, "c.txt"), "3"); + git(["add", "-A"], dir); + git(["commit", "-m", "Fix The Thing", "-m", "body mentioning windmills"], dir); + fs.writeFileSync(path.join(dir, "d.txt"), "4"); + git(["add", "-A"], dir); + git( + ["-c", "user.name=Ada Lovelace", "-c", "user.email=ada@example.invalid", + "commit", "-m", "an unrelated subject"], + dir + ); + fs.writeFileSync(path.join(dir, "e.txt"), "5"); + git(["add", "-A"], dir); + git(["commit", "-m", "parsing of (unbalanced"], dir); + }, 180000); + + afterAll(() => { + for (const candidate of dirs) fs.rmSync(candidate, { recursive: true, force: true }); + }); + + function requireAddon(context: { skip: (message?: string) => never }) { + if (loadEngineAddon() === null) { + context.skip("Engine addon not built — run pnpm run engine:build for the engine half."); + } + } + + async function expectSearchParity( + query: string, + label: string, + expectedServed: boolean + ): Promise { + const args = { + repoPath: dir, + git: simpleGit(dir), + query, + maxResults: 50, + showRemoteBranches: true, + showTags: true, + branches: null, + authors: null, + tags: null, + dateType: "Commit Date" as const + }; + resetEngineServedRead(); + const viaAuto = await createRepoReader({ preference: "auto", gitPath: "git" }).searchCommits( + args + ); + const served = didEngineServeRead(); + const viaCli = await createRepoReader({ preference: "git-cli", gitPath: "git" }).searchCommits( + args + ); + const direct = await searchCommitsQuery(simpleGit(dir), { + query, + maxResults: 50, + showRemoteBranches: true, + showTags: true, + branches: null, + authors: null, + tags: null, + dateType: "Commit Date", + repo: dir + }); + expect(served, `${label} served`).toBe(expectedServed); + expect(viaAuto, `${label} auto`).toEqual(direct); + expect(viaCli, `${label} git-cli`).toEqual(direct); + } + + it("matches a literal dot rather than any character", async (context) => { + requireAddon(context); + await expectSearchParity("a.c", "literal dot", true); + }, 120000); + + it("searches a query that is not valid regex", async (context) => { + requireAddon(context); + await expectSearchParity("(", "invalid regex", true); + }, 120000); + + it("matches regardless of case", async (context) => { + requireAddon(context); + await expectSearchParity("fix the thing", "case", true); + }, 120000); + + it("reaches the body but reports the subject", async (context) => { + requireAddon(context); + await expectSearchParity("windmills", "body", true); + }, 120000); + + it("matches the author as well as the message", async (context) => { + requireAddon(context); + await expectSearchParity("lovelace", "author", true); + }, 120000); + + it("finds nothing for a term that is absent", async (context) => { + requireAddon(context); + await expectSearchParity("nothingmatchesthis", "absent", true); + }, 120000); + + it("resolves an abbreviated hash", async (context) => { + requireAddon(context); + const head = cp + .execFileSync("git", ["rev-parse", "HEAD"], { cwd: dir, encoding: "utf8" }) + .trim(); + await expectSearchParity(head.slice(0, 8), "hash prefix", true); + }, 120000); + + it("declines a glob ref pattern to the CLI", async (context) => { + requireAddon(context); + const args = { + repoPath: dir, + git: simpleGit(dir), + query: "release", + maxResults: 50, + showRemoteBranches: true, + showTags: true, + branches: ["--glob=refs/heads/*"], + authors: null, + tags: null, + dateType: "Commit Date" as const + }; + resetEngineServedRead(); + const viaAuto = await createRepoReader({ preference: "auto", gitPath: "git" }).searchCommits( + args + ); + expect(didEngineServeRead(), "glob served").toBe(false); + const direct = await searchCommitsQuery(simpleGit(dir), { + query: "release", + maxResults: 50, + showRemoteBranches: true, + showTags: true, + branches: ["--glob=refs/heads/*"], + authors: null, + tags: null, + dateType: "Commit Date", + repo: dir + }); + expect(viaAuto, "glob result").toEqual(direct); + }, 120000); +}); diff --git a/tests/backend/engine/reader.test.ts b/tests/backend/engine/reader.test.ts index d88c1b34..54c6a85f 100644 --- a/tests/backend/engine/reader.test.ts +++ b/tests/backend/engine/reader.test.ts @@ -146,6 +146,7 @@ function fakeAddon(implementation: (repoPath: string) => Promise) authors: async () => "[]", loadConfig: async () => JSON.stringify({ remotes: [] }), currentBranchName: async () => null, + searchCommits: async () => "[]", loadRefs: async () => JSON.stringify({ head: null }), configList: async () => { throw new Error("Unsupported: config not stubbed"); @@ -413,6 +414,7 @@ describe("createRepoReader repoInfo", () => { remotes: [{ name: "origin", url: "https://github.com/some/repo.git", pushUrl: null }] }), currentBranchName: async () => null, + searchCommits: async () => "[]", loadRefs: async () => JSON.stringify({ head: null }), configList: async (_repo: string, local: boolean) => JSON.stringify(local ? { "user.name": "T", "user.email": "t@t.com" } : {}), @@ -465,6 +467,7 @@ describe("createRepoReader repoInfo", () => { authors: async () => "[]", loadConfig: async () => JSON.stringify({ remotes: [] }), currentBranchName: async () => null, + searchCommits: async () => "[]", loadRefs: async () => JSON.stringify({ head: null }), configList: async () => { throw new Error("Unsupported: config not stubbed"); @@ -519,6 +522,7 @@ describe("createRepoReader repoInfo", () => { authors: async () => "[]", loadConfig: async () => JSON.stringify({ remotes: [] }), currentBranchName: async () => null, + searchCommits: async () => "[]", loadRefs: async () => JSON.stringify({ head: null }), configList: async () => { throw new Error("Unsupported: config not stubbed"); @@ -572,6 +576,7 @@ describe("createRepoReader repoInfo", () => { authors: async () => "[]", loadConfig: async () => JSON.stringify({ remotes: [] }), currentBranchName: async () => null, + searchCommits: async () => "[]", loadRefs: async () => JSON.stringify({ head: null }), configList: async () => { throw new Error("Unsupported: config not stubbed"); @@ -626,6 +631,7 @@ describe("createRepoReader repoInfo", () => { authors: async () => "[]", loadConfig: async () => JSON.stringify({ remotes: [] }), currentBranchName: async () => null, + searchCommits: async () => "[]", loadRefs: async () => JSON.stringify({ head: null }), configList: async () => { throw new Error("Unsupported: config not stubbed"); @@ -784,6 +790,7 @@ describe("createRepoReader loadCommits", () => { authors: unsupported, loadConfig: unsupported, currentBranchName: unsupported, + searchCommits: unsupported, loadRefs: unsupported, configList: unsupported, closeRepository: unsupported, @@ -911,6 +918,7 @@ describe("createRepoReader loadCommits", () => { authors: async () => "[]", loadConfig: async () => JSON.stringify({ remotes: [] }), currentBranchName: async () => null, + searchCommits: async () => "[]", loadRefs: async () => JSON.stringify({ head: null }), configList: async () => { throw new Error("Unsupported: config not stubbed"); @@ -1041,6 +1049,7 @@ describe("createRepoReader loadCommits", () => { authors: async () => "[]", loadConfig: async () => JSON.stringify({ remotes: [] }), currentBranchName: async () => null, + searchCommits: async () => "[]", loadRefs: async () => JSON.stringify({ head: null }), configList: async () => { throw new Error("Unsupported: config not stubbed"); @@ -1149,6 +1158,7 @@ describe("createRepoReader loadCommitDetails", () => { authors: unsupported, loadConfig: unsupported, currentBranchName: unsupported, + searchCommits: unsupported, loadRefs: unsupported, configList: unsupported, closeRepository: unsupported, @@ -1425,6 +1435,7 @@ describe("createRepoReader loadCommitComparison", () => { authors: unsupported, loadConfig: unsupported, currentBranchName: unsupported, + searchCommits: unsupported, loadRefs: unsupported, configList: unsupported, closeRepository: unsupported, From cf41ce4d46ab6f48fbf5068a43e442128142dd73 Mon Sep 17 00:00:00 2001 From: Zamkorus Date: Wed, 23 Sep 2026 20:27:30 +0200 Subject: [PATCH 08/10] refactor(engine): remove search_history and the write-flow reads, dropping regex Five exports whose consumers a permanent non-goal blocks forever, not merely functions nothing calls today. `search_history` was superseded by `search_commits` in the previous commit. `current_branch_name`, `current_branch_upstream`, `remote_names` and `load_commit_subject` are all read inside `kind: "action"` write flows, and "every write stays on `runGitRaw`" is the first entry in the permanent non-goals - so none of them had a reachable future. This is worth doing rather than leaving alone because a `#[napi]` function is an exported symbol, so it is a linker root and LTO cannot strip it: a dead export is genuinely carried in every shipped binary. `search_history` was also the only consumer of the `regex` crate, which goes with it. before 6,183,072 bytes after 4,814,416 bytes saving 1,368,656 (22.1%) per platform, ~10.4 MB across all eight Their `api::Engine` methods went too, along with the now-orphaned `GitHistoryMatch`, `SEARCH_LIMIT` and `collapse_whitespace`, and their tests. `Repo::remote_names` is a different function and stays - `graph.rs` needs it for `load_commits`. The bare-repository test keeps its object-read coverage and is renamed for what it now proves; the `Engine` smoke test reads the checked-out branch from `info.head`. TypeScript loses `currentBranchName`, which was declared on `EngineAddon` *and* in the `isEngineAddon` load-time guard - it could have rejected a good engine binary over a method nothing called. The 13 exports still unwired are kept and inventoried in the knowledge base with what each would serve, including `author_stats` and `activity_heatmap`, which the maintainer intends to wire for a Statistics tab. That slice has no CLI counterpart, so it is a deliberate exception to the two-backends-agree rule and is recorded as one. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 2 + docs/AI_DEV_KNOWLEDGE_BASE.md | 85 +++++++++++- engine/Cargo.lock | 33 ----- engine/Cargo.toml | 1 - engine/native/core/Cargo.toml | 1 - engine/native/core/src/api.rs | 24 +--- engine/native/core/src/config.rs | 53 -------- engine/native/core/src/details.rs | 18 --- engine/native/core/src/log.rs | 76 +---------- engine/native/core/src/search.rs | 6 +- engine/native/core/src/stats.rs | 2 +- engine/native/core/src/types.rs | 11 -- engine/native/core/tests/api.rs | 8 +- engine/native/core/tests/queries.rs | 197 +--------------------------- engine/native/node/src/lib.rs | 55 -------- src/backend/engine/addon.ts | 8 +- src/backend/engine/search.ts | 12 +- tests/backend/engine/addon.test.ts | 1 - tests/backend/engine/reader.test.ts | 11 -- 19 files changed, 100 insertions(+), 504 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 79b695ea..e778cd8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 16 ms, reading the branch, tag, remote and stash lists from 60 ms to 8 ms, and loading a page of commits from 12 ms to 8 ms. Loading commits no longer starts a `git` process at all. +- **The engine is about 1.4 MB smaller per platform**, after removing code + that could never be reached — around 10 MB off the download. - **A built-in Git engine, so the graph stops waiting on `git` processes.** Reading a repository — opening the graph, loading a page of commits, opening commit details, comparing two commits, reading a file at a revision — now diff --git a/docs/AI_DEV_KNOWLEDGE_BASE.md b/docs/AI_DEV_KNOWLEDGE_BASE.md index 1379ed25..082e67b5 100644 --- a/docs/AI_DEV_KNOWLEDGE_BASE.md +++ b/docs/AI_DEV_KNOWLEDGE_BASE.md @@ -2528,10 +2528,11 @@ Implementation record (`2026-09-23`, four subslice commits, all signed): - Declines, each with its reason — no code, no behavior change: `search_history` (the engine matches message regex while the CLI searches fixed strings plus author, hash, positions and ref filters) - — **superseded on `2026-09-23`**: rather than wire the mismatched - function, the engine gained a `search_commits` that reproduces this - project's semantics, and the search now routes through it. See "Search - in the engine" below. `search_history` itself stays unused; + — **superseded and then removed on `2026-09-23`**: rather than wire the + mismatched function, the engine gained a `search_commits` that + reproduces this project's semantics, and the search now routes through + it. `search_history` itself was deleted. See "Search in the engine" and + "The engine surface: what is wired, what was removed, what is kept"; `load_tag_details` (the engine reports signatures present but unverified; verification is a permanent non-goal); `current_branch_*` and `remote_names` (their consumers are `kind: "action"` write flows, @@ -2900,8 +2901,8 @@ project's search answer different questions. That decline was correct and the conclusion drawn from it was not: the fix is not to wire the mismatched function, and not to leave search on four `git` processes, but to give the engine a search with *these* semantics. `search_commits` is that function, new -in `engine/native/core/src/search.rs`, and `search_history` stays where it is, -unused. +in `engine/native/core/src/search.rs`. `search_history` was then removed +outright — see the surface audit below. What had to be reproduced, and what the engine's own search does instead: @@ -2956,6 +2957,78 @@ catch bugs in whichever one is wrong. Second only to `loadRepoInfo` among the wired reads, because four processes collapse to one walk. +#### The engine surface: what is wired, what was removed, what is kept (`2026-09-23`) + +An audit of every `#[napi]` export against the TypeScript that calls it. **31 +exports, 18 wired, 13 unwired.** The unwired ones are not an oversight — each +is listed below with why it is there — but they are not free either: a +`#[napi]` function is an exported symbol, so it is a linker *root* and LTO +cannot strip it. Dead exports are genuinely in every shipped binary. + +**Removed (`2026-09-23`).** Five exports, chosen because a recorded permanent +non-goal blocks them forever, not because nothing calls them today: + +| removed | why it could never be wired | +| --- | --- | +| `search_history` | superseded by `search_commits`; its regex semantics are wrong here | +| `current_branch_name` | consumer is a write flow | +| `current_branch_upstream` | consumer is a write flow | +| `remote_names` | consumer is a write flow | +| `load_commit_subject` | only consumer is the amend action | + +"Every write stays on `runGitRaw`" is the first permanent non-goal, so the +last four had no reachable future. Their `api::Engine` methods, tests and the +orphaned `GitHistoryMatch`, `SEARCH_LIMIT` and `collapse_whitespace` went with +them. `Repo::remote_names` is a *different* function and stays — `graph.rs` +needs it for `load_commits`. + +The TypeScript side lost `currentBranchName` too. It was declared on +`EngineAddon` **and in the `isEngineAddon` load-time guard**, so it could have +rejected a perfectly good engine binary over a method nothing called. + +**What it saved.** `search_history` was the only consumer of the `regex` +crate, so the dependency went as well: + +| | bytes | +| --- | ---: | +| before | `6,183,072` | +| after | `4,814,416` | +| saving | **`1,368,656` (22.1%) per platform, ~10.4 MB across all eight** | + +**Kept deliberately — available for later wiring.** Nothing below is dead by +intent; each is a function whose consumer does not exist *yet*: + +| kept | what it would serve | +| --- | --- | +| `activity_heatmap`, `author_stats` | **a Statistics tab** — see below | +| `load_uncommitted_details` | the `*` row, if it moves off the CLI (16.6) | +| `count_uncommitted_changes` | that row's count — redundant while `load_commits` builds it | +| `load_commit_file_diff` | a unified-diff renderer, replacing VS Code's diff editor | +| `new_path_of_renamed_file` | rename tracking between a commit and the working tree | +| `load_commit_bodies`, `load_commit_summaries` | batch message reads, if a view ever wants them | +| `count_commits_before` | anything needing `rev-list` counts | +| `load_tag_details` | blocked on signature verification, a permanent non-goal | +| `repo_root`, `submodules` | blocked on discovery semantics diverging from the CLI's | +| `open_repository` | explicit handle opening; handles currently open implicitly | + +The last three rows are blocked rather than merely unwired, and could be +removed on the same reasoning as the five above. They are kept because the +maintainer may want the option, and because the saving is already taken. + +**Planned: a Statistics tab.** The maintainer intends to add one +(`2026-09-23`). `stats.rs` already provides both halves — `author_stats` +(commit counts per author across every ref, merges excluded) and +`activity_heatmap` (author-local weekday/hour cells, sparse, merges excluded). +Both are inherited from `vscode-git-graph-rs`, where they backed that +project's own Statistics view, and neither has ever been wired here. Wiring +them is a feature slice, not a parity slice: there is no CLI implementation to +agree with, so the usual "two implementations, proven to agree" rule does not +apply and the engine would be the *only* backend. That is a deliberate +exception to design rule 2 and must be recorded as one when it happens — +including what the view does when the engine is unavailable, which for every +other read is "fall back to the CLI" and here would have to be "the tab is not +offered". + #### What the page-size defaults are actually worth (`2026-09-23`) Asked why `initialLoadCommits` is `300` when 1,000 costs only milliseconds diff --git a/engine/Cargo.lock b/engine/Cargo.lock index 02741fbb..aba769d3 100644 --- a/engine/Cargo.lock +++ b/engine/Cargo.lock @@ -2,15 +2,6 @@ # It is not intended for manual editing. version = 3 -[[package]] -name = "aho-corasick" -version = "1.1.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c982642fa9e8606056828ee9a8505737230110bb1099153c79efe865c59d12ba" -dependencies = [ - "memchr", -] - [[package]] name = "allocator-api2" version = "0.2.21" @@ -401,7 +392,6 @@ version = "1.0.24" dependencies = [ "bstr", "gix", - "regex", "serde", "serde_json", "tempfile", @@ -1574,34 +1564,11 @@ dependencies = [ "bitflags 2.13.1", ] -[[package]] -name = "regex" -version = "1.13.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f020237b6c8eed93db2e2cb53c00c60a8e1bc73da7d073199a1180401450218d" -dependencies = [ - "aho-corasick", - "memchr", - "regex-automata", - "regex-syntax", -] - [[package]] name = "regex-automata" version = "0.4.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ad8553b9b26413251cbf30e620595c7a41b3887f03da04579c0e6b0d6a06b4b2" -dependencies = [ - "aho-corasick", - "memchr", - "regex-syntax", -] - -[[package]] -name = "regex-syntax" -version = "0.8.11" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d6f6ff9a378485b298a5286656da665ba74413d36db0979633275d2e708145d4" [[package]] name = "rustc-hash" diff --git a/engine/Cargo.toml b/engine/Cargo.toml index 13396ef4..f94d031a 100644 --- a/engine/Cargo.toml +++ b/engine/Cargo.toml @@ -10,7 +10,6 @@ license = "MIT" [workspace.dependencies] gix = { version = "0.87", default-features = false } -regex = "1" thiserror = "2" serde = { version = "1", features = ["derive"] } serde_json = "1" diff --git a/engine/native/core/Cargo.toml b/engine/native/core/Cargo.toml index e93c21fb..dd4b151c 100644 --- a/engine/native/core/Cargo.toml +++ b/engine/native/core/Cargo.toml @@ -19,7 +19,6 @@ gix = { version = "0.87", default-features = false, features = [ ] } bstr = "1" thiserror.workspace = true -regex.workspace = true serde.workspace = true serde_json.workspace = true diff --git a/engine/native/core/src/api.rs b/engine/native/core/src/api.rs index 41792c63..e0d35b97 100644 --- a/engine/native/core/src/api.rs +++ b/engine/native/core/src/api.rs @@ -37,8 +37,8 @@ use crate::repository::{Repo, RepoManager}; use crate::status::ScmChange; use crate::types::{ CommitFile, CommitOrdering, ConfigSnapshot, GitActivityCell, GitAuthor, GitAuthorStat, - GitCommitData, GitCommitDetails, GitFileChange, GitHistoryMatch, GitRepoInfo, GitStash, - GitTagDetails, LogOptions, RefReadOptions, RefSnapshot, + GitCommitData, GitCommitDetails, GitFileChange, GitRepoInfo, GitStash, GitTagDetails, + LogOptions, RefReadOptions, RefSnapshot, }; /// How a graph page is loaded. `Default` is the view's own default request: every local @@ -180,12 +180,6 @@ impl Engine { pub fn stashes(&self) -> Result> { crate::stash::read_stashes(&self.repo) } - - /// Search commit messages, newest first (`git log --all -E -i --grep`). - pub fn search_history(&self, query: &str) -> Result> { - crate::log::search_history(&self.repo, query) - } - /* ---------- Commits ---------- */ /// A commit in full: message, author, signature, parents and the files it changed. @@ -197,11 +191,6 @@ impl Engine { pub fn commit_bodies(&self, hashes: &[String]) -> Result> { crate::details::commit_bodies(&self.repo, hashes) } - - pub fn commit_subject(&self, hash: &str) -> Result { - crate::details::commit_subject(&self.repo, hash) - } - pub fn tag(&self, name: &str) -> Result { crate::details::tag_details(&self.repo, name) } @@ -264,15 +253,6 @@ impl Engine { pub fn config(&self) -> Result { crate::config::read_config(&self.repo) } - - pub fn current_branch(&self) -> Result> { - crate::config::current_branch_name(&self.repo) - } - - pub fn upstream_of_current_branch(&self) -> Result> { - crate::config::current_branch_upstream(&self.repo) - } - pub fn remote_url(&self, remote: &str) -> Result> { crate::config::remote_url(&self.repo, remote) } diff --git a/engine/native/core/src/config.rs b/engine/native/core/src/config.rs index ccb9f9f5..54af6775 100644 --- a/engine/native/core/src/config.rs +++ b/engine/native/core/src/config.rs @@ -73,42 +73,6 @@ pub fn remote_url(repo: &Repo, remote: &str) -> Result> { .string(format!("remote.{remote}.url").as_str()) .map(|value| value.to_string())) } - -/// The upstream of the checked-out branch, short-spelled as `git rev-parse --abbrev-ref -/// --symbolic-full-name @{upstream}` prints it (`origin/main`), or `None` when there is none. -pub fn current_branch_upstream(repo: &Repo) -> Result> { - let git = repo.borrow(); - let branch = git.head_name().ok().flatten().and_then(|name| { - name.as_bstr() - .strip_prefix(b"refs/heads/".as_slice()) - .map(|branch| String::from_utf8_lossy(branch).into_owned()) - }); - let Some(branch) = branch else { - // Detached HEAD tracks nothing. - return Ok(None); - }; - - let config = git.config_snapshot(); - let string = |key: String| config.string(key.as_str()).map(|value| value.to_string()); - let remote = string(format!("branch.{branch}.remote")); - let merge = string(format!("branch.{branch}.merge")); - let (remote, merge) = match (remote, merge) { - (Some(remote), Some(merge)) => (remote, merge), - _ => return Ok(None), - }; - - // `remote = .` means the upstream is local: the merge ref itself is the branch followed. - let short = merge - .strip_prefix("refs/heads/") - .unwrap_or(&merge) - .to_string(); - Ok(Some(if remote == "." { - short - } else { - format!("{remote}/{short}") - })) -} - /// The roots of the repository's initialised submodules, as the original extension gathered them /// from `.gitmodules`. /// @@ -159,23 +123,6 @@ fn submodule_root(root: &Path, path: &str) -> Option { } /* ---------- The remaining reads the settings panel and dialogs make ---------- */ - -/// The names of the repository's remotes, as `git remote` lists them (alphabetical). -pub fn remote_names(repo: &Repo) -> Result> { - Ok(repo.remote_names()) -} - -/// The checked-out branch's short name, or `None` when HEAD is detached — the answer -/// `git symbolic-ref --short HEAD` gives (an unborn branch still has its name). -pub fn current_branch_name(repo: &Repo) -> Result> { - let git = repo.borrow(); - Ok(git.head_name().ok().flatten().and_then(|name| { - name.as_bstr() - .strip_prefix(b"refs/heads/".as_slice()) - .map(|branch| String::from_utf8_lossy(branch).into_owned()) - })) -} - /// One location a configuration entry can live in, matching `git config --local` / `--global`. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ConfigLocation { diff --git a/engine/native/core/src/details.rs b/engine/native/core/src/details.rs index c53e3d93..c9ede594 100644 --- a/engine/native/core/src/details.rs +++ b/engine/native/core/src/details.rs @@ -153,19 +153,6 @@ pub fn commit_bodies(repo: &Repo, hashes: &[String]) -> Result Result { - let git = repo.borrow(); - let id = crate::repository::resolve_commit_in(&git, hash)?; - let commit = git.find_commit(id).git_ctx("Could not read the commit")?; - let message = commit - .message() - .git_ctx("Could not decode the commit message")?; - Ok(collapse_whitespace(message.summary().to_string())) -} - /// The summary of each of the given commits (author, email, author date, full message), keyed by /// hash — what the Commit Comparison View titles its two sides with. pub fn commit_summaries( @@ -333,8 +320,3 @@ fn strip_trailing_blank_lines(message: String) -> String { } lines.join("\n") } - -/// Trim and collapse every run of whitespace into a single space. -fn collapse_whitespace(text: String) -> String { - text.split_whitespace().collect::>().join(" ") -} diff --git a/engine/native/core/src/log.rs b/engine/native/core/src/log.rs index 59f3fc48..013c1685 100644 --- a/engine/native/core/src/log.rs +++ b/engine/native/core/src/log.rs @@ -19,7 +19,7 @@ use gix::ObjectId; use crate::error::{Error, Result, ResultExt}; use crate::repository::Repo; -use crate::types::{CommitOrdering, CommitRecord, GitAuthor, GitHistoryMatch}; +use crate::types::{CommitOrdering, CommitRecord, GitAuthor}; /// How many commits are read for every commit displayed, so that the topological re-ordering has /// enough of the graph to be exact over the page it returns. @@ -508,80 +508,6 @@ pub fn all_tips(repo: &Repo, include_tags: bool, include_remotes: bool) -> Resul /* ---------- History search ---------- */ -/// How many hits the Find dialogue shows, matching the original's `--max-count=100`. -const SEARCH_LIMIT: usize = 100; - -/// Search every commit message for a pattern, newest first, as `git log --all -E -i --grep`. -/// -/// The tips are everything `git log --all` walks from — local branches, tags, remote-tracking -/// branches, HEAD, and the stash, whose ref lives in `refs/` even though the graph never shows it. -/// The walk is commit-date ordered (git's default for `--grep`), not topologically constrained, so -/// it needs none of the windowed re-ordering the graph does. -pub fn search_history(repo: &Repo, query: &str) -> Result> { - let matcher = regex::RegexBuilder::new(query) - .case_insensitive(true) - .build() - .map_err(|e| Error::invalid_argument(format!("Invalid search query: {e}")))?; - - // A repository with no refs at all has nothing to search; git's `--all` simply matches nothing. - let mut tips = all_tips(repo, true, true).unwrap_or_default(); - if let Some(stash) = stash_tip(repo) { - if !tips.contains(&stash) { - tips.push(stash); - } - } - if tips.is_empty() { - return Ok(Vec::new()); - } - - let git = repo.borrow(); - let walk = git - .rev_walk(tips.iter().copied()) - .sorting(gix::revision::walk::Sorting::ByCommitTime( - gix::traverse::commit::simple::CommitTimeOrder::NewestFirst, - )) - .all() - .git_ctx("Could not walk the commit graph")?; - - let mut matches: Vec = Vec::new(); - for info in walk { - let info = match info { - Ok(info) => info, - // A missing object truncates the search rather than failing it, as it truncates the - // graph walk. - Err(_) => break, - }; - let commit = match git.find_commit(info.id) { - Ok(commit) => commit, - Err(_) => continue, - }; - let raw = commit - .message_raw() - .git_ctx("Could not decode the commit message")? - .to_string(); - if !matcher.is_match(&raw) { - continue; - } - let author = commit - .author() - .git_ctx("Could not decode the commit author")?; - matches.push(GitHistoryMatch { - hash: commit.id().detach().to_string(), - author: author.name.to_string(), - date: author.time().map(|time| time.seconds).unwrap_or(0), - message: commit - .message() - .git_ctx("Could not decode the commit message")? - .summary() - .to_string(), - }); - if matches.len() >= SEARCH_LIMIT { - break; - } - } - Ok(matches) -} - /// The commit `refs/stash` points at, if a stash exists. pub(crate) fn stash_tip(repo: &Repo) -> Option { let git = repo.borrow(); diff --git a/engine/native/core/src/search.rs b/engine/native/core/src/search.rs index a3e309e1..dd27547d 100644 --- a/engine/native/core/src/search.rs +++ b/engine/native/core/src/search.rs @@ -1,10 +1,10 @@ //! The Find dialogue's commit search, reproducing what the `git` CLI backend does. //! -//! This is deliberately *not* the engine's original `log::search_history`, which matches a regular +//! This is deliberately *not* the engine's original `log::search_history`, which matched a regular //! expression against commit messages across every ref. This project's search is a different -//! question, and wiring the regex one would have changed what users see: +//! question, and wiring the regex one would have changed what users see, so it was removed instead: //! -//! | | this search | `log::search_history` | +//! | | this search | `log::search_history` (removed) | //! | --- | --- | --- | //! | message match | literal substring, case-insensitive | regular expression | //! | author match | yes, literal substring | no | diff --git a/engine/native/core/src/stats.rs b/engine/native/core/src/stats.rs index 74125189..504271fc 100644 --- a/engine/native/core/src/stats.rs +++ b/engine/native/core/src/stats.rs @@ -14,7 +14,7 @@ use crate::log::{all_tips, stash_tip}; use crate::repository::Repo; use crate::types::{GitActivityCell, GitAuthorStat}; -/// `all_tips` plus the stash tip (if any), deduplicated - the same merge `search_history` +/// `all_tips` plus the stash tip (if any), deduplicated - the same merge `search_commits` /// performs, duplicated here rather than factored out so this module cannot change that /// already-tested walk's behaviour. fn all_tips_with_stash(repo: &Repo) -> Result> { diff --git a/engine/native/core/src/types.rs b/engine/native/core/src/types.rs index 12a9ed72..e56f49bc 100644 --- a/engine/native/core/src/types.rs +++ b/engine/native/core/src/types.rs @@ -359,17 +359,6 @@ pub struct GitActivityCell { pub count: usize, } -/// One hit of a commit-message search, as the Find dialogue lists them. -#[derive(Debug, Clone, Serialize, Deserialize)] -#[serde(rename_all = "camelCase")] -pub struct GitHistoryMatch { - pub hash: String, - pub author: String, - pub date: i64, - /// The commit subject. - pub message: String, -} - /* ---------- Tag details ---------- */ /// An annotated tag in full, or the fields a lightweight tag can fill in. diff --git a/engine/native/core/tests/api.rs b/engine/native/core/tests/api.rs index 43da1a20..806f04bb 100644 --- a/engine/native/core/tests/api.rs +++ b/engine/native/core/tests/api.rs @@ -41,7 +41,8 @@ fn the_engine_facade_covers_the_host_workflow() { let info = engine.info(&GraphOptions::default()).unwrap(); assert_eq!(info.branches, ["main"]); assert_eq!(info.tags, ["v1"]); - assert_eq!(engine.current_branch().unwrap().as_deref(), Some("main")); + // The checked-out branch, which the repo info already carries. + assert_eq!(info.head.as_deref(), Some("main")); // Commit details and the files it touched. let details = engine.commit(&second).unwrap(); @@ -81,12 +82,9 @@ fn the_engine_facade_covers_the_host_workflow() { let new = status.iter().find(|c| c.path == "new.txt").unwrap(); assert!(new.untracked); - // Diffs between revisions, and history search. + // Diffs between revisions. let changes = engine.diff(&first, &second).unwrap(); assert_eq!(changes[0].new_file_path, "a.txt"); - let hits = engine.search_history("second").unwrap(); - assert_eq!(hits.len(), 1); - assert_eq!(engine.commit_subject(&first).unwrap(), "first commit"); assert_eq!(engine.authors().unwrap()[0].name, "Test User"); engine.close(); diff --git a/engine/native/core/tests/queries.rs b/engine/native/core/tests/queries.rs index b1f4a035..bf941815 100644 --- a/engine/native/core/tests/queries.rs +++ b/engine/native/core/tests/queries.rs @@ -46,27 +46,6 @@ fn commit_bodies_fail_on_an_unknown_hash() { assert_eq!(error.kind, ErrorKind::NotFound); } -#[test] -fn reads_a_folded_subject_as_git_folds_it() { - require_git!(); - let mut repo = TestRepo::new(); - let hash = repo.commit_file( - "a.txt", - "1\n", - "a subject that spans\nseveral lines\n\nthe body is separate", - ); - - let expected = repo - .git(&["log", "--format=%s", "-n", "1", &hash]) - .split_whitespace() - .collect::>() - .join(" "); - - let engine = open(&repo); - assert_eq!(details::commit_subject(&engine, &hash).unwrap(), expected); - assert_eq!(expected, "a subject that spans several lines"); -} - #[test] fn reads_commit_summaries_with_the_author_date() { require_git!(); @@ -87,61 +66,6 @@ fn reads_commit_summaries_with_the_author_date() { assert_eq!(summary.date.to_string(), git_date.trim()); } -#[test] -fn searches_history_like_git_log_grep() { - require_git!(); - let mut repo = TestRepo::new(); - let alpha = repo.commit_file("a.txt", "1\n", "add the alpha feature"); - repo.commit_file("b.txt", "2\n", "an unrelated change"); - let gamma = repo.commit_file("c.txt", "3\n", "polish the gamma FEATURE"); - repo.git(&["checkout", "--quiet", "-b", "side"]); - let delta = repo.commit_file("d.txt", "4\n", "the delta feature lands"); - - // A stash is reachable from `--all` through refs/stash; its message must be searchable too. - repo.write("a.txt", "stashed\n"); - repo.git(&["stash", "push", "--quiet", "-m", "the stashed feature work"]); - let stash = repo.rev_parse("refs/stash"); - - let expected = repo.log_hashes(&["--all", "-i", "--grep=feature"]); - - let engine = open(&repo); - let matches = log::search_history(&engine, "feature").unwrap(); - let hashes: Vec<&String> = matches.iter().map(|m| &m.hash).collect(); - - assert_eq!(hashes, expected.iter().collect::>()); - assert!(hashes.contains(&&gamma)); - assert!(hashes.contains(&&stash), "the stash ref is part of --all"); - assert!( - hashes.contains(&&alpha), - "the alpha commit's message matches too" - ); - - // Subjects and authors come back with the hash. - for m in &matches { - if m.hash == delta { - assert_eq!(m.message, "the delta feature lands"); - assert_eq!(m.author, "Test User"); - } - } - - // A pattern that matches nothing matches nothing. - assert!(log::search_history(&engine, "no-such-thing-at-all") - .unwrap() - .is_empty()); -} - -#[test] -fn search_rejects_a_pattern_that_is_not_a_regex() { - require_git!(); - let mut repo = TestRepo::new(); - repo.commit_file("a.txt", "1\n", "first"); - - let engine = open(&repo); - let error = log::search_history(&engine, "(unclosed").unwrap_err(); - - assert_eq!(error.kind, ErrorKind::InvalidArgument); -} - #[test] fn reads_an_annotated_tag_in_full() { require_git!(); @@ -215,43 +139,6 @@ fn reads_a_remote_url_and_reports_an_absent_one() { assert_eq!(config::remote_url(&engine, "no-such-remote").unwrap(), None); } -#[test] -fn reads_the_upstream_of_the_checked_out_branch() { - require_git!(); - let mut repo = TestRepo::new(); - repo.commit_file("a.txt", "1\n", "first"); - repo.add_fake_remote("origin", "main", &repo.head()); - repo.git(&["config", "branch.main.remote", "origin"]); - repo.git(&["config", "branch.main.merge", "refs/heads/main"]); - - let expected = repo - .git(&[ - "rev-parse", - "--abbrev-ref", - "--symbolic-full-name", - "@{upstream}", - ]) - .trim() - .to_string(); - - let engine = open(&repo); - assert_eq!( - config::current_branch_upstream(&engine).unwrap().as_deref(), - Some(expected.as_str()) - ); - assert_eq!(expected, "origin/main"); -} - -#[test] -fn a_branch_without_an_upstream_has_none() { - require_git!(); - let mut repo = TestRepo::new(); - repo.commit_file("a.txt", "1\n", "first"); - - let engine = open(&repo); - assert_eq!(config::current_branch_upstream(&engine).unwrap(), None); -} - #[test] fn lists_initialised_submodules_only() { require_git!(); @@ -422,7 +309,7 @@ fn rejects_tag_names_git_would_reject() { } #[test] -fn a_bare_repository_has_no_upstream_and_no_submodules() { +fn a_bare_repository_has_no_submodules_and_still_reads_objects() { require_git!(); let mut repo = TestRepo::new(); let hash = repo.commit_file("a.txt", "1\n", "the subject\n\nand a body"); @@ -442,53 +329,15 @@ fn a_bare_repository_has_no_upstream_and_no_submodules() { ); let engine = Repo::open(bare.path()).expect("could not open the bare repository"); - assert_eq!(config::current_branch_upstream(&engine).unwrap(), None); assert!(config::submodules(&engine).unwrap().is_empty()); // The object database is all a bare repository has, and it is enough for every object read. - assert_eq!( - details::commit_subject(&engine, &hash).unwrap(), - "the subject" - ); assert_eq!( details::commit_bodies(&engine, std::slice::from_ref(&hash)).unwrap()[&hash], "the subject\n\nand a body" ); } -#[test] -fn a_detached_head_has_no_upstream() { - require_git!(); - let mut repo = TestRepo::new(); - let first = repo.commit_file("a.txt", "1\n", "first"); - repo.commit_file("b.txt", "2\n", "second"); - repo.git(&["checkout", "--quiet", "--detach", &first]); - - let engine = open(&repo); - assert_eq!(config::current_branch_upstream(&engine).unwrap(), None); -} - -#[test] -fn searching_a_repository_without_commits_matches_nothing() { - require_git!(); - let repo = TestRepo::new(); - - let engine = open(&repo); - assert!(log::search_history(&engine, "anything").unwrap().is_empty()); - assert_eq!( - log::count_commits_before( - &engine, - None, - "0123456789012345678901234567890123456789", - true, - false - ) - .unwrap_err() - .kind, - ErrorKind::NotFound - ); -} - /// The assertion helper the loop above uses, so each rejected name is reported individually. trait UnwrapErrOrElse { fn unwrap_err_or_else(self, message: impl FnOnce() -> String) -> git_graph_core::Error; @@ -761,47 +610,3 @@ fn aggregates_authors_like_shortlog() { // The same walk git's shortlog makes: three Test User commits against one Second Author. assert!(expected.contains("Second Author"), "shortlog: {expected}"); } - -#[test] -fn reads_the_checked_out_branch_name() { - require_git!(); - let mut repo = TestRepo::new(); - repo.commit_file("a.txt", "1\n", "first"); - - let engine = open(&repo); - assert_eq!( - config::current_branch_name(&engine).unwrap().as_deref(), - Some("main") - ); - - repo.git(&["checkout", "--quiet", "--detach", "HEAD"]); - assert_eq!(config::current_branch_name(&engine).unwrap(), None); -} - -#[test] -fn an_unborn_head_still_names_its_branch() { - require_git!(); - let repo = TestRepo::new(); - - let engine = open(&repo); - // `git symbolic-ref --short HEAD` prints the branch even before the first commit exists. - assert_eq!( - config::current_branch_name(&engine).unwrap().as_deref(), - Some("main") - ); -} - -#[test] -fn lists_remote_names_alphabetically() { - require_git!(); - let mut repo = TestRepo::new(); - repo.commit_file("a.txt", "1\n", "first"); - repo.git(&["remote", "add", "zeta", "https://example.invalid/z.git"]); - repo.git(&["remote", "add", "alpha", "https://example.invalid/a.git"]); - - let engine = open(&repo); - assert_eq!( - config::remote_names(&engine).unwrap(), - vec!["alpha", "zeta"] - ); -} diff --git a/engine/native/node/src/lib.rs b/engine/native/node/src/lib.rs index 6e53c7e5..8342c6a4 100644 --- a/engine/native/node/src/lib.rs +++ b/engine/native/node/src/lib.rs @@ -230,17 +230,6 @@ pub async fn load_commit_bodies(path: String, hashes: Vec) -> Result Result { - run(move || { - let repo = RepoManager::global().get(&path)?; - details::commit_subject(&repo, &hash) - }) - .await -} - /// The summary of each of the given commits, keyed by hash, as a JSON object. #[napi] pub async fn load_commit_summaries(path: String, hashes: Vec) -> Result { @@ -250,17 +239,6 @@ pub async fn load_commit_summaries(path: String, hashes: Vec) -> Result< }) .await } - -/// The commits whose message matches a pattern, newest first, as a JSON array. -#[napi] -pub async fn search_history(path: String, query: String) -> Result { - run(move || { - let repo = RepoManager::global().get(&path)?; - encode(&log::search_history(&repo, &query)?) - }) - .await -} - /// A tag in full (tagger, message, signature presence), as a JSON object. #[napi] pub async fn load_tag_details(path: String, tag_name: String) -> Result { @@ -304,17 +282,6 @@ pub async fn submodules(path: String) -> Result> { }) .await } - -/// The upstream of the checked-out branch (`origin/main`), or NULL when there is none. -#[napi] -pub async fn current_branch_upstream(path: String) -> Result> { - run(move || { - let repo = RepoManager::global().get(&path)?; - config::current_branch_upstream(&repo) - }) - .await -} - /// How many commits are reachable from the shown refs but not from `hash` — `git rev-list --count`. #[napi] pub async fn count_commits_before( @@ -342,17 +309,6 @@ pub async fn count_commits_before( pub async fn repo_root(path: String) -> Result { run(move || git_graph_core::repository::repo_root(&path)).await } - -/// The names of the repository's remotes. -#[napi] -pub async fn remote_names(path: String) -> Result> { - run(move || { - let repo = RepoManager::global().get(&path)?; - config::remote_names(&repo) - }) - .await -} - /// The distinct commit authors of the current branch's history, as a JSON array. #[napi] pub async fn authors(path: String) -> Result { @@ -401,17 +357,6 @@ pub async fn config_list(path: String, local: bool) -> Result { }) .await } - -/// The checked-out branch's short name, or NULL when HEAD is detached. -#[napi] -pub async fn current_branch_name(path: String) -> Result> { - run(move || { - let repo = RepoManager::global().get(&path)?; - config::current_branch_name(&repo) - }) - .await -} - /// The engine's version, so the extension can report which backend it is running. #[napi] pub fn engine_version() -> String { diff --git a/src/backend/engine/addon.ts b/src/backend/engine/addon.ts index 91f01d1f..b9e85395 100644 --- a/src/backend/engine/addon.ts +++ b/src/backend/engine/addon.ts @@ -46,8 +46,8 @@ export type EngineAddon = { /** * `search_commits` encoded as JSON: the Find dialogue's hits, each carrying * its position in the graph walk. Options are the JSON built by - * `buildSearchOptions`. Distinct from the engine's own `search_history`, - * which is a regex over messages and is deliberately not used. + * `buildSearchOptions`. This replaced the engine's own `search_history`, + * a regex over messages whose semantics did not match this project's. */ searchCommits(repoPath: string, optionsJson: string): Promise; /** @@ -101,8 +101,6 @@ export type EngineAddon = { * shows, including each remote's fetch and push URL. */ loadConfig(repoPath: string): Promise; - /** The checked-out branch's short name, or null when HEAD is detached. */ - currentBranchName(repoPath: string): Promise; /** * `load_refs` encoded as JSON. Only `head` is read here — the commit HEAD * resolves to, which `load_repo_info` does not carry (its `head` is the @@ -268,7 +266,6 @@ function isEngineAddon(loaded: unknown): loaded is EngineAddon { configList?: unknown; authors?: unknown; loadConfig?: unknown; - currentBranchName?: unknown; searchCommits?: unknown; loadRefs?: unknown; closeRepository?: unknown; @@ -289,7 +286,6 @@ function isEngineAddon(loaded: unknown): loaded is EngineAddon { typeof candidate.configList === "function" && typeof candidate.authors === "function" && typeof candidate.loadConfig === "function" && - typeof candidate.currentBranchName === "function" && typeof candidate.searchCommits === "function" && typeof candidate.loadRefs === "function" && typeof candidate.closeRepository === "function" && diff --git a/src/backend/engine/search.ts b/src/backend/engine/search.ts index 72206f7c..3e501f89 100644 --- a/src/backend/engine/search.ts +++ b/src/backend/engine/search.ts @@ -8,12 +8,12 @@ * * ### Why this is not the engine's own `search_history` * - * The engine ships a search already, and wiring *that* one would have changed - * what users see: it matches a **regular expression** against messages only, - * across every ref, ignoring the author filter, and numbers nothing. Slice - * 16.7 declined it for exactly that reason. `search_commits` was added to the - * engine instead, reproducing this project's semantics; the regex one is left - * where it is, unused. + * The engine shipped a search already, and wiring *that* one would have + * changed what users see: it matched a **regular expression** against messages + * only, across every ref, ignoring the author filter, and numbered nothing. + * Slice 16.7 declined it for exactly that reason. `search_commits` was added + * to the engine instead, reproducing this project's semantics, and the regex + * one has since been removed. * * ### Declines * diff --git a/tests/backend/engine/addon.test.ts b/tests/backend/engine/addon.test.ts index 58e78240..27c92729 100644 --- a/tests/backend/engine/addon.test.ts +++ b/tests/backend/engine/addon.test.ts @@ -91,7 +91,6 @@ describe("engine addon loader", () => { configList: async () => "{}", authors: async () => "[]", loadConfig: async () => "{}", - currentBranchName: async () => null, searchCommits: async () => "[]", loadRefs: async () => "{}", closeRepository: () => {}, diff --git a/tests/backend/engine/reader.test.ts b/tests/backend/engine/reader.test.ts index 54c6a85f..d9a21a90 100644 --- a/tests/backend/engine/reader.test.ts +++ b/tests/backend/engine/reader.test.ts @@ -145,7 +145,6 @@ function fakeAddon(implementation: (repoPath: string) => Promise) }, authors: async () => "[]", loadConfig: async () => JSON.stringify({ remotes: [] }), - currentBranchName: async () => null, searchCommits: async () => "[]", loadRefs: async () => JSON.stringify({ head: null }), configList: async () => { @@ -413,7 +412,6 @@ describe("createRepoReader repoInfo", () => { JSON.stringify({ remotes: [{ name: "origin", url: "https://github.com/some/repo.git", pushUrl: null }] }), - currentBranchName: async () => null, searchCommits: async () => "[]", loadRefs: async () => JSON.stringify({ head: null }), configList: async (_repo: string, local: boolean) => @@ -466,7 +464,6 @@ describe("createRepoReader repoInfo", () => { }, authors: async () => "[]", loadConfig: async () => JSON.stringify({ remotes: [] }), - currentBranchName: async () => null, searchCommits: async () => "[]", loadRefs: async () => JSON.stringify({ head: null }), configList: async () => { @@ -521,7 +518,6 @@ describe("createRepoReader repoInfo", () => { }, authors: async () => "[]", loadConfig: async () => JSON.stringify({ remotes: [] }), - currentBranchName: async () => null, searchCommits: async () => "[]", loadRefs: async () => JSON.stringify({ head: null }), configList: async () => { @@ -575,7 +571,6 @@ describe("createRepoReader repoInfo", () => { }, authors: async () => "[]", loadConfig: async () => JSON.stringify({ remotes: [] }), - currentBranchName: async () => null, searchCommits: async () => "[]", loadRefs: async () => JSON.stringify({ head: null }), configList: async () => { @@ -630,7 +625,6 @@ describe("createRepoReader repoInfo", () => { }, authors: async () => "[]", loadConfig: async () => JSON.stringify({ remotes: [] }), - currentBranchName: async () => null, searchCommits: async () => "[]", loadRefs: async () => JSON.stringify({ head: null }), configList: async () => { @@ -789,7 +783,6 @@ describe("createRepoReader loadCommits", () => { loadCommitFile: unsupported, authors: unsupported, loadConfig: unsupported, - currentBranchName: unsupported, searchCommits: unsupported, loadRefs: unsupported, configList: unsupported, @@ -917,7 +910,6 @@ describe("createRepoReader loadCommits", () => { }, authors: async () => "[]", loadConfig: async () => JSON.stringify({ remotes: [] }), - currentBranchName: async () => null, searchCommits: async () => "[]", loadRefs: async () => JSON.stringify({ head: null }), configList: async () => { @@ -1048,7 +1040,6 @@ describe("createRepoReader loadCommits", () => { }, authors: async () => "[]", loadConfig: async () => JSON.stringify({ remotes: [] }), - currentBranchName: async () => null, searchCommits: async () => "[]", loadRefs: async () => JSON.stringify({ head: null }), configList: async () => { @@ -1157,7 +1148,6 @@ describe("createRepoReader loadCommitDetails", () => { loadCommitFile: unsupported, authors: unsupported, loadConfig: unsupported, - currentBranchName: unsupported, searchCommits: unsupported, loadRefs: unsupported, configList: unsupported, @@ -1434,7 +1424,6 @@ describe("createRepoReader loadCommitComparison", () => { loadCommitFile: unsupported, authors: unsupported, loadConfig: unsupported, - currentBranchName: unsupported, searchCommits: unsupported, loadRefs: unsupported, configList: unsupported, From 11a39527f20afe6a44adc464a6c6f09f544f8454 Mon Sep 17 00:00:00 2001 From: Zamkorus Date: Sat, 3 Oct 2026 20:32:13 +0200 Subject: [PATCH 09/10] feat(dependabot): enhance configuration for GitHub Actions, npm, and cargo updates --- .github/dependabot.yml | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 5ace4600..c95d7453 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -1,6 +1,44 @@ version: 2 updates: + # One PR for all actions, so upload-artifact and download-artifact always + # move together. - package-ecosystem: "github-actions" directory: "/" schedule: interval: "weekly" + groups: + actions: + patterns: ["*"] + + # The extension's packages. engine/package.json declares none of its own. + - package-ecosystem: "npm" + directory: "/" + schedule: + interval: "weekly" + # pnpm refuses anything published less than a day ago (minimumReleaseAge), + # so give a release a few days before proposing it. + cooldown: + default-days: 3 + groups: + npm-minor-patch: + update-types: ["minor", "patch"] + ignore: + # Must not run ahead of engines.vscode, or vsce refuses to package. + # Raise both together, by hand, when the minimum VS Code moves. + - dependency-name: "@types/vscode" + update-types: ["version-update:semver-major", "version-update:semver-minor"] + + # The Rust engine. rust-toolchain.toml is left out on purpose: raising the + # channel is a deliberate act (see that file). + - package-ecosystem: "cargo" + directory: "/engine" + schedule: + interval: "weekly" + groups: + # The napi crates move in lockstep, majors included. + napi: + patterns: ["napi", "napi-derive", "napi-build"] + # gix is pre-1.0, so its "minor" bumps break; keep them in their own PR. + cargo-minor-patch: + update-types: ["minor", "patch"] + exclude-patterns: ["gix"] From 734ee6c514ed29015c9a88887f2b8e05098a566d Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sat, 3 Oct 2026 18:37:11 +0000 Subject: [PATCH 10/10] build(deps): bump simple-git from 3.36.0 to 4.0.2 Bumps [simple-git](https://github.com/steveukx/git-js/tree/HEAD/simple-git) from 3.36.0 to 4.0.2. - [Release notes](https://github.com/steveukx/git-js/releases) - [Changelog](https://github.com/steveukx/git-js/blob/main/simple-git/CHANGELOG.md) - [Commits](https://github.com/steveukx/git-js/commits/simple-git@4.0.2/simple-git) --- updated-dependencies: - dependency-name: simple-git dependency-version: 4.0.2 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- package.json | 2 +- pnpm-lock.yaml | 28 ++++++++++++++-------------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/package.json b/package.json index b4ca0d50..4edac3c0 100644 --- a/package.json +++ b/package.json @@ -118,7 +118,7 @@ "icons:generate": "node ./scripts/generate-octicons.js && biome format --write src/octicons.ts" }, "dependencies": { - "simple-git": "^3.36.0" + "simple-git": "^4.0.2" }, "devDependencies": { "@biomejs/biome": "^2.5.13", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 43d12653..beecd36d 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -110,8 +110,8 @@ importers: .: dependencies: simple-git: - specifier: ^3.36.0 - version: 3.36.0(supports-color@8.1.1) + specifier: ^4.0.2 + version: 4.0.2(supports-color@8.1.1) devDependencies: '@biomejs/biome': specifier: ^2.5.13 @@ -1585,11 +1585,11 @@ packages: resolution: {integrity: sha512-Nqc90v4lWCXyakD6xNyNACBJNJ0tNCwj2WNk/7ivyacYHxiITVgmLUFXTBOeCdy79iz6HtN9Y31uw/jbLrdOAg==} engines: {node: '>=20.0.0'} - '@simple-git/args-pathspec@1.0.3': - resolution: {integrity: sha512-ngJMaHlsWDTfjyq9F3VIQ8b7NXbBLq5j9i5bJ6XLYtD6qlDXT7fdKY2KscWWUF8t18xx052Y/PUO1K1TRc9yKA==} + '@simple-git/args-pathspec@1.0.4': + resolution: {integrity: sha512-EtMX6XjRWSastG2SdkmSQByPtJ9dx/NIjnHbpQPCYt62j4RmSx5rgLTGpw0YCjF5h191gZOmBfheOT23cRSFdw==} - '@simple-git/argv-parser@1.1.1': - resolution: {integrity: sha512-Q9lBcfQ+VQCpQqGJFHe5yooOS5hGdLFFbJ5R+R5aDsnkPCahtn1hSkMcORX65J2Z5lxSkD0lQorMsncuBQxYUw==} + '@simple-git/argv-parser@2.0.1': + resolution: {integrity: sha512-M++IaVWrN+vYalilOHfwvT2IL3wYZtVEBQpmX6DLSOJLtxbZ+SUeIDE3IiPogzhPACLajwtxfa5rp+ZX1FczzQ==} '@sindresorhus/merge-streams@2.3.0': resolution: {integrity: sha512-LtoMMhxAlorcGhmFYI+LhPgbPZCkgP6ra1YL604EeF6U98pLlQ3iWIGMdWSC+vWmPBWBNgmDBAhnAobLROJmwg==} @@ -3264,8 +3264,8 @@ packages: simple-get@4.0.1: resolution: {integrity: sha512-brv7p5WgH0jmQJr1ZDDfKDOSeWWg+OVypG99A/5vYGPqJ6pxiaHLy8nxtFjBA7oMa01ebA9gfh1uMCFqOuXxvA==} - simple-git@3.36.0: - resolution: {integrity: sha512-cGQjLjK8bxJw4QuYT7gxHw3/IouVESbhahSsHrX97MzCL1gu2u7oy38W6L2ZIGECEfIBG4BabsWDPjBxJENv9Q==} + simple-git@4.0.2: + resolution: {integrity: sha512-l0sIsv9VrPwovKELFDBOj/lhmIcbMvxBk2B+hRhsMM4/VL3gKtuZvYnBNixVo+N/0HnqQx2cBdLvxnAbygHpqA==} simple-invariant@2.0.1: resolution: {integrity: sha512-1sbhsxqI+I2tqlmjbz99GXNmZtr6tKIyEgGGnJw/MKGblalqk/XoOYYFJlBzTKZCxx8kLaD3FD5s9BEEjx5Pyg==} @@ -4912,11 +4912,11 @@ snapshots: '@secretlint/types@10.2.2': {} - '@simple-git/args-pathspec@1.0.3': {} + '@simple-git/args-pathspec@1.0.4': {} - '@simple-git/argv-parser@1.1.1': + '@simple-git/argv-parser@2.0.1': dependencies: - '@simple-git/args-pathspec': 1.0.3 + '@simple-git/args-pathspec': 1.0.4 '@sindresorhus/merge-streams@2.3.0': {} @@ -6677,12 +6677,12 @@ snapshots: simple-concat: 1.0.1 optional: true - simple-git@3.36.0(supports-color@8.1.1): + simple-git@4.0.2(supports-color@8.1.1): dependencies: '@kwsites/file-exists': 1.1.1(supports-color@8.1.1) '@kwsites/promise-deferred': 1.1.1 - '@simple-git/args-pathspec': 1.0.3 - '@simple-git/argv-parser': 1.1.1 + '@simple-git/args-pathspec': 1.0.4 + '@simple-git/argv-parser': 2.0.1 debug: 4.4.3(supports-color@8.1.1) transitivePeerDependencies: - supports-color