Skip to content

Fix UUID pattern anchoring#1116

Open
fallintoplace wants to merge 1 commit into
NVIDIA:mainfrom
fallintoplace:fix/uuid-pattern-anchors
Open

Fix UUID pattern anchoring#1116
fallintoplace wants to merge 1 commit into
NVIDIA:mainfrom
fallintoplace:fix/uuid-pattern-anchors

Conversation

@fallintoplace

@fallintoplace fallintoplace commented Jun 20, 2026

Copy link
Copy Markdown

Summary

Fix UuidPattern so all supported UUID formats are validated as complete strings.

Details

The previous pattern anchored only the first and last alternatives because regex alternation has lower precedence than the anchors. That allowed partial strings to validate when they started with a 32-character UUID, contained an old-format UUID substring, or ended with a group UUID.

This wraps the alternatives in a non-capturing group under one start/end anchor pair. The existing lowercase-only UUID formats remain unchanged.

Validation

  • /Users/hoangvu/go/bin/bazelisk test //src/lib/utils/tests:test_common
  • /Users/hoangvu/go/bin/bazelisk test //src/lib/utils:common-pylint //src/lib/utils/tests:test_common-pylint

Summary by CodeRabbit

  • Bug Fixes

    • Improved UUID validation to correctly enforce supported formats and prevent partial matches from being accepted
  • Tests

    • Added comprehensive validation test coverage for UUID format acceptance and rejection rules

@fallintoplace fallintoplace requested a review from a team as a code owner June 20, 2026 12:18
@github-actions github-actions Bot added the external The author is not in @NVIDIA/osmo-dev label Jun 20, 2026
@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 412dde06-1840-493f-8c94-88fa904b2d6f

📥 Commits

Reviewing files that changed from the base of the PR and between cc7e0bf and 269ae95.

📒 Files selected for processing (2)
  • src/lib/utils/common.py
  • src/lib/utils/tests/test_common.py

📝 Walkthrough

Walkthrough

The UuidPattern pydantic field regex in common.py is corrected to wrap the three UUID format alternatives in a non-capturing group with full-string anchors. A new TestUuidPattern test class is added with two methods covering accepted UUID formats and rejected partial/uppercase inputs.

Changes

UuidPattern Regex Fix and Tests

Layer / File(s) Summary
UuidPattern regex fix and validation tests
src/lib/utils/common.py, src/lib/utils/tests/test_common.py
Regex changed from ^A|B|C$ to ^(?:A|B|C)$ so anchors apply to the full alternation; TestUuidPattern verifies accepted formats (osmo- prefix, 32-char hex, alphabetic form) and asserts pydantic.ValidationError for partial, uppercase, or otherwise invalid inputs.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Poem

A regex once wandered, unanchored and free,
Its alternation split like branches on a tree.
A non-capturing group now gathers them tight,
^(?:A|B|C)$ anchors left and right.
The tests hop along and confirm what is true—
🐇 Only valid UUIDs shall pass right through!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fix UUID pattern anchoring' is a concise, specific description that clearly summarizes the main change in the PR—correcting regex anchor placement in UUID validation.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

external The author is not in @NVIDIA/osmo-dev

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant