Skip to content

feat(core): add governed Position reporting-change review - #95

Open
seonghobae wants to merge 4 commits into
developfrom
feat/position-reporting-change-review
Open

feat(core): add governed Position reporting-change review#95
seonghobae wants to merge 4 commits into
developfrom
feat/position-reporting-change-review

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Scope

Adds one bounded, transport-neutral pre-mutation governance boundary for reviewing a solid-line Position-to-Position reporting reassignment. It binds reviewed organizational scope and accountable human review without copying Person PII, worker values, compensation, ratings, free-form personal reasons, or employment decisions into the evidence envelope.

This slice deliberately does not persist or mutate reporting-line truth. It is independent of active PR #94 and requires authoritative Position/reporting resolution before any later mutation. No cross-service application-table SQL or foreign CWL repository write is included.

Test-first repair sequence

  • RED contract: 69c4baaacd3dc2851fa9f3ae6d7a0e068bf1d466 added the package/workflow/regressions while imported production module review was intentionally absent. No terminal hosted RED run materialized before subsequent repair commits superseded that head, so absent/cancelled evidence is not presented as terminal RED proof.
  • Root implementation: 8d28a7eadc238658c9f37becee5c11f468167148 implemented PositionReportingChangeReviewPacket, buyer docs, ADR 0095, traceability and current primary-source doctoring.
  • Review repairs: af6a915132254ec1088329bb4009b024daedfdcb removed the unused adversarial-test import; current head adf055d79d188ba18d06ecf80dc1117858c987f4 completed the adversarial integer ordering surface after the valid GitHub Code Quality finding. Only addressed threads are resolved.

Governed contract

The packet keeps subordinate/current-manager/proposed-manager Position references separate; rejects self-reporting/no-op proposals; preserves authoritative tenant/Position operational UUID interoperability while leaf-owned change/actor correlations require UUIDv4; separates business effective_on from system-recorded evidence recorded_at; binds Position/organization scope using lowercase SHA-256 digests and a controlled reason vocabulary; and requires requester/reviewer separation plus authoritative actor re-resolution before mutation.

It fixes review_state=requires_human_review, scope_verification_state=requires_authoritative_resolution, mutation_state=not_authorized_to_apply, and decision_authority=human_review_only. Exact built-in trust primitives, a redacted repr, deterministic canonical JSON/SHA-256 and process-local post-construction tamper detection are enforced. The next action requires same-tenant bitemporal Position/reporting resolution, Position validity/staffability, authoritative actor separation, cycle/multiple-manager rejection, scope-evidence verification and immutable audit/outbox before any later mutation.

NIST SP 800-53 Rev. 5 Release 5.2.0, final NIST Privacy Framework 1.0, and RFC 9562 are recorded under docs/doctoring; no certification or universal legal-compliance claim is made.

Exact-current-head evidence

Head: adf055d79d188ba18d06ecf80dc1117858c987f4
Base: develop@9e3e4847510e1e612b48474ba42b177b8ed824df

Terminal GREEN on the unchanged exact head:

  • Position Reporting Change Review Quality 32622161836
  • Foundation CI 32622161826
  • Recovery Rehearsal Quality 32622161831
  • SAST Semgrep 32622161829
  • Security Scan 32622161830
  • CodeRabbit status success

Focused job 97151830119 checked out exactly adf055d79d188ba18d06ecf80dc1117858c987f4, used CPython 3.14.7, built one wheel, installed it through a computed SHA-256 requirement, ran the installed artifact, passed 45 tests, and achieved 160/160 statements plus 52/52 branches = 100% owned statement/branch coverage with a clean checkout.

Fresh review state has no unresolved defect thread and no CHANGES_REQUESTED. Qualifying independent non-author APPROVE is still absent.

Merge policy

This PR is ready for independent review but is intentionally unmerged. Do not reuse predecessor evidence. Merge only if the unchanged head still satisfies all live gates, a qualifying independent non-author approval exists, and develop has real enforceable protection. Issue #89 currently tracks the repository-level protection defect.

Summary by CodeRabbit

  • 새 기능

    • 포지션 간 보고 라인 변경을 사전에 검토할 수 있는 거버넌스 검토 패킷을 추가했습니다.
    • 변경 대상, 적용일, 검토자, 사유 및 증거 정보를 표준화하고 개인정보와 인사 결정을 제외합니다.
    • SHA-256 기반 무결성 검증과 엄격한 입력·권한·관계 검증을 제공합니다.
  • 문서

    • 사용 방법, 적용 범위, 감사·권한 분리 원칙 및 추적성 문서를 추가했습니다.
  • 품질 개선

    • 자동 빌드, 테스트, 커버리지 및 패키지 무결성 검증을 위한 품질 게이트를 추가했습니다.

@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 93a7ee99-f034-47c9-a5f2-59f0722c5ae8

📥 Commits

Reviewing files that changed from the base of the PR and between 9e3e484 and adf055d.

📒 Files selected for processing (11)
  • .github/workflows/position-reporting-change-review-quality.yml
  • docs/adr/0095-governed-position-reporting-change-review.md
  • docs/doctoring/position-reporting-change-review-references.md
  • docs/traceability/position-reporting-change-review.md
  • packages/position-reporting-change-review/CHANGELOG.md
  • packages/position-reporting-change-review/README.md
  • packages/position-reporting-change-review/pyproject.toml
  • packages/position-reporting-change-review/src/orgmetra_position_reporting_change_review/__init__.py
  • packages/position-reporting-change-review/src/orgmetra_position_reporting_change_review/review.py
  • packages/position-reporting-change-review/tests/test_repository_contract.py
  • packages/position-reporting-change-review/tests/test_review.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

PositionReportingChangeReviewPacket 패키지와 관련 거버넌스 문서를 추가했습니다. 패킷은 입력 검증, canonical JSON, SHA-256 무결성 검증을 제공합니다. 테스트와 GitHub Actions 워크플로는 wheel 설치, 해시 검증, 100% 브랜치 커버리지와 깨끗한 checkout을 확인합니다.

Changes

포지션 보고 변경 검토

Layer / File(s) Summary
거버넌스 계약과 패키지 공개 범위
docs/adr/..., docs/doctoring/..., docs/traceability/..., packages/position-reporting-change-review/README.md, CHANGELOG.md, pyproject.toml, src/.../__init__.py
검토 패킷의 범위, 제외 정보, 식별자 규칙, 권한 경계와 추적성을 문서화했습니다. 패키지 빌드 설정과 공개 API를 추가했습니다.
패킷 데이터 구조와 입력 검증
packages/position-reporting-change-review/src/.../review.py
패킷 상태와 참조를 정의했습니다. UUID, digest, 코드, 시간, 행위자 분리, 관계 및 개인정보 최소화 규칙을 검증합니다.
정규화와 증거 무결성
packages/position-reporting-change-review/src/.../review.py
canonical JSON을 생성합니다. 생성 후 필드 변경을 탐지하고 SHA-256 evidence digest를 반환합니다.
테스트와 재현 가능한 품질 게이트
packages/position-reporting-change-review/tests/*, .github/workflows/position-reporting-change-review-quality.yml
정상·오류 입력, 타입 위조, UTC 정규화, 변조 탐지와 문서 경로를 테스트합니다. 고정 Python, 해시 검증 wheel, 의존성 검사, 테스트, 100% 브랜치 커버리지와 깨끗한 checkout을 자동 실행합니다.

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

Merge Risk: 🟡 Moderate · up to adf05

The change adds a bounded pre-mutation review boundary without changing reporting-line data, and no user-facing or production defect is identified. The current head should not merge yet because the required independent non-author approval is still missing.

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant build_position_reporting_change_review_packet
  participant PositionReportingChangeReviewPacket
  participant QualityWorkflow
  Caller->>build_position_reporting_change_review_packet: reporting-change 입력 전달
  build_position_reporting_change_review_packet->>PositionReportingChangeReviewPacket: 검증된 패킷 생성
  PositionReportingChangeReviewPacket-->>Caller: canonical evidence와 SHA-256 digest 제공
  QualityWorkflow->>PositionReportingChangeReviewPacket: 설치된 wheel 테스트 실행
  QualityWorkflow-->>Caller: 커버리지와 checkout 검증 결과 반환
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring check was indeterminate for this PR — some files could not be analyzed in time. Not blocking.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 거버넌스가 적용된 Position 보고 변경 검토 기능 추가라는 주요 변경을 정확하고 간결하게 설명합니다.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/position-reporting-change-review

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.

github-code-quality[bot]

This comment was marked as resolved.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

Open in Devin Review

Comment on lines +272 to +281
def canonical_json(self) -> str:
"""Return one verified snapshot of deterministic canonical audit evidence."""
payload = _payload(self)
payload_json = _canonical_payload_json(payload)
current_digest = sha256(payload_json.encode("utf-8")).hexdigest()
with _REGISTRY_LOCK:
creation_digest = _CREATION_DIGESTS.get(self)
if current_digest != creation_digest:
raise ValueError("position reporting-change evidence changed after issuance")
return payload_json

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Tamper detection is process-local

canonical_json (review.py) matches the payload against a digest stored in a module-global WeakKeyDictionary keyed by object identity. A packet reconstructed without __post_init__ (pickle, copy.deepcopy of a frozen-slots dataclass) has no entry, so the lookup returns None and a valid packet raises "changed after issuance". No pickle/copy path exists here and the PR states this is deliberately process-local. Relevant only for future callers that serialize via pickle rather than canonical_json.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@seonghobae
seonghobae marked this pull request as draft August 24, 2026 15:39
@seonghobae
seonghobae marked this pull request as ready for review August 24, 2026 15:39
@seonghobae seonghobae closed this Aug 24, 2026
@seonghobae seonghobae reopened this Aug 24, 2026
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