Skip to content

refactor(server): one taken-date and filter predicate for the timeline, suggestions, facets, map and search - #1158

Open
luminoso wants to merge 8 commits into
open-noodle:mainfrom
luminoso:refactor/asset-filter-predicate
Open

luminoso wants to merge 8 commits into
open-noodle:mainfrom
luminoso:refactor/asset-filter-predicate

Conversation

@luminoso

@luminoso luminoso commented Oct 4, 2026

Copy link
Copy Markdown

Summary

The filter panel's taken range, place, camera, lens and rating filters were encoded separately in the timeline, filter suggestions, tag suggestions, smart-search facets, the filtered map, the space people lists and search. The copies had drifted: the timeline compared localDateTime with an inclusive end, suggestions and the map compared fileCreatedAt, suggestions ignored null place filters, and the map only got a minimum rating because the service remembered to ask for one.

This PR moves those filters into one fork-only module, server/src/utils/asset-filter.ts, and routes every fork surface through it. Search results go through it too, by stripping the taken range from upstream's searchAssetBuilderLegacy at the fork call site and reapplying the module's. The upstream builder itself is not modified. A new index keeps the moved queries on an index scan, and mobile now sends the same wall-clock ranges as web.

Decision

Filter Meaning on every fork surface
Taken after asset.localDateTime >= takenAfter (inclusive)
Taken before asset.localDateTime < takenBefore (exclusive)
City, state, country, make, model, lens value: equals; null: IS NULL; missing or '': no filter
Rating value: at least that rating; null: unrated

Why localDateTime: the timeline groups its buckets by localDateTime, the photo's wall-clock time stored as UTC. The web panel's month and day ranges are built in that same frame (Date.UTC(year, month, 1) and the start of the day after the last chosen day, see web/src/lib/components/filter-panel/filter-panel.ts). Comparing those ranges with fileCreatedAt, a real instant, misfiles every photo taken within the user's UTC offset of a boundary. The exclusive end matches what the panel sends.

The user chose full alignment: localDateTime is "taken" on every surface the fork controls, search results included, and both clients send wall-clock ranges.

Changes

  • Server module: asset-filter.ts exposes assetFilterConditions(filter, exclude?) for raw-SQL callers, withAssetFilter(qb, filter, exclude?) for query builders, assetFilterKeys for the strip-and-reapply seam, and searchAssetBuilderWithLocalTaken for search. Place, camera and rating run as one EXISTS on asset_exif; it is a primary-key probe, so the plan matches the old join. The timeline (withTimeBucketAssetFilters), filter suggestions (buildFilteredAssetIds), smart-search facets (buildSmartFacetFilteredAssetIds, with its facet exclusions mapped to filter keys), tag suggestions, the filtered map and the space people list, count and face statistics use it. The map's dead ratingIsMinimum workaround is gone.
  • Search call sites: smart search, metadata search, search statistics, random search and large-asset search call searchAssetBuilderWithLocalTaken. Only the taken range moves. Metadata search keeps upstream's exact-rating default, and the other filters stay with upstream. Metadata search now sorts by localDateTime, the column it filters on, so a date-filtered page walks the new index. searchAssetBuilderLegacy in utils/database.ts is unchanged.
  • Migration: fork migration 1794000000000-AddAssetLocalDateTimeIndex creates asset_localDateTime_range_idx, a plain btree on asset("localDateTime"). The index is declared on AssetTable so the schema-drift check covers it. It is also added to the gallery ORDER manifest and to scripts/revert-to-immich.sql.
  • Mobile: SearchDateFilter gains takenAfterParam and takenBeforeParam. They build DateTime.utc from the chosen calendar days: midnight of the first day, and midnight of the day after the last one. Smart search, metadata search and filter suggestions send those. The stored filter state and the picker's selection checks are unchanged.
  • OpenAPI: the taken-range descriptions on TimeBucketDto, FilteredMapMarkerDto, the search DTOs and the suggestion DTOs now say the time is local, the start inclusive and the end exclusive. The spec was regenerated. The TypeScript SDK changes only in doc comments on body fields. The Dart client is generated at build time.

Behaviour changes

  • Month-boundary fix for non-UTC users: a photo taken Jan 1 05:00 at UTC+10 is January everywhere. On main it showed in the January timeline but was missing from suggestion counts, facets, map pins and search results.
  • The timeline's upper bound is now exclusive, so an asset at exactly the next month's local midnight no longer appears in the previous month's filter.
  • Search results now use the same taken range as the timeline. In web smart-search mode, the facet counts and the result grid agree again. On mobile, suggestions and results agree.
  • The web search bar's date range (search-manager.svelte.ts) is now compared with the local taken time too. It already sent the picked days as wall-clock time in UTC (asLocalTimeISO), so main compared a wall-clock range with an instant and was off by the viewer's UTC offset at both ends. Its end-of-day bound (23:59:59.999) is unchanged and still covers the whole last day under the exclusive <.
  • Metadata search now sorts by localDateTime instead of fileCreatedAt, which is the timeline's wall-clock order. The order only differs for photos from different time zones taken within hours of each other. The reason is measured: a filter on localDateTime with a sort on fileCreatedAt makes Postgres walk asset_fileCreatedAt_idx backwards past every newer asset. On a 300,000-asset seed, the first page took 22.5 ms for a 2020 range and 45.9 ms for 2016; with the sort on localDateTime it takes 0.11 ms. One btree cannot serve a range on one column and a sort on another.
  • An empty string for a place or camera filter now means no filter on the facets and the map. It used to match the empty string. No client sends it.
  • Mobile clients without this change send device-local instants, so their month filters are shifted by the device's timezone offset until they update. For a UTC+10 phone, January starts ten hours early.
  • Themed memories search with searchSmart, so they now see localDateTime too. The two-day search margin in theme-search.adapter.ts is now redundant but kept, so the top-N results a theme sees do not change.

Migration

The new index is a CREATE INDEX IF NOT EXISTS inside the migration transaction, like the one precedent, the trigram index migration on asset_exif. It runs once at server start. Postgres holds a SHARE lock on asset while it builds, which blocks writes to asset (uploads, edits) but not reads. On a 300,000-asset seed the build took about 100 ms with fsync off. Expect a few seconds per million assets on real storage. Libraries large enough for that to matter will see uploads pause briefly during the first start after upgrade.

Testing

  • Parity spec server/test/medium/specs/repositories/asset-filter-parity.spec.ts: one fixture with a Jan 1 05:00 UTC+10 asset, an asset at exactly the exclusive bound, and null place and rating values. It checks 13 filters across the timeline, the suggestion universe, the filtered map, smart search, metadata search and the facet total. On main's code, 6 of 13 fail: January, city: null, country: null, rating minimum, rating: null, and January plus a make. On this branch all 13 pass.
  • EXPLAIN ANALYZE on a migrated database seeded with 300,000 assets for one owner, for a one-year 2016 range. Each pair is with the new index dropped, then with it:
Query Index dropped With index
Suggestion universe with a make 44.3 ms, parallel seq scan on asset 24.9 ms, parallel index scan on asset_localDateTime_range_idx
Timeline buckets 31.8 ms, parallel seq scan 16.7 ms, index scan on asset_localDateTime_range_idx
Search statistics 25.7 ms, parallel seq scan 7.2 ms, index scan on asset_localDateTime_range_idx
Metadata search, first page 31.0 ms, parallel seq scan 0.11 ms, index scan backward on asset_localDateTime_range_idx

Before the sort change, metadata search filtered on localDateTime but walked asset_fileCreatedAt_idx backwards. That took 22.5 ms for 2020 and 45.9 ms for 2016, against 0.17 ms on main, which is why the sort moved.

  • After the second review: touched unit specs (src/schema, src/dtos, search.service.spec.ts), 19 files, 472 passed. Medium parity, schema-drift, database-migration service and search repository, 4 files, 110 passed. The database-migration spec reverts and re-runs the new migration. pnpm lint and pnpm check are clean.
  • server: full unit suite (vitest --config test/vitest.config.mjs --run), 6433 passed, 1 expected fail, 12 skipped.
  • Medium (vitest --config test/vitest.config.medium.mjs --run, 16 files: parity, schema-drift, database-migration service, asset, search, shared-space, map, face matching, timeline visibility and album specs, search, timeline and memory services and repository): 804 passed, 12 skipped. With the migration file removed, schema-drift fails with "The index asset.asset_localDateTime_range_idx is missing", so the check guards it.
  • pnpm lint and pnpm check are clean, and the gallery migration order check passes. src/queries/search.repository.sql was regenerated with sync-sql.
  • Mobile: tests were added to test/infrastructure/repositories/search_api_repository_test.dart (smart and metadata search send 2024-01-01T00:00Z to 2024-02-01T00:00Z for a chosen January), and test/providers/photos_filter/filter_suggestions_provider_test.dart was updated. flutter test was not run locally: there is no Flutter, Dart or mise on the machine, so neither codegen nor the tests could run, and dart format was not run either (the two long lines were wrapped by hand to match the neighbouring ones). The mobile changes rely on CI, and the reviewer read them line by line.
  • Web: svelte-check and tsc were not run, because the worktree has no web dependencies installed. The only SDK change is doc comments.
  • E2E was not run. The metadata search ordering assertions in e2e/src/specs/server/api/search.e2e-spec.ts were checked against the fixtures' EXIF dates, and none of them change order.

Follow-ups

  • Tags, people and albums still differ per surface. Tags are any-of on the timeline and in suggestions but all-of in facets and search. Folding them into the module would change behaviour on some surface, so it is left for a separate PR.
  • Metadata search keeps upstream's exact rating unless asked for a minimum. Every other surface treats rating as a minimum.
  • The timeline and suggestion DTOs cannot express null for place or camera filters, so "no city" there is still search, facets and map only.
  • The memory theme search margin can be removed once someone checks that themed memories do not need the wider window.

…faces

The timeline, filter suggestions, tag suggestions, smart-search facets, the
filtered map and the space people lists each re-encoded the filter panel's
taken range, place/camera/lens and rating filters, and had drifted apart.

src/utils/asset-filter.ts now defines them once:
- taken: asset.localDateTime (the column the timeline buckets by), takenAfter
  inclusive, takenBefore exclusive (the panel sends the next day/month start)
- place/camera/lens: null means IS NULL, undefined or '' means no filter
- rating: a minimum (>=), null means unrated

Behaviour change: suggestions, facets, the map and the space people lists
used fileCreatedAt, so assets taken near a month boundary in a non-UTC offset
showed in the timeline but not in suggestion counts or map pins. The timeline
upper bound becomes exclusive, so an asset at exactly the next month's local
midnight no longer appears in the previous month's filter.

Upstream searchAssetBuilderLegacy is untouched and keeps fileCreatedAt; the
map strips the filter keys from it and applies the shared predicate instead.

A medium parity spec checks every filter against the timeline, the suggestion
universe, the map and the facet total over one fixture.
…range

The shared asset filter compares "localDateTime" >= / < directly, which
the existing ::date and date_trunc expression indexes cannot serve. Add a
plain btree (fork migration 1794000000000) and declare it on AssetTable so
the schema-drift check covers it.
Smart search, metadata search, search statistics, random and large-asset
search now take their takenAfter/takenBefore from src/utils/asset-filter.ts
(localDateTime, exclusive end) through searchAssetBuilderWithLocalTaken,
which strips the two keys from upstream searchAssetBuilderLegacy and
reapplies the module's predicate. The upstream builder is unchanged.

Metadata search now orders by localDateTime so a date-filtered page walks
the new index instead of scanning backwards through fileCreatedAt.

The parity spec covers smart and metadata search results plus the
country, state, model and rating: null cases; the timeline bound test
keeps its just-past-the-bound probe.
…eline and map DTOs

The timeline and filtered-map taken range now compares asset.localDateTime
with an exclusive upper bound. Regenerated the OpenAPI spec; the TypeScript
SDK carries no query-parameter descriptions, so it is unchanged.
The server now compares takenAfter/takenBefore with asset.localDateTime,
the photo's wall-clock time stored as UTC, with an exclusive upper bound.
Mobile sent device-local instants (Jan 1 00:00 .. Jan 31 23:59:59 local),
which the Dart client converts to UTC, so a UTC+10 device asked for a
range ten hours early.

SearchDateFilter gains takenAfterParam / takenBeforeParam, which build
DateTime.utc from the chosen calendar days: midnight of the first day and
midnight of the day after the last. Smart search, metadata search and the
filter suggestions use them; the stored filter state and its selection
checks are unchanged.
…Immich

revert-to-immich.sql drops the fork-only asset_localDateTime_range_idx and
removes its migration row, as the revert-to-immich spec requires for every
migrations-gallery migration.
- search.dto.ts: the search and suggestion takenAfter/takenBefore
  descriptions now state local time, inclusive start and exclusive end;
  OpenAPI spec and TypeScript SDK doc comments regenerated.
- Metadata search's sort comment says it is the timeline's wall-clock
  order and why it must stay.
- The localDateTime index migration uses CREATE INDEX IF NOT EXISTS, like
  the trigram index migration.
- Mobile: wrap the two takenAfter lines that exceeded the 120-column
  page width, the way dart format wraps the takenBefore lines below them.
@github-actions

github-actions Bot commented Oct 4, 2026

Copy link
Copy Markdown

Label error. Requires exactly 1 of: changelog:.*. Found: . A maintainer will add the required label.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant