feat: sync filters UI for issue #1#2
Conversation
Tentative design for the ignore-patterns UI promised in issue #1. Captures preset catalog, first-run sheet, conflict→ignore button, and migration plan for the 3 silent defaults. Open for vitaly74's input before implementation begins.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (3)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (2)ios/**/*.swift📄 CodeRabbit inference engine (README.md)
Files:
⚙️ CodeRabbit configuration file
Files:
**/*⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (1)📓 Common learnings🔇 Additional comments (8)
📝 WalkthroughWalkthroughAdds per‑vault Sync Filters: Go scanner reports heavy-folder detections; Swift models/presets and a bridge wrapper decode them; SyncthingManager APIs manage .stignore and recommendation state; new IgnorePatternsView and recommendation sheet UI; conflict/vault views integrate skip actions; tests, localizations, docs, and version bump included. ChangesSync Filters — Ignore Patterns
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 Pre-merge checks | ✅ 8✅ Passed checks (8 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
Detects heavy directories (.git, .copilot-index, node_modules, .obsidian/cache) in a vault and reports their byte size + file count. Powers the "Found in this vault" section of the Sync Filters UI.
- IgnorePreset catalog with 6 presets (Workspace, Trash, Git, macOS, Copilot, Obsidian app cache); Recommended = Workspace + Trash - DetectedPattern Swift mirror of the Go scanner JSON - SyncthingManager: ignorePatterns / setIgnorePatterns / togglePreset / addIgnorePattern / scanFolderForKnownPatterns + per-folder recommendation-sheet flag in UserDefaults - defaultIgnorePatterns now derived from IgnorePreset.recommended; legacy folders migrate silently (their existing 3 lines remain on disk and the derived state shows Workspace + Trash as active) - 12 Swift unit tests covering catalog and sheet-flag persistence
- IgnorePatternsView: per-vault editor with Recommended, Found in this vault (scan results with size/file-count), Other presets, and Custom patterns sections; toggles write through immediately - SyncFilterRecommendationSheet: first-run sheet shown on first vault-detail open per folder; pre-checks recommended + auto-checks any heavy folders the scan finds - ContentView: NavigationLink in vaultDetailView between Conflicts and Shared With; .sheet(item:) trigger for the recommendation sheet - ConflictDiffView: top-bar Menu with "Always skip on this iPhone" that adds the conflict's exact relPath to the folder's ignore list
All preset labels, descriptions, screen titles, sheet copy, and conflict-button strings translated. German and Chinese mirror the existing tone of the rest of the app.
In typical Obsidian use, the sync folder is the Obsidian root and individual vaults live as immediate subdirectories. The previous scanner only stat'd folder.Path/.git etc., so multi-vault layouts saw an empty "Found in this vault" section even when vaults contained large .git or .copilot-index folders. Now: check the root and one level deep into non-hidden subdirs, aggregating matches per pattern (e.g. ".git in 3 vaults — 127 MB"). Pattern matching itself was already correct — Syncthing auto-expands unanchored patterns (lib/ignore/ignore.go:474-488), so .git, .Trash, and .obsidian/workspace.json already match at any depth without a **/ prefix. No preset or migration changes needed. Tests: 2 new cases (nested vaults aggregate; hidden subdirs are not descended into to avoid double-counting). Full bridge suite passes.
psimaker
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
go/bridge/folderscan.go (1)
121-124: ⚖️ Poor tradeoffSilent error handling may hide permission issues.
The
filepath.Walkerror handler returnsnilfor all errors, silently continuing the traversal. While this prevents scan failures due to unreadable subdirectories, it could mask real issues like filesystem corruption or unexpected permission errors that might warrant user attention.Given that this is a non-critical scan operation for UI hints, the current approach is acceptable. However, consider logging errors or tracking a count of skipped directories in a future iteration if users report incomplete scan results.
🤖 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 `@go/bridge/folderscan.go` around lines 121 - 124, The filepath.Walk callback currently swallows all errors by always returning nil, which can hide permission or filesystem issues; update the anonymous Walk callback used with filepath.Walk(root, ...) to surface or record errors instead of silently ignoring them — for example, call a logger (e.g., log.Printf or the package logger) with context including path and err, or increment a skippedDirs counter when err != nil, then continue the traversal by returning nil so behavior stays non-fatal; reference the anonymous callback used in the filepath.Walk call and the root variable when making these changes.
🤖 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.
Inline comments:
In `@docs/sync-filters-ux.md`:
- Around line 3-7: Update the top-matter and any instances of pre-implementation
language to indicate the spec is implemented: replace the "Status: **draft for
review**" string with something like "Status: implemented" (or "implemented —
see PR/commit link"), update the "Last updated" date if needed, and remove or
reword sentences containing "before writing implementation code" and "before I
start building" (and the similar phrasing at lines 166-174) to reflect that the
design has been implemented and point readers to the implementation/PR where
applicable; search for and edit the exact phrases "draft for review", "before
writing implementation code", and "before I start building" to update them.
In `@ios/VaultSync/Models/IgnorePreset.swift`:
- Around line 60-62: The preset catalog deliberately omits node_modules but
localization strings for "Node modules" remain; either remove those unused
localization keys or add node_modules as a real preset. If you choose removal:
delete the "Node modules" entries from Localizable.strings (en, de, zh-Hans) and
any related NSLocalizedString keys. If you choose to expose it: add a new
IgnorePreset case (e.g., .nodeModules) and implement its display/localization,
include it in IgnorePreset.all, and update any pattern-detection mapping and the
test presetForDetectedPatternReturnsNilForUnknown to expect the new preset
instead of nil.
In `@ios/VaultSync/Services/SyncthingManager.swift`:
- Around line 79-85: defaultIgnorePatterns currently returns
IgnorePreset.recommended.flatMap(\.patterns), which ties silent migration
defaults to future catalog changes; change defaultIgnorePatterns to return a
hard-coded array containing only the three migration-safe silent patterns
[".Trash", ".obsidian/workspace.json", ".obsidian/workspace-mobile.json"] so
existing vaults are not implicitly mutated on startup, and keep any use of
IgnorePreset.recommended for new-folder UI/first-run recommendation flows
separate.
In `@ios/VaultSync/Views/SyncFilterRecommendationSheet.swift`:
- Around line 47-51: The Done button currently calls apply(), marks the sheet
shown and dismisses even if syncthingManager.setIgnorePatterns fails and apply()
only appends enabled patterns (never removes deselected ones). Change apply()
(or create applyChanges()) to return a Result/throwing error and to compute the
final .stignore contents by removing any previously-managed patterns that were
deselected and replacing them with the currently enabled set (use the same
identifiers the manager uses to recognize managed patterns). In the Button
action, await/catch the result of setIgnorePatterns (via the updated apply()),
and only call syncthingManager.markRecommendationSheetShown(folderID:) and
dismiss() on success; on failure surface/log the error to the user instead of
silently dropping it. Ensure you reference and update uses of setIgnorePatterns
and the apply() caller sites to handle the new error/return semantics.
---
Nitpick comments:
In `@go/bridge/folderscan.go`:
- Around line 121-124: The filepath.Walk callback currently swallows all errors
by always returning nil, which can hide permission or filesystem issues; update
the anonymous Walk callback used with filepath.Walk(root, ...) to surface or
record errors instead of silently ignoring them — for example, call a logger
(e.g., log.Printf or the package logger) with context including path and err, or
increment a skippedDirs counter when err != nil, then continue the traversal by
returning nil so behavior stays non-fatal; reference the anonymous callback used
in the filepath.Walk call and the root variable when making these changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0a84bdb8-fbd9-470a-9252-07a71c5786d6
📒 Files selected for processing (16)
docs/sync-filters-ux.mdgo/bridge/folderscan.gogo/bridge/folderscan_test.goios/VaultSync/Models/DetectedPattern.swiftios/VaultSync/Models/IgnorePreset.swiftios/VaultSync/Services/SyncBridgeService.swiftios/VaultSync/Services/SyncthingManager.swiftios/VaultSync/Views/ConflictDiffView.swiftios/VaultSync/Views/ContentView.swiftios/VaultSync/Views/IgnorePatternsView.swiftios/VaultSync/Views/SyncFilterRecommendationSheet.swiftios/VaultSync/de.lproj/Localizable.stringsios/VaultSync/en.lproj/Localizable.stringsios/VaultSync/zh-Hans.lproj/Localizable.stringsios/VaultSyncTests/IgnorePresetTests.swiftios/VaultSyncTests/SyncFiltersTests.swift
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*
⚙️ CodeRabbit configuration file
**/*: VaultSync syncs private Obsidian notes through Syncthing. Treat data loss,
privacy leaks, security regressions, and broken sync behavior as high priority.
Do not nitpick formatting unless it affects maintainability, correctness, or public API clarity.
Flag any accidental logging, telemetry, crash reporting, or network transfer of note contents,
vault paths, filenames with private context, API keys, APNs tokens, relay keys, or security-scoped bookmark data.
Files:
ios/VaultSync/Services/SyncBridgeService.swiftios/VaultSync/Models/DetectedPattern.swiftdocs/sync-filters-ux.mdgo/bridge/folderscan.goios/VaultSyncTests/SyncFiltersTests.swiftios/VaultSync/en.lproj/Localizable.stringsios/VaultSync/Views/ConflictDiffView.swiftios/VaultSync/Views/SyncFilterRecommendationSheet.swiftios/VaultSyncTests/IgnorePresetTests.swiftios/VaultSync/Models/IgnorePreset.swiftios/VaultSync/zh-Hans.lproj/Localizable.stringsgo/bridge/folderscan_test.goios/VaultSync/de.lproj/Localizable.stringsios/VaultSync/Views/IgnorePatternsView.swiftios/VaultSync/Views/ContentView.swiftios/VaultSync/Services/SyncthingManager.swift
ios/**/*.swift
⚙️ CodeRabbit configuration file
ios/**/*.swift: Focus on Swift 6 strict concurrency, Sendable/MainActor correctness, Task cancellation,
retain cycles, memory pressure, SwiftUI observation state, StoreKit/APNs flows, and iOS background execution limits.
Pay special attention to BGAppRefreshTask and BGContinuedProcessingTask behavior, expiration handling,
bounded work, and cleanup when the app is suspended or terminated.
Files:
ios/VaultSync/Services/SyncBridgeService.swiftios/VaultSync/Models/DetectedPattern.swiftios/VaultSyncTests/SyncFiltersTests.swiftios/VaultSync/Views/ConflictDiffView.swiftios/VaultSync/Views/SyncFilterRecommendationSheet.swiftios/VaultSyncTests/IgnorePresetTests.swiftios/VaultSync/Models/IgnorePreset.swiftios/VaultSync/Views/IgnorePatternsView.swiftios/VaultSync/Views/ContentView.swiftios/VaultSync/Services/SyncthingManager.swift
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: Review public documentation for technical accuracy, privacy/security claims, App Store-facing wording,
setup correctness, and consistency with the free app plus optional Cloud Relay subscription model.
Files:
docs/sync-filters-ux.md
go/bridge/**/*.go
⚙️ CodeRabbit configuration file
go/bridge/**/*.go: This code crosses the gomobile Swift-Go boundary. Verify exported signatures use only gomobile-safe primitive types,
preserve the JSON string contract, keep empty-string success conventions intact, and avoid breaking Swift decoding tests.
Review Syncthing lifecycle, locking, error strings, and noassets build assumptions carefully.
Files:
go/bridge/folderscan.gogo/bridge/folderscan_test.go
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-05-09T09:37:05.022Z
Learning: Do not expose raw Syncthing pattern syntax in the UI; instead use plain language descriptions like 'keep this off my iPhone' to help users understand file exclusion
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-05-09T09:37:05.022Z
Learning: The Sync Filters UI should be positioned on the vault detail screen between Conflicts and Shared With sections
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-05-09T09:37:05.022Z
Learning: All filter toggles should write through to `.stignore` immediately without requiring a save button
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-05-09T09:37:05.022Z
Learning: The vault scanner should check both the sync folder root and one level deep to detect heavy folders, and aggregate matches per pattern across multiple vaults
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-05-09T09:37:05.022Z
Learning: Preset catalog must include: workspace state (`.obsidian/workspace.json`, `.obsidian/workspace-mobile.json`) ON by default, trash (`.Trash`) ON by default, git repository (`.git`) OFF, macOS metadata (`.DS_Store`, `._*`) OFF, Copilot index (`.copilot-index`) OFF, and Obsidian app cache (`.obsidian/cache`) OFF
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-05-09T09:37:05.022Z
Learning: The first-run recommendation sheet should be shown only on the first vault detail screen open per vault, with state persisted via a UserDefaults array of folder IDs
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-05-09T09:37:05.022Z
Learning: In ConflictDiffView, add a toolbar menu option to 'Always skip on this iPhone' that adds the conflict's exact relative path (not a smart glob pattern) to the folder's ignore list
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-05-09T09:37:05.022Z
Learning: Use consistent terminology throughout the app: 'Sync Filters' for section title, 'Skip on this iPhone' and 'Always skip on this iPhone' for CTAs, avoiding 'Ignore patterns', 'Exclusions', and 'Filter rules'
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-05-09T09:37:05.022Z
Learning: Ship all new strings in English, German, and Simplified Chinese locales
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-05-09T09:37:05.022Z
Learning: Pattern matching should handle unanchored patterns (without leading slash) transparently, allowing patterns like `.git` to match at any depth without requiring `**/` prefix
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-05-09T09:37:05.022Z
Learning: For existing vaults during migration, keep the 3 silent default patterns (`.Trash`, `.obsidian/workspace.json`, `.obsidian/workspace-mobile.json`) on disk untouched and automatically show them as ON in the derived state without disk changes
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-05-09T09:37:05.022Z
Learning: For new vaults added after this feature lands, silently apply Recommended presets to prevent immediate sync conflicts, and show the first-run sheet with Recommended already checked and scan results pre-checked
🪛 LanguageTool
docs/sync-filters-ux.md
[uncategorized] ~80-~80: The operating system from Apple is written “macOS”.
Context: ...t| OFF (auto-on if scan finds it) | |macos| macOS metadata |.DS_Store, ._*` ...
(MAC_OS)
[style] ~143-~143: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... - No migration sheet. No disk changes. No surprise. For new vaults added after t...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~173-~173: The present participle “” normally requires “is”.
Context: ...allback. No strong opinion either way. Anything else missing? The whole point of doing this ...
(ANYONE_ELSE_WHO_IS_VBG)
🪛 markdownlint-cli2 (0.22.1)
docs/sync-filters-ux.md
[warning] 20-20: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 33-33: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 91-91: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 121-121: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (10)
ios/VaultSync/zh-Hans.lproj/Localizable.strings (1)
462-496: LGTM! Clean localization implementation.The Simplified Chinese strings correctly cover all Sync Filters UI elements, preset descriptions, and skip-flow messaging. The translations align with the feature requirements and maintain consistent terminology throughout. Based on learnings, the terminology correctly avoids exposing raw Syncthing pattern syntax and uses plain language descriptions.
go/bridge/folderscan.go (2)
30-38: LGTM! Clean preset catalog definition.The
heavyDirCandidatesslice correctly defines the known patterns to detect, matching the preset catalog in the Swift layer. The pattern/label pairs align with theIgnorePresetdefinitions and the localization strings.
118-132: ⚡ Quick winRemove the symlink concern—
filepath.Walkdoes not follow symlinks.Go's
filepath.Walkexplicitly does not follow symbolic links (it usesos.Lstatinternally), so the vault cannot be traversed outside its boundaries through symlinks in.git,.obsidian, or subdirectories. The code is safe as written.ios/VaultSync/Services/SyncBridgeService.swift (1)
179-183: LGTM! Correct bridge wrapper implementation.The new
scanFolderForKnownPatternsmethod correctly follows the established bridge service pattern, calling the Go function and returning the JSON string result. The signature matches the Go bridge contract and the comment clearly documents the return format.ios/VaultSync/Models/DetectedPattern.swift (1)
1-16: LGTM! Clean model implementation.The
DetectedPatternandDetectedScanmodels correctly mirror the Go bridge JSON structure. TheSendableconformance is appropriate since all properties are value types, making these safe for concurrent use across actors. Usingpatternas theidis correct since patterns are unique per the Go scanner's aggregation logic.ios/VaultSync/Models/IgnorePreset.swift (1)
14-62: LGTM! Preset catalog correctly implements requirements.The preset definitions align perfectly with the learnings:
workspaceandtrashare inrecommended(auto-applied to new vaults)git,macos,copilot, andobsidianCacheare available but not recommended by default- Pattern strings match the Go scanner's
heavyDirCandidatesfor proper detection mappingBased on learnings, the preset catalog must include workspace state, trash, git repository, macOS metadata, Copilot index, and Obsidian cache with the specified default states.
ios/VaultSyncTests/IgnorePresetTests.swift (1)
1-47: LGTM! Comprehensive preset validation.The test suite thoroughly validates the preset catalog against requirements:
- Recommended presets match learnings (workspace + trash only)
- All presets have non-empty patterns (prevents UI bugs)
- Preset IDs are unique (prevents catalog conflicts)
- Detection mapping works correctly for supported patterns
The test at line 43-46 confirms that
node_modulesintentionally has no preset mapping, which aligns with my earlier observation.ios/VaultSync/en.lproj/Localizable.strings (1)
463-496: LGTM! Consistent terminology and complete coverage.The English localization correctly implements the terminology guidelines from learnings:
- Section title: "Sync Filters" (not "Ignore patterns")
- Actions: "Skip", "Always skip on this iPhone" (user-friendly language)
- Preset descriptions use plain language (e.g., "rarely useful on iPhone") rather than technical pattern syntax
All new UI strings, preset labels, and error messages are covered. Based on learnings, use consistent terminology throughout: 'Sync Filters' for section title, 'Skip on this iPhone' and 'Always skip on this iPhone' for CTAs.
ios/VaultSync/de.lproj/Localizable.strings (1)
462-495: LGTM! Complete German localization.The German translations correctly cover all Sync Filters UI elements, maintaining consistent terminology:
- "Sync-Filter" for the feature name
- "Überspringen" / "Auf diesem iPhone immer überspringen" for skip actions
- Clear, user-friendly descriptions for each preset
The translations align with the English source and follow the same structure as the other locale files. Based on learnings, ship all new strings in English, German, and Simplified Chinese locales.
go/bridge/folderscan_test.go (1)
71-73: ⚡ Quick winUse structural JSON checks instead of raw string equality in empty-result tests.
Comparing to
{"detected":[]}is brittle and can fail on non-semantic JSON formatting/order changes.[ suggest_recommended_refactor ]
Refactor pattern
- raw := ScanFolderForKnownPatterns(folderID) - if raw != `{"detected":[]}` { - t.Errorf("got %q, want empty detected list", raw) - } + raw := ScanFolderForKnownPatterns(folderID) + var result ScanResult + if err := json.Unmarshal([]byte(raw), &result); err != nil { + t.Fatalf("unmarshal failed: %v (raw=%s)", err, raw) + } + if len(result.Detected) != 0 { + t.Errorf("expected empty detected list, got %d (raw=%s)", len(result.Detected), raw) + }Also applies to: 84-86, 256-258
| static let all: [IgnorePreset] = [ | ||
| .workspace, .trash, .git, .macos, .copilot, .obsidianCache, | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if nodeModules preset is defined elsewhere or if the design changed
# Search for any nodeModules preset definition
rg -n 'nodeModules.*IgnorePreset' ios/VaultSync/
# Check if the "Node modules" localization string is actually used in the UI
rg -n '"Node modules"' ios/VaultSync/Views/
# Look for related comments or documentation explaining the decision
rg -n -C2 'node_modules.*preset|preset.*node_modules' ios/VaultSync/Repository: psimaker/vaultsync
Length of output: 44
🏁 Script executed:
# First, let's see what presets are actually defined in IgnorePreset.swift
cat -n ios/VaultSync/Models/IgnorePreset.swift | head -100Repository: psimaker/vaultsync
Length of output: 2747
🏁 Script executed:
# Check if the test file exists and read around line 44
if [ -f ios/VaultSync/Models/IgnorePresetTests.swift ]; then
cat -n ios/VaultSync/Models/IgnorePresetTests.swift | sed -n '35,55p'
fiRepository: psimaker/vaultsync
Length of output: 44
🏁 Script executed:
# Find localization files and search for "Node modules"
find ios -name "*.strings" -o -name "*.stringsdict" -o -name "*Localizable*" | head -20Repository: psimaker/vaultsync
Length of output: 622
🏁 Script executed:
# Search more broadly for node_modules mentions
rg -i 'node_modules' ios/VaultSync/ --type swift | head -20Repository: psimaker/vaultsync
Length of output: 44
🏁 Script executed:
# Check if Go code mentions node_modules
rg -i 'node_modules' . --type go | head -20Repository: psimaker/vaultsync
Length of output: 334
🏁 Script executed:
# Find the test file
find ios -name "*IgnorePresetTests*" -o -name "*IgnorePresetTest*"Repository: psimaker/vaultsync
Length of output: 105
🏁 Script executed:
# Search for the test file in different locations
find ios -type f -name "*Test*.swift" | rg -i presetRepository: psimaker/vaultsync
Length of output: 105
🏁 Script executed:
# Look for any Swift test files
find ios -type f -name "*Tests.swift" | head -20Repository: psimaker/vaultsync
Length of output: 441
🏁 Script executed:
# Search the localization strings for "Node modules"
rg -n "Node modules" ios/VaultSync/Repository: psimaker/vaultsync
Length of output: 301
🏁 Script executed:
# Check the entire content of the English localization file to see if "Node modules" is present
cat ios/VaultSync/en.lproj/Localizable.strings | grep -i "node"Repository: psimaker/vaultsync
Length of output: 95
🏁 Script executed:
# Read the test file to verify the claim about line 44
cat -n ios/VaultSyncTests/IgnorePresetTests.swiftRepository: psimaker/vaultsync
Length of output: 1940
node_modules is intentionally excluded from presets despite Go detection.
The Go scanner detects node_modules and the codebase includes localization strings for "Node modules" (en, de, zh-Hans). However, the Swift preset catalog deliberately omits it—confirmed by the test presetForDetectedPatternReturnsNilForUnknown at line 44 of IgnorePresetTests.swift, which explicitly expects nil when checking node_modules.
This means users will see node_modules flagged when it's detected in the vault, but cannot toggle it as a preset and must add it manually as a custom pattern instead.
If node_modules will never be exposed as a preset, remove the unused localization strings to avoid confusion. If it should become a preset in the future, add it to the catalog.
🤖 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 `@ios/VaultSync/Models/IgnorePreset.swift` around lines 60 - 62, The preset
catalog deliberately omits node_modules but localization strings for "Node
modules" remain; either remove those unused localization keys or add
node_modules as a real preset. If you choose removal: delete the "Node modules"
entries from Localizable.strings (en, de, zh-Hans) and any related
NSLocalizedString keys. If you choose to expose it: add a new IgnorePreset case
(e.g., .nodeModules) and implement its display/localization, include it in
IgnorePreset.all, and update any pattern-detection mapping and the test
presetForDetectedPatternReturnsNilForUnknown to expect the new preset instead of
nil.
- Version 1.1.0 → 1.2.0 (minor bump for the new Sync Filters feature) - Build 20 → 21 (App + Widget targets) - CHANGELOG: new [1.2.0] entry covering Sync Filters, vault scan, conflict-skip, first-run sheet, custom patterns, localization - README: What's New block replaced with v1.2.0 highlights
Three follow-ups from the automated review: 1. SyncthingManager.defaultIgnorePatterns is now hard-coded to the historical migration-safe set (.Trash + the two workspace.json variants) instead of being derived from IgnorePreset.recommended. Future changes to the catalog can no longer silently mutate .stignore on existing vaults during the startup auto-merge. The recommendation sheet uses IgnorePreset.recommended directly. 2. SyncFilterRecommendationSheet.apply() now properly handles deselect and surfaces errors. It computes the final pattern list by removing all sheet-managed entries (presets + detected items) from the existing .stignore first, then re-adds only the currently enabled ones. Custom patterns the user added previously are preserved. Done returns the SyncUserError from setIgnorePatterns; on failure an alert is shown and the sheet is neither marked-shown nor dismissed, so the user can retry. 3. Spec doc status updated from "draft for review" to "implemented", and the open-questions section reframed as "future considerations" for follow-up reference. CodeRabbit's nitpick about node_modules localization being unused is incorrect (the strings are surfaced via the Go scanner's item.label when node_modules is detected), so it stays as-is.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@docs/sync-filters-ux.md`:
- Around line 20-27: The markdown fenced code blocks shown (the block starting
with "Vault (name, path)" and the blocks showing the Sync
Filters UI like "┌─ Sync Filters — \"My Vault\" ─────────────┐", "┌─ Sync
Filters ──────────────────────────┐", and the menu block starting with "⋯ menu")
are missing language identifiers which triggers MD040; update each opening fence
to use a language tag (use "text") so they become ```text ... ``` ensuring all
four affected fenced blocks include the language identifier.
In `@ios/VaultSync/Services/SyncthingManager.swift`:
- Around line 1491-1497: The ignorePatterns(folderID:) helper currently swallows
JSON decode failures by returning an empty list; change it to surface read/parse
errors instead (e.g., make ignorePatterns throw or return Result<[String],
Error> or an optional) so callers can abort write flows when the current
.stignore cannot be parsed; update callers (e.g., any code invoking
ignorePatterns and subsequent read-modify-write operations) to check the
error/optional and refuse to perform writes when
SyncBridgeService.getFolderIgnores(folderID:) output cannot be decoded, ensuring
existing ignore rules aren’t silently overwritten.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 69d787a4-6e71-4ae9-945c-6b81829571d0
📒 Files selected for processing (6)
docs/sync-filters-ux.mdios/VaultSync/Services/SyncthingManager.swiftios/VaultSync/Views/SyncFilterRecommendationSheet.swiftios/VaultSync/de.lproj/Localizable.stringsios/VaultSync/en.lproj/Localizable.stringsios/VaultSync/zh-Hans.lproj/Localizable.strings
✅ Files skipped from review due to trivial changes (3)
- ios/VaultSync/en.lproj/Localizable.strings
- ios/VaultSync/de.lproj/Localizable.strings
- ios/VaultSync/zh-Hans.lproj/Localizable.strings
📜 Review details
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build & Test
🧰 Additional context used
📓 Path-based instructions (3)
ios/**/*.swift
📄 CodeRabbit inference engine (README.md)
Follow Swift API Design Guidelines
Use Swift strict concurrency where applicable
Files:
ios/VaultSync/Services/SyncthingManager.swiftios/VaultSync/Views/SyncFilterRecommendationSheet.swift
⚙️ CodeRabbit configuration file
ios/**/*.swift: Focus on Swift 6 strict concurrency, Sendable/MainActor correctness, Task cancellation,
retain cycles, memory pressure, SwiftUI observation state, StoreKit/APNs flows, and iOS background execution limits.
Pay special attention to BGAppRefreshTask and BGContinuedProcessingTask behavior, expiration handling,
bounded work, and cleanup when the app is suspended or terminated.
Files:
ios/VaultSync/Services/SyncthingManager.swiftios/VaultSync/Views/SyncFilterRecommendationSheet.swift
**/*
⚙️ CodeRabbit configuration file
**/*: VaultSync syncs private Obsidian notes through Syncthing. Treat data loss,
privacy leaks, security regressions, and broken sync behavior as high priority.
Do not nitpick formatting unless it affects maintainability, correctness, or public API clarity.
Flag any accidental logging, telemetry, crash reporting, or network transfer of note contents,
vault paths, filenames with private context, API keys, APNs tokens, relay keys, or security-scoped bookmark data.
Files:
ios/VaultSync/Services/SyncthingManager.swiftios/VaultSync/Views/SyncFilterRecommendationSheet.swiftdocs/sync-filters-ux.md
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: Review public documentation for technical accuracy, privacy/security claims, App Store-facing wording,
setup correctness, and consistency with the free app plus optional Cloud Relay subscription model.
Files:
docs/sync-filters-ux.md
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-05-09T09:53:00.268Z
Learning: Use Conventional Commits for commit messages
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-05-09T09:53:00.268Z
Learning: Provide clear PR (pull request) descriptions
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-05-09T09:53:15.932Z
Learning: For users updating from a previous build, keep existing silent default patterns (`.Trash`, `.obsidian/workspace.json`, `.obsidian/workspace-mobile.json`) on disk untouched without migration sheet or surprise disk changes
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-05-09T09:53:15.932Z
Learning: Do not include 'Plugin caches' as a single umbrella preset; instead use specific narrow presets per plugin since different plugins store caches in different locations
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-05-09T09:53:15.932Z
Learning: Do not include general `.gitignore` or `.gitattributes` in presets, as these are typically small text files that users want to sync for configuration purposes
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-05-09T09:53:15.932Z
Learning: The Sync Filters feature should be placed per-vault on the vault detail screen as a link between the Conflicts and Shared With sections
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-05-09T09:53:15.932Z
Learning: Pattern matching should handle multi-vault setups transparently by automatically expanding unanchored patterns (without leading `/`) to match at any depth, so `.git` covers both root-level and nested vault `.git` directories
🪛 LanguageTool
docs/sync-filters-ux.md
[uncategorized] ~80-~80: The operating system from Apple is written “macOS”.
Context: ...t| OFF (auto-on if scan finds it) | |macos| macOS metadata |.DS_Store, ._*` ...
(MAC_OS)
[style] ~143-~143: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... - No migration sheet. No disk changes. No surprise. For new vaults added after t...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.22.1)
docs/sync-filters-ux.md
[warning] 20-20: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 33-33: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 91-91: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 121-121: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (2)
ios/VaultSync/Services/SyncthingManager.swift (1)
79-89: Migration-safe default ignore patterns are correctly decoupled from evolving presets.Nice fix: the silent auto-apply set is now explicitly pinned, so future preset catalog changes won’t silently change existing vault
.stignorefiles.ios/VaultSync/Views/SyncFilterRecommendationSheet.swift (1)
48-55: Apply flow now correctly handles deselection, preserves custom patterns, and surfaces save failures.This addresses the risky path well: success-only dismissal plus managed-pattern replacement is the right behavior for reliable per-vault filter updates.
Also applies to: 60-64, 145-160
| ``` | ||
| Vault (name, path) | ||
| Sync Status (state, completion, errors) | ||
| Conflicts (when present) | ||
| ► Sync Filters ← new | ||
| Shared With (devices) | ||
| Rescan Vault | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced code blocks (MD040).
At Line 20, Line 33, Line 91, and Line 121, the fenced blocks are missing a language tag, which can keep markdownlint warnings active in CI.
🛠️ Minimal lint-fix diff
-```
+```text
Vault (name, path)
...
-```
+```
-```
+```text
┌─ Sync Filters — "My Vault" ─────────────┐
...
-```
+```
-```
+```text
┌─ Sync Filters ──────────────────────────┐
...
-```
+```
-```
+```text
⋯ menu
└─ Always skip on this iPhone
-```
+```Also applies to: 33-61, 91-107, 121-124
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 20-20: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@docs/sync-filters-ux.md` around lines 20 - 27, The markdown fenced code
blocks shown (the block starting with "Vault (name, path)"
and the blocks showing the Sync Filters UI like "┌─ Sync Filters — \"My Vault\"
─────────────┐", "┌─ Sync Filters ──────────────────────────┐", and the menu
block starting with "⋯ menu") are missing language identifiers which triggers
MD040; update each opening fence to use a language tag (use "text") so they
become ```text ... ``` ensuring all four affected fenced blocks include the
language identifier.
Read-modify-write flows in SyncthingManager (togglePreset, addIgnorePattern, and the new applyRecommendedFilters used by SyncFilterRecommendationSheet) now abort with a SyncUserError when the bridge response cannot be decoded, instead of treating decode failure as an empty list. A malformed read could otherwise have silently overwritten the user's existing .stignore on the next write — exactly the data-loss class CodeRabbit flagged. - New private readIgnorePatternsOrNil() helper distinguishes empty list from decode failure. - Public ignorePatterns() keeps returning [String] for display (with \`?? []\`), so view code that just renders a list isn't forced to handle the nil case. - New applyRecommendedFilters(...) consolidates the recommendation sheet's "preserve custom patterns + deselect-aware merge + safe read" logic next to the rest of the filter API. - L10n: en/de/zh-Hans copy for the new error message.
1 participant
Closes #1.
Adds a per-vault Sync Filters UI on top of the existing
GetFolderIgnores/SetFolderIgnoresbridge functions. Users can toggle preset ignore-pattern groups in plain language, edit custom patterns, see a vault scan that surfaces actual byte sizes of heavy folders, and skip conflicting files in one tap from the conflict resolver.What's in
Bridge
ScanFolderForKnownPatterns(Go) — checks the sync folder root and one level deep into each non-hidden subdirectory, aggregates matches per pattern (designed for "Obsidian root + multiple vaults as subdirs" layouts). Detects.git,.copilot-index,node_modules,.obsidian/cache. 7 unit tests including nested-vault and hidden-subdir cases.Models & service
IgnorePresetcatalog: Workspace state, Trash, Git repository, macOS metadata, Copilot index, Obsidian app cache. Recommended set (Workspace + Trash) is silently applied to new folders.SyncthingManagerfilter API:ignorePatterns,setIgnorePatterns,togglePreset,addIgnorePattern,scanFolderForKnownPatterns, plus per-folder UserDefaults flag for the first-run sheet.UI
IgnorePatternsView(Recommended / Found in this vault / Other presets / Custom patterns / footer link to Syncthing pattern docs).SyncFilterRecommendationSheetshown once per vault on first detail-view open; pre-checks Recommended + auto-checks anything heavy the scan found.ContentView:NavigationLinkbetween Conflicts and Shared With sections.ConflictDiffView: top-bar menu with Always skip on this iPhone (adds the conflict's exact relative path to the folder's ignore list).Localization
en, de, zh-Hans for all new strings.
Pattern semantics — verified
Confirmed via
lib/ignore/ignore.go:474-488: Syncthing auto-expands every unanchored pattern to also match at any depth. So.git,.obsidian/workspace.json,.Trashetc. correctly match in deeply nested vault subdirectories without needing a**/prefix.Spec
Full UX spec lives in
docs/sync-filters-ux.md(mockups, preset catalog, migration plan, multi-vault behavior).Tests
Sync Filters UI for per-vault ignore patterns
Adds a user-facing "Sync Filters" UI to manage
.stignorepatterns per vault, allowing users to exclude heavy or irrelevant folders (.git,node_modules,.obsidian/cache, etc.) from syncing without manually editing configuration files.User-visible features
Sync Filters screen: New "Sync Filters" link in vault detail view. Displays five sections: Recommended presets (Workspace state, Trash), detected patterns found by folder scanning with sizes/file counts, other available presets (Git, macOS metadata, Copilot index, Obsidian cache), custom pattern editor (add/delete), and link to Syncthing docs.
First-run recommendation sheet: Shown once per vault (tracked via UserDefaults). Auto-detects heavy folders and pre-checks recommended presets; users can toggle selections and apply or skip.
Conflict resolution action: New "Always skip on this iPhone" toolbar action in
ConflictDiffViewthat adds a conflict path to the folder's ignore list with undoable confirmation.Smart defaults: New vaults silently apply Workspace + Trash presets by default. Existing vaults preserve their current
.stignorewithout overwriting on app update.Localization: All new UI strings translated to English, German, and Simplified Chinese.
Technical implementation
Go bridge: New
ScanFolderForKnownPatterns()function scans vault root and one directory level deep (skipping hidden dirs), totals file sizes and counts, and returns detected patterns as JSON.Pattern semantics: Uses Syncthing-compatible patterns that auto-expand unanchored patterns to match at any depth, ensuring consistency with Syncthing's native behavior.
Safe read-modify-write:
.stignorereads are guarded against malformed files; writes abort withSyncUserErroron decode failure to prevent silent overwrites.Preset management:
IgnorePresetcatalog maps preset IDs to pattern groups; toggle operations preserve user-added custom patterns and handle deselection-aware merges.Testing
.gitdetection, empty vaults, multiple candidates, nested vaults, hidden directories, and empty directories. Full bridge regression suite (~40s).Privacy/security & background execution
No background execution changes. Folder scanning happens only on-demand when the Sync Filters sheet is presented. No privacy impact; scanning is local to the vault and detection results are stored only in memory or UserDefaults for the "shown" flag.