Skip to content

fix(network): first-wins Map lookups for colliding graph ids - #1382

Merged
seonghobae merged 6 commits into
developfrom
cursor/bc-1ce3afac-6386-46aa-a7e4-70eeb673b2f1-fe7a
Aug 17, 2026
Merged

fix(network): first-wins Map lookups for colliding graph ids#1382
seonghobae merged 6 commits into
developfrom
cursor/bc-1ce3afac-6386-46aa-a7e4-70eeb673b2f1-fe7a

Conversation

@cursor

@cursor cursor Bot commented Aug 16, 2026

Copy link
Copy Markdown

Description

#1377 already replaced linear NetworkGraph selection scans with memoized Maps. That head still built edgeMap / nodeInstanceMap with last-wins new Map(items.map(...)), while nodeMap and the previous .find() path were first-wins. A repeated relationship id therefore opened the later edge while labels still described the first node pair.

This successor keeps constant-time lookups and makes colliding ids select the first instance. Rendered vis-network selectNode / selectEdge events are now exercised with mixed numeric and string ids instead of relying only on a source-substring scan.

Fixes the remaining review gap on #1377. Supersedes #1377 for merge. #1358 remains a narrower node-label-only slice and should not land in parallel.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

TDD boundary

The RED run of keeps the first edge instance when duplicate relationship ids collide failed on e75ce01a with last-wins detail 선택된 관계: 김지현 -> 일정 (일정 후보 1건). The production fix is e74695e5.

Exact verification commands

Run from frontend/ on the unchanged candidate head:

pnpm test -- src/components/NetworkGraph.test.tsx src/components/NetworkGraph.map-lookup.test.ts
pnpm exec eslint src/components/NetworkGraph.tsx src/components/NetworkGraph.test.tsx src/components/NetworkGraph.map-lookup.test.ts
pnpm typecheck

Local evidence on e74695e5: 432 passed, ESLint clean, tsc --noEmit clean. Hosted exact-head Application CI, security, and required central workflows remain authoritative. Queued, skipped, stale, predecessor-head, or model-only results are non-passing.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Merge boundary

Merge only if the unchanged final head satisfies every live repository + inherited organization rule, every required exact-head check is terminal-success, all addressed review threads are resolved, and a qualifying independent non-author approval exists after the last push. No self-approval or bypass. Do not merge #1377 or #1358 separately.

Open in Web View Automation 

seonghobae and others added 5 commits August 16, 2026 13:56
Replace last-wins Map construction with firstGraphEntryById so duplicate
node/edge ids keep the same instance that nodeMap already uses for labels.
Add rendered vis-network selectNode/selectEdge coverage for mixed numeric
and string ids, and lock the first edge when a relationship id repeats.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
@github-actions

github-actions Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

PR governance metadata gate is not ready for f8317e5c9c49315e3daef5453ca4aa8c78529ad3:

  • Required check metadata could not be read; see the workflow run log.

@seonghobae
seonghobae marked this pull request as ready for review August 16, 2026 19:07
@cursor

cursor Bot commented Aug 16, 2026

Copy link
Copy Markdown
Author

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

Resolve the NetworkGraph.tsx conflict by keeping first-wins nodeMap
selection labels. Develop only added comments and a per-instance label
read that would break the colliding-id contract tests.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
@cursor

cursor Bot commented Aug 17, 2026

Copy link
Copy Markdown
Author

Merged current develop to clear the DIRTY conflict in frontend/src/components/NetworkGraph.tsx.

Exact head f8317e5c keeps first-wins nodeMap.get(...) selection labels. Develop’s extra comments and the per-instance node.label read were dropped so colliding ids still describe the first node. Focused vitest: src/components/NetworkGraph.map-lookup.test.ts and src/components/NetworkGraph.test.tsx — 13 passed.

@seonghobae seonghobae left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

APPROVE on f8317e5. Last commit author/committer is cursoragent, not this reviewer. frontend, backend Python 3.14, coverage-evidence, images, Semgrep, CodeQL, security are green. metadata-only gate failed because required-check metadata could not be read (GitHub partial outage), not a product defect.

@seonghobae
seonghobae merged commit a81ac4f into develop Aug 17, 2026
40 of 44 checks passed
@seonghobae
seonghobae deleted the cursor/bc-1ce3afac-6386-46aa-a7e4-70eeb673b2f1-fe7a branch August 17, 2026 18:29
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.

2 participants