Turning a Pexels-only hero picker into a provider registry (and the review that caught a license forgery)

I picked this up planning to refactor blog-manager’s hero-image picker from Pexels-only into something that could plug in Unsplash and Openverse later. What I actually got out of it was a decent lesson in why an automated multi-angle review pays for itself even on a “boring” refactor.

What I was trying to do

The picker only ever talked to Pexels: one client, one route, one view. Two more providers (Unsplash, Openverse) were already filed as separate issues, both blocked on this one. The ask was to pull a provider-agnostic layer out from under the Pexels-specific code so those two issues would have something to plug into, without changing anything a user could see - Pexels still had to work exactly the same.

What I built

A small registry module, ImageSearch: a PROVIDERS hash keyed by provider name, each entry holding a configured? check and a build lambda that constructs that provider’s client from app settings. ImageSearch.search fans out to every configured provider, caches each one’s results separately, and interleaves them round-robin so no single provider crowds out the others once there’s more than one.

Pexels::Client#search now returns a normalized hash shape instead of raw Pexels API photo objects - provider, id, urls, credit, license, an extras bucket for provider-specific stuff. The controller actions got renamed from search_pexels/select_pexels_image to search_images/select_image, and a new shared _hero_attribution.html.erb partial replaced two copies of the same “Photo by X on Y” markup.

A decision that came from reading the code, not the issue

The issue’s scope section asked for a migration adding hero_image_source and license columns. I went to write it and found it already existed - an earlier issue, a few days before, had added the exact same columns plus a backfill for existing Pexels heroes. Issue descriptions age; I’d rather catch that by reading db/schema.rb before writing a migration than by writing a duplicate one and having it collide in review.

The issue also specified a hardcoded footer: “Photos provided by Pexels, Unsplash and Openverse.” I built it from ImageSearch.configured_providers instead, since Unsplash and Openverse don’t have real clients yet - crediting two providers that can’t actually return a photo felt worse than a footer that just grows as the registry does.

What the review caught

I ran an 8-angle automated review before merging (line-by-line, removed-behavior audit, cross-file trace, reuse, simplification, efficiency, altitude, conventions), then verified each candidate independently before acting on it. Two of the angles found the same thing from different directions, and it was worse than I’d noticed writing the code: select_image was writing params[:provider] straight into the enum column with no check. The obvious failure is an unhandled 500 on a bad value. The one I hadn’t clocked: since openverse is a valid enum value that also flips on self-hosting, a request could pair provider=openverse with a Pexels image URL and forged license text, and the app would download and commit that image into the public blog repo believing it was legitimately self-hostable. Same bug, much worse blast radius once you follow it one layer further.

A second finding: the new code’s own comment promised a failing provider “never fails the whole search,” but the rescue clause only caught my own ImageSearch::Error hierarchy - a real Pexels timeout or a malformed JSON response would have sailed right past it and 500’d the request anyway. The promise was aspirational, not actually true.

Both got fixed before merge: a whitelist check against Post.hero_image_sources before the update, and a broadened rescue that also catches the transport-level failures Pexels can genuinely throw.

What surprised me

How much sharper “a forged provider param could commit a mislicensed image to the live blog” lands than “an invalid param causes a 500.” Same root cause, same one-line fix, but the review angle that framed it around the actual consequence (license forgery, not just a crash) is the version that would have made me stop and fix it immediately even on a rushed day. Framing the failure scenario concretely, not just naming the missing validation, turned out to matter more than I expected.

Also worth remembering for next time: one of the review’s parallel verifier passes hit a rate limit mid-check, said so explicitly instead of quietly proceeding as if it had gotten a second opinion, and kept going on the strength of its own direct file reads. Small thing, but it’s the difference between a tool you can trust and one you have to double-check.

What’s next

Unsplash and Openverse are the two issues this was built for - they’ll be the first real test of whether the registry’s extension points (the configured/build lambdas, the normalized result shape) hold up past a single provider. The review also flagged that ImageSearch::PROVIDERS and Post::HERO_IMAGE_SOURCE_INFO now both carry a per-provider display name in two separate hashes that have already drifted once (Unsplash/Openverse are in one but not the other) - worth collapsing into one source of truth before a third provider makes it worse.