KB-7533

GPT Review — 23-P3D Notification Outbox Design Note rev1

8 min read Revision 1
gpt-reviewpack-23p3dnotificationoutboxread-staterev2-required

GPT Review — 23-P3D Notification Outbox Design Note rev1

Date: 2026-05-07
Reviewer: GPT-5.5 Thinking / Incomex Hội đồng AI
Reviewed: knowledge/dev/laws/dieu44-trien-khai/design/23-p3d-notification-outbox-design-note.md rev1

Verdict

Direction accepted, but revise design note to rev2 before execution prompt.

Opus’s core approach is correct: PG-native event outbox, durable table as source of truth, per-actor read state, triggers close to data mutation, lightweight payload refs, no external queue as primary storage.

However, rev1 needs several operational clarifications before implementation, mainly around stale/resolved notifications, audience/stream control, and read vs review semantics.

Accepted design decisions

  • One iu_notification_event table with event_type is accepted for Phase 1.
  • iu_notification_read(event_id, actor_ref) per-event/per-actor read state is accepted over global read flags and over timestamp watermark.
  • PG triggers are accepted for Phase 1 because they avoid rewriting protected Pack 23 functions.
  • Payload should be refs only, no body.
  • LISTEN/NOTIFY is deferred; table remains source of truth.
  • Birth/version_seq=1 should not create notification events.
  • Stale-base side effects should not generate their own notification event.
  • Self-notify exclusion is accepted as a default UX rule, but needs an override/exception note for reviewer roles.

Required rev2 fixes

P1 — Separate “notification history” from “actionable inbox”

Rev1 has only fn_iu_unread(p_actor) and would return all unread events except self-created events. This can show stale tasks: e.g. a draft_created event may remain unread even after another reviewer has applied or closed the draft.

Add design distinction:

  • iu_notification_event = durable event history;
  • fn_iu_unread(...) / inbox = actionable/current unread view.

For actionable inbox:

  • draft_created should only show if referenced draft is still open;
  • if draft is applied, stale_base, or withdrawn later, it should not appear as actionable review work;
  • comment_added and version_applied may remain informational until read.

This can be implemented in query logic rather than mutating event rows.

P2 — Add event category / stream field

Add a lightweight classification column to support separate lists without multiple tables:

event_stream text NOT NULL -- e.g. 'comment', 'review', 'update'

Or document that event_type maps to streams:

  • comment_addedcomment stream;
  • draft_createdreview stream;
  • version_appliedupdate stream.

User explicitly wants comment/update lists separate. If using one table, the stream mapping must be first-class and indexed.

P3 — Add actor/audience model note

Rev1 effectively broadcasts all non-self events to everyone. This is acceptable as Phase 1 default for GPT/Opus, but it must be documented as a default broadcast model, not the final routing model.

Add Phase 1 rule:

  • events are visible to all reviewers/actors except creator by default;
  • later Phase 2 can add target/audience tables or subscriptions.

Consider adding optional columns now if cheap:

target_role text NULL

or keep payload.target_role. Do not overbuild, but document the future extension.

P4 — Clarify self-notify exclusion

Self-notify exclusion is good for normal creators. But if a reviewer uses the same actor identity to create a draft they later need to review, it could hide work.

Add explicit rule:

  • default fn_iu_unread(p_actor) excludes actor_ref=p_actor;
  • optional parameter later could allow include_self boolean default false;
  • Phase 1 can keep default exclusion but design should not make it impossible to inspect self-events.

Recommended signature:

fn_iu_unread(p_actor text, p_stream text DEFAULT NULL, p_include_self boolean DEFAULT false)

P5 — Mark read function should validate event ids and return details

fn_iu_mark_read should not silently accept arbitrary UUIDs without feedback.

Design return JSON should include:

  • requested_count;
  • existing_count;
  • newly_marked_count;
  • already_marked_count;
  • actor_ref.

Unknown event ids can be ignored but should be counted.

P6 — Indexes need actor unread query support

idx_notif_read_lookup(event_id, actor_ref) is necessary but not enough. For mark/read lookup it is fine, but unread queries join events to reads by actor.

Add or evaluate:

CREATE UNIQUE INDEX uq_iu_notification_read_event_actor ON iu_notification_read(event_id, actor_ref);
CREATE INDEX idx_iu_notification_read_actor_event ON iu_notification_read(actor_ref, event_id);
CREATE INDEX idx_iu_notification_event_stream_created ON iu_notification_event(event_stream, created_at DESC);
CREATE INDEX idx_iu_notification_event_unit_created ON iu_notification_event(unit_id, created_at DESC);

P7 — Trigger actor fields must match actual schema

Rev1 assumes:

  • unit_edit_comment.author_ref;
  • unit_edit_draft.created_by;
  • unit_version.created_by.

Before execution prompt, design must require runtime inspection of these exact columns. If unit_version lacks created_by, event actor for version_applied should come from payload/system comment/draft/applied_by if available, not guessed.

Add preflight requirement: inspect columns for unit_edit_comment, unit_edit_draft, unit_version.

P8 — Avoid duplicate/noisy comment events from system comments

P3C2 apply creates system/apply comments. If comment trigger emits comment_added for every comment, apply may create both:

  • version_applied event;
  • comment_added system comment event.

That may be noisy. Decide:

  • either include all comments, including system comments, because they are traceable;
  • or suppress system/apply comments from comment_added event when comment_kind/author_type indicates system.

GPT recommendation: suppress system/apply comments from comment notification if they are already represented by version_applied, but still keep them in unit_edit_comment audit trail.

P9 — Event source should include trigger name/source

Add optional metadata for debugging:

source text DEFAULT 'pg_trigger'

or in payload:

{"source":"trg_iu_notif_comment"}

This helps trace event creation without making a general activity log.

P10 — Phase split should be execution-oriented

Rev1 proposes P3D-A/B/C/D but says they can be gộp/tách. For reliability, GPT recommends two packs:

  • P3D1 schema + helper functions only: create 2 tables, indexes, fn_iu_unread, fn_iu_mark_read; no triggers yet.
  • P3D2 triggers + event creation tests: add triggers and test comment/draft/apply event flow.

Reason: triggers are behavior-changing. Table/function scaffolding can be verified first.

If Opus still wants one execution pack, it must be transaction-wrapped and include rollback/drop on fail, but two packs is safer.

Directive to Opus

Patch the design note to rev2 with P1–P10.

Path:

knowledge/dev/laws/dieu44-trien-khai/design/23-p3d-notification-outbox-design-note.md

Do not create execution prompt yet. Do not dispatch. Return for GPT/User review.

Hard boundaries

  • No implementation.
  • No PG mutation.
  • No trigger creation.
  • No table DDL.
  • No Pack 23 function changes.
  • No external queue as source of truth.
  • No global read flag.
  • No mixing with general activity log.

Summary

The P3D design is on the right track. Rev2 should distinguish durable event history from actionable inbox, add stream separation, make per-actor read state queryable at scale, handle resolved drafts, and clarify system-comment noise before implementation planning.

Back to Knowledge Hub knowledge/dev/laws/dieu44-trien-khai/reviews/gpt-review-23-p3d-notification-outbox-design-note-rev1-2026-05-07.md