[fix] Resolve broken tracing and workflow events#4491
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
dosubot
Bot
added
size:XXL
dosubot
Bot
added
size:XL
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR consolidates application and evaluator revision events into a unified workflow-domain event stream. It introduces an ChangesRevision Event Domain Consolidation and Idempotent Commits
🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR attempts to fix tracing endpoint usage and align revision workflow events so application/evaluator revisions emit under the workflow event domain. It also changes initial revision creation paths to use commit flows and adds trace/revision event publication in several routers.
Changes:
- Updates web tracing clients from
/tracing/...routes to newer/spansand/tracesroutes. - Adds workflow revision event types and switches application/evaluator revision events to
workflow. - Routes revision creation through commit methods with an
initialguard and adds trace/read event emission.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
web/packages/agenta-entities/src/trace/state/store.ts |
Updates tracing endpoint documentation. |
web/packages/agenta-entities/src/trace/api/api.ts |
Changes trace API endpoint paths. |
web/oss/src/services/variantConfigs/api/index.ts |
Removes variant config fetch API module. |
web/oss/src/services/tracing/api/index.ts |
Changes OSS tracing endpoint paths. |
api/oss/tests/pytest/unit/events/test_service_commit_emission.py |
Updates commit emission expectations to workflow events. |
api/oss/tests/pytest/unit/events/test_events_utils.py |
Updates event utility tests for workflow revision events. |
api/oss/src/dbs/postgres/git/dao.py |
Adds initial-commit guard to revision commits. |
api/oss/src/core/workflows/service.py |
Emits workflow commit events from workflow commits. |
api/oss/src/core/webhooks/types.py |
Updates subscribable webhook revision event types. |
api/oss/src/core/testsets/service.py |
Adds initial flag and uses commit for simple create placeholder revision. |
api/oss/src/core/queries/service.py |
Adds initial flag and uses commit for simple create placeholder revision. |
api/oss/src/core/git/interfaces.py |
Extends commit revision interface with initial flag. |
api/oss/src/core/events/utils.py |
Replaces application/evaluator revision mappings with workflow mappings. |
api/oss/src/core/events/types.py |
Adds workflow revision event enum values and removes active app/evaluator values. |
api/oss/src/core/evaluators/service.py |
Emits evaluator commits as workflow commit events. |
api/oss/src/core/environments/service.py |
Passes initial flag through environment commit flow. |
api/oss/src/core/applications/service.py |
Emits application commits as workflow commit events. |
api/oss/src/apis/fastapi/workflows/router.py |
Routes workflow revision creation through commit and emits read events. |
api/oss/src/apis/fastapi/tracing/router.py |
Adds trace fetched/queried event publication. |
api/oss/src/apis/fastapi/traces/router.py |
Adds trace event publication for trace routes. |
api/oss/src/apis/fastapi/testsets/router.py |
Routes testset revision creation through commit. |
api/oss/src/apis/fastapi/queries/router.py |
Routes query revision creation through commit. |
api/oss/src/apis/fastapi/evaluators/router.py |
Routes evaluator revision creation through commit and emits workflow read events. |
api/oss/src/apis/fastapi/environments/router.py |
Routes environment revision creation through commit. |
api/oss/src/apis/fastapi/applications/router.py |
Routes application revision creation through commit and emits workflow read events. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dosubot
Bot
added
size:L
Railway Preview Environment
Updated at 2026-05-29T16:05:31.953Z |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
api/oss/src/core/webhooks/types.py (1)
87-102:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject legacy application/evaluator revision event types on create/edit.
These values are still part of
WebhookSubscriptionData.event_types, so new subscriptions can now target event types that this PR explicitly stops emitting. That creates silently broken webhook subscriptions. Keep them readable for old rows, but remove them from the write contract or validate them out on create/edit.Possible fix
-from pydantic import BaseModel, HttpUrl +from pydantic import BaseModel, HttpUrl, model_validator @@ class WebhookEventType(str, Enum): @@ EVALUATORS_REVISIONS_LOGGED = EventType.EVALUATORS_REVISIONS_LOGGED.value EVALUATORS_REVISIONS_COMMITTED = EventType.EVALUATORS_REVISIONS_COMMITTED.value @@ +LEGACY_NON_EMITTED_WEBHOOK_EVENT_TYPES = frozenset( + { + WebhookEventType.APPLICATIONS_REVISIONS_RETRIEVED, + WebhookEventType.APPLICATIONS_REVISIONS_FETCHED, + WebhookEventType.APPLICATIONS_REVISIONS_QUERIED, + WebhookEventType.APPLICATIONS_REVISIONS_LOGGED, + WebhookEventType.APPLICATIONS_REVISIONS_COMMITTED, + WebhookEventType.EVALUATORS_REVISIONS_RETRIEVED, + WebhookEventType.EVALUATORS_REVISIONS_FETCHED, + WebhookEventType.EVALUATORS_REVISIONS_QUERIED, + WebhookEventType.EVALUATORS_REVISIONS_LOGGED, + WebhookEventType.EVALUATORS_REVISIONS_COMMITTED, + } +) + class WebhookSubscriptionCreate(Header, Metadata): data: WebhookSubscriptionData secret: Optional[str] = None + + `@model_validator`(mode="after") + def validate_event_types(self): + unsupported = set(self.data.event_types or []) & LEGACY_NON_EMITTED_WEBHOOK_EVENT_TYPES + if unsupported: + raise ValueError( + "Legacy application/evaluator revision webhook event types are read-only and cannot be subscribed to." + ) + return selfApply the same validation to
WebhookSubscriptionEdit.</details> Also applies to: 119-147 </blockquote></details> <details> <summary>api/oss/src/core/environments/service.py (1)</summary><blockquote> `1007-1016`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Preserve `initial` when the delta path re-enters commit.** `commit_environment_revision(..., initial=True)` forwards the new flag to the DAO on the direct path, but the delta branch drops it when it bounces through `_commit_environment_revision_delta()`. That means retries of an initial delta commit bypass the new idempotency guard. <details> <summary>Suggested fix</summary> ```diff return await self._commit_environment_revision_delta( project_id=project_id, user_id=user_id, environment_revision_commit=environment_revision_commit, + initial=initial, ) @@ async def _commit_environment_revision_delta( self, *, project_id: UUID, user_id: UUID, # environment_revision_commit: EnvironmentRevisionCommit, + initial: bool = False, ) -> Optional[EnvironmentRevision]: @@ return await self.commit_environment_revision( project_id=project_id, user_id=user_id, environment_revision_commit=environment_revision_commit, + initial=initial, _normalize_references=False, )Also applies to: 1103-1178
api/oss/src/core/queries/service.py (1)
1060-1083:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPass
initial=Trueon the placeholder commit.This is now the initial revision path, but it still uses the default
initial=False. That skips the new duplicate guard incommit_revision(...), so retries can still append an extra placeholder revision and emit an extra commit event.💡 Suggested fix
placeholder_revision = await self.queries_service.commit_query_revision( project_id=project_id, user_id=user_id, # query_revision_commit=_query_revision_commit, + initial=True, )api/oss/src/core/testsets/service.py (1)
1271-1296:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMark the placeholder testset revision as the initial commit.
This first revision now goes through
commit_testset_revision(...), but the call still usesinitial=False. As written, the new initial-commit guard never applies to the placeholder v0 path, so repeated create attempts can still produce an extra initial revision/event.💡 Suggested fix
testset_revision: Optional[ TestsetRevision ] = await self.testsets_service.commit_testset_revision( project_id=project_id, user_id=user_id, # testset_revision_commit=testset_revision_commit, + initial=True, )api/oss/src/apis/fastapi/applications/router.py (1)
1423-1457:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMirror the commit route's git error handling here.
create_application_revisionnow goes throughcommit_application_revision(...), but it still lacks@handle_git_exceptions(). That makes initial-create failures surface differently from/applications/revisions/commitfor the same git-layer conflict/validation paths.💡 Suggested fix
- `@intercept_exceptions`() + `@intercept_exceptions`() + `@handle_git_exceptions`() async def create_application_revision(api/oss/src/apis/fastapi/environments/router.py (1)
849-877:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winDon't let
/revisions/bypass guarded-environment checks.This endpoint now performs a real commit, but unlike
/environments/revisions/commitit never callsensure_environment_deploy_allowed(...). That lets a caller withEDIT_ENVIRONMENTSbut withoutDEPLOY_ENVIRONMENTScreate the initial revision on a guarded environment through this path.💡 Suggested fix
- `@intercept_exceptions`() + `@intercept_exceptions`() + `@handle_git_exceptions`() async def create_environment_revision( self, request: Request, *, environment_revision_create_request: EnvironmentRevisionCreateRequest, @@ - environment_revision = await self.environments_service.commit_environment_revision( + commit = EnvironmentRevisionCommit( + **environment_revision_create_request.environment_revision.model_dump( + mode="json", + exclude_none=True, + ), + message="Initial revision", + ) + + await ensure_environment_deploy_allowed( + project_id=UUID(request.state.project_id), + user_id=UUID(request.state.user_id), + environment_id=commit.environment_id, + environments_service=self.environments_service, + ) + + environment_revision = await self.environments_service.commit_environment_revision( project_id=UUID(request.state.project_id), user_id=UUID(request.state.user_id), # - environment_revision_commit=EnvironmentRevisionCommit( - **environment_revision_create_request.environment_revision.model_dump( - mode="json", - exclude_none=True, - ), - message="Initial revision", - ), + environment_revision_commit=commit, # initial=True, )
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8c393205-ac6d-45c2-81d3-0e00bb27cb1a
📒 Files selected for processing (23)
api/oss/src/apis/fastapi/applications/router.pyapi/oss/src/apis/fastapi/environments/router.pyapi/oss/src/apis/fastapi/evaluators/router.pyapi/oss/src/apis/fastapi/queries/router.pyapi/oss/src/apis/fastapi/testsets/router.pyapi/oss/src/apis/fastapi/traces/router.pyapi/oss/src/apis/fastapi/tracing/router.pyapi/oss/src/apis/fastapi/workflows/router.pyapi/oss/src/core/applications/service.pyapi/oss/src/core/environments/service.pyapi/oss/src/core/evaluators/service.pyapi/oss/src/core/events/types.pyapi/oss/src/core/events/utils.pyapi/oss/src/core/git/interfaces.pyapi/oss/src/core/queries/service.pyapi/oss/src/core/testsets/service.pyapi/oss/src/core/webhooks/types.pyapi/oss/src/core/workflows/service.pyapi/oss/src/dbs/postgres/git/dao.pyapi/oss/tests/pytest/unit/events/test_events_utils.pyapi/oss/tests/pytest/unit/events/test_service_commit_emission.pyweb/oss/src/services/variantConfigs/api/index.tsweb/packages/agenta-entities/src/trace/state/store.ts
💤 Files with no reviewable changes (1)
- web/oss/src/services/variantConfigs/api/index.ts
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 18f99132-e1a8-4cfb-8f82-1a33bb7be2df
📒 Files selected for processing (9)
api/oss/src/apis/fastapi/applications/router.pyapi/oss/src/apis/fastapi/environments/router.pyapi/oss/src/apis/fastapi/evaluators/router.pyapi/oss/src/apis/fastapi/git/exceptions.pyapi/oss/src/apis/fastapi/queries/router.pyapi/oss/src/apis/fastapi/testsets/router.pyapi/oss/src/apis/fastapi/workflows/router.pyapi/oss/src/core/git/types.pyapi/oss/src/dbs/postgres/git/dao.py
🚧 Files skipped from review as they are similar to previous changes (6)
- api/oss/src/apis/fastapi/queries/router.py
- api/oss/src/apis/fastapi/testsets/router.py
- api/oss/src/apis/fastapi/workflows/router.py
- api/oss/src/apis/fastapi/environments/router.py
- api/oss/src/apis/fastapi/evaluators/router.py
- api/oss/src/apis/fastapi/applications/router.py
Removes APPLICATIONS_REVISIONS_* and EVALUATORS_REVISIONS_* from EventType and WebhookEventType (applications/evaluators now emit as workflow events). Adds two defensive guards so existing stored data with the old string values doesn't cause runtime errors: - mappings.py: fall back to EventType.UNKNOWN for unrecognized DB rows - webhooks/types.py: drop unknown event_type strings from subscription data instead of raising a validation error on load Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The coercion belongs at the type boundary, not the mapping function. Any code path constructing an Event from a raw string now gets the fallback to UNKNOWN automatically. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
dosubot
Bot
added
size:XL
No description provided.