fix(mcp): validate all schema-covered types when schema_validate gets no arguments - #1060
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 670dc89e82
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review recheck |
670dc89 to
a8d6ea1
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8d6ea135c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… no arguments schema_validate() called without note_type or identifier returned an empty report, which the MCP tool rendered as the misleading "No Notes Found of Type 'unknown'" guidance (#1013). Router: when neither parameter is given, enumerate every note type that has a schema note (matching the frontmatter entity field against stored note types via generate_permalink normalization, the same rule implicit type lookup uses), validate all of them, and return an aggregated ValidationReport with a new per-type type_summaries breakdown. The shared per-entity validation loop is extracted into _validate_note_entities. MCP tool: detect all-types mode up front. With no schemas defined, return "No Schemas Defined" guidance (or a clear JSON error) instead of the 'unknown' message; otherwise render the report with a By Type section (e.g. person: 1/1 valid, meeting: no notes). The CLI's 'bm schema validate' with no target flows through the same path and now works as its help text always claimed. Fixes #1013 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: phernandez <paul@basicmachines.co>
Signed-off-by: phernandez <paul@basicmachines.co>
a8d6ea1 to
3e2d905
Compare
|
All current review findings are addressed on a main-rebased head. All-types validation now discovers implicit schema targets, inline schemas, and explicit cross-type schema references. The full schema router suite passes (22 tests), with Ruff, formatting, type checking, and diff checks clean. @codex review\n\n/recheck |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
schema_validate()called with noidentifierand nonote_typenow validates every note type that has a schema defined, returning a per-type breakdown — instead of reporting the misleading "No Notes Found of Type 'unknown'".Root cause
src/basic_memory/api/v2/routers/schema_router.py: the batch path wasentities = await _find_by_note_type(...) if note_type else []— with nonote_typethere was no all-types branch, so the router always returned an empty report.src/basic_memory/mcp/tools/schema.py:effective_type = note_type or result.note_type or "unknown"fed the empty report into thetotal_entities == 0guard, rendering "No Notes Found of Type 'unknown'".Fix
entityfield, matched against stored note types viagenerate_permalinknormalization — the same rule implicit type lookup uses), validate entities across all of them, and return an aggregatedValidationReportwith a newtype_summariesfield (TypeValidationSummaryper type). The per-entity validation loop is extracted into_validate_note_entitiesand shared with the by-type path.ValidationReportgainstype_summaries: list[TypeValidationSummary](defaults to empty, backward compatible).## By Typesection, e.g.person: 1/1 valid,meeting: no notes.bm schema validatewith no target flows through the same path and now works as its help text always claimed.Test plan
tests/api/v2/test_schema_router.py: all-types with two schemas (one with notes, one without), no schemas defined, and target-type normalization (entity: Personcovering snake_casepersonnotes).tests/mcp/test_tool_schema.py: no-args text + JSON output with schemas present, and "No Schemas Defined" guidance + JSON error with none defined (regression for the 'unknown' message).uv run pytest tests/api/v2/test_schema_router.py tests/mcp/test_tool_schema.py tests/cli/test_cli_schema.py tests/mcp/test_client_schema.py— 63 passed.uv run ruff check src tests test-int,uv run ruff format,just typecheck— clean.Fixes #1013
🤖 Generated with Claude Code