Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
124 changes: 124 additions & 0 deletions crates/agentflare-backend/src/ask_event.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
use crate::error::Result;
use rusqlite::{Connection, params};

/// One "the supervisor asked a human" occurrence — the persisted counterpart
/// of `quota::decide::Decision::ask()`, which itself never writes anything
/// (it's pure). Recorded so attention cost stops being an ephemeral side
/// effect of a lifecycle flip and becomes queryable for performance review.
#[derive(Debug, Clone, serde::Serialize)]
pub struct AskEvent {
pub id: String,
pub project_id: String,
pub goal_item_id: Option<String>,
pub item_id: String,
pub agent: Option<String>,
pub reason: String,
pub gate_question: Option<String>,
pub created_at: i64,
}

#[allow(clippy::too_many_arguments)]
pub fn record(
conn: &Connection,
project_id: &str,
goal_item_id: Option<&str>,
item_id: &str,
agent: Option<&str>,
reason: &str,
gate_question: Option<&str>,
now: i64,
) -> Result<String> {
let id = db_kit::ids::new_id();
conn.execute(
"INSERT INTO ask_events
(id, project_id, goal_item_id, item_id, agent, reason, gate_question, created_at)
VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8)",
params![
id,
project_id,
goal_item_id,
item_id,
agent,
reason,
gate_question,
now
],
)?;
Ok(id)
}

pub fn count_since(
conn: &Connection,
project_id: &str,
agent: Option<&str>,
since: i64,
) -> Result<i64> {
Ok(conn.query_row(
"SELECT COUNT(*) FROM ask_events
WHERE project_id = ?1 AND created_at >= ?2 AND (?3 IS NULL OR agent = ?3)",
params![project_id, since, agent],
|r| r.get(0),
)?)
Comment on lines +50 to +61

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Bound ask-event counts by the review end time.

count_since only filters created_at >= since. src/review.rs, Lines 259-298 accepts until but cannot pass it to this API. Events after until inflate attention_asks in historical performance reviews.

Add an upper timestamp bound to this query. Pass until from performance_review. Add an exclusive-after-until test.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/agentflare-backend/src/ask_event.rs` around lines 50 - 61, Update
count_since to accept an until timestamp and constrain ask_events.created_at to
be before it, then pass the review’s until value from performance_review. Add a
test covering an event after until and verify it is excluded from the count.

}

#[cfg(test)]
mod tests {
use super::*;

fn seed_project(conn: &Connection) -> String {
let workspace = crate::workspace::create(
conn,
crate::workspace::CreateWorkspace {
name: "ws".into(),
slug: "ws".into(),
owner_agent: None,
item_label: None,
},
)
.unwrap();
let project = crate::project::create(
conn,
crate::project::CreateProject {
workspace_id: workspace.id,
name: "proj".into(),
identifier: "proj".into(),
external_source: None,
external_id: None,
},
)
.unwrap();
project.id
}

#[test]
fn record_then_count_since_scopes_by_project_and_agent() {
let conn = crate::db::open_in_memory().unwrap();
let pid = seed_project(&conn);
record(
&conn,
&pid,
None,
"item-1",
Some("claude-code"),
"gated",
Some("q?"),
100,
)
.unwrap();
record(
&conn,
&pid,
None,
"item-2",
Some("opencode"),
"gated",
None,
200,
)
.unwrap();

assert_eq!(count_since(&conn, &pid, None, 0).unwrap(), 2);
assert_eq!(count_since(&conn, &pid, Some("claude-code"), 0).unwrap(), 1);
assert_eq!(count_since(&conn, &pid, None, 150).unwrap(), 1);
}
}
Comment on lines +64 to +124

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Test the supervisor persistence path.

record_then_count_since_scopes_by_project_and_agent tests direct database writes. It does not verify that a supervisor Ask decision creates an event. Add an integration test that calls decide_for_supervisor and then reads the matching ask_events row.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/agentflare-backend/src/ask_event.rs` around lines 64 - 127, The
existing test only exercises record and count_since directly; add an integration
test in the tests module that invokes decide_for_supervisor with a supervisor
Ask decision, then queries ask_events and asserts the matching persisted event
fields. Reuse seed_project and the established in-memory connection setup, and
verify the event is associated with the expected project, item, agent, decision,
question, and timestamp as applicable.

1 change: 1 addition & 0 deletions crates/agentflare-backend/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const MIGRATION_LIST: &[M<'static>] = &[
M::up(include_str!("migrations/0004_item_comments.sql")),
M::up(include_str!("migrations/0005_items_fts.sql")),
M::up(include_str!("migrations/0006_vents.sql")),
M::up(include_str!("migrations/0007_ask_events.sql")),
];
const MIGRATIONS: Migrations = Migrations::from_slice(MIGRATION_LIST);

Expand Down
1 change: 1 addition & 0 deletions crates/agentflare-backend/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
pub mod ask_event;
pub mod asset;
pub mod claim;
pub mod comment;
Expand Down
11 changes: 11 additions & 0 deletions crates/agentflare-backend/src/migrations/0007_ask_events.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
CREATE TABLE IF NOT EXISTS ask_events (
id TEXT PRIMARY KEY,
project_id TEXT NOT NULL REFERENCES projects(id) ON DELETE CASCADE,
goal_item_id TEXT REFERENCES items(id) ON DELETE SET NULL,
item_id TEXT NOT NULL,
agent TEXT,
reason TEXT NOT NULL,
gate_question TEXT,
created_at INTEGER NOT NULL
);
CREATE INDEX IF NOT EXISTS idx_ask_events_project_created ON ask_events(project_id, created_at);
115 changes: 115 additions & 0 deletions src/cli/review.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,29 @@ pub enum ReviewAction {
#[arg(long)]
json: bool,
},
/// Compute one agent's project-level performance review (quantity,
/// quality, cost, attention) and save it to the memory store as a
/// `performance_review` observation. Run `agentflare memory sync`
/// afterward to share it with other workstations.
Performance {
/// Backend project id (agentflare item-tracker project — run
/// `agentflare memory observations` or check the dashboard to find
/// it; this is NOT the same identifier as --repo).
#[arg(long)]
project: String,
/// Agent name (default: detected, same convention as `submit`).
#[arg(long)]
agent: Option<String>,
/// Scope quality scoring to one repo (default: current repo).
#[arg(long)]
repo: Option<String>,
/// Window size in days ending now, for quantity/attention/cost
/// (quality is always all-time — see `scores`).
#[arg(long, default_value = "7")]
days: i64,
Comment on lines +97 to +100

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

clap 4 value_parser range i64 argument validation

💡 Result:

In clap 4, you can validate that an i64 (or other numeric) argument falls within a specific range by using the value_parser! macro combined with the.range method [1][2][3]. For basic numeric validation, clap provides RangedI64ValueParser (and similar variants for other types), which can be initialized via the value_parser! macro or by passing a range expression directly to value_parser [4][5][6][3]. Examples of how to apply this: Using the derive API: #[arg(long, value_parser = clap::value_parser!(i64).range(1..100))] pub value: i64, Using the builder API: let mut cmd = clap::Command::new("example").arg( clap::Arg::new("count").long("count").value_parser(clap::value_parser!(i64).range(1..100))); Key points to remember: 1. Range Expressions: You can use standard Rust range expressions like 1..100 (exclusive upper bound) or 1..=100 (inclusive upper bound) [5][6][3]. 2. Negative Numbers: If you need to accept negative numbers, you must additionally configure the argument using.allow_negative_numbers(true) or.allow_hyphen_values(true) on the Arg, otherwise clap may misinterpret a negative sign as a command-line flag [4][7]. 3. Under the hood: clap uses RangedI64ValueParser to handle these constraints, which ensures the input is parsed into the requested type and checked against the bounds before your application logic receives the value [4][5][3]. If you have validation requirements that go beyond simple numeric ranges (e.g., custom string formats or file checks), you can pass a custom function to value_parser that returns a Result<T, String> [8].

Citations:


🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- clap declarations ---'
rg -n -C 3 '(^|\s)clap(\s*=|[[:space:]]*\{)|clap::|days:' --glob 'Cargo.toml' --glob 'Cargo.lock' --glob '*.rs' .
printf '%s\n' '--- review implementation ---'
sed -n '80,115p;270,305p' src/cli/review.rs

Repository: getappz/agentflare

Length of output: 19504


🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- lockfiles and clap resolution ---'
find . -name Cargo.lock -print
if [ -f Cargo.lock ]; then
  rg -n -A8 '^name = "clap"$|^name = "clap_builder"$|^name = "clap_derive"$' Cargo.lock
fi
printf '%s\n' '--- performance execution path ---'
sed -n '115,285p' src/cli/review.rs
printf '%s\n' '--- clap parser usage patterns ---'
rg -n -C 2 'value_parser!|value_parser\s*=|\.range\(' --glob '*.rs' --glob 'Cargo.toml' .

Repository: getappz/agentflare

Length of output: 8224


🌐 Web query:

clap 4.6.1 value_parser!(i64).range(1..) RangedI64ValueParser docs

💡 Result:

In clap 4.6.1, clap::value_parser!(i64).range(1..) is a valid way to create a value parser that restricts input to a specific numeric range [1][2]. The clap::value_parser!(T) macro selects an appropriate ValueParser implementation based on the provided type T [2]. When i64 (or other supported numeric types like u8, u16, i32, u64) is used, the macro returns a RangedI64ValueParser (or RangedU64ValueParser for unsigned types), which provides a .range(range) method to narrow the allowed values [1][3][2]. Key points regarding this usage: - Range Syntax: The .range() method accepts any type that implements RangeBounds<i64> (e.g., 1.., 1..=10, ..100), allowing for flexible boundary definitions [1][3]. - Supported Types: This pattern works for built-in numeric types that map to RangedI64ValueParser or RangedU64ValueParser [4][2]. If you use a type that does not support the .range() method (such as usize in some older contexts or custom types), you may encounter a "method not found" error because those types might return an _AnonymousValueParser instead [5]. - Negative Values: If you intend to use ranges that include negative numbers, ensure the argument is configured to allow them (e.g., by setting .allow_negative_numbers(true) on the Arg), otherwise the parser may reject inputs starting with a hyphen [1]. Example usage:.value_parser(clap::value_parser!(i64).range(1..)) [1][2]

Citations:


Reject non-positive --days values.

days.max(1) queries one day, but the text renderer prints the raw value. Add value_parser = clap::value_parser!(i64).range(1..) to reject 0 and negative values. This syntax is supported by clap 4.6.1.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli/review.rs` around lines 97 - 100, Update the days argument definition
to use Clap’s ranged i64 value parser with a minimum of 1, so --days rejects
zero and negative values before processing. Preserve the existing default value
and field type.

#[arg(long)]
json: bool,
},
}

impl ReviewArgs {
Expand Down Expand Up @@ -220,6 +243,98 @@ impl ReviewArgs {
}
}
}
ReviewAction::Performance {
project,
agent,
repo,
days,
json,
} => {
let agent = agent.unwrap_or_else(crate::review::submitter_name);
let repo = repo.or_else(|| crate::claims::resolve_repo(None));
let now = crate::claims::now();
let since = now - days.max(1) * 86_400;

let backend_conn =
match agentflare_backend::db::open_db(&crate::vent::paths::backend_db_path()) {
Ok(c) => c,
Err(e) => fail(format!("cannot open backend db: {e}")),
};

let today = chrono::Local::now().date_naive();
let cost_start = today - chrono::Duration::days(days.max(1) - 1);
let cost_totals =
crate::cost::summarize((cost_start, today), crate::cost::GroupBy::Project);
let cost_key = crate::mcp_server::AgentflareMcp::resolve_project_name();
let project_cost_usd = cost_totals
.get(&cost_key)
.map(|t| t.cost_usd)
.unwrap_or(0.0);

let review = match crate::review::performance_review(
&backend_conn,
&conn,
&project,
repo.as_deref(),
&agent,
since,
now,
project_cost_usd,
) {
Ok(r) => r,
Err(e) => fail(format!("performance_review failed: {e}")),
};

if json {
println!(
"{}",
serde_json::to_string_pretty(&review).unwrap_or_default()
);
} else {
println!("{agent} — project {project} — last {days}d");
println!(" completed: {}", review.quantity_completed);
match review.quality_accuracy {
Some(acc) => println!(
" quality: {:.0}% ({}/{} verified, {} round(s), all-time)",
acc * 100.0,
review.quality_findings,
review.quality_findings,
review.quality_rounds
),
None => println!(" quality: no recorded review rounds"),
}
Comment on lines +296 to +305

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

The "verified" count prints the findings total.

review.quality_findings fills both the numerator and the denominator, so the line always reads "N/N verified". That contradicts the accuracy percentage printed on the same line: an agent at 50% accuracy renders as "50% (2/2 verified)". The Scores arm at lines 236-241 prints s.verified and s.findings correctly.

The root cause is in src/review.rs: PerformanceReview copies accuracy, findings, and rounds from AgentScore but drops verified. The JSON output and the persisted memory observation are missing it too. Add quality_verified: u32 to the struct, populate it from agent_score, and print it here.

🐛 Proposed fix

In src/review.rs:

     pub quality_accuracy: Option<f64>,
     pub quality_findings: u32,
+    pub quality_verified: u32,
     pub quality_rounds: u32,
         quality_findings: agent_score.as_ref().map(|s| s.findings).unwrap_or(0),
+        quality_verified: agent_score.as_ref().map(|s| s.verified).unwrap_or(0),
         quality_rounds: agent_score.as_ref().map(|s| s.rounds).unwrap_or(0),

Then here:

                             acc * 100.0,
-                            review.quality_findings,
+                            review.quality_verified,
                             review.quality_findings,
                             review.quality_rounds
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
match review.quality_accuracy {
Some(acc) => println!(
" quality: {:.0}% ({}/{} verified, {} round(s), all-time)",
acc * 100.0,
review.quality_findings,
review.quality_findings,
review.quality_rounds
),
None => println!(" quality: no recorded review rounds"),
}
match review.quality_accuracy {
Some(acc) => println!(
" quality: {:.0}% ({}/{} verified, {} round(s), all-time)",
acc * 100.0,
review.quality_verified,
review.quality_findings,
review.quality_rounds
),
None => println!(" quality: no recorded review rounds"),
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli/review.rs` around lines 295 - 304, Add a quality_verified field to
PerformanceReview, populate it from AgentScore when constructing the review, and
include it in JSON and persisted memory output. Update the quality_accuracy
formatting in the review CLI to print quality_verified as the verified numerator
while retaining quality_findings as the denominator.

println!(" attention: {} ask(s)", review.attention_asks);
println!(
" cost: ${:.4} (whole project, all agents, {days}d window)",
review.project_cost_usd
);
}

let mem_conn = match crate::memory::store::open() {
Ok(c) => c,
Err(e) => fail(format!("cannot open memory store: {e}")),
};
let content = serde_json::to_string(&review).unwrap_or_default();
let topic_key = format!("perf_review:{project}:{agent}");
match crate::memory::observations::save(
&mem_conn,
None,
"performance_review",
&format!("{agent} performance — {project}"),
&content,
None,
Some(&project),
Some("workstation"),
Some(&topic_key),
) {
Ok(_) => println!(
"\nsaved — run `agentflare memory sync` to share across workstations"
),
Err(e) => crate::ui::error(&format!(
"warning: review computed but not saved to memory: {e}"
)),
}
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/mcp_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,7 @@ impl AgentflareMcp {

/// Derives a project name from the git remote (`getappz/agentflare` →
/// `agentflare`) or, outside a repo, the directory basename.
fn resolve_project_name() -> String {
pub(crate) fn resolve_project_name() -> String {
if let Some(repo) = Self::run_git(&["remote", "get-url", "origin"]) {
let normalized = crate::claims::normalize_repo(&repo);
if let Some(name) = normalized.rsplit('/').next().filter(|s| !s.is_empty()) {
Expand Down
33 changes: 33 additions & 0 deletions src/quota/decide.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ pub fn decide_for_supervisor(
mcp: &crate::mcp_server::AgentflareMcp,
item: &agentflare_backend::item::Item,
) -> EffectiveAction {
let now = crate::claims::now();
let decision = mcp
.with_backend_db(|conn| decide(conn, item))
.unwrap_or_else(|_| Decision::fail_closed("could not open backend db"));
Expand All @@ -240,6 +241,7 @@ pub fn decide_for_supervisor(
}
EffectiveActionInternal::Wait => EffectiveAction::Wait,
EffectiveActionInternal::Ask => {
let goal_item_id = goal.as_ref().map(|(gi, _)| gi.id.clone());
if let Some((goal_item, mut meta)) = goal {
meta.consecutive_self_repairs = 0;
if let Ok(next) = meta.lifecycle.apply(super::lifecycle::LifecycleEvent::Gate) {
Expand All @@ -249,6 +251,18 @@ pub fn decide_for_supervisor(
super::goal::save_goal_metadata(conn, &goal_item.id, &meta)
});
}
let _ = mcp.with_backend_db(|conn| {
agentflare_backend::ask_event::record(
conn,
&item.project_id,
goal_item_id.as_deref(),
&item.id,
item.assignee_agent.as_deref(),
&decision.reason,
decision.gate_question.as_deref(),
now,
)
});
EffectiveAction::Ask(
decision
.gate_question
Expand Down Expand Up @@ -455,6 +469,25 @@ mod tests {
assert_eq!(decision.effective_action, EffectiveActionInternal::Ask);
}

#[test]
fn gated_lifecycle_records_an_ask_event() {
let conn = test_conn();
let (pid, sid) = seed_project(&conn);
let goal = make_goal_item(&conn, &pid, &sid, GoalLifecycle::Gated, 0);
let todo = make_todo(&conn, &pid, &sid, &goal.id);

let decision = decide(&conn, &todo);
assert_eq!(decision.effective_action, EffectiveActionInternal::Ask);

// decide() itself is pure and does not write; recording happens in
// decide_for_supervisor, which needs an AgentflareMcp and is covered
// by ask_event's own unit test for the write path. This pins that
// decide() still reaches Ask for a gated goal, which
// decide_for_supervisor's Ask arm relies on to call
// ask_event::record.
assert!(decision.gate_question.is_some());
}
Comment on lines +472 to +489

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Rename or replace this test.

This test calls decide, which is pure. It cannot record an ask event. The test only verifies that a gated lifecycle returns Ask with a gate question.

Rename the test to describe that behavior. Add the persistence assertion through decide_for_supervisor in a separate integration test.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/quota/decide.rs` around lines 472 - 489, Rename
gated_lifecycle_records_an_ask_event to reflect that decide returns Ask with a
gate question without persistence. Add a separate integration test using
decide_for_supervisor and AgentflareMcp that verifies the ask event is recorded,
reusing the existing ask_event persistence coverage patterns.


#[test]
fn active_lifecycle_with_no_vent_does_not_ask() {
let conn = test_conn();
Expand Down
Loading
Loading