Skip to content

fix(pds): Remove empty collections from cache on record delete#157

Merged
ascorbic merged 7 commits into
ascorbic:mainfrom
NuroDev:NuroDev/pds-remove-empty-collections
May 12, 2026
Merged

fix(pds): Remove empty collections from cache on record delete#157
ascorbic merged 7 commits into
ascorbic:mainfrom
NuroDev:NuroDev/pds-remove-empty-collections

Conversation

@NuroDev
Copy link
Copy Markdown
Contributor

@NuroDev NuroDev commented May 1, 2026

Summary

  • When the last record in a collection was deleted, the collection name remained in the SQLite collections cache, so describeRepo kept reporting empty collections.
  • After applying a delete, walk the repo from the ${collection}/ prefix and call a new SqliteRepoStorage.removeCollection() if no records remain in that collection.
  • Adds unit tests covering the cache helper directly and the rpcDeleteRecord integration (last-record removal vs. records-remaining retention).

Test plan

  • pnpm --filter @getcirrus/pds test passes (249 tests)
  • Manually verify describeRepo stops listing a collection after deleting its final record

🤖 Generated with Claude Code

Copy link
Copy Markdown
Owner

@ascorbic ascorbic left a comment

Choose a reason for hiding this comment

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

Thanks!

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 4, 2026

Open in StackBlitz

npm i https://pkg.pr.new/create-pds@157
npm i https://pkg.pr.new/@getcirrus/oauth-provider@157
npm i https://pkg.pr.new/@getcirrus/pds@157

commit: 9e2d8f5

Copy link
Copy Markdown
Owner

@ascorbic ascorbic left a comment

Choose a reason for hiding this comment

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

Actually, I realise this should also be applied to rpcApplyWrites, because that can also be used to delete records (and is in fact the more common way)

Mirror the rpcDeleteRecord fix for the more common deletion path:
applyWrites batches that empty a collection now drop the cache entry.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@NuroDev
Copy link
Copy Markdown
Contributor Author

NuroDev commented May 4, 2026

Actually, I realise this should also be applied to rpcApplyWrites, because that can also be used to delete records (and is in fact the more common way)

Ah okay, good point. Pushed a new commit that includes this change + tests.

@ascorbic ascorbic merged commit 241a5fc into ascorbic:main May 12, 2026
5 checks passed
@mixie-bot mixie-bot Bot mentioned this pull request May 12, 2026
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.

2 participants