feat(analyzer): entity list endpoint - #258
Conversation
| <BulkTimelineRequest<QueryFilter, OperatorFilter> as TS>::export_all(&cfg)?; | ||
| <BulkTimelinesResponse as TS>::export_all(&cfg)?; | ||
|
|
||
| <EntityListRequest<QueryFilter> as TS>::export_all(&cfg)?; |
There was a problem hiding this comment.
I think we need to be able to filter by operator here. Not sure if the exact right location, but the FE will want to filter long entities by operator presumably.
There was a problem hiding this comment.
Add a dedicated, paginated entity-list query: POST /api/engines/{id}/entities.
The contract (entities module: scope, window, filter, sort, page, plus per-query
application params) and a generic FiniteStateMachine::try_from_fsm live in
quent-ui; UiAnalyzer::list_entities and its analyzer helper rank any application
FSM by its longest usage span on an optional scope, tiebroken by entity UUID.
Application-specific filters ride in the per-query params: the simulator filters
by operator via OperatorFilter, mirroring the timeline. Additive only — no
timeline changes.
The functional test captures the fixed scenario in memory via a CallbackExporter
(rapidsai#257) and reconstructs the event stream with a new
quent-simulator-instrumentation::test_utils helper. ts-rs bindings included.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…elper Move scope resolution to EntityScope::resolve in quent-ui (it is generic over Model, not query-engine specific) and drop the free resolve_scope. Group the generic list_entities helper's per-query inputs into a ListQuery struct so it takes three arguments, and trim its docstrings. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
# Conflicts: # domains/query_engine/tests/fixed/Cargo.toml # examples/simulator/analyzer/src/lib.rs
| } => { | ||
| let tree = ResourceTreeNode::try_new(model, *resource_group_id)?; | ||
| Ok(tree | ||
| .iter_leaf_ids() |
There was a problem hiding this comment.
does this mean that only leaf resources should have valid usages ?
There was a problem hiding this comment.
Yes, this limitation is removed as a part of #191.
The resource concept will be unrelated to hierarchical trees of entities, also see:
https://github.com/rapidsai/quent/blob/main/crates/ref-tree/src/lib.rs
https://github.com/rapidsai/quent/blob/main/crates/resource/src/lib.rs
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: QUIET Plan: Enterprise Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a public entity-list query API with filtering, scope resolution, usage sorting, pagination, analyzer execution, an HTTP endpoint, simulator support, TypeScript exports, and fixed-scenario functional tests. ChangesEntity list query
Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (2)
domains/query_engine/server/src/ui.rs-265-276 (1)
265-276: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winPublish the concrete entity-list schemas.
request_body = Objectandbody = Objecthide the actual contract for this endpoint; generated clients can’t discoverentry,app_params, paging, ortotal. Register the concrete request/response types instead, along with any nestedToSchematypes they depend on.🤖 Prompt for 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. In `@domains/query_engine/server/src/ui.rs` around lines 265 - 276, Update the Swagger metadata for the entity-list endpoint around its utoipa path definition to reference the concrete request and response types instead of generic Object schemas. Ensure the request type exposes entry, app_params, and paging fields, the response type exposes the paged entities and total, and register all nested types required by those schemas with ToSchema.domains/query_engine/analyzer/src/entities.rs-101-106 (1)
101-106: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winPrevent 32-bit pagination-offset overflow.
PageParamsusesu32, so(p.page as usize) * (p.max as usize)can overflow on 32-bit targets and panic or wrap; usesaturating_mulso out-of-range pages return an empty slice.🤖 Prompt for 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. In `@domains/query_engine/analyzer/src/entities.rs` around lines 101 - 106, Update the pagination iterator construction in the page match to calculate the skip offset with saturating multiplication, using the u32 page and max values before or during conversion as appropriate. Ensure out-of-range pagination produces an empty result without 32-bit overflow, while preserving the existing take behavior.
🧹 Nitpick comments (1)
crates/ui/src/lib.rs (1)
257-261: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDescribe the conversion contract rather than the grouping implementation.
This docstring explains the internal state-name grouping strategy. As per path instructions, “Docstrings state the contract, not the mechanism; do not restate types.”
Proposed documentation change
-/// Build from any application FSM via the [`FsmUsages`](a::fsm::FsmUsages) -/// interface. -/// -/// Usages are grouped onto their state's transition by state name, which is -/// unique within an FSM. +/// Converts an application FSM into its UI representation relative to `epoch`.🤖 Prompt for 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. In `@crates/ui/src/lib.rs` around lines 257 - 261, Update the documentation for the conversion API near FsmUsages to describe its public conversion contract: it builds from any application FSM implementing FsmUsages and preserves the associated state-transition usages. Remove the explanation of grouping by state name and avoid restating implementation details or type mechanics.Source: Path instructions
🤖 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 `@examples/simulator/analyzer/src/lib.rs`:
- Around line 262-290: The list_entities method must restrict task candidates to
request.query_id, not only the optional operator_id. Update the predicate passed
to entities::list_entities to require the task’s query/view identity to match
the requested query while preserving the operator filter, and add a regression
case covering overlapping queries in one model.
---
Other comments:
In `@domains/query_engine/analyzer/src/entities.rs`:
- Around line 101-106: Update the pagination iterator construction in the page
match to calculate the skip offset with saturating multiplication, using the u32
page and max values before or during conversion as appropriate. Ensure
out-of-range pagination produces an empty result without 32-bit overflow, while
preserving the existing take behavior.
In `@domains/query_engine/server/src/ui.rs`:
- Around line 265-276: Update the Swagger metadata for the entity-list endpoint
around its utoipa path definition to reference the concrete request and response
types instead of generic Object schemas. Ensure the request type exposes entry,
app_params, and paging fields, the response type exposes the paged entities and
total, and register all nested types required by those schemas with ToSchema.
---
Nitpick comments:
In `@crates/ui/src/lib.rs`:
- Around line 257-261: Update the documentation for the conversion API near
FsmUsages to describe its public conversion contract: it builds from any
application FSM implementing FsmUsages and preserves the associated
state-transition usages. Remove the explanation of grouping by state name and
avoid restating implementation details or type mechanics.
🪄 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: Path: .coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 0ecb4efe-9538-4efd-bd6b-c71bec48d098
⛔ Files ignored due to path filters (11)
Cargo.lockis excluded by!**/*.lock,!Cargo.lockexamples/simulator/server/ts-bindings/EntityListEntry.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/EntityListFilter.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/EntityListRequest.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/EntityListResponse.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/EntityScope.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/EntitySortKey.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/PageParams.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/Sort.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/SortDir.tsis excluded by!examples/simulator/server/ts-bindings/**examples/simulator/server/ts-bindings/TimeWindow.tsis excluded by!examples/simulator/server/ts-bindings/**
📒 Files selected for processing (17)
crates/ui/src/entities/mod.rscrates/ui/src/entities/request.rscrates/ui/src/entities/response.rscrates/ui/src/lib.rscrates/ui/src/paginate.rsdomains/query_engine/analyzer/src/entities.rsdomains/query_engine/analyzer/src/lib.rsdomains/query_engine/analyzer/src/ui.rsdomains/query_engine/server/src/timeline_cache.rsdomains/query_engine/server/src/ui.rsdomains/query_engine/tests/fixed/Cargo.tomldomains/query_engine/tests/fixed/tests/list_entities.rsexamples/simulator/analyzer/src/lib.rsexamples/simulator/instrumentation/Cargo.tomlexamples/simulator/instrumentation/src/lib.rsexamples/simulator/instrumentation/src/test_utils.rsexamples/simulator/server/build.rs
- list_entities: restrict task candidates to the requested query, not only the optional operator filter. A task belongs to a query iff its operator is one of that query's operators; without this, tasks from a different query sharing a resource and overlapping the window leaked in. - entities pagination: use saturating multiplication for the skip offset so an out-of-range page yields an empty page instead of overflowing usize on a 32-bit target. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
/merge |
…rom_fsm Rebasing onto main surfaced two integration points: - main's edits to examples/simulator/analyzer/Cargo.toml overlapped this branch's `quent-attributes` addition; the merge silently dropped it, breaking the simulator analyzer's rate synthesis. Restore it. - main added FiniteStateMachine::try_from_fsm (rapidsai#258); populate the new attributes / derived_attributes fields there. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Felipe Aramburu <felipearamburu@gmail.com>
Description
Adds
POST /api/engines/{engine_id}/entities: a paginated query listing aquery's FSM entities ranked by longest resource-usage span (tiebroken by UUID),
to back the UI's long-entities view. Additive — no timeline changes.
Request: optional scope (resource / group), query-relative window, filters
(
entity_type_name,min_usage_s), sort, page, and a per-queryOperatorFilter.Contract lives in
quent-ui; a genericentities::list_entitiesranks anyFsmCollectionFSM, with operator filtering applied by the analyzer (kept out ofthe generic filter).
Functional test over the fixed scenario captures events in memory via
CallbackExporterand reconstructs them with a newquent-simulator-instrumentation::test_utilshelper.Depends on #256 and #257 (merged).
🤖 Generated with Claude Code
Related Issues
Part of #214, isolated from #240