feat(geocoding): implement reverse geocoding #157

Enter-tainer · 2025-11-14T17:09:09Z

修改内容：

添加了 reverse geocoding 功能。默认关闭，可以通过config开启。可选 osm, mapbox, auto
build manifest 时，会进行跨worker进程的ratelimit，来避免把osm的reverse geoencoding打爆
build manifest 时，会对有 gps 坐标但是没有 location 信息的图片尝试进行 reverse geoencoding
现在在前端会展示城市信息和完整地址信息。不太确定这个是不是只展示完整地址就行了？可能也有人想只展示城市。不太确定，感觉有点纠结

vercel · 2025-11-14T17:09:16Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
afilmory-demo	Ready	Preview	Comment	Nov 24, 2025 6:53am
afilmory-docs	Ready	Preview	Comment	Nov 24, 2025 6:53am
afilmory-landing	Ready	Preview	Comment	Nov 24, 2025 6:53am

Enter-tainer · 2025-11-14T17:11:06Z

packages/builder/src/photo/geocoding.ts

+  }
+}
+
+async function withInterprocessLock<T>(key: string, fn: () => Promise<T>): Promise<T> {


这里要搞跨worker的rate limit，codex写了个很神奇的文件锁的代码。。不过感觉nodejs好像确实没有正经的跨进程的同步 primitive？

Enter-tainer · 2025-11-15T04:05:14Z

@codex 帮我review一下，中文回复

chatgpt-codex-connector · 2025-11-15T04:09:07Z

Codex Review: Didn't find any major issues. 🎉

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

safedep · 2025-11-15T16:12:27Z

SafeDep Report Summary

No dependency changes detected. Nothing to scan.

_{This report is generated by SafeDep Github App}

Copilot

Pull Request Overview

This PR implements reverse geocoding functionality to automatically extract location information (country, city, and address) from GPS coordinates embedded in photo EXIF data. The feature is disabled by default and supports both Mapbox (commercial, higher rate limits) and Nominatim (free, OpenStreetMap-based) providers with intelligent caching and cross-process rate limiting to respect API constraints.

Key Changes

Adds configurable reverse geocoding with provider selection (Mapbox/Nominatim/auto)
Implements inter-process rate limiting using filesystem-based locks to prevent API abuse across worker processes
Integrates location data processing into the photo pipeline with smart caching to minimize API calls
Updates UI to display city and full address information in the EXIF panel

Reviewed Changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 16 comments.

Show a summary per file

File	Description
`packages/builder/src/types/photo.ts`	Adds `LocationInfo` interface for storing geographic data
`packages/builder/src/types/config.ts`	Adds geocoding configuration options (provider, tokens, URLs)
`packages/builder/src/photo/logger-adapter.ts`	Adds location-specific logger for geocoding operations
`packages/builder/src/photo/image-pipeline.ts`	Integrates location processing step into photo processing pipeline
`packages/builder/src/photo/geocoding.ts`	Implements core geocoding logic with Mapbox/Nominatim providers and rate limiting
`packages/builder/src/photo/data-processors.ts`	Adds `processLocationData` function with caching and provider management
`packages/builder/src/photo/README.md`	Documents geocoding module and usage examples
`packages/builder/src/config/index.ts`	Applies geocoding configuration overrides
`packages/builder/src/config/defaults.ts`	Sets default geocoding configuration (disabled by default)
`packages/builder/src/builder/builder.ts`	Adds `retryMissingLocations` to backfill location data for existing photos
`packages/builder/README.md`	Documents geocoding feature capabilities
`locales/app/*.json`	Adds translation keys for city and address display in multiple languages
`apps/web/src/components/ui/photo-viewer/ExifPanel.tsx`	Displays reverse geocoded location information in the UI
`apps/docs/contents/index.mdx`	Adds geocoding configuration documentation and examples

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-15T16:17:38Z

packages/builder/src/photo/image-pipeline.ts

          ? {
-            type: 'motion-photo',
-            offset: motionPhotoMetadata.motionPhotoOffset,
-            size: motionPhotoMetadata.motionPhotoVideoSize,
-            presentationTimestamp: motionPhotoMetadata.presentationTimestampUs,
-          }
+              type: 'motion-photo',
+              offset: motionPhotoMetadata.motionPhotoOffset,
+              size: motionPhotoMetadata.motionPhotoVideoSize,
+              presentationTimestamp: motionPhotoMetadata.presentationTimestampUs,
+            }
          : livePhotoResult.isLivePhoto
            ? {
-              type: 'live-photo',
-              videoUrl: livePhotoResult.livePhotoVideoUrl!,
-              s3Key: livePhotoResult.livePhotoVideoS3Key!,
-            }
+                type: 'live-photo',
+                videoUrl: livePhotoResult.livePhotoVideoUrl!,
+                s3Key: livePhotoResult.livePhotoVideoS3Key!,
+              }


Inconsistent indentation in the video object. The nested properties should be consistently indented. Lines 241-245 and 248-251 have inconsistent indentation compared to the rest of the object structure (using 14 spaces instead of aligning with the parent object).

Copilot · 2025-11-15T16:17:39Z

packages/builder/src/photo/geocoding.ts

+        url.searchParams.set('longitude', lon.toString())
+        url.searchParams.set('latitude', lat.toString())
+        url.searchParams.set('types', 'address,place,district,region,country')
+        url.searchParams.set('language', 'zh-Hants')


[nitpick] The provider API returns a single string for language parameter but documentation typically requires language codes in a specific format. The Mapbox API expects ISO 639-1 language codes. 'zh-Hants' is used here, but it's worth verifying this is the correct format - Mapbox typically uses simpler codes like 'zh' or 'zh-CN'. Consider documenting or validating the expected language format.

Suggested change

url.searchParams.set('language', 'zh-Hants')

// Mapbox expects ISO 639-1 or BCP 47 language codes, e.g. 'zh', 'zh-CN', 'zh-TW'

url.searchParams.set('language', 'zh-TW')

Copilot · 2025-11-15T16:17:39Z

packages/builder/src/photo/geocoding.ts

+const isLockStale = async (lockPath: string): Promise<boolean> => {
+  try {
+    const stat = await fs.stat(lockPath)
+    return Date.now() - stat.mtimeMs > LOCK_STALE_TIMEOUT_MS
+  } catch (error) {
+    if ((error as NodeJS.ErrnoException).code === 'ENOENT') {


Similar error handling issue: casting to NodeJS.ErrnoException without type validation.

Suggested change

const isLockStale = async (lockPath: string): Promise<boolean> => {

try {

const stat = await fs.stat(lockPath)

return Date.now() - stat.mtimeMs > LOCK_STALE_TIMEOUT_MS

} catch (error) {

if ((error as NodeJS.ErrnoException).code === 'ENOENT') {

// Type guard for NodeJS.ErrnoException

function isErrnoException(error: unknown): error is NodeJS.ErrnoException {

return typeof error === 'object' && error !== null && 'code' in error && typeof (error as any).code === 'string';

}

const isLockStale = async (lockPath: string): Promise<boolean> => {

try {

const stat = await fs.stat(lockPath)

return Date.now() - stat.mtimeMs > LOCK_STALE_TIMEOUT_MS

} catch (error) {

if (isErrnoException(error) && error.code === 'ENOENT') {

apps/docs/contents/index.mdx

Copilot · 2025-11-15T16:17:39Z

packages/builder/src/photo/data-processors.ts

+// 单例提供者（避免重复创建）
+let cachedProvider: GeocodingProvider | null = null
+let lastProviderConfig: string | null = null
+
+/**
+ * 处理位置数据（反向地理编码）
+ * 优先复用现有数据，如果不存在或需要强制更新则进行地理编码
+ */
+export async function processLocationData(
+  exifData: PickedExif | null,
+  photoKey: string,
+  existingItem: PhotoManifestItem | undefined,
+  options: PhotoProcessorOptions,
+): Promise<LocationInfo | null> {
+  const loggers = getGlobalLoggers()
+
+  try {
+    // 获取配置
+    const context = getPhotoExecutionContext()
+    const config = context.builder.getConfig()
+    const processingSettings = config.system.processing
+
+    // 检查是否启用地理编码
+    if (!processingSettings.enableGeocoding) {
+      return null
+    }
+
+    // 检查是否可以复用现有数据
+    if (!options.isForceMode && !options.isForceManifest && existingItem?.location) {
+      const photoId = path.basename(photoKey, path.extname(photoKey))
+      loggers.location.info(`复用现有位置数据：${photoId}`)
+      return existingItem.location
+    }
+
+    // 检查 EXIF 是否包含 GPS 数据
+    if (!exifData) {
+      return null
+    }
+
+    // 解析 GPS 坐标
+    const { latitude, longitude } = parseGPSCoordinates(exifData)
+
+    if (latitude === undefined || longitude === undefined) {
+      return null
+    }
+
+    // 生成缓存 key（精确到小数点后4位）
+    const cacheKey = `${latitude.toFixed(4)},${longitude.toFixed(4)}`
+
+    // 检查缓存
+    if (locationCache.has(cacheKey)) {
+      const cached = locationCache.get(cacheKey)
+      const photoId = path.basename(photoKey, path.extname(photoKey))
+      loggers.location.info(`使用缓存的位置数据：${photoId} (${cacheKey})`)
+      return cached ?? null
+    }
+
+    // 创建或复用地理编码提供者
+    const providerType = processingSettings.geocodingProvider || 'auto'
+    const providerConfigKey = `${providerType}:${processingSettings.mapboxToken || ''}:${processingSettings.nominatimBaseUrl || ''}`
+
+    if (!cachedProvider || lastProviderConfig !== providerConfigKey) {
+      cachedProvider = createGeocodingProvider(
+        providerType,
+        processingSettings.mapboxToken,
+        processingSettings.nominatimBaseUrl,
+      )
+      lastProviderConfig = providerConfigKey


[nitpick] The cached provider instance is stored in module-level state (cachedProvider and lastProviderConfig), which could lead to stale provider instances if configuration changes between builds. While this may be intentional for performance, consider documenting this behavior or adding a mechanism to invalidate the cache when a new build starts. This is especially important if the builder is reused across multiple build cycles.

Copilot · 2025-11-15T16:17:41Z

packages/builder/src/photo/data-processors.ts

 import { generateThumbnailAndBlurhash, thumbnailExists } from '../image/thumbnail.js'
 import { workdir } from '../path.js'
-import type { PhotoManifestItem, PickedExif, ToneAnalysis } from '../types/photo.js'
+import type { LocationInfo,PhotoManifestItem, PickedExif, ToneAnalysis } from '../types/photo.js'


Missing space after comma in import statement. TypeScript style guidelines recommend consistent spacing in import lists.

Suggested change

import type { LocationInfo,PhotoManifestItem, PickedExif, ToneAnalysis } from '../types/photo.js'

import type { LocationInfo, PhotoManifestItem, PickedExif, ToneAnalysis } from '../types/photo.js'

Copilot · 2025-11-15T16:17:41Z

packages/builder/src/photo/data-processors.ts

-import type { PhotoManifestItem, PickedExif, ToneAnalysis } from '../types/photo.js'
+import type { LocationInfo,PhotoManifestItem, PickedExif, ToneAnalysis } from '../types/photo.js'
+import { getPhotoExecutionContext } from './execution-context.js'
+import type {GeocodingProvider} from './geocoding.js';


Missing space after comma in import statement. TypeScript style guidelines recommend consistent spacing in import lists.

Suggested change

import type {GeocodingProvider} from './geocoding.js';

import type { GeocodingProvider } from './geocoding.js';

Copilot · 2025-11-15T16:17:41Z

packages/builder/src/photo/geocoding.ts

+      if (delay > 0) {
+        await sleep(delay)
+      }
+
+      this.lastTimestamp = Date.now()


The rate limiter calculation in SequentialRateLimiter.wait() has a potential race condition. After checking elapsed < this.intervalMs and calculating delay, time passes before this.lastTimestamp = Date.now() is executed. This means the actual interval could be longer than intended. Consider setting this.lastTimestamp immediately after calculating the delay to minimize timing drift, or use a single Date.now() call stored in a variable.

Suggested change

if (delay > 0) {

await sleep(delay)

}

this.lastTimestamp = Date.now()

this.lastTimestamp = now + delay

if (delay > 0) {

await sleep(delay)

}

Copilot · 2025-11-15T16:17:42Z

packages/builder/src/photo/geocoding.ts

+    } catch (error) {
+      if ((error as NodeJS.ErrnoException).code !== 'ENOENT') {
+        throw error
+      }


Same issue: the error is cast to NodeJS.ErrnoException without validation. If the error is not of this type, accessing .code could return undefined, which is checked, but it's better practice to validate the error type first.

Copilot · 2025-11-15T16:17:42Z

packages/builder/src/photo/geocoding.ts

+export class MapboxGeocodingProvider implements GeocodingProvider {
+  private readonly accessToken: string
+  private readonly baseUrl = 'https://api.mapbox.com'
+  private readonly rateLimitMs = 100 // Mapbox 速率限制：1000次/分钟


The Mapbox rate limit is set to 100ms (10 requests/second) with a comment stating "1000次/分钟" (1000 times/minute), but 10 requests/second equals 600 requests/minute, not 1000. The comment and implementation are inconsistent. According to Mapbox documentation, the rate limit for the Geocoding API is typically 600 requests per minute for most plans. Please verify the correct rate limit for your use case and update either the code or comment accordingly.

Suggested change

private readonly rateLimitMs = 100 // Mapbox 速率限制：1000次/分钟

private readonly rateLimitMs = 100 // Mapbox 速率限制：600次/分钟

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

apps/docs/contents/index.mdx

Copilot

Pull Request Overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 10 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-20T14:53:06Z

packages/builder/src/photo/geocoding.ts

+    if (elapsed < intervalMs) {
+      await sleep(intervalMs - elapsed)
+    }
+
+    await fs.writeFile(timestampPath, `${Date.now()}`)


The timestamp is written after the sleep, which could lead to a race condition. If the process crashes or is interrupted between sleeping (line 102) and writing the timestamp (line 105), the next request won't be properly rate-limited. Consider writing the timestamp before sleeping or using the timestamp as the reference for when the next request should be allowed.

Suggested change

if (elapsed < intervalMs) {

await sleep(intervalMs - elapsed)

}

await fs.writeFile(timestampPath, `${Date.now()}`)

let nextAllowedTime = now;

if (elapsed < intervalMs) {

nextAllowedTime = lastRequestTime + intervalMs;

await fs.writeFile(timestampPath, `${nextAllowedTime}`);

await sleep(nextAllowedTime - now);

} else {

await fs.writeFile(timestampPath, `${now}`);

}

Copilot · 2025-11-20T14:53:06Z

packages/builder/src/photo/geocoding.ts

+        // 提取城市信息 - 拼接多个层级（从小到大）
+        const cityParts = [
+          address.village,
+          address.hamlet,
+          address.neighbourhood,
+          address.suburb,
+          address.district,
+          address.city,
+          address.town,
+          address.county,
+          address.state,
+        ].filter(Boolean)
+
+        // 去重并拼接（保持顺序，最多取2个层级）
+        const uniqueCityParts = [...new Set(cityParts)].slice(0, 2)
+        const city = uniqueCityParts.length > 0 ? uniqueCityParts.join(', ') : undefined


[nitpick] Same ordering concern as in Mapbox provider: the comment states "从小到大" (from small to large), but the array order starts with village/hamlet (smallest) and ends with state (largest). This is consistent with the comment, but verify if this produces the desired city display format (e.g., "Village, District" vs "City, State").

Copilot · 2025-11-20T14:53:06Z

packages/builder/src/photo/geocoding.ts

+  return exponential + jitter
+}
+
+const INTERPROCESS_RATE_LIMIT_DIR = path.join(os.tmpdir(), 'afilmory-geocoding-rate-limit')


[nitpick] The directory name 'afilmory-geocoding-rate-limit' should follow a more standard naming convention. Consider using a prefix that includes the package name or a more specific identifier to avoid potential conflicts with other applications in the system temp directory, such as 'afilmory-builder-geocoding-rate-limit'.

Suggested change

const INTERPROCESS_RATE_LIMIT_DIR = path.join(os.tmpdir(), 'afilmory-geocoding-rate-limit')

const INTERPROCESS_RATE_LIMIT_DIR = path.join(os.tmpdir(), 'afilmory-builder-geocoding-rate-limit')

Copilot · 2025-11-20T14:53:07Z

packages/builder/src/photo/geocoding.ts

+        url.searchParams.set('lon', lon.toString())
+        url.searchParams.set('format', 'json')
+        url.searchParams.set('addressdetails', '1')
+        url.searchParams.set('accept-language', 'zh-CN,zh,en')


The language is hard-coded to 'zh-CN,zh,en'. This should be configurable to support different user preferences. Consider adding a language configuration option that matches the Mapbox provider's approach or derives from system settings.

Copilot · 2025-11-20T14:53:07Z

packages/builder/src/photo/geocoding.ts

+        url.searchParams.set('types', 'address,place,district,region,country')
+        url.searchParams.set('language', 'zh-Hants')
+
+        log.info(`调用 Mapbox API: ${lat}, ${lon}`)


Security concern: The Mapbox access token is included in the URL query string (line 187), which may be logged in server logs, browser history, or network monitoring tools. While this is how Mapbox's API works, consider logging the URL without the token for security. The log message on line 193 should not include url.toString() to avoid exposing the token in logs.

Copilot · 2025-11-20T14:53:07Z

packages/builder/src/photo/data-processors.ts

+const locationCache = new Map<string, LocationInfo | null>()
+
+// 单例提供者（避免重复创建）
+let cachedProvider: GeocodingProvider | null = null
+let lastProviderConfig: string | null = null


The module-level cache locationCache and singleton provider cachedProvider are not thread-safe and will be shared across all concurrent photo processing. In a worker pool scenario, this could lead to race conditions. Consider moving these caches into the execution context or using proper synchronization mechanisms. The locationCache Map is not designed for concurrent access and could corrupt data if multiple workers access it simultaneously.

Copilot · 2025-11-20T14:53:07Z

packages/builder/src/photo/geocoding.ts

+        url.searchParams.set('longitude', lon.toString())
+        url.searchParams.set('latitude', lat.toString())
+        url.searchParams.set('types', 'address,place,district,region,country')
+        url.searchParams.set('language', 'zh-Hants')


The language is hard-coded to 'zh-Hants' (Traditional Chinese). This should be configurable or derived from user settings to support international users. Consider adding a language configuration option or using a default that matches the system/user locale.

Copilot · 2025-11-20T14:53:08Z

packages/builder/src/photo/geocoding.ts

+        // 提取城市信息 - 拼接多个层级（从小到大）
+        const cityParts = [
+          context.locality?.name,
+          context.neighborhood?.name,
+          context.district?.name,
+          context.place?.name,
+          context.region?.name,
+        ].filter(Boolean)
+
+        // 去重并拼接（保持顺序，最多取2个层级）
+        const uniqueCityParts = [...new Set(cityParts)].slice(0, 2)
+        const city = uniqueCityParts.length > 0 ? uniqueCityParts.join(', ') : undefined


[nitpick] Comment says "从小到大" (from small to large) but the order in the array suggests locality and neighborhood (small) come before district, place, and region (large). The comment correctly describes the order, but consider whether this is the intended hierarchy for displaying city information, as it may result in showing smaller administrative divisions first.

Copilot · 2025-11-20T14:53:08Z

packages/builder/src/photo/geocoding.ts

+        if (!data || data.error) {
+          throw new Error(`Nominatim API 返回错误: ${data?.error}`)
+        }


The error check on line 313 will throw an error if data is falsy, but then tries to access data?.error. If data is null or undefined, the error message will show "Nominatim API 返回错误: undefined". Consider separating these checks: first check if data is falsy and throw a different error, then check data.error separately for a more meaningful error message.

Suggested change

if (!data || data.error) {

throw new Error(`Nominatim API 返回错误: ${data?.error}`)

}

if (!data) {

throw new Error('Nominatim API 返回空响应')

}

if (data.error) {

throw new Error(`Nominatim API 返回错误: ${data.error}`)

}

Copilot · 2025-11-20T14:53:08Z

packages/builder/src/photo/data-processors.ts

+
+    // 创建或复用地理编码提供者
+    const providerType = processingSettings.geocodingProvider || 'auto'
+    const providerConfigKey = `${providerType}:${processingSettings.mapboxToken || ''}:${processingSettings.nominatimBaseUrl || ''}`


Security concern: The provider config key includes the full Mapbox token in plain text (line 196). This means the token could be exposed in logs or debugging output. Consider using a hash of the token instead of the full token value when creating the config key, similar to how the interprocess key uses a hash for file paths.

Copilot

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-22T11:39:04Z

packages/builder/src/photo/geocoding.ts

+        // 提取城市信息 - 拼接多个层级（从小到大）
+        const cityParts = [
+          address.village,
+          address.hamlet,
+          address.neighbourhood,
+          address.suburb,
+          address.district,
+          address.city,
+          address.town,
+          address.county,
+          address.state,
+        ].filter(Boolean)


The comment says "拼接多个层级（从小到大）" (join multiple levels from small to large), but the actual ordering in the cityParts array doesn't follow a consistent small-to-large pattern. For Nominatim, the order mixes village/hamlet (very small), then neighborhood/suburb (medium), then city/town (large), then county/state (very large). Consider reordering to actually follow a consistent size order, or update the comment to accurately describe the ordering strategy.

Copilot · 2025-11-22T11:39:05Z

packages/builder/src/plugins/geocoding.ts

+
+            for (const item of payload.manifest) {
+              if (!item) continue
+              if (item.location) continue


In force mode (isForceMode or isForceManifest), this logic will skip items that already have a location, which is inconsistent with the behavior in afterPhotoProcess. The afterPhotoProcess hook respects shouldOverwriteExisting, but here the check if (item.location) continue happens before calling resolveLocationForItem. This means existing locations won't be re-geocoded during batch processing even in force mode. Consider changing to:

if (item.location && !shouldOverwriteExisting) continue

Suggested change

if (item.location) continue

if (item.location && !shouldOverwriteExisting) continue

Copilot · 2025-11-22T11:39:05Z

packages/builder/src/photo/geocoding.ts

+  const existing = globalObject.__afilmoryGeocodingRateLimiters.get(key)
+  if (existing) {
+    return existing
+  }
+
+  const limiter = new SequentialRateLimiter(intervalMs)
+  globalObject.__afilmoryGeocodingRateLimiters.set(key, limiter)


The getGlobalRateLimiter function returns an existing rate limiter if one exists for the given key, but it doesn't verify that the intervalMs matches. If the same key is used with different intervals (e.g., if the configuration changes between builds), the old limiter with the old interval will be used, which could lead to incorrect rate limiting behavior. Consider including intervalMs in the key, or checking if the interval matches and creating a new limiter if it doesn't:

const cacheKey = `${key}:${intervalMs}` const existing = globalObject.__afilmoryGeocodingRateLimiters.get(cacheKey)

Suggested change

const existing = globalObject.__afilmoryGeocodingRateLimiters.get(key)

if (existing) {

return existing

}

const limiter = new SequentialRateLimiter(intervalMs)

globalObject.__afilmoryGeocodingRateLimiters.set(key, limiter)

const cacheKey = `${key}:${intervalMs}`

const existing = globalObject.__afilmoryGeocodingRateLimiters.get(cacheKey)

if (existing) {

return existing

}

const limiter = new SequentialRateLimiter(intervalMs)

globalObject.__afilmoryGeocodingRateLimiters.set(cacheKey, limiter)

…cation data to photo processing

Co-authored-by: Copilot <[email protected]>

Copilot

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 9 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-24T06:54:40Z

packages/builder/src/photo/image-pipeline.ts

+    // 8. 处理影调分析
    const toneAnalysis = await processToneAnalysis(sharpInstance, photoKey, existingItem, options)

-    // 8. 提取照片信息
+    // 9. 提取照片信息
    const photoInfo = extractPhotoInfo(photoKey, exifData)

-    // 9. 构建照片清单项
+    // 10. 构建照片清单项


The comment numbering in the image pipeline appears to be inconsistent. Comments are numbered as "8", "9", "10" but the actual processing steps are out of sequence (the comment says "8. 处理影调分析" but earlier there are only 6 documented steps before it). Consider renumbering these comments to accurately reflect the processing sequence.

Copilot · 2025-11-24T06:54:40Z

packages/builder/src/photo/geocoding.ts

+      await handle.write(`${process.pid}:${Date.now()}`)
+      await handle.close()


The file handle from fs.open(lockPath, 'wx') is not properly closed if an error occurs after opening. If handle.write() or handle.close() throws an error, the file handle leaks. Wrap the write and close operations in a try-finally block to ensure the handle is always closed, or use handle.close() in a finally block.

Suggested change

await handle.write(`${process.pid}:${Date.now()}`)

await handle.close()

try {

await handle.write(`${process.pid}:${Date.now()}`)

} finally {

try {

await handle.close()

} catch (_) {

// Ignore errors during close

}

}

Copilot · 2025-11-24T06:54:41Z

packages/builder/src/plugins/geocoding.ts

+    provider: options.provider ?? 'auto',
+    mapboxToken: options.mapboxToken,
+    nominatimBaseUrl: options.nominatimBaseUrl,
+    cachePrecision: normalizeCachePrecision(options.cachePrecision ?? DEFAULT_CACHE_PRECISION),


There's a redundant operation here: normalizeCachePrecision(options.cachePrecision ?? DEFAULT_CACHE_PRECISION). The normalizeCachePrecision function already handles undefined by returning DEFAULT_CACHE_PRECISION (line 56-58), so the ?? DEFAULT_CACHE_PRECISION is unnecessary and could cause confusion. If options.cachePrecision is explicitly set to 0, the current code would still pass 0 to the normalization function (which correctly clamps it), but the redundant fallback suggests a misunderstanding. Simplify to cachePrecision: normalizeCachePrecision(options.cachePrecision),.

Suggested change

cachePrecision: normalizeCachePrecision(options.cachePrecision ?? DEFAULT_CACHE_PRECISION),

cachePrecision: normalizeCachePrecision(options.cachePrecision),

Copilot · 2025-11-24T06:54:41Z

packages/builder/src/photo/geocoding.ts

+          throw new Error(`Mapbox API 错误: ${response.status} ${response.statusText}`)
+        }
+
+        const data = await response.json()


The response.json() call can throw an error if the response body is not valid JSON. This error would be caught by the outer try-catch and trigger a retry, which may not be appropriate for malformed JSON responses (which likely won't succeed on retry). Consider wrapping the JSON parsing in a separate try-catch to handle JSON parsing errors differently, or at least log them distinctly to aid debugging.

Suggested change

const data = await response.json()

let data;

try {

data = await response.json();

} catch (jsonError) {

log.error('Mapbox API 响应不是有效的 JSON', jsonError);

return null;

}

Copilot · 2025-11-24T06:54:41Z

packages/builder/src/photo/geocoding.ts

+        // 提取城市信息 - 拼接多个层级（从小到大）
+        const cityParts = [
+          address.village,
+          address.hamlet,
+          address.neighbourhood,
+          address.suburb,
+          address.district,
+          address.city,
+          address.town,
+          address.county,
+          address.state,
+        ].filter(Boolean)
+
+        // 去重并拼接（保持顺序，最多取2个层级）
+        const uniqueCityParts = [...new Set(cityParts)].slice(0, 2)
+        const city = uniqueCityParts.length > 0 ? uniqueCityParts.join(', ') : undefined


Similar to the Mapbox implementation, the comment states "拼接多个层级（从小到大）" but the order in the array [village, hamlet, neighbourhood, suburb, district, city, town, county, state] doesn't strictly follow a small-to-large hierarchy. For example, village and hamlet are roughly the same size, and city and town overlap. This ordering may work for deduplication purposes, but the comment is misleading. Consider either reordering to match a clearer hierarchy or updating the comment to reflect that this is a priority order for display rather than a strict hierarchical ordering.

Copilot · 2025-11-24T06:54:42Z

packages/builder/src/plugins/geocoding.ts

+            for (const item of payload.manifest) {
+              if (!item) continue
+              if (item.location) continue
+
+              const { attempted: didAttempt, updated: didUpdate } = await resolveLocationForItem(
+                item,
+                item.exif,
+                state,
+                currentSettings,
+                provider,
+                shouldOverwriteExisting,
+              )
+
+              if (didAttempt) {
+                attempted++
+                if (didUpdate) {
+                  updated++
+                }
+              }
+            }


The afterProcessTasks loop processes manifest items sequentially (line 295), but it doesn't take advantage of caching that may have already been populated during the afterPhotoProcess hook. For large photo sets, this can be inefficient. Since the provider and cache are already shared via runShared state, items that had their location resolved in afterPhotoProcess will hit the cache here. However, the if (item.location) continue check at line 297 prevents any processing, meaning items that failed to resolve during afterPhotoProcess won't be retried here. Consider whether this is intentional behavior or if failed geocoding attempts should be retried in this batch processing phase.

Copilot · 2025-11-24T06:54:42Z

packages/builder/src/plugins/geocoding.ts

+          provider: normalizedOptions.provider,
+          mapboxToken: normalizedOptions.mapboxToken,
+          nominatimBaseUrl: normalizedOptions.nominatimBaseUrl,
+          cachePrecision: normalizedOptions.cachePrecision ?? DEFAULT_CACHE_PRECISION,


Similar to line 77, there's a redundant fallback here. The normalizedOptions.cachePrecision is already guaranteed to be a valid number from resolveSettings() (line 77), which calls normalizeCachePrecision(). The ?? DEFAULT_CACHE_PRECISION fallback is unnecessary and creates confusion about when the precision could be undefined. Remove the fallback and use cachePrecision: normalizedOptions.cachePrecision, directly.

Suggested change

cachePrecision: normalizedOptions.cachePrecision ?? DEFAULT_CACHE_PRECISION,

cachePrecision: normalizedOptions.cachePrecision,

Copilot · 2025-11-24T06:54:42Z

packages/builder/src/photo/geocoding.ts

+          throw new Error(`Nominatim API 错误: ${response.status} ${response.statusText}`)
+        }
+
+        const data = await response.json()


Same issue as with Mapbox: the response.json() call can throw an error if the response body is not valid JSON. This would trigger unnecessary retries for malformed responses. Consider handling JSON parsing errors separately from network/API errors.

Suggested change

const data = await response.json()

let data

try {

data = await response.json()

} catch (jsonError) {

throw new Error(`Nominatim API 返回无效 JSON: ${jsonError}`)

}

Copilot · 2025-11-24T06:54:42Z

packages/builder/src/photo/geocoding.ts

+  if (provider === 'nominatim' || provider === 'auto') {
+    return new NominatimGeocodingProvider(nominatimBaseUrl, language)
+  }
+


The provider selection logic has an edge case: if provider is set to 'mapbox' but mapboxToken is not provided, the function returns null (line 401). However, there's no explicit error or warning to inform the user that their configuration is invalid. Consider adding validation or a warning log when Mapbox is explicitly requested but the token is missing, to help users debug configuration issues.

Suggested change

// Edge case: provider is 'mapbox' but no token provided

if (provider === 'mapbox' && !mapboxToken) {

const log = getGlobalLoggers().location

log.warn('Mapbox 地理编码提供者被请求，但未提供 Mapbox Token。请检查您的配置。')

}

Enter-tainer commented Nov 14, 2025

View reviewed changes

vercel bot deployed to Preview November 14, 2025 17:11 View deployment

Enter-tainer force-pushed the mgt/reverse-geo-encoding branch from 102758e to 3554312 Compare November 14, 2025 17:14

vercel bot deployed to Preview November 14, 2025 17:16 View deployment

vercel bot deployed to Preview November 14, 2025 17:32 View deployment

Enter-tainer marked this pull request as ready for review November 15, 2025 16:12

Copilot AI review requested due to automatic review settings November 15, 2025 16:12

Copilot started reviewing on behalf of Enter-tainer November 15, 2025 16:12 View session

Copilot finished reviewing on behalf of Enter-tainer November 15, 2025 16:15

Copilot AI reviewed Nov 15, 2025

View reviewed changes

Enter-tainer force-pushed the mgt/reverse-geo-encoding branch from a7bae64 to 605fde0 Compare November 16, 2025 07:55

vercel bot deployed to Preview November 16, 2025 07:56 View deployment

Innei reviewed Nov 19, 2025

View reviewed changes

apps/docs/contents/index.mdx Outdated Show resolved Hide resolved

Copilot AI review requested due to automatic review settings November 20, 2025 14:46

Enter-tainer force-pushed the mgt/reverse-geo-encoding branch from 605fde0 to 9d9a7c4 Compare November 20, 2025 14:46

Copilot started reviewing on behalf of Enter-tainer November 20, 2025 14:47 View session

vercel bot deployed to Preview – afilmory-demo November 20, 2025 14:48 View deployment

vercel bot deployed to Preview – afilmory-landing November 20, 2025 14:49 View deployment

Copilot finished reviewing on behalf of Enter-tainer November 20, 2025 14:50

Copilot AI reviewed Nov 20, 2025

View reviewed changes

Enter-tainer force-pushed the mgt/reverse-geo-encoding branch from 9d9a7c4 to 20558e4 Compare November 20, 2025 15:41

vercel bot deployed to Preview – afilmory-demo November 20, 2025 15:43 View deployment

vercel bot deployed to Preview – afilmory-landing November 20, 2025 15:44 View deployment

Copilot AI review requested due to automatic review settings November 20, 2025 16:20

Innei approved these changes Nov 20, 2025

View reviewed changes

vercel bot deployed to Preview – afilmory-demo November 21, 2025 15:18 View deployment

vercel bot deployed to Preview – afilmory-landing November 21, 2025 15:19 View deployment

Copilot AI review requested due to automatic review settings November 22, 2025 11:35

Enter-tainer force-pushed the mgt/reverse-geo-encoding branch from a6928fc to f6b66f4 Compare November 22, 2025 11:35

Copilot started reviewing on behalf of Enter-tainer November 22, 2025 11:35 View session

vercel bot deployed to Preview – afilmory-demo November 22, 2025 11:36 View deployment

vercel bot deployed to Preview – afilmory-landing November 22, 2025 11:36 View deployment

Copilot finished reviewing on behalf of Enter-tainer November 22, 2025 11:38

Copilot AI reviewed Nov 22, 2025

View reviewed changes

Enter-tainer and others added 7 commits November 23, 2025 13:33

feat(geocoding): implement reverse geocoding functionality and add lo…

fb301be

…cation data to photo processing

feat: add position filling stage in manifest building

7c709cd

docs: update doc for reverse geoencoding

0b2fcde

chore: move to user section

40ecd23

Apply suggestions from code review

48eae66

Co-authored-by: Copilot <[email protected]>

feat: switch to plugin

5961387

fix: pass language tag in config

9bee09b

Enter-tainer force-pushed the mgt/reverse-geo-encoding branch from f6b66f4 to 9bee09b Compare November 23, 2025 05:33

vercel bot deployed to Preview – afilmory-demo November 23, 2025 05:35 View deployment

vercel bot deployed to Preview – afilmory-landing November 23, 2025 05:35 View deployment

Innei approved these changes Nov 24, 2025

View reviewed changes

Merge branch 'main' into mgt/reverse-geo-encoding

f70ff50

Copilot AI review requested due to automatic review settings November 24, 2025 06:48

Copilot started reviewing on behalf of Innei November 24, 2025 06:48 View session

Innei merged commit 58a989c into main Nov 24, 2025
7 of 10 checks passed

vercel bot deployed to Preview – afilmory-docs November 24, 2025 06:49 View deployment

vercel bot deployed to Preview – afilmory-demo November 24, 2025 06:53 View deployment

Copilot finished reviewing on behalf of Innei November 24, 2025 06:53

vercel bot deployed to Preview – afilmory-landing November 24, 2025 06:53 View deployment

Copilot AI reviewed Nov 24, 2025

View reviewed changes

ChrAlpha mentioned this pull request Nov 24, 2025

feat(location): add location-based search functionality #168

Merged

	url.searchParams.set('language', 'zh-Hants')
	// Mapbox expects ISO 639-1 or BCP 47 language codes, e.g. 'zh', 'zh-CN', 'zh-TW'
	url.searchParams.set('language', 'zh-TW')

	import type { LocationInfo,PhotoManifestItem, PickedExif, ToneAnalysis } from '../types/photo.js'
	import type { LocationInfo, PhotoManifestItem, PickedExif, ToneAnalysis } from '../types/photo.js'

	import type {GeocodingProvider} from './geocoding.js';
	import type { GeocodingProvider } from './geocoding.js';

	private readonly rateLimitMs = 100 // Mapbox 速率限制：1000次/分钟
	private readonly rateLimitMs = 100 // Mapbox 速率限制：600次/分钟

-    if (elapsed < intervalMs) {
-      await sleep(intervalMs - elapsed)
-    }
-    await fs.writeFile(timestampPath, `${Date.now()}`)
+    let nextAllowedTime = now;
+    if (elapsed < intervalMs) {
+      nextAllowedTime = lastRequestTime + intervalMs;
+      await fs.writeFile(timestampPath, `${nextAllowedTime}`);
+      await sleep(nextAllowedTime - now);
+    } else {
+      await fs.writeFile(timestampPath, `${now}`);
+    }

	const INTERPROCESS_RATE_LIMIT_DIR = path.join(os.tmpdir(), 'afilmory-geocoding-rate-limit')
	const INTERPROCESS_RATE_LIMIT_DIR = path.join(os.tmpdir(), 'afilmory-builder-geocoding-rate-limit')

-        if (!data || data.error) {
-          throw new Error(`Nominatim API 返回错误: ${data?.error}`)
-        }
+        if (!data) {
+          throw new Error('Nominatim API 返回空响应')
+        }
+        if (data.error) {
+          throw new Error(`Nominatim API 返回错误: ${data.error}`)
+        }

	if (item.location) continue
	if (item.location && !shouldOverwriteExisting) continue

		await handle.write(`${process.pid}:${Date.now()}`)
		await handle.close()

	cachePrecision: normalizeCachePrecision(options.cachePrecision ?? DEFAULT_CACHE_PRECISION),
	cachePrecision: normalizeCachePrecision(options.cachePrecision),

-        const data = await response.json()
+        let data;
+        try {
+          data = await response.json();
+        } catch (jsonError) {
+          log.error('Mapbox API 响应不是有效的 JSON', jsonError);
+          return null;
+        }

	cachePrecision: normalizedOptions.cachePrecision ?? DEFAULT_CACHE_PRECISION,
	cachePrecision: normalizedOptions.cachePrecision,

+  // Edge case: provider is 'mapbox' but no token provided
+  if (provider === 'mapbox' && !mapboxToken) {
+    const log = getGlobalLoggers().location
+    log.warn('Mapbox 地理编码提供者被请求，但未提供 Mapbox Token。请检查您的配置。')
+  }

feat(geocoding): implement reverse geocoding #157

feat(geocoding): implement reverse geocoding #157

Uh oh!

Conversation

Enter-tainer commented Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vercel bot commented Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Enter-tainer Nov 14, 2025

Choose a reason for hiding this comment

Uh oh!

Enter-tainer commented Nov 15, 2025

Uh oh!

chatgpt-codex-connector bot commented Nov 15, 2025

Uh oh!

safedep bot commented Nov 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

SafeDep Report Summary

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Key Changes

Reviewed Changes

Uh oh!

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

Enter-tainer commented Nov 14, 2025 •

edited

Loading

vercel bot commented Nov 14, 2025 •

edited

Loading

safedep bot commented Nov 15, 2025 •

edited

Loading