KB-2522

Codex Static Re-Review — PIDX v0.2

23 min read Revision 1
workflow-managepidxv0.2static-reviewcodex2026-06-23

Codex Static Re-Review — PIDX v0.2

1. Executive verdict

PASS_WITH_CAVEATS_REQUIRES_PATCH

v0.2 resolves all five v0.1 P0/BLOCK parser, collection, label, zero-required, and warning-aggregation defects at the design level. The 2T2V architecture remains coherent and thin.

The package is not yet ready to proceed directly to Owner build authorization. Remaining HIGH issues are bounded patches, not an architectural redesign:

  1. lifecycle/usability does not gate the procedure row whose readiness is being reported, and unknown lifecycle values can still satisfy a required ingredient without warning;
  2. immutable identity and retire-only behavior remain policy statements rather than enforced infrastructure;
  3. rollback does not actually abort on non-seed data and cannot prove ingredient provenance;
  4. the governed build/access/idempotency contract is not complete;
  5. the deterministic test expectations contain a 12-vs-13 row count contradiction and miss the remaining false-green cases.

Final gate: patch v0.2 into v0.3, then perform one more static review before Owner build authorization discussion.

2. Files read

Read through Agent Data MCP in the main process:

Foundation/rules:

  • .claude/skills/incomex-rules.md — 36 items / 8-step workflow (local 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/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 prior findings:

  • knowledge/dev/laws-new/workflow-manage/reports/codex-pidx-full-review-2026-06-23.md — revision 1.
  • knowledge/dev/laws-new/workflow-manage/reports/codex-pidx-required-patches-2026-06-23.md — revision 1.

Required v0.2 package:

  • knowledge/dev/laws-new/workflow-manage/design/pidx-build-design-v0.2.md — revision 1.
  • knowledge/dev/laws-new/workflow-manage/design/pidx-ddl-candidate-v0.2.sql.md — revision 1, full 46,568 characters read in bounded chunks.
  • knowledge/dev/laws-new/workflow-manage/design/pidx-readiness-logic-v0.2.md — revision 1.
  • knowledge/dev/laws-new/workflow-manage/design/pidx-seed-slice-v0.2.md — revision 1.
  • knowledge/dev/laws-new/workflow-manage/design/pidx-test-plan-v0.2.md — revision 1.
  • knowledge/dev/laws-new/workflow-manage/design/pidx-codex-review-packet-v0.2.md — revision 1.
  • knowledge/dev/laws-new/workflow-manage/reports/pidx-build-design-patch-round1-2026-06-23.md — revision 1.
  • knowledge/dev/laws-new/workflow-manage/reports/pidx-build-design-go-no-go-v0.2.md — revision 1.

The canonical problem statement was already read in the immediately preceding v0.1 review and was not re-opened because v0.2 introduces no philosophical contradiction.

External primary reference checks used PostgreSQL 16 documentation for constraints/FKs, schema privileges, table functions/aliases, arrays/cardinality, and system privilege functions.

3. v0.1 finding resolution matrix

P0-1 — warning_flags aggregation / PostgreSQL grouping safety

  • v0.1 issue: correlated warning aggregation referenced grouped outer columns and likely prevented view creation.
  • v0.2 resolution: t7.warns produces per-row arrays; proc_warn aggregates by procedure_code; roll joins once.
  • Review verdict: RESOLVED
  • Remaining severity: LOW
  • Required action: make the table-function output alias explicit (LATERAL unnest(t7.warns) AS u(w)) to remove scalar/whole-row alias ambiguity before execution.

P0-2 — strict ref parser / prefix / arity / INVALID_REF precedence

  • v0.1 issue: prefix mismatch and missing/extra segments could reach a probe and false-exist.
  • v0.2 resolution: t1t4 enforce first-colon tokenization, prefix=kind, exact per-kind arity, no empty segments, and validity independent of stored ref_status; invalid refs cannot set found=true.
  • Review verdict: RESOLVED
  • Remaining severity: NOTE
  • Required action: optionally reject additional : characters for CODE kinds to make the grammar fully closed; current behavior fails missing rather than green, so this is not blocking.

P0-3 — collection logical-vs-physical false EXISTS

  • v0.1 issue: Directus logical name ignored schema and could create false EXISTS; logical-only rows were omitted.
  • v0.2 resolution: physical base table is existence truth; logical presence is public-only and separate; XOR mismatch is surfaced; logical-only becomes MISSING; inventory uses a FULL OUTER JOIN.
  • Review verdict: RESOLVED
  • Remaining severity: LOW
  • Required action: document that inventory may intentionally contain exists_bool=false logical-only rows, which differs from the original “one inventory row means exists” contract.

P0-4 — label facet grammar / AMBIGUOUS_LABEL

  • v0.1 issue: facet-qualified refs were compared as a single code and false-missed.
  • v0.2 resolution: readiness correctly probes facet_id::text + code; bare codes warn when multiplicity >1; inventory emits facet-qualified refs.
  • Review verdict: RESOLVED
  • Remaining severity: MEDIUM
  • Required action: inventory currently attaches AMBIGUOUS_LABEL to each facet-qualified row when a code is duplicated, while readiness warns only for a bare ref. Align warning semantics. Current live data has no duplicate codes, so this is inert today.

P0-5 — zero-required false READY

  • v0.1 issue: optional-only procedures could reach READY.
  • v0.2 resolution: required_count=0 -> UNMAPPED precedes all green branches.
  • Review verdict: RESOLVED
  • Remaining severity: NOTE
  • Required action: keep S1 invariant and correct the aggregate seed counts in T18.

P1-1 — lifecycle/usability predicates

  • v0.1 issue: existence was treated as usability.
  • v0.2 resolution: lifecycle-bearing ingredient sources receive tri-state usable; proven false blocks a required ingredient.
  • Review verdict: PARTIAL
  • Remaining severity: HIGH
  • Required action:
    • gate the top-level pidx_procedure.status; currently a draft/retired procedure can have satisfied ingredients and return READY;
    • do not allow NULL/unrecognized lifecycle values on lifecycle-bearing sources to yield bare READY. Emit SOURCE_USABILITY_UNKNOWN and at least downgrade to READY_WITH_WARNINGS, or treat required unknown usability as unsatisfied;
    • add required inactive-event/retired-procedure tests, not only an optional inactive event.

P1-2 — warning completeness and propagation

  • v0.1 issue: READY_WITH_WARNINGS could have an empty causal warning list.
  • v0.2 resolution: optional failure flags and a per-procedure distinct warning aggregate were added; the READY/RWW invariant is structurally sound.
  • Review verdict: PARTIAL
  • Remaining severity: MEDIUM
  • Required action: an inactive approval with an implemented handler is currently labeled APPROVAL_HANDLER_UNIMPLEMENTED because the warning checks only usable=false. Split handler absence from lifecycle unusability. Also add an unknown-usability flag and align inventory/readiness warning semantics.

P1-3 — approval handler gating

  • v0.1 issue: required unimplemented approval could be READY_WITH_WARNINGS.
  • v0.2 resolution: unimplemented handler yields usable=false; required ingredient becomes NOT_READY.
  • Review verdict: RESOLVED
  • Remaining severity: NOTE
  • Required action: Owner should affirm the safe rule; do not weaken it to READY_WITH_WARNINGS for required ingredients.

P1-4 — identity/delete safety

  • v0.1 issue: cascade update/delete and mutable natural identity.
  • v0.2 resolution: surrogate identity and RESTRICT/RESTRICT FK remove cascade loss.
  • Review verdict: PARTIAL
  • Remaining severity: HIGH
  • Required action: “immutable by policy” is not an infrastructure gate. A zero-ingredient procedure can still be deleted, and an unreferenced procedure_code can still be updated. The governed write role must lack direct DELETE/code-update ability; retirement and immutable-code behavior require grants/gateway enforcement plus a paired verifier.

P1-5 — Điều 33 / Directus registration / classification

  • v0.1 issue: table classification and canonical shape were unresolved.
  • v0.2 resolution: recommends PG-native technical-index classification and adopts most canonical fields.
  • Review verdict: NEEDS_OWNER_DECISION
  • Remaining severity: HIGH
  • Required action: Owner must choose PG-native technical index versus governed Directus entity before build. If governed, the delta is more than “one-line rename”: confirm procedure_code versus canonical code, plural table naming, child-table classification, Directus registration, permissions, and lifecycle enforcement.

P1-6 — rollback hardening

  • v0.1 issue: unconditional destructive drops and weak evidence.
  • v0.2 resolution: adds preflight queries, a transaction, external-view dependency check, and a name-set fingerprint.
  • Review verdict: NOT_RESOLVED
  • Remaining severity: HIGH
  • Required action:
    • P-3 is advisory only; the transaction does not abort when non-seed data exists;
    • ingredient rows have no provenance column, so “non-seed ingredient” cannot be determined;
    • the dependency preflight covers pg_rewrite view dependencies, not all dependency classes (DROP without CASCADE still provides a final fail-closed backstop);
    • the MD5 is only a relation-name-set hash, not an object-definition/catalog fingerprint. Add enforced data/provenance guards, backup proof, and structural fingerprints.

P1-7 — deterministic test coverage

  • v0.1 issue: hidden DML, conditional/meaningless expectations, and missing negative cases.
  • v0.2 resolution: 13 seeded procedures, 19 read-only tests, and 6 invariants cover the main parser/collection/warning paths.
  • Review verdict: PARTIAL
  • Remaining severity: HIGH
  • Required action:
    • correct the rollup totals: the table yields READY×3, READY_WITH_WARNINGS×2, NOT_READY×6, UNMAPPED×2 = 13; the seed/test documents say NOT_READY×5 and total only 12;
    • add top-level draft/retired procedure and unknown lifecycle-value tests;
    • test required inactive lifecycle directly;
    • T13 claims both facet-qualified and bare resolution but seeds/tests only the facet-qualified ref;
    • add a build-time assertion for object access/grants and the exact complete view definitions.

P2-1 — READ_BLOCKED / privilege-derived handling

  • v0.1 issue: fixed schema-name list.
  • v0.2 resolution: derives schema USAGE through the OID form of has_schema_privilege; missing schema does not error.
  • Review verdict: PARTIAL
  • Remaining severity: MEDIUM
  • Required action: schema USAGE alone does not prove object SELECT/EXECUTE visibility. Define whether READ_BLOCKED is schema-only or derive object-level privileges; otherwise an accessible schema with an unreadable object can false-MISSING.

P2-2 — missing-route handling

  • v0.1 issue: route target existence was not exposed.
  • v0.2 resolution: missing_route_exists is included in ingredient JSON.
  • Review verdict: PARTIAL
  • Remaining severity: LOW
  • Required action: rename the field to clarify it checks PIDX-row existence only, or also expose route usability/status. A retired target currently returns true.

P2-3 — readiness drift logic

  • v0.1 issue: duplicated rollup CASE could diverge and produce nullable drift.
  • v0.2 resolution: readiness computed once in roll; drift derived outside with COALESCE.
  • Review verdict: RESOLVED
  • Remaining severity: NOTE
  • Required action: none beyond the top-level lifecycle patch.

P2-4 — inventory deduplication

  • v0.1 issue: duplicate canonical refs for overloaded functions and multi-event triggers.
  • v0.2 resolution: GROUP BY canonical identity with signatures/event arrays.
  • Review verdict: RESOLVED
  • Remaining severity: LOW
  • Required action: add a deterministic trigger dedup assertion; current test verifies only functions.

P2-5 — links_jsonb wording / non-M2M scope

  • v0.1 issue: design referenced a nonexistent links_jsonb column.
  • v0.2 resolution: removes the claim and limits links to procedure: ingredients or missing routes.
  • Review verdict: RESOLVED
  • Remaining severity: NOTE
  • Required action: none.

4. SQL static review

CONCERN — complete, mostly plausible, not execution-ready.

Completeness:

  • two complete CREATE TABLE statements: PASS;
  • three complete indexes: PASS;
  • complete readiness view: PASS;
  • complete 10-kind inventory view: PASS;
  • no abbreviated branches / “same as above”: PASS;
  • rollback and verification SQL present: PASS, semantics incomplete as noted;
  • complete ownership/GRANT/REVOKE/security contract: FAIL;
  • idempotent governed apply wrapper: FAIL (described in prose only).

Static SQL observations:

  • Strict parser ordering and exact arity are coherent.
  • event first-dot recovery is coherent for dotted event types.
  • OID-form has_schema_privilege is supported by PostgreSQL 16.
  • FK-to-UNIQUE is valid PostgreSQL.
  • array_remove(...,NULL) plus cardinality is coherent for empty warning arrays.
  • FULL OUTER JOIN logic is coherent assuming directus_collections.collection is unique.
  • Aggregate trigger/function branches are syntactically plausible.
  • Full CREATE VIEW parsing remains UNVERIFIED because no complete view was created or prepared; the reported live probe omitted the real tables/final view assembly.
  • Make unnest's output column explicit in proc_warn to eliminate alias/type ambiguity.
  • No object owner, SELECT grant, write-role restriction, column-update restriction, or view security mode is defined. The proposed context_pack_readonly post-build tests may therefore be blocked or may run under unintended default privileges.
  • Candidate CREATE statements themselves are not idempotent; idempotency is deferred to an unspecified future DOT wrapper.

5. False READY risk review

Resolved false-green paths:

  • required_count=0;
  • optional-only procedures;
  • prefix mismatch;
  • missing/extra/empty ref segments;
  • logical-only collection;
  • required unimplemented approval handler;
  • overloaded function produces a warning;
  • required UNKNOWN_SOURCE, MISSING, INVALID_REF, and schema-level READ_BLOCKED.

Remaining paths:

  1. Top-level retired/draft procedure: roll ignores a.status, so its own ingredient set can yield READY.
  2. Unknown lifecycle value: lifecycle-bearing usable=NULL satisfies because usable IS NOT FALSE; no warning is emitted. This includes null/unrecognized DOT/approval/workflow/label status and nullable event active.
  3. Object-level unreadable source: schema USAGE can be true while the object is not readable; information_schema may hide it and cause MISSING instead of READ_BLOCKED (false negative, not green, but wrong truth).
  4. Bare ambiguous label usability: LIMIT 1 without ordering can pick different lifecycle rows. The warning prevents bare READY, but readiness may nondeterministically toggle between RWW and NOT_READY.
  5. Inactive approval warning cause: false usability is always labeled handler-unimplemented, hiding lifecycle truth.

Required invariant after patch:

READY => procedure.status=active
      AND required_count>0
      AND every required ref is structurally valid, readable, found
      AND every lifecycle-bearing required source is proven usable
      AND warning_flags={}

6. Scope creep review

2T2V: PASS. Seed classification: ACCEPTABLE_TEST_FIXTURE with conditions.

Still only:

  • pidx_procedure;
  • pidx_procedure_ingredient;
  • v_pidx_inventory_current;
  • v_pidx_procedure_readiness.

No procedure-link table, workflow engine, scheduler, bus, materialized view, RAG/vector, pg_trgm, Nuxt, governance/birth/KG dependency, auto-execution, auto-fix, auto-register, auto-schema, or auto-approval was added.

The increase 9→13 is justified for parser/lifecycle/collection fixtures and is not architectural scope creep. Before persistent prototype seeding, Owner must decide whether four PROC_FIXTURE_* rows may live as active public PIDX procedures or should exist only in a controlled dry-run fixture phase. Fixture persistence must not become operational truth.

7. PG-as-truth / no-new-truth review

PASS WITH CAVEAT.

  • Readiness uses PG/catalog/registry facts, not manifest, note, RAG, vector, agent memory, or declared maturity.
  • Ingredient Map remains a lazy declaration/cache.
  • Missing classifications stay UNMAPPED; unknown sources stay UNKNOWN_SOURCE.
  • PIDX does not enrich source registries or implement governance.

Caveat: the hardcoded usability vocabularies are interpretations of source lifecycle truth. Unknown values currently become silent usable=NULL and may green. The source vocabulary must be complete and verified or unknown usability must be explicitly surfaced; PIDX must not invent a favorable interpretation.

8. LEGO / no-hardcode / automation / DOT discipline review

PASS WITH CAVEATS.

  • LEGO separation is preserved across tables and CTE stages.
  • Hardcoded values are mostly minimal contracts: kind list, grammar, statuses, warning names.
  • No readiness outcome is hardcoded for specific objects; seed refs are controlled fixtures.
  • Automation remains seeing/checking/routing only.
  • Mutation remains Owner-gated in principle.

Caveats:

  • Lifecycle status mappings are operational semantics and need a source-backed contract or fail-closed unknown handling.
  • Governed apply/verify is asserted but not specified as executable, idempotent candidate artifacts.
  • “Immutable by policy” violates the “cannot make the mistake” infrastructure standard until permissions/gateway/verifier enforce it.

9. Test plan review

PARTIAL — strong negative-path expansion, not yet sufficient.

Covered deterministically: EXISTS, MISSING, UNKNOWN_SOURCE, INVALID_REF, schema READ_BLOCKED, READY, RWW, NOT_READY, UNMAPPED, drift, collection mismatch, approval handler, function overload, zero-required, prefix mismatch, segment arity, warning non-empty invariant.

Gaps/defects:

  • rollup summary count is arithmetically wrong (NOT_READY is 6, not 5);
  • no test for top-level procedure lifecycle;
  • no test for unknown/unrecognized lifecycle values;
  • inactive event is optional, so required-not-usable is only tested through approval;
  • bare-label success is claimed but not seeded in the view test;
  • AMBIGUOUS_LABEL logic remains unexercised (current-data N/A is proven, which is acceptable if documented as untested behavior);
  • no deterministic trigger-dedup test;
  • no privilege/grant/view-security test;
  • no static parser proof for the complete CREATE VIEW statements.

10. Rollback/safety review

NOT SUFFICIENT FOR NEXT GATE.

Positive changes:

  • correct drop order;
  • transaction;
  • no CASCADE;
  • unexpected view dependents checked;
  • PIDX-only object names;
  • source readability checks.

Blocking gaps:

  • non-seed data check does not abort rollback;
  • ingredient provenance is unavailable;
  • “backup if needed” is a comment, not a verified prerequisite;
  • dependency query is not comprehensive;
  • relation-name hash is not a catalog/object-definition fingerprint;
  • pre-build versus post-rollback comparison mechanism and evidence storage are unspecified.

11. Owner-decision items before build

  1. Điều 33 classification: choose PG-native technical index or governed Directus entity.
  2. Directus registration: if technical index, explicitly approve PG-only prototype status and access path; if governed, approve canonical rename/registration/permissions plan.
  3. Approval handler rule: affirm v0.2 safe rule—required unimplemented handler is NOT_READY.
  4. Seed 13: approve the count and decide whether PROC_FIXTURE_* rows persist or exist only in controlled dry-run fixtures.
  5. Governed build route: recommend a registered migration DOT with a paired Cấp-A verifier; patch_ops_code APR may authorize that migration but should not replace the repeatable migration artifact.
  6. UNKNOWN_SOURCE acceptance: confirm io/checker/template/report remain non-green and do not block the prototype itself.
  7. Access model: choose owner/write/read roles, view security mode, grants, immutable-code/delete restrictions, and how context_pack_readonly receives SELECT.

These decisions are identified only; this report does not request or grant authorization.

12. Required patches

BLOCK/HIGH patches remain and are listed in:

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

No v0.2 source design file was modified.

13. Final recommendation

Patch v0.2 into v0.3 then re-review.

Preserve 2T2V. Patch lifecycle gating, infrastructure-enforced identity/delete rules, rollback data/provenance guards, access/idempotent governed-build contract, and deterministic tests. Then perform a final static review before proceeding to Owner build authorization discussion.

14. INCOMEX step report and evidence

  • Step 0: read skill, OR v7.58, Constitution v4.6.3, foundation/Điều 20/Điều 33/dependency laws.
  • Step 1: one static re-review objective; no background agent/subagent, DDL/DML, source edit, or production mutation.
  • Step 2: target DB=directus, layer=Kho, Assembly First=review PG-native candidate; Nuxt/build=N/A and prohibited.
  • Step 3: read all required v0.1/v0.2 documents and all 46,568 characters of candidate SQL.
  • Step 4 coder hat: checked tables, indexes, full CTE resolver, both views, seeds, tests, rollback.
  • Step 4 reviewer hat: independently rechecked every P0/P1/P2 item and new false-green/access/rollback risks.
  • Step 5: no production verification claimed; full build objects do not exist and DDL/DML was prohibited. Evidence is MCP revision/content readback, PostgreSQL 16 primary documentation, and local report structure checks.
  • Step 6: report and required patch list written/uploaded. OR update not needed—no new operating principle. Patch list is the explicit TD/handoff; no separate governance mutation is authorized.

Actual local coverage output before upload:

$ wc -l -c codex-pidx-v0.2-static-review-2026-06-23.md codex-pidx-v0.2-required-patches-2026-06-23.md
382 22663 codex-pidx-v0.2-static-review-2026-06-23.md
 70  3889 codex-pidx-v0.2-required-patches-2026-06-23.md
452 26552 total

$ rg '^### P[012]-' main-report
P0 items: 5
P1 items: 7
P2 items: 5
Total adjudicated: 17

$ rg required headings main-report
Sections 1 through 14: present