Skip to content

Commit 7f0802a

Browse files
committed
fix(handlers): address maintainer feedback on lines_of_code semantics
- activity.ts: guard sessionDiffTotals writes so setBoundedMap only fires on insert; updates to an existing key now use plain .set(). Prevents evicting an unrelated active session when the map is at MAX_PENDING, which would cause that session's next session.diff to be treated as first-seen and re-emit its full cumulative diff as a delta — reintroducing the double-count bug for the evicted session. - docs: stop calling the counter "net". It is gross positive churn — negative deltas (additions/deletions shrinking, e.g. cumulative +10/-0 -> +5/-5) are dropped, so sums do not reconcile to net after any revert, full or partial. lines_of_code.total gauge is the authoritative live cumulative. Counter description, gauge description, handler docstring, and README all updated. - tests: add regression for the partial-revert case (+10/-0 -> +5/-5) that exercises the semantics gap the maintainer flagged — asserts counter emits gross (added=10, removed=5) and gauge tracks live cumulative (added=[10,5], removed=[0,5]).
1 parent a25022c commit 7f0802a

4 files changed

Lines changed: 40 additions & 9 deletions

File tree

README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ An [opencode](https://opencode.ai) plugin that exports telemetry via OpenTelemet
3131
| `opencode.session.count` | Counter | Incremented on each `session.created` event |
3232
| `opencode.token.usage` | Counter | Per token type: `input`, `output`, `reasoning`, `cacheRead`, `cacheCreation` |
3333
| `opencode.cost.usage` | Counter | USD cost per completed assistant message |
34-
| `opencode.lines_of_code.count` | Counter | Emits only the positive delta since the previous `session.diff`, so summing the counter avoids the old double-counting. **Caveats:** when opencode reports a cumulative drop (a revert), the negative delta is skipped — so summing the counter reflects gross additions across non-revert deltas and can overstate net after a revert. Rewrites that happen and are reverted *within a single message* aren't captured at all (opencode publishes the per-message cumulative, not intra-message churn). Use `opencode.lines_of_code.total` for the authoritative live total. |
35-
| `opencode.lines_of_code.total` | Gauge | Current cumulative lines added/removed for the session, refreshed on every `session.diff`. Drops back to `0` if opencode reports a revert to baseline. Authoritative for "what does this session currently amount to". |
34+
| `opencode.lines_of_code.count` | Counter | **Gross positive churn, not a net total.** Emits the positive delta of `additions`/`deletions` since the previous `session.diff` for the same session; negative deltas (when opencode's cumulative `additions` or `deletions` shrinks vs. the last event) are dropped. Summing the counter therefore reports gross lines added/removed across forward transitions — it does *not* reconcile back to the session's current state after any revert (full or partial). Intra-message rewrites that opencode collapses in its per-message cumulative are not visible here at all. Use `opencode.lines_of_code.total` for the authoritative live cumulative. |
35+
| `opencode.lines_of_code.total` | Gauge | **Authoritative live cumulative lines added/removed for the session.** Refreshed on every `session.diff` with opencode's current cumulative value. Drops back to `0` if opencode reports a revert to baseline, and tracks partial reverts faithfully. Query this (not the counter) to answer "what does this session currently amount to". |
3636
| `opencode.commit.count` | Counter | Git commits detected via bash tool |
3737
| `opencode.tool.duration` | Histogram | Tool execution time in milliseconds |
3838
| `opencode.cache.count` | Counter | Cache activity per message: `type=cacheRead` or `type=cacheCreation` |

src/handlers/activity.ts

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,12 @@ import type { HandlerContext } from "../types.ts"
66
/**
77
* Records lines-added/removed for a `session.diff` event. opencode publishes each event
88
* with the cumulative session diff (first snapshot → latest), so we emit two instruments:
9-
* `opencode.lines_of_code.count` (Counter) receives only the positive delta since the
10-
* previous event for this session, so summing across a session yields the net total without
11-
* double counting. `opencode.lines_of_code.total` (Gauge) records the current cumulative
12-
* total, overwritten on every event.
9+
* `opencode.lines_of_code.count` (Counter) receives only the *positive* per-event delta
10+
* for each dimension (additions, deletions). Negative deltas — opencode reporting a smaller
11+
* cumulative for a dimension than the previous event — are dropped, so the counter reports
12+
* gross positive churn and does not reconcile to net after any revert (full or partial).
13+
* `opencode.lines_of_code.total` (Gauge) mirrors opencode's current cumulative value on
14+
* every event and is the authoritative live view.
1315
*/
1416
export function handleSessionDiff(e: EventSessionDiff, ctx: HandlerContext) {
1517
const sessionID = e.properties.sessionID
@@ -25,7 +27,15 @@ export function handleSessionDiff(e: EventSessionDiff, ctx: HandlerContext) {
2527
const prev = ctx.sessionDiffTotals.get(sessionID) ?? { additions: 0, deletions: 0 }
2628
const deltaAdded = totalAdded - prev.additions
2729
const deltaRemoved = totalRemoved - prev.deletions
28-
setBoundedMap(ctx.sessionDiffTotals, sessionID, { additions: totalAdded, deletions: totalRemoved })
30+
const nextTotals = { additions: totalAdded, deletions: totalRemoved }
31+
if (ctx.sessionDiffTotals.has(sessionID)) {
32+
// Existing session: update in place. Calling setBoundedMap on a full map would
33+
// evict an unrelated session here, and that session's next session.diff would
34+
// be treated as first-seen — reintroducing the cumulative double-count bug.
35+
ctx.sessionDiffTotals.set(sessionID, nextTotals)
36+
} else {
37+
setBoundedMap(ctx.sessionDiffTotals, sessionID, nextTotals)
38+
}
2939

3040
const baseAttrs = { ...ctx.commonAttrs, "session.id": sessionID }
3141

src/otel.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,11 +147,11 @@ export function createInstruments(prefix: string): Instruments {
147147
}),
148148
linesCounter: meter.createCounter(`${prefix}lines_of_code.count`, {
149149
unit: "{line}",
150-
description: "Positive per-event delta of lines added/removed; sum reflects gross additions across non-revert deltas and may overstate net after a revert. Use lines_of_code.total for the authoritative live cumulative value.",
150+
description: "Gross positive churn of lines added/removed across a session. Emits the positive delta vs. the previous session.diff; negative deltas (cumulative shrinkage) are dropped, so sums do not reconcile to net after any revert. Use lines_of_code.total for the authoritative live cumulative.",
151151
}),
152152
linesTotalGauge: meter.createGauge(`${prefix}lines_of_code.total`, {
153153
unit: "{line}",
154-
description: "Authoritative live cumulative lines added/removed for the current session, refreshed on every session.diff; drops back to 0 if opencode reports a revert to baseline.",
154+
description: "Authoritative live cumulative lines added/removed for the current session. Mirrors opencode's session.diff cumulative value on every event; tracks partial and full reverts faithfully.",
155155
}),
156156
commitCounter: meter.createCounter(`${prefix}commit.count`, {
157157
unit: "{commit}",

tests/handlers/activity.test.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,27 @@ describe("handleSessionDiff", () => {
9191
expect(added).toEqual([5])
9292
})
9393

94+
test("linesCounter is gross-only across a partial revert (additions shrink, deletions grow)", () => {
95+
// Cumulative goes {additions:10, deletions:0} -> {additions:5, deletions:5}.
96+
// Delta is {added:-5, removed:+5}. Negative added is skipped; positive removed
97+
// is emitted. Counter ends at added=10, removed=5 while the authoritative live
98+
// cumulative is added=5, removed=5 — the counter is GROSS, not net. Live
99+
// cumulative state is surfaced via linesTotalGauge (see next test).
100+
const { ctx, counters, gauges } = makeCtx()
101+
handleSessionDiff(makeSessionDiff("ses_1", [{ file: "a.ts", additions: 10, deletions: 0 }]), ctx)
102+
handleSessionDiff(makeSessionDiff("ses_1", [{ file: "a.ts", additions: 5, deletions: 5 }]), ctx)
103+
104+
const added = counters.lines.calls.filter((c) => c.attrs["type"] === "added").map((c) => c.value)
105+
const removed = counters.lines.calls.filter((c) => c.attrs["type"] === "removed").map((c) => c.value)
106+
expect(added).toEqual([10])
107+
expect(removed).toEqual([5])
108+
109+
const gaugeAdded = gauges.linesTotal.calls.filter((c) => c.attrs["type"] === "added").map((c) => c.value)
110+
const gaugeRemoved = gauges.linesTotal.calls.filter((c) => c.attrs["type"] === "removed").map((c) => c.value)
111+
expect(gaugeAdded).toEqual([10, 5])
112+
expect(gaugeRemoved).toEqual([0, 5])
113+
})
114+
94115
test("linesTotalGauge records cumulative totals, including zero after revert", () => {
95116
const { ctx, gauges } = makeCtx()
96117
handleSessionDiff(makeSessionDiff("ses_1", [{ file: "a.ts", additions: 5, deletions: 2 }]), ctx)

0 commit comments

Comments
 (0)