From 3c778a8eabb1645c2d799d1998745c8a99eb1fad Mon Sep 17 00:00:00 2001 From: ajianaz Date: Thu, 10 Sep 2026 22:25:11 +0700 Subject: [PATCH 1/2] fix(comments): persist threads_comment_id, dedup re-fetches, in-place attribution backfill (#261) ## What - Store::insert_comment now accepts and persists threads_comment_id (column existed since migration 001 but the insert never wrote it, which made POST /api/comments/{id}/reply permanently fail with NO_THREADS_ID). - Dedup on insert: exact threads_comment_id match reuses the row; Meta's standard-access /replies may omit id/from for third-party commenters (observed + consistent with the docs note that username/ permalink only work for public users and your own user), so id-less rows dedup by (post_id, text). - Attribution backfill: when an id-less row later arrives with id and author, the legacy row is updated in place via COALESCE (original row id, sentiment and reply state preserved) instead of inserting a twin. - Empty-string Meta ids ("" when the field is absent) normalize to None on both the API and MCP fetch paths. - 5 new integration tests (store_comments.rs): id persistence, id dedup, text-fallback dedup, backfill-in-place, distinct-text separation. ## Why First real inbound comment exposed the pipeline end-to-end broken: text stored but author/id null, reply-via-API guaranteed to fail, and re-fetches duplicating rows. Verified live on v0.9.3 across two accounts (#261). Identity redaction behavior validated against the official Threads docs (username/permalink are conditional), so the text-fallback dedup is the durable design, not a workaround. ## Testing - 5 new integration tests, all green. - Full workspace: 245 passed / 0 failed. - clippy -D warnings clean, cargo fmt clean. --- crates/titen-api/src/routes/comments.rs | 3 + crates/titen-api/tests/store_comments.rs | 182 +++++++++++++++++++++ crates/titen-api/tests/store_timestamps.rs | 2 +- crates/titen-core/src/store.rs | 65 +++++++- crates/titen-mcp/src/main.rs | 3 + crates/titen-mcp/tests/mcp_integration.rs | 1 + 6 files changed, 254 insertions(+), 2 deletions(-) create mode 100644 crates/titen-api/tests/store_comments.rs diff --git a/crates/titen-api/src/routes/comments.rs b/crates/titen-api/src/routes/comments.rs index f47e1e9..472fbec 100644 --- a/crates/titen-api/src/routes/comments.rs +++ b/crates/titen-api/src/routes/comments.rs @@ -90,11 +90,14 @@ pub async fn fetch_comments( let mut stored = Vec::new(); for cd in &comment_data { let id = Uuid::now_v7().to_string(); + // Meta returns "" for missing ids under standard access — treat as None. + let tcid = Some(cd.threads_comment_id.as_str()).filter(|s| !s.is_empty()); match state .store .insert_comment( &id, &post_id, + tcid, cd.author_username.as_deref(), cd.author_user_id.as_deref(), &cd.text, diff --git a/crates/titen-api/tests/store_comments.rs b/crates/titen-api/tests/store_comments.rs new file mode 100644 index 0000000..8ab3dc0 --- /dev/null +++ b/crates/titen-api/tests/store_comments.rs @@ -0,0 +1,182 @@ +//! #261 — comment persistence: threads_comment_id stored, dedup by id, +//! text-fallback dedup for id-less rows (Meta standard access omits id/from), +//! and in-place attribution backfill. + +use sqlx::{ + SqlitePool, + sqlite::{SqliteConnectOptions, SqlitePoolOptions}, +}; +use std::str::FromStr; + +async fn pool() -> SqlitePool { + let options = SqliteConnectOptions::from_str("sqlite::memory:").expect("in-memory sqlite"); + let pool = SqlitePoolOptions::new() + .max_connections(1) // single connection = single in-memory DB + .connect_with(options) + .await + .expect("pool"); + let store = titen_core::Store::new(pool.clone()); + store.migrate().await.expect("migrations"); + seed_post(&store, &pool).await; + pool +} + +async fn seed_post(store: &titen_core::Store, pool: &SqlitePool) { + sqlx::query( + "INSERT INTO accounts (id, username, user_id, access_token, expires_at, is_active, created_at, updated_at) \ + VALUES ('acc-1', 'cmttest', '123654', 'FAKE_TEST_TOKEN', '2026-12-01T00:00:00+00:00', 1, datetime('now'), datetime('now')) \ + ON CONFLICT(id) DO NOTHING", + ) + .execute(pool) + .await + .expect("seed account"); + + let input = titen_core::models::CreatePost { + account_id: "acc-1".into(), + media_type: Some("CAROUSEL".into()), + caption: Some("post under test".into()), + text_attachment: None, + image_url: None, + video_url: None, + image_urls: None, + media_ids: None, + alt_text: None, + reply_to_id: None, + }; + store + .create_post("post-1", &input) + .await + .expect("seed post"); +} + +#[tokio::test] +async fn threads_comment_id_is_persisted() { + let pool = pool().await; + let store = titen_core::Store::new(pool.clone()); + + let c = store + .insert_comment( + "c1", + "post-1", + Some("17891234567890123"), + Some("alice"), + Some("u1"), + "hello", + ) + .await + .expect("insert"); + + assert_eq!(c.threads_comment_id.as_deref(), Some("17891234567890123")); + assert_eq!(c.author_username.as_deref(), Some("alice")); +} + +#[tokio::test] +async fn same_threads_id_dedups_to_one_row() { + let pool = pool().await; + let store = titen_core::Store::new(pool.clone()); + + let a = store + .insert_comment("c1", "post-1", Some("tc-1"), Some("alice"), None, "hello") + .await + .expect("first"); + let b = store + .insert_comment("c2", "post-1", Some("tc-1"), Some("alice"), None, "hello") + .await + .expect("second (dup)"); + + assert_eq!( + a.id, b.id, + "re-fetch of the same comment must reuse the row" + ); + let count: (i64,) = sqlx::query_as("SELECT COUNT(*) FROM comments WHERE post_id = 'post-1'") + .fetch_one(&pool) + .await + .expect("count"); + assert_eq!(count.0, 1); +} + +#[tokio::test] +async fn idless_comments_dedup_by_text() { + let pool = pool().await; + let store = titen_core::Store::new(pool.clone()); + + let a = store + .insert_comment("c1", "post-1", None, None, None, "anonymous question") + .await + .expect("first"); + let b = store + .insert_comment("c2", "post-1", None, None, None, "anonymous question") + .await + .expect("second (dup)"); + + assert_eq!(a.id, b.id, "id-less re-fetch must dedup by (post_id, text)"); + let count: (i64,) = sqlx::query_as("SELECT COUNT(*) FROM comments WHERE post_id = 'post-1'") + .fetch_one(&pool) + .await + .expect("count"); + assert_eq!(count.0, 1); +} + +#[tokio::test] +async fn attribution_backfill_fills_legacy_row_in_place() { + let pool = pool().await; + let store = titen_core::Store::new(pool.clone()); + + // Legacy fetch: Meta returned no id/from (standard access). + let legacy = store + .insert_comment("c1", "post-1", None, None, None, "How does supersede work?") + .await + .expect("legacy row"); + assert_eq!(legacy.threads_comment_id, None); + + // Later fetch (or enrichment): id + author now known, same text. + let backfilled = store + .insert_comment( + "c2", + "post-1", + Some("tc-backfill"), + Some("taufikabayy"), + Some("user_9"), + "How does supersede work?", + ) + .await + .expect("backfilled"); + + assert_eq!( + backfilled.id, legacy.id, + "backfill must update in place, not insert" + ); + assert_eq!( + backfilled.threads_comment_id.as_deref(), + Some("tc-backfill") + ); + assert_eq!(backfilled.author_username.as_deref(), Some("taufikabayy")); + assert_eq!(backfilled.author_user_id.as_deref(), Some("user_9")); + + let count: (i64,) = sqlx::query_as("SELECT COUNT(*) FROM comments WHERE post_id = 'post-1'") + .fetch_one(&pool) + .await + .expect("count"); + assert_eq!(count.0, 1); +} + +#[tokio::test] +async fn different_idless_comments_are_not_collapsed() { + let pool = pool().await; + let store = titen_core::Store::new(pool.clone()); + + store + .insert_comment("c1", "post-1", None, None, None, "first question") + .await + .expect("first"); + store + .insert_comment("c2", "post-1", None, None, None, "second question") + .await + .expect("second"); + + let count: (i64,) = sqlx::query_as("SELECT COUNT(*) FROM comments WHERE post_id = 'post-1'") + .fetch_one(&pool) + .await + .expect("count"); + assert_eq!(count.0, 2, "distinct texts must stay distinct rows"); +} diff --git a/crates/titen-api/tests/store_timestamps.rs b/crates/titen-api/tests/store_timestamps.rs index 5e2c22d..53e4a52 100644 --- a/crates/titen-api/tests/store_timestamps.rs +++ b/crates/titen-api/tests/store_timestamps.rs @@ -223,7 +223,7 @@ async fn comment_insert_fetched_at_is_canonical() { .expect("create post"); store - .insert_comment("cmt-t1", &post.id, Some("tester"), None, "nice") + .insert_comment("cmt-t1", &post.id, None, Some("tester"), None, "nice") .await .expect("insert comment"); diff --git a/crates/titen-core/src/store.rs b/crates/titen-core/src/store.rs index b2ba60d..95a74ae 100644 --- a/crates/titen-core/src/store.rs +++ b/crates/titen-core/src/store.rs @@ -1519,24 +1519,87 @@ impl Store { .map_err(Into::into) } + /// Insert a fetched comment, with dedup (#261). + /// + /// Natural key: `threads_comment_id` when Meta returns one. Meta's + /// standard-access `/replies` may omit `id`/`from` for third-party + /// commenters — in that case dedup falls back to `(post_id, text)` among + /// rows that also lack a Threads id, so re-fetches no longer duplicate + /// rows. When an id-less row later gets attributed (backfill case), the + /// existing row is updated in place: threads_comment_id/author fields are + /// filled via COALESCE, fetched_at is bumped, and the row id (plus its + /// sentiment/reply state) is preserved. pub async fn insert_comment( &self, id: &str, post_id: &str, + threads_comment_id: Option<&str>, author_username: Option<&str>, author_user_id: Option<&str>, text: &str, ) -> Result { + let existing: Option = if let Some(tcid) = threads_comment_id { + // Exact id match first; else an id-less row with identical text + // (attribution backfill onto a legacy row). + sqlx::query_as::<_, Comment>( + "SELECT * FROM comments \ + WHERE post_id = ?1 AND (threads_comment_id = ?2 OR (threads_comment_id IS NULL AND text = ?3)) \ + ORDER BY (threads_comment_id IS NOT NULL) DESC LIMIT 1", + ) + .bind(post_id) + .bind(tcid) + .bind(text) + .fetch_optional(&self.pool) + .await? + } else { + sqlx::query_as::<_, Comment>( + "SELECT * FROM comments \ + WHERE post_id = ?1 AND text = ?2 AND threads_comment_id IS NULL", + ) + .bind(post_id) + .bind(text) + .fetch_optional(&self.pool) + .await? + }; + // Explicit canonical fetched_at so the row never depends on the // space-format schema default, which breaks RFC3339 comparisons. let fetched_at = chrono::Utc::now() .format(crate::time::CANONICAL_FMT) .to_string(); + + if let Some(prev) = existing { + // Refresh attribution + freshness; keep the original row id, + // sentiment, and reply state. + sqlx::query( + "UPDATE comments SET \ + threads_comment_id = COALESCE(?1, threads_comment_id), \ + author_username = COALESCE(?2, author_username), \ + author_user_id = COALESCE(?3, author_user_id), \ + fetched_at = ?4 \ + WHERE id = ?5", + ) + .bind(threads_comment_id) + .bind(author_username) + .bind(author_user_id) + .bind(&fetched_at) + .bind(&prev.id) + .execute(&self.pool) + .await?; + + return sqlx::query_as::<_, Comment>("SELECT * FROM comments WHERE id = ?") + .bind(&prev.id) + .fetch_one(&self.pool) + .await + .map_err(Into::into); + } + sqlx::query( - "INSERT INTO comments (id, post_id, author_username, author_user_id, text, fetched_at) VALUES (?, ?, ?, ?, ?, ?)", + "INSERT INTO comments (id, post_id, threads_comment_id, author_username, author_user_id, text, fetched_at) VALUES (?, ?, ?, ?, ?, ?, ?)", ) .bind(id) .bind(post_id) + .bind(threads_comment_id) .bind(author_username) .bind(author_user_id) .bind(text) diff --git a/crates/titen-mcp/src/main.rs b/crates/titen-mcp/src/main.rs index b9e4574..d8abbdc 100644 --- a/crates/titen-mcp/src/main.rs +++ b/crates/titen-mcp/src/main.rs @@ -768,10 +768,13 @@ fn handle_tool_call( let mut stored = Vec::new(); for cd in &comment_data { let id = uuid::Uuid::now_v7().to_string(); + // Meta returns "" for missing ids under standard access (#261). + let tcid = Some(cd.threads_comment_id.as_str()).filter(|s| !s.is_empty()); match store .insert_comment( &id, post_id, + tcid, cd.author_username.as_deref(), cd.author_user_id.as_deref(), &cd.text, diff --git a/crates/titen-mcp/tests/mcp_integration.rs b/crates/titen-mcp/tests/mcp_integration.rs index d44f69e..0675734 100644 --- a/crates/titen-mcp/tests/mcp_integration.rs +++ b/crates/titen-mcp/tests/mcp_integration.rs @@ -310,6 +310,7 @@ async fn test_insert_and_list_comment() { .insert_comment( "comment-1", "post-1", + None, Some("commenter"), Some("user_456"), "Great post!", From eb4c9563119c4a238747c1bb355ba269f6bfadb4 Mon Sep 17 00:00:00 2001 From: ajianaz Date: Thu, 10 Sep 2026 22:35:45 +0700 Subject: [PATCH 2/2] =?UTF-8?q?fix(comments):=20author-aware=20dedup=20key?= =?UTF-8?q?s=20=E2=80=94=20address=20CodeCora=20code-scanning=20findings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Id-less dedup now matches author too: two different users posting identical text stay distinct rows. - Attribution backfill restricted to same-author or still-anonymous rows (most recent candidate wins): an unrelated commenter's row can never absorb the incoming threads_comment_id. - +2 regression tests covering both scenarios (7 total in suite). Workspace: 247 passed / 0 failed; clippy -D warnings + fmt clean. --- crates/titen-api/tests/store_comments.rs | 68 ++++++++++++++++++++++++ crates/titen-core/src/store.rs | 40 ++++++++++---- 2 files changed, 99 insertions(+), 9 deletions(-) diff --git a/crates/titen-api/tests/store_comments.rs b/crates/titen-api/tests/store_comments.rs index 8ab3dc0..5291aae 100644 --- a/crates/titen-api/tests/store_comments.rs +++ b/crates/titen-api/tests/store_comments.rs @@ -180,3 +180,71 @@ async fn different_idless_comments_are_not_collapsed() { .expect("count"); assert_eq!(count.0, 2, "distinct texts must stay distinct rows"); } + +#[tokio::test] +async fn same_text_from_different_authors_stays_distinct() { + let pool = pool().await; + let store = titen_core::Store::new(pool.clone()); + + // Meta omits id AND author for both — but authors were known on earlier + // fetches. Identical text from different users must NOT collapse. + store + .insert_comment("c1", "post-1", None, Some("alice"), None, "Great post!") + .await + .expect("alice row"); + store + .insert_comment("c2", "post-1", None, Some("bob"), None, "Great post!") + .await + .expect("bob row"); + + let count: (i64,) = sqlx::query_as("SELECT COUNT(*) FROM comments WHERE post_id = 'post-1'") + .fetch_one(&pool) + .await + .expect("count"); + assert_eq!(count.0, 2, "same text + different authors = distinct rows"); +} + +#[tokio::test] +async fn backfill_never_grafts_onto_another_authors_row() { + let pool = pool().await; + let store = titen_core::Store::new(pool.clone()); + + // alice's id-less comment… + let alice = store + .insert_comment( + "c1", + "post-1", + None, + Some("alice"), + None, + "How does supersede work?", + ) + .await + .expect("alice row"); + + // …then the SAME text arrives attributed to taufikabayy (a different + // person who coincidentally wrote the same words). It must become its + // own row, NOT graft the id onto alice's row. + let other = store + .insert_comment( + "c2", + "post-1", + Some("tc-x"), + Some("taufikabayy"), + Some("user_9"), + "How does supersede work?", + ) + .await + .expect("taufik row"); + + assert_ne!(alice.id, other.id); + assert_eq!( + alice.threads_comment_id, None, + "alice's row must stay untouched" + ); + assert_eq!(other.threads_comment_id.as_deref(), Some("tc-x")); + + let alice_after = store.get_comment(&alice.id).await.expect("reload"); + assert_eq!(alice_after.author_username.as_deref(), Some("alice")); + assert_eq!(alice_after.threads_comment_id, None); +} diff --git a/crates/titen-core/src/store.rs b/crates/titen-core/src/store.rs index 95a74ae..6e4dba3 100644 --- a/crates/titen-core/src/store.rs +++ b/crates/titen-core/src/store.rs @@ -1539,25 +1539,47 @@ impl Store { text: &str, ) -> Result { let existing: Option = if let Some(tcid) = threads_comment_id { - // Exact id match first; else an id-less row with identical text - // (attribution backfill onto a legacy row). - sqlx::query_as::<_, Comment>( - "SELECT * FROM comments \ - WHERE post_id = ?1 AND (threads_comment_id = ?2 OR (threads_comment_id IS NULL AND text = ?3)) \ - ORDER BY (threads_comment_id IS NOT NULL) DESC LIMIT 1", + // Exact id match first. + let exact = sqlx::query_as::<_, Comment>( + "SELECT * FROM comments WHERE post_id = ?1 AND threads_comment_id = ?2 LIMIT 1", ) .bind(post_id) .bind(tcid) - .bind(text) .fetch_optional(&self.pool) - .await? + .await?; + if exact.is_some() { + exact + } else { + // Attribution backfill (#261): a legacy id-less row with the + // same text may be enriched with the incoming id/author. + // Restricted to same-author or still-anonymous rows so an + // unrelated commenter's row can never absorb the id, and the + // most recently fetched candidate wins. + sqlx::query_as::<_, Comment>( + "SELECT * FROM comments \ + WHERE post_id = ?1 AND text = ?2 AND threads_comment_id IS NULL \ + AND (author_username IS ?3 OR author_username IS NULL) \ + ORDER BY (author_username IS NULL) ASC, fetched_at DESC LIMIT 1", + ) + .bind(post_id) + .bind(text) + .bind(author_username) + .fetch_optional(&self.pool) + .await? + } } else { + // Id-less incoming (Meta omitted id/from): dedup only against + // id-less rows with the same text AND the same (possibly absent) + // author — two different users posting identical text stay + // distinct rows. sqlx::query_as::<_, Comment>( "SELECT * FROM comments \ - WHERE post_id = ?1 AND text = ?2 AND threads_comment_id IS NULL", + WHERE post_id = ?1 AND text = ?2 AND threads_comment_id IS NULL \ + AND author_username IS ?3", ) .bind(post_id) .bind(text) + .bind(author_username) .fetch_optional(&self.pool) .await? };