fix(review): reserve reasoning output and report balance failures [skip ci]

Refs #7067: Low-risk review passes previously capped reasoning and visible
text together at 4,000 output tokens. Raise the tier allowance to 8,192 for
reasoning/unknown routes, retaining the route or explicit output ceiling.
Use the same floor and normalized bounded reasoning effort in ReviewTool;
share incomplete-output wording with the CLI and restore the existing
accept_single_review documentation to its function. This is the intermediate
review repair; removing caps and implementing skip-before-start money control
remain planned Review v2 work.

Refs #7055: preserve balance HTTP/parse failures through Result and show
them in explicit /balance output. Distinguish timeouts and connection errors
without exposing the request URL. Automatic chip refresh keeps its previous
value. The DeepSeek URL fix predates this slice; real-key proof remains open.
Update both changelogs. Based on the stopped issue drafter batch 3, including
its reviewed effort normalization; transport classification fixes review m1.

Verified on a0053edcda plus this seven-file diff:
- Hermetic dev-cargo nextest, codewhale-tui --lib, focused review/triage/core,
  reserve and balance filters: 74 tests run, 74 passed, 15,228 skipped.
- All 2,180 recorded source hashes were unchanged during compilation.
- Single-file Rust formatting and lib.rs stdin comparison passed;
  git diff --check and sync-changelog.sh --check passed.
- Linker emitted its existing-size warning: __eh_frame exceeds the 16 MB
  compact-unwind limit; compilation and all selected tests completed.

No new tests/comments, provider call, CI dispatch, release or deployment.
These existing checks do not exercise a live reasoning route or key.
Full local receipt: CW/artifacts/codex-0103-takeover-20261025/batch3-result.json.

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Co-Authored-By: OpenAI Codex <[email protected]>

CHANGELOG.md

@@ -48,11 +48,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   unchanged (#6436, #7082).
 - `codewhale review` no longer accepts leaked tool-call markup as a review: a
   pass whose reply is a DeepSeek DSML (or other) tool-call wrapper outside code
-  now fails as incomplete, with no tally and nothing posted (#7072). A Low-risk
-  review on a reasoning model caps its reasoning at low so the 4,000-token
-  allowance leaves room for the review, and an exhausted pass now reports the
-  allowance and reserve actually sent and says "cut off" when visible text came
-  back (#7067).
+  now fails as incomplete, with no tally and nothing posted (#7072). A review on a
+  reasoning model now raises its tier output allowance to 8,192 tokens (a
+  Low-risk review used to get 4,000, which a reasoning pass could spend entirely
+  on thinking), the `review` tool bounds its reasoning level to leave room for
+  the review, and an exhausted pass reports the allowance and reserve actually
+  sent and says "cut off" when visible text came back (#7067).
 - `--full-access` is now the documented flag for Full Access (`--yolo` stays an
   alias), and `serve` accepts it after the subcommand: `codewhale serve --acp
   --yolo` used to fail with "unexpected argument". `serve --acp` also writes one
@@ -83,7 +84,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `/balance` and the balance status chip work on the default DeepSeek route
   again. The balance request now goes to the DeepSeek API root instead of the
   `/beta` inference path, which failed with "Could not fetch DeepSeek account
-  balance" (#7055).
+  balance". A failed balance request now names its reason, such as HTTP 401
+  Unauthorized (#7055).
 - An `AGENTS.md` or global instruction file over 100 KiB is no longer
   skipped (which could let a sibling file load in its place): Codewhale loads
   its first 100 KiB, ends it with a marker giving the bytes omitted, and names

crates/tui/CHANGELOG.md

@@ -48,11 +48,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   unchanged (#6436, #7082).
 - `codewhale review` no longer accepts leaked tool-call markup as a review: a
   pass whose reply is a DeepSeek DSML (or other) tool-call wrapper outside code
-  now fails as incomplete, with no tally and nothing posted (#7072). A Low-risk
-  review on a reasoning model caps its reasoning at low so the 4,000-token
-  allowance leaves room for the review, and an exhausted pass now reports the
-  allowance and reserve actually sent and says "cut off" when visible text came
-  back (#7067).
+  now fails as incomplete, with no tally and nothing posted (#7072). A review on a
+  reasoning model now raises its tier output allowance to 8,192 tokens (a
+  Low-risk review used to get 4,000, which a reasoning pass could spend entirely
+  on thinking), the `review` tool bounds its reasoning level to leave room for
+  the review, and an exhausted pass reports the allowance and reserve actually
+  sent and says "cut off" when visible text came back (#7067).
 - `--full-access` is now the documented flag for Full Access (`--yolo` stays an
   alias), and `serve` accepts it after the subcommand: `codewhale serve --acp
   --yolo` used to fail with "unexpected argument". `serve --acp` also writes one
@@ -83,7 +84,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `/balance` and the balance status chip work on the default DeepSeek route
   again. The balance request now goes to the DeepSeek API root instead of the
   `/beta` inference path, which failed with "Could not fetch DeepSeek account
-  balance" (#7055).
+  balance". A failed balance request now names its reason, such as HTTP 401
+  Unauthorized (#7055).
 - An `AGENTS.md` or global instruction file over 100 KiB is no longer
   skipped (which could let a sibling file load in its place): Codewhale loads
   its first 100 KiB, ends it with a marker giving the bytes omitted, and names

crates/tui/src/lib.rs

@@ -10490,18 +10490,20 @@ async fn run_review(config: &Config, args: ReviewArgs) -> Result<()> {
         }),
         _ => None,
     };
-    let review_budget = crate::tools::review_triage::budget_for(&review_triage).with_money_cap(
-        args.money_cap_usd,
-        review_rates.map(|rates| (rates.input_usd_per_million, rates.output_usd_per_million)),
-    );
     // Visible-text reserve for this exact route/model (#6285). A reasoning route
     // shares one `max_tokens` allowance between hidden reasoning and visible
     // text, so the review request has to keep room for the review itself.
+    let review_reserve_percent =
+        crate::route_budget::review_visible_text_reserve_percent(&request_route.model);
+    let review_budget = crate::tools::review_triage::budget_for(&review_triage)
+        .with_reasoning_floor(review_reserve_percent)
+        .with_money_cap(
+            args.money_cap_usd,
+            review_rates.map(|rates| (rates.input_usd_per_million, rates.output_usd_per_million)),
+        );
     let review_allowance = client
         .effective_max_output_tokens(&request_route.model)
         .min(review_budget.max_output_tokens);
-    let review_reserve_percent =
-        crate::route_budget::review_visible_text_reserve_percent(&request_route.model);
     let review_reserve_tokens = crate::route_budget::review_visible_text_reserve_tokens(
         &request_route.model,
         review_allowance,
@@ -10578,19 +10580,11 @@ async fn run_review(config: &Config, args: ReviewArgs) -> Result<()> {
             // #6285: budget exhaustion is an infrastructure outcome, not a
             // review verdict. The PR was never judged, so the message must not
             // read as findings and must name what was and was not reviewed.
-            let reasoning_tokens = usage
+            let reasoning_tokens = result
+                .usage
                 .reasoning_tokens
                 .map_or_else(|| "unknown".to_string(), |tokens| tokens.to_string());
-            let visible_tokens = usage
-                .output_tokens
-                .saturating_sub(usage.reasoning_tokens.unwrap_or(0));
-            let outcome_clause = if visible_tokens > 0 {
-                format!(
-                    "was cut off before the review finished ({visible_tokens} visible tokens came back)"
-                )
-            } else {
-                "produced no review text".to_string()
-            };
+            let outcome_clause = crate::tools::review::exhausted_pass_clause(&result.usage);
             let reserve_clause = if review_reserve_percent == 0 {
                 "no reasoning reserve was needed for this model".to_string()
             } else {

crates/tui/src/tools/review.rs

@@ -88,7 +88,7 @@ pub(crate) fn bounded_review_reasoning_effort(
         // Half the allowance must survive as text.
         _ => 2,
     };
-    if reserve_percent > 0 && allowance <= 8_192 {
+    if reserve_percent > 0 && allowance <= super::review_triage::REASONING_ROUTE_MIN_OUTPUT_TOKENS {
         ceiling = ceiling.min(2);
     }
     reasoning_effort_from_rank(reasoning_effort_rank(requested).min(ceiling))
@@ -1574,7 +1574,29 @@ impl ToolSpec for ReviewTool {
             _ => super::review_core::repository_tool_workspace(&context.workspace, None),
         };
         let triage = super::review_triage::triage_review(&review_diff);
-        let budget = super::review_triage::budget_for(&triage);
+        let reserve_percent =
+            crate::route_budget::review_visible_text_reserve_percent(&route.model);
+        let budget =
+            super::review_triage::budget_for(&triage).with_reasoning_floor(reserve_percent);
+        let allowance = client
+            .effective_max_output_tokens(&route.model)
+            .min(budget.max_output_tokens);
+        let reasoning_effort =
+            (codewhale_models::model_reasoning_capability(&route.model) == Some(true)).then(|| {
+                bounded_review_reasoning_effort(
+                    crate::reasoning_preference::ReasoningEffort::Max,
+                    reserve_percent,
+                    allowance,
+                )
+                .normalize_for_route_identity(
+                    client.api_provider(),
+                    client.admitted_provider_identity().key.as_str(),
+                    client.base_url(),
+                    &route.model,
+                )
+                .as_setting()
+                .to_string()
+            });
         let mut usage = Usage::default();
         let mut accumulator = plan.as_ref().map(PrReviewAccumulator::new);
         let mut single_output = None;
@@ -1590,11 +1612,8 @@ impl ToolSpec for ReviewTool {
             let mut request = super::review_core::ReviewCoreRequest::new(&review_diff, prompt);
             request.workspace = tool_workspace.clone();
             request.budget = budget.tool_budget();
-            request.max_output_tokens = Some(
-                client
-                    .effective_max_output_tokens(&route.model)
-                    .min(budget.max_output_tokens),
-            );
+            request.max_output_tokens = Some(allowance);
+            request.reasoning_effort = reasoning_effort.clone();
             let result =
                 match super::review_core::run_review_core(&client, &self.model, request).await {
                     Ok(result) => result,
@@ -1633,10 +1652,11 @@ impl ToolSpec for ReviewTool {
                     &route,
                     &usage,
                     format!(
-                        "Review pass {}/{} response incomplete: provider stop reason `{}`; the partial review was not accepted.",
+                        "Review pass {}/{} response incomplete: provider stop reason `{}`; the pass {}. The partial review was not accepted.",
                         index + 1,
                         plan.as_ref().map_or(1, |plan| plan.passes.len()),
-                        codewhale_models::stop_reason_detail(result.stop_reason.as_deref())
+                        codewhale_models::stop_reason_detail(result.stop_reason.as_deref()),
+                        exhausted_pass_clause(&result.usage)
                     ),
                 ));
             }
@@ -1729,17 +1749,30 @@ impl ToolSpec for ReviewTool {
     }
 }
 
-/// Accept one non-PR review reply (#6561 D03-m4). Prose that ignores the
-/// JSON contract is still a review (see [`ReviewOutput::from_str`]), but an
-/// empty reply, or JSON that carries no summary, issue, suggestion or
-/// assessment (`{}`, an unrelated object), used to become an empty
-/// successful review that read as clean.
 pub(crate) fn leaked_tool_call_message(pass: usize, passes: usize) -> String {
     format!(
         "Review pass {pass}/{passes} returned tool-call markup instead of a review: the model tried to call tools this pass did not offer. This is not a review result, and the response was not accepted or posted."
     )
 }
 
+pub(crate) fn exhausted_pass_clause(usage: &Usage) -> String {
+    let visible_tokens = usage
+        .output_tokens
+        .saturating_sub(usage.reasoning_tokens.unwrap_or(0));
+    if visible_tokens > 0 {
+        format!(
+            "was cut off before the review finished ({visible_tokens} visible tokens came back)"
+        )
+    } else {
+        "produced no review text".to_string()
+    }
+}
+
+/// Accept one non-PR review reply (#6561 D03-m4). Prose that ignores the
+/// JSON contract is still a review (see [`ReviewOutput::from_str`]), but an
+/// empty reply, or JSON that carries no summary, issue, suggestion or
+/// assessment (`{}`, an unrelated object), used to become an empty
+/// successful review that read as clean.
 fn accept_single_review(response_text: &str) -> Result<ReviewOutput, String> {
     let output = ReviewOutput::from_str(response_text);
     if response_text.trim().is_empty()

crates/tui/src/tools/review_triage.rs

@@ -181,6 +181,8 @@ pub fn triage_review(diff: &str) -> ReviewTriage {
     }
 }
 
+pub const REASONING_ROUTE_MIN_OUTPUT_TOKENS: u32 = 8_192;
+
 /// The budget a tier buys. Low risk still gets a real review of the diff, just
 /// without repository tools; high risk gets dependent tracing and room to
 /// reproduce.
@@ -211,6 +213,16 @@ impl ReviewBudget {
         }
     }
 
+    #[must_use]
+    pub fn with_reasoning_floor(mut self, reserve_percent: u32) -> Self {
+        if reserve_percent > 0 {
+            self.max_output_tokens = self
+                .max_output_tokens
+                .max(REASONING_ROUTE_MIN_OUTPUT_TOKENS);
+        }
+        self
+    }
+
     /// Attach a per-review money cap. A cap without a price is not enforceable,
     /// and saying so is the point: `money_limit_supported` stays false until a
     /// rate is supplied, so a caller can never report a cap it did not apply.

crates/tui/src/tui/ui/apply.rs

@@ -2105,7 +2105,7 @@ async fn apply_command_result_inner(
                     } else {
                         let base_url = config.active_route_base_url();
                         match fetch_provider_balance(provider, &api_key, &base_url).await {
-                            Some(info) => {
+                            Ok(info) => {
                                 if let Ok(mut guard) =
                                     balance_cell_for_route(app, provider, &api_key, &base_url)
                                         .lock()
@@ -2117,7 +2117,7 @@ async fn apply_command_result_inner(
                                     content: info.report(provider.provider().display_name()),
                                 });
                             }
-                            None => {
+                            Err(reason) => {
                                 let fallback = app
                                     .balance_cell
                                     .lock()
@@ -2126,7 +2126,7 @@ async fn apply_command_result_inner(
                                     .and_then(|info| {
                                         info.chip_label().map(|amount| {
                                             format!(
-                                                "Could not refresh {} balance; last known: {amount}",
+                                                "Could not refresh {} balance ({reason}); last known: {amount}",
                                                 provider.provider().display_name()
                                             )
                                         })
@@ -2134,7 +2134,7 @@ async fn apply_command_result_inner(
                                 app.add_message(HistoryCell::System {
                                     content: fallback.unwrap_or_else(|| {
                                         format!(
-                                            "Could not fetch {} account balance. Check the provider dashboard.",
+                                            "Could not fetch {} account balance ({reason}). Check the provider dashboard.",
                                             provider.provider().display_name()
                                         )
                                     }),

crates/tui/src/tui/ui/provider_routes.rs

@@ -184,18 +184,18 @@ fn balance_api_for(provider: ProviderKind) -> Option<BalanceApi> {
 
 /// Fetch remaining credit for the active prepaid provider.
 ///
-/// Returns `None` on any error (network, auth, parse) — callers treat that
+/// Returns the reason on any error (network, auth, parse) — callers treat that
 /// as "balance unknown" and keep the previous value.
 pub(crate) async fn fetch_provider_balance(
     provider: ProviderKind,
     api_key: &str,
     base_url: &str,
-) -> Option<crate::pricing::BalanceInfo> {
+) -> Result<crate::pricing::BalanceInfo, String> {
     let api_key = api_key.trim();
     if api_key.is_empty() {
-        return None;
+        return Err("no API key".to_string());
     }
-    match balance_api_for(provider)? {
+    match balance_api_for(provider).ok_or_else(|| "balance is not supported".to_string())? {
         BalanceApi::DeepSeekUserBalance => fetch_deepseek_user_balance(api_key, base_url).await,
         BalanceApi::OpenRouterCredits => fetch_openrouter_credits(api_key, base_url).await,
         BalanceApi::SiliconFlowUserInfo => {
@@ -207,7 +207,7 @@ pub(crate) async fn fetch_provider_balance(
 async fn fetch_deepseek_user_balance(
     api_key: &str,
     base_url: &str,
-) -> Option<crate::pricing::BalanceInfo> {
+) -> Result<crate::pricing::BalanceInfo, String> {
     let origin = base_url.trim_end_matches('/');
     let origin = origin
         .strip_suffix("/beta")
@@ -215,7 +215,10 @@ async fn fetch_deepseek_user_balance(
         .unwrap_or(origin);
     let url = format!("{origin}/user/balance");
     let body: crate::pricing::BalanceResponse = balance_get_json(api_key, &url).await?;
-    body.balance_infos.into_iter().next()
+    body.balance_infos
+        .into_iter()
+        .next()
+        .ok_or_else(|| "the response listed no balance".to_string())
 }
 
 #[derive(serde::Deserialize)]
@@ -236,11 +239,11 @@ fn openrouter_remaining_credits(total_credits: f64, total_usage: f64) -> f64 {
 async fn fetch_openrouter_credits(
     api_key: &str,
     base_url: &str,
-) -> Option<crate::pricing::BalanceInfo> {
+) -> Result<crate::pricing::BalanceInfo, String> {
     let url = format!("{}/credits", base_url.trim_end_matches('/'));
     let body: OpenRouterCreditsResponse = balance_get_json(api_key, &url).await?;
     let remaining = openrouter_remaining_credits(body.data.total_credits, body.data.total_usage);
-    Some(crate::pricing::BalanceInfo {
+    Ok(crate::pricing::BalanceInfo {
         currency: "USD".to_string(),
         total_balance: format!("{remaining:.2}"),
         topped_up_balance: format!("{:.2}", body.data.total_credits),
@@ -267,22 +270,25 @@ async fn fetch_siliconflow_user_info(
     api_key: &str,
     base_url: &str,
     provider: ProviderKind,
-) -> Option<crate::pricing::BalanceInfo> {
+) -> Result<crate::pricing::BalanceInfo, String> {
     let url = format!("{}/user/info", base_url.trim_end_matches('/'));
     let body: SiliconFlowUserInfo = balance_get_json(api_key, &url).await?;
-    let data = body.data?;
+    let data = body
+        .data
+        .ok_or_else(|| "the response carried no account data".to_string())?;
     let total = data
         .total_balance
         .as_deref()
         .or(data.balance.as_deref())
         .map(str::trim)
-        .filter(|value| !value.is_empty())?;
+        .filter(|value| !value.is_empty())
+        .ok_or_else(|| "the response carried no balance".to_string())?;
     let currency = if provider == ProviderKind::SiliconflowCN {
         "CNY"
     } else {
         "USD"
     };
-    Some(crate::pricing::BalanceInfo {
+    Ok(crate::pricing::BalanceInfo {
         currency: currency.to_string(),
         total_balance: total.to_string(),
         topped_up_balance: data.charge_balance.unwrap_or_default(),
@@ -290,23 +296,38 @@ async fn fetch_siliconflow_user_info(
     })
 }
 
-async fn balance_get_json<T: serde::de::DeserializeOwned>(api_key: &str, url: &str) -> Option<T> {
+async fn balance_get_json<T: serde::de::DeserializeOwned>(
+    api_key: &str,
+    url: &str,
+) -> Result<T, String> {
     let client = &*BALANCE_CLIENT;
     let response = client
         .get(url)
         .header("Authorization", format!("Bearer {api_key}"))
         .send()
         .await
-        .ok()?;
-    if !response.status().is_success() {
+        .map_err(|error| {
+            if error.is_timeout() {
+                "request timed out".to_string()
+            } else if error.is_connect() {
+                "could not connect to the balance service".to_string()
+            } else {
+                "request failed".to_string()
+            }
+        })?;
+    let status = response.status();
+    if !status.is_success() {
         tracing::debug!(
             "balance API returned {}: {}",
-            response.status().as_u16(),
+            status.as_u16(),
             response.text().await.unwrap_or_default()
         );
-        return None;
+        return Err(format!("HTTP {status}"));
     }
-    response.json().await.ok()
+    response
+        .json()
+        .await
+        .map_err(|_| "the response was not readable".to_string())
 }
 
 pub(crate) fn should_fetch_provider_balance(app: &App) -> bool {
@@ -372,7 +393,7 @@ pub(crate) fn schedule_balance_fetch(app: &mut App, api_key: &str, base_url: &st
     let api_key = api_key.to_string();
     let base_url = base_url.to_string();
     tokio::spawn(async move {
-        if let Some(info) = fetch_provider_balance(provider, &api_key, &base_url).await
+        if let Ok(info) = fetch_provider_balance(provider, &api_key, &base_url).await
             && let Ok(mut guard) = cell.lock()
         {
             *guard = Some(info);