fix(mcp): wait through complete generation phase budgets - #2612
Conversation
Preserve 0.5.7 release notes and keep the MCP timeout fix under Unreleased.
|
[High risk] Changes timeout and budget logic for model generation. The reviewed changes appear safe to merge; no outstanding finding remains. SummaryMCP generation timeouts now allow for cold loading, reference transcription, queueing, and progress-extended synthesis while retaining explicit timeout overrides.
Reviews (2) · Last reviewed commit: "fix(mcp): include serial reference trans..." |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe MCP generation timeout now includes model loading, queueing, execution, reference transcription, and progress-extension allowances. Tests and performance documentation cover the updated timeout budget and CPU-budget behavior. ChangesMCP generation timeout budget
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to MCP speech tools now wait through model loading, queueing and progress extensions before timing out. No concrete merge-blocking risk remains in the supplied context. As the author notes, the branch should be refreshed against current main before landing. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to Speech requests can wait longer, but their processing limits and access controls are unchanged. No new privilege path was found. Concurrent-request limits for network deployments remain unverified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 8 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (8 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 10.53% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 19 functions across 3 files. (2 skipped: 2 unsupported.)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Cover each guarded job's queue and progress-extension allowance, add two-job regressions, and restore Unreleased highlights.
Summary
Evidence
The previous MCP wait was 9,030 seconds for a 2,000-character request. A transcript-less CPU clone can use separate reference-recognition and synthesis jobs, totaling up to 36,000 seconds under default backend guards. The MCP backstop now allows 36,050 seconds; the backend still returns as soon as generation finishes or its own watchdog stops it.
Review and verification
28801c89, preserving the complete 0.5.7 release notes and owner changescde96096c53b0131b4daac91b2e97b684244b70e, including 10,051 main tests, 472 backend tests, 107 frontend tests, Electron suites and Linux/macOS/macOS Intel/Windows smokesBackend compute allowances, permissions, dependencies, workflows and versions are unchanged.
MCP generation waits now include model loading, queue time, execution budget, sidecar grace, and progress extensions; explicit timeouts and transcription waits remain unchanged. This prevents long or cold CPU renders from timing out before generation completes. Refreshed against current main, including a separate reference-transcription allowance; final merge gates passed.