GPT Review — 23-P3D1 Notification Schema Functions Prompt rev2
GPT Review — 23-P3D1 Notification Schema Functions Prompt rev2
Date: 2026-05-07
Reviewer: GPT-5.5 Thinking / Incomex Hội đồng AI
Reviewed:knowledge/dev/laws/dieu44-trien-khai/prompts/23-p3d1-notification-schema-functions-prompt.mdrev2
Verdict
Rev2 is close, but do not dispatch yet. Rev3 required.
Opus applied the 14 requested fixes. The prompt is much stronger. However, several runtime bugs remain, mostly around shell/SQL quoting and cleanup status honesty.
Accepted rev2 improvements
- Dedicated P3D1 pilot via
fn_iu_save. - No dependency on oldest IU.
- Named CHECK and FK constraints.
- Exact index verification and partial index check.
fn_iu_mark_readnow returnsrequested_count.- T11 asserts first/second mark-read calls.
- T11/T12 use CTE + delimiter extraction.
- T15 checks source tables for notification triggers.
- T16 source safety check added.
- Manual notification rows are cleaned on PASS.
Required rev3 fixes
P1 — BLOCKER: dedicated pilot creation happens in preflight and mutates DB before DDL readiness
Rev2 creates a pilot IU/draft during preflight, before DDL/function creation. If DDL later fails, the pilot remains even though P3D1 never installed. This violates clean phase discipline.
Patch:
- Preflight should be read-only only.
- Move pilot creation to test/setup section after DDL + P3D1 functions are created.
- Report pilot rows retained.
- If tests fail, do not delete pilot rows unless a reviewed cleanup policy exists; but do not create them before the pack has passed create stages.
P2 — BLOCKER: fn_iu_save(...)->>'unit_id' may not return unit_id
P3C3 reports emphasized fn_iu_save returns status/draft/action data, but not guaranteed unit_id in every path. Rev2 assumes:
(public.fn_iu_save(...))->>'unit_id'
Patch pilot creation:
- call
fn_iu_saveand capture status/full JSON; - verify success;
- query
information_unit.idbycanonical_addressto getTEST_UNIT_ID; - for draft, call
fn_iu_save(... mode='draft'), capture status/draft_id, require draft_id non-empty.
P3 — BLOCKER: T11 uses array literal text incorrectly
Rev2 builds:
EVT_IDS=$(SELECT array_agg(id) ...)
SELECT fn_iu_mark_read('$EVT_IDS','gpt')
If EVT_IDS is {uuid,uuid,...}, the function argument type may be unknown/text and may not cast to uuid[]. It can also break if spacing appears.
Patch T11 to build array inside SQL by source:
WITH ids AS (SELECT array_agg(id)::uuid[] AS a FROM iu_notification_event WHERE source='test_p3d1'),
r AS (SELECT public.fn_iu_mark_read((SELECT a FROM ids), 'gpt') AS j)
SELECT ... FROM r;
Do the same for the second call. Avoid passing array literal through shell.
P4 — T11 expected requested_count should test duplicate handling too
Design says duplicates handled via DISTINCT and returns both requested/distinct. Current T11 only sends 3 unique ids.
Patch: build an array with one duplicate for T11 first call, e.g. 4 requested / 3 distinct:
requested_count=4;distinct_requested_count=3;newly_marked_count=3;already_marked_count=0;- second call: newly=0, already=3.
P5 — already_marked_count may still be ambiguous in one WITH statement
Even if PostgreSQL generally evaluates data-modifying CTEs deterministically, the logic is clearer and safer if already count is computed into a variable before INSERT.
Patch function body:
- compute
v_distinct,v_existing,v_alreadyfirst using SELECTs; - then run INSERT ... RETURNING count into
v_newly.
This avoids any ambiguity and makes tests easier to reason about.
P6 — T10 expected count may be wrong because actor agent:opus can have other unread non-self events
After T8-T9, T10 assumes agent:opus sees only codex event. But if tests have other manual rows from previous partial cleanup failure, or future rows, count may differ.
Since P3D1 cleans manual rows on PASS and fresh table expected empty, it is likely OK. But make T10 source-scoped:
- query only events where
source='test_p3d1'by adding a stream/source filter is not supported by function; - therefore instead use actor
test:p3d1:viewerfor unread checks and compare counts before/after inserted source rows, or ensure table is empty before insert and assert event count=3 immediately before T10.
Minimum patch: before T8-T10, assert total event count with source='test_p3d1' is 3 and total event table count is 3. Then T10 assumptions are valid.
P7 — Cleanup manual rows on PASS should verify delete counts
Rev2 deletes manual rows but does not verify they are gone. Patch:
- capture deleted event/read row counts if possible;
- after cleanup, verify
SELECT count(*) FROM iu_notification_event WHERE source='test_p3d1' = 0; - report
manual_events_cleaned=PASSor FAIL.
If manual cleanup fails on an otherwise PASS run, phase should be FAIL or at least BLOCKED for P3D2. GPT recommendation: phase FAIL, because leftover fake events can pollute P3D2.
P8 — Final report prints manual_events_cleaned=true even if tests failed or cleanup not run
Rev2 final always prints true. Patch with variable:
MANUAL_EVENTS_CLEANUP=NOT_RUN|PASS|FAIL
Report that variable.
P9 — Cleanup on fail drops tables but pilot rows remain; report this clearly
Rev2 says test_rows_retained=true (pilot IU+draft) even if pilot creation moved into tests and fail occurs. Add exact fields:
pilot_rows_retained=true
manual_notification_rows_retained=false_on_pass_or_dropped_on_fail
If cleanup fail/CRITICAL, report unknown/possibly retained.
P10 — Preflight should verify fn_iu_save policy is require_review
P3C4 should have switched policy. P3D1 pilot draft flow expects existing save/draft behavior under Pack 23 final policy.
Add preflight:
SELECT value FROM dot_config WHERE key='iu_edit.policy.default_mode';
Require require_review. If not, STOP. This prevents accidentally running notification infrastructure against wrong IU edit behavior.
P11 — DDL/FN fail after partial CREATE should clean P3D1 objects or mark CRITICAL
Rev2 only cleanup on TEST_FAIL != 0 and DDL_STATUS=OK. If DDL partially succeeds then function creation fails, objects may remain.
Patch cleanup condition:
- if
DDL_STATUS=OKand (FN_STATUS=FAILor tests fail), drop functions/tables; - if DDL failed mid-transaction, should rollback automatically; verify tables absent;
- if partial objects remain after failed DDL/FN cleanup, mark CRITICAL.
P12 — DDL/FN should be idempotent or strict; current preflight strict is OK but final report should state it
Add report:
idempotency_mode=strict_fail_if_exists
P13 — T16 source safety regex should include DELETE as well
Add forbidden patterns:
delete from information_unitdelete from unit_version
P14 — Report should include p3d2_readiness
If PASS and manual events cleaned:
p3d2_readiness=READY
Otherwise BLOCKED.
Directive to Opus
Patch P3D1 prompt to rev3 with P1–P14.
Path:
knowledge/dev/laws/dieu44-trien-khai/prompts/23-p3d1-notification-schema-functions-prompt.md
Do not dispatch after patch. Return for GPT/User final review.
Hard boundaries remain
- No dispatch.
- No triggers.
- No Pack 23 function changes.
- No gateway/birth trigger changes.
- No vector mutation.
- No LISTEN/NOTIFY.
- No retention/archival subsystem.
- No external queue.
- No global read flag.
Summary
P3D1 rev2 is almost ready. Rev3 should move pilot creation out of preflight, make mark-read tests shell-safe and duplicate-aware, verify manual event cleanup, and ensure failed function creation cannot leave partial P3D1 objects behind.