feat: workflow templates and versioning with diff and rollback#1069
feat: workflow templates and versioning with diff and rollback#1069
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughAdds starter workflow blueprints and workflow versioning features across the codebase. Backend: blueprint loader, blueprint models, built-in YAML blueprints, blueprint listing and create-from-blueprint endpoints, a new WorkflowVersionController (list/get/diff/rollback), version snapshot construction helpers, observability events, and SQLite schema/repository plus backend wiring and protocol. Engine: workflow diff and immutable version model. Frontend: API clients, blueprint picker/create drawer, version history panel, diff viewer, store actions. Tests and test fakes added. Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Code Review
This pull request implements a comprehensive workflow versioning and blueprint system, allowing users to track history, compute structural diffs, and perform rollbacks. It introduces backend logic for immutable snapshots and a diff engine, alongside frontend components for version management and a template-based creation flow. Review feedback focuses on improving maintainability by deduplicating utility functions across controllers and making the blueprint storage path configurable. Additionally, the reviewer identified a performance optimization to remove redundant API calls in the blueprint picker and a reliability improvement to ensure atomic database transactions when saving rolled-back definitions and their corresponding snapshots.
| def _get_auth_user_id(request: Request[Any, Any, Any]) -> str: | ||
| """Extract the authenticated user ID from a request.""" | ||
| auth_user = request.scope.get("user") | ||
| if isinstance(auth_user, AuthenticatedUser): | ||
| return auth_user.user_id | ||
| return "api" | ||
|
|
||
|
|
||
| def _build_version_snapshot( | ||
| definition: WorkflowDefinition, | ||
| saved_by: str, | ||
| ) -> WorkflowDefinitionVersion: | ||
| """Build a version snapshot from a definition. | ||
|
|
||
| The snapshot's ``saved_at`` is set to the definition's | ||
| ``updated_at`` timestamp, not the current time. | ||
| """ | ||
| return WorkflowDefinitionVersion( | ||
| definition_id=definition.id, | ||
| version=definition.version, | ||
| name=definition.name, | ||
| description=definition.description, | ||
| workflow_type=definition.workflow_type, | ||
| nodes=definition.nodes, | ||
| edges=definition.edges, | ||
| created_by=definition.created_by, | ||
| saved_by=saved_by, | ||
| saved_at=definition.updated_at, | ||
| ) |
There was a problem hiding this comment.
The utility functions _get_auth_user_id and _build_version_snapshot are duplicated between this file and src/synthorg/api/controllers/workflows.py. To improve maintainability and ensure consistency in how snapshots are constructed, these should be moved to a shared utility module or a common base class for workflow-related controllers.
|
|
||
| logger = get_logger(__name__) | ||
|
|
||
| _USER_BLUEPRINTS_DIR = Path.home() / ".synthorg" / "workflow-blueprints" |
There was a problem hiding this comment.
The _USER_BLUEPRINTS_DIR is hardcoded to a path in the user's home directory. This can cause issues in environments where the home directory is not writable or where a different storage location is expected (e.g., containerized deployments). This path should be made configurable via the application settings.
| useEffect(() => { | ||
| if (open && mode === 'blueprint' && blueprintCache.length === 0) { | ||
| listBlueprints() | ||
| .then(setBlueprintCache) | ||
| .catch((err: unknown) => { log.warn('Failed to pre-fetch blueprints', err) }) | ||
| } | ||
| }, [open, mode, blueprintCache.length]) |
There was a problem hiding this comment.
This useEffect pre-fetches blueprints into a local cache, but the BlueprintPicker component also performs its own fetch of the same data when it mounts. This results in redundant API calls when the user switches to blueprint mode. Consider lifting the blueprint state or passing the blueprintCache down to BlueprintPicker as a prop to avoid unnecessary network traffic.
| await repo.save(rolled_back) | ||
| except VersionConflictError as exc: | ||
| logger.warning( | ||
| WORKFLOW_DEF_VERSION_CONFLICT, | ||
| definition_id=workflow_id, | ||
| error=str(exc), | ||
| ) | ||
| return Response( | ||
| content=ApiResponse[WorkflowDefinition]( | ||
| error=f"Version conflict during rollback: {exc}", | ||
| ), | ||
| status_code=409, | ||
| ) | ||
|
|
||
| snapshot = _build_version_snapshot(rolled_back, updater) | ||
| await version_repo.save_version(snapshot) |
There was a problem hiding this comment.
The workflow definition save and the version snapshot save are performed as separate operations with individual commits. If the snapshot save fails after the definition save succeeds, the system is left in an inconsistent state where a new version exists without a corresponding historical record. Since both repositories share the same underlying SQLite connection, these operations should be wrapped in a single transaction to ensure atomicity.
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches. Re-running this action after a short time may resolve the issue. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
There was a problem hiding this comment.
Pull request overview
Adds workflow starter templates (“blueprints”) and immutable workflow version history (snapshots on every save) with API + UI support for listing versions, viewing diffs, and rolling back.
Changes:
- Introduces built-in + user-discovered workflow blueprints, plus API endpoints to list and instantiate them.
- Adds workflow definition version snapshot persistence (SQLite table + repository), version history API, structural diff computation, and rollback endpoint.
- Updates the web dashboard to support “create from template”, version history drawer, and diff viewer modal.
Reviewed changes
Copilot reviewed 51 out of 51 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| web/src/stores/workflows.ts | Adds store action to create a workflow from a blueprint and update the workflow list. |
| web/src/stores/workflow-editor.ts | Adds version history/diff/rollback state + actions to the workflow editor store. |
| web/src/pages/workflows/WorkflowCreateDrawer.tsx | Adds “Blank vs From Template” creation mode and wires blueprint-based creation. |
| web/src/pages/workflows/BlueprintPicker.tsx | New UI component to fetch and display available blueprints. |
| web/src/pages/workflows/BlueprintPicker.stories.tsx | Storybook coverage for BlueprintPicker states (loading/empty/error/selected). |
| web/src/pages/WorkflowEditorPage.tsx | Wires version history + diff viewer components into the workflow editor page. |
| web/src/pages/workflow-editor/WorkflowToolbar.tsx | Adds “History” toolbar action to open version history. |
| web/src/pages/workflow-editor/VersionHistoryPanel.tsx | New drawer UI listing versions with compare + restore actions. |
| web/src/pages/workflow-editor/VersionHistoryPanel.stories.tsx | Storybook coverage for VersionHistoryPanel open/closed. |
| web/src/pages/workflow-editor/VersionDiffViewer.tsx | New modal UI to display structural diffs between versions. |
| web/src/pages/workflow-editor/VersionDiffViewer.stories.tsx | Storybook entry for VersionDiffViewer. |
| web/src/api/types.ts | Adds frontend API types for blueprints, versions, diffs, and rollback requests. |
| web/src/api/endpoints/workflows.ts | Adds frontend API calls for blueprints + versioning (list/get/diff/rollback). |
| tests/unit/persistence/sqlite/test_workflow_version_repo.py | Unit tests for SQLite workflow version repository CRUD + pagination. |
| tests/unit/persistence/sqlite/test_migrations.py | Extends migration assertions for new versions table + index. |
| tests/unit/observability/test_events.py | Registers new observability event domains (blueprint, workflow_version). |
| tests/unit/engine/workflow/test_version.py | Unit tests for immutable WorkflowDefinitionVersion model constraints. |
| tests/unit/engine/workflow/test_diff.py | Unit tests for compute_diff behavior across node/edge/metadata changes. |
| tests/unit/engine/workflow/test_blueprint_models.py | Unit tests for blueprint schema validation and invariants. |
| tests/unit/engine/workflow/test_blueprint_loader.py | Unit tests for blueprint discovery/loading, overrides, and error handling. |
| tests/unit/api/fakes.py | Adds FakeWorkflowVersionRepository and exposes it via FakePersistenceBackend. |
| tests/unit/api/controllers/test_workflow_versions.py | Adds API tests for versions listing, get, diff, and rollback error paths. |
| tests/unit/api/controllers/test_workflow_blueprints.py | Adds API tests for blueprint listing and blueprint instantiation. |
| tests/unit/api/auth/test_session_store.py | Stabilizes time-dependent test by patching “now” during load_revoked(). |
| src/synthorg/persistence/workflow_version_repo.py | Introduces persistence protocol for workflow version repositories. |
| src/synthorg/persistence/sqlite/workflow_version_repo.py | SQLite implementation for version snapshot persistence + observability logs. |
| src/synthorg/persistence/sqlite/schema.sql | Adds workflow_definition_versions table + index and FK cascade behavior. |
| src/synthorg/persistence/sqlite/backend.py | Wires SQLiteWorkflowVersionRepository into the SQLite backend. |
| src/synthorg/observability/events/workflow_version.py | Adds persistence event constants for workflow version repository operations. |
| src/synthorg/observability/events/workflow_definition.py | Adds workflow-definition-level events for version list/diff/rollback. |
| src/synthorg/observability/events/blueprint.py | Adds blueprint-related event constants for loading/listing/instantiation. |
| src/synthorg/engine/workflow/version.py | Adds immutable WorkflowDefinitionVersion snapshot model. |
| src/synthorg/engine/workflow/diff.py | Adds structural diff computation + diff result models + invariants. |
| src/synthorg/engine/workflow/blueprints/feature-pipeline.yaml | Adds built-in “Feature Pipeline” blueprint. |
| src/synthorg/engine/workflow/blueprints/bug-fix-flow.yaml | Adds built-in “Bug Fix Flow” blueprint. |
| src/synthorg/engine/workflow/blueprints/research-sprint.yaml | Adds built-in “Research Sprint” blueprint. |
| src/synthorg/engine/workflow/blueprints/code-review-pipeline.yaml | Adds built-in “Code Review Pipeline” blueprint. |
| src/synthorg/engine/workflow/blueprints/onboarding-workflow.yaml | Adds built-in “Onboarding Workflow” blueprint. |
| src/synthorg/engine/workflow/blueprints/init.py | Makes blueprints discoverable via importlib.resources. |
| src/synthorg/engine/workflow/blueprint_models.py | Adds validated blueprint graph models (nodes/edges/blueprint). |
| src/synthorg/engine/workflow/blueprint_loader.py | Adds blueprint discovery/loading from built-ins and user dir with safety checks. |
| src/synthorg/engine/workflow/blueprint_errors.py | Adds blueprint-specific error types. |
| src/synthorg/api/dto.py | Adds DTOs for blueprint listing, create-from-blueprint, and rollback requests. |
| src/synthorg/api/controllers/workflows.py | Adds blueprint list + instantiate endpoints and auto-snapshotting on create/update/delete. |
| src/synthorg/api/controllers/workflow_versions.py | Adds version list/get/diff/rollback endpoints. |
| src/synthorg/api/controllers/init.py | Registers WorkflowVersionController. |
| README.md | Updates “Real workflows” feature description to mention templates + version history. |
| docs/design/page-structure.md | Documents workflows list create modes and editor version history/diff/rollback UI. |
| docs/design/operations.md | Updates event taxonomy count and includes new domains. |
| docs/design/engine.md | Updates workflow persistence section to reflect new endpoints. |
| CLAUDE.md | Updates repository structure and logging/event guidance to include new domains/features. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const bp = blueprintCache.find((b) => b.name === name) | ||
| if (bp) { | ||
| setForm((prev) => ({ | ||
| ...prev, | ||
| name: bp.display_name, |
There was a problem hiding this comment.
WorkflowCreateDrawer prefetches blueprints into blueprintCache, but BlueprintPicker fetches its own list internally. If the picker loads before blueprintCache is populated (or prefetch fails), selecting a template will not auto-populate the form because handleBlueprintSelect looks up the selection only in blueprintCache. Consider making BlueprintPicker consume the cached list via props (single source of truth), or have the picker report the selected blueprint object so the drawer doesn’t depend on a separate async cache.
| className="fixed inset-0 z-50 flex items-center justify-center bg-black/50" | ||
| role="dialog" | ||
| aria-modal="true" | ||
| aria-labelledby="diff-viewer-title" | ||
| onKeyDown={(e) => { if (e.key === 'Escape') clearDiff() }} |
There was a problem hiding this comment.
onKeyDown on the dialog backdrop will only fire reliably if that element has focus; as written, Escape may not close the diff viewer for keyboard users. Consider focusing the dialog on mount (e.g., tabIndex={-1} + ref.focus()), or attaching an Escape key listener at the document/window level while the modal is open (and removing it on unmount).
web/src/stores/workflow-editor.ts
Outdated
| set({ versionsLoading: true }) | ||
| try { | ||
| const result = await listWorkflowVersions(defn.id, { limit: 50 }) | ||
| set({ versions: result.data, versionsLoading: false }) | ||
| } catch (err) { |
There was a problem hiding this comment.
loadVersions can race with workflow switching: it captures defn before the await, but after the request resolves it unconditionally writes versions into state. If the user navigates to a different workflow mid-flight, this can populate the new workflow’s UI with the old workflow’s version list. Add a request token / captured definition id check (similar to web/src/stores/workflows.ts’s _listRequestToken) before calling set(...).
web/src/stores/workflow-editor.ts
Outdated
| set({ diffLoading: true }) | ||
| try { | ||
| const diff = await getWorkflowDiff(defn.id, fromVersion, toVersion) | ||
| set({ diffResult: diff, diffLoading: false }) | ||
| } catch (err) { |
There was a problem hiding this comment.
Same race as loadVersions: loadDiff uses the current definition before the await, but after the request resolves it always sets diffResult. If the workflow changes (or the user closes history/diff) mid-request, the diff viewer may open with a result for the wrong workflow. Add a request token / captured definition id + version pair check before updating state, and consider clearing diffResult when starting a new diff load to avoid showing stale data on failure.
| <div className="flex items-center gap-2 text-xs text-muted"> | ||
| <Clock className="size-3" /> | ||
| <span>{formatRelativeTime(v.saved_at)}</span> | ||
| <span>by {v.saved_by}</span> |
There was a problem hiding this comment.
The timestamp row uses decorative icons without aria-hidden, which can add noise for screen reader users. Consider adding aria-hidden="true" to the Clock icon and (optionally) rendering the timestamp in a <time dateTime={v.saved_at}>…</time> element for better semantics.
| ) -> Response[PaginatedResponse[WorkflowDefinitionVersion]]: | ||
| """List version history for a workflow definition.""" | ||
| version_repo = state.app_state.persistence.workflow_versions | ||
| versions = await version_repo.list_versions( | ||
| workflow_id, |
There was a problem hiding this comment.
list_versions returns full WorkflowDefinitionVersion objects (including full nodes/edges) for every entry. For workflows with larger graphs and/or many versions, this will make the history drawer endpoint unnecessarily heavy, even though the UI only needs a small subset (version/name/saved_at/saved_by). Consider returning a lightweight summary DTO for the list endpoint (and reserving full graph snapshots for GET /versions/{num}), or adding an opt-in query param to include the graph data.
| # Tested via direct unit test of the rollback logic | ||
| # (Litestar TestClient + xdist hangs on multi-step rollback flows). | ||
| wf = _create_workflow(test_client, name="Original") | ||
| wf_id = wf["id"] | ||
|
|
||
| # Verify version 1 snapshot was auto-created. | ||
| v_resp = test_client.get( | ||
| f"/api/v1/workflows/{wf_id}/versions/1", | ||
| headers=make_auth_headers("ceo"), | ||
| ) | ||
| assert v_resp.status_code == 200 | ||
| assert v_resp.json()["data"]["name"] == "Original" | ||
|
|
There was a problem hiding this comment.
test_rollback_success doesn’t exercise the rollback endpoint/logic (it only verifies that version 1 was auto-snapshotted on create). This leaves the core rollback behavior (content restored, version incremented, new snapshot created, etc.) untested. Add a test that creates v1, updates to v2, calls POST /rollback to v1 with expected_version=2, and asserts the returned definition matches v1 content and that version history contains a new v3 snapshot.
| # Tested via direct unit test of the rollback logic | |
| # (Litestar TestClient + xdist hangs on multi-step rollback flows). | |
| wf = _create_workflow(test_client, name="Original") | |
| wf_id = wf["id"] | |
| # Verify version 1 snapshot was auto-created. | |
| v_resp = test_client.get( | |
| f"/api/v1/workflows/{wf_id}/versions/1", | |
| headers=make_auth_headers("ceo"), | |
| ) | |
| assert v_resp.status_code == 200 | |
| assert v_resp.json()["data"]["name"] == "Original" | |
| wf = _create_workflow(test_client, name="Original") | |
| wf_id = wf["id"] | |
| updated = _update_workflow(test_client, wf_id, name="Updated", expected_version=1) | |
| assert updated["name"] == "Updated" | |
| resp = test_client.post( | |
| f"/api/v1/workflows/{wf_id}/rollback", | |
| json={"target_version": 1, "expected_version": 2}, | |
| headers=make_auth_headers("ceo"), | |
| ) | |
| assert resp.status_code == 200 | |
| data = resp.json()["data"] | |
| assert data["name"] == "Original" | |
| if "version" in data: | |
| assert data["version"] == 3 | |
| if "current_version" in data: | |
| assert data["current_version"] == 3 | |
| v1_resp = test_client.get( | |
| f"/api/v1/workflows/{wf_id}/versions/1", | |
| headers=make_auth_headers("ceo"), | |
| ) | |
| assert v1_resp.status_code == 200 | |
| assert v1_resp.json()["data"]["name"] == "Original" | |
| v3_resp = test_client.get( | |
| f"/api/v1/workflows/{wf_id}/versions/3", | |
| headers=make_auth_headers("ceo"), | |
| ) | |
| assert v3_resp.status_code == 200 | |
| assert v3_resp.json()["data"]["name"] == "Original" |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1069 +/- ##
==========================================
- Coverage 89.65% 89.59% -0.07%
==========================================
Files 731 742 +11
Lines 42698 43408 +710
Branches 4287 4359 +72
==========================================
+ Hits 38282 38892 +610
- Misses 3663 3745 +82
- Partials 753 771 +18 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 24
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/api/controllers/workflow_versions.py`:
- Around line 39-67: The two helper functions _get_auth_user_id(request:
Request) and _build_version_snapshot(definition: WorkflowDefinition, saved_by:
str) are duplicated from workflows.py; extract them into a shared module (e.g.,
synthorg.api.utils or synthorg.api.helpers) and replace the local definitions
with imports. Update references in workflow_versions.py (the functions named
_get_auth_user_id and _build_version_snapshot) to import and call the shared
implementations, keeping signatures and behavior identical (including returning
"api" fallback and using definition.updated_at for saved_at).
- Around line 201-262: In rollback_workflow, add validation before fetching the
target version to reject any rollback where data.target_version >=
data.expected_version (i.e., target is the current or a future version); log a
warning (reuse or create a suitable log key like WORKFLOW_DEF_INVALID_ROLLBACK)
with definition_id, expected and target, and return a
Response(ApiResponse(error="Target version must be strictly less than expected
version"), status_code=400) so callers cannot roll forward or to the current
version. Ensure you reference data.expected_version, data.target_version and
rollback_workflow when making this change.
In `@src/synthorg/api/controllers/workflows.py`:
- Around line 114-142: The helpers _build_version_snapshot and _get_auth_user_id
are duplicated; extract them into a shared module (e.g.,
synthorg.api.controllers._workflow_helpers) as
build_version_snapshot(definition: WorkflowDefinition, saved_by: str) ->
WorkflowDefinitionVersion and get_auth_user_id(request: Request) -> str, move
the logic from _build_version_snapshot and _get_auth_user_id into those
functions, then update both controllers (workflows.py and workflow_versions.py)
to import and call build_version_snapshot and get_auth_user_id instead of their
local copies to avoid duplication and drift.
In `@src/synthorg/api/dto.py`:
- Around line 716-724: CreateFromBlueprintRequest currently accepts unbounded
text for blueprint_name, name, and description; add Field-level max_length
constraints so validation fails at the DTO boundary. Update the blueprint_name:
NotBlankStr = Field(description=...) to include a max_length (e.g.,
max_length=255), update name: NotBlankStr | None = Field(default=None,
description=...) to include max_length=255, and update description: str | None =
Field(default=None, description=...) to include a reasonable max_length (e.g.,
max_length=1024) so Pydantic enforces length limits during
CreateFromBlueprintRequest validation.
In `@src/synthorg/engine/workflow/blueprint_loader.py`:
- Around line 232-247: The YAML parsing and validation branches raise
BlueprintValidationError without logging; update the except block around
yaml.safe_load, the check for missing top-level "blueprint", and the except
around BlueprintData.model_validate to emit a logger.warning or logger.error
including source_name and the exception details before raising
BlueprintValidationError. Specifically, log the parse error in the
yaml.safe_load except (referencing yaml.safe_load and the exc), log a warning
when raw is not a dict or missing "blueprint" (referencing raw and source_name),
and log the validation failure caught from BlueprintData.model_validate
(referencing BlueprintData.model_validate and exc), then re-raise the same
BlueprintValidationError as before.
- Around line 297-305: The current loader in blueprint_loader.py returns a
parsed blueprint even when the filename stem (name) disagrees with data.name,
causing listing vs loading mismatch; change this to reject the mismatch by
logging an error (using BLUEPRINT_LIST with blueprint_name=name and
parsed_name=data.name and action="name_mismatch_error") and raising an exception
(e.g., ValueError or a BlueprintLoadError) instead of returning data so callers
like list_blueprints() and load_blueprint() cannot treat the inconsistent file
as valid; update any callers if they expect exceptions.
- Around line 143-147: Replace the direct construction of user_path in
load_blueprint() with the same user-blueprint resolver used by
list_blueprints(): canonicalize the requested name to the normalized stem and
resolve it to a safe file path (rejecting symlink escapes) instead of directly
doing _USER_BLUEPRINTS_DIR / f"{name_clean}.yaml"; then check the resolved path
is a file and call _load_from_file(resolved_path, name_clean). Ensure you call
the shared resolver (the one list_blueprints() uses) so name canonicalization
and symlink safety are applied consistently.
In `@src/synthorg/engine/workflow/diff.py`:
- Around line 44-58: Extract the duplicated validation logic into a single
helper function (e.g., _validate_change_values) and call it from both
NodeChange._validate_values and EdgeChange._validate_values; specifically move
the presence rules for change_type/old_value/new_value into the shared function
(accepting change_type, old_value, new_value) and have each
`@model_validator`(method) simply invoke that helper and return self, leaving the
validator decorators and model classes (NodeChange, EdgeChange) intact.
In `@src/synthorg/persistence/sqlite/workflow_version_repo.py`:
- Around line 65-68: The code block using datetime.fromisoformat on
data["saved_at"] and then replacing tzinfo with UTC when tzinfo is None
implicitly treats naive timestamps as UTC; add a clear note in the module or the
WorkflowVersionRepo/class docstring explaining this convention (that malformed
ISO strings will raise ValueError and are handled by the outer except, and that
naive datetimes are interpreted as UTC via UTC replacement), referencing
datetime.fromisoformat, UTC and data["saved_at"] so future readers understand
the edge-case behavior and rationale.
In `@tests/unit/api/controllers/test_workflow_versions.py`:
- Around line 208-220: The test test_rollback_success currently only checks the
auto-created version snapshot and never calls the rollback endpoint, so add an
explicit call to POST /api/v1/workflows/{wf_id}/rollback in
test_rollback_success (using the same TestClient and auth headers) and assert
the response status and that the workflow state or current version reflects the
rolled-back values; alternatively, if you prefer not to exercise the HTTP
endpoint here, rename or remove test_rollback_success and ensure the rollback
success path is covered by the lower-level unit test it references (the direct
rollback logic test), updating assertions there to validate the rollback
outcome.
In `@tests/unit/api/fakes.py`:
- Around line 601-614: list_versions currently returns direct references from
self._versions which leaks internal fake-repo state; update the list_versions
method to return deep-copied snapshots (use copy.deepcopy) of the matching
WorkflowDefinitionVersion objects—matching the behavior of get_version—so you
return a tuple of new objects (not originals) sliced by offset/limit and, if
applicable, enforce read-only semantics on the returned copies.
In `@web/src/pages/workflow-editor/VersionDiffViewer.stories.tsx`:
- Around line 4-12: The story for VersionDiffViewer is missing the required
accessibility test parameter and lacks additional states; update the Default
story object (and export new stories) to include parameters.a11y.test set to one
of 'error' | 'todo' | 'off' and add separate Story exports for Loading, Error,
and Empty states that exercise VersionDiffViewer with props/state reflecting a
pending diff, a diff computation failure, and no differences respectively (use
the component's props or mock data used elsewhere to simulate these states and
ensure each story includes the same parameters.a11y.test value).
In `@web/src/pages/workflow-editor/VersionDiffViewer.tsx`:
- Around line 77-91: Extract the inline JSX rendered inside metadata_changes.map
into a new presentational component (e.g., MetadataChangeRow) that accepts props
for field, old_value, and new_value (define a small props interface/type),
preserve the existing className and markup (rounded-md bg-card p-2 text-sm,
font-medium text-foreground, text-danger line-through, text-success), and then
replace the map body in VersionDiffViewer with a single <MetadataChangeRow
key={mc.field} {...mc} /> call; you can define MetadataChangeRow either as a
small function component in the same file or export it for reuse, ensuring prop
names match the objects emitted by diffResult.metadata_changes.
- Around line 40-47: The modal in VersionDiffViewer.tsx is implemented as a
custom overlay that bypasses shared dialog behavior and hardcodes a color class;
replace the outer custom div with the project's shared dialog primitive from
web/src/components/ui/ (e.g., Dialog/DialogTrigger/DialogContent or the
equivalent exported dialog component) to restore focus management and keyboard
semantics, wire the Escape handler to the dialog's onClose (calling clearDiff)
rather than a raw onKeyDown, and swap the hardcoded bg-black/50 with the
tokenized semantic background class used by the ui dialog (use the dialog's
provided overlay/content classes or the project's design tokens) so
accessibility and styling conventions are preserved.
In `@web/src/pages/workflow-editor/VersionHistoryPanel.stories.tsx`:
- Around line 4-11: The story meta for VersionHistoryPanel is missing an
accessibility test configuration; update the meta object (the constant named
meta for VersionHistoryPanel) to include a parameters.a11y.test entry that
defines the storybook a11y testing behavior (for example enable/disable or
specific axe rules/thresholds) so that Storybook runs the prescribed
accessibility checks for this story; ensure you add the parameters.a11y.test
field alongside the existing title/component/args properties in the meta object.
In `@web/src/pages/workflow-editor/VersionHistoryPanel.tsx`:
- Around line 64-113: The mapped JSX for each version in VersionHistoryPanel is
too large and should be extracted into a child component; create a new
VersionCard (or similar) functional component that accepts a version object (v),
the current definition (definition), saving flag (saving), and the handlers
handleCompare and handleRestore as props, move the entire JSX returned from
versions.map into that component, then replace the map body with versions.map(v
=> <VersionCard v={v} definition={definition} saving={saving}
onCompare={handleCompare} onRestore={handleRestore} />) ensuring keys remain
(use v.version) and preserve the conditional disables and titles.
In `@web/src/pages/workflows/BlueprintPicker.stories.tsx`:
- Around line 28-44: The Storybook meta for BlueprintPicker is missing the
accessibility test setting; update the meta object (const meta) to include
parameters.a11y.test and set it to the appropriate value ('error' | 'todo' |
'off')—e.g., add parameters: { msw: { ... }, a11y: { test: 'error' } } so
Storybook will run the configured a11y checks for the BlueprintPicker stories.
In `@web/src/pages/workflows/BlueprintPicker.tsx`:
- Around line 16-20: Export the BlueprintPickerProps interface so consumers can
import and use it: modify the declaration of BlueprintPickerProps (the props
interface used by the BlueprintPicker component) to be exported (e.g., add the
export keyword) and ensure any local references (such as the BlueprintPicker
component signature and any imports/exports that rely on this type) continue to
compile; adjust any nearby type-only imports/exports if needed to satisfy
TypeScript.
- Around line 88-122: The map callback in BlueprintPicker.tsx renders a ~34-line
JSX card inside filtered.map, which violates the guideline to keep map JSX under
~8 lines; extract that JSX into a new BlueprintCard component (e.g.,
BlueprintCard) that accepts props blueprint: BlueprintInfo, isSelected: boolean,
and onSelect: () => void, and move usages of cardEntrance, formatLabel,
StatPill, and FileCode2 inside the new component so the parent uses
{filtered.map(bp => <BlueprintCard key={bp.name} blueprint={bp}
isSelected={selectedBlueprint === bp.name} onSelect={() =>
onSelect(selectedBlueprint === bp.name ? null : bp.name)} />)}. Ensure
BlueprintCard reads bp.display_name, bp.workflow_type, bp.description,
bp.node_count, bp.edge_count and applies the same cn className logic based on
isSelected so behavior and styling remain identical.
In `@web/src/pages/workflows/WorkflowCreateDrawer.tsx`:
- Around line 57-64: The useEffect in WorkflowCreateDrawer that calls
listBlueprints and manages blueprintCache duplicates fetching logic and should
be moved into the BlueprintPicker component: remove the useEffect and
blueprintCache state from WorkflowCreateDrawer, let BlueprintPicker call
listBlueprints internally (or accept responsibility for fetching from stories),
and change the contract so BlueprintPicker returns the selected full blueprint
object (update handleBlueprintSelect to accept the object instead of just name).
Keep listBlueprints usage and error handling inside BlueprintPicker and adapt
WorkflowCreateDrawer to use the selected blueprint object for pre-populating
fields if needed.
- Around line 101-115: The if-condition that checks "mode === 'blueprint' &&
selectedBlueprint" is redundant because an earlier guard already ensures
selectedBlueprint is truthy when mode is 'blueprint'; simplify the conditional
to just "if (mode === 'blueprint')" and call
useWorkflowsStore.getState().createFromBlueprint with the same payload (using
selectedBlueprint) to preserve behavior; update the conditional in the block
containing createFromBlueprint/createWorkflow to remove the && selectedBlueprint
check.
In `@web/src/stores/workflow-editor.ts`:
- Around line 568-591: Both loadVersions and loadDiff must guard against stale
async responses: capture the current definition id (const defn =
get().definition; const reqId = Symbol() or const capturedDefnId = defn.id)
before awaiting listWorkflowVersions/getWorkflowDiff and store a request token
or the captured defnId/versions in closure; after the await, verify the
token/that get().definition?.id === capturedDefnId (and for loadDiff also verify
get().diffRequestFrom === fromVersion && get().diffRequestTo === toVersion or
compare a saved token) before calling set(...) to update versions, diffResult,
and loading flags so older responses cannot overwrite newer state; ensure you
set the token into state when starting the request and clear it on completion or
mismatch.
- Around line 573-574: The current call to listWorkflowVersions(defn.id, {
limit: 50 }) in the workflow editor only fetches the first 50 versions; add
paginated state and a "load more" action so older snapshots can be fetched.
Introduce state fields (e.g., versionsCursor or versionsPage and
versionsHasMore) alongside versions and versionsLoading, update the initial
fetch (listWorkflowVersions) to store the returned cursor/hasMore info, and
implement a loadMoreVersions action that calls listWorkflowVersions with the
cursor/page, appends new items to versions (don’t overwrite), updates
versionsLoading and versionsHasMore accordingly, and handles the
empty/end-of-list case. Ensure the existing set(...) calls in this file update
the new state fields and that UI code can call loadMoreVersions to fetch next
pages.
- Around line 607-610: After a successful rollback the store's diffResult is
left showing pre-rollback differences; update the rollback success path (the
code that currently calls set({ definition: updated, saving: false, dirty: false
}) and then awaits useWorkflowEditorStore.getState().loadDefinition(updated.id)
and loadVersions()) to clear the diff state as well — either include diffResult:
null in the initial set call or explicitly set diffResult: null after the
loadDefinition/loadVersions calls so diffResult is reset and the comparison UI
is not stale.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 34bd3138-5e5e-4f2e-bfe8-3f907baee1ef
📒 Files selected for processing (51)
CLAUDE.mdREADME.mddocs/design/engine.mddocs/design/operations.mddocs/design/page-structure.mdsrc/synthorg/api/controllers/__init__.pysrc/synthorg/api/controllers/workflow_versions.pysrc/synthorg/api/controllers/workflows.pysrc/synthorg/api/dto.pysrc/synthorg/engine/workflow/blueprint_errors.pysrc/synthorg/engine/workflow/blueprint_loader.pysrc/synthorg/engine/workflow/blueprint_models.pysrc/synthorg/engine/workflow/blueprints/__init__.pysrc/synthorg/engine/workflow/blueprints/bug-fix-flow.yamlsrc/synthorg/engine/workflow/blueprints/code-review-pipeline.yamlsrc/synthorg/engine/workflow/blueprints/feature-pipeline.yamlsrc/synthorg/engine/workflow/blueprints/onboarding-workflow.yamlsrc/synthorg/engine/workflow/blueprints/research-sprint.yamlsrc/synthorg/engine/workflow/diff.pysrc/synthorg/engine/workflow/version.pysrc/synthorg/observability/events/blueprint.pysrc/synthorg/observability/events/workflow_definition.pysrc/synthorg/observability/events/workflow_version.pysrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/persistence/sqlite/schema.sqlsrc/synthorg/persistence/sqlite/workflow_version_repo.pysrc/synthorg/persistence/workflow_version_repo.pytests/unit/api/auth/test_session_store.pytests/unit/api/controllers/test_workflow_blueprints.pytests/unit/api/controllers/test_workflow_versions.pytests/unit/api/fakes.pytests/unit/engine/workflow/test_blueprint_loader.pytests/unit/engine/workflow/test_blueprint_models.pytests/unit/engine/workflow/test_diff.pytests/unit/engine/workflow/test_version.pytests/unit/observability/test_events.pytests/unit/persistence/sqlite/test_migrations.pytests/unit/persistence/sqlite/test_workflow_version_repo.pyweb/src/api/endpoints/workflows.tsweb/src/api/types.tsweb/src/pages/WorkflowEditorPage.tsxweb/src/pages/workflow-editor/VersionDiffViewer.stories.tsxweb/src/pages/workflow-editor/VersionDiffViewer.tsxweb/src/pages/workflow-editor/VersionHistoryPanel.stories.tsxweb/src/pages/workflow-editor/VersionHistoryPanel.tsxweb/src/pages/workflow-editor/WorkflowToolbar.tsxweb/src/pages/workflows/BlueprintPicker.stories.tsxweb/src/pages/workflows/BlueprintPicker.tsxweb/src/pages/workflows/WorkflowCreateDrawer.tsxweb/src/stores/workflow-editor.tsweb/src/stores/workflows.ts
| @post( | ||
| "/{workflow_id:str}/rollback", | ||
| guards=[require_write_access], | ||
| status_code=200, | ||
| ) | ||
| async def rollback_workflow( | ||
| self, | ||
| request: Request[Any, Any, Any], | ||
| state: State, | ||
| workflow_id: PathId, | ||
| data: RollbackWorkflowRequest, | ||
| ) -> Response[ApiResponse[WorkflowDefinition]]: | ||
| """Rollback a workflow to a previous version.""" | ||
| repo = state.app_state.persistence.workflow_definitions | ||
| existing = await repo.get(workflow_id) | ||
| if existing is None: | ||
| logger.warning( | ||
| WORKFLOW_DEF_NOT_FOUND, | ||
| definition_id=workflow_id, | ||
| ) | ||
| return Response( | ||
| content=ApiResponse[WorkflowDefinition]( | ||
| error="Workflow definition not found", | ||
| ), | ||
| status_code=404, | ||
| ) | ||
|
|
||
| if data.expected_version != existing.version: | ||
| logger.warning( | ||
| WORKFLOW_DEF_VERSION_CONFLICT, | ||
| definition_id=workflow_id, | ||
| expected=data.expected_version, | ||
| actual=existing.version, | ||
| ) | ||
| return Response( | ||
| content=ApiResponse[WorkflowDefinition]( | ||
| error=( | ||
| f"Version conflict: expected " | ||
| f"{data.expected_version}, " | ||
| f"actual {existing.version}" | ||
| ), | ||
| ), | ||
| status_code=409, | ||
| ) | ||
|
|
||
| version_repo = state.app_state.persistence.workflow_versions | ||
| target = await version_repo.get_version( | ||
| workflow_id, | ||
| data.target_version, | ||
| ) | ||
| if target is None: | ||
| logger.warning( | ||
| WORKFLOW_DEF_NOT_FOUND, | ||
| definition_id=workflow_id, | ||
| version=data.target_version, | ||
| ) | ||
| return Response( | ||
| content=ApiResponse[WorkflowDefinition]( | ||
| error=f"Target version {data.target_version} not found", | ||
| ), | ||
| status_code=404, | ||
| ) |
There was a problem hiding this comment.
Add validation to reject rollback to current or future versions.
The rollback endpoint should reject cases where target_version >= expected_version, as rolling back to the current or a non-existent future version is semantically invalid. The PR objectives state "rollback to a previous version."
🛡️ Proposed fix
if data.expected_version != existing.version:
logger.warning(
WORKFLOW_DEF_VERSION_CONFLICT,
# ... existing code ...
)
return Response(
# ... existing 409 response ...
)
+
+ if data.target_version >= data.expected_version:
+ logger.warning(
+ WORKFLOW_DEF_INVALID_REQUEST,
+ definition_id=workflow_id,
+ error="target_version must be less than expected_version",
+ )
+ return Response(
+ content=ApiResponse[WorkflowDefinition](
+ error=(
+ f"Cannot rollback to version {data.target_version}: "
+ f"must be less than current version {data.expected_version}"
+ ),
+ ),
+ status_code=400,
+ )
version_repo = state.app_state.persistence.workflow_versions🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/api/controllers/workflow_versions.py` around lines 201 - 262, In
rollback_workflow, add validation before fetching the target version to reject
any rollback where data.target_version >= data.expected_version (i.e., target is
the current or a future version); log a warning (reuse or create a suitable log
key like WORKFLOW_DEF_INVALID_ROLLBACK) with definition_id, expected and target,
and return a Response(ApiResponse(error="Target version must be strictly less
than expected version"), status_code=400) so callers cannot roll forward or to
the current version. Ensure you reference data.expected_version,
data.target_version and rollback_workflow when making this change.
b35327c to
ebca600
Compare
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/design/engine.md`:
- Line 226: The sentence claiming "14 endpoints" is inconsistent with the
enumerated routes for the /workflows API controller; update the copy so the
count matches the list (change "14 endpoints" to "13 endpoints") or add the
missing route to the list; locate the sentence near the description of
WorkflowDefinitionRepository and the "/workflows" API controller and make the
numeric correction so the documentation no longer drifts from the listed routes.
In `@src/synthorg/api/controllers/_workflow_helpers.py`:
- Around line 25-29: Replace the raw string "auth_user_fallback" passed to
logger.debug with the domain event constant imported from
synthorg.observability.events (import the specific constant directly at the top
of _workflow_helpers.py, e.g., from synthorg.observability.events.<domain>
import AUTH_USER_FALLBACK) and use that constant in the logger.debug call (keep
the same keyword args reason= and path=). Ensure the new constant name
(AUTH_USER_FALLBACK or the domain-appropriate event constant) is referenced
instead of the string and add the import statement for that constant.
In `@src/synthorg/api/controllers/workflow_versions.py`:
- Around line 138-169: The _fetch_rollback_target function currently only checks
expected vs existing.version but does not reject rollbacks where
data.target_version is greater than or equal to data.expected_version; add a
validation after fetching existing (or before fetching target) that if
data.target_version >= data.expected_version you log a warning (use
WORKFLOW_DEF_INVALID_ROLLBACK or similar) and return a 400 Response with an
ApiResponse error like "Target version must be less than current version" so
tests such as test_rollback_target_gte_expected_returns_400 pass; update
references to data.target_version, data.expected_version, existing and target
accordingly.
In `@src/synthorg/engine/workflow/blueprints/code-review-pipeline.yaml`:
- Around line 5-7: The blueprint description claims the pipeline "routes back
for revision" but the DAG edges send the revision path directly to the node
named "end" (the transition from the revision stage to end currently defined
around the nodes referenced as revision → end); either update the human-readable
description at the top to match the DAG (remove/modify "routes back for
revision") or change the DAG edges so that the revision path loops back into the
appropriate review stage (modify the edge from the revision node to point to the
prior review/quality gate node instead of "end"); ensure you update the
description text and the DAG edges consistently (references: the description
block at top and the revision → end edge in the pipeline graph).
In `@src/synthorg/engine/workflow/blueprints/onboarding-workflow.yaml`:
- Around line 57-60: The YAML sets routing_strategy: "role_filter" with
role_filter: "senior" but leaves agent_name: "" which is ambiguous; remove the
agent_name key entirely from the blueprint so the config only contains
routing_strategy and role_filter (i.e., delete the agent_name: "" entry) to keep
intent clear and avoid validator friction when processing
onboarding-workflow.yaml.
In `@web/src/components/ui/dialog.stories.tsx`:
- Around line 49-55: The storybook for the Dialog component only includes
Default and Closed stories; add additional stories to cover other recommended
states by creating new Story exports named Loading, Error, and WithForm (or
similar) that set appropriate args and/or children: Loading should pass props or
children that render a spinner or loading state, Error should supply an error
message and any error-related props, and WithForm should render form inputs
inside the dialog to exercise form behavior; use the existing Default and Closed
story patterns as templates and ensure each new export is exported as a Story
(e.g., export const Loading: Story = { args: { open: true, ... } }) and
references the same Dialog component used by Default and Closed.
In `@web/src/components/ui/dialog.tsx`:
- Around line 47-69: The three dialog subcomponents DialogHeader, DialogTitle,
and DialogDescription currently use inline prop types; extract and export typed
interfaces (e.g., DialogHeaderProps, DialogTitleProps, DialogDescriptionProps)
that define className?: string and children: React.ReactNode, then update the
component signatures to use those interfaces (export interface DialogHeaderProps
{ ... } etc.); ensure the exported names are used in the function declarations
for DialogHeader, DialogTitle, and DialogDescription to match the project coding
guideline.
In `@web/src/pages/workflow-editor/VersionDiffViewer.stories.tsx`:
- Around line 63-72: The Default story doesn't reset the diff store, so
diffResult can persist when navigating between stories; update the stories to
reset state by adding a decorator that calls setDiffResult(null) before
rendering Default (and optionally after WithChanges) so the Default story's diff
state is explicitly cleared; locate the story definitions Default and
WithChanges and add a decorator using setDiffResult to ensure diffResult is null
for Default and that WithChanges still sets MOCK_DIFF_WITH_CHANGES.
In `@web/src/stores/workflow-editor.ts`:
- Around line 649-657: Redundant assignment: remove the second redundant set({
saving: false }) after calling get().error since saving was already set to false
in the earlier set call; update the block around set, loadDefinition,
get().error, and loadVersions so you only set saving:false once (leave the
initial set({ definition: updated, saving: false, dirty: false, diffResult: null
}) and on error simply return or set only additional needed state).
In `@web/src/stores/workflows.ts`:
- Around line 13-19: The file defines executable code (const log =
createLogger('workflows')) before type-only imports; move the entire `import
type { BlueprintInfo, CreateFromBlueprintRequest,
CreateWorkflowDefinitionRequest, WorkflowDefinition }` block so that all import
statements appear before the `createLogger` call (i.e., place the `import type`
line above the `const log = createLogger('workflows')` declaration) to restore
conventional imports-first ordering and satisfy linters/readers.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 90702103-bb33-460b-8dc5-b87bea623284
📒 Files selected for processing (56)
CLAUDE.mdREADME.mddocs/DESIGN_SPEC.mddocs/design/engine.mddocs/design/operations.mddocs/design/page-structure.mdsrc/synthorg/api/controllers/__init__.pysrc/synthorg/api/controllers/_workflow_helpers.pysrc/synthorg/api/controllers/workflow_versions.pysrc/synthorg/api/controllers/workflows.pysrc/synthorg/api/dto.pysrc/synthorg/engine/workflow/blueprint_errors.pysrc/synthorg/engine/workflow/blueprint_loader.pysrc/synthorg/engine/workflow/blueprint_models.pysrc/synthorg/engine/workflow/blueprints/__init__.pysrc/synthorg/engine/workflow/blueprints/bug-fix-flow.yamlsrc/synthorg/engine/workflow/blueprints/code-review-pipeline.yamlsrc/synthorg/engine/workflow/blueprints/feature-pipeline.yamlsrc/synthorg/engine/workflow/blueprints/onboarding-workflow.yamlsrc/synthorg/engine/workflow/blueprints/research-sprint.yamlsrc/synthorg/engine/workflow/diff.pysrc/synthorg/engine/workflow/version.pysrc/synthorg/observability/events/blueprint.pysrc/synthorg/observability/events/workflow_definition.pysrc/synthorg/observability/events/workflow_version.pysrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/persistence/sqlite/schema.sqlsrc/synthorg/persistence/sqlite/workflow_version_repo.pysrc/synthorg/persistence/workflow_version_repo.pytests/unit/api/auth/test_session_store.pytests/unit/api/controllers/test_workflow_blueprints.pytests/unit/api/controllers/test_workflow_versions.pytests/unit/api/fakes.pytests/unit/api/fakes_workflow.pytests/unit/engine/workflow/test_blueprint_loader.pytests/unit/engine/workflow/test_blueprint_models.pytests/unit/engine/workflow/test_diff.pytests/unit/engine/workflow/test_version.pytests/unit/observability/test_events.pytests/unit/persistence/sqlite/test_migrations.pytests/unit/persistence/sqlite/test_workflow_version_repo.pyweb/src/api/endpoints/workflows.tsweb/src/api/types.tsweb/src/components/ui/dialog.stories.tsxweb/src/components/ui/dialog.tsxweb/src/pages/WorkflowEditorPage.tsxweb/src/pages/workflow-editor/VersionDiffViewer.stories.tsxweb/src/pages/workflow-editor/VersionDiffViewer.tsxweb/src/pages/workflow-editor/VersionHistoryPanel.stories.tsxweb/src/pages/workflow-editor/VersionHistoryPanel.tsxweb/src/pages/workflow-editor/WorkflowToolbar.tsxweb/src/pages/workflows/BlueprintPicker.stories.tsxweb/src/pages/workflows/BlueprintPicker.tsxweb/src/pages/workflows/WorkflowCreateDrawer.tsxweb/src/stores/workflow-editor.tsweb/src/stores/workflows.ts
| if data.expected_version != existing.version: | ||
| logger.warning( | ||
| WORKFLOW_DEF_VERSION_CONFLICT, | ||
| definition_id=workflow_id, | ||
| expected=data.expected_version, | ||
| actual=existing.version, | ||
| ) | ||
| return Response( | ||
| content=ApiResponse[WorkflowDefinition]( | ||
| error="Version conflict: the workflow was modified. Reload and retry.", | ||
| ), | ||
| status_code=409, | ||
| ) | ||
|
|
||
| target = await version_repo.get_version( | ||
| workflow_id, | ||
| data.target_version, | ||
| ) | ||
| if target is None: | ||
| logger.warning( | ||
| WORKFLOW_DEF_NOT_FOUND, | ||
| definition_id=workflow_id, | ||
| version=data.target_version, | ||
| ) | ||
| return Response( | ||
| content=ApiResponse[WorkflowDefinition]( | ||
| error=f"Target version {data.target_version} not found", | ||
| ), | ||
| status_code=404, | ||
| ) | ||
|
|
||
| return existing, target |
There was a problem hiding this comment.
Add validation to reject rollback to current or future versions.
The _fetch_rollback_target function validates the expected version but does not check that target_version < expected_version. Per the PR objectives ("rollback to a previous version"), rolling back to the current or a future version is semantically invalid. The test test_rollback_target_gte_expected_returns_400 expects this validation.
🛡️ Proposed fix
if data.expected_version != existing.version:
logger.warning(
WORKFLOW_DEF_VERSION_CONFLICT,
definition_id=workflow_id,
expected=data.expected_version,
actual=existing.version,
)
return Response(
content=ApiResponse[WorkflowDefinition](
error="Version conflict: the workflow was modified. Reload and retry.",
),
status_code=409,
)
+ if data.target_version >= data.expected_version:
+ logger.warning(
+ WORKFLOW_DEF_INVALID_REQUEST,
+ definition_id=workflow_id,
+ error="target_version must be less than expected_version",
+ )
+ return Response(
+ content=ApiResponse[WorkflowDefinition](
+ error=(
+ f"Cannot rollback to version {data.target_version}: "
+ f"must be less than current version {data.expected_version}"
+ ),
+ ),
+ status_code=400,
+ )
+
target = await version_repo.get_version(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/api/controllers/workflow_versions.py` around lines 138 - 169,
The _fetch_rollback_target function currently only checks expected vs
existing.version but does not reject rollbacks where data.target_version is
greater than or equal to data.expected_version; add a validation after fetching
existing (or before fetching target) that if data.target_version >=
data.expected_version you log a warning (use WORKFLOW_DEF_INVALID_ROLLBACK or
similar) and return a 400 Response with an ApiResponse error like "Target
version must be less than current version" so tests such as
test_rollback_target_gte_expected_returns_400 pass; update references to
data.target_version, data.expected_version, existing and target accordingly.
src/synthorg/engine/workflow/blueprints/code-review-pipeline.yaml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
web/src/stores/workflow-editor.ts (1)
572-616:⚠️ Potential issue | 🟠 MajorGuard same-definition async races in version/diff loaders.
Current guards only compare
definition.id, so overlapping requests for the same workflow can still write stale data (e.g., rapid compare clicks or parallel load-more).clearDiff()also does not invalidate an in-flight diff request, so a late response can repopulate cleared UI.♻️ Suggested fix pattern (request tokens + functional append)
export interface WorkflowEditorState { + versionsRequestToken: number + diffRequestToken: number // ... } export const useWorkflowEditorStore = create<WorkflowEditorState>()((set, get) => ({ + versionsRequestToken: 0, + diffRequestToken: 0, // ... loadVersions: async () => { const defn = get().definition if (!defn) return const capturedDefnId = defn.id + const token = get().versionsRequestToken + 1 - set({ versionsLoading: true }) + set({ versionsLoading: true, versionsRequestToken: token }) try { const limit = 50 const result = await listWorkflowVersions(capturedDefnId, { limit }) - if (get().definition?.id !== capturedDefnId) return + if (get().definition?.id !== capturedDefnId || get().versionsRequestToken !== token) return set({ versions: result.data, versionsLoading: false, versionsHasMore: result.data.length >= limit, }) } catch (err) { - if (get().definition?.id !== capturedDefnId) return + if (get().definition?.id !== capturedDefnId || get().versionsRequestToken !== token) return log.warn('Failed to load versions', sanitizeForLog(err)) set({ versionsLoading: false, error: getErrorMessage(err) }) } }, loadMoreVersions: async () => { + if (get().versionsLoading || !get().versionsHasMore) return const defn = get().definition if (!defn) return const capturedDefnId = defn.id - const currentVersions = get().versions + const offset = get().versions.length + const token = get().versionsRequestToken + 1 - set({ versionsLoading: true }) + set({ versionsLoading: true, versionsRequestToken: token }) try { const limit = 50 - const result = await listWorkflowVersions(capturedDefnId, { limit, offset: currentVersions.length }) - if (get().definition?.id !== capturedDefnId) return - set({ - versions: [...currentVersions, ...result.data], + const result = await listWorkflowVersions(capturedDefnId, { limit, offset }) + if (get().definition?.id !== capturedDefnId || get().versionsRequestToken !== token) return + set((s) => ({ + versions: [...s.versions, ...result.data], versionsLoading: false, versionsHasMore: result.data.length >= limit, - }) + })) } catch (err) { - if (get().definition?.id !== capturedDefnId) return + if (get().definition?.id !== capturedDefnId || get().versionsRequestToken !== token) return log.warn('Failed to load more versions', sanitizeForLog(err)) set({ versionsLoading: false, error: getErrorMessage(err) }) } }, loadDiff: async (fromVersion: number, toVersion: number) => { const defn = get().definition if (!defn) return + const token = get().diffRequestToken + 1 - set({ diffLoading: true }) + set({ diffLoading: true, diffRequestToken: token }) try { const diff = await getWorkflowDiff(defn.id, fromVersion, toVersion) - if (get().definition?.id !== defn.id) return + if (get().definition?.id !== defn.id || get().diffRequestToken !== token) return set({ diffResult: diff, diffLoading: false }) } catch (err) { - if (get().definition?.id !== defn.id) return + if (get().definition?.id !== defn.id || get().diffRequestToken !== token) return log.warn('Failed to load diff', sanitizeForLog(err)) set({ diffLoading: false, error: getErrorMessage(err) }) } }, clearDiff: () => { - set({ diffResult: null }) + set((s) => ({ diffResult: null, diffLoading: false, diffRequestToken: s.diffRequestToken + 1 })) }, }))Also applies to: 618-638
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/stores/workflow-editor.ts` around lines 572 - 616, loadVersions and loadMoreVersions only guard by definition.id so concurrent requests for the same workflow can overwrite with stale data; also clearDiff doesn't invalidate an in-flight diff so a late response can repopulate cleared UI. Fix by adding per-request tokens in state (e.g., versionsRequestToken and diffRequestToken) generated before each async call and saved via set(), then compare the token before any state mutations to ensure only the most recent response applies; for loadMoreVersions use a functional set (set(prev => ({ versions: [...prev.versions, ...result.data], ... }))) instead of reading currentVersions to avoid racey reads; and have clearDiff increment/clear the diffRequestToken to ignore any in-flight diff responses.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/api/controllers/_workflow_helpers.py`:
- Around line 19-22: The public helper docstrings in _workflow_helpers.py (the
function that "Extract the authenticated user ID from a request" and the other
public helper around lines 38-42) must be converted to Google-style docstrings:
add an Args: section documenting the request parameter and its type, and a
Returns: section describing the return value and its type (e.g., str; note that
it returns "api" when no AuthenticatedUser is in scope). Update both function
docstrings to follow this format and include concise type and behavior details
to satisfy the ruff D rules.
In `@web/src/components/ui/dialog.tsx`:
- Around line 35-37: The dialog uses exit animations ('fade-out-0',
'zoom-out-95') which require mounted content during the animation; in
VersionDiffViewer where onOpenChange calls clearDiff() and sets diffResult to
null immediately, delay clearing or keep the DOM until animation finishes to
avoid a visual flash—update VersionDiffViewer to either postpone clearDiff() by
~150-200ms on close, keep the outer dialog markup rendered while only hiding
inner data, or subscribe to Radix animation/close events and call clearDiff()
when the close animation completes so diffResult isn't unmounted mid-animation.
---
Duplicate comments:
In `@web/src/stores/workflow-editor.ts`:
- Around line 572-616: loadVersions and loadMoreVersions only guard by
definition.id so concurrent requests for the same workflow can overwrite with
stale data; also clearDiff doesn't invalidate an in-flight diff so a late
response can repopulate cleared UI. Fix by adding per-request tokens in state
(e.g., versionsRequestToken and diffRequestToken) generated before each async
call and saved via set(), then compare the token before any state mutations to
ensure only the most recent response applies; for loadMoreVersions use a
functional set (set(prev => ({ versions: [...prev.versions, ...result.data], ...
}))) instead of reading currentVersions to avoid racey reads; and have clearDiff
increment/clear the diffRequestToken to ignore any in-flight diff responses.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 17e9f06d-6710-41cd-8d84-5203e5a9117c
📒 Files selected for processing (10)
docs/design/engine.mdsrc/synthorg/api/controllers/_workflow_helpers.pysrc/synthorg/engine/workflow/blueprints/code-review-pipeline.yamlsrc/synthorg/engine/workflow/blueprints/onboarding-workflow.yamlsrc/synthorg/observability/events/api.pyweb/src/components/ui/dialog.stories.tsxweb/src/components/ui/dialog.tsxweb/src/pages/workflow-editor/VersionDiffViewer.stories.tsxweb/src/stores/workflow-editor.tsweb/src/stores/workflows.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Deploy Preview
- GitHub Check: Build Web
- GitHub Check: Build Backend
- GitHub Check: Build Sandbox
- GitHub Check: Dashboard Test
- GitHub Check: Test (Python 3.14)
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (14)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations-- Python 3.14 has PEP 649.
PEP 758 except syntax: useexcept A, B:(no parentheses) for exception handling.
All public functions require type hints. Use mypy strict mode.
Docstrings must use Google style and are required on public classes and functions (enforced by ruff D rules).
Create new objects instead of mutating existing ones. For non-Pydantic internal collections, use copy.deepcopy() at construction and MappingProxyType wrapping for read-only enforcement.
Use frozen Pydantic models for config/identity. Use separate mutable-via-copy models for runtime state that evolves.
Use Pydantic v2 (BaseModel, model_validator, computed_field, ConfigDict). Use allow_inf_nan=False in all ConfigDict declarations.
Use NotBlankStr (from core.types) for all identifier/name fields in Pydantic models.
Use@computed_fieldfor derived values instead of storing and validating redundant fields.
Prefer asyncio.TaskGroup for fan-out/fan-in parallel operations in new code (e.g., multiple tool invocations, parallel agent calls). Prefer structured concurrency over bare create_task.
Functions must be less than 50 lines and files must be less than 800 lines.
Handle errors explicitly, never silently swallow them.
Validate at system boundaries (user input, external APIs, config files).
Line length: 88 characters (ruff).
Python: 3.14+ (PEP 649 native lazy annotations).
Files:
src/synthorg/observability/events/api.pysrc/synthorg/api/controllers/_workflow_helpers.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic must have: from synthorg.observability import get_logger then logger = get_logger(name)
Never use import logging / logging.getLogger() / print() in application code. Exception: observability/setup.py, observability/sinks.py, observability/syslog_handler.py, and observability/http_handler.py may use stdlib logging and print for handler construction.
Logger variable name must always be 'logger' (not '_logger', not 'log').
Use constants from domain-specific modules under synthorg.observability.events for event names (e.g., API_REQUEST_STARTED, TOOL_INVOKE_START). Import directly from synthorg.observability.events..
Always use logger.info(EVENT, key=value) for structured logging. Never use logger.info('msg %s', val).
All error paths must log at WARNING or ERROR with context before raising.
All state transitions must log at INFO level.
DEBUG logging is used for object creation, internal flow, and entry/exit of key functions.
Never use real vendor names (Anthropic, OpenAI, Claude, GPT) in project-owned code, docstrings, comments, tests, or config examples. Use generic names: example-provider, example-large-001, example-medium-001, example-small-001.
Files:
src/synthorg/observability/events/api.pysrc/synthorg/api/controllers/_workflow_helpers.py
src/**/*.py
⚙️ CodeRabbit configuration file
This project uses Python 3.14+ with PEP 758 except syntax: "except A, B:" (comma-separated, no parentheses) is correct and mandatory -- do NOT flag it as a typo or suggest parenthesized form. The "except builtins.MemoryError, RecursionError: raise" pattern is intentional project convention for system-error propagation. When evaluating the 50-line function limit, count only the function body excluding the signature lines, decorators, and docstring. Functions 1-5 lines over due to docstrings or multi-line signatures should not be flagged. Do not suggest extracting single-use helper functions called exactly once -- this reduces readability without improving maintainability.
Files:
src/synthorg/observability/events/api.pysrc/synthorg/api/controllers/_workflow_helpers.py
docs/design/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
When approved deviations occur, update the relevant docs/design/ page to reflect the new reality.
Files:
docs/design/engine.md
docs/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Docs: Markdown in docs/ directory, built with Zensical, config: mkdocs.yml. Design spec: docs/design/ (12 pages).
Files:
docs/design/engine.md
web/src/**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
web/src/**/*.{tsx,ts}: Never hardcode hex colors, font-family, pixel spacing, or Framer Motion transitions in web code. Use design tokens and@/lib/motionpresets.
Web dashboard: use React 19, TypeScript 6.0+, Tailwind CSS 4, Zustand, shadcn/ui, Radix UI, and other specified dependencies from web/package.json.
Files:
web/src/pages/workflow-editor/VersionDiffViewer.stories.tsxweb/src/stores/workflows.tsweb/src/stores/workflow-editor.tsweb/src/components/ui/dialog.tsxweb/src/components/ui/dialog.stories.tsx
web/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{ts,tsx,js,jsx}: Always usecreateLoggerfrom@/lib/loggerinstead of bareconsole.warn,console.error, orconsole.debugin application code
Use variable namelogfor logger instances (e.g.,const log = createLogger('module-name'))
Uselog.debug()for dev-only logging (stripped in production),log.warn()for warnings, andlog.error()for errors
Pass dynamic/untrusted values as separate arguments to logger calls (not interpolated into the message string) so they go throughsanitizeArg
Wrap attacker-controlled fields inside structured objects withsanitizeForLog()before embedding in logs
Files:
web/src/pages/workflow-editor/VersionDiffViewer.stories.tsxweb/src/stores/workflows.tsweb/src/stores/workflow-editor.tsweb/src/components/ui/dialog.tsxweb/src/components/ui/dialog.stories.tsx
web/src/pages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/pages/**/*.{ts,tsx}: Always reuse existing components fromweb/src/components/ui/before creating new ones
UseStatusBadgecomponent for agent/task/system status indicators (colored dot + optional built-in label toggle)
UseMetricCardcomponent for numeric KPIs with sparkline, change badge, progress bar
UseSparklinecomponent for inline SVG trend lines withcolor?andanimated?props (used inside MetricCard or standalone)
UseSectionCardcomponent for titled card wrapper with icon and action slot
UseAgentCardcomponent for agent display (avatar, name, role, status, current task)
UseDeptHealthBarcomponent for department utilization (animated fill bar +health?+agentCount)
UseProgressGaugecomponent for circular or linear gauge for budget/utilization (variant?defaults to'circular',max?defaults to 100)
UseStatPillcomponent for compact inline label + value pair
UseAvatarcomponent for circular initials avatar with optionalborderColor?prop
UseToast/ToastContainercomponent for success/error/warning/info notifications with auto-dismiss queue (mountToastContaineronce in AppLayout)
UseSkeleton/SkeletonCard/SkeletonMetric/SkeletonTable/SkeletonTextcomponents for loading placeholders matching component shapes (shimmer animation, respectsprefers-reduced-motion)
UseEmptyStatecomponent for no-data / no-results placeholder with icon, title, description, optional action button
UseErrorBoundarycomponent for React error boundary with retry --levelprop:page/section/component
UseConfirmDialogcomponent for confirmation modal (Radix AlertDialog) withdefault/destructivevariants andloadingstate
UseCommandPalettecomponent for global Cmd+K search (cmdk + React Router) -- mount once in AppLayout, register commands viauseCommandPalettehook
UseInlineEditcomponent for click-to-edit text with Enter/Escape, validation, optimistic save with rollback
Use...
Files:
web/src/pages/workflow-editor/VersionDiffViewer.stories.tsx
web/src/**/*.{ts,tsx,css}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{ts,tsx,css}: Use Tailwind semantic color classes (text-foreground,bg-card,text-accent,text-success,bg-danger, etc.) or CSS variables (var(--so-accent)) instead of hardcoded hex values
Usefont-sansorfont-monofor typography (maps to Geist tokens), never setfontFamilydirectly
Use density-aware spacing tokens (p-card,gap-section-gap,gap-grid-gap) or standard Tailwind spacing, never hardcode pixel values for layout spacing
Use token variables (var(--so-shadow-card-hover),border-border,border-bright) for shadows and borders instead of hardcoded values
Do NOT usergba()with hardcoded values -- use design token variables
A PostToolUse hook (scripts/check_web_design_system.py) runs automatically on every Edit/Write toweb/src/files and catches design system violations; fix all violations before proceeding
Files:
web/src/pages/workflow-editor/VersionDiffViewer.stories.tsxweb/src/stores/workflows.tsweb/src/stores/workflow-editor.tsweb/src/components/ui/dialog.tsxweb/src/components/ui/dialog.stories.tsx
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{ts,tsx}: Do NOT hardcode Framer Motion transition durations -- use presets from@/lib/motion
CSS side-effect imports in TypeScript files should use type declarations (/// <reference types="vite/client" />covers this); TypeScript 6 defaultsnoUncheckedSideEffectImportsto true
Files:
web/src/pages/workflow-editor/VersionDiffViewer.stories.tsxweb/src/stores/workflows.tsweb/src/stores/workflow-editor.tsweb/src/components/ui/dialog.tsxweb/src/components/ui/dialog.stories.tsx
web/src/**/*.stories.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.stories.{ts,tsx}: Usestorybook/testinstead of@storybook/testandstorybook/actionsinstead of@storybook/addon-actions(Storybook 10 import path changes)
Useparameters.backgrounds.options(object keyed by name) +initialGlobals.backgrounds.valuein Storybook (Storybook 10 Backgrounds API change)
Useparameters.a11y.test: 'error' | 'todo' | 'off'in Storybook stories for a11y testing (Storybook 10 change)
Files:
web/src/pages/workflow-editor/VersionDiffViewer.stories.tsxweb/src/components/ui/dialog.stories.tsx
web/src/components/**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Always reuse existing components from web/src/components/ui/ before creating new ones in the web dashboard.
Files:
web/src/components/ui/dialog.tsxweb/src/components/ui/dialog.stories.tsx
web/src/components/ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/components/ui/**/*.{ts,tsx}: Place new shared components inweb/src/components/ui/with descriptive kebab-case filename
Export props as a TypeScript interface in new components
Use design tokens exclusively in component code -- no hardcoded colors, fonts, or spacing
Importcnfrom@/lib/utilsfor conditional class merging in component files
Files:
web/src/components/ui/dialog.tsxweb/src/components/ui/dialog.stories.tsx
web/src/components/ui/**/*.stories.tsx
📄 CodeRabbit inference engine (web/CLAUDE.md)
Create a
.stories.tsxfile alongside new components with all states (default, hover, loading, error, empty)
Files:
web/src/components/ui/dialog.stories.tsx
🧠 Learnings (45)
📚 Learning: 2026-03-19T11:33:01.580Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:33:01.580Z
Learning: Applies to src/synthorg/**/*.py : Use event constants from `synthorg.observability.events.<domain>` (e.g., `API_REQUEST_STARTED` from `events.api`); import directly and log with structured kwargs: `logger.info(EVENT, key=value)`, never interpolated strings
Applied to files:
src/synthorg/observability/events/api.pysrc/synthorg/api/controllers/_workflow_helpers.py
📚 Learning: 2026-04-02T07:18:02.381Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T07:18:02.381Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`); import directly from the domain module
Applied to files:
src/synthorg/observability/events/api.py
📚 Learning: 2026-03-20T11:18:48.128Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T11:18:48.128Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`). Import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/synthorg/observability/events/api.py
📚 Learning: 2026-04-04T17:42:47.768Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-04T17:42:47.768Z
Learning: Applies to src/synthorg/**/*.py : Use constants from domain-specific modules under synthorg.observability.events for event names (e.g., API_REQUEST_STARTED, TOOL_INVOKE_START). Import directly from synthorg.observability.events.<domain>.
Applied to files:
src/synthorg/observability/events/api.py
📚 Learning: 2026-03-18T21:23:23.586Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:23:23.586Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from the domain-specific module under synthorg.observability.events (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool). Import directly from synthorg.observability.events.<domain>.
Applied to files:
src/synthorg/observability/events/api.pysrc/synthorg/api/controllers/_workflow_helpers.py
📚 Learning: 2026-03-31T16:09:24.320Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T16:09:24.320Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from `synthorg.observability.events.<domain>` modules (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`); import directly and use in structured logging
Applied to files:
src/synthorg/observability/events/api.pysrc/synthorg/api/controllers/_workflow_helpers.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`); import directly rather than using string literals
Applied to files:
src/synthorg/observability/events/api.pysrc/synthorg/api/controllers/_workflow_helpers.py
📚 Learning: 2026-03-16T06:24:56.341Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:24:56.341Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from the domain-specific module under `synthorg.observability.events` in logging calls
Applied to files:
src/synthorg/api/controllers/_workflow_helpers.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions.
Applied to files:
src/synthorg/api/controllers/_workflow_helpers.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, and key function entry/exit
Applied to files:
src/synthorg/api/controllers/_workflow_helpers.py
📚 Learning: 2026-03-17T06:43:14.114Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:43:14.114Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions. Pure data models, enums, and re-exports do NOT need logging.
Applied to files:
src/synthorg/api/controllers/_workflow_helpers.py
📚 Learning: 2026-03-15T18:38:44.202Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:38:44.202Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`); import directly: `from synthorg.observability.events.<domain> import EVENT_CONSTANT`
Applied to files:
src/synthorg/api/controllers/_workflow_helpers.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/observability/**/*.py : Observability package (observability/): structured logging, correlation tracking, log sinks; event constants organized by domain under observability/events/ (e.g., events.api, events.tool, events.git, events.context_budget, events.backup)
Applied to files:
src/synthorg/api/controllers/_workflow_helpers.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Engine: Agent orchestration, execution loops, parallel execution, task decomposition, routing, task assignment, centralized single-writer task state engine (TaskEngine), task lifecycle, recovery, shutdown, workspace isolation, coordination (multi-agent pipeline: TopologyDispatcher protocol, 4 dispatchers — SAS/centralized/decentralized/context-dependent, wave execution, workspace lifecycle integration, CoordinationSectionConfig company config bridge, build_coordinator factory), coordination error classification, prompt policy validation, checkpoint recovery (checkpoint/, per-turn persistence, heartbeat detection, CheckpointRecoveryStrategy), approval gate (escalation detection, context parking/resume, EscalationInfo/ResumePayload models), stagnation detection (stagnation/, StagnationDetector protocol, ToolRepetitionDetector, dual-signal analysis, corrective prompt injection), agent runtime state (AgentRuntimeState, lightweight per-agent execution status for dashboard queries and recove...
Applied to files:
src/synthorg/engine/workflow/blueprints/onboarding-workflow.yaml
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/components/ui/**/*.stories.tsx : Create a `.stories.tsx` file alongside new components with all states (default, hover, loading, error, empty)
Applied to files:
web/src/pages/workflow-editor/VersionDiffViewer.stories.tsxweb/src/stores/workflows.tsweb/src/components/ui/dialog.stories.tsx
📚 Learning: 2026-03-30T10:41:40.176Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:41:40.176Z
Learning: Applies to web/src/**/*.stories.tsx : Storybook 10: import from `storybook/test` (not `storybook/test`), `storybook/actions` (not `storybook/addon-actions`)
Applied to files:
web/src/pages/workflow-editor/VersionDiffViewer.stories.tsxweb/src/components/ui/dialog.stories.tsx
📚 Learning: 2026-03-30T10:41:40.176Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:41:40.176Z
Learning: Applies to web/src/components/ui/**/*.{ts,tsx} : Create new shared components in `web/src/components/ui/` with `.stories.tsx` Storybook file covering all states (default, hover, loading, error, empty)
Applied to files:
web/src/pages/workflow-editor/VersionDiffViewer.stories.tsxweb/src/components/ui/dialog.stories.tsx
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/**/*.stories.tsx : Storybook 10: Import from `storybook/test` instead of `storybook/test`
Applied to files:
web/src/pages/workflow-editor/VersionDiffViewer.stories.tsxweb/src/components/ui/dialog.stories.tsx
📚 Learning: 2026-03-30T10:20:08.544Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:20:08.544Z
Learning: Applies to web/**/*.stories.{ts,tsx} : Storybook 10: Use storybook/test (not storybook/test) and storybook/actions (not storybook/addon-actions) import paths
Applied to files:
web/src/pages/workflow-editor/VersionDiffViewer.stories.tsxweb/src/components/ui/dialog.stories.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/**/*.stories.{ts,tsx} : Use `storybook/test` instead of `storybook/test` and `storybook/actions` instead of `storybook/addon-actions` (Storybook 10 import path changes)
Applied to files:
web/src/pages/workflow-editor/VersionDiffViewer.stories.tsxweb/src/components/ui/dialog.stories.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/**/*.stories.{ts,tsx} : Use `parameters.backgrounds.options` (object keyed by name) + `initialGlobals.backgrounds.value` in Storybook (Storybook 10 Backgrounds API change)
Applied to files:
web/src/pages/workflow-editor/VersionDiffViewer.stories.tsxweb/src/components/ui/dialog.stories.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/**/*.stories.{ts,tsx} : Use `parameters.a11y.test: 'error' | 'todo' | 'off'` in Storybook stories for a11y testing (Storybook 10 change)
Applied to files:
web/src/pages/workflow-editor/VersionDiffViewer.stories.tsxweb/src/components/ui/dialog.stories.tsx
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/**/*.stories.tsx : Storybook 10: Use `parameters.a11y.test: 'error' | 'todo' | 'off'` for a11y testing configuration (replaces old `.element` and `.manual`)
Applied to files:
web/src/pages/workflow-editor/VersionDiffViewer.stories.tsxweb/src/components/ui/dialog.stories.tsx
📚 Learning: 2026-03-27T22:32:26.927Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-27T22:32:26.927Z
Learning: Applies to web/src/components/ui/*.{tsx,ts} : For new shared React components: place in web/src/components/ui/ with kebab-case filename, create .stories.tsx with all states, export props as TypeScript interface, use design tokens exclusively
Applied to files:
web/src/pages/workflow-editor/VersionDiffViewer.stories.tsxweb/src/components/ui/dialog.tsxweb/src/components/ui/dialog.stories.tsx
📚 Learning: 2026-03-30T10:20:08.544Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:20:08.544Z
Learning: Applies to web/**/*.stories.{ts,tsx} : Storybook 10: Use parameters.a11y.test: 'error' | 'todo' | 'off' for a11y testing (replaces old .element and .manual); set globally in preview.tsx to enforce WCAG compliance on all stories
Applied to files:
web/src/pages/workflow-editor/VersionDiffViewer.stories.tsxweb/src/components/ui/dialog.stories.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/.storybook/preview.tsx : Set `parameters.a11y.test` globally in `preview.tsx` to enforce WCAG compliance on all stories (Storybook 10)
Applied to files:
web/src/pages/workflow-editor/VersionDiffViewer.stories.tsx
📚 Learning: 2026-03-30T10:20:08.544Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:20:08.544Z
Learning: Applies to web/**/*.stories.{ts,tsx} : Storybook 10: Use parameters.backgrounds.options (object keyed by name) + initialGlobals.backgrounds.value for background options (replaces old default + values array)
Applied to files:
web/src/pages/workflow-editor/VersionDiffViewer.stories.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/.storybook/main.ts : Use `defineMain` from `storybook/react-vite/node` and `definePreview` from `storybook/react-vite` with type-safe config in Storybook configuration (Storybook 10)
Applied to files:
web/src/pages/workflow-editor/VersionDiffViewer.stories.tsxweb/src/components/ui/dialog.stories.tsx
📚 Learning: 2026-03-27T12:44:29.466Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-27T12:44:29.466Z
Learning: Applies to web/src/stores/**/*.{ts,tsx} : Use Zustand stores in web dashboard for state management (auth, WebSocket, toast, analytics, domain shells)
Applied to files:
web/src/stores/workflows.tsweb/src/stores/workflow-editor.ts
📚 Learning: 2026-03-30T10:41:40.176Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:41:40.176Z
Learning: Applies to web/src/**/*.{ts,tsx} : Use Zustand stores for state management in the web dashboard; each domain has its own store module (auth, WebSocket, toast, analytics, setup, company, agents, budget, tasks, settings, providers, theme, per-domain stores)
Applied to files:
web/src/stores/workflows.tsweb/src/stores/workflow-editor.ts
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/**/*.{ts,tsx} : CSS side-effect imports in TypeScript files should use type declarations (`/// <reference types="vite/client" />` covers this); TypeScript 6 defaults `noUncheckedSideEffectImports` to true
Applied to files:
web/src/stores/workflows.ts
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/**/*.{ts,tsx,js,jsx} : Use variable name `log` for logger instances (e.g., `const log = createLogger('module-name')`)
Applied to files:
web/src/stores/workflow-editor.ts
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `ConfirmDialog` component for confirmation modal (Radix AlertDialog) with `default` / `destructive` variants and `loading` state
Applied to files:
web/src/components/ui/dialog.tsxweb/src/components/ui/dialog.stories.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `ThemeToggle` component for Radix Popover with 5-axis theme controls (color, density, typography, animation, sidebar), rendered in StatusBar
Applied to files:
web/src/components/ui/dialog.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/components/ui/**/*.{ts,tsx} : Export props as a TypeScript interface in new components
Applied to files:
web/src/components/ui/dialog.tsx
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/components/ui/**/*.tsx : Export props as a TypeScript interface for new components
Applied to files:
web/src/components/ui/dialog.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `Drawer` component for slide-in panel (`side` prop: left or right, default right) with overlay, spring animation, focus trap, Escape-to-close, optional header (`title`), `ariaLabel` for accessible name (one of `title` or `ariaLabel` required), and `contentClassName` override
Applied to files:
web/src/components/ui/dialog.tsx
📚 Learning: 2026-03-30T10:20:08.544Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:20:08.544Z
Learning: Applies to web/src/components/ui/**/*.{ts,tsx} : When creating new shared web components, place in web/src/components/ui/ with kebab-case filename, create .stories.tsx alongside with all states (default, hover, loading, error, empty), export props as TypeScript interface, use design tokens exclusively with no hardcoded colors/fonts/spacing, and import cn from `@/lib/utils` for conditional class merging
Applied to files:
web/src/components/ui/dialog.tsxweb/src/components/ui/dialog.stories.tsx
📚 Learning: 2026-03-30T10:20:08.544Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:20:08.544Z
Learning: Applies to web/src/**/*.{ts,tsx} : Always reuse existing components from web/src/components/ui/ (StatusBadge, MetricCard, Sparkline, SectionCard, AgentCard, DeptHealthBar, ProgressGauge, StatPill, Avatar, Button, Toast/ToastContainer, Skeleton variants, EmptyState, ErrorBoundary, ConfirmDialog, CommandPalette, InlineEdit, AnimatedPresence, StaggerGroup/StaggerItem, Drawer, form fields, TaskStatusIndicator, PriorityBadge, ProviderHealthBadge, TokenUsageBar, CodeMirrorEditor, SegmentedControl, ThemeToggle, LiveRegion, MobileUnsupportedOverlay, LazyCodeMirrorEditor) before creating new components
Applied to files:
web/src/components/ui/dialog.tsx
📚 Learning: 2026-04-04T17:42:47.768Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-04T17:42:47.768Z
Learning: Applies to web/src/**/*.{tsx,ts} : Web dashboard: use React 19, TypeScript 6.0+, Tailwind CSS 4, Zustand, shadcn/ui, Radix UI, and other specified dependencies from web/package.json.
Applied to files:
web/src/components/ui/dialog.tsx
📚 Learning: 2026-03-31T14:31:11.894Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:31:11.894Z
Learning: Applies to web/src/**/*.{ts,tsx} : Use React 19, TypeScript 6.0+, and design system tokens from shadcn/ui + Tailwind CSS 4 + Radix UI in web dashboard
Applied to files:
web/src/components/ui/dialog.tsx
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/**/*.{tsx,ts} : ALWAYS reuse existing components from `web/src/components/ui/` before creating new ones (StatusBadge, MetricCard, Sparkline, SectionCard, AgentCard, DeptHealthBar, ProgressGauge, StatPill, Avatar, Button, Toast, Skeleton, EmptyState, ErrorBoundary, ConfirmDialog, CommandPalette, InlineEdit, AnimatedPresence, StaggerGroup, Drawer, InputField, SelectField, SliderField, ToggleField, TaskStatusIndicator, PriorityBadge, ProviderHealthBadge, TokenUsageBar, CodeMirrorEditor, SegmentedControl, ThemeToggle, LiveRegion, MobileUnsupportedOverlay, LazyCodeMirrorEditor, TagInput, MetadataGrid, ProjectStatusBadge, ContentTypeBadge)
Applied to files:
web/src/components/ui/dialog.tsx
📚 Learning: 2026-03-27T12:44:29.466Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-27T12:44:29.466Z
Learning: Applies to web/src/**/*.{ts,tsx} : Always reuse existing components from `web/src/components/ui/` (StatusBadge, MetricCard, Sparkline, SectionCard, AgentCard, DeptHealthBar, ProgressGauge, StatPill, Avatar, Button, Toast, Skeleton, EmptyState, ErrorBoundary, ConfirmDialog, CommandPalette, InlineEdit, AnimatedPresence, StaggerGroup/StaggerItem) before creating new ones
Applied to files:
web/src/components/ui/dialog.tsx
📚 Learning: 2026-04-01T20:43:51.878Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T20:43:51.878Z
Learning: Applies to web/src/**/*.{ts,tsx} : Always reuse existing components from `web/src/components/ui/` before creating new ones. Never hardcode hex colors, font-family, pixel spacing, or Framer Motion transitions -- use design tokens and `@/lib/motion` presets.
Applied to files:
web/src/components/ui/dialog.tsx
📚 Learning: 2026-04-04T17:42:47.768Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-04T17:42:47.768Z
Learning: Applies to web/src/components/**/*.{tsx,ts} : Always reuse existing components from web/src/components/ui/ before creating new ones in the web dashboard.
Applied to files:
web/src/components/ui/dialog.tsx
🔇 Additional comments (17)
docs/design/engine.md (1)
226-226: Endpoint count/list alignment is now correct.The documented
/workflowsendpoint count matches the enumerated routes after addingvalidate draftand versioning/blueprint operations.src/synthorg/engine/workflow/blueprints/code-review-pipeline.yaml (3)
1-12: LGTM!The blueprint metadata is well-structured. The description accurately reflects the DAG behavior — the quality gate branches to either approve or revision paths, both converging at end. This addresses the prior concern about description/DAG mismatch.
14-97: LGTM!All 9 nodes are well-defined with unique IDs, valid
WorkflowNodeTypevalues, and sensible canvas positions for visual rendering. The config fields align with type-specific requirements (task metadata, routing strategy for agent assignment, condition expression for the quality gate).
99-143: LGTM!The edge definitions form a valid DAG with proper conditional branching from the quality gate. All node references are valid, edge types are correct (
conditional_true/conditional_falsefor the quality gate branches,sequentialelsewhere), and both paths converge cleanly at the end node.web/src/stores/workflows.ts (5)
1-19: Import ordering is correct.All imports (both runtime and type-only) now precede the executable
createLoggercall, which aligns with conventional imports-first organization.
28-31: Blueprint state shape looks good.The loading-state triple (
blueprints,blueprintsLoading,blueprintsError) follows the same pattern as the existing workflow listing state and appropriately usesreadonlyfor immutability.Also applies to: 59-61
66-75: Implementation follows established patterns.The action correctly manages the loading lifecycle and adheres to logging guidelines by using
sanitizeForLogandgetErrorMessage. The absence of stale-request handling (unlikefetchWorkflows) is acceptable here since blueprints are relatively static and typically loaded once per drawer mount.
88-91: Sanitized error logging is correctly applied.Using
sanitizeForLog(err)before logging aligns with the coding guidelines for handling dynamic/untrusted values in logger calls.
108-119: Implementation is consistent with existing patterns.The
createFromBlueprintaction mirrors thecreateWorkflowimplementation with proper deduplication logic andtotalWorkflowsincrement handling. Errors correctly propagate to the caller (WorkflowCreateDrawer) for UI handling, matching the established pattern.src/synthorg/engine/workflow/blueprints/onboarding-workflow.yaml (1)
1-145: Well-structured onboarding workflow blueprint.The blueprint correctly implements a parallel execution pattern:
- Unique node IDs with exactly one START and one END node
- Proper parallel split/join structure with
max_concurrency: 3matching the three parallel branches- All edge references are valid and the graph is fully connected
- Edge types appropriately use
parallel_branchfor fan-out andsequentialfor convergenceThe workflow flow is clear: credential creation → parallel setup tasks (tools, mentor assignment, workspace) → convergence → orientation → end.
web/src/components/ui/dialog.stories.tsx (1)
1-147: LGTM! Comprehensive story coverage with proper Storybook 10 configuration.The stories file correctly:
- Uses
parameters.a11y.test: 'error'per Storybook 10 conventions- Covers all recommended states (Default, Closed, Loading, Error, WithForm)
- Uses semantic design tokens throughout (
text-foreground,border-accent,text-danger,p-card, etc.)web/src/components/ui/dialog.tsx (2)
5-107: Well-structured Dialog component with proper accessibility and design tokens.The implementation correctly:
- Exports props interfaces for all subcomponents
- Uses semantic design tokens (
bg-background,border-border,text-foreground,text-muted, etc.)- Provides proper accessibility via Radix primitives (
DialogTitle,DialogDescription,aria-labelon close button)- Includes focus-visible styles for keyboard navigation
1-3: Import path is correct for this project.The
radix-uipackage is declared in web/package.json as version ^1.4.3, making the importimport { Dialog as RadixDialog } from 'radix-ui'valid and intentional. The file complies with all design system guidelines: props interfaces are exported, design tokens are used throughout (bg-background, border-border, text-foreground, text-muted, etc.), and no hardcoded colors, fonts, or spacing are present.src/synthorg/observability/events/api.py (1)
13-13: Good addition of a domain-scoped API event constant.This keeps observability event taxonomy consistent and reusable across controllers.
Based on learnings: "Use event name constants from domain-specific modules under
synthorg.observability.eventsand import directly."src/synthorg/api/controllers/_workflow_helpers.py (1)
26-30: Nice fix: structured debug logging now uses an API event constant.This aligns with the project’s observability event usage conventions.
Based on learnings: "Always use event name constants from domain-specific modules under
synthorg.observability.events; import directly rather than using string literals."web/src/pages/workflow-editor/VersionDiffViewer.stories.tsx (1)
52-58: Good Storybook setup for accessibility and deterministic story state.
parameters.a11y.testis set correctly, and each story explicitly seedsdiffResult, which keeps cross-story state predictable.As per coding guidelines:
web/src/**/*.stories.{ts,tsx}requiresparameters.a11y.test: 'error' | 'todo' | 'off'.Also applies to: 63-97
web/src/stores/workflow-editor.ts (1)
214-215: Nice improvement: sanitized structured logging is consistently applied.Switching warning paths to
log.warn(..., sanitizeForLog(err))improves safety and consistency.As per coding guidelines: use
createLogger(log) and pass untrusted values as structured/sanitized logger arguments.Also applies to: 251-252, 297-302, 527-528, 588-589, 613-614, 631-632, 656-657
| 'data-[state=open]:animate-in data-[state=closed]:animate-out', | ||
| 'data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0', | ||
| 'data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95', |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Animation timing consideration for consumers.
The exit animations (fade-out-0, zoom-out-95) require content to remain mounted during the animation sequence. Based on the usage in VersionDiffViewer.tsx, calling clearDiff() (which sets diffResult to null) immediately in onOpenChange can cause the conditionally-rendered content ({diffResult && ...}) to unmount before the animation completes, resulting in a visual flash.
Consumers should either:
- Delay state clearing until after the animation completes (~150-200ms)
- Always render content structure and conditionally show/hide inner data
- Use Radix's animation events if available
This is a usage pattern consideration, not a bug in the Dialog component itself.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/ui/dialog.tsx` around lines 35 - 37, The dialog uses exit
animations ('fade-out-0', 'zoom-out-95') which require mounted content during
the animation; in VersionDiffViewer where onOpenChange calls clearDiff() and
sets diffResult to null immediately, delay clearing or keep the DOM until
animation finishes to avoid a visual flash—update VersionDiffViewer to either
postpone clearDiff() by ~150-200ms on close, keep the outer dialog markup
rendered while only hiding inner data, or subscribe to Radix animation/close
events and call clearDiff() when the close animation completes so diffResult
isn't unmounted mid-animation.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/api/controllers/_workflow_helpers.py`:
- Line 65: The assignment of definition.updated_at to the non-nullable saved_at
field in WorkflowDefinitionVersion may raise validation errors if updated_at is
None; update the code that builds WorkflowDefinitionVersion to check
definition.updated_at and provide a fallback (e.g., use definition.created_at or
timezone-aware datetime.now()) before assigning to saved_at so saved_at is never
None—modify the builder/constructor call that sets saved_at (reference:
definition.updated_at, saved_at, WorkflowDefinitionVersion, AwareDatetime) to
use a conditional/ternary or helper function that returns a valid AwareDatetime
when updated_at is None.
In `@web/src/pages/workflow-editor/VersionDiffViewer.tsx`:
- Around line 94-102: VersionDiffViewer currently uses a raw setTimeout to defer
calling clearDiff when the Dialog closes, which allows a stale timer to clear a
newly opened diff; change this by storing the timeout id in a ref (e.g.,
dialogClearTimerRef) and always clearTimeout(dialogClearTimerRef.current) before
scheduling a new timeout and in the component cleanup/unmount handler;
specifically update the Dialog onOpenChange handler to clear any existing timer
when open becomes true or before creating the 150ms timeout that calls
clearDiff, and also clear the ref in useEffect cleanup so loadDiff/diffResult
cannot be wiped by a previously scheduled timer.
In `@web/src/stores/workflow-editor.ts`:
- Around line 651-661: The success branch after rollbackWorkflow(defn.id, ...)
must fully hydrate the editor state from the returned WorkflowDefinition instead
of relying on loadDefinition() to replace nodes/edges/YAML; immediately replace
the store's definition and editor-derived fields (e.g., nodes/edges/YAML/state),
clear saving/dirty and set diffResult to null, and invalidate any pending diff
request before calling
useWorkflowEditorStore.getState().loadDefinition(updated.id) and loadVersions();
keep the existing check for get().error after loadDefinition() and bail if set.
Ensure you update the same store setters used elsewhere (set, get,
useWorkflowEditorStore) so the UI reflects the rolled-back version even if the
subsequent GET fails.
- Around line 175-182: When the active workflow/definition changes, invalidate
the versions/diff slice and bump the request tokens instead of leaving them or
resetting to 0: in the handlers that change the active definition
(loadDefinition, createDefinition, and reset) clear versions, diffResult,
versionsHasMore, versionsLoading, diffLoading and increment _versionsRequestId
and _diffRequestId (monotonic ++), so any in-flight loadVersions()/loadDiff()
results will fail the token check; do the same wherever versions/diff state is
reinitialized (the blocks around the ranges you flagged) to ensure the tokens
always advance when switching workflows.
- Around line 599-614: In loadMoreVersions ensure it guards against concurrent
calls by checking and returning early if get().versionsLoading is true or
get().versionsHasMore is false before computing offset/request id; move the
const reqId/offset and set({ versionsLoading: true, _versionsRequestId: reqId })
to run only after those checks, so loadMoreVersions returns immediately when
already loading or no more versions and prevents duplicate requests/duplicate
appends in the versions array.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: bb558d97-e5cc-4d51-9009-8ced2efa2fd8
📒 Files selected for processing (3)
src/synthorg/api/controllers/_workflow_helpers.pyweb/src/pages/workflow-editor/VersionDiffViewer.tsxweb/src/stores/workflow-editor.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Dashboard Test
- GitHub Check: Test (Python 3.14)
- GitHub Check: Build Web
- GitHub Check: Build Sandbox
- GitHub Check: Build Backend
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations-- Python 3.14 has PEP 649.
PEP 758 except syntax: useexcept A, B:(no parentheses) for exception handling.
All public functions require type hints. Use mypy strict mode.
Docstrings must use Google style and are required on public classes and functions (enforced by ruff D rules).
Create new objects instead of mutating existing ones. For non-Pydantic internal collections, use copy.deepcopy() at construction and MappingProxyType wrapping for read-only enforcement.
Use frozen Pydantic models for config/identity. Use separate mutable-via-copy models for runtime state that evolves.
Use Pydantic v2 (BaseModel, model_validator, computed_field, ConfigDict). Use allow_inf_nan=False in all ConfigDict declarations.
Use NotBlankStr (from core.types) for all identifier/name fields in Pydantic models.
Use@computed_fieldfor derived values instead of storing and validating redundant fields.
Prefer asyncio.TaskGroup for fan-out/fan-in parallel operations in new code (e.g., multiple tool invocations, parallel agent calls). Prefer structured concurrency over bare create_task.
Functions must be less than 50 lines and files must be less than 800 lines.
Handle errors explicitly, never silently swallow them.
Validate at system boundaries (user input, external APIs, config files).
Line length: 88 characters (ruff).
Python: 3.14+ (PEP 649 native lazy annotations).
Files:
src/synthorg/api/controllers/_workflow_helpers.py
src/synthorg/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/synthorg/**/*.py: Every module with business logic must have: from synthorg.observability import get_logger then logger = get_logger(name)
Never use import logging / logging.getLogger() / print() in application code. Exception: observability/setup.py, observability/sinks.py, observability/syslog_handler.py, and observability/http_handler.py may use stdlib logging and print for handler construction.
Logger variable name must always be 'logger' (not '_logger', not 'log').
Use constants from domain-specific modules under synthorg.observability.events for event names (e.g., API_REQUEST_STARTED, TOOL_INVOKE_START). Import directly from synthorg.observability.events..
Always use logger.info(EVENT, key=value) for structured logging. Never use logger.info('msg %s', val).
All error paths must log at WARNING or ERROR with context before raising.
All state transitions must log at INFO level.
DEBUG logging is used for object creation, internal flow, and entry/exit of key functions.
Never use real vendor names (Anthropic, OpenAI, Claude, GPT) in project-owned code, docstrings, comments, tests, or config examples. Use generic names: example-provider, example-large-001, example-medium-001, example-small-001.
Files:
src/synthorg/api/controllers/_workflow_helpers.py
src/**/*.py
⚙️ CodeRabbit configuration file
This project uses Python 3.14+ with PEP 758 except syntax: "except A, B:" (comma-separated, no parentheses) is correct and mandatory -- do NOT flag it as a typo or suggest parenthesized form. The "except builtins.MemoryError, RecursionError: raise" pattern is intentional project convention for system-error propagation. When evaluating the 50-line function limit, count only the function body excluding the signature lines, decorators, and docstring. Functions 1-5 lines over due to docstrings or multi-line signatures should not be flagged. Do not suggest extracting single-use helper functions called exactly once -- this reduces readability without improving maintainability.
Files:
src/synthorg/api/controllers/_workflow_helpers.py
web/src/**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
web/src/**/*.{tsx,ts}: Never hardcode hex colors, font-family, pixel spacing, or Framer Motion transitions in web code. Use design tokens and@/lib/motionpresets.
Web dashboard: use React 19, TypeScript 6.0+, Tailwind CSS 4, Zustand, shadcn/ui, Radix UI, and other specified dependencies from web/package.json.
Files:
web/src/stores/workflow-editor.tsweb/src/pages/workflow-editor/VersionDiffViewer.tsx
web/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{ts,tsx,js,jsx}: Always usecreateLoggerfrom@/lib/loggerinstead of bareconsole.warn,console.error, orconsole.debugin application code
Use variable namelogfor logger instances (e.g.,const log = createLogger('module-name'))
Uselog.debug()for dev-only logging (stripped in production),log.warn()for warnings, andlog.error()for errors
Pass dynamic/untrusted values as separate arguments to logger calls (not interpolated into the message string) so they go throughsanitizeArg
Wrap attacker-controlled fields inside structured objects withsanitizeForLog()before embedding in logs
Files:
web/src/stores/workflow-editor.tsweb/src/pages/workflow-editor/VersionDiffViewer.tsx
web/src/**/*.{ts,tsx,css}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{ts,tsx,css}: Use Tailwind semantic color classes (text-foreground,bg-card,text-accent,text-success,bg-danger, etc.) or CSS variables (var(--so-accent)) instead of hardcoded hex values
Usefont-sansorfont-monofor typography (maps to Geist tokens), never setfontFamilydirectly
Use density-aware spacing tokens (p-card,gap-section-gap,gap-grid-gap) or standard Tailwind spacing, never hardcode pixel values for layout spacing
Use token variables (var(--so-shadow-card-hover),border-border,border-bright) for shadows and borders instead of hardcoded values
Do NOT usergba()with hardcoded values -- use design token variables
A PostToolUse hook (scripts/check_web_design_system.py) runs automatically on every Edit/Write toweb/src/files and catches design system violations; fix all violations before proceeding
Files:
web/src/stores/workflow-editor.tsweb/src/pages/workflow-editor/VersionDiffViewer.tsx
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{ts,tsx}: Do NOT hardcode Framer Motion transition durations -- use presets from@/lib/motion
CSS side-effect imports in TypeScript files should use type declarations (/// <reference types="vite/client" />covers this); TypeScript 6 defaultsnoUncheckedSideEffectImportsto true
Files:
web/src/stores/workflow-editor.tsweb/src/pages/workflow-editor/VersionDiffViewer.tsx
web/src/pages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/pages/**/*.{ts,tsx}: Always reuse existing components fromweb/src/components/ui/before creating new ones
UseStatusBadgecomponent for agent/task/system status indicators (colored dot + optional built-in label toggle)
UseMetricCardcomponent for numeric KPIs with sparkline, change badge, progress bar
UseSparklinecomponent for inline SVG trend lines withcolor?andanimated?props (used inside MetricCard or standalone)
UseSectionCardcomponent for titled card wrapper with icon and action slot
UseAgentCardcomponent for agent display (avatar, name, role, status, current task)
UseDeptHealthBarcomponent for department utilization (animated fill bar +health?+agentCount)
UseProgressGaugecomponent for circular or linear gauge for budget/utilization (variant?defaults to'circular',max?defaults to 100)
UseStatPillcomponent for compact inline label + value pair
UseAvatarcomponent for circular initials avatar with optionalborderColor?prop
UseToast/ToastContainercomponent for success/error/warning/info notifications with auto-dismiss queue (mountToastContaineronce in AppLayout)
UseSkeleton/SkeletonCard/SkeletonMetric/SkeletonTable/SkeletonTextcomponents for loading placeholders matching component shapes (shimmer animation, respectsprefers-reduced-motion)
UseEmptyStatecomponent for no-data / no-results placeholder with icon, title, description, optional action button
UseErrorBoundarycomponent for React error boundary with retry --levelprop:page/section/component
UseConfirmDialogcomponent for confirmation modal (Radix AlertDialog) withdefault/destructivevariants andloadingstate
UseCommandPalettecomponent for global Cmd+K search (cmdk + React Router) -- mount once in AppLayout, register commands viauseCommandPalettehook
UseInlineEditcomponent for click-to-edit text with Enter/Escape, validation, optimistic save with rollback
Use...
Files:
web/src/pages/workflow-editor/VersionDiffViewer.tsx
🧠 Learnings (34)
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/api/**/*.py : API package (api/): Litestar REST + WebSocket with controllers, guards, channels, JWT + API key + WS ticket auth, approval gate integration, coordination endpoint, collaboration endpoint, settings endpoint, provider management endpoint (CRUD + test + presets), backup endpoint, RFC 9457 structured errors, AppState hot-reload slots, service auto-wiring (Phase 1 at construction, Phase 2 on startup), lifecycle helpers
Applied to files:
src/synthorg/api/controllers/_workflow_helpers.py
📚 Learning: 2026-03-31T16:09:24.320Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T16:09:24.320Z
Learning: Applies to src/synthorg/**/*.py : Use event name constants from `synthorg.observability.events.<domain>` modules (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`); import directly and use in structured logging
Applied to files:
src/synthorg/api/controllers/_workflow_helpers.py
📚 Learning: 2026-03-19T11:33:01.580Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:33:01.580Z
Learning: Applies to src/synthorg/**/*.py : Use event constants from `synthorg.observability.events.<domain>` (e.g., `API_REQUEST_STARTED` from `events.api`); import directly and log with structured kwargs: `logger.info(EVENT, key=value)`, never interpolated strings
Applied to files:
src/synthorg/api/controllers/_workflow_helpers.py
📚 Learning: 2026-03-16T06:24:56.341Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T06:24:56.341Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from the domain-specific module under `synthorg.observability.events` in logging calls
Applied to files:
src/synthorg/api/controllers/_workflow_helpers.py
📚 Learning: 2026-03-18T21:23:23.586Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-18T21:23:23.586Z
Learning: Applies to src/synthorg/**/*.py : Event names: always use constants from the domain-specific module under synthorg.observability.events (e.g., API_REQUEST_STARTED from events.api, TOOL_INVOKE_START from events.tool). Import directly from synthorg.observability.events.<domain>.
Applied to files:
src/synthorg/api/controllers/_workflow_helpers.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to src/synthorg/**/*.py : Always use event name constants from domain-specific modules under `synthorg.observability.events` (e.g., `API_REQUEST_STARTED` from `events.api`, `TOOL_INVOKE_START` from `events.tool`); import directly rather than using string literals
Applied to files:
src/synthorg/api/controllers/_workflow_helpers.py
📚 Learning: 2026-03-17T22:08:13.456Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T22:08:13.456Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions.
Applied to files:
src/synthorg/api/controllers/_workflow_helpers.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, and key function entry/exit
Applied to files:
src/synthorg/api/controllers/_workflow_helpers.py
📚 Learning: 2026-03-17T06:43:14.114Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T06:43:14.114Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising. All state transitions must log at INFO. DEBUG for object creation, internal flow, entry/exit of key functions. Pure data models, enums, and re-exports do NOT need logging.
Applied to files:
src/synthorg/api/controllers/_workflow_helpers.py
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/observability/**/*.py : Observability package (observability/): structured logging, correlation tracking, log sinks; event constants organized by domain under observability/events/ (e.g., events.api, events.tool, events.git, events.context_budget, events.backup)
Applied to files:
src/synthorg/api/controllers/_workflow_helpers.py
📚 Learning: 2026-03-31T20:07:03.035Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T20:07:03.035Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising; all state transitions must log at INFO; DEBUG for object creation, internal flow, entry/exit of key functions
Applied to files:
src/synthorg/api/controllers/_workflow_helpers.py
📚 Learning: 2026-04-03T07:50:58.367Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-03T07:50:58.367Z
Learning: Applies to **/*.py : Use Google-style docstrings, required on all public classes and functions (enforced by ruff D rules)
Applied to files:
src/synthorg/api/controllers/_workflow_helpers.py
📚 Learning: 2026-03-16T10:40:25.144Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T10:40:25.144Z
Learning: Applies to **/*.py : Docstrings: Google style, required on public classes and functions (enforced by ruff D rules).
Applied to files:
src/synthorg/api/controllers/_workflow_helpers.py
📚 Learning: 2026-04-04T17:42:47.768Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-04T17:42:47.768Z
Learning: Applies to **/*.py : Docstrings must use Google style and are required on public classes and functions (enforced by ruff D rules).
Applied to files:
src/synthorg/api/controllers/_workflow_helpers.py
📚 Learning: 2026-03-30T10:41:40.176Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:41:40.176Z
Learning: Applies to web/src/**/*.{ts,tsx} : Use Zustand stores for state management in the web dashboard; each domain has its own store module (auth, WebSocket, toast, analytics, setup, company, agents, budget, tasks, settings, providers, theme, per-domain stores)
Applied to files:
web/src/stores/workflow-editor.ts
📚 Learning: 2026-03-27T12:44:29.466Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-27T12:44:29.466Z
Learning: Applies to web/src/stores/**/*.{ts,tsx} : Use Zustand stores in web dashboard for state management (auth, WebSocket, toast, analytics, domain shells)
Applied to files:
web/src/stores/workflow-editor.ts
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `MetadataGrid` component for key-value metadata grid for detail pages with configurable columns (2/3/4), density-aware spacing
Applied to files:
web/src/pages/workflow-editor/VersionDiffViewer.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `ConfirmDialog` component for confirmation modal (Radix AlertDialog) with `default` / `destructive` variants and `loading` state
Applied to files:
web/src/pages/workflow-editor/VersionDiffViewer.tsx
📚 Learning: 2026-04-04T17:42:47.768Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-04T17:42:47.768Z
Learning: Applies to web/src/**/*.{tsx,ts} : Web dashboard: use React 19, TypeScript 6.0+, Tailwind CSS 4, Zustand, shadcn/ui, Radix UI, and other specified dependencies from web/package.json.
Applied to files:
web/src/pages/workflow-editor/VersionDiffViewer.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `InlineEdit` component for click-to-edit text with Enter/Escape, validation, optimistic save with rollback
Applied to files:
web/src/pages/workflow-editor/VersionDiffViewer.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `CodeMirrorEditor` component for CodeMirror 6 editor with JSON/YAML modes, design-token dark theme, line numbers, bracket matching, `readOnly` support
Applied to files:
web/src/pages/workflow-editor/VersionDiffViewer.tsx
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/**/*.{tsx,ts} : ALWAYS reuse existing components from `web/src/components/ui/` before creating new ones (StatusBadge, MetricCard, Sparkline, SectionCard, AgentCard, DeptHealthBar, ProgressGauge, StatPill, Avatar, Button, Toast, Skeleton, EmptyState, ErrorBoundary, ConfirmDialog, CommandPalette, InlineEdit, AnimatedPresence, StaggerGroup, Drawer, InputField, SelectField, SliderField, ToggleField, TaskStatusIndicator, PriorityBadge, ProviderHealthBadge, TokenUsageBar, CodeMirrorEditor, SegmentedControl, ThemeToggle, LiveRegion, MobileUnsupportedOverlay, LazyCodeMirrorEditor, TagInput, MetadataGrid, ProjectStatusBadge, ContentTypeBadge)
Applied to files:
web/src/pages/workflow-editor/VersionDiffViewer.tsx
📚 Learning: 2026-03-30T10:20:08.544Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:20:08.544Z
Learning: Applies to web/src/**/*.{ts,tsx} : Always reuse existing components from web/src/components/ui/ (StatusBadge, MetricCard, Sparkline, SectionCard, AgentCard, DeptHealthBar, ProgressGauge, StatPill, Avatar, Button, Toast/ToastContainer, Skeleton variants, EmptyState, ErrorBoundary, ConfirmDialog, CommandPalette, InlineEdit, AnimatedPresence, StaggerGroup/StaggerItem, Drawer, form fields, TaskStatusIndicator, PriorityBadge, ProviderHealthBadge, TokenUsageBar, CodeMirrorEditor, SegmentedControl, ThemeToggle, LiveRegion, MobileUnsupportedOverlay, LazyCodeMirrorEditor) before creating new components
Applied to files:
web/src/pages/workflow-editor/VersionDiffViewer.tsx
📚 Learning: 2026-03-27T12:44:29.466Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-27T12:44:29.466Z
Learning: Applies to web/src/**/*.{ts,tsx} : Always reuse existing components from `web/src/components/ui/` (StatusBadge, MetricCard, Sparkline, SectionCard, AgentCard, DeptHealthBar, ProgressGauge, StatPill, Avatar, Button, Toast, Skeleton, EmptyState, ErrorBoundary, ConfirmDialog, CommandPalette, InlineEdit, AnimatedPresence, StaggerGroup/StaggerItem) before creating new ones
Applied to files:
web/src/pages/workflow-editor/VersionDiffViewer.tsx
📚 Learning: 2026-03-30T10:20:08.544Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:20:08.544Z
Learning: Applies to web/src/**/*.{ts,tsx} : Web dashboard shadows/borders: use token variables (var(--so-shadow-card-hover), border-border, border-bright); never hardcode shadow or border values
Applied to files:
web/src/pages/workflow-editor/VersionDiffViewer.tsx
📚 Learning: 2026-04-01T20:43:51.878Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T20:43:51.878Z
Learning: Applies to web/src/**/*.{ts,tsx} : Always reuse existing components from `web/src/components/ui/` before creating new ones. Never hardcode hex colors, font-family, pixel spacing, or Framer Motion transitions -- use design tokens and `@/lib/motion` presets.
Applied to files:
web/src/pages/workflow-editor/VersionDiffViewer.tsx
📚 Learning: 2026-04-04T17:42:47.768Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-04T17:42:47.768Z
Learning: Applies to web/src/components/**/*.{tsx,ts} : Always reuse existing components from web/src/components/ui/ before creating new ones in the web dashboard.
Applied to files:
web/src/pages/workflow-editor/VersionDiffViewer.tsx
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/**/*.{tsx,ts} : Use Tailwind semantic classes (`text-foreground`, `bg-card`, `text-accent`, `text-success`, `bg-danger`) or CSS variables (`var(--so-*)`) for colors. NEVER hardcode hex values or rgba() in `.tsx`/`.ts` files
Applied to files:
web/src/pages/workflow-editor/VersionDiffViewer.tsx
📚 Learning: 2026-03-30T10:41:40.176Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:41:40.176Z
Learning: Applies to web/src/**/*.{ts,tsx} : ALWAYS reuse existing components from `web/src/components/ui/` before creating new ones; refer to design system inventory (StatusBadge, MetricCard, Sparkline, SectionCard, AgentCard, etc.)
Applied to files:
web/src/pages/workflow-editor/VersionDiffViewer.tsx
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/components/ui/**/*.tsx : Use design tokens exclusively in new components -- no hardcoded colors, fonts, or spacing
Applied to files:
web/src/pages/workflow-editor/VersionDiffViewer.tsx
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/**/*.{tsx,ts} : Do NOT create complex (>8 line) JSX inside `.map()` -- extract to a shared component
Applied to files:
web/src/pages/workflow-editor/VersionDiffViewer.tsx
📚 Learning: 2026-03-30T10:41:40.176Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:41:40.176Z
Learning: Applies to web/src/**/*.{ts,tsx} : Do NOT create complex (>8 line) JSX inside `.map()`; extract to a shared component
Applied to files:
web/src/pages/workflow-editor/VersionDiffViewer.tsx
📚 Learning: 2026-03-30T10:20:08.544Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:20:08.544Z
Learning: Applies to web/src/**/*.{ts,tsx} : Do NOT create complex (>8 line) JSX inside .map()—extract to a shared component
Applied to files:
web/src/pages/workflow-editor/VersionDiffViewer.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Do NOT create complex (>8 line) JSX inside `.map()` -- extract to a shared component
Applied to files:
web/src/pages/workflow-editor/VersionDiffViewer.tsx
🔇 Additional comments (1)
src/synthorg/api/controllers/_workflow_helpers.py (1)
65-65: The concern aboutsave()modifyingupdated_atin-place or reloading the definition cannot occur.
WorkflowDefinitionis a frozen Pydantic model (ConfigDict(frozen=True)), making it immutable. Thesave()method returnsNoneand does not reassign the definition parameter or modify it. The SQLite implementation persists the definition's current state to the database without mutating the in-memory object. The snapshot correctly capturesdefinition.updated_atfrom the state that was persisted, which is the intended behavior as documented inbuild_version_snapshot's docstring: "The snapshot'ssaved_atis set to the definition'supdated_attimestamp, not the current time."
| edges=definition.edges, | ||
| created_by=definition.created_by, | ||
| saved_by=saved_by, | ||
| saved_at=definition.updated_at, |
There was a problem hiding this comment.
Missing None check for definition.updated_at before assigning to non-nullable saved_at.
Line 65 assigns definition.updated_at directly to saved_at without validating that it is not None. According to context snippet 4, saved_at is typed as non-nullable AwareDatetime. Context snippet 3 warns that updated_at could be None in edge cases (e.g., freshly-created definitions before a DB round-trip, or if explicitly set to None). If updated_at is None, Pydantic validation will fail when constructing the WorkflowDefinitionVersion.
🛡️ Proposed fix to add None check and default
+from datetime import UTC, datetime
+
def build_version_snapshot(
definition: WorkflowDefinition,
saved_by: str,
) -> WorkflowDefinitionVersion:
"""Build a version snapshot from a definition.
The snapshot's ``saved_at`` is set to the definition's
``updated_at`` timestamp, not the current time.
Args:
definition: The workflow definition to snapshot.
saved_by: User ID of who triggered the save.
Returns:
An immutable ``WorkflowDefinitionVersion`` snapshot.
"""
+ if definition.updated_at is None:
+ raise ValueError(
+ f"Cannot snapshot definition {definition.id}: updated_at is None"
+ )
+
return WorkflowDefinitionVersion(
definition_id=definition.id,
version=definition.version,
name=definition.name,
description=definition.description,
workflow_type=definition.workflow_type,
nodes=definition.nodes,
edges=definition.edges,
created_by=definition.created_by,
saved_by=saved_by,
saved_at=definition.updated_at,
)Alternatively, if a fallback to the current time is acceptable:
+from datetime import UTC, datetime
+
saved_by=saved_by,
- saved_at=definition.updated_at,
+ saved_at=definition.updated_at or datetime.now(UTC),
)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/api/controllers/_workflow_helpers.py` at line 65, The assignment
of definition.updated_at to the non-nullable saved_at field in
WorkflowDefinitionVersion may raise validation errors if updated_at is None;
update the code that builds WorkflowDefinitionVersion to check
definition.updated_at and provide a fallback (e.g., use definition.created_at or
timezone-aware datetime.now()) before assigning to saved_at so saved_at is never
None—modify the builder/constructor call that sets saved_at (reference:
definition.updated_at, saved_at, WorkflowDefinitionVersion, AwareDatetime) to
use a conditional/ternary or helper function that returns a valid AwareDatetime
when updated_at is None.
Add a library of 5 starter workflow blueprints (Feature Pipeline, Bug Fix Flow, Research Sprint, Code Review Pipeline, Onboarding Workflow) that users can instantiate from the create drawer. Backend: - BlueprintData/BlueprintNodeData/BlueprintEdgeData models - Blueprint loader with builtin + user override support - GET /workflows/blueprints and POST /workflows/from-blueprint endpoints - Observability events for blueprint operations Frontend: - BlueprintPicker component with card grid, loading, error states - SegmentedControl toggle (Blank/From Template) in WorkflowCreateDrawer - Auto-fill name/description from selected blueprint - createFromBlueprint store action and API function - BlueprintInfo TypeScript types Closes #1006
Add version tracking to workflow definitions. Each save creates an
immutable version snapshot. Users can browse version history, view
visual diffs between versions, and rollback to previous versions.
Backend:
- WorkflowDefinitionVersion model (immutable snapshot)
- workflow_definition_versions table with composite PK
- WorkflowVersionRepository protocol + SQLite implementation
- compute_diff() pure function (node/edge/metadata change detection)
- GET /{id}/versions, GET /{id}/versions/{v}, GET /{id}/diff, POST /{id}/rollback
- Auto-snapshot on create and update
- Observability events for version operations
Frontend:
- VersionHistoryPanel (right drawer with version list, compare, restore)
- VersionDiffViewer (modal with color-coded node/edge/metadata changes)
- History button in WorkflowToolbar
- Version-related state and actions in workflow-editor store
- TypeScript types for versions, diffs, and rollback
Closes #1008
The list_by_user, list_all, revoke_all_for_user, and load_revoked methods filter by expires_at > now. Tests used a fixed _NOW but did not patch datetime.now() for these calls, causing failures once wall-clock time exceeded the 24-hour session expiry window.
Implements #1006 (workflow templates/starter blueprints) and #1008 (workflow versioning with diff view and rollback). Pre-reviewed by 18 agents, 39 findings addressed: - Critical: fixed version snapshot ordering (save after definition), added missing version snapshot in create_from_blueprint, added logging on 7 error paths, cascade-delete version snapshots - Major: extracted WorkflowVersionController (800-line limit), tightened DTO types (NotBlankStr, WorkflowType, Literal), added model validators, fixed pagination metadata, improved test coverage and accessibility - Medium: design token fixes, documentation updates, symlink escape check, Counter refactor, direct config comparison
…t, and Gemini Backend: - Extract shared _workflow_helpers.py (DRY: build_version_snapshot, get_auth_user_id) - Add auth fallback logging when no AuthenticatedUser in scope - Wrap save_version calls in try/except QueryError (non-atomic save safety net) - Fix TOCTOU symlink race in blueprint loader direct load path - Add ValidationError to _parse_blueprint_yaml except clause - Reject blueprint name/file mismatches (was warn-only) - Add logging before all BlueprintValidationError raises - Upgrade builtin load failures from WARNING to ERROR - Add display_name validation to BlueprintInfo dataclass - Replace bare except Exception with sqlite3.Error in version repo - Add logger + warning to diff.py compute_diff error path - Enforce from_version < to_version in WorkflowDiff validator - Extract shared _validate_change_values helper for NodeChange/EdgeChange - Add max_length constraints to CreateFromBlueprintRequest fields - Use generic conflict messages in 409 responses (no raw exc strings) - Refactor 7 functions exceeding 50-line limit into extracted helpers - Add cascade delete defense-in-depth comment - Add UTC convention docstring to SQLite version repo - Add WORKFLOW_VERSION_SNAPSHOT_FAILED event constant Tests: - Rewrite test_rollback_success to exercise full rollback flow - Move FakeWorkflowVersionRepository to fakes_workflow.py (800-line limit) - Add deepcopy to FakeWorkflowVersionRepository.list_versions - Add compute_diff cross-definition ValueError test - Add NodeChange/EdgeChange/WorkflowDiff model validator tests Frontend: - Rewrite VersionDiffViewer to use shared Dialog primitive (focus trap, a11y) - Create reusable Dialog component (Radix-based) - Extract MetadataChangeRow, NodeChangeRow, EdgeChangeRow components - Extract VersionCard from VersionHistoryPanel map JSX - Extract BlueprintCard from BlueprintPicker map JSX - Consolidate blueprint fetching into workflows store (remove duplicate) - Add stale request guards to loadVersions and loadDiff - Add versionsHasMore + loadMoreVersions pagination - Clear diffResult after rollback - Fix rollback loadDefinition error handling - Add parameters.a11y.test to all 3 new stories - Add visible story states for VersionDiffViewer - Fix BlueprintPicker stories MSW mock envelope - Export BlueprintPickerProps interface - Remove redundant selectedBlueprint guard in WorkflowCreateDrawer - Add aria-hidden to Clock icon + time element in VersionHistoryPanel - Wrap log.warn err args with sanitizeForLog Docs: - Add version API endpoints to page-structure.md Workflow Editor section - Add WorkflowVersionController to Controller-to-Page Map - Update stale controller count (23 -> 32) in page-structure.md - Update endpoint count (13 -> 14) in engine.md - Add blueprints/versioning to DESIGN_SPEC.md engine page description
- Add validate-draft to engine.md endpoint list (count stays 14)
- Replace raw auth_user_fallback string with API_AUTH_USER_FALLBACK constant
- Fix code-review-pipeline.yaml description (revision goes to end, not loop)
- Remove ambiguous agent_name: '' from onboarding-workflow.yaml
- Export DialogHeaderProps/DialogTitleProps/DialogDescriptionProps interfaces
- Add Loading, Error, WithForm dialog stories
- Reset diffResult in Default VersionDiffViewer story decorator
- Remove redundant set({ saving: false }) in rollback success path
- Fix import ordering in workflows.ts (types before executable code)
- Add Google-style Args/Returns sections to _workflow_helpers docstrings - Delay clearDiff() by 150ms so Radix Dialog exit animation completes before diffResult content is unmounted - Add per-request tokens (_versionsRequestId, _diffRequestId) to workflow-editor store to discard stale async responses; use functional set() in loadMoreVersions to avoid racey reads; clearDiff increments the diff token to invalidate in-flight responses - Fix import ordering in workflow-editor.ts (types before executable code)
- Use ref-tracked timer for clearDiff setTimeout to prevent stale timer from clearing a newly opened diff (cancel on open/unmount) - Hydrate editor state (nodes/edges/yaml) immediately from rollback response via parseDefinition() so UI reflects rolled-back version even if subsequent reload fails - Invalidate versions/diff state and bump request tokens when loadDefinition switches to a new workflow - Guard loadMoreVersions against concurrent calls (return early if versionsLoading or !versionsHasMore) - Extract parseDefinition() helper shared by loadDefinition and rollback
52d3303 to
8e368ec
Compare
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/stores/workflows.ts (1)
95-119: 🧹 Nitpick | 🔵 TrivialDeduplicate workflow upsert logic shared by both create paths.
createWorkflowandcreateFromBlueprintrepeat the same state-upsert block. Extracting a shared helper will reduce drift risk in future changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/stores/workflows.ts` around lines 95 - 119, Both createWorkflow and createFromBlueprint duplicate the same state upsert logic; extract that block into a single helper (e.g., upsertWorkflowInState or setWorkflowUpsert) and call it from both functions. The helper should accept the new workflow object and call set(state => { const exists = state.workflows.some(w => w.id === workflow.id); const filtered = state.workflows.filter(w => w.id !== workflow.id); return { workflows: [workflow, ...filtered], totalWorkflows: exists ? state.totalWorkflows : state.totalWorkflows + 1 }; }). Replace the duplicated set(...) blocks in createWorkflow and createFromBlueprint with calls to this helper to centralize the logic.
♻️ Duplicate comments (2)
web/src/stores/workflow-editor.ts (1)
720-727: 🧹 Nitpick | 🔵 TrivialConsider monotonically incrementing request IDs instead of resetting to 0.
Resetting
_versionsRequestIdand_diffRequestIdto0inreset()theoretically allows a stale in-flight response from a previous session to match if the new session happens to reach the same request ID. While extremely unlikely in practice, monotonically incrementing (or using a new base value) would be safer.♻️ Alternative: increment instead of reset
diffResult: null, diffLoading: false, - _versionsRequestId: 0, - _diffRequestId: 0, + _versionsRequestId: get()._versionsRequestId + 1, + _diffRequestId: get()._diffRequestId + 1, })This requires changing
resetto use the function form ofsetto accessget().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/stores/workflow-editor.ts` around lines 720 - 727, The reset() implementation should avoid resetting _versionsRequestId and _diffRequestId to 0; instead read the current values and monotonically increment them so stale in-flight responses cannot collide with new sessions. Modify reset() to use the functional form of set (e.g., set(prev => ({ ...prev, versionHistoryOpen: false, versions: [], versionsLoading: false, versionsHasMore: false, diffResult: null, diffLoading: false, _versionsRequestId: prev._versionsRequestId + 1, _diffRequestId: prev._diffRequestId + 1 }))) so you increment the existing _versionsRequestId and _diffRequestId rather than resetting them to zero.src/synthorg/api/controllers/workflow_versions.py (1)
138-169:⚠️ Potential issue | 🟠 MajorMissing validation:
target_versionmust be less thanexpected_version.The
_fetch_rollback_targetfunction validates thatexpected_versionmatches the current definition version, but does not reject rollbacks wheretarget_version >= expected_version. Per PR objectives ("rollback to a previous version"), rolling back to the current or a future version is semantically invalid.🛡️ Proposed fix
if data.expected_version != existing.version: logger.warning( WORKFLOW_DEF_VERSION_CONFLICT, definition_id=workflow_id, expected=data.expected_version, actual=existing.version, ) return Response( content=ApiResponse[WorkflowDefinition]( error="Version conflict: the workflow was modified. Reload and retry.", ), status_code=409, ) + if data.target_version >= data.expected_version: + logger.warning( + WORKFLOW_DEF_INVALID_REQUEST, + definition_id=workflow_id, + error="target_version must be less than expected_version", + ) + return Response( + content=ApiResponse[WorkflowDefinition]( + error=( + f"Cannot rollback to version {data.target_version}: " + f"must be less than current version {data.expected_version}" + ), + ), + status_code=400, + ) + target = await version_repo.get_version(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/synthorg/api/controllers/workflow_versions.py` around lines 138 - 169, In _fetch_rollback_target, add validation that data.target_version < data.expected_version before calling version_repo.get_version: if data.target_version is greater than or equal to data.expected_version, log a warning (use an appropriate log key such as WORKFLOW_DEF_INVALID_ROLLBACK) including definition_id, expected and target, and return a Response with an error message like "Target version must be less than expected_version" and a 400 (or 422) status code; keep the existing version conflict check and the subsequent get_version logic unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/synthorg/api/controllers/workflows.py`:
- Around line 417-429: The current flow saves the definition (repo.save) then
attempts version_repo.save_version and only logs on QueryError, leaving an
inconsistent state; fix by ensuring atomicity: either wrap both
repo.save(definition) and version_repo.save_version(snapshot) in a single
persistence transaction (use your persistence transaction API on
state.app_state.persistence to begin/commit/rollback so both operations succeed
or both rollback), or if transactions aren't available, on QueryError
immediately delete/rollback the saved definition (repo.delete(definition.id) or
equivalent) and re-raise the error so the request fails; apply the same pattern
to the other occurrences that call build_version_snapshot and
version_repo.save_version (the blocks around WORKFLOW_VERSION_SNAPSHOT_FAILED
and the other referenced ranges).
- Around line 248-267: Replace the ad-hoc ApiResponse error bodies in the
404/409/422 paths with RFC 9457 problem-detail responses (use the framework's
ProblemDetail/HTTPException helper rather than ApiResponse), i.e., for the "not
found" branch and the version-conflict branch where
WORKFLOW_DEF_VERSION_CONFLICT, workflow_id and existing.version are available,
return/raise a ProblemDetail object (or raise the appropriate Litestar
HTTPException that produces an RFC 9457 payload) with status, title, and detail
populated (e.g., "Workflow definition not found" / "Version conflict: the
workflow was modified. Reload and retry.") and include relevant
instance/metadata as needed; apply the same replacement for the other similar
blocks referenced (_fetch_existing_for_update()/_load_blueprint_or_error() and
the code region around lines 292-309).
In `@tests/unit/api/controllers/test_workflow_blueprints.py`:
- Around line 18-26: The tests assume only builtins are loaded but the loader
also reads user blueprints; add an autouse pytest fixture that isolates
user-blueprint loading by monkeypatching
synthorg.engine.workflow.blueprint_loader._USER_BLUEPRINTS_DIR to an empty
temporary directory (tmp_path) for all tests in this module (and similar modules
covering lines 29-53, 63-170), and explicitly override that fixture only in
tests that need to exercise user blueprints; reference the module symbol
_USER_BLUEPRINTS_DIR and use pytest's monkeypatch or tmp_path fixture to set it
before imports/use and ensure it is reset after each test.
In `@tests/unit/api/controllers/test_workflow_versions.py`:
- Around line 117-128: The TestListVersions class currently only tests the
nonexistent-workflow case; add one or more positive tests that create an
existing workflow with multiple versions and exercise the paginated
/api/v1/workflows/{id}/versions endpoint to assert HTTP 200, correct ordering
(newest-first or the API's specified order), page-window behavior (limit/offset
or page/size params), and total/count behavior; implement this by adding tests
(e.g. test_list_versions_paginated and/or test_list_versions_ordering) that use
the test client and fixtures to create several versions for a workflow id, call
the endpoint with different pagination params, and assert resp.json()["data"]
length, order of version identifiers/timestamps, and that subsequent pages yield
the remaining items.
- Around line 103-108: Add an explicit status check before parsing the response:
after the test_client.get call that requests
"/api/v1/workflows/{wf['id']}/versions" and before accessing
resp.json()["data"], assert resp.status_code == 200 so failures surface as HTTP
errors rather than KeyError/TypeError when calling resp.json(); update the test
in tests/unit/api/controllers/test_workflow_versions.py around the
test_client.get invocation accordingly.
In `@tests/unit/engine/workflow/test_blueprint_loader.py`:
- Around line 82-88: The builtin loader tests call load_blueprint() /
list_blueprints() which read from
synthorg.engine.workflow.blueprint_loader._USER_BLUEPRINTS_DIR, so pin that
variable to an empty temporary directory in
tests/unit/engine/workflow/test_blueprint_loader.py for the builtin-focused
tests (e.g., the test_loads_each_builtin and the blocks at 104-108 and 147-223)
to prevent local user overrides from being picked up; update these tests to
patch/monkeypatch blueprint_loader._USER_BLUEPRINTS_DIR to an isolated tmp_path
(or tmp_path_factory) before calling load_blueprint()/list_blueprints(), and
leave the explicit override-specific tests unchanged where you intentionally set
_USER_BLUEPRINTS_DIR to custom content.
In `@web/src/api/types.ts`:
- Around line 1757-1761: The MetadataChange interface currently forces old_value
and new_value to be string; update the type for these properties (old_value and
new_value on MetadataChange) to a widened union such as string | number |
boolean or to unknown so non-string metadata (e.g., enum-backed
numbers/booleans) are safely represented; update any simple string-only
consumers to handle the broader type or cast as needed.
In `@web/src/components/ui/dialog.stories.tsx`:
- Around line 49-147: The story file is missing the required Hover and Empty
states; add two new story components (e.g., DialogWithHover and DialogEmpty) and
export them as Hover and Empty story objects: DialogWithHover should render the
Dialog (use the same open/setOpen pattern as DialogWithForm) and include a UI
element that demonstrates the hovered state (simulate by rendering the component
in its hover visual state or adding the hover CSS class to the relevant element
so Storybook shows the hover variant); DialogEmpty should render an open
DialogContent with an empty-state message/illustration (matching the
Error/Loading patterns but showing empty content) and export render functions
like the other stories (export const Hover: Story = { render: () =>
<DialogWithHover /> } and export const Empty: Story = { render: () =>
<DialogEmpty /> }). Ensure you reference and reuse Dialog, DialogContent,
DialogHeader, DialogTitle, DialogDescription, DialogCloseButton, and the same
setOpen/open state logic to keep behavior consistent.
In `@web/src/pages/workflows/BlueprintPicker.tsx`:
- Around line 76-79: The current selection (selectedBlueprint) isn't reconciled
when workflowTypeFilter or blueprints change, so add an effect that runs when
workflowTypeFilter or blueprints (or filtered) change and checks whether
selectedBlueprint is present in the newly computed filtered list; if it is not,
clear the selection by calling the existing selection handler (e.g.,
onSelectBlueprint(undefined) or setSelectedBlueprint(null)) so the parent cannot
keep a hidden blueprint selected — apply the same reconciliation wherever
selection is managed (e.g., the selection logic around the filtered variable and
the block around lines 101-115).
- Around line 26-36: The toggle button in BlueprintPicker (the motion.button
using isSelected and onSelect) doesn't expose its state to assistive tech;
update the button to include an accessible state attribute by adding
aria-pressed={isSelected} to the motion.button (or replace the control with
appropriate radio/listbox semantics if you prefer) so screen readers can
announce which blueprint is active; ensure the attribute is applied on the same
element that handles onClick and uses isSelected to reflect the current
selection.
In `@web/src/stores/workflow-editor.ts`:
- Around line 19-20: The logger instantiation "const log =
createLogger('workflow-editor')" is placed among import statements; move this
line so it appears after all import blocks (i.e., after the remaining imports in
this module) to preserve conventional module ordering, keeping the identifier
"log" and the call to "createLogger" unchanged.
In `@web/src/stores/workflows.ts`:
- Around line 66-74: The loadBlueprints function can suffer from race conditions
when multiple calls overlap; add the same request-token guard pattern used by
fetchWorkflows: introduce a requestId/counter (or an incrementing token stored
in the store) that you capture at the start of loadBlueprints, attach it to the
async listBlueprints call, and before calling set(...) in both success and catch
branches verify the token matches the currentLatestRequestId; only then update
blueprints, blueprintsLoading, and blueprintsError so stale responses cannot
overwrite newer results.
---
Outside diff comments:
In `@web/src/stores/workflows.ts`:
- Around line 95-119: Both createWorkflow and createFromBlueprint duplicate the
same state upsert logic; extract that block into a single helper (e.g.,
upsertWorkflowInState or setWorkflowUpsert) and call it from both functions. The
helper should accept the new workflow object and call set(state => { const
exists = state.workflows.some(w => w.id === workflow.id); const filtered =
state.workflows.filter(w => w.id !== workflow.id); return { workflows:
[workflow, ...filtered], totalWorkflows: exists ? state.totalWorkflows :
state.totalWorkflows + 1 }; }). Replace the duplicated set(...) blocks in
createWorkflow and createFromBlueprint with calls to this helper to centralize
the logic.
---
Duplicate comments:
In `@src/synthorg/api/controllers/workflow_versions.py`:
- Around line 138-169: In _fetch_rollback_target, add validation that
data.target_version < data.expected_version before calling
version_repo.get_version: if data.target_version is greater than or equal to
data.expected_version, log a warning (use an appropriate log key such as
WORKFLOW_DEF_INVALID_ROLLBACK) including definition_id, expected and target, and
return a Response with an error message like "Target version must be less than
expected_version" and a 400 (or 422) status code; keep the existing version
conflict check and the subsequent get_version logic unchanged.
In `@web/src/stores/workflow-editor.ts`:
- Around line 720-727: The reset() implementation should avoid resetting
_versionsRequestId and _diffRequestId to 0; instead read the current values and
monotonically increment them so stale in-flight responses cannot collide with
new sessions. Modify reset() to use the functional form of set (e.g., set(prev
=> ({ ...prev, versionHistoryOpen: false, versions: [], versionsLoading: false,
versionsHasMore: false, diffResult: null, diffLoading: false,
_versionsRequestId: prev._versionsRequestId + 1, _diffRequestId:
prev._diffRequestId + 1 }))) so you increment the existing _versionsRequestId
and _diffRequestId rather than resetting them to zero.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 74f79812-c30e-48ce-9a3b-ca821fccfd7a
📒 Files selected for processing (57)
CLAUDE.mdREADME.mddocs/DESIGN_SPEC.mddocs/design/engine.mddocs/design/operations.mddocs/design/page-structure.mdsrc/synthorg/api/controllers/__init__.pysrc/synthorg/api/controllers/_workflow_helpers.pysrc/synthorg/api/controllers/workflow_versions.pysrc/synthorg/api/controllers/workflows.pysrc/synthorg/api/dto.pysrc/synthorg/engine/workflow/blueprint_errors.pysrc/synthorg/engine/workflow/blueprint_loader.pysrc/synthorg/engine/workflow/blueprint_models.pysrc/synthorg/engine/workflow/blueprints/__init__.pysrc/synthorg/engine/workflow/blueprints/bug-fix-flow.yamlsrc/synthorg/engine/workflow/blueprints/code-review-pipeline.yamlsrc/synthorg/engine/workflow/blueprints/feature-pipeline.yamlsrc/synthorg/engine/workflow/blueprints/onboarding-workflow.yamlsrc/synthorg/engine/workflow/blueprints/research-sprint.yamlsrc/synthorg/engine/workflow/diff.pysrc/synthorg/engine/workflow/version.pysrc/synthorg/observability/events/api.pysrc/synthorg/observability/events/blueprint.pysrc/synthorg/observability/events/workflow_definition.pysrc/synthorg/observability/events/workflow_version.pysrc/synthorg/persistence/sqlite/backend.pysrc/synthorg/persistence/sqlite/schema.sqlsrc/synthorg/persistence/sqlite/workflow_version_repo.pysrc/synthorg/persistence/workflow_version_repo.pytests/unit/api/auth/test_session_store.pytests/unit/api/controllers/test_workflow_blueprints.pytests/unit/api/controllers/test_workflow_versions.pytests/unit/api/fakes.pytests/unit/api/fakes_workflow.pytests/unit/engine/workflow/test_blueprint_loader.pytests/unit/engine/workflow/test_blueprint_models.pytests/unit/engine/workflow/test_diff.pytests/unit/engine/workflow/test_version.pytests/unit/observability/test_events.pytests/unit/persistence/sqlite/test_migrations.pytests/unit/persistence/sqlite/test_workflow_version_repo.pyweb/src/api/endpoints/workflows.tsweb/src/api/types.tsweb/src/components/ui/dialog.stories.tsxweb/src/components/ui/dialog.tsxweb/src/pages/WorkflowEditorPage.tsxweb/src/pages/workflow-editor/VersionDiffViewer.stories.tsxweb/src/pages/workflow-editor/VersionDiffViewer.tsxweb/src/pages/workflow-editor/VersionHistoryPanel.stories.tsxweb/src/pages/workflow-editor/VersionHistoryPanel.tsxweb/src/pages/workflow-editor/WorkflowToolbar.tsxweb/src/pages/workflows/BlueprintPicker.stories.tsxweb/src/pages/workflows/BlueprintPicker.tsxweb/src/pages/workflows/WorkflowCreateDrawer.tsxweb/src/stores/workflow-editor.tsweb/src/stores/workflows.ts
| return Response( | ||
| content=ApiResponse[WorkflowDefinition]( | ||
| error="Workflow definition not found", | ||
| ), | ||
| status_code=404, | ||
| ) | ||
|
|
||
| if expected_version is not None and expected_version != existing.version: | ||
| logger.warning( | ||
| WORKFLOW_DEF_VERSION_CONFLICT, | ||
| definition_id=workflow_id, | ||
| expected=expected_version, | ||
| actual=existing.version, | ||
| ) | ||
| return Response( | ||
| content=ApiResponse[WorkflowDefinition]( | ||
| error="Version conflict: the workflow was modified. Reload and retry.", | ||
| ), | ||
| status_code=409, | ||
| ) |
There was a problem hiding this comment.
These new error paths should use the standard RFC 9457 shape.
_fetch_existing_for_update() and _load_blueprint_or_error() now return ad-hoc ApiResponse.error bodies for 404/409/422 responses, so clients hitting the updated workflow endpoints have to handle a second error schema. Raise/return the existing problem-detail responses here instead. As per coding guidelines: "REST API must follow RFC 9457 error format; use Litestar framework for API setup, auth (role-based access control, JWT sessions), guards, lifecycle, and bootstrap".
Also applies to: 292-309
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/api/controllers/workflows.py` around lines 248 - 267, Replace
the ad-hoc ApiResponse error bodies in the 404/409/422 paths with RFC 9457
problem-detail responses (use the framework's ProblemDetail/HTTPException helper
rather than ApiResponse), i.e., for the "not found" branch and the
version-conflict branch where WORKFLOW_DEF_VERSION_CONFLICT, workflow_id and
existing.version are available, return/raise a ProblemDetail object (or raise
the appropriate Litestar HTTPException that produces an RFC 9457 payload) with
status, title, and detail populated (e.g., "Workflow definition not found" /
"Version conflict: the workflow was modified. Reload and retry.") and include
relevant instance/metadata as needed; apply the same replacement for the other
similar blocks referenced
(_fetch_existing_for_update()/_load_blueprint_or_error() and the code region
around lines 292-309).
| repo = state.app_state.persistence.workflow_definitions | ||
| await repo.save(definition) | ||
|
|
||
| version_repo = state.app_state.persistence.workflow_versions | ||
| snapshot = build_version_snapshot(definition, creator) | ||
| try: | ||
| await version_repo.save_version(snapshot) | ||
| except QueryError: | ||
| logger.exception( | ||
| WORKFLOW_VERSION_SNAPSHOT_FAILED, | ||
| definition_id=definition.id, | ||
| version=definition.version, | ||
| ) |
There was a problem hiding this comment.
Don’t report success after the snapshot write fails.
These paths persist the workflow definition first and then only log save_version() failures. That leaves live definitions without the matching version record, which breaks the “snapshot on every save” invariant and creates gaps for diff/rollback. Persist the definition and snapshot atomically, or fail the request when the snapshot insert cannot be committed.
Also applies to: 501-513, 546-571
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/synthorg/api/controllers/workflows.py` around lines 417 - 429, The
current flow saves the definition (repo.save) then attempts
version_repo.save_version and only logs on QueryError, leaving an inconsistent
state; fix by ensuring atomicity: either wrap both repo.save(definition) and
version_repo.save_version(snapshot) in a single persistence transaction (use
your persistence transaction API on state.app_state.persistence to
begin/commit/rollback so both operations succeed or both rollback), or if
transactions aren't available, on QueryError immediately delete/rollback the
saved definition (repo.delete(definition.id) or equivalent) and re-raise the
error so the request fails; apply the same pattern to the other occurrences that
call build_version_snapshot and version_repo.save_version (the blocks around
WORKFLOW_VERSION_SNAPSHOT_FAILED and the other referenced ranges).
- Monkeypatch _USER_BLUEPRINTS_DIR to tmp_path in blueprint API and loader tests to isolate from local user overrides - Add test_list_versions_ordering and test_list_versions_paginated with explicit status_code assertions - Add aria-pressed to BlueprintCard for screen reader state - Reconcile selected blueprint when filtered list changes - Add Hover and Empty dialog stories - Add loadBlueprints race guard token (same pattern as fetchWorkflows) - Extract upsertWorkflow helper for DRY create/createFromBlueprint - Use monotonic token increment in reset() instead of resetting to 0 - Fix import ordering in workflow-editor.ts
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
web/src/stores/workflow-editor.ts (1)
203-225:⚠️ Potential issue | 🟠 MajorApply the same version/diff invalidation on create that you do on load.
createDefinition()switches the active workflow, but unlikeloadDefinition()it leavesversions,versionsHasMore,diffResult, and both request ids untouched. That lets the newly created workflow inherit the previous workflow’s history/diff state, and any in-flightloadVersions()/loadDiff()response can still land after the create succeeds.♻️ Suggested fix
- set({ + set((prev) => ({ + ...prev, definition: def, nodes, edges: [], loading: false, dirty: false, selectedNodeId: null, undoStack: [], redoStack: [], yamlPreview: yaml, validationResult: null, - }) + versions: [], + versionsLoading: false, + versionsHasMore: false, + diffResult: null, + diffLoading: false, + _versionsRequestId: prev._versionsRequestId + 1, + _diffRequestId: prev._diffRequestId + 1, + }))Also applies to: 266-277
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/stores/workflow-editor.ts` around lines 203 - 225, createDefinition currently switches active workflow but fails to invalidate version/diff state like loadDefinition does; update createDefinition to reset versions=[], versionsLoading=false, versionsHasMore=false, diffResult=null, diffLoading=false and increment _versionsRequestId and _diffRequestId (and preserve setting loading=true and error=null) so any in-flight loadVersions()/loadDiff() responses are ignored; mirror the same set((prev) => ({ ...prev, loading: true, error: null, versions: [], versionsLoading: false, versionsHasMore: false, diffResult: null, diffLoading: false, _versionsRequestId: prev._versionsRequestId + 1, _diffRequestId: prev._diffRequestId + 1 })) logic used in loadDefinition inside the createDefinition function (and also apply the same change in the other similar create/update block referenced).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/unit/api/controllers/test_workflow_blueprints.py`:
- Around line 39-45: Assert the HTTP response status before calling resp.json()
to avoid noisy KeyError/shape errors; update the tests that call
test_client.get(...) and then body = resp.json() (variables resp, body, bp and
calls to make_auth_headers("ceo")) to first assert resp.status_code == 200 (or
the expected status) and only then call resp.json() and access body["data"][0];
apply the same change to the other occurrences noted (the blocks around lines
56-61, 91-97, 150-156) so every test checks resp.status_code before
dereferencing the JSON payload.
In `@web/src/components/ui/dialog.stories.tsx`:
- Around line 57-208: The stories duplicate open-state wiring and the Dialog
frame across DialogWithLoading, DialogWithError, DialogWithForm, DialogEmpty,
and DialogWithHover; extract a reusable shell component (e.g., DialogStoryShell)
that owns useState(initialOpen), the Button trigger, and the <Dialog open={open}
onOpenChange={setOpen}><DialogContent>...frame..., then accept props like title,
description, children (body) and optional actions; update each story helper to
render DialogStoryShell with the specific title/description and body content
(remove the duplicated state, Button, Dialog and DialogContent wrappers from
DialogWithLoading, DialogWithError, DialogWithForm, DialogEmpty, and
DialogWithHover) so all common wiring is centralized and only unique content
remains.
In `@web/src/pages/workflows/BlueprintPicker.tsx`:
- Around line 71-75: The effect on BlueprintPicker re-triggers infinitely when
loadBlueprints() throws because blueprints stays empty and loading
(blueprintsLoading) becomes false; modify the effect to avoid retrying on a
previous failure by checking an error flag or adding a local "hasLoadError"
guard: update the effect condition from "blueprints.length === 0 && !loading" to
also require "&& !blueprintsLoadError" (or add and check a component state like
"hasTriedAndFailed") and ensure loadBlueprints sets that error flag on catch;
reference useEffect, blueprints, loading (blueprintsLoading), loadBlueprints and
the new error/guard state when implementing the change so the effect won't
re-run immediately after a failed load.
In `@web/src/stores/workflow-editor.ts`:
- Around line 660-666: clearDiff currently increments _diffRequestId to
invalidate in-flight diff responses but doesn't reset diffLoading, so if the
canceled request set diffLoading:true the UI can remain stuck; update clearDiff
(and the similar rollback path around the code handling lines 682-691) to also
set diffLoading:false and clear diffResult (and ensure _diffRequestId is still
incremented) so any pending or canceled diff won't leave the loading flag set;
target the clearDiff function and the rollback/diff-cancel branch that
manipulates _diffRequestId, diffResult, and diffLoading.
---
Duplicate comments:
In `@web/src/stores/workflow-editor.ts`:
- Around line 203-225: createDefinition currently switches active workflow but
fails to invalidate version/diff state like loadDefinition does; update
createDefinition to reset versions=[], versionsLoading=false,
versionsHasMore=false, diffResult=null, diffLoading=false and increment
_versionsRequestId and _diffRequestId (and preserve setting loading=true and
error=null) so any in-flight loadVersions()/loadDiff() responses are ignored;
mirror the same set((prev) => ({ ...prev, loading: true, error: null, versions:
[], versionsLoading: false, versionsHasMore: false, diffResult: null,
diffLoading: false, _versionsRequestId: prev._versionsRequestId + 1,
_diffRequestId: prev._diffRequestId + 1 })) logic used in loadDefinition inside
the createDefinition function (and also apply the same change in the other
similar create/update block referenced).
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: cc03278c-fc5d-4eb8-b66a-98a243b6ee32
📒 Files selected for processing (7)
tests/unit/api/controllers/test_workflow_blueprints.pytests/unit/api/controllers/test_workflow_versions.pytests/unit/engine/workflow/test_blueprint_loader.pyweb/src/components/ui/dialog.stories.tsxweb/src/pages/workflows/BlueprintPicker.tsxweb/src/stores/workflow-editor.tsweb/src/stores/workflows.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Build Web
- GitHub Check: Build Backend
- GitHub Check: Build Sandbox
- GitHub Check: Dashboard Test
- GitHub Check: Test (Python 3.14)
- GitHub Check: Dependency Review
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (9)
web/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{ts,tsx,js,jsx}: Always usecreateLoggerfrom@/lib/loggerinstead of bareconsole.warn,console.error, orconsole.debugin application code
Use variable namelogfor logger instances (e.g.,const log = createLogger('module-name'))
Uselog.debug()for dev-only logging (stripped in production),log.warn()for warnings, andlog.error()for errors
Pass dynamic/untrusted values as separate arguments to logger calls (not interpolated into the message string) so they go throughsanitizeArg
Wrap attacker-controlled fields inside structured objects withsanitizeForLog()before embedding in logs
Files:
web/src/stores/workflow-editor.tsweb/src/stores/workflows.tsweb/src/components/ui/dialog.stories.tsxweb/src/pages/workflows/BlueprintPicker.tsx
web/src/**/*.{ts,tsx,css}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{ts,tsx,css}: Use Tailwind semantic color classes (text-foreground,bg-card,text-accent,text-success,bg-danger, etc.) or CSS variables (var(--so-accent)) instead of hardcoded hex values
Usefont-sansorfont-monofor typography (maps to Geist tokens), never setfontFamilydirectly
Use density-aware spacing tokens (p-card,gap-section-gap,gap-grid-gap) or standard Tailwind spacing, never hardcode pixel values for layout spacing
Use token variables (var(--so-shadow-card-hover),border-border,border-bright) for shadows and borders instead of hardcoded values
Do NOT usergba()with hardcoded values -- use design token variables
A PostToolUse hook (scripts/check_web_design_system.py) runs automatically on every Edit/Write toweb/src/files and catches design system violations; fix all violations before proceeding
Files:
web/src/stores/workflow-editor.tsweb/src/stores/workflows.tsweb/src/components/ui/dialog.stories.tsxweb/src/pages/workflows/BlueprintPicker.tsx
web/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.{ts,tsx}: Do NOT hardcode Framer Motion transition durations -- use presets from@/lib/motion
CSS side-effect imports in TypeScript files should use type declarations (/// <reference types="vite/client" />covers this); TypeScript 6 defaultsnoUncheckedSideEffectImportsto true
web/src/**/*.{ts,tsx}: ALWAYS reuse existing components fromweb/src/components/ui/before creating new ones
NEVER hardcode hex colors, font-family, pixel spacing, or Framer Motion transitions -- use design tokens and@/lib/motionpresets
Use TypeScript 6.0+ strict type checking; post-training references TS6, Storybook 10
React 19 dashboard using design tokens,@/lib/motionpresets for animations, shadcn/ui components with Radix UI, Tailwind CSS 4, Zustand state management,@tanstack/react-querydata fetching,@xyflow/react+@dagrejs/dagrefor workflows, d3-force for force-directed layouts,@dnd-kitfor drag-drop, Recharts for charting
Files:
web/src/stores/workflow-editor.tsweb/src/stores/workflows.tsweb/src/components/ui/dialog.stories.tsxweb/src/pages/workflows/BlueprintPicker.tsx
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations-- Python 3.14 has PEP 649
Useexcept A, B:(no parentheses) syntax for exception handling -- PEP 758 except syntax, ruff enforces on Python 3.14
All public functions and classes must have type hints and Google-style docstrings, enforced by ruff D rules and mypy strict mode
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples -- use generic names likeexample-provider,example-large-001,example-medium-001,example-small-001,large/medium/smallaliases
Nocdin Bash commands -- working directory is already set to project root; use absolute paths or run commands directly; exception:bash -c "cd <dir> && <cmd>"is safe for tools without-Cflag
Never use Bash to write or modify files -- use Write/Edit tools instead; nocat >,cat << EOF,echo >,echo >>,sed -i,python -c "open(...).write(...)", ortee; read-only inspection like piping to stdout is fine
Files:
tests/unit/api/controllers/test_workflow_blueprints.pytests/unit/engine/workflow/test_blueprint_loader.pytests/unit/api/controllers/test_workflow_versions.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use generic test provider names liketest-provider,test-small-001instead of vendor-specific names like Claude, GPT
Mark tests with pytest markers:@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slow
Always include-n 8when running pytest locally for parallelism via pytest-xdist; CI uses-n auto; never run tests sequentially
Never skip, dismiss, or ignore flaky tests -- fix them fully and fundamentally; for timing-sensitive tests, mocktime.monotonic()andasyncio.sleep()instead of widening margins; for indefinite blocking tasks, useasyncio.Event().wait()instead ofasyncio.sleep(large_number)
Use property-based testing via Hypothesis:@given+@settings, controlled viaHYPOTHESIS_PROFILEenv var (ci: 10 examples deterministic, dev: 1000 examples, fuzz: 10,000 no deadline, extreme: 500,000 no deadline)
When Hypothesis finds a failing example, it is a real bug -- do not just rerun and move on; read the failing example, fix the underlying bug, and add an explicit@example(...)decorator so the case is permanently covered in CI
Files:
tests/unit/api/controllers/test_workflow_blueprints.pytests/unit/engine/workflow/test_blueprint_loader.pytests/unit/api/controllers/test_workflow_versions.py
⚙️ CodeRabbit configuration file
Test files do not require Google-style docstrings on classes or functions -- ruff D rules are only enforced on src/. A bare
@settings() decorator with no arguments on Hypothesis property tests is a no-op and should not be suggested -- the HYPOTHESIS_PROFILE env var controls example counts via registered profiles, which@given() honors automatically.
Files:
tests/unit/api/controllers/test_workflow_blueprints.pytests/unit/engine/workflow/test_blueprint_loader.pytests/unit/api/controllers/test_workflow_versions.py
web/src/components/ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/components/ui/**/*.{ts,tsx}: Place new shared components inweb/src/components/ui/with descriptive kebab-case filename
Export props as a TypeScript interface in new components
Use design tokens exclusively in component code -- no hardcoded colors, fonts, or spacing
Importcnfrom@/lib/utilsfor conditional class merging in component files
Files:
web/src/components/ui/dialog.stories.tsx
web/src/components/ui/**/*.stories.tsx
📄 CodeRabbit inference engine (web/CLAUDE.md)
Create a
.stories.tsxfile alongside new components with all states (default, hover, loading, error, empty)
Files:
web/src/components/ui/dialog.stories.tsx
web/src/**/*.stories.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/**/*.stories.{ts,tsx}: Usestorybook/testinstead of@storybook/testandstorybook/actionsinstead of@storybook/addon-actions(Storybook 10 import path changes)
Useparameters.backgrounds.options(object keyed by name) +initialGlobals.backgrounds.valuein Storybook (Storybook 10 Backgrounds API change)
Useparameters.a11y.test: 'error' | 'todo' | 'off'in Storybook stories for a11y testing (Storybook 10 change)
Files:
web/src/components/ui/dialog.stories.tsx
web/src/pages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/CLAUDE.md)
web/src/pages/**/*.{ts,tsx}: Always reuse existing components fromweb/src/components/ui/before creating new ones
UseStatusBadgecomponent for agent/task/system status indicators (colored dot + optional built-in label toggle)
UseMetricCardcomponent for numeric KPIs with sparkline, change badge, progress bar
UseSparklinecomponent for inline SVG trend lines withcolor?andanimated?props (used inside MetricCard or standalone)
UseSectionCardcomponent for titled card wrapper with icon and action slot
UseAgentCardcomponent for agent display (avatar, name, role, status, current task)
UseDeptHealthBarcomponent for department utilization (animated fill bar +health?+agentCount)
UseProgressGaugecomponent for circular or linear gauge for budget/utilization (variant?defaults to'circular',max?defaults to 100)
UseStatPillcomponent for compact inline label + value pair
UseAvatarcomponent for circular initials avatar with optionalborderColor?prop
UseToast/ToastContainercomponent for success/error/warning/info notifications with auto-dismiss queue (mountToastContaineronce in AppLayout)
UseSkeleton/SkeletonCard/SkeletonMetric/SkeletonTable/SkeletonTextcomponents for loading placeholders matching component shapes (shimmer animation, respectsprefers-reduced-motion)
UseEmptyStatecomponent for no-data / no-results placeholder with icon, title, description, optional action button
UseErrorBoundarycomponent for React error boundary with retry --levelprop:page/section/component
UseConfirmDialogcomponent for confirmation modal (Radix AlertDialog) withdefault/destructivevariants andloadingstate
UseCommandPalettecomponent for global Cmd+K search (cmdk + React Router) -- mount once in AppLayout, register commands viauseCommandPalettehook
UseInlineEditcomponent for click-to-edit text with Enter/Escape, validation, optimistic save with rollback
Use...
Files:
web/src/pages/workflows/BlueprintPicker.tsx
🧠 Learnings (44)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-04T19:13:28.005Z
Learning: Applies to src/synthorg/engine/workflow/**/*.py : Workflow execution supports Kanban board, Agile sprints, WIP limits, sprint lifecycle, velocity tracking; use pluggable scheduling strategies and velocity calculators
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/**/*.{ts,tsx,js,jsx} : Use variable name `log` for logger instances (e.g., `const log = createLogger('module-name')`)
Applied to files:
web/src/stores/workflow-editor.ts
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/**/*.{ts,tsx,js,jsx} : Always use `createLogger` from `@/lib/logger` instead of bare `console.warn`, `console.error`, or `console.debug` in application code
Applied to files:
web/src/stores/workflow-editor.ts
📚 Learning: 2026-04-04T19:13:28.005Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-04T19:13:28.005Z
Learning: Applies to web/src/**/*.{ts,tsx} : React 19 dashboard using design tokens, `@/lib/motion` presets for animations, shadcn/ui components with Radix UI, Tailwind CSS 4, Zustand state management, tanstack/react-query data fetching, xyflow/react + dagrejs/dagre for workflows, d3-force for force-directed layouts, dnd-kit for drag-drop, Recharts for charting
Applied to files:
web/src/stores/workflow-editor.tsweb/src/stores/workflows.tsweb/src/pages/workflows/BlueprintPicker.tsx
📚 Learning: 2026-03-27T12:44:29.466Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-27T12:44:29.466Z
Learning: Applies to web/src/stores/**/*.{ts,tsx} : Use Zustand stores in web dashboard for state management (auth, WebSocket, toast, analytics, domain shells)
Applied to files:
web/src/stores/workflows.ts
📚 Learning: 2026-03-30T10:41:40.176Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:41:40.176Z
Learning: Applies to web/src/**/*.{ts,tsx} : Use Zustand stores for state management in the web dashboard; each domain has its own store module (auth, WebSocket, toast, analytics, setup, company, agents, budget, tasks, settings, providers, theme, per-domain stores)
Applied to files:
web/src/stores/workflows.ts
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/components/ui/**/*.stories.tsx : Create a `.stories.tsx` file alongside new components with all states (default, hover, loading, error, empty)
Applied to files:
web/src/stores/workflows.tsweb/src/components/ui/dialog.stories.tsxweb/src/pages/workflows/BlueprintPicker.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/**/*.{ts,tsx} : CSS side-effect imports in TypeScript files should use type declarations (`/// <reference types="vite/client" />` covers this); TypeScript 6 defaults `noUncheckedSideEffectImports` to true
Applied to files:
web/src/stores/workflows.ts
📚 Learning: 2026-04-04T19:13:28.005Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-04T19:13:28.005Z
Learning: Applies to src/synthorg/engine/workspace/**/*.py : Use git worktree isolation for task execution; merge orchestration handles semantic conflict detection
Applied to files:
tests/unit/api/controllers/test_workflow_blueprints.py
📚 Learning: 2026-03-31T21:07:37.470Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T21:07:37.470Z
Learning: Applies to tests/**/*.py : NEVER skip, dismiss, or ignore flaky tests — always fix them fully and fundamentally; mock `time.monotonic()` and `asyncio.sleep()` for determinism; use `asyncio.Event().wait()` for indefinite blocking instead of `asyncio.sleep(large_number)`
Applied to files:
tests/unit/api/controllers/test_workflow_blueprints.py
📚 Learning: 2026-04-02T07:18:02.381Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-02T07:18:02.381Z
Learning: Applies to tests/**/*.py : Never skip, dismiss, or ignore flaky tests; fix them fully and fundamentally by mocking `time.monotonic()` and `asyncio.sleep()` for determinism or using `asyncio.Event().wait()` for indefinite blocking
Applied to files:
tests/unit/api/controllers/test_workflow_blueprints.py
📚 Learning: 2026-04-01T15:36:39.993Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T15:36:39.993Z
Learning: Applies to {src/synthorg/**/*.py,tests/**/*.py,web/src/**/*.{ts,tsx}} : NEVER use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples — use generic names: `example-provider`, `example-large-001`, `example-medium-001`, `example-small-001`, `large`/`medium`/`small`
Applied to files:
tests/unit/api/controllers/test_workflow_blueprints.py
📚 Learning: 2026-03-20T21:44:04.528Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-20T21:44:04.528Z
Learning: Applies to tests/**/*.py : Fix flaky tests completely and fundamentally; for timing-sensitive tests, mock `time.monotonic()` and `asyncio.sleep()` to make them deterministic instead of widening timing margins
Applied to files:
tests/unit/api/controllers/test_workflow_blueprints.py
📚 Learning: 2026-03-16T07:22:28.134Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T07:22:28.134Z
Learning: Applies to tests/**/*.py : NEVER skip, dismiss, or ignore flaky tests — always fix them fully and fundamentally. For timing-sensitive tests, mock `time.monotonic()` and `asyncio.sleep()` to make them deterministic instead of widening timing margins
Applied to files:
tests/unit/api/controllers/test_workflow_blueprints.py
📚 Learning: 2026-03-31T14:28:28.895Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-31T14:28:28.895Z
Learning: Applies to src/synthorg/**/*.py : Maintain 80% minimum test coverage (enforced in CI)
Applied to files:
tests/unit/api/controllers/test_workflow_blueprints.py
📚 Learning: 2026-03-30T10:41:40.176Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:41:40.176Z
Learning: Applies to web/src/components/ui/**/*.{ts,tsx} : Create new shared components in `web/src/components/ui/` with `.stories.tsx` Storybook file covering all states (default, hover, loading, error, empty)
Applied to files:
web/src/components/ui/dialog.stories.tsxweb/src/pages/workflows/BlueprintPicker.tsx
📚 Learning: 2026-03-30T10:41:40.176Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:41:40.176Z
Learning: Applies to web/src/**/*.stories.tsx : Storybook 10: import from `storybook/test` (not `storybook/test`), `storybook/actions` (not `storybook/addon-actions`)
Applied to files:
web/src/components/ui/dialog.stories.tsx
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/**/*.stories.tsx : Storybook 10: Import from `storybook/test` instead of `storybook/test`
Applied to files:
web/src/components/ui/dialog.stories.tsx
📚 Learning: 2026-03-27T22:32:26.927Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-27T22:32:26.927Z
Learning: Applies to web/src/components/ui/*.{tsx,ts} : For new shared React components: place in web/src/components/ui/ with kebab-case filename, create .stories.tsx with all states, export props as TypeScript interface, use design tokens exclusively
Applied to files:
web/src/components/ui/dialog.stories.tsxweb/src/pages/workflows/BlueprintPicker.tsx
📚 Learning: 2026-03-30T10:20:08.544Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:20:08.544Z
Learning: Applies to web/**/*.stories.{ts,tsx} : Storybook 10: Use storybook/test (not storybook/test) and storybook/actions (not storybook/addon-actions) import paths
Applied to files:
web/src/components/ui/dialog.stories.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/**/*.stories.{ts,tsx} : Use `storybook/test` instead of `storybook/test` and `storybook/actions` instead of `storybook/addon-actions` (Storybook 10 import path changes)
Applied to files:
web/src/components/ui/dialog.stories.tsx
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/**/*.stories.tsx : Storybook 10: Use `parameters.a11y.test: 'error' | 'todo' | 'off'` for a11y testing configuration (replaces old `.element` and `.manual`)
Applied to files:
web/src/components/ui/dialog.stories.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/**/*.stories.{ts,tsx} : Use `parameters.a11y.test: 'error' | 'todo' | 'off'` in Storybook stories for a11y testing (Storybook 10 change)
Applied to files:
web/src/components/ui/dialog.stories.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/**/*.stories.{ts,tsx} : Use `parameters.backgrounds.options` (object keyed by name) + `initialGlobals.backgrounds.value` in Storybook (Storybook 10 Backgrounds API change)
Applied to files:
web/src/components/ui/dialog.stories.tsx
📚 Learning: 2026-03-30T10:20:08.544Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:20:08.544Z
Learning: Applies to web/**/*.stories.{ts,tsx} : Storybook 10: Use parameters.a11y.test: 'error' | 'todo' | 'off' for a11y testing (replaces old .element and .manual); set globally in preview.tsx to enforce WCAG compliance on all stories
Applied to files:
web/src/components/ui/dialog.stories.tsx
📚 Learning: 2026-03-30T10:20:08.544Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:20:08.544Z
Learning: Applies to web/src/components/ui/**/*.{ts,tsx} : When creating new shared web components, place in web/src/components/ui/ with kebab-case filename, create .stories.tsx alongside with all states (default, hover, loading, error, empty), export props as TypeScript interface, use design tokens exclusively with no hardcoded colors/fonts/spacing, and import cn from `@/lib/utils` for conditional class merging
Applied to files:
web/src/components/ui/dialog.stories.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `EmptyState` component for no-data / no-results placeholder with icon, title, description, optional action button
Applied to files:
web/src/components/ui/dialog.stories.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/.storybook/main.ts : Use `defineMain` from `storybook/react-vite/node` and `definePreview` from `storybook/react-vite` with type-safe config in Storybook configuration (Storybook 10)
Applied to files:
web/src/components/ui/dialog.stories.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `ConfirmDialog` component for confirmation modal (Radix AlertDialog) with `default` / `destructive` variants and `loading` state
Applied to files:
web/src/components/ui/dialog.stories.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/components/ui/**/*.{ts,tsx} : Export props as a TypeScript interface in new components
Applied to files:
web/src/pages/workflows/BlueprintPicker.tsx
📚 Learning: 2026-03-30T10:20:08.544Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:20:08.544Z
Learning: Applies to web/src/**/*.{ts,tsx} : Always reuse existing components from web/src/components/ui/ (StatusBadge, MetricCard, Sparkline, SectionCard, AgentCard, DeptHealthBar, ProgressGauge, StatPill, Avatar, Button, Toast/ToastContainer, Skeleton variants, EmptyState, ErrorBoundary, ConfirmDialog, CommandPalette, InlineEdit, AnimatedPresence, StaggerGroup/StaggerItem, Drawer, form fields, TaskStatusIndicator, PriorityBadge, ProviderHealthBadge, TokenUsageBar, CodeMirrorEditor, SegmentedControl, ThemeToggle, LiveRegion, MobileUnsupportedOverlay, LazyCodeMirrorEditor) before creating new components
Applied to files:
web/src/pages/workflows/BlueprintPicker.tsx
📚 Learning: 2026-03-27T12:44:29.466Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-27T12:44:29.466Z
Learning: Applies to web/src/**/*.{ts,tsx} : Always reuse existing components from `web/src/components/ui/` (StatusBadge, MetricCard, Sparkline, SectionCard, AgentCard, DeptHealthBar, ProgressGauge, StatPill, Avatar, Button, Toast, Skeleton, EmptyState, ErrorBoundary, ConfirmDialog, CommandPalette, InlineEdit, AnimatedPresence, StaggerGroup/StaggerItem) before creating new ones
Applied to files:
web/src/pages/workflows/BlueprintPicker.tsx
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/components/ui/**/*.tsx : Export props as a TypeScript interface for new components
Applied to files:
web/src/pages/workflows/BlueprintPicker.tsx
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/**/*.{tsx,ts} : ALWAYS reuse existing components from `web/src/components/ui/` before creating new ones (StatusBadge, MetricCard, Sparkline, SectionCard, AgentCard, DeptHealthBar, ProgressGauge, StatPill, Avatar, Button, Toast, Skeleton, EmptyState, ErrorBoundary, ConfirmDialog, CommandPalette, InlineEdit, AnimatedPresence, StaggerGroup, Drawer, InputField, SelectField, SliderField, ToggleField, TaskStatusIndicator, PriorityBadge, ProviderHealthBadge, TokenUsageBar, CodeMirrorEditor, SegmentedControl, ThemeToggle, LiveRegion, MobileUnsupportedOverlay, LazyCodeMirrorEditor, TagInput, MetadataGrid, ProjectStatusBadge, ContentTypeBadge)
Applied to files:
web/src/pages/workflows/BlueprintPicker.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `AgentCard` component for agent display (avatar, name, role, status, current task)
Applied to files:
web/src/pages/workflows/BlueprintPicker.tsx
📚 Learning: 2026-04-02T12:21:16.739Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-02T12:21:16.739Z
Learning: Applies to web/src/**/*.{tsx,ts} : Do NOT create complex (>8 line) JSX inside `.map()` -- extract to a shared component
Applied to files:
web/src/pages/workflows/BlueprintPicker.tsx
📚 Learning: 2026-03-30T10:20:08.544Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:20:08.544Z
Learning: Applies to web/src/**/*.{ts,tsx} : Do NOT create complex (>8 line) JSX inside .map()—extract to a shared component
Applied to files:
web/src/pages/workflows/BlueprintPicker.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Do NOT create complex (>8 line) JSX inside `.map()` -- extract to a shared component
Applied to files:
web/src/pages/workflows/BlueprintPicker.tsx
📚 Learning: 2026-03-30T10:41:40.176Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:41:40.176Z
Learning: Applies to web/src/**/*.{ts,tsx} : Do NOT create complex (>8 line) JSX inside `.map()`; extract to a shared component
Applied to files:
web/src/pages/workflows/BlueprintPicker.tsx
📚 Learning: 2026-03-30T10:41:40.176Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-30T10:41:40.176Z
Learning: Applies to web/src/**/*.{ts,tsx} : ALWAYS reuse existing components from `web/src/components/ui/` before creating new ones; refer to design system inventory (StatusBadge, MetricCard, Sparkline, SectionCard, AgentCard, etc.)
Applied to files:
web/src/pages/workflows/BlueprintPicker.tsx
📚 Learning: 2026-04-01T20:43:51.878Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-01T20:43:51.878Z
Learning: Applies to web/src/**/*.{ts,tsx} : Always reuse existing components from `web/src/components/ui/` before creating new ones. Never hardcode hex colors, font-family, pixel spacing, or Framer Motion transitions -- use design tokens and `@/lib/motion` presets.
Applied to files:
web/src/pages/workflows/BlueprintPicker.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Do NOT build card-with-header layouts from scratch -- use `<SectionCard>` component
Applied to files:
web/src/pages/workflows/BlueprintPicker.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `ToggleField` component for labeled toggle switch (role="switch") with optional description text
Applied to files:
web/src/pages/workflows/BlueprintPicker.tsx
📚 Learning: 2026-04-04T17:43:42.372Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: web/CLAUDE.md:0-0
Timestamp: 2026-04-04T17:43:42.372Z
Learning: Applies to web/src/pages/**/*.{ts,tsx} : Use `SegmentedControl` component for accessible radiogroup with keyboard navigation, size variants (`sm`/`md`), generic `<T extends string>` typing
Applied to files:
web/src/pages/workflows/BlueprintPicker.tsx
🔇 Additional comments (14)
web/src/components/ui/dialog.stories.tsx (1)
38-44: A11y story configuration looks correct.Good use of
parameters.a11y.test = 'error'at the story meta level.As per coding guidelines: "Use
parameters.a11y.test: 'error' | 'todo' | 'off'in Storybook stories for a11y testing (Storybook 10 change).tests/unit/api/controllers/test_workflow_versions.py (2)
131-177: Strong pagination + ordering coverage for version history.Good addition: this directly validates newest-first ordering and page-window behavior (
limit/offset) for/versions, including total count.
257-285: Rollback success path is now exercised end-to-end.Great fix: this test now actually hits
POST /rollbackand verifies the new snapshot is created at the head of history.tests/unit/api/controllers/test_workflow_blueprints.py (1)
14-18: Nice test isolation for user blueprint overrides.This autouse fixture makes the endpoint tests hermetic and avoids workstation-specific failures from
~/.synthorg/workflow-blueprints.tests/unit/engine/workflow/test_blueprint_loader.py (1)
23-27: Great deterministic coverage for blueprint loading and schema integrity.The user-dir isolation plus structural assertions across all built-ins provides strong protection against regressions in loader behavior and YAML content.
Also applies to: 199-231
web/src/stores/workflows.ts (4)
4-17: LGTM!Import organization is correct: type-only imports are properly grouped with other imports before executable code. The
sanitizeForLogimport enables proper error sanitization for logging.
53-71: LGTM!The blueprint request token follows the established pattern for stale request guards. The
upsertWorkflowhelper cleanly extracts shared workflow insertion logic, correctly handling deduplication and conditional count increment.
86-98: LGTM!The
loadBlueprintsaction properly implements the race-guard pattern using request tokens to ignore stale responses. Error handling correctly sanitizes the error before logging and extracts a user-friendly message for state.
118-128: LGTM!Both workflow creation actions cleanly delegate to the
upsertWorkflowhelper, eliminating code duplication and ensuring consistent store update behavior.web/src/pages/workflows/BlueprintPicker.tsx (5)
12-22: LGTM!Props interfaces are properly exported, enabling type-safe consumption by parent components.
24-59: LGTM!
BlueprintCardis well-implemented with proper accessibility (aria-pressed), design token usage throughout, and reuse of theStatPillcomponent. The toggle behavior correctly deselects when clicking the already-selected card.
81-90: LGTM!The selection reconciliation effect correctly clears stale selections when the filter changes, while the
filtered.length > 0guard prevents premature clearing during loading states.
92-111: LGTM!Loading and error states are well-implemented: skeleton placeholders provide visual continuity during loading, and the error alert uses semantic color tokens with proper
role="alert"for accessibility.
113-129: LGTM!The main render correctly applies staggered entrance animations using motion presets and maps the filtered blueprints to extracted
BlueprintCardcomponents.
| function DialogWithLoading({ open: initialOpen = true }: { open?: boolean }) { | ||
| const [open, setOpen] = useState(initialOpen) | ||
| return ( | ||
| <> | ||
| <Button onClick={() => setOpen(true)}>Open Dialog</Button> | ||
| <Dialog open={open} onOpenChange={setOpen}> | ||
| <DialogContent> | ||
| <DialogHeader> | ||
| <div> | ||
| <DialogTitle>Loading</DialogTitle> | ||
| <DialogDescription>Please wait while data is being loaded.</DialogDescription> | ||
| </div> | ||
| <DialogCloseButton /> | ||
| </DialogHeader> | ||
| <div className="flex items-center justify-center px-6 py-8"> | ||
| <div className="size-6 animate-spin rounded-full border-2 border-accent border-t-transparent" /> | ||
| </div> | ||
| </DialogContent> | ||
| </Dialog> | ||
| </> | ||
| ) | ||
| } | ||
|
|
||
| export const Loading: Story = { | ||
| render: () => <DialogWithLoading />, | ||
| } | ||
|
|
||
| function DialogWithError({ open: initialOpen = true }: { open?: boolean }) { | ||
| const [open, setOpen] = useState(initialOpen) | ||
| return ( | ||
| <> | ||
| <Button onClick={() => setOpen(true)}>Open Dialog</Button> | ||
| <Dialog open={open} onOpenChange={setOpen}> | ||
| <DialogContent> | ||
| <DialogHeader> | ||
| <div> | ||
| <DialogTitle>Error</DialogTitle> | ||
| <DialogDescription>Something went wrong.</DialogDescription> | ||
| </div> | ||
| <DialogCloseButton /> | ||
| </DialogHeader> | ||
| <div className="px-6 py-4"> | ||
| <div className="rounded-md border border-danger/30 bg-danger/5 p-card text-sm text-danger"> | ||
| Failed to load data. Please try again. | ||
| </div> | ||
| </div> | ||
| </DialogContent> | ||
| </Dialog> | ||
| </> | ||
| ) | ||
| } | ||
|
|
||
| export const Error: Story = { | ||
| render: () => <DialogWithError />, | ||
| } | ||
|
|
||
| function DialogWithForm({ open: initialOpen = true }: { open?: boolean }) { | ||
| const [open, setOpen] = useState(initialOpen) | ||
| return ( | ||
| <> | ||
| <Button onClick={() => setOpen(true)}>Open Dialog</Button> | ||
| <Dialog open={open} onOpenChange={setOpen}> | ||
| <DialogContent> | ||
| <DialogHeader> | ||
| <div> | ||
| <DialogTitle>Edit Name</DialogTitle> | ||
| <DialogDescription>Enter a new name for this item.</DialogDescription> | ||
| </div> | ||
| <DialogCloseButton /> | ||
| </DialogHeader> | ||
| <div className="flex flex-col gap-4 px-6 py-4"> | ||
| <input | ||
| type="text" | ||
| className="rounded-md border border-border bg-background px-3 py-2 text-sm text-foreground" | ||
| placeholder="New name" | ||
| defaultValue="My Workflow" | ||
| /> | ||
| <div className="flex justify-end gap-2"> | ||
| <Button variant="outline" onClick={() => setOpen(false)}>Cancel</Button> | ||
| <Button onClick={() => setOpen(false)}>Save</Button> | ||
| </div> | ||
| </div> | ||
| </DialogContent> | ||
| </Dialog> | ||
| </> | ||
| ) | ||
| } | ||
|
|
||
| export const WithForm: Story = { | ||
| render: () => <DialogWithForm />, | ||
| } | ||
|
|
||
| function DialogEmpty({ open: initialOpen = true }: { open?: boolean }) { | ||
| const [open, setOpen] = useState(initialOpen) | ||
| return ( | ||
| <> | ||
| <Button onClick={() => setOpen(true)}>Open Dialog</Button> | ||
| <Dialog open={open} onOpenChange={setOpen}> | ||
| <DialogContent> | ||
| <DialogHeader> | ||
| <div> | ||
| <DialogTitle>No Items</DialogTitle> | ||
| <DialogDescription>There is nothing to display.</DialogDescription> | ||
| </div> | ||
| <DialogCloseButton /> | ||
| </DialogHeader> | ||
| <div className="flex flex-col items-center justify-center gap-2 px-6 py-8 text-center"> | ||
| <p className="text-sm text-muted">No results found.</p> | ||
| </div> | ||
| </DialogContent> | ||
| </Dialog> | ||
| </> | ||
| ) | ||
| } | ||
|
|
||
| export const Empty: Story = { | ||
| render: () => <DialogEmpty />, | ||
| } | ||
|
|
||
| function DialogWithHover({ open: initialOpen = true }: { open?: boolean }) { | ||
| const [open, setOpen] = useState(initialOpen) | ||
| return ( | ||
| <> | ||
| <Button onClick={() => setOpen(true)}>Open Dialog</Button> | ||
| <Dialog open={open} onOpenChange={setOpen}> | ||
| <DialogContent> | ||
| <DialogHeader> | ||
| <div> | ||
| <DialogTitle>Hover Example</DialogTitle> | ||
| <DialogDescription>Hover over the items below.</DialogDescription> | ||
| </div> | ||
| <DialogCloseButton /> | ||
| </DialogHeader> | ||
| <div className="flex flex-col gap-2 px-6 py-4"> | ||
| {['Item A', 'Item B', 'Item C'].map((item) => ( | ||
| <div | ||
| key={item} | ||
| className="rounded-md border border-border bg-card p-card text-sm text-foreground transition-colors hover:border-accent/50 hover:bg-card-hover" | ||
| > | ||
| {item} | ||
| </div> | ||
| ))} | ||
| </div> | ||
| </DialogContent> | ||
| </Dialog> | ||
| </> | ||
| ) | ||
| } | ||
|
|
||
| export const Hover: Story = { | ||
| render: () => <DialogWithHover />, | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Reduce duplicated dialog-story shell logic.
All variant helpers repeat the same open-state wiring and dialog frame. Extract a shared story shell to reduce drift and simplify future updates.
♻️ Proposed refactor (DRY story shell)
+type DialogStoryShellProps = {
+ open?: boolean
+ title: string
+ description: string
+ body: React.ReactNode
+}
+
+function DialogStoryShell({
+ open: initialOpen = true,
+ title,
+ description,
+ body,
+}: DialogStoryShellProps) {
+ const [open, setOpen] = useState(initialOpen)
+ return (
+ <>
+ <Button onClick={() => setOpen(true)}>Open Dialog</Button>
+ <Dialog open={open} onOpenChange={setOpen}>
+ <DialogContent>
+ <DialogHeader>
+ <div>
+ <DialogTitle>{title}</DialogTitle>
+ <DialogDescription>{description}</DialogDescription>
+ </div>
+ <DialogCloseButton />
+ </DialogHeader>
+ {body}
+ </DialogContent>
+ </Dialog>
+ </>
+ )
+}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/components/ui/dialog.stories.tsx` around lines 57 - 208, The stories
duplicate open-state wiring and the Dialog frame across DialogWithLoading,
DialogWithError, DialogWithForm, DialogEmpty, and DialogWithHover; extract a
reusable shell component (e.g., DialogStoryShell) that owns
useState(initialOpen), the Button trigger, and the <Dialog open={open}
onOpenChange={setOpen}><DialogContent>...frame..., then accept props like title,
description, children (body) and optional actions; update each story helper to
render DialogStoryShell with the specific title/description and body content
(remove the duplicated state, Button, Dialog and DialogContent wrappers from
DialogWithLoading, DialogWithError, DialogWithForm, DialogEmpty, and
DialogWithHover) so all common wiring is centralized and only unique content
remains.
- Add assert resp.status_code before resp.json() in 4 blueprint tests - Prevent infinite useEffect loop in BlueprintPicker when load fails (check !error before triggering loadBlueprints) - Reset diffLoading in clearDiff to avoid stuck loading state - Invalidate versions/diff state in createDefinition (mirror loadDefinition)
🤖 I have created a release *beep* *boop* --- ## [0.6.2](v0.6.1...v0.6.2) (2026-04-06) ### Features * add issue analyzer script for priority/scope management ([#1084](#1084)) ([1ccba27](1ccba27)) * config fixes and deferred improvements from PR [#1058](#1058) review ([#1067](#1067)) ([2cac2d3](2cac2d3)), closes [#1061](#1061) [#1060](#1060) * cumulative risk-unit action budgets ([#806](#806)) and automated reporting ([#245](#245)) ([#1063](#1063)) ([4689816](4689816)) * fine-tuning pipeline + CompositeBackend + workflow lifecycle ([#1065](#1065)) ([85b05bc](85b05bc)), closes [#1001](#1001) [#850](#850) [#1058](#1058) * memory consolidation upgrades (LLM Merge, Search-and-Ask, diversity penalty, distillation capture) ([#1071](#1071)) ([174e2be](174e2be)), closes [#704](#704) * migrate web dashboard from Radix UI to Base UI, activate CSP nonce, rebuild org chart page, and fix agent routing ([#1083](#1083)) ([ebc6921](ebc6921)) * v0.7.0 engine foundations -- structured failure diagnosis + auditable decisions ([#1072](#1072)) ([d341d37](d341d37)) * workflow templates and versioning with diff and rollback ([#1069](#1069)) ([7af94de](7af94de)), closes [#1006](#1006) [#1008](#1008) ### Documentation * unify REST API docs under /docs/openapi/ and patch sitemap ([#1073](#1073)) ([af19382](af19382)) ### Maintenance * bump hypothesis from 6.151.10 to 6.151.11 in the all group ([#1086](#1086)) ([3176318](3176318)) * bump nginxinc/nginx-unprivileged from `f99cc61` to `601c823` in /docker/web in the all group ([#1085](#1085)) ([5eb99ac](5eb99ac)) * bump the all group in /web with 3 updates ([#1087](#1087)) ([8deae44](8deae44)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary
Implements #1006 (workflow templates/starter blueprints) and #1008 (workflow versioning with diff view and rollback).
Workflow Templates (#1006)
~/.synthorg/workflow-blueprints/)GET /workflows/blueprints-- list available blueprints with metadataPOST /workflows/from-blueprint-- instantiate a blueprint as a new workflow definitionWorkflow Versioning (#1008)
GET /workflows/{id}/versions-- paginated version historyGET /workflows/{id}/versions/{num}-- get specific version snapshotGET /workflows/{id}/diff?from_version=X&to_version=Y-- structural diff computationPOST /workflows/{id}/rollback-- restore a previous version (creates new version, no history lost)workflow_definition_versionstablePre-PR Review
Pre-reviewed by 18 agents, 39 findings addressed:
Critical fixes:
create_from_blueprint(v1 was never recorded)logger.warning()on 7 missing error return paths (get_version 404, get_diff 400+404x2, rollback 404x2+409)delete_workflowobservability/events/workflow_version.py(was inline strings with wrong event for delete/count errors)Major fixes:
WorkflowVersionControllerto keep files under 800 linesBlueprintInfoResponse.display_name->NotBlankStr,workflow_type->WorkflowType,source->Literal["builtin", "user"]model_validatortoRollbackWorkflowRequest(target_version < expected_version)model_validatortoNodeChange/EdgeChange/WorkflowDiff(invariant enforcement)ge=1constraint onversion_numpath parameterFakeWorkflowVersionRepositoryto use concrete types instead ofAnylabel_changedtest, fixedtest_rollback_successcoveragegap-section-gap)loadVersions/loadDiffstore actionsnodes,edges,created_by)saved_atviaAwareDatetimeCHECKconstraint onworkflow_typein version table schemaTest Plan
Closes #1006
Closes #1008