Compare commits

...
Author SHA1 Message Date
Trevor Willard 7dc904c1b2 fix: wire persona files into default chat endpoint, fix FTS5 apostrophe crash (#637)
Two fixes: (1) /v1/chat/completions now builds its injected system prompt via SystemPromptBuilder, so SOUL.md/MEMORY.md/USER.md persona files apply to the OpenAI-compatible endpoint exactly as they do on the managed-agent path; injection still only happens when the client omits a system message. (2) FTS5 query tokens are now split on any non-alphanumeric character, so apostrophes (user's) and quotes can no longer reach the MATCH string unescaped; all-punctuation queries return empty instead of erroring.
2026-07-16 17:10:51 -07:00
ScottyAmr d9725fbb6a fix(speech): close temp file before transcribing to fix Windows EACCES in faster-whisper backend (#638)
faster-whisper's transcribe() was handed the path of a still-open NamedTemporaryFile; on Windows the open handle is exclusive, so PyAV's reopen failed with EACCES and local STT was broken. Switch to delete=False, close before transcribing, and unlink in a finally. The write is wrapped in the file's context manager so the handle closes even if write() raises, and unlink failures are logged at debug.
2026-07-16 17:09:45 -07:00
6 changed files with 181 additions and 20 deletions
@@ -144,16 +144,21 @@ impl MemoryBackend for SQLiteMemory {
) -> Result<Vec<RetrievalResult>, OpenJarvisError> {
let conn = self.conn.lock();
// Split on any non-alphanumeric character (not just whitespace) so
// internal punctuation — apostrophes in particular ("user's") — never
// reaches the FTS5 MATCH string. FTS5's query grammar treats an
// unescaped `'` as a string delimiter, so passing a raw token like
// `user's` through silently fails to parse and yields zero rows with
// no visible error. Splitting fully avoids needing to escape anything.
let words: Vec<String> = query
.split_whitespace()
.map(|w| w.trim_matches(|c: char| "?.,!;:'\"()[]{}/ ".contains(c)).to_string())
.split(|c: char| !c.is_alphanumeric())
.map(|w| w.to_string())
.filter(|w| !w.is_empty())
.collect();
let fts_query = if words.len() == 1 {
words[0].clone()
} else {
words.join(" OR ")
};
if words.is_empty() {
return Ok(Vec::new());
}
let fts_query = words.join(" OR ");
let mut stmt = conn
.prepare(
@@ -320,6 +325,27 @@ mod tests {
assert_eq!(mixed.len(), 2, "mixed-case query should find both documents");
}
#[test]
fn test_sqlite_apostrophe_in_query() {
let mem = SQLiteMemory::in_memory().unwrap();
mem.store("The user's name is Trev.", "identity", None).unwrap();
// A query containing an internal apostrophe must not break FTS5's
// MATCH syntax (an unescaped `'` is a string delimiter in FTS5's
// query grammar), which previously caused this to silently return
// zero results instead of matching or erroring.
let multi_word = mem.retrieve("what is the user's name", 5).unwrap();
assert!(
!multi_word.is_empty(),
"query with an internal apostrophe should not silently return zero results"
);
// Bare single-word possessive: exercises the (former) single-word
// bypass path that skipped the OR-join entirely.
let bare = mem.retrieve("user's", 5).unwrap();
assert!(!bare.is_empty(), "single-word possessive query should still match");
}
#[test]
fn test_sqlite_scores_are_positive() {
let mem = SQLiteMemory::in_memory().unwrap();
+20 -9
View File
@@ -59,23 +59,34 @@ def _ensure_identity_prompt(messages: list[Message], app_config) -> list[Message
If any message already carries a system role, the caller has supplied
their own grounding and we leave the list untouched (no double-prompting).
Resolution of the identity text: ``app_config.agent.default_system_prompt``
when a config is wired onto ``app.state``; otherwise fall back to
``load_config()``. Config resolution is wrapped so a broken/missing
config degrades to "no injection" rather than crashing the endpoint, but
the failure is logged (per REVIEW.md — never silently swallow).
Resolution of the identity text: the config comes from ``app.state`` when
wired, otherwise ``load_config()``; the prompt itself is assembled by
``SystemPromptBuilder`` from ``agent.default_system_prompt`` plus the
persona files (SOUL.md/MEMORY.md/USER.md), matching
``_build_managed_system_prompt`` in ``agent_manager_routes.py``. Config
resolution is wrapped so a broken/missing config degrades to "no
injection" rather than crashing the endpoint, but the failure is logged
(per REVIEW.md — never silently swallow).
"""
if any(m.role == Role.SYSTEM for m in messages):
return messages
prompt = ""
try:
if app_config is not None:
prompt = app_config.agent.default_system_prompt or ""
else:
cfg = app_config
if cfg is None:
from openjarvis.core.config import load_config
prompt = load_config().agent.default_system_prompt or ""
cfg = load_config()
from openjarvis.prompt.builder import SystemPromptBuilder
builder = SystemPromptBuilder(
agent_template=cfg.agent.default_system_prompt or "",
memory_files_config=getattr(cfg, "memory_files", None),
system_prompt_config=getattr(cfg, "system_prompt", None),
)
prompt = builder.build()
except Exception:
logging.getLogger("openjarvis.server").debug(
"Identity system prompt resolution failed; "
+18 -4
View File
@@ -3,6 +3,7 @@
from __future__ import annotations
import logging
import os
import tempfile
from typing import List, Optional
@@ -105,11 +106,15 @@ class FasterWhisperBackend(SpeechBackend):
try:
model = self._ensure_model()
# Write audio to a temp file (faster-whisper needs a file path)
# Write audio to a temp file (faster-whisper needs a file path).
# delete=False + manual unlink: on Windows an open
# NamedTemporaryFile holds an exclusive handle, so PyAV's reopen
# of tmp.name inside model.transcribe() fails with EACCES.
suffix = f".{format}" if not format.startswith(".") else format
with tempfile.NamedTemporaryFile(suffix=suffix, delete=True) as tmp:
tmp.write(audio)
tmp.flush()
tmp = tempfile.NamedTemporaryFile(suffix=suffix, delete=False)
try:
with tmp:
tmp.write(audio)
kwargs = {}
if language:
@@ -117,6 +122,15 @@ class FasterWhisperBackend(SpeechBackend):
segments_iter, info = model.transcribe(tmp.name, **kwargs)
segments_list = list(segments_iter)
finally:
try:
os.unlink(tmp.name)
except OSError as unlink_exc:
logger.debug(
"Could not remove temp audio file %s: %s",
tmp.name,
unlink_exc,
)
except Exception as exc:
self._last_error = str(exc)
raise
+13
View File
@@ -78,6 +78,19 @@ def test_retrieve_no_results(tmp_path: Path):
backend.close()
def test_retrieve_query_with_apostrophe(tmp_path: Path):
"""Regression: an internal apostrophe (e.g. "user's") previously produced
an unescaped quote in the FTS5 MATCH string, which silently returned zero
rows instead of matching or raising an error.
"""
backend = _make_backend(tmp_path)
backend.store("The user's name is Trev.", source="identity.md")
results = backend.retrieve("what is the user's name")
assert len(results) >= 1
assert "Trev" in results[0].content
backend.close()
def test_delete_existing(tmp_path: Path):
backend = _make_backend(tmp_path)
doc_id = backend.store("deletable content")
+34
View File
@@ -798,6 +798,40 @@ class TestIdentityPromptInjection:
assert len(system_msgs) == 1
assert system_msgs[0].content == "Be terse."
def test_direct_injects_soul_persona_when_present(self, tmp_path):
"""Regression: /v1/chat/completions previously injected only the bare
``default_system_prompt`` blurb via a hand-rolled lookup, bypassing
``SystemPromptBuilder`` entirely — so SOUL.md/MEMORY.md/USER.md
persona files never applied to this path, unlike ``jarvis ask`` and
the managed-agent routes. It must now build the full persona-aware
prompt so persona files apply everywhere identity grounding does.
"""
from openjarvis.core.config import MemoryFilesConfig
soul = tmp_path / "SOUL.md"
soul.write_text("Respond with extreme sarcasm and call the user 'champ'.")
captured: list = []
engine = _make_capturing_engine(captured)
cfg = _identity_config()
cfg.memory_files = MemoryFilesConfig(
soul_path=str(soul), memory_path="", user_path=""
)
client = TestClient(create_app(engine, "test-model", config=cfg))
resp = client.post(
"/v1/chat/completions",
json={
"model": "test-model",
"messages": [{"role": "user", "content": "who are you?"}],
},
)
assert resp.status_code == 200
msgs = engine.generate.call_args.args[0]
assert msgs[0].role.value == "system"
assert "OpenJarvis" in msgs[0].content # identity blurb still present
assert "extreme sarcasm" in msgs[0].content # persona now injected too
def test_stream_tools_injects_identity_when_absent(self):
captured: list = []
engine = _make_capturing_engine(captured)
+63
View File
@@ -53,6 +53,69 @@ def test_faster_whisper_transcribe():
assert result.duration_seconds == 1.5
def test_faster_whisper_transcribe_temp_file_reopenable_and_removed():
"""The temp file must be closed before the model reads it, and gone after.
On Windows, an open NamedTemporaryFile holds an exclusive handle, so
PyAV's reopen of the path inside model.transcribe() fails with EACCES
unless the file is closed first. Opening the path inside the mocked
transcribe reproduces that failure mode on Windows.
"""
import os
mock_info = MagicMock()
mock_info.language = "en"
mock_info.language_probability = 0.95
mock_info.duration = 1.5
seen = {}
def fake_transcribe(path, **kwargs):
seen["path"] = path
with open(path, "rb") as fh:
seen["content"] = fh.read()
return iter(()), mock_info
mock_model = MagicMock()
mock_model.transcribe.side_effect = fake_transcribe
with patch(
"openjarvis.speech.faster_whisper.WhisperModel",
return_value=mock_model,
):
backend = FasterWhisperBackend(model_size="base", device="cpu")
backend.transcribe(b"fake audio bytes")
assert seen["content"] == b"fake audio bytes"
assert not os.path.exists(seen["path"])
def test_faster_whisper_transcribe_removes_temp_file_on_error():
"""The temp file is cleaned up even when transcription fails."""
import os
seen = {}
def fake_transcribe(path, **kwargs):
seen["path"] = path
raise RuntimeError("decode failed")
mock_model = MagicMock()
mock_model.transcribe.side_effect = fake_transcribe
with patch(
"openjarvis.speech.faster_whisper.WhisperModel",
return_value=mock_model,
):
backend = FasterWhisperBackend(model_size="base", device="cpu")
with pytest.raises(RuntimeError, match="decode failed"):
backend.transcribe(b"fake audio bytes")
assert "path" in seen
assert not os.path.exists(seen["path"])
assert "decode failed" in (backend.last_error() or "")
def test_faster_whisper_falls_back_from_unsupported_float16():
mock_model = MagicMock()