Skip to content

mailcore: a partial delta item no longer blanks a message's headers (#1899) - #1902

Merged
yeroo merged 2 commits into
mainfrom
issue-1899-mailcore-a-partial-delta-item-mu
Nov 5, 2026
Merged

yeroo merged 2 commits into
mainfrom
issue-1899-mailcore-a-partial-delta-item-mu

Conversation

@yeroo

@yeroo yeroo commented Nov 5, 2026

Copy link
Copy Markdown
Owner

Closes #1899

What and why

A Microsoft Graph delta item may carry only some properties ({id, isRead} for a read change, a bare
{id} for an item created and changed between two rounds). Message::from_json defaults every absent
field to empty, and upsert_message overwrote every column, so new mail landed with no subject, sender
or date, and existing rows could be blanked.

  • MessageFields (graph/model.rs) records which properties a JSON item carried (a JSON null
    counts as carried). DeltaItem::Upsert is now Upsert(Message, MessageFields). This changes a public
    enum; nothing outside mailcore matches on it.
  • Store::merge_message writes only the carried columns: one static INSERT … ON CONFLICT DO UPDATE SET col = CASE WHEN ?present …. It handles every delta upsert. upsert_message now delegates to it
    with every field present, so there is a single statement. The meeting-request and draft flags are
    written when their key is present or the item is complete, which keeps today's behaviour for complete
    items.
  • Repair (Engine::repair_headerless) runs after each folder's delta. It handles up to 50 rows
    per folder per pass that have received_at = '' and are not docxy's own (excluded: local: ids,
    is_draft = 1, and local_id IS NOT NULL, i.e. drafts reconciled and then sent).
    • Each row is fetched with GET /me/messages/{id}?$select=… and merged.
    • A 404 deletes the row.
    • A 4xx or parse error skips that id and the repair continues.
    • Auth, throttle or transport errors abort the pass, as the delta call does.
    • This completes new header-less items in the same pass and repairs rows that older builds blanked.
  • Events. One SyncEvent::Error per folder names how many header-less rows are being fetched. A
    partial update merged into a complete row is silent. MessagesUpdated is sent once the delta is
    stored, before the repair, and again if the repair changed a row.

Tests

16 new tests across model, client, store and engine. The engine tests:

  • partial_delta_item_keeps_headers_and_fetches_new_item, the issue's acceptance case: M1 keeps its
    headers with the new isRead and is not refetched; M3 is fetched and stored complete; one notice.
  • headerless_row_is_repaired_on_next_pass
  • repair_deletes_row_graph_no_longer_has
  • repair_continues_past_a_failing_id
  • throttled_repair_still_reports_the_folders_delta

The store tests:

  • upsert_writes_every_column, which also fails if the schema gains a column the test does not cover
  • the draft and meeting-flag parity tests
  • headerless_ids_skip_reconciled_sent_draft

The implementer checked that the engine tests fail when the old upsert path is put back.

All of these pass: cargo fmt --check, cargo clippy -p mailcore -p lookxy --all-targets -- -D warnings,
cargo test -p mailcore (300 passed) and cargo test -p lookxy (333 passed). lookxy needed no change.

Review record

  • Plan: two rounds with the implementer. v2 added the single write path, the local_id exclusion,
    and skipping past a failing id; the implementer's draft-flag addendum was accepted.
  • revmux round 1: 4 Minor findings, 0 Major, all fixed in 251f676.
    • duplicated upsert SQL
    • the repair abort suppressing MessagesUpdated
    • the notice counting merged read changes
    • the notice being an Error; kept as the issue asks, and now rare and documented
  • review stopped: round 1 had no Major; all findings fixed

🤖 Generated with Claude Code

yeroo and others added 2 commits November 5, 2026 10:57
…1899)

A Graph delta item can carry only some of a message's properties
({id, isRead} for an update, a bare {id} for an item created and changed
between two rounds). Message::from_json turns every absent key into an
empty value and upsert_message overwrote every column with it, so new
mail landed as header-less rows (no subject, sender or date) and existing
rows could be blanked.

- graph::model::MessageFields records which keys an item carried (a JSON
  null counts as carried); is_partial() is "receivedDateTime, subject or
  from absent". DeltaItem::Upsert is now Upsert(Message, MessageFields):
  a change to a public mailcore enum (nothing outside mailcore matches on
  it in this repository).
- Store::merge_message writes only the carried columns on an existing row
  and inserts a missing one with what it has. Every delta item goes
  through it; with every key carried it writes what upsert_message writes
  (is_meeting_request and is_draft are written for a complete item even
  without @odata.type / isDraft, as before). upsert_message is unchanged.
- After a folder's delta pages, the engine completes up to 50 header-less
  rows (received_at empty; not local: ids, drafts or rows with a local_id,
  i.e. not docxy's own drafts after send) from the new
  GraphClient::get_message (GET /me/messages/{id}?$select=MESSAGE_SELECT):
  a 404 deletes the row, auth/throttle/transport abort the pass, any other
  failure skips that id and is reported once per folder. This also repairs
  rows an older build blanked.
- Partial items are reported as one SyncEvent::Error per folder per pass
  with their count and the folder's name.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…sert statement (#1899 r1)

- upsert_message is merge_message with every property carried, so the two
  cannot drift; upsert_writes_every_column pins every column by hand in
  place of the comparison that would now compare a path with itself.
- MessagesUpdated is emitted once a folder's delta pages are stored, before
  the header-less repair, and again only when the repair completed or
  deleted a row: a repair fetch that aborts the pass (401, 429/503,
  transport) no longer hides delta changes already in the store.
- The Error notice now comes from repair_headerless, once per folder when
  it finds header-less rows ("N message(s) in folder X were stored without
  headers; fetching them"), so a partial read change merged into a complete
  row is silent. SyncEvent::Error's doc names the notice.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 99.50434% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
mailcore/src/graph/model.rs 97.97% 2 Missing ⚠️
mailcore/src/store/mod.rs 99.70% 1 Missing ⚠️
mailcore/src/sync/engine.rs 99.68% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@yeroo
yeroo merged commit 84a2627 into main Nov 5, 2026
14 checks passed
@yeroo
yeroo deleted the issue-1899-mailcore-a-partial-delta-item-mu branch November 5, 2026 07:46
@yeroo

yeroo commented Nov 5, 2026

Copy link
Copy Markdown
Owner Author

Merged automatically (auto-merge is on for this checkout): review stopped: round 1 had no Major; all findings fixed; merge rounds: none; whole suite green on 251f676 (633 tests: cargo fmt --check, clippy -D warnings and cargo test for mailcore and lookxy; CI 14/14 passed); merge-check ok.

No follow-ups: all four Minor findings from revmux round 1 were fixed in this PR, nothing deferred or disputed.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

mailcore: a partial delta item must not blank a message's headers (new mail stored without subject, sender or date)

2 participants