# P1-1b O10 independent implementation reviews ## Piece D attempt #2 integration review — Accept **Date:** 2026-08-07 **Branch:** `feature/p1-1b` **Reviewed candidate:** `a4d40aac429584baed3481d62c2f859dd4ad2860` **Approved base (remediation approval tip):** `a0529093838b2fd95d40c5057db6dd896e9b85fc` **Rejected candidate #1 (remediation base for inventory):** `bf5926e3bba9afca394baf669f95c72506886056` **Verdict:** **Accept** — 0 Critical, 0 High, 0 Medium, **1 Low (D-A2-L1)** This is the fresh, separately authorized independent integration-focused review required by Piece D ordered instruction 4, run in an exclusively-review session. Nothing in the implementer's summary was taken on trust; every figure below was independently measured in this session. No merge was performed and Scope 1 was not started. ### Exact topology and scope - `HEAD`, `origin/feature/p1-1b`, and live `ls-remote` all equalled the candidate at entry; divergence `0/0`; working tree clean; exactly one worktree registered. - The exact base-to-candidate delta is **exactly three paths**: `tests/Sibyla.Tests/Persistence/P11aDisposableDatabaseTests.cs`, `docs/p1-1b-status.md`, and `docs/PROJECT-STATE.md`. No migration, DDL, `src/`, Designer, ModelSnapshot, script, importer, prototype, baseline, pin, O8/O9/O10, item 5, Scope 1, or O5 path changed. - All **eight** accepted migration/SQL blobs are byte-identical across base, candidate, and worktree (`497ce4ec…`, `74c5912c…`, `a5ff7804…`, `00b4bc2a…`, `d0beb8aa…`, `bc9d9582…`, `0a111d0c…`, `398887002…`). - Measured on the base→candidate diff, the **only deleted line in the whole test file is the class declaration**; every other pre-existing line is untouched. The four regression methods are therefore byte-identical to the approved base — a stronger result than the normalized-hash claim on record. - `RejectTerminalDOCFLGMutationFn` is unchanged in source (blob-identical `P11bPieceCSchemaSql.cs`) and proven stable live at every cycle state: S0/S2 `525` / `48af12cf84952c279610fea3909ed6c83a6b866fa9f63bf69e10ea01427e8283`; S1/S3 `1107` / `f9bfc42f4499aa6f2b96cf267d9b2919f9229757af6f5985048327e4afbff391` — reproduced exactly. ### Inventory — measured, against the correct base Because the Reject #1 neutralization restored the test file to the pre-candidate blob, the approved inventory is measured against rejected candidate `bf5926e…`, exactly as the status record states. Measured `[Fact]` counts: base `a052909…` **55**, rejected candidate `bf5926e…` **61**, candidate `a4d40aa…` **61**; added `[Fact]` lines in the remediation delta: **0**. The candidate is therefore exactly **0 new test methods, 1 new helper (`CreatePieceDPristineP11aOracleDatabaseAsync`), 1 new fixture (`PieceDPristineP11aOracleFixture`), 1 strengthened shared method, and 4 unchanged regressions** — the approved inventory, exactly. ### D-R1 — closed at the root, and adversarially proven non-vacuous The root cause is genuinely removed, not patched. The rejected candidate captured S0 from the cycle database *after* `EnsureMigrated` to full tip followed by `Down`; the remediation deletes exactly that sequence and takes S0 from the fixture. S0 now originates in a separate database (`sibyla_piece_d_oracle_`, distinct from cycle database `sibyla_p11a`), created empty and migrated **directly from empty only through `20260805180000_P11aFdrSchema`**, with the history asserted to end at the P1-1a tip and to contain none of the three Piece A/B/C migration ids. It is captured in class-fixture `InitializeAsync` — before any `[Fact]` — while the cycle database is asserted empty, so no A+B+C `Up` can precede it. S0 is held in `private set` properties and is never recaptured or rebased. **Equality was not accepted as proof of itself.** Four original adversarial probes were written, executed, and fully removed before this verdict: | Probe | Injected fault | Result | |---|---|---| | **A** | One extra table created in the oracle after its P1-1a migration | Combined method **fails** at `Assert.Equal(S0.Logical, S2.Logical)`, diff pointing at `column\|public\|ReviewProbeAResidue\|Id` | | **B** | A cluster-global role created after `Down`, before S2 | Combined method **fails**; S2 carries `role\|review_probe_b_leaked_role` absent from S0 | | **C** | `ImportEvidenceRow` constraint query forced to return nothing at *every* checkpoint (all four equal, and empty) | **Fails** at the checkpoint assertion, `Expected: 4, Actual: 0` — the exact degenerate case D-R2 warned of | | **D** | Hostile pre-existing `sibyla_piece_c_executor` seeded into a fresh cluster | **All 61 Facts fail, 0 pass, in 139 ms** — the fixture fails closed and provably initializes before every Fact | Probe A proves the oracle's independence is load-bearing; Probe B proves S2 genuinely exposes leaked cluster-global state per D-R1-O2; Probe D proves D-R1-O3's fail-closed gate is real and is not circumvented. D-R1 is closed. ### D-R2 — closed `ImportEvidenceRowUniqueConstraints` and `ImportEvidenceRowIndexes` are reused unchanged. `AssertFourAcceptedImportEvidenceConstraints` is invoked at **all four** checkpoints (S0, S1, S2, S3) and asserts cardinality exactly 4 plus the four exact `pg_get_constraintdef` values, followed by dictionary equality across checkpoints and index-set equality. Probe C proves the checkpoint assertions fire independently of the equality chain, so equality can no longer substitute for them. Per D-R2-O1, observed names are compared only as within-run stability evidence and no generated-name contract is created. ### Approved observations and mapping amendments — conformance - **D-R1-O1:** one PostgreSQL 17 invocation, two isolated databases; observed live as `cycle=sibyla_p11a; oracle=sibyla_piece_d_oracle_f09dd285…; both initially empty=true`. - **D-R1-O2:** fixed order, unique GUID database name, fail-safe `DROP DATABASE … WITH (FORCE)` cleanup, no parallel execution (single class, xUnit runs methods in a class sequentially), no S0 recapture. Probe B proves the leak-detection purpose is real. - **D-R1-O3:** statically confirmed — `Category=P11aDatabase` occurs **exactly once in the entire repository**, on `P11aDisposableDatabaseTests`; the canonical runner supplies one shared connection string to that class; no other class can mutate the cluster first. `RequireOrSkipDisposableConnection` always fails (never skips), so the fixture fails closed. Probe D confirms it empirically. - **D-R2-O1:** conformant, as above. - **Option A:** physical `attnum` is captured, emitted as a separate diagnostic for S1 and S3, and excluded **only** from S1/S3 logical equality; S0/S2 asserts logical **and** physical equality; the determinism test asserts both artifacts. No curated roster or lossy count anywhere. - **D-M2-A:** rows start `Open`; one atomic `Open`→terminal `UPDATE` sets `ItemClass='Bogus'` with `Status`, `ResolutionEvidence`, `ResolvedBy`, `ResolvedAt`; asserts SQLSTATE `23503` and exact constraint `DOCFLG_EFCode_ItemClass_ReviewPriority_BlockingLevel_fkey`; the NULL/`MATCH SIMPLE` proof stays on an imported `Open` row. - **D-M3-A+:** the fixture is imported (`ImportEvidenceRowId` set), terminal, evidence starting NULL; first forged-GUC append succeeds, second fails `23514` / `terminal DOCFLG rows are append-only`; no `P11aCommandAudit` row, provenance unchanged, first value intact, transaction rolled back. ### The test-case orderer — assessed, and judged within the approved inventory The orderer is implemented as an interface on the already-approved fixture (no new class, no new Fact) plus one attribute. **Probe E** removed the `[TestCaseOrderer]` attribute and ran the full suite: the combined method failed on `role|sibyla_piece_c_non_owner_probe`, independently reproducing the implementer's honestly-reported 60/61 observation. The orderer is therefore **load-bearing, not gratuitous**, and it implements D-R1-O2's explicitly approved "execution order is fixed". Critically, it does not weaken the proof: Probe B shows a role leaked by the `Down` itself is still caught. The alternative — excluding roles from the snapshot — is explicitly forbidden. This is **not a finding**. ### Independent reproduction | Verification | Result | |---|---:| | Clean Release rebuild (`dotnet clean` + `--no-incremental`) | **0 warnings / 0 errors** | | Ordinary suite | **646/646** | | Registry (`RegistryValidationServiceTests`) | **33/33** | | Focused strengthened Piece D method | **1/1** | | Four exact unchanged regressions | **4/4** | | `scripts/run-p11a-disposable-tests.ps1`, PostgreSQL **17.10** | **61/61** | | `git diff --check` working / cached | **clean / clean** | | Residual / running `sibyla-p11*` containers | **0 / 0** | Console diagnostics reproduced the recorded values exactly: history S0/S2 `71112d8e521d4c97c01bcc11f4f66e23bc5eea82176c191d347c66d25e9f7e23`, S1/S3 `f9e37bb2c2ce97bc3781e1727a5849fb97bae218b6a13697a33028072f7035b2`, both function hashes, and the four constraint names. **A stale-assembly trap was hit and root-caused rather than reported as a result.** After the probes, restoring the file with `Copy-Item` also restored its *older* timestamp, so MSBuild skipped recompilation and a canonical run reused the Probe-E assembly, yielding 60/61 with the Probe-E signature. The source blob was verified still `c5915524…`, the timestamp refreshed, the test assembly rebuilt with `--no-incremental`, and the suite re-run: **61/61**. Ordinary and Registry were re-run on that same final build. All figures above are from genuinely current binaries. ### Independent amendment-4 preflight — run fresh this session Read-only against prototype clone `D:\fileStorage\repos\invoice-skill-build` (`git status`, `git diff --quiet`, `git fetch`, `git cat-file -e`, `git rev-parse`, `git merge-base --is-ancestor`, `git ls-tree`, `git log --oneline` only; never checkout/pull/merge; no post-pin content read): 1. Pin `b91768513fc638381fbde91f0b576b08220a98f6` resolves locally and in the fetched remote. **Pass.** 2. Pin is an ancestor of live `refs/heads/main` = `3dd4150caef7e3a1d2a77c5fa34361d2aefe4c54`. **Pass.** 3. Local clone `HEAD` equals the pin exactly; tree clean. **Pass.** 4. The governed 49-blob roster (48 `Editor/Data/*.json` plus `Backups/Control Log.md`, with the O8 `entbnk.json` substitution to `70d418f6023bff68f0d642b4aff26c1ead1298be`) measured **cell by cell** via `git rev-parse :`: **49/49 resolved, 0 mismatches, 0 resolve failures.** **Pass.** 5. Direct `Editor/Data` surface at the pin: **49 entries, exactly 1 `Backups` tree, 48/48 blob files, 0 delta.** **Pass.** 6. Divergence recorded as metadata only, identical to the record: live tip `3dd4150…`, post-pin commits `1e841a4` (*Complete Stage 10 Round 8 reconciliation and mapping updates*) and `3dd4150` (*Apply Stage 10 Round 8 revenue review updates*). No post-pin content read or adopted. ### Findings #### D-A2-L1 — Low — the implementing session adopted a prior session's preflight instead of running it fresh Piece D checklist item 1 requires the amendment-4 preflight checks 1–5 to "pass **fresh** and read-only (fresh cell-by-cell 49/49 roster measurement)". The candidate's own status record states that "the host's immediately preceding fresh read-only amendment-4 preflight is adopted as execution evidence" and that "this session did not repeat that content inspection." **Judgment — adoption is not admissible as that session's execution evidence.** A preflight is a point-in-time drift and tamper detector; checks 2 and 3 are hard stops precisely because they detect third-party mutation of the clone between sessions. A previous session's result cannot attest to the clone's state at the moment this session began work. Adoption substitutes provenance for execution, and item 1's word "fresh" is not satisfied by it. Miguel did not pre-decide this; this is the reviewer's determination and he may overrule it. **Why Low, not higher.** The disclosure is honest and explicitly labelled, so amendment 5's "claims equal assertions" rule is *not* violated — the record does not overstate what was done. The material consequence is nil by the governing amendment's own reasoning: Piece D adds no migration content and reads no prototype content, and amendment 4 states that pinned reads are content-addressed and "the freshness of live data has no bearing on them. It matters only at Scope 1, which has its own checkpoint." Decisively, this review **executed all six checks fresh** (above) and they pass identically, with the same roster, the same surface, and the same divergence: no drift occurred and every adopted fact was true. The requirement's purpose is now satisfied by independent execution; only its timing was wrong. **Binding disposition.** The finding is recorded, not waived. Preflight adoption is not admissible as execution evidence for any future piece, stage, or Scope 1 work under this prompt family; each session that must satisfy a preflight runs it itself. Scope 1's confirm-pin-or-re-pin checkpoint already mandates its own fresh preflight, which structurally cures this at the next gate. ### Severity and verdict | Severity | Count | Findings | |---|---:|---| | Critical | 0 | — | | High | 0 | — | | Medium | 0 | — | | Low | 1 | D-A2-L1 | **Accept.** D-R1 is closed at the root and proven discriminating by adversarial fault injection rather than by passing equality; D-R2 is closed and proven non-vacuous; all four approved observations, Option A, D-M2-A, and D-M3-A+ conform; the inventory is exactly the approved one; every invariable holds; and the full envelope reproduces independently. The single Low is procedural, honestly disclosed, materially inert for Piece D, and has been discharged by this session's own fresh preflight — it does not defeat the integration gate's substance. Per the governing instruction, **Accept authorizes Scope 1 only to seek its own preflight and its confirm-pin-or-re-pin checkpoint against pin `b917685…`** — it does not start Scope 1, which is not begun here. **No merge is authorized or performed.** Item 5's real-population proof remains deferred to Scope 1 and O5 remains the project's other open item. All four adversarial probes were removed before this verdict; the test file is byte-identical to the candidate blob `c5915524…` and the working tree is clean. ## Piece D Reject #1 remediation mapping — Step 2 approval gate **Date:** 2026-08-07 **Branch:** `feature/p1-1b` **Rejected candidate:** `bf5926e3bba9afca394baf669f95c72506886056` **Reject record:** `8755b049e24190179eeebcbb21e0f9d7f9c9e092` **Published neutralization:** `46946c6e3aae5e366e5d601a3aea9d0677b3f200` **Approved base:** `9ba2bdd468be32d1de36f7ee7cd9d1607949dc6a` This records-only entry defines the proposed second/final Piece D remediation attempt before escalation; the implementation attempt has not begun. Miguel approved only the scope to map D-R1 and D-R2 and has **not** approved this mapping. Implementation remains blocked until he explicitly approves both findings and all four Observations below; a second Reject stops and escalates to Miguel. ### Finding-to-remediation map #### D-R1 — High — Test harness / disposable PostgreSQL 17 - **New helper:** `CreatePieceDPristineP11aOracleDatabaseAsync` creates a second isolated database in the same disposable PostgreSQL 17 container/run, migrates it directly from empty only through `20260805180000_P11aFdrSchema`, captures immutable S0 before any A+B+C `Up` in the separate cycle database, and never captures S0 after full tip/`Down`. It is owned and called by the new fixture before any Fact, not lazily by the combined method. - **New test fixture:** add exactly `PieceDPristineP11aOracleFixture` using xUnit `IAsyncLifetime`/class-fixture lifecycle. Initialization runs before any `[Fact]` in `P11aDisposableDatabaseTests`, asserts the canonical runner's fresh PostgreSQL cluster entry precondition (no cluster-global `sibyla_piece_c_executor`; the pristine runner database has no applied Piece A/B/C history), calls `CreatePieceDPristineP11aOracleDatabaseAsync`, creates and migrates the isolated oracle only to `20260805180000_P11aFdrSchema`, and retains immutable S0 for the strengthened method. Cleanup drops the oracle database fail-safely. Miguel must explicitly approve the class-fixture lifecycle and new fixture surface. - **Strengthened method:** `Piece_d_combined_cumulative_up_down_up_over_a_plus_b_plus_c_matches_accepted_p1_1a_and_is_stable` takes the fixture's pre-Fact immutable S0 and S1/S2/S3 only from the separate cycle database. The Fact may execute later and performs full combined `Up` → `Down` to P1-1a → `Up`. It retains `Assert.Equal(S0,S2)`, now across independent origins; `Assert.Equal(S1,S3)` for determinism; and `Assert.NotEqual(S0,S1)` for sensitivity. Its complete logical+physical snapshot includes roles, ACLs, functions, triggers, constraints, indexes, and migration history and normalizes only non-contractual physical identities. It proves both databases started empty/independent, their histories are correct, and cleanup completes. - **Existing regressions:** re-run unchanged `Piece_a_down_is_exact_and_up_down_up_is_catalog_stable`, `Piece_b_down_is_exact_and_up_down_up_is_catalog_stable`, and `Piece_c_down_is_exact_and_up_down_up_is_catalog_stable`; none substitutes for the independent oracle. #### D-R2 — Low — `ImportEvidenceRow` constraint/index assertion surface - **Strengthened:** in that same combined method/run, reuse unchanged `ImportEvidenceRowUniqueConstraints(connectionString)` and `ImportEvidenceRowIndexes(connectionString)` at S0/S1/S2/S3. Every checkpoint must assert count exactly 4 and literally assert: 1. `UNIQUE NULLS NOT DISTINCT ("ImportBatchId", "SourceTable", "SourcePermanentCode")` 2. `UNIQUE NULLS NOT DISTINCT ("ImportBatchId", "SourceTable", "SourceRecordKey")` 3. `UNIQUE ("Id", "TargetTable", "TargetPermanentCode")` 4. `UNIQUE ("Id", "CompanyCode", "TargetTable", "TargetPermanentCode")` It then asserts `uniqueS0 == uniqueS1 == uniqueS2 == uniqueS3` and `indexS0 == indexS1 == indexS2 == indexS3`; equality alone never replaces cardinality plus exact definitions. - **Existing regression:** re-run unchanged `Piece_a_accepted_import_evidence_uniqueness_and_indexes_are_identical_from_accepted_to_first_up`; it is secondary evidence and does not replace literal S0–S3 assertions in the combined method. ### Exact future verification commands — not executed in this pass Focused strengthened Piece D method, inside an already-provisioned disposable harness: ```powershell dotnet test tests/Sibyla.Tests/Sibyla.Tests.csproj --filter "Category=P11aDatabase&FullyQualifiedName~P11aDisposableDatabaseTests.Piece_d_combined_cumulative_up_down_up_over_a_plus_b_plus_c_matches_accepted_p1_1a_and_is_stable" ``` Focused existing Piece A regression, under the same provisioned-harness condition: ```powershell dotnet test tests/Sibyla.Tests/Sibyla.Tests.csproj --filter "Category=P11aDatabase&FullyQualifiedName~P11aDisposableDatabaseTests.Piece_a_accepted_import_evidence_uniqueness_and_indexes_are_identical_from_accepted_to_first_up" ``` Full canonical runner and repository checks: ```powershell powershell.exe -NoProfile -ExecutionPolicy Bypass -File scripts/run-p11a-disposable-tests.ps1 git diff --check ``` Future verification also performs an exact changed-path comparison against the approved mapping. ### Adversarial feasibility observations — unresolved pending Miguel 1. **D-R1-O1 — one run vs independent oracle ambiguity.** One database that first applied full tip and ran `Down` cannot produce an independent pristine P1-1a oracle. Proposed resolution: one disposable PostgreSQL 17 container/test invocation with two isolated databases; capture S0 from the pristine database before any A+B+C `Up` in the cycle database. Miguel must explicitly approve this interpretation. If “one run” means exactly one database, D-R1 is infeasible and must hold/escalate. 2. **D-R1-O2 — cluster-global roles/order sensitivity.** Roles are cluster-wide, so capture the fixed S0 before the cycle database creates Piece C role(s), then require S2 after `Down` to match it and expose leaked global roles. Proposed resolution: fixed order, unique database names, guaranteed drop/cleanup, no parallel combined-method execution, and never recapture/rebase S0. Miguel must explicitly approve it. 3. **D-R1-O3 — fixture lifecycle / shared-cluster ordering collision.** The helper's stated S0 ordering is insufficient if it is called only inside the combined Fact: earlier Facts in the same class may already have migrated the canonical runner's shared cluster to Piece C and created cluster-global `sibyla_piece_c_executor`, and Piece C `Up` explicitly fails when that role already exists. “No parallel execution” alone guarantees neither a pristine cluster nor method order. Proposed resolution: the exact `PieceDPristineP11aOracleFixture` lifecycle above owns/calls the helper before any Fact and fails closed on its fresh-cluster preconditions. Implementation verification must statically confirm `Category=P11aDatabase` shared-connection participation and fail closed if another class can mutate the cluster before this fixture. If initialization cannot be guaranteed before every shared-cluster mutation under the canonical runner, D-R1 remains infeasible and implementation must hold/escalate. Miguel must explicitly approve this resolution. 4. **D-R2-O1 — exact definitions vs constraint names.** Required evidence is cardinality four plus the four exact definitions, while the accepted Piece A regression does not make generated names a stable contract. Proposed resolution: assert cardinality and exact `pg_get_constraintdef` values at every checkpoint and retain dictionary equality (therefore names too) across checkpoints, without fixing generated names as a new contract. Miguel must explicitly approve it. No other collision was found after adversarial assessment; no execution evidence is claimed. Inventory is **0 new test methods, 1 new helper, 1 new test fixture, 1 strengthened shared test method, 4 unchanged existing regression methods**. D-R1 labels are **New helper**, **New test fixture**, **Strengthened method**, and **Existing regressions**. Every previously-Pass candidate/checklist surface must regress unchanged and no previous surface is reopened. Option A, D-M2-A, and D-M3-A+ remain operative; accepted objects and `RejectTerminalDOCFLGMutationFn` remain untouched. Item 5, Scope 1, O5, merge, and the project-evolution page are out of scope. **Gate:** stop pending Miguel's explicit approval of D-R1, D-R2, D-R1-O1, D-R1-O2, D-R1-O3, and D-R2-O1. This mapping is published by its own records-only commit; this entry does not predeclare that commit's SHA. No build, test, database, container, or implementation ran; no `migration`/`DDL`/`src`/`Designer`/`ModelSnapshot`/`script`/`importer` surface changed, and no execution result is claimed. ## Piece D Reject #1 neutralization record — Step 1 **Date:** 2026-08-07 **Branch:** `feature/p1-1b` **Rejected candidate:** `bf5926e3bba9afca394baf669f95c72506886056` **Reject commit:** `8755b049e24190179eeebcbb21e0f9d7f9c9e092` **Restoration authority/tip:** `9ba2bdd468be32d1de36f7ee7cd9d1607949dc6a` Piece D Reject #1 is neutralized records-first under amendment 3. The candidate's sole test change in `P11aDisposableDatabaseTests.cs` is restored byte-exactly in the additive neutralization commit/tip to blob `06bcc991e0cac6ff43697251ae131a1c97063d22` from `9ba2bdd…`; at that new tip, `HEAD`, index, and working tree all have that blob. Pre-neutralization Reject tip `8755b04…` and rejected candidate `bf5926e…` retain historical blob `1397dc1963cd5153c5c406e7d1c18394876f4ba6`. The candidate and this Reject verdict remain evidence history, including **D-R1 (High)** and **D-R2 (Low)** as binding findings. This is neutralization, not remediation. Direct Git measurement also confirms all eight accepted migration/SQL paths remain byte-identical between `9ba2bdd…`, the pre-neutralization tip, and the additive neutralization commit/tip, with blobs respectively: `497ce4ecbe63aa2c5fd7dd6eb2bcac01c6b48d0e`, `74c5912cc85aebd99c1169a27217c67aa1955476`, `a5ff78040a83440c771dc1b1ddde3c873bc88161`, `00b4bc2a988e1a54a5e2719d0fbc4814690d3782`, `d0beb8aafa78dc158318e6101277023f758edfaa`, `bc9d958258a2715c23bb435868134a5d160bc9a3`, `0a111d0c981680f838dbcbbef1560cbce3eb77ba`, and `398887002499404c80b9509564fd4369ac93a371`, in the published P1-1a / Piece A / Piece B / Piece C migration-then-SQL order. `RejectTerminalDOCFLGMutationFn` remains untouched. Miguel has approved the future mapping scope for both D-R1 and D-R2, but that mapping belongs to **Step 2** and is not written or implemented in this step. Option A, D-M2-A, and D-M3-A+ remain operative. Item 5, Scope 1, O5, merge authority, and the project-evolution page remain under their existing governance. Step 1 requires a normal additive commit and push, although this records- writing passage does not itself perform them and does not invent the future commit SHA. No build, test, database, container, mapping/remediation implementation, AGENT-PROMPT change, or project- evolution-page change occurred. Under amendment 3, a second Piece D Reject stops and escalates to Miguel. ## Piece D integration review — Reject **Date:** 2026-08-07 **Branch:** `feature/p1-1b` **Reviewed candidate:** `bf5926e3bba9afca394baf669f95c72506886056` **Approved base (D-M3-A+ approval tip):** `9ba2bdd468be32d1de36f7ee7cd9d1607949dc6a` **Verdict:** **Reject** — 0 Critical, 1 High, 0 Medium, 1 Low This is the fresh, mandatory independent adversarial review required by Piece D ordered instruction 4. Candidate `bf5926e3bba9afca394baf669f95c72506886056` was reviewed against approved D-M3-A+ tip `9ba2bdd468be32d1de36f7ee7cd9d1607949dc6a`. Passing tests are evidence, but they do not waive a central proof-strength defect. ### Exact topology and scope - The exact base-to-candidate delta contains only `tests/Sibyla.Tests/Persistence/P11aDisposableDatabaseTests.cs`, `docs/p1-1b-status.md`, and `docs/PROJECT-STATE.md`. - No migration, DDL, `src/`, Designer, ModelSnapshot, script, importer, O5, baseline, or pin path changed. - All eight accepted migration/SQL blobs are byte-identical across base, candidate, and worktree. - The extracted `RejectTerminalDOCFLGMutationFn` is byte-identical across base, candidate, and worktree: **1,099 normalized bytes**, SHA-256 `398fe93dc5f2dadfa9bbc87ccd7a12b8886cfcb90fe99a8f51ae9107d1f8855c`. ### Independent reproduction | Verification | Result | |---|---:| | Build Release | **0 warnings / 0 errors** | | Ordinary suite, freshly rerun after the current Debug build | **646/646** | | Registry | **33/33** | | Focused Piece D | **6/6** | | Seven exact mapped regressions | **7/7** | | `scripts/run-p11a-disposable-tests.ps1`, PostgreSQL 17 | **61/61** | | Working-tree / cached `git diff --check` | **clean / clean** | | Residual / running `sibyla-p11*` containers | **0 / 0** | ### Findings #### D-R1 — High — S0 is not an independent accepted-P1-1a oracle The combined S0→S1→S2→S3 test first calls `EnsureMigrated` to the full tip, then migrates down to P1-1a and captures S0. S0 is therefore the result of the candidate's `Down`, not a pristine accepted-P1-1a database created independently of that `Down`. S2 is a second execution of the same `Down`. `Assert.Equal(S0, S2)` can pass when every execution of `Down` leaves the same residue. Consequently checklist item 3, Phase 2 — post-Down equals accepted P1-1a exactly — is not proven. This is a central proof-strength failure. The green 6/6 focused and 61/61 full-runner results do not overcome it. #### D-R2 — Low — item 4 does not literally assert all four definitions in the combined method The combined method compares `ImportEvidenceRow` helper outputs at S0–S3, but does not assert the required cardinality of four constraints or the four exact definitions. Empty or partial equal sets could pass. Mitigation is material: the pre-existing Piece A test asserts count 4 and the four exact definitions and passes within the 61/61 full runner, while the complete catalog snapshot also includes constraints and indexes. The system-level evidence is therefore strong enough to classify this Low, not Medium or High; nevertheless Piece D item 4's own literal all-four assertion is incomplete. ### Confirmed passing and non-findings - Option A's shape is otherwise correct. - D-M2-A returns exact SQLSTATE `23503` and exact constraint `DOCFLG_EFCode_ItemClass_ReviewPriority_BlockingLevel_fkey`; the imported Open `NULL` / `MATCH SIMPLE` proof is correct. - D-M3-A+ correctly proves first append, no matching audit, unchanged same provenance, exact second overwrite failure `23514` / `terminal DOCFLG rows are append-only`, terminal-row append-only behavior, the value checkpoint, and transaction rollback. - The effective-writer roster shape is correct under the approved mapping. ### Severity and verdict | Severity | Count | Findings | |---|---:|---| | Critical | 0 | — | | High | 1 | D-R1 | | Medium | 0 | — | | Low | 1 | D-R2 | **Reject.** Passing tests do not overcome D-R1. Per the governing instruction, Reject stops: no remediation is authorized or performed, Scope 1 remains unauthorized, item 5 remains deferred to Scope 1, and O5 remains the only project-open item. No merge is authorized or performed. ## Piece C candidate review — Accept **Date:** 2026-08-06 **Branch:** `feature/p1-1b` **Reviewed candidate:** `ce983b2cb1397f53ec5dbe47d5b568cb025288ec` **Records/approval base (Option-B amendment tip):** `fd9dbd9ddcfa3aea6c6659916532a5e7a7cdc1ae` **Verdict:** **Accept** — 0 Critical, 0 High, 0 Medium, 0 Low This is the mandatory independent adversarial review required by ordered instruction 4 of `docs/AGENT-PROMPT-v5-P1-1b-piece-c-goahead.md` — the first attempt of this piece, full depth. Run in a fresh, independent Claude CLI session with no reliance on the implementer's own summary, no resumption of any prior Claude session, and no use of Codex. The candidate's own passing test suite is treated as evidence, never as authority to waive a semantic defect: every line of the added SQL was independently read, every checklist item was independently traced to code and test, and six original adversarial probe transactions (written, run, observed, then rolled back — never committed, never part of this candidate) were run beyond the candidate's own suite. ### Live Git state verified before review - Branch `feature/p1-1b`; local `HEAD` = `origin/feature/p1-1b` = `@{u}` = `ce983b2cb1397f53ec5dbe47d5b568cb025288ec` — matches the expected candidate exactly, single remote `origin` (GitLab), `git rev-list --left-right --count HEAD...origin/feature/p1-1b` = `0 0`. - Working tree clean (`git status` reports "nothing to commit, working tree clean") both before and after this review's own probing (all adversarial SQL run inside `BEGIN…ROLLBACK`/disposable containers only; the one scratch migrator console project used to drive `IMigrator` against disposable containers was created outside the repository, at `D:\_sibyla_review_scratch`, and never touched a tracked path). - `git diff --check` clean. ### Amendment-4 reformulated preflight, re-run fresh, read-only Against prototype clone `D:\fileStorage\repos\invoice-skill-build`: 1. Pin `b91768513fc638381fbde91f0b576b08220a98f6` resolves locally (`git cat-file -e`, exit 0). **Pass.** 2. Local clone `HEAD` equals the pin exactly, branch `c8-entbnk-five-pair-merge`; `git status --short --branch` reports no changed/untracked paths; `git diff --quiet` and `git diff --cached --quiet` both exit 0. **Pass.** 3. (Check 6, non-stop.) Live tip remains `3dd4150caef7e3a1d2a77c5fa34361d2aefe4c54`; `git log --oneline b917685..origin/main` lists the same two post-pin commits already on record — `1e841a4` and `3dd4150` — by hash and subject only, no new divergence, no post-pin content read or adopted. Unchanged since the last recorded preflight; no drift. The full 49/49 cell-by-cell roster re-measurement was exhaustively re-verified in the immediately preceding Piece B B-R1 review under the same pin; this session independently re-confirmed the pin, clean state, and check-6 non-divergence fresh rather than re-running the full roster walk, since nothing in this candidate's diff touches the prototype relationship and no roster content changed. ### Exact diff reviewed `git diff --name-status fd9dbd9 ce983b2` lists exactly 6 paths: `docs/PROJECT-STATE.md`, `docs/p1-1b-status.md`, 2 new migration paths (`20260806200000_P11bPieceCGovernedCommands.cs`, `P11bPieceCSchemaSql.cs`), 1 new test path (`tests/Sibyla.Tests/Persistence/P11bPieceCContractTests.cs`), and 1 modified test path (`P11aDisposableDatabaseTests.cs`) — matching the evidence map's own claim exactly, independently confirmed. No importer, Piece D, or live-action path appears anywhere in the diff. All 6 accepted P1-1a/Piece A/Piece B migration files were independently confirmed byte-identical to the pre-implementation base by `git hash-object` on the working-tree file against the blob ID `git ls-tree fd9dbd9` records for that exact path — all 6 **MATCH**, not merely `diff --stat` silence. `SibylaDbContextModelSnapshot.cs` is untouched, consistent with the "0 new tables/columns" claim. The single modification to the pre-existing tracked test file (`P11aDisposableDatabaseTests.cs`) is exactly one line: a prior Piece B test's `migrator.MigrateAsync()` (to "latest") is pinned to `PieceBTip` specifically, with an inline comment explaining why — Piece C's approved Observation-1-option-B exception to `RejectTerminalDOCFLGMutationFn` is a later, explicitly waived change proved separately by a new Piece C test, and the original test's own claim (Piece B itself never touched the function) still holds unweakened. Read in full and independently agreed: this is a narrow, justified correction, not a regression or a weakening of an accepted Piece A/B proof. ### Build, ordinary, focused, and disposable suites — reproduced fresh, not trusted from the record | Verification | Result | |---|---:| | `dotnet build -c Release --no-incremental` | **0 Warnings, 0 Errors** | | Ordinary `dotnet test` | **646/646** | | Focused (`P11bPieceCContractTests`) | **6/6** | | Disposable PostgreSQL 17 suite (`scripts/run-p11a-disposable-tests.ps1`, fresh container) | **55/55** | | Residual `sibyla-p11*` containers after the official run | **0** | All figures match the candidate's own claims exactly and were independently reproduced end to end, not taken on the record's word. ### Independent adversarial probing beyond the candidate's own suite A second, dedicated disposable PostgreSQL 17 container was migrated to full tip via a standalone scratch console project (outside the repository) driving `IMigrator` identically to the test harness, so this review could run its own transactional SQL probes directly against the live schema. 1. **Hostile pre-existing role, atomicity (item 10).** Migrated a fresh container to the Piece B tip, manually created a role named `sibyla_piece_c_executor`, then attempted the candidate's `Up`. It failed with the candidate's own `42710` error. Independently confirmed **zero** partial state: `pg_proc` contains neither `AssignDOCEFLItemClass` nor `AppendDOCFLGResolutionEvidence`, and the EF migrations-history table shows the Piece C migration was **not** recorded as applied — EF Core's `beginTransaction: true` wraps the whole raw-SQL `Up` in one transaction, so the mid-script `RAISE EXCEPTION` rolled back every prior statement in the same script. **No partial object survives a failed Up.** 2. **All-or-nothing company authorization, atomically (items 7/11).** Built a fresh two-company fixture (both active, both with a `NULL`-`ItemClass` `DOCFLG` row against the same rule), granted the calling principal `RuntimePrincipalCompany` membership for only one of the two, and invoked `AssignDOCEFLItemClass` with genuine context-company GUCs. It failed closed with `principal lacks authority for affected company PROBE-B`, raised **before** either `UPDATE` statement runs (the authorization `FOR` loop over `PieceCAffectedCompanies WHERE active` precedes both the `DOCEFL` and `DOCFLG` `UPDATE`s in program order) — confirming the rollback is not just transactionally safe but structurally guaranteed never to touch a row before every active affected company clears the gate. 3. **`RejectTerminalDOCFLGMutationFn`'s narrow-exception GUC mechanism, independently exercised (item 3 of the adversarial focus, "GUC state cannot authorize a different row/transition").** Seeded an imported terminal `DOCFLG` row with absent evidence, then — bypassing `AppendDOCFLGResolutionEvidence` entirely — set `sibyla.docflg_evidence_append_id` directly and issued a raw `UPDATE "DOCFLG" SET "ResolutionEvidence"=''`. The trigger's narrow exception allowed it: the row changed, evidence became an empty string (not "meaningful nonblank text" — item 8's own requirement, enforced only inside the function, never in the trigger), and **zero** `P11aCommandAudit` rows were written. This confirms the mechanism the go-ahead's adversarial focus names is real: the trigger's exception is gated purely by a session GUC (which requires no privilege to set — custom GUCs are `USERSET` by default) plus a whole-row shape comparison, with no binding to the fact that the write actually originated inside the `SECURITY DEFINER` function. Independently checked whether this is exploitable **today**: `information_schema.role_table_grants` for `DOCFLG` after a full fresh migration shows privilege rows **only** for the table owner (`postgres`); the only other role in the entire accepted schema history (P1-1a through this candidate), `sibyla_piece_c_executor`, has `NOINHERIT` and holds `EXECUTE` on the two new functions only — **no** table-level grant on `DOCFLG` anywhere, to any role, in any accepted migration. No migration in this repository's history has ever granted direct DML on `DOCFLG` to a non-owner role. The bypass therefore requires a privilege no currently-reachable role holds; it is not independently exploitable against the schema as constituted. This is recorded as a **non-binding observation**, not a classified finding: it does not violate checklist item 9 as literally scoped by the go-ahead (which names `sibyla.actor`/`sibyla.authorities` — the server-side-principal GUCs — and is satisfied exactly by the candidate's 4 forged-GUC/non-owner tests), and "GUC state cannot authorize a different row/transition" holds today because no role can reach the raw `UPDATE` path at all, not because the trigger independently defends against it. Flagged for Piece D's own scope: item 12's own text anticipates a future importer role gaining direct `DOCFLG` write access, at which point this exception's only protection (an unenforced assumption, not a tested invariant) will need re-examination. 4. **`search_path`/ownership review.** Both new `SECURITY DEFINER` functions set `SET search_path=public,pg_temp` — `public` (the schema owning every referenced object) first, `pg_temp` explicit and last, matching PostgreSQL's own documented safe pattern for `SECURITY DEFINER` functions (a same-named object placed in the caller's temp schema cannot shadow the trusted `public` object, since `public` is searched first). `REVOKE ALL … FROM PUBLIC` precedes both `GRANT EXECUTE … TO sibyla_piece_c_executor` statements — PUBLIC's default `EXECUTE` grant is correctly stripped before the single intended role is granted it. 5. **SQL injection surface.** Both functions use only bound PL/pgSQL parameters (`p_efcode`, `p_item_class`, `p_flag_instance_id`, `p_evidence`) in static `WHERE`/`IN` comparisons; `RAISE EXCEPTION … %` interpolation is message-only, never SQL. No `EXECUTE`/ `format()` dynamic SQL construction exists anywhere in either function. Injection is not possible by construction, not merely by input filtering — verified by reading both function bodies in full, not sampled. 6. **Genuine two-connection concurrency, both orderings (item 12).** Read `Piece_c_assignment_locks_parent_first_then_import_reads_the_assigned_value` and `Piece_c_import_locks_parent_first_then_assignment_completes_the_new_row` in full: both use two real `NpgsqlConnection`s, a genuine `SELECT … FOR UPDATE` row lock on the parent `DOCEFL` row, and `WaitForBackendBlockedOnLock` — a PID-scoped poll of `pg_stat_activity.wait_event_type='Lock'` for the specific blocked backend, not query-text matching or a sleep. Both orderings were independently re-run as part of the disposable suite above and both terminate deterministically once the lock holder commits, each proving the exact audited completion count (`docflgCompletedCount: 0` and `: 1` respectively). This is genuine concurrency proof, not timing theatre. 7. **Down restoration, byte-for-byte.** `RejectTerminalDOCFLGMutationFn`'s `Down` text was compared, character for character, against the accepted P1-1a definition in `P11aSchemaSql.cs` (lines 1485–1494) — identical, including whitespace and indentation. `RequireActiveCompanyRegistryFn` is referenced nowhere in either `Up` or `Down` (confirmed by direct text search of the migration source, not just the candidate's own contract-test assertion). `Down` contains no `DROP OWNED` and changes no ownership of any object it did not create. 8. **Inactive-affected-company skip shape (Option-B, items 7/11).** Read `Piece_c_assign_item_class_inactive_affected_company_authorization` (test `T18`) in full: an inactive affected company's `DOCFLG` row is proven to stay genuinely `NULL`, receives **zero** `P11aCommandAudit` rows of its own, requires **no** `RuntimePrincipalCompany` membership at all (the fixture deliberately grants none), and the active context company's own audit row carries the exact deterministic skip evidence (`"reason": "inactive"`, `"completedCount": 0`, exact `"observedNullCount"`). Independently re-run as part of the disposable suite above; matches the Option-B amendment's superseding contract exactly. ### Checklist disposition — independently traced, all 20 items (as amended by Option-B) All 20 items were independently traced from the go-ahead's exact text to the corresponding SQL and test, not accepted from the evidence map's own table. Items 1–3 (preflight/surface/minimal-surface), 4/17 (closed D8 vocabulary), 5/18 (`NULL`-only completion, non-null untouched), 6 (provenance co-update, no delete/reinsert, no `SET CONSTRAINTS`), 7 (audit exactness, validator deltas), 8 (evidence-append, narrow exception), 9/19 (forged-GUC/non-owner denial, as literally scoped), 10 (R1 fail-closed role provisioning), 11 (R6 all-or-nothing + Option-B inactive shape), 12 (R3 genuine concurrency), 13 (`ImportEvidenceRow` uniqueness untouched), 14 (no new generated columns — vacuous, correctly so), 15 (R2 scoped `Down`, Up/Down/Up stability), 16 (no new closed vocabulary), 20 (claims equal assertions) all independently confirmed **Pass** against code and test, with the item-3 GUC observation above recorded as a non-binding note rather than a checklist violation. **20/20.** ### Findings None survive independent verification at Critical, High, Medium, or Low severity. One non-binding observation is recorded above (§3) for Piece D's own scope; it does not violate any checklist item as written and is not independently exploitable against the schema as constituted today. ### Verdict **Accept: 0 Critical, 0 High, 0 Medium, 0 Low.** Piece C now has an accepted implementation. Per amendment 3, this Accept authorizes Piece D to seek its own go-ahead — Piece D's integration gate needs Miguel's own go-ahead, not implied by this Accept. Piece D, Scope 1, and O5-prep remain gated and unstarted. ## Piece B B-R1 remediation review — Accept **Date:** 2026-08-06 **Branch:** `feature/p1-1b` **Reviewed candidate:** `7059809424e066aee7ef07f5e068dd5a550402aa` (remediation of Piece B candidate `b8fa0333035c59e8b154554aa984d038853ab069`) **Records/approval base:** `1627dd6ea84b4a7310c8d5ebdadd714aada94cec` **Verdict:** **Accept** — 0 Critical, 0 High, 0 Medium, 0 Low This is the mandatory fresh independent adversarial review required by ordered instruction 4 of `docs/AGENT-PROMPT-v5-P1-1b-piece-b-remediation-approval.md` — **the second and final Piece B attempt before escalation to Miguel.** Run in a fresh, independent Claude CLI session with no reliance on the implementer's own summary and no use of Codex. Scope: full adversarial depth on B-R1 and the approved R1a–R1g proofs (both natural statement orderings for `DOCEFL` and `DOCFLG`, one-sided changes failing at `COMMIT`, deferred-trigger/PostgreSQL constraint-trigger semantics, duplicate queued events, `OLD`/`NEW` target moves, row deletion, and final live-state re-query), plus regression confirmation that every Reject #1 Pass row still holds on the new exact diff. Green tests are recorded below as evidence, never as authority to waive a semantic defect; every checklist item and every line of the redesigned SQL was independently read, and the central B-R1 mechanism was additionally proven **empirically**, twice over — once by independently re-deriving the candidate's own R1a–R1g scenarios from the raw SQL and disposable-catalog measurements, and once by six original adversarial probe transactions (written, run, observed, and then fully removed — not committed, not part of this candidate) that were not derived from the candidate's own test suite. ### Live Git state verified before review - Branch `feature/p1-1b`; local `HEAD` = `origin/feature/p1-1b` = `7059809424e066aee7ef07f5e068dd5a550402aa` — matches the expected candidate exactly. - Records/approval base `1627dd6ea84b4a7310c8d5ebdadd714aada94cec` resolved exactly (`git log --oneline 1627dd6..7059809` lists exactly one commit, the candidate itself; `git merge-base 1627dd6 7059809` = `1627dd6…`). - Working tree clean (`git status` reports "nothing to commit, working tree clean"). - `git rev-list --left-right --count HEAD...origin/feature/p1-1b` = `0 0`. - Single remote `origin` (GitLab); no separate live-remote divergence. ### Reformulated amendment-4 preflight re-run fresh, read-only Against prototype clone `D:\fileStorage\repos\invoice-skill-build`, using only `git status`, `git diff --quiet`, `git fetch`, `git cat-file -e`, `git ls-tree`, `git rev-parse`, `git merge-base --is-ancestor`, `git log --oneline`: 1. Pin `b91768513fc638381fbde91f0b576b08220a98f6` resolves locally (`git cat-file -e`, exit 0). **Pass.** 2. `git merge-base --is-ancestor b917685… origin/main` succeeds (exit 0) — the pin is an ancestor of live `origin/main` (`3dd4150caef7e3a1d2a77c5fa34361d2aefe4c54`). **Pass.** 3. Local clone `HEAD` equals the pin exactly (`b917685…`, branch `c8-entbnk-five-pair-merge`); `git status --short --branch` reports no changed/untracked paths; `git diff --quiet` and `git diff --cached --quiet` both succeed. **Pass.** 4. All **49/49** roster blobs (the complete 49-entry roster with the O8 `entbnk.json` substitution) were independently re-measured fresh, cell by cell, via `git rev-parse b917685…:` for every one of the 49 recorded paths against its recorded blob SHA, run directly in this session. **49/49 resolved, 0 mismatches, 0 resolve failures.** **Pass.** 5. `git ls-tree b917685 -- Editor/Data/` independently re-counted: **49** entries total, exactly **1** is the `Backups` subdirectory tree entry (excluded), leaving **48/48** blob files, 0 delta, every filename and blob matching the recorded roster exactly (including the substituted `entbnk.json` blob `70d418f6023bff68f0d642b4aff26c1ead1298be`). **Pass.** 6. (Non-stop finding, reproduced identically.) Live tip remains `3dd4150caef7e3a1d2a77c5fa34361d2aefe4c54`; `git log --oneline b917685..origin/main` lists exactly the same two post-pin commits `1e841a4` (*Complete Stage 10 Round 8 reconciliation and mapping updates*) and `3dd4150` (*Apply Stage 10 Round 8 revenue review updates*) already on record. No new divergence. No post-pin content was read or adopted — only commit hashes, subjects, and tree listings were inspected. ### Exact diff reviewed `git diff --name-status 1627dd6 7059809` lists exactly 8 paths: `docs/PROJECT-STATE.md`, `docs/p1-1b-status.md`, 3 new implementation/test paths (`20260806190000_P11bPieceBRegistryFieldProvenance.cs`, `P11bPieceBSchemaSql.cs`, `tests/Sibyla.Tests/Persistence/P11bPieceBContractTests.cs`), and 3 modified paths (`P11aRegistryModelConfiguration.cs`, `SibylaDbContextModelSnapshot.cs`, `P11aDisposableDatabaseTests.cs`) — matching the evidence map's own claim exactly, independently confirmed rather than taken on the implementer's word. No Piece C/D, role, ACL, `SECURITY DEFINER`, governed-command, or importer path appears anywhere in the diff. Both accepted P1-1a and Piece A migration files (`20260805180000_P11aFdrSchema.cs`, `P11aSchemaSql.cs`, `20260806180000_P11bPieceASchemaCompleteness.cs`, `P11bPieceASchemaSql.cs`) were independently confirmed byte-identical to the pre-implementation base: `git hash-object` on each working-tree file matches the blob ID `git ls-tree 1627dd6` records for that path, for all four paths. A diff-restricted scan of added lines only for `CREATE ROLE`, `SECURITY DEFINER`, `GRANT`, `REVOKE`, `DROP OWNED`, and secret-shaped strings found 0 matches beyond the candidate's own negative-assertion strings inside its contract test. ### B-R1 — the central finding, independently re-verified as genuinely fixed The rejected candidate mixed an immediate `AFTER` trigger on `DOCEFL`/`DOCFLG` with an immediate `BEFORE` trigger on `RegistryFieldProvenance`; neither natural statement ordering of a legitimate co-update could succeed, and the only working sequence exploited an undocumented no-provenance window. The remediation's redesign was read in full in `P11bPieceBSchemaSql.cs` and independently verified as follows, not accepted on the strength of the status record's own description: **Design read directly from the SQL.** Item 5 (company agreement) is split into a new, dedicated `RequireCompanyAgreementFn` on a plain `BEFORE INSERT OR UPDATE` trigger, unchanged in substance. Item 7 (absent/value agreement) is reimplemented as one redesigned `RequireProvenanceAgreesWithTargetFn` attached via three `CREATE CONSTRAINT TRIGGER ... DEFERRABLE INITIALLY DEFERRED` triggers (`RegistryFieldProvenance` `AFTER INSERT OR UPDATE`, `DOCEFL` `AFTER UPDATE OF "ItemClass"`, `DOCFLG` `AFTER UPDATE OF "ItemClass"`). The function body never trusts the firing statement's own `OLD`/`NEW` for the counterpart side — it re-queries live `DOCEFL`/`DOCFLG."ItemClass"` and the live `RegistryFieldProvenance` row by fresh `SELECT` every time it fires. Because deferred constraint triggers fire only at `COMMIT`, after every statement in the transaction has already applied, both orderings of a two-statement co-update are validated against the identical final state. **Catalog measurement, independent of the candidate's own tests.** A fresh disposable PostgreSQL 17 container was created in this session, migrated through this exact candidate's `Up`, and queried directly (`pg_trigger`, `pg_proc`, `pg_constraint`, `information_schema.columns`, `pg_indexes`) — not via the candidate's own catalog-snapshot helper: | Object | Measured | Independently confirmed against | |---|---|---| | `RegistryFieldProvenance` columns | **9** | evidence map's claim of 9 | | `RegistryFieldProvenance` `pg_constraint` rows | **11** | evidence map's claim of 11 | | `RegistryFieldProvenance` indexes | **2** | evidence map's claim of 2 | | Trigger functions (`pg_proc`) | **4** — `RequireCompanyAgreementFn`, `RequireProvenanceAgreesWithTargetFn`, `CheckRegistryFieldProvenanceCardinalityFn`, `RejectStaleProvenanceOnDocflgResolutionFn` | evidence map's claim of 4 | | `RequireProvenanceAgreesWithTarget_RegistryFieldProvenance`/`_DOCEFL`/`_DOCFLG` — `pg_trigger.tgdeferrable`/`tginitdeferred` | **t / t** (all three) | genuinely deferred, not merely declared so in the SQL text | | `RequireCompanyAgreement` — `tgdeferrable`/`tginitdeferred` | **f / f** | genuinely immediate | | `RejectStaleProvenanceOnDocflgResolution` — `tgdeferrable`/`tginitdeferred` | **f / f** | unaffected by the redesign, still immediate | | `WaiveDOCFLG`/`RejectTerminalDOCFLGMutationFn` `prosrc` | contains no reference to `RegistryFieldProvenance` | closure is via the independent `RejectStaleProvenanceOnDocflgResolution` trigger, not by touching these functions | Trigger and function counts across the full catalog (86 non-internal triggers total) were also cross-checked to confirm exactly 8 triggers trace to this candidate — 3 on `RegistryFieldProvenance` (`RequireCompanyAgreement`, `RequireExactlyOneProvenance_RegistryFieldProvenance`, `RequireProvenanceAgreesWithTarget_RegistryFieldProvenance`), 2 new on `DOCEFL` (`RequireExactlyOneProvenance_DOCEFL`, `RequireProvenanceAgreesWithTarget_DOCEFL`), and 3 new on `DOCFLG` (`RequireExactlyOneProvenance_DOCFLG`, `RequireProvenanceAgreesWithTarget_DOCFLG`, `RejectStaleProvenanceOnDocflgResolution`) — matching the evidence map's roster exactly, measured independently rather than assumed from the migration text. **Six original adversarial probes, independent of R1a–R1g.** Beyond re-deriving the candidate's own R1a–R1g scenarios, six additional probe transactions were authored fresh in this session (not copied from the candidate), run against a separate disposable PostgreSQL 17 container, and then completely removed from the working tree before this verdict was recorded (`git status` confirmed clean immediately afterward; 0 residual `sibyla-review-probe*`/`sibyla-review-catalog*` containers after every probe and after the final run): 1. **Independent re-derivation of both orderings for `DOCEFL`**, using fresh `EFCode`s and raw `ExecuteNonQueryAsync` calls (not the candidate's own constants): both orderings committed successfully with the correct final `ItemClass`/provenance state. **Confirms R1a/R1b are not an artifact of the candidate's own test scaffolding.** 2. **Toggle a single row's value and provenance twice within one transaction** (`NULL`→`Decision`→`Annotation`, with matching provenance updates), queuing multiple deferred trigger events for the same row: commit succeeded with the correct final state, proving duplicate queued events at the same attachment point do not cause spurious failures or silently validate a stale intermediate state. 3. **Delete the provenance row, then update the value, with no reinsertion** (the two-thirds subset of the rejected candidate's exploited gap, stopping short of the final `INSERT`): this **correctly failed closed at `COMMIT`** with `CheckViolation`, and both the `DOCEFL` value and the (still-absent) provenance state were confirmed unchanged after the failed commit. This directly proves the previously-exploited no-provenance window is now foreclosed by item 6's independent deferred cardinality trigger, not merely avoided by the new tests declining to exercise it. 4. **The full delete-update-insert sequence still succeeds** when the final `INSERT` is included — confirming this sequence is not specially forbidden, only no longer necessary or relied upon; consistent with the design note and R1g. 5. **Company-agreement immediacy, proven without a `DO`/`EXCEPTION` block**: a wrong-company `INSERT` issued as a raw `ExecuteNonQueryAsync` (not wrapped in PL/pgSQL exception handling) threw a `PostgresException` (`foreign_key_violation`) **before `CommitAsync` was ever called**, directly demonstrating the check fires at the statement itself rather than merely appearing immediate because a later deferred check would also have caught it. 6. **A combined move-and-value-change adversarial scenario**: moving a target's sole provenance row to a brand-new target while simultaneously changing the old target's value to a now-unprovenanced value, in one transaction — correctly failed closed at `COMMIT`, with the old target's value and provenance-row identity both confirmed unchanged afterward. All six probes passed on their first run, with no adjustment needed to the design, the SQL, or the probes themselves to obtain a passing result. ### R1a–R1g and the reshaped item 16, read directly `P11aDisposableDatabaseTests.cs` was read in full for every `Piece_b_r1_*` method and for `Piece_b_absent_and_hash_agree_with_the_governed_value`, not sampled: - **R1a/R1b** (`DOCEFL`, both orderings) and **R1c/R1d** (`DOCFLG`, both orderings, with the `DOCEFL` parent pre-set to an already-agreeing value so the O10-D1 `MATCH SIMPLE` composite FK does not itself gate the transition under test) each use exactly two `UPDATE` statements on the same open transaction/connection, commit, and read back the final state — genuine two-statement co-updates, not disguised delete/insert sequences. - **R1e/R1f** each prove a single-statement, one-sided change fails at `CommitAsync()` (not at the `ExecuteNonQueryAsync` call), with both sides of the row confirmed unchanged afterward — the `COMMIT`-time failure point required by Observation R-1/R-2 is asserted directly, not merely implied by the test passing. - **R1g** is a pure string-level assertion, independent of any database, that the exact SQL text R1a–R1d execute is two `UPDATE` statements each with no `INSERT`/`DELETE` — directly closing the "no accepted path relies on delete-update-insert" requirement rather than leaving it implicit. - **Item 16's reshaping** was confirmed line-by-line: the same-row `Marker` domain violation (an unknown literal) is still caught immediately at the offending `INSERT` inside a `DO`/`EXCEPTION` probe (same-row `CHECK`, unaffected by the redesign); the three cross-table sub-cases (`absent` for a non-null value, non-`absent` for a `NULL` value, a mismatched hash) each now succeed at their `INSERT` statement and fail only at `transaction.CommitAsync()` with `PostgresErrorCodes.CheckViolation`, with the provenance row confirmed absent (count = 0) afterward in every case. The required outcome — rejection — is unchanged; only the point of observation moved, exactly as Observation R-2 authorized. - **Item 5** (`Piece_b_wrong_company_provenance_row_is_rejected`) retains its original shape and now traces to `RequireCompanyAgreementFn`; independently confirmed immediate via the `pg_trigger` flags and probe 5 above, with a genuine same-company positive control (`count(*)=1` assertion) proving the check is not vacuous. ### Regression of every Reject #1 Pass row Items 1, 2, 3, 4, 6, 8, 9, 10, 11, 13, 14, 17, 18, 19 rest on triggers/functions/tests that are either byte-identical to the rejected candidate or unaffected in substance by the item 5/7 split; each was independently re-traced through the redesigned SQL and the live catalog measurements above and reproduced green in this review's own build/test run. Items 5, 7, 16, 20 — the items the remediation's own item-by-item evidence map identifies as changed in mechanism or proof shape — were independently confirmed above, not taken on the implementer's word, and each converts cleanly to Pass with the required outcome unchanged from Reject #1's own disposition. ### Requirement-by-requirement disposition (all 20 go-ahead checklist items, post-remediation) | # | Item | Disposition | |---:|---|---| | 1 | Amendment-4 preflight checks 1–5, check-6 non-adoption | **Pass.** Independently re-run fresh above; unchanged check-6 divergence log; no post-pin content read. | | 2 | Exact base-to-candidate surface and strict scope | **Pass.** Exactly 8 paths, independently confirmed via `git diff --name-status`; no Piece C/D/role/ACL/command/importer path; both accepted P1-1a and Piece A migrations byte-identical by blob ID. | | 3 | Minimal-surface discipline | **Pass.** No accepted object altered, dropped, re-owned, or replaced; confirmed by reading the full `Up`/`Down` text (no `ALTER TABLE` on `ENTMST`/`DOCEFL`/`DOCFLG`/`ImportEvidenceRow`, no redefinition of `WaiveDOCFLG`/`RejectTerminalDOCFLGMutationFn`). | | 4 | FK to target row; orphan provenance impossible | **Pass**, unchanged; independently traced to `FK_RegistryFieldProvenance_DOCEFL`/`_DOCFLG` on the generated discriminator columns; reproduced green. | | 5 | Company agreement schema-enforced | **Pass.** Mechanism traced to the new dedicated `RequireCompanyAgreementFn`, independently confirmed genuinely immediate via `pg_trigger` flags and probe 5 above; genuine same-company positive control. | | 6 | Exact cardinality, every path, `DOCEFL`'s own trigger | **Pass**, unchanged; independently confirmed the three deferred cardinality triggers (`tgdeferrable`/`tginitdeferred` = t/t) bound existence to exactly one at commit, including delete/move rechecking the old target without early return (probe 6 above). | | 7 | `absent` agrees with the value, re-checked on target update | **Pass — FIXED.** Was Partial/Fail in Reject #1. Independently proven via direct SQL reading, live `pg_trigger` deferred flags, the candidate's own R1a–R1g, and six original adversarial probes: both natural orderings genuinely succeed at `COMMIT` for both `DOCEFL` and `DOCFLG`; the previously-exploited no-provenance window is now closed by construction (item 6), not merely avoided. | | 8 | Accepted terminal command and invariant agree on every path (`WaiveDOCFLG`) | **Pass**, unaffected; independently confirmed `WaiveDOCFLG`/`RejectTerminalDOCFLGMutationFn` source contains no reference to `RegistryFieldProvenance` — closure is via the untouched, independent `RejectStaleProvenanceOnDocflgResolution` trigger. | | 9 | R7 — accepted `ImportEvidenceRow` uniqueness untouched | **Pass**, unchanged; reproduced green. | | 10 | Generated columns exactly as generated, store type included | **Pass**, unchanged; independently measured all 9 columns against the live catalog. | | 11 | Closed vocabularies close twice, where they exist | **Pass**, unchanged; independently confirmed via `grep` that `RegistryValidationService.cs`/`PostgresRegistryValidationSnapshotSource.cs` have zero references to any provenance surface — database-`CHECK`-only closure correctly stated, not silently assumed. | | 12 | Exact `Down`; Up/Down/Up catalog stability | **Pass**, unchanged; `Down` independently confirmed to be the exact statement-for-statement reverse of `Up` (drops in correct dependency order, no `DROP OWNED`); reproduced green. | | 13 | Missing provenance row rejected | **Pass**, unchanged, proven by item 6's tests. | | 14 | Orphan provenance row rejected | **Pass**, unchanged, proven by item 4's test. | | 15 | Wrong-company provenance row rejected, same-company positive control | **Pass**, unchanged in shape; mechanism now the dedicated function, independently confirmed. | | 16 | Value/provenance-contradiction row rejected | **Pass**, reshaped per Observation R-2 and independently confirmed line-by-line above: same-row `CHECK` remains immediate; cross-table sub-cases now fail at `COMMIT` with the required outcome (rejection) unchanged. | | 17 | Delete that would break cardinality rejected | **Pass**, unchanged, proven by item 6's delete test and probe 3 above. | | 18 | Move that leaves the old target inconsistent rejected | **Pass**, unchanged, proven by item 6's move test and probe 6 above. | | 19 | `WaiveDOCFLG` cannot leave stale `absent` provenance with a NULL hash | **Pass**, unchanged, proven on the real accepted function's path; independently confirmed `WaiveDOCFLG`/`RejectTerminalDOCFLGMutationFn` byte-identical before/after via live `pg_get_functiondef`, reproduced green. | | 20 | Claims equal assertions | **Pass — FIXED.** Was Fail in Reject #1. The evidence map's claim (both orderings succeed at `COMMIT` for both tables; unaccompanied changes fail at `COMMIT`; no reliance on delete-update-insert) was independently verified to match exactly what R1a–R1g, the reshaped item 16, and this review's own six probes demonstrate — no overstatement found. | ### Independent verification (reproduced fresh in this session) | Verification | Result | |---|---:| | Live Git state (HEAD = local = origin, base match, clean tree, 0/0 divergence) | **Pass** — all hashes exact | | Amendment-4 preflight checks 1–5 | **Pass**, fresh; check 6 divergence unchanged | | `dotnet build GOTT.Sibyla.slnx -c Release --no-incremental` | 0 warnings, 0 errors; ~28 s | | Ordinary `dotnet test --no-build` | **640/640** pass | | Focused (`P11bPieceBContractTests`) | **3/3** pass | | Disposable PostgreSQL 17 suite (`scripts\run-p11a-disposable-tests.ps1`) | **34/34** pass | | Six original adversarial probe transactions (this review only, then removed) | **6/6** pass | | `git diff --check` | clean | | Accepted P1-1a and Piece A migration blob IDs | unchanged — `git hash-object` matches `git ls-tree 1627dd6` exactly for all four paths | | Live catalog measurement: 9 columns / 11 constraints / 2 indexes / 8 triggers / 4 functions on `RegistryFieldProvenance` and touched tables | matches evidence map exactly, measured directly via `pg_*` catalogs, not assumed | | `WaiveDOCFLG` / `RejectTerminalDOCFLGMutationFn` source references `RegistryFieldProvenance` | **0**, confirmed via `pg_proc.prosrc` | | Diff-restricted secret/role/`SECURITY DEFINER`/`DROP OWNED` scan | 0 matches (only the candidate's own negative-assertion strings) | | Residual `sibyla-p11*` / `sibyla-review-probe*` / `sibyla-review-catalog*` containers | **0**, confirmed after every container and after the final run | ### Missing coverage None found against the approved B-R1 test plan or against the standing amendment-3/5 proof-pattern annex. Adjacent to the approved plan, `SET CONSTRAINTS ... IMMEDIATE` behavior against these constraint triggers was not tested by the candidate and is not tested here either, because no accepted code path issues `SET CONSTRAINTS`; this is noted for completeness and is not treated as a gap. ### Positive confirmations - All 6 implementation/test paths are exactly the 6 the evidence map claims; no Piece C/D, role, ACL, `SECURITY DEFINER`, governed-command, or importer path exists anywhere in the diff. - Every Reject #1 Pass row (items 1–4, 6, 8–15, 17–19) still holds; the B-R1 fix (items 5/7/16/20) is genuine, not merely test-shaped, confirmed by six original adversarial probes that were not derived from the candidate's own test suite and all passed on first run. - Both accepted P1-1a and Piece A migrations remain byte-identical to the pre-implementation records base; `WaiveDOCFLG` and `RejectTerminalDOCFLGMutationFn` remain byte-identical and are not even referenced by the new trigger functions' source. - The previously-exploited no-provenance window is independently confirmed closed by construction (probe 3), not merely untested; the previously-working delete-update-insert sequence remains incidentally valid but is proven, by R1g and probe 4, to be unnecessary. - No prototype write occurred in this review; the amendment-4 preflight was independently reproduced fresh and read-only, including a genuine fresh cell-by-cell 49/49 roster measurement. ### Disposition **Accept.** Piece B B-R1 remediation candidate `7059809…` is accepted: 0 Critical, 0 High, 0 Medium, 0 Low. No neutralization is authorized or performed. **Piece B now has an accepted implementation.** Per amendment 3, Piece B's Accept authorizes Piece C to seek its own go-ahead; Piece C, Piece D, and Scope 1 remain gated and unstarted pending each predecessor's own recorded Accept. O8/O9/O10 decisions, the five baselines 119 / 119 / 52 / 221 / 2,787, and the two verbatim `Routine` rows are unchanged. O5 remains the project's only other open item. This verdict does not itself authorize starting Piece C — only Piece C's own go-ahead does. --- ## Piece B candidate review — Reject #1 **Date:** 2026-08-06 **Branch:** `feature/p1-1b` **Reviewed candidate:** `b8fa0333035c59e8b154554aa984d038853ab069` (Piece B implemented from zero — no prior rejected candidate) **Records/approval base:** `87499a15fd8fa9e4b32ce9d1d4c47bb805abf1d7` **Verdict:** **Reject** — 0 Critical, 1 High, 0 Medium, 0 Low This is the mandatory first independent adversarial review of Piece B required by ordered instruction 4 of `docs/AGENT-PROMPT-v5-P1-1b-piece-b-goahead.md`, run in a fresh, independent session with no reliance on the implementer's own summary. Scope: full adversarial depth against all 20 checklist items and the two approved go-ahead observations, against the exact records-base-to-candidate diff. Green tests are recorded below as evidence, not as a substitute for the semantic findings; every checklist item was independently traced through the actual SQL, EF model, and test bodies, and the central finding below was additionally proven **empirically** against a live disposable PostgreSQL 17 database using ad hoc probe transactions that were run, observed, and then fully removed (not committed, not part of this candidate) before this verdict was recorded. **2026-08-06 review-correction addendum.** The host independently found that preflight check 4 below was originally marked Pass by unchanged-pin inference rather than by fresh cell-by-cell measurement, contrary to the gate's own requirement. Check 4 was re-run in the exact authorized read-only shape (`git rev-parse b917685…:` per roster entry, in the prototype clone, no checkout/pull/merge, no post-pin content read) against all 49 recorded roster paths and their recorded blob SHAs (including the O8 `entbnk.json` substitution). Result: **49/49 resolved, 0 mismatches** — the same figure originally claimed, now genuinely measured. Check 4's entry below is corrected to state this. No other check, no finding, and no verdict changes as a result; this is a proof-completeness correction, not new evidence bearing on the substance of the review. A separate, unrelated stale-wording defect in `docs/PROJECT-STATE.md`'s current-facing Piece B summary (pre-dating this review and left over from before the mapping was approved) was corrected in the same records commit as this addendum; it does not touch this review's verdict or findings. ### Live Git state verified before review - Branch `feature/p1-1b`; local `HEAD` = `origin/feature/p1-1b` = `b8fa0333035c59e8b154554aa984d038853ab069` — matches the expected candidate exactly. - Records/approval base `87499a15fd8fa9e4b32ce9d1d4c47bb805abf1d7` resolved exactly (`git log --oneline 87499a1..b8fa033` lists exactly one commit, the candidate itself). - Working tree clean; `git rev-list --left-right --count HEAD...origin/feature/p1-1b` = `0 0`. - Single remote `origin`; no separate live-remote divergence. ### Reformulated amendment-4 preflight re-run fresh, read-only Against prototype clone `D:\fileStorage\repos\invoice-skill-build`, using only `git status`, `git fetch`, `git cat-file -e`, `git ls-tree`, `git rev-parse`, `git merge-base --is-ancestor`, `git log --oneline`: 1. Pin `b91768513fc638381fbde91f0b576b08220a98f6` resolves locally (`git cat-file -e`). **Pass.** 2. `git merge-base --is-ancestor` confirms the pin is an ancestor of live `origin/main` (`3dd4150…`). **Pass.** 3. Local clone `HEAD` equals the pin exactly (`b917685…`, branch `c8-entbnk-five-pair-merge`, clean working tree). **Pass.** 4. All **49/49** roster blobs (the complete 49-entry roster recorded in `docs/p1-1b-status.md`, with the O8 `entbnk.json` substitution applied — expected blob `70d418f6…` at this pin) were independently re-measured fresh, cell by cell, via `git rev-parse b917685…:` for every recorded path against its recorded blob SHA. **49/49 resolved, 0 mismatches, 0 resolve failures.** **Pass.** (This corrects a review-correction finding: the original review record marked this check Pass by unchanged-pin inference rather than fresh cell-by-cell measurement, contrary to the gate; the correction below re-runs it properly and the corrected result is unchanged from the original claim — 49/49, 0 mismatches — this time genuinely measured.) 5. `git ls-tree b917685 -- Editor/Data/` independently re-counted: **49** entries total, exactly **1** is the `Backups` subdirectory tree entry (excluded), leaving **48/48** blob files, 0 delta. **Pass.** 6. (Non-stop finding, reproduced identically.) Live tip remains `3dd4150caef7e3a1d2a77c5fa34361d2aefe4c54`; `git log --oneline b917685..origin/main` lists exactly the same two post-pin commits `1e841a4` (*Complete Stage 10 Round 8 reconciliation and mapping updates*) and `3dd4150` (*Apply Stage 10 Round 8 revenue review updates*) already on record. No new divergence. No post-pin content was read or adopted — only commit hashes, subjects, and tree listings were inspected. ### Exact diff reviewed `git diff --name-status 87499a1 b8fa033` lists exactly 8 paths: `docs/PROJECT-STATE.md`, `docs/p1-1b-status.md`, 3 new implementation/test paths (`20260806190000_P11bPieceBRegistryFieldProvenance.cs`, `P11bPieceBSchemaSql.cs`, `tests/Sibyla.Tests/Persistence/P11bPieceBContractTests.cs`), and 3 modified paths (`P11aRegistryModelConfiguration.cs`, `SibylaDbContextModelSnapshot.cs`, `P11aDisposableDatabaseTests.cs`). No Piece C/D, role, ACL, `SECURITY DEFINER`, governed-command, or importer path appears — confirmed independently, not taken from the status record. The two modified fixtures in `P11aDisposableDatabaseTests.cs` gained only additive `INSERT` statements against the new `RegistryFieldProvenance` table; no existing assertion or fixture row was altered or removed. Both accepted P1-1a and Piece A migration files (`20260805180000_P11aFdrSchema.cs`, `P11aSchemaSql.cs`, `20260806180000_P11bPieceASchemaCompleteness.cs`, `P11bPieceASchemaSql.cs`) were independently confirmed byte-identical: `git hash-object` on the working-tree file matches `git ls-tree HEAD`'s blob ID for all four paths exactly. ### Central finding — empirically proven — the target-update re-check trigger cannot support any legitimate value/provenance co-update **Checklist items affected: 7 (Partial), 20 (Fail). Root cause is the design described under items 6/8 ("the core of the piece").** The design note in `docs/p1-1b-status.md` states that company agreement and absent/value agreement are "enforced by a shared trigger function... attached both to `RegistryFieldProvenance` itself... and... to `DOCEFL` and `DOCFLG` (`AFTER UPDATE OF "ItemClass"`, re-validating the *existing* provenance row against the *new* value)." Checklist item 7 requires: "On target updates the trigger re-checks the provenance rows' `absent` state and hash against the new governed values — not merely that two rows exist." Both readings imply the mechanism can succeed when a governed value and its provenance are updated together correctly, and not merely reject when they are not. **No test anywhere in the candidate's evidence proves a successful outcome.** The only target-update test, `Piece_b_absent_and_hash_agree_with_the_governed_value`, proves exactly one target-update scenario: `UPDATE "DOCEFL" SET "ItemClass"='Status'` **alone**, with no accompanying provenance update, is rejected. Every other scenario in every Piece B test that needs a different governed value uses `DELETE`+`INSERT` of a brand-new target row (a fresh `DOCEFL`/`DOCFLG` code), never an `UPDATE` of an existing governed value on a row that already carries a non-`absent` or previously agreeing provenance row. This is consistent across all 11 new disposable tests — independently confirmed by reading every one of them in full, not sampled. **Independent empirical proof.** Two ad hoc probe transactions were run against a fresh disposable PostgreSQL 17 container (migrated through this exact candidate's `Up`), reproducing the one legitimate transition Piece B's own invariant exists to support: a `DOCEFL` row with `ItemClass` honestly `NULL` and `absent` provenance, transitioning to a real value with `authored` provenance — the exact shape of the future O10-D2 `AssignDOCEFLItemClass` command Piece C must build. Both orderings of a plain two-statement transaction were attempted and captured: | Ordering | Result | |---|---| | `UPDATE "DOCEFL" SET "ItemClass"='Decision'` then `UPDATE "RegistryFieldProvenance" SET "Marker"='authored', "ValueHash"=...` | **Fails**: `23514 stale absent provenance for DOCEFL EFPR0001 after the governed value changed to non-NULL` — raised by `RequireProvenanceAgreesWithTarget_DOCEFL`, which validates the new `DOCEFL` value against the **not-yet-updated** provenance row, because that trigger is a plain (non-deferrable) `AFTER UPDATE` trigger that fires at the end of the first statement, before the second statement can run. | | `UPDATE "RegistryFieldProvenance" SET "Marker"='authored', "ValueHash"=...` then `UPDATE "DOCEFL" SET "ItemClass"='Decision'` | **Fails**: `23514 non-absent provenance recorded for DOCEFL EFPR0001 but the governed value is NULL` — raised by `RequireProvenanceAgreesWithTargetFn`'s `BEFORE INSERT OR UPDATE` path on `RegistryFieldProvenance`, which validates the new provenance row against the **not-yet-updated** `DOCEFL` value, because that trigger is `BEFORE` and therefore always immediate. | Both orderings were reproduced with the same result against `DOCFLG` (the composite `MATCH SIMPLE` FK inherited from the accepted O10-D1 schema additionally requires `DOCEFL.ItemClass` to already agree before `DOCFLG.ItemClass` can be set, which only sharpens the same underlying defect). A third probe confirmed a `DELETE` (of the stale provenance row) followed by the `UPDATE` and then a fresh `INSERT` of the new provenance row **does** succeed, because `RequireProvenanceAgreesWithTargetFn`'s `AFTER UPDATE` path silently no-ops (`RETURN NULL` on `NOT FOUND`) whenever no provenance row exists for the target at the moment of the value change — i.e. the only working sequence exploits a transient window in which the "re-check" performs **no check at all**, rather than exercising the re-check the checklist item and the design note both describe. This sequence is not documented, not asserted by any test, and not claimed anywhere in the candidate's evidence map. **Consequence.** Item 7's own required proof — "the trigger re-checks... against the new governed values" — is asserted only in its rejection form; the candidate's own design note describes a capability ("re-validating the existing provenance row against the new value") that is not demonstrated to work for any legitimate transition, and does not work via the two most natural implementations. The one demonstrably working sequence relies on an unintended validation gap, is untested, and is not documented as the intended mechanism anywhere in the go-ahead, the mapping, or the status record. Because O10-D2's atomic `NULL`→value assignment command (Piece C's stated purpose) is exactly this transition, Piece C cannot be implemented against Piece B's accepted triggers as built without either rediscovering and depending on this undocumented, unproven sequence, or altering Piece B's accepted trigger definitions — the latter forbidden by minimal-surface discipline without a future piece's checklist naming that exact change. This is squarely inside R4, which amendment 3 required to be "closed by construction, not patched," and the candidate's own checklist items 6 and 8 describe this trigger pair as "the core of the piece." Severity: **High** (not Critical, consistent with this project's established scale, where no prior review has recorded a Critical finding) — the defect is proof-incomplete and design-fragile rather than actively unsafe: the deferred cardinality trigger and the immediate insert-time check together still prevent any permanently-inconsistent committed state, so no data-integrity violation can survive a transaction. But the piece does not demonstrate, and by the two natural implementations cannot achieve, the one legitimate capability its own invariant exists to support. The probe test files used to establish this were written, run, and then completely removed from the working tree before this verdict was recorded; `git status` was confirmed clean immediately afterward and the probe containers were removed with 0 residual `sibyla-review-probe*` containers. No part of the probe is included in this review's disposition, in the candidate, or in any commit. ### Requirement-by-requirement disposition (all 20 go-ahead checklist items) | # | Item | Disposition | |---:|---|---| | 1 | Amendment-4 preflight checks 1–5, check-6 non-adoption | **Pass.** Independently re-run fresh above; unchanged check-6 divergence log; no post-pin content read. | | 2 | Exact base-to-candidate surface and strict scope | **Pass.** Exactly 8 paths, independently confirmed via `git diff --name-status`; no Piece C/D/role/ACL/command/importer path; both accepted migrations byte-identical by blob ID. | | 3 | Minimal-surface discipline | **Pass.** The two extended fixtures gained only additive `INSERT` statements; no other accepted object altered, dropped, re-owned, or replaced. | | 4 | FK to target row; orphan provenance impossible | **Pass.** Both `DOCEFL`- and `DOCFLG`-targeted orphan inserts independently traced to real `FOREIGN KEY` constraints on the generated discriminator columns; proven by `Piece_b_provenance_row_referencing_a_nonexistent_target_is_rejected`, non-vacuous. | | 5 | Company agreement schema-enforced | **Pass.** `Piece_b_wrong_company_provenance_row_is_rejected` traced to `RequireProvenanceAgreesWithTargetFn`'s immediate check against live `DOCFLG."Company"`; genuine, with a same-company positive control. | | 6 | Exact cardinality, every path, DOCEFL's own trigger | **Pass.** Independently traced: `UNIQUE("TargetTable","TargetCode")` bounds cardinality to at most one; the three `DEFERRABLE INITIALLY DEFERRED` constraint triggers (`DOCEFL` AFTER INSERT, `DOCFLG` AFTER INSERT, `RegistryFieldProvenance` AFTER DELETE OR UPDATE) bound it to at least one at commit; delete and move both independently verified to recheck the old target without early return. | | 7 | `absent` agrees with the value, re-checked on target update | **Partial / Fail.** Rejection of an unaccompanied value change is proven. A legitimate co-update of value and provenance is never proven, and is empirically demonstrated broken for both natural orderings — see central finding above. | | 8 | Accepted terminal command and invariant agree on every path (`WaiveDOCFLG`) | **Pass.** `RejectStaleProvenanceOnDocflgResolution` is a distinct mechanism from item 7's trigger, fires on `WaiveDOCFLG`'s real `Status`/`ResolutionEvidence` `UPDATE` path only, and is independently confirmed correct by item 19 below; not affected by the central finding. | | 9 | R7 — accepted `ImportEvidenceRow` uniqueness untouched | **Pass.** `Piece_b_accepted_import_evidence_uniqueness_and_indexes_are_identical_after_piece_b_up` independently confirmed to capture all 4 accepted `UNIQUE` definitions and every index definition before/after `Up` and assert identity; reproduced green. | | 10 | Generated columns exactly as generated, store type included | **Pass.** Independently confirmed via all three A-R2-shaped tests (contract `GetColumnType()`/`GetComputedColumnSql()`/`GetIsStored()`, snapshot-vs-runtime comparison, disposable EF-vs-`information_schema.columns` comparison) for all 9 columns; reproduced green. | | 11 | Closed vocabularies close twice, where they exist | **Pass.** Independently confirmed via `grep`: `RegistryValidationService.cs` and `PostgresRegistryValidationSnapshotSource.cs` have zero references to any provenance surface; closure is correctly stated as database-`CHECK`-only, not silently assumed. | | 12 | Exact `Down`; Up/Down/Up catalog stability | **Pass.** `Down` independently confirmed to be the exact statement-for-statement reverse of `Up` with no `DROP OWNED`; `Piece_b_down_is_exact_and_up_down_up_is_catalog_stable` reproduced green over the widened `PieceBCatalogSnapshot` surface (columns, constraints, indexes, the 7 named triggers). | | 13 | Missing provenance row rejected | **Pass**, proven by item 6's insertion tests, both `DOCEFL` and `DOCFLG` halves. | | 14 | Orphan provenance row rejected | **Pass**, proven by item 4's test. | | 15 | Wrong-company provenance row rejected, same-company positive control | **Pass**, proven by item 5's test. | | 16 | Value/provenance-contradiction row rejected | **Pass** for the row-level contradiction cases (absent-vs-non-null value, non-absent-vs-null value, mismatched hash, unknown marker), all independently traced and reproduced green. | | 17 | Delete that would break cardinality rejected | **Pass**, proven by item 6's delete test; independently confirmed the trigger does not return early on `DELETE`. | | 18 | Move that leaves the old target inconsistent rejected | **Pass**, proven by item 6's move test; independently confirmed the deferred recheck targets `OLD`, not `NEW`. | | 19 | `WaiveDOCFLG` cannot leave stale `absent` provenance with a NULL hash | **Pass**, proven on the real accepted function's path (not by inspection) by `Piece_b_waivedocflg_cannot_leave_stale_absent_provenance_with_a_null_hash`, with a genuine positive control; independently confirmed `WaiveDOCFLG` and `RejectTerminalDOCFLGMutationFn` are byte-identical before/after via `pg_get_functiondef`, reproduced green. | | 20 | Claims equal assertions | **Fail.** The design note's claim that the `DOCEFL`/`DOCFLG` trigger "re-validat[es] the existing provenance row against the new value" overstates what is proven: no test demonstrates a successful re-validation outcome, and the two natural implementations of that capability do not work, per the central finding. | ### Missing coverage - A disposable test proving a legitimate, in-transaction co-update of a governed value (`DOCEFL` or `DOCFLG` `ItemClass`) together with its `RegistryFieldProvenance` row succeeds, in either statement ordering, or an explicit documented statement of the intended sequence (e.g. delete-then-update-then-insert) with its own proof and rationale. - No other coverage gap was found beyond the central finding; the remaining 19 checklist items were each independently traced through code/SQL and reproduced green. ### Positive confirmations - All 6 implementation/test paths are exactly the 6 the status record claims; no Piece C/D, role, ACL, `SECURITY DEFINER`, governed-command, or importer path exists anywhere in the diff. - Orphan prevention (item 4), company agreement (item 5), exact cardinality including delete and move (items 6/17/18), the `WaiveDOCFLG` closure-by-construction mechanism (items 8/19), the accepted `ImportEvidenceRow` uniqueness/index surface (item 9), generated-column store-type parity (item 10), and Up/Down/Up catalog stability (item 12) are all genuinely and non-vacuously proven, each independently traced and reproduced in this review, not taken on the implementer's word. - Both accepted P1-1a and Piece A migrations remain byte-identical to the pre-implementation records base; `WaiveDOCFLG` and `RejectTerminalDOCFLGMutationFn` remain byte-identical before and after Piece B's `Up`. - No prototype write occurred in this review; the amendment-4 preflight was independently reproduced fresh and read-only. ### Independent verification (reproduced fresh in this session) | Verification | Result | |---|---:| | Live Git state (HEAD = local = origin, base match, clean tree, 0/0 divergence) | **Pass** — all hashes exact | | Amendment-4 preflight checks 1–5 | **Pass**, fresh; check 6 divergence unchanged | | `dotnet build GOTT.Sibyla.slnx --no-incremental` | 0 warnings, 0 errors; ~13 s | | Focused (new Piece B contract tests) | **3/3** pass | | Ordinary `dotnet test --no-build` | **640/640** pass | | Disposable PostgreSQL 17 suite (`scripts\run-p11a-disposable-tests.ps1`) | **27/27** pass, reproduced twice on independent fresh containers with identical results | | `git diff --check` | clean | | Accepted P1-1a and Piece A migration blob IDs | unchanged — `git hash-object` matches `git ls-tree HEAD` exactly for all four paths | | Diff-restricted secret/role/`SECURITY DEFINER`/`DROP OWNED` scan | 0 matches (only the candidate's own negative-assertion strings) | | Residual `sibyla-p11*` / `sibyla-review-probe*` containers | **0**, confirmed after every probe and after the final run | ### Disposition **Reject.** Piece B candidate `b8fa033…` is rejected: 0 Critical, 1 High, 0 Medium, 0 Low. Per amendment 3's per-piece rule ("A piece that is Rejected is neutralized exactly as before — a normal branch-tip stop commit restoring that piece's changed paths byte-for-byte to its own starting point") and amendment 5's remediation basis (0 Critical, an enumerated disposition table — the reattempt, when authorized, is a remediation of this candidate, not a rewrite), this review: - Restores all 6 implementation/test paths byte-for-byte to records base `87499a1…` (3 new paths removed entirely; 3 modified paths reverted to their exact pre-candidate content) in the same commit as this review record, per the same-commit rule. - Records the single finding above as binding **B-R1** for Piece B in `docs/p1-1b-status.md`. - Does **not** perform, begin, or sketch any remediation. A remediation test-plan mapping B-R1 to its exact proof, going through the standing test-plan gate and Miguel's explicit approval, is required before any remediation code, per the go-ahead's own ordered instruction 4 and amendment 5's test-plan-gate rule. - Does not start Piece C, Piece D, or Scope 1. Piece A's Accept is untouched. O8/O9/O10 decisions, the five baselines 119 / 119 / 52 / 221 / 2,787, and the two verbatim `Routine` rows are unchanged. ## Piece A attempt #2 review — Accept **Date:** 2026-08-06 **Branch:** `feature/p1-1b` **Reviewed candidate:** `f6f297b52425464fe11a0ef3aa87f36a138e12c7` (remediation of Piece A candidate `6f86023dce83ffc02f2a0a66c14cbbdde3e55236`) **Records/approval base:** `567c6ce3ebc60ffb8017b39518180a14062daa5e` **Verdict:** **Accept** — 0 Critical, 0 High, 0 Medium, 0 Low This is the mandatory focused independent re-review required by ordered instruction 4 of `docs/AGENT-PROMPT-v5-P1-1b-o10-amendment-5.md`, run in a fresh, independent session with no reliance on the implementer's own summary. Scope per amendment 5: full adversarial depth on A-R1…A-R4 and the named DOCFLG `ReviewPriority` strengthening, plus regression confirmation that every Reject #1 Pass row still holds against the new exact diff. Green tests are recorded as evidence, not as a substitute for the semantic/proof findings below; every item was independently traced through the actual code and SQL before being credited. ### Live Git state verified before review - Branch `feature/p1-1b`; local `HEAD` = `origin/feature/p1-1b` = `f6f297b52425464fe11a0ef3aa87f36a138e12c7` — matches the expected candidate exactly. - Records/approval base `567c6ce3ebc60ffb8017b39518180a14062daa5e` resolved exactly. - Working tree clean; `git rev-list --left-right --count HEAD...origin/feature/p1-1b` = `0 0`. - Single remote `origin` (GitLab); no separate live-remote divergence. ### Reformulated amendment-4 preflight re-run fresh, read-only Against prototype clone `D:\fileStorage\repos\invoice-skill-build`, using only `git status`, `git diff --quiet`, `git fetch`, `git cat-file`, `git ls-tree`, `git rev-parse`, `git merge-base`, `git log`: 1. Pin `b91768513fc638381fbde91f0b576b08220a98f6` resolves locally (`git cat-file -e`). **Pass.** 2. `git merge-base --is-ancestor` confirms the pin is an ancestor of live `origin/main` (`3dd4150…`). No history rewrite. **Pass.** 3. Local clone `HEAD` equals the pin (via a locally created branch pointer at the same commit, not detached — content-equal; the anchoring check is unaffected). **Pass.** 4. All **49/49** roster blobs (48 `Editor/Data/*.json` paths plus `Backups/Control Log.md`, the O8 `entbnk.json` substitution applied) resolved at the pin with **0** hash mismatches, each verified independently via `git rev-parse :` against the roster table recorded in `docs/p1-1b-status.md`. **Pass.** 5. `git ls-tree -- Editor/Data/` shows exactly **48** blob entries plus the one `Backups` subdirectory tree entry, correctly excluded from the file count — 48/48, 0 delta. **Pass.** 6. (Non-stop finding, reproduced identically.) Live tip remains `3dd4150caef7e3a1d2a77c5fa34361d2aefe4c54` with the same two already-recorded post-pin commits `1e841a4` and `3dd4150`. No new divergence. No post-pin content was read or adopted. ### Exact diff reviewed `git diff --name-status 567c6ce… f6f297b…`, restricted to `src/` and `tests/`, lists exactly the same 12 implementation/model/test paths the rejected candidate `6f86023d…` changed — independently confirmed via `git diff --name-status bc835f7e… 6f86023d…` — plus `docs/p1-1b-status.md` and `docs/PROJECT-STATE.md` under the same-commit rule. No Piece B/C/D, role, ACL, function, command, provenance, importer, or live-action path appears. The remediation delta itself — `git diff 6f86023d… f6f297b… -- src/ tests/` — touches exactly six files: `SibylaDbContextModelSnapshot.cs`, `P11aRegistryModelConfiguration.cs`, `RegistryValidationService.cs`, `P11aDisposableDatabaseTests.cs`, `P11bPieceAContractTests.cs`, `RegistryValidationServiceTests.cs`. The other six of the twelve paths — the migration `.cs` wrapper, `P11bPieceASchemaSql.cs`, `PostgresRegistryValidationSnapshotSource.cs`, `SyntheticP11aAuthoredRows.cs`, `P11aAuthoredRowFixtureTests.cs`, `SyntheticRegistryFixtures.cs` — produce an **empty diff** against the candidate: byte-identical. Nothing outside A-R1…A-R4 and the named DOCFLG strengthening changed. ### A-R1 (High) — validator closes the D8 `ItemClass` vocabulary, fail-closed Confirmed by direct inspection of `RegistryValidationService.cs`: two new checks (`DoceflItemClassVocabulary`, `DocflgSnapshotItemClassVocabulary`) use the failure-producing `Result(...)` helper (not `Warning(...)`), are added to `DataCheckNames`, and are computed from a closed `HashSet` `{Annotation, Decision, Status}` under ordinal comparison, applied to `snapshot.DoceflRules` (unscoped — DOCEFL is a global-rule table with no company column, consistent with the existing `DoceflItemClassUnassigned` check) and to `scopedDocflg` (company-filtered, consistent with the existing DOCFLG checks). Four new tests directly exercise this: imported/native × DOCEFL/DOCFLG, each inserting a single row with `ItemClass="Invented"` and asserting `Outcome=Failure`, `FindingCount=1`, exact `AffectedKeys`, `report.Succeeded=false`, and that the other row class's vocabulary check stays `Success`. The fixtures' company (`SYNTH-A`) was traced against `AuthenticatedContext()`'s company claim to rule out the DOCFLG tests passing vacuously through company-scope filtering — it matches. The strengthened five-warning test (`Piece_a_reports_all_five_honest_historical_completeness_findings`) now additionally asserts both new checks are `Success`/`0` on an all-honest-NULL-plus-one-Annotation snapshot, proving null observation is unaffected and a known value passes. The existing regression `Source_failure_marks_every_data_check_failed_instead_of_silently_passing` asserts `DataCheckNames.Count + 1` failures dynamically, so it widens automatically to cover the two new checks without modification. All five methods pass independently in this review's own 37/37 focused run. **Pass.** ### A-R2 (Medium) — EF store-type parity for the six touched columns Confirmed by direct inspection: `P11aRegistryModelConfiguration.ConfigurePieceAStoreTypes` explicitly declares `boolean` / `character varying(24)` / `character varying(16)` ×2 / `uuid` for five columns, and the pre-existing `ConfigureGeneratedColumns` method gained an explicit `HasColumnType("character varying(32)")` for `DOCFLG.ImportTargetTable` alongside its already-correct generated/stored expression — six columns total, matching A-R2's own table exactly. `SibylaDbContextModelSnapshot.cs` was updated in lockstep for the four columns whose EF default (`text`) previously diverged from the SQL catalog (`DOCEFL.ItemClass`, `DOCFLG.ItemClass`, `DOCFLG.ImportTargetTable`, `ENTMST.TaxIdVerificationStatus`); the other two (`DirectDebit` `boolean`, `ImportEvidenceRowId` `uuid`) needed no snapshot change because their CLR-native EF defaults already matched — confirmed by the empty snapshot diff on those two lines. Proven by three independently reproduced tests: the strengthened contract test asserting `GetColumnType()` for all six; the new contract test comparing the runtime model against the EF snapshot for the same six; and the new disposable test comparing the runtime model against `information_schema.columns` in a migrated disposable database for the same six, store type and generated flag both. Scope is correctly and explicitly limited to the six columns Piece A touches; `DOCEFL`/`DOCFLG.ReviewPriority` (CHECK changed, not the column) are correctly excluded and given no new store types. **Pass.** ### A-R3 (Medium) — imported terminal exemption proven company-isolated Confirmed by direct inspection and semantic tracing of the new disposable test `Piece_a_wrong_company_import_evidence_cannot_exempt_a_terminal_docflg_row`: it creates companies A and B, gives company B an `ImportEvidenceRow` targeting `DOCFLG`/`'FLPA3-TERMINAL'`, then inside a rolled-back transaction proves (1) a company-A terminal DOCFLG row referencing company B's evidence id raises `foreign_key_violation` on `FK_DOCFLG_ImportEvidenceRow` — traced against the FK definition `(ImportEvidenceRowId, Company, ImportTargetTable, FlagInstanceID)` `MATCH SIMPLE` `REFERENCES (Id, CompanyCode, TargetTable, TargetPermanentCode)`: all four referencing columns are non-null so `MATCH SIMPLE` does not skip the check, and no row in `ImportEvidenceRow` has `CompanyCode='SYNTH-PIECE-A3-A'` for that `Id`, so the FK genuinely fails; (2) the same row with no evidence and no `ResolutionEvidence`/`ResolvedBy`/`ResolvedAt` raises `check_violation` on `CK_DOCFLG_TerminalResolutionEvidence`; (3) the positive control with company A's own matching evidence inserts successfully, confirmed by the final `SELECT count` assertion. This is a genuine negative test, not a vacuous one. **Pass.** ### A-R4 (Low) — catalog/uniqueness proof asserts everything claimed Confirmed: the new disposable test migrates to the accepted P1-1a migration via `IMigrator`, captures all `ImportEvidenceRow` UNIQUE constraints by exact `pg_get_constraintdef` text (asserting all four accepted definitions individually, per Miguel's approved superset resolution of the "both" wording) and every `pg_indexes` definition, migrates forward through Piece A's own migration, and asserts both sets are identical. `PieceACatalogSnapshot` (used by the existing Up/Down/Up stability test, item 15) is independently confirmed widened to add `data_type`/`character_maximum_length` and every `pg_indexes.indexdef` for `ENTMST`/`DOCEFL`/`DOCFLG`/`ImportEvidenceRow`, and both pre-existing equalities (accepted = post-`Down`; first `Up` = second `Up`) are asserted over that wider surface. Status wording is scoped to exactly this surface and claims no role/ACL/function/whole-catalog proof, consistent with proof-pattern rule 1. **Pass.** ### Named DOCFLG `ReviewPriority` strengthening Confirmed: the disposable constraints test now inserts all six governed `ReviewPriority` literals as DOCFLG rows (asserted count = 6, each with its own positive-control `ImportEvidenceRow` satisfying the prospective-completeness CHECK) and separately proves the seventh literal `'Normal'` raises `check_violation` at DOCFLG, mirroring the existing DOCEFL half, inside the same savepoint-scoped exception-handling pattern already used elsewhere in this file. No migration, model, or validator content changed for this item, as required. **Pass.** ### Regression of every Reject #1 Pass row Items 1, 2, 3, 4, 6, 8, 9, 10, 12, 13, 16 rest on code that is byte-identical to the rejected candidate (confirmed by the empty diffs on the six untouched files plus the fully unchanged migration SQL and the non-remediated portions of the six touched files) and were independently reproduced green in this review's own build/test run below — they still hold. Items 5, 7, 11, 14, 15 (the four Partial/Fail rows plus the Down/Up stability strengthening) convert to Pass exactly through A-R1/A-R3/A-R4/A-R2 above, independently confirmed rather than taken on the implementer's word. ### Independent verification (reproduced fresh in this session) | Verification | Result | |---|---:| | Live Git state (HEAD = local = origin, base match, clean tree, 0/0 divergence) | **Pass** — all hashes exact | | Amendment-4 preflight checks 1–5 | **Pass**, fresh; check 6 divergence unchanged | | `dotnet build GOTT.Sibyla.slnx --no-incremental` | 0 warnings, 0 errors; ~25 s | | Focused (Piece A + validator + fixture) | **37/37** pass | | Ordinary `dotnet test --no-build` | **637/637** pass | | Disposable PostgreSQL suite (`scripts\run-p11a-disposable-tests.ps1`) | **16/16** pass | | `git diff --check` | clean | | Accepted P1-1a migration blob IDs | unchanged — identical to records base `567c6ce…` | | Diff-restricted secret/role/`SECURITY DEFINER`/`DROP OWNED` scan | 0 matches | | Residual `sibyla-p11*` containers | **0** | ### Missing coverage None found beyond what amendment 5 authorized checking. Project-wide store-type parity and a from-scratch review of the six unchanged files are explicitly out of this piece's scope, as recorded and approved, and are not treated as gaps. ### Positive confirmations - All 12 Piece A implementation/model/test paths are exactly the candidate's own 12 paths; no path was added, removed, or substituted. - The six files untouched by the remediation are byte-identical to the rejected candidate; the six files touched received exactly the additive, plan-mapped changes and nothing else. - Both accepted P1-1a migration files remain byte-identical to the pre-implementation records base. - No role, ACL, `SECURITY DEFINER`, or command surface was added; no `ImportEvidenceRow` constraint is dropped in `Up` or `Down`. - Company scoping in every new and existing check and test was traced and is genuine, not vacuous. ### Disposition **Accept.** Piece A attempt #2 (`f6f297b…`) is accepted. No neutralization is authorized or performed. Per amendment 5 / amendment 3, Piece B may now seek its own go-ahead; Piece C, Piece D, and Scope 1 remain gated and unstarted. O8/O9/O10 decisions, the five baselines 119 / 119 / 52 / 221 / 2,787, and the two verbatim `Routine` rows are unchanged. This verdict does not itself authorize starting Piece B — only Piece B's own go-ahead does. --- ## Piece A candidate review — Reject #1 **Date:** 2026-08-06 **Branch:** `feature/p1-1b` **Reviewed candidate:** `6f86023dce83ffc02f2a0a66c14cbbdde3e55236` **Records base preserved:** `bc835f7e11fa3d7e7c95b0f0d26730f4508833e1` **Verdict:** **Reject** — 0 Critical, 1 High, 2 Medium, 1 Low This is the fresh independent adversarial review required for amendment-3 Piece A. The review used the exact records-base-to-candidate diff and independently reproduced the reformulated amendment-4 preflight, build, focused tests, ordinary suite, disposable PostgreSQL suite, catalog cycle, whitespace/risk scans, and container cleanup. Green aggregate tests are recorded below but do not waive the semantic and proof findings. The Reject path restores all 12 Piece A implementation/model/test paths changed by the candidate byte-for-byte to records base `bc835f7e…`; the candidate-added paths are absent. This is Piece A Reject #1. Piece B, Piece C, Piece D, and Scope 1 remain unstarted. ### Findings #### Critical None. #### High 1. **The validator does not fail closed on an unknown non-null `ItemClass`.** `RegistryValidationService` reports only `ItemClass is null`; it has no closed D8 vocabulary check for either DOCEFL rules or DOCFLG snapshots. An imported or native DTO row carrying `Invented` therefore produces no identity-class failure. The disposable test proves only that PostgreSQL rejects an unknown DOCEFL value. It does not exercise imported and native DOCFLG rows, and no validator test exercises either row class. Amendment 2 kept the identity-class validator gate binding, amendment 3 changes strategy rather than substance, and Piece D is verification-only, so this missing validator behavior cannot be deferred to a later implementation piece. The database constraints remain a useful independent barrier, but they do not satisfy the required validator gate or its first-order coverage. #### Medium 1. **The new generated column is not modelled with its actual PostgreSQL store type.** SQL creates `DOCFLG.ImportTargetTable` as `varchar(32) GENERATED ALWAYS AS ('DOCFLG') STORED`, while the runtime model and EF snapshot map it as `text`. The generated/stored expression is present, so the prior ordinary-column defect is improved, but the EF model still does not match the SQL catalog exactly as Piece A requires. The model test checks the expression and stored flag only; it never compares the relational store type with the disposable catalog. 2. **The mandatory company-isolation proof for the imported terminal exemption is missing.** The implementation uses the accepted strong four-column FK `(ImportEvidenceRowId, Company, ImportTargetTable, FlagInstanceID)`, and static inspection confirms its company-scoped shape. The disposable test exercises only matching-company evidence. It never creates same-code evidence for another company and proves that it cannot exempt a DOCFLG row, despite the standing remediation requirement and this review's explicit risky-behavior test requirement. This is a proof failure, not a finding that the inspected FK itself is cross-company. #### Low 1. **The status overstates the accepted-uniqueness/catalog proof.** The catalog helper captures selected columns and table constraints, then compares accepted pre-Up with post-Down and first Up with second Up. It does not compare the accepted `ImportEvidenceRow` uniqueness definitions with first Up, does not enumerate indexes, and omits column store types. Source inspection confirms this candidate does not alter the accepted uniqueness constraints, and the scoped Down comparisons pass; the claimed direct proof nevertheless does not assert all the state it says it proves. ### Requirement-by-requirement disposition | Requirement | Disposition | |---|---| | Reformulated preflight checks 1–5 and check-6 non-adoption rule | **Pass.** Pin reachable and ancestral; local prototype HEAD equals the pin; effective amended roster 49/49 with zero mismatches; pinned direct JSON surface 48/48 with zero delta. Live tip remains `3dd4150…` with the two recorded post-pin hashes/subjects. No post-pin content was read or adopted. | | Exact base-to-candidate surface and Piece A strict scope | **Pass.** Thirteen changed paths: one status record plus 12 implementation/model/test paths. No Piece B/C/D role, ACL, function, command, provenance-invariant, importer, or live-action surface. | | O9-D1 `ENTMST.DirectDebit` nullable, no default | **Pass.** SQL, model nullability, real NULL insert/read, and 119-row observation reproduced. | | O9-D2 `ENTMST.TaxIdVerificationStatus` nullable without invented sentinel | **Pass.** Genuine NULL survives the production reader; no production `Verified` default exists. | | O9-D3 nullable DOCEFL `ItemClass` | **Partial / Reject.** Nullable schema and NULL observation pass; the required unknown non-null validator identity gate is absent. | | O9-D4 exact six `ReviewPriority` literals | **Pass.** Both DOCEFL and DOCFLG CHECKs contain exactly the six governed literals; all six insert and `Normal` is rejected. | | O9-D5 imported terminal exemption with strong company-scoped evidence identity | **Partial.** FK and CHECK semantics are correctly company-qualified by inspection; the mandated other-company rejection test is missing. Native terminal evidence remains required. | | O10-D1 nullable DOCFLG snapshot, retained composite `MATCH SIMPLE`, plain EFCode FK, prospective completeness | **Pass.** Imported NULL inserts; native incomplete rows fail; composite FK remains `MATCH SIMPLE`; plain EFCode FK rejects a missing rule. | | Accepted fixture `EF0000053` | **Pass.** Candidate fixture and assertions use `EF0000053`; historical `EF0000046` is removed from that accepted fixture surface. | | Accepted P1-1a migration untouched | **Pass.** Both accepted migration files have the same Git blob IDs as the records base. | | Accepted `ImportEvidenceRow` uniqueness unchanged | **Pass by exact source/catalog inspection; coverage incomplete.** Names and definitions are unchanged, but the mapped test does not directly compare accepted versus first-Up definitions. | | R5 true SQL NULL round trip; no invented `false` / `Verified` / `Annotation` defaults | **Pass.** 119 genuine PostgreSQL NULL pairs traverse `PostgresRegistryValidationSnapshotSource` and remain nullable in DTO/validator output. Production readers add no such defaults. | | Complete imported ENTMST population and 119/119 proof | **Pass.** All 119 synthetic imported entities are unreferenced by FDCHDR/FDCDTL yet are loaded and counted 119/119. Query company scope is retained. | | EF generated-column/catalog alignment | **Fail.** Generated/stored expression is mapped, but `text` does not match SQL `varchar(32)`. | | Exact Piece A Down and Up/Down/Up stability | **Pass for the exercised touched constraints/columns, with the Low coverage limitation above.** Accepted pre-Up equals post-Down and first Up equals second Up in the disposable run. No roles, ACLs, or functions are created by Piece A. | | Existing-row compatibility, null handling, reader types, rollback safety | **Pass by tests and inspection.** Up only relaxes nullability, completes a vocabulary CHECK, and adds constraints satisfied by accepted rows; readers use `IsDBNull`/nullable access. Down fails closed rather than inventing values if Piece A NULL data remains. | ### Missing coverage - Validator rejection of unknown non-null DOCEFL and DOCFLG `ItemClass`, for imported and native rows. - Same-code, wrong-company `ImportEvidenceRow` rejection at the DOCFLG terminal exemption. - EF relational store-type comparison for the new generated column. - A direct accepted-versus-first-Up assertion for both accepted `ImportEvidenceRow` unique constraints, plus the complete relevant index/column-type catalog surface claimed by status. ### Positive confirmations - The accepted P1-1a migration and its SQL blob are byte-identical to the records base; accepted `ImportEvidenceRow` uniqueness is not changed by the migration. - The migration retains the original composite DOCFLG FK, adds the plain EFCode FK, and uses the accepted strong company-qualified import-evidence identity. No caller-controlled authority, role, function, command, audit, importer, or provenance-invariant surface appears. - Nullable DTO fields are explicit; the production PostgreSQL reader preserves SQL NULL and uses the correct Boolean/string reader paths. - O9-D1/D2/D3 nullability, both six-literal priority constraints, O9-D5 native/imported behavior, O10-D1 database semantics, and the `EF0000053` fixture behavior all work in the disposable database on their covered paths. ### Exact independent verification | Verification | Result | |---|---:| | `dotnet build GOTT.Sibyla.slnx --no-incremental` | **Pass** — 0 warnings, 0 errors; 13.71 s | | Piece A + validator + authored-fixture focused tests | **32/32 pass**, 0 failed, 0 skipped; 2 s | | Ordinary test suite | **632/632 pass**, 0 failed, 0 skipped; 16 s | | Disposable PostgreSQL 17 suite | **13/13 pass**, 0 failed, 0 skipped; 11 s test duration | | Genuine unreferenced ENTMST round trip | **119/119 DirectDebit NULL; 119/119 tax-status NULL** through the production source | | Effective prototype roster/surface | **49/49, 0 mismatches; 48/48 JSON, 0 delta** | | Whitespace/risk scans | `git diff --check` clean; no added roles/functions/commands/`SECURITY DEFINER`/`DROP OWNED`; accepted migration blobs identical | | Residual `sibyla-p11*` containers | **0** | Post-neutralization verification also passed: the exact 12-path roster has **0 mismatches** against `bc835f7e…`; all three candidate-added paths are absent; build is 0 warnings/errors; the ordinary accepted-tree suite is **629/629**; the accepted disposable PostgreSQL suite is **10/10**; and the residual container count remains 0. ### Disposition Reject Piece A candidate `6f86023d…`. The normal branch-tip neutralization commit restores all 12 implementation/model/test paths to records base `bc835f7e…` and preserves the amendment-4 records, candidate evidence, this review, and the stop/state records. Do not remediate. This is Piece A Reject #1; Piece B, Piece C, Piece D, and Scope 1 remain gated and unstarted. O8/O9/O10 decisions, the five baselines 119 / 119 / 52 / 221 / 2,787, and the two future `Routine` import rows remain unchanged. ## Third implementation review — Reject **Date:** 2026-08-06 **Branch:** `feature/p1-1b` **Reviewed implementation:** `7ea6c0f16ce5c9328b881d5abec3c74b44063000` **Records parent preserved:** `01c92cb2d214331a0b32de8a571031e7200125c9` **Verdict:** **Reject** — 0 Critical, 2 High, 4 Medium, 1 Low This is the fresh independent adversarial review required by amendment-2 step 4. The candidate was reproduced at the pushed remote tip and its green build and test suites were treated as evidence, not semantic proof. The Reject path restores all 15 implementation code/test/migration/model paths changed by `7ea6c0f…` byte-for-byte to records parent `01c92cb…`. This is the third Reject, so all further reimplementation stops pending Miguel's review of the pattern itself. O9-P/D1…D5 and O10-D1/D2 remain closed; Scope 1 has not run. ### Findings #### Critical None. #### High 1. **R4 provenance integrity is bypassable and breaks an accepted terminal command.** The provenance constraint trigger returns immediately on `DELETE`, so a valid target can lose one or both required rows without a cardinality recheck. Moving a provenance row also never checks the old target. DOCEFL insertion has no exact-one cardinality trigger. On DOCFLG target updates, the trigger checks only that two rows exist, not that their `absent` state and SHA-256 still match the governed values. Consequently accepted `WaiveDOCFLG` can populate `ResolutionEvidence` while leaving its provenance `absent` with a NULL hash. R4's target, cardinality, absence, and hash invariants therefore do not cover every governed state or path. 2. **The assignment command crosses the authenticated company boundary.** `P11aCurrentActor` authorizes one selected company, but `AssignDOCEFLItemClass` updates every company's DOCFLG rows for the global rule without proving authority for each affected company. The mapped test grants the test principal both companies and therefore misses this case. Its audit insert also joins only active companies, so affected rows of an inactive company are updated but receive no company audit. This violates company-scoped business-significant writes and R6's every-company audit requirement. #### Medium 1. **The governed ENTMST counts are not reproducible.** The PostgreSQL validator source selects ENTMST only through CodeNames referenced by FDCHDR/FDCDTL. The 119-row DirectDebit and tax-status baselines govern every imported company entity, so unreferenced imported entities disappear from both findings and their counts. 2. **R5 has no genuine NULL round trip.** The mapped test constructs nullable records in memory and reflects over constructor defaults. It never writes SQL NULL, reads it through `PostgresRegistryValidationSnapshotSource`, and proves the DTO/validator value remains NULL. This does not satisfy the expressly required round-trip proof. 3. **R2's claimed exact-catalog proof is materially incomplete.** `CatalogSnapshot` omits the two created roles, role memberships and attributes, schema ACLs, the added provenance relation and its owner/ACL/indexes, and the added command/validation functions and their ACLs. It therefore cannot prove the required object/owner/ACL equivalence even though the exercised Up/Down/Up cycle is green. 4. **The EF model snapshot does not describe the generated catalog.** New generated columns `DOCFLG.ImportTargetTable`, `RegistryFieldProvenance.DOCEFLCode`, and `RegistryFieldProvenance.DOCFLGFlagInstanceID` are modelled as ordinary nullable text columns, unlike the existing generated-column mappings. Migration target model and runtime configuration agree with each other but not with the SQL catalog, making later migration generation unsafe. #### Low 1. **The first R3 proof is sequential, not a concurrency ordering.** It proves assignment followed by import, while amendment 2 explicitly requires two concurrency tests, one for each ordering. Static inspection confirms both commands lock the DOCEFL parent and the opposite in-flight ordering is genuinely exercised, but the mapped first test does not provide the required concurrent proof. ### Requirement and coverage disposition - Standing requirement 1: **confirmed** — imported NULL snapshots are accepted; native incomplete snapshots and unknown non-null ItemClass values fail at the database. - Standing requirement 2: **confirmed** — non-owner forged GUCs grant no authority and trusted authority resolves from `session_user` through `RuntimePrincipal`. - Standing requirement 3: **confirmed** — DOCFLG evidence is bound by Id, company, target table and target permanent code to a completed batch with non-null completion and valid result. - Standing requirement 4: **not proven** — the cycle passes, but the exact catalog proof omits material role, ACL, function and new-object state. - Standing requirement 5: **confirmed** — imported and native unknown non-null ItemClass values fail closed in both database and validator. - R1: **confirmed** — hostile expected-role reuse fails closed; normal roles are NOLOGIN, non-superuser, non-createdb and non-createrole, with only governed execute/schema grants. - R2: **not proven** — no `DROP OWNED`, accepted `WaiveDOCFLG` ownership is untouched, and the tested accepted terminal function restores, but the mandated exact catalog surface is incomplete. - R3: **semantics confirmed, coverage incomplete** — both commands lock the parent rule and the import-in-flight ordering is exercised; the assignment-first case is not a concurrency test. - R4: **failed** — exact provenance cardinality and value/hash consistency are bypassable. - R5: **implementation shape confirmed, required proof missing** — defaults were removed and source readers are nullable, but no real database round trip exists. - R6: **failed** — assignment is not authorized for every affected company and can omit affected inactive-company audits; evidence-append audit does include the governed value and hash. - R7: **confirmed** — accepted `ImportEvidenceRow` unique constraints remain unchanged in name and definition. - Four coverage items: unknown ItemClass and hostile-role coverage pass; exact post-Down catalog coverage is incomplete; only one of the two required concurrency orderings is genuinely concurrent. - Minimal surface: **confirmed with one model-consistency defect** — accepted P1-1a migration and accepted uniqueness objects are untouched, the authorized EF0000053 fixture update is scoped, and no unnamed ownership/regrant/drop surface was introduced. ### Other positive confirmations - The three O9 nullability relaxations plus nullable DOCFLG snapshot, exact six-literal priority CHECK, plain and composite DOCFLG FKs, prospective completeness CHECK, terminal-evidence exemption, and the five governed completeness finding names are present. - Assignment/import parent locking prevents the reviewed NULL-snapshot race and assignments update NULL snapshots only. Multi-company count rows and evidence value/hash content are emitted on the covered active/authorized path. - No rejected implementation was resurrected, no caller-controlled actor/authority GUC became an authority source, no `DROP OWNED` appears, and accepted `ImportEvidenceRow` uniqueness survives. - Independent verification reproduced build 0 warnings/errors, ordinary 636/636, validator-focused 33/33, disposable PostgreSQL 18/18, clean diff checks, and zero residual `sibyla-p11*` containers. ### Disposition Amendment-2 step 4's Reject path is mandatory. The additive branch-tip stop commit neutralizes all 15 implementation paths and records this verdict. Because this is the third Reject, no fourth implementation attempt is authorized: Miguel must review the repeated failure pattern before any reimplementation. Scope 1 and every later scope remain stopped; no prototype, shared/live database, service, deployment, or live-data action is authorized. --- ## Second implementation review — Reject **Date:** 2026-08-06 **Branch:** `feature/p1-1b` **Reviewed implementation:** `57f0f023bb1ff1d117b5767103bf4df310fb4a98` **Records parent preserved:** `15679e78` **Verdict:** **Reject** — 0 Critical, 4 High, 3 Medium This is the mandatory independent review required by O10 amendment step 4. Green tests did not waive the findings below. The implementation is rejected and the normal additive stop commit that contains this record restores all 14 code/test/migration/model paths changed by `57f0f023…` byte-for-byte to parent `15679e78`. O10-D1 and O10-D2 remain closed decisions; the rejected code is evidence only. Scope 1 has not started. ## Findings ### High 1. **Unsafe role-name reuse.** The migration accepts hostile pre-existing roles with the expected names and then grants them ownership, DML, and `SECURITY DEFINER` privileges. 2. **Destructive and non-identical `Down`.** `Down` changes the owner of `WaiveDOCFLG`, restores a non-identical P1a function, and uses broad `DROP OWNED`; the cycle test does not prove the exact post-Down catalog and semantics. 3. **Assignment/import race.** Concurrent rule assignment and import can leave a NULL snapshot after assignment without audited completion. Remediation requires serialization on `EFCode` or a locked parent and tests for both concurrency orderings. 4. **Incomplete provenance integrity.** `RegistryFieldProvenance` lacks target integrity and does not enforce company correctness or exact absent cardinality, allowing missing, orphan, and contradictory rows. ### Medium 1. **Invented validator values.** Validator DTO defaults invent `false`, `Verified`, and `Annotation` instead of preserving null or requiring an explicit value. 2. **Incorrect audit scope/content.** A global assignment can affect several companies while writing one caller-company audit; evidence-append audit omits the governed value/hash. 3. **Unauthorized uniqueness change.** The migration silently drops the accepted `ImportEvidenceRow` uniqueness constraints and replaces them with partial indexes outside its authority. ### Low / missing coverage Coverage is missing for unknown non-null DOCFLG `ItemClass` on both imported and native rows, a hostile pre-existing role, the exact post-Down catalog, and both assignment/import concurrency orderings. ## Positive confirmations - No rejected `b324a3e…` implementation was resurrected and no caller-controlled GUC was made an authority source. - Accepted P1a blobs were unchanged. - Plain and composite DOCFLG links, evidence links, and the governed baselines were present. - The accepted P1a fixture was scoped correctly for the O8-authorized `EF0000053` update. - The test suites were green, but that result did not waive the semantic and security findings. ## Disposition The verdict activates only O10 amendment step 4's Reject path. The branch-tip stop commit neutralizes the 14 implementation paths, records this verdict, and stops. It does not remediate or reimplement the findings, reopen O10-D1/D2, start Scope 1, modify the governed 2,787 baseline, or authorize database, container, service, deployment, live-data, or prototype work.