fix: include rendered_at in 304 cache check so re-renders invalidate

After re-cropping an image, the renderer regenerates the .bin and
advances the asset's rendered_at, but the device's 304 short-circuit
still matched on (image_id, orientation) only — so the device kept
serving the old upside-down/stale bytes from its local cache despite
the server having freshly-rendered correct ones.

Adds device.current_rendered_at, populated whenever a 200 response is
served, and tightens the 304 condition to require all three (image id,
orientation, rendered_at) to match. The asset lookup now happens before
the 304 check so its rendered_at is in scope for the comparison.

No firmware change — this is server-side cache logic. Existing devices
get null current_rendered_at after the migration; their next poll falls
through 304 and re-fetches once, then the cache is in sync.

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

migrations/Version20260507230000.php

@@ -0,0 +1,27 @@
+<?php
+
+declare(strict_types=1);
+
+namespace DoctrineMigrations;
+
+use Doctrine\DBAL\Schema\Schema;
+use Doctrine\Migrations\AbstractMigration;
+
+final class Version20260507230000 extends AbstractMigration
+{
+    public function getDescription(): string
+    {
+        return 'Add device.current_rendered_at so 304 cache check can detect re-renders';
+    }
+
+    public function up(Schema $schema): void
+    {
+        $this->addSql('ALTER TABLE device ADD current_rendered_at TIMESTAMP(0) WITHOUT TIME ZONE DEFAULT NULL');
+        $this->addSql("COMMENT ON COLUMN device.current_rendered_at IS '(DC2Type:datetime_immutable)'");
+    }
+
+    public function down(Schema $schema): void
+    {
+        $this->addSql('ALTER TABLE device DROP COLUMN current_rendered_at');
+    }
+}

src/Controller/DeviceImageController.php

@@ -68,23 +68,9 @@ class DeviceImageController extends AbstractController
             return $r;
         }
 
-        // 304: device already has this image — skip the binary transfer and redraw.
+        // Asset lookup is needed before the 304 check so we can compare its
-        // Both the image and the orientation it was last rendered at must match;
+        // rendered_at timestamp — otherwise a re-cropped image (same id, same
-        // otherwise the device's cached .bin is stale and we must re-send.
+        // orientation, new bytes) would be incorrectly served as 304.
-        if ($image->getId() === $currentImageId
-            && $device->getCurrentImageOrientation() === $device->getOrientation()) {
-            $this->logger->info('device.poll.no_change', [
-                'device_id'   => $device->getId(),
-                'mac'         => $mac,
-                'image_id'    => $image->getId(),
-                'interval_ms' => $intervalMs,
-            ]);
-            $r = new Response(null, Response::HTTP_NOT_MODIFIED);
-            $r->headers->set('X-Image-Id',    (string) $image->getId());
-            $r->headers->set('X-Interval-Ms', (string) $intervalMs);
-            return $r;
-        }
-
         $asset = $em->getRepository(RenderedAsset::class)->findOneBy([
             'image'       => $image,
             'deviceModel' => $device->getModel(),
@@ -105,6 +91,27 @@ class DeviceImageController extends AbstractController
             return $r;
         }
 
+        // 304: device already has this image — skip the binary transfer and redraw.
+        // The image id, the orientation we last served at, AND the asset's
+        // rendered_at must all match. A re-render (e.g. after re-crop) advances
+        // rendered_at, so the device's cached bytes are stale and we re-send.
+        $renderedAt = $asset->getRenderedAt();
+        if ($image->getId() === $currentImageId
+            && $device->getCurrentImageOrientation() === $device->getOrientation()
+            && $renderedAt !== null
+            && $device->getCurrentRenderedAt()?->getTimestamp() === $renderedAt->getTimestamp()) {
+            $this->logger->info('device.poll.no_change', [
+                'device_id'   => $device->getId(),
+                'mac'         => $mac,
+                'image_id'    => $image->getId(),
+                'interval_ms' => $intervalMs,
+            ]);
+            $r = new Response(null, Response::HTTP_NOT_MODIFIED);
+            $r->headers->set('X-Image-Id',    (string) $image->getId());
+            $r->headers->set('X-Interval-Ms', (string) $intervalMs);
+            return $r;
+        }
+
         $binPath = $this->projectDir . '/' . $asset->getFilePath();
         if (!file_exists($binPath)) {
             $this->logger->error('device.poll.file_missing', [
@@ -118,10 +125,10 @@ class DeviceImageController extends AbstractController
             return $r;
         }
 
-        // Record the orientation we're serving the image at so the next poll's
+        // Record the orientation and rendered_at we're serving at so the next
-        // 304 check can detect a flip and force a re-fetch. Flushed via the
+        // poll's 304 check can detect a flip or a re-render and force a re-fetch.
-        // controller's EM (markSeen() above already dirties the row).
         $device->setCurrentImageOrientation($device->getOrientation());
+        $device->setCurrentRenderedAt($renderedAt);
         $em->flush();
 
         $this->logger->info('device.poll.served', [

src/Entity/Device.php

@@ -74,6 +74,15 @@ class Device
     #[ORM\Column(nullable: true, enumType: Orientation::class)]
     private ?Orientation $currentImageOrientation = null;
 
+    /**
+     * RenderedAsset.rendered_at timestamp at the moment we last served this
+     * device its current image. Lets the 304 cache check detect a re-render
+     * (e.g. user re-cropped the image) — the image id and orientation may
+     * still match, but the bytes have changed and we must re-send.
+     */
+    #[ORM\Column(nullable: true)]
+    private ?\DateTimeImmutable $currentRenderedAt = null;
+
     public function __construct()
     {
         $this->linkedAt = new \DateTimeImmutable();
@@ -122,4 +131,7 @@ class Device
 
     public function getCurrentImageOrientation(): ?Orientation { return $this->currentImageOrientation; }
     public function setCurrentImageOrientation(?Orientation $o): static { $this->currentImageOrientation = $o; return $this; }
+
+    public function getCurrentRenderedAt(): ?\DateTimeImmutable { return $this->currentRenderedAt; }
+    public function setCurrentRenderedAt(?\DateTimeImmutable $t): static { $this->currentRenderedAt = $t; return $this; }
 }

tests/Functional/Controller/DeviceImageControllerTest.php

@@ -66,7 +66,8 @@ class DeviceImageControllerTest extends AppWebTestCase
                 ->setDeviceModel(DeviceModel::V1)
                 ->setOrientation(Orientation::Landscape)
                 ->setStatus(RenderStatus::Ready)
-                ->setFilePath(self::BIN_PATH);
+                ->setFilePath(self::BIN_PATH)
+                ->setRenderedAt(new \DateTimeImmutable());
             $this->em()->persist($asset);
         }
 
@@ -166,10 +167,13 @@ class DeviceImageControllerTest extends AppWebTestCase
         $image   = $setup['image'];
         $imageId = $image->getId();
 
-        $device->setLockedImage($image);
         // Simulate the device having already received this image at the current
-        // orientation — the 304 path now requires this to match too.
+        // orientation and rendered_at — the 304 path now requires all three
+        // (image id, orientation, rendered_at) to match.
+        $asset = $this->em()->getRepository(RenderedAsset::class)->findOneBy(['image' => $image]);
+        $device->setLockedImage($image);
         $device->setCurrentImageOrientation(Orientation::Landscape);
+        $device->setCurrentRenderedAt($asset->getRenderedAt());
         $this->em()->flush();
 
         $this->client->request('GET', '/api/device/' . self::MAC . '/image', [], [], [
@@ -211,6 +215,34 @@ class DeviceImageControllerTest extends AppWebTestCase
         $this->assertResponseStatusCodeSame(200);
     }
 
+    public function test_re_render_returns_200_even_when_image_id_and_orientation_match(): void
+    {
+        $setup    = $this->createTestSetup();
+        $deviceId = $setup['device']->getId();
+        $imageId  = $setup['image']->getId();
+
+        // Seed: device receives the image once, server stores currentRenderedAt.
+        $this->client->request('GET', '/api/device/' . self::MAC . '/image');
+        $this->assertResponseStatusCodeSame(200);
+
+        // Simulate a re-render: the asset's rendered_at advances (e.g. user
+        // re-cropped the image, RenderImageMessageHandler ran again).
+        $this->em()->clear();
+        $asset = $this->em()->getRepository(RenderedAsset::class)->findOneBy([
+            'image'       => $this->em()->find(Image::class, $imageId),
+            'orientation' => Orientation::Landscape,
+        ]);
+        $asset->setRenderedAt(new \DateTimeImmutable('+1 minute'));
+        $this->em()->flush();
+
+        // Same image id, same orientation — but the bytes have changed, so
+        // the 304 cache must invalidate and the device must re-fetch.
+        $this->client->request('GET', '/api/device/' . self::MAC . '/image', [], [], [
+            'HTTP_X-Current-Image-Id' => (string) $imageId,
+        ]);
+        $this->assertResponseStatusCodeSame(200);
+    }
+
     public function test_poll_advances_current_image(): void
     {
         $setup    = $this->createTestSetup();