fix(mail): file Gmail TRASH as spam instead of live support work - #182
fix(mail): file Gmail TRASH as spam instead of live support work#182zaridan wants to merge 3 commits into
Conversation
…rits it The verdict table defines 'clean' as an affirmative judgment, but the Gmail implementation derives it from a bare absence of the SPAM label. Those are different claims, and the gap is invisible today only because §4.1 files 'clean' and 'unknown' identically. They collapse for Gmail because Gmail classifies every message it accepts, so absence of the junk marker IS the verdict. Record that as the specific licence it is, bind future transports to earning 'clean' the same way, and note that §3.2's header scoring is what makes the distinction load-bearing. No behaviour change. Decision D6 added to the §7 ledger.
A Gmail message carrying TRASH fell through spamVerdictOf to 'clean' and was created as an ordinary `active` conversation. Mail the operator's own mailbox had already thrown away — by a delete-on-arrival filter, or by hand before the reconcile window ran — became visible support work in the inbox. TRASH reaches the handler for the same reason SPAM does: history.list is an unfiltered delta stream and the client set-unions labelsAdded without ever REMOVING a label, so both ['TRASH'] and ['INBOX','TRASH'] arrive intact. spamVerdictOf now maps either label to 'spam'. This widens what that value asserts — from 'a classifier called this junk' to 'the source mailbox has already discarded this as unwanted' — so §3.1's definition widens with it rather than letting the field lie. Nothing downstream branches on which of the two produced the verdict; the only thing either may change is the status a brand-new conversation is born at. Nothing is dropped (inbound-ingestion.md §1 invariant #3): the message is parsed, stored, threaded and attachment-linked identically, sits readable in the Spam folder, and a reply reopens it to `active` (§4a) — so a message trashed in error self-heals. Not a regression from the spam-verdict work; before it, every ingested message became `active` regardless of labels. Decisions D7 and D7a added to the §7 ledger.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Adversarial review of the TRASH mapping surfaced a real one: the history client requests messageAdded and labelAdded, never labelRemoved, so labelIds is the set of labels a message has EVER carried in the window rather than its state now. Delete a just-arrived message, hit undo, and TRASH stays in the union — the conversation is filed spam while sitting in the operator's Inbox. The overapproximation is not new (mark-as-junk then not-junk has always had it), but TRASH extends it to a one-click action Gmail offers to undo, so the exposure is materially larger and should not go unrecorded. Accepted rather than fixed: reading labelRemoved would re-open the SENT/INBOX split-delta race the union was introduced to close, and the misfile is bounded and self-correcting — fully stored, readable in the Spam folder, and reopened to active by any reply. Decision D8 added to the §7 ledger, marked INFERRED.
Adversarial review pass — adjudicatedSubstitution disclosed: CodeRabbit was not used. This PR's base is 6 findings — 1 real and fixed, 1 rejected, 4 confirmed non-defects.
Finding 1 — verified independently, then accepted with a recordI checked the mechanism rather than taking the reviewer's word: The overapproximation is not new (mark-as-junk-then-not-junk has always had it), but Accepted, not fixed in code ( This adds a second INFERRED row, so the verdict stays 🟡. D8 was never put to the maintainer as its own question. Gates re-run on |
1 participant
🟡 NEEDS YOUR DECISION
One derived rule is INFERRED (D7a, below). No review bot has run yet — this opens 🟡 by protocol and moves to 🟢 only after a bot review lands on the current head and its findings are adjudicated.
Stacked on #180 (base is
feat/mail-spam-classification, notmain). Retargets tomainonce #180 merges. Review bots skip non-mainbase branches, so the required review pass will be an adversarial substitute run and disclosed as such.Decision provenance
spam" from three options (separate signal filed asclosed/ file asspam/ leave as-is)No one-way doors. Nothing here writes to the operator's mailbox (D3 is "no writeback"), no schema change, no public promise altered. Filing is reversible in the UI and self-heals on reply.
What changed
spamVerdictOfmapped aTRASH-labeled Gmail message to'clean', soingestcreated it as an ordinaryactiveconversation. Mail the operator's mailbox had already discarded — by a delete-on-arrival filter, or by hand before the reconcile window ran — became live support work.TRASHreaches the handler for the same reasonSPAMdoes:history.listis an unfiltered delta stream, and the history client set-unionslabelsAddedwithout ever removing a label. So both['TRASH']and['INBOX','TRASH']arrive intact.Three files:
src/mail/gmail-reconcile.ts—SPAMorTRASH⇒'spam'. One condition; the rest is the doc comment explaining thatTRASHis an operator action rather than a classifier verdict, and why they are nonetheless collapsed.specs/mail/spam-classification.md— §3.1's'spam'definition widens to match what the value now asserts, with the two-meanings consequence stated for §3.2's unbuilt header scoring; §1 and the delta-window caveat pick upTRASH; §7 gains D6, D7, D7a. Separately, the'clean'honesty gap (§3.1) is closed in prose: each transport must now earn'clean', and Gmail's licence for reading bare absence-of-SPAMthat way is stated as the Gmail-specific fact it is.src/mail/gmail-reconcile.test.ts— four cases:['TRASH'],['INBOX','TRASH'](provingINBOXdoesn't out-vote it — the case that regressed),['SPAM','TRASH'](D7a), and['SENT','TRASH']still skipped by the self-echo filter with no raw fetch.Invariants held
TRASHmessage is parsed, stored, threaded and attachment-linked exactly as before — one column differs. It is readable in the Spam folder and a reply reopens it toactive(agent-inbox-v1.md §4a), so a message trashed in error self-heals.createConversationInTxsites; a reply threading onto an existing conversation never reads it (spam-classification.md §4.2).activeregardless of labels.INBOXonly, so a server-side delete means the message is never fetched — the field stays'unknown', which is the honest verdict.Known limit, unchanged
A
TRASHlabel applied after the reconcile window'shistory.listsnapshot is not covered — that message is already ingested asactive, and the next reconcile deliberately ignoreslabelsAddedfor an id it did not itself newly add. Deleting a message you have already seen arrive in Helpthread does not retroactively file it. That is §5's reclassification problem, recorded in the spec rather than silently left as a surprise.Gates
Run locally on this head, exit codes captured directly:
npm run typecheck— exit 0biome check .— exit 0, 318 files checkednpm test— exit 0, 87 files / 1762 tests passedReview
Pending — no bot has run. Verdict stays 🟡 until a review lands on the current head and every finding gets an individual verdict.