Skip to content

feat: API endpoint for matching training data - #1204

Open
DarshanCode2005 wants to merge 33 commits into
NixOS:mainfrom
DarshanCode2005:matching-training-api
Open

feat: API endpoint for matching training data#1204
DarshanCode2005 wants to merge 33 commits into
NixOS:mainfrom
DarshanCode2005:matching-training-api

Conversation

@DarshanCode2005

Copy link
Copy Markdown
Collaborator

Summary

Related files to this PR

  • src/api/matching/*
  • src/api/tests/test_matching_training_data.py

Comment thread src/api/urls.py Outdated
path("v1/server-info", ServerInfoView.as_view(), name="server-info"),
path("v1/tokens/me", TokenManagementView.as_view(), name="token-management"),
path(
"v1/matching/training-data",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@florentc opinions on where to put this, since you've been touching a lot of this recently?

@florentc florentc Jul 20, 2026

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.

Not really.

Seems to me this is not an API endpoint a regular sec tracker user would use. If it's an admin restricted thing I guess it should live under a v1/admin prefix.

I don't see what else we would put under matching beside training-data. Do we have future endpoints in mind? If not maybe let's flatten the route.

Comment thread src/api/matching/views.py Outdated
"""Staff or security-team members (see shared.auth.isadmin)."""

def has_permission(self, request: Request, view: APIView) -> bool: # pyright: ignore[reportIncompatibleMethodOverride]
return bool(request.user and isadmin(request.user))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should probably be a separate role so contributors can obtain the samples

@DarshanCode2005

Copy link
Copy Markdown
Collaborator Author

So should I change the route to v1/matching-training-data?

@fricklerhandwerk

Copy link
Copy Markdown
Collaborator

Yeah let's do that

@DarshanCode2005

Copy link
Copy Markdown
Collaborator Author

Clarifying against the plan for #1038 : this endpoint is read-only export of curated proposals. Auth was intentionally locked to isadmin , because in the next follow-up PR fetch_matching_training_data will use a Knox/session token against staging/prod. You suggested a separate role so contributors can obtain samples. What I need to know:

  1. Who is "contributor" here? : Matching-algorithm contributors outside security/committers, or should we reuse an existing bar (iscommitter/user_can_edit_suggestion)?
  2. Should we widen access in this PR , or keep isadmin for this current PR and add a dedicated role when the Github team/ group exists?
  3. Is the sensitivity of the payload ( labels, comments, overlays, rejection, reasons) a reason to keep this elevated, or is "any authenticated user who can mint a token" fine for public-ish training corpus?

@fricklerhandwerk

Copy link
Copy Markdown
Collaborator
  1. Let's just make a new Django group and assign people manually via database for now

    I suggest to hard-code that into a migration so we can't forget to deploy it.

  2. As above

  3. The concern is not sensitive data but server load. We don't have a concept for implementing rate limiting yet, so authentication to a limited group is the stopgap.

@DarshanCode2005
DarshanCode2005 force-pushed the matching-training-api branch from b077e92 to cc729f5 Compare July 30, 2026 09:29
Comment thread src/shared/matching_training_data/schema.py Fixed
Comment thread src/shared/matching_training_data/schema.py Fixed
Comment thread src/shared/matching_training_data/serializers.py Fixed
@DarshanCode2005
DarshanCode2005 force-pushed the matching-training-api branch from cc729f5 to bb0c813 Compare July 30, 2026 10:32
introduce schema_version=1 dataclasses and dict roundtrip helpers the offline matching corpus
Assert schema dict roundtrip, curated filter, export -> import -> re-export equality, auto-reject labels, and idempotent import by cve id
Replace hand-rolled dataclasses with round-tripping DRF serializers so
export/import share the same validated wire format.
Move SCHEMA_VERSION into constants.py so serializers no longer import
schema, and allow blank proposal comments during round-trip.
NixChannel.release_branch is now a foreign key and the repository field
is gone, so the synthetic benchmark channel needs its own branch row.
Add untouched() for pending proposals and user_curated() for
non-pending proposals, providing complementary helpers with shared
vocabulary. This allows NixOS#1211 to use
CVEDerivationClusterProposal.objects.untouched() instead of inlining
status=PENDING, while training export can use user_curated().

Update docstrings to cross-reference both methods and add tests
covering the complementary behavior.
Remove the hand-built _sample_record() helper from the schema tests.

Build proposal objects using the existing fixtures and serialize them
through CVEDerivationClusterProposal(...).data, matching the same code
path used by the API. Keep the schema version test by mutating the
serialized payload after export.

This makes the tests exercise the production serializer instead of a
manually constructed payload.
Inline the Organization.objects.get_or_create() call into create()
and remove the _ensure_organization() helper.

Keep the _TRAINING_ORG_UUID constant while avoiding an unnecessary
single-use helper, as suggested during review.
Extract a shared derivation_as_key() helper in evaluation.py and have
EvaluatedAttribute.as_key() delegate to it.

Reuse the same helper during training import deduplication by
constructing a deterministic synthetic drv_path from the derivation
attribute, name, and system. This keeps the training data format
portable while sharing the same keying logic as evaluation ingestion.
Drop duplicated nested OpenAPI field definitions; the API adapter
subclasses the shared round-tripping schema.
Renumber the matching_training_data group migration after NixpkgsBranch
landed as 0101, and drop the ruff-format blank line in the API view.
Reuse shared CVEDerivationClusterProposal serialization and user_curated queryset after the NixOS#1203 wrapper removed

@fricklerhandwerk fricklerhandwerk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Good! Just a few style nits in DarshanCode2005#3 (reviewed together)

- remove the redundant MatchingTrainingRecordSerializer adapter
  and use CVEDerivationClusterProposal directly
- add make_client helper for API test clients
- update the default client fixture to use make_client
- simplify assertions for curated training-data responses
- parameterize pagination tests across count and page size
- use explicit formulas for expected pagination results
- clean up unused imports and references to the removed serializer
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.

4 participants