Repository navigation
fix: scope the dropped-system-message hint to retrieval memories - #91
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Pardon the interruption, but your automated checks are ready! 🛎️I've aggregated the results of the automated checks for this PR below. 🔍 LintChecking the boxes and crossing the T's. 🖋️ ❌ ruff: issues found in 18 file(s) — see job log ⚖️ License CheckVerifying the source of all binary files. 💾 ✅ No license violations found. Policy: Apache 2.0 (universal donor). StrongCopyleft / NetworkCopyleft / WeakCopyleft / Other / Error categories fail. MPL allowed. 🔌 Skill Tests (ovoscope)Checking if the skill meets our quality standards. ✅ ✅ 3/3 passed ✅ TestOpenAIPerSessionMemory — 1/1 🚌 Bus CoverageIs the message bus fully covered? Let's see the stats. 📊 🔨 Build TestsI've put your code through the build grinder. ☕ ✅ All versions pass
📊 CoverageScanning for any 'untested' alerts! 🚨 ✅ 89.3% total coverage Per-file coverage (8 files)
Full report: download the A robot's work is never done... but this PR check is! ⚙️ |
The drop warning told the user to set the memory plugin's 'inject_mode' to one this engine keeps. Only the retrieval memories read inject_mode (BaseRetrievalMemory in ovos-memory-plugins, and the rag memory in this package). The entity and longterm memories always send a system message and have no such option, so the advice sent their users to a setting that does not exist. The hint now names 'allow_system_prompts' first and limits the inject_mode advice to retrieval memories, with the value 'user'. Before the change the new test fails on the old text; after it, all 81 tests pass. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dc5716b to
9b29e4c
Compare
The warning for a dropped system message ends with advice to set the memory plugin's 'inject_mode'. That advice is wrong for the entity and longterm memories. They have no such setting. It is also incomplete for the retrieval memories.
Only the retrieval memories read 'inject_mode': lexical, local-rag and composite passages (ovos-memory-plugins 1.2.0a3, base.py:91, class BaseRetrievalMemory), and the rag memory in this package (ovos_openai_plugin/rag_memory.py:121). Entity and longterm always send a system message (entity.py:184-188, longterm.py:238). A composite passes their blocks as system messages whatever its own inject_mode is (composite.py:173-186).
A memory 'system_prompt' still goes out as a system message in every mode except 'system_prompt' (base.py:246-249, rag_memory.py:211-212). So 'inject_mode: user' alone keeps the warning. This engine adds its own 'system_prompt' after the drop (chat.py:89-99), so the working fix is to move the prompt there.
The warning now ends: "set 'allow_system_prompts', or, for a retrieval memory (lexical, local-rag, the passages of a composite, or ovos-openai-rag-memory-plugin), set its 'inject_mode' to 'user' and move its 'system_prompt' to this engine's 'system_prompt'."
The new test asserts that text. It fails on the old text and passes on the new text. The full suite gives 81 passed. The README and docs held no copy of the old advice.
🤖 Generated with Claude Code