Skip to content

feat: sharded notes refs for reduced push contention#818

Draft
svarlamov wants to merge 9 commits intofix/notes-merge-fallback-and-retryfrom
feat/sharded-notes-refs
Draft

feat: sharded notes refs for reduced push contention#818
svarlamov wants to merge 9 commits intofix/notes-merge-fallback-and-retryfrom
feat/sharded-notes-refs

Conversation

@svarlamov
Copy link
Copy Markdown
Member

@svarlamov svarlamov commented Mar 27, 2026

Summary

  • Introduces opt-in sharded notes architecture splitting refs/notes/ai into 256 independent shard refs (refs/notes/ai-s/XX), keyed by first 2 hex chars of annotated commit SHA
  • Dual-write (legacy + shard), union-read (shard first, legacy fallback), shard-aware fetch/push with wildcard refspecs
  • Fully backward-compatible: old clients continue reading/writing the legacy ref; gated behind sharded_notes feature flag (default off)

Test plan

  • test_sharded_notes_dual_write_on_push — verifies both legacy and shard refs reach upstream with identical content
  • test_sharded_notes_union_read_legacy_fallback — verifies blame works via legacy fallback when no shard refs exist
  • test_sharded_notes_mixed_team_interop — verifies non-sharded clone can read notes pushed by sharded clone
  • All existing unit and integration tests pass (no regressions)

🤖 Generated with Claude Code


Open with Devin

@svarlamov svarlamov marked this pull request as ready for review March 28, 2026 00:28
Copy link
Copy Markdown
Contributor

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

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

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

View in Devin Review to see 5 additional findings.

Open in Devin Review

@svarlamov svarlamov force-pushed the feat/sharded-notes-refs branch from 2a4ec88 to cffeccb Compare March 28, 2026 01:32
@svarlamov svarlamov marked this pull request as draft March 28, 2026 01:32
@svarlamov svarlamov changed the base branch from main to fix/notes-merge-fallback-and-retry March 28, 2026 01:33
Copy link
Copy Markdown
Contributor

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

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

View 10 additional findings in Devin Review.

Open in Devin Review

Comment on lines +357 to +360
match exec_git(&shard_fetch) {
Ok(_) => {
shards_found = true;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 False positive NotesExistence::Found when shard wildcard fetch matches nothing

fetch_authorship_notes sets shards_found = true whenever the shard wildcard git fetch exits successfully (line 359). However, as the code's own comment on line 362 notes, "wildcard refspecs that match nothing don't error" — git fetch origin +refs/notes/ai-s/*:…* returns exit code 0 even when no shard refs exist on the remote. This means when sharded_notes is enabled and the remote has no notes at all (neither legacy nor shard), shards_found is true, the !legacy_found && !shards_found guard at line 368 is bypassed, and the function returns NotesExistence::Found instead of the correct NotesExistence::NotFound. This surfaces as an incorrect "notes_existence": "found" in the JSON response of the fetch-authorship-notes command.

Prompt for agents
In src/git/sync_authorship.rs, the `fetch_authorship_notes` function around lines 357-365, `shards_found` is set to `true` on any successful shard fetch, but a wildcard fetch that matches no remote refs also succeeds (exit 0). To fix this, after the shard fetch succeeds, verify that at least one shard tracking ref was actually created. For example, call `resolve_shard_ref_pairs(repository, remote_name)` (which is already defined in this file) and check if the result is non-empty before setting `shards_found = true`. Alternatively, check if any refs matching the `shard_tracking_ref_prefix(remote_name)` pattern exist locally via a quick `for-each-ref` call.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

svarlamov and others added 8 commits March 28, 2026 03:38
Introduces an opt-in sharded notes architecture that splits refs/notes/ai
into 256 independent shard refs (refs/notes/ai-s/XX), keyed by the first
two hex chars of the annotated commit SHA. This dramatically reduces push
contention on large monorepos by narrowing the conflict window from all
notes to ~1/256th.

Design:
- Dual-write: always writes to legacy refs/notes/ai AND shard ref
- Union-read: checks shard ref first, falls back to legacy
- Sync: fetches/pushes both legacy and shard wildcard refspecs
- Backward-compatible: old clients read/write legacy ref as before
- Gated behind `sharded_notes` feature flag (default off)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace per-shard git-notes-merge loop with a batched approach:
- 2 git processes to discover+resolve all shard pairs (for-each-ref + cat-file --batch-check)
- Skip unchanged shards (tracking OID == local OID) at zero cost
- Batch all "copy" operations into one update-ref --stdin call
- Only run git notes merge for genuinely diverged shards (typically 1-2 per fetch)

Previously: up to 3N git processes (N = number of shards: ref_exists × 2 + merge)
Now: 2 + 1 (batch copy) + M (only diverged merges, typically 1-2)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Returns fallback "00" shard instead of panicking if SHA is < 2 chars.
Includes debug_assert for development visibility.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When sharding is enabled, fetch legacy and shard refs as separate git
fetch calls. This prevents a missing legacy ref on the remote from
blocking shard fetching (and vice versa), which matters for forward
compatibility if we ever deprecate the legacy ref.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@svarlamov svarlamov force-pushed the feat/sharded-notes-refs branch from cffeccb to 3d50a91 Compare March 28, 2026 03:38
@svarlamov svarlamov closed this Mar 28, 2026
@svarlamov svarlamov reopened this Mar 28, 2026
@svarlamov svarlamov marked this pull request as ready for review March 28, 2026 06:24
@svarlamov svarlamov marked this pull request as draft March 28, 2026 06:25
@svarlamov svarlamov marked this pull request as ready for review March 28, 2026 06:26
@svarlamov svarlamov changed the base branch from fix/notes-merge-fallback-and-retry to main March 28, 2026 06:27
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@svarlamov svarlamov force-pushed the feat/sharded-notes-refs branch from 5c5f6c8 to 9faf183 Compare March 28, 2026 06:28
@svarlamov svarlamov changed the base branch from main to fix/notes-merge-fallback-and-retry March 28, 2026 06:28
@svarlamov svarlamov marked this pull request as draft March 28, 2026 06:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant