From 59acebe4876ed283344b28e975b46d0c3120e745 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Fri, 16 Oct 2026 14:19:39 +0800 Subject: [PATCH 1/3] refactor(prompt): share context section rendering and tighten tests --- crates/oxide-code/src/prompt.rs | 341 +++++++++---------- crates/oxide-code/src/prompt/environment.rs | 37 +- crates/oxide-code/src/prompt/instructions.rs | 59 ++-- 3 files changed, 210 insertions(+), 227 deletions(-) diff --git a/crates/oxide-code/src/prompt.rs b/crates/oxide-code/src/prompt.rs index fcb35b0a..ba1655a1 100644 --- a/crates/oxide-code/src/prompt.rs +++ b/crates/oxide-code/src/prompt.rs @@ -162,34 +162,23 @@ fn system_sections(environment: &Environment) -> Vec { } fn render_user_context(context: &UserContext) -> String { - let claude_md_section = if context.instructions.is_empty() { - String::new() - } else { - formatdoc! {" - # CLAUDE.md - - {instructions} - - ", instructions = context.instructions} - }; + let mut sections = Vec::new(); + if !context.instructions.is_empty() { + sections.push(render_instructions(&context.instructions)); + } + sections.push(render_date(&context.date)); + sections.push(render_model(&context.model)); formatdoc! {" As you answer the user's questions, you can use the following context: - {claude_md_section}# Current date - - Today's date is {date}. - - # Model - - {model} + {sections} IMPORTANT: this context may or may not be relevant to your tasks. You should not respond to this context unless it is highly relevant to your task. ", - date = context.date, - model = context.model, + sections = sections.join("\n\n"), } } @@ -202,27 +191,13 @@ fn render_reminder(seen: &UserContext, fresh: &UserContext) -> String { } else { &fresh.instructions }; - sections.push(formatdoc! {" - # CLAUDE.md - - {body}" - }); + sections.push(render_instructions(body)); } if fresh.date != seen.date { - sections.push(formatdoc! {" - # Current date - - Today's date is {date}.", - date = fresh.date, - }); + sections.push(render_date(&fresh.date)); } if fresh.model != seen.model { - sections.push(formatdoc! {" - # Model - - {model}", - model = fresh.model, - }); + sections.push(render_model(&fresh.model)); } formatdoc! {" @@ -235,6 +210,30 @@ fn render_reminder(seen: &UserContext, fresh: &UserContext) -> String { } } +fn render_instructions(instructions: &str) -> String { + formatdoc! {" + # CLAUDE.md + + {instructions}" + } +} + +fn render_date(date: &str) -> String { + formatdoc! {" + # Current date + + Today's date is {date}." + } +} + +fn render_model(model: &str) -> String { + formatdoc! {" + # Model + + {model}" + } +} + /// Whether `text` is a context reminder, which transcript displays leave out. pub(crate) fn is_system_reminder(text: &str) -> bool { text.starts_with("") @@ -268,6 +267,54 @@ mod tests { use super::*; use crate::util::git::testing::{isolated_git, with_isolated_git_env_async}; + const IDENTITY: &str = "You are powered by the model test-model."; + + fn cwd() -> PathBuf { + std::env::current_dir().expect("cwd should be available") + } + + fn spec(id: &str) -> ModelSpec { + ModelSpec::resolve(id.to_owned()) + } + + fn context(instructions: &str, date: &str) -> UserContext { + UserContext { + instructions: instructions.to_owned(), + date: date.to_owned(), + model: IDENTITY.to_owned(), + } + } + + /// A recording from a machine unlike this one, so a replayed environment is recognizable. + fn recorded_context(cwd: &Path) -> SessionContext { + serde_json::from_value(serde_json::json!({ + "environment": { + "cwd": cwd, + "is_git": false, + "platform": "plan9", + "shell": "rc", + "os_version": "Plan 9 4e", + }, + "user_context": "recorded context", + "seen": { + "instructions": "Recorded rules.", + "date": "2026-04-12", + "model": IDENTITY, + }, + })) + .unwrap() + } + + fn init_git_repo(path: &Path) { + let status = isolated_git(path) + .args(["init"]) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .expect("failed to spawn git"); + assert!(status.success(), "git init exited with non-zero status"); + } + // ── PromptParts::catch_up ── #[test] @@ -334,30 +381,6 @@ mod tests { // ── build_prompt ── - #[tokio::test] - async fn build_prompt_system_starts_with_intro() { - let parts = build_prompt(&spec("test-model"), &cwd()).await; - assert!( - parts - .system_joined() - .starts_with("You are an interactive agent"), - "system should start with intro section" - ); - } - - #[tokio::test] - async fn build_prompt_system_contains_all_static_sections() { - let parts = build_prompt(&spec("test-model"), &cwd()).await; - let joined = parts.system_joined(); - assert!(joined.contains("# System")); - assert!(joined.contains("# Doing tasks")); - assert!(joined.contains("# Executing actions with care")); - assert!(joined.contains("# Using your tools")); - assert!(joined.contains("# Tone and style")); - assert!(joined.contains("# Output efficiency")); - assert!(joined.contains("# Environment")); - } - #[tokio::test] async fn build_prompt_names_the_model_in_user_context_only() { let model = spec("claude-opus-5-5"); @@ -375,27 +398,6 @@ mod tests { assert!(!parts.system_joined().contains("claude-opus-5-5")); } - #[tokio::test] - async fn build_prompt_system_does_not_contain_user_instructions() { - let parts = build_prompt(&spec("test-model"), &cwd()).await; - assert!( - !parts.system_joined().contains("# User instructions"), - "CLAUDE.md should be in user_context, not system" - ); - } - - #[tokio::test] - async fn build_prompt_sections_joined_with_double_newline() { - let parts = build_prompt(&spec("test-model"), &cwd()).await; - let joined = parts.system_joined(); - let system_start = joined.find("# System").expect("system section missing"); - let before = &joined[..system_start]; - assert!( - before.ends_with("\n\n"), - "sections should be joined with double newline" - ); - } - // ── restore_prompt ── #[tokio::test] @@ -447,6 +449,7 @@ mod tests { "# Environment", ]; let joined = parts.system_joined(); + assert!(joined.starts_with(INTRO)); let mut prev_pos = 0; for header in &expected_system_sections { let pos = joined @@ -474,23 +477,13 @@ mod tests { "system_sections should contain the boundary marker" ); + assert!(!joined.contains("Test project rules.")); + let ctx = &parts.user_context; assert!(ctx.contains("Test project rules.")); assert!(ctx.contains("")); } - #[tokio::test] - async fn assemble_without_project_claude_md_keeps_system_clean() { - let tmp = tempfile::tempdir().expect("failed to create tempdir"); - init_git_repo(tmp.path()); - - let parts = assemble(&spec("test-model"), tmp.path(), Some(tmp.path())).await; - assert!( - !parts.system_joined().contains("# User instructions"), - "system should never contain user instructions" - ); - } - #[tokio::test] async fn assemble_walks_root_to_cwd_for_instruction_discovery() { let tmp = tempfile::tempdir().expect("failed to create tempdir"); @@ -519,54 +512,110 @@ mod tests { // ── render_user_context ── #[test] - fn render_user_context_with_instructions() { - let ctx = render_user_context(&context("Project rules.", "2026-04-12")); - assert!(ctx.starts_with("")); - assert!(ctx.ends_with("")); - assert!(ctx.contains(indoc! {" - # CLAUDE.md + fn render_user_context_with_instructions_includes_every_section() { + assert_eq!( + render_user_context(&context("Project rules.", "2026-04-12")), + indoc! {" + + As you answer the user's questions, you can use the following context: - Project rules."})); - assert!(ctx.contains(indoc! {" - # Current date + # CLAUDE.md - Today's date is 2026-04-12."})); - assert!(ctx.contains(&formatdoc! {" - # Model + Project rules. + + # Current date - {IDENTITY}"})); - assert!(ctx.contains("IMPORTANT: this context may or may not be relevant")); + Today's date is 2026-04-12. + + # Model + + You are powered by the model test-model. + + IMPORTANT: this context may or may not be relevant to your tasks. You should + not respond to this context unless it is highly relevant to your task. + "}, + ); } #[test] fn render_user_context_without_instructions_still_carries_date() { - let ctx = render_user_context(&context("", "2026-04-12")); - assert!(ctx.starts_with("")); - assert!(ctx.contains(indoc::indoc! {" - following context: + assert_eq!( + render_user_context(&context("", "2026-04-12")), + indoc! {" + + As you answer the user's questions, you can use the following context: - # Current date + # Current date - Today's date is 2026-04-12. + Today's date is 2026-04-12. - # Model + # Model + + You are powered by the model test-model. + + IMPORTANT: this context may or may not be relevant to your tasks. You should + not respond to this context unless it is highly relevant to your task. + "}, + ); + } + + // ── render_reminder ── + + #[test] + fn render_reminder_includes_every_changed_section_in_order() { + assert_eq!( + render_reminder( + &UserContext::default(), + &context("Project rules.", "2026-04-12") + ), + indoc! {" + + The context from the start of this conversation has changed. Use these values from now on: + + # CLAUDE.md - You are powered by the model test-model. - "})); - assert!(!ctx.contains("# CLAUDE.md")); + Project rules. + + # Current date + + Today's date is 2026-04-12. + + # Model + + You are powered by the model test-model. + "}, + ); + } + + #[test] + fn render_reminder_removed_instructions_revoke_earlier_rules() { + assert_eq!( + render_reminder( + &context("Project rules.", "2026-04-12"), + &context("", "2026-04-12") + ), + indoc! {" + + The context from the start of this conversation has changed. Use these values from now on: + + # CLAUDE.md + + The instruction files were removed, so the earlier instructions no longer apply. + "}, + ); } // ── find_git_root ── #[tokio::test] - async fn find_git_root_inside_repo() { + async fn find_git_root_inside_repo_finds_the_root() { let tmp = tempfile::tempdir().expect("failed to create tempdir"); init_git_repo(tmp.path()); let root = with_isolated_git_env_async(find_git_root(tmp.path())) .await .expect("expected git root"); - // Compare via canonicalize — macOS routes tempdirs through `/private`. + // macOS routes tempdirs through `/private`. let expected = tmp.path().canonicalize().unwrap(); let actual = root.canonicalize().unwrap(); assert_eq!(actual, expected); @@ -574,13 +623,13 @@ mod tests { } #[tokio::test] - async fn find_git_root_outside_repo() { + async fn find_git_root_outside_repo_finds_no_root() { let tmp = tempfile::tempdir().expect("failed to create tempdir"); let root = with_isolated_git_env_async(find_git_root(tmp.path())).await; assert!(root.is_none()); } - // ── constant content ── + // ── sections ── #[test] fn prompt_constants_have_no_leading_whitespace() { @@ -600,54 +649,4 @@ mod tests { ); } } - - // ── helpers ── - - fn cwd() -> PathBuf { - std::env::current_dir().expect("cwd should be available") - } - - const IDENTITY: &str = "You are powered by the model test-model."; - - fn spec(id: &str) -> ModelSpec { - ModelSpec::resolve(id.to_owned()) - } - - fn context(instructions: &str, date: &str) -> UserContext { - UserContext { - instructions: instructions.to_owned(), - date: date.to_owned(), - model: IDENTITY.to_owned(), - } - } - - /// A recording from a machine unlike this one, so a replayed environment is recognizable. - fn recorded_context(cwd: &Path) -> SessionContext { - serde_json::from_value(serde_json::json!({ - "environment": { - "cwd": cwd, - "is_git": false, - "platform": "plan9", - "shell": "rc", - "os_version": "Plan 9 4e", - }, - "user_context": "recorded context", - "seen": { - "instructions": "Recorded rules.", - "date": "2026-04-12", - "model": IDENTITY, - }, - })) - .unwrap() - } - - fn init_git_repo(path: &Path) { - let status = isolated_git(path) - .args(["init"]) - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::null()) - .status() - .expect("failed to spawn git"); - assert!(status.success(), "git init exited with non-zero status"); - } } diff --git a/crates/oxide-code/src/prompt/environment.rs b/crates/oxide-code/src/prompt/environment.rs index 84501b0a..43c4b8df 100644 --- a/crates/oxide-code/src/prompt/environment.rs +++ b/crates/oxide-code/src/prompt/environment.rs @@ -78,7 +78,7 @@ fn normalize_node_platform(os: &str) -> &'static str { fn detect_shell() -> String { let shell = env::string("SHELL").unwrap_or_else(|| "unknown".to_owned()); - std::path::Path::new(&shell) + Path::new(&shell) .file_name() .and_then(|n| n.to_str()) .unwrap_or(&shell) @@ -107,13 +107,10 @@ pub(super) fn current_date() -> String { } fn format_local_date(now: time::OffsetDateTime, offset: time::UtcOffset) -> String { - let date = now.to_offset(offset).date(); - format!( - "{}-{:02}-{:02}", - date.year(), - date.month() as u8, - date.day() - ) + now.to_offset(offset) + .date() + .format(time::macros::format_description!("[year]-[month]-[day]")) + .expect("static `[year]-[month]-[day]` description never fails on a valid date") } // ── Model Identity ── @@ -138,6 +135,10 @@ pub(super) fn model_identity(model: &ModelSpec) -> String { mod tests { use super::*; + fn spec(id: &str) -> ModelSpec { + ModelSpec::resolve(id.to_owned()) + } + // ── Environment::detect ── #[test] @@ -189,14 +190,14 @@ mod tests { // ── normalize_node_platform ── #[test] - fn normalize_node_platform_known_values() { + fn normalize_node_platform_maps_rust_names_to_node_names() { assert_eq!(normalize_node_platform("macos"), "darwin"); assert_eq!(normalize_node_platform("linux"), "linux"); assert_eq!(normalize_node_platform("windows"), "win32"); } #[test] - fn normalize_node_platform_unknown_value() { + fn normalize_node_platform_unrecognized_name_is_unknown() { assert_eq!(normalize_node_platform("haiku"), "unknown"); } @@ -212,11 +213,13 @@ mod tests { // ── detect_os_version ── #[test] - fn detect_os_version_contains_platform_and_version() { + fn detect_os_version_joins_sysname_and_release() { let version = detect_os_version(); - assert!( - version.contains(' '), - "expected 'Platform Version', got: {version:?}", + let info = PlatformInfo::new().unwrap(); + let sysname = info.sysname().to_string_lossy(); + assert_eq!( + version, + format!("{sysname} {}", info.release().to_string_lossy()) ); } @@ -280,10 +283,4 @@ mod tests { "You are powered by the model custom-model-v1.", ); } - - // ── helpers ── - - fn spec(id: &str) -> ModelSpec { - ModelSpec::resolve(id.to_owned()) - } } diff --git a/crates/oxide-code/src/prompt/instructions.rs b/crates/oxide-code/src/prompt/instructions.rs index e24169bf..337452d8 100644 --- a/crates/oxide-code/src/prompt/instructions.rs +++ b/crates/oxide-code/src/prompt/instructions.rs @@ -17,7 +17,7 @@ struct Slot { label: &'static str, } -struct MemoryFile { +struct InstructionFile { path: PathBuf, content: String, label: &'static str, @@ -76,8 +76,6 @@ fn walk_root_to_cwd(root: &Path, cwd: &Path) -> Vec { return vec![root.to_path_buf()]; }; - // cwd == root yields an empty relative path, so the loop below is skipped and only [root] is - // returned. let mut dirs = vec![root.to_path_buf()]; let mut current = root.to_path_buf(); for component in relative.components() { @@ -88,7 +86,7 @@ fn walk_root_to_cwd(root: &Path, cwd: &Path) -> Vec { dirs } -async fn load_files(slots: Vec) -> Vec { +async fn load_files(slots: Vec) -> Vec { let mut files = Vec::new(); for Slot { candidates, label } in slots { @@ -96,7 +94,7 @@ async fn load_files(slots: Vec) -> Vec { if let Ok(content) = fs::read_to_string(&path).await { let content = content.trim().to_owned(); if !content.is_empty() { - files.push(MemoryFile { + files.push(InstructionFile { path, content, label, @@ -110,7 +108,7 @@ async fn load_files(slots: Vec) -> Vec { files } -fn render(files: &[MemoryFile]) -> String { +fn render(files: &[InstructionFile]) -> String { let header = indoc! {" # User instructions @@ -173,7 +171,7 @@ mod tests { // ── candidate_slots ── #[test] - fn candidate_slots_cwd_equals_root() { + fn candidate_slots_at_root_includes_project_and_companion_files() { let root = PathBuf::from("/home/user/project"); let slots = candidate_slots(&root, &root); @@ -187,7 +185,6 @@ mod tests { vec![root.join("CLAUDE.md"), root.join("AGENTS.md")] ); - // Verify the .claude/ companion slot is also present. let claude_dir: Vec<_> = slots .iter() .filter(|s| s.label == "project instructions (.claude/)") @@ -246,14 +243,14 @@ mod tests { // ── walk_root_to_cwd ── #[test] - fn walk_root_to_cwd_same_directory() { + fn walk_root_to_cwd_same_directory_includes_root_only() { let root = PathBuf::from("/repo"); let dirs = walk_root_to_cwd(&root, &root); assert_eq!(dirs, vec![PathBuf::from("/repo")]); } #[test] - fn walk_root_to_cwd_nested() { + fn walk_root_to_cwd_nested_includes_each_directory_in_order() { let root = PathBuf::from("/repo"); let cwd = PathBuf::from("/repo/a/b"); let dirs = walk_root_to_cwd(&root, &cwd); @@ -268,7 +265,7 @@ mod tests { } #[test] - fn walk_root_to_cwd_outside_root_yields_root_only() { + fn walk_root_to_cwd_outside_root_includes_root_only() { let root = PathBuf::from("/repo"); let cwd = PathBuf::from("/other/dir"); let dirs = walk_root_to_cwd(&root, &cwd); @@ -370,12 +367,12 @@ mod tests { #[test] fn render_formats_files_with_header_and_preserves_order() { let files = vec![ - MemoryFile { + InstructionFile { path: PathBuf::from("/home/.claude/CLAUDE.md"), content: "Global rules.".to_owned(), label: "user's global instructions", }, - MemoryFile { + InstructionFile { path: PathBuf::from("/project/CLAUDE.md"), content: "Project rules.".to_owned(), label: "project instructions", @@ -383,31 +380,21 @@ mod tests { ]; let out = render(&files); - assert!(out.starts_with("# User instructions")); - assert!(out.contains("Be sure to adhere to these instructions.")); - assert!(out.contains("Contents of /home/.claude/CLAUDE.md (user's global instructions):")); - assert!(out.contains("Contents of /project/CLAUDE.md (project instructions):")); + assert_eq!( + out, + indoc! {" + # User instructions - let global_pos = out.find("Global rules.").expect("global content missing"); - let project_pos = out.find("Project rules.").expect("project content missing"); - assert!( - global_pos < project_pos, - "global should come before project" - ); - } + Codebase and user instructions are shown below. + Be sure to adhere to these instructions. - #[test] - fn render_single_file() { - let files = vec![MemoryFile { - path: PathBuf::from("/project/CLAUDE.md"), - content: "Only file.".to_owned(), - label: "project instructions", - }]; - let out = render(&files); + Contents of /home/.claude/CLAUDE.md (user's global instructions): + + Global rules. - assert!(out.starts_with("# User instructions")); - // Exactly one "Contents of" block. - assert_eq!(out.matches("Contents of").count(), 1); - assert!(out.contains("Only file.")); + Contents of /project/CLAUDE.md (project instructions): + + Project rules."}, + ); } } From b506276f764293e5a731b60d871b42894986b14a Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Fri, 16 Oct 2026 14:24:41 +0800 Subject: [PATCH 2/3] refactor(model): align imports and drop the getter test --- crates/oxide-code/src/model.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/crates/oxide-code/src/model.rs b/crates/oxide-code/src/model.rs index 6240ecc5..2940981a 100644 --- a/crates/oxide-code/src/model.rs +++ b/crates/oxide-code/src/model.rs @@ -12,7 +12,6 @@ use std::fmt::Write; use serde::{Deserialize, Deserializer, Serialize, Serializer}; use crate::config::{BlockBinding, Effort, ThinkingConfig, ThinkingDisplay}; - pub(crate) use pricing::TokenCostRates; use pricing::{HAIKU_5_5_RATES, OPUS_5_5_RATES, SONNET_5_5_RATES}; @@ -515,12 +514,4 @@ mod tests { assert_eq!(spec(info.id).unsupported_notice(), None, "{}", info.id); } } - - // ── ResolvedModelId ── - - #[test] - fn resolved_model_id_into_inner_returns_wrapped_string() { - let id = ResolvedModelId::new("claude-opus-5-5".to_owned()); - assert_eq!(id.into_inner(), "claude-opus-5-5"); - } } From 84b58f5c09784148da1dd69dec2cad49691e82a6 Mon Sep 17 00:00:00 2001 From: Hakula Chen Date: Fri, 16 Oct 2026 14:24:44 +0800 Subject: [PATCH 3/3] test(cli): share command setup and identify storage failures --- crates/oxide-code/tests/cli.rs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/crates/oxide-code/tests/cli.rs b/crates/oxide-code/tests/cli.rs index dd7a1544..dd6cee74 100644 --- a/crates/oxide-code/tests/cli.rs +++ b/crates/oxide-code/tests/cli.rs @@ -4,10 +4,14 @@ use std::process::{Command, Stdio}; use indoc::indoc; +fn ox() -> Command { + Command::new(env!("CARGO_BIN_EXE_ox")) +} + #[test] -fn list_empty_project() { +fn list_in_empty_project_reports_no_sessions() { let dir = tempfile::tempdir().unwrap(); - let output = Command::new(env!("CARGO_BIN_EXE_ox")) + let output = ox() .arg("--list") .current_dir(dir.path()) .env("XDG_DATA_HOME", dir.path()) @@ -22,7 +26,7 @@ fn list_empty_project() { #[test] fn list_with_unusable_storage_fails() { let file = tempfile::NamedTempFile::new().unwrap(); - let output = Command::new(env!("CARGO_BIN_EXE_ox")) + let output = ox() .arg("--list") .env("XDG_DATA_HOME", file.path()) .output() @@ -32,6 +36,10 @@ fn list_with_unusable_storage_fails() { assert!(output.stdout.is_empty(), "{:?}", output.stdout); let stderr = String::from_utf8(output.stderr).unwrap(); assert!(stderr.starts_with("Error: "), "{stderr}"); + assert!( + stderr.contains(&file.path().display().to_string()), + "{stderr}" + ); } #[test] @@ -48,7 +56,7 @@ fn continue_in_the_repl_unarchives_the_session_with_a_note() { ) .unwrap(); - let output = Command::new(env!("CARGO_BIN_EXE_ox")) + let output = ox() .arg("--no-tui") .arg("--continue") .arg(&session) @@ -80,7 +88,7 @@ fn continue_in_the_repl_unarchives_the_session_with_a_note() { #[test] fn resume_without_a_terminal_fails_before_writing_anything() { let dir = tempfile::tempdir().unwrap(); - let output = Command::new(env!("CARGO_BIN_EXE_ox")) + let output = ox() .arg("--resume") .current_dir(dir.path()) .env("XDG_DATA_HOME", dir.path())