Skip to content

Notifications: fix list/count divergence, typed JSONB payloads, live unread SSE - #229

Merged
martsokha merged 6 commits into
mainfrom
fix/notifications-list-count-agree
Aug 13, 2026
Merged

Notifications: fix list/count divergence, typed JSONB payloads, live unread SSE#229
martsokha merged 6 commits into
mainfrom
fix/notifications-list-count-agree

Conversation

@martsokha

Copy link
Copy Markdown
Member

Summary

Fixes the reported backend bug where GET /notifications/ returned items: [] while GET /notifications/unread/ reported a count > 0 — the list silently dropped rows whose stored params didn't decode, while the count counted them. Along the way the notification/activity payload storage is made typed and fail-closed, event names are normalized, and a live unread-count SSE channel is added.

The core fix (list/count divergence)

from_model returned Option and the list dropped the Nones, so a row whose stored params didn't match its type vanished from the list but still counted toward unread. Now the mapping is total: an undecodable row always appears, only its payload is absent — so the list can never silently disagree with the count over the same rows.

Typed JSONB payloads

  • Json<T> — a typed JSONB column wrapper with explicit per-call read policies (strict / or_default / optional), so a column reads as Json<NotificationPayload> rather than a bare Value.
  • NotificationPayload / ActivityPayload — internally-tagged enums (notifyType / activityType) with one *Params struct per variant.
  • Notification.payload / Activity.payload are Option<P>, omitted when the stored blob doesn't decode. The generated OpenAPI schema is exactly P (optional) — fully typed, no unknown. (An earlier JsonBody<P> = P | Value produced anyOf: [P, <any>], which collapsed to unknown for openapi-typescript consumers; that's now gone.)

Dotted event names

Every event/enum string moves from the mixed colon+dot form (member:invited, connection:sync.completed) to all-dots (member.invited, connection.sync.completed), matching Stripe/GitHub/CloudEvents and making the names valid NATS subjects directly. WebhookEvent::as_subject collapses to self.into() via per-variant strum(serialize).

Live unread-count SSE channel

New GET /notifications/unread/events/ streams the account's unread count over SSE so a badge updates live instead of polling. Modeled on the existing run-status stream: subscribe to the account's core-NATS unread subject before reading the current count, emit it immediately, forward each change, with a 30s DB-reread fallback that self-heals a dropped best-effort broadcast. The emitter broadcasts a fresh count on every insert and the mark-read handlers broadcast the decrement, so the badge tracks both directions. Stored rows stay authoritative; broadcasts are best-effort.

Refactors folded in

  • Avatar extractor/response — the avatar upload/serve free fns become one Avatar(Vec<u8>) newtype implementing FromRequest + IntoResponse (+ OpenAPI), moved to crate::extract.
  • SseResponse<E> — now holds a boxed, type-erased event stream; new takes impl Stream<Item = Event> and adds the Ok/keep-alive framing internally, so both SSE handlers return a plain Result<SseResponse<E>> instead of leaking impl Stream<Item = Result<Event, Infallible>>.
  • Fully-qualified the schema-gated schemars::JsonSchema derive across nvisy-postgres.
  • Renamed handler/utility/sse.rssse_response.rs.

Testing

Full gate green: cargo check, cargo +nightly fmt --check, cargo clippy -D warnings, cargo test, RUSTDOCFLAGS="-D warnings" cargo doc — all pass. DB reset + make generate-migrations applied cleanly with the dotted enum labels.

🤖 Generated with Claude Code

martsokha and others added 6 commits August 12, 2026 23:38
…st/count fix

Fix the core bug: GET /notifications/ silently dropped rows whose stored params
did not deserialize, while the unread-count query counted them — so the list
could report empty while the count was >0. Reads are now total: a row that does
not match its type is surfaced as a raw fallback, never dropped, so list and
count always agree over the same rows.

Data layer — one home for JSONB value types (crates/nvisy-postgres/src/types/json/):
- `TypedJson<P>`: a `JSONB` column wrapper parameterized by its payload type,
  owning the symmetric round-trip — `encode(&P)` stores the self-describing
  tagged object (tag kept in the body), `decode() -> TypedBody<P>`.
- `TypedBody<P>` = `Known(P) | Unknown(Value)`: fail-closed decode result.
- Notification + activity payload enums (one `*Params` per variant) live here,
  beside the workspace settings / retention value types (moved from
  types/settings/). The tag stays a separate indexed column too (for SQL
  filtering/counts) and is derived from the same event on write.

Notifications:
- `is_read` column dropped — `read_at IS NULL` is the single source of truth
  (model helpers, queries, DTO, migration).
- List handler maps every row (from_cursor_page), no silent filtering.

Activities: moved to the type + json-payload model like notifications:
- Dropped server-rendered `description`; `metadata` -> `params` (`TypedJson`).
- `ActivityType` reshaped to the full audit catalog (added pipeline:*/policy:*,
  aligned member:added and connection:sync.completed/failed, dropped custom and
  workspace:exported/imported); typed `ActivityPayload` per variant.

Migrations edited in place; schema regenerated. Full gate green
(check / fmt / clippy / doc / test).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…pe per column

Generalize the typed-payload work into a reusable JSONB layer and apply it to
every column with a known shape, keeping free-form columns as raw Value.

Core (crates/nvisy-postgres/src/types/json/):
- `Json<T>`: a `JSONB` column wrapper parameterized by its value type. One write
  path (`encode`); reads pick a per-call policy — `strict()` (error on
  mismatch), `or_default()` (repair to T::default for config), `typed()` (a
  fail-closed `JsonBody<T>` that never drops a row).
- `JsonBody<T>` = `Known(T) | Unknown(Value)`: the fail-closed decode result.
- One value type per column, each in its own file: notification/activity payload
  enums, workspace/pipeline/run metadata, webhook headers, workspace settings,
  retention. `RetentionOverride` sits with `PipelineMetadata`.

Columns typed:
- account_notifications.params -> Json<NotificationPayload> (read via typed())
- workspace_activities.params  -> Json<ActivityPayload>    (read via typed())
- workspaces.settings          -> Json<WorkspaceSettings>  (read via or_default())
- workspaces.metadata          -> Json<WorkspaceMetadata>
- workspace_pipelines.metadata -> Json<PipelineMetadata>   (retention override is
  now a typed field, replacing the PIPELINE_RETENTION_KEY string access)
- workspace_pipeline_runs.metadata -> Json<RunMetadata { tags, error }>
- workspace_webhooks.headers   -> Json<WebhookHeaders>, a validating newtype
  (rejects malformed header names/values at the request boundary; delivery still
  filters reserved names). Request/response DTOs use BTreeMap for deterministic
  order.

`metadata` columns that are genuinely user-free-form (file.metadata) stay
serde_json::Value by design.

Schema unchanged (columns remain JSONB); only Rust types changed. Full gate
green (check / fmt / clippy / doc / test).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Collapse the avatar request/response free fns into a single `Avatar(Vec<u8>)`
newtype that implements both `FromRequest` (reads the first file field of a
multipart upload) and `IntoResponse` (serves WebP with an immutable cache
header), plus `OperationInput`/`OperationOutput` so both directions appear in
the generated OpenAPI. Handlers extract `Avatar(bytes)` on upload and return
`Avatar(bytes)` on serve. Move it to `crate::extract` alongside the other
extractors.

Rename `handler/utility/sse.rs` to `sse_response.rs` to match its
`SseResponse` contents.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Switch every event/enum string from the mixed colon+dot form
(member:invited, connection:sync.completed) to all-dots
(member.invited, connection.sync.completed), matching the Stripe/GitHub/
CloudEvents convention and making the names valid NATS subjects directly.
Add per-variant strum(serialize) + IntoStaticStr on WebhookEvent so
as_subject collapses to self.into() instead of a 22-arm match. Migration
enum labels and the workspaces notification_events_app default array move to
dots in lockstep.

Fully-qualify the schema-gated derive as
cfg_attr(feature = "schema", derive(schemars::JsonSchema)) across
nvisy-postgres and drop the paired cfg-gated `use schemars::JsonSchema;`
imports.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The response payload was a JsonBody<P> (an untagged Known(P) | Unknown(Value)
enum). Its generated JSON Schema was anyOf: [P, <any>], and the unconstrained
Value arm collapses the whole type to `unknown` for openapi-typescript
consumers — degrading Notification.payload / Activity.payload back to untyped
and undoing the payload typing.

The DTOs only needed to keep the row when a stored blob is undecodable, not to
expose the raw blob. Replace Json::typed() -> JsonBody<T> with
Json::optional() -> Option<T>, make payload an Option<P> that is omitted when
the stored params do not decode, and delete JsonBody entirely (its only users
were these two DTOs). The schema is now exactly P (optional) — typed, and
honest: the wire matches it. An undecodable row still appears (payload absent),
so a list can never silently disagree with its count.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add GET /notifications/unread/events/ — a Server-Sent Events stream of the
authenticated account's unread count, so a badge updates live instead of
polling. Modeled on the run-status stream: subscribe to the account's
core-NATS unread subject before reading the current count (broadcasts are not
replayed), emit the current count immediately, then forward each change with a
30s DB-reread fallback that self-heals a dropped best-effort broadcast. Stays
open until the client disconnects.

NotificationEmitter recomputes and broadcasts the count on every insert
(notify_account_direct / notify_account / notify_workspace_roles, per
recipient), and the mark-read handlers broadcast the decremented count, so the
badge tracks both directions. Broadcasts are best-effort — the stored rows stay
authoritative.

Also collapse the SseResponse signature: it now holds a boxed, type-erased
event stream and its new() takes `impl Stream<Item = Event>`, wrapping each in
Ok and applying keep-alive internally. Both SSE handlers drop the
`impl Stream<Item = Result<Event, Infallible>>` generic and the Ok/keep-alive
boilerplate, returning a plain Result<SseResponse<E>>.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@martsokha martsokha added bug something isn't working as intended feat request for or implementation of a new feature server API handlers, middleware, auth postgres ORM, models, queries, migrations refactor code restructuring without behavior change labels Aug 13, 2026
@martsokha martsokha self-assigned this Aug 13, 2026
@martsokha
martsokha merged commit 3ca6f17 into main Aug 13, 2026
8 checks passed
@martsokha
martsokha deleted the fix/notifications-list-count-agree branch August 13, 2026 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug something isn't working as intended feat request for or implementation of a new feature postgres ORM, models, queries, migrations refactor code restructuring without behavior change server API handlers, middleware, auth

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant