Skip to content

feat(cli): add --type to the write-note tool command#907

Merged
phernandez merged 1 commit into
mainfrom
feat/cli-write-note-type
Jun 8, 2026
Merged

feat(cli): add --type to the write-note tool command#907
phernandez merged 1 commit into
mainfrom
feat/cli-write-note-type

Conversation

@phernandez
Copy link
Copy Markdown
Member

Summary

Adds a --type option to bm tool write-note, exposing the MCP write_note note_type parameter (which the CLI wrapper previously didn't surface). Locks in note_type behavior with integration tests so a future entity_service change can't silently break typed-note callers (e.g. the Claude Code memory bridge). Closes #875 (the descoped scope: passthrough flag + tests only).

What changed

  • src/basic_memory/cli/commands/tool.py: added a note_type typer.Option bound to --type (default "note") on the write-note command and passed note_type=note_type into mcp_write_note(...). Help text documents that a type: in the note's own content frontmatter takes precedence over --type (preserving today's entity_service._apply_schema_frontmatter_overrides behavior). Per the maintainer's descope, the "explicit --type wins over content frontmatter" precedence sentinel was intentionally NOT implemented.
  • test-int/mcp/test_write_note_type_integration.py (new, real harness, no mocks):
    • content frontmatter type: session persists and is returned by search_notes(note_types=["session"]).
    • update path: writing type: session then overwriting with type: schema flips the persisted type to schema.
  • test-int/cli/test_cli_tool_write_note_type_integration.py (new): bm tool write-note ... --type guide round-trips — read-note --include-frontmatter shows type: guide and search-notes --type guide finds it; a content-frontmatter type: wins over the --type flag (documented behavior).
  • tests/cli/test_cli_tool_json_output.py: mock-based assertions that --type forwards note_type and that it defaults to "note".

Testing (commands + results)

  • uv run ruff check . --fix / uv run ruff format . — clean.
  • uv run ty check src tests test-int — All checks passed!
  • uv run pytest tests/cli/test_cli_tool_json_output.py::test_write_note_type_passthrough tests/cli/test_cli_tool_json_output.py::test_write_note_type_defaults_to_note test-int/mcp/test_write_note_type_integration.py test-int/cli/test_cli_tool_write_note_type_integration.py -q --no-cov — 6 passed.
  • Regression: uv run pytest tests/cli/test_cli_tool_json_output.py test-int/cli/test_cli_tool_json_integration.py test-int/cli/test_search_notes_meta_integration.py tests/mcp/test_tool_write_note.py test-int/mcp/test_write_note_integration.py -q --no-cov — all passed.
  • Coverage: the new tool.py lines (--type option + note_type= kwarg) are not in the missing-lines set for cli/commands/tool.py. Did not run the Postgres variants — the change is in the CLI passthrough layer, not the search/db layer.

Risk

Low. Additive CLI option with a default that preserves prior behavior; no change to the entity/persistence layer. Content-frontmatter type: precedence is unchanged and documented.

Closes #875

🤖 Generated with Claude Code

Expose the MCP write_note note_type parameter through the CLI wrapper as
a --type option (defaults to "note"), so callers can state the note type
explicitly instead of relying on content frontmatter derivation.

A 'type:' in the note's own content frontmatter still takes precedence over
--type (entity_service _apply_schema_frontmatter_overrides), preserving
existing behavior; the help text documents this.

Adds integration coverage (real harness, no mocks) locking in note_type
round-trip behavior: content-frontmatter type persists and is searchable via
the note_types filter, the update path flips the persisted type, and the CLI
--type round-trips through read-note/search-notes. Mock-based CLI tests assert
the --type passthrough and the "note" default.

Closes #875

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: phernandez <paul@basicmachines.co>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 7, 2026

Claude finished @phernandez's task in 1m 11s —— View job


PR Review: feat(cli): add --type to the write-note tool command

Tasks

  • Read context and understand the request
  • Read changed files
  • Run team checklist review
  • Post final review

Summary

Clean, focused change that exposes the existing note_type MCP parameter through the CLI. The implementation is correct and the tests cover the documented behavior well.


Team Checklist

Item
Follows Basic Memory coding conventions (CLAUDE.md)
Python 3.12+ type annotations and async patterns
SQLAlchemy 2.0 — no DB changes needed
FastAPI and Typer conventions followed
100-character line length maintained
No commented-out code
Unit tests added (tests/cli/test_cli_tool_json_output.py)
Integration tests added (test-int/cli/, test-int/mcp/)
Edge cases covered (default value, content-frontmatter precedence, overwrite type flip)
100% coverage — new lines exercised by new tests
No database changes; no Alembic migration needed
Local-first architecture preserved
No hardcoded secrets or credentials
No path comparison issues; no emoji/unicode in console output

Implementation (src/basic_memory/cli/commands/tool.py:80-89)

The note_type parameter placement and Typer binding are correct. Using Annotated[str, typer.Option("--type", ...)] to bind to the explicit --type flag name (rather than relying on name derivation) is the right approach, especially since the variable name is note_type.

The help string documents the content-frontmatter-wins behavior inline, which is the right place for it:

"A 'type:' in the note's own content frontmatter takes precedence."

The default of "note" is consistent with the MCP tool's own default.


Tests

tests/cli/test_cli_tool_json_output.py — Good mock-based passthrough tests. The two tests are minimal and targeted: one confirms note_type is forwarded, one confirms the default. ✅

test-int/cli/test_cli_tool_write_note_type_integration.py — Real harness, no mocks. Both tests are well-named and cover the two documented behaviors:

  1. --type guide round-trips through read and search.
  2. Content frontmatter type: session wins over --type guide.

Minor nit: the --local flag is passed to search-notes on line 51 but omitted from the preceding write-note and read-note invocations. This is harmless in the test environment (all commands default to local routing) but is slightly inconsistent. Not worth a block.

test-int/mcp/test_write_note_type_integration.py — Covers the underlying MCP-layer persistence behavior that the CLI passthrough depends on. The overwrite-flips-type test (test_write_note_overwrite_flips_persisted_type) is particularly valuable as regression coverage for the conflict-resolution update path.


No Issues Found

The PR is additive-only. The default "note" preserves prior behavior for all existing callers. No changes to the entity/persistence layer, no new API surface, no migration required.

Verdict: ✅ Looks good to merge.

@phernandez phernandez merged commit 0ef03ed into main Jun 8, 2026
40 of 41 checks passed
@phernandez phernandez deleted the feat/cli-write-note-type branch June 8, 2026 01:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

write-note: add explicit --type arg; lock in content-frontmatter note_type with an integration test

1 participant