Skip to content

BCH-1155: Populate ExecutionStreamUUID in WorkflowExecution.GetByID#401

Merged
saltpy-cs merged 3 commits into
mainfrom
fix/BCH-1155
May 26, 2026
Merged

BCH-1155: Populate ExecutionStreamUUID in WorkflowExecution.GetByID#401
saltpy-cs merged 3 commits into
mainfrom
fix/BCH-1155

Conversation

@saltpy-cs
Copy link
Copy Markdown
Contributor

@saltpy-cs saltpy-cs commented May 22, 2026

Add a computed ExecutionStreamUUID field to WorkflowExecution and derive it deterministically in GetByID using a pure SHA256-based helper that mirrors the algorithm already used by EvidenceIntegration.

Summary by CodeRabbit

  • New Features

    • Added a read-only execution_stream_uuid to workflow executions for stable, unique execution identification.
  • Documentation

    • API schemas and docs updated to expose the new execution_stream_uuid field (uuid, read-only, computed).
  • Tests

    • Added tests ensuring the execution_stream_uuid is consistently generated and returned.

Review Change Stack

Add a computed ExecutionStreamUUID field to WorkflowExecution and
derive it deterministically in GetByID using a pure SHA256-based
helper that mirrors the algorithm already used by EvidenceIntegration.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 22, 2026

Caution

Review failed

Failed to post review comments

📝 Walkthrough

Walkthrough

This PR introduces a computed execution_stream_uuid field to workflow executions. The field is derived deterministically from definition, instance, and execution IDs using SHA-256 hashing, integrated into the WorkflowExecution model and service layer, populated in the GetByID method, validated by tests, and documented in the API schema.

Changes

Execution Stream UUID Feature

Layer / File(s) Summary
Deterministic UUID computation algorithm
internal/service/relational/workflows/stream_uuid.go
ComputeExecutionStreamUUID derives a UUID deterministically from definition, instance, and execution IDs by constructing a versioned seed string, hashing with SHA-256, converting to hex, formatting into a UUID string, and parsing the result.
Model definition and service integration
internal/service/relational/workflows/workflow_execution.go, internal/service/relational/workflows/workflow_execution_service.go
WorkflowExecution struct adds ExecutionStreamUUID field (non-persisted, JSON-exposed); GetByID conditionally populates it by calling ComputeExecutionStreamUUID when all required IDs and related entities are present.
Service test validation
internal/service/relational/workflows/workflow_execution_service_test.go
TestWorkflowExecutionService_GetByID_PopulatesExecutionStreamUUID verifies the field is populated with the deterministic UUID and that repeated calls return the same value for the same execution.
API schema documentation updates
docs/swagger.json, docs/swagger.yaml, docs/docs.go
Swagger, YAML, and generated docs schemas document the new execution_stream_uuid field as a computed (non-persisted), read-only string with format: "uuid".

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested reviewers

  • gusfcarvalho

Poem

I nibble code where hashes bloom,
Seeds of IDs in a secret room,
SHA-256 hums a steady tune,
A UUID appears—no DB tomb,
Docs and tests celebrate under moon 🐇✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: adding and populating ExecutionStreamUUID in WorkflowExecution.GetByID, which is the core purpose of this pull request.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 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 `@docs/swagger.json`:
- Around line 40223-40226: The schema for the computed field
execution_stream_uuid currently only declares "type": "string" but should
declare a UUID format to enforce the intended shape; update the JSON schema for
the execution_stream_uuid property to add "format": "uuid" alongside "type":
"string" so generated clients and validators will treat it as a UUID.

In `@docs/swagger.yaml`:
- Around line 8922-8924: The OpenAPI schema entry for execution_stream_uuid is
too permissive; update the execution_stream_uuid property (in docs/swagger.yaml)
to include format: uuid and readOnly: true while keeping type: string and the
existing description ("Computed field (not persisted)") so generated clients and
validators treat it as a server-computed UUID-only, read-only field.

In `@internal/service/relational/workflows/stream_uuid.go`:
- Around line 23-24: The code currently ignores the error from uuid.Parse when
building streamUUID (streamUUID, _ := uuid.Parse(...)); change it to capture and
handle the error (streamUUID, err := uuid.Parse(...)) and propagate it by
updating the function signature to return (uuid.UUID, error) or otherwise return
uuid.Nil with a wrapped error; update callers to handle the error so a malformed
UUID assembly cannot silently produce a uuid.Nil used for execution_stream_uuid.

In `@internal/service/relational/workflows/workflow_execution_service_test.go`:
- Around line 119-127: The test currently only asserts stability of
ExecutionStreamUUID across fetches; update it to also assert the canonical
expected UUID and guard against nil on the second retrieval. After calling
retrieved2, require.NoError(t, err) (already present) and add require.NotNil(t,
retrieved2.ExecutionStreamUUID) before dereferencing it, then compute the
expected UUID using the same deterministic generator the service uses (e.g., the
function that produces the execution stream UUID in your implementation — call
it directly, e.g., computeExecutionStreamUUID(execution) or
ExecutionStreamUUIDFor(execution)) and assert assert.Equal(t, expectedUUID,
*retrieved2.ExecutionStreamUUID) instead of only comparing retrieved to
retrieved2.
🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d1e82e79-2c59-4f5d-a466-1f6014476acc

📥 Commits

Reviewing files that changed from the base of the PR and between 7597d41 and 9dd6b9a.

📒 Files selected for processing (7)
  • docs/docs.go
  • docs/swagger.json
  • docs/swagger.yaml
  • internal/service/relational/workflows/stream_uuid.go
  • internal/service/relational/workflows/workflow_execution.go
  • internal/service/relational/workflows/workflow_execution_service.go
  • internal/service/relational/workflows/workflow_execution_service_test.go

Comment thread docs/swagger.json
Comment thread docs/swagger.yaml
Comment thread internal/service/relational/workflows/stream_uuid.go Outdated
Comment thread internal/service/relational/workflows/workflow_execution_service_test.go Outdated
saltpy-cs and others added 2 commits May 26, 2026 09:27
- Return error from ComputeExecutionStreamUUID instead of discarding it
- Propagate the error in GetByID
- Assert canonical computed UUID value in test and add nil guard for
  second retrieval pointer
- Add format:uuid and readOnly:true to ExecutionStreamUUID struct tag
  so generated swagger docs tighten the API contract

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@saltpy-cs saltpy-cs merged commit f236d42 into main May 26, 2026
5 checks passed
@saltpy-cs saltpy-cs deleted the fix/BCH-1155 branch May 26, 2026 08:41
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