fix(guard): auto-clear the per-turn gate on a genuine preflight miss (v5.0.1) - #184
Conversation
When the vault was searched and nothing scored as relevant to the turn's task, the gate used to stay armed and force a manual search-vault + recall-note round trip on a note that, by construction, isn't relevant — the "read any note to dodge the gate" failure retrieval was built to prevent, one layer up. Auto-clear instead (always logged, never silent); OMI_GATE_MISS_STRICT=1 restores the old behavior. Co-Authored-By: Claude Sonnet 5 <[email protected]>
Co-Authored-By: Claude Sonnet 5 <[email protected]>
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe OMI consult gate now auto-clears after a non-empty-task relevance miss, logs the decision, and preserves strict behavior through ChangesConsult gate behavior
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Agent
participant preflight_turn
participant retrieve.relevant_titles
participant compliance
Agent->>preflight_turn: submit task
preflight_turn->>retrieve.relevant_titles: search for relevant titles
retrieve.relevant_titles-->>preflight_turn: no relevant matches
preflight_turn->>compliance: log auto-clear decision
preflight_turn-->>Agent: proceed without forced read
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Line 10: Insert a blank line immediately after the “### Changed” heading in
CHANGELOG.md, before the following list content, to satisfy Markdownlint MD022.
In `@src/omind/guard.py`:
- Around line 1297-1299: Update preflight_turn to normalize the captured task by
trimming whitespace before the empty-task miss check, so whitespace-only tasks
remain strict and cannot trigger auto-clear. Add a regression test covering a
task containing only whitespace.
- Line 1322: Update the compliance log detail construction near the task guard
to stop persisting any raw task content; replace task[:120] with a fixed reason
and only non-sensitive metadata such as task length or a one-way hash,
preserving the existing compliance logging flow.
- Around line 60-68: Revise the auto-clear audit wording to describe compliance
logging as best-effort because compliance.log_event may discard records on
OSError: update the explanation in src/omind/guard.py lines 60-68, document in
README.md lines 274-281 that logging is attempted but may be unavailable, and
apply the same best-effort wording to the release note in CHANGELOG.md lines
18-20.
- Around line 1313-1314: Update the condition in the task retrieval flow around
record_consult so it auto-clears only when titles is explicitly an empty result
([]), not when retrieval returns None. Preserve the older armed-gate fallback
for None retrieval failures, and add a regression test covering that failure
path.
In `@tests/test_guard.py`:
- Around line 507-515: Add the pytest monkeypatch fixture to
test_turn_preflight_without_match_auto_clears_gate and delete
guard.MISS_STRICT_ENV from the environment with raising=False before invoking
guard.preflight_turn, ensuring the test always exercises default mode.
- Around line 518-520: Update the assertions in the test around
compliance.read_events() to select the event belonging to the current session
and guard.GATE_NO_MATCH_RULE before checking its outcome. Replace the
order-dependent events[-1] access with session- and rule-filtered event
selection, then assert that the selected event has outcome "auto-clear".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e7e918ea-a650-4eb8-a8de-4ea636d469a3
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
CHANGELOG.mdREADME.mdpyproject.tomlsrc/omind/__init__.pysrc/omind/guard.pytests/test_guard.py
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: test (windows-latest, 3.10)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Retrieval must fail open: every search layer returnsNoneon errors and callers fall back to the older path. Missing models, corrupt indexes, locked databases, and missing FTS5 support must degrade search rather than break it; failure branches must be tested.
All note writes must go throughOmiStore; external writers usenotes.upsert_note. Writes must retain flocking, atomic rename, LamportRev:stamping, and soft deletion. Deletes archive notes withDisabled: true; onlyomind mesh purgepermanently removes them.
OmiStore.safe_namemust guard every note read and write so path traversal remains impossible.
Credential notes must be de-prioritized in search and gate suggestions unless the query concerns credentials, using_CREDENTIAL_PENALTY; the gate must never steer agents toward secrets notes.
MCP tools must never return unbounded output. Every list-shaped tool must paginate throughserver._pageand exposelimit,offset,total, andhas_more.
index.mdandMemory Template.mdare scaffolding, not memories; reading either must not clear the consult gate. Preservepaths.NON_CONSULT_FILENAMES.
Recency is a ranking leg only: it may re-rank notes already matched by content legs but must never add unmatched notes to search results.
link_targets()must preserve wikilink case for dangling-link reports; only link resolution lowercases it.
Do not mutate aNoteSummaryreturned from_cached_summary; it is shared cached state. Usedataclasses.replace, as instore._indexed_search.
Treatembed.encoderesults as potentially plain lists in tests; coerce vectors throughsearchindex._query_vectorrather than assuming a.shapeattribute.
Do not reintroduce document-frequency query-term filtering; use threshold-free graded matching, where all-words matches rank above any-words matches.
Files:
src/omind/__init__.pysrc/omind/guard.pytests/test_guard.py
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Markdown note files are the source of truth; indexes, caches, and vectors are derived and disposable and must live only in
paths.state_dir(), never in the vault.
Files:
CHANGELOG.mdREADME.md
README.md
📄 CodeRabbit inference engine (AGENTS.md)
The README must use the centered banner version of the project footer rather than the standard documentation footer.
Files:
README.md
🪛 markdownlint-cli2 (0.23.0)
CHANGELOG.md
[warning] 10-10: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🔇 Additional comments (5)
pyproject.toml (1)
3-3: LGTM!src/omind/__init__.py (1)
5-5: LGTM!src/omind/guard.py (1)
69-71: LGTM!Also applies to: 1283-1284
tests/test_guard.py (1)
18-18: LGTM!Also applies to: 523-544
CHANGELOG.md (1)
11-17: LGTM!Also applies to: 21-24
|
|
||
| ## [Unreleased] | ||
|
|
||
| ### Changed |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a blank line after the heading.
Markdownlint MD022 flags ### Changed because the list begins immediately below it.
Proposed fix
### Changed
+
- **A genuine preflight miss auto-clears📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ### Changed | |
| ### Changed | |
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 10-10: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CHANGELOG.md` at line 10, Insert a blank line immediately after the “###
Changed” heading in CHANGELOG.md, before the following list content, to satisfy
Markdownlint MD022.
Source: Linters/SAST tools
| #: Opt back into the old strict behavior: a preflight MISS (the vault was | ||
| #: searched and nothing scored as relevant to the turn's task) forces a manual | ||
| #: search-vault/recall-note round trip instead of auto-clearing the gate. Default | ||
| #: off, mirroring OMI_VERIFY_REQUIRE's "cheap default, opt-in strictness" shape — | ||
| #: forcing a consult of a note that is, by construction, not relevant to the task | ||
| #: is the exact "read any note to dodge the gate" failure retrieval was built to | ||
| #: prevent; it just reappears one layer up when the vault genuinely has nothing | ||
| #: on-topic. The auto-clear is always logged to the compliance log (never silent) | ||
| #: so a session that's mostly missing is visible in `omind guard log`. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Align the audit guarantee with the best-effort logger.
compliance.log_event can silently discard the record on OSError, so these descriptions overstate auditability. Qualify the behavior as best-effort or make logging failure observable before claiming the auto-clear is never silent.
src/omind/guard.py#L60-L68: change “always logged”/“never silent” to best-effort wording.README.md#L274-L281: document that logging is attempted and may be unavailable.CHANGELOG.md#L18-L20: use the same best-effort wording in the release note.
📍 Affects 3 files
src/omind/guard.py#L60-L68(this comment)README.md#L274-L281CHANGELOG.md#L18-L20
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/omind/guard.py` around lines 60 - 68, Revise the auto-clear audit wording
to describe compliance logging as best-effort because compliance.log_event may
discard records on OSError: update the explanation in src/omind/guard.py lines
60-68, document in README.md lines 274-281 that logging is attempted but may be
unavailable, and apply the same best-effort wording to the release note in
CHANGELOG.md lines 18-20.
| An EMPTY task (nothing captured to search with) is not a miss — it means we | ||
| never ran the search at all, so it stays strict; we can't judge "nothing is | ||
| relevant" without having looked. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Treat whitespace-only tasks as empty.
preflight_turn uses the raw captured string, so a task such as " " is treated as non-empty and can trigger auto-clear despite this documentation promising that empty tasks remain strict. Normalize the task before the miss check and add a whitespace-only regression test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/omind/guard.py` around lines 1297 - 1299, Update preflight_turn to
normalize the captured task by trimming whitespace before the empty-task miss
check, so whitespace-only tasks remain strict and cannot trigger auto-clear. Add
a regression test covering a task containing only whitespace.
| if task and not titles and not _miss_strict(): | ||
| record_consult(session, kind="no-match", target="", relevant=False) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Distinguish retrieval errors from a successful empty result.
not titles conflates [] (a clean relevance miss) with None (a retrieval failure under the repository contract). Missing models, corrupt indexes, locked databases, or unavailable FTS5 could therefore be logged as no-match and auto-clear the gate instead of following the older armed-gate fallback. Only auto-clear on an explicit empty result and add a regression test for the error path.
Proposed condition
- if task and not titles and not _miss_strict():
+ if task and titles == [] and not _miss_strict():As per coding guidelines, retrieval errors must return None, callers must fall back to the older path, and failure branches must be tested.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if task and not titles and not _miss_strict(): | |
| record_consult(session, kind="no-match", target="", relevant=False) | |
| if task and titles == [] and not _miss_strict(): | |
| record_consult(session, kind="no-match", target="", relevant=False) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/omind/guard.py` around lines 1313 - 1314, Update the condition in the
task retrieval flow around record_consult so it auto-clears only when titles is
explicitly an empty result ([]), not when retrieval returns None. Preserve the
older armed-gate fallback for None retrieval failures, and add a regression test
covering that failure path.
Source: Coding guidelines
| rule_id=GATE_NO_MATCH_RULE, | ||
| severity="soft", | ||
| outcome="auto-clear", | ||
| detail=f"task={task[:120]!r}", |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not persist the raw task in the compliance log.
detail is written to compliance.jsonl; truncating to 120 characters does not prevent prompts containing credentials, tokens, emails, or other user data from being retained. Log a fixed reason plus non-sensitive metadata such as length or a hash instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/omind/guard.py` at line 1322, Update the compliance log detail
construction near the task guard to stop persisting any raw task content;
replace task[:120] with a fixed reason and only non-sensitive metadata such as
task length or a one-way hash, preserving the existing compliance logging flow.
| def test_turn_preflight_without_match_auto_clears_gate(tmp_path: Path) -> None: | ||
| omi = tmp_path / "OMI" | ||
| omi.mkdir() | ||
| context = guard.preflight_turn( | ||
| {"session_id": "preflight-none", "prompt": "unmatched subject"}, | ||
| omi, | ||
| ) | ||
| assert "found nothing relevant" in context | ||
| assert guard.MISS_STRICT_ENV in context |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Isolate the default-mode test from the ambient environment.
This test assumes OMI_GATE_MISS_STRICT is unset, but does not remove it. A developer or CI process with that variable exported will make the default auto-clear test fail. Add monkeypatch.delenv(guard.MISS_STRICT_ENV, raising=False).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_guard.py` around lines 507 - 515, Add the pytest monkeypatch
fixture to test_turn_preflight_without_match_auto_clears_gate and delete
guard.MISS_STRICT_ENV from the environment with raising=False before invoking
guard.preflight_turn, ensuring the test always exercises default mode.
| events = compliance.read_events() | ||
| assert events[-1]["rule_id"] == guard.GATE_NO_MATCH_RULE | ||
| assert events[-1]["outcome"] == "auto-clear" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Assert against this session’s event, not the global last event.
compliance.read_events() reads the shared JSONL log, so events[-1] is order-dependent and can inspect another test’s event under parallel execution. Filter by session and rule ID before asserting the outcome.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_guard.py` around lines 518 - 520, Update the assertions in the
test around compliance.read_events() to select the event belonging to the
current session and guard.GATE_NO_MATCH_RULE before checking its outcome.
Replace the order-dependent events[-1] access with session- and rule-filtered
event selection, then assert that the selected event has outcome "auto-clear".
1 participant
Summary
search-vault+recall-noteround trip on a note that, by construction, isn't relevant — the "read any note to dodge the gate" failure the relevance mapping was built to prevent, one layer up. Auto-clear instead (always logged to the compliance log, never silent);OMI_GATE_MISS_STRICT=1restores the old forced-consult behavior. An empty/uncaptured task still leaves the gate armed since that's "we never looked," not "we looked and found nothing."pyproject.toml/__init__.pyto 5.0.1.Test plan
./scripts/test.sh -q— 840 passed, 1 skippedruff checkcleanmypy src/omind/guard.pycleanOMI_GATE_MISS_STRICT=1opt-out, empty-task stays strict🤖 Generated with Claude Code