feat(email): Set email sender as Signal/Noise with email_filters#2190
Merged
evanhutnik merged 4 commits intomainfrom Mar 26, 2026
Merged
feat(email): Set email sender as Signal/Noise with email_filters#2190evanhutnik merged 4 commits intomainfrom
evanhutnik merged 4 commits intomainfrom
Conversation
Contributor
📝 WalkthroughWalkthroughAdds sender-based email filtering end-to-end: frontend actions, UI menu items, hotkeys, context handlers, and client queries; server-side API, DB migration, repo, service, dynamic-query logic, fixtures, and tests to persist and evaluate "signal"/"noise" sender filters. Changes
Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@js/app/packages/app/component/next-soup/actions/make-sender-filter-action.ts`:
- Around line 11-16: The dedupe uses raw entity.senderEmail so mixed-case or
whitespace variants slip through; compute a normalized key (e.g., const key =
entity.senderEmail.trim().toLowerCase()) and use that for seen.has(...) and
seen.add(...), while still passing the original entity.senderEmail to await
action(...) if you want to preserve original casing in the UI/API call.
In `@js/app/packages/queries/email/thread.ts`:
- Around line 431-449: The undo currently always deletes the created/updated
rule using filterId (result[1].filter.id) which loses any previous preference;
change the logic in the toast success undo handler (the onClick that calls
emailClient.deleteEmailFilter) to capture the prior rule state before upserting
(e.g., previous is_important) and on undo either restore the previous row via an
upsert/update call (re-apply the prior is_important and other fields) or delete
only if the rule was newly created; reference the upsert response
(result[1].filter), the variable filterId, and replace the delete-only call to
emailClient.deleteEmailFilter with conditional logic that restores the previous
override using emailClient.upsertEmailFilter or updates the existing filter to
the previous is_important value.
- Around line 421-449: The upsertEmailFilter/deleteEmailFilter flows update
which tab a thread belongs to but never refresh the Signal/Noise lists; after a
successful upsert (successful result in upsertEmailFilter) and after a
successful undo (successful result in deleteEmailFilter inside the toast
onClick), call the cache refresh/invalidate routine that the app uses (e.g.,
queryClient.invalidateQueries or the app's refresh helper) for the preview and
soup queries so the Signal/Noise lists immediately reflect the change; add those
invalidations in the success branch after using result[1].filter.id and in the
else branch where undo succeeds.
In `@rust/cloud-storage/email/src/outbound/email_pg_repo/mod.rs`:
- Around line 261-287: The upsert_email_filter implementation currently prefers
email_address when both fields are set and panics with unreachable! when neither
is set; change it to explicitly validate UpsertEmailFilterInput shapes and
return an error for invalid shapes instead of panicking or silently choosing one
field. Update async fn upsert_email_filter to: handle the exact two valid cases
(Some(address) && None => call email_filter::upsert_email_filter_by_address,
None && Some(domain) => call email_filter::upsert_email_filter_by_domain), and
for the other shapes (both Some or both None) return an appropriate
Err(Self::Err) (create or reuse a descriptive error variant such as
InvalidFilterShape or BadRequest) so callers get a recoverable error rather than
an unwind or silent preference. Ensure the returned error flows through the
function signature and tests cover both invalid-shape cases.
In
`@rust/cloud-storage/macro_db_client/migrations/20260325102949_email_filters.sql`:
- Around line 10-13: The XOR constraint allows empty strings; update the CHECK
for email_filters_address_xor_domain_chk to require a non-empty value by
ensuring (email_address IS NOT NULL AND trim(email_address) <> '') is exclusive
with (email_domain IS NOT NULL AND trim(email_domain) <> ''); also tighten
email_filters_email_domain_format_chk to only evaluate the position check when
email_domain is non-empty/null (e.g., require email_domain IS NULL OR
(trim(email_domain) <> '' AND position('@' IN email_domain) = 0)) so empty
strings are rejected for both email_address and email_domain.
- Around line 1-25: The migration added the email_filters table and indexes but
there are no Rust queries to exercise SQLx and generate its cache; add query
implementations in macro_db_client/src (for example functions named
upsert_email_filter, list_email_filters, delete_email_filter) that contain the
SQL statements (INSERT ... ON CONFLICT ... RETURNING / SELECT ... WHERE link_id
= $1 / DELETE ... WHERE id = $1) matching the new schema, ensure they use the
exact column names (email_address, email_domain, is_important, link_id) and any
lower(...) index usage in WHERE clauses, then run just prepare_db in the
macro_db_client directory to update SQLx cache artifacts; if the table is
intentionally schema-only, add a short note in the PR explaining that instead of
adding queries.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0333dd5b-c062-40c7-a23d-6ce91be2fe37
⛔ Files ignored due to path filters (24)
js/app/packages/service-clients/service-email/generated/client.tsis excluded by!**/generated/**js/app/packages/service-clients/service-email/generated/schemas/apiEmailFilter.tsis excluded by!**/generated/**js/app/packages/service-clients/service-email/generated/schemas/apiEmailFilterEmailAddress.tsis excluded by!**/generated/**js/app/packages/service-clients/service-email/generated/schemas/apiEmailFilterEmailDomain.tsis excluded by!**/generated/**js/app/packages/service-clients/service-email/generated/schemas/index.tsis excluded by!**/generated/**js/app/packages/service-clients/service-email/generated/schemas/listEmailFiltersResponse.tsis excluded by!**/generated/**js/app/packages/service-clients/service-email/generated/schemas/upsertEmailFilterRequest.tsis excluded by!**/generated/**js/app/packages/service-clients/service-email/generated/schemas/upsertEmailFilterRequestEmailAddress.tsis excluded by!**/generated/**js/app/packages/service-clients/service-email/generated/schemas/upsertEmailFilterRequestEmailDomain.tsis excluded by!**/generated/**js/app/packages/service-clients/service-email/generated/schemas/upsertEmailFilterResponse.tsis excluded by!**/generated/**rust/cloud-storage/email/.sqlx/query-37b5168f739c490a3373c65eab35f247a34af0a8e659cbad61cb99e3094e5a60.jsonis excluded by!**/.sqlx/**rust/cloud-storage/email/.sqlx/query-3a7be99469ed150e0506efa786b672198a6d3419c6bd5548418520cd3e6a8e95.jsonis excluded by!**/.sqlx/**rust/cloud-storage/email/.sqlx/query-48df79c41ba4ec7fa2454ac962aee9ff97875c9e2335b46fe13b2bbdee721186.jsonis excluded by!**/.sqlx/**rust/cloud-storage/email/.sqlx/query-4afa79ca95bdbcebe9d3de7ebb204636419e6cf89e650a8716c40ab2e588aa5b.jsonis excluded by!**/.sqlx/**rust/cloud-storage/email/.sqlx/query-4c695b011d49ba7d5fce4b309875ed952dc386ac760444d7883c5a76718f15be.jsonis excluded by!**/.sqlx/**rust/cloud-storage/email/.sqlx/query-54e3755d63a676a7fa70da77321e618032f0130c73eb6b0ae985f07c5c6c448b.jsonis excluded by!**/.sqlx/**rust/cloud-storage/email/.sqlx/query-679d0dd86957b91b759135cf9fc7314630cfd377b004a8c1dfedbf5764137ebd.jsonis excluded by!**/.sqlx/**rust/cloud-storage/email/.sqlx/query-7617f808df94ffe77c8195a911ee6b230a21fe9935e099d1310d244248ca253a.jsonis excluded by!**/.sqlx/**rust/cloud-storage/email/.sqlx/query-79d3bc72e6f424765706f7deea426379ae49cbd70dd122daedd95343618b9ac6.jsonis excluded by!**/.sqlx/**rust/cloud-storage/email/.sqlx/query-96d853b887f2e6d7aac6b15ddd2b71a8d8900d2ef30047da7ca1a5940503b6d0.jsonis excluded by!**/.sqlx/**rust/cloud-storage/email/.sqlx/query-b3aeddd507e0efee0e0cecef6eae7503820f0f3302e95a459790c5cb78e5dfb0.jsonis excluded by!**/.sqlx/**rust/cloud-storage/email/.sqlx/query-c4d2fed1a0fb95c20ef9b4049cc5cacfb7f0a441c630528d2cde076211ff63f9.jsonis excluded by!**/.sqlx/**rust/cloud-storage/email/.sqlx/query-e70d5acce88125ae894d9d57356559c6e59e236c9650211a52fdc90602318cc3.jsonis excluded by!**/.sqlx/**rust/cloud-storage/email/.sqlx/query-e7235d6ad2033a2ff7be0bf57ab042cd71e3b3d0e1f8a9750e55cdda92e70c6f.jsonis excluded by!**/.sqlx/**
📒 Files selected for processing (31)
js/app/packages/app/component/next-soup/actions/index.tsjs/app/packages/app/component/next-soup/actions/make-mark-sender-important-action.tsjs/app/packages/app/component/next-soup/actions/make-mark-sender-noise-action.tsjs/app/packages/app/component/next-soup/actions/make-sender-filter-action.tsjs/app/packages/app/component/next-soup/filters/inbox-filters.tsjs/app/packages/app/component/next-soup/soup-view/soup-entity-actions-menu.tsxjs/app/packages/block-email/component/Email.tsxjs/app/packages/block-email/component/EmailContext.tsxjs/app/packages/block-email/util/emailHotkeys.tsjs/app/packages/core/constant/featureFlags.tsjs/app/packages/core/hotkey/tokens.tsjs/app/packages/queries/email/thread.tsjs/app/packages/service-clients/service-email/client.tsjs/app/packages/service-clients/service-email/openapi.jsonrust/cloud-storage/email/fixtures/email_dynamic_query_email_filters.sqlrust/cloud-storage/email/src/domain/models/email_filter.rsrust/cloud-storage/email/src/domain/models/error.rsrust/cloud-storage/email/src/domain/models/mod.rsrust/cloud-storage/email/src/domain/ports.rsrust/cloud-storage/email/src/domain/service/mod.rsrust/cloud-storage/email/src/inbound/axum.rsrust/cloud-storage/email/src/inbound/axum/email_filter_router.rsrust/cloud-storage/email/src/outbound/email_pg_repo/dynamic/filters.rsrust/cloud-storage/email/src/outbound/email_pg_repo/dynamic/tests.rsrust/cloud-storage/email/src/outbound/email_pg_repo/email_filter.rsrust/cloud-storage/email/src/outbound/email_pg_repo/mod.rsrust/cloud-storage/email/src/outbound/email_pg_repo/test/dynamic_query.rsrust/cloud-storage/email_service/src/api/email/filters/mod.rsrust/cloud-storage/email_service/src/api/email/mod.rsrust/cloud-storage/email_service/src/api/swagger.rsrust/cloud-storage/macro_db_client/migrations/20260325102949_email_filters.sql
js/app/packages/app/component/next-soup/actions/make-sender-filter-action.ts
Show resolved
Hide resolved
Comment on lines
+431
to
+449
| const filterId = result[1].filter.id; | ||
|
|
||
| toast.success( | ||
| `Sender marked as ${label}`, | ||
| `Messages from ${senderEmail} will appear in ${label}`, | ||
| { | ||
| text: 'Undo', | ||
| onClick: async () => { | ||
| const undoResult = await emailClient.deleteEmailFilter({ | ||
| id: filterId, | ||
| }); | ||
| if (isErr(undoResult)) { | ||
| toast.failure('Failed to undo', senderEmail); | ||
| } else { | ||
| toast.success('Sender filter removed'); | ||
| } | ||
| }, | ||
| } | ||
| ); |
Contributor
There was a problem hiding this comment.
Undo needs to restore an existing override, not always delete it.
Line 431 only captures the returned filter.id. If this upsert updated an existing sender rule, the undo handler on Lines 439-446 deletes that row and loses the previous preference instead of restoring the prior is_important value.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/app/packages/queries/email/thread.ts` around lines 431 - 449, The undo
currently always deletes the created/updated rule using filterId
(result[1].filter.id) which loses any previous preference; change the logic in
the toast success undo handler (the onClick that calls
emailClient.deleteEmailFilter) to capture the prior rule state before upserting
(e.g., previous is_important) and on undo either restore the previous row via an
upsert/update call (re-apply the prior is_important and other fields) or delete
only if the rule was newly created; reference the upsert response
(result[1].filter), the variable filterId, and replace the delete-only call to
emailClient.deleteEmailFilter with conditional logic that restores the previous
override using emailClient.upsertEmailFilter or updates the existing filter to
the previous is_important value.
Comment on lines
+261
to
+287
| async fn upsert_email_filter( | ||
| &self, | ||
| link_id: Uuid, | ||
| input: UpsertEmailFilterInput, | ||
| ) -> Result<EmailFilter, Self::Err> { | ||
| if let Some(address) = &input.email_address { | ||
| email_filter::upsert_email_filter_by_address( | ||
| &self.pool, | ||
| link_id, | ||
| address, | ||
| input.is_important, | ||
| ) | ||
| .await | ||
| } else if let Some(domain) = &input.email_domain { | ||
| email_filter::upsert_email_filter_by_domain( | ||
| &self.pool, | ||
| link_id, | ||
| domain, | ||
| input.is_important, | ||
| ) | ||
| .await | ||
| } else { | ||
| unreachable!( | ||
| "UpsertEmailFilterInput must have either email_address or email_domain; validated by service layer" | ||
| ) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Reject invalid filter shapes instead of panicking or silently preferring one field.
This dispatch accepts { email_address: Some, email_domain: Some } by ignoring the domain, and { None, None } hits unreachable!. Please match the two options explicitly and return an error for both invalid shapes so the repo can't panic—or write the wrong rule—if service-side validation regresses.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rust/cloud-storage/email/src/outbound/email_pg_repo/mod.rs` around lines 261
- 287, The upsert_email_filter implementation currently prefers email_address
when both fields are set and panics with unreachable! when neither is set;
change it to explicitly validate UpsertEmailFilterInput shapes and return an
error for invalid shapes instead of panicking or silently choosing one field.
Update async fn upsert_email_filter to: handle the exact two valid cases
(Some(address) && None => call email_filter::upsert_email_filter_by_address,
None && Some(domain) => call email_filter::upsert_email_filter_by_domain), and
for the other shapes (both Some or both None) return an appropriate
Err(Self::Err) (create or reuse a descriptive error variant such as
InvalidFilterShape or BadRequest) so callers get a recoverable error rather than
an unwind or silent preference. Ensure the returned error flows through the
function signature and tests cover both invalid-shape cases.
rust/cloud-storage/macro_db_client/migrations/20260325102949_email_filters.sql
Outdated
Show resolved
Hide resolved
evanhutnik
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
js/app/packages/queries/email/thread.ts (1)
440-450:⚠️ Potential issue | 🟠 MajorUndo deletes filter instead of restoring prior state.
If this upsert updated an existing filter (e.g., changing a sender from Signal to Noise), the undo action deletes the filter entirely rather than restoring the previous
is_importantvalue. This loses the user's prior preference.Consider capturing the previous filter state before upserting. One approach:
- Query existing filters to find if one already exists for this sender
- Store the previous
is_importantvalue (if any)- On undo: if there was a prior filter, upsert the old value; otherwise delete
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@js/app/packages/queries/email/thread.ts` around lines 440 - 450, The undo handler currently always calls emailClient.deleteEmailFilter (in the onClick block) which deletes a filter instead of restoring a prior is_important state; change the flow so before calling the upsert you read and cache the existing filter (e.g., via emailClient.getFilters or the existing query that finds a filter by sender) and store its previous is_important (or a flag that no prior filter existed) alongside filterId/senderEmail; then update the onClick undo logic to: if a previous filter existed, call emailClient.upsertEmailFilter with the previous is_important value to restore it, otherwise call emailClient.deleteEmailFilter to remove the newly created filter, and keep the invalidateAllSoup() / toast.* calls as appropriate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@rust/cloud-storage/macro_db_client/migrations/20260325102949_email_filters.sql`:
- Around line 1-29: Add a reversible DOWN migration or rollback notes for the
new email_filters schema: provide SQL to drop the table and associated
indexes/constraints (DROP TABLE IF EXISTS email_filters CASCADE or explicit DROP
INDEX for email_filters_link_id_email_address_uq,
email_filters_link_id_email_domain_uq, idx_email_filters_link_id and then DROP
TABLE email_filters) or add a documented rollback section describing the manual
steps to remove the email_filters table and its constraints/indexes; reference
the created objects email_filters, email_filters_pkey,
email_filters_address_xor_domain_chk, email_filters_email_domain_format_chk and
the three index names to ensure the DOWN migration fully reverses the UP
migration.
---
Duplicate comments:
In `@js/app/packages/queries/email/thread.ts`:
- Around line 440-450: The undo handler currently always calls
emailClient.deleteEmailFilter (in the onClick block) which deletes a filter
instead of restoring a prior is_important state; change the flow so before
calling the upsert you read and cache the existing filter (e.g., via
emailClient.getFilters or the existing query that finds a filter by sender) and
store its previous is_important (or a flag that no prior filter existed)
alongside filterId/senderEmail; then update the onClick undo logic to: if a
previous filter existed, call emailClient.upsertEmailFilter with the previous
is_important value to restore it, otherwise call emailClient.deleteEmailFilter
to remove the newly created filter, and keep the invalidateAllSoup() / toast.*
calls as appropriate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0275a467-2fcf-4cdb-a765-170506e0786a
📒 Files selected for processing (3)
js/app/packages/app/component/next-soup/actions/make-sender-filter-action.tsjs/app/packages/queries/email/thread.tsrust/cloud-storage/macro_db_client/migrations/20260325102949_email_filters.sql
Comment on lines
+1
to
+29
| CREATE TABLE IF NOT EXISTS email_filters | ||
| ( | ||
| id UUID DEFAULT gen_random_uuid() NOT NULL, | ||
| link_id UUID NOT NULL REFERENCES email_links (id) ON DELETE CASCADE, | ||
| email_address VARCHAR(320), | ||
| email_domain VARCHAR(255), | ||
| is_important BOOLEAN NOT NULL, | ||
| created_at timestamptz NOT NULL DEFAULT now(), | ||
| CONSTRAINT email_filters_pkey PRIMARY KEY (id), | ||
| CONSTRAINT email_filters_address_xor_domain_chk | ||
| CHECK ( | ||
| (email_address IS NOT NULL AND trim(email_address) <> '') | ||
| <> | ||
| (email_domain IS NOT NULL AND trim(email_domain) <> '') | ||
| ), | ||
| CONSTRAINT email_filters_email_domain_format_chk | ||
| CHECK (email_domain IS NULL OR (trim(email_domain) <> '' AND position('@' IN email_domain) = 0)) | ||
| ); | ||
|
|
||
| CREATE UNIQUE INDEX IF NOT EXISTS email_filters_link_id_email_address_uq | ||
| ON email_filters (link_id, lower(email_address)) | ||
| WHERE email_address IS NOT NULL; | ||
|
|
||
| CREATE UNIQUE INDEX IF NOT EXISTS email_filters_link_id_email_domain_uq | ||
| ON email_filters (link_id, lower(email_domain)) | ||
| WHERE email_domain IS NOT NULL; | ||
|
|
||
| CREATE INDEX IF NOT EXISTS idx_email_filters_link_id | ||
| ON email_filters (link_id); No newline at end of file |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding a DOWN migration for reversibility.
The migration lacks a corresponding DOWN migration (or explicit reversibility strategy). While this may align with project conventions, documenting the rollback approach (manual DROP TABLE, schema version reset, etc.) helps operational teams handle deployment issues.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@rust/cloud-storage/macro_db_client/migrations/20260325102949_email_filters.sql`
around lines 1 - 29, Add a reversible DOWN migration or rollback notes for the
new email_filters schema: provide SQL to drop the table and associated
indexes/constraints (DROP TABLE IF EXISTS email_filters CASCADE or explicit DROP
INDEX for email_filters_link_id_email_address_uq,
email_filters_link_id_email_domain_uq, idx_email_filters_link_id and then DROP
TABLE email_filters) or add a documented rollback section describing the manual
steps to remove the email_filters table and its constraints/indexes; reference
the created objects email_filters, email_filters_pkey,
email_filters_address_xor_domain_chk, email_filters_email_domain_format_chk and
the three index names to ensure the DOWN migration fully reverses the UP
migration.
This was referenced Mar 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
email_filterstable with DB migration — stores per-sender importance overrides by email address or domain, with address rules taking precedence over domain rulesPUT /email/filters,GET /email/filters,DELETE /email/filters/{id}) for upserting, listing, and deleting sender filters, with input validation (trim, lowercase, format checks) in the service layerImportance(true/false)SQL filters now incorporateemail_filtersoverrides, with explicit exclusion of the opposite override to prevent threads appearing in both Signal and NoiseENABLE_CLIENT_EMAIL_SIGNAL_FILTER(defaultfalse) to disable redundant client-side signal/noise email filtering, deferring to backend filtering