Repository navigation
Recording history retention controls (#264) - #416
Merged
Merged
Conversation
The count cap (previously a hardcoded MAX_ENTRIES=50) and a new time-based window are both read fresh via getRetentionPolicy, an injected adapter rather than constructor args - they're live Settings values, so a change takes effect on the very next write or read with no need to recreate the store. Purging runs on every write AND every read, not just at record() time - an expired entry is meant to be actually gone from disk once its window passes, not just hidden from whoever next asks. retentionDays: 0 means never expire by time, still bounded by the count cap. now is injectable for tests, same reasoning as settingsStore.js's filePath injection - no real clock needed to prove the expiry logic. Claude-Session: https://claude.ai/code/session_0144sxo5CJiyk99vaR7sgjuj
Defaults come from recordingHistoryStore.js's own constants (DEFAULT_RETENTION_DAYS=30, MAX_ENTRIES=50) so the two files can't drift apart. 30 days by default rather than "forever" - the issue's own framing is that privacy-sensitive users want a real expiry on by default, not a setting they have to go find first. Claude-Session: https://claude.ai/code/session_0144sxo5CJiyk99vaR7sgjuj
recordingHistoryStore's getRetentionPolicy reads settingsStore.get() fresh on every call - no caching, no extra onChange subscription needed, since the store already reads it live per the previous commit. IPC + bridge: settings:set-history-retention-days, settings:set-history-max-entries. Claude-Session: https://claude.ai/code/session_0144sxo5CJiyk99vaR7sgjuj
Two selects, same preset-plus-custom-value pattern as idle unload - "Keep recording history for" (0/7/30/90/365 days) and "Recording history size" (10/25/50/100/250 entries). Independent limits, whichever is hit first applies. History's own header used to hardcode "last 50" - now reflects whatever the two settings actually are. Claude-Session: https://claude.ai/code/session_0144sxo5CJiyk99vaR7sgjuj
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #264. Retention controls for #136's recording history: auto-expire by time, a configurable count cap, both independent of the existing clear-all.
Scoped down from the issue text
The issue was written expecting audio might be retained ("auto-delete audio after X days") - it isn't. #136 only ever stores the transcribed text, never the recording itself, so there's no audio-retention leg to build here. What's left, and what this PR builds, is exactly the other two asks: a time-based expiry and a configurable count cap.
What's built
recordingHistoryStore.js: the count cap (previously a hardcoded 50) and a new time window are both read live viagetRetentionPolicy, an injected adapter rather than constructor args - a change in Settings takes effect on the very next write or read, no store recreation needed. Purging runs on every write and every read, not just when a new dictation comes in - an expired entry is meant to be actually gone from disk once its window passes, not just hidden until the next record().historyRetentionDays(default 30 - the issue's own framing is that privacy-sensitive users want a real expiry on by default, not a setting to go find first) andhistoryMaxEntries(default 50, unchanged from before).retentionDays: 0means never expire by time, still bounded by the count.Judgement calls to veto
record()and everylist()/get()call - rather than a timer running for the app's lifetime. If the user sets a short window and then doesn't dictate or open History for a while, stale entries sit on disk a bit longer than the window technically allows, cleaned up the next time either happens. Given this is text-only (no audio, no live-streaming privacy exposure), I judged an always-running timer wasn't worth it for a v1. Flagging in case the "privacy-sensitive users" framing wants stronger guarantees than this.Tests
recordingHistoryStore.test.js(+8, incl. expiry-on-record, expiry-on-read-alone, persisted purge surviving a reopen, and a live policy change taking effect without recreating the store),settingsStore.test.js(+2). Full suite (376) + typecheck + build green.🤖 Generated with Claude Code