Incomex AI Portal
KB-4FC3

Codex Full Review — PG-native Procedure Index v0.1

24 min read Revision 1
workflow-managepidxcodex-reviewreview-only2026-06-23

Codex Full Review — PG-native Procedure Index v0.1

1. Executive verdict

PASS_WITH_CAVEATS_REQUIRES_PATCH

The architecture is coherent, thin, and aligned with “PG reads PG”: 2 tables + 2 views, no engine, scheduler, vector, Nuxt, governance, KG, or birth dependency. The truth/declaration separation is also correct in principle.

The package is not ready for Owner build authorization yet. The candidate readiness view has one likely PostgreSQL build failure and several false-green paths. These are bounded design/SQL patches, not a reason to redesign the 2T2V architecture.

Required next gate: apply the BLOCK/HIGH patches in the companion patch list, statically re-review the revised SQL, then seek Owner authorization for a governed prototype build.

2. Files read

All knowledge documents below were read through Agent Data MCP in the main process.

Foundation and mission rules:

  • .claude/skills/incomex-rules.md — 36 items / 8-step workflow (local mission skill, full file).
  • knowledge/dev/ssot/operating-rules.md — OR v7.58, revision 51.
  • knowledge/dev/laws/constitution.md — Constitution v4.6.3 BAN HÀNH, revision 44.
  • knowledge/dev/laws/law-01-foundation-principles.md — v3.3, revision 12.
  • knowledge/dev/ssot/anti-patterns.md — revision 9.
  • knowledge/dev/laws/dieu20-thiet-ke-truoc-trien-khai.md — v1.2 FINAL.
  • knowledge/dev/laws/dieu33-postgresql-law.md — v2.1 BAN HÀNH.
  • knowledge/dev/laws/law-08-dependency.md — revision 1.

Required PIDX package:

  • knowledge/dev/laws-new/workflow-manage/de-bai-quan-ly-quy-trinh.md — v0.8, revision 21.
  • knowledge/dev/laws-new/workflow-manage/reports/wf-procedure-index-readonly-survey-2026-06-23.md.
  • knowledge/dev/laws-new/workflow-manage/design/ref-grammar-v0.1.md.
  • knowledge/dev/laws-new/workflow-manage/design/minimal-technical-design-v0.1.md.
  • knowledge/dev/laws-new/workflow-manage/design/seed-procedure-candidates-v0.1.md.
  • knowledge/dev/laws-new/workflow-manage/reports/wf-procedure-index-go-no-go-v0.1.md.
  • knowledge/dev/laws-new/workflow-manage/design/pidx-build-design-v0.1.md.
  • knowledge/dev/laws-new/workflow-manage/design/pidx-ddl-candidate-v0.1.sql.md.
  • knowledge/dev/laws-new/workflow-manage/design/pidx-readiness-logic-v0.1.md.
  • knowledge/dev/laws-new/workflow-manage/design/pidx-seed-slice-v0.1.md.
  • knowledge/dev/laws-new/workflow-manage/design/pidx-test-plan-v0.1.md.
  • knowledge/dev/laws-new/workflow-manage/design/pidx-codex-review-packet-v0.1.md.
  • knowledge/dev/laws-new/workflow-manage/reports/pidx-build-design-go-no-go-v0.1.md.

Background files in request §2D were not read because the required package plus the enacted foundation/PostgreSQL/dependency laws resolved the review questions.

External syntax reference used: PostgreSQL 16 documentation for grouped SELECT rules and split_part behavior.

3. Problem statement alignment

PASS

  • The design preserves PG as operational truth and treats the Procedure Index as an eye, not a hand.
  • Ingredient rows remain lazy declarations. manifest_jsonb, notes, declared maturity, seeds, and future RAG/vector output are not read by the resolver to produce READY.
  • v0.1 has no workflow engine, scheduler, event bus, vector extension, Nuxt dependency, governance dependency, KG dependency, or birth dependency.
  • No all-object table is introduced. The inventory object is a view over existing sources.
  • pidx_* is clearer than wf_* and avoids conceptual collision with the existing workflow/scanner family.
  • Deferring pidx_procedure_link is safe for the prototype. However, the build design says links temporarily live in links_jsonb, while the candidate table has no links_jsonb; either remove that statement or explicitly use the existing non-authoritative manifest_jsonb.

Mission-specific declarations:

  • Permanent: the review requires one canonical resolver/parser contract so future kinds cannot create divergent hand-written parsing.
  • Mistake-resistant: exact prefix, segment-count, namespace, lifecycle, and warning gates must make malformed or ambiguous refs unable to return bare READY.
  • 100% automatic: readiness remains a pure SQL computation over PG facts; no human assertion can turn a procedure green.
  • Conservation: the candidate ON UPDATE CASCADE ON DELETE CASCADE conflicts with immutable identity/dependency conservation and must be changed before build.

4. Technical design alignment

CONCERN

The T3 decisions are mostly valid:

  • Option B-minus / 2T2V is the thinnest viable implementation.
  • pidx_* naming is appropriate; wf_* avoidance is justified.
  • pidx_procedure_link can remain deferred.
  • Inventory sources and UNKNOWN_SOURCE treatment for io/checker/template/report are appropriate.
  • RAG/vector deferral is correct.
  • The narrow performance model is credible in principle.

Required design corrections:

  1. Ref grammar is not yet implemented by one strict parser. The candidate SQL parses the identifier but never proves that the prefix equals ingredient_kind, never enforces exact arity, and does not implement documented default-schema behavior.
  2. Facet-qualified labels are documented as preferred but cannot resolve in the candidate SQL.
  3. “Exists” and “usable” are conflated. Inactive/retired DOTs, events, approvals, labels, or procedures can currently satisfy a required ingredient.
  4. A procedure with ingredients but zero required rows can become READY by vacuous truth.
  5. The Directus logical-collection model is internally inconsistent between inventory and readiness.
  6. The package does not classify the new tables against Điều 33 canonical-table rules. Before build, state whether PIDX tables are PG-only technical index tables or governed Directus entities. If governed, the current singular table names, natural text PK, title, and created_at/updated_at require explicit conformance resolution.

5. Construction design / SQL review

BLOCK

Build-blocking findings

  1. Grouped correlated warning aggregation is not valid as written. The warning_flags scalar subquery references s.w_fn, s.w_appr, and s.w_label from grouped outer rows without grouping or aggregating those outer columns. PostgreSQL grouped SELECT rules reject ungrouped references. Move warning expansion/aggregation into a CTE keyed by procedure_code and join the result to the rollup.

  2. Kind/prefix mismatch can produce false EXISTS. Resolution uses ingredient_kind for the CASE and only strips text after the first colon. Example: (kind='approval', ref='dot:patch_ops_code') probes apr_action_types for patch_ops_code and can return EXISTS. Prefix equality must be part of parse validity.

  3. split_part silently ignores malformed arity. Extra segments are ignored by probes, so field:public.dot_tools.code.extra can resolve the real code column. Missing segments with default ref_status='NORMALIZED' become MISSING, not INVALID_REF. Exact per-kind segment counts are required before any probe.

  4. Facet-qualified labels are broken. label:facet.code is compared as taxonomy.code = 'facet.code'; facet_id is never parsed. The inventory emits only label:<code>. The documented preferred canonical form therefore false-misses.

  5. Logical collection matching can false-exist across schemas. The Directus probe ignores the parsed schema and matches only dc.collection = p_rel; for example, a non-public schema ref can be marked EXISTS because a same-named Directus collection exists. Inventory also starts from physical tables, so logical-only collections are omitted and XOR warnings are one-sided.

  6. Zero required ingredients can return READY. Any procedure with one or more optional ingredients, all existing and warning-free, reaches the final ELSE 'READY'. Require required_count > 0; otherwise return UNMAPPED or a separately named non-ready state.

High/medium construction findings

  • Resolver warnings implement only function, approval, and label. LOGICAL_PHYSICAL_MISMATCH, STALE_SOURCE, and UNKNOWN_SOURCE_ACCEPTED are specified but not produced. Optional missing/unknown refs cause READY_WITH_WARNINGS while warning_flags can remain empty.
  • Lifecycle is ignored. event_type_registry.active=false, retired/inactive approval rows, retired/draft PIDX procedures, inactive legacy workflows, retired labels, or non-usable DOT statuses can satisfy readiness.
  • APPROVAL_HANDLER_UNIMPLEMENTED is surfaced, which is better than silent green, but READY_WITH_WARNINGS is too weak when a required ingredient needs an executable handler. Safest v0.1 rule: required unimplemented handler => NOT_READY; otherwise the Owner must explicitly define non-executable readiness semantics.
  • Function overloads are surfaced as READY_WITH_WARNINGS. This is acceptable only for prototype discovery and only if READY_WITH_WARNINGS never authorizes execution. Signature-specific readiness remains unverified.
  • Function inventory emits one row per overload with the same object_ref; multi-event triggers can similarly duplicate a trigger ref. Normalize inventory to one row per canonical ref with aggregate metadata.
  • READ_BLOCKED is hardcoded to two schema names instead of derived from PG privileges. Other unreadable schemas can be misreported as MISSING.
  • missing_route_procedure_code is returned as text but never resolved, despite the design saying the route target is probed.
  • declared_maturity, automation_mode, and safety_class are nullable. A null maturity can yield null readiness_drift instead of false. Use explicit defaults/not-null or COALESCE in the view.
  • ON UPDATE CASCADE ON DELETE CASCADE allows stable procedure identity changes and silently deletes ingredient relationships. Use immutable keys plus ON UPDATE RESTRICT/NO ACTION, ON DELETE RESTRICT/NO ACTION, and lifecycle retirement.
  • updated_at is not automatically maintained, so inventory freshness metadata can lie after updates.
  • Candidate DDL is not idempotent, while Điều 33 E1 requires governed schema DOTs to be idempotent and paired with a verifier. The construction package needs the eventual Cấp-B apply/Cấp-A verify contract before Owner build authorization.

6. Known issue adjudication

1. warning_flags aggregation / correlated array_agg

  • Verdict: BLOCK
  • Reason: grouped outer columns are referenced inside a correlated scalar subquery without a valid aggregate boundary; the readiness view is likely not creatable.
  • Required change: aggregate warnings in a separate per-procedure CTE (or a correctly isolated lateral subquery before final grouping), then join once.
  • Severity: BLOCK

2. readiness_drift CASE duplication

  • Verdict: CONCERN
  • Reason: the duplicate CASE already has different semantics from the main rollup and will drift as states/warnings evolve.
  • Required change: compute computed_readiness once, then compute drift from that value in the next CTE.
  • Severity: MEDIUM

3. split_part segment safety

  • Verdict: BLOCK
  • Reason: only colon presence/non-empty identifier is validated. Missing/extra dot segments and kind/prefix mismatches are not rejected and can false-exist.
  • Required change: strict per-kind prefix and exact-arity validation before probes; implement schema defaulting explicitly.
  • Severity: BLOCK

4. collection physical/logical existence

  • Verdict: BLOCK
  • Reason: readiness ignores schema for logical matches; inventory omits logical-only rows; mismatch detection is not XOR and is not propagated into readiness.
  • Required change: define public-only Directus logical semantics, create a full physical/logical normalized set, compute XOR, and reuse the same resolver logic.
  • Severity: BLOCK

5. function name-only overload handling

  • Verdict: CONCERN
  • Reason: the warning prevents bare READY, but existence of some overload does not prove the required callable signature exists.
  • Required change: keep v0.1 name-only only for discovery and prohibit READY_WITH_WARNINGS from authorizing execution; add signature grammar before executable automation depends on it.
  • Severity: MEDIUM

6. label ambiguity / facet qualification

  • Verdict: BLOCK
  • Reason: bare ambiguity is warned, but the preferred label:FACET.CODE form is not parsed and cannot resolve.
  • Required change: parse facet and code separately, resolve facet identity explicitly, and emit facet-qualified inventory refs; retain bare-code ambiguity warning.
  • Severity: HIGH

7. approval handler unimplemented

  • Verdict: CONCERN
  • Reason: it is surfaced, but a required non-executable approval can still yield READY_WITH_WARNINGS.
  • Required change: safest rule is required unimplemented handler => NOT_READY; otherwise require an Owner decision defining when declaration-only existence is enough.
  • Severity: HIGH

8. soft ingredient_ref constraints

  • Verdict: BLOCK
  • Reason: soft storage is sound, but the computed parser is incomplete and ref_status defaults to NORMALIZED, allowing bad input to bypass INVALID_REF.
  • Required change: retain liberal storage but compute parse validity independently; never trust the stored flag alone. Add an optional write-side normalizer that sets the flag, not a regex that rejects rows.
  • Severity: BLOCK

9. pidx_* object-name collision

  • Verdict: PASS
  • Reason: the 2026-06-23 read-only evidence reports all four names absent, and pidx_* avoids the existing wf_* family.
  • Required change: repeat collision/dependency preflight immediately before authorized build; fail closed if any object exists.
  • Severity: NOTE

10. rollback limited to pidx_* objects

  • Verdict: CONCERN
  • Reason: object order and scope are correct and no CASCADE is used, but unconditional drops can destroy post-build PIDX data. Approximate source counts do not prove non-damage.
  • Required change: transactional rollback with dependency/data preconditions, backup or seed-only proof, exact catalog absence checks, and source schema fingerprints/baseline evidence.
  • Severity: HIGH

7. Truth model review

Conceptual model: PASS. Candidate implementation: CONCERN.

  • PG/SQL facts are intended to be the only readiness source.
  • Ingredient Map is declaration/cache only.
  • Manifest, note, RAG/vector, and seed status cannot directly set READY.
  • Required UNKNOWN_SOURCE becomes NOT_READY; non-gating UNKNOWN_SOURCE should become READY_WITH_WARNINGS.

The implementation is not safe until parser, lifecycle, collection, warning, and zero-required fixes land. Those paths allow PG to compute the wrong truth from an invalid or incomplete probe even though declarations do not directly set readiness.

8. False READY risk review

Current or near-current false-green paths:

  1. ingredient_kind disagrees with the ref prefix, but the kind-selected probe succeeds.
  2. Extra ref segments are ignored and a truncated valid object is found.
  3. A logical Directus collection matches by name while the supplied schema is wrong.
  4. A procedure has no required ingredients but optional ingredients exist.
  5. A required object row exists but is inactive, retired, disabled, draft, or otherwise unusable.
  6. A required approval action exists but its handler is null/unimplemented.
  7. A required function name exists but only the wrong overload/signature exists.
  8. Required collection logical/physical mismatch is not propagated into readiness.
  9. Warning-producing optional failures can yield READY_WITH_WARNINGS with an empty top-level flag list, inviting consumers to treat it as green.
  10. A stored ref_status='NORMALIZED' is trusted despite malformed per-kind structure.

Required mitigation: one strict parser/resolver output containing parse validity, existence, usability, authority, freshness, and warnings; roll up only that output.

9. Ref grammar review

CONCERN — smallest required patch:

  • Require prefix equality: parsed kind must equal ingredient_kind.
  • Require exact identifier arity per kind; reject extra as well as missing segments.
  • Normalize/default CATALOG schema to public before storage/probe, or reject noncanonical storage consistently. Do not document both behaviors.
  • Implement label:<facet>.<code> against both facet and code and emit the same canonical ref in inventory.
  • Treat unsupported quoted identifiers/dots inside identifiers as INVALID_REF in v0.1 rather than guessing.
  • Derive READ_BLOCKED from privileges/source accessibility, not a two-schema hardcode.
  • Keep function name-only temporarily only with mandatory overload warning and a non-authorizing READY_WITH_WARNINGS contract.

10. Performance review

PASS WITH VERIFICATION REQUIRED

  • The intended path is narrow: procedure PK -> indexed ingredient rows -> per-kind point probes.
  • The readiness SQL does not reference the full inventory view, so it does not inherently materialize all catalog branches.
  • PostgreSQL 16 can inline side-effect-free CTEs and push a procedure filter, but the actual plan remains UNVERIFIED until the objects exist.
  • Repeated function count/approval/label probes add a small constant factor per ingredient, not an all-catalog scan.
  • Full inventory browsing legitimately scans its branch sources; function and field branches should not be on the hot path.

Build acceptance must include EXPLAIN (ANALYZE, BUFFERS, false where appropriate) for one procedure and a multi-procedure browse, with evidence that the inventory view is not scanned for the readiness hot path.

11. Seed and test plan review

CONCERN

The 9 procedures cover the named states at a basic level, but the suite is not sufficient for false-green safety.

Add fixtures/assertions for:

  • kind/prefix mismatch;
  • too few and too many ref segments with ref_status='NORMALIZED';
  • zero required but one optional-existing ingredient;
  • facet-qualified label success and bare ambiguous label warning;
  • logical-only, physical-only, both, and wrong-schema collections;
  • inactive/retired objects for every lifecycle-bearing kind;
  • overloaded function at readiness grain;
  • required unimplemented approval behavior;
  • warning flag completeness for optional missing/unknown and collection mismatch;
  • route-target existence;
  • null/default column behavior.

Test-plan defects:

  • S4 claims to require at least one required ingredient but checks only total ingredient_count; it misses the zero-required false green.
  • T11 says the plan is SELECT-only but requires declaring a scratch ingredient, which is DML. Pre-seed a controlled fixture in the authorized seed slice or mark this as a separate transactional fixture step.
  • S2 expects >=0 rows, which proves nothing.
  • ingredients is a jsonb array value, not SQL type jsonb[] as stated in the output contract.

12. Rollback/safety review

CONCERN

  • Drop order is correct: readiness view -> inventory view -> child table -> parent table.
  • Scope is correctly limited to the four PIDX objects; indexes, comments, and owned identity sequence disappear with their tables.
  • No CASCADE is used, so unexpected dependencies fail closed.

Before authorization, add:

  • pre-build object collision and dependency snapshot;
  • transactional rollback wrapper and explicit STOP on unexpected rows/dependents;
  • backup/seed-only proof before dropping tables;
  • exact object-definition fingerprints rather than approximate source row counts;
  • post-rollback catalog comparison proving only the four PIDX objects changed.

Also replace FK cascades with restrictive lifecycle-safe behavior.

13. Required patches

Required BLOCK/HIGH patches are listed in:

knowledge/dev/laws-new/workflow-manage/reports/codex-pidx-required-patches-2026-06-23.md

No source design file was modified by this review.

14. Final recommendation

Patch design then re-review.

Do not proceed to Owner build authorization yet. Preserve the 2T2V architecture and make the bounded resolver/rollup/rollback/law-conformance patches. After a revised candidate SQL passes static review and the expanded test plan has exact expected outputs, proceed to Owner authorization for a governed prototype build through a paired apply/verify DOT path.

15. INCOMEX step report and evidence

  • Step 0 — Foundation: read skill, OR v7.58, Constitution v4.6.3, foundation law v3.3, anti-patterns, Điều 20, Điều 33, and dependency law through the main process.
  • Step 1 — Mission: one review-only objective; no DDL/DML, source design edit, PG/Directus/DOT/governance mutation, production write, background agent, or subagent.
  • Step 2 — Design checkpoint: target DB = directus; layer = Kho; assembly gate = reuse PG catalog/registries and review existing candidate SQL; Nuxt = N/A; code/build = prohibited.
  • Step 3 — Review: inspected all required design and SQL documents. No source package changes.
  • Step 4 — Coder hat: checked schema, resolver, rollup, indexes, seeds, tests, and rollback as construction artifacts.
  • Step 4 — Reviewer hat: rechecked every known issue, all Q1–Q7 questions, false-green paths, conservation law, and PostgreSQL grouped-query/string parsing rules.
  • Step 5 — Verification: production execution is N/A and prohibited because the four objects do not exist and this is review-only. Evidence is document revision/readback plus local report coverage checks; no claim of production PASS is made.
  • Step 6 — Close: main report and required patch list written. OR update: not required—no new operating principle identified. TD/handoff: the patch list is the explicit carry-forward; no separate governance or handoff mutation is authorized by this review.

Actual local verification output:

$ wc -l codex-pidx-full-review-2026-06-23.md codex-pidx-required-patches-2026-06-23.md
313 codex-pidx-full-review-2026-06-23.md
123 codex-pidx-required-patches-2026-06-23.md
436 total

$ rg required report headings
3:## 1. Executive verdict
13:## 2. Files read
48:## 3. Problem statement alignment
66:## 4. Technical design alignment
88:## 5. Construction design / SQL review
120:## 6. Known issue adjudication
192:## 7. Truth model review
203:## 8. False READY risk review
220:## 9. Ref grammar review
232:## 10. Performance review
244:## 11. Seed and test plan review
271:## 12. Rollback/safety review
289:## 13. Required patches
297:## 14. Final recommendation
303:## 15. INCOMEX step report and evidence

$ rg known issue headings
122:### 1. warning_flags aggregation / correlated array_agg
129:### 2. readiness_drift CASE duplication
136:### 3. split_part segment safety
143:### 4. collection physical/logical existence
150:### 5. function name-only overload handling
157:### 6. label ambiguity / facet qualification
164:### 7. approval handler unimplemented
171:### 8. soft ingredient_ref constraints
178:### 9. pidx_* object-name collision
185:### 10. rollback limited to pidx_* objects