Skip to content

feat: record explicit assignment category - #163

Draft
seonghobae wants to merge 94 commits into
developfrom
feat/explicit-assignment-category
Draft

feat: record explicit assignment category#163
seonghobae wants to merge 94 commits into
developfrom
feat/explicit-assignment-category

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Current exact stack

Refs #162. Assignment-category owner remains exact head 5d4133b29f19a8db92bda0e03ba3ae1470aae594. The valid semantic delta is preserved on predecessor protected base develop@ef1b143368cb6249c9520ca8cae10ebe844a5aa1; against current protected develop@eb9757f8649aaad026a9865508d9aad50c1a7a4f the PR remains open · Draft · non-mergeable. This is a repair/restack finding, not a close condition.

Orgmetra remains authoritative for Assignment category. New facts use explicit primary | concurrent_secondary; legacy_unspecified is historical/restoration provenance only. Correction/supersession remains owned by #164/#165.

Protected-owner prerequisites

Only after #63#64#65#141 integrate through ordinary protected gates should #163 non-force adopt resulting protected develop, preserve the explicit Assignment-category delta, resolve actual conflicts causally, and reacquire exact-head product/PostgreSQL/security/review evidence. #165 remains dependent.

Do not copy mutable-owner source, force-push, destructively rebase, self-approve, administrator-bypass, weaken gates, no-op retrigger, or transfer predecessor evidence.

@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Assignment 생성에 primary 또는 concurrent_secondary category를 요구합니다. 기존 행은 legacy_unspecified로 보존합니다. 도메인, HTTP, PostgreSQL, OpenAPI, 문서 및 품질 검증이 같은 계약을 사용합니다.

Changes

Assignment Category 계약

Layer / File(s) Summary
도메인 및 명령 계약
packages/hris-kernel/src/orgmetra_hris_kernel/*, services/people-api/src/orgmetra_people_api/mutations.py, services/people-api/tests/*category*, tests/*numeric*
AssignmentFact와 mutation 명령에 category를 추가했습니다. primary, concurrent_secondary, legacy_unspecified를 검증하고 중복 visible primary를 거부합니다. Decimal subclass도 거부합니다.
HTTP 및 PostgreSQL 저장소 연결
services/people-api/src/orgmetra_people_api/mutation_http.py, services/people-api/src/orgmetra_people_api/postgres_mutations.py, services/people-api/tests/*
HTTP 요청에서 category를 파싱합니다. Idempotency digest에 category를 포함합니다. PostgreSQL 읽기·쓰기와 AssignmentFact 복원에 category를 연결합니다.
PostgreSQL 마이그레이션 및 불변식
database/migrations/0017_assignment_category_code.sql, tests/test_assignment_category_postgres.sh
기존 행을 legacy_unspecified로 백필합니다. category CHECK, 쓰기 guard 및 tenant/employment 범위의 primary GiST exclusion을 추가합니다. 롤백, 신규 쓰기, 재분류 및 중복 primary를 검증합니다.
공개 계약 및 품질 검증
schemas/openapi.yaml, docs/*, docs/adr/*, .github/workflows/*, manifest.json, scripts/foundation-contract-core.mjs, tests/*
OpenAPI와 문서에 명시적 category 계약을 반영합니다. ADR, traceability, manifest 및 필수 파일 검증을 추가합니다. PostgreSQL·provenance·OpenAPI 품질 검증 워크플로를 추가합니다.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: 🟡 Moderate · up to e27c5

New kernel assignments can bypass explicit category classification, while some contract-only changes can skip the dedicated validation workflow. Both issues should be corrected before merge.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant PeopleAPI
  participant Kernel
  participant PostgreSQL
  Client->>PeopleAPI: assignment_category_code 포함 생성 요청
  PeopleAPI->>Kernel: category 및 allocation 검증
  Kernel-->>PeopleAPI: 검증된 AssignmentFact
  PeopleAPI->>PostgreSQL: category 포함 INSERT
  PostgreSQL-->>PeopleAPI: 불변식 검증 및 저장 결과
  PeopleAPI-->>Client: Assignment 생성 결과
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.98% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 41 functions across 16 files. (11 skipped… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed PR은 이슈 #162의 핵심 요구사항을 충족합니다. 신규 assignment는 primary 또는 concurrent_secondary를 명시하고, 기존 행은 legacy_unspecified로 보존합니다. category는 Python, HTTP, PostgreSQL, OpenAPI, persistence 및 문서 계약에 반영됩니다. 멱등성 digest와…
Out of Scope Changes check ✅ Passed 변경 사항은 이슈 #162의 명시적 assignment category 계약, 영속성, 검증, 마이그레이션, 문서화 및 품질 게이트 범위에 있습니다. Decimal runtime integrity 보강도 이슈에서 요구한 fail-closed 검증에 해당합니다. 이슈 #164의 correction/supersession 기능이나 기타 unrelated 기능은…
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 신규 assignment category를 명시적으로 기록하는 주요 변경 사항을 정확하고 간결하게 설명합니다.
Full details: Docstring Coverage

Explanation

Docstring coverage is 60.98% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 41 functions across 16 files. (11 skipped: 11 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/explicit-assignment-category

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 1a3e146d07b4fb5ef8a5e77abd7b4848f5bfb35e. The prior two findings are now repaired and their threads resolved: new AssignmentFact construction no longer defaults to historical legacy_unspecified, and Assignment Category Quality covers the governed OpenAPI/docs/manifest validation surfaces with provenance resealed. Treat all predecessor review/check evidence as stale for this head.

@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown

@seonghobae I will review exact head 1a3e146d07b4fb5ef8a5e77abd7b4848f5bfb35e. I will treat all predecessor review and check evidence as stale.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

Please review exact current head 5d4133b29f19a8db92bda0e03ba3ae1470aae594, including the already-repaired explicit-category constructor/quality-trigger findings and the non-force adoption of protected develop@ef1b143368cb6249c9520ca8cae10ebe844a5aa1. Prior review/check evidence predates this exact head and is stale for integration authority.

@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown

@seonghobae I will review exact head 5d4133b29f19a8db92bda0e03ba3ae1470aae594. I will treat all predecessor review and check evidence as stale. I will include the repaired constructor and quality-trigger changes and the adoption of develop@ef1b143368cb6249c9520ca8cae10ebe844a5aa1.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor Author

Owner-path freshness update: canonical authorization/authentication owner #65 advanced non-force from e588c9165f2683a7d3399cc4f5e299b3207a6b95 to 5e1eb4a5146ca1fb29e93a66f7475c469c14a9ed. The new material repair makes tuple-backed People/Job Analysis principal __ne__ consistent with strict principal-only __eq__; existing compatibility tests already required raw-tuple inequality in both operand orders. #163 must not copy or consume mutable #65 source. Keep causal order #161#64/#65 ordinary protected integration → non-force protected adoption into #163 → exact-head revalidation.

Copy link
Copy Markdown
Contributor Author

Owner-path successor update: #65 advanced non-force again to exact head b468accb8a1db0f268b0c44f65e8f4bbb16e95e0. RED 92f7eb41ab37247a65c1da5950ae6965e78abe2a proved AuthenticatedPrincipal.tenant_record_id still exposed the UUID object stored inside the tuple, allowing low-level mutation of the returned value to retarget tenant evidence. Fixes 64deb92f… / b468accb… store the validated UUID integer and reconstruct a fresh UUID on access. #163 must still wait for ordinary #65 integration and adopt only protected truth non-force; the prior 5e1eb4a… static review does not transfer.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant