docs(proforge): plan Claude max_tokens/timeout right-sizing - #728
Merged
Merged
Conversation
🤖 CodeAnt AI — Review Status
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
Reviewer's GuideAdds a prep-only implementation plan for right-sizing Claude token ceilings and timeouts in the ProForge web path, while explicitly deferring end-to-end SSE streaming to a separately designed follow-up. No production code or tests are changed. Sequence diagram for current non-streaming Claude response pathsequenceDiagram
participant Agent as ProForge agent
participant Service as aiProviderService
participant Relay as Claude proxy edge function
participant Claude as Claude API
Agent->>Service: deliverAnthropicResponse
Service->>Relay: fetch request
Relay->>Claude: fetch request
Claude-->>Relay: complete response body
Relay-->>Service: one text response
Service-->>Agent: callbacks.onChunk
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Review limit reachedNext included review available in 50 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 71 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (1)
Comment |
🏁 CodeAnt Quality Gate ResultsCommit: ✅ Overall Status: PASSEDQuality Gate Details
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
21 tasks
CodeAnt Nitpicks1 code suggestion1. This describes proof and publishing as whole-manuscript workloads, but proof truncates input to 12,000 characters and publishing sends only two 500-character excerpts.Comment mismatch · |
There was a problem hiding this comment.
All reported issues were addressed across 1 file
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Owner
Author
Execution owner / issue linkageThis prep-only PR is the planning/evidence artifact for #731 — Canonical relationship: #728 captures the current 4K/8K/20s capacity findings and an interim right-sizing candidate, while #731 owns the full implementation contract including host-limit requalification and real SSE streaming. The candidate Sibling audit tracks:
This PR does not close #731. |
2 tasks
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc49c3c63f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
32 tasks
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
qnbs
added a commit
that referenced
this pull request
Sep 15, 2026
…main 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.
…main Review wave on PR #728 (cubic, chatgpt-codex-connector) converged on several real inaccuracies, verified against current code: - Most significant: raising the four hardcoded 4000-token per-call caps does nothing for a default user. Math.min(config.maxTokens, N) is bounded by the smaller value, and the effective config.maxTokens for a real ProForge run is settings.advancedAi.maxTokens (features/settings/settingsSlice.ts:73 seeds it at 4096), not DEFAULT_PIPELINE_CONFIG.maxTokens (8000) as an earlier pass of this plan assumed -- Redux state is never undefined once the slice initializes, so useProForgeOrchestrator.ts's "?? 8000" fallback never fires in practice. Math.min(4096, 16000) is still 4096: a 96-token increase over today's 4000, not the intended one. The plan now says Part A must also raise the effective default budget, not just the four literals. - publishingAgent.ts/proofAgent.ts were described as "whole-manuscript scope"; proofAgent.ts:34 truncates to 12,000 characters and publishingAgent.ts:31-32 sends two 500-character excerpts. Corrected the input-side framing while keeping the higher-cap rationale (heavier output relative to that truncated/excerpted input). - deliverAnthropicResponse moved from aiProviderService.ts to services/ai/providers/anthropicProvider.ts by PR #759 (merged since this plan was written) -- same staleness already caught and fixed on PR #727's companion plan. Updated the reference. - BaseAgent.buildAiOpts() forwards maxTokens identically regardless of provider -- raising the four agent caps affects Gemini/OpenAI/Grok/Ollama too, not just Claude. Flagged as an open scoping decision. - The 20s OUTBOUND_TIMEOUT_MS is explicitly documented in docs/SECURITY-THREAT-MODEL.md as part of the CWE-400 abuse-control bundle for this public, unauthenticated endpoint. Raising it to 55s needs an explicit resource-exhaustion re-assessment and a threat-model doc update, not a five-file mechanical diff -- added to the Part A scope and the implementation checklist. - Part B needed both upstream paths named: the web proxy AND the Tauri desktop path (services/ai/providers/anthropicProvider.ts:37-43) both omit stream: true today; fixing only one would leave the other's response parsing broken by a mismatched assumption. - Corrected a genuinely false claim from an earlier pass: Grok does not fake streaming. streamGrok() (services/ai/providers/openaiProvider.ts:195-224) sends stream: true and uses a real SSE reader loop (consumeOpenAiCompatibleStream). Verified against current code before accepting a codex-connector finding that asserted the opposite. - Strengthened the implementation checklist per codex-connector's finding: ci:prepush and focused unit coverage for the proxy bounds/timeout and agent token forwarding, not just typecheck/lint/live-key sanity check, per AGENTS.md's own verification bar for a non-trivial behavior change.
qnbs
force-pushed
the
fix/proforge-anthropic-maxtokens-ceiling
branch
from
September 15, 2026 19:12
bc49c3c to
0362970
Compare
There was a problem hiding this comment.
No application code in the PR — skipped Code Health checks.
See analysis details in CodeScene
Quality Gate Profile: The Bare Minimum
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
qnbs
added a commit
that referenced
this pull request
Sep 15, 2026
…main Review wave on PR #730 (CodeAnt, cubic, chatgpt-codex-connector) converged on several real issues, verified against current code: - Eight generate() invocations, not six: DiagnosticAgent and StructuralAgent each retry once on an incoherent self-reflection (diagnosticAgent.ts:89, structuralAgent.ts:82). Both retry call sites now explicitly listed in the plumbing step, alongside the six initial calls. - Stale references from PR #759's provider-adapter extraction (merged since this plan was written), same class of staleness already fixed on PR #727 and #728's companion plans: deliverAnthropicResponse and streamAnthropic's two branches moved from aiProviderService.ts to services/ai/providers/anthropicProvider.ts. Updated every reference and the desktop/proxy branch line numbers. - The "other two audit follow-ups" are PR #727 and PR #728 -- both real, neither merged yet at review time, which is exactly what made the reference look dangling. Named the PRs explicitly instead of just the file paths. - Real architectural gap in option (b) (the @anthropic-ai/sdk path): the Tauri-desktop branch in anthropicProvider.ts sends requests straight to api.anthropic.com from the client, bypassing the edge function entirely. A server-only SDK/converter as (b) originally proposed has no way to produce the sanitized schema for that path. Added this as a concrete reason favoring option (a), the local sanitizer, unless someone wants to also solve client-side SDK bundling. - Silently stripping unsupported Zod bounds (min/max/length) makes the wire schema weaker than the Zod validator still guarding the response -- several bounds (publishing blurb lengths, prose score ranges) encode real semantics not otherwise stated in the prompts. Added a requirement to fold every stripped bound into a description or prompt-level instruction rather than just deleting it, and extended the sanitizer's verification script to assert this. - npx -> pnpm exec tsx, matching this repo's pinned dependency-execution convention (AGENTS.md). - Added committed, focused regression coverage to the verification plan (extending tests/unit/aiProviderService.test.ts and tests/unit/api/claudeProxyCore.test.ts) for output_config.format forwarding on both branches -- not just a throwaway script and manual live-key smoke test, per AGENTS.md's verification bar for a non-trivial network-request behavior change.
qnbs
added a commit
that referenced
this pull request
Sep 15, 2026
* docs(proforge): plan real token accounting for the Claude path * docs(proforge): correct token-accounting plan against post-#719/#759 main 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): 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): 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.
qnbs
added a commit
that referenced
this pull request
Sep 15, 2026
* docs(proforge): plan Claude structured-outputs wiring * docs(proforge): correct structured-outputs plan against post-#719/#759 main Review wave on PR #730 (CodeAnt, cubic, chatgpt-codex-connector) converged on several real issues, verified against current code: - Eight generate() invocations, not six: DiagnosticAgent and StructuralAgent each retry once on an incoherent self-reflection (diagnosticAgent.ts:89, structuralAgent.ts:82). Both retry call sites now explicitly listed in the plumbing step, alongside the six initial calls. - Stale references from PR #759's provider-adapter extraction (merged since this plan was written), same class of staleness already fixed on PR #727 and #728's companion plans: deliverAnthropicResponse and streamAnthropic's two branches moved from aiProviderService.ts to services/ai/providers/anthropicProvider.ts. Updated every reference and the desktop/proxy branch line numbers. - The "other two audit follow-ups" are PR #727 and PR #728 -- both real, neither merged yet at review time, which is exactly what made the reference look dangling. Named the PRs explicitly instead of just the file paths. - Real architectural gap in option (b) (the @anthropic-ai/sdk path): the Tauri-desktop branch in anthropicProvider.ts sends requests straight to api.anthropic.com from the client, bypassing the edge function entirely. A server-only SDK/converter as (b) originally proposed has no way to produce the sanitized schema for that path. Added this as a concrete reason favoring option (a), the local sanitizer, unless someone wants to also solve client-side SDK bundling. - Silently stripping unsupported Zod bounds (min/max/length) makes the wire schema weaker than the Zod validator still guarding the response -- several bounds (publishing blurb lengths, prose score ranges) encode real semantics not otherwise stated in the prompts. Added a requirement to fold every stripped bound into a description or prompt-level instruction rather than just deleting it, and extended the sanitizer's verification script to assert this. - npx -> pnpm exec tsx, matching this repo's pinned dependency-execution convention (AGENTS.md). - Added committed, focused regression coverage to the verification plan (extending tests/unit/aiProviderService.test.ts and tests/unit/api/claudeProxyCore.test.ts) for output_config.format forwarding on both branches -- not just a throwaway script and manual live-key smoke test, per AGENTS.md's verification bar for a non-trivial network-request behavior change. * docs(proforge): reference PR #730 in CHANGELOG.md [Unreleased] This PR's title is governed (feat(...)), so scripts/check-pr-changelog-reference.mjs requires a real bullet citing "PR #730" before merge — same rule this session has already hit and fixed on other PRs this cycle. Added under the existing Documentation section, framed honestly as a not-yet-implemented planning doc.
This was referenced Sep 15, 2026
test(feature-flags): establish exhaustive coverage authority and prevent opt-in E2E blind spots
#709
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Summary
/claude-api prompt-auditpass — captures a concrete finding and fix plan so next week's session can implement directly instead of re-auditing.api/_shared/claudeProxyCore.ts) and 4 ProForge agents cap Claude output at 4-8K tokens with a 20s non-streaming timeout — sized for an older/smaller model than the ones this app's own catalog now targets (claude-opus-5/claude-sonnet-5, which support up to 128K output but need streaming to use it, and where Opus 5 runs adaptive thinking on by default).docs/PROFORGE-CLAUDE-MAXTOKENS-CEILING-PLAN.md— exact file:line targets, a low-risk mechanical fix (raise the ceiling/timeout constants), and a flagged larger follow-up (real SSE streaming through the edge function) that needs its own design pass.Test plan
pnpm run typecheck+pnpm run lint, then this PR is ready to merge.Summary by Sourcery
Document the plan to right-size Claude response limits and clarify the follow-up work required for reliable large-output streaming.
Enhancements:
Documentation:
CodeAnt-AI Description
Document a corrected plan for handling larger Claude responses in ProForge
What Changed
Impact
✅ Clearer plan for larger ProForge outputs✅ Fewer unexpected Claude timeouts after implementation✅ Explicit security review for longer public relay requests💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.