GPT Review — 23-P3D Notification Outbox Design Note rev1
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.mdrev1
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_eventtable withevent_typeis 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_createdshould only show if referenced draft is stillopen;- if draft is
applied,stale_base, or withdrawn later, it should not appear as actionable review work; comment_addedandversion_appliedmay 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_added→commentstream;draft_created→reviewstream;version_applied→updatestream.
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)excludesactor_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_appliedevent;comment_addedsystem 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_addedevent whencomment_kind/author_typeindicates 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.