fix: simplify() honours edge.attr.comb on an already-simple graph again

[krlmlr]

edge.attr.comb does not only combine attributes across merged edges — it decides which survive at all. An edge attribute the combination list does not name is dropped even when every group has exactly one member. The default list ends in "ignore", so simplify(g) is meant to keep weight and drop everything else, and edge.attr.comb = "ignore" is meant to leave no edge attributes at all.

#1981 opened simplify() with if (is_simple(graph)) return(graph), on the grounds that a graph with no loops and no multiple edges has nothing for simplify_impl() to remove. That is true of its structure and false of its attributes.

g <- make_graph(c(1, 2, 2, 3), directed = FALSE)
E(g)$weight <- c(1, 2)
E(g)$foo <- c("a", "b")

edge_attr_names(simplify(g))                            # CRAN 2.3.3: "weight"
edge_attr_names(simplify(g, edge.attr.comb = "ignore")) # CRAN 2.3.3: character(0)

On main both return c("weight", "foo") — silently, with no warning.

Guarding on "has no edge attributes" does not fix it

This is the part worth writing down, because it is the obvious fix and it does not work:

if (is_simple(graph) && length(edge_attr_names(graph)) == 0) return(graph)

is_simple() populates the C core's property cache, and simplify.c has a cache fast path of its own that returns early — without applying edge_comb — once the cache says there is nothing to remove. So merely asking whether a graph is simple changes what simplifying it does. Verified against CRAN 2.3.3:

edge_attr_names(simplify(cold, edge.attr.comb = "ignore"))  # character(0)
invisible(is_simple(warm))
edge_attr_names(simplify(warm, edge.attr.comb = "ignore"))  # "weight" "foo"

2.3.3 has this divergence too. What #1981 changed was making the cache-warm answer the only answer, by warming the cache on every call.

The fix

The check moves to the call site that wants it. graph_from_literal_i() is what #824 and #1981 were about, and it simplifies a graph it has only just built from the formula, before any attribute is set on it — so skipping simplify() there is unobservable beyond the edge order, which is exactly the property #824 asked for.

if (simplify && !is_simple(res)) {
  res <- simplify(res)
}

Everywhere else simplify() goes through simplify_impl() as it always did. That restores edge.attr.comb for every caller, and restores the canonical edge order for direct callers:

g <- make_graph(c(1, 2, 2, 3, 1, 4, 2, 5, 2, 6, 1, 7, 2, 8), directed = FALSE)
as_edgelist(simplify(g))
# this branch and CRAN 2.3.3: 1-2 1-4 1-7 2-3 2-5 2-6 2-8
# main:                       1-2 2-3 1-4 2-5 2-6 1-7 2-8

as_edgelist(graph_from_literal(X -+ Z -+ Y, Y -+ X, X -+ Y))
# unchanged from main, i.e. the formula order #824 asked for:
#   X-Z Z-Y Y-X X-Y

Not fixed here

The residual cache sensitivity is a C-core issue and belongs in src/vendor/cigraph/src/operators/simplify.c, upstream: the fast path taken when the cache says there is nothing to remove should still apply edge_comb. A test pins the current behaviour so that fixing it upstream is noticed here rather than surfacing as a mystery diff.

Where this came from

Triaging the reverse-dependency failures in #2646. Two of the [NEEDS TRIAGE] packages — archeofrag and gemtc — call simplify() directly on already-simple, attribute-free graphs and broke on the edge-order half of this. Both were reproduced against 2.3.3 and against main, and both are restored by this change. (Both also turned out to have genuine bugs of their own that this merely exposed, so they are being reported upstream regardless.)

Testing

Full testthat::test_local() green apart from test-foreign.R:69, which downloads from github.com/igraph/graphsdb and has no network here.

Three new tests in test-operators.R: edge.attr.comb applied to an already-simple graph for the default list, "ignore", and an explicit per-attribute list; the cache sensitivity above; and graph_from_literal()'s edge order, both when the formula needs simplifying and when it does not.

🤖 Generated with Claude Code

https://claude.ai/code/session_01D1xpHRV7yVfgtJg4vp9P7z

---

Generated by Claude Code

[github-actions]

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 15e906e is merged into main:

• ✔️as_adjacency_matrix: 804ms -> 803ms [-1.84%, +1.71%]
• 🚀as_biadjacency_matrix: 809ms -> 800ms [-2.26%, -0.04%]
• ✔️as_data_frame_both: 1.63ms -> 1.61ms [-3.42%, +0.93%]
• ✔️as_long_data_frame: 4.06ms -> 4.04ms [-3.07%, +1.65%]
• ✔️es_attr_filter: 2.89ms -> 2.95ms [-2.98%, +6.99%]
• ✔️graph_from_adjacency_matrix: 125ms -> 127ms [-0.96%, +3.71%]
• ✔️graph_from_data_frame: 3.79ms -> 3.84ms [-0.62%, +2.8%]
• ✔️vs_attr_filter: 1.7ms -> 1.69ms [-3.53%, +2.45%]
• ✔️vs_by_name: 1.06ms -> 1.07ms [-1.65%, +3.65%]
  Further explanation regarding interpretation and methodology can be found in the documentation.