From 86ab7abd0aa7d91b28e4ea55858c9e13a3ce110c Mon Sep 17 00:00:00 2001 From: Simon Couch Date: Mon, 10 Aug 2026 15:54:51 -0500 Subject: [PATCH 1/3] merge warehouse dictionary metadata --- R/catalog.R | 269 +++++++++++++++++++++++ R/data-dictionary.R | 21 +- R/data-source.R | 26 +++ R/tools.R | 2 +- man/data_source.Rd | 4 + tests/testthat/_snaps/data-dictionary.md | 19 ++ tests/testthat/helper-live-warehouses.R | 16 ++ tests/testthat/test-data-dictionary.R | 168 ++++++++++++++ tests/testthat/test-live-warehouses.R | 75 ++++++- 9 files changed, 595 insertions(+), 5 deletions(-) diff --git a/R/catalog.R b/R/catalog.R index f41b010..8b533ea 100644 --- a/R/catalog.R +++ b/R/catalog.R @@ -94,3 +94,272 @@ catalog_match_exact_relation <- function(relations, id) { relation$id <- id relation } + +catalog_merge_dictionary <- function( + dictionary, + relations, + con, + describe_relation, + identifier_case, + call = rlang::caller_env() +) { + if (is.null(dictionary) || length(dictionary$tables) == 0L) { + return(list(dictionary = dictionary, relations = relations)) + } + + matches <- catalog_dictionary_matches( + dictionary, + relations, + identifier_case, + call = call + ) + tables <- list() + for (authored_name in names(matches)) { + label <- matches[[authored_name]] + if (is.na(label)) { + next + } + relation <- relations[[label]] + columns <- describe_relation(con, relation$id, call = call) + relation$columns <- columns + relations[[label]] <- relation + tables[[label]] <- catalog_merge_dictionary_table( + dictionary$tables[[authored_name]], + authored_name, + label, + relation, + columns, + identifier_case, + call = call + ) + } + dictionary$tables <- tables + + list(dictionary = dictionary, relations = relations) +} + +catalog_dictionary_matches <- function( + dictionary, + relations, + identifier_case, + call = rlang::caller_env() +) { + matches <- stats::setNames( + rep(NA_character_, length(dictionary$tables)), + names(dictionary$tables) + ) + claimed <- character() + for (authored_name in names(dictionary$tables)) { + label <- catalog_dictionary_match( + authored_name, + relations, + identifier_case, + call = call + ) + if (is.null(label)) { + next + } + if (label %in% claimed) { + other <- names(matches)[which(matches == label)] + cli::cli_abort( + "Authored tables {.val {c(other, authored_name)}} both match selected + relation {.val {label}}.", + call = call + ) + } + matches[[authored_name]] <- label + claimed <- c(claimed, label) + } + matches +} + +catalog_dictionary_match <- function( + authored_name, + relations, + identifier_case, + call = rlang::caller_env() +) { + labels <- names(relations) + exact <- labels[labels == authored_name] + if (length(exact) == 1L) { + return(exact) + } + + authored <- catalog_normalize_identifier(authored_name, identifier_case) + normalized_labels <- catalog_normalize_identifier(labels, identifier_case) + exact <- labels[normalized_labels == authored] + if (length(exact) == 1L) { + return(exact) + } + if (length(exact) > 1L) { + catalog_abort_ambiguous_dictionary_table( + authored_name, + exact, + call = call + ) + } + + relation_names <- vapply( + relations, + function(relation) relation$id@name[["table"]], + character(1) + ) + relative <- labels[ + catalog_normalize_identifier(relation_names, identifier_case) == authored + ] + if (length(relative) == 0L && grepl(".", authored_name, fixed = TRUE)) { + authored_path <- strsplit(authored_name, ".", fixed = TRUE)[[1]] + relative <- labels[vapply( + relations, + catalog_relation_has_suffix, + logical(1), + suffix = authored_path, + identifier_case = identifier_case + )] + } + if (length(relative) > 1L) { + catalog_abort_ambiguous_dictionary_table( + authored_name, + relative, + call = call + ) + } + if (length(relative) == 1L) relative else NULL +} + +catalog_relation_has_suffix <- function(relation, suffix, identifier_case) { + path <- unname(relation$id@name) + if (length(suffix) > length(path)) { + return(FALSE) + } + path <- utils::tail(path, length(suffix)) + identical( + catalog_normalize_identifier(path, identifier_case), + catalog_normalize_identifier(suffix, identifier_case) + ) +} + +catalog_abort_ambiguous_dictionary_table <- function( + authored_name, + matches, + call = rlang::caller_env() +) { + cli::cli_abort( + c( + "Authored table {.val {authored_name}} matches more than one selected + relation: {.val {matches}}.", + "i" = "Use its fully qualified name in the data dictionary." + ), + call = call + ) +} + +catalog_merge_dictionary_table <- function( + authored, + authored_name, + selected_name, + relation, + columns, + identifier_case, + call = rlang::caller_env() +) { + authored$description <- catalog_authored_prose( + authored$description, + relation$description + ) + authored$kind <- relation$kind %||% authored$kind + authored$columns <- catalog_merge_dictionary_columns( + authored$columns, + columns, + identifier_case, + call = call + ) + if (!identical(authored_name, selected_name)) { + authored$.authored_name <- authored_name + } + + definition_names <- names(authored$definitions) + shadowed <- definition_names[ + catalog_normalize_identifier(definition_names, identifier_case) %in% + catalog_normalize_identifier(names(authored$columns), identifier_case) + ] + if (length(shadowed)) { + cli::cli_abort( + "Definitions on table {.val {selected_name}} must not share a name with + its discovered columns: {.val {shadowed}}.", + call = call + ) + } + authored +} + +catalog_merge_dictionary_columns <- function( + authored, + discovered, + identifier_case, + call = rlang::caller_env() +) { + out <- lapply(seq_len(nrow(discovered)), function(i) { + catalog_dictionary_column(discovered, i) + }) + names(out) <- discovered$column + + normalized <- catalog_normalize_identifier(names(out), identifier_case) + for (authored_name in names(authored)) { + exact <- which(names(out) == authored_name) + candidates <- if (length(exact) == 1L) { + exact + } else { + which( + normalized == + catalog_normalize_identifier(authored_name, identifier_case) + ) + } + if (length(candidates) > 1L) { + cli::cli_abort( + "Authored column {.val {authored_name}} matches more than one discovered + column: {.val {names(out)[candidates]}}.", + call = call + ) + } + if (length(candidates) == 0L) { + out[[authored_name]] <- authored[[authored_name]] + next + } + + discovered_name <- names(out)[[candidates]] + column <- utils::modifyList( + out[[discovered_name]], + authored[[authored_name]], + keep.null = TRUE + ) + column$type <- out[[discovered_name]]$type + column$nullable <- out[[discovered_name]]$nullable + column$description <- catalog_authored_prose( + authored[[authored_name]]$description, + out[[discovered_name]]$description + ) + out[[discovered_name]] <- column + } + out +} + +catalog_dictionary_column <- function(discovered, i) { + description <- discovered$description[[i]] + if (is.na(description) || !nzchar(description)) { + description <- NULL + } + list( + type = discovered$type[[i]], + nullable = discovered$nullable[[i]], + description = description + ) +} + +catalog_authored_prose <- function(authored, discovered) { + if (is.null(authored) || !nzchar(authored)) discovered else authored +} + +catalog_normalize_identifier <- function(x, identifier_case) { + switch(identifier_case, upper = toupper(x), lower = tolower(x), x) +} diff --git a/R/data-dictionary.R b/R/data-dictionary.R index 069b69f..4877aa6 100644 --- a/R/data-dictionary.R +++ b/R/data-dictionary.R @@ -184,7 +184,12 @@ dictionary_columns_text <- function(columns, live = NULL) { dictionary_column_line <- function(name, spec, live_type = NULL) { spec <- spec %||% list() qualifier <- paste( - c(spec$type %||% live_type, spec$units, unlist(spec$constraints)), + c( + spec$type %||% live_type, + dictionary_nullability_fact(spec$nullable), + spec$units, + unlist(spec$constraints) + ), collapse = ", " ) facts <- c( @@ -206,6 +211,13 @@ dictionary_column_line <- function(name, spec, live_type = NULL) { line } +dictionary_nullability_fact <- function(nullable) { + if (!is.logical(nullable) || length(nullable) != 1L || is.na(nullable)) { + return(NULL) + } + if (nullable) "nullable" else "not nullable" +} + # `values` can be a sequence ([M, F]) or a map of value to meaning # ({M: Male}). dictionary_values_fact <- function(values) { @@ -238,11 +250,16 @@ dictionary_examples_fact <- function(examples) { dictionary_relationships_text <- function(dictionary, table) { relationships <- dictionary$relationships + table_names <- c(table, dictionary$tables[[table]][[".authored_name"]]) mentions <- vapply( relationships, function(rel) { text <- paste(c(rel$join, rel$description), collapse = " ") - grepl(word_pattern(table), text, ignore.case = TRUE) + any(vapply( + table_names, + function(name) grepl(word_pattern(name), text, ignore.case = TRUE), + logical(1) + )) }, logical(1) ) diff --git a/R/data-source.R b/R/data-source.R index b59e9cd..6629f23 100644 --- a/R/data-source.R +++ b/R/data-source.R @@ -63,6 +63,10 @@ #' the tool result: its prose, documented columns, relationships, and #' definitions of glossary terms it references. `describe_table` merges #' documented columns with the table's live schema. +#' * For Snowflake and Databricks sources, a fully qualified dictionary table +#' name matches the same selected relation. A relative name is accepted when +#' it matches only one selected relation. Authored prose takes precedence, +#' while warehouse column types remain authoritative. #' * When the agent also has a [context_layer()], the dictionary's prose is #' indexed for the `search_context` tool. #' @@ -152,6 +156,16 @@ data_source_connection <- function( if (length(table_registry$validate$labels)) { check_table_ids_exist(con, table_registry$validate, call = call) } + merged <- catalog_merge_dictionary( + dictionary, + table_registry$relations, + con, + snowflake_describe_relation, + "upper", + call = call + ) + dictionary <- merged$dictionary + table_registry$relations <- merged$relations commons_span_set_attribute( span, "commons.data_source.n_tables", @@ -172,6 +186,16 @@ data_source_connection <- function( if (length(table_registry$validate$labels)) { check_table_ids_exist(con, table_registry$validate, call = call) } + merged <- catalog_merge_dictionary( + dictionary, + table_registry$relations, + con, + databricks_describe_relation, + "lower", + call = call + ) + dictionary <- merged$dictionary + table_registry$relations <- merged$relations commons_span_set_attribute( span, "commons.data_source.n_tables", @@ -502,6 +526,8 @@ source_describe <- function( type = vapply(sample, function(x) class(x)[[1]], character(1)), row.names = NULL ) + } else if (!is.null(relation$columns)) { + schema <- relation$columns } else if (is_snowflake_connection(source$con)) { schema <- snowflake_describe_relation(source$con, id, call = call) } else { diff --git a/R/tools.R b/R/tools.R index 937672e..2c76ad9 100644 --- a/R/tools.R +++ b/R/tools.R @@ -347,7 +347,7 @@ describe_table_tool <- function(source, table, source_name = NULL, tracker = NUL entry <- source$dictionary$tables[[table]] relation <- c( if (!is.null(d$kind)) sprintf("Relation type: %s.", d$kind), - d$description + if (is.null(entry)) d$description ) sample <- sprintf( diff --git a/man/data_source.Rd b/man/data_source.Rd index f7dcb52..19b5c59 100644 --- a/man/data_source.Rd +++ b/man/data_source.Rd @@ -79,6 +79,10 @@ tool or a SQL query---the table's full dictionary entry rides along with the tool result: its prose, documented columns, relationships, and definitions of glossary terms it references. \code{describe_table} merges documented columns with the table's live schema. +\item For Snowflake and Databricks sources, a fully qualified dictionary table +name matches the same selected relation. A relative name is accepted when +it matches only one selected relation. Authored prose takes precedence, +while warehouse column types remain authoritative. \item When the agent also has a \code{\link[=context_layer]{context_layer()}}, the dictionary's prose is indexed for the \code{search_context} tool. } diff --git a/tests/testthat/_snaps/data-dictionary.md b/tests/testthat/_snaps/data-dictionary.md index 1394f81..4c9249c 100644 --- a/tests/testthat/_snaps/data-dictionary.md +++ b/tests/testthat/_snaps/data-dictionary.md @@ -23,3 +23,22 @@ Error in `data_source()`: ! `dictionary` must be a path to a data-dict.yaml file. +# relative authored table names must be unambiguous + + Code + catalog_merge_dictionary(dictionary, relations, NULL, function(...) NULL, + "upper") + Condition + Error: + ! Authored table "orders" matches more than one selected relation: "ANALYTICS.PUBLIC.ORDERS" and "ANALYTICS.STAGING.ORDERS". + i Use its fully qualified name in the data dictionary. + +# discovered columns cannot shadow governed definitions + + Code + catalog_merge_dictionary(dictionary, relations, NULL, function(...) + catalog_test_columns(), "upper") + Condition + Error: + ! Definitions on table "ANALYTICS.PUBLIC.ORDERS" must not share a name with its discovered columns: "order_id". + diff --git a/tests/testthat/helper-live-warehouses.R b/tests/testthat/helper-live-warehouses.R index 564952e..9812d44 100644 --- a/tests/testthat/helper-live-warehouses.R +++ b/tests/testthat/helper-live-warehouses.R @@ -42,3 +42,19 @@ warehouse_read_one <- function(con, id) { ) DBI::dbGetQuery(con, sql) } + +warehouse_test_dictionary <- function(table, column) { + new_data_dictionary(list(tables = stats::setNames( + list(list( + description = "Authored live table description.", + columns = stats::setNames( + list(list( + type = "authored_type", + description = "Authored live column description." + )), + column + ) + )), + table + ))) +} diff --git a/tests/testthat/test-data-dictionary.R b/tests/testthat/test-data-dictionary.R index f54ec11..59089b6 100644 --- a/tests/testthat/test-data-dictionary.R +++ b/tests/testthat/test-data-dictionary.R @@ -42,6 +42,16 @@ local_dict_source <- function(env = parent.frame()) { data_source(sales = test_sales(), dictionary = local_dict_path(env)) } +catalog_test_columns <- function() { + data.frame( + column = c("AMOUNT", "ORDER_ID"), + type = c("NUMBER(38,2)", "NUMBER(38,0)"), + nullable = c(TRUE, FALSE), + description = c("Warehouse amount description.", "Warehouse key."), + row.names = NULL + ) +} + test_that("data_dictionary() reads and keys tables and columns", { skip_if_not_installed("yaml") dict <- data_dictionary(local_dict_path()) @@ -169,6 +179,164 @@ test_that("describe_table merges the dictionary with the live schema", { expect_match(res@value, "Sample summary", fixed = TRUE) }) +test_that("warehouse metadata supplements an authored dictionary", { + relations <- list( + "ANALYTICS.PUBLIC.ORDERS" = list( + id = DBI::Id( + catalog = "ANALYTICS", + schema = "PUBLIC", + table = "ORDERS" + ), + kind = "table", + description = "Warehouse table description." + ) + ) + dictionary <- new_data_dictionary(list( + tables = list( + orders = list( + description = "Authored table description.", + columns = list( + amount = list( + type = "number(quantity)", + units = "USD", + description = "Authored column description." + ), + missing = list(description = "Not in the warehouse.") + ) + ), + unselected = list(description = "Not selected.") + ) + )) + + merged <- catalog_merge_dictionary( + dictionary, + relations, + NULL, + function(...) catalog_test_columns(), + "upper" + ) + table <- merged$dictionary$tables[["ANALYTICS.PUBLIC.ORDERS"]] + + expect_named(merged$dictionary$tables, "ANALYTICS.PUBLIC.ORDERS") + expect_equal(table$description, "Authored table description.") + expect_equal(table$kind, "table") + expect_named(table$columns, c("AMOUNT", "ORDER_ID", "missing")) + expect_equal(table$columns$AMOUNT$type, "NUMBER(38,2)") + expect_true(table$columns$AMOUNT$nullable) + expect_equal(table$columns$AMOUNT$units, "USD") + expect_equal( + table$columns$AMOUNT$description, + "Authored column description." + ) + expect_equal(table$columns$ORDER_ID$description, "Warehouse key.") + rendered <- dictionary_columns_text(table$columns, catalog_test_columns()) + expect_match(rendered, "AMOUNT (NUMBER(38,2), nullable, USD)", fixed = TRUE) + expect_no_match(rendered, "Warehouse amount description.", fixed = TRUE) + expect_equal( + merged$relations[["ANALYTICS.PUBLIC.ORDERS"]]$columns, + catalog_test_columns() + ) +}) + +test_that("fully qualified authored names match before relative names", { + relations <- list( + "ANALYTICS.PUBLIC.ORDERS" = list( + id = DBI::Id( + catalog = "ANALYTICS", + schema = "PUBLIC", + table = "ORDERS" + ), + kind = "table", + description = NULL + ), + "ANALYTICS.STAGING.ORDERS" = list( + id = DBI::Id( + catalog = "ANALYTICS", + schema = "STAGING", + table = "ORDERS" + ), + kind = "table", + description = NULL + ) + ) + dictionary <- new_data_dictionary(list( + tables = list( + "analytics.public.orders" = list(description = "Public orders.") + ) + )) + merged <- catalog_merge_dictionary( + dictionary, + relations, + NULL, + function(...) catalog_test_columns()[1, ], + "upper" + ) + + expect_named(merged$dictionary$tables, "ANALYTICS.PUBLIC.ORDERS") +}) + +test_that("relative authored table names must be unambiguous", { + relations <- lapply(c("PUBLIC", "STAGING"), function(schema) { + list( + id = DBI::Id( + catalog = "ANALYTICS", + schema = schema, + table = "ORDERS" + ), + kind = "table", + description = NULL + ) + }) + names(relations) <- c( + "ANALYTICS.PUBLIC.ORDERS", + "ANALYTICS.STAGING.ORDERS" + ) + dictionary <- new_data_dictionary(list( + tables = list(orders = list(description = "Orders.")) + )) + + expect_snapshot( + catalog_merge_dictionary( + dictionary, + relations, + NULL, + function(...) NULL, + "upper" + ), + error = TRUE + ) +}) + +test_that("discovered columns cannot shadow governed definitions", { + relations <- list( + "ANALYTICS.PUBLIC.ORDERS" = list( + id = DBI::Id( + catalog = "ANALYTICS", + schema = "PUBLIC", + table = "ORDERS" + ), + kind = "table", + description = NULL + ) + ) + dictionary <- new_data_dictionary(list( + tables = list(orders = list(definitions = list( + order_id = list(expr = "ORDER_ID", type = "number") + ))) + )) + + expect_snapshot( + catalog_merge_dictionary( + dictionary, + relations, + NULL, + function(...) catalog_test_columns(), + "upper" + ), + error = TRUE + ) +}) + test_that("a SQL query delivers a table's entry once", { skip_if_not_installed("yaml") src <- local_dict_source() diff --git a/tests/testthat/test-live-warehouses.R b/tests/testthat/test-live-warehouses.R index c9faf3d..28361b1 100644 --- a/tests/testthat/test-live-warehouses.R +++ b/tests/testthat/test-live-warehouses.R @@ -20,7 +20,9 @@ test_that("live Snowflake discovers and describes catalog relations", { names(session) <- tolower(names(session)) label <- table_id_label(table) - exact <- data_source(con, tables = table) + column <- names(rows)[[1]] + dictionary <- warehouse_test_dictionary(label, column) + exact <- data_source(con, tables = table, dictionary = dictionary) described <- source_describe(exact, label) namespace <- DBI::Id( @@ -62,6 +64,18 @@ test_that("live Snowflake discovers and describes catalog relations", { ) expect_true(nrow(described$sample) <= 5) expect_equal(names(described$sample), described$schema$column) + expect_equal( + exact$dictionary$tables[[label]]$description, + "Authored live table description." + ) + expect_equal( + exact$dictionary$tables[[label]]$columns[[column]]$type, + described$schema$type[match(column, described$schema$column)] + ) + expect_equal( + exact$dictionary$tables[[label]]$columns[[column]]$description, + "Authored live column description." + ) expect_true(exact$relations[[label]]$kind %in% c("table", "view")) expect_true(label %in% list_tables(schema_source)) expect_true(label %in% list_tables(catalog_source)) @@ -78,6 +92,46 @@ test_that("live Snowflake discovers and describes catalog relations", { expect_match(tool@value, "Sample summary") }) +test_that("live Snowflake rejects an ambiguous relative dictionary table", { + table <- warehouse_test_table("snowflake") + con <- local_warehouse_connection("snowflake") + catalog <- table@name[["catalog"]] + relations <- snowflake_list_relations( + con, + DBI::Id(catalog = catalog) + ) + relation_names <- vapply( + relations, + function(relation) relation$id@name[["table"]], + character(1) + ) + duplicated_names <- unique(relation_names[ + duplicated(relation_names) | duplicated(relation_names, fromLast = TRUE) + ]) + skip_if( + length(duplicated_names) == 0L, + "The selected Snowflake catalog has no ambiguous relation names" + ) + + authored_name <- duplicated_names[[1]] + selected <- relations[relation_names == authored_name][1:2] + dictionary <- new_data_dictionary(list( + tables = stats::setNames( + list(list(description = "Ambiguous authored description.")), + authored_name + ) + )) + + expect_error( + data_source( + con, + tables = lapply(selected, `[[`, "id"), + dictionary = dictionary + ), + "matches more than one selected relation" + ) +}) + test_that("live Databricks discovers and describes catalog relations", { table <- warehouse_test_table("databricks") con <- local_warehouse_connection("databricks") @@ -99,7 +153,12 @@ test_that("live Databricks discovers and describes catalog relations", { names(session) <- tolower(names(session)) label <- table_id_label(table) - exact <- data_source(con, tables = table) + column <- names(rows)[[1]] + dictionary <- warehouse_test_dictionary( + components[["table"]], + column + ) + exact <- data_source(con, tables = table, dictionary = dictionary) described <- source_describe(exact, label) namespace <- DBI::Id( @@ -127,6 +186,18 @@ test_that("live Databricks discovers and describes catalog relations", { expect_false(anyNA(described$schema$nullable)) expect_true(nrow(described$sample) <= 5) expect_equal(names(described$sample), described$schema$column) + expect_equal( + exact$dictionary$tables[[label]]$description, + "Authored live table description." + ) + expect_equal( + exact$dictionary$tables[[label]]$columns[[column]]$type, + described$schema$type[match(column, described$schema$column)] + ) + expect_equal( + exact$dictionary$tables[[label]]$columns[[column]]$description, + "Authored live column description." + ) expect_true(exact$relations[[label]]$kind %in% c("table", "view")) expect_true(label %in% list_tables(schema_source)) expect_true(label %in% list_tables(catalog_source)) From 17f2071209cc5b47b39e65a2ed33df8e116d3fb8 Mon Sep 17 00:00:00 2001 From: Simon Couch Date: Mon, 10 Aug 2026 16:01:58 -0500 Subject: [PATCH 2/3] tighten dictionary merge scope --- R/catalog.R | 34 +++++++++++++++++++++++++++ R/data-dictionary.R | 16 ++++++++----- R/definitions.R | 6 +++-- R/tools.R | 6 +++-- tests/testthat/test-data-dictionary.R | 18 ++++++++++++++ tests/testthat/test-definitions.R | 11 +++++++++ 6 files changed, 81 insertions(+), 10 deletions(-) diff --git a/R/catalog.R b/R/catalog.R index 8b533ea..098c8d5 100644 --- a/R/catalog.R +++ b/R/catalog.R @@ -113,6 +113,10 @@ catalog_merge_dictionary <- function( identifier_case, call = call ) + dictionary$relationships <- catalog_scope_dictionary_relationships( + dictionary, + matches + ) tables <- list() for (authored_name in names(matches)) { label <- matches[[authored_name]] @@ -138,6 +142,36 @@ catalog_merge_dictionary <- function( list(dictionary = dictionary, relations = relations) } +catalog_scope_dictionary_relationships <- function(dictionary, matches) { + dropped <- names(matches)[is.na(matches)] + if (length(dropped) == 0L) { + return(dictionary$relationships) + } + keep <- vapply( + dictionary$relationships, + catalog_dictionary_relationship_in_scope, + logical(1), + dropped = dropped, + dictionary = dictionary + ) + dictionary$relationships[keep] +} + +catalog_dictionary_relationship_in_scope <- function( + relationship, + dropped, + dictionary +) { + text <- paste(c(relationship$join, relationship$description), collapse = " ") + !any(vapply( + dropped, + dictionary_table_mentioned, + logical(1), + dictionary = dictionary, + text = text + )) +} + catalog_dictionary_matches <- function( dictionary, relations, diff --git a/R/data-dictionary.R b/R/data-dictionary.R index 4877aa6..17a018c 100644 --- a/R/data-dictionary.R +++ b/R/data-dictionary.R @@ -250,16 +250,11 @@ dictionary_examples_fact <- function(examples) { dictionary_relationships_text <- function(dictionary, table) { relationships <- dictionary$relationships - table_names <- c(table, dictionary$tables[[table]][[".authored_name"]]) mentions <- vapply( relationships, function(rel) { text <- paste(c(rel$join, rel$description), collapse = " ") - any(vapply( - table_names, - function(name) grepl(word_pattern(name), text, ignore.case = TRUE), - logical(1) - )) + dictionary_table_mentioned(table, dictionary, text) }, logical(1) ) @@ -288,6 +283,15 @@ dictionary_relationships_text <- function(dictionary, table) { paste0("Relationships:\n\n", paste(lines, collapse = "\n")) } +dictionary_table_mentioned <- function(table, dictionary, text) { + table_names <- c(table, dictionary$tables[[table]][[".authored_name"]]) + any(vapply( + table_names, + function(name) grepl(word_pattern(name), text, ignore.case = TRUE), + logical(1) + )) +} + # Definitions of glossary terms the entry references but the system prompt # doesn't already carry (i.e. terms past the ambient cap). dictionary_terms_text <- function(dictionary, text) { diff --git a/R/definitions.R b/R/definitions.R index f91986b..be479c6 100644 --- a/R/definitions.R +++ b/R/definitions.R @@ -242,7 +242,7 @@ definition_base_type <- function(type) { } definition_token_pattern <- - "\\{\\{\\s*([A-Za-z_][A-Za-z0-9_]*(?:\\.[A-Za-z_][A-Za-z0-9_]*)?)\\s*\\}\\}" + "\\{\\{\\s*([A-Za-z_][A-Za-z0-9_]*(?:\\.[A-Za-z_][A-Za-z0-9_]*)*)\\s*\\}\\}" # The expansion step of run_sql, before check_query() so the denylist sees # the SQL that will actually execute. @@ -294,7 +294,9 @@ resolve_definition_token <- function( ) { if (grepl(".", token, fixed = TRUE)) { parts <- strsplit(token, ".", fixed = TRUE)[[1]] - hits <- defs[defs$table == parts[[1]] & defs$name == parts[[2]], ] + name <- utils::tail(parts, 1L) + table <- paste(utils::head(parts, -1L), collapse = ".") + hits <- defs[defs$table == table & defs$name == name, ] if (nrow(hits) == 0) { abort_unknown_token(token, defs, call = call) } diff --git a/R/tools.R b/R/tools.R index 2c76ad9..0ea9e56 100644 --- a/R/tools.R +++ b/R/tools.R @@ -417,8 +417,10 @@ dictionary_sql_entries <- function(source, sql, source_name, tracker) { tables <- names(dictionary$tables) hits <- tables[vapply( tables, - function(table) grepl(word_pattern(table), sql, ignore.case = TRUE), - logical(1) + dictionary_table_mentioned, + logical(1), + dictionary = dictionary, + text = sql )] hits <- hits[!vapply( hits, diff --git a/tests/testthat/test-data-dictionary.R b/tests/testthat/test-data-dictionary.R index 59089b6..a4bef66 100644 --- a/tests/testthat/test-data-dictionary.R +++ b/tests/testthat/test-data-dictionary.R @@ -205,6 +205,10 @@ test_that("warehouse metadata supplements an authored dictionary", { ) ), unselected = list(description = "Not selected.") + ), + relationships = list( + list(join = "orders.order_id = unselected.order_id"), + list(join = "orders.order_id = external.order_id") ) )) @@ -236,6 +240,20 @@ test_that("warehouse metadata supplements an authored dictionary", { merged$relations[["ANALYTICS.PUBLIC.ORDERS"]]$columns, catalog_test_columns() ) + expect_length(merged$dictionary$relationships, 1L) + entry <- dictionary_entry_text( + merged$dictionary, + "ANALYTICS.PUBLIC.ORDERS" + ) + expect_match(entry, "external.order_id", fixed = TRUE) + expect_no_match(entry, "unselected.order_id", fixed = TRUE) + sql_entry <- dictionary_sql_entries( + list(dictionary = merged$dictionary), + "SELECT * FROM orders", + NULL, + NULL + ) + expect_match(sql_entry, "Authored table description.", fixed = TRUE) }) test_that("fully qualified authored names match before relative names", { diff --git a/tests/testthat/test-definitions.R b/tests/testthat/test-definitions.R index c341a30..9832f3a 100644 --- a/tests/testthat/test-definitions.R +++ b/tests/testthat/test-definitions.R @@ -227,6 +227,17 @@ test_that("same-named definitions on several tables disambiguate by scope", { records ) expect_match(qualified$sql, "revenue > 0", fixed = TRUE) + + qualified_records <- records[records$table == "sales", , drop = FALSE] + qualified_records$table <- "ANALYTICS.PUBLIC.SALES" + qualified <- expand_definitions( + paste( + "SELECT count(*) FROM ANALYTICS.PUBLIC.SALES", + "WHERE {{ANALYTICS.PUBLIC.SALES.deduplicated}}" + ), + qualified_records + ) + expect_match(qualified$sql, "revenue > 0", fixed = TRUE) }) test_that("run_sql results note the definitions applied", { From 4f46c25cae2efd63d6dd1cb3290d732b789a5978 Mon Sep 17 00:00:00 2001 From: Simon Couch Date: Mon, 10 Aug 2026 16:08:07 -0500 Subject: [PATCH 3/3] explain authored alias --- R/catalog.R | 1 + 1 file changed, 1 insertion(+) diff --git a/R/catalog.R b/R/catalog.R index 098c8d5..65f08f0 100644 --- a/R/catalog.R +++ b/R/catalog.R @@ -309,6 +309,7 @@ catalog_merge_dictionary_table <- function( call = call ) if (!identical(authored_name, selected_name)) { + # Preserve the authored alias for first-touch and relationship matching after re-keying. authored$.authored_name <- authored_name }