feat: add endpoint for returning all AIBOM's, optionally filtered#2325
Merged
jcrossley3 merged 2 commits intoguacsec:mainfrom Apr 20, 2026
Merged
feat: add endpoint for returning all AIBOM's, optionally filtered#2325jcrossley3 merged 2 commits intoguacsec:mainfrom
jcrossley3 merged 2 commits intoguacsec:mainfrom
Conversation
Contributor
Reviewer's GuideAdds a new v2 endpoint and OpenAPI definition to list all AI SBOM models across SBOMs with existing query/sort/pagination semantics, refactors the service method to accept an optional SBOM id so it can be reused by both per-SBOM and global listings, and introduces tests that validate the new endpoint’s query behavior using ingested AIBOM fixtures. Sequence diagram for the new listAllModels v2 endpointsequenceDiagram
actor ApiClient
participant ActixRouter
participant AllModelsHandler as all_models
participant SbomService
participant Database
ApiClient->>ActixRouter: GET /api/v2/sbom/models?q&sort&offset&limit
ActixRouter->>AllModelsHandler: Route request
AllModelsHandler->>Database: begin_read()
Database-->>AllModelsHandler: read_transaction
AllModelsHandler->>SbomService: fetch_sbom_models(None, search, paginated, read_transaction)
SbomService->>Database: execute AI models query with optional sbom_id filter
Database-->>SbomService: PaginatedResults<SbomModel>
SbomService-->>AllModelsHandler: PaginatedResults<SbomModel>
AllModelsHandler-->>ApiClient: 200 OK (application/json)
Updated class diagram for SbomService and AI model listing endpointsclassDiagram
class SbomService {
+fetch_sbom_models(sbom_id: Option_Uuid, search: Query, paginated: Paginated, connection: ConnectionTrait) PaginatedResults_SbomModel
}
class SbomEndpoints {
+models(fetch: SbomService, db: Database, search: Query, paginated: Paginated, read_guard: ReadSbom) HttpResponse
+all_models(fetch: SbomService, db: Database, search: Query, paginated: Paginated, read_guard: ReadSbom) HttpResponse
}
class Database {
+begin_read() Transaction
}
class Query {
}
class Paginated {
+offset: int64
+limit: int64
}
class PaginatedResults_SbomModel {
+items: List_SbomModel
+total: int64
+offset: int64
+limit: int64
}
class SbomModel {
+id: string
}
class ReadSbom {
}
class ConnectionTrait {
}
class Transaction {
}
SbomEndpoints --> SbomService
SbomEndpoints --> Database
SbomEndpoints --> Query
SbomEndpoints --> Paginated
SbomEndpoints --> ReadSbom
SbomService ..> ConnectionTrait
SbomService --> PaginatedResults_SbomModel
PaginatedResults_SbomModel "1" o-- "*" SbomModel
Database --> Transaction
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="modules/fundamental/src/sbom/endpoints/test.rs" line_range="1861-1870" />
<code_context>
+#[test_context(TrustifyContext)]
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test case for calling `/api/v2/sbom/models` without a `q` parameter to verify the default behavior returns all models and aligns with pagination defaults.
The current parameterized tests only cover variations of the `q` filter and never call `/api/v2/sbom/models` without it (or with an empty string). Please add a dedicated test (or rstest case) that omits `q` and asserts the expected `total` and pagination metadata for the default "list all" behavior to protect against regressions in default pagination handling.
Suggested implementation:
```rust
#[test_context(TrustifyContext)]
#[rstest]
#[case("hugging", 2)]
#[case("granite", 1)]
#[case("pkg:huggingface/ibm-granite", 1)]
#[case("pkg:huggingface/ibm-granite/granite-docling-258M", 1)]
#[case("pkg:huggingface/ibm-granite/granite-docling-258M@1.0", 1)]
#[case("purl=pkg:huggingface/ibm-granite/granite-docling-258M@1.0", 1)]
#[case("purl~granite", 1)]
#[case("purl:namespace=ibm-granite&purl:version=1.0&purl:type=huggingface", 1)]
#[case("name~granite", 1)]
#[test_context(TrustifyContext)]
#[tokio::test]
async fn list_models_without_q_returns_default_pagination(ctx: &TrustifyContext) -> anyhow::Result<()> {
// When: calling `/api/v2/sbom/models` without a `q` parameter, we expect the default
// "list all models" behavior and default pagination metadata.
let response = ctx
.client
.get("/api/v2/sbom/models")
.send()
.await?;
assert_eq!(response.status(), reqwest::StatusCode::OK);
#[derive(serde::Deserialize)]
struct ModelsPage<T> {
total: u64,
items: Vec<T>,
offset: u64,
limit: u64,
}
// Use the minimal shape needed for the assertions; the full model type is not required here.
#[derive(serde::Deserialize)]
struct ModelSummary {
id: String,
}
let page: ModelsPage<ModelSummary> = response.json().await?;
// Then: verify default pagination semantics for "list all".
//
// `total` should reflect the total number of models in the system and must be at
// least as large as the number of items in this page.
assert!(page.total >= page.items.len() as u64);
// Default listing should start at the beginning.
assert_eq!(page.offset, 0);
// `limit` should be the configured default page size; at minimum it must be at
// least as large as the number of items returned.
assert!(page.limit >= page.items.len() as u64);
Ok(())
}
```
Depending on the existing test harness, you may need to:
1. Adjust the way the HTTP client is accessed:
- If `TrustifyContext` exposes the client differently (e.g. `ctx.http`, `ctx.api`, `ctx.app`, or a helper like `ctx.get("/path").await`), replace `ctx.client.get(...).send().await?` with the appropriate call.
2. Align the HTTP client and status imports:
- If you are not already using `reqwest`, replace `reqwest::StatusCode` with the status type you use elsewhere (for example, `actix_web::http::StatusCode` or `hyper::StatusCode`), and ensure any necessary `use` statements are present at the top of the file.
3. Reuse existing pagination/model types if available:
- If the project already defines shared pagination or model summary DTOs (e.g. `Page<T>`, `PaginatedResponse<T>`, or a `ModelSummary` type), remove the local `ModelsPage` and `ModelSummary` structs and deserialize directly into those shared types instead, updating the field names in the assertions if they differ (`total_count` vs `total`, `data` vs `items`, etc.).
4. Keep test attributes consistent:
- If other tests in this file use a different asynchronous test attribute (e.g. `#[actix_rt::test]` or `#[tokio::test(flavor = "multi_thread")]`), update the new test's attribute to match.
</issue_to_address>
### Comment 2
<location path="modules/fundamental/src/sbom/endpoints/test.rs" line_range="1902-1904" />
<code_context>
+
+ let uri = format!("/api/v2/sbom/models?q={}", encode(q));
+ let req = TestRequest::get().uri(&uri).to_request();
+ let response: Value = app.call_and_read_body_json(req).await;
+
+ assert_eq!(response["total"].as_i64(), Some(count), "q: {q}");
+
+ Ok(())
</code_context>
<issue_to_address>
**suggestion (testing):** Strengthen assertions by checking the actual returned models (e.g., IDs or names) in addition to the `total` count.
Asserting only on `response["total"]` would still pass if the wrong models are returned but the count matches. Please also validate the contents of `items` (e.g., expected IDs/names/purls present and unexpected ones absent, or a sorted list of IDs/names) so the test actually checks the filtering behavior, not just the count.
Suggested implementation:
```rust
let uri = format!("/api/v2/sbom/models?q={}", encode(q));
let req = TestRequest::get().uri(&uri).to_request();
let response: Value = app.call_and_read_body_json(req).await;
// Keep existing count assertion
assert_eq!(response["total"].as_i64(), Some(count), "q: {q}");
// Assert items length matches reported total
let items = response["items"]
.as_array()
.expect("response.items should be an array");
assert_eq!(
items.len() as i64,
count,
"items length should match total for q: {q}"
);
// Collect a string identifier for each item (prefer purl, fall back to name)
let item_strings: Vec<String> = items
.iter()
.map(|item| {
item["purl"]
.as_str()
.or_else(|| item["name"].as_str())
.unwrap_or_default()
.to_string()
})
.collect();
// Strengthen assertions by validating which models are returned
match q {
// Broad search, should include both models
"hugging" => {
assert!(
item_strings
.iter()
.any(|s| s.contains("granite-docling-258M")),
"expected granite-docling-258M in results for q: {q}"
);
assert!(
item_strings.iter().any(|s| s.contains("canary-1b-v2")),
"expected canary-1b-v2 in results for q: {q}"
);
}
// All other cases in this test are granite-specific queries
_ => {
// Only the granite model should be returned
assert!(
item_strings
.iter()
.all(|s| s.contains("granite-docling-258M")),
"only granite-docling-258M should match for q: {q}"
);
assert!(
!item_strings.iter().any(|s| s.contains("canary-1b-v2")),
"canary-1b-v2 should not be returned for q: {q}"
);
}
}
Ok(())
}
```
1. Ensure `serde_json::Value` is imported at the top of the file, if it is not already:
`use serde_json::Value;`
2. If the actual identifying field is not `purl` or `name`, adjust the `item_strings` extraction to use the appropriate key (e.g., `id` or another field).
3. If the model identifiers differ from `"granite-docling-258M"` and `"canary-1b-v2"` in the actual API response, update the `contains(...)` substrings accordingly to match the real values (IDs, names, or purls).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2325 +/- ##
==========================================
+ Coverage 68.96% 68.99% +0.02%
==========================================
Files 435 435
Lines 24366 24381 +15
Branches 24366 24381 +15
==========================================
+ Hits 16805 16822 +17
+ Misses 6678 6667 -11
- Partials 883 892 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ctron
reviewed
Apr 14, 2026
Contributor
ctron
left a comment
ctron
left a comment
There was a problem hiding this comment.
I think the AI is right. So I think it makes sense:
- add a test variant without
q - not only test for the number of results, but for the actual models returned (which implies the number then)
- also: add some tests to the scale-testing repository using this new endpoint. Not only with one case, but using a few with
q(and without)
Contributor
Author
done |
Contributor
Sorry I can't find the new tests in the scale-testing repository. Could you please point me towards them. Also (not a blocker), I'm wondering how the user can get a reference on the SBOM with the search results of the models? There seems to be no "sbom ID" as part of the response. |
Contributor
carlosthe19916
left a comment
carlosthe19916
left a comment
There was a problem hiding this comment.
@jcrossley3 sorry for my late review. Looks good; only one thing missing
The mockups (image below) contains the column SBOMs which is supposed to count the number of SBOMs that contain a particular AI Model. How can I filter SBOMs based on a Model?
Contributor
Author
I created an issue in that repo: guacsec/trustify-scale-testing#99 This would have to me merged before work can proceed on that, right? |
Contributor
Author
Hmmm. I totally missed that. I assume your question is because that's what's required in response to a user clicking the "sbom count" link? |
Contributor
Following the Licenses Page Pattern, if the user clicks on the "SBOM Count" then he would be redirected to the SBOM List page with a pre-defined filter that renders only those SBOMs. Watch the video below of the licenses. In the UI side:
Screencast.From.2026-04-15.15-22-27.mp4 |
Contributor
|
@jcrossley3 If it help I think we can tackle the "sbom count" in a separate PR so the UI takes this one and move things forward. If you want to do everything here it is also fine, whatever generates less problems for you |
Contributor
Author
Yes, please. It's not a trivial change, so deserving of its own PR, I think. |
carlosthe19916
approved these changes
Apr 15, 2026
Contributor
carlosthe19916
left a comment
carlosthe19916
left a comment
There was a problem hiding this comment.
LGTM, I tested it locally and there is this PR on the UI side that renders the models guacsec/trustify-ui#995
As agreed with @jcrossley3 the column SBOM Count can be done in a separate PR
Contributor
I'd say it can be run in parallel. And you should be able to run a test locally, just to show some numbers. |
Merged
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2324
Summary by Sourcery
Add a new API endpoint to list all AI SBOM models with flexible querying and pagination, alongside adapting existing model retrieval logic to support both per-SBOM and global queries.
New Features:
Enhancements:
Tests: