fix(layout): group_in_a_box_layout returned duplicate node ids at conflicting coordinates (#1961)

[lmeyerov]

Fixes #1961

Root cause

partitioned_layout() defaults to bulk_mode=True, and no caller ever passes False. In bulk mode layout_bulk_mode() positions the whole graph in one pass and its frame is appended at :105. The singleton (id_count == 1), pair (id_count == 2) and edgeless (id_count > 2 and degree_max == 0) fallbacks were then appended again at :122 / :134 / :154 — on top of rows the bulk pass had already positioned, rather than replacing them. Each such node came back twice at two different x/y.

Those fallbacks are correct and necessary for bulk_mode=False, where layout_non_bulk_mode() only lays out remaining (id_count > 2 and degree_max > 0). They are now scoped to that mode — the if True and ... prefixes were the tell that the guard was never finished.

Why skip rather than drop the bulk rows. Both restore the invariant; the question is which positions survive. Measured on the issue's repro:

partition	bulk row (kept)	fallback row (kept)
singleton	box corner (a 1-row partition normalizes to 0 either way)	box corner — identical
pair	10.95 / 15.92, spread across the whole box	10.95 / 12.76, clustered at 0 and 0.33 of box width
edgeless (size > 2)	fa2 + per-partition circle layout	uniform(0, 1) noise

Bulk positions are identical for singletons and strictly better for pairs and edgeless groups, because layout_bulk_mode already hands circle_layout_params={'partition_by': partition_key} to fa2_layout — that is the degenerate-partition handling the constants were a stand-in for. So no positioning quality is traded away; bulk_mode=False keeps the constants unchanged.

Adjacent defects fixed in the same function

• edgeless['y'] was never assigned (:146 re-assigned x). Reachable via bulk_mode=False.
• The NaN backstop asserted the negation of its own guard (assert combined_nodes.y.isna().sum() == 0 inside if combined_nodes.y.isna().any():), so the fillna two lines below was dead and any unpositioned node raised a message-less AssertionError. The backstop is now one engine-agnostic loop over ['x', 'y'] built on df_cons(engine), which also removes the cuDF half's cp.random.rand(n, 1, ...) — verified locally that cudf.Series rejects it with ValueError: Data must be 1-dimensional, i.e. the cuDF backstop would have crashed had it ever been reachable.

Invariant pinned

The output node frame carries the same id multiset as the input — no duplicates, no drops — for every partition-size mix, on pandas and cuDF.

Verification

• Red at merge-base: the 7 new pins were run against merge-base 09633aa product code (also verified at 0aaa862 before rebase) in a throwaway worktree — 7/7 red (9 failures total; 2 are the pre-existing test_gib_cudf / test_gib_cudf_with_partitions, which need cugraph, absent here). Green with the fix.

• Mutation check, reverting each site individually (counts exclude the 2 pre-existing cugraph failures):

  reverted site	pins that go red
  singleton guard → if True	4
  pair guard → if True	3
  edgeless guard → if True	2
  edgeless['y'] → edgeless['x']	1
  re-add assert na_count == 0	2
  NA loop ['x', 'y'] → ['x']	2

• Failure-set comparison, full graphistry/tests (14774 tests) at head vs merge-base, both directions: identical 120-failure set, zero new, zero disappeared (the 120 are pre-existing optional-dependency failures). Head is +5 passed (new CPU pins) and +2 skipped (new cuDF pins, skipped without TEST_CUDF=1).

• GPU exercised for real, not indicative: cudf 25.10 + cupy 13.6 on an RTX 3080 Ti, with LD_LIBRARY_PATH pointed at the nvidia/*/lib wheels (the worktree default is missing libnvrtc.so.12 / libcurand.so.10). TEST_CUDF=1 runs both new cuDF pins green on the fix and red at merge-base. cugraph is not installed, so the cuGraph-backed default layout path is untested — the cuDF pins use a layout callable, which still drives the cuDF df_concat / isna / fillna / groupby / merge path end to end.

• bin/ci_comment_density_guard.py, ./bin/lint.sh, ./bin/typecheck.sh all rc=0, run from inside the worktree.

• Changed-line coverage: 10/10 = 100% (the engine-agnostic NA loop is what makes the cuDF-side lines reachable from CPU lanes; the previous engine-branched form scored 4/6 = 67% and would have failed the 80% gate).

Deliberately not changed

• layout(gib): group_in_a_box_layout duplicates every size-1/size-2 partition (silent row-count blowup) + message-less AssertionError on edgeless partitions #1961 Defect 3 (unseeded RNG / no seed= parameter) — the issue itself files it as a separate documentation/API question. Noted in the CHANGELOG as still-unseeded.
• The type column stays NaN for pairs in bulk_mode=False — pre-existing, unrelated to the duplication.
• The per-partition normalization puts a 1-row partition at its box corner (dx = max(x_max - x_min, 1) makes a degenerate range normalize to 0). Pre-existing and identical in both modes before and after this change; the x=0.5, y=0.5 singleton constant was already defeated by it.
• layout: tree_layout() bare-KeyErrors on ANY disconnected graph — Sugiyama lays out components[0] only #1962-layout: circle_layout entry guards — TypeError on edge-only graphs (len() before materialize_nodes), bare KeyError('x') for the no-arg form, IntCastingNaNError on duplicate bounding_box keys #1968 are separate lanes; nothing here touches them.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AjbKuKheqDu78oapRT5AYm

_{Rebased onto 09633aa after #1969 landed; the only conflict was the adjacent ## [Development] → ### Fixed CHANGELOG line, resolved by keeping both entries.}