Add [output] audio_dir to store recordings separately from transcripts by aaugustin · Pull Request #37 · paberr/ownscribe

aaugustin · 2026-07-12T09:43:23Z

Audio is a disposable cache once transcription succeeds, while transcripts and summaries are the real output worth syncing across devices and backing up. Storing the audio elsewhere avoids overloading a sync or backup system with uncompressed audio files.

audio_dir defaults to empty (same as dir, current behavior) and, when set, keeps the audio directory in sync with the standard output directory's timestamp name and title-slug rename across the record, transcribe, summarize, resume, and cleanup flows.

aaugustin · 2026-07-12T09:44:14Z

For clarity: the code is half Claude half me; yes it started with a prompt, but I reworked heavily to bring to PR to an acceptable level of quality.

Copilot

Pull request overview

Adds an optional output.audio_dir configuration to store raw recordings in a separate “audio cache” directory while keeping transcript/summary output in the usual output.dir. This aligns with ownscribe’s pipeline design where transcripts/summaries are the durable artifacts and audio can be treated as disposable once transcription succeeds.

Changes:

Introduces output.audio_dir (default empty → current colocated behavior) with resolved_audio_dir path resolution.
Updates pipeline flows to record into audio_dir, keep audio/output directories name-aligned during title-slug renames, and allow resume to locate audio in the separate audio directory.
Extends cleanup to remove the configured audio directory and adds/updates tests and documentation.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/test_pipeline.py	Adds coverage for recording to `audio_dir`, renaming behavior, deletion when `keep_recording=false`, and `resume` lookup in the audio cache.
tests/test_config.py	Adds tests for `OutputConfig.resolved_audio_dir` tilde expansion, absolute paths, and empty fallback behavior.
tests/test_cli.py	Adds cleanup tests ensuring separate `audio_dir` is removed for `--all` and `--output` flows.
src/ownscribe/pipeline.py	Implements audio output directory selection, directory renaming helper, audio deletion using the actual audio path, and `resume` lookup in audio cache.
src/ownscribe/config.py	Adds `audio_dir` to `OutputConfig` plus `resolved_audio_dir` property and default config TOML update.
src/ownscribe/cli.py	Extends `cleanup` target selection to include separate audio directory.
README.md	Documents the new `[output].audio_dir` setting in the config snippet.

Comments suppressed due to low confidence (1)

src/ownscribe/cli.py:304

In interactive cleanup mode, the code appends to targets while iterating it, and then passes the original (unconfirmed) targets list to _remove_targets(). This can lead to repeated prompts, duplicate entries, and (most importantly) deleting directories the user did not confirm.

            if size == "(not found)":
                click.echo(f"  {label}: {path} — {size}, skipping")
                continue
            if yes or click.confirm(f"  Remove {label}: {path} ({size})?"):
                targets.append((label, path))

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

aaugustin · 2026-07-12T10:08:50Z

+    # First look for an audio file in the target directory, since the user is
+    # explicitly resuming there. If none is found and a separate directory is
+    # configured for audio, search that directory as well.
    audio = _find_audio(dir_path)
+    if audio is None:
+        candidate = config.output.resolved_audio_dir / dir_path.name
+        if candidate != dir_path and candidate.is_dir():
+            audio = _find_audio(candidate)


This case is already covered by the condition if candidate != dir_path two lines below and explained in the comment two lines above.

see my comment above, on a second look, I think CoPilot is correct

aaugustin · 2026-07-12T10:11:08Z

+        audio_dir = config.output.resolved_audio_dir / out_dir.name
+        out_dir, old_out_dir = _rename_output_dir(out_dir, title_slug), out_dir
+        # Rename audio_dir only if it's separate from out_dir
+        # and if out_dir was successfully renamed.
+        if audio_dir != old_out_dir and out_dir != old_out_dir:


The case when output.audio_dir is unset is covered by if audio_dir != old_out_dir.

Targeting and renaming would still work fine when the summarizing a transcript outside the configured output tree as far as I can tell.

Hm... I think Co-Pilot is right in the following case:

When you call ownscribe summarize ~/notes/foo/transcript.md, audio_dir is unset and resolved_audio_dir falls back to resolved_dir, which would try to rename ~/ownscribe/foo.
The guard only helps if the transcript is inside the output tree.

Possible fix: only do the second rename if config.output.audio_dir is set and the directory exists.

Yes, I had missed the fact that you can summarize an arbitrary file, even if it's outside of ownscribe's output tree.

paberr

Thanks a lot for the work, this is a good addition.
I personally just have keep_recording = false all the time, but I can see how you might want to keep it but in a different directory.

I looked through the PR and found a few edge cases. After addressing them, it should be ready!

paberr · 2026-07-15T15:43:50Z

    else:
-        # Interactive: prompt for each directory
-        for label, path in [
+        targets = [


Here we have a clash in variable naming. targets is now the name for the list of things to prompt for and the name for the variable storing where the user replied "yes".
Maybe rename the first to candidates or so?

Oops. You're right.

paberr · 2026-07-15T15:57:41Z

+            # and if out_dir was successfully renamed.
+            if audio_dir != old_out_dir and out_dir != old_out_dir:
+                audio_dir = _rename_output_dir(audio_dir, title_slug)
+            audio_path = audio_dir / audio_path.name


If I see correctly, this will break the old behaviour:

Assume keep_recording = false and no separate audio directory, but we have a title_slug.

Then, audio_dir is only assigned to audio_path.parent, which is the pre-rename folder.

So, later on audio_path.exists() will return false.

In that case, audio_dir should become out_dir. The logic is currently a bit messy, maybe we can clean it up a bit?

Yes, good catch.

I'm straightening the logic with clearer conditions and comments.

paberr · 2026-07-15T16:03:09Z

+        audio_dir = config.output.resolved_audio_dir / out_dir.name
+        out_dir, old_out_dir = _rename_output_dir(out_dir, title_slug), out_dir
+        # Rename audio_dir only if it's separate from out_dir
+        # and if out_dir was successfully renamed.
+        if audio_dir != old_out_dir and out_dir != old_out_dir:


Hm... I think Co-Pilot is right in the following case:

When you call ownscribe summarize ~/notes/foo/transcript.md, audio_dir is unset and resolved_audio_dir falls back to resolved_dir, which would try to rename ~/ownscribe/foo.
The guard only helps if the transcript is inside the output tree.

Possible fix: only do the second rename if config.output.audio_dir is set and the directory exists.

paberr · 2026-07-15T16:04:16Z

+    # First look for an audio file in the target directory, since the user is
+    # explicitly resuming there. If none is found and a separate directory is
+    # configured for audio, search that directory as well.
    audio = _find_audio(dir_path)
+    if audio is None:
+        candidate = config.output.resolved_audio_dir / dir_path.name
+        if candidate != dir_path and candidate.is_dir():
+            audio = _find_audio(candidate)


see my comment above, on a second look, I think CoPilot is correct

Audio is a disposable cache once transcription succeeds, while transcripts and summaries are the real output worth syncing across devices and backing up. Storing the audio elsewhere avoids overloading a sync or backup system with uncompressed audio files. audio_dir defaults to empty (same as dir, current behavior) and, when set, keeps the audio directory in sync with the standard output directory's timestamp name and title-slug rename across the record, transcribe, summarize, resume, and cleanup flows. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Also delete the directory containing the audio recording when audio is stored in a separate directory and deleted, to avoid leaving behind an empty directory.

aaugustin · 2026-07-17T06:23:41Z

I believe I have addressed all your comments, which were correct.

Also, I improved the branch that deletes the recording after transcription: I'm also deleting the directory if it's empty, which will be the standard case when storing audio in a separate directory.

- Remove the emptied audio directory with rmdir(); unlink() cannot remove directories and crashed keep_recording=false runs with a separate audio_dir. - Configure audio_dir in the separate-audio-dir tests so they exercise the feature. - Follow out_dir's rename when the recording is colocated with the text output even though a separate audio_dir is configured (e.g. resuming a directory recorded before audio_dir was set). - Only rename a matching audio directory when the summarized transcript lives in the output tree, so summarizing a transcript elsewhere cannot rename an unrelated audio directory that shares its name.

paberr · 2026-07-20T13:55:37Z

Thanks for the quick turnaround on the review comments! Rather than another review round-trip, I pushed a small fixup commit (50db237) directly:

audio_path.parent.unlink() can't remove a directory (PermissionError on macOS, IsADirectoryError on Linux), so every keep_recording=false run with a separate audio_dir crashed right after deleting the recording — now rmdir().
The two new TestDoTranscribeAndSummarize tests didn't set output.audio_dir, so they exercised the default colocated path instead of the feature; one of them was failing. They now configure the separate dir.
Two edge-case guards while I was in there: resuming a directory that still holds its own recording (made before audio_dir was configured) now follows the out_dir rename instead of logging a spurious warning and leaving a stale path; and summarize on a transcript outside the output tree no longer renames a same-named, unrelated directory under audio_dir.

Full suite is green locally (256 passed). Merging once CI passes — thanks for the contribution!

paberr

All review feedback addressed; remaining issues fixed directly on the branch (see comment). LGTM.

aaugustin · 2026-07-21T10:25:21Z

Thank you for adding the missing tests and bringing this to the finish line.

I should have actually tested removing the directory 🤦

paberr · 2026-07-21T10:39:11Z

No worries, all good! I appreciate the contributions. :)

Copilot AI review requested due to automatic review settings July 12, 2026 09:43

Copilot started reviewing on behalf of aaugustin July 12, 2026 09:43 View session

aaugustin force-pushed the feature/move-audio-to-cache-directory branch from 6aba379 to 0ec1188 Compare July 12, 2026 09:45

Copilot AI reviewed Jul 12, 2026

View reviewed changes

paberr requested changes Jul 15, 2026

View reviewed changes

aaugustin and others added 2 commits July 17, 2026 07:40

Account for all review comments.

0babd7c

Also delete the directory containing the audio recording when audio is stored in a separate directory and deleted, to avoid leaving behind an empty directory.

aaugustin force-pushed the feature/move-audio-to-cache-directory branch from 0ec1188 to 0babd7c Compare July 17, 2026 06:21

paberr approved these changes Jul 20, 2026

View reviewed changes

paberr merged commit afc1d18 into paberr:main Jul 21, 2026
2 checks passed

Conversation

aaugustin commented Jul 12, 2026

Uh oh!

aaugustin commented Jul 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paberr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aaugustin commented Jul 17, 2026

Uh oh!

paberr commented Jul 20, 2026

Uh oh!

paberr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aaugustin commented Jul 21, 2026

Uh oh!

paberr commented Jul 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants