Skip to content

feat(domain): add team-department conformance evidence - #12

Open
phibkro wants to merge 1 commit into
publish/0003-0005-accepted-basefrom
publish/0004-team-domain
Open

feat(domain): add team-department conformance evidence#12
phibkro wants to merge 1 commit into
publish/0003-0005-accepted-basefrom
publish/0004-team-domain

Conversation

@phibkro

@phibkro phibkro commented Aug 12, 2026

Copy link
Copy Markdown
Member

Design spec

design-specs/0004-team-department-conformance-evidence.md

Journey

A maintainer runs the bounded team/department domain proof and receives deterministic evidence for department tenancy, team identity, and accepted relation classifications.

Experience it

  1. Check out this stacked branch.
  2. Run the package commands recorded in the spec Evidence section.
  3. Inspect the 15-fixture conformance report.

Evidence

  • Independent code and runtime review: PASS.
  • Scope is exactly the 0004 spec, packages/domain/**, and the derived root lock delta.
  • No SDK, Alchemy, workflow, or root-manifest payload leaks into this PR.

What is real

The bounded S-DEP-2-TEAM evidence is conforming. Person authority remains partial/informational; this PR does not claim full S-DEP-1, temporal completeness, persistence, provider behavior, or production acceptance.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 96eba1a09e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +251 to +252
const personCompleteness: Completeness =
options.personAuthority === undefined || personMissing > 0 ? "PARTIAL" : "FULL";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Include global-only users in authority completeness

When a supplied projection omits a user who appears only in globalMemberships, that user is never checked because authority coverage is evaluated only in the local-membership loop, so this marks the person axis FULL; a global-only dataset with authorityFromEntries([]) can therefore produce PASS. This also makes the explicit empty-set authority supplied for the global user in F-global-hovedstyret irrelevant. Require every observed global-only user to have an authority entry, even if its department set is empty, before claiming full person completeness.

Useful? React with 👍 / 👎.

}
const authorized = options.personAuthority.departmentIdsByUser.get(membership.userId);
if (authorized === undefined) {
personMissing += 1;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Count missing authority users only once

When one user has multiple accepted local memberships but no authority entry, this increments personMissing once per edge. The resulting personComparison.missingUsers and Markdown label “Missing authority users” therefore report the number of memberships rather than distinct users—for example, two memberships for one missing user yield missingUsers: 2. Track missing user IDs in a set and report its size.

Useful? React with 👍 / 👎.

@@ -0,0 +1,260 @@
import { readFile } from "node:fs/promises";
import { join } from "node:path";

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Code smell for idiomatic Effect. No direct runtime imports (node, bun, deno) environment capabilities should always go through the established Effect Platforms services


export type RequiredFile = (typeof REQUIRED_FILES)[number];

export type DatasetErrorCode =

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

why not Effect Literal?

file: RequiredFile,
decoder: (value: unknown) => DecodeResult<A>,
): { readonly rows: ReadonlyArray<A>; readonly failures: ReadonlyArray<DecodeFailure> } => {
try {

@phibkro phibkro Aug 12, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Code smell Try Catch block. yield* Effect should be used instead for tracking Error dependencies

};

const readErrorCode = (error: unknown): string | undefined => {
if (typeof error !== "object" || error === null || !(("code" in error))) return undefined;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Code smell. Data shape validation should use Effect Schema

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