feat: project-level performance review (loopx Loop Engineering principle 7) - #404
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change adds persistent supervisor ask events and integrates them into ask decisions. It also adds performance review aggregation and a CLI action that reports and stores project-level agent metrics. ChangesAsk Events and Performance Reviews
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Supervisor
participant decide_for_supervisor
participant ask_event
participant SQLite_ask_events
Supervisor->>decide_for_supervisor: evaluate quota decision
decide_for_supervisor->>ask_event: record ask metadata
ask_event->>SQLite_ask_events: insert ask event
SQLite_ask_events-->>ask_event: return event ID
decide_for_supervisor-->>Supervisor: return Ask decision
sequenceDiagram
participant CLI
participant performance_review
participant BackendDatabase
participant MainDatabase
participant Memory
CLI->>BackendDatabase: open review database
CLI->>performance_review: request project performance
performance_review->>BackendDatabase: read completed items and attention requests
performance_review->>MainDatabase: read quality scores
performance_review-->>CLI: return PerformanceReview
CLI->>Memory: save review observation
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
src/review.rs (1)
859-876: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the window bounds and the agent filter.
The test passes
since = 0anduntil = i64::MAX, so no assertion exercises thecompleted_at >= since && completed_at <= untilfilter or the per-agent filter. Those two filters carry the whole correctness of the projection. Add a second case with an item completed outside the window, an ask event recorded beforesince, and an item assigned to another agent. Assert all three are excluded.🤖 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/review.rs` around lines 859 - 876, Extend the performance_review test around the existing review assertion to add records completed outside the requested window, an ask event before since, and an item assigned to a different agent. Invoke performance_review with bounded since/until values and assert those three records are excluded while the in-window, matching-agent results remain included.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@crates/agentflare-backend/src/ask_event.rs`:
- Around line 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.
- Around line 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.
In `@src/cli/review.rs`:
- Around line 265-271: Fix the cost attribution in the review performance flow
around cost_totals and project_cost_usd so --project does not use
AgentflareMcp::resolve_project_name() as its key. Resolve the backend project
record’s canonical cost key for the supplied project id and use it for the
lookup and saved observation; alternatively, explicitly document in the output
and --project help text that costs are always scoped to the current directory.
- Around line 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.
- Around line 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.
In `@src/quota/decide.rs`:
- Around line 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.
In `@src/review.rs`:
- Around line 272-285: Update the review calculation around quantity_completed
and attention_asks to propagate or otherwise surface errors from list_by_project
and count_since instead of converting failures to zero values. Keep the
successful counting behavior unchanged, and ensure review generation cannot
persist or sync misleading zero metrics after a backend read failure.
- Around line 283-285: Update the attention_asks calculation in the review
projection to honor the requested until boundary by using a backend count query
that filters both since and until; preserve the existing zero fallback and u32
conversion, and update the relevant backend symbol such as count_since to accept
and apply the upper bound.
---
Nitpick comments:
In `@src/review.rs`:
- Around line 859-876: Extend the performance_review test around the existing
review assertion to add records completed outside the requested window, an ask
event before since, and an item assigned to a different agent. Invoke
performance_review with bounded since/until values and assert those three
records are excluded while the in-window, matching-agent results remain
included.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d8ce87e5-8742-4a20-ac37-f16737aeedb6
📒 Files selected for processing (8)
crates/agentflare-backend/src/ask_event.rscrates/agentflare-backend/src/db.rscrates/agentflare-backend/src/lib.rscrates/agentflare-backend/src/migrations/0007_ask_events.sqlsrc/cli/review.rssrc/mcp_server.rssrc/quota/decide.rssrc/review.rs
| 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), | ||
| )?) |
There was a problem hiding this comment.
🎯 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); | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 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.
| /// Window size in days ending now, for quantity/attention/cost | ||
| /// (quality is always all-time — see `scores`). | ||
| #[arg(long, default_value = "7")] | ||
| days: i64, |
There was a problem hiding this comment.
🎯 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:
- 1: https://docs.rs/clap/latest/clap/macro.value_parser.html
- 2: https://stackoverflow.com/questions/76230294/how-to-validate-a-cli-argument-in-clap-4-0-9
- 3: https://docs.rs/clap/4.4.6/clap/struct.Arg.html
- 4: https://docs.rs/clap/latest/aarch64-apple-darwin/clap/builder/struct.RangedI64ValueParser.html
- 5: https://github.com/clap-rs/clap/blob/master/clap_builder/src/builder/value_parser.rs
- 6: https://docs.rs/clap/latest/clap/builder/struct.ValueParser.html
- 7: https://shadow.github.io/docs/rust/clap/builder/struct.RangedI64ValueParser.html
- 8: https://rust-lang-nursery.github.io/rust-cookbook/cli/clap-validation.html
🏁 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.rsRepository: 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:
- 1: https://docs.rs/clap/latest/clap/builder/struct.RangedI64ValueParser.html
- 2: https://willcrichton.net/misc/scrape-examples/small-first-example/clap/macro.value_parser.html
- 3: https://github.com/clap-rs/clap/blob/master/clap_builder/src/builder/value_parser.rs
- 4: https://doc.rust-lang.org/stable/nightly-rustc/cargo/util/command_prelude/macro.value_parser.html
- 5: range method not found in
_AnonymousValueParser(usize)clap-rs/clap#4253
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.
| 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); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
The cost figure ignores --project.
project is a backend project id, and the argument doc at lines 86-88 states it is not the repo identifier. resolve_project_name() (src/mcp_server.rs lines 655-667) returns a git-remote basename or the cwd basename. The cost rollup is therefore keyed by the current working directory, not by the --project the user passed. Running agentflare review performance --project <other-project-id> from this repo attributes this repo's spend to that other project, and the saved observation records it under that project id.
Resolve the cost key from the backend project record, or state in the printed output and in the --project help text that cost is always scoped to the current directory.
🤖 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 265 - 271, Fix the cost attribution in the
review performance flow around cost_totals and project_cost_usd so --project
does not use AgentflareMcp::resolve_project_name() as its key. Resolve the
backend project record’s canonical cost key for the supplied project id and use
it for the lookup and saved observation; alternatively, explicitly document in
the output and --project help text that costs are always scoped to the current
directory.
| 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"), | ||
| } |
There was a problem hiding this comment.
🎯 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.
| 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.
| #[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()); | ||
| } |
There was a problem hiding this comment.
📐 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.
| let quantity_completed = agentflare_backend::item::list_by_project(backend_conn, project_id) | ||
| .unwrap_or_default() | ||
| .into_iter() | ||
| .filter(|i| i.assignee_agent.as_deref() == Some(agent)) | ||
| .filter(|i| i.completed_at.is_some_and(|t| t >= since && t <= until)) | ||
| .count() as u32; | ||
|
|
||
| let agent_score = scores(main_conn, repo)? | ||
| .into_iter() | ||
| .find(|s| s.agent == agent); | ||
|
|
||
| let attention_asks = | ||
| agentflare_backend::ask_event::count_since(backend_conn, project_id, Some(agent), since) | ||
| .unwrap_or(0) as u32; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Backend read failures become zeros without any signal.
list_by_project errors collapse to unwrap_or_default() and count_since errors collapse to unwrap_or(0), while scores errors propagate. A locked or corrupt backend DB therefore produces a review that reports quantity_completed: 0 and attention_asks: 0 as if they were measured values. The CLI then persists that review as a performance_review observation and syncs it across workstations, so the wrong zeros outlive the failed run.
Propagate the backend errors, or at least surface them to the caller.
🤖 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/review.rs` around lines 272 - 285, Update the review calculation around
quantity_completed and attention_asks to propagate or otherwise surface errors
from list_by_project and count_since instead of converting failures to zero
values. Keep the successful counting behavior unchanged, and ensure review
generation cannot persist or sync misleading zero metrics after a backend read
failure.
| let attention_asks = | ||
| agentflare_backend::ask_event::count_since(backend_conn, project_id, Some(agent), since) | ||
| .unwrap_or(0) as u32; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
attention_asks ignores until.
count_since filters only on created_at >= since (see crates/agentflare-backend/src/ask_event.rs lines 50-62). quantity_completed respects both since and until, and the returned projection carries an until field. For any call where until is not "now", the attention count covers a wider window than the rest of the review. The CLI always passes now, so this is latent today, but the function is public and takes an arbitrary until.
Either add an upper bound to the backend query, or state the limitation in the doc comment the same way cost and quality limitations are already stated.
📝 Minimal documentation-only option
-/// agent dimension) and quality is all-time (`score_events` isn't date-
-/// filtered) — both are reported as such, not silently narrowed.
+/// agent dimension) and quality is all-time (`score_events` isn't date-
+/// filtered) — both are reported as such, not silently narrowed. Note
+/// `attention_asks` is bounded by `since` only: `ask_event::count_since`
+/// has no upper bound, so an `until` in the past does not clip it.🤖 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/review.rs` around lines 283 - 285, Update the attention_asks calculation
in the review projection to honor the requested until boundary by using a
backend count query that filters both since and until; preserve the existing
zero fallback and u32 conversion, and update the relevant backend symbol such as
count_since to accept and apply the upper bound.
e3e5c13 to
f42b096
Compare
1 participant
Summary
Closes the highest-leverage gap between agentflare and loopx's Loop Engineering principles — principle 7, "Loop Agents need performance review": join agentflare's existing, separately-tracked quantity/quality/cost/attention signals into one per-agent, per-project record, synced cross-workstation via the existing memory-sync mechanism (#400).
ask_eventstable +agentflare_backend::ask_eventmodule — makes attention cost (times a human was asked) queryable instead of an ephemeral side effect of a lifecycle flipquota::decide_for_supervisornow records an ask event whenever it gates a goalreview::performance_review()— joins quantity (completed items), quality (review scores, all-time), cost (Claude Code session rollup, whole-project), and attention (ask events) for one agentagentflare review performanceCLI command — computes the join and saves it to the memory store as aperformance_reviewobservation;agentflare memory syncpropagates it to other workstations unchanged, no new sync channelDeliberately scoped to principle 7 only — principle 3 (blocked-route visibility across workstations) is a real next step but depends on a prerequisite that doesn't exist yet:
GoalLifecycle::Clear(the transition that un-gates a goal) has no caller anywhere in the codebase. See the plan doc (local, not committed —docs/is gitignored) for the full gap analysis and out-of-scope notes.Test plan
ask_event::unit tests (record/count_since scoping)quota::tests (25, including newgated_lifecycle_records_an_ask_event)performance_reviewjoin test (quantity/quality/cost/attention all asserted)agentflare review performanceagainst this repo's own agentflare project, confirmed the observation landed viaagentflare memory observationsagentflare+agentflare-backendsuite passesSummary by CodeRabbit
New Features
review performancereports covering completed work, review quality, attention requests, and project costs over a configurable time period.Bug Fixes