tighten comment hygiene and extract magic-literal consts

Author: alexylon

AGENTS.md

@@ -436,6 +436,7 @@ Gitignored scratchpad for helper files the user asks to be created there — typ
     - `cargo clippy --all-targets -- -D warnings`
     - `cargo test --all`
 - Before finishing, review the change for bugs and corner cases.
+- After each final modification, suggest a clear one-line commit message.
 
 ---
 

src/api/anthropic.rs

@@ -6,9 +6,13 @@ use reqwest::header::{HeaderMap, HeaderValue};
 use std::sync::Arc;
 use std::sync::atomic::{AtomicBool, Ordering};
 
-const API_BASE: &str = "https://api.anthropic.com/v1";
-const API_VERSION: &str = "2023-06-01";
-/// Header name for Anthropic feature opt-ins.
+const ANTHROPIC_API_BASE: &str = "https://api.anthropic.com/v1";
+/// Pinned `anthropic-version` header value. `2023-06-01` is the
+/// version the public Anthropic Messages API is documented against;
+/// bumping it changes streaming-event shapes and request fields, so
+/// any change here needs a smoke test against both streaming and
+/// non-streaming paths.
+const ANTHROPIC_API_VERSION: &str = "2023-06-01";
 const BETA_HEADER_NAME: &str = "anthropic-beta";
 
 /// Universal beta token: shrinks tool-call envelopes. Supported on
@@ -112,7 +116,10 @@ impl AnthropicClient {
             HeaderValue::from_str(&api_key)
                 .map_err(|e| SofosError::Config(format!("Invalid API key format: {}", e)))?,
         );
-        headers.insert("anthropic-version", HeaderValue::from_static(API_VERSION));
+        headers.insert(
+            "anthropic-version",
+            HeaderValue::from_static(ANTHROPIC_API_VERSION),
+        );
         // `anthropic-beta` is set per-request by `anthropic_beta_for`
         // so the compaction beta only ships when the target model
         // actually supports it.
@@ -126,7 +133,7 @@ impl AnthropicClient {
     pub async fn check_connectivity(&self) -> Result<()> {
         utils::check_api_connectivity(
             &self.client,
-            API_BASE,
+            ANTHROPIC_API_BASE,
             "Anthropic",
             "https://status.anthropic.com",
         )
@@ -157,7 +164,7 @@ impl AnthropicClient {
         &self,
         request: CreateMessageRequest,
     ) -> Result<CreateMessageResponse> {
-        let url = format!("{}/messages", API_BASE);
+        let url = format!("{}/messages", ANTHROPIC_API_BASE);
         let request = Self::prepare_request(request);
         let beta = anthropic_beta_for(&request.model);
 
@@ -189,7 +196,7 @@ impl AnthropicClient {
         request.stream = Some(true);
         let beta = anthropic_beta_for(&request.model);
 
-        let url = format!("{}/messages", API_BASE);
+        let url = format!("{}/messages", ANTHROPIC_API_BASE);
 
         let response = utils::send_once(
             "Anthropic",

src/api/openai.rs

@@ -6,6 +6,7 @@ use serde::Deserialize;
 use serde_json::json;
 
 const OPENAI_API_BASE: &str = "https://api.openai.com/v1";
+const TOOL_CHOICE_AUTO: &str = "auto";
 
 #[derive(Clone)]
 pub struct OpenAIClient {
@@ -272,7 +273,7 @@ fn build_responses_body(request: &CreateMessageRequest) -> serde_json::Value {
 
         if !tools.is_empty() {
             body["tools"] = json!(tools);
-            body["tool_choice"] = json!("auto");
+            body["tool_choice"] = json!(TOOL_CHOICE_AUTO);
         }
     }
 

src/clipboard.rs

@@ -43,12 +43,8 @@ pub fn strip_paste_markers(input: &str) -> (String, Vec<usize>) {
     (cleaned.trim().to_string(), indices)
 }
 
-/// Ceiling on pasted clipboard images. Matches the 20 MB cap Anthropic
-/// imposes on base64-encoded image bodies in the Messages API — a
-/// larger screenshot would just get rejected at request time with a
-/// confusing 400. Checked on both the raw PNG buffer (encoder output)
-/// and the encoded base64 so a huge image never makes it into the
-/// conversation state.
+/// Matches the 20 MB cap Anthropic imposes on base64-encoded image
+/// bodies in the Messages API.
 const MAX_CLIPBOARD_IMAGE_BYTES: usize = 20 * 1024 * 1024;
 
 pub fn get_clipboard_image() -> Option<PastedImage> {

src/config.rs

@@ -1,6 +1,6 @@
 /// Central configuration for Sofos. The actual file-size and bash-output
 /// caps live next to the code that enforces them — `MAX_FILE_SIZE` in
-/// `src/tools/filesystem.rs` (50 MB) and `MAX_OUTPUT_SIZE` in
+/// `src/tools/filesystem.rs` (50 MB) and `MAX_BASH_OUTPUT_BYTES` in
 /// `src/tools/bashexec.rs` (10 MB) — not here, so this struct only
 /// carries config values that the rest of the crate actually reads.
 ///

src/error.rs

@@ -1,5 +1,26 @@
 use thiserror::Error;
 
+pub const DEFAULT_PARENT_DIR: &str = ".";
+
+/// Substrings that, when found in a `ToolExecution` error message,
+/// classify the error as an *expected* security block rather than an
+/// actual failure. Both casings of "blocked" / "Blocked" are listed
+/// because tool messages are constructed inline by call sites that
+/// don't share a casing convention.
+const BLOCKED_KEYWORDS: &[&str] = &[
+    "blocked",
+    "Blocked",
+    "denied",
+    "not allowed",
+    "not explicitly allowed",
+    "outside workspace",
+    "absolute paths",
+    "contains '..'",
+    "tilde paths",
+    "output redirection",
+    "here-doc",
+];
+
 #[derive(Error, Debug)]
 pub enum SofosError {
     #[error("API error: {0}")]
@@ -55,19 +76,7 @@ impl SofosError {
     pub fn is_blocked(&self) -> bool {
         match self {
             Self::PathViolation(_) => true,
-            Self::ToolExecution(msg) => {
-                msg.contains("blocked")
-                    || msg.contains("Blocked")
-                    || msg.contains("denied")
-                    || msg.contains("not allowed")
-                    || msg.contains("not explicitly allowed")
-                    || msg.contains("outside workspace")
-                    || msg.contains("absolute paths")
-                    || msg.contains("contains '..'")
-                    || msg.contains("tilde paths")
-                    || msg.contains("output redirection")
-                    || msg.contains("here-doc")
-            }
+            Self::ToolExecution(msg) => BLOCKED_KEYWORDS.iter().any(|kw| msg.contains(kw)),
             Self::McpError(_) => false,
             Self::Join(_) => false,
             Self::Context { source, .. } => source.is_blocked(),
@@ -81,7 +90,7 @@ impl SofosError {
                 let parent = std::path::Path::new(path)
                     .parent()
                     .and_then(|p| p.to_str())
-                    .unwrap_or(".");
+                    .unwrap_or(DEFAULT_PARENT_DIR);
                 Some(format!(
                     "Use list_directory on '{}' to see available files",
                     parent

src/repl/conversation.rs

@@ -1,15 +1,24 @@
 use crate::api::{Message, SystemPrompt, utils::truncate_at_char_boundary};
 use crate::config::SofosConfig;
 
+/// Hard floor on the number of messages `trim_if_needed` will keep,
+/// even when the per-message budget would normally drop more. Below
+/// this, conversations lose enough context that the model starts
+/// hallucinating prior tool results.
+const TRIM_MIN_MESSAGES: usize = 10;
+
+/// Per-end cap on retained characters when `truncate_tool_results`
+/// shortens a long tool output during compaction. The middle is
+/// replaced with an elision marker.
+const COMPACTION_TOOL_RESULT_KEEP_CHARS: usize = 500;
+
 #[derive(Clone)]
 pub struct ConversationHistory {
     messages: Vec<Message>,
     system_prompt: Vec<SystemPrompt>,
     config: SofosConfig,
-    /// Set when `trim_if_needed` printed the floor-hit warning; cleared
-    /// the next time we end a trim under budget. Stops the warning from
-    /// firing on every message append once we're stuck at the 10-message
-    /// floor.
+    /// Latches the floor-hit warning so it fires once per stuck-at-floor
+    /// episode, not on every append.
     warned_at_floor: bool,
     /// Index of the message whose last block carries the secondary
     /// Anthropic `cache_control` marker (the "anchor"). Stays put across
@@ -300,17 +309,21 @@ Show imperial units only when the user explicitly asks for them."#,
         // The warning describes our internal trim heuristic, not the
         // model's API context window — those are different numbers.
         // The condition below means: we tried to trim down to budget
-        // but hit the 10-message floor. The model API will still accept
-        // the request; this just warns the user that auto-trim can't
-        // help further. Dedup with `warned_at_floor` so a long agent
-        // loop doesn't print the warning on every tool round-trip.
-        let at_floor = total_tokens > self.config.max_context_tokens && self.messages.len() <= 10;
+        // but hit the `TRIM_MIN_MESSAGES` floor. The model API will
+        // still accept the request; this just warns the user that
+        // auto-trim can't help further. Dedup with `warned_at_floor`
+        // so a long agent loop doesn't print the warning on every
+        // tool round-trip.
+        let at_floor = total_tokens > self.config.max_context_tokens
+            && self.messages.len() <= TRIM_MIN_MESSAGES;
         if at_floor {
             if !self.warned_at_floor {
                 eprintln!(
-                    "⚠️  Auto-trim hit the 10-message floor at ~{} tokens (budget {}). \
+                    "⚠️  Auto-trim hit the {floor}-message floor at ~{tokens} tokens (budget {budget}). \
                      Run /compact or /clear if responses start degrading.",
-                    total_tokens, self.config.max_context_tokens
+                    floor = TRIM_MIN_MESSAGES,
+                    tokens = total_tokens,
+                    budget = self.config.max_context_tokens,
                 );
                 self.warned_at_floor = true;
             }
@@ -573,7 +586,7 @@ Show imperial units only when the user explicitly asks for them."#,
         // stamp a marker on a now-mismatched position.
         self.cache_anchor_message_idx = None;
         let threshold = self.config.tool_result_truncate_threshold;
-        let keep_chars = 500;
+        let keep_chars = COMPACTION_TOOL_RESULT_KEEP_CHARS;
 
         for msg in self.messages[..up_to].iter_mut() {
             if let crate::api::MessageContent::Blocks { content } = &mut msg.content {

src/repl/mod.rs

@@ -77,13 +77,10 @@ pub struct Repl {
     /// iterations so the user can redirect in-flight work without having
     /// to interrupt it.
     steer_queue: SteerQueue,
-    /// Text the TUI should print through its captured-stdout pipe right
-    /// after `OutputCapture` is installed. Collected in `main.rs` (logo,
-    /// workspace, model, reasoning/thinking, morph availability) so the
-    /// same lines that used to go to the real tty before the TUI took
-    /// over now flow through the history pipeline — keeping the viewport
-    /// from overwriting them on terminals whose cursor-position DSR
-    /// doesn't answer (e.g. Ghostty), where our fallback was `(0, 0)`.
+    /// Queued through the TUI's captured-stdout pipe so the banner
+    /// survives terminals whose cursor-position DSR doesn't answer
+    /// (e.g. Ghostty), where the fallback origin would let the viewport
+    /// overwrite the lines.
     startup_banner: String,
     /// Shared tokio runtime driving every `block_on` in the REPL
     /// (initial request, compaction summary, tool-list refresh). Built

src/repl/response_handler.rs

@@ -65,11 +65,6 @@ impl ResponseHandler {
         }
     }
 
-    /// Fold a `Usage` payload into the per-turn running totals carried
-    /// by `handle_response`. Centralised so the counter increments
-    /// stay consistent across the three sites that consume responses
-    /// (auto-continue after reasoning-only blocks, tool-result loop,
-    /// max-iterations summary).
     fn accumulate_usage(
         usage: &crate::api::Usage,
         total_input: &mut u32,

src/repl/tui/mod.rs

@@ -40,6 +40,9 @@ use app::{App, Picker};
 use event::{Job, UiEvent};
 use output::OutputCapture;
 
+/// Background tick cadence — frame rate vs. event-loop responsiveness
+/// tradeoff. ~11 Hz is fast enough that streamed output looks fluid
+/// without burning CPU on a quiet conversation.
 const TICK_INTERVAL: Duration = Duration::from_millis(90);
 /// Maximum captured output lines coalesced into a single `insert_before`
 /// call. A high value amortises terminal I/O when a tool streams a lot of