Skip to content

Commit 8a6a28c

Browse files
authored
style: comment consistency sweep across the crate (#20)
## Summary Follow-up style pass deferred from PR #19's `dda3ff5`, which only cleaned up comments added that round. This PR extends the cleanup to the rest of `crates/oxide-code/src/`, closing four drift classes identified by a fresh survey. Pure style / docs; `cargo test` passes with the same 733 count before and after, and `cargo llvm-cov` stays at 93.08% line / 92.57% region. - **Drop design-attribution references to `claude-code`** (`bd932d2`) — rewrites comments like "matches Claude Code's `prependUserContext()`" into rationale that stands on its own (cache-invalidation behavior, API-shape requirements). Wire-compat literals (`CLAUDE_CODE_BETA_HEADER`, identity prefix, `x-claude-code-session-id` header, Keychain service name, billing salt) and user-facing CLI prose in `config` / `config::oauth` stay verbatim — those aren't attribution, they're interop identifiers or the user's mental model. - **Add module-level `//!` preambles across the crate** (`0c2a03a`) — lifts coverage from 8 of 50 `.rs` files to 25 of 50. All ten top-level module roots (`agent`, `client`, `config`, `main`, `message`, `prompt`, `session`, `tool`, `tui`, `util`) plus seven heavyweight submodules (`client/anthropic`, `config/oauth`, `session/{manager, store, entry}`, `tui/app`, `tui/markdown/render` — all the ~1.5K+ line files) now open with a noun-phrase role statement. Trivial / name-self-describing submodules (per-tool files, TUI style helpers, `util/env`, `util/lock`, etc.) intentionally left uncommented. - **Unify first-line doc-comment voice to third-person indicative** (`b8886b9`) — the crate was ~50/50 between imperative (`Build a foo.`) and third-person indicative (`Builds a foo.`). Stdlib is consistently third-person, and the existing `//!` preambles already follow it; this commit aligns every free-function, method, and associated-function first line. CLI `--help` strings (clap `#[arg(...)]`) stay imperative — that's the `man`-page convention and mixing voices in a single help output would read worse than diverging from the rest of the crate. - **Trim narrate-the-what residue** (`3b74af8`) — two targeted fixes: `ToolOutput::with_title` doc now explains fluent-construction rationale instead of restating the method signature, and `prompt::instructions::load`'s discovery-locations list gets a missing terminal period on item 1. The re-survey also revealed that the original plan over-scoped two drift classes: numbered-list formatting was already canonical on all four sites after PR #19 (only the one missing period remained), and narrate-the-what was not a class-wide issue (two real cases, not a sweep). First-line voice was missing from the original plan entirely and ended up the largest class. ## Changes | Area | Files | Notes | | ---- | ----- | ----- | | Attribution rewrites | `client/anthropic.rs`, `prompt.rs`, `prompt/environment.rs` | 10 design-attribution sites rephrased as cache / API-shape rationale; +25 / −25 | | Module preambles | 17 files (10 top-level roots + 7 heavyweight submodules) | +137 / 0, all additive `//!` blocks | | Voice unification | 27 files across `client`, `config`, `message`, `prompt`, `session`, `tool`, `tui`, `util` | +119 / −119 mechanical; verb takes `-s` suffix on first-line summaries | | Narrate-the-what | `tool.rs`, `prompt/instructions.rs` | 2 targeted fixes | ## Test plan - [x] `cargo fmt --all --check` - [x] `cargo clippy --all-targets -- -D warnings` — zero warnings - [x] `cargo test` — 733 pass (unchanged from `main`) - [x] `cargo llvm-cov --ignore-filename-regex 'main\.rs'` — 93.08% line / 93.65% function / 92.57% region (unchanged from `main`) - [x] `pnpm lint` — 0 errors across 16 files - [x] `pnpm spellcheck` — 0 errors across 72 files; `.cspell/words.txt` unchanged - [ ] GitHub Actions: `rust-check`, `coverage`, `node-check` all green on this PR
1 parent 519a324 commit 8a6a28c

35 files changed

Lines changed: 283 additions & 145 deletions

crates/oxide-code/src/agent.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,10 @@
1+
//! Agent turn loop.
2+
//!
3+
//! Drives one user → assistant round: streams the model response,
4+
//! dispatches any tool calls it emits, records each turn to the
5+
//! session, and stops when the model returns text only or the safety
6+
//! cap [`MAX_TOOL_ROUNDS`] trips.
7+
18
pub(crate) mod event;
29

310
use anyhow::{Context, Result, bail};

crates/oxide-code/src/client.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,4 @@
1+
//! Anthropic Messages API client.
2+
13
pub mod anthropic;
24
mod billing;

crates/oxide-code/src/client/anthropic.rs

Lines changed: 43 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,18 @@
1+
//! Anthropic Messages API streaming client.
2+
//!
3+
//! [`Client::stream_message`] drives the main agent loop: assembles
4+
//! the request (identity prefix, billing attestation for OAuth,
5+
//! static / dynamic system-block split for cache reuse), POSTs
6+
//! `/v1/messages` with SSE streaming, and forwards parsed
7+
//! [`StreamEvent`]s on an mpsc channel. [`Client::complete`] covers
8+
//! non-streaming one-shots (title generation today, future
9+
//! classifiers) with optional JSON-schema-constrained output.
10+
//!
11+
//! Per-request `anthropic-beta` headers are computed from the model's
12+
//! [`crate::model::Capabilities`] via [`compute_betas`], so gateways
13+
//! that reject unsupported betas (Haiku, subscriptions without 1M)
14+
//! don't 400 on spurious feature flags.
15+
116
use std::time::Duration;
217

318
use anyhow::{Context, Result, bail};
@@ -50,8 +65,7 @@ struct CreateMessageRequest<'a> {
5065
#[serde(skip_serializing_if = "Option::is_none")]
5166
thinking: Option<&'a ThinkingConfig>,
5267
/// JSON-schema-constrained output format for one-shot utility calls
53-
/// (title generation, future classifiers). Mirrors claude-code's
54-
/// `extraBodyParams.output_config`. Must travel alongside the
68+
/// (title generation, future classifiers). Must travel alongside the
5569
/// `structured-outputs-2025-12-15` beta header; both are gated on
5670
/// `Capabilities::structured_outputs` so unsupported models silently
5771
/// drop back to free-form text rather than 400ing the gateway.
@@ -77,7 +91,7 @@ pub(crate) struct OutputFormat {
7791
}
7892

7993
impl OutputFormat {
80-
/// Build a `json_schema` output format from a precomputed schema
94+
/// Builds a `json_schema` output format from a precomputed schema
8195
/// value. The schema must already match Anthropic's expectations
8296
/// (`type: "object"`, `additionalProperties: false`, explicit
8397
/// `required` array) — we don't validate here.
@@ -89,7 +103,7 @@ impl OutputFormat {
89103
}
90104
}
91105

92-
/// Request metadata matching Claude Code's `getAPIMetadata()` format.
106+
/// Top-level `metadata` object on every outbound request.
93107
///
94108
/// `user_id` is a stringified JSON object containing `session_id` (and
95109
/// optionally `device_id` / `account_uuid`). The API receives it as a
@@ -354,13 +368,14 @@ impl Client {
354368

355369
/// Stream a message response from the Anthropic API.
356370
///
357-
/// `system_sections` are the static system prompt sections (one text
358-
/// block per section, matching Claude Code's multi-block layout).
371+
/// `system_sections` are the static system prompt sections, each
372+
/// shipped as its own `system` text block so `cache_control` can
373+
/// apply to the static prefix only.
359374
///
360375
/// `user_context` is a `<system-reminder>`-wrapped string that gets
361-
/// prepended to the messages array as a synthetic user message,
362-
/// matching Claude Code's `prependUserContext()` pattern. This keeps
363-
/// dynamic content (CLAUDE.md) out of the `system` parameter.
376+
/// prepended to the messages array as a synthetic user message, so
377+
/// dynamic content (CLAUDE.md) stays out of the `system` parameter
378+
/// and doesn't invalidate the static cache.
364379
///
365380
/// Returns a channel receiver yielding [`StreamEvent`]s. The caller
366381
/// should recv events as they arrive for real-time output.
@@ -393,7 +408,7 @@ impl Client {
393408
None
394409
};
395410

396-
// Build system blocks matching Claude Code's `splitSysPromptPrefix`:
411+
// Assemble the system-block array:
397412
// 1. Billing header (no cache_control)
398413
// 2. Identity prefix (no cache_control)
399414
// 3. Static sections joined (cache_control: ephemeral, scope: global)
@@ -469,7 +484,7 @@ impl Client {
469484
Ok(rx)
470485
}
471486

472-
/// Send a single non-streaming completion request and return the
487+
/// Sends a single non-streaming completion request and returns the
473488
/// concatenated text of the assistant's response.
474489
///
475490
/// Used for background helpers like AI title generation — a one-shot
@@ -533,7 +548,7 @@ impl Client {
533548
Ok(join_text_blocks(content))
534549
}
535550

536-
/// Build the `metadata.user_id` field as a stringified JSON object.
551+
/// Builds the `metadata.user_id` field as a stringified JSON object.
537552
fn build_metadata(&self) -> RequestMetadata {
538553
build_metadata(&self.session_id)
539554
}
@@ -547,7 +562,7 @@ fn build_metadata(session_id: &str) -> RequestMetadata {
547562
RequestMetadata { user_id }
548563
}
549564

550-
/// Compute the `anthropic-beta` header value for a request targeting
565+
/// Computes the `anthropic-beta` header value for a request targeting
551566
/// `model`. Each beta is gated on a specific `Capabilities` flag from
552567
/// the [`crate::model`] lookup table, so adding or bumping a model only
553568
/// means editing that one table — this function stays fixed.
@@ -644,7 +659,7 @@ pub(crate) fn supports_structured_outputs(model: &str) -> bool {
644659
crate::model::lookup(model).is_some_and(|info| info.capabilities.structured_outputs)
645660
}
646661

647-
/// Strip the `[1m]` tag (if any) from a caller-supplied model string,
662+
/// Strips the `[1m]` tag (if any) from a caller-supplied model string,
648663
/// returning the canonical API model ID. The tag is a client-side
649664
/// convention — the Anthropic API rejects it on the wire.
650665
///
@@ -677,7 +692,7 @@ fn tag_offset(model: &str) -> Option<usize> {
677692
model.to_lowercase().find("[1m]")
678693
}
679694

680-
/// Serialize the JSON request body for [`Client::complete`]. Extracted
695+
/// Serializes the JSON request body for [`Client::complete`]. Extracted
681696
/// so the billing-header / identity-prefix / system-block assembly can
682697
/// be asserted on without a live HTTP client; the HTTP leg of
683698
/// [`Client::complete`] is covered behind a wiremock harness (see
@@ -753,7 +768,7 @@ fn build_completion_body(
753768
Ok(body)
754769
}
755770

756-
/// Flatten a `messages.create` response's content array into the
771+
/// Flattens a `messages.create` response's content array into the
757772
/// assistant's user-visible text. Extracted so the filter logic is
758773
/// reusable and independently testable.
759774
fn join_text_blocks(content: Vec<ContentBlock>) -> String {
@@ -774,7 +789,7 @@ struct CompletionResponse {
774789
content: Vec<ContentBlock>,
775790
}
776791

777-
/// Map `std::env::consts::OS` to the Stainless SDK's `normalizePlatform` names.
792+
/// Maps `std::env::consts::OS` to the Stainless SDK's `normalizePlatform` names.
778793
fn normalize_platform(os: &str) -> &'static str {
779794
match os {
780795
"macos" => "MacOS",
@@ -788,7 +803,7 @@ fn normalize_platform(os: &str) -> &'static str {
788803
}
789804
}
790805

791-
/// Map `std::env::consts::ARCH` to the Stainless SDK's `normalizeArch` names.
806+
/// Maps `std::env::consts::ARCH` to the Stainless SDK's `normalizeArch` names.
792807
fn normalize_arch(arch: &str) -> &'static str {
793808
match arch {
794809
"x86" => "x32",
@@ -799,7 +814,7 @@ fn normalize_arch(arch: &str) -> &'static str {
799814
}
800815
}
801816

802-
/// Split system sections at the boundary marker into static and dynamic parts.
817+
/// Splits system sections at the boundary marker into static and dynamic parts.
803818
///
804819
/// Returns `(static_sections, dynamic_sections)`. The boundary marker itself
805820
/// is excluded from both. Sections before the boundary are static (globally
@@ -828,7 +843,7 @@ fn split_at_boundary<'a>(sections: &[&'a str]) -> (Vec<&'a str>, Vec<&'a str>) {
828843
}
829844
}
830845

831-
/// Extract the text of the first user message for fingerprint computation.
846+
/// Extracts the text of the first user message for fingerprint computation.
832847
fn first_user_text(messages: &[Message]) -> &str {
833848
messages
834849
.iter()
@@ -911,7 +926,7 @@ async fn stream_sse(
911926
Ok(())
912927
}
913928

914-
/// Parse a single SSE frame into a [`StreamEvent`].
929+
/// Parses a single SSE frame into a [`StreamEvent`].
915930
///
916931
/// Per the SSE spec, multiple `data:` lines concatenate with `\n`.
917932
/// Anthropic currently emits single-line data, but we follow the spec
@@ -1266,10 +1281,10 @@ mod tests {
12661281

12671282
#[test]
12681283
fn build_metadata_wraps_session_id_in_stringified_json() {
1269-
// The API accepts `metadata.user_id` as a string, not a nested
1270-
// object — claude-code stringifies a JSON object with `session_id`
1271-
// (and sometimes `device_id` / `account_uuid`). Round-trip check
1272-
// keeps the contract explicit.
1284+
// `metadata.user_id` is a stringified JSON object on the wire
1285+
// (contains `session_id` and optionally `device_id` /
1286+
// `account_uuid`), not a nested object. Round-trip check keeps
1287+
// the double-encoding explicit.
12731288
let meta = build_metadata("abc-123");
12741289
let parsed: serde_json::Value = serde_json::from_str(&meta.user_id).unwrap();
12751290
assert_eq!(parsed["session_id"], "abc-123");
@@ -1338,9 +1353,9 @@ mod tests {
13381353

13391354
#[test]
13401355
fn build_completion_body_oauth_injects_billing_header_and_cch() {
1341-
// OAuth must emit an initial billing block (Claude Code's fingerprint
1342-
// contract) and the placeholder `cch=00000` must be replaced by an
1343-
// actual 5-hex-digit tag via `inject_cch`.
1356+
// OAuth must emit an initial billing block carrying the version
1357+
// fingerprint, and the placeholder `cch=00000` must be replaced
1358+
// by an actual 5-hex-digit tag via `inject_cch`.
13441359
let body = build_completion_body(
13451360
"claude-haiku-4-5",
13461361
"sys-prompt",

crates/oxide-code/src/client/billing.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ const CCH_PLACEHOLDER: &str = "cch=00000";
1515

1616
// ── Public API ──
1717

18-
/// Compute the 3-character hex fingerprint suffix for `cc_version`.
18+
/// Computes the 3-character hex fingerprint suffix for `cc_version`.
1919
///
2020
/// `SHA-256(salt + chars_at_indices + version)`, take first 3 hex chars.
2121
/// Character extraction uses Unicode scalar indexing, which matches
@@ -31,7 +31,7 @@ pub(super) fn compute_fingerprint(first_user_message: &str, version: &str) -> St
3131
format!("{:02x}{:02x}", hash[0], hash[1])[..3].to_string()
3232
}
3333

34-
/// Build the billing attribution header with a `cch=00000` placeholder.
34+
/// Builds the billing attribution header with a `cch=00000` placeholder.
3535
///
3636
/// The placeholder is later replaced by [`inject_cch`] with the computed hash.
3737
pub(super) fn build_billing_header(version: &str, fingerprint: &str) -> String {
@@ -43,7 +43,7 @@ pub(super) fn build_billing_header(version: &str, fingerprint: &str) -> String {
4343
)
4444
}
4545

46-
/// Compute xxHash64 of the request body and replace the `cch` placeholder.
46+
/// Computes xxHash64 of the request body and replaces the `cch` placeholder.
4747
///
4848
/// The hash is computed over the body *with* the placeholder in place,
4949
/// then the placeholder is replaced with the 5-char hex result. Only the

crates/oxide-code/src/config.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,10 @@
1+
//! Configuration loading.
2+
//!
3+
//! Layered precedence (highest wins): env vars > project `ox.toml` >
4+
//! user `~/.config/ox/config.toml` > built-in defaults. Auth follows
5+
//! the same precedence but terminates at the first source that
6+
//! resolves (API key env > API key in file > OAuth credentials).
7+
18
mod file;
29
mod oauth;
310

@@ -36,7 +43,7 @@ pub struct Config {
3643
}
3744

3845
impl Config {
39-
/// Load configuration from files and environment variables.
46+
/// Loads configuration from files and environment variables.
4047
///
4148
/// Precedence (highest wins): env vars > project `ox.toml` > user
4249
/// `~/.config/ox/config.toml` > built-in defaults.

crates/oxide-code/src/config/file.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ pub(super) struct TuiConfig {
5151
// ── Merge ──
5252

5353
impl FileConfig {
54-
/// Merge two configs. Fields in `other` take precedence over `self`.
54+
/// Merges two configs. Fields in `other` take precedence over `self`.
5555
fn merge(self, other: Self) -> Self {
5656
Self {
5757
client: merge_section(self.client, other.client, ClientConfig::merge),
@@ -83,7 +83,7 @@ impl TuiConfig {
8383
}
8484
}
8585

86-
/// Merge two optional config sections. When both are present, merge their
86+
/// Merges two optional config sections. When both are present, merges their
8787
/// fields. When only one is present, use it as-is.
8888
fn merge_section<T>(base: Option<T>, other: Option<T>, merge: fn(T, T) -> T) -> Option<T> {
8989
match (base, other) {
@@ -94,7 +94,7 @@ fn merge_section<T>(base: Option<T>, other: Option<T>, merge: fn(T, T) -> T) ->
9494

9595
// ── Loading ──
9696

97-
/// Load and merge configuration from user and project TOML files.
97+
/// Loads and merges configuration from user and project TOML files.
9898
///
9999
/// Precedence (highest wins): project config > user config.
100100
/// Environment variable overrides are applied later in [`super::Config::load`].
@@ -136,12 +136,12 @@ fn user_config_path() -> Option<PathBuf> {
136136
)
137137
}
138138

139-
/// Walk from CWD upward to find the nearest `ox.toml`.
139+
/// Walks from CWD upward to find the nearest `ox.toml`.
140140
fn find_project_config() -> Option<PathBuf> {
141141
find_project_config_from(std::env::current_dir().ok()?)
142142
}
143143

144-
/// Walk from `start` upward to find the nearest `ox.toml`.
144+
/// Walks from `start` upward to find the nearest `ox.toml`.
145145
///
146146
/// Separated from [`find_project_config`] for testability (avoids changing
147147
/// the process CWD).

crates/oxide-code/src/config/oauth.rs

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,13 @@
1+
//! Claude Code OAuth credentials.
2+
//!
3+
//! Loads the OAuth access token from the macOS Keychain (service
4+
//! `"Claude Code-credentials"`) and `~/.claude/.credentials.json`,
5+
//! preferring whichever has the later expiry. Refreshes via the
6+
//! Anthropic token endpoint when the access token is expired or about
7+
//! to expire, writing the new pair back to both sources. A directory-
8+
//! based advisory lock keeps two `ox` instances from refreshing
9+
//! concurrently and clobbering each other's tokens.
10+
111
use std::path::{Path, PathBuf};
212
use std::time::Duration;
313

@@ -54,7 +64,7 @@ impl OAuthCredential {
5464

5565
// ── Token Loading ──
5666

57-
/// Load an OAuth access token from Claude Code credentials, refreshing
67+
/// Loads an OAuth access token from Claude Code credentials, refreshing
5868
/// proactively if the token is within 5 minutes of expiry.
5969
pub async fn load_token() -> Result<String> {
6070
let file_path = credentials_path().context("could not determine home directory")?;
@@ -105,7 +115,7 @@ pub async fn load_token() -> Result<String> {
105115
}
106116
}
107117

108-
/// Load credentials, preferring the macOS Keychain over the file.
118+
/// Loads credentials, preferring the macOS Keychain over the file.
109119
///
110120
/// File contents are user-writable, so an attacker with write access to
111121
/// `~/.claude/.credentials.json` could otherwise spoof a far-future expiry
@@ -243,7 +253,7 @@ struct RefreshResponse {
243253
scope: Option<String>,
244254
}
245255

246-
/// Write refreshed tokens back to the credentials file (and macOS Keychain),
256+
/// Writes refreshed tokens back to the credentials file (and macOS Keychain),
247257
/// preserving unknown fields.
248258
///
249259
/// Replaces the file atomically (temp + rename) so a crash mid-write cannot
@@ -283,7 +293,7 @@ fn write_refreshed_credentials(path: &Path, response: &RefreshResponse) -> Resul
283293
Ok(())
284294
}
285295

286-
/// Write `bytes` to `path` atomically with owner-only (`0o600`) permissions
296+
/// Writes `bytes` to `path` atomically with owner-only (`0o600`) permissions
287297
/// on Unix. Creates a sibling `.tmp.<uuid>` file then renames — the rename
288298
/// is atomic on POSIX, so readers always see either the old or new content.
289299
fn atomic_write_private(path: &Path, bytes: &[u8]) -> Result<()> {

crates/oxide-code/src/main.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,10 @@
1+
//! Binary entry point.
2+
//!
3+
//! Parses CLI flags, loads [`Config`], resolves which session to
4+
//! resume (if any), and dispatches into one of three run modes: TUI
5+
//! (default), bare REPL (`--no-tui`), or headless one-shot (`-p`).
6+
//! Signal handling and session summary writes on abort live here.
7+
18
mod agent;
29
mod client;
310
mod config;
@@ -142,7 +149,7 @@ async fn async_main() -> Result<()> {
142149

143150
// ── Session Helpers ──
144151

145-
/// Print a table of recent sessions and exit. With `all = true`, spans
152+
/// Prints a table of recent sessions and exits. With `all = true`, spans
146153
/// every project; otherwise scoped to the current working directory.
147154
fn list_sessions(all: bool) -> Result<()> {
148155
let store = SessionStore::open()?;
@@ -157,7 +164,7 @@ fn list_sessions(all: bool) -> Result<()> {
157164
)
158165
}
159166

160-
/// Detect the terminal width for title truncation in `--list`.
167+
/// Detects the terminal width for title truncation in `--list`.
161168
/// Returns `None` when stdout is not a TTY (piped / redirected) or
162169
/// when the window size cannot be queried — the renderer skips
163170
/// truncation in either case so downstream tools see the full title.
@@ -181,7 +188,7 @@ fn create_tool_registry() -> ToolRegistry {
181188
])
182189
}
183190

184-
/// Wait for any shutdown signal — SIGINT (portable), SIGTERM, or
191+
/// Waits for any shutdown signal — SIGINT (portable), SIGTERM, or
185192
/// SIGHUP (Unix only). Returns when the first signal arrives.
186193
///
187194
/// Installs the handlers lazily on first call. Callers that embed this

0 commit comments

Comments
 (0)