grain.social/grain

Switch repository

Loading…

What changes

social.grain.notification.updateSeen (src/notification.rs). It marks the caller's notifications seen as of seenAt, as app.bsky.notification.updateSeen does. Without a service token it answers 401 AuthenticationRequired. A seenAt that is not an RFC 3339 datetime is 400, as is one UTC cannot write with a four-digit year (9999-12-31T23:59:59-01:00). The time is kept in UTC to the millisecond (2026-10-31T12:00:00.000Z), so it compares as text with the times records carry. A later call replaces it, earlier or not, as Bluesky's does.

Notifications' state gets tables of its own (migration V5). notification_seen (did, seen_at) and notification_pref (did, kind, push, in_app, from_follows), one row a kind (favorites, follows, comments, mentions). A kind with no row reaches the account every way, from anyone. Both sit with what the network cannot rebuild, in schema.rs's docs.

The import carries hatk's values over. _preferences.lastSeenNotifications becomes notification_seen, its time normalised to UTC. notificationPrefs becomes a row of notification_pref for each kind it set, read as hatk read it: pushed, or shown in the app, unless it says false; from anyone unless from is "follows". A kind kept as something other than an object, or a value that is not JSON, said nothing hatk acted on, and is not counted. Every other key goes to preference as before. Each is its own counted table in the summary, so a seen time that does not parse fails the import, as any row the index refuses does. V5 moves the same two keys out of preference for an index imported before it.

Failure::authentication_required is the 401 that Reader::signed_in, indexRecords and updateSeen share.

Why

The first half of the notifications step. listNotifications and getUnreadCount follow, stacked on this pull: both read these tables. The user chose Bluesky's model, a seen time set by updateSeen, with isRead per item and the count as its own method, over hatk's lastSeenNotifications preference.

Why separate tables rather than leaving both keys in preference. The list and the count filter on them in SQL, every arm of the union, by (did, kind). Reading JSON out of a key-value row in each arm would cost more. The typed notificationsPref that putPreferences will take maps onto these rows directly. A seen time is not a preference in the new API anyway: no putPreferences call sets it.

Where this differs from hatk:

• The seen time is normalised to UTC. hatk kept whatever string the client sent and compared it as text, so a +05:30 offset compared wrongly.
• Times are checked. hatk's putPreference took any JSON as lastSeenNotifications.

How it was checked

• tests/notifications.rs (a new binary) calls updateSeen with a service token from the reference PDS: an offset time is kept in UTC, a later call replaces it, a time finer than a millisecond is kept to the millisecond. It is refused without a token (401), with a missing, non-string, non-time or date-only seenAt, or one past year 9999 or before year 0 in UTC (400), and on a server with no index (501). Each answer goes through common, for answers.mjs.
• import/tests.rs: the fixture gains notification preferences for three accounts: an offset seen time, kinds set to false, from: "follows", a kind that is not an object, an unknown kind, a missing field, and a value that is not JSON. The test checks the rows each table gets and the summary's counts. A new test shows a seen time that is no time fails the import, naming the row.
• index/tests.rs: an index at V4 holding both keys in preference migrates to V5 with them moved, bad values dropped, and other keys left alone.

I ran cargo check and the index:: unit tests here once. CI runs fmt, clippy, the whole suite, the coverage gate and the answers check.

Shortcomings

• Only the import writes notification_pref so far. putPreferences (a later step) will write it. Until then an account's notification preferences are whatever it had in hatk.
• V5's move drops a seen time that does not parse rather than failing the migration, since a failed migration stops the server starting. The import fails on the same row instead, because there an operator is watching.

Review

A quick look. Start at V5 in index/schema.rs and the three _preferences entries in import.rs's KEPT.

🤖 Generated with Claude Code

This adds social.grain.notification.updateSeen. It also adds the notification_seen and notification_pref tables (V5), and the import and migration carry hatk's two preference keys into them. The diff matches the description. I found nothing that must change, and I did not run the code or read the checks.

Two small edge cases, neither blocking:

• Index::set_seen gives strftime a datetime that valid_datetime has accepted. SQLite can still return NULL for one, for example a year 0000 time with a positive offset, or a year 9999 time with a negative one. The NOT NULL insert then fails and the caller gets a 500 instead of a 400.
• SQLite's %f rounds to the millisecond, so .9996 becomes the next second. The docs and the PR say "truncated". The test only uses .123456, so it doesn't catch the difference.

The new commit fixes the first edge case from my earlier review. Index::set_seen now computes the UTC text first and returns false if SQLite gives NULL or a string valid_datetime rejects. updateSeen turns that into a 400 instead of a 500 on the NOT NULL insert. The new tests cover 0000-01-01T00:00:00+01:00 and 9999-12-31T23:59:59-01:00. I read the diff only and did not run the code or read the checks.

The second point is only half-addressed, and it doesn't block the merge. SQLite's %f rounds to the millisecond, so .9996 becomes the next second. The doc comment now says "as SQLite's strftime writes it", which is accurate. The PR body still says "truncated", and the test only uses .123456. If you want the test to pin the behaviour, add a case like .9996.

On the rounding: the body no longer says truncated (it says kept to the millisecond), so the review read an earlier edit of it. I have left the test without a .9996 case on purpose. Whether %f rounds or truncates the sub-millisecond part depends on the SQLite version: the system sqlite3 here gives 2026-01-01T00:00:00.999Z for .9996Z. Pinning either result would tie the test to the bundled SQLite's version. The seen time only has to compare consistently with record times, and either behaviour does that.