Perf by VincentGuyader · Pull Request #1 · ThinkR-open/datadiff

VincentGuyader · 2026-06-13T15:29:55Z

No description provided.

… cas vert) Le cout etait a ~99% dans pointblank, par-colonne, quasi quadratique en nombre de colonnes. Trois leviers, chacun garde par des tests d'equivalence (verdict + cellules KO identiques) poses AVANT optimisation : 1. Court-circuit vert : verdict deduit des booleens de tolerance deja calcules, sans agent pointblank quand tout passe. 2. Etapes pointblank ciblees sur les seules colonnes fautives (cout proportionnel aux ecarts, plus au total). 3. Kernel de tolerance par-colonne + bind unique : supprime la croissance quadratique de add_tolerance_columns.

Copilot

Pull request overview

This PR introduces performance-focused refactors to speed up dataset comparisons by reducing per-column overhead, while adding regression tests to lock in equivalence of results before/after optimization. It also includes several user-facing default/documentation/test changes (notably around language defaults and test file handling) that need cleanup to avoid breaking compatibility and test reliability.

Changes:

Refactors numeric tolerance handling into a per-column kernel and avoids quadratic data.frame growth when adding tolerance columns.
Adds a fast short-circuit path in compare_datasets_from_yaml() to skip building a full pointblank agent when all validations pass, and to build steps only for failing columns otherwise.
Adds new test suites to pin correctness (failing-cell extracts) and cover the tolerance kernel.

Reviewed changes

Copilot reviewed 17 out of 20 changed files in this pull request and generated 22 comments.

Show a summary per file

File	Description
`R/tolerance.R`	Extracts `compute_tolerance_col()` and rewrites local tolerance-column generation to bind once for performance.
`R/fast_path.R`	Adds fast-pass and failing-column selection helpers plus a minimal “pass” agent builder.
`R/compare_datasets_from_yaml.R`	Integrates fast-path logic; updates docs/messages/defaults (some mismatches detected).
`R/pointblank_setup.R`	Adjusts agent setup; changes lang/locale defaults.
`R/utils.R`	Adds `@export` to `%
`man/write_rules_template.Rd`	Updates return docs (currently inconsistent with implementation).
`man/compare_datasets_from_yaml.Rd`	Updates parameter docs (currently inconsistent with implementation).
`man/setup_pointblank_agent.Rd`	Updates default lang/locale docs.
`DESCRIPTION`	Bumps package version to `0.4.5`.
`tests/testthat/test-tolerance-kernel.R`	New unit tests for the per-column tolerance kernel.
`tests/testthat/helper-equivalence.R`	New helper to extract failing-cell identities via pointblank extracts.
`tests/testthat/test-equivalence-guard.R`	New equivalence guard tests + perf invariant checks (short-circuit).
`tests/testthat/test-template-parameters.R`	Test changes; introduces fixed filenames and duplicate cleanup calls.
`tests/testthat/test-main.R`	Test changes; introduces fixed filenames in working directory.
`tests/testthat/test-row-validation.R`	Test changes; introduces fixed filenames in working directory.
`tests/testthat/test-main-integration.R`	Test changes; introduces fixed filenames in working directory.
`tests/testthat/test-key-parameter.R`	Test changes; introduces fixed filenames; contains `$response` vs `$reponse` mismatch and R-version-incompatible lambda syntax.
`tests/testthat/test-input-validation.R`	Test changes; introduces many fixed filenames in working directory.
`tests/testthat/test-pointblank-setup.R`	Updates expectations for default language/locale.
`tests/testthat/test-edge-cases.R`	Updates expected message text.

Files not reviewed (3)

man/compare_datasets_from_yaml.Rd: Generated file
man/setup_pointblank_agent.Rd: Generated file
man/write_rules_template.Rd: Generated file

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-06-13T17:24:39Z

  # Generate template with all parameters configured
-  template_path <- tempfile(fileext = ".yaml")
-  on.exit(unlink(template_path), add = TRUE)
+  template_path <- "test_full_config.yaml"
  write_rules_template(


Fixed in the latest commit. All fixed filenames in test-template-parameters.R have been replaced with tempfile() paths and on.exit(unlink(...), add = TRUE) cleanup. For tests that used multiple paths within one block, all paths are declared upfront and cleaned up in a single on.exit call.

  w <- tryCatch(
    compare_datasets_from_yaml(ref, cand, key = "id"),
-    warning = function(w) conditionMessage(w)
+    warning = \(w) conditionMessage(w)
  )


Copilot · 2026-06-13T17:23:04Z

+  writeLines(text = yaml_content, con = "valid_test.yaml")

-  expect_no_error(object = read_rules(valid_path))
+  expect_no_error(object = read_rules("valid_test.yaml"))

  # Invalid version
  invalid_rules <- list(version = 2, defaults = list(), by_type = list(), by_name = list())
  yaml_content <- yaml::as.yaml(x = invalid_rules)
-  writeLines(text = yaml_content, con = invalid_path)
+  writeLines(text = yaml_content, con = "invalid_test.yaml")
+
+  expect_error(object = read_rules("invalid_test.yaml"))

-  expect_error(object = read_rules(invalid_path))
+  # Clean up
+  unlink(x = c("valid_test.yaml", "invalid_test.yaml"))


Fixed in the latest commit by replacing the manual unlink(temp_path) call with on.exit(unlink(temp_path), add = TRUE) placed immediately after the tempfile() call, ensuring cleanup happens even if an assertion fails mid-test.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 17 out of 20 changed files in this pull request and generated 17 comments.

Files not reviewed (3)

man/compare_datasets_from_yaml.Rd: Generated file
man/setup_pointblank_agent.Rd: Generated file
man/write_rules_template.Rd: Generated file

+eq_col_passes <- function(tbl, col, ref_suffix, na_equal) {
+  eq_precomputed <- tbl[[paste0(col, "__eq")]]
+  if (!is.null(eq_precomputed)) {
+    return(isTRUE(all(eq_precomputed)))
+  }
+  cand_vals <- tbl[[col]]
+  ref_vals  <- tbl[[paste0(col, ref_suffix)]]
+  cmp_res   <- cand_vals == ref_vals
+  row_pass  <- ifelse(is.na(cmp_res), na_equal, cmp_res)
+  isTRUE(all(row_pass))
+}


+build_pass_agent <- function(tbl, label, warn_at, stop_at, lang, locale) {
+  slim <- tbl[, 1, drop = FALSE]
+  agent <- pointblank::create_agent(
+    tbl = slim, label = label,
+    actions = pointblank::action_levels(warn_at = warn_at, stop_at = stop_at),
+    lang = lang, locale = locale
+  )
+  agent %>% pointblank::col_exists(columns = tidyselect::all_of(names(slim)[1]))
+}


 # or non-Parquet formats.
 arrow_dataset_to_duckdb <- function(ds, con, tbl_name) {
-  files <- tryCatch(ds$files, error = function(e) NULL)
+  files <- tryCatch(ds$files, error = \(e) NULL)


        examples_ref <- if (n_dup_keys_ref <= 3) {
-          apply(key_cols_ref, 1, function(r) paste(key, "=", r, collapse = ", "))
+          apply(key_cols_ref, 1, \(r) paste(key, "=", r, collapse = ", "))
        } else {
-          c(apply(key_cols_ref[1:3, , drop = FALSE], 1, function(r) paste(key, "=", r, collapse = ", ")), "...")
+          c(apply(key_cols_ref[1:3, , drop = FALSE], 1, \(r) paste(key, "=", r, collapse = ", ")), "...")
        }


 #' @param date_equal_mode Default comparison mode for date columns
 #' @param datetime_equal_mode Default comparison mode for datetime columns
 #' @param logical_equal_mode Default comparison mode for logical columns
-#' @return The path to the created YAML file, invisibly.
+#' @return Invisible NULL (writes file to disk)
 #' @importFrom yaml write_yaml


    abs: 0.5
 '

-  yaml_path <- tempfile(fileext = ".yaml")
-  on.exit(unlink(yaml_path), add = TRUE)
-  writeLines(yaml_content, yaml_path)
+  writeLines(yaml_content, 'test_key_param.yaml')

  # Test 1: Without key parameter, should use YAML key (customer_id)
-  result_yaml <- compare_datasets_from_yaml(ref, cand, path = yaml_path)
+  result_yaml <- compare_datasets_from_yaml(ref, cand, path = 'test_key_param.yaml')
  expect_true(result_yaml$all_passed)


  # Test with non-existent column (YAML accepts it, validation happens later)
+  template_path <- "test_ignore_invalid.yaml"
  write_rules_template(df, key = "id", ignore_columns_default = "nonexistent", path = template_path)
  rules <- read_rules(template_path)
  expect_equal(rules$defaults$ignore_columns, "nonexistent")
+  unlink(template_path)


+  tmp <- tempfile(fileext = ".yml")
+  write_rules_template(
+    ref,


                                       ref_suffix = "__reference",
                                       label = NULL,
                                       error_msg_no_key = "Without keys, both tables must have the same number of rows.",
-                                       lang = getOption("datadiff.lang", "en"),
-                                       locale = getOption("datadiff.locale", "en_US"),
+                                       lang = "fr",
+                                       locale = "fr_FR",


+# kernel which becomes memory-bandwidth bound on tall inputs.
+
+test_that("absolute tolerance decides ok elementwise", {


Copilot

Pull request overview

Copilot reviewed 20 out of 27 changed files in this pull request and generated 13 comments.

Files not reviewed (7)

man/all_validations_pass.Rd: Generated file
man/build_pass_agent.Rd: Generated file
man/compare_datasets_from_yaml.Rd: Generated file
man/compute_tolerance_col.Rd: Generated file
man/failing_columns.Rd: Generated file
man/setup_pointblank_agent.Rd: Generated file
man/write_rules_template.Rd: Generated file

+# kernel which becomes memory-bandwidth bound on tall inputs.
+
+test_that("absolute tolerance decides ok elementwise", {


@@ -0,0 +1,34 @@
+test_that("build_pass_agent tolerates a zero-column table", {


+  tmp <- tempfile(fileext = ".yml")
+  write_rules_template(
+    ref,
+    key                 = key,
+    numeric_abs         = numeric_abs,
+    integer_abs         = integer_abs,
+    na_equal_default    = na_equal_default,
+    check_count_default = check_count_default,
+    path                = tmp
+  )


+  writeLines(yaml_content, 'test_key_param.yaml')

  # Test 1: Without key parameter, should use YAML key (customer_id)
-  result_yaml <- compare_datasets_from_yaml(ref, cand, path = yaml_path)
+  result_yaml <- compare_datasets_from_yaml(ref, cand, path = 'test_key_param.yaml')
  expect_true(result_yaml$all_passed)


+  writeLines(yaml_content, 'test_key_null.yaml')

  # Test 1: NULL key parameter with no keys in YAML should work (no key comparison)
-  result_null <- compare_datasets_from_yaml(ref, cand, key = NULL, path = yaml_path)
+  result_null <- compare_datasets_from_yaml(ref, cand, key = NULL, path = 'test_key_null.yaml')
  expect_true(result_null$all_passed)


  # Test with non-existent column (YAML accepts it, validation happens later)
+  template_path <- "test_ignore_invalid.yaml"
  write_rules_template(df, key = "id", ignore_columns_default = "nonexistent", path = template_path)
  rules <- read_rules(template_path)
  expect_equal(rules$defaults$ignore_columns, "nonexistent")
+  unlink(template_path)

  # Test with mixed valid/invalid columns
+  template_path <- "test_ignore_mixed.yaml"
  write_rules_template(df, key = "id", ignore_columns_default = c("value", "invalid"), path = template_path)
  rules <- read_rules(template_path)
  expect_equal(rules$defaults$ignore_columns, c("value", "invalid"))
+  unlink(template_path)


+  expect_no_error(write_rules_template(no_rows_df, key = "id", path = "test_no_rows.yaml"))
+  if (file.exists("test_no_rows.yaml")) unlink("test_no_rows.yaml")


+  template_path <- "test_null_label.yaml"
  write_rules_template(df, key = "id", label = NULL, path = template_path)
  rules <- read_rules(template_path)
-  expect_match(rules$defaults$label, "comparison")
+  expect_match(rules$defaults$label, "comparaison")
+  unlink(template_path)


 setup_pointblank_agent <- function(cmp, cols_reference, common_cols, tol_cols,
                                   row_validation_info = NULL, ref_suffix, warn_at, stop_at, label,
-                                   na_equal, lang = getOption("datadiff.lang", "en"), locale = getOption("datadiff.locale", "en_US"),
+                                   na_equal, lang = "fr", locale = "fr_FR",
                                   missing_in_candidate = character(0),


+tol_col_passes <- function(tbl, col) {
+  isTRUE(all(tbl[[paste0(col, "__ok")]]))
+}
+
+eq_col_passes <- function(tbl, col, ref_suffix, na_equal) {
+  eq_precomputed <- tbl[[paste0(col, "__eq")]]
+  if (!is.null(eq_precomputed)) {
+    return(isTRUE(all(eq_precomputed)))
+  }
+  cand_vals <- tbl[[col]]
+  ref_vals  <- tbl[[paste0(col, ref_suffix)]]
+  cmp_res   <- cand_vals == ref_vals
+  row_pass  <- ifelse(is.na(cmp_res), na_equal, cmp_res)
+  isTRUE(all(row_pass))
+}


…on, doc lang exacte (issue #26) Le commit Perf (#1) post-0.4.4 avait re-exporte %||% (masquant rlang::%||% et base::%||% R >= 4.4 au chargement), re-bascule le label par defaut en francais et laisse la vignette pretendre un defaut anglais - le tout sans entree NEWS, contredisant les annonces de la 0.4.4. - %||% re-internalise (conforme NEWS 0.4.4) ; le test qui verrouillait l'export accidentel verrouille desormais l'inverse - prefixe de label du template : comparison (annonce 0.4.4) - vignette : defaut de langue reel documente (fr / fr_FR + options) - normalize_text, validate_row_counts, setup_pointblank_agent documentes dans la section utilitaires de la vignette avec cas d'usage reels (ils n'apparaissaient nulle part)

VincentGuyader added 2 commits June 13, 2026 17:29

bump version

002f12d

VincentGuyader requested a review from Copilot June 13, 2026 15:30

Copilot started reviewing on behalf of VincentGuyader June 13, 2026 15:30

View session

Copilot AI reviewed Jun 13, 2026

View reviewed changes

Potential fix for pull request finding

14560cd

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot started work on behalf of VincentGuyader June 13, 2026 16:25

View session

Potential fix for pull request finding

b7289ae

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot started work on behalf of VincentGuyader June 13, 2026 16:25

View session

Copilot AI added 2 commits June 13, 2026 16:26

Use tempfile in main test

a9ed9a7

test: use tempfile cleanup in row validation integration test

6a60f5e

Copilot finished work on behalf of VincentGuyader June 13, 2026 16:27

Use tempfile and on.exit in main integration tests

734c3a2

Copilot finished work on behalf of VincentGuyader June 13, 2026 16:28

VincentGuyader and others added 2 commits June 13, 2026 18:47

Potential fix for pull request finding

e64fc4c

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Potential fix for pull request finding

e0d996e

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot started work on behalf of VincentGuyader June 13, 2026 16:48

View session

Fix review comment: avoid R 4.1 lambda shorthand in targeted test

2a36c9d

Copilot finished work on behalf of VincentGuyader June 13, 2026 16:50

Copilot started work on behalf of VincentGuyader June 13, 2026 16:54

View session

Fix R 4.0-compatible warning handler syntax in key parameter test

afc25d0

Copilot finished work on behalf of VincentGuyader June 13, 2026 16:56

test: use tempfile cleanup in read_rules YAML validation test

ebcc7b2

Copilot finished work on behalf of VincentGuyader June 13, 2026 16:57

VincentGuyader requested a review from Copilot June 13, 2026 17:20

Copilot started reviewing on behalf of VincentGuyader June 13, 2026 17:20

View session

Copilot started work on behalf of VincentGuyader June 13, 2026 17:21

View session

fix: use on.exit cleanup for tempfile in test-main.R

d448ff8

Copilot finished work on behalf of VincentGuyader June 13, 2026 17:23

Copilot finished work on behalf of VincentGuyader June 13, 2026 17:24

Use tempfile() + on.exit cleanup in test-template-parameters.R

d12e083

Copilot finished work on behalf of VincentGuyader June 13, 2026 17:24

Copilot AI reviewed Jun 13, 2026

View reviewed changes

road to cran

b8915f0

VincentGuyader requested a review from Copilot June 13, 2026 18:23

Copilot started reviewing on behalf of VincentGuyader June 13, 2026 18:23

View session

Copilot AI reviewed Jun 13, 2026

View reviewed changes

VincentGuyader merged commit d98ae33 into main Jun 13, 2026
6 checks passed

VincentGuyader mentioned this pull request Jul 2, 2026

[API] Surface d'export à assainir et contradictions NEWS : %||% ré-exporté, défaut lang="fr" non documenté, exports sans usage documenté #26

Closed

4 tasks

VincentGuyader mentioned this pull request Jul 3, 2026

fix: contradictions NEWS/code resolues - %||% interne, label comparison, doc lang exacte #53

Merged

Navigation Menu

Repository navigation

Perf - #1

Perf#1
VincentGuyader merged 15 commits into
mainfrom
perf

VincentGuyader commented Jun 13, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jun 13, 2026

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jun 13, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		# kernel which becomes memory-bandwidth bound on tall inputs.

		test_that("absolute tolerance decides ok elementwise", {

		@@ -0,0 +1,34 @@
		test_that("build_pass_agent tolerates a zero-column table", {

		expect_no_error(write_rules_template(no_rows_df, key = "id", path = "test_no_rows.yaml"))
		if (file.exists("test_no_rows.yaml")) unlink("test_no_rows.yaml")

Conversation

VincentGuyader commented Jun 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jun 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jun 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants