Skip to content

feat(email): extract action items into a persistent task list#1917

Open
kovtcharov wants to merge 1 commit into
mainfrom
claudia/task-e79043a6
Open

feat(email): extract action items into a persistent task list#1917
kovtcharov wants to merge 1 commit into
mainfrom
claudia/task-e79043a6

Conversation

@kovtcharov

Copy link
Copy Markdown
Collaborator

Action items extracted during triage were inline-only — useful in the moment, gone with the response, so commitments buried across threads silently slipped. Now every POST /v1/email/triage / triage/batch also persists each extracted item as a task row in the sidecar's local SQLite, linked back to the source message_id and de-duplicated per message, so re-triaging never duplicates a task. Fully additive: the wire response, contract, and SCHEMA_VERSION are unchanged.

The L8 cross-agent task store (#1521) isn't in-tree yet, so tasks persist to an email-local email_tasks table; task_store.record_action_items is the single seam that becomes the adapter forwarding tasks (source_ref=message_id) when that EPIC lands. MCP stdio triage stays analyze-only for now — persistence is wired at the REST surface the Agent UI drives (wiring the MCP subprocess needs a test seam like GAIA_EMAIL_MCP_FAKE_SEND; noted as follow-up).

Closes #1605

Test plan

  • python -m pytest hub/agents/python/email/tests/test_email_task_store.py -v — store unit tests (create/link, no-dup on re-record, normalized dedup key, fail-loud on missing message_id) + REST surface tests (triage persists linked tasks, re-triage creates no duplicates, batch dedups per message)
  • python -m pytest hub/agents/python/email/tests/ tests/test_email_openapi_conformance.py tests/unit/agents/email/ tests/mcp/test_email_mcp_stdio_parity.py tests/test_api.py — 733 passed (contract freshness, MCP parity, existing triage behavior unchanged)
  • python util/lint.py --all — clean

Triage returned action_items inline-only, so commitments extracted from
email evaporated with the response. The REST triage endpoints now also
write each extracted item to a persistent email_tasks table (the same
SQLite as the action log), linked back to the source message_id and
de-duplicated per message on the normalized description — re-triaging a
message never duplicates tasks. Additive: the wire response, contract,
and SCHEMA_VERSION are unchanged; results without a message_id are not
persisted.

The cross-agent L8 task store (#1521) is not in-tree yet, so the store
is email-local: task_store.record_action_items is the single seam that
becomes the adapter forwarding tasks (source_ref=message_id) when it
lands. MCP stdio triage stays analyze-only for now — persistence is
wired at the REST surface the Agent UI drives.
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Verdict: Approve with suggestions — one edge-path reliability issue worth fixing before merge.

This PR persists triage action items into a local email_tasks SQLite table, linked to the source message and de-duplicated per message so re-triaging never duplicates a task. It's genuinely additive on the wire, cleanly separated behind a single record_action_items seam for the future cross-agent task store, thoroughly documented across every surface (README/SPEC/SKILL/CHANGELOG/openapi/spec HTML/guide), and well tested at both the store and REST layers.

The one thing to address: the new persistence runs after the triage result is computed but its errors aren't isolated. If a task write fails, a successful triage now returns a 500 instead of the result — and for the batch endpoint a single item's write failure collapses the entire per-item-isolated response into one 500, breaking the documented "per-item failures isolate" contract. The most concrete trigger is two overlapping triages of the same message racing on the UNIQUE index. Recommend swallowing that specific constraint hit as the dedup path (it is the invariant firing) and keeping batch persistence per-item isolated.

🔍 Technical details

🟡 Important

Persistence errors aren't isolated from the triage response — breaks batch per-item isolation (api_routes.py:157-168, 1475-1505)

_persist_triage_tasks runs after the response is built, but the routes only catch LLMTriageError / EmailSummarizeError. Any exception from the task write therefore escapes as an unhandled 500:

  • Single /triage: a successful analysis is lost — caller gets a 500 instead of the result the PR says is "byte-for-byte unchanged."
  • Batch /triage/batch: _triage_batch_and_persist loops persisting each item; one write raising throws out of the whole handler → 500, discarding every other item's result. This directly violates the endpoint's documented guarantee that "per-item failures surface as BatchItemResult.error (HTTP 200 …)".

The most realistic trigger is a race, not disk failure: DatabaseMixin opens the connection with check_same_thread=False and no lock (src/gaia/database/mixin.py:63), triage runs in asyncio.to_thread, and record_action_items does read-existing-then-insert. Two overlapping triages of the same message_id both read an empty set and both insert — the second hits idx_email_tasks_msg_desc and raises sqlite3.IntegrityError. The module comment even claims "A UNIQUE index backs the invariant at the schema level," but the insert path never handles the constraint firing, so instead of silently de-duplicating it 500s.

Cleanest targeted fix — treat the UNIQUE hit as the dedup path it's meant to be (needs import sqlite3 at the top of task_store.py):

        existing.add(norm)
        task_id = uuid.uuid4().hex
        try:
            db.insert(
                "email_tasks",
                {
                    "task_id": task_id,
                    "message_id": message_id,
                    "description": item.description,
                    "description_norm": norm,
                    "due_hint": item.due_hint,
                    "item_type": item.type,
                    "url": item.url,
                    "status": "open",
                    "created_at": time.time(),
                    "completed_at": None,
                },
            )
        except sqlite3.IntegrityError:
            # A concurrent triage of the same message already recorded this
            # item — the UNIQUE(message_id, description_norm) index firing IS
            # the dedup invariant, not an error to surface.
            continue
        created.append(task_id)

That handles the race for both endpoints. Separately, the batch loop should keep a single item's persistence failure from sinking the whole response — the batch already isolates per-item triage failures, and the side-effect should match that (e.g. persist per-item under a narrow guard that logs and continues, rather than letting it propagate past _triage_batch_and_persist). This isn't a "silent fallback" — the caller's requested operation (triage) succeeded and should be returned; the task write is a genuine secondary side effect.

🟢 Minor

Strengths

  • Doc sync is exemplary — README, SPEC, SKILL, CHANGELOG, openapi.email.json, spec_html.py, specification.html, and docs/guides/email.mdx all updated together, plus the capability-14 status moved Planned→Wired. Exactly what CLAUDE.md's "update EVERY doc" rule asks for.
  • Clean seam designrecord_action_items as the single write entry point, pure functions over a DatabaseMixin first arg mirroring action_store, so the EPIC: L8 Task & To-Do Store (cross-agent, core) #1521 adapter swap is localized.
  • Solid test coverage — normalized-dedup key, same-item-different-message distinctness, fail-loud on empty message_id, idempotent completion, and the REST surface (persist / re-triage no-dup / batch per-message dedup) via the existing resolve_action_db seam with an in-memory DB.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent::email Email agent changes documentation Documentation changes tests Test changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(email): extract action items into a persistent task list

1 participant