Scope logbook PDF invalidation to the day that actually changed - #10
Merged
Conversation
Adding a remark to one day dropped every archived PDF the device had, because `invalidateLogbookArchive` deletes the whole `logbook/<deviceId>/` prefix. With a single device in the bucket that is "delete everything", so one remark threw away weeks of renders and left only whatever the nightly Workflow wrote next — the expensive re-rendering this cache exists to avoid. A remark is filed against its own `dateString` and cannot appear in any other day's log, so the new `invalidateLogbookArchiveDay` drops just that one object. The whole-device invalidation is still right for timing points and logbook config, which do rewrite every past day, and is now also called when a device is renamed or its display distance unit changes on the admin devices page. Both are printed into the PDF (the name in its title, the unit on every distance), so archived copies stopped matching the page after either edit and nothing dropped them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KdZht5FMiuDuD4HhKJHupa
There was a problem hiding this comment.
Pull request overview
This PR fixes overly-broad logbook PDF cache invalidation in R2 by scoping deletions to the smallest affected set of archived PDFs, avoiding expensive and unnecessary Browser Rendering re-renders.
Changes:
- Add
invalidateLogbookArchiveDay(env, deviceId, dateString)to delete a single day’s archived PDF key. - Update logbook remark submission to invalidate only the edited day’s PDF instead of the entire device prefix.
- Update admin device updates to invalidate the full device archive only when PDF-affecting device fields (name, display distance unit) actually change, and document the invalidation behavior in the README.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| website/app/routes/date/logbook.tsx | Switch remark edits from whole-device invalidation to day-scoped invalidation. |
| website/app/routes/admin/devices.tsx | Invalidate device PDF archive only when name or display distance unit changes. |
| website/app/logbook/pdfArchive.server.ts | Introduce invalidateLogbookArchiveDay and clarify when to use each invalidation function. |
| Readme.md | Document that scheduled jobs don’t delete R2 objects and point to invalidation/lifecycle rules as causes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+126
to
+130
| await invalidateLogbookArchiveDay( | ||
| getCloudflareContext(context).env, | ||
| deviceId, | ||
| urlDate, | ||
| ); |
Comment on lines
+268
to
+274
| if ( | ||
| before && | ||
| (before.name !== name || | ||
| before.displayDistanceUnit !== displayDistanceUnit) | ||
| ) { | ||
| await invalidateLogbookArchive(getCloudflareContext(context).env, id); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Investigating why the
trackerR2 bucket only holds one object.It isn't the daily workflow
Neither scheduled thing deletes from R2:
daily-logbook-email(website/workers/logbookEmailWorkflow.ts,schedules: ["5 0 * * *"]) only ever callsgetOrRenderLogbookPdf, which doesgetthenput. There is nodeletein it..github/workflows/cloudflare-workers-deploy.yml) runswrangler deploy --keep-varsplusd1 migrations apply. Neither touches bucket objects, and there has never been a second, daily Action in this repo's history.There is exactly one
R2_BUCKET.delete(...)call in the whole codebase:invalidateLogbookArchiveinwebsite/app/logbook/pdfArchive.server.ts.What was actually emptying it
invalidateLogbookArchivedeletes the entirelogbook/{deviceId}/prefix. With a single device that is "empty the bucket", and it was being called for edits that only affect one day — most notably adding a remark on the logbook page. One remark on one day discarded every archived PDF the device had, leaving only whatever the nightly Workflow wrote next. That matches the observed steady state of one object.The archive has been live since 2026-07-26, so roughly twenty days' worth of renders were being thrown away and re-rendered through Browser Rendering, which is the single most expensive thing the app does and exactly what this cache exists to avoid.
Changes
invalidateLogbookArchiveDay(env, deviceId, dateString)deletes one key. A remark is filed against its owndateString(seeLogbookRemarks) and cannot appear in any other day's log, so the remark path inapp/routes/date/logbook.tsxnow uses it.app/routes/admin/devices.tsxnow reads those two fields before the update and invalidates the device's archive only when one of them actually changed.Worth checking outside the repo
If the bucket still drains after this, the remaining candidate is an object lifecycle rule on the
trackerbucket in the Cloudflare dashboard — that is configured outside this repository and code review cannot rule it out. The key of the one surviving object is a good tell: if it is yesterday's date, invalidation was the cause; an older arbitrary date would point at renders never happening in the first place.Testing
npx tsc -bpasses with no new errors. Two pre-existing failures remain inapp/routes/date/timingPoints.tsxandapp/routes/date/timingPointsHistoricComparison.tsx; both reproduce unchanged on the base commit and are untouched by this diff.