Incomex AI Portal
KB-2383

GPT Review — 23-P3C2 Prompt rev2

9 min read Revision 1
gpt-reviewpack-23p3c2rev3-requiredapply-wrappernotification-outbox

GPT Review — 23-P3C2 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-p3c2-iu-apply-edit-functions-prompt.md rev2
  • knowledge/dev/laws/dieu44-trien-khai/design/23-p3d-notification-outbox-roadmap-note.md rev1

Verdict

P3C2 rev2 is much improved but NOT ready to dispatch. Rev3 required.

Opus correctly fixed the critical content_anchor_ref mistake and created the P3D notification roadmap note. The overall direction is accepted. However, rev2 still has one runtime-convention blocker and several test/reporting hardening issues.

Accepted rev2 improvements

  • content_anchor_ref = v_new_uv_id::text now matches Pack 22 invariant.
  • Preflight verifies existing anchor convention.
  • JSON tests use WITH r AS ... SELECT fields instead of casting formatted psql output.
  • Exact unique (unit_id, version_seq) check is attempted.
  • Lifecycle count uses SQL count, not grep.
  • IU update is a single UPDATE.
  • P3C1 source hashes are captured before/after.
  • P3D notification outbox hook is recorded.
  • Official rows retained on test fail are acknowledged.

P3D roadmap note verdict

Accepted as roadmap.

The two-log split is right:

  • comment event log;
  • update/apply event log.

They should be lightweight work queues, not general activity logs and not full body stores. PG triggers are acceptable in P3D. This should stay deferred until P3C2 PASS.

Minor future addition for P3D design: include event status/processing fields only if needed after inspection, e.g. processed_at, processor_ref, or use per-actor watermark instead. Do not overbuild in the roadmap note now.

Required rev3 fixes

P1 — BLOCKER: lifecycle convention GUC is transaction-local and unavailable when function runs

Rev2 sets:

SELECT set_config('app.p3c2_uv_lifecycle', :'uv_lifecycle', true);

inside the CREATE FUNCTION transaction. But fn_iu_apply_edit_draft is executed later in separate test sessions. Therefore:

current_setting('app.p3c2_uv_lifecycle', true)

inside the function will be empty and fallback to 'draft'.

This happens to match current runtime, but it is not the deterministic convention design we intended.

Patch function logic to determine lifecycle at runtime before writes:

SELECT count(DISTINCT lifecycle_status), min(lifecycle_status)
INTO v_lifecycle_count, v_uv_lifecycle
FROM public.unit_version;

IF v_lifecycle_count != 1 OR v_uv_lifecycle IS NULL OR btrim(v_uv_lifecycle) = '' THEN
  RETURN jsonb_build_object(
    'status','lifecycle_ambiguous',
    'guidance','unit_version.lifecycle_status convention is not uniquely determined.',
    'next_action','stop_for_gpt_review'
  );
END IF;

Then use v_uv_lifecycle in the INSERT.

Remove dependence on app.p3c2_uv_lifecycle from function runtime. Keep preflight lifecycle report for evidence.

P2 — Fix exact unique index check ordering

Rev2 uses array_agg(a.attname ORDER BY a.attnum), which can false-pass if index column order differs but table attnum order is the same. Use unnest(i.indkey) WITH ORDINALITY and order by ordinality.

Expected exact array:

ARRAY['unit_id','version_seq']

Expected count exactly 1.

P3 — P3C1 function count uses grep -c ... || echo 0 bug

Rev2 has:

P3C1_COUNT=$(echo "$P3C1_HASHES_BEFORE" | tr '|' '\n' | grep -c . 2>/dev/null || echo "0")

If empty, this can produce 0\n0. Use SQL count directly:

SELECT count(*) FROM pg_proc ... WHERE proname IN (...)

Keep hashes separately.

P4 — Test JSON field parsing should use a delimiter that cannot appear in JSON text, or separate SQL fields

Rev2 parses T1_P with cut -d'|'. This is probably okay but not guaranteed if JSON text/comment contains | in future.

Safer options:

  • output fields separately without the full JSON in the same pipe line; or
  • use ASCII unit separator via psql -F $'\x1f'; or
  • for report, capture full JSON in a separate query after status/id extraction.

Patch minimally: remove full JSON from the pipe-parsed line and capture UX JSON separately.

Apply to T1/T11.

P5 — T19 invariant check should parse all_pass, not grep for true

Rev2 does:

echo "$T1_INV" | grep -q "true"

This can false-pass if any subfield contains true while all_pass=false.

Patch T1 extraction to include:

j->'invariants'->>'all_pass'

and T19 requires exactly true.

P6 — Security grantee check still uses raw role interpolation

Rev2 still uses:

has_function_privilege('$R','public.$SIG','EXECUTE')

Patch to heredoc with psql variables or validate role names strictly before interpolation.

Preferred:

CAN=$("${PSQL_NOSTOP[@]}" -v role="$R" -v sig="public.$SIG" -t -A <<'SQL'
SELECT has_function_privilege(:'role', :'sig', 'EXECUTE');
SQL
)

This uses heredoc, not -c with :'var'.

P7 — Source check should ensure only apply, not wrapper, has IU/UV write paths; include all write verbs/tables

Rev2 wrapper check includes some write patterns but not all combinations. Patch wrapper forbidden regex to include:

  • app\.canonical_writer
  • insert into information_unit
  • update information_unit
  • delete from information_unit
  • insert into unit_version
  • update unit_version
  • delete from unit_version

Apply function should have gateway marker and expected INSERT/UPDATE paths. It must not have DELETE paths.

P8 — Cleanup exact signature DROP should not use unquoted public.$SIG in a way that shell splits

Current loop uses:

DROP FUNCTION IF EXISTS public.$SIG;

Given SIG contains no spaces, this likely works. To be safer, use explicit statements rather than loop interpolation:

DROP FUNCTION IF EXISTS public.fn_iu_edit(text,text,text,text,text,text);
DROP FUNCTION IF EXISTS public.fn_iu_apply_edit_draft(uuid,text,text);

No CASCADE.

P9 — Preflight direct-write test uses invalid payload; may be blocked by other constraints before gateway

The direct write test inserts an invalid IU payload. Gateway likely fires first, but better use the same style as P3A/P3C1: verify error contains IU Gateway blocked; if not, report output and fail. Rev2 already does this. Add note that failure by other constraint is not acceptable for IU direct write because gateway should fire first due trigger order.

No code change required unless Opus wants to strengthen payload.

P10 — Report must include apply invariant JSON and anchor convention explicitly

Add report lines:

content_anchor_ref_verified=new_uv_id_text
invariant_all_pass=true
invariant_json=<...>

P11 — Notification roadmap should be referenced in final report as deferred dependency for Hermes

Rev2 already prints notification deferred. Add exact doc path:

knowledge/dev/laws/dieu44-trien-khai/design/23-p3d-notification-outbox-roadmap-note.md

and:

next_required_pack=P3D_NOTIFICATION_OUTBOX_BEFORE_HERMES_PRODUCTION

P12 — Official test writes retained must be visible in report

Add:

  • official_test_rows_retained_on_pass=true
  • new UV ids/sequences from T1 and T11;
  • applied/stale draft IDs.

Directive to Opus

Patch P3C2 prompt to rev3 with P1–P12.

Path:

knowledge/dev/laws/dieu44-trien-khai/prompts/23-p3c2-iu-apply-edit-functions-prompt.md

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

Hard boundaries remain

  • No dispatch.
  • No table DDL.
  • No trigger/gateway changes.
  • No vector mutation.
  • No cleanup.
  • No notification log implementation in P3C2.
  • No Pack 2C.

Summary

Rev2 fixed the most important invariant bug. The remaining blocker is that the lifecycle convention is passed via a transaction-local GUC that will not exist when the function is later called. Move lifecycle determination into the function or persist a config; GPT recommends runtime inspection with STOP/status if convention is not unique. After rev3, P3C2 should be close to dispatch-ready.