docs: scope client attempt-id and workspace-scoping for billing telemetry (Phase B)

[claude]

Part of the billing & subscription telemetry observability program.

Program context

📋 Program dossier — the public plan doc for this program: the problem, the two-rail billing architecture, the tool split, the phase structure, and the standing constraints on every PR in the program. Written to be read by agent reviewers as well as humans; about three minutes.

How to review this PR

What this PR does. Adds docs/adr/0014-billing-telemetry-attempt-correlation-and-workspace-scoping.md (94 lines) and updates the docs/adr/README.md index. Documentation only — no application code, no tests. Status is Proposed, so what you are approving is the recommendation, not shipped behavior.

It covers two Phase B design decisions that had no written recommendation:

1. Client-side billing_attempt_id. billing_op_id is minted by the backend and only reaches the client once the initiating request succeeds. If that single initiating call throws or times out first, the resulting failure event has nothing to join against. Recommendation: mint a client-side UUID and stamp it on every event for that attempt; do not send it to the backend yet.
2. workspace_id + a PostHog group. getBillingTelemetryEventPayload() carries no workspace identity, and posthog.group() is called nowhere in the codebase. Recommendation: add workspace_id to the shared payload builder (so every provider gets it for free, Datadog RUM included) and register the PostHog group.

Where it sits in the program. This is the design gate in front of b4-attempt-id and b5-workspace-scoping. Both implementation items are blocked on the recommendations here, so this is a decision review rather than a code review.

Focus your review on:

1. Decision 1 — is deferring the backend-coordinated attempt ID the right call? The ADR rejects a backend-issued, request-plumbed ID as disproportionate "until failure-rate data justifies it," and is explicit that the client-side-only version cannot join backend records. That trade is the substance of this PR. The honest counter-argument is that the program's whole reason for existing is that frontend attempts and backend outcomes cannot be joined; if you think this defers the actual problem, say so here.
2. Decision 2 — is shipping both the payload field and the group call justified? The ADR argues the field alone misses PostHog's group UI and the group call alone does nothing for Datadog RUM. Check that reasoning holds.
3. The open questions in ## Notes are the right open questions, and that none of them is actually a blocker disguised as a follow-up — particularly whether personal (non-team) workspaces should register as a PostHog group at all.
4. Format and house style against the existing series (ADR-0011/0012 are the most recent examples).

Known gap a reviewer should weigh. Program research since this ADR was drafted found that the backend keys its billing events on the workspace creator while the frontend keys on the acting user. Any team operation performed by someone who did not create the workspace therefore shows 100% drop-off at the first backend step, and personal workspaces are unaffected — which is exactly why it survives casual testing. That makes Decision 2 a prerequisite for any working funnel, not the "cheap nice-to-have" the ADR currently frames it as. It is worth deciding whether this ADR should be amended to say so before it is accepted.

Deliberately out of scope. No implementation — the payload builder, the store, and the providers are all untouched. No backend changes. No decision on Datadog RUM global-context registration (noted as a follow-up rather than decided).

⚠️ Merge order — #14208 must merge before this PR. This PR reflows every column of the ADR index table in docs/adr/README.md while #14208 adds a single 0013 row, so the two will conflict. The intended sequence is: merge #14208, then rebase this branch. Please do not pre-resolve it here. (The index in this branch already leaves the 0013 slot to #14208 and adds only 0014.)

Summary

Adds ADR-0014 recording recommendations for two Phase B billing-telemetry design decisions: a client-side billing_attempt_id for correlating failures that occur before any server-issued billing_op_id exists, and a workspace_id / PostHog-group association so billing funnels and Datadog RUM sessions can be segmented by workspace.

Each decision documents context, the alternatives considered, the recommendation, and consequences — grounded in the current getBillingTelemetryEventPayload() payload shape, billingOperationStore.ts's operation lifecycle, and the existing (group-less) PostHog and Datadog RUM provider code.

Changes

• New: docs/adr/0014-billing-telemetry-attempt-correlation-and-workspace-scoping.md — status Proposed.
• Modified: docs/adr/README.md — index entry for 0014.
• Breaking: none. Documentation only.

Verification

Documentation change — no test or behavior surface. CI green.

Screenshots (if applicable)

N/A — documentation change.

---

🤖 Generated with Claude Code

[christian-byrne]

Should be in docs/adr. remove any private context

[claude]

@christian-byrne replying to your review (Should be in docs/adr. remove any private context):

Done on both points:

• Moved the scoping doc into docs/adr/0013-billing-telemetry-attempt-correlation-and-workspace-scoping.md, matching the existing ADR format (Status/Context/Decision/Consequences/Notes) used by ADR-0001–0012, and added it to the docs/adr/README.md index. The old docs/telemetry/phase-b-attempt-id-and-workspace-scoping.md is deleted.
• Stripped private context from the ADR: no Comfy-Org/cloud/comfy-infra repo names, no Slack links, no internal dashboard/PostHog/Datadog IDs, no internal person names, and no PR numbers of uncertain provenance. Where the original said a decision "requires a Comfy-Org/cloud change," it now reads "requires a corresponding backend change to accept and persist the field" — same technical shape, no private repo name. Also removed the incident-88 labeling and the dead link to a non-existent internal master-plan doc from the PR description prose (the required Slack-thread attribution line stays, per the separate attribution mechanism).

Verified by grepping the final ADR file for Comfy-Org/cloud, comfy-infra, and comfy-organization.slack.com — no matches.

Pushed to docs/phase-b-scoping-v2 at 52b6c9c.

---

Generated by Claude Code

[christian-byrne]

@DrJKL @dante01yoon — requesting review; this PR had no reviewer requested at all until now. It should be quick: 2 files, docs only (a 94-line ADR plus the ADR index).

Status is Proposed, so what you'd be approving is the recommendation, not shipped behavior. It covers two Phase B design decisions — a client-side billing_attempt_id, and workspace_id + a PostHog group on the shared telemetry payload. @dante01yoon these are from your correlation-gap list, so your read on decision 1 (deferring a backend-coordinated attempt ID) is the one that matters most.

The PR body has a "how to review this" primer, links the public program dossier, and flags one thing worth deciding before this is accepted: later research found the backend keys billing events on the workspace creator while the frontend keys on the acting user, so any team operation by a non-creator shows 100% drop-off at the first backend step. That makes decision 2 a prerequisite for any working funnel rather than the cheap nice-to-have the ADR currently calls it — possibly worth an amendment.

Merge order: after #14208 (ADR index conflict).

[dante01yoon]

The overall direction is sound, but the workspace-scoping decision still permits incorrect attribution. The three inline issues need to be resolved before this ADR becomes the implementation gate for B4/B5.

[dante01yoon]

issue: Reading workspaceId inside getBillingTelemetryEventPayload() captures the workspace at emission time, not the workspace that initiated the billing attempt. Several events are emitted after an await, and billingOperationStore.stopIfTimedOut() can emit before its inactive-workspace guard; switching from workspace A to B can therefore label A’s terminal event as B. Could the decision require workspace_id to be snapshotted with billing_attempt_id at attempt start, retained on the operation, and supplied on every event instead? PostHog’s per-event group attribution should use that same snapshot so $groups.workspace cannot disagree with workspace_id.

[claude]

Addressed in 61fc7cc. Decision 1 now requires workspace_id to be snapshotted alongside billing_attempt_id at attempt start and retained on the useBillingOperationStore operation record. Decision 2 now requires every event — including ones fired after an await or from stopIfTimedOut() — to read that snapshot rather than a live useTeamWorkspaceStore().workspaceId read at emission time, and requires posthog.group('workspace', workspaceId) to use that same snapshot, so $groups.workspace can never disagree with the workspace_id payload field.

---

Generated by Claude Code

[dante01yoon]

issue: The PR body documents a material fact that the durable ADR omits: backend billing records are keyed by workspace creator while frontend events are keyed by acting user, so team operations by non-creators appear as 100% first-step drop-off and personal-workspace testing masks it. That makes workspace_id the prerequisite common scope for a valid cross-layer funnel, not just a cheap enhancement for group UI. Please add this to Context and reflect the requirement in the decision/consequences so future readers understand why B5 is mandatory.

[claude]

Addressed in 61fc7cc. Added the creator-vs-acting-user key mismatch to Context (new paragraph after the "Workspace scoping" gap description), and reframed Decision 2's opening line and the Consequences/Positive bullets to state workspace_id is the prerequisite common scope for a valid cross-layer funnel, not an optional enhancement for group UI.

---

Generated by Claude Code

[dante01yoon]

issue: Personal-workspace handling cannot remain an implementation follow-up because posthog.group() persists $groups under this project’s localStorage+cookie configuration. If the watcher simply skips personal workspaces, switching team → personal leaves subsequent events associated with the prior team, including after reload. Please decide the transition semantics here: either group every non-null workspace ID (including personal), or explicitly clear the workspace group when entering personal context.

[claude]

Addressed in 61fc7cc. Decided in Decision 2 rather than deferred: group every non-null workspaceId, including personal workspaces. useTeamWorkspaceStore already models a personal workspace as an ordinary entry with its own id (never a null/special state), so there's nothing to special-case, and it avoids the stale-$groups hazard under this project's persistence: 'localStorage+cookie' PostHog config that skipping personal workspaces would create on a team → personal transition. Removed the corresponding "open" item from Consequences/Negative and Notes.

---

Generated by Claude Code

[dante01yoon]

The original three findings are addressed, but the revised carrier and group semantics introduce two correctness gaps. These need clarification before the ADR is a safe implementation gate.

[dante01yoon]

issue: useBillingOperationStore cannot be the universal carrier described here. Its records require a server-issued opId and are created by startOperation() only after the initiating response, while the motivating pre-response failure has no record; resubscribe and legacy-rail mutations also emit without entering this store. Please define a rail-neutral attempt context, created in the initiating composable before its first async call, and carry billing_attempt_id plus snapshotted workspace_id on every BillingTelemetryEvent. When polling does begin, the same context can then be passed into startOperation() for the store to retain.

[claude]

Addressed in a5d661f. Decision 1 no longer names useBillingOperationStore as the carrier. It's now a rail-neutral attempt context — a plain object created by the initiating composable (useSubscriptionCheckout.ts, useResubscribe.ts, the legacy top-up rail) before its first await — holding billing_attempt_id and the workspace_id snapshot. Every BillingTelemetryEvent for the attempt, including pre-response failures, reads from that context, not the store. Once/if a server-issued opId arrives, the same context is passed into startOperation() so the store retains it for the polling-driven terminal events.

---

Generated by Claude Code

[dante01yoon]

issue: posthog.group() mutates persistent global $groups state; it is not an event-local association. If workspace B is active and a late terminal event from A calls group(..., A), that event is labeled correctly but every later unrelated event remains attributed to A because the active-workspace watcher will not fire again (and the configured persistence can carry this across reloads). Please make the active-workspace watcher the sole owner of posthog.group(), and require the billing capture itself to attach event-local $groups: { workspace: snapshottedWorkspaceId } alongside workspace_id without mutating global group state.

[claude]

Addressed in a5d661f. Decision 2 now makes a dedicated active-workspace watcher (analogous to the existing watch(tier, ...) in PostHogTelemetryProvider.setSubscriptionProperties()) the sole caller of posthog.group(), since that call mutates persistent $groups state via register() under this project's persistence: 'localStorage+cookie' config. The billing capture path no longer calls group() at all — it attaches the snapshotted workspace_id as an event-local override via $groups: { workspace: workspaceId } in the properties passed to posthog.capture(), alongside the workspace_id payload field, so a late/stale-workspace event is labeled correctly on itself without touching global group state that later unrelated events would inherit.

---

Generated by Claude Code

[dosubot]

📄 Knowledge review

Dosu skipped reviewing this PR because your organization has used its 200 included credits for the month. Your usage will reset on 2026-08-01. To have Dosu review this PR before then, ask your organization admin to upgrade to a pro account.

---

[christian-byrne]

Verdict

Post-merge review (this landed at 919d6ca4 on 2026-07-30) — so everything below is amendment-shaped, not blocking. The headline is good news: the thing I came here to flag is already fixed. Earlier drafts framed Decision 2 as a segmentation nicety; the merged Context section now states the creator-vs-actor mismatch outright and calls workspace_id "the prerequisite common scope for a valid cross-layer funnel, not an optional enhancement". That is exactly right, and it is the single most important sentence in the document. I verified every code claim in the ADR against main and they all hold.

Two things I would still amend. One is substantive: the ADR claims the frontend change closes the creator-vs-actor mismatch, and it does not — not on its own. The other is that the doc grew from 94 lines to 165 after the concision pass, and is now longer than every ADR in the recent series. Everything else is nits.

---

Findings

1. (moderate, overclaims the fix) workspace_id on the client does not by itself make the cross-layer funnel join

Line 130-134, Consequences → Positive:

• workspace_id becomes available on every billing event across PostHog
  and Datadog RUM through one shared payload builder, closing the
  creator-vs-actor key mismatch that otherwise breaks cross-layer funnels
  for team operations.

The backend already sends workspace_id as a plain event property today, so after this change both sides will carry a matching property — useful for filtering, but PostHog funnels aggregate on the person by default, not on an arbitrary property. Making the funnel actually join requires it to be group-aggregated, and a group-aggregated insight silently excludes events that carry no $groups value. The backend forwarder captures under the workspace creator's user ID and sets no $groups at all.

So shipping Decision 2 as written moves the failure rather than removing it: instead of backend events landing on the wrong person, they drop out of a workspace-aggregated funnel entirely. Same 100% apparent drop-off, different cause — and harder to spot, because the frontend side will look correctly instrumented.

Proposed replacement for that bullet:

- `workspace_id` becomes available on every billing event across PostHog and
  Datadog RUM through one shared payload builder, giving both layers a common
  scope. This is the necessary half of closing the creator-vs-actor key
  mismatch, not the whole of it: a PostHog funnel only joins on workspace if
  it is group-aggregated, and group-aggregated insights exclude events with
  no `$groups` value. The backend forwarder currently sets none, so a paired
  backend change — emitting the same `workspace` group on forwarded billing
  events — is required before any FE→BE funnel joins for team operations.

...and a matching entry under ### Negative:

- Until the backend forwarder emits a matching `workspace` group, backend
  billing events will be absent from any workspace-aggregated funnel rather
  than merely mis-attributed. Track that as an explicit dependency of this
  ADR, not a follow-up nicety.

2. (minor, precision) "Backend billing records are keyed by the workspace creator" is not quite where the bug lives

Line 32-34:

Backend billing records are
keyed by the workspace creator, while frontend telemetry events are
keyed by the acting user.

The persisted billing records are not keyed that way — the substitution happens at the PostHog forwarding boundary, which captures under ws.CreatedByUserID. As written, a backend engineer reading this will go looking for the mis-keying in the billing tables, not find it, and conclude the ADR is wrong.

Proposed:

Backend billing events are forwarded to PostHog under the workspace
**creator's** user ID, while frontend telemetry events are captured under the
**acting user's** ID.

3. (minor, brevity/format drift) The ADR is now the longest of the recent series

165 lines / 1,228 words. It was trimmed to 94 lines on 2026-07-29 in response to the concision feedback, then grew back across 61fc7cc8 and a5d661f0. For scale: 0012 = 74 lines, 0010 = 92, 0011 = 120. Only 0008 (284) and 0009 (329) are longer, and both of those are far larger decisions.

The growth is concentrated in two places, and both are mechanism, not decision:

• Decision 1 is one unbroken 22-line paragraph (L50-71) that explains why the operation store can't be the carrier.
• Decision 2's posthog.group() persistence explanation (L93-118) is 26 lines on super-property semantics and stale-group hazards.

Both are correct and worth keeping — but they read as implementation notes inside a decision section. Suggested follow-up: keep a two-or-three-sentence decision in each, and move the mechanics into a short ### Mechanics subsection or into ## Notes. That would land the doc around 110-120 lines, in line with 0011.

4. (nit) groupIdentify() appears in Consequences and Notes but is never introduced

L154 and L165 both refer to groupIdentify(), but the Decision section only ever decides on posthog.group(type, key). A reader meets the term for the first time in a consequence. Either name it in Decision 2 (posthog.group()'s third argument routes to groupIdentify), or reword both to "what additional properties the workspace group should carry".

5. (nit, format) Missing the status-legend comment

0010/0011/0012 and the template in docs/adr/README.md all carry this directly under the status value; 0014 omits it:

<!-- [Proposed | Accepted | Rejected | Deprecated | Superseded by [ADR-NNNN](NNNN-title.md)] -->

6. (nit) ## Context's payload-field list reads as exhaustive but isn't

L23-26 lists the builder's output as operation, stage, outcome, billing_op_id, tier, cycle, checkout_type, failure_category. getBillingTelemetryEventPayload() (src/platform/telemetry/types.ts:742) also spreads operation_type and payment_intent_source. Doesn't change the argument — the "no workspace identity" point stands — but "purely from ... fields" plus a list invites the reader to treat it as complete.

7. (nit) Proposed status carrying RFC-2119 MUST

L78: "Every emitted event for an attempt — terminal or otherwise — MUST use the workspace_id carried in the attempt context". Normative language in a Proposed ADR is a small mixed signal. Either flip to Accepted when b4/b5 are approved, or soften to "should".

---

What I checked and found fine

• Decision 2 is correctly framed as a prerequisite, not a nice-to-have. L28-46 states the creator-vs-actor mismatch, the 100%-drop-off consequence, and — importantly — why it survives testing (personal workspaces have creator == actor). That last clause is the part that makes the finding actionable rather than abstract. This was my main concern coming in and it is fully addressed.
• Every code claim verified against main:
  • posthog.group / groupIdentify / $groups — zero occurrences anywhere in src/. Claim accurate.
  • persistence: 'localStorage+cookie' — confirmed at PostHogTelemetryProvider.ts:154, so the super-property/reload-survival reasoning in Decision 2 is sound.
  • The watch(tier, ...) analogue in setSubscriptionProperties() — confirmed at PostHogTelemetryProvider.ts:350-353.
  • getBillingTelemetryEventPayload() carries no workspace identity — confirmed at types.ts:742 (see nit 6 for the field-list quibble).
  • resubscribe() and the legacy top-up rail emit BillingTelemetryEvents without calling startOperation() — confirmed; this is why the rail-neutral attempt context rather than the operation store is the right carrier. That reasoning is the strongest part of the ADR.
• Private context. Clean. Grepped the merged file for infra repo names, monitor/dashboard IDs, analytics project/insight IDs, Slack links or channel names, on-call handles, and personal names — zero hits. Unlike 0013, this one carries no residual Slack references. The PR body's <!-- ccr-slack-attribution --> header carries no URL and no personal name; that scrub held through merge.
• Numbering. No collision and no dangling 0013 reference anywhere in the merged file. The 0013 slot the index leaves open is filled by #14208, which has since merged main and adds its row above the 0014 row with matching column widths.
• Format. Heading order matches the README template and 0010-0012 exactly (# N. Title / Date: / ## Status / ## Context / ## Decision / ## Consequences → ### Positive / ### Negative / ## Notes).
• Durability. No PR numbers presented as current state, no "we are currently doing X", no counts that will drift. The Proposed status is honest about what merging did and didn't commit to.
• The deferral in Decision 1 is well-argued. Rejecting a backend-issued request-plumbed ID as disproportionate until failure-rate data justifies it, while naming the exact open question that would settle it (pre- vs post-response failure split, first item in ## Notes), is the right shape for a deferral — it is falsifiable rather than open-ended.

[christian-byrne]

This overclaims what the frontend change buys. The backend already sends workspace_id as a plain event property, so after this lands both sides carry a matching property — but PostHog funnels aggregate on the person by default, not on an arbitrary property. Joining on workspace requires a group-aggregated funnel, and group-aggregated insights exclude events carrying no $groups value. The backend forwarder captures under the workspace creator's user ID and sets no $groups at all.

So as written, Decision 2 relocates the failure rather than closing it: backend events go from landing on the wrong person to being absent from the funnel entirely — and it is harder to spot, because the frontend will look correctly instrumented.

Suggested replacement for this bullet:

- `workspace_id` becomes available on every billing event across PostHog and
  Datadog RUM through one shared payload builder, giving both layers a common
  scope. This is the necessary half of closing the creator-vs-actor key
  mismatch, not the whole of it: a PostHog funnel only joins on workspace if
  it is group-aggregated, and group-aggregated insights exclude events with
  no `$groups` value. The backend forwarder currently sets none, so a paired
  backend change — emitting the same `workspace` group on forwarded billing
  events — is required before any FE-to-BE funnel joins for team operations.

Plus a matching ### Negative entry so the dependency is tracked rather than assumed.

[christian-byrne]

Precision nit on an otherwise excellent paragraph — this whole Context addition is the right call and I would not want it weakened.

The persisted billing records aren't keyed by the creator; the substitution happens at the PostHog forwarding boundary, which captures under ws.CreatedByUserID. As written, a backend reader goes looking in the billing tables, doesn't find it, and concludes the ADR is wrong.

Backend billing events are forwarded to PostHog under the workspace
**creator's** user ID, while frontend telemetry events are captured under the
**acting user's** ID.

[christian-byrne]

Nit: this list reads as exhaustive ("purely from ... fields") but getBillingTelemetryEventPayload() (src/platform/telemetry/types.ts:742) also spreads operation_type and payment_intent_source. Doesn't affect the argument — the "no workspace identity" point stands.

[christian-byrne]

groupIdentify() is introduced for the first time here (and again in ## Notes at L165), but the Decision section only ever decides on posthog.group(type, key). Either name it in Decision 2 — posthog.group()'s third argument is what routes to groupIdentify — or reword both to "what additional properties the workspace group should carry".

[christian-byrne]

Nit, format: 0010/0011/0012 and the template in docs/adr/README.md all carry the status legend directly under the status value. 0014 omits it:

<!-- [Proposed | Accepted | Rejected | Deprecated | Superseded by [ADR-NNNN](NNNN-title.md)] -->

Separate nit on the same section: L78 uses RFC-2119 MUST while the status is Proposed. Worth either flipping to Accepted once b4/b5 are approved, or softening to "should".