Skip to content

feat(workforce-validation): adopt registry into owner persistence - #248

Draft
seonghobae wants to merge 18 commits into
feat/workforce-validation-registry-boundaryfrom
feat/workforce-validation-owner-persistence
Draft

feat(workforce-validation): adopt registry into owner persistence#248
seonghobae wants to merge 18 commits into
feat/workforce-validation-registry-boundaryfrom
feat/workforce-validation-owner-persistence

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Why and live stack truth — 2026-09-06

Issue #247 owns the next FR-007 durability increment. Protected architecture assigns the validity-study registry to workforce_validation; parent #235 establishes the governed application boundary and owner-schema bootstrap, while this Draft child adopts the existing registry relation into the owner schema and adds the least-privilege PostgreSQL read adapter.

Current child exact head remains d54d44d795444df572efbb301a667d74ac574d58. Its recorded mutable-parent base remains old 656a0c41c06bc517b2cf7c554e35a6fb4f8c4f4b, while parent #235 is exact dd95dd7256f37aab2c4f26aa1fb43e8c867f4e4d. Mechanical mergeability is not acceptance evidence for this moving-parent stack. The substantive prerequisite is still protected integration of #235; #248 must not copy or prematurely absorb mutable parent source.

Child lineage retained

The child preserves owner-port and forward PostgreSQL adoption contracts proving relation identity, inbound FK, forced RLS and runtime ACL preservation rather than registry copying. The forward migration uses ALTER TABLE public.validity_study SET SCHEMA workforce_validation, retains a NOLOGIN schema/migration owner and a separate deny-default runtime role, and repairs the pre-existing normalized-case trigger function in place so its study lookup follows the owner relation. PostgresValidityStudyReadPort remains schema-qualified, read-only, transaction-local for tenant context, UUID-snapshot-bound and bounded. #250/#251/#252 retain the tuple-owned connection capability, migrated trigger-function lookup and exact stored-callable execution regressions.

No compatibility copy/view, cross-service application SQL, protected global migration rewrite, force-push, destructive rebase, gate weakening, administrator bypass or mutable People/#235 source copy belongs here.

Parent exact-head evidence

Parent predecessor e87d28a32683c6e6f115b3d13645b7d263451795 produced the real #255 RED: Workforce Validation ran 25/25 tests but reached only 99.04% coverage on the fail-closed exact-string permitted_fields branch. Test-only successor dd95dd725... closes that branch without production change.

On exact parent dd95dd725..., Foundation 33986151272 is terminal SUCCESS, including owned unit/service contracts and isolated PostgreSQL contracts; SAST 33986151255 is terminal SUCCESS. Security 33986151270 is terminal FAILURE only because the central fail-closed dependency-review support probe cannot establish the public dependency comparison and therefore does not run the pinned action; owner .github#810. CodeQL 33986151302 is terminal FAILURE only after both Python and Actions compatibility jobs successfully request current-head dispatch and then cannot obtain the terminal central verdict; active owner incidents .github#1927/#1929. These are non-passing parent controls, not child source defects and not permission for a leaf workaround. No qualifying parent APPROVED review is established.

Required non-force adoption order

#235 must first complete authoritative central/security/review acceptance and integrate normally to protected develop@eb9757f8649aaad026a9865508d9aad50c1a7a4f or its then-current successor. Only then may #248 non-force adopt that protected parent truth and retarget to develop.

The reconciled child must preserve all valid parent trust/provenance delta—#249 exact repository-capability identity, #253 Protocol-placeholder rejection, #254 dual inventory + manifest provenance, #255 exact policy-field coverage regression—together with this child's owner migration/adapter/#250/#251/#252 delta. It must then reacquire fresh exact-head 100% owned coverage, isolated PostgreSQL owner-adoption GREEN, terminal required security/central workflows, current review evidence and normal protected admission. The earlier CodeRabbit no-defect review on d54d44d... remains static evidence only and cannot authorize a later restacked head.

Issues #247/#250/#251/#252 stay open through fresh acceptance and protected integration. Stale base, temporary mergeability, source implementation, predecessor review or mutable-parent success is not a close condition.

@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Please review exact head 3b5ebe8341711e72cf3bacc592af8eddae06c9f5 against parent 656a0c41c06bc517b2cf7c554e35a6fb4f8c4f4b, focusing on Issue #247: PostgreSQL ALTER TABLE ... SET SCHEMA relation/FK/RLS/trigger preservation, least-privilege runtime-role semantics, transaction-local tenant binding, DB-API row trust boundaries, UUID snapshot/reconstruction ordering, and 100% branch-coverage plausibility. Treat this as independent static review only; do not infer protected-branch approval or hosted GREEN.

@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

parent='656a0c41c06bc517b2cf7c554e35a6fb4f8c4f4b'
head='3b5ebe8341711e72cf3bacc592af8eddae06c9f5'

echo '== Commit availability =='
git show -s --format='%H%n%P%n%s' "$parent"
git show -s --format='%H%n%P%n%s' "$head"

echo '== Changed files =='
git diff --name-status "$parent" "$head"

echo '== Diff summary =='
git diff --stat "$parent" "$head"

echo '== Diff =='
git diff --find-renames --find-copies --unified=80 "$parent" "$head"

echo '== Candidate source and test file outlines =='
fd -t f -E .git | rg -i '(postgres|validity|registry|migration|workforce|read.*port|contract|test)'

Length of output: 46415


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== Registry model outline =='
ast-grep outline services/workforce-validation-api/src/orgmetra_workforce_validation_api/registry.py --items all

echo '== Registry model UUID and record implementation =='
rg -n -A35 -B8 \
  'def _store_operational_uuid|def _restore_operational_uuid|class ValidityStudyRecord|def __post_init__' \
  services/workforce-validation-api/src/orgmetra_workforce_validation_api/registry.py

echo '== Foundation validity-study DDL, RLS, triggers, grants, and tenant helper =='
rg -n -i -A45 -B12 \
  'create table.*validity_study|alter table.*validity_study|create policy|validity_study_bitemporal_guard|current_tenant_record_id|grant .*validity_study|revoke .*validity_study|default privileges' \
  database/migrations/0001_foundation_schema.sql

echo '== Owner schema migration and package coverage configuration =='
cat -n services/workforce-validation-api/database/migrations/0001_owner_schema.sql
cat -n services/workforce-validation-api/pyproject.toml

echo '== Existing PostgreSQL adapter transaction patterns =='
ast-grep outline services/job-analysis-api/src/orgmetra_job_analysis_api/postgres.py --items all
rg -n -A25 -B10 \
  'SET TRANSACTION READ ONLY|set_config|fetchmany|fetchone|connection_factory' \
  services/job-analysis-api/src/orgmetra_job_analysis_api/postgres.py \
  services/people-api/src/orgmetra_people_api/postgres.py \
  services/people-api/src/orgmetra_people_api/postgres_hire.py

echo '== Coverage workflow and service test selection =='
rg -n -A20 -B10 \
  'workforce-validation-api|coverage|branch|test_workforce_validation_owner_schema_postgres' \
  .github scripts package.json pyproject.toml 2>/dev/null || true

Length of output: 98825


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '== Complete ValidityStudyRecord construction and properties =='
sed -n '190,270p' services/workforce-validation-api/src/orgmetra_workforce_validation_api/registry.py

echo '== All direct SQL references to validity_study, excluding generated artifacts =='
rg -n -i -C3 \
  '\b(public\.)?validity_study\b|validity_study_scope_policy|current_tenant_record_id\s*\(' \
  -g '!**/.git/**' -g '!**/node_modules/**' .

echo '== Privilege and membership statements for workforce roles =='
rg -n -i -C4 \
  '\b(create|alter|grant|revoke|drop)\s+(role|schema|table|function)|workforce_validation_(runtime_)?role' \
  database services tests .github

echo '== Exact adapter line map and test assertions =='
nl -ba services/workforce-validation-api/src/orgmetra_workforce_validation_api/postgres_registry.py
nl -ba services/workforce-validation-api/tests/test_postgres_registry.py

echo '== Migration ordering and schema-qualified migration references =='
rg -n -C3 \
  '0001_owner_schema|0002_registry_adoption|database/migrations' \
  README.md ARCHITECTURE.md TRD.md .github services tests scripts 2>/dev/null || true

echo '== Git diff check for unreviewed changes =='
git diff --check 656a0c41c06bc517b2cf7c554e35a6fb4f8c4f4b 3b5ebe8341711e72cf3bacc592af8eddae06c9f5

Length of output: 207696


@seonghobae

Static review finding

[P1] Update the existing validity-study case trigger function during adoption

services/workforce-validation-api/database/migrations/0002_registry_adoption.sql moves public.validity_study at Line 15. However, database/migrations/0010_validity_study_case_integrity.sql defines public.validate_validity_study_case() with FROM public.validity_study AS study at Line 142.

ALTER TABLE ... SET SCHEMA preserves relation-attached objects. It does not rewrite the schema-qualified SQL text in a PL/pgSQL function body. After adoption, inserts or updates that invoke the validity_study_case_record validation trigger will fail because public.validity_study no longer exists.

Update or replace that function in the adoption migration so that it reads workforce_validation.validity_study. Extend the isolated PostgreSQL contract to apply the existing case-integrity migration before adoption and then execute a valid case write. This proves that the preserved foreign key and the trigger-function dependency both remain operational.

The static review found no additional defects in the requested areas. The migration uses relation identity preservation. The adapter uses a read-only transaction, transaction-local tenant context, schema-qualified parameterized SQL, pre-connection UUID snapshots, bounded fetch, canonical row reconstruction, and target validation. The unit-test structure also makes 100% branch coverage plausible. This review does not infer hosted GREEN, protected-branch approval, or merge eligibility.

You are interacting with an AI system.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Please review exact child head 5d390bc4e86c6db7bcf95a51662cc71500811ad3 against its recorded parent base 656a0c41c06bc517b2cf7c554e35a6fb4f8c4f4b, focusing on #250: PostgresValidityStudyReadPort must preserve its validated connection_factory against retained-reference replacement while keeping #247 read-only/RLS/schema-qualified/target-integrity behavior intact. Parent #235 is mutable; this review is independent static evidence only and must not be treated as protected-branch merge acceptance.

@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

base='656a0c41c06bc517b2cf7c554e35a6fb4f8c4f4b'
head='5d390bc4e86c6db7bcf95a51662cc71500811ad3'

echo '== Exact commits =='
git show -s --format='commit=%H%nparents=%P%nsubject=%s' "$base"
git show -s --format='commit=%H%nparents=%P%nsubject=%s' "$head"

echo '== Changed files =='
git diff --name-status "$base" "$head"
echo '== Diff check =='
git diff --check "$base" "$head"
echo '== Full review diff =='
git diff --find-renames --find-copies --unified=100 "$base" "$head"

echo '== Candidate files =='
fd -t f -E .git | rg '(postgres_registry|test_postgres_registry|registry\.py|registry_adoption|validity_study_case_integrity|foundation_schema|pyproject\.toml)$'

echo '== Adapter and test outlines =='
ast-grep outline services/workforce-validation-api/src/orgmetra_workforce_validation_api/postgres_registry.py --items all
ast-grep outline services/workforce-validation-api/tests/test_postgres_registry.py --items all

echo '== Complete adapter and contract test sources =='
nl -ba services/workforce-validation-api/src/orgmetra_workforce_validation_api/postgres_registry.py
nl -ba services/workforce-validation-api/tests/test_postgres_registry.py

echo '== Model construction and UUID snapshot/reconstruction contract =='
rg -n -A55 -B12 \
  'class ValidityStudyRecord|def __post_init__|def _store_operational_uuid|def _restore_operational_uuid' \
  services/workforce-validation-api/src/orgmetra_workforce_validation_api/registry.py

echo '== Adoption and pre-existing trigger SQL =='
nl -ba services/workforce-validation-api/database/migrations/0002_registry_adoption.sql
sed -n '110,175p' database/migrations/0010_validity_study_case_integrity.sql

echo '== Adapter use sites and package exports =='
rg -n -C4 \
  '\bPostgresValidityStudyReadPort\b|\bconnection_factory\b|from .*postgres_registry import' \
  services/workforce-validation-api

Length of output: 42608


@seonghobae

Static review result

The #250 change correctly prevents retained-reference replacement of the accepted connection_factory.

PostgresValidityStudyReadPort has no instance dictionary or writable slots. It stores the validated factory in tuple storage. The connection_factory property reads that tuple value. The regression test uses object.__setattr__, confirms the assignment fails, and confirms that only the original factory runs.

The adapter retains the #247 controls:

  • It validates and snapshots both UUID targets before it calls the factory.
  • It starts a read-only transaction.
  • It sets tenant context with transaction-local pg_catalog.set_config.
  • It uses parameterized SQL against workforce_validation.validity_study.
  • It fetches at most two rows.
  • It rejects non-canonical, malformed, duplicate, and foreign-target rows.
  • It reconstructs validated ValidityStudyRecord evidence before it returns a result.

Unresolved finding

[P1] Update the preserved case-integrity trigger function during adoption

services/workforce-validation-api/database/migrations/0002_registry_adoption.sql moves public.validity_study at Line 15. The existing public.validate_validity_study_case() function in database/migrations/0010_validity_study_case_integrity.sql still queries public.validity_study.

ALTER TABLE ... SET SCHEMA preserves the trigger attachment, but it does not rewrite schema-qualified SQL inside the PL/pgSQL function body. If the case-integrity migration has run, a case write that invokes this function will fail after adoption because public.validity_study no longer exists.

Update or replace the function in the adoption migration so it queries workforce_validation.validity_study. Extend tests/test_workforce_validation_owner_schema_postgres.sh to apply the case-integrity migration before adoption and then perform a valid case write.

I found no additional static defect in the #250 dependency-binding change. This result is independent static evidence only. It does not establish hosted GREEN, protected-branch approval, or merge acceptance.

You are interacting with an AI system.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Please review exact head b210b6bd201a9b62949da33bc4a9feb92ec6a095 for #251 against the current protected-owner context. Focus on whether forward registry adoption preserves the existing public.validate_validity_study_case() trigger semantics after public.validity_study moves to workforce_validation.validity_study, including the normalized case insert regression, function identity/signature, non-study evidence reads, inbound FKs, RLS, bitemporal guard, runtime ACL, and absence of compatibility copies/views or protected global migration rewrites. Do not transfer predecessor review evidence.

@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

base='656a0c41c06bc517b2cf7c554e35a6fb4f8c4f4b'
head='b210b6bd201a9b62949da33bc4a9feb92ec6a095'

echo '== Exact commits =='
git show -s --format='commit=%H%nparents=%P%nsubject=%s' "$base"
git show -s --format='commit=%H%nparents=%P%nsubject=%s' "$head"

echo '== Requested-head ancestry and changes =='
git merge-base --is-ancestor "$base" "$head" && echo 'recorded base is an ancestor'
git diff --check "$base" "$head"
git diff --name-status "$base" "$head"
git diff --stat "$base" "$head"
git diff --find-renames --find-copies --unified=100 "$base" "$head" -- \
  'services/workforce-validation-api/database/migrations' \
  'services/workforce-validation-api/tests' \
  'database/migrations/0010_validity_study_case_integrity.sql' \
  'database/migrations/0001_foundation_schema.sql'

echo '== Candidate file locations =='
fd -t f -E .git | rg '(^|/)(0002_registry_adoption\.sql|0010_validity_study_case_integrity\.sql|0001_foundation_schema\.sql|test_workforce_validation_owner_schema_postgres\.sh|test_postgres_registry\.py)$'

echo '== Adoption migration at exact head =='
git show "$head:services/workforce-validation-api/database/migrations/0002_registry_adoption.sql" | nl -ba

echo '== Protected function definition and relation governance before adoption =='
git show "$head:database/migrations/0010_validity_study_case_integrity.sql" | nl -ba | sed -n '1,280p'
git show "$head:database/migrations/0001_foundation_schema.sql" | nl -ba | \
  rg -n -C 20 'validity_study|bitemporal|current_tenant_record_id|CREATE POLICY|FORCE ROW LEVEL SECURITY|GRANT|REVOKE'

echo '== Owner PostgreSQL contract at exact head =='
git show "$head:services/workforce-validation-api/tests/test_workforce_validation_owner_schema_postgres.sh" | nl -ba

echo '== All relevant exact-head SQL references and compatibility-object checks =='
git grep -n -i -E \
  'public\.validity_study|workforce_validation\.validity_study|validate_validity_study_case|validity_study_case_record|CREATE( OR REPLACE)? (TABLE|VIEW).*validity_study|ALTER TABLE.*validity_study.*SET SCHEMA|workforce_validation_runtime_role' \
  "$head" -- \
  'database' 'services/workforce-validation-api' || true

echo '== Current GitHub PR context (metadata only) =='
gh pr view 248 --repo ContextualWisdomLab/Orgmetra \
  --json number,state,isDraft,baseRefName,baseRefOid,headRefName,headRefOid,mergeable,reviewDecision,url
gh pr view 235 --repo ContextualWisdomLab/Orgmetra \
  --json number,state,isDraft,baseRefName,baseRefOid,headRefName,headRefOid,mergeable,reviewDecision,url

Length of output: 21458


🏁 Script executed:

#!/bin/bash
set -euo pipefail

head='b210b6bd201a9b62949da33bc4a9feb92ec6a095'

echo '== Adoption migration at exact head =='
git show "$head:services/workforce-validation-api/database/migrations/0002_registry_adoption.sql" | cat -n

echo '== Protected case-trigger function before adoption =='
git show "$head:database/migrations/0010_validity_study_case_integrity.sql" | cat -n | sed -n '1,300p'

echo '== Validity-study definition, FK, RLS, trigger, and privilege context =='
git show "$head:database/migrations/0001_foundation_schema.sql" | cat -n | \
  grep -E -i -C 20 \
  'validity_study|validity_study_bitemporal_guard|current_tenant_record_id|CREATE POLICY|FORCE ROW LEVEL SECURITY|GRANT|REVOKE'

echo '== Owner PostgreSQL contract at exact head =='
git show "$head:tests/test_workforce_validation_owner_schema_postgres.sh" | cat -n

echo '== Exact-head relevant SQL references =='
git grep -n -i -E \
  'public\.validity_study|workforce_validation\.validity_study|validate_validity_study_case|validity_study_case_record|CREATE( OR REPLACE)? (TABLE|VIEW).*validity_study|ALTER TABLE.*validity_study.*SET SCHEMA|workforce_validation_runtime_role' \
  "$head" -- database services/workforce-validation-api tests || true

echo '== Migration inventory and explicit application order in the contract =='
git ls-tree -r --name-only "$head" -- database/migrations services/workforce-validation-api/database/migrations
git show "$head:tests/test_workforce_validation_owner_schema_postgres.sh" | \
  grep -n -E -C 4 '0001|0010|0002|psql|migration|validity_study_case_record'

echo '== Current GitHub PR context (metadata only) =='
gh pr view 248 --repo ContextualWisdomLab/Orgmetra \
  --json number,state,isDraft,baseRefName,baseRefOid,headRefName,headRefOid,mergeable,reviewDecision,url
gh pr view 235 --repo ContextualWisdomLab/Orgmetra \
  --json number,state,isDraft,baseRefName,baseRefOid,headRefName,headRefOid,mergeable,reviewDecision,url

Length of output: 92254


🏁 Script executed:

#!/bin/bash
set -euo pipefail

base='656a0c41c06bc517b2cf7c554e35a6fb4f8c4f4b'
parent_head='0ac2255321eaf1d0068978b931990f4d9c9f1c85'
child_head='b210b6bd201a9b62949da33bc4a9feb92ec6a095'

echo '== Current parent availability and delta from child base =='
git show -s --format='commit=%H%nparents=%P%nsubject=%s' "$parent_head"
git merge-base --is-ancestor "$base" "$parent_head" && echo 'recorded child base is an ancestor of current parent head'
git diff --name-status "$base" "$parent_head"
git diff --find-renames --find-copies --unified=40 "$base" "$parent_head" -- \
  database/migrations tests services/workforce-validation-api || true

echo '== Current parent and exact child references to the registry relation =='
for revision in "$parent_head" "$child_head"; do
  echo "-- ${revision} --"
  git grep -n -i -E \
    'public\.validity_study|workforce_validation\.validity_study|validate_validity_study_case|validity_study_case_record' \
    "$revision" -- database services tests || true
done

echo '== Full post-0010 global migrations at current parent head =='
for migration in \
  database/migrations/0011_criterion_observation_scope.sql \
  database/migrations/0012_people_mutation_idempotency.sql \
  database/migrations/0013_job_analysis_snapshot.sql
do
  echo "-- ${migration} --"
  git show "${parent_head}:${migration}" | cat -n
done

echo '== Replacement-function equivalence except the intended relation qualification =='
git show "$base:database/migrations/0010_validity_study_case_integrity.sql" \
  | sed -n '/CREATE FUNCTION public\.validate_validity_study_case()/,/^\$\$;/p' \
  | sed 's/public\.validity_study/workforce_validation.validity_study/g' \
  > /tmp/protected_function_normalized.sql
git show "$child_head:services/workforce-validation-api/database/migrations/0002_registry_adoption.sql" \
  | sed -n '/CREATE OR REPLACE FUNCTION public\.validate_validity_study_case()/,/^\$\$;/p' \
  | sed 's/CREATE OR REPLACE FUNCTION/CREATE FUNCTION/' \
  > /tmp/adoption_function_normalized.sql
diff -u /tmp/protected_function_normalized.sql /tmp/adoption_function_normalized.sql || true

Length of output: 69642


@seonghobae

Static review result

I found no defect in the requested #251 scope at exact head b210b6bd201a9b62949da33bc4a9feb92ec6a095.

services/workforce-validation-api/database/migrations/0002_registry_adoption.sql moves the registry with:

ALTER TABLE public.validity_study SET SCHEMA workforce_validation;

This preserves the table object. The contract verifies the same relation OID before and after adoption. It also verifies the existing inbound foreign-key dependency count, forced RLS flags, and validity_study_bitemporal_guard trigger.

The migration replaces public.validate_validity_study_case() in place. It retains the function name, zero-argument trigger signature, RETURNS trigger, plpgsql language, and search path. The function body matches the protected definition except for the required relation change from public.validity_study to workforce_validation.validity_study. The non-study reads remain qualified to their existing public evidence relations.

The existing validity_study_case_governance_guard stays attached to public.validity_study_case_record. PostgreSQL CREATE OR REPLACE FUNCTION updates the existing function body without changing the trigger function identity. The isolated PostgreSQL contract applies migrations through 0010_validity_study_case_integrity.sql, performs adoption, inserts a valid normalized case record, and verifies that the record persists. This regression would fail with the predecessor function body.

The adoption migration creates no compatibility view or copied registry. It removes the old public.validity_study relation name. It grants the separate workforce_validation_runtime_role only schema USAGE, registry SELECT, and execution of public.current_tenant_record_id(). The contract checks deny-default role flags, absence of mutation privileges, no read without tenant context, and a tenant-scoped read with context.

The current parent head 0ac2255321eaf1d0068978b931990f4d9c9f1c85 adds no database migration or additional registry dependency after the child base. The current-parent check found no new public.validity_study dependency outside the protected 0010 definition that this adoption migration replaces at runtime.

This is independent static evidence only. I did not execute the PostgreSQL contract or hosted workflows. GitHub currently reports #235 and #248 as Draft, and #235 still requires review. This result does not establish protected-branch acceptance, hosted GREEN, or merge eligibility.

You are interacting with an AI system.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Please review exact current head d54d44d795444df572efbb301a667d74ac574d58 against parent/base snapshot and focus on #252: PostgresValidityStudyReadPort.read_validity_study(...) must invoke the exact connection_factory stored by its validating tuple constructor, not a subclass-overridable dynamic property. Verify the test-first predecessor ceb72ff997fdf54640a146c8569b0ee225312507, direct tuple.__getitem__ repair, preservation of UUID snapshots/read-only tenant-bound SQL/row reconstruction, and absence of unrelated parent-owned changes. This review is static evidence only; do not treat it as hosted GREEN or merge authorization.

@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

base='656a0c41c06bc517b2cf7c554e35a6fb4f8c4f4b'
previous='b210b6bd201a9b62949da33bc4a9feb92ec6a095'
red='ceb72ff997fdf54640a146c8569b0ee225312507'
head='d54d44d795444df572efbb301a667d74ac574d58'

echo '== Exact commit metadata =='
for rev in "$base" "$previous" "$red" "$head"; do
  git show -s --format='commit=%H%nparents=%P%nsubject=%s%n' "$rev"
done

echo '== Ancestry =='
git merge-base --is-ancestor "$base" "$head" && echo 'base is ancestor of head'
git merge-base --is-ancestor "$previous" "$head" && echo 'previous child head is ancestor of head'
git merge-base --is-ancestor "$red" "$head" && echo 'test-first predecessor is ancestor of head'

echo '== Changed files: base to head =='
git diff --name-status "$base" "$head"
echo '== Changed files: previous child head to head =='
git diff --name-status "$previous" "$head"
echo '== Changed files: test-first predecessor to production head =='
git diff --name-status "$red" "$head"
git diff --check "$base" "$head"

echo '== Focused production delta =='
git diff --find-renames --find-copies --unified=100 "$previous" "$head" -- \
  services/workforce-validation-api/src/orgmetra_workforce_validation_api/postgres_registry.py \
  services/workforce-validation-api/tests/test_postgres_registry.py

echo '== Test-first predecessor delta =='
git diff --find-renames --find-copies --unified=100 "$previous" "$red" -- \
  services/workforce-validation-api/src/orgmetra_workforce_validation_api/postgres_registry.py \
  services/workforce-validation-api/tests/test_postgres_registry.py

echo '== Exact source and tests =='
ast-grep outline services/workforce-validation-api/src/orgmetra_workforce_validation_api/postgres_registry.py --items all
ast-grep outline services/workforce-validation-api/tests/test_postgres_registry.py --items all
git show "$head:services/workforce-validation-api/src/orgmetra_workforce_validation_api/postgres_registry.py" | nl -ba
git show "$head:services/workforce-validation-api/tests/test_postgres_registry.py" | nl -ba

echo '== Model UUID conversion and adapter use sites =='
git show "$head:services/workforce-validation-api/src/orgmetra_workforce_validation_api/registry.py" | nl -ba | sed -n '1,330p'
git grep -n -C 4 -E \
  'PostgresValidityStudyReadPort|connection_factory|read_validity_study|tuple\.__getitem__' \
  "$head" -- services/workforce-validation-api

echo '== Parent-owned scope check =='
git diff --name-status "$previous" "$head" -- database tests services/workforce-validation-api .github scripts README.md ARCHITECTURE.md TRD.md

Length of output: 12296


🏁 Script executed:

#!/bin/bash
set -euo pipefail

base='656a0c41c06bc517b2cf7c554e35a6fb4f8c4f4b'
previous='b210b6bd201a9b62949da33bc4a9feb92ec6a095'
red='ceb72ff997fdf54640a146c8569b0ee225312507'
head='d54d44d795444df572efbb301a667d74ac574d58'

echo '== Production adapter at exact head =='
git show "$head:services/workforce-validation-api/src/orgmetra_workforce_validation_api/postgres_registry.py" | cat -n

echo '== Existing adapter contract tests at exact head =='
git show "$head:services/workforce-validation-api/tests/test_postgres_registry.py" | cat -n

echo '== Bound-capability test-first predecessor at exact commit =='
git show "$red:services/workforce-validation-api/tests/test_postgres_registry_bound_capability.py" | cat -n

echo '== Bound-capability test at production head =='
git show "$head:services/workforce-validation-api/tests/test_postgres_registry_bound_capability.py" | cat -n

echo '== UUID model helpers and record validation =='
git show "$head:services/workforce-validation-api/src/orgmetra_workforce_validation_api/registry.py" \
  | grep -n -E -A55 -B12 \
    'def _store_operational_uuid|def _restore_operational_uuid|class ValidityStudyRecord|def __post_init__'

echo '== Test-first commit content and production repair content =='
git show --format=fuller --stat "$red"
git show --format=fuller --stat "$head"
git diff --unified=100 "$previous" "$red" -- \
  services/workforce-validation-api/tests/test_postgres_registry_bound_capability.py
git diff --unified=100 "$red" "$head" -- \
  services/workforce-validation-api/src/orgmetra_workforce_validation_api/postgres_registry.py

echo '== Complete child delta, with paths =='
git diff --find-renames --find-copies --name-status "$base" "$head"
echo '== Scope after prior child head =='
git diff --find-renames --find-copies --name-status "$previous" "$head"

echo '== Dynamic-property call and tuple-index access checks =='
git grep -n -C 3 -E \
  'self\.connection_factory\s*\(|tuple\.__getitem__\(self,\s*0\)|def connection_factory' \
  "$head" -- services/workforce-validation-api

Length of output: 43249


@seonghobae

Static review result

I found no defect in the requested #252 scope at exact head d54d44d795444df572efbb301a667d74ac574d58.

The test-first predecessor ceb72ff997fdf54640a146c8569b0ee225312507 adds a subclass regression. _SwitchingPort.connection_factory returns REPLACEMENT_FACTORY. The test requires inherited read_validity_study(...) to call only ORIGINAL_FACTORY, which the base constructor accepted and stored.

The production commit directly follows that predecessor. It changes only services/workforce-validation-api/src/orgmetra_workforce_validation_api/postgres_registry.py.

PostgresValidityStudyReadPort.read_validity_study(...) now obtains the executable factory with:

connection_factory = tuple.__getitem__(self, 0)

It then invokes the local connection_factory. This bypasses subclass property dispatch. A subclass cannot replace the factory selected by read_validity_study(...) through an overridden connection_factory property.

The public connection_factory property also reads via tuple.__getitem__(self, 0). Tuple storage and __slots__ = () continue to prevent direct retained-reference replacement.

The adapter retains the existing controls:

  • It snapshots and reconstructs both UUID targets before factory invocation.
  • It starts a read-only transaction.
  • It binds tenant context with transaction-local pg_catalog.set_config.
  • It uses parameterized SQL against workforce_validation.validity_study.
  • It fetches at most two rows.
  • It rejects non-canonical, malformed, duplicate, and target-mismatched rows.
  • It reconstructs a validated ValidityStudyRecord before return.

The focused delta after prior child head b210b6bd201a9b62949da33bc4a9feb92ec6a095 contains only the adapter repair and the new bound-capability regression test. It contains no unrelated parent-owned changes.

This is static evidence only. I did not execute tests, PostgreSQL acceptance checks, coverage collection, or hosted workflows. This review does not establish hosted GREEN, protected-branch approval, or merge authorization.

You are interacting with an AI system.

Copy link
Copy Markdown
Contributor Author

Parent-status correction for dependency ordering: #235 remains exact dd95dd7256f37aab2c4f26aa1fb43e8c867f4e4d and Draft, but its previously queued lanes have settled. SAST 33986151255 is terminal SUCCESS; Security 33986151270 is terminal FAILURE at the fail-closed Dependency Review support probe (central owner .github#810); CodeQL 33986151302 is terminal FAILURE after actions/python compatibility jobs successfully request exact-head dispatch and fail only waiting for an authenticated terminal verdict (central settings owner .github#1927/#1929 after protected #1932/#1926 source repairs). No qualifying approval exists.

This does not change the child order. #248 remains on old mutable-parent snapshot 656a0c41... at exact child d54d44d... and must not absorb #235 source directly. Wait for normal #235 protected integration; then non-force adopt protected truth, preserve parent #249/#253/#254/#255 plus child #250/#251/#252, retarget to develop, and reacquire all exact-head PostgreSQL/coverage/security/review evidence. The PR body's older queued/non-terminal parent paragraph is superseded by this comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant