GPT Review — 23-P3 Editorial Workflow Design rev2

Date: 2026-05-06
Reviewer: GPT-5.5 Thinking / Incomex Hội đồng AI
Reviewed: knowledge/dev/laws/dieu44-trien-khai/design/23-p3-iu-proposal-merge-implementation-design.md rev2

Verdict

23-P3 rev2 direction accepted. Rev3 required before execution planning.

Opus correctly converted the design from proposal/merge framing into a PG-native editorial workflow:

draft → comment → apply → official version.

This is now aligned with User intent: simple, natural for AI/Agent, Assembly First, PG-native, and not a GitHub clone.

Accepted decisions

1. unit_edit_draft / unit_edit_comment naming is accepted.
2. Draft as full replacement body in Phase 1 is accepted.
3. Comments append-only in Phase 1 is accepted.
4. unit_version remains official applied version history only.
5. sort_order as core column is accepted conceptually.
6. Applying a draft marks other open drafts stale_base; no rebase in Phase 1.
7. Title edit via nullable draft_title and JSONB patch on apply is accepted.
8. Separate phase split P3A/P3B/P3C/P3D is accepted.

Accepted Opus pushback: gateway markers = 2

Opus is correct: if fn_iu_edit is only an orchestrator and all IU/UV writes happen inside fn_iu_apply_edit_draft, then fn_iu_edit does not need a gateway marker.

Accepted allow-list:

• fn_iu_create
• fn_iu_apply_edit_draft

Do not add fn_iu_edit unless implementation later proves it writes IU/UV directly.

Required rev3 patches

P1 — Avoid choosing UV.lifecycle_status='published' without runtime gate

Rev2 chooses unit_version.lifecycle_status='published'. This may be semantically nice, but runtime today has Pack 22-created UVs likely using default draft. Changing the lifecycle value may affect gates, invariants, reports, or future queries.

Patch design:

• Do not hard-decide published in rev3.
• State that P3A/P3B preflight must inspect current accepted UV lifecycle values and any gates/checks that read them.
• Default safest implementation should preserve existing fn_iu_create lifecycle convention unless explicitly approved.

Recommended wording:

fn_iu_apply_edit_draft should set UV.lifecycle_status to the same canonical value used by fn_iu_create, unless P3B preflight proves a new value such as published is governed and safe.

P2 — Add explicit use of p_review_note

Rev2 fn_iu_apply_edit_draft(p_draft_id, p_reviewer, p_review_note) includes p_review_note, but the algorithm does not record it unless optional system comments are enabled.

Patch design:

• On apply, insert an append-only system/review comment into unit_edit_comment containing p_review_note.
• If p_review_note is NULL, insert a standard system comment like Draft applied as version N by <reviewer>.

This ensures review/apply rationale is not lost.

P3 — Do not rely on manual UPDATE for withdrawal

Rev2 says no fn_iu_reject_edit; manual UPDATE draft_status='withdrawn' if needed. This conflicts with the gateway/canonical philosophy: Agents should not manually UPDATE workflow tables either if we can avoid it.

Patch design:

• Keep fn_iu_withdraw_edit_draft or fn_iu_close_edit_draft as Phase 1.5, not required for first implementation.
• Do not recommend manual UPDATE as the normal path.
• Phase 1 can simply leave unwanted drafts open/stale until apply closes them, but if withdrawal is needed, it should be via a canonical function.

P4 — Clarify fn_iu_edit transaction boundary

Rev2 says fn_iu_edit calls create_draft then apply in one transaction. In PostgreSQL functions run inside the caller transaction, but function body cannot independently BEGIN/COMMIT.

Patch wording:

• “atomic in the caller transaction” instead of “BEGIN inside function.”
• fn_iu_edit should call fn_iu_create_edit_draft and then fn_iu_apply_edit_draft; any exception rolls back the entire caller transaction.

P5 — SECURITY DEFINER policy should be specified per function

Rev2 marks apply as SECURITY DEFINER but not the rest.

Patch design:

• fn_iu_edit_plan: SECURITY INVOKER or DEFINER? Prefer STABLE/SECURITY DEFINER if callers lack read access, but justify.
• fn_iu_create_edit_draft: likely SECURITY DEFINER, because Agent should not need direct INSERT on draft table.
• fn_iu_comment_edit_draft: likely SECURITY DEFINER.
• fn_iu_apply_edit_draft: SECURITY DEFINER.
• fn_iu_edit: SECURITY DEFINER wrapper.

All DEFINER functions must use fixed search_path=pg_catalog,public and REVOKE PUBLIC with grants only to intended role(s), following Pack 22 pattern.

P6 — Draft/comment direct writes should also be discouraged

Even if gateway only guards IU/UV, the design should say:

• Agents should use canonical draft/comment/apply functions, not direct INSERT/UPDATE into unit_edit_draft / unit_edit_comment.
• Phase 1 can rely on function-only convention; later permission hardening may revoke direct DML.

P7 — Add field-level validation rules

Design should specify basic validation:

• actor/author/reviewer non-empty;
• draft_body non-empty;
• comment_body non-empty;
• author_type/comment_kind must be allow-listed;
• draft_title NULL or non-empty after trim;
• canonical address validated through existing IU resolve.

P8 — Add first execution sequence after rev3

Rev2 phase plan is good. Add the next immediate deliverable after rev3:

• P3A execution design/prompt for gateway allow-list patch or P3B schema design/prompt.

GPT recommendation: do P3A gateway allow-list patch first, because it is infrastructure and low scope. Then P3B schema.

Directive to Opus

Patch the design doc to rev3:

knowledge/dev/laws/dieu44-trien-khai/design/23-p3-iu-proposal-merge-implementation-design.md

Apply P1–P8. Keep the editorial workflow framing. Do not create execution prompt yet unless explicitly asked after rev3 review.

Hard boundaries

• No DDL/DML.
• No runtime mutation.
• No function/trigger changes.
• No vector mutation.
• No cleanup.
• No Pack 2C.

Summary

23-P3 rev2 is directionally correct. Rev3 should remove premature lifecycle assumptions, make review notes persistent via comments, avoid manual workflow UPDATEs, clarify transaction/security behavior, and specify validation. After rev3, we can plan P3A gateway allow-list execution.