KB-95E3

GPT Review — 22-P1 Prompt rev2 Final Risk Check

6 min read Revision 1
gpt-reviewpack-22p1helper-functionsrev3-requiredfinal-risk-check

GPT Review — 22-P1 Prompt rev2 Final Risk Check

Date: 2026-05-06 Reviewer: GPT-5.5 Thinking / Incomex Hội đồng AI Reviewed: knowledge/dev/laws/dieu44-trien-khai/prompts/22-p1-iu-native-create-helper-functions-prompt.md rev2

Verdict

P1 rev2 is much stronger, but not yet dispatchable. Rev3 required.

The prompt now has the right safety shape: CREATE FUNCTION only, transaction-wrapped, no OR REPLACE, no row creation, no adapter, PUBLIC revoke. However, several details still risk false rollback, ambiguous helper output, or future hardcode drift.

Required rev3 patches

P1 — Do not hard-gate on live row counts

Rev2 requires before/after counts to match exactly and says rollback if any count differs, including birth_total.

This is unsafe on a live system. Another process can create unrelated birth rows while P1 is running, causing a false rollback of valid function DDL.

Patch:

  • Counts are audit, not rollback gate.
  • Since all helpers are declared IMMUTABLE/STABLE and tests are read-only, they cannot create rows.
  • If counts drift, report as external drift warning; do not rollback solely because counts changed.
  • Still rollback on any function creation/test failure.

Suggested wording:

IU/UV/IU-birth/total-birth counts are audit-only in P1. Any drift should be reported. Do not treat count drift alone as proof that P1 mutated data.

P2 — Schema-qualify function creation and conflict checks

Rev2 creates unqualified functions:

CREATE FUNCTION fn_content_hash(...)

This depends on current search_path. Rev3 should use explicit public schema everywhere:

CREATE FUNCTION public.fn_content_hash(...)
CREATE FUNCTION public.fn_iu_resolve_default(...)
...

Preflight conflict check should also be schema-specific:

SELECT n.nspname, p.proname
FROM pg_proc p
JOIN pg_namespace n ON n.oid = p.pronamespace
WHERE n.nspname='public'
  AND p.proname IN (...);

P3 — fn_iu_classify_existing should return JSON not_found, not SQL NULL

Rev2 still returns NULL for absent IU. That makes downstream logic need special NULL handling and creates ambiguity between “not found” and function failure/missing JSON.

Patch:

IF NOT FOUND THEN
  RETURN jsonb_build_object('status','not_found','canonical_address',p_addr,'issues',jsonb_build_array());
END IF;

Update tests accordingly.

P4 — Invalid configured default must not silently fall through

In fn_iu_resolve_default, if iu_create.default_* exists but points to a removed vocab value, rev2 raises warning and falls through to auto-single. This hides a misconfigured default.

Patch:

  • If configured default exists and is invalid, return status='invalid_config' with message.
  • Do not fall through to auto-single.

This preserves observability: broken config must ring a bell.

P5 — Avoid hardcoded UV birth pattern when unit_version strategy changes

Rev2 fn_iu_verify_invariants says if unit_version strategy differs from subordinate, check for unit_version::<id> birth row.

That is a hardcoded future assumption. If strategy changes, entity_code may not be unit_version::<id>.

Patch:

  • If strategy is subordinate or NULL: expect no UV birth, as current semantics.
  • If strategy is anything else: return warning/status uv_strategy_changed; do not hardcode a birth entity_code pattern.
  • Set i5_uv_birth_ok to false or null with issue, and all_pass=false until verification semantics are explicitly updated.

P6 — Preflight trigger discovery should detect duplicates

Rev2 SELECT pg_get_triggerdef INTO v_birth_def can fail if multiple triggers match, or silently become ambiguous depending exact PL/pgSQL behavior.

Patch:

  • Count matching enabled birth/L1/L2 triggers by category.
  • Require exactly one for each category.
  • If count != 1 → RAISE EXCEPTION with count.
  • Then select definition.

This matches the “exactly one” principle already used for FK.

P7 — Add rollback/partial report instruction for failure inside transaction

Rev2 says rollback if fail, but execution prompt should make it explicit:

  • If any SQL in transaction fails, run ROLLBACK if transaction is still open.
  • Upload partial report with the failing SQL block/error.
  • Do not retry with modified SQL unless GPT/User approves.

P8 — Keep fn_sbx_compute_content_hash inspection out of transaction or make it report-only

Rev2 default is independent fn_content_hash(text), good. Make clear the fn_sbx_compute_content_hash inspection is report-only and must not influence SQL generation in P1.

This prevents the Agent from improvising a reuse branch.

Directive to Opus

Patch P1 prompt to rev3 with P1–P8. Then return for GPT/User final approval.

Hard boundaries remain

  • helper function DDL only;
  • no IU/UV rows;
  • no raw birth_registry insert;
  • no table/schema DDL;
  • no adapter;
  • no dot_tools registration;
  • no iu_create.* default seeding;
  • no broad PUBLIC execute;
  • no count-drift false rollback.

Summary

P1 is close. The remaining fixes are mostly about long-term operational stability: live-system count drift, schema-qualified DDL, consistent JSON outputs, config error visibility, no future strategy hardcode, duplicate trigger detection, and safe failure reporting.

Back to Knowledge Hub knowledge/dev/laws/dieu44-trien-khai/reviews/gpt-review-22-p1-prompt-rev2-final-risk-check-2026-05-06.md