Skip to content

⚙️ FEATURE-#40: Use Optional[bool] for Query flags to remove ambiguity #60

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
FernandoCelmer merged 2 commits into from
Apr 16, 2026
Merged

⚙️ FEATURE-#40: Use Optional[bool] for Query flags to remove ambiguity #60

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e8a5e97
⚙️ FEATURE-#40: Use Optional[bool] for Query flags to remove ambiguity
FernandoCelmer Apr 16, 2026
669bbe0
✨ FEATURE-#40: Only filter on True, reject contradictory seen/unseen
FernandoCelmer Apr 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions email_profile/clients/imap/query.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from datetime import date
from typing import Optional, Union

from pydantic import BaseModel, ConfigDict, Field
from pydantic import BaseModel, ConfigDict, Field, model_validator


def _ascii(value: str) -> str:
Expand Down Expand Up @@ -194,7 +194,7 @@ class Query(BaseModel):
deleted: Optional[bool] = None
draft: Optional[bool] = None

unseen: bool = False

@FernandoCelmer FernandoCelmer Apr 16, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Blocking]

Problem — This is a silent breaking change for existing consumers. Previously, Query(unseen=False) was the default and meant "no unseen filter" — the _flag_clauses method only checked if self.unseen: which was falsy for False. After this PR, Query(unseen=False) now emits (SEEN), actively filtering for seen-only messages.

Failure scenario —

# Existing user code (pre-PR behavior):
q = Query(subject="report", unseen=False)
# Before: searches ALL emails with subject 'report'
# After:  searches ONLY SEEN emails with subject 'report'
#         -> silently drops unread emails from results!

Since the library is at version 1.0.0.dev1, there may not be many external callers yet, but this still changes documented behavior without a deprecation path.

Fix — Document this as a breaking change in the PR description and changelog. Consider if the default change from False to None is sufficient, or if unseen=False should remain a no-op for backward compatibility.

unseen: Optional[bool] = None

def _date_clauses(self) -> list[str]:
fields = (
Expand Down Expand Up @@ -229,6 +229,12 @@ def _size_clauses(self) -> list[str]:
f"({name} {value})" for name, value in fields if value is not None
]

@model_validator(mode="after")
def _check_seen_unseen(self) -> Query:
if self.seen is True and self.unseen is True:
raise ValueError("Cannot set both seen=True and unseen=True.")
return self

def _flag_clauses(self) -> list[str]:
fields = (
(self.seen, "SEEN"),
Expand All @@ -241,9 +247,7 @@ def _flag_clauses(self) -> list[str]:
for flag, name in fields:
if flag is True:
parts.append(f"({name})")

@FernandoCelmer FernandoCelmer Apr 16, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion]

Problem — Query(seen=True, unseen=True) now produces both (SEEN) and (UNSEEN) in the IMAP search command — a contradictory query that will return zero results on most servers.

The unseen field is the logical inverse of the seen field, but both can be set independently. There's also redundancy: Query(unseen=False) and Query(seen=True) now produce identical IMAP queries.

Fix — Add a Pydantic model_validator to reject contradictory combinations:

@model_validator(mode='after')
def _check_seen_unseen(self) -> 'Query':
    if self.seen is not None and self.unseen is not None:
        raise ValueError(
            'Cannot set both seen and unseen. Use seen=True or unseen=True, not both.'
        )
    return self

elif flag is False:
parts.append(f"(UN{name})")
if self.unseen:
if self.unseen is True:
parts.append("(UNSEEN)")
return parts

Expand Down
11 changes: 9 additions & 2 deletions tests/clients/imap/test_query.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,15 @@ def test_date_format(self):

def test_flags(self):
self.assertEqual(Query(seen=True).mount(), "(SEEN)")
self.assertEqual(Query(seen=False).mount(), "(UNSEEN)")
self.assertEqual(Query(answered=False).mount(), "(UNANSWERED)")
self.assertEqual(Query(seen=False).mount(), "ALL")
self.assertEqual(Query(answered=True).mount(), "(ANSWERED)")
self.assertEqual(Query(answered=False).mount(), "ALL")
self.assertEqual(Query(unseen=True).mount(), "(UNSEEN)")
self.assertEqual(Query(unseen=False).mount(), "ALL")

def test_contradictory_seen_unseen_rejected(self):
with self.assertRaises(ValidationError):
Query(seen=True, unseen=True)

def test_size_filters(self):
out = Query(larger=1024, smaller=4096).mount()
Expand Down
Loading