KB-6BA2

GPT Review — 23-P3D1 Notification Schema Functions Prompt rev2

8 min read Revision 1
gpt-reviewpack-23p3d1notificationschemarev3-required

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

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_read now returns requested_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:

  1. call fn_iu_save and capture status/full JSON;
  2. verify success;
  3. query information_unit.id by canonical_address to get TEST_UNIT_ID;
  4. 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_already first 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:viewer for 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=PASS or 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=OK and (FN_STATUS=FAIL or 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_unit
  • delete 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.

Back to Knowledge Hub knowledge/dev/laws/dieu44-trien-khai/reviews/gpt-review-23-p3d1-notification-schema-functions-prompt-rev2-2026-05-07.md