mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-30 23:14:37 +00:00
Fix UTF-8 body_preview slicing in memory ingest (#1620)
Co-authored-by: honor2030 <19909783+honor2030@users.noreply.github.com> Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>
This commit is contained in:
co-authored by
honor2030
Steven Enamakel
parent
8ae921dd68
commit
4870bc2078
@@ -26,6 +26,8 @@ use crate::openhuman::memory::tree::types::SourceKind;
|
||||
use crate::openhuman::memory::tree::util::redact::redact;
|
||||
use std::time::{SystemTime, UNIX_EPOCH};
|
||||
|
||||
const BODY_PREVIEW_MAX_BYTES: usize = 2048;
|
||||
|
||||
/// Outcome of one ingest call.
|
||||
#[derive(Clone, Debug, Serialize, Deserialize)]
|
||||
pub struct IngestResult {
|
||||
@@ -150,16 +152,6 @@ async fn already_ingested(
|
||||
.map_err(|e| anyhow::anyhow!("already_ingested join error: {e}"))?
|
||||
}
|
||||
|
||||
/// Build a trailing body preview (last ~2048 bytes), safe for multibyte UTF-8.
|
||||
fn build_body_preview(md: &str) -> String {
|
||||
let len = md.len();
|
||||
if len <= 2048 {
|
||||
return md.to_string();
|
||||
}
|
||||
let start = crate::openhuman::util::floor_char_boundary(md, len - 2048);
|
||||
md[start..].to_string()
|
||||
}
|
||||
|
||||
async fn persist(
|
||||
config: &Config,
|
||||
source_id: &str,
|
||||
@@ -168,12 +160,14 @@ async fn persist(
|
||||
let source_kind_for_store = canonical.metadata.source_kind;
|
||||
|
||||
// Capture body_preview before the canonical markdown is moved into the chunker.
|
||||
// For email and document sources: last ≤ 2 048 chars of the canonical markdown
|
||||
// are enough for signature parsing and similar lightweight subscribers. Chat
|
||||
// sources are conversational and have no trailing structure worth scanning, so
|
||||
// they get body_preview = None.
|
||||
// For email and document sources: the trailing canonical markdown, capped at
|
||||
// 2 048 bytes, is enough for signature parsing and similar lightweight
|
||||
// subscribers. Chat sources are conversational and have no trailing structure
|
||||
// worth scanning, so they get body_preview = None.
|
||||
let body_preview: Option<String> = match source_kind_for_store {
|
||||
SourceKind::Email | SourceKind::Document => Some(build_body_preview(&canonical.markdown)),
|
||||
SourceKind::Email | SourceKind::Document => {
|
||||
Some(markdown_body_preview(&canonical.markdown))
|
||||
}
|
||||
_ => None,
|
||||
};
|
||||
|
||||
@@ -363,6 +357,22 @@ async fn persist(
|
||||
})
|
||||
}
|
||||
|
||||
/// Returns the trailing slice of `md` capped at [`BODY_PREVIEW_MAX_BYTES`] bytes.
|
||||
///
|
||||
/// Uses `ceil_char_boundary` (rounds the cut point *forward*) so the returned
|
||||
/// slice is always `<= BODY_PREVIEW_MAX_BYTES` bytes — `floor_char_boundary`
|
||||
/// (rounds backward) can return up to 3 extra bytes when the cut falls inside
|
||||
/// a multi-byte codepoint, violating the hard cap.
|
||||
fn markdown_body_preview(md: &str) -> String {
|
||||
let len = md.len();
|
||||
if len <= BODY_PREVIEW_MAX_BYTES {
|
||||
md.to_string()
|
||||
} else {
|
||||
let start = md.ceil_char_boundary(len - BODY_PREVIEW_MAX_BYTES);
|
||||
md[start..].to_string()
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -487,6 +497,41 @@ mod tests {
|
||||
assert_eq!(count_scores(&cfg).unwrap(), 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn markdown_body_preview_respects_utf8_boundary_and_byte_cap() {
|
||||
let md = format!("{}{}{}\n", "a".repeat(17), '\u{200c}', "b".repeat(2045));
|
||||
let requested_start = md.len() - BODY_PREVIEW_MAX_BYTES;
|
||||
assert!(
|
||||
!md.is_char_boundary(requested_start),
|
||||
"test fixture must put the requested preview boundary inside a multi-byte character"
|
||||
);
|
||||
|
||||
let preview = markdown_body_preview(&md);
|
||||
|
||||
assert!(preview.len() <= BODY_PREVIEW_MAX_BYTES);
|
||||
assert_eq!(preview, format!("{}\n", "b".repeat(2045)));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn ingest_document_handles_utf8_at_body_preview_boundary() {
|
||||
let (_tmp, cfg) = test_config();
|
||||
let body = format!("{}{}{}", "a".repeat(17), '\u{200c}', "b".repeat(2045));
|
||||
|
||||
let doc = DocumentInput {
|
||||
provider: "notion".into(),
|
||||
title: "Unicode boundary".into(),
|
||||
body,
|
||||
modified_at: Utc.timestamp_millis_opt(1_700_000_000_000).unwrap(),
|
||||
source_ref: Some("notion://page/unicode-boundary".into()),
|
||||
};
|
||||
|
||||
let out = ingest_document(&cfg, "notion:utf8-boundary", "alice", vec![], doc)
|
||||
.await
|
||||
.unwrap();
|
||||
assert!(!out.already_ingested);
|
||||
assert!(out.chunks_written >= 1);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn second_ingest_document_with_same_source_id_is_short_circuited() {
|
||||
let (_tmp, cfg) = test_config();
|
||||
@@ -543,32 +588,4 @@ mod tests {
|
||||
assert_eq!(count_chunks(&cfg).unwrap(), 1);
|
||||
assert_eq!(count_scores(&cfg).unwrap(), 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn body_preview_short_string_returned_whole() {
|
||||
let short = "Hello world";
|
||||
assert_eq!(super::build_body_preview(short), short);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn body_preview_long_ascii_truncates_to_trailing_bytes() {
|
||||
let long = "A".repeat(4096);
|
||||
let preview = super::build_body_preview(&long);
|
||||
assert_eq!(preview.len(), 2048); // ASCII has no multibyte rounding
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn body_preview_multibyte_at_cut_point_does_not_panic() {
|
||||
// U+200C (ZWNJ) is 3 bytes: E2 80 8C
|
||||
// Construct so len - 2048 lands inside the 3-byte codepoint.
|
||||
let prefix = "X".repeat(2045);
|
||||
let zwnj = "\u{200C}"; // 3 bytes at positions 2045..2048
|
||||
let suffix = "Y".repeat(2046); // total = 2045 + 3 + 2046 = 4094; len-2048 = 2046 → inside ZWNJ
|
||||
let input = format!("{}{}{}", prefix, zwnj, suffix);
|
||||
assert_eq!(input.len(), 4094);
|
||||
// This is the exact regression case: len - 2048 = 2046, byte index inside ZWNJ
|
||||
let preview = super::build_body_preview(&input); // must not panic
|
||||
assert!(preview.is_char_boundary(0));
|
||||
assert!(preview.len() >= 2046); // at least the suffix
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user