Repository navigation
fix(customer-master): preserve entities when visible hierarchy contains cycles (#906) #996
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
seonghobae
wants to merge
26
commits into
main
Choose a base branch
from
autoresearch/906-customer-cycle-20260910
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 5 commits
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
829df21
experiment: RED cover #906 cycle/self-parent entity loss in customer …
seonghobae bd9131c
experiment: keep #906 cycle/self-parent entities via broken-edge root…
seonghobae 9f90aea
experiment: extract customer forest to its own module, drop fast-refr…
seonghobae 759fd3c
experiment: cover #906 cycle badge at row render level
seonghobae 89cf4f0
experiment: add #906 malformed-forest stories + inventory row
seonghobae f4d17b7
experiment: add #906 OrdinaryTree control story + inventory row
seonghobae 54399b6
perf(customer-master): linearize cycle-safe visible forest
seonghobae cb3c995
perf(customer-master): adopt OrdinaryTree and linearize visible forest
seonghobae 7ba6a9e
test(customer-master): pin deterministic tail-into-cycle convergence
seonghobae 203af4b
fix(i18n): translate #906 ancestry notes across ko/zh/ja/vi
seonghobae 9a9772e
fix(i18n): keep customer-master ancestry copy on versioned ledger bou…
seonghobae 8f199f2
test(customer-master): pin cycle break across input permutations
seonghobae 55e9db7
fix(customer-master): stabilize cycle root across source ordering
seonghobae 5caf5d3
test(customer-master): reject duplicate canonical entity ids
seonghobae e04e157
fix(customer-master): fail closed on duplicate entity identity
seonghobae 0c60602
revert(customer-master): avoid unbounded duplicate-id render failure
seonghobae db9ad95
revert(test): keep duplicate-id case pending bounded UI failure
seonghobae 842425b
test(storybook): cover Customer Master loading and empty states
seonghobae bbc5a80
docs(storybook): inventory Customer Master loading and empty states
seonghobae c56b3d6
test(storybook): exercise malformed Customer Master responsive presets
seonghobae 04fa970
test(storybook): require Storybook 10 Customer Master viewport globals
seonghobae 75709b3
fix(storybook): use Storybook 10 viewport globals
seonghobae 65d3020
docs(storybook): record Customer Master responsive evidence
seonghobae a640df4
fix(customer-master): separate cycle evidence rows
seonghobae 0fbd3fe
test(customer-master): require production tree chrome in stories
seonghobae 74c4293
fix(customer-master): render stories with production tree chrome
seonghobae File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| import type { Meta, StoryObj } from "@storybook/react-vite"; | ||
| import { expect, within } from "storybook/test"; | ||
| import { CustomerEntityTreeRow } from "./App"; | ||
| import { buildCustomerEntityTree } from "./customerEntityTree"; | ||
| import type { CustomerMasterEntity } from "./api"; | ||
|
|
||
| function entity( | ||
| id: string, | ||
| parent: string | null, | ||
| name = id, | ||
| ): CustomerMasterEntity { | ||
| return { | ||
| corporate_entity_id: id, | ||
| entity_name: name, | ||
| corporate_entity_code: id, | ||
| entity_level_code: "L", | ||
| entity_level_label: "Level", | ||
| parent_entity_id: parent, | ||
| }; | ||
| } | ||
|
|
||
| const callbacks = { | ||
| onToggle: () => undefined, | ||
| onOpenPost: () => undefined, | ||
| }; | ||
|
|
||
| const meta = { | ||
| title: "Customers/EntityTreeRow", | ||
| component: CustomerEntityTreeRow, | ||
| } satisfies Meta<typeof CustomerEntityTreeRow>; | ||
| export default meta; | ||
| type Story = StoryObj<typeof meta>; | ||
|
|
||
| function ForestList({ nodes }: { nodes: ReturnType<typeof buildCustomerEntityTree> }) { | ||
| return ( | ||
| <ul> | ||
| {nodes.map((node) => ( | ||
| <CustomerEntityTreeRow | ||
| key={node.entity.corporate_entity_id} | ||
| node={node} | ||
| depth={0} | ||
| expandedEntityId={null} | ||
| relatedByEntity={{}} | ||
| relatedLoading={null} | ||
| {...callbacks} | ||
| /> | ||
| ))} | ||
| </ul> | ||
| ); | ||
| } | ||
|
|
||
| export const PureCycle: Story = { | ||
| render: () => ( | ||
| <ForestList | ||
| nodes={buildCustomerEntityTree([ | ||
| entity("A", "B", "Alpha"), | ||
| entity("B", "A", "Beta"), | ||
| ])} | ||
| /> | ||
| ), | ||
| play: async ({ canvasElement }) => { | ||
| const canvas = within(canvasElement); | ||
| await expect(canvas.getByText("Alpha")).toBeInTheDocument(); | ||
| await expect(canvas.getByText("Beta")).toBeInTheDocument(); | ||
| await expect( | ||
| canvas.getByText("Shown as top level: listed parent forms a cycle."), | ||
| ).toBeInTheDocument(); | ||
| }, | ||
| }; | ||
|
|
||
| export const SelfParent: Story = { | ||
| render: () => ( | ||
| <ForestList nodes={buildCustomerEntityTree([entity("S", "S", "Solo")])} /> | ||
| ), | ||
| play: async ({ canvasElement }) => { | ||
| const canvas = within(canvasElement); | ||
| await expect(canvas.getByText("Solo")).toBeInTheDocument(); | ||
| await expect( | ||
| canvas.getByText("Shown as top level: entity lists itself as parent."), | ||
| ).toBeInTheDocument(); | ||
| }, | ||
| }; | ||
|
|
||
| export const UnlistedParent: Story = { | ||
| render: () => ( | ||
| <ForestList | ||
| nodes={buildCustomerEntityTree([entity("child", "ghost", "Orphan")])} | ||
| /> | ||
| ), | ||
| play: async ({ canvasElement }) => { | ||
| const canvas = within(canvasElement); | ||
| await expect(canvas.getByText("Orphan")).toBeDefined(); | ||
| await expect( | ||
| canvas.getByText("Shown as top level: listed parent is not visible."), | ||
| ).toBeInTheDocument(); | ||
| }, | ||
| }; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,109 @@ | ||
| import { render, screen } from "@testing-library/react"; | ||
| import { beforeEach, describe, expect, it, vi } from "vitest"; | ||
| import { CustomerEntityTreeRow } from "./App"; | ||
| import { buildCustomerEntityTree } from "./customerEntityTree"; | ||
| import type { CustomerMasterEntity } from "./api"; | ||
| import { setLocale } from "./i18n"; | ||
|
|
||
| vi.mock("react-oidc-context", () => ({ | ||
| useAuth: () => ({ | ||
| isLoading: false, | ||
| isAuthenticated: false, | ||
| error: undefined, | ||
| user: undefined, | ||
| signinRedirect: vi.fn(), | ||
| signoutRedirect: vi.fn(), | ||
| }), | ||
| })); | ||
|
|
||
| beforeEach(() => { | ||
| setLocale("en"); | ||
| }); | ||
|
|
||
| function entity( | ||
| id: string, | ||
| parent: string | null, | ||
| name = id, | ||
| ): CustomerMasterEntity { | ||
| return { | ||
| corporate_entity_id: id, | ||
| entity_name: name, | ||
| corporate_entity_code: id, | ||
| entity_level_code: "L", | ||
| entity_level_label: "Level", | ||
| parent_entity_id: parent, | ||
| }; | ||
| } | ||
|
|
||
| function flattenIds( | ||
| nodes: ReturnType<typeof buildCustomerEntityTree>, | ||
| ): string[] { | ||
| const out: string[] = []; | ||
| const walk = ( | ||
| list: ReturnType<typeof buildCustomerEntityTree>, | ||
| ): void => { | ||
| for (const node of list) { | ||
| out.push(node.entity.corporate_entity_id); | ||
| walk(node.children); | ||
| } | ||
| }; | ||
| walk(nodes); | ||
| return out; | ||
| } | ||
|
|
||
| describe("#906 cycle-safe customer forest", () => { | ||
| it("emits every entity exactly once for a pure two-entity cycle", () => { | ||
| const tree = buildCustomerEntityTree([ | ||
| entity("A", "B"), | ||
| entity("B", "A"), | ||
| ]); | ||
| expect(flattenIds(tree).sort()).toEqual(["A", "B"]); | ||
| expect(tree).toHaveLength(1); | ||
| expect(tree[0].ancestryNote).toBe("cycle-broken"); | ||
| }); | ||
|
|
||
| it("emits a self-parent entity once instead of dropping it", () => { | ||
| const tree = buildCustomerEntityTree([entity("S", "S")]); | ||
| expect(flattenIds(tree)).toEqual(["S"]); | ||
| expect(tree[0].ancestryNote).toBe("self-parent"); | ||
| }); | ||
|
|
||
| it("preserves missing-parent-as-root and ordinary order", () => { | ||
| const tree = buildCustomerEntityTree([ | ||
| entity("child", "ghost"), | ||
| entity("root", null), | ||
| entity("kid", "root"), | ||
| ]); | ||
| expect(flattenIds(tree)).toEqual(["child", "root", "kid"]); | ||
| expect(tree[0].ancestryNote).toBe("unlisted-parent"); | ||
| expect(tree[1].ancestryNote).toBeUndefined(); | ||
| }); | ||
|
|
||
| it("renders both cycle members with an explicit ancestry note", () => { | ||
| const tree = buildCustomerEntityTree([ | ||
| entity("A", "B", "Alpha"), | ||
| entity("B", "A", "Beta"), | ||
| ]); | ||
| render( | ||
| <ul> | ||
| {tree.map((node) => ( | ||
| <CustomerEntityTreeRow | ||
| key={node.entity.corporate_entity_id} | ||
| node={node} | ||
| depth={0} | ||
| expandedEntityId={null} | ||
| relatedByEntity={{}} | ||
| relatedLoading={null} | ||
| onToggle={() => undefined} | ||
| onOpenPost={() => undefined} | ||
| /> | ||
| ))} | ||
| </ul>, | ||
| ); | ||
| expect(screen.getByText("Alpha")).toBeDefined(); | ||
| expect(screen.getByText("Beta")).toBeDefined(); | ||
| expect( | ||
| screen.getByText("Shown as top level: listed parent forms a cycle."), | ||
| ).toBeDefined(); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| import type { CustomerMasterEntity } from "./api"; | ||
|
|
||
| export interface CustomerEntityTreeNode { | ||
| entity: CustomerMasterEntity; | ||
| children: CustomerEntityTreeNode[]; | ||
| // Presentation-only ancestry flag. The entity DTO is never mutated: | ||
| // a broken presentation edge still renders the entity exactly once. | ||
| ancestryNote?: "cycle-broken" | "self-parent" | "unlisted-parent"; | ||
| } | ||
|
|
||
| // Customer Master's visible entities form a forest, but ABAC can authorize | ||
| // a child without its parent and stored parents can cycle (or self-point), | ||
| // so the drawable forest must be derived defensively: every input entity is | ||
| // emitted exactly once, ordinary order is preserved, and only the | ||
| // presentation edge needed to establish a root is broken (#906). | ||
| export function buildCustomerEntityTree(entities: CustomerMasterEntity[]): CustomerEntityTreeNode[] { | ||
| const byId = new Map(entities.map((entity) => [entity.corporate_entity_id, entity])); | ||
| // Tentative presentation parents: visible and non-self only. | ||
| const parentOf = new Map<string, string | null>(); | ||
| const noteOf = new Map<string, NonNullable<CustomerEntityTreeNode["ancestryNote"]>>(); | ||
| for (const entity of entities) { | ||
| const id = entity.corporate_entity_id; | ||
| const parent = entity.parent_entity_id; | ||
| if (!parent || !byId.has(parent)) { | ||
| parentOf.set(id, null); | ||
| if (parent) { | ||
| noteOf.set(id, "unlisted-parent"); | ||
| } | ||
| } else if (parent === id) { | ||
| parentOf.set(id, null); | ||
| noteOf.set(id, "self-parent"); | ||
| } else { | ||
| parentOf.set(id, parent); | ||
| } | ||
| } | ||
| // Break presentation cycles deterministically: walking from each entity | ||
| // in input order, the edge closing a revisited ancestor is cut so its | ||
| // predecessor becomes a root. Earlier members keep their edges, so the | ||
| // result is stable for a given input order and always acyclic. | ||
| for (const entity of entities) { | ||
| const path: string[] = []; | ||
| let cursor: string | null = entity.corporate_entity_id; | ||
| while (cursor !== null && !path.includes(cursor)) { | ||
| path.push(cursor); | ||
| cursor = parentOf.get(cursor) ?? null; | ||
| } | ||
| if (cursor !== null) { | ||
| const predecessor = path[path.length - 1]; | ||
| parentOf.set(predecessor, null); | ||
| noteOf.set(predecessor, "cycle-broken"); | ||
| } | ||
| } | ||
| const childrenByParent = new Map<string, CustomerMasterEntity[]>(); | ||
| const roots: CustomerMasterEntity[] = []; | ||
| for (const entity of entities) { | ||
| const parent = parentOf.get(entity.corporate_entity_id); | ||
| if (parent) { | ||
| const siblings = childrenByParent.get(parent) ?? []; | ||
| siblings.push(entity); | ||
| childrenByParent.set(parent, siblings); | ||
| } else { | ||
| roots.push(entity); | ||
| } | ||
| } | ||
| const toNode = (entity: CustomerMasterEntity): CustomerEntityTreeNode => ({ | ||
| entity, | ||
| children: (childrenByParent.get(entity.corporate_entity_id) ?? []).map(toNode), | ||
| ...(noteOf.has(entity.corporate_entity_id) | ||
| ? { ancestryNote: noteOf.get(entity.corporate_entity_id) } | ||
| : {}), | ||
| }); | ||
| return roots.map(toNode); | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: ContextualWisdomLab/LineageWeave
Length of output: 1532
🏁 Script executed:
Repository: ContextualWisdomLab/LineageWeave
Length of output: 22813
🏁 Script executed:
Repository: ContextualWisdomLab/LineageWeave
Length of output: 537
🏁 Script executed:
Repository: ContextualWisdomLab/LineageWeave
Length of output: 34430
🏁 Script executed:
Repository: ContextualWisdomLab/LineageWeave
Length of output: 5057
새 ancestry 문구를 번역 원장에 등록하세요.
지원 로케일은
en,ko,zh,ja,vi입니다. 세 문구는TRANSLATIONS에 없으므로t()가 모든 로케일에서 영어 키를 반환합니다. 병합 전에 다섯 로케일의 번역을 등록하고 비영어 로케일의 번역 테스트를 추가하세요.🤖 Prompt for AI Agents
✅ Addressed in commits 7ba6a9e to 203af4b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finding is valid at the buyer-visible localization boundary, but the proposed local fix is no longer valid under the current architecture contract.
frontend/src/i18n.tsis not allowed to become the new authority for these Customer Master messages: product copy must converge through the versioned PostgreSQL translation-ledger/screen-key path owned by #929/#932 across ko/en/ja/zh/vi/es/de/fr. Commit 203af4b added only ko/zh/ja/vi inline entries and therefore both duplicated the owner and remained incomplete. I preserved that intervening commit in history, then reverted only its two file deltas non-force at 9a9772e. This PR is Draft again and this thread stays unresolved until the canonical translation-ledger consumer supplies the three ancestry messages with current browser/a11y evidence.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@seonghobae, thanks for the architecture clarification.The localization finding remains valid. The proposed
frontend/src/i18n.tschange does not match the current ownership contract.The canonical versioned PostgreSQL translation-ledger/screen-key path from
#929and#932must provide these three Customer Master messages forko,en,ja,zh,vi,es,de, andfr. The required evidence remains current browser and accessibility validation. I will keep this thread open.✏️ Learnings added
You are interacting with an AI system.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Current canonical evidence has advanced without copying translation authority into this hierarchy PR. Parent #929 exact
64fa200e46854878b3154880af678afa69284fdbnow seeds a replay-safe version-1lineageweave/customer-masterdraft with the exact governed 37-key set acrossko/en/ja/zh/vi/es/de/fr; the seed includes all three ancestry-note strings from this finding. Consumer #932 is ordinarily/non-force converged to that parent at8133220817a875da72f4bbc5dd011f30f060a126. I have refreshed this PR body to those live authorities. The thread remains unresolved because the candidate copy is still draft: parent current-head Tests are Draft-skipped rather than PostgreSQL GREEN, qualifying independent language/product review is absent, and the one-way publication transition/browser-a11y evidence has not occurred. No inline/static fallback is being promoted as authority.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@seonghobae, acknowledged. The canonical draft now includes the three ancestry-note messages through parent#929and consumer#932.The translation authority remains outside this hierarchy PR. No
frontend/src/i18n.tsfallback is required.This thread remains open until the canonical path has PostgreSQL GREEN validation, qualifying independent language and product review, one-way publication evidence, and current browser and accessibility evidence.
🐇
🧠 Learnings used
You are interacting with an AI system.