Conversation
Add 7 needs_warnings quality gates to the scaffold conf.py template so that new workspaces created via 'memory init' get automatic Sphinx-native validation. Warnings include: missing_topic_tag, empty_body, deprecated_without_supersede, tag_case_mismatch, missing_repo_tag, isolated_decision, and draft_too_short.
…memory warning - Add owner to memory_update MCP schema, review_days to memory_add MCP schema - Add TYPE_DEFAULT_CONFIDENCE with per-type defaults (fact/dec/pref=high, q=low) - Richer confidence description in memory_add tool - Expanded relates description encouraging specific link types over generic relates - Add suspicious_high_confidence and isolated_memory needs_warnings - Add test_type_default_confidence_covers_all_types
There was a problem hiding this comment.
Pull request overview
This PR significantly streamlines the AI Memory Protocol codebase by removing ~3,300 LOC of dead code (capture, planner, executor modules and their tests) while adding core editing capabilities for memory body text and titles. The changes migrate quality checks from code-based "planner detectors" to Sphinx-native needs_warnings configuration, reducing complexity while improving build-time validation.
Changes:
- Deleted capture.py, planner.py, executor.py modules and their tests (~3,300 LOC)
- Added body and title editing functions (update_body_in_rst, update_title_in_rst) with full test coverage
- Introduced type-aware confidence defaults (fact/dec/pref=high, mem/risk/goal=medium, q=low)
- Added 9 Sphinx-native warning rules in scaffold template for build-time validation
- Removed 5 MCP tools (capture_git, capture_ci, capture_discussion, plan, apply) and 1 CLI command (doctor)
- Added owner, review_days, body, and title parameters to memory_update for API/CLI parity
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ai_memory_protocol/capture.py | Deleted: git/CI/discussion memory capture module (829 LOC) |
| src/ai_memory_protocol/planner.py | Deleted: maintenance plan detection algorithms (490 LOC) |
| src/ai_memory_protocol/executor.py | Deleted: plan execution and rollback logic (448 LOC) |
| src/ai_memory_protocol/rst.py | Added update_body_in_rst and update_title_in_rst functions for content editing |
| src/ai_memory_protocol/scaffold.py | Added 9 needs_warnings rules for Sphinx-native validation |
| src/ai_memory_protocol/config.py | Added TYPE_DEFAULT_CONFIDENCE mapping for type-aware defaults |
| src/ai_memory_protocol/mcp_server.py | Removed 5 tools, added body/title/owner parameters, applied type-aware confidence defaults |
| src/ai_memory_protocol/cli.py | Removed doctor/capture/plan/apply commands, added --body/--title to update command |
| tests/test_capture.py | Deleted: 744 LOC of capture module tests |
| tests/test_planner.py | Deleted: 555 LOC of planner module tests |
| tests/test_executor.py | Deleted: 283 LOC of executor module tests |
| tests/test_cli.py | Removed doctor command test |
| tests/test_rst.py | Added comprehensive tests for body/title editing functions |
| tests/test_scaffold.py | Added test verifying needs_warnings in generated conf.py |
| tests/test_config.py | Added test ensuring TYPE_DEFAULT_CONFIDENCE covers all types |
| tests/test_mcp_server.py | Updated tool count and verified removed tools are absent |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Fix CLI/MCP confidence parity: cmd_add uses TYPE_DEFAULT_CONFIDENCE - Update README: remove stale capture/planner/executor references - Bump version to 0.3.1 in __init__.py and pyproject.toml - Sanitize title input in update_title_in_rst (strip newlines) - Update memory_update MCP description to mention body/title - Add owner to MCP memory_add schema and handler - Move lazy imports to top-level in mcp_server.py and cli.py - Update cmd_update no-change hint to list all flags - Change tool count assertion to exact (== 8) - Add edge case tests for body/title editing - Add scaffold test assertions for new warnings
- Remove `memory doctor` from CI smoke test (command removed in v0.3.1) - Simplify isolated_memory warning expression using sum of lengths - Fix ruff formatting in test_rst.py and test_scaffold.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
needs_warningsin scaffold template and live workspace conf.py (7 → 9 warnings)memory_update(MCP + CLI)ownerin update,review_daysin addrelatesandconfidencefieldssuspicious_high_confidenceandisolated_memoryneeds_warningsMetrics
Test plan