validate: correctness, perf cleanup, and polish from code review

[jqnatividad]

Summary

A multi-pass review of src/cmd/validate.rs (~2,650 lines) surfaced a mix of correctness bugs, perf-leaning cleanups, and polish items. This PR lands all three groups as separate commits so each can be read and reverted independently.

Commit 1 — fix(validate): correctness fixes in JSON Schema validation

• UniqueCombinedWithValidator::is_valid always returns false, forcing callers through validate() (which atomically check-and-inserts under lock). The previous impl read seen_combinations but never inserted, so any caller invoking Validator::is_valid() against a uniqueCombinedWith schema would let duplicates pass silently.
• Substring schema-feature detection replaced with a tree walker (detect_custom_schema_features). The substring approach was whitespace-sensitive ("format" : "currency" silently disabled the custom format) and could false-match on description/title text containing literal keyword names. The tree walk runs once after the schema is parsed.
• load_json now checks response.status().is_success() and propagates response.text() errors instead of .unwrap_or_default() flowing empty bodies into the JSON parser as confusing parse errors.
• Email options apply display_text and domain_literal toggles explicitly in both directions (no more reliance on jsonschema crate defaults that may flip in future versions).
• valid_flags is now extended by batch.len() rather than batch_size, so the last partial batch doesn't leave trailing valid flags for nonexistent rows.
• Documented the implicit serde_json/preserve_order dependency at the uniqueCombinedWith index-resolution site.
• Updated validate_schema_subcommand_with_invalid_url_schema to accept the new "HTTP error" substring.

Commit 2 — refactor(validate): perf and cleanup tweaks

• UniqueCombinedWithValidator: RwLock<HashSet> → Mutex<HashSet> (validate path always takes the write lock; no read-only path benefits from RwLock).
• BitVec::set_unchecked (unsafe) → BitVec::set (safe). This path is dominated by validator work, so the micro-opt is irrelevant.
• Documented the row-number-as-extra-ByteRecord-field convention used to pass the row index into the parallel closure.
• Added a doc comment on split_invalid_records explaining the intentional re-read (page cache + all-valid hot path) instead of streaming.
• Extracted a shared load_dynenum_set helper; lite and non-lite factories now delegate column resolution + drain to one function instead of duplicating ~60 lines.

Commit 3 — chore(validate): polish — re-entrant safety, arg checks, test hygiene

• OnceLock::set(...).unwrap() / ? replaced with get_or_init / let _ = .set() for NULL_TYPE, DELIMITER, QSV_CACHE_DIR, CKAN_API, and CKAN_TOKEN. The previous code panics or returns Err on re-entry (e.g. qsv used as a library, or under qsvmcp).
• flag_no_headers rejection moved earlier in run() so we don't open a CSV reader only to immediately drop it on user error.
• Warn when a .json file appears earlier in the input list — schema detection only inspects the last positional, so a misordered argument silently fell into RFC 4180 mode.
• test_load_json_via_url marked #[ignore] so CI/offline runs are deterministic; run with --ignored to exercise it.

Test plan

• cargo build --bin qsv -F all_features
• cargo build --bin qsvlite -F lite
• cargo clippy --bin qsv -F all_features -- -D warnings (clean)
• cargo test -F all_features → 2715 passed, 0 failed, 29 ignored
• Each commit independently builds and passes cargo test validate -F all_features

🤖 Generated with Claude Code

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 12 complexity

Metric	Results
Complexity	12

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[Copilot]

Pull request overview

This PR improves the validate command’s JSON Schema validation by fixing correctness issues (notably around stateful uniqueCombinedWith), tightening network/file error handling for schema loading, and applying a set of performance/maintainability cleanups.

Changes:

• Fix uniqueCombinedWith correctness and concurrency behavior; improve schema feature detection via a parsed-schema tree walk.
• Refactor/cleanup validation hot-path code (batch flag handling, removal of unchecked bitvec writes, shared dynamicEnum loader).
• Harden operational behavior (re-entrant OnceLock usage, earlier arg rejection, deterministic test behavior).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
src/cmd/validate.rs	Correctness fixes for custom validators, safer schema feature detection, error-handling improvements, and perf/cleanup refactors.
tests/test_validate.rs	Adjust test to accept the new HTTP error wording when schema fetch returns a non-success status.