Skip to content

fix(status): detect committed but unindexed changes - #1843

Closed
bompus wants to merge 2 commits into
colbymchenry:mainfrom
bompus:fix/upstream-committed-index-detection
Closed

bompus wants to merge 2 commits into
colbymchenry:mainfrom
bompus:fix/upstream-committed-index-detection

Conversation

@bompus

@bompus bompus commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Committing an addition, edit, or deletion before indexing currently makes codegraph status report zero pending changes. This change compares the full in-scope source inventory with indexed hashes, so pending changes remain visible until sync absorbs them.

Fixes #1829.

The existing inventory comparison now handles Git projects as well as non-Git projects. It retains scope filtering and checks disk existence for removals, since git ls-files can still list unstaged deletions. No schema change or new dependency is needed.

Regression tests cover additions, edits, and deletions before and after committing, recovery after sync, committed renames, and graph preservation across repeated no-op syncs. The three addition/edit/deletion cases failed at the post-commit assertion before the fix.

Validation

Based on upstream 3ed73bc127323e63153bf6ec8354afa82ce36aaf; head a5709f47 contains only this fix and its tests.

  • Exact PR branch, WSL/Linux, Node 24.21.0: 59 tests passed across sync, include-ignored-config, git-changed-untracked-dir, and extraction-old-git, with CODEGRAPH_KERNEL=0, one worker. TypeScript compilation and diff whitespace checks passed.
  • Deployed downstream integration 3f1c3309, including this fix and other fork changes: 4,756 native tests and 4,754 WASM tests passed, zero failures; all five downstream source probes passed. These are integration results, not a full-suite run of this exact PR branch.
  • Windows and the viewer build were not tested for this PR.

Performance tradeoff

Reading and hashing the full inventory costs more than the incomplete Git-status candidate list. A disposable VS Code checkout at 7b7e49c83affacfac726040280da69b4999f3e01 contained 14,486 in-scope files, 166.77 MiB:

State Before median Fixed median Fixed min–max
Clean 50.2 ms 614.2 ms 600.0–642.2 ms
One uncommitted edit 59.1 ms 676.7 ms 604.6–780.7 ms
Same edit committed without indexing 51.1 ms (incorrectly clean) 604.4 ms (correctly modified) 596.0–634.5 ms

Measured on WSL/ext4 with Node 24.21.0 and nice -n 10: two warm-ups, then ten samples per arm per case, alternating arm order. The actual before/fixed detection methods from downstream d5607a08 and 6cb53e82 used the same supporting modules and SQLite file records seeded from the checkout's current hashes. Those methods differ only by this fix; no symbol graph was extracted. All returned change sets were asserted.

These are warm-cache change-detection timings, excluding CLI startup and other status queries—not end-to-end CLI or cold-storage measurements. The shared host had background services; the Android build had completed before the reported run. An earlier run during build activity was excluded. The measured added cost is about 0.55–0.62 seconds on this repository.

@inth3shadows

Copy link
Copy Markdown

Opened #1848 as an alternative shape for this same bug — not a competing claim on it, your diagnosis matches mine exactly and you were first. Flagging it here so the two can be compared side by side.

It keeps the git fast path and adds the missing half by stamping the commit the index was built at, then asking git for the committed diff since that commit. All three arms measured on one machine against VS Code at 7b7e49c8 — the same commit your table uses — reading a single index of 14,137 files, timing getChangedFiles() in-process, median of 10 with the arm order reversed on a second pass:

State main @ 3ed73bc #1843 #1848
Clean 119 ms 1470 ms 132 ms
One uncommitted edit 131 ms 1488 ms 136 ms
Committed, not indexed 122 ms (reports 0) 1547 ms 146 ms

Absolute numbers run higher than yours on every arm — different, busier machine — so the ratio is the comparable part.

One case yours catches and mine does not: a file hidden with git update-index --assume-unchanged and then edited. Your inventory walk sees it; neither git status nor git diff reports it, so #1848 misses it exactly as main does. Verified on all three builds. If that case outweighs the per-call cost, yours is the right fix and mine isn't — happy either way.

@bompus

bompus commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for measuring all three arms on one machine — that table is the comparison I'd want a maintainer to see.

I'd take #1848. The per-call cost of the inventory walk is paid by status, every sync, and the watcher's catch-up, and a ~11x hit on a large checkout is a bad trade for catching one rare case. The commit stamp closes the real gap (committed add/modify/delete since the index) at ~1.1x, and the fallback to the full scan when the stamp can't resolve means an old index heals itself after one call.

On the assume-unchanged case: it's real but it's a user opting out of git's own change tracking, and the previous behaviour missed it too, so #1848 is no regression there. If it ever matters, the inventory walk from this PR could sit behind an explicit status --full or a periodic check rather than the hot path.

Two things I'd carry over from here if #1848 lands: the committed-rename test, and the disk-existence check on removals so an unstaged delete that git ls-files still lists is reported as removed. If both are already covered by your nine cases, ignore this.

Happy for the maintainer to close this in favour of #1848. I'll leave it open until they pick a shape.

@bompus

bompus commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

Correction to my last paragraph: both carry-over items are already covered in #1848 — the committed-rename case has its own test there, and an unstaged delete is still reported by the unchanged git status path, so the disk-existence check only matters on the inventory walk #1848 doesn't take. Disregard that paragraph.

bompus added a commit to bompus/codegraph that referenced this pull request Sep 11, 2026
…es (colbymchenry#1829)

Replace the full source-inventory comparison from colbymchenry#1843 with upstream PR colbymchenry#1848:
the index stamps the commit it was built at and change detection adds the
committed diff since that commit to the git status candidates. Falls back to
the full scan when the stamp cannot be resolved.
@bompus

bompus commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

Closing in favour of #1848. My fork has adopted that shape; the full native and WASM suites pass on it with the same tests. The inventory walk here is available in history if the assume-unchanged case ever needs it.

@bompus bompus closed this Sep 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

status.pendingChanges reports zero for a committed-but-unindexed file — the git fast path never hash-checks what git omits

2 participants