Skip to content

fix(mcp): wait through complete generation phase budgets - #2612

Merged
debpalash merged 3 commits into
mainfrom
fix/mcp-generation-phase-budgets
Oct 5, 2026
Merged

debpalash merged 3 commits into
mainfrom
fix/mcp-generation-phase-budgets

Conversation

@debpalash

@debpalash debpalash commented Oct 5, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Follow up fix(cpu): scale the default generation compute-time budget on CPU hosts #2611: MCP speech tools wait through cold model loading, reference transcription, queueing, sidecar grace and progress extensions
  • Preserve explicit MCP timeouts, standalone transcription behavior and the legacy progress-extension setting
  • Add serial-job, phase-budget, override and constant-parity regressions; clarify CPU budgets in performance docs

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

  • Refreshed against current main 28801c89, preserving the complete 0.5.7 release notes and owner changes
  • Fixed the serial reference-transcription gap found in review and restored Unreleased highlights
  • Independent standard-library validation: 18 lightweight committed timeout cases, six two-job parity cases using extracted real backend functions, and 5,184 backend/client budget comparisons passed
  • Fail-before proof: 11 updated timeout cases fail against the one-job implementation, including both new regression tests
  • Syntax, torch-free import, constant parity and changelog-style checks passed
  • Full hosted CI passed on exact head cde96096c53b0131b4daac91b2e97b684244b70e, including 10,051 main tests, 472 backend tests, 107 frontend tests, Electron suites and Linux/macOS/macOS Intel/Windows smokes
  • Security workflow, identity and CLA gates passed on that head
  • Both refreshed bot reviews report no outstanding findings; independent exact-head review found no remaining blocker
  • Full pytest was not run locally; hosted CI supplied the full-suite verification

Backend 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.

Preserve 0.5.7 release notes and keep the MCP timeout fix under Unreleased.
@debpalash
debpalash marked this pull request as ready for review October 5, 2026 17:34
@greptile-apps

greptile-apps Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Retrigger

[High risk] Changes timeout and budget logic for model generation.

The reviewed changes appear safe to merge; no outstanding finding remains.

Summary

MCP generation timeouts now allow for cold loading, reference transcription, queueing, and progress-extended synthesis while retaining explicit timeout overrides.

  • Regression tests cover phase budgets and backend constant parity.
  • Performance documentation and Unreleased notes reflect the changed wait.

Reviews (2) · Last reviewed commit: "fix(mcp): include serial reference trans..."

Comment thread backend/mcp_server.py
Comment thread CHANGELOG.md
@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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
  • Configuration used: Repository: debpalash/VoiceStudio/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: d3c8100b-1a6b-4457-98f1-29817c1e7064
📥 Commits

Reviewing files that changed from the base of the PR and between 1e58ee2 and cde9609.

📒 Files selected for processing (4)
  • CHANGELOG.md
  • backend/mcp_server.py
  • docs/performance.md
  • tests/test_mcp_timeouts_2040.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • CHANGELOG.md
  • docs/performance.md

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The 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.

Changes

MCP generation timeout budget

Layer / File(s) Summary
Generation wait budget
backend/mcp_server.py, tests/test_generate_abort_budget.py, tests/test_mcp_timeouts_2040.py, docs/performance.md, CHANGELOG.md
The MCP generation wait calculation now includes a 900-second minimum base, model-load and queue budgets, execution time, reference-transcription allowance, and progress extensions. Tests cover these budget components, generation and transcription timeout separation, and explicit MCP timeout precedence. The performance guide describes CPU scaling and client wait allowances. The changelog records the timeout fix.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: paoloantinori

Merge Risk: ⚪ Minimal · up to cde96

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 Review

Security architecture risk: 🔵 Low · up to cde96

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
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The incremental exposure is longer pending-call occupancy for existing speech-generation and voice-design callers, especially over remote HTTP. Caller text can influence the estimate, but no new route, backend authority or attackable asset class was identified. The effect of concurrent calls remains deployment-dependent.

Trust Boundaries and Controls

  • observed — The existing in-process loopback path and remote backend-authentication header path are unchanged. Speech-profile resolution still occurs before submission. The timeout calculation neither replaces these controls nor changes the request destination.

Resilience and Maintainability Implications

  • observed — Backend queue cancellation, execution abandonment and pool-reset behavior remain owned by the existing guards. The classic route retains distinct saturation and execution-timeout responses, does not silently rerun failed remote generation, and finishes temporary-reference ownership in its finally block.

Hardening Proposals

  • proposed — For network-exposed installations, validate concurrent tool-call containment and disconnect behavior against the longer wait envelope. Where necessary, use existing deployment admission limits or the explicit client timeout. This is a deployment-validation proposal, not an observed vulnerability.
🚥 Pre-merge checks | ✅ 8 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning 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: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (8 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cross-Platform Default Parity ✅ Passed The PR adds no platform-divergent default. backend/mcp_server.py computes the generation wait from the same environment defaults and shared, platform-neutral `core.generate_budget.client_execution_b…
I18n Completeness (21 Locales) ✅ Passed The PR changes only CHANGELOG.md, backend/mcp_server.py, docs/performance.md, and two test files. No Electron UI code changed, so the PR adds or changes no Electron t(...) keys or hardcoded Electron U…
Local-First Guarantee ✅ Passed The PR adds no new outbound calls, cloud dependencies, accounts, or API keys. Its only runtime change is MCP generation-timeout budgeting in backend/mcp_server.py; the other changes are documentatio…
Backward Compatibility ✅ Passed No backward-compatibility failure is introduced. The PR changes only the changelog, performance documentation, timeout budgeting in backend/mcp_server.py, and timeout tests. The implementation exten…
Title check ✅ Passed The title follows the required conventional-commit format with the mcp scope and describes the timeout change. The description includes issue reference #2611.
Description check ✅ Passed The description explains the change and provides detailed testing and verification information. It does not include the template’s Type, Checklist, or Release cadence sections.
Full details: Docstring Coverage

Explanation

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.)

  • Fix all pre-merge checks with AI
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Cover each guarded job's queue and progress-extension allowance, add two-job regressions, and restore Unreleased highlights.
@debpalash
debpalash merged commit 286f46b into main Oct 5, 2026
26 checks passed
@debpalash
debpalash deleted the fix/mcp-generation-phase-budgets branch October 5, 2026 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant