Skip to content

fix(ers): marshal/unmarshal metadata correctly.#3672

Merged
c-r33d merged 7 commits into
mainfrom
multi-ers-sql
Jul 24, 2026
Merged

fix(ers): marshal/unmarshal metadata correctly.#3672
c-r33d merged 7 commits into
mainfrom
multi-ers-sql

Conversation

@c-r33d

@c-r33d c-r33d commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Proposed Changes

Checklist

  • I have added or updated unit tests
  • I have added or updated integration tests (if appropriate)
  • I have added or updated documentation

Testing Instructions

Summary by CodeRabbit

  • Bug Fixes

    • Entity resolution results now consistently normalize claim data, including nested values, arrays, maps, and numeric types.
    • Invalid claim values are safely skipped with error handling instead of producing malformed results.
  • Tests

    • Added coverage for claim normalization, empty claims, complex values, and unsupported data types.

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the entity resolution service by adding support for the pgx PostgreSQL driver and implementing a robust normalization mechanism for struct values. These changes ensure better compatibility with database drivers and improve data integrity when processing claims and metadata.

Highlights

  • Database Driver Support: Added the pgx driver for PostgreSQL to the SQL provider, enabling improved database connectivity.
  • Data Normalization: Introduced a normalization utility to ensure consistent handling of struct values during entity resolution, specifically addressing claim and metadata processing.
New Features

🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.


The driver is set for the pgx, To keep all the data in flux. With values now clean, And logic serene, We've squashed all the nasty old bugs.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 013091ad-ac67-47ae-a29d-677c622c8a4a

📥 Commits

Reviewing files that changed from the base of the PR and between 2f78ea9 and dc27adf.

⛔ Files ignored due to path filters (1)
  • service/go.sum is excluded by !**/*.sum
📒 Files selected for processing (2)
  • service/entityresolution/multi-strategy/v2/registration_test.go
  • service/go.mod

📝 Walkthrough

Walkthrough

Resolved entity claims are normalized through JSON conversion before entity representations are created. Conversion failures are logged and skipped. Tests cover nil, nested, collection, numeric, and unsupported claim values, while the SQLite dependency is upgraded.

Changes

Resolved claim normalization

Layer / File(s) Summary
Normalize claims before entity creation
service/entityresolution/multi-strategy/v2/registration.go, service/entityresolution/multi-strategy/v2/registration_test.go
ResolveEntities converts claims to JSON-compatible values, logs conversion errors, and skips affected entities. Tests cover normalized values and marshal failures.

SQLite dependency update

Layer / File(s) Summary
Update SQLite dependency
service/go.mod
The direct github.com/mattn/go-sqlite3 requirement is updated from v1.14.29 to v1.14.48.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: jakedoublev

Poem

A rabbit hops through claims so neat,
Turning nested maps to treats.
Bad channels make the bunny pause,
Good JSON follows all the laws.
SQLite gets a newer carrot too!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: fixing metadata marshal/unmarshal handling.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch multi-ers-sql

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request replaces the legacy PostgreSQL driver with the pgx driver in the SQL provider and introduces a normalizeStructValue helper to normalize claims and metadata before protobuf conversion. Feedback highlights that removing the postgres driver import will cause runtime failures for existing configurations unless postgres is mapped to pgx during initialization. Additionally, the normalizeStructValue implementation is limited and could cause protobuf conversion failures for unsupported slice or map types; using a JSON round-trip is recommended for robust normalization.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread service/entityresolution/multi-strategy/v2/registration.go Outdated
Comment thread service/entityresolution/multi-strategy/providers/sql/sql_provider.go Outdated
@github-actions

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 208.958049ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 102.080155ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 421.986007ms
Throughput 236.97 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 45.438862476s
Average Latency 453.041274ms
Throughput 110.04 requests/second

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Deprecation Warning: The deny-licenses option is deprecated for possible removal in the next major release. For more information, see issue 997.

Dependency Review

The following issues were found:
  • ❌ 1 vulnerable package(s)
  • ✅ 0 package(s) with incompatible licenses
  • ✅ 0 package(s) with invalid SPDX license definitions
  • ⚠️ 1 package(s) with unknown licenses.
See the Details below.

Vulnerabilities

service/go.mod

NameVersionVulnerabilitySeverity
filippo.io/edwards255191.1.0filippo.io/edwards25519 MultiScalarMult produces invalid results or undefined behavior if receiver is not the identitylow

License Issues

service/go.mod

PackageVersionLicenseIssue Type
github.com/go-sql-driver/mysql1.9.2NullUnknown License
Denied Licenses: GPL-2.0, AGPL-1.0, AGPL-1.0-or-later, AGPL-1.0-only, AGPL-3.0, AGPL-3.0-only, AGPL-3.0-or-later, GPL-1.0, GPL-1.0+, GPL-1.0-only, GPL-1.0-or-later, CNRI-Python-GPL-Compatible, GPL-2.0+, GPL-2.0-only, GPL-2.0-or-later, GPL-2.0-with-GCC-exception, GPL-2.0-with-autoconf-exception, GPL-2.0-with-bison-exception, GPL-2.0-with-classpath-exception, GPL-2.0-with-font-exception, GPL-3.0, GPL-3.0+, GPL-3.0-only, GPL-3.0-or-later, GPL-3.0-with-GCC-exception, GPL-3.0-with-autoconf-exception, LGPL-2.0, LGPL-2.0+, LGPL-2.0-only, LGPL-2.0-or-later, LGPL-2.1, LGPL-2.1+, LGPL-2.1-only, LGPL-2.1-or-later, LGPL-3.0, LGPL-3.0+, LGPL-3.0-only, LGPL-3.0-or-later, LGPLLR, NGPL
Excluded from license check: pkg:githubactions/SonarSource/sonarqube-scan-action

OpenSSF Scorecard

PackageVersionScoreDetails
gomod/filippo.io/edwards25519 1.1.0 UnknownUnknown
gomod/github.com/go-sql-driver/mysql 1.9.2 UnknownUnknown

Scanned Files

  • service/go.mod

@c-r33d
c-r33d marked this pull request as ready for review July 24, 2026 13:46
@c-r33d
c-r33d requested review from a team as code owners July 24, 2026 13:46
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@service/entityresolution/multi-strategy/providers/sql/sql_provider.go`:
- Line 257: Canonicalize config.Driver before opening the database: normalize it
once at startup and pass the canonical value to both buildConnectionString and
sql.Open. Update the surrounding provider initialization flow so mixed-case
"PGX"/"POSTGRES" inputs resolve to the registered lowercase driver names while
preserving existing driver handling.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 247864f7-357d-4db3-8603-1b3d2e67fb03

📥 Commits

Reviewing files that changed from the base of the PR and between 2911e22 and 2f78ea9.

⛔ Files ignored due to path filters (1)
  • service/go.sum is excluded by !**/*.sum
📒 Files selected for processing (4)
  • service/entityresolution/multi-strategy/providers/sql/sql_provider.go
  • service/entityresolution/multi-strategy/v2/registration.go
  • service/entityresolution/multi-strategy/v2/registration_test.go
  • service/go.mod

Comment thread service/entityresolution/multi-strategy/providers/sql/sql_provider.go Outdated
@github-actions

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 227.901317ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 117.235768ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 424.740108ms
Throughput 235.44 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 50.191962866s
Average Latency 499.498909ms
Throughput 99.62 requests/second

@github-actions

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 171.473006ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 87.438106ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 412.90661ms
Throughput 242.19 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 48.446068478s
Average Latency 482.469154ms
Throughput 103.21 requests/second

@github-actions

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 218.596721ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 115.167672ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 411.424694ms
Throughput 243.06 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 49.856053585s
Average Latency 496.21547ms
Throughput 100.29 requests/second

alkalescent
alkalescent previously approved these changes Jul 24, 2026
Comment thread service/entityresolution/multi-strategy/v2/registration_test.go Outdated
Comment thread service/entityresolution/multi-strategy/v2/registration_test.go Outdated
Comment thread service/entityresolution/multi-strategy/v2/registration_test.go Outdated
Comment thread service/entityresolution/multi-strategy/v2/registration_test.go Outdated
Comment thread service/entityresolution/multi-strategy/v2/registration_test.go Outdated
elizabethhealy
elizabethhealy previously approved these changes Jul 24, 2026

@elizabethhealy elizabethhealy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

optional nits

@c-r33d
c-r33d dismissed stale reviews from elizabethhealy and alkalescent via dc27adf July 24, 2026 16:12
@c-r33d c-r33d changed the title fix(ers): Add pgx driver and marshal/unmarshal metadata correctly. fix(ers): marshal/unmarshal metadata correctly. Jul 24, 2026
@github-actions

Copy link
Copy Markdown
Contributor
Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 198.893342ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 109.984722ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 572.564562ms
Throughput 174.65 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 48.284288896s
Average Latency 480.538015ms
Throughput 103.55 requests/second

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Govulncheck found vulnerabilities ⚠️

The following modules have known vulnerabilities:

  • examples
  • otdfctl
  • sdk
  • service
  • lib/fixtures
  • tests-bdd

See the workflow run for details.

@c-r33d
c-r33d added this pull request to the merge queue Jul 24, 2026
Merged via the queue into main with commit 225f7ab Jul 24, 2026
73 of 82 checks passed
@c-r33d
c-r33d deleted the multi-ers-sql branch July 24, 2026 17:45
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.

3 participants