#4120: Selection CredentialSelector — filter candidates by PD field ID values by stevenvegt · Pull Request #4121 · nuts-foundation/nuts-node

stevenvegt · 2026-03-25T16:35:10Z

Summary

Introduces NewSelectionSelector factory in vcr/pe/selector.go that creates a CredentialSelector filtering candidates by PD field ID values
Selection keys are validated against PD field IDs at construction time — unknown keys return an error
For input descriptors with matching selection keys: resolves field values via existing matchConstraint logic, filters by equality
For input descriptors without matching selection keys: falls back to FirstMatchSelector
Zero matches → ErrNoCredentials (soft failure via Refactor PD matching to return all candidates per input descriptor #4088), multiple matches → ErrMultipleCredentials

Closes #4120
Part of #4067

Test plan

Selection picks the correct credential by field value
Zero matches returns ErrNoCredentials
Multiple matches returns ErrMultipleCredentials
Unknown selection key returns construction error
No selection keys for descriptor → fallback to first match
Multiple selection keys on same descriptor use AND semantics
Multiple descriptors with independent selection keys

🤖 Generated with Claude Code

…D values NewSelectionSelector creates a CredentialSelector that filters candidates using named field ID values from the credential_selection API parameter. Selection keys are validated against PD field IDs at construction time. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

qltysh · 2026-03-25T16:36:41Z

0 new issues

Tool	Category	Rule	Count

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

qltysh · 2026-03-25T16:46:50Z

Coverage Impact

This PR will not change total coverage.

Modified Files with Diff Coverage (1)

Rating	File	% Diff	Uncovered Line #s
	vcr/pe/selector.go	84.2%	40, 44, 72, 76...
	Total	84.2%

🤖 Increase coverage with AI coding...

In the `feature/4120-selection-selector` branch, add test coverage for this new code:

- `vcr/pe/selector.go` -- Lines 40, 44, 72, 76, 97-98, and 103-105

🚦 See full report on Qlty Cloud »

🛟 Help

Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.
Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.
File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)
- Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

Copilot

Pull request overview

Adds a new Presentation Exchange CredentialSelector factory (NewSelectionSelector) that narrows already-matching credential candidates using credential_selection key/value pairs that reference PD constraint field IDs. This supports deterministic credential selection when multiple VCs satisfy the same input descriptor.

Changes:

Introduce NewSelectionSelector in vcr/pe/selector.go, including construction-time validation of selection keys against PD field IDs and per-descriptor fallback behavior.
Add comprehensive unit tests for selection behavior, including AND semantics, fallback behavior, and expected error cases.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
`vcr/pe/selector.go`	Implements `NewSelectionSelector` and the field-value filtering logic for candidates.
`vcr/pe/selector_test.go`	Adds tests covering single match, zero/multiple matches, unknown key validation, fallback, AND semantics, and multi-descriptor behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-26T11:57:12Z

vcr/pe/selector.go

+		var matched []vc.VerifiableCredential
+		for _, candidate := range candidates {
+			if descriptor.Constraints == nil {
+				continue
+			}
+			isMatch, values, err := matchConstraint(descriptor.Constraints, candidate)
+			if err != nil || !isMatch {
+				continue
+			}
+			if matchesSelections(values, selections) {
+				matched = append(matched, candidate)


NewSelectionSelector re-runs matchConstraint for each candidate even though the selector contract says all candidates already match the input descriptor (they’re produced by PresentationDefinition.matchConstraints). This duplicates JSON remarshal + full constraint evaluation and can be a noticeable overhead when wallets contain many VCs. Consider resolving only the selected field values (e.g., build a fieldID→Field map at construction and use matchField/getValueAtPath on a single remarshal), without re-checking the entire constraint set.

Copilot · 2026-03-26T11:57:12Z

vcr/pe/selector.go

+				continue
+			}
+			isMatch, values, err := matchConstraint(descriptor.Constraints, candidate)
+			if err != nil || !isMatch {


Errors from matchConstraint are currently swallowed (if err != nil || !isMatch { continue }). If matchConstraint ever fails (e.g., due to unexpected credential encoding/path issues), this will silently drop candidates and surface as ErrNoCredentials/ErrMultipleCredentials, masking the real problem. Return the error immediately when err != nil so the caller gets a hard failure with the root cause.

Suggested change

if err != nil || !isMatch {

if err != nil {

return nil, fmt.Errorf("input descriptor '%s': %w", descriptor.Id, err)

}

if !isMatch {

Copilot · 2026-03-26T11:57:12Z

vcr/pe/selector.go

+	return func(descriptor InputDescriptor, candidates []vc.VerifiableCredential) (*vc.VerifiableCredential, error) {
+		selections, ok := descriptorSelections[descriptor.Id]
+		if !ok {
+			return fallback(descriptor, candidates)
+		}


NewSelectionSelector may call fallback(descriptor, candidates) without guarding against a nil fallback. Since this is a public factory, passing a nil fallback would panic at runtime. Consider defaulting fallback to FirstMatchSelector when nil (or returning an error at construction time).

reinkrul · 2026-03-26T11:51:11Z

vcr/pe/selector.go

+
+// NewSelectionSelector creates a CredentialSelector that filters candidates using
+// named field ID values from the credential_selection parameter.
+func NewSelectionSelector(selection map[string]string, pd PresentationDefinition, fallback CredentialSelector) (CredentialSelector, error) {


Sounds like it selects a selection, but it just selects credentials? So why not NewCredentialSelector?

reinkrul · 2026-03-26T11:55:08Z

vcr/pe/selector.go

+				continue
+			}
+			isMatch, values, err := matchConstraint(descriptor.Constraints, candidate)
+			if err != nil || !isMatch {


is it worth logging the error, if we're not returning it? In which cases cant his occur?

reinkrul · 2026-03-26T11:56:53Z

vcr/pe/selector.go

+			if descriptor.Constraints == nil {
+				continue
+			}
+			isMatch, values, err := matchConstraint(descriptor.Constraints, candidate)


We have multiple constraint types in Presentation Definitions. We generally use constant (value must be exactly X) or pattern (value must comply to regex pattern Y). Do we want to support both?

Does that work properly with matchesSelections() which compares the given value to the matched value? Because it sounds like we only want to support constant.

reinkrul · 2026-03-26T11:58:01Z

vcr/pe/selector.go

+			if str != sel.expected {
+				return false
+			}
+		} else if fmt.Sprintf("%v", resolved) != sel.expected {


why are we doing this? Support for bools and numbers? Does that actually work?

Why not just comparison through ==?

stevenvegt requested review from gerardsn, reinkrul and woutslakhorst as code owners March 25, 2026 16:35

stevenvegt mentioned this pull request Mar 25, 2026

Selection CredentialSelector: filter candidates by PD field ID values #4120

Open

16 tasks

#4120: Simplify key validation with matchedKeys set

4ec5a9a

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

stevenvegt linked an issue Mar 25, 2026 that may be closed by this pull request

Selection CredentialSelector: filter candidates by PD field ID values #4120

Open

16 tasks

This was referenced Mar 25, 2026

#4087: DCQL credential query parser #4091

Closed

Credential selection when multiple credential from wallet match #4067

Open

reinkrul requested a review from Copilot March 26, 2026 11:52

Copilot started reviewing on behalf of reinkrul March 26, 2026 11:53 View session

Copilot AI reviewed Mar 26, 2026

View reviewed changes

reinkrul requested changes Mar 26, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#4120: Selection CredentialSelector — filter candidates by PD field ID values#4121

#4120: Selection CredentialSelector — filter candidates by PD field ID values#4121
stevenvegt wants to merge 2 commits intofeature/4088-credential-selectorfrom
feature/4120-selection-selector

stevenvegt commented Mar 25, 2026

Uh oh!

qltysh bot commented Mar 25, 2026

Uh oh!

qltysh bot commented Mar 25, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

reinkrul Mar 26, 2026

Uh oh!

reinkrul Mar 26, 2026

Uh oh!

reinkrul Mar 26, 2026

Uh oh!

reinkrul Mar 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-			if err != nil || !isMatch {
+			if err != nil {
+				return nil, fmt.Errorf("input descriptor '%s': %w", descriptor.Id, err)
+			}
+			if !isMatch {

Conversation

stevenvegt commented Mar 25, 2026

Summary

Test plan

Uh oh!

qltysh bot commented Mar 25, 2026

0 new issues

Uh oh!

qltysh bot commented Mar 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

reinkrul Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

reinkrul Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

reinkrul Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

reinkrul Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants