chore(mcp): remove deprecated graph aliases - #185
Conversation
Co-authored-by: Aaron K. Clark (CryptoJones) <cryptojones@owasp.org> Signed-off-by: Aaron K. Clark (CryptoJones) <cryptojones@owasp.org>
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR removes four deprecated graph MCP aliases and updates graph tests to use the unified ChangesGraph alias removal
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
| def run(omi_dir: Path | str, *, queries: tuple[str, ...] = SAMPLE_QUERIES) -> Report: | ||
| """Measure retrieval on ``omi_dir`` and return the report.""" | ||
| from omind import ai_usage, embed, hooks, recall, searchindex | ||
| from omind.server import build_server |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@AGENTS.md`:
- Around line 132-133: Reconcile the handoff status across AGENTS.md and
BACKLOG.md: update the decision list and the references around issues `#172`,
`#174`, and `#177` so only active decisions remain, and shipped items are marked
consistently with BACKLOG.md. Remove stale open-status references and do not
retain `#172` as a decision item if BACKLOG.md marks it done.
In `@CHANGELOG.md`:
- Around line 10-17: Add a blank line immediately after the “### Removed” and
“### Changed” headings in the changelog, keeping their existing list content
unchanged.
In `@README.md`:
- Line 408: Update the README command comment for omind bench to mention both
MCP measurements: the number of tools exposed and the MCP tool schema size,
while preserving the existing latency and token cost descriptions.
In `@tests/test_bench.py`:
- Around line 49-53: Update test_report_measures_the_reduced_mcp_surface so the
“MCP tool schemas” measurement is checked against the documented 2,114-token
baseline, using an explicitly documented tolerance or upper bound if dependency
variation requires it, instead of only asserting it is positive.
🪄 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: 6cb97b12-ed9e-447c-9844-9ca5819f87ae
📒 Files selected for processing (8)
AGENTS.mdBACKLOG.mdCHANGELOG.mdREADME.mdsrc/omind/bench.pysrc/omind/server.pytests/test_bench.pytests/test_server.py
📜 Review details
⏰ Context from checks skipped due to timeout. (10)
- GitHub Check: test (ubuntu-latest, 3.11)
- GitHub Check: test (ubuntu-latest, 3.14)
- GitHub Check: test (ubuntu-latest, 3.12)
- GitHub Check: test (macos-latest, 3.14)
- GitHub Check: test (ubuntu-latest, 3.10)
- GitHub Check: test (windows-latest, 3.14)
- GitHub Check: test (windows-latest, 3.10)
- GitHub Check: test (ubuntu-latest, 3.13)
- GitHub Check: test (macos-latest, 3.10)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{md,py}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{md,py}: Treat Markdown vault files as the source of truth; keep indexes, caches, vectors, and other derived data underpaths.state_dir(), never in the vault.
Documentation files must include the exact footer*Proudly Made in Nebraska. Go Big Red! 🌽 <https://xkcd.com/2347/>*; the README uses the centered banner variant.
Files:
BACKLOG.mdREADME.mdAGENTS.mdCHANGELOG.mdsrc/omind/server.pytests/test_bench.pytests/test_server.pysrc/omind/bench.py
**/*.{py,md}
📄 CodeRabbit inference engine (AGENTS.md)
Read and follow the retrieval design documented in
docs/retrieval.mdwhen changing retrieval behavior.
Files:
BACKLOG.mdREADME.mdAGENTS.mdCHANGELOG.mdsrc/omind/server.pytests/test_bench.pytests/test_server.pysrc/omind/bench.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Retrieval must fail open: every search layer returnsNoneon errors, allowing fallback to an older path. Test failure branches as well as successful searches.
Route all note writes throughOmiStore; external writers should usenotes.upsert_note. Preserve flocking, atomic rename, LamportRev:stamping, and soft deletion. Deletes must archive withDisabled: true; onlyomind mesh purgemay permanently remove notes.
UseOmiStore.safe_namefor 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 the credential penalty behavior. Never steer agents toward secrets notes.
MCP tools must never return unbounded output. Every list-shaped tool must paginate withlimit,offset,total, andhas_morethroughserver._page.
Treatindex.mdandMemory Template.mdas scaffolding rather than memories; reading them must not clear the consult gate. Preservepaths.NON_CONSULT_FILENAMES.
Recency is a ranking leg only: it may reorder notes matched by content legs but must never add otherwise-unmatched notes.
Do not replacelint.py's code-fence handling with search-index behavior; the index does not strip code fences from[[wikilinks]], while lint does.
link_targets()must preserve the author's link casing for dangling-link reports; only resolution may lowercase links.
Never mutate aNoteSummaryreturned from_cached_summary; usedataclasses.replaceinstead.
Coerce embedding results throughsearchindex._query_vector; do not assumeembed.encodereturns an object with.shape, because test backends may return a plain list.
The embedding extra may contacthuggingface.coon first use, but retrieval must fail open when offline.
Do not reintroduce document-frequency filtering of query terms; use threshold-free graded matching, where all-words matches rank above any-words matches.
Files:
src/omind/server.pytests/test_bench.pytests/test_server.pysrc/omind/bench.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: CryptoJones/omind
Timestamp: 2026-08-01T04:53:58.135Z
Learning: Do not commit or push unless explicitly asked. Create branches from `main` using `feat/`, `fix/`, `docs/`, `chore/`, or `refactor/` prefixes and use conventional-commit subjects.
Learnt from: CR
Repo: CryptoJones/omind
Timestamp: 2026-08-01T04:53:58.135Z
Learning: Report failures honestly, including failed quality gates and skipped scope, rather than presenting incomplete work as finished.
🪛 ast-grep (0.45.0)
src/omind/bench.py
[info] 125-129: use jsonify instead of json.dumps for JSON output
Context: json.dumps(
[tool.model_dump(mode="json", exclude_none=True) for tool in tools],
ensure_ascii=False,
separators=(",", ":"),
)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 markdownlint-cli2 (0.23.1)
CHANGELOG.md
[warning] 10-10: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 15-15: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🔇 Additional comments (6)
src/omind/server.py (2)
140-140: LGTM!
462-468: 🗄️ Data Integrity & IntegrationNo stale MCP alias references remain.
tests/test_server.py (1)
41-42: LGTM!Also applies to: 188-189, 273-281, 306-311
BACKLOG.md (1)
24-27: LGTM!src/omind/bench.py (1)
13-24: LGTM!Also applies to: 110-110, 125-132, 232-232
tests/test_bench.py (1)
38-39: LGTM!
| **One item needs the user's decision before any code:** | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Reconcile the handoff status with BACKLOG.md.
This hunk removes #177 from the decision list, but AGENTS.md still says #174 and #177 were left open at Line 151 through Line 153. It also keeps #172 as the remaining decision item, while BACKLOG.md marks #172 as done at Lines 36-40. Update the handoff to list only active decisions and shipped work consistently.
🤖 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 `@AGENTS.md` around lines 132 - 133, Reconcile the handoff status across
AGENTS.md and BACKLOG.md: update the decision list and the references around
issues `#172`, `#174`, and `#177` so only active decisions remain, and shipped items
are marked consistently with BACKLOG.md. Remove stale open-status references and
do not retain `#172` as a decision item if BACKLOG.md marks it done.
| ### Removed | ||
| - Remove the deprecated `graph-path`, `graph-orphans`, `graph-dangling`, and | ||
| `graph-stats` MCP compatibility aliases after the 5.0 bridge release. Use the | ||
| unified `graph(op=path|orphans|dangling|stats)` tool; `graph-neighbors` stays. | ||
|
|
||
| ### Changed | ||
| - `omind bench` now reports the exposed MCP tool count and serialized schema | ||
| token estimate so future surface changes remain measurable. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add blank lines after the new headings.
markdownlint-cli2 reports MD022 at Line 10 and Line 15 because each heading is followed immediately by a list. Add one blank line after ### Removed and ### Changed.
Proposed fix
### Removed
+
- Remove the deprecated `graph-path`, `graph-orphans`, `graph-dangling`, and
`graph-stats` MCP compatibility aliases after the 5.0 bridge release. Use the
unified `graph(op=path|orphans|dangling|stats)` tool; `graph-neighbors` stays.
### Changed
+
- `omind bench` now reports the exposed MCP tool count and serialized schema
token estimate so future surface changes remain measurable.📝 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.
| ### Removed | |
| - Remove the deprecated `graph-path`, `graph-orphans`, `graph-dangling`, and | |
| `graph-stats` MCP compatibility aliases after the 5.0 bridge release. Use the | |
| unified `graph(op=path|orphans|dangling|stats)` tool; `graph-neighbors` stays. | |
| ### Changed | |
| - `omind bench` now reports the exposed MCP tool count and serialized schema | |
| token estimate so future surface changes remain measurable. | |
| ### Removed | |
| - Remove the deprecated `graph-path`, `graph-orphans`, `graph-dangling`, and | |
| `graph-stats` MCP compatibility aliases after the 5.0 bridge release. Use the | |
| unified `graph(op=path|orphans|dangling|stats)` tool; `graph-neighbors` stays. | |
| ### Changed | |
| - `omind bench` now reports the exposed MCP tool count and serialized schema | |
| token estimate so future surface changes remain measurable. |
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 10-10: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 15-15: 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` around lines 10 - 17, Add a blank line immediately after the
“### Removed” and “### Changed” headings in the changelog, keeping their
existing list content unchanged.
Source: Linters/SAST tools
| omind search "why did release signing fail" --explain | ||
| omind reindex --index-only # refresh the derived index (search does this too) | ||
| omind bench # latency and token cost on your own vault | ||
| omind bench # latency, MCP schema size, and token cost |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document both MCP surface measurements.
omind bench reports MCP tools exposed and MCP tool schemas. The command comment mentions schema size but omits the tool count. Update the description so it matches the benchmark output.
Proposed fix
-omind bench # latency, MCP schema size, and token cost
+omind bench # latency, MCP tool count/schema size, and token cost📝 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.
| omind bench # latency, MCP schema size, and token cost | |
| omind bench # latency, MCP tool count/schema size, and token cost |
🤖 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 `@README.md` at line 408, Update the README command comment for omind bench to
mention both MCP measurements: the number of tools exposed and the MCP tool
schema size, while preserving the existing latency and token cost descriptions.
| def test_report_measures_the_reduced_mcp_surface(tmp_path: Path) -> None: | ||
| report = bench.run(_vault(tmp_path), queries=("nebraska",)) | ||
| by_name = {m.name: m.value for m in report.measurements} | ||
| assert by_name["MCP tools exposed"] == 13 | ||
| assert by_name["MCP tool schemas"] > 0 |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
Protect the schema-token metric with a budget.
The assertion MCP tool schemas > 0 only proves that serialization returned data. It does not detect schema growth or shrinkage. Assert the recorded 2,114-token baseline, or use a documented tolerance or upper bound when the MCP dependency can vary.
🤖 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_bench.py` around lines 49 - 53, Update
test_report_measures_the_reduced_mcp_surface so the “MCP tool schemas”
measurement is checked against the documented 2,114-token baseline, using an
explicitly documented tolerance or upper bound if dependency variation requires
it, instead of only asserting it is positive.
Summary
graph-path,graph-orphans,graph-dangling, andgraph-statsMCP aliases after the 5.0 bridge releasegraphsurface andgraph-neighborsomind bench(13 tools; 2,114 estimated schema tokens in the disposable benchmark)Closes #181.
Verification
uv run ruff check .uv run mypy srcuv run pip-audit— no known vulnerabilitiesuv run pytest -q— 841 passed, 1 skippedBuzz channel: Engineering (
08358f80-03c1-4eba-b19b-1295099d5215)Proudly Made in Nebraska. Go Big Red! 🌽 https://xkcd.com/2347/