mirror of
https://github.com/tiennm99/DocsGPT.git
synced 2026-10-05 20:14:47 +00:00
* feat: SSE notification system
Adds a per-user SSE pipe (GET /api/events) plus a per-message
chat-stream reconnect endpoint (GET /api/messages/<id>/events).
Backend substrate:
- application/events/ — durable journal (Redis Streams) + live
pub/sub for user-scoped events, with publish_user_event() as
the worker-side entrypoint.
- application/streaming/ — broadcast_channel for pub/sub fanout
and event_replay for the per-message snapshot+tail path.
- application/storage/db/repositories/message_events.py +
alembic 0007 — Postgres journal for chat-stream events.
- application/worker.py — ingest/reingest/remote/connector/
attachment/mcp_oauth tasks publish queued/progress/completed/
failed envelopes alongside their existing status updates.
Frontend client:
- frontend/src/events/ — connect/reconnect, Last-Event-ID cursor,
backoff with jitter. Each tab runs its own connection; no
cross-tab dedup (future work).
- frontend/src/notifications/ — recentEvents ring, cursor
tracking, tool-approval toast.
- frontend/src/upload/uploadSlice.ts — extraReducers for
source.ingest.* and attachment.* events.
Coverage: 132 SSE tests across events substrate, replay, journal,
routes, and worker publishes.
* refactor(attachments): remove polling, SSE-only
frontend/src/components/MessageInput.tsx no longer runs a 2s
setInterval against getTaskStatus for every processing
attachment. The attachment.* SSE reducers in uploadSlice.ts are
now the sole driver of attachment state transitions.
* feat(connector): consume source.ingest.* SSE, remove polling
frontend/src/components/ConnectorTree.tsx now mirrors FileTree's
slice-walking pattern: it watches notifications.recentEvents
for source.ingest.{completed,failed} envelopes matching the
sync's source id, and no longer polls /task_status every 2s.
* refactor(source-ingest): remove polling, SSE-only
frontend/src/upload/Upload.tsx and
frontend/src/components/FileTree.tsx no longer run getTaskStatus
polling fallbacks. The source.ingest.* SSE reducers in
uploadSlice.ts and FileTree's slice walk are now the sole
drivers of upload/reingest state transitions.
* refactor(mcp-oauth): carry authorization_url in SSE, remove polling
application/worker.py::mcp_oauth now publishes
authorization_url on the mcp.oauth.awaiting_redirect envelope.
frontend/src/modals/MCPServerModal.tsx consumes it from SSE
instead of polling /oauth_status/<task_id> every 1s.
The URL is generated inside DocsGPTOAuth.redirect_handler when
the FastMCP client triggers OAuth. The worker now plumbs a
publish callback through tool_config -> MCPTool -> DocsGPTOAuth
so the awaiting_redirect publish fires from inside the handler
at the exact point the URL becomes known. The legacy Redis
mcp_oauth_status setex writes and the GET
/api/mcp_server/oauth_status/<task_id> endpoint are kept as
belt-and-suspenders; nothing in the frontend reads them now.
* feat(source-ingest): plumb limited flag through SSE for token-cap UX
application/worker.py::ingest_worker and remote_worker now publish
``limited: bool`` on the source.ingest.completed envelope.
uploadSlice routes ``payload.limited === true`` to a failed status
with a ``tokenLimitReached`` flag, and UploadToast surfaces the
translated tokenLimit i18n string. No worker code path sets
limited=true today; this is a forward-looking contract so when
token-cap detection lands, the UX is already wired.
* refactor(mcp-oauth): read status from SSE journal, drop polling endpoint
MCPOAuthManager.get_oauth_status now walks the per-user SSE Streams
journal (user:{user_id}:stream) for the latest mcp.oauth.* envelope
matching the task id, returning the status string derived from the
event type suffix and the payload fields. The worker is the single
source of truth — its publish_user_event calls write the same
record the SSE client receives live.
Removed:
- /api/mcp_server/oauth_status/<task_id> route in
application/api/user/tools/mcp.py
- mcp_oauth_status worker function and mcp_oauth_status_task Celery
wrapper
- All mcp_oauth_status:{task_id} Redis setex writes (4 in mcp_oauth,
2 in DocsGPTOAuth.redirect_handler / callback_handler)
- The update_status closure in mcp_oauth that wrote the polling
payload
Tests updated:
- get_oauth_status now takes (task_id, user_id); new coverage walks
a fake xrevrange response for the completed envelope, the no-match
case, and a Redis-down case
- Removed TestMCPOAuthStatus route tests and TestMcpOauthStatusTask
celery-wrapper test
- Removed the two oauth_status methods from the integration runner
mcp_oauth:auth_url/state/code/error Redis keys remain — they are
the OAuth flow's own state (not the dropped polling payload).
* chore(mcp-oauth): delete orphaned getMCPOAuthStatus client
The /api/mcp_server/oauth_status/<task_id> endpoint was removed in
the prior commit; the corresponding userService method and the
MCP_OAUTH_STATUS endpoint constant had no remaining callers in the
frontend, so they're deleted along with it.
* fix(events): drop live publish when journal write fails
application/events/publisher.py returned an envelope to live
pubsub subscribers even when the XADD to the durable journal
failed. The envelope had no ``id`` field, which bypassed the SSE
route's dedup floor and broke ``Last-Event-ID`` semantics for any
reconnecting client.
Best-effort delivery means dropping consistently, not delivering
inconsistent state. Now: if the journal write fails the publisher
returns None and skips the live publish entirely.
* fix(notifications): dedupe sseEventReceived against immediate dupes
Snapshot replay + live tail can both deliver the same id when the
live pubsub frame and the replay XRANGE overlap. The route's own
dedup floor catches the common case, but consumers walking
``recentEvents`` (FileTree, ConnectorTree, MCPServerModal,
ToolApprovalToast) would otherwise act on the same envelope
twice when a duplicate slipped through.
Belt-and-suspenders: short-circuit when the most recent id in
the ring matches the incoming one.
* fix(events): skip replay budget INCR when no snapshot work possible
_allow_replay incremented the per-user counter on every
/api/events GET, including no-op connects from a fresh client
with no cursor against an empty backlog. React StrictMode dev
double-mounts plus a few tabs trivially tripped the default
30-per-60s budget on idle reconnects.
XLEN pre-check: when last_event_id is None and the user stream
is empty, the connect can't do snapshot work — return True
without INCR. Cursor-bearing connects still INCR unconditionally
(probing the cursor's relationship to stream contents would
require a redundant XRANGE).
* fix(streaming): tighten journal contract + recover from seq collisions
Two related fixes to application/streaming/message_journal.py.
1. record_event now rejects non-dict payloads at the gate. The
live path (base.py::_emit) wrapped non-dicts as
{"value": payload}; the replay path in event_replay synthesized
{"type": event_type}. A reconnecting client would receive a
different envelope than the one originally streamed. Now both
paths see byte-identical envelopes because non-dicts can't be
journaled at all. The corresponding event_replay fallback is
replaced with a warn-and-skip for any legacy rows.
2. record_event handles IntegrityError on (message_id, sequence_no)
collisions by reading latest_sequence_no and retrying once with
latest+1. The most likely cause is a stale seq seed on a
continuation retry where the route read MAX(seq) from a
separate connection before another writer committed past it.
Previously the error was swallowed and the event silently
dropped from the journal; now it lands at the next available
seq. The live pubsub publish uses the materialised seq so the
journal row and the live frame agree.
* perf(streaming): batch message_events INSERTs per stream
complete_stream previously opened a fresh db_session() per yielded
event, doing one Postgres INSERT + commit per chunk on the WSGI
thread. Streaming answers emit ~100s of answer chunks per response,
so the route was paying ~100 PG roundtrips per stream serialized on
commit latency.
New BatchedJournalWriter in application/streaming/message_journal.py
accumulates rows per stream and flushes on three triggers:
- size: buffer reaches 16 entries
- time: 100ms elapsed since the last flush
- lifecycle: close() at end-of-stream
Live pubsub publishes still fire synchronously per record(), so
subscribers see events in real time — only the durable journal write
is amortized. On bulk INSERT IntegrityError the writer falls back to
per-row record() with the existing seq+1 retry so a single colliding
seq doesn't drop the rest of the batch.
complete_stream wires journal_writer.close() into every exit path
(happy end, tool-approval-paused end, GeneratorExit, error handler)
so the terminal event is committed before the generator returns —
otherwise a reconnecting client could snapshot up to the last flush
boundary and live-tail waiting for an end that's still in memory.
Repository gets bulk_record() — one SQLAlchemy executemany INSERT
for the bulk path. All-or-nothing on collision (Postgres aborts the
whole batch); the writer's per-row fallback handles recovery.
* chore(upload): drop dead UploadTask.lastEventAt field
The lastEventAt field on UploadTask had no remaining consumers — the
matching Attachment.lastEventAt was cleaned up earlier. Remove the
field declaration and the slice write site.
* chore(frontend): drop orphaned getTaskStatus client
After the polling-removal sweep no caller in frontend/src/ references
userService.getTaskStatus or endpoints.USER.TASK_STATUS. The backend
route /api/task_status itself stays — agents, webhooks, e2e specs,
and the public docs still depend on it.
* docs(repo): remove stale planning docs from repo root
notification-channel-design.md, plan.md, and reminder-tool-design.md
were leftover Claude planning artifacts from the SSE substrate work
that landed accidentally. CLAUDE.md prohibits creating planning docs
unless asked — delete them.
* docs(message-events): clarify repo vs wrapper payload contract
MessageEventsRepository.record accepts any JSONB-compatible value; the
streaming wrapper record_event tightens this to dicts only because the
live and replay paths reconstruct non-dict payloads differently. Spell
the split out so the next reader of the repo method doesn't assume the
wrapper's contract applies here.
* refactor(events): raise on malformed stream id instead of lex fallback
stream_id_compare's lex-fallback branch was a footgun: a malformed id
that sorts lex-greater than a real one would pin live-tail dedup
forever, dropping every subsequent legitimate event silently. Both
current callers in application/api/events/routes.py pre-validate
inputs against _STREAM_ID_RE before calling, so changing the function
to raise ValueError is a no-op on the happy path and turns the future-
caller footgun into a loud failure.
* test(tasks): cover cleanup_message_events task body
Adds skipped-when-no-POSTGRES_URI and happy-path coverage for the
Celery janitor. The skipped path returns the documented short-circuit
shape without touching the repo. The happy path seeds a backdated
row, runs the task against the pg_conn fixture, and asserts the
retention window's row is deleted while in-window rows survive.
Mirrors the TestCleanupPendingToolState pattern.
* fix(notifications): treat /c/new as no current conversation
useMatch('/c/:conversationId') treats the literal URL /c/new as a
real conversation id, so the toast suppression check confused
'user is on /c/new' with 'user is on the conversation needing
approval'. Explicit guard: when the matched id is 'new', fall
through to the no-match case so approval toasts still surface.
* docs(events): enumerate publish_user_event None-return paths
The function returns Optional[str] today, with None conflating five
distinct outcomes (missing args / push disabled / unserialisable /
Redis down / XADD failed). Every current call site is fire-and-
forget and ignores the return, so the right move is to document the
five cases rather than promote to an enum return — keeps the API
small while making the diagnostic surface (logs) obvious. If a
future caller needs to react differently per reason, promote then.
* refactor(sources): move source-id derivation out of worker module
application/api/user/sources/upload.py imported _derive_source_id
from application.worker — pulling the entire Celery worker module
into the API process at import time just for a two-line helper.
Move DOCSGPT_INGEST_NAMESPACE and the derivation function to a
new application/storage/db/source_ids.py module that both layers
can import without that dependency edge. worker.py re-exports the
old names (_derive_source_id, DOCSGPT_INGEST_NAMESPACE) for
backward-compatible imports from tests and any other in-tree
callers; new code should import from the new module directly.
* fix(cache): enable Redis health_check_interval to surface half-open TCP
Without health_check_interval, a half-open TCP socket (NAT silently
dropped state, ELB idle-close) can leave pubsub.get_message hanging
past the SSE generator's keepalive cadence — the kernel never
surfaces the dead socket because no payload is in flight. Setting
health_check_interval=10 makes redis-py ping every 10s when
otherwise idle, so the next get_message after the dead window
raises and the SSE loop falls into its reconnect path instead of
silently freezing on the user.
* chore(events): rename attachment.processing.progress to attachment.progress
The event-type taxonomy was inconsistent: source ingest emits
source.ingest.progress (three segments) while attachments emitted
attachment.processing.progress (four segments). Drops the
.processing. infix for parity. Worker publish sites, the slice
reducer's match, and the worker tests all flip together.
No external consumers — the event type is purely internal between
the publisher and the in-tab slice; safe to rename in one commit.
* feat: events cleanup
* fix: better docs
* fix: e2e tests
697 lines
31 KiB
Python
697 lines
31 KiB
Python
"""Unit tests for ``application/streaming/message_journal.py``.
|
||
|
||
The journal hook is best-effort by contract — its failure modes are
|
||
the most important thing to lock down so a streaming hiccup never
|
||
crashes ``complete_stream``.
|
||
"""
|
||
|
||
from __future__ import annotations
|
||
|
||
import json
|
||
from unittest.mock import MagicMock, patch
|
||
|
||
import pytest
|
||
|
||
from application.streaming.message_journal import (
|
||
BatchedJournalWriter,
|
||
record_event,
|
||
)
|
||
|
||
|
||
@pytest.mark.unit
|
||
class TestRecordEvent:
|
||
def test_invalid_args_return_false(self):
|
||
assert record_event("", 0, "answer") is False
|
||
assert record_event("msg-1", 0, "") is False
|
||
|
||
def test_happy_path_writes_and_publishes(self):
|
||
with patch(
|
||
"application.streaming.message_journal.db_session"
|
||
) as mock_session, patch(
|
||
"application.streaming.message_journal.MessageEventsRepository"
|
||
) as mock_repo_cls, patch(
|
||
"application.streaming.message_journal.Topic"
|
||
) as mock_topic_cls:
|
||
mock_session.return_value.__enter__.return_value = MagicMock()
|
||
mock_repo = MagicMock()
|
||
mock_repo_cls.return_value = mock_repo
|
||
mock_topic = MagicMock()
|
||
mock_topic_cls.return_value = mock_topic
|
||
|
||
result = record_event(
|
||
"msg-1", 5, "answer", {"type": "answer", "answer": "ok"}
|
||
)
|
||
|
||
assert result is True
|
||
mock_repo.record.assert_called_once_with(
|
||
"msg-1", 5, "answer", {"type": "answer", "answer": "ok"}
|
||
)
|
||
mock_topic_cls.assert_called_once_with("channel:msg-1")
|
||
mock_topic.publish.assert_called_once()
|
||
wire = mock_topic.publish.call_args[0][0]
|
||
envelope = json.loads(wire)
|
||
assert envelope["sequence_no"] == 5
|
||
assert envelope["event_type"] == "answer"
|
||
assert envelope["payload"] == {"type": "answer", "answer": "ok"}
|
||
|
||
def test_publish_attempted_even_when_journal_fails(self):
|
||
"""A DB hiccup must not stop the live tail — currently-attached
|
||
subscribers should still receive the live event so their UI is
|
||
live even if a future reconnect's snapshot is missing this row.
|
||
"""
|
||
with patch(
|
||
"application.streaming.message_journal.db_session"
|
||
) as mock_session, patch(
|
||
"application.streaming.message_journal.MessageEventsRepository"
|
||
), patch(
|
||
"application.streaming.message_journal.Topic"
|
||
) as mock_topic_cls:
|
||
mock_session.return_value.__enter__.side_effect = RuntimeError("pg down")
|
||
mock_topic = MagicMock()
|
||
mock_topic_cls.return_value = mock_topic
|
||
|
||
result = record_event(
|
||
"msg-1", 1, "answer", {"type": "answer", "answer": "x"}
|
||
)
|
||
|
||
assert result is False
|
||
mock_topic.publish.assert_called_once()
|
||
|
||
def test_publish_failure_does_not_raise(self):
|
||
with patch(
|
||
"application.streaming.message_journal.db_session"
|
||
) as mock_session, patch(
|
||
"application.streaming.message_journal.MessageEventsRepository"
|
||
) as mock_repo_cls, patch(
|
||
"application.streaming.message_journal.Topic"
|
||
) as mock_topic_cls:
|
||
mock_session.return_value.__enter__.return_value = MagicMock()
|
||
mock_repo_cls.return_value.record = MagicMock()
|
||
mock_topic = MagicMock()
|
||
mock_topic.publish.side_effect = RuntimeError("redis down")
|
||
mock_topic_cls.return_value = mock_topic
|
||
|
||
# Must not raise.
|
||
result = record_event("msg-1", 0, "answer", {"answer": "y"})
|
||
assert result is True # Journal still committed.
|
||
|
||
def test_payload_none_treated_as_empty_dict(self):
|
||
with patch(
|
||
"application.streaming.message_journal.db_session"
|
||
) as mock_session, patch(
|
||
"application.streaming.message_journal.MessageEventsRepository"
|
||
) as mock_repo_cls, patch(
|
||
"application.streaming.message_journal.Topic"
|
||
) as mock_topic_cls:
|
||
mock_session.return_value.__enter__.return_value = MagicMock()
|
||
mock_repo = MagicMock()
|
||
mock_repo_cls.return_value = mock_repo
|
||
mock_topic_cls.return_value = MagicMock()
|
||
|
||
record_event("msg-1", 0, "end", None)
|
||
mock_repo.record.assert_called_once_with("msg-1", 0, "end", {})
|
||
|
||
def test_payload_non_dict_rejected_at_gate(self):
|
||
"""Contract: payload must be a dict (or None). Lists, strings,
|
||
ints, and other shapes are rejected without writing or
|
||
publishing.
|
||
|
||
Background: the live path (``base.py::_emit``) and the replay
|
||
path (``event_replay``) previously reconstructed non-dicts
|
||
differently — ``{"value": payload}`` live vs.
|
||
``{"type": event_type}`` on replay — so a reconnecting client
|
||
would receive a different envelope than the one originally
|
||
streamed. Rejecting at this gate keeps the two paths
|
||
byte-identical.
|
||
"""
|
||
with patch(
|
||
"application.streaming.message_journal.db_session"
|
||
) as mock_session, patch(
|
||
"application.streaming.message_journal.MessageEventsRepository"
|
||
) as mock_repo_cls, patch(
|
||
"application.streaming.message_journal.Topic"
|
||
) as mock_topic_cls:
|
||
mock_session.return_value.__enter__.return_value = MagicMock()
|
||
mock_repo = MagicMock()
|
||
mock_repo_cls.return_value = mock_repo
|
||
mock_topic = MagicMock()
|
||
mock_topic_cls.return_value = mock_topic
|
||
|
||
result = record_event("msg-1", 0, "end", ["unexpected", "list"])
|
||
|
||
assert result is False
|
||
mock_repo.record.assert_not_called()
|
||
mock_topic.publish.assert_not_called()
|
||
|
||
def test_integrity_error_retries_with_seq_plus_one(self):
|
||
"""Composite-PK collision on (message_id, sequence_no) is
|
||
recovered by one retry against ``latest_sequence_no + 1`` —
|
||
the most likely cause is a stale seq seed on a continuation
|
||
retry, where the route read MAX(seq) from a separate
|
||
connection before another writer committed past it.
|
||
|
||
On success the live pubsub publish uses the retried seq so
|
||
the journal row and the live frame agree, even though the
|
||
caller's original POST stream still carries the pre-retry id.
|
||
"""
|
||
from sqlalchemy.exc import IntegrityError
|
||
|
||
# Two repo instances: one per ``with db_session()`` block.
|
||
# repo_first.record raises IntegrityError; the readonly
|
||
# session returns latest=7; repo_retry.record succeeds at 8.
|
||
repo_first = MagicMock(name="repo_first")
|
||
repo_first.record.side_effect = IntegrityError("stmt", {}, Exception())
|
||
repo_readonly = MagicMock(name="repo_readonly")
|
||
repo_readonly.latest_sequence_no.return_value = 7
|
||
repo_retry = MagicMock(name="repo_retry")
|
||
|
||
repo_instances = iter([repo_first, repo_readonly, repo_retry])
|
||
|
||
with patch(
|
||
"application.streaming.message_journal.db_session"
|
||
) as mock_session, patch(
|
||
"application.streaming.message_journal.db_readonly"
|
||
) as mock_readonly, patch(
|
||
"application.streaming.message_journal.MessageEventsRepository",
|
||
side_effect=lambda conn: next(repo_instances),
|
||
), patch(
|
||
"application.streaming.message_journal.Topic"
|
||
) as mock_topic_cls:
|
||
mock_session.return_value.__enter__.return_value = MagicMock()
|
||
mock_readonly.return_value.__enter__.return_value = MagicMock()
|
||
mock_topic = MagicMock()
|
||
mock_topic_cls.return_value = mock_topic
|
||
|
||
result = record_event("msg-1", 3, "answer", {"text": "hi"})
|
||
|
||
assert result is True
|
||
# First INSERT attempted at the caller's seq=3.
|
||
repo_first.record.assert_called_once_with(
|
||
"msg-1", 3, "answer", {"text": "hi"}
|
||
)
|
||
# Latest probed for the retry seed.
|
||
repo_readonly.latest_sequence_no.assert_called_once_with("msg-1")
|
||
# Retry INSERT at latest+1 = 8.
|
||
repo_retry.record.assert_called_once_with(
|
||
"msg-1", 8, "answer", {"text": "hi"}
|
||
)
|
||
# Live publish uses the materialised seq so the wire and
|
||
# the journal row stay in lockstep on the retry path.
|
||
mock_topic.publish.assert_called_once()
|
||
wire = json.loads(mock_topic.publish.call_args[0][0])
|
||
assert wire["sequence_no"] == 8
|
||
|
||
def test_integrity_error_retry_failure_drops_silently(self):
|
||
"""If the retry collides again (truly concurrent writers in
|
||
lockstep) the journal write is dropped but the function still
|
||
returns ``False`` without raising — the streaming loop must
|
||
not be killed by a journal hiccup.
|
||
"""
|
||
from sqlalchemy.exc import IntegrityError
|
||
|
||
repo_first = MagicMock(name="repo_first")
|
||
repo_first.record.side_effect = IntegrityError("stmt", {}, Exception())
|
||
repo_readonly = MagicMock(name="repo_readonly")
|
||
repo_readonly.latest_sequence_no.return_value = 3
|
||
repo_retry = MagicMock(name="repo_retry")
|
||
repo_retry.record.side_effect = IntegrityError("stmt", {}, Exception())
|
||
|
||
repo_instances = iter([repo_first, repo_readonly, repo_retry])
|
||
|
||
with patch(
|
||
"application.streaming.message_journal.db_session"
|
||
) as mock_session, patch(
|
||
"application.streaming.message_journal.db_readonly"
|
||
) as mock_readonly, patch(
|
||
"application.streaming.message_journal.MessageEventsRepository",
|
||
side_effect=lambda conn: next(repo_instances),
|
||
), patch(
|
||
"application.streaming.message_journal.Topic"
|
||
) as mock_topic_cls:
|
||
mock_session.return_value.__enter__.return_value = MagicMock()
|
||
mock_readonly.return_value.__enter__.return_value = MagicMock()
|
||
mock_topic = MagicMock()
|
||
mock_topic_cls.return_value = mock_topic
|
||
|
||
result = record_event("msg-1", 0, "answer", {"text": "hi"})
|
||
|
||
assert result is False
|
||
# Both INSERT attempts fired; the second raised too.
|
||
assert repo_first.record.call_count == 1
|
||
assert repo_retry.record.call_count == 1
|
||
# Live publish still fires on the materialised seq path —
|
||
# subscribers downgrade to keepalives if they were waiting
|
||
# on this event, which is correct since the journal can't
|
||
# serve it on a future reconnect anyway.
|
||
mock_topic.publish.assert_called_once()
|
||
|
||
def test_payload_with_null_byte_stripped_before_insert(self):
|
||
"""``\\x00`` in a string value would crash JSONB INSERT — strip
|
||
at the journal boundary so the row lands and reconnecting
|
||
clients see the chunk.
|
||
"""
|
||
with patch(
|
||
"application.streaming.message_journal.db_session"
|
||
) as mock_session, patch(
|
||
"application.streaming.message_journal.MessageEventsRepository"
|
||
) as mock_repo_cls, patch(
|
||
"application.streaming.message_journal.Topic"
|
||
) as mock_topic_cls:
|
||
mock_session.return_value.__enter__.return_value = MagicMock()
|
||
mock_repo = MagicMock()
|
||
mock_repo_cls.return_value = mock_repo
|
||
mock_topic_cls.return_value = MagicMock()
|
||
|
||
record_event(
|
||
"msg-1", 0, "answer", {"answer": "hel\x00lo", "type": "answer"}
|
||
)
|
||
|
||
mock_repo.record.assert_called_once_with(
|
||
"msg-1", 0, "answer", {"answer": "hello", "type": "answer"}
|
||
)
|
||
|
||
def test_payload_with_nested_null_bytes_stripped_recursively(self):
|
||
"""Strip walks nested dicts and lists — and string keys, to be
|
||
safe against an LLM emitting structured data with NULs.
|
||
"""
|
||
with patch(
|
||
"application.streaming.message_journal.db_session"
|
||
) as mock_session, patch(
|
||
"application.streaming.message_journal.MessageEventsRepository"
|
||
) as mock_repo_cls, patch(
|
||
"application.streaming.message_journal.Topic"
|
||
) as mock_topic_cls:
|
||
mock_session.return_value.__enter__.return_value = MagicMock()
|
||
mock_repo = MagicMock()
|
||
mock_repo_cls.return_value = mock_repo
|
||
mock_topic_cls.return_value = MagicMock()
|
||
|
||
record_event(
|
||
"msg-1",
|
||
0,
|
||
"answer",
|
||
{
|
||
"outer": {"inner\x00key": "a\x00b"},
|
||
"items": ["x\x00y", {"k": "v\x00"}],
|
||
"clean": "ok",
|
||
},
|
||
)
|
||
|
||
mock_repo.record.assert_called_once_with(
|
||
"msg-1",
|
||
0,
|
||
"answer",
|
||
{
|
||
"outer": {"innerkey": "ab"},
|
||
"items": ["xy", {"k": "v"}],
|
||
"clean": "ok",
|
||
},
|
||
)
|
||
|
||
def test_payload_without_null_bytes_unchanged(self):
|
||
"""Common-case: no NUL, no allocation churn — payload passed
|
||
through identically.
|
||
"""
|
||
with patch(
|
||
"application.streaming.message_journal.db_session"
|
||
) as mock_session, patch(
|
||
"application.streaming.message_journal.MessageEventsRepository"
|
||
) as mock_repo_cls, patch(
|
||
"application.streaming.message_journal.Topic"
|
||
) as mock_topic_cls:
|
||
mock_session.return_value.__enter__.return_value = MagicMock()
|
||
mock_repo = MagicMock()
|
||
mock_repo_cls.return_value = mock_repo
|
||
mock_topic_cls.return_value = MagicMock()
|
||
|
||
payload = {"a": "1", "nested": {"b": [1, 2, "three"]}}
|
||
record_event("msg-1", 0, "answer", payload)
|
||
|
||
mock_repo.record.assert_called_once_with(
|
||
"msg-1", 0, "answer", payload
|
||
)
|
||
|
||
|
||
@pytest.mark.unit
|
||
class TestBatchedJournalWriter:
|
||
"""``BatchedJournalWriter`` amortizes per-emit PG commits.
|
||
|
||
Tests cover the four flush triggers (size, time, lifecycle,
|
||
explicit), the bulk→per-row IntegrityError fallback, and the
|
||
live-publish-stays-synchronous invariant.
|
||
"""
|
||
|
||
def _patch_io(self):
|
||
"""Standard patch set: stub PG sessions and Redis pubsub.
|
||
|
||
Returns a tuple ``(session_cm, readonly_cm, repo_factory, topic)``.
|
||
The repo_factory is a list — each ``MessageEventsRepository(...)``
|
||
call inside the writer pops one element off the front. Tests
|
||
push fakes onto it before triggering the path that needs them.
|
||
"""
|
||
from unittest.mock import patch as _patch
|
||
|
||
session_cm = _patch("application.streaming.message_journal.db_session")
|
||
readonly_cm = _patch(
|
||
"application.streaming.message_journal.db_readonly"
|
||
)
|
||
repo_cls = _patch(
|
||
"application.streaming.message_journal.MessageEventsRepository"
|
||
)
|
||
topic_cls = _patch("application.streaming.message_journal.Topic")
|
||
return session_cm, readonly_cm, repo_cls, topic_cls
|
||
|
||
def test_size_trigger_flushes_at_batch_size(self):
|
||
"""Buffer reaches ``batch_size`` → one bulk_record call covers
|
||
all rows; publishes fire after the bulk commit, one per row.
|
||
"""
|
||
session_cm, readonly_cm, repo_cls_p, topic_cls_p = self._patch_io()
|
||
with session_cm as mock_session, readonly_cm, repo_cls_p as mock_repo_cls, topic_cls_p as mock_topic_cls:
|
||
mock_session.return_value.__enter__.return_value = MagicMock()
|
||
mock_repo = MagicMock()
|
||
mock_repo_cls.return_value = mock_repo
|
||
mock_topic = MagicMock()
|
||
mock_topic_cls.return_value = mock_topic
|
||
|
||
writer = BatchedJournalWriter(
|
||
"msg-1", batch_size=3, batch_interval_ms=10_000
|
||
)
|
||
# First two records: buffered, no bulk_record yet, no publish.
|
||
writer.record(0, "answer", {"text": "a"})
|
||
writer.record(1, "answer", {"text": "b"})
|
||
mock_repo.bulk_record.assert_not_called()
|
||
assert mock_topic.publish.call_count == 0
|
||
# Third record: triggers a size-based flush.
|
||
writer.record(2, "answer", {"text": "c"})
|
||
mock_repo.bulk_record.assert_called_once()
|
||
args = mock_repo.bulk_record.call_args
|
||
assert args.args[0] == "msg-1"
|
||
buffered = args.args[1]
|
||
assert len(buffered) == 3
|
||
assert [seq for seq, _, _ in buffered] == [0, 1, 2]
|
||
# Publishes fire after the bulk commit, one per row in order.
|
||
assert mock_topic.publish.call_count == 3
|
||
published_seqs = [
|
||
json.loads(call.args[0])["sequence_no"]
|
||
for call in mock_topic.publish.call_args_list
|
||
]
|
||
assert published_seqs == [0, 1, 2]
|
||
|
||
def test_time_trigger_flushes_after_interval(self):
|
||
"""When the elapsed time since the last flush exceeds
|
||
``batch_interval_ms``, the next ``record()`` flushes — even if
|
||
the buffer is well below ``batch_size``. Drives reconnect
|
||
visibility for slow producers.
|
||
"""
|
||
session_cm, readonly_cm, repo_cls_p, topic_cls_p = self._patch_io()
|
||
with session_cm as mock_session, readonly_cm, repo_cls_p as mock_repo_cls, topic_cls_p as mock_topic_cls, patch(
|
||
"application.streaming.message_journal.time.monotonic"
|
||
) as mock_mono:
|
||
mock_session.return_value.__enter__.return_value = MagicMock()
|
||
mock_repo = MagicMock()
|
||
mock_repo_cls.return_value = mock_repo
|
||
mock_topic_cls.return_value = MagicMock()
|
||
|
||
# First call from __init__ at t=0; subsequent calls drive
|
||
# the time-trigger check inside ``_should_flush``.
|
||
# t=0.000 → __init__ snapshots last_flush
|
||
# t=0.001 → record(0) — 1ms elapsed → no flush
|
||
# t=0.001 → after record, last_flush snapshot when flush called (n/a)
|
||
# t=0.200 → record(1) — 200ms elapsed → flush
|
||
# t=0.200 → flush updates last_flush
|
||
mock_mono.side_effect = iter([0.000, 0.001, 0.200, 0.200])
|
||
|
||
writer = BatchedJournalWriter(
|
||
"msg-1", batch_size=100, batch_interval_ms=100
|
||
)
|
||
writer.record(0, "answer", {"text": "a"})
|
||
mock_repo.bulk_record.assert_not_called()
|
||
writer.record(1, "answer", {"text": "b"})
|
||
mock_repo.bulk_record.assert_called_once()
|
||
|
||
def test_close_drains_remaining_buffer(self):
|
||
"""``close()`` is the lifecycle flush — at end of stream every
|
||
buffered event must commit before the writer goes silent.
|
||
Idempotent so it's safe in multiple finally clauses.
|
||
"""
|
||
session_cm, readonly_cm, repo_cls_p, topic_cls_p = self._patch_io()
|
||
with session_cm as mock_session, readonly_cm, repo_cls_p as mock_repo_cls, topic_cls_p as mock_topic_cls:
|
||
mock_session.return_value.__enter__.return_value = MagicMock()
|
||
mock_repo = MagicMock()
|
||
mock_repo_cls.return_value = mock_repo
|
||
mock_topic_cls.return_value = MagicMock()
|
||
|
||
writer = BatchedJournalWriter(
|
||
"msg-1", batch_size=100, batch_interval_ms=100_000
|
||
)
|
||
writer.record(0, "answer", {"text": "a"})
|
||
writer.record(1, "end", {"type": "end"})
|
||
mock_repo.bulk_record.assert_not_called()
|
||
|
||
writer.close()
|
||
mock_repo.bulk_record.assert_called_once()
|
||
assert len(mock_repo.bulk_record.call_args.args[1]) == 2
|
||
|
||
# Second close is a no-op.
|
||
writer.close()
|
||
assert mock_repo.bulk_record.call_count == 1
|
||
|
||
def test_record_after_close_returns_false(self):
|
||
"""Writing past ``close()`` must not silently land in a stale
|
||
buffer — return False so the caller's flow surfaces the bug.
|
||
"""
|
||
session_cm, readonly_cm, repo_cls_p, topic_cls_p = self._patch_io()
|
||
with session_cm as mock_session, readonly_cm, repo_cls_p as mock_repo_cls, topic_cls_p as mock_topic_cls:
|
||
mock_session.return_value.__enter__.return_value = MagicMock()
|
||
mock_repo_cls.return_value = MagicMock()
|
||
mock_topic_cls.return_value = MagicMock()
|
||
|
||
writer = BatchedJournalWriter("msg-1", batch_size=100)
|
||
writer.close()
|
||
assert writer.record(0, "answer", {"text": "a"}) is False
|
||
|
||
def test_record_rejects_non_dict_payload(self):
|
||
"""Same contract as ``record_event``: non-dict payloads are
|
||
rejected to keep live and replay paths byte-identical.
|
||
"""
|
||
session_cm, readonly_cm, repo_cls_p, topic_cls_p = self._patch_io()
|
||
with session_cm as mock_session, readonly_cm, repo_cls_p as mock_repo_cls, topic_cls_p as mock_topic_cls:
|
||
mock_session.return_value.__enter__.return_value = MagicMock()
|
||
mock_repo_cls.return_value = MagicMock()
|
||
mock_topic = MagicMock()
|
||
mock_topic_cls.return_value = mock_topic
|
||
|
||
writer = BatchedJournalWriter("msg-1")
|
||
assert writer.record(0, "answer", ["bad", "list"]) is False
|
||
# Nothing publishes either — the gate fires before the wire.
|
||
mock_topic.publish.assert_not_called()
|
||
|
||
def test_bulk_collision_falls_back_to_per_row(self):
|
||
"""Bulk INSERT fails with IntegrityError → writer retries each
|
||
row individually via the legacy ``record_event`` retry path,
|
||
so a single colliding seq doesn't drop the whole batch.
|
||
"""
|
||
from sqlalchemy.exc import IntegrityError
|
||
|
||
session_cm, readonly_cm, repo_cls_p, topic_cls_p = self._patch_io()
|
||
with session_cm as mock_session, readonly_cm as mock_readonly, repo_cls_p as mock_repo_cls, topic_cls_p as mock_topic_cls:
|
||
mock_session.return_value.__enter__.return_value = MagicMock()
|
||
mock_readonly.return_value.__enter__.return_value = MagicMock()
|
||
|
||
# First call: bulk_record raises. Subsequent calls: per-row
|
||
# record succeeds. Use a single fake whose first
|
||
# ``bulk_record`` call raises and whose ``record`` succeeds.
|
||
bulk_repo = MagicMock(name="bulk_repo")
|
||
bulk_repo.bulk_record.side_effect = IntegrityError(
|
||
"stmt", {}, Exception()
|
||
)
|
||
per_row_repo = MagicMock(name="per_row_repo")
|
||
# MessageEventsRepository(conn) is called once per session
|
||
# opened. Bulk path = 1; per-row fallback = 2 rows × 1 each.
|
||
mock_repo_cls.side_effect = [bulk_repo, per_row_repo, per_row_repo]
|
||
|
||
mock_topic_cls.return_value = MagicMock()
|
||
|
||
writer = BatchedJournalWriter("msg-1", batch_size=2)
|
||
writer.record(0, "answer", {"text": "a"})
|
||
writer.record(1, "answer", {"text": "b"})
|
||
|
||
bulk_repo.bulk_record.assert_called_once()
|
||
# Per-row fallback wrote each row in its own session.
|
||
assert per_row_repo.record.call_count == 2
|
||
assert per_row_repo.record.call_args_list[0].args[1] == 0
|
||
assert per_row_repo.record.call_args_list[1].args[1] == 1
|
||
|
||
def test_flush_clears_buffer_even_on_total_failure(self):
|
||
"""A flush that fails with a non-IntegrityError exception must
|
||
still clear the buffer — leaving rows would grow memory
|
||
unbounded across the remainder of the stream. Degraded UX
|
||
(missing snapshot rows) beats runaway memory.
|
||
"""
|
||
session_cm, readonly_cm, repo_cls_p, topic_cls_p = self._patch_io()
|
||
with session_cm as mock_session, readonly_cm, repo_cls_p as mock_repo_cls, topic_cls_p as mock_topic_cls:
|
||
mock_session.return_value.__enter__.return_value = MagicMock()
|
||
mock_repo = MagicMock()
|
||
mock_repo.bulk_record.side_effect = RuntimeError("PG gone")
|
||
mock_repo_cls.return_value = mock_repo
|
||
mock_topic_cls.return_value = MagicMock()
|
||
|
||
writer = BatchedJournalWriter("msg-1", batch_size=2)
|
||
writer.record(0, "answer", {"text": "a"})
|
||
writer.record(1, "answer", {"text": "b"})
|
||
# Bulk failed, but the buffer is cleared so a subsequent
|
||
# close() is a no-op and memory stays bounded.
|
||
assert writer._buffer == []
|
||
writer.close()
|
||
# Only the one flush we forced — no double-attempt on
|
||
# close after the buffer was already drained.
|
||
assert mock_repo.bulk_record.call_count == 1
|
||
|
||
def test_record_does_not_publish_synchronously(self):
|
||
"""Regression guard: ``record()`` must not publish before the
|
||
journal INSERT commits. Buffering an event with no flush yet
|
||
means zero pubsub frames on the wire — otherwise a reconnect
|
||
snapshot could miss a row that subscribers already received.
|
||
"""
|
||
session_cm, readonly_cm, repo_cls_p, topic_cls_p = self._patch_io()
|
||
with session_cm as mock_session, readonly_cm, repo_cls_p as mock_repo_cls, topic_cls_p as mock_topic_cls:
|
||
mock_session.return_value.__enter__.return_value = MagicMock()
|
||
mock_repo_cls.return_value = MagicMock()
|
||
mock_topic = MagicMock()
|
||
mock_topic_cls.return_value = mock_topic
|
||
|
||
writer = BatchedJournalWriter("msg-1", batch_size=100)
|
||
for i in range(5):
|
||
writer.record(i, "answer", {"text": str(i)})
|
||
mock_topic.publish.assert_not_called()
|
||
|
||
def test_flush_publishes_each_buffered_event_after_bulk_insert_in_order(
|
||
self,
|
||
):
|
||
"""Happy path: flush issues one bulk INSERT, then publishes each
|
||
buffered frame in the order it was recorded.
|
||
"""
|
||
session_cm, readonly_cm, repo_cls_p, topic_cls_p = self._patch_io()
|
||
with session_cm as mock_session, readonly_cm, repo_cls_p as mock_repo_cls, topic_cls_p as mock_topic_cls:
|
||
mock_session.return_value.__enter__.return_value = MagicMock()
|
||
mock_repo = MagicMock()
|
||
mock_repo_cls.return_value = mock_repo
|
||
mock_topic = MagicMock()
|
||
mock_topic_cls.return_value = mock_topic
|
||
|
||
writer = BatchedJournalWriter(
|
||
"msg-1", batch_size=100, batch_interval_ms=100_000
|
||
)
|
||
for i in range(4):
|
||
writer.record(i, "answer", {"text": str(i)})
|
||
mock_topic.publish.assert_not_called()
|
||
|
||
writer.flush()
|
||
mock_repo.bulk_record.assert_called_once()
|
||
assert mock_topic.publish.call_count == 4
|
||
published_seqs = [
|
||
json.loads(call.args[0])["sequence_no"]
|
||
for call in mock_topic.publish.call_args_list
|
||
]
|
||
assert published_seqs == [0, 1, 2, 3]
|
||
|
||
def test_flush_bulk_exception_drops_both_row_and_publish(self):
|
||
"""Non-IntegrityError from bulk_record → both the journal row
|
||
and the live publish are dropped together. Matches the
|
||
publish_user_event "drop live publish when durable write fails"
|
||
contract.
|
||
"""
|
||
session_cm, readonly_cm, repo_cls_p, topic_cls_p = self._patch_io()
|
||
with session_cm as mock_session, readonly_cm, repo_cls_p as mock_repo_cls, topic_cls_p as mock_topic_cls:
|
||
mock_session.return_value.__enter__.return_value = MagicMock()
|
||
mock_repo = MagicMock()
|
||
mock_repo.bulk_record.side_effect = RuntimeError("PG gone")
|
||
mock_repo_cls.return_value = mock_repo
|
||
mock_topic = MagicMock()
|
||
mock_topic_cls.return_value = mock_topic
|
||
|
||
writer = BatchedJournalWriter("msg-1", batch_size=2)
|
||
writer.record(0, "answer", {"text": "a"})
|
||
writer.record(1, "answer", {"text": "b"})
|
||
|
||
mock_repo.bulk_record.assert_called_once()
|
||
mock_topic.publish.assert_not_called()
|
||
|
||
def test_flush_per_row_publishes_on_commit_skips_on_persistent_drop(self):
|
||
"""IntegrityError-after-retry path: per-row fallback publishes
|
||
each row that lands in PG (including those rewritten at
|
||
latest+1), and skips publish for any row dropped after the
|
||
retry collision.
|
||
"""
|
||
from sqlalchemy.exc import IntegrityError
|
||
|
||
session_cm, readonly_cm, repo_cls_p, topic_cls_p = self._patch_io()
|
||
with session_cm as mock_session, readonly_cm as mock_readonly, repo_cls_p as mock_repo_cls, topic_cls_p as mock_topic_cls:
|
||
mock_session.return_value.__enter__.return_value = MagicMock()
|
||
mock_readonly.return_value.__enter__.return_value = MagicMock()
|
||
|
||
# Bulk attempt collides. Per-row: row 0 commits at its seq;
|
||
# row 1 collides, readonly probe returns latest=9, retry at
|
||
# 10 collides too → dropped without publish.
|
||
bulk_repo = MagicMock(name="bulk_repo")
|
||
bulk_repo.bulk_record.side_effect = IntegrityError(
|
||
"stmt", {}, Exception()
|
||
)
|
||
row0_repo = MagicMock(name="row0_repo")
|
||
row1_first_repo = MagicMock(name="row1_first_repo")
|
||
row1_first_repo.record.side_effect = IntegrityError(
|
||
"stmt", {}, Exception()
|
||
)
|
||
row1_readonly_repo = MagicMock(name="row1_readonly_repo")
|
||
row1_readonly_repo.latest_sequence_no.return_value = 9
|
||
row1_retry_repo = MagicMock(name="row1_retry_repo")
|
||
row1_retry_repo.record.side_effect = IntegrityError(
|
||
"stmt", {}, Exception()
|
||
)
|
||
mock_repo_cls.side_effect = [
|
||
bulk_repo,
|
||
row0_repo,
|
||
row1_first_repo,
|
||
row1_readonly_repo,
|
||
row1_retry_repo,
|
||
]
|
||
|
||
mock_topic = MagicMock()
|
||
mock_topic_cls.return_value = mock_topic
|
||
|
||
writer = BatchedJournalWriter("msg-1", batch_size=2)
|
||
writer.record(0, "answer", {"text": "a"})
|
||
writer.record(1, "answer", {"text": "b"})
|
||
|
||
# Only row 0 publishes — row 1 was dropped after the retry
|
||
# collision, so no live frame reaches the wire for it.
|
||
assert mock_topic.publish.call_count == 1
|
||
wire = json.loads(mock_topic.publish.call_args[0][0])
|
||
assert wire["sequence_no"] == 0
|
||
assert wire["payload"] == {"text": "a"}
|
||
|
||
def test_record_strips_null_bytes_before_buffer(self):
|
||
"""Null-byte sanitization applies at the writer boundary too —
|
||
otherwise a NUL-bearing chunk would land in the buffer, the
|
||
bulk INSERT would fail with ``DataError``, and the per-row
|
||
fallback would re-attempt the same broken row. Strip at the
|
||
gate so the buffered tuple is JSONB-safe.
|
||
"""
|
||
session_cm, readonly_cm, repo_cls_p, topic_cls_p = self._patch_io()
|
||
with session_cm as mock_session, readonly_cm, repo_cls_p as mock_repo_cls, topic_cls_p as mock_topic_cls:
|
||
mock_session.return_value.__enter__.return_value = MagicMock()
|
||
mock_repo = MagicMock()
|
||
mock_repo_cls.return_value = mock_repo
|
||
mock_topic_cls.return_value = MagicMock()
|
||
|
||
writer = BatchedJournalWriter("msg-1", batch_size=2)
|
||
writer.record(0, "answer", {"answer": "hel\x00lo"})
|
||
writer.record(
|
||
1, "answer", {"nested": {"k\x00": ["a\x00b", "ok"]}}
|
||
)
|
||
|
||
mock_repo.bulk_record.assert_called_once()
|
||
buffered = mock_repo.bulk_record.call_args.args[1]
|
||
assert buffered[0][2] == {"answer": "hello"}
|
||
assert buffered[1][2] == {"nested": {"k": ["ab", "ok"]}}
|