fix: erreur explicite quand une colonne cle est absente d'un dataset

DESCRIPTION

@@ -1,6 +1,6 @@
 Package: datadiff
 Title: Data Validation Based on 'YAML' Rules
-Version: 0.4.9
+Version: 0.4.9.9000
 Authors@R: c(
     person("Vincent", "Guyader", , "vincent@thinkr.fr", role = c("cre", "aut"),
            comment = c(ORCID = "0000-0003-0671-9270")),

NEWS.md

@@ -1,3 +1,20 @@
+# datadiff (development version)
+
+## Breaking changes
+
+* `compare_datasets_from_yaml()` now raises an explicit error when one or more
+  key columns are absent from either dataset, naming the missing column(s) and
+  the dataset(s) concerned, on every code path (with or without an explicit
+  YAML rules file). It previously emitted a vague message
+  ("could not find key in both data") and returned a truncated 6-field list
+  (no `coverage`, no `summary`) that broke downstream consumers such as
+  `datadiff_report_html()` (issue #16).
+
+* `compare_datasets_from_yaml()` rejects an empty (`character(0)`) or
+  non-character `key` with a clear error. An empty key previously fell through
+  to a keyless cross join (deprecated dplyr behavior producing a cartesian
+  product).
+
 # datadiff 0.4.9
 
 ## Bug fixes

R/compare_datasets_from_yaml.R

@@ -111,6 +111,49 @@ read_rules <- function(path) {
   r
 }
 
+#' Validate a comparison key against both datasets
+#'
+#' Shared guard for every code path that consumes a key: checks the type and
+#' emptiness of the key, then its presence in both datasets, and raises an
+#' explicit error naming the missing column(s) and the dataset(s) concerned.
+#'
+#' @param key Character vector of key column names.
+#' @param ref_cols Column names of the reference dataset.
+#' @param cand_cols Column names of the candidate dataset.
+#' @return \code{NULL}, invisibly. Called for its side effect (error).
+#' @noRd
+validate_comparison_key <- function(key, ref_cols, cand_cols) {
+  if (!is.character(key) || length(key) == 0) {
+    stop(
+      "Parameter 'key' must be a non-empty character vector specifying column name(s) to use as join key(s).",
+      call. = FALSE
+    )
+  }
+  missing_key_ref  <- setdiff(key, ref_cols)
+  missing_key_cand <- setdiff(key, cand_cols)
+  if (length(missing_key_ref) > 0 || length(missing_key_cand) > 0) {
+    describe_missing <- function(cols, dataset_name) {
+      if (length(cols) == 0) {
+        return(NULL)
+      }
+      sprintf(
+        "%s missing in %s",
+        paste(sprintf("'%s'", cols), collapse = ", "),
+        dataset_name
+      )
+    }
+    details <- c(
+      describe_missing(missing_key_ref, dataset_name = "data_reference"),
+      describe_missing(missing_key_cand, dataset_name = "data_candidate")
+    )
+    stop(sprintf(
+      "Key column(s) not found: %s.",
+      paste(details, collapse = "; ")
+    ), call. = FALSE)
+  }
+  invisible(NULL)
+}
+
 #' Compare datasets using YAML validation rules
 #'
 #' Main function for comparing reference and candidate datasets using configurable
@@ -119,7 +162,9 @@ read_rules <- function(path) {
 #'
 #' @param data_reference Reference dataframe, tibble, or lazy table (tbl_lazy)
 #' @param data_candidate Candidate dataframe to validate against reference
-#' @param key Optional character vector of column names to use as join keys for ordered comparison
+#' @param key Optional character vector of column names to use as join keys for
+#'   ordered comparison. Every key column must exist in both datasets; otherwise
+#'   an error is raised naming the missing column(s) and the dataset(s) concerned.
 #' @param path Path to YAML file containing validation rules. If NULL, default rules are
 #'   generated automatically based on the reference dataset structure.
 #' @param warn_at Warning threshold as fraction of failing tests (default: 1e-14)
@@ -264,6 +309,18 @@ compare_datasets_from_yaml <- function(data_reference,
       data_candidate <- arrow_dataset_to_duckdb(data_candidate, fresh_con, "datadiff_cand")
   }
 
+  # Validate the key argument against BOTH datasets before any other consumer:
+  # the auto-generated-template path (path = NULL) would otherwise reach
+  # write_rules_template() first, which validates the reference only and with
+  # a less specific error.
+  if (!is.null(key)) {
+    validate_comparison_key(
+      key,
+      ref_cols = get_col_names(data_reference),
+      cand_cols = get_col_names(data_candidate)
+    )
+  }
+
   # If no path provided, create a temporary YAML with default rules
   if (is.null(path)) {
     path <- tempfile(fileext = ".yaml")
@@ -403,19 +460,13 @@ compare_datasets_from_yaml <- function(data_reference,
   row_validation_info <- validate_row_counts(data_reference_p, data_candidate_p, rules)
 
   if (!is.null(key)) {
-    if (isFALSE(all(key %in% get_col_names(data_reference_p))) | isFALSE(all(key %in% get_col_names(data_candidate_p)))) {
-      message("could not find key in both data")
-      return(
-        list(
-          all_passed = FALSE,
-          agent = NULL,
-          reponse = NULL,
-          missing_in_candidate = NULL,
-          extra_in_candidate = NULL,
-          applied_rules = NULL
-        )
-      )
-    }
+    # Re-validated here because the key may come from the YAML rules
+    # (defaults$keys), which the argument-level check upstream cannot see.
+    validate_comparison_key(
+      key,
+      ref_cols = get_col_names(data_reference_p),
+      cand_cols = get_col_names(data_candidate_p)
+    )
 
     # Join candidate to reference on key to handle different row counts
     cmp <- left_join(data_candidate_p, data_reference_p, by = key, suffix = c("", ref_suffix))

tests/testthat/test-key-parameter.R

@@ -72,7 +72,7 @@ by_type:
   unlink('test_key_null.yaml')
 })
 
-test_that("compare_datasets_from_yaml key parameter validation works", {
+test_that("compare_datasets_from_yaml errors when key columns are absent", {
   ref <- data.frame(id = 1:3, value = c(10.0, 20.0, 30.0))
   cand <- data.frame(id = 1:3, value = c(10.1, 20.1, 30.1))
 
@@ -85,17 +85,92 @@ by_type:
     abs: 0.5
 '
 
-  writeLines(yaml_content, 'test_key_validation.yaml')
+  yaml_path <- tempfile(fileext = ".yaml")
+  on.exit(unlink(yaml_path), add = TRUE)
+  writeLines(yaml_content, con = yaml_path)
 
-  # Test: Invalid key should return an empty output with all_passed FALSE
+  # Key absent from both datasets: explicit error naming both datasets
+  err_both <- tryCatch(
+    compare_datasets_from_yaml(ref, cand, key = "nonexistent", path = yaml_path),
+    error = function(e) {
+      conditionMessage(e)
+    }
+  )
+  expect_match(err_both, "Key column\\(s\\) not found")
+  expect_match(err_both, "'nonexistent'", fixed = TRUE)
+  expect_match(err_both, "data_reference", fixed = TRUE)
+  expect_match(err_both, "data_candidate", fixed = TRUE)
+
+  # Key present in the reference but absent from the candidate:
+  # only the candidate is named
+  cand_no_id <- data.frame(idx = 1:3, value = c(10.1, 20.1, 30.1))
+  err_cand <- tryCatch(
+    compare_datasets_from_yaml(ref, cand_no_id, key = "id", path = yaml_path),
+    error = function(e) {
+      conditionMessage(e)
+    }
+  )
+  expect_match(err_cand, "'id'", fixed = TRUE)
+  expect_match(err_cand, "data_candidate", fixed = TRUE)
+  expect_no_match(err_cand, "data_reference", fixed = TRUE)
+
+  # Same scenario without an explicit YAML path (auto-generated template)
+  expect_error(
+    compare_datasets_from_yaml(ref, cand_no_id, key = "id"),
+    regexp = "missing in data_candidate"
+  )
 
-  res <- compare_datasets_from_yaml(ref, cand, key = "nonexistent", path = 'test_key_validation.yaml')
+  # Without an explicit YAML path, a key absent from the *reference* must get
+  # the same explicit error format (not the reference-only error of
+  # write_rules_template on the auto-generated-template path)
+  ref_no_id <- data.frame(idx = 1:3, value = c(10.0, 20.0, 30.0))
+  cand_id   <- data.frame(id = 1:3, value = c(10.1, 20.1, 30.1))
+  err_ref <- tryCatch(
+    compare_datasets_from_yaml(ref_no_id, cand_id, key = "id"),
+    error = function(e) {
+      conditionMessage(e)
+    }
+  )
+  expect_match(err_ref, "'id'", fixed = TRUE)
+  expect_match(err_ref, "missing in data_reference", fixed = TRUE)
+  expect_no_match(err_ref, "data_candidate", fixed = TRUE)
+
+  # Multi-column key with a single absent column: only that column is reported
+  ref_multi  <- data.frame(id = 1:3, grp = 1:3, value = 1:3)
+  cand_multi <- data.frame(id = 1:3, value = 1:3)
+  err_multi <- tryCatch(
+    compare_datasets_from_yaml(ref_multi, cand_multi, key = c("id", "grp"), path = yaml_path),
+    error = function(e) {
+      conditionMessage(e)
+    }
+  )
+  expect_match(err_multi, "'grp'", fixed = TRUE)
+  expect_no_match(err_multi, "'id'", fixed = TRUE)
+})
 
-  expect_false(res$all_passed)
-  expect_null(res$agent)
-  expect_null(res$reponse)
-  # Clean up
-  unlink('test_key_validation.yaml')
+test_that("compare_datasets_from_yaml rejects empty or non-character keys", {
+  ref  <- data.frame(id = 1:3, value = c(10.0, 20.0, 30.0))
+  cand <- data.frame(id = 1:3, value = c(10.1, 20.1, 30.1))
+
+  yaml_path <- tempfile(fileext = ".yaml")
+  on.exit(unlink(yaml_path), add = TRUE)
+  write_rules_template(ref, key = "id", path = yaml_path)
+
+  # An empty key must not silently fall through to a keyless cross join
+  expect_error(
+    compare_datasets_from_yaml(ref, cand, key = character(0), path = yaml_path),
+    regexp = "non-empty character vector"
+  )
+  expect_error(
+    compare_datasets_from_yaml(ref, cand, key = character(0)),
+    regexp = "non-empty character vector"
+  )
+
+  # A non-character key gets a clear type error, not a missing-column error
+  expect_error(
+    compare_datasets_from_yaml(ref, cand, key = 1L, path = yaml_path),
+    regexp = "non-empty character vector"
+  )
 })
 
 test_that("key parameter takes precedence over YAML in edge cases", {