Skip to content

docs: recommend Git LFS for team-shared graph artifact - #1697

Merged
DeusData merged 4 commits into
DeusData:mainfrom
angusgastle:docs/team-artifact-lfs-guidance
Sep 3, 2026
Merged

docs: recommend Git LFS for team-shared graph artifact#1697
DeusData merged 4 commits into
DeusData:mainfrom
angusgastle:docs/team-artifact-lfs-guidance

Conversation

@angusgastle

Copy link
Copy Markdown
Contributor

Summary

Adds guidance to the Team-Shared Graph Artifact section recommending Git LFS for the .codebase-memory/graph.db.zst artifact in team-shared repos.

The problem we hit

The existing guidance told teams to commit graph.db.zst as a regular file with merge=ours. That works for merge conflicts but ignores the bigger issue: the artifact is a ~20MB binary that regenerates on every re-index, and committing it plainly stores a full new blob each time.

We discovered this on a real monorepo (internal HR/finance app, ~2047 files). Within a normal week of indexing:

  • The graph.db.zst blob was committed hundreds of times across history.
  • The repo ballooned to ~7GB of bloat from that single file.
  • Fresh clones were downloading gigabytes of binary blobs that were functionally identical snapshots.
  • main alone carried 352 commits with the raw ~20MB blob (~6 GB).

We reclaimed the space with a git-filter-repo history rewrite that converted the artifact to a Git LFS pointer across all history. After the rewrite, a fresh clone dropped from ~6 GB to ~534 MB — roughly a 10× reduction — and main's .git went from 5.9 GB to under 1 GB.

The fix

The artifact still regenerates, but with LFS the binary is stored out-of-band rather than as a new blob per commit. The .gitattributes tracking line:

.codebase-memory/graph.db.zst filter=lfs diff=lfs merge=lfs -text

The merge=ours conflict-avoidance the original guidance emphasized is preserved by LFS's own merge=lfs handler.

What this PR changes

  • Adds a "Use Git LFS for team repos" bullet to the Team-Shared Graph Artifact section.
  • Notes that migrating an existing repo where the artifact was already committed plainly requires stripping historical blobs with git-filter-repo first (the merge=ours → LFS switch on its own only affects future commits).

Notes

  • This is a docs-only change; no code paths are touched.
  • The export path still auto-creates the merge=ours line on first export. A follow-up could auto-emit the LFS line (or detect an existing LFS setup), but that's a code change left for a separate PR.
  • The ~7GB figure comes from our internal history-rewrite log; the ratio (10× clone reduction after LFS) is what generalizes.

🤖 Generated with Claude Code

@angusgastle
angusgastle requested a review from DeusData as a code owner August 17, 2026 22:21
@github-actions

Copy link
Copy Markdown

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

  • It will not be closed for inactivity. No stale bot touches pull requests here.
  • It may still sit a while before a human reads it. That is on us, not on you.
  • Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

  • Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
  • Get CI green, or say which failures you believe are pre-existing.
  • Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
  • Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

@angusgastle
angusgastle force-pushed the docs/team-artifact-lfs-guidance branch 2 times, most recently from 9abcf4f to 5dc36a9 Compare August 18, 2026 01:20
@DeusData DeusData added documentation Improvements or additions to documentation priority/backlog Valuable contribution, lower scheduling urgency; review when maintainer capacity opens. labels Aug 18, 2026
@angusgastle
angusgastle force-pushed the docs/team-artifact-lfs-guidance branch from 5dc36a9 to 22f90fe Compare September 1, 2026 02:54
@DeusData

DeusData commented Sep 1, 2026

Copy link
Copy Markdown
Owner

Landed as #1992 — thank you, and sorry it took two weeks to reach you while you were keeping the branch rebased.

The problem you found is real and the README should have warned about it. The artifact regenerates on every index, including the watcher's Fast tier, and nothing in that section connected "rewritten on every index" to "commit it to your repo". You hit the consequence on a real monorepo, measured it, rewrote ~6 GB back out with git-filter-repo, and reported the ~10x clone reduction. That is a better report than most, and #1992 carries you as co-author.

Two things changed on the way in. The second one matters, because it would have bitten anyone who followed the instruction as written.

The .gitattributes placement. The bullet said to replace the auto-created merge=ours entry — but that entry is not at the repo root. It is written to .codebase-memory/.gitattributes with the bare pattern graph.db.zst (ensure_gitattributes, src/pipeline/artifact.c:776). A pattern containing a slash is anchored to the directory holding its .gitattributes file, so .codebase-memory/graph.db.zst placed inside .codebase-memory/ resolves to .codebase-memory/.codebase-memory/graph.db.zst and matches nothing. Replacing the line there would leave LFS quietly disengaged and drop merge=ours — worse than doing nothing — and since ensure_gitattributes opens with O_EXCL it never rewrites an existing file, so nothing would put it back.

The version that works keeps both files: the LFS line at the repo root, the auto-created one untouched. Attributes resolve per attribute from the nearest .gitattributes outward, so merge=ours goes on coming from .codebase-memory/ and only filter=lfs comes from the root. You get LFS and the conflict avoidance together.

To be clear about where the fault lies: the old README wording said only "a .gitattributes line" and never named the file, which is exactly what makes "replace it" the natural reading. That wording is fixed in #1992 too.

The costs got named. Putting LFS in the README commits every reader to it, and it is a metered store rather than a free one — GitHub bills LFS storage and bandwidth, LFS objects cannot be pruned without contacting support, and a teammate who clones without git lfs install gets a pointer file that the integrity-checked import refuses, dropping them into the full reindex the artifact exists to avoid. So #1992 leads with the cheaper fix — commit on a cadence you choose rather than on every regeneration — and offers LFS as the answer for teams that genuinely need the file to move on every commit.

Thanks again. This is the kind of thing that only surfaces after someone runs into it for real, and the docs are better for it.

@DeusData DeusData closed this Sep 1, 2026
@DeusData DeusData reopened this Sep 1, 2026
@DeusData

DeusData commented Sep 1, 2026

Copy link
Copy Markdown
Owner

Reopening, and apologies for the churn — that close was wrong of me. You rebased this only a few hours ago, so you are plainly still on it, and a PR that is one edit away from landing should stay yours rather than be reimplemented around you. I have closed #1992. This is the one that should merge.

The comment above is the whole review; nothing new. Concretely, three changes and it is ready:

1. Put the LFS line at the repo root, and keep the auto-created file. Don't tell readers to replace the merge=ours entry — it isn't at the root, and the replacement wouldn't match. Root .gitattributes gets the LFS line, .codebase-memory/.gitattributes stays exactly as the tool wrote it. Attributes resolve per attribute from the nearest file outward, so merge=ours keeps coming from .codebase-memory/ and only filter=lfs comes from the root — you get both.

2. Name the costs. GitHub meters LFS storage and bandwidth, its objects can't be pruned without contacting support, and a teammate without git lfs install checks out a pointer file that the integrity-checked import refuses — which drops them into the full reindex the artifact exists to avoid.

3. Lead with the cheaper fix. The root cause in your story is committing every regeneration, including the watcher's Fast tier. LFS is the right answer for teams that genuinely need the file to move on every commit; for everyone else, choosing a cadence is cheaper and has no quota attached.

If it saves you time, take this wording verbatim — it's yours, no attribution needed. It replaces the existing No merge pain bullet (which also gains the filename, since that omission is what made "replace it" the natural reading):

- **No merge pain**: a `.codebase-memory/.gitattributes` line with `merge=ours` is auto-created on first export, so concurrent edits don't produce conflicts on the binary artifact
- **Commit it deliberately**: the artifact is rewritten on every index, including the watcher's Fast tier, and git stores each rewrite as a full new blob. Committing every refresh is what turns a 20 MB file into gigabytes of history — one team reached ~6 GB across ~350 commits of this single path. Pick a cadence (a release, a milestone, a nightly job) rather than committing every save.
- **Git LFS, if it must move on every commit**: track it from the **repo-root** `.gitattributes` and leave the auto-created `.codebase-memory/.gitattributes` in place — the nearer file goes on supplying `merge=ours`, and only `filter` comes from the root:
  ```gitattributes
  .codebase-memory/graph.db.zst filter=lfs diff=lfs merge=lfs -text
  ```
  Track only the `.zst`; `artifact.json` is small and carries the schema version. The attribute applies to future commits only, so a repo that already has the blobs in history needs `git-filter-repo` to rewrite them first. Two costs to weigh before adopting it: GitHub meters LFS storage and bandwidth, and its objects cannot be pruned without contacting support; and every teammate needs `git lfs install` — without it their checkout leaves a pointer file where the artifact should be, the integrity-checked import refuses it, and they fall back to a full reindex.

Push whenever suits you and I'll take it from there. Thanks for your patience with the round trip.

Signed-off-by: Angus Gastle <angusgastle@gmail.com>
@angusgastle
angusgastle force-pushed the docs/team-artifact-lfs-guidance branch from 22f90fe to 4b844b7 Compare September 1, 2026 16:44
angusgastle added a commit to angusgastle/codebase-memory-mcp that referenced this pull request Sep 1, 2026
Address maintainer review on PR DeusData#1697:
- LFS pattern goes in the repo-root .gitattributes; the auto-created
  .codebase-memory/.gitattributes (bare graph.db.zst, merge=ours) is
  kept as-is, since a slash-containing pattern inside it would anchor
  to .codebase-memory/ and match nothing
- Lead with commit cadence as the cheaper fix; LFS is for teams that
  need the artifact to move on every commit
- Name the LFS costs: metered storage/bandwidth, no pruning without
  support, and teammates without `git lfs install` fall back to a
  full reindex

Co-Authored-By: Claude Code <noreply@anthropic.com>
@DeusData

DeusData commented Sep 1, 2026

Copy link
Copy Markdown
Owner

That is exactly it — thank you for turning it around so quickly, and for writing a commit message that explains why the pattern moves to the repo root rather than just that it does. The next person to touch this will be glad it is there.

One thing left, and it is small: 4b844b7d has no sign-off. The first commit has it; the second does not, so dco is red. Amend and force-push:

git commit --amend -s --no-edit
git push --force-with-lease

To save you second-guessing it: the Co-Authored-By: Claude Code trailer is fine and does not need removing. Honest attribution of tool use is welcome here. The sign-off is the only trailer that is required.

The analyze and security / codeql-gate failures are not yours. main does not currently compile — two changes landed the same type short-name index into type_registry.h and git merged them into duplicate struct members, so anything that builds against main fails, including CodeQL on a docs-only PR. #1993 is the repair. Once it lands and you have pushed the sign-off, this should go green and I will merge it.

Nothing else outstanding.

Address maintainer review on PR DeusData#1697:
- LFS pattern goes in the repo-root .gitattributes; the auto-created
  .codebase-memory/.gitattributes (bare graph.db.zst, merge=ours) is
  kept as-is, since a slash-containing pattern inside it would anchor
  to .codebase-memory/ and match nothing
- Lead with commit cadence as the cheaper fix; LFS is for teams that
  need the artifact to move on every commit
- Name the LFS costs: metered storage/bandwidth, no pruning without
  support, and teammates without `git lfs install` fall back to a
  full reindex

Co-Authored-By: Claude Code <noreply@anthropic.com>
Signed-off-by: Angus Gastle <angusgastle@gmail.com>
@angusgastle
angusgastle force-pushed the docs/team-artifact-lfs-guidance branch from 4b844b7 to 957b27f Compare September 1, 2026 18:07
@angusgastle

Copy link
Copy Markdown
Contributor Author

Thank you for the thorough review — and for reopening this after the #1992 close. The anchoring catch in particular (graph.db.zst written inside .codebase-memory/.gitattributes would resolve to .codebase-memory/.codebase-memory/graph.db.zst and match nothing, silently dropping both LFS and merge=ours) is exactly the kind of thing that would have shipped and not been noticed until someone's clone was 6 GB again. The suggestion to resolve attributes per-attribute from the nearest file outward is what made keeping both files work cleanly.

All three points from the review are now pushed as 957b27fc (amended from 4b844b7d):

  1. Repo-root .gitattributes for the LFS line, auto-created file untouched. The README now instructs readers to track from the repo root and explicitly says to leave .codebase-memory/.gitattributes alone — merge=ours keeps coming from the nearer file, filter=lfs from the root.
  2. Commit cadence leads. "Commit it deliberately" is now the second bullet and names the root cause directly: the artifact is rewritten on every index including the watcher's Fast tier, and git stores each rewrite as a full new blob — that's what turns a 20 MB file into gigabytes of history. LFS is framed as the answer only for teams that genuinely need the file to move on every commit.
  3. Costs named. The LFS bullet covers metered storage/bandwidth, no pruning without contacting support, and the failure mode where a teammate without git lfs install checks out a pointer file that the integrity-checked import refuses — falling back to the full reindex the artifact exists to avoid.

Also added per your last note: a Track only the .zst line (artifact.json is small and carries the schema version) and the git-filter-repo requirement for repos that already have plain blobs in history.

Sign-off: good catch — 4b844b7d was missing Signed-off-by (only the first commit had it). Amended and force-pushed; dco should go green on the next run. Keeping the Co-Authored-By: Claude Code trailer per your note.

CI: understood on analyze / security / codeql-gate / the test matrix — a docs-only diff can't cause those, and the duplicate struct members in type_registry.h explain all of them. Watching #1993; happy to rebase once it lands if that helps get this green.

Thanks again for the patience on both ends of this round trip.

@DeusData DeusData closed this Sep 1, 2026
@DeusData DeusData reopened this Sep 1, 2026
@DeusData

DeusData commented Sep 1, 2026

Copy link
Copy Markdown
Owner

Housekeeping note, so the close-and-reopen above does not look alarming: it was a CI plumbing fix, not a decision about this PR.

Your three checks — dco, analyze and CodeQL — are all green. But ci-ok is a required check here and it never appeared, because the PR workflow run had wedged: it sat in pending for the best part of an hour with zero jobs and its timestamp frozen, while other pull requests' runs started and finished around it.

The workflow uses concurrency: group: pr-<number> with cancel-in-progress, so re-running the stuck run just made it contend with the group it was already holding — a second attempt sat pending exactly the same way. Only a genuinely new run can take that group, and a reopen is the cheapest way to trigger one without touching your branch or adding a commit to your history. A fresh run now exists and the stuck one has been released.

Nothing is being asked of you and nothing about the change has changed. Once ci-ok lands green this merges.

@DeusData
DeusData merged commit 2f9828d into DeusData:main Sep 3, 2026
34 checks passed
@DeusData

DeusData commented Sep 3, 2026

Copy link
Copy Markdown
Owner

Merged as 2f9828d -- thank you, Angus, and welcome as a contributor!

This one took a longer road than a seven-line README change deserved, and the delay was on our side. What made it worth the round trips is that the guidance now describes what the code actually does: the auto-created merge=ours line lives in .codebase-memory/.gitattributes, the LFS filter goes in the repo-root file so the two never fight over the same path, and the cadence bullet names the real root cause (committing every Fast-tier rewrite) before anyone reaches for LFS at all. The commit message explaining why the pattern moves to the root is the kind of thing the next person reading git blame will be grateful for.

The 6 GB / ~350-commit data point from your own team is what makes the section land -- abstract warnings about binary bloat are easy to skim past; a number is not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation priority/backlog Valuable contribution, lower scheduling urgency; review when maintainer capacity opens.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants