GPT Review — 23-P3D1 Notification Schema Functions Prompt rev1

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.md rev1

Verdict

Direction accepted, but do not dispatch yet. Rev2 required.

P3D1 rev1 follows the approved design and correctly excludes triggers. However, several execution bugs/test fragilities need to be fixed before creating new tables/functions.

Accepted points

• Scope is correct: 2 tables + indexes + 2 query functions only.
• No triggers in P3D1.
• fn_iu_unread as STABLE and fn_iu_mark_read as VOLATILE is appropriate.
• Cleanup on test failure drops P3D1 objects only.
• Pack 23 protected hashes are checked.
• Security pattern mirrors Pack 23 functions.

Required rev2 fixes

P1 — BLOCKER: manual review/draft test event may use empty TEST_DRAFT_ID

Rev1 selects:

TEST_DRAFT_ID=$(SELECT id FROM unit_edit_draft WHERE draft_status='open' AND unit_id='$TEST_UNIT_ID' LIMIT 1)

But the earliest information_unit may not have an open draft. Then the INSERT for draft_created uses empty uuid and fails.

Patch test data setup:

• use an existing open draft if available;
• if none exists, create a dedicated open draft through fn_iu_save(TEST_ADDR, body, actor, ..., mode='draft') on a dedicated P3D1 test IU/address;
• capture TEST_DRAFT_ID from the returned JSON;
• do not direct insert into unit_edit_draft.

Prefer using fn_iu_save to create a dedicated pilot IU + draft so the test is deterministic.

P2 — Test events should use a dedicated P3D1 pilot address, not oldest IU

Rev1 uses the oldest IU:

SELECT id FROM information_unit ORDER BY created_at LIMIT 1;

This can couple P3D1 tests to arbitrary legacy state.

Patch:

• create/use TEST_ADDR='test/p3d1/pilot-$TS' via fn_iu_save new address;
• then create one draft for that address via fn_iu_save(..., mode='draft');
• use that unit_id/draft_id for manual notification events.

This will mutate Pack 23 data by creating a pilot IU/draft, which is acceptable if explicitly reported and retained. It is safer than relying on arbitrary old rows.

P3 — fn_iu_mark_read currently omits requested_count

Design required both requested_count and distinct_requested_count. Rev1 returns only distinct count.

Patch function to return:

• requested_count = array_length(p_event_ids,1);
• distinct_requested_count;
• existing/newly/already/unknown.

P4 — already_marked_count logic is wrong on first call

In rev1, already CTE is evaluated after inserted CTE in the same statement context. Depending on planner semantics, it may count newly inserted rows as already marked, or be ambiguous.

Patch by computing already-marked before insert in separate CTE/variable, or use a before_existing_read CTE explicitly referenced before insert.

Expected:

• first call: newly_marked_count=3, already_marked_count=0;
• second call: newly_marked_count=0, already_marked_count=3.

T11 should assert both.

P5 — JSON parsing in T11/T12 is unsafe

Rev1 does:

SELECT ($T11_RAW::jsonb)->>'newly_marked_count'

If JSON contains quotes/spaces or shell special characters, SQL breaks.

Patch tests to extract fields in the same SQL call using CTE:

WITH r AS (SELECT public.fn_iu_mark_read(...) AS j)
SELECT j->>'newly_marked_count', j->>'already_marked_count', j::text FROM r;

Use US delimiter as in prior prompts.

P6 — T7 empty unread may fail if manual/pilot events exist from a previous partial run

Rev1 requires fn_iu_unread('gpt') returns 0 immediately after table creation. If P3D1 objects were already created is preflight fail, OK. But if a prior failed run left rows but cleanup failed, preflight currently stops on table exists, so less likely.

Still, make T7 precise:

• after fresh table creation, event table count should be 0;
• then fn_iu_unread('gpt') returns 0.

This distinguishes “function bug” from “table not empty”.

P7 — T4 index check by LIKE '%notif%' is too loose

Rev1 counts indexes whose names contain notif. Verify exact expected index names:

• uq_notif_event_type_ref
• idx_notif_event_stream_created
• idx_notif_event_unit_created
• idx_notif_event_created
• uq_notif_read_event_actor
• idx_notif_read_actor_event

Require all six exist. Also verify uq_notif_event_type_ref is partial (indpred IS NOT NULL).

P8 — T2 constraints should verify names/types, not only count >=3

Count check may pass with wrong constraints. Add named constraints in DDL, then test by name:

• chk_notif_event_type
• chk_notif_event_stream
• chk_notif_event_type_stream
• chk_notif_event_canonical_address_nonempty
• chk_notif_event_actor_ref_nonempty
• chk_notif_event_source_nonempty
• chk_notif_read_actor_ref_nonempty

T2 should require these names exist.

P9 — DDL should name FK constraints

For report/debug, name FKs:

• fk_notif_event_unit
• fk_notif_read_event

T3 should verify these exact names and fk_notif_read_event delete rule is CASCADE.

P10 — Function source safety check missing

Add test to verify P3D1 functions do not mutate Pack 23 tables:

Forbidden patterns in fn_iu_unread and fn_iu_mark_read source:

• INSERT INTO information_unit
• UPDATE information_unit
• INSERT INTO unit_version
• UPDATE unit_version
• app.canonical_writer

fn_iu_mark_read may insert only into iu_notification_read.

P11 — No triggers test should check all source tables too

Rev1 checks triggers on notification tables only. P3D1 must create no triggers anywhere.

Patch T15 to verify no user triggers with names matching trg_%iu_notif% on:

• unit_edit_comment
• unit_edit_draft
• unit_version
• iu_notification_event
• iu_notification_read

Expected 0.

P12 — Report should include P3D1 row counts and pilot IDs

Final report should include:

• notif_event_count;
• notif_read_count;
• test_addr;
• test_unit_id;
• test_draft_id;
• manual event ids if captured;
• p3d1_functions_created=true.

P13 — Grantee verification should test all grantees can execute

Rev1 grants but T6 only checks PUBLIC=0, owner/security/search_path. Add loop to verify each discovered grantee has EXECUTE on both functions.

P14 — Decide cleanup semantics for P3D1 PASS vs FAIL

Rev1 says test_rows_retained=true. That is acceptable, but P3D1 manual notification rows are not created by triggers and may pollute future P3D2 tests.

Recommendation:

• On PASS: retain tables/functions, but delete manual test notification rows with source='test_p3d1'?
• Or keep them and make P3D2 filter by source/time.

GPT recommends delete manual notification rows after tests PASS, but keep pilot IU/draft rows. This prevents P3D2 starting with fake notification events.

If Opus prefers retention, report clearly and P3D2 must account for them. Patch prompt to choose one. GPT preference: cleanup manual notification rows on PASS only; on FAIL cleanup drops whole P3D1 objects.

Directive to Opus

Patch P3D1 prompt to rev2 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 rev1 has the right structure. Rev2 should make tests deterministic with a dedicated pilot IU/draft, tighten schema verification by name, fix fn_iu_mark_read counting and JSON parsing, and avoid leaving fake notification events that could confuse P3D2.