Skip to content

[#4253 3/8] refactor(pe): rewire Match onto Select, remove selector abstraction#4284

Draft
stevenvegt wants to merge 2 commits into
4253-1-select-enginefrom
4253-3-rewire-select
Draft

[#4253 3/8] refactor(pe): rewire Match onto Select, remove selector abstraction#4284
stevenvegt wants to merge 2 commits into
4253-1-select-enginefrom
4253-3-rewire-select

Conversation

@stevenvegt

@stevenvegt stevenvegt commented May 27, 2026

Copy link
Copy Markdown
Member

Parent PRD

#4253

Item 3 of 8. Depends on item #1 (#4280) — branched off 4253-1-select-engine, retargeted to the feature branch once #1 merges.

Summary

Rewire the public vcr/pe matching API onto Select and delete the CredentialSelector abstraction. The credential_selection path stops constructing a selector and instead passes the selection as Select initial bindings. Net effect: less surface, and a single matching engine. Behavior is preserved for every PD that does not reuse an id across descriptors (i.e. every PD today).

Pre-work already on this branch (2026-07-16)

The first commit deepens the shared matching primitive (decided with the user during the #4280
close-out review; PRD "the PD is the data contract" rationale):

  • matchConstraint = matchConstraintOnMap (evaluation on a pre-converted JSON object,
    per-field fieldResult outcomes) + a thin converting wrapper. Token introspection
    (ResolveConstraintsFields) keeps calling the wrapper, so selection and introspection still
    share one definition of field resolution.
  • The engine converts each candidate once per Select call (was: once per descriptor; the
    dominant step-1 cost at 20k-credential wallets) and caches the PD-level format gate.
  • The MatchReport derives dismissals from the same per-field outcomes the selection used;
    the explainFieldMismatch re-walk is deleted.

Behavior-preserving; both black-box suites (engine + presentation-definition/introspection)
pass unchanged apart from the mechanical matchField signature update in its unit test.

Implementation Spec

vcr/pe/presentation_definition.go

  • Match(vcs []vc.VerifiableCredential, opts ...Option) ([]vc.VerifiableCredential, []InputDescriptorMappingObject, error) — gains variadic options that pass through to Select. Discovery's pd.Match(creds) keeps compiling unchanged (no opts ⇒ FirstMatch, no bindings).
  • Delete MatchWithSelector.
  • Delete matchConstraints, matchBasic, and matchSubmissionRequirements. Their submission-requirement satisfaction now lives inside Select (item added baseline funcs from old repo #1, which reuses the unchanged groups()/apply()/deduplicate() helpers). Match becomes a thin wrapper: build Select options, call Select(pd, vcs, opts...), then build the []InputDescriptorMappingObject descriptor map (paths + dedup, plus the single-VC JSON-LD vs JWT path fixup) from Result.

vcr/pe/selector.go

Delete entirely: CredentialSelector, FirstMatchSelector, NewFieldSelector, fieldSelection, matchesSelections.

Builder (vcr/pe/presentation_submission.go)

Presenter (vcr/holder/presenter.go)

  • Lines 64-72 collapse to: if credentialSelection is non-empty, builder.SetInitialBindings(credentialSelection). The NewFieldSelector construction and its unknown-key error (line 67-70) are removed.
  • Unknown selection keys are now silently dropped by Select; typo protection moves to ValidateSelectionKeys at the API layer (Add verifiable data registry #6). This realizes the PRD's "NewFieldSelector becomes lenient" by deletion.

Behavior preservation (the bar)

For any PD without same-id reuse — every PD deployed today — FirstMatch plus the absence of cross-descriptor bindings means Select picks the same VC per descriptor that FirstMatchSelector / NewFieldSelector did, with no backtracking. So:

  • The existing TestMatch, the submission-requirement, and PresentationSubmission.Validate test scenarios are the characterization safety net for this rewire and pass with no assertion changes. TestPresentationDefinition_MatchWithSelector_SubmissionRequirements is retargeted to call Match (with options) since MatchWithSelector is deleted, but its assertions stay the same.
  • selector_test.go is deleted: its TestNewFieldSelector scenarios are covered by added baseline funcs from old repo #1's characterization tests (Select with bindings), and the strict-unknown-key assertion is dropped (its replacement is ValidateSelectionKeys in added nuts-go-test #2).
  • Any holder/presenter test asserting an error on unknown selection keys is updated to assert the key is dropped (no error).

Submission-requirement satisfaction (apply/groups/deduplicate) is in scope and reused unchanged inside Select. What stays deferred per the PRD is the binding search cooperating with pick min>=1 subset selection: if Select's coarse skip yields a combination that a pick/min rule rejects, it does not re-search for a different binding-consistent combination to satisfy the floor; it reports the failure. Tie-breakers among multiple valid assignments are also deferred.

Testing

  • Existing vcr/pe and vcr/holder suites green, with only the deletions/updates noted above.
  • A focused test that Match(vcs, WithInitialBindings(sel)) produces the same result the old NewFieldSelector path did for a representative selection (belt-and-suspenders alongside added baseline funcs from old repo #1's characterization tests).

Acceptance Criteria

  • Match takes variadic Options and is a thin wrapper over Select (builds the descriptor map from Result via the shared private helper); MatchWithSelector, matchConstraints, matchBasic, and matchSubmissionRequirements removed.
  • selector.go deleted; no references to CredentialSelector/FirstMatchSelector/NewFieldSelector remain.
  • Builder exposes SetInitialBindings; presenter wires credentialSelection through it with no unknown-key error.
  • All existing vcr/pe and vcr/holder tests pass (updated only as described); selector_test.go removed.
  • go build ./... and go test ./vcr/... pass.

@qltysh

qltysh Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

Qlty


Coverage Impact

⬇️ Merging this pull request will decrease total coverage on 4253-1-select-engine by 0.01%.

Modified Files with Diff Coverage (3)

RatingFile% DiffUncovered Line #s
Coverage rating: A Coverage rating: A
vcr/pe/select.go88.2%151-152, 728-729
Coverage rating: A Coverage rating: A
vcr/pe/match_report.go92.9%251-252
Coverage rating: A Coverage rating: A
vcr/pe/presentation_definition.go100.0%
Total93.3%
🤖 Increase coverage with AI coding...
In the `4253-3-rewire-select` branch, add test coverage for this new code:

- `vcr/pe/match_report.go` -- Line 251-252
- `vcr/pe/select.go` -- Lines 151-152 and 728-729

🚦 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.

matchConstraint splits into matchConstraintOnMap (evaluation on a
pre-converted JSON object, returning per-field outcomes) and a thin
converting wrapper that token introspection keeps using, so selection
and introspection still share one definition of field resolution. The
engine converts every candidate once per Select call instead of once
per descriptor (the dominant step-1 cost at large wallets: D-fold
JSON remarshaling removed) and caches the PD-level format gate per
credential. matchField returns a fieldResult carrying the rejection
detail, which lets the MatchReport render dismissals from the same
evaluation the selection used: the explainFieldMismatch re-walk is
gone, removing the last place where report reasons could drift from
selection behavior.

Assisted-by: AI
@stevenvegt stevenvegt force-pushed the 4253-3-rewire-select branch from caa5170 to 281d6f3 Compare July 16, 2026 06:29
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.

1 participant