Skip to content

Commit bd1c713

Browse files
jamiepineclaude
andcommitted
further review fixes: treat absent FK fields as failed, drop NULL indexed_at
convert_fks_to_uuids_batch previously treated a missing local_field as null. convert_fk_to_uuid errors in that case — only an explicit JSON null is a legitimate null FK — so the batch helper now flags absent fields as failed to match the per-record contract. Entry::query_for_sync no longer falls back to modified_at when indexed_at is NULL. The cursor filter/order uses indexed_at exclusively, so a returned cursor derived from modified_at wouldn't match the next query's predicate. The indexed_at backfill migration populated existing rows; any NULL here is a data bug, logged and skipped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 96debbd commit bd1c713

2 files changed

Lines changed: 29 additions & 7 deletions

File tree

core/src/infra/db/entities/entry.rs

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,21 @@ impl crate::infra::sync::Syncable for Model {
302302
None => continue,
303303
};
304304

305+
// The sync cursor filters/orders on `indexed_at`, so a row with a
306+
// NULL `indexed_at` would emit a cursor the next query predicate
307+
// doesn't represent. Skip them — the indexed_at backfill migration
308+
// populated every existing row, so this should be unreachable.
309+
let indexed_at = match entry.indexed_at {
310+
Some(ts) => ts,
311+
None => {
312+
tracing::warn!(
313+
uuid = %uuid,
314+
"Entry has NULL indexed_at; skipping sync until it's populated"
315+
);
316+
continue;
317+
}
318+
};
319+
305320
let mut json = match entry.to_sync_json() {
306321
Ok(j) => j,
307322
Err(e) => {
@@ -323,8 +338,7 @@ impl crate::infra::sync::Syncable for Model {
323338
}
324339
}
325340

326-
let timestamp = entry.indexed_at.unwrap_or(entry.modified_at);
327-
staged.push((uuid, json, timestamp));
341+
staged.push((uuid, json, indexed_at));
328342
}
329343

330344
// Batch-convert FK integer IDs to UUIDs one FK type at a time across

core/src/infra/sync/fk_mapper.rs

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -127,13 +127,20 @@ pub async fn convert_fks_to_uuids_batch(
127127

128128
let uuid_field = fk.uuid_field_name();
129129

130-
// First pass: collect IDs and flag records whose local_field isn't a valid
131-
// integer so they can be dropped by the caller.
130+
// First pass: collect IDs and flag records that can't be resolved. An
131+
// absent `local_field` is treated as failed (matches `convert_fk_to_uuid`
132+
// semantics — only an explicit JSON `null` is a legitimate null FK).
132133
let mut ids_to_lookup: HashSet<i32> = HashSet::new();
133134
for (idx, json) in records.iter().enumerate() {
134135
match json.get(fk.local_field) {
135-
None => { /* absent — treated as null below */ }
136-
Some(v) if v.is_null() => { /* null — treated as null below */ }
136+
None => {
137+
tracing::warn!(
138+
fk_field = fk.local_field,
139+
"FK field missing in sync payload; dropping record"
140+
);
141+
failed.insert(idx);
142+
}
143+
Some(v) if v.is_null() => { /* explicit null — treated as null below */ }
137144
Some(v) => match v.as_i64() {
138145
Some(id) => {
139146
ids_to_lookup.insert(id as i32);
@@ -165,7 +172,8 @@ pub async fn convert_fks_to_uuids_batch(
165172

166173
match local_field_value {
167174
None => {
168-
json[&uuid_field] = Value::Null;
175+
// First pass already flagged this as failed; nothing to do.
176+
continue;
169177
}
170178
Some(v) if v.is_null() => {
171179
json[&uuid_field] = Value::Null;

0 commit comments

Comments
 (0)