Apps/ImageApi
1
0
Fork 0
Code Issues Pull Requests 1 Releases Wiki Activity

Reels pre-gen: fix runtime breakers from review (1-5)

Browse Source 
1. Drop the unregistered prefs_dao/reel_dao web::Data extractors from
   create_reel_handler / precomputed_reel_handler and read the DAOs off
   AppState instead (consistent with the scheduler). Missing app_data
   would have 500'd every POST /reels and /reels/precomputed at runtime.
2. Restore the dropped 'return' in the cache-hit branch — without it a
   cache hit fell through, overwrote the Done job with Queued, and
   re-ran the whole TTS+render pipeline on every request.
3. Make secs_until_next_run_hour minute/second-accurate so a batch that
   finishes inside the run hour sleeps ~24h instead of busy-looping
   (wake, re-run, sleep 0) for the rest of the hour. Tests updated.
4. Prune photo/user-bound tools (get_file_tags, get_faces_in_photo,
   recall_facts_for_photo, recall_facts_for_entity) from the agentic
   reel scripter's allow-list — they no-op/error with the empty
   file/user context and only burn iterations.
5. Align AGENTIC_SYSTEM_PROMPT's advertised tool list with the actual
   (pruned) allow-list.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Cameron Cordes
2026-06-13 15:14:36 -04:00
parent 5c9ee56527
commit e4d8d374fb
2 changed files with 52 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/reels/mod.rs
+40 -32
View File
@@ -18,7 +18,7 @@ pub mod selector;
use std::collections::HashMap;
use std::path::{Path, PathBuf};
use std::sync::{Arc, LazyLock, Mutex, Mutex as StdMutex};
use std::sync::{LazyLock, Mutex, Mutex as StdMutex};
use std::time::{Duration, Instant};
use actix_files::NamedFile;
@@ -30,7 +30,7 @@ use serde_json::json;
use uuid::Uuid;
use crate::data::Claims;
use crate::database::{ExifDao, InsightDao, PrecomputedReelDao, UserAiPrefsDao};
use crate::database::{ExifDao, InsightDao};
use crate::libraries::{Library, resolve_library_param};
use crate::memories::MemoriesSpan;
use crate::otel::extract_context_from_request;
@@ -61,7 +61,6 @@ pub fn normalize_library_key(libs: &[Library], param: Option<&str>) -> String {
/// caller regardless of DB errors.
fn capture_prefs(
app_state: &AppState,
prefs_dao: &web::Data<Arc<Mutex<Box<dyn UserAiPrefsDao>>>>,
req: &web::Json<CreateReelRequest>,
library_param: Option<&str>,
) -> Result<(), anyhow::Error> {
@@ -77,7 +76,7 @@ fn capture_prefs(
}
_ => "all".to_string(),
};
let mut dao = prefs_dao.lock().expect("lock");
let mut dao = app_state.user_ai_prefs_dao.lock().expect("lock");
let ctx = opentelemetry::Context::new();
dao.upsert_prefs(
&ctx,
@@ -385,7 +384,6 @@ pub async fn create_reel_handler(
app_state: web::Data<AppState>,
exif_dao: web::Data<Mutex<Box<dyn ExifDao>>>,
insight_dao: web::Data<Mutex<Box<dyn InsightDao>>>,
prefs_dao: web::Data<Arc<Mutex<Box<dyn UserAiPrefsDao>>>>,
) -> impl Responder {
let span_context = extract_context_from_request(&http_request);
@@ -455,8 +453,8 @@ pub async fn create_reel_handler(
},
);
// Capture params for passive prefs mirror (best-effort, never fails).
let _ = capture_prefs(&app_state, &prefs_dao, &req, req.library.as_deref());
HttpResponse::Accepted().json(ReelJobCreatedResponse {
let _ = capture_prefs(&app_state, &req, req.library.as_deref());
return HttpResponse::Accepted().json(ReelJobCreatedResponse {
job_id: job_id.to_string(),
status: ReelJobStatus::Done,
});
@@ -515,7 +513,7 @@ pub async fn create_reel_handler(
with_job(job_id, |job| job.abort = Some(handle.abort_handle()));
// Capture params for passive prefs mirror (best-effort, never fails).
let _ = capture_prefs(&app_state, &prefs_dao, &req, req.library.as_deref());
let _ = capture_prefs(&app_state, &req, req.library.as_deref());
HttpResponse::Accepted().json(ReelJobCreatedResponse {
job_id: job_id.to_string(),
@@ -584,7 +582,6 @@ pub async fn precomputed_reel_handler(
_claims: Claims,
query: web::Query<HashMap<String, String>>,
app_state: web::Data<AppState>,
reel_dao: web::Data<Mutex<Box<dyn PrecomputedReelDao>>>,
) -> impl Responder {
let span = query.get("span").map(|s| s.as_str()).unwrap_or("day");
let library_key = normalize_library_key(
@@ -605,7 +602,10 @@ pub async fn precomputed_reel_handler(
let min_generated_at = now - (max_age_hours * 3600);
let ctx = opentelemetry::Context::new();
let mut dao = reel_dao.lock().expect("Unable to lock PrecomputedReelDao");
let mut dao = app_state
.precomputed_reel_dao
.lock()
.expect("Unable to lock PrecomputedReelDao");
// Fast existence gate: is there a fresh row at all?
if !dao
@@ -923,19 +923,23 @@ fn pregen_week_dow() -> u32 {
.unwrap_or(1)
}
/// Pure: seconds until the next run of `run_hour` given the current local time.
/// Handles same-day vs wrap-around. Recomputed each loop iteration to absorb
/// DST shifts.
/// Pure: seconds until the next `run_hour:00:00` strictly after `now`.
///
/// Minute/second-accurate (not just hour-granular): when `now` is already at or
/// past the target this wraps to the same hour tomorrow, so a batch that
/// finishes inside the run hour sleeps ~24h rather than busy-looping (waking,
/// re-running, and re-sleeping 0s) for the rest of that hour. The tradeoff is
/// that booting at or after `run_hour` waits until the next day. Recomputed each
/// loop iteration from `Local::now()` so DST shifts are absorbed.
pub(crate) fn secs_until_next_run_hour(now: chrono::DateTime<chrono::Local>, run_hour: u32) -> u64 {
let now_hour = now.hour();
let diff = if now_hour > run_hour {
24 - now_hour + run_hour
} else if now_hour == run_hour {
0
let now_secs = now.hour() * 3600 + now.minute() * 60 + now.second();
let target_secs = run_hour * 3600;
let diff = if target_secs > now_secs {
target_secs - now_secs
} else {
run_hour - now_hour
86_400 - now_secs + target_secs
};
(diff * 3600) as u64
diff as u64
}
/// Load pre-gen parameters: tries the user_ai_prefs DB row first, falls back
@@ -1367,21 +1371,25 @@ mod tests {
}
#[test]
fn secs_until_next_run_hour_same_hour_returns_zero() {
fn secs_until_next_run_hour_within_run_hour_wraps_to_tomorrow() {
// 03:30, run 3 → already past today's 03:00, so wait until tomorrow
// 03:00 (23h30m). Crucially NOT 0 — that would busy-loop the scheduler
// for the rest of the hour.
let dt = chrono::Local
.with_ymd_and_hms(2026, 6, 13, 3, 30, 0)
.single()
.expect("valid datetime");
assert_eq!(secs_until_next_run_hour(dt, 3), 0);
assert_eq!(secs_until_next_run_hour(dt, 3), 23 * 3600 + 30 * 60);
}
#[test]
fn secs_until_next_run_hour_future_today_returns_remaining() {
fn secs_until_next_run_hour_future_today_counts_minutes() {
// 10:15 → 14:00 is 3h45m, not a whole-hour 4h (minutes count).
let dt = chrono::Local
.with_ymd_and_hms(2026, 6, 13, 10, 0, 0)
.with_ymd_and_hms(2026, 6, 13, 10, 15, 0)
.single()
.expect("valid datetime");
assert_eq!(secs_until_next_run_hour(dt, 14), 4 * 3600);
assert_eq!(secs_until_next_run_hour(dt, 14), 3 * 3600 + 45 * 60);
}
#[test]
@@ -1401,19 +1409,19 @@ mod tests {
.expect("valid datetime");
// 0:00, run at 3 → 3 hours
assert_eq!(secs_until_next_run_hour(dt, 3), 3 * 3600);
// 0:00, run at 0 → 0 (immediate)
assert_eq!(secs_until_next_run_hour(dt, 0), 0);
// 0:00 exactly, run at 0 → wraps to next midnight (not 0, so no busy loop)
assert_eq!(secs_until_next_run_hour(dt, 0), 86_400);
}
#[test]
fn secs_until_next_run_hour_last_hour() {
fn secs_until_next_run_hour_just_before_target() {
// 23:30, run 0 → 30 minutes to midnight (minute-accurate, not 1h).
let dt = chrono::Local
.with_ymd_and_hms(2026, 6, 13, 23, 30, 0)
.single()
.expect("valid datetime");
// 23:30, run at 23 → 0 (still in hour 23)
assert_eq!(secs_until_next_run_hour(dt, 23), 0);
// 23:30, run at 0 → 1 hour
assert_eq!(secs_until_next_run_hour(dt, 0), 3600);
assert_eq!(secs_until_next_run_hour(dt, 0), 30 * 60);
// 23:30, run 23 → already past today's 23:00, wait until tomorrow.
assert_eq!(secs_until_next_run_hour(dt, 23), 86_400 - 30 * 60);
}
}
src/reels/script.rs
+12 -8
View File
@@ -53,10 +53,12 @@ Be concrete and grounded in the details given; never invent names, places, or \
events that aren't supported. Keep each line to one or two short sentences that \
can be read aloud in a few seconds. Avoid generic filler like \"what a \
wonderful day\" — if you have little to go on, simply describe the moment \
plainly.\n\nYou may call read-only tools (search_messages, get_file_tags, \
reverse_geocode, get_current_datetime, recall_entities, recall_facts_for_photo, \
recall_facts_for_entity) to ground each line in real context. Never invent \
details. Return ONLY the JSON object, no prose or code fences.";
plainly.\n\nYou may call read-only tools (search_rag, search_messages, \
get_sms_messages, get_calendar_events, get_location_history, reverse_geocode, \
get_personal_place_at, recall_entities, get_current_datetime) to ground each \
line in real context — e.g. reverse_geocode a moment's GPS to name the place, \
or check the calendar/messages around its date. Never invent details. Return \
ONLY the JSON object, no prose or code fences.";
/// Maximum agentic tool iterations for pre-generation. Tunable via
/// `REEL_PREGEN_MAX_TOOL_ITERS` (default 8).
@@ -317,19 +319,21 @@ pub async fn generate_script_agentic(
// then filter out write tools.
let gate = generator.current_gate_opts_for_persona(false, None);
let all_tools = InsightGenerator::build_tool_definitions(gate);
// Whole-reel calls have no single photo and no authenticated user, so the
// loop runs execute_tool with empty file/image context and user_id=0. Only
// tools that work without that context are useful here — photo/user-bound
// tools (get_file_tags, get_faces_in_photo, recall_facts_for_photo,
// recall_facts_for_entity) would just no-op or error, burning iterations,
// so they're excluded.
let read_only_names: std::collections::HashSet<&str> = [
"search_rag",
"search_messages",
"get_sms_messages",
"get_calendar_events",
"get_location_history",
"get_file_tags",
"get_faces_in_photo",
"reverse_geocode",
"get_personal_place_at",
"recall_entities",
"recall_facts_for_photo",
"recall_facts_for_entity",
"get_current_datetime",
]
.into_iter()