feat(core): add bitemporal position reporting hierarchy - #94
Conversation
|
Warning Review limit reached
Next review available in: 57 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| def _freeze_known_at(value: datetime) -> datetime: | ||
| """Detach one caller-provided timezone offset into an exact built-in UTC instant.""" | ||
| if type(value) is not datetime: | ||
| raise PositionReportingHierarchyError( | ||
| "known_at must be an exact built-in datetime.", | ||
| next_action="Use the authoritative UTC system-knowledge timestamp, then rebuild the chart.", | ||
| ) | ||
| if value.tzinfo is None: | ||
| raise PositionReportingHierarchyError( | ||
| "known_at must be a timezone-aware datetime.", | ||
| next_action="Attach the authoritative timezone or convert the knowledge cutoff to UTC.", | ||
| ) | ||
| try: | ||
| offset = value.utcoffset() | ||
| except Exception as exc: | ||
| raise PositionReportingHierarchyError( | ||
| "known_at timezone could not be resolved safely.", | ||
| next_action="Convert the knowledge cutoff to a fixed UTC timestamp before rebuilding the chart.", | ||
| ) from exc | ||
| if type(offset) is not timedelta: | ||
| raise PositionReportingHierarchyError( | ||
| "known_at must have one concrete UTC offset.", | ||
| next_action="Convert the knowledge cutoff to a fixed UTC timestamp before rebuilding the chart.", | ||
| ) | ||
| wall_time = datetime( | ||
| value.year, | ||
| value.month, | ||
| value.day, | ||
| value.hour, | ||
| value.minute, | ||
| value.second, | ||
| value.microsecond, | ||
| tzinfo=timezone.utc, | ||
| fold=value.fold, | ||
| ) | ||
| return wall_time - offset |
There was a problem hiding this comment.
📝 Info: UTC normalization correct across offsets
_freeze_known_at at position_reporting.py:59-70 tags the caller's wall components as UTC then subtracts utcoffset(), giving the correct absolute instant (UTC = local - offset). value.fold is honored so DST-ambiguous inputs resolve consistently, and a UTC input round-trips unchanged. Not a bug.
Was this helpful? React with 👍 or 👎 to provide feedback.
| for start in manager_by_subordinate: | ||
| seen: set[UUID] = set() | ||
| current: UUID | None = start | ||
| while current is not None: | ||
| if current in seen: | ||
| raise PositionReportingHierarchyError( | ||
| "Visible solid-line position reporting relationships form a cycle in this tenant.", | ||
| next_action="Close or correct one reporting edge in the cycle, then rebuild the chart.", | ||
| ) | ||
| seen.add(current) | ||
| current = manager_by_subordinate.get(current) |
There was a problem hiding this comment.
📝 Info: Cycle detection sound but redundant
The one-manager-per-subordinate guard makes manager_by_subordinate a functional graph, so the per-start forward walk terminates and flags any reachable cycle with no false positives. Self-loops are already rejected at construction. Walks over shared chains repeat work (O(n^2) worst case) but the result is correct.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if subordinate in manager_by_subordinate: | ||
| raise PositionReportingHierarchyError( | ||
| "A position resolves to more than one solid-line manager at this coordinate.", | ||
| next_action="Close or correct the superseded reporting relationship, then rebuild the chart.", | ||
| ) |
There was a problem hiding this comment.
📝 Info: Identical duplicate edge rejected, not deduplicated
Two visible edges sharing a subordinate raise the "more than one solid-line manager" error even when they name the same manager, because the guard keys only on the subordinate. This is fail-closed behavior; a redundant identical edge is rejected rather than collapsed.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
@opencode-agent Please review the current unchanged head against protected |
1 participant
Buyer-visible gap
Protected
develop@9e3e4847510e1e612b48474ba42b177b8ed824dfseparates Job, Position and Assignment and models organization-unit hierarchy, but it has no position-to-position solid-line reporting relationship. A commercial HRIS therefore cannot reconstruct which Position a seat reports to at one business date and system-knowledge cutoff without incorrectly deriving supervision from Person/Assignment or organization-unit parentage.This Orgmetra-only slice defines a tenant-scoped, bitemporal position-reporting contract. Managerial hierarchy is Position-to-Position, not Person-to-Person; Assignment remains an independent multiple-membership fact.
RED → root-cause implementation
36f8f7d0605688c95ddebdc6d6f513eb81d4e144defines deterministic subordinate→manager evidence, tenant isolation, effective/system-time visibility, one visible solid-line manager per subordinate, staffable endpoint seats, cycle/self-report rejection, trust-bearing runtime-type hardening, and redacted routine representation before productionposition_reportingexists.32616830004, job97138821309, checked out that exact RED SHA and failed at the first owning boundary withModuleNotFoundError: No module named 'orgmetra_hris_kernel.position_reporting'.f75ef9a7d785229d2e8a11fe3a5257ce40a5e0c8addsPositionReportingRelationship,PositionReportingSnapshot, and bitemporal reconstruction over authoritative PositionVersion evidence only.Review-driven public API repair
Fresh Devin review found a valid buyer-facing integration defect on the previous GREEN head: the new position-reporting contract existed only in
orgmetra_hris_kernel.position_reporting; the package root and__all__did not exposePositionReportingRelationship,PositionReportingSnapshot,PositionReportingHierarchyError, orbuild_position_reporting_snapshot, unlike peer HRIS-kernel contracts. Submodule-only tests hid that gap while README advertised the capability.fb78cbb9f85fd1c7765b98afbcd609829dba543daddstest_position_reporting_public_api.pyand requires the four governed symbols to be reachable fromorgmetra_hris_kerneland listed in__all__. This regression head was immediately followed by the repair before a terminal hosted RED run materialized, so no cancelled/absent hosted execution is claimed as RED evidence.3f67182bb3065f2fc8fd974bfdd75a390d8a8fdcexports the existing governed types/function from the package root without duplicating implementation or moving unrelated error ownership.Governed behavior
A visible solid-line reporting edge must reference exactly one same-tenant
activeoropenPositionVersion at the requested business/system coordinate for both subordinate and manager. One subordinate can have only one visible solid-line manager; self-reporting and cycles fail closed. Caller-defined relationship/PositionVersion/date/datetime subclasses cannot control trust-bearing identity or temporal comparisons, and caller-owned timezone behavior is resolved once and detached into a built-in UTC instant. Routine representations redact position-correlation UUIDs.The snapshot is descriptive organizational evidence only. It does not identify the worker occupying either seat, infer a manager from Assignment, reinterpret organization-unit parentage as supervisory authority, or grant employment-decision authority.
Exact-current-head evidence
Current exact head:
3f67182bb3065f2fc8fd974bfdd75a390d8a8fdc.Fresh live base:
develop@9e3e4847510e1e612b48474ba42b177b8ed824df.GitHub reports the PR open, ready-for-review and mergeable.
Every applicable exact-current-head hosted workflow is terminal GREEN:
32619302985— success. Job97144896045checked out exactly3f67182bb3065f2fc8fd974bfdd75a390d8a8fdc, ran 186 HRIS-kernel tests, and measured 799 statements / 328 branches = exactly 100% statement and branch coverage;position_reporting.pyis 94 statements / 40 branches = 100%, the package-root module is 100%, the new public-API regression passed, and clean checkout passed.32619302996— success.32619302991— success.32619302968— success.32619302984— success.32619302979— success.32619302994— success.Fresh submitted review state has no qualifying independent non-author
APPROVEand noCHANGES_REQUESTED. The public-API BUG thread is addressed/resolved; remaining current review threads are informational.Scope and merge governance
Write scope is Orgmetra only. No dedicated-writer dependency repository, source, ref, workflow, settings, PR state, credential, or application table is modified, and no cross-service SQL is introduced. Persistence, authorized reporting-line mutation with immutable audit/outbox, and accessible organization-chart UI remain separate future owner slices.
This unchanged exact head has terminal applicable GREEN evidence, fresh mergeability and no verified unresolved defect, so the lane is ready for qualifying independent review. Ready-for-review is not approval and does not authorize merge. Orgmetra issue #89 remains open because GitHub reports
developasprotected: truewhile effective required-check enforcement is disabled/empty. Keep unmerged until qualifying independent non-author approval and enforceable protected-branch policy are both present on fresh state. Immediately before any merge, refetch exact head, live base, rules/protection, reviews, threads and checks and use expected-head protection only if every live gate is satisfied. Do not self-approve, bypass protection, weaken a gate, or reuse predecessor evidence.