mirror of
https://github.com/karatektus/pl_mail.git
synced 2026-10-25 01:54:10 +00:00
main
57
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
13303b247e |
v0.3.9 milestone (#283)
* Run the end-to-end suite on pull requests into milestone and wip branches
A release's work is now collected on a milestone branch (milestone/v0.4.0)
before it reaches main, and the suite only ran on pull requests into main,
so nothing proposed for the milestone was tested. `milestone/**` joins
`main`, and `wip/**` comes along from wip/28, where it already runs the
suite on the phases of #28 — the same block on both, so they merge
without a conflict.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit
|
||
|
Paul LütznerandClaude Opus 5.5
|
9c76bbc636 |
The blob prune follows APP_STORAGE_DIR and clears orphaned thumbnails (#234)
app:prune:blobs looked under a hardcoded var/, so installs with another APP_STORAGE_DIR reclaimed nothing; it and the thumbnail cache now use APP_STORAGE_DIR. A third pass deletes attachment thumbnails whose part no longer exists. Closes #232, closes #233. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
1b04e697f6 |
All Mail: every conversation outside Spam and the bin, as an opt-in system label (#57)
Archived mail had no list to be browsed in, only search. All Mail lists every conversation outside Spam and the bin (what arrived, what was sent, drafts and archived mail), as Gmail's All Mail does, across the reader's active accounts or within one. It is LabelRole::AllMail, a view and not a place, like Unread: nothing attaches it to mail, it is created hidden when label settings is opened, it is bound to no account and it refuses moves. Its eye in Settings → Mail → Labels shows or hides both the row beside Drafts and the row at the top of each account's folders. Archiving or snoozing from it leaves the row in place. One list scope, `all_mail`, with the account id as its value or empty for every account; AllMailView holds the rules the row buttons, the bulk toolbar, Move to and select-all ask. No migration. Co-authored-by: shady2k <3366415+shady2k@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
210fae090e |
Show the category tabs on an account's own inbox, behind a setting that is off by default (#55)
Clicking an account in the sidebar opened a plain list, so on an Outlook account (or any account) the category tabs were only on the top-level Inbox. An account's page can now draw the same tabs, the reader's own included, chosen by the same rule as the unified inbox and counted from that account's mail alone. The strip is plainer than the unified one: no "new" pills, sender hints, unread tint or drop targets, since those count every account. It scrolls the open tab into view on a phone. Settings → Mail → Categories has a new "Tabs on each account" switch, off by default. Off, an account's page stays its whole inbox in one list, as before, and a stale `?tab=` link opens that too. The switch is offered only while "Inbox tabs" is on. Stored in the user's settings bag and carried by a config backup. No migration. Co-authored-by: Walid Mani <15911342+Flouss@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Paul LütznerandClaude Opus 5.5
|
3c0769c1e6 |
Order every inbox tab, and let a built-in one be switched off
One order across the built-in tabs and the reader's own, and a set of built-ins switched off, both beside the other sorting choices on the user (CategorySorting). The order replaces the categories' own position column, which never shipped: the strip is one list, so its order is too. Primary is always first and cannot be switched off — it is where the rest falls back to. A switched-off category takes no mail. Whatever would have gone there — by the header rules, Google's label or the assistant — is in Primary, and the message panel says it would have been that category. The answer is mapped rather than the next rule consulted: "Forums is off" does not make a mailing list Promotions for its List-Unsubscribe. Until the re-sort a switch queues has run, and for a conversation somebody pinned there, Primary lists and counts the switched-off category's conversations too, so nothing is in no tab at all. InboxTabs is where order and on/off are applied, so the strip, the drop targets and every key that resolves agree. A restore rewrites the order's own-tab keys to the categories' new ids. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Paul LütznerandClaude Opus 5.5
|
52ad273359 |
Let a filter file Gmail mail, and refuse every built-in tab name
A filter only filed Gmail-labelled mail while the reader had asked plMail's sorting to overrule Google's. Google's category and plMail's rules are both guesses, and the override decides between them; a filter is an instruction the reader wrote, and outranks both. Out of the filer, the threader and the two recompute statements alike. A new tab could be called "Werbung" beside the built-in Werbung: the reserved names were the enum's English values only. Every interface language's label is refused now. The per-account recompute sorted every message on the installation for each account it was asked about. It reads only that account's now — the same rows, since a conversation belongs to one account — which matters more once switching a tab on or off re-sorts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Paul Lützner
|
7898182e00 |
Merge branch 'pr-87' into review-87
# Conflicts: # docs/de/features/mail.md # docs/de/install/config-backup.md # docs/de/internals/mail-ingest.md |
||
|
|
938d549347 |
fix(insight): re-reading old mail no longer rewrites newer cards or fills Happening soon (#101)
* fix(insight): let an older mail leave a card as a newer one made it The harvester's upsert lets the last mail it reads overwrite a card's status, payload, ETA and link, on the assumption that the last one read is the newest. A sync reads in date order, but the insights backfill walks by id, and the Graph import stored mail newest-first: a parcel's "out for delivery" mail was read before its "announced" one, and the older statement overwrote it, so a re-read left every card on the first status of its series. A mail strictly older than the one the card already stands on now changes nothing. Mail of the same instant keeps the old behaviour (the last read wins), and so does a card whose mail was deleted, which has nothing to be older than. land() reports whether it wrote, so the backfill's count does not include skipped drafts. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * fix(insight): window undated cards on their mail's date, not their row's The undated cards (GitHub activity and the like) are listed for the last seven days, windowed and ordered on createdAt, the instant the harvester made the row. That is the mail's age only while the harvester runs as mail arrives. The insights backfill re-reads old mail and creates its cards now, so a card made from a month-old notification was a card of the last minute and took a place among the newest twenty for a week; the Graph import's newest-first ids put the same distortion into the order. The query now joins the card's mail and windows and orders on its receivedAt, which is the newest mail of the series because the card links that one. A card whose mail was deleted keeps its row's date, the only one it has. createdAt keeps meaning when the row was created, so nothing is stamped differently and no migration is needed. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * fix(insight): offer a code that states no expiry for fifteen minutes OtpExtractor sets a code card's happensAt to its expiry, so the radar drops it when that passes, and leaves it null when the mail states no lifetime, because a guessed default would retire a code that might still be good. The radar's list of undated cards then took such a code like any undated card (GitHub activity and the like): seven days, with the code readable on the card. Undated codes are now offered for fifteen minutes of their mail's date instead of the week. That is a guess about how long a code lives, made on the display side and not in the extractor, and it errs towards showing too little. The extractor's decision not to invent an expiry stands, the code is still on the mail itself, and codes that do state an expiry take the dated path as before. Every other undated kind keeps its week. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> --------- Co-authored-by: mani <15911342+Flouss@users.noreply.github.com> Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com> |
||
|
maniandClaude Sonnet 5.5
|
92adf5133b |
fix(mail): select nothing for a vanished own tab, and keep the Gmail rule true after filing
Select-all on a tab the reader made, after that tab was deleted elsewhere, fell back to Primary: the whole-view job resolves its view when it RUNS, so "archive everything" would have reached up to ten thousand conversations that were never on screen. A key shaped like an own tab that no longer resolves now selects nothing, as a label that is not the reader's does; an unknown built-in name keeps meaning Primary. A filter's verdict on Gmail-labelled mail counted only at filing time, so switching "sort over Google" off later, which queues a re-sort, left Google's mail in a tab of the reader's own. Both recompute statements and the threader now apply the same test. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> |
||
|
maniandClaude Sonnet 5.5
|
1298d1edce |
refactor(mail): make a category's owner and name non-null, and satisfy PHPStan
InboxCategory::$usr and $name are NOT NULL columns, so they are declared that way (Message::$account is the precedent) instead of nullable with a check nobody can reach. FilterDescriber keys its category names by the user identifier rather than reading an id off a UserInterface, which adds nothing to the baseline. Test fixtures stop storing threads they never read back. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> |
||
|
maniandClaude Sonnet 5.5
|
b2b9a90840 |
feat(mail): show the reader's own categories as inbox tabs
A category the reader made is a tab beside the five, shown while it holds mail (or is being looked at) and otherwise only as a drop target. A conversation is in exactly one tab: its own when it has been filed in one, else the built-in one, so a carrier's notice is listed in Shipping and no longer in Promotions. The grouped counts, the new-mail dots, the sender hint, the select-all view and the counts endpoint all key a tab the same way. InboxTab is the one value for "a tab", whose key for a built-in is exactly what it always was. InboxTabs resolves a key against the asking reader's own categories, answers a key that cannot be custom without a query, and reads the categories once per request. Another reader's category, or a deleted one, is no tab and opens the inbox like any unknown key. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> |
||
|
maniandClaude Sonnet 5.5
|
4c1b3dc238 |
feat(mail): resolve a conversation's own-category tab like the built-in one
A thread is shown in one of the reader's own tabs when its newest message was filed there: MessageThreader adopts the verdict alongside the built-in category (and moves the thread back out when a newer message is filed nowhere), and both recompute statements carry it in the same pass. A pinned thread keeps its tab, as it keeps its built-in category. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> |
||
|
Paul LütznerandClaude Opus 5.5
|
8579c3c425 |
Ask one place whether a label name is taken, and ask it on a rename too
|
||
|
Paul LütznerandClaude Opus 5.5
|
d3292e3a6d |
Refuse a label name that differs from an existing one only in capitals
"work" could be created beside "Work". The sibling check was an exact match, and the strictest provider's rule is not: Gmail holds the two to be one label and answers the second with "409: Label name exists or conflicts" — in ApplyGmailLabelsHandler, in a worker, after the request that made the label had answered "created". So the label existed here, could be put on mail here, and never reached Gmail, with an error in the log and nothing on any screen. It was found from the Android app, which had been told the create worked. The check in front of a person typing a name now ignores case, in Mailbox/set (create and rename) and in the browser's new-label form. It is a second repository method rather than a change to findOneChildByName(), and the two must stay apart: that one is find-or-create for mail arriving from a provider, where an exact match is the only safe one — a case-sensitive IMAP server may really hold `work` and `Work` as two folders, and folding them would file two folders' mail under one label. Mailbox/set's description is written for a person, because a client shows it to one. When capitals are the whole difference it names the existing label as it is spelled — somebody who typed "work" and is told "work already exists" goes looking for a label that is not there — and it carries `properties: ["name"]`. Roles are included: a custom "inbox" beside the Inbox is the same collision with a name the provider reserves. A label may still recapitalise itself. Nothing is done about pairs that already exist, and the handler still logs the 409 for them: adopting Gmail's label on a conflict would bind two labels here to one there. The browser's rename form has no duplicate check at all, before or after this. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Paul LütznerandClaude Opus 5.5
|
4e975d4db4 |
perf(jmap): Mailbox/get counts only what it will print
Every call ran two full-account aggregates over labelled mail, including the ones asking for names and roles only. A grain is now read only when one of its properties is requested, and only for the named mailboxes when ids are given. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Paul LütznerandClaude Opus 5.5
|
8289891830 |
feat: failed requests raise a toast instead of breaking the page
A failed Turbo response used to be rendered: the error page drawn as the page (or merged under it — a picked PDF avatar left plMail's 500 hanging off the settings screen), or "Content missing" in a frame. - request_errors.js catches every Turbo visit, form and frame. A failed response is not rendered; the page stays as it was and an error toast says what happened. 422 (a form with its errors), 401/403 (the session guard) and elements marked data-turbo-errors="self" (the compose dock) are left alone. A network failure gets its own words; a frame response of the wrong shape is visited as a page. - requestFailed() gives controllers' own fetch() calls the same toast. An audit of the 60 calls fixed the 22 files whose user-initiated requests failed silently (star, archive, labels, drag and drop, bulk actions, reorders, appearance save, undo/cancel send, and more), and left background polling and preferences quiet on purpose. - Every request gets an 8-hex id: X-Request-Id on the response, `id` on its log entries, "Ref. …" in the toast with a copy button, and a Reference filter in Admin → Logs. A proxy's own id is kept. - toast.js clones the server's toast partial, so a script's toast is the same pane and theme as a streamed one. The sync button uses it, and its hardcoded English is now translated. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Paul LütznerandClaude Opus 5
|
a1af660b45 |
fix(search): let the words win before the vector suggests
Searching one company name returned 63 results where Gmail, over substantially the same mail, returned 16. Sixteen were matches on words. The other forty-seven had merely cleared the vector's similarity threshold — job alerts, a product newsletter, an invoice, outright spam — every one correctly badged "meaning", every one useless, and together enough to fill the first page exactly. The best match in the mailbox, a reply whose subject AND sender both carried the term, was on page two. **The threshold was not mistuned.** It was measured against the twelve-message corpus in EmbeddingPreset, where a false-positive rate of a couple of percent is invisible, and then applied to the two thousand most recent messages, where the same rate is forty-seven rows. No stricter fraction fixes that — the fix for "a small fraction of two thousand is still a lot" is a cap. So `sem_hits` takes the ten nearest and stops. The cap is INSIDE that CTE, which is what makes it safe: the note above it argued a top-k would make `COUNT(*) OVER ()` mean "lexical plus up to k, on page one only", and that is true of a top-k over the UNION, which sees OFFSET. A CTE is evaluated once per statement and the candidate window is deterministic, so the admitted set is identical on every page and the total stays honest. The earlier reasoning was right about the wrong position. Ordering carried the other half, and it is why the best match was displaced rather than merely outnumbered. A semantic hit can only be drawn from the most recent SEMANTIC_CANDIDATES messages, so under "most recent" every one of them is structurally newer than most of the mail it competes with — they do not tend to win, they cannot lose. `semantic_only ASC` now leads both sort orders. That fixes relevance too, where the disease was worse: `rank` is `GREATEST(ts_rank, cosine)`, and measured against this schema's weights a subject match scores 0.6079, a body match 0.1216, a token-part match 0.0608, while any admitted vector hit is ≥ 0.42 by definition. Sorting by relevance put every barely-admissible vector hit above every body match. Leading with provenance means rank is only ever compared within one scale. Nothing is removed. The meaning matches are all still there, badged, at the end. The body-substring rescue is counted over lexical rows now rather than over the page. It exists to catch a needle inside a token the tokenizer split the wrong way, and forty-seven vector hits padding a page to exactly fifty made `50 < 50` false — so the one arm that could still have found the mail never ran, and nothing said so. Both new tests were checked against the old behaviour and fail there: without the cap the fixture admits two rows instead of one, and without the ordering the newer meaning match takes first place. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Paul LütznerandClaude Opus 5
|
7740620020 |
Open the MIME container a blank message's body is actually stored in
The repair shipped in v0.2.29 looked for text/plain and text/html parts. On the
installation that reported the bug there are none: the row is
multipart/related 09704cea 8
— the whole block AROUND the body, not the body. So the repair ran, matched
nothing, and recorded itself as done, which is worse than failing because a
completed task stops asking. The ingest fix missed the same mail for the same
reason, so new messages kept arriving blank.
A `multipart/related` HOLDING EXACTLY ONE PART is not descended into when it
sits inside another multipart. Measured against the library rather than reasoned
about:
alt > [plain, rel[html]] html=EMPTY attachment: multipart/related
alt > [plain, rel[html,img]] html=ok attachment: image/png
alt > [rel[html]] html=EMPTY attachment: multipart/related
alt > [rel[html,img]] html=ok attachment: image/png
The second column is why this hid: one inline image beside the HTML and the
same message parses correctly, and almost every HTML mail has one. Mail that
does not — a rejection from a recruiting system — lands in the broken row every
time.
OPENED, NOT DECODED. The content is a MIME block, so it is rebuilt into a
message and handed back to the parser. That also settles the charset for free:
inside a message of its own the HTML is a BODY part again, and webklex converts
those — the step it skips for an attachment. The boundary is not in the database
(content_type keeps no parameters) but a multipart body opens with its own
delimiter, so the first line carries it. That is what makes an already-stored
part repairable without a mail server.
A container that also carries files is LEFT CLOSED. This understands one shape;
taking the body out of an unfamiliar one while dropping its attachments would
turn a blank message into a blank message that has lost things.
The task is renamed because the ledger keys on its name and every affected
install already has a row saying the old one is done.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Paul LütznerandClaude Opus 5
|
5a620f4be7 |
Run one-time repairs once per installation, and repair the lost bodies
The previous commit stops bodies being filed as attachments. It does nothing for the mail already in the mailbox, which stays blank — and there was nowhere for that kind of work to live. WHY NOT THE FOUR PLACES IT COULD HAVE GONE. A migration runs exactly once on update and is the right home for schema, but this repair needs the attachment store and the body sanitizer, and a migration reaching into the container breaks the day one of them moves — halfway through an update, with the schema half applied. The entrypoint is downtime: it runs before the server accepts a request, so a walk over every message put there means an install that updates at eight in the evening is unreachable until it finishes. BackfillTaskInterface is deliberately manual, which is right for "regenerate the categories because the classifier changed" and wrong for damage nobody should read a changelog to discover. BackgroundJob is per-user, and this is not owned by a user. So: a ledger, in the same shape as Doctrine's own migration_versions and readable the same way when the application will not start. A task claims its row BEFORE running, so a container killed mid-walk leaves evidence rather than looking like one that never started; three attempts and it stops rather than taking the worker down every ten minutes forever. The runner re-reads the row afterwards because tasks clear the EntityManager between batches and the claimed instance is detached by then — writing through it would flush nothing and the task would run again for ever. Scheduled every ten minutes, and what runs every ten minutes is the QUESTION, not the work: one indexed lookup per registered task against a table with single-digit rows. What it buys is the gap between an update landing and the repair happening. The repair is the first task and implements both interfaces — automatic for everybody, and app:backfill misfiled-bodies for anyone who wants it now. It needs no mail server: persistAttachment() stored the bytes on the way past, so the body has been on disk the whole time it was missing from the screen. It does lose the declared charset, which MessagePart does not keep, so CharsetHelper works it out from the bytes — exact for UTF-8 and a reasoned guess otherwise, against a round trip per message to a server with nothing else to say. EXISTS rather than a join in the lookup, and not as a preference: a multipart/alternative can have both halves misfiled, and SELECT DISTINCT cannot run at all here — Message carries json columns and Postgres has no equality operator for json. The repository test caught that on its first run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Paul LütznerandClaude Opus 5
|
ffe9457b42 |
Give the search model its query instruction and its own threshold
Search by meaning was answering "mails about food" with a page of login links
and newsletters, ranked confidently, with nothing anywhere saying it had gone
wrong. Two settings decide whether it works and plMail had neither.
THE QUERY INSTRUCTION. Embedding models are asymmetric: they are trained to be
told which side of a search they are looking at. An unlabelled query is treated
as one more document, and it lands nearest whatever is most boilerplate — in a
mailbox, login links and newsletters. Measured on qwen3-embedding:0.6b over
twelve German and English messages, five about food:
query non-food ranked above the best food mail
"mails about food" 5 (raw) 0 (instructed)
"essen" 7 (raw) 0 (instructed)
"restaurant reservation" 2 (raw) 0 (instructed)
"essen" — the least ambiguous word available — put every food mail below every
non-food mail. With the model's own instruction, perfect separation on all
three. It goes on the QUERY ONLY, so stored vectors are untouched and nothing
has to be re-indexed.
THE THRESHOLD. 0.55 was one constant for every installation, and cosine
similarity is not comparable between models — or even between the instructed
and uninstructed forms of one model. Best threshold by F1, same corpus:
qwen3-embedding:0.6b no instruction 0.20 F1 0.59 precision 0.42
qwen3-embedding:0.6b with instruction 0.42 F1 0.85 precision 1.00
nomic-embed-text no instruction 0.45 F1 0.74 precision 0.65
all-minilm no instruction 0.20 F1 0.71 precision 0.77
0.55 is above all of them. So it travels on SemanticSearch next to the model and
width it has to agree with, and it is a setting rather than a release.
Both are columns, and EmbeddingPreset holds the measured pair for each model the
release was tested against, with a line on what it is good and bad at. The model
field stays free text — a closed dropdown would go stale and refuse something
that works — and choosing a preset fills all three fields at once, because they
are one decision written in three boxes: any two from different models give a
search that is confidently wrong rather than visibly broken.
qwen3-embedding:0.6b becomes the documented default. Multilingual matters more
than any benchmark for a mailbox that is not all in one language, and it was the
only combination measured at precision 1.00.
Existing installations keep 0.55 and no instruction. Backfilling 0.42 would
apply Qwen's number to whatever model is configured, and on all-minilm (0.20)
that is twice as tight as it should be — a migration is the wrong place to
change what somebody's search returns. The panel now shows both values and sets
them on one click.
The instruction field is trim: false, which a test caught rather than a guess:
every working instruction ends in a separator, Qwen's is "…\nQuery: ", and
Symfony's default trim turned it into "Query:mails about food".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Paul LütznerandClaude Opus 5
|
b5718aa21c |
Filter the semantic candidate window on its own table alias
The vector arm scores the most recent SEMANTIC_CANDIDATES messages, and the
subquery that picks them scans `message` a second time as `m2`. It was handed
$mine, which is written for the outer alias: `m.account_id IN (...)`. Legal SQL,
entirely different query.
Postgres showed both halves of the damage in one plan. The account test became
`One-Time Filter: (m.account_id = ANY (...))` — a per-outer-row constant — over
a `Seq Scan on message m2` carrying no filter whatsoever, and the correlation
put the whole Limit/Sort on the inner side of a nested loop. The entire message
table, re-sorted for every candidate row being tested.
Measured on 40,000 messages of 1024 dimensions across two accounts:
shipped (m) cancelled at the 5,000 ms budget, every time
fixed (m2) Hash Semi Join, evaluated once, 2,000 scored, 188 ms
The cancellation is what reaches the searcher, and since the previous commit it
reaches them by its right name: "Searching by meaning took too long here". It
was not slow hardware and it was not the model. It was this.
The quieter half outlives any plan. LIMIT 2000 was taking the most recent rows
in the TABLE and only then filtering to this user's accounts, so on the same
fixture account 1 got 1,000 candidates out of the 2,000 the constant promises —
roughly 2000/N for N busy accounts. Nobody ever saw another mailbox; the outer
WHERE held. The entire cost landed on the searcher as absence, with the page
saying nothing, because as far as it knew the pass had run.
SEMANTIC_CANDIDATES becomes injectable for the same reason the two timeouts
already are: at 2,000 this needs a fixture no test can afford, and at 2 it is
three rows. The test fails against the old alias with 0 results.
Also corrects two comments that claimed the recency ordering is served by an
index. There is no index on (account_id, received_at), and building one moves
the arm from 188 ms to 186 ms — the 2,000 distance calls are the whole cost.
Plausible, unmeasured, and load-bearing for nothing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Paul LütznerandClaude Opus 5
|
6a4118ddbb |
Stop reporting a failed semantic search as a slow model
The search page has two ways to print "The model took too long, so this search used words only", and only one of them is about the model. SemanticQuery answers TimedOut when Ollama does not return a query vector inside EMBED_TIMEOUT. That one is honest. MessageThreadRepository::cheapRows() answered the same TimedOut when the vector arm blew its 5s statement budget — by which point the model had already answered, in milliseconds, and is sitting in ai_call_metric marked a success. An operator reading that sentence goes to the model host, where there is nothing to find. SemanticQuery's own docblock guards the mirror image of this: the embedding is done in the controller so that a slow model is never reported as a database fault. This was the same confusion pointing the other way, and it is the worse direction, because it names a machine that is working. Worse, the catch was on DriverException, which is every driver fault Postgres can raise. DBAL's PostgreSQL ExceptionConverter maps deadlocks, constraint violations and syntax errors onto dedicated classes and lets 57014 fall through to a plain DriverException, so "the budget fired" and "the query is broken" arrived as the same type. A missing plmail_embed_distance(), a stored width that disagrees with its vector, a connection dropped mid-statement: all deterministic, all degrading every search instantly and for as long as the cause stands, all reported as an occasional overrun — and none of them logged anywhere at all, because the exception was discarded unnamed. So: SearchTooSlow for a real cancellation, SearchFailed for everything else, each with its own sentence in all three catalogues, and both logged with their SQLSTATE. The repository gets its first logger for it. A rare give-up path that leaves no trace is indistinguishable from a common one, which is how this stayed invisible. The new test drops plmail_embed_distance inside the test transaction rather than making the query slow, so it cannot pass by accident on a busy machine: 42883 is not 57014 under any load. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Paul LütznerandClaude Opus 5
|
d8f8acf29b |
feat(ai): tunable keep-alive per model, and a warm-up button
The first "Help me write" after a quiet spell shows nothing for tens of seconds while the Ollama host reads twenty gigabytes back off disk. How long it waits before evicting a model is OLLAMA_KEEP_ALIVE — an environment variable on a machine plMail can talk to but cannot configure. The API takes the same setting per request and a per-request value wins, so this is the only lever available from inside the app. Two nullable columns on ai_settings, one per model, because the two are opposite trades: the writing model is eighteen gigabytes and asked for occasionally, the embedding model a few hundred megabytes and asked for on every search and every indexed message. Both ship at 5m, which is Ollama's own default, so an upgrade changes nothing about how a host behaves — the boxes just say out loud what was already happening. -1 is recommended for the search model in the help text and the handbook rather than shipped, because plMail cannot see how much memory that host has. Empty means send no keep_alive at all and let the host decide, which is a real selectable state and not an unfinished one. The wire representation is the part that is easy to get wrong: Ollama parses a JSON string as a Go duration, and a Go duration needs a unit, so "-1" and "3600" as strings are a 400 on every model call from a setting that reads as correct. KeepAlive::forBody() is the only thing that puts the value in a body — int for the numeric spellings, string for the durations, null to omit the field entirely, since an explicit null is read as zero and unloads immediately. One helper adds it to all four bodies so no call site can get that wrong. Validated with the same expression the body is built from, refused rather than coerced, and the message lives in the validators domain where Symfony actually looks for it. Warm up now: POST /admin/ai/warmup issues the no-prompt /api/generate, times the round trip and reports it. PHP's 30s execution limit is raised for it — a cold load is not 30 seconds — and the upstream timeout sits below that so it gives up first. A host that is unreachable and a model that is not on the host are distinct, translated answers. Also carries both columns through the configuration backup, and adds the AI feature to the README, which had never mentioned it. Fixes a search test that asserted how fast the machine was: it gave the last-resort body pass a one-millisecond budget and one row to scan, so on quick hardware the pass finished in time and the test failed. It now provokes a genuine statement_timeout cancellation with pg_sleep, which exercises the same guard without a clock in the assertion. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Paul LütznerandClaude Opus 5
|
2ca7824783 |
test(ai): two tests that only failed on a machine that had run the app
Both passed in CI against a fresh database and failed locally, which is the shape of a test that reads state it did not write. AiBackfillStateRepositoryTest works in a transaction, and the transaction unwinds what the tests write — not what was already there. The table is a singleton, so on any machine that has run the app its one row is present before the first test starts, carrying a real interactive_seen_at from whenever somebody last opened the composer. touchInteractive() refuses to move that stamp backwards, which is the behaviour it exists to guarantee, so a row stamped later than the fixed date the test picks made the test fail against its own input. The row is now cleared in setUp, inside the transaction, so each test establishes the state it asserts on; the rollback puts the original back. IndexNewMailCommandTest::testItSweepsEveryMailboxWhenNoneIsNamed asserted the queue was exactly its own fixtures after an UNSCOPED sweep. The first test in the same file carries a docblock warning about precisely this — "it would be an assertion about what else happens to be in the database, which is the shape of a test that fails on a second afternoon" — and scopes itself with --email to avoid it. The unscoped one relied instead on the other seeded mailboxes being empty, and they stopped being empty. It now asserts the claim that is actually worth holding: the mailbox nobody named still got swept, newest mail first, and the ceiling still stopped it short of draining the mailbox. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Paul LütznerandClaude Opus 5
|
dec2dd56cd |
fix(search): a vector that ran out of time says so
The statement budget added with the performance work drops the vector and finishes on keywords alone, which is the right thing to do. What it then told the reader was wrong. The controller built its report from the search that was ASKED FOR rather than the one that answered, so a search whose vector expired reached the page as "searching by meaning found nothing the words had not already" — a sentence about a comparison that happened, describing one that never did. Telling somebody their mail holds nothing similar on the strength of a comparison nobody made is exactly the silent degradation the skip reasons were built to stop, and the timeout written to protect the page had quietly reintroduced it. So the expiry carries its reason instead of a bare null, SearchPage carries the search that actually ran, and the controller reports that one. The catch-up indexing reads it too — there is no point treating a vector that never finished as evidence the model is warm. The key already existed: search.semantic.skipped.timed_out has been in the catalogue since the skip reasons shipped, waiting for something to raise it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Paul LütznerandClaude Opus 5
|
037219e307 |
perf(search): compute the vector distance once, on candidates only
Semantic search measured 12.9s mean and 47s max in production, and it was getting worse as indexing progressed — 61.9% indexed then, projecting to ~19.6s at 100%. The arithmetic was exact: plmail_embed_distance costs 0.132ms a call at 1024 dimensions, and one search made about 92,000 of them. Two faults multiplied. The semantic ARM was correctly capped at SEMANTIC_CANDIDATES. The outer `LEFT JOIN message_embedding sem` was not capped at all, so the rank expression scored every embedded message the query matched — including rows only a keyword had found. And it scored them TWICE. The comment above $semanticRank said Postgres matches identical aggregate calls and evaluates the argument once per row, which is true, and the code carefully built one string to get it. DBAL expands the same named :queryVector into a separate positional parameter at each occurrence, so what reached the server were two textually different expressions. Production's own pg_stat_statements showed it: $7(...$3 AS real[]) beside $10(...$4 AS real[]). Both go away by construction rather than by care. The distance moves into a materialised CTE over the candidates, the arm reads from it, the outer query joins it, and plmail_embed_distance and :queryVector each now appear exactly once in the whole statement. Measured on 20,000 rows: 12,616 calls and 3,781ms become 2,000 calls and 602ms. MATERIALIZED is load-bearing, not decoration — without it the planner inlines the CTE at both references and 3.6x of that goes back. Two CTEs rather than one for a neighbouring reason: with the threshold applied inside a derived table the subquery is flattened, similarity lands in both the filter and the target list, and every row that PASSES is scored twice. The function's cost is corrected too, and derived rather than guessed: calibrating two index-only scans against their planner costs gives ~1.5us per cost unit, so 132us is about 33,000 of them. Rounded up to 50,000, because underestimating is the failure being fixed. Only the plpgsql body — the pgvector body inlines before costing. The cheap pass now runs under a statement_timeout and drops the vector for the rest of the search when it expires, so a pathological plan is paid once rather than up to four times. ONE BEHAVIOUR CHANGE, deliberate. Ranking moves for 3,966 of 5,308 rows in the benchmark, every one a keyword row: the old unbounded join had no threshold and no recency bound, so GREATEST(ts_rank, 1 - distance) took the vector's number on rows the vector had never found. 80.5% of keyword rows were ranked by a below-threshold similarity averaging 0.308 against a true ts_rank of 0.067. All 308 semantic-only rows keep their rank exactly, and EXCEPT ALL in both directions on (thread_id, semantic_only) is empty. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Paul LütznerandClaude Opus 5
|
377d6026b8 |
feat(ai): make the language model observable, and the composer honest
"I click the AI button in the composer and nothing happens" turned out not to be about the model at all. #editor() queried data-compose--toolbar-target while the composer renders data-compose--compose-toolbar-target, so run() returned at its first guard: no request, no status, no error. The feature had never worked once. #subject() was wrong the same way and quieter — every prompt went out with an empty subject. And the status element lived INSIDE the dropdown that the same click closes, so even once working it would have written into a hidden subtree. The rest of this is what makes the next such thing findable. RECORDING One row per model call, tagged by the workload that caused it, and nothing about what was in it — no prompt, no completion, no message id, no subject, no address. error_kind is a category from a closed set for the same reason: an HTTP client's exception message routinely quotes the request body back, and here that is somebody's mail. OllamaClient was discarding Ollama's whole timing block; it now returns it, and AiAssistant — the only place that knows both the feature and the model — writes the row. DBAL and deliberately no entity, following EmbeddingStore: BackfillEmbeddings clears the EntityManager after every chunk, so an unflushed ORM row would be detached and silently dropped, fifty at a time, for as long as a backfill runs — which is exactly the window the panel exists to show. AiFeature could not carry this. A four-character search query somebody is waiting on and two thousand characters of mail body written unattended a hundred thousand times both went through embed() with no discriminator, and averaged together the second buries the first — the panel would report backfill throughput under the heading "search". AiCallFeature splits them. THE PANEL Live state from /api/ps, which is passive and safe to poll. Six states that look different, and the one that justifies the work is "partly on the CPU": size_vram < size means layers spilled, throughput collapses, and the model still answers correctly — invisible everywhere else, so everything is simply slow forever. THE COMPOSER Tokens stream into a provisional surface and commit in one operation, because hundreds of per-token insertions destroy the undo stack and Ctrl+Z is the first thing anyone tries. Waiting, generating, done and failed are distinct states with reasons: outside the keep-alive window the first thirteen seconds produce nothing while 20 GiB loads, and a featureless spinner for that IS the bug. Cancellation propagates to the host — an abandoned draft otherwise holds the GPU and everything else queues behind it. SEARCH Silent degradation to keyword-only is replaced by the reason it degraded, and rows found by meaning say so. The semantic arm and its join are now matched on model AND width: vectors from different models are not comparable, and after somebody changes the search model every existing vector is dead weight. On a pgvector install comparing different widths raises an error — a 500 on /mail/search caused by an admin dropdown — and on the shipped loop it silently compares the overlap, which is worse. Coverage is stated where results are shown, because semantic search over an 8%-indexed mailbox is a trap. BACKFILL Progress is counted from the database, never accumulated, so a killed worker resumes. It stands aside while anyone is using the AI and comes back on its own: both compete for one integrated GPU, and a batch in flight when somebody presses the button puts them straight back to "nothing happens". Also carries the cron line for app:jobs:reap from the commit before this one. Verified against the real host: reachable, both configured models installed, timings parsed, 1024-dimension embeddings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Paul LütznerandClaude Opus 5
|
92004ee576 |
fix(jobs): background work would not open, and finished work never left
Two faults in the same indicator, reported together as "it fades in and out but clicking does nothing" plus three "Marking as read" jobs stuck at partial progress that had stopped weeks earlier. The menu was hidden by Tailwind's `hidden` CLASS while ui--dropdown opens a menu by clearing the `hidden` PROPERTY and never touches classes. So the panel was display:none for the whole life of the page — and because the attribute was absent, the first click read the menu as already open and CLOSED it. No error, no console warning, nothing to search for. Every other dropdown in the app uses the attribute; this was the one that did not, and a static test now says so. The stuck jobs were a different fault with the same face. findVisibleForUser matched `state IN (queued, running)` with no time bound of any kind, so "still going" and "abandoned" were literally the same row and no amount of waiting would ever change it. A job that fails properly calls finish() and clears after five minutes; these never reached that line — a killed worker, a restart mid-deploy, a fatal that bypassed the failure path. The entity had no progress timestamp, so a long legitimate job could not be told from an abandoned one by time. It has one now, stamped by begin() and advance() so the counter and the stamp cannot come apart, the query is bounded by it, and app:jobs:reap marks the stragglers failed so they are seen once and then go. The migration backfills last_progress_at from created_at, which is the only value that gets both directions right: a job stranded weeks ago is instantly outside the window and swept, while a bulk action started two minutes before the deploy is inside it and left alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Paul LütznerandClaude Opus 5
|
469a2e8107 |
feat(ai): semantic search, writing help, and the pass that embeds a mailbox
The three remaining pieces. Written to the seams the survey found, and
keeping the traps it named.
SEARCH — ANOTHER ARM OF THE SAME UNION
A vector hit is a candidate on exactly the same footing as a lexical one:
it lands in COUNT(*) OVER (), it pages correctly, and it needs no second
statement to reconcile with. Four things had to hold and each is written
down where it applies:
· The query is embedded ONCE, in the controller. buildSearchSql() runs
up to four times per search — the cheap pass, the body rescue, and
twice more when a page past the end recovers its total — and one of
those is inside a statement-timeout transaction, where a slow model
would be reported as a database fault.
· The candidate set is BOUNDED to the most recent 2,000 messages. The
distance function costs 0.11ms a row; over a whole mailbox it is
eleven seconds, worse than the pathology the UNION shape exists to
escape.
· A THRESHOLD, not a top-k. The total rides on the same statement, and
"lexical matches plus up to k" is a number that is only true on page
one.
· The inner LIMIT is parenthesised, because these arms are joined with
a bare ' UNION ' and an unparenthesised LIMIT binds to the whole
union rather than to one arm.
The rank join is 1:1, which is the property the label joins lost and had
to be given back in v0.1.40. Three tests: a message found by meaning that
no lexical arm can reach, one rejected by the threshold, and one with no
embedding still found by words — that last is what proves the LEFT JOIN
does not drop rows.
WRITING HELP — THE SERVER MAKES THE CALL
connect-src 'self' is enforced, so a browser could not reach the model
host even if it should — and it should not: that address is on the
operator's private network, and putting it in a page hands it to every
script the page loads.
The route carries NO draft id. A composer opens before one exists, which
is exactly when somebody wants a first draft; a URL built at render time
with an id in it is stale in that window. Nothing is saved as a side
effect either.
Four tasks, a closed set. A free-form instruction parameter would be a
prompt an attacker could write, with the user's own mail as context and
the answer pasted into the user's own draft. The message being replied to
is read from the DATABASE by id, ownership checked — never from the page.
Generated text is INSERTED, never substituted. A rewrite that overwrote
the original would be a model deleting somebody's words with no undo they
can see. It goes in as a plain text node at the end of the body: a styled
wrapper would be subtracted by the composer's typed-length calculation
and could stop the draft autosaving, and "at the caret" is a position the
writer did not choose, because opening the menu moved focus.
The menu is behind a Twig FUNCTION, not a variable — this window is
included from four places, two of them undo streams that include it
`only`, and strict_variables makes a variable threaded through three of
four a 500 on the undo path rather than a missing button.
EMBEDDING — ARRIVALS ON INGEST, THE REST ON DEMAND
A post-ingest step for new mail, and a command for everything already
there. The backfill re-dispatches itself with a cursor rather than
looping: a hundred thousand messages at a round trip each is hours, and a
single job holding that is killed by any worker restart with nothing to
show for it.
Ascending id, because it is the one ordering nothing can change under a
walk that long — mail arriving during it gets a higher id and is met by
the pass still coming. The vectors already stored ARE the progress
record, so there is no second one to keep in step, and the cursor
advances past everything LOOKED AT rather than everything stored, so one
message the model cannot answer for does not become a wall it restarts at
forever.
On the maintenance transport, not ingest: a backfill running for hours
would otherwise sit in front of every arriving message, and new mail
would stop appearing until an old mailbox had finished being catalogued.
KNOWN FAILING, AND NOT FROM THIS
compose-send-optimistic.spec.ts:124 fails in the FULL e2e run and passes
in isolation and in pairs — verified with the AI menu both present and
removed, 24 passed either way. It is the same spec that broke in the
earlier seed race, and the row it cannot click is present but still
mid-entrance. Cross-file interference, pre-existing, and left for the
debugging pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Paul LütznerandClaude Opus 5
|
331c97d5c8 |
fix(search): every search paid for label joins it never read
Reported from the live install: one search for "amazon", 2.1 seconds mean
over two executions, from the slow-query panel.
The statement joins `thread_label` and `label` unconditionally. Exactly
two filters ever read them — `label:` and the mailbox role behind `in:` —
and a free-text search reads neither. Both joins are to-many, so every
(thread, matching message) row was multiplied by the number of labels the
conversation carries, and the GROUP BY then collapsed the duplicates back
down. A mailbox whose threads sit in an inbox, a category and a user
label did three times the work to build a set it discarded.
The answers were always right. That is why it survived: GROUP BY hides
the duplication perfectly, and the only symptom is the clock.
Joined now only when something asks about a label.
THE TEST IS ABOUT THE SQL, NOT THE RESULTS
Unusual here and worth stating: there is nothing observable to assert on.
The result set was identical before and after, so an assertion about rows
could not fail if the joins came back. The test reads the built statement
and requires the label tables to be absent for free text and present for
`label:` and `in:`.
ALSO: a flaky booking spec, fixed properly
calendar-sharing checked a weekday box with check({force:true}). The input
is `sr-only` — clipped to one pixel and out of flow — so there was never
anything to click, and once the modal grew tall enough to scroll it
failed with "Element is outside of the viewport": a message about
geometry for what was really a wrong target. It clicks the <label> that
wraps the input now, which is what a person clicks and what toggles the
box. Matching that label by text does not work either, because the
template indents and its textContent is "\n Sat\n " — so it goes from
the input to its ancestor label instead.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Paul LütznerandClaude Opus 5
|
247938a2d8 |
fix(search): the body pass could spend the whole request and answer with a 500
Reported from the live install as
MaxExecutionTimeError: Maximum execution time of 30 seconds exceeded
at Statement.php line 55
which is one SQL statement, not an N+1.
WHICH STATEMENT
Free-text search runs a cheap indexed pass and, when that comes back with
less than a screenful, falls back to scanning message bodies for the raw
substring. buildSearchSql() has carried a note about that fallback for a
while, and the note already said what happened next: through DBAL's bind
parameters one term went "from 13 seconds to a 30-second PHP timeout".
The mitigation at the time made the pass CONDITIONAL. That made it rare.
It did not make it bounded, and a large enough mailbox found the gap.
It was also the only query in the codebase without a budget.
TypeAheadSearch has had `SET LOCAL statement_timeout` since it was
written, for exactly this hazard, one class over.
THE FIX
Five seconds, enforced by Postgres, and on expiry the rows the cheap pass
already found are returned. Losing an occasional infix match is the
deliberate price; a 500 after thirty seconds was not a price anyone chose.
Postgres rather than PHP on purpose. `max_execution_time` kills the PHP
process and leaves the database still executing a query whose answer
nobody is waiting for any more — the request is gone and the scan is not.
`statement_timeout` stops the actual work.
WHY THE TESTS PROVE SOMETHING
A budget test that passes because the pass succeeded proves nothing, so
the pair is built around "elica", which sits inside "pelican" and nowhere
else: full-text stems whole words, the prefix pass matches `'elica':*`
which "pelican" does not start with, and it is in no subject or sender
column. Only `body_text ILIKE '%elica%'` can find that row. It IS found
at the default budget and is NOT found at one millisecond, with nothing
thrown — so the pass demonstrably runs, and the guard demonstrably bounds
it.
The budget is injectable for that reason. A give-up path nothing ever
exercises is one nobody knows the shape of.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
||
|
Paul LütznerandClaude Opus 5
|
6c6b412367 |
fix(mail): break ties in the three orders that were not breaking them
ListSortOrder::applyTo() already carries the rule for the folder lists — sort, then id — and says why: LIMIT/OFFSET over a non-deterministic sort can show one conversation on two pages and another on none, and lastMessageAt ties are ordinary in a mailbox rather than an edge case. Three queries spell their own ORDER BY and so never got it. The one that matters is findMatchingNormalizedSubjectThreadForAccount, and not for pagination — it takes ONE row. Two candidate conversations sharing a lastMessageAt left "which conversation does this message join" to the query planner, so the same mailbox could thread the same message differently on two runs. It was picking the OLDER of the two, which the new test pins by id: of two equally recent conversations, the one started later is the better guess. findDueSnoozed ties by construction — snoozing a selection gives every thread in it one wake time — and everything due is woken whatever the order, but with a cap on the batch the boundary was arbitrary. findForLabels is the paginated one, and the only one where a tie could actually lose a row off a page. It has no callers anywhere in the codebase; it is fixed rather than deleted because removing it is a separate decision from correcting it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Paul LütznerandClaude Opus 5
|
4cfeee9ab5 |
feat(mail): the unread badge opens the mail it counted
Asked for as "a click on the unread badge should show a view from that label with all the unread mails". The badge already knew the number; what it could not do was take you to it. Clicking one now opens that same view with `?unread=1` — the inbox, Starred, Archive, Spam, Snoozed, or a label, under one account or across all of them. A filter on the view rather than a route of its own, exactly as `?account=` is: the account narrowing, the sort order and the tab all have to keep working underneath it, and a second route would have had to reimplement each of them. Deliberately NOT a link into search, which was the cheap way to do this and would have been wrong at the edge that matters. Search can already say `label:Work is:unread`, but it has no `account:` operator — and the folder rows under an expanded account count within THAT account on purpose, because a badge promising more than the list beside it shows is the bug forLabelInAccount() exists to prevent. A search link would have reintroduced it for exactly those rows. ## The counts had to change first They were SUM(t.unreadCount) — unread MESSAGES — while every list in the app draws one row per CONVERSATION. Harmless while the badge was only ever read. Not harmless once it is the thing you click to see that mail: "Inbox 4" would have opened three rows, which is the Trash "188 unread against a list of 193" again, one click apart instead of one glance. So role, per-label and the LABELS roll-up all count conversations holding unread mail now. countUnreadForStarred() already did, so this also ends a disagreement between the badges themselves — Starred counted conversations while everything else counted messages, and nothing said so. The handbook already described the roll-up as "a count of conversations", which was not true when it was written and is now. UnreadFilteredListsTest asserts the count and the list AGAINST EACH OTHER rather than against fixed numbers, on fixtures whose threads hold several unread messages each — the one shape under which the two rules differ, so a revert to summing fails rather than passing by luck. ## The badge is a real link Which is why the system rows are a .nav-item DIV wrapping their anchor now, the shape the label tree and the account folder rows were already built in — an <a> inside an <a> is not markup that survives parsing, and the row would have kept whichever one the parser chose. The alternative, a span with a click handler, would have shipped a feature keyboard users simply do not have. data-nav-tier rides on the LINK rather than on the pill, and that is not cosmetic: the tier is padding-block, padding on the wrapper is area the anchor does not cover, and the row would have looked 36px tall while answering a click on 20 of them. The wrapper takes its height from the link, so the geometry is unchanged to the pixel — density.spec.ts measures the anchor and still reads 36 / 48 / 36. Trash and Drafts stay spans: those badges are totals, so there is no unread question being asked. So does the LABELS roll-up, which stands in for several lists and has no single one to open. A narrowed list says "Unread only" with "Show all" beside it, in the shared toolbar rather than in six view templates — a filtered list and a genuinely quiet one look identical otherwise. The way back is this route rebuilt from the request rather than named, so route parameters and query parameters both survive it; only `unread` and `page` are dropped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Paul LütznerandClaude Opus 5
|
b3a763f6c9 |
perf(search): the result total rides along with the page instead of re-running it
Every search ran two statements with the same WHERE: one for the page, one `SELECT COUNT(DISTINCT t.id)` for the "1-50 of N" in the toolbar. On the 300,000-message corpus the second measured 620ms alone and 450-830ms per request — a third of the wait, for a number nobody navigates by, produced by repeating every scan the first statement had already paid for. It is now `COUNT(*) OVER ()` on the page query. The window runs over the grouped rows the GROUP BY has produced anyway, and the sort above it already materialises them, so it measured 35ms — about three per cent. Bounding the count instead — `SELECT count(*) FROM (SELECT … LIMIT 1001)`, rendered as "1000+" — was the obvious cheaper move and does not work here. The LIMIT sits above a DISTINCT, Postgres answers that with a HashAggregate, and a HashAggregate reads all of its input before emitting its first row. Measured at 717ms against the unbounded 620ms: it made the query slower AND the number a lie. Deferring it into a lazy frame would have kept the same database work and added a request. It also removes a way the two could disagree. search() re-runs with the body-substring rescue when a page comes back thin; countSearch() never did, so a rescued page could show more rows than the total above it admitted existed. One statement cannot contradict itself. The one case a window count cannot answer is a page past the end, which has no row to carry it — answering zero there would tell somebody following a stale `?page=12` link that their query matches nothing, with the pager gone and no way back. totalForEmptyPage() reads one row off the front instead. Medians of seven, through the app: 1910 -> 1586ms, 3120 -> 2693ms, 3013 -> 2375ms, and one slow query per search rather than two. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Paul LütznerandClaude Opus 5
|
721afdba32 |
feat(mail): an account row opens that account's inbox
Clicking your own address in the sidebar listed everything the account held, whatever it was labelled — so the list opened with your sent mail interleaved through the mail you had received, drafts and spam among it. That is an archive. What a person means by clicking a mailbox is "show me what came in", and the sidebar's top-level Inbox is the same question across every account. findForAccountInbox() is findForUnifiedInbox() narrowed to one account and widened across every category. Two differences from that query, both deliberate: no category filter, because the tabs belong to that one screen while this is the account's Inbox FOLDER — the same list its own Inbox row in the expanded tree gives; and no isActive filter, because a switched-off account is still in the sidebar and still clickable, and answering with an empty list because it is asleep would read as lost mail. Nothing became unreachable. Sent, Drafts, Spam and the bin are folder rows under the account, one click further, which is where mail that is not incoming has always been. The heading names both facts — "Inbox" and the address — because either alone fails to distinguish this screen from the unified inbox above it or from the account's other folders beside it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Paul LütznerandClaude Opus 5
|
a18be77a51 |
feat: a sidebar that folds away, and remembers it wherever you sign in
The LABELS and ACCOUNTS headings are disclosures now. Collapsing one is remembered against the user rather than the browser, so it follows you to your phone, and it is rendered into the first paint rather than applied afterwards, so nothing opens and then snaps shut on the way past. The unit of collapse is the SECTION, decided from the data: user label trees are flat in practice — 24 custom labels in the development mailbox and not one of them nested — so per-label collapse had nothing to fold. What actually pushes the accounts off the bottom of the nav is the wall of labels itself, and that is what now folds. The per-label trees keep working for anyone who does nest, out of the same stored set. Reuses the idiom already in the file rather than adding a second one beside it. Both headings are <details>/<summary>, like the "More" block above them, so the chevron, the keyboard activation, the accessible name and the expanded state a screen reader hears are all the ones this sidebar already had. The only difference from More is where "should this be open?" comes from: More asks the route, because the honest answer there is "are you in it?", and these ask the settings bag, because the honest answer here is "how did you leave it". It also retires the localStorage the label trees used to live in. That was per browser and was re-applied on connect, so a collapsed tree was rendered open and shut a frame later, on every single navigation. One key space now — section:labels, section:accounts, label:<full name> — one endpoint, one handler, stored as COLLAPSED so a label created tomorrow shows up expanded without backfilling anybody's settings. The collapsed LABELS heading carries the unread that is hidden under it, because a section you cannot see is a section whose mail you cannot see either. It is not the sum of the per-label counts: a conversation filed under two labels would be counted twice and the heading would promise more than expanding it could show, so it is a distinct-thread query with tests built to contain exactly that case. ACCOUNTS gets no such number, because there is no honest one — those rows count within an account and only the expanded one is ever loaded. Badges inside a shut section are hidden rather than omitted, so refreshCounts() still patches them by data-count-key and a reopened section is never showing numbers from three syncs ago. The new roll-up rides the same counts payload. No migration: it is a key in the settings bag. It joins the config backup, unlike its sidebar.expanded_account neighbour, because it holds section names and label full names rather than a row id that means someone else on the target host. Storage is separate from SETTING_SIDEBAR_ACCOUNT on purpose — that answers which account is expanded, this answers whether the accounts are on screen at all — and neither writes the other, so collapsing and reopening the section brings the same account back still open. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Paul LütznerandClaude Fable 5
|
52daa63ea3 |
fix(mail): a thread reads in conversation order, replies included
A message the account sent has a sentAt but no receivedAt, and the thread's messages were ordered on receivedAt alone — so Postgres sorted every reply to the very bottom of its own thread, beneath mail that arrived after it, and the reply zone quoted the wrong 'latest' message. The thread view now loads its messages through a custom query ordered by COALESCE(receivedAt, sentAt, createdAt) — a HIDDEN alias, since DQL can't ORDER BY a function — and threads the real latest through to the reply zone. No stored column: the three dates already hold the answer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
Paul Lützner
|
110cfb298c |
Merge fix/search-routing-i18n
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> # Conflicts: # migrations/Version20260811160000.php # templates/calendar/_event_modal.html.twig # templates/mail/_message_details.html.twig # templates/mail/print.html.twig |
||
|
Claude
|
30a71adac3 |
fix: satisfy phpstan, and stop a third test asserting the old date arrangement
coerce() now returns DateTimeImmutable rather than DateTimeInterface: the
interface has no setTimezone(), and narrowing it also removes the chance of
editing a mutable DateTime the template handed us just by rendering it.
The hostile-query test asserted only that a search returned an array, which
phpstan rightly called always-true. It asserts counts now, which is the actual
claim: punctuation is stripped and the words around it still work, and a query
made of nothing but punctuation finds nothing rather than everything — the
latter being what `!` would do if it reached to_tsquery as an operator.
PublicCalendarStyleTest built its expected date with format('j M Y'), the same
way the two tests fixed in the previous commit did.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
||
|
Claude
|
5ff870c3b7 |
fix(search,routing,a11y): prefix+substring search, page clamps, 404s, contrast
H-04 search found only exact whole words. The tsvector tokenizer emits "help.wirhub.de" as one host lexeme, so "wirhub" is a substring of a lexeme and no full-text query can reach it, while "Testmai" needs a prefix operator websearch_to_tsquery has no way to express. Free text now compiles to three OR-ed passes — websearch as before, a to_tsquery with :* per term, and an ILIKE needle for single terms of 3+ characters — with pg_trgm GIN indexes so the third is index-backed. Deliberate websearch syntax (quotes, OR, negation) suppresses the extra passes rather than widening what was narrowed on purpose. H-06 page numbers past the end rendered "201-193 of 193" over an empty list. The six role views were six copies of the same eight lines, so they are now one. H-07 /mail/thread/abc and ?tab=quatsch were 500s: a non-numeric id reaching the entity resolver, and MessageCategory::from on a URL parameter. Error pages are branded and localised, and deliberately standalone — the app shell asks for a session, which an anonymous 404 does not have. A-01/A-02 <html> had no lang at all, and the viewport forbade zoom. A-07 accent ink was effectively always white (2.34:1 on the reported pink); it is now chosen by WCAG relative luminance on gamma-expanded channels, scoring both candidates instead of guessing at a threshold. Every theme's ink ramp now clears 4.5:1 on its own surface and still runs strongest-to-faintest. I-02 search.title existed in no catalogue. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
Paul LütznerandClaude Fable 5
|
66de18afaf |
fix(accounts): one account order, in the sidebar and in settings
The sidebar showed sven, uptime, pmd; settings showed pmd, sven, uptime; and the drag handles meant to arrange them are on the settings page, which then sorted their result away by name on the next render. So the control arranged nothing and the two lists disagreed. Two defects. Settings (and the stream that replaces its list after a toggle or delete) read findForUserOrderedByName, discarding sortOrder entirely. And the sidebar ordered by sortOrder with no tiebreak — the column defaults to 0 and is only rewritten by a drag or a delete, so on an install where nobody has dragged anything every row sits at 0 and the order is whatever Postgres felt like. Both now read the arranged order with a name tiebreak, so an untouched install is alphabetical in both places and an arranged one is arranged in both. The rule editor's account picker joins them; the calendar source lookups keep their own ordering, being a discovery list rather than the user's arrangement. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
Paul LütznerandClaude Fable 5
|
c68e992c85 |
feat(imap): an expunge the server announces sweeps now, not on the cadence
The IDLE supervisor read the server's untagged EXPUNGE lines off the wire and dropped them — it only ever matched EXISTS. It now answers a deletion the way it answers an arrival: which message died stays deliberately unresolved (a sequence number, and the sweep is the authority with the safety rails); the folder's sweep clock is backdated and the ordinary sync dispatched, so the deletion lands in seconds instead of within two fifteen-minute cycles. The clock only ever moves backward — LEAST(), and NULL stays NULL — because the reaper's coverage rule trusts these timestamps, and a clock advanced without a listing would claim one that never happened. Pinned from both sides in MailboxSweepDueTest. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
Paul LütznerandClaude Fable 5
|
576a886071 |
fix(search): trash stays in trash, and the default calendar speaks the user's language
Search now excludes threads carrying the Trash label unless the query says in:trash — via NOT EXISTS, because the label join exists once per label and a condition on it would let a deleted thread keep matching through its other labels. The provisioned default calendar takes its name from the catalogue in the owner's locale; existing calendars keep whatever name they have, because renaming data the user may have edited is an overwrite, not a translation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
Paul LütznerandClaude Fable 5
|
aba7d34df5 |
fix(mail): archive files somewhere, spam has a row, and the bin stays in the bin
Three reports that turn out to be three holes in the same model — a list is a label query, so anything the label does not say cannot be shown. ARCHIVE. archive() removed the Inbox label and added nothing. The docblock said "Archive is the removal of the Inbox label", which reads like Gmail's rule and is not: Gmail can afford it because it has All Mail, and plMail's Archive screen is findForRole(Archive) — a query for a label nothing ever attached. So archiving filed mail nowhere at all: out of the inbox, absent from Archive, its badge stuck at zero, reachable only by search or by a custom label it happened to carry. The missing line is the one trash() has always had. Custom labels are untouched, so a conversation keeps its labels the way Gmail's does. The sidebar's "More" disclosure also snapped shut on every navigation, so choosing Archive from it looked like the menu had collapsed instead of doing anything. The sidebar is deliberately not turbo-permanent and re-renders whole, and unlike the label trees this <details> had no persistence at all. It now renders `open` when you are on one of the pages inside it — decided on the server, because "should this be open?" means "are you in it?", and the server already knows. Archive also gains the heading Trash has; an unnamed empty list was the entirety of the feedback anyone got. SPAM had no row, no route and no translation key. The eye toggle in label settings happily switched it visible — that route allows system labels on purpose — and nothing anywhere was looking, because the system nav is a hand-written sequence of anchors rather than a loop. Now there is an app_mail_spam route, a view shaped on Trash's, and a row behind the same toggle Archive's is behind, sitting where LabelRole::sortOrder() already put it. hasVisibleArchive() generalised to hasVisibleRole(). TRASH leaked everywhere. Thread labels are the union of message labels, and trashing keeps every custom label a message had, so a deleted conversation went on listing under "Receipts", under its account, in Starred and in the inbox — looking live. Every list query now excludes threads carrying a Trash label, except the bin's own view and the per-account Trash folder row, which goes through findForLabel() like any other label and would otherwise render permanently empty. The unread badges exclude it too, so a count agrees with the list under it. findForAccount() had to become a QueryBuilder to say it at all: "not in the bin" is a condition on a to-many, which findBy() cannot express. Search is deliberately untouched — it is raw DBAL with its own WHERE builder and an `in:trash` operator to respect, so it is a separate change with its own risks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
Claude
|
1195a9d7a3 |
Sort search results by date, and let the reader say otherwise
Search answered in ts_rank order and only that, so a keyword that matched a mail from 2004 and two from 2026 put them on the page in an order with no relation to when any of them arrived. Read as a page that had ignored the question. The default is now newest-first — what the reader expected, and what every mail client they already use does — with relevance kept as the other position of a switch beside the pagination. The switch is two links into the same route inside the list frame, so changing order is one navigation rather than a write followed by a reload, and the controller remembers the choice as a side effect of answering it. Remembered in the user's settings bag, beside the sidebar's expanded account and the calendar pane's mode, because it is the same kind of thing: one string of view state that nothing queries by and that should follow the user to their other devices. Switching starts again at page one — page four of one order is page four of nothing in the other — while paging carries the order in the URL. Both orders now end in thread_id DESC, which fixes a bug that predates the switch. `ORDER BY rank DESC, last_message_at DESC` is not a total order, and LIMIT/OFFSET over a partial one is free to break ties differently per page: measured on 20k tied rows, three pages of 50 returned 148 distinct rows — two shown twice, two shown never. Not exotic either, since ts_rank is degenerate for a query that stems to nothing and ties every row in the result. JMAP's EmailQueryRunner compiles its own ordering and is untouched. |
||
|
Paul Lützner
|
84a5eed8b3 | Merge branch 'feat/push-monitoring' | ||
|
Paul Lützner
|
2bf3c38add | small fixes that claude couldnt commit because who knows | ||
|
Paul LütznerandClaude Fable 5
|
287c93805d |
Record where every push went, and stop scheduled sends being invisible
Every attempt both senders make lands in push_delivery - transport, device, outcome, the error name that tells a dead phone from a Firebase outage, latency - browsable under /admin/push/deliveries and echoed per device in the user's notification settings. Nothing of the payload past its @type is stored, and the record survives the subscription it is about. A held submission now answers EmailSubmission/get as pending with its real release time, a cancelled one as canceled, and the changes feed reports both transitions - the release time used to exist only in the create response, which forced clients to keep schedules device-local. Cancelling a never-submitted mail is refused instead of arming a flag the next real send would silently trip over. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
Paul Lützner
|
622f3bb75d |
Break an autocomplete tie by who was written to most recently
Frequency alone was the whole order, and a tie was left to whatever
order Postgres felt like returning. Ties are not the exotic case here —
most of an address book is people seen exactly once — so that was most
of the list. Two symptoms fell out of it: suggestions that reshuffled
between keystrokes as the plan changed, and the colleague mailed last
week sitting below a stranger from two years ago because both had
frequency 1.
The order is now `frequency DESC, last_seen_at DESC`. Recency is a
tie-break and not a competitor: somebody written to twenty times last
year still outranks somebody seen once this morning, and there is a test
named for that so the new key cannot quietly become the primary one.
This changes the web composer too, deliberately. Both surfaces read
ContactRepository::findForAutocomplete, which is the whole reason the
ranking lives there — two rankings would make the suggestion order
depend on which device you were addressing mail from.
The ORDER BY carries an explicit nulls-last CASE, and it is worth saying
why it looks like that and why it is there at all:
DQL cannot say NULLS LAST The ORM 3.6 parser rejects the suffix
outright. A CASE in the ORDER BY is the
spelling that survives, and it compiles to
the SQL you would have written by hand.
Postgres would get it NULLs sort FIRST on a DESC order, so the
backwards database default would put never-seen
contacts at the top of every list.
It cannot fire today last_seen_at is NOT NULL — one migration,
never altered — and both write paths stamp
it: the entity constructor, and upsertBatch
on insert and on conflict alike.
That last line is the honest caveat. The nulls-last branch is
unreachable while the column is NOT NULL, and it therefore has no test,
because a test for it would have to assert the shape of a query rather
than a behaviour. It is in the ORDER BY so that making the column
nullable later — an imported contact, a merge — does not silently invert
every suggestion list, which is the failure this file already collects
several of. ContactRepositoryTest says the same thing where the fixture
that would write the NULL would have gone.
Three tests. Two of them were mutation-checked the usual way — the
ordering reverted, the suite run, both confirmed failing on the
reshuffle. The third passes with or without the change by design: it is
the guard that recency did not take over as the primary key.
|
||
|
Paul LütznerandClaude Opus 5
|
5713910f88 |
Cover the reads and writes nothing else was watching
Lines 49.4% to 51.0%, but the number is not the point — these are six
files where a defect is silent, chosen over the larger uncovered classes
because line count and risk are not the same thing.
ContactRepository the upsert is one statement doing four jobs, and
every one of them fails quietly: a lost
correspondent flag files a colleague's reply as an
advert, a bad address becomes an autocomplete
suggestion.
QueueMonitor the panel somebody reads precisely when mail has
stopped, so a wrong number there is worse than no
panel.
GraphSubscriptionMgr push failing is designed to be non-fatal, which
also means nothing ever says it happened.
MailRuleController the refusals are the feature: a rule acts on mail
with nobody watching.
MessageLookup dedup and the keyset walk — one duplicates mail,
the other silently skips it.
Two bugs fell out. The queue panel reported an unreadable message as
"MessageDecodingFailedException", because PhpSerializer returns a failure
wrapped in an envelope rather than throwing, so the catch never fired.
And phpstan reads the whole tree, not src/ — every narrower run this
session was less strict than CI.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|