Repository navigation
Conversation
The valid_uuid audit only allowed version digits 1-5, so well-formed UUIDv6, UUIDv7 and UUIDv8 values (RFC 9562) were reported as invalid. Widen the version digit to 1-8 and update the documented pattern. Signed-off-by: Dhruv Gupta <todhruvg@gmail.com>
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
UUID version acceptance and verification sqlmesh/core/audit/builtin.py, tests/core/test_audit.py, docs/concepts/audits.md |
The audit pattern accepts UUID version digits 1–8. The test checks UUIDs from versions 1–8 and invalid inputs. The documentation shows the updated range. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Medium
Merge Risk: 
2c4e1
-
The UUID audit appears ready to merge after normal checks; no actionable merge risk remains.
Security architecture risk: 2c4e1
-
The change accepts newer standard UUID versions while preserving the existing format checks. The reviewed change introduces no material security risk, new privileges, or externally reachable endpoint.
Retained concerns
No architecture-level concerns identified.Security review details
-
Security Blast Radius
- inferred — The effective scope is models configured to use this built-in audit. Because model audits are blocking by default, newly accepted UUID versions can avoid an audit failure that previously stopped pipeline execution. Deployment-specific tenant scope and downstream security reliance are not established.
Trust Boundaries and Controls
- observed — The flagged public entrypoint is test_valid_uuid_audit, and exp is a SQL expression import used by the test. Neither constitutes a newly exposed production endpoint. The production predicate retains its existing format and variant controls.
-
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within survi… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description check The description explains the change, links it to issue #6167, documents the test plan, reports validation results, and identifies unrelated pyodbc failures. It also records that make fast-test was not…Title check The title clearly and concisely identifies the main change: allowing UUID versions 6–8 in the valid_uuid audit. Linked Issues check Issue #6167requires the built-invalid_uuidaudit to accept well-formed UUID versions 1–8.sqlmesh/core/audit/builtin.pynow uses[1-8]for the version digit and keeps the[89ab]variant chec…Out of Scope Changes check The reported changes are limited to the valid_uuidaudit pattern, its documentation, and automated coverage for the requested behavior. These changes directly support issue#6167. No unrelated chang…Full details: Description check
-
Explanation
The description explains the change, links it to issue
#6167, documents the test plan, reports validation results, and identifies unrelated pyodbc failures. It also records that make fast-test was not completed.
Full details: Linked Issues check
-
Explanation
Issue
#6167requires the built-invalid_uuidaudit to accept well-formed UUID versions 1–8.sqlmesh/core/audit/builtin.pynow uses[1-8]for the version digit and keeps the[89ab]variant check and remaining pattern unchanged. The addedtest_valid_uuid_auditcovers UUID versions 1–8, uppercase input, invalid variants, malformed values, andNULL. The documentation update records the same pattern change.
Full details: Out of Scope Changes check
-
Explanation
The reported changes are limited to the
valid_uuidaudit pattern, its documentation, and automated coverage for the requested behavior. These changes directly support issue#6167. No unrelated change is identified in the supplied pull request evidence.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 2 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
Description
Fixes #6167
The
valid_uuidaudit only allowed version digits1-5, so well-formed UUIDv6, UUIDv7 and UUIDv8 values (RFC 9562) failed the audit. UUIDv7 in particular is now a common key format (uuidv7()in DuckDB and PostgreSQL 18, many application libraries).This widens the version digit in the pattern to
1-8and updates the pattern shown in the audits docs. The variant check ([89ab]) and the rest of the pattern are unchanged.Models that use
valid_uuidwill see the audit change as a metadata change on their next plan, since the built-in audit query changed.Test Plan
test_valid_uuid_auditintests/core/test_audit.py. It checks the rendered query and runs it on DuckDB against the RFC 9562 example UUIDs of versions 1-8 (one upper case), plus values with an unknown version, a non-RFC variant, no hyphens, a non-UUID string and NULL. Only the invalid values are returned. The test fails onmainand passes with this change.pytest tests/core/test_audit.py tests/core/test_model.py tests/core/test_snapshot.py tests/core/test_context.py -n 8 -m "fast and not cicdonly and not isolated": 611 passed.pytest -n auto -m "fast and not cicdonly and not isolated": 2695 passed, 6 skipped, 6 failed. The 6 failures are the pyodbc connection-string tests intests/core/test_connection_config.py, which cannot import pyodbc locally because unixODBC is not installed; they are unrelated to this change.SKIP=prettier,eslint pre-commit run --all-files(ruff, ruff-format, mypy, valid migrations): all passed.Checklist
make styleand fixed any issuesmake fast-test)git commit -s) per the DCOSummary by CodeRabbit