Skip to content

B1: persistence primitives — reserveMessage + updateContent pipeline ops#32225

Merged
dvargasfuertes merged 4 commits into
mainfrom
apollo/persistence-primitives-b1
May 27, 2026
Merged

B1: persistence primitives — reserveMessage + updateContent pipeline ops#32225
dvargasfuertes merged 4 commits into
mainfrom
apollo/persistence-primitives-b1

Conversation

@vellum-apollo-bot
Copy link
Copy Markdown
Contributor

What

First PR in the backend-anchor workstream (B1 of B1→B5). Adds two persistence primitives — reserveMessage and an updateContent pipeline op — with no production callers. Future PRs will adopt them in the agent loop.

Why

We're moving the agent loop toward pre-allocating the assistant message id at LLM-call start, so every SSE event (deltas, tool_use, tool_result, message_complete) can be stamped with a stable, server-minted messageId from the first byte. That lets us delete the frontend's content-match reconcile path entirely and avoid an optimistic→server id swap dance.

To get there without a 1000-file PR, we ship the building blocks first.

What's in this PR

Memory layer (assistant/src/memory/conversation-crud.ts)

  • New: reserveMessage(conversationId, role, metadata?) — pre-allocates a row with content: "[]", returns the row. Mirrors addMessage's SQLITE_BUSY/SQLITE_IOERR retry shape, but intentionally skips Qdrant indexing and attention projection (an empty placeholder isn't meaningful for either).
  • updateMessageContent was already there; no change.

Pipeline (assistant/src/plugins/types.ts, assistant/src/plugins/defaults/persistence.ts)

  • Extends the discriminated PersistArgs / PersistResult unions:
    • reservereserveMessage
    • updateContentupdateMessageContent
  • Both flow through the default-persistence pipeline terminal, so plugin middleware can observe, redirect, or short-circuit them.

Tests (assistant/src/__tests__/persistence-pipeline.test.ts)

  • New test: reserve op produces a row with content: "[]" and the supplied metadata.
  • New test: updateContent op overwrites a previously-reserved row, with direct-call parity against updateMessageContent.
  • Existing redirect test now exercises the two new variants on the in-memory mock store.

What's NOT in this PR

  • Any production caller — agent loop, conversation-routes, etc. are untouched.
  • The new SSE event type (assistant_turn_start) and messageId? wire field — that's B2.
  • Agent-loop adoption — B3.

Verification

cd assistant
bunx tsc --noEmit            # clean
bunx eslint <touched files>  # clean
bun test src/__tests__/persistence-pipeline.test.ts
# 7 pass, 0 fail, 41 expect() calls

Plan reference

Backend queue (B1 → B5):

  1. B1 — Persistence primitives (this PR)
  2. B2 — Wire protocol additions: assistant_turn_start event + messageId? on streaming events. Pure additive, no emit sites.
  3. B3 — Agent loop adopts pre-allocation: reserveMessage at every LLM call start, stamp every SSE event with the anchor id.
  4. B4 — Bump self-hosted daemon floor so frontend can hard-require B3.
  5. B5 — Frontend reconcile rewrite consuming the new anchor identity end-to-end.

…llocation

Writes an empty placeholder row (content: "[]") at a known id so the
agent loop can stamp streaming events with stable identity before any
content is produced. Mirrors addMessage's retry shape but skips Qdrant
indexing + attention projection — an empty placeholder is not meaningful
for either. The caller later writes final content via updateMessageContent.

No callers yet; this is B1 of the assistant-anchor backend workstream
(persistence primitives, no consumers).
Extends the discriminated PersistArgs/PersistResult unions with two
new ops:

- reserve       → reserveMessage (PR 1 of this workstream)
- updateContent → updateMessageContent

Both flow through the default-persistence pipeline terminal so plugin
middleware can observe, redirect, or short-circuit them the same way
they already can with add/update/delete.

No production call sites use the new ops yet; the agent-loop migration
to anchor pre-allocation lands in a follow-up PR. Updated the
in-memory mock store in persistence-pipeline.test.ts to cover the new
variants so the existing redirect test still type-checks.
Adds two focused tests against defaultPersistenceTerminal:
- reserve pre-allocates a row with content "[]" and the supplied
  metadata
- updateContent overwrites a previously reserved row in place and
  matches a direct updateMessageContent call

Extends the existing redirect short-circuit test so its mock store
exercises both new variants too.
…k.module sites

Bun's `mock.module(...)` fully replaces a module's exports, so any test
that mocks `memory/conversation-crud.js` and transitively loads the
new persistence default plugin (which now imports `reserveMessage`)
crashes at import-time with:

  SyntaxError: Export named 'reserveMessage' not found in module

CI caught `background-workers-disk-pressure.test.ts` on this PR
because its fixture chain loads the persistence plugin. The other 80
mock.module sites don't trigger today but will the moment any of them
gain a transitive import that reaches the persistence pipeline — so
we patch them all up-front, the same way PR #27459 taught us to handle
`resolveQdrantUrl`.

Stub shape mirrors the other primitives (`mock(async () => ({ id: ... }))`)
so callers that `await reserveMessage(...)` see a Promise-shaped return.
@vellum-apollo-bot
Copy link
Copy Markdown
Contributor Author

Fixes the Test check failure on this PR.

Root cause. mock.module("../memory/conversation-crud.js", () => ({...})) replaces the module wholesale. background-workers-disk-pressure.test.ts transitively imports the new defaults/persistence.ts, which imports reserveMessage from conversation-crud. The mock factory didn't declare that symbol, so Bun raised SyntaxError: Export named 'reserveMessage' not found in module between tests.

Fix. Added a reserveMessage stub to every conversation-crud mock.module site (81 files). Only one of them was failing in CI today, but every other site is one transitive import away from the same crash — same pattern as PR #27459 with resolveQdrantUrl.

Discovered + patched via skills/software-engineering/scripts/mock-module-audit.ts, the workflow encoded in references/unit-testing.md rule #3.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant