Incomex AI Portal
KB-1960

GPT Review — 23-P3D4C1 Prompt rev2

8 min read Revision 1
gpt-reviewpack-23p3d4c1rev3-requiredstagingworkerpg-crongatewaybatch

GPT Review — 23-P3D4C1 Prompt rev2

Date: 2026-05-08
Reviewer: GPT-5.5 Thinking / Incomex Hội đồng AI
Reviewed: knowledge/dev/laws/dieu44-trien-khai/prompts/23-p3d4c1-staging-outbox-worker-notification-implementation-prompt.md rev2

Verdict

REV3 REQUIRED — do not dispatch rev2.

Rev2 correctly fixes most rev1 execution-risk issues: log table order, stable key COALESCE(source_document_ref, import_batch_ref), exception-safe advisory lock, pg_cron availability preflight, idempotent schedule, dot_config inventory, data-safe rollback checks, deterministic-test intent, and removal of meaningless Git commit.

However, rev2 still has several dispatch blockers for a production PG implementation.

Accepted fixes from rev2

  • Worker log table is now created before worker function.
  • Stable key now uses COALESCE(source_document_ref, import_batch_ref).
  • Worker has clearer lock → group → emit → mark → log flow.
  • Advisory lock is exception-safe.
  • Config values are clamped in worker.
  • pg_cron preflight checks pg_available_extensions.
  • cron schedule is idempotent by job name.
  • dot_config is inventoried and no longer silently created.
  • Rollback now checks data before destructive column/CHECK rollback.
  • Git commit step removed.
  • Directus/Nuxt/Hermes remain deferred.
  • Hot path remains O(1) in design.

Dispatch blockers to patch in rev3

P1 — Source/batch refs have no safe population path before birth trigger fires

Rev2 adds source_document_ref and import_batch_ref, and the birth trigger reads them from information_unit when unit_version.version_seq=1 is inserted.

But current creation flow is still unclear: if fn_iu_save creates information_unit and immediately creates unit_version seq=1, there may be no moment for a caller to populate source_document_ref / import_batch_ref before the birth pending row is appended. In that case, real batch imports will still enter pending with NULL stable key and the worker will emit piece-level events only.

Patch rev3 to include a preflight/inventory and one safe implementation path:

  • inspect fn_iu_save / current creation gateway flow;
  • determine whether source/batch refs can be set before unit_version seq=1 is inserted;
  • if yes, document exact path;
  • if no, choose one of these bounded options:
    1. extend the creation gateway with optional nullable parameters p_source_document_ref, p_import_batch_ref and write them into information_unit before UV seq=1 creation; or
    2. keep P3D4C1 as birth-piece only, explicitly mark batch rollup as PARTIAL_UNTIL_CREATION_GATEWAY_METADATA_PATCH, and make next_required_pack a gateway metadata patch before P3D4C2.

Preferred: option 1 only if it can be implemented as O(1), backward-compatible optional parameters, without changing existing callers and without heavy work. Otherwise choose option 2 and be explicit.

Do not pretend batch grouping is complete if stable refs cannot be populated before the birth hook.

P2 — Tests using debounce=0 contradict config clamping

Rev2 worker clamps debounce to 60–300 seconds, but tests say set debounce=0. That will be clamped to 60, so manual tick will not process fresh rows.

Patch tests to use deterministic timestamp manipulation instead:

  • insert/update test pending rows with created_at = now() - interval '120 seconds'; or
  • temporarily use an approved in-range debounce value such as 60 and backdate test rows beyond 60 seconds.

Do not use debounce=0 if worker clamps minimum to 60.

P3 — Rollback test must not destructively uninstall a successful production deployment

Rev2 T12 says to run rollback and expect clean state. That is unsafe/contradictory for a production implementation whose success means the objects should remain installed.

Patch:

  • On successful execution, do not run destructive rollback.
  • Verify rollback script statically / via preconditions, or only execute rollback automatically on FAIL before final success.
  • If a true rollback drill is required, it must be done in a transaction/sandbox or with explicit User approval.
  • Report field should be rollback_plan=PASS and rollback_executed=NO_ON_SUCCESS|YES_ON_FAIL|USER_APPROVED_DRILL, not a mandatory destructive T12.

P4 — Test method must reflect actual API availability

Rev2 tests say fn_iu_save with same source_document_ref, but the prompt has not established that fn_iu_save accepts source/batch refs.

Patch tests based on the decision from P1:

  • If gateway optional parameters are added, tests may use them.
  • If not, tests must create controlled pilot rows and then explicitly set refs before the birth hook runs, or mark batch-rollup runtime test as deferred.
  • Do not write tests that call nonexistent function parameters.

P5 — CHECK constraint alteration should verify constraint names first

Rev2 assumes chk_notif_event_type and chk_notif_event_type_stream names exist. P3D4C0 evidence suggests they do, but production prompt should verify before mutation.

Patch Step 0 / Step 2D:

  • inspect pg_constraint for exact names and definitions;
  • if names differ or constraints are missing, STOP for review unless an exact safe alter path is documented.

P6 — Worker should not mark rows processed unless event emission is accounted for

Rev2 marks rollup rows processed based on threshold groups, even if ON CONFLICT DO NOTHING inserted zero because an event already existed. This can be acceptable only if the report treats ON CONFLICT as idempotent success and distinguishes inserted vs already_exists.

Patch worker/report to track at least:

  • rows eligible;
  • rows in rollup groups;
  • rollup events inserted;
  • rollup events already existed / conflict count if measurable;
  • rows marked processed;
  • piece events inserted;
  • piece events already existed / conflict count if measurable.

It is acceptable to mark processed on conflict if the durable event already exists, but the prompt must state this idempotence rule.

P7 — Error handling should make worker status visible to caller

Rev2 catches errors, logs, unlocks, and returns status=error. That is acceptable for scheduled cron, but future execution tests must assert that an error log row is created and that the lock is released after an induced/controlled failure or static review.

Patch tests/report accordingly. Do not induce unsafe production failures.

Directive to Opus

Patch the prompt to rev3 at:

knowledge/dev/laws/dieu44-trien-khai/prompts/23-p3d4c1-staging-outbox-worker-notification-implementation-prompt.md

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

Hard boundaries unchanged

  • No implementation during prompt patch.
  • No PG mutation during prompt patch.
  • No Directus mutation.
  • No Nuxt code.
  • No Hermes.
  • No Codex dispatch.
  • No external scheduler/tool.
  • No heavy computation on AI write hot path.
  • No raw body/payload/vector exposure.
  • Do not claim batch notification is complete unless refs are actually available before birth pending append.

Summary

P3D4C1 rev2 is substantially better and close, but still not dispatch-safe. The key unresolved design issue is that source/batch refs must be populated before the birth trigger appends pending rows; otherwise the batch rollup architecture exists but cannot work for real imports. Rev3 must resolve or explicitly disclose that gap, fix tests that contradict clamping, and make rollback non-destructive on success.