Skip to content

Conversation

@ahouseholder
Copy link
Contributor

@ahouseholder ahouseholder commented Sep 15, 2025

This PR replaces the compiled NS_PATTERN with the pattern string when creating the field specs.

A side effect of this change is that the pattern no longer needs the lookahead for length check, as we're specifying as part of the pydantic class and that carries into the JSON schema directly without it having to be part of the regex.

Copilot Summary

This pull request simplifies and standardizes the namespace pattern validation logic across several schema and utility files. The main change is the removal of an unnecessary length check from the regular expression pattern, relying instead on explicit minLength and maxLength constraints. This makes the regex easier to maintain and ensures consistency between schema validation and Python-side checks.

Most important changes:

Schema pattern updates:

  • Removed the inline length check ((?=.{3,1000}$)) from the pattern field in the JSON schema definitions for DecisionPoint_2_0_0.schema.json, DecisionPointGroup_2_0_0.schema.json, DecisionTable_2_0_0.schema.json, SelectionList_2_0_0.schema.json, and SsvcObjectRegistry_2_0_0.schema.json, so that the regex only enforces the namespace format, not its length. Length is now enforced solely by the minLength and maxLength schema properties. [1] [2] [3] [4] [5] [6] [7]

Python utility and test consistency:

  • Updated the namespace regex in src/ssvc/utils/patterns.py to remove the length check from the pattern string, matching the schema change.
  • Changed src/ssvc/utils/field_specs.py to use the raw pattern string from the compiled regex, ensuring consistency between the schema and Python validation.
  • Improved the namespace pattern unit test in src/test/test_namespaces.py to accumulate and report all unexpected matches, making debugging easier.

… the compiled pattern.

This required removing the lookahead portion of the string, but because we're using min_length and max_length in the spec it shouldn't matter.
@ahouseholder ahouseholder self-assigned this Sep 15, 2025
@ahouseholder ahouseholder added tech/backend Back-end tools, code, infrastructure tech/data Data implementation (content of /data, data object instances, etc.) labels Sep 15, 2025
@ahouseholder ahouseholder added this to the 2025-09 milestone Sep 15, 2025
Copy link
Contributor

@sei-vsarvepalli sei-vsarvepalli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well.. the tests are running okay. This regex is one I cannot read :)

@ahouseholder
Copy link
Contributor Author

ahouseholder commented Sep 16, 2025

The only change to the regex is that we're no longer checking min/max length within the pattern itself. This is okay because both pydantic and json schema have length enforcement via first-order properties that we're already using. So the pattern doesn't need to care about length anyway. This was discovered because when I changed NS_PATTERN to NS_PATTERN.pattern pydantic complained that the Rust-based regex interpreter doesn't do lookahead. Removing the lookahead from the pattern and relying on the code to enforce length requirements made sense here.

Also, this was the only place where we were using a compiled pattern in creating a pydantic object. Other places where we used patterns we were always including the pattern string instead, so this makes us consistent throughout.

@ahouseholder ahouseholder merged commit ba10598 into main Sep 16, 2025
4 checks passed
@ahouseholder ahouseholder deleted the 916_followup branch September 16, 2025 13:56
@bernhardreiter
Copy link
Contributor

What about using NS_PATTERN_STR instead of NS_PATTERN.pattern and remove NS_PATTERN entirely? It would be an additional simplification.

This regex is one I cannot read :)

If you look at the different parts of the regex in pattern.py, that is close to the ABNF and can be understood. The one in NS_PATTERN_STR is just unrolled. ;)

@sei-vsarvepalli
Copy link
Contributor

What about using NS_PATTERN_STR instead of NS_PATTERN.pattern and remove NS_PATTERN entirely? It would be an additional simplification.

This regex is one I cannot read :)

If you look at the different parts of the regex in pattern.py, that is close to the ABNF and can be understood. The one in NS_PATTERN_STR is just unrolled. ;)

Hi Bernhard - I think ABNF is helpful. We are not pursuing much more update on it at this time.

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

Labels

tech/backend Back-end tools, code, infrastructure tech/data Data implementation (content of /data, data object instances, etc.)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use pattern string instead of compiled pattern in field_spec.py

4 participants