1
Fork 0
mirror of https://github.com/thegeneralist01/archivr synced 2026-10-09 12:55:00 +02:00

fix: retain completed summary during regeneration display

This commit is contained in:
archivr-qa 2026-08-24 16:50:51 +02:00
parent 13ae315d99
commit 33cdb21926
No known key found for this signature in database
4 changed files with 110 additions and 31 deletions

View file

@ -51,9 +51,12 @@ pub struct EntryDetail {
pub source_metadata_json: String, pub source_metadata_json: String,
pub display_metadata_json: Option<String>, pub display_metadata_json: Option<String>,
pub artifacts: Vec<EntryArtifactSummary>, pub artifacts: Vec<EntryArtifactSummary>,
/// Most recently updated summary for this entry, if any has ever been /// Most recent completed summary for this entry. Always `None` on a fresh
/// requested. Always `None` on a fresh capture — summarization is manual. /// capture — summarization is manual.
pub latest_summary: Option<EntrySummaryView>, pub latest_summary: Option<EntrySummaryView>,
/// Latest non-completed generation attempt, kept separate so a replacement
/// never displaces readable completed content.
pub summary_attempt: Option<EntrySummaryView>,
} }
#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize)] #[derive(Debug, Clone, PartialEq, Eq, serde::Serialize)]
@ -354,7 +357,8 @@ pub fn get_entry_detail(
})? })?
.collect::<rusqlite::Result<Vec<_>>>()?; .collect::<rusqlite::Result<Vec<_>>>()?;
let latest_summary = database::latest_entry_summary(conn, entry_id)?; let latest_summary = database::latest_completed_entry_summary(conn, entry_id)?;
let summary_attempt = database::latest_entry_summary_attempt(conn, entry_id)?;
Ok(Some(EntryDetail { Ok(Some(EntryDetail {
summary, summary,
@ -363,6 +367,7 @@ pub fn get_entry_detail(
display_metadata_json, display_metadata_json,
artifacts, artifacts,
latest_summary, latest_summary,
summary_attempt,
})) }))
} }

View file

@ -1644,7 +1644,8 @@ pub fn find_entry_summary(
} }
/// Most recently touched summary for an entry, whatever its status. /// Most recently touched summary for an entry, whatever its status.
/// Backs `EntryDetail.latest_summary` and the GET summary route. /// Used where a caller explicitly needs the most recent attempt regardless of
/// whether it has completed.
pub fn latest_entry_summary( pub fn latest_entry_summary(
conn: &Connection, conn: &Connection,
entry_id: i64, entry_id: i64,
@ -1679,6 +1680,24 @@ pub fn latest_completed_entry_summary(
.map_err(Into::into) .map_err(Into::into)
} }
/// The newest replacement attempt that has not completed. This includes failed
/// rows so authenticated callers can show the failure beside a retained result.
pub fn latest_entry_summary_attempt(
conn: &Connection,
entry_id: i64,
) -> Result<Option<EntrySummaryRecord>> {
conn.query_row(
&format!(
"{ENTRY_SUMMARY_COLS} WHERE s.entry_id = ?1 AND s.status != 'completed'
ORDER BY s.updated_at DESC, s.id DESC LIMIT 1"
),
[entry_id],
map_entry_summary,
)
.optional()
.map_err(Into::into)
}
pub fn create_archive_run( pub fn create_archive_run(
conn: &Connection, conn: &Connection,
created_by_user_id: i64, created_by_user_id: i64,
@ -4719,6 +4738,36 @@ mod tests {
); );
} }
#[test]
fn latest_summary_attempt_is_separate_from_the_retained_completed_summary() {
let c = conn();
let entry = create_entry_fixture(&c, "private", None, None);
let completed =
upsert_pending_entry_summary(&c, entry.id, "claude_cli", None, "v1", "old").unwrap();
update_entry_summary_status(&c, &completed, "completed", Some("previous"), None).unwrap();
let pending =
upsert_pending_entry_summary(&c, entry.id, "claude_cli", None, "v1", "new").unwrap();
assert_eq!(
latest_completed_entry_summary(&c, entry.id).unwrap().unwrap().summary_uid,
completed
);
assert_eq!(
latest_entry_summary_attempt(&c, entry.id).unwrap().unwrap().summary_uid,
pending
);
update_entry_summary_status(&c, &pending, "failed", None, Some("boom")).unwrap();
assert_eq!(
latest_completed_entry_summary(&c, entry.id).unwrap().unwrap().summary_text.as_deref(),
Some("previous")
);
assert_eq!(
latest_entry_summary_attempt(&c, entry.id).unwrap().unwrap().status,
"failed"
);
}
#[test] #[test]
fn fail_stalled_entry_summaries_marks_pending_and_running_with_restart_message() { fn fail_stalled_entry_summaries_marks_pending_and_running_with_restart_message() {
let c = conn(); let c = conn();

View file

@ -519,6 +519,7 @@ async fn entry_detail(
let entry_id = database::entry_id_for_uid(&conn, &entry_uid)? let entry_id = database::entry_id_for_uid(&conn, &entry_uid)?
.ok_or(ApiError::not_found("entry not found"))?; .ok_or(ApiError::not_found("entry not found"))?;
detail.latest_summary = database::latest_completed_entry_summary(&conn, entry_id)?; detail.latest_summary = database::latest_completed_entry_summary(&conn, entry_id)?;
detail.summary_attempt = None;
} }
Ok(Json(detail)) Ok(Json(detail))
} }
@ -552,14 +553,19 @@ async fn entry_summary_handler(
} }
let entry_id = database::entry_id_for_uid(&conn, &entry_uid)? let entry_id = database::entry_id_for_uid(&conn, &entry_uid)?
.ok_or(ApiError::not_found("entry not found"))?; .ok_or(ApiError::not_found("entry not found"))?;
let summary = if matches!(auth_user, AuthUser::Guest) { let (summary, attempt) = if matches!(auth_user, AuthUser::Guest) {
database::latest_completed_entry_summary(&conn, entry_id)? (database::latest_completed_entry_summary(&conn, entry_id)?, None)
} else { } else {
database::latest_entry_summary(&conn, entry_id)? (
database::latest_completed_entry_summary(&conn, entry_id)?,
database::latest_entry_summary_attempt(&conn, entry_id)?,
)
}; };
Ok(Json( let mut body = serde_json::json!({ "entry_uid": entry_uid, "summary": summary });
serde_json::json!({ "entry_uid": entry_uid, "summary": summary }), if !matches!(auth_user, AuthUser::Guest) {
)) body["attempt"] = serde_json::to_value(attempt)?;
}
Ok(Json(body))
} }
/// `POST /api/archives/:id/entries/:uid/summary` /// `POST /api/archives/:id/entries/:uid/summary`
@ -3209,6 +3215,14 @@ mod tests {
let entry = make_test_entry(&archive_path); let entry = make_test_entry(&archive_path);
let session = make_test_session(&auth_path); let session = make_test_session(&auth_path);
let conn = database::open_or_initialize(&archive_path).unwrap(); let conn = database::open_or_initialize(&archive_path).unwrap();
let completed_uid = database::upsert_pending_entry_summary(
&conn, entry.id, "codex_cli", None, summarizer::PROMPT_VERSION, "completed-public-test",
)
.unwrap();
database::update_entry_summary_status(
&conn, &completed_uid, "completed", Some("previous completed summary"), None,
)
.unwrap();
let summary_uid = database::upsert_pending_entry_summary( let summary_uid = database::upsert_pending_entry_summary(
&conn, entry.id, "codex_cli", None, summarizer::PROMPT_VERSION, "failed-public-test", &conn, entry.id, "codex_cli", None, summarizer::PROMPT_VERSION, "failed-public-test",
) )
@ -3233,7 +3247,9 @@ mod tests {
.body(Body::empty()).unwrap()) .body(Body::empty()).unwrap())
.await.unwrap(); .await.unwrap();
assert_eq!(public_summary.status(), StatusCode::OK); assert_eq!(public_summary.status(), StatusCode::OK);
assert!(body_json(public_summary).await["summary"].is_null()); let public_summary = body_json(public_summary).await;
assert_eq!(public_summary["summary"]["summary_uid"], completed_uid);
assert!(public_summary.get("attempt").is_none());
let public_detail = app(registry.clone(), auth_path.clone()) let public_detail = app(registry.clone(), auth_path.clone())
.oneshot(Request::builder() .oneshot(Request::builder()
@ -3241,7 +3257,9 @@ mod tests {
.body(Body::empty()).unwrap()) .body(Body::empty()).unwrap())
.await.unwrap(); .await.unwrap();
assert_eq!(public_detail.status(), StatusCode::OK); assert_eq!(public_detail.status(), StatusCode::OK);
assert!(body_json(public_detail).await["latest_summary"].is_null()); let public_detail = body_json(public_detail).await;
assert_eq!(public_detail["latest_summary"]["summary_uid"], completed_uid);
assert!(public_detail["summary_attempt"].is_null());
let authenticated_summary = app(registry.clone(), auth_path.clone()) let authenticated_summary = app(registry.clone(), auth_path.clone())
.oneshot(Request::builder() .oneshot(Request::builder()
@ -3250,8 +3268,10 @@ mod tests {
.body(Body::empty()).unwrap()) .body(Body::empty()).unwrap())
.await.unwrap(); .await.unwrap();
let authenticated_summary = body_json(authenticated_summary).await; let authenticated_summary = body_json(authenticated_summary).await;
assert_eq!(authenticated_summary["summary"]["status"], "failed"); assert_eq!(authenticated_summary["summary"]["summary_uid"], completed_uid);
assert_eq!(authenticated_summary["summary"]["error_text"], "provider secret: raw diagnostic"); assert_eq!(authenticated_summary["summary"]["summary_text"], "previous completed summary");
assert_eq!(authenticated_summary["attempt"]["status"], "failed");
assert_eq!(authenticated_summary["attempt"]["error_text"], "provider secret: raw diagnostic");
let authenticated_detail = app(registry, auth_path) let authenticated_detail = app(registry, auth_path)
.oneshot(Request::builder() .oneshot(Request::builder()
@ -3260,8 +3280,10 @@ mod tests {
.body(Body::empty()).unwrap()) .body(Body::empty()).unwrap())
.await.unwrap(); .await.unwrap();
let authenticated_detail = body_json(authenticated_detail).await; let authenticated_detail = body_json(authenticated_detail).await;
assert_eq!(authenticated_detail["latest_summary"]["status"], "failed"); assert_eq!(authenticated_detail["latest_summary"]["summary_uid"], completed_uid);
assert_eq!(authenticated_detail["latest_summary"]["error_text"], "provider secret: raw diagnostic"); assert_eq!(authenticated_detail["latest_summary"]["summary_text"], "previous completed summary");
assert_eq!(authenticated_detail["summary_attempt"]["status"], "failed");
assert_eq!(authenticated_detail["summary_attempt"]["error_text"], "provider secret: raw diagnostic");
} }
#[test] #[test]

View file

@ -61,10 +61,10 @@ export default function ContextRail({ archiveId, selectedEntry, selectedUids, se
useEffect(() => { setFontsOpen(false) }, [detail?.summary?.entry_uid]) useEffect(() => { setFontsOpen(false) }, [detail?.summary?.entry_uid])
// ── Summary state ─────────────────────────────────────────────────────── // ── Summary state ───────────────────────────────────────────────────────
// `summary` mirrors the server row. It is seeded from detail.latest_summary so // A completed summary and its replacement attempt are intentionally separate:
// the section renders immediately on selection, then kept fresh by polling // regeneration must not blank or overwrite readable content while it runs.
// only while a job is non-terminal.
const [summary, setSummary] = useState(null) const [summary, setSummary] = useState(null)
const [summaryAttempt, setSummaryAttempt] = useState(null)
const [summaryError, setSummaryError] = useState('') const [summaryError, setSummaryError] = useState('')
const [summaryBusy, setSummaryBusy] = useState(false) const [summaryBusy, setSummaryBusy] = useState(false)
const [summaryProvider, setSummaryProvider] = useState(() => { const [summaryProvider, setSummaryProvider] = useState(() => {
@ -145,19 +145,20 @@ export default function ContextRail({ archiveId, selectedEntry, selectedUids, se
summaryGenerateAbortRef.current = null summaryGenerateAbortRef.current = null
const detailMatchesSelection = detail?.summary?.entry_uid === selectedEntry?.entry_uid const detailMatchesSelection = detail?.summary?.entry_uid === selectedEntry?.entry_uid
setSummary(detailMatchesSelection ? detail.latest_summary ?? null : null) setSummary(detailMatchesSelection ? detail.latest_summary ?? null : null)
setSummaryAttempt(detailMatchesSelection ? detail.summary_attempt ?? null : null)
setSummaryError('') setSummaryError('')
setSummaryBusy(false) setSummaryBusy(false)
setIncludeSummaryImages(false) setIncludeSummaryImages(false)
}, [archiveId, selectedEntry?.entry_uid, detail?.summary?.entry_uid]) }, [archiveId, selectedEntry?.entry_uid, detail?.summary?.entry_uid])
// Poll only while the latest summary is non-terminal. Anchoring the effect on // Poll only while a replacement attempt is non-terminal. Anchoring the effect
// the status (rather than starting a timer inside the click handler) means a // on its status means a job still running when the user navigates away and
// job still running when the user navigates away and back is picked up again. // back is picked up again without displacing completed content.
const summaryStatus = summary?.status const summaryAttemptStatus = summaryAttempt?.status
useEffect(() => { useEffect(() => {
clearInterval(summaryPollRef.current) clearInterval(summaryPollRef.current)
summaryPollRef.current = null summaryPollRef.current = null
if (summaryStatus !== 'pending' && summaryStatus !== 'running') return if (summaryAttemptStatus !== 'pending' && summaryAttemptStatus !== 'running') return
if (!archiveId || !detail?.summary?.entry_uid) return if (!archiveId || !detail?.summary?.entry_uid) return
const entryUid = detail.summary.entry_uid const entryUid = detail.summary.entry_uid
const selectionKey = `${archiveId}:${entryUid}` const selectionKey = `${archiveId}:${entryUid}`
@ -169,7 +170,8 @@ export default function ContextRail({ archiveId, selectedEntry, selectedUids, se
const res = await fetchEntrySummary(archiveId, entryUid, { signal: controller.signal }) const res = await fetchEntrySummary(archiveId, entryUid, { signal: controller.signal })
if (controller.signal.aborted || summarySelectionRef.current !== selectionKey) return if (controller.signal.aborted || summarySelectionRef.current !== selectionKey) return
setSummary(res.summary ?? null) setSummary(res.summary ?? null)
const st = res.summary?.status setSummaryAttempt(res.attempt ?? null)
const st = res.attempt?.status
if (st !== 'pending' && st !== 'running') { if (st !== 'pending' && st !== 'running') {
clearInterval(intervalId) clearInterval(intervalId)
if (summaryPollRef.current === intervalId) summaryPollRef.current = null if (summaryPollRef.current === intervalId) summaryPollRef.current = null
@ -190,7 +192,7 @@ export default function ContextRail({ archiveId, selectedEntry, selectedUids, se
controller.abort() controller.abort()
if (summaryPollAbortRef.current === controller) summaryPollAbortRef.current = null if (summaryPollAbortRef.current === controller) summaryPollAbortRef.current = null
} }
}, [summaryStatus, archiveId, selectedEntry?.entry_uid, detail?.summary?.entry_uid, summarySelectionKey]) }, [summaryAttemptStatus, archiveId, selectedEntry?.entry_uid, detail?.summary?.entry_uid, summarySelectionKey])
useEffect(() => () => { useEffect(() => () => {
clearInterval(summaryPollRef.current) clearInterval(summaryPollRef.current)
@ -219,12 +221,13 @@ export default function ContextRail({ archiveId, selectedEntry, selectedUids, se
if (res.status === 'completed') { if (res.status === 'completed') {
// 200 cache hit: the response *is* the row, no polling needed. // 200 cache hit: the response *is* the row, no polling needed.
setSummary(res) setSummary(res)
setSummaryAttempt(null)
setSummaryBusy(false) setSummaryBusy(false)
if (summarySelectionRef.current === selectionKey) onDetailRefresh?.() if (summarySelectionRef.current === selectionKey) onDetailRefresh?.()
} else { } else {
// 202: seed a local pending row so the poll effect starts immediately // 202: seed a local pending row so the poll effect starts immediately
// rather than waiting a tick for the first GET. // rather than waiting a tick for the first GET.
setSummary({ ...(res ?? {}), status: 'pending' }) setSummaryAttempt({ ...(res ?? {}), status: 'pending' })
} }
} catch (e) { } catch (e) {
if (controller.signal.aborted || summarySelectionRef.current !== selectionKey) return if (controller.signal.aborted || summarySelectionRef.current !== selectionKey) return
@ -607,9 +610,9 @@ export default function ContextRail({ archiveId, selectedEntry, selectedUids, se
const parsed = summary?.status === 'completed' const parsed = summary?.status === 'completed'
? parseSummaryText(summary.summary_text) ? parseSummaryText(summary.summary_text)
: null : null
const running = summary?.status === 'pending' || summary?.status === 'running' const running = summaryAttempt?.status === 'pending' || summaryAttempt?.status === 'running'
const unsupportedContent = const unsupportedContent =
(summary?.status === 'failed' && summary.error_text === UNSUPPORTED_SUMMARY_CONTENT_MESSAGE) || (summaryAttempt?.status === 'failed' && summaryAttempt.error_text === UNSUPPORTED_SUMMARY_CONTENT_MESSAGE) ||
summaryError === UNSUPPORTED_SUMMARY_CONTENT_MESSAGE summaryError === UNSUPPORTED_SUMMARY_CONTENT_MESSAGE
if (isPublicSession && !parsed) return null if (isPublicSession && !parsed) return null
return ( return (
@ -649,9 +652,9 @@ export default function ContextRail({ archiveId, selectedEntry, selectedUids, se
<p className="rail-summary-info__detail">{UNSUPPORTED_SUMMARY_CONTENT_DETAIL}</p> <p className="rail-summary-info__detail">{UNSUPPORTED_SUMMARY_CONTENT_DETAIL}</p>
</div> </div>
)} )}
{summary?.status === 'failed' && summary.error_text && !unsupportedContent && !isPublicSession && ( {summaryAttempt?.status === 'failed' && summaryAttempt.error_text && !unsupportedContent && !isPublicSession && (
<p className="form-msg form-msg--err rail-summary-error"> <p className="form-msg form-msg--err rail-summary-error">
{summary.error_text} {summaryAttempt.error_text}
</p> </p>
)} )}
{summaryError && !unsupportedContent && ( {summaryError && !unsupportedContent && (