From ebdcb3c4a74fc9960923c4d63d762a280a36f6dc Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Sat, 12 Sep 2026 10:05:31 +0200 Subject: [PATCH 1/4] docs(proforge): plan real token accounting for the Claude path --- docs/PROFORGE-CLAUDE-TOKEN-ACCOUNTING-PLAN.md | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 docs/PROFORGE-CLAUDE-TOKEN-ACCOUNTING-PLAN.md diff --git a/docs/PROFORGE-CLAUDE-TOKEN-ACCOUNTING-PLAN.md b/docs/PROFORGE-CLAUDE-TOKEN-ACCOUNTING-PLAN.md new file mode 100644 index 000000000..1d0001c89 --- /dev/null +++ b/docs/PROFORGE-CLAUDE-TOKEN-ACCOUNTING-PLAN.md @@ -0,0 +1,41 @@ +# Plan: real token accounting for the Claude path (ProForge) + +Status: **planned, not yet implemented** — prep doc from a `/claude-api prompt-audit` pass (2026-09-12). Implement in this branch (`fix/proforge-token-accounting`) when work resumes. + +## Finding + +`ProForge` agents label a raw **character** count as "tokens": + +- `services/proForge/pipelineAgents/structuralAgent.ts:67` — `tokensConsumed += response.length;` +- `services/proForge/pipelineAgents/diagnosticAgent.ts:72,91` — same pattern (initial call + retry) +- `services/proForge/pipelineAgents/publishingAgent.ts:52` +- `services/proForge/pipelineAgents/proofAgent.ts:51` +- `services/proForge/pipelineAgents/proseAgent.ts:91` (per-section loop) +- `services/proForge/pipelineAgents/copyEditAgent.ts:67` (per-section loop) +- `services/proForge/pipelineAgents/baseAgent.ts:213` — `selfReflect()`'s `tokensUsed: response.text.length` + +`services/aiProviderService.ts:362-369` (`deliverAnthropicResponse`) reads the full Anthropic response JSON and discards `json.usage` entirely — the real `usage.input_tokens`/`usage.output_tokens` (and thinking-token spend on Opus 5, which runs adaptive thinking by default) never reach the app. `AnalyticsAgent` has no AI call and needs no change. + +## Small fix (low risk, do first) + +Swap the character count for the existing token estimator already used elsewhere in this codebase (`services/ragPromptAssembly.ts:51-53`, `estimateTokens`): + +```ts +export function estimateTokens(text: string): number { + return Math.ceil((text.length / 4) * 1.3); +} +``` + +Import it in each of the six files above and replace `response.length` / `response.text.length` with `estimateTokens(response)` / `estimateTokens(response.text)`. Mechanical, no interface changes, no test breakage expected beyond any test asserting the old raw-length value. + +## Larger fix (design decision needed, not a blind diff) + +Thread Anthropic's real `usage` object back through the call chain instead of estimating: + +1. `deliverAnthropicResponse` (`services/aiProviderService.ts:357-372`) already has `json.usage` available — capture `{ inputTokens, outputTokens }` instead of discarding it. +2. That requires extending `AIStreamCallbacks`/`generateText`'s return shape (currently just `Promise`) to optionally carry usage, or a side-channel the ProForge agents can read. This is an interface change affecting every provider path (only Anthropic can populate it for now; others stay `undefined`) — decide the shape with the user before implementing, don't force it through as a mechanical hunk. +3. Once available, `structuralAgent.ts` etc. should prefer real `usage.output_tokens` when present, falling back to `estimateTokens()` for providers that don't return it. + +## Why this matters + +Without real usage data, the `MAX_TOKENS_CEILING`/timeout tuning in the companion plan (`docs/PROFORGE-CLAUDE-MAXTOKENS-CEILING-PLAN.md`) can't be validated from measurement — right now nobody can tell whether the app's self-imposed ceilings are actually being hit. From b0d25de45a5920430924ee4d54fccd4ff250f61d Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Tue, 15 Sep 2026 20:58:43 +0200 Subject: [PATCH 2/4] docs(proforge): correct token-accounting plan against post-#719/#759 main MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review wave on PR #727 (sourcery, CodeAnt, cubic, coderabbitai) converged on several real inaccuracies in the plan document, verified against current code: - "Import it in each of the six files" omitted baseAgent.ts, the seventh file from the plan's own Finding section — and baseAgent.ts isn't a minor extra: structuralAgent.ts/diagnosticAgent.ts both fold selfReflect()'s raw character count into their own tokensConsumed via `+= reflection.tokensUsed`, so skipping baseAgent.ts leaves a leak in two of the "fixed" six files. - The plan named services/aiProviderService.ts:357-372 for deliverAnthropicResponse; PR #759 (merged today) moved that function to services/ai/providers/anthropicProvider.ts:8-20 as part of its provider- adapter extraction. Updated the reference and re-verified the json.usage discard is still there at the new location. - The plan said the interface to extend was AIStreamCallbacks/generateText; the actual ProForge-facing boundary is GenerateResult (services/ai/inferenceGateway.ts), returned by InferenceGateway.generate() to BaseAgent. Named the full real chain instead (deliverAnthropicResponse's callback-only shape -> generateText's plain-string return -> GenerateResult -> both DefaultInferenceGateway and NodeInferenceGateway), and kept the "decide the exact shape with the user" framing for the still-genuinely-open part rather than picking one. - Clarified that each file's existing per-call `+=` accounting (primary call, reflection, retry, per-section loop) must stay additive when the source changes from response.length to usage?.outputTokens ?? estimateTokens(...) -- not collapse to one final usage value. - Flagged that importing estimateTokens from ragPromptAssembly.ts directly would drag browser-only Web Worker/WebGPU/DuckDB-WASM modules into the Node/MCP ProForge capability path; recommends extracting it into a new dependency-free module first. - Softened the companion-plan reference (docs/PROFORGE-CLAUDE-MAXTOKENS-CEILING-PLAN.md) to note it's tracked in parallel, not-yet-merged PR #728, rather than citing it as an existing file. --- docs/PROFORGE-CLAUDE-TOKEN-ACCOUNTING-PLAN.md | 78 ++++++++++++++++--- 1 file changed, 67 insertions(+), 11 deletions(-) diff --git a/docs/PROFORGE-CLAUDE-TOKEN-ACCOUNTING-PLAN.md b/docs/PROFORGE-CLAUDE-TOKEN-ACCOUNTING-PLAN.md index 1d0001c89..7458219cf 100644 --- a/docs/PROFORGE-CLAUDE-TOKEN-ACCOUNTING-PLAN.md +++ b/docs/PROFORGE-CLAUDE-TOKEN-ACCOUNTING-PLAN.md @@ -1,10 +1,12 @@ # Plan: real token accounting for the Claude path (ProForge) -Status: **planned, not yet implemented** — prep doc from a `/claude-api prompt-audit` pass (2026-09-12). Implement in this branch (`fix/proforge-token-accounting`) when work resumes. +Status: **planned, not yet implemented** — prep doc from a `/claude-api prompt-audit` pass (2026-09-12, +corrected 2026-09-15 against post-#719/#759 `main`). Implement in this branch +(`fix/proforge-token-accounting`) when work resumes. ## Finding -`ProForge` agents label a raw **character** count as "tokens": +`ProForge` agents label a raw **character** count as "tokens", across seven files: - `services/proForge/pipelineAgents/structuralAgent.ts:67` — `tokensConsumed += response.length;` - `services/proForge/pipelineAgents/diagnosticAgent.ts:72,91` — same pattern (initial call + retry) @@ -12,13 +14,24 @@ Status: **planned, not yet implemented** — prep doc from a `/claude-api prompt - `services/proForge/pipelineAgents/proofAgent.ts:51` - `services/proForge/pipelineAgents/proseAgent.ts:91` (per-section loop) - `services/proForge/pipelineAgents/copyEditAgent.ts:67` (per-section loop) -- `services/proForge/pipelineAgents/baseAgent.ts:213` — `selfReflect()`'s `tokensUsed: response.text.length` +- `services/proForge/pipelineAgents/baseAgent.ts:220` — `selfReflect()`'s `tokensUsed: response.text.length` -`services/aiProviderService.ts:362-369` (`deliverAnthropicResponse`) reads the full Anthropic response JSON and discards `json.usage` entirely — the real `usage.input_tokens`/`usage.output_tokens` (and thinking-token spend on Opus 5, which runs adaptive thinking by default) never reach the app. `AnalyticsAgent` has no AI call and needs no change. +`baseAgent.ts` is not a minor eighth concern — `structuralAgent.ts` and `diagnosticAgent.ts` both do +`tokensConsumed += reflection.tokensUsed;` after calling `selfReflect()`, so its raw character count +already leaks into two of the other six files' totals today. Fixing those six without also fixing +`baseAgent.ts` leaves that leak in place. + +`AnalyticsAgent` has no AI call and needs no change. + +`deliverAnthropicResponse` (`services/ai/providers/anthropicProvider.ts:8-20`, moved here from +`aiProviderService.ts` by PR #759's provider-adapter extraction) reads the full Anthropic response +JSON and discards `json.usage` entirely — the real `usage.input_tokens`/`usage.output_tokens` (and +thinking-token spend on Opus 5, which runs adaptive thinking by default) never reach the app. ## Small fix (low risk, do first) -Swap the character count for the existing token estimator already used elsewhere in this codebase (`services/ragPromptAssembly.ts:51-53`, `estimateTokens`): +The existing token estimator (`services/ragPromptAssembly.ts:52-54`, `estimateTokens`) is the right +formula, but importing it as-is is not safe for every caller of these seven files: ```ts export function estimateTokens(text: string): number { @@ -26,16 +39,59 @@ export function estimateTokens(text: string): number { } ``` -Import it in each of the six files above and replace `response.length` / `response.text.length` with `estimateTokens(response)` / `estimateTokens(response.text)`. Mechanical, no interface changes, no test breakage expected beyond any test asserting the old raw-length value. +`ragPromptAssembly.ts` transitively imports `services/ai/localEmbeddingService.ts` (Web Worker via +`workerBusManager`) and pulls in `services/localRagService.ts`'s WebGPU/DuckDB-WASM chain through +sibling module graphs — browser-only. ProForge agents also load under the Node-based MCP server +(`.mcp/proforge-mcp-server`), so importing `estimateTokens` straight from `ragPromptAssembly.ts` +risks breaking that path before an agent even runs. Extract `estimateTokens` into a new +dependency-free module (e.g. `services/tokenEstimate.ts`) first, have `ragPromptAssembly.ts` import +it from there instead of defining it locally, and have the seven agent files import from the new +module. + +Then, in each of the seven files above, replace every raw character count +(`response.length` / `retryRaw.length` / `response.text.length`) with the matching +`estimateTokens(...)` call. Mechanical, no interface changes, no test breakage expected beyond any +test asserting the old raw-length value. ## Larger fix (design decision needed, not a blind diff) -Thread Anthropic's real `usage` object back through the call chain instead of estimating: +Thread Anthropic's real `usage` object back through the call chain instead of estimating. The +ProForge-facing boundary is **not** `generateText`'s bare `Promise` — it's `GenerateResult` +(`services/ai/inferenceGateway.ts`), the type `InferenceGateway.generate()` actually returns to +`BaseAgent`/the pipeline agents: -1. `deliverAnthropicResponse` (`services/aiProviderService.ts:357-372`) already has `json.usage` available — capture `{ inputTokens, outputTokens }` instead of discarding it. -2. That requires extending `AIStreamCallbacks`/`generateText`'s return shape (currently just `Promise`) to optionally carry usage, or a side-channel the ProForge agents can read. This is an interface change affecting every provider path (only Anthropic can populate it for now; others stay `undefined`) — decide the shape with the user before implementing, don't force it through as a mechanical hunk. -3. Once available, `structuralAgent.ts` etc. should prefer real `usage.output_tokens` when present, falling back to `estimateTokens()` for providers that don't return it. +1. `deliverAnthropicResponse` (`services/ai/providers/anthropicProvider.ts:8-20`) already has + `json.usage` available — capture `{ inputTokens, outputTokens }` instead of discarding it. Its + only way to communicate today is `AIStreamCallbacks` (`onChunk`/`onDone`/`onError`), and + `streamAnthropic` itself returns `Promise` — so this needs a new optional + `onUsage?: (usage: { inputTokens: number; outputTokens: number }) => void` callback (or an + equivalent side-channel `generateText` can read after the stream completes). +2. `generateText` (`services/aiProviderService.ts:276-281`, `Promise`) needs a way to + surface that captured usage to its caller without breaking its existing plain-string callers. + This is an interface change affecting every provider path (only Anthropic can populate it for + now; Gemini/local/other providers stay `undefined`) — **decide the exact shape with the user + before implementing** (an overload, a second parallel function, or a mutable out-param are all + plausible; this doc intentionally does not pick one). +3. `DefaultInferenceGateway.generate()` (`services/ai/inferenceGateway.ts`) needs to capture that + usage and add an optional `usage?: { inputTokens: number; outputTokens: number }` field to + `GenerateResult`. `NodeInferenceGateway.generate()` (`services/proForge/adapters/nodeInferenceGateway.ts`, + Gemini-backed) always returns `usage: undefined` — Gemini's usage metadata is a separate, + unaudited follow-up, out of scope here. +4. Once available, each of the seven files' existing per-call accounting stays additive exactly as + it is today (`structuralAgent.ts`/`diagnosticAgent.ts` already do `tokensConsumed +=` once for + the primary call, once for `selfReflect()`, and again for a retry; `proseAgent.ts`/`copyEditAgent.ts` + already do it once per qualifying section) — only the *source* of each addend changes, from + `response.length` to `usage?.outputTokens ?? estimateTokens(response)` for that specific call. + Do not collapse a multi-call agent's total down to a single final `usage.output_tokens` value. +5. `tokensConsumed` today only ever accumulated an output-side estimate. Decide with the user + whether it should stay output-only (cheapest to implement, matches current semantics) or become + `inputTokens + outputTokens` (more honest cost signal, but a metric-contract change every + consumer of `tokensConsumed` needs to tolerate) before implementing — don't silently change what + the number means. ## Why this matters -Without real usage data, the `MAX_TOKENS_CEILING`/timeout tuning in the companion plan (`docs/PROFORGE-CLAUDE-MAXTOKENS-CEILING-PLAN.md`) can't be validated from measurement — right now nobody can tell whether the app's self-imposed ceilings are actually being hit. +Without real usage data, the `MAX_TOKENS_CEILING`/timeout tuning proposed in the companion plan +(tracked in parallel PR #728, `docs/PROFORGE-CLAUDE-MAXTOKENS-CEILING-PLAN.md`, not yet merged) +can't be validated from measurement — right now nobody can tell whether the app's self-imposed +ceilings are actually being hit. From 9df8537e01dc87fd835dc9c0bc0a80c77106e050 Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Tue, 15 Sep 2026 22:00:44 +0200 Subject: [PATCH 3/4] docs(proforge): fix resulting-wave findings on the token-accounting plan Fresh review wave after the previous correction push (graphite-app, cubic, coderabbitai), all verified real: - "eighth" -> "seventh": the doc lists seven files total; baseAgent.ts is the seventh, not an eighth item, and the prior wording contradicted the file's own "across seven files" opening line. - Extracting estimateTokens out of ragPromptAssembly.ts would remove the export tests/unit/ragPromptAssembly.test.ts and tests/unit/services/ragPromptAssembly.test.ts import directly today -- confirmed via grep. Added the re-export requirement. - baseAgent.ts's selfReflect() returns an object with a .text property, not a bare string -- its fallback needs estimateTokens(response.text), not estimateTokens(response) like the other six call sites. The generic wording would have miscounted or failed type checking if copied verbatim. --- docs/PROFORGE-CLAUDE-TOKEN-ACCOUNTING-PLAN.md | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/docs/PROFORGE-CLAUDE-TOKEN-ACCOUNTING-PLAN.md b/docs/PROFORGE-CLAUDE-TOKEN-ACCOUNTING-PLAN.md index 7458219cf..fb8c5c060 100644 --- a/docs/PROFORGE-CLAUDE-TOKEN-ACCOUNTING-PLAN.md +++ b/docs/PROFORGE-CLAUDE-TOKEN-ACCOUNTING-PLAN.md @@ -16,7 +16,7 @@ corrected 2026-09-15 against post-#719/#759 `main`). Implement in this branch - `services/proForge/pipelineAgents/copyEditAgent.ts:67` (per-section loop) - `services/proForge/pipelineAgents/baseAgent.ts:220` — `selfReflect()`'s `tokensUsed: response.text.length` -`baseAgent.ts` is not a minor eighth concern — `structuralAgent.ts` and `diagnosticAgent.ts` both do +`baseAgent.ts` is not a minor seventh concern — `structuralAgent.ts` and `diagnosticAgent.ts` both do `tokensConsumed += reflection.tokensUsed;` after calling `selfReflect()`, so its raw character count already leaks into two of the other six files' totals today. Fixing those six without also fixing `baseAgent.ts` leaves that leak in place. @@ -45,8 +45,10 @@ sibling module graphs — browser-only. ProForge agents also load under the Node (`.mcp/proforge-mcp-server`), so importing `estimateTokens` straight from `ragPromptAssembly.ts` risks breaking that path before an agent even runs. Extract `estimateTokens` into a new dependency-free module (e.g. `services/tokenEstimate.ts`) first, have `ragPromptAssembly.ts` import -it from there instead of defining it locally, and have the seven agent files import from the new -module. +it from there and re-export it (both `tests/unit/ragPromptAssembly.test.ts` and +`tests/unit/services/ragPromptAssembly.test.ts` import `estimateTokens` from +`ragPromptAssembly.ts` directly today — removing that export without a re-export breaks them), and +have the seven agent files import from the new module. Then, in each of the seven files above, replace every raw character count (`response.length` / `retryRaw.length` / `response.text.length`) with the matching @@ -80,9 +82,11 @@ ProForge-facing boundary is **not** `generateText`'s bare `Promise` — 4. Once available, each of the seven files' existing per-call accounting stays additive exactly as it is today (`structuralAgent.ts`/`diagnosticAgent.ts` already do `tokensConsumed +=` once for the primary call, once for `selfReflect()`, and again for a retry; `proseAgent.ts`/`copyEditAgent.ts` - already do it once per qualifying section) — only the *source* of each addend changes, from - `response.length` to `usage?.outputTokens ?? estimateTokens(response)` for that specific call. - Do not collapse a multi-call agent's total down to a single final `usage.output_tokens` value. + already do it once per qualifying section) — only the *source* of each addend changes: use + `usage?.outputTokens ?? estimateTokens(response)` for the six string-returning call sites, and + `usage?.outputTokens ?? estimateTokens(response.text)` in `baseAgent.ts`'s `selfReflect()`, + which returns an object, not a bare string. Do not collapse a multi-call agent's total down to a + single final `usage.output_tokens` value. 5. `tokensConsumed` today only ever accumulated an output-side estimate. Decide with the user whether it should stay output-only (cheapest to implement, matches current semantics) or become `inputTokens + outputTokens` (more honest cost signal, but a metric-contract change every From d87c7cc73f63554cc1728405067703185c30666a Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Tue, 15 Sep 2026 22:29:00 +0200 Subject: [PATCH 4/4] docs(proforge): clarify eight call sites (not six) in the token-accounting plan cubic-dev-ai caught a genuine undercount in the prior wording: structuralAgent.ts and diagnosticAgent.ts each have two response-producing call sites (the primary call's `response` and the retry's `retryRaw`), not one, so "six string-returning call sites" undercounted by two and didn't name which variable each site actually holds. Clarified to eight sites across six files, with the response/retryRaw distinction spelled out. --- docs/PROFORGE-CLAUDE-TOKEN-ACCOUNTING-PLAN.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/PROFORGE-CLAUDE-TOKEN-ACCOUNTING-PLAN.md b/docs/PROFORGE-CLAUDE-TOKEN-ACCOUNTING-PLAN.md index fb8c5c060..486c1e35b 100644 --- a/docs/PROFORGE-CLAUDE-TOKEN-ACCOUNTING-PLAN.md +++ b/docs/PROFORGE-CLAUDE-TOKEN-ACCOUNTING-PLAN.md @@ -83,10 +83,12 @@ ProForge-facing boundary is **not** `generateText`'s bare `Promise` — it is today (`structuralAgent.ts`/`diagnosticAgent.ts` already do `tokensConsumed +=` once for the primary call, once for `selfReflect()`, and again for a retry; `proseAgent.ts`/`copyEditAgent.ts` already do it once per qualifying section) — only the *source* of each addend changes: use - `usage?.outputTokens ?? estimateTokens(response)` for the six string-returning call sites, and - `usage?.outputTokens ?? estimateTokens(response.text)` in `baseAgent.ts`'s `selfReflect()`, - which returns an object, not a bare string. Do not collapse a multi-call agent's total down to a - single final `usage.output_tokens` value. + `usage?.outputTokens ?? estimateTokens(response)` for the **eight** string-returning call sites + across the six agent files (`structuralAgent.ts` and `diagnosticAgent.ts` each have two — the + primary call's `response` and the retry's `retryRaw` — so use whichever variable that specific + call site actually holds), and `usage?.outputTokens ?? estimateTokens(response.text)` in + `baseAgent.ts`'s `selfReflect()`, which returns an object, not a bare string. Do not collapse a + multi-call agent's total down to a single final `usage.output_tokens` value. 5. `tokensConsumed` today only ever accumulated an output-side estimate. Decide with the user whether it should stay output-only (cheapest to implement, matches current semantics) or become `inputTokens + outputTokens` (more honest cost signal, but a metric-contract change every