fix(memory): stop extract_facts panicking on non-ASCII input#635
Open
harrythomasben wants to merge 1 commit into
Open
fix(memory): stop extract_facts panicking on non-ASCII input#635harrythomasben wants to merge 1 commit into
harrythomasben wants to merge 1 commit into
Conversation
The explicit-"remember" fast-path in extract_facts guarded a byte-index slice (trimmed[..8]) with only a byte-length check (trimmed.len() >= 8). len() counts bytes, so for a 14-byte non-ASCII utterance the guard passed and trimmed[..8] sliced at byte 8 — inside a multi-byte UTF-8 char — and panicked, taking down fact extraction for any non-Latin-script household. Replace the unsound guard with a UTF-8-safe starts_with_ascii_ci helper (str::get returns None on a non-boundary instead of panicking) and use it at both the call-site guard and the twin slice in extract_remember. Fixes GeniePod#634
matedev01
approved these changes
Jul 6, 2026
matedev01
left a comment
Contributor
There was a problem hiding this comment.
LGTM — fixes non-ASCII panic in extract_facts. Needs rebase to resolve merge conflict (unrelated CHANGELOG clash). 826 lib, BFCL 96% after rebase.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #634.
memory::extract::extract_factspanics on non-ASCII input: the explicit-"remember" fast-path guards a byte-index slice with only a byte-length check, so any utterance whose 8th byte falls inside a multi-byte UTF-8 char slices on a non-char-boundary and crashes.extract_factsruns on raw chat, voice-transcript, and profile-document text, so the input is user-controlled — this takes down fact extraction for any non-Latin-script household.Changes
starts_with_ascii_ci(text, prefix)helper that usesstr::get(..prefix.len())(returnsNoneon a non-char-boundary or too-short input) instead of a byte-length guard plustext[..n]slice.extract_facts(wastrimmed.len() >= 8 && trimmed[..8].eq_ignore_ascii_case("remember")).extract_remember(text[..PREFIX.len()]), which had the same unsound pattern.tests/extract_facts_utf8_test.rs: regression tests for CJK / accented / emoji input (no panic), short input, and the unchanged ASCII "remember" path.Behavior is unchanged for ASCII input; the only difference is that non-ASCII utterances no longer panic (they correctly fall through the "remember" fast-path).
Real Behavior Proof
Tested profile / hardware (check all that apply):
jetsonraspberry_piportable_sbclaptopmacWhat I ran
On an x86_64 Linux laptop (Rust 1.96):
"日本語hello":byte index 8 is not a char boundary; it is inside '語' (bytes 6..9 of string)(process aborted, exit 101).cargo test -p genie-core --test extract_facts_utf8_test→ 4 passed.cargo test -p genie-core extract→ 11 passed (existing extraction tests, no regression).cargo clippy -p genie-core --tests→ clean;cargo fmtapplied.This is a pure input-parsing panic with no hardware, voice, audio, or Home Assistant dependency — the deterministic
cargo testrepro exercises the full fix path, so Jetson verification is not applicable.What I observed
After the fix,
extract_facts("日本語hello")and the other non-ASCII inputs return normally (no "remember" fact, no panic), whileextract_facts("remember to call mom")and"Remember that ..."still extract the fact unchanged.Test plan
cargo test -p genie-core --test extract_facts_utf8_testcargo test -p genie-core extractto confirm existing extraction behavior is unchanged.