Treat label crops as content, not cache (#4926)

A crop is a screenshot of the pano canvas taken in the labeler's browser
as the label was placed, and nothing in this app regenerates one. The
Street View Static still used as a fallback elsewhere is a different,
smaller image of a pano the provider must still serve — and roughly half
the labels on prod sit on panos already marked expired.

So the guardrails move the crop directory to the fatal tier alongside the
pano and story-media directories: a stage that would place it inside the
tree `sbt clean stage` deletes refuses to boot, and a signed crop URL
whose file has vanished logs at ERROR. Cached share previews are now the
only entry that still rebuilds on demand.

Every deployed stage already points SIDEWALK_IMAGES_DIR outside the build
tree, and CI's e2e-smoke job exports it, so no stage newly fails the
check.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: jonfroehlich

app/controllers/ImageController.scala

@@ -140,7 +140,7 @@ class ImageController @Inject() (
           case None =>
             // Reaching here means the file was on disk when this URL was signed (backupImageUrl and
             // getBackupImageMetadata both check first) and is gone within the signature's ~75-minute life. For an
-            // expired pano this store holds the only copy left anywhere, so say so (#4926) — the 404 stays bare.
+            // pano the provider no longer serves nothing can re-fetch it, so say so (#4926) — the 404 stays bare.
             lostMediaLog.reportMissing(
               "pano",
               panoId,
@@ -199,8 +199,9 @@ class ImageController @Inject() (
           Future.successful(Ok.sendFile(file, inline = true).as("image/png"))
         } else {
           // Same signed-URL reasoning as serveBackupImage above: cropUrl only signs a crop it just saw on disk, so a
-          // miss here is a file that vanished. A crop can be re-cut from pano imagery, so this is the warning tier.
-          lostMediaLog.reportMissing("crop", s"$labelType/$labelId", file.getAbsolutePath, irreplaceable = false)
+          // miss here is a file that vanished — and nothing recreates it: the crop was captured in the labeler's
+          // browser at labeling time, from a pano the provider may no longer serve.
+          lostMediaLog.reportMissing("crop", s"$labelType/$labelId", file.getAbsolutePath, irreplaceable = true)
           Future.successful(NotFound("Crop image not found"))
         }
     }

app/modules/PersistentMediaDirCheck.scala

@@ -23,10 +23,10 @@ import scala.util.{Failure, Success, Try}
  * incomplete file (the #4925 failure) would disarm the guard exactly when it is needed. Dev and test runs skip it;
  * there the application root is the hand-managed repo checkout, where the relative defaults are the point.
  *
- * A directory holding irreplaceable bytes — user uploads, or our only copies of provider-expired imagery — is
- * **fatal**: refusing to boot is far cheaper than accepting content we already know will be destroyed, and the test
- * stage redeploys on every push to `develop` while prod waits for a release tag, so a missing variable surfaces on
- * test long before it can reach prod. The rest is derived data whose loss costs rebuild time, so it logs and lets
+ * A directory holding irreplaceable bytes — user uploads, label crops, or imagery that cannot be re-fetched — is
+ * **fatal**: refusing to boot is far cheaper than accepting content we already know will be destroyed,
+ * and the test stage redeploys on every push to `develop` while prod waits for a release tag, so a missing variable
+ * surfaces on test long before it can reach prod. Cached share previews rebuild on demand, so that one logs and lets
  * the app run.
  */
 @Singleton
@@ -59,8 +59,8 @@ object PersistentMediaDirCheck {
    * @param key           Config key naming the directory.
    * @param envVar        Environment variable a deployment sets it with, named in the failure message so the fix
    *                      doesn't require reading the config.
-   * @param irreplaceable Whether it holds bytes no rebuild can recreate — user uploads, or the only surviving copy
-   *                      of provider-expired imagery. These form the fatal tier; the rest only log.
+   * @param irreplaceable Whether it holds bytes no rebuild can recreate. These form the fatal tier; the rest only
+   *                      log.
    */
   case class PersistentDir(key: String, envVar: String, irreplaceable: Boolean)
 
@@ -79,14 +79,17 @@ object PersistentMediaDirCheck {
   def arms(environment: Environment): Boolean = environment.mode == Mode.Prod
 
   val persistentDirs: Seq[PersistentDir] = Seq(
-    // Crops and share previews are derived: a crop can be re-cut from pano imagery and a share preview rebuilds on
-    // demand, so losing them costs rebuild time, not content.
-    PersistentDir("cropped.image.directory", "SIDEWALK_IMAGES_DIR", irreplaceable = false),
-    PersistentDir("share.image.directory", "SIDEWALK_SHARE_IMAGES_DIR", irreplaceable = false),
-    // The self-hosted pano store backs up GSV imagery Google has already expired (pano_data.has_backup) — for those
-    // panos it is the only copy left anywhere, as unrecoverable as a user upload.
+    // A crop is a screenshot of the pano canvas taken in the labeler's browser as the label was placed
+    // (Canvas.saveCanvasScreenshot); nothing here rebuilds one, and the Static API still we fall back to is a
+    // different image that only exists while the provider still serves that pano — about half the labels on prod sit
+    // on panos already marked expired.
+    PersistentDir("cropped.image.directory", "SIDEWALK_IMAGES_DIR", irreplaceable = true),
+    // Locally stored pano imagery (pano_data.has_backup) the providers no longer serve, so it cannot be re-fetched.
     PersistentDir("pano.images.directory", "SIDEWALK_PANO_DIR", irreplaceable = true),
-    PersistentDir("story.media.directory", "SIDEWALK_STORY_MEDIA_DIR", irreplaceable = true)
+    PersistentDir("story.media.directory", "SIDEWALK_STORY_MEDIA_DIR", irreplaceable = true),
+    // Share previews are the one cache here: each rebuilds on demand from the label's crop, or from a Street View
+    // still when the crop is gone.
+    PersistentDir("share.image.directory", "SIDEWALK_SHARE_IMAGES_DIR", irreplaceable = false)
   )
 
   /**

conf/application.conf

@@ -265,13 +265,15 @@ custom.news.ribbon.link = null
 # at boot in prod mode, and every consumer resolves its path through service.MediaDirs so the check can't drift from
 # the write paths.
 
-# Directory to store cropped images.
+# Directory for label crops, organized as <dir>/<city-id>/<labelType>/crop_<labelId>.png. Each crop is a screenshot
+# of the pano canvas taken in the labeler's browser as the label was placed, and nothing here rebuilds one.
+# Irreplaceable, like the pano and story dirs.
 cropped.image.directory = ".crops"
 cropped.image.directory = ${?SIDEWALK_IMAGES_DIR}
 
-# Directory containing self-hosted pano images, organized as <dir>/<city-id>/<panoId[0:2]>/<panoId>.<ext>. These back
-# up GSV imagery Google has already expired (pano_data.has_backup), so for those panos this holds the only copy
-# anywhere — irreplaceable, like the story media below.
+# Directory containing self-hosted pano images, organized as <dir>/<city-id>/<panoId[0:2]>/<panoId>.<ext>. Holds
+# imagery the providers no longer serve (pano_data.has_backup), which cannot be re-fetched — irreplaceable, like the
+# story media below.
 pano.images.directory = ".panos"
 pano.images.directory = ${?SIDEWALK_PANO_DIR}
 

docs/deployment-and-stages.md

@@ -289,10 +289,14 @@ outside the build tree** via its environment variable (a variable that is set bu
 | Config key | Env var | Holds | Missing on a deployed stage |
 |---|---|---|---|
 | `story.media.directory` | `SIDEWALK_STORY_MEDIA_DIR` | User-uploaded story photos (**irreplaceable**) | **App refuses to start** |
-| `pano.images.directory` | `SIDEWALK_PANO_DIR` | Self-hosted pano store — the only copies of GSV imagery Google has expired (**irreplaceable**) | **App refuses to start** |
-| `cropped.image.directory` | `SIDEWALK_IMAGES_DIR` | Label crops (re-derivable from pano imagery) | Error logged at boot |
+| `pano.images.directory` | `SIDEWALK_PANO_DIR` | Locally stored pano imagery the app serves itself (**irreplaceable**) | **App refuses to start** |
+| `cropped.image.directory` | `SIDEWALK_IMAGES_DIR` | Label crops — browser captures of the pano as it was labeled, with no rebuild path (**irreplaceable**) | **App refuses to start** |
 | `share.image.directory` | `SIDEWALK_SHARE_IMAGES_DIR` | Cached social-share previews (regenerable) | Error logged at boot |
 
+Crops sit in the fatal tier because nothing in this app regenerates one: `/saveImage` stores a canvas screenshot the
+labeler's browser took as the label was placed, and the Street View Static still used as a fallback elsewhere is a
+different, smaller image of a pano the provider must still serve — which it often no longer does.
+
 `PersistentMediaDirCheck` enforces this at boot in **prod mode** — what every staged binary runs in — so it covers
 every deployed stage *and* a staged binary run by hand (export the four variables to `/tmp` paths for that; CI's
 `e2e-smoke` job does exactly this). It deliberately does not key on `ENV_TYPE`: that variable arrives through the

test/controllers/ImageControllerSpec.scala

@@ -196,8 +196,9 @@ class ImageControllerSpec extends PlaySpec with AnonSession with GuiceOneAppPerS
         }
 
         events must have size 1
-        // A crop can be re-cut from pano imagery, so it is the warning tier rather than the paging one.
-        events.head.getLevel mustBe Level.WARN
+        // A crop exists in one place only — it was captured in the labeler's browser as the label was placed — so
+        // its disappearance is the error tier, not a rebuild cost.
+        events.head.getLevel mustBe Level.ERROR
         val message = events.head.getFormattedMessage
         message must include("crop")
         message must include(syntheticLabelId.toString)
@@ -207,8 +208,8 @@ class ImageControllerSpec extends PlaySpec with AnonSession with GuiceOneAppPerS
     }
 
     "answer a signed pano URL with no backup image with a plain 404, every time it is asked" in {
-      // For a pano whose source imagery has expired this store holds the only copy left anywhere, so the loss is the
-      // error tier. The log deduplicates the repeat (LostMediaLogSpec pins that); the responses must not.
+      // Nothing can re-fetch a pano the provider no longer serves, so the loss is the error tier. The log
+      // deduplicates the repeat (LostMediaLogSpec pins that); the responses must not.
       val panoId = "sidewalkSpecNoSuchPano4926"
       val url    = signingService.signedUrl(s"/backupImage/$panoId")
 

test/js/healthMediaPanel.test.js

@@ -115,8 +115,8 @@ describe('HealthPage media storage panel', () => {
         });
 
         it('says of a rebuildable directory that losing it costs a rebuild, not content', async () => {
-            const crops = { ...OK_DIR, key: 'cropped.image.directory', irreplaceable: false };
-            const { dirs } = await render({ directories: [crops], enforced: true, story_media: null });
+            const shareImages = { ...OK_DIR, key: 'share.image.directory', irreplaceable: false };
+            const { dirs } = await render({ directories: [shareImages], enforced: true, story_media: null });
 
             expect(dirs).toContain('Yes — rebuilt on demand');
         });

test/modules/PersistentMediaDirCheckSpec.scala

@@ -9,8 +9,8 @@ import scala.io.Source
 import scala.util.Using
 
 /**
- * The deployment contract behind the media directories: anything irreplaceable — user uploads, our only copies of
- * provider-expired panos — has to land outside the build output tree, because a deploy deletes that whole tree
+ * The deployment contract behind the media directories: anything irreplaceable — user uploads, imagery that cannot
+ * be re-fetched — has to land outside the build output tree, because a deploy deletes that whole tree
  * (`sbt clean`) and rebuilds it (#4925).
  *
  * Nothing else can catch a violation. The configuration that lost a story photo was correct in dev, correct in CI,
@@ -74,11 +74,12 @@ class PersistentMediaDirCheckSpec extends PlaySpec {
   }
 
   "the fatal set" should {
-    // Refusing to boot is only justified for bytes no rebuild can recreate: the story photos users gave us, and the
-    // self-hosted pano store — it backs up GSV imagery Google has already expired, so for those panos it is the only
-    // copy anywhere. Crops and share previews re-derive from them, so they must stay warn-only.
+    // Refusing to boot is only justified for bytes no rebuild can recreate: the story photos users gave us, imagery
+    // the providers no longer serve, and the label crops, each captured once in a labeler's browser. Cached share
+    // previews rebuild on demand, so that one must stay warn-only.
     "be exactly the irreplaceable directories" in {
-      persistentDirs.filter(_.irreplaceable).map(_.key) mustBe Seq("pano.images.directory", "story.media.directory")
+      persistentDirs.filter(_.irreplaceable).map(_.key) mustBe
+        Seq("cropped.image.directory", "pano.images.directory", "story.media.directory")
     }
 
     // The failure message tells the operator which variable to set. If this mapping drifts from application.conf,

test/service/LostMediaLogSpec.scala

@@ -82,7 +82,7 @@ class LostMediaLogSpec extends PlaySpec {
       // The tiering is the same call PersistentMediaDirCheck makes, and it is what decides whether anyone is paged.
       val error = captured(_.reportMissing("pano", "abc", "/srv/panos/abc.jpg", irreplaceable = true))
       error.head.getLevel mustBe Level.ERROR
-      val warn = captured(_.reportMissing("crop", "CurbRamp/7", "/srv/crops/crop_7.png", irreplaceable = false))
+      val warn = captured(_.reportMissing("share_image", "7", "/srv/share/share_7.jpg", irreplaceable = false))
       warn.head.getLevel mustBe Level.WARN
     }
 

test/service/MediaIntegritySpec.scala

@@ -87,30 +87,31 @@ class MediaIntegritySpec extends PlaySpec {
     }
   }
 
-  private val storyDir = persistentDirs.find(_.irreplaceable).value
+  private val irreplaceableDir = persistentDirs.find(_.irreplaceable).value
 
   // The permission branches can't be provoked through the filesystem from a suite that runs as root — which CI and
   // the dev container both do, and where chmod 000 still reads and writes fine — so they are pinned on the rules
   // themselves.
   "dirStatus" should {
     "call a directory this process cannot read bad, since nothing in it can be verified" in {
       val probe  = MediaIntegrity.DirProbe(exists = true, readable = false, writable = true)
-      val status = MediaIntegrity.dirStatus(storyDir, "/srv/media", probe, None, enforced = true)
+      val status = MediaIntegrity.dirStatus(irreplaceableDir, "/srv/media", probe, None, enforced = true)
       status.status mustBe "not_readable"
       status.severity mustBe "bad"
-      status.detail.value must include(storyDir.envVar)
+      status.detail.value must include(irreplaceableDir.envVar)
     }
 
     "call a directory this process cannot write to bad, since uploads will fail against it" in {
       val probe  = MediaIntegrity.DirProbe(exists = true, readable = true, writable = false)
-      val status = MediaIntegrity.dirStatus(storyDir, "/srv/media", probe, None, enforced = true)
+      val status = MediaIntegrity.dirStatus(irreplaceableDir, "/srv/media", probe, None, enforced = true)
       status.status mustBe "not_writable"
       status.severity mustBe "bad"
     }
 
     "report an unsafe directory before either permission, since a deploy deleting it outranks both" in {
       val probe  = MediaIntegrity.DirProbe(exists = true, readable = false, writable = false)
-      val status = MediaIntegrity.dirStatus(storyDir, "/srv/media", probe, Some("in the wipe zone"), enforced = true)
+      val status =
+        MediaIntegrity.dirStatus(irreplaceableDir, "/srv/media", probe, Some("in the wipe zone"), enforced = true)
       status.status mustBe "unsafe"
     }
   }