diff --git a/crates/frameshift-cli/src/cmd/use_persona.rs b/crates/frameshift-cli/src/cmd/use_persona.rs index e7ab889..3e28711 100644 --- a/crates/frameshift-cli/src/cmd/use_persona.rs +++ b/crates/frameshift-cli/src/cmd/use_persona.rs @@ -7,6 +7,7 @@ use std::path::{Path, PathBuf}; use clap::Args; use frameshift_client::{Client, ClientError, InstallRequest, InstallSource, PersonaSpec}; +use frameshift_orchestrator::Preferences; use crate::util::CliError; @@ -76,6 +77,18 @@ pub fn run_use(client: &Client, args: UseArgs) -> Result<(), CliError> { other => CliError::Orchestrator(other.to_string()), })?; + // Learn from the explicit choice: nudge future automatic selection toward + // the persona the user activated. This writes the same `automate-prefs.json` + // that `select` and the daemon read, so the bias actually closes the loop. + // Best-effort -- activation has already succeeded, so a preferences failure + // must not fail the command. + if let Ok(state_dir) = client.orchestrator_state_dir(&project_root) { + let prefs_path = state_dir.join("automate-prefs.json"); + if let Err(e) = record_persona_use(&prefs_path, &args.name) { + eprintln!("warning: could not record persona preference: {e}"); + } + } + // Read and print the rendered persona for the claude target. let rendered = client.rendered_persona(&project_root, &args.name, "claude")?; println!("{}", rendered); @@ -103,3 +116,53 @@ fn read_pack_version(persona_dir: &Path) -> Option { } None } + +/// Record that the user explicitly activated `persona`, nudging future +/// automatic selection toward it. +/// +/// Loads the shared `automate-prefs.json` (the same store `select` and the +/// daemon read), bumps the persona's bias via [`Preferences::record_override`] +/// (with no auto-pick to penalize -- an explicit `use` is a positive signal, +/// not a correction of a specific automatic pick), and persists it atomically. +fn record_persona_use(prefs_path: &Path, persona: &str) -> Result<(), String> { + let mut prefs = Preferences::load(prefs_path).map_err(|e| e.to_string())?; + prefs.record_override(None, persona); + prefs.save(prefs_path).map_err(|e| e.to_string()) +} + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::TempDir; + + /// Recording a use bumps the persona's bias and persists it to the shared + /// preferences file so later selection can read it back. + #[test] + fn record_persona_use_biases_persona() { + let tmp = TempDir::new().unwrap(); + let prefs_path = tmp.path().join("automate-prefs.json"); + + record_persona_use(&prefs_path, "rust").unwrap(); + + let prefs = Preferences::load(&prefs_path).unwrap(); + assert!( + prefs.bias_for("rust") > 0.0, + "an explicit `use` should bias the persona upward" + ); + } + + /// Repeated uses accumulate bias (capped by the feedback layer) and never + /// error on an existing preferences file. + #[test] + fn repeated_use_accumulates_and_persists() { + let tmp = TempDir::new().unwrap(); + let prefs_path = tmp.path().join("automate-prefs.json"); + + record_persona_use(&prefs_path, "rust").unwrap(); + let first = Preferences::load(&prefs_path).unwrap().bias_for("rust"); + record_persona_use(&prefs_path, "rust").unwrap(); + let second = Preferences::load(&prefs_path).unwrap().bias_for("rust"); + + assert!(second >= first, "bias should not decrease on repeated use"); + } +} diff --git a/crates/frameshift-daemon/src/orchestrator.rs b/crates/frameshift-daemon/src/orchestrator.rs index 0a43985..faef248 100644 --- a/crates/frameshift-daemon/src/orchestrator.rs +++ b/crates/frameshift-daemon/src/orchestrator.rs @@ -17,6 +17,20 @@ use frameshift_orchestrator::{ run::{select, SelectionInputs}, }; +/// Read the persona name recorded in the project's active marker, if any. +/// +/// Returns `None` when the marker is absent, unreadable, or empty after +/// trimming. Shared by the manual-override check and the audit `from` capture. +fn read_active_persona(client: &Client, project_root: &Path) -> Option { + match client.project_paths(project_root) { + Ok(paths) if paths.active_path.exists() => std::fs::read_to_string(&paths.active_path) + .ok() + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()), + _ => None, + } +} + /// Evaluate the current project context and apply a persona switch if warranted. /// /// Steps: @@ -66,6 +80,31 @@ pub fn evaluate_and_apply(client: &Client, controller: &mut SwitchController, pr return; } + // Load learned preferences (shared with the CLI and the selection pass). + let mut prefs = Preferences::load(&prefs_path).unwrap_or_default(); + + // Manual-override learning (A1): if the persona on disk differs from the + // auto-pick the controller last applied, the user switched by hand. Reward + // their choice, decay the rejected auto-pick, persist the preference, and + // re-baseline the controller so the same override is not re-learned on every + // subsequent tick. The updated bias also feeds this tick's selection. + let auto_pick = controller.active_persona().map(str::to_string); + let active_now = read_active_persona(client, project_root); + if let (Some(auto), Some(chosen)) = (auto_pick.as_deref(), active_now.as_deref()) { + if auto != chosen { + prefs.record_override(Some(auto), chosen); + if let Err(e) = prefs.save(&prefs_path) { + tracing::warn!(error = %e, "orchestrator: failed to persist override preference"); + } + controller.adopt_active(chosen); + tracing::info!( + from = %auto, + to = %chosen, + "orchestrator: learned manual persona override" + ); + } + } + // Step 3: collect persona source dirs and run selection. let source_dirs = match client.installed_persona_source_dirs(project_root) { Ok(dirs) => dirs, @@ -75,8 +114,6 @@ pub fn evaluate_and_apply(client: &Client, controller: &mut SwitchController, pr } }; - let prefs = Preferences::load(&prefs_path).unwrap_or_default(); - let inputs = SelectionInputs { project_root, task_hint: None, @@ -105,13 +142,7 @@ pub fn evaluate_and_apply(client: &Client, controller: &mut SwitchController, pr } = decision { // Read the currently active persona before overwriting the marker. - let from = match client.project_paths(project_root) { - Ok(paths) if paths.active_path.exists() => std::fs::read_to_string(&paths.active_path) - .ok() - .map(|s| s.trim().to_string()) - .filter(|s| !s.is_empty()), - _ => None, - }; + let from = read_active_persona(client, project_root); tracing::info!( persona = %to, @@ -382,4 +413,59 @@ mod tests { } } } + + /// A manual switch away from the daemon's auto-pick is learned: the chosen + /// persona is rewarded and the rejected auto-pick is decayed in the shared + /// preferences file, and the controller re-baselines to the manual choice. + /// Prior to A1 the daemon applied its own switches but never learned when + /// the user overrode them by hand. + #[test] + fn evaluate_learns_from_manual_override() { + let tmp = tempfile::tempdir().unwrap(); + let project_root = tmp.path().join("project"); + fs::create_dir_all(&project_root).unwrap(); + + let data_root = tmp.path().join("data"); + let client = test_client(&data_root); + + // Enable automate mode (no lock) so evaluation proceeds. + let state_dir = client.orchestrator_state_dir(&project_root).unwrap(); + fs::create_dir_all(&state_dir).unwrap(); + let mode = ModeState { + mode: Mode::On, + sensitivity: 0.5, + }; + mode.save(&state_dir.join("automate.json")).unwrap(); + + // Seed the controller's auto-pick deterministically, as if the daemon + // had activated "auto-pick" on a prior tick. + let mut controller = SwitchController::new(SwitchPolicy::default()); + controller.adopt_active("auto-pick"); + + // Simulate the user manually switching to a different persona by writing + // the on-disk active marker directly. + let paths = client.project_paths(&project_root).unwrap(); + if let Some(parent) = paths.active_path.parent() { + fs::create_dir_all(parent).unwrap(); + } + fs::write(&paths.active_path, "manual-choice\n").unwrap(); + + // Tick: the daemon must detect the divergence and learn from it. + evaluate_and_apply(&client, &mut controller, &project_root); + + let prefs_path = state_dir.join("automate-prefs.json"); + let prefs = Preferences::load(&prefs_path).expect("preferences should load"); + assert!( + prefs.bias_for("manual-choice") > 0.0, + "the user's manual choice must be rewarded" + ); + assert!( + prefs.bias_for("auto-pick") < 0.0, + "the rejected auto-pick must be decayed" + ); + + // The controller re-baselines to the manual choice so the same override + // is not re-learned on the next tick. + assert_eq!(controller.active_persona(), Some("manual-choice")); + } } diff --git a/crates/frameshift-orchestrator/src/audit.rs b/crates/frameshift-orchestrator/src/audit.rs index fc1cb30..e903589 100644 --- a/crates/frameshift-orchestrator/src/audit.rs +++ b/crates/frameshift-orchestrator/src/audit.rs @@ -1,5 +1,7 @@ //! Explainable audit log of persona transitions. +use std::collections::VecDeque; +use std::io::{BufRead, BufReader}; use std::path::Path; use chrono::Utc; @@ -34,25 +36,44 @@ pub struct AuditLog { } impl AuditLog { + /// Maximum number of audit entries retained in memory when loading. Entries + /// older than the most recent `MAX_AUDIT_ENTRIES` are dropped so a log grown + /// over a long-lived deployment cannot exhaust memory on load. + const MAX_AUDIT_ENTRIES: usize = 50_000; + /// Load an audit log from a JSON-lines file. /// - /// Returns an empty `AuditLog` if the file does not exist. Each line must - /// be a valid JSON object matching `Transition`. + /// Returns an empty `AuditLog` if the file does not exist. Each non-empty + /// line must be a valid JSON object matching `Transition`. Only the most + /// recent `MAX_AUDIT_ENTRIES` are retained. pub fn load(path: &Path) -> Result { + Self::load_capped(path, Self::MAX_AUDIT_ENTRIES) + } + + /// Load at most `max` most-recent entries, streaming the file line by line + /// so the full file contents are never held in memory at once. + fn load_capped(path: &Path, max: usize) -> Result { if !path.exists() { return Ok(AuditLog::default()); } - let data = std::fs::read_to_string(path)?; - let mut entries = Vec::new(); - for line in data.lines() { + let file = std::fs::File::open(path)?; + let reader = BufReader::new(file); + let mut entries: VecDeque = VecDeque::new(); + for line in reader.lines() { + let line = line?; let trimmed = line.trim(); if trimmed.is_empty() { continue; } let t: Transition = serde_json::from_str(trimmed)?; - entries.push(t); + entries.push_back(t); + if max > 0 && entries.len() > max { + entries.pop_front(); + } } - Ok(AuditLog { entries }) + Ok(AuditLog { + entries: entries.into(), + }) } /// Append a transition to both the in-memory log and the backing file. @@ -181,4 +202,22 @@ mod tests { assert_eq!(log.recent(100).len(), 1); } + + /// load_capped retains only the most recent `max` entries from the file. + #[test] + fn load_capped_keeps_most_recent() { + let tmp = TempDir::new().unwrap(); + let path = tmp.path().join("audit.jsonl"); + + let mut log = AuditLog::default(); + for i in 0..5 { + log.append(&path, make_transition(None, &format!("p{i}"))) + .unwrap(); + } + + let loaded = AuditLog::load_capped(&path, 3).unwrap(); + assert_eq!(loaded.entries.len(), 3, "only the 3 most recent retained"); + assert_eq!(loaded.entries.first().unwrap().to, "p2"); + assert_eq!(loaded.entries.last().unwrap().to, "p4"); + } } diff --git a/crates/frameshift-orchestrator/src/context.rs b/crates/frameshift-orchestrator/src/context.rs index ce85a1a..ebca587 100644 --- a/crates/frameshift-orchestrator/src/context.rs +++ b/crates/frameshift-orchestrator/src/context.rs @@ -20,6 +20,33 @@ const SKIP_DIRS: &[&str] = &[ ".svn", ]; +/// Weight assigned to languages present in the active git working set. +/// +/// Sits above the file-count census ceiling (1.0) but below the prose sentinel +/// (2.0, see [`augment_languages_from_task`]) so that what the user is actively +/// editing outranks the static census without overriding an explicit +/// prose-writing signal. +const ACTIVE_LANGUAGE_WEIGHT: f32 = 1.5; + +/// Upper bound on changed-file paths processed from `git status` output. +/// +/// Bounds work on pathological repos (e.g. a freshly checked-out tree reported +/// as fully untracked); far more than enough to characterize the active set. +const MAX_CHANGED_FILES: usize = 5000; + +/// Upper bound on dependency-name tokens harvested from project manifests. +/// +/// Keeps the additive context-token signal bounded on dependency-heavy projects. +const MAX_DEP_TOKENS: usize = 100; + +/// Manifest section headers under which a TOML key names a dependency. +const CARGO_DEP_SECTIONS: &[&str] = &[ + "[dependencies]", + "[dev-dependencies]", + "[build-dependencies]", + "[workspace.dependencies]", +]; + /// A snapshot of the inferred work context for a project directory. #[derive(Debug, Clone, PartialEq)] pub struct ContextSignal { @@ -39,6 +66,12 @@ pub struct ContextSignal { /// Used by the policy scorer for lexical matching against persona keywords. pub task_tokens: Vec, + /// Lowercase dependency names parsed from project manifests (Cargo.toml, + /// package.json). Scored by the policy as a small additive bonus when they + /// match persona keywords -- they sharpen framework-level matching (e.g. + /// `axum` vs `actix`) without diluting the task-token lexical score. + pub context_tokens: Vec, + /// The inferred task intent from task token analysis, if any. pub inferred_intent: Option, } @@ -91,6 +124,16 @@ pub fn sense(project_root: &Path, task_hint: Option<&str>) -> ContextSignal { // personas can compete with code-language personas on equal footing. let languages = augment_languages_from_task(languages, &task_tokens); + // Emphasize the languages of the active git working set (best-effort): the + // files the user is editing right now are a stronger signal than the static + // whole-repo census. Non-git directories degrade to the census unchanged. + let changed_languages = changed_file_languages(&git_changed_paths(project_root)); + let languages = augment_languages_from_git(languages, &changed_languages); + + // Harvest dependency names from project manifests (best-effort). These feed + // the policy as a small additive bonus, sharpening framework-level matching. + let context_tokens = manifest_dependency_tokens(project_root); + // Classify the inferred task intent from task token analysis. let inferred_intent = crate::intent::classify(&task_tokens); @@ -99,6 +142,7 @@ pub fn sense(project_root: &Path, task_hint: Option<&str>) -> ContextSignal { languages, frameworks, task_tokens, + context_tokens, inferred_intent, } } @@ -129,16 +173,26 @@ fn walk( break; } + // Use the entry's own file type (this does not follow symlinks) so a + // planted symlink cannot redirect the walk outside the project tree. + let file_type = match entry.file_type() { + Ok(ft) => ft, + Err(_) => continue, + }; + if file_type.is_symlink() { + continue; + } + let path = entry.path(); let name = entry.file_name(); let name_str = name.to_string_lossy(); - if path.is_dir() { + if file_type.is_dir() { if SKIP_DIRS.contains(&name_str.as_ref()) { continue; } walk(&path, depth + 1, raw_counts, frameworks, file_count); - } else if path.is_file() { + } else if file_type.is_file() { *file_count += 1; // Check for framework marker files. @@ -361,6 +415,245 @@ fn augment_languages_from_task( languages } +/// Best-effort list of files in the active git working set for `project_root`. +/// +/// Runs `git status --porcelain` (which reports staged, unstaged, and untracked +/// changes) with the project as the working directory. Returns an empty vector +/// on any failure -- not a git repository, git not installed, or a non-zero +/// exit -- so callers degrade gracefully to the static project census. +fn git_changed_paths(project_root: &Path) -> Vec { + let output = std::process::Command::new("git") + .arg("-C") + .arg(project_root) + .args(["status", "--porcelain", "--untracked-files=all"]) + .output(); + match output { + Ok(o) if o.status.success() => { + let stdout = String::from_utf8_lossy(&o.stdout); + parse_porcelain_paths(&stdout) + } + _ => Vec::new(), + } +} + +/// Extract file paths from `git status --porcelain` (v1) output. +/// +/// Each line is `XY `: a two-character status code, a space, then the +/// path. Rename and copy entries use `XY -> `; the destination path +/// is taken. Surrounding quotes (added by git for paths with special +/// characters) are stripped -- extension mapping still works on the suffix. +/// Capped at [`MAX_CHANGED_FILES`] entries. +fn parse_porcelain_paths(porcelain: &str) -> Vec { + porcelain + .lines() + .filter_map(|line| { + // Need at least the 2 status chars, a space, and one path char. + if line.len() < 4 { + return None; + } + let rest = &line[3..]; + // Rename/copy: "old -> new" keeps the destination path. + let path = match rest.rsplit_once(" -> ") { + Some((_, new)) => new, + None => rest, + }; + let path = path.trim().trim_matches('"'); + if path.is_empty() { + None + } else { + Some(path.to_string()) + } + }) + .take(MAX_CHANGED_FILES) + .collect() +} + +/// Map a set of changed file paths to per-language hit counts using the same +/// extension table as the project census ([`ext_to_language`]). Paths whose +/// extension does not map to a tracked language are ignored. +pub(crate) fn changed_file_languages(paths: &[String]) -> BTreeMap { + let mut counts: BTreeMap = BTreeMap::new(); + for path in paths { + if let Some(ext) = Path::new(path).extension().and_then(|e| e.to_str()) { + if let Some(lang) = ext_to_language(ext) { + *counts.entry(lang.to_string()).or_insert(0) += 1; + } + } + } + counts +} + +/// Raise the weight of every language in the active git working set to +/// [`ACTIVE_LANGUAGE_WEIGHT`], so files the user is currently editing outrank +/// the static file-count census. A language already weighted higher (e.g. the +/// prose sentinel) is left untouched. +fn augment_languages_from_git( + mut languages: BTreeMap, + changed: &BTreeMap, +) -> BTreeMap { + for lang in changed.keys() { + let entry = languages.entry(lang.clone()).or_insert(0.0); + if *entry < ACTIVE_LANGUAGE_WEIGHT { + *entry = ACTIVE_LANGUAGE_WEIGHT; + } + } + languages +} + +/// Best-effort dependency-name tokens harvested from the project's root +/// manifests (`Cargo.toml` and `package.json`). +/// +/// Returns lowercase, deduplicated names (length >= 2) capped at +/// [`MAX_DEP_TOKENS`], in first-seen order. Missing or unreadable manifests +/// contribute nothing, so non-manifest projects yield an empty list. +fn manifest_dependency_tokens(project_root: &Path) -> Vec { + let mut names: Vec = Vec::new(); + if let Some(content) = read_capped(&project_root.join("Cargo.toml")) { + names.extend(parse_cargo_dependencies(&content)); + } + if let Some(content) = read_capped(&project_root.join("package.json")) { + names.extend(parse_package_json_dependencies(&content)); + } + + let mut seen = std::collections::HashSet::new(); + names + .into_iter() + .map(|n| n.to_lowercase()) + .filter(|n| n.len() >= 2 && seen.insert(n.clone())) + .take(MAX_DEP_TOKENS) + .collect() +} + +/// Read a file to a string, returning `None` on any error and truncating (at a +/// char boundary) to a bounded size so a pathological manifest cannot blow up +/// the scan. Manifests are normally tiny; the cap is purely defensive. +fn read_capped(path: &Path) -> Option { + const MAX_MANIFEST_BYTES: usize = 256 * 1024; + let mut content = std::fs::read_to_string(path).ok()?; + if content.len() > MAX_MANIFEST_BYTES { + let mut end = MAX_MANIFEST_BYTES; + while end > 0 && !content.is_char_boundary(end) { + end -= 1; + } + content.truncate(end); + } + Some(content) +} + +/// Extract dependency names from `Cargo.toml` content via a line scan. +/// +/// Tracks the current `[section]` header and, while inside a dependency section +/// ([`CARGO_DEP_SECTIONS`]), takes the key to the left of `=` on each entry. The +/// `name.workspace = true` / `name.version = ".."` forms yield `name` (the part +/// before the first dot). The `[dependencies.NAME]` sub-table header yields +/// `NAME`. This is a best-effort scan, not a full TOML parse -- unusual +/// formatting simply yields fewer names. +fn parse_cargo_dependencies(content: &str) -> Vec { + let mut names = Vec::new(); + let mut in_dep_section = false; + for raw in content.lines() { + let line = raw.trim(); + if line.is_empty() || line.starts_with('#') { + continue; + } + if line.starts_with('[') { + in_dep_section = CARGO_DEP_SECTIONS.contains(&line); + if let Some(name) = cargo_subtable_dep(line) { + if is_valid_dep_name(&name) { + names.push(name); + } + } + continue; + } + if !in_dep_section { + continue; + } + if let Some((key, _)) = line.split_once('=') { + // `name.workspace`/`name.version` -> take the leading segment. + let name = key + .trim() + .split('.') + .next() + .unwrap_or("") + .trim() + .trim_matches('"'); + if is_valid_dep_name(name) { + names.push(name.to_string()); + } + } + } + names +} + +/// Extract the dependency name from a `[dependencies.NAME]` style sub-table +/// header, or `None` if the header is not a dependency sub-table. +fn cargo_subtable_dep(header: &str) -> Option { + let inner = header.strip_prefix('[')?.strip_suffix(']')?; + for prefix in [ + "dependencies.", + "dev-dependencies.", + "build-dependencies.", + "workspace.dependencies.", + ] { + if let Some(rest) = inner.strip_prefix(prefix) { + let name = rest.split('.').next().unwrap_or("").trim(); + return Some(name.to_string()); + } + } + None +} + +/// Extract dependency names from `package.json` content via a line scan. +/// +/// Tracks whether the scan is inside a dependency object (`dependencies`, +/// `devDependencies`, `peerDependencies`, `optionalDependencies`) and takes the +/// quoted key from each `"name": "range"` entry, stopping at the closing brace. +/// Best-effort for the conventional pretty-printed layout (one dependency per +/// line); a minified single-line file yields little or nothing. +fn parse_package_json_dependencies(content: &str) -> Vec { + let mut names = Vec::new(); + let mut in_deps = false; + for raw in content.lines() { + let line = raw.trim(); + if line.contains("\"dependencies\"") + || line.contains("\"devDependencies\"") + || line.contains("\"peerDependencies\"") + || line.contains("\"optionalDependencies\"") + { + in_deps = true; + continue; + } + if !in_deps { + continue; + } + if line.starts_with('}') { + in_deps = false; + continue; + } + // Entry form: `"name": "range",`. + if let Some(rest) = line.strip_prefix('"') { + if let Some((key, _)) = rest.split_once('"') { + if is_valid_dep_name(key) { + names.push(key.to_string()); + } + } + } + } + names +} + +/// Whether `name` looks like a real dependency identifier rather than a parsing +/// artifact: non-empty, bounded length, made of name characters, and containing +/// at least one alphanumeric. Allows the npm scope/path characters `@` and `/`. +fn is_valid_dep_name(name: &str) -> bool { + !name.is_empty() + && name.len() <= 64 + && name + .chars() + .all(|c| c.is_ascii_alphanumeric() || matches!(c, '-' | '_' | '/' | '@' | '.')) + && name.chars().any(|c| c.is_ascii_alphanumeric()) +} + /// Push `value` into `vec` only if not already present (cheap dedup during walk). fn push_unique(vec: &mut Vec, value: &str) { if !vec.iter().any(|v| v == value) { @@ -491,4 +784,225 @@ mod tests { assert!(tokens.contains(&"security".to_string())); assert!(tokens.contains(&"cve".to_string())); } + + /// Symlinked directories are not followed during the project walk. + #[cfg(unix)] + #[test] + fn walk_does_not_follow_symlinks() { + use std::os::unix::fs::symlink; + let tmp = tempfile::tempdir().unwrap(); + + // A directory outside the project, full of rust files. + let outside = tmp.path().join("outside"); + fs::create_dir_all(&outside).unwrap(); + for i in 0..3 { + fs::write(outside.join(format!("lib{i}.rs")), "fn x() {}").unwrap(); + } + + // The project contains one python file and a symlink to `outside`. + let project = tmp.path().join("project"); + fs::create_dir_all(&project).unwrap(); + fs::write(project.join("app.py"), "x = 1").unwrap(); + symlink(&outside, project.join("linked")).unwrap(); + + let sig = sense(&project, None); + assert!( + sig.languages.contains_key("python"), + "the real project file should still be detected" + ); + assert!( + !sig.languages.contains_key("rust"), + "a symlinked directory must not be followed" + ); + } + + /// changed_file_languages maps recognized extensions and ignores the rest. + #[test] + fn changed_file_languages_maps_extensions() { + let paths = vec![ + "src/main.rs".to_string(), + "web/app.py".to_string(), + "README".to_string(), + "notes.rs".to_string(), + ]; + let langs = changed_file_languages(&paths); + assert_eq!(langs.get("rust").copied(), Some(2)); + assert_eq!(langs.get("python").copied(), Some(1)); + // README has no extension and contributes nothing. + assert_eq!(langs.len(), 2); + } + + /// parse_porcelain_paths handles status codes, untracked, and renames. + #[test] + fn parse_porcelain_extracts_paths() { + let out = " M src/main.rs\n?? new.py\nA staged.go\nR old.rs -> renamed.rs\n"; + let paths = parse_porcelain_paths(out); + assert!(paths.contains(&"src/main.rs".to_string())); + assert!(paths.contains(&"new.py".to_string())); + assert!(paths.contains(&"staged.go".to_string())); + // Rename keeps the destination, not the source. + assert!(paths.contains(&"renamed.rs".to_string())); + assert!(!paths.iter().any(|p| p.contains("old.rs"))); + } + + /// The git boost lifts an actively-edited minority language above the + /// census-dominant one. + #[test] + fn git_boost_lifts_minority_language() { + use std::collections::BTreeMap; + let mut census = BTreeMap::new(); + census.insert("javascript".to_string(), 1.0_f32); + census.insert("rust".to_string(), 0.3_f32); + let mut changed = BTreeMap::new(); + changed.insert("rust".to_string(), 2_usize); + + let boosted = augment_languages_from_git(census, &changed); + assert_eq!(boosted.get("rust").copied(), Some(ACTIVE_LANGUAGE_WEIGHT)); + assert!(boosted["rust"] > boosted["javascript"]); + } + + /// The git boost never lowers a language already weighted above the active + /// weight (e.g. the prose sentinel). + #[test] + fn git_boost_preserves_prose_sentinel() { + use std::collections::BTreeMap; + let mut census = BTreeMap::new(); + census.insert("prose".to_string(), 2.0_f32); + let mut changed = BTreeMap::new(); + changed.insert("rust".to_string(), 1_usize); + + let boosted = augment_languages_from_git(census, &changed); + assert_eq!(boosted.get("prose").copied(), Some(2.0)); + assert_eq!(boosted.get("rust").copied(), Some(ACTIVE_LANGUAGE_WEIGHT)); + } + + /// A non-git directory yields no boost: the census weight (<= 1.0) stands + /// and sense() does not panic. + #[test] + fn sense_on_non_git_dir_does_not_boost() { + let tmp = make_rust_project(); + let sig = sense(tmp.path(), None); + let rust_w = sig.languages.get("rust").copied().unwrap_or(0.0); + assert!( + rust_w > 0.0 && rust_w <= 1.0, + "expected census weight, not the active boost: {rust_w}" + ); + } + + /// In a real repo, an actively-edited Rust file outranks a census-dominant + /// JavaScript majority. Skipped if git is unavailable in the environment. + #[test] + fn sense_boosts_git_changed_languages() { + use std::process::Command; + if Command::new("git").arg("--version").output().is_err() { + return; + } + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + let git = |args: &[&str]| { + Command::new("git") + .arg("-C") + .arg(root) + .args(args) + .output() + .expect("git invocation"); + }; + git(&["init", "-q"]); + git(&["config", "user.email", "t@example.com"]); + git(&["config", "user.name", "tester"]); + + // Census-dominant language: three committed JavaScript files. + for i in 0..3 { + fs::write(root.join(format!("a{i}.js")), "var x = 1;").unwrap(); + } + git(&["add", "-A"]); + git(&["commit", "-q", "-m", "init"]); + + // Active edit: a single untracked Rust file. + fs::write(root.join("lib.rs"), "fn x() {}").unwrap(); + + let sig = sense(root, None); + let rust_w = sig.languages.get("rust").copied().unwrap_or(0.0); + let js_w = sig.languages.get("javascript").copied().unwrap_or(0.0); + assert!( + rust_w >= ACTIVE_LANGUAGE_WEIGHT, + "the actively-edited rust file should boost rust, got {rust_w}" + ); + assert!( + rust_w > js_w, + "actively-edited rust ({rust_w}) should outrank census-dominant js ({js_w})" + ); + } + + /// parse_cargo_dependencies pulls names from dependency sections only. + #[test] + fn parse_cargo_deps_extracts_names() { + let toml = "[package]\nname = \"x\"\n\n[dependencies]\nserde = \"1\"\n\ + axum = { version = \"0.7\" }\ntokio.workspace = true\n\n\ + [dev-dependencies]\ntempfile = \"3\"\n"; + let deps = parse_cargo_dependencies(toml); + assert!(deps.contains(&"serde".to_string())); + assert!(deps.contains(&"axum".to_string())); + assert!(deps.contains(&"tokio".to_string())); + assert!(deps.contains(&"tempfile".to_string())); + // The [package] name field is not a dependency. + assert!(!deps.contains(&"name".to_string())); + } + + /// parse_cargo_dependencies handles `[dependencies.NAME]` sub-tables and does + /// not treat their fields as dependencies. + #[test] + fn parse_cargo_deps_handles_subtables() { + let toml = "[dependencies.bb8]\nversion = \"0.9\"\n[dependencies]\nserde = \"1\"\n"; + let deps = parse_cargo_dependencies(toml); + assert!(deps.contains(&"bb8".to_string())); + assert!(deps.contains(&"serde".to_string())); + assert!(!deps.contains(&"version".to_string())); + } + + /// parse_package_json_dependencies pulls names from dependency objects only. + #[test] + fn parse_package_json_deps_extracts_names() { + let json = "{\n \"name\": \"app\",\n \"dependencies\": {\n \ + \"react\": \"^18\",\n \"next\": \"14\"\n },\n \ + \"devDependencies\": {\n \"vitest\": \"^1\"\n }\n}\n"; + let deps = parse_package_json_dependencies(json); + assert!(deps.contains(&"react".to_string())); + assert!(deps.contains(&"next".to_string())); + assert!(deps.contains(&"vitest".to_string())); + // The top-level package name is not a dependency. + assert!(!deps.contains(&"name".to_string())); + } + + /// manifest_dependency_tokens lowercases and deduplicates harvested names. + #[test] + fn manifest_tokens_lowercases_and_dedups() { + let tmp = tempfile::tempdir().unwrap(); + fs::write( + tmp.path().join("Cargo.toml"), + "[dependencies]\naxum = \"0.7\"\nSERDE = \"1\"\naxum = \"0.7\"\n", + ) + .unwrap(); + let tokens = manifest_dependency_tokens(tmp.path()); + assert!(tokens.contains(&"axum".to_string())); + assert!(tokens.contains(&"serde".to_string())); + assert_eq!( + tokens.iter().filter(|t| *t == "axum").count(), + 1, + "duplicate dependency names are collapsed" + ); + } + + /// sense() exposes manifest dependency names via context_tokens. + #[test] + fn sense_populates_context_tokens_from_manifest() { + let tmp = tempfile::tempdir().unwrap(); + fs::write( + tmp.path().join("Cargo.toml"), + "[package]\nname = \"x\"\n[dependencies]\naxum = \"0.7\"\n", + ) + .unwrap(); + let sig = sense(tmp.path(), None); + assert!(sig.context_tokens.contains(&"axum".to_string())); + } } diff --git a/crates/frameshift-orchestrator/src/controller.rs b/crates/frameshift-orchestrator/src/controller.rs index 95a108c..f04fc99 100644 --- a/crates/frameshift-orchestrator/src/controller.rs +++ b/crates/frameshift-orchestrator/src/controller.rs @@ -54,11 +54,16 @@ impl SwitchPolicy { /// - z_threshold: 1.0 - sensitivity * 0.7 (range 1.0 to 0.3) /// - min_gap_fraction: 0.15 - sensitivity * 0.12 (range 0.15 to 0.03) /// - debounce_ticks: max(0, 3 - floor(sensitivity * 3)) (range 3 to 0) + /// - min_confidence: max(0, 0.5 - sensitivity) * 0.6 (range 0.3 to 0.0) + /// - switch_margin: max(0, 0.5 - sensitivity) * 0.3 (range 0.15 to 0.0) pub fn from_sensitivity(sensitivity: f32) -> Self { let s = sensitivity.clamp(0.0, 1.0); SwitchPolicy { - min_confidence: 0.0, - switch_margin: 0.0, + // Below the midpoint (the conservative end) impose a confidence floor + // and a score margin the challenger must clear; at and above 0.5 both + // are 0.0, leaving switching to the distribution/debounce heuristics. + min_confidence: (0.5 - s).max(0.0) * 0.6, + switch_margin: (0.5 - s).max(0.0) * 0.3, debounce_ticks: (3.0 - (s * 3.0).floor()).max(0.0) as u32, z_threshold: 1.0 - s * 0.7, min_gap_fraction: 0.15 - s * 0.12, @@ -178,6 +183,47 @@ impl SwitchController { &self.state } + /// Return the name of the persona the controller currently tracks as active. + /// + /// Returns `Some` when the controller is `Active` or `Locked` on a named + /// persona, and `None` when `Off`, `Armed`, or `Locked` with no name. The + /// daemon uses this as its "last auto-pick" to detect when the on-disk + /// active marker has been changed out from under it (a manual override). + pub fn active_persona(&self) -> Option<&str> { + match &self.state { + AutomateState::Active { persona, .. } | AutomateState::Locked { persona } => { + if persona.is_empty() { + None + } else { + Some(persona.as_str()) + } + } + AutomateState::Off | AutomateState::Armed => None, + } + } + + /// Adopt an externally-applied persona as the new active baseline. + /// + /// Used when something outside the controller (a user manual switch) has + /// already changed the active persona. The controller records `persona` as + /// `Active` so subsequent `decide` calls apply hysteresis from the user's + /// choice rather than the controller's stale auto-pick, and so the same + /// override is not detected and re-learned on every tick. Debounce and + /// challenger tracking are reset. A user-asserted choice is treated as + /// maximally confident; the stored confidence is informational only and is + /// not read by `decide`. Locked state is left untouched. + pub fn adopt_active(&mut self, persona: &str) { + if matches!(self.state, AutomateState::Locked { .. }) { + return; + } + self.state = AutomateState::Active { + persona: persona.to_string(), + confidence: 1.0, + }; + self.debounce_count = 0; + self.challenger = None; + } + /// Evaluate a fresh ranking and decide whether to switch personas. /// /// Logic: @@ -204,7 +250,10 @@ impl SwitchController { match &self.state.clone() { AutomateState::Off | AutomateState::Armed => { - // Armed/Off: accept top candidate immediately without threshold gating. + // Do not activate a candidate below the confidence floor. + if top.confidence < self.policy.min_confidence { + return Decision::Hold; + } self.state = AutomateState::Active { persona: top.persona.clone(), confidence: top.confidence, @@ -228,6 +277,27 @@ impl SwitchController { return Decision::Hold; } + // Confidence floor: never switch to a challenger we are not + // confident in. + if top.confidence < self.policy.min_confidence { + self.debounce_count = 0; + self.challenger = None; + return Decision::Hold; + } + + // Margin floor: the challenger must beat the current active persona + // by at least switch_margin before any switch is considered. + let current_score = ranked + .iter() + .find(|s| s.persona == *current) + .map(|s| s.score) + .unwrap_or(0.0); + if top.score - current_score < self.policy.switch_margin { + self.debounce_count = 0; + self.challenger = None; + return Decision::Hold; + } + // Distribution-aware switching: compute mean and stddev of all scores. let scores: Vec = ranked.iter().map(|s| s.score).collect(); let mean = scores.iter().sum::() / scores.len() as f32; @@ -317,6 +387,8 @@ mod tests { lexical: 0.0, intent: 0.0, capability: 0.0, + context: 0.0, + semantic: 0.0, }, } } @@ -520,5 +592,50 @@ mod tests { let policy = SwitchPolicy::from_sensitivity(0.5); assert!((policy.z_threshold - 0.65).abs() < 0.01); assert_eq!(policy.debounce_ticks, 2); + // At and above the midpoint the confidence/margin floors are disabled, + // so default behavior is governed by the distribution heuristics only. + assert_eq!(policy.min_confidence, 0.0); + assert_eq!(policy.switch_margin, 0.0); + } + + /// A top candidate below min_confidence does not activate. + #[test] + fn min_confidence_blocks_low_confidence_activation() { + let policy = SwitchPolicy { + min_confidence: 0.5, + switch_margin: 0.0, + debounce_ticks: 0, + z_threshold: 0.65, + min_gap_fraction: 0.09, + }; + let mut ctrl = SwitchController::new(policy); + ctrl.arm(); + // Top candidate confidence 0.3 is below the 0.5 floor. + let ranked = vec![scored("alpha", 0.9, 0.3)]; + let d = ctrl.decide(&ranked); + assert_eq!(d, Decision::Hold, "low-confidence top must not activate"); + assert_eq!(*ctrl.state(), AutomateState::Armed, "state stays Armed"); + } + + /// A challenger that does not beat the active persona by switch_margin holds. + #[test] + fn switch_margin_blocks_marginal_challenger() { + let policy = SwitchPolicy { + min_confidence: 0.0, + switch_margin: 0.3, + debounce_ticks: 0, + z_threshold: 0.0, + min_gap_fraction: 0.0, + }; + let mut ctrl = SwitchController::new(policy); + ctrl.arm(); + // Activate alpha. + let ranked_a = vec![scored("alpha", 0.60, 0.9), scored("beta", 0.10, 0.2)]; + ctrl.decide(&ranked_a); + // Beta edges ahead by only 0.05, below the 0.3 switch_margin. Without the + // margin floor the lenient z/gap/debounce settings would switch here. + let ranked_b = vec![scored("beta", 0.65, 0.9), scored("alpha", 0.60, 0.5)]; + let d = ctrl.decide(&ranked_b); + assert_eq!(d, Decision::Hold, "margin below switch_margin must hold"); } } diff --git a/crates/frameshift-orchestrator/src/embed.rs b/crates/frameshift-orchestrator/src/embed.rs new file mode 100644 index 0000000..815e078 --- /dev/null +++ b/crates/frameshift-orchestrator/src/embed.rs @@ -0,0 +1,137 @@ +//! Semantic embedding abstraction for meaning-based persona matching. +//! +//! This is the dependency-free Phase 1 scaffolding for semantic selection: it +//! defines the [`Embedder`] trait and the cosine-similarity math the policy +//! scorer uses, but ships no concrete embedding engine. A real embedder (a +//! pure-Rust `candle` model, or `fastembed`/`ort`, plus model distribution) is +//! Phase 2 and is gated on an explicit decision because it adds a heavy ML +//! dependency to an otherwise lean workspace. +//! +//! Because no production caller supplies an [`Embedder`] yet, the semantic +//! channel contributes `0.0` everywhere and selection behavior is unchanged. + +/// Produces a dense vector embedding for a piece of text. +/// +/// Implementations map natural-language text to a fixed-dimensional vector +/// whose geometry encodes meaning, so that related texts have a high cosine +/// similarity. The trait is object-safe (`&dyn Embedder`) so the scorer can +/// accept an optional embedder without being generic over its type. Two calls +/// with equal input should return equal output (determinism), and all vectors +/// from a given embedder should share one dimensionality. +pub trait Embedder { + /// Return the embedding vector for `text`. + fn embed(&self, text: &str) -> Vec; +} + +/// Cosine similarity between two equal-length vectors, clamped to `[0.0, 1.0]`. +/// +/// Returns `0.0` (a safe no-signal value) rather than panicking or producing +/// `NaN` when the inputs are degenerate: empty vectors, vectors of differing +/// length, or a vector whose L2 norm is effectively zero. Negative cosine +/// (anti-correlated vectors) is clamped to `0.0` because the scorer treats this +/// as an additive similarity bonus, never a penalty. +pub fn cosine_similarity(a: &[f32], b: &[f32]) -> f32 { + if a.is_empty() || a.len() != b.len() { + return 0.0; + } + let dot: f32 = a.iter().zip(b.iter()).map(|(x, y)| x * y).sum(); + let norm_a = a.iter().map(|x| x * x).sum::().sqrt(); + let norm_b = b.iter().map(|x| x * x).sum::().sqrt(); + if norm_a <= f32::EPSILON || norm_b <= f32::EPSILON { + return 0.0; + } + (dot / (norm_a * norm_b)).clamp(0.0, 1.0) +} + +/// Embed both texts with `embedder` and return their cosine similarity in +/// `[0.0, 1.0]`. +/// +/// Convenience wrapper over [`cosine_similarity`]. Inherits its degenerate-case +/// handling, so an embedder that returns empty or mismatched-length vectors +/// yields `0.0` rather than an error. +pub fn semantic_similarity(embedder: &dyn Embedder, a: &str, b: &str) -> f32 { + let va = embedder.embed(a); + let vb = embedder.embed(b); + cosine_similarity(&va, &vb) +} + +/// A deterministic bag-of-words embedder used only by tests. +/// +/// Hashes each whitespace-separated, lowercased word into a fixed-width bucket +/// and counts occurrences. It is order-insensitive and dependency-free, so two +/// texts that share words have overlapping nonzero buckets and therefore a +/// positive cosine similarity -- enough to exercise the semantic channel +/// without a real embedding model. +#[cfg(test)] +pub(crate) struct BagOfWordsEmbedder; + +#[cfg(test)] +impl Embedder for BagOfWordsEmbedder { + /// Embed `text` as a 64-dimensional word-occurrence histogram. + fn embed(&self, text: &str) -> Vec { + const DIM: usize = 64; + let mut v = vec![0.0f32; DIM]; + for word in text.split_whitespace() { + let lowered = word.to_lowercase(); + // FNV-1a hash -> bucket index. Stable across runs (no randomness). + let mut h: u64 = 0xcbf29ce484222325; + for byte in lowered.bytes() { + h ^= byte as u64; + h = h.wrapping_mul(0x100000001b3); + } + v[(h as usize) % DIM] += 1.0; + } + v + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Identical vectors have cosine similarity 1.0. + #[test] + fn identical_vectors_are_maximally_similar() { + let v = vec![1.0, 2.0, 3.0]; + assert!((cosine_similarity(&v, &v) - 1.0).abs() < 1e-6); + } + + /// Orthogonal vectors have cosine similarity 0.0. + #[test] + fn orthogonal_vectors_are_dissimilar() { + let a = vec![1.0, 0.0]; + let b = vec![0.0, 1.0]; + assert_eq!(cosine_similarity(&a, &b), 0.0); + } + + /// Anti-correlated vectors clamp to 0.0 (no negative bonus). + #[test] + fn anti_correlated_clamps_to_zero() { + let a = vec![1.0, 0.0]; + let b = vec![-1.0, 0.0]; + assert_eq!(cosine_similarity(&a, &b), 0.0); + } + + /// Empty, mismatched-length, and zero-norm inputs yield 0.0 (no panic, no NaN). + #[test] + fn degenerate_inputs_yield_zero() { + assert_eq!(cosine_similarity(&[], &[]), 0.0); + assert_eq!(cosine_similarity(&[1.0, 2.0], &[1.0]), 0.0); + assert_eq!(cosine_similarity(&[0.0, 0.0], &[1.0, 1.0]), 0.0); + } + + /// The mock embedder gives a high similarity to identical text and a + /// positive-but-lower similarity to text that merely shares words. + #[test] + fn mock_embedder_reflects_word_overlap() { + let emb = BagOfWordsEmbedder; + let same = semantic_similarity(&emb, "rust cargo clippy", "rust cargo clippy"); + let related = semantic_similarity(&emb, "rust cargo clippy", "rust cargo ownership"); + let unrelated = semantic_similarity(&emb, "rust cargo clippy", "tacos"); + + assert!((same - 1.0).abs() < 1e-6, "identical text -> 1.0"); + assert!(related > 0.0, "shared words -> positive similarity"); + assert!(related < same, "partial overlap < full overlap"); + assert!(unrelated < related, "no shared words -> least similar"); + } +} diff --git a/crates/frameshift-orchestrator/src/feedback.rs b/crates/frameshift-orchestrator/src/feedback.rs index 25b57de..8ae0359 100644 --- a/crates/frameshift-orchestrator/src/feedback.rs +++ b/crates/frameshift-orchestrator/src/feedback.rs @@ -188,12 +188,23 @@ impl Preferences { } /// Persist preferences to a JSON file, creating parent directories as needed. + /// + /// The write is atomic: data is serialized to a uniquely-named temp file in + /// the same directory and then renamed over the target, so a crash or a + /// concurrent writer never leaves a half-written file that `load` would fail + /// to parse. pub fn save(&self, path: &Path) -> Result<(), OrchestratorError> { if let Some(parent) = path.parent() { std::fs::create_dir_all(parent)?; } let data = serde_json::to_string_pretty(self)?; - std::fs::write(path, data)?; + let file_name = path + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or("preferences.json"); + let tmp_path = path.with_file_name(format!(".{file_name}.tmp.{}", std::process::id())); + std::fs::write(&tmp_path, data.as_bytes())?; + std::fs::rename(&tmp_path, path)?; Ok(()) } } @@ -262,6 +273,28 @@ mod tests { assert!((loaded.bias_for("a") - prefs.bias_for("a")).abs() < f32::EPSILON); } + /// Saving writes atomically and leaves no temporary file behind. + #[test] + fn save_leaves_no_temp_file() { + let tmp = TempDir::new().unwrap(); + let path = tmp.path().join("prefs.json"); + + let mut prefs = Preferences::new(); + prefs.record_override(Some("a"), "b"); + prefs.save(&path).unwrap(); + + assert!(path.exists()); + let leftovers: Vec<_> = std::fs::read_dir(tmp.path()) + .unwrap() + .filter_map(|e| e.ok()) + .filter(|e| e.file_name().to_string_lossy().contains(".tmp.")) + .collect(); + // No temp file should remain after a successful save. + assert!(leftovers.is_empty()); + // And the saved file loads cleanly. + Preferences::load(&path).unwrap(); + } + /// record_override_with_intent records a per-intent bias for the chosen persona. #[test] fn per_intent_bias_is_recorded() { diff --git a/crates/frameshift-orchestrator/src/lib.rs b/crates/frameshift-orchestrator/src/lib.rs index 3360a2c..ca18e32 100644 --- a/crates/frameshift-orchestrator/src/lib.rs +++ b/crates/frameshift-orchestrator/src/lib.rs @@ -15,6 +15,7 @@ pub mod audit; pub mod context; pub mod controller; +pub mod embed; pub mod error; pub mod feedback; pub mod index; @@ -27,14 +28,16 @@ pub mod run; pub use audit::{now_timestamp, AuditLog, Transition}; pub use context::{sense, ContextSignal}; pub use controller::{AutomateState, Decision, SwitchController, SwitchPolicy}; +pub use embed::{cosine_similarity, semantic_similarity, Embedder}; pub use error::OrchestratorError; pub use feedback::Preferences; pub use index::{PersonaIndex, PersonaProfile}; pub use intent::{classify as classify_intent, Intent}; pub use mode::{Mode, ModeState}; -pub use policy::{rank, PolicyWeights, ScoreComponents, Scored}; +pub use policy::{rank, rank_with_embedder, PolicyWeights, ScoreComponents, Scored}; pub use run::{ - select, select_rich, CandidateOutput, ContextSnapshot, SelectionInputs, SelectionOutput, + select, select_rich, select_with_embedder, CandidateOutput, ContextSnapshot, SelectionInputs, + SelectionOutput, }; /// Facade that wires together the index, weights, policy, preferences, and controller. @@ -136,6 +139,7 @@ mod tests { }, frameworks: vec![], task_tokens: vec![], + context_tokens: vec![], inferred_intent: None, }; // With only one persona and confidence depends on scoring; just verify no panic. diff --git a/crates/frameshift-orchestrator/src/policy.rs b/crates/frameshift-orchestrator/src/policy.rs index 0f68c4e..8647ac6 100644 --- a/crates/frameshift-orchestrator/src/policy.rs +++ b/crates/frameshift-orchestrator/src/policy.rs @@ -1,8 +1,26 @@ //! Scoring policy: rank personas against a context signal. use crate::context::ContextSignal; +use crate::embed::{semantic_similarity, Embedder}; use crate::feedback::Preferences; -use crate::index::PersonaIndex; +use crate::index::{PersonaIndex, PersonaProfile}; + +/// Score added per project-dependency token (from `ContextSignal::context_tokens`) +/// that matches a persona keyword. +const CONTEXT_TOKEN_HIT: f32 = 0.04; + +/// Maximum additive bonus contributed by the semantic-similarity channel. +/// +/// The cosine similarity in `[0, 1]` is scaled by this weight and added on top +/// of the weighted blend, mirroring the context bonus: it can lift a persona +/// whose meaning matches the task but never penalizes others or redistributes +/// the primary weights. Contributes `0.0` when no embedder is supplied. +const SEMANTIC_WEIGHT: f32 = 0.15; + +/// Maximum total bonus contributed by dependency-token matches. Capped so a +/// dependency-heavy project cannot let framework matching dominate the language, +/// lexical, and intent signals. +const CONTEXT_TOKEN_CAP: f32 = 0.12; /// Weights controlling the relative contribution of each scoring component. /// @@ -43,6 +61,14 @@ pub struct ScoreComponents { pub intent: f32, /// Capability heuristic component (0..1). pub capability: f32, + /// Project-signal match bonus (0..[`CONTEXT_TOKEN_CAP`]) from dependency and + /// build-framework matches, added on top of the weighted blend rather than + /// diluting the lexical channel. + pub context: f32, + /// Semantic-similarity bonus (0..[`SEMANTIC_WEIGHT`]) from cosine distance + /// between the task text and the persona text, when an embedder is supplied. + /// `0.0` when no embedder is in use (the default), so it is inert by default. + pub semantic: f32, } /// A scored persona with rationale and confidence information. @@ -65,7 +91,35 @@ pub struct Scored { pub components: ScoreComponents, } -/// Rank all personas in `index` for the given `ctx`. +/// Rank all personas in `index` for the given `ctx` without a semantic embedder. +/// +/// Thin wrapper over [`rank_with_embedder`] with the embedder disabled. This is +/// the default entry point; the semantic channel contributes `0.0`, so results +/// are identical to the scorer before semantic matching was added. +pub fn rank( + ctx: &ContextSignal, + index: &PersonaIndex, + weights: &PolicyWeights, + prefs: &Preferences, +) -> Vec { + rank_with_embedder(ctx, index, weights, prefs, None) +} + +/// Build the text used to embed a persona for semantic matching. +/// +/// Concatenates the persona's optional description with its keyword set so the +/// embedder sees both the prose summary and the discriminating terms. Keyword +/// order is unspecified (it is a set), which suits bag-of-words style embedders. +fn persona_text(profile: &PersonaProfile) -> String { + let mut text = profile.description.clone().unwrap_or_default(); + for kw in &profile.keywords { + text.push(' '); + text.push_str(kw); + } + text +} + +/// Rank all personas in `index` for the given `ctx`, optionally using `embedder`. /// /// Scoring: /// - Language score: sum of `ctx.languages` weights for languages present in @@ -74,16 +128,21 @@ pub struct Scored { /// keywords; 0.0 if there are no task tokens. /// - Capability score: small bonus if the persona has no required tools and /// no network egress (i.e. "safe" / simple persona). +/// - Semantic score: when `embedder` is `Some` and the task has tokens, the +/// cosine similarity between the task text and each persona's text, scaled by +/// [`SEMANTIC_WEIGHT`] and added to the blend. `None` contributes `0.0`, so +/// the default path has no semantic component and no behavior change. /// - Per-persona preference bias from `prefs` is added after blending and /// clamped to [0.0, 1.0]. /// /// Returns results sorted descending by blended score. Confidence is computed /// after sorting based on the top-vs-runner-up gap and absolute score. -pub fn rank( +pub fn rank_with_embedder( ctx: &ContextSignal, index: &PersonaIndex, weights: &PolicyWeights, prefs: &Preferences, + embedder: Option<&dyn Embedder>, ) -> Vec { // Precompute IDF for each task token: tokens that appear in fewer persona // keyword sets are weighted higher (rare tokens are more discriminating). @@ -176,7 +235,12 @@ pub fn rank( } else { ctx.task_tokens .iter() - .filter(|t| profile.anti_keywords.contains(t)) + .filter(|t| { + profile + .anti_keywords + .iter() + .any(|ak| ak.eq_ignore_ascii_case(t.as_str())) + }) .count() }; let lex_score = if anti_hit_count > 0 && !ctx.task_tokens.is_empty() { @@ -195,11 +259,44 @@ pub fn rank( 0.0 }; - // Blended score. + // Context-signal bonus: a small, capped reward for personas whose + // keywords match a project signal -- either a declared dependency + // (`context_tokens`) or a detected build framework (`frameworks`, + // e.g. cargo/npm/go/python). Kept separate from the lexical IDF + // channel so that signals matching no persona cannot dilute + // task-token scoring. + let context_hits = ctx + .context_tokens + .iter() + .chain(ctx.frameworks.iter()) + .filter(|t| profile.keywords.contains(*t)) + .count(); + let context_score = (context_hits as f32 * CONTEXT_TOKEN_HIT).min(CONTEXT_TOKEN_CAP); + + // Semantic-similarity bonus: when an embedder is supplied, reward + // personas whose text is close in MEANING to the task -- catching + // matches the exact-token lexical channel misses. Additive and capped + // by SEMANTIC_WEIGHT, like the context bonus. The task text is the + // normalized task tokens; Phase 2 may embed the raw task hint instead. + // No embedder (the default) yields 0.0, so there is no regression. + let semantic_score = match embedder { + Some(emb) if !ctx.task_tokens.is_empty() => { + let task_text = ctx.task_tokens.join(" "); + let sim = semantic_similarity(emb, &task_text, &persona_text(profile)); + SEMANTIC_WEIGHT * sim + } + _ => 0.0, + }; + + // Blended score. The context and semantic bonuses are additive (not + // weighted): they can only raise a persona that matches the project + // or the task meaning, never lower others or redistribute the weights. let blended = weights.language * lang_score + weights.lexical * lex_score + weights.intent * intent_score - + weights.capability * cap_score; + + weights.capability * cap_score + + context_score + + semantic_score; // Apply preference bias and clamp. // Use intent-aware lookup when the context carries an inferred intent; @@ -245,6 +342,23 @@ pub fn rank( if cap_score > 0.0 { rationale_parts.push(format!("cap_score={:.2}", cap_score)); } + if context_score > 0.0 { + let hit_signals: Vec<&str> = ctx + .context_tokens + .iter() + .chain(ctx.frameworks.iter()) + .filter(|t| profile.keywords.contains(*t)) + .map(|t| t.as_str()) + .collect(); + rationale_parts.push(format!( + "project signals [{}] match: context_score={:.2}", + hit_signals.join(","), + context_score + )); + } + if semantic_score > 0.0 { + rationale_parts.push(format!("semantic_score={:.2}", semantic_score)); + } if bias.abs() > f32::EPSILON { rationale_parts.push(format!("pref_bias={:.3}", bias)); } @@ -264,6 +378,8 @@ pub fn rank( lexical: lex_score, intent: intent_score, capability: cap_score, + context: context_score, + semantic: semantic_score, }; Scored { @@ -332,6 +448,7 @@ mod tests { .collect::>(), frameworks: vec![], task_tokens: task_tokens.iter().map(|t| t.to_string()).collect(), + context_tokens: vec![], inferred_intent: None, } } @@ -434,6 +551,7 @@ mod tests { }, frameworks: vec![], task_tokens: vec!["debugging".to_string(), "error".to_string()], + context_tokens: vec![], inferred_intent: Some(Intent::Debugging), }; @@ -464,4 +582,117 @@ mod tests { "anti-keywords should penalize lexical score" ); } + + /// Anti-keyword matching is case-insensitive: uppercase manifest entries + /// still penalize lowercase task tokens. + #[test] + fn anti_keywords_penalize_case_insensitive() { + let mut persona_a = make_profile("backend", &["rust"], &["rust", "api"]); + persona_a.anti_keywords = vec!["CSS".to_string(), "Frontend".to_string()]; + + let persona_b = make_profile("frontend", &["typescript"], &["react", "css"]); + + let index = PersonaIndex { + profiles: vec![persona_a, persona_b], + }; + let ctx = make_ctx(&[("rust", 1.0)], &["css", "styling", "frontend"]); + + let ranked = rank(&ctx, &index, &PolicyWeights::default(), &Preferences::new()); + let backend_entry = ranked.iter().find(|s| s.persona == "backend").unwrap(); + assert!( + backend_entry.components.lexical < 0.3, + "case-insensitive anti-keywords should penalize lexical score" + ); + } + + /// A dependency token that matches a persona keyword adds a context bonus + /// and breaks a tie against an otherwise-equal persona. + #[test] + fn context_tokens_bonus_rewards_dependency_match() { + let web = make_profile("web-rust", &["rust"], &["rust", "axum"]); + let plain = make_profile("plain-rust", &["rust"], &["rust"]); + let index = PersonaIndex { + profiles: vec![plain, web], + }; + let mut ctx = make_ctx(&[("rust", 1.0)], &[]); + ctx.context_tokens = vec!["axum".to_string(), "tokio".to_string()]; + + let ranked = rank(&ctx, &index, &PolicyWeights::default(), &Preferences::new()); + assert_eq!( + ranked[0].persona, "web-rust", + "the axum dependency should boost the axum-keyworded persona" + ); + let web_entry = ranked.iter().find(|s| s.persona == "web-rust").unwrap(); + assert!(web_entry.components.context > 0.0); + } + + /// An empty context_tokens set contributes no bonus (regression guard). + #[test] + fn empty_context_tokens_add_no_bonus() { + let p = make_profile("rust-expert", &["rust"], &["rust", "axum"]); + let index = PersonaIndex { profiles: vec![p] }; + let ctx = make_ctx(&[("rust", 1.0)], &[]); + let ranked = rank(&ctx, &index, &PolicyWeights::default(), &Preferences::new()); + assert_eq!(ranked[0].components.context, 0.0); + } + + /// A detected build framework (e.g. `cargo`) contributes the same context + /// bonus as a dependency when it matches a persona keyword. + #[test] + fn framework_marker_contributes_context_bonus() { + let rust_dev = make_profile("rust-dev", &["rust"], &["rust", "cargo"]); + let plain = make_profile("plain", &["rust"], &["rust"]); + let index = PersonaIndex { + profiles: vec![plain, rust_dev], + }; + let mut ctx = make_ctx(&[("rust", 1.0)], &[]); + ctx.frameworks = vec!["cargo".to_string()]; + + let ranked = rank(&ctx, &index, &PolicyWeights::default(), &Preferences::new()); + let entry = ranked.iter().find(|s| s.persona == "rust-dev").unwrap(); + assert!( + entry.components.context > 0.0, + "the cargo build framework should give the cargo-keyworded persona a bonus" + ); + assert_eq!(ranked[0].persona, "rust-dev"); + } + + /// Without an embedder the semantic channel is inert: `rank` (and any caller + /// passing `None`) yields a semantic component of exactly 0.0. Regression + /// guard against the semantic bonus leaking into the default scoring path. + #[test] + fn semantic_channel_is_zero_without_embedder() { + let p = make_profile("rust-dev", &["rust"], &["rust", "cargo", "ownership"]); + let index = PersonaIndex { profiles: vec![p] }; + let ctx = make_ctx(&[("rust", 1.0)], &["ownership"]); + let ranked = rank(&ctx, &index, &PolicyWeights::default(), &Preferences::new()); + assert_eq!(ranked[0].components.semantic, 0.0); + } + + /// With a (mock) embedder, a task hint that overlaps a persona's text earns a + /// positive semantic bonus that lifts the blended score above the no-embedder + /// baseline. + #[test] + fn semantic_channel_rewards_related_hint() { + use crate::embed::BagOfWordsEmbedder; + + let p = make_profile("rust-dev", &["rust"], &["rust", "cargo", "ownership"]); + let index = PersonaIndex { profiles: vec![p] }; + let ctx = make_ctx(&[("rust", 1.0)], &["ownership", "cargo"]); + let weights = PolicyWeights::default(); + + let baseline = rank(&ctx, &index, &weights, &Preferences::new()); + + let emb = BagOfWordsEmbedder; + let ranked = rank_with_embedder(&ctx, &index, &weights, &Preferences::new(), Some(&emb)); + + assert!( + ranked[0].components.semantic > 0.0, + "an overlapping hint should produce a semantic bonus" + ); + assert!( + ranked[0].score >= baseline[0].score, + "the semantic bonus must not lower the score" + ); + } } diff --git a/crates/frameshift-orchestrator/src/run.rs b/crates/frameshift-orchestrator/src/run.rs index 7ca3bfd..2ce7c14 100644 --- a/crates/frameshift-orchestrator/src/run.rs +++ b/crates/frameshift-orchestrator/src/run.rs @@ -25,11 +25,12 @@ use std::path::{Path, PathBuf}; use serde::Serialize; use crate::context::sense; +use crate::embed::Embedder; use crate::error::OrchestratorError; use crate::feedback::Preferences; use crate::index::PersonaIndex; use crate::intent::Intent; -use crate::policy::{rank, PolicyWeights, Scored}; +use crate::policy::{rank, rank_with_embedder, PolicyWeights, Scored}; /// All inputs required to run a persona selection pass. pub struct SelectionInputs<'a> { @@ -127,6 +128,18 @@ pub struct ComponentsOutput { /// Returns an empty `Vec` (not an error) when both `catalog_root` is absent /// and `source_dirs` is empty. pub fn select(inputs: &SelectionInputs<'_>) -> Result, OrchestratorError> { + select_with_embedder(inputs, None) +} + +/// Run a persona selection pass using an optional semantic `embedder`. +/// +/// Identical to [`select`] but threads `embedder` into the scorer so the +/// semantic-similarity channel is active when an embedder is supplied. Passing +/// `None` reproduces [`select`] exactly, so existing callers are unaffected. +pub fn select_with_embedder( + inputs: &SelectionInputs<'_>, + embedder: Option<&dyn Embedder>, +) -> Result, OrchestratorError> { let index = if let Some(catalog_root) = &inputs.catalog_root { PersonaIndex::from_catalog(catalog_root)? } else { @@ -141,7 +154,7 @@ pub fn select(inputs: &SelectionInputs<'_>) -> Result, OrchestratorE } let ctx = sense(inputs.project_root, inputs.task_hint); - let ranked = rank(&ctx, &index, &inputs.weights, &inputs.prefs); + let ranked = rank_with_embedder(&ctx, &index, &inputs.weights, &inputs.prefs, embedder); Ok(ranked) } diff --git a/crates/frameshift-server/src/routes/downloads.rs b/crates/frameshift-server/src/routes/downloads.rs index 273ac6e..e6a8d1e 100644 --- a/crates/frameshift-server/src/routes/downloads.rs +++ b/crates/frameshift-server/src/routes/downloads.rs @@ -253,5 +253,12 @@ pub async fn stream_signed_download( // Count successful signed-download responses (alongside direct pack downloads). state.metrics.pack_downloads_total.inc(); + // NOTE: total_downloads is NOT incremented here. This path has only a + // content hash -- pack name and version are not available, so + // increment_download_counter cannot be called. The official client + // (frameshift-client) downloads via `/v1/packs/{name}/versions/{version}/pack`, + // which does increment total_downloads. Callers that want counted downloads + // should use that route. + Ok(response) } diff --git a/crates/frameshift-server/src/routes/packs.rs b/crates/frameshift-server/src/routes/packs.rs index b0d9aa1..0258725 100644 --- a/crates/frameshift-server/src/routes/packs.rs +++ b/crates/frameshift-server/src/routes/packs.rs @@ -911,11 +911,25 @@ pub async fn download_pack_bytes( // Count successful direct-download responses. state.metrics.pack_downloads_total.inc(); - // Record the download for trending ranking. Best-effort: a failure here must - // not fail the download the client already received. + // Record the download event for trending ranking -- feeds the 7-day velocity + // used by SortMode::Trending. Best-effort: a failure here must not fail the + // download the client already received. if let Err(e) = state.catalog.record_download(&name, &version).await { tracing::warn!(pack = %name, version = %version, error = %e, "record_download failed"); } + // Increment the cumulative download counter -- feeds `total_downloads` on + // the pack record shown on the marketplace catalog page. Best-effort: same + // policy as record_download above; warn and continue on failure. NotFound + // is unreachable here (the version record was fetched above), but the + // best-effort pattern handles it safely regardless. + if let Err(e) = state + .catalog + .increment_download_counter(&name, &version) + .await + { + tracing::warn!(pack = %name, version = %version, error = %e, "increment_download_counter failed"); + } + Ok(response) } diff --git a/crates/frameshift-server/tests/integration.rs b/crates/frameshift-server/tests/integration.rs index fdb6a0b..16b0b8a 100644 --- a/crates/frameshift-server/tests/integration.rs +++ b/crates/frameshift-server/tests/integration.rs @@ -308,6 +308,55 @@ async fn download_pack_502_when_blob_missing_from_objects() { assert_eq!(body["error"], "upstream backend mismatch"); } +/// `GET /v1/packs/{name}/versions/{version}/pack` calls `increment_download_counter` +/// on a successful 200 response. +/// +/// Regression test: before the fix, `download_pack_bytes` only called +/// `record_download` (trending) and never `increment_download_counter`, so +/// `total_downloads` on every pack was permanently 0. +#[tokio::test] +async fn download_pack_increments_download_counter() { + let blob = b"counted".to_vec(); + let hash = ObjectHash::of(&blob); + let author_key = Ed25519PublicKey([5u8; 32]); + + let catalog = MockCatalog::new(); + { + let mut state = catalog.state.write().unwrap(); + state.packs.insert( + "counted-pack".to_string(), + make_pack("counted-pack", author_key), + ); + state.versions.insert( + ("counted-pack".to_string(), "1.0.0".to_string()), + make_version("counted-pack", "1.0.0", hash, author_key), + ); + } + // Clone before moving into make_state -- both sides share the same + // Arc>, so writes through AppState are visible here. + let catalog_observer = catalog.clone(); + + let objects = MockPackStore::new(); + objects.insert(hash, blob.clone()); + + let state = make_state(catalog, objects); + let resp = oneshot_get(state, "/v1/packs/counted-pack/versions/1.0.0/pack").await; + assert_eq!(resp.status(), StatusCode::OK, "download should succeed"); + + let increments = catalog_observer + .state + .read() + .unwrap() + .download_counter_increments + .get(&("counted-pack".to_string(), "1.0.0".to_string())) + .copied() + .unwrap_or(0); + assert_eq!( + increments, 1, + "increment_download_counter must be called exactly once per successful download" + ); +} + // --------------------------------------------------------------------------- // /v1/authors // --------------------------------------------------------------------------- diff --git a/crates/frameshift-server/tests/mocks/catalog.rs b/crates/frameshift-server/tests/mocks/catalog.rs index c9f5457..6cad18e 100644 --- a/crates/frameshift-server/tests/mocks/catalog.rs +++ b/crates/frameshift-server/tests/mocks/catalog.rs @@ -48,6 +48,12 @@ pub struct MockState { /// When `true`, the next mutating call returns `CatalogError::Conflict`. pub inject_conflict: bool, + + /// Number of `increment_download_counter` calls per `(pack_name, version)`. + /// + /// Tests read this to assert that the cumulative download counter was + /// incremented after a successful download response. + pub download_counter_increments: HashMap<(String, String), u64>, } /// In-memory [`CatalogBackend`] for integration tests. @@ -259,13 +265,23 @@ impl CatalogBackend for MockCatalog { Ok(results) } - /// Increment download counter (no-op in mock). + /// Increment the download counter for a pack version. + /// + /// Records the call in `state.download_counter_increments` so tests can + /// assert that `download_pack_bytes` actually invoked this method. async fn increment_download_counter( &self, - _name: &str, - _version: &str, + name: &str, + version: &str, ) -> Result { - Ok(0) + let mut state = self + .state + .write() + .map_err(|e| CatalogError::BackendError(e.to_string().into()))?; + let key = (name.to_string(), version.to_string()); + let count = state.download_counter_increments.entry(key).or_insert(0); + *count += 1; + Ok(*count) } /// Tombstone a pack version (no-op in mock).