GPT Review — 22-P1 Helper Functions Prompt 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

Verdict

Do not dispatch current P1 prompt yet. Patch rev2 required.

The prompt is directionally correct but this is the first mutation step for a core creation contract. It must be tightened before execution to avoid creating brittle or overly-permissive helper functions.

Main risks found

R1 — CREATE OR REPLACE FUNCTION is unsafe for first install

The prompt says preflight expects 0 helper conflicts, then uses CREATE OR REPLACE. That can silently overwrite a function if a race or manual change happens after preflight.

Patch:

• Use CREATE FUNCTION, not CREATE OR REPLACE FUNCTION, for new helpers.
• If a helper already exists, STOP and report.
• If rerun is needed later, create an explicit migration pack.

R2 — Wrap all function DDL in a transaction

Current prompt does not clearly say BEGIN/COMMIT and rollback on verification failure.

Patch:

• Capture baseline counts first.
• BEGIN.
• Create helper functions.
• Verify signatures/existence.
• Run read-only helper tests.
• Verify IU/UV/birth counts unchanged.
• COMMIT only if all checks pass.
• ROLLBACK on any failure and upload partial report.

R3 — Do not adapt fn_sbx_compute_content_hash with placeholder args

The prompt includes:

SELECT fn_sbx_compute_content_hash(p_body, ...adapted args...)

This is too risky. A 4-arg sandbox hash may encode sandbox semantics that do not belong to IU content hashing.

Patch:

• Inspect fn_sbx_compute_content_hash and report it.
• Do not call or wrap it in P1 unless exact compatibility is proven and explicitly approved.
• Default safe path for P1: create fn_content_hash(text) using digest(text,'sha256') as the IU contract helper.
• Record TD to rationalize with fn_sbx_compute_content_hash later.

R4 — Revoke default PUBLIC EXECUTE on new helper functions

PostgreSQL grants EXECUTE on functions to PUBLIC by default in many setups. Current prompt does not revoke.

Patch after creation:

REVOKE ALL ON FUNCTION public.fn_content_hash(text) FROM PUBLIC;
REVOKE ALL ON FUNCTION public.fn_iu_resolve_default(text,text,text) FROM PUBLIC;
REVOKE ALL ON FUNCTION public.fn_iu_classify_existing(text) FROM PUBLIC;
REVOKE ALL ON FUNCTION public.fn_iu_create_preflight() FROM PUBLIC;
REVOKE ALL ON FUNCTION public.fn_iu_verify_invariants(text) FROM PUBLIC;

Do not grant to adapter role in P1 unless needed. P2/P3 can grant plan/create functions deliberately.

R5 — fn_iu_resolve_default should avoid LIKE/replace pitfalls

Current implementation uses:

• key LIKE p_vocab_prefix || '%'
• replace(key, p_vocab_prefix, '')

This is fragile for prefixes containing wildcard characters and replace-all behavior.

Patch:

• Use left(key, length(p_vocab_prefix)) = p_vocab_prefix.
• Extract token with substring(key from length(p_vocab_prefix)+1).
• Order values deterministically.
• Check configured default key count; if duplicate config keys exist, return invalid_config or raise.

R6 — fn_iu_classify_existing boolean logic can return exists_unknown_state when anchors are NULL

v_anchor_exact can be NULL, and WHEN NOT v_anchor_exact does not catch NULL.

Patch:

• Initialize booleans with COALESCE(..., false).
• Use IS TRUE / IS NOT TRUE.
• Filter birth by collection_name='information_unit'.
• Prefer returning an issues array so missing birth and invalid anchor can both be visible.
• Return consistent JSON object for not found, e.g. {'status':'not_found'}, instead of SQL NULL.

R7 — fn_iu_verify_invariants must handle duplicate/no UV defensively

Current function selects one UV v1 row without counting duplicates.

Patch:

• Count UV v1 rows first.
• 0 → i2_uv_linked=false, issue missing_version.

• 1 → issue duplicate_version.

• exactly 1 → verify anchors.

Also for unit_version birth expectation:

• If birth_code_strategy='subordinate' → expect no UV birth.
• If strategy differs → do not assume unit_version::<id> pattern blindly; report uv_strategy_changed / warning and do not hardcode a new expectation.

R8 — Preflight trigger checks need behavior/timing, not just function names

Current fn_iu_create_preflight checks only for enabled triggers by function name. It should also verify trigger definitions/timing categories from pg_get_triggerdef():

• birth trigger: enabled AFTER INSERT on information_unit, function fn_birth_registry_auto or P0-approved equivalent;
• L1: enabled BEFORE INSERT;
• L2: enabled constraint trigger, deferrable, initially deferred where current contract expects.

If function names remain canonical candidates, mention they are P0-confirmed names from the P0 report, not timeless assumptions.

R9 — Baseline counts must be captured before function DDL

Report asks before/after counts but prompt only gives after query.

Patch:

• Before BEGIN, record IU count, UV count, IU birth count, total birth count as audit.
• After tests, verify IU/UV/IU-birth unchanged. Total birth can be audit only if system is live.

R10 — Clarify DDL boundary

P1 necessarily performs function DDL. Hard boundary should say:

• allowed: CREATE FUNCTION helper DDL only;
• forbidden: table/schema DDL, DML rows, DOT patch, adapter, birth_registry insert.

Directive to Opus

Patch P1 prompt to rev2 with R1–R10. Then return for GPT/User review before dispatch.

Hard boundaries for rev2

• helper function DDL only;
• no IU/UV rows;
• no birth_registry rows;
• no table/schema DDL;
• no DOT adapter;
• no dot_tools registration;
• no iu_create.* default seeding;
• no broad PUBLIC function execution.

Summary

P1 is the right next step, but must be treated like a migration for core infrastructure. The main fixes are: no OR REPLACE, transaction + rollback, no sandbox hash adaptation, revoke PUBLIC execute, safer vocab lookup, robust classification/verification, and behavior-based trigger preflight.