Skip to content

fix: heal main — unleak warnPartialMatchArgs on R 4.2 and name args at norm_coords()/make_star() call sites for the lifecycle-errors leg - #2799

Merged
krlmlr merged 3 commits into
mainfrom
claude/fix-main-codegen
Jul 27, 2026
Merged

fix: heal main — unleak warnPartialMatchArgs on R 4.2 and name args at norm_coords()/make_star() call sites for the lifecycle-errors leg#2799
krlmlr merged 3 commits into
mainfrom
claude/fix-main-codegen

Conversation

@krlmlr

@krlmlr krlmlr commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Prepared with Claude Code (Claude Fable 5).

  • By submitting this pull request, I assign the copyright of my contribution to The igraph development team.

main has been red since 17:36 UTC today,
after #2787 (guard-emitting migrations generator)
and the ellipsis-refactor train merged close together.
The diagnosis found four independent breakages;
the first was fixed by #2798 while this PR was in preparation
(it carries the identical regeneration this branch originally produced,
so that commit dropped out on rebase),
and this PR delivers the remaining three.

1. (landed via #2798) rcc-smoke migrations drift check

For the record, every main push from 17:36 to 19:04 UTC failed
"Smoke test: stock R" at Run ./.github/workflows/custom/after-install:

##[error]Generated ARG_HANDLE blocks are out of sync with the registry (tools/migrations/).
Run 'Rscript tools/generate-migrations.R' and commit the result.

because the refactor PRs had regenerated their ARG_HANDLE blocks
on branches cut before #2787's generator learned to emit
migrate_check_call_tags() guards.
#2798 committed the regenerated blocks;
on this branch Rscript tools/generate-migrations.R
is a verified no-op on R/.

2. rcc: ubuntu-26.04 (4.2) rewrites the migration-fixture snapshot

The runs whose smoke test still passed
(e.g. #2787's own merge run, 30289034966)
failed only in the R 4.2 leg,
at Run ./.github/workflows/update-snapshots,
which keeps force-pushing snapshot PR #2796 and then fails via false.
The recorded difference, in
tests/testthat/test-migration-fixture.R >
test_that("forbidden prefixes error only when legacy arguments engage recovery"):

     Code
       migration_fixture_prefix(c(2, 2), 0.5, di = 2)
     Condition
+      Warning in `migration_fixture_prefix()`:
+      partial argument match of 'di' to 'dimvector'
       Error in `migration_fixture_prefix()`:
       ! Argument `di` matches multiple formal arguments of `migration_fixture_prefix()`.

Root cause: an earlier test in the same file enables
warnPartialMatchArgs via rlang::local_options().
On R < 4.3, restoring that option to NULL (unset)
does not switch the warning off again —
the internal flag keeps its last set value (fixed in R 4.3) —
so it leaks into every later test in the file, on R 4.2 only.
Verified on stock R 4.2.3 (via rig):
set TRUE → restore to NULL still warns, while R 4.5.3 does not.
Beyond the snapshot, the leak would also fail the
expect_no_condition() test two blocks later on R 4.2,
so a skip alone would not heal the leg.

Fix (two layers):

  • pin options(warnPartialMatchArgs = FALSE) before flipping it on,
    so the scoped restore lands on FALSE,
    which resets the flag on every R version;
  • per maintainer decision, additionally skip the affected snapshot test
    on R 4.2 (skip_if(getRversion() < "4.3")),
    mirroring the existing version-gated skips in
    test-adjacency.R and test-community.R.
    (A sweep of every failed rcc run across all branches since 2026-07-18
    confirmed this is the only snapshot that ever diverges on the 4.2 leg;
    the local suite runs R 4.5.3, so the skip is exercised only in CI.)

3. The never-yet-run "with lifecycle errors" leg would fail next

#2782 added a matrix leg (ubuntu-26.04, R release,
IGRAPH_LIFECYCLE_ERRORS=true) that fails any test
in which an unasserted lifecycle deprecation fires.
It has not executed yet — the smoke gate broke first —
but once smoke passes it will,
and the refactors that merged after #2782 was cut
left exactly two unasserted call sites
(all 36 deprecation warnings of the default suite reduce to them):

  • plot.igraph() itself called
    norm_coords(layout, -1, 1, -1, 1) positionally (R/plot.R) —
    32 warnings across the plotting tests;
  • make_star(5, "undirected") with positional mode
    in test-stochastic_matrix.R and test-centralization.R — 5 warnings.

Fix: name the arguments at both sites
(norm_coords(layout, xmin = -1, xmax = 1, ymin = -1, ymax = 1),
make_star(5, mode = "undirected")).

The matrix plumbing itself is sound:
a faithful offline simulation of
.github/workflows/versions-matrix/action.R as of #2798
(stubbing only the svn fetch)
sources .github/versions-matrix.R cleanly
and appends the leg
{"os":"ubuntu-26.04","r":"4.6","env":"IGRAPH_LIFECYCLE_ERRORS=true","desc":"with lifecycle errors"}
to matrix JSON that jsonlite parses.

Validation (all on this branch, R 4.5.3)

  • Rscript tools/generate-migrations.R is a no-op on R/
    (three consecutive runs; git status --porcelain R/ stays empty),
  • air format R/ tools/ tests/ is a no-op,
  • devtools::document() leaves man/ and NAMESPACE untouched,
  • migration-focused tests
    (filter = "migration|generate-migrations|constant-defaults"):
    FAIL 0 | WARN 0 | SKIP 0 | PASS 83,
  • full suite: FAIL 1 | WARN 36 | SKIP 8 | PASS 9181 before the
    call-site fixes,
  • full suite with IGRAPH_LIFECYCLE_ERRORS=true after them:
    FAIL 1 | WARN 0 | SKIP 8 | PASS 9181
    the single failure is the sandbox-only test-foreign.R
    graph_from_graphdb() download
    (no network in the sandbox; CI can reach it).

Generated by Claude Code

claude added 3 commits July 27, 2026 19:18
The "head args go through base R partial matching" test enables
`warnPartialMatchArgs` via `rlang::local_options()`.
On R < 4.3, restoring the option to NULL (unset) at test exit
does not switch the warning off again --
the internal flag keeps its last set value --
so the warning leaked into every later test in the file.
On the rcc-full ubuntu-26.04 (4.2) leg this rewrote
tests/testthat/_snaps/migration-fixture.md,
prepending "partial argument match of 'di' to 'dimvector'"
to the forbidden-prefix guard snapshot,
and failed the update-snapshots step (snapshot PR #2796).

Pin the option to FALSE before flipping it on,
so the scoped restore lands on FALSE --
which does reset the flag -- on every R version.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016M32izVHZPfxAqemAe4BrX
…ors leg

#2782 added an rcc-full leg (IGRAPH_LIFECYCLE_ERRORS=true)
that fails any test in which an unasserted lifecycle deprecation fires.
The ellipsis refactors that merged after #2782 was cut
left two call sites behind:
`plot.igraph()` called `norm_coords(layout, -1, 1, -1, 1)` positionally,
and tests called `make_star(5, "undirected")` with positional `mode`.
Together they account for all 36 deprecation warnings
in the default test run,
and each would fail its test on the new leg --
which has never executed yet because the smoke gate broke first.

Name the arguments at both sites.
With this, the full suite passes with IGRAPH_LIFECYCLE_ERRORS=true:
FAIL 0 (plus the sandbox-only test-foreign.R network error), WARN 0.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016M32izVHZPfxAqemAe4BrX
On the rcc-full ubuntu-26.04 (4.2) leg, this snapshot records an extra

    Warning in `migration_fixture_prefix()`:
    partial argument match of 'di' to 'dimvector'

ahead of the guard error,
which rewrites tests/testthat/_snaps/migration-fixture.md
and keeps force-pushing snapshot PR #2796.
Per maintainer decision, skip the test on R 4.2,
mirroring the existing version-gated skips
(e.g. test-adjacency.R, test-community.R).

The `warnPartialMatchArgs` pin in the previous commit
already removes the R 4.2 leak that produced the extra warning;
the skip makes the intent explicit
and keeps the snapshot single-variant on older R either way.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016M32izVHZPfxAqemAe4BrX
@krlmlr
krlmlr enabled auto-merge (squash) July 27, 2026 19:30
@github-actions

Copy link
Copy Markdown
Contributor

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

  • ✔️as_adjacency_matrix: 792ms -> 793ms [-0.33%, +0.45%]
  • ✔️as_biadjacency_matrix: 775ms -> 775ms [-1.06%, +1.19%]
  • ✔️as_data_frame_both: 1.55ms -> 1.54ms [-1.4%, +0.31%]
  • ✔️as_long_data_frame: 3.92ms -> 3.93ms [-0.61%, +0.85%]
  • ✔️es_attr_filter: 2.73ms -> 2.73ms [-0.8%, +0.76%]
  • ✔️graph_from_adjacency_matrix: 139ms -> 141ms [-0.57%, +2.87%]
  • ✔️graph_from_data_frame: 3.57ms -> 3.55ms [-2.02%, +0.61%]
  • ✔️vs_attr_filter: 1.57ms -> 1.56ms [-1.64%, +0.31%]
  • ✔️vs_by_name: 1.01ms -> 1ms [-1.99%, +0.23%]
    Further explanation regarding interpretation and methodology can be found in the documentation.

@krlmlr
krlmlr merged commit e591b4e into main Jul 27, 2026
8 of 9 checks passed
@krlmlr
krlmlr deleted the claude/fix-main-codegen branch July 27, 2026 19:42
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