feat(java): workflow dashboard + saga node fields#390
Conversation
📝 WalkthroughWalkthroughThis PR adds workflow-run inspection across the JNI boundary, Java SDK, and dashboard API. It also extends workflow status payloads with compensation and parent-linkage fields, and adds dashboard tests for the new read endpoints. ChangesWorkflow Run Inspection Feature
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant DashboardServer
participant WorkflowsHandlers
participant Taskito as Taskito (JniQueueBackend)
participant Rust as taskito-java JNI
Client->>DashboardServer: GET /api/workflows/runs/{id}/dag
DashboardServer->>WorkflowsHandlers: dag(id)
WorkflowsHandlers->>Taskito: getWorkflowDag(id)
Taskito->>Rust: getWorkflowDag(handle, runId)
Rust-->>Taskito: DAG JSON string
Taskito-->>WorkflowsHandlers: Optional<String>
WorkflowsHandlers->>WorkflowsHandlers: enrichDag(dagJson, nodes)
WorkflowsHandlers-->>DashboardServer: enriched dag response
DashboardServer-->>Client: JSON response
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
sdks/java/src/test/java/org/byteveda/taskito/dashboard/DashboardWorkflowTest.java (1)
25-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider covering compensation fields and query filters.
Good coverage of the four endpoints' happy path. Since this PR also adds
compensation_job_id/compensation_started_at/compensation_completed_at/compensation_errortoContract.workflowNode, andruns()supportsdefinition_name/state/limit/offsetfiltering, a follow-up test asserting on these would guard against regressions in the new plumbing.🤖 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 `@sdks/java/src/test/java/org/byteveda/taskito/dashboard/DashboardWorkflowTest.java` around lines 25 - 47, Extend DashboardWorkflowTest to cover the new workflow node compensation fields and run filtering support. In the existing DashboardServer/DashboardClient happy-path test around listsRunsAndReturnsDetail, add assertions that the run detail payload includes Contract.workflowNode compensation fields such as compensation_job_id, compensation_started_at, compensation_completed_at, and compensation_error. Also add coverage for DashboardClient.runs()/the /api/workflows/runs endpoint with definition_name, state, limit, and offset filters to verify the query plumbing returns the expected filtered subset.crates/taskito-java/src/workflows/mod.rs (1)
125-153: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider a lighter view for bulk run listings.
WorkflowRunView(used previously for the single-run status view) is now also returned as a JSON array forlistWorkflowRunsandgetWorkflowChildren(Lines 1426, 1467). Each row still includesparams/error, which can be an arbitrary, potentially large or sensitive payload. For a dashboard "list runs" table this multiplies exposure/payload size across every row, where a summary typically only needs id/definitionId/state/timestamps.Consider a slimmer
WorkflowRunSummaryView(orOption-gate the raw params) for the list/children endpoints, keeping the fullparams/errorfor the single-run detail lookup.🤖 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/taskito-java/src/workflows/mod.rs` around lines 125 - 153, `WorkflowRunView` is being reused for bulk listing endpoints, so the list/children JSON unnecessarily includes large or sensitive `params` and `error` fields on every row. Introduce a slimmer view such as `WorkflowRunSummaryView` for `listWorkflowRuns` and `getWorkflowChildren` that keeps only the summary fields (id, definition_id, state, timestamps, and parent metadata), while preserving `WorkflowRunView` with full details for the single-run status path. Update the conversions/return types near `WorkflowRunView` and the list/children handlers to use the new summary type.sdks/java/src/main/java/org/byteveda/taskito/Taskito.java (1)
296-307: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider typing
listWorkflowRuns’sstatefilter asWorkflowState. The backend already uses the enum’s lowercase wire form, so this keeps the public API aligned withWorkflowRunInfo.stateand prevents invalid values from reaching the native layer.DefaultTaskitocan passstate == null ? null : state.wire().🤖 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 `@sdks/java/src/main/java/org/byteveda/taskito/Taskito.java` around lines 296 - 307, Update the public Taskito API so `listWorkflowRuns` uses `WorkflowState` for the `state` filter instead of `String`, aligning it with `WorkflowRunInfo.state` and preventing invalid values from being passed through. Adjust the `Taskito` interface and the `DefaultTaskito.listWorkflowRuns` implementation to convert the enum to its wire value with `state == null ? null : state.wire()`, while keeping the null-no-filter behavior intact.
🤖 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
`@sdks/java/src/main/java/org/byteveda/taskito/dashboard/api/WorkflowsHandlers.java`:
- Around line 27-39: The runs handler is parsing limit and offset with the local
longParam helper, which throws on malformed values and bubbles up as a 500.
Update WorkflowsHandlers.runs to use the shared Http.longParam parser for both
parameters so invalid query values are handled consistently as a 400, and keep
the rest of the listWorkflowRuns flow unchanged.
---
Nitpick comments:
In `@crates/taskito-java/src/workflows/mod.rs`:
- Around line 125-153: `WorkflowRunView` is being reused for bulk listing
endpoints, so the list/children JSON unnecessarily includes large or sensitive
`params` and `error` fields on every row. Introduce a slimmer view such as
`WorkflowRunSummaryView` for `listWorkflowRuns` and `getWorkflowChildren` that
keeps only the summary fields (id, definition_id, state, timestamps, and parent
metadata), while preserving `WorkflowRunView` with full details for the
single-run status path. Update the conversions/return types near
`WorkflowRunView` and the list/children handlers to use the new summary type.
In `@sdks/java/src/main/java/org/byteveda/taskito/Taskito.java`:
- Around line 296-307: Update the public Taskito API so `listWorkflowRuns` uses
`WorkflowState` for the `state` filter instead of `String`, aligning it with
`WorkflowRunInfo.state` and preventing invalid values from being passed through.
Adjust the `Taskito` interface and the `DefaultTaskito.listWorkflowRuns`
implementation to convert the enum to its wire value with `state == null ? null
: state.wire()`, while keeping the null-no-filter behavior intact.
In
`@sdks/java/src/test/java/org/byteveda/taskito/dashboard/DashboardWorkflowTest.java`:
- Around line 25-47: Extend DashboardWorkflowTest to cover the new workflow node
compensation fields and run filtering support. In the existing
DashboardServer/DashboardClient happy-path test around
listsRunsAndReturnsDetail, add assertions that the run detail payload includes
Contract.workflowNode compensation fields such as compensation_job_id,
compensation_started_at, compensation_completed_at, and compensation_error. Also
add coverage for DashboardClient.runs()/the /api/workflows/runs endpoint with
definition_name, state, limit, and offset filters to verify the query plumbing
returns the expected filtered subset.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 50935558-9f5a-4d0a-be7f-fa3e78cc2e21
📒 Files selected for processing (12)
crates/taskito-java/src/workflows/mod.rssdks/java/src/main/java/org/byteveda/taskito/DefaultTaskito.javasdks/java/src/main/java/org/byteveda/taskito/Taskito.javasdks/java/src/main/java/org/byteveda/taskito/dashboard/DashboardServer.javasdks/java/src/main/java/org/byteveda/taskito/dashboard/api/Contract.javasdks/java/src/main/java/org/byteveda/taskito/dashboard/api/WorkflowsHandlers.javasdks/java/src/main/java/org/byteveda/taskito/internal/JniQueueBackend.javasdks/java/src/main/java/org/byteveda/taskito/internal/NativeWorkflows.javasdks/java/src/main/java/org/byteveda/taskito/model/WorkflowRunInfo.javasdks/java/src/main/java/org/byteveda/taskito/spi/QueueBackend.javasdks/java/src/main/java/org/byteveda/taskito/workflows/NodeSnapshot.javasdks/java/src/test/java/org/byteveda/taskito/dashboard/DashboardWorkflowTest.java
|
Addressed in 4f515c1: |
New NativeWorkflows read bindings (listWorkflowRuns, getWorkflowRun, getWorkflowChildren, getWorkflowDag) and the four saga compensation fields on the node view/NodeSnapshot. Dashboard gains /api/workflows/runs, /runs/{id}, /runs/{id}/dag (edge-enriched), /runs/{id}/children. New WorkflowRunInfo model.
Addresses PR review: WorkflowsHandlers.runs used a local longParam that 500'd on malformed limit/offset; route through Http.longParam so bad input returns 400.
4f515c1 to
d487449
Compare
Exposes the workflow run/DAG/children read surface to the dashboard, plus the saga compensation fields on workflow nodes.
Native (Rust JNI,
NativeWorkflows)listWorkflowRuns(definitionName, state, limit, offset)— run summaries, filtered + paged.getWorkflowRun(runId)— a single run summary.getWorkflowChildren(runId)— sub-workflow runs spawned by a run.getWorkflowDag(runId)— the serialized DAG backing a run.NodeViewgains the four saga fields (compensation_job_id,compensation_started_at,compensation_completed_at,compensation_error), already on the coreWorkflowNode.Java + dashboard
Wired through
NativeWorkflows→QueueBackend(default-throwing for non-workflow backends) →JniQueueBackend→Taskito/DefaultTaskito. NewWorkflowRunInfomodel;NodeSnapshotextended with the saga fields.Dashboard routes:
/api/workflows/runs(list),/api/workflows/runs/<id>(run + nodes),/api/workflows/runs/<id>/dag(edge-enriched for the graph view),/api/workflows/runs/<id>/children.Tests
DashboardWorkflowTestsubmits a workflow and asserts the list/detail/DAG/children endpoints. Full Java suite + checkstyle + Spotless +cargo fmt/clippyclean.Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Tests