Turn a mail into task candidates, and into nothing else (#246) #64

Closed
claude wants to merge 1 commits from overnight/email-extract into overnight/email-imap
Contributor

Second of the email-reader chain (Vikunja #246). Extraction plus the core-side seam; the poller daemon is the next branch.

What changed

  • internal/email/extract.goExtractor.Extract asks the resident model, under a GBNF grammar, for ≤3 candidates {text, due}. Junk and empty messages never reach the model. Repeats and impossible dates are dropped.
  • internal/ipc — new MethodIngestMail + IngestMailReq/Resp, client method, and a Server.IngestMailFn hook (same pattern as StepUp/UnlockFn; not a CoreAPI method).
  • internal/auth/policy.goMethodIngestMail listed explicitly at AuthRead, with the reasoning: it can only produce candidates.
  • internal/configemail block (max_tasks, timeout). No host, no credential, no mailbox: core does not connect to IMAP.
  • cmd/mavend/mail.go — the intake: extract, then CaptureTask with Status: candidate, Source: "email:<mailbox>", Evidence: <subject>. Wired on both startup paths (boot and passkey unlock).

Why this shape

Everything mail can produce is a candidate task. No reminder (a reminder fires — a misread mail must never be able to speak), no fact (Maven would later recite it as true), no calendar event, no note. The due date is stored where no scheduler reads it.

Core owns extraction because llama-server lives in core's process; the credential stays in the reader daemon, so core never sees it. The IPC hook is nil unless there is both an email block and a llama-server phraser — an unconfigured core answers ErrUnknownMethod rather than silently ignoring a reader. No keyword fallback: "the subject became a task" is a mailbox rendered as a to-do list.

Verified

make build and make test (go test -race) green. New tests: extraction (grammar used, empty array is normal, junk skips the model, caps, repeats/bad dates dropped, parse error hides mail text), intake (candidate status + source + evidence + due, idempotent on a re-read, junk writes nothing, no reminder is ever created, off without config), and the IPC hook (ErrUnknownMethod when unwired, round-trip when wired).

Vikunja #246

Second of the email-reader chain (Vikunja #246). Extraction plus the core-side seam; the poller daemon is the next branch. ## What changed - `internal/email/extract.go` — `Extractor.Extract` asks the resident model, under a GBNF grammar, for ≤3 candidates `{text, due}`. Junk and empty messages never reach the model. Repeats and impossible dates are dropped. - `internal/ipc` — new `MethodIngestMail` + `IngestMailReq/Resp`, client method, and a `Server.IngestMailFn` hook (same pattern as `StepUp`/`UnlockFn`; not a CoreAPI method). - `internal/auth/policy.go` — `MethodIngestMail` listed explicitly at AuthRead, with the reasoning: it can only produce candidates. - `internal/config` — `email` block (`max_tasks`, `timeout`). No host, no credential, no mailbox: core does not connect to IMAP. - `cmd/mavend/mail.go` — the intake: extract, then `CaptureTask` with `Status: candidate`, `Source: "email:<mailbox>"`, `Evidence: <subject>`. Wired on both startup paths (boot and passkey unlock). ## Why this shape Everything mail can produce is a candidate task. No reminder (a reminder fires — a misread mail must never be able to speak), no fact (Maven would later recite it as true), no calendar event, no note. The due date is stored where no scheduler reads it. Core owns extraction because llama-server lives in core's process; the credential stays in the reader daemon, so core never sees it. The IPC hook is nil unless there is both an `email` block and a llama-server phraser — an unconfigured core answers `ErrUnknownMethod` rather than silently ignoring a reader. No keyword fallback: "the subject became a task" is a mailbox rendered as a to-do list. ## Verified `make build` and `make test` (go test -race) green. New tests: extraction (grammar used, empty array is normal, junk skips the model, caps, repeats/bad dates dropped, parse error hides mail text), intake (candidate status + source + evidence + due, idempotent on a re-read, junk writes nothing, **no reminder is ever created**, off without config), and the IPC hook (ErrUnknownMethod when unwired, round-trip when wired). Vikunja #246
claude added 1 commit 2026-08-01 01:07:13 +02:00
The extraction half. internal/email.Extractor asks the resident Qwen3-1.7B,
under a GBNF grammar, what one message requires of him, and returns at most
three short candidates with an optional date.

Everything it can produce is a row in `tasks` with status "candidate",
written through the intake seam #130 built for exactly this (Source
"email:<mailbox>", Evidence = the subject line). No reminder, no fact, no
note, no calendar event. That bound is the design: a reminder FIRES, so a
1.7B misreading "встреча была в четверг" as a future appointment would wake
him up about it, whereas a wrong candidate is a line he dismisses in one
click. A due date the model read out of the mail is stored on the candidate,
where no scheduler reads it — the review page sorts by it. Relative wording
("до пятницы") is deliberately left in the text rather than resolved to a
date the model would get wrong.

The prompt is written against the two things a small model does here: it
summarises when asked to extract, and it invents an obligation out of a
polite closing line. Hence the demand for a verb phrase, and an explicit
empty array — most mail contains no task, and a model with no way to say
"nothing" says something.

Wiring: core owns extraction because llama-server lives in core's process,
so the reader hands messages over a new ipc.MethodIngestMail. It is a Server
hook (like StepUp/UnlockFn), not a CoreAPI method — not a store operation,
and no CoreAPI implementation should have to carry it. The hook stays nil
without an `email` config block or without a llama-server phraser, so the
method answers ErrUnknownMethod: off unless configured, twice over. There is
no keyword fallback on purpose — "the subject became a task" is a mailbox
rendered as a to-do list, not extraction.

Privacy: junk is refused before the model is called, mail text is never
search input, extraction errors carry byte counts rather than the reply, the
stored evidence is a truncated subject, and the log line names the mailbox
and the UID only.
claude reviewed 2026-08-01 11:33:39 +02:00
claude left a comment
Author
Contributor

The bound is real and it is structural, not a promise in a comment. Extractor holds no store and no writer, so it cannot persist anything. mailIntake.ingest hardcodes store.TaskCandidate and calls only CaptureTask, so no fact, reminder or nudge is reachable from a parsed mail. The double gate is the right shape too: no email block or no LLMPhraser leaves IngestMailFn nil, and MethodIngestMail then answers ErrUnknownMethod rather than succeeding quietly.

On the two boundaries you asked about, both hold in this PR. Core never learns the IMAP host, user or password. EmailConfig carries only max_tasks and timeout, and IngestMailReq carries only mailbox name and message text. Mail text also has no path to an external engine. Extractor talks to llm.New(lp.BaseURL(), ...), which is the local llama-server. The only outbound readers in the tree are internal/crawl and internal/kiwix, and this file touches neither. wireMailIntake on the unlock path also picks up the reassigned st and phr, so it binds the unlocked store and not the placeholder.

Three findings.

1. Extraction and the voice turn share one llama-server slot, and extraction is allowed two minutes.

startLlamaProc passes -m -c -ngl --no-webui and no -np. llama-server runs one slot, so requests queue. DefaultEmailTimeout is 2 minutes, and mailIntake.ingest spends it on a Thinking 1.7B reading up to MaxBodyBytes of 4000 characters with a 512-token budget.

Walk it. A mail arrives. mavmaild calls ingest_mail. The model starts extracting. He says something to Maven three seconds later. The router's Complete queues behind the extraction. Router p50 is 2.7s on this box, so the turn now takes as long as the extraction has left. The router degrades to the classifier cascade on error. That is the designed floor, so he gets 36.8% routing accuracy while his mail is being read. The phraser has no such floor and just waits.

PR 65 makes this worse, since the poller sends Max messages back to back and each is its own ingest call. A first poll against a mailbox with 40 unseen messages serialises 40 extractions ahead of every voice turn.

Nothing in this PR is wrong in isolation. The gap is that mail extraction is a background job competing with a foreground one for a single-slot resource. There is no priority, no queue depth limit and no backpressure. Give llama-server a second slot. Or have ingest decline while a turn is in flight. Or bound the daemon to one extraction per N seconds.

2. The extraction timeout also bounds the capture writes, so a slow model loses the candidates it just produced.

ctx, cancel := context.WithTimeout(ctx, m.timeout)
defer cancel()
cands, err := m.ex.Extract(ctx, msg)
...
id, created, err := m.st.CaptureTask(ctx, t)

Same ctx. The config comment calls Timeout a "per-message extraction budget", and it is not. Say extraction returns at 119 seconds of a 120-second budget. The first CaptureTask then gets one second against an encrypted store, and the third gets none. ingest then returns mail intake: capture: context deadline exceeded with a partially populated resp. The model did the work, the answer was good, and it is dropped. Derive a fresh context for the write loop, or scope the timeout to the Extract call only.

3. MethodIngestMail is the first AuthRead method that spends the resident model.

The policy comment argues the rung from what the method writes, and that argument is sound. Candidates are the cheapest possible wrong outcome. But the rung is also being asked to cover what the method costs, and that side is new. AuthRead requires nothing. Any process that can open the socket can post arbitrary text as ingest_mail. That pins the resident model for up to two minutes per call, repeatedly. Finding 1 turns that into a mute assistant. Every other AuthRead method is a store read or a bounded write.

Related, req.Mailbox goes into the source string unvalidated:

source := email.SourcePrefix + req.Mailbox

An empty mailbox gives source = "email:", and an arbitrary string gives an arbitrary provenance under the email: namespace. The source vocabulary is what the loop's rules trust. Validate it against the same shape the other sources use, and reject empty.

Smaller notes.

  • The enable log reads cfg.Email.MaxTasks while NewExtractor has already normalised it. With "email": {} in mavend.json the daemon logs mail intake: enabled (max 0 candidates per message, timeout 2m0s) and then allows three. Log the normalised value.
  • parseCandidates says it tolerates "leading reasoning before the array", but extractGrammar pins the first token to [. The tolerance cannot be exercised through Extract. Harmless, except the comment tells the next reader that thinking output is expected here when the grammar forbids it.
  • newMailIntake disables mail on a failed phr.(*phraser.LLMPhraser) assertion. That is the correct default. But the log line says "configured but no llama-server phraser" for what could equally be a wrapped phraser. If anything ever wraps the phraser, mail turns itself off and the log points at the wrong cause.
  • ingest returns IngestMailResp{} for "the model ran and found no task" and IngestMailResp{Skipped: true} for "we did not ask". The distinction is right. State in the IngestMailResp doc that Created == 0 && !Skipped means the model was consulted. PR 65 has to decide whether to mark such a UID seen.
  • Candidate text is a model paraphrase of mail content and lands in tasks.text. Evidence is subject-only, so the "no copy of his mailbox" claim holds today. It stops holding the moment anything assembles a context block from live tasks. Worth a line in the package comment so the next person adding task context to a prompt sees the constraint.
The bound is real and it is structural, not a promise in a comment. `Extractor` holds no store and no writer, so it cannot persist anything. `mailIntake.ingest` hardcodes `store.TaskCandidate` and calls only `CaptureTask`, so no fact, reminder or nudge is reachable from a parsed mail. The double gate is the right shape too: no `email` block or no `LLMPhraser` leaves `IngestMailFn` nil, and `MethodIngestMail` then answers `ErrUnknownMethod` rather than succeeding quietly. On the two boundaries you asked about, both hold in this PR. Core never learns the IMAP host, user or password. `EmailConfig` carries only `max_tasks` and `timeout`, and `IngestMailReq` carries only mailbox name and message text. Mail text also has no path to an external engine. `Extractor` talks to `llm.New(lp.BaseURL(), ...)`, which is the local llama-server. The only outbound readers in the tree are `internal/crawl` and `internal/kiwix`, and this file touches neither. `wireMailIntake` on the unlock path also picks up the reassigned `st` and `phr`, so it binds the unlocked store and not the placeholder. Three findings. **1. Extraction and the voice turn share one llama-server slot, and extraction is allowed two minutes.** `startLlamaProc` passes `-m -c -ngl --no-webui` and no `-np`. llama-server runs one slot, so requests queue. `DefaultEmailTimeout` is 2 minutes, and `mailIntake.ingest` spends it on a Thinking 1.7B reading up to `MaxBodyBytes` of 4000 characters with a 512-token budget. Walk it. A mail arrives. mavmaild calls `ingest_mail`. The model starts extracting. He says something to Maven three seconds later. The router's `Complete` queues behind the extraction. Router p50 is 2.7s on this box, so the turn now takes as long as the extraction has left. The router degrades to the classifier cascade on error. That is the designed floor, so he gets 36.8% routing accuracy while his mail is being read. The phraser has no such floor and just waits. PR 65 makes this worse, since the poller sends `Max` messages back to back and each is its own ingest call. A first poll against a mailbox with 40 unseen messages serialises 40 extractions ahead of every voice turn. Nothing in this PR is wrong in isolation. The gap is that mail extraction is a background job competing with a foreground one for a single-slot resource. There is no priority, no queue depth limit and no backpressure. Give llama-server a second slot. Or have `ingest` decline while a turn is in flight. Or bound the daemon to one extraction per N seconds. **2. The extraction timeout also bounds the capture writes, so a slow model loses the candidates it just produced.** ```go ctx, cancel := context.WithTimeout(ctx, m.timeout) defer cancel() cands, err := m.ex.Extract(ctx, msg) ... id, created, err := m.st.CaptureTask(ctx, t) ``` Same `ctx`. The config comment calls `Timeout` a "per-message extraction budget", and it is not. Say extraction returns at 119 seconds of a 120-second budget. The first `CaptureTask` then gets one second against an encrypted store, and the third gets none. `ingest` then returns `mail intake: capture: context deadline exceeded` with a partially populated `resp`. The model did the work, the answer was good, and it is dropped. Derive a fresh context for the write loop, or scope the timeout to the `Extract` call only. **3. `MethodIngestMail` is the first `AuthRead` method that spends the resident model.** The policy comment argues the rung from what the method writes, and that argument is sound. Candidates are the cheapest possible wrong outcome. But the rung is also being asked to cover what the method costs, and that side is new. `AuthRead` requires nothing. Any process that can open the socket can post arbitrary text as `ingest_mail`. That pins the resident model for up to two minutes per call, repeatedly. Finding 1 turns that into a mute assistant. Every other `AuthRead` method is a store read or a bounded write. Related, `req.Mailbox` goes into the source string unvalidated: ```go source := email.SourcePrefix + req.Mailbox ``` An empty mailbox gives `source = "email:"`, and an arbitrary string gives an arbitrary provenance under the `email:` namespace. The source vocabulary is what the loop's rules trust. Validate it against the same shape the other sources use, and reject empty. Smaller notes. - The enable log reads `cfg.Email.MaxTasks` while `NewExtractor` has already normalised it. With `"email": {}` in mavend.json the daemon logs `mail intake: enabled (max 0 candidates per message, timeout 2m0s)` and then allows three. Log the normalised value. - `parseCandidates` says it tolerates "leading reasoning before the array", but `extractGrammar` pins the first token to `[`. The tolerance cannot be exercised through `Extract`. Harmless, except the comment tells the next reader that thinking output is expected here when the grammar forbids it. - `newMailIntake` disables mail on a failed `phr.(*phraser.LLMPhraser)` assertion. That is the correct default. But the log line says "configured but no llama-server phraser" for what could equally be a wrapped phraser. If anything ever wraps the phraser, mail turns itself off and the log points at the wrong cause. - `ingest` returns `IngestMailResp{}` for "the model ran and found no task" and `IngestMailResp{Skipped: true}` for "we did not ask". The distinction is right. State in the `IngestMailResp` doc that `Created == 0 && !Skipped` means the model was consulted. PR 65 has to decide whether to mark such a UID seen. - Candidate text is a model paraphrase of mail content and lands in `tasks.text`. Evidence is subject-only, so the "no copy of his mailbox" claim holds today. It stops holding the moment anything assembles a context block from live tasks. Worth a line in the package comment so the next person adding task context to a prompt sees the constraint.
kami closed this pull request 2026-08-01 14:51:49 +02:00
Owner

Landed on master. The stack was one linear chain, so #84 carried every commit from #50 up, and master now contains this branch in full. Merging this PR on its own is an empty diff, so it is closed rather than merged. The review findings for it were fixed in the 2026-08-01 pass and are on master as commits on the stack tip, not on this branch.

Landed on master. The stack was one linear chain, so #84 carried every commit from #50 up, and master now contains this branch in full. Merging this PR on its own is an empty diff, so it is closed rather than merged. The review findings for it were fixed in the 2026-08-01 pass and are on master as commits on the stack tip, not on this branch.

Pull request closed

Sign in to join this conversation.
No Reviewers
No Label
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: kami/Maven#64