GPT Review — P3D Pack 1 Phase 4B Discovery Prompt rev5 Not Approved: set -e / detect_col Bug

Date: 2026-05-11 Reviewer: GPT-5.5 Thinking / Incomex Hội đồng AI Reviewed:

• knowledge/dev/laws/dieu44-trien-khai/prompts/p3d-pack1-phase4b-readonly-species-composition-registry-discovery-prompt.md rev5
• knowledge/dev/laws/dieu44-trien-khai/reports/p3d-pack1-phase4b-discovery-prompt-rev5-patch-report.md
• previous GPT review requiring true introspect-first behavior

Verdict

Discovery prompt rev5 is NOT approved for agent dispatch yet.

It is much improved and now largely introspect-first, but it contains a critical bash control-flow bug that can abort the script when optional columns are absent.

What rev5 fixed well

1. Added run_query wrapper so each query can fail independently.
2. Added col_exists and detect_col helpers.
3. Added column detection for TAC parent column.
4. Added universal_edges fallback via to_jsonb(...)::text scan.
5. Removed many static schema assumptions.
6. Preserved legal alignment with Điều 0-B, Điều 0-G, Điều 29, QT-001/QT-005.
7. Kept read-only/no-seed/no-migration boundaries.

Blocking issue

set -e + detect_col command substitution can abort the script

The prompt uses:

set -euo pipefail

and defines:

detect_col() {
  local tbl="$1"; shift
  for candidate in "$@"; do
    if col_exists "$tbl" "$candidate"; then
      echo "$candidate"
      return 0
    fi
  done
  echo ""
  return 1
}

Then it assigns:

PARENT_SP=$(detect_col entity_species parent_id parent_ref parent_species_id)
DEPTH_SP=$(detect_col entity_species depth tree_depth level)
SCM_COLNAME=$(detect_col species_collection_map collection_name name table_name)
...

Under set -e, an assignment using a command substitution can inherit the non-zero status of the command substitution. If an optional column is not found, detect_col returns 1, and the whole script may exit immediately.

This directly violates the intended graceful-degradation behavior.

Required patch

Patch prompt to rev6.

Either:

Option A — safest

Make detect_col always return 0 and encode absence as empty stdout:

detect_col() {
  local tbl="$1"; shift
  for candidate in "$@"; do
    if col_exists "$tbl" "$candidate"; then
      echo "$candidate"
      return 0
    fi
  done
  echo ""
  return 0
}

Option B — guard every assignment

PARENT_SP=$(detect_col entity_species parent_id parent_ref parent_species_id || true)

Option A is preferred because it centralizes the graceful-degradation contract.

Additional minor fixes recommended

1. Add a preflight read-only proof:

run_query "PRE: read-only proof" -c "SET default_transaction_read_only = on; SHOW default_transaction_read_only;"

Since each psql call is separate, this setting will not persist globally, so phrase it as a proof/intent, not a global guarantee. No write statements are present, so this is acceptable.

2. Add table existence helper or tolerate missing tables more explicitly. Current col_exists naturally returns no columns if table is missing; that is acceptable for discovery.

3. In report requirements, ask the Agent to state whether any detect_col result was empty and confirm the script did not abort due to optional missing columns.

Status

phase4b_discovery_prompt_rev5=NOT_APPROVED_FOR_DISPATCH
reason=set_e_detect_col_optional_column_abort_risk
legal_alignment_addendum=ACCEPTED
phase4b_design=ACCEPTED_DIRECTIONALLY
migration_allowed=false
agent_dispatch_allowed=false
next_action=OPUS_PATCH_DISCOVERY_PROMPT_REV6_FIX_DETECT_COL_GRACEFUL_DEGRADATION