Skip to content

Refactor SchemaMapResolver::add to use reidentify() function#33

Closed
staging-devin-ai-integration[bot] wants to merge 1 commit intomainfrom
core-refac-413e999b
Closed

Refactor SchemaMapResolver::add to use reidentify() function#33
staging-devin-ai-integration[bot] wants to merge 1 commit intomainfrom
core-refac-413e999b

Conversation

@staging-devin-ai-integration
Copy link
Copy Markdown

Refactor SchemaMapResolver::add to use reidentify() function

Summary

Replaced manual ID assignment logic in SchemaMapResolver::add with a call to the existing reidentify() function to eliminate code duplication. The original code manually checked vocabulary URIs to determine whether to use "id" or "$id" as the identifier keyword, but this logic was already implemented in the reidentify() utility function.

Changes:

  • Removed 15 lines of manual vocabulary checking and ID assignment
  • Replaced with single call to sourcemeta::core::reidentify(subschema, key.second, *this, entry.dialect)
  • Eliminated TODO comment about de-duplication since the logic is no longer duplicated

Review & Testing Checklist for Human

  • Verify ID keyword assignment across schema drafts: Test that schemas from different JSON Schema drafts (draft-00 through draft-04 vs. newer drafts) correctly get assigned "id" vs "$id" keywords respectively
  • Validate parameter mapping: Confirm that passing entry.dialect as the default_dialect parameter to reidentify() produces the same behavior as the original vocabulary-based logic
  • Test edge cases: Verify behavior with schemas that have ambiguous or missing dialect information

Notes

The refactoring relies on the assumption that entry.dialect provides equivalent information to the original subschema_vocabularies.contains() checks. While all existing tests pass, the subtle difference in dialect determination approach warrants careful testing with various schema draft versions.

Link to Devin run: https://staging.itsdev.in/sessions/401ea5f3a27a40a3a9d2e80079a3f984
Requested by: @jviotti

Replace manual ID assignment logic with existing reidentify() function
to eliminate code duplication. The reidentify() function handles
dialect-specific ID keyword assignment ('id' vs '$id') automatically.

- Removed TODO comment about de-duplication from bundle.cc
- Replaced manual vocabulary checking with reidentify() call
- Maintains same functionality while eliminating duplicate logic

Co-Authored-By: Juan Cruz Viotti <jv@jviotti.com>
@staging-devin-ai-integration
Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@staging-devin-ai-integration
Copy link
Copy Markdown
Author

Closing due to inactivity for more than 7 days. Configure here.

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.

0 participants