fix: address PR 185 review follow-ups#186
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds ContentHash field tracking to relayfile File objects, integrating it throughout write operations, tree listings, event generation, and startup state initialization. The PR also adds logging to mount sync bootstrap path and tightens OpenAPI validation. ChangesContentHash Field and Integration
Mount Sync Logging
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…cile Persists ContentHash on the File struct so reconcile no longer rehashes every read. Backfills on disk load for older snapshots. Prerequisite for bootstrap skip-on-hash (#183). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Relayfile Eval ReviewRun: Passed: 4 | Needs human: 0 | Reviewable: 0 | Missing output: 0 | Failed: 0 | Skipped: 0 Human Review CasesNo reviewable human-review cases captured Relayfile output. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/relayfile/store.go (1)
3904-3937:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPersist the migration after backfilling missing
ContentHashvalues.Lines 3910-3937 only update the in-memory snapshot. If the process restarts before any later write/save, the backend still contains empty
contentHashfields, so every legacy file and fork overlay gets rehashed again on the next boot and the migration never becomes durable.Proposed fix
func (s *Store) loadFromDisk() error { if s.stateBackend == nil { return nil } snapshot, err := s.stateBackend.Load() @@ if snapshot == nil { return nil } + dirty := false if snapshot.Workspaces != nil { s.workspaces = snapshot.Workspaces for _, ws := range s.workspaces { if ws.Files == nil { ws.Files = map[string]File{} } for path, file := range ws.Files { - ws.Files[path] = ensureStoredContentHash(file) + updated := ensureStoredContentHash(file) + if updated.ContentHash != file.ContentHash { + dirty = true + } + ws.Files[path] = updated } @@ if snapshot.Forks != nil { s.forks = snapshot.Forks for _, fork := range s.forks { if fork.Overlay == nil { fork.Overlay = map[string]ForkOverlayEntry{} } for path, entry := range fork.Overlay { if entry.File == nil { continue } - file := ensureStoredContentHash(*entry.File) + original := *entry.File + file := ensureStoredContentHash(original) + if file.ContentHash != original.ContentHash { + dirty = true + } entry.File = &file fork.Overlay[path] = entry } } } @@ s.revCounter = snapshot.RevCounter s.opCounter = snapshot.OpCounter s.eventCounter = snapshot.EventCounter + if dirty { + return s.saveLocked() + } return nil }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/relayfile/store.go` around lines 3904 - 3937, After backfilling missing ContentHash values via ensureStoredContentHash for s.workspaces and s.forks, persist the updated in-memory snapshot to the backend so the migration is durable: invoke the store's snapshot persistence method (e.g., call s.saveSnapshot(...) or implement and call s.persistSnapshot(...) immediately after the loops that update s.workspaces and fork.Overlay), handle and log any error and retry/fail fast as appropriate so the updated contentHash values are written to the backend rather than only kept in memory.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@internal/relayfile/store.go`:
- Around line 3904-3937: After backfilling missing ContentHash values via
ensureStoredContentHash for s.workspaces and s.forks, persist the updated
in-memory snapshot to the backend so the migration is durable: invoke the
store's snapshot persistence method (e.g., call s.saveSnapshot(...) or implement
and call s.persistSnapshot(...) immediately after the loops that update
s.workspaces and fork.Overlay), handle and log any error and retry/fail fast as
appropriate so the updated contentHash values are written to the backend rather
than only kept in memory.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 35d5de67-20b2-4726-8124-b31247644973
📒 Files selected for processing (4)
internal/mountsync/syncer.gointernal/relayfile/store.gointernal/relayfile/store_test.goopenapi/relayfile-v1.openapi.yaml
1 participant
Summary
Validation