Skip to main content

max / makenotwork

multithreaded: make read position monotonic, fix soft-delete filter drift Three fixes from re-verifying the Run #3 carried cluster. update_read_position was an unconditional SET, so the view handler's bump to the last post on the current page dragged the position backward whenever a reader revisited an earlier page, and list_tracked_threads re-counted already-read posts as unread. The bump is also spawned off the response path, so concurrent views of different pages raced. Guard it in SQL rather than in Rust: Postgres re-evaluates the comparison against the winning row under read-committed, so the race resolves correctly instead of last-writer-wins. A post id that names no row now leaves the position untouched. list_communities and list_archived_communities counted soft-deleted threads toward the thread total on the browse pages, so the count exceeded what a visitor could open. count_threads_in_community keeps its omission: its doc comment says including soft-deleted is deliberate there. The search post_matches CTE filtered posts by removed_at but not deleted_at, unlike queries/user.rs which applies both. Latent today (posts.deleted_at is dormant, as migration 031 records) but it would leak body snippets of author-deleted posts the day a delete-my-own-post path ships. Findings filed as problems 8bcca40d, 9b71f267, 14a0f91a.
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-26 23:04 UTC
Signed with PGP, not checked
Commit: aac668201ee4f4c80ffc1968774089c89357d132
Parent: 6402bf4
10 files changed, +199 insertions, -103 deletions
@@ -176,6 +176,83 @@
176 176 );
177 177 }
178 178
179 + /// Regression (Run #3 re-verify, read-position monotonicity): the read position
180 + /// only moves forward. The view handler bumps to the last post on the page being
181 + /// viewed, so revisiting page 1 of a long thread must not drag the position back
182 + /// and resurrect already-read posts as unread.
183 + #[tokio::test]
184 + async fn read_position_never_moves_backward() {
185 + let mut h = TestHarness::new().await;
186 + let user_id = h.login_as("monotonic").await;
187 + let comm_id = h.create_community("Test", "test").await;
188 + let cat_id = h.create_category(comm_id, "General", "general").await;
189 + h.add_membership(user_id, comm_id, "member").await;
190 +
191 + let thread_id = h
192 + .create_thread_with_post(cat_id, user_id, "Monotonic", "First post")
193 + .await;
194 +
195 + let first_post: uuid::Uuid = sqlx::query_scalar(
196 + "SELECT id FROM posts WHERE thread_id = $1 ORDER BY created_at ASC LIMIT 1",
197 + )
198 + .bind(thread_id)
199 + .fetch_one(&h.db)
200 + .await
201 + .unwrap();
202 +
203 + let later_post =
204 + mt_db::mutations::create_post(&h.db, thread_id, user_id, "Later", "<p>Later</p>")
205 + .await
206 + .unwrap();
207 +
208 + mt_db::mutations::track_thread(&h.db, user_id, thread_id)
209 + .await
210 + .unwrap();
211 +
212 + let read_position = async || -> Option<uuid::Uuid> {
213 + sqlx::query_scalar(
214 + "SELECT last_read_post_id FROM tracked_threads WHERE user_id = $1 AND thread_id = $2",
215 + )
216 + .bind(user_id)
217 + .bind(thread_id)
218 + .fetch_one(&h.db)
219 + .await
220 + .unwrap()
221 + };
222 +
223 + // Forward from NULL: the first bump always takes.
224 + mt_db::mutations::update_read_position(&h.db, user_id, thread_id, later_post)
225 + .await
226 + .unwrap();
227 + assert_eq!(read_position().await, Some(later_post));
228 +
229 + // Backward: viewing an earlier page must leave the position alone.
230 + mt_db::mutations::update_read_position(&h.db, user_id, thread_id, first_post)
231 + .await
232 + .unwrap();
233 + assert_eq!(
234 + read_position().await,
235 + Some(later_post),
236 + "read position must not move backward to an earlier post"
237 + );
238 +
239 + // And the unread count stays settled rather than re-surfacing read posts.
240 + let tracked = mt_db::queries::list_tracked_threads(&h.db, user_id, 50, 0)
241 + .await
242 + .unwrap();
243 + assert_eq!(tracked.len(), 1);
244 + assert_eq!(
245 + tracked[0].unread_count, 0,
246 + "a backward bump must not resurrect already-read posts as unread"
247 + );
248 +
249 + // Re-bumping the same post is a no-op, not an error.
250 + mt_db::mutations::update_read_position(&h.db, user_id, thread_id, later_post)
251 + .await
252 + .unwrap();
253 + assert_eq!(read_position().await, Some(later_post));
254 + }
255 +
179 256 #[tokio::test]
180 257 async fn unread_count_tracking() {
181 258 let mut h = TestHarness::new().await;
@@ -45,7 +45,20 @@
45 45 Ok(())
46 46 }
47 47
48 - /// Update the read position for a tracked thread (set last_read_post_id to the last post).
48 + /// Advance the read position for a tracked thread to `last_post_id`.
49 + ///
50 + /// Monotonic: the position only ever moves forward in thread order. The caller
51 + /// (`routes::forum::thread`) bumps to the last post *on the page being viewed*,
52 + /// so an unguarded write would drag the position backward whenever a reader
53 + /// revisits an earlier page, and `list_tracked_threads` would then re-count
54 + /// already-read posts as unread. The two writes are also spawned off the
55 + /// response path, so concurrent views of different pages race; keeping the
56 + /// comparison in SQL means Postgres re-evaluates it against the winning row
57 + /// under read-committed rather than letting the last writer win.
58 + ///
59 + /// A `last_post_id` that names no row leaves the position untouched: the
60 + /// scalar subquery yields NULL, the comparison is not true, and the guard
61 + /// fails closed.
49 62 #[tracing::instrument(skip_all)]
50 63 pub async fn update_read_position(
51 64 pool: &PgPool,
@@ -55,7 +68,10 @@
55 68 ) -> Result<(), sqlx::Error> {
56 69 sqlx::query!(
57 70 "UPDATE tracked_threads SET last_read_post_id = $3
58 - WHERE user_id = $1 AND thread_id = $2",
71 + WHERE user_id = $1 AND thread_id = $2
72 + AND (last_read_post_id IS NULL
73 + OR (SELECT np.created_at FROM posts np WHERE np.id = $3)
74 + > (SELECT cp.created_at FROM posts cp WHERE cp.id = last_read_post_id))",
59 75 user_id,
60 76 thread_id,
61 77 last_post_id,
@@ -42,7 +42,8 @@
42 42 (SELECT COUNT(*) FROM categories c WHERE c.community_id = co.id) AS "category_count!",
43 43 (SELECT COUNT(*) FROM threads t
44 44 JOIN categories c2 ON c2.id = t.category_id
45 - WHERE c2.community_id = co.id) AS "thread_count!"
45 + WHERE c2.community_id = co.id
46 + AND t.deleted_at IS NULL) AS "thread_count!"
46 47 FROM communities co
47 48 WHERE co.suspended_at IS NULL
48 49 AND co.state <> 'archived'
@@ -80,7 +81,8 @@
80 81 (SELECT COUNT(*) FROM categories c WHERE c.community_id = co.id) AS "category_count!",
81 82 (SELECT COUNT(*) FROM threads t
82 83 JOIN categories c2 ON c2.id = t.category_id
83 - WHERE c2.community_id = co.id) AS "thread_count!"
84 + WHERE c2.community_id = co.id
85 + AND t.deleted_at IS NULL) AS "thread_count!"
84 86 FROM communities co
85 87 WHERE co.suspended_at IS NULL
86 88 AND co.state = 'archived'
@@ -91,6 +91,7 @@
91 91 WHERE t.deleted_at IS NULL
92 92 AND co.suspended_at IS NULL
93 93 AND p.removed_at IS NULL
94 + AND p.deleted_at IS NULL
94 95 AND p.search_tsv @@ q.tsq
95 96 AND ($3::text IS NULL OR co.slug = $3)
96 97 AND NOT EXISTS (SELECT 1 FROM thread_matches tm WHERE tm.thread_id = t.id)
@@ -1,6 +1,6 @@
1 1 {
2 2 "db_name": "PostgreSQL",
3 - "query": "SELECT co.name, co.slug, co.description,\n (SELECT COUNT(*) FROM categories c WHERE c.community_id = co.id) AS \"category_count!\",\n (SELECT COUNT(*) FROM threads t\n JOIN categories c2 ON c2.id = t.category_id\n WHERE c2.community_id = co.id) AS \"thread_count!\"\n FROM communities co\n WHERE co.suspended_at IS NULL\n AND co.state = 'archived'\n ORDER BY co.name\n LIMIT $1 OFFSET $2",
3 + "query": "SELECT co.name, co.slug, co.description,\n (SELECT COUNT(*) FROM categories c WHERE c.community_id = co.id) AS \"category_count!\",\n (SELECT COUNT(*) FROM threads t\n JOIN categories c2 ON c2.id = t.category_id\n WHERE c2.community_id = co.id\n AND t.deleted_at IS NULL) AS \"thread_count!\"\n FROM communities co\n WHERE co.suspended_at IS NULL\n AND co.state <> 'archived'\n ORDER BY co.name\n LIMIT $1 OFFSET $2",
4 4 "describe": {
5 5 "columns": [
6 6 {
@@ -43,5 +43,5 @@
43 43 null
44 44 ]
45 45 },
46 - "hash": "68bb07e798bc5550381a6c7dac4321b58a4ba0f1f863df60c0f43d458ef2aebf"
46 + "hash": "92a01d6aec22be89ec9e080575e3ece0346f854c99ea664560eceb38b127f9bf"
47 47 }
@@ -1,6 +1,6 @@
1 1 {
2 2 "db_name": "PostgreSQL",
3 - "query": "SELECT co.name, co.slug, co.description,\n (SELECT COUNT(*) FROM categories c WHERE c.community_id = co.id) AS \"category_count!\",\n (SELECT COUNT(*) FROM threads t\n JOIN categories c2 ON c2.id = t.category_id\n WHERE c2.community_id = co.id) AS \"thread_count!\"\n FROM communities co\n WHERE co.suspended_at IS NULL\n AND co.state <> 'archived'\n ORDER BY co.name\n LIMIT $1 OFFSET $2",
3 + "query": "SELECT co.name, co.slug, co.description,\n (SELECT COUNT(*) FROM categories c WHERE c.community_id = co.id) AS \"category_count!\",\n (SELECT COUNT(*) FROM threads t\n JOIN categories c2 ON c2.id = t.category_id\n WHERE c2.community_id = co.id\n AND t.deleted_at IS NULL) AS \"thread_count!\"\n FROM communities co\n WHERE co.suspended_at IS NULL\n AND co.state = 'archived'\n ORDER BY co.name\n LIMIT $1 OFFSET $2",
4 4 "describe": {
5 5 "columns": [
6 6 {
@@ -43,5 +43,5 @@
43 43 null
44 44 ]
45 45 },
46 - "hash": "faffc4231ca1cda3a3848f5b8a78138b1fdba1157e477b704888f58d7895818b"
46 + "hash": "9fe04a758f6d502ebba687a48f6e67d35238d0761d6ae3c75d8461891624d3e9"
47 47 }
@@ -1,0 +1,79 @@
1 + {
2 + "db_name": "PostgreSQL",
3 + "query": "WITH q AS (\n SELECT websearch_to_tsquery('english', $1) AS tsq\n ),\n thread_matches AS (\n SELECT t.id AS thread_id,\n t.title AS thread_title,\n COALESCE(u.display_name, u.username) AS author_username,\n co.name AS community_name,\n co.slug AS community_slug,\n c.name AS category_name,\n c.slug AS category_slug,\n LEFT(t.title, 200) AS snippet,\n t.last_activity_at,\n (ts_rank(t.search_tsv, q.tsq) * 2.0\n + similarity(t.title, $2)) AS rank\n FROM threads t\n JOIN categories c ON c.id = t.category_id\n JOIN communities co ON co.id = c.community_id\n JOIN users u ON u.mnw_account_id = t.author_id\n CROSS JOIN q\n WHERE t.deleted_at IS NULL\n AND co.suspended_at IS NULL\n AND (t.search_tsv @@ q.tsq\n OR t.title % $2)\n AND ($3::text IS NULL OR co.slug = $3)\n ),\n post_matches AS (\n SELECT DISTINCT ON (t.id)\n t.id AS thread_id,\n t.title AS thread_title,\n COALESCE(pu.display_name, pu.username) AS author_username,\n co.name AS community_name,\n co.slug AS community_slug,\n c.name AS category_name,\n c.slug AS category_slug,\n LEFT(p.body_markdown, 200) AS snippet,\n t.last_activity_at,\n ts_rank(p.search_tsv, q.tsq) AS rank\n FROM posts p\n JOIN threads t ON t.id = p.thread_id\n JOIN categories c ON c.id = t.category_id\n JOIN communities co ON co.id = c.community_id\n -- Author of the matched *reply* (`p.author_id`), not the thread OP\n -- (`t.author_id`): the snippet is the reply's body, so it must be\n -- attributed to whoever wrote it.\n JOIN users pu ON pu.mnw_account_id = p.author_id\n CROSS JOIN q\n WHERE t.deleted_at IS NULL\n AND co.suspended_at IS NULL\n AND p.removed_at IS NULL\n AND p.deleted_at IS NULL\n AND p.search_tsv @@ q.tsq\n AND ($3::text IS NULL OR co.slug = $3)\n AND NOT EXISTS (SELECT 1 FROM thread_matches tm WHERE tm.thread_id = t.id)\n ORDER BY t.id, ts_rank(p.search_tsv, q.tsq) DESC\n )\n SELECT\n thread_id AS \"thread_id!\",\n thread_title AS \"thread_title!\",\n author_username AS \"author_username!\",\n community_name AS \"community_name!\",\n community_slug AS \"community_slug!\",\n category_name AS \"category_name!\",\n category_slug AS \"category_slug!\",\n snippet AS \"snippet!\",\n last_activity_at AS \"last_activity_at!: chrono::DateTime<chrono::Utc>\",\n rank AS \"rank!\"\n FROM (\n SELECT * FROM thread_matches\n UNION ALL\n SELECT * FROM post_matches\n ) results\n ORDER BY rank DESC, last_activity_at DESC\n LIMIT $4",
4 + "describe": {
5 + "columns": [
6 + {
7 + "ordinal": 0,
8 + "name": "thread_id!",
9 + "type_info": "Uuid"
10 + },
11 + {
12 + "ordinal": 1,
13 + "name": "thread_title!",
14 + "type_info": "Text"
15 + },
16 + {
17 + "ordinal": 2,
18 + "name": "author_username!",
19 + "type_info": "Text"
20 + },
21 + {
22 + "ordinal": 3,
23 + "name": "community_name!",
24 + "type_info": "Text"
25 + },
26 + {
27 + "ordinal": 4,
28 + "name": "community_slug!",
29 + "type_info": "Text"
30 + },
31 + {
32 + "ordinal": 5,
33 + "name": "category_name!",
34 + "type_info": "Text"
35 + },
36 + {
37 + "ordinal": 6,
38 + "name": "category_slug!",
39 + "type_info": "Text"
40 + },
41 + {
42 + "ordinal": 7,
43 + "name": "snippet!",
44 + "type_info": "Text"
45 + },
46 + {
47 + "ordinal": 8,
48 + "name": "last_activity_at!: chrono::DateTime<chrono::Utc>",
49 + "type_info": "Timestamptz"
50 + },
51 + {
52 + "ordinal": 9,
53 + "name": "rank!",
54 + "type_info": "Float8"
55 + }
56 + ],
57 + "parameters": {
58 + "Left": [
59 + "Text",
60 + "Text",
61 + "Text",
62 + "Int8"
63 + ]
64 + },
65 + "nullable": [
66 + null,
67 + null,
68 + null,
69 + null,
70 + null,
71 + null,
72 + null,
73 + null,
74 + null,
75 + null
76 + ]
77 + },
78 + "hash": "621bd1c7591800e361cb9d05faeca2890182c691f37c0e9ec5d15434bb628d88"
79 + }
@@ -1,79 +1,0 @@
1 - {
2 - "db_name": "PostgreSQL",
3 - "query": "WITH q AS (\n SELECT websearch_to_tsquery('english', $1) AS tsq\n ),\n thread_matches AS (\n SELECT t.id AS thread_id,\n t.title AS thread_title,\n COALESCE(u.display_name, u.username) AS author_username,\n co.name AS community_name,\n co.slug AS community_slug,\n c.name AS category_name,\n c.slug AS category_slug,\n LEFT(t.title, 200) AS snippet,\n t.last_activity_at,\n (ts_rank(t.search_tsv, q.tsq) * 2.0\n + similarity(t.title, $2)) AS rank\n FROM threads t\n JOIN categories c ON c.id = t.category_id\n JOIN communities co ON co.id = c.community_id\n JOIN users u ON u.mnw_account_id = t.author_id\n CROSS JOIN q\n WHERE t.deleted_at IS NULL\n AND co.suspended_at IS NULL\n AND (t.search_tsv @@ q.tsq\n OR t.title % $2)\n AND ($3::text IS NULL OR co.slug = $3)\n ),\n post_matches AS (\n SELECT DISTINCT ON (t.id)\n t.id AS thread_id,\n t.title AS thread_title,\n COALESCE(pu.display_name, pu.username) AS author_username,\n co.name AS community_name,\n co.slug AS community_slug,\n c.name AS category_name,\n c.slug AS category_slug,\n LEFT(p.body_markdown, 200) AS snippet,\n t.last_activity_at,\n ts_rank(p.search_tsv, q.tsq) AS rank\n FROM posts p\n JOIN threads t ON t.id = p.thread_id\n JOIN categories c ON c.id = t.category_id\n JOIN communities co ON co.id = c.community_id\n -- Author of the matched *reply* (`p.author_id`), not the thread OP\n -- (`t.author_id`): the snippet is the reply's body, so it must be\n -- attributed to whoever wrote it.\n JOIN users pu ON pu.mnw_account_id = p.author_id\n CROSS JOIN q\n WHERE t.deleted_at IS NULL\n AND co.suspended_at IS NULL\n AND p.removed_at IS NULL\n AND p.search_tsv @@ q.tsq\n AND ($3::text IS NULL OR co.slug = $3)\n AND NOT EXISTS (SELECT 1 FROM thread_matches tm WHERE tm.thread_id = t.id)\n ORDER BY t.id, ts_rank(p.search_tsv, q.tsq) DESC\n )\n SELECT\n thread_id AS \"thread_id!\",\n thread_title AS \"thread_title!\",\n author_username AS \"author_username!\",\n community_name AS \"community_name!\",\n community_slug AS \"community_slug!\",\n category_name AS \"category_name!\",\n category_slug AS \"category_slug!\",\n snippet AS \"snippet!\",\n last_activity_at AS \"last_activity_at!: chrono::DateTime<chrono::Utc>\",\n rank AS \"rank!\"\n FROM (\n SELECT * FROM thread_matches\n UNION ALL\n SELECT * FROM post_matches\n ) results\n ORDER BY rank DESC, last_activity_at DESC\n LIMIT $4",
4 - "describe": {
5 - "columns": [
6 - {
7 - "ordinal": 0,
8 - "name": "thread_id!",
9 - "type_info": "Uuid"
10 - },
11 - {
12 - "ordinal": 1,
13 - "name": "thread_title!",
14 - "type_info": "Text"
15 - },
16 - {
17 - "ordinal": 2,
18 - "name": "author_username!",
19 - "type_info": "Text"
20 - },
21 - {
22 - "ordinal": 3,
23 - "name": "community_name!",
24 - "type_info": "Text"
25 - },
26 - {
27 - "ordinal": 4,
28 - "name": "community_slug!",
29 - "type_info": "Text"
30 - },
31 - {
32 - "ordinal": 5,
33 - "name": "category_name!",
34 - "type_info": "Text"
35 - },
36 - {
37 - "ordinal": 6,
38 - "name": "category_slug!",
39 - "type_info": "Text"
40 - },
41 - {
42 - "ordinal": 7,
43 - "name": "snippet!",
44 - "type_info": "Text"
45 - },
46 - {
47 - "ordinal": 8,
48 - "name": "last_activity_at!: chrono::DateTime<chrono::Utc>",
49 - "type_info": "Timestamptz"
50 - },
51 - {
52 - "ordinal": 9,
53 - "name": "rank!",
54 - "type_info": "Float8"
55 - }
56 - ],
57 - "parameters": {
58 - "Left": [
59 - "Text",
60 - "Text",
61 - "Text",
62 - "Int8"
63 - ]
64 - },
65 - "nullable": [
66 - null,
67 - null,
68 - null,
69 - null,
70 - null,
71 - null,
72 - null,
73 - null,
74 - null,
75 - null
76 - ]
77 - },
78 - "hash": "9327e3845bd956638d9589cfbc92ca9259524556d45ec2b954d88dd812ac1a10"
79 - }
@@ -1,0 +1,16 @@
1 + {
2 + "db_name": "PostgreSQL",
3 + "query": "UPDATE tracked_threads SET last_read_post_id = $3\n WHERE user_id = $1 AND thread_id = $2\n AND (last_read_post_id IS NULL\n OR (SELECT np.created_at FROM posts np WHERE np.id = $3)\n > (SELECT cp.created_at FROM posts cp WHERE cp.id = last_read_post_id))",
4 + "describe": {
5 + "columns": [],
6 + "parameters": {
7 + "Left": [
8 + "Uuid",
9 + "Uuid",
10 + "Uuid"
11 + ]
12 + },
13 + "nullable": []
14 + },
15 + "hash": "9f60124c6b7f6aa2be1dd6fdfe31eae415a92b783930ba25292014cf2c04e88e"
16 + }
@@ -1,16 +1,0 @@
1 - {
2 - "db_name": "PostgreSQL",
3 - "query": "UPDATE tracked_threads SET last_read_post_id = $3\n WHERE user_id = $1 AND thread_id = $2",
4 - "describe": {
5 - "columns": [],
6 - "parameters": {
7 - "Left": [
8 - "Uuid",
9 - "Uuid",
10 - "Uuid"
11 - ]
12 - },
13 - "nullable": []
14 - },
15 - "hash": "fa7ffad5cf9a95d6e7ec56fa6f3567d4a9098728992fc0f090f770f8be100112"
16 - }