From 90a32b0c2442cddeca6b5ef483af7cdca8eecc52 Mon Sep 17 00:00:00 2001 From: Iurii Bobrykov Date: Sat, 8 Aug 2026 20:55:24 +1200 Subject: [PATCH 1/2] feat: system prompt --- Cargo.lock | 1 + crates/dch-config/src/lib.rs | 388 ++++++++++++- crates/dch-loop/Cargo.toml | 6 +- crates/dch-loop/src/lib.rs | 11 + crates/dch-loop/src/project.rs | 766 +++++++++++++++++++++++++ crates/dch-loop/src/prompt.rs | 442 ++++++++++++++ crates/dch-loop/tests/system_prompt.rs | 29 + 7 files changed, 1627 insertions(+), 16 deletions(-) create mode 100644 crates/dch-loop/src/project.rs create mode 100644 crates/dch-loop/src/prompt.rs create mode 100644 crates/dch-loop/tests/system_prompt.rs diff --git a/Cargo.lock b/Cargo.lock index 0d0a6b0..4db577d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -464,6 +464,7 @@ dependencies = [ "loopctl", "serde", "serde_json", + "tempfile", "thiserror", "tokio", "tracing", diff --git a/crates/dch-config/src/lib.rs b/crates/dch-config/src/lib.rs index 3df237f..9f5cd0c 100644 --- a/crates/dch-config/src/lib.rs +++ b/crates/dch-config/src/lib.rs @@ -136,6 +136,201 @@ pub enum PermissionMode { Interactive, } +/// Which role the agent takes on — *who* it acts as for this session. +/// +/// Each variant selects a distinct body of guidance that shapes how the agent +/// approaches the work — what it optimizes for, what it may edit, and how it +/// sequences exploration and action. It is consumed from +/// [`RunnerConfig::role`] and (de)serialized as its `snake_case` name, +/// matching [`PermissionMode`]'s convention. +/// +/// The role is the *instruction* axis (what the agent is told to do and how to +/// think about the task); it is orthogonal to [`PermissionMode`], which is the +/// *enforcement* axis (whether a side effect is allowed to run), and to the +/// project/tech context, which is the *subject* axis (what stack the agent +/// works on — detected from the repo, overridable in `[project]`). Enforcement +/// is handled by the permission layer at runtime regardless of which role is +/// selected. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default, serde::Deserialize, serde::Serialize)] +#[serde(rename_all = "snake_case")] +pub enum Role { + /// General assistance — PC help, sysadmin, app configuration (the default). + /// + /// Directs the agent to diagnose and act on the user's machine: read + /// logs (`journalctl`, `dmesg`), configure applications, inspect services + /// and processes, run shell commands, and edit config files. Not + /// repo-centric; the explore-the-codebase framing of the coding roles + /// does not apply. dch's primary mode for non-programming work. + #[default] + General, + + /// Implement features and fixes end-to-end. + /// + /// Directs the agent to read enough of the surrounding code to make a + /// correct, idiomatic change, apply it, and verify it with the build or + /// tests. Exploration and reads proceed freely; sizable writes are + /// surfaced as a plan before being applied. + Coding, + + /// Improve structure without changing behavior. + /// + /// Directs the agent to restructure code while preserving observable + /// behavior, re-running the tests to prove nothing regressed. A behavior + /// change forced by the refactor is called out explicitly rather than + /// folded in silently. + Refactor, + + /// Reproduce, isolate, then fix. + /// + /// Directs the agent to reproduce the failure first, form a hypothesis, + /// probe to narrow the cause, and only then apply the smallest correct + /// fix — distinguishing root cause from symptom. Verification confirms the + /// fix and checks for regressions. + Debug, + + /// Read-only critical pass over a diff or area. + /// + /// Directs the agent to inspect and report findings — bugs, smells, risks + /// — without editing source. Suggested fixes are offered as proposals, + /// never applied. No side effects on the codebase. + Review, + + /// Author or revise documentation. + /// + /// Directs the agent to match existing voice and structure, write + /// substantive content rather than filler, and keep code examples + /// runnable. May edit documentation and doc-comments, not source logic. + Docs, + + /// Write and improve tests. + /// + /// Directs the agent to cover behavior rather than implementation, prefer + /// meaningful cases over rote enumeration, and run the suite to confirm + /// green. May edit test files; treats the code under test as read-only + /// context. + Tests, +} + +/// Role body for [`Role::General`]: general assistance and sysadmin work. +pub(crate) const GENERAL_ROLE: &str = "\ +YOUR ROLE: GENERAL ASSISTANCE +- Help with the user's machine: diagnose issues, configure applications, inspect + services and processes, read logs and status, run shell commands. +- Read the evidence before theorizing: check logs (`journalctl`, `dmesg`, + app logs), service status, and config files. Form a hypothesis from what you + observe, then act. +- Prefer the least-invasive change that resolves the issue. Editing a config + line or restarting a service beats reinstalling a package. +- Confirm before destructive or system-wide actions (package removals, force + reloads, edits under `/etc`). State what you intend and why, then act. +- For commands you are unsure of, check `--help` or the man page before + running; a wrong flag on a system tool can be costly."; + +/// Role body for [`Role::Coding`]: implement features end-to-end. +pub(crate) const CODING_ROLE: &str = "\ +YOUR ROLE: IMPLEMENT FEATURES AND FIXES +- Read enough of the surrounding code to make a correct, idiomatic change. +- Make the change with Edit or MultiEdit so the diff is visible and reviewable; + never mutate files with shell scripting (sed, awk, inline python) — those + edits are invisible in review. +- Verify the change: run the build and the relevant tests (the detected + commands, if any, are listed in the prompt). Treat a green check as the + signal the task is done, not the edit itself. +- Keep edits targeted. A single Edit should carry one intent; split work that + does several things into several edits. +- For unfamiliar or complex operations, look up the established pattern in the + repo (or via WebFetch) before inventing a new one."; + +/// Role body for [`Role::Refactor`]: restructure without behavior change. +pub(crate) const REFACTOR_ROLE: &str = "\ +YOUR ROLE: IMPROVE STRUCTURE WITHOUT CHANGING BEHAVIOR +- First characterize the behavior you must preserve: read the code and its + tests. The tests are the contract — a successful refactor leaves them green. +- Restructure in reviewable steps. After each step, run the tests; if any + regress, you have changed behavior, not just structure. +- If a behavior change is forced by the refactor, stop and call it out + explicitly rather than folding it in silently. Refactors and behavior changes + do not mix in one change. +- Prefer the smallest mechanical move that clarifies the code. Rename, extract, + inline — one kind of step at a time is easier to review than a mixed rewrite."; + +/// Role body for [`Role::Debug`]: reproduce, isolate, then fix. +pub(crate) const DEBUG_ROLE: &str = "\ +YOUR ROLE: REPRODUCE, ISOLATE, AND FIX +- Reproduce the failure first. A reproducible failure is fixable; an + un-reproduced one is a guess. Capture the exact command, input, and observed + output before theorizing. +- Form one hypothesis and probe it with Read, Grep, and Bash (logs, verbose + flags, a minimal repro script). Narrow the cause before touching a fix. +- Fix the root cause, not the symptom. The smallest change that removes the + failure mode at its source is usually right; papering over a symptom moves + the bug elsewhere. +- After fixing, confirm the repro now passes and run the surrounding tests to + catch regressions. Distinguish clearly between what you observed, what you + inferred, and what you changed."; + +/// Role body for [`Role::Review`]: read-only critical pass. +pub(crate) const REVIEW_ROLE: &str = "\ +YOUR ROLE: REVIEW AND REPORT — DO NOT EDIT SOURCE +- Treat this as a read-only pass. Inspect the diff or area with Read, Grep, and + Bash (git, tests) and report findings; do not apply changes to source. +- Organize findings by severity: correctness bugs first, then risks and design + smells, then style. For each, name the file and line and explain the concern + concretely. +- Offer suggested fixes as proposals (\"consider extracting X\", \"this could + overflow if N < 0\"), not as applied edits. Let the user decide what to act + on. +- Call out anything you could not verify. A reviewer's value is honesty about + what was checked and what wasn't."; + +/// Role body for [`Role::Docs`]: author or revise documentation. +pub(crate) const DOCS_ROLE: &str = "\ +YOUR ROLE: WRITE OR REVISE DOCUMENTATION +- Match the existing voice, structure, and formatting of the docs around you. + Consistency with neighbors reads as one coherent document; a clashing style + reads as noise. +- Write substantive content. Document the why and the how-to, with runnable + examples; avoid filler lines that exist only to pad length. +- Keep code examples accurate and runnable. If you cannot verify a command or + snippet, say so rather than presenting it as tested. +- You may edit documentation files and doc-comments. Do not change source + logic under the documentation — if the docs and the code disagree, flag the + discrepancy rather than silently \"fixing\" one to match the other."; + +/// Role body for [`Role::Tests`]: write and improve tests. +pub(crate) const TESTS_ROLE: &str = "\ +YOUR ROLE: WRITE AND IMPROVE TESTS +- Cover behavior, not implementation. A test that pins a public outcome + survives refactors; one that asserts private call shape breaks under + harmless restructuring. +- Prefer a few meaningful cases (including the edge: empty, off-by-one, the + bug being fixed) over rote enumeration of identical inputs. Each test should + fail for one identifiable reason if it fails. +- Run the suite to confirm green after writing. A test that does not yet pass + is a finding, not a deliverable — report it and its cause. +- You may edit test files. Treat the code under test as read-only context; if + the code itself is wrong, say so rather than weakening a test to match it."; + +impl Role { + /// The role's system-prompt prose body. + /// + /// Each variant returns its own [`Role`]-specific guidance. The runner + /// prepends the shared agent discipline, the detected tech profile, and + /// the per-tool fragments; this is only the role-specific portion. + #[must_use] + pub const fn system_prompt(self) -> &'static str { + match self { + Role::General => GENERAL_ROLE, + Role::Coding => CODING_ROLE, + Role::Refactor => REFACTOR_ROLE, + Role::Debug => DEBUG_ROLE, + Role::Review => REVIEW_ROLE, + Role::Docs => DOCS_ROLE, + Role::Tests => TESTS_ROLE, + } + } +} + /// Errors arising while loading configuration. /// /// Returned by [`DchConfig::load`] and [`DchConfig::load_from_dir`]. A missing @@ -186,6 +381,14 @@ pub struct DchConfig { #[serde(default)] pub runner: RunnerConfig, + /// Project / tech-stack context. + /// + /// Optional overrides for the auto-detected tech profile (language, build + /// and test commands, conventions). When a field is `None`, the runner + /// uses the value it detected from the repo. See [`ProjectConfig`]. + #[serde(default)] + pub project: ProjectConfig, + /// Telemetry / logging settings. /// /// Log level and output format. See [`TelemetryConfig`]. @@ -311,12 +514,132 @@ pub struct RunnerConfig { /// [`PermissionMode::Auto`]. See [`PermissionMode`]. pub permission_mode: PermissionMode, - /// Optional override for the generated system prompt. + /// Which role the agent takes on for this session. /// - /// When set, replaces the built-in system prompt entirely. Defaults to - /// `None`, meaning the default prompt is generated. Carried into - /// [`DchConfig::to_session_config`]. - pub system_prompt: Option, + /// Selects the guidance the agent receives about how to approach the work. + /// Defaults to [`Role::General`]. See [`Role`]. Consumed by the runner + /// (which composes the full prompt from the role's prose, the detected + /// tech stack, and the per-tool fragments); it is **not** carried by + /// [`DchConfig::to_session_config`], because composing the prompt needs + /// the tool registry and the repo root, which the config layer does not + /// own. + #[serde(default)] + pub role: Role, + + /// Per-role prose overrides. + /// + /// Each entry replaces the built-in prose of one [`Role`] (the shared + /// discipline, detected tech stack, and per-tool fragments still append). + /// A role with no entry uses its built-in [`Role::system_prompt`]. This is + /// the escape hatch for users who want to customize how a specific role + /// instructs the agent without editing the binary. Loaded from + /// `[[runner.role_overrides]]`. + #[serde(default)] + pub role_overrides: Vec, +} + +/// A user-supplied replacement for one [`Role`]'s built-in prose. +/// +/// The runner looks up the selected [`Role`] in [`RunnerConfig::role_overrides`] +/// and, if present, uses `prompt` in place of [`Role::system_prompt`]. The +/// shared discipline, tech profile, and per-tool fragments still append — only +/// the role-specific body is replaced. +#[derive(Debug, Clone, PartialEq, Eq, serde::Deserialize, serde::Serialize)] +pub struct RoleOverride { + /// The role whose built-in prose this entry replaces. + /// + /// Matched against the selected [`RunnerConfig::role`]; only an entry + /// whose role matches the selection takes effect. + pub role: Role, + + /// The replacement prose. Used verbatim in place of + /// [`Role::system_prompt`] for this role. + pub prompt: String, +} + +impl RunnerConfig { + /// Look up a user override for `role`, if any. + /// + /// Returns the override's prompt when an entry for `role` exists in + /// [`Self::role_overrides`], otherwise `None` (meaning: use the role's + /// built-in [`Role::system_prompt`]). + #[must_use] + pub fn role_override(&self, role: Role) -> Option<&str> { + self.role_overrides + .iter() + .find(|o| o.role == role) + .map(|o| o.prompt.as_str()) + } +} + +/// One technology in a project, with its toolchain and conventions. +/// +/// Most real projects are polyglot — a Rust core, a `TypeScript` frontend, a +/// Python tooling script — and each language has its own build/test/lint +/// commands and its own conventions. This struct captures one such language; +/// [`ProjectConfig`] holds a `Vec` for all of them. +/// +/// Every field except `language` is optional: set only what detection got +/// wrong or can't infer. Loaded from the `[[project.techs]]` array-of-tables. +#[derive(Debug, Clone, Default, PartialEq, Eq, serde::Deserialize, serde::Serialize)] +pub struct Tech { + /// The language this entry describes, e.g. `"rust"` or `"cpp"`. + /// + /// Matches against the detected language so the runner can merge detected + /// and configured entries by language. Required — an entry with no + /// language names nothing. + pub language: String, + + /// Command that builds this language's code. + /// + /// Overrides the detected build command for this language, e.g. + /// `"cargo build"`. `None` keeps the detected value (or leaves it empty + /// when detection found none). + pub build: Option, + + /// Command that runs this language's tests. + /// + /// Overrides the detected test command, e.g. `"cargo test"`. `None` + /// keeps the detected value. + pub test: Option, + + /// Command that lints this language's code. + /// + /// Overrides the detected lint command, e.g. `"cargo clippy"`. `None` + /// keeps the detected value. + pub lint: Option, + + /// Free-form conventions for this language: style rules, module layout, + /// anything detection can't capture. Appended to the prompt verbatim under + /// this language's section. + pub conventions: Option, +} + +/// Project / tech-stack overrides for the auto-detected profile. +/// +/// Polyglot projects set `[[project.techs]]` once per language; the runner +/// merges each with its detected counterpart by language (set fields override, +/// configured languages detection missed are appended, detected languages the +/// config doesn't mention are kept). `conventions` holds project-wide +/// conventions that span all languages (commit format, branch policy). +/// +/// Loaded from the `[project]` table. Absent entirely when the user relies on +/// auto-detection. +#[derive(Debug, Clone, Default, serde::Deserialize, serde::Serialize)] +#[serde(default)] +pub struct ProjectConfig { + /// Per-language tech entries. + /// + /// One `[[project.techs]]` table per language the user wants to declare or + /// override. The runner merges each with its detected counterpart by + /// language; entries for languages detection missed are appended. + pub techs: Vec, + + /// Free-form project-wide conventions that apply across all languages. + /// + /// Prose for anything detection can't capture: commit-message format, + /// branch policy, where new modules go. Appended to the prompt verbatim. + pub conventions: Option, } /// Telemetry / logging settings. @@ -370,7 +693,8 @@ impl Default for RunnerConfig { auto_compact: true, compact_threshold: 80, permission_mode: PermissionMode::default(), - system_prompt: None, + role: Role::default(), + role_overrides: Vec::new(), } } } @@ -449,7 +773,7 @@ impl DchConfig { #[must_use] pub fn to_session_config(&self) -> loopctl::config::SessionConfig { loopctl::config::SessionConfig { - system_prompt: self.runner.system_prompt.clone(), + system_prompt: None, context_window: self.api.context_window, compact_threshold: self.runner.compact_threshold.min(100), auto_compact: self.runner.auto_compact, @@ -524,7 +848,11 @@ max_turns = 100 auto_compact = false compact_threshold = 75 permission_mode = "accept_edits" -system_prompt = "You are a careful coding assistant." +role = "coding" + +[[runner.role_overrides]] +role = "coding" +prompt = "You are a careful coding assistant." [telemetry] level = "debug" @@ -573,8 +901,9 @@ json_logs = true assert!(!c.runner.auto_compact); assert_eq!(c.runner.compact_threshold, 75); assert_eq!(c.runner.permission_mode, PermissionMode::AcceptEdits); + assert_eq!(c.runner.role, Role::Coding); assert_eq!( - c.runner.system_prompt.as_deref(), + c.runner.role_override(Role::Coding), Some("You are a careful coding assistant.") ); @@ -641,10 +970,7 @@ json_logs = true assert!(!sc.auto_compact); assert_eq!(sc.compact_threshold, 75); - assert_eq!( - sc.system_prompt.as_deref(), - Some("You are a careful coding assistant.") - ); + assert!(sc.system_prompt.is_none()); assert_eq!(sc.context_window, 128_000); } @@ -731,9 +1057,41 @@ json_logs = true } #[test] - fn test_to_session_config_system_prompt_none_round_trips() { + fn default_has_no_role_overrides_and_session_prompt_is_none() { let c = DchConfig::default(); - assert!(c.runner.system_prompt.is_none()); + assert!( + c.runner.role_overrides.is_empty(), + "no overrides by default" + ); + assert!(c.runner.role_override(Role::Coding).is_none()); assert!(c.to_session_config().system_prompt.is_none()); } + + #[test] + fn role_default_is_general() { + assert_eq!(Role::default(), Role::General); + } + + #[test] + fn runner_config_default_carries_general_role() { + assert_eq!(RunnerConfig::default().role, Role::General); + } + + #[test] + fn role_round_trips_through_toml_as_snake_case() { + let tmp = tempfile::TempDir::new().unwrap(); + write_config(tmp.path(), "config.toml", "[runner]\nrole = \"debug\"\n"); + let c = DchConfig::load_from_dir(tmp.path()).unwrap(); + assert_eq!(c.runner.role, Role::Debug); + + let serialized = toml::to_string(&RunnerConfig { + role: Role::Refactor, + ..RunnerConfig::default() + }) + .unwrap(); + assert!( + serialized.contains("role = \"refactor\""), + "snake_case serialization: {serialized}" + ); + } } diff --git a/crates/dch-loop/Cargo.toml b/crates/dch-loop/Cargo.toml index f733699..093224c 100644 --- a/crates/dch-loop/Cargo.toml +++ b/crates/dch-loop/Cargo.toml @@ -7,7 +7,7 @@ license.workspace = true description = "Composition layer for dch: loop, provider factory, system prompt, observers" [dependencies] -loopctl = { workspace = true, features = ["openai", "anthropic", "gemini"] } +loopctl = { workspace = true, features = ["ollama", "openai", "anthropic", "gemini"] } dch-tools = { workspace = true } dch-config = { workspace = true } tokio = { workspace = true } @@ -16,5 +16,9 @@ serde_json = { workspace = true } thiserror = { workspace = true } tracing = { workspace = true } +[dev-dependencies] +tempfile = { workspace = true } +loopctl = { workspace = true, features = ["testing"] } + [lints] workspace = true diff --git a/crates/dch-loop/src/lib.rs b/crates/dch-loop/src/lib.rs index 7ac0053..793dba4 100644 --- a/crates/dch-loop/src/lib.rs +++ b/crates/dch-loop/src/lib.rs @@ -3,8 +3,19 @@ #![warn(missing_docs)] pub mod error; +pub mod project; +pub mod prompt; pub mod provider; pub use dch_config::{ApiConfig, ApiType, DchConfigError}; pub use error::RunnerError; +pub use project::MessageAnalysis; +pub use project::TechProfile; +pub use project::analyze_message; +pub use project::detect_tech_stack; +pub use project::merge_by_language; +pub use project::render_techs; +pub use prompt::build_system_prompt; +pub use prompt::with_context; +pub use prompt::with_role; pub use provider::create_client; diff --git a/crates/dch-loop/src/project.rs b/crates/dch-loop/src/project.rs new file mode 100644 index 0000000..cce140d --- /dev/null +++ b/crates/dch-loop/src/project.rs @@ -0,0 +1,766 @@ +//! Project / tech-stack detection and merging. +//! +//! Infers one [`TechProfile`] per detected language (a polyglot repo yields +//! several) from the repo's marker files, then merges them with the user's +//! `[project]` overrides. The runner injects the rendered profile into the +//! system prompt so the agent knows which stacks it is working on. + +use std::path::Path; + +use dch_config::ProjectConfig; +use dch_config::Role; +use dch_config::Tech; +use loopctl::api::ApiClient; +use loopctl::message::Message; +use loopctl::structured::StructuredError; +use loopctl::structured::StructuredOutput; +use loopctl::structured::request_structured; +use serde::Deserialize; +use serde::Serialize; + +/// A detected (or configured) description of one language in the project. +/// +/// Built by [`detect_tech_stack`] from a marker file, then refined by +/// [`merge_by_language`] with the matching [`Tech`] from `[project].techs`. +/// A polyglot repo has several of these; the runner renders them all. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct TechProfile { + /// The language this entry describes. + /// + /// Lowercased marker-derived name, e.g. `"rust"`, `"typescript"`, + /// `"make"`. Used as the merge key against configured [`Tech`] entries and + /// rendered as the section header. Never empty for a detected profile. + pub language: String, + + /// Command that builds this language's code. + /// + /// Marker-derived, e.g. `"cargo build"`. Empty when the marker implies no + /// build step or the user left the override unset. + pub build: String, + + /// Command that runs this language's tests. + /// + /// Marker-derived, e.g. `"cargo test"`. Empty when the marker implies no + /// test step or the user left the override unset. + pub test: String, + + /// Command that lints this language's code. + /// + /// Marker-derived, e.g. `"cargo clippy"`. Empty when the marker implies no + /// lint step or the user left the override unset. + pub lint: String, + + /// Free-form conventions prose for this language. + /// + /// Carried from a configured [`Tech`] override's `conventions` field; + /// detection never sets it. Empty when the user supplied none. + pub conventions: String, +} + +impl TechProfile { + /// Apply the set fields of a [`Tech`] override onto this profile. + /// + /// Fields the user left `None` keep the detected value, so a `[project]` + /// entry acts as a partial override. `language` is never overridden here — + /// matching is by language, so the identity stays the detected one. + pub fn apply_override(&mut self, tech: &Tech) { + if let Some(build) = &tech.build { + self.build.clone_from(build); + } + if let Some(test) = &tech.test { + self.test.clone_from(test); + } + if let Some(lint) = &tech.lint { + self.lint.clone_from(lint); + } + if let Some(conv) = &tech.conventions { + self.conventions.clone_from(conv); + } + } +} + +/// One marker file and the profile it implies. +/// +/// A row in [`MARKERS`]: the presence of `file` at a repo root implies the +/// language and the default build/test/lint commands. Detection is a best +/// guess — a marker doesn't authoritatively determine the language (a +/// `Makefile` could belong to any language's task runner); the `[project]` +/// override exists to correct wrong guesses. +struct Marker { + /// Filename (or glob) to look for at the repo root, e.g. `"Cargo.toml"` + /// or `"*.csproj"`. + /// + /// Compared literally for fixed names; a `*` triggers a glob match via + /// [`has_glob_match`]. + file: &'static str, + + /// The language name this marker implies. + /// + /// A best-guess label like `"rust"`, `"make"`, `"csharp"`; used as the + /// merge key and the rendered header. + language: &'static str, + + /// Default build command for this language. + /// + /// Copied into [`TechProfile::build`] on detection; overridable. + build: &'static str, + + /// Default test command for this language. + /// + /// Copied into [`TechProfile::test`] on detection; overridable. + test: &'static str, + + /// Default lint command for this language. + /// + /// Copied into [`TechProfile::lint`] on detection; overridable. + lint: &'static str, +} + +/// The marker files `detect_tech_stack` recognizes, in detection order. +/// +/// Order is the tie-breaker only when two markers of the *same* language are +/// both present (e.g. `pyproject.toml` and `setup.py`); across languages, all +/// matches are returned. Marker-to-language is a heuristic, not authoritative +/// — a user corrects a wrong guess via `[project].techs`. Add a language by +/// adding a row here. +const MARKERS: &[Marker] = &[ + Marker { + file: "Cargo.toml", + language: "rust", + build: "cargo build", + test: "cargo test", + lint: "cargo clippy", + }, + Marker { + file: "go.mod", + language: "go", + build: "go build ./...", + test: "go test ./...", + lint: "go vet ./...", + }, + Marker { + file: "package.json", + language: "typescript", + build: "npm run build", + test: "npm test", + lint: "npm run lint", + }, + Marker { + file: "pyproject.toml", + language: "python", + build: "pip install -e .", + test: "pytest", + lint: "ruff check", + }, + Marker { + file: "setup.py", + language: "python", + build: "pip install -e .", + test: "pytest", + lint: "ruff check", + }, + Marker { + file: "pom.xml", + language: "java", + build: "mvn compile", + test: "mvn test", + lint: "mvn checkstyle:check", + }, + Marker { + file: "build.gradle", + language: "java", + build: "gradle build", + test: "gradle test", + lint: "gradle check", + }, + Marker { + file: "build.gradle.kts", + language: "kotlin", + build: "gradle build", + test: "gradle test", + lint: "gradle check", + }, + Marker { + file: "CMakeLists.txt", + language: "cpp", + build: "cmake --build build", + test: "ctest --test-dir build", + lint: "cmake --build build --target lint", + }, + Marker { + file: "Makefile", + language: "make", + build: "make", + test: "make test", + lint: "make lint", + }, + Marker { + file: "Mix.exs", + language: "elixir", + build: "mix compile", + test: "mix test", + lint: "mix credo", + }, + Marker { + file: "dub.json", + language: "d", + build: "dub build", + test: "dub test", + lint: "dub lint", + }, + Marker { + file: "*.csproj", + language: "csharp", + build: "dotnet build", + test: "dotnet test", + lint: "dotnet format --verify-no-changes", + }, +]; + +/// Detect every tech stack present at `root`, one [`TechProfile`] per language. +/// +/// All matching markers contribute; duplicates by language are collapsed +/// (first marker for a language wins, so `pyproject.toml` beats `setup.py`). +/// A root with no recognized marker yields an empty `Vec`; the caller then +/// relies on the agent exploring the repo, or on `[project].techs` overrides +/// merged in afterward by [`merge_by_language`]. +/// +/// `*.csproj` (C#) is a glob rather than a fixed filename; it matches any +/// direct child of `root` ending in `.csproj`. +#[must_use] +pub fn detect_tech_stack(root: &Path) -> Vec { + let mut profiles: Vec = Vec::new(); + for marker in MARKERS { + let matched = if marker.file.contains('*') { + has_glob_match(root, marker.file) + } else { + root.join(marker.file).exists() + }; + if matched + && !profiles.iter().any(|p| p.language == marker.language) + && let Some(profile) = language_profile(marker.language) + { + profiles.push(profile); + } + } + profiles +} + +/// Look up the default [`TechProfile`] for a language name. +/// +/// Shared by detection ([`detect_tech_stack`]) and inference +/// ([`infer_from_message`]) so they agree on what a language means — +/// "rust" resolves to the same build/test/lint whether it was found by a +/// `Cargo.toml` marker or inferred from a "build a Rust CLI" message. +/// Returns `None` for an unknown language (e.g. an inferred name with no +/// marker row); the caller skips it. +fn language_profile(language: &str) -> Option { + MARKERS + .iter() + .find(|m| m.language == language) + .map(|m| TechProfile { + language: m.language.to_string(), + build: m.build.to_string(), + test: m.test.to_string(), + lint: m.lint.to_string(), + conventions: String::new(), + }) +} + +/// The model's analysis of a first user message: its intent and its stack. +/// +/// Captured pre-session via [`analyze_message`] so the runner can resolve the +/// role (suggesting a switch if the configured role doesn't fit) and assemble +/// the tech profile (unioning analyzed languages with filesystem detection) +/// *before* the loopctl session is constructed. Both fields degrade gracefully +/// — `None` role / empty languages leave the configured role and filesystem +/// detection in charge. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct MessageAnalysis { + /// The role that best fits the user's intent. + /// + /// `Some(Role::Coding)` for "create a CLI", `Some(Role::General)` for "how + /// does systemd work?", `None` when unclear. Compared against the + /// configured role by the runner; a mismatch prompts a TUI suggestion or + /// is respected in headless. + pub suggested_role: Option, + + /// Languages the message implies. + /// + /// E.g. `["rust"]`, or `["rust", "typescript"]` for polyglot intent. Empty + /// for non-code messages. Unioned with filesystem detection by the runner. + pub languages: Vec, +} + +impl StructuredOutput for MessageAnalysis { + fn name() -> &'static str { + "message_analysis" + } + + fn schema() -> serde_json::Value { + serde_json::json!({ + "type": "object", + "properties": { + "suggested_role": { + "type": ["string", "null"], + "enum": [ + "general", "coding", "refactor", "debug", + "review", "docs", "tests", null + ], + "description": "The role that best fits what the user wants to do. null if unclear." + }, + "languages": { + "type": "array", + "items": { "type": "string" }, + "description": "Programming languages the message implies (e.g. \"rust\", \"go\", \"python\", \"typescript\", \"cpp\", \"csharp\", \"java\", \"kotlin\", \"bash\"). Empty array for non-code questions." + } + }, + "required": ["suggested_role", "languages"], + "additionalProperties": false + }) + } +} + +impl MessageAnalysis { + /// Build [`TechProfile`]s for the analyzed languages. + /// + /// Each language resolves via the shared per-language lookup (the same one + /// detection uses, so "rust" means the same commands whether found by a + /// `Cargo.toml` marker or inferred from a message). Unknown languages are + /// skipped rather than producing empty or panicking profiles. + #[must_use] + pub fn tech_profiles(&self) -> Vec { + self.languages + .iter() + .filter_map(|lang| language_profile(lang)) + .collect() + } +} + +/// Analyze the first user message for intent (→ suggested role) and stack. +/// +/// One LLM call via loopctl's [`StructuredOutput`] machinery, using the same +/// model the session will use (the `client` is built pre-session). The result +/// is typed — no free-text parsing. See [`MessageAnalysis`] for the fields. +/// +/// # Errors +/// +/// Returns the loopctl [`StructuredError`] if the call fails or the response +/// does not match the schema. The caller treats any error as "no analysis" — +/// keep the configured role and rely on filesystem detection only — rather +/// than failing the session. +pub async fn analyze_message( + client: &dyn ApiClient, + first_message: &str, +) -> Result { + let messages = vec![Message::user(first_message)]; + let system = Some( + "Classify the user's first message. Identify the role that best fits \ + their intent (general assistance, coding, refactor, debug, review, \ + docs, or tests — null if unclear) and any programming languages it \ + implies (empty array for non-code questions)." + .to_string(), + ); + request_structured::(client, messages, system).await +} + +/// True if any direct child of `root` matches the single-segment `glob` +/// (e.g. `"*.csproj"`). +fn has_glob_match(root: &Path, glob: &str) -> bool { + let Ok(entries) = std::fs::read_dir(root) else { + return false; + }; + for entry in entries.flatten() { + if let Some(name) = entry.file_name().to_str() + && dch_tools::walk::wildcard_match(name, glob) + { + return true; + } + } + false +} + +/// Merge detected profiles with the user's `[project]` overrides by language. +/// +/// For each detected profile whose language matches a configured [`Tech`], +/// that tech's set fields override the detected ones. Configured techs whose +/// language was not detected are appended (the user adds a language detection +/// missed). Detected techs the config doesn't mention are kept as-is. Order is +/// detected-first, then appended config-only techs. +#[must_use] +pub fn merge_by_language( + mut detected: Vec, + config: &ProjectConfig, +) -> Vec { + for tech in &config.techs { + if let Some(profile) = detected.iter_mut().find(|p| p.language == tech.language) { + profile.apply_override(tech); + } else { + detected.push(TechProfile::from_tech(tech)); + } + } + detected +} + +impl TechProfile { + /// Build a profile from a configured [`Tech`] (for languages detection + /// missed entirely). + fn from_tech(tech: &Tech) -> TechProfile { + TechProfile { + language: tech.language.clone(), + build: tech.build.clone().unwrap_or_default(), + test: tech.test.clone().unwrap_or_default(), + lint: tech.lint.clone().unwrap_or_default(), + conventions: tech.conventions.clone().unwrap_or_default(), + } + } +} + +/// Render a polyglot tech list plus project-wide conventions as the prose +/// block injected into the system prompt. +/// +/// Returns an empty string when there are no techs and no project conventions, +/// so the caller can skip the section entirely. +#[must_use] +pub fn render_techs(techs: &[TechProfile], project_conventions: Option<&str>) -> String { + let no_techs = techs.iter().all(all_fields_empty); + let no_conventions = project_conventions.is_none_or(str::is_empty); + if no_techs && no_conventions { + return String::new(); + } + let mut out = String::from("PROJECT"); + for tech in techs { + if all_fields_empty(tech) { + continue; + } + out.push_str("\n\n- Language: "); + out.push_str(&tech.language); + if !tech.build.is_empty() { + out.push_str("\n Build: "); + out.push_str(&tech.build); + } + if !tech.test.is_empty() { + out.push_str("\n Test: "); + out.push_str(&tech.test); + } + if !tech.lint.is_empty() { + out.push_str("\n Lint: "); + out.push_str(&tech.lint); + } + if !tech.conventions.is_empty() { + out.push_str("\n Conventions: "); + out.push_str(&tech.conventions); + } + } + if let Some(conv) = project_conventions + && !conv.is_empty() + { + out.push_str("\n\nProject conventions: "); + out.push_str(conv); + } + out +} + +/// True when every field of `tech` other than `language` is empty. +/// +/// A profile that is bare (only `language` set, no commands or conventions) +/// contributes nothing actionable to the prompt, so [`render_techs`] skips it. +/// This means a `[project].techs` entry that names only a language — e.g. to +/// nudge detection toward a language with no marker — does not appear in the +/// rendered PROJECT section on its own; pair it with at least one command or +/// convention to make it render. +fn all_fields_empty(tech: &TechProfile) -> bool { + tech.build.is_empty() + && tech.test.is_empty() + && tech.lint.is_empty() + && tech.conventions.is_empty() +} + +#[cfg(test)] +#[allow( + clippy::expect_used, + clippy::unwrap_used, + clippy::panic, + clippy::missing_panics_doc, + clippy::missing_errors_doc, + clippy::indexing_slicing +)] +mod tests { + use super::*; + use dch_config::ProjectConfig; + use dch_config::Tech; + + #[test] + fn empty_root_yields_no_profiles() { + let tmp = tempfile::TempDir::new().unwrap(); + assert!(detect_tech_stack(tmp.path()).is_empty()); + assert!(render_techs(&[], None).is_empty()); + } + + #[test] + fn cargo_toml_detects_rust_with_lint() { + let tmp = tempfile::TempDir::new().unwrap(); + std::fs::write(tmp.path().join("Cargo.toml"), "").unwrap(); + let profiles = detect_tech_stack(tmp.path()); + assert_eq!(profiles.len(), 1); + let p = &profiles[0]; + assert_eq!(p.language, "rust"); + assert_eq!(p.build, "cargo build"); + assert_eq!(p.test, "cargo test"); + assert_eq!(p.lint, "cargo clippy"); + } + + #[test] + fn polyglot_repo_detects_each_language() { + let tmp = tempfile::TempDir::new().unwrap(); + std::fs::write(tmp.path().join("Cargo.toml"), "").unwrap(); + std::fs::write(tmp.path().join("package.json"), "").unwrap(); + std::fs::write(tmp.path().join("go.mod"), "").unwrap(); + let profiles = detect_tech_stack(tmp.path()); + let langs: Vec<&str> = profiles.iter().map(|p| p.language.as_str()).collect(); + assert!(langs.contains(&"rust"), "{langs:?}"); + assert!(langs.contains(&"typescript"), "{langs:?}"); + assert!(langs.contains(&"go"), "{langs:?}"); + assert_eq!(langs.len(), 3, "one profile per language"); + } + + #[test] + fn duplicate_marker_for_same_language_dedups() { + // pyproject.toml and setup.py both imply python; only one profile. + let tmp = tempfile::TempDir::new().unwrap(); + std::fs::write(tmp.path().join("pyproject.toml"), "").unwrap(); + std::fs::write(tmp.path().join("setup.py"), "").unwrap(); + let profiles = detect_tech_stack(tmp.path()); + let py = profiles.iter().filter(|p| p.language == "python").count(); + assert_eq!(py, 1, "python should appear once: {profiles:?}"); + } + + #[test] + fn csproj_glob_detects_csharp() { + let tmp = tempfile::TempDir::new().unwrap(); + std::fs::write(tmp.path().join("App.csproj"), "").unwrap(); + let profiles = detect_tech_stack(tmp.path()); + assert_eq!(profiles[0].language, "csharp"); + } + + #[test] + fn has_glob_match_finds_matching_child() { + let tmp = tempfile::TempDir::new().unwrap(); + std::fs::write(tmp.path().join("App.csproj"), "").unwrap(); + assert!(has_glob_match(tmp.path(), "*.csproj")); + } + + #[test] + fn has_glob_match_false_when_no_child_matches() { + let tmp = tempfile::TempDir::new().unwrap(); + std::fs::write(tmp.path().join("README.md"), "").unwrap(); + std::fs::write(tmp.path().join("Cargo.toml"), "").unwrap(); + assert!(!has_glob_match(tmp.path(), "*.csproj")); + } + + #[test] + fn has_glob_match_finds_needle_among_other_entries() { + let tmp = tempfile::TempDir::new().unwrap(); + std::fs::write(tmp.path().join("README.md"), "").unwrap(); + std::fs::create_dir_all(tmp.path().join("src")).unwrap(); + std::fs::write(tmp.path().join("Lib.csproj"), "").unwrap(); + assert!( + has_glob_match(tmp.path(), "*.csproj"), + "should find the .csproj among the other entries" + ); + } + + #[test] + fn has_glob_match_nonexistent_root_returns_false() { + // read_dir fails on a path that doesn't exist; the function must + // return false rather than panicking. This path is only reachable + // via a direct call — detect_tech_stack probes real tempdirs. + assert!(!has_glob_match( + Path::new("/no/such/dir/anywhere"), + "*.csproj" + )); + } + + #[test] + fn merge_overrides_matching_language_and_appends_new() { + let detected = vec![ + TechProfile { + language: "rust".to_string(), + build: "cargo build".to_string(), + test: "cargo test".to_string(), + lint: "cargo clippy".to_string(), + conventions: String::new(), + }, + TechProfile { + language: "typescript".to_string(), + build: "npm run build".to_string(), + test: "npm test".to_string(), + lint: String::new(), + conventions: String::new(), + }, + ]; + let config = ProjectConfig { + techs: vec![ + // Override rust's build only; keep detected test/lint. + Tech { + language: "rust".to_string(), + build: Some("cargo build --release".to_string()), + test: None, + lint: None, + conventions: Some("no unwrap".to_string()), + }, + // Add a language detection missed. + Tech { + language: "bash".to_string(), + build: None, + test: Some("bats".to_string()), + lint: Some("shellcheck".to_string()), + conventions: None, + }, + ], + conventions: Some("conventional commits".to_string()), + }; + let merged = merge_by_language(detected, &config); + let rust = merged.iter().find(|p| p.language == "rust").unwrap(); + assert_eq!(rust.build, "cargo build --release", "override applied"); + assert_eq!(rust.test, "cargo test", "unset field kept detected"); + assert_eq!(rust.lint, "cargo clippy", "unset field kept detected"); + assert_eq!(rust.conventions, "no unwrap"); + let bash = merged.iter().find(|p| p.language == "bash").unwrap(); + assert_eq!(bash.test, "bats", "appended config-only tech"); + assert_eq!(bash.lint, "shellcheck"); + assert_eq!(merged.len(), 3, "rust + typescript + bash"); + } + + #[test] + fn render_lists_each_tech_and_project_conventions() { + let techs = vec![ + TechProfile { + language: "rust".to_string(), + build: "cargo build".to_string(), + test: "cargo test".to_string(), + lint: "cargo clippy".to_string(), + conventions: String::new(), + }, + TechProfile { + language: "bash".to_string(), + build: String::new(), + test: "bats".to_string(), + lint: String::new(), + conventions: String::new(), + }, + ]; + let rendered = render_techs(&techs, Some("conventional commits")); + assert!(rendered.contains("PROJECT"), "{rendered}"); + assert!(rendered.contains("Language: rust"), "{rendered}"); + assert!(rendered.contains("Build: cargo build"), "{rendered}"); + assert!(rendered.contains("Lint: cargo clippy"), "{rendered}"); + assert!(rendered.contains("Language: bash"), "{rendered}"); + assert!(rendered.contains("Test: bats"), "{rendered}"); + assert!( + !rendered.contains("Build: ") || rendered.matches("Build: ").count() == 1, + "bash omits empty build: {rendered}" + ); + assert!( + rendered.contains("Project conventions: conventional commits"), + "{rendered}" + ); + } + + // ---- MessageAnalysis + analyze_message tests ---- + // + // The pure logic (MessageAnalysis::tech_profiles — does "rust" resolve to + // the right profile?) is tested directly below. The full analyze_message + // integration (does the LLM round-trip work end to end?) is #[ignore]'d + // because loopctl's MockApiClient does not yet honor the response_format + // option that StructuredOutput requires (the default + // create_message_with_options rejects it). That's a loopctl gap to file; + // when MockApiClient supports response_format, un-ignore these. + + #[test] + fn tech_profiles_from_coding_intent() { + let analysis = MessageAnalysis { + suggested_role: Some(Role::Coding), + languages: vec!["rust".to_string()], + }; + let profiles = analysis.tech_profiles(); + assert_eq!(profiles.len(), 1); + assert_eq!(profiles[0].language, "rust"); + assert_eq!(profiles[0].build, "cargo build"); + assert_eq!(profiles[0].test, "cargo test"); + assert_eq!(profiles[0].lint, "cargo clippy"); + } + + #[test] + fn tech_profiles_empty_for_pure_question() { + let analysis = MessageAnalysis { + suggested_role: Some(Role::General), + languages: vec![], + }; + assert!(analysis.tech_profiles().is_empty()); + } + + #[test] + fn tech_profiles_polyglot() { + let analysis = MessageAnalysis { + suggested_role: Some(Role::Coding), + languages: vec!["rust".to_string(), "typescript".to_string()], + }; + let profiles = analysis.tech_profiles(); + let langs: Vec<&str> = profiles.iter().map(|p| p.language.as_str()).collect(); + assert!(langs.contains(&"rust"), "{langs:?}"); + assert!(langs.contains(&"typescript"), "{langs:?}"); + } + + #[test] + fn tech_profiles_unknown_language_skipped() { + let analysis = MessageAnalysis { + suggested_role: None, + languages: vec!["brainfuck".to_string()], + }; + assert!( + analysis.tech_profiles().is_empty(), + "unknown lang should be skipped" + ); + } + + #[test] + fn analyzed_commands_match_detected_commands() { + // A language resolved via analysis (tech_profiles) yields the same + // build/test/lint as detect_tech_stack — proves they share + // language_profile / MARKERS. + let tmp = tempfile::TempDir::new().unwrap(); + std::fs::write(tmp.path().join("Cargo.toml"), "").unwrap(); + let detected = detect_tech_stack(tmp.path()); + assert_eq!(detected.len(), 1); + let detected_rust = &detected[0]; + + let analyzed = MessageAnalysis { + suggested_role: Some(Role::Coding), + languages: vec!["rust".to_string()], + }; + let analyzed_rust = &analyzed.tech_profiles()[0]; + + assert_eq!(analyzed_rust.build, detected_rust.build); + assert_eq!(analyzed_rust.test, detected_rust.test); + assert_eq!(analyzed_rust.lint, detected_rust.lint); + } + + #[tokio::test] + #[ignore = "until MockApiClient supports response_format (loopctl gap)"] + async fn analyze_message_round_trip_with_mock() { + let client = loopctl::testing::MockApiClient::new("test-model") + .with_text_response(r#"{"suggested_role":"coding","languages":["rust"]}"#); + let analysis = analyze_message(&client, "build a Rust CLI").await.unwrap(); + assert_eq!(analysis.suggested_role, Some(Role::Coding)); + assert_eq!(analysis.tech_profiles().len(), 1); + } + + #[tokio::test] + #[ignore = "until MockApiClient supports response_format (loopctl gap)"] + async fn analyze_message_error_is_graceful() { + let client = loopctl::testing::MockApiClient::new("test-model").with_error("boom"); + let result = analyze_message(&client, "anything").await; + assert!(result.is_err(), "expected StructuredError"); + } +} diff --git a/crates/dch-loop/src/prompt.rs b/crates/dch-loop/src/prompt.rs new file mode 100644 index 0000000..9cdd126 --- /dev/null +++ b/crates/dch-loop/src/prompt.rs @@ -0,0 +1,442 @@ +//! System-prompt assembly for the agent loop. +//! +//! The system prompt is built from three parts: a [`Role`] (who the agent +//! acts as — its prose comes from [`Role::system_prompt`]), an optional set of +//! [`TechProfile`]s (what stacks it works on — detected from the repo, or +//! overridden in `[project]` config), and one short prose fragment per +//! registered tool whose [`loopctl::tool::Tool::system_prompt`] returns +//! `Some`. The role text lives with the role; the tech profiles live in +//! [`crate::project`]; the per-tool fragments are contributed by the tools +//! themselves at runtime. +//! +//! The builder is pure and side-effect-free: given the same inputs it yields +//! the same string. Working and temporary directory context is appended by +//! the caller at runner construction, not here. + +use crate::project::TechProfile; +use crate::project::render_techs; +use dch_config::Role; +use loopctl::tool::ToolRegistry; + +/// Agent discipline shared by every role. +/// +/// Prepended to each role's body so the cross-cutting conduct rules live in +/// exactly one place. Deliberately stack-agnostic: it says *how* to work +/// (understand before acting, read what you change), not *what* stack the +/// project is — the stack is the tech-profile axis, detected separately, so a +/// C++ or Haskell or Bash project is not mis-described by hard-coded language +/// defaults. +const SHARED_DISCIPLINE: &str = "\ +You are a proactive agent operating on the user's machine. + +CORE CONDUCT +- Understand before you act. Read the relevant files, logs, or config before + changing anything; inspect the thing you intend to modify. +- Form a complete enough picture of the task before editing, adding, or + removing anything. Ask one clarifying question only when a genuine ambiguity + blocks progress — never ask \"which file?\" or \"what should I do?\"; find + it and make a reasonable choice. +- Surface a brief plan before sizable writes so the change is reviewable; then + apply it. +- When you do not know the stack or conventions, explore the project (list + files, read config and build manifests) and follow what you find rather than + guessing. + +IMAGES +- Read detects image files (png, jpg, jpeg, webp, gif) and returns them as + structured image content. Do not call external or non-existent tools such as + \"analyze_image\" — they do not exist. + +FILE PATHS +- Write within the working directory unless the user names a specific location. + Do not write to system directories. Use the temp directory for scratch and + intermediate output (the caller appends both paths to the prompt)."; + +/// Assemble the system prompt from the default role, no tech profiles, and +/// each registered tool's [`loopctl::tool::Tool::system_prompt`] fragment. +/// +/// Equivalent to [`with_context`] with [`Role::General`] (the default), no +/// techs, and no project conventions. The runner composes the full prompt — +/// with detected techs and cwd/temp — via [`with_context`]. +/// +/// # Examples +/// +/// ``` +/// use dch_loop::build_system_prompt; +/// let registry = loopctl::tool::ToolRegistry::new(); +/// let prompt = build_system_prompt(®istry); +/// assert!(!prompt.is_empty()); +/// ``` +#[must_use] +pub fn build_system_prompt(tools: &ToolRegistry) -> String { + with_context(Role::General, &[], None, None, tools) +} + +/// Assemble the system prompt from a chosen [`Role`] and each registered +/// tool's [`loopctl::tool::Tool::system_prompt`] fragment, with no tech +/// profiles. +/// +/// Convenience for callers that have selected a role but have no detected +/// stack; equivalent to [`with_context`] with empty techs and no conventions. +#[must_use] +pub fn with_role(role: Role, tools: &ToolRegistry) -> String { + with_context(role, &[], None, None, tools) +} + +/// Assemble the full system prompt from a [`Role`], tech profiles, and each +/// registered tool's [`loopctl::tool::Tool::system_prompt`] fragment. +/// +/// The result is the shared discipline, then the role body, then (when +/// non-empty) a `PROJECT` section rendered from `techs` and +/// `project_conventions`, then one `## ` section per tool whose +/// `system_prompt()` returns `Some` — appended in alphabetical order by tool +/// name. Tools returning `None` contribute nothing. Order is deterministic: +/// the same inputs always yield the same string. +/// +/// `role_prompt_override`, when `Some`, replaces the role body — the shared +/// discipline, tech profile, and per-tool fragments still append. The runner +/// resolves it from `[runner].role_overrides` for the selected role (see +/// `RunnerConfig::role_override`); pass `None` to use the role's built-in +/// [`Role::system_prompt`]. +/// +/// The caller (the runner) is responsible for appending the working and temp +/// directory paths; this function returns role + project + fragments only. +#[must_use] +pub fn with_context( + role: Role, + techs: &[TechProfile], + project_conventions: Option<&str>, + role_prompt_override: Option<&str>, + tools: &ToolRegistry, +) -> String { + let mut out = String::new(); + out.push_str(SHARED_DISCIPLINE); + out.push_str("\n\n"); + let role_body = role_prompt_override.unwrap_or_else(|| role.system_prompt()); + out.push_str(role_body); + let rendered = render_techs(techs, project_conventions); + if !rendered.is_empty() { + out.push_str("\n\n"); + out.push_str(&rendered); + } + append_fragments(&mut out, tools); + out +} + +/// Append one `## ` section per tool that has a `system_prompt` +/// fragment, in alphabetical order by tool name. +fn append_fragments(out: &mut String, tools: &ToolRegistry) { + let mut fragments: Vec<(String, String)> = tools + .tool_names() + .into_iter() + .filter_map(|name| { + let tool = tools.get(&name)?; + tool.system_prompt().map(|frag| (name, frag)) + }) + .collect(); + // tool_names() is already sorted; re-sort locally so the ordering + // contract is owned by this module and survives a future change upstream. + fragments.sort_by(|a, b| a.0.cmp(&b.0)); + for (name, frag) in fragments { + out.push_str("\n\n## "); + out.push_str(&name); + out.push_str("\n\n"); + out.push_str(&frag); + } +} + +#[cfg(test)] +#[allow( + clippy::expect_used, + clippy::unwrap_used, + clippy::panic, + clippy::missing_panics_doc, + clippy::missing_errors_doc, + clippy::indexing_slicing +)] +mod tests { + use super::*; + use crate::project::TechProfile; + use loopctl::tool::Tool; + use loopctl::tool::ToolContext; + use loopctl::tool::ToolError; + use loopctl::tool::ToolOutput; + use loopctl::tool::ToolSchema; + use serde_json::Value; + use serde_json::json; + use std::future::Future; + use std::pin::Pin; + + /// A throwaway tool that optionally contributes a system-prompt fragment. + struct FragTool { + name: &'static str, + frag: Option<&'static str>, + } + + impl Tool for FragTool { + fn name(&self) -> &'static str { + self.name + } + fn description(&self) -> &'static str { + "test tool" + } + fn schema(&self) -> ToolSchema { + ToolSchema { + tool: self.name.to_string(), + description: self.description().to_string(), + input_schema: json!({}), + } + } + fn call( + &self, + _input: Value, + _ctx: &ToolContext, + ) -> Pin> + Send + '_>> { + Box::pin(async { Ok(ToolOutput::text("ok")) }) + } + fn system_prompt(&self) -> Option { + self.frag.map(str::to_string) + } + } + + /// Build a registry from a list of test tools. + fn reg(tools: Vec) -> ToolRegistry { + let mut r = ToolRegistry::new(); + for t in tools { + r.register(t); + } + r + } + + #[test] + fn default_role_is_general_and_present_on_empty_registry() { + let prompt = build_system_prompt(&ToolRegistry::new()); + assert!(prompt.contains("GENERAL ASSISTANCE"), "{prompt}"); + assert!( + !prompt.contains("\n\n## "), + "no fragments on empty registry: {prompt}" + ); + } + + #[test] + fn every_role_returns_distinct_prose_via_system_prompt() { + // The role is the single source of truth: each variant's + // system_prompt() must return distinct prose with its task header. + let roles = [ + (Role::General, "GENERAL ASSISTANCE"), + (Role::Coding, "IMPLEMENT FEATURES AND FIXES"), + (Role::Refactor, "WITHOUT CHANGING BEHAVIOR"), + (Role::Debug, "REPRODUCE, ISOLATE"), + (Role::Review, "REVIEW AND REPORT"), + (Role::Docs, "WRITE OR REVISE DOCUMENTATION"), + (Role::Tests, "WRITE AND IMPROVE TESTS"), + ]; + let mut bodies: Vec<&'static str> = Vec::new(); + for (role, header) in roles { + let prose = role.system_prompt(); + assert!(prose.contains(header), "{role:?}: missing {header:?}"); + bodies.push(prose); + } + let unique: std::collections::HashSet<&&str> = bodies.iter().collect(); + assert_eq!(unique.len(), bodies.len(), "roles must differ"); + } + + #[test] + fn shared_discipline_present_in_every_role_and_is_stack_agnostic() { + for role in [ + Role::General, + Role::Coding, + Role::Refactor, + Role::Debug, + Role::Review, + Role::Docs, + Role::Tests, + ] { + let prompt = with_role(role, &ToolRegistry::new()); + assert!( + prompt.contains("Understand before you act"), + "role {role:?} missing conduct rule" + ); + assert!( + prompt.contains("IMAGES"), + "role {role:?} missing image policy" + ); + assert!( + prompt.contains("FILE PATHS"), + "role {role:?} missing path policy" + ); + assert!( + !prompt.contains("DEFAULT FILE SEARCH PATTERNS"), + "role {role:?} leaked language globs into shared discipline" + ); + } + } + + #[test] + fn banned_terms_absent_from_every_role() { + for role in [ + Role::General, + Role::Coding, + Role::Refactor, + Role::Debug, + Role::Review, + Role::Docs, + Role::Tests, + ] { + let prompt = with_role(role, &ToolRegistry::new()); + for banned in [ + "TaskStatus", + "NARROW", + "subagent", + "clamped to 600", + "WebSearch", + ] { + assert!( + !prompt.contains(banned), + "role {role:?} contains banned term {banned:?}" + ); + } + } + } + + #[test] + fn with_context_injects_polyglot_techs_and_conventions() { + let empty = ToolRegistry::new(); + let techs = vec![ + TechProfile { + language: "rust".to_string(), + build: "cargo build".to_string(), + test: "cargo test".to_string(), + lint: "cargo clippy".to_string(), + conventions: String::new(), + }, + TechProfile { + language: "bash".to_string(), + build: String::new(), + test: "bats".to_string(), + lint: String::new(), + conventions: String::new(), + }, + ]; + let prompt = with_context( + Role::Coding, + &techs, + Some("conventional commits"), + None, + &empty, + ); + assert!(prompt.contains("PROJECT"), "{prompt}"); + assert!(prompt.contains("Language: rust"), "{prompt}"); + assert!(prompt.contains("Language: bash"), "{prompt}"); + assert!( + prompt.contains("Project conventions: conventional commits"), + "{prompt}" + ); + } + + #[test] + fn with_context_omits_project_section_when_empty() { + let empty = ToolRegistry::new(); + let prompt = with_context(Role::Coding, &[], None, None, &empty); + assert!( + !prompt.contains("PROJECT"), + "empty techs + no conventions should add no section: {prompt}" + ); + } + + #[test] + fn single_fragment_appended_under_header() { + let r = reg(vec![FragTool { + name: "Alpha", + frag: Some("do alpha"), + }]); + let prompt = with_role(Role::Coding, &r); + assert!(prompt.contains("\n\n## Alpha\n\ndo alpha"), "{prompt}"); + } + + #[test] + fn none_fragment_omitted() { + let r = reg(vec![FragTool { + name: "Silent", + frag: None, + }]); + let prompt = with_role(Role::Coding, &r); + assert!( + !prompt.contains("## Silent"), + "None-fragment tool should be silent: {prompt}" + ); + } + + #[test] + fn multiple_fragments_sorted_and_stable() { + let r = reg(vec![ + FragTool { + name: "Zeta", + frag: Some("z"), + }, + FragTool { + name: "Alpha", + frag: Some("a"), + }, + FragTool { + name: "Mu", + frag: Some("m"), + }, + ]); + let prompt = with_role(Role::Coding, &r); + let alpha = prompt.find("## Alpha").unwrap(); + let mu = prompt.find("## Mu").unwrap(); + let zeta = prompt.find("## Zeta").unwrap(); + assert!( + alpha < mu && mu < zeta, + "fragments must be alphabetical: {prompt}" + ); + let prompt2 = with_role(Role::Coding, &r); + assert_eq!(prompt, prompt2); + } + + #[test] + fn role_prompt_override_replaces_role_body_keeps_rest() { + // When set, the override replaces the role's built-in prose; the + // shared discipline, tech profile, and fragments still append. + let empty = ToolRegistry::new(); + let override_body = "CUSTOM ROLE PROSE: do the thing your way."; + let prompt = with_context(Role::Debug, &[], None, Some(override_body), &empty); + // Override is used in place of the role's built-in body. + assert!( + prompt.contains("CUSTOM ROLE PROSE"), + "override missing: {prompt}" + ); + assert!( + !prompt.contains("REPRODUCE, ISOLATE"), + "built-in debug prose should be replaced, not appended: {prompt}" + ); + // Shared discipline still present. + assert!( + prompt.contains("Understand before you act"), + "discipline dropped: {prompt}" + ); + } + + #[test] + fn role_prompt_override_still_appends_fragments_and_techs() { + let r = reg(vec![FragTool { + name: "Tool", + frag: Some("frag"), + }]); + let techs = vec![TechProfile { + language: "rust".to_string(), + build: "cargo build".to_string(), + test: String::new(), + lint: String::new(), + conventions: String::new(), + }]; + let prompt = with_context(Role::Coding, &techs, None, Some("OVERRIDE BODY"), &r); + assert!(prompt.contains("OVERRIDE BODY"), "override: {prompt}"); + assert!(prompt.contains("Language: rust"), "techs: {prompt}"); + assert!( + prompt.contains("\n\n## Tool\n\nfrag"), + "fragments: {prompt}" + ); + } +} diff --git a/crates/dch-loop/tests/system_prompt.rs b/crates/dch-loop/tests/system_prompt.rs new file mode 100644 index 0000000..12e0085 --- /dev/null +++ b/crates/dch-loop/tests/system_prompt.rs @@ -0,0 +1,29 @@ +//! End-to-end check that the real builtin registry's `system_prompt()` +//! fragments agree with the documented fragment set. +//! +//! Ignored until the `Read` (and `TodoWrite`) tools land their `system_prompt()` +//! fragments. When un-ignored it asserts the documented headers are present +//! and that tools without a fragment contribute none. + +#![allow(clippy::missing_panics_doc, clippy::missing_errors_doc)] + +use dch_loop::build_system_prompt; +use dch_tools::builtin_registry; + +#[test] +#[ignore = "until the Read and TodoWrite tools land their system_prompt() fragments"] +fn builtin_registry_emits_documented_fragment_headers() { + let prompt = build_system_prompt(&builtin_registry()); + for header in ["## Bash", "## Read", "## Write", "## Edit", "## TodoWrite"] { + assert!( + prompt.contains(header), + "expected fragment header {header:?} in prompt" + ); + } + for silent in ["## Glob", "## Grep", "## Tree", "## FileViewer"] { + assert!( + !prompt.contains(silent), + "tool {silent:?} has no system_prompt fragment; none expected" + ); + } +} From 4cc10d44c879574d0c5aca876a0f4a1ef8fdae58 Mon Sep 17 00:00:00 2001 From: Iurii Bobrykov Date: Sun, 9 Aug 2026 08:00:14 +1200 Subject: [PATCH 2/2] fix: system prompt, unify tech profile, case-insensitive matching --- crates/dch-config/src/lib.rs | 61 +++++-- crates/dch-loop/src/lib.rs | 3 +- crates/dch-loop/src/project.rs | 289 +++++++++++++++++---------------- crates/dch-loop/src/prompt.rs | 59 +++++-- 4 files changed, 243 insertions(+), 169 deletions(-) diff --git a/crates/dch-config/src/lib.rs b/crates/dch-config/src/lib.rs index 9f5cd0c..8eb4c0b 100644 --- a/crates/dch-config/src/lib.rs +++ b/crates/dch-config/src/lib.rs @@ -151,7 +151,9 @@ pub enum PermissionMode { /// works on — detected from the repo, overridable in `[project]`). Enforcement /// is handled by the permission layer at runtime regardless of which role is /// selected. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Default, serde::Deserialize, serde::Serialize)] +#[derive( + Debug, Clone, Copy, PartialEq, Eq, Hash, Default, serde::Deserialize, serde::Serialize, +)] #[serde(rename_all = "snake_case")] pub enum Role { /// General assistance — PC help, sysadmin, app configuration (the default). @@ -239,7 +241,7 @@ YOUR ROLE: IMPLEMENT FEATURES AND FIXES - Keep edits targeted. A single Edit should carry one intent; split work that does several things into several edits. - For unfamiliar or complex operations, look up the established pattern in the - repo (or via WebFetch) before inventing a new one."; + repo before inventing a new one."; /// Role body for [`Role::Refactor`]: restructure without behavior change. pub(crate) const REFACTOR_ROLE: &str = "\ @@ -329,6 +331,22 @@ impl Role { Role::Tests => TESTS_ROLE, } } + + /// All variants, in declaration order. + /// + /// Single source of truth for consumers that need to enumerate roles + /// (e.g. the message-analysis schema's `enum` constraint, a `--help` + /// listing). Adding a variant to `Role` and forgetting to add it here is + /// caught by the `all_roles_covered` test. + pub const ALL: [Role; 7] = [ + Role::General, + Role::Coding, + Role::Refactor, + Role::Debug, + Role::Review, + Role::Docs, + Role::Tests, + ]; } /// Errors arising while loading configuration. @@ -552,8 +570,13 @@ pub struct RoleOverride { /// whose role matches the selection takes effect. pub role: Role, - /// The replacement prose. Used verbatim in place of - /// [`Role::system_prompt`] for this role. + /// The replacement prose. + /// + /// Used verbatim in place of [`Role::system_prompt`] for this role. The + /// shared agent discipline, detected tech profile, and per-tool fragments + /// still append — only the role-specific body is replaced. There is no + /// length limit; the caller composes the full prompt from this string plus + /// the other parts. pub prompt: String, } @@ -562,7 +585,8 @@ impl RunnerConfig { /// /// Returns the override's prompt when an entry for `role` exists in /// [`Self::role_overrides`], otherwise `None` (meaning: use the role's - /// built-in [`Role::system_prompt`]). + /// built-in [`Role::system_prompt`]). When multiple entries share the + /// same role, the first declaration wins. #[must_use] pub fn role_override(&self, role: Role) -> Option<&str> { self.role_overrides @@ -577,12 +601,12 @@ impl RunnerConfig { /// Most real projects are polyglot — a Rust core, a `TypeScript` frontend, a /// Python tooling script — and each language has its own build/test/lint /// commands and its own conventions. This struct captures one such language; -/// [`ProjectConfig`] holds a `Vec` for all of them. +/// [`ProjectConfig`] holds a `Vec` for all of them. /// /// Every field except `language` is optional: set only what detection got /// wrong or can't infer. Loaded from the `[[project.techs]]` array-of-tables. #[derive(Debug, Clone, Default, PartialEq, Eq, serde::Deserialize, serde::Serialize)] -pub struct Tech { +pub struct TechProfile { /// The language this entry describes, e.g. `"rust"` or `"cpp"`. /// /// Matches against the detected language so the runner can merge detected @@ -633,7 +657,7 @@ pub struct ProjectConfig { /// One `[[project.techs]]` table per language the user wants to declare or /// override. The runner merges each with its detected counterpart by /// language; entries for languages detection missed are appended. - pub techs: Vec, + pub techs: Vec, /// Free-form project-wide conventions that apply across all languages. /// @@ -749,8 +773,12 @@ impl DchConfig { /// Map the session-scoped fields to a [`loopctl::config::SessionConfig`]. /// - /// Carries the system prompt, context window, compaction threshold, and - /// auto-compact flag — the settings that are stable across `run()` calls + /// Carries the context window, compaction threshold, and auto-compact + /// flag — the settings that are stable across `run()` calls. The + /// `system_prompt` is **not** carried here (it is set to `None`); the + /// runner composes it from the selected role, detected tech stack, and + /// per-tool fragments via the prompt builder and installs it on + /// the session after construction. /// on the same agent. Provider-specific fields (`model`, `max_tokens`) are /// not session-config concerns; they are consumed by the API client via /// [`ApiConfig`] directly. The session id is minted at runtime by loopctl, @@ -1072,6 +1100,19 @@ json_logs = true assert_eq!(Role::default(), Role::General); } + #[test] + fn role_all_covers_every_variant() { + let mut seen = std::collections::HashSet::new(); + for role in Role::ALL { + assert!(seen.insert(role), "duplicate variant in ALL: {role:?}"); + } + assert_eq!( + seen.len(), + 7, + "ALL must contain all 7 variants — add new ones here" + ); + } + #[test] fn runner_config_default_carries_general_role() { assert_eq!(RunnerConfig::default().role, Role::General); diff --git a/crates/dch-loop/src/lib.rs b/crates/dch-loop/src/lib.rs index 793dba4..beb3083 100644 --- a/crates/dch-loop/src/lib.rs +++ b/crates/dch-loop/src/lib.rs @@ -7,10 +7,9 @@ pub mod project; pub mod prompt; pub mod provider; -pub use dch_config::{ApiConfig, ApiType, DchConfigError}; +pub use dch_config::{ApiConfig, ApiType, DchConfigError, TechProfile}; pub use error::RunnerError; pub use project::MessageAnalysis; -pub use project::TechProfile; pub use project::analyze_message; pub use project::detect_tech_stack; pub use project::merge_by_language; diff --git a/crates/dch-loop/src/project.rs b/crates/dch-loop/src/project.rs index cce140d..d045317 100644 --- a/crates/dch-loop/src/project.rs +++ b/crates/dch-loop/src/project.rs @@ -9,7 +9,7 @@ use std::path::Path; use dch_config::ProjectConfig; use dch_config::Role; -use dch_config::Tech; +use dch_config::TechProfile; use loopctl::api::ApiClient; use loopctl::message::Message; use loopctl::structured::StructuredError; @@ -18,67 +18,6 @@ use loopctl::structured::request_structured; use serde::Deserialize; use serde::Serialize; -/// A detected (or configured) description of one language in the project. -/// -/// Built by [`detect_tech_stack`] from a marker file, then refined by -/// [`merge_by_language`] with the matching [`Tech`] from `[project].techs`. -/// A polyglot repo has several of these; the runner renders them all. -#[derive(Debug, Clone, Default, PartialEq, Eq)] -pub struct TechProfile { - /// The language this entry describes. - /// - /// Lowercased marker-derived name, e.g. `"rust"`, `"typescript"`, - /// `"make"`. Used as the merge key against configured [`Tech`] entries and - /// rendered as the section header. Never empty for a detected profile. - pub language: String, - - /// Command that builds this language's code. - /// - /// Marker-derived, e.g. `"cargo build"`. Empty when the marker implies no - /// build step or the user left the override unset. - pub build: String, - - /// Command that runs this language's tests. - /// - /// Marker-derived, e.g. `"cargo test"`. Empty when the marker implies no - /// test step or the user left the override unset. - pub test: String, - - /// Command that lints this language's code. - /// - /// Marker-derived, e.g. `"cargo clippy"`. Empty when the marker implies no - /// lint step or the user left the override unset. - pub lint: String, - - /// Free-form conventions prose for this language. - /// - /// Carried from a configured [`Tech`] override's `conventions` field; - /// detection never sets it. Empty when the user supplied none. - pub conventions: String, -} - -impl TechProfile { - /// Apply the set fields of a [`Tech`] override onto this profile. - /// - /// Fields the user left `None` keep the detected value, so a `[project]` - /// entry acts as a partial override. `language` is never overridden here — - /// matching is by language, so the identity stays the detected one. - pub fn apply_override(&mut self, tech: &Tech) { - if let Some(build) = &tech.build { - self.build.clone_from(build); - } - if let Some(test) = &tech.test { - self.test.clone_from(test); - } - if let Some(lint) = &tech.lint { - self.lint.clone_from(lint); - } - if let Some(conv) = &tech.conventions { - self.conventions.clone_from(conv); - } - } -} - /// One marker file and the profile it implies. /// /// A row in [`MARKERS`]: the presence of `file` at a repo root implies the @@ -195,7 +134,7 @@ const MARKERS: &[Marker] = &[ lint: "make lint", }, Marker { - file: "Mix.exs", + file: "mix.exs", language: "elixir", build: "mix compile", test: "mix test", @@ -236,16 +175,31 @@ pub fn detect_tech_stack(root: &Path) -> Vec { } else { root.join(marker.file).exists() }; - if matched - && !profiles.iter().any(|p| p.language == marker.language) - && let Some(profile) = language_profile(marker.language) - { - profiles.push(profile); + if matched && !profiles.iter().any(|p| p.language == marker.language) { + profiles.push(marker_profile(marker)); } } profiles } +/// Resolve a matched [`Marker`] to its [`TechProfile`]. +/// +/// Companion to [`language_profile`]: both return a [`TechProfile`], but from +/// different sources — this one from a detected marker (so every command is +/// known and set to `Some(...)`), [`language_profile`] from a language name +/// (e.g. an inferred name from [`analyze_message`]). Conventions are `None` +/// here; detection never infers them, only a `[project]` override does via +/// [`merge_by_language`]. +fn marker_profile(m: &Marker) -> TechProfile { + TechProfile { + language: m.language.to_string(), + build: Some(m.build.to_string()), + test: Some(m.test.to_string()), + lint: Some(m.lint.to_string()), + conventions: None, + } +} + /// Look up the default [`TechProfile`] for a language name. /// /// Shared by detection ([`detect_tech_stack`]) and inference @@ -255,16 +209,11 @@ pub fn detect_tech_stack(root: &Path) -> Vec { /// Returns `None` for an unknown language (e.g. an inferred name with no /// marker row); the caller skips it. fn language_profile(language: &str) -> Option { + let lang = language.to_lowercase(); MARKERS .iter() - .find(|m| m.language == language) - .map(|m| TechProfile { - language: m.language.to_string(), - build: m.build.to_string(), - test: m.test.to_string(), - lint: m.lint.to_string(), - conventions: String::new(), - }) + .find(|m| m.language == lang) + .map(marker_profile) } /// The model's analysis of a first user message: its intent and its stack. @@ -298,15 +247,18 @@ impl StructuredOutput for MessageAnalysis { } fn schema() -> serde_json::Value { + let role_names: Vec = Role::ALL + .iter() + .map(|r| serde_json::to_value(r).unwrap_or_default()) + .collect(); + let mut role_enum = role_names; + role_enum.push(serde_json::Value::Null); serde_json::json!({ "type": "object", "properties": { "suggested_role": { "type": ["string", "null"], - "enum": [ - "general", "coding", "refactor", "debug", - "review", "docs", "tests", null - ], + "enum": role_enum, "description": "The role that best fits what the user wants to do. null if unclear." }, "languages": { @@ -366,6 +318,11 @@ pub async fn analyze_message( /// True if any direct child of `root` matches the single-segment `glob` /// (e.g. `"*.csproj"`). +/// +/// Used by [`detect_tech_stack`] for glob-based markers (those whose `file` +/// contains `*`). Reads only the immediate children of `root` — does not +/// descend into subdirectories, since markers are root-level files. A +/// nonexistent or unreadable `root` returns `false` rather than panicking. fn has_glob_match(root: &Path, glob: &str) -> bool { let Ok(entries) = std::fs::read_dir(root) else { return false; @@ -382,7 +339,7 @@ fn has_glob_match(root: &Path, glob: &str) -> bool { /// Merge detected profiles with the user's `[project]` overrides by language. /// -/// For each detected profile whose language matches a configured [`Tech`], +/// For each detected profile whose language matches a configured [`TechProfile`], /// that tech's set fields override the detected ones. Configured techs whose /// language was not detected are appended (the user adds a language detection /// missed). Detected techs the config doesn't mention are kept as-is. Order is @@ -393,29 +350,29 @@ pub fn merge_by_language( config: &ProjectConfig, ) -> Vec { for tech in &config.techs { - if let Some(profile) = detected.iter_mut().find(|p| p.language == tech.language) { - profile.apply_override(tech); + if let Some(profile) = detected + .iter_mut() + .find(|p| p.language.eq_ignore_ascii_case(&tech.language)) + { + if tech.build.is_some() { + profile.build.clone_from(&tech.build); + } + if tech.test.is_some() { + profile.test.clone_from(&tech.test); + } + if tech.lint.is_some() { + profile.lint.clone_from(&tech.lint); + } + if tech.conventions.is_some() { + profile.conventions.clone_from(&tech.conventions); + } } else { - detected.push(TechProfile::from_tech(tech)); + detected.push(tech.clone()); } } detected } -impl TechProfile { - /// Build a profile from a configured [`Tech`] (for languages detection - /// missed entirely). - fn from_tech(tech: &Tech) -> TechProfile { - TechProfile { - language: tech.language.clone(), - build: tech.build.clone().unwrap_or_default(), - test: tech.test.clone().unwrap_or_default(), - lint: tech.lint.clone().unwrap_or_default(), - conventions: tech.conventions.clone().unwrap_or_default(), - } - } -} - /// Render a polyglot tech list plus project-wide conventions as the prose /// block injected into the system prompt. /// @@ -435,21 +392,21 @@ pub fn render_techs(techs: &[TechProfile], project_conventions: Option<&str>) -> } out.push_str("\n\n- Language: "); out.push_str(&tech.language); - if !tech.build.is_empty() { + if let Some(v) = tech.build.as_deref().filter(|v| !v.is_empty()) { out.push_str("\n Build: "); - out.push_str(&tech.build); + out.push_str(v); } - if !tech.test.is_empty() { + if let Some(v) = tech.test.as_deref().filter(|v| !v.is_empty()) { out.push_str("\n Test: "); - out.push_str(&tech.test); + out.push_str(v); } - if !tech.lint.is_empty() { + if let Some(v) = tech.lint.as_deref().filter(|v| !v.is_empty()) { out.push_str("\n Lint: "); - out.push_str(&tech.lint); + out.push_str(v); } - if !tech.conventions.is_empty() { + if let Some(v) = tech.conventions.as_deref().filter(|v| !v.is_empty()) { out.push_str("\n Conventions: "); - out.push_str(&tech.conventions); + out.push_str(v); } } if let Some(conv) = project_conventions @@ -461,7 +418,7 @@ pub fn render_techs(techs: &[TechProfile], project_conventions: Option<&str>) -> out } -/// True when every field of `tech` other than `language` is empty. +/// True when every field of `tech` other than `language` is unset or empty. /// /// A profile that is bare (only `language` set, no commands or conventions) /// contributes nothing actionable to the prompt, so [`render_techs`] skips it. @@ -470,10 +427,10 @@ pub fn render_techs(techs: &[TechProfile], project_conventions: Option<&str>) -> /// rendered PROJECT section on its own; pair it with at least one command or /// convention to make it render. fn all_fields_empty(tech: &TechProfile) -> bool { - tech.build.is_empty() - && tech.test.is_empty() - && tech.lint.is_empty() - && tech.conventions.is_empty() + tech.build.as_deref().is_none_or(str::is_empty) + && tech.test.as_deref().is_none_or(str::is_empty) + && tech.lint.as_deref().is_none_or(str::is_empty) + && tech.conventions.as_deref().is_none_or(str::is_empty) } #[cfg(test)] @@ -488,7 +445,7 @@ fn all_fields_empty(tech: &TechProfile) -> bool { mod tests { use super::*; use dch_config::ProjectConfig; - use dch_config::Tech; + use dch_config::TechProfile; #[test] fn empty_root_yields_no_profiles() { @@ -505,9 +462,9 @@ mod tests { assert_eq!(profiles.len(), 1); let p = &profiles[0]; assert_eq!(p.language, "rust"); - assert_eq!(p.build, "cargo build"); - assert_eq!(p.test, "cargo test"); - assert_eq!(p.lint, "cargo clippy"); + assert_eq!(p.build.as_deref(), Some("cargo build")); + assert_eq!(p.test.as_deref(), Some("cargo test")); + assert_eq!(p.lint.as_deref(), Some("cargo clippy")); } #[test] @@ -581,28 +538,60 @@ mod tests { )); } + #[test] + fn mix_exs_lowercase_detects_elixir() { + let tmp = tempfile::TempDir::new().unwrap(); + std::fs::write(tmp.path().join("mix.exs"), "").unwrap(); + let profiles = detect_tech_stack(tmp.path()); + assert_eq!(profiles.len(), 1); + assert_eq!(profiles[0].language, "elixir"); + } + + #[test] + fn build_gradle_detects_java_with_gradle_build() { + let tmp = tempfile::TempDir::new().unwrap(); + std::fs::write(tmp.path().join("build.gradle"), "").unwrap(); + let profiles = detect_tech_stack(tmp.path()); + assert_eq!(profiles.len(), 1); + assert_eq!(profiles[0].language, "java"); + assert_eq!(profiles[0].build.as_deref(), Some("gradle build")); + } + + #[test] + fn case_insensitive_language_matching_in_tech_profiles() { + // Model may return "Rust" (capitalized) — must resolve to the lowercase + // profile, not be silently dropped. + let analysis = MessageAnalysis { + suggested_role: Some(Role::Coding), + languages: vec!["Rust".to_string()], + }; + let profiles = analysis.tech_profiles(); + assert_eq!(profiles.len(), 1); + assert_eq!(profiles[0].language, "rust"); + } + #[test] fn merge_overrides_matching_language_and_appends_new() { let detected = vec![ TechProfile { language: "rust".to_string(), - build: "cargo build".to_string(), - test: "cargo test".to_string(), - lint: "cargo clippy".to_string(), - conventions: String::new(), + build: Some("cargo build".to_string()), + test: Some("cargo test".to_string()), + lint: Some("cargo clippy".to_string()), + conventions: None, }, TechProfile { language: "typescript".to_string(), - build: "npm run build".to_string(), - test: "npm test".to_string(), - lint: String::new(), - conventions: String::new(), + build: Some("npm run build".to_string()), + test: Some("npm test".to_string()), + lint: None, + conventions: None, }, ]; let config = ProjectConfig { techs: vec![ // Override rust's build only; keep detected test/lint. - Tech { + TechProfile { language: "rust".to_string(), build: Some("cargo build --release".to_string()), test: None, @@ -610,7 +599,7 @@ mod tests { conventions: Some("no unwrap".to_string()), }, // Add a language detection missed. - Tech { + TechProfile { language: "bash".to_string(), build: None, test: Some("bats".to_string()), @@ -622,13 +611,29 @@ mod tests { }; let merged = merge_by_language(detected, &config); let rust = merged.iter().find(|p| p.language == "rust").unwrap(); - assert_eq!(rust.build, "cargo build --release", "override applied"); - assert_eq!(rust.test, "cargo test", "unset field kept detected"); - assert_eq!(rust.lint, "cargo clippy", "unset field kept detected"); - assert_eq!(rust.conventions, "no unwrap"); + assert_eq!( + rust.build.as_deref(), + Some("cargo build --release"), + "override applied" + ); + assert_eq!( + rust.test.as_deref(), + Some("cargo test"), + "unset field kept detected" + ); + assert_eq!( + rust.lint.as_deref(), + Some("cargo clippy"), + "unset field kept detected" + ); + assert_eq!(rust.conventions.as_deref(), Some("no unwrap")); let bash = merged.iter().find(|p| p.language == "bash").unwrap(); - assert_eq!(bash.test, "bats", "appended config-only tech"); - assert_eq!(bash.lint, "shellcheck"); + assert_eq!( + bash.test.as_deref(), + Some("bats"), + "appended config-only tech" + ); + assert_eq!(bash.lint.as_deref(), Some("shellcheck")); assert_eq!(merged.len(), 3, "rust + typescript + bash"); } @@ -637,17 +642,17 @@ mod tests { let techs = vec![ TechProfile { language: "rust".to_string(), - build: "cargo build".to_string(), - test: "cargo test".to_string(), - lint: "cargo clippy".to_string(), - conventions: String::new(), + build: Some("cargo build".to_string()), + test: Some("cargo test".to_string()), + lint: Some("cargo clippy".to_string()), + conventions: None, }, TechProfile { language: "bash".to_string(), - build: String::new(), - test: "bats".to_string(), - lint: String::new(), - conventions: String::new(), + build: None, + test: Some("bats".to_string()), + lint: None, + conventions: None, }, ]; let rendered = render_techs(&techs, Some("conventional commits")); @@ -686,9 +691,9 @@ mod tests { let profiles = analysis.tech_profiles(); assert_eq!(profiles.len(), 1); assert_eq!(profiles[0].language, "rust"); - assert_eq!(profiles[0].build, "cargo build"); - assert_eq!(profiles[0].test, "cargo test"); - assert_eq!(profiles[0].lint, "cargo clippy"); + assert_eq!(profiles[0].build.as_deref(), Some("cargo build")); + assert_eq!(profiles[0].test.as_deref(), Some("cargo test")); + assert_eq!(profiles[0].lint.as_deref(), Some("cargo clippy")); } #[test] diff --git a/crates/dch-loop/src/prompt.rs b/crates/dch-loop/src/prompt.rs index 9cdd126..2f9b59e 100644 --- a/crates/dch-loop/src/prompt.rs +++ b/crates/dch-loop/src/prompt.rs @@ -13,9 +13,9 @@ //! the same string. Working and temporary directory context is appended by //! the caller at runner construction, not here. -use crate::project::TechProfile; use crate::project::render_techs; use dch_config::Role; +use dch_config::TechProfile; use loopctl::tool::ToolRegistry; /// Agent discipline shared by every role. @@ -50,7 +50,7 @@ IMAGES FILE PATHS - Write within the working directory unless the user names a specific location. Do not write to system directories. Use the temp directory for scratch and - intermediate output (the caller appends both paths to the prompt)."; + intermediate output."; /// Assemble the system prompt from the default role, no tech profiles, and /// each registered tool's [`loopctl::tool::Tool::system_prompt`] fragment. @@ -125,6 +125,11 @@ pub fn with_context( /// Append one `## ` section per tool that has a `system_prompt` /// fragment, in alphabetical order by tool name. +/// +/// Tools whose [`Tool::system_prompt`] returns `None` are skipped — they +/// contribute nothing to the prompt. The local re-sort (after `tool_names()` +/// already sorts) keeps the ordering contract owned by this module, so a +/// future loopctl change to `tool_names()` cannot silently reorder fragments. fn append_fragments(out: &mut String, tools: &ToolRegistry) { let mut fragments: Vec<(String, String)> = tools .tool_names() @@ -156,7 +161,7 @@ fn append_fragments(out: &mut String, tools: &ToolRegistry) { )] mod tests { use super::*; - use crate::project::TechProfile; + use dch_config::TechProfile; use loopctl::tool::Tool; use loopctl::tool::ToolContext; use loopctl::tool::ToolError; @@ -305,17 +310,17 @@ mod tests { let techs = vec![ TechProfile { language: "rust".to_string(), - build: "cargo build".to_string(), - test: "cargo test".to_string(), - lint: "cargo clippy".to_string(), - conventions: String::new(), + build: Some("cargo build".to_string()), + test: Some("cargo test".to_string()), + lint: Some("cargo clippy".to_string()), + conventions: None, }, TechProfile { language: "bash".to_string(), - build: String::new(), - test: "bats".to_string(), - lint: String::new(), - conventions: String::new(), + build: None, + test: Some("bats".to_string()), + lint: None, + conventions: None, }, ]; let prompt = with_context( @@ -334,6 +339,30 @@ mod tests { ); } + #[test] + fn with_context_sections_in_documented_order() { + let r = reg(vec![FragTool { + name: "Zeta", + frag: Some("z"), + }]); + let techs = vec![TechProfile { + language: "rust".to_string(), + build: Some("cargo build".to_string()), + test: None, + lint: None, + conventions: None, + }]; + let prompt = with_context(Role::Coding, &techs, Some("fmt"), None, &r); + let discipline = prompt.find("CORE CONDUCT").unwrap(); + let role_body = prompt.find("IMPLEMENT FEATURES").unwrap(); + let project = prompt.find("PROJECT").unwrap(); + let fragment = prompt.find("## Zeta").unwrap(); + assert!( + discipline < role_body && role_body < project && project < fragment, + "sections out of order: discipline={discipline}, role={role_body}, project={project}, fragment={fragment}\n{prompt}" + ); + } + #[test] fn with_context_omits_project_section_when_empty() { let empty = ToolRegistry::new(); @@ -426,10 +455,10 @@ mod tests { }]); let techs = vec![TechProfile { language: "rust".to_string(), - build: "cargo build".to_string(), - test: String::new(), - lint: String::new(), - conventions: String::new(), + build: Some("cargo build".to_string()), + test: None, + lint: None, + conventions: None, }]; let prompt = with_context(Role::Coding, &techs, None, Some("OVERRIDE BODY"), &r); assert!(prompt.contains("OVERRIDE BODY"), "override: {prompt}");