Comfy-Org / Comfy-Org/ComfyUI_frontend
Agent CRDT batch-settle outcomes reach only the debug panel, never telemetry
- Dominant language
- TypeScript
- Stars
- 2k
- Forks
- 699
- Avg merge
- 1d 7h
- Merged PRs (30d)
- 490
Description
## Problem / Goal
`useAgentCrdtFollower.ts:273` routes every batch-settle outcome — rejections included — to the dev panel and nowhere else:
```ts
onBatchSettled: (outcome) => recordDevEvent('human_ops_settled', outcome)
```
`recordDevEvent` opens with `if (!isCrdtDebugEnabled()) return` (`devPanelLog.ts:73`), so without `?crdtDebug=1` it is a complete no-op, and `devPanelLog.ts` contains no `reportError` call at all. In production, a human op the host rejects produces no signal in either console.
This is the failure mode the reporting convention exists for: it reads as zero in Sentry and zero in Datadog, so a rejection storm is indistinguishable from a healthy session. It is also self-concealing — the only way to see it is to already suspect it and re-run with the debug flag on.
Goal: host rejections of human ops are visible in telemetry without a debug flag.
## Proposed Solution
Not at this call site. The frontend follower does not run the applier, so it only ever sees the settled outcome, not the reason. The right seam is the `doc_ops_result` outcome union — whatever carries the rejection reason across the wire should decide what is reportable, and rejections should go through `reportError()` with a stable `errorType`, alongside (not instead of) the existing dev-panel tap.
Decide at the same time which rejection kinds are expected — a stale `baseVersion` losing a race is ordinary — versus genuinely anomalous, so the reportable set does not become noise. See `assertFailureReporter.ts` for the existing dedupe/cap pattern.
## Acceptance Criteria
- [ ] A host-rejected human op emits a `reportError()` with a stable `errorType` in a build with no debug flag set.
- [ ] Expected-loss rejections (e.g. stale `baseVersion`) are classified and do not report.
- [ ] Report volume is bounded per session, following the `assertFailureReporter.ts` dedupe pattern.
- [ ] The dev-panel tap still records the same outcomes when `?crdtDebug=1` is on.
- [ ] A unit test asserts the reporting path with `isCrdtDebugEnabled()` false, so a regression to dev-panel-only fails.
## Provenance
Surfaced while reviewing #17059, which proposed a CMP event-sink adapter as the fix. That PR was closed — the adapter had no consumer and no `applyOps` seam to attach to — but the underlying gap is real and outlived it.
Related: #17287, #17289, #17290.
Contributor guide
Research direction
Start at useAgentCrdtFollower.ts:273 and trace the doc_ops_result outcome union to the seam where rejection reasons cross the wire. Read devPanelLog.ts:73 and assertFailureReporter.ts to understand debug gating and bounded deduplication, then locate the existing unit-test conventions for reportError(). Done means anomalous host rejections report stable errorType values without the debug flag, expected stale-baseVersion losses stay quiet, and the debug-panel tap still works.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- frontend, observability
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100