-
Notifications
You must be signed in to change notification settings - Fork 7
docs(proforge): plan real token accounting for the Claude path #727
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -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. | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue: The mechanical-fix instructions omit Triggers: When the next session follows the “six files” instruction literally. Suggested fix: List all seven affected files, or explicitly include
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: “Import it in each of the six files above” omits the seventh Assessment: 🟠 Prompt for AI Agent 🤖This is a comment left during a code review.
**Path:** docs/PROFORGE-CLAUDE-TOKEN-ACCOUNTING-PLAN.md
**Line:** 29:29
**Comment:**
*Incomplete Implementation: “Import it in each of the six files above” omits the seventh `baseAgent.ts` target and the structural retry count, so those paths remain character-counted.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fixThere was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: Following this instruction leaves Prompt for AI agentsThere was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win Include The finding lists List all seven affected files, or document and test the reason for excluding Proposed plan correction-Import it in each of the six files above and replace `response.length` / `response.text.length` with `estimateTokens(response)` / `estimateTokens(response.text)`.
+Import it in each of the seven listed files, including `baseAgent.ts`, and replace each raw character count with the appropriate `estimateTokens(...)` call.📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||
|
|
||||||||||
| ## 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<string>`) 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. | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: The plan proposes changing Assessment: 🔴 Prompt for AI Agent 🤖This is a comment left during a code review.
**Path:** docs/PROFORGE-CLAUDE-TOKEN-ACCOUNTING-PLAN.md
**Line:** 36:36
**Comment:**
*Api Mismatch: The plan proposes changing `generateText`, but ProForge consumes `InferenceGateway.generate()`'s `GenerateResult`; implementing only this documented change breaks the gateway and existing string callers.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fixThere was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: Usage will be lost at the actual ProForge boundary if only Prompt for AI agents |
||||||||||
| 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. | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: This recommendation still undercounts prompt tokens because Prompt for AI agentsThere was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- plan ---'
cat -n docs/PROFORGE-CLAUDE-TOKEN-ACCOUNTING-PLAN.md | sed -n '1,55p'
printf '%s\n' '--- candidate files ---'
fd -t f -E node_modules | rg '(^|/)(diagnosticAgent|proseAgent|copyEditAgent|structuralAgent|baseAgent)\.(ts|tsx|js|jsx)$|estimateTokens|token'
printf '%s\n' '--- relevant symbols and usage ---'
rg -n -C 3 'output_tokens|estimateTokens|diagnosticAgent|proseAgent|copyEditAgent|structuralAgent|retry|section' --glob '*.{ts,tsx,js,jsx}' .Repository: qnbs/WorldScript-Studio Length of output: 50380 🤖 get_repo_knowledge executed:
Length of output: 43498 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- plan ---'
cat -n docs/PROFORGE-CLAUDE-TOKEN-ACCOUNTING-PLAN.md | sed -n '1,50p'
printf '%s\n' '--- ProForge files ---'
git ls-files | rg -i '(^|/)(diagnosticAgent|proseAgent|copyEditAgent|structuralAgent|baseAgent|estimateTokens)|proforge'
printf '%s\n' '--- accounting references in ProForge ---'
rg -n -C 4 --glob '*.{ts,tsx}' 'output_tokens|estimateTokens|usage' services features tests/unit/proForge 2>/dev/null | head -n 500Repository: qnbs/WorldScript-Studio Length of output: 40304 🏁 Script executed: #!/bin/bash
set -e
for f in \
services/proForge/pipelineAgents/baseAgent.ts \
services/proForge/pipelineAgents/diagnosticAgent.ts \
services/proForge/pipelineAgents/proseAgent.ts \
services/proForge/pipelineAgents/copyEditAgent.ts \
services/proForge/pipelineAgents/structuralAgent.ts \
services/proForge/pipelineAgents/agentRegistry.ts \
services/proForge/pipelineAgents/diagnosticAgent.test.ts \
tests/unit/proForge/pipelineAgents/proseAgent.test.ts \
tests/unit/proForge/pipelineAgents/copyEditAgent.test.ts
do
echo "--- $f ---"
wc -l "$f"
done
printf '%s\n' '--- baseAgent relevant range ---'
sed -n '1,280p' services/proForge/pipelineAgents/baseAgent.ts
printf '%s\n' '--- diagnosticAgent ---'
cat -n services/proForge/pipelineAgents/diagnosticAgent.ts
printf '%s\n' '--- proseAgent ---'
cat -n services/proForge/pipelineAgents/proseAgent.ts
printf '%s\n' '--- copyEditAgent ---'
cat -n services/proForge/pipelineAgents/copyEditAgent.ts
printf '%s\n' '--- structuralAgent ---'
cat -n services/proForge/pipelineAgents/structuralAgent.tsRepository: qnbs/WorldScript-Studio Length of output: 979 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- baseAgent ---'
cat -n services/proForge/pipelineAgents/baseAgent.ts | sed -n '1,225p'
printf '%s\n' '--- diagnosticAgent ---'
cat -n services/proForge/pipelineAgents/diagnosticAgent.ts | sed -n '1,225p'
printf '%s\n' '--- proseAgent accounting ranges ---'
cat -n services/proForge/pipelineAgents/proseAgent.ts | sed -n '1,125p'
printf '%s\n' '--- copyEditAgent accounting ranges ---'
cat -n services/proForge/pipelineAgents/copyEditAgent.ts | sed -n '1,105p'
printf '%s\n' '--- structuralAgent accounting ranges ---'
cat -n services/proForge/pipelineAgents/structuralAgent.ts | sed -n '1,85p'
printf '%s\n' '--- agent test paths ---'
git ls-files 'tests/unit/proForge/pipelineAgents/*Agent.test.ts' | sortRepository: qnbs/WorldScript-Studio Length of output: 33850 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- gateway contract and implementations ---'
rg -n -C 8 'interface InferenceGateway|type InferenceGateway|generate\(' services/ai/inferenceGateway.ts services/ai services/proForge/adapters --glob '*.ts' | head -n 350
printf '%s\n' '--- agent metric returns ---'
rg -n -C 5 'tokensConsumed|tokensUsed|metrics:' services/proForge/pipelineAgents/{diagnosticAgent,structuralAgent,proseAgent,copyEditAgent}.ts
printf '%s\n' '--- relevant tests ---'
rg -n -C 5 'tokensConsumed|tokensUsed|aiCalls|retry|sections|metrics' tests/unit/proForge/pipelineAgents/{diagnosticAgent,structuralAgent,proseAgent,copyEditAgent}.test.ts | head -n 500Repository: qnbs/WorldScript-Studio Length of output: 50379 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- inference gateway types and default generate ---'
cat -n services/ai/inferenceGateway.ts | sed -n '1,135p'
printf '%s\n' '--- node gateway generate result ---'
cat -n services/proForge/adapters/nodeInferenceGateway.ts | sed -n '50,115p'
printf '%s\n' '--- browser gateway generate result ---'
cat -n services/proForge/adapters/browserProForgeCapability.ts | sed -n '1,150p'
printf '%s\n' '--- test gateway result helper and base generate ---'
rg -n -C 5 'function gatewayResult|const gatewayResult|mockGenerate|publicGenerate|GenerateResult' tests/unit/proForge/pipelineAgents/{baseAgent,diagnosticAgent,proseAgent,copyEditAgent}.test.tsRepository: qnbs/WorldScript-Studio Length of output: 50381 Define additive usage accounting for every provider call.
Proposed plan correction-Once available, `structuralAgent.ts` etc. should prefer real `usage.output_tokens` when present, falling back to `estimateTokens()` for providers that don't return it.
+Once available, each provider call should add its `usage.output_tokens` when present, falling back to `estimateTokens()` for that call when usage is unavailable. Sum all calls, including reflection, retries, and per-section loops.📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||
|
|
||||||||||
| ## 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. | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nitpick: The plan links to Suggested fix: Add the companion document or correct the reference to the file that contains the MAX_TOKENS_CEILING and timeout plan. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: The validation section points to a companion plan that is absent, so readers cannot inspect the ceiling and timeout assumptions. Add that document or correct this reference. Prompt for AI agents |
||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: When the Node/MCP capability loads a ProForge agent, this import also loads browser-only RAG modules, so the planned mechanical fix can fail before the agent runs. Extract
estimateTokensinto a dependency-free module and import that instead.Prompt for AI agents