feat: Handle analyze retry with failure marker
When a previous analysis attempt fails and leaves a failure marker, the retry mechanism now correctly identifies this state. It removes the stale input file, allowing the retry to proceed without a conflict error. This ensures users can recover from analysis failures more gracefully.
This commit is contained in:
@@ -119,6 +119,11 @@ impl LlmBackend {
|
|||||||
self.format_instruction = Some(instruction.into());
|
self.format_instruction = Some(instruction.into());
|
||||||
self
|
self
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn with_max_completion_tokens(mut self, tokens: u32) -> Self {
|
||||||
|
self.max_completion_tokens = tokens;
|
||||||
|
self
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Ionos provider: shared endpoint, `IONOS_API_KEY` env var as bearer token.
|
/// Ionos provider: shared endpoint, `IONOS_API_KEY` env var as bearer token.
|
||||||
@@ -149,15 +154,17 @@ pub fn backends() -> &'static [LlmBackend] {
|
|||||||
static B: OnceLock<Vec<LlmBackend>> = OnceLock::new();
|
static B: OnceLock<Vec<LlmBackend>> = OnceLock::new();
|
||||||
B.get_or_init(|| {
|
B.get_or_init(|| {
|
||||||
vec![
|
vec![
|
||||||
// gpt-oss-120b: schema OFF on purpose. With `reasoning_effort:
|
// gpt-oss-120b: schema OFF on purpose. Reasoning tokens count
|
||||||
// medium`, reasoning tokens count against `max_completion_tokens`
|
// against `max_completion_tokens`; with `high` reasoning the
|
||||||
// (4096); on large bodies (e.g. 6 recordings, ~3.8k chars) the
|
// CoT alone can eat several thousand tokens, so the budget is
|
||||||
// reasoning eats the budget and the model returns
|
// doubled to 8192 to leave room for the actual answer. Without
|
||||||
// `choices[0].message.content == null` — the LlmError::Parse path.
|
// headroom the model returns `choices[0].message.content == null`
|
||||||
// Pre-multi-backend (commit bf6464d) gpt-oss ran without schema
|
// — the LlmError::Parse path. Pre-multi-backend (commit bf6464d)
|
||||||
// and worked fine; this restores that body byte-for-byte.
|
// gpt-oss ran without schema and worked fine; this restores
|
||||||
|
// that body byte-for-byte except for the larger budget.
|
||||||
ionos_backend("gpt_oss_120b", "GPT OSS 120b", "openai/gpt-oss-120b")
|
ionos_backend("gpt_oss_120b", "GPT OSS 120b", "openai/gpt-oss-120b")
|
||||||
.with_reasoning_effort("medium")
|
.with_reasoning_effort("high")
|
||||||
|
.with_max_completion_tokens(8192)
|
||||||
.with_use_json_schema(false),
|
.with_use_json_schema(false),
|
||||||
ionos_backend(
|
ionos_backend(
|
||||||
"llama_3_1_405b",
|
"llama_3_1_405b",
|
||||||
@@ -234,7 +241,7 @@ mod tests {
|
|||||||
fn reasoning_effort_is_set_per_model_family() {
|
fn reasoning_effort_is_set_per_model_family() {
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
find_backend("gpt_oss_120b").unwrap().reasoning_effort,
|
find_backend("gpt_oss_120b").unwrap().reasoning_effort,
|
||||||
Some("medium".to_string())
|
Some("high".to_string())
|
||||||
);
|
);
|
||||||
assert!(
|
assert!(
|
||||||
find_backend("llama_3_1_405b")
|
find_backend("llama_3_1_405b")
|
||||||
@@ -304,11 +311,11 @@ mod tests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// gpt-oss-120b runs in plain-text mode (no `response_format`, no
|
/// gpt-oss-120b runs in plain-text mode (no `response_format`, no
|
||||||
/// `top_p`, no second system message). Reason: with
|
/// `top_p`, no second system message). Reason: schema-constrained
|
||||||
/// `reasoning_effort: medium`, schema-constrained decoding plus reasoning
|
/// decoding plus any non-zero `reasoning_effort` can exhaust
|
||||||
/// can exhaust `max_completion_tokens` on large bodies and return
|
/// `max_completion_tokens` on large bodies and return `content: null`.
|
||||||
/// `content: null`. This pin guarantees the body sent to Ionos stays
|
/// This pin guarantees the body sent to Ionos stays byte-identical to
|
||||||
/// byte-identical to the pre-multi-backend path (commit bf6464d).
|
/// the pre-multi-backend path (commit bf6464d).
|
||||||
#[test]
|
#[test]
|
||||||
fn gpt_oss_runs_in_plain_text_mode_no_schema_no_top_p_no_format_hint() {
|
fn gpt_oss_runs_in_plain_text_mode_no_schema_no_top_p_no_format_hint() {
|
||||||
let b = find_backend("gpt_oss_120b").unwrap();
|
let b = find_backend("gpt_oss_120b").unwrap();
|
||||||
|
|||||||
@@ -14,6 +14,7 @@ use tracing::{info, warn};
|
|||||||
use doctate_common::timestamp::{filename_stem_to_recorded_at, now_rfc3339};
|
use doctate_common::timestamp::{filename_stem_to_recorded_at, now_rfc3339};
|
||||||
use doctate_common::{RECORDING_META_SUFFIX, TranscriptState};
|
use doctate_common::{RECORDING_META_SUFFIX, TranscriptState};
|
||||||
|
|
||||||
|
use crate::analyze::auto_trigger::FAILURE_MARKER_FILE;
|
||||||
use crate::analyze::backend::{any_backend_available, find_backend};
|
use crate::analyze::backend::{any_backend_available, find_backend};
|
||||||
use crate::analyze::{
|
use crate::analyze::{
|
||||||
ANALYSIS_INPUT_FILE, AnalysisInput, AnalyzeJob, AnalyzeSender, DOCUMENT_FILE, RecordingInput,
|
ANALYSIS_INPUT_FILE, AnalysisInput, AnalyzeJob, AnalyzeSender, DOCUMENT_FILE, RecordingInput,
|
||||||
@@ -107,6 +108,17 @@ pub async fn handle_analyze_case(
|
|||||||
|
|
||||||
let input = build_analysis_input(&case_dir, &backend_id).await?;
|
let input = build_analysis_input(&case_dir, &backend_id).await?;
|
||||||
let input_path = case_dir.join(ANALYSIS_INPUT_FILE);
|
let input_path = case_dir.join(ANALYSIS_INPUT_FILE);
|
||||||
|
|
||||||
|
// Manual retry path: a failure marker means the worker has explicitly
|
||||||
|
// given up on the prior input. Drop the stale sentinel so create_new
|
||||||
|
// does not reject the legitimate retry as "Analyse läuft bereits".
|
||||||
|
// The marker itself is left in place — the worker overwrites it on
|
||||||
|
// re-failure and removes it on success, so the UI's "analyzing" flag
|
||||||
|
// (driven by input file presence) takes over from the next render.
|
||||||
|
if case_dir.join(FAILURE_MARKER_FILE).exists() {
|
||||||
|
remove_if_exists(&input_path).await?;
|
||||||
|
}
|
||||||
|
|
||||||
write_input_create_new(&input_path, &input).await?;
|
write_input_create_new(&input_path, &input).await?;
|
||||||
|
|
||||||
// Unbounded send: only fails if the worker has gone away (programmer error
|
// Unbounded send: only fails if the worker has gone away (programmer error
|
||||||
|
|||||||
@@ -272,6 +272,59 @@ async fn analyze_case_second_time_returns_409() {
|
|||||||
assert_eq!(second.status(), StatusCode::CONFLICT);
|
assert_eq!(second.status(), StatusCode::CONFLICT);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Bug regression (case c414cf52, 2026-05-03): a prior LLM run failed,
|
||||||
|
/// leaving `analysis_input.json` AND `.analysis_failed.json` on disk.
|
||||||
|
/// The user clicks "Erneut versuchen" — the handler must accept the
|
||||||
|
/// retry instead of rejecting it as 409 Conflict ("Analyse läuft
|
||||||
|
/// bereits"). The failure marker is the explicit "worker has given up"
|
||||||
|
/// signal that legitimises overwriting the stale sentinel.
|
||||||
|
#[tokio::test]
|
||||||
|
async fn analyze_retry_after_failure_marker_overwrites_stale_input() {
|
||||||
|
let (cfg, settings) = cfg_llm("retry-after-fail");
|
||||||
|
let case_id = "11111111-1111-1111-1111-111111111111";
|
||||||
|
let case_dir = seed_case(&cfg.data_path, "dr_a", case_id);
|
||||||
|
seed_recording(&case_dir, "2026-04-15T10-00-00Z", Some("frischer Text"));
|
||||||
|
|
||||||
|
// Simulate the post-failure on-disk state: stale input from the
|
||||||
|
// attempt the worker gave up on, plus the matching failure marker.
|
||||||
|
std::fs::write(
|
||||||
|
case_dir.join(ANALYSIS_INPUT_FILE),
|
||||||
|
r#"{"last_recording_mtime":"2026-04-15T10:00:00Z","recordings":[{"recorded_at":"2026-04-15T10:00:00Z","text":"alter Text"}],"backend_id":""}"#,
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
std::fs::write(
|
||||||
|
case_dir.join(".analysis_failed.json"),
|
||||||
|
r#"{"last_recording_mtime":"2026-04-15T10:00:00Z","reason":"llm: connect timeout","failed_at":"2026-04-15T10:05:00Z"}"#,
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
let (app, store) = doctate_server::create_router_and_session_store_with_settings(cfg, settings);
|
||||||
|
let (cookie, csrf) = login_with_csrf(&app, &store, "dr_a", "s").await;
|
||||||
|
|
||||||
|
let resp = app
|
||||||
|
.oneshot(csrf_form_post(
|
||||||
|
paths::case_analyze(case_id),
|
||||||
|
&cookie,
|
||||||
|
&csrf,
|
||||||
|
"",
|
||||||
|
))
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(
|
||||||
|
resp.status(),
|
||||||
|
StatusCode::SEE_OTHER,
|
||||||
|
"retry with failure marker present must succeed, not 409"
|
||||||
|
);
|
||||||
|
|
||||||
|
// Input was rewritten with the fresh transcript — the stale "alter Text"
|
||||||
|
// must be gone.
|
||||||
|
let raw = std::fs::read_to_string(case_dir.join(ANALYSIS_INPUT_FILE)).unwrap();
|
||||||
|
let parsed: Value = serde_json::from_str(&raw).unwrap();
|
||||||
|
let recs = parsed["recordings"].as_array().unwrap();
|
||||||
|
assert_eq!(recs.len(), 1);
|
||||||
|
assert_eq!(recs[0]["text"], "frischer Text");
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn analyze_case_skips_blank_transcript_from_recordings() {
|
async fn analyze_case_skips_blank_transcript_from_recordings() {
|
||||||
let (cfg, settings) = cfg_llm("i");
|
let (cfg, settings) = cfg_llm("i");
|
||||||
|
|||||||
@@ -116,6 +116,9 @@ async fn case_page_shows_analyze_button_when_transcribed_without_document() {
|
|||||||
/// and the auto-trigger refuses to retry as long as the marker matches the
|
/// and the auto-trigger refuses to retry as long as the marker matches the
|
||||||
/// current input. The case page must therefore surface a recovery banner
|
/// current input. The case page must therefore surface a recovery banner
|
||||||
/// that exposes the reason and a retry form, otherwise the user is stuck.
|
/// that exposes the reason and a retry form, otherwise the user is stuck.
|
||||||
|
///
|
||||||
|
/// Doctor variant: only the default-backend "Erneut versuchen" button is
|
||||||
|
/// shown; per-backend choice is admin-only (covered by the sibling test).
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn case_page_shows_failure_banner_with_retry_when_marker_present() {
|
async fn case_page_shows_failure_banner_with_retry_when_marker_present() {
|
||||||
let (cfg, settings) = TestConfig::new()
|
let (cfg, settings) = TestConfig::new()
|
||||||
@@ -173,8 +176,12 @@ async fn case_page_shows_failure_banner_with_retry_when_marker_present() {
|
|||||||
"retry form must POST to the analyze endpoint"
|
"retry form must POST to the analyze endpoint"
|
||||||
);
|
);
|
||||||
assert!(
|
assert!(
|
||||||
body.contains("Erneut mit "),
|
body.contains(">Erneut versuchen</button>"),
|
||||||
"retry button label missing (expected 'Erneut mit <backend>')"
|
"default retry button missing (expected 'Erneut versuchen')"
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
!body.contains("Erneut mit "),
|
||||||
|
"per-backend retry buttons must not be shown to non-admins"
|
||||||
);
|
);
|
||||||
assert!(
|
assert!(
|
||||||
!body.contains("Wird analysiert"),
|
!body.contains("Wird analysiert"),
|
||||||
@@ -182,6 +189,57 @@ async fn case_page_shows_failure_banner_with_retry_when_marker_present() {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Admin variant of the failure-banner test: in addition to the default
|
||||||
|
/// "Erneut versuchen" button shown to every user, an admin sees one
|
||||||
|
/// "Erneut mit <backend>" button per configured backend, letting them
|
||||||
|
/// re-analyse with a different model when one is misbehaving. Pins the
|
||||||
|
/// `{% if is_admin %}` gate in `case_page.html` so a future refactor
|
||||||
|
/// can't silently expose backend choice to all users (matches the
|
||||||
|
/// `analyze` handler's 403 guard) or remove it from admins.
|
||||||
|
#[tokio::test]
|
||||||
|
async fn case_page_failure_banner_admin_can_choose_backend() {
|
||||||
|
let (cfg, settings) = TestConfig::new()
|
||||||
|
.with_label("cp-fail-banner-admin")
|
||||||
|
.with_user(test_admin("dr_a"))
|
||||||
|
.with_llm("http://unused")
|
||||||
|
.build_pair();
|
||||||
|
let case_id = "11111111-1111-1111-1111-111111111111";
|
||||||
|
let case_dir = seed_case(&cfg.data_path, "dr_a", case_id);
|
||||||
|
seed_recording(
|
||||||
|
&case_dir,
|
||||||
|
"2026-04-15T10-00-00Z",
|
||||||
|
Some("transkribierter Text"),
|
||||||
|
);
|
||||||
|
std::fs::write(
|
||||||
|
case_dir.join(".analysis_failed.json"),
|
||||||
|
r#"{"last_recording_mtime":"2026-04-15T10:00:00Z","reason":"llm: connect timeout","failed_at":"2026-04-15T10:05:00Z"}"#,
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
let app = doctate_server::create_router_with_settings(cfg, settings);
|
||||||
|
let cookie = common::login(&app, "dr_a", "s").await;
|
||||||
|
|
||||||
|
let resp = app
|
||||||
|
.oneshot(get_with_cookie(paths::case_detail(case_id), &cookie))
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(resp.status(), StatusCode::OK);
|
||||||
|
let body = body_string(resp).await;
|
||||||
|
|
||||||
|
assert!(
|
||||||
|
body.contains(r#"class="failure-banner""#),
|
||||||
|
"failure-banner div missing for admin"
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
body.contains(">Erneut versuchen</button>"),
|
||||||
|
"default retry button must remain visible for admin too"
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
body.contains("Erneut mit "),
|
||||||
|
"admin must see at least one 'Erneut mit <backend>' button"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
#[ignore = "post-refactor any_backend_available() reads IONOS_API_KEY which TestConfig pre-sets; cannot be unset for one test in a shared process"]
|
#[ignore = "post-refactor any_backend_available() reads IONOS_API_KEY which TestConfig pre-sets; cannot be unset for one test in a shared process"]
|
||||||
async fn case_page_shows_llm_missing_hint_without_llm() {
|
async fn case_page_shows_llm_missing_hint_without_llm() {
|
||||||
|
|||||||
Reference in New Issue
Block a user