Skip to content

To try and resolve #1233 - #1249

Merged
sei-vsarvepalli merged 4 commits into
CERTCC:mainfrom
sei-vsarvepalli:fix-1233
Sep 17, 2026
Merged

sei-vsarvepalli merged 4 commits into
CERTCC:mainfrom
sei-vsarvepalli:fix-1233

Conversation

@sei-vsarvepalli

@sei-vsarvepalli sei-vsarvepalli commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

This pull request updates the namespace extension pattern logic and its documentation to clarify the handling of language extensions, especially around empty language extensions and their valid forms. It also adjusts tests and workflow configuration for better correctness and coverage. The most important changes are outlined below:

Namespace extension pattern logic and documentation:

  • Updated the ABNF (ssvc_namespace_pattern.abnf) and Python regex (namespace_patterns.py) to clarify and correct the rules for namespace extensions, distinguishing between empty language extensions and BCP-47 language codes, and specifying the valid forms for extension segments. [1] [2]

Test coverage improvements:

  • Modified test cases in test_namespaces_pattern.py to remove an invalid case (x_example.test#test/) and add an invalid case (ssvc/) for empty language extensions, ensuring the tests align with the updated pattern rules. [1] [2]

Workflow configuration:

  • Updated the GitHub Actions workflow (run_doctools.yml) to check out the correct repository and commit SHA for pull requests, improving CI reliability and correctness.

  change ABNF to disallow things like `ssvc/`, update the python pattern
  and the test cases accordingly.

  use a simplified ABNF rule for readablity,
  because `[ 1*elem ]` equals `*elem` and whitespace after braces is not
  necessary (and not always used in the file).

resolve CERTCC#1233
@sei-vsarvepalli
sei-vsarvepalli marked this pull request as ready for review September 17, 2026 15:47
Copilot AI lite review requested due to automatic review settings September 17, 2026 15:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The workflow checkout configuration can prevent doctools updates from being pushed, and the documented ssvc/ behavior is inconsistent with its test.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR tightens namespace extension validation, regenerates related schemas and regexes, updates tests, and adjusts the doctools workflow.

Changes:

  • Clarifies namespace extension grammar and validation.
  • Updates generated schemas, registry data, and tests.
  • Revises workflow checkout and action configuration.
File summaries
File Summary
src/test/test_namespaces_pattern.py Tests bare ssvc/ as invalid; description requires clarification.
src/ssvc/utils/ssvc_namespace_pattern.abnf Updates namespace extension grammar.
src/ssvc/utils/namespace_patterns.py Updates generated namespace regex logic.
data/schema/v2/SsvcObjectRegistry_2_0_0.schema.json Updates generated namespace validation patterns.
data/schema/v2/SelectionList_2_0_0.schema.json Updates generated namespace validation patterns.
data/schema/v2/DecisionTable_2_0_0.schema.json Updates generated namespace validation patterns.
data/schema/v2/DecisionPointGroup_2_0_0.schema.json Updates generated namespace validation patterns.
data/schema/v2/DecisionPoint_2_0_0.schema.json Updates generated namespace validation patterns.
data/json/ssvc_object_registry.json Refreshes registry metadata.
.github/workflows/run_doctools.yml Checkout ref must preserve the PR head branch for subsequent pushes.
Review details
  • Files reviewed: 10/10 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/workflows/run_doctools.yml
Comment thread src/test/test_namespaces_pattern.py

@ahouseholder ahouseholder left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Assuming and/or based on the "expect fail ssvc/ test passing, I'm okay with this change. The trailing slash was unintended.

@sei-vsarvepalli
sei-vsarvepalli merged commit fa6a3a4 into CERTCC:main Sep 17, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants