GPT Review — 23-P3D2 Notification Triggers Prompt rev4

Date: 2026-05-08
Reviewer: GPT-5.5 Thinking / Incomex Hội đồng AI
Reviewed: knowledge/dev/laws/dieu44-trien-khai/prompts/23-p3d2-notification-triggers-prompt.md rev4

Verdict

Rev4 is close, but do not dispatch yet. Rev5 required.

Opus resolved the major structural blockers: Variant B is now complete, author_type is required, the board reports read_status_source, and implicit self-read is encoded. However, two runtime blockers remain and several test assertions need tightening before trigger deployment.

Accepted rev4 improvements

• Variant B is now full SQL; no “copy from Variant A” remains.
• unit_edit_comment.author_type is required and suppression uses comment_kind='system' OR author_type='system'.
• uq_notif_read_event_actor is checked.
• Board includes read_status_source.
• T16 verifies explicit_read, implicit_self, and unread statuses.
• Exact regprocedure signatures are checked.
• T14 requires event-table insert and forbids unsafe writes.
• Cleanup is null-safe.
• Final report includes actor variant, comment suppression mode, self-read policy, and board read-status source.

Required rev5 fixes

P1 — BLOCKER: trigger functions reference NEW.id, but source tables may not use id

Rev4 preflight validates many columns but does not validate id columns on:

• unit_edit_comment.id;
• unit_edit_draft.id;
• unit_version.id.

Trigger bodies use NEW.id for ref_id in all three trigger functions. Add these to exact required-column preflight. If missing, STOP.

P2 — BLOCKER: T5 assumes unit_edit_comment.id='$T5_REF' but T5_REF may be empty

If comment notification fails, T5_REF is empty and the SQL:

SELECT count(*) FROM unit_edit_comment WHERE id='$T5_REF';

can error or behave unpredictably depending UUID casting.

Patch T5:

• require T5_REF non-empty before querying comment table;
• if empty, T5 fails cleanly and does not run UUID comparison query;
• if non-empty, verify it exists.

Same pattern should apply to LAST_REF in T13 if empty.

P3 — T11 mark-read should not pass NULL array to fn_iu_mark_read

If fn_iu_unread('gpt') returns zero, array_agg returns NULL and fn_iu_mark_read(NULL,'gpt') should return invalid input or error. T11 already requires T11_GPT>=1, but it calls mark_read before asserting.

Patch:

• first compute T11_GPT; if <1, fail T11 and skip mark_read call;
• only call mark_read when there is at least one unread event.

P4 — T12 self-exclusion is logically ambiguous after T11 mark-read

T12 currently only checks T12A < T12B for actor agent:opus. Because Opus is the actor on normal comment/draft events, include_self=true should show self events, but by that point some events may be applied/hidden or read states may affect counts.

Patch T12 to use board/read_status_source instead of count comparison:

• query fn_iu_notification_board('agent:opus') for events with actor=agent:opus and address=TEST_ADDR;
• require at least one row with read_status_source='implicit_self' and unread_for_actor=false.

This directly tests the User requirement and avoids count ambiguity.

P5 — T16 should verify latest_readers contains GPT

Rev4 dropped the earlier latest_readers assertion. Add it back:

• for at least one marked event in board for GPT, latest_readers contains actor gpt;
• read_status_source='explicit_read' and unread_for_actor=false for that event.

P6 — Board function unread_for_actor should respect actionable filter language

Current board filters out non-actionable draft events entirely, same as unread. That is acceptable, but final report and context should clarify:

• board shows actionable/current event board, not full historical board;
• draft_created events for applied/stale drafts are hidden from board because of actionable filter;
• durable history remains in iu_notification_event until retention/cleanup.

Add report field:

board_scope=actionable_current_events

P7 — T13 duplicate insert uses actor_ref='test', which violates non-empty but may pass; verify duplicate did not change by event_type/ref_id specifically

Rev4 checks total event count before/after. Better:

• check count for the specific (event_type='comment_added', ref_id=LAST_REF) remains exactly 1;
• if LAST_REF empty, skip T13 as FAIL.

P8 — Cleanup deletes all test address events on PASS, so notification_runtime=ACTIVE but test event evidence disappears

That is okay, but final report should capture event counts before cleanup:

• test_event_count_before_cleanup;
• counts by event_type before cleanup;
• cleanup_removed_event_count.

Otherwise report evidence after cleanup can appear empty/confusing.

P9 — Failure cleanup should not run after PASS cleanup failure before preserving report evidence

If cleanup fails, current code enters fail cleanup and drops all triggers/functions, making debugging harder. That is acceptable for safety, but report should clearly show:

• cleanup_pass_status=FAIL;
• fail_cleanup_due_to=TEST_EVENT_CLEANUP_FAILED.

Add variables so report is explicit.

P10 — Context update next pack should mention notification commands and board semantics

Rev4 has next_required_pack=P3D3_CONTEXT_UPDATE_FOR_NOTIFICATION_COMMANDS, good. Add report line:

p3d3_context_must_document=fn_iu_unread,fn_iu_mark_read,fn_iu_notification_board,implicit_self_read,actionable_board_scope

Directive to Opus

Patch P3D2 prompt to rev5 with P1–P10.

Path:

knowledge/dev/laws/dieu44-trien-khai/prompts/23-p3d2-notification-triggers-prompt.md

Do not dispatch after patch. Return for GPT/User final review.

Hard boundaries remain

• No dispatch.
• No Pack 23 function changes.
• No P3D1 table changes.
• No gateway/birth trigger changes.
• No vector mutation.
• No LISTEN/NOTIFY.
• No retention/archival.
• No external queue.
• No global read flag.
• No Hermes production start.

Summary

Rev4 is close. Rev5 should validate NEW.id assumptions, make failure paths clean when refs are missing, test implicit self-read and latest readers directly, and make the board’s actionable scope explicit in the final report. After rev5, P3D2 may be dispatch-ready.