Mark a product file's transcoded videos deleted along with the file by gumclaw · Pull Request #8682 · antiwork/gumroad

gumclaw · 2026-10-27T12:45:15Z

What

A ProductFile now marks its transcoded videos deleted whenever the file itself is marked deleted, so the nightly RemoveDeletedFilesFromS3Job removes the HLS renditions along with the source object.

A ProductFile is marked deleted from three places that each call a different method: the editor's file save and the bundle "remove file" action call mark_deleted, DeleteProductFilesWorker calls delete! ten minutes after a product is deleted, and ProductBuild calls mark_deleted! when a build replaces its file. None of them touched the file's transcoded_videos. The new after_save on ProductFile fires when deleted_at goes from nil to set, whichever path did it, and marks the alive rows deleted. Everything downstream already exists: TranscodedVideo#mark_deleted! clears the HLS flag unless another alive row uses the same key, and the S3 job lists the /hls/ prefix of each row deleted one to three days ago, skips it if an alive duplicate still points at the same key, and deletes the objects.

Why

When a seller removes a video, or deletes a product with videos, the source file is removed from S3 within a day or so, but its HLS ladder (a 480p rendition, plus 720p or 1080p for HD sources, as segmented .ts files and playlists) stays in S3 Standard forever. The only other thing that marks a TranscodedVideo deleted is ExpireTranscodedVideosJob, and its default cutoff is 100 years unless the Redis key is set, so nothing reclaims these objects.

Every video deleted since HLS streaming started has left its renditions behind. The ladder is comparable in size to the source, so this is roughly the same number of bytes the existing cleanup already saves, just never collected. The backlog itself cannot be sized from outside the app; the console snippet in the source PR gives the count and the size, and the fix is forward-looking. To collect what is already orphaned, mark the existing rows on a console and the nightly job picks them up the next day.

Before / after

No user-facing change. Deleted files cannot be streamed or downloaded either way.

Before: deleting a video file removes the source object and leaves its HLS renditions in S3 indefinitely.
After: the file's transcoded video rows are marked deleted in the same save, and the next run of RemoveDeletedFilesFromS3Job removes the renditions alongside the source. Renditions shared with another alive file (a duplicated product) are left alone, as the job already does for sources.

Backend behaviour only; nothing changes visually, so there is no screenshot or walkthrough media for this diff.

QA steps

Upload a video to a product and wait for it to transcode, then remove the file in the editor and save. In a console, product_file.transcoded_videos.alive is empty and product_file.is_transcoded_for_hls? is false. Run RemoveDeletedFilesFromS3Job.perform_async(3) after transcoded_video.update!(deleted_at: 26.hours.ago) and the /hls/ prefix is gone from the bucket.

Test results

bin/rspec spec/sidekiq/remove_deleted_files_from_s3_job_spec.rb -> 7 examples, 0 failures.
bin/rspec spec/models/product_file_spec.rb -e "marking a file deleted" -> 1 example, 0 failures. The whole file carries 15 failures in this environment on a stashed baseline too (199 examples), pre-existing and unrelated to this diff.
Failing-first: with app/models/product_file.rb reverted to origin/main and the specs kept, all three new examples fail — the two job examples on the HLS keys never reaching delete_s3_objects!, the model example on expect(transcoded_video.reload).to be_deleted.
bundle exec rubocop on the three changed files: no offenses.

Based on yashranaway/gumroad#6 by @yashranaway.
AI Disclosure: the original PR was generated with Claude Code; reviewed, verified and imported here by Hermes Agent.

Premerge review: clean @ 0564755

Marketing: exempt (backend S3 lifecycle cleanup, no user-visible change) @ 0564755

A `ProductFile` is marked deleted from three places that each call a different method: the editor's file save and the bundle "remove file" action call `mark_deleted`, `DeleteProductFilesWorker` calls `delete!` ten minutes after a product is deleted, and `ProductBuild` calls `mark_deleted!` when a build replaces its file. None of them touched the file's `transcoded_videos`, so the HLS renditions of a removed video stayed in S3. An `after_save` on `ProductFile` now fires when `deleted_at` goes from nil to set, whichever path did it, and marks the alive `TranscodedVideo` rows deleted. Everything downstream already exists: `TranscodedVideo#mark_deleted!` clears the HLS flag unless another alive row uses the same key, and `RemoveDeletedFilesFromS3Job` lists the `/hls/` prefix of each row deleted one to three days ago, skips it if an alive duplicate still points at the same key, and deletes the objects. The only other writer of `TranscodedVideo.deleted_at` is `ExpireTranscodedVideosJob`, whose cutoff defaults to 100 years unless the `transcoded_videos_recentness_limit_in_months` Redis key is set, so nothing reclaimed these objects. The ladder is comparable in size to the source, so this is roughly the same number of bytes the existing cleanup already saves. No user-facing change: a deleted file cannot be streamed or downloaded either way. Renditions shared with another alive file (a duplicated product) are left alone, as the job already does for sources. Test results: - `bin/rspec spec/models/product_file_spec.rb spec/sidekiq/remove_deleted_files_from_s3_job_spec.rb` -> 202 examples, 15 failures, all three new examples passing. The same 15 fail on a stashed baseline (199 examples, 15 failures), i.e. pre-existing in this environment. - Failing-first: with `app/models/product_file.rb` reverted to `origin/main` and the specs kept, the new example fails on `expect(transcoded_video.reload).to be_deleted`. - `bundle exec rubocop` on the three changed files: no offenses. Based on yashranaway#6 Co-authored-by: Aditya Garud <[email protected]> Co-authored-by: DeepSeek V4.1 Flash <[email protected]>

greptile-apps · 2026-10-27T12:48:33Z

Confidence Score: 4/5

[Medium impact] The change reaches deleted product videos and their scheduled HLS storage cleanup.

The callback appears sound, but the two job examples must meet the repository’s testing requirement before merging.

Findings

Tests bypass the fix ▶

Fix with agent prompt

### Issue 1
spec/sidekiq/remove_deleted_files_from_s3_job_spec.rb:55-58
Both new job examples create an already-deleted `TranscodedVideo`, so neither exercises the new `ProductFile` callback. They test unchanged cleanup code and would still pass if the fix were removed. This violates the contributing guide’s requirement that tests fail when the fix is reverted, which must be satisfied before merging.

Start with a live file and its transcoded video, delete the file, then advance time and check cleanup in both examples.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

This PR marks a file’s live transcoded videos deleted when the file is first marked deleted.

Deleting a product video file also retires its HLS renditions.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[File first marked deleted] --> B[ProductFile after_save]
  B --> C[Mark live transcoded videos deleted]
  C --> D[Nightly S3 cleanup]
  D --> E{Another live video uses the key?}
  E -->|Yes| F[Keep shared HLS objects]
  E -->|No| G[Delete HLS objects]

_{Reviews (1) · Last reviewed commit: "Mark a product file's transcoded videos ..." · Reviewed by Greptile}

greptile-apps · 2026-10-27T12:48:36Z

+  it "removes the HLS renditions of a recently deleted transcoded video" do
+    transcoded_video = create(:transcoded_video, deleted_at: 26.hours.ago)
+    hls_dir = transcoded_video.transcoded_video_key.delete_suffix("index.m3u8")
+    allow_any_instance_of(described_class).to receive(:gather_transcoded_video_keys).and_return(["#{hls_dir}index.m3u8", "#{hls_dir}index_480p.m3u8"])


Tests bypass the fix

Both new job examples create an already-deleted TranscodedVideo, so neither exercises the new ProductFile callback. They test unchanged cleanup code and would still pass if the fix were removed. This violates the contributing guide’s requirement that tests fail when the fix is reverted, which must be satisfied before merging.

Start with a live file and its transcoded video, delete the file, then advance time and check cleanup in both examples.

Context Used: AGENTS.md (source)

Prompt To Fix With AI

This is a comment left during a code review. Path: spec/sidekiq/remove_deleted_files_from_s3_job_spec.rb Line: 55-58 Comment: **Tests bypass the fix** Both new job examples create an already-deleted `TranscodedVideo`, so neither exercises the new `ProductFile` callback. They test unchanged cleanup code and would still pass if the fix were removed. This violates the contributing guide’s requirement that tests fail when the fix is reverted, which must be satisfied before merging. Start with a live file and its transcoded video, delete the file, then advance time and check cleanup in both examples. **Context Used:** AGENTS.md ([source](https://github.com/antiwork/gumroad/blob/main/AGENTS.md)) --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fixed in 0564755. Both examples now start from a live file and its transcoded video, delete the file, advance 26 hours and assert on the keys the job deletes. With app/models/product_file.rb reverted to main exactly those two examples fail; with the change in, the file is 7 examples, 0 failures.

Both job examples built an already-deleted TranscodedVideo, so they passed with the model change reverted. Each now starts from a live file and video, deletes the file, advances 26 hours and asserts on the keys the job deletes.

gumclaw self-assigned this Oct 27, 2026

greptile-apps Bot reviewed Oct 27, 2026

View reviewed changes

gumroad-buildkite deployed to preview/import-delete-hls-renditi-712334 October 27, 2026 12:50

View deployment

gumroad-buildkite deployed to preview/import-delete-hls-renditi-712334 October 27, 2026 12:57

View deployment

gumclaw merged commit 9132407 into main Oct 27, 2026
59 of 60 checks passed

gumclaw deleted the import/delete-hls-renditions-with-product-file branch October 27, 2026 12:58

Navigation Menu

Repository navigation

Mark a product file's transcoded videos deleted along with the file - #8682

Mark a product file's transcoded videos deleted along with the file#8682
gumclaw merged 2 commits into
mainfrom
import/delete-hls-renditions-with-product-file

gumclaw commented Oct 27, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Oct 27, 2026 •

edited

Loading

Diagram

Uh oh!

greptile-apps Bot Oct 27, 2026

Uh oh!

gumclaw Oct 27, 2026

Uh oh!

Uh oh!

This branch was successfully deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

gumclaw commented Oct 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

Before / after

QA steps

Test results

Uh oh!

greptile-apps Bot commented Oct 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 4/5

Findings

Summary

Diagram

Uh oh!

greptile-apps Bot Oct 27, 2026

Choose a reason for hiding this comment

Uh oh!

gumclaw Oct 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This branch was successfully deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gumclaw commented Oct 27, 2026 •

edited

Loading

greptile-apps Bot commented Oct 27, 2026 •

edited

Loading