Skip to content

Capture nested chunks of plain tibbles in one native call - #470

Merged
jbearak merged 28 commits into
mainfrom
claude/nest-chunks
Oct 27, 2026
Merged

jbearak merged 28 commits into
mainfrom
claude/nest-chunks

Conversation

@jbearak

@jbearak jbearak commented Oct 27, 2026 •

Copy link
Copy Markdown
Owner

Summary

nest_by() and group_nest() gather each group's rows into a plain tibble, nest the tibbles in a list_of and capture that list when they publish the result. On main, nest_by(b, caseid) over the 42,739 women in the first 100,000 rows of the India 2021 DHS births file takes about 3.6 seconds. Most of it went to three R steps that ran once for each group: the verdict of which columns to gather natively, a second selection of the nested columns, and the capture's per-tibble work in a hash table, attributes(), .row_names_info() and .metadata_copy() for each column.

This PR moves the verdict into C_dtatools_gather_native(), selects the nested columns once, and adds C_dtatools_capture_frame_list(), which captures a list of plain tibbles in one call as the R code does. ADR 0126 records the details.

Behavior

Results are unchanged. The native calls return NULL, and the R code runs, wherever the R code could run a provider's code or coerce a value: ALTREP lists, names, declarations, row names and columns other than the package's own, classed lists other than a list_of, other attributes, attributed or S4 names, classes and row names. The capture copies attributes in the order the R code sets them.

.dibble_nest() selects the nested columns once instead of once for each group. Before each group's chunk, C_dtatools_nested_unchanged() checks what the selection reads: keep, the grouping names, the list of columns, its names and the selected columns' positions. Once that check fails, or a gather reaches R code, each later group selects the columns again, as the R code does. Every group selects them when R code could run as the chunks are built: a foreign ALTREP column, list of names or group's rows, rows with a class, or a keep, list or grouping names that are objects, ALTREP or carry attributes. isTRUE() and setdiff() see at most two calls, for the selection and the prototype, until a group selects again.

vctrs::new_list_of() gives R's ALTREP wrapper for a list of 64 or more chunks, which the capture cannot see through with R's C API, so .dibble_nest() sets the list_of's attributes on the new chunks' list in place. Chunks with a missing name still go through vctrs::new_list_of().

A traced or replaced .declared_dta_storage(), .dta_merge_has_compact_storage(), .data_columns(), .row_names_info(), .metadata_copy(), utils::hashtab() or rlang::obj_address() no longer sees these calls. The R capture moves to .capture_dibble_nested_r(), so the test that edits a string literal of each profiled function now skips .capture_dibble_nested(), whose body holds none.

Timing

Best of three runs in fresh R sessions, in seconds, with the builds interleaved. Before is main at cd976e8.

Call Before After Tibble
nest_by(b, caseid) 3.562 0.950 0.481
group_nest(b, caseid) 3.448 0.643 0.540
group_split(group_by(b, caseid)) 5.106 4.561 0.426

tidyr::nest(b, data = -caseid) takes about 4.6 seconds with either build. Over five sessions each, interleaved, its fastest run took 4.605 seconds on main and 4.611 with this PR. group_split() still spends most of its time building each group's dibble.

Tests

  • test-group-capture.R compares the native capture with the R capture for lists of tibbles, a tibble that appears twice and as the prototype, reversed attribute orders and a named list_of. It checks that the capture declines other lists and tibbles, ALTREP columns, names and row names, names with attributes, and S4 names, classes and row names.
  • test-split-column.R compares the native verdict with the R verdict for every column of three files, materialized compact columns, broken declarations, data frames and ALTREP lists, declarations and names, and checks that ALTREP names' callbacks do not run in the native call.
  • test-group-capture.R checks, for group_nest() and nest_by(), that the second group's chunk takes a column renamed by reference after the first group's: by a vctrs proxy method, by the provider of a foreign ALTREP column, rows or keep, by a length() method of classed rows or keep, and by a traced tibble::new_tibble(). It also checks that a class a traced new_tibble() gives keep changes the second group's columns.
  • The full suite passes: 170 files, 2,310 tests. The native manifest and repository guard tests pass.
  • Codex review ran thirteen rounds. The first found that provider callbacks could run in another order than the R code's, so the capture checks every tibble before it copies a column and declines foreign ALTREP. The second found that names with attributes could be coerced differently. The third found that the capture set names before other attributes and dropped the S4 flag of class vectors. The fourth found that ALTREP column names could run a provider's code after the verdict. The fifth found that the capture's preflight checked a repeated tibble more than once. The fifth through the twelfth found ways R code could run between groups and change what the cached selection reads: a foreign ALTREP column, a vctrs gather, a group's foreign ALTREP or classed rows, a classed or foreign ALTREP keep, a traced tibble::new_tibble(), and a class added to keep by reference. The eighth also found a stale manifest. This PR fixes each, and the thirteenth review is clean.

# Conflicts:
#	r-package/dtatools/NEWS.md
#	r-package/dtatools/R/dibble-rows.R
#	r-package/dtatools/src/dtatools-internal.h
#	r-package/dtatools/src/init.c
#	r-package/dtatools/tools/native-test-manifest.json
@coderabbitai

coderabbitai Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

# Conflicts:
#	r-package/dtatools/NEWS.md
@jbearak
jbearak merged commit 79d9721 into main Oct 27, 2026
4 checks passed
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.

1 participant