Skip to content

fix(export-notes-json): stop silently dropping/duplicating notes with duplicate titles - #116

Merged
sweetrb merged 2 commits into
sweetrb:mainfrom
draku:fix/export-notes-json-silent-drop-duplciate-title
Aug 1, 2026
Merged

fix(export-notes-json): stop silently dropping/duplicating notes with duplicate titles#116
sweetrb merged 2 commits into
sweetrb:mainfrom
draku:fix/export-notes-json-silent-drop-duplciate-title

Conversation

@draku

@draku draku commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Description

export-notes-json silently dropped and duplicated notes whenever two or more notes in the same account/folder shared an exact title. exportNotesAsJson() listed note titles per folder, then re-fetched each note a second time by title (getNoteDetails(title, account) / getNoteContent(title, account)). AppleScript's note "<name>" specifier resolves an ambiguous (duplicated) name deterministically to the same one note every time, so every loop iteration for a title with N duplicates re-fetched the same underlying note N times — even though the bulk AppleScript listing one call earlier had already correctly deduped and counted N distinct notes by id. The export ended up with one note repeated N times (same id, same content) and the other N-1 real notes missing, with no error or count mismatch surfaced.

The fix threads the ids already produced by the bulk listing through the export loop and re-fetches by id (getNoteById / getNoteContentById) instead of by title — the same identity-safe pattern searchNotes and deleteNoteById already use elsewhere in this file. listNotes()'s public string[]-of-titles contract (used by the list-notes tool) is unchanged.

Fixes #115.

Type of Change

  • Bug fix
  • New feature
  • Documentation update
  • Refactoring
  • Other (describe):

Testing

  • Tests pass locally (pnpm test) — 519 passed, including a new regression test (exports both notes when two notes share an exact title) that reproduces the actual bug mechanism: a mock that mirrors AppleScript's real ambiguous by-name resolution, hand-verified to fail against the old title-based code and pass against the new id-based code.
  • Linting passes (pnpm run lint)
  • Typecheck and formatting pass (pnpm run typecheck && pnpm run format:check)
  • Build succeeds (pnpm run build)

Checklist

  • I have read the CONTRIBUTING guidelines
  • My code follows the project's style
  • I have updated documentation if needed (CHANGELOG.md)
  • If this PR changes shipped code (src/** excluding tests, or the runtime dependencies in package.json): version bumped at least a patch (2.6.112.6.12) + a CHANGELOG.md entry — the require-version-bump CI check enforces this (docs-only and test-only PRs are exempt)

draku and others added 2 commits August 1, 2026 08:09
… duplicate titles

`exportNotesAsJson` listed each folder's note titles via `listNotes()`,
then re-fetched each note a second time by title via
`getNoteDetails(title, account)` / `getNoteContent(title, account)`.
AppleScript's `note "<name>"` specifier resolves an ambiguous
(duplicated) name deterministically to the same one note every time,
so every loop iteration for a title with N duplicates re-fetched the
*same* underlying note N times — even though the bulk listing one call
earlier had already correctly deduped and counted N distinct notes by
id. The export ended up with one note repeated N times (same id, same
content) and the other N-1 real notes silently missing, with no error
or count mismatch surfaced.

The fix threads the ids already produced by the bulk AppleScript
listing through the export loop and re-fetches by id (`getNoteById` /
`getNoteContentById`) instead of by title — the same identity-safe
pattern `searchNotes` and `deleteNoteById` already use elsewhere in
this file. `listNotes()`'s public `string[]`-of-titles contract is
unchanged.

Added a regression test that reproduces the actual bug mechanism (two
distinct notes sharing a title, with a mock that mirrors AppleScript's
real ambiguous-name resolution) rather than one that merely queues
mock responses in call order.

Version bumped to 2.6.12 and CHANGELOG.md updated per this repo's
release conventions (CONTRIBUTING.md).

Fixes sweetrb#115

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…adings

The [Unreleased] section had accumulated entries for two versions that
already shipped (v2.6.10 2026-07-22, v2.6.11 2026-07-31), because nothing
in the release path renames [Unreleased] to the version being cut — the
`version` lifecycle script only syncs the plugin manifests. Entries for
released code sitting under [Unreleased] misreport what a given install
contains.

Files each entry under the release that actually shipped it, and puts this
PR's export-notes-json fix under [2.6.12]. Bullet text is untouched; only
headings move. An empty [Unreleased] is deliberately kept at the top —
dependabot-rebuild.yml hard-exits when that marker is missing.
@sweetrb

sweetrb commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Reviewed — merging. Excellent root-cause work, @draku.

The diagnosis is the valuable part here: spotting that parseBulkListOutput had already deduped by id and proven N distinct notes existed, and that discarding everything but the title was what threw that away, is precisely right. Re-fetching by id via getNoteById/getNoteContentById matches what searchNotes and deleteNoteById already do, so this converges the file on one identity model instead of adding a special case.

Two things I checked carefully and want to record:

  • listNoteRefs is behaviourally identical to the listNotes(account, folder) call it replaces. With limit undefined, listNotes skips the sliceLimit path entirely and falls through to buildBulkListCommand({ folderRef, dateSetup }) — the same command listNoteRefs issues. That matters because only the sliceLimit branch emits the leading totalCount header, so parsing result.output directly (no header strip) is correct here and would not have been on the sliced path. resolveAccount is likewise a no-op difference, since exportNotesAsJson always passes a concrete account.name from listAccounts().
  • note.account = account.name is the right patch-up. getNoteById deliberately omits account (ids are application-level), while the old getNoteDetails set it from targetAccount — without that line every exported note would have fallen back to exportNote's "iCloud" default and mislabelled non-iCloud accounts.

listNotes's public string[] contract is preserved via the .map(ref => ref.title) at both return sites, so list-notes is untouched.

Verified against a real 359-note library (4 accounts, 8 folders, 5 genuinely-duplicated titles), driving export-notes-json over stdio against both bundles:

build exported entries distinct ids repeated ids
2.6.11 (pre-fix) 359 351 5 — 8 real notes silently missing
2.6.12 (this PR) 359 359 0

The 5 duplicate titles correctly survive in both, since those are distinct notes that legitimately share a name. That's the bug reproduced and closed on real data, not just in the mock.

Your regression test is the right shape too — mirroring AppleScript's ambiguous note "<name>" resolution in the mock is what makes it actually fail against the old code rather than trivially pass.

I pushed one follow-up commit (4db7f41, docs only): your entry was filed under ## [Unreleased], but this repo's release path never renames that section, so entries for shipped versions get stranded there — 2.6.10 and 2.6.11 had already been orphaned that way. I filed each entry under the release that actually shipped it and put yours under ## [2.6.12]. Bullet text is untouched; only headings moved. An empty ## [Unreleased] stays at the top because dependabot-rebuild.yml hard-exits without that marker. I'm adding a version-guard rule across all four repos so a bump without a matching heading fails CI instead of drifting again.

Also worth noting: CI initially showed no checks at all on this PR — the first-time-contributor approval gate, not anything wrong with your branch. Approved and everything passes (test (22), test (24), require-version-bump, bundle-boots, integration, CodeQL).

Shipping in 2.6.12. Thanks for the thorough writeup on #115 as well — the real-world examples ("Rust Package Registry", 8 notes collapsing to 1) made the impact immediately legible.

@sweetrb
sweetrb merged commit 4643e65 into sweetrb:main Aug 1, 2026
10 checks passed
@draku

draku commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

Happy to help. Thx

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.

export-notes-json silently drops/duplicates notes with identical titles

2 participants