Cloud pulls make 10 bridge calls per file instead of 16

docs/cloud-pull-bridge-calls-2026-10-26.md

@@ -0,0 +1,136 @@
+# Cloud pull: fewer bridge calls per file, 26 October 2026
+
+Branch `perf/cloud-pull-bridge-calls`, cut from `origin/main` at 3609c83
+(1.1.46). Changes: `src/bridge/cloud-sync-repository.ts`,
+`src/bridge/native-fs.ts`, `src/bridge/mobile-cloud-sync.ts`, and tests. No
+native code, no Capacitor config and no app-core change.
+
+## Why
+
+A Discord report (yeminn) had phones sitting in "Syncing..." through a long
+first Cloud sync. Measured on an API 37 emulator, a pull costs the same per
+file at 600, 2,500 and 5,000 notes, and that cost is almost all Capacitor
+bridge round trips: about 16 per pulled file at about 1 ms each, 10.3 s of a
+10.7 s 600-note sync. So the lever for big first syncs is calls per file.
+
+## What changed
+
+Five calls per pulled file were redundant. Each is gone only where it could
+not have guarded a file.
+
+| Call | Where | Why it cannot lose a file |
+|---|---|---|
+| `deleteFile(backup)` on every write | `NativeCloudSyncContent.write` | A backup exists only when an original was moved aside (`before`). A new file never makes one, so the delete always failed into its `.catch`. Replacing a file still deletes its backup. |
+| `stat(temporary)` after publishing | same | A rename that resolved moved the temporary onto the path. Capacitor's local rename either `renameTo`s or copies and then deletes the source, rejecting if that delete fails; SAF moves the document. Every path that did not publish still checks and removes the temporary. |
+| `readdir(.zennotes/sync)` on every write | `assertReady` | Recovery files are now relisted only when one may have appeared: after a write that failed, after a backup that would not delete, and after any staged publish (which keeps its own backups). The first write, every scan and every staged download still list first. A completed write that removed its backup leaves nothing to find. |
+| `Filesystem.getUri` before every `inspect` (local tier) | `NativeFs.readForSync` | `getUri` names a file by the root's URI plus the path with each segment escaped exactly as `encodeURIComponent` does. The root is resolved once at open, and the shortcut is used only for the same `Directory` it was resolved in; otherwise, and before open, it asks `getUri` as before. Checked on the emulator for 13 awkward names: identical strings and identical bytes. SAF vaults still stat for their document URI. |
+| `mkdir(parent)` before every rename | `hostVault` in `mobile-cloud-sync.ts` | Each sync operation remembers the folders it has made. A rename into a folder that went missing fails before it changes anything (Capacitor checks the parent first; SAF makes it), and the cache forgets that folder so the next rename makes it again. The next sync starts with an empty cache. |
+
+Kept as they were: the pre-publish `stat(path)` (a new file must still be
+absent), both verifies (the temporary before publishing, the path after),
+the original's verify before it moves aside, and every rollback.
+
+One stricter behavior: a staged publish (large or referenced files) whose
+rollback copy would not delete now stops the next write in the same run
+with "Cloud sync needs recovery". Before, the run went on and the next sync's
+scan stopped at the same file, so the user lands in the same place one step
+sooner, the same way an inline write's leftover backup already behaved.
+
+## Results
+
+API 37 emulator (AVD `yeminn-conflict`, `-gpu host -cores 2 -memory 2048`),
+fake Cloud fixture on the Mac, small inline notes in four folders. Fresh app
+each run (`pm clear`, onboard, sign in, link), bridge calls counted by
+wrapping `Capacitor.nativePromise`. Before and after APKs were built from the
+same worktree, differing only in the three source files, and run
+alternately. An earlier after build, which joined the root and the path with
+a doubled slash (same bytes, same call count), ran 600 notes in 6.84 s and is
+left out of the averages.
+
+| Run | Before | After | Change |
+|---|---|---|---|
+| First sync, 600 notes | 10.13 s, 59.3 notes/s (3 runs) | 6.65 s, 90.4 notes/s (2 runs) | 34% less time |
+| Change feed, 1,000 notes added (2 runs each) | 16.44 s, 60.8 notes/s | 11.12 s, 90.0 notes/s | 32% less time |
+| First sync, 2,500 notes | 40.70 s, 61.5 notes/s (2 runs) | 28.68 s, 87.2 notes/s (1 run) | 30% less time |
+
+Bridge calls per pulled file, first sync of 600:
+
+| Call | Before | After |
+|---|---|---|
+| `Filesystem.stat` | 5.02 | 4.02 |
+| `ZenDirectUpload.inspect` | 3.00 | 3.00 |
+| `Filesystem.getUri` | 2.00 | 0 |
+| `Filesystem.readdir` | 1.05 | 0.05 |
+| `Filesystem.readFile` | 1.01 | 1.01 |
+| `Filesystem.writeFile` | 1.00 | 1.00 |
+| `Filesystem.mkdir` | 1.00 | 0.01 |
+| `Filesystem.rename` | 1.00 | 1.00 |
+| `Filesystem.deleteFile` | 1.00 | 0 |
+| **Total** | **16.11** | **10.12** |
+
+The change feed went from 16.5 to 10.5 calls per file, and 2,500 notes from
+16.03 to 10.03.
+
+Same results: the vault's files hashed identically (SHA-256 of every file
+outside `.zennotes`) between before and after for every pair: 602 files at
+600, 1,602 after the change feed, 2,502 at 2,500. `.zennotes/sync` was empty
+after every run. Resolving the first-sync conflict with Cloud's version (a
+staged publish over an existing note) landed the PC's text and left no
+rollback file.
+
+## What is left per file
+
+About 10 calls: `stat` 4 (core's existence check, the write's own, the
+pre-publish check, the post-sync scan's stability check), `inspect` 3 (both
+verifies, and the scan reading the new file), and one each of `writeFile`,
+`rename` and `readFile` (the app's rescan after sync). The core's check and
+the write's own `stat` look alike but sit on either side of the core
+boundary; merging them is a core API change. Seeding the scan cache from the
+write would need its own before and after `stat` to stay trustworthy, which
+costs what it saves.
+
+## Tests
+
+`npm test`: 322 pass (312 before, 10 new). `npm run typecheck` and
+`npm run boundaries:check` clean.
+
+- `cloud-sync-repository.test.ts`, "bridge calls per pulled file": the exact
+  call list for a new file (one recovery listing, no backup delete, no
+  temporary check); a replaced original is still verified, moved aside and its
+  rollback copy deleted; a rollback copy that will not delete stops the next
+  write; a failed write lists recovery files again; a staged publish that kept
+  its rollback copy stops the next write.
+- `native-fs-sync-uri.test.ts`: local file URIs come from the root resolved at
+  open and match Android's `getUri` encoding for awkward names, with or without
+  the folder slash; before open, or after the root moved, it asks `getUri`;
+  a known URI and the SAF document lookup are unchanged.
+- `mobile-cloud-sync.integration.test.ts`: each folder is made once per sync,
+  and again on the next sync.
+
+Against the old code, the call-count tests fail (as they should) and the
+safety tests pass.
+
+## How to test locally
+
+1. `npm test` and `npm run typecheck` in this checkout.
+2. On a device or emulator with a debug build, open the WebView over CDP
+   (`adb forward tcp:9333 localabstract:webview_devtools_remote_$(adb shell pidof md.zennotes)`)
+   and count calls during a sync:
+
+   ```js
+   const cap = window.Capacitor, stats = {}
+   cap.__orig ??= cap.nativePromise
+   cap.nativePromise = function (plugin, method, options) {
+     const s = (stats[`${plugin}.${method}`] ??= { n: 0 })
+     s.n++
+     return cap.__orig.call(this, plugin, method, options)
+   }
+   const summary = await window.zen.syncCloudVault()
+   Object.entries(stats).map(([k, v]) => `${k} ${(v.n / summary.pulled).toFixed(2)}`)
+   ```
+
+   A sync that pulls new notes should show no `getUri` or `deleteFile` and
+   about 10 calls per file in total.
+3. Pull a note into a new folder and edit a pulled note on another device,
+   then sync: both land, and `.zennotes/sync` holds nothing but an empty
+   `downloads` folder afterwards.

src/bridge/cloud-sync-repository.test.ts

@@ -633,3 +633,131 @@ describe('cached scan with the pinned conflict coordinator', () => {
     assert.deepEqual(h.reads, ['renamed.md'])
   })
 })
+
+describe('bridge calls per pulled file', () => {
+  const ID = /[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}/g
+  /** The repository over call-logging copies of the harness's fs and native files. */
+  function logged(h: ReturnType<typeof harness>, extra: Record<string, unknown> = {}) {
+    const calls: string[] = []
+    const log = <T extends Record<string, unknown>>(target: T) => Object.fromEntries(Object.entries(target).map(
+      ([name, value]) => [name, typeof value !== 'function' ? value : (...args: unknown[]) => {
+        calls.push([name, ...args.filter((arg) => typeof arg === 'string')].join(' ').replace(ID, '<id>'))
+        return value(...args)
+      }]
+    )) as T
+    const repository = new CachedCloudSyncRepository(log(h.fs), log({ ...h.native, ...extra }), h.store)
+    return { calls, repository }
+  }
+  const upsert = (sequence: number, path: string, text: string) => ({
+    sequence, item_id: `item-${sequence}`, revision: 1, type: 'upsert' as const, path, previous_path: null, content: content(text)
+  })
+  const tracked = (path: string, text: string) => ({
+    item_id: 'item-0', path, kind: 'text' as const, revision: 1,
+    sha256: content(text).sha256, byte_length: Buffer.byteLength(text), media_type: 'text/markdown'
+  })
+
+  it('lists recovery files once and skips backup and temporary cleanup for new files', async () => {
+    const h = harness({})
+    const { calls, repository } = logged(h)
+    await repository.apply(upsert(1, 'a.md', 'A'), undefined)
+    const first = calls.splice(0)
+    await repository.apply(upsert(2, 'folder/b.md', 'B'), undefined)
+    await repository.apply(upsert(3, 'folder/c.md', 'C'), undefined)
+
+    assert.deepEqual(first, [
+      'stat a.md',
+      'readdirStrict .zennotes/sync',
+      'stat a.md',
+      'writeText .zennotes/sync/write-<id>.md A',
+      'readForSync .zennotes/sync/write-<id>.md',
+      'stat a.md',
+      'rename .zennotes/sync/write-<id>.md a.md',
+      'readForSync a.md'
+    ])
+    assert.equal(calls.filter((call) => call.startsWith('readdirStrict')).length, 0)
+    assert.equal(calls.filter((call) => call.startsWith('deleteFile')).length, 0)
+    assert.equal(calls.filter((call) => call.startsWith('stat .zennotes')).length, 0)
+    assert.deepEqual([...h.files.keys()].sort(), ['a.md', 'folder/b.md', 'folder/c.md'])
+    assert.equal(h.files.get('folder/c.md')?.bytes.toString(), 'C')
+  })
+
+  it('still verifies a replaced original and removes its rollback copy', async () => {
+    const h = harness({ 'note.md': 'Old' })
+    const { calls, repository } = logged(h)
+    await repository.apply({ ...upsert(2, 'note.md', 'New'), item_id: 'item-0', revision: 2 }, tracked('note.md', 'Old'))
+    assert.equal(h.files.get('note.md')?.bytes.toString(), 'New')
+    assert.deepEqual([...h.files.keys()], ['note.md'])
+    assert.deepEqual(calls.filter((call) => /^(rename|deleteFile)/.test(call)), [
+      'rename note.md .zennotes/sync/rollback-<id>.md',
+      'rename .zennotes/sync/write-<id>.md note.md',
+      'deleteFile .zennotes/sync/rollback-<id>.md'
+    ])
+    // The original is verified before it moves aside, the new bytes after they land.
+    const aside = calls.indexOf('rename note.md .zennotes/sync/rollback-<id>.md')
+    const landed = calls.indexOf('rename .zennotes/sync/write-<id>.md note.md')
+    assert.equal(calls[aside - 1], 'readForSync note.md')
+    assert.equal(calls[landed + 1], 'readForSync note.md')
+  })
+
+  it('stops the next write when a rollback copy would not delete', async () => {
+    const h = harness({ 'note.md': 'Old' })
+    const deleteFile = h.fs.deleteFile
+    h.fs.deleteFile = async (path: string) => {
+      if (path.includes('/rollback-')) throw new Error('Delete refused')
+      await deleteFile(path)
+    }
+    const repository = new CachedCloudSyncRepository(h.fs, h.native, h.store)
+    await repository.apply({ ...upsert(2, 'note.md', 'New'), item_id: 'item-0', revision: 2 }, tracked('note.md', 'Old'))
+    const rollback = [...h.files.keys()].find((path) => path.includes('/rollback-'))
+    assert.equal(h.files.get(rollback!)?.bytes.toString(), 'Old')
+    await assert.rejects(repository.apply(upsert(3, 'other.md', 'Other'), undefined), /needs recovery/)
+    assert.equal(h.files.has('other.md'), false)
+  })
+
+  it('lists recovery files again after a failed write', async () => {
+    const h = harness({})
+    const { calls, repository } = logged(h)
+    h.setWriteHook((path) => {
+      h.setWriteHook(undefined)
+      throw new Error(`Native write failed for ${path}`)
+    })
+    await assert.rejects(repository.apply(upsert(1, 'a.md', 'A'), undefined), /Native write failed/)
+    await repository.apply(upsert(2, 'b.md', 'B'), undefined)
+    assert.equal(calls.filter((call) => call === 'readdirStrict .zennotes/sync').length, 2)
+    assert.deepEqual([...h.files.keys()], ['b.md'])
+  })
+
+  it('stops the next write when a staged publish kept its rollback copy', async () => {
+    const h = harness({ 'note.md': 'Old' })
+    const cloud = Buffer.from('Cloud version')
+    const ref = {
+      item_id: 'item-0', revision: 2, encoding: 'utf8', sha256: createHash('sha256').update(cloud).digest('hex'),
+      byte_length: cloud.length, media_type: 'text/markdown'
+    }
+    const { repository } = logged(h, {
+      statVerified: async (path: string) => (await h.fs.stat(path)),
+      download: async (options: { to: string }) => { h.put(options.to, cloud) },
+      rename: h.fs.rename,
+      deleteFile: async (path: string) => {
+        if (path.includes('/rollback-')) throw new Error('Delete refused')
+        await h.fs.deleteFile(path)
+      },
+      mkdir: async () => {},
+      readText: async (path: string) => h.files.get(path)!.bytes.toString()
+    })
+    const file = await repository.stageCloudContent!({
+      reference: ref, previewLimitBytes: 262_144, allowInsecureLoopback: false,
+      getInstruction: async () => ({
+        item_id: ref.item_id, revision: ref.revision,
+        content: { encoding: ref.encoding, sha256: ref.sha256, byte_length: ref.byte_length, media_type: ref.media_type },
+        download: { url: 'https://objects.example.test/item-0/2', method: 'GET', headers: {},
+          expires_at: new Date(Date.now() + 300_000).toISOString() }
+      })
+    })
+    await repository.applyStagedCloudContent!({ sequence: 2, item_id: 'item-0', type: 'upsert', path: 'note.md',
+      previous_path: null, revision: 2, content_ref: ref }, tracked('note.md', 'Old'), file)
+    assert.equal(h.files.get('note.md')?.bytes.toString(), 'Cloud version')
+    await assert.rejects(repository.apply(upsert(3, 'other.md', 'Other'), undefined), /needs recovery/)
+    assert.equal(h.files.has('other.md'), false)
+  })
+})

src/bridge/cloud-sync-repository.ts

@@ -81,8 +81,14 @@ export class CachedCloudSyncRepository extends PortableCloudSyncRepository {
           await contentFiles.assertReady(true)
           return staging.stage(source)
         },
-        applyStagedCloudContent: (...args: Parameters<NativeCloudStaging['apply']>) => staging.apply(...args),
-        resolveStagedCloudConflict: (input: Parameters<NativeCloudStaging['resolve']>[0]) => staging.resolve(input)
+        // A staged publish keeps its own rollback copy, so the next read or
+        // write looks for one again.
+        applyStagedCloudContent: async (...args: Parameters<NativeCloudStaging['apply']>) => {
+          try { return await staging.apply(...args) } finally { contentFiles.recheckRecovery() }
+        },
+        resolveStagedCloudConflict: async (input: Parameters<NativeCloudStaging['resolve']>[0]) => {
+          try { return await staging.resolve(input) } finally { contentFiles.recheckRecovery() }
+        }
       } : {}),
       readdir: (path: string) => fs.readdir(path),
       stat: (path: string) => fs.stat(path),
@@ -207,6 +213,11 @@ class NativeCloudSyncContent {
     this.recoveryChecked = true
   }
 
+  /** Something may have left a rollback file since the last listing. */
+  recheckRecovery(): void {
+    this.recoveryChecked = false
+  }
+
   async read(path: string, uri?: string): Promise<CloudSyncLocalItem> {
     await this.assertReady()
     const file = await this.native.readForSync(path, TEXT_EXTENSIONS.has(extension(path)), uri)
@@ -270,6 +281,7 @@ class NativeCloudSyncContent {
     const temporary = `.zennotes/sync/write-${randomUUID()}${extension(path)}`
     const backup = `.zennotes/sync/rollback-${randomUUID()}${extension(path)}`
     let publishing = false
+    let published = false
     let completed = false
     try {
       const source = this.sources.get(content)
@@ -290,6 +302,7 @@ class NativeCloudSyncContent {
       }
       publishing = true
       await this.fs.rename(temporary, path)
+      published = true
       await this.verify(path, content)
       completed = true
     } catch (error) {
@@ -307,9 +320,13 @@ class NativeCloudSyncContent {
       }
       throw error
     } finally {
-      this.recoveryChecked = false
-      if (await this.fs.stat(temporary) === 'file') await this.fs.deleteFile(temporary).catch(() => {})
-      if (completed) await this.fs.deleteFile(backup).catch(() => {})
+      // A write that completed and removed its backup leaves no rollback
+      // file, so the listing it started from still holds.
+      if (!completed) this.recoveryChecked = false
+      // A rename that resolved moved the temporary onto the path.
+      if (!published && await this.fs.stat(temporary) === 'file') await this.fs.deleteFile(temporary).catch(() => {})
+      // There is a backup only when an original was moved aside.
+      if (completed && before) await this.fs.deleteFile(backup).catch(() => { this.recoveryChecked = false })
     }
   }
 }

src/bridge/mobile-cloud-sync.integration.test.ts

@@ -225,6 +225,24 @@ describe('mobile Cloud adapter wiring', () => {
     })
   }
 
+  it('makes each folder once per sync however many pulled files land in it', async () => {
+    const h = await fixture({})
+    for (const path of ['notes/a.md', 'notes/b.md', 'notes/deeper/c.md', 'notes/d.md', 'top.md']) {
+      h.remoteText(path, `Content of ${path}`)
+    }
+    await h.sync()
+    assert.deepEqual([...h.files.keys()].sort(), ['notes/a.md', 'notes/b.md', 'notes/d.md', 'notes/deeper/c.md', 'top.md'])
+    assert.deepEqual(h.createdDirectories, ['notes', 'notes/deeper'])
+    // A folder removed between syncs comes back: each sync asks again. The
+    // edit's original moves aside into .zennotes/sync before the new bytes land.
+    h.createdDirectories.length = 0
+    h.remoteText('notes/e.md', 'Fifth note')
+    h.remoteText('notes/b.md', 'Edited on desktop')
+    await h.sync()
+    assert.deepEqual(h.createdDirectories, ['notes', '.zennotes/sync'])
+    assert.equal(h.files.get('notes/b.md')?.bytes.toString(), 'Edited on desktop')
+  })
+
   it('detects remote changes from a one-item metadata manifest without scanning local files', async () => {
     const h = await fixture()
     assert.equal(await h.api.hasMobileCloudVaultChanges(h.vault), true)

src/bridge/mobile-cloud-sync.ts

@@ -341,6 +341,9 @@ export async function restoreMobileCloudBackupNote(
 }
 
 function hostVault(vault: MobileVault, cacheScan = false): CloudSyncHostVault {
+  // Folders this operation has made, so a pull asks for each one once rather
+  // than before every file it renames into place.
+  const folders = new Set<string>()
   const fs: PortableCloudSyncFileSystem = {
     // Sync must distinguish an unreadable provider from an empty vault.
     readdir: async (directory) =>
@@ -356,8 +359,18 @@ function hostVault(vault: MobileVault, cacheScan = false): CloudSyncHostVault {
     deleteFile: (path) => vault.fs.deleteFile(path),
     rename: async (from, to) => {
       const parent = to.includes('/') ? to.slice(0, to.lastIndexOf('/')) : ''
-      if (parent) await vault.fs.mkdir(parent)
-      await vault.fs.rename(from, to)
+      if (parent && !folders.has(parent)) {
+        await vault.fs.mkdir(parent)
+        folders.add(parent)
+      }
+      try {
+        await vault.fs.rename(from, to)
+      } catch (error) {
+        // The folder may have gone since: a rename into a missing folder
+        // fails before it changes anything, and the next one makes it again.
+        folders.delete(parent)
+        throw error
+      }
     }
   }