Use EXIF OffsetTimeOriginal for accurate saleStart, don't guess otherwise#401
Conversation
…ovides it DateTimeOriginal alone has no timezone, so we were assuming UTC for every photo regardless of where it was actually taken. Some cameras/phones (EXIF 2.31+) also write OffsetTimeOriginal, which lets us compute the true UTC capture instant instead of guessing. Deliberately not falling back to a self-reported or profile-configured timezone when OffsetTimeOriginal is absent: saleStart is written on-chain, and an unverified guess is worse there than the existing, honest fallback to upload time. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The previous commit still fell back to treating DateTimeOriginal as UTC when OffsetTimeOriginal was absent — but assuming a 0 offset is just as much a guess as assuming any other timezone. Since saleStart is written on-chain, skip EXIF entirely in that case and let the caller fall back to upload time, consistent with having no capture-date signal at all. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds a new ChangesEXIF offset-aware capture date extraction
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/lib/telegram/chat/__tests__/extractExifCaptureDate.test.tsOops! Something went wrong! :( ESLint: 9.39.4 TypeError: Converting circular structure to JSON ... [truncated 446 characters] ... c/dist/eslintrc.cjs:3261:25) src/lib/telegram/chat/__tests__/parseExifOffsetMs.test.tsOops! Something went wrong! :( ESLint: 9.39.4 TypeError: Converting circular structure to JSON ... [truncated 446 characters] ... c/dist/eslintrc.cjs:3261:25) src/lib/telegram/chat/extractExifCaptureDate.tsOops! Something went wrong! :( ESLint: 9.39.4 TypeError: Converting circular structure to JSON ... [truncated 446 characters] ... c/dist/eslintrc.cjs:3261:25)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
src/lib/telegram/chat/__tests__/extractExifCaptureDate.test.ts (1)
31-55: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a test case for
+00:00(UTC) offset.The suite covers positive (
+09:00) and negative (-05:00) offsets but not the UTC zero-offset case. A+00:00offset should produce a valid timestamp (equal toDate.UTC(...) / 1000), notundefined. This would catch a regression where0is mistakenly treated as a missing offset at the integration level.✅ Suggested test addition
it('subtracts a negative OffsetTimeOriginal correctly', async () => { mockParse.mockResolvedValue({ DateTimeOriginal: '2026:07:09 23:32:10', OffsetTimeOriginal: '-05:00', }); const result = await extractExifCaptureDate(Buffer.from('jpeg-bytes')); expect(result).toBe( (Date.UTC(2026, 6, 9, 23, 32, 10) + 5 * 60 * 60_000) / 1000 ); }); + + it('returns a valid timestamp for +00:00 (UTC) offset', async () => { + mockParse.mockResolvedValue({ + DateTimeOriginal: '2026:07:09 23:32:10', + OffsetTimeOriginal: '+00:00', + }); + + const result = await extractExifCaptureDate(Buffer.from('jpeg-bytes')); + + expect(result).toBe(Date.UTC(2026, 6, 9, 23, 32, 10) / 1000); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/telegram/chat/__tests__/extractExifCaptureDate.test.ts` around lines 31 - 55, Add a test in extractExifCaptureDate.test.ts for the extractExifCaptureDate path when parse returns DateTimeOriginal with OffsetTimeOriginal set to +00:00. Assert the result equals Date.UTC(...) / 1000 and is not undefined, so the UTC-zero case is covered alongside the existing positive and negative offset tests.src/lib/telegram/chat/extractExifCaptureDate.ts (1)
2-2: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
@/*path alias instead of relative import.Per coding guidelines, imports pointing to
./src/*should use the@/*alias.♻️ Suggested change
-import parseExifOffsetMs from './parseExifOffsetMs'; +import parseExifOffsetMs from '`@/lib/telegram/chat/parseExifOffsetMs`';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/telegram/chat/extractExifCaptureDate.ts` at line 2, The import in extractExifCaptureDate should use the project path alias instead of a relative reference. Update the parseExifOffsetMs import to follow the `@/`* convention used by the codebase, keeping the symbol name the same while changing only the import path style.Source: Coding guidelines
src/lib/telegram/chat/__tests__/parseExifOffsetMs.test.ts (2)
2-2: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
@/*path alias instead of relative import.Per coding guidelines, imports pointing to
./src/*should use the@/*alias.♻️ Suggested change
-import parseExifOffsetMs from '../parseExifOffsetMs'; +import parseExifOffsetMs from '`@/lib/telegram/chat/parseExifOffsetMs`';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/telegram/chat/__tests__/parseExifOffsetMs.test.ts` at line 2, The test file currently imports parseExifOffsetMs using a relative path; update the import in parseExifOffsetMs.test.ts to use the `@/`* alias instead of ../parseExifOffsetMs. Keep the same symbol name, and make sure all imports in this area follow the alias-based path convention used by the project.Source: Coding guidelines
4-28: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a test case for
+00:00(UTC offset).The suite doesn't cover the
+00:00case, which returns0— a valid offset distinct fromundefined. The downstream consumer (extractExifCaptureDate.ts) correctly checksoffsetMs === undefinedrather than a falsy check, but a test here would guard against a future regression where someone switches to!offsetMsand silently breaks UTC photos.✅ Suggested test addition
it('returns undefined for a non-string value', () => { expect(parseExifOffsetMs(540)).toBeUndefined(); }); + + it('returns 0 for a +00:00 (UTC) offset, not undefined', () => { + expect(parseExifOffsetMs('+00:00')).toBe(0); + }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/telegram/chat/__tests__/parseExifOffsetMs.test.ts` around lines 4 - 28, Add a test in parseExifOffsetMs.test.ts to cover the UTC case `+00:00`, since `parseExifOffsetMs` should return `0` and not `undefined`. Update the existing `parseExifOffsetMs` test suite by adding a case alongside the other offset parsing checks, and make sure it explicitly asserts zero is returned for `+00:00` to protect callers like `extractExifCaptureDate` from regressions that treat `0` as falsy.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/lib/telegram/chat/__tests__/extractExifCaptureDate.test.ts`:
- Around line 31-55: Add a test in extractExifCaptureDate.test.ts for the
extractExifCaptureDate path when parse returns DateTimeOriginal with
OffsetTimeOriginal set to +00:00. Assert the result equals Date.UTC(...) / 1000
and is not undefined, so the UTC-zero case is covered alongside the existing
positive and negative offset tests.
In `@src/lib/telegram/chat/__tests__/parseExifOffsetMs.test.ts`:
- Line 2: The test file currently imports parseExifOffsetMs using a relative
path; update the import in parseExifOffsetMs.test.ts to use the `@/`* alias
instead of ../parseExifOffsetMs. Keep the same symbol name, and make sure all
imports in this area follow the alias-based path convention used by the project.
- Around line 4-28: Add a test in parseExifOffsetMs.test.ts to cover the UTC
case `+00:00`, since `parseExifOffsetMs` should return `0` and not `undefined`.
Update the existing `parseExifOffsetMs` test suite by adding a case alongside
the other offset parsing checks, and make sure it explicitly asserts zero is
returned for `+00:00` to protect callers like `extractExifCaptureDate` from
regressions that treat `0` as falsy.
In `@src/lib/telegram/chat/extractExifCaptureDate.ts`:
- Line 2: The import in extractExifCaptureDate should use the project path alias
instead of a relative reference. Update the parseExifOffsetMs import to follow
the `@/`* convention used by the codebase, keeping the symbol name the same while
changing only the import path style.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 584a9e4e-3928-42a3-a0db-541a500b4d87
📒 Files selected for processing (4)
src/lib/telegram/chat/__tests__/extractExifCaptureDate.test.tssrc/lib/telegram/chat/__tests__/parseExifOffsetMs.test.tssrc/lib/telegram/chat/extractExifCaptureDate.tssrc/lib/telegram/chat/parseExifOffsetMs.ts
1 participant
Summary
DateTimeOriginalas UTC whenever timezone info was missing, which is itself an unverified guess (offset 0 is no more justified than any other offset).OffsetTimeOriginal(EXIF 2.31+), use it to compute the true UTC capture instant — this is verified data written by the device at capture time.OffsetTimeOriginalis absent, skip EXIF entirely (even ifDateTimeOriginalis present) and fall back to upload time, same as having no capture-date signal at all.saleStartis written on-chain, so an unverified guess is worse there than an honest fallback.Test plan
pnpm vitest run src/lib/telegram— 41 files / 237 tests passingpnpm lintclean on touched filesOffsetTimeOriginal, so they correctly fall back to upload time (previously they were incorrectly treated as UTC, causing display glitches like the capture date appearing to roll to the next day for viewers in other timezones)Summary by CodeRabbit
Bug Fixes
New Features
+HH:MM/-HH:MMformat.Tests