Skip to content

fix(shared): identify unique-ids in complex patterns#2290

Draft
markscott-ms wants to merge 4 commits intofinos:mainfrom
markscott-ms:fix/2230
Draft

fix(shared): identify unique-ids in complex patterns#2290
markscott-ms wants to merge 4 commits intofinos:mainfrom
markscott-ms:fix/2230

Conversation

@markscott-ms
Copy link
Copy Markdown
Contributor

Description

fixes #2230 - @aidanm3341 you didn't include an example failure on your bug report, and CALM validate correctly identifies duplicate unique-ids, assuming a pattern does not use oneOf or anyOf, so this is an educated (AI-assisted) guess at what your problem is. Look at the test cases to see if I've caught your actual issue.

This pull request updates the logic for checking uniqueness of unique-id values in pattern schemas, particularly improving support for nodes and relationships defined inside oneOf and anyOf blocks. The main change is that duplicate IDs are now only flagged if they occur outside of, or across, different choice groups—duplicate IDs within the same oneOf or anyOf block are allowed, since only one alternative will be instantiated. The test suite has been expanded to cover these scenarios.

Enhancements to unique-id detection logic:

  • Refactored ids-are-unique.ts to recursively detect all unique-id values, including those nested within oneOf and anyOf blocks, and to allow duplicates within the same choice group but not across different groups. Introduced a helper function getChoiceGroup to determine group membership by JSON pointer.

Test coverage improvements:

  • Added multiple test cases in ids-are-unique.spec.ts to verify correct handling of duplicate IDs inside and across anyOf/oneOf blocks, including:
    • Duplicates between regular nodes and those inside anyOf/oneOf
    • No error for duplicates within the same choice group
    • Errors for duplicates across different groups

Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 📚 Documentation update
  • 🎨 Code style/formatting changes
  • ♻️ Refactoring (no functional changes)
  • ⚡ Performance improvements
  • ✅ Test additions or updates
  • 🔧 Chore (maintenance, dependencies, CI, etc.)

Affected Components

  • CLI (cli/)
  • Schema (calm/)
  • CALM AI (calm-ai/)
  • CALM Hub (calm-hub/)
  • CALM Hub UI (calm-hub-ui/)
  • CALM Server (calm-server/)
  • CALM Widgets (calm-widgets/)
  • Documentation (docs/)
  • Shared (shared/)
  • VS Code Extension (calm-plugins/vscode/)
  • Dependencies
  • CI/CD

Commit Message Format ✅

Testing

  • I have tested my changes locally
  • I have added/updated unit tests
  • All existing tests pass

Checklist

  • My commits follow the conventional commit format
  • I have updated documentation if necessary
  • I have added tests for my changes (if applicable)
  • My changes follow the project's coding standards

@jpgough-ms
Copy link
Copy Markdown
Member

@markscott-ms although code is cheap now, design is expensive. I think confirming the question on the issue before implementation is a better approach than spamming in PRs.

@markscott-ms markscott-ms marked this pull request as draft April 2, 2026 14:29
@markscott-ms
Copy link
Copy Markdown
Contributor Author

markscott-ms commented Apr 3, 2026

After discussion with @niamhg-ms I'm reviewing the change as unique-id really must be unique otherwise option relationships can't select a node/relationship to include.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Gap in CALM Validate for Patterns

2 participants