mirror of
https://github.com/thegeneralist01/archivr
synced 2026-10-09 12:55:00 +02:00
Merge branch 'fix/civil-summary-errors' into integration-all-three
This commit is contained in:
commit
8f36617c5d
4 changed files with 265 additions and 26 deletions
|
|
@ -40,6 +40,15 @@ pub const MAX_SUMMARY_IMAGE_BYTES: u64 = 5 * 1024 * 1024;
|
|||
/// Maximum combined byte size for explicitly opted-in images.
|
||||
pub const MAX_SUMMARY_IMAGE_TOTAL_BYTES: u64 = 12 * 1024 * 1024;
|
||||
|
||||
/// User-safe copy for entries whose archived artifacts do not contain
|
||||
/// summarizable text. Keep this separate from provider and archive failures.
|
||||
pub const UNSUPPORTED_SUMMARY_CONTENT_HEADING: &str = "This entry can’t be summarized yet.";
|
||||
pub const UNSUPPORTED_SUMMARY_CONTENT_DETAIL: &str = "It doesn’t contain archived text that a summary provider can read. Summaries currently support text notes, web pages, X posts and threads, and X Articles. Video, audio, and image-only entries need a transcript or text source.";
|
||||
pub const UNSUPPORTED_SUMMARY_CONTENT_MESSAGE: &str = concat!(
|
||||
"This entry can’t be summarized yet.\n\n",
|
||||
"It doesn’t contain archived text that a summary provider can read. Summaries currently support text notes, web pages, X posts and threads, and X Articles. Video, audio, and image-only entries need a transcript or text source."
|
||||
);
|
||||
|
||||
/// Upper bound on characters fed to a model. Archived pages run to hundreds of
|
||||
/// kilobytes; past this point we are paying for tokens that do not change a
|
||||
/// five-sentence summary. Truncation happens *before* hashing so the cache key
|
||||
|
|
@ -49,6 +58,28 @@ const MAX_INPUT_CHARS: usize = 48_000;
|
|||
const DEFAULT_HTTP_TIMEOUT_SECS: u64 = 120;
|
||||
const DEFAULT_CLI_TIMEOUT_SECS: u64 = 300;
|
||||
|
||||
#[derive(Debug)]
|
||||
struct UnsupportedSummaryContent;
|
||||
|
||||
impl std::fmt::Display for UnsupportedSummaryContent {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
f.write_str("unsupported summary content")
|
||||
}
|
||||
}
|
||||
|
||||
impl std::error::Error for UnsupportedSummaryContent {}
|
||||
|
||||
fn unsupported_summary_content_error() -> anyhow::Error {
|
||||
anyhow::Error::new(UnsupportedSummaryContent)
|
||||
}
|
||||
|
||||
/// True only for expected, pre-provider summary-input limitations.
|
||||
pub fn is_unsupported_summary_content_error(error: &anyhow::Error) -> bool {
|
||||
error
|
||||
.chain()
|
||||
.any(|cause| cause.downcast_ref::<UnsupportedSummaryContent>().is_some())
|
||||
}
|
||||
|
||||
/// The instruction half of the prompt. JSON output is requested because parsing
|
||||
/// prose out of a free-form answer is the single most fragile part of an LLM
|
||||
/// integration; a JSON object survives models that like to add pleasantries.
|
||||
|
|
@ -1026,7 +1057,7 @@ pub fn build_summary_input(
|
|||
artifacts = load_summary_artifacts(&conn, entry_id, "primary_media")?;
|
||||
}
|
||||
if artifacts.is_empty() {
|
||||
bail!("entry {entry_uid} has no {primary_role} artifact to summarize");
|
||||
return Err(unsupported_summary_content_error());
|
||||
}
|
||||
|
||||
let mut pieces: Vec<String> = Vec::with_capacity(artifacts.len());
|
||||
|
|
@ -1051,11 +1082,7 @@ pub fn build_summary_input(
|
|||
.with_context(|| format!("{} is not valid JSON", abs.display()))?;
|
||||
extract_tweet_text(&parsed).unwrap_or_default()
|
||||
} else {
|
||||
bail!(
|
||||
"no text content available for this entry kind — v1 unsupported \
|
||||
(artifact {relpath}, mime {})",
|
||||
if mime.is_empty() { "unknown" } else { &mime }
|
||||
);
|
||||
return Err(unsupported_summary_content_error());
|
||||
};
|
||||
if !piece.trim().is_empty() {
|
||||
pieces.push(piece);
|
||||
|
|
@ -1065,7 +1092,7 @@ pub fn build_summary_input(
|
|||
// humans and models. Single-piece entries never render the separator.
|
||||
let content = pieces.join("\n\n---\n\n").trim().to_string();
|
||||
if content.is_empty() {
|
||||
bail!("no text content available for this entry kind — v1 unsupported (extracted text was empty)");
|
||||
return Err(unsupported_summary_content_error());
|
||||
}
|
||||
// Truncate on a char boundary, then hash: the digest must describe the
|
||||
// bytes actually sent, or the cache would key on content the model never saw.
|
||||
|
|
@ -1743,6 +1770,69 @@ mod tests {
|
|||
(temp, paths, entry)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unsupported_summary_content_errors_are_classified_without_relabeling_other_errors() {
|
||||
let (_temp, paths, entry) = summary_image_fixture();
|
||||
let conn = database::open_or_initialize(&paths.archive_path).unwrap();
|
||||
conn.execute(
|
||||
"DELETE FROM entry_artifacts WHERE entry_id = ?1",
|
||||
[entry.id],
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let no_artifact = build_summary_input(&paths, &entry.entry_uid, SummaryBuildOptions::default())
|
||||
.unwrap_err();
|
||||
assert!(is_unsupported_summary_content_error(&no_artifact));
|
||||
|
||||
add_summary_image_artifact(
|
||||
&paths,
|
||||
entry.id,
|
||||
99,
|
||||
"primary_media",
|
||||
"mp4",
|
||||
"video/mp4",
|
||||
1,
|
||||
);
|
||||
let video = build_summary_input(&paths, &entry.entry_uid, SummaryBuildOptions::default())
|
||||
.unwrap_err();
|
||||
assert!(is_unsupported_summary_content_error(&video));
|
||||
|
||||
let conn = database::open_or_initialize(&paths.archive_path).unwrap();
|
||||
conn.execute(
|
||||
"DELETE FROM entry_artifacts WHERE entry_id = ?1",
|
||||
[entry.id],
|
||||
)
|
||||
.unwrap();
|
||||
let empty_relpath = "raw/empty-summary.txt";
|
||||
std::fs::write(paths.store_path.join(empty_relpath), "").unwrap();
|
||||
database::add_entry_artifact(
|
||||
&conn,
|
||||
&database::NewArtifact {
|
||||
entry_id: entry.id,
|
||||
artifact_role: "primary_media".to_string(),
|
||||
storage_area: "raw".to_string(),
|
||||
relpath: empty_relpath.to_string(),
|
||||
blob_id: None,
|
||||
logical_path: None,
|
||||
metadata_json: None,
|
||||
},
|
||||
)
|
||||
.unwrap();
|
||||
drop(conn);
|
||||
let empty_text = build_summary_input(&paths, &entry.entry_uid, SummaryBuildOptions::default())
|
||||
.unwrap_err();
|
||||
assert!(is_unsupported_summary_content_error(&empty_text));
|
||||
|
||||
std::fs::remove_file(paths.store_path.join(empty_relpath)).unwrap();
|
||||
let read_error = build_summary_input(&paths, &entry.entry_uid, SummaryBuildOptions::default())
|
||||
.unwrap_err();
|
||||
assert!(!is_unsupported_summary_content_error(&read_error));
|
||||
assert_eq!(UNSUPPORTED_SUMMARY_CONTENT_MESSAGE, "This entry can’t be summarized yet.\n\nIt doesn’t contain archived text that a summary provider can read. Summaries currently support text notes, web pages, X posts and threads, and X Articles. Video, audio, and image-only entries need a transcript or text source.");
|
||||
|
||||
assert!(!is_unsupported_summary_content_error(&anyhow!("provider timeout")));
|
||||
assert!(!is_unsupported_summary_content_error(&anyhow!("entry not found: {}", entry.entry_uid)));
|
||||
}
|
||||
|
||||
fn add_summary_image_artifact(
|
||||
paths: &ArchivePaths,
|
||||
entry_id: i64,
|
||||
|
|
|
|||
|
|
@ -597,7 +597,13 @@ async fn request_entry_summary_handler(
|
|||
// 2. Extract the same content the summarizer will feed the model, so the
|
||||
// digest below is the identical cache key summarize_entry will compute.
|
||||
let input = summarizer::build_summary_input(&archive_paths, &entry_uid, summary_options)
|
||||
.map_err(|e| ApiError::bad_request(&format!("{e:#}")))?;
|
||||
.map_err(|e| {
|
||||
if summarizer::is_unsupported_summary_content_error(&e) {
|
||||
ApiError::bad_request(summarizer::UNSUPPORTED_SUMMARY_CONTENT_MESSAGE)
|
||||
} else {
|
||||
ApiError::bad_request(&format!("{e:#}"))
|
||||
}
|
||||
})?;
|
||||
|
||||
// 3. Cache hit: identical entry + provider + model + prompt + input.
|
||||
if !body.force {
|
||||
|
|
@ -651,20 +657,7 @@ async fn request_entry_summary_handler(
|
|||
)
|
||||
{
|
||||
eprintln!("warn: summary {summary_uid_bg}: {e:#}");
|
||||
if let Ok(conn) = database::open_or_initialize(&archive_path) {
|
||||
if let Ok(Some(row)) = database::get_entry_summary_by_uid(&conn, &summary_uid_bg) {
|
||||
if row.status != "failed" {
|
||||
database::update_entry_summary_status(
|
||||
&conn,
|
||||
&summary_uid_bg,
|
||||
"failed",
|
||||
None,
|
||||
Some(&format!("{e:#}")),
|
||||
)
|
||||
.ok();
|
||||
}
|
||||
}
|
||||
}
|
||||
record_background_summary_failure(&archive_path, &summary_uid_bg, &e);
|
||||
}
|
||||
});
|
||||
|
||||
|
|
@ -678,6 +671,35 @@ async fn request_entry_summary_handler(
|
|||
))
|
||||
}
|
||||
|
||||
fn summary_failure_error_text(error: &anyhow::Error) -> String {
|
||||
if summarizer::is_unsupported_summary_content_error(error) {
|
||||
summarizer::UNSUPPORTED_SUMMARY_CONTENT_MESSAGE.to_string()
|
||||
} else {
|
||||
format!("{error:#}")
|
||||
}
|
||||
}
|
||||
|
||||
fn record_background_summary_failure(
|
||||
archive_path: &std::path::Path,
|
||||
summary_uid: &str,
|
||||
error: &anyhow::Error,
|
||||
) {
|
||||
if let Ok(conn) = database::open_or_initialize(archive_path) {
|
||||
if let Ok(Some(row)) = database::get_entry_summary_by_uid(&conn, summary_uid) {
|
||||
if row.status != "failed" {
|
||||
database::update_entry_summary_status(
|
||||
&conn,
|
||||
summary_uid,
|
||||
"failed",
|
||||
None,
|
||||
Some(&summary_failure_error_text(error)),
|
||||
)
|
||||
.ok();
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async fn list_runs(
|
||||
State(state): State<AppState>,
|
||||
auth_user: AuthUser,
|
||||
|
|
@ -3098,6 +3120,101 @@ mod tests {
|
|||
assert!(database::latest_entry_summary(&conn, entry.id).unwrap().is_none());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn summary_preflight_returns_safe_message_for_unsupported_video_content() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let (registry, archive_path, auth_path) = make_test_registry(&dir);
|
||||
let entry = make_test_entry(&archive_path);
|
||||
add_summary_test_artifact(
|
||||
&archive_path,
|
||||
entry.id,
|
||||
"raw/private-video.mp4",
|
||||
"primary_media",
|
||||
"video/mp4",
|
||||
b"video fixture",
|
||||
);
|
||||
let session_cookie = make_test_session(&auth_path);
|
||||
let previous_codex_cli = std::env::var_os("ARCHIVR_CODEX_CLI");
|
||||
unsafe { std::env::set_var("ARCHIVR_CODEX_CLI", "/usr/bin/false") };
|
||||
|
||||
let response = app(registry, auth_path)
|
||||
.oneshot(
|
||||
Request::builder()
|
||||
.method("POST")
|
||||
.uri(format!("/api/archives/test/entries/{}/summary", entry.entry_uid))
|
||||
.header("content-type", "application/json")
|
||||
.header("cookie", &session_cookie)
|
||||
.body(json_body(&serde_json::json!({ "provider": "codex_cli" })))
|
||||
.unwrap(),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
match previous_codex_cli {
|
||||
Some(value) => unsafe { std::env::set_var("ARCHIVR_CODEX_CLI", value) },
|
||||
None => unsafe { std::env::remove_var("ARCHIVR_CODEX_CLI") },
|
||||
}
|
||||
|
||||
assert_eq!(response.status(), StatusCode::BAD_REQUEST);
|
||||
let error = body_json(response).await["error"].as_str().unwrap().to_string();
|
||||
assert_eq!(error, summarizer::UNSUPPORTED_SUMMARY_CONTENT_MESSAGE);
|
||||
assert!(!error.contains("raw/"));
|
||||
assert!(!error.contains("mime"));
|
||||
assert!(!error.contains("v1 unsupported"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn background_summary_failure_stores_safe_copy_only_for_unsupported_content() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let (_registry, archive_path, _auth_path) = make_test_registry(&dir);
|
||||
let entry = make_test_entry(&archive_path);
|
||||
add_summary_test_artifact(
|
||||
&archive_path,
|
||||
entry.id,
|
||||
"raw/private-video.mp4",
|
||||
"primary_media",
|
||||
"video/mp4",
|
||||
b"video fixture",
|
||||
);
|
||||
let unsupported = summarizer::build_summary_input(
|
||||
&archive::read_archive_paths(&archive_path).unwrap(),
|
||||
&entry.entry_uid,
|
||||
summarizer::SummaryBuildOptions::default(),
|
||||
)
|
||||
.unwrap_err();
|
||||
assert_eq!(
|
||||
summary_failure_error_text(&unsupported),
|
||||
summarizer::UNSUPPORTED_SUMMARY_CONTENT_MESSAGE
|
||||
);
|
||||
|
||||
let conn = database::open_or_initialize(&archive_path).unwrap();
|
||||
let summary_uid = database::upsert_pending_entry_summary(
|
||||
&conn,
|
||||
entry.id,
|
||||
"codex_cli",
|
||||
Some("test"),
|
||||
summarizer::PROMPT_VERSION,
|
||||
"unsupported-content-test",
|
||||
)
|
||||
.unwrap();
|
||||
drop(conn);
|
||||
record_background_summary_failure(&archive_path, &summary_uid, &unsupported);
|
||||
let conn = database::open_or_initialize(&archive_path).unwrap();
|
||||
let stored = database::get_entry_summary_by_uid(&conn, &summary_uid)
|
||||
.unwrap()
|
||||
.unwrap();
|
||||
assert_eq!(stored.status, "failed");
|
||||
assert_eq!(
|
||||
stored.error_text.as_deref(),
|
||||
Some(summarizer::UNSUPPORTED_SUMMARY_CONTENT_MESSAGE)
|
||||
);
|
||||
|
||||
let provider = anyhow::anyhow!("provider response was malformed");
|
||||
assert_eq!(
|
||||
summary_failure_error_text(&provider),
|
||||
"provider response was malformed"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn summary_include_images_uses_distinct_cache_identity() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
|
|
|
|||
|
|
@ -15,6 +15,9 @@ const SUMMARY_PROVIDERS = [
|
|||
const PROVIDER_LABEL = Object.fromEntries(SUMMARY_PROVIDERS.map(p => [p.value, p.label]))
|
||||
const SUMMARY_PROVIDER_KEY = 'archivr:summary:provider'
|
||||
const SUMMARY_POLL_MS = 1500
|
||||
const UNSUPPORTED_SUMMARY_CONTENT_HEADING = 'This entry can’t be summarized yet.'
|
||||
const UNSUPPORTED_SUMMARY_CONTENT_DETAIL = 'It doesn’t contain archived text that a summary provider can read. Summaries currently support text notes, web pages, X posts and threads, and X Articles. Video, audio, and image-only entries need a transcript or text source.'
|
||||
const UNSUPPORTED_SUMMARY_CONTENT_MESSAGE = `${UNSUPPORTED_SUMMARY_CONTENT_HEADING}\n\n${UNSUPPORTED_SUMMARY_CONTENT_DETAIL}`
|
||||
|
||||
// Summaries are stored as the raw JSON string the model produced (normalized
|
||||
// server-side to {tldr, summary, tags}). Parsing can still fail for rows written
|
||||
|
|
@ -566,6 +569,9 @@ export default function ContextRail({ archiveId, selectedEntry, selectedUids, se
|
|||
? parseSummaryText(summary.summary_text)
|
||||
: null
|
||||
const running = summary?.status === 'pending' || summary?.status === 'running'
|
||||
const unsupportedContent =
|
||||
(summary?.status === 'failed' && summary.error_text === UNSUPPORTED_SUMMARY_CONTENT_MESSAGE) ||
|
||||
summaryError === UNSUPPORTED_SUMMARY_CONTENT_MESSAGE
|
||||
if (isPublicSession && !parsed) return null
|
||||
return (
|
||||
<div className="rail-section rail-summary">
|
||||
|
|
@ -596,13 +602,19 @@ export default function ContextRail({ archiveId, selectedEntry, selectedUids, se
|
|||
</p>
|
||||
)}
|
||||
|
||||
{summary?.status === 'failed' && summary.error_text && !isPublicSession && (
|
||||
<p className="form-msg form-msg--err" style={{ margin: '0 0 8px' }}>
|
||||
{unsupportedContent && !isPublicSession && (
|
||||
<div className="rail-summary-info" role="status">
|
||||
<p className="rail-summary-info__heading">{UNSUPPORTED_SUMMARY_CONTENT_HEADING}</p>
|
||||
<p className="rail-summary-info__detail">{UNSUPPORTED_SUMMARY_CONTENT_DETAIL}</p>
|
||||
</div>
|
||||
)}
|
||||
{summary?.status === 'failed' && summary.error_text && !unsupportedContent && !isPublicSession && (
|
||||
<p className="form-msg form-msg--err rail-summary-error">
|
||||
{summary.error_text}
|
||||
</p>
|
||||
)}
|
||||
{summaryError && (
|
||||
<p className="form-msg form-msg--err" style={{ margin: '0 0 8px' }}>
|
||||
{summaryError && !unsupportedContent && (
|
||||
<p className="form-msg form-msg--err rail-summary-error">
|
||||
{summaryError}
|
||||
</p>
|
||||
)}
|
||||
|
|
|
|||
|
|
@ -3262,6 +3262,26 @@ body.has-audio-bar { padding-bottom: 56px; }
|
|||
font-size: 12.5px;
|
||||
color: var(--muted);
|
||||
}
|
||||
.rail-summary-info {
|
||||
margin: 0 0 8px;
|
||||
padding: 8px;
|
||||
color: var(--muted);
|
||||
background: var(--paper-2);
|
||||
border: 1px solid var(--line-soft);
|
||||
border-radius: 4px;
|
||||
}
|
||||
.rail-summary-info__heading {
|
||||
margin: 0 0 4px;
|
||||
color: var(--ink);
|
||||
font-size: 12.5px;
|
||||
font-weight: 600;
|
||||
}
|
||||
.rail-summary-info__detail {
|
||||
margin: 0;
|
||||
font-size: 12px;
|
||||
line-height: 1.45;
|
||||
}
|
||||
.rail-summary-error { margin: 0 0 8px; }
|
||||
.rail-summary-spinner {
|
||||
width: 11px; height: 11px;
|
||||
border: 1.5px solid var(--line);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue