refactor: remove cipher/playbook references, migrate to mem0#77
Merged
Conversation
…r skip logic - Replace `cat > ... << 'EVIDENCE'` heredoc patterns with Write tool instructions in actor, monitor, and predictor agent templates - Add predictor skip logic: skip for low-risk or medium-risk subtasks that only create new files with complexity <= 4 - Add tier_hint routing from orchestrator to predictor agent to avoid redundant triage re-derivation - Fix minor formatting in hook templates (black-compatible) - Sync all changes to src/mapify_cli/templates/
Remove legacy dual-memory system (cipher MCP + playbook.db) remnants across the entire codebase, leaving mem0 as the sole pattern storage. - Delete docs/research/, presentation/, templates/agents.backup/ - Delete playbook_delta_operations.json, playbook-system.md, curator_operations.json - Update 8 agent templates: replace "playbook" with "mem0 patterns" - Update 5 command templates: replace "Playbook Context" with "mem0 Context" - Update 7 skills files: remove playbook-system.md links, update workflows - Update settings.json: remove sqlite3 playbook.db auto-approval - Update .gitignore: remove playbook.db ignore patterns - Update 7 doc files: rewrite playbook refs to mem0 across ARCHITECTURE, USAGE, INSTALL, CLI refs, COMPLETE_WORKFLOW - Remove SCHEMA_V3_0_SQL from schemas.py, PlaybookManager refs from graph_query.py, contradiction_detector.py, entity_extractor.py, relationship_detector.py, __init__.py - Update test data in test_relationship_detector.py and test_agent_cli_correctness.py - Inline KG schema in test_contradiction_detector.py - Sync all .claude/ changes to src/mapify_cli/templates/ All 709 tests pass. Only intentional "playbook" references remain in CHANGELOG.md (historical) and CLI error-handling for the removed command.
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis PR migrates repository terminology and workflows from a legacy playbook-centric model to mem0 MCP-based pattern storage, removes multiple legacy playbook/KG modules and presentation docs, adds predictor skip/tier_hint decision logic with Write-tool evidence paths, and updates many agent/command templates and CLI references accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Orchestrator
participant DecisionLogic as Decision Logic / Template
participant Predictor
participant WriteTool as Write Tool
participant mem0
rect rgba(63, 81, 181, 0.5)
Orchestrator->>DecisionLogic: provide subtask (risk_level, complexity_score, files_changed, security_critical)
DecisionLogic->>DecisionLogic: compute skip_predictor & tier_hint
alt skip_predictor == true
DecisionLogic->>WriteTool: write minimal predictor_evidence.json (absolute .map path)
WriteTool->>Orchestrator: confirm evidence written
else skip_predictor == false
DecisionLogic->>Predictor: invoke with tier_hint and MAP inputs
Predictor->>mem0: (optional) quote matching mem0 pattern
Predictor-->>Orchestrator: return tier verdict
end
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request completes the migration from legacy playbook.db-based pattern storage to mem0 MCP as the sole pattern storage system. The changes systematically remove references to "cipher" and "playbook" throughout the codebase, replacing them with "mem0" or generic "pattern store" terminology.
Changes:
- Removed SCHEMA_V3_0_SQL constant from schemas.py (SQLite schema for Knowledge Graph)
- Updated 32+ files across agent templates, command templates, skills, documentation, and Python source code
- Deleted legacy directories (docs/research/, presentation/, templates/agents.backup/)
- Inlined KG schema in test files for backward compatibility testing
- Renamed entity references in relationship detector tests (playbook.db → pattern-store)
Reviewed changes
Copilot reviewed 87 out of 88 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_relationship_detector.py | Updated test entity names from playbook-related to pattern-store terminology |
| tests/test_contradiction_detector.py | Inlined KG schema for testing; removed dependency on schemas.SCHEMA_V3_0_SQL |
| tests/test_agent_cli_correctness.py | Removed tests for deprecated playbook CLI commands |
| src/mapify_cli/schemas.py | Removed SCHEMA_V3_0_SQL constant; file now only contains JSON schemas |
| src/mapify_cli/*.py | Updated comments and docstrings to reference mem0 instead of playbook |
| docs/*.md | Updated documentation references from playbook to mem0/pattern store |
| .claude/agents/*.md | Updated agent templates to reference mem0 patterns instead of playbook |
| .claude/commands/*.md | Updated command templates to reference mem0 patterns instead of playbook |
| .claude/skills/*.md | Updated skills documentation to reference mem0 patterns instead of playbook |
| .gitignore | Removed playbook.db related entries |
| src/mapify_cli/templates/settings.json | Removed auto-approved sqlite3 playbook.db command |
Comments suppressed due to low confidence (1)
.claude/agents/curator.md:788
- The code example references an undefined variable
db_connon line 784. The comment on line 780 states "Patterns stored in mem0 (no local DB needed)" which contradicts the use ofdb_connparameter.
This code example appears to be outdated and inconsistent with the migration to mem0. Either:
- The example should be updated to not require a database connection if patterns are truly in mem0
- Or the comment should clarify that a legacy database is still needed for Knowledge Graph queries despite pattern storage being in mem0
- Or this example section should be removed entirely if contradiction detection no longer works this way
The inconsistency between the comment and the code will confuse users of this agent template.
# Patterns stored in mem0 (no local DB needed)
# Check for conflicts with existing knowledge graph data
conflicts = check_new_pattern_conflicts(
db_conn=db_conn,
pattern_text=bullet_content,
entities=entities,
min_confidence=0.7 # Only high-confidence conflicts
)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/test_contradiction_detector.py
Outdated
Comment on lines
30
to
81
| # Knowledge Graph schema (inlined; was SCHEMA_V3_0_SQL before removal from schemas.py) | ||
| _KG_SCHEMA_SQL = """ | ||
| CREATE TABLE IF NOT EXISTS entities ( | ||
| id TEXT PRIMARY KEY, | ||
| type TEXT NOT NULL CHECK(type IN ('TOOL','PATTERN','CONCEPT','ERROR_TYPE','TECHNOLOGY','WORKFLOW','ANTIPATTERN')), | ||
| name TEXT NOT NULL, | ||
| first_seen_at TEXT NOT NULL, | ||
| last_seen_at TEXT NOT NULL, | ||
| confidence REAL NOT NULL DEFAULT 0.8 CHECK(confidence >= 0.0 AND confidence <= 1.0), | ||
| metadata TEXT, | ||
| created_at TEXT NOT NULL DEFAULT (datetime('now')), | ||
| updated_at TEXT NOT NULL DEFAULT (datetime('now')) | ||
| ); | ||
| CREATE INDEX IF NOT EXISTS idx_entities_type ON entities(type); | ||
| CREATE INDEX IF NOT EXISTS idx_entities_name ON entities(name COLLATE NOCASE); | ||
|
|
||
| CREATE TABLE IF NOT EXISTS relationships ( | ||
| id TEXT PRIMARY KEY, | ||
| source_entity_id TEXT NOT NULL, | ||
| target_entity_id TEXT NOT NULL, | ||
| type TEXT NOT NULL CHECK(type IN ('USES','DEPENDS_ON','CONTRADICTS','SUPERSEDES','RELATED_TO','IMPLEMENTS','CAUSES','PREVENTS','ALTERNATIVE_TO')), | ||
| created_from_bullet_id TEXT NOT NULL, | ||
| confidence REAL NOT NULL DEFAULT 0.8 CHECK(confidence >= 0.0 AND confidence <= 1.0), | ||
| metadata TEXT, | ||
| created_at TEXT NOT NULL DEFAULT (datetime('now')), | ||
| updated_at TEXT NOT NULL DEFAULT (datetime('now')), | ||
| FOREIGN KEY (source_entity_id) REFERENCES entities(id) ON DELETE CASCADE, | ||
| FOREIGN KEY (target_entity_id) REFERENCES entities(id) ON DELETE CASCADE, | ||
| FOREIGN KEY (created_from_bullet_id) REFERENCES bullets(id) ON DELETE CASCADE, | ||
| UNIQUE(source_entity_id, target_entity_id, type) | ||
| ); | ||
| CREATE INDEX IF NOT EXISTS idx_rel_source ON relationships(source_entity_id, type); | ||
| CREATE INDEX IF NOT EXISTS idx_rel_target ON relationships(target_entity_id, type); | ||
|
|
||
| CREATE TABLE IF NOT EXISTS provenance ( | ||
| id TEXT PRIMARY KEY, | ||
| entity_id TEXT, | ||
| relationship_id TEXT, | ||
| source_bullet_id TEXT NOT NULL, | ||
| extraction_method TEXT NOT NULL CHECK(extraction_method IN ('MANUAL','NLP_REGEX','LLM_GPT4','LLM_CLAUDE','RULE_BASED')), | ||
| extraction_confidence REAL NOT NULL DEFAULT 0.8, | ||
| extracted_at TEXT NOT NULL, | ||
| metadata TEXT, | ||
| FOREIGN KEY (entity_id) REFERENCES entities(id) ON DELETE CASCADE, | ||
| FOREIGN KEY (relationship_id) REFERENCES relationships(id) ON DELETE CASCADE, | ||
| FOREIGN KEY (source_bullet_id) REFERENCES bullets(id) ON DELETE CASCADE, | ||
| CHECK((entity_id IS NOT NULL AND relationship_id IS NULL) OR (entity_id IS NULL AND relationship_id IS NOT NULL)) | ||
| ); | ||
|
|
||
| INSERT OR REPLACE INTO metadata (key, value) VALUES ('schema_version', '3.0'); | ||
| INSERT OR IGNORE INTO metadata (key, value) VALUES ('kg_enabled', '1'); | ||
| """ |
There was a problem hiding this comment.
The inlined SQL schema _KG_SCHEMA_SQL duplicates the schema definition that was removed from schemas.py. This creates a maintenance burden - if the schema needs updates, it must be changed in multiple places. Consider one of these alternatives:
- Keep a minimal schema constant in
schemas.pyspecifically for testing (if tests are the only remaining user) - Use a shared test fixture/helper that loads the schema from a single source
- Document why this duplication is intentional if the schema is truly legacy and frozen
This is especially important since the PR description states "SCHEMA_V3_0_SQL removed" - having it inlined here contradicts that intent.
tests/test_contradiction_detector.py
Outdated
Comment on lines
30
to
81
| # Knowledge Graph schema (inlined; was SCHEMA_V3_0_SQL before removal from schemas.py) | ||
| _KG_SCHEMA_SQL = """ | ||
| CREATE TABLE IF NOT EXISTS entities ( | ||
| id TEXT PRIMARY KEY, | ||
| type TEXT NOT NULL CHECK(type IN ('TOOL','PATTERN','CONCEPT','ERROR_TYPE','TECHNOLOGY','WORKFLOW','ANTIPATTERN')), | ||
| name TEXT NOT NULL, | ||
| first_seen_at TEXT NOT NULL, | ||
| last_seen_at TEXT NOT NULL, | ||
| confidence REAL NOT NULL DEFAULT 0.8 CHECK(confidence >= 0.0 AND confidence <= 1.0), | ||
| metadata TEXT, | ||
| created_at TEXT NOT NULL DEFAULT (datetime('now')), | ||
| updated_at TEXT NOT NULL DEFAULT (datetime('now')) | ||
| ); | ||
| CREATE INDEX IF NOT EXISTS idx_entities_type ON entities(type); | ||
| CREATE INDEX IF NOT EXISTS idx_entities_name ON entities(name COLLATE NOCASE); | ||
|
|
||
| CREATE TABLE IF NOT EXISTS relationships ( | ||
| id TEXT PRIMARY KEY, | ||
| source_entity_id TEXT NOT NULL, | ||
| target_entity_id TEXT NOT NULL, | ||
| type TEXT NOT NULL CHECK(type IN ('USES','DEPENDS_ON','CONTRADICTS','SUPERSEDES','RELATED_TO','IMPLEMENTS','CAUSES','PREVENTS','ALTERNATIVE_TO')), | ||
| created_from_bullet_id TEXT NOT NULL, | ||
| confidence REAL NOT NULL DEFAULT 0.8 CHECK(confidence >= 0.0 AND confidence <= 1.0), | ||
| metadata TEXT, | ||
| created_at TEXT NOT NULL DEFAULT (datetime('now')), | ||
| updated_at TEXT NOT NULL DEFAULT (datetime('now')), | ||
| FOREIGN KEY (source_entity_id) REFERENCES entities(id) ON DELETE CASCADE, | ||
| FOREIGN KEY (target_entity_id) REFERENCES entities(id) ON DELETE CASCADE, | ||
| FOREIGN KEY (created_from_bullet_id) REFERENCES bullets(id) ON DELETE CASCADE, | ||
| UNIQUE(source_entity_id, target_entity_id, type) | ||
| ); | ||
| CREATE INDEX IF NOT EXISTS idx_rel_source ON relationships(source_entity_id, type); | ||
| CREATE INDEX IF NOT EXISTS idx_rel_target ON relationships(target_entity_id, type); | ||
|
|
||
| CREATE TABLE IF NOT EXISTS provenance ( | ||
| id TEXT PRIMARY KEY, | ||
| entity_id TEXT, | ||
| relationship_id TEXT, | ||
| source_bullet_id TEXT NOT NULL, | ||
| extraction_method TEXT NOT NULL CHECK(extraction_method IN ('MANUAL','NLP_REGEX','LLM_GPT4','LLM_CLAUDE','RULE_BASED')), | ||
| extraction_confidence REAL NOT NULL DEFAULT 0.8, | ||
| extracted_at TEXT NOT NULL, | ||
| metadata TEXT, | ||
| FOREIGN KEY (entity_id) REFERENCES entities(id) ON DELETE CASCADE, | ||
| FOREIGN KEY (relationship_id) REFERENCES relationships(id) ON DELETE CASCADE, | ||
| FOREIGN KEY (source_bullet_id) REFERENCES bullets(id) ON DELETE CASCADE, | ||
| CHECK((entity_id IS NOT NULL AND relationship_id IS NULL) OR (entity_id IS NULL AND relationship_id IS NOT NULL)) | ||
| ); | ||
|
|
||
| INSERT OR REPLACE INTO metadata (key, value) VALUES ('schema_version', '3.0'); | ||
| INSERT OR IGNORE INTO metadata (key, value) VALUES ('kg_enabled', '1'); | ||
| """ |
There was a problem hiding this comment.
The comment states "Knowledge Graph schema (inlined; was SCHEMA_V3_0_SQL before removal from schemas.py)" which documents this is intentional inlining. However, the comment on line 780 states "Patterns stored in mem0 (no local DB needed)" which creates confusion - if patterns are in mem0 and no local DB is needed, why does this test need the full KG schema with entities, relationships, and provenance tables?
Consider whether these tests should be updated to not require the legacy KG schema at all, or clearly document why the legacy schema is still needed for testing purposes despite the migration to mem0.
Major changes across agent templates, commands, and documentation: - Agent templates (.claude/agents/ and templates/agents/): * Replace "Playbook" terminology with "mem0 patterns" throughout * Update curator references from playbook patterns to knowledge base patterns * Remove playbook_delta_operations.json and playbook-system.md * Clarify pattern storage uses mem0 MCP (not local playbook) - Command templates (.claude/commands/ and templates/commands/): * Update map-debate, map-debug, map-fast, map-release, map-review - Skills and workflows: * Remove playbook-system.md from map-workflows-guide resources * Update README and agent architecture documentation - Python source code (src/mapify_cli/): * Update all imports and type hints referencing schemas * Adjust entity_extractor, relationship_detector, contradiction_detector * Update __init__.py version and module exports - Tests (tests/): * Update test fixtures to match new terminology * Fix test assertions for agent behavior changes - Documentation (docs/): * Update USAGE.md, ARCHITECTURE.md, CLI_REFERENCE.json * Update INSTALL.md and COMPLETE_WORKFLOW.md - Presentation materials: * Remove presentation/ directory (no longer maintained) - Configuration: * Update settings.json and .gitignore * Delete unused backup directories This commit consolidates the v4.0 migration from local playbook storage to mem0 MCP-based pattern management and removes obsolete terminology.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
.claude/skills/map-workflows-guide/resources/map-fast-deep-dive.md (1)
1-240:⚠️ Potential issue | 🟠 MajorEnsure
.claude/changes are synced to templates.
This file lives under.claude/, so it must be copied to the matching path undersrc/mapify_cli/templates/before finish (e.g., viamake sync-templates, then confirm withgit diff/git status).As per coding guidelines: “If you change anything under
.claude/that is shipped to users, you MUST copy it to the matching path undersrc/mapify_cli/templates/before finishing.”src/mapify_cli/templates/agents/curator.md (1)
776-787:⚠️ Potential issue | 🟡 MinorClarify conflict-check data source after mem0 migration.
Line 780 says no local DB is needed, yet the example still imports
sqlite3and passesdb_conn. That’s confusing after the mem0 migration. Either remove the sqlite example or explain howdb_connis obtained from mem0 (e.g., an exported KG or adapter).✏️ Example clarification
-import sqlite3 - -from mapify_cli.contradiction_detector import check_new_pattern_conflicts - -# Patterns stored in mem0 (no local DB needed) +from mapify_cli.contradiction_detector import check_new_pattern_conflicts + +# Patterns stored in mem0. Provide a mem0-backed KG/adapter connection here..claude/agents/curator.md (1)
776-784:⚠️ Potential issue | 🟡 Minor
db_connis referenced without being defined in the contradiction check snippet.
The snippet now says no local DB is needed but still passesdb_conn, which is undefined. Either reintroduce the KG connection or remove the parameter if the API no longer requires it.✍️ Suggested fix (define the KG connection explicitly)
-import sqlite3 +import sqlite3 @@ -# Patterns stored in mem0 (no local DB needed) +# Patterns stored in mem0; Knowledge Graph remains local for contradiction checks +db_conn = sqlite3.connect(".claude/knowledge_graph.db")docs/CLI_COMMAND_REFERENCE.md (1)
236-242:⚠️ Potential issue | 🟠 Majormem0 MCP example parameters don't match current tool signatures.
The example usescontent/category/tierandpattern_id, while curator templates showtext,section,user_id,run_id,agent_origin,scope, andmemory_id. This mismatch can mislead users and tooling that consume this reference.✍️ Suggested update to align with mem0 MCP signatures
-# Curator stores patterns via mem0 MCP: -mcp__mem0__map_add_pattern(content="...", category="implementation", tier="project") +# Curator stores patterns via mem0 MCP: +mcp__mem0__map_add_pattern( + text="...", + user_id="org:ORG_NAME", + run_id="proj:PROJECT_NAME:branch:BRANCH_NAME", + section="IMPLEMENTATION_PATTERNS", + agent_origin="curator", + scope="branch" +) @@ -mcp__mem0__map_archive_pattern(pattern_id="impl-0042", reason="Superseded") +mcp__mem0__map_archive_pattern( + memory_id="mem_impl0042", + reason="Superseded", + archived_by="curator" +)
🤖 Fix all issues with AI agents
In @.claude/agents/monitor.md:
- Around line 444-447: The fenced code block under the "Pattern Conflict
Resolution" section (the block starting with IF mem0 pattern conflicts with
dimension requirement:) lacks a language tag and triggers markdownlint MD040;
update that fenced block to include a language identifier such as `text` (e.g.,
change ``` to ```text) so the markdown linter recognizes the fence language.
In @.claude/commands/map-debate.md:
- Around line 322-338: The skip_predictor logic can still bypass Predictor when
subtask.escalation_required is true; update the condition so escalation_required
always forces a call. Modify the skip_predictor expression (the variable named
skip_predictor that uses subtask.risk_level, subtask.affected_files,
subtask.complexity_score, and subtask.security_critical) to additionally require
that subtask.escalation_required is false (e.g., by ANDing a check like not
subtask.escalation_required into the combined skip condition or by explicitly
short-circuiting: only allow skipping when escalation_required is false).
In @.claude/commands/map-efficient.md:
- Around line 347-376: The skip logic for predictor (variable skip_predictor)
erroneously treats an empty subtask.affected_files as satisfying the "all(not
file_exists(f))" check due to Python's all() vacuous truth; update the condition
used when building skip_predictor to require that subtask.affected_files is
non-empty (e.g., include a check like "and subtask.affected_files" before
calling all) so that empty affected_files no longer triggers the skip; keep
references to file_exists and subtask.complexity_score and
subtask.security_critical unchanged.
In `@CLAUDE.md`:
- Around line 74-87: In the CLAUDE.md section titled "Git commands: do NOT use
`-C` when already in the repo", replace the developer-specific absolute path
string '/Users/azalio/gitroot/azalio/map-framework' with a generic placeholder
like '/path/to/repo' wherever it appears (the two occurrences in the git example
lines) so the documentation is not tied to a single developer environment.
In `@docs/CLI_REFERENCE.json`:
- Around line 164-207: The mem0_mcp_tools schema is out of sync: update the
definitions for mem0_mcp_tools.tiered_search, .add_pattern, .archive_pattern and
.promote_pattern to match the actual signatures in curator.md—specifically add
parameters user_id, run_id, min_quality_score to tiered_search; rename
add_pattern fields to text, section, scope and add user_id, run_id,
agent_origin, code_example, tech_stack, tags; rename archive_pattern's
pattern_id to memory_id and add superseded_by and archived_by; and document
promote_pattern with memory_id, target_scope, user_id, target_run_id,
promoted_by, promotion_reason; then propagate these changes into the
higher-level documentation sections that reference mem0_mcp_tools in USAGE and
ARCHITECTURE so examples and descriptions match the new parameter names and
required fields.
In `@docs/USAGE.md`:
- Around line 377-379: The examples in the knowledge-graph snippets reference
pm.db_conn but the project switched to using db_conn; update all occurrences of
pm.db_conn (e.g., in the contradiction detection examples around the snippets at
lines mentioned) to use db_conn instead so the examples run as written, and
ensure the surrounding example shows db_conn being created/initialized (the same
handle used by detect_contradictions and any other detector methods).
In `@src/mapify_cli/templates/agents/monitor.md`:
- Around line 2501-2516: The template contradicts the read-only policy by
forbidding the Write tool while the MONITOR phase requires using the Write tool
to create <project_root>/.map/<branch>/evidence/monitor_<subtask_id>.json;
update the template so the monitor agent is allowed to emit evidence by either
(A) removing or relaxing the read-only restriction for the MONITOR phase or (B)
adding a sanctioned evidence-write action that uses a dedicated, minimal-scope
Write tool/permission named "evidence_writer" invoked by the MONITOR agent to
write the JSON with keys
"phase","subtask_id","timestamp","valid","issues_found","recommendation"; ensure
the docs in monitor.md clearly state the allowed tool name ("Write tool" or
"evidence_writer") and the required output path and JSON schema so the
enforcement rule and the evidence step are consistent.
In `@src/mapify_cli/templates/commands/map-debate.md`:
- Around line 322-339: The skip_predictor expression currently allows skipping
even when subtask.security_critical or subtask.escalation_required are true;
update the boolean logic for skip_predictor so it only evaluates to True when
neither security_critical nor escalation_required are set (i.e., require not
subtask.security_critical and not subtask.escalation_required as a prerequisite
for any skip). Locate the skip_predictor variable and the uses of
subtask.risk_level, subtask.security_critical, subtask.escalation_required,
subtask.affected_files, subtask.complexity_score and ensure the top-level skip
condition short-circuits to False whenever security_critical or
escalation_required is True, leaving the existing
low/medium/new-files/complexity checks intact otherwise.
In `@src/mapify_cli/templates/commands/map-efficient.md`:
- Around line 347-363: The skip logic for calling the predictor is missing a
safeguard for escalation: update the skip_predictor boolean so it will never be
true when subtask.escalation_required is true (i.e., add an explicit check for
not subtask.escalation_required or short-circuit so escalation_required forces
predictor call). Locate the skip_predictor expression and incorporate
subtask.escalation_required into the condition (for example, ensure
skip_predictor includes "and not subtask.escalation_required" or place a
preceding guard that returns False when subtask.escalation_required is True) so
the predictor always runs for escalations.
- Around line 365-375: The evidence block uses an undocumented tier and
string-typed confidence; update the evidence to use a documented tier and
numeric confidence: change tier_selected from "skipped" to the documented
"degraded" (or if you intentionally need "skipped", add "skipped" to the
predictor.md schema and tests) and change confidence_score from the string
"0.95" to the numeric 0.95 so it matches the documented <0.30-0.95> format and
validate_step("2.6") expectations.
🧹 Nitpick comments (2)
src/mapify_cli/templates/skills/map-cli-reference/scripts/check-command.sh (1)
10-15: Inconsistent terminology: "Removed" in help text but "deprecated" retained elsewhere.Lines 10 and 33 were updated to say "removed," but several other references still use "deprecated":
- Line 15: exit code description
2 - Command deprecated- Line 38–39:
DEPRECATED_COMMANDSvariable and comment- Line 44: comment
# Check deprecated first- Line 47: error message
is deprecated (removed in v4.0+)Consider aligning these for consistency with the rest of the PR's migration away from "deprecated" toward "removed."
Also applies to: 33-34, 38-47
tests/test_agent_cli_correctness.py (1)
63-76: Align test name/docstring with reference-only check.Line 63-76 now validates only
<mapify_cli_reference>, but the test name/docstring still say “or examples,” which is misleading.♻️ Suggested update
- def test_agents_have_cli_reference_or_examples(self, agent_files): - """Test that agents either have CLI reference section or proper examples.""" + def test_agents_have_cli_reference(self, agent_files): + """Test that agents include a CLI reference section.""" ... - # Check if agent has CLI reference section or examples + # Check if agent has CLI reference section
Comment on lines
365
to
375
| if skip_predictor: | ||
| # Write minimal evidence directly (no agent call needed) | ||
| # Use Write tool → <project_root>/.map/<branch>/evidence/predictor_<subtask_id>.json | ||
| { | ||
| "phase": "PREDICTOR", | ||
| "subtask_id": "<id>", | ||
| "timestamp": "<ISO 8601 UTC>", | ||
| "risk_assessment": "low", | ||
| "confidence_score": "0.95", | ||
| "tier_selected": "skipped", | ||
| "skip_reason": "New files only, no existing callers, complexity <= 4" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for validate_step, evidence validation, and predictor-related validation
rg -n "validate_step|evidence.*predictor|predictor_.*json|tier_selected|confidence_score" -S --max-count=50Repository: azalio/map-framework
Length of output: 4920
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the validate_step function from map_orchestrator.py
sed -n '413,500p' src/mapify_cli/templates/map/scripts/map_orchestrator.pyRepository: azalio/map-framework
Length of output: 3511
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find EVIDENCE_REQUIRED definition
rg -n "EVIDENCE_REQUIRED" -A 30 src/mapify_cli/templates/map/scripts/map_orchestrator.py | head -50Repository: azalio/map-framework
Length of output: 2116
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search predictor.md for all tier_selected values and documentation
rg -n "tier_selected|tier" src/mapify_cli/templates/agents/predictor.md | head -80Repository: azalio/map-framework
Length of output: 3254
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if there's any validation of confidence_score or tier_selected elsewhere
rg -n "tier_selected|confidence_score" -B 2 -A 2 src/mapify_cli/templates/map/scripts/map_orchestrator.pyRepository: azalio/map-framework
Length of output: 46
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if "skipped" is mentioned anywhere in predictor.md
rg -n "skip" src/mapify_cli/templates/agents/predictor.md
# Also check map-efficient.md to see the full context around line 365-375
echo "---"
echo "Context from map-efficient.md:"
sed -n '360,380p' src/mapify_cli/templates/commands/map-efficient.mdRepository: azalio/map-framework
Length of output: 940
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the full predictor evidence structure to see all documented fields
sed -n '1680,1820p' src/mapify_cli/templates/agents/predictor.mdRepository: azalio/map-framework
Length of output: 5026
Use documented tier value or update predictor.md schema.
The validate_step("2.6") function only validates three required fields—phase, subtask_id, timestamp—and will accept the minimal evidence block. However, tier_selected: "skipped" is not a documented valid tier value. Predictor documentation specifies only "1", "2", "3", or "degraded" (when tools fail). Either use the documented "degraded" tier for this skip scenario, add "skipped" to the predictor.md schema, or document why this tier is needed. Additionally, confidence_score should be numeric (e.g., 0.95) rather than a string to match the documented range format <0.30-0.95> and type consistency in other evidence files.
🤖 Prompt for AI Agents
In `@src/mapify_cli/templates/commands/map-efficient.md` around lines 365 - 375,
The evidence block uses an undocumented tier and string-typed confidence; update
the evidence to use a documented tier and numeric confidence: change
tier_selected from "skipped" to the documented "degraded" (or if you
intentionally need "skipped", add "skipped" to the predictor.md schema and
tests) and change confidence_score from the string "0.95" to the numeric 0.95 so
it matches the documented <0.30-0.95> format and validate_step("2.6")
expectations.
Delete 4 source modules (graph_query, contradiction_detector, entity_extractor, relationship_detector) and their 3 test files. These were orphaned after migrating to mem0 MCP for pattern storage. Rewrite curator.md CONTRADICTION DETECTION section to use mem0 search instead of SQLite-based Python modules. Remove KG Python API section from USAGE.md and KG section from ARCHITECTURE.md.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.claude/agents/curator.md:
- Around line 763-765: The fenced code block containing the call
mcp__mem0__map_tiered_search(query="<key terms from new pattern>") lacks a
language identifier (MD040); update the fence to include a language token (for
example "text") so the block becomes ```text ... ```, ensuring the markdownlint
warning is resolved while preserving the existing content.
In `@src/mapify_cli/templates/agents/curator.md`:
- Around line 763-765: The fenced code block containing the example call
mcp__mem0__map_tiered_search(query="<key terms from new pattern>") lacks a
language identifier and triggers markdownlint MD040; update that fenced block to
include a language identifier (e.g., "text") so the opening triple backticks
become ```text to satisfy the linter and keep the snippet unchanged.
…ates Fix skip_predictor logic to enforce escalation_required/security_critical guards and prevent vacuous all() on empty affected_files. Fix confidence_score type from string to numeric. Document "skipped" tier in predictor schema. Resolve monitor.md Write tool contradiction, align CLI_REFERENCE.json mem0 tool signatures with curator.md, rename DEPRECATED_COMMANDS to REMOVED_COMMANDS, clean up test name, and remove hardcoded developer path from CLAUDE.md.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/mapify_cli/templates/agents/predictor.md (1)
1733-1737:⚠️ Potential issue | 🟡 MinorAlign tier_selected values across sections.
The field requirements now allow
skipped, but the JSON schema example above still shows"1|2|3". Update one of them so the allowed values are consistent.Suggested update to the JSON schema example
- "tier_selected": "1|2|3", + "tier_selected": "1|2|3|skipped",.claude/agents/predictor.md (1)
1684-1811:⚠️ Potential issue | 🟡 MinorResolve schema inconsistencies for
tier_selectedandconfidence_score.
The JSON schema still statestier_selected: "1|2|3"while the field requirements includeskipped. Also, the evidence example showsconfidence_scoreas a string, but the rest of the doc/examples treat it as numeric. Align these to avoid mismatched implementations.
🤖 Fix all issues with AI agents
In @.claude/agents/monitor.md:
- Around line 2502-2515: The example evidence JSON currently uses a quoted
string for the issues_found field; change it to a numeric placeholder (unquoted)
so issues_found is emitted as a number (e.g., issues_found: 0) when writing the
monitor_<subtask_id>.json file in the Write tool step; ensure the JSON produced
by the code that constructs the object (phase, subtask_id, timestamp, valid,
issues_found, recommendation) serializes issues_found as a numeric type rather
than a string.
- Around line 23-28: Update the monitor.md wording so the Write-tool exception
is explicit and consistent: replace the current conflicting lines ("NEVER: Use
Edit or MultiEdit tools", "EXCEPTION: Write tool is permitted ONLY for evidence
artifacts (.map/ directory)", and "WHY: workflow-gate.py will BLOCK Edit/Write
during monitor phase") with a single clear statement that workflow-gate.py
blocks Edit/MultiEdit/Write by default during the monitor phase but explicitly
allows Write operations to the evidence artifacts directory (.map/) only; also
update the template text to match this phrasing so the exception cannot be
misread as a blanket block.
In `@docs/CLI_REFERENCE.json`:
- Around line 164-185: The tiered_search documentation for mem0_mcp_tools
(mcp__mem0__map_tiered_search) is inconsistent: the parameters list includes
user_id and run_id but the second example omits them; update the
CLI_REFERENCE.json entry for "mem0_mcp_tools" -> "tiered_search" so the example
either includes user_id and run_id (e.g., add user_id="org:acme-corp" and
run_id="proj:my-app:branch:feat-auth" to the second example call) or explicitly
mark user_id and run_id as optional in the "parameters" descriptions (add
"(optional)" to those fields) to make the contract clear. Ensure the change
references mcp__mem0__map_tiered_search and keep example semantics intact.
In `@src/mapify_cli/templates/agents/monitor.md`:
- Around line 2502-2515: The template currently renders "issues_found" as a
quoted string in the evidence JSON; change it to a numeric placeholder so the
field is emitted as a number (not a string) when using the Write tool to create
`<project_root>/.map/<branch>/evidence/monitor_<subtask_id>.json`; update the
JSON example in src/mapify_cli/templates/agents/monitor.md so the "issues_found"
value is an unquoted numeric placeholder (e.g., 0 or <number>) and ensure
adjacent fields ("phase", "subtask_id", "timestamp", "valid", "recommendation")
remain unchanged.
- Around line 23-28: Update the rationale so it's consistent with the Write-tool
exception: modify the "📋 WHY: workflow-gate.py will BLOCK Edit/Write during
monitor phase" line to explicitly state that workflow-gate.py blocks Edit/Write
for source files during the monitor phase but allows the Write tool for evidence
artifacts as covered by "⚠️ EXCEPTION: Write tool is permitted ONLY for evidence
artifacts (.map/ directory)"; keep the other bullets (❌ NEVER lines and 🔄 FLOW)
intact and ensure the phrasing references the .map/ directory exception to avoid
a blanket prohibition.
🧹 Nitpick comments (3)
src/mapify_cli/templates/agents/predictor.md (1)
1797-1811: Prefer numericconfidence_scorein evidence JSON.Elsewhere
confidence.scoreis numeric; keeping the evidence file numeric avoids type drift for consumers that parse the artifact.Suggested update
- "confidence_score": "<0.30-0.95>", + "confidence_score": 0.85,.claude/commands/map-efficient.md (1)
1-29: Remember to sync.claude/changes tosrc/mapify_cli/templates/.
Please runmake sync-templates(or verify withgit diff/git status) so shipped templates match.As per coding guidelines: “If you change anything under
.claude/that is shipped to users, you MUST copy it to the matching path undersrc/mapify_cli/templates/before finishing.”.claude/agents/predictor.md (1)
1-7: Sync templates and run agent template sync test.
After.claude/agents/changes, runmake sync-templatesandpytest tests/test_template_sync.py -v.As per coding guidelines: “If you change anything under
.claude/that is shipped to users, you MUST copy it to the matching path undersrc/mapify_cli/templates/before finishing.”
As per coding guidelines: “Runpytest tests/test_template_sync.py -vto enforce agent template sync verification.”
Add language tag to curator.md fenced block, clarify monitor.md workflow-gate wording for Write-tool exception, fix issues_found type from string to numeric, add user_id/run_id to second tiered_search example in CLI_REFERENCE.json.
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
1 participant
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
docs/research/,presentation/,templates/agents.backup/(32 files)SCHEMA_V3_0_SQLSQLite schema constant andPlaybookManagerreferences from Python codeTest plan
pytest tests/test_template_sync.py -v— all 13 pass (template sync verified)pytest— all 709 pass, 1 skippedgrep -ri "cipher"across .claude/, src/, docs/ — zero matchesgrep -ri "playbook"— only intentional references in CHANGELOG.md and CLI error-handling remainSummary by CodeRabbit
Documentation
New Features
Chores