perf(metadata): avoid no-op artwork GC healing

[blurbery]

Problem

Related issue: N/A — narrow fix

Artwork revision GC was doing healing work even when there was nothing to heal. After deleting an object set, it could run 24 UPDATEs across 14 artwork surfaces for every candidate, whether anything still referenced that path or not.

Over a 46.6-hour production sample, the final reference check and two people.photo_path healing statements alone accumulated about 83 minutes of database execution and 619 million shared-block hits.

This is background/recovery cost, not the normal browsing path.

Approach

Keep the existing correctness gates and remove the no-op healing work:

• Retain the final under-lock reference check, object deletion, and durable tombstone for each candidate.
• Run one materialized reference sweep for the completed batch.
• Finalize unreferenced tombstones without running surface healing statements.
• Heal survivors one path at a time, preserving the existing lock scope.
• Keep each source update and candidate lease restoration in the same transaction.
• Run one confirmation sweep, then rearm candidates that remain referenced instead of spinning.

The batch remains capped at 100 candidates. There is no migration or API change.

Validation

Production validation of this exact patch:

• 9 hourly runs processed 900 candidates.
• All 900 were deleted, with zero heals, retries, or task errors.
• The matched eight-run average fell from 127.3 seconds to 68.2 seconds, a 46.4% reduction.
• In a fresh 100-candidate sample, batch finalization and rearm took 298.6 ms with 45,592 shared-block hits.
• No guarded or legacy healing statement ran in that sample.
• No tombstones or expired leases were left behind.

The exact implementation also passed the Docs hygiene, Go, and Web GitHub Actions jobs on fork PR #42.

The generated DELETE statements parsed and passed plain EXPLAIN inside a read-only transaction. EXPLAIN ANALYZE was not used because it would execute the deletes.

Added coverage for batch healing, lease rollback, visible-reference rearm, retracking before and during the surface sweep, and the SQL shape of the materialized reference check.

Local checks:

git diff --check upstream/main...HEAD
make verify-local-paths

Both pass. Database-backed tests skip in GitHub Actions unless SILO_TEST_DATABASE_URL is available.

Risks

Object deletion now precedes the shared reference sweep for a bounded batch of up to 100 candidates. This widens the delete-to-heal window within that batch. The final under-lock check still runs before each object deletion, and survivors still go through the guarded one-path heal.

The tombstone refreshes its lease once, so the 15-minute lease must cover the remaining object deletions and finalization. Observed runs completed in 63–75 seconds.

No schema, migration, configuration, compatibility, or security change is involved.

AI Disclosure

• Tool(s): OpenAI Codex
• Model(s): GPT-5
• Involvement: AI-assisted
• Adversarial review: Reviewed the upstream-only diff for lock ordering, retry behavior, retracking, lease rollback, and race conditions. No material findings. Two pre-existing theoretical races remain outside this change: a same-path trigger deadlock and an untracked writer after the final reference snapshot. Neither has a deterministic reproduction; this change retains the under-lock check and guarded final sweep.

Checklist

• I read and can explain the complete diff.
• This pull request addresses one concern.

Summary by CodeRabbit

• Bug Fixes
  • Improved cleanup of outdated artwork revisions while preserving revisions that become referenced or tracked again.
  • Added safeguards to prevent active artwork paths from being removed during concurrent updates.
  • Improved recovery when cleanup encounters temporary failures, with targeted retries and rollback protection.
  • Enhanced batch processing so one failed cleanup does not interrupt processing of other revisions.
• Reliability
  • Added broader coverage for reference changes, retries, and concurrent artwork updates.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b4dd6b9e-260f-45ee-bf9a-c35a7e6ab2a8

📥 Commits

Reviewing files that changed from the base of the PR and between cfaab97 and 955fb4c.

📒 Files selected for processing (2)

• internal/metadata/artwork_revision_gc.go
• internal/metadata/artwork_revision_gc_test.go

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.

---

📝 Walkthrough

Walkthrough

Artwork revision GC now separates tombstone creation from reference healing and finalization. It processes references in batches, uses lease-aware transactions, retries incomplete work, and preserves re-tracked revisions.

Changes

Artwork revision garbage collection

Layer / File(s)	Summary
Tombstone processing and batch orchestration internal/metadata/artwork_revision_gc.go	Candidate processing now returns pending healing work. Deletion commits a durable tombstone before healing. Batch processing tracks healed, finalized, and rearmed candidates.
Batched healing and guarded finalization internal/metadata/artwork_revision_gc.go	Reference paths are checked in batches. Lease-aware transactions heal re-referenced paths. Guarded finalization and targeted retries handle races and failures.
Healing and race-condition validation internal/metadata/artwork_revision_gc_test.go	Tests cover SQL shape, batch continuation, rollback, retries, reference rearming, and retracking before or during healing.

Estimated code review effort: 4 (Complex) | ~60 minutes

Merge Risk: ⚪ Minimal · up to 955fb

This PR reduces unnecessary background artwork cleanup work while preserving the existing deletion and healing safeguards. No actionable merge-blocking risk remains beyond normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant GC as Artwork revision GC
  participant DB as PostgreSQL
  participant Healer as Healing transaction
  participant Finalizer as Tombstone finalizer

  GC->>DB: Mark candidate with deletion tombstone
  DB-->>GC: Return pending heal work
  GC->>DB: Query referenced paths in batch
  DB-->>GC: Return referenced paths
  GC->>Healer: Heal paths with lease and tombstone checks
  Healer->>DB: Update reference surfaces
  DB-->>Healer: Commit or roll back
  GC->>Finalizer: Finalize remaining tombstones
  Finalizer->>DB: Guard finalization and rearm survivors
``

</details>

<!-- walkthrough_end -->
<!-- pre_merge_checks_walkthrough_start -->

<details>
<summary>🚥 Pre-merge checks | ✅ 5</summary>

<details>
<summary>✅ Passed checks (5 passed)</summary>

|         Check name         | Status   | Explanation                                                                                               |
| :------------------------: | :------- | :-------------------------------------------------------------------------------------------------------- |
|      Description Check     | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled.                                               |
|         Title check        | ✅ Passed | The title clearly and concisely describes the main optimization: avoiding unnecessary artwork GC healing. |
|     Docstring Coverage     | ✅ Passed | Docstring check was indeterminate for this PR — some files could not be analyzed in time. Not blocking.   |
|     Linked Issues check    | ✅ Passed | Check skipped because no linked issues were found for this pull request.                                  |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request.                                  |

</details>

</details>

<!-- pre_merge_checks_walkthrough_end -->
<!-- finishing_touch_checkbox_start -->

<details>
<summary>✨ Finishing Touches</summary>

<details>
<summary>🧪 Generate unit tests (beta)</summary>

- [ ] <!-- {"checkboxId": "f47ac10b-58cc-4372-a567-0e02b2c3d479", "radioGroupId": "utg-output-choice-group-unknown_comment_id"} -->   Create PR with unit tests

</details>

</details>

<!-- finishing_touch_checkbox_end -->
<!-- tips_start -->

---

Thanks for using [CodeRabbit](https://coderabbit.ai?utm_source=oss&utm_medium=github&utm_campaign=Silo-Server/silo-server&utm_content=735)! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

<details>
<summary>❤️ Share</summary>

- [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai)
- [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai)
- [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai)
- [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)

</details>


<sub>Comment `@coderabbitai help` to get the list of available commands.</sub>

<!-- tips_end -->

Loading