fix(search): soft-fail on per-row excluded type and malformed regex name
Cross-task review found two edge cases that violated the basket-mode soft-fail contract: - Per-row excluded type (e.g. type = c(NA, "special_district")) hit .coerce_type()'s abort inside the per-row resolver, killing the whole basket call. Now treated as no_match in the sidecar. - Malformed regex in the substring fallback (e.g. name = "San(Diego") propagated DuckDB engine errors. .escape_regex() now backslash- escapes meta characters before the regexp_matches call. Utility- mode regex behavior is unchanged. Plus a new public-surface test for the all-no-match case.
This commit is contained in:
+4
-1
@@ -39,9 +39,12 @@
|
|||||||
}
|
}
|
||||||
|
|
||||||
# Convert a type input (integer-like or label) into the canonical label
|
# Convert a type input (integer-like or label) into the canonical label
|
||||||
# string used in the sidecar query_type column.
|
# string used in the sidecar query_type column. Excluded types (4/5 /
|
||||||
|
# special_district / school_district) are returned as-is so the sidecar
|
||||||
|
# records what the user passed without calling .coerce_type() (which aborts).
|
||||||
#' @noRd
|
#' @noRd
|
||||||
.type_to_label <- function(type) {
|
.type_to_label <- function(type) {
|
||||||
|
if (.is_excluded_type(type)) return(as.character(type))
|
||||||
int_type <- .coerce_type(type)
|
int_type <- .coerce_type(type)
|
||||||
unname(c("0" = "state", "1" = "county", "2" = "city", "3" = "township")[[as.character(int_type)]])
|
unname(c("0" = "state", "1" = "county", "2" = "city", "3" = "township")[[as.character(int_type)]])
|
||||||
}
|
}
|
||||||
|
|||||||
+22
-1
@@ -132,6 +132,14 @@ cog_gov_search <- function(name = NULL, state = NULL, type = NULL) {
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#' @noRd
|
||||||
|
.escape_regex <- function(x) {
|
||||||
|
# Backslash-escape POSIX regex metacharacters so `name` is treated as a
|
||||||
|
# literal substring in the DuckDB regexp_matches call (substring fallback
|
||||||
|
# only; utility-mode intentionally preserves regex behavior).
|
||||||
|
gsub("([\\^$.|?*+(){}\\[\\]])", "\\\\\\1", x, perl = TRUE)
|
||||||
|
}
|
||||||
|
|
||||||
#' @noRd
|
#' @noRd
|
||||||
.is_excluded_type <- function(type) {
|
.is_excluded_type <- function(type) {
|
||||||
excluded <- c("4", "5", "special_district", "school_district")
|
excluded <- c("4", "5", "special_district", "school_district")
|
||||||
@@ -245,6 +253,19 @@ cog_gov_search <- function(name = NULL, state = NULL, type = NULL) {
|
|||||||
))
|
))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
# Short-circuit: excluded type (4/5 / special_district / school_district)
|
||||||
|
# -> no_match without SQL, preserving soft-fail contract.
|
||||||
|
if (!is.na(type) && .is_excluded_type(type)) {
|
||||||
|
empty <- .empty_xwalk_tibble()
|
||||||
|
return(list(
|
||||||
|
status = "no_match",
|
||||||
|
match_method = NA_character_,
|
||||||
|
n_candidates = 0L,
|
||||||
|
row = empty,
|
||||||
|
candidates = empty
|
||||||
|
))
|
||||||
|
}
|
||||||
|
|
||||||
preds <- character(0)
|
preds <- character(0)
|
||||||
if (!is.na(state)) {
|
if (!is.na(state)) {
|
||||||
st_fips <- .coerce_state_to_fips(state)
|
st_fips <- .coerce_state_to_fips(state)
|
||||||
@@ -282,7 +303,7 @@ cog_gov_search <- function(name = NULL, state = NULL, type = NULL) {
|
|||||||
"SELECT * FROM canonical_fips_xwalk",
|
"SELECT * FROM canonical_fips_xwalk",
|
||||||
base_where,
|
base_where,
|
||||||
conj,
|
conj,
|
||||||
sprintf("regexp_matches(gov_name, %s, 'i')", .sql_lit_chr(name))
|
sprintf("regexp_matches(gov_name, %s, 'i')", .sql_lit_chr(.escape_regex(name)))
|
||||||
)
|
)
|
||||||
sub <- tibble::as_tibble(DBI::dbGetQuery(con, sub_sql))
|
sub <- tibble::as_tibble(DBI::dbGetQuery(con, sub_sql))
|
||||||
|
|
||||||
|
|||||||
@@ -375,3 +375,63 @@ test_that("cog_gov_search basket mode message points to the sidecar accessor", {
|
|||||||
regexp = "cog_basket_resolution"
|
regexp = "cog_basket_resolution"
|
||||||
)
|
)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
# ---- F1: per-row excluded type soft-fail ----
|
||||||
|
|
||||||
|
test_that(".resolve_basket_row treats excluded type as no_match (not abort)", {
|
||||||
|
con <- uscogdata:::.ensure_session()
|
||||||
|
out <- uscogdata:::.resolve_basket_row(
|
||||||
|
name = "Some District", state = "CA", type = "special_district", con = con
|
||||||
|
)
|
||||||
|
expect_equal(out$status, "no_match")
|
||||||
|
expect_true(is.na(out$match_method))
|
||||||
|
expect_equal(out$n_candidates, 0L)
|
||||||
|
})
|
||||||
|
|
||||||
|
test_that("cog_gov_search basket mode skips per-row excluded type without aborting", {
|
||||||
|
basket <- suppressMessages(cog_gov_search(
|
||||||
|
name = c("BROWARD COUNTY", "Some District"),
|
||||||
|
state = c("FL", "FL"),
|
||||||
|
type = c(NA, "special_district")
|
||||||
|
))
|
||||||
|
# Broward should resolve; the special_district row should be no_match.
|
||||||
|
expect_equal(nrow(basket), 1L)
|
||||||
|
expect_equal(basket$canonical_govid, "101006006")
|
||||||
|
res <- attr(basket, "resolution")
|
||||||
|
expect_equal(res$status, c("resolved", "no_match"))
|
||||||
|
# query_type should record what the user passed for the excluded-type row
|
||||||
|
expect_equal(res$query_type, c(NA_character_, "special_district"))
|
||||||
|
})
|
||||||
|
|
||||||
|
# ---- F2: malformed regex name soft-fail ----
|
||||||
|
|
||||||
|
test_that(".resolve_basket_row treats malformed regex name as no_match", {
|
||||||
|
con <- uscogdata:::.ensure_session()
|
||||||
|
# Unbalanced parens would be a regex parse error if not escaped.
|
||||||
|
out <- uscogdata:::.resolve_basket_row(
|
||||||
|
name = "San(Diego", state = "CA", type = NA_character_, con = con
|
||||||
|
)
|
||||||
|
expect_equal(out$status, "no_match")
|
||||||
|
})
|
||||||
|
|
||||||
|
test_that(".resolve_basket_row escapes regex metacharacters in name", {
|
||||||
|
con <- uscogdata:::.ensure_session()
|
||||||
|
# Confirm that names with various metacharacters don't error.
|
||||||
|
expect_no_error(uscogdata:::.resolve_basket_row(
|
||||||
|
name = "Foo*Bar+Baz", state = "FL", type = NA_character_, con = con
|
||||||
|
))
|
||||||
|
})
|
||||||
|
|
||||||
|
# ---- F3: all-no-match basket public surface ----
|
||||||
|
|
||||||
|
test_that("cog_gov_search basket all-no-match returns 0-row tibble with full sidecar", {
|
||||||
|
basket <- suppressMessages(cog_gov_search(
|
||||||
|
name = c("Notarealplace1", "Notarealplace2"),
|
||||||
|
state = c("NY", "CA")
|
||||||
|
))
|
||||||
|
expect_equal(nrow(basket), 0L)
|
||||||
|
expect_true("canonical_govid" %in% names(basket))
|
||||||
|
res <- attr(basket, "resolution")
|
||||||
|
expect_equal(nrow(res), 2L)
|
||||||
|
expect_true(all(res$status == "no_match"))
|
||||||
|
})
|
||||||
|
|||||||
Reference in New Issue
Block a user