Comfy-Org / Comfy-Org/ComfyUI_frontend

GraphMutations.batch failure attribution is lossy — three failure reasons collapse into one Sentry message

Open
#16,672 0 comments 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
TypeScript
Stars
2k
Forks
702
Avg merge
1d 8h
Merged PRs (30d)
512

Description

## Problem
`GraphMutations.batch` in `src/workbench/extensions/agent/crdt/ecsFollowerAdapter.ts` returns `false` for three unrelated reasons:

1. `deps.getScope()` is `null` — a transient state (`AgentPanelRoot` returns `null` until `boundTabFor(workflowId)?.activeState?.id` hydrates), not a real failure.
2. `fail(message)` on `prepare` validation — a genuine malformed-payload rejection, whose reason string currently only reaches `console.error`.
3. `!session` in `applyFrame` — a third, distinct cause.

All three currently surface in Sentry as the same synthetic `agent_crdt_projection_failure` message, so a transient no-scope race (expected, recoverable) cannot be told apart from a genuinely malformed doc payload (a real bug) from the telemetry alone.

## Context
Discovered during review of [FE #16382](https://github.com/Comfy-Org/ComfyUI_frontend/pull/16382) (`fix(agent): surface host op nacks and projection failures`). Flagged by christian-byrne in [review thread F3](https://github.com/Comfy-Org/ComfyUI_frontend/pull/16382#discussion_r3910477320); explicitly deferred out of that PR ("Left as-is: attribution fix belongs in `GraphMutations.batch` and its adapter callers, a separate change.").

## Suggested Approach
Either have `GraphMutations.batch` return a discriminated result (e.g. `{ ok: true } | { ok: false, reason: 'no-scope' | 'invalid-payload' | 'no-session', detail?: string }`), or have the adapter throw with the specific reason, so `ecsFollowerAdapter.ts`'s Sentry report (`agent_crdt_projection_failure`) and any other callers can attribute failures correctly instead of collapsing them into one message.

## Estimated Effort
small (~1hr)

Contributor guide

Open the contributing guide

Research direction

Start in src/workbench/extensions/agent/crdt/ecsFollowerAdapter.ts at GraphMutations.batch and its agent_crdt_projection_failure Sentry report. Trace the adapter callers and the three reported causes: null scope, prepare validation failure, and missing session. Done means telemetry distinguishes no-scope, invalid-payload, and no-session failures without collapsing them into one message.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
distributed-systems, frontend, observability
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.